diff --git a/apps/web/src/components/ThreadNotificationCoordinator.badge.browser.test.ts b/apps/web/src/components/ThreadNotificationCoordinator.badge.browser.test.ts index ae2ed3d10de3c..257994ccc76de 100644 --- a/apps/web/src/components/ThreadNotificationCoordinator.badge.browser.test.ts +++ b/apps/web/src/components/ThreadNotificationCoordinator.badge.browser.test.ts @@ -106,6 +106,7 @@ const thread = { archivedAt: null as string | null, hasPendingApprovals: false, hasPendingUserInput: false, + pendingRequestId: "request-1", session: null as { status: string } | null, latestTurn: { turnId: "turn", state: "running", completedAt: null as string | null }, }; @@ -144,9 +145,9 @@ function toV2ThreadShell(input: typeof thread) { ? "failed" : "running", pendingRuntimeRequest: input.hasPendingUserInput - ? { id: "request-1", kind: "user_input", createdAt: SHELL_NOW } + ? { id: input.pendingRequestId, kind: "user_input", createdAt: SHELL_NOW } : input.hasPendingApprovals - ? { id: "request-1", kind: "command", createdAt: SHELL_NOW } + ? { id: input.pendingRequestId, kind: "command", createdAt: SHELL_NOW } : null, latestVisibleMessage: null, latestUserMessageAt: null, @@ -240,6 +241,18 @@ it.each(["hasPendingApprovals", "hasPendingUserInput"] as const)( }, ); +it.each(["hasPendingApprovals", "hasPendingUserInput"] as const)( + "notifies again when a new %s request replaces the pending one in the same run", + async (flag) => { + await using app = setup(); + app.start(); + state.shells.set("one", shell({ [flag]: true, pendingRequestId: "request-1" })); + state.shells.set("one", shell({ [flag]: true, pendingRequestId: "request-2" })); + const title = flag === "hasPendingApprovals" ? "Approval needed" : "Input needed"; + expect(TestNotification.sent.map(({ title }) => title)).toEqual([title, title]); + }, +); + it("presents completion notifications and sound as soon as the shell completes", async () => { await using app = setup(); state.mode = "notifications-and-sound"; diff --git a/apps/web/src/components/ThreadNotificationCoordinator.logic.test.ts b/apps/web/src/components/ThreadNotificationCoordinator.logic.test.ts index a89e4159cae10..1c98f89aa56ca 100644 --- a/apps/web/src/components/ThreadNotificationCoordinator.logic.test.ts +++ b/apps/web/src/components/ThreadNotificationCoordinator.logic.test.ts @@ -7,11 +7,13 @@ import { const working: ThreadNotificationObservation = { attention: null, + requestId: null, settled: false, completedAt: null, }; const completed = (at: number): ThreadNotificationObservation => ({ attention: null, + requestId: null, settled: true, completedAt: at, }); @@ -54,17 +56,37 @@ describe("createThreadNotificationGate", () => { expect(notify).toHaveBeenCalledTimes(2); }); - it("notifies attention once and still notifies the subsequent completion", () => { + it("notifies each attention once per stretch and still notifies the subsequent completion", () => { const { notify, observe } = harness(); observe(working); - const attention = { attention: "run-1:approval", settled: false, completedAt: null }; + const attention = { + attention: "run-1:approval:a", + requestId: "a", + settled: false, + completedAt: null, + }; observe(attention); expect(notify.mock.calls).toEqual([["thread", "input"]]); observe({ ...attention }); expect(notify).toHaveBeenCalledTimes(1); + observe({ ...attention, attention: "run-1:approval:b", requestId: "b" }); + observe(attention); + expect(notify).toHaveBeenCalledTimes(2); + observe(working); + observe(attention); + expect(notify).toHaveBeenCalledTimes(3); + const subagent = { ...attention, attention: "run-1:approval:", requestId: null }; + observe(subagent); + observe(attention); + observe(subagent); + expect(notify).toHaveBeenCalledTimes(5); observe(working); observe(completed(1)); expect(notify.mock.calls).toEqual([ + ["thread", "input"], + ["thread", "input"], + ["thread", "input"], + ["thread", "input"], ["thread", "input"], ["thread", "completion"], ]); @@ -74,7 +96,7 @@ describe("createThreadNotificationGate", () => { const { notify, observe } = harness(); observe(working); observe(completed(1)); - observe({ attention: "run-1:limited", settled: false, completedAt: null }); + observe({ attention: "run-1:limited", requestId: null, settled: false, completedAt: null }); observe(completed(1)); expect(notify.mock.calls).toEqual([ ["thread", "completion"], diff --git a/apps/web/src/components/ThreadNotificationCoordinator.logic.ts b/apps/web/src/components/ThreadNotificationCoordinator.logic.ts index 1fe126ad37da3..6414f2e88f1a3 100644 --- a/apps/web/src/components/ThreadNotificationCoordinator.logic.ts +++ b/apps/web/src/components/ThreadNotificationCoordinator.logic.ts @@ -1,8 +1,10 @@ export type ThreadNotificationKind = "input" | "completion"; export interface ThreadNotificationObservation { - /** Run and status key while the thread needs attention, otherwise null. */ + /** Run, status and request key while the thread needs attention, otherwise null. */ readonly attention: string | null; + /** The pending request's id, when it has one; null whenever attention is. */ + readonly requestId: string | null; /** The thread is idle and its latest run completed. */ readonly settled: boolean; /** When the latest run completed, if it did. */ @@ -17,6 +19,24 @@ export function createThreadNotificationGate( const skip = (key: Key, at: number | null) => { if (at !== null && at > (handled.get(key) ?? -Infinity)) handled.set(key, at); }; + // Attention keys with a request id seen since the thread last stopped needing attention. + const attended = new Map>(); + const attend = ( + key: Key, + prior: ThreadNotificationObservation | undefined, + next: ThreadNotificationObservation, + ): boolean => { + if (next.attention === null) { + attended.delete(key); + return false; + } + if (next.requestId === null) return next.attention !== prior?.attention; + const seen = attended.get(key); + if (seen?.has(next.attention)) return false; + if (seen) seen.add(next.attention); + else attended.set(key, new Set([next.attention])); + return true; + }; return { observe( key: Key, @@ -26,8 +46,9 @@ export function createThreadNotificationGate( ): void { // The first sighting, archived threads, and stopped or silent runs never notify completion. if (!prior || options.archived || options.quiet) skip(key, next.completedAt); + const fresh = attend(key, prior, next); if (!prior || options.archived) return; - if (next.attention && next.attention !== prior.attention) { + if (fresh) { notify(key, "input"); return; } @@ -39,9 +60,11 @@ export function createThreadNotificationGate( /** Forgets threads that left the snapshot. */ retain(keys: ReadonlySet): void { for (const key of handled.keys()) if (!keys.has(key)) handled.delete(key); + for (const key of attended.keys()) if (!keys.has(key)) attended.delete(key); }, clear(): void { handled.clear(); + attended.clear(); }, }; } diff --git a/apps/web/src/components/ThreadNotificationCoordinator.tsx b/apps/web/src/components/ThreadNotificationCoordinator.tsx index a7134b481d131..4d772458a028e 100644 --- a/apps/web/src/components/ThreadNotificationCoordinator.tsx +++ b/apps/web/src/components/ThreadNotificationCoordinator.tsx @@ -30,6 +30,7 @@ import { threadNotificationTag, unlockNotificationAudio, } from "../threadNotifications"; +import { resolveBackgroundThreadRequest } from "./backgroundThreadAnnouncement"; import { resolveSidebarThreadStatus, type SidebarThreadStatus } from "./Sidebar.logic"; import { createThreadNotificationGate, @@ -252,14 +253,16 @@ export function startThreadNotifications( const thread = presentThreadShell(environmentId, rawThread, subagentRequest); let status = resolveSidebarThreadStatus(thread); if (status === "ready" && thread.latestRun?.status === "failed") status = "failed"; - const attention = + const needsAttention = status === "input" || status === "request" || status === "approval" || status === "failed" || - status === "limited" - ? `${thread.latestRun?.runId ?? ""}:${status}` - : null; + status === "limited"; + const requestId = needsAttention ? resolveBackgroundThreadRequest(thread).requestId : null; + const attention = needsAttention + ? `${thread.latestRun?.runId ?? ""}:${status}:${requestId ?? ""}` + : null; const completedAt = Date.parse(thread.latestRun?.completedAt ?? ""); const settled = status === "ready" && thread.latestRun?.status === "completed"; const state = { @@ -268,6 +271,7 @@ export function startThreadNotifications( thread, status, attention, + requestId, settled, completedAt: settled && Number.isFinite(completedAt) ? completedAt : null, };