feat(orchestrator): replace /invoke-workflow text sentinel with typed invoke_workflow MCP tool#1122
feat(orchestrator): replace /invoke-workflow text sentinel with typed invoke_workflow MCP tool#1122mhooooo wants to merge 2 commits intocoleam00:mainfrom
Conversation
…lack When the Claude Code SDK rejects a resume attempt with "No conversation found" (the SDK session ID is gone), the orchestrator now transparently resets the session and retries the query instead of surfacing an error that the user has to /reset manually. Also accepts bare 'reset' without the leading slash on Slack, since Slack intercepts /reset as its own slash command. Changes: - claude.ts: classify stale_session as a non-retryable error class (checked before 'crash' — specific wins over generic); export STALE_SESSION_PATTERNS as the single source of truth for both the classifier and the orchestrator's isStaleSessionError() helper - session-transitions.ts: new 'stale-session-cleared' transition (deactivates — next message creates a fresh session) - orchestrator-agent.ts: isStaleSessionError() helper; SLACK_BARE_COMMANDS normalization scoped to Slack platform only; handleStreamMode and handleBatchMode wrap their AI query loops in runStreamQuery() / runBatchQuery() functions so a catch block can reset sessionForQuery and re-run with the fresh session ID; state reset before retry (allMessages/allChunks/assistantMessages/commandDetected) so partial content from the failed attempt never bleeds into the fresh response - claude.test.ts: stale_session classification tests, including priority over 'crash' on overlapping error messages, and .cause assertions - orchestrator.test.ts: parameterized stream/batch retry tests covering successful reset+retry, no-third-retry guard, null-session skip, and fresh session ID assertion on retry Ported from the dynamous/remote-coding-agent fork (commit 229217cf) with the following intentional deltas against the new v0.3.5 base: - Dropped defaultCodebase auto-scoping block (Patch 1 not carried — CONFIG-REPLACEABLE per investigation verdict) - Slack bare-command normalization scoped to Slack platform only (fork shipped unscoped initially; this change came in a later review-findings sub-commit) - runStreamQuery/runBatchQuery keep upstream's 4-arg aiClient.sendQuery signature including requestOptions (fork was on pre-v0.3.2 3-arg shape) - Upstream deterministic command list preserved (help, status, reset, workflow, register-project, update-project, remove-project, commands, init, worktree) — fork only had 5 - No CHANGELOG / bun.lock / package.json / docs changes — those will be rebuilt on top of the v0.3.5 base Upstream-PR candidate for coleam00/Archon.
… tool
The previous /invoke-workflow text-sentinel approach was unreliable —
Claude would emit the sentinel inconsistently (mid-response, inside code
blocks, with extra text, or not at all). The fallback post-loop regex
parser caught some cases but left a persistent failure mode where
workflows either didn't dispatch or dispatched at the wrong time.
Migrate to an in-process MCP server exposing invoke_workflow as a real
typed tool call. Claude now dispatches workflows by calling a function
with structured parameters, which is deterministic and reliable.
Changes:
- packages/core/src/orchestrator/workflow-tool.ts (NEW): buildWorkflowMcpServer
factory using createSdkMcpServer. Registers invoke_workflow tool with
zod schema for workflow_name / project_name / task_description. Tool is
fire-and-forget — it kicks off dispatchOrchestratorWorkflow via the
injected dispatch callback and returns immediately so the conversation
turn can end cleanly.
- packages/core/src/orchestrator/codebase-utils.ts (NEW): findCodebaseByName
helper — org-qualified and case-insensitive project matching, extracted
from orchestrator-agent.ts to eliminate duplication between workflow-tool.ts
and the register-project handler.
- packages/core/src/orchestrator/workflow-tool.test.ts (NEW): 8 tests
covering server shape, error paths, dispatch happy path, error handling,
case-insensitive project matching, org-qualified matching, and zod
validation of task_description.
- orchestrator-agent.ts:
- handleStreamMode and handleBatchMode each build a workflowMcpServer
at entry via buildWorkflowMcpServer({ ... dispatch }), then pass it
via requestOptions.mcpServers['archon-tools'] to aiClient.sendQuery.
Caller-provided requestOptions are merged, not overwritten, so outer
MCP config still works.
- /invoke-workflow text-sentinel detection removed from both stream
and batch post-loop command parsers. /register-project still uses
the text sentinel since it needs inline-parseable user-visible output.
- handleWorkflowInvocationResult function deleted (dead code after
sentinel removal).
- issueContext parameter renamed to _issueContext in handleStreamMode
and handleBatchMode to document that it's unused — issue context now
travels through the task_description field of the tool call instead.
- Imports: buildWorkflowMcpServer and findCodebaseByName added.
- prompt-builder.ts: router description rewritten to describe the
invoke_workflow tool interface (tool parameters) instead of the
text-sentinel command syntax.
- orchestrator-agent.test.ts: workflow-tool module mock added.
- prompt-builder.test.ts: assertions updated to match the new
tool-based routing instructions.
- error-formatter.ts: "Use /reset" → "Use `reset`" for the session-error
fallback message (Slack intercepts /reset as its own slash command;
bare 'reset' is accepted by the orchestrator after commit 1's
SLACK_BARE_COMMANDS normalization).
- error-formatter.test.ts: test expectation updated.
Ported from dynamous/remote-coding-agent fork commit 3df00e1b with the
following intentional deltas:
- Dropped: Slack thinking indicator (⏳ emoji) — the feature was broken
in v0.2 (emoji flashed on then off because the wrapper's await fn()
returned immediately on a fire-and-forget handler) and not worth
carrying forward without a fix.
- Kept: upstream's 4-arg aiClient.sendQuery signature with requestOptions;
MCP server merged into caller-provided requestOptions rather than
replacing them.
- Kept: upstream's longer deterministic command list (help, status,
reset, workflow, register-project, update-project, remove-project,
commands, init, worktree) — fork only had 5.
- Skipped: CHANGELOG, CLAUDE.md, docs/adapters/slack.md changes —
upstream docs have diverged; docs will be rebuilt on top of v0.3.5.
- Skipped: packages/core/package.json MCP SDK dependency bump — will
resolve naturally via bun install once the runtime is assembled.
Upstream-PR candidate for coleam00/Archon.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Thanks for the thorough work here, @mhooooo — the bug you're pointing at is real (Claude does emit the Why this is going to be closed1. It's solving a fragility in a narrow non-primary surface. 2. The primitive cuts against the multi-provider architecture. 3. MCP is the wrong layer for an in-process callback. 4. The pattern isn't eliminated, just halved. 5. Tool-call response patterns ≠ text-response patterns. If you want to revisit the underlying problemThree lighter paths that wouldn't have these issues:
Unrelated issues worth flagging
What we'd love to see insteadIf you want to keep contributing — and we do — #1121 is the one with real returns on effort. It's a smaller, targeted bug fix whose design we've already agreed on. Getting that ported to the new architecture (see my earlier porting checklist) is the highest-leverage next step. Closing this one. Thanks again for the care you put into it — the concern is real, just not right-sized for where it lives in the architecture. |
2 participants
Summary
The current orchestrator detects workflow dispatch via a text-based
/invoke-workflowsentinel — Claude is prompted to emit a magic string, and stream/batch handlers post-parse for the pattern. This is fragile: Claude emits the sentinel inconsistently (in code blocks, with leading text, paraphrased, or not at all). Workflow dispatch is important and deserves to be deterministic.This PR registers an in-process MCP server exposing
invoke_workflowas a real tool. Claude calls a typed function with structured parameters (workflow_name,project_name,task_description) instead of emitting a string convention. Dispatch becomes machine-structured and deterministic.Changes
packages/core/src/orchestrator/workflow-tool.ts(NEW)buildWorkflowMcpServer(deps)factory usingcreateSdkMcpServerinvoke_workflowtool with zod schema:workflow_name,project_name,task_descriptionpackages/core/src/orchestrator/codebase-utils.ts(NEW)findCodebaseByNameextracted from orchestrator-agent.ts (DRY)packages/core/src/orchestrator/orchestrator-agent.tsworkflowMcpServer, inject viarequestOptions.mcpServershandleWorkflowInvocationResultdeleted (dead code)packages/core/src/orchestrator/prompt-builder.tspackages/core/src/utils/error-formatter.ts/reset→`reset`in error messages (Slack intercepts/reset)Tests
workflow-tool.test.ts— 8 tests: factory, dispatch, error paths, case matching, zod validationScope
/register-projectstill uses text sentinel (different shape, less error-prone) — separable follow-uperror-formatter.tschange is incidental — happy to split outCarried forward from
dynamous-community/remote-coding-agent @ v0.2.12(commit 3df00e1b). Rebased onto latest main.Test plan
bun run type-check— cleanbun test packages/core/src/orchestrator/workflow-tool.test.tsbun test packages/core/src/orchestrator/orchestrator.test.tsbun test packages/core/src/orchestrator/prompt-builder.test.tsbun run test— full suite exit 0/invoke-workflowtext is treated as literal message (no longer triggers dispatch)