--- id: review-wave-3 name: "Wave 3 gate: the server-mappings review" status: completed depends_on: [w3-seam-config, w3-decline-tracing, w3-git-mapping, w3-tty-session, w3-tunnel-direct, w3-tunnel-listen] scope: narrow risk: low impact: phase level: review --- ## Description The wave-3 review gate. The server mappings are the crate's spine landed in wire-capable form, and the wave-4 client direction reuses the same grammar/rows pointed upstream, wave 5 builds the full matrix on these rows, and the published crate freezes this surface. The gate reads the landed mappings against the architecture line by line: - **mapping.md line-by-line**, covering each of the spine's server rows (session/pty/shell/pipe; git exec; direct-tcpip; direct-streamlocal; forwarded-tcpip; the global listen rows; the request-level table) read against the landed code; the §exit derivation contract rules 1–6 verified in code + rows; the decline matrix green **per feature build** through the harness (including the unset-field rows, with the feature on and the handle unset). - **ADR-006**, route-1 bridge (accept only after the fabric open succeeds; `ConnectFailed` on failure), exec-only git (no subsystem dispatch anywhere in the grammar), parse-only dispatch still structural (no request-less serving), spawn-and-return over the **real** serving paths (the wave-2 constraint re-pinned over mappings, not stubs). - **ADR-003**, the per-build matrices; honest advertisement unchanged (an unfeatured type declines; the sftp feature still gates nothing, its rows still decline in every build, per OQ-13); x11/auth-agent never-feature rows untouched. - **Auth.md / identity**, the identity ride observable at every seam (alktty/alkgit params; the channels connection for tunnels), `set_identity` once-only mechanics recorded; the parked wave-2 `auth_succeeded` item verified discharged; no username anywhere. - **OQ resolutions in code**, namely OQ-08 (unset-declines on shell/wrapper rows), OQ-09 (env declined), OQ-10 (derivation rules + the PTY-path fidelity limit documented), OQ-11 (runtime-Unix gates compile-everywhere + clean off-Unix declines), OQ-13 (sftp rows unchanged). - **Bounded resources**, with no new unbounded shape: the cap across the fabric window (release-on-failure pinned), the listener bookkeeping bounded, the control-row surfaces bounded, the bridge adapters bounded. - **Tracing**, the uniform event vocabulary across all landed serve/decline paths; the safety posture held (no secrets, no username). - **The smokes**, with the three real-tool postures recorded honestly (required/conditionally-skipped with reason); nothing silently green. - **Wave-2 gate's booked items**: every one discharged or explicitly re-booked (decline tracing = landed; acceptor vocabulary = landed in `w3-tunnel-listen`; identity landing = discharged; the inherited-auth-agent-open watch item = unchanged or re-read at any pin touch). - **Docs honesty**: module docs name what each replaced stub now is, what remains wave-4/5 material, and any new upstream notes (portable-pty's PTY-path exit shape; the `take_call_connection` semantics reading). Verdict: green (wave 4 decomposes, absorbing corrections) or findings booked (remediation before wave 4 decomposes). ## Acceptance Criteria - [x] Every ADR/mapping surface above read and confirmed (or corrected with the correction landed) - [x] The decline matrix verified per build through the harness; the derivation contract verified against its rules; the spawn-and-return constraint re-pinned over real serving paths - [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 4 decomposition unblocked or findings booked ## References - docs/plans/implementation.md (wave 3 section + this gate's role) - docs/architecture/mapping.md, auth.md, client.md (the mirror), overview.md, open-questions.md, ADRs 002/003/004/005/006 - docs/research/poc-3-mapping-bakeoff.md (stages A/C/D, the evidence the contract tests re-land) ## Notes ### Gate reading (line by line, 2026-10-29; commits 270365e…3d0253d) - **mapping.md line-by-line** ✓, each spine row read against the landed code: the session/pty/shell/pipe rows (translation bridge, OQ-08 argv conventions, pipe-less shell serves pipe-mode), the git exec row (full grammar, no-oracle validation, alkgit dispatch), the direct-tcpip / direct-streamlocal rows (route-1 bridge), the forwarded-tcpip row (echo args + bound port, teardown-at-birth on correlation miss), the global listen rows (verbatim resource + bound-port out-param; one doc correction landed there: §The listen dial now states the port appending, the code's `host:port` form), the request-level table, and the derivation contract rules 1–6 in `derive_exit` (incl. `i32::MIN` via `checked_abs` → 255, `core_dumped` never, rule 3's table entry unreachable by construction, the PTY-path rule-5 limit documented). The decline matrix is green per feature build through the harness (per-build cfg-pinned modules; the unset-field rows (feature on, handle unset) present in every featured build). - **ADR-006** ✓, route-1 bridge (accept only after the fabric open; `ConnectFailed` on failure; the wave-2 stub retired into the real decline), exec-only git (no subsystem dispatch anywhere in the grammar; git service names decline), parse-only dispatch (no detect residue), spawn-and-return re-pinned over the real serving paths (the wave-2 mechanics rows still green; the inline listen dial is the one recorded exception, bounded by `LISTEN_DIAL_TIMEOUT` with the wire-necessity reasoning documented). - **ADR-003** ✓, per-build matrices; honest advertisement unchanged; the sftp feature still gates nothing and its rows decline in every build (OQ-13); x11/auth-agent never-features untouched; feature-to-dep isolation (`cargo tree` proven in `w3-seam-config`). - **Auth/identity** ✓, the resolved identity observed at every seam: alktty (the ownership witness row), alkgit (the authorize/ deny row), tunnels (the CF-005/CF-006 establisher + ACL rows); `set_identity`/`take_call_connection` appears **nowhere in src/** (the detach decision recorded, the upstream ask booked (see below for final disposition); the wave-2 `auth_succeeded` parked item discharged as ride-along store; no username anywhere (the safety-scan row). - **OQ resolutions in code** ✓, OQ-08 (unset-declines, both field classes, `wrapper ++ [cmd]` verbatim-append), OQ-09 (env declined unconditionally), OQ-10 (derivation rules + PTY-path limit documented), OQ-11 (runtime-Unix gates compile-everywhere with dial-call-witness-at-zero off-Unix rows), OQ-13 (sftp rows unchanged in every build). - **Bounded resources** ✓, with the cap across the fabric window (admit at the hook, release-before-reject on fabric failure, disjoint saturating release sites verified per row), the listener bookkeeping bounded by a dedicated `max_concurrent_channels`-ceiling counter, the route map bounded by construction (minted only at accepted listens, removed at cancels + session teardown, and the `Drop` really removes from the shared router), the control-row surfaces bounded (`try_send`, never awaited on the packet loop), the bridge adapters bounded (mpsc 64, 64 KiB duplexes, 16 KiB read buffers, `MAX_CHUNK_LEN` frame gates). No unbounded shape found in any serving path. - **Tracing** ✓, the uniform event vocabulary at every decline/serve site (row + feature_build + channel_id + class + reason + exec_route; declines `debug`, the one `warn` the serving-failure arm), safety posture pinned by scan (no username, no fingerprint, no secret ever in an event). - **The smokes** ✓, git clone 2.43 / `ssh -L` + curl / `ssh -tt` pty / `ssh -R` + curl: all four landed and green in this environment; every one carries a check-gated skip-with-reason posture, nothing silently green. - **Wave-2 booked items** ✓, with decline tracing landed (`w3-decline-tracing`); the acceptor vocabulary landed (`w3-tunnel-listen`: harness accept hook + per-arrival cap); the deferred identity-landing assertion discharged (each seam rides the resolved identity, wire-witnessed); the inherited `auth-agent`/`forwarded-streamlocal`-open watch item unchanged (no pin touch this wave). - **Docs honesty** ✓, as each replaced stub's module docs name what it now is and what remains wave-4/5; the `set_identity` reading, the resource-naming convention, the inline-dial bound, and portable-pty's PTY-path exit shape are all recorded in module docs (one overbroad Drop doc corrected inline, below). ### Interpretations (flagged at decomposition), all confirmed 1. **Whole-`Option` struct per seam** (vs flat `Option` fields), confirmed: each seam is consumed atomically by exactly one seam call; the flat single-field tunnel handle stays flat. Recorded in `w3-seam-config`. 2. **The server-side sftp bullet already discharged in wave 2**, confirmed: the rows are in `tests/dispatch_core.rs` per build. 3. **The `-R` listen flow's biggest open closed without a fork**, confirmed: the topology closed over the published alkcall 0.10.1 / alktunnels 0.4.1 surfaces via the `TunnelListenSeam` course-correction; a scratch probe (published crates only) proved the flow before landing; nothing forked or absorbed. 4. **The wave-2 booked items' wave-3 homes**, confirmed as above. ### Minor issues found and fixed inline (this session) Source/doc fixes (all commit-tested; suite re-run green after): - **Duplicate `tunnel/forwarded` registration now fails construction** (the gate's one real fix): alkcall's `OperationRegistry::register` silently *replaces* an existing op entry, contradicting the `install_forwarded_surface` docs' recorded fail-closed claim: a second adapter over the same shared seam registry would silently steal the first adapter's arrival routes. `install_forwarded_surface` now pre-checks the op (`registry.registration(OP_TUNNEL_FORWARDED)`) and refuses with `AdapterError::ListenRegistration`; new wire-level row `a_second_adapter_over_a_shared_listen_registry_fails_construction` pins both constructs/refuse. - **channel_cap.rs**, the bridge-failure release doc said "after the reject is answered"; the code (and the pinned test) release *before* the reject. Doc corrected. - **handler.rs**, the `Drop` doc claimed "every spawned serving task watches" the teardown signal; scoping corrected (the session rows watch; the git task ends through the duplex, bounded by alkgit's `session_wall_clock`). The cancel-form doc's recorded ordering (router-before-map) was inverted vs the code; corrected, with the post-cancel-race caveat recorded. - **tunnel_map.rs**, the "never half-serves" bookkeeping claim now records the one-arrival caveat: an arrival that cloned its route just before a cancel completes independently (never half-open; sshd parity). - **tty_map.rs**, a stale ADR cross-ref on the exit chunk (alktty ADR-005 → ADR-004 on the alknet ADR-055 lineage). - **lib.rs**, the crate-level wave-state doc still said "wave 2 decomposes after this gate"; updated to the landed state. - **tests/dispatch_core.rs**, the stale `unknown_git_services_route_to_the_pipe_row` name/doc superseded by the w3 decision ("off the pipe row"); renamed, verdicts preserved (the wire shape coincides, which is why it never failed). - **handler.rs (git row)**, a pty-then-git-exec in a combined build left the stored pty entry until close; the git take now drops it (the pipe row's consumption shape). - **mapping.md**, the §listen-dial resource wording corrected to the code's assembled form (address verbatim + tcp port appended; `0` stays `0`). ### Booked findings (not remediation this gate, wave-4 material) 1. **The git row's status-before-close is honored by scheduling, not by construction** (the gate's only structural moderate): `serve_duplex_resolved` splits the channel stream internally and its session halves drop when the serve returns, so russh's `ChannelCloseOnDrop` rx-half emits a server→client `CHANNEL_CLOSE` *inside* the seam call, before `serve_git_row`'s explicit `exit_status_request`. Today the ordering holds (the task's continuation queues the report in the same poll, ahead of the drop's send) and every wire row passes, but a full `Sender` buffer at the serve tail could order the implicit close first and silently drop the report (russh no-ops `exit_status_request` for an already-closed channel). A fix wants a seam/shape change (split halves into the seam, or queue the report while the stream is alive), beyond this gate's inline budget; book a wave-4 hardening item. (The tty rows are NOT in this class: their report precedes any close-contracting drop in one task.) 2. **The `channel-not-held` guard lacks wire test rows**, a documented decline class on all three rows (git/tty pipe/shell), constructible on the wire (a second exec on a served channel), but no row asserts it in any build; mapping.md's every-row tested-in-every-build convention names it contract territory. Test-only task; book for wave 4's decomposition. 3. **Tty coverage gaps** (minors, one ticket's worth): the `channel_close`-fired per-channel cancel path is unexercised on the wire (only the teardown signal is); the Closed/Break no-exit-chunk close shape has no wire row; the tty stdout `write_all` carries no deadline (backpressure bounded, liveness covered by inactivity timeout; record it as a known limit only). 4. **Defensive `ListenRouter` token-collision pre-check** (minor): `register` silently replaces on token collision; safety rests on the fabric's ADR-008 mint discipline. A fail-closed decline-the-listen-on-collision is a cheap wave-4 addition. 5. **Upstream asks to file** (booked with their owners, not this crate): the alktunnels/alkcall "non-detaching post-construction identity seam" note (`w3-tunnel-direct`'s recorded decision, under which the door keeps the no-detach posture until upstream proposes a seam), and mapping.md's recorded event-vocabulary nuance for the unreachable mismatch arms (`correlation-miss` naming). No blocking issues; no code shape errors found. Wave 4 decomposition is unblocked and should absorb items 1–4 above. ### Verification (all green, 2026-10-29) `cargo test` (default build) · `cargo test --all-features` (173 rows across 13 binaries) · `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` ✓ (26 tasks). ## Summary **Verdict: green.** The wave-3 server mappings read clean against mapping.md/ADR-006/ADR-003/auth.md line by line: the translation bridge + derivation contract, the exec-only git grammar with the no-oracle dispatch and the byte-pinned collapsed denial, the route-1 bridge and the full `-R` listen flow closed over the published surfaces, per-build decline matrices, identity observed at every seam, bounded bookkeeping throughout, and a uniform safe tracing vocabulary. All four flagged interpretations confirmed; one real fail-closed hole fixed inline (duplicate listen registration) plus eight doc/test-hygiene minors; four findings booked for wave 4 (the git exit-report close race, the `channel-not-held` wire rows, the tty coverage minors, the router token-collision pre-check). Wave 4 decomposition is unblocked.