review: whole-repo round — claims projection off the claim path, single-pass find/list, outbound frame cap, typed SDK errors

Makefile

@@ -10,8 +10,8 @@ SANDBOXD_VERSION ?= $(shell git describe --tags --match 'v*' --always --dirty)
 
 EXTRACT_IMAGE ?= $(BOOT_IMAGE)
 
-# The parent workspace's go.work excludes these modules; GOWORK=off keeps
-# local invocations identical to CI.
+# The parent workspace's go.work includes these modules; GOWORK=off keeps
+# local invocations identical to CI, which has no workspace.
 GO_MODULES := protocol/wire sandboxd sdk/go e2e mcp
 GO_OSES := linux darwin
 

README.md

@@ -37,7 +37,7 @@ performance) — source in
   writable), signed preview URLs, the HTTP-upgrade byte relay to silkd,
   usage + audit journals, /metrics, reap + restart reconcile, memberlist
   mesh with redirect placement
-- `sdk/go/` — Go SDK (stdlib-only): `Connect/New/Lookup`, `Exec/Run`, files,
+- `sdk/go/` — Go SDK: `Connect/New/Lookup`, `Exec/Run`, files,
   `Push/Pull`, sessions, `Find/Replace`, `Watch`, git verbs, `OpenPty`,
   `Fork/Hibernate/Promote/Checkpoint`, `DialPort/ProxyPort/PreviewURL`,
   `StartLsp`, `Spawn/Ps/Kill/Logs/Attach`; `protocol/wire` carries the frame
@@ -87,8 +87,9 @@ make silkd-image   # silkd release binary in a scratch carrier image
 make images        # base + python images against the local boot + silkd images
 ```
 
-The parent workspace's `go.work` excludes these modules; the Makefile forces
-`GOWORK=off` so local runs match CI. silkd's integration tests spawn real
+The parent workspace's `go.work` includes these modules, which is why the
+Makefile forces `GOWORK=off`: CI has no workspace, and a workspace build
+resolves sibling checkouts instead of the pinned versions. silkd's integration tests spawn real
 processes — run them in a Linux container too (`docker run rust:1 … cargo
 test`) before touching platform-sensitive paths; macOS green alone has hidden
 Linux-only breakage before.

boot/init/src/boot.rs

@@ -43,6 +43,7 @@ impl Marks {
 }
 
 pub fn run() -> ! {
+    let mut marks = Marks::new();
     // Best-effort: if devtmpfs fails there is no console either; later
     // failures then power off silently, which is still the right end state.
     let _ = sys::mount(
@@ -65,7 +66,7 @@ pub fn run() -> ! {
         Ok(cfg) => cfg,
         Err(err) => sys::fatal(&err, cfg::debug_requested(&cmdline)),
     };
-    let mut marks = Marks::new();
+    marks.mark("early");
     if let Err(err) = assemble(&cfg, &mut marks) {
         sys::fatal(&err, cfg.debug);
     }
@@ -158,8 +159,9 @@ fn persist_network(cfg: &BootCfg) {
         eprintln!("sandbox-init: WARN: mkdir {dir}: {err}");
         return;
     }
-    for ip in &cfg.ips {
-        let Some(mac) = wait_nic_mac(&ip.device, NIC_TIMEOUT) else {
+    let devices: Vec<&str> = cfg.ips.iter().map(|ip| ip.device.as_str()).collect();
+    for (ip, mac) in cfg.ips.iter().zip(wait_nic_macs(&devices, NIC_TIMEOUT)) {
+        let Some(mac) = mac else {
             eprintln!(
                 "sandbox-init: WARN: NIC {} not found, static config skipped",
                 ip.device
@@ -173,23 +175,30 @@ fn persist_network(cfg: &BootCfg) {
     }
 }
 
-fn wait_nic_mac(device: &str, timeout: Duration) -> Option<String> {
-    let path = format!("/sys/class/net/{device}/address");
+/// Resolves every NIC's MAC in one sysfs sweep per poll iteration, against one
+/// shared deadline — a missing NIC must cost the timeout once, not once each.
+fn wait_nic_macs(devices: &[&str], timeout: Duration) -> Vec<Option<String>> {
     let deadline = Instant::now() + timeout;
+    let mut found: Vec<Option<String>> = vec![None; devices.len()];
     loop {
-        if let Ok(mac) = fs::read_to_string(&path) {
-            let mac = mac.trim_end().to_string();
-            if !mac.is_empty() && mac != "00:00:00:00:00:00" {
-                return Some(mac);
+        for (slot, device) in found.iter_mut().zip(devices) {
+            if slot.is_none() {
+                *slot = read_nic_mac(device);
             }
         }
-        if Instant::now() >= deadline {
-            return None;
+        if found.iter().all(Option::is_some) || Instant::now() >= deadline {
+            return found;
         }
         std::thread::sleep(POLL_INTERVAL);
     }
 }
 
+fn read_nic_mac(device: &str) -> Option<String> {
+    let mac = fs::read_to_string(format!("/sys/class/net/{device}/address")).ok()?;
+    let mac = mac.trim_end();
+    (!mac.is_empty() && mac != "00:00:00:00:00:00").then(|| mac.to_string())
+}
+
 fn mkdir_all(path: &str) -> Result<(), String> {
     fs::create_dir_all(path).map_err(|err| format!("mkdir {path}: {err}"))
 }

boot/init/src/cfg.rs

@@ -243,7 +243,6 @@ mod tests {
         assert!(!debug_requested("a sandbox.debug=0 b"));
         assert!(!debug_requested("a sandbox.debugger b"));
         assert!(!debug_requested(""));
-        // Last occurrence wins, like parse().
         assert!(!debug_requested("sandbox.debug=1 x sandbox.debug=0"));
         assert!(debug_requested("sandbox.debug=0 x sandbox.debug"));
     }
@@ -298,7 +297,6 @@ mod tests {
 
     #[test]
     fn parse_cocoon_ip_param() {
-        // Exact shape BuildIPParams emits, with both DNS servers.
         let cfg = parse(
             "cocoon.layers=l0 cocoon.cow=cow \
              ip=172.20.100.5::172.20.100.1:255.255.255.0:vm1:eth0:off:8.8.8.8:8.8.4.4",

docs/langchain.md

@@ -19,7 +19,7 @@ schemas, sync-native with `asyncio.to_thread` async bridges):
 
 | tool | what it does |
 |---|---|
-| `sandbox_exec` | run a shell command; stdout/stderr/exit code; disk state persists across calls |
+| `sandbox_exec` | run a shell command, cut off after 5 minutes; stdout/stderr/exit code; disk state persists across calls |
 | `sandbox_write_file` | write a text file (atomic on the guest) |
 | `sandbox_read_file` | read a text file |
 | `sandbox_list_dir` | list a directory as JSON |

docs/sandboxd-api.md

@@ -500,8 +500,9 @@ silently. Folding rules: billable compute seconds per sandbox =
 Σ(hibernate→wake) minus the archived span; archived storage seconds =
 Σ(archive→unarchive/archive_delete), a cheaper store tier than a hibernated
 VM's RAM. An interval left open by a crash clamps to the claim's
-deadline, and the next reconcile emits `reap` for claims it drops. The `vm`
-name joins cocoon's machine-level metering ledger for audit cross-checks.
+deadline; reconcile records nothing for the claims it drops, so a restart
+leaves those intervals without a terminal event. The `vm` name joins
+cocoon's machine-level metering ledger for audit cross-checks.
 
 ## GET /v1/sandboxes/{id}/agent
 

docs/sdk-python.md

@@ -237,13 +237,21 @@ records, while `client.checkpoint(id)` creates an entry-node-bound handle for
 an already known id without listing. `new()` follows an owner redirect and may
 heal a missing record locally; `delete()` acts on the handle's bound node.
 
+`delete()` also asks every peer that node currently sees to drop any replica a
+heal pulled — best-effort eventual cleanup, not a fleet-wide revocation. A peer
+that misses the broadcast (offline, partitioned, or joined later) keeps serving
+branches from its replica until the node's `checkpoint_ttl_hours` ages it out;
+with that TTL at its default of 0 (keep forever), an unreachable peer's replica
+has no cleanup bound at all.
+
 ## Language servers (LSP)
 
 ```python
 lsp = sb.start_lsp("python", "/work")   # flavor image provides the server
 stream = lsp.request()                  # JSON-RPC byte stream (frame it yourself)
 # ... speak Content-Length-framed JSON-RPC over stream.send()/recv() ...
 stream.close()                          # ends the session and reaps the server
+lsp.close()                             # same as lsp.stop(); both are context-managers
 ```
 
 `start_lsp` spawns the language server the flavor image ships for the
@@ -319,7 +327,7 @@ A session is a real persistent shell: cwd, env and shell state survive
 across calls.
 
 ```python
-sess = sb.session(cwd="/work", env={"PATH": "..."})
+sess = sb.session(cwd="/work", env={"PATH": "..."})   # context-manager
 sess.exec("export", "MARK=1")            # persists
 sess.exec("sh", "-c", "echo $MARK")      # "1\n"
 sess.close()
@@ -435,7 +443,8 @@ zero.
 - `SilkdError(kind, message)` — typed guest failure; `kind` is
   `bad_request` / `not_found` / `unimplemented` / `internal`
 - `ExitError(code, stderr, stdout)` — non-zero exit from `exec`
-- `ProtocolError` — broken stream
+- `ProtocolError` — broken stream (EOF, oversized or undecodable frame)
+- `StreamTimeout` — a socket operation hit the client `timeout`; retry the call
 
 ```python
 try:

docs/sdk.md

@@ -4,9 +4,10 @@
 import sandbox "github.com/cocoonstack/sandbox/sdk/go"
 ```
 
-The SDK is stdlib-only. One `Client` talks to one entry node; sandbox
-handles dial their owning node directly, so a client works unchanged against
-a single node or a cluster.
+The SDK has no third-party dependencies (only the sibling `protocol/wire`
+module). One `Client` talks to one entry node; sandbox handles dial their
+owning node directly, so a client works unchanged against a single node or a
+cluster.
 
 ## A complete example
 
@@ -529,5 +530,7 @@ if errors.As(err, &e) && e.Kind == wire.KindUnimplemented {
 }
 ```
 
-Context cancellation is honored on every call: canceling the ctx closes the
-underlying connection and the call returns `ctx.Err()`.
+Context cancellation is honored on every call that takes a ctx: canceling it
+closes the underlying connection. Data-plane calls return `ctx.Err()`
+directly; control-plane verbs return a `*url.Error` wrapping it, so test with
+`errors.Is`, never `==`. `Close` is the exception: it takes no ctx.

docs/silkd.md

@@ -35,12 +35,12 @@ a frame only one side can parse fails CI.
 | sessions | `session_create {id?, cwd?, env?}` / `session_list` / `session_rm {id}` | a session is a real persistent bash; `exec` with `session` runs inside it. Idle sessions are reaped after 30 minutes |
 | fs | `fs_write {path, mode?}` (+`data`/`data_end` frames) / `fs_read` / `fs_list` / `fs_stat` / `fs_mkdir {parents?}` / `fs_rm {recursive?}` / `fs_rename {from, to}` | streaming both directions; write commits atomically via temp+rename and inherits an overwritten file's mode; `fs_list` streams 4096-entry batches |
 | tree | `fs_push {dest}` (+tar as `data` frames) / `fs_pull {path}` | whole trees as tar streams through the guest tar |
-| search | `fs_find {path, pattern, glob?}` → `match{file, line, content}`… → `done` / `fs_replace {files, pattern, replacement}` → `replaced{file, replacements}`… → `done` | regex as data, no shell quoting; `glob` is anchored `*`/`?` wildcards over file names; find skips binary and >8 MiB files |
+| search | `fs_find {path, pattern, glob?}` → `match{file, line, content}`… → `done` / `fs_replace {files, pattern, replacement}` → `replaced{file, replacements}`… → `done` | regex as data, no shell quoting; `glob` is anchored `*`/`?` wildcards over file names; find skips binary and >8 MiB files, and replace skips the same 8 MiB bound with a zero count |
 | watch | `fs_watch {path, recursive?}` | `ready` once armed (events after it are guaranteed captured), then `event{kind, path}` until the client disconnects; watcher or delivery-queue overflow errors arrive as a terminal `error` instead of silently losing events |
 | pty | `pty_open {cols, rows, cwd?, env?, user?}` / `pty_resize {pid, cols, rows}` | a shell under a pseudo-terminal; output as `stdout` frames, input as `stdin` frames, exit terminal. PTYs register in the proc table like any exec |
 | git | `git_clone {url, path, branch?, depth?, auth?}` / `git_status {path}` / `git_add {path, files}` / `git_commit {path, message, author}` / `git_push {path, auth?}` / `git_pull {path, auth?}` / `git_branch {path, action, name?}` | structured results (porcelain-v2 status, commit hash, branch list). `auth` is injected as an in-memory header, never written to guest disk |
 | port | `port_forward {port}` | relays guest TCP 127.0.0.1:port over this connection: `ready` once connected, then `data` both ways (`data_end` half-closes the guest socket); the server closing ends the stream with `done`. Works on both lanes — the no-network lane's only way in |
-| lsp | `lsp_start {language, root?}` / `lsp_request {server_id}` / `lsp_stop {server_id}` | a broker for the language server a flavor image ships: `lsp_start` spawns the argv named in `/etc/silkd/lsp.d/<language>` (absent on the base image → `not_found` naming the flavor) and answers `lsp_started{server_id}`; `lsp_request` attaches the JSON-RPC byte stream (`ready`, then `data` both ways — silkd pipes bytes, it never parses LSP; `data_end` half-closes the server's stdin) and the stream ending reaps the server (v1 single-shot); `lsp_stop` kills it early |
+| lsp | `lsp_start {language, root?}` / `lsp_request {server_id}` / `lsp_stop {server_id}` | a broker for the language server a flavor image ships: `lsp_start` spawns the argv named in `/etc/silkd/lsp.d/<language>` (absent on the base image → `not_found` naming the flavor) and answers `lsp_started{server_id}`; `lsp_request` attaches the JSON-RPC byte stream (`ready`, then `data` both ways — silkd pipes bytes, it never parses LSP; `data_end` half-closes the server's stdin) and the stream ending reaps the server (v1 single-shot); `lsp_stop` kills it early, and a server nothing attaches within 5 minutes is reaped |
 | misc | `info` | `{version, proto, uptime_secs, procs, sessions}` — the in-guest readiness probe (sandboxd consumes it; no SDK surface). Distinct from the control plane's `GET /v1/info`, which reports node pools and claims |
 
 ## Error kinds

e2e/cmd/androidsmoke/main.go

@@ -96,7 +96,7 @@ func initTree(ctx context.Context, sb *sandbox.Sandbox) error {
 			return nil
 		}
 		if time.Now().After(deadline) {
-			return fmt.Errorf("framework never came up:\n%s", tail(out))
+			return fmt.Errorf("framework never came up:\n%s", head(out))
 		}
 		time.Sleep(3 * time.Second)
 	}
@@ -151,7 +151,7 @@ func dialAdb(ctx context.Context, sb *sandbox.Sandbox) (string, error) {
 	return reply, nil
 }
 
-func tail(s string) string {
+func head(s string) string {
 	lines := strings.Split(strings.TrimSpace(s), "\n")
 	if len(lines) > 12 {
 		lines = lines[:12]

e2e/e2e_test.go

@@ -49,8 +49,6 @@ func TestEndToEnd(t *testing.T) {
 	})
 
 	t.Run("refill then warm claim", func(t *testing.T) {
-		// Warm-hit = zero VM ops is proven by the pool unit tests; asserting
-		// create counts here would race the background refill ticker.
 		waitFor(t, func() bool {
 			infos, _ := stack.mgr.Info()
 			return len(infos) == 1 && infos[0].Warm >= 1
@@ -102,8 +100,7 @@ func TestForkEndToEnd(t *testing.T) {
 			t.Fatalf("child %d exec: %q, %v", i, out, err)
 		}
 	}
-	// Children are independent claims: releasing one leaves the sibling and
-	// the parent alive.
+
 	if err := children[0].Close(); err != nil {
 		t.Fatalf("close child 0: %v", err)
 	}
@@ -131,8 +128,7 @@ func TestPromoteEndToEnd(t *testing.T) {
 	if tpl.ContentDigest == "" {
 		t.Fatal("Promote returned an empty content digest")
 	}
-	// Both claim surfaces must work: the owner-bound handle and the
-	// name-based Client call on the (single) node that holds the template.
+
 	child, err := tpl.New(t.Context())
 	if err != nil {
 		t.Fatalf("claim via template handle: %v", err)
@@ -206,9 +202,6 @@ func TestCheckpointEndToEnd(t *testing.T) {
 	}
 }
 
-// TestTwoTenantFlow drives two tenants through the real stack: each claims
-// under its own token, quotas bind per tenant, checkpoint listings are
-// isolated, and operator surfaces refuse tenant tokens with 403.
 func TestTwoTenantFlow(t *testing.T) {
 	tenants := []config.TenantSpec{
 		{Name: "acme", Token: "acme-tok", MaxClaims: 1},
@@ -354,9 +347,6 @@ func TestVolumesEndToEnd(t *testing.T) {
 	}
 }
 
-// TestWritableVolumeEndToEnd drives one writable claim through the whole
-// stack: the SDK's mode reaches the engine as a writable attach, a live writer
-// refuses every other claim on the name, and release unmounts before removal.
 func TestWritableVolumeEndToEnd(t *testing.T) {
 	scratch := writeVolumeImage(t, "scratch.img", "scratch-bytes")
 	stack := startTenantStack(t, "node-token", nil,
@@ -389,9 +379,8 @@ func TestWritableVolumeEndToEnd(t *testing.T) {
 	}
 }
 
-// TestVolumeModeWireShape pins the claim reply's volume bytes independently of
-// the SDK mirror. The read-only leg runs second on purpose: it is admitted
-// only because the writer's release cleared the dirty marker.
+// The read-only leg runs second: it is admitted only once the writer's
+// release has cleared the dirty marker.
 func TestVolumeModeWireShape(t *testing.T) {
 	scratch := writeVolumeImage(t, "scratch.img", "scratch-bytes")
 	stack := startTenantStack(t, "node-token", nil,
@@ -448,10 +437,6 @@ func TestClaimRefRoundTrip(t *testing.T) {
 	}
 }
 
-// TestAttachOnlyVolumeEndToEnd drives one attach-only writable claim through
-// the whole stack: the device is attached writable and nothing else happens —
-// no mount, no marker, no unmount at release — while admission still excludes
-// every other claim on the name, which is what protects third parties.
 func TestAttachOnlyVolumeEndToEnd(t *testing.T) {
 	scratch := writeVolumeImage(t, "scratch.img", "scratch-bytes")
 	stack := startTenantStack(t, "node-token", nil,
@@ -487,9 +472,6 @@ func TestAttachOnlyVolumeEndToEnd(t *testing.T) {
 	assertNoDirtyMarker(t, scratch, "release")
 }
 
-// TestAttachOnlyVolumeWireShape pins both claim replies' volume bytes: an
-// attach-only entry echoes without a mount, the eager entry is unchanged, and
-// the request flag never rides back in either.
 func TestAttachOnlyVolumeWireShape(t *testing.T) {
 	scratch := writeVolumeImage(t, "scratch.img", "scratch-bytes")
 	stack := startTenantStack(t, "node-token", nil,
@@ -570,7 +552,7 @@ func startStack(t *testing.T, apiToken string, pools ...config.PoolSpec) *stack
 
 func startTenantStack(t *testing.T, apiToken string, tenants []config.TenantSpec, volumes []config.VolumeSpec, pools ...config.PoolSpec) *stack {
 	t.Helper()
-	// Short prefix: the sockets under it must fit darwin's 104-byte sun_path.
+
 	dir, err := os.MkdirTemp("", "sbx")
 	if err != nil {
 		t.Fatalf("setup: %v", err)

e2e/fakeengine_test.go

@@ -16,9 +16,8 @@ import (
 	"github.com/cocoonstack/sandbox/sdk/go/silkd/silkdtest"
 )
 
-// fakeEngine replaces only the cocoon CLI: every "VM" it creates is a
-// silkdtest daemon behind a real hybrid-vsock UDS, so probing and the data
-// plane run the production code paths end to end.
+// fakeEngine replaces only the cocoon CLI: every "VM" is a silkdtest daemon
+// behind a real hybrid-vsock UDS, so the data plane runs production code.
 type fakeEngine struct {
 	real *engine.Engine
 	dir  string
@@ -82,9 +81,8 @@ func (f *fakeEngine) SnapshotRemove(_ context.Context, _ string) error { return
 
 func (f *fakeEngine) SnapshotList(_ context.Context) ([]string, error) { return nil, nil }
 
-// Hibernate closes the VM's silkd listener (the "VMM died") and Restore
-// brings a fresh one up, mirroring the real stop/resume observable from the
-// control plane.
+// Hibernate closes the VM's silkd listener and Restore brings a fresh one up,
+// mirroring the stop/resume the control plane observes.
 func (f *fakeEngine) Hibernate(ctx context.Context, name, _ string) error {
 	return f.Remove(ctx, name)
 }

mcp/server_test.go

@@ -81,7 +81,7 @@ func TestServeSpeaksMCP(t *testing.T) {
 	if got := toolText(t, replies[5]); got != "released" {
 		t.Errorf("release: %q", got)
 	}
-	// exec against the released sandbox: an in-band tool error, not a crash.
+
 	execReply := replies[6]["result"].(map[string]any)
 	if execReply["isError"] != true {
 		t.Errorf("exec after release: %+v, want isError", execReply)