Run jupyter_env verification outside the notebook kernel - #1233
surajsharan wants to merge 2 commits into
Conversation
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>
There was a problem hiding this comment.
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_codeattribute (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.
Sent by Cursor Automation: Release
There was a problem hiding this comment.
Stale comment
REQUEST_CHANGES at
29bf23b1— deeper E2B lifecycle reviewWithdrawing 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:
Infrastructure failures are scored as agent failures.
E2BSandbox.run_command()catches every start/wait exception and returnsCellResult(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.Kill does not establish terminality. On a wait failure the code ignores
handle.kill() == Falseand suppresses kill exceptions, then continues toward reward collection. More importantly, E2B's current kill is PID-only: hosted E2B issue #1034 reproduceskill() == truewhile 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 becausekill()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.
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.
|
Both points are fair. Pushed a second commit that changes what 1. Infrastructure failures are no longer scored. 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, For the record on the timeout case, this is stricter than Unchanged and still out of scope: the root kernel and login-shell limitation, and the fresh-sandbox boundary in #1232. Tests: 16 pass in Repository CI still needs maintainer Approve-and-run on this fork PR. |
There was a problem hiding this comment.
APPROVE at f29093d6
Both REQUEST_CHANGES items are fixed correctly:
- Infrastructure failures propagate —
run_commandonly returns aCellResultfor a command that ran and exited (incl. non-zero viaCommandExitException). Start/wait failures withoutexit_coderaise;_run_verify_commands/_read_reward_overridetherefore stop before scoring. Docs match. - Uncertain deadline is not a verdict — timeout/connection-loss path kills best-effort (
suppress), then raises; killFalse/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.
Sent by Cursor Automation: Release


Summary
E2BSandbox.run_shellexecutes a command by sending asubprocess.runcell to the same persistent kernel the agent writes to withadd_and_execute_code_cell. Verification went through it, so a single cell that rebindssubprocess.rundecided 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 editsos.environchanged what verify commands ran against, which can affect honest agents too.E2BSandbox.run_commandruns a command as its own process through E2B's process API (Sandbox.commands.run, the same APIopencode_env's backend uses) and reports the real exit status. A non-zero exit raisesCommandExitException, which it maps tosuccess=Falsewith the exit code and output.mainthey propagate out ofrun_codethe same way. Verification then stops with no verify results, no reward and an unfinished episode.background=Trueand 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 (SendSignalin envd, andexec.CommandContextdoes 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.rootin/home/userwith the existing 120 s timeout. E2B's defaultcode-interpreter-v1template runs the kernel as root with the notebook in/home/user, so verify commands keep the permissions and working directory they have today.execute_shell_commandand the setup commands, which run before the agent acts, keep usingrun_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 noUser=, and the server runs withallow_root = True),commands.runstarts/bin/bash -l -cand 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_shellignores the command's exit code onmain(the bug #1200's second commit fixes), so a verify command that genuinely fails counts as passed: a submission whosepytestrun 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_commandsand_read_reward_overrideinjupyter_environment.py. This applies tomainon its own; whichever lands second I'll rebase, and after both, #1200's clear-before-verify check goes throughrun_commandtoo. #1200'srun_shellexit-code fix still matters after this, forexecute_shell_commandand setup commands, which keep usingrun_shell.Closes #1210
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 in
tests/envs/test_jupyter_environment.pydrive the realE2BSandboxagainst 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:subprocess.run, both verify commands (exit 1) still fail, and the reward is0.0. No verify command reaches the kernel.subprocess.runto report"1.0", a verify command that writes0.8yields a reward of0.8.os.chdir, andpwdin verification still reports/home/user. Every call goes out in the background, asroot, in/home/user, with a 120 s timeout.run_commandmaps a non-zero exit tosuccess=Falsewith 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, againste2b.CommandExitExceptionitself (pinnede2b2.20.2 /e2b-code-interpreter2.6.1).False, or raising, does not change that or mask the original error. A command that cannot be started raises.final_answercomes back as a tool error, with no verify results, no reward anddonestill false.run_command, notrun_shell.On
mainthe same tests fail the way #1210 describes: bothexit 1commands pass, the reward read returns the rebound1.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 andGITHUB_ACTIONS=true: 2979 passed, 103 skipped, 0 failed. On Python 3.12 CI's--all-extrasinstall brings ine2b2.50.0 throughharbor[e2b], so the SDK-class test runs there against it too; on 3.11 it skips.ruff format --checkandruff checkonsrc/andtests/(CI's scope, ruff 0.16.8 as locked) andscripts/sync_env_docs.py --checkpass, and the changed hunks inenvs/areruff format-clean.usort check src/ tests/(1.1.3, as locked) flags onlytests/envs/test_julia_env.pyandtests/envs/test_grid_world.py, which this PR doesn't touch; it flags the same two onmain.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.Sandboxthat runs things where E2B's default template does: a real IPython kernel started as root with the notebook in/home/user, andcommands.runas/bin/bash -l -cprocesses that raise the SDK's ownCommandExitExceptionandTimeoutException(pinnede2b2.20.2) and, withbackground=True, return a handle withwait()andkill(), inside a Debian container with auseraccount. Each scenario went throughreset, the agent tools andfinal_answer, and the first three also through the server andJupyterEnvover the WebSocket:mainsubprocess.run, both checksexit 1os.chdir('/tmp')and breaksPATH; verify runspwd/tmp/home/useras rootpytestfails0.8while a cell forges"1.0"execute_shell_commandexit 0into/root/.bash_profilerun_commanditself reportedexit code 7with stdout and stderr kept for a non-zero exit, raisedTimeoutExceptionfor a command that outlived its timeout, and ran asrootin/home/user. A command still running at its timeout (sleep 3; echo late > /home/user/late.txtwithtimeout_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:Out of scope
jupyter_env, where the agent writes kernel Python directly. The other sandbox environments' agents reach the sandbox through shell commands and file tools, but they can still change files and processes the verifier depends on, so they are listed in Verify sandbox environments outside the agent's sandbox #1232 too.Claude Code Review
N/A