alksocks/tasks/implementation/fix-coverage-residue.md

---
id: implementation/fix-coverage-residue
name: "Tests: the review-002 coverage residue (N-8: stage exits, splits, spent arms)"
status: completed
depends_on: [implementation/fix-bind-reply2-reentry, implementation/fix-producer-gate-policy, implementation/fix-rfc1928-posture]
scope: moderate
risk: low
impact: component
level: implementation
---

## Description

The review-002 coverage residue: the load-bearing gaps plus the
single-arm cosmetics, attributed in
`docs/reviews/002-protocol-core-implementation-review.md` §N-8 so
the next `cargo llvm-cov` run needs no re-derivation. Tests only;
rides *after* the three code-fix tasks so every test lands on the
final shapes (the C-1 spent posture, the C-3 gate entry, the N-4
vocabulary).

**The headline gaps** (the review's named priority):

1. **The datagram stage's three failure exits**
   (`src/producer.rs:374-386`, `405-414`): the ASSOCIATE teardown
   surface, untested anywhere (the harness's egress never fails
   post-reply):
   - `ReplyFrameRejected` (an egress reply violating the joint
     bound)
   - `EgressEnded` via `next_reply`'s `Err` (the relay's end)
   - `EgressEnded` via a failing `egress.send`; at minimum this
     one gets the drop-witness treatment
     `egress_factory_called_once_and_dropped_with_the_stage` gives
     the happy path
   - the stage's `Io` exit (a failing write/read)
2. **The two untested `split`s** (`src/consumer.rs:437`,
   `771-774`): `AssociateSession::split` and `BindSession::split`
   on a *live* stream; the front-door-bridge surfaces
   (consumer.md), load-bearing for the batch-2 UDP/BIND doors. One
   test each (or the explicit "batch-2 door exercises it" note;
   prefer the test, since the doors are not here yet).

**The single-arm residue** (batch freely; each is one small test):

3. `consumer.rs:356-359`: `send_datagram`'s spent-stream `Io` arm
   (mirror the `spent_session_*` shapes; the spent-`recv` arm is
   covered).
4. `consumer.rs:419`: `recv_datagram`'s mid-batch later-frame `Err`
   arm (first frame `Ok`, a later one garbage → the pending `Ok`s
   dropped with the session; pin the fail-loud-drops-pending
   semantics).
5. `consumer.rs:453-456`: `AssociateSession::into_inner`'s `Ok`
   arm (mirror `into_inner_pairs_the_pipe_with_the_teardown_owner`).
6. `consumer.rs:636-642`: `BindDataPlane`'s `poll_flush`/
   `poll_shutdown` passthroughs (the BIND door's half-close rides
   these; one shutdown test).
7. `consumer.rs:843`: `read_reply2`'s version-mismatch arm (a
   foreign producer's non-5 version reply#2; one unit test on
   `read_reply2` directly).
8. `producer.rs:560` (the quiet wait's `Err(_)`, i.e. an I/O error, not
   EOF); `producer.rs:568` (the BIND reply#2 Domain-peer
   fallback (`TargetAddr::Domain(_, port)` → `0.0.0.0:port`)): one
   factory returning a Domain peer pins it.
9. `egress.rs:72-74, 226-230, 275-279`: the three `Debug` impl
   bodies (render once each), plus `error.rs:113-115`:
   `SocksEstablishmentReason`'s `Display`. Cosmetic; batch freely.

**Explicitly out of scope** (the review's accepted-uncovered
posture): `rfc1928/client.rs:217-219` (`use_password_auth`'s misuse
arm: review-001-era accepted-uncovered with a translated-error
note) and the fork's accepted-uncovered blocks. Do not chase them.

If any arm became unreachable by the code-fix tasks (e.g. the C-1
take-shape changes `BindSession::split`'s reachability), record the
arm as resolved-by-change in the Notes rather than forcing a test
through an artificial shape.

## Acceptance Criteria

- [x] The stage's failure exits tested (`ReplyFrameRejected`,
      `EgressEnded` both arms, `Io`), with the `send`-failure arm using
      the drop-witness assertion
- [x] `AssociateSession::split` + `BindSession::split` tested on a
      live stream (or the batch-2-door note recorded per arm)
- [x] The single-arm residue covered: spent-`send`, mid-batch
      later-frame `Err`, ASSOCIATE `into_inner`'s `Ok`,
      `BindDataPlane` shutdown, `read_reply2` version mismatch, the
      quiet-wait `Err(_)` arm, the Domain-peer fallback, the
      `Debug`/`Display` renders
- [x] `cargo llvm-cov` re-run: the named lines covered or recorded
      as accepted-uncovered with a reason in the task Notes
- [x] `cargo test` green; clippy/fmt clean; both wasm checks clean

## References

- docs/reviews/002-protocol-core-implementation-review.md §N-8 (the
  attributed line list, the task's source of truth)
- tests/harness.rs (the topology + egress shapes to extend)
- src/producer.rs `egress_factory_called_once_and_dropped_with_the_stage`
  (the drop-witness pattern)
- tests/consumer-connect.rs `into_inner_pairs_the_pipe_with_the_teardown_owner`
  (the into_inner pattern)

## Notes

> Filled by the implementation agent (2026-09-24):

- **All N-8-named lines covered; no accepted-uncovered entries
  added.** The `cargo llvm-cov --show-missing-lines` diff before →
  after: every line the review attributed (consumer.rs 356-359/
  419/437/453-456/636-642/771-774/843 pre-fix numbering; producer.rs
  374-386/405-414/560/568; egress.rs 72-74/226-230/275-279;
  error.rs 113-115) is now covered. The suite grew 357 → 375 tests
  (145 lib unit / 38 associate / 41 bind / 40 connect / 41
  integration / 30 harness / 37 differential / 3 doc-tests);
  TOTAL 96.31% regions / 97.22% lines.
- **The stage's failure exits were pinned as direct unit tests on
  `run_datagram_stage`** (the serve layer discards `StageExit`, so
  the unit shape is the only place the exact exit matrix is
  assertable): `datagram_stage_reply_frame_rejected_exit` (an
  egress reply whose inner bytes violate the u16 joint bound →
  `ReplyFrameRejected`), `datagram_stage_egress_relay_ended_exit_
  via_next_reply` (`next_reply`'s `Err` → `EgressEnded`),
  `datagram_stage_write_failure_is_the_io_exit` + `datagram_stage_
  read_failure_is_the_io_exit` (a failing channel write/read →
  `Io`). The `send`-failure arm got the serve-layer drop-witness
  treatment: `udp_egress_send_failure_ends_the_association_and_
  drops_the_egress` (a full ASSOCIATE conversation, one datagram in,
  the egress refuses it, EOF reaches the client, the stage exits,
  and the drop flag flips: teardown-at-birth on the failure path).
  The tests ride `FailStream` (a poll-failing stream) and
  `OneReplyEgress`/`RelayEndedEgress`/`ParkedEgress` (stage-shaped
  egress fakes), all inside the producer test module.
- **The two splits tested on live streams.** `associate_split_
  yields_live_halves_and_survives_the_round_trip` and `bind_split_
  yields_live_halves_pre_reply2` exercise `tokio::io::split` on a
  live `Socks5Stream<BiStream>` built by a new test helper
  (`build_skip_auth_bi_stream`: two duplexes joined via
  `BiStream::from_joined`, the same shape `open_and_negotiate`
  assembles): raw framed bytes pass through both directions on the
  borrowed halves, and (BIND) the split leaves the take-guard
  untouched; `await_inbound` still reads the reply#2 phase after
  the halves are dropped. No "batch-2 door exercises it" note
  needed; the tests stand alone.
- **The single-arm residue, test by test:**
  `send_datagram_on_a_spent_stream_is_the_io_arm` (the spent-stream
  `Io` arm, with the spent-`recv` shape asserted in the same test:
  `Ok(None)`, the family's read-side posture),
  `recv_datagram_mid_batch_later_frame_err_drops_the_pending_oks`
  (first frame `Ok`, a later one garbage → the pending `Ok`s dropped
  with the session, the fail-loud-drops-pending semantics pinned),
  `associate_into_inner_pairs_the_stream_with_the_teardown_owner`
  (the `TakenAssociateInner` Ok arm, the CONNECT
  `into_inner_pairs_the_pipe_with_the_teardown_owner` shape's
  ASSOCIATE mirror),
  `bind_data_plane_poll_flush_and_poll_shutdown_pass_through`
  (write → `poll_flush` → `poll_shutdown`: the bytes reach the peer
  and the EOF sentinel follows),
  `read_reply2_rejects_a_foreign_producers_version` (a non-5
  version reply#2 → `UnsupportedSocksVersion(0x04)`, loud),
  `bind_quiet_wait_read_error_ends_the_wait_cleanly` (the quiet
  wait's `Err(_)` arm driven directly on `bind_quiet_wait_and_serve`,
  where a read *error* ends the wait `Ok(())` and the accept future
  drops with its owned state, the client-gone contract's
  error-shaped twin), `bind_domain_peer_falls_back_to_the_
  unspecified_address` (a foreign accept handle carrying
  `TargetAddr::Domain(_, port)` peer → reply#2 carries
  `0.0.0.0:port`, data plane live),
  `debug_impls_render_the_handle_surfaces` (the three egress.rs
  `Debug` bodies: TargetHandle non-exhaustive, BindTargetHandle
  names the peer, BindAccept names the bind address),
  `establishment_reason_display_renders_the_wire_code` (the
  `SocksEstablishmentReason` `Display`; each variant renders as its
  canonical wire code), `associate_debug_renders_channel_liveness_
  and_pending` (the ASSOCIATE + BIND session `Debug` bodies),
  `spent_associate_close_reaps_a_no_op` (the `close`-on-spent arm),
  and `establisher_records_identity_and_defaults_the_establishment`
  extended with the `None`-witness arm.
- **A quiet-wait select-shape discovery (test-only note):**
  `tokio::select!` polls branches in random order: a read branch
  that errors on its *first* poll can win before the accept branch
  is ever polled, dropping the accept future at zero polls (the
  owned state never constructed; a drop flag would never fire, and the
  first cut of the test caught this flake). The test's `FailReadStream`
  parks once (with a wake) then errors on the second poll so both
  branches are guaranteed started; the assertion then holds
  deterministically. Production semantics unaffected (either exit
  drops the accept future; the contract is exit-shaped, not
  poll-count-shaped).
- **One review-item line numbers shifted post-fix-tasks**: the
  review's `consumer.rs:396-398` (the covered spent-`recv` arm) is
  now `418` (the `inner.as_mut()` else arm after the C-1/D-1
  changes); it is covered by the same test that pins the spent-send
  arm. `consumer.rs:937` (the spent-bind test's `err().map()`
  diagnostic closure) stays uncovered, since it only evaluates on `Err`,
  and the spent shape is `Ok(None)` by design (the C-1 posture);
  cosmetic, not chased.
- **Reachability check for the arm-became-unreachable clause**: no
  N-8 arm was made unreachable by the three code-fix tasks; every
  named arm still exists at its (shifted) location and is now
  tested. The explicit out-of-scope items stayed out of scope:
  `rfc1928/client.rs:217-219` remains accepted-uncovered (with its
  translated-error note), as do the fork's other accepted-uncovered
  blocks.
- **Residual uncovered lines after this task (none new)**: producer
  .rs 188-189 (the pump handler's `accept_bi` substrate-failure log
  arm: pre-existing, not in N-8's list, and untestable without a
  broken `Connection` outside alkcall's test surface),
  rfc1928/client.rs 217-219 + the fork internals (accepted-
  uncovered per review-001/002), wire.rs/mod.rs/target_addr.rs
  internals (pre-existing, not in N-8's list), and the test modules'
  own panic-guard/unreachable lines (coverage-of-tests noise).

## Summary

Closed review 002's N-8 coverage residue with 18 new tests and zero
production-code changes: the datagram stage's three failure exits
pinned as direct unit tests on `run_datagram_stage`
(`ReplyFrameRejected`, `EgressEnded` via `next_reply`, `Io` via a
failing write and a failing read) plus the serve-layer drop-witness
test for the `send`-failure arm (the egress drops with the stage on
the failure path too); the two front-door-bridge `split`s tested on
live `Socks5Stream<BiStream>` streams (raw bytes through both
halves; the BIND split leaves the take-guard untouched); and the
single-arm residue (spent `send_datagram`'s `Io` arm + the spent
`recv` shape, the mid-batch later-frame `Err` dropping pending `Ok`s,
ASSOCIATE `into_inner`'s `Ok` pairing, `BindDataPlane`'s
`poll_flush`/`poll_shutdown` passthroughs, `read_reply2`'s version
pin, the quiet wait's `Err(_)` arm, the BIND reply#2 Domain-peer
fallback `0.0.0.0:port`, the three egress `Debug` renders, the
`SocksEstablishmentReason` `Display`, the ASSOCIATE/BIND session
`Debug` bodies, the spent-`close` no-op, and the establisher's
`None`-witness arm). `cargo llvm-cov` re-run: every N-8-named line
covered, no new accepted-uncovered entries (the residual uncovered
lines are the pre-existing out-of-scope set + producer.rs 188-189's
untestable `accept_bi` log arm). Verification: `cargo test` +
`--all-features` green (375 tests), clippy `-D warnings` clean
(native + wasm), fmt clean, `cargo doc` clean, both wasm checks
clean; coverage 96.31% regions / 97.22% lines.