alktunnels/docs/reviews/002-graduation-implementation-review.md

Review 002 — The Graduation Implementation (direct op + bind-first + forwarded op)

Status

Conformant with findings. Six sessions reviewed (52b6252..474844e — the taskgraph fix + the six graduation feature commits): the direct op end to end, the bind-first establisher + accept dispatcher, the forwarded op's serving side, the local bind-first halves, and TunnelListener. Wire surface, scope separation, typed errors, teardown ownership, and the R-02/ADR-050 contracts all conform (verified against the alkcall 0.8.0 source, not just its docs). No [critical] findings. One [minor] implementation gap (M-1) and a cluster of [major]-labeled doc/spec-sync items (D-1..D-4) that are doc-only in code impact — the code is the wire-stable side and is correct; the docs lag it. Nothing blocks the graduation consumers.

Findings use prefix M (minor code), D (doc/spec sync), N (notes). This is the second review in this crate's series (numbering follows the alkcall house pattern — each review numbers independently).

Scope

The six sessions' work at tree 474844e reviewed against ADR-007, ADR-008 (as amended), the updated wire.md/consumer.md/producer.md, the AGENTS.md conventions, and — load-bearing — the alkcall 0.8.0 source (/workspace/@alkdev/alkcall/src/channels/{operations,pump,manager,mux,source,client}.rs, src/registry/registration.rs), cross-checking the six contracts this implementation leans on.

Verified against: alktunnels 474844e; alkcall 0.8.0 (source-read)
Reading list: src/{lib,params,producer,consumer,error,wire}.rs,
  src/local/mod.rs, tests/{harness,producer_bindfirst,producer_direct_op,
  producer_forwarded,consumer_listener,local_bindfirst}.rs,
  docs/architecture/*.md incl. decisions/007, 008
Executed: cargo test (132 default / 157 --features local, doc-tests 2/2),
  clippy --all-targets -D warnings (native + wasm32), fmt --check,
  cargo doc --no-deps (0 warnings), cargo check + clippy on
  wasm32-unknown-unknown — all green, matching the commit messages' claims;
  cargo-llvm-cov 0.8.4 line/region coverage, default (95.63%) +
  --features local (93.66%) — see the coverage appendix
Source-read (alkcall 0.8.0): pump_bidi shutdown-on-completion,
  teardown_channel drop-sender EOF, mux implicit-EOF sentinel,
  yield-once accept_bi + accepted-flag telemetry,
  Establishment::with_reply_field + reserved-key guard,
  open_channel_with_reply shape, registry input_schema gate
  (compiled at registration, runs after ACL, INVALID_INPUT on violation)

Severity legend

Same scale as review 001 / the alkcall reviews:

• [critical] — a decided spec invariant violated so a promised capability is unreachable end-to-end; or data corruption.
• [major] — a core protocol path cannot serve a decided behavior; or a wire/ABI promise is not delivered by the code.
• [minor] — drift, doc/spec inconsistency, or a missing convenience with no correctness impact.

---

Checklist verdicts

1. Wire conformance (ADR-007/008 + wire.md) — PASS. The direct op's spec (tunnel_direct_spec) and the forwarded op's spec (tunnel_forwarded_spec) match wire.md §The Direct Op / §The Forwarded Op: op ids, Sub type, alk/tunnel ALPN marker, the per-substrate if/then target/peer sub-schemas with additionalProperties: false, and the three independent scopes (tunnel:open/tunnel:direct/tunnel:forwarded) — each proven never-implied by wire-level tests in both directions (producer_direct_op.rs tunnel_open_scope_does_not_imply_*, producer_forwarded.rs::forwarded_scope_gate_is_independent_grant). The listen token's positivity is enforced at three layers (schema minimum: 1, the positive_u32 serde pin, the reply projection's > 0 filter) with the mint never yielding 0 (mint_token seeded at 1, wrap-skip pinned by test). The reply projection's asymmetry is deliberate and tested both ways: parse_listen_reply is the loud typed viewer (malformed → MalformedReply), listen_reply_fields the lenient session-surface projection (malformed → both-None + loud log) — the doc comments on both correctly state which to use when, and the additive posture (unknown reply fields tolerated) is pinned by test.

2. Producer conformance — PASS. The bind-first establisher is the awaited bounded establishment phase (registry gate → mint → bind → reply fields → Establishment::new(plan)), the plan is typed-opaque (BindFirstPlan downcast test), and the accept dispatcher is a faithful ADR-008 §1 implementation: accept_bi FIRST (the R-02 accepted-flag telemetry — verified in upstream operations.rs:954-1000, the wrapper's birth-teardown hint keys on it), the idle-stream EOF watch races the accept loop in select!, per-accept forwarded opens are isolated (drop the accept, keep serving — pinned by two tests), loop-critical accept errors end the loop, drop(plan) tears the listener down, and the handler's JoinHandle awaits every per-accept pump (R-02 — no forwarded pump outlives its listener). The forwarded-op serving handler parses params (deny-unknown + positive-listen serde pin) and accepts the BiStream, awaiting the injected handler's JoinHandle inline — a malformed open tears down at birth with no phantom channel (pinned: forwarded_malformed_params_tear_down_loud). register_openable (no establisher) for the forwarded op matches ADR-008 §3's no-establishment shape.

3. Consumer conformance — PASS. TunnelSession::open_direct is the separate constructor ADR-007 §5 pins (not a params enum — two scopes, the caller states which it holds); the session type, data planes, and teardown API are identical to open. TunnelListener holds the write half IDLE (never exposed, never pumped), projects the reply fields via the lenient path, and owns teardown (close = graceful shutdown → EOF sentinel → reap; Drop = implicit-EOF chunk → reap — the mux's implicit-EOF path is real, verified in upstream mux.rs:67-82). No Clone compile-asserted for both session types. adopt gains nothing — pinned by test at both the standalone and through-the-real-forwarded-registration levels.

4. Conventions sweep — PASS with notes. No unwrap/expect/ panics outside tests (the unreachable! arms in take_halves/pump_against are unreachable by the documented data-plane-XOR-pump invariant and are the pre-existing v1 shape). No substrate types outside src/local/ (grep-verified). No comments beyond doc comments (the inline // density in producer.rs/local/mod.rs is high but they are documenting ADR-pinned contracts, which AGENTS.md convention 1's exception covers — correctness constraints that would otherwise be missed). The wasm-clean invariant holds (cargo check + clippy on wasm32-unknown-unknown pass; tokio features unchanged; no new non-local deps). Feature-gate verification: both cargo test (default) and cargo test --features local pass.

5. Conformance to the amended ADR-008 — PASS. Amendment 1's producer-minted token is implemented exactly as pinned: minted at establisher time (non-zero-seeded AtomicU32, per-open fresh, per-producer unique, wrap-skip of the illegal 0), carried twice (plan + listen reply field), stamped per-dispatcher from its own plan (pinned by the two-open token-discipline test), and consumed by the opener-side correlation (assembly-side bookkeeping — the crate provides the op + params, not the wait state machine).

---

Findings

M-1 [minor] — The accept dispatcher's substrate fallback is silent and wrong-shaped

src/producer.rs:766-768 (make_bindfirst_accept_handler):

let substrate = serde_json::from_value::<TunnelParams>(input.clone())
    .map(|p| p.substrate)
    .unwrap_or(Substrate::Tcp);

The listen open's input params have already passed the registry's schema gate (enum-validated) and the establisher's parse_params (which rejects unparseable input with handler_error), so in the wired path the fallback is unreachable. But the fallback shape itself is the problem: if the parse ever fails (a registry whose schema gate is looser, an assembly variant, future drift), the dispatcher silently stamps Tcp into every forwarded open — for a UDP listen tunnel that is a raw-vs-framed data-plane mismatch on the opener side, the exact invariant class review 001 U-1 was about, and it degrades silently instead of loudly.

The same silent-fallback shape exists in no other handler: the forwarded-op handler (make_forwarded_op_handler, src/producer.rs:993-1002) parses and returns on failure (teardown-at-birth, the correct loud shape); the pump handler downcasts the plan and returns on failure.

Suggested fix (small, mechanical): on parse failure, log loud and return (teardown-at-birth — the channel reaps, the far listener dies with the plan, the same posture as every other early-return in this file). Alternatively, since the establisher already parsed the params successfully, stamp the substrate into BindFirstPlan at establisher time and drop the second parse entirely — one parse, one source of truth, no fallback branch at all. The second shape is strictly better (the plan is already the per-open carrier; the substrate is per-open-static) but changes a public-ish struct.

Why not fixed inline: the choice between "log-and-return" and "stamp the plan" is a small API-shape decision (BindFirstPlan gains a field — a plan-payload change, cheap now, pre-consumer) that the next session should make deliberately, and both need a failure-path test.

D-1 [major-as-labeled, doc-only] — wire.md omits the port bounds the shipped schema enforces

The shipped direct-op and forwarded-op schemas pin "port": {"minimum": 0, "maximum": 65535} plus "additionalProperties": false on the target/peer objects (src/params.rs:208-222, 453-467), and listen with "minimum": 1 (src/params.rs:442). wire.md (the normative WHAT) says only port: <u16> and "integer > 0" in prose — it does not record the bounds as schema properties, nor the additionalProperties: false gate that makes cross-shape targets fail the registry's INVALID_INPUT class before the establisher (the behavior malformed_target_is_schema_class_and_leaves_no_channel pins on the wire). wire.md is the wire-stable contract doc: the schema details are part of the one-way door (ADR-007 §"Wire-stable": "the target sub-schema" is listed as wire-stable). The BAST doc (bast.md) covers only the data-plane codec, not the params schemas, so there is no second source recording these bounds.

Impact: none at runtime (the code is the gate); the risk is a future consumer implementing from wire.md alone and accepting a shape the reference producer rejects (or vice versa). Sync the doc before the first consumer (alksocks) ships.

D-2 [major-as-labeled, doc-only] — wire.md's if/then structure differs textually from ADR-007's oneOf

ADR-007 §2 says "the target sub-schema is per-substrate (oneOf on the sibling substrate value, validated at establisher time — see Consequences)"; the shipped schema (and wire.md §The Forwarded Op's structure, and the unit tests pinning it) use allOf of two if/then branches instead. oneOf of full-target-schemas would actually be wrong here (a unix-substrate open would need to fail the tcp branch — plain oneOf on sibling-conditioned subschemas is awkward; if/then composes correctly with the base properties/required). The code is right; ADR-007's one-word schema-structure reference is stale. One-line ADR erratum (or an amendment note), plus ADR-008 §3's "validated at establisher time" (ADR-007 §2's parenthetical) vs the shipped registry-side gate: the establisher-time sentence in ADR-007 §2 line 82 ("validated at establisher time") contradicts both the shipped design and the wire.md text ("validated at establisher time" appears in wire.md §The Direct Op but the test proves the registry gate fires first — malformed_target_is_schema_class_and_leaves_no_channel asserts the establisher's dial never runs). The ADR/wire.md sentence should read "validated registry-side (the schema gate), before the establisher; re-checked by serde at establisher time as the backstop" — which is exactly what src/params.rs's doc comments already say.

D-3 [minor] — jsonschema_valid in src/params.rs tests is a stub, not a validator

src/params.rs:628-637: jsonschema_valid parses the value with serde and pattern-matches the substrate string — it validates the serde layer, not the JSON Schema. Both call-sites' names (schema_rejects_unknown_substrate_value, direct_target_schema_gates_shape_per_substrate — the latter correctly does structural schema assertions instead and is honest about it) overpromise for the former: schema_rejects_unknown_ substrate_value does not execute the spec's input_schema at all. The registry-side gate IS exercised end-to-end by the integration tests (schema_rejects_unknown_substrate_loudly, malformed_target_is_schema_class…, the forwarded listen: 0 gate), so coverage is not lost — but the unit test's name promises more than it delivers. Either rename it (e.g. serde_rejects_unknown_substrate_value) or drop it in favor of the integration probes. alkcall already compiles the real validators at registration; a jsonschema dev-dependency purely for this unit test is not warranted.

D-4 [minor] — CHANGELOG "Unreleased" preamble is stale

CHANGELOG.md's Unreleased section still opens with "implementation is decomposed in tasks/tunnels/ and lands in the next release… the only behavior-affecting change so far (no wire or API change at this level yet)" — written before the six implementation commits. The section below it correctly lists all six features. The preamble contradicts the section it introduces. Trim the preamble to a one-liner (or drop it) before release.

N-1 [note] — parse_listen_reply's malformed-listen message vs the serde pin's semantics

parse_listen_reply requires listen to be "a positive u32" and rejects 0 (src/params.rs:329-339) — matching the forwarded params' wire contract (minimum: 1). Consistent and intentional, but note the asymmetry with bound: a bound: null and a listen: null are both treated as absent (None | Some(Value::Null) => None) — fine, JSON-null-as-absent is the standard lenient reading, and the loud path still fires for present-but-malformed shapes. No action; noting the deliberate shapes for the record since TunnelListenReply carries deny_unknown_fields while the parse function tolerates unknown fields — the struct-level attribute is only load-bearing if someone deserializes the struct directly from a full reply, which the doc comment explicitly warns against ("Parse the full reply with parse_listen_reply, never with deny_unknown_fields deserialization of the whole reply"). The warning is doing real work; keep both.

N-2 [note] — The idle-stream EOF watch treats ANY byte as teardown

make_bindfirst_accept_handler's eof_watch (src/producer.rs:781-794) triggers teardown on Ok(0), Err(_), and Ok(1..) — a misbehaving opener that writes to the idle listen channel also tears the listener down. The doc comment owns this ("a peer that writes is misbehaving; treat any non-empty read as an EOF trigger too"), and it is the safe direction (fail closed — the channel is not a data plane). Verified upstream that a MpscSendStream drop without shutdown produces the implicit-EOF chunk (the Ok(0) path) and a shutdown produces the sentinel — both arrive as Ok(0) at the far reader, so both TunnelListener::close (explicit shutdown) and Drop (implicit EOF) trigger the far teardown. Correct. No action; recorded because a reviewer reading only ADR-008 §1 ("its EOF cancels the loop") might implement the narrower Ok(0)-only check and hang on a write-happy peer — the code is right, the ADR sentence is thinner than the implementation.

N-3 [note] — make_forwarded_op_handler parses params BEFORE accept_bi

src/producer.rs:993-1006: the malformed-params early return happens before conn.accept_bi(), so the R-02 accepted-flag telemetry reads false for a malformed-params open (the wrapper logs the birth-teardown hint). This is the deliberate reading of R-02 — the flag is telemetry for the "handler exited without accepting" hint, and a malformed-params open IS a birth-teardown shape — but the ordering means the hint fires for a handled-rejection case. Cosmetic telemetry noise at worst (a tracing::debug!); the test suite pins the teardown behavior, which is what matters. The alternative (accept first, then parse) would hold a stream open for a params set the producer already got wrong — the current order is the better one. Documented here so a future alkcall reviewer doesn't flag the flag.

N-4 [note] — SubstrateAddr untagged leniency is schema-gated, and that's the right call (but it is a latent sharp edge)

The untagged SubstrateAddr accepts extra fields (untagged variants cannot deny_unknown_fields), so a cross-shape target like {"path": "/p", "host": "h"} parses as the Path variant at serde level. Both the direct-op and forwarded-op tests pin this asymmetry with the same rationale: the SCHEMA is the shape-vs-substrate gate (additionalProperties: false per branch). That holds for the registry-dispatched path. Two things to keep in view:

• The establisher-level backstop (the serde parse inside direct_establisher) keys render_target off the sibling substrate — a cross-shape target that sneaked past a looser registry would render something (/p for the unix case) rather than fail. Acceptable (garbage-in handled by the schema's absence), but worth an ADR-line if a future consumer runs a schema-less registry variant.
• If SubstrateAddr grows a variant whose shapes overlap (e.g. a future {host, port, flowlabel}), untagged disambiguation starts ordering — at that point switch to #[serde(untagged)]-with-content or an internally-tagged form. Pre-consumer, additive; just don't let it happen by accident.

N-5 [note] — The token-mint discipline is sound; the concurrent-mint shape is untestable at this layer and fine

mint_token is a relaxed-ordering fetch_add loop skipping 0. The "2^32 concurrent opens" wrap-collision math in the doc comment is right (and the collision consequence — an opener correlating a forwarded open to the wrong listener — requires the same producer to have 2^32 live listeners simultaneously, impossible under the 256-channel per-connection cap; the token is per-producer-connection, and each open rides one connection). Worth noting the cap makes the wrap analysis even more favorable than the comment states. No action.

N-6 [note] — Test-only race window in bindfirst_open_surfaces_reply_fields_and_teardown_cascades

tests/consumer_listener.rs:276 (and several sibling tests) settle the forwarded open with tokio::time::sleep(300ms) then poll channel_ids(). Sleep-then-assert is the established house pattern in this suite (the harness has no push-based settle surface), but these are the tests most likely to flake on a loaded CI box (the forwarded open traverses: producer accept → call plane → opener dispatch → registry → wrapper → handler spawn). The suite has been 3x-repeat stable historically (review 001's methodology); if CI ever shows a flake here, the fix is a bounded poll loop on channel_ids() (the teardown assertions already use that shape — extend it to the arrivals). No action now.

N-7 [note] — test_bind()'s listener is a bare Arc<u32>

The test bind closures in producer_bindfirst.rs / consumer_listener.rs put Arc::new(4242u32) in the plan's opaque listener slot. Fine for tests that feed accepts via side channels — and the mismatch probe (accept_pair_loop_downcast_miss_fails_loud) proves the real dispatch path fails loud on exactly this shape — but if a test ever wired local_bindfirst_accept_loop() against test_bind(), the failure would surface as the generic handler_error, not a test bug message. The tests consistently use the queue loops instead. No action; the downcast-miss test covers the failure mode.

---

What was checked and found conformant (highlights)

• The token path end-to-end (Amendment 1): mint (seed-1, wrap-skip) → reply field (reserved-key guard upstream keeps channel_id wrapper-owned — verified in operations.rs:927-946) → lenient projection → forwarded-param keying → opener-side correlation → per-dispatcher own-plan stamping (two-open test) → hub-relay pass-through (params untouched, reply fields per-hop-truthful — upstream ADR-051 machinery, source-verified in the relay docs).
• The teardown cascades: listener drop → implicit-EOF chunk (upstream mux.rs implicit-EOF path verified) → far dispatcher's idle-read fires → loop breaks → drop(plan) (listener dies) → pumps awaited → handler task exits → wrapper's handler-exit teardown reaps the serving-side entry. Both directions (close's explicit shutdown sentinel and Drop's implicit path) tested; the leak-free guarantee asserted via channel_ids() on both managers.
• Per-accept isolation: forwarded-open failure (FORBIDDEN / NOT_FOUND / correlation miss) drops only that accept; the loop serves the next (two tests pin queue-drain + no-stray-channels).
• Scope separation proven structurally at the wire level in all three pairwise directions, at both the raw-call and typed-session layers.
• CF-005/CF-006 identity surfaces: the establisher witnesses the END CALLER's identity on base/direct/bind-first paths (witness wrapper for bind-first, inline for the others); the forwarded op's opener-side gate sees the producer's per-call identity (the upstream per-call overlay — tested via the scope gate).
• Schema-gate ordering: ACL → input_schema → establisher → handler, verified in upstream registration.rs (all three dispatch entry points) + pinned by the tests that assert the establisher's dial never runs on a malformed target.
• Doc-test parity: both compile_fail asserts (no-Clone) live and green.

---

Remediation

Ordered smallest-first; nothing here blocks alksocks composition.

1. M-1 — the accept dispatcher's substrate fallback (log-and-return, or stamp the substrate into BindFirstPlan). Small; needs a failure-path test.
2. D-3, D-4 — rename/strengthen the unit-test helper; trim the CHANGELOG preamble. Trivial.
3. D-1, D-2 — sync wire.md's schema bounds + ADR-007's oneOf→if/then wording + the "establisher-time" → "registry-side-gate, serde-backstop" sentence. Doc-only but wire-contract-adjacent: do it before the first consumer ships.

---

Coverage review (appendix — cargo-llvm-cov 0.8.4, line/region mode)

Measured with cargo llvm-cov (default) and cargo llvm-cov --features local; branch-level instrumentation needs a nightly toolchain (-Z coverage-options=branch), so these are line/region numbers — branch coverage is analyzed manually below where it matters.

Headline numbers

Configuration	Line coverage	Note
default	95.63% (74 missed of 1692)	the wire/protocol surface
--features local	93.66% (133 missed of 2098)	adds src/local/mod.rs (84.0% lines)

By file (default / local): error.rs 100/100, params.rs 100/100, wire.rs 96.0/96.0, producer.rs 93.8/94.7, consumer.rs 90.5/90.5, local/mod.rs —/84.0.

The numbers are healthy for this suite's design (the integration tests drive the real paths over the harness duplex and — with local — real sockets); what remains uncovered is overwhelmingly defensive early-return arms that no wired path can reach plus a handful of genuine, cheap-to-close gaps. Classification follows.

Genuine gaps (worth closing)

G-1 — TunnelSession::open/open_direct/TunnelListener::open params-serialize error closures

src/consumer.rs:94-97, 134-137, 487-490 (and open_reverse_channel's twin at src/consumer.rs:593-594). All four are serde_json::to_value(..).map_err(..) arms over types with no non-string keys or fallible serializers — unreachable in practice, and the established house shape for this conversion. Closing these needs a test-only serializer poison (serde_json can't fail on these types without try_collect tricks). Recommendation: leave uncovered; do not chase. If the suite ever grows a shared "unreachable error arm" prose note, these four are its members. (Alternative: collapse the four duplicated arms into one fn params_to_input<T: Serialize>(..) helper in consumer.rs — then only ONE closure exists and the gap shrinks by 3. Cosmetic; fine to skip.)

G-2 — TunnelListener::close's shutdown arm never observed at the far end

src/consumer.rs:529-531: close() runs write.shutdown().await (the explicit EOF sentinel) then reaps — but no test currently observes the FAR accept dispatcher's idle-read firing from a close() (the graceful path). The Drop path (implicit EOF → far teardown) IS observed end-to-end (bindfirst_open_surfaces_reply_fields_and_teardown_cascades), and the close path is pinned at the reap level (close_and_drop_reap_the_listen_channel) — but the sentinel's far-side effect for close() specifically is asserted nowhere: the "the pushed accept was never consumed" assertion runs only in the Drop-shaped tests. Recommendation: one small test — open against a bind-first producer, push an accept, listener.close().await, then poll that the far accept loop is gone (the same queue-len assertion the Drop tests use). This is the only gap that touches a promised behavior (ADR-008 §1's teardown contract, graceful half) with no direct observation. Cheap: mirror bindfirst_full_flow…'s harness with a close() instead of a drop.

G-3 — TunnelSession::join's pump-Err arm (src/consumer.rs:368-373)

The JoinHandle error path (pump aborted → JoinError → warn + (0, 0)) never fires in tests: close()/Drop abort the pump and reap without joining; join() is only ever exercised on live pumps. A one-line test: open a session, pump_against, close().await (aborts the pump), then… join consumes self — the shape needs a pump_against session dropped into a spawn that is aborted before join. Practical shape: let s = session.pump_against(dup).await; take the pump out via s.join() racing a s.abort()-equivalent — there is no public abort on the session, so the test should close the channel out-of-band (manager.teardown_channel) while join() is pending: the pump's copies error/EOF, pump.await still resolves Ok — actually the JoinError arm fires only on abort/cancel, which the session type never does before join. Recommendation: leave the arm as-is (defensive, log-only) but note it in the review as unreachable-by-public-API — the only way to hit it is a pump panicking, which is out of contract. Alternatively add a #[cfg(test)]-only unit test driving join on a session whose pump handle was manually aborted via a test seam — not worth the surface pollution on a warn-log arm.

G-4 — local/mod.rs UnixListenerHalves accept-first loop never driven (591-609) + TcpListenerHalves::accept_pair downcast path in the queue-shaped tests

UnixListenerHalves::accept_loop (src/local/mod.rs:591-607) has zero coverage: no test constructs it (the unix dial test hand-rolls its own echo listener; the bind-first unix test uses accept_pair). It is the unix parity twin of the covered TcpListenerHalves::accept_loop (hit at line 107-125) and its body is 15 lines of the same shape. Recommendation: one small unix listen-producer test (mirror producer_listen.rs::accept_loop_stops_feeding_after_close_and_maps_the_drop with bind_unix + a temp path) — closes the entire 593-607 range and exercises UnixListenerHalves beyond the bind-first path. Low value (copy-shaped), but it is the largest single uncovered block in the crate.

G-5 — The unnamed-peer error arms (unix)

src/local/mod.rs:626-633 (the accept_pair unnamed-peer handler_error) and src/local/mod.rs:578-583 (the local_addr unnamed-bind error) never fire: the unix bind-first test binds a named client socket deliberately (the parity case), and no test binds an abstract/autobind unix listener. Both are fail-loud guards the ADR pins (ADR-008 §3 — "a producer that cannot determine the peer does not serve a bind-first listener over that substrate"). Recommendation: one compact test — bind an abstract unix listener (Linux: a leading-\0 path via socket2, or UnixListener::bind on an unnamed autobind socket), call local_addr() (expect the "no filesystem path" handler_error), and drive one autobind-client accept through accept_pair expecting the unnamed-peer error. Cheap, deterministic, and it pins the two ADR sentences the doc comments promise.

G-6 — UDP bind-family resolve edge (src/local/mod.rs:180-181)

connect_udp's no addresses arm (the resolver returning an empty list). connect_udp_unresolvable_target_maps_to_dial_failed covers the to_socket_addrs Err arm; the empty-success arm needs a resolvable-but-zero-address name, which is not deterministically producible with std resolution. Recommendation: leave uncovered (documented as the std resolver's unreachable-in-practice arm) — or refactor connect_udp to take the resolution behind a small ResolveFn seam so the empty case is unit-testable. The seam is the better long-term shape (it also removes the blocking to_socket_addrs call from the async context — see N-8 below), so if this gap is addressed, do it via the seam.

G-7 — UdpHalf poll_read/poll_flush readiness-error arms

(src/local/mod.rs:369-377, 395-396, 422-423, 427, 520-521): the poll_recv_ready/poll_send_ready Ready(Err) arms and the high-water Pending-after-drain arm. The veth sender-block test (udp_half_write_side_blocks_stashes_and_drains_on_resolve) covers the stash/queue/resolve spine (lines 505-528 mostly hit; 519-521's poll_send_ready Pending arm never resolves Pending inside the test window because the veth's EAGAIN pattern resolves readiness between polls). These arms are one-poll-window states — deterministic capture needs a mocked readiness source, not a socket. Recommendation: leave; the veth test already pins the observable behavior (stash → retry → deliver), and the arms are unreachable differently per-kernel (host-notes in the test document this).

Deliberately uncovered (defensive; no action)

• Handler early-return arms (src/producer.rs:446-455, 759-764, 1004-1005): the accept_bi-failed / wrong-plan-payload / try_unwrap-failed telemetry returns. A wired open always has a plan of the right type; these fire only on a protocol bug or a mismatched registration (assembly error, not wire state). Covered in spirit by the unit-level downcast tests. To hit them, a test would need a handler registered against a plan the establisher never produced — an upstream-invariant violation, not a code path.
• warn!/error! telemetry bodies (src/producer.rs:825, 833, 838-849; src/consumer.rs:368-373): per-accept forwarded-open failure logging (the FORBIDDEN isolation test hits the drop branch's effects but the tracing body's region shows missed because the macro expands past the branch) and the loop-critical accept error arm — wait: the loop-critical arm IS behaviorally covered (accept-loop-ended via the queue_loop's closed-listener Err in the teardown-cascade tests); the missed lines 838-849 are the second tracing::warn! expansion inside that arm. Logging regions in macro expansion show as missed lines while the branch executes. Verified: listen_channel_eof_cancels_loop_and_tears_listener_down drives a loop-critical accept error. No action.
• unreachable! arms (src/wire.rs:113, src/consumer.rs:248, 328, 333): guarded by invariants (current set before the else branch; data-plane XOR pump; the spent-session double-take the type makes impossible). Standard posture; the debug_assert-shaped gaps review 001 already passed.
• wire.rs:184 — the datagram_split_across_awkward_chunks_reassembles test's failure panic (the test always passes the reassembly).
• Default for DatagramReader (src/wire.rs:82-84): derive-shaped boilerplate; all tests construct via new(). Trivial to close if wanted (let _: DatagramReader = Default::default(); in an existing codec test) — otherwise delete the impl in favor of #[derive(Default)].
• src/params.rs 99.3% — the two "missed regions" are the serde_json::from_value error closures inside parse_listen_reply's bound/listen arms; the listen_reply_rejects_malformed_shapes unit test exercises both paths (the macro-expansion region accounting again). Line-level: 100%. No gap.
• Debug impls (src/producer.rs:66, 109): TargetHandle's and BindFirstPlan's manual fmt. Never invoked in tests (no {:?} on those types). One-line fix if desired: assert_ne! two plans in the token-discipline unit test (forces Debug), or drop the manual impls for #[derive(Debug)] with the opaque fields skipped (#[derive(Debug)] on Arc<dyn Any…> prints the pointer — the manual impl exists to keep the listener opaque). Keep the impls; optionally add a debug_fmt_smoke unit test.
• witness_establisher's None arm (src/producer.rs:917): the registration wrapper's None => establisher branch — every test registers with Some(witness). One test registering register_tunnel_bindfirst_openable with identity_witness: None would cover it; trivial.

Branch-coverage notes (manual, no nightly needed here)

The instrumented run is line/region only (stable; branch mode requires -Z coverage-options=branch). The manually-reviewed decision points and their test-pinned states:

• AcceptQueue::pop's three arms — all covered (producer_listen.rs: empty-then-push, closed-and-empty, the lost-wakeup window test).
• UdpHalf::poll_read's staged-frame / fresh-recv / WouldBlock / error arms — all covered except the poll_recv_ready Err/Pending arms (G-7).
• UdpHalf::poll_write's high-water arm — covered by the veth test (Pending observed); the inner Poll::Ready(Err) return (422-423) uncovered (G-7's class).
• pump_queue_to_socket's WouldBlock/ENOBUFS stash arm — covered (veth test); the poll_send_ready triple (519-521) uncovered except via the flush-driven path (G-7).
• parse_listen_reply's malformed arms — covered (unit test); line-level 100%.
• The idle-read Ok(_) arm (src/producer.rs:792) — same body as the Ok(0) | Err(_) arm; llvm-cov counts the second match arm's region separately. The write-happy-peer teardown shape is documented (N-2 in the main review) but untested — a test would write a byte to the idle channel and assert far teardown. Same harness shape as the EOF tests; cheap to add with G-2 if desired (it pins the "any non-empty read tears down" posture).

Verdict

No coverage gap hides a correctness risk in the shipped paths: the uncovered set is defensive arms + two doc-promised fail-loud behaviors (G-5) + one ADR-contract half (G-2). The suite's integration-first design (real sockets with local, real forwarded-openable registrations, real EOF sentinels) is why the uncovered residue is this small. Recommended additions, in order of value: G-2, G-5, G-4, G-7's poll_send_ready Err arm (skip — kernel-dependent), G-6 via the ResolveFn seam (optional), G-3 (document-only). Combined with M-1's failure-path test, that is roughly one session's worth of test work.

References

• The six commits: 0789ab6 (direct op), d586a5d (forwarded params
  • plan type), 7d23a89 (bind-first establisher + accept dispatcher), 3fb26d9 (forwarded serving + reply projection), 514e0c1 (local bind-first halves), 474844e (TunnelListener)
• ADR-007, ADR-008 (as amended), wire.md, consumer.md, producer.md
• alkcall 0.8.0 surfaces this work rides: open_channel_with_reply, Establishment::with_reply_field + the reserved-key guard, flavor-form op ids (ADR-047 amendment 3), pump_bidi (ADR-050), the registry input_schema gate, the R-02 accepted-flag telemetry
  • birth-teardown hint
• Review 001 (the prior review; U-1's framed-adapter fix is re-verified here by the direct-op UDP round-trip test)