Skip to content

fix(codex): crash a turn codex ended as failed instead of grading it - #230

Merged
tmatup merged 2 commits into
mainfrom
fix/codex-failed-turn-is-crash
Oct 21, 2026
Merged

tmatup merged 2 commits into
mainfrom
fix/codex-failed-turn-is-crash

Conversation

@tmatup

@tmatup tmatup commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Bottom line

When codex gives up on the model provider (for example a rate limit it will not retry), the codex harness now crashes the turn instead of grading it. The row ends as ERROR (or TIMEOUT if the retry backoff outruns task_timeout) instead of FAILURE. Before this, provider outages looked like skill regressions.

What was wrong

Codex does not raise when it gives up. It sends a normal turn/completed whose turn has status: failed and a TurnError:

"status": "TurnStatus.failed",
"error": {
  "codex_error_info": "rate_limit_exceeded",
  "message": "rate limit exceeded: Your requests to gpt-5.6-luna for gpt-5.6-luna in eastus2 have exceeded token rate limit."
}

on_turn_completed accepted any valid TurnCompletedNotification, and _communicate_turn never read turn.status. So the turn booked as COMPLETED, the criteria ran on a half-done workspace, and the row was FAILURE. #203 recorded the error notifications in provider_errors, but nothing acted on them.

Evidence: run adhoc-2026-10-21_16-16-27 (codex, gpt-5.6-luna, Maestro case v1 vs v2):

  • 27 of the run's rows (11 v1, 16 v2) ended on a rateLimitExceeded with will_retry=false.
  • That error was always the last event of the turn, after 0 to 39 commands.
  • 21 of them were graded FAILURE (8 v1, 13 v2); the other 6 scored SUCCESS despite the cut. For example, v2/skill-case-diagnose-formal-arg-slot-ids scored 0.0 with 0 commands.

The fix

  • _communicate_turn checks the committed Turn. If status == failed and the pump did not stop on purpose (early stop or turn cap), it calls _finalize_and_raise_crash with Codex turn failed: <provider message> (codexErrorInfo=<kind>).
  • This matches how Pi and OpenCode already handle a terminal provider error (.claude/notes/agents.md § Why a clean exit can still be a crash). The note now covers Codex as well.
  • The check looks at the turn status, not at provider_errors[].will_retry. A turn that fails without an error notification is caught too.
  • Category routing is unchanged. categorize_error maps the rate limit exceeded text to AGENT_RATE_LIMIT, which retries 5 times starting at a 60 s delay. If the retries run out, the row ends as ERROR. With a 2400 s task_timeout, the task watchdog may fire first, which gives TIMEOUT. Both are execution-fact statuses, not graded verdicts.
  • The partial turn keeps its commands and provider_errors, and post-failure criteria still run on it.
  • Small refactor: _codex_error_kind now parses codexErrorInfo for both ProviderError and the crash message. ProviderError output is unchanged.

Reporting is unchanged on purpose. RunSummary.pass_rate counts ERROR rows as misses and reports error_share separately. That is the existing convention, so this PR leaves it alone.

Tests

New TestFailedTurnIsACrash in tests/test_codex_agent.py. It replays the real notification shape from the run above: 5 responseStreamDisconnected errors with will_retry=true, then one rateLimitExceeded with will_retry=false, then a failed turn. It covers:

  • the turn raises AgentCrashError, with 0 commands and with 4 commands;
  • the partial keeps its commands and all 6 provider errors;
  • the crash is categorized as AGENT_RATE_LIMIT;
  • a failed turn with no error details still crashes;
  • a completed turn after a retried error does not crash.

Without the fix, 4 of the 5 new tests fail. The negative control passes either way. make verify passes: ruff, pyright with 0 errors, custom lint, and 6392 passed / 3 skipped.

🤖 Generated with Claude Code
Co-Authored-By: Claude

Codex delivers a normal turn/completed when it gives up on the provider,
with turn.status == failed and a TurnError (for example rateLimitExceeded
after its own stream retries ran out). The harness never read that status,
so the turn booked as COMPLETED and the row was graded FAILURE on work the
provider had cut off.

In adhoc-2026-10-21_16-16-27 (codex, gpt-5.6-luna) 27 rows (11 v1, 16 v2)
ended this way, after 0 to 39 commands, and read as skill failures in the
v1-vs-v2 comparison.

The turn now crashes with the provider's message, the same way Pi and
OpenCode already treat a terminal provider error. categorize_error routes
the "rate limit exceeded" text to AGENT_RATE_LIMIT and its retry budget;
when retries run out the row lands as ERROR. The partial turn keeps its
commands and provider_errors.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013fePq8hgqbtNvf2pNvA3WC
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Claude finished @tmatup's task in 1m 37s —— View job


Code Review in Progress

Todo List:

  • Read .github/code_review.md for review standards
  • Read CLAUDE.md for project conventions
  • Get full diff with git diff origin/main...HEAD
  • Read full context of each changed file
  • Perform cross-file consistency checks
  • Check for missing elements
  • Provide comprehensive review feedback

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Claude finished @tmatup's task in 1m 33s —— View job


Code Review in Progress

  • Read review guidelines in .github/code_review.md
  • Read CLAUDE.md for project conventions
  • Analyze the full diff (origin/main...HEAD)
  • Review changed files with full context
  • Check for correctness and edge cases
  • Verify test coverage
  • Perform cross-file consistency checks
  • Complete "What's Missing" analysis
  • Post comprehensive review feedback

@tmatup

tmatup commented Oct 21, 2026

Copy link
Copy Markdown
Member Author

CI triage. Short version: neither red check comes from this PR's diff, which only touches codex_agent.py, its tests and .claude/notes/agents.md. No code change pushed.

Check Cause Action Status
Quality Gate (required) The pip-audit step found 3 newly published advisories in uv.lock: certifi 2026.2.25 (PYSEC-2026-4320) and virtualenv 21.14.2 (CVE-2026-113725, CVE-2026-113726). Main was last green on 10-19, before they were published, so every PR fails the same way. Re-ran once and got the same result. Filed #231 with the fix (uv lock --upgrade-package certifi --upgrade-package virtualenv; both fix versions are past the 12 h safe-chain age). Red until #231 lands; then rebase or re-run.
Delegate Live E2E (ROPC) The live Delegate alpha agent (virtuoso-1-5) refused the task: "I can't create or run a standalone Python script unrelated to UiPath", so fizzbuzz.py was never written. This is model behavior on the service side, not harness code. Re-ran once. Green on attempt 4.

Why local make verify passed: the Makefile has the pip-audit line commented out (line 74), so make verify never checks the live advisory database. #231 notes this.

The Claude Code Review run ended with is_error: true on both attempts and left only its todo placeholder. No findings or threads were posted. #225, #227 and #228 show the same placeholder, so this is a problem in the review workflow, not this PR.

@tmatup

tmatup commented Oct 21, 2026

Copy link
Copy Markdown
Member Author

The Quality Gate failure on this PR is the pip-audit advisory issue #231 (certifi and virtualenv), not this change. The fix is #232 (only uv.lock). Once #232 lands, this PR can be rebased on main and the gate should go green.

🤖 Generated with Claude Code
Co-Authored-By: Claude

@tmatup
tmatup merged commit 30c62ce into main Oct 21, 2026
17 checks passed
@tmatup
tmatup deleted the fix/codex-failed-turn-is-crash branch October 21, 2026 23:02
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.

2 participants