mirror of
https://github.com/coder/coder.git
synced 2026-09-22 05:05:20 +08:00
fix(site): deduplicate agent chime across browser tabs (#22972)
## 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.
This commit is contained in:
Kyle Carberry
@@ -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<string>();
|
||||
|
||||
async request(
|
||||
name: string,
|
||||
options: LockOptions,
|
||||
callback: (lock: Lock | null) => Promise<void>,
|
||||
): Promise<void> {
|
||||
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<typeof vi.fn>;
|
||||
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<void> {
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user