From df20a375528321ecc2b374dc1524cc7b2e23a77e Mon Sep 17 00:00:00 2001 From: CMGS Date: Sat, 17 Oct 2026 23:33:39 +0800 Subject: [PATCH 1/6] e2e: pin the SDK control-plane structs to the server's JSON shape The SDK hand-mirrors about 40 request and reply structs from sandboxd, and only the per-feature WireShape tests noticed drift, after the fact. One table now pairs each SDK struct with its server counterpart and compares the JSON field names; for structs the SDK encodes it also compares the omit options. The SDK side is read from source with go/parser because most of the mirrors are unexported. Each pair carries an allowlist of fields that deliberately differ: server only fields (ClaimRequest.egress, ForkRequest.claim_ref_prefix and on_expire, RenewRequest.on_expire, the node-local Checkpoint and SandboxSummary.token fields, the config-owned PoolSpec fields) and omit options that differ by design (PoolSpec net and size default on the server, TenantSpec token keeps the stored token when absent). An allowlist entry that no longer differs fails too, so the list cannot go stale. VolumePatch add and remove differ (omitzero in the SDK, omitempty on the server); they are allowlisted here and settled in the next commit. --- e2e/wire_parity_test.go | 156 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 156 insertions(+) create mode 100644 e2e/wire_parity_test.go diff --git a/e2e/wire_parity_test.go b/e2e/wire_parity_test.go new file mode 100644 index 00000000..9093a678 --- /dev/null +++ b/e2e/wire_parity_test.go @@ -0,0 +1,156 @@ +package e2e + +import ( + "cmp" + "go/ast" + "go/parser" + "go/token" + "maps" + "path/filepath" + "reflect" + "slices" + "strconv" + "strings" + "testing" + + "github.com/cocoonstack/sandbox/sandboxd/config" + "github.com/cocoonstack/sandbox/sandboxd/pool" + "github.com/cocoonstack/sandbox/sandboxd/server" + "github.com/cocoonstack/sandbox/sandboxd/types" +) + +const sdkDir = "../sdk/go" + +var wirePairs = []struct { + sdk string + sends bool + server any + allow []string +}{ + {"claimRequest", true, types.ClaimRequest{}, []string{"egress"}}, + {"claimResponse", false, types.ClaimResponse{}, nil}, + {"forkRequest", true, types.ForkRequest{}, []string{"claim_ref_prefix", "on_expire"}}, + {"forkResponse", false, types.ForkResponse{}, nil}, + {"renewRequest", true, types.RenewRequest{}, []string{"on_expire"}}, + {"renewResponse", false, types.RenewResponse{}, nil}, + {"promoteRequest", true, types.PromoteRequest{}, nil}, + {"promoteResponse", false, types.PromoteResponse{}, nil}, + {"previewRequest", true, types.PreviewRequest{}, nil}, + {"previewResponse", false, types.PreviewResponse{}, nil}, + {"checkpointRequest", true, types.CheckpointRequest{}, nil}, + {"checkpointResponse", false, types.CheckpointResponse{}, nil}, + {"checkpointClaimRequest", true, types.CheckpointClaimRequest{}, nil}, + {"checkpointListResponse", false, types.CheckpointListResponse{}, nil}, + {"checkpointRecord", false, types.Checkpoint{}, []string{"key", "tenant", "policy_source", "no_egress", "guest_env", "archive"}}, + {"volumeListResponse", false, types.VolumeListResponse{}, nil}, + {"VolumeInfo", false, types.VolumeInfo{}, nil}, + {"VolumeState", false, types.VolumeState{}, nil}, + {"SandboxVolumesResponse", false, types.SandboxVolumesResponse{}, nil}, + {"VolumePatch", true, types.VolumePatch{}, []string{"add", "remove"}}, + {"Volume", true, types.Volume{}, nil}, + {"EnvVar", true, types.EnvVar{}, nil}, + {"EnvInject", true, types.EnvInject{}, nil}, + {"envBody", true, server.SandboxEnv{}, nil}, + {"envPatchBody", true, server.SandboxEnvPatch{}, nil}, + {"PoolKey", false, types.PoolKey{}, nil}, + {"NodeInfo", false, server.InfoResponse{}, nil}, + {"PoolStatus", false, pool.PoolInfo{}, nil}, + {"TemplateStatus", false, pool.TemplateInfo{}, nil}, + {"SandboxSummary", false, pool.SandboxSummary{}, []string{"token"}}, + {"sandboxListResponse", false, server.SandboxListResponse{}, nil}, + {"poolUpdate", true, server.PoolUpdateRequest{}, nil}, + {"PoolSpec", true, config.PoolSpec{}, []string{"egress", "warmup", "egress_upstream_env", "capture_trim", "storage", "net", "size"}}, + {"tenantsUpdate", true, server.TenantsRequest{}, nil}, + {"tenantUpdate", true, server.TenantRequest{}, nil}, + {"TenantSpec", true, config.TenantSpec{}, []string{"token"}}, + {"TenantInfo", false, pool.TenantInfo{}, nil}, + {"TenantList", false, pool.TenantPage{}, nil}, +} + +func TestSDKStructsMatchServerWireShape(t *testing.T) { + sdkFields := parseSDKFields(t) + for _, tt := range wirePairs { + t.Run(tt.sdk, func(t *testing.T) { + got, ok := sdkFields[tt.sdk] + if !ok { + t.Fatalf("sdk struct %s not found in %s", tt.sdk, sdkDir) + } + want := serverFields(reflect.TypeOf(tt.server)) + for name, opts := range want { + gotOpts, present := got[name] + differs := !present || (tt.sends && gotOpts != opts) + switch allowed := slices.Contains(tt.allow, name); { + case differs && !allowed: + t.Errorf("%s: field %q differs: sdk present=%t options=%q, server options=%q", tt.sdk, name, present, gotOpts, opts) + case !differs && allowed: + t.Errorf("%s: field %q is allowlisted but matches", tt.sdk, name) + } + } + for name := range got { + if _, present := want[name]; !present { + t.Errorf("%s: field %q is in the SDK but not on the server", tt.sdk, name) + } + } + }) + } +} + +func serverFields(typ reflect.Type) map[string]string { + fields := map[string]string{} + for f := range typ.Fields() { + name, opts := splitJSONTag(f.Tag.Get("json")) + switch { + case name == "-" && opts == "": + case f.Anonymous && name == "" && f.Type.Kind() == reflect.Struct: + maps.Copy(fields, serverFields(f.Type)) + case f.IsExported(): + fields[cmp.Or(name, f.Name)] = opts + } + } + return fields +} + +func parseSDKFields(t *testing.T) map[string]map[string]string { + t.Helper() + paths, err := filepath.Glob(filepath.Join(sdkDir, "*.go")) + if err != nil { + t.Fatal(err) + } + out := map[string]map[string]string{} + for _, path := range paths { + if strings.HasSuffix(path, "_test.go") { + continue + } + src, err := parser.ParseFile(token.NewFileSet(), path, nil, parser.SkipObjectResolution) + if err != nil { + t.Fatal(err) + } + ast.Inspect(src, func(n ast.Node) bool { + spec, ok := n.(*ast.TypeSpec) + if !ok { + return true + } + if st, ok := spec.Type.(*ast.StructType); ok { + out[spec.Name.Name] = map[string]string{} + for _, f := range st.Fields.List { + if f.Tag == nil { + continue + } + tag, _ := strconv.Unquote(f.Tag.Value) + if name, opts := splitJSONTag(reflect.StructTag(tag).Get("json")); name != "" && name != "-" { + out[spec.Name.Name][name] = opts + } + } + } + return true + }) + } + return out +} + +func splitJSONTag(tag string) (name, opts string) { + name, rest, _ := strings.Cut(tag, ",") + options := strings.Split(rest, ",") + slices.Sort(options) + return name, strings.Join(options, ",") +} From bb68d6d698d0035852ede40d0df04e756f45da65 Mon Sep 17 00:00:00 2001 From: CMGS Date: Sat, 17 Oct 2026 23:34:02 +0800 Subject: [PATCH 2/6] types: tag VolumePatch add and remove omitzero like the SDK The parity test found the SDK encoding VolumePatch with omitzero while the server type said omitempty. The SDK is the side to keep: an explicit empty slice goes on the wire as [] and a nil one is omitted, the same contract SetVolumes documents, and sdk/go already pins it. omitempty would drop a caller's [] silently. The server reads [] and absent alike (both mean change nothing, not detach all), so no decoded request changes; only the declared shape now agrees. The new e2e test sends an empty patch to a real server and checks the held volume stays attached. --- e2e/volume_mutation_test.go | 17 +++++++++++++++++ e2e/wire_parity_test.go | 2 +- sandboxd/types/api.go | 4 ++-- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/e2e/volume_mutation_test.go b/e2e/volume_mutation_test.go index 9fd17e57..5fcedef8 100644 --- a/e2e/volume_mutation_test.go +++ b/e2e/volume_mutation_test.go @@ -60,3 +60,20 @@ func TestRejectedVolumeTargetExplainsRefusalWithoutChangingState(t *testing.T) { t.Fatalf("rejected request changed settled state: %+v err=%v", view, err) } } + +func TestEmptyVolumePatchChangesNothing(t *testing.T) { + image := writeVolumeImage(t, "dataset.img", "dataset-bytes") + stack := startTenantStack(t, "node-token", nil, []config.VolumeSpec{{Name: "dataset", Path: image}}) + sb, err := stack.client.New(t.Context(), "rt:24.04") + if err != nil { + t.Fatal(err) + } + defer sb.Close() + if _, err = sb.SetVolumes(t.Context(), []sandbox.Volume{{Name: "dataset"}}); err != nil { + t.Fatal(err) + } + got, err := sb.PatchVolumes(t.Context(), sandbox.VolumePatch{Add: []sandbox.Volume{}, Remove: []string{}}) + if err != nil || len(got.Volumes) != 1 || got.Volumes[0].Name != "dataset" || got.Converging { + t.Fatalf("empty patch: %+v err=%v", got, err) + } +} diff --git a/e2e/wire_parity_test.go b/e2e/wire_parity_test.go index 9093a678..8dcff27c 100644 --- a/e2e/wire_parity_test.go +++ b/e2e/wire_parity_test.go @@ -46,7 +46,7 @@ var wirePairs = []struct { {"VolumeInfo", false, types.VolumeInfo{}, nil}, {"VolumeState", false, types.VolumeState{}, nil}, {"SandboxVolumesResponse", false, types.SandboxVolumesResponse{}, nil}, - {"VolumePatch", true, types.VolumePatch{}, []string{"add", "remove"}}, + {"VolumePatch", true, types.VolumePatch{}, nil}, {"Volume", true, types.Volume{}, nil}, {"EnvVar", true, types.EnvVar{}, nil}, {"EnvInject", true, types.EnvInject{}, nil}, diff --git a/sandboxd/types/api.go b/sandboxd/types/api.go index 3699c09b..ac0cd6b4 100644 --- a/sandboxd/types/api.go +++ b/sandboxd/types/api.go @@ -24,8 +24,8 @@ type VolumeInfo struct { // VolumePatch is the wire body of PATCH /v1/sandboxes/{id}/volumes. type VolumePatch struct { - Add []Volume `json:"add,omitempty"` - Remove []string `json:"remove,omitempty"` + Add []Volume `json:"add,omitzero"` + Remove []string `json:"remove,omitzero"` } // VolumePhase is how far one held volume has converged. From c6d72fe9f5449a2fb09f236f514ddbad34c2642f Mon Sep 17 00:00:00 2001 From: CMGS Date: Sat, 17 Oct 2026 23:34:30 +0800 Subject: [PATCH 3/6] mcp: report the release version in serverInfo The initialize reply hard-coded 0.1.0 while the repo is at v0.1.17. sandboxd already takes its version from a main.version variable set by goreleaser ldflags; mcp now does the same, so the release build stamps {{.Version}} and a plain go build or go test reports "devel". Reading the module version from runtime/debug.ReadBuildInfo was rejected: for a binary built inside this multi-module repo the main module version is "(devel)", so it would add a fallback path that never carries the release number. --- .goreleaser.yml | 1 + mcp/server.go | 4 +++- mcp/server_test.go | 3 +++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/.goreleaser.yml b/.goreleaser.yml index 39ac510a..17d7bc1b 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -48,6 +48,7 @@ builds: - GOWORK=off ldflags: - -s + - -X main.version={{.Version}} goos: - linux goarch: diff --git a/mcp/server.go b/mcp/server.go index acd2badc..a38912c9 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -24,6 +24,8 @@ const ( defaultToolTTL = time.Hour ) +var version = "devel" + type rpcRequest struct { ID jsontext.Value `json:"id"` Method string `json:"method"` @@ -107,7 +109,7 @@ func (s *server) dispatch(ctx context.Context, req *rpcRequest) rpcResponse { return result(req.ID, map[string]any{ "protocolVersion": protocolVersion, "capabilities": map[string]any{"tools": map[string]any{}}, - "serverInfo": map[string]any{"name": "sandbox-mcp", "version": "0.1.0"}, + "serverInfo": map[string]any{"name": "sandbox-mcp", "version": version}, }) case "tools/list": return result(req.ID, map[string]any{"tools": toolSpecs()}) diff --git a/mcp/server_test.go b/mcp/server_test.go index c86f3fa8..07f7cb56 100644 --- a/mcp/server_test.go +++ b/mcp/server_test.go @@ -57,6 +57,9 @@ func TestServeSpeaksMCP(t *testing.T) { if initResult["protocolVersion"] != protocolVersion { t.Errorf("initialize protocolVersion = %v", initResult["protocolVersion"]) } + if info := initResult["serverInfo"].(map[string]any); info["version"] != version { + t.Errorf("initialize serverInfo = %v, want version %q", info, version) + } tools := replies[2]["result"].(map[string]any)["tools"].([]any) if len(tools) != len(toolSpecs()) { t.Errorf("tools/list: %d tools, want %d", len(tools), len(toolSpecs())) From b2327ffd40237f9d73c301adbcf4649be27a7540 Mon Sep 17 00:00:00 2001 From: CMGS Date: Sat, 17 Oct 2026 23:38:50 +0800 Subject: [PATCH 4/6] wire: export DecodeData and use it for the guest port read path sandboxd/engine re-implemented the canonical data-frame decode that wire.decodeBulk already owns, only to skip the &DataResp{} that DecodeResponseInto builds. wire.DecodeData exposes the same decode on the data head, so the engine's copy (fastPortData, portDataHead) is deleted and a change to the bulk frame shape now lands in one place. A/B of the port read decode, both arms interleaved in one binary with the order swapped each round, 10 rounds per order (count 20 per arm), 1, 64 and 256 KiB frames: allocs/op 0 vs 0 on every size; sec/op 562.5n vs 566.0n, 34.64u vs 36.13u, 144.0u vs 144.8u, all within noise (p 0.51 to 0.80). The engine still passes the line without its trailing newline, since the wire decoder expects a bare frame. --- protocol/wire/frame.go | 7 +++++++ protocol/wire/frame_test.go | 19 +++++++++++++++++++ sandboxd/engine/portconn.go | 27 +-------------------------- 3 files changed, 27 insertions(+), 26 deletions(-) diff --git a/protocol/wire/frame.go b/protocol/wire/frame.go index eaf0a635..16a5f8ed 100644 --- a/protocol/wire/frame.go +++ b/protocol/wire/frame.go @@ -133,6 +133,8 @@ var ( func(d []byte) Response { return &Stderr{Data: d} }, func(d []byte) Response { return &DataResp{Data: d} }, ) + + dataRespHead = bulkResponses["data"].head ) // Request is a client→server frame; Op is its wire tag. @@ -700,6 +702,11 @@ func DecodeResponseInto(line []byte, scratch *[]byte) (Response, error) { return dec(line) } +// DecodeData slices a canonical data response frame's payload into *scratch without allocating a Response; ok is false for any other shape, which takes DecodeResponseInto. +func DecodeData(line []byte, scratch *[]byte) ([]byte, bool) { + return decodeBulk(line, dataRespHead, scratch) +} + // NewFrameScanner wraps r for newline-delimited frames capped at MaxFrame. func NewFrameScanner(r io.Reader) *bufio.Scanner { sc := bufio.NewScanner(r) diff --git a/protocol/wire/frame_test.go b/protocol/wire/frame_test.go index 95b39b44..6eb316cd 100644 --- a/protocol/wire/frame_test.go +++ b/protocol/wire/frame_test.go @@ -274,6 +274,25 @@ func TestDecodeResponseIntoReusesScratch(t *testing.T) { } } +func TestDecodeDataTakesOnlyCanonicalDataFrames(t *testing.T) { + var scratch []byte + got, ok := DecodeData([]byte(`{"type":"data","data":"aGVsbG8="}`), &scratch) + if !ok || string(got) != "hello" { + t.Fatalf("canonical frame: %q, %v", got, ok) + } + for _, frame := range []string{ + `{"type":"stdout","data":"aGVsbG8="}`, + `{"type":"data","meta":{"data":"WFhY"},"data":"aGk="}`, + `{"type":"data","data":"aGk="}garbage`, + "{\"type\":\"data\",\"data\":\"Q\nQ==\"}", + `{"type":"done"}`, + } { + if data, ok := DecodeData([]byte(frame), &scratch); ok { + t.Errorf("frame %s decoded as %q", frame, data) + } + } +} + func TestDecodeResponseIntoMatchesDecodeResponse(t *testing.T) { entries, err := os.ReadDir(fixtureDir) if err != nil { diff --git a/sandboxd/engine/portconn.go b/sandboxd/engine/portconn.go index a2d3b84b..05312151 100644 --- a/sandboxd/engine/portconn.go +++ b/sandboxd/engine/portconn.go @@ -3,7 +3,6 @@ package engine import ( "bufio" "bytes" - "encoding/base64" "encoding/json/v2" "errors" "fmt" @@ -16,8 +15,6 @@ import ( // portReadBuf fits silkd's data frames in one buffered read. const portReadBuf = wire.BulkChunk*4/3 + 64 -var portDataHead = []byte(`{"type":"data","data":"`) - // guestPortConn adapts silkd's port_forward data frames to a plain net.Conn. type guestPortConn struct { net.Conn @@ -37,7 +34,7 @@ func (g *guestPortConn) Read(p []byte) (int, error) { if err != nil { return 0, err } - if data, ok := g.fastPortData(line); ok { + if data, ok := wire.DecodeData(bytes.TrimSuffix(line, []byte("\n")), &g.rbuf); ok { g.pending = data continue } @@ -102,25 +99,3 @@ func (g *guestPortConn) readFrame() ([]byte, error) { } return long, err } - -// fastPortData slices the canonical data frame's base64 out without a JSON parse. -func (g *guestPortConn) fastPortData(line []byte) ([]byte, bool) { - after, ok := bytes.CutPrefix(line, portDataHead) - if !ok { - return nil, false - } - b64, rest, ok := bytes.Cut(after, []byte(`"`)) - if !ok || !bytes.Equal(rest, []byte("}\n")) { - return nil, false - } - if size := base64.StdEncoding.DecodedLen(len(b64)); cap(g.rbuf) < size { - g.rbuf = make([]byte, size) - } else { - g.rbuf = g.rbuf[:size] - } - n, err := base64.StdEncoding.Decode(g.rbuf, b64) - if err != nil || base64.StdEncoding.EncodedLen(n) != len(b64) { - return nil, false - } - return g.rbuf[:n], true -} From 7bf595f8e99d682c9a457fceacf7161e865deae2 Mon Sep 17 00:00:00 2001 From: CMGS Date: Sat, 17 Oct 2026 23:43:01 +0800 Subject: [PATCH 5/6] e2e: the parity test reads the server-owned request and response types --- e2e/wire_parity_test.go | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/e2e/wire_parity_test.go b/e2e/wire_parity_test.go index 8dcff27c..f9172ba7 100644 --- a/e2e/wire_parity_test.go +++ b/e2e/wire_parity_test.go @@ -27,22 +27,22 @@ var wirePairs = []struct { server any allow []string }{ - {"claimRequest", true, types.ClaimRequest{}, []string{"egress"}}, - {"claimResponse", false, types.ClaimResponse{}, nil}, - {"forkRequest", true, types.ForkRequest{}, []string{"claim_ref_prefix", "on_expire"}}, - {"forkResponse", false, types.ForkResponse{}, nil}, - {"renewRequest", true, types.RenewRequest{}, []string{"on_expire"}}, - {"renewResponse", false, types.RenewResponse{}, nil}, - {"promoteRequest", true, types.PromoteRequest{}, nil}, - {"promoteResponse", false, types.PromoteResponse{}, nil}, - {"previewRequest", true, types.PreviewRequest{}, nil}, - {"previewResponse", false, types.PreviewResponse{}, nil}, - {"checkpointRequest", true, types.CheckpointRequest{}, nil}, - {"checkpointResponse", false, types.CheckpointResponse{}, nil}, - {"checkpointClaimRequest", true, types.CheckpointClaimRequest{}, nil}, - {"checkpointListResponse", false, types.CheckpointListResponse{}, nil}, + {"claimRequest", true, server.ClaimRequest{}, []string{"egress"}}, + {"claimResponse", false, server.ClaimResponse{}, nil}, + {"forkRequest", true, server.ForkRequest{}, []string{"claim_ref_prefix", "on_expire"}}, + {"forkResponse", false, server.ForkResponse{}, nil}, + {"renewRequest", true, server.RenewRequest{}, []string{"on_expire"}}, + {"renewResponse", false, server.RenewResponse{}, nil}, + {"promoteRequest", true, server.PromoteRequest{}, nil}, + {"promoteResponse", false, server.PromoteResponse{}, nil}, + {"previewRequest", true, server.PreviewRequest{}, nil}, + {"previewResponse", false, server.PreviewResponse{}, nil}, + {"checkpointRequest", true, server.CheckpointRequest{}, nil}, + {"checkpointResponse", false, server.CheckpointResponse{}, nil}, + {"checkpointClaimRequest", true, server.CheckpointClaimRequest{}, nil}, + {"checkpointListResponse", false, server.CheckpointListResponse{}, nil}, {"checkpointRecord", false, types.Checkpoint{}, []string{"key", "tenant", "policy_source", "no_egress", "guest_env", "archive"}}, - {"volumeListResponse", false, types.VolumeListResponse{}, nil}, + {"volumeListResponse", false, server.VolumeListResponse{}, nil}, {"VolumeInfo", false, types.VolumeInfo{}, nil}, {"VolumeState", false, types.VolumeState{}, nil}, {"SandboxVolumesResponse", false, types.SandboxVolumesResponse{}, nil}, From 8432d63fb84189acd3b5417e72719bb8020dc565 Mon Sep 17 00:00:00 2001 From: CMGS Date: Sun, 18 Oct 2026 00:42:00 +0800 Subject: [PATCH 6/6] e2e: derive tenant parity type from the server request --- e2e/wire_parity_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/wire_parity_test.go b/e2e/wire_parity_test.go index f9172ba7..84312e84 100644 --- a/e2e/wire_parity_test.go +++ b/e2e/wire_parity_test.go @@ -62,7 +62,7 @@ var wirePairs = []struct { {"PoolSpec", true, config.PoolSpec{}, []string{"egress", "warmup", "egress_upstream_env", "capture_trim", "storage", "net", "size"}}, {"tenantsUpdate", true, server.TenantsRequest{}, nil}, {"tenantUpdate", true, server.TenantRequest{}, nil}, - {"TenantSpec", true, config.TenantSpec{}, []string{"token"}}, + {"TenantSpec", true, reflect.Zero(reflect.TypeOf(server.TenantsRequest{}.Tenants).Elem()).Interface(), []string{"token"}}, {"TenantInfo", false, pool.TenantInfo{}, nil}, {"TenantList", false, pool.TenantPage{}, nil}, }