fix(agent-core-v2): stop tool actor teardown from aborting sibling tools - #3626
Conversation
|
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4e2342dc4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| function createExecuteActor(executor: ToolExecutor) { | ||
| return fromCallback<ToolEvent, ToolInput>(({ input, sendBack, receive }) => { | ||
| const controller = new AbortController(); | ||
| return fromCallback<ToolEvent, ToolInput>(({ input, sendBack }) => { |
There was a problem hiding this comment.
When a tool has called detach, the foreground turn can finish and AgentLoopService.dispose() sees no active turn, so it calls engine.stop() without invoking any of the new controller-abort actions. Stopping the spawned callback actor no longer aborts its signal here because its teardown was removed, allowing a signal-aware background MCP/process tool to keep executing after its agent or session has been disposed. Preserve an actor-stop cleanup that aborts the externally owned controller, or explicitly abort every background controller before stopping the engine.
Useful? React with 👍 / 👎.
| const waitForTasks = createWaitForTasks(self); | ||
| const turnTools = { ...context.turnTools }; | ||
| for (const toolCall of event.toolCalls) { | ||
| const controller = new AbortController(); |
There was a problem hiding this comment.
This commit fixes a user-visible CLI regression—parallel tool calls previously surfaced spurious abort errors—but it contains no .changeset entry. The repository's changeset policy requires user-perceivable internal-package bug fixes to record a patch for @moonshot-ai/kimi-code; otherwise this fix will be omitted from the generated release changelog.
AGENTS.md reference: AGENTS.md:L85-L87
Useful? React with 👍 / 👎.
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
1 participant
Related Issue
No linked issue — regression found by bisecting release e2e failures to #3580.
Problem
When the model emits a batch of parallel tool calls, the first tool to finish aborts its still-running siblings: users see
TOOL ERROR/ "Tool X was aborted" on calls that should have completed normally, and MCP tools emit spuriousnotifications/cancelled.Mechanism: #3580 gave every tool actor its own
AbortControllerand merged all of a batch's signals viaAbortSignal.anyfor the executor service. When a tool completes, XState stops the invoked callback actor and its teardown hook unconditionally aborts the controller — firing the merged batch signal, so the executor service marks every still-running sibling as aborted. The pre-#3580 loop used a turn/step-level shared signal, so completing one tool never aborted anything.What changed
AbortControllerper tool call (ToolInput.signal) and the turn machine creates one per LLM attempt (LlmInput.signal). Tool and request actors are now pure execute-and-report: no controller, noreceive, no teardown hook. A finished tool's teardown can no longer fire the shared batch signal — the sibling abort is eliminated structurally, not guarded.input.abort/turn.abortabort the owned controllers directly (thetool.abortevent remains for tools still waiting on permission);stopTurnToolsaborts-then-stops. Thellm.abortcommand event and the requester machine'sstreaming/abortingsubstates are removed.thinking/retrying/draininggo straight toaborted, salvaging the partial assistant message.actinggainsrunning/abortingsubstates: tool outcomes keep collecting real results during a grace window (abortGraceMs, default 2.5s, aligned with the executor's 2s abort grace), then missing outcomes are synthesized asaborted; a repeated abort completes immediately.Verified: focused agent-core-v2 suites (150 tests),
tsc --noEmit, scoped oxlint.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.