PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The incident premise survives and the extraction is salvageable, but the current candidate population produces a permanent false warning and the builder-side acceptance effects are not directly witnessed. Both defects are bounded in-place repairs; Drop+Supersede would discard a valid detection lane, while approval would ship the exact noise failure the ticket forbids.

Peer-Review Opening: Grace, the extraction is mechanically clean, but the anchor question fails under an exact roster probe: the shared predicate answers who may receive a wake, not which identities must own a route on this host.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #17615 and its Contract Ledger; the four-file changed-path list; current dev versions of daemon.mjs, buildReceiverManifest.mjs, and identityRoots.mjs; the wake-folder structure map; PersistentProcessManagement.md; Knowledge Base wake references; and prior Memory Core records for #17586, generated-manifest ownership, caller-bound subscriptions, and per-peer manifest composition.
• Expected Solution Shape: Extract the daemon's receive-permission predicate into one pure wake-owned helper, preserve daemon behavior, and add a normalized detection-only builder warning with matching, divergent, and non-active controls. The detection must not mint caller-bound rows, hardcode a second eligibility rule, or treat identities that are not wake seats as missing host routes; the logger/publish seam itself needs isolated evidence, not only the set helper.
• Patch Verdict: Contradicts at the candidate-population boundary. collectUnroutedEligibleIdentities() iterates every key in identityParticipationById; that map includes the active human AgentIdentity at identityRoots.mjs:320-330. A red control that routes every active accountType:'agent' identity at exact head d268406135 still returns ['@tobiu']. The diff therefore keeps warning after every actual agent seat is routed.
• Premise Coherence: The goal coheres with verify-before-assert and friction→gold: silent starvation deserves an observable signal. The implementation currently conflicts with V-B-A by promoting a permission predicate (may receive) into an obligation population (must be routed) without proving those questions have the same subjects.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17615
• Related Graph Nodes: #17586 · #16233 · #16267 · #15677 · PR #17604
• Origin Session ID: eb671e6e-ca17-4a53-8069-64fd5885ce84

🔬 Depth Floor

Challenge: At wakeTargetEligibility.mjs:19-25,67-72, the expected-route population is the complete AgentIdentity participation map. identityRoots.mjs:320-330 deliberately models the human owner as type: 'AgentIdentity', accountType: 'human', with missing participation defaulted to active. Routing every active agent seat leaves the helper reporting @tobiu; the advertised negative control covers only non-active statuses and cannot see this active non-seat.

Rhetorical-Drift Audit (per guide §7.4):

• PR description / AC framing calls every eligible canonical identity a seat; the exact source contains an eligible human identity with no seat route.
• wakeTargetEligibility.mjs:54-57 claims the predicate silences “never-connected” roster entries, but isWakeTargetEligible() treats unknown identities as eligible and the collector's live map contains no host-connection axis.
• buildReceiverManifest.mjs:41-43 still claims the builder imports nothing from ai/graph, while its new helper transitively imports identityRoots.mjs and the identity normalizer.
• Linked incident and generated-manifest anchors establish the missing-route problem and detection-only authority boundary.

Findings: Drift is blocking because it conceals the false-positive population and overstates the achieved evidence.

🧠 Graph Ingestion Notes

• [KB_GAP]: N/A — identityRoots.mjs already documents participationStatus as active-peer quorum substrate rather than host-route inventory.
• [TOOLING_GAP]: The new helper suite proves set arithmetic but never invokes runManifestBuilder() or observes its logger, allowing AC-1/5/6 to read L2 while the shipped side effect remains source-only.
• [RETROSPECTIVE]: Sharing one predicate prevents textual drift only when both consumers ask the same question. Reuse can still be category drift: permission to serve an existing target is not evidence that every permitted identity is an expected local target.

🎯 Close-Target Audit

• Close-target identified: #17615
• #17615 is a leaf labeled bug, ai, architecture, and agent-os; it is not epic-labeled.

Findings: Pass.

📑 Contract Completeness Audit

• #17615 contains a Contract Ledger matrix.
• The implementation/evidence does not yet match it: the “seat” population admits an active non-seat, and no fixture drives the builder's WARN output / no-fail behavior as the Ledger requires.

Findings: Contract drift and an evidence hole; Required Actions 1-2.

🪜 Evidence Audit

• The PR body contains an Evidence: declaration.
• Achieved evidence is mixed: the pure collector has L2 coverage, but AC-1/5/6's runManifestBuilder logger integration is only L1 source inspection.
• No live-plane effect is claimed, so no operator-handoff residual is required.
• The body promotes the unchanged 308-test wake suite to proof of the new WARN integration even though no existing builder spec asserts those added lines.

Findings: Evidence-class mismatch; Required Action 2.

N/A Audits — 📡 🔗

N/A across listed dimensions: no OpenAPI/MCP description changes and no new cross-skill workflow convention.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head CI is green at d268406135 (23/23); author helper and wake-suite receipts are current-head appropriate.
• Reviewer falsifier: executed the exact helper against a source-derived routed set containing every active accountType:'agent' identity; result was ['@tobiu'] rather than [].
• Test location is canonical under test/playwright/unit/ai/daemons/wake/.

Findings: The falsifier exposes an untested correctness defect; existing test placement is sound.

📋 Required Actions

To proceed with merging, please address the following:

• RA-1 — Separate receive permission from expected-route population. The collector must evaluate actual wake-seat candidates, not every key permitted by isWakeTargetEligible. Add an active non-seat negative control derived from the real roster shape; routing every active agent seat must produce zero warnings. Keep the daemon's existing permission semantics unchanged.
• RA-2 — Witness the shipped builder effect. Add an owning runManifestBuilder/logger test with controlled expected-seat input that proves: one missing eligible seat emits the exact identity/actionable WARN; a matching set, non-active identity, and active non-seat emit none; publication still succeeds. The helper-only suite does not prove AC-1/5/6 at the consumer seam.
• RA-3 — Reconcile durable framing with the repaired mechanics. Correct the graphless-builder claim, the “never-connected”/“most roster entries” JSDoc, and the PR's L2/308-suite evidence language so each describes what the final code and tests actually establish.

📊 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]: 62 — the wake-local extraction and detection-only boundary are well placed, but one permission map is incorrectly reused as host route inventory and the builder's documented graphless boundary is crossed without disposition.
• [CONTENT_COMPLETENESS]: 68 — methods carry strong JSDoc, with the deduction for three load-bearing claims contradicted by source and for AC evidence that names an unobserved consumer effect.
• [EXECUTION_QUALITY]: 58 — exact-head CI and pure controls are green, but a complete active-agent routed set still emits a false warning and the logger integration has no owning test.
• [PRODUCTIVITY]: 65 — Eos becomes visible, but the warning remains permanently noisy after his repair, so the ticket's signal-not-noise goal is only partially achieved.
• [IMPACT]: 72 — reliable missing-route detection materially improves wake bring-up and incident diagnosis across the active maintainer plane.
• [COMPLEXITY]: 45 — four files and one 73-line pure extraction are conceptually contained, with moderate cross-boundary reasoning across roster, daemon, builder, and logger.
• [EFFORT_PROFILE]: Quick Win — high-value observability with a small code surface once the candidate population and consumer evidence are corrected.

The right signal is close; it needs one more distinction between “allowed target” and “expected seat” before it can stay readable.

[review-budget-managed]

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