Repository navigation
fix(codex): healthy app-server sessions survive long post-tool reasoning silence (#112928, salvage #112932) - #113079
Merged
Merged
Conversation
…ing silence After a tool result, `CodexAppServerSession._run_started_turn` armed a 90s wire-silence watchdog that sent turn/interrupt and retired the session. On large contexts codex legitimately emits no events for minutes while it reasons after a big tool output, and the app-server answers RPCs the whole time (the interrupt was acknowledged in ~25ms). Silence is not evidence of a wedged process, so the watchdog killed healthy work and surfaced as protocol_violation / crashed workers. Keep `post_tool_quiet_timeout` as observability only: past the threshold log one warning per tool result and keep waiting. Retirement still happens on the two real signals the poll loop already checks every iteration -- subprocess death (`_subprocess_died`) and the overall `turn_timeout` deadline -- so a truly wedged codex stays bounded. The existing monotonic-clock watchdog test becomes the invariant for the new behaviour: tool item, silence past the threshold, then turn/completed -> no interrupt, no retirement, a warning logged. Fixes #112928 Co-authored-by: KoNit-K <124019182+KoNit-K@users.noreply.github.com>
૮ >ﻌ< ა ci reviewran on 54623b0 — fix(codex): drop the stale 'watchdog tripped' retirement cau
|
…-scope the reset test The post-tool quiet timer no longer retires the session (it only warns), so the codex_runtime retirement comment listed a cause that cannot happen and the retained test's name/docstring still promised a watchdog that cannot trip. Comment and test wording now describe the warning semantics; assertions unchanged.
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.
A Codex app-server session that goes quiet on the wire after a tool result — because the model is reasoning over a large context — is no longer interrupted and retired after 90s; it now completes normally and is retired only when the subprocess actually dies or the overall turn deadline is hit.
agent/transports/codex_app_server_session.py::CodexAppServerSession._run_started_turn: the post-tool quiet timer is now observability only. Pastpost_tool_quiet_timeoutit logs one warning per tool result (codex has emitted no events for 90s after a tool result; still waiting (turn deadline 600s)) and keeps polling; it no longer sendsturn/interruptor setsshould_retire._subprocess_died(process/transport gone) and theturn_timeoutdeadline. A truly wedged codex is still bounded.test_post_tool_silence_warns_but_does_not_retire_a_healthy_turn(tool item → silence past the threshold →turn/completed: no interrupt, no retirement, warning logged). Red on base, green on this branch.test_post_tool_watchdog_resets_on_further_activityunchanged.grep -rn post_tool_quiet website/docs docs= 0; no non-test caller passes it) — nothing to document.Live repro (fake app-server client: tool completion, then 0.6s of silence with
is_alive()true,post_tool_quiet_timeout=0.2, thenturn/completed; plus two controls):before (origin/main 9796235):
after:
Tests:
scripts/run_tests.sh tests/agent/transports/test_codex_app_server_session.py→ 35 passed;test_codex_app_server_runtime.py+test_codex_event_projector.py→ 45 passed.Root cause: the watchdog treated N seconds of post-tool wire silence as proof of a wedged process, but the app-server emits zero events during long reasoning phases while remaining fully responsive (the interrupt was acknowledged in ~25ms), so healthy turns were killed and surfaced as
protocol_violation/crashedworkers.Fixes #112928
Salvages #112932 (@KoNit-K) — same direction (silence must not retire); this PR keeps the quiet threshold as a logged warning instead of deleting the timer and the
before_pollhook, and keeps the existing reset-on-activity test.Dropped hunks
post_tool_quiet_timeoutkwarg, thebefore_pollhook on_drive_turn, and the quiet timer itself — retained as observability so operators can still see long post-tool silences in the log (the reporter's own recommendation).Infographic
Review follow-up
watchdog trippedcomment atagent/codex_runtime.py:493and thetest_post_tool_watchdog_resets_on_further_activityname/docstring — fixed @54623b0: comment now lists only the real retirement causes and notes post-tool silence only warns (Codex app-server sessions are incorrectly retired after 90s of post-tool silence #112928); test renamedtest_post_tool_activity_clears_the_quiet_timer_and_never_retireswith a docstring on the warning semantics (assertions unchanged; 35/35 file tests pass).turn_timeout→ residual (below); larger than a cheap fix.Known residuals
agent/transports/codex_app_server_session.py::_run_started_turn.warn_if_quiet: The case fix(codex-runtime): retire wedged sessions + post-tool watchdog + OAuth refresh classify #25769 (12f755c) deliberately guarded — a codex subprocess that stays alive but never emits (CPU-spin/wedge) — is no longer fast-failed and now burns the full turn_timeout, which the only production caller (agent/codex_runtime.py run_turn(user_input=...)) leaves at the hardcoded, non-configurable 600s default; not fail-open forever (deadline still interrupts+retires), but 90s→600s per wedged turn with no operator knob. Suggested: make turn_timeout configurable/plumbed from codex_runtime, or replace pure-silence retirement with an RPC liveness ping so a genuinely wedged-alive process is still retired early.