Skip to content

feat(sandbox): detached PTY exec — run long commands without holding a connection - #2558

Merged
bxyu-nvidia merged 3 commits into
mainfrom
hemil/pty-detached-exec
Aug 26, 2026
Merged

feat(sandbox): detached PTY exec — run long commands without holding a connection#2558
bxyu-nvidia merged 3 commits into
mainfrom
hemil/pty-detached-exec

Conversation

@hemildesai

Copy link
Copy Markdown
Contributor

What

pty.exec(detach=True) runs a command without holding a connection while it works: the command starts in a PTY session, the WebSocket is dropped, and the session is briefly re-attached every poll_interval_s to check for completion. A long-running command occupies a connection for milliseconds per poll instead of its whole runtime.

Why

At eval scale, session-mode PTY holds one WebSocket per rollout for the rollout's lifetime — thousands of standing connections held for hours. Detached exec replaces that with a handful of short-lived polls (~200 ms each), a ~100x reduction in standing connections, and structurally avoids the failure modes long-lived sockets are exposed to (load-balancer idle timeouts on quiet sessions, server deploys interrupting streams).

How

  • OpenSandboxPtySession.detach() / reattach(): built on the existing resume machinery (takeover=1, since=<bytes received>). execd sessions run fine with no client attached — the socket is a view, not the session's lifeline. A detached session is not closed, so provider pruning leaves it alone; close() still releases and ends it.
  • pty.exec(..., detach=True, poll_interval_s=...): same marker discipline as session-mode exec, plus file capture inside the sandbox (>cap.out 2>cap.err) because the server retains only ~1 MiB of terminal output across a detach. Output is cat-collected on completion, so stdout/stderr come back separated in both pty and pipe modes. A fast command that finishes within the first quiet window never detaches at all.
  • Without session a private session is opened (never registered as the default-shell session) and closed afterwards. An explicitly passed session is detached while the command works and comes back attached and reusable.

Testing

  • Unit: wire-level detach/reattach (no DELETE on detach, since/takeover on re-dial, prune safety, close-after-detach) and facade-level detached exec (poll cycle, fast path, private-session lifecycle, timeout parity with exec()). 107 tests passing in test_opensandbox_pty.py + test_sandbox.py.
  • E2E SWE-bench eval with the agent command running detached, with connection-count telemetry: in progress, will post results here.

🤖 Generated with Claude Code

…a connection

pty.exec(detach=True) starts the command in a session, drops the
WebSocket, and briefly re-attaches every poll_interval_s to check for
completion, so a long-running command occupies a connection for
milliseconds per poll instead of its whole runtime. At eval scale this
replaces thousands of hours-held sockets with a handful of short-lived
polls, and structurally avoids the failure modes long-lived connections
are exposed to (load-balancer idle timeouts, server deploys).

Output is captured to files inside the sandbox — the server retains
only ~1 MiB of terminal output across a detach — and collected when the
command finishes, so stdout and stderr come back separated in both pty
and pipe modes. Without a session a private one is opened and closed;
an explicitly passed session is detached while the command works and is
attached and reusable again on return.

Mechanically: OpenSandboxPtySession gains detach()/reattach() built on
the existing resume machinery (takeover + since=<bytes received>); a
detached session is not treated as closed, so provider pruning leaves
it alone, and close() still releases and ends it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Hemil Desai <hemild@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Aug 14, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@hemildesai

Copy link
Copy Markdown
Contributor Author

E2E validation: 500-rollout SWE-bench Verified (Qwen3.6-27B), agent command detached

Quality — parity with attached mode. Pass rate 71.8% (329/458; run stopped at the ≥450 sufficiency marker), vs 69.2–70.2% for the attached-PTY baselines and 67.7–70.0% for the exec-path baselines on the same setup. Zero SandboxPtyError, zero PTY-related incidents over the run; one transient ServerDisconnectedError on a single request (unrelated to the detach path).

Connections — ~10× fewer held. NLB ActiveFlowCount during steady collection (all 500 rollouts running their opencode command detached, 15s polls):

active flows (fleet-wide)
attached-PTY baseline run ~1,750–2,300 sustained
this run ~520–590 total, of which ~300–500 is unrelated background traffic

Net of background: roughly ~170 vs ~1,600 run-attributable standing connections. The create/install burst and the verify-phase grading burst show as short transients; steady state holds.

Integration in the eval was 3 lines: the agent's long opencode command runs session=pty_session, detach=True, poll_interval_s=15; the short execs stay attached and reuse the same session afterwards (the session comes back attached).

🤖 Generated with Claude Code

@hemildesai
hemildesai marked this pull request as ready for review August 14, 2026 16:49
@hemildesai

Copy link
Copy Markdown
Contributor Author

/ok to test 8697184

@hemildesai

Copy link
Copy Markdown
Contributor Author

/claude review

@claude

claude Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

SHIP WITH CARE

Focused, well-reasoned addition of a detached-exec mode to the sandbox PTY (detach=True): start a command, drop the socket, poll by re-attach so a long command holds a connection for milliseconds per poll. The detach/reattach lifecycle on OpenSandboxPtySession is handled carefully — the pump skips finalization on detach so reads and the exit future survive reattach, close() re-emits EOF for a detached session so readers unblock, closed treats detached-but-alive correctly, and _send refuses I/O while detached. Good test coverage of the session lifecycle and the exec orchestration (poll/collect, fast-path no-detach, private-session cleanup, timeout parity).

One correctness concern worth resolving before this feeds real output back to callers (inline on api.py):

  • RISK — detached output is captured to clean files but collected by cat-ing them through _run_in_pty_session on the same TTY, which prepends the shell's echo of the cat/rm wrapper lines to result.stdout/result.stderr on the default pty=True path. The docstring promises separated output like sandbox.exec(); callers parsing stdout will get an echoed command prefix. The passing test only masks it because the _DetachShellSession fake doesn't model echo for cat (unlike _LiveShellSession). Pipe mode (pty=False) is unaffected.

Minor, author's call: on a timeout mid-poll an explicitly-passed session is left detached rather than reattached, which contradicts the docstring's "attached and reusable again when this returns" — but since the timeout result flags reusable=False and tells the caller to discard, this is consistent enough.

Comment thread nemo_gym/sandbox/api.py Outdated
@hemildesai
hemildesai marked this pull request as draft August 14, 2026 17:05
…o sandbox filesystem writes

Reworked after review. The detached machinery moves out of the public
api module into the session that owns the transport:
OpenSandboxPtySession.run_detached() writes the marker-wrapped command,
drops the socket, and re-attaches every poll to drain output from the
server's retained window. Nothing is written to the sandbox filesystem
(the previous file-capture design could fill a tmpfs-backed /tmp), and
a new replay-gap counter turns window overflow into a loud error
instead of silently truncated output — bulk-output commands belong on
attached exec or the exec API's background mode.

exec(detach=True) is now a thin dispatch: session lifecycle, the same
per-sandbox serialization as attached execs, timeout mapping, and a
reattach so an explicitly passed session always comes back attached —
including on timeout. detach() refuses sessions whose pump already
ended (a dead session must not dodge provider pruning), and reads on a
detached session fail fast instead of blocking. Output is one merged
stream (replay carries no channel split), stderr is None, matching the
PTY-mode exec contract.

Validated live against the deployed endpoint: 40s command over 8s polls
returns complete merged output with the session reusable after, and a
3 MB burst produced while detached raises the designed retained-window
error.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Hemil Desai <hemild@nvidia.com>
@hemildesai

hemildesai commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Redesign in 36536298: replay-window based, no sandbox filesystem writes

A simplicity review (codex, /simplicity-first rubric) plus design discussion flagged two structural problems with the first cut: the detached state machine lived in the public api.py, and file-capture could pressure a tmpfs-backed /tmp on output-heavy commands. Both are gone:

  • The machinery moved into the session that owns the transport: OpenSandboxPtySession.run_detached() (pty.py) writes the marker-wrapped command, drops the socket, and re-attaches every poll_interval_s to drain output from execd's retained replay window. exec(detach=True) in api.py is now a thin dispatch: session lifecycle, the same per-sandbox serialization as attached execs, timeout mapping, reattach-on-return.
  • Nothing is written to the sandbox filesystem. Output rides the server's ~1 MiB retained window between polls; a new replay-gap counter in the pump turns window overflow into a loud SandboxPtyError instead of silently truncated output. Bulk-output commands belong on attached exec or the exec API's background_exec mode — documented.
  • Output is one merged stream with stderr=None (replay carries no channel split), matching the PTY-mode exec contract.

Review findings addressed: internal collection commands no longer exist (no output contamination); an explicitly passed session comes back attached even on timeout; the exec lock now covers the whole detached run exactly like attached session execs; detach() refuses dead-pump sessions (no prune evasion) and reads on a detached session fail fast; no capture files to leak; the single-call-site helpers are gone.

Live validation against the deployed endpoint: a 40s command over 8s polls returns complete merged output (env carried, session reusable after), and a 3 MB burst produced while detached raises the designed retained-window error rather than returning a hole. 111 unit tests passing. Eval-scale re-validation of this design queued next.

🤖 Generated with Claude Code

@hemildesai

Copy link
Copy Markdown
Contributor Author

/ok to test 3653629

@hemildesai

Copy link
Copy Markdown
Contributor Author

E2E re-validation of the replay-window design (36536298): clean

Second 500-rollout SWE-bench Verified run (Qwen3.6-27B), agent command detached with 15s polls, now on the ring-based implementation:

  • Pass rate 68.1% (323/474, stopped at the ≥450 sufficiency marker) — within the run-to-run band of all baselines (attached PTY 69.2–70.2%, exec path 67.7–70.0%, file-based detached 71.8%).
  • Zero retained-window errors across the whole run — the real opencode workload fits comfortably inside the ~1 MiB-between-polls budget, so the loud-overflow path never triggered outside the synthetic probe.
  • Zero PTY errors, zero tracebacks. Throughput ~15–20 rollouts/min, parity with all prior variants.
  • Connection profile unchanged from the first validation: ~620–700 total fleet flows during steady collection (including a few hundred of unrelated background) vs ~1,750–2,300 for attached mode.

No sandbox filesystem writes in this design — output rides the server's retained window between polls, overflow raises instead of truncating.

🤖 Generated with Claude Code

@hemildesai

Copy link
Copy Markdown
Contributor Author

/claude review

await self.write(
f"{{ {command}\n}} </dev/null\nprintf '%s%s:%s\\n' '{token[:5]}' '{token[5:]}' \"$?\"\n".encode()
)
buffer = bytearray()

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.

RISK — stale _replay_gap can spuriously fail a reused session's detached run.

_replay_gap is a monotonic per-session accumulator (pty.py:151) that is never reset. run_detached checks if self._replay_gap: (pty.py:370) to detect output evicted during this command, but the counter also gets bumped by the ordinary connection-loss recovery path: _reattach_socket re-dials with since=self._received, and if the server evicted bytes during the outage, _pump_socket increments _replay_gap.

What breaks: an explicitly-passed live session that survived one socket drop with eviction (exactly the case _pump/_reattach_socket exist to handle in the proxy-shedding environment) carries a nonzero _replay_gap into a later pty.exec(..., detach=True). The very first poll's drain then raises "PTY output exceeded the server's retained window" even though this command lost nothing.

Blast radius: a false-negative hard failure of an otherwise-successful detached command on a reused session — a spuriously failed eval/training step, not silent corruption (it raises).

Fix: reset the window at the start of run_detached so it only measures loss for the current command, e.g. set self._replay_gap = 0 right after writing the launch line (before the poll loop at pty.py:357).

@claude

claude Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

SHIP WITH CARE — internal sandbox PTY transport addition (exec(detach=True)); no verifier/scorer/reward or async-HTTP surface touched, and coverage is solid.

Scope check. Adds a detached exec mode: launch a marker-delimited command, drop the WebSocket, and re-attach every poll_interval_s to drain output and detect completion. Reuses the existing marker discipline (_run_in_pty_session), serializes under the same _session_exec_lock, mirrors exec() timeout semantics, and closes/reattaches correctly on the private-vs-explicit-session split. All awaits present; no httpx, no ray.get().

One RISK, posted inline (pty.py:357): _replay_gap is a monotonic session counter that connection-loss recovery also increments, so a reused session that survived a socket drop with eviction carries stale gap state into the next run_detached, tripping a spurious 'retained window exceeded' failure. It raises rather than corrupts — false-negative failure of a good command. Reset _replay_gap = 0 at the start of run_detached.

Nits I'll leave to your judgment (not blocking): the launch-line write in run_detached isn't wrapped in the timeout the way the poll loop is (a hung initial write would only be bounded by the outer asyncio.timeout(timeout_s) in _exec_detached, which is fine); and the merged-stderr fold at the tail only captures stderr delivered on the final attached read, which the docstring already calls best-effort.

Nothing else material. Good tests — the eviction and stdin-EOF cases are real assertions, not pass-throughs.

@bxyu-nvidia
bxyu-nvidia marked this pull request as ready for review August 26, 2026 03:39
@bxyu-nvidia

Copy link
Copy Markdown
Contributor

/ok to test 53ffec2

@bxyu-nvidia
bxyu-nvidia merged commit e8fa681 into main Aug 26, 2026
35 checks passed
@bxyu-nvidia
bxyu-nvidia deleted the hemil/pty-detached-exec branch August 26, 2026 03:55
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.

2 participants