Repository navigation
fix(codex): release a rejected steering mention's working hold promptly - #919
Merged
Merged
Conversation
When `turn/steer` or the preceding history read fails, the catch path posts "Steering was not accepted" but never adds the request to `active.requests`, so its `replied` promise could not resolve and the `workingUntil` race fell through to the previous turn's `tail`. The rejected mention kept its working reactions until that unrelated turn ended, and with channel-scoped sessions a follow-up in another thread kept that thread typing after the rejection. Resolve the request's own `replied` promise via `answered()` as soon as the rejection is posted. The `steered.tail` fallback still applies only to accepted steering or a turn started in its place. Fixes the P2 from #860 (discussion_r4243890351). Test: hold a turn open, fail `turn/steer` with an RPC error, and assert the rejected mention's hold settles while the running turn's hold stays pending and no replacement turn starts; the turn then completes and answers normally. Fails on the previous runtime (`expected false to be true` on the rejected hold) and passes with the fix. Validation (bin/pnpm, Hermit toolchain): - `vitest run src/bundled/codex`: 4 files, 64 tests passed - `tsc --noEmit`: clean Co-Authored-By: Claude Fable 5.1 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
matt2e
marked this pull request as ready for review
October 13, 2026 00:59
wesbillman
reviewed
Oct 13, 2026
wesbillman
left a comment
wesbillman
left a comment
Collaborator
There was a problem hiding this comment.
No further changes requested: the rejection now settles its request’s working hold after publication, without ending the active turn or changing the accepted/replacement-turn fallback. The new regression distinguishes the rejected hold from the still-running turn.
Star Lord’s automated source review via Wes’s account — head c6c04d9e58d10d52c78c717e33cf7ef99a0cc5b4, base 544d0cd5b2a2b79242fc3452b94d04231cec1fbf.
Source-only: no tests or app execution; CI was incomplete at the snapshot. Inspected the attached still, but could not decode the video. Live rejection/reaction/typing behavior and human acceptance remain unverified.
baxen
approved these changes
Oct 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
When
turn/steer(or the history read that precedes it) fails, the catch path insrc/bundled/codex/runtime.tsposts "Steering was not accepted" but never adds the request toactive.requests. Itsrepliedpromise therefore never resolves, and theworkingUntilrace falls through to the previous turn'stail. The rejected mention's 👀💬 reactions and local typing linger until that unrelated turn ends; with channel-scoped sessions, a follow-up in another thread keeps that thread showing as typing after the rejection.The fix: the catch path now calls
request.answered()right after posting "Steering was not accepted", so the rejected mention's own hold settles at the rejection. Thesteered.tailfallback is unchanged and still applies only to accepted steering or to a turn started in its place.Addresses the P2 review finding from @wesbillman on #860: #860 (comment)
Before / after
codex-steer-rejection-before-after.mp4
Left = before (
c6c04d9e^), right = after (c6c04d9e). Both halves run the same scenario; on the right, mention 2's indicator clears at the rejection (0 ms after it) instead of staying on until turn 1 ends (5015 ms after it).How it was made: a scratch driver (outside the repo) runs each commit's real
CodexRuntimeagainst the test suite's in-memory fake app-server, withturn/steeranswered by an RPC error (-32600 "model busy"). Each mention's hold-release timestamp mirrorsAgentsService.working()/hold(), which is what ends the 👀💬 reactions and local typing in the app. The video is a real-time replay of the two recorded timelines rendered side by side; it is not a live app capture, and the reaction/typing publication itself was not exercised.Validation
src/bundled/codex/runtime.test.ts: holds a turn open, failsturn/steerwith an RPC error, and asserts the rejected mention's hold settles while the running turn's hold stays pending and no replacement turn starts; the turn then completes and answers normally. Fails without the fix (expected false to be trueon the rejected hold) and passes with it.bin/pnpm vitest run src/bundled/codex: 4 files, 64 tests passedtsc --noEmit: cleanDeferred
🤖 Generated with Claude Code