feat(codex): harden turn timeout — interrupt on deadline, bound stalled turn/start, options threading#1
Open
russjhammond wants to merge 1 commit into
Conversation
…ed turn/start, thread budget via options Follow-up to openai#376 (the turn-await hang fix), porting the three improvements axisrow found while adapting openai#376 into their fork, with attribution: 1. Arm the deadline BEFORE turn/start and race the whole turn lifecycle, so a stalled turn/start (app-server alive but never responding) is bounded too — not just the post-start completion await. 2. On the deadline (or any lifecycle error), best-effort interrupt the in-flight turn so the broker stops executing a write-capable task after the job is already reported failed. 3. Resolve the per-turn budget from options and thread it through the request (buildTaskRequest -> executeTaskRun/executeReviewRun -> runAppServerTurn/ Review -> captureTurn) instead of mutating process.env — retiring the foreground->background env-inheritance hazard. Background runs get the full default budget. Also drops the redundant exitRaceSettled guard (state.completed already covers the late-exit-after-completion race). Kept openai#376's exit-wiring placement AFTER startRequest resolves (commit 688192a): hoisting it with the deadline would reject an unobserved state.completion when startRequest itself rejects, reintroducing the unhandled-rejection that openai#376's review already fixed. The deadline alone bounds the stalled-turn/start case. Test: a foreground task whose turn/start never responds now times out on the turn budget instead of hanging (new stalled-turn-start fake-codex behavior). tests/helpers.mjs run() gains an optional timeout so the hang path can't block CI. Credits: axisrow#26 (port) and openai#28 (these three findings). Refs openai#49. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A5iLDcuiEzAxs1vBEwafur
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.
Follow-up to openai#376 — hardening the turn timeout
Stacked on openai#376 (the turn-await hang fix). This ports the
three follow-up improvements @axisrow found while
adapting openai#376 into their fork
(axisrow/codex-plugin-cc#28),
with attribution. On openai#376 I'd deliberately scoped these out as follow-ups; this
PR is that follow-up.
What changes
turn/starttoo. Arm the deadline beforeturn/startand race the whole turn lifecycle against it. Previously the deadline only
covered the post-start completion await, so an app-server that accepted the
connection but never answered
turn/startstill hung until the host'sexternal ceiling.
best-effort
interruptAppServerTurn(...)so the broker stops executing awrite-capable turn after we've already reported the job failed. Failures to
interrupt (broker gone, network down) never mask the original error.
process.env.resolveTurnTimeoutMsFromOptionsresolves flag → env → mode-default and flowsthrough
buildTaskRequest → executeTaskRun/executeReviewRun → runAppServerTurn/Review → captureTurn, replacing theCODEX_TURN_TIMEOUT_MSmutation. This retires the foreground→background env-inheritance hazard
entirely; background runs get the full default budget (no external ceiling to
collide with).
Also drops the redundant
exitRaceSettledguard —state.completedalreadycovers the late-exit-after-completion race.
One deliberate divergence from axisrow#28
I kept openai#376's exit-wiring placement after
startRequestresolves (commit688192a). Hoisting it up with the deadline (as openai#28 does) would reject anunobserved
state.completionwhenstartRequestitself rejects —reintroducing exactly the unhandled-rejection that openai#376's review already fixed.
The deadline alone bounds the stalled-
turn/startcase, so the hoist isn'tneeded for that.
Test
New
stalled-turn-startfake-codex behavior + a test asserting a foregroundtask whose
turn/startnever responds times out on the turn budget instead ofhanging.
tests/helpers.mjsrun()gains an optionaltimeoutso the hangpath can't block CI. Full suite: no new failures (the 4 reds are pre-existing,
env/temp-dir related, unchanged on
main).Refs openai#49. Credits axisrow#26 (port) and openai#28 (findings).