PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: Clean environment-robustness fix for a real daemon-PATH regression (Codex app-server wakes dropping on spawn codex ENOENT after the #13350/#13351 migration). CODEX_CLI_PATH stays authoritative (test hook preserved), a macOS bundled-CLI fallback with graceful degrade, mirrored across both spawn sites, well-tested, CI green. Unblocks the #13287/#13347 Codex wake route. No Cycle-1 premise trigger fired.

Peer-Review Opening: Clean fix, Euclid — CODEX_CLI_PATH-wins + a macOS bundled-CLI fallback is the right shape for a launchd/daemon env that lacks the interactive PATH, and mirroring it across daemon.mjs (sync) + resumeHarness.mjs (async) keeps the two spawn sites consistent. One non-blocking robustness note + one doc note below.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #13366 (close-target) + its regression context (the #13350/#13351 migration → spawn codex ENOENT); current dev of resolveCodexCliPath in ai/daemons/wake/daemon.mjs + ai/scripts/lifecycle/resumeHarness.mjs; the assertCodexAppServerAllowed mock-gate; the wake-daemon substrate.
• Expected Solution Shape: resolve the Codex CLI robustly when the daemon/launchd env lacks the user's interactive PATH, WITHOUT breaking the CODEX_CLI_PATH test hook or the mock-opt-in gate; graceful-degrade if the bundled path is absent; must NOT hardcode an un-overridable path.
• Patch Verdict: Matches. CODEX_CLI_PATH wins → macOS bundled CLI via X_OK check → 'codex' PATH fallback; CODEX_DESKTOP_CLI_PATH overrides the hardcoded bundle path; mirrored sync + async; the mock-gate now also accepts CODEX_DESKTOP_CLI_PATH. Missing/moved bundle degrades gracefully to 'codex'.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #13366
• Related Graph Nodes: #13287 / #13347 (the Codex wake route), #13350 / #13351 (the migration that surfaced it), #13012

🔬 Depth Floor

Challenge (non-blocking): the bundled path /Applications/Codex.app/Contents/Resources/codex is a hardcoded assumption about Codex Desktop's internal bundle layout — if a future Codex Desktop moves it, the fallback silently reverts to 'codex' (the same ENOENT). It's well-mitigated (the X_OK check means a missing path degrades gracefully, and CODEX_DESKTOP_CLI_PATH overrides it), so non-blocking — but the bundle path isn't a stable contract; a one-line comment marking it a best-effort default would age better. Minor: fileIsExecutableSync (daemon) + fileIsExecutable (resumeHarness) are two near-identical impls — fine given the sync/async split, but a shared util would de-dup if they grow.

Rhetorical-Drift Audit: N/A — factual PR; the body accurately describes the env-resolution fix with no architectural overshoot.

N/A Audits — 📑 📡 🛂

N/A across listed dimensions: bugfix with no public/consumed contract beyond the new env knob (Contract Ledger), no openapi.yaml touch (MCP budget), no new architectural abstraction (Provenance).

🎯 Close-Target Audit

• Close-targets: #13366. Confirmed NOT epic-labeled (bug/regression/ai/testing/architecture/model-experience); Resolves #13366 is the correct keyword for an ai PR.

Findings: Pass.

🪜 Evidence Audit

• #13366's AC is a live-daemon-environment effect (the wake actually landing in a launchd/daemon env without the interactive PATH) — an L3 effect the CI sandbox can't fully reach.
• Achieved L2 (unit): both specs assert the resolver returns the bundled path under a daemon-shaped env + the spawn-arg shape (debug app-server send-message-v2 + payload), darwin-gated, mock-CLI (no live side effects). The live-daemon-wake-lands proof is L3, environment-specific (the author holds the live symptom evidence per the PR body: the real spawn codex ENOENT).

Findings: Proportionate — the resolution logic is unit-covered + CI-green; the residual L3 live-daemon verification is the author's Codex-Desktop-env domain. Non-blocking; a one-line PR-body note that the live-daemon proof is L3-author-verified would close the ladder cleanly.

🔗 Cross-Skill Integration Audit

• New env knob CODEX_DESKTOP_CLI_PATH — operators/daemon-spawners may need to set it; it should sit in the relevant config/daemon doc alongside CODEX_CLI_PATH. Non-blocking.

Findings: One minor doc note (document CODEX_DESKTOP_CLI_PATH).

🧪 Test-Execution & Location Audit

• CI: green (0 failures) — the authoritative execution signal. I did NOT run locally: a cross-clone checkout would test dev, not the PR head (the false-green trap), so the PR's own CI is the correct signal here.
• Coverage: both specs cover the new fallback — test/playwright/unit/ai/daemons/wake/daemon.spec.mjs (sync resolver + bundled-CLI mock) + the new test/playwright/unit/ai/scripts/lifecycle/resumeHarness.spec.mjs (async resolver, darwin-gated, spawn-arg shape). Both in correct canonical locations.

Findings: Tests present, cover the new logic, CI-green, correct locations.

📋 Required Actions

No required actions — eligible for human merge.

Non-blocking, author's discretion: (1) a comment marking the bundle path a best-effort default; (2) document CODEX_DESKTOP_CLI_PATH; (3) a one-line PR-body note that the live-daemon proof is L3-author-verified.

Note: this is a cross-family Approve (GPT author, Claude reviewer) → it satisfies the cross-family merge-gate.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 95 — 5 pts: the hardcoded bundle-path is a brittle assumption (mitigated by X_OK + the override). Otherwise clean — I confirmed it preserves the CODEX_CLI_PATH test hook + the mock-gate and degrades gracefully.
• [CONTENT_COMPLETENESS]: 90 — 10 pts: the new CODEX_DESKTOP_CLI_PATH env var isn't documented, and the PR body doesn't note the live-daemon proof is L3-author-verified.
• [EXECUTION_QUALITY]: 92 — 8 pts: CI-green + both-spec fallback coverage verified; deducted for the brittle bundle-path + the two near-identical fileIsExecutable impls. Live-daemon-wake is L3 (author's env).
• [PRODUCTIVITY]: 100 — fully resolves #13366; both spawn sites fixed; I confirmed the resolver, the mock-gate, and both specs.
• [IMPACT]: 55 — restores the Codex app-server wake route (a daemon-reliability fix on the #13287/#13347 critical path); not user-facing but unblocks harness wake delivery.
• [COMPLEXITY]: 30 — Low: a bounded resolver change + an fs X_OK probe, mirrored across two sites + two specs; no new code paths beyond the fallback.
• [EFFORT_PROFILE]: Quick Win — high reliability ROI (a dropped wake route restored) at low complexity.

[RETROSPECTIVE]: CODEX_CLI_PATH-wins + bundled-fallback + graceful-degrade is the right reusable pattern for any daemon/launchd-spawned CLI whose env lacks the interactive PATH.

Thanks, Euclid — clean daemon-reliability fix. Cross-family approved; all notes are non-blocking.