Repository navigation
fix(e2b): real command exit codes, pod-IP wait after resume, warm-pool status - #653
Merged
Merged
Conversation
…l status
Three defects found by running the official E2B SDK examples against a live
cluster (tests/e2b_official_aligned.py, aligned with docs.e2b.dev/client).
1. sandboxd: commands.run always reported exit_code -1 (streaming) or 0
(buffered) even though stdout/stderr were correct.
Root cause: main.setupSignals installed SIGCHLD as SIG_IGN plus
SA_NOCLDWAIT. Both make the kernel auto-reap children, so wait4 on any
child returns -ECHILD and leaves the status word untouched — every exec's
exit code silently collapsed to its fallback.
Fix: install a real (non-SIG_IGN) SIGCHLD handler, which keeps children
waitable. exec.zig, pty.zig and venv.zig already wait, so they now recover
the true status. background.zig relied on auto-reap for its children, so it
now drains them with a detached reaper thread; its completion signal is
unchanged (the BG_WRAPPER EXIT trap still writes the exit_code file).
The wait sites also distinguish a real exit from a signal death, reporting
128+signo the way a shell does instead of the bare -1 that hid "killed".
Verified on Linux with PID-1 semantics (sandboxd run as PID 1 via
`unshare -r --pid --fork --mount-proc`):
/exec exit 42 old: exit_code:0 new: exit_code:42
/exec/stream exit 7 old: {"exit":-1} new: {"exit":7}
/exec/stream python 40+2 new: stdout 42, {"exit":0}
background exit 5 old: 5 new: 5 (unchanged)
zombies old: 0 new: 0 (reaper holds parity)
2. e2b: the first call after a resume failed with "session has no pod IP"
(E2B watch_dir, files.stat, and any envd traffic on a paused sandbox).
wakeForTraffic auto-resumed but returned the pre-resume session view, whose
PodIp is empty because the old pod is gone and the new one has not been
assigned an address yet. Callers address sandboxd by pod IP, so the request
failed immediately.
Fix: poll for the new pod IP (bounded, 30s) and return the fresh view. The
same wait covers the explicit-resume case, where the phase is already
running while the pod is still being created.
3. sandbox-matrix: WarmPool .status was never written.
The CRD has carried readyCount/pendingCount since KIP-30 but
reconcileSinglePool never populated them, so `kubectl get
sandboxwarmpools` reported an empty READY column and operators had no
in-cluster signal for whether the pool was serving.
Fix: write readyCount (running + Ready condition) and pendingCount (the gap
the pass is closing) each pass.
All new tests are regression tests: each fails without its fix and passes with
it. No change to any pre-existing failure.
Open
Contributor
…us change
Code-quality follow-up on the e2b PR, flagged by DeepSource (Go) and
SonarCloud's quality gate. No behaviour change beyond removing one invented
field and one latent panic.
- Drop the duplicate PodReady helper. pkg/sandboxmatrix/grpc already exposes
sandboxgrpc.PodReadyCondition with identical semantics, so the local copy
was pure duplication (SonarCloud's duplication metric).
- Stop writing status["size"]. SandboxWarmPoolStatus declares only readyCount
and pendingCount, so writing an undeclared field produced data the CRD
schema drops on read, duplicating spec.size for nothing.
- Remove a latent panic. The status write indexed
pool.Object["spec"].(map[string]interface{}) with an unchecked assertion,
which panics on any pool whose spec is absent or not an object. Read it
through the two-value form like the field above it.
- Rename the loop variable to index the slice rather than copying each pod,
since PodReadyCondition takes a pointer.
- gofmt / zig fmt the touched files.
Pre-existing findings are left alone: pkg/sandboxmatrix/api/v1alpha1/types.go
and five sandboxd Zig files are already unformatted on main, and the 12
sandboxd macOS test failures are unrelated.
SonarCloud S1117 on gateway_podip_test.go lines 120 and 155: both declared a variable solely to compare it against the empty string in the same condition. The condition is the only use, so the temporary is pure noise — use the expression directly. The neighbouring `got` is kept: it is reported in the failure message.
SonarCloud's Quality Gate failed on 5.6% duplicated lines in new code, all of it in controller_test.go: the two WarmPool status tests repeated the same arrange/act/observe block — create the pool CR, seed the pods, reconcile, read the status back — twice, which buried the one assertion each test actually makes. Extract warmPoolStatusAfterReconcile so the sequence exists once and each test states only its own expectation. Same coverage, same A/B: both tests still fail without the status write and pass with it.
|
This was referenced Oct 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three defects found by running the official E2B SDK examples (docs.e2b.dev/client) against a live cluster. The reproducer is
tests/e2b_official_aligned.py.1.
commands.runalways reportedexit_code=-1(or0)Stdout/stderr were correct, exit codes were not — the blocker for real agent workflows.
Root cause:
sandboxd/src/main.zig:setupSignals()installedSIGCHLDasSIG_IGN+SA_NOCLDWAIT. Both make the kernel auto-reap children, sowait4on any child returns-ECHILDwith the status word untouched, and every exec silently fell back to its hardcoded value (-1streaming,0buffered — socmd; echo $?lied to every caller).Fix: install a real (non-
SIG_IGN) handler, which keeps children waitable.exec.zig/pty.zig/venv.zigalready wait, so they now recover the true status.background.zigrelied on auto-reap for its children, so it drains them with a detached reaper (its completion signal is unchanged — theBG_WRAPPEREXIT trap still writes the exit_code file). Signal deaths now report128+signolike a shell instead of a bare-1.Verified on Linux with PID-1 semantics (
unshare -r --pid --fork --mount-proc):/execexit 42exit_code:0exit_code:42/exec/streamexit 7{"exit":-1}{"exit":7}/exec/streampython3 40+2{"exit":-1}42,{"exit":0}exit 555(unchanged)2. First call after a resume: "session has no pod IP"
wakeForTrafficauto-resumed a paused sandbox but returned the pre-resume session view, whosePodIpis empty — the old pod is gone and the new one has no address yet. Callers address sandboxd by pod IP, so E2Bwatch_dirand every other envd call failed. Fix: poll for the new pod IP (bounded, 30s) and return the fresh view. The same wait covers explicitresume(), where the phase is already running while the pod is still being created.3.
WarmPool .statuswas never writtenThe CRD has carried
readyCount/pendingCountsince KIP-30 butreconcileSinglePoolnever populated them, sokubectl get sandboxwarmpoolsshowed an empty READY column. Now written every pass.Tests
Every new test is a regression test — each fails without its fix and passes with it:
pkg/sandbox/e2b/gateway_podip_test.go—TestWakeForTrafficWaitsForPodIPAfterResume,TestWakeForTrafficRunningWithoutPodIPWaitspkg/sandboxmatrix/controller_test.go—TestReconcileSinglePool_PopulatesStatus,TestReconcileSinglePool_ReportsPendingWhenFillingNo change to any pre-existing failure (the 12
sandboxdtest failures on macOS — PTY tests plus a 1 MiBreadAllFromFdTruncoff-by-one — are identical before and after; they are Linux-side and tracked separately).