--- id: review-wave-4 name: "Wave 4 gate: the client-direction review" status: completed depends_on: [w4-dial, w4-op-rows, w4-reverse-listen, w4-lifecycle, w4-git-exit-close, w4-channel-not-held-rows, w4-tty-coverage, w4-router-collision] scope: narrow risk: low impact: phase level: review --- ## Description The wave-4 review gate. The client direction completes the two-direction door, and wave 5 builds the full matrix and the release on both directions, and the published crate freezes this surface. The gate reads the landed client direction against the architecture line by line: - **client.md line-by-line**, reading it section by section: §The dial surface (key source decrypt-at-dial fail-closed; the policy at `check_server_key`; the bounded knobs with the keepalive 15 s / 3 posture), §Op rows (each table row read against the landed code, the POC-3 §D.3 mechanics pinned in code + rows), §The acceptor (served only under a registered feature's acceptor; unregistered/unfeatured declined cleanly, never accept-and-drop; late-install across a live connection), §Lifecycle primitives (explicitly cancellable handles; `Disconnect::ByApplication`; **no reconnect/restore policy in the crate**, a boundary held). - **ADR-003 symmetry**, where the same features gate both directions; the per-build matrices green client-side (the acceptor rows per build); honest advertisement unchanged; the sftp placeholder declines in every build both directions (OQ-13); x11/auth-agent never-features untouched. - **ADR-005 / OQ-05 / OQ-07**, decrypt-at-dial with every `KeySourceError` posture a no-dial outcome; the policy seam consumed at the hook (crate default connects to nothing; no accept-all built-in); the `Zeroizing` wrap posture intact. - **The wave-3 gate's booked items 1–4**, every one discharged or explicitly re-booked: the git status-before-close by construction (`w4-git-exit-close`), the channel-not-held wire rows (`w4-channel-not-held-rows`), the tty coverage minors (`w4-tty-coverage`), the router token-collision pre-check (`w4-router-collision`). Item 5 (the upstream asks) verified still booked with their owners, not absorbed silently. - **Bounded resources**, with no new unbounded shape on the client surfaces: the acceptor registry bounded, the listener bookkeeping bounded, the pumps bounded (the convention-9 read over every new serving/pumping path). - **Tracing**, the uniform event vocabulary across the new client-side decline/serve paths; the safety posture held (no secrets, no key material, no username). - **The smokes**, with the client-direction real-tool postures recorded honestly (required/conditionally-skipped with reason); nothing silently green; anything named wave-5 e2e territory named explicitly. - **Docs honesty**: module docs name what each new surface is, what remains wave-5 material, and any new upstream notes (the russh client-hook shapes at this pin; the acceptor-registry shape decision; the DialConfig disposition). Verdict: green (wave 5 decomposes, absorbing corrections) or findings booked (remediation before wave 5 decomposes). ## Acceptance Criteria - [x] Every client.md/ADR surface above read and confirmed (or corrected with the correction landed) - [x] The client-side per-build matrices verified through the server double; the acceptor contract verified (registered/unregistered/ unfeatured/late-install); the wave-3 booked items 1–4 verified discharged - [x] Full verification set green (`cargo test`, `--all-features`, clippy ×2 `-D warnings`, fmt, `cargo doc --no-deps`) - [x] `taskgraph validate` clean - [x] Verdict recorded; wave 5 decomposition unblocked or findings booked ## References - docs/plans/implementation.md (wave 4 section + this gate's role) - docs/architecture/client.md (the wave's contract), mapping.md (the server-direction mirror), overview.md, open-questions.md (OQ-05/ 07/11/12/13), ADRs 002/003/005/006 - tasks/implementation/review-wave-3.md (the booked items this gate verifies discharged) - docs/research/reverse-runner.md (the consumer pattern the client direction serves), docs/research/poc-3-mapping-bakeoff.md §D.3 ## Notes ### Gate reading (line by line, 2026-10-29; commits 9cf5bfb…dec644e) Commits verified against the decomposition: all eight tasks' hashes match `docs/plans/implementation.md`'s decomposition record; every task file carries its completion records and checked acceptance criteria. - **client.md line-by-line** ✓, read against the landed code: §The dial surface (`client.rs`, the four-step fail-closed sequence: decrypt-at-dial key load, the bounded russh `Config` with the keepalive 15 s / 3 posture and the always-set wall clock, the policy at `check_server_key`, the single publickey-only auth; the sanctioned `username` `DialConfig` amendment recorded honestly (a required parameter, no crate default, never identity, never logged); §Op rows, covering `client_ops.rs`, where every table row is present: the exec/pty/shell rows under `session`, the direct rows under `tunnel`, the git exec row under `git` over the shared grammar's construction arm, the sftp placeholder in every build; POC-3 §D.3 mechanics pinned in code plus rows (`exec()` not `request_exec`, `request_exec`, the `wait()`-drain Success/Failure, split-halves wait-pumps for the exit-consuming rows and straight `into_stream` for the exit-less, the fire-and-forget controls sent from the live pump, no queue consumption before any conversion); §The acceptor (`client_accept.rs` + the `client_handler.rs` hooks, being per-type fixed slots, refuse-never- replace, arrival-time reads, decline-not-buffer, late-install over the live connection, listen rows gated pre-wire, the two dedicated counters against the one ceiling); §Lifecycle primitives (`client_accept.rs` handles + `client.rs` §Lifecycle posture, with explicit cancels, `Disconnect::ByApplication` the only clean path, drop-is-never-the-clean-path row-pinned both ways, the no-reconnect/restore-policy boundary stated in module docs; the crate owns no policy). client.md's landing blocks (acceptor/ lifecycle/op-row shapes recorded at landing) are in the doc. - **ADR-003 symmetry** ✓, as the features gate both directions (`Cargo.toml` unchanged: `session`/`git`/`tunnel`/`sftp`, `default = []`); the client exports ride the same gates (`RemoteSession` under `session`, `TunnelChannel` + the acceptor surface + `ReverseListener` under `tunnel`, `GitPackSession` under `git`, the sftp row feature-free); the per-build matrices are green (the default build's 16 binaries plus per-feature/ `--all-features`; the unfeatured builds' fail-closed wire rows ride `tests/client_dial.rs`'s decline-all row, and the `client_handler.rs` hooks are cfg-declined in every build); honest advertisement unchanged; x11/agent never-features untouched on both directions. - **ADR-005 / OQ-05 / OQ-07** ✓, decrypt-at-dial with every `KeySourceError` posture a no-dial outcome (`DialError::KeySource` preserved per-variant; `error.rs` taxonomy rows); the policy seam consumed at the hook (`client_handler.rs::check_server_key`, where the fingerprint rides the crate's single path, `PresentedHostKey` carries host/port, the verdict passes through; `RejectAll` connects to nothing, no accept-all built-in); the wave-1 `Zeroizing` wrap posture intact (`key_source.rs`, untouched). - **The wave-3 gate's booked items 1–4** ✓, all discharged: item 1 (`w4-git-exit-close`), the split-halves seam duplex (`ChannelHalveReader` over the owned close-free read half; no drop path ever messages the wire) with the by-construction property wire-pinned and the row falsifying the old shape deterministic-red-under-revert; item 2 (`w4-channel-not-held-rows`), the guard's wire rows on all three served rows per build, vocabulary-asserting; item 3 (`w4-tty-coverage`), the `channel_close`-fired cancel path wire-witnessed, the Closed/Break no-exit-chunk shape row-pinned (Break's frame classification unit-rowed, wire-unreachable, recorded), the `write_all` no-deadline limit recorded in module docs; item 4 (`w4-router-collision`), the register-refuses pre-check with the error-surface decision recorded honestly (the wire-time decline disposition, not a taxonomy variant that would kill the session). **Item 5 verified still booked with its owners and not absorbed**: no `set_identity`/`take_call_connection` anywhere in `src/` code (doc references only, recording the no-detach posture + the upstream ask), and mapping.md's correlation-miss event-vocabulary nuance records stand. - **Bounded resources** ✓, with the convention-9 read over every new client serving/pumping path showing the acceptor registry bounded by construction (two slots); the reverse-listen count and the accepted-open handoff count are two dedicated `ChannelCap` counters against the same `Limits::max_concurrent_channels` ceiling (admit pre-wire/arrival, release at confirmed cancels / refused paths / the handoff task's end); the pumps bounded (64 KiB duplexes, the 16 KiB git pump buffer, the 1024-message launch-drain cap, russh windows verbatim, the always-on wall clock); the git row's command/repo gates answer pre-wire. One gap found and fixed inline (below). - **Tracing** ✓, with the client-side decline paths riding the uniform vocabulary (`decline_open` with `no-acceptor` / `cap-exceeded` / `never-feature`; the skeleton's decline rows event-pinned); the safety posture held, as structurally no username/fingerprint/secret parameter exists on any event helper (the `&'static str` vocabulary); the client op rows' typed surfaces also carry outcomes without events (the recorded asymmetry decision). - **The smokes** ✓, three recorded honestly: the alkssh-client ↔ alkssh-server dial smoke (`tests/client_dial.rs`), the client-exec ↔ server-session-rows smoke (`tests/client_ops.rs`, check-gated on `sh`), the full-circle reverse-listen smoke (`tests/tunnel_mapping.rs`); real-`sshd` e2e named wave-5 territory explicitly in each task's records, with nothing silently green, nothing dropped. - **Docs honesty** ✓, as module docs name each new surface and its remaining wave-5 material (the fake-`sshd` posture, the DialConfig amendment, the russh client hook shapes at this pin, the acceptor-registry shape decision, and the recorded wire facts the port-0-only mint, the drop/is-closed semantics); one stale crate-level sentence fixed inline (below). ### Minor issues found and fixed inline (this session) - **The accepted-open handoff counter's panic-unsafe release**, the gate's one code fix: the counter released only after `acceptor.accept(...)` returned, so an assembly acceptor panicking inside `accept` would strand one held slot for the connection's lifetime, a bounded-resource leak path on the new cap. Fixed with a panic-safe guard (`HandoffCapGuard` in `src/client_accept.rs`): the release fires at the handoff task's end, whether on completion or unwind; the recorded release-site wording updated in the module docs, the trait docs, and `client_handler.rs`'s two references; a unit row (`the_handoff_guard_releases_on_unwind`) pins the unwind release. No surface change, no behavior change on any happy path. - **`lib.rs` crate-level wave-state doc** still said the lifecycle primitives were "later implementation waves" (stale over `w4-lifecycle`; the same staleness class the wave-3 gate fixed inline). Corrected to the landed state. ### Interpretations (flagged at decomposition), all confirmed 1. **The fail-closed client Handler skeleton in `w4-dial`, the registry in `w4-reverse-listen`**, confirmed: the skeleton's decline-all default is row-pinned in every build, and the registry grows exactly the two forwarded hooks; the split kept both surfaces reviewable. 2. **The fake-`sshd` client-direction vehicle in `tests/common`**, confirmed: vanilla-parity, real loopback TCP, one dial per instance, and the harness's `ForwardedAcceptor` vocabulary was grown (not forked) into `client_accept.rs`'s real surface. 3. **No `DialConfig` course-correction anticipated** was confirmed, with the one sanctioned exception recorded in-task (the required `username`, the USERAUTH_REQUEST's structural requirement; the amendment follows the `w3-seam-config` pattern and is honestly documented). 4. **The hardening tasks ordered by single-writer needs** were confirmed: `tests/git_mapping.rs` then `tests/tty_mapping.rs` sequencing held, no cross-task file conflicts in the landed history. ### Verification (all green, 2026-10-29) `cargo test` (default build) · `cargo test --all-features` (all 16 binaries green) · `cargo test --features tunnel` (the gate's per-feature spot re-run, covering the edited acceptor surface + the new guard row) · `cargo clippy --all-targets -- -D warnings` ✓ · `cargo clippy --all-features --all-targets -- -D warnings` ✓ · `cargo fmt --check` ✓ · `cargo doc --no-deps` 0 warnings · `taskgraph validate` ✓ (35 tasks). ## Summary **Verdict: green.** The client direction reads clean against client.md/ADR-003/ADR-005 line by line: the four-step fail-closed dial over the bounded config (the sanctioned `username` amendment recorded), the op rows with the POC-3 mechanics pinned in code (split-halves wait-pumps vs straight conversions; the typed-open decline mirror family-typed; the sftp placeholder in every build), the acceptor registry as the refuse-never-replace per-type-slot gate with arrival-time declines and bounded two-counter bookkeeping, and the lifecycle primitives with the drop/clean-close boundary row-pinned both ways and the no-policy posture stated. All three interpretations confirmed except as recorded (all favorable); the wave-3 booked items 1–4 verified discharged and item 5 verified still booked upstream, with nothing absorbed. The full verification set is green. Two minors fixed inline (the handoff counter's panic-unsafe release, the one code fix with its guard and unit row; a stale crate-level wave-state doc sentence). No structural findings; wave 5 decomposition is unblocked and should absorb nothing beyond its own plan (the decline matrix, real-tool e2e, fuzzing, docs, the publish). ## Summary **Verdict: green** (2026-10-29); see Notes for the full gate reading. All eight wave-4 tasks verified against the client.md/ADR contract surfaces line by line; the wave-3 gate's booked items 1–4 discharged, item 5 verified still booked upstream; the full verification set + `taskgraph validate` green. Two inline fixes landed with the gate: the accepted-open handoff counter's panic-safe release (`HandoffCapGuard`, the gate's one code fix) and the stale crate-level wave-state doc sentence in `lib.rs`. Wave 5 decomposition is unblocked.