Skip to content

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

Open
beruro wants to merge 1 commit into
developfrom
beruro/fix-foreground-session-notifications
Open

fix(notifications): only alert for background session completions#726
beruro wants to merge 1 commit 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

The local matrix below passed on pre-rebase head 2fde94437. GitHub rebased the unchanged one-commit scope onto current develop at 7be527a7a, producing head b6e4537ee; CI is rerunning. The prior frontend red was limited to four unrelated Markdown SSR baseline failures now isolated in #753; this PR’s Rust check passed.

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

@beruro
beruro force-pushed the beruro/fix-foreground-session-notifications branch from 64f6298 to 5d5ca7e Compare August 10, 2026 04:04
@beruro
beruro force-pushed the beruro/fix-foreground-session-notifications branch from 2fde944 to b6e4537 Compare August 10, 2026 05:46
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.

1 participant