fix(mothership): run client-routed workflow tools server-side in headless execution#4870
Conversation
…less execution Headless Mothership (Mothership block, no browser) could not run workflows. The run_workflow/run_workflow_until_block/run_block/run_from_block tools are registered with route 'client', so the executor gate (isSimExecuted) skipped their registered server handlers and fell through to executeAppTool, throwing 'Tool not found'. Interactive runs delegate these to the browser before reaching the executor, so only the headless path broke. Allow a client-routed tool to use its registered server handler when one exists, which only affects the four run tools (the only client-routed tools, all of which have server handlers).
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@greptile review |
PR SummaryLow Risk Overview The executor now routes client tools through their registered handler when one exists ( Reviewed by Cursor Bugbot for commit 688c738. Bugbot is set up for automated code reviews on this repo. Configure here. |
Greptile SummaryThis PR fixes headless Mothership execution (no browser context) for the four workflow-runner tools (
Confidence Score: 5/5Safe to merge — the change is a minimal, well-guarded extension of the existing routing gate with no risk of unintended tool promotion. The routing condition now adds isClientExecuted(toolId) && hasHandler(toolId) as an alternative path. Both guards must be true simultaneously, so only tools that are explicitly registered as client-routed AND have a server handler registered will take the new path. The four affected tools already had working server handlers; this just removes the gate that was blocking them in headless mode. Test isolation is correctly handled with clearHandlers() in beforeEach. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[executeTool called] --> B{isKnownTool?}
B -- No --> C[buildAppToolParams]
C --> D[executeAppTool]
B -- Yes --> E{isSimExecuted?}
E -- Yes --> F[canUseRegisteredHandler = true]
E -- No --> G{isClientExecuted?}
G -- No --> C
G -- Yes --> H{hasHandler?}
H -- No --> C
H -- Yes --> F
F --> I{abortSignal aborted?}
I -- Yes --> J[Return abort error]
I -- No --> K{handler in registry?}
K -- No --> L[Return no handler error]
K -- Yes --> M[handler params, context]
M --> N[Return ToolExecutionResult]
D --> N
Reviews (3): Last reviewed commit: "test(mothership): clear handler registry..." | Re-trigger Greptile |
Greptile SummaryThis PR fixes headless Mothership execution by allowing client-routed tools (
Confidence Score: 4/5Safe to merge; the fix is narrowly scoped to client-routed tools that already have a registered server handler, which today are exactly the four Mothership run tools. The logic change is well-bounded: the new branch only activates when both isClientExecuted and hasHandler are true, so no existing behavior is altered for sim- or go-routed tools, and client tools without a server handler continue to fall back to executeAppTool as before. The one flag is that handlerRegistry is mutated in a test without cleanup, which could cause subtle inter-test ordering effects in watch mode. executor.test.ts — the registerHandler call inside the test body leaves state in the module-level registry with no afterEach cleanup. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[executeTool called] --> B{isKnownTool?}
B -- No --> C[buildAppToolParams to executeAppTool]
B -- Yes --> D{isSimExecuted?}
D -- Yes --> G[Use registered handler]
D -- No --> E{isClientExecuted AND hasHandler?}
E -- Yes --> G
E -- No --> C
G --> H{abortSignal aborted?}
H -- Yes --> I[Return aborted error]
H -- No --> J{handler in registry?}
J -- No --> K[Return no handler error]
J -- Yes --> L[handler params context to ToolExecutionResult]
Reviews (1): Last reviewed commit: "fix(mothership): run client-routed workf..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Add clearHandlers() helper and reset the module-level handler registry in beforeEach so handlers registered in one test do not leak into the next.
|
@greptile review |
1 participant
Summary
run_workflow,run_workflow_until_block,run_block, andrun_from_blockall failed withTool not foundroute: 'client', so the executor gate (isSimExecuted) skipped their registered server handlers and fell through toexecuteAppTool, which threw. Interactive runs delegate them to the browser before reaching the executor, so only the headless path was brokenroute: 'client'tools, all of which already have server handlersType of Change
Testing
Tested manually. Added unit tests for the new headless routing (handler used for client tool with a registered handler; still falls back to
executeAppToolwhen none). Confirmed against production ECS logs on/api/mothership/execute: 7xrun_workflow, 2xrun_workflow_until_block, plusrun_block/run_from_block, allTool not foundfrom the autonomousrunsubagent (executor: client,simExecutable: false). Lint andcheck:api-validation:strictpass.Checklist