Repository navigation
fix(native-chat): a refused steer leaves its turn as the chat's latest request - #29907
Conversation
…t request A steer Codex answered into a turn that then ended without taking it is rejected and moved to the thread, so the sidebar verdict and the completion feed treated it as a refused request of its own and reported it in place of the turn it joined.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 2 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
A follow-up sent while Codex was replying could be treated as a new failed request when rejection moved its bubble outside the turn. The sidebar and completion notification then attributed the outcome to that refused follow-up instead of the turn it joined. This PR uses the existing saved steer association to keep the turn as the reported request. A failed turn still reads Failed, with its own date and completion identity; a successful turn with a refused steer retains its success. Smart sort also uses the turn's completion date for ordering and age. The refusal bubble and ordinary start failures keep their existing behavior. No saved-data or wire format changes are introduced; older hosts without the association retain their behavior. Review: one correctness round, an independent architecture challenge, and the final readiness/performance review completed on Verification:
Model timing/wording, promotion-toast timing, and an extra driver call to an internal repository-refresh API were recorded and checked as unrelated variations. No unexpected status regression was observed in the exercised native flows. For Brennan's review These are whole-window captures from the real runs. The final row documents why the failed-turn case could not be produced; it is not proof of the attribution fix.
The judgment points are the selector, where semantic turn membership survives rejection relocation, and the existing behavioral fixture, which now models that relocated shape. The common pattern separates turn identity from message placement; the intended difference here is that host-owned status projection continues to support older paired clients without a new wire requirement. |
ELI5
When you send a follow-up message while a Codex chat is still answering (a "steer") and that answer ends without Codex ever taking your follow-up, Orca marks the follow-up as refused. The sidebar's verdict and the "chat finished" notification then talked about that refused follow-up as if it were the chat's latest request, instead of about the answer it was attached to. This makes the answer's turn stay the request the sidebar and notification report.
What Changed
Before: a Codex turn ends while one of your steers is still waiting to be taken. Orca refuses the steer and moves its bubble to the end of the conversation. Because the bubble no longer sits inside the turn, the code that picks "the chat's latest request" took the refused steer as a request of its own. Today that happens when the turn fails: the sidebar's Failed verdict, its time and the completion notification belong to the steer instead of to the turn.
After: a refused steer is never treated as the latest request. The turn it was sent into answers for it, as the comment at the top of this file already said was intended. A message refused at the start of a new turn (not a steer) still reports as a refused request, as before.
Mechanism:
latestStructuredAgentSessionRequestalready skips a steer while it sits inside its turn. A rejection relocates the message to the thread (placeRejectedMessage), which erased that signal. The saved rejection still records how the message joined the turn (answeredInTurn.via === 'steer'), so the function now reads that instead of the bubble's current position.Why
This is the first, standalone step of the native-chat send-lifecycle work (stuck "Working" chats). A later step will refuse more Codex steers from saved facts (for example, a steer a hook blocked inside a turn that otherwise succeeded). Without this fix, each of those would turn a successful turn's sidebar verdict into Failed. Landing it first keeps that later change from making the sidebar wrong.
Alternative considered: leave a refused steer inside its turn instead of moving it. Moving it to where it was refused is deliberate (the conversation shows when the refusal happened), and the saved
answeredInTurnfact already answers this question without changing placement.This was carried over from #28630, which was closed because the larger refactor replaces the rest of it.
Linked Issue
Tracked in #28630 (closed; this is its latest-request part).
Visual Proof
The change is in the sidebar verdict and completion notification for a Codex chat whose turn ended with a steer not taken. Screenshots from the real QA run will be added here.
Testing
The existing "a steer the provider refused inside a turn that then succeeded" case in
structured-agent-session-latest-request.test.tsnow models the saved data as it really is (the refused steer moved to the thread, withansweredInTurn.via: 'steer'). Before this fix the case only passed because its fixture kept the steer inside the turn, which a real rejection never does.CI runs the test, lint and typecheck.
I manually tested these changes locally
Automated tests added/updated, or explained why not below
Review
Agent skill upstream boundary
Notes
Remote and mixed versions: reads a field (
answeredInTurn) that hosts already save. Rows from older hosts lack it and behave exactly as today. No wire or saved-data change. Platform-independent.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)