final review - #223
Merged
Merged
final review#223
Conversation
heldConn's comment still named preview after dialPort went generic; keepAlive restated the const block; three restating comments on e2e constants and a helper; three wrapped comments folded to one line; a test helper carried a doc block.
sysutil's private make_controlling_tty sat between two public fns; proc's State enum is vocabulary and now clusters with Chunk ahead of Table.
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.
Whole-repo review round on main f36a3d9, after #220, #221 and #222 landed.
Every file whose blob changed since the last closed round was read in full against the style, judge and LOC lenses (22 style, 17 judge); the hygiene ledger is at 0 due for both. Nothing in executable code needed to change.
What this PR changes
Two
review:commits, comments and layout only:heldConn's doc still said "preview connection" after sandboxd: a guest-port relay and lease renewal #221 madedialPortserve port relays too (the comment dates from pool: keep the idle sweep off a sandbox with a live connection #176);keepAlive's doc restated the const block's WHY; three restating docs on e2e constants and a helper; three wrapped two-line docs folded to one line; a test helper insilkd/tests/carried a doc block. Net −13 comment lines, no executable line touched.sysutil's privatemake_controlling_ttysat between two public fns;proc'sStateenum is vocabulary and now clusters withChunkahead ofTable. Pure moves.What was verified and kept
Renew's archive guard and CAS rollback (Codex P2-2/P2-3) carry three real-trigger tests; a zero-valueclaimSnapshotreachingrecommitis a no-op (commitskipsseq <= written).keepAliveon the hijacked conn is kept: the API server terminates no TLS, so the*net.TCPConnassertion always holds; without it a client that dies without a FIN leaves a held sandbox until lease expiry.forward.rs's independent half-close, theRingtag coalescing,find's byte budget and the Python whole-dial deadline all re-read clean; no unjustified clone in the touched Rust./portsand/renewsurfaces are per-connection and per-call.LOC
Cut-list: empty. The two sub-threshold folds (
decodeBody/decodeOptionalBody−5;HTTPOverPort/relayGet−10 in e2e scaffolding) are recorded as skipped. Shippable Go prod moved +106 across the five PRs; the rest of the range's prod growth (+960) is e2e scaffolding.Gates
make go-lint10×0 issues.;asl -forwarder=falseclean on both GOOS in all five modules;go test -racegreen;GOOS=windows go veton sdk/go green.-D warnings, tests green on macOS and in a linux/arm64 container (silkd 128 passed, boot/init 17).