Generalize Slack messaging into a MessagingPlatform interface#1
Draft
0xdiid wants to merge 5 commits into
Draft
Conversation
Move every Slack-specific bit of the API into a new platforms/ package
so additional messaging integrations (Discord, etc.) can be added by
implementing a single adapter instead of branching on platform strings
across agent.py, runtime_control.py, tool_manager.py, workflow_engine.py,
and the slack_thread_turn workflow.
New api/platforms/ package:
- MessagingPlatform base class with no-op defaults for sanitization,
identity recovery, system-prompt injection, live-streaming session
lifecycle, outbound channel posts, and active-thread tool-call capture.
- SlackPlatform owns all existing Slack behaviour: text scrubbing
(sanitize_for_slack), slackbot HTTP client, GitHub-from-Slack-profile
identity extraction, Slack formatting rules, live session + harness
events + assistant status, and slack.send_message active-thread
capture.
- DevPlatform: no-op fallback used when delivery is missing/unknown.
- Registry + resolvers: resolve_platform(name), resolve_for_delivery,
resolve_for_thread_key, resolve_for_metadata.
Production-code rewires:
- agent.py: _build_session_context and _resolve_requester_identity
dispatch via resolve_platform(); removed inline Slack-formatting and
GitHub-handle helpers (now on SlackPlatform).
- runtime_control.py: execution worker resolves platform once from
delivery and calls platform.session_done / harness_event /
assistant_status; dropped dead _send_slackbot_canonical_event and
_canonical_text_blocks helpers.
- tool_manager.py: _capture_live_slack_send → generic
_capture_active_thread_tool_call dispatching to the platform resolved
from thread_key prefix.
- workflow_engine.py: Delivery.for_channel(platform, ...) added (with
Delivery.slack as a thin wrapper); ctx.post_to_channel(platform, ...)
added (with ctx.post_to_slack wrapping it); auto-handler loader now
honors both SLACK_CHANNEL (legacy) and DELIVERY = {platform, channel}
(forward-compat); schedule loader honors a delivery field.
- workflows/slack_thread_turn.py: canonical WORKFLOW_NAME is now
messaging_thread_turn with WORKFLOW_ALIASES = ("slack_thread_turn",)
so the slackbot keeps working unchanged; _load_workflow_file registers
aliases, _workflow_request_hash treats both names identically.
- routers/agent.py: one _resolve_delivery_fallback helper for spawn +
execute; logs a deprecation note when callers rely on the implicit
default platform.
Back-compat shims:
- api/slack_sanitize.py and api/slackbot_client.py re-export the public
surface from api.platforms.slack so external imports keep working.
Tests:
- New tests/test_platforms.py covers registration, resolvers, sanitizer,
identity-line + formatting-rule generation, and Slack delivery
predicates.
- Existing tests that patched api.workflow_engine.slackbot_client.X /
api.runtime_control.slackbot_client.X now patch the platform method
directly via patch.object(SLACK_PLATFORM, ...).
- test_slackbot_client.py retargeted to patch
api.platforms.slack.{asyncio,httpx} since the HTTP client lives there.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ead code Quick-win cluster from the multi-agent review of e2a54ef: - Promote requester identity to a frozen `RequesterIdentity` dataclass (platforms/__init__.py) instead of `dict[str, str | bool]`. Field names drop the `slack_` prefix since the data is platform-agnostic; per-platform rendering ("Slack user ID: ...") stays on `system_prompt_identity_lines`. Updates `SlackPlatform.load_requester_identity`, `system_prompt_identity_lines`, and the two test files that constructed the old dict shape. - Delete dead code: `resolve_for_metadata` had zero callers; `_DEV_FALLBACK` was unreachable defensive code since `DevPlatform` registers eagerly. - Centralize platform registration via `register_builtin_platforms()` in `platforms/__init__.py`. `platforms/dev.py` and `platforms/slack.py` now only expose their singleton (`DEV_PLATFORM`, `SLACK_PLATFORM`); the registration call lives in one place. Removes the bottom-of-module side-effect imports with their `noqa: E402, F401` lies. - Document the platform-agnostic execution metadata keys (`slackbot_live_delivery`, `slackbot_agent_session_id`, etc.) in the `api.platforms` module docstring. They're Slack-named for historical reasons but read by all platforms; future schema cleanup will rename them to `live_session_*`. - Add `MessagingPlatform.intercepts_tool_call(tool_name, method_name)` sync hint. `tool_manager._capture_active_thread_tool_call` now skips the async dispatch hop into `capture_active_thread_tool_call` for any tool call the active platform doesn't intercept. `SlackPlatform` returns `True` only for `slack.send_message`, restoring the pre-PR fast path for non-send-message tool calls on Slack threads. - Dedupe schedule registration for workflows with `WORKFLOW_ALIASES` (workflow_engine.py:_registered_schedule_specs). The alias system registers the same `_RegisteredHandler` under multiple names; without deduping, a future workflow with both aliases and a `CRON`/`SCHEDULE` would fire its schedule N times per tick. Currently latent (`slack_thread_turn` has no schedule), but fixed proactively. - Narrow over-broad `except (ValueError, TypeError)` to `json.JSONDecodeError` in `SlackPlatform.send_channel_message`. Verified via uv run ruff check api/ and uv run pytest -q on test_platforms.py (17 cases including 4 new), test_integration.py, test_slackbot_client*, test_workflow_idempotency_unit.py, and the CI-targeted subset of test_workflows.py and test_agent_control_plane.py. 78/78 targeted tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously SlackPlatform.capture_active_thread_tool_call reached into ``request.app.state.db_pool`` and ran a raw SQL query against ``agent_execution_requests`` to fetch the live-session id. That coupled every platform adapter to FastAPI app state and the execution-request schema — a second platform (Discord) would have to duplicate the query or accept the same coupling. Split the responsibility cleanly: - New ``ActiveThreadCapture`` NamedTuple in ``api.platforms`` carries the pure-data result of a capture decision (text to forward into the live session + envelope returned to the agent). - ``MessagingPlatform.match_active_thread_capture(thread_key, tool_name, method_name, args)`` is now the platform's only capture surface. It's synchronous, takes pure data, returns ``ActiveThreadCapture | None``, and performs zero I/O. Replaces both the old async ``capture_active_thread_tool_call`` and the ``intercepts_tool_call`` sync hint introduced in the prior commit. - ``runtime_control.get_live_session_id_for_thread(pool, thread_key)`` centralizes the live-session lookup query (which reads the platform-agnostic ``slackbot_live_delivery`` / ``slackbot_agent_session_id`` metadata keys documented on ``api.platforms``). - ``tool_manager._capture_active_thread_tool_call`` is the only place that wires the two together: resolve platform, ask platform to match (sync), look up session_id (async DB), forward text via ``platform.session_text``. Logs as ``active_thread_tool_call_captured`` with the platform name. Adds 6 new test cases in ``test_platforms.py`` for the match function (non-send-message tool, foreign-channel write, foreign-thread write, empty text, non-slack thread_key, base no-op). Verified: uv run ruff check api/ clean; 82/82 targeted tests pass (platform suite, slackbot_client suite, workflow idempotency, integration, CI-targeted workflows + agent_control_plane). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…deprec shim
P3 cleanup bundle from the multi-agent review:
- Drop the ``"slackbot" + "_v" + "2_live_delivery"`` Python-side concat
and the matching SQL ``'slackbot' || '_v' || '2_live_delivery'`` trick.
Write the legacy key as a plain string with an explanatory comment so
future readers (and grep) see what's actually being checked.
- Simplify ``_valid_github_handle`` triple-strip + path-trim into a single
``.strip("@ \t\n").rstrip("/").split("/", 1)[0]`` chain with a comment
documenting the inputs it accepts.
- Promote ``HASH_IGNORED_INPUT_KEYS`` to a workflow module attribute.
``_RegisteredHandler`` now carries the tuple; ``_workflow_request_hash``
consults the registered handler instead of the hard-coded
``{"messaging_thread_turn", "slack_thread_turn"}`` set. The
``messaging_thread_turn`` workflow declares
``HASH_IGNORED_INPUT_KEYS = ("history_messages",)`` directly.
- Move ``_SLACK_CHANNEL_ID_RE`` next to its sole consumer; collect the
Slack formatting-rule lines into a module-level
``_SLACK_FORMATTING_RULES`` tuple constant so
``system_prompt_rules`` shrinks to one composition expression.
- Emit a ``DeprecationWarning`` on import of ``api.slackbot_client`` and
``api.slack_sanitize`` so the shims don't accrete new callers before
the planned removal PR.
- Trim "Phase 0 → Phase 1" refactor-history wording from module
docstrings now that we're past that transition; replace with concrete
forward-looking migration guidance.
Verified: uv run ruff check api/ clean; 82/82 targeted tests pass; full
test_workflows.py shows only the pre-existing
``test_slack_thread_turn_attachment_roundtrip_to_agent`` failure
(unrelated to this PR, fails on pristine main as well).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…schedule platform Four post-review polish items flagged by the remote review: - Workflow alias collision now logs ``workflow_alias_collision`` when a later loader's alias would clobber an existing canonical handler from a different module. Pure observability — no current in-tree collision, but the alias machinery introduced in the prior commits creates the surface area; load-order-dependent silent overwrites now show up in the logs instead of as ghost behaviour. - Schedule auto-derivation of ``delivery.platform`` from ``thread_key`` now treats 2-part legacy keys (``CHANNEL:TS``) as slack and only reads a platform namespace from 3-part keys (``platform:channel:ts``). Restores the back-compat the inline comment already promised but the previous ``":" in thread_key`` shortcut bypassed for the 2-part shape that ``_split_thread_key`` still accepts. - Delete dead ``_clip_slackbot`` helper and ``_MAX_SLACKBOT_TEXT_CHARS`` / ``_MAX_SLACKBOT_STEP_CHARS`` constants in runtime_control.py. Their only consumer (``_send_slackbot_canonical_event``) was removed earlier in the branch; clipping responsibility moved to ``MessagingPlatform.clip_text``. - Fix stale ``dict[str, str | bool] | None`` annotations on ``_resolve_requester_identity`` and ``_build_session_context``'s ``requester_identity`` parameter. The dataclass migration earlier in the branch made the runtime type ``RequesterIdentity | None``; annotations now match. Verified: uv run ruff check api/ clean; 82/82 targeted tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.