From e1c48fa9f43686d81151a6365d6611e4d3b75142 Mon Sep 17 00:00:00 2001 From: Liam Russell <17897133+liam-russell@users.noreply.github.com> Date: Mon, 6 Jul 2026 21:22:24 +1000 Subject: [PATCH 1/2] feat(notifications): OS notification when a background agent session 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 --- app/src/main/index.ts | 3 + app/src/main/ipc/notifications.ts | 35 ++++++ app/src/main/ipc/terminal.ts | 27 +++++ app/src/preload/index.ts | 19 ++++ .../agent-session-notification-settings.ts | 13 +++ .../hooks/useAgentSessionNotifications.ts | 70 ++++++++++++ app/src/renderer/routes/settings.tsx | 2 + app/src/renderer/routes/workspace.tsx | 12 +- .../settings/NotificationsSection.tsx | 58 ++++++++++ packages/terminal/package.json | 7 +- .../src/__tests__/idle-tracker.test.ts | 47 ++++++++ .../select-idle-agent-sessions.test.ts | 40 +++++++ packages/terminal/src/idle-tracker.ts | 33 ++++++ packages/terminal/src/index.ts | 12 +- packages/terminal/src/terminal-manager.ts | 107 ++++++++++++++++-- packages/types/src/index.ts | 1 + packages/types/src/ipc.ts | 7 ++ packages/types/src/notifications.ts | 31 +++++ pnpm-lock.yaml | 3 + 19 files changed, 513 insertions(+), 14 deletions(-) create mode 100644 app/src/main/ipc/notifications.ts create mode 100644 app/src/renderer/agent-session-notification-settings.ts create mode 100644 app/src/renderer/hooks/useAgentSessionNotifications.ts create mode 100644 app/src/renderer/settings/NotificationsSection.tsx create mode 100644 packages/terminal/src/__tests__/idle-tracker.test.ts create mode 100644 packages/terminal/src/__tests__/select-idle-agent-sessions.test.ts create mode 100644 packages/terminal/src/idle-tracker.ts create mode 100644 packages/types/src/notifications.ts diff --git a/app/src/main/index.ts b/app/src/main/index.ts index 5e4411e0..c1ad2349 100644 --- a/app/src/main/index.ts +++ b/app/src/main/index.ts @@ -13,6 +13,7 @@ import { registerSystemHandlers } from './ipc/system.js'; import { registerGithubHandlers } from './ipc/github.js'; import { registerHookHandlers } from './ipc/hooks.js'; import { registerAgentHandlers } from './ipc/agents.js'; +import { registerNotificationHandlers } from './ipc/notifications.js'; import { registerChatHandlers } from './ipc/chat.js'; import { registerCommitMessageGeneratorHandlers } from './ipc/commit-message-generator.js'; import { registerProjectIdeaGeneratorHandlers } from './ipc/project-idea-generator.js'; @@ -303,6 +304,8 @@ app.whenReady().then(() => { if (isE2EMode) log.info('[e2e] hook handlers ok'); registerAgentHandlers(configDb, userDataPath); if (isE2EMode) log.info('[e2e] agent handlers ok'); + registerNotificationHandlers(); + if (isE2EMode) log.info('[e2e] notification handlers ok'); registerChatHandlers(configDb, userDataPath); if (isE2EMode) log.info('[e2e] chat handlers ok'); registerCommitMessageGeneratorHandlers(); diff --git a/app/src/main/ipc/notifications.ts b/app/src/main/ipc/notifications.ts new file mode 100644 index 00000000..1921f571 --- /dev/null +++ b/app/src/main/ipc/notifications.ts @@ -0,0 +1,35 @@ +/** + * Native OS notification for a background agent session finishing/idling. + * + * The renderer owns the *policy* (is the setting enabled, is this worktree + * currently out of view) since only it knows which worktree/tab is active — + * this module just owns the Electron `Notification` API and the + * focus-and-jump-back behaviour on click. + */ +import { BrowserWindow, Notification } from 'electron'; +import { IPC } from '@sproutgit/types'; +import type { ShowAgentNotificationArgs, NotificationClickedEvent } from '@sproutgit/types'; +import { handle } from './handle.js'; + +export function registerNotificationHandlers(): void { + handle(IPC.NOTIFICATION_SHOW, (_e, args: ShowAgentNotificationArgs) => { + if (!Notification.isSupported()) return; + + const win = BrowserWindow.fromWebContents(_e.sender); + const notification = new Notification({ title: args.title, body: args.body }); + + notification.on('click', () => { + if (win && !win.isDestroyed()) { + if (win.isMinimized()) win.restore(); + win.show(); + win.focus(); + win.webContents.send(IPC.EVENT_NOTIFICATION_CLICKED, { + worktreePath: args.worktreePath, + terminalId: args.terminalId, + } satisfies NotificationClickedEvent); + } + }); + + notification.show(); + }); +} diff --git a/app/src/main/ipc/terminal.ts b/app/src/main/ipc/terminal.ts index eba7cfa8..c207661f 100644 --- a/app/src/main/ipc/terminal.ts +++ b/app/src/main/ipc/terminal.ts @@ -1,5 +1,6 @@ import { ipcMain, BrowserWindow } from 'electron'; import { IPC } from '@sproutgit/types'; +import type { AgentSessionStatusEvent } from '@sproutgit/types'; import { TerminalManagerWithMeta } from '@sproutgit/terminal'; import { handle } from './handle.js'; import { log } from '../telemetry.js'; @@ -22,9 +23,24 @@ export const manager = new TerminalManagerWithMeta( } }, (id, exitCode) => { + // Read before handleSessionExit() (which runs after this callback, per + // TerminalManagerWithMeta's exit ordering) deletes it — a session + // closed deliberately via close() has already had its metadata removed + // by the time this fires, so `meta` is undefined and no status event is + // sent for that case, which is the desired behaviour (no notification + // for a terminal the user closed on purpose). + const meta = manager.getMeta(id); const win = sessionWindows.get(id); if (win && !win.isDestroyed()) { win.webContents.send(IPC.TERMINAL_EXIT, { id }); + if (meta && meta.agentId !== null) { + win.webContents.send(IPC.EVENT_AGENT_SESSION_STATUS, { + id, + cwd: meta.cwd, + agentName: meta.agentName, + reason: 'exited', + } satisfies AgentSessionStatusEvent); + } } const hookHandler = hookExitHandlers.get(id); if (hookHandler) { @@ -33,6 +49,17 @@ export const manager = new TerminalManagerWithMeta( } sessionWindows.delete(id); }, + (id, meta) => { + const win = sessionWindows.get(id); + if (win && !win.isDestroyed()) { + win.webContents.send(IPC.EVENT_AGENT_SESSION_STATUS, { + id, + cwd: meta.cwd, + agentName: meta.agentName, + reason: 'idle', + } satisfies AgentSessionStatusEvent); + } + }, ); export function registerTerminalHandlers(): void { diff --git a/app/src/preload/index.ts b/app/src/preload/index.ts index 8f56d8c1..cfc2ccd3 100644 --- a/app/src/preload/index.ts +++ b/app/src/preload/index.ts @@ -56,6 +56,9 @@ import type { McpConfigWriteResult, GlobalErrorEvent, ProjectIdeaGenerateResult, + AgentSessionStatusEvent, + ShowAgentNotificationArgs, + NotificationClickedEvent, } from '@sproutgit/types'; /** @@ -226,6 +229,22 @@ const api = { return () => ipcRenderer.off(IPC.TERMINAL_EXIT, handler); }, + onAgentSessionStatus: (callback: (event: AgentSessionStatusEvent) => void) => { + const handler = (_e: Electron.IpcRendererEvent, payload: AgentSessionStatusEvent) => callback(payload); + ipcRenderer.on(IPC.EVENT_AGENT_SESSION_STATUS, handler); + return () => ipcRenderer.off(IPC.EVENT_AGENT_SESSION_STATUS, handler); + }, + + // ── Notifications ───────────────────────────────────────────────────────── + showAgentSessionNotification: (args: ShowAgentNotificationArgs): Promise => + invoke(IPC.NOTIFICATION_SHOW, args), + + onNotificationClicked: (callback: (event: NotificationClickedEvent) => void) => { + const handler = (_e: Electron.IpcRendererEvent, payload: NotificationClickedEvent) => callback(payload); + ipcRenderer.on(IPC.EVENT_NOTIFICATION_CLICKED, handler); + return () => ipcRenderer.off(IPC.EVENT_NOTIFICATION_CLICKED, handler); + }, + // ── Workspace / recent ──────────────────────────────────────────────────── listRecentWorkspaces: (): Promise => invoke(IPC.WORKSPACE_LIST_RECENT), diff --git a/app/src/renderer/agent-session-notification-settings.ts b/app/src/renderer/agent-session-notification-settings.ts new file mode 100644 index 00000000..4b3b88aa --- /dev/null +++ b/app/src/renderer/agent-session-notification-settings.ts @@ -0,0 +1,13 @@ +import { api } from './api.js'; + +const AGENT_SESSION_NOTIFICATIONS_KEY = 'agentSessionNotificationsEnabled'; + +/** Whether to show an OS notification when a background worktree's agent session finishes/idles. Defaults to enabled. */ +export async function loadAgentSessionNotificationsEnabled(): Promise { + const raw = await api.getSetting(AGENT_SESSION_NOTIFICATIONS_KEY); + return raw !== 'false'; +} + +export async function saveAgentSessionNotificationsEnabled(enabled: boolean): Promise { + await api.setSetting(AGENT_SESSION_NOTIFICATIONS_KEY, String(enabled)); +} diff --git a/app/src/renderer/hooks/useAgentSessionNotifications.ts b/app/src/renderer/hooks/useAgentSessionNotifications.ts new file mode 100644 index 00000000..dea84933 --- /dev/null +++ b/app/src/renderer/hooks/useAgentSessionNotifications.ts @@ -0,0 +1,70 @@ +import { useEffect, useRef } from 'react'; +import { api } from '../api.js'; +import { useWorkspaceStore } from '../stores/workspace-store.js'; +import { loadAgentSessionNotificationsEnabled } from '../agent-session-notification-settings.js'; +import type { WorktreeInfo, AgentSessionStatusEvent, NotificationClickedEvent } from '@sproutgit/types'; + +function worktreeLabel(cwd: string, worktrees: WorktreeInfo[]): string { + const wt = worktrees.find(w => w.path === cwd); + if (!wt) return cwd.split(/[\\/]/).pop() ?? cwd; + return wt.branch ?? (wt.detached ? 'detached HEAD' : (wt.path.split(/[\\/]/).pop() ?? wt.path)); +} + +/** + * Fires a native OS notification when a background worktree's agent session + * finishes or goes idle, and jumps back to that session when the + * notification is clicked. + * + * "Background" means the app window isn't focused, a different worktree is + * selected, or the Terminal tab isn't active — only the renderer has all + * three signals, so (unlike idle *detection*, which lives in the main + * process's TerminalManager) the decision of whether to actually show a + * notification is made here, not in main. + */ +export function useAgentSessionNotifications(params: { + worktrees: WorktreeInfo[]; + onJump: (session: { id: string; cwd: string }) => void; +}) { + // Refs (not effect deps) so the listeners below can read the latest + // worktrees/onJump without unsubscribing/resubscribing on every render. + const worktreesRef = useRef(params.worktrees); + worktreesRef.current = params.worktrees; + const onJumpRef = useRef(params.onJump); + onJumpRef.current = params.onJump; + + useEffect(() => { + const offStatus = api.onAgentSessionStatus((event: AgentSessionStatusEvent) => { + void (async () => { + const enabled = await loadAgentSessionNotificationsEnabled(); + if (!enabled) return; + + const { activeWorktree, activeTab } = useWorkspaceStore.getState(); + const isForeground = + document.hasFocus() && activeWorktree?.path === event.cwd && activeTab === 'terminal'; + if (isForeground) return; + + const agentLabel = event.agentName ? `"${event.agentName}"` : 'Agent'; + const title = event.reason === 'exited' + ? `${agentLabel} session finished` + : `${agentLabel} session went idle`; + const body = `${worktreeLabel(event.cwd, worktreesRef.current)} — click to jump back in`; + + await api.showAgentSessionNotification({ + title, + body, + worktreePath: event.cwd, + terminalId: event.id, + }); + })(); + }); + + const offClicked = api.onNotificationClicked((event: NotificationClickedEvent) => { + onJumpRef.current({ id: event.terminalId, cwd: event.worktreePath }); + }); + + return () => { + offStatus(); + offClicked(); + }; + }, []); +} diff --git a/app/src/renderer/routes/settings.tsx b/app/src/renderer/routes/settings.tsx index 55a38544..8668480d 100644 --- a/app/src/renderer/routes/settings.tsx +++ b/app/src/renderer/routes/settings.tsx @@ -13,6 +13,7 @@ import { ShellSection } from '../settings/ShellSection.js'; import { AppSection } from '../settings/AppSection.js'; import { AISection } from '../settings/AISection.js'; import { McpSection } from '../settings/McpSection.js'; +import { NotificationsSection } from '../settings/NotificationsSection.js'; // ── Route definition ────────────────────────────────────────────────────────── @@ -91,6 +92,7 @@ function SettingsPage() {
+
diff --git a/app/src/renderer/routes/workspace.tsx b/app/src/renderer/routes/workspace.tsx index ae773573..afc24bb8 100644 --- a/app/src/renderer/routes/workspace.tsx +++ b/app/src/renderer/routes/workspace.tsx @@ -5,7 +5,7 @@ import { useState } from "react"; import { useQueryClient } from "@tanstack/react-query"; import { StagingPanel, Spinner, ContextMenuProvider } from "@sproutgit/ui"; import { GitBranch, GitMerge, Terminal, Bot, FileCode2 } from "lucide-react"; -import type { WorktreeInfo, TerminalInfo } from "@sproutgit/types"; +import type { WorktreeInfo } from "@sproutgit/types"; import { useToast } from "../toast-context.js"; import { useWorkspaceStore } from "../stores/workspace-store.js"; import { @@ -32,6 +32,7 @@ import { useScaffoldKickoff } from "../hooks/useScaffoldKickoff.js"; import { useWorktreeSelection } from "../hooks/useWorktreeSelection.js"; import { useCommitDiffState } from "../hooks/useCommitDiffState.js"; import { useTerminalManager } from "../hooks/useTerminalManager.js"; +import { useAgentSessionNotifications } from "../hooks/useAgentSessionNotifications.js"; import { useAutoUpdateListeners } from "../hooks/useAutoUpdateListeners.js"; import { useWorkspaceFileWatchers } from "../hooks/useWorkspaceFileWatchers.js"; import { useWorkspaceUiPersistence } from "../hooks/useWorkspaceUiPersistence.js"; @@ -198,9 +199,10 @@ function WorkspaceInner() { closeDeleteDialog: () => setDeleteTarget(null), }); - /** Jump-to-terminal action from the agent sessions dashboard: switches to - * the session's worktree (if not already active) and focuses its terminal tab. */ - function handleJumpToSession(session: TerminalInfo) { + /** Jump-to-terminal action from the agent sessions dashboard (and from + * clicking an agent-session-finished OS notification): switches to the + * session's worktree (if not already active) and focuses its terminal tab. */ + function handleJumpToSession(session: { id: string; cwd: string }) { const wt = worktrees.find((w) => w.path === session.cwd); // The dashboard already filters to this workspace's worktrees, but guard // here too — jumping to a session with no matching worktree would leave @@ -220,6 +222,8 @@ function WorkspaceInner() { setSessionsPanelOpen(false); } + useAgentSessionNotifications({ worktrees, onJump: handleJumpToSession }); + // ── Commit diff state ────────────────────────────────────────────────── const { selectedCommits, diff --git a/app/src/renderer/settings/NotificationsSection.tsx b/app/src/renderer/settings/NotificationsSection.tsx new file mode 100644 index 00000000..3e81b6ff --- /dev/null +++ b/app/src/renderer/settings/NotificationsSection.tsx @@ -0,0 +1,58 @@ +import { Bell } from 'lucide-react'; +import { useEffect, useState } from 'react'; +import type { ToastData } from '@sproutgit/ui'; +import { loadAgentSessionNotificationsEnabled, saveAgentSessionNotificationsEnabled } from '../agent-session-notification-settings.js'; + +interface Props { + onToast: (msg: string, variant?: ToastData['variant']) => void; +} + +export function NotificationsSection({ onToast }: Props) { + const [enabled, setEnabled] = useState(true); + const [loaded, setLoaded] = useState(false); + + useEffect(() => { + void loadAgentSessionNotificationsEnabled().then(value => { + setEnabled(value); + setLoaded(true); + }); + }, []); + + async function toggle() { + const next = !enabled; + setEnabled(next); + try { + await saveAgentSessionNotificationsEnabled(next); + } catch (err) { + onToast(String(err), 'error'); + } + } + + return ( +
+

+ Notifications +

+
+
+

Agent session finished

+

+ Show an OS notification when a background worktree's agent session finishes or goes idle. +

+
+ +
+
+ ); +} diff --git a/packages/terminal/package.json b/packages/terminal/package.json index e987903a..2efa4abe 100644 --- a/packages/terminal/package.json +++ b/packages/terminal/package.json @@ -9,7 +9,9 @@ ".": "./src/index.ts" }, "scripts": { - "typecheck": "tsgo --noEmit" + "typecheck": "tsgo --noEmit", + "test": "vitest run", + "test:watch": "vitest" }, "dependencies": { "@sproutgit/paths": "workspace:*", @@ -19,7 +21,8 @@ "devDependencies": { "@sproutgit/ts-config": "workspace:*", "@types/node": "^26.1.0", - "typescript": "npm:@typescript/native-preview@7.0.0-dev.20260701.1" + "typescript": "npm:@typescript/native-preview@7.0.0-dev.20260701.1", + "vitest": "^4.1.9" }, "license": "MIT" } diff --git a/packages/terminal/src/__tests__/idle-tracker.test.ts b/packages/terminal/src/__tests__/idle-tracker.test.ts new file mode 100644 index 00000000..a381b8a2 --- /dev/null +++ b/packages/terminal/src/__tests__/idle-tracker.test.ts @@ -0,0 +1,47 @@ +import { describe, it, expect } from 'vitest'; +import { IdleTracker } from '../idle-tracker.js'; + +describe('IdleTracker', () => { + it('does not report a session before it crosses the threshold', () => { + const tracker = new IdleTracker(); + tracker.touch('a', 1000); + expect(tracker.checkIdle(500, 1499)).toEqual([]); + }); + + it('reports a session once it crosses the threshold', () => { + const tracker = new IdleTracker(); + tracker.touch('a', 1000); + expect(tracker.checkIdle(500, 1500)).toEqual(['a']); + }); + + it('does not report the same idle period twice', () => { + const tracker = new IdleTracker(); + tracker.touch('a', 1000); + expect(tracker.checkIdle(500, 1500)).toEqual(['a']); + expect(tracker.checkIdle(500, 2000)).toEqual([]); + }); + + it('re-arms after new activity, allowing a later idle period to report again', () => { + const tracker = new IdleTracker(); + tracker.touch('a', 1000); + expect(tracker.checkIdle(500, 1500)).toEqual(['a']); + tracker.touch('a', 1600); + expect(tracker.checkIdle(500, 1900)).toEqual([]); + expect(tracker.checkIdle(500, 2100)).toEqual(['a']); + }); + + it('stops tracking a session once removed', () => { + const tracker = new IdleTracker(); + tracker.touch('a', 1000); + tracker.remove('a'); + expect(tracker.checkIdle(500, 2000)).toEqual([]); + }); + + it('tracks multiple sessions independently', () => { + const tracker = new IdleTracker(); + tracker.touch('a', 1000); + tracker.touch('b', 1400); + expect(tracker.checkIdle(500, 1500)).toEqual(['a']); + expect(tracker.checkIdle(500, 1900)).toEqual(['b']); + }); +}); diff --git a/packages/terminal/src/__tests__/select-idle-agent-sessions.test.ts b/packages/terminal/src/__tests__/select-idle-agent-sessions.test.ts new file mode 100644 index 00000000..d261466c --- /dev/null +++ b/packages/terminal/src/__tests__/select-idle-agent-sessions.test.ts @@ -0,0 +1,40 @@ +import { describe, it, expect } from 'vitest'; +import { selectIdleAgentSessions, type SessionMeta } from '../terminal-manager.js'; + +function makeMeta(overrides: Partial = {}): SessionMeta { + return { + cwd: '/repo/worktree-a', + label: 'AI Agent', + agentId: 'agent', + agentName: 'claude', + startedAt: 0, + ...overrides, + }; +} + +describe('selectIdleAgentSessions', () => { + it('includes an idle session that was agent-launched', () => { + const meta = new Map([['a', makeMeta()]]); + expect(selectIdleAgentSessions(['a'], meta)).toEqual([{ id: 'a', meta: meta.get('a') }]); + }); + + it('excludes a plain (non-agent) terminal sitting idle at a prompt', () => { + const meta = new Map([['a', makeMeta({ agentId: null, agentName: null })]]); + expect(selectIdleAgentSessions(['a'], meta)).toEqual([]); + }); + + it('excludes an idle id with no known metadata (already exited)', () => { + const meta = new Map(); + expect(selectIdleAgentSessions(['gone'], meta)).toEqual([]); + }); + + it('filters a mixed batch down to only the agent-launched sessions', () => { + const meta = new Map([ + ['agent-1', makeMeta({ cwd: '/repo/a' })], + ['shell-1', makeMeta({ agentId: null, agentName: null, cwd: '/repo/b' })], + ]); + expect(selectIdleAgentSessions(['agent-1', 'shell-1'], meta)).toEqual([ + { id: 'agent-1', meta: meta.get('agent-1') }, + ]); + }); +}); diff --git a/packages/terminal/src/idle-tracker.ts b/packages/terminal/src/idle-tracker.ts new file mode 100644 index 00000000..bdb996fa --- /dev/null +++ b/packages/terminal/src/idle-tracker.ts @@ -0,0 +1,33 @@ +/** + * Tracks last-activity timestamps for PTY sessions and reports which ones + * have just crossed an idle threshold, so a session is only reported once + * per idle period — touch() (new output) clears the report so the next + * quiet period can be reported again. + */ +export class IdleTracker { + private lastActivity = new Map(); + private reportedIdle = new Set(); + + touch(id: string, now = Date.now()): void { + this.lastActivity.set(id, now); + this.reportedIdle.delete(id); + } + + remove(id: string): void { + this.lastActivity.delete(id); + this.reportedIdle.delete(id); + } + + /** Returns session ids that just crossed `thresholdMs` of inactivity since the last check. */ + checkIdle(thresholdMs: number, now = Date.now()): string[] { + const idle: string[] = []; + for (const [id, last] of this.lastActivity) { + if (this.reportedIdle.has(id)) continue; + if (now - last >= thresholdMs) { + this.reportedIdle.add(id); + idle.push(id); + } + } + return idle; + } +} diff --git a/packages/terminal/src/index.ts b/packages/terminal/src/index.ts index 852e72b5..198bf7e5 100644 --- a/packages/terminal/src/index.ts +++ b/packages/terminal/src/index.ts @@ -1 +1,11 @@ -export { TerminalManager, TerminalManagerWithMeta, type SpawnOptions, type TerminalDataCallback, type TerminalExitCallback } from './terminal-manager.js'; +export { + TerminalManager, + TerminalManagerWithMeta, + selectIdleAgentSessions, + type SpawnOptions, + type SessionMeta, + type TerminalDataCallback, + type TerminalExitCallback, + type TerminalIdleCallback, +} from './terminal-manager.js'; +export { IdleTracker } from './idle-tracker.js'; diff --git a/packages/terminal/src/terminal-manager.ts b/packages/terminal/src/terminal-manager.ts index 1df84dfe..8c103f20 100644 --- a/packages/terminal/src/terminal-manager.ts +++ b/packages/terminal/src/terminal-manager.ts @@ -4,9 +4,11 @@ import { execFileSync } from 'node:child_process'; import { accessSync, constants, existsSync } from 'node:fs'; import { type WorkspaceHookShell } from '@sproutgit/types'; import { isPathWithin } from '@sproutgit/paths'; +import { IdleTracker } from './idle-tracker.js'; export type TerminalDataCallback = (sessionId: string, data: string) => void; export type TerminalExitCallback = (sessionId: string, exitCode: number) => void; +export type TerminalIdleCallback = (sessionId: string, meta: SessionMeta) => void; export type SpawnOptions = { cwd: string; @@ -113,15 +115,22 @@ export class TerminalManager { ...(process.platform === 'win32' && { useConpty: true }), }); - proc.onData(data => this.onData(id, data)); + proc.onData(data => { + this.handleSessionData(id, data); + this.onData(id, data); + }); proc.onExit(({ exitCode }) => { this.sessions.delete(id); - // Fires whether the process exited on its own or was killed via - // close() — close() already removes its own session-scoped state - // synchronously, so this hook must be idempotent for that case too. - this.handleSessionExit(id); + // Fired before handleSessionExit() so a subclass's exit callback can + // still read per-session metadata (e.g. cwd, agentId) that + // handleSessionExit is about to delete — see TerminalManagerWithMeta's + // getMeta(). When the session was already close()'d, that metadata is + // gone by this point regardless of ordering (close() deletes it + // synchronously) — the exit callback naturally sees no metadata, and + // finally handleSessionExit() below still runs idempotently. this.onExit(id, exitCode); + this.handleSessionExit(id); }); // Inject a one-shot command (e.g. from a hook launch event). @@ -180,6 +189,46 @@ export class TerminalManager { protected handleSessionExit(_id: string): void { // No extra state to clean up in the base class. } + + /** + * Hook for subclasses to observe each chunk of PTY output (e.g. to track + * last-activity time for idle detection) without needing to duplicate the + * onData wiring. + */ + protected handleSessionData(_id: string, _data: string): void { + // No-op in the base class. + } +} + +/** Per-session metadata tracked by TerminalManagerWithMeta. */ +export type SessionMeta = { + cwd: string; + label: string; + agentId: string | null; + agentName: string | null; + startedAt: number; +}; + +/** How long a session's output must be quiet before it's reported idle. */ +const DEFAULT_IDLE_THRESHOLD_MS = 20_000; +/** How often to scan for newly-idle sessions. */ +const IDLE_CHECK_INTERVAL_MS = 3_000; + +/** + * Filters idle-tracker results down to agent-launched sessions — a plain + * shell tab sitting at an idle prompt is expected and shouldn't be reported + * as a "finished" agent session. + */ +export function selectIdleAgentSessions( + idleIds: string[], + meta: ReadonlyMap, +): { id: string; meta: SessionMeta }[] { + const result: { id: string; meta: SessionMeta }[] = []; + for (const id of idleIds) { + const m = meta.get(id); + if (m && m.agentId !== null) result.push({ id, meta: m }); + } + return result; } /** @@ -187,7 +236,35 @@ export class TerminalManager { * so `closeForPath` can work correctly and the renderer can rebuild labels. */ export class TerminalManagerWithMeta extends TerminalManager { - protected meta = new Map(); + protected meta = new Map(); + private idleTracker = new IdleTracker(); + private readonly onIdle: TerminalIdleCallback | null; + private readonly idleThresholdMs: number; + private readonly idleTimer: ReturnType; + + constructor( + onData: TerminalDataCallback, + onExit: TerminalExitCallback, + onIdle?: TerminalIdleCallback, + idleThresholdMs = DEFAULT_IDLE_THRESHOLD_MS, + ) { + super(onData, onExit); + this.onIdle = onIdle ?? null; + this.idleThresholdMs = idleThresholdMs; + this.idleTimer = setInterval(() => this.pollIdleSessions(), IDLE_CHECK_INTERVAL_MS); + // Doesn't need to keep the process alive by itself — the Electron main + // process is already kept alive by its window(s). + this.idleTimer.unref?.(); + } + + /** Scans for sessions that just crossed the idle threshold and reports the agent-launched ones. Exposed (not private) so tests can drive it with a controlled clock instead of waiting on the real timer. */ + pollIdleSessions(now = Date.now()): void { + if (!this.onIdle) return; + const idleIds = this.idleTracker.checkIdle(this.idleThresholdMs, now); + for (const { id, meta } of selectIdleAgentSessions(idleIds, this.meta)) { + this.onIdle(id, meta); + } + } override spawn(options: SpawnOptions & { label?: string; agentId?: string; agentName?: string }): string { const id = super.spawn(options); @@ -198,16 +275,32 @@ export class TerminalManagerWithMeta extends TerminalManager { agentName: options.agentName ?? null, startedAt: Date.now(), }); + this.idleTracker.touch(id); return id; } + protected override handleSessionData(id: string): void { + this.idleTracker.touch(id); + } + override close(sessionId: string): void { super.close(sessionId); this.meta.delete(sessionId); + this.idleTracker.remove(sessionId); } protected override handleSessionExit(id: string): void { this.meta.delete(id); + this.idleTracker.remove(id); + } + + /** + * Reads a session's metadata without deleting it — used by the exit + * callback (registered at construction, fired before handleSessionExit()) + * to tell whether the session that just exited was agent-launched. + */ + getMeta(sessionId: string): SessionMeta | undefined { + return this.meta.get(sessionId); } override closeForPath(pathPrefix: string): void { @@ -231,7 +324,7 @@ export class TerminalManagerWithMeta extends TerminalManager { return this.meta.get(sessionId)?.label; } - listSessions(): { id: string; cwd: string; label: string; agentId: string | null; agentName: string | null; startedAt: number }[] { + listSessions(): ({ id: string } & SessionMeta)[] { return Array.from(this.meta.entries()).map(([id, m]) => ({ id, cwd: m.cwd, diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 100e31ff..8d671a70 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -15,3 +15,4 @@ export * from './files.js'; export * from './mcp.js'; export * from './errors.js'; export * from './project-idea.js'; +export * from './notifications.js'; diff --git a/packages/types/src/ipc.ts b/packages/types/src/ipc.ts index f7fd313f..024c78bf 100644 --- a/packages/types/src/ipc.ts +++ b/packages/types/src/ipc.ts @@ -49,6 +49,10 @@ export const IPC = { TERMINAL_CLOSE: 'terminal:close', TERMINAL_LIST: 'terminal:list', TERMINAL_CLOSE_ALL: 'terminal:closeAll', + EVENT_AGENT_SESSION_STATUS: 'event:agentSessionStatus', + // ── Notifications ──────────────────────────────────────────────────────── + NOTIFICATION_SHOW: 'notification:show', + EVENT_NOTIFICATION_CLICKED: 'event:notificationClicked', // ── Settings ───────────────────────────────────────────────────────────── SETTINGS_GET: 'settings:get', SETTINGS_SET: 'settings:set', @@ -215,6 +219,7 @@ import type { IssueTrackerPattern } from './issuetracker.js'; import type { ProviderIssue } from './providers.js'; import type { FileTreeNode, FileReadResult } from './files.js'; import type { McpClientId, McpServerStatus, McpConfigWriteResult } from './mcp.js'; +import type { ShowAgentNotificationArgs } from './notifications.js'; /** Shape of one row returned by WORKTREE_GET_META / WORKTREE_SET_META. */ export type WorktreeMetaRow = { @@ -317,6 +322,8 @@ export type IpcMap = { 'terminal:closeForPath': { args: [pathPrefix: string]; result: void }; 'terminal:closeAll': { args: []; result: void }; 'terminal:list': { args: []; result: TerminalInfo[] }; + // ── Notifications ───────────────────────────────────────────────────────── + 'notification:show': { args: [args: ShowAgentNotificationArgs]; result: void }; // ── Settings ────────────────────────────────────────────────────────────── 'settings:get': { args: [key: string]; result: string | null }; 'settings:set': { args: [args: { key: string; value: string }]; result: void }; diff --git a/packages/types/src/notifications.ts b/packages/types/src/notifications.ts new file mode 100644 index 00000000..741b0a5a --- /dev/null +++ b/packages/types/src/notifications.ts @@ -0,0 +1,31 @@ +/** + * Pushed to the renderer when an agent-launched terminal session exits or its + * output has been quiet for a while. The renderer (not main) decides whether + * a background OS notification is warranted — it's the only side that knows + * which worktree/tab is currently in view. + */ +export type AgentSessionStatusEvent = { + id: string; + cwd: string; + agentName: string | null; + reason: 'exited' | 'idle'; +}; + +/** + * Renderer → main request to show a native OS notification. The renderer has + * already decided notifications are enabled and the session is in the + * background; main just owns the `Notification` API and focuses/jumps back + * to the session on click. + */ +export type ShowAgentNotificationArgs = { + title: string; + body: string; + worktreePath: string; + terminalId: string; +}; + +/** Pushed to the renderer when the user clicks an agent-session-finished notification. */ +export type NotificationClickedEvent = { + worktreePath: string; + terminalId: string; +}; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 290b18d3..1e4b0b2c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -372,6 +372,9 @@ importers: typescript: specifier: npm:@typescript/native-preview@7.0.0-dev.20260701.1 version: '@typescript/native-preview@7.0.0-dev.20260701.1' + vitest: + specifier: ^4.1.9 + version: 4.1.9(@types/node@26.1.0)(jsdom@29.1.1(@noble/hashes@2.2.0))(vite@8.1.2(@types/node@26.1.0)(esbuild@0.28.1)(jiti@2.7.0)(tsx@4.22.4)(yaml@2.9.0)) packages/ts-config: {} From 5d8e483959ac2becbdeeed209674fc080af556ae Mon Sep 17 00:00:00 2001 From: Liam Russell <17897133+liam-russell@users.noreply.github.com> Date: Tue, 7 Jul 2026 07:25:05 +1000 Subject: [PATCH 2/2] fix: address Copilot review feedback on notifications PR - 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 --- .../renderer/hooks/useAgentSessionNotifications.ts | 2 +- app/src/renderer/settings/NotificationsSection.tsx | 11 +++++++---- packages/terminal/src/terminal-manager.ts | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/app/src/renderer/hooks/useAgentSessionNotifications.ts b/app/src/renderer/hooks/useAgentSessionNotifications.ts index dea84933..d4536de8 100644 --- a/app/src/renderer/hooks/useAgentSessionNotifications.ts +++ b/app/src/renderer/hooks/useAgentSessionNotifications.ts @@ -55,7 +55,7 @@ export function useAgentSessionNotifications(params: { worktreePath: event.cwd, terminalId: event.id, }); - })(); + })().catch(err => console.error('[agent-session-notifications]', err)); }); const offClicked = api.onNotificationClicked((event: NotificationClickedEvent) => { diff --git a/app/src/renderer/settings/NotificationsSection.tsx b/app/src/renderer/settings/NotificationsSection.tsx index 3e81b6ff..cc8dce63 100644 --- a/app/src/renderer/settings/NotificationsSection.tsx +++ b/app/src/renderer/settings/NotificationsSection.tsx @@ -12,18 +12,21 @@ export function NotificationsSection({ onToast }: Props) { const [loaded, setLoaded] = useState(false); useEffect(() => { - void loadAgentSessionNotificationsEnabled().then(value => { - setEnabled(value); - setLoaded(true); - }); + void loadAgentSessionNotificationsEnabled() + .then(value => setEnabled(value)) + .catch(err => onToast(String(err), 'error')) + .finally(() => setLoaded(true)); + // eslint-disable-next-line react-hooks/exhaustive-deps }, []); async function toggle() { + const previous = enabled; const next = !enabled; setEnabled(next); try { await saveAgentSessionNotificationsEnabled(next); } catch (err) { + setEnabled(previous); onToast(String(err), 'error'); } } diff --git a/packages/terminal/src/terminal-manager.ts b/packages/terminal/src/terminal-manager.ts index 8c103f20..f163ea78 100644 --- a/packages/terminal/src/terminal-manager.ts +++ b/packages/terminal/src/terminal-manager.ts @@ -279,7 +279,7 @@ export class TerminalManagerWithMeta extends TerminalManager { return id; } - protected override handleSessionData(id: string): void { + protected override handleSessionData(id: string, _data: string): void { this.idleTracker.touch(id); }