🤖 fix: drop withdrawn queued wakes from pending work - #4067
Conversation
A queued entry whose cancelSignal has fired can never start a turn, yet it
still counted as pending work: hasQueuedMessages("tool-end") kept arming the
stream's tool-end stop, and hasPendingQueuedOrPreparingTurn kept reporting
the workspace busy until stream end drained the dead entry. When a bash
monitor wake was withdrawn mid-turn (task_await had already shown the
output), that stop cut the model's stream at a tool boundary with no
successor turn.
- MessageQueue.getNextDispatchableMode() reports the first non-aborted
entry's mode; hasQueuedMessages and every setMessageQueued site use it.
- The soft-stop drain keeps its physical isEmpty check so aborted entries
still drain and fire onCanceled.
- dispatchBashMonitorWake removes the queued wake by its per-dispatch
dedupe key as soon as the reconciler aborts the dispatch, so the key is
released and onCanceled fires exactly once at withdrawal time.
Complements the recovery side in #4065; the stop-condition race can still
lose when withdrawal lands after stopWhen evaluated.
- messageQueue: getNextDispatchableMode skips aborted entries while the queue stays physically non-empty. - agentSession.queueDispatch: an aborted tool-end head no longer soft-stops after a provider-executed tool result, does not hide a later turn-end entry's mode, and setMessageQueued follows the dispatchable state. - workspaceService: aborting a queued monitor wake removes it from the queue, releases its dedupe key for a re-dispatch, and fires onDeferred exactly once.
This comment has been minimized.
This comment has been minimized.
|
@codex review |
This comment has been minimized.
This comment has been minimized.
![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: b3042cce47
ℹ️ 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".
…tchable mode A monitor wake withdrawn before dispatchBashMonitorWake attached its abort listener, or during sendMessage preflight, could still be enqueued as an already-aborted entry that held its queue slot and dedupe key until the stream drained it. Skip such a dispatch outright and run the cancel-before-acceptance handshake at the queue-branch chokepoint instead of enqueuing. queueMessage now returns the same next-dispatchable mode it uses to arm tool-end stops, so foreground task waits follow the entry that will run rather than a withdrawn FIFO head.
|
@codex review Addressed both P2 findings in c8d34f9: the queue branch of WorkspaceService.sendMessage refuses an already-aborted cancel signal at the enqueue point (and dispatchBashMonitorWake returns "deferred" for a signal that aborted before the listener attached), and queueMessage returns the same next-dispatchable mode it uses for the dispatch signal. |
|
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". |
This comment has been minimized.
This comment has been minimized.
OpenAI now answers a nonexistent model with HTTP 404 (error.code still model_not_found); the classifier only accepted the code on a 400, so the stream error surfaced as 'api' and stayed retryable. Unblocks the modelNotFound integration suite on main.
|
Codex Review: Didn't find any major issues. You're on a roll. 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". |
This comment has been minimized.
This comment has been minimized.
Reconcile with #4067 (withdrawn queued wakes no longer count as pending work): the post-abort dispatch keeps main's physical queue check so a canceled wake still drains, because that drain's idle transition is what sweeps the owed continuation. Adds the test for that composed path.
1 participant
Drops withdrawn queued wakes from pending work calculations so they cannot block dispatch or show false waiting indicators, and fixes the OpenAI
model_not_foundclassification that was failing the integration suite onmain.Withdrawn queued wakes
agentSessionqueue dispatch refuses already-withdrawn wakes at enqueue time and returns the dispatchable mode instead of a fixed onemessageQueueandworkspaceService.hasPendingWorkskip withdrawn entriesOpenAI
model_not_foundclassificationUnrelated to the wake change, folded in because it was the only red check on this PR (and on the other open PRs today). OpenAI now answers a nonexistent model with HTTP 404 while still setting
error.code === "model_not_found"; the classifier only accepted that code on a 400, so the stream error surfaced asapiand stayed retryable. The check now keys on the error code regardless of status. Adds a unit guard for the 404 shape (fails withapion the previous code) and updates the live integration test's wording.Validation
tests/ipc/config/modelNotFound.test.tsreproduced locally (FAIL,Received: "api") before the fix and passes after itqueuedMessages*andbackgroundBash*integration suites pass locallyGenerated with
xum• Model:anthropic:claude-fable-5-1• Thinking:xhigh• Cost:$27.79