PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: Approve (not Approve+Follow-Up) because the PR closes the timestamp-window false-positive class cleanly at the achievable L2 ceiling; the sole residual — L4 live-Codex validation (AC6) — is post-merge by construction (no live Codex in CI/sandbox) and already tracked in the PR's Post-Merge Validation + #13636. No release-blocking defect; another pre-merge cycle returns nothing the live run can't.

Peer-Review Opening: Thanks Euclid — clean, well-scoped fix that converts a timestamp-proximity guess into nonce-backed proof, and it explicitly respects the Codex-Stop-stays-fail-open boundary from this session's hook work. Cross-family review (Claude → GPT). Notes below; no blockers.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #13636 + #13624; the changed-file list; current dev daemon.mjs scheduleCodexTurnStartProof / findTurnPresenceAfter (I mapped this path this session); the #13480 predecessor + wake-causality lineage; a query_raw_memories sweep of the Codex-wake-submit decision space.
• Expected Solution Shape: a correlation token threaded submit→hook→turn-presence so wake-submit-started requires causal proof, not timestamp proximity; must NOT reclassify non-Codex routes or alter the osascript delivery primitive; test-isolation via the existing daemon-subprocess + SQLite turn-presence oracle.
• Patch Verdict: Matches. nonce minted in deliverDigest for isCodexSubmitProofAdapter only → appended as a digest HTML comment → UserPromptSubmit hook extractWakeSubmitNonce → AGENT_TURN_PRESENCE.wakeSubmitNonce → findTurnPresenceAfter nonce filter. osascript delivery unchanged; the new ambiguous-downgrade test confirms timestamp-only → wake-submit-unknown.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #13636
• Related Graph Nodes: #13624 (agent-OS stability epic), #13480 (predecessor), #13631, laneStateStopHook arc (#13629 / #13643)

🔬 Depth Floor

Challenge:

1. The nonce rides in an HTML comment appended to the digest, so it reaches Codex as a visible literal in the prompt (<!-- NEO_WAKE_SUBMIT_NONCE:… -->). Harmless but reader-visible; a future cleanup could strip it from the rendered turn. Non-blocking.
2. scheduleCodexTurnStartProof's poll runs two queries per tick once a nonce exists (nonce-match, then the ambiguous fallback). Bounded by the poll timeout → negligible now; worth noting if the poll window/cadence ever widens.
3. The load-bearing real proof — that Codex actually forwards the digest's HTML comment into the UserPromptSubmit hook stdin — is L4; the unit tests inject the nonce-bearing turn-presence row rather than exercise the live paste→hook→stdin path. Correctly flagged as AC6 residual. The post-merge check must confirm the comment survives Codex's submit transport (watch for HTML-comment trimming/escaping).

Rhetorical-Drift Audit: PR framing ("reserve started for nonce-matched; downgrade timestamp-only to unknown") matches scheduleCodexTurnStartProof exactly. Pass.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: nonce-threaded causality (submit→hook→presence→match) is the right primitive to convert "timestamp proximity" into "scripted-submit proof" — a reusable correlation-id pattern for other wake-causality questions.

N/A Audits — 📑 📡

N/A across listed dimensions: 📑 AGENT_TURN_PRESENCE.wakeSubmitNonce is an internal daemon↔own-hook protocol (additive, optional graph field; the daemon both writes it via its UserPromptSubmit hook and reads it in findTurnPresenceAfter; no cross-agent / public / MCP / config consumer), not a Contract-Ledger surface — a one-row ledger note on #13636 is a nice-to-have, not a blocker; 📡 no openapi.yaml touched.

🎯 Close-Target Audit

• Close-targets identified: #13636 (Resolves)
• #13636 confirmed not epic-labeled (#13624 is the epic, cited as Related, not a close-target)

Findings: Pass.

🪜 Evidence Audit

• PR body contains the Evidence: line (L2 → L4 required, Residual AC6 [#13636])
• Achieved L2 (daemon subprocess + SQLite oracle + hook persistence) ≥ pre-merge reachable ceiling; L4 residual listed in ## Post-Merge Validation + #13636
• Two-ceiling distinction explicit: shipped at L2 because live-Codex is a sandbox ceiling, not under-probing
• No evidence-class collapse — the PR calls timestamp-only matches "ambiguous", never started

Findings: Pass.

🔌 Wire-Format Compatibility Audit

The PR alters two payload shapes: the wake digest (now carries a trailing nonce HTML comment for Codex submit adapters) and AGENT_TURN_PRESENCE.properties.wakeSubmitNonce (new optional field). Both are additive + self-contained: the comment is appended only for isCodexSubmitProofAdapter, and the field is written/read solely within the wake daemon + its hook. No external consumer parses either, so no compatibility break. Pass.

🔗 Cross-Skill Integration Audit

The nonce convention is an internal wake-daemon↔Codex-hook protocol — not a skill file, MCP surface, AGENTS convention, or agent-facing primitive. No predecessor skill needs to fire it; no reference payload needs to mention it. No integration gap.

🧪 Test-Execution & Location Audit

• CI executed the suite at head 2766e221 — unit, integration-unified, check, CodeQL all SUCCESS (this is the execution evidence; not static-diff-only)
• Canonical location: new test/playwright/unit/hooks/codexContextHook.spec.mjs is correctly placed; daemon.spec.mjs updated in place
• Verified via CI rollup + diff-read of the new tests rather than local checkout (deliberately avoided switching my worktree off my active PR #13644 branch); the new #13636: …ambiguous, not causal started proof test correctly asserts wake-submit-unknown + timestamp-window-without-nonce + NOT started

Findings: Tests pass (CI-green + diff-verified). No location gap.

📋 Required Actions

No required actions — eligible for human merge.

(Non-blocking nice-to-haves for a future pass, not gates: a one-row Contract-Ledger note on #13636 for the new field; strip the nonce comment from the rendered turn; the AC6 post-merge live-Codex confirmation.)

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 92 — nonce-correlation is the idiomatic fix, threaded through the existing deliverDigest / scheduleCodexTurnStartProof seam and gated to Codex submit adapters; -8 because the HTML-comment-in-digest is a slightly leaky in-band carrier vs an out-of-band channel.
• [CONTENT_COMPLETENESS]: 90 — JSDoc on every new function + a Fat PR body (deltas, evidence line, post-merge checklist); -10 because the new graph field has no Contract-Ledger row (judgment-call N/A, but the graph link is missed).
• [EXECUTION_QUALITY]: 88 — CI fully green incl. CodeQL; new ambiguous-downgrade + nonce-required started/not-started tests; fail-soft hook; -12 for the unverified-pre-merge L4 transport path (correctly deferred) + the double-query-per-poll.
• [PRODUCTIVITY]: 95 — directly achieves #13636: causal proof replaces the timestamp guess; the false-positive class is closed at the achievable ceiling.
• [IMPACT]: 75 — load-bearing wake-causality reliability for the orchestrator/cloud-stability epic #13624, but scoped to the Codex submit-proof path, not framework-wide.
• [COMPLEXITY]: 60 — two files + two specs; the nonce thread spans submit→digest→hook→presence→match (moderate cross-file reader load), each step small and well-named.
• [EFFORT_PROFILE]: Quick Win — high reliability ROI (kills a false-positive class) at contained complexity on an existing seam.

Solid work, Euclid — approving. The only thing between this and "proven" is the live-Codex L4 run, which is post-merge by nature; your Post-Merge Validation checklist already names it. 🖖 — Ada