Repository navigation
Only accept reward.txt written during verification - #1200
Conversation
There was a problem hiding this comment.
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.txtduring the verify window. jupyter_envshares the kernel with the agent, so monkeypatchingsubprocess.runstill defeats verify. Closing that needs verify outside the agent sandbox (e.g. E2Bcommands.run, asopencode_envalready does).
Alignment notes (non-blocking)
jupyter_envexecute_shell_commandnow surfaces non-zero exits asERROR: SystemExit: N— intentional align with terminus, but a distribution shift for models trained on old traces.- Overlaps
#729on_read_reward_overridein 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.
Sent by Cursor Automation: Release
sergiopaniego
left a comment
There was a problem hiding this comment.
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: |
There was a problem hiding this comment.
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.
60055ba to
a47eeaf
Compare
|
Thanks @sergiopaniego for the review. Both points are done. The 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, Rebased onto current #729: understood, nothing on my side depends on it. tbench2: happy to take that one. Confirmed the same hole: Tests: full suite with CI's dependency set and |
|
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. |


Summary
terminus_env,jupyter_env,coding_tools_env,opencode_envandpi_envlet a float in<HOME>/logs/verifier/reward.txtoverride the verify pass rate. The file sits in the agent's own sandbox and was never cleared before verify ran, so an agent that wrote1.0there before submitting got full reward whatever the verify commands found. The value was parsed with a barefloat(), sonanandinfbecame the reward as well.In each env:
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.reward_override_ignoredfield on the env's state (RolloutResultfor 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. Itsrun_shellnever checked the return code:successwas onlyFalsewhen 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 raisesSystemExiton a non-zero code, the same wayterminus_env's wrapper does. This one is visible to the agent:execute_shell_commandnow reports a failing command as a cell error instead of plain output, asterminus_env'sterminalalready does. The existing jupyter test only saw1/2 passedbecause its fake returns failure forexit 1; the new test runs the realrun_shellcode 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 withscripts/sync_env_docs.py --fix.Closes #1199
Type of Change
Alignment Checklist
.claude/docs/PRINCIPLES.mdand this PR aligns with our principles.claude/docs/INVARIANTS.mdand no invariants are violatedbash .claude/hooks/lint.shand tests and addressed all issuesRFC Status
Test Plan
New tests for each env (
tests/envs/test_{terminus,jupyter,coding_tools}_environment.py, newtests/envs/test_{opencode,pi}_reward_override.py). The first three extend the existingFakeSandboxso a shell command can write the reward file; the opencode and pi tests drive the real_run_rollout_implwith a fake session factory.1.0to the reward file before submitting, with one of two verify commands passing: reward is0.5. coding_tools covers both itsbashandwritetools.0.8: reward is0.8, as before.nan,inf,-inforpassed: reward falls back to0.5andreward_override_ignorednames the value.-0.5or1.5: the reward is the value as written, since a verify command is the task author's.final_answercalls; the second submission still scores0.5.run_shellreturnssuccess=Falseforexit 3and a failingtest -e, andTruefortrue.I also ran the exact clear and read commands through the real
terminus,jupyterandcoding_toolssandbox 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 makesclearedfalse, and a value written by a verify command reads back as0.8.On main, the same tests fail with the agent-written reward coming through as
1.0in all five envs,nanandinfcoming through as the reward, and jupyter reportingexit 3as success.Full suite with CI's dependency set and
GITHUB_ACTIONS=true: 3217 passed, 103 skipped, 0 failed. Lint:ruff format --checkandruff checkonsrc/ tests/are clean, andscripts/sync_env_docs.py --checkpasses.usort check src/ tests/flags onlytests/envs/test_julia_env.pyandtests/envs/test_grid_world.py, which this PR doesn't touch and which it flags on main too. The five env modules carryruff formatdrift 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
jupyter_envthe agent shares the Python kernel thatrun_shellexecutes in: a code cell that replacessubprocess.runmakes 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'scommands.runinstead of the kernel, asopencode_env's backend already does (envs/opencode_env/sandbox/e2b.py:93). Happy to do that as a follow-up.terminus_envletsfinal_answerbe called more than once, andcoding_tools_envscores an empty verify list as0.0where the other envs returnNone. Both change reward values.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