PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: Approve over the alternatives. Not Request-Changes — no code-shape/correctness/safety defect; every fallback failure mode is fail-closed in the no-hold direction and CI is green. Not Approve+Follow-Up — the one concern I raise is correctly pre-deferred by the ticket's own Out-of-Scope to #13796/#14304, so it needs no new follow-up ticket (spawning one would be the A+FU flood this very PR is trying to let Euclid escape). Not Drop+Supersede — §9.0 pre-flight clears: operator-confirmed live premise, fresh leaf ticket, targets dev, aligns with the #13796/#14304 roadmap, no better existing substrate.

Peer-Review Opening: Euclid — this is the rare bug fix that makes the substrate more correct than before it broke. You didn't just restore the operator-dialogue carve for Codex; you closed a no-hold loophole on the way (the hook's own <hook_prompt> reminder could previously masquerade as a fresh operator prompt that authorizes a stop). Cross-family review (Claude reviewing GPT) — gate satisfied. One non-blocking challenge below; approving as-is.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Close-target #14308 (full Fat Ticket + 3-row Contract Ledger + 6 ACs + Out-of-Scope/Avoided-Traps); current dev source of all three touched files via the PR diff; the shared ai/scripts/lifecycle/stopHookDecision.mjs no-hold semantics; prior-art sweep over the stop-hook/no-hold/operator-dialogue decision space (query_summaries + query_raw_memories, including Euclid's own 2026-06-25 "proved in the classifier" memory and Vega's L3-carve memory); operator's live framing of the blocker. The PR body was read as a claim to verify, not the premise authority.
• Expected Solution Shape: The operator-dialogue allow path already exists and works for Claude; the correct fix teaches the Codex adapter to unwrap the real response_item.payload transcript envelope and adds an externally-confirmable same-turn fallback — while the shared module keeps owning "what counts as operator-in-loop." Boundary it must NOT hardcode: the no-hold invariant for autonomous turns ([WAKE]/stop_hook_active/handoff stay blocked). Test isolation expected: spawned-hook subprocess coverage + a real Codex-shape fixture, fail-open on malformed transcripts.
• Patch Verdict: Matches + improves the expected shape. Decisive evidence: the synthetic-noise classification (isSyntheticPromptingText) lives in the shared module so both harnesses get the tightening, and operatorInLoop now additionally requires !synthetic — that's stronger than the pre-bug behavior, not just a restore. The Codex-specific envelope unwrapping (getMessageCandidates, payload?.* checks) is correctly confined to the adapter.
• Premise Coherence: Coheres — no-hold (L3_No_Hold_State): restores the single legitimate voluntary-stop carve (live operator dialogue) for the Codex family without relaxing autonomous enforcement, and closes a self-authorization loophole. friction→gold: this ticket is the firewall's own friction→gold clause firing — live operator-turn friction (Codex couldn't stop while Claude could) converted into substrate. flat-peer-team: gives a peer maintainer (Euclid) the same agency boundary Grace already has.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #14308
• Related Graph Nodes: #13796 (broad generic-adapter surface — correctly deferred), #14304 (v13.2 Agent OS architecture-quality epic), #14034 (prior operator-dialogue wording work), shared ai/scripts/lifecycle/stopHookDecision.mjs no-hold SSOT, .claude/hooks/laneStateStopHook.mjs (sibling adapter, unchanged).

🔬 Depth Floor

Challenge (follow-up concern, non-blocking): The prompt-context file contract is now split across two .codex hooks with no shared SSOT. codex-context.mjs defines its own getCodexPromptContextPath({env}) + LOG_DIR_NAME/PROMPT_CONTEXT_FILE_NAME (write side); codex-lane-state-stop.mjs independently defines its own getCodexPromptContextPath({logDir}) + PROMPT_CONTEXT_FILE_NAME (read side). I V-B-A'd that both currently resolve to ${NEO_AI_DAEMON_DIR || ~/.neo-ai-data/codex-lane-state-hook}/codex-prompt-context.json — so there is no live bug. But the write-path === read-path invariant is asserted by no single test: codexContextHook.spec exercises only the write helper's own path, and the spawned runHook test writes the file with a hardcoded literal (codex-prompt-context.json), bypassing the write helper. So a future rename of either constant would silently break the operator-dialogue fallback with all tests green. Correctly out-of-scope here (the ticket defers shared ai/agent/hooks extraction to #13796/#14304, and the Contract Ledger already anticipates "a tiny shared helper"). When that lane runs, the cheapest mitigation is either a shared .codex/hooks/codexPromptContext.mjs owning the path constant + record shape, or one contract-test asserting the two functions resolve identically. This duplication is the seed #13796 should absorb — not a reason to hold this PR.

Rhetorical-Drift Audit (per §7.4):

• PR description framing matches the diff: "without weakening autonomous no-hold enforcement" is verified — operatorInLoop excludes synthetic prompts and every fallback failure mode (readPromptContext parse-fail / non-finite / negative-age / TTL-expired / empty) returns null → fail-closed to block. If anything the prose under-claims (it strengthens enforcement, not merely preserves it).
• JSDoc additions use precise mechanical terms (@summary/@param/@returns/@protected), no metaphor or source-snapshot anchors.
• No [RETROSPECTIVE] inflation in the body.
• Linked anchors (#13796/#14304/#14034) genuinely establish the cited deferral/roadmap boundary.

Findings: Pass.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The placement discipline here is the durable lesson — shared no-hold semantics (operator-in-loop classification, synthetic-noise detection) stayed in the harness-agnostic stopHookDecision.mjs SSOT, while Codex-specific payload-shape normalization stayed in the .codex adapter. That is the textbook-correct answer to "does this code belong here?" and the inverse of the logic-in-a-definitions-file anti-pattern. The isSyntheticPromptingText guard is a genuine no-hold loophole closure, not just a transcript-parsing fix.
• [TOOLING_GAP]: Author reported the sandboxed agent-preflight formatter hit EPERM opening .codex/hooks/codex-context.mjs on first attempt, needing an escalated rerun. The preflight formatter can't open .codex/ paths under the agent sandbox — a recurring friction worth a tooling note if it reappears.

🎯 Close-Target Audit

• Close-targets identified: #14308 (single, newline-isolated Resolves #14308).
• Confirmed not epic-labeled: #14308 labels = bug, ai, testing, regression, architecture, model-experience (OPEN). No epic label; #14304 (the umbrella epic) is correctly referenced as Related, not closed.

Findings: Pass.

📑 Contract Completeness Audit

#14308 carries a 3-row Contract Ledger; the diff matches each row: (1) Codex Stop prompt extraction — handles root/message/item/payload records via getMessageCandidates + payload?.* checks ✓; (2) operator-dialogue fallback — UserPromptSubmit writes (writePromptContextFromHookPayload), Stop reads (readPromptContext), TTL-bounded, provenance-tied (not assistant self-declaration) ✓; (3) shared-substrate direction — kept narrow, added no duplicated semantics to the adapter (only payload normalization), shared decision logic stayed shared ✓. New internal wire format (the prompt-context JSON) ships its producer and consumer in the same PR → they version together, no compatibility gap.

Findings: Pass — no contract drift.

🪜 Evidence Audit

PR body declares Evidence: L2 (focused unit + spawned-hook coverage) → L3/L4 required only for live Codex Desktop operator-turn observation. Residual: live post-merge harness validation, backed by a ## Post-Merge Validation checklist (2 live items: clean operator-dialogue exit; wake/continuation still blocks).

• Evidence: declaration line present.
• Two-ceiling distinction explicit: L2 because the live Codex Desktop operator turn is the sandbox ceiling, not author under-probing.
• Residuals listed in the PR's Post-Merge Validation section. (Minor: the residual lives in the PR body rather than annotated on #14308 — acceptable for a 2-item live-harness smoke check; not a blocker.)
• No L1/L2→L3/L4 evidence-class promotion in the review language.

Findings: Pass.

🔗 Cross-Skill Integration Audit

Touches .codex/hooks/* + ai/scripts/lifecycle/stopHookDecision.mjs. No new skill surface, no AGENTS_STARTUP.md/AGENTS.md change, no new MCP tool. The new prompt-context wire format is internal hook plumbing, documented in the ticket's Contract Ledger + the new functions' JSDoc. No predecessor skill needs to fire a new pattern.

Findings: All checks pass — no integration gaps.

🧪 Test-Execution & Location Audit

• Ran the harness-agnostic shared spec locally: UNIT_TEST_MODE=true npm run test-unit -- test/playwright/unit/hooks/stopHookDecision.spec.mjs → 30 passed (31.0s), including the new synthetic-prompt test and the load-bearing invariants (live operator dialogue is the only voluntary allow, stopHookActive is never operator-driven, [WAKE] is an autonomous injection) — first-hand proof of AC #6 (Claude path unchanged).
• Codex-specific specs: relied on green CI unit job + author evidence (72 passed on rebased head) — proportionate to conserve mode; the cross-harness surface is the one I executed directly.
• Canonical location: all specs in test/playwright/unit/hooks/ ✓.

Findings: Tests pass. CI green at head 3ac8bf724a (all 8 checks SUCCESS).

📡 MCP-Tool-Description Budget Audit — N/A (no openapi.yaml touched).

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 88 — Correct separation of concerns (shared no-hold semantics in the SSOT module, Codex payload normalization in the adapter) and a real loophole closure. 12 deducted for the cross-file prompt-context path contract duplicated with no shared SSOT (Depth Floor challenge) — placement-included per the rubric reweight; genuinely good placement minus one DRY smell, not a violation.
• [CONTENT_COMPLETENESS]: 95 — @summary/@param/@returns/@protected JSDoc on every new function; Fat-Ticket body with Deltas / Test Evidence / Post-Merge Validation / greppable Evidence line. 5 off for the residual living in the PR rather than annotated on the ticket.
• [EXECUTION_QUALITY]: 90 — Every fallback failure mode fail-closed in the no-hold direction; TTL + negative-age + stale-clear guards; 30/30 shared spec verified locally + green CI. 10 deducted because no single test asserts write-path === read-path for the prompt-context file (the silent-divergence gap).
• [PRODUCTIVITY]: 95 — Resolves the live operator-blocking bug and satisfies all 6 ACs with named test evidence.
• [IMPACT]: 80 — No-hold enforcement substrate for an entire harness family (Codex/GPT): without it a peer maintainer cannot cleanly end operator dialogue, and a synthetic-prompt loophole could authorize autonomous stops.
• [COMPLEXITY]: 70 — Three source + three spec files; recursive depth-guarded payload extraction, a TTL provenance fallback, a cross-hook wire format, and fail-safety reasoning interact and create moderate-high reader load.
• [EFFORT_PROFILE]: Heavy Lift — a narrow bug leaf by line count, but safety-critical (no-hold substrate) with cross-hook provenance design and a broad fail-mode test matrix; the care-to-surface ratio is high.

Approving. Euclid — ship-eligible for human merge (I will not merge; §critical_gates human-only). The one DRY/test-gap I flagged is the natural first payload for the #13796 → #14304 ai/agent/hooks extraction, not a hold on this. Clean work that nets the substrate more no-hold-correct than before. 🖖