diff --git a/src/client/src/components/MachineSwitcher.ts b/src/client/src/components/MachineSwitcher.ts index da7b7d2..a271c07 100644 --- a/src/client/src/components/MachineSwitcher.ts +++ b/src/client/src/components/MachineSwitcher.ts @@ -16,6 +16,7 @@ export class MachineSwitcher extends LitElement implements KeyboardNavigableSect @property({ attribute: false }) statuses: Record = {}; @property({ attribute: false }) activities: Record> = {}; @property({ attribute: false }) notificationBadges: Record = {}; + @property({ attribute: false }) notificationHeadingBadge?: SessionNotificationBadgeModel; @property({ attribute: false }) onSelect?: (machine: Machine) => void | Promise; @property({ attribute: false }) onRemove?: (machine: Machine) => void | Promise; @property({ attribute: false }) onFocusNextSection?: () => void | Promise; @@ -57,13 +58,14 @@ export class MachineSwitcher extends LitElement implements KeyboardNavigableSect if (selected === undefined) return null; const status = machineStatus(selected, this.statuses); const label = selected.name; + const notificationBadge = machineSwitcherNotificationBadge(selected.id, this.notificationBadges, this.notificationHeadingBadge); return html`
${this.open ? html` @@ -136,9 +138,9 @@ export class MachineSwitcher extends LitElement implements KeyboardNavigableSect return this.selected ?? this.machines.find((machine) => machine.id === "local") ?? this.machines[0]; } - private machineSwitcherAriaLabel(machine: Machine): string { - const notificationLabel = this.notificationBadges[machine.id]?.accessibleLabel; - return `Machine: ${machine.name}.${notificationLabel === undefined ? "" : ` ${notificationLabel}.`} Switch machine.`; + private machineSwitcherAriaLabel(machine: Machine, notificationBadge: SessionNotificationBadgeModel | undefined): string { + const notificationLabel = notificationBadge?.accessibleLabel; + return `Machine: ${machine.name}.${notificationLabel === undefined ? "" : ` Notifications across machines: ${notificationLabel}.`} Switch machine.`; } private switcherButton(): HTMLElement | null { @@ -312,6 +314,14 @@ export class MachineSwitcher extends LitElement implements KeyboardNavigableSect `; } +export function machineSwitcherNotificationBadge( + selectedMachineId: string, + notificationBadges: Readonly>, + notificationHeadingBadge: SessionNotificationBadgeModel | undefined, +): SessionNotificationBadgeModel | undefined { + return notificationHeadingBadge ?? notificationBadges[selectedMachineId]; +} + function machineStatus(machine: Machine, statuses: Record): MachineStatus { return statuses[machine.id]?.status ?? machine.status ?? "unknown"; } diff --git a/src/client/src/components/PiWebApp.notifications.test.ts b/src/client/src/components/PiWebApp.notifications.test.ts index 2daadf0..c242f81 100644 --- a/src/client/src/components/PiWebApp.notifications.test.ts +++ b/src/client/src/components/PiWebApp.notifications.test.ts @@ -2,6 +2,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { initialAppState, type AppState } from "../appState"; import type { SessionInfo } from "../api"; import type { NavigationNotificationBadges } from "./appShell/AppNavigationPanel"; +import { machineSwitcherNotificationBadge } from "./MachineSwitcher"; import { PiWebApp } from "./PiWebApp"; const currentSession: SessionInfo = { @@ -83,6 +84,8 @@ describe("PiWebApp notification hierarchy models", () => { expect(badges.projects["project-1"]).toMatchObject({ text: "3+", severity: "error" }); expect(badges.machines["local"]).toMatchObject({ text: "3+", severity: "error" }); expect(badges.machines["remote"]).toMatchObject({ text: "5", severity: "info" }); + expect(badges.machinesHeading).toMatchObject({ text: "8+", severity: "error" }); + expect(machineSwitcherNotificationBadge("local", badges.machines, badges.machinesHeading)).toBe(badges.machinesHeading); expect(badges.sessionsHeading).toMatchObject({ text: "2", severity: "error" }); expect(mobile).toMatchObject({ text: "8+", severity: "error" }); }); diff --git a/src/client/src/components/appShell/AppNavigationPanel.ts b/src/client/src/components/appShell/AppNavigationPanel.ts index 8799cfa..3f14a55 100644 --- a/src/client/src/components/appShell/AppNavigationPanel.ts +++ b/src/client/src/components/appShell/AppNavigationPanel.ts @@ -116,6 +116,7 @@ export class AppNavigationPanel extends LitElement { .statuses=${this.machineStatuses} .activities=${this.machineActivities} .notificationBadges=${this.notificationBadges.machines} + .notificationHeadingBadge=${this.notificationBadges.machinesHeading} .onSelect=${(machine: Machine) => this.onSelectMachine?.(machine)} .onRemove=${(machine: Machine) => this.onRemoveMachine?.(machine)} .onFocusNextSection=${() => { this.focusNextFrom("machines"); }} diff --git a/src/client/src/controllers/sessionNotificationController.test.ts b/src/client/src/controllers/sessionNotificationController.test.ts index e6ffda0..3557228 100644 --- a/src/client/src/controllers/sessionNotificationController.test.ts +++ b/src/client/src/controllers/sessionNotificationController.test.ts @@ -337,6 +337,31 @@ describe("SessionNotificationController capability and joins", () => { }); describe("SessionNotificationController optimistic mutations", () => { + it("does not let a delayed refresh snapshot roll back a newer dismissal response", async () => { + const initial = inboxSnapshot([entry(1)], { inboxRevision: 1, catalogRevision: 1 }); + const delayedRefresh = deferred(); + const notificationInbox = vi.fn() + .mockResolvedValueOnce(initial) + .mockImplementationOnce(() => delayedRefresh.promise); + const dismissed = inboxSnapshot([], { inboxRevision: 2, catalogRevision: 2 }); + const harness = createHarness(capableState(), { + notificationInbox, + dismissNotification: vi.fn(() => Promise.resolve(dismissed)), + }); + + harness.controller.prepareSelectedSession(session, "local"); + await harness.controller.refreshSelectedSession(session, "local"); + const refresh = harness.controller.refreshSelectedSession(session, "local"); + await harness.controller.dismissNotification("daemon-a:1"); + + expect(selectedNotificationView(harness.state.selectedNotificationInbox)?.notifications).toEqual([]); + delayedRefresh.resolve(initial); + await refresh; + + expect(harness.state.selectedNotificationInbox?.summary?.inboxRevision).toBe(2); + expect(selectedNotificationView(harness.state.selectedNotificationInbox)?.notifications).toEqual([]); + }); + it("optimistically dismisses one card, reconciles the response, and rolls back/refetches on failure", async () => { const dismiss = deferred(); const refreshAfterFailure = deferred(); diff --git a/src/client/src/controllers/sessionNotificationController.ts b/src/client/src/controllers/sessionNotificationController.ts index 80f633e..da523ec 100644 --- a/src/client/src/controllers/sessionNotificationController.ts +++ b/src/client/src/controllers/sessionNotificationController.ts @@ -301,7 +301,10 @@ export class SessionNotificationController { return; } this.acceptedSupportByMachine.add(target.machineId); - let inbox = installSelectedNotificationSnapshot(this.getState().selectedNotificationInbox, target, snapshot); + const current = this.getState().selectedNotificationInbox; + let inbox = current === undefined || shouldInstallSelectedSnapshot(current, target, snapshot) + ? installSelectedNotificationSnapshot(current, target, snapshot) + : current; const catalogEvents = [snapshotSummaryEvent(snapshot)]; for (const event of [...join.events].sort((left, right) => left.summary.inboxRevision - right.summary.inboxRevision)) { const result = applySelectedNotificationEvent(inbox, target, event); @@ -389,10 +392,9 @@ export class SessionNotificationController { this.setState({ selectedNotificationInbox: { ...removeOverlay(current), status: "stale" } }); return; } - const shouldInstall = current.daemonInstanceId !== snapshot.daemonInstanceId - || current.summary === undefined - || snapshot.summary.inboxRevision >= current.summary.inboxRevision; - const authoritative = shouldInstall ? installSelectedNotificationSnapshot(current, target, snapshot) : current; + const authoritative = shouldInstallSelectedSnapshot(current, target, snapshot) + ? installSelectedNotificationSnapshot(current, target, snapshot) + : current; this.setState({ selectedNotificationInbox: removeOverlay(authoritative) }); this.applyCatalogSummary(target.machineId, snapshotSummaryEvent(snapshot)); } @@ -578,6 +580,17 @@ function targetFromInbox(inbox: SelectedSessionNotificationInbox): SessionNotifi return { machineId: inbox.machineId, sessionId: inbox.sessionId, cwd: inbox.cwd }; } +function shouldInstallSelectedSnapshot( + current: SelectedSessionNotificationInbox, + target: SessionNotificationTarget, + snapshot: SessionNotificationInboxSnapshot, +): boolean { + return !notificationTargetsEqual(current, target) + || current.daemonInstanceId !== snapshot.daemonInstanceId + || current.summary === undefined + || snapshot.summary.inboxRevision >= current.summary.inboxRevision; +} + async function forEachWithConcurrency(items: readonly T[], concurrency: number, worker: (item: T) => Promise): Promise { let nextIndex = 0; async function run(): Promise { diff --git a/src/server/sessions/piSessionService.lifecycle.test.ts b/src/server/sessions/piSessionService.lifecycle.test.ts index cafcde0..8de38b0 100644 --- a/src/server/sessions/piSessionService.lifecycle.test.ts +++ b/src/server/sessions/piSessionService.lifecycle.test.ts @@ -1,6 +1,6 @@ import { mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { join, resolve, sep } from "node:path"; import { describe, expect, it, vi } from "vitest"; import { PiSessionService, type PiAgentSession, type PiSessionRuntime } from "./piSessionService.js"; import { SessionNotificationStore } from "./sessionNotificationStore.js"; @@ -404,8 +404,10 @@ describe("PiSessionService lifecycle, listing, and reload", () => { const hub = new CapturingSessionEventHub(); const store = notificationStore(); const branch = [{ type: "message", message: { role: "user", content: "existing" } }]; + const canonicalCwd = resolve(tmpdir(), "pi-web-notification-workspace"); + const rawEquivalentCwd = `${canonicalCwd}${sep}nested${sep}..`; const fake = fakeRuntime("notification-session", { - sessionManager: fakeSessionManager("/workspace", { + sessionManager: fakeSessionManager(rawEquivalentCwd, { getSessionId: () => "notification-session", getBranch: () => branch, }), @@ -419,12 +421,13 @@ describe("PiSessionService lifecycle, listing, and reload", () => { heartbeatIntervalMs: 60_000, }); - await service.start("/workspace"); + await service.start(canonicalCwd); const notify = boundNotify(fake); notify("duplicate", "warning"); notify("duplicate", "error"); - const snapshot = service.notificationInbox(sessionRef("notification-session")); + const snapshot = service.notificationInbox({ id: "notification-session", cwd: canonicalCwd }); + expect(snapshot.summary.cwd).toBe(canonicalCwd); expect(snapshot.notifications).toMatchObject([ { id: "daemon-lifecycle-test:2", message: "duplicate", severity: "error" }, { id: "daemon-lifecycle-test:1", message: "duplicate", severity: "warning" }, diff --git a/src/server/sessions/piSessionService.ts b/src/server/sessions/piSessionService.ts index 4ef1c62..7322a83 100644 --- a/src/server/sessions/piSessionService.ts +++ b/src/server/sessions/piSessionService.ts @@ -1429,10 +1429,7 @@ export class PiSessionService implements SessionRouteService { try { await session.reload(priorGeneration === undefined ? undefined : { beforeSessionStart: () => { - candidateGeneration = this.notificationStore.beginReplacement(priorGeneration, { - sessionId: session.sessionId, - cwd: session.sessionManager.getCwd(), - }); + candidateGeneration = this.notificationStore.beginReplacement(priorGeneration, notificationIdentityForSession(session)); this.notificationGenerationBySession.set(session, candidateGeneration); this.replaceSessionNotificationContext(session, candidateGeneration); }, @@ -1629,8 +1626,7 @@ export class PiSessionService implements SessionRouteService { if (this.hasActiveWork(session)) throw new Error("Stop current session activity before reloading"); const priorGeneration = this.notificationGenerationBySession.get(session); - const sessionId = session.sessionId; - const cwd = session.sessionManager.getCwd(); + const { sessionId, cwd } = notificationIdentityForSession(session); let candidateGeneration: SessionNotificationGeneration | undefined; try { await this.closeActive( @@ -2016,17 +2012,18 @@ export class PiSessionService implements SessionRouteService { : "external"; if (notificationOwnership === "registered") { + const notificationIdentity = notificationIdentityForSession(runtime.session); const existingCandidate = this.notificationStore.beginReplacementForSession( - runtime.session.sessionId, - runtime.session.sessionManager.getCwd(), + notificationIdentity.sessionId, + notificationIdentity.cwd, ); if (existingCandidate !== undefined) { notificationGeneration = existingCandidate; notificationOwnership = "replacement"; } else { const registration = this.notificationStore.registerSession( - runtime.session.sessionId, - runtime.session.sessionManager.getCwd(), + notificationIdentity.sessionId, + notificationIdentity.cwd, ); notificationGeneration = registration.generation; this.publishNotificationMutations(registration.mutations); @@ -2042,10 +2039,7 @@ export class PiSessionService implements SessionRouteService { let candidateGeneration: SessionNotificationGeneration | undefined; try { if (priorGeneration !== undefined) { - candidateGeneration = this.notificationStore.beginReplacement(priorGeneration, { - sessionId: session.sessionId, - cwd: session.sessionManager.getCwd(), - }); + candidateGeneration = this.notificationStore.beginReplacement(priorGeneration, notificationIdentityForSession(session)); this.notificationGenerationBySession.set(session, candidateGeneration); } this.bindRuntime(active, session); @@ -2512,6 +2506,13 @@ function modelToClientModel(model: PiAgentSession["model"]): ClientSessionModel }; } +function notificationIdentityForSession(session: PiAgentSession): { sessionId: string; cwd: string } { + return { + sessionId: session.sessionId, + cwd: canonicalizeStoredCwd(session.sessionManager.getCwd()), + }; +} + function clientSessionFromListEntry(session: PiSessionListEntry): ClientSession { return { id: session.id,