Files
alkssh/tasks/implementation/review-wave-3.md
T
glm-5.3-flash 930d9c1996 docs(review): close the wave-3 review gate — verdict green (review-wave-3)
The wave-3 gate read the landed server mappings against
mapping.md/ADR-006/ADR-003/auth.md line by line: session/pty/shell/pipe
(translation bridge + the derivation contract rules 1-6), git exec
(full grammar, no-oracle validation, collapsed denial byte order),
route-1 bridge direct rows, the full -R listen flow + forwarded-tcpip
data leg, per-build decline matrices, identity observed at every seam,
bounded bookkeeping, and the uniform safe tracing vocabulary. All four
decomposition-time interpretations confirmed.

Fixed inline (this gate):
- install_forwarded_surface pre-checks the tunnel/forwarded op —
  alkcall's register silently replaces an existing entry, so a second
  adapter over the same shared seam registry silently stole the first
  adapter's arrival routes; now a failed construction
  (AdapterError::ListenRegistration), pinned by a new wire row
- git take drops a stored pty entry (combined-build stale state)
- doc hygiene: channel_cap release ordering, the session-row scoping
  of the handler Drop, the cancel-order recording, the post-cancel
  arrival caveat, the exit-chunk ADR lineage cross-ref, the stale
  lib.rs wave-state doc, the superseded dispatch test name, and
  mapping.md's listen-resource wording (address verbatim + tcp port)

Booked for wave 4: the git row's status-before-close honored by
scheduling not construction (needs a seam/shape change), the
channel-not-held wire test rows, the tty coverage minors (cancel path,
Closed/Break shape, stdout write deadline), and a defensive
ListenRouter token-collision pre-check.

Verified: cargo test (default) + --all-features (173 rows, 13
binaries), clippy --all-targets -D warnings ×2 (default +
--all-features), fmt --check, cargo doc --no-deps (0 warnings),
taskgraph validate (26 tasks).
2026-10-29 05:22:14 +00:00

16 KiB
Raw Permalink Blame History

id, name, status, depends_on, scope, risk, impact, level
id name status depends_on scope risk impact level
review-wave-3 Wave 3 gate — the server-mappings review completed
w3-seam-config
w3-decline-tracing
w3-git-mapping
w3-tty-session
w3-tunnel-direct
w3-tunnel-listen
narrow low phase review

Description

The wave-3 review gate. The server mappings are the crate's spine landed in wire-capable form — 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 — 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 — feature on, 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 — 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 — 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 — 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 — 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

  • Every ADR/mapping surface above read and confirmed (or corrected with the correction landed)
  • 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
  • Full verification set green (cargo test, --all-features, clippy ×2 -D warnings, fmt, cargo doc --no-deps)
  • taskgraph validate clean
  • 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: §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; 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 — 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 ✓ — 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 — 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 ✓ — 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 ✓ — 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 — 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 — 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 — 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 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 — 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.