Skip to content

Only accept reward.txt written during verification - #1200

Merged
sergiopaniego merged 5 commits into
openenv:mainfrom
surajsharan:fix/verifier-reward-override
Sep 30, 2026
Merged

sergiopaniego merged 5 commits into
openenv:mainfrom
surajsharan:fix/verifier-reward-override

Conversation

@surajsharan

@surajsharan surajsharan commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

terminus_env, jupyter_env, coding_tools_env, opencode_env and pi_env let a float in <HOME>/logs/verifier/reward.txt override the verify pass rate. The file sits in the agent's own sandbox and was never cleared before verify ran, so an agent that wrote 1.0 there before submitting got full reward whatever the verify commands found. The value was parsed with a bare float(), so nan and inf became the reward as well.

In each env:

  • The reward file is deleted right before the verify commands run (rm -f ... && test ! -e ...). If it cannot be removed, for example because the agent made the directory read-only, the override is ignored for that run.
  • The override is accepted if it is a finite number. It comes from a verify command, so the task author sets it: negative rewards and values above 1 are kept as written, and only a non-finite value falls back to the pass rate, as in Harbor's verifier.
  • When an override is ignored, the reason is recorded in a new optional reward_override_ignored field on the env's state (RolloutResult for opencode and pi). It is not shown to the agent.

The second commit fixes a related verifier bug in jupyter_env, kept separate so it can be reverted on its own. Its run_shell never checked the return code: success was only False when the Python cell raised, so every verify command counted as passed and the pass-rate reward was always 1.0, and a failing setup command was not detected at reset. It now raises SystemExit on a non-zero code, the same way terminus_env's wrapper does. This one is visible to the agent: execute_shell_command now reports a failing command as a cell error instead of plain output, as terminus_env's terminal already does. The existing jupyter test only saw 1/2 passed because its fake returns failure for exit 1; the new test runs the real run_shell code against a local kernel.

The parsing helper is word-for-word identical in all five envs. I kept it per env on purpose: the envs read files through three different sandbox APIs, and a shared helper would mean a new core API. Happy to consolidate in a follow-up if you want one.

Tasks whose verify commands write the file behave exactly as before, which is what the READMEs describe. The READMEs now also say that the file is cleared before verify, and coding_tools_env's README documents the override for the first time. Docs stubs regenerated with scripts/sync_env_docs.py --fix.

Closes #1199

Type of Change

  • Bug fix
  • Documentation

Alignment Checklist

  • I have read .claude/docs/PRINCIPLES.md and this PR aligns with our principles
  • I have checked .claude/docs/INVARIANTS.md and no invariants are violated
  • I have run bash .claude/hooks/lint.sh and tests and addressed all issues

RFC Status

  • Not required (bug fix, docs, minor refactoring)

Test Plan

New tests for each env (tests/envs/test_{terminus,jupyter,coding_tools}_environment.py, new tests/envs/test_{opencode,pi}_reward_override.py). The first three extend the existing FakeSandbox so a shell command can write the reward file; the opencode and pi tests drive the real _run_rollout_impl with a fake session factory.

  • The agent writes 1.0 to the reward file before submitting, with one of two verify commands passing: reward is 0.5. coding_tools covers both its bash and write tools.
  • A verify command writes 0.8: reward is 0.8, as before.
  • A verify command writes nan, inf, -inf or passed: reward falls back to 0.5 and reward_override_ignored names the value.
  • A verify command writes -0.5 or 1.5: the reward is the value as written, since a verify command is the task author's.
  • The reward file cannot be removed before verify: the override is ignored.
  • terminus: the agent writes the file between two final_answer calls; the second submission still scores 0.5.
  • jupyter's run_shell returns success=False for exit 3 and a failing test -e, and True for true.

I also ran the exact clear and read commands through the real terminus, jupyter and coding_tools sandbox wrappers, with a local kernel executing the generated cell code, against real files: a missing file counts as cleared, a planted file is deleted, a directory or a read-only parent in the way makes cleared false, and a value written by a verify command reads back as 0.8.

On main, the same tests fail with the agent-written reward coming through as 1.0 in all five envs, nan and inf coming through as the reward, and jupyter reporting exit 3 as success.

Full suite with CI's dependency set and GITHUB_ACTIONS=true: 3217 passed, 103 skipped, 0 failed. Lint: ruff format --check and ruff check on src/ tests/ are clean, and scripts/sync_env_docs.py --check passes. usort check src/ tests/ flags only tests/envs/test_julia_env.py and tests/envs/test_grid_world.py, which this PR doesn't touch and which it flags on main too. The five env modules carry ruff format drift that is identical on main, so none of it comes from this change; the lines added here are formatted.

Not run against a live E2B or HF sandbox.

Out of scope

  • This closes the documented path (writing the file) and the value checks. It does not make in-sandbox verification tamper-proof. Verify still runs inside the agent's sandbox, so a process the agent leaves running in the background can write the file while verify runs, and in jupyter_env the agent shares the Python kernel that run_shell executes in: a code cell that replaces subprocess.run makes every later shell command, including the clear check and the verify commands, report success. Closing those needs verify to run outside the agent's reach, for jupyter through E2B's commands.run instead of the kernel, as opencode_env's backend already does (envs/opencode_env/sandbox/e2b.py:93). Happy to do that as a follow-up.
  • terminus_env lets final_answer be called more than once, and coding_tools_env scores an empty verify list as 0.0 where the other envs return None. Both change reward values.
  • Update Terminus env harness and sandbox backends #729 also touches terminus_env's _read_reward_override. The overlap is that one function, and I'll rebase whichever of the two lands second.

Claude Code Review

N/A

@cursor
cursor Bot requested review from Darktex and sergiopaniego September 18, 2026 10:25

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Alignment review — APPROVE

Head e88416ef. Closes the agent-written reward.txt cheat across terminus_env, jupyter_env, coding_tools_env, opencode_env, and pi_env, plus the jupyter run_shell pass-rate always-1.0 bug.

Verified

  • Clear-before-verify (rm -f … && test ! -e …) and the per-sandbox success flags (.success / .ok / exit_code == 0) look correct.
  • Override accepted only when finite and in [0, 1]; ignore reason stays on orchestration state / RolloutResult, not in MCP tool output.
  • Focused suite: 62/62 green locally (test_{terminus,jupyter,coding_tools}_environment, test_{opencode,pi}_reward_override).

Residual (author-acknowledged; out of scope here)

  • Background process can still write reward.txt during the verify window.
  • jupyter_env shares the kernel with the agent, so monkeypatching subprocess.run still defeats verify. Closing that needs verify outside the agent sandbox (e.g. E2B commands.run, as opencode_env already does).

Alignment notes (non-blocking)

  • jupyter_env execute_shell_command now surfaces non-zero exits as ERROR: SystemExit: N — intentional align with terminus, but a distribution shift for models trained on old traces.
  • Overlaps #729 on _read_reward_override in terminus; whichever lands second rebases that helper.

Gate

Fork PR — workflows are action_required. Needs a maintainer Approve and run before merge. Not a core-package / 0.5.1 cargo item unless we want the env reward integrity fix in the next cut's Spaces rebuild set.

View PR

Open in Web View Automation 

Sent by Cursor Automation: Release

@sergiopaniego sergiopaniego left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the PR and for the detailed issue (#1199)!

One change I'd like before merging, about the [0, 1] range check (inline comment below). Other than that it looks good to me.

A couple of small things:

  • CI hasn't run on this one yet and the branch is behind main. Could you rebase? We'll approve the workflows once it's updated.
  • About #729: it's outdated right now, so the plan is to merge this one first. No need to wait on it.

Follow-up, not for this PR: tbench2_env's _parse_canonical_reward also uses a bare float(), so nan/inf get through there too. Would be nice to add the same isfinite check in a separate PR if you're up for it.

except ValueError:
return None
return None, f"not a number: {raw[:40]!r}"
if not math.isfinite(value) or not 0.0 <= value <= 1.0:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Keeping the isfinite check makes sense, a nan reward breaks training. I'd drop the [0, 1] range though.

Once the file is cleared before verify, only the verify commands can write it, and those come from the task author, who we trust. So the range check doesn't guard against the agent anymore. What it does is limit what task authors can write. Negative rewards (penalties) or rewards above 1 are valid in RL, and with this change they would silently fall back to the pass rate, which is hard to debug. The READMEs also only say "a float", with no range.

It would also match Harbor, where the reward.txt convention comes from. Its parser rejects non-finite values but accepts any finite float (src/harbor/verifier/verifier.py).

Something like:

if not math.isfinite(value):
    return None, f"not finite: {raw[:40]!r}"

Same change in the other four envs (coding_tools_env, jupyter_env, opencode_env, pi_env), and the -0.5 / 1.5 test cases would move to the accepted side.

terminus_env, jupyter_env, coding_tools_env, opencode_env and pi_env let a
float in <HOME>/logs/verifier/reward.txt override the verify pass rate. The
file sits in the agent's own sandbox and was never cleared before verify
ran, so an agent could write 1.0 there and collect full reward. Any float,
including nan, inf and negatives, passed through unchecked.

Delete the file before the verify commands run and ignore it if it cannot
be removed, accept only finite values in [0, 1], and record why an
override was ignored in reward_override_ignored on the env state
(RolloutResult for opencode and pi).

Closes openenv#1199
E2BSandbox.run_shell ran the command through subprocess.run in the kernel
and printed its output, but never looked at the return code, so success
was only False when the Python cell itself raised. Every verify command
counted as passed, putting the pass-rate reward at 1.0 for any submission,
and a failing setup command went unnoticed at reset.

Raise SystemExit on a non-zero return code, as terminus_env's wrapper
already does. execute_shell_command now reports a failing command as a
cell error instead of plain output.

Refs openenv#1199
The override is only read from a file that is cleared before verification,
so the value comes from the task author's verify command, not from the
agent. A [0, 1] range check therefore no longer guards anything, and it
takes away rewards that are ordinary in RL: a penalty below zero, or a
score above one. Those would silently fall back to the pass rate, which
is hard to debug, and the READMEs only ever promised "a float".

Reject only what would poison training, as Harbor's verifier does, where
this reward.txt convention comes from: values that are not finite.
@surajsharan
surajsharan force-pushed the fix/verifier-reward-override branch from 60055ba to a47eeaf Compare September 29, 2026 18:15
@surajsharan

Copy link
Copy Markdown
Contributor Author

Thanks @sergiopaniego for the review. Both points are done.

The [0, 1] check is gone, isfinite stays. Your reasoning matches what the clear-before-verify step already guarantees: once the file only survives if a verify command writes it, the value is the task author's, and a range check stops being a guard against the agent and starts being a limit on the task author. Negative rewards and values above 1 are now kept as written, and only a value that is not a finite number falls back to the pass rate, with the reason still recorded. I checked Harbor's parser before changing it: _parse_reward_text in src/harbor/verifier/verifier.py accepts any finite float and raises only on non-finite, so this now matches where the convention comes from.

Same change in all five envs, with the READMEs and the generated docs stubs updated to say "a finite number" instead of a range. In the tests, -0.5 and 1.5 moved from the rejected list to a new case that asserts the value is kept as the reward; nan, inf, -inf and non-numeric text still fall back.

Rebased onto current main (4f4c85f), so CI can run on it.

#729: understood, nothing on my side depends on it.

tbench2: happy to take that one. Confirmed the same hole: _parse_canonical_reward does a bare float(raw), so nan or inf in reward.txt reaches _require_canonical_verdict and becomes the episode reward. Returning None for a non-finite value fits what that function already does with an unusable verdict: it raises, so the episode is reported as a scoring failure rather than trained on. I'll open it as a separate issue and PR.

Tests: full suite with CI's dependency set and GITHUB_ACTIONS=true, 3217 passed, 103 skipped, 0 failed. Lint and scripts/sync_env_docs.py --check pass.

@bot-ci-comment

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@sergiopaniego sergiopaniego left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

thanks!

@sergiopaniego
sergiopaniego merged commit 3ede040 into openenv:main Sep 30, 2026
11 checks passed
@cursor cursor Bot mentioned this pull request Oct 1, 2026
5 of 19 tasks
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.

Sandbox envs take their reward from a file the agent can write

2 participants