Skip to content

fix(evals): fail eval runs that never started or had no sandbox - #10077

Merged
kyle-sexton merged 2 commits into
mainfrom
fix/evals-sandbox-validity-gate
Oct 25, 2026
Merged

kyle-sexton merged 2 commits into
mainfrom
fix/evals-sandbox-validity-gate

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

Closes #9514
Closes #9611

Summary

run-validity.py called two kinds of broken eval run VALID. In one, a run ended at 0 turns with error: null because the eval sandbox could not write its own temp directory (#9514). In the other, every Bash call failed because the shell sandbox never started under a long TMPDIR (#9611). Both scored a session that never ran, or ran with no shell. The owner chose one check for both causes (session, 2026-10-25).

Fix

  • A new session started check in run-validity.py, applied to both arms. It fails a run that ended at 0 turns with no run error, and a run whose kept trace shows the sandbox failing to start. Rows that run errors already fails are skipped, so nothing is reported twice.
  • reference/sandboxes.md gains a Sandbox start section. It names the workaround (a short TMPDIR outside the system temp directory) and its pointer record: [BUG] plugin eval: sandbox denies every write in the agent's workspace — //tmp denyWrite covers the /tmp/claude-eval-* root anthropics/claude-code#104631 and #104797, and unix(7) for the long-TMPDIR case (a correlation; no upstream source names it as the cause). The hub's existing "when error is null, run the gate" Gotcha points there, which keeps SKILL.md under its 500-line cap.

Verification

Related

🤖 Generated with Claude Code

run-validity.py passed runs that ended at 0 turns with error null, and
runs whose every Bash call failed because the shell sandbox never
started. Both scored a session that never ran. A new session started
check fails either, in both arms. The plugin-eval sandbox notes give
the short-TMPDIR workaround with pointers to the upstream issues.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 25, 2026 18:06
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-25T18:08:07.359435Z 6712e1f Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 24d0b3f01bb6c7a7a07aa680956e3f0997ab67f7: 0 finding(s), 0 as inline comments.

No security issues found in the incremental change to run-validity.py and test_run_validity.py.

The change narrows the sandbox-failure scan to failed Bash tool results and skips the 0-turn check for runs whose trace already errored. It is an offline validity gate over local eval result files. It adds no new input sources, subprocess calls, network access, secrets handling or path handling.

The new helpers use only the existing as_list/as_dict guards and constants. They read the trace as data and run no code from it. Matching SANDBOX_FAILURE_TEXT against json.dumps of a result's content only decides a pass or fail verdict.

The tighter match could in theory let a run hide a sandbox failure by not emitting a Bash tool_use with a matching id. That is a self-reported eval trace with no attacker-controlled trust boundary, and run errors plus the 0-turn check still apply. I did not count it as a finding.

No manifest files, trusted paths or instruction surfaces changed in this increment, so there are no manifest <field> lines. The PR context has no embedded directives. No cost or hold applies.

critical-findings: 0

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6712e1f1a8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed 24d0b3f01bb6c7a7a07aa680956e3f0997ab67f7: 0 finding(s), 0 as inline comments.

Incremental review of run-validity.py and test_run_validity.py since 6712e1f. I found no issues, and I did not run the tests (this lane has no shell).

  • sandbox_failed: the scan now needs a failed tool_result (is_error true) whose tool_use_id matches an earlier Bash tool_use, so a trace that only quotes the sandbox text no longer fails. It uses the same as_list/as_dict guards as denials(). Both blocks are in the same trace, so ordering holds, because a result always follows its call.
  • trace_errored: the logic that check_errors already used now sits in one helper. check_started uses the same helper to skip a 0-turn run whose trace ended in an error. That run is therefore reported once, under run errors, and the new test covers it.
  • Tests: the new tests cover the error-trace dedupe and a sandbox-failure mention in a Read result and in a non-failed Bash result, which now passes. The existing sandbox-failure test now includes the Bash tool_use, so it still reaches the failing path.
  • Closing issues: the change still meets evals: run-validity calls runs VALID when every Bash call failed to start the sandbox (long TMPDIR) #9611, because a failed Bash result reporting the sandbox failure fails the run. It still meets evals: plugin eval with-arm runs end at 0 turns when a skill's ! pre-compute line fails in the sandbox, and the validity gate still says VALID #9514, because a 0-turn run with no error still fails. The narrowing leaves one gap: if the sandbox failure arrived as a non-Bash tool's error, or with is_error unset, the gate would not catch it. The issue's own trace shows Bash with is_error: true, so I am not flagging it.
  • Cost: nothing here changes spend or billing, and no hold applies.

@kyle-sexton

Copy link
Copy Markdown
Member Author
Finding (claude-review) Classification Evidence
session started skips only 2 of run errors' 3 conditions VALID (fix now) check_errors also fails a run whose trace result line has is_error: true (run-validity.py:343-349), and check_started does not skip it, so a 0-turn run that ends in a trace error is reported twice. Fix: one shared trace_errored helper used by both checks, plus a test for that case.

…rored traces

The session started check matched the sandbox failure string anywhere in
a trace, so a healthy run that read run-validity.py or quoted the error
was marked INVALID. It now counts only an is_error tool_result of a Bash
call. It also skipped only two of check_errors' three conditions, so a
0-turn run whose trace ended in an error was reported twice; both checks
now share trace_errored.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton

Copy link
Copy Markdown
Member Author

claude-review finding fixed in 24d0b3f: check_errors and check_started now share trace_errored, so a 0-turn run whose trace ends in an error is reported once, by run errors. test_a_zero_turn_run_whose_trace_ends_in_an_error_is_reported_once failed before the fix and passes now (57 tests OK).

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

Labels

None yet

Projects

None yet

1 participant