Skip to content

fix(codex): add progress-stall watchdog for keepalive-only Codex hangs - #27

Merged
Kyzcreig merged 1 commit into
mainfrom
fix/codex-progress-stall-watchdog
Jun 9, 2026
Merged

Kyzcreig merged 1 commit into
mainfrom
fix/codex-progress-stall-watchdog

Conversation

@Kyzcreig

@Kyzcreig Kyzcreig commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Problem

chatgpt.com/backend-api/codex has a failure mode where it keeps the
socket alive with periodic keepalive / response.in_progress frames but
never emits a real delta or completes. Both existing Codex watchdogs
are satisfied by these frames:

  • the no-byte TTFB detector sees events arriving (_codex_stream_last_event_ts is set), so it never fires;
  • the stream-idle detector sees the keepalive gap stay under its threshold, so it never fires.

So the call rides the socket all the way to the blunt wall-clock stale
timeout and burns it on every retry — observed in production: 90s × 6 retries ≈ 9 min of dead time before the fallback model kicks in. A fresh
reconnect to the same backend succeeds in ~1s (verified by direct probe).

This killed a scheduled digest run during a ~35-min Codex blip: 22 consecutive
Non-streaming API call stale for 90s kills, the 40s fast-reconnect cutoff
armed every retry but never won.

Root cause

run_codex_stream reset _codex_stream_last_progress_ts (chat_completion_helpers.py:378) but never set or read it anywhere — a dead, half-wired signal. The intended "forward progress" watchdog was stubbed and never finished.

Fix

Wire up the progress signal and add the watchdog:

  1. codex_runtime._on_event now stamps _codex_stream_last_progress_ts only on real forward progress — text/reasoning deltas, function_call frames, response.output_item.done, terminal/error events — distinct from _codex_stream_last_event_ts, which any frame (including keepalives) refreshes.
  2. A progress-stall watchdog in interruptible_api_call kills a stream that is emitting frames but making zero forward progress past the fast-reconnect cutoff (default = the no-byte TTFB cutoff, clamped strictly below the stale timer), so the retry loop reconnects in ~1s instead of waiting out the blunt stale timeout. Tunable via HERMES_CODEX_PROGRESS_STALE_TIMEOUT_SECONDS (0 disables). Close reason: codex_progress_stall_kill.

The no-byte (first-byte) and stream-idle paths are unchanged; the new branch only arms once at least one event has arrived, so it never races the no-byte detector.

Verification

  • Reproduced the exact prod signature in a harness: a keepalive-only stall that burned 92s before now dies at ~42s with codex_progress_stall_kill.
  • Two regression tests added next to the existing watchdog suite:
    • test_progress_stall_kills_keepalive_only_stream — keepalive-only stream is killed fast via the progress path, not the 90s stale path.
    • test_progress_stall_does_not_kill_streaming_progress — a stream making real progress for longer than the fast cutoff is not killed.
  • Gold-standard proof: with the impl reverted, test_progress_stall_kills_keepalive_only_stream FAILS (rides to the ~90s stale timer, elapsed ≫ 20s); with the fix it passes at ~42s.
  • Full tests/agent/test_codex_ttfb_watchdog.py suite: 13 passed (11 existing + 2 new), on both the fork base and upstream base.

The chatgpt.com/backend-api/codex endpoint has a failure mode where it
keeps the socket alive with periodic keepalive / response.in_progress
frames but never emits a real delta or completes. Both existing
watchdogs were satisfied by these frames (the no-byte TTFB detector saw
events arriving; the stream-idle detector saw the keepalive gap stay
under its threshold), so the call rode the socket all the way to the
blunt wall-clock stale timeout — burning the full timeout on every
retry (observed: 90s x 6 retries ~= 9 min) before falling back.

run_codex_stream already reset _codex_stream_last_progress_ts but never
set or read it — a dead, half-wired signal. Wire it up:

- codex_runtime._on_event now stamps _codex_stream_last_progress_ts only
  on REAL forward progress (text/reasoning deltas, function-call frames,
  output-item completions, terminal/error events), distinct from
  _codex_stream_last_event_ts which any frame (incl. keepalive) refreshes.
- A new progress-stall watchdog in interruptible_api_call kills a stream
  that is emitting frames but making zero forward progress past the
  fast-reconnect cutoff (default = the no-byte TTFB cutoff, clamped below
  the stale timer), so the retry loop reconnects in ~1s instead of
  waiting out the blunt stale timeout. Tunable via
  HERMES_CODEX_PROGRESS_STALE_TIMEOUT_SECONDS (0 disables).

Verified against the captured prod failure signature: a keepalive-only
stall that burned 92s now dies at ~42s with codex_progress_stall_kill.
Two regression tests added; the no-byte and slow-first-event paths are
unchanged.
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

🔎 Lint report: fix/codex-progress-stall-watchdog vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 9955 on HEAD, 9955 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 5159 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@Kyzcreig
Kyzcreig merged commit cb30b79 into main Jun 9, 2026
23 checks passed
@Kyzcreig
Kyzcreig deleted the fix/codex-progress-stall-watchdog branch June 9, 2026 16:51
@Kyzcreig
Kyzcreig restored the fix/codex-progress-stall-watchdog branch September 21, 2026 10:32
ang-fleet-workers Bot added a commit that referenced this pull request Sep 27, 2026
- kanban_db #23: the in-txn takeover audit fires only when the txn changed
  the target card's guarded columns (status/assignee/priority/session_id),
  not on any side write (reclaim_refused), so a refused mutation never records
  or re-homes. No longer reads conn.total_changes (fixes write_txn_busy_retry
  fakes, CI slice 13).
- kanban_db #44: birth home re-resolved at the TOP of the write txn, before
  the duplicate / parent checks.
- kanban_db #70: no own-claim exception by profile name; without a run id
  only a lapsed run (claim expired, no live owner) can be closed.
- checkout_admission #52: read-check-replace under a per-consumer flock.
- discord #30/#31: cancellation recorded before fut.done(); a cancelled
  disconnect waits for the in-flight flush and still runs every flush.
- run.py #36: lock held until the worker write finishes on cancel; every
  write runs in FIFO order (no skipped-but-reported resets).
- ci_overflow_integration #27: documented + tested that a secondary-limit 403
  arrives as RateLimited(429) and fails closed.

New tests, each red against the previous commit.
ang-fleet-workers Bot added a commit that referenced this pull request Sep 27, 2026
…, PR954)

PATCH a nonexistent variable: 403 = denied (PASS), anything else fails closed.
The live slot variables are never GET-then-PATCHed, so a fail-open identity
can no longer revert a controller change.

Tests: test_variable_write_probe_never_writes_an_existing_variable (red on
base), test_variable_write_probe_fails_closed_unless_403.
ang-fleet-workers Bot added a commit that referenced this pull request Sep 27, 2026
- kanban_db #23: the in-txn takeover audit fires only when the txn changed
  the target card's guarded columns (status/assignee/priority/session_id),
  not on any side write (reclaim_refused), so a refused mutation never records
  or re-homes. No longer reads conn.total_changes (fixes write_txn_busy_retry
  fakes, CI slice 13).
- kanban_db #44: birth home re-resolved at the TOP of the write txn, before
  the duplicate / parent checks.
- kanban_db #70: no own-claim exception by profile name; without a run id
  only a lapsed run (claim expired, no live owner) can be closed.
- checkout_admission #52: read-check-replace under a per-consumer flock.
- discord #30/#31: cancellation recorded before fut.done(); a cancelled
  disconnect waits for the in-flight flush and still runs every flush.
- run.py #36: lock held until the worker write finishes on cancel; every
  write runs in FIFO order (no skipped-but-reported resets).
- ci_overflow_integration #27: documented + tested that a secondary-limit 403
  arrives as RateLimited(429) and fails closed.

New tests, each red against the previous commit.
ang-fleet-workers Bot added a commit that referenced this pull request Sep 27, 2026
…, PR954)

PATCH a nonexistent variable: 403 = denied (PASS), anything else fails closed.
The live slot variables are never GET-then-PATCHed, so a fail-open identity
can no longer revert a controller change.

Tests: test_variable_write_probe_never_writes_an_existing_variable (red on
base), test_variable_write_probe_fails_closed_unless_403.
ang-fleet-workers Bot added a commit that referenced this pull request Sep 27, 2026
- kanban_db #23: the in-txn takeover audit fires only when the txn changed
  the target card's guarded columns (status/assignee/priority/session_id),
  not on any side write (reclaim_refused), so a refused mutation never records
  or re-homes. No longer reads conn.total_changes (fixes write_txn_busy_retry
  fakes, CI slice 13).
- kanban_db #44: birth home re-resolved at the TOP of the write txn, before
  the duplicate / parent checks.
- kanban_db #70: no own-claim exception by profile name; without a run id
  only a lapsed run (claim expired, no live owner) can be closed.
- checkout_admission #52: read-check-replace under a per-consumer flock.
- discord #30/#31: cancellation recorded before fut.done(); a cancelled
  disconnect waits for the in-flight flush and still runs every flush.
- run.py #36: lock held until the worker write finishes on cancel; every
  write runs in FIFO order (no skipped-but-reported resets).
- ci_overflow_integration #27: documented + tested that a secondary-limit 403
  arrives as RateLimited(429) and fails closed.

New tests, each red against the previous commit.
ang-fleet-workers Bot added a commit that referenced this pull request Sep 28, 2026
…, PR954)

PATCH a nonexistent variable: 403 = denied (PASS), anything else fails closed.
The live slot variables are never GET-then-PATCHed, so a fail-open identity
can no longer revert a controller change.

Tests: test_variable_write_probe_never_writes_an_existing_variable (red on
base), test_variable_write_probe_fails_closed_unless_403.
ang-fleet-workers Bot added a commit that referenced this pull request Sep 28, 2026
- kanban_db #23: the in-txn takeover audit fires only when the txn changed
  the target card's guarded columns (status/assignee/priority/session_id),
  not on any side write (reclaim_refused), so a refused mutation never records
  or re-homes. No longer reads conn.total_changes (fixes write_txn_busy_retry
  fakes, CI slice 13).
- kanban_db #44: birth home re-resolved at the TOP of the write txn, before
  the duplicate / parent checks.
- kanban_db #70: no own-claim exception by profile name; without a run id
  only a lapsed run (claim expired, no live owner) can be closed.
- checkout_admission #52: read-check-replace under a per-consumer flock.
- discord #30/#31: cancellation recorded before fut.done(); a cancelled
  disconnect waits for the in-flight flush and still runs every flush.
- run.py #36: lock held until the worker write finishes on cancel; every
  write runs in FIFO order (no skipped-but-reported resets).
- ci_overflow_integration #27: documented + tested that a secondary-limit 403
  arrives as RateLimited(429) and fails closed.

New tests, each red against the previous commit.
Kyzcreig pushed a commit that referenced this pull request Sep 28, 2026
ang-fleet-workers Bot added a commit that referenced this pull request Sep 28, 2026
ang-fleet-workers Bot added a commit that referenced this pull request Sep 28, 2026
ang-fleet-workers Bot added a commit that referenced this pull request Oct 4, 2026
…w#27)

fleet-floor-workflows#27 dropped the ref-keyed cancel-in-progress group from the
reusable override-lint.yml, so a push + PR-body edit on one head no longer leaves a
cancelled override_lint twin. Pin is the #27 merge commit (REST merge_commit_sha).

Card: t_67545768
ang-fleet-workers Bot added a commit that referenced this pull request Oct 14, 2026
…8370e)

SHA-only. Callee concurrency/on: non-comment content identical to the old pin
(js-ci, js-typecheck, js-dep-audit, sast, secret-scan; override-lint: comment-only
diff since b701cc0). R18 control at 68136b5 rc=0: push:main backstop-safe, PR supersedes.
ang-fleet-workers Bot added a commit that referenced this pull request Oct 14, 2026
…8370e)

SHA-only. Callee concurrency/on: non-comment content identical to the old pin
(js-ci, js-typecheck, js-dep-audit, sast, secret-scan; override-lint: comment-only
diff since b701cc0). R18 control at 68136b5 rc=0: push:main backstop-safe, PR supersedes.
Kyzcreig added a commit that referenced this pull request Oct 14, 2026
… (t_7458370e) (#1994)

* ci: reusable callees never cancel a caller's push:main backstop (R18) (t_7458370e)

tests.yml, e2e-desktop{,-core,-update}.yml, lockfile-diff.yml and windows-install-update-e2e.yml
are workflow_call callees of ci.yaml/ci-local.yaml. Their github.ref-keyed cancel-in-progress group
applies to every caller run, so the next merge cancelled the running push:main backstop (2 of the
last 18 ci.yaml push:main runs ended cancelled). On refs/heads/main the group is now github.run_id
and cancel-in-progress is false. Every other ref (PR, merge_group queue refs, wine2e-install/**)
keeps its previous group and cancel.

Verified: ci_speed_lint R18 ERROR 6 -> 0 on these files; yaml.safe_load over all workflows ok.

* ci: pin fleet-floor-workflows callers to 68136b5 (#27 #28 #29) (t_7458370e)

SHA-only. Callee concurrency/on: non-comment content identical to the old pin
(js-ci, js-typecheck, js-dep-audit, sast, secret-scan; override-lint: comment-only
diff since b701cc0). R18 control at 68136b5 rc=0: push:main backstop-safe, PR supersedes.

---------

Co-authored-by: Kyzcreig <9063726+Kyzcreig@users.noreply.github.com>
Co-authored-by: ang-fleet-workers[bot] <333956806+ang-fleet-workers[bot]@users.noreply.github.com>
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