From d495a4eddb967455b18253e943f3026a7f8f07ea Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Wed, 11 Mar 2026 16:27:59 -0700 Subject: [PATCH] fix(site): deduplicate agent chime across browser tabs (#22972) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem When multiple tabs are open on `/agents`, every tab receives the same WebSocket status transitions and independently plays the completion chime — resulting in overlapping sounds. ## Fix Use the [Web Locks API](https://developer.mozilla.org/en-US/docs/Web/API/Web_Locks_API) (`navigator.locks`) to coordinate across tabs. When a tab decides a chime should play: 1. It calls `navigator.locks.request(lockName, { ifAvailable: true }, callback)`. 2. Only the first tab to acquire the per-chatID lock plays the sound. 3. The lock is held for 2 seconds, covering any reasonable WebSocket delivery skew between tabs. 4. Other tabs get `lock === null` and silently skip. Falls back to immediate playback (original behavior) when the Web Locks API is unavailable. --- .../AgentDetail/useAgentChime.test.ts | 180 +++++++++++++----- .../AgentsPage/AgentDetail/useAgentChime.ts | 63 +++++- 2 files changed, 198 insertions(+), 45 deletions(-) diff --git a/site/src/pages/AgentsPage/AgentDetail/useAgentChime.test.ts b/site/src/pages/AgentsPage/AgentDetail/useAgentChime.test.ts index 2e5dc16ef1..cbc110fb6b 100644 --- a/site/src/pages/AgentsPage/AgentDetail/useAgentChime.test.ts +++ b/site/src/pages/AgentsPage/AgentDetail/useAgentChime.test.ts @@ -1,10 +1,40 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { getChimeEnabled, + LOCK_HOLD_MS, maybePlayChime, setChimeEnabled, } from "./useAgentChime"; +// --------------------------------------------------------------------------- +// navigator.locks mock +// --------------------------------------------------------------------------- + +// jsdom does not provide navigator.locks, so we supply a minimal +// in-process implementation that mirrors the real Web Locks API +// semantics used by useAgentChime: request() with ifAvailable. + +class MockLockManager { + private held = new Set(); + + async request( + name: string, + options: LockOptions, + callback: (lock: Lock | null) => Promise, + ): Promise { + if (options.ifAvailable && this.held.has(name)) { + await callback(null); + return; + } + this.held.add(name); + try { + await callback({ name, mode: "exclusive" } as Lock); + } finally { + this.held.delete(name); + } + } +} + // --------------------------------------------------------------------------- // Preference helpers // --------------------------------------------------------------------------- @@ -45,98 +75,162 @@ describe("getChimeEnabled / setChimeEnabled", () => { describe("maybePlayChime", () => { let playSpy: ReturnType; + let mockLocks: MockLockManager; beforeEach(() => { + vi.useFakeTimers(); localStorage.clear(); + + mockLocks = new MockLockManager(); + Object.defineProperty(navigator, "locks", { + value: mockLocks, + writable: true, + configurable: true, + }); + playSpy = vi .spyOn(HTMLMediaElement.prototype, "play") .mockResolvedValue(undefined); }); afterEach(() => { + vi.useRealTimers(); vi.restoreAllMocks(); }); + // Helper: trigger maybePlayChime and flush the microtask + // queue so the async navigator.locks.request() callback + // runs, then advance past the LOCK_HOLD_MS hold period. + async function triggerAndSettle( + prev: string | undefined, + next: string, + chatID: string, + activeChatID: string | undefined, + ): Promise { + maybePlayChime(prev, next, chatID, activeChatID); + // Flush the microtask queue so the lock callback executes. + await vi.advanceTimersByTimeAsync(LOCK_HOLD_MS + 50); + } + // -- Chime SHOULD play -- - it("chimes on running → waiting when viewing a different chat", () => { + it("chimes on running → waiting when viewing a different chat", async () => { vi.spyOn(document, "hidden", "get").mockReturnValue(false); - maybePlayChime("running", "waiting", "chat-1", "chat-2"); + await triggerAndSettle("running", "waiting", "chat-1", "chat-2"); expect(playSpy).toHaveBeenCalledTimes(1); }); - it("chimes on running → pending when viewing a different chat", () => { + it("chimes on running → pending when viewing a different chat", async () => { vi.spyOn(document, "hidden", "get").mockReturnValue(false); - maybePlayChime("running", "pending", "chat-1", "chat-2"); + await triggerAndSettle("running", "pending", "chat-1", "chat-2"); expect(playSpy).toHaveBeenCalledTimes(1); }); - it("chimes on pending → waiting (watchChats skips running)", () => { + it("chimes on pending → waiting (watchChats skips running)", async () => { vi.spyOn(document, "hidden", "get").mockReturnValue(false); - maybePlayChime("pending", "waiting", "chat-1", "chat-2"); + await triggerAndSettle("pending", "waiting", "chat-1", "chat-2"); expect(playSpy).toHaveBeenCalledTimes(1); }); - it("chimes on running → waiting when tab is hidden (same chat)", () => { + it("chimes on running → waiting when tab is hidden (same chat)", async () => { vi.spyOn(document, "hidden", "get").mockReturnValue(true); - maybePlayChime("running", "waiting", "chat-1", "chat-1"); + await triggerAndSettle("running", "waiting", "chat-1", "chat-1"); expect(playSpy).toHaveBeenCalledTimes(1); }); - it("chimes on running → waiting when tab is hidden (no active chat)", () => { + it("chimes on running → waiting when tab is hidden (no active chat)", async () => { vi.spyOn(document, "hidden", "get").mockReturnValue(true); - maybePlayChime("running", "waiting", "chat-1", undefined); + await triggerAndSettle("running", "waiting", "chat-1", undefined); expect(playSpy).toHaveBeenCalledTimes(1); }); // -- Chime should NOT play -- - it("does NOT chime when viewing the finishing chat on a visible tab", () => { + it("does NOT chime when viewing the finishing chat on a visible tab", async () => { vi.spyOn(document, "hidden", "get").mockReturnValue(false); - maybePlayChime("running", "waiting", "chat-1", "chat-1"); + await triggerAndSettle("running", "waiting", "chat-1", "chat-1"); expect(playSpy).not.toHaveBeenCalled(); }); - it("does NOT chime when preference is disabled", () => { + it("does NOT chime when preference is disabled", async () => { setChimeEnabled(false); vi.spyOn(document, "hidden", "get").mockReturnValue(true); + await triggerAndSettle("running", "waiting", "chat-1", "chat-2"); + expect(playSpy).not.toHaveBeenCalled(); + }); + + it("does NOT chime on running → error", async () => { + vi.spyOn(document, "hidden", "get").mockReturnValue(true); + await triggerAndSettle("running", "error", "chat-1", "chat-2"); + expect(playSpy).not.toHaveBeenCalled(); + }); + + it("does NOT chime on waiting → running (wrong direction)", async () => { + vi.spyOn(document, "hidden", "get").mockReturnValue(true); + await triggerAndSettle("waiting", "running", "chat-1", "chat-2"); + expect(playSpy).not.toHaveBeenCalled(); + }); + + it("does NOT chime when previous status is undefined", async () => { + vi.spyOn(document, "hidden", "get").mockReturnValue(true); + await triggerAndSettle(undefined, "waiting", "chat-1", "chat-2"); + expect(playSpy).not.toHaveBeenCalled(); + }); + + it("does NOT chime when status has not changed", async () => { + vi.spyOn(document, "hidden", "get").mockReturnValue(true); + await triggerAndSettle("running", "running", "chat-1", "chat-2"); + expect(playSpy).not.toHaveBeenCalled(); + }); + + it("does NOT chime on error → waiting", async () => { + vi.spyOn(document, "hidden", "get").mockReturnValue(true); + await triggerAndSettle("error", "waiting", "chat-1", "chat-2"); + expect(playSpy).not.toHaveBeenCalled(); + }); + + it("does NOT chime on pending → pending (no change)", async () => { + vi.spyOn(document, "hidden", "get").mockReturnValue(true); + await triggerAndSettle("pending", "pending", "chat-1", "chat-2"); + expect(playSpy).not.toHaveBeenCalled(); + }); + + // -- Cross-tab deduplication -- + + it("second tab is blocked while first tab holds the lock", async () => { + vi.spyOn(document, "hidden", "get").mockReturnValue(true); + + // Simulate two tabs calling maybePlayChime for the same + // chatID. The first acquires the lock; the second sees + // ifAvailable=false and skips. maybePlayChime("running", "waiting", "chat-1", "chat-2"); - expect(playSpy).not.toHaveBeenCalled(); + maybePlayChime("running", "waiting", "chat-1", "chat-2"); + + await vi.advanceTimersByTimeAsync(LOCK_HOLD_MS + 50); + expect(playSpy).toHaveBeenCalledTimes(1); }); - it("does NOT chime on running → error", () => { + it("different chatIDs acquire independent locks", async () => { vi.spyOn(document, "hidden", "get").mockReturnValue(true); - maybePlayChime("running", "error", "chat-1", "chat-2"); - expect(playSpy).not.toHaveBeenCalled(); + + maybePlayChime("running", "waiting", "chat-1", "chat-2"); + maybePlayChime("running", "waiting", "chat-3", "chat-2"); + + await vi.advanceTimersByTimeAsync(LOCK_HOLD_MS + 50); + expect(playSpy).toHaveBeenCalledTimes(2); }); - it("does NOT chime on waiting → running (wrong direction)", () => { - vi.spyOn(document, "hidden", "get").mockReturnValue(true); - maybePlayChime("waiting", "running", "chat-1", "chat-2"); - expect(playSpy).not.toHaveBeenCalled(); - }); + it("falls back to immediate play when navigator.locks is unavailable", async () => { + // Remove the locks API to simulate an older browser. + Object.defineProperty(navigator, "locks", { + value: undefined, + writable: true, + configurable: true, + }); - it("does NOT chime when previous status is undefined", () => { vi.spyOn(document, "hidden", "get").mockReturnValue(true); - maybePlayChime(undefined, "waiting", "chat-1", "chat-2"); - expect(playSpy).not.toHaveBeenCalled(); - }); - - it("does NOT chime when status has not changed", () => { - vi.spyOn(document, "hidden", "get").mockReturnValue(true); - maybePlayChime("running", "running", "chat-1", "chat-2"); - expect(playSpy).not.toHaveBeenCalled(); - }); - - it("does NOT chime on error → waiting", () => { - vi.spyOn(document, "hidden", "get").mockReturnValue(true); - maybePlayChime("error", "waiting", "chat-1", "chat-2"); - expect(playSpy).not.toHaveBeenCalled(); - }); - - it("does NOT chime on pending → pending (no change)", () => { - vi.spyOn(document, "hidden", "get").mockReturnValue(true); - maybePlayChime("pending", "pending", "chat-1", "chat-2"); - expect(playSpy).not.toHaveBeenCalled(); + maybePlayChime("running", "waiting", "chat-1", "chat-2"); + // Should play immediately without needing to advance timers. + expect(playSpy).toHaveBeenCalledTimes(1); }); }); diff --git a/site/src/pages/AgentsPage/AgentDetail/useAgentChime.ts b/site/src/pages/AgentsPage/AgentDetail/useAgentChime.ts index 1ea31e1670..c6eb20d0fa 100644 --- a/site/src/pages/AgentsPage/AgentDetail/useAgentChime.ts +++ b/site/src/pages/AgentsPage/AgentDetail/useAgentChime.ts @@ -28,7 +28,7 @@ export function setChimeEnabled(enabled: boolean): void { */ let chimeAudio: HTMLAudioElement | null = null; -function playChime(): void { +function playChimeAudio(): void { try { if (!chimeAudio) { chimeAudio = new Audio("/chime.mp3"); @@ -44,6 +44,65 @@ function playChime(): void { } } +// -- Cross-tab chime deduplication via Web Locks API ---------- +// +// When multiple tabs are open on /agents, every tab receives the +// same WebSocket status transitions and would independently +// decide to play the chime. We use navigator.locks to acquire a +// short-lived, per-chatID lock. Only the tab that successfully +// acquires the lock plays the sound. The lock is held for a +// short duration to prevent other tabs from acquiring it for the +// same event. +// +// Falls back to always playing (original single-tab behavior) +// when the Web Locks API is unavailable. + +/** + * How long to hold the lock after playing the chime (ms). This + * prevents other tabs whose WebSocket event arrives slightly + * later from also acquiring the lock for the same transition. + */ +export const LOCK_HOLD_MS = 2000; + +/** + * Coordinate across tabs so that only one tab plays the chime + * for a given chatID. Uses navigator.locks.request() with + * ifAvailable: true — the first tab to acquire the lock plays, + * all others silently skip. The lock is held for LOCK_HOLD_MS + * to cover the window in which other tabs receive the same + * WebSocket event. + * + * Falls back to playing immediately when the Web Locks API is + * not available (preserving the original single-tab behavior). + */ +function playChime(chatID: string): void { + if (typeof navigator === "undefined" || !navigator.locks) { + playChimeAudio(); + return; + } + + const lockName = `coder-agent-chime:${chatID}`; + + void navigator.locks.request( + lockName, + { ifAvailable: true }, + async (lock) => { + if (!lock) { + // Another tab already holds the lock for this + // chatID — skip playback. + return; + } + + playChimeAudio(); + + // Hold the lock briefly so that tabs receiving the + // WebSocket event a bit later will see the lock as + // held and skip. + await new Promise((resolve) => setTimeout(resolve, LOCK_HOLD_MS)); + }, + ); +} + /** * Check whether a chat status transition should trigger a chime * and play it if so. A chime fires when a chat reaches a @@ -90,5 +149,5 @@ export function maybePlayChime( return; } - playChime(); + playChime(chatID); }