alksocks/docs/reviews/003-assembly-layer-implementation-review.md

status, last_updated, reviewed_artifacts, tool, reviewer, scope_note

status	last_updated	reviewed_artifacts	tool	reviewer	scope_note
resolved	2026-09-27 (the residue round closed — S-1/N-1/N-2 doc notes, D-1/D-2/D-3 + the N-8 probes landed as tests, the frontmatter YAML fixed; every finding closed or re-dispositioned)	src/door.rs src/local/mod.rs src/local/producer_door.rs src/composed/mod.rs src/consumer.rs (the batch-2 additions — TokenClient + the *_with_token twins) src/producer.rs (spot-check — the gate/builder surface) src/rfc1928/ (spot-check — the cred-bearing door's forked surface) docs/architecture/ (overview, wire, producer, consumer, rfc1928, README, decisions 003–014) tests/ (all batch-2 suites read; coverage attributed)	manual source read + cargo test/clippy/fmt/doc/wasm/publish-dry-run + cargo-llvm-cov + alktunnels 0.2.0 surface cross-checks	general implementation review (correctness + security + smells + coverage)	the batch-2 assembly layer only — the five new modules plus the batch-2 additions to consumer.rs/producer.rs; the batch-1 core was reviewed by review 002 and is not re-litigated here

Review 003 — The Assembly-Layer Implementation (Post-Landing Full Review)

Purpose

General review of the batch-2 assembly layer after landing — the eight-task group (tasks/implementation/{local-*,door-*,composed-*, assembly-integration-suite,review-assembly-layer}.md, commits a867f0c..04c76e3). The narrow review (tasks/implementation/review-assembly-layer.md) checked the eight gate criteria (never-binds, seam contracts, door fidelity, graduation shape, conventions, feature hygiene, POC fidelity, docs sync) and fixed R-1/R-2 inline; this one asks the four questions that review did not, following review 002's template:

1. Correctness — hazards in the landed door/provider behavior itself: teardown races, pump-lifetime paths, protocol-state corruption, error-shape fidelity at the door boundary.
2. Security — resource exhaustion on the kernel-socket paths the batch-2 code newly opens (doors, producer backend), gate fidelity, panic surfaces, log/credential handling.
3. Code smells — convention compliance plus the structural smells an 8-task implementation round leaves behind (dead surface, duplication, doc drift, test-side posture).
4. Coverage (cargo-llvm-cov) — important gaps, attributed to specific behaviors and their risk.

Method note: line-level over the five batch-2 modules + the batch-2 diff surface of consumer.rs/producer.rs, with cross-checks against alktunnels 0.2.0 (make_forwarded_op_handler, ForwardedHandler, TunnelListener, TakenHalves) and the ADR set 003–014. The RFC 1928 core and the protocol-layer pump are re-read only where batch 2 calls into them.

Verification Baseline

All commands on the reviewed tree (post-fix; the review change itself):

• cargo test: 375 green (default; the batch-2 suites are feature-gated out of the default build).
• cargo test --all-features: 620 green (up 5 from the gate's 615 — the review's pinning test + the UDP-door exit change).
• cargo clippy --all-targets --all-features -- -D warnings: clean.
• cargo fmt --check: clean.
• cargo doc --no-deps: clean (0 warnings).
• cargo publish --dry-run --allow-dirty: completes.
• Wasm checks: default + --features composed clean (check + clippy); local on wasm fails with exactly the expected mio unsupported-wasm error (the alktty posture, observed at the gate and re-verified).
• cargo llvm-cov --all-features: 93.68% lines / 91.01% functions / 92.97% regions (6485 covered / 410 missed).

Coverage breakdown

Module	Lines	Functions	Regions
composed/mod.rs	77.32%	65.31%	80.45%
consumer.rs	95.25%	93.60%	93.88%
door.rs	81.45%	81.08%	79.05%
egress.rs	99.67%	100.00%	99.70%
error.rs	100.00%	100.00%	97.48%
local/mod.rs	84.91%	74.29%	86.94%
local/producer_door.rs	76.98%	84.21%	77.46%
params.rs	100.00%	100.00%	100.00%
producer.rs	95.70%	91.83%	96.74%
rfc1928/ (all five)	96.5–100%	95.65–100%	78.6–99.6%
wire.rs	96.58%	97.06%	97.19%
TOTAL	93.68%	91.01%	92.97%

Healthy overall; the batch-2 modules carry the lower numbers (D-1, D-2, N-8 attribute them). The protocol core's residue is unchanged from review 002's attribution; the assembly layer's gaps are the findings below.

Summary Statistics

Severity	Count	IDs
Critical	0	—
Major	2	F-1, F-2
Minor	4	S-1, D-1, D-2, D-3
Observation	8	N-1..N-8

No critical findings. The two majors (F-1, F-2) are door-boundary fidelity/teardown corrections, both fixed in the review change: F-1 is the cred-bearing door silently swallowing the in-band refusals' mapped reply codes (a vanilla client got a bare EOF where ADR-007 pins a reply-code-shaped refusal); F-2 is the UDP door's fatal-I/O swallow (serve_on returned Ok(()) on a relay-socket failure the docs call fatal). One coverage finding (D-2) is the batch-2 analogue of review 002's D-2 (the spec'd surface unexercised); the rest are small.

Resolution (2026-09-27): the residue round closed the review — S-1/N-1/N-2/N-3 as the recommended doc notes, D-1/D-2/D-3 as the recommended tests, N-8's worthwhile probes landed (the remainder accepted-uncovered with the disposition recorded per section); the frontmatter's invalid YAML (plain-scalar colons in reviewed_artifacts) fixed. Full verification green; see the disposition table for commit pointers.

Prefixes follow the review-002 legend: F (correctness, fixed inline — continuing 002's C family with the batch's review-fixed disposition), S (security posture), D (dead surface/coverage), N (observations/notes/smells).

---

Part A — Findings

F-1 [major, fixed] — The cred-bearing door swallowed every in-band refusal's mapped reply code: a vanilla client got a bare EOF where the RFC (and ADR-007's failure mapping) promises a reply-code-shaped refusal

Files: src/door.rs (cred_bearing's CONNECT/BIND session-open error arms + bind_quiet_and_pipe's accept-stage error arm; pre-fix state at git 04c76e3)

Problem: ADR-007 pins the failure mapping as RFC-faithful end to end: factory failure → reply#1's mapped code; accept-stage failure → reply#2's mapped code. The producer honors this in-band — but the cred-bearing door sat between the in-band conversation and the vanilla client and dropped the mapped code on the floor at three sites:

1. connect_with_token refused (dial refused per target policy — the in-band CONNECT reply carried 0x05/0x02/0x03/…) → the door returned DoorExit::Open(err.to_string()) and the vanilla connection closed with no reply at all.
2. bind_with_token refused (the in-band BIND command gate's 0x02 — an identity with socks5:open but not socks5:bind) → same: bare EOF where a vanilla client expects reply#1 ConnectionNotAllowed.
3. await_inbound()'s accept-stage failure (Err) — the quiet wait's accept refused per policy — the door closed the connection with no reply#2.

The producer-side in-band paths (the pump handler, serve_socks5_over) write the RFC reply before returning the session error; the door — the one surface whose whole job is RFC codec translation — discarded it. The observable: curl --proxy socks5://user:pass@host against a target-policy refusal hangs-then-drops instead of printing "proxy CONNECT aborted" with a SOCKS error — and, worse, a BIND command-gate refusal (the ADR-006 security boundary working as designed) was indistinguishable from a crashed proxy. The door's own module doc promises "the RFC codec lives only here" and "zero protocol translation" of payloads — refusal re-encoding is the codec's job.

Fix (landed in this review change): a write_refusal_reply helper (the door's boundary codec, best-effort — a failed write means the client is gone either way) re-encodes SocksSessionError::Rfc( ReplyError)-shaped refusals to the vanilla client with the RFC's placeholder BND.ADDR (0.0.0.0:0, the same form reply_error uses), at all three sites (CONNECT open, BIND open, the accept-stage Err). Non-reply-code shapes (open/transport/I-O) stay a bare close (nothing RFC-shaped to encode). Pinned by cred_door_in_band_refusal_ surfaces_the_reply_code (a socks5:open-only credential BINDs → reply#1 carries 0x02 + the placeholder address, within a timeout — the old shape failed the test by never writing a reply).

Lift: closes the last wire-fidelity gap between the in-band and vanilla surfaces. Every other door path re-encodes its reply (pipe_connect_plane, pipe_bind_plane, the UDP door's codec); the refusal paths were the asymmetric ones.

---

F-2 [major, fixed] — UdpFrontDoor::serve_on returned Ok(()) on a fatal relay-socket I/O failure, against its own contract

Files: src/door.rs (UdpFrontDoor::serve_on's final Ok(()), pre-fix at 04c76e3); doc: "Runs until the association ends (the in-band channel EOF, the client idle bound, or a fatal I/O failure)"

Problem: the two-pump select! resolves with a UdpDoorExit; an Io exit (a real relay-socket failure — recv_from erroring on a non-idle socket) is documented as one of the run's fatal outcomes, but the method mapped every exit to Ok(()). The caller (assembly wiring, the serve wrapper) cannot distinguish "the association ended cleanly" from "the relay socket died" — the exact distinction the doc's contract claims to carry. The TCP door has the same return shape but its per-accept isolation posture makes per-connection I/O failure genuinely non-fatal (the loop continues); the UDP door is one association per call — an Io exit there is the call's outcome. Latent: nothing in the suite exercised a relay-socket error, so the swallow passed every test.

Fix (landed in this review change): the exit match surfaces UdpDoorExit::Io as Err(err); the idle bound, the association end, and a session error stay clean Ok(()) (the association's own ends — each already logs at debug). All UDP-door tests unchanged and green (the idle-bound and teardown tests assert clean exits, which the match preserves).

Lift: the caller-visible contract matches the doc; an assembly monitoring serve() can now distinguish fatal from graceful.

---

S-1 [minor, closed 2026-09-27] — The UDP door's reply path trusts the recorded client address for the association's whole lifetime: a spoofed first datagram hijacks every subsequent reply

Files: src/door.rs (UdpFrontDoor::serve_on's client_addr capture — the first inbound datagram's source wins permanently)

Problem: the door serves one association per relay socket and "learns" the client's source address from the first datagram, using it for every reply. Two residual shapes:

• An off-path spoofed datagram arriving first (before the real client's) pins the replies to the spoofer's address. For the documented posture this is near-moot: the relay socket is bound loopback by the builder's posture (a wildcard bind is the caller's explicit variant), and an attacker who can inject datagrams into the loopback path owns the host anyway. But a deployment that does spell the wildcard bind gets a one-datagram reply-hijack primitive per association.
• A client that changes source port mid-association (NAT rebinding) silently stops receiving replies (replies go to the learned address).

Recommendation: accepted-uncovered posture note rather than a fix — RFC 1928's own ASSOCIATE has the same per-association peer expectation (the RFC's relay is expected to drop datagrams from non-expected peers; learning-on-first is the v1 shape the POC validated). One doc line on serve_on naming the residual ("the client's source address is learned from the first inbound datagram and pinned for the association's lifetime — a wildcard-bind deployment accepts the off-path-first-datagram shape, the RFC's own posture") so the next reader does not mistake it for a bug. Filing now so the residual is a named decision, not an accident.

Closed (the residue round): the recommendation landed — the accepted-uncovered posture note is on UdpFrontDoor::serve_on's doc (commit 87afe70).

D-1 [minor, fixed 2026-09-27] — associate_with_token remains 0%-covered dead surface (the batch-2 completeness twin with no caller)

Files: src/consumer.rs:450-475 (associate_with_token — uncovered; contrast connect_with_token/bind_with_token, both covered via the door tests), the doc claim: "the session family's completeness … and the wrapper-aware deployment's surface"

Problem: the TCP door refuses ASSOCIATE before any session; the UDP door rides AssociateSession::associate (no token — the UDP door is NoAuth by construction). No test drives the token twin. This is review 002 D-1's shape again — spec'd machinery with zero callers — but here the doc already carries the honest posture (the door does not call it), so this is not doc-vs-code drift. Still: a constructor-twin whose negotiation body could drift from associate's (e.g. the DST.ADDR form) would pass the suite.

Fix (small): one test mirroring bind_session_with_token_round_ trip's shape (drive associate_with_token directly against the token-provider topology) — or an explicit accepted-uncovered note on the method. Either closes the review-002-D-1-class trigger before it fires a third time.

Fixed (the residue round): associate_session_with_token_round_trip (tests/door-tcp.rs, commit 59e8fb1) — the probe drives the token twin against the token-provider topology (the CF-005 open, the sentinel BND.ADDR, a datagram round-trip through the echo egress, shutdown reap). The review-002-D-1-class trigger is closed with the test, not the note.

D-2 [minor, fixed 2026-09-27] — ComposedEgress's failure/edge matrix is the batch-2 coverage gap: the LRU's structural Err arm, relay_read's sender-closed exit, and the eviction-vs-live-relay accounting are untested

Files: src/composed/mod.rs (module total 77.32% lines / 65.31% functions — the suite's lowest), tests/composed-egress.rs (6 tests, all happy-path or one-shot EOF)

Problem: the composed egress carries several load-bearing failure shapes that no test drives:

• entry_for's post-insert invariant Err (the structural diagnostic; unreachable by construction — pinned nowhere, not even by an unreachable-class comment beyond its own doc).
• relay_read's tx.send(..).is_err() early return (the shared queue closed with the egress — the stage exiting mid-relay). The Inbound::Ended path is tested (closed_queue_yields_err_...), but the sender-closed path is a different exit.
• The interaction between LRU eviction and live_relays: evicting an entry drops its TableEntry (session + write half), which EOFs the tunnel and ends the relay task → an Ended marker → the count decrements. An association that has evicted all of its (capped) tunnels while parked in next_reply would then trip the ever_opened && live_relays == 0 association-end — correct, but the eviction-drives-association-end composition is unpinned (the existing tests evict and re-dial within a live set).
• inner_bytes' oversize guard (EgressError::General on a reply over the joint bound) — the tunnel codec caps at 65535 inner bytes upstream, so the guard should be unreachable, but nothing pins the guard fires (or that the tunnel codec's cap makes it unreachable).

Recommendation: one small test round before release polish: (a) an egress with max_tunnels: 1 whose single tunnel's far side EOFs → the association-end Err fires through the eviction path; (b) a unit probe on the invariant-Err arm's message shape (or fold it into (a)). The module's 17 uncovered functions are nearly all these two regions.

Fixed (the residue round): commit b43bf54 — (a) landed as eviction_to_a_failed_dial_ends_the_association_when_the_relay_ends (tests/composed-egress.rs; the genuine eviction→association-end shape is the cap-1 eviction whose replacement dial fails — the far side EOFs the evicted tunnel, the count reaches 0, the parked next_reply returns the association-end Err; the originally suggested far-side-EOF shape cannot reach 0 live relays because the replacement tunnel's relay is always live — the test taught the composition); the relay-exit pair pinned directly in src/composed/mod.rs's egress_unit_tests (the sender-closed silent exit + the EOF→Ended path, with Inbound gaining #[derive(Debug)]); the oversize guard pinned at the bound edge; the Ended-marker saturating_sub discipline pinned (a live relay keeps the stage serving beside an ended one). Module coverage: 77.32% → 85.49% lines.

D-3 [minor, fixed 2026-09-27] — The local providers' refusal-mapping tables are partially uncovered: dial_error_of's TimedOut/PermissionDenied arms and bind_accept's Shortage-arm splits are compile-checked only

Files: src/local/mod.rs:76-81 (dial_error_of's NetworkUnreachable/HostUnreachable/TimedOut/PermissionDenied arms — unreachable from the loopback tests, which can only produce ConnectionRefused), src/local/mod.rs:253-259 (the bind-map's PermissionDenied/AddrNotAvailable/_ arms — the tests drive AddrInUse only), src/local/mod.rs:108-110 (the "no addresses tried" terminal — unreachable: the resolved set is never empty after the empty-check), src/local/mod.rs:63-64 (the domain-resolve "no addresses" arm), src/local/mod.rs:214 (try_recv_from's non-WouldBlock error arm)

Problem: the task notes record the mapping table as the deliverable, and to_reply_error() is table-tested — but the io error kind → SocksDialError / io kind → BindAcceptFactoryError splits themselves are only half-driven. A refactor that swaps TimedOut into the _ arm (losing the 0x03 mapping) would pass the suite. Same class as review 002's N-8 single-arm cosmetics, but these are security-adjacent (the refusal vocabulary is the target policy's observable surface).

Fix (small): table-driven unit tests on dial_error_of and the bind-map's closure (they are pure functions of io::ErrorKind — construct errors of each kind with io::Error::new); two tests, no new harness. The two structural arms (no addresses tried, the resolve-empty arm) stay accepted-uncovered with a note each.

Fixed (the residue round): commit 59e8fb1 — four table tests in src/local/tests.rs: the dial_error_of kind→arm table (all six arms, incl. the TimedOut-shares-NetworkUnreachable and the PermissionDenied→General shapes + the context rides the message), the dial vocabulary's reply codes, the bind-map's kind→variant table (AddrInUse/PermissionDenied→Shortage, AddrNotAvailable→ Refused, the _ catch-all→General), and the bind map's reply codes. The two structural arms (no addresses tried, the resolve-empty arm) stay accepted-uncovered as recommended.

N-1 [observation, closed 2026-09-27] — ProducerDoorGuard::close's drop(self)-then-await shape is correct but non-obvious: one doc line would prevent a "simplification" that deadlocks

Files: src/local/producer_door.rs:194-202

Problem: close drops self (releasing the watch::Receiver) before awaiting the loop task — load-bearing: the loop's done_tx.closed() only resolves when all receivers are dropped, so holding the receiver across the await would deadlock the graceful stop. The code is correct; the doc says "ends the accept loop gracefully" without naming why the drop precedes the await. A future refactor that "simplifies" the drop(self) away (it looks redundant next to the field-Noneing above it) reintroduces a hang.

Fix (one line): extend close's doc: "the receiver is dropped before the await — the loop's done_tx.closed() fires only when every receiver is gone; holding it here would deadlock." Zero behavior change.

Closed (the residue round): the doc line landed on close (commit 87afe70).

N-2 [observation, closed 2026-09-27] — bindfirst_accept's factory errors map to reply#1 codes whose vocabulary is wider than the producer's gate: NotAllowed/Shortage from a listen resource deserve a doc cross-check against the producer-side bind_policy gate

Files: src/composed/mod.rs:718-820 (the factory's Refused/NotAllowed/Shortage mappings)

Problem: the composed factory returns BindAcceptFactoryError variants that the producer maps to reply#1 codes — including NotAllowed (0x02), the same code the ADR-006 command gate uses for the ACL refusal. On the channels path the two layers stack: an identity without socks5:bind never reaches the factory (the gate precedes the seam), so a factory-side NotAllowed can only mean the tunnel layer's per-listener ACL — semantically distinct, wire-identical. That is RFC-faithful (0x02 is the ruleset code) and the composed variant's per-listener ACL is a real policy source (ADR-007's per-listener ACL for free), so the behavior is correct — but nothing in the composed module's doc names the stacking ("the factory's 0x02 can also mean the tunnel layer's ACL, distinct from the command gate's"). An operator debugging a refused BIND sees one code with two policy sources.

Recommendation: one doc line on bindfirst_accept naming the layering. No code change.

Closed (the residue round): the layering note landed on bindfirst_accept's doc (commit 87afe70).

N-3 [observation, closed 2026-09-27] — FrontDoor::serve_on's per-accept task holds the session-cap permit for the connection's whole life, but the idle-first-byte timeout does not release early: a 256-cap door parks at "cap reached" for connections that are merely slow readers

Files: src/door.rs:338-374 (the accept loop + Semaphore), src/door.rs:393-401 (the idle-first-byte gate)

Problem: the permit is acquired at accept and dropped at task end — correct for the cap's purpose (bounding concurrent sessions). The residual: a connection that passes the idle gate (sent one byte) but is then slow consuming the producer's conversation holds its permit for the RFC conversation's full lifetime — the cap counts accepted connections, not active sessions. That is the cap working as designed (ADR-014 knob 2 bounds concurrent connections, which is the slowloris-relevant resource), so this is a naming residual only: the builder doc says "max-concurrent-sessions" where the mechanism is "max concurrent connections" (including idle-past- first-byte ones). Same shape on ProducerDoor.

Recommendation: one doc word ("connections" not "sessions") on both builders, or leave as-is — the knob's name is the docs' vocabulary, not a contract. Zero code.

Closed (the residue round): both builders' max_sessions docs now say "max-concurrent-connections" with the mechanism named (commit 87afe70).

N-4 [observation] — The cred-bearing door's local_rfc1929_half check closure writes the mapped identity through a std::sync::Mutex witness even when the check rejects: the None slot survives a failed check (harmless, but the witness's semantics are "last lookup", not "accepted identity")

Files: src/door.rs:530-557

Problem: the checker sets *slot = mapped (including None on a miss) before returning accepted; the later .take().ok_or(...) correctly treats None as rejected. Residual: a checker that ran twice (upstream's accept_password_auth calls it once per conversation — fine here) would let a later miss overwrite an earlier hit. Reachable only through API misuse (the closure runs once per conversation by construction). Harmless as landed; the "the check IS the table lookup — one table, one truth" doc claim is accurate for the single-call shape.

Recommendation: none. Recorded to bound re-review (the shape is the sync-witness pattern's own; a second call site would need Option::take-semantics discipline).

N-5 [observation] — The two doors' accept loops duplicate the idle-peek + semaphore-cap + per-accept-isolation shape (FrontDoor::serve_on vs ProducerDoor's serve_on_parts) — a third door shape should extract it

Files: src/door.rs:338-374, src/local/producer_door.rs:261-294

Problem: ~35 lines of structural duplication (the Semaphore::try_acquire_owned cap refusal, the spawn + _permit-guard shape, the accept-error isolation) — the same N-5 class as review 002's harness consolidation, across a feature boundary this time (door.rs and local/producer_door.rs could share a helper; both are local-gated so no feature-gate obstacle). Deliberately not fixed inline: the two loops differ in the producer injection (serve_socks5_over vs the variant split) and a third consumer doesn't exist yet; a premature extraction would trade two readable loops for one parameterized one.

Fix (small, when a third shape lands): a crate::local::accept_loop-shaped helper (idle gate + cap + spawn + isolation) consumed by both. Until then, the duplication is honest.

N-6 [observation] — InbandBridge::recv's 16 KiB scratch allocates per call (per datagram when the queue is empty) — bounded, but the retained-capacity note from review 002 S-1 applies one layer up

Files: src/door.rs (InbandBridge::recv's vec![0u8; 16 * 1024])

Problem: each decode call allocates a fresh 16 KiB read buffer (the AssociateSession::recv_datagram shape it mirrors does the same). Bounded per call, dropped per call — fine; the S-1-style residual (a 65507-byte frame's BytesMut retaining ~64 KiB of capacity in the bridge's reader for the association's lifetime) is inherited from DatagramReader's design and already recorded in review 002 S-1. Per-association bounded, unshrunk — same accepted posture, now with two owners (the session and the door's bridge).

Recommendation: none (the review-002 posture covers it). One line on InbandBridge's doc pointing at the shared posture, if the next touch lands.

N-7 [observation] — ProducerDoor::builder() requires .producer but not .bind — the bind decision moves to serve(), an asymmetry with FrontDoor's builder (where .bind is required at build time)

Files: src/local/producer_door.rs:144-155 + 222-239 vs src/door.rs:279-288 + 305-309

Problem: two builder shapes for sibling types: FrontDoor's missing-bind is a build()-time error (DoorBuildError:: MissingBindAddr); ProducerDoor's is a serve()-time error (ProducerDoorError::MissingBindAddr). The producer door's split exists for a reason (its doc says so — serve_on needs no bind), but FrontDoor could have made the same split and didn't. The asymmetry is documented on both sides; a deployer writing both shapes hits the difference on the first compile error and reads two docs.

Recommendation: none (both postures are typed, convention-2 compliant, and documented). Recorded to bound re-litigation: the split-vs-eager-bind choice is per-type, not a pattern to propagate.

N-8 [observation, closed 2026-09-27] — Coverage residue worth one line each (or accepted-uncovered with a note), beyond D-2/D-3

The --all-features --show-missing-lines sweep attributed; these are the residuals beyond the findings (each small, named so the next coverage run needs no re-derivation):

• door.rs:160-168 — CredentialTable::len/is_empty: observability accessors, no caller (one probe or an accepted-uncovered note).
• door.rs:194-196 — DoorExit::LocalRfc's Display arm (the cred-bearing door's local-RFC exit is reached in tests, but that variant's arm render is not asserted): cosmetic.
• door.rs:305-309 — FrontDoor::serve (the bind-then-serve wrapper; every test drives serve_on): one test or a "the suite drives serve_on" note.
• door.rs:414 — the (Some(_), None) unreachable-by-construction arm: accepted-uncovered by design (convention 2's exhaustive-match shape), fine.
• door.rs:483-487 — raw_pipe's open-failure exit (a NoAuth door whose channel open fails): one probe would pin it.
• door.rs:595-611 — pipe_connect_plane's Domain-reply form + the None-reply + write-failure arms: the suite drives v4 replies only; a domain-reply unit probe or accepted-uncovered.
• door.rs:636-656 — pipe_bind_plane's Domain-form reply wire (the BIND door's domain-BND re-encode): the suite's BIND tests drive IP forms; the Domain arm is hand-written wire code with no test — the batch-2 suite's most substantive single-arm gap.
• door.rs:722 — write_refusal_reply's _ => return arm (a non-reply-code error stays a bare close): one probe.
• door.rs:755-758 — UdpDoorExit's Display arms: cosmetic.
• door.rs:810-814 — UdpFrontDoor::serve (same as the TCP door's).
• door.rs:842-845 — serve_on's spent-split arm (Ok(()) on a spent association): one probe.
• door.rs:886 — the client_addr-already-set skip; door.rs:908- 913 — the reply-before-first-inbound park (continue when no client address is known): both single-arm, timing-shaped.
• door.rs:941 — the Io-exit-as-Err arm (F-2's new shape): the fix landed untested (a relay-socket read error is not producible over a test duplex without a poisoned socket) — accepted-uncovered with this note, or a UdpSocket-pair probe.
• door.rs:1005-1048 — the bridge's error arms (a decode garbage in a later batch frame, the Address variant): the unit tests pin the happy shapes; one garbage-later-batch probe.
• consumer.rs:1145-1147 — TokenClient's Debug: cosmetic.
• consumer.rs:1185 — the spent-shape assert's err().map(to_string) formatting arm: cosmetic.
• local/mod.rs:63-64, 108-110, 214, 257-259 — D-3's terminal arms.
• local/producer_door.rs:71-72 — BackendExit::Session's Display arm; :121-124 — the builder's idle_first_byte/max_sessions setters (the tests drive both — via serve()'s defaults only); :200 — close's already-closed arm; :222-239 — serve's MissingBindAddr/Bind error arms; :269-271 — the accept-error continue arm; :314 — the idle-peek Io arm.
• composed/mod.rs — D-2's matrix (the module's own gap).
• egress.rs:518 — the sync-Err factory test's panic arm (the match's Ok arm — unreachable in the passing test): cosmetic.

The headline gaps: the BIND door's Domain-form reply wire (D-2's neighbor), the composed egress's failure matrix, and the local/mod.rs mapping-table arms.

Closed (the residue round, commit b43cbd2 + the D-2/D-3 commits): the worthwhile probes landed; the timing-shaped/structural remainder stays accepted-uncovered:

• Probed: CredentialTable::len/is_empty (credential_table_len_and_is_empty_accessors); FrontDoor::serve (front_door_serve_wrapper_round_trips) and UdpFrontDoor::serve (udp_door_serve_wrapper_round_trips) — both bind-then-serve wrappers now round-trip; write_refusal_reply's _ => return arm (write_refusal_reply_skips_non_reply_code_shapes) AND the re-encode's exact wire bytes pinned (write_refusal_reply_encodes_the_mapped_code); the bridge's garbage-in-a-later-batch probe (garbage_in_a_later_batch_frame_fails_loud); the BIND door's Domain-form reply wire — the batch-2 suite's most substantive single-arm gap — closed by extracting the hand-written wire into bind_reply1_wire (a pure function) and pinning both forms' bytes (bind_reply1_wire_forms); D-2's matrix and D-3's tables (their own sections above).
• Accepted-uncovered (unchanged): the DoorExit/UdpDoorExit/ BackendExit Display arms and TokenClient's Debug (cosmetic); the (Some(_), None) unreachable-by-construction arm (by design); the timing-shaped UDP-door single arms (the client_addr-set skip, the reply-before-first-inbound park, the spent-split arm, the F-2 Io-exit arm — not producible without a poisoned socket); the local/mod.rs structural terminals (D-3's accepted-uncovered pair); the producer_door.rs accept-loop/error arms of the same timing-shaped class; egress.rs:518's test-internal panic arm; consumer.rs's two formatting arms; raw_pipe's open-failure exit and pipe_connect_plane's Domain/None-reply arms (the NoAuth open-failure shape needs a poisoned topology the harness cannot produce; the CONNECT Domain reply rides the codec's own new_reply-adjacent path, lower-value than the BIND wire).
• Total coverage moved 93.68% → 93.94% lines (92.67% functions / 94.74% regions); composed/mod.rs 77.32% → 85.49%, door.rs 81.45% → 85.84%, local/mod.rs 84.91% → 89.19%.

---

Part B — Non-findings (verified correct, recorded to bound the re-review)

• The never-binds posture survived batch 2 (re-swept): the protocol layer names no kernel-socket type; every bind is a local-gated constructor's explicit decision; the default + composed wasm checks stay clean; local-on-wasm fails with the expected mio error. The doors' binds are builder-.bind()-driven; LocalEgress::bind's loopback and bind_accept's explicit host are the doc-pinned explicit shapes.
• The cred-bearing door's re-origination holds under adversarial reading: the door's RFC half consumes greeting + credentials + one command; the in-band conversation is fresh (the never-pipes- raw pin); the token is the mapped identity's id riding the open-op auth_token payload (CF-005 precedence (1)); the password never crosses the channel and never reaches a log line (swept: no credential is ever in a tracing call).
• The UDP door's InbandBridge carries the ADR-003 reader-state contract (the gate review's R-1 fix verified independently): the pending queue drains in order, partial frames survive call boundaries, EOF mid-frame fails loud — the three unit pins all fail against a per-call reader, and the end-to-end doubled-echo test guards the class through the real stack.
• The composed accept's teardown-at-birth is structural: the TunnelListener rides inside the write payload (ForwardedOwner); the disjoint-capture trap is doc-pinned and the whole-struct rebinding is in place; the client-gone select! drop tears the listener and the oneshot down together; a correlation miss drops the forwarded channel at birth (pinned).
• The forwarded handler's inline pump await (R-02) is real: the handler's task lives exactly as long as the data plane (either- EOF completion), the wrapper's inline await tracks it — verified against alktunnels' make_forwarded_op_handler (the inline pump.await), not just the doc comments.
• The composed egress's readiness contract is count-based and sound: the ever_opened guard (the assembly suite's drift fix) is correct — a never-opened table parks (the stage's channel branch governs); the ended-relay Err fires only after a tunnel existed and every relay ended; saturating_sub bounds the count; the closed-queue test awaits the Err (never a hot Ok(None)).
• The sync-bound wrinkle's Sync proofs are no-unsafe: both SyncHalves and OwnedWrite ride Mutex<T: Send> (Sync for T: Send, std guarantee); poisoned locks unwrap via into_inner (convention 2); contention is structurally impossible (one pump task at a time — the doc says so and the pump shapes confirm).
• No panics in non-test code (re-swept across all of src/, including the batch-2 modules): every unwrap/expect/panic! hit is in a #[cfg(test)] region or a test file. The batch-2 expect sweep (gate review R-2) held through the review's read.
• Wire-driven allocation is bounded in the batch-2 code: the doors' scratch buffers are fixed (1-byte peek, 64 KiB relay, 16 KiB bridge chunk); parse_udp_request's domain arm stays 255-bounded; the UDP door's 65535-byte recv is the kernel's datagram bound; the cred-bearing door's username/password reads are 1-byte-bounded by the RFC 1929 length fields (the forked codec's bounded-read posture). No unbounded wire-driven allocation exists.
• The doors' failure isolation is real: per-accept spawn, the accept loop continues on per-connection failure (both doors + the producer door, each pinned); a cap-refused accept closes the socket with no channel opened; the idle gate fires before any channel opens (witnessed by the channel-count probes).
• pump_bidi is awaited inline everywhere in batch 2 (the CONNECT/BIND doors' copy_bidirectional against taken in-band streams; the composed accept's middle pump; the producer door rides serve_socks5_over's own inline await) — no spawn-and-forget data plane exists.
• MSRV, wasm, publish: cargo publish --dry-run completes; both wasm checks clean; rust-version = "1.88" unchanged (the batch-2 additions are plain 2021-edition async/await; no let-else/OnceLock or other MSRV-relevant surface).
• License/provenance posture unchanged: src/rfc1928/LICENSE and the module docs still carry the full fork-point statement; batch 2 touched no forked file except server.rs's already-pinned accept_password_auth consumption (doc-level only).

---

Part C — What's Good

• The door boundary is where the complexity belongs, and it is contained. The entire RFC-codec surface (1929 negotiation, reply re-encoding, the UDP header codec) lives in door.rs (1135 lines, 70% tests) behind one feature gate; the protocol layer's growth for all of it was TokenClient + three constructor twins + a reply_addr accessor. The F-1 fix was possible because the codec lives at the door — one helper closed all three sites.
• The provider/producer split is honest. ProducerDoor is pure accept-loop assembly over the landed serve_socks5_over (zero new protocol-layer code, the module doc's claim verified line by line); the ADR-014 knobs are two local types (peek gate, Semaphore) with door-side refusals — the protocol layer never learned about timers, exactly per ADR-014.
• The Sync-bound wrinkle's resolution is textbook: no unsafe, no new trait, two small adapters with the Sync proof in the type system, the owner riding inside the payload so the compiler enforces the teardown — and the whole thing is pinned by a compile-shape test against the real alktunnels surface.
• The assembly integration suite is the batch's real deliverable. It surfaced the one genuine implementation drift (ComposedEgress's birth-killed association) that every per-component suite missed, and it pins the compositions the component tasks could not (doors + native + composed concurrent, the idle-bound teardown matrix, the domain-resolution pin). The mixed-assembly tests are exactly the roles-not-sides posture made executable.
• The docs discipline held through an 8-task batch: the reconciliations the tasks recorded (the ever_opened fix, the TokenClient door-only contract, the one-command-per-connection posture, producer.md's local-backend rewrite) were all verified against the tree; the review found no further spec-vs-code drift beyond the ones already reconciled.

---

Suggested Disposition

The batch-2 gate closed with the crate feature-complete; this review found the residue a full-surface read always finds. Disposition (the residue round executed 2026-09-27 — every row closed except the per-item defers recorded in the sections above):

Item	Action	Shape
F-1, F-2	Fixed in this review change	the refusal re-encoding + the UDP-door exit match + the pinning test
S-1	Closed — the accepted-uncovered posture note	UdpFrontDoor::serve_on's doc (87afe70)
D-1	Fixed — the test, not the note	associate_session_with_token_round_trip (59e8fb1)
D-2	Fixed — the failure matrix	the eviction→association-end + the relay-exit/oversize unit probes (b43bf54)
D-3	Fixed — the io-kind tables	four table tests in src/local/tests.rs (59e8fb1)
N-1	Closed — the doc line	the close drop-before-await note (87afe70)
N-2	Closed — the doc line	the bindfirst_accept 0x02 layering note (87afe70)
N-3	Closed — the doc word	"connections" on both builders (87afe70)
N-4	none	recorded to bound re-review
N-5	defer	extract when a third accept-loop shape lands
N-6	none	covered by review 002 S-1's posture
N-7	none	recorded to bound re-review
N-8	Closed — the worthwhile probes landed, the remainder dispositioned	the BIND Domain wire helper + table, the refusal-recode pins, the serve() wrappers, len/is_empty, the bridge garbage probe (b43cbd2)

Verification after the residue round: cargo test (default) and cargo test --all-features green (15 suites); clippy (--all-targets --all-features and both wasm targets) clean; fmt/doc clean; cargo publish --dry-run --allow-dirty completes; coverage 93.94% lines (93.68% at review time).

The natural home for the residue is the release-polish task (publish prep, OQ-SK2-08) the batch-2 decomposition deferred — the review's test-shaped items ride alongside it; the doc-shaped items are one-line changes in the same round.

References

• docs/reviews/002-protocol-core-implementation-review.md (the template; the C/D/S/N legend; the N-5/N-8 lineage this review's D/N items extend)
• tasks/implementation/review-assembly-layer.md (the narrow gate — its R-1 fix is this review's F-class context; its R-2 sweep is the no-panic posture re-verified in Part B)
• docs/architecture/decisions/003/005/006/007/008/009/014 (the ADRs the batch-2 surface implements)
• alktunnels 0.2.0 (producer.rs make_forwarded_op_handler — the R-02 inline-await cross-check; ForwardedHandler's ownership shape)
• ADR-014 (the accept-path defense obligations the doors inherit — the F-2/S-1/N-3 posture context)
• The POC crates (/workspace/alksocks-{connect,udp,bind}-poc — the fidelity sources the gate review diffed; this review's POC-drift check found no further divergence)