🤖 fix: bound task_terminate with abort support, timeouts, and per-task errors#3720
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca611f9d5c
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c844bc7859
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01c2291a4a
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88327b7ad7
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4aa625697e
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
task_terminatecould hang forever with no feedback, no timeout, and no way to cancel: the tool ignored the abort signal, every await down the termination chain was unbounded, and one hung branch rejected the whole call. This PR makes the tool interruptible and bounds every hang path so failures surface as per-task error results instead of silence.Background
A stuck child stream (e.g. a child wedged in a tool call that does not honor abort) kept
streamManager.cancelStreamSafely'sawait processingPromisepending forever.task_terminateawaited that chain with no deadline, so the tool spun indefinitely; user interrupt showed "INTERRUPTING..." forever because stream teardown waits on the running tool.Implementation
Layered fix, smallest-risk pieces first:
task_terminate.ts):executenow consumesoptions.abortSignal. Each task ID branch is wrapped in a per-task promise raced via a newraceWithAbortAndTimeouthelper (5 min backstop per ID); abort/timeout produce a per-taskstatus: "error"result while other IDs still resolve. A try/catch around each branch keeps one rejection from failing the wholePromise.all.taskService.terminateDescendantAgentTask): per descendant,stopStreamis bounded at 20s (on timeout the workspace is NOT deleted under a possibly-live stream; siblings continue) andworkspaceService.removeat 120s. Errors aggregate instead of aborting the loop.workspaceService.remove's forced path bounds its ownstopStreamthe same way and refuses deletion on timeout.timeoutMs(execFileAsyncalready supported it; no timer was installed), and descendant traversal uses a visited set so cyclic task metadata cannot loop forever.Timeout constants live in
src/constants/terminationTimeouts.ts; the race helper insrc/node/utils/concurrency/withTimeout.tsregisters promise settlement before arming the timer so an already-settled promise wins over a simultaneous timeout.Explicitly deferred (higher-risk follow-ups): abandoning
processingPromiseincleanupAbortedStream(needs a central abandoned-guard for late history/partial writers) and narrowing the task mutex scope.Validation
terminated+not_foundper-task results in seconds, child session/worktree fully removed; re-terminating the same ID returnednot_foundpromptly.Risks
Termination and workspace removal are shared paths (user-initiated workspace deletion also flows through
workspaceService.remove). Behavior change is bounded: previously-infinite waits now end with explicit errors, and a stream-stop timeout now skips deletion where it previously waited forever. If a slow-but-healthy stream stop exceeds 20s, termination reports an error instead of eventually succeeding; the child workspace survives for inspection and can be re-terminated.Generated with
mux• Model:anthropic:claude-fable-5• Thinking:xhigh• Cost:$20.46