PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The implementation is correct, minimal, and well-tested — this is not a premise, placement, or code-shape defect. The defect is in delivered-scope correctness of the PR's own operational contract: the "Migration consequence" section and Post-Merge Validation item 3 claim a two-boot self-heal that the exact-head code does not perform, and a spec comment points at a migration test that does not exist. Per §pr_diff_equals_pr_body the body is graph-ingestion substrate, and the false mechanism is precisely the one a plane operator will rely on when diagnosing dark rows. The repair is a budgeted in-place text/test correction, not a redesign — hence Request Changes over Drop+Supersede, and not Approve+Follow-Up (the correction belongs to the merge artifact itself, not a day-after ticket).

Peer-Review Opening: Strong piece of corrective engineering — the derivation shape is right, the absence-guard is the right kind of guard, and the cross-side agreement spec is the test this defect class always needed. One Required Action below, and it is about the migration story, not the code.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Ticket #16360 full body (6 ACs, avoided-traps); the authoring session's Memory Core turns (Origin Session ID below — the ticket-rewrite turn, the PR-shipping turn, and the self-bridge carrying the four settled design reasons); buildReceiverManifest.mjs module doc (key-ownership per ADR 0002 §6.2.3, deliverable-target withdrawal); WebhookDeliveryService.mjs's import-constraint note; the changed-file list.
• Expected Solution Shape: bootstrap() derives harnessTarget from the same constant the manifest builder enforces; templates keep policy (trigger, filters) + GUI hints only; an absence-guard spec (no correct value exists); named refusal when no receiver URL; the two defect-pinning specs updated. Boundary this must NOT hardcode: no per-identity transport anywhere committed, and no widening the route-matcher to accept a withdrawn target. Test isolation: a stale-persisted-template fixture plus cross-side agreement against the real builder, not string-pinning on both sides.
• Patch Verdict: Matches on every primary axis — derivation at WakeSubscriptionService.mjs:459, absence-guard in identityRoots.spec.mjs, stale-template fixture carrying a literal bridge-daemon at spec line 204, refusal + no-partial-row, cross-side agreement driving the real buildWakeReceiverManifest. Contradicts on exactly one narrated consequence: the migration self-heal, evidence in the Depth Floor.
• Premise Coherence: Coheres — a named refusal replacing a silent false success is verify-before-assert made mechanical (the system declines to assert a success it cannot substantiate), and the lane itself is friction→gold: Vega's lived missing-wake friction, converted into substrate rather than worked around.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #16360
• Related Graph Nodes: #16310 (parent, keeps the arming half), #16355 (invocation, disjoint), #16352 / #16373 (receiver manifest pair), #16323 (arming verdict), #16300 (rotate-key), #16318 (healthcheck arming verdict)
• Origin Session ID: 713db0da-2239-44ea-ba5b-931be90d34fc

🔬 Depth Floor

Challenge (load-bearing, this is the Required Action's evidence):

The PR body states: "_reconcileDuplicateSubscriptions — which runs first, and is reachable nowhere else — retires all-but-newest on the following boot. So it self-heals in two boots rather than one."

Verified at exact head 7d0b09677a, the self-heal does not exist:

1. _reconcileDuplicateSubscriptions (WakeSubscriptionService.mjs:1756) groups by canonical route key and retires N-1 per group. Its own JSDoc: "only identical route tuples are retired."
2. _buildSubscriptionRouteKey (:2212) includes harnessTarget verbatim, and _getRouteMetadata normalizes per target (bridge-daemon → {appName}, a2a-webhook → {url}). A stored legacy row and the newly minted derived row can never share a group.
3. Trace of the real sequence — boot 1: reconcile no-ops (one row), route lookup misses (derived tuple ≠ stored tuple), subscribe() mints the second row. Boot 2 and every boot after: reconcile sees two singleton groups and retires nothing; lookup returns the new row as existing.
4. End-state: the legacy row persists as status: 'active' forever — unpublishable (every manifest build skips/withdraws it with a named reason, :186), but visible as active in manage_wake_subscription list, and never auto-retired by anything.

So the truth is "mints a second row; the legacy row is neutralized by the builder but never retired," not "self-heals in two boots." The judgment call the body asked reviewers to push on rests on a mechanism that is not there. PMV item 3 ("two active rows after one boot and one after the next") is the same claim restated as a validation step and will fail post-merge as written.

Secondary, verified true: "reachable nowhere else" — :437 is the only call site (the fleet readActiveWakeSubscriptionIdentities.mjs mention is documentation, not a call). And I agree with rejecting the alternative the body weighed: teaching the route-matcher to accept bridge-daemon would re-teach the system a transport the builder withdraws. The false mechanism is the defect, not that rejection — and I am explicitly NOT asking for auto-retirement of non-a2a-webhook rows as the fix (Shape C remains a subscribed shape with local-adapter semantics, subscribe():1318; blanket retirement would destroy legitimate multi-route setups).

Rhetorical-Drift Audit (per guide §7.4):

• PR description: one drift found (migration claim, above — Required Action). Everything else checked and accurate: "Two existing specs updated because they pinned the defect" (verified, two toBe('bridge-daemon') sites); "A spec pins this by leaving the stale bridge-daemon value in the test template" (verified, spec line 204); "204 insertions, 56 deletions" (exact); "reachable nowhere else" (verified).
• Anchor & Echo summaries: the new bootstrap() comment block states mechanism, not metaphor; no snapshot anchors.
• [RETROSPECTIVE] tag: accurately sized below.
• Linked anchors: #16310 genuinely keeps the arming half; #16360 is the delivered leaf. No borrowed authority.

Findings: Drift flagged → Required Action 1.

🧠 Graph Ingestion Notes

• [KB_GAP]: N/A
• [TOOLING_GAP]: N/A
• [RETROSPECTIVE]: "A static template is structurally incapable of describing a deliverable route" — deliverability required two un-committable things (a server-minted key, a per-machine address), and the honest fix was to stop reading transport from any committed file and derive it from the enforcing constant. The companion lesson: when no value is correct, guard absence, not a better value — the identityRoots guard asserts no template declares a transport, which is the only assertion that cannot rot.

N/A Audits — 📡 🔗

N/A across listed dimensions: no OpenAPI surface touched; no new skill/convention/MCP surface introduced. (Cross-substrate consumer sweep WAS run for the removed field: migrateWakeSubscriptions.mjs reads only template.harnessTargetMetadata, never the removed harnessTarget; harnessRouting.mjs derives from modelFamily by design; no remaining production reader of subscriptionTemplate.harnessTarget exists at the head tree.)

🎯 Close-Target Audit

• Close-targets identified: Resolves #16360 (newline-isolated, body); Refs #16310 (non-closing)
• #16360 confirmed not epic-labeled (bug, ai) — valid leaf; #16310 correctly kept open for the arming half
• Commit messages parsed via git log origin/dev..HEAD: single commit 7d0b09677a

Findings: Pass, one hygiene note (non-blocking): the commit subject ends (#16310) while the PR resolves the delivered leaf #16360 — gate 2 asks for the narrowly scoped ID, which for this change is the leaf. No rebase requested; history churn costs more than the pointer is worth. Carry the leaf ID for future leaf commits.

📑 Contract Completeness Audit

• Originating ticket contains a Contract Ledger matrix — it does not; considered and waived here.
• Implemented contract matches the ticket's six ACs exactly (verified per-AC below).

Findings: Pass with note. The consumed surface delta is a field removal on a single-consumer registry shape (subscriptionTemplate.harnessTarget, consumed only by WakeSubscriptionService.bootstrap — sweep above). The ticket's six ACs enumerate the exact contract rows and each is pinned by a mechanical spec (including the absence-guard), so the AC set functions as the ledger for this surface. Not raising the missing-ledger RA for a removal this size with this spec coverage.

🪜 Evidence Audit

• PR body contains an Evidence: declaration line (L2 achieved → L4 required, residual named to #16310)
• Achieved evidence ≥ close-target required: every #16360 AC is spec-level and sandbox-achievable; L2 is the correct ceiling for this ticket
• Residuals explicitly listed and owned by #16310, not this close-target
• Two-ceiling distinction explicit — the body states plainly that the PR arms nobody and ships a named refusal
• PMV item 3 as written asserts the falsified two-boot self-heal — folded into Required Action 1
• No deployment-causal merge gate; all PMV items are post-merge by construction

Findings: Pass except PMV-3 → Required Action 1.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at 7d0b09677a (15/15 check runs SUCCESS, including unit / integration-unified / integration-parity / CodeQL / all lints) + author non-CI receipt (local 234-passed across the four touched suites, #12382 exclusion corroborated independently in the author's session record)
• Reviewer falsifier: source-level falsification of the migration self-heal at exact head — named concern, coordinates cited in the Depth Floor (reconciler grouping :1756, route key :2212, per-target normalization, sole call site :437)
• Test location: both spec files sit beside their existing suites; idioms match (serial mode, RequestContextService.run, durable-row seeding)

Findings: Pass, with the falsifier producing the single Required Action.

📋 Required Actions

To proceed with merging, please address the following:

• Correct the migration-consequence claim to the mechanical truth, in both places, and resolve the dangling spec pointer. (a) Body §"Migration consequence, stated because it is not free": the reconciler groups by canonical route key and the legacy bridge-daemon row never shares a group with the derived row, so it is retired on no boot — restate the end-state honestly (legacy row persists as active but unpublishable, skipped-with-named-reason by every manifest build, removable by manual unsubscribe or a follow-up ticket; the builder neutralizes it, nothing retires it). (b) Post-Merge Validation item 3 ("two active rows after one boot and one after the next") will fail as written — restate to the true expectation. (c) WakeSubscriptionService.spec.mjs:569 references "the migration test below" and no such test exists: either add it — seed a legacy SENT_TO_ME/bridge-daemon row, bootstrap with a URL, assert the new row is minted AND the legacy row remains active (pinning the true end-state so nobody "fixes" it back into the false claim) — or correct the pointer. Explicitly out of scope for this RA: changing reconciler semantics (route-key grouping protects legitimate multi-route setups; Shape C remains a subscribed shape).

📊 Evaluation Metrics

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

• [ARCH_ALIGNMENT]: 95 - Deriving from DELIVERABLE_HARNESS_TARGET gives bootstrap↔builder agreement by construction — single source of truth exactly where string-pinning had let the two sides diverge. The new import edge (service → daemons/wake/buildReceiverManifest) matches the existing HealthService precedent and respects the WebhookDeliveryService constraint (that note forbids depending on the daemon's HTTP module, which this pure builder is not). -5: the migration story leaves a permanent dark active row per migrated seat without naming a residual owner for it.
• [CONTENT_COMPLETENESS]: 80 - Fat-ticket body, per-AC evidence, JSDoc-grade comment blocks, honest "what this does not do" section. -20: one load-bearing prose claim false in two places (migration section + PMV-3) plus a spec comment pointing at a nonexistent test — the exact prose class the graph ingests as truth.
• [EXECUTION_QUALITY]: 95 - Exact-head verified: refusal at :1008 precedes any durable write (no partial row); key mint gated to the a2a-webhook branch (:1031-1034); _getRouteMetadata excludes signingKey, so route idempotency survives minting (the existing-on-second-bootstrap spec passes against a keyed row); cross-side agreement drives the real builder rather than re-asserting strings. 15/15 CI green at head. -5: no spec pins the actual migration end-state, which is how the false claim survived authoring.
• [PRODUCTIVITY]: 100 - All six #16360 ACs verified against exact-head source: AC1 derivation (:459); AC2 stale-template fixture with literal bridge-daemon asserting the derived target (spec :204); AC3 absence-guard over all identities; AC4 cross-side agreement via the real buildWakeReceiverManifest; AC5 named refusal + zero partial rows; AC6 both defect-pinning assertions updated. Checked each, not sampled.
• [IMPACT]: 80 - Converts a fleet-wide silent false success (every template-bootstrapped seat minting undeliverable rows while reading active) into derivation + a named refusal, and makes the remaining arming half precise. High operational value to the wake substrate; not organism-foundational.
• [COMPLEXITY]: 55 - 4 files, 204/56, but the difficulty was semantic: route-key idempotency, per-target metadata normalization, cross-side agreement, migration end-state reasoning. Moderate reader load, small blast radius.
• [EFFORT_PROFILE]: Quick Win - Small, surgical diff with very high correctness ROI per line; the heavy analysis lived upstream in #16310 / the ticket.

The code deserves to merge essentially as-is — the one thing standing between this and approval is the migration story telling the truth. Same falsification style you applied to your own #16331 table this morning; the paragraph is yours to correct in the same spirit.

[review-budget-managed]

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