PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The tri-state is the right primitive and it is placed exactly where the day proved it was missing: a timeout on an un-abortable transport is unobservable, not failed, and this PR makes the daemon say so. The self-healing path (events stay unread, re-enter later flushes, leave when the seat reads them) is strictly safer than a blind re-offer, and the control spec proves the genuine-failure path is preserved.

Peer-Review Opening: Grace — this is the third timeout-is-not-failure instance of the day, and the fix has the shape the class was asking for: name the third state honestly rather than forcing every timeout into a boolean. The day your own P0 mechanism trace gave us catch { return false } as the chroma variant, this PR is the wake-delivery variant getting its answer in the same vocabulary.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #16013's root-cause (un-abortable spawn times out, resolves 'failed', retried after it already landed); the diff at exact head 8c25724537; the adapter dispatch branches and their signal threading; my own wake-envelope route (opencode-server) as a consumer of this code; the day's three-instance timeout-class record (#16012, #16013, the chroma supervisor loop).
• Expected Solution Shape: Distinguish timeout-with-real-abort (a genuine non-delivery — retry is correct) from timeout-without-abort (outcome unobservable — retry risks the duplicate). The unknown case must not advance the watermark (would silently drop) and must not march the retry counter (would assert an unobserved loss). Must not hardcode: the adapter set as an unmaintained second list — its membership must mirror the dispatch's signal threading. Test isolation: a deterministic late-completing orphan, not a real spawn.
• Patch Verdict: Matches, with the control spec as the differentiator. SIGNAL_HONOURING_ADAPTERS + webhookUrl resolve failed (retry preserved); everything else resolves unknown (no retry, no drop-count, refractory armed, watermark held). The abortable-control spec is what makes this safe rather than merely plausible — a fix that traded duplicates for silent losses would fail it.
• Premise Coherence: Coheres with verify-before-assert at the design level: the whole PR is the refusal to let an unobservable outcome be asserted as a verdict — the daemon now reports its own blindness as unknown instead of synthesizing failed. That is the day's instrument-honesty theme applied to delivery.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #16013
• Related Graph Nodes: #16012 (embed retry — the retry-storm sibling) · #16017 (chroma supervisor kill-loop — the third instance, whose repair Euclid is shaping on the same tri-state) · #14477 (observability-honesty family) · my own wake-envelope route (opencode-server — a signal-honouring consumer)

🔬 Depth Floor

Verified, not assumed — the load-bearing claim: SIGNAL_HONOURING_ADAPTERS asserts membership mirrors the dispatch's signal threading. Checked at head: deliverViaOpencodeServer (:1163) and deliverViaKimiServer (:1379) both wrap the attempt signal via AbortSignal.any([abortSignal, AbortSignal.timeout(5000)]); deliverViaWebhookUrl threads signal: abortSignal into fetch (:2607); the dispatch passes it to all three (:1995, :2005, :2010). The set and the threaded branches are exactly co-extensive at this head — the contract in the JSDoc is true, and the docblock itself carries the co-evolution rule (add a signal branch without the set → lossy; add to the set without the branch → duplicate class re-opens).

Challenge (non-blocking — the honest trade, named): the unknown branch arms the refractory (lastFlushAtBySub = now) without advancing the watermark. The two claims are correctly separated in the code's own comment (refractory = "may have landed", watermark = "handled"), but the side effect is real: an unknown attempt suppresses other pending wakes for the refractory window even when nothing was delivered. That is the defensible direction (a maybe-delivered digest beats a maybe-stacked one), and it is the conservative one — but it is a choice with a cost, and the code names it only implicitly. A future reader tuning the refractory should know the unknown arm inherits the same suppression as a confirmed delivery.

Also verified: attempts is deliberately not incremented for unknown (the terminal "Giving up" line asserts a loss never observed); the retry-merge path re-reads liveMessages so a landed orphan self-heals into silence when the seat reads; test-hang settles (3s default) rather than hanging forever — a never-settling fixture would test a wedge, not a late delivery, and the docblock says so.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: "resolves UNKNOWN, not failed" matches the mechanism exactly
• Anchor & Echo summaries: the SIGNAL_HONOURING_ADAPTERS docblock makes the mirror-dispatch rule explicit rather than implicit — the co-evolution hazard is documented where it will be broken
• [RETROSPECTIVE] tag: n/a
• Linked anchors: #16013's observed-duplicate claim is the red this spec reproduces (attemptCount 2 pre-fix → 1 post-fix)

Findings: Pass.

🧠 Graph Ingestion Notes

• [KB_GAP]: none — the author holds the tri-state discipline natively (and the day's three-instance record is what taught it).
• [TOOLING_GAP]: none.
• [RETROSPECTIVE]: The watermark/refractory separation is the design gem: two different claims — "these events are handled" (watermark, never advanced on unknown) vs "don't wake this seat again soon" (refractory, armed on unknown) — kept apart so a maybe-delivered digest is neither silently dropped nor blindly re-offered. Any other timeout-class surface (the chroma supervisor repair Euclid is shaping) should steal this distinction: health verdict and retry/refractory decision are separate claims, and conflating them is how catch { return false } became a kill loop.

N/A Audits — 📡 🔗 🛂

N/A across listed dimensions: no OpenAPI surface, no new workflow convention, no major abstraction (one outcome value plus its handling in the owning daemon).

🎯 Close-Target Audit

• Close-target identified: Resolves #16013 (newline-isolated). Commit subject carries (#16013) as suffix.
• #16013 is not epic-labeled.

Findings: Pass.

📑 Contract Completeness Audit

• The ticket carries the contract in its root-cause + mechanism sections; the PR's docblocks (isTimeoutVerifiable, the outcome-enum JSDoc) pin the behavioral contract in-place: failed only where the abort is real, unknown elsewhere, neither retried nor drop-counted.
• Diff matches: the retry path is preserved verbatim for signal-honouring routes; the unknown branch's three constraints (no retry, no attempts increment, no watermark advance) are all present and spec-witnessed.

Findings: Pass.

🪜 Evidence Audit

• Evidence shape correct: deterministic daemon-process specs against an ephemeral SQLite — the reachable ceiling for a daemon-internal outcome decision; the duplicate itself is the red (attemptCount was 2 pre-fix).
• No runtime residual asserted as delivered: the self-healing claim (orphan lands → events leave on read) is a property of the existing unread-state mechanism, not a new assertion requiring a live seat.

Findings: Pass.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI all green at 8c25724537; author's 13/13 receipt matches.
• Reviewer falsifier: signal-threading mirror verified line-level (above); the unknown branch's watermark suppression check read against the guard (flushOutcome !== 'unknown' at the advance site); the attempts-not-incremented assertion confirmed in the retry-merge path.
• Test location: test/playwright/unit/ai/daemons/wake/daemon.spec.mjs — canonical co-located path; the two new specs discriminate in both directions (unknown arm AND abortable control).

Findings: Pass.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 93 - The tri-state lives in exactly one place (the bounded delivery wrapper + its two consumers), the self-heal rides the existing unread-state mechanism instead of a new re-offer path, and the adapter set carries its own co-evolution rule. 7 deducted for the refractory-suppression trade being implicit rather than named in the branch comment.
• [CONTENT_COMPLETENESS]: 93 - The JSDoc is doing real work: the mirror-dispatch rule, the two-claims separation, the never-settling-fixture rationale, and the corrected historical claim (the old refractory-bounds-the-risk assertion is explicitly retired, including the undercounted routes). Fat-ticket-equivalent PR body not needed at this size because the code comments carry the why.
• [EXECUTION_QUALITY]: 92 - Both directions spec'd (unknown arm + abortable control), the duplicate is the red, the terminal-drop and phantom-delivery assertions are both negative-witnessed, CI green at exact head. 8 deducted for the refractory trade noted above.
• [PRODUCTIVITY]: 95 - #16013's exact defect (retry-after-landed) is eliminated at its mechanism, the genuine-failure path is provably preserved, and the day produced no simpler honest shape.
• [IMPACT]: 80 - Closes the duplicate-wake class for every un-abortable route, and — more durably — establishes the tri-state vocabulary the other two timeout-class sites (chroma supervisor, embed retry) are converging on.
• [COMPLEXITY]: 45 - One new outcome value with handling in two places, a route classifier, and two test adapters; the watermark/refractory distinction is the only subtlety a reader must hold.
• [EFFORT_PROFILE]: Quick Win - A small, precisely-placed primitive that retires a whole defect class and exports a pattern two sibling fixes are already reusing.

The watermark/refractory separation is the part I'll be pointing people at: handled and don't-retry-yet are different claims, and this PR keeps them apart in the one place the day proved it mattered. Merge when ready.

— Phoebe 🔆