docs(review): close the wave-4 review gate — verdict green (review-wave-4)
- Gate reading recorded line by line: client.md (dial surface, op rows, acceptor, lifecycle primitives), ADR-003 symmetry, ADR-005/ OQ-05/OQ-07, bounded resources, tracing, smokes, docs honesty — all confirmed - Wave-3 gate booked items 1-4 verified discharged; item 5 verified still booked with its owners (nothing absorbed) - Inline fix: the accepted-open handoff counter now releases on completion or unwind (HandoffCapGuard panic-safety; a panicking assembly acceptor can no longer strand a held slot) + unit row - Inline fix: lib.rs crate-level wave-state doc sentence (stale over w4-lifecycle) - w4-reverse-listen / w4-lifecycle Summary placeholder lines filled Verification: cargo test (default / --all-features / --features tunnel) all green; clippy x2 -D warnings clean; cargo fmt --check clean; cargo doc --no-deps 0 warnings; taskgraph validate (35 tasks)
This commit is contained in:
1 parent
dec644e653
commit
671a779ce3
6 files changed
+272
-26
No files matched your search
@@ -1,7 +1,7 @@
|
||||
---
|
||||
id: review-wave-4
|
||||
name: Wave 4 gate — the client-direction review
|
||||
status: pending
|
||||
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
|
||||
@@ -64,16 +64,16 @@ booked (remediation before wave 5 decomposes).
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [ ] Every client.md/ADR surface above read and confirmed (or
|
||||
- [x] Every client.md/ADR surface above read and confirmed (or
|
||||
corrected with the correction landed)
|
||||
- [ ] The client-side per-build matrices verified through the server
|
||||
- [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
|
||||
- [ ] Full verification set green (`cargo test`, `--all-features`,
|
||||
- [x] Full verification set green (`cargo test`, `--all-features`,
|
||||
clippy ×2 `-D warnings`, fmt, `cargo doc --no-deps`)
|
||||
- [ ] `taskgraph validate` clean
|
||||
- [ ] Verdict recorded; wave 5 decomposition unblocked or findings
|
||||
- [x] `taskgraph validate` clean
|
||||
- [x] Verdict recorded; wave 5 decomposition unblocked or findings
|
||||
booked
|
||||
|
||||
## References
|
||||
@@ -89,8 +89,192 @@ booked (remediation before wave 5 decomposes).
|
||||
|
||||
## Notes
|
||||
|
||||
> To be filled by implementation agent
|
||||
### 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 (`client_ops.rs` — every table row 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 + rows — `exec()` not
|
||||
`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 — 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 — 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** ✓ — 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 + per-feature/
|
||||
`--all-features`; the unfeatured builds' fail-closed wire rows
|
||||
ride `tests/client_dial.rs`'s decline-all row — 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` — 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, 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** ✓ (convention 9 read over every new
|
||||
client serving/pumping path) — 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** ✓ — the client-side decline paths ride the uniform
|
||||
vocabulary (`decline_open` with `no-acceptor` / `cap-exceeded` /
|
||||
`never-feature`; the skeleton's decline rows event-pinned); the
|
||||
safety posture held — structurally no username/fingerprint/secret
|
||||
parameter exists on any event helper (the `&'static str`
|
||||
vocabulary); the client op rows' typed surfaces 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 — nothing silently
|
||||
green, nothing dropped.
|
||||
- **Docs honesty** ✓ — 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, 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 — an assembly acceptor panicking
|
||||
inside `accept` stranded 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, 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** — 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** —
|
||||
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
|
||||
|
||||
> To be filled on completion
|
||||
**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 — nothing absorbed. The full verification set
|
||||
is green. Two minors fixed inline (the handoff counter's
|
||||
panic-unsafe release — the one code fix, guard + 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.
|
||||
@@ -165,7 +165,17 @@ remain under `--all-features`, unchanged).
|
||||
|
||||
## Summary
|
||||
|
||||
> To be filled on completion
|
||||
The OQ-12 primitives landed: the `ReverseListener` handles carrying
|
||||
the actually-bound endpoint with the explicit wire-honest `cancel()`
|
||||
(releases the listener count exactly once per confirmed cancel; the
|
||||
stateless cancel forms compose into the same shared paths), the
|
||||
`SshConnection` internals restructured behind the arc so handles
|
||||
cancel against a live stack independently, the public surface
|
||||
unchanged, the abrupt-drop/clean-close boundary row-pinned both ways
|
||||
(drop is never the clean path; `close()` is), and the
|
||||
primitives-not-policy posture stated in the module docs and
|
||||
client.md's landing block. No reconnect/restore policy in the crate.
|
||||
Details in the `Landed` block below.
|
||||
|
||||
Landed (`feat(client)`: w4-lifecycle):
|
||||
|
||||
|
||||
@@ -158,7 +158,18 @@ aborts explicitly now).
|
||||
|
||||
## Summary
|
||||
|
||||
> To be filled on completion
|
||||
The `-R` leg landed complete: the forward-request rows
|
||||
(`tcpip_forward`/`streamlocal_forward` + the cancel forms, the
|
||||
runtime-Unix gate on the listen side, the port-0-only mint fact
|
||||
surfaced), the acceptor registry as the real crate surface
|
||||
(per-type fixed slots, refuse-never-replace, arrival-time reads,
|
||||
decline-not-buffer, late-install), the forwarded hooks routed
|
||||
through it with bounded two-counter bookkeeping, the data leg
|
||||
handed to the assembly acceptor on spawned handoff tasks, and the
|
||||
per-build contract rows through the fake-`sshd`'s real-socket `-R`
|
||||
server half plus the full-circle smoke through the server
|
||||
direction's fabric. Real-`sshd` `-R` stays wave-5 e2e, recorded.
|
||||
Details in the `Landed` block below.
|
||||
|
||||
Landed (`feat(client)`: w4-reverse-listen):
|
||||
|
||||
|
||||
Reference in new issue
Block a user