From a19cdc4832173d949a079d01d45b29b5eded4c37 Mon Sep 17 00:00:00 2001 From: "glm-5.3-flash" Date: Wed, 28 Oct 2026 07:22:24 +0000 Subject: [PATCH] =?UTF-8?q?docs(tasks):=20close=20the=20wave-1=20review=20?= =?UTF-8?q?gate=20=E2=80=94=20verdict=20green?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review task record: the line-by-line ADR/OQ gate reading, the three flagged interpretations confirmed (sibling-dep timing, vault-double shape, policy verdict type), the minor inline fixes, and the green verification summary. Wave 2 decomposition unblocked. --- tasks/implementation/review-wave-1.md | 110 ++++++++++++++++++++++++-- 1 file changed, 104 insertions(+), 6 deletions(-) diff --git a/tasks/implementation/review-wave-1.md b/tasks/implementation/review-wave-1.md index a3a4e9d..ee22f16 100644 --- a/tasks/implementation/review-wave-1.md +++ b/tasks/implementation/review-wave-1.md @@ -1,7 +1,7 @@ --- id: review-wave-1 name: Wave 1 gate — the foundations review -status: pending +status: completed depends_on: [w1-manifest, w1-error-model, w1-key-sources, w1-host-key-policy, w1-fingerprint, w1-config, w1-test-doubles] scope: narrow risk: low @@ -46,13 +46,13 @@ booked (remediation before wave 2 decomposes). ## Acceptance Criteria -- [ ] Every ADR/OQ surface above read and confirmed (or corrected +- [x] Every ADR/OQ surface above read and confirmed (or corrected with the correction landed) -- [ ] The recorded interpretations confirmed or fixed (sibling-dep +- [x] The recorded interpretations confirmed or fixed (sibling-dep timing; the vault-double shape; the policy verdict type) -- [ ] Full verification set green (`cargo test`, `--all-features`, +- [x] Full verification set green (`cargo test`, `--all-features`, clippy, fmt) -- [ ] Verdict recorded; wave 2 decomposition unblocked or findings +- [x] Verdict recorded; wave 2 decomposition unblocked or findings booked ## References @@ -64,6 +64,104 @@ booked (remediation before wave 2 decomposes). > To be filled by implementation agent +### Gate reading (line by line, 2026-10-28) + +- **ADR-002** ✓ — `Cargo.toml` carries `russh = { version = "0.65", + default-features = false, features = ["aws-lc-rs"] }`; edition 2024, + `rust-version = "1.89"`; `cargo tree -e normal` confirms **no + `rsa`, no `flate2`, no `ring`**, and the recorded + `ssh-key =0.7.0-rc.11` RC exception riding the russh keys path is + present exactly as ADR-002 records it. +- **ADR-003** ✓ — `session`/`git`/`tunnel`/`sftp` declared, flat, + `default = []`, nothing implied; the per-feature sibling deps + (`alktty`/`alktunnels`/`alkgit`) are **not** in the manifest — the + recorded interpretation (they land with their first consuming wave) + confirmed as recorded. +- **ADR-004** ✓ — `src/fingerprint.rs` is the single fingerprint path; + both parity suites (`tests/fingerprint_parity.rs`, + `tests/doubles.rs` fixture rows) are green against the + `ssh-keygen -lf`-recorded manifest; `Limits::max_auth_attempts` + defaults to 10 with the decision-6 rationale in its doc. +- **ADR-005** ✓ — the custody chain is in code: one + `vault.decrypt` → `Zeroizing` wrap at the adapter seam → + one-time `decode_secret_key`; the vault's taxonomy reshaped to the + five distinct variants (no re-export, no collapse); `EncryptedData` + opaque in crate code (tests mutate fields only to *tamper*); host + keys decrypt-at-construction via `VaultHostKeySource` (empty set → + `Absent`, locked → no key material); rotation-preservation tested; + no plaintext persistence anywhere. The known-gap (vault's plain + `String` return) is documented as the ADR-005 decision-3 contract, + not silently absorbed. +- **OQ-05** ✓ — `HostKeyPolicy` seam with `RejectAll` fail-closed + default; **no accept-all built-in** (the permissive posture exists + only as the double's explicit `with_default(true)` choice in tests — + correct posture); the crate is policy-blind. +- **OQ-08/09** ✓ — `shell_argv`/`exec_wrapper_argv` are `Option` with + unset→decline semantics in the type docs; no construction-time + validation, no config-error type (the recorded interpretation + confirmed — none was needed); `session_cwd`/`session_env` are + assembly-supplied, never wire-derived; `channel_client` absent → + tunnel rows decline even under the feature. +- **Error model** ✓ — five closed variants, no catch-all; the + dispositions-not-errors contract (auth declines, wire declines, + config validation) is written into `error.rs`'s module docs. +- **Docs honesty** ✓ — every module's docs state what is deliberately + not here yet (russh `Config` construction and the dispatch are + wave-2; dial wiring is wave-4; the grammar is wave-3). + +### Interpretations (flagged for this gate) — all confirmed + +1. Sibling-dep timing — confirmed (see ADR-003 above). +2. Vault-double shape — confirmed: `TestVault` wraps alkvault 0.1's + own in-memory construction (`VaultServiceHandle::new()` + + `unlock_new`) rather than re-implementing behind the source trait, + so the real error taxonomy and GCM behavior ride every row (the + decision is recorded in `tests/common/mod.rs`'s module docs). +3. Policy verdict type — confirmed: accept/reject `bool`, pinned to + the russh 0.65 `check_server_key → Result` hook shape, + with the `Err` channel held for wire/IO failures (recorded in + `host_key.rs`'s module docs). + +### Minor issues found and fixed inline (in this session, same commit set) + +Small doc/polish findings; each is a one-to-few-line fix: + +- `cargo doc --no-deps` emitted 2 rustdoc warnings (ambiguous + `[`async_trait`]` intra-doc links — crate and attribute-macro share + the name) in `key_source.rs` and `host_key.rs` → resolved as plain + code spans. The crates' verification set does not run rustdoc, so + these had gone unnoticed — worth remembering at wave 5's + "public-API rustdoc warning-free" gate. +- `key_source.rs` `HostKeySource` docs: stray `)` typo ("on host + *pin*) trust"). +- `lib.rs` crate docs: stale line ("the dependency surface and + feature graph land with implementation start") — now landed; + rewritten to state the landed/future split honestly. +- Two redundant test lines removed (a discarded pre-probe + `try_sign` in `key_source.rs`'s usability helper; a redundant + `None` re-assignment in `config.rs`'s independent-flip row). + +Nothing found rose to the level of a booked finding — no shape error +in the config types, the error taxonomy, or the key-source seam. + +### Verification (all green) + +`cargo test` 47/47 · `cargo test --all-features` 47/47 · +`cargo clippy --all-targets -- -D warnings` ✓ · +`cargo clippy --all-features --all-targets -- -D warnings` ✓ · +`cargo fmt --check` ✓ · `cargo doc --no-deps` 0 warnings. + ## Summary -> To be filled on completion \ No newline at end of file +> To be filled on completion + +**Verdict: green.** The wave-1 foundations read clean against +ADR-002/003/004/005 and the OQ-05/08/09 resolutions, line by line; +all three recorded interpretations (sibling-dep timing, vault-double +shape, policy verdict type) were confirmed as recorded. Only minor +doc/polish findings surfaced — fixed inline (rustdoc link +disambiguation, a doc typo, a stale crate-doc line, two redundant +test lines), recorded above. No shape errors in the one-way-door +surfaces. Wave 2 decomposition is unblocked; it should absorb the +rustdoc-hygiene lesson (run `cargo doc --no-deps` on gates) — no fix +tasks needed. \ No newline at end of file