[nwparker]

Summary

Codex under "Approve for me" no longer raises false "approval required" desktop notifications. When Codex pauses for a permission decision that its auto-review agent then approves, the pane resumes on its own and no attention notification fires. A permission pause that a human actually needs to resolve still notifies — just after a short quiet window.

Root cause

Codex emits its PermissionRequest hook at the human-input boundary before the approval decision is made. That hook maps unconditionally to state: 'waiting' (src/shared/agent-hook-listener.ts:3129-3130), and the completion coordinator dispatched the OS attention notification immediately on any waiting/blocked hook:

• src/renderer/src/components/terminal-pane/agent-completion-coordinator.ts — observeHookStatus → dispatchAttention() fired synchronously with no debounce.

Under "Approve for me", the review agent approves and Codex resumes (a working hook) a moment later — but the attention notification had already fired, producing a false alarm. The existing yolo suppressor (codex-auto-approval-notification-suppression.ts) only covers full-auto launch flags, not the "Approve for me" review-agent path, so those pauses fell through to the immediate notification.

The fix

Debounce only the Codex OS attention notification behind a CODEX_ATTENTION_QUIET_MS = 1500ms quiet window, mirroring the coordinator's existing HOOK_DONE_QUIET_MS pattern:

• On a Codex waiting/blocked attention hook, schedule the notification instead of firing it. A working or completion hook arriving inside the window cancels the pending dispatch.
• Cancellation is wired into every path that ends or supersedes the pause: the working hook, the completion-hook branch, the suppressed-attention path, the internal dispatchCompletion (covers title/process-exit completion sources), resetCompletionState, and dispose.
• The scheduled callback re-checks isLive() and hasAgentRunEvidence before firing, so a torn-down pane can never notify.

Why zero-regression:

• Strictly scoped to payload.agentType === 'codex'. Every other agent's waiting/blocked pause dispatches immediately, exactly as before (asserted by a test that also checks no timer is armed for non-Codex).
• Visual status stays immediate. The sidebar agent status is set by store.setAgentStatus at the IPC ingestion layer (useIpcEvents.ts:3096) before the coordinator's notification path runs (:3125), so the "needs input" indicator is unaffected by the debounce. The coordinator only gates the OS notification.
• Does not touch src/shared/agent-hook-listener.ts, so the prior fix fix(agent-status): clear answered Claude question waits #8311 (Claude sticky waits scoped to PermissionRequest) is untouched.
• Timer is single-instance (clear-before-arm) and cleaned up on dispose/reset — no leak.

Evidence

UI screenshots aren't feasible in headless CI; the co-located fake-timer tests are the authoritative proof.

Before (fix reverted, tests present) — the immediate-dispatch bug:

❯ agent-completion-coordinator.test.ts (63 tests | 2 failed | 60 skipped)
  × cancels the debounced Codex attention notification when work resumes in the quiet window
    AssertionError: expected "vi.fn()" to not be called at all, but actually been called 1 times
  × dispatches the debounced Codex attention notification after the quiet window elapses
    AssertionError: expected "vi.fn()" to not be called at all, but actually been called 1 times
 Tests  2 failed | 1 passed | 60 skipped (63)

After (fix applied):

 Test Files  1 passed (1)
      Tests  3 passed | 60 skipped (63)   # Codex attention tests

 Test Files  6 passed (6)
      Tests  112 passed (112)             # full coordinator + notification-hook suites

pnpm run typecheck:tsc:web → exit 0. oxlint on all changed files → exit 0.

The three coordinator tests prove: (a) working → waiting(codex) → working inside the window ⇒ attention not dispatched (canceled); (b) working → waiting(codex) then advance past the window ⇒ attention is dispatched with the exact codex payload; (c) a non-Codex waiting dispatches immediately and arms no timer.

ELI5

Codex raises its hand ("can I run this?") before the robot reviewer decides. Orca used to buzz your phone the instant the hand went up. But with "Approve for me", the reviewer says "yes" a second later and Codex keeps going — so the buzz was pointless. Now Orca waits about a second and a half; if Codex is already back to work, it stays quiet. If Codex is genuinely stuck waiting for you, the buzz still comes through.

Trade-offs

• A genuine approval prompt's OS notification is now delayed by ≈1.5s (the quiet window). This is the deliberate cost of eliminating the false alarms, and it only affects Codex. The visual sidebar status is still immediate, so nothing is hidden — only the OS banner is slightly delayed.
• Codex-only by design: if a future provider needs the same behavior, the debounce would need to be extended to it explicitly.
• If Codex ever fails to emit a resume working/completion hook after an auto-approval, the notification still fires (fail-open) once the window elapses — no notification is ever lost, only debounced.

Relationship to existing PR

Connected PR #8443 by @rodboev independently lands the same core approach (Codex-scoped 1500ms quiet window; non-Codex immediate; yolo still suppressed by the existing predicate; change kept inside the coordinator that owns notification cadence). Credit to @rodboev — the diagnosis and design match. I implemented my own version before reading theirs and then compared.

Two deliberate differences in this PR:

1. Visual/lifecycle stays strictly immediate. fix(codex): debounce auto-resolved approval notifications (#8387) #8443 routes the whole dispatchAttention (including the per-pane dispatchHookLifecycle cursor/cache effect) through the timer; here only the OS notification is deferred, keeping every non-notification effect immediate.
2. Integration tests are updated too. The debounce lives in the shared coordinator, so the three Codex permission-request tests in agent-hook-completion-notifications.test.ts need to advance past the quiet window. fix(codex): debounce auto-resolved approval notifications (#8387) #8443 doesn't touch that file, so those integration tests would fail on its branch. This PR updates them (and factors their shared setup into one helper to stay under the max-lines limit without a suppression).

Recommendation: land whichever review prefers; the two are functionally equivalent for the real "Approve for me" scenario. This PR is the more complete/self-contained of the two.

Review loops

2 fresh independent reviewer passes (Opus). Round 1: CLEAN with two non-blocking nits (belt-and-suspenders cancel on non-hook completion sources; strengthen the non-Codex test with a timer-count assertion) — both applied. Round 2: CLEAN.

⚠️ Bug-bash PR — please review; do not merge yet.

Fixes #8387

Made with Orca 🐋

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 297bc0a8-e7a3-46b4-838a-85e05f2cfbd0

📥 Commits

Reviewing files that changed from the base of the PR and between cfb3b67 and 09bcf50.

📒 Files selected for processing (2)

• src/renderer/src/components/terminal-pane/agent-completion-coordinator.test.ts
• src/renderer/src/components/terminal-pane/agent-completion-coordinator.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/renderer/src/components/terminal-pane/agent-completion-coordinator.ts

---

📝 Walkthrough

Walkthrough

The coordinator adds a 1.5-second Codex attention quiet window and tracks its pending timer. Codex attention callbacks are delayed while non-Codex callbacks remain immediate. Pending notifications are canceled when work resumes, completion occurs, state resets, or the coordinator is disposed. Tests cover delayed and canceled Codex notifications, immediate non-Codex notifications, and shared Codex permission-pause handling for waiting and blocked states.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the Codex-only debounce for false 'Approve for me' approval notifications.
Description check	✅ Passed	The description covers the summary, rationale, evidence, review, security, and notes, with only a missing Screenshots heading.
Linked Issues check	✅ Passed	The changes implement the Codex-only notification debounce requested by #8387 and leave the Claude-specific behavior from #8311 unchanged.
Out of Scope Changes check	✅ Passed	The diff stays focused on the coordinator and its tests; no unrelated feature, API, or public-entity changes appear.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[nwparker]

Adversarial hardening review — 3 rounds, fresh reviewer each

Ran three hostile review passes over the Codex attention debounce. The core design (debounce the OS "approval required" banner behind a 1500 ms quiet window, scoped to agentType === 'codex', visual sidebar status still immediate) is sound. Two fail-open gaps were found and fixed (commit 09bcf50); no must-fix remains.

Fixed

1. Evidence teardown could drop / duplicate a genuine pause banner (fail-open).
   The coordinator is also instantiated in pty-connection.ts with real process polling. While the debounce timer was pending, a transient null/shell foreground blip — a documented Codex behavior, and the normal case for a remote/SSH inspection that can't resolve the foreground — could reach clearAgentRunEvidence/process-exit in handleProcessInspectionResult, tearing down hasAgentRunEvidence. The timer's fire-time guard then silently dropped the genuine "needs input" banner (or a false "task complete" completion raced it → double banner). The existing pendingHookDoneTimer guard already protects the analogous done-quiet-window; extended it to cover pendingCodexAttentionTimer too.

2. Title-driven resume didn't cancel the debounce.
   recordTitleWorking() cancelled the provisional done but not the debounced attention. A Codex resume that surfaces as a working-spinner title before/without the resume working hook would still fire the false banner. Added clearPendingCodexAttention() there, placed after the completion-replay guard so only an authoritative resume (not a stale post-completion title replay) can drop a still-pending genuine pause.

Tests

+7 discriminating fake-timer tests (each verified to fail if its guard is reverted): evidence-teardown blip guard, title-resume cancel, blocked parity, done-cancels-attention (no double-notify), dispose clears timer, second-distinct-pause re-arm. 175 tests green across all coordinator suites; web typecheck + oxlint clean.

Verdicts by dimension

• Fail-open: holds in both coordinator instantiations — a genuine non-resolving Codex pause fires after 1500 ms.
• Perf / leaks: at most one debounce timer (re-arm clears first; callback self-nulls); cleared on every exit path (completion / working / suppressed-yolo / completion-hook / title-resume / resetCompletionState / dispose / evidence-guard). No accumulation or wedge.
• Win / Linux / Mac: setTimeout only, no platform APIs.
• Remote SSH/relay: coordinator is renderer-side, transport-agnostic; fix Linux support #1 specifically hardens the remote null-foreground case.
• Scope: Codex-only; visual status still immediate via dispatchHookLifecycle before the timer.
• fix(agent-status): clear answered Claude question waits #8311: orthogonal (main-process, Claude-only).

Accepted trade-offs (not changed)

• The ~1.5 s notification delay on a genuine Codex approval is still justified and minimal — it's the tightest window that reliably absorbs the "Approve for me" self-resolve, and the visual status is unaffected.
• A Codex crash within ~1.5 s of emitting waiting can yield a needs-input + task-complete pair. This is not a regression from the pre-debounce baseline (which always produced both) and is extremely narrow; left as-is rather than add coupling.