feat(notifications): OS notification when a background agent session finishes#155
Open
liam-russell wants to merge 2 commits into
Open
feat(notifications): OS notification when a background agent session finishes#155liam-russell wants to merge 2 commits into
liam-russell wants to merge 2 commits into
Conversation
…finishes Detects agent session exit and output-idle (via a new IdleTracker in TerminalManager) and, when the affected worktree isn't in view, shows a native OS notification that jumps back to the session on click. Adds a Settings toggle to opt out. Closes #92
Contributor
There was a problem hiding this comment.
Pull request overview
Adds native OS notifications for background agent sessions finishing or going idle, including click-to-focus/jump behavior and a Settings toggle so users can disable the feature.
Changes:
- Introduces an idle-detection mechanism in the terminal manager and emits agent-session status events (idle/exited).
- Adds renderer-side notification policy + OS notification IPC handler + click event to jump back to the session.
- Adds a Settings “Notifications” section and unit tests for the idle tracking/filtering logic.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Locks the new Vitest dependency resolution. |
| packages/types/src/notifications.ts | Adds shared IPC payload types for session status + notifications. |
| packages/types/src/ipc.ts | Adds IPC channel constants and the notification:show invoke contract. |
| packages/types/src/index.ts | Re-exports the new notifications types. |
| packages/terminal/src/terminal-manager.ts | Adds idle tracking hooks, session metadata type, and idle polling/reporting. |
| packages/terminal/src/index.ts | Exports idle tracker utilities/types from the terminal package. |
| packages/terminal/src/idle-tracker.ts | Implements IdleTracker (last-activity + one-shot idle reporting). |
| packages/terminal/src/tests/select-idle-agent-sessions.test.ts | Tests filtering idle sessions down to agent-launched ones. |
| packages/terminal/src/tests/idle-tracker.test.ts | Tests IdleTracker idle transition and re-arming behavior. |
| packages/terminal/package.json | Adds Vitest scripts and devDependency for the terminal package tests. |
| app/src/renderer/settings/NotificationsSection.tsx | Adds Settings UI to enable/disable agent session notifications. |
| app/src/renderer/routes/workspace.tsx | Wires the notifications hook and reuses the jump-to-session handler. |
| app/src/renderer/routes/settings.tsx | Renders the new Notifications settings section. |
| app/src/renderer/hooks/useAgentSessionNotifications.ts | Implements renderer-side policy + request to show OS notifications + click handling. |
| app/src/renderer/agent-session-notification-settings.ts | Persists the enable/disable setting via existing settings IPC. |
| app/src/preload/index.ts | Exposes new notification/status APIs and events to the renderer. |
| app/src/main/ipc/terminal.ts | Emits agent session status events on exit/idle from main to renderer. |
| app/src/main/ipc/notifications.ts | Implements notification:show using Electron Notification and click-to-jump event. |
| app/src/main/index.ts | Registers the new notification IPC handlers during app startup. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comment on lines
+14
to
+19
| useEffect(() => { | ||
| void loadAgentSessionNotificationsEnabled().then(value => { | ||
| setEnabled(value); | ||
| setLoaded(true); | ||
| }); | ||
| }, []); |
Contributor
Author
There was a problem hiding this comment.
Fixed in 5d8e483 — the load now always flips loaded to true via .finally() and surfaces a toast on failure instead of leaving the checkbox stuck disabled.
Comment on lines
+21
to
+29
| async function toggle() { | ||
| const next = !enabled; | ||
| setEnabled(next); | ||
| try { | ||
| await saveAgentSessionNotificationsEnabled(next); | ||
| } catch (err) { | ||
| onToast(String(err), 'error'); | ||
| } | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Fixed in 5d8e483 — reverts enabled back to the previous value if the save fails, so the UI doesn't desync from the persisted setting.
| worktreePath: event.cwd, | ||
| terminalId: event.id, | ||
| }); | ||
| })(); |
Contributor
Author
There was a problem hiding this comment.
Fixed in 5d8e483 — the async IIFE now has a .catch that logs the error instead of letting it become an unhandled rejection.
- Match handleSessionData's override signature to the base class for clarity - Surface load/save errors in NotificationsSection instead of silently leaving the checkbox disabled or the UI desynced from the stored setting - Log (not swallow) failures inside the agent-session-status handler
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type
Summary
Why
Closes #92. Users running agents across several worktrees at once had no signal when a background session finished — issue #92 called this out as a differentiator, building on the session-status plumbing added for the agent sessions dashboard (#96).
Screenshots / recordings
New "Notifications" section in Settings (right column, below "Default Shell"):
This environment is a shared multi-agent host without a working Electron
debug/CDP target I could safely attach to for this change (the project's
.claude/launch.jsonat the repo root is currently owned by anotherconcurrent agent's dev server), so I wasn't able to capture an actual
screenshot. The component reuses the exact checkbox/label/section markup
already used by
McpSection/ShellSectionin Settings, verified byreading the rendered JSX and cross-checking Tailwind classes against the
existing sections.
Manual verification steps for a reviewer:
pnpm devBreaking changes
None. New IPC channels (
notification:show,event:agentSessionStatus,event:notificationClicked) are additive;TerminalManagerWithMeta's constructor gained two new optional parameters.Test plan
packages/terminal/src/__tests__/idle-tracker.test.ts— theIdleTrackerclass (touch/remove/checkIdle transitions, re-arming after new activity).packages/terminal/src/__tests__/select-idle-agent-sessions.test.ts— filtering idle sessions down to agent-launched ones only.pnpm lint && pnpm typecheckpass cleanly (verified via the pre-commit hook and standalone runs).pnpm test: the new/changed packages (@sproutgit/terminal,@sproutgit/types) pass consistently, including a full clean run. Some pre-existing tests inpackages/gitandapp(e.g.remote.test.ts,tool-test.test.ts,worktree-lifecycle.test.ts) intermittently failed locally withEAGAIN/EPIPEfork errors from heavy concurrent load on this shared multi-agent host — not from this diff (none of those files were touched, the failing subset changed between runs, and they pass individually when the host isn't under load). Recommend relying on CI for the definitive signal there.