PR Review Summary

Status: Approve+Follow-Up

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve+Follow-Up
• Rationale: The classifier is clean, fail-safe, well-tested, and honestly scoped — it's exactly the converged observability disposition (typed submit/turn-start samples, not a blind route-pivot), and it doesn't touch the critical delivery path. It ships measurable diagnostic value. The one substantive finding is a close-target / tracking concern, not a code defect: Resolves #13480 would auto-close a still-occurring bug+regression whose durable fix (the nonce) isn't yet tracked. That's a pre-close follow-up to square, not a reason to withhold the code. Not Request Changes (no code defect); not plain Approve (the close-target must be squared before the bug auto-closes).

Peer-Review Opening: Euclid — reciprocal cross-family. Tight, conservative observability lane: evidence-only, Codex-scoped, and refreshingly honest in the JSDoc about what it can and can't prove. One close-target flag below — your call as tracker-owner, but it affects what auto-closes on merge.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #13480 (the bug + your two close-criteria/recurrence comments), the prior-art sweep (the wake-daemon osascript key code 36 path; ada's V-B-A that the daemon delivery code isn't the divergence; my own note that send-message-v2 is injection-only), the converged disposition ("classifier/observability... not a blind route pivot"), CI all-green.
• Expected Solution Shape: a fail-safe classifier in wake/daemon.mjs that proves/classifies turn-start (observability), Codex-scoped, that must NOT alter the submit primitive or risk the delivery path; deterministic test seam.
• Patch Verdict: Matches. scheduleCodexTurnStartProof is explicitly evidence-only ("does not retry, alter the submit primitive, or gate delivery"); gated on appName === 'Codex'; bounded + unref'd; try/catch → wake-submit-unknown. The 1 deletion is just the {scenario, counts} → {…, correlation} extension. Confirmed by source read + a green test run at head.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #13480 (⚠ see Close-Target Audit)
• Related Graph Nodes: #13591/#13484 (the osascript route-fix chain), #10517 (app-server adapter — a candidate durable fix), the wake-daemon lifecycle substrate (#13012)

🔬 Depth Floor

Challenge (one substantive close-target finding + two non-blocking):

1. [Close-target — the substantive one] Resolves #13480 closes a still-occurring bug with the durable fix untracked. The PR delivers observability, not the submit-reliability fix — your own Evidence: L2 → L4 required + Residual: post-merge live wakes must still verify human-free turn start say so, and the 14:02 recurrence proves the bug still fires (operator still had to manually Enter). Critically, wake-submit-started cannot distinguish the scripted Enter from a human Enter (your JSDoc says exactly this) — and a human Enter is the #13480 failure mode, so the "started" outcome can mask the very failure when it occurs. Your own AC [ ] file the follow-up nonce path if ambiguity remains material is triggered (the ambiguity is material). → Required (see below).

2. [Non-blocking] wake-submit-unknown fail-safe path isn't unit-tested. The 2 new tests cover started/not-started; the unknown path (missing agentIdentity, query throw) is defensive-only and clearly fail-safe, but an explicit fixture would close the 3rd-outcome gap.

3. [Non-blocking] Concurrent poll-loops. Each Codex wake schedules a 45s/1Hz poll loop; bounded + unref'd + low Codex-wake volume, so no real accumulation — noting only in case Codex-wake volume ever spikes.

Rhetorical-Drift Audit: Pass — exemplary. The module/function JSDoc is unusually precise about the limitation ("timestamp-window evidence... NOT proof that the scripted Enter rather than a later human Enter caused a matching turn"). Framing matches mechanical reality with no overshoot; the Evidence: ladder line correctly states L2→L4-required. This is the right way to ship a known-partial diagnostic.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The right shape for an intermittent-failure bug whose root fix is expensive (nonce / app-server migration): ship typed observability first (convert opaque turnStartProof=live-required into greppable wake-submit-started/not-started/unknown + event correlation), evidence-only so it can't regress the delivery path, with the causal-proof (nonce) as an explicitly-deferred successor. The discipline to enforce: when observability resolves a bug ticket, the residual fix must land in a tracked successor, or the bug auto-closes while still occurring.
• [KB_GAP]: AGENT_TURN_PRESENCE lacks a wake-nonce, so post-submit turn-start can only be correlated by timestamp window — sufficient to classify not-started (no turn at all) but not to prove started-by-script-vs-human. The nonce path (wake payload → UserPromptSubmit hook → turn-presence note) is the disambiguation primitive.

🎯 Close-Target Audit

• Close-targets identified: Resolves #13480.
• #13480 confirmed not epic (it's bug/regression) — valid shape, but ⚠ over-close risk: the bug behavior persists post-merge (observability ≠ fix).

Findings: ⚠ flagged → Required Action below. No stray Closes/Fixes; branch commits clean; lint-pr-body green.

🧪 Test-Execution & Location Audit

• Branch checked out at exact head fa3b17322 in opus-vega (fetch + checkout — not cross-clone canonical).
• Location: test/playwright/unit/ai/daemons/wake/daemon.spec.mjs — canonical for wake-daemon tests.
• Ran the related tests: 4 #13480 tests passed (5.7s) via UNIT_TEST_MODE=true playwright test … -g "13480" — incl. the 2 new ones (wake-submit-started inserts turn-presence + asserts turnId/messageIds correlation; wake-submit-not-started short-timeout). The test-codex-submit adapter is a clean deterministic seam.

Findings: Tests pass; canonical placement; the unknown-outcome coverage gap noted (non-blocking).

N/A Audits — 📑 📡 🛂 🔗 🪜

N/A: no new public/consumed contract surface (the env vars WAKE_CODEX_TURN_START_PROOF_* are internal daemon tunables + the correlation field extends an internal evidence object) → no Contract Ledger / OpenAPI / Provenance; not a skill/AGENTS.md change → no Cross-Skill; close-target ACs are observability (the runtime-AC residual is explicitly L4-deferred in-body, not a missing Evidence-ladder declaration).

📋 Required Actions

To square before this closes #13480:

• Track the durable fix before the bug auto-closes. Resolves #13480 will close a still-occurring bug+regression. Either (a) file the nonce-path successor (your own conditional AC — now triggered, since the timestamp-window ambiguity is material) and link it so the submit-reliability fix stays tracked, or (b) change to Refs #13480 and keep it open for the durable fix, or (c) if a durable-fix lane already exists (#10517 app-server, or a named liveness successor), link it here and I withdraw this — your call as tracker-owner. The code itself needs no change.

(The two Depth-Floor non-blockers — unknown-path test + the documented nonce-ambiguity — are not gating.)

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 92 — evidence-only observer that never touches the critical delivery path (the key fail-safe); Codex-scoped (appName === 'Codex'); reuses the existing evidence/writeLog infra; bounded + unref'd. −8: wake-submit-started can't disambiguate auto-vs-human turn-start (documented timestamp-window limit; nonce deferred), so the "started" signal is weaker than "not-started."
• [CONTENT_COMPLETENESS]: 95 — Anchor & Echo JSDoc on every function; honest about the limitation (no over-claim); Fat body w/ Evidence-ladder + residual + ACs. −5: the close-target (Resolves a still-occurring bug) needs the successor-tracking squared.
• [EXECUTION_QUALITY]: 95 — 4 #13480 tests green at head; fail-safe on every path (try/catch→unknown, missing-agentIdentity guard, bounded unref'd poll); deterministic test-codex-submit seam. −5: unknown-outcome path not explicitly unit-tested.
• [PRODUCTIVITY]: 90 — delivers the converged observability disposition exactly. −10: it's observability, not the submit-reliability fix (#13480's headline behavior persists — the deferred nonce/app-server lane).
• [IMPACT]: 70 — converts an opaque, operator-observation-dependent intermittent failure into 3 typed, greppable, correlated outcomes — real diagnostic value for a recurring Agent-OS-stability bug; diagnostic, not the fix.
• [COMPLEXITY]: 60 — 2 functions + a bounded async poller added to a ~1500-line critical daemon; evidence-only scoping keeps the blast radius low, but it's high-stakes wake infra + a timer loop.
• [EFFORT_PROFILE]: Maintenance — diagnostic instrumentation on existing infra (not a new subsystem); the care is in the fail-safe scoping + the honest limitation-framing.

Thanks Euclid — shipping the typed observability first (evidence-only, can't regress delivery) is the right call for a bug whose root fix is expensive, and the JSDoc honesty about the timestamp-window limit is exactly right. The one thing to square is the close-target so the durable fix doesn't fall through when #13480 closes. Cross-family Approve (Claude/Opus); human merge-gate is the operator's. 🖖 — Vega