From e18442edd140d3d76bc04657fffae04b6c29ce57 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 18:14:25 +0800 Subject: [PATCH 01/17] engine: read cocoon daemon's opening snapshot of a node past 3,000 VMs cocoon sends every VM record in the sync event's one data line, about 300 bytes a record, so the 1 MiB line cap broke the stream at every connect on a full node and liveness fell back to polling vm list for good. --- sandboxd/engine/cocoond.go | 2 +- sandboxd/engine/cocoond_test.go | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/sandboxd/engine/cocoond.go b/sandboxd/engine/cocoond.go index 9d195bab..e05140cf 100644 --- a/sandboxd/engine/cocoond.go +++ b/sandboxd/engine/cocoond.go @@ -15,7 +15,7 @@ const ( VMDeleted = "DELETED" cocoondEventsURL = "http://cocoond/v1/events" - sseMaxLine = 1 << 20 + sseMaxLine = 64 << 20 ) // VMStatus is cocoon daemon's view of one supervised VM. diff --git a/sandboxd/engine/cocoond_test.go b/sandboxd/engine/cocoond_test.go index 24f44cd7..d2d8bf96 100644 --- a/sandboxd/engine/cocoond_test.go +++ b/sandboxd/engine/cocoond_test.go @@ -6,6 +6,7 @@ import ( "net" "net/http" "slices" + "strings" "testing" "time" @@ -32,6 +33,25 @@ func TestVMEventsDeliversTheSnapshotThenChanges(t *testing.T) { } } +func TestVMEventsReadsTheSnapshotOfAFullNode(t *testing.T) { + socket := sockPath(t) + var sync strings.Builder + sync.WriteString("event: sync\ndata: {\"vms\":[") + for i := range 5000 { + if i > 0 { + sync.WriteByte(',') + } + fmt.Fprintf(&sync, `{"backend":"cloud-hypervisor","id":"%064d","name":"sbx-%d","state":"stopped","transition_generation":3,"last_transition_reason":"hibernate","live":false,"verified_at":"2026-11-05T10:00:00Z"}`, i, i) + } + sync.WriteString("]}\n\n") + serveCocoond(t, socket, sync.String()) + var synced []VMStatus + _ = newTestEngine().VMEvents(t.Context(), socket, func(vms []VMStatus) { synced = vms }, func(VMChange) {}) + if len(synced) != 5000 { + t.Errorf("synced %d VMs from a %d-byte snapshot line, want 5000", len(synced), sync.Len()) + } +} + func TestVMEventsFailsWithoutADaemon(t *testing.T) { ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) defer cancel() From d4e22ba1d671e5e8824121e475c356529767ac35 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 18:15:52 +0800 Subject: [PATCH 02/17] pool: record a release's usage event once the release commits, not after its teardown The claim is durably gone at the commit; a restart or crash during a slow teardown (volume quiesce, vm rm) left a claim event with no release, which the folding rule bills up to the claim's deadline. --- sandboxd/pool/claim.go | 4 ++-- sandboxd/pool/pool_test.go | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/sandboxd/pool/claim.go b/sandboxd/pool/claim.go index 812914e7..6d53ef9d 100644 --- a/sandboxd/pool/claim.go +++ b/sandboxd/pool/claim.go @@ -333,6 +333,8 @@ func (m *Manager) releaseResolved(ctx context.Context, id string, sb *types.Sand logger.Errorf(ctx, saveErr, "persist release of %s", id) return fmt.Errorf("release %s: %w", id, saveErr) } + m.counters.releases.Add(1) + m.recordUsage(ctx, usage.Event{Event: "release", ID: id, VMName: vmName, Tenant: sb.Tenant}) // Cleanup must survive the caller hanging up; the claim is already dropped. ctx = context.WithoutCancel(ctx) if ck != "" { @@ -356,8 +358,6 @@ func (m *Manager) releaseResolved(ctx context.Context, id string, sb *types.Sand } m.out.Disarm(id, removed) m.dropSnap(ctx, snap) - m.counters.releases.Add(1) - m.recordUsage(ctx, usage.Event{Event: "release", ID: id, VMName: vmName, Tenant: sb.Tenant}) return err } diff --git a/sandboxd/pool/pool_test.go b/sandboxd/pool/pool_test.go index 5a3e492e..4299e406 100644 --- a/sandboxd/pool/pool_test.go +++ b/sandboxd/pool/pool_test.go @@ -234,6 +234,23 @@ func TestReleaseAfterResolveTearsDownOnce(t *testing.T) { } } +func TestReleaseRecordsItsUsageEventBeforeTheTeardown(t *testing.T) { + eng := newFakeEngine() + m := newTestManager(t, eng) + sb := mustClaim(t, m, testKey) + stall := make(chan struct{}) + eng.mu.Lock() + eng.removeStall = stall + eng.mu.Unlock() + done := make(chan error, 1) + go func() { done <- m.Release(t.Context(), sb.ID, Cred{Token: sb.Token}) }() + waitFor(t, func() bool { return strings.HasSuffix(usageEventsOf(t, m, sb.ID), "release") }) + close(stall) + if err := <-done; err != nil { + t.Fatalf("release: %v", err) + } +} + func TestReleaseDelayQueuesTeardown(t *testing.T) { synctest.Test(t, func(t *testing.T) { eng := newFakeEngine() From 3597b693a0ef8562af2541a247c2b54a0b42d8b0 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 18:21:20 +0800 Subject: [PATCH 03/17] pool: load a forked claim's tenant for operator forks too, so its cap applies The quota check reads the tenant cache only; an operator fork skipped the lookup that fills it, so after a cache flush or restart the tenant's max_claims read as none and the fork overshot it. A removed tenant still does not stop the operator. --- sandboxd/pool/fork.go | 7 +++---- sandboxd/pool/fork_test.go | 10 ++++++++++ 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/sandboxd/pool/fork.go b/sandboxd/pool/fork.go index 86fe7e3b..e6481827 100644 --- a/sandboxd/pool/fork.go +++ b/sandboxd/pool/fork.go @@ -2,6 +2,7 @@ package pool import ( "context" + "errors" "fmt" "os" "path/filepath" @@ -17,10 +18,8 @@ func (m *Manager) Fork(ctx context.Context, id string, cred Cred, count int, ttl if !ok { return nil, ErrUnknownSandbox } - if !cred.Operator { - if err := m.tenantRemoved(ctx, sb.Tenant); err != nil { - return nil, err - } + if err := m.tenantRemoved(ctx, sb.Tenant); err != nil && (!cred.Operator || !errors.Is(err, ErrTenantRemoved)) { + return nil, err } maxFork := m.view.Load().maxFork if count < 1 || count > maxFork { diff --git a/sandboxd/pool/fork_test.go b/sandboxd/pool/fork_test.go index 3cbde8ff..038c16c8 100644 --- a/sandboxd/pool/fork_test.go +++ b/sandboxd/pool/fork_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/cocoonstack/sandbox/sandboxd/tenants" "github.com/cocoonstack/sandbox/sandboxd/types" ) @@ -53,6 +54,15 @@ func TestForkRacingHibernateUsesPrivateSource(t *testing.T) { } } +func TestAnOperatorForkCountsAgainstACapTheCacheHasNotLoaded(t *testing.T) { + m := tenantManager(t, t.TempDir(), tenants.TenantSpec{Name: "acme", Token: "acme-tok", MaxClaims: 2}) + parent := claimTenant(t, m, "acme") + m.tenants = &coldTenants{Source: m.tenants} + if _, err := m.Fork(t.Context(), parent.ID, Cred{Operator: true}, 2, 0, "", ""); !errors.Is(err, ErrQuota) { + t.Errorf("operator fork of 2 under a cap of 2 with 1 live = %v, want ErrQuota", err) + } +} + func TestForkOfARunningClaimClonesOneTransientSnapshot(t *testing.T) { eng := newFakeEngine() m := newTestManager(t, eng) From 9224bafb528e0a14496c8ead467c68db074027e2 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 18:23:45 +0800 Subject: [PATCH 04/17] pool: answer an invalid PUT /v1/pools spec as invalid config naming its pool, not as a bad fork count --- sandboxd/pool/pool_test.go | 4 ++-- sandboxd/pool/setpools.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sandboxd/pool/pool_test.go b/sandboxd/pool/pool_test.go index 4299e406..3cd5ddec 100644 --- a/sandboxd/pool/pool_test.go +++ b/sandboxd/pool/pool_test.go @@ -632,8 +632,8 @@ func TestSetPoolsRejectsInvalidSpec(t *testing.T) { Template: "rt:24.04", Net: types.NetNone, Size: types.SizeSmall, Warm: -1, }}) - if !errors.Is(err, ErrBadCount) { - t.Fatalf("got %v, want ErrBadCount", err) + if !errors.Is(err, ErrBadConfig) || errors.Is(err, ErrBadCount) { + t.Fatalf("got %v, want ErrBadConfig naming the pool", err) } } diff --git a/sandboxd/pool/setpools.go b/sandboxd/pool/setpools.go index 0c7e9878..9cc121c1 100644 --- a/sandboxd/pool/setpools.go +++ b/sandboxd/pool/setpools.go @@ -38,7 +38,7 @@ func (m *Manager) desiredPools(specs []config.PoolSpec) (map[types.PoolKey]confi return nil, err } if err := spec.ValidateLimits(); err != nil { - return nil, fmt.Errorf("%w: %w", ErrBadCount, err) + return nil, fmt.Errorf("%w: pool %s: %w", ErrBadConfig, spec.Template, err) } // Config-owned fields; accepting one here would silently drop it for _, f := range []struct { From 55fca6a55e9dda0120bf922b58bea5df80ea9d36 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 18:24:14 +0800 Subject: [PATCH 05/17] server: count a checkpoint claim handed to a peer in sandboxd_claim_redirects_total --- sandboxd/server/server.go | 1 + sandboxd/server/server_test.go | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/sandboxd/server/server.go b/sandboxd/server/server.go index aa0f4f7c..fb1bff06 100644 --- a/sandboxd/server/server.go +++ b/sandboxd/server/server.go @@ -678,6 +678,7 @@ func (s *Server) handleClaimCheckpoint(w http.ResponseWriter, r *http.Request) { sb, err := s.mgr.ClaimCheckpoint(r.Context(), ckptID, o) if errors.Is(err, pool.ErrUnknownCheckpoint) { if !req.NoRedirect && s.prober != nil && s.writeRedirect(w, s.prober.Owners(r.Context(), ckptID)) { + s.claimRedirects.Add(1) return } sb, err = s.mgr.ClaimCheckpointHeal(r.Context(), ckptID, o) diff --git a/sandboxd/server/server_test.go b/sandboxd/server/server_test.go index d3643eb3..281fb00d 100644 --- a/sandboxd/server/server_test.go +++ b/sandboxd/server/server_test.go @@ -649,6 +649,16 @@ func TestClaimRedirectsAreCounted(t *testing.T) { } } +func TestCheckpointClaimRedirectsAreCounted(t *testing.T) { + ts := newProberTestServer(t, "", &fakeManager{}, &fakeProber{owners: []string{"peer:7777"}}) + if code := statusOf(t, http.MethodPost, ts.URL+"/v1/checkpoints/ck_0011223344556677/claim", "", `{}`); code != http.StatusOK { + t.Fatalf("claim status %d, want 200 with a redirect", code) + } + if want, raw := "sandboxd_claim_redirects_total 1\n", scrapeMetrics(t, ts.URL); !strings.Contains(raw, want) { + t.Errorf("missing %q in:\n%s", want, raw) + } +} + func TestPutPoolsRejectsUnknownFields(t *testing.T) { ts := newTestServer(t, "sekret", &fakeManager{}, nil) if code := statusOf(t, http.MethodPut, ts.URL+"/v1/pools", "sekret", `{"pools":[{"template":"rt:24.04","net":"none","size":"small","egres":{"allow":[]}}]}`); code != http.StatusBadRequest { From 9b587c2064b15ec5b285b7b7f8df882b47f69e5f Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 18:26:32 +0800 Subject: [PATCH 06/17] test: fix the pool benchmarks the claim index and running count broke, and run every benchmark once in go-test Four benchmarks seeded claims around addClaimLocked or without a store row and failed the cleanup invariants; no CI step ran -bench, so nobody saw it. --- Makefile | 4 ++-- sandboxd/pool/checkpoint_bench_test.go | 10 +++++++++- sandboxd/pool/sweep_bench_test.go | 1 + sandboxd/pool/volume_bench_test.go | 2 +- 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index ff2ed85e..b0c3de6f 100644 --- a/Makefile +++ b/Makefile @@ -57,8 +57,8 @@ sandboxd: ## build dist/sandboxd mkdir -p dist cd sandboxd && GOWORK=off go build -ldflags "-X main.version=$(SANDBOXD_VERSION)" -o ../dist/sandboxd . -go-test: ## go test -race across the Go modules - for m in $(GO_MODULES); do (cd $$m && GOWORK=off go test -race ./...) || exit 1; done +go-test: ## go test -race and one pass of every benchmark across the Go modules + for m in $(GO_MODULES); do (cd $$m && GOWORK=off go test -race ./... && GOWORK=off go test -run '^$$' -bench . -benchtime 1x ./...) || exit 1; done bench: ## claim-tier + data-plane benchmarks on this node; removes every sbx-* VM (see docs/benchmarks.md) bash scripts/bench.sh diff --git a/sandboxd/pool/checkpoint_bench_test.go b/sandboxd/pool/checkpoint_bench_test.go index e3ba1b6c..836ff72a 100644 --- a/sandboxd/pool/checkpoint_bench_test.go +++ b/sandboxd/pool/checkpoint_bench_test.go @@ -38,7 +38,7 @@ func BenchmarkPinnedArchiveCheckpoints(b *testing.B) { for _, checkpoints := range []bool{false, true} { b.Run(fmt.Sprintf("claims=5000/checkpoints=%t", checkpoints), func(b *testing.B) { m := newTestManager(b, newFakeEngine(), config.PoolSpec{PoolKey: testKey}) - m.claimed = checkpointBenchClaims(checkpoints) + adoptBenchClaims(m, checkpointBenchClaims(checkpoints)) if err := m.store.save(m.claimed); err != nil { b.Fatal(err) } @@ -56,6 +56,14 @@ func BenchmarkPinnedArchiveCheckpoints(b *testing.B) { } } +func adoptBenchClaims(m *Manager, claims map[string]*types.Sandbox) { + m.mu.Lock() + defer m.mu.Unlock() + for _, sb := range claims { + m.addClaimLocked(sb) + } +} + func checkpointBenchClaims(checkpoints bool) map[string]*types.Sandbox { claims := benchClaims(5000) if checkpoints { diff --git a/sandboxd/pool/sweep_bench_test.go b/sandboxd/pool/sweep_bench_test.go index 54fcfb3d..4657adea 100644 --- a/sandboxd/pool/sweep_bench_test.go +++ b/sandboxd/pool/sweep_bench_test.go @@ -42,6 +42,7 @@ func sweepManager(b *testing.B, idleSeconds int) *Manager { sb := &types.Sandbox{ID: fmt.Sprintf("sb_%05d", i), VMName: fmt.Sprintf("sbx-%05d", i), Key: testKey, Deadline: now.Add(time.Hour)} sb.TouchAt(now) m.addClaimLocked(sb) + m.store.set(sb) } m.mu.Unlock() return m diff --git a/sandboxd/pool/volume_bench_test.go b/sandboxd/pool/volume_bench_test.go index a25f7098..a7b285aa 100644 --- a/sandboxd/pool/volume_bench_test.go +++ b/sandboxd/pool/volume_bench_test.go @@ -39,7 +39,7 @@ func BenchmarkVolumeAdmission(b *testing.B) { func volumeBenchManager(b *testing.B) *Manager { b.Helper() m := newTestManager(b, newFakeEngine(), config.PoolSpec{PoolKey: testKey}) - m.claimed = checkpointBenchClaims(false) + adoptBenchClaims(m, checkpointBenchClaims(false)) if err := m.store.save(m.claimed); err != nil { b.Fatal(err) } From be8b2762470bcf4615060324f704ab319d7ec9e3 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 18:29:39 +0800 Subject: [PATCH 07/17] cut: drop the info frame's proto field and the keep-alive constant nothing reads since the SDKs stopped probing --- docs/silkd.md | 7 +++---- mcp/server_test.go | 2 +- protocol/wire/fixtures/v1/resp_info.json | 2 +- protocol/wire/frame.go | 3 --- sandboxd/engine/engine_test.go | 2 +- sandboxd/engine/fakesilkd_test.go | 2 +- sdk/go/sandbox_test.go | 2 -- sdk/go/silkd/silkdtest/silkdtest.go | 2 +- sdk/python/cocoonsandbox/frames.py | 2 -- sdk/python/tests/test_keepalive.py | 5 ++--- sdk/python/tests/test_upgrade.py | 3 +-- silkd/src/proto.rs | 3 --- silkd/src/server.rs | 1 - silkd/tests/exec_e2e.rs | 3 +-- 14 files changed, 12 insertions(+), 27 deletions(-) diff --git a/docs/silkd.md b/docs/silkd.md index 5948f8f0..03ef6e64 100644 --- a/docs/silkd.md +++ b/docs/silkd.md @@ -25,9 +25,8 @@ then reads the next request on the same connection. `stdin`, `stdin_close`, terminal frame they are dropped, and a request frame sent while a streaming RPC still runs ends that RPC's input and is served after it, except in the upload of `fs_write` or `fs_push`: there a request frame before `data_end` -fails the upload with `bad_request` and is dropped. A client that -closes after one RPC is served exactly as before; `info` reports `proto: 2` -on a daemon that keeps the connection. Binary payloads ride base64 in `data` +fails the upload with `bad_request` and is dropped. A client may close +after any terminal frame. Binary payloads ride base64 in `data` fields. Frames are capped at 8 MiB; requests carry `"v": 1` and unknown fields are ignored (the forward-compatibility story). @@ -59,7 +58,7 @@ suites — a frame only one side can parse fails CI. | 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?}` | `git_status_result` (porcelain v2), `git_commit_result{hash}`, `git_branches{current, branches}` for `git_branch {action: "list"}`; every other verb terminates with `done`. A status past about 1 MiB of entries carries `truncated: true` with the head of the list, so one frame never exceeds the cap. `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. Each direction ends on its own: `data_end` half-closes the guest socket, `done` reports the guest closing its write side, and the RPC ends once both have. A client that only drops the connection leaves the relay open until the guest side closes or writes. 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/` (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` | `info{version, proto, uptime_secs, procs, sessions}` — sandboxd uses it for readiness and the SDKs use `proto` to negotiate connection reuse; there is no public SDK method. Distinct from the control plane's `GET /v1/info`, which reports node pools and claims | +| misc | `info` | `info{version, uptime_secs, procs, sessions}` — sandboxd uses it for readiness; there is no public SDK method. Distinct from the control plane's `GET /v1/info`, which reports node pools and claims | ## Error kinds diff --git a/mcp/server_test.go b/mcp/server_test.go index 428938ac..a57122db 100644 --- a/mcp/server_test.go +++ b/mcp/server_test.go @@ -21,7 +21,7 @@ import ( ) var infoFrame = func() string { - frame, err := wire.EncodeResponse(&wire.InfoResp{Version: "test", Proto: wire.KeepAliveProto}) + frame, err := wire.EncodeResponse(&wire.InfoResp{Version: "test"}) if err != nil { panic(err) } diff --git a/protocol/wire/fixtures/v1/resp_info.json b/protocol/wire/fixtures/v1/resp_info.json index 2c91fc30..b4e1ef81 100644 --- a/protocol/wire/fixtures/v1/resp_info.json +++ b/protocol/wire/fixtures/v1/resp_info.json @@ -1 +1 @@ -{"type":"info","version":"0.1.0","proto":2,"uptime_secs":12,"procs":1,"sessions":0} +{"type":"info","version":"0.1.0","uptime_secs":12,"procs":1,"sessions":0} diff --git a/protocol/wire/frame.go b/protocol/wire/frame.go index 73f3c741..7d12dc0b 100644 --- a/protocol/wire/frame.go +++ b/protocol/wire/frame.go @@ -15,8 +15,6 @@ import ( const ( // ProtoVersion is stamped into every request as "v"; silkd ignores unknown fields. ProtoVersion = 1 - // KeepAliveProto is the InfoResp.Proto from which silkd serves RPCs back to back on one connection. - KeepAliveProto = 2 // MaxFrame mirrors silkd's frame cap. MaxFrame = 8 << 20 // BulkChunk mirrors silkd's BULK_CHUNK, the payload of one bulk data frame. @@ -500,7 +498,6 @@ func (e *ErrorResp) Error() string { return e.Kind + ": " + e.Message } // InfoResp answers Info. type InfoResp struct { Version string `json:"version"` - Proto uint32 `json:"proto"` UptimeSecs uint64 `json:"uptime_secs"` Procs int `json:"procs"` Sessions int `json:"sessions"` diff --git a/sandboxd/engine/engine_test.go b/sandboxd/engine/engine_test.go index 94c40af2..4aeff264 100644 --- a/sandboxd/engine/engine_test.go +++ b/sandboxd/engine/engine_test.go @@ -21,7 +21,7 @@ import ( ) const ( - infoFrame = `{"type":"info","version":"test","proto":1,"uptime_secs":0,"procs":0,"sessions":0}` + "\n" + infoFrame = `{"type":"info","version":"test","uptime_secs":0,"procs":0,"sessions":0}` + "\n" errFrame = `{"type":"error","kind":"internal","message":"boom"}` + "\n" ) diff --git a/sandboxd/engine/fakesilkd_test.go b/sandboxd/engine/fakesilkd_test.go index 720d6b04..ba79ce31 100644 --- a/sandboxd/engine/fakesilkd_test.go +++ b/sandboxd/engine/fakesilkd_test.go @@ -127,7 +127,7 @@ func (f *fakeSilkd) serve(conn net.Conn) { } switch req.Op { case "info": - writeFakeSilkdResponse(conn, &wire.InfoResp{Version: "test", Proto: wire.KeepAliveProto}) + writeFakeSilkdResponse(conn, &wire.InfoResp{Version: "test"}) case "fs_write": f.handleWrite(conn, r, req.Path, req.Mode) case "fs_list": diff --git a/sdk/go/sandbox_test.go b/sdk/go/sandbox_test.go index a40a8975..1966d969 100644 --- a/sdk/go/sandbox_test.go +++ b/sdk/go/sandbox_test.go @@ -95,8 +95,6 @@ func TestRunCancelKillsTheCommandItStarted(t *testing.T) { return } switch req := req.(type) { - case *wire.Info: - _, _ = io.WriteString(conn, `{"type":"info","version":"test","proto":2}`+"\n") case *wire.Exec: _, _ = io.WriteString(conn, `{"type":"started","pid":7}`+"\n"+`{"type":"stdout","data":"NDIK"}`+"\n") _, _ = io.Copy(io.Discard, r) diff --git a/sdk/go/silkd/silkdtest/silkdtest.go b/sdk/go/silkd/silkdtest/silkdtest.go index cfc2010c..192d19d6 100644 --- a/sdk/go/silkd/silkdtest/silkdtest.go +++ b/sdk/go/silkd/silkdtest/silkdtest.go @@ -76,7 +76,7 @@ func serveConn(conn net.Conn, r *bufio.Reader, serve func(net.Conn, *bufio.Reade case *wire.Stdin, *wire.StdinClose, *wire.Data, *wire.DataEnd: continue case *wire.Info: - send(conn, &wire.InfoResp{Version: "silkdtest", Proto: wire.KeepAliveProto}) + send(conn, &wire.InfoResp{Version: "silkdtest"}) default: serve(conn, r, req) } diff --git a/sdk/python/cocoonsandbox/frames.py b/sdk/python/cocoonsandbox/frames.py index 45e41cd4..681d2f96 100644 --- a/sdk/python/cocoonsandbox/frames.py +++ b/sdk/python/cocoonsandbox/frames.py @@ -10,8 +10,6 @@ from typing import Any PROTO_VERSION = 1 -# the info proto from which silkd serves RPCs back to back on one connection -KEEP_ALIVE_PROTO = 2 MAX_FRAME = 8 * 1024 * 1024 BULK_CHUNK = 256 * 1024 # tar and port streams chunk at 1 MiB, still under MAX_FRAME after base64 diff --git a/sdk/python/tests/test_keepalive.py b/sdk/python/tests/test_keepalive.py index 7491c65c..b70c2897 100644 --- a/sdk/python/tests/test_keepalive.py +++ b/sdk/python/tests/test_keepalive.py @@ -10,7 +10,6 @@ from conftest import accept_upgrade, sandbox_at, wait_until from cocoonsandbox import SilkdError -from cocoonsandbox.frames import KEEP_ALIVE_PROTO INPUT_OPS = ("stdin", "stdin_close", "data", "data_end") @@ -64,7 +63,7 @@ def _serve(self, conn: socket.socket) -> None: def _answer(self, op: str) -> list: if op == "info": - return [{"type": "info", "version": "fake", "proto": KEEP_ALIVE_PROTO, "uptime_secs": 0, "procs": 0}] + return [{"type": "info", "version": "fake", "uptime_secs": 0, "procs": 0}] if op == "fs_stat": return [{"type": "stat", "info": {"kind": "dir", "size": 0, "mode": 0o755, "mtime_epoch_secs": 0}}] if op == "exec": @@ -145,7 +144,7 @@ def serve(): for line in iter(reader.readline, b""): op = json.loads(line)["op"] if op == "info": - info = {"type": "info", "version": "fake", "proto": KEEP_ALIVE_PROTO, "uptime_secs": 0} + info = {"type": "info", "version": "fake", "uptime_secs": 0} conn.sendall(json.dumps(info).encode() + b"\n") elif op == "exec": conn.sendall(b'{"type":"started","pid":1}\n{"type":"exit","code":0}\n') diff --git a/sdk/python/tests/test_upgrade.py b/sdk/python/tests/test_upgrade.py index a68cf561..da95eed6 100644 --- a/sdk/python/tests/test_upgrade.py +++ b/sdk/python/tests/test_upgrade.py @@ -10,10 +10,9 @@ from conftest import sandbox_at from cocoonsandbox import APIError, SandboxTimeout -from cocoonsandbox.frames import KEEP_ALIVE_PROTO REPLIES = { - "info": {"type": "info", "version": "fake", "proto": KEEP_ALIVE_PROTO, "uptime_secs": 0, "procs": 0}, + "info": {"type": "info", "version": "fake", "uptime_secs": 0, "procs": 0}, "fs_mkdir": {"type": "done"}, } REJECT = b"HTTP/1.1 404 Not Found\r\nContent-Type: text/plain\r\nContent-Length: 15\r\n\r\nunknown sandbox" diff --git a/silkd/src/proto.rs b/silkd/src/proto.rs index da9b76dd..39c87b2e 100644 --- a/silkd/src/proto.rs +++ b/silkd/src/proto.rs @@ -14,8 +14,6 @@ use tokio::io::{ /// Mirrors cocoon-agent's frame cap so a malformed peer can't OOM us. pub const MAX_FRAME: usize = 8 * 1024 * 1024; -/// 2 serves RPCs back to back on one connection; 1 closed after one. -pub const KEEP_ALIVE_PROTO: u32 = 2; /// Read size for exec, pty and session output. pub const READ_CHUNK: usize = 32 * 1024; @@ -243,7 +241,6 @@ pub enum Response { }, Info { version: Cow<'static, str>, - proto: u32, uptime_secs: u64, procs: usize, sessions: usize, diff --git a/silkd/src/server.rs b/silkd/src/server.rs index 79f394ea..59486950 100644 --- a/silkd/src/server.rs +++ b/silkd/src/server.rs @@ -232,7 +232,6 @@ impl State { w, &Response::Info { version: Cow::Borrowed(env!("CARGO_PKG_VERSION")), - proto: proto::KEEP_ALIVE_PROTO, uptime_secs: self.started.elapsed().as_secs(), procs: self.table.len(), sessions: self.sessions.len(), diff --git a/silkd/tests/exec_e2e.rs b/silkd/tests/exec_e2e.rs index 7ff6ac8d..1651c4ed 100644 --- a/silkd/tests/exec_e2e.rs +++ b/silkd/tests/exec_e2e.rs @@ -103,10 +103,9 @@ async fn empty_argv_is_a_bad_request() { } #[tokio::test] -async fn info_reports_version_and_proto() { +async fn info_reports_the_version() { let frames = roundtrip(r#"{"op":"info"}"#).await; assert_eq!(type_of(&frames[0]), "info"); - assert_eq!(frames[0]["proto"], 2); assert!(frames[0]["version"].as_str().is_some()); } From 3b1e87d5cf5b9bf203ae2e90318a2aab7bcd2ca6 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 18:32:01 +0800 Subject: [PATCH 08/17] cut: resolve a claim's egress layers from its recorded layer alone, dropping the live-pool fallback for records older than the field Every claim records its layer at commit since #216 and adopt and move carry it, so the fallback served only pre-#216 records; the Pooled callback threaded through outbound existed for it alone. --- docs/egress.md | 3 +-- sandboxd/outbound/claim.go | 2 +- sandboxd/outbound/outbound.go | 6 +----- sandboxd/outbound/outbound_test.go | 1 - sandboxd/outbound/view.go | 8 ++++---- sandboxd/pool/egress.go | 20 +------------------- sandboxd/pool/egress_test.go | 20 +++++++++----------- sandboxd/pool/env.go | 4 ++-- sandboxd/pool/env_test.go | 2 +- sandboxd/pool/reload.go | 2 +- sandboxd/pool/tenants_test.go | 4 ++-- sandboxd/types/types.go | 5 ++--- 12 files changed, 25 insertions(+), 52 deletions(-) diff --git a/docs/egress.md b/docs/egress.md index 25d6480e..2cbf2be7 100644 --- a/docs/egress.md +++ b/docs/egress.md @@ -191,8 +191,7 @@ stays denied). A claim with `"egress": false` takes no policy at all, whatever its pool's: no door is bound, so the guest's dial is refused. Which layers a claim has is settled when it is made and kept for its life: a `PUT /v1/pools` that adds or drops a pool -without a config-file policy changes the claims made after it, never a live one (a record from before this -rule resolves against the live pool set). Secrets are registered separately and +without a config-file policy changes the claims made after it, never a live one. Secrets are registered separately and referenced by name — the value comes from the environment, never the config file: a claim's own `guest: false` env first, the node's second ([below](#claim-env-and-secrets)). diff --git a/sandboxd/outbound/claim.go b/sandboxd/outbound/claim.go index 237389ae..5f9d78ee 100644 --- a/sandboxd/outbound/claim.go +++ b/sandboxd/outbound/claim.go @@ -146,7 +146,7 @@ func (c *claim) current() egress.Evaluator { if r := c.cur.Load(); r.view == v { return r.eval } - eval, ok := v.Resolve(c.sb, c.h.o.Pooled) + eval, ok := v.Resolve(c.sb) if !ok { eval = egress.Policy{} } diff --git a/sandboxd/outbound/outbound.go b/sandboxd/outbound/outbound.go index cb48f5b5..11519287 100644 --- a/sandboxd/outbound/outbound.go +++ b/sandboxd/outbound/outbound.go @@ -28,9 +28,6 @@ type Engine interface { InstallCACert(ctx context.Context, vsockSocket string, certPEM []byte) error } -// PooledFunc reports whether sb's key had a pool when it was claimed; it may take the caller's lock. -type PooledFunc func(sb *types.Sandbox) bool - // TapFunc applies or drops the nft lock on one tap. type TapFunc func(tap string) error @@ -41,7 +38,6 @@ type VerdictFunc func(ip netip.Addr, port uint16) (bool, error) type Options struct { Engine Engine View func() *View - Pooled PooledFunc Record func(ctx context.Context, id, tenant string, ev egress.Event) Transfer func(ctx context.Context, id, tenant string, ev egress.Event, sent, received int64) CA *egress.CA @@ -108,7 +104,7 @@ func (h *Host) ArmProxy(ctx context.Context, sb *types.Sandbox) error { displaced.close() } v := h.o.View() - policy, ok := v.Resolve(sb, h.o.Pooled) + policy, ok := v.Resolve(sb) if !ok || sb.VsockSocket == "" { if d != nil { d.close() diff --git a/sandboxd/outbound/outbound_test.go b/sandboxd/outbound/outbound_test.go index a3193089..54dfacbe 100644 --- a/sandboxd/outbound/outbound_test.go +++ b/sandboxd/outbound/outbound_test.go @@ -264,7 +264,6 @@ func newHost(t *testing.T, eng *fakeEngine, cfg *config.Config, edit func(*Optio o := Options{ Engine: eng, View: func() *View { return v }, - Pooled: func(*types.Sandbox) bool { return false }, Record: func(context.Context, string, string, egress.Event) {}, Transfer: func(context.Context, string, string, egress.Event, int64, int64) {}, Lock: func(string) error { return nil }, diff --git a/sandboxd/outbound/view.go b/sandboxd/outbound/view.go index 44047033..f1eb3e01 100644 --- a/sandboxd/outbound/view.go +++ b/sandboxd/outbound/view.go @@ -85,14 +85,14 @@ func (v *View) HasClass(name string) bool { } // Resolve is sb's effective policy, pool ∩ tenant: root has no tenant layer, an unpooled key no pool one, a NoEgress claim none at all; pooled runs only when the key has no pool policy. -func (v *View) Resolve(sb *types.Sandbox, pooled PooledFunc) (egress.PolicyEvaluator, bool) { +func (v *View) Resolve(sb *types.Sandbox) (egress.PolicyEvaluator, bool) { if sb.NoEgress { return nil, false } poolPol := v.poolEgress[sb.PolicyKey()] tenantPol := v.classEgress[sb.EgressClass] if poolPol == nil { - if sb.Tenant == "" || tenantPol == nil || pooled(sb) { + if sb.Tenant == "" || tenantPol == nil || sb.Layer == types.LayerPooled { return nil, false } return *tenantPol, true @@ -107,8 +107,8 @@ func (v *View) Resolve(sb *types.Sandbox, pooled PooledFunc) (egress.PolicyEvalu } // InjectGap names, in env order, each inject host sb's egress does not intercept and each header a covering pool secret already sets. -func (v *View) InjectGap(sb *types.Sandbox, env types.Env, pooled PooledFunc) []string { - eval, ok := v.Resolve(sb, pooled) +func (v *View) InjectGap(sb *types.Sandbox, env types.Env) []string { + eval, ok := v.Resolve(sb) var gap []string pool := v.poolEgress[sb.PolicyKey()] for _, name := range slices.Sorted(maps.Keys(env)) { diff --git a/sandboxd/pool/egress.go b/sandboxd/pool/egress.go index c956b670..130a7d65 100644 --- a/sandboxd/pool/egress.go +++ b/sandboxd/pool/egress.go @@ -29,23 +29,6 @@ func (m *Manager) disarmIfReleased(sb *types.Sandbox) bool { return !live } -func (m *Manager) claimPooled(sb *types.Sandbox) bool { - if sb.Layer != "" { - return sb.Layer == types.LayerPooled - } - m.mu.Lock() - defer m.mu.Unlock() - return m.claimPooledLocked(sb) -} - -func (m *Manager) claimPooledLocked(sb *types.Sandbox) bool { - if sb.Layer != "" { - return sb.Layer == types.LayerPooled - } - _, ok := m.activePool(sb.PolicyKey()) - return ok -} - // deniedClaims counts per policy key the live claims v denies for lack of a pool policy. func (m *Manager) deniedClaims(v *outbound.View) map[types.PoolKey]int { denied := map[types.PoolKey]int{} @@ -56,7 +39,7 @@ func (m *Manager) deniedClaims(v *outbound.View) map[types.PoolKey]int { if sb.NoEgress || v.HasPolicy(key) { continue } - if _, ok := v.Resolve(sb, m.claimPooledLocked); !ok { + if _, ok := v.Resolve(sb); !ok { denied[key]++ } } @@ -78,7 +61,6 @@ func (m *Manager) outboundOptions(ca *egress.CA) outbound.Options { return outbound.Options{ Engine: m.eng, View: func() *outbound.View { return m.view.Load().out }, - Pooled: m.claimPooled, Record: m.recordEgress, Transfer: func(ctx context.Context, id, tenant string, ev egress.Event, sent, received int64) { m.recordUsage(ctx, usage.Event{Event: "egress_bytes", ID: id, Tenant: tenant, Reference: ev.Host, Upstream: ev.Upstream, Sent: sent, Received: received}) diff --git a/sandboxd/pool/egress_test.go b/sandboxd/pool/egress_test.go index d991ca5f..3bb89c1b 100644 --- a/sandboxd/pool/egress_test.go +++ b/sandboxd/pool/egress_test.go @@ -113,11 +113,11 @@ func TestEffectivePolicyKeepsTheLayerPinnedAtClaim(t *testing.T) { }) m.pools = map[types.PoolKey]*pool{testKey: newPool(testKey)} - if _, ok := m.view.Load().out.Resolve(&types.Sandbox{Key: testKey, Tenant: "acme", EgressClass: "desk", Layer: types.LayerUnpooled}, m.claimPooled); !ok { + if _, ok := m.view.Load().out.Resolve(&types.Sandbox{Key: testKey, Tenant: "acme", EgressClass: "desk", Layer: types.LayerUnpooled}); !ok { t.Error("a claim made on an unpooled key lost its tenant policy when the key gained a pool") } m.pools = map[types.PoolKey]*pool{} - if _, ok := m.view.Load().out.Resolve(&types.Sandbox{Key: testKey, Tenant: "acme", EgressClass: "desk", Layer: types.LayerPooled}, m.claimPooled); ok { + if _, ok := m.view.Load().out.Resolve(&types.Sandbox{Key: testKey, Tenant: "acme", EgressClass: "desk", Layer: types.LayerPooled}); ok { t.Error("a claim made on a policy-less pool gained the tenant policy when the pool was dropped") } } @@ -163,22 +163,20 @@ func TestEffectivePolicyIntersectsPoolAndTenantPolicies(t *testing.T) { } for _, tt := range cases { t.Run(tt.name, func(t *testing.T) { - m.pools = map[types.PoolKey]*pool{} - if tt.pooled { - m.pools[testKey] = newPool(testKey) - m.pools[testKey].removed = tt.removed - } editEgress(t, m, func(cfg *config.Config) { cfg.Pools = []config.PoolSpec{{PoolKey: testKey, Egress: tt.pool}} if tt.tnPol != nil { cfg.EgressClasses = []config.EgressClass{{Name: "desk", Egress: tt.tnPol}} } }) - sb := &types.Sandbox{Key: testKey, Tenant: tt.tenant} + sb := &types.Sandbox{Key: testKey, Tenant: tt.tenant, Layer: types.LayerUnpooled} + if tt.pooled && !tt.removed { + sb.Layer = types.LayerPooled + } if tt.tenant != "" { sb.EgressClass = "desk" } - eval, ok := m.view.Load().out.Resolve(sb, m.claimPooled) + eval, ok := m.view.Load().out.Resolve(sb) if ok != tt.wantArmed { t.Fatalf("armed=%v, want %v", ok, tt.wantArmed) } @@ -335,7 +333,7 @@ func TestSetPoolsPreservesEgressPolicy(t *testing.T) { m.pools[egKey].goldenDir = gd m.mu.Unlock() policyLive := func() bool { - _, ok := m.view.Load().out.Resolve(&types.Sandbox{Key: egKey}, m.claimPooled) + _, ok := m.view.Load().out.Resolve(&types.Sandbox{Key: egKey}) return ok } if err := m.SetPools(t.Context(), []config.PoolSpec{{PoolKey: egKey, WarmMax: 3}}); err != nil { @@ -461,7 +459,7 @@ func TestAPromotedTemplatesCloneEgressesAsItsSourcePool(t *testing.T) { t.Fatalf("clone policy source %+v layer %q, want the source pool %+v and its pooled layer", clone.PolicySource, clone.Layer, testKey) } dialDoors(t, clone) - eval, ok := m.view.Load().out.Resolve(clone, m.claimPooled) + eval, ok := m.view.Load().out.Resolve(clone) if !ok { t.Fatal("the clone resolved no policy") } diff --git a/sandboxd/pool/env.go b/sandboxd/pool/env.go index 26691629..dd9646d2 100644 --- a/sandboxd/pool/env.go +++ b/sandboxd/pool/env.go @@ -67,7 +67,7 @@ func (m *Manager) writeEnv(ctx context.Context, id, tenant, verb string, next fu return err } if fresh := env.InjectsChanged(prev); fresh != nil { - if gap := m.view.Load().out.InjectGap(sb, fresh, m.claimPooledLocked); len(gap) > 0 { + if gap := m.view.Load().out.InjectGap(sb, fresh); len(gap) > 0 { m.mu.Unlock() return fmt.Errorf("%w: inject %s", ErrBadEnv, strings.Join(gap, ", ")) } @@ -148,7 +148,7 @@ func (m *Manager) checkInjects(v *configView, sbs []*types.Sandbox) error { if !sb.Env.HasInject() { continue } - if gap := v.out.InjectGap(sb, sb.Env, m.claimPooled); len(gap) > 0 { + if gap := v.out.InjectGap(sb, sb.Env); len(gap) > 0 { return fmt.Errorf("%w: inject %s", ErrBadEnv, strings.Join(gap, ", ")) } } diff --git a/sandboxd/pool/env_test.go b/sandboxd/pool/env_test.go index ff113250..424d61cb 100644 --- a/sandboxd/pool/env_test.go +++ b/sandboxd/pool/env_test.go @@ -565,7 +565,7 @@ func TestInjectsSurviveARestartAndAReloadThatDropsTheInterceptStopsThem(t *testi cfg.Pools = []config.PoolSpec{{PoolKey: testKey, Warm: 1, Egress: &egress.Policy{Allow: []egress.Rule{{Host: "api.github.com"}}}}} }) v := m2.view.Load() - if gap := v.out.InjectGap(restored, env, m2.claimPooled); len(gap) != 1 { + if gap := v.out.InjectGap(restored, env); len(gap) != 1 { t.Errorf("gap after the intercept rule went %v, want the inject named", gap) } } diff --git a/sandboxd/pool/reload.go b/sandboxd/pool/reload.go index a155934e..1fbcc93b 100644 --- a/sandboxd/pool/reload.go +++ b/sandboxd/pool/reload.go @@ -78,7 +78,7 @@ func (m *Manager) warnLostInjects(ctx context.Context, v *configView, sbs []*typ m.mu.Lock() env := sb.Env m.mu.Unlock() - if gap := v.out.InjectGap(sb, env, m.claimPooled); len(gap) > 0 { + if gap := v.out.InjectGap(sb, env); len(gap) > 0 { log.WithFunc("pool.warnLostInjects").Warnf(ctx, "claim %s: inject no longer intercepted: %s", sb.ID, strings.Join(gap, ", ")) } } diff --git a/sandboxd/pool/tenants_test.go b/sandboxd/pool/tenants_test.go index 61934856..6f51a041 100644 --- a/sandboxd/pool/tenants_test.go +++ b/sandboxd/pool/tenants_test.go @@ -273,7 +273,7 @@ func TestARuntimeTenantTakesItsEgressClassWithoutARestart(t *testing.T) { if err != nil { t.Fatalf("claim u-1: %v", err) } - eval, ok := m.view.Load().out.Resolve(classed, m.claimPooled) + eval, ok := m.view.Load().out.Resolve(classed) if !ok || classed.EgressClass != "desk" || dtoOf(classed).EgressClass != "desk" { t.Fatalf("u-1 claim class %q armed %v, want the desk class stamped and journaled", classed.EgressClass, ok) } @@ -312,7 +312,7 @@ func TestARuntimeTenantTakesItsEgressClassWithoutARestart(t *testing.T) { if err != nil { t.Fatalf("claim u-2: %v", err) } - if _, ok = m.view.Load().out.Resolve(bare, m.claimPooled); ok { + if _, ok = m.view.Load().out.Resolve(bare); ok { t.Error("a tenant without a class gained egress") } diff --git a/sandboxd/types/types.go b/sandboxd/types/types.go index 87b45de7..82cf0550 100644 --- a/sandboxd/types/types.go +++ b/sandboxd/types/types.go @@ -260,9 +260,8 @@ type Sandbox struct { // ClaimedAt is the first grant; renew and wake move Deadline, never this. ClaimedAt time.Time `json:"claimed_at,omitzero"` // LeaseSeconds is the lease the claim was granted; a wake from the archive grants it again. - LeaseSeconds int `json:"lease_seconds,omitzero"` - // Layer is empty on a record older than the field, which resolves against the live pool set. - Layer PolicyLayer `json:"policy_layer,omitempty"` + LeaseSeconds int `json:"lease_seconds,omitzero"` + Layer PolicyLayer `json:"policy_layer,omitempty"` // PolicySource is the pool whose egress policy a clone of a promoted template or its checkpoint inherits. PolicySource PoolKey `json:"policy_source,omitzero"` // NoEgress pins a claim that asked for no egress policy. From 0354eb00c6fb8ceb43297e56fd1b81c2b2c3e078 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 18:46:07 +0800 Subject: [PATCH 09/17] pool: keep a claim's last replica attempt across ticks it cannot replicate replicateOnce dropped the attempt time whenever a claim was hibernated, archiving or settling, so a claim that idles and wakes inside every interval never came due, had no registry row, and was lost with its node. --- docs/cluster.md | 2 +- sandboxd/pool/replica.go | 3 +++ sandboxd/pool/replica_test.go | 17 +++++++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/docs/cluster.md b/docs/cluster.md index 45f25e45..e92411c3 100644 --- a/docs/cluster.md +++ b/docs/cluster.md @@ -481,7 +481,7 @@ checkpoint store and keeps each claim's registry row on the latest snapshot: restart, or after it wakes from an archive or an adoption; until a claim woken that way commits its first replica it has no row, and it is lost with its node. A hibernated, archived or failed claim, one whose tenant was removed, and one with volumes (capture refuses them) is - skipped; a replicated claim that gains volumes + skipped, and a skipped claim keeps its interval, so one that wakes past it is due at once; a replicated claim that gains volumes drops its row and replica. The claim's policy pool, not its own template key, picks the interval, so a claim cloned from a template promoted out of a replicating pool follows that pool. Rounds share the maintenance budget with diff --git a/sandboxd/pool/replica.go b/sandboxd/pool/replica.go index 4e5a8848..68ffb283 100644 --- a/sandboxd/pool/replica.go +++ b/sandboxd/pool/replica.go @@ -29,6 +29,9 @@ func (m *Manager) replicateOnce(ctx context.Context) { retire = append(retire, sb) } case !m.replicable(sb): + if !tried.IsZero() { + next[id] = tried + } case m.hasVolumeHolds(sb): if sb.HasReplica() { retire = append(retire, sb) diff --git a/sandboxd/pool/replica_test.go b/sandboxd/pool/replica_test.go index 7b46fb4a..7ccd1f71 100644 --- a/sandboxd/pool/replica_test.go +++ b/sandboxd/pool/replica_test.go @@ -67,6 +67,23 @@ func TestReplicaRoundPublishesMovesTheRowAndDropsThePrevious(t *testing.T) { } } +func TestReplicaIntervalSurvivesATickTheClaimCannotReplicate(t *testing.T) { + c := newReplicaCluster(t) + a := c.node(t, "a", t.TempDir(), keepAnHour) + sb := mustClaim(t, a, testKey) + last := time.Now().Add(-2 * replicateEvery * time.Second) + a.mu.Lock() + a.replicas = map[string]time.Time{sb.ID: last} + sb.ArchivePending = "ck_pending" + a.mu.Unlock() + a.replicateOnce(t.Context()) + a.mu.Lock() + defer a.mu.Unlock() + if got := a.replicas[sb.ID]; !got.Equal(last) { + t.Errorf("last attempt %v after a tick the claim could not replicate, want %v kept so a woken claim is due at once", got, last) + } +} + func TestReplicaRefusedPutDropsTheCopyAndItsExport(t *testing.T) { c := newReplicaCluster(t) a, b := c.node(t, "a", t.TempDir(), keepAnHour), c.node(t, "b", t.TempDir(), keepAnHour) From e7189e9e5df1dc074c09bb66dd59c43a7deb9345 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 18:47:09 +0800 Subject: [PATCH 10/17] cut: decode request bodies, config and MCP tool arguments with json/v2's exact names, dropping the v1 case-insensitive matching the v2 move kept A case-variant key is now an unknown member: config and strict bodies reject it, other bodies ignore it. --- mcp/tools.go | 2 +- sandboxd/config/config.go | 2 +- sandboxd/config/config_test.go | 1 - sandboxd/server/server_http.go | 4 ++-- sandboxd/server/server_test.go | 21 ++++----------------- sandboxd/utils/utils.go | 2 +- sandboxd/utils/utils_test.go | 2 +- 7 files changed, 10 insertions(+), 24 deletions(-) diff --git a/mcp/tools.go b/mcp/tools.go index 06aa820f..1b7a74cf 100644 --- a/mcp/tools.go +++ b/mcp/tools.go @@ -431,7 +431,7 @@ func parse(raw jsontext.Value, v any) error { } func unmarshalInput(raw []byte, v any) error { - return json.Unmarshal(raw, v, json.MatchCaseInsensitiveNames(true), jsontext.AllowInvalidUTF8(true)) + return json.Unmarshal(raw, v, jsontext.AllowInvalidUTF8(true)) } func jsonText(v any) string { diff --git a/sandboxd/config/config.go b/sandboxd/config/config.go index 3c0ae416..17fa0716 100644 --- a/sandboxd/config/config.go +++ b/sandboxd/config/config.go @@ -77,7 +77,7 @@ func (s *PoolSpec) UnmarshalJSONFrom(dec *jsontext.Decoder) error { var probe struct { Warm *int `json:"warm"` } - _ = json.Unmarshal(raw, &probe, json.MatchCaseInsensitiveNames(true)) + _ = json.Unmarshal(raw, &probe) s.warmSet = probe.Warm != nil return nil } diff --git a/sandboxd/config/config_test.go b/sandboxd/config/config_test.go index e7b3f5af..d8e3d970 100644 --- a/sandboxd/config/config_test.go +++ b/sandboxd/config/config_test.go @@ -419,7 +419,6 @@ func TestLoadDefaultsWarmOnlyWhenOmitted(t *testing.T) { {"omitted", `{"template":"rt:24.04","net":"none","size":"small"}`, defaultWarm}, {"explicit zero", `{"template":"rt:24.04","net":"none","size":"small","warm":0}`, 0}, {"explicit zero under warm_max", `{"template":"rt:24.04","net":"none","size":"small","warm":0,"warm_max":8}`, 0}, - {"explicit zero, other case", `{"template":"rt:24.04","net":"none","size":"small","WARM":0}`, 0}, {"explicit three", `{"template":"rt:24.04","net":"none","size":"small","warm":3}`, 3}, } for _, tt := range tests { diff --git a/sandboxd/server/server_http.go b/sandboxd/server/server_http.go index 94e3944f..4bfa0e11 100644 --- a/sandboxd/server/server_http.go +++ b/sandboxd/server/server_http.go @@ -109,7 +109,7 @@ func hijack(ctx context.Context, w http.ResponseWriter) (net.Conn, *bufio.Reader func decodeBody[T any](w http.ResponseWriter, r *http.Request) (T, bool) { var v T - if err := json.UnmarshalRead(http.MaxBytesReader(w, r.Body, maxBodyBytes), &v, json.MatchCaseInsensitiveNames(true)); err != nil { + if err := json.UnmarshalRead(http.MaxBytesReader(w, r.Body, maxBodyBytes), &v); err != nil { writeErr(w, http.StatusBadRequest, "invalid request body") return v, false } @@ -119,7 +119,7 @@ func decodeBody[T any](w http.ResponseWriter, r *http.Request) (T, bool) { // decodeOptionalBody reads an absent body as the zero value, for verbs whose fields all default. func decodeOptionalBody[T any](w http.ResponseWriter, r *http.Request) (T, bool) { var v T - err := json.UnmarshalDecode(jsontext.NewDecoder(http.MaxBytesReader(w, r.Body, maxBodyBytes)), &v, json.MatchCaseInsensitiveNames(true)) + err := json.UnmarshalDecode(jsontext.NewDecoder(http.MaxBytesReader(w, r.Body, maxBodyBytes)), &v) if err != nil && !errors.Is(err, io.EOF) { writeErr(w, http.StatusBadRequest, "invalid request body") return v, false diff --git a/sandboxd/server/server_test.go b/sandboxd/server/server_test.go index 281fb00d..25ad2040 100644 --- a/sandboxd/server/server_test.go +++ b/sandboxd/server/server_test.go @@ -168,29 +168,16 @@ func TestRenewWithoutABodyTakesTheNodeDefault(t *testing.T) { } } -func TestRenewMatchesBodyKeysCaseInsensitively(t *testing.T) { - var gotTTL time.Duration - mgr := &fakeManager{renew: func(_, _ string, ttl time.Duration) (time.Time, error) { - gotTTL = ttl - return time.Now(), nil - }} - ts := newTestServer(t, "", mgr, nil) - - if code := statusOf(t, http.MethodPost, ts.URL+"/v1/sandboxes/sb_1/renew", "tok", `{"TTL_Seconds":3600}`); code != http.StatusOK || gotTTL != time.Hour { - t.Errorf("status %d ttl %v, want 200 and 1h", code, gotTTL) - } -} - -func TestClaimMatchesBodyKeysCaseInsensitively(t *testing.T) { +func TestClaimTakesExactBodyKeys(t *testing.T) { var gotKey types.PoolKey mgr := &fakeManager{claim: func(_ context.Context, key types.PoolKey, _ time.Duration) (*types.Sandbox, error) { gotKey = key return &types.Sandbox{ID: "sb_1"}, nil }} ts := newTestServer(t, "", mgr, nil) - - if code := statusOf(t, http.MethodPost, ts.URL+"/v1/claim", "", `{"Template":"rt:24.04"}`); code != http.StatusOK || gotKey.Template != "rt:24.04" { - t.Errorf("status %d template %q, want 200 and rt:24.04", code, gotKey.Template) + statusOf(t, http.MethodPost, ts.URL+"/v1/claim", "", `{"Template":"rt:24.04"}`) + if gotKey.Template != "" { + t.Errorf("a case-variant key set template %q", gotKey.Template) } } diff --git a/sandboxd/utils/utils.go b/sandboxd/utils/utils.go index 8d4180f7..307b1e2e 100644 --- a/sandboxd/utils/utils.go +++ b/sandboxd/utils/utils.go @@ -127,7 +127,7 @@ func RemoveDirEntries(dir string, match func(name string) bool) error { // DecodeStrictJSON decodes one JSON value into v, refusing unknown fields, duplicates and trailing data. func DecodeStrictJSON(raw []byte, v any) error { - return json.Unmarshal(raw, v, json.RejectUnknownMembers(true), json.MatchCaseInsensitiveNames(true)) + return json.Unmarshal(raw, v, json.RejectUnknownMembers(true)) } // DigestJSON encodes v with sorted map keys, so equal values always digest equal. diff --git a/sandboxd/utils/utils_test.go b/sandboxd/utils/utils_test.go index 68e4f5b5..c29eeb70 100644 --- a/sandboxd/utils/utils_test.go +++ b/sandboxd/utils/utils_test.go @@ -16,7 +16,7 @@ func TestDecodeStrictJSON(t *testing.T) { {"case-folded duplicate", `{"methods":["GET"],"Methods":[]}`, false}, {"trailing whitespace ok", `{"methods":["GET"]} `, true}, {"trailing data", `{"methods":["GET"]} {"methods":[]}`, false}, - {"case-insensitive name", `{"Methods":["GET"]}`, true}, + {"case-variant name", `{"Methods":["GET"]}`, false}, {"invalid utf-8", "{\"methods\":[\"\xff\"]}", false}, {"null field", `{"methods":null}`, true}, } { From 12e0ae3aef12495f8129cf5713ed569ccc65dd6b Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 18:48:11 +0800 Subject: [PATCH 11/17] silkd: end a session command when its shell exits, not only on the shell's stdout EOF A background job started in the session holds the stdout pipe past the shell, so an exit, a set -e failure or an OOM kill of the shell left the command hung, its session's io lock held and the session never reaped. --- silkd/src/session.rs | 20 +++++++++++++------- silkd/tests/session_e2e.rs | 12 ++++++++++++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/silkd/src/session.rs b/silkd/src/session.rs index 3b824f7a..f6bb9e49 100644 --- a/silkd/src/session.rs +++ b/silkd/src/session.rs @@ -64,7 +64,7 @@ impl Table { io: tokio::sync::Mutex::new(Io { stdin, stdout, - _child: child, + child, cmd_buf: String::new(), acc: Vec::new(), frame: Vec::new(), @@ -188,7 +188,7 @@ impl Session { struct Io { stdin: ChildStdin, stdout: ChildStdout, - _child: Child, + child: Child, cmd_buf: String, acc: Vec, frame: Vec, @@ -217,17 +217,14 @@ impl Io { let keep = mb.len().saturating_sub(1); self.acc.clear(); loop { - self.acc.reserve(READ_CHUNK); - let n = self.stdout.read_buf(&mut self.acc).await?; - if n == 0 { + if self.read_more().await? == 0 { return Err(io::Error::new(io::ErrorKind::UnexpectedEof, "shell exited")); } if let Some(pos) = memchr::memmem::find(&self.acc, mb) { emit(&mut out, &mut self.frame, &self.acc[..pos]).await; self.acc.drain(..pos + mb.len()); while memchr::memchr(b'\n', &self.acc).is_none() && self.acc.len() < EXIT_TAIL_MAX { - self.acc.reserve(READ_CHUNK); - if self.stdout.read_buf(&mut self.acc).await? == 0 { + if self.read_more().await? == 0 { break; } } @@ -245,6 +242,15 @@ impl Io { } } } + + async fn read_more(&mut self) -> io::Result { + self.acc.reserve(READ_CHUNK); + tokio::select! { + biased; + n = self.stdout.read_buf(&mut self.acc) => n, + _ = self.child.wait() => Ok(0), + } + } } /// Periodically reaps idle sessions until the process exits. diff --git a/silkd/tests/session_e2e.rs b/silkd/tests/session_e2e.rs index 76280c0c..8ae5677d 100644 --- a/silkd/tests/session_e2e.rs +++ b/silkd/tests/session_e2e.rs @@ -313,3 +313,15 @@ async fn a_dead_session_command_leaves_a_recreated_session_alone() { "the recreated session died: {got:?}" ); } + +#[tokio::test] +async fn a_shell_that_exits_ends_its_command_while_a_job_holds_its_output() { + let state = Arc::new(State::new()); + let id = create(&state).await; + let bg = sh(&state, &id, &["sh", "-c", "sleep 30 &"]).await; + assert_eq!(type_of(bg.last().unwrap()), "exit", "{bg:?}"); + let frames = tokio::time::timeout(Duration::from_secs(5), sh(&state, &id, &["exit"])) + .await + .expect("exit hung while a background job held the session's stdout"); + assert_eq!(type_of(frames.last().unwrap()), "error", "{frames:?}"); +} From 251547fca6b7abdfe91619205fca5eaf6a96064c Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 18:49:05 +0800 Subject: [PATCH 12/17] silkd: open the pty master and slave close-on-exec so the shell and its jobs do not inherit them libc openpty leaves both fds inheritable; every pty shell held a spare master, so a job outliving the shell never saw its terminal hang up. --- silkd/src/sysutil.rs | 4 +++- silkd/tests/pty_e2e.rs | 24 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/silkd/src/sysutil.rs b/silkd/src/sysutil.rs index a8e1097c..ac251625 100644 --- a/silkd/src/sysutil.rs +++ b/silkd/src/sysutil.rs @@ -10,7 +10,7 @@ use std::process::ExitStatus; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::{LazyLock, Mutex}; -use nix::fcntl::{FcntlArg, OFlag, fcntl}; +use nix::fcntl::{FcntlArg, FdFlag, OFlag, fcntl}; use tokio::io::unix::AsyncFd; use tokio::process::Command; @@ -150,6 +150,8 @@ pub fn openpty(cols: u16, rows: u16) -> std::io::Result<(OwnedFd, OwnedFd)> { ws_ypixel: 0, }; let pty = nix::pty::openpty(&ws, None)?; + fcntl(&pty.master, FcntlArg::F_SETFD(FdFlag::FD_CLOEXEC))?; + fcntl(&pty.slave, FcntlArg::F_SETFD(FdFlag::FD_CLOEXEC))?; let flags = OFlag::from_bits_retain(fcntl(&pty.master, FcntlArg::F_GETFL)?); fcntl(&pty.master, FcntlArg::F_SETFL(flags | OFlag::O_NONBLOCK))?; Ok((pty.master, pty.slave)) diff --git a/silkd/tests/pty_e2e.rs b/silkd/tests/pty_e2e.rs index bba55342..723a66f9 100644 --- a/silkd/tests/pty_e2e.rs +++ b/silkd/tests/pty_e2e.rs @@ -86,6 +86,30 @@ async fn pty_runs_a_shell_and_echoes() { handle.await.unwrap().unwrap(); } +#[cfg(target_os = "linux")] +#[tokio::test] +async fn the_pty_shell_holds_no_master_fd() { + let state = Arc::new(State::new()); + let (mut cw, mut lines, handle) = connect(&state); + send(&mut cw, json!({"op":"pty_open","cols":80,"rows":24})).await; + read_until(&mut lines, |v| v["type"] == "started").await; + send( + &mut cw, + json!({"op":"stdin","data":b64(b"echo silk-$(ls -l /proc/$$/fd | grep -c ptmx)-masters\n")}), + ) + .await; + let mut out = String::new(); + while !(0..10).any(|n| out.contains(&format!("silk-{n}-masters"))) { + let frame = read_until(&mut lines, |v| v["type"] == "stdout").await; + out.push_str(&String::from_utf8_lossy(&decode(&frame))); + } + assert!(out.contains("silk-0-masters"), "{out}"); + send(&mut cw, json!({"op":"stdin","data":b64(b"exit\n")})).await; + read_until(&mut lines, |v| v["type"] == "exit").await; + cw.shutdown().await.unwrap(); + handle.await.unwrap().unwrap(); +} + #[tokio::test] async fn pty_appears_in_ps_and_resizes() { let state = Arc::new(State::new()); From b1d2a5f3d3c81c1b33eff99f5b4197b3409461de Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 18:50:10 +0800 Subject: [PATCH 13/17] docs: match the docs and godocs to the code after #457 and #459-#467 Drops the Python mount row and the pre-proto-2 sentences #457 left, the images trigger path, and the old-template digest note; corrects the data_dir entries, the drain error, the shared-store probe, idle upstream conns, env snapshot reads, lineage markers, secret valuation, exit codes, Ready, the tenant_claims help, Metas, the checkpoint fallback and the Python lookup and keep_alive docs. --- README.md | 2 +- docs/deploy.md | 2 +- docs/egress.md | 11 ++++++----- docs/sandboxd-api.md | 14 ++++++++------ docs/sdk-python.md | 8 ++------ docs/sdk.md | 5 +---- protocol/wire/frame.go | 6 +++--- sandboxd/egress/secrets.go | 2 +- sandboxd/pool/pool.go | 2 +- sandboxd/server/metrics.go | 2 +- sandboxd/store/store.go | 2 +- sdk/go/checkpoint.go | 2 +- sdk/python/cocoonsandbox/client.py | 4 ++-- sdk/python/cocoonsandbox/sandbox.py | 2 +- 14 files changed, 30 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index 3f431e5b..52c096ab 100644 --- a/README.md +++ b/README.md @@ -108,7 +108,7 @@ sandboxd](docs/deploy.md) covers release binaries, the full config and systemd. base SDK - `shell.yml` — shellcheck over every tracked shell script - `images.yml` — the single image entry point: on a push touching - `boot/**`, `silkd/**`, `protocol/**`, or `os-image/**` it builds the + `boot/**`, `silkd/**`, or `os-image/**` it builds the changed carriers (via `build-boot.yml` / `build-silkd.yml`, `workflow_call`) then os-images, in order, so the chain is deterministic - `build-os-images.yml` — bakes base + flavors FROM the sha-pinned carriers diff --git a/docs/deploy.md b/docs/deploy.md index 7c5b6996..392e805e 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -425,7 +425,7 @@ generation sweep. | `/labels.json` | Optional labels, removed with the record. | | `manifests//.json` | One JSON manifest per published generation: `lineage` and, per file, `path`, `size` and the present `chunks` (`index`, `sha256`). `` is `export-<16 hex>`. | | `chunks//` | A 16 MiB chunk. A lineage is the dedup scope: it defaults to the record id, and every checkpoint and archive of one claim shares the claim's sandbox id. A chunk is shared by every record of its lineage that names the same digest, and only by those. | -| `lineages//` | Zero-byte marker: record `` published into ``, written after the manifest and before `meta.json`. A publish into a shared lineage reads only the records these list, so a crash between manifest and marker only costs a missed dedup. It only drives dedup, never chunk liveness. | +| `lineages//` | Zero-byte marker: record `` published into another record's lineage ``, written after the manifest and before `meta.json`; a record in its own lineage writes none. A publish into a shared lineage reads only the records these list, so a crash between manifest and marker only costs a missed dedup. It only drives dedup, never chunk liveness. | **Offline reclamation only.** Stop every sandboxd node using the prefix and wait for all in-flight publishes, deletes and fetches to finish. Keep the diff --git a/docs/egress.md b/docs/egress.md index 2cbf2be7..704a6138 100644 --- a/docs/egress.md +++ b/docs/egress.md @@ -45,8 +45,9 @@ supported*), no authentication (the socket path already carries the sandbox's identity), and a `DOMAINNAME` destination is resolved host-side, so the guest still needs no resolver. Each door serves at most 256 connections per sandbox at a time; later ones sit unserved in the socket backlog until one closes, and -the proxy keeps at most 64 idle upstream connections per sandbox — 64 more for -decrypted traffic when the pool intercepts — so a guest cannot turn its proxy +the proxy keeps at most 64 idle upstream connections per sandbox and exit (the +node's own and each upstream proxy the claim has used), as many again for +decrypted traffic when the pool intercepts, so a guest cannot turn its proxy into a host descriptor sink. A SOCKS5 tunnel takes the decision an HTTP `CONNECT` to the same host takes, @@ -310,9 +311,9 @@ gateway key per user, while the guest holds a placeholder. never logged, listed or returned. The proxy reads a snapshot of the claim's env that each env write publishes, -without a lock, once per request whose matched rule names a secret or whose -host has a claim credential, and once per `CONNECT` an `"inject"` rule covers; -other requests pay nothing. +without a lock, once per intercepted request, once per request whose matched +rule names a secret, and once per `CONNECT` an `"inject"` rule covers; other +requests pay nothing. ### Claim credentials diff --git a/docs/sandboxd-api.md b/docs/sandboxd-api.md index daee9f94..5e0afcec 100644 --- a/docs/sandboxd-api.md +++ b/docs/sandboxd-api.md @@ -320,7 +320,7 @@ Malformed or conflicting metadata, including a saved path outside `volume_roots` prevents startup. The catalog is node-local with one sandboxd writer. Sandboxd does not create, check, or delete backings. Keep them outside -`data_dir`: sandboxd clears its `readers`, `goldens`, `fork-*`, +`data_dir`: sandboxd clears its `readers`, `goldens`, `moves`, `fork-*`, `checkpoint-staging` and `template-staging` entries. ### Volume access lists @@ -804,7 +804,7 @@ A refused reload changes nothing. ## POST /v1/drain Auth: root only (tenant tokens get 403). Cordons the node for maintenance: claim/fork/branch answer -429 `node draining` (on a cluster a non-volume claim tries a peer redirect first, and +429 with an error ending `node draining` (on a cluster a non-volume claim tries a peer redirect first, and peers stop redirecting here within a tick, because this node gossips no pools), unclaimed warm VMs are destroyed, and live claims keep serving until release or TTL. Pool ownership is untouched — no pools.json write, no config change. @@ -924,8 +924,9 @@ never the source sandbox's. 400 metadata over a bound, an unknown `on_expire`, or a bad `env` entry. Checkpoints are node-local (unless the store is shared — see -[Configuration](deploy.md#configuration)). On a miss the node probes its peers -and answers with a redirect shaped like a warm-miss +[Configuration](deploy.md#configuration)). On a miss a node with a node-local +store probes its peers (a shared s3 store skips the probe: every node already +resolves every record) and answers with a redirect shaped like a warm-miss [`POST /v1/claim`](#post-v1claim), at most 3 addresses; retry the same body (+`no_redirect: true`) at each candidate. With nothing to redirect to (or `no_redirect` set) and `checkpoint_peer_heal` on, it pulls the record and serves @@ -1139,8 +1140,9 @@ them in `secret` (`gh,claim:`, never a value). An egress event routed thr [upstream proxy](egress.md#upstream-proxies) carries `upstream` (its `host:port`), and with `egress_usage_bytes` on, `egress_bytes` events add the connection's payload `tx`/`rx` bytes. A sandbox records at most 10 denied or rate-limited egress decisions per second after a burst of 100, so `egress` events do not count every refusal. A volume claim also carries `volumes`, the applied catalog names, and — omitted when empty — `volumes_rw`, -the subset of those names claimed `rw`, so billing can discriminate write -access (mounts and host paths are not billing dimensions). The file rotates at +the subset of those names claimed `rw`, on its `claim` event, so billing can +discriminate write access at claim (mounts and host paths are not billing +dimensions); a live attach or detach records no usage event. The file rotates at 64 MiB keeping one `.1` backup, so a tailing collector never loses a window silently. A last line a crash cut short is ended with a newline when the node reopens the journal, so it never swallows the next event. A [live move](#post-v1sandboxesidmove) diff --git a/docs/sdk-python.md b/docs/sdk-python.md index 0b3f06ca..48e51da2 100644 --- a/docs/sdk-python.md +++ b/docs/sdk-python.md @@ -90,7 +90,7 @@ directly. To recover a handle when only `id` + `token` survived (say, across a process restart): ```python -sb = client.lookup(id, token) # probes the entry node and every mesh peer concurrently +sb = client.lookup(id, token) # lists the peers, then probes the entry node and every peer concurrently ``` ## Claiming @@ -107,7 +107,6 @@ sb = client.new("ghcr.io/cocoonstack/sandbox/rt:24.04", | `net` | `"none"`, `"egress"` | `"none"` | Cloud Hypervisor network shape: `none` disables the NIC and uses vsock-only I/O; `egress` attaches a bridge/CNI NIC | | `size` | `"small"`, `"medium"`, `"large"`, `"xlarge"`, `"2xlarge"` | `"small"` | resource tier: 1cpu/512M, 2cpu/1G, 4cpu/4G, 4cpu/8G, 8cpu/16G | | `volumes` | bare names or `{name, mount?, mode?, attach_only?}` mappings | `None` | attach and mount up to eight unique catalog dataset disks; an omitted mount defaults to `/volumes/`; `mode` is `"ro"` (default) or `"rw"` — `"rw"` requires the catalog entry's `writable: true`; accepted by `Client.new` and `Template.new` | -| `mount` | bool | `True` | mount every requested volume. `False` attaches the devices and leaves the mounting to the workload; sandboxd then rejects a mapping carrying `mount` with 400 | | `ttl_seconds` | int | server default 5m | sandbox TTL, server-capped at 24h. The node reaps the sandbox after the TTL even if the client vanishes | | `deadline` | `time.monotonic()` value | `None` | keyword-only wall clock over the whole claim, redirect targets included: every request is bounded by whatever is left and a spent deadline raises `TimeoutError`. Without it each redirect candidate gets a fresh `timeout`, so a cluster with unreachable peers can cost a multiple of it. Also on `Template.new` and `Checkpoint.new` | @@ -238,8 +237,7 @@ handle parks at most 8 idle connections. A kept connection counts as live for `idle_hibernate_seconds` until it closes, so keep the window below that setting; `close` and `hibernate` drop it at once. Streams (`watch`, `open_pty`, `dial_port`, an LSP session) take a connection -of their own, and a guest whose silkd predates the back-to-back protocol -gets one connection per call as before. +of their own. The handle's `sb.deadline` remains the value returned when it was created or last renewed, so after an archive/wake transition read the current one from @@ -281,8 +279,6 @@ tick — prefer the handle right after promoting (see `tpl.content_digest` identifies the published export bytes. A caller pinning the mutable name can compare a claim's `template_digest` with its expected value and close/refuse a mismatch. -Templates published by an older sandboxd have empty digests until they are -re-promoted after the node is upgraded. ## Checkpoints — branching and time travel diff --git a/docs/sdk.md b/docs/sdk.md index e09edf68..5e178e29 100644 --- a/docs/sdk.md +++ b/docs/sdk.md @@ -166,8 +166,7 @@ at most 8 idle connections. A kept connection counts as live for setting; `Close` and `Hibernate` drop it at once. Long-lived streams (`Watch`, `OpenPty`, `DialPort`, an LSP session) take a connection of their own. Reuse needs a Unix client (the liveness peek is a `MSG_PEEK`); on -Windows, and against a guest whose silkd predates the back-to-back protocol, -every call dials as before. +Windows every call dials as before. An explicit scheme in an owner, redirect, peer, or `Attach` address wins; a bare address inherits the entry client's scheme. Trust settings are shared @@ -425,8 +424,6 @@ replaces the template. `Template.ContentDigest` identifies the published export bytes; a claim from that exact generation carries the same value in `Sandbox.TemplateDigest`. A caller pinning a mutable template name can compare the claim's value with its expected digest and close/refuse a mismatch. -Templates published by an older sandboxd have empty digests until they are -re-promoted after the node is upgraded. **On the default local-disk backend templates live on one node**, and on a cluster the parent claim may have been redirected — the returned `Template` diff --git a/protocol/wire/frame.go b/protocol/wire/frame.go index 7d12dc0b..02710366 100644 --- a/protocol/wire/frame.go +++ b/protocol/wire/frame.go @@ -447,7 +447,7 @@ type DataEnd struct{} func (DataEnd) Op() string { return "data_end" } -// Started reports the spawned pid (synthetic when the OS pid is unknown). +// Started reports the spawned pid. type Started struct { PID uint32 `json:"pid"` } @@ -468,7 +468,7 @@ type Stderr struct { func (Stderr) RespType() string { return "stderr" } -// Exit is the terminal frame of a foreground exec; -1 means killed or unknown. +// Exit is the terminal frame of a foreground exec; a signaled child reports 128+signal, -1 an unknown status. type Exit struct { Code int32 `json:"code"` } @@ -480,7 +480,7 @@ type Done struct{} func (Done) RespType() string { return "done" } -// Ready acknowledges an armed watch (events after it are guaranteed captured) or a connected port_forward. +// Ready acknowledges an armed watch (events after it are guaranteed captured), a connected port_forward or an attached lsp_request. type Ready struct{} func (Ready) RespType() string { return "ready" } diff --git a/sandboxd/egress/secrets.go b/sandboxd/egress/secrets.go index 89773482..06fc3a46 100644 --- a/sandboxd/egress/secrets.go +++ b/sandboxd/egress/secrets.go @@ -14,7 +14,7 @@ import ( // nonInjectable lists the secret headers the transport owns or strips. var nonInjectable = slices.Concat([]string{"Host", "Content-Length"}, hopHeaders) -// SecretSpec declares a credential the proxy injects, valued from the node env ValueEnv names, or from the claim env Name when ValueEnv is empty. +// SecretSpec declares a credential the proxy injects, valued from the env ValueEnv names (a claim's host-only entry first, then the node's), or from the claim env Name when ValueEnv is empty. type SecretSpec struct { Name string `json:"name"` Header string `json:"header"` diff --git a/sandboxd/pool/pool.go b/sandboxd/pool/pool.go index 28cfb35a..fa8ac459 100644 --- a/sandboxd/pool/pool.go +++ b/sandboxd/pool/pool.go @@ -45,7 +45,7 @@ const ( coldProbeTimeout = 90 * time.Second // A clone answers in ~1s even saturated, so a silent one is replaced, not waited on warmProbeTimeout = 5 * time.Second - // One list; a wrong answer only costs an extra sweep. + // One inspect; a wrong answer only costs an extra sweep. removeVerifyTimeout = 15 * time.Second cellRetry = 5 * time.Second // A full node clears only when VMs go away; retrying sooner buys nothing. diff --git a/sandboxd/server/metrics.go b/sandboxd/server/metrics.go index 1a75b0d7..1897b036 100644 --- a/sandboxd/server/metrics.go +++ b/sandboxd/server/metrics.go @@ -42,7 +42,7 @@ func (s *Server) handleMetrics(w http.ResponseWriter, _ *http.Request) { scalar("anon_bytes", "node anon memory, 0 when memory_limit has no anon limit", g.AnonBytes) if tenants := s.mgr.TenantClaims(); len(tenants) > 0 { - metric("tenant_claims", "gauge", "live claims per configured tenant") + metric("tenant_claims", "gauge", "live claims per tenant holding any") for _, name := range slices.Sorted(maps.Keys(tenants)) { _, _ = fmt.Fprintf(w, "sandboxd_tenant_claims{tenant=%q} %d\n", name, tenants[name]) } diff --git a/sandboxd/store/store.go b/sandboxd/store/store.go index f75501fd..6a0b794c 100644 --- a/sandboxd/store/store.go +++ b/sandboxd/store/store.go @@ -71,7 +71,7 @@ type Store interface { ReadMeta(ctx context.Context, id string) ([]byte, error) // ReadDigest returns a record's export digest, empty when it was published undigested. ReadDigest(ctx context.Context, id string) (string, error) - // Metas lists the metadata and digest of every record in this instance's id namespace. + // Metas lists every record of this instance's id namespace, with labels where the store reads them. Metas(ctx context.Context) ([]Record, error) // SetLabels replaces a record's labels; nil removes them. The caller holds the record lock. SetLabels(ctx context.Context, id string, labels []byte) error diff --git a/sdk/go/checkpoint.go b/sdk/go/checkpoint.go index 57d3874f..877f942f 100644 --- a/sdk/go/checkpoint.go +++ b/sdk/go/checkpoint.go @@ -53,7 +53,7 @@ type Checkpoint struct { // New claims a sandbox cloned from the checkpoint on the checkpoint's node. // The snapshot pins the key axes; WithTimeout, WithMetadata, // WithArchiveOnExpire and WithEnv apply. A node that no longer holds the -// checkpoint redirects to a probed owner, which New follows; if every +// checkpoint redirects to a probed owner, which New follows; if the last // candidate fails transiently, New falls back to the origin once to heal there. func (ck *Checkpoint) New(ctx context.Context, opts ...Option) (*Sandbox, error) { var claim claimRequest diff --git a/sdk/python/cocoonsandbox/client.py b/sdk/python/cocoonsandbox/client.py index 6c1c4881..e65fa037 100644 --- a/sdk/python/cocoonsandbox/client.py +++ b/sdk/python/cocoonsandbox/client.py @@ -38,7 +38,7 @@ def __init__( ssl_context: ssl.SSLContext | None = None, keep_alive: float = 30.0, ) -> None: - """keep_alive keeps a handle's idle relay connection, which holds the sandbox's idle clock; 0 dials per call. + """keep_alive parks idle control and relay connections, which hold the sandbox's idle clock; 0 dials per call. ssl_context is set to offer ALPN http/1.1, which the relay upgrade needs; pass a dedicated context. """ @@ -82,7 +82,7 @@ def delete_template(self, template: str, net: str = "", size: str = "") -> None: _try_each(candidates, lambda peer: self._request(peer, "DELETE", path, None, "delete template")) def lookup(self, id: str, token: str) -> Sandbox: - """Finds the owner by probing the entry and peers concurrently.""" + """Finds the owner by listing the peers, then probing the entry and every peer concurrently.""" def probe(addr: str) -> Sandbox: reply = self._request( diff --git a/sdk/python/cocoonsandbox/sandbox.py b/sdk/python/cocoonsandbox/sandbox.py index 3e7d8c60..f1a0a142 100644 --- a/sdk/python/cocoonsandbox/sandbox.py +++ b/sdk/python/cocoonsandbox/sandbox.py @@ -600,7 +600,7 @@ def close(self) -> None: class Watcher(_Closeable): - """A live filesystem event stream; iterate for {kind, path} events.""" + """A live filesystem event stream; iterate for {type, kind, path} events.""" def __init__(self, conn: Conn) -> None: self._conn = conn From d3d444f1670d3cf6d9f8ca10a7b976ae330a1c55 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 18:50:42 +0800 Subject: [PATCH 14/17] review: delete restating comments and fold over-long ones to the one fact the code cannot carry --- e2e/cmd/smoke/main.go | 2 +- sandboxd/config/config.go | 3 +-- sandboxd/egress/proxy.go | 1 - sandboxd/engine/engine.go | 4 ++-- sandboxd/outbound/outbound.go | 1 - sandboxd/pool/checkpoint.go | 2 -- sandboxd/pool/operator.go | 1 - sandboxd/pool/stats.go | 1 - sandboxd/pool/view_test.go | 1 - sandboxd/server/preview.go | 2 +- sandboxd/store/peer/peer_test.go | 2 +- sandboxd/store/peer/probe.go | 6 +++--- sdk/openai/cocoonsandbox_openai/adapter.py | 1 - sdk/python/cocoonsandbox/frames.py | 2 +- silkd/src/git.rs | 2 -- silkd/src/proc.rs | 2 +- 16 files changed, 11 insertions(+), 22 deletions(-) diff --git a/e2e/cmd/smoke/main.go b/e2e/cmd/smoke/main.go index cd2d4843..2bdf3002 100644 --- a/e2e/cmd/smoke/main.go +++ b/e2e/cmd/smoke/main.go @@ -319,7 +319,7 @@ func smokeHibernate(ctx context.Context, client *sandbox.Client, sb *sandbox.San if err = wantHibernated(ctx, client, 1); err != nil { return err } - out, err := sess.Exec(ctx, "sh", "-c", "echo $HIBMARK") // first guest access wakes the VM + out, err := sess.Exec(ctx, "sh", "-c", "echo $HIBMARK") if err != nil { return fmt.Errorf("exec after hibernate: %w", err) } diff --git a/sandboxd/config/config.go b/sandboxd/config/config.go index 17fa0716..eb0e1ba9 100644 --- a/sandboxd/config/config.go +++ b/sandboxd/config/config.go @@ -34,8 +34,7 @@ const ( defaultWarm = 4 refillFloor = 4 refillCeiling = 256 - // cocoon builds a VM with a 10G disk by default, so a pool never asks for less - minStorageBytes = 10 << 30 + minStorageBytes = 10 << 30 ) // PoolSpec declares one warm pool and its target of claim-ready VMs. diff --git a/sandboxd/egress/proxy.go b/sandboxd/egress/proxy.go index b89183c5..928cc859 100644 --- a/sandboxd/egress/proxy.go +++ b/sandboxd/egress/proxy.go @@ -149,7 +149,6 @@ type Proxy struct { pools atomic.Pointer[connPools] refusals *rate.Limiter - // leaves caches this sandbox's interception leaves; per-proxy, never shared. leafMu sync.Mutex leaves map[string]*tls.Certificate // tickets lets a guest resume an intercepted TLS session on its next CONNECT. diff --git a/sandboxd/engine/engine.go b/sandboxd/engine/engine.go index fd604208..8e71355a 100644 --- a/sandboxd/engine/engine.go +++ b/sandboxd/engine/engine.go @@ -47,8 +47,8 @@ const ( argNetwork = "--network" argBridge = "--bridge" formatJSON = "json" - silkdPort = 2048 // silkd's fixed guest vsock port, the claim-ready anchor - egressPort = 2049 // guest→host egress port; VMM maps it to _2049 + silkdPort = 2048 + egressPort = 2049 socksPort = 2050 cmdTimeout = 2 * time.Minute dumpTimeout = 15 * time.Minute diff --git a/sandboxd/outbound/outbound.go b/sandboxd/outbound/outbound.go index 11519287..4c5484f9 100644 --- a/sandboxd/outbound/outbound.go +++ b/sandboxd/outbound/outbound.go @@ -113,7 +113,6 @@ func (h *Host) ArmProxy(ctx context.Context, sb *types.Sandbox) error { } id, tenant := sb.ID, sb.Tenant evCtx := context.WithoutCancel(ctx) - // Only an intercepting pool pays for the TLS transport and leaf cache ca := h.o.CA if !v.Intercepts(sb.PolicyKey()) { ca = nil diff --git a/sandboxd/pool/checkpoint.go b/sandboxd/pool/checkpoint.go index 374cd164..ce743805 100644 --- a/sandboxd/pool/checkpoint.go +++ b/sandboxd/pool/checkpoint.go @@ -354,14 +354,12 @@ type healVetoes struct { pending, aborted map[string]struct{} } -// mark records ckptID as staging or pulling, so a delete can veto it. func (h *healVetoes) mark(ckptID string) { h.mu.Lock() defer h.mu.Unlock() h.pending[ckptID] = struct{}{} } -// clear un-marks ckptID and reports a veto. func (h *healVetoes) clear(ckptID string) (aborted bool) { h.mu.Lock() defer h.mu.Unlock() diff --git a/sandboxd/pool/operator.go b/sandboxd/pool/operator.go index 746fae19..407b8681 100644 --- a/sandboxd/pool/operator.go +++ b/sandboxd/pool/operator.go @@ -64,7 +64,6 @@ func (m *Manager) SetInstanceMetadata(ctx context.Context, id string, doc []byte return m.eng.WriteInstanceMetadata(ctx, sb.VsockSocket, doc) } -// resolve authorizes id under cred. func (m *Manager) resolve(id string, cred Cred) (*types.Sandbox, bool) { if cred.Operator { return m.byID(id) diff --git a/sandboxd/pool/stats.go b/sandboxd/pool/stats.go index 20f781a1..1b41ee2f 100644 --- a/sandboxd/pool/stats.go +++ b/sandboxd/pool/stats.go @@ -59,7 +59,6 @@ func (m *Manager) vmResidentBytes(ctx context.Context, vmName string) (int64, bo return residentBytes(vm.PID) } -// residentBytes reads a process's resident page count from statm's second field. func residentBytes(pid int) (int64, bool) { b, err := os.ReadFile(filepath.Join("/proc", strconv.Itoa(pid), "statm")) if err != nil { diff --git a/sandboxd/pool/view_test.go b/sandboxd/pool/view_test.go index d97a5101..a08c3519 100644 --- a/sandboxd/pool/view_test.go +++ b/sandboxd/pool/view_test.go @@ -21,7 +21,6 @@ func editView(m *Manager, edit func(v *configView)) { m.view.Store(&next) } -// editEgress builds the view without secrets. func editEgress(t *testing.T, m *Manager, edit func(cfg *config.Config)) { t.Helper() cfg := *m.cfg diff --git a/sandboxd/server/preview.go b/sandboxd/server/preview.go index 7ca909ea..138b1950 100644 --- a/sandboxd/server/preview.go +++ b/sandboxd/server/preview.go @@ -20,7 +20,7 @@ import ( "github.com/cocoonstack/sandbox/sandboxd/pool" ) -// forwardedHeader marks a request one owner already forwarded; a token whose owner string no longer names this node must not bounce forever. +// forwardedHeader stops a forward loop when a token's owner no longer names this node. const forwardedHeader = "X-Sandbox-Preview-Forwarded" type previewClaims struct { diff --git a/sandboxd/store/peer/peer_test.go b/sandboxd/store/peer/peer_test.go index 6e2b15b8..2722f7e2 100644 --- a/sandboxd/store/peer/peer_test.go +++ b/sandboxd/store/peer/peer_test.go @@ -97,7 +97,7 @@ func TestPullDoesNotDedupConcurrentCalls(t *testing.T) { } } if len(puller.asked) != 2 { - t.Errorf("puller called %d times, want 2: Pull must not dedup — that's the caller's job now", len(puller.asked)) + t.Errorf("puller called %d times, want 2 (Pull does not dedup)", len(puller.asked)) } for _, dst := range []string{dst1, dst2} { if got, err := os.ReadFile(filepath.Join(dst, store.ExportDir, "mem")); err != nil || string(got) != "guest-pages" { diff --git a/sandboxd/store/peer/probe.go b/sandboxd/store/peer/probe.go index 89f2bd4a..1aa94c26 100644 --- a/sandboxd/store/peer/probe.go +++ b/sandboxd/store/peer/probe.go @@ -19,8 +19,8 @@ import ( const ( probeTimeout = time.Second fanOutTimeout = 2 * time.Second - maxRedirectOwners = 3 // a redirect decision needs a couple of candidates, not the whole fleet - maxHealOwners = 8 // a heal wants the widest source list a few full/corrupt owners can't hide behind + maxRedirectOwners = 3 + maxHealOwners = 8 redirectCacheTTL = 5 * time.Second // probeGrace bounds how long a fan-out waits for more owners once it has its first. @@ -50,7 +50,7 @@ type HTTPProber struct { cacheMu sync.Mutex cache map[string]redirectCacheEntry - // cacheTTL overrides redirectCacheTTL; a test seam, 5 seconds is slow to wait out. + // cacheTTL overrides redirectCacheTTL in tests. cacheTTL time.Duration } diff --git a/sdk/openai/cocoonsandbox_openai/adapter.py b/sdk/openai/cocoonsandbox_openai/adapter.py index a3f50ef9..817e9618 100644 --- a/sdk/openai/cocoonsandbox_openai/adapter.py +++ b/sdk/openai/cocoonsandbox_openai/adapter.py @@ -101,7 +101,6 @@ async def _exec_internal(self, *command: str | Path, timeout: float | None = Non stdout, stderr = bytearray(), bytearray() def run() -> int: - # the SDK's wall clock cuts the connection, kills the reported pid, and raises TimeoutError return sb.run(argv, on_stdout=stdout.extend, on_stderr=stderr.extend, timeout=timeout) code = await asyncio.to_thread(run) diff --git a/sdk/python/cocoonsandbox/frames.py b/sdk/python/cocoonsandbox/frames.py index 681d2f96..d69f7a9d 100644 --- a/sdk/python/cocoonsandbox/frames.py +++ b/sdk/python/cocoonsandbox/frames.py @@ -20,7 +20,7 @@ def encode_request(op: str, **fields: object) -> bytes: """Encodes one request, omitting None fields and base64-encoding byte values.""" if len(fields) == 1 and isinstance(data := fields.get("data"), (bytes, bytearray, memoryview)): - # base64 needs no JSON escaping, so a bare data frame renders without a str round trip + # base64 needs no JSON escaping return b'{"v":%d,"op":"%s","data":"%s"}\n' % (PROTO_VERSION, op.encode(), base64.b64encode(data)) frame = {"v": PROTO_VERSION, "op": op} for key, value in fields.items(): diff --git a/silkd/src/git.rs b/silkd/src/git.rs index 7a422ff0..0f6e0e77 100644 --- a/silkd/src/git.rs +++ b/silkd/src/git.rs @@ -164,7 +164,6 @@ async fn git( git_cmd(dir, auth).args(args).output().await } -/// Injects git config as GIT_CONFIG_COUNT/KEY_n/VALUE_n env pairs. fn apply_config(cmd: &mut Command, auth: Option<&str>) { cmd.env("GIT_CONFIG_COUNT", if auth.is_some() { "2" } else { "1" }) .env("GIT_CONFIG_KEY_0", "core.quotePath") @@ -177,7 +176,6 @@ fn apply_config(cmd: &mut Command, auth: Option<&str>) { } } -/// Writes Done on success, else the git stderr as an error frame. async fn terminal( w: &mut W, verb: &str, diff --git a/silkd/src/proc.rs b/silkd/src/proc.rs index 973f5e6d..770b20a6 100644 --- a/silkd/src/proc.rs +++ b/silkd/src/proc.rs @@ -129,7 +129,7 @@ impl Proc { self.emit(&Chunk::Exit(code)); } - /// Records a pty master fd (a dup, owned here) so `resize` can reach it. + /// Records a pty master fd so `resize` can reach it. pub fn set_pty_master(&self, fd: OwnedFd) { *sysutil::lock(&self.pty_master) = Some(fd); } From 97da59d126efaea722a0af9e08d6c675dd5042d4 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 18:51:26 +0800 Subject: [PATCH 15/17] cut: inline single-caller helpers in the reader router, SDK and silkdtest, drop an unreachable preview guard and lint exclusions for directories that do not exist --- .golangci.yml | 2 -- sandboxd/server/preview.go | 5 +---- sandboxd/server/reader.go | 25 +++++-------------------- sdk/go/client.go | 13 +++++-------- sdk/go/silkd/silkdtest/silkdtest.go | 9 +-------- 5 files changed, 12 insertions(+), 42 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 57085644..9ffd6d5f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -59,8 +59,6 @@ linters: exclusions: generated: strict paths: - - vendor - - mocks - dist rules: - path: _test\.go diff --git a/sandboxd/server/preview.go b/sandboxd/server/preview.go index 138b1950..683dda87 100644 --- a/sandboxd/server/preview.go +++ b/sandboxd/server/preview.go @@ -45,11 +45,8 @@ type PreviewServer struct { transport *http.Transport } -// NewPreviewServer returns nil when preview is not configured. +// NewPreviewServer serves preview URLs signed with secret and minted under base. func NewPreviewServer(secret, base, owner string, mgr PreviewManager) *PreviewServer { - if secret == "" { - return nil - } p := &PreviewServer{secret: []byte(secret), base: base, owner: owner, mgr: mgr} p.transport = &http.Transport{ // A pooled guest conn skips PreviewDial, and with it wakeResolved and the Transition lock diff --git a/sandboxd/server/reader.go b/sandboxd/server/reader.go index bf8eab2f..c6abe8ca 100644 --- a/sandboxd/server/reader.go +++ b/sandboxd/server/reader.go @@ -91,12 +91,15 @@ func (r *router) toReader(head []byte, partial bool) error { r.closeReader() } if r.reader == nil { - if err := r.dialReader(); err != nil { + conn, source, err := r.s.mgr.DialReader(r.ctx, r.id, r.token) + if err != nil { return err } + r.reader, r.source = conn, source + r.startPump() } if r.s.mgr.AuditEnabled() { - r.audit(head, partial) + r.s.mgr.Audit(r.ctx, r.id, bytes.TrimSuffix(head, newline)) } if err := r.pass(head, partial, r.reader); err != nil { return errReaderGone @@ -104,16 +107,6 @@ func (r *router) toReader(head []byte, partial bool) error { return nil } -func (r *router) dialReader() error { - conn, source, err := r.s.mgr.DialReader(r.ctx, r.id, r.token) - if err != nil { - return err - } - r.reader, r.source = conn, source - r.startPump() - return nil -} - func (r *router) handover(head []byte) (net.Conn, func(), bool) { r.closeReader() guest, done, err := r.s.openGuest(r.ctx, r.id, r.token, r.s.mgr.WakeAgentSocket) @@ -196,14 +189,6 @@ func (r *router) fail(err error) { _, _ = r.client.Write(append(frame, '\n')) } -func (r *router) audit(head []byte, partial bool) { - if partial { - r.s.mgr.Audit(r.ctx, r.id, head) - return - } - r.s.mgr.Audit(r.ctx, r.id, bytes.TrimSuffix(head, newline)) -} - // serveSleeping answers the 101 before reading any frame, since a client's first frame can wait on it. func (s *Server) serveSleeping(ctx context.Context, w http.ResponseWriter, id, token string) { client, clientBuf, ok := hijack(ctx, w) diff --git a/sdk/go/client.go b/sdk/go/client.go index 453aa1b0..816672f7 100644 --- a/sdk/go/client.go +++ b/sdk/go/client.go @@ -265,15 +265,12 @@ func (c *Client) deleteTemplates(ctx context.Context, addr string, u url.Values) } } -func (c *Client) requestURL(addr, path string) string { - if strings.Contains(addr, "://") { - return addr + path - } - return c.scheme + "://" + addr + path -} - func (c *Client) roundTrip(ctx context.Context, method, addr, path string, body io.Reader, bearer string) (*http.Response, error) { - req, err := http.NewRequestWithContext(ctx, method, c.requestURL(addr, path), body) + u := addr + path + if !strings.Contains(addr, "://") { + u = c.scheme + "://" + u + } + req, err := http.NewRequestWithContext(ctx, method, u, body) if err != nil { return nil, err } diff --git a/sdk/go/silkd/silkdtest/silkdtest.go b/sdk/go/silkd/silkdtest/silkdtest.go index 192d19d6..49667a9c 100644 --- a/sdk/go/silkd/silkdtest/silkdtest.go +++ b/sdk/go/silkd/silkdtest/silkdtest.go @@ -84,21 +84,14 @@ func serveConn(conn net.Conn, r *bufio.Reader, serve func(net.Conn, *bufio.Reade } func serveStateless(conn net.Conn, r *bufio.Reader, req wire.Request) { - if !serveCommon(conn, r, req) { - errFrame(conn, wire.KindUnimplemented, "silkdtest: "+req.Op()) - } -} - -func serveCommon(conn net.Conn, r *bufio.Reader, req wire.Request) bool { switch req := req.(type) { case *wire.Exec: serveExec(conn, r, req) case *wire.PortForward: portEcho(conn, r, req.Port) default: - return false + errFrame(conn, wire.KindUnimplemented, "silkdtest: "+req.Op()) } - return true } func acceptLoop(l net.Listener, serve func(net.Conn)) { From e0d52bb8ccb5db0762848d633d7475a14632a831 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 18:53:32 +0800 Subject: [PATCH 16/17] test: settle refills before counting clones, fail fast on a claims load error, scope table contexts to their subtest, cover every decoder with a fixture, and drop dead or always-true checks The fixture test now ranges the decoder tables instead of a hand list a new verb could miss; the watcher garbage test asserts the error it promises; smoke and bench tools type their 409 check and stop racing the handshake reassignment. --- e2e/cmd/rpcbench/main.go | 5 +- e2e/cmd/volumesmoke/main.go | 3 +- protocol/wire/frame_test.go | 76 +++-------------------------- sandboxd/config/reload_test.go | 2 +- sandboxd/pool/interrupted_test.go | 12 ++--- sandboxd/pool/refillbackoff_test.go | 1 + sandboxd/pool/refillkick_test.go | 1 + sandboxd/pool/replica_test.go | 8 ++- sdk/go/checkpoint_test.go | 2 +- sdk/go/sandbox_test.go | 6 +-- sdk/go/volumes_test.go | 11 ----- sdk/langchain/tests/test_toolkit.py | 4 +- sdk/python/tests/test_hardening.py | 1 + sdk/python/tests/test_stream.py | 12 ++--- 14 files changed, 37 insertions(+), 107 deletions(-) diff --git a/e2e/cmd/rpcbench/main.go b/e2e/cmd/rpcbench/main.go index 6e7e2874..ccd8e667 100644 --- a/e2e/cmd/rpcbench/main.go +++ b/e2e/cmd/rpcbench/main.go @@ -179,11 +179,12 @@ func edgeTLS(addr, caCert string) (*tls.Config, error) { func dialAgent(ctx context.Context, addr, id, token string, tlsCfg *tls.Config) (net.Conn, error) { host := strings.TrimPrefix(strings.TrimPrefix(addr, "https://"), "http://") var d net.Dialer - raw, err := d.DialContext(ctx, "tcp", host) + dialed, err := d.DialContext(ctx, "tcp", host) if err != nil { return nil, err } - stop := context.AfterFunc(ctx, func() { _ = raw.Close() }) + raw := dialed + stop := context.AfterFunc(ctx, func() { _ = dialed.Close() }) defer stop() scheme := "http" if tlsCfg != nil { diff --git a/e2e/cmd/volumesmoke/main.go b/e2e/cmd/volumesmoke/main.go index 14f79780..0a80d5bc 100644 --- a/e2e/cmd/volumesmoke/main.go +++ b/e2e/cmd/volumesmoke/main.go @@ -7,6 +7,7 @@ import ( "errors" "flag" "fmt" + "net/http" "os" "path" "path/filepath" @@ -224,7 +225,7 @@ func runWritable(ctx context.Context, client *sandbox.Client, template, volume s _ = busy.Close() return errors.New("second writable claim succeeded while the first held the image") } - if !strings.Contains(busyErr.Error(), "409") { + if apiErr, ok := errors.AsType[*sandbox.APIError](busyErr); !ok || apiErr.Status != http.StatusConflict { return fmt.Errorf("second writable claim failed without 409: %w", busyErr) } if err = writer.Close(); err != nil { diff --git a/protocol/wire/frame_test.go b/protocol/wire/frame_test.go index bcf2c473..5fb5f2c6 100644 --- a/protocol/wire/frame_test.go +++ b/protocol/wire/frame_test.go @@ -64,70 +64,6 @@ func TestFixtureCorpusRoundTrips(t *testing.T) { } func TestEveryVerbHasAFixture(t *testing.T) { - allRequests := []Request{ - Exec{}, - Info{}, - Ps{}, - Kill{}, - Attach{}, - Logs{}, - SessionCreate{}, - SessionList{}, - SessionRm{}, - Stdin{}, - StdinClose{}, - FsWrite{}, - FsRead{}, - FsList{}, - FsStat{}, - FsMkdir{}, - FsRm{}, - FsRename{}, - FsPush{}, - FsPull{}, - FsFind{}, - FsReplace{}, - FsWatch{}, - PtyOpen{}, - PtyResize{}, - PortForward{}, - GitClone{}, - GitStatus{}, - GitAdd{}, - GitCommit{}, - GitPush{}, - GitPull{}, - GitBranch{}, - LspStart{}, - LspRequest{}, - LspStop{}, - Data{}, - DataEnd{}, - } - allResponses := []Response{ - Started{}, - Stdout{}, - Stderr{}, - Exit{}, - Done{}, - Ready{}, - ErrorResp{}, - InfoResp{}, - Procs{}, - DataResp{}, - Entries{}, - Stat{}, - SessionCreated{}, - Sessions{}, - Match{}, - Replaced{}, - Event{}, - GitStatusResult{}, - GitCommitResult{}, - GitBranches{}, - LspStarted{}, - } - entries, err := os.ReadDir(fixtureDir) if err != nil { t.Fatalf("fixtures dir: %v", err) @@ -150,14 +86,14 @@ func TestEveryVerbHasAFixture(t *testing.T) { } } } - for _, r := range allRequests { - if !haveReq[r.Op()] { - t.Errorf("request %q has no fixture", r.Op()) + for op := range requestDecoders { + if !haveReq[op] { + t.Errorf("request %q has no fixture", op) } } - for _, r := range allResponses { - if !haveResp[r.RespType()] { - t.Errorf("response %q has no fixture", r.RespType()) + for typ := range responseDecoders { + if !haveResp[typ] { + t.Errorf("response %q has no fixture", typ) } } } diff --git a/sandboxd/config/reload_test.go b/sandboxd/config/reload_test.go index 08ad22f8..95aa8996 100644 --- a/sandboxd/config/reload_test.go +++ b/sandboxd/config/reload_test.go @@ -63,7 +63,7 @@ func TestReloadDiffSortsReloadableOwnedAndRestartFields(t *testing.T) { } restart := *cur - restart.Listen, restart.Bridges, restart.EgressUsageBytes = "127.0.0.1:8888", []string{"br1"}, true + restart.Listen, restart.Bridges = "127.0.0.1:8888", []string{"br1"} if _, _, err := cur.ReloadDiff(&restart); err == nil || err.Error() != "listen, bridges change only at a restart" { t.Errorf("ReloadDiff over restart fields: %v", err) } diff --git a/sandboxd/pool/interrupted_test.go b/sandboxd/pool/interrupted_test.go index bc427c55..d46f2d0a 100644 --- a/sandboxd/pool/interrupted_test.go +++ b/sandboxd/pool/interrupted_test.go @@ -470,14 +470,14 @@ func TestReconcileWithoutASnapshotListSettlesAStoppedHibernate(t *testing.T) { func TestACaptureSettlesTheIntentAFailedHibernateLeft(t *testing.T) { for _, tt := range []struct { name string - capture func(m *Manager, sb *types.Sandbox) error + capture func(ctx context.Context, m *Manager, sb *types.Sandbox) error }{ - {"fork", func(m *Manager, sb *types.Sandbox) error { - _, err := m.Fork(t.Context(), sb.ID, Cred{Token: sb.Token}, 1, 0, "", "") + {"fork", func(ctx context.Context, m *Manager, sb *types.Sandbox) error { + _, err := m.Fork(ctx, sb.ID, Cred{Token: sb.Token}, 1, 0, "", "") return err }}, - {"checkpoint", func(m *Manager, sb *types.Sandbox) error { - _, err := m.Checkpoint(t.Context(), sb.ID, Cred{Token: sb.Token}, "", "") + {"checkpoint", func(ctx context.Context, m *Manager, sb *types.Sandbox) error { + _, err := m.Checkpoint(ctx, sb.ID, Cred{Token: sb.Token}, "", "") return err }}, } { @@ -491,7 +491,7 @@ func TestACaptureSettlesTheIntentAFailedHibernateLeft(t *testing.T) { m.mu.Lock() sb.PendingSnap = interruptedHib m.mu.Unlock() - if err := tt.capture(m, sb); err != nil { + if err := tt.capture(t.Context(), m, sb); err != nil { t.Fatalf("capture: %v", err) } if len(eng.snapSaves) != 0 || sb.HibernateSnap != interruptedHib { diff --git a/sandboxd/pool/refillbackoff_test.go b/sandboxd/pool/refillbackoff_test.go index ef24cca5..22e41b61 100644 --- a/sandboxd/pool/refillbackoff_test.go +++ b/sandboxd/pool/refillbackoff_test.go @@ -97,6 +97,7 @@ func TestRefillOnceHonorsTheBackoff(t *testing.T) { for range 5 { m.refillOnce(t.Context()) } + synctest.Wait() if n := eng.cloneCount(); n != attempts { t.Errorf("clones=%d after backing off, want %d: a backed-off pool must not attempt again", n, attempts) } diff --git a/sandboxd/pool/refillkick_test.go b/sandboxd/pool/refillkick_test.go index 556ac527..2698142e 100644 --- a/sandboxd/pool/refillkick_test.go +++ b/sandboxd/pool/refillkick_test.go @@ -76,6 +76,7 @@ func TestNodeAtCapacityParksRefillInsteadOfRetrying(t *testing.T) { for range 5 { m.refillOnce(t.Context()) } + synctest.Wait() if n := eng.cloneCount(); n != attempts { t.Errorf("clones=%d after parking, want %d: a parked node must not attempt again", n, attempts) } diff --git a/sandboxd/pool/replica_test.go b/sandboxd/pool/replica_test.go index 7ccd1f71..8f43ba9a 100644 --- a/sandboxd/pool/replica_test.go +++ b/sandboxd/pool/replica_test.go @@ -35,8 +35,12 @@ func TestReplicaRoundPublishesMovesTheRowAndDropsThePrevious(t *testing.T) { if first == "" || rec.Owner != "a" || rec.Checkpoint != first || rec.Consumed || rec.TokenSHA256 != tenants.TokenSHA256(sb.Token) || !rec.Deadline.IsZero() { t.Fatalf("row %+v with replica %q, want a live row of a on the replica with the token hash and no deadline", rec, first) } - if stored, loadErr := newClaimStore(a.dataDir, false).load(); loadErr != nil || stored[sb.ID].ReplicaCk != first || stored[sb.ID].ArchiveEpoch != rec.Epoch { - t.Errorf("claims.json replica %q epoch %d (%v), want %q at %d", stored[sb.ID].ReplicaCk, stored[sb.ID].ArchiveEpoch, loadErr, first, rec.Epoch) + stored, loadErr := newClaimStore(a.dataDir, false).load() + if loadErr != nil { + t.Fatalf("load claims.json: %v", loadErr) + } + if stored[sb.ID].ReplicaCk != first || stored[sb.ID].ArchiveEpoch != rec.Epoch { + t.Errorf("claims.json replica %q epoch %d, want %q at %d", stored[sb.ID].ReplicaCk, stored[sb.ID].ArchiveEpoch, first, rec.Epoch) } meta, err := a.loadCheckpoint(t.Context(), first) if err != nil || !meta.Archive { diff --git a/sdk/go/checkpoint_test.go b/sdk/go/checkpoint_test.go index 888ceee9..18ec7786 100644 --- a/sdk/go/checkpoint_test.go +++ b/sdk/go/checkpoint_test.go @@ -57,7 +57,7 @@ func TestCheckpointNewRejectsVolumeOptionsLocally(t *testing.T) { t.Run(tt.name, func(t *testing.T) { ck := &Checkpoint{} sb, err := ck.New(t.Context(), tt.opt) - if err == nil || !strings.Contains(err.Error(), tt.name) { + if err == nil || !strings.Contains(err.Error(), "checkpoint claims do not accept") { t.Errorf("err %v, want local %s rejection", err, tt.name) } if sb != nil { diff --git a/sdk/go/sandbox_test.go b/sdk/go/sandbox_test.go index 1966d969..f29455d8 100644 --- a/sdk/go/sandbox_test.go +++ b/sdk/go/sandbox_test.go @@ -164,11 +164,7 @@ func TestFirstCallSendsItsFramesAheadOfThe101(t *testing.T) { defer conn.Close() _ = conn.SetReadDeadline(time.Now().Add(5 * time.Second)) var got []string - for range 1 { - line, readErr := brw.ReadBytes('\n') - if readErr != nil { - break - } + if line, readErr := brw.ReadBytes('\n'); readErr == nil { if req, decodeErr := wire.DecodeRequest(line); decodeErr == nil { got = append(got, req.Op()) } diff --git a/sdk/go/volumes_test.go b/sdk/go/volumes_test.go index 7e4a50dc..e2d46cb7 100644 --- a/sdk/go/volumes_test.go +++ b/sdk/go/volumes_test.go @@ -179,17 +179,6 @@ func TestVolumeClaimRedirectPreservesEntries(t *testing.T) { } } -func TestCheckpointNewRejectsVolumesLocally(t *testing.T) { - ck := &Checkpoint{} - sb, err := ck.New(t.Context(), WithVolumes(Volume{Name: "imagenet"})) - if err == nil || !strings.Contains(err.Error(), "do not accept WithVolumes") { - t.Errorf("err %v, want local WithVolumes rejection", err) - } - if sb != nil { - t.Errorf("sandbox %+v, want nil", sb) - } -} - func TestWithVolumesEncodesMode(t *testing.T) { tests := []struct { name string diff --git a/sdk/langchain/tests/test_toolkit.py b/sdk/langchain/tests/test_toolkit.py index c8945045..f792230d 100644 --- a/sdk/langchain/tests/test_toolkit.py +++ b/sdk/langchain/tests/test_toolkit.py @@ -19,7 +19,7 @@ def __init__(self): def run(self, argv, cwd="", on_stdout=None, on_stderr=None, timeout=None, **_): assert argv[:2] == ["sh", "-c"], argv - assert timeout is not None and 0 < timeout <= 300, timeout + assert timeout is not None and 0 < timeout <= CALL_TIMEOUT, timeout if argv[2] == "boom": on_stderr(b"kaboom\n") return 3 @@ -59,7 +59,7 @@ def test_exec_tool_output(monkeypatch): assert exec_tool.invoke({"command": "echo hi"}) == "ran: echo hi\n" out = exec_tool.invoke({"command": "boom"}) assert "kaboom" in out and "exit code: 3" in out - assert exec_tool.invoke({"command": "hang"}) == "partial\n\ncut off after 300s" + assert exec_tool.invoke({"command": "hang"}) == f"partial\n\ncut off after {CALL_TIMEOUT}s" def test_async_bridge(monkeypatch): diff --git a/sdk/python/tests/test_hardening.py b/sdk/python/tests/test_hardening.py index f2b5e8a2..286736e0 100644 --- a/sdk/python/tests/test_hardening.py +++ b/sdk/python/tests/test_hardening.py @@ -72,6 +72,7 @@ def test_watcher_ends_cleanly_on_garbage_frame(conn_pair): guest_sock.close() watcher = Watcher(conn) assert list(watcher) == [] + assert isinstance(watcher.error, ProtocolError), watcher.error @pytest.mark.parametrize( diff --git a/sdk/python/tests/test_stream.py b/sdk/python/tests/test_stream.py index 55208fd2..a565a72a 100644 --- a/sdk/python/tests/test_stream.py +++ b/sdk/python/tests/test_stream.py @@ -21,7 +21,7 @@ def test_port_stream_outlives_the_client_timeout(): addr = f"127.0.0.1:{server.getsockname()[1]}" ops: list[str] = [] threading.Thread(target=serve_port_forward, args=(server, 3 * TIMEOUT, ops), daemon=True).start() - sb = legacy_sandbox(addr) + sb = short_timeout_sandbox(addr) try: with sb.dial_port(5000) as port: assert port.recv() == b"late" @@ -36,7 +36,7 @@ def test_run_timeout_cuts_a_silent_command_and_kills_it(): addr = f"127.0.0.1:{server.getsockname()[1]}" kills: list[dict] = [] threading.Thread(target=serve_started_then_hang, args=(server, 5 * TIMEOUT, kills), daemon=True).start() - sb = legacy_sandbox(addr) + sb = short_timeout_sandbox(addr) started = time.monotonic() try: with pytest.raises(TimeoutError): @@ -53,7 +53,7 @@ def test_run_timeout_bounds_the_kill_it_sends(monkeypatch): addr = f"127.0.0.1:{server.getsockname()[1]}" kills: list[dict] = [] threading.Thread(target=serve_started_then_trickle_the_kill_reply, args=(server, kills), daemon=True).start() - sb = legacy_sandbox(addr) + sb = short_timeout_sandbox(addr) started = time.monotonic() try: with pytest.raises(TimeoutError): @@ -73,7 +73,7 @@ def test_run_timeout_cuts_a_blocked_exec_send(black_hole): def test_run_rejects_a_non_positive_timeout(): - sb = legacy_sandbox("127.0.0.1:1") + sb = short_timeout_sandbox("127.0.0.1:1") with pytest.raises(ValueError): sb.run(["true"], timeout=0) @@ -82,7 +82,7 @@ def test_dial_is_still_bounded_by_the_client_timeout(): server = socket.create_server(("127.0.0.1", 0)) addr = f"127.0.0.1:{server.getsockname()[1]}" threading.Thread(target=serve_silence, args=(server,), daemon=True).start() - sb = legacy_sandbox(addr) + sb = short_timeout_sandbox(addr) started = time.monotonic() try: with pytest.raises(SandboxTimeout): @@ -97,7 +97,7 @@ def test_handshake_failures_are_typed(exc, want): assert isinstance(conn_module._handshake_error("agent upgrade", exc), want) -def legacy_sandbox(addr: str) -> Sandbox: +def short_timeout_sandbox(addr: str) -> Sandbox: return sandbox_at(addr, timeout=TIMEOUT, keep_alive=0) From 76dd3b42df55983b9f7279da9106f4cca92b8090 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 18:59:27 +0800 Subject: [PATCH 17/17] pool: refuse a move while a failed save capture is still settling moveBlocker refused an unsettled hibernate intent but not an unsettled save; a move that won the transition over the save's settle carried SaveSnap to the target and left a listed fork snapshot on the source that the startup sweep keeps. --- sandboxd/pool/move_out.go | 2 ++ sandboxd/pool/move_test.go | 11 +++++++++++ 2 files changed, 13 insertions(+) diff --git a/sandboxd/pool/move_out.go b/sandboxd/pool/move_out.go index d6df65ae..42f9d5b2 100644 --- a/sandboxd/pool/move_out.go +++ b/sandboxd/pool/move_out.go @@ -193,6 +193,8 @@ func (m *Manager) moveBlocker(sb *types.Sandbox) string { return "it is archived or archiving" case sb.HibernateSnap != "" || sb.PendingSnap != "": return "it is hibernated" + case sb.SaveSnap != "": + return "a capture of it is settling" case sb.Failed != "": return "it failed" case sb.Waking: diff --git a/sandboxd/pool/move_test.go b/sandboxd/pool/move_test.go index 32c0a2ad..35083f89 100644 --- a/sandboxd/pool/move_test.go +++ b/sandboxd/pool/move_test.go @@ -77,6 +77,17 @@ func TestMoveHandsARunningClaimToThePeerWithBothNodesCountingIt(t *testing.T) { assertNoMoveResidue(t, mc.t, sb.ID) } +func TestMoveRefusesAClaimWhoseSaveIsUnsettled(t *testing.T) { + mc := newMoveCluster(t) + sb := claimTenant(t, mc.s, "acme") + mc.s.mu.Lock() + sb.SaveSnap = "sbx-fork-unsettled" + mc.s.mu.Unlock() + if _, err := mc.s.Move(t.Context(), sb.ID, "b", time.Minute); !errors.Is(err, ErrNotMovable) { + t.Errorf("move with an unsettled save: %v, want ErrNotMovable", err) + } +} + func TestMoveDisarmsTheSourceDoorsAndArmsTheTarget(t *testing.T) { mc := newMoveCluster(t) sb := mustClaim(t, mc.s, testKey)