alkssh/tasks/implementation/review-wave-3.md

---
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<Msg>` 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.