Skip to content

fix(sandbox): post-merge review fixes for the sandbox wave - #4642

Merged
yannickmonney merged 7 commits into
mainfrom
fix/sandbox-wave4-review
Oct 9, 2026
Merged

yannickmonney merged 7 commits into
mainfrom
fix/sandbox-wave4-review

Conversation

@yannickmonney

Copy link
Copy Markdown
Contributor

What

These fixes come from an adversarial post-merge review of the sandbox wave (#4626–#4634), plus a full local run of main's checks. I checked each finding against the code and confirmed it before fixing. Every new test fails on the old code and passes with the fix.

Checks

  • Spawner and runtime lint (type-aware) and type-checks are clean.
  • The session-routes suites (CPU pressure, first-come line, memory, disk, egress, fake runnerd), devices/hub, the Docker backend, buildkit-resources, the egress firewall, the staged manifest and the egress recovery tests pass, except listWorkspaces, which fails the same way on main on macOS.
  • The platform integration-scope guard passes.

The CPU gate let a start in only with no waiter ahead in the line, of any
kind. A create waiting for a slot that will not free (a full host whose
sessions are busy or pinned) then held back every warm activation under
CPU pressure, though an activation needs no slot and was admitted before
the gate existed. A waiter now records whether its last refusal was for
the CPU, and only such waiters hold the CPU turn.

An attach whose stream stops short of the exec's end now runs the same
eviction check as the first exec window and, when Docker recorded the
OOM killer in the dead container, ends the exec as SESSION_OOM: most
long turns follow their exec over attach, where a container's OOM death
read as a lost session and was retried at once.
A device now reports a create it is admitting as starting within a
second, while the hub still counted the same create as in flight until
the device answered: the slot counted twice, and a device with room was
passed over for the server, where a session stays for its life. The hub
keeps in-flight creates by id and counts only those the device does not
report yet; memory headroom still counts them all, since a starting
session's memory is not in the device's reading.
…starts

The create read the proxy address at its start, seconds before docker
run, and could join an older read: a deploy that recreated sandbox-egress
meanwhile left a healthy session labelled with the old address, and the
sweep recycled it at its first idle release. The create now reads afresh
right before its run arguments. A caller whose budget is already spent
starts no read, which would otherwise end unobserved.
A cap past 4294967295 passed the entrypoint's check, iptables refused
it, and the proxy started uncapped with a warning that blamed the kernel.
Such a value now refuses the start like any malformed one.
The decoder rejected the whole manifest for one entry out of shape: a
source id longer than its 1024 limit (the API allows 2048) or a stat with
a pre-1970 time, which the writer persisted. Every restart then fetched
and hashed every staged input again. The limit now matches the API, a
malformed entry is dropped alone (the signature already vouches for the
file), and a file dated before 1970 is hashed instead of recorded by a
stat the manifest cannot keep.
…solver

The nat-restore test read the host's /etc/resolv.conf: it passed on a
runner but failed inside a container on a Docker user network, and the
DNS DNAT half of the batch went untested. The entrypoint reads the
resolver from _RESOLV_CONF, and the test covers both resolvers.
file-ops.ts, which the integration suite reads, now imports
staged-manifest.ts, and the integration-scope guard failed on main until
the integration filter names it.
@yannickmonney
yannickmonney merged commit b3976a0 into main Oct 9, 2026
22 of 25 checks passed
@yannickmonney
yannickmonney deleted the fix/sandbox-wave4-review branch October 9, 2026 10:12
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