glm-5.3-flash
f4b5f896b4
Renames the written-usage rule into practice across the contract surface: docs/ (architecture, research, plans, sdd), tasks/ (all 43 task files, taskgraph validate passes), AGENTS.md, README.md, CHANGELOG.md, and the .opencode/agents/ prompts. Every replacement is a sense-preserving rewrite (colons, semicolons, commas, parentheses, sentence splits, varied), never a mechanical swap; no spaced hyphens were substituted. Enum fields, frontmatter, tables, wire data, and cited identifiers survive verbatim; en dashes inside numeric ranges keep their meaning. Verification: taskgraph validate (43 tasks), repo grep clean (single exempt hit: the verification command's own search pattern in AGENTS.md), cargo test, clippy --all-targets -D warnings, fmt --check, doc all green.
280 lines
15 KiB
Markdown
280 lines
15 KiB
Markdown
---
|
||
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. |