Skip to content

feat(desktop): notify when a Session's run ends while Pace is in the background - #461

Merged
bubbleptr merged 2 commits into
mainfrom
feat/desktop-notifications
Oct 11, 2026
Merged

bubbleptr merged 2 commits into
mainfrom
feat/desktop-notifications

Conversation

@bubbleptr

@bubbleptr bubbleptr commented Oct 11, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • When the window is not in front (!(document.visibilityState === "visible" && document.hasFocus())) and a Session's run ends, Pace shows a system notification through the renderer Web Notification API. The behavior matches PiNomad's feat/web-notifications, minus the parts Pace has no state for (see below).
  • Triggered by status changes, not snapshots. diffSessionNotices(previous, sessions) is a pure function. A Session that was running and is now completed/waiting (idle) gets Finished; one that is now failed gets Failed. The first list only seeds the baseline. A Session with no entry in the previous baseline, or one leaving creating, never notifies: a fork lands as creating and resyncs to the source's end status without running anything.
  • Title is the sidebar title, tag is the Session id (a newer notice replaces the older one), and clicking runs window.focus(), opens that Session, and closes the notice.
  • One app-level mount. <SessionNotifications> lives in app/main.tsx inside SessionProjectionsProvider, so the baseline survives route changes and runs that end while you're on Usage/Packages/Trajectory still notify. The logic lives in a new features/session-notifications slice (ADR-0045).
  • Opening a Session from another page. The target travels as ?session=<id>. AgentWorkspaceSessionsPage selects it, strips the param, and holds back its first-session fallback while a request is pending. Notification clicks and sidebar clicks (AppFrame.handleOpenSession) both use it. This also fixes an existing bug: clicking a sidebar Session from a non-Sessions page used to open the project's first Session instead.
  • Docs: CONTEXT.md adds a Session Notification entry, and the "代码在哪里" table in docs/architecture.md gets a row.

Differences from the shared spec

  • No Waiting for your answer notice. Pace has no state for "waiting on the user". The SDK driver calls session.bindExtensions({ onError }) with no uiContext, so Pi's extension runner uses noOpUIContext: ctx.ui.select/confirm/input resolve to undefined/false immediately and never block a run. This notice can be added once extension UI requests reach the Runtime Gateway (ADR-0018).

  • No subagent rules. In Pace, subagents are SubagentRecords on the parent Session's runtime model, not separate Sessions in the projection store. Their run ends therefore never notify, which matches the spec, and they have no waiting state.

  • No permission request in the renderer. The main window uses the default session, and no setPermissionRequestHandler/setPermissionCheckHandler is registered on it. I probed Electron 42.5.0 with the same webPreferences: Notification.permission was "granted" on both file:// and http://localhost.

  • macOS system authorization (known, intentionally not handled here). Electron 42 moved to UNNotification (electron#47817). NotificationPresenterMac calls requestAuthorizationWithOptions when it is created, which happens lazily on the first notification. That means:

    • The macOS permission prompt appears with the first notification, and that notification may not show.
    • If the user denies at the OS level, the renderer still sees "granted" and gets no error event.
    • Notifications require a code-signed app: the signed DMG is fine, package:mac:unsigned builds won't show them, and the dev Electron.app is only ad-hoc signed.

    To enable: System Settings → Notifications → Pace.

Test plan

  • session-notices.test.ts: running→completed/idle gives Finished; running→failed gives Failed; no notice out of creating (fork/creation), for waiting→running or completed→completed, on the first snapshot, or for a Session first seen already ended; several Sessions ending in one update each notify; Sessions that disappear leave the baseline
  • use-session-notifications.test.tsx (global Notification and document.hasFocus stubbed): no notice while focused; while unfocused, one notice with title/body/tag, and clicking calls window.focus, opens the Session, and closes the notice; no notice when permission isn't granted
  • session-notifications.test.tsx: with the real SessionProjectionsProvider, a store-driven run end notifies and a click hands back { id, projectId }
  • agent-workspace.test.tsx: ?session=<non-first id> selects that Session and the param is removed
  • bun run typecheck / bun run lint / bun run test (173 files / 2120 tests) / bun run build all green

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 3 potential issues.

Devin Review

Comment on lines +1574 to +1579
useSessionNotifications(notificationSources, (sessionId) => {
const session = sessions.find((candidate) => candidate.id === sessionId);
if (session) {
handleOpenSession(session.id, session.projection.projectId);
}
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Notification opens the wrong session

Clicking a notification outside the Sessions page updates handleOpenSession's local selection before navigating. The Sessions page owns separate selection state and opens its first session instead.

Learn more

A notification can arrive while Usage, Packages, or Trajectory renders its own AppFrame. Its session selection lives in that frame, but the Sessions route mounts a different page with its own selectedSessionId state. AgentWorkspaceSessionsPage initializes that state to null and its fallback selection chooses the first session for the project. The notification click therefore focuses Pace and navigates to the right project, but selects the wrong session.

Example: Project P has sessions A and B, with A first in the list. While on Usage, B finishes and its notification is clicked. Pace navigates to P and selects A rather than B.

Recommended fix: Carry the target session ID across the route transition, such as a route search parameter consumed by AgentWorkspaceSessionsPage, or move selected-session state above the routes. Verify clicks from Usage and Trajectory as well as clicks within the Sessions page.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1390c26. The target Session now travels in the URL as ?session=<id>: AppFrame.handleOpenSession and notification clicks both navigate with it. AgentWorkspaceSessionsPage selects that Session, removes the param, and holds the first-session fallback back while a request is pending (otherwise the fallback runs in the same commit with a stale null and wins). The same bug also affected sidebar clicks from non-Sessions pages, so this fixes those too. Test: selects the Session named by the session search param, then strips it. It failed before the fix.

Comment on lines +45 to +50
if (session.status === "completed" || session.status === "waiting") {
notices.push({
sessionId: session.id,
title: session.title,
body: "Finished",
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Idle fork gets false completion notice

When a fork finishes while Pace is backgrounded, diffSessionNotices reports Finished for creating to waiting. Fork creation starts no run, so no run ended.

Learn more

A fork is inserted with status creating, then handleForkMessage applies its idle runtime snapshot via runtime-state-resynced without sending a prompt. The projection reducer maps that idle state to waiting. The notification diff treats this transition as a finished run although the new fork only copied conversation context.

Example: Alice forks a message, switches to another application during checkout, and the new fork becomes idle. Pace announces Finished even though Alice has not submitted its prefilled message.

Recommended fix: Track whether the creating session actually entered an active run, or distinguish fork creation from prompt-driven creation before notifying on creating to waiting. Keep notifications for genuine run completions and creation failures.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1390c26. Only running counts as a run in flight now. A fork is inserted as creating and then resyncs to whatever status the source session ended in, so leaving creating can never mean a run finished. We accept the trade-off: a creation that fails before its first run no longer notifies. Tests cover creating→waiting/completed/failed producing no notice.

sessions: readonly SessionNotificationSource[],
onOpen: (sessionId: string) => void,
): void {
const baselineRef = useRef<SessionStatusBaseline | undefined>(undefined);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Run ending during navigation goes unnoticed

When a run ends between page mounts, baselineRef resets and the new frame seeds its finished status silently. No notification appears for that run.

Learn more

Each route page renders a separate AppFrame, while the projection store persists above the router. Changing routes unmounts the notification hook and loses its status baseline. If a run ends after the old frame unmounts but before the new frame effects run, the new hook treats the completed status as its first snapshot and suppresses the notification.

Example: Session A is running while Alice navigates from Sessions to Usage. A completes during the route transition, after the Sessions frame unmounts but before Usage mounts. Usage seeds completed and shows no notice.

Recommended fix: Mount the watcher once above the route outlet, alongside SessionProjectionsProvider, or keep its baseline in an app-scoped store. Preserve first-hydration suppression while retaining observed running states through navigation.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1390c26. The hook moved out of AppFrame into a single app-level <SessionNotifications> in app/main.tsx (inside SessionProjectionsProvider), so the baseline survives route changes. Clicking a notification navigates with the same ?session= param as above. Covered by session-notifications.test.tsx, which uses the real provider and store.

@bubbleptr
bubbleptr merged commit 81aed93 into main Oct 11, 2026
1 check passed
@bubbleptr
bubbleptr deleted the feat/desktop-notifications branch October 11, 2026 11:06
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