Repository navigation
review: event stream cap, billing order, replica interval, silkd session and pty, compat cuts - #469
Merged
Merged
Conversation
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.
…ter 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.
… 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.
…ts pool, not as a bad fork count
…, 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.
…thing reads since the SDKs stopped probing
…icate 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.
…2'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.
…ell'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.
…ts 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.
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.
…fact the code cannot carry
…test, drop an unreachable preview guard and lint exclusions for directories that do not exist
…ad 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.
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.
CMGS
changed the title
CMGS
added a commit
that referenced
this pull request
Nov 9, 2026
* sdk/python: a callback's SDK error drops the exec stream instead of parking it mid-flight run, logs and attach parked the connection on any SilkdError, including one the caller's own on_stdout/on_stderr raised. The stream's remaining frames then sat in the reader buffer, where the quiet probe cannot see them, and the next call on the handle read them as its reply; a stdin pump blocked behind unread output also hung run's join. _deliver now aborts the connection, which unblocks the pump and makes the next take discard it. Twin of the Go fix in #524. * sdk/go: Run reports a stdin read failure even when the exit frame beats the pump pumpStdin sent stdin_close before handing its read error back, and Run only looked at that error once stdinDone had closed. A command that exits on stdin EOF (cat > f) answers exit right after the stdin_close, so the exit frame could arrive while the pump goroutine was still between its send and close(stdinDone); Run then returned the exit code with no error for truncated input, the gap 3464794 closed. The pump now publishes the error on a buffered channel before the stdin_close, and Run reads it without needing the pump to have returned. * pool: a retention purge records its reap before the archive_delete The fold caps a reap at its deadline only while the claim is still in a state; an archive_delete inside archived ends the span at its own t. The purge branch wrote archive_delete first, after the checkpoint delete, so every purged archive billed archived storage past its retention deadline by the reap tick, the journal fsync, the claims commit and the store delete, and by the whole downtime when the node was down across the deadline. Recording the reap first ends the span at the deadline, matching the order the restart path already uses. * pool: drop releaseRowTun, which never released an address purgeArchiveRow's rows all come from Sweepable, which returns rows without their claims (NULL::jsonb), so decoding rec.Claim failed on every purge: each logged a decode warning and released nothing. The address of a purged row already converges through reconcileTunAddrs, which reclaims it once no node has confirmed it for a day and its row is gone. * pool: a dropped move-in clears its staging dir before it frees the claim slot dropMoveIn forgot the claim first and removed moves/<id> only after the VM removal, which can take seconds. Once the claim was forgotten AdmitMove accepted a retry of the same id and wiped and refilled the staging dir, and the old drop then deleted the retry's rounds, failing the retried move at receive. Removing the dir while the claim still holds the id keeps the retry's files, the sibling of the round and commit cases the reservation already guards. * ci: run the root-only pty test with the other root-only silkd tests #524 added a_user_pty_shell_owns_its_tty behind #[ignore = "needs root"], but the sudo step lists its test binaries by name and pty_e2e was not among them, so the devpts ownership check never ran in CI. * egress: give the SOCKS non-ASCII deny test a name that lowercases into its rule x.examplİ.com lowercases to x.exampli.com, which *.example.com never matched, so the case passed on the rule alone and deleting the plainHost check would not fail it. x.apİ.internal lowercases to x.api.internal, the vector the forward door's test already uses. * sdk: the LangChain toolkit and OpenAI adapter close their client when the release fails Both released the sandbox and then closed the Client, so a release that raised skipped the close and left the parked control connections open for as long as the closed toolkit or deleted session stayed referenced; the error-path half of the #524 adapter-close fix. * docs: drift against the code - deploy.md: balloon is in the node-wide reload list, and the unit runs /usr/local/bin/sandboxd - sandboxd-api.md: a failed archive at expiry retries a minute later; every wakes_refused reason counts a refusal whether or not a hand-off ran - performance.md: there is no proto 2 since #469 - sdk.md: disk_reserve also refuses a hibernated wake; the PutTenantCluster example keeps the egress class it would otherwise clear - silkd.md: the lsp not_found names the manifest path; the user errors quote the name - protocol/README.md: Go and Rust both pin the corpus size, and only Go checks every verb has a fixture - README.md: a fresh repo builds the whole image chain on its own; carriers are sha-pinned only when rebuilt in the run - engine: the package doc names the cocoon daemon event stream - metrics: anon_bytes help includes used swap * docs: a preview wake can hand an archived claim to a peer A preview hit dials through wakeArchived like any call, so on a node over memory_limit, max_running or disk_reserve it can hand the claim off and answer 404, after which the URL minted on the old owner never serves again. #527 made memory pressure a trigger; the preview and hand-off sections now say so. * review: own-line comments start uppercase, the retention sweep logs a retried list failure as a warning, the meminfo parse error has its colon The hourly checkpoint TTL sweep logged a store list or delete failure at Error while every sibling sweep (orphan archives, archive deletes, stale moves, tun) logs the same retried failure at Warn. balloonDue's godoc repeats balloonTo's encoding of want. * cut: balloonTo no longer marks the sweep on for an idle target balloonOnce sets balloonSeen on every tick while balloon reclaim is on, before idleOnce starts the tick's hibernates, so the only window the extra store covered is a reload that turns reclaim on and a second reload that turns it off again inside one 5 s tick, with an idle hibernate inflating in between. The test that reached it by resetting balloonSeen by hand goes with it. * review: new(mode) for the fs_write mode, done's godoc names the unwrapped-frame rule, peersTimeout's godoc covers the owner probe, and volumes.go's reply no longer shadows the wire package name lease.done deliberately type-asserts *wire.ErrorResp instead of errors.As (the #524 fix); the godoc now states it so an errors.AsType sweep does not undo it. * cut: SILKD_NET is an Option<bool>, and the egress relay spells its Arc clones as Arc::clone The i8 tri-state copied LANE's encoding, which only an AtomicI8 needs; has_egress now reads SILKD_NET.unwrap_or_else(DEVICE_BACKED). The two Arc clones match the crate's Arc::clone(&x) spelling everywhere else. * review: sysutil declares its consts above the statics that read them, its module doc no longer claims every unsafe block, and the silkd Dockerfile pin comment is one line tun_uring.rs keeps its own SAFETY-documented unsafe for the ring buffer lifetime. * review: socket.timeout is TimeoutError on the package's Python 3.10 floor * ci: base names its one buildx secret, and the carrier merges read the metadata JSON from its env The secret discovery step scanned os-image/<name>/*.sh and the tag dir for a single result: the tag dir holds only the Dockerfile and install-agent.sh is the only script, while any new one needs its own --mount line anyway. The merge jobs pasted steps.meta.outputs.json inside single quotes in run:, which a repo description with an apostrophe would break; DOCKER_METADATA_OUTPUT_JSON carries the same value. build-boot's sha ref reuses the kernel version step output. * ci: os-images drop cache only on a manual no-cache run The per-entry no-cache flag forced a cacheless rebuild whenever a non-Dockerfile input changed, on the premise that such files ride in as buildx secrets. Since #525 only base takes a secret, and base declares INPUTS_HASH before its RUN, so any input change already misses that layer; flavors COPY their files, which buildkit hashes by content. The depth rule also misfired on e2b-ci's systemd units, rebuilding its whole pip install from scratch on a unit edit. * review: compress the kernel fetch, agent unit and W+X narrative comments to their one fact * cut: test fixtures reuse their helpers and constants archivePool(keepAnHour) instead of the literal, mustManager and statusServer instead of hand-rolled copies, abortRecord without its one-use mustStale, a dispatchBlocked that takes no ctx its only caller derives from t, the fake silkd recording detach inside handleExec's lock, set() for the umount hook, fire/lost below the fixture types, tunnelProxy after tunnel's methods, a redundant linux build tag and tar close, a hoisted zero-chunk sum, and a tun test message that says what it checks. The removed api_test.go duplicated TestSecondsSaturatesInsteadOfWrapping through a one-line forwarder. * cut: silkd tests name their connection tuple once and spell Arc clones as Arc::clone * pool: the reap purge benchmark waits out the previous round's teardown reapOnce hands the purge teardown to runBounded and returns, so each round re-added the same 500 ids while the last round's goroutines were still disarming, deleting checkpoints and untracking them, racing the new claims and burning CPU inside the timed region. The untimed setup now waits until every earlier purge has counted its reap. * cut: mcp tests drop the fs_stat replies read_file no longer asks for * pool: an archive row purge records a reap at the row's deadline before the archive_delete The sweep of a departed owner's expired row and reconcileOwnRow's unheld expired row recorded only archive_delete. Inside archived that ends the span at its own t, so archived storage billed from the retention deadline to the purge: up to an hour of sweep lag plus the owner's downtime. A reap carrying the row's deadline ends the span there first, the order 4a0cb95 set for the local purge. The purging node cannot tell a row whose claim a restart already reaped from one still archived, so the first case now journals a second reap; in ended it closes a zero-length span and bills nothing. * sdk/openai: create closes its client when the claim or the session state fails delete already closes the client in a finally (77738dd); create left it open when client.new raised or the state construction failed, so the parked control connections lived until GC. * review: test fixtures name the hour retention, the purge benchmark waits on the reap counter, and setup writes fail with context Five archivePool(3600) sites a4fb52a missed now use keepAnHour; the benchmark's untimed wait built the whole Counters snapshot per spin; four bare t.Fatal(err) setup calls get the context the move tests already have. * ci: silkd's root-only step runs every integration test binary The step listed its test binaries by name, so #524's pty test sat outside it until 45605d4. Every ignored silkd test is root-only and the throughput one stays skipped, so --test '*' runs the same tests today and catches the next new file. * review: tun_e2e groups silkd with the other external crates, as the other integration tests do * review: comments say one fact where they sit, and the kernel fragment's header covers arm64 boot/Dockerfile's mirror note moves above the RUN it explains and its olddefconfig note becomes one line; sandbox.config said it overlays x86_64_defconfig only while arm64 uses it too; install-agent's header splits its facts; deploy.md's 'Its' pointed at the binary; done's godoc drops a clause the type assertion shows; DEVICE_BACKED gets the doc form its neighbours use. * pool: an archive row purge records only its archive_delete again (reverts 513cd63) The purging node cannot tell the fold state of the row's claim. A replicated claim whose node restarted with the registry down journals only its claim, so it is still running in the fold; the reap carrying the row's retention deadline then ended that compute span at the retention deadline instead of the lease's. archive_delete outside archived already caps at the last deadline, so the row purge goes back to it alone; the archived-storage tail past the deadline is left to a fold rule change. * pool: the registry-down restart test follows the orphan replica row through retention and purge The claim journals only its claim, so the purge must not end its running span at the row's retention deadline; the test now pins claim,archive_delete, which fails if a reap comes back. * pool: the meminfo parse error names the field once; each field already ends in a colon
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
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.
Second whole-repo static audit, on
mainat bcca6c8 (after #459-#467): every source file and all 13 flows re-read in full under every lens, with the rules added after #457's misses (every OK window row states its end state, every holder of a shared lock is listed, env-gated tests are checked against CI, tests that assert a loss are checked against the docs, cross-language caps are checked at the boundary). No hardware or E2E run in this round.Bugs fixed (each with a test that fails on the old code)
data:line (~300 bytes each); the 1 MiB line cap broke the stream at every connect on a full node, so liveness fell back to polling for good. Test: a 5,000-VM snapshot (1.26 MB line) synced 0 VMs before.releaseevent was written after the teardown (volume quiesce,vm rm); a restart or crash in that window left a claim event with no end, billed up to the deadline.max_claimsread as none. Move admission already did the lookup.sandboxd_claim_redirects_totalmissed checkpoint redirects.-bench.make go-testnow runs each benchmark once.replicate_secondsnever came due, had no registry row, and was lost with its node.exit,set -e, OOM), its io lock held and the session never reaped.silk-1-mastersbefore,0after).moveBlockerrefused an unsettled hibernate intent but not an unsettled save.Removed (no backward compatibility)
protofield andKeepAliveProtoin Rust, Go, Python and the fixture: nothing reads it since the SDKs stopped probing.Layeregress fallback and thePooledcallback it needed (records older than review: whole-repo round on main 893bf20 #216).Cleanups
Docs and godocs matched to the code, including leftovers #457 itself left (the Python
mountrow, pre-proto-2 sentences, the images trigger path, the secret valuation godoc); restating comments removed; single-caller helpers inlined in the reader router, SDK and silkdtest; an unreachable preview guard and dead lint exclusions removed; test checks that could not fail or could panic on the failure path fixed; the fixture test now ranges the decoder tables.Numbers
Production -55 net (+86 -141), tests +35 net, docs +24 -30. Comment lines in Go and Rust: +11 -25.
Kept without code
DiskAttachSnapshotrefuses a snapshot whose layers and blobs differ from the reader VM's, so the read wakes the claim; no sandboxd mechanism needed.TestS3BackendContractRealEndpointnever runs in CI: MinIO images are no longer publicly pullable; picking a replacement S3 server is an owner decision.Gates
GOWORK=off make go-lint(5 modules x linux/darwin): 10 x0 issues.(golangci cache cleaned first: a warm cache panics SA4023 in e2e)asl -forwarder=false ./...both GOOS, all 5 modules: cleanGOWORK=off make go-test(race, plus one pass of every benchmark): greenmake lint+make teston macOS and inrust:1linux/arm64: green (silkd 165 tests, boot/init 17)