Skip to content

Run jupyter_env verification outside the notebook kernel - #1233

Open
surajsharan wants to merge 2 commits into
huggingface:mainfrom
surajsharan:fix/jupyter-verify-outside-kernel
Open

surajsharan wants to merge 2 commits into
huggingface:mainfrom
surajsharan:fix/jupyter-verify-outside-kernel

Conversation

@surajsharan

@surajsharan surajsharan commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Summary

E2BSandbox.run_shell executes a command by sending a subprocess.run cell to the same persistent kernel the agent writes to with add_and_execute_code_cell. Verification went through it, so a single cell that rebinds subprocess.run decided every verify result and the reward-file read (#1210). Ordinary notebook state leaked in the same way: a cell that changes the working directory or edits os.environ changed what verify commands ran against, which can affect honest agents too.

  • E2BSandbox.run_command runs a command as its own process through E2B's process API (Sandbox.commands.run, the same API opencode_env's backend uses) and reports the real exit status. A non-zero exit raises CommandExitException, which it maps to success=False with the exit code and output.
  • Only a command that ran and exited becomes a result. A command that cannot be started, a lost connection, an expired sandbox or an auth failure is raised instead of being scored: none of them say anything about the agent's work, and on main they propagate out of run_code the same way. Verification then stops with no verify results, no reward and an unfinished episode.
  • It starts the command with background=True and waits on the handle. A wait that ends without an exit status is raised too, after asking E2B to kill the command. The kill signals the command's own process and not the children its shell started (SendSignal in envd, and exec.CommandContext does the same at the deadline; Executing AsyncCommandHandle.kill() will cause blocking and cannot kill the process. e2b-dev/E2B#1034 reports kills that do not stop the process), so the command cannot be shown to be finished and the reward file is not read as if it were. The kill is best effort: its result is not used, and a kill that fails does not replace the original error.
  • It runs as root in /home/user with the existing 120 s timeout. E2B's default code-interpreter-v1 template runs the kernel as root with the notebook in /home/user, so verify commands keep the permissions and working directory they have today.
  • The verify commands and the reward-file read use it. The agent's own execute_shell_command and the setup commands, which run before the agent acts, keep using run_shell, so nothing the agent sees changes.

This removes the notebook-kernel coupling and hardens verification. It is not an isolation boundary: the agent's cells run as root (the template starts Jupyter with sudo systemctl start jupyter, the unit sets no User=, and the server runs with allow_root = True), commands.run starts /bin/bash -l -c and so sources login files a root agent can rewrite, and the verify commands read files the agent controls. Verifying outside the agent's sandbox, with the invariant that agent-controlled state cannot alter verifier startup, execution, or result collection, is tracked in #1232.

It also fixes scoring for honest agents. run_shell ignores the command's exit code on main (the bug #1200's second commit fixes), so a verify command that genuinely fails counts as passed: a submission whose pytest run fails scores 1.0. Verification now reads real exit codes whether or not #1200 lands.

The reproduction from #1210 by @k21993 is the regression test, and they are co-author on the commit.

Relation to #1200: both change _run_verify_commands and _read_reward_override in jupyter_environment.py. This applies to main on its own; whichever lands second I'll rebase, and after both, #1200's clear-before-verify check goes through run_command too. #1200's run_shell exit-code fix still matters after this, for execute_shell_command and setup commands, which keep using run_shell.

Closes #1210

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 in tests/envs/test_jupyter_environment.py drive the real E2BSandbox against a kernel that executes each cell in a namespace that persists between cells, as E2B's does, and a process runner that runs each command in a fresh bash process with a real exit status:

  • jupyter_env: verification runs in the agent's own kernel, so a rollout can report any reward #1210's reproduction: a cell rebinds subprocess.run, both verify commands (exit 1) still fail, and the reward is 0.0. No verify command reaches the kernel.
  • Reward-file read: with the same cell rebinding subprocess.run to report "1.0", a verify command that writes 0.8 yields a reward of 0.8.
  • Notebook directory: a cell calls os.chdir, and pwd in verification still reports /home/user. Every call goes out in the background, as root, in /home/user, with a 120 s timeout.
  • run_command maps a non-zero exit to success=False with the exit code, stdout and stderr, and does not kill the handle, since the command has exited. This is checked against a stand-in exception and, when the SDK is installed, against e2b.CommandExitException itself (pinned e2b 2.20.2 / e2b-code-interpreter 2.6.1).
  • A wait that ends without an exit status raises, and the command is killed first; the kill reporting False, or raising, does not change that or mask the original error. A command that cannot be started raises.
  • A sandbox that fails during verification is not scored: final_answer comes back as a tool error, with no verify results, no reward and done still false.
  • The existing verify test now also asserts the commands went through run_command, not run_shell.

On main the same tests fail the way #1210 describes: both exit 1 commands pass, the reward read returns the rebound 1.0, and verification runs in the directory the cell set.

pytest tests/envs/test_jupyter_environment.py → 16 passed with the SDK installed. Full suite with CI's dependency set and GITHUB_ACTIONS=true: 2979 passed, 103 skipped, 0 failed. On Python 3.12 CI's --all-extras install brings in e2b 2.50.0 through harbor[e2b], so the SDK-class test runs there against it too; on 3.11 it skips. ruff format --check and ruff check on src/ and tests/ (CI's scope, ruff 0.16.8 as locked) and scripts/sync_env_docs.py --check pass, and the changed hunks in envs/ are ruff format-clean. usort check src/ tests/ (1.1.3, as locked) flags only tests/envs/test_julia_env.py and tests/envs/test_grid_world.py, which this PR doesn't touch; it flags the same two on main.

End to end, on a local E2B stand-in

I don't have an E2B account, so I ran the environment end to end on a stand-in for e2b_code_interpreter.Sandbox that runs things where E2B's default template does: a real IPython kernel started as root with the notebook in /home/user, and commands.run as /bin/bash -l -c processes that raise the SDK's own CommandExitException and TimeoutException (pinned e2b 2.20.2) and, with background=True, return a handle with wait() and kill(), inside a Debian container with a user account. Each scenario went through reset, the agent tools and final_answer, and the first three also through the server and JupyterEnv over the WebSocket:

scenario main this branch
#1210: a cell rebinds subprocess.run, both checks exit 1 1.0, 2/2 pass 0.0, 0/2
a cell os.chdir('/tmp') and breaks PATH; verify runs pwd runs in /tmp runs in /home/user as root
honest task, one of three checks fails 1.0, 3/3 0.667, 2/3
buggy solution, pytest fails 1.0 0.0
verify writes 0.8 while a cell forges "1.0" 1.0 0.8
agent's execute_shell_command unchanged unchanged
a root cell writes exit 0 into /root/.bash_profile 1.0 1.0, the limit stated above
the sandbox becomes unreachable during verification tool error, no reward tool error, no reward

run_command itself reported exit code 7 with stdout and stderr kept for a non-zero exit, raised TimeoutException for a command that outlived its timeout, and ran as root in /home/user. A command still running at its timeout (sleep 3; echo late > /home/user/late.txt with timeout_s=1) was killed, and the late write never happened.

Not run against a live E2B sandbox. With an E2B_API_KEY, this checks the change and the user and directory assumption in one go:

from jupyter_env.server.jupyter_environment import JupyterEnvironment
from openenv.core.env_server.mcp_types import CallToolAction

env = JupyterEnvironment()
env.reset(verify=["exit 1", 'test "$(id -u)" -eq 0 && test "$(pwd)" = /home/user'])
env.step(CallToolAction(
    tool_name="add_and_execute_code_cell",
    arguments={"code": "import subprocess\nclass F: returncode = 0; stdout = '1.0'; stderr = ''\nsubprocess.run = lambda *a, **k: F()"},
))
env.step(CallToolAction(tool_name="final_answer", arguments={"answer": "done"}))
print(env.state.last_reward, [r.success for r in env.state.verify_results])
# this branch: 0.5 [False, True]    main: 1.0 [True, True]

Out of scope

Claude Code Review

N/A

E2BSandbox.run_shell executes commands by sending a subprocess.run cell
to the same kernel the agent writes to. Verification went through it, so
a cell that rebinds subprocess.run decided every verify result and the
reward-file read, and ordinary notebook state (a changed working
directory, edited os.environ) leaked into what verification saw.

Add E2BSandbox.run_command, which runs a command as its own process
through E2B's process API, as root in /home/user (the user and directory
verify commands run with today), and reports the real exit status. It
waits on a background handle and, if the wait fails without an exit
status, kills the command, as run_shell's subprocess timeout does; envd
keeps a command running when the request that started it is cancelled,
for example on a dropped connection. The verify commands and the
reward-file read use it; the agent's execute_shell_command keeps using
run_shell. Since run_shell ignores exit codes, this also stops a
genuinely failing verify command from counting as passed.

This removes the coupling to the kernel. It is not an isolation
boundary: the kernel runs as root, so agent code can still change what
processes in the sandbox see. Verifying outside the agent's sandbox is
tracked in huggingface#1232.

Closes huggingface#1210

Co-authored-by: Karthik Suresh <7954591+k21993@users.noreply.github.com>

@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.

APPROVE at 29bf23b1

Closes the #1210 jupyter kernel-coupling hole: verify commands and the reward.txt read now go through E2B's process API (run_command) as root in /home/user, not run_shell inside the persistent notebook kernel. Agent subprocess.run rebinds, chdir, and os.environ edits no longer decide verify outcomes.

Implementation looks right:

  • Background start + wait() with kill-on-timeout/connection-loss (avoids a late writer after the reward read).
  • Non-zero exit matched by exit_code attribute (works with or without the e2b SDK installed at import time).
  • Setup / agent shell tool correctly stay on run_shell.
  • Docs honestly state this is not a full isolation boundary while the kernel is root — same residual class as #1200 (in-sandbox FS / shell-startup tamper).

Validation: focused suite 14/14 pass locally, including rebound-subprocess, reward-file-outside-kernel, cwd isolation, exit/timeout/start failures, and real e2b.CommandExitException matching.

Gates still open: fork — needs maintainer Approve and run for repository CI. Env/Spaces only — not Thursday 0.6.0 wheel cargo (no src/openenv/**). Complements #1200 rather than replacing it.

View PR

Open in Web View Automation 

Sent by Cursor Automation: Release

@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.

Stale comment

REQUEST_CHANGES at 29bf23b1 — deeper E2B lifecycle review

Withdrawing the earlier approval. Moving verification out of the notebook kernel is the correct #1210 fix, and the E2B API shape is compatible, but two report/reward-path issues remain:

  1. Infrastructure failures are scored as agent failures. E2BSandbox.run_command() catches every start/wait exception and returns CellResult(success=False). _run_verify_commands() then counts authentication failures, sandbox expiry, network loss, and SDK/transport faults as failed verifier checks and emits a reduced terminal reward; _read_reward_override() similarly turns a process-API failure into a silent fallback to pass ratio. Before this PR, a process/provider failure propagated as a tool/environment error rather than grading the agent. Keep command non-zero exits (and a confirmed command deadline) as verifier failures, but propagate provider/transport/start failures, with regressions proving they do not produce a reward.

  2. Kill does not establish terminality. On a wait failure the code ignores handle.kill() == False and suppresses kill exceptions, then continues toward reward collection. More importantly, E2B's current kill is PID-only: hosted E2B issue #1034 reproduces kill() == true while shell child processes remain alive. A surviving descendant can still write after the verifier has supposedly stopped. Do not claim/assume the command is gone merely because kill() returned. At minimum, uncertain cleanup must surface as an environment error and must not continue to reward-file collection; use a termination mechanism with the required scope (or terminate the sandbox) where late writes must be excluded.

The documented root/login-shell limitation and fresh-sandbox boundary in #1232 remain valid out-of-scope residuals. This PR still complements #1200: when rebased together, preserve #1200's clear/range checks but route its verification and reward-file operations through the process API.

Also still required: maintainer Approve-and-run for repository CI and explicit human reward/security-owner review.

View PR

Open in Web View Automation 

Sent by Cursor Automation: Release

run_command turned every failure into a failed CellResult, so an
unreachable or expired sandbox, a lost connection, an auth failure or a
command that never started counted as a failed verify check and lowered
the reward. Before verification moved out of the kernel those errors
came out of run_code and propagated, and none of them say anything
about the agent's work.

Report only a command that ran and exited: a non-zero exit is still a
failed check, everything else is raised. A wait that ends without an
exit status is raised too, after asking E2B to kill the command. That
kill signals the command's own process and not the children it started,
so nothing here can call the command finished, and the reward file must
not be read as if it were. The kill is best effort: its result is not
used, and a kill that fails does not replace the original error.

Verification then stops with no verify results, no reward and an
unfinished episode, which is what happens on main today when the
sandbox fails during verification.
@surajsharan

Copy link
Copy Markdown
Contributor Author

Both points are fair. Pushed a second commit that changes what run_command treats as a result.

1. Infrastructure failures are no longer scored. run_command now reports only a command that ran and exited. A non-zero exit is still a failed verify check; a command that cannot be started, a lost connection, an expired sandbox or an auth failure is raised instead. _run_verify_commands and _read_reward_override then stop: the tool call comes back as an error, verify_results stays empty, last_reward stays None and the episode does not finish. That is what main does today, where the same failures come out of run_code. I checked both on a local E2B stand-in, with the sandbox made unreachable after the agent's work:

main:   error="Error calling tool 'final_answer': sandbox unreachable" reward=None verify_results=[] done=False
branch: error="Error calling tool 'final_answer': sandbox unreachable" reward=None verify_results=[] done=False

2. A deadline is no longer treated as a verdict either. You are right that the kill does not establish terminality, and I had the same finding from the other side: in envd, SendSignal signals the command's own pid, so a child the command's shell started survives it, and e2b-dev/E2B#1034 reports cases where the kill does not stop the process at all. Since the command cannot be shown to be gone, a wait that ends without an exit status is raised rather than counted as a failed check, and the reward file is not read afterwards. The kill is still attempted, to narrow the window, but nothing depends on it now: the return value is not used, and a kill that raises does not mask the original failure. Tests cover a kill that returns False and a kill that raises.

For the record on the timeout case, this is stricter than main: there subprocess.run(timeout=...) inside the kernel turned a hanging verify command into a failed check. Grading an agent on a command we cannot account for seemed worse than surfacing it, but if you would rather keep a confirmed deadline as a verifier failure, say so and I will split it that way.

Unchanged and still out of scope: the root kernel and login-shell limitation, and the fresh-sandbox boundary in #1232.

Tests: 16 pass in tests/envs/test_jupyter_environment.py, including the two new kill cases, a start failure that propagates, and an end-to-end case where a sandbox failure during verification produces an error and no reward. Full suite with CI's dependency set and GITHUB_ACTIONS=true: 2979 passed, 103 skipped, 0 failed. Lint and scripts/sync_env_docs.py --check pass.

Repository CI still needs maintainer Approve-and-run on this fork PR.

@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.

APPROVE at f29093d6

Both REQUEST_CHANGES items are fixed correctly:

  1. Infrastructure failures propagaterun_command only returns a CellResult for a command that ran and exited (incl. non-zero via CommandExitException). Start/wait failures without exit_code raise; _run_verify_commands / _read_reward_override therefore stop before scoring. Docs match.
  2. Uncertain deadline is not a verdict — timeout/connection-loss path kills best-effort (suppress), then raises; kill False/kill errors do not mask or invent a failed check. Regressions cover both kill outcomes, start failure, and end-to-end “sandbox unreachable during verify → error, no reward, not done”.

Accept the stricter-than-main timeout behavior (surface rather than grade an unaccounted command).

Residuals unchanged / out of scope: root kernel + login-shell trust boundary; #1232 fresh-sandbox; #1200 rebase interaction if that lands later.

Not 0.6.0 wheel cargo (env/Spaces). Fork CI still needs maintainer Approve-and-run.

View PR

Open in Web View Automation 

Sent by Cursor Automation: Release

This branch has not been deployed

No deployments
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.

jupyter_env: verification runs in the agent's own kernel, so a rollout can report any reward

1 participant