Stop duplicate agent-task-complete notifications from background plugin hooks

[nwparker]

Description

Fixes #4630 by preventing background Claude-Mem/plugin hooks after a completed turn from starting a second completion-notification cycle without a fresh user prompt.

Evidence of fix

Before: a normal Claude UserPromptSubmit → Stop dispatched one completion, then a Claude-Mem-shaped PostToolUse → Stop background cycle on the same pane dispatched a second notification (2 actual vs 1 expected).

After: the regression loop keeps the first completion and ignores the post-completion background cycle until a new prompt begins. Focused tests, lint/typechecks, and GitHub verify pass.

ELI5

After Claude said “done,” a memory plugin kept tidying up in the background. Orca mistook that cleanup for a brand-new task finishing and rang the bell again. The fix requires a new user request before another completion bell.

User-facing trade-offs

• Background-only hook activity after completion no longer produces another task-complete notification.
• A plugin that intentionally starts meaningful work without a fresh prompt will stay quiet until the next prompt; this is the conservative choice that prevents notification spam.
• Normal prompt-driven Claude and subagent completion notifications remain intact.

---

Summary

Fixes #4630.

Claude-Mem and similar Claude Code plugins can run background tool hooks on the same pane after the user-initiated turn has already completed. The completion coordinator previously treated every later working -> done hook cycle as a fresh turn, so a post-turn memory write could re-arm and fire another agent-task-complete notification.

This PR keeps the bug fix and now applies cleanly on origin/main:

• Forwards the existing hook turn-boundary fields (hookEventName, hasExplicitPrompt) through live agentStatus:set IPC and snapshot IPC.
• Uses the same shared tool-progress hook classifier in main and renderer, avoiding allowlist drift.
• Suppresses only background tool-progress working frames that arrive with no explicit prompt after Orca already notified completion for that same turn, plus the matching background done and title-spinner flicker.
• Leaves real new prompts, agents without hook identity, title-only/process-exit completion, and metadata-less remote-runtime paths on the existing behavior.

Tradeoffs / regressions

No known tradeoff remains.

The suppression is intentionally narrow: it requires a known tool-progress hook event (PreToolUse, PostToolUse, or PostToolUseFailure), no explicit prompt, and an already-notified hook completion for the current turn. A later UserPromptSubmit clears the suppression and re-arms completion normally. SSH/relay hook events preserve the metadata; paths that do not carry hook identity simply do not engage the suppression.

The previous snapshot-symmetry caveat is gone: agentStatus:getSnapshot now forwards hookEventName / hasExplicitPrompt too, and server.test.ts covers that contract.

Testing

• ./node_modules/.bin/vitest run --config config/vitest.config.ts src/renderer/src/components/terminal-pane/agent-completion-coordinator.test.ts src/renderer/src/hooks/agent-hook-completion-notifications.test.ts src/renderer/src/hooks/useIpcEvents.test.ts src/main/agent-hooks/server.test.ts — 4 files, 371 tests passed
• ./node_modules/.bin/oxlint src/main/agent-hooks/server.ts src/main/agent-hooks/server.test.ts src/main/index.ts src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts src/renderer/src/components/terminal-pane/agent-completion-coordinator.ts src/renderer/src/components/terminal-pane/agent-completion-coordinator.test.ts src/renderer/src/hooks/agent-hook-completion-notifications.test.ts src/renderer/src/hooks/useIpcEvents.ts src/renderer/src/hooks/useIpcEvents.test.ts src/shared/agent-status-types.ts src/shared/agent-tool-progress-hooks.ts
• ./node_modules/.bin/tsgo --noEmit -p config/tsconfig.node.json
• ./node_modules/.bin/tsgo --noEmit -p config/tsconfig.tc.web.json
• Merged origin/main (8c49426224) and resolved the only conflict in agent-completion-coordinator.test.ts

Reviewer notes

Added/updated focused coverage:

• agent-completion-coordinator.test.ts: user turn notifies once; background Claude-Mem-style tool churn does not re-notify; a genuine second prompt still notifies; title-spinner flicker during background churn stays suppressed.
• agent-hook-completion-notifications.test.ts: end-to-end renderer notification observer path does not dispatch the duplicate background completion.
• useIpcEvents.test.ts: hookEventName, hasExplicitPrompt, and stateStartedAt are forwarded to the completion observer.
• server.test.ts: status snapshots preserve hook turn-boundary metadata.

[coderabbitai]

Warning

Review limit reached

@nwparker, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 22 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: a8845a2a-edab-4d48-be5f-7225ca1b345a

📥 Commits

Reviewing files that changed from the base of the PR and between 8c49426 and fbe865b.

📒 Files selected for processing (11)

• src/main/agent-hooks/server.test.ts
• src/main/agent-hooks/server.ts
• src/main/index.ts
• src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts
• src/renderer/src/components/terminal-pane/agent-completion-coordinator.test.ts
• src/renderer/src/components/terminal-pane/agent-completion-coordinator.ts
• src/renderer/src/hooks/agent-hook-completion-notifications.test.ts
• src/renderer/src/hooks/useIpcEvents.test.ts
• src/renderer/src/hooks/useIpcEvents.ts
• src/shared/agent-status-types.ts
• src/shared/agent-tool-progress-hooks.ts

---

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]

Evidence

Test commands + output

Coordinator regression tests (new background plugin hook churn (Claude-Mem) describe block):

$ ./node_modules/.bin/vitest run --config config/vitest.config.ts \
    src/renderer/src/components/terminal-pane/agent-completion-coordinator.test.ts

 Test Files  1 passed (1)
      Tests  48 passed (48)

End-to-end notification path + main hook server, all green:

$ ./node_modules/.bin/vitest run --config config/vitest.config.ts \
    src/renderer/src/components/terminal-pane/agent-completion-coordinator.test.ts \
    src/renderer/src/hooks/agent-hook-completion-notifications.test.ts \
    src/main/agent-hooks/server.test.ts

 Test Files  3 passed (3)
      Tests  285 passed (285)

Lint + typecheck (scoped to changed files / their projects):

$ ./node_modules/.bin/oxlint <changed files>            # exit 0
$ ./node_modules/.bin/tsgo --noEmit -p config/tsconfig.node.json     # exit 0
$ ./node_modules/.bin/tsgo --noEmit -p config/tsconfig.tc.web.json   # exit 0

Before/after proof (the tests catch the regression)

I temporarily disabled the suppression guard (forced the new working-branch condition to false) and re-ran the new tests to confirm they actually fail without the fix:

FAIL > background plugin hook churn (Claude-Mem) > does not re-notify when a background tool-progress turn runs after the user turn
  AssertionError: expected "vi.fn()" to be called 1 times, but got 2 times
FAIL > background plugin hook churn (Claude-Mem) > still notifies for a genuinely new UserPromptSubmit turn after background churn
  AssertionError: expected "vi.fn()" to be called 1 times, but got 2 times
FAIL > background plugin hook churn (Claude-Mem) > ignores title spinner flicker during a background memory write
  AssertionError: expected "vi.fn()" to be called 1 times, but got 2 times

Restoring the fix makes all four pass. This is the literal #4630 symptom: a second dispatchTerminalNotification('agent-task-complete', ...) firing for a background Claude-Mem working->done cycle on an already-completed turn.

• Before: observeHookStatus({state:'working', hookEventName:'PostToolUse'}) after the user's turn bumped currentTurn -> the following background Stop passed dispatchCompletion's guards -> duplicate notification.
• After: that tool-progress working frame (no explicit prompt) is recognized as background churn for the already-notified turn and suppressed; the matching background done and the title-spinner flicker are suppressed too. A real UserPromptSubmit (explicit prompt) still re-arms and notifies.

Electron / CDP

Launched the dev build for this worktree over CDP and confirmed it compiles and runs cleanly with the change: attached instance identity Orca: nwparker/fix-4630 (/private/tmp/orca-bb/4630), workspaceSessionReady: true, notifications.enabled: true, notifications.agentTaskComplete: true. A fully authentic live repro requires installing the third-party claude-mem Claude Code plugin plus a real Claude session, which is not available in this validation sandbox — so the deterministic unit/integration tests above (which exercise the exact observeAgentHookCompletionForNotification -> observeHookStatus consumer path the IPC handler calls) are the authoritative regression evidence.

[nwparker]

Takeover update for PR #6595:

• Merged origin/main (8c49426224) into the PR branch and resolved the only conflict in agent-completion-coordinator.test.ts; GitHub reports the PR as mergeable after push.
• Removed the remaining caveat: agentStatus:getSnapshot now forwards hookEventName / hasExplicitPrompt, and the tool-progress hook allowlist is shared by main + renderer.
• Tradeoff status: zero known tradeoffs/regressions. Suppression remains limited to post-completion tool-progress hooks with no explicit prompt; real UserPromptSubmit turns and metadata-less paths keep existing behavior.
• Validation: focused vitest suite passed (4 files, 371 tests), scoped oxlint passed, tsgo --noEmit -p config/tsconfig.node.json passed, and tsgo --noEmit -p config/tsconfig.tc.web.json passed.

[nwparker]

CI follow-up: GitHub PR Checks verify passed on fbe865b90c in 11m12s. The job completed lint, styled-scrollbar guard, project-owned .d.ts guard, feature-wall budget, macOS entitlements, typecheck, tests, unpacked app build, and packaged CLI smoke successfully.

Only annotation: a React hook dependency warning in src/renderer/src/components/right-sidebar/useGitStatusPolling.ts, which is outside this PR diff.