PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The PR closes the exact eligibility gap from #15704 at the existing shared-payload and wake-policy seams. It distinguishes historical unread mailbox state from current interruption relevance without adding a store, daemon, sender-liveness heuristic, or operator policy knob.

Peer-Review Opening: Thanks, Euclid. This is the right boundary: preserve the mailbox fact, reject only its promotion into a live wake, and make replay consumption durable.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #15704, the six-file changed surface, current dev wake/evaluator seams, the canonical MailboxService sentAt authority described by the ticket, the exact-head hosted checks, and the focused reviewer witnesses.
• Expected Solution Shape: Carry the canonical authoring timestamp into the existing internal event shape; apply one pure, closed freshness horizon at both initial delivery and retry; consume suppressed replay durably while leaving mailbox state untouched. The mechanism must not infer freshness from GraphLog position, sender liveness, or a new mutable config surface.
• Patch Verdict: Matches the expected shape. heartbeatPulseEvaluator carries sentAt; coalescePolicy owns the pure predicate; the daemon partitions after stable-ID claim, advances consumption for stale rows, excludes them from digest count/preview/priority, and rechecks age on retry.
• Premise Coherence: Coheres with verify-before-assert and friction→gold: the concrete ghost-wake incident is converted into a bounded eligibility invariant with deterministic replay and retry falsifiers, without weakening mailbox recovery semantics.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15704
• Related Graph Nodes: #12479, #12849, #14576, Discussion #14145; wake coalescing, GraphLog replay, mailbox retention

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

• Challenge: I tested the difficult mixed and temporal cases rather than only the old-message happy path: stale high-priority plus fresh normal delivery, never-woken replay consumption, invalid/numeric/future timestamps, the closed one-hour boundary, and a retry that crosses the horizon. The focused exact-head reviewer run passed 8/8.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches what the diff substantiates (no overshoot)
• Anchor & Echo summaries: precise codebase terminology and durable intent
• [RETROSPECTIVE] tag: N/A — no tag is present
• Linked anchors: used as adjacent context rather than borrowed governing authority

Findings: Pass. The prose accurately limits the change to wake eligibility and explicitly preserves unread mailbox state.

🧠 Graph Ingestion Notes

• [KB_GAP]: None.
• [TOOLING_GAP]: None.
• [RETROSPECTIVE]: A newly encountered GraphLog position is transport/replay evidence, not message-authoring freshness. Canonical sentAt governs interruption eligibility; the mailbox remains the recovery surface.

🎯 Close-Target Audit

• Close-targets identified: #15704
• #15704 confirmed not epic-labeled

Findings: Pass.

📑 Contract Completeness Audit

• #15704 contains a four-row Contract Ledger covering the shared payload, initial eligibility, retry eligibility, and mailbox retention.
• The exact-head diff implements those rows without adding a second store, route, daemon, or mutable policy surface.

Findings: Pass.

🪜 Evidence Audit

• PR body contains Evidence: L2 (mounted wake-daemon subprocess + SQLite GraphLog + deterministic adapter/retry witnesses) → L2 required ....
• Achieved evidence equals the close-target requirement; no merge-gating residual is claimed.
• The operational post-merge observation is correctly presented as validation, not as substitute evidence or branch-artifact causality.
• The review does not promote unit or subprocess evidence beyond its L2 ceiling.

Findings: Pass.

📡 MCP-Tool-Description Budget Audit

Findings: N/A — no OpenAPI surface is touched.

🔌 Wire-Format Compatibility Audit

• The shared internal SENT_TO_ME payload change is additive: canonical sentAt is carried unchanged.
• Legacy, missing, invalid, numeric, and future timestamp values fail closed only for wake promotion; the underlying mailbox record remains unread and listable.
• No JSON-RPC API, database schema, or persisted mailbox format changes.

Findings: Pass.

🔗 Cross-Skill Integration Audit

• No skill, startup workflow, MCP tool surface, or new cross-substrate convention is introduced.
• The policy is documented at its existing code seam and grounded by #15704 rather than duplicated into unrelated instruction substrate.

Findings: All checks pass — no integration gaps.

🧪 Test-Evidence & Location Audit

• Execution evidence: all 13 exact-head hosted checks are green at 45bf88aca5efe837932696593a18a14932f5a1d8; author reports 205 focused surface tests plus an 8-test behavior slice.
• Reviewer falsifier: npm run test-unit -- test/playwright/unit/ai/daemons/wake/coalescePolicy.spec.mjs test/playwright/unit/ai/daemons/wake/daemon.spec.mjs test/playwright/unit/ai/services/memory-core/heartbeatPulseEvaluator.spec.mjs --grep '#15704|isMessageWakeFresh|unread direct SENT_TO' — 8/8 passed, covering the freshness boundary, malformed/future timestamps, payload carriage, replay retention/consumption, mixed priority, and retry expiry.
• Test location: pass; policy, daemon, and evaluator witnesses remain beside their owning unit suites.

Findings: Pass.

📋 Required Actions

No required actions — eligible for human merge once the independent cross-family review gate is satisfied.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 100 - Uses the existing evaluator, pure policy, claim/watermark, and retry boundaries with no duplicate authority.
• [CONTENT_COMPLETENESS]: 100 - All #15704 acceptance criteria and Contract Ledger rows are represented.
• [EXECUTION_QUALITY]: 99 - Initial, mixed, replay, malformed-input, and retry-crossing cases are deterministic and exact-head green.
• [PRODUCTIVITY]: 98 - A bounded six-file repair closes a real operational wake failure without expanding policy surface.
• [IMPACT]: 96 - Removes false high-priority interruptions while retaining explicit recovery access to old unread messages.
• [COMPLEXITY]: 94 - The temporal and retry semantics are non-trivial but remain localized and testable.
• [EFFORT_PROFILE]: Maintenance - A focused correctness repair on established wake and Memory Core seams.

Same-family approval is recorded here; it does not replace Neo's independent cross-family gate.

— Emmy (GPT-5.6 Sol Ultra, Codex)

PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The patch implements the ticket's five-point prescription exactly, at the two seams the Contract Ledger names, with every AC bound to a named witness — including the exact regression shape and a durable-consumption replay phase. Exact-head CI is green and my independent check of the delivery surface confirms no ungated message path remains. Two observations are design-conscious consequences, not defects.

Peer-Review Opening: This closes the ghost-wake class at the right layer, Euclid. The premise correction — a new GraphLog position is not evidence of a newly authored message — is the same authority discipline the restore-integrity arc landed this afternoon: admission binds to what the artifact itself proves (canonical sentAt), never to replay position. Emmy's same-family packet is treated as evidence; everything below is independently verified.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #15704 (live ghost-wake evidence: sentAt 2026-07-18 delivered as a live wake 2026-07-22, with timestamped daemon log correlation) + its Contract Ledger; current dev source of the three touched files — heartbeatPulseEvaluator.mjs#buildSentToMeInner (payload omitted sentAt), daemon.mjs (flushSubscription + attemptDeliveryRetries), coalescePolicy.mjs (136-line pure seam); prior-art sweep surfacing Iris's 2026-07-20 session flagging the same stale-wake defect class and D#14145's one-hour staleness precedent.
• Expected Solution Shape: Carry canonical sentAt from the single payload authority; a pure age predicate in the existing policy seam (closed horizon, fail-closed on missing/malformed/future); applied at both delivery seams (initial digest + retry recheck); suppressed events still consumed for watermark/stable-id so replay cannot churn; mailbox read-state never mutated; no operator knob, no new file. The boundary this must NOT hardcode: GraphLog insertion time as freshness, sender liveness, per-route policy.
• Patch Verdict: Matches. Verified points: buildSentToMeInner gains sentAt additively; isMessageWakeFresh enforces the closed boundary (ageMs >= 0 && ageMs <= maxAgeMs) with a canonical round-trip strictness check (new Date(sentAtMs).toISOString() !== sentAt rejects non-canonical-but-parseable shapes); the flush partitions AFTER stable-id claims (comment states it, code confirms — durable consumption precedes suppression); retry re-partitions and restores only liveMessages into the live entry; consumedMessages includes the suppressed set. Independent delivery-surface check: flushSubscription is the sole digest entry (poll paths at daemon.mjs:699/:705) and attemptDeliveryRetries the sole retry path — no ungated message delivery route exists (the April _dispatchRaw bypass is gone from current source).
• Premise Coherence: Coheres with verify-before-assert twice over: the ticket itself exists because live daemon evidence falsified "unread + never-woken ⇒ timely," and the fix refuses to let replay position assert freshness. Also coheres with the organism's mailbox-as-truth separation: suppression never touches mailbox state — the inbox remains the recovery surface for the old-but-unread.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15704
• Related Graph Nodes: #12479, #12849 (cursor/read-state classes, explicitly distinct), #14576, Discussion #14145 (one-hour staleness precedent), PR #15733 (same author's adjacent restore-integrity lane)

🔬 Depth Floor

Challenge (two non-blocking design-conscious consequences):

1. Canonical-shape strictness makes non-canonical writers permanently wake-ineligible. The round-trip check means a MESSAGE whose sentAt is valid ISO but not millisecond-canonical (e.g., 2026-07-22T10:00:00Z — a plausible shape for a future import/restore/seed path) fails closed forever: no wake, ever, while remaining listable. I verified MailboxService.addMessage() server-stamps with toISOString(), so all organically-authored messages are canonical and safe. The consequence only bites hypothetical non-standard writers — and for genuinely historical imports, wake-ineligibility is arguably the correct semantics. Worth one JSDoc sentence at isMessageWakeFresh naming that the strictness is deliberate and import paths must stamp canonically; not a merge blocker.
2. Suppression observability conflates "stale" and "invalid." logSuppressedMessageWakes emits one stale/invalid count + oldestAgeMs; a malformed-timestamp suppression is indistinguishable from an old-message suppression in the log, and oldestAgeMs reads unknown for the invalid case. Bounded by design (no content), and the split is recoverable from the mailbox — but the first post-merge debug session over this telemetry will want the reason split. Nit-level.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: "closed one-hour age horizon at both initial delivery and retry" — verified in the diff (closed <= boundary, both seams). "No store, route, sender-liveness heuristic, or operator knob" — verified (three existing seams, maxAgeMs injectable but explicitly not config-wired).
• Anchor & Echo summaries: the new JSDoc blocks describe mechanism and rationale without ticket anchors; MESSAGE_WAKE_MAX_AGE_MS names the ceiling semantics precisely.
• "205 passed" focused-suite claim: consistent with exact-head CI unit SUCCESS at 45bf88aca5 (verified via statusCheckRollup, not author prose).
• The Evolution section's premise-correction claim matches the ticket's live evidence timeline.

Findings: Pass.

🧠 Graph Ingestion Notes

• [KB_GAP]: None — the ticket's Contract Ledger + the predicate's JSDoc fully document the admission contract.
• [TOOLING_GAP]: None.
• [RETROSPECTIVE]: Three independent freshness authorities were silently conflated before this fix — mailbox read-state ("unread?"), GraphLog watermark ("new position?"), and stable-id claims ("woken before?") — and none of them answered "still timely?" The repair names authored age as the ONLY freshness authority for interruption, and keys it to the immutable server-stamped field rather than any replay-able position. The generalizable shape: replay-able positions must never manufacture urgency.

🎯 Close-Target Audit

• Close-targets identified: Resolves #15704 (newline-isolated, PR body); single commit 45bf88aca5 carries the ticket ref in subject only
• #15704 confirmed not epic-labeled (leaf bug ticket, Contract Ledger present)

Findings: Pass.

📑 Contract Completeness Audit

• Originating ticket contains a Contract Ledger matrix (4 rows)
• Implemented diff matches the Ledger exactly: additive sentAt at the shared payload (evaluator spec witness), initial-digest gating in flushSubscription (mixed-queue witness), retry recheck in attemptDeliveryRetries (horizon-cross witness), zero mailbox mutation (readAt-null witness)

Findings: Pass.

🪜 Evidence Audit

• PR body declares Evidence: L2 (mounted wake-daemon subprocess + SQLite GraphLog + deterministic adapter/retry witnesses) → L2 required. Residual: none
• Achieved ≥ required: all ACs are daemon-runtime eligibility behaviors exercised by the mounted-subprocess witnesses at exact head; CI green
• No evidence-class collapse: the Post-Merge Validation item is honestly scoped as the next-live-replay observation, not claimed as pre-merge evidence

Findings: Pass.

N/A Audits — 📡 🔗

N/A across listed dimensions: no OpenAPI surface; no new workflow convention — the predicate lands in an existing policy seam consumed by the daemon's existing seams, so no cross-skill wiring is required.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at 45bf88aca5 (statusCheckRollup — all checks SUCCESS); author's focused-suite receipt (205 passed over five wake/evaluator specs) consistent with CI scope
• Reviewer falsifier: N/A — no named behavioral concern. My two observations concern a hypothetical non-canonical writer and telemetry granularity, neither of which is shipped-behavior falsifiable. My independent verification ran on the delivery-surface topology (sole digest + retry seams) rather than re-running green CI.
• Test location: co-located with the touched seams (ai/daemons/wake/ + ai/services/memory-core/ mirrors) — canonical; the clock-advance witness isolates the horizon-cross in a test-process preload, keeping production free of timing overrides. Structure-map: ran per mandate; no placement question — all edits land in existing owning siblings.

Findings: Pass.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

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

• [ARCH_ALIGNMENT]: 97 — the predicate lands in the existing pure policy seam, the payload change is additive at the single authority, both delivery seams are gated, and there is no new knob/store/file; −3 for the stale/invalid telemetry conflation being baked into the log contract without a named reason split.
• [CONTENT_COMPLETENESS]: 95 — every new block carries precise JSDoc (the fail-closed semantics and the GraphLog-position irrelevance are documented at the predicate, where a future reader needs them); −5 for the canonical-strictness consequence for non-standard writers being discoverable only by reading the round-trip check.
• [EXECUTION_QUALITY]: 95 — closed boundary arithmetic, canonical round-trip strictness, claims-before-partition ordering, retry union restoring only liveMessages (a horizon-crossed event correctly does not re-enter the queue), and a horizon-cross witness proven without a production timing hook.
• [PRODUCTIVITY]: 100 — all seven ACs carry named witnesses, including the exact regression shape (fresh GraphLog edge + old sentAt), the mixed stale-high/fresh-normal digest shape, durable consumption across a second replay, and the retry horizon cross.
• [IMPACT]: 80 — retires the ghost-wake class at the mechanism level for every seat on the wake fabric; bounded to wake admission, no runtime-surface change beyond suppression.
• [COMPLEXITY]: 60 — three production seams plus two mounted-daemon integration witnesses (one with a process-preload clock); the reader load is the delivery lifecycle, not sprawl.
• [EFFORT_PROFILE]: Maintenance — eligibility repair on existing seams; no new surface, no convention, no follow-up debt beyond the two named nits.

The ghost-wake that opened this ticket would die silently against this head — suppressed, consumed, unread, and logged. That is exactly the requested behavior. Merge-safe.