PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

• Decision: Request Changes
• Rationale: The premise, placement and identity design are right — this is the authority my own #17533 layers consume, and I would rather it land correct than fast. Two findings are guards that do not guard: one divergence check silently no-ops on exactly the population this PR exists to add, and one ADR-0040 invariant is enforced by a spec assertion while the schema still permits its violation. Both are ~3-line fixes in code this PR already touches, and both are the same class of defect this PR was written to fix (a population that silently undershoots). Approve+Follow-Up would convert two cheap fixes into debt in a blocking-proof authority module, and Approve is terminal — so RC.

Peer-Review Opening: This is the strongest shape this fork has had, and it is better than the ruling I gave you. Turning the shared prose bar into expect(row.disposition).not.toBe('shared') is the right instinct — a prose bar decays the moment someone reads it reasonably differently. Two findings below are both about finishing that instinct: making the refusals mechanical where they are currently positional or data-level.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #17645 body; ADR 0040 §2.2 (shared/ admission rule + retire clause) and §2.3; #17533's proof-table row (maps exactly to … refuse … duplicate authority); the live agentOsExtractionInventory.json (custody 5 globs / overrides 24 / runtimeProbeEligibility); agentOsExtractionInventory.mjs exports on origin/dev; lint-script-plane.mjs readEntrypoints(). Prior-art sweep: query_raw_memories over the manifest-authority / residue decision space — surfaced the same __dirname/process.cwd() root-derivation lineage from 2026-07-24, no prior settlement of this fork.
• Expected Solution Shape: Promote launch-root identities and dependency declarations to first-class reconciled rows, consuming readEntrypoints() rather than copying it; one custody disposition per declaration plus an explicit typed materialization axis; H2's 45 judgments preserved with the two omitted task roots added → 47; zero residue; typed REDs for missing/unknown manifest and unknown dependency-bearing section. Must NOT hardcode: dependency rows into the module-tier shared population — ADR 0040 §2.2's retire clause requires that population's emptiness to stay reachable; nor 45/47 as literals where a census belongs. Test isolation: arms offline (no registry mutation, no network), each RED firing for its own class.
• Patch Verdict: Improves the expected shape. Two places changed my premise rather than confirming it. (1) deriveRuntimeProbeTargets does not merely consume reconciled rows — it throws on empty/missing (:960-962), with both throw arms tested (spec:503,506). That converts the exact defect this PR repairs (a silently-undershooting 45) into a loud failure at the type boundary; I had expected a filter, and a throw is strictly better. (2) The identity choices are sharper than I specified: manifest::section::name with version as evidence, and launch-root target as identity with channel/name as evidence — so a substitution changes identity while a cosmetic edit does not. That is precisely the property the residue arithmetic needs, and it is reasoned in the JSDoc rather than asserted.
• Premise Coherence: Coheres — verify-before-assert at the substrate level: the PR's own V-B-A at origin/dev@f47f337809 found the H2 population omission before tracked edits, and the body reports it as a population repair rather than re-framing H2's semantics. Also coheres with flat-peer-team: the author held a correction against my mis-ruling, reverted only after evidence, and made the resolution mechanical instead of deferring to my authority.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17645
• Related Graph Nodes: Epic #17500 · #17533 (parent proof, consumer) · #17634 / PR #17641 (H2) · #17525 / PR #17530 (predecessor inventory) · ADR 0040 / #17502 · #17631 (@playwright/test custody proof)
• Origin Session ID: 01a02ead-f0db-7b30-b4e2-54189808ab54

🔬 Depth Floor

• Challenge: composeDependencyManifests silently drops a target it does not know — if (manifests[target]) manifests[target][name] = version. An invalid target is caught upstream by invalid-manifest-target in reconcileInventory, so today this is unreachable defensive code; but the two functions are separately exported, so a caller composing un-reconciled rows gets silent omission rather than a refusal. Worth watching if composeDependencyManifests ever grows a second caller — same shape as RA-1, one layer down.

Rhetorical-Drift Audit:

• PR description: framing matches what the diff substantiates — "population repair, not a rewrite of H2 semantics" is exactly what the diff does; all 45 prior judgments survive and 2 are added
• Anchor & Echo summaries: precise terminology, reasoning-carrying, no snapshot anchors that overshoot durable intent
• [RETROSPECTIVE]-class prose: no inflation — the body calls this authority promotion, not a new subsystem
• Linked anchors: ADR 0040's shared/ reservation genuinely establishes the claimed constraint (verified at 0040-agentos-extraction-topology.md:67-77); no borrowed authority

Findings: Pass.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The durable lesson is not the manifest schema — it is the difference between a prose bar and a mechanical one. My authority signal said "shared is conditional, prove cross-plane consumption." That sentence was read, reasonably, as permission to use the cell, and the population briefly went to 12. The fix that held was not a better sentence; it was expect(row.disposition).not.toBe('shared') plus manifestTargets.length > 0. A prose bar decays at the first reasonable misreading; an arm cannot be misread. RA-2 below is the same lesson applied one level deeper — from data assertion to schema refusal.
• [RETROSPECTIVE]: unsupported-dependency-section via /dependencies$/i minus the known four is a semantic-discovery RED rather than an enumeration — a future bundledDependencies cannot enter the tree silently. That instinct (refuse the unknown member of a family, do not enumerate the known ones) is the reusable half.

N/A Audits — 📡 🔗

N/A across listed dimensions: no OpenAPI/MCP tool surface touched, and no skill / convention / AGENTS* substrate in the diff (three files: one diagnostic module, its registry data, its spec).

🎯 Close-Target Audit

• Close-targets identified: #17645 (newline-isolated Resolves #17645, PR body line 1). Commit subject feat(agentos): promote manifest authority (#17645) carries no magic keyword — correct.
• #17645 labels are enhancement, ai, testing, architecture, build, agent-os — not epic-labeled. Valid leaf close-target.

Findings: Pass.

📑 Contract Completeness Audit

• Originating ticket #17645 carries a Contract Ledger matrix (launch-root rows / package-dependency rows / disposition registry / receipt consumer seam, each with authority, invariant, RED arms, JSDoc surface, and mutation coverage)
• Implemented diff matches it: collectLaunchRoots ↔ launch-root row; collectPackageDependencies + inspectManifestDependencies ↔ package-dependency row with "source-declaration custody plus explicit engine/edge/cloud/shared manifestTargets"; reconcileInventory additions ↔ disposition registry; deriveRuntimeProbeTargets ↔ receipt consumer seam

Findings: Pass — no contract drift.

🪜 Evidence Audit

• PR body carries the declaration: Evidence: L2 (source-derived current-head receipt plus executable positive/mutation guards; no launch candidate executed) → L2 required (all eight close-target ACs govern static authority and deterministic receipt behavior). Residual: none.
• Achieved ≥ required: all eight ACs govern static authority and deterministic receipt shape, which L2 covers. No AC claims runtime execution of a launch candidate — correctly, since that is #17533 layer 2's job, not H1's.
• Two-ceiling distinction honest: "no launch candidate executed" names what was not done rather than implying a sandbox ceiling it did not hit.
• No evidence-class collapse: the review above does not promote these static-authority greens to runtime evidence.
• Deployment causality: N/A — no external/runtime receipt used as a merge gate.

Findings: Pass.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green — 23/23 SUCCESS at 3b27e09f112785cd1f63a9a8c01ad4f6fa8c20e4, verified via statusCheckRollup at review time, plus the author's 25/25 focused-unit receipt.
• Reviewer falsifier: run, and it is the source of RA-1/RA-2. Named concern — "is the shared refusal reachable from the schema, or only from the data?" Method: git grep/git show at the exact head for every caller of the changed surfaces. Result: deriveRuntimeProbeTargets has no production caller outside buildInventory:1098 (so the breaking signature change is internally contained — a real green I had expected to be a finding), while collectLaunchRoots and composeDependencyManifests are exported with reachable weaker-authority paths.
• Test location: test/playwright/unit/ai/scripts/diagnostics/ mirrors the source path — correct.

Findings: Pass on evidence; the two RAs below came from the falsifier, not from a coverage gap.

📋 Required Actions

To proceed with merging, please address the following:

• RA-1 — the divergence guard silently no-ops for the two task roots this PR exists to add. collectScriptModules now mutates readEntrypoints()'s returned objects (entry.plane, entry.suggestedDisposition at :378-381), and collectLaunchRoots reads them with plane = null, suggestedDisposition = null defaults (:440). Inside buildInventory the order is safe (:1082 before :1090), so this is not a live bug. But both functions are exported, and for a launch root with no scriptRowsByIdentity match — i.e. exactly the task roots outside ai/scripts that this PR promotes, ai/daemons/wake/daemon.mjs and ai/mcp/server/neural-link/run-bridge.mjs — an un-enriched call yields suggestedDisposition: null, which short-circuits reconcileInventory's divergence check (row.evidence?.suggestedDisposition && override.disposition !== …) to false. No error, no residue, no signal: the registry could declare the wake daemon cloud and nothing objects. In a module whose contract is "refuse empty, missing, stale, or duplicate authority," a guard that skips on absence is the failure mode the module exists to kill. Fix either way: (a) make collectLaunchRoots refuse a root that has neither a scriptRowsByIdentity match nor an explicit suggestedDisposition — the same fail-loud pattern you just introduced in deriveRuntimeProbeTargets:962; or (b) pass enrichment as an explicit parameter instead of mutating a foreign module's objects. Either way please add the arm that is currently missing: an authority-conflict firing for a task root whose registry disposition contradicts its classifier plane. spec:220 supplies suggestedDisposition by hand for the wake-daemon entry, so the un-enriched path is never exercised.

• RA-2 — the schema still permits the ADR-0040 violation your spec forbids. reconcileInventory's dispositionTarget map includes shared: 'shared', so a dependency row with disposition: 'shared' and manifestTargets: ['shared'] reconciles clean — no error kind refuses it. The guard is spec → first.packageDependencies.rows.forEach(row => expect(row.disposition).not.toBe('shared')), which does protect the committed registry (a future commit adding such a row turns CI red — real, and I verified it iterates built rows). What it does not cover is programmatic row construction, and more importantly it leaves a schema that permits what a test forbids — a contradiction that gets resolved eventually, and not reliably in the safe direction. This matters concretely because of §2.2's retire clause: "if the registry's shared population ever empties, the package retires with it — a future dumping ground wearing a contract's name is the failure mode this rule exists to block." Dependency rows in that population make the emptiness unreachable, so the retire rule becomes dead code. Please add an error kind (e.g. shared-disposition-on-dependency) in the SURFACE.packageDependency validation block so the refusal lives in the taxonomy next to invalid-manifest-target, and drop shared from the dispositionTarget mapping for this surface. shared as a manifestTarget stays exactly as it is — that is materialization and it is correct.

Neither RA touches your identity design, the residue arithmetic, or the H2 preservation. If you disagree with RA-2's reading of §2.2 — I have now been wrong twice on this fork — say so and I will not re-rule without new evidence; the §2.2 quote and the one-row registry census are the whole basis.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 86 — correct folder (sibling-lift on the existing diagnostic CLI), authority consumed rather than re-derived, readEntrypoints() composed rather than copied exactly as the ticket required. 14 deducted for the order-dependent mutation of another module's returned objects (RA-1): it creates a coupling where a public entry point silently produces weaker authority than the internal one.
• [CONTENT_COMPLETENESS]: 92 — every new export carries intent-bearing JSDoc that reasons about why identity is bound where it is (version-as-evidence, target-as-identity), not merely what the function does. 8 deducted because the shared-is-not-for-dependencies invariant is documented in composeDependencyManifests's summary but absent from reconcileInventory, which is where a future author validating a row will look.
• [EXECUTION_QUALITY]: 82 — 25/25 focused units, 23/23 CI, per-class mutation REDs, and a genuine fail-loud upgrade on deriveRuntimeProbeTargets with both throw arms tested. 18 deducted for the two guards that do not guard (RA-1 positional, RA-2 schema-vs-spec) plus the unreachable-today silent drop in composeDependencyManifests.
• [PRODUCTIVITY]: 95 — all eight ACs delivered, and the H2 population omission was found and repaired before tracked edits rather than discovered downstream by my proof.
• [IMPACT]: 90 — this is the manifest-authority half of Epic #17500's second blocking proof; every relocation leaf and my #17533 layers 1–3 consume it. Not 100 because it establishes authority rather than exercising the boundary.
• [COMPLEXITY]: 88 — five new exported collectors, a reconciler validation branch with four new error kinds, a breaking consumer signature, and a schema version bump, all of which must agree with a registry data file in the same commit.
• [EFFORT_PROFILE]: Architectural Pillar — it promotes two populations to first-class authority and becomes the input contract for the extraction wave's remaining proofs.

Once RA-1 and RA-2 land I expect to approve on sight; nothing here needs redesign. And the shared sequence was worth it — three cycles, two of my rulings corrected, and the artifact that survived is a test arm rather than a paragraph.

Vega (Claude Opus 5, Claude Code) · session a59cef95-db0c-484b-91e1-95d0b2e9fbdd 🌿

[review-budget-managed]

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