fix(agent-completion): stop PermissionRequest hooks from firing false completion notifications

[rodboev]

Summary

• Every Claude, Devin, Kimi, and Copilot PermissionRequest hook was triggering a false agent-task-complete desktop notification because isCompletionHookState in the agent-completion coordinator treated 'waiting' (PermissionRequest) and 'blocked' (Copilot elicitation dialog) as completion states.
• Removed both states from completion eligibility. Added isAttentionHookState that calls clearPendingHookDone() to cancel any quiet-window timer that started on a prior done event — preventing a delayed false notification if a permission prompt arrives within the 1.5-second quiet window.
• Only 'done' now triggers the completion path. The sidebar attention indicator already surfaces permission-prompt state inline without a notification.

Closes #5698.

Screenshots

No visual change.

Testing

• pnpm lint
• pnpm typecheck
• pnpm test
• pnpm build
• Added or updated high-quality tests that would catch regressions, or explained why tests were not needed

Focused validation run in this session:

• pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/terminal-pane/agent-completion-coordinator.test.ts — 46 tests passed (0 failed). Covers: updated regression test asserting 0 dispatches on waiting (was 2); new blocked no-dispatch test; new debounce-cancellation edge case (working → done → waiting before quiet window, 0 dispatches after timer advance).
• pnpm run typecheck:web — passed (0 errors).

AI Review Report

Reviewed agent-completion-coordinator.ts change and the surrounding observeHookStatus logic on macOS, Linux, and Windows. The change is a pure coordinator logic fix — no Electron, IPC, SSH, terminal, or browser surface touched. The helper isAttentionHookState mirrors the existing isCompletionHookState pattern. The clearPendingHookDone() call matches every other early-return site in observeHookStatus that clears pending timers. The existing 1.5-second quiet-window path for real done events is unchanged. Provider neutrality: all four provider normalization functions (Claude, Devin, Kimi, Copilot) map PermissionRequest/blocked to 'waiting'/'blocked' — those mappings are correct and unaffected. Mobile/desktop notification routing unchanged (DesktopNotificationSource type is not modified). No cross-platform risk; pure logic change in renderer TypeScript.

Security Audit

No IPC paths, filesystem access, subprocesses, auth, or secrets touched. The change affects only which coordinator state transitions dispatch a notification — no security surface.

Notes

Routing 'waiting'/'blocked' to a new agent-attention-needed notification source was considered. That would be a product feature (separate notification category with its own settings, throttling, and mobile semantics) and is out of scope for this correctness fix. The sidebar attention indicator already covers the attention state inline.

[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: 09ee22fd-058c-4111-adda-1345d9ba0c7c

📥 Commits

Reviewing files that changed from the base of the PR and between bad9afe and a7cbe5b.

📒 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

---

📝 Walkthrough

Walkthrough

The coordinator now treats only done as a completion hook state and treats waiting and blocked as attention states. When an attention state arrives, pending hook-done quieting is cleared and the handler returns early. Tests cover mid-turn waiting and blocked transitions, plus cancellation of a pending done timer when waiting arrives before the quiet window ends.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately summarizes the main fix to agent-completion notifications.
Description check	✅ Passed	The description matches the required template and includes summary, screenshots, testing, AI review, security audit, and notes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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.}