Skip to content

fix: thread turn-timeout-ms into adversarial-review's app-server turn - #39

Closed
axisrow wants to merge 1 commit into
mainfrom
ao/codex-plugin-cc-6/fix-adversarial-turn-timeout-38
Closed

fix: thread turn-timeout-ms into adversarial-review's app-server turn#39
axisrow wants to merge 1 commit into
mainfrom
ao/codex-plugin-cc-6/fix-adversarial-turn-timeout-38

Conversation

@axisrow

@axisrow axisrow commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Summary

Closes #38.

executeReviewRun's adversarial-review branch called runAppServerTurn without turnTimeoutMs, even though the native review branch (runAppServerReview) and the task branch both pass it through. As a result, --turn-timeout-ms was silently ignored for adversarial-review — a stalled turn fell back to the library's 600s DEFAULT_TURN_TIMEOUT_MS instead of the caller's requested budget.

The fix is a one-line addition of turnTimeoutMs: request.turnTimeoutMs alongside the existing effort/sandbox options in that call (plugins/codex/scripts/codex-companion.mjs).

Context

Same investigation that turned up this bug also looked at the env-var-based turn-budget approach landing upstream in openai#376. That PR's asymmetry doesn't reproduce here — there the budget is threaded via an env mutation that both branches pick up implicitly. This bug is fork-specific: a side effect of this fork's move to options-threading in fork PR #28 (fix: turn-timeout gaps), where the native-review and task call sites were updated but the adversarial-review call site was missed.

No version bump — 1.0.6-fork.6 was just released; this isn't a release PR.

Test plan

  • Added a red test first (tests/runtime.test.mjs): adversarial-review with stalled turn/start honors --turn-timeout-ms instead of the library default, modeled on the existing task with stalled turn/start times out via --turn-timeout-ms instead of hanging forever test. Confirmed it hung toward the 600s library default before the fix.
  • Applied the one-line fix; test now passes in ~5s.
  • npm test: 175/178 pass. The 3 failures are pre-existing state-dir/job-listing flakes unrelated to this change (same failures reproduce on fork/main before this commit).
  • npm run build: passes (tsc -p tsconfig.app-server.json), no new TS errors.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BuAt7yyXHmZPpKFTJTkpdP

…closes #38)

The adversarial-review branch of executeReviewRun called runAppServerTurn
without turnTimeoutMs, even though the native review branch and the task
branch both pass it through. --turn-timeout-ms was silently ignored for
adversarial-review, so a stalled turn fell back to the library's 600s
default instead of the requested budget.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BuAt7yyXHmZPpKFTJTkpdP
@axisrow

axisrow commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

🔍 Local review (cycle 1)

Reviewed locally (/review + Codex companion, model=sol effort=xhigh), no bots pinged.

Verdict Reviewer Finding Location
(none) claude No issues found — one-line fix mirrors the review/task call sites, correctly wired to resolveTurnTimeoutMsFromOptions; test is a faithful adaptation of the existing task stalled-turn test. plugins/codex/scripts/codex-companion.mjs:470
(none) codex Codex verdict: approve — "No critical ship-blocking bug, vulnerability, data-loss risk, or logical failure is supported by the provided diff."

Totals: 0 FIX, 0 SKIP, 0 UNVERIFIED. Clean review — no blocking issues.

@axisrow

axisrow commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

📋 Review summary — all cycles

Cycle Reviewer Finding Verdict Resolution
1 claude No critical issues — fix correctly mirrors review/task call sites (none) N/A
1 codex approve — no ship-blocking bug/vuln/data-loss/logical failure found (none) N/A

Totals: 0 FIX, 0 SKIP, 0 UNVERIFIED. Clean on the first cycle — no cleanup pass needed.

Review mode: local. This PR was not merged automatically (local mode is review-only on merge) — merge is yours to trigger when ready.

@axisrow

axisrow commented Aug 2, 2026

Copy link
Copy Markdown
Owner Author

Superseded by #44 (identical fix, already merged, closed #38).

@axisrow axisrow closed this Aug 2, 2026
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.

adversarial-review ignores --turn-timeout-ms — turnTimeoutMs not forwarded to runAppServerTurn

1 participant