alktunnels/tasks/tunnels/review-002-g2-graceful-teardown-observation.md

id, name, status, depends_on, scope, risk, impact, level, tags

id	name	status	depends_on	scope	risk	impact	level	tags
tunnels/review-002-g2-graceful-teardown-observation	G-2 — observe close()'s EOF sentinel at the far accept dispatcher (+ N-2's write-happy-peer teardown pin)	completed		narrow	low	component	implementation	tests consumer bind-first teardown review-002 review-remediation

Description

Review 002 coverage gaps G-2 and the N-2 branch note, batched (the review itself suggests the same harness shape covers both — see its §Branch-coverage notes, last bullet).

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 only at the reap level (close_and_drop_reap_the_listen_channel) — 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.

This is the only coverage gap touching a promised behavior (ADR-008 §1's teardown contract, graceful half) with no direct observation.

Test: mirror bindfirst_full_flow…'s harness (the tests/consumer_listener.rs bind-first topology) with a close() instead of a drop — open against a bind-first producer, push an accept, listener.close().await, then assert the far accept loop is gone (the same queue-len / never-consumed assertion the Drop tests use) and the serving-side listen channel reaped.

N-2's testable residue — the write-happy-peer teardown posture

The idle-stream 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 (fail-closed; the channel is not a data plane; the doc comment owns this). N-2 records the shape as correct, but the review's branch-coverage notes flag that the Ok(_) (non-empty read) arm is untested: a test would write a byte to the idle channel and assert far teardown, pinning the "any non-empty read tears down" posture.

Test: same harness — open the listen channel, write a byte into the idle stream from the opener side, assert the accept loop cancels and the listener tears down (the fail-closed posture).

Acceptance Criteria

• The graceful-close() far-teardown test added (the sentinel's far-side effect observed)
• The write-happy-peer teardown test added (the Ok(_) idle-read arm pinned)
• cargo test --features local green; clippy --all-targets -D warnings clean (native + wasm32); fmt clean

References

• docs/reviews/002-graduation-implementation-review.md §G-2 (Coverage appendix), §N-2, and §Branch-coverage notes (last bullet)
• src/consumer.rs:529-531 (TunnelListener::close), src/producer.rs (the idle-stream EOF watch)
• tests/consumer_listener.rs (the harness shape to mirror)
• docs/architecture/decisions/008-listen-metadata-forwarded-op.md §1 (the teardown contract's graceful half)

Notes

Agent fills during implementation.

Filled (2026-09-20):

• Both tests landed in tests/consumer_listener.rs — no src/ changes needed (pure coverage additions):
  • graceful_close_eof_sentinel_observed_at_far_accept_dispatcher (G-2): mirrors the bind-first full-flow harness with a close() instead of a drop. Proves the accept loop alive first (a consumed accept), then asserts after listener.close().await that the serving-side listen channel reaped AND a follow-on pushed accept is never consumed (the same queue-len assertion the Drop-shaped tests use) — the sentinel's far-side effect for the graceful path.
  • write_happy_peer_on_idle_listen_channel_tears_down_far_listener (N-2 residue): opens the listen channel via the raw client surface (open_channel_with_reply, both halves held alive), proves the accept loop alive, writes one byte into the idle stream, and asserts the fail-closed teardown (channel reaped + the follow-on accept never consumed). The typed TunnelListener never exposes its idle write half by contract, so the misbehaving-opener shape requires the raw client — that shape difference is the point.
• Harness notes: the write-happy test installs NO forwarded-op serving stub — the pre-write consumed accept fails its forwarded open (per-accept isolation), which is fine: the assertion only needs the queue emptied (loop alive), and the isolation path is pinned by open_only_opener_still_opens_listen_but_forwarded_fails. The listener session's own _recv half is dropped in the write-happy test (the write half alone keeps the pump alive; the recv half is the opener's unused read side of the idle channel).
• Verification: cargo test --features local green (163 tests across all targets + 2 doc-tests), cargo clippy --all-targets --features local -- -D warnings clean, cargo clippy --target wasm32-unknown-unknown -- -D warnings clean, cargo check --target wasm32-unknown-unknown clean, cargo fmt --check clean.