From 9f35f4ccbb5b93446a92469f603a935f0bb6b658 Mon Sep 17 00:00:00 2001 From: spoukyii <61633921+spoukyii@users.noreply.github.com> Date: Fri, 6 Nov 2026 02:11:06 -0800 Subject: [PATCH 1/2] test(web): finished review guide notifications fit the Windows toast tag limit --- .../PullRequestGuideRunCoordinator.test.ts | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/apps/web/src/components/pullRequest/guide/PullRequestGuideRunCoordinator.test.ts b/apps/web/src/components/pullRequest/guide/PullRequestGuideRunCoordinator.test.ts index a764c3c221d34..cd481e2212847 100644 --- a/apps/web/src/components/pullRequest/guide/PullRequestGuideRunCoordinator.test.ts +++ b/apps/web/src/components/pullRequest/guide/PullRequestGuideRunCoordinator.test.ts @@ -5,9 +5,14 @@ import { type PullRequestRef, type PullRequestReviewGuideResult, } from "@t3tools/contracts"; +import { DEFAULT_CLIENT_SETTINGS } from "@t3tools/contracts/settings"; import { AsyncResult, Atom, AtomRegistry } from "effect/reactivity"; import { expect, it, vi } from "vite-plus/test"; +import { + __resetClientSettingsPersistenceForTests, + __setClientSettingsForTests, +} from "~/hooks/useSettings"; import { guideRunKey, useGuideRunStore } from "./guideRunStore"; import { startGuideRunNotifications } from "./PullRequestGuideRunCoordinator"; @@ -120,3 +125,51 @@ it("does not report the previous run's failure when a Running answer starts a wa expect(useGuideRunStore.getState().watches[key]).toBe(undefined); stop(); }); + +it("gives each finished guide's notification its own tag within the Windows toast limit", () => { + using _cleanup = { + [Symbol.dispose]: () => { + vi.unstubAllGlobals(); + __resetClientSettingsPersistenceForTests(); + }, + }; + const tags: string[] = []; + vi.stubGlobal("document", { visibilityState: "hidden", hasFocus: () => false }); + vi.stubGlobal( + "Notification", + class { + static permission = "granted"; + constructor(_title: string, options: NotificationOptions) { + tags.push(options.tag!); + } + addEventListener() {} + }, + ); + __setClientSettingsForTests({ ...DEFAULT_CLIENT_SETTINGS, notificationMode: "notifications" }); + const registry = AtomRegistry.make(); + const stop = startGuideRunNotifications(registry, { navigate: vi.fn() } as unknown as AnyRouter); + const uuidEnvironmentId = EnvironmentId.make("fd6fff30-f90b-4ee7-9812-4cf497791b90"); + const written = { + guide: { generatedAt: "2026-10-01T00:00:00.000Z" }, + run: null, + } as unknown as PullRequestReviewGuideResult; + for (const number of [1234, 1235]) { + registry.set(guideAtom(`${uuidEnvironmentId}:${number}`), AsyncResult.success(written)); + useGuideRunStore.getState().watch({ + environmentId: uuidEnvironmentId, + reference: { + projectId: ProjectId.make("a8395a98-a186-4fe7-9d4c-fb8ba63b7588"), + repository: "pingdotgg/t3code", + number, + }, + title: null, + previousGuideAt: null, + previousFailedAt: null, + since: 0, + }); + } + expect(tags).toHaveLength(2); + for (const tag of tags) expect(tag.length).toBeLessThanOrEqual(64); + expect(new Set(tags).size).toBe(2); + stop(); +}); From 30eaaef27b2c3010f30ab91e483c96a1479d0510 Mon Sep 17 00:00:00 2001 From: spoukyii <61633921+spoukyii@users.noreply.github.com> Date: Fri, 6 Nov 2026 02:13:19 -0800 Subject: [PATCH 2/2] fix(web): finished review guide notifications reach Windows The desktop notification for a finished or failed review guide used `guide:${runKey}` as its tag. The run key is the environment id plus the JSON pull request key (project id, host, repository, number), so a real pull request gives a tag of about 110 characters. Windows drops a toast whose tag is longer than 64 characters, the same reason #12289 hashed the thread notification tag. The hash in threadNotifications.ts now takes any key, and both the thread and the guide notifications pass their key through it. Thread tags are unchanged; guide tags become the same 16-character hex form, still one per pull request, so a repeat alert for the same guide replaces the old one. Bug: a finished review guide never shows a Windows notification, because its notification tag is longer than Windows allows --- apps/web/src/components/ThreadNotificationCoordinator.tsx | 4 ++-- .../pullRequest/guide/PullRequestGuideRunCoordinator.tsx | 7 ++++++- apps/web/src/threadNotifications.ts | 6 ++---- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/apps/web/src/components/ThreadNotificationCoordinator.tsx b/apps/web/src/components/ThreadNotificationCoordinator.tsx index a7134b481d131..10b7ecdc9d965 100644 --- a/apps/web/src/components/ThreadNotificationCoordinator.tsx +++ b/apps/web/src/components/ThreadNotificationCoordinator.tsx @@ -27,7 +27,7 @@ import { hasNotificationSound, playNotificationSound, setNotificationBadge, - threadNotificationTag, + notificationTag, unlockNotificationAudio, } from "../threadNotifications"; import { resolveSidebarThreadStatus, type SidebarThreadStatus } from "./Sidebar.logic"; @@ -196,7 +196,7 @@ export function startThreadNotifications( try { const notification = new Notification(title, { body: thread.title, - tag: threadNotificationTag(environmentId, thread.id), + tag: notificationTag(`${environmentId}:${thread.id}`), silent: true, // Windows drops clicks after a toast moves into Notification Center. requireInteraction: kind === "completion" && isWindowsPlatform(navigator.platform), diff --git a/apps/web/src/components/pullRequest/guide/PullRequestGuideRunCoordinator.tsx b/apps/web/src/components/pullRequest/guide/PullRequestGuideRunCoordinator.tsx index f73755ed7d452..789ff522f2e3b 100644 --- a/apps/web/src/components/pullRequest/guide/PullRequestGuideRunCoordinator.tsx +++ b/apps/web/src/components/pullRequest/guide/PullRequestGuideRunCoordinator.tsx @@ -8,6 +8,7 @@ import { observedPullRequestTitle, pullRequestEnvironment } from "~/state/pullRe import { hasDesktopNotifications, hasNotificationSound, + notificationTag, playNotificationSound, } from "~/threadNotifications"; @@ -145,7 +146,11 @@ function watchGuideRun( ) return; try { - const notification = new Notification(title, { body, tag: `guide:${runKey}`, silent: true }); + const notification = new Notification(title, { + body, + tag: notificationTag(`guide:${runKey}`), + silent: true, + }); notification.addEventListener("click", () => { notification.close(); window.focus(); diff --git a/apps/web/src/threadNotifications.ts b/apps/web/src/threadNotifications.ts index 8784f79baceef..697bef640847f 100644 --- a/apps/web/src/threadNotifications.ts +++ b/apps/web/src/threadNotifications.ts @@ -1,4 +1,3 @@ -import type { EnvironmentId, ThreadId } from "@t3tools/contracts"; import type { ClientSettings } from "@t3tools/contracts/settings"; import completionUrl from "./assets/notification-completion.mp3"; @@ -24,9 +23,8 @@ const TAG_HASH_OFFSET_BASIS = 0xcbf29ce484222325n; const TAG_HASH_PRIME = 0x100000001b3n; const TAG_HASH_MASK = 0xffffffffffffffffn; -/** Windows drops toast notifications with the 73-character environment/thread tag. */ -export function threadNotificationTag(environmentId: EnvironmentId, threadId: ThreadId): string { - const key = `${environmentId}:${threadId}`; +/** Windows drops toast notifications whose tag is longer than 64 characters. */ +export function notificationTag(key: string): string { let hash = TAG_HASH_OFFSET_BASIS; for (let index = 0; index < key.length; index += 1) { hash ^= BigInt(key.charCodeAt(index));