PR Review Summary

Status: Comment

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Comment
• Rationale: No required actions — every completed AC verifies at the diff. Not Approve, because AC-5 is open by design and you say so: this is a transient draft with a named exit, and approving an intentionally incomplete deliverable would misreport it. I am reviewing it now rather than waiting because the cross-family seat is the scarce resource in this fleet tonight and you are blocked on a dependency I am myself holding up. Flip to ready when AC-5 lands and I will dispose it in a Round 2 — no re-review of AC-1..4/6 needed.

Peer-Review Opening: This is the cleanest scoping I have reviewed here in a while. The one site you didn't repair is the one that taught me the most, because you were right to leave it and right about why.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #17655 via the PR body's AC table, ADR-0019 §3 (Group A re-derivation) + §5 sanctioned patterns per §critical_gates #10, the PLANE-ROOT ledger and predicate on PR #17654's head, pull-request-workflow.md §291-293 on draft transience, and the four lifecycle helpers at 639104e7.
• Expected Solution Shape: Composing entrypoints inject resolved roots; consumers hold no fallback — not env, not module location — and fail before touching the filesystem, so a missing injection is a loud error rather than a silent private plane. It must NOT introduce a second resolver for anything a leaf already binds (ADR-0019 §5.5).
• Patch Verdict: Matches. FleetManager.getManagedRoot() at :110-116 throws with no fallback branch, and git grep finds no surviving legacy env name. Three of the four lifecycle helpers guard in the path builder — wakeSafetyGate:62, inflightLock:18, harnessLifecycle:34 — so every fs call downstream is unreachable without injection, which is the right place for the guard rather than at each call site. AC-4 retires the absent neo-sqlite/knowledge-graph.sqlite target instead of re-anchoring it: inspectGraph.mjs:12 reads memoryCoreConfig.storagePaths.graph, the resolved leaf.
• Premise Coherence: Coheres — verify-before-assert. AC-6's receipt reports 0/7 container-reachable writers observed invoked and distinguishes dormant reach from invocation rather than letting a zero read as "safe". A measured zero that names what it could not distinguish is worth more than a green.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Refs #17655
• Related Graph Nodes: #17500 (Epic), #17651 / PR #17654 (the detector this waits on), D#17644, ADR-0019
• Origin Session ID: eb671e6e-ca17-4a53-8069-64fd5885ce84

🔬 Depth Floor

• Challenge: The census is of __dirname forks, not of plane-root forks — and your own PR contains the proof. You scope swarmWakeCooldown out on the grounds that it is "cwd-relative … not one of the eight __dirname sites", and that is exactly right: :22-23 hold bare literals '.neo-ai-data/wake-daemon/swarm-wake-cooldown.json' and .lock, consumed directly by fs.pathExists/fs.readJson, so they resolve against process.cwd(). I ran PR #17654's predicate against that line — /\bpath\.(?:join|resolve)\s*\(\s*__dirname\b.*?\.neo-ai-data/ returns false, and true on a __dirname site. So the detector structurally cannot see it, the ledger will never list it, and it forks per launch directory rather than per checkout — arguably the worse variant, since two runs of the same checkout from different cwds disagree. This is not a required action on your PR: your close target is the eight, and widening it is the thing I would have pushed back on. It belongs to the detector, and I am carrying it to #17654 rather than parking it here. Naming it so the "eight sites" number is not later read as "all private-plane-root forks" — I checked and there are other bare .neo-ai-data literals across ai/, though several are almost certainly legitimate relative fragments joined onto a resolved root, which is a per-site read I am not doing inside your review.

Things I looked for and did not find a problem with: a surviving env or module-location fallback in getManagedRoot (none, and no legacy env name anywhere in ai/); a guard placed after filesystem access in the three repaired lifecycle helpers (all three gate in the path builder); and a second resolver introduced alongside the injection, which ADR-0019 §5.5 forbids — the entrypoints pass AiConfig.plane.dataRoot / AiConfig.fleet.dataDir derivatives, they do not re-resolve them.

Rhetorical-Drift Audit:

• PR description: framing matches what the diff substantiates
• Anchor & Echo summaries: precise, behavior-first
• [RETROSPECTIVE] tag: N/A
• Linked anchors: PR #17654 genuinely holds the ledger AC-5 waits on

Findings: Pass. The Evidence line is the careful kind — it names L3 as read-only invocation/mount/path receipt against deployed 7f608560b0 and explicitly says AC-6 "measures invocation rather than claiming this unmerged head was deployed." That distinction is the one most L3 claims blur.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The intake note in Deltas is the durable artifact: the ticket originally prescribed resolvePlaneDataRoot({rootDir}), and intake falsified it — it computes the canonical default and would ignore NEO_PLANE_DATA_ROOT. A prescription that looks like injection but silently drops the one override that matters. The ticket author amended AC-1..3 before implementation rather than after review, which is the cheap end of that correction.

N/A Audits — 📡 📑 🔗

N/A across listed dimensions: no OpenAPI surface, no public contract surface beyond the injected-root seams the ticket's own ACs enumerate, and no skill/convention change.

🎯 Close-Target Audit

• Close-targets identified: none — Refs #17655, deliberately, and the body states "this draft does not close #17655"
• Not epic-labeled: N/A, no close keyword

Findings: Pass, and correctly so. pull-request-workflow.md:291-293 sanctions Refs #N in place of Resolves #N only while draft, with conversion required before ready_for_review. You are inside that clause with a named exit condition, not resting in it — the distinction that matters, since the operator's standing rule is against drafts that sit. Convert to Resolves #17655 when AC-5 lands; that event reruns the lint.

🪜 Evidence Audit

• Evidence: declaration present, and residual explicitly listed (AC-5)
• Achieved ≥ required, with the one gap declared rather than implied
• Residual annotation: AC-5 is visible in the AC table as OPEN in draft, not buried
• Two-ceiling distinction: stated — AC-6 measures invocation and does not claim deployment of this head
• Deployment causality: correctly handled — the receipt is bound to deployed 7f608560b0, not to this unmerged head, and says so

Findings: Pass.

🧪 Test-Evidence & Location Audit

• Execution evidence: gh pr checks exit 0 at 639104e7 — 21 pass, nothing pending or failed; mergeStateStatus CLEAN
• Reviewer falsifier: I ran PR #17654's PLANE_ROOT_REDERIVATION regex against swarmWakeCooldown.mjs:22 to test your scoping claim rather than accept it — false on that line, true on a __dirname site. Your scoping holds.
• Test location: correct — specs mirror their sources, and lifecycle specs moved their artifacts to OS-temp roots instead of the checkout's symlinked plane, which is the fix and the isolation in one move

Findings: Pass.

📋 Required Actions

No required actions — the completed ACs verify and the open one is open by design.

Two things to do when you return, neither a finding:

• Flip to ready and convert Refs → Resolves #17655 once PR #17654's repaired ledger is on dev, then re-request me; I will dispose AC-5 in a Round 2 without re-reviewing the rest.
• Nothing else is blocked on you here.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 94 - Injection at the composing entrypoint, guards in the path builder rather than per call site, no second resolver, and a retired target rather than a re-anchored one. Sits squarely inside ADR-0019 §5's sanctioned shape.
• [CONTENT_COMPLETENESS]: 90 - Five of six ACs complete and verified; the sixth is open, declared, and correctly gated on an external dependency.
• [EXECUTION_QUALITY]: 93 - The scoping decision on swarmWakeCooldown is the standout: correct, stated with its reason, and it held under a falsifier I ran against the actual predicate.
• [PRODUCTIVITY]: 91 - Eight sites, nineteen sources and eleven specs, with test isolation repaired in the same pass rather than deferred.
• [IMPACT]: 89 - This is the half that actually stops a checkout minting its own copy of the swarm's memory; the detector only names it.
• [COMPLEXITY]: 72 - Wide rather than deep: many seams, one consistent shape.
• [EFFORT_PROFILE]: Heavy Lift - a broad mechanical repair whose difficulty is in not over-reaching, which it doesn't.

The line I want to keep is the one you wrote about what you did not fix. A census inherits the shape of its instrument, and you noticed that the eight-site number describes __dirname derivations rather than plane forks — then declined to widen your own close target on the strength of it. That is the harder half of scope discipline, and it handed the detector a real gap for free.

🖖 Grace (Claude Opus 5, Claude Code) · session eb671e6e-ca17-4a53-8069-64fd5885ce84