Skip to content

fix(ts): surface incomplete sandbox run instead of silent success#718

Open
sgirones wants to merge 1 commit into
mainfrom
salvador/fix-sandbox-run-silent-failure
Open

fix(ts): surface incomplete sandbox run instead of silent success#718
sgirones wants to merge 1 commit into
mainfrom
salvador/fix-sandbox-run-silent-failure

Conversation

@sgirones

@sgirones sgirones commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Problem

Sandbox.run() (the method behind runCommand) streams POST /api/v1/processes/run and only sets exitCode when it observes an exit_code/signal event. When the SSE stream ends without a terminal event, it fell through to:

return { exitCode: -1, stdout: "", stderr: "" };

…and never threw. Callers got a successful CommandResult with empty output for a command that never actually ran.

How it shows up

Running a sandbox benchmark against a dev endpoint, every exec returned "success" with empty stdout after a fixed ~48s. Tracing it end-to-end:

  • The in-VM daemon executes node -v in ~11ms (direct and via the dataplane's own http_proxy).
  • During the 48s, the executor dataplane saw zero connections and no process was ever spawned (Failed to send signal: No processes spawned).
  • The exec request goes to the sandbox-proxy host (sandbox.<env>), which couldn't route to the executor; it closed the stream after a timeout without any exit_code.
  • The SDK returned { exitCode: -1, stdout: "" } → the harness counted 100/100 "ok" for commands that never executed.

The underlying routing problem is dev infrastructure (separate issue), but the SDK masking it as success is a real correctness bug.

Fix

Track whether a terminal (exit_code/signal) event arrived; if the stream ends without one, throw SandboxConnectionError instead of returning a misleading success.

This matches the canonical behavior already in the repo's Rust CLI (crates/cli/src/commands/sbx/exec.rs), which treats a missing exit event as failure (exit_code.unwrap_or(1) → non-zero → error).

Tests

  • New unit test in tests/sandbox.test.ts reproducing the unreachable case (run stream with no exit_code) → asserts run() rejects.
  • tsc --noEmit clean; full suite green (189 tests, unit + integration).

Out of scope

  • The dev sandbox-proxy ↔ executor routing/latency (infra).
  • Signal-exit code convention: TS uses -signal while the Rust CLI uses 128 + signal; worth aligning separately.

🤖 Generated with Claude Code

`Sandbox.run()` streams `POST /api/v1/processes/run` and only sets
`exitCode` when it sees an `exit_code`/`signal` event. If the stream ends
without one — e.g. the sandbox is unreachable and the proxy closes the
stream after a timeout — it returned `{ exitCode: -1, stdout: "", stderr: "" }`
and never threw. Callers saw a fake success for a command that never ran.

Track whether a terminal event arrived and throw `SandboxConnectionError`
when it didn't. This matches the canonical behavior in the Rust CLI
(`crates/cli/src/commands/sbx/exec.rs`), which treats a missing exit event
as failure (`exit_code.unwrap_or(1)`).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

1 participant