---
status: resolved
last_updated: 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)
reviewed_artifacts:
  - 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)
tool: manual source read + cargo test/clippy/fmt/doc/wasm/publish-dry-run + cargo-llvm-cov + alktunnels 0.2.0 surface cross-checks
reviewer: general implementation review (correctness + security + smells + coverage)
scope_note: 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-`None`ing 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)