# Grill Report: Orca v0.3 — scheduling-streaming **Date:** 2026-08-01 **Reviewer:** ci-griller (red-team, adversarial) **Plan under review:** `.ciagent/PLAN_v0.3.md` (commit 89fa172) **Branch:** `phase/00-pre-execution` **Mode:** Full autonomy --- ## Methodology Every claim in `PLAN_v0.3.md` and `RESEARCH_v0.3.md` was cross-checked against the actual codebase (the 7 source files listed in the task, plus `migrate.go`, `store.go`, `doctor_test.go`, `security/integration_test.go`, `security/ca.go`, `security/csr.go`, `model/node.go`, `model/job.go`, and `cli/doctor.go`). Findings are scored on 9 axes. Binding verdicts are ACCEPT (plan must change), REJECT (concern noted, plan stands), or DEFER (address during execution). --- ## Summary Verdict | Severity | Count | |----------|-------| | CRITICAL | 2 | | HIGH | 2 | | MEDIUM | 5 | | LOW | 3 | | **Total** | **12** | **Overall verdict: PROCEED WITH CHANGES** The plan is fundamentally sound — the scope is right-sized, the requirements coverage is complete, the persona territories are respected, and the no-new-dependencies promise holds. However, two CRITICAL findings require plan changes before execution begins. Neither is a scope expansion; both are correctness fixes to the design as written. With the 2 ACCEPT changes applied, this plan is ready to execute. --- ## Per-Axis Findings ### Axis 1 — Feasibility (can each task actually be implemented?) #### F-01 [CRITICAL] — Watch yields per-row but CLI table mode requires full-snapshot-per-tick **Severity:** CRITICAL **Axis:** Feasibility / Vertical slice integrity **Binding verdict:** ACCEPT (plan must change) **Finding:** The plan is internally contradictory about what `Watch` yields. - D-028 (RESEARCH:88) says Watch "yields the **full current snapshot** (one element per row)." - Task 01-01-01 (PLAN:30) says Watch "yields one `*model.Job` per row via `scanJob`" — i.e., `iter.Seq[*model.Job]`, one element per row per tick. - Task 01-02-02 (PLAN:42) says the CLI table render "collect the full snapshot from `seq` into a `[]*model.Job`" then compares against the previous snapshot's rendered table. These are incompatible. `iter.Seq[*model.Job]` yields individual jobs with **no tick-boundary signal**. The CLI ranging `for job := range seq` receives a flat stream of jobs and cannot know when a tick's snapshot is complete. It cannot collect "the full snapshot" because it cannot detect the end of a tick. The JSON mode (01-02-03) can work without tick boundaries (per-element dedup via `map[string][]byte`), but the **table mode cannot**. Table mode needs the complete snapshot to render the table, clear the screen, and compare against the previous frame. **Evidence:** - `RESEARCH_v0.3.md:106-142` — implementation yields `yield(j)` per row inside `for rows.Next()`, not `yield(allJobs)` per tick. - `PLAN_v0.3.md:30` — "yields one `*model.Job` per row" - `PLAN_v0.3.md:42` — "collect the full snapshot from `seq` into a `[]*model.Job`" - `PLAN_v0.3.md:44` (01-02-04) — nodeListCmd watch bypasses registry, same per-row yield. - D-028 says "full current snapshot" but the code yields per-row. **Required change:** Change the `Watch` element type from `iter.Seq[*model.Job]` to `iter.Seq[[]*model.Job]` (and `iter.Seq[[]*model.Node]` analogously). Each tick yields the **full snapshot as a single slice**. This: 1. Makes D-028 ("yields the full current snapshot") literally true. 2. Makes table mode trivial: `for snapshot := range seq { render(snapshot) }`. 3. Makes JSON mode cleaner: per-tick, diff the snapshot against the previous one, emit one JSON line per changed element. This also enables a natural `"delete"` event for elements that disappeared (not possible with per-row yield). 4. Simplifies the test contract: `TestWatch_YieldsSnapshots` ranges over `iter.Seq[[]*model.Job]` and each yield is a complete tick — no timing ambiguity about "did I get all rows for this tick?" **Impact on plan:** - Tasks 01-01-01, 01-01-02: signature changes to `iter.Seq[[]*model.Job]` / `iter.Seq[[]*model.Node]`. Implementation collects all rows into a slice per tick, then `yield(slice)`. - Task 01-02-02 (table): `for snapshot := range seq { ... }` — direct, no collection needed. - Task 01-02-03 (JSON): per-tick diff against previous snapshot's `map[string][]byte`. Emit `"init"`/`"update"`/`"delete"` events. - Task 01-01-04 (tests): assert each yield is a complete snapshot slice. - D-026, D-028, D-046: update to reflect slice-per-tick semantics. - Must-have criteria for 01-01-01/01-01-02: update signature assertions. This is a mechanical change to the plan, not a scope change. The implementation is simpler (no tick-boundary detection needed). **Confidence:** 0.92 --- #### F-02 [CRITICAL] — First-tick delay: Watch waits a full interval before first yield **Severity:** CRITICAL **Axis:** Feasibility / UX correctness **Binding verdict:** ACCEPT (plan must change) **Finding:** The Watch implementation (RESEARCH:110-116) has this structure: ```go ticker := time.NewTicker(1 * time.Second) defer ticker.Stop() for { select { case <-ctx.Done(): return case <-ticker.C: // <-- waits 1s BEFORE first query } // query + yield } ``` The `select` waits for the first ticker pulse **before** running the first query. With a 1s default interval, `orca job list --watch` shows **nothing for 1 full second**, then the first snapshot appears. For a CLI tool, a 1s blank screen is a poor UX and looks broken. The user expects immediate output, then refreshes every 1s. The tests (01-01-04) use `watchInterval=10ms`, so the delay is only 10ms and the test passes — but the test does NOT catch this UX bug because the interval is tiny. In production (1s), the bug is visible. **Evidence:** - `RESEARCH_v0.3.md:110-116` — `select` before first query. - `PLAN_v0.3.md:30` — "pull-based inline polling loop on a 1s ticker" — no mention of immediate first yield. - Standard `top`-like tools yield immediately, then tick. **Required change:** Add to tasks 01-01-01 and 01-01-02: the polling loop must **query and yield immediately on the first iteration**, then `select` on the ticker for subsequent ticks. Implementation shape: ```go for { // query + yield (runs immediately on first iteration) rows, err := r.db.QueryContext(ctx, ...) // ... yield snapshot ... select { case <-ctx.Done(): return case <-ticker.C: } } ``` Or equivalently, query once before the loop, then loop with select-first. The must-have criteria should add: "first yield occurs immediately (no `watchInterval` delay before first snapshot)." **Impact on plan:** - Tasks 01-01-01, 01-01-02: add "immediate first yield" to description + must-have. - Task 01-01-04 (tests): add assertion that the first snapshot appears within a short deadline (e.g., <50ms) even with `watchInterval=10ms` — proving the first yield is not tick-gated. **Confidence:** 0.95 --- #### F-03 [HIGH] — P01 and P02 both modify `internal/cli/node.go` (file-disjoint claim is false) **Severity:** HIGH **Axis:** Feasibility / Timeline (parallelism) **Binding verdict:** ACCEPT (plan must change) **Finding:** D-042 (PLAN:133, RESEARCH:489) claims "P01 and P02 are file-disjoint — no file is modified by both." This is **false**. - P01 task 01-02-04 (PLAN:44) modifies `internal/cli/node.go` — adds `--watch` flag + render modes to `nodeListCmd`. - P02 task 02-01-01 (PLAN:76) modifies `internal/cli/node.go` — removes the `dbPath` function and updates `openDB` to call `certpaths.DBPath()`. Both phases touch `internal/cli/node.go`. If developed in parallel (as D-042 permits), this causes merge conflicts. **Evidence:** - `PLAN_v0.3.md:44` — 01-02-04 files: `internal/cli/node.go` - `PLAN_v0.3.md:76` — 02-01-01 files: `internal/cli/node.go` (remove old `dbPath`) - `PLAN_v0.3.md:133` — "P01 and P02 are file-disjoint" - Actual code: `internal/cli/node.go:22-28` defines `dbPath`; `:30-36` defines `openDB` which calls `dbPath()`. `openDB` is used by 14 call sites across `job.go`, `daemon.go`, `node_capacity.go`, `audit.go`, `node.go`. **Required change:** Update D-042 and the cross-phase notes (PLAN:131-133) to acknowledge the overlap. Two options (pick one): 1. **Serialize:** P02 Wave 1 (02-01-01) runs before P01 Wave 2 (01-02-04). P02 Wave 1 is a prerequisite for P01 Wave 2 on the `node.go` file. P01 Wave 1 (store layer) and P02 Wave 1 can still run in parallel. 2. **Merge the changes:** task 02-01-01 is folded into P01 Wave 2's `node.go` modification (the cli-engineer updates `openDB` to use `certpaths.DBPath()` while also adding `--watch`). Recommended: Option 1 (serialize P02 Wave 1 before P01 Wave 2). It preserves the wave structure and persona assignments. Update the cross-phase note to say: "P02 Wave 1 (02-01-01) must complete before P01 Wave 2 (01-02-04) due to shared `internal/cli/node.go` modification. P01 Wave 1 and P02 Wave 1 may run in parallel." **Confidence:** 0.90 --- #### F-04 [HIGH] — D-037 ServerName = node.Name assumption is fragile and unverified against real join flow **Severity:** HIGH **Axis:** Feasibility / Security **Binding verdict:** DEFER (address in execution, with documentation) **Finding:** D-037 (RESEARCH:261, confidence 0.80) assumes `serverName = node.Name` for the mTLS health probe. The TLS client's `ServerName` must match a SAN entry on the peer's server cert. But `GenerateCSR(commonName, sans)` (csr.go:24) takes the commonName and SANs as **separate arguments**. The commonName becomes the cert Subject CN, but `ServerName` in `tls.Config` is matched against **SANs** (DNSNames/IPAddresses), not the CN (per Go's `crypto/tls` behavior since Go 1.15). If a node joined with `--name node-b` but its cert SAN is `localhost` (or an IP), `serverName = "node-b"` will **fail the TLS handshake** with a "certificate is valid for localhost, not node-b" error — even though the peer is perfectly healthy. The research (RESEARCH:261) says "confirmed in `integration_test.go:41` `GenerateCSR("test-server", ...)`" — but that test uses `serverName = "localhost"` (integration_test.go:83), which matches the SAN `localhost`, not the commonName `test-server`. The test proves SAN-matching, not CN-matching. **Evidence:** - `internal/security/csr.go:24` — `GenerateCSR(commonName, sans)` — CN and SANs are separate. - `internal/security/integration_test.go:41` — `GenerateCSR("test-server", []string{"localhost", "127.0.0.1"})` — CN is "test-server", SANs are localhost/127.0.0.1. - `internal/security/integration_test.go:83` — `ClientTLSConfig(..., "localhost", ...)` — serverName = "localhost" (a SAN), NOT "test-server" (the CN). - `internal/transport/mtls.go:49-51` — `serverName` is required and set as `tls.Config.ServerName` (matched against SANs). - `PLAN_v0.3.md:88` — 02-02-03: `serverName = n.Name`. **Mitigation (DEFER to execution):** 1. Document the assumption in the `Network()` check message: "probing at (assuming cert SAN = node name)". 2. If the handshake fails with a SAN mismatch error, the FAIL message should include the cert's actual SANs (parsed from the error) so the operator can diagnose. This is a refinement, not a plan blocker. 3. The test 02-02-05(e) uses `Name = "localhost"` which matches the SAN — so the test passes, but it doesn't prove the general case. Add a test comment noting this assumption. **Why DEFER not ACCEPT:** The assumption is documented (D-037, 0.80 confidence), the failure mode is graceful (FAIL with handshake error, not a crash), and fixing it properly (storing SANs in the nodes table) is a scope expansion beyond v0.3. The plan should note the limitation; execution should add diagnostic context to the error message. **Confidence:** 0.78 --- #### F-05 [MEDIUM] — `store.Open` runs migrations before integrity_check can run **Severity:** MEDIUM **Axis:** Feasibility / Testing **Binding verdict:** REJECT (concern noted, plan stands) **Finding:** The DB check (02-02-01) calls `store.Open(path)` which runs `migrate(db)` (store .go:39) before the integrity_check executes. On a truly corrupt DB, `store.Open` fails at `Ping()` or `migrate()` — the integrity_check never runs. The check returns FAIL with the open/migrate error, which is the correct outcome (a DB that can't be opened is broken), but the message says "open : " not "integrity_check failed." The plan's `TestDBCheck_Corrupt` (RESEARCH:469) is explicitly called "brittle" and made optional. The plan accepts that integrity_check is somewhat redundant with `store.Open`'s own validation. **Evidence:** - `internal/store/store.go:37-41` — `db.Ping()` then `migrate(db)` inside `Open`. - `PLAN_v0.3.md:86` — 02-02-01: `db, err := store.Open(path)`. - `RESEARCH_v0.3.md:455-456` — pitfall table acknowledges this. **Why REJECT:** The failure surfaces correctly (FAIL with error message). The integrity_check adds value for the case where the DB opens but has logical corruption (e.g., foreign key violations, orphaned pages) that Ping/migrate don't catch. The plan's approach is acceptable for v0.3. The optional corrupt test is correctly deferred. **Confidence:** 0.85 --- ### Axis 2 — Scope #### F-06 [MEDIUM] — No "delete" event in JSON watch mode (with per-row yield) **Severity:** MEDIUM **Axis:** Scope / Completeness **Binding verdict:** DEFER (address in execution) **Finding:** With the current per-row `iter.Seq[*model.Job]` design (F-01), the JSON watch mode (01-02-03) emits `"init"` and `"update"` events but has no way to emit `"delete"` events — a job that disappears from the snapshot simply stops being yielded, and the CLI has no tick boundary to detect "this ID was in the previous tick but not this one." With the F-01 fix (`iter.Seq[[]*model.Job]`, full snapshot per tick), `"delete"` events become trivially possible: diff the previous snapshot's ID set against the current snapshot's ID set. The plan should add `"delete"` event semantics to D-046. **Evidence:** - `PLAN_v0.3.md:43` — 01-02-03: only `"init"` and `"update"` events. - `PLAN_v0.3.md:139` — D-046: only `"init"` and `"update"`. - Neither jobs nor nodes are hard-deleted in the current CLI (`node leave` sets state to `left`, doesn't delete the row), so `"delete"` events are not strictly needed for v0.3. But the `NodeRepo.Delete` method exists and could be used by future code. **Mitigation (DEFER):** If F-01 is accepted (slice-per-tick), add `"delete"` event to D-046 as a natural extension. If F-01 is not accepted, document the no-delete-event limitation explicitly. **Confidence:** 0.70 --- ### Axis 3 — Testing #### F-07 [MEDIUM] — Test timing fragility: 10ms tick + 30ms insert + 80ms cancel **Severity:** MEDIUM **Axis:** Testing **Binding verdict:** DEFER (address in execution) **Finding:** The store-layer tests (01-01-04) use `watchInterval=10ms` with timing-based assertions: "insert a 2nd job from a goroutine after ~30ms, cancel ctx after ~80ms." Under CI load (especially with `-race` overhead), 10ms ticks can be missed or delayed. A 10ms ticker pulse is not guaranteed to fire within 10ms under load — the Go runtime scheduler may delay it. If the 2nd job is inserted at 30ms but the 2nd tick fires at 45ms, the test might see the 2nd job in the 3rd tick (at ~55ms) which is still before the 80ms cancel — so it likely passes, but it's fragile. **Evidence:** - `PLAN_v0.3.md:33` — 01-01-04: "after ~30ms", "after ~80ms". - `time.NewTicker` does not guarantee exact timing under load. **Mitigation (DEFER):** Use more generous margins (e.g., 50ms insert, 200ms cancel) or a synchronization mechanism (e.g., insert the 2nd job, then poll the collected slice with a 500ms timeout). The test hook (`watchInterval`) already enables fast tests; the margins just need to be wider. Execution should validate the tests pass reliably under `-race` in CI before marking Wave 1 complete. **Confidence:** 0.75 --- #### F-08 [LOW] — `-race` does not detect goroutine leaks; the plan claims it does **Severity:** LOW **Axis:** Testing **Binding verdict:** REJECT (concern noted, plan stands) **Finding:** The plan (01-01-04 must-have, PLAN:33) says "`-race` reports no leaks/data races." `go test -race` detects **data races**, not **goroutine leaks**. Goroutine leak detection requires `goleak` or explicit goroutine-count assertions. The claim is technically incorrect. However, the actual risk is negligible: Watch does not spawn a goroutine (D-032, inline pull loop). `time.NewTicker` spawns an internal goroutine, but `defer ticker.Stop()` terminates it. There is nothing to leak. The `no-goroutine-leak` constraint (data-engineer persona) is satisfied by design, not by testing. **Evidence:** - `PLAN_v0.3.md:33` — "`-race` reports no leaks/data races" - `RESEARCH_v0.3.md:92` — D-032: "No goroutine is spawned by Watch." - Go `-race` detector documentation: detects concurrent access, not leaks. **Why REJECT:** The claim is imprecise but the risk is zero by design. Execution may optionally add `runtime.NumGoroutine()` before/after assertions for belt-and-suspenders, but it's not required. **Confidence:** 0.90 --- ### Axis 4 — Security #### F-09 [MEDIUM] — Doctor network check probes peers using the local server cert as client cert (confirmed valid, but undocumented) **Severity:** MEDIUM **Axis:** Security **Binding verdict:** DEFER (document in execution) **Finding:** The plan (02-02-03, PLAN:88) uses `certpaths.ServerCertPath()`/`ServerKeyPath()` as the client cert for the mTLS health probe. I verified this is **valid**: `security/ca.go:255` signs server certs with `ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}` — the server cert has both ServerAuth and ClientAuth EKUs, so it can be presented as a client cert. The daemon's `RequireAndVerifyClientCert` (security/tls_config.go:92) will accept it. This is correct and feasible. The finding is that this cross-use (server cert as client cert) is not documented in the plan or the security architecture. A security auditor might flag it as "server cert used for client auth — is this intended?" **Evidence:** - `internal/security/ca.go:255` — `ExtKeyUsage: ServerAuth, ClientAuth`. - `internal/security/tls_config.go:92` — `ClientAuth: RequireAndVerifyClientCert`. - `PLAN_v0.3.md:88` — 02-02-03 uses `ServerCertPath()`/`ServerKeyPath()`. - `RESEARCH_v0.3.md:261` — D-037: "presents the local node's client cert." **Mitigation (DEFER):** Add a code comment in `probeHealthz` and a note in ARCHITECTURE.md §5 explaining that the local server cert doubles as the client cert for doctor probes (justified by the dual EKU). This is documentation, not a code change. **Confidence:** 0.88 --- ### Axis 5 — Performance #### F-10 [LOW] — 1s poll ticker re-runs full List query every second; no concern but worth noting **Severity:** LOW **Axis:** Performance **Binding verdict:** REJECT (concern noted, plan stands) **Finding:** The 1s ticker (D-019) re-runs `SELECT ... FROM jobs ORDER BY created_at DESC` every second. For a CLI tool run by a human watching a terminal, this is fine — the query is cheap (single table, no joins, indexed by `created_at` if an index exists). For an AI agent tailing `--watch --json` for hours, this is 1 query/second × 3600 = 3600 queries/hour. SQLite handles this trivially in WAL mode (store.go:36). The cadence is correct for a "top-like" refresh. Faster (e.g., 100ms) would waste CPU; slower (e.g., 5s) would feel sluggish. 1s is the right default. **Evidence:** - `PROJECT.md:116` — D-019: "Poll-based, 1s ticker" (confidence 0.90). - `internal/store/store.go:36` — WAL mode enabled. **Why REJECT:** The cadence is justified. No change needed. **Confidence:** 0.92 --- ### Axis 6 — Maintainability #### F-11 [LOW] — `watchInterval` package var is mutable global state (test hook) **Severity:** LOW **Axis:** Maintainability **Binding verdict:** REJECT (concern noted, plan stands) **Finding:** D-043 (PLAN:136) uses an unexported package var `watchInterval = 1 * time.Second` in `internal/store`, overridable from `_test.go`. This is mutable global state — if tests run in parallel within the `internal/store` package and one test sets `watchInterval=10ms` while another expects `1s`, they interfere. However, Go tests within a single package run **sequentially** by default unless `t.Parallel()` is called. I verified no test in `internal/store` calls `t.Parallel()` (grep found 0 matches). So the global var is safe as long as no Watch test calls `t.Parallel()`. The plan should note this constraint. **Evidence:** - `PLAN_v0.3.md:32` — 01-01-03: "unexported package var `watchInterval`" - `PLAN_v0.3.md:136` — D-043. - grep for `t.Parallel()` in `internal/`: 0 matches. **Why REJECT:** The approach is pragmatic and safe given sequential test execution. The alternative (a `WatchWithInterval` constructor or an option pattern) would leak test-only API into production, which D-043 explicitly avoids. Execution should add a comment: "do not call t.Parallel() in Watch tests — they share the watchInterval package var." **Confidence:** 0.85 --- ### Axis 7 — Completeness #### F-12 [MEDIUM] — Plan does not address `openDB()` being the single chokepoint for dbPath relocation **Severity:** MEDIUM **Axis:** Completeness / Feasibility **Binding verdict:** DEFER (clarify in execution) **Finding:** Task 02-01-01 (PLAN:76) says "Update `internal/cli/node.go` (and any other `internal/cli` caller of the old unexported `dbPath`) to call `certpaths.DBPath()`." This is imprecise. `dbPath()` is defined in `node.go:22` and called only by `openDB()` in `node.go:31`. `openDB()` is then called by 14 sites across `job.go`, `daemon.go`, `node_capacity.go`, `audit.go`, `node.go`. The correct change is: 1. Add `certpaths.DBPath()`. 2. Change `openDB()` body from `store.Open(dbPath())` to `store.Open(certpaths.DBPath())`. 3. Delete the `dbPath()` function from `node.go`. No other caller needs changing — they all go through `openDB()`. The plan's "any other `internal/cli` caller" language suggests a broader scan that isn't needed. This is a clarity issue, not a correctness issue. **Evidence:** - `internal/cli/node.go:22-28` — `dbPath()` definition. - `internal/cli/node.go:30-36` — `openDB()` calls `dbPath()`. - grep `openDB()`: 14 call sites, all in `internal/cli/`. - grep `dbPath()`: only in `node.go:31` (inside `openDB`). **Mitigation (DEFER):** Execution should note that `openDB()` is the single chokepoint — update its body and delete `dbPath()`. No other file needs changes. The plan's must-have ("`internal/cli` no longer defines `dbPath`") is correct. **Confidence:** 0.88 --- ### Axis 8 — Vertical Slice Integrity Covered by F-01 (the tick-boundary problem breaks the Wave 1 → Wave 2 vertical slice: Wave 1 produces `iter.Seq[*model.Job]` which Wave 2's table mode cannot consume correctly). With F-01's fix (`iter.Seq[[]*model.Job]`), the vertical slice is clean: Wave 1 yields full snapshots, Wave 2 renders them. ### Axis 9 — Risk **Highest-risk task:** 02-02-05(e) `TestNetworkCheck_PeerReachable` — integration test requiring CA bootstrap, server cert signing with correct SAN, httptest TLS server with `RequireAndVerifyClientCert`, node row insert, and mTLS probe. Has the most moving parts and the most assumptions (D-037 ServerName, dual-EKU client cert, httptest HTTP/1.1 vs h2c quirks per integration_test.go:100-126). If D-037 is wrong in production (not in test, since the test uses `Name = "localhost"` matching the SAN), the network check fails for real deployments but the test passes — a false-positive. **What could go catastrophically wrong:** The F-01 tick-boundary issue, if not caught, would cause `orca job list --watch` (table mode) to either hang (trying to collect a "full snapshot" that never completes) or render incomplete tables (rendering after each row instead of after a full tick). This is a user-visible broken feature shipped as "complete." --- ## Binding Decisions (G-series) | ID | Decision | Rationale | Confidence | Verdict | |----|----------|-----------|------------|---------| | G-001 | Change `Watch` to `iter.Seq[[]*model.Job]` / `iter.Seq[[]*model.Node]` (full snapshot per tick) | F-01: per-row yield has no tick boundary; table mode needs full snapshot. Slice-per-tick makes D-028 literally true and simplifies both render modes. | 0.92 | ACCEPT | | G-002 | Watch must yield immediately on first iteration, then tick | F-02: current design waits 1s before first output. Unacceptable UX. | 0.95 | ACCEPT | | G-003 | P02 Wave 1 (02-01-01) must complete before P01 Wave 2 (01-02-04) — shared `internal/cli/node.go` | F-03: D-042 file-disjoint claim is false for `node.go`. | 0.90 | ACCEPT | | G-004 | D-037 ServerName = node.Name assumption is deferred; execution must add diagnostic context to handshake-fail errors | F-04: assumption is documented (0.80), failure is graceful, proper fix is out of v0.3 scope. | 0.78 | DEFER | | G-005 | `store.Open` runs migrations before integrity_check — acceptable | F-05: failure surfaces correctly as FAIL. | 0.85 | REJECT | | G-006 | Add `"delete"` event to JSON watch mode if G-001 is accepted | F-06: slice-per-tick makes delete events trivial. | 0.70 | DEFER | | G-007 | Widen test timing margins (10ms tick → generous insert/cancel margins) | F-07: 10ms ticker under CI load is fragile. | 0.75 | DEFER | | G-008 | `-race` does not detect goroutine leaks — claim is imprecise but risk is zero by design | F-08: no goroutine spawned. | 0.90 | REJECT | | G-009 | Document dual-EKU (server cert as client cert) in `probeHealthz` + ARCHITECTURE.md | F-09: valid but undocumented. | 0.88 | DEFER | | G-010 | 1s poll ticker cadence is correct | F-10: justified by D-019. | 0.92 | REJECT | | G-011 | `watchInterval` package var is safe (no `t.Parallel` in store tests) | F-11: pragmatic, avoids leaking test API. | 0.85 | REJECT | | G-012 | `openDB()` is the single chokepoint for dbPath relocation — clarify in execution | F-12: plan is imprecise but correct. | 0.88 | DEFER | --- ## Escalations None. All 12 findings are resolved with confidence ≥ 0.60 (either ACCEPT, REJECT, or DEFER). No axis requires human escalation. --- ## Overall Verdict **PROCEED WITH CHANGES** The plan is approved for execution **after** the 3 ACCEPT binding verdicts (G-001, G-002, G-003) are applied to `PLAN_v0.3.md`: 1. **G-001:** Change `Watch` element type to `iter.Seq[[]*model.Job]` / `iter.Seq[[]*model.Node]` (full snapshot per tick). Update tasks 01-01-01, 01-01-02, 01-02-02, 01-02-03, 01-02-04, 01-01-04, and decisions D-026, D-028, D-046. 2. **G-002:** Add "immediate first yield" to tasks 01-01-01, 01-01-02 and must-have criteria + test assertion in 01-01-04. 3. **G-003:** Update D-042 and cross-phase notes: P02 Wave 1 (02-01-01) precedes P01 Wave 2 (01-02-04) due to shared `internal/cli/node.go`. The 5 DEFER items (G-004, G-006, G-007, G-009, G-012) are execution-time refinements that do not block the plan. The scope is right-sized (21 tasks across 5 waves, 2 execution phases + 1 review phase). No requirements gaps exist between REQ-022/030/032 and the plan tasks. The no-new-dependencies promise holds. The persona territories are respected. The test strategy is adequate (with the timing-margin note in G-007). The plan does not violate the minimalist pillar. **Confidence in verdict:** 0.88