Skip to content

fix(notifications): only alert for background session completions - #726

Open
beruro wants to merge 2 commits into
developfrom
beruro/fix-foreground-session-notifications
Open

fix(notifications): only alert for background session completions#726
beruro wants to merge 2 commits into
developfrom
beruro/fix-foreground-session-notifications

Conversation

@beruro

@beruro beruro commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Problem

A foreground Session completion followed the same task-completion delivery path as background work. The notification policy used background: false to suppress only the native system alert, while still playing the completion sound. The CLI monitor also relied on the persisted background flag without checking whether another Session was currently active.

Solution

Suppress taskCompletion delivery when its context explicitly shows that the Session already has user attention. CLI completion events now combine the persisted background flag with the active Session ID before applying the existing document visibility and focus check. Background completions continue to alert, while approvals, errors, Team Inbox notifications, quiet hours, and per-Session muting keep their existing behavior.

The change adds a typed foreground-session suppression reason and regression coverage at both the policy and sound-delivery boundaries. It does not change settings, persistence, IPC, or wire formats.

Potential risks

During a transient state where no Session is active, a completion is conservatively treated as needing attention and may alert. Completion calls without a Session attention context retain their previous behavior; the Session terminal delivery boundary supplies that context. Native desktop audio was not manually exercised, but the sound invocation boundary is covered by tests. Rollback is a normal revert of this commit; no data migration or recovery is required.

Verification

  • npx eslint src/api/services/notificationPolicy.ts src/types/ui/notification.ts src/hooks/cliSession/useBackgroundSessionMonitor.ts src/api/services/notificationPolicy.test.ts src/api/services/notification.test.ts — passed
  • npx vitest run src/api/services/notificationPolicy.test.ts src/api/services/notification.test.ts src/hooks/session/sessionTerminalNotifications.test.ts — 3 files, 28 tests passed
  • npm run typecheck — passed
  • Repository pre-commit hook — lint-staged passed, TypeScript check passed, staged-file ESLint passed; Rust clippy skipped because no Rust files changed
  • git diff --check origin/develop...HEAD — passed
  • Base integrity — latest origin/develop is an ancestor; branch is 0 behind / 1 commit ahead

UI evidence

No screenshot is included because this changes notification/audio behavior without changing rendered UI or layout.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants