Skip to content

fix(codex): time out silent app-server turns#82142

Merged
steipete merged 1 commit into
mainfrom
codex/fix-codex-app-server-silent-turn-timeout
May 15, 2026
Merged

fix(codex): time out silent app-server turns#82142
steipete merged 1 commit into
mainfrom
codex/fix-codex-app-server-silent-turn-timeout

Conversation

@steipete

Copy link
Copy Markdown
Contributor

Summary

  • Arm the short Codex app-server completion-idle watchdog immediately after turn/start accepts a turn, before replaying queued notifications.
  • Keep queued early turn progress from causing false silent-turn interrupts.
  • Update Codex plugin config help, docs, reference docs, and changelog.

Fixes #82129.

Verification

Behavior addressed: Codex app-server turns accepted by turn/start but emitting no current-turn progress now interrupt and release the OpenClaw session lane after appServer.turnCompletionIdleTimeoutMs instead of waiting for the outer model timeout.
Real environment tested: Blacksmith Testbox tbx_01krntdyatxvkm91rap5bb69sr; local Codex review.
Exact steps or command run after this patch:

  • pnpm test extensions/codex/src/app-server/run-attempt.test.ts -- --reporter=verbose
  • node scripts/run-vitest.mjs extensions/codex/src/app-server/run-attempt.test.ts -t "releases the session when Codex accepts|does not treat global rate-limit notifications as turn progress|does not time out when turn progress arrives"
  • pnpm check:changed
  • codex review --uncommitted
  • git diff --check
  • pnpm docs:list
    Evidence after fix: run-attempt.test.ts now proves accepted/no-progress turns use the short completion-idle watchdog, global rate-limit notifications do not reset turn progress, and queued turn/started progress before turn/start returns does not falsely time out.
    Observed result after fix: focused Codex app-server Vitest passed 119 tests; targeted Codex review Vitest passed 3 selected tests; Testbox pnpm check:changed exited 0; Codex review reported no actionable correctness issues.
    What was not tested: live Codex app-server/OAuth model call; coverage is via mocked app-server harness plus changed-check lanes.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation extensions: codex size: S maintainer Maintainer-authored PR labels May 15, 2026
@clawsweeper

clawsweeper Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge.

Summary
The PR arms the Codex app-server completion-idle watchdog immediately after turn/start, adds focused timeout coverage, and updates Codex plugin docs, manifest help, and changelog text.

Reproducibility: yes. Current main source shows the accepted-turn path calls touchTurnCompletionActivity("turn:start") without arming the short completion-idle watchdog, and the linked regression provides CLI steps plus timeout logs; I did not run a live Codex/OAuth reproduction in this read-only review.

Real behavior proof
Not applicable: The PR is protected with the maintainer label, so the external-contributor real-behavior proof gate does not apply; the body still records Testbox mocked-harness proof and explicitly notes no live Codex app-server/OAuth call.

Next step before merge
Protected maintainer label plus an open implementation PR means the next action is maintainer review, checks, and landing rather than cleanup closure or ClawSweeper repair automation.

Security
Cleared: The diff only changes Codex plugin timeout logic, focused tests, docs/help text, and changelog; I found no concrete security or supply-chain concern.

Review details

Best possible solution:

Merge this narrow Codex plugin fix after maintainer-approved checks/proof, then close #82129 as fixed by the landed PR.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main source shows the accepted-turn path calls touchTurnCompletionActivity("turn:start") without arming the short completion-idle watchdog, and the linked regression provides CLI steps plus timeout logs; I did not run a live Codex/OAuth reproduction in this read-only review.

Is this the best way to solve the issue?

Yes. The PR uses the existing completion-idle watchdog at the accepted-turn boundary and preserves queued early progress handling, which is the narrow maintainable fix for the observed gap.

Acceptance criteria:

  • pnpm test extensions/codex/src/app-server/run-attempt.test.ts -- --reporter=verbose
  • node scripts/run-vitest.mjs extensions/codex/src/app-server/run-attempt.test.ts -t "releases the session when Codex accepts|does not treat global rate-limit notifications as turn progress|does not time out when turn progress arrives"
  • pnpm check:changed
  • git diff --check

What I checked:

  • Current main does not arm the short watchdog at accepted-turn start: Current main sets turnTerminalIdleWatchArmed = true and calls touchTurnCompletionActivity("turn:start") after pending notification replay, but does not pass { arm: true }, so the completion-idle timer is not armed for an accepted turn with no current-turn progress. (extensions/codex/src/app-server/run-attempt.ts:1672, d91f58ee25b5)
  • Existing helper contract requires explicit arming: touchTurnCompletionActivity only flips turnCompletionIdleWatchArmed when options.arm is true, matching the PR's approach of reusing the existing watchdog rather than adding a parallel timeout path. (extensions/codex/src/app-server/run-attempt.ts:1121, d91f58ee25b5)
  • PR diff is narrow and targets the implicated boundary: The PR moves terminal-watch arming before pending notification replay and changes touchTurnCompletionActivity("turn:start") to touchTurnCompletionActivity("turn:start", { arm: true }), with matching tests/docs/help/changelog updates. (extensions/codex/src/app-server/run-attempt.ts:1638, 1bb4916cc073)
  • Focused tests cover the timeout behavior and early progress guard: The PR switches the silent accepted-turn tests from turnTerminalIdleTimeoutMs to turnCompletionIdleTimeoutMs and adds coverage that a queued turn/started notification before turn/start returns does not trigger a false interrupt. (extensions/codex/src/app-server/run-attempt.test.ts:1916, 1bb4916cc073)
  • Live PR state is protected and not a cleanup-close candidate: GitHub API shows the PR is open, non-draft, mergeable, at head 1bb4916cc0733c0f50fcc51e7c375bd42e7165dd, and labeled maintainer; protected labels require maintainer handling rather than automated stale/cleanup closure. (1bb4916cc073)
  • Linked regression provides user-level reproduction evidence: The linked open bug reports OpenClaw 2026.5.12 Codex embedded tool/status turns timing out while text-only turns succeed, with CLI reproduction steps and sanitized timeout/stalled-session logs.

Likely related people:

  • steipete: Current-main blame for the implicated watchdog/turn-start code points to Peter Steinberger's Codex-area commit, and the local shortlog over the touched Codex app-server, manifest, and docs paths shows Peter as the primary contributor. (role: dominant Codex area contributor; confidence: high; commits: c0fe7ab34ab8, f066dd2f31c2, 4862d349257a; files: extensions/codex/src/app-server/run-attempt.ts, extensions/codex/src/app-server/run-attempt.test.ts, extensions/codex/openclaw.plugin.json)
  • duqaXxX: Recent history on the Codex plugin/docs/manifest surface includes Codex harness activation work, making this a plausible secondary routing signal for adjacent plugin behavior, though not the central timeout code. (role: adjacent Codex plugin contributor; confidence: low; commits: 69ba924b530e; files: extensions/codex/openclaw.plugin.json, docs/plugins/codex-harness.md, docs/plugins/codex-harness-reference.md)

Remaining risk / open question:

  • The PR body reports mocked app-server harness and Testbox changed-check proof, but no live Codex app-server/OAuth model call after the patch; the protected maintainer review should decide whether that proof is sufficient before landing.

Codex review notes: model gpt-5.5, reasoning high; reviewed against d91f58ee25b5.

Re-review progress:

@steipete
steipete marked this pull request as ready for review May 15, 2026 13:35
@steipete
steipete force-pushed the codex/fix-codex-app-server-silent-turn-timeout branch from 1bb4916 to c9d8990 Compare May 15, 2026 13:46
@steipete

Copy link
Copy Markdown
Contributor Author

Landing proof for rebased head c9d8990b03fe39eae00b56ccdb596cf5acee54f2:

  • pnpm test extensions/codex/src/app-server/run-attempt.test.ts -- --reporter=verbose -> 119 passed
  • env -u OPENCLAW_TESTBOX -u OPENCLAW_TESTBOX_REMOTE_RUN pnpm check:changed -> passed
  • codex review --uncommitted before commit -> no actionable correctness issues
  • git diff --check before commit -> passed
  • Earlier pre-rebase Testbox proof: pnpm check:changed on tbx_01krntdyatxvkm91rap5bb69sr -> exit 0
  • Rebased Testbox attempt tbx_01krnxkn8kve790fp46e484ne2 did not acquire a worker and exited 143 while still queued; no remote test failure observed.

Known proof gap: no live Codex app-server/OAuth model call; covered by mocked app-server regression tests plus changed gates.

@steipete
steipete merged commit 1e31bd2 into main May 15, 2026
84 of 85 checks passed
@steipete
steipete deleted the codex/fix-codex-app-server-silent-turn-timeout branch May 15, 2026 13:46
@steipete

Copy link
Copy Markdown
Contributor Author

Landed via rebase onto main.

  • Gate: pnpm test extensions/codex/src/app-server/run-attempt.test.ts -- --reporter=verbose -> 119 passed
  • Gate: env -u OPENCLAW_TESTBOX -u OPENCLAW_TESTBOX_REMOTE_RUN pnpm check:changed -> passed
  • Review: codex review --uncommitted before commit -> no actionable correctness issues
  • Source head: c9d8990
  • Landed commit: 1e31bd2

Thanks @Francois3d for the report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Improvements or additions to documentation extensions: codex maintainer Maintainer-authored PR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: 2026.5.12 regression: Codex embedded tool/status runs time out while text-only runs succeed

1 participant