review: whole-repo round after #177 (fixes, simplifications, docs, cut-list) - #179
Merged
Merged
Conversation
skipValue recursed once per nesting level, so a guest that put a deeply nested value ahead of the type key could grow a host goroutine stack by megabytes per frame. The scan is now a flat loop with a small depth cap; wire values nest two levels at most.
…ive ttl A cut-off or dropped exec answered with the bare error and threw away the output the command had produced; the agent now gets the collected stdout and stderr next to the error field. Each stream keeps its first 1 MiB and flags truncated past that, so a firehose cannot grow the process. A negative ttl_seconds used to reach the node, which clamps it to its own 5-minute default; it is now a bad-argument error.
drain returned on the sandbox's error frame or a dropped connection without running the dial's cleanup, so the relay connection and its context hook lived until the caller's ctx ended; only Close released them.
Since the stream socket lost its timeout (#178), the openai adapter's wait_for could cancel the awaiting coroutine while the worker thread stayed blocked on the relay, and the langchain tool's 5-minute promise had nothing behind it. Both now run the command under timeout(1) -s KILL, which kills the process group at the deadline; the adapter maps exit 124 to the TimeoutError the Agents SDK documents, the tool reports it as exit code 124.
…ertise_addr on a mesh warm defaulted through cmp.Or, so a pool written as warm 0 with a warm_max started at 4 instead of empty; the default now applies only when neither is set. advertise_addr defaulted to the listen address even when that was a wildcard, and a mesh node gossiped 0.0.0.0 or an empty host as its owner address to peers; a mesh config now fails to load until it names a routable host.
…nd WriteTo cleanly A preview token names its owner by advertise_addr; after that address changed, the node forwarded the request to its old name, which routed back to itself and forwarded again until the connections ran out. The forward now marks the request and a marked request that still misses answers 502. proxyLocal rewrote only URL.Path and left the inbound RawPath behind, so a %2F in the guest path reached the guest decoded. auditTee.WriteTo returned io.EOF where io.WriterTo promises nil, and collectExec now accumulates into builders instead of copying the whole output once more into strings.
…ifications handleClaim hashed the pool key on every claim although only the redirect and error paths read it; the digest is now computed where it is used, and writeResult logs the key itself. Also: the orphan-snapshot filter is a slices.DeleteFunc, confirmGone names its logger once, SetPools and the egress dialer wrap their causes with %w, ClusterDigest reuses DecodedKey instead of decoding the cluster key again, portconn's lone constant uses the bare form, and redirectFallback is a plain loop without the retryAny predicate that existed only for it.
…opy, latch a present lane verdict scan_file checked the size through metadata and then read the file unbounded, so a file growing between the two calls was read whole; the read now stops at FIND_MAX_FILE on the same handle. rand_token fell back to a per-process counter when no entropy source answered, which made the session marker forgeable by the shell's own commands; session.create now fails instead. routes_directly re-read /etc/silkd-lane on every exec of a direct-lane guest; the host writes the file once per claim, so a present verdict is latched and only its absence is re-read.
fs.replace expands capture groups through a counting Replacer instead of allocating a String per match; the pty pre_exec registration moves into sysutil so every unsafe block lives in one module; tree's stderr cap is a file-level constant; Table::get is crate-visible; boot-init renders its marks into one String, trims the MAC in place, and formats the device path only for a serial that matched. Doc lines trimmed to one fact.
Inline comments open lowercase in the STE register (61 lines); godocs that restated a short body are gone (checkpointURL, probeMAC, guardsEgressLane, validateArchiveWindow, synced, goldenCAMatches, two desktopsmoke steps) and multi-sentence comments in the wire package and the Go SDK are one line each. The openai adapter lists its public methods before the SDK hooks; two Python **fields parameters are typed; three test fakes close their socket, narrow a suppress, and report the observed op to the main thread instead of asserting inside it.
…gress doors off a released claim Checkpoint, Promote and Fork of an archived claim ran the engine with an empty VM name and Hibernate journaled a bogus intent onto it; they now answer ErrArchived (409) and Hibernate is the no-op it already is for a hibernated claim. archive() and commitWake re-set the record on a failed commit even when a concurrent Release had deleted it, which resurrected the claim in claims.json; the rollback is now gated on the claim still being live. armEgressProxy closes the listener it displaces, and resyncEgress leaves hibernated and archived claims to their wake, so a guarded claim woken after a restart no longer leaks its startup door; finalizeBatch disarms a claim released in its own arm window. Also: an oversized request frame records as op oversized instead of escaping the audit journal; the pool passes no CA to a proxy whose pool never intercepts (one Transport clone and a leaf map less per claim); Sandboxes sorts outside m.mu; idleEnabled/archiveEnabled are atomics (read off m.mu by the sweeps); ClaimWarm kicks the refill before it answers ErrNoWarm; sweepExpiredCheckpoints owns its TTL guard; DeleteCheckpoint asks archiveCkPinned; quiesce gives the fallback sync its own budget; a persisted pool that fails validation names pools.json.
…ped leaf cached r.Clone carried the guest's Connection: close into the upstream request, so an HTTP/1.0 guest or one sending the header paid a fresh upstream connect (and TLS handshake on the intercept path) per request; the relay clears out.Close like httputil.ReverseProxy does. A leaf whose end is clamped to the intermediate's own could never be renewed, so once the intermediate entered its last 24 h every intercepted session re-signed; such a leaf stays cached.
…ied owner address A preview token names its owner by the advertise address at mint time; a node that holds the claim now serves it locally whatever that string says, and the one-hop marker stays as the backstop. The raw path suffix is derived by segments, so a %70 spelling of the prefix keeps %2F too. A claim response and /owner no longer emit a wildcard advertise address (:7777, 0.0.0.0) as owner_addr: an empty owner makes both SDKs reuse the address they dialed, which is what a default single-node config needs. The audit tee records an oversized first line as such instead of dropping it; writeResult keeps a string id (the template name), and ErrArchived maps to 409.
read_file buffered whatever the guest streamed, so a device or a multi-gigabyte file grew the MCP process the way exec used to; it now stats first and refuses anything but a regular file up to 1 MiB with a pointer to exec head/tail. The tool tests share one fake node and a reply decoder.
Pty.drain returned on the exit or error frame without the dial's cleanup, the same leak Watcher had; the drain now owns it.
Sandbox.run and exec take timeout, a wall clock over the dial and the run: a watchdog cuts the connection at the deadline, which makes silkd kill the command, and TimeoutError is raised; the dial itself is bounded by the remaining time. The openai adapter passes its per-call timeout straight through, and the langchain tool carries one deadline across the lazy claim, the dial and the command, so neither depends on timeout(1) in the guest image or on exit code 124.
fs.replace still sized the file by path and then read it whole by path; both verbs now share read_bounded, which opens once, reserves the size it measured and stops at FIND_MAX_FILE.
Marks.render goes back to the iterator form (a once-per-boot path with at most seven marks), and routes_directly latches through Option::inspect.
README lists the boot-init and shell workflows; index drops the network adb path the android flavor never had; the desktop pages name the whole app set; performance records the refill-time door bind; sdk-python states the peer-heal TTL requirement and the run/exec timeout; the adapter, langchain, mcp and API pages describe the exec timeout, the capped exec result, the bounded read_file, the archived 409 and the oversized audit record.
…n close ReadFileTo hands each data frame to an io.Writer and stops when the writer refuses, so a caller can bound a read without buffering it. Run sent stdin_close synchronously and returned its send error before reading anything, so a guest that had already answered and closed cost the caller the exit code it had sent; the frames are read regardless.
The stat-then-read guard was not a bound: a file can grow or be swapped between the two RPCs. read_file now streams through the capped writer and stops the read at 1 MiB with the same pointer to exec head/tail.
The ErrArchived prechecks in Checkpoint, Promote and Fork ran before sb.Transition was taken, and archive() clears VMName under that lock, so a capture racing an archive could still snapshot an empty VM name. The check now lives in sourceSnap, which every capture path calls under the lock; the prechecks are gone.
…budget A lazy claim that consumed the five minutes handed the command a one-second timeout; the tool now reports the cut-off instead.
fs_read opened whatever the path named: a device streamed forever and a FIFO without a writer parked a blocking-pool thread in open for the life of the daemon. A path that is not a regular file now answers bad_request before the open.
read_file stats first and rejects a directory, device or FIFO with the kind in the message; the streamed 1 MiB cap stays the size bound.
… the transition lock archive() clears VMName under sb.Transition; the two events read it after their capture released the lock.
The MCP read test reuses the hijack's err instead of shadowing it; the pool's lockedVMName sits below the Manager method set.
A path could be swapped for a FIFO between the stat and the open, and a writerless FIFO still parked a blocking-pool thread there. fs_read now opens with O_NONBLOCK and fstats the descriptor it will stream, so the kind it checks is the kind it reads; the test adds a FIFO to the directory and /dev/null cases.
Close closed only the guest side of a CONNECT or SOCKS5 tunnel. When the upstream stayed silent after the half-close, the copy goroutine, the upstream socket and the sandbox hold outlived the proxy for the life of the daemon.
… finish A wake whose commit landed before a concurrent Release removed the claim returned without dropping the hibernate image it had just resumed from; Release saw an empty HibernateSnap and left it too, so the image stayed on disk until the next restart's Reconcile. No cheap test: the window sits between the commit and the release check with no seam to hold it open.
DeleteTemplate released its lock with recDone, so the per-id mutex stayed in recLocks after the record was gone, and resolveGolden created one for every key it looked up and did not find. The checkpoint paths already evict on delete and gate the lookup before locking.
Egress: the audit record's field names, the SSRF guard's NAT64 handling, which lane a tenant policy may run on, the CONNECT rule wording, the archive wording and the internal-range file. Security and silkd pages name the SOCKS5 door and every not_found case. API: PUT /v1/pools 400 cases, the probe replay window, archive-backed checkpoints, usage-event tenant fields. Cluster gossip digest and epoch rule; the claims-journal write lock. SDK pages: which node verbs need the root token, the idle sweep's live-connection hold, spawn bad_request kinds, the checkpoint delete peer drop (and the Go godoc's stale TTL clause), the Python lookup, proxy_port and adapter state; langchain install and write contract; MCP defaults, per-call cap and child lifetimes. README: sh-lint target and the arm64 kernel; desktop, browser, benchmarks and image READMEs match their Dockerfiles and scripts.
The idle-sweep sentence promises only what the sweep's re-check observes; a bare egress rule reads as admitting CONNECT; the checkpoint-delete paragraphs and the Go godoc carry the one reachable zero-TTL case (a node later run with healing off); the PUT /v1/pools 400 list names negative archive durations.
finalizeBatch re-took m.mu after arming every guarded claim to catch a release inside the arm window. Only a root client that lists and deletes the id within that sub-millisecond window can reach it, and the residue is one door listener and one nft table until the next restart; the wake paths keep the check, where the claimant holds the token and the window is hundreds of milliseconds.
Both store backends returned func() {} from Fetch and every caller composed it into a no-op; the read pin is the pool's record lock. The pty finish guard tested a state only finish sets, from two mutually exclusive call sites. The other cut-list entries stay: the sweep skeletons save under ten lines for a generic helper, the two SDK options are published API, and the silkd helper the ledger named does not exist.
releaseResolved ran MkdirAll and WriteFile under m.mu for an archived claim; reapOnce already writes the same marker after unlocking. The marker is written first and the claim re-checked under the lock, looping if the archive checkpoint changed meanwhile, so a wake or archive racing the release still ends with a marker for the checkpoint the release removes.
The opt-in sentence said every rule without CONNECT is rejected; a rule with an empty methods list admits it, as Validate checks.
The previous commit left the call one column past the 120-character limit, which fails ruff format and E501 in CI.
CMGS
added a commit
that referenced
this pull request
Sep 14, 2026
…ulk-frame cases (#180) Follow-up to #179: the three items its review round classified as fix-now. - pool/watermark: arrivals are counted per bin of at least one second and the bin's arrivals per second fold into the rate EWMA, with the stored rate decayed over the bin first; two claims a millisecond apart no longer read as 300/s and pin the target at warm_max for minutes. Testbed: two back-to-back claims left the target at the floor (base: 8, six extra VMs); ~14 claims/s for six seconds raised it to 4 (2 x rate x the 170 ms lead) and it returned to the floor within a minute of silence (base: still 8 after three minutes). - pool/egress: each door serves at most 256 connections per sandbox (LimitListener wrapped after the pre-bind probe) and the upstream transport keeps at most 64 idle connections per sandbox. Testbed: a 300-connection flood left the daemon at 272 descriptors instead of 316, a sibling sandbox's fetch was unaffected and the flooder's 301st request waited unserved. - sdk/python: the bulk-frame fast path's rejection cases mirror the Go decoder's. Seven-leg E2E regression green on the branch binary twice; Codex: "No blockers remain for the follow-up branch."
This was referenced Sep 14, 2026
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.
Whole-repo review round on
main(3db7456, after #176/#177): every Go, Rust and Python source file and every docs page read in full by lens readers (style, correctness/concurrency/hot-path/over-design judgment, LOC justification, docs-vs-code), every finding adjudicated against the source, fixes and simplifications applied in themed commits, four /simplify lenses and seven Codex rounds over the diff, and the PR's own sandboxd + silkd run through the full E2E regression on the testbed.Fixes (each with a regression test unless noted)
skipValuerecursed once per nesting level: a frame with a deeply nested value ahead of the tag key grows a host goroutine stack by megabytesmaxSkipDepth16 (wire values nest two deep)execanswered a cut-off or dropped run with the bare error and threw the output away; stdout/stderr builders were unbounded; a negativettl_secondsreached the node and was clamped to its 5-minute defaulterrorfield (isErrorstays true); each stream keeps its first 1 MiB and setstruncated; negative ttl is a bad-argument errorread_filebuffered whatever the guest streamed (a device, a multi-GB file)Sandbox.ReadFileTo, 64b578c) and stops the read at 1 MiB with an error pointing at exec head/tailWatcher.drainandPty.drainreturned on the sandbox's terminal or error frame without the dial's cleanup, so the relay conn and its context hook lived until the caller's ctx endedRunsentstdin_closesynchronously and returned its send error before reading anything, so a guest that had already answered and closed cost the caller the exit code it had sentwait_forcancelled the coroutine while the worker thread stayed blocked on the relay, and the langchain tool's "cut off after 5 minutes" had nothing behind itSandbox.run/exectaketimeout, a wall clock over the dial and the run: a watchdog cuts the connection at the deadline (silkd kills the command on disconnect) andTimeoutErroris raised; the adapter passes its per-call timeout through, the tool carries one deadline across claim, dial and command and refuses the command when the claim used it up (d66e083);timeout <= 0is aValueErrorwarm: 0under awarm_maxbecame 4; a mesh node with a wildcardlistenand noadvertise_addrgossiped0.0.0.0:portas its owner addressadvertise_addrnames a routable host/ownerhanded a wildcard advertise address (:7777) to remote SDK clients, which dialed their own machineadvertise_addrchange a token's owner string no longer matched and the node forwarded to its old name, which routed back to itself, forever;proxyLocalrewrote onlyURL.Path, so%2Freached the guest decoded;auditTee.WriteToreturnedio.EOFwhereio.WriterTopromises nil%70prefix keeps%2Ftoo); WriteTo returns nil at EOFVMNameafter their capture released the lockErrArchived(409), checked insourceSnapundersb.Transition(the lockarchive()clearsVMNameunder); Hibernate is the no-op it already is for a hibernated claim; the events read the name under the lockarchive()andcommitWakere-set the record on a failed commit even when a concurrent Release had deleted it, resurrecting the claim in claims.jsonarmEgressProxyoverwrote the listener without closing it; a release inside a claim's own arm window left a door and an nft lock behindresyncEgressleaves hibernated/archived claims to their wakeidleEnabled/archiveEnabledwere plain bools written underm.muand read off it by the sweeps; a zerockptTTLsweep would delete every checkpoint if called unguarded; the quiesce fallback sync ran on a budget the unmounts had spentoversized;atomic.Bool; the guard lives insweepExpiredCheckpoints; the sync gets its ownVolumeCallTimeoutConnection: close, so an HTTP/1.0 guest or one sending the header paid a fresh upstream connect (and TLS handshake on the intercept path) per request; a leaf clamped to the intermediate's own end could never be renewed, so once the intermediate entered its last 24 h every intercepted session re-signedout.Close = falselikehttputil.ReverseProxy; such a leaf stays cachedProxy.Closeclosed only the guest side of a CONNECT or SOCKS5 tunnel; an upstream that stayed silent after the half-close kept the copy goroutine, its socket and the sandbox hold alive for the life of the daemonHibernateSnapand left it until the next restart's ReconcileDeleteTemplatekept the template's record-lock slot after the record was gone, andresolveGoldencreated one for every key it looked up and did not findreleaseResolvedranMkdirAllandWriteFilefor the archive-delete marker underm.mu, the lock every claim, refill and reap tick contends;reapOncealready wrote the same marker after unlockingfs_readopened whatever the path named: a device streamed forever and a FIFO without a writer parked a blocking-pool thread inopenfor the life of the daemonO_NONBLOCKand the kind check runs on the opened descriptor, so anything but a regular file answersbad_requestand a swap between check and open changes nothing (every client, not only MCP)findsize-checked through metadata and then read unbounded,replacedid the same by path;rand_tokenfell back to a per-process counter when no entropy source answered, so the session marker was forgeable by the shell's own commands;routes_directlyre-read/etc/silkd-laneon every exec of a direct-lane guestread_boundedhelper (open once, reserve the measured size,take(FIND_MAX_FILE));rand_tokenreturnsio::Resultandsession.createfails instead; a present lane verdict latches, only absence re-readsReview commits (no behavior change intended)
handleClaimno longer sha256-hashes the pool key on every claim (only redirect and error paths need it;writeResultlogs the template). Also: orphan-snapshot filter viaslices.DeleteFunc, one logger inconfirmGone,%wwraps inSetPoolsand the egress dialer,ClusterDigestviaDecodedKey, bare-form lone const inportconn.go,redirectFallbackas a plain loop (retryAnyexisted only for it).Transport.Clone, atls.Configand a leaf map less per claim; one fewer policy walk per CONNECT),Sandboxessorts outsidem.mu,ClaimWarmkicks the refill before it answersErrNoWarm,DeleteCheckpointasksarchiveCkPinned, a persisted pool that fails validation names pools.json.fs.replaceexpands capture groups through a countingregex::Replacer(noStringper match); the ptypre_execregistration lives insysutil::adopt_controlling_tty, so everyunsafeblock sits in one module; tree's stderr cap is a file constant;Table::getispub(crate); boot-init trims the MAC in place and formats the device path only for a serial that matched.**fieldstyped; three test fakes tidied.finalizeBatchno longer re-takesm.muafter arming a guarded claim to catch a release inside the arm window: the claimant has no token until the claim returns, reap needs a deadline at least a second away, the sweeps skip a just-touched claim, so only a root client that lists and deletes the id within that sub-millisecond window reaches it; the residue is one door listener and one nft table until the next restart. The wake paths keep the check.store.Store.Fetchloses its release value and silkd's ptyfinishits dead guard.not_foundcase;PUT /v1/pools400 cases, the probe replay window, archive-backed checkpoints, usage-eventtenant; the gossip digest and epoch rule; the claims-journal write lock; which node verbs need the root token, the idle sweep's live-connection hold, spawnbad_requestkinds, the checkpoint delete peer drop (and the Go godoc's clause describing a state config.Load refuses), the Pythonlookup,proxy_portand adapter state; langchain install and write contract; MCP defaults, per-call cap and child lifetimes; READMEmake sh-lintand the arm64 kernel; desktop, browser, benchmarks and image READMEs against their Dockerfiles and scripts.Findings adjudicated without code
Metasfails the whole listing on an unreadablemeta.json(dir, s3)budget/len(owners)leaves leftovers unusedhealBudgetis 30 min, four owners still get 7.5 min eachtarIntoskips non-regular entries silentlyTestTarSkipsSymlinksAtSourcepins that contract and untar mirrors itTestClaimProvisionOneVolumeAttachFailureFailsWholeClaimandvolume_attach_testpin both; an rw claim recovers a marked image, and a marker no unmount clears would lock readers out foreverproxyConnwaits for the client to close after guest EOFdrainGraceio.MultiReaderprefix "kills the splice"multiReader.WriteToexists since Go 1.22; the splice runs per sub-reader, andauditTee.WriteToexists for the same reasonexecKillWait, and that exec already failed/sys/blockserial per pollLockisUnlock+Lockin two transactionsLockruns at refill, off the claim pathreached()probe undersyscall.ForkLockon the claim pathAccept4(SOCK_CLOEXEC)is Linux-only and deserves its own A/B (perf ledger)relaymark (Codex MAJOR)relayonly for a tap it just locked; a CNI-lane claim adopted under a bridges config is quarantined and removed before that (its tap lives in the VM netns), and refill warmups never latch because the file is absentWarm intcannot tell absent from explicit 0 (config vsPUT /v1/poolssemantics)warm: 0underwarm_maxMAX_FRAMEkills the terminal frameHijack's buffered reader is discarded, so bytes a guest pipelines ahead of the CONNECT 200 are lostIssueIntermediatedoes not check the root key against the root certLoadCAand re-minting convergesTransitionlive-captures the running VM before the re-check discards itcommitWakegives a woken archive a fresh 5-minute lease whatever the original TTLsync.Once→sync.OnceFuncin a test fixture; PythonClientper adapter call; conftest import grouping;hostPort/e2e step renames; silkdwrite_frameVec, fan-outchunk.clone(),net_egressspawn per loopback conn;hibernateLocked'sm.mutake; per-batchKey.Hashin finalize (0.3 µs)/loc-justify — cut-list re-checked against the source, two entries applied
Numbers (
git ls-filescounts, base = 3d70391 one month back):Comments are not the story (below 10% everywhere). Test scaffolding counted as prod by the commands: e2e/ 2733, silkdtest/ 621, storetest/ 197. Growth since base is feature commits (#148 +221, #161 +180, #127 +159, #129 +156, #177 +79, #176 +63); every review/cut commit in the range is net-negative or zero prod except two review rounds (+31, +30).
Cut-list as first reported, with the verdict after re-reading every entry against the source (4b8db0b applies the first and the last; the prod saving is 4 lines, the rest is test plumbing and shorter lines — the point is the phantom release contract, not the count):
store.Store.Fetchrelease funcfunc() {}; every caller composes a no-op; the read pin is the pool's record lock — APPLIED in 4b8db0b (−4 prod, −30 test)archiveOnce/idleOnceshared sweep shapefs.rs/tree.rsdone_or_errduplicateWithHTTPClientWithSessionEnvpty.rsfinishguardfinish, whose two call sites are mutually exclusive — APPLIED in 4b8db0bDropped from the earlier cut-list:
auditTee.WriteTo(−19) is load-bearing — it restores the splice after the first audited line.Follow-ups (not in this PR)
Perf ledger rows for #172: wire
fastBulkper-frame alloc+copy (append-decode into the caller's buffer); usage/audit journal appends under one mutex with an unbuffered write on the claim and data paths; the reap tick's three O(live) scans underm.muper 5 s; O(live) claims.json rewrite per claim; one usage line per proxied egress request (a guest can roll the journal);reached()underForkLock.Issues to open: store/peer probe double fan-out on a miss; fleet delete resurrecting through a down peer (tombstones); s3
SweepGenerationsno-op and the missing storetest retention contract;warmpresence (config defaults to 4,PUT /v1/poolsmeans 0); the watermark's 1/dt EWMA pins atwarm_maxafter any concurrent pair for minutes;releaseResolvedmarks an archive ck underm.mu; host-side per-sandbox relay fan-out cap; Python wire-binding tests lack the bulk-frame cases; a per-sandbox cap on egress door connections (aLimitListenerwith a chosen cap); a golden tied to its template's identity (a re-import under the same tag keeps a stale golden today; needs an engine-side image digest); a Reconcile sweep for the golden a removed pool leaves on disk; the tap's nft lock taken at refill instead of claim (a perf row for #172, constrained by the hibernate rule).Gates (full suite on 1f275a0; sandboxd re-linted and re-tested on both GOOS and silkd re-gated on mac and in the linux/arm64 container on every later commit through e6fc001)
Round LOC (
git diff --numstat main...HEAD, 38 commits): prod +216 net (fix commits +275, each justified by its row above; review, cut and docs commits −59, of which a6d0faf +17 for the counting Replacer and the unsafe wrapper), tests +554; comment lines +124/−146 across the branch./simplify lenses on the diff
Four agents over
git diff main...HEAD(reuse, simplification, efficiency, altitude); every finding applied or skipped with a reason:slices.DeleteFuncpredicate in its positive form (applied);Marks.renderback to the iterator (applied);routes_directlyviaOption::inspect(applied); preview table test (applied);writeResultback to a string id (applied); countingReplacer— kept, it removes one allocation per match;defer w.Close()inWatcher.drain— skipped, errcheck noise for zero gain.writeResultboxing — resolved by the string id;key.Hash()twice on a miss that then hits quota — skipped, it follows a full provision attempt;take()losingFile's size reserve — applied (read_boundedreserves the measured length); eagererr.Error()in the tool-call error path — applied.replacesharingfind's bounded read — applied;Pty.draintwin of the watcher leak — applied;read_filebound at the point a guest stream becomes a reply — applied (streaming read, hard cap); Python guest-sidetimeout(1)— replaced by an SDK-level deadline;Warmpresence — issue; wildcard owner_addr for single nodes — applied (blank when unspecified); preview locality by holding the claim, not by string compare — applied; silkd direct latch vs a late relay mark — [contrived], see the Codex row.Codex
Round 1 (on 05b0bda): two MAJOR, five MINOR. The adapter wall-clock MAJOR and the three timeout MINORs are fixed by the SDK-level deadline (c209a6d); the
%70prefix MINOR by segment stripping (098804d); the mcp.md MINOR by 5f0f600. The silkd direct-latch MAJOR is pushed back with the code: the only post-claim writer of the lane file is Reconcile, which marksrelayonly for a tap it just locked, and a CNI-lane claim adopted under a bridges config is quarantined before that because its tap is not in the root netns.Round 2 (on 5f0f600): two MAJOR, one MINOR. The archived-claim precheck outside
sb.Transitionis fixed by 7659b82 (the check moved intosourceSnap, which every capture path calls under the lock); theread_filestat/read TOCTOU was already fixed by dc453a8 (streaming cap); the langchain deadline MINOR is met halfway by d66e083 (the tool refuses the command when the claim used the budget; carrying an absolute deadline through the Python client's redirect walk is a client API change left to a follow-up).Round 3 (on d66e083): one MAJOR, two MINOR.
read_fileof/dev/nullanswered empty and a writerless FIFO would block in silkd's open: fixed at both ends (f8f7d64 in silkd, 235d7e4 in MCP). The fork/checkpoint usage events' unlockedVMNameread: fixed (7d2b509). The langchain redirect-walk deadline stays a follow-up.Round 4 (on 7d2b509): one MAJOR, two MINOR. The silkd stat-then-open window (a path swapped for a FIFO between the two) is closed by 3cc28e7: the open is
O_NONBLOCKand the kind is checked on the opened descriptor; a FIFO joins the test. The usage-eventVMNameMINOR is accepted as is (the locked read is consistent; an empty name means the parent was archived mid-fork); the langchain redirect walk stays a follow-up.Round 5 (on 3cc28e7): one MINOR (the same langchain redirect-walk deadline, a Python client API change left to a follow-up) and the closing sentence verbatim: "No blockers remain for the review branch."
Round 6 (on 1f275a0): five MINOR, no MAJOR, and the closing sentence verbatim: "No blockers remain for the review branch." Four of the five are docs precision and are applied in 5a82908: the idle-sweep sentence now promises only what the sweep's own re-check observes (a connection live at the check), the CONNECT rule wording says "a nonempty
methodslist that omitsCONNECT" so a bare rule reads as admitting it, the checkpoint-delete paragraphs and the Go godoc carry the one reachable zero-TTL case (a node later run with healing off and the TTL back at 0), and thePUT /v1/pools400 list names negative archive durations. The fifth is the langchain redirect-walk deadline, the same follow-up as rounds 2 to 5.Round 7 (on e6fc001): three MINOR, no MAJOR, and the closing sentence verbatim: "No blockers remain for the review branch." The SOCKS5 opt-in sentence in
docs/egress.mdis qualified to "a nonemptymethodslist" in ef9a3f6. The claim-path liveness check Codex asks to restore needs attl_seconds=1guarded claim whoseclaims.jsoncommit takes longer than a second while the reap tick lands in that window; the residue is one door listener and one nft table until the next restart, so it stays removed as [contrived]. The langchain redirect-walk deadline is the same follow-up as rounds 2 to 6.Hardware
Isolated kit on the bare-metal testbed (pinned cocoon binary, sqlite meta, its own root/run/log dirs, bridge
sbxbr0for the egress lane), guest images built as overlays ofrt:24.04anddesktop:24.04with this branch's silkd, host binaries built from this branch (GOOS=linux GOARCH=amd64 CGO_ENABLED=0). Seven full runs: at 05b0bda, dc453a8, d66e083, 7d2b509, 3cc28e7, e8bcac7 and e6fc001, the last code commit (the final HEAD ef9a3f6 adds one docs sentence; host binaries rebuilt per run; guest images rebuilt at 5f0f600, 7d2b509, 3cc28e7 and e6fc001 for the silkd changes). Every leg passed every time; the block below is run 7.Warm claims in the smoke: 0.3–1.6 ms across the seven runs.
Warm-claim A/B on the same testbed class (claimab2, warm 8, 6 × 8 claims per daemon start, A B B A × 3, n = 288 per arm and config; A = main 3db7456, B = this branch at 5a82908), p50 | p90:
Consecutive same-arm rounds differ by up to 150 µs on the guarded configs, so every difference above sits inside the noise: the branch's claim-path changes (one digest less per claim, no interception transport for a pool that never intercepts, one map lookup more on guarded pools) are below what this harness resolves, and the no-policy path is unchanged.