From 97a10353dac46aa6474fd02cde4e53d491820026 Mon Sep 17 00:00:00 2001 From: Jon Chery Date: Tue, 4 Aug 2026 00:48:30 +0000 Subject: [PATCH] =?UTF-8?q?docs(P00):=20grill=20v0.8=20plan=20=E2=80=94=20?= =?UTF-8?q?PROCEED-WITH-CONDITION=20(4=20binding=20fixes=20applied)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GRILL_v0.8.md (30KB): 9-axis adversarial review, overall verdict PROCEED-WITH-CONDITION (confidence 0.78). 7 PROCEED + 4 PROCEED-WITH-CONDITION + 0 REPLAN findings. 4 binding plan changes applied to PLAN_v0.8.md: #1 T02.6 relabeled as v0.6 ship-defect bugfix (not v0.8 feature); P04 audit must record ship-defect closure #2 T02.9 doctor proxmox parity — P02 not complete until both bootstrap + doctor callbacks use capture-fix wrapper #3 T01.6 cli coverage escape valve — ship at 65% if 70% not reached after Wave 2 (RESEARCH §1.4 flags 55-65% realistic); do not block P02/P03 on the last 5% #4 T03.1 verify-reqs regex substring-tolerant (matches v0.2 header variant) + reverse-direction assertion (REQUIREMENTS Complete ↔ ROADMAP COMPLETE); scope note: doc-vs-doc drift only T02.10 case 7 added (known_hosts pre-populated v0.6→v0.8 migration path). No escalations; all axes resolved at confidence ≥ 0.60. ---ci--- project: orca phase: 0 milestone: v0.8 status: grill ---/ci--- --- .ciagent/CHECKPOINT.json | 4 +- .ciagent/GRILL_v0.8.md | 592 +++++++++++++++++++++++++++++++++++++++ .ciagent/PLAN_v0.8.md | 10 +- 3 files changed, 599 insertions(+), 7 deletions(-) create mode 100644 .ciagent/GRILL_v0.8.md diff --git a/.ciagent/CHECKPOINT.json b/.ciagent/CHECKPOINT.json index f0c9de6..86221a1 100644 --- a/.ciagent/CHECKPOINT.json +++ b/.ciagent/CHECKPOINT.json @@ -1,11 +1,11 @@ { "phase": 0, - "stage": "plan", + "stage": "grill", "milestone": "v0.8", "milestone_slug": "coverage-trust-hardening", "phase_role": "pre_execution", "attempts": 0, - "updated_at": "2026-08-04T00:46:00Z", + "updated_at": "2026-08-04T00:48:00Z", "milestone_complete": false, "next_milestone": null } \ No newline at end of file diff --git a/.ciagent/GRILL_v0.8.md b/.ciagent/GRILL_v0.8.md new file mode 100644 index 0000000..93d0eae --- /dev/null +++ b/.ciagent/GRILL_v0.8.md @@ -0,0 +1,592 @@ +# Grill Report: Orca v0.8 — Coverage & Trust Hardening + +**Date:** 2026-08-04 +**Reviewer:** ci-griller (red-team, adversarial) +**Plan under review:** `.ciagent/PLAN_v0.8.md` (commit 4780e4d) +**Branch:** `phase/00-specify` (milestone `milestone/v0.8-coverage-trust-hardening`) +**Mode:** Full autonomy + +--- + +## Methodology + +Every material claim in `PLAN_v0.8.md` and `RESEARCH_v0.8.md` was cross-checked +against the actual codebase (verified coverage baselines via `go test -cover`, +read `internal/proxmox/bootstrap.go:75-234`, `internal/security/ca.go`, +`internal/doctor/doctor.go`, `.ciagent/ROADMAP.md`, `.ciagent/REQUIREMENTS.md`, +PERSONAS, ARCHITECTURE) AND the `golang.org/x/crypto` v0.54.0 source for +`knownhosts.New` / `checkAddr` behavior. The TOFU-capture claim was not taken +on faith — the upstream `checkAddr` (knownhosts.go:370-385) was read directly. + +Findings are scored on the 9 axes. Binding verdicts are **PROCEED**, +**PROCEED-WITH-CONDITION** (plan proceeds but must incorporate a named change), +or **REPLAN** (axis has a fatal flaw; revise before execution). + +--- + +## Summary Verdict + +| Verdict | Count | +|---------|-------| +| PROCEED | 7 | +| PROCEED-WITH-CONDITION | 4 | +| REPLAN | 0 | + +**Overall verdict: PROCEED-WITH-CONDITION** + +The v0.8 plan is fundamentally sound: scope is right-sized, the no-new-deps +promise holds (verified `ssh.FingerprintSHA256` + `knownhosts.Line` are in the +existing `golang.org/x/crypto` v0.54.0 dep), the tiered coverage floor (D-047) +is realistic per-package with the named seams, and the persona territory +collision on `internal/cli/node.go` is explicitly adjudicated in PERSONAS.md +(backend owns implementation, lead owns `_test.go`). The 4 conditions below are +**targeted correctness fixes**, not scope expansions: + +1. **P02 must add a regression test asserting first-connect Proxmox join + succeeds end-to-end** (the latent TOFU bug means v0.6's first-connect has + been broken since ship; the fix in T02.6 is correct but must be proven by a + test that would have failed pre-fix). +2. **P03's verify-reqs regex must match `**COMPLETE**` as a *substring* within + the bold span** (v0.2's header `**COMPLETE (merged to main via v0.3)**` is + not matched by the current `\*\*COMPLETE\*\*` literal — a silent blind spot). +3. **P03 must add a second assertion: every REQUIREMENTS row marked `Complete` + must reference a milestone ROADMAP marks COMPLETE** (the reverse direction). + The v0.7 `cert_repo_test.go` omission (REQ-053 marked Complete but the test + file does not exist) proves forward-direction-only checks miss the most + dangerous drift class: *claimed-Complete-but-actually-incomplete*. +4. **P02 T02.6's TOFU fix must be reviewed against `doctor proxmox`'s callback + (T02.9) as a paired change, not a follow-on** — they share the exact + `knownhosts.New` defect; fixing one and not the other in the same phase + creates an inconsistent trust surface. + +With these 4 conditions applied, this plan is ready to execute. No REPLAN. + +--- + +## Per-Axis Findings + +### Axis 1 — Business Case + +#### A1-F1 — Is v0.8 the right next milestone, or polish-for-polish's-sake? + +**Evidence:** +- v0.7 P03 (REQ-055) shipped a ≥50% coverage floor; v0.8 re-baselines six + packages still under 50% (engine 8.3%, proxmox 5.1%, cli 27.6%, transport + 26.3%, store 47.2%, jobspec 47.6%) — **verified identical via `go test + -cover`**. +- RESEARCH §2.1 surfaces a **latent v0.6 defect**: `knownhosts.New` returns + `KeyError{Want:[]}` on first connect and does NOT auto-write. Verified + directly in `golang.org/x/crypto@v0.54.0/ssh/knownhosts/knownhosts.go:370-385` + (`checkAddr` returns `&KeyError{}` with empty `Want` when no line matches). + `bootstrap.go:140-142` treats this as a dial failure. **This means + first-connect `orca node join --type proxmox` has been broken since v0.6 + shipped** (the v0.6 RESEARCH §A.5 claim that `knownhosts.New` "handles both + capture and verify" was wrong). +- `bootstrap.go:123` comment is literally false: "on first connect it captures + the host key" — it does not. + +**Confidence:** 0.90 that v0.8 is the right next milestone. +**Verdict:** **PROCEED**. v0.8 is not polish-for-polish: it closes a real +security defect (TOFU broken since v0.6), populates a `Result` field that D-045 +*assumed* was already populated (it isn't — `bootstrap.go:195-198`), and lifts +coverage off floors that v0.7 explicitly under-shot. The diminishing-returns +risk is real for the 3 zero-test toe-holds (audit/certpaths/cmd-orca), but +D-047 tiered them to 50% precisely to avoid the rathole — that call is sound. + +--- + +### Axis 2 — Scope and Requirements + +#### A2-F1 — Is the TOFU bugfix correctly scoped into P02, or should it be a hotfix on main? + +**Evidence:** +- The TOFU capture bug (RESEARCH §2.1, PLAN T02.6) is a v0.6 latent defect, + not a v0.8 feature. First-connect Proxmox join is broken **today on main**. +- PLAN bundles the fix into P02 (trust hardening phase) alongside REQ-058 + (`--host-key-fingerprint`) and REQ-059 (`key-reset`). +- ROADMAP tags run on the v0.7.x patch line: `v0.7.0` (P0) … `v0.7.4` (P04). + P02 ships as `v0.7.2` — i.e., the fix lands on a milestone branch, not main, + and only reaches main at P04 merge (`v0.7.4`). + +**Confidence:** 0.62 that bundling into P02 is the right call (low confidence — +this is a judgment call with real downside). +**Verdict:** **PROCEED-WITH-CONDITION.** The fix is correctly designed (T02.6's +`KeyError{Want:[]}` capture-and-persist is the right shape), but the plan must +either (a) document explicitly *why* this isn't hotfixed on main (e.g., "no +operator has hit first-connect yet because all deployments pre-populate +`known_hosts` manually — confirmed by the v0.6 ship audit"), OR (b) flag the +bug in the P04 audit as a v0.6 ship-defect with a post-mortem note. **The plan +currently treats T02.6 as a feature task; it is a bugfix for shipped code and +must be labeled as such** so the P04 audit can distinguish "new hardening" from +"closing a v0.6 gap." Blast radius if T02.6's fix is wrong: every existing +Proxmox node's `known_hosts` could be re-pinned on next join — moderate, but +mitigated by T02.10 case 3/4/5 integration tests. + +**Condition:** Add a note to T02.6 in PLAN marking it as a **v0.6 ship-defect +bugfix** (not a v0.8 feature), and ensure P04 audit (T04.2) records it as such. + +#### A2-F2 — Are the 3 zero-test packages worth a 50% toe-hold, or scope creep? + +**Evidence:** +- `cmd/orca` is 15 LOC of glue (`main()` → `cli.Execute()`). 50% coverage = ~7 + lines. RESEARCH §1.1, §5 pitfall #6 explicitly flags the effort:coverage + ratio as poor. +- `internal/certpaths` is 64 LOC of pure path-join functions. 50% is trivial. +- `internal/audit` is 125 LOC, 4 exported funcs. 50% is trivial. +- D-047 explicitly tiered these to 50% to avoid a coverage rathole; v0.9 can + raise the floor. + +**Confidence:** 0.85. +**Verdict:** **PROCEED.** The tiered floor is the right call. The +`cmd/orca` toe-hold is low-value but low-cost (one `run() int` refactor + one +smoke test), and dropping it would leave a `covdata` tooling error in CI output +that looks like a broken build to a casual reader. Keeping it at 50% is +defensible. + +#### A2-F3 — Scope size: 4 REQs, 37 tasks — too lean, too fat, or right? + +**Evidence:** +- 37 tasks, 36 must-haves, 4 phases each shipping a patch. Comparable to v0.7 + (5 phases, similar task density). +- P01 is the heaviest (12 tasks, 9 packages) — the risk concentration is here. + +**Confidence:** 0.80. +**Verdict:** **PROCEED.** Right-sized for an NFR milestone. P01 density is the +watch item (see Axis 5). + +--- + +### Axis 3 — Architecture and Technical Feasibility + +#### A3-F1 — Do the proxmox `sessionRunner` and engine `peerDispatcher` seams leak test concerns into production? + +**Evidence:** +- T01.1 `sessionRunner` (`internal/proxmox/bootstrap.go`): 1 interface, + ~10 LOC, `CombinedOutput(cmd) ([]byte, error)`. Default impl wraps + `*ssh.Client.NewSession().CombinedOutput(...)`. Backward compatible — + existing callers unchanged. This is the **same pattern as the existing + `sshDialer` seam** (`bootstrap.go:201-213`), which shipped in v0.6 without + concern. The seam is a standard testability extraction, not a test concern + leak. +- T01.2 `peerDispatcher` (`internal/engine/dispatcher.go`): **conditional** — + only added if T01.4 cannot hit 70% via `httptest.NewTLSServer` alone. Plan + explicitly prefers `httptest.NewTLSServer` (RESEARCH §1.3 gap #2, §5 pitfall + #8). This is the right ordering: try the stdlib test fixture first, add the + seam only if needed. + +**Confidence:** 0.88. +**Verdict:** **PROCEED.** Both seams are backward-compatible interface +extractions matching an existing pattern (`sshDialer`). No test-concern leak. +The conditional-gate on T01.2 is correctly conservative. + +#### A3-F2 — Does P02's trust work stay within the existing security boundary? + +**Evidence:** +- P02 touches `internal/proxmox/bootstrap.go` (pinned callback, TOFU fix), + `internal/cli/node.go` (flag + subcommand), `internal/security/sshkey.go` + (fingerprint helper), `internal/doctor/doctor.go` (T02.9 TOFU fix). All + within the existing SSH trust surface established in v0.6. +- No new crypto, no new CA, no new X.509. `ssh.FingerprintSHA256` is in the + existing `golang.org/x/crypto` v0.54.0 dep (verified: not a new direct dep). +- PERSONAS correctly keeps `security-engineer` deactivated — the work is SSH + dialer + known_hosts file manipulation, not new security architecture. + +**Confidence:** 0.90. +**Verdict:** **PROCEED.** Boundary is respected. + +#### A3-F3 — T02.9 (doctor proxmox TOFU fix) is a paired change with T02.6, not a follow-on + +**Evidence:** +- `internal/doctor/doctor.go:412` uses the **exact same** `knownhosts.New(...)` + callback pattern as `bootstrap.go:125`. Both share the latent defect. +- T02.9 is listed as a separate task ("Apply the TOFU capture-fix to `doctor + proxmox` probe") but is in the same Wave 2 as T02.6. If T02.6 lands and T02.9 + doesn't (e.g., a mid-phase blocker), the trust surface is **inconsistent**: + join captures, doctor fails. + +**Confidence:** 0.75. +**Verdict:** **PROCEED-WITH-CONDITION.** T02.6 and T02.9 must be reviewed as a +paired change in P02 verification — the phase is not done until BOTH callbacks +use the capture-fix wrapper. Add to P02 Verification: "doctor proxmox +first-connect → captures + succeeds (mirrors T02.10 case 3 for bootstrap)." + +**Condition:** Add a P02 verification line asserting doctor proxmox +first-connect parity with bootstrap. + +--- + +### Axis 4 — People, Skills, and Organization + +#### A4-F1 — Territory collision on `internal/cli/node.go` + +**Evidence:** +- PERSONAS.md line 62: lead-developer territory = `internal/cli/**`. +- PERSONAS.md line 70: backend-engineer territory = `internal/cli/node.go`. +- PERSONAS.md line 107 explicitly adjudicates: "backend owns the command + implementation; lead owns the test files (`node_test.go`)." +- Territory mode is `warn` (not `block`) — collisions log but don't fail. + +**Confidence:** 0.82. +**Verdict:** **PROCEED.** The collision is **explicitly adjudicated** in +PERSONAS.md with a clean boundary (impl vs test files). This is the right +answer. The `warn` mode means a backend commit touching `node_test.go` (or a +lead commit touching `node.go` impl) would log — acceptable for a 3-persona +team. No replan. + +#### A4-F2 — Key-person dependency: is the 3-persona roster sufficient? + +**Evidence:** +- 3 active personas, all retained from v0.7. No phase-specific personas. +- backend-engineer owns 60%+ of P02 (the security-critical phase). If + backend-engineer is unavailable, P02 stalls entirely. + +**Confidence:** 0.70. +**Verdict:** **PROCEED.** Key-person risk is real but inherent to a 3-persona +NFR milestone. The work is not novel (refining existing surface), so the bus +factor is acceptable for hardening. Flagged, not blocking. + +--- + +### Axis 5 — Timeline and Estimates + +#### A5-F1 — Is the 70% coverage target for 6 packages in one phase (P01) realistic? + +**Evidence:** +- RESEARCH §1.1 + §1.4 per-package achievability assessments: + - engine → 70% REALISTIC (with LocalExecutor stubs + `openTestDB`). + - proxmox → 70% REALISTIC **but requires the `sessionRunner` seam (T01.1)** — + without it, only 50-55% (validation paths + sudoersContent asserts, already + done). + - cli → 70% AMBITIOUS (17 files, ~2000 LOC); RESEARCH says "55-65% is more + realistic for one phase" even with `daemon.go` excluded. + - transport → 70% REALISTIC (`httptest.NewTLSServer` is standard). + - store → 70% REALISTIC (cert_repo_test.go gap is the main lift). + - jobspec → 70% REALISTIC (easiest of the six). +- **`internal/cli` is the swing package.** RESEARCH explicitly says 55-65% is + the realistic single-phase outcome, not 70%. The plan sets the floor at 70% + "excluding daemon.go" — but even excluding daemon.go, RESEARCH's own evidence + says 70% is a stretch. + +**Confidence:** 0.65 (split: 5 of 6 packages at 0.85, cli at 0.45). +**Verdict:** **PROCEED-WITH-CONDITION.** The plan must add an explicit fallback +for `internal/cli`: if T01.6 hits ≥65% (excluding daemon.go) but not 70% after +a reasonable effort, the phase ships at 65% with a documented note + a v0.9 +follow-up to lift to 70%. **Hard-requiring 70% on cli risks a coverage rathole +that delays the entire milestone** (P02/P03 are gated on P01 ship). The other 5 +packages at 70% is realistic. + +**Condition:** Add to T01.6 acceptance criterion: "If ≥65% (excluding +daemon.go) is achieved but 70% is not after Wave 2 effort, document the gap in +the task comment + record a v0.9 follow-up; ship at 65%. Do NOT block P02/P03 +on the last 5% of cli coverage." (This mirrors RESEARCH §1.4's own flag, which +the plan currently does not carry forward as an escape valve.) + +--- + +### Axis 6 — Budget and Financial Realism + +#### A6-F1 — Zero new deps: is that realistic given P02's needs? + +**Evidence:** +- `ssh.FingerprintSHA256`: verified in `golang.org/x/crypto/ssh` (direct dep + since v0.6 D-030). +- `knownhosts.Line` / `Normalize` / `KeyError`: same `golang.org/x/crypto` + module (already imported in `bootstrap.go:32` and `doctor.go:29`). +- `verify-reqs`: stdlib only (`regexp`, `os`, `fmt`). +- `go.mod` unchanged by v0.8 (PLAN line 62). + +**Confidence:** 0.95. +**Verdict:** **PROCEED.** Zero-new-deps is verified and realistic. + +--- + +### Axis 7 — Risks, Assumptions, and Dependencies + +#### A7-F1 — The 10 pitfalls: are mitigations real or hand-waves? + +**Evidence (spot-check of the 4 most material pitfalls):** +- **Pitfall #1 (TOFU broken):** Mitigation T02.6 is **concrete and correct** — + wrap `knownhosts.New`, capture on `KeyError{Want:[]}` via `knownhosts.Line` + + `security.WriteAtomic`, return nil. Verified against x/crypto v0.54.0 + `checkAddr` semantics. **Real mitigation.** +- **Pitfall #2 (Result.HostKeyFingerprint never populated):** T02.7 adds + `ssh.FingerprintSHA256(hostKey)`. 1-line once host key is available. **Real.** +- **Pitfall #3 (no sessionRunner seam):** T01.1 adds it, ~10 LOC. **Real.** +- **Pitfall #10 (writeAtomic unexported):** T02.2 exports it. Verified + `ca.go:305` — `func writeAtomic(...)` is indeed unexported. **Real.** + +**Confidence:** 0.88. +**Verdict:** **PROCEED.** Mitigations are concrete, not hand-waves. + +#### A7-F2 — TOFI bugfix blast radius if P02's fix is wrong + +**Evidence:** +- T02.6 changes the `HostKeyCallback` for every `orca node join --type proxmox` + + every `doctor proxmox` probe. If the capture-and-persist logic is wrong, + every existing Proxmox node's `known_hosts` could be corrupted (e.g., + duplicate entries, wrong-format lines, partial writes on crash). +- Mitigations: T02.10 integration tests (cases 3/4/5 cover first-connect, + second-connect, mismatch); AD-029 atomic rewrite via `security.WriteAtomic`. +- **Gap:** no test for "known_hosts already has an entry, join re-connects" — + i.e., the idempotent re-run path after the fix. T02.10 case 4 covers + second-connect-match, but not "known_hosts was written by the OLD (broken) + code path and is now being read by the NEW code path." + +**Confidence:** 0.70. +**Verdict:** **PROCEED-WITH-CONDITION.** T02.10 must add a case for +"known_hosts pre-populated in the expected format (e.g., from a manual +`ssh-keyscan` or a prior v0.6 deployment that somehow succeeded) → +second-connect matches + succeeds." This covers the migration path from +v0.6's (broken) state to v0.8's fixed state. + +**Condition:** Add T02.10 case 7: "known_hosts pre-populated with a valid +OpenSSH line for the host → connect matches + succeeds (covers v0.6→v0.8 +migration)." + +--- + +### Axis 8 — Governance, Decision-Making, and Communication + +#### A8-F1 — Does `make verify-reqs` actually prevent drift, or is it cosmetic? + +**Evidence:** +- T03.1 regex (PLAN line 216): + - ROADMAP milestone-complete: `^##\s*Milestone\s+v0\.\d+:.*—\s*\*\*COMPLETE\*\*` + - REQUIREMENTS row: `^\|\s*(REQ-\d+)\s*\|.*?\|\s*\*\*(Complete|Pending)\*\*\s*\|` +- **ROADMAP v0.2 header (line 23):** `## Milestone v0.2: Networking, + Observability, Security Hardening — **COMPLETE (merged to main via v0.3)**` +- The regex `\*\*COMPLETE\*\*` requires the literal `**COMPLETE**` with closing + `**` immediately after `COMPLETE`. v0.2's header has `**COMPLETE (merged to + main via v0.3)**` — the `**` closes after the parenthetical, NOT after + `COMPLETE`. **The regex does NOT match v0.2 as complete.** +- **Consequence:** all v0.2 REQs (REQ-011, 014, 023, 025-040) are **silently + exempted** from the check. A stale v0.2 REQ-035 row (marked Pending) would + NOT fail the gate. +- **ROADMAP v0.6 has TWO headers** (line 92 without COMPLETE, line 94 with) — + the regex matches line 94, but the duplicate is a markdown smell that could + confuse the milestone→REQ mapping if the parser takes the first match. + +**Confidence:** 0.92 (high — the regex mismatch is verifiable). +**Verdict:** **PROCEED-WITH-CONDITION.** The regex must match `**COMPLETE**` +as a *substring within the bold span*, not as a literal `**COMPLETE**` token. +Change to `—\s*\*\*[^*]*COMPLETE[^*]*\*\*` (matches `**COMPLETE**`, +`**COMPLETE (merged to main via v0.3)**`, and any future variant). Add a +golden-file test case (T03.2) with the v0.2-style parenthetical header to +prevent regression. + +**Condition:** T03.1 regex changed to substring-match COMPLETE within the bold +span; T03.2 adds a golden fixture with `**COMPLETE (merged to main via v0.3)**`. + +#### A8-F2 — Is the single-direction check (ROADMAP→REQUIREMENTS) enough? + +**Evidence:** +- PLAN line 35-37 explicitly scopes out the reverse direction: "forward + direction (ROADMAP-shipped → REQUIREMENTS Complete) is the priority per the + v0.7 drift that motivated REQ-060." +- **But the v0.7 drift had TWO symptoms:** + 1. ROADMAP said COMPLETE, REQUIREMENTS said Pending (forward drift — caught + by the current check). + 2. **REQ-053 was marked Complete in REQUIREMENTS, but + `internal/store/cert_repo_test.go` was never written** — verified: only + `cert_repo.go` exists in `internal/store/`. The "Complete" status was + false. **No markdown-based check can catch this** (it's a code-vs-doc + drift, not a doc-vs-doc drift). +- The reverse-direction check (REQUIREMENTS Complete ↔ ROADMAP COMPLETE) would + catch a different class: a REQ marked Complete in REQUIREMENTS for a + milestone ROADMAP does NOT mark COMPLETE (e.g., premature marking). This is + a cheaper class of drift but still real. + +**Confidence:** 0.78. +**Verdict:** **PROCEED-WITH-CONDITION.** Add the reverse-direction assertion +to T03.1 (it's ~10 LOC on top of the existing parser — same maps, just diff +both ways). Document explicitly that **no markdown check can catch the +code-vs-doc drift** (REQ-053 case) — that requires a code-level audit +(`ciagent-audit` in P04). The plan should note this as a known limitation of +REQ-060, not pretend the gate is complete. + +**Condition:** T03.1 adds reverse-direction assertion; PLAN adds a note that +REQ-060 catches doc-vs-doc drift only, not code-vs-doc (the REQ-053 +cert_repo_test.go case). + +#### A8-F3 — Is there a "stop the project" trigger? + +**Evidence:** P04 (T04.1-T04.9) is the final review + ship. No explicit +"stop" trigger if P01 coverage stalls or P02 TOFU fix proves unfixable. + +**Confidence:** 0.60. +**Verdict:** **PROCEED.** The 4-phase structure with per-phase tags means a +stall is visible (phase tag doesn't ship). Acceptable for an NFR milestone. + +--- + +### Axis 9 — Change, Adoption, and Operational Readiness + +#### A9-F1 — Who benefits from v0.8? Is there operator pull for `--host-key-fingerprint`? + +**Evidence:** +- `--host-key-fingerprint` (REQ-058) is operator-facing: pre-pinning a + Proxmox host's SSH key before first join. This is the standard + high-security-deployment pattern (the v0.6 D-035 caveat explicitly promised + it as a "future enhancement"). +- `orca node key-reset` (REQ-059) is operator-facing: the `ssh-keygen -R` + equivalent for orca's known_hosts. +- The TOFU bugfix (T02.6) benefits **every operator who has tried + first-connect Proxmox join since v0.6** — i.e., it fixes a feature that was + advertised as working but wasn't. +- Coverage uplift (REQ-057) is developer-facing (no operator pull). +- verify-reqs (REQ-060) is internal-governance (no operator pull). + +**Confidence:** 0.82. +**Verdict:** **PROCEED.** The trust features have real operator pull +(pre-pinning is a documented security best practice; the v0.6 caveat promised +it). The coverage + hygiene work is internal-debt paydown — justified by the +v0.7 under-shot, not by operator demand. The mix is appropriate for an NFR +milestone. + +#### A9-F2 — Rollback plan if P02's trust changes go wrong + +**Evidence:** +- P02 changes `HostKeyCallback` for all Proxmox joins + doctor probes. If the + capture-fix corrupts `known_hosts`, the rollback is: revert the phase commit + + manually restore `known_hosts` from backup. +- No data migration in P02 (known_hosts is a flat file; atomic rewrite via + `WriteAtomic` preserves crash safety). +- `key-reset` (T02.8) is local-only (D-046) — no remote side effects to + reverse. + +**Confidence:** 0.80. +**Verdict:** **PROCEED.** Rollback is straightforward (revert + file restore). +The atomic-rewrite requirement (AD-029) is the right mitigation. + +--- + +## Binding Verdicts Table + +| # | Axis | Finding | Verdict | Condition | Confidence | +|---|------|---------|---------|-----------|------------| +| A2-F1 | Scope | TOFU bugfix is a v0.6 ship-defect bundled into P02 as a feature task | PROCEED-WITH-CONDITION | Label T02.6 as a v0.6 bugfix in PLAN; P04 audit records it as a ship-defect closure | 0.62 | +| A2-F2 | Scope | 3 zero-test packages at 50% toe-hold | PROCEED | — | 0.85 | +| A2-F3 | Scope | 37 tasks / 4 phases size | PROCEED | — | 0.80 | +| A1-F1 | Business | v0.8 is the right next milestone (not polish) | PROCEED | — | 0.90 | +| A3-F1 | Architecture | sessionRunner + peerDispatcher seams do not leak test concerns | PROCEED | — | 0.88 | +| A3-F2 | Architecture | P02 stays within existing security boundary | PROCEED | — | 0.90 | +| A3-F3 | Architecture | T02.6 + T02.9 are paired changes (bootstrap + doctor share the defect) | PROCEED-WITH-CONDITION | Add P02 verification line for doctor proxmox first-connect parity with bootstrap | 0.75 | +| A4-F1 | People | internal/cli/node.go territory collision adjudicated | PROCEED | — | 0.82 | +| A4-F2 | People | Key-person risk on backend-engineer in P02 | PROCEED | — | 0.70 | +| A5-F1 | Timeline | 70% cli coverage in one phase is a stretch (RESEARCH says 55-65%) | PROCEED-WITH-CONDITION | Add escape valve: ship cli at 65% if 70% not reached after Wave 2; do not block P02/P03 | 0.65 | +| A6-F1 | Budget | Zero new deps verified | PROCEED | — | 0.95 | +| A7-F1 | Risks | 10 pitfalls mitigations are concrete | PROCEED | — | 0.88 | +| A7-F2 | Risks | TOFU fix blast radius — no migration-path test | PROCEED-WITH-CONDITION | Add T02.10 case 7: known_hosts pre-populated → second-connect matches (v0.6→v0.8 migration) | 0.70 | +| A8-F1 | Governance | verify-reqs regex does not match v0.2's `**COMPLETE (merged...)**` header | PROCEED-WITH-CONDITION | Change regex to substring-match COMPLETE within bold span; add golden fixture | 0.92 | +| A8-F2 | Governance | Single-direction check misses reverse drift + code-vs-doc drift (REQ-053 case) | PROCEED-WITH-CONDITION | Add reverse-direction assertion; document that code-vs-doc drift is out of scope for REQ-060 | 0.78 | +| A8-F3 | Governance | No explicit "stop" trigger | PROCEED | — | 0.60 | +| A9-F1 | Adoption | Operator pull exists for trust features; coverage/hygiene is internal debt | PROCEED | — | 0.82 | +| A9-F2 | Adoption | Rollback plan is straightforward (revert + file restore) | PROCEED | — | 0.80 | + +--- + +## Required Plan Changes (4 conditions) + +1. **T02.6 labeling (A2-F1):** Add a note to T02.6 in `PLAN_v0.8.md` marking + it as a **v0.6 ship-defect bugfix** (first-connect Proxmox join has been + broken since v0.6 shipped due to `knownhosts.New` returning + `KeyError{Want:[]}` with no capture-and-persist). P04 audit (T04.2) must + record it as a ship-defect closure, not a v0.8 feature. + +2. **P02 verification parity for doctor (A3-F3):** Add to Phase 2 Verification: + "`doctor proxmox` first-connect on a node with empty known_hosts → captures + the key + writes known_hosts + probe succeeds (mirrors T02.10 case 3 for + bootstrap). T02.6 and T02.9 are a paired change; the phase is not complete + until both callbacks use the capture-fix wrapper." + +3. **T01.6 cli coverage escape valve (A5-F1):** Add to T01.6 acceptance + criterion: "If ≥65% (excluding `daemon.go`) is achieved but 70% is not after + Wave 2 effort, document the gap in a test-file comment + record a v0.9 + follow-up; ship P01 at 65% for cli. Do NOT block P02/P03 on the last 5% of + cli coverage." (Carries forward RESEARCH §1.4's own flag as an explicit + escape valve.) + +4. **verify-reqs regex + reverse direction (A8-F1 + A8-F2):** + - Change T03.1 ROADMAP-complete regex from + `^##\s*Milestone\s+v0\.\d+:.*—\s*\*\*COMPLETE\*\*` to + `^##\s*Milestone\s+v0\.\d+:.*—\s*\*\*[^*]*COMPLETE[^*]*\*\*` (substring + match within the bold span — handles `**COMPLETE**`, + `**COMPLETE (merged to main via v0.3)**`, and future variants). + - Add T03.2 golden fixture: a ROADMAP with + `**COMPLETE (merged to main via v0.3)**` → assert the milestone is + detected as complete. + - Add reverse-direction assertion to T03.1: every REQUIREMENTS row marked + `**Complete**` must reference a milestone ROADMAP marks COMPLETE (catches + premature-Complete drift). + - Add a PLAN note: "REQ-060 catches doc-vs-doc drift only. Code-vs-doc + drift (e.g., REQ-053 marked Complete but `cert_repo_test.go` missing — + verified missing in v0.7 ship) is NOT caught by this gate; it requires + the P04 `ciagent-audit` code-level review." + +Additionally (lower-priority, from A7-F2): + +5. **T02.10 case 7 (A7-F2):** Add integration test case: "known_hosts + pre-populated with a valid OpenSSH line for the host (simulating a v0.6 + deployment or manual `ssh-keyscan`) → connect matches + succeeds. Covers + the v0.6→v0.8 migration path." + +--- + +## Escalations + +None. All 9 axes resolved at confidence ≥ 0.60. No axis requires escalation to +the operator; the 4 conditions are within the plan-author's authority to apply +before P01 execution begins. + +--- + +## What the Plan Is NOT Doing (and should it?) + +- **Not lifting the 3 zero-test packages to 70%.** Correct per D-047 — deferred + to v0.9. Not a gap. +- **Not adding a `peerDispatcher` seam unless needed.** Correct — conditional + on T01.4's 70% via `httptest.NewTLSServer`. Not a gap. +- **Not pre-populating `known_hosts` from a remote keyscan API.** Correct — + TOFU + manual `--host-key-fingerprint` cover the v0.8 surface. Not a gap. +- **Not catching code-vs-doc drift in verify-reqs.** **Known limitation** — + REQ-060 is a markdown-vs-markdown check. The REQ-053 + `cert_repo_test.go`-missing case proves this class of drift is real. P04 + `ciagent-audit` is the backstop. Documented in condition #4. + +--- + +## Simplest 80%-of-the-value version + +If forced to cut v0.8 to its smallest valuable form: **keep P02 (trust +hardening + TOFU bugfix) and P03 (verify-reqs); drop P01's coverage uplift for +the 3 zero-test packages + cli.** The TOFU bugfix alone (T02.6 + T02.9) fixes a +shipped security defect — that's the highest-value work. The verify-reqs gate +prevents the v0.7 drift from recurring. The coverage uplift on the 6 +under-50% packages is valuable but not urgent; the 3 zero-test toe-holds are +the lowest-value work in the milestone. **The plan as written does not over- +scope** — it includes all of the above because the marginal cost is low — but +if P01 slips, the 3 toe-holds + cli are the first cuts to make. + +--- + +## What Would Have to Be True for v0.8 to Succeed in the Next 90 Days + +1. The `sessionRunner` seam (T01.1) unlocks proxmox 70% — **plausible** (same + pattern as the existing `sshDialer` seam). +2. `httptest.NewTLSServer` suffices for transport 70% without a new seam — + **plausible** (standard Go testing fixture). +3. The TOFU capture-fix (T02.6) is correct — **plausible** (verified against + x/crypto v0.54.0 semantics; integration tests T02.10 cover the cases). +4. `verify-reqs` regex matches all ROADMAP milestone header variants — **NOT + true today** (v0.2 header mismatch — condition #4 fixes this). +5. cli hits 70% in one phase — **NOT confirmed** (RESEARCH says 55-65%; + condition #3 adds the escape valve). + +(4) and (5) are the two conditions that move the plan from "optimistic" to +"sound." Both are addressed by the 4 required changes. + +--- + +**End of grill report.** Apply the 4 conditions to `PLAN_v0.8.md` before P01 +execution. No REPLAN; no escalations. Overall verdict: **PROCEED-WITH- +CONDITION** (confidence 0.78). \ No newline at end of file diff --git a/.ciagent/PLAN_v0.8.md b/.ciagent/PLAN_v0.8.md index 3dfa44b..ffbc6c7 100644 --- a/.ciagent/PLAN_v0.8.md +++ b/.ciagent/PLAN_v0.8.md @@ -103,7 +103,7 @@ other P01 tasks add `_test.go` files only. | T01.3 | backend-engineer | 2 | Y | `internal/transport` tests → ≥ 70% | `internal/transport/mtls_test.go` (NEW), `internal/transport/dispatch_test.go` (NEW), `internal/transport/handshake_log_test.go` (NEW), `internal/transport/retry_test.go` (NEW, extend) | `httptest.NewTLSServer` with a test CA (reuse `security.CAInit`/`GenerateCSR`/`SignCSR` per RESEARCH §1.2) for mTLS handshake paths; `stubDispatcher` (daemon/dispatch_test.go:24) pattern for Dispatch RPC; capture slog via a test `slog.Handler` for handshake_log. `go test -cover ./internal/transport` → ≥ 70% (was 26.3%). | | T01.4 | backend-engineer | 2 | Y | `internal/engine` tests → ≥ 70% | `internal/engine/executor_test.go` (NEW), `internal/engine/dispatcher_test.go` (NEW), `internal/engine/peer_test.go` (NEW), `internal/engine/scheduler_test.go` (extend), `internal/engine/registry_test.go` (NEW, if registry exists) | `Executor.Start`/`Wait` lifecycle (echo/false/ctx-cancel/Env propagation per REQ-021); `Dispatcher.Submit` with stubbed `LocalExecutor` + (if T01.2 added) stubbed `peerDispatcher` OR `httptest.NewTLSServer`; `PeerRegistry` in-memory Add/Remove/All/Get. Reuse `openTestDB` (node_repo_test.go:12). `go test -cover ./internal/engine` → ≥ 70% (was 8.3%). | | T01.5 | backend-engineer | 2 | Y | `internal/proxmox` tests → ≥ 70% | `internal/proxmox/bootstrap_test.go` (extend) | Swap `sshDialer` (existing seam) for a fake returning a mock `*ssh.Client`; swap `sessionRunner` (T01.1 seam) for a fake that returns canned `CombinedOutput` bytes. Assert full bootstrap sequence calls the right shell commands in order; idempotent re-run ("already exists" → no-op); SSH auth failure → wrapped error; no password logged (D-031). `go test -cover ./internal/proxmox` → ≥ 70% (was 5.1%). | -| T01.6 | lead-developer | 2 | Y | `internal/cli` tests → ≥ 70% (excluding daemon.go) | `internal/cli/node_test.go` (NEW), `internal/cli/job_test.go` (NEW), `internal/cli/cert_test.go` (NEW), `internal/cli/doctor_test.go` (NEW), `internal/cli/audit_test.go` (NEW), `internal/cli/status_test.go` (NEW), `internal/cli/version_test.go` (NEW), `internal/cli/node_capacity_test.go` (NEW) | Table-driven `rootCmd.Execute()` against temp `ORCA_HOME` per subcommand (reuse `initTestEnv`/`resetRootFlags`/`discardWriter` per RESEARCH §1.2). Mock the proxmox path via `sshDialer` + `sessionRunner` seams. `daemon.go` is excluded — covered by `internal/daemon/server_test.go`. `go test -cover ./internal/cli` → ≥ 70% of non-daemon files (document the exclusion in a test-file comment). | +| T01.6 | lead-developer | 2 | Y | `internal/cli` tests → ≥ 70% (excluding daemon.go) with GRILL condition #3 escape valve | `internal/cli/node_test.go` (NEW), `internal/cli/job_test.go` (NEW), `internal/cli/cert_test.go` (NEW), `internal/cli/doctor_test.go` (NEW), `internal/cli/audit_test.go` (NEW), `internal/cli/status_test.go` (NEW), `internal/cli/version_test.go` (NEW), `internal/cli/node_capacity_test.go` (NEW) | Table-driven `rootCmd.Execute()` against temp `ORCA_HOME` per subcommand (reuse `initTestEnv`/`resetRootFlags`/`discardWriter` per RESEARCH §1.2). Mock the proxmox path via `sshDialer` + `sessionRunner` seams. `daemon.go` is excluded — covered by `internal/daemon/server_test.go`. `go test -cover ./internal/cli` → ≥ 70% of non-daemon files (document the exclusion in a test-file comment). **GRILL condition #3 escape valve**: if 70% is not reached after Wave 2 effort and ≥ 65% is achieved (RESEARCH §1.4 flags 55-65% as realistic for one phase), ship cli at 65% and do NOT block P02/P03 on the last 5%; record the shortfall + rationale in the P01 verification commit. | | T01.7 | data-engineer | 2 | Y | `internal/store` tests → ≥ 70% (incl. missing `cert_repo_test.go`) | `internal/store/cert_repo_test.go` (NEW — v0.7 P01 leftover, RESEARCH §1.1), `internal/store/node_repo_test.go` (extend), `internal/store/job_task_repo_test.go` (extend), `internal/store/audit_repo_test.go` (extend), `internal/store/capacity_repo_test.go` (extend) | `cert_repo_test.go`: Insert/Get/List/ListByNode/LatestForKind/PruneOlderThan/Delete + N=3 rotation history per REQ-025 + duplicate-serial error. Reuse `openTestDB`/`withFastWatch` (RESEARCH §1.2). `go test -cover ./internal/store` → ≥ 70% (was 47.2%). | | T01.8 | data-engineer | 2 | Y | `internal/jobspec` tests → ≥ 70% | `internal/jobspec/spec_test.go` (extend), `internal/jobspec/testdata/*.hcl` (NEW golden fixtures) | Golden-file HCL fixtures (multi-task, env vars, args) + error-path table (missing job, no tasks, missing command, malformed HCL, empty file, nonexistent file for `ParseFile`). `go test -cover ./internal/jobspec` → ≥ 70% (was 47.6%). | | T01.9 | data-engineer | 2 | Y | `internal/audit` first tests → ≥ 50% toe-hold | `internal/audit/audit_test.go` (NEW) | Construct `Audit` with real `engine.Audit` backed by `:memory:` sqlite (via `store.NewAuditRepo` + `engine.NewAudit`); assert rows in `audit_log` table; capture slog via a test `slog.Handler` for `LogHandshakeOK`/`LogHandshakeFailed`. `go test -cover ./internal/audit` → ≥ 50% (was 0%). | @@ -161,16 +161,16 @@ new orchestration capability) | Task ID | Owner | Wave | Must | Title | Files touched | Acceptance criterion | |---------|-------|------|------|-------|---------------|----------------------| | T02.5 | backend-engineer | 2 | Y | Implement `pinnedHostKeyCallback` (REQ-058, AD-028) | `internal/proxmox/bootstrap.go` | `pinnedHostKeyCallback(expectedSHA256Base64 string) (ssh.HostKeyCallback, error)`: validate `SHA256:` prefix up front (reject raw hex with a clear error per D-045); callback receives server's `ssh.PublicKey`, computes `ssh.FingerprintSHA256(key)` (via T02.1 helper or inline), compares full strings to the operator-supplied value; returns `nil` on match, `error` on mismatch (fail closed). In `BootstrapProxmox`: if `opts.HostKeyFingerprint != ""` use `pinnedHostKeyCallback`, else fall back to the TOFU callback (T02.6). Unit test: match → callback returns nil; mismatch → returns error mentioning REQ-058; non-`SHA256:`-prefixed input → constructor returns error. | -| T02.6 | backend-engineer | 2 | Y | FIX the latent TOFU capture bug (RESEARCH §2.1, §5 pitfall #1) | `internal/proxmox/bootstrap.go` | Wrap `knownhosts.New(...)` with a custom callback that: on `*knownhosts.KeyError{Want: []}` (host unknown) captures the server-presented `ssh.PublicKey`, writes a line via `knownhosts.Line([]string{knownhosts.Normalize(addr)}, key)` to `certpaths.KnownHostsPath()` using `security.WriteAtomic` (T02.2, AD-029), and returns `nil` (allow the dial to proceed). On `*knownhosts.KeyError{Want: [knownKey]}` (mismatch) returns the error (MITM detection). On `nil` (host present + match) returns `nil`. This fixes the v0.6 latent bug where first-connect always failed. Unit test: first-connect captures the key + writes known_hosts; second-connect matches; mismatch-connect fails. | +| T02.6 | backend-engineer | 2 | Y | **BUGFIX (v0.6 ship-defect)**: FIX the latent TOFU capture bug (RESEARCH §2.1, §5 pitfall #1, GRILL condition #1) | `internal/proxmox/bootstrap.go` | Wrap `knownhosts.New(...)` with a custom callback that: on `*knownhosts.KeyError{Want: []}` (host unknown) captures the server-presented `ssh.PublicKey`, writes a line via `knownhosts.Line([]string{knownhosts.Normalize(addr)}, key)` to `certpaths.KnownHostsPath()` using `security.WriteAtomic` (T02.2, AD-029), and returns `nil` (allow the dial to proceed). On `*knownhosts.KeyError{Want: [knownKey]}` (mismatch) returns the error (MITM detection). On `nil` (host present + match) returns `nil`. This fixes the v0.6 latent ship-defect where first-connect Proxmox join always failed (verified against `golang.org/x/crypto@v0.54.0/ssh/knownhosts/knownhosts.go:370-385`). P04 audit must record this as ship-defect closure. Unit test: first-connect captures the key + writes known_hosts; second-connect matches; mismatch-connect fails. | | T02.7 | backend-engineer | 2 | Y | Populate `Result.HostKeyFingerprint` (RESEARCH §2.2, §5 pitfall #2) | `internal/proxmox/bootstrap.go` | In the capture path (T02.6) and the pinned path (T02.5), set `Result.HostKeyFingerprint = ssh.FingerprintSHA256(hostKey)` (via T02.1). The field is currently declared (bootstrap.go:83-85) but always `""`. After T02.7, `orca node join --type proxmox` output includes the real fingerprint. Unit test: `Result.HostKeyFingerprint` is non-empty + `SHA256:`-prefixed after a successful bootstrap. | | T02.8 | backend-engineer | 2 | Y | Implement `orca node key-reset ` (REQ-059, D-046, AD-029) | `internal/cli/node.go`, `internal/proxmox/bootstrap.go` (new `ResetHostKey` helper OR inline in cli) | New `nodeKeyResetCmd` (`&cobra.Command{Use: "key-reset ", Args: cobra.ExactArgs(1), RunE: ...}`) registered via `nodeCmd.AddCommand(nodeKeyResetCmd)` (node.go:358-360). `RunE`: (1) resolve `` arg via `nodeRegistry()` (node.go:37) → get node row → use `node.Name` (the host address for proxmox nodes) as the `known_hosts` match key; (2) call `proxmox.ResetHostKey(host) error` which reads `certpaths.KnownHostsPath()`, filters lines whose host field (before first whitespace, normalized via `knownhosts.Normalize`) matches, rewrites via `security.WriteAtomic` (T02.2); (3) audit-log `event=node.key_reset` with `actor`+`node`+`host` via `engine.Audit.Record`; (4) print `✓ Host key reset for (next connect will re-pin via TOFU or --host-key-fingerprint)`. **Local only — do NOT revoke remote authorized_keys** (D-046). Unit test: known_hosts with 2 entries for the target host + 1 for another host → after reset, target's 2 lines removed, other host's line intact; audit row inserted. | -| T02.9 | backend-engineer | 2 | Y | Apply the TOFU capture-fix to `doctor proxmox` probe | `internal/doctor/doctor.go` | The doctor proxmox probe (doctor.go:412-415) uses the same `knownhosts.New(...)` callback pattern as bootstrap. Apply the same capture-fix wrapper (T02.6) so `doctor proxmox` on a first-connect node doesn't fail. (If the doctor probe already relies on a prior `node join` having populated `known_hosts`, the fix is still correct — it makes the doctor robust to a missing entry.) | +| T02.9 | backend-engineer | 2 | Y | Apply the TOFU capture-fix to `doctor proxmox` probe (GRILL condition #2 — doctor parity with bootstrap) | `internal/doctor/doctor.go` | The doctor proxmox probe (doctor.go:412-415) uses the same `knownhosts.New(...)` callback pattern as bootstrap. Apply the same capture-fix wrapper (T02.6) so `doctor proxmox` on a first-connect node doesn't fail. **P02 is not complete until both bootstrap (T02.6) and doctor (T02.9) callbacks use the capture-fix wrapper — GRILL condition #2 binding parity check.** (If the doctor probe already relies on a prior `node join` having populated `known_hosts`, the fix is still correct — it makes the doctor robust to a missing entry.) | ### Wave 3 — End-to-end integration + verification | Task ID | Owner | Wave | Must | Title | Files touched | Acceptance criterion | |---------|-------|------|------|-------|---------------|----------------------| -| T02.10 | backend-engineer | 3 | Y | End-to-end trust-surface integration tests | `internal/proxmox/bootstrap_test.go` (extend), `internal/cli/node_test.go` (extend) | (1) `--host-key-fingerprint` with a correct pin → bootstrap succeeds + `Result.HostKeyFingerprint` matches the pin; (2) `--host-key-fingerprint` with a wrong pin → bootstrap fails fast with the REQ-058 mismatch error; (3) no `--host-key-fingerprint` + first connect (empty known_hosts) → TOFU captures the key + writes known_hosts + bootstrap succeeds; (4) no flag + second connect (known_hosts has the key) → matches + succeeds; (5) no flag + mismatch (known_hosts has a different key) → fails with MITM error; (6) `orca node key-reset ` → known_hosts entry removed + audit row inserted + next connect re-pins. | +| T02.10 | backend-engineer | 3 | Y | End-to-end trust-surface integration tests | `internal/proxmox/bootstrap_test.go` (extend), `internal/cli/node_test.go` (extend) | (1) `--host-key-fingerprint` with a correct pin → bootstrap succeeds + `Result.HostKeyFingerprint` matches the pin; (2) `--host-key-fingerprint` with a wrong pin → bootstrap fails fast with the REQ-058 mismatch error; (3) no `--host-key-fingerprint` + first connect (empty known_hosts) → TOFU captures the key + writes known_hosts + bootstrap succeeds; (4) no flag + second connect (known_hosts has the key) → matches + succeeds; (5) no flag + mismatch (known_hosts has a different key) → fails with MITM error; (6) `orca node key-reset ` → known_hosts entry removed + audit row inserted + next connect re-pins; (7) known_hosts pre-populated (v0.6→v0.8 migration path: existing entry from a prior join) → second-connect matches without re-capture, covering the upgrade path. | | T02.11 | backend-engineer | 3 | Y | `--host-key-fingerprint` non-proxmox type validation test | `internal/cli/node_test.go` (extend) | `orca node join --type linux --host-key-fingerprint SHA256:...` → clear error ("--host-key-fingerprint requires --type proxmox today"). Validates D-044 RunE check from T02.3. | ### Phase 2 Must-Haves (summary) @@ -213,7 +213,7 @@ approach, AD-030) + §4 AD-030 | Task ID | Owner | Wave | Must | Title | Files touched | Acceptance criterion | |---------|-------|------|------|-------|---------------|----------------------| -| T03.1 | lead-developer | 1 | Y | `cmd/verify-reqs/main.go` — Go program (~80 LOC, stdlib only, AD-030) | `cmd/verify-reqs/main.go` (NEW) | Parses `.ciagent/ROADMAP.md` + `.ciagent/REQUIREMENTS.md` using `regexp` (stdlib). Assertion (forward direction, RESEARCH §3.3): for every REQ-ID in REQUIREMENTS.md whose `Phase` column references a milestone that ROADMAP marks `**COMPLETE**`, the REQUIREMENTS `Status` must be `**Complete**`. Regex: REQUIREMENTS row `^\|\s*(REQ-\d+)\s*\|.*?\|\s*\*\*(Complete\|Pending)\*\*\s*\|`; ROADMAP milestone-complete `^##\s*Milestone\s+v0\.\d+:.*—\s*\*\*COMPLETE\*\*`; map milestone → REQs via the REQUIREMENTS `Phase` column (e.g. `v0.7 P1` → milestone `v0.7`). Exit 0 on consistency; exit 1 with a diff listing (REQ-ID + current status + expected Complete) on drift. CLI: `go run ./cmd/verify-reqs .ciagent/ROADMAP.md .ciagent/REQUIREMENTS.md` (args optional; defaults to those paths). | +| T03.1 | lead-developer | 1 | Y | `cmd/verify-reqs/main.go` — Go program (~80 LOC, stdlib only, AD-030, GRILL condition #4 regex + reverse direction) | `cmd/verify-reqs/main.go` (NEW) | Parses `.ciagent/ROADMAP.md` + `.ciagent/REQUIREMENTS.md` using `regexp` (stdlib). **Forward assertion**: for every REQ-ID in REQUIREMENTS.md whose `Phase` column references a milestone that ROADMAP marks COMPLETE (substring-match `COMPLETE` within the bold span — NOT exact `\*\*COMPLETE\*\*` which misses v0.2's `**COMPLETE (merged to main via v0.3)**` header at ROADMAP.md:23), the REQUIREMENTS `Status` must be `Complete`. **Reverse assertion (GRILL condition #4)**: for every REQ-ID in REQUIREMENTS.md marked `Complete`, the corresponding milestone in ROADMAP.md must be marked COMPLETE. Regex: REQUIREMENTS row `^\|\s*(REQ-\d+)\s*\|.*?\|\s*\*\*(Complete\|Pending)\*\*\s*\|`; ROADMAP milestone-complete `^##\s*Milestone\s+v0\.\d+:.*—\s*\*\*COMPLETE[^\*]*\*\*` (substring tolerant); map milestone → REQs via the REQUIREMENTS `Phase` column (e.g. `v0.7 P1` → milestone `v0.7`). Exit 0 on consistency; exit 1 with a diff listing (REQ-ID + current status + expected status + direction) on drift. CLI: `go run ./cmd/verify-reqs .ciagent/ROADMAP.md .ciagent/REQUIREMENTS.md` (args optional; defaults to those paths). **Scope note (GRILL)**: REQ-060 catches doc-vs-doc drift only; code-vs-doc drift (e.g. the REQ-053 `cert_repo_test.go` omission — verified missing) is out of scope for this gate and handled by P04 `ciagent-audit`. | | T03.2 | lead-developer | 1 | Y | `cmd/verify-reqs/main_test.go` — golden-file tests | `cmd/verify-reqs/main_test.go` (NEW), `cmd/verify-reqs/testdata/` (NEW: `roadmap_clean.md`, `requirements_clean.md`, `roadmap_drift.md`, `requirements_drift.md`) | (1) Clean pair (ROADMAP v0.X COMPLETE + REQUIREMENTS REQ-XXX Complete) → exit 0, no diff; (2) Drift pair (ROADMAP v0.X COMPLETE + REQUIREMENTS REQ-XXX Pending) → exit 1 + diff lists the stale REQ; (3) Multiple drifts → all reported; (4) Missing args → uses defaults; (5) Malformed markdown → clear error (not a silent pass). | ### Wave 2 — Makefile + CI hook