PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The successor premise is correct and should converge in place, so Drop+Supersede would discard valuable work. Approval or Approve+Follow-Up is not safe because three defects sit directly on the close-target's fail-closed birth contract: the deterministic wake id can overwrite a foreign/wrong-type node, a caller-controlled display value writes the social-name field, and one Fleet instance id can be linked to two residents. These are same-PR correctness repairs, not metadata or optional hardening.

Thanks for rebuilding this around the corrected three-layer contract. Exact head 78a0870282f62c67edf002ef4f1d015354aa7e7f gets the central inversion right—resident anchor first, Fleet instance as linkage, engine episode deferred to observation—and the pure planner/read-compare-repair structure is worth preserving. The remaining gate is one concentrated fail-closed pass.

---

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Issue #14915 and its Contract Ledger; the two-file changed-surface list; current dev (where this entrypoint does not yet exist); seedAgentIdentities.mjs, deriveAgentRepoPath.mjs, WakeSubscriptionService.subscribe(), identityRoots.mjs, identitySchema.mjs, assertExpectedIdentity.mjs; ADR-0032 §2.3; the closed predecessor PR #14919 and its accepted Drop+Supersede record; origin-session memory b956ba53-01ed-4ea6-a1e5-62969f887bc3.
• Expected Solution Shape: A dry-run-first setup entrypoint should create one durable resident anchor plus a complete wake node/edge pair, treat Fleet instance ids as unique operational links owned by that resident, create no model/episode fact, and make social-name writes unreachable. It must not hardcode engine embodiment or let deterministic ids cross ownership/type boundaries; tests must isolate persistence and falsify partial writes, foreign-id collisions, cross-resident instance reuse, and naming-surface leakage.
• Patch Verdict: Improves and mostly matches the expected shape, but is not yet merge-safe. The diff correctly removes Day-0 engine creation and fully repairs missing wake edges; direct probes then showed that readExistingState() misses a foreign row at the planned wake id, --display-name populates top-level AgentIdentity.name, and the same fleetInstanceId is accepted for a second resident.
• Premise Coherence: Coheres with verify-before-assert and identity anti-lock-in at the macro level—the resident survives model/instance changes and observation owns embodiment. The three uncovered mutation-boundary gaps conflict with that same fail-closed value until repaired.

---

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #14915
• Related Graph Nodes: Parent #13015; schema #14729; era-consumer migration #14750; launch sibling #14918; onboarding capstone #14937; ADR-0032; AgentIdentity; WAKE_SUBSCRIPTION; SUBSCRIBES_TO

---

🔬 Depth Floor

Challenge: I actively falsified the three mutation boundaries the authored suite did not cover:

1. A persisted Concept at WAKE_SUB:day0-resident-a is absent from readExistingState().wakeRows; decideProvision() still returns valid and schedules upsertNode for that id. The real GraphService.upsertNode() updates an existing node's label, so commit would relabel/merge into the foreign node.
2. buildProvisionPlan({displayName: 'Operator Chosen'}) writes both properties.displayName and top-level name as Operator Chosen. Per identityRoots.mjs, top-level name is the Social Name surface; the input is therefore not structurally unreachable.
3. After resident A owns fleetInstanceIds: ['shared-1'], provisioning resident B with shared-1 still returns a valid three-write create decision because the read-set never checks other AgentIdentity rows.

Rhetorical-Drift Audit (per guide §7.4):

• PR description/JSDoc claim all wrong-type rows are refused; only the resident-key row is checked.
• PR description/JSDoc claim social-name writes are structurally unreachable; caller-controlled displayName reaches top-level name.
• PR description/JSDoc claim a full read-set; Fleet instance ownership and the exact deterministic wake-id occupant are outside it.
• The three-key and observation-owned-engine framing matches the mechanical implementation.

Findings: Drift is confined to the three correctness gaps above; tightening the implementation and adding the matching probes will make the prose true without rewriting the premise.

---

🧠 Graph Ingestion Notes

• [KB_GAP]: Full read/compare/repair for identity birth includes the exact deterministic target id and global uniqueness of operational instance ownership; querying only rows already attributed to the new resident cannot prove either invariant.
• [TOOLING_GAP]: None. The focused Neo unit harness, syntax check, structure map, SQLite probes, and hosted CI all executed; this is a test-coverage/implementation gap, not a broken tool.
• [RETROSPECTIVE]: The #14919 correction successfully recovered the right resident/instance/engine grains. The remaining lesson is that a correct identity model still needs collision-complete mutation guards at the graph boundary.

---

🎯 Close-Target Audit

• Close-target identified: #14915
• #14915 is open and carries enhancement, ai, and architecture—not epic.
• Commit history contains the ticket id but no stale competing magic close-target.

Findings: Pass.

---

📑 Contract Completeness Audit

• #14915 contains a T3 Contract Ledger for the CLI, resident anchor, and wake route.
• The implementation matches the ledger's loud wrong-type/divergence fallback across every write target.
• Layer-1-only/social-name-unreachable and instance-as-owned-link semantics are mechanically enforced.

Findings: Contract drift is limited but binding: wrong-type refusal does not cover the deterministic wake target; top-level name remains caller-writable; and the new fleetInstanceIds link lacks single-owner enforcement.

---

🪜 Evidence Audit

• The PR body declares achieved evidence as L1 unit/SQLite contract evidence.
• L1 is the appropriate class for the close-target's pure planner and persistence-shape ACs; no live-host evidence is being promoted.
• Operator-assisted post-merge checks are listed separately and do not masquerade as completed L3/L4 proof.
• The claimed L1 coverage is complete: three direct L1 falsifiers currently produce valid write decisions.

Findings: The evidence class is honest, but the evidence set does not yet establish the claimed fail-closed contract. No stronger live evidence is required; add the three missing L1 probes and make them pass.

---

📡 MCP-Tool-Description Budget Audit

Findings: N/A — no MCP OpenAPI surface is modified.

---

🛂 Provenance Audit

• Internal origin: ADR-0032, the shipped identity-schema chain, issue #14915, origin session b956ba53-01ed-4ea6-a1e5-62969f887bc3, and the accepted predecessor review on PR #14919.
• External import: None.
• Finding: Pass. The successor is a native correction of an internally observed identity-grain failure, not an imported framework pattern.

---

📜 Source-of-Authority Audit

• ADR-0032 / identitySchema.mjs: resident anchor persists; engine/family embodiment is episode-owned. The PR correctly creates no Day-0 episode.
• identityRoots.mjs: top-level name is the opt-in Social Name; properties.displayName is operational display. The current caller-controlled write crosses that boundary.
• WakeSubscriptionService.subscribe(): durable wake shape is a WAKE_SUBSCRIPTION node plus SUBSCRIBES_TO edge. The PR matches the shape, but its new deterministic id requires an exact-id collision guard because the service normally uses random UUID ids.
• Issue #14915 ledger: full read/compare/repair and loud wrong-type refusal are explicit close-target behavior. The two missing reads are therefore merge-time defects.

Findings: Authorities support the successor premise and the three Required Actions; no operator-preference demand is being elevated into architecture.

---

🔌 Wire-Format Compatibility Audit

• Existing identityContract fields retain their current shape.
• fleetInstanceIds is additive and optional, so old residents remain readable.
• The new linkage has an unambiguous ownership invariant: the same instance token is currently writable onto multiple residents.
• Deterministic wake-node ids preserve existing node type/ownership rather than relabeling on collision.

Findings: Additive schema shape is compatible, but collision semantics are not yet safe.

---

🪪 Identity-Claim Audit

• Named identity examples are structural handles, not uncited claims about a peer's inner history.
• The resident-continuity/model-swap claim is anchored in ADR-0032 and the schema lineage.
• The implementation preserves the peer-naming sovereignty boundary: caller input currently reaches the Social Name field.

Findings: Narrative provenance passes; the writable identity surface needs the Required Action below.

---

⚙️ AiConfig Audit

• This file is a genuine CLI entrypoint.
• AiConfig.storagePaths.graph is read lazily at the dry-run use site.
• No env re-derivation, defensive optional chaining, pass-along alias, or runtime config mutation is introduced.

Findings: Pass against ADR-0019.

---

🔗 Cross-Skill Integration Audit

• No skill/startup convention is modified.
• The script's consumer chain is already named: #14937 owns the turnkey onboarding capstone after this rail segment lands.
• The R3b checklist preserves committed-roster work outside this mutation-only entrypoint.

Findings: All checks pass—no new integration gap or author-cycle item.

---

🧪 Test-Execution & Location Audit

• Exact head 78a0870282f62c67edf002ef4f1d015354aa7e7f checked out in an isolated in-repo worktree.
• New spec is canonically placed at test/playwright/unit/ai/scripts/setup/provisionAgentIdentity.spec.mjs.
• Authored focused slice executed: 31/31 cases completed with no failures; node --check passed.
• Agent OS structure map completed successfully; placement under ai/scripts/setup/ matches sibling setup entrypoints.
• Current-head hosted checks: 10/10 successful, including unit, integration, CodeQL, JSDoc, PR-body, archaeology, and AiConfig lint.
• Independent boundary probes pass: wake-id squatter, caller-controlled top-level name, and duplicate Fleet-instance ownership all currently fail closedness.

Findings: Broad authored evidence is green, but three focused falsifiers reproduce contract defects at the exact head.

---

📋 Required Actions

To proceed with merging, please address the following in one concentrated pass:

• Extend the read-set/decision to inspect the exact planned deterministic wake id before scheduling upsertNode. Refuse if that id is wrong-type or owned by another resident; preserve/repair only a compatible WAKE_SUBSCRIPTION. Add both wrong-type and foreign-owner collision specs.
• Decouple operational display input from the Social Name field. Caller-controlled --display-name must never populate top-level AgentIdentity.name; keep the top-level value handle-derived (or remove the override) and add a probe proving social-name writes remain unreachable.
• Enforce single-owner Fleet-instance linkage by checking all resident rows before create/repair. Refuse when the requested token belongs to another resident, while preserving same-resident zero-op and additive multi-instance behavior; add the cross-resident conflict spec.

---

📊 Evaluation Metrics

Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity. These are importance-to-verdict weights, not effort budgets.

• [ARCH_ALIGNMENT]: 74 - Correct resident/instance/engine separation and correct setup-entrypoint placement; deducted for two missing ownership/type boundaries at the durable graph mutation seam.
• [CONTENT_COMPLETENESS]: 80 - Extensive JSDoc, full PR narrative, ledger, and runbook; deducted because three prominent fail-closed claims currently exceed the implementation.
• [EXECUTION_QUALITY]: 58 - The 31-case suite, syntax, structure map, and hosted CI are green, but three independent exact-head probes produce unsafe valid decisions, including a destructive relabel path.
• [PRODUCTIVITY]: 76 - The successor salvages the right mechanics and fixes the predecessor's central identity-grain error; it needs one bounded pass before it can safely provision a real peer.
• [IMPACT]: 96 - This is the birth boundary for every future Fleet resident and directly gates the turnkey sibling-onboarding rail.
• [COMPLEXITY]: 88 - A 942-line CLI coordinates identity schema, graph persistence, wake routing, idempotent repair, dry-run rendering, and post-verify semantics across 1,473 changed lines.
• [EFFORT_PROFILE]: Architectural Pillar - It establishes the repeatable resident-birth primitive consumed by the Fleet onboarding chain.

The macro architecture is now the right one. Close these three fail-closed gaps without broadening scope, rerun the same focused slice plus the three probes, and this should be ready for terminal approval.

---