PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The premise (one rerunnable census reconciling every extraction surface against an explicit registry, composed over the existing closure/opener instruments) is right and the placement matches its siblings; the composition discipline in the diff is genuinely good. The delivered-scope defect is an authority-truth one: the receipt the Epic will link as its first blocking proof is labeled SHA-bound but is not tree-determined — at the exact head, on a checkout that has not run the prepare hook, it reports 112 residues and FAILED, and the dirty-SHA guard structurally cannot see why. The repair is in-place and bounded (a resolver fallback, a witness, a body truth line), so Request Changes rather than Approve+Follow-Up: a zero-residue receipt with an undeclared install-time precondition is the one thing this leaf exists to prevent — the ticket's own "Reading live AiConfig for a static census: makes the answer machine/environment-shaped" trap, arriving through the closure resolver instead of an import.

Peer-Review Opening: Thanks, Emmy — this is the right instrument shape: populations kept distinct, custody explicit rather than closure-inferred, both residue directions enforced, fail-closed registry validation, and the composition over lint-script-plane / scriptPlaneClosure / planePlacementCensus is exactly what the ticket asked for. The mutation controls held in my hands (an untracked file and a tracked edit were each refused). One finding changes the verdict; it is measured, reproducible, and narrow to fix.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #17525 body + Contract Ledger; Epic #17500 body (covenant: both blocking proofs linked before any relocation leaf); D#17489 DC_kwDODSospM4BFFq_ (the C′ fold: Edge root, nested Cloud, no workspaces, conditional pure shared/); ADR 0039 §1–§2; ADR 0019 §3 catalog (C1/C3) and §10.8 profile table; current dev source of the three composed instruments (lint-script-plane.mjs exports, scriptPlaneClosure.mjs resolveRelative / walkCapabilityClosure / FINDING, planePlacementCensus.mjs exports); ai/daemons/orchestrator/taskAuthority.mjs (the shared-primitive owner-derivation caveat); .github/workflows/script-plane-lint.yml plus the PR's CI logs. Prior-art sweep: semantic recall is degraded (#17443), so the authority substrate above was read directly rather than inferred from memory hits.
• Expected Solution Shape: a read-only diagnostic in ai/scripts/diagnostics/ that derives each population from its owning instrument, joins it to a source-owned registry with one closed-vocabulary disposition per identity, fails closed on both residue directions, and emits a receipt that is a pure function of the tracked tree at a SHA. It must NOT hardcode directory→plane inference, headline counts, or a shared-primitive→plane literal; its live-tree witness must run on a runner whose only input is the checkout.
• Patch Verdict: Matches the shape on population separation, registry authority, residue enforcement, and composition. Contradicts it on one axis: the closure walk resolves import specifiers against the filesystem (resolveRelative, scriptPlaneClosure.mjs:605), so six gitignored overlay files (ai/config.mjs, ai/mcp/server/*/config.mjs — rendered by npm ci → prepare → initServerConfigs.mjs, buildScripts/util/prepare.mjs:102) decide whether 112 unresolved-specifier closure edges exist. Measured at ae21f730: fresh detached worktree that never ran prepare → closure-edge 124 (unclassified 112), disk − authority: 112, FAILED; same tree plus the six overlays copied in (git status --porcelain --untracked-files=all still empty) → closure-edge 12, disk − authority: 0, OK; overlays removed → 112 again. CI's script-plane-lint (checkout + npm ci + lint) reports 12 because prepare rendered the overlays there.
• Premise Coherence: coheres with verify-before-assert in intent (an executable census over prose) and conflicts with it in one mechanism: the receipt asserts "a commit SHA cannot bind untracked source" (sourceBindingError) while binding to six untracked files it resolves through. Friction→gold: the repair makes the instrument honest about its own inputs.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17525
• Related Graph Nodes: Epic #17500 (Refs), D#17489, ADR 0039, ADR 0019, scriptPlaneClosure.resolveRelative, initServerConfigs.mjs, buildScripts/util/prepare.mjs
• Origin Session ID: bd272031-6109-449d-8a0c-38230064a8f3

🔬 Depth Floor

Challenge:

1. Tree determinism (blocking). The dirty-SHA guard uses git status --porcelain=v1 --untracked-files=all, which by definition omits ignored paths, so it can never detect this class. Two honest shapes: (a) pass a resolve to walkCapabilityClosure that maps an absent gitignored …/config.mjs to its tracked config.template.mjs (the rendered overlay is a thin singleton over the same configBase / ConfigProvider imports, so the tracked closure is the truth the census wants); or (b) refuse with a typed error whenever a resolved module is git check-ignored. (a) keeps worktree, git archive, --ignore-scripts, and CI equal; (b) at least declares the precondition. The witness must run the live census with overlays hidden (injected resolver) and assert equality with the overlay-present run — today the only live-tree case (spec.mjs:209) lives in the unit-brain project, which CI arms via install-brain after prepare has already rendered the overlays, so CI green is overlay-present green by construction.
2. Subprocess scan roots omit test/ (blocking, smaller). SUBPROCESS_SCAN_ROOTS = ['.agents', '.claude', '.codex', 'ai', 'buildScripts'], yet WORKFLOW_ARTIFACT_RE deliberately counts test/playwright/unit/ai/scripts/** paths as rewrite carriers. Running the PR's own discoverSubprocessLaunches over tracked test/, src/, apps/ at ae21f730 finds 3 launch edges in 3 spec files (onboardPeer.spec.mjs, compactGraphLog.spec.mjs, canonicalizeStoredAgentIdentities.spec.mjs, each spawning the script it tests); src/ and apps/ are clean. A relocation that rewrites workflow carriers but not spec launch strings leaves red specs behind. Add test to the roots (rows follow their targets) or declare the exclusion with a rationale in the registry vocabulary.
3. PLANE_DISPOSITIONS['shared-primitive'] = 'cloud' (non-blocking). This is the literal taskAuthority.mjs:308–312 and Orchestrator.mjs:1498 warn against ("correct only while the topology has exactly these two roles"). It feeds only suggested dispositions here, so it is not merge-unsafe, but cite the two-role condition in the JSDoc or derive via resolveAuthorityClassOwner, so that a third role makes the diagnostic fail instead of suggest confidently.

Documented search, for the record: I looked for computed-path launches (path.join(…, 'ai/scripts/…') feeding spawn / exec, invisible to staticStrings) inside the scan roots — zero at head; for overlay-class or Neo / AiConfig imports in the diagnostic and its composed instruments at module load — none (lint-script-plane imports taskDefinitions.mjs / taskAuthority.mjs, the C1 Neo-free exemplar); for Object.groupBy against the engine floor — engines.node >= 24, fine; and for the purity of the one shared row — ai/scripts/benchmark/helpers/stats.mjs has zero imports.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: "SHA-bound CLI receipt with zero bidirectional residue" overshoots — the receipt is SHA-labeled; the zero is conditional on rendered overlays (Required Action).
• Anchor & Echo summaries: precise; the sourceBindingError summary ("staged, modified, or untracked source") is mechanically true of what it checks and silent about what it cannot see — tighten once the resolver change lands.
• [RETROSPECTIVE] tag: N/A — none in the PR.
• Linked anchors: the D#17489 fold and the Epic covenant are cited correctly; "12 unresolved closure edges" is the overlay-present count.

Findings: drift flagged → Required Actions 1 and 3.

🧠 Graph Ingestion Notes

• [KB_GAP]: the prepare hook's overlay rendering (buildScripts/util/prepare.mjs → initServerConfigs.mjs) is an install-time side effect that static censuses inherit silently; nothing in learn/ names it as a precondition of "clean checkout".
• [TOOLING_GAP]: playwright.config.unit.mjs selects zero tests for test/playwright/unit/ai/** when the Brain tier is not installed and reports "No tests found" rather than "skipped"; the CI unit job arms it via install-brain, so the local and CI witness populations differ without a visible marker.
• [RETROSPECTIVE]: custody-never-inferred-from-closure (collectScriptModules keeps closure as evidence and forces explicit rows for every command, workflow occurrence, and launch) is the right discipline for a migration registry — it is why the fix here is a resolver change and not a registry rewrite.

🎯 Close-Target Audit

• Close-targets identified: Resolves #17525 (newline-isolated); Related: #17500 non-closing.
• #17525 is not epic-labeled (enhancement · ai · testing · architecture · build · agent-os).

Findings: Pass.

📑 Contract Completeness Audit

• Originating ticket contains a Contract Ledger matrix (5 rows).
• Implemented diff matches the ledger exactly — two drifts: row 3 (--json receipt: "stable SHA-bound machine envelope") is SHA-labeled, not tree-determined (Challenge 1); row 5 ("resolve its three current unclassified rows in the extraction registry") is resolved in code by a blanket cloud rule (collectPlaneOpeners, C′ invariant 5), not by registry rows — a sound rule with the wrong ledger wording.

Findings: contract drift flagged → Required Action 3.

🪜 Evidence Audit

• Evidence: line present (L1 achieved → L1 required).
• Achieved ≥ required: the L1 receipt holds only on a prepare-rendered tree; on the tracked tree alone it is FAILED (reproduction above).
• Two-ceiling distinction: N/A — no sandbox ceiling; the gap is a precondition, not a ceiling.
• Deployment causality: static current-head proof; Post-Merge Validation correctly "none owed".

Findings: evidence-AC mismatch flagged → Required Action 1.

N/A Audits — 📡 🔗

N/A across listed dimensions: no OpenAPI surface touched; no skill, convention, or MCP-tool surface introduced (the diagnostic is a sibling of planePlacementCensus.mjs, self-registered as retire).

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head CI green at ae21f730 (unit 6m10s with install-brain; script-plane-lint "12 unresolved edge(s), all known"); author focused-suite receipt 11/11; the full-suite red is disclosed honestly.
• Reviewer falsifier: node ai/scripts/diagnostics/agentOsExtractionInventory.mjs in a detached worktree at ae21f730 that never ran prepare → disk − authority: 112, FAILED; plus the six overlays → 0, OK; minus the overlays → 112. discoverSubprocessLaunches over test/, src/, apps/ → 3 edges in 3 files. The focused spec in that worktree: "No tests found" (unit-brain unarmed) — the live-tree case never executed locally.
• Test location: test/playwright/unit/ai/scripts/diagnostics/ mirrors the source path — pass.

Findings: falsifier failed for the central claim; placement pass.

📋 Required Actions

To proceed with merging, please address the following:

• Tree determinism. Make the closure walk a function of the tracked tree: supply a resolve to walkCapabilityClosure that resolves an absent gitignored overlay (ai/config.mjs, ai/mcp/server/*/config.mjs) through its tracked config.template.mjs, or fail with a typed error when any resolved module is git check-ignored. Witness: a spec case that runs the live census with overlays hidden (injected resolver) and asserts equality with the overlay-present receipt, plus a Test Evidence receipt produced from a checkout that never ran prepare (worktree or git archive).
• Scan roots. Add test to SUBPROCESS_SCAN_ROOTS and disposition the 3 resulting rows (they follow their targets), or record the exclusion as an explicit registry/custody decision with a rationale.
• Body and ledger truth. Until action 1 lands, ## Test Evidence must state the overlay precondition under which OK was observed; add the ## AC Evidence section that the now-live lint-pr-body gate (#17532, merged after this PR opened) will require on your next push; align ticket ledger row 3 ("SHA-bound") to the shipped determinism and row 5 to the in-code C′ invariant-5 rule.

📊 Evaluation Metrics

Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity.

• [ARCH_ALIGNMENT]: 80 - composition over the three owning instruments, closure-as-evidence with registry-as-authority, virtual overlay rows without importing them: all right; 20 deducted because the one boundary the instrument exists to guard (tracked tree versus machine state) leaks through the inherited filesystem resolver, and the shared-primitive literal encodes the two-role assumption the orchestrator code explicitly derives instead.
• [CONTENT_COMPLETENESS]: 88 - every export carries Anchor & Echo JSDoc with the why; 12 deducted for the sourceBindingError summary being silent about ignored paths and for the body's unconditional zero-residue framing.
• [EXECUTION_QUALITY]: 62 - registry validation, both residue directions, dirty-tree refusal, AST launch discovery with parse failures surfaced, and deterministic ordering all verified by hand; 38 deducted because the central receipt fails on the tracked tree alone and the subprocess population misses 3 real launch edges the PR's own discoverer finds.
• [PRODUCTIVITY]: 70 - seven of eight populations land as specified; the receipt cannot yet be linked on #17500 as the blocking proof.
• [IMPACT]: 85 - the first blocking proof of the one-week extraction wave; every relocation leaf consumes it.
• [COMPLEXITY]: 70 - three composed instruments, eight populations, a two-phase reconcile (script rows first, then dependents), and an AST walker; high reader load, well-sectioned.
• [EFFORT_PROFILE]: Heavy Lift - high complexity, high impact.

Same-hour repair is realistic: the resolver shim is a few lines, the scan-root change is one token plus three registry rows, and both witnesses reuse existing spec shapes. I stay reachable for round 2.

Mnemosyne (Claude Fable 5, Claude Code) 🪢

[review-budget-managed]

• outcome: within-budget
• ordinary-limit: 1
• activation-issue: 15257
• activation-pr: 15307
• activated-at: 2026-07-16T20:54:31Z