Repository navigation
e2e: pin the SDK control-plane structs to the server's JSON shape - #357
Merged
Merged
Conversation
CMGS
force-pushed
the
refactor/api-vocabulary-placement
branch
from
October 17, 2026 16:28
f808535 to
3edf2d7
Compare
CMGS
force-pushed
the
test/sdk-server-wire-parity
branch
from
October 17, 2026 16:28
2106888 to
4cbd937
Compare
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.
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.
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.
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.
CMGS
force-pushed
the
test/sdk-server-wire-parity
branch
from
October 17, 2026 17:00
60ac2b6 to
8432d63
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #356 (it moves the server-only DTOs into package
server; this test reads them there). Merge #356 first.What
VolumePatch.add/removebecomeomitzero, matching the SDK (the one real divergence the test found), with an e2e regression test that sends[]/[]to a real server.serverInfo.versionreports the release version via-X main.version(goreleaser), as sandboxd does; it was a hard-coded0.1.0.DecodeData; the engine's private copy of the bulk data-frame decode (fastPortData,portDataHead) is deleted.Why
The SDK hand-mirrors the server's wire structs and nothing kept them in step; the per-feature
*WireShapetests only caught drift after the fact. The types stay where they are: wire is the silkd frame contract and the server types carry validation.Allowlisted on purpose: server-only
claim.egress,fork.claim_ref_prefix/on_expire,renew.on_expire, node-local checkpoint and summary fields, config-ownedPoolSpecfields, andPoolSpec.net/sizeandTenantSpec.token(omit option differs by design).VolumePatch
An empty or absent add/remove means "change nothing" on the server, so both shapes decode the same.
omitemptyon the server type disagreed with the SDK'somitzero, which sends an explicit[]and omits only nil; the SDK side is kept so a caller's[]is never dropped on the wire.Tests
TestSDKStructsMatchServerWireShape(mutation-checked: renaming an SDK tag fails it with the pair and field).TestEmptyVolumePatchChangesNothing,TestDecodeDataTakesOnlyCanonicalDataFrames, mcp initialize version assertion.make go-lint: 10 x0 issues.;make go-testgreen;asl -forwarder=falseclean on e2e, sandboxd, mcp, protocol/wire for linux and darwin.Hot-path cost
Only the
DecodeDatachange touches a hot path (guest port read). A/B, both arms interleaved in one binary with the order swapped, 20 samples per arm:benchstat reports no significant difference (p 0.51 to 0.80). No new work on the claim path.