Skip to content

fix(session-runtime): don't charge a retired turn's late frames to the next execution#618

Merged
gbasin merged 1 commit into
masterfrom
fix/stale-turn-frame-misattribution
Jul 20, 2026
Merged

fix(session-runtime): don't charge a retired turn's late frames to the next execution#618
gbasin merged 1 commit into
masterfrom
fix/stale-turn-frame-misattribution

Conversation

@gbasin

@gbasin gbasin commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

What happened

Session 362d3277 in prod hit the codex usage limit. The retry ("Retry the failed turn.") died 40ms after birth with a misleading turn completed with status failed before final answer — and never reached the model at all. Its prompt was silently swallowed.

Root cause

The codex app-server emitted turn/completed for the already-errored turn 7.5s after the error frame that ended it — by which time the retry was the active execution:

time event
12:58:41.07 error (usage limit) ends exe_f8be…; pump records terminal, calls forget()
12:58:48.49 retry creates exe_03b05…, becomes active
12:58:48.52 late turn/completed for the old turn arrives → charged to the retry → kills it

forget() purged turn_execution_by_id, erasing the very mapping the existing late-frame guard (execution_for_line, lib.rs:5919) depends on. With no owner found, the frame fell through to active_execution_id.

Fix

forget() now retires the execution's turn/item ids into a bounded FIFO tombstone (1024 ids) instead of dropping them, so late frames stay attributable to the execution that owned them and are discarded rather than misattributed.

This would corrupt any retry landing inside the window where a prior turn's completion frame is still in flight — not specific to the rate-limit case.

Testing

  • New regression test stdout_state_drops_late_terminal_frame_from_a_retired_turn reproduces the exact prod sequence. Verified it fails without the fix (asserts Some("exe-retry") vs None) and passes with it.
  • cargo test -p centaur-session-runtime --lib — 130 passed
  • cargo clippy --all-targets clean, cargo fmt --check clean

Not run: the live-cluster e2e. This is a pure state-machine change in the stdout pump, proven by unit test; standing up k8s to exercise a tombstone map seemed disproportionate. Happy to run it if you'd rather.

…e next execution

The codex app-server can emit a turn's `turn/completed` seconds after the
`error` frame that already ended that turn. When a user retries in that
window, the late frame arrives after the pump has already called
`forget()` on the finished execution — which erased the turn->execution
mapping the late-frame guard depends on. `execution_for_line` then found
no owner and fell through to the active execution, charging a dead turn's
`status: failed` to the retry and terminating it before its prompt ever
reached the model.

Observed in prod: a session hit the codex usage limit, and the retry died
40ms after birth with the misleading `turn completed with status failed
before final answer` — the real reason (out of credits) was masked.

`forget()` now retires the execution's turn/item ids into a bounded FIFO
tombstone instead of dropping them, so late frames stay attributable to
the execution that owned them and are discarded.
@gbasin
gbasin enabled auto-merge July 20, 2026 14:01
@gbasin
gbasin added this pull request to the merge queue Jul 20, 2026
Merged via the queue into master with commit 7521fb1 Jul 20, 2026
18 checks passed
@gbasin
gbasin deleted the fix/stale-turn-frame-misattribution branch July 20, 2026 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant