fix(alerts): one chime per alert across open tabs

CHANGELOG.md

@@ -8,6 +8,9 @@ between releases do not reach them. Remote-PC runtime bundles are versioned sepa
 ## [Unreleased]
 
 ### Fixed
+- Open tabs take turns chiming for the same alert. A question or a finish is still heard if
+  the tab that claimed it closes or cannot play it, including on plain-HTTP LAN addresses.
+  ([#632](https://github.com/devswha/herdr-web-ui/pull/632))
 - Chat reads the transcript of an omp started with `--profile <name>`, whose sessions live
   under `~/.omp/profiles/<name>/agent/sessions`; such panes showed only the terminal text.
   ([#618](https://github.com/devswha/herdr-web-ui/pull/618) by @frankjoshua)

scripts/alert-sound-regression.ts

@@ -1,8 +1,9 @@
+import "./test-herdr.ts";
 import assert from "node:assert/strict";
 import { mkdirSync, mkdtempSync, rmSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
-import type { Browser, Page } from "playwright-core";
+import { errors as browserErrors, type Browser, type Page } from "playwright-core";
 import { herdrRpc, workspaceClose, workspaceCreate } from "../server/herdr/client.ts";
 import { openSettingsPage } from "./settings-page.ts";
 
@@ -54,6 +55,15 @@ export async function checkAlertSound(browser: Browser, origin: string): Promise
         }
       }
       Object.assign(window, { AudioContext: RecordingAudioContext });
+      // `slowChimed` makes this tab confirm its chimes late, as one whose audio waits behind another
+      const post = BroadcastChannel.prototype.postMessage;
+      BroadcastChannel.prototype.postMessage = function (message: unknown) {
+        if ((window as unknown as { slowChimed?: boolean }).slowChimed && (message as { type?: string } | null)?.type === "chimed") {
+          setTimeout(() => post.call(this, message), 1_000);
+          return;
+        }
+        post.call(this, message);
+      };
       // A visible working badge can come from a roster snapshot before the collector has
       // subscribed to these newly created panes. Observe a real status event before testing alerts.
       const statuses: Record<string, string> = {};
@@ -107,12 +117,19 @@ export async function checkAlertSound(browser: Browser, origin: string): Promise
 
     // The collector reopens its subscriptions after the pane set changes. Prime only owned
     // fixture statuses until working arrives as an event, not merely in a polled snapshot.
+    // Each attempt waits (bounded) for its own event before reporting again, so setup does not
+    // flood the subscription it is priming.
     for (const deadline = Date.now() + 10_000; !(await page.evaluate((pane) =>
       (window as unknown as { soundStatuses: Record<string, string> }).soundStatuses[pane] === "working", otherPane));) {
       assert.ok(Date.now() < deadline, "the status subscription includes the new panes");
       await report(otherPane, "idle");
+      const received = page.waitForFunction((pane) =>
+        (window as unknown as { soundStatuses: Record<string, string> }).soundStatuses[pane] === "working",
+      otherPane, { timeout: Math.max(1, Math.min(1_000, deadline - Date.now())) }).catch((error) => {
+        if (!(error instanceof browserErrors.TimeoutError)) throw error;
+      });
       await report(otherPane, "working");
-      await page.waitForTimeout(50);
+      await received;
     }
     // no tap yet: the page may not play, and nothing is kept to sound later
     await block(otherPane);
@@ -131,6 +148,7 @@ export async function checkAlertSound(browser: Browser, origin: string): Promise
     // with the chime: by the time the card for the pane behind shows, a chime for the pane in
     // front would have sounded already
     await page.locator(".droplet-card").waitFor({ state: "visible" });
+    await page.waitForFunction(() => (window as unknown as { chimes: number[] }).chimes.length >= 2);
     assert.deepEqual(await chimes(), [660, 880], "one rising chime, for the pane not in front");
     console.log("PASS a pane that waits chimes, the one in front does not");
 
@@ -179,6 +197,73 @@ export async function checkAlertSound(browser: Browser, origin: string): Promise
     await page.evaluate(() => { (window as unknown as { rosterFirst: boolean }).rosterFirst = false; });
     console.log("PASS a wait the roster showed before its event still chimes");
 
+    // Both pages share the origin and BroadcastChannel, not just the same browser process.
+    const second = await context.newPage();
+    second.on("pageerror", (error) => errors.push(error.message));
+    await second.goto(`${origin}/?pane=${encodeURIComponent(openPane)}`);
+    await second.locator(".conn-live").waitFor();
+    // A header gesture unlocks audio without typing into any terminal.
+    await second.getByRole("button", { name: "Settings", exact: true }).click();
+    await second.keyboard.press("Escape");
+    await second.getByRole("dialog", { name: "Settings" }).waitFor({ state: "detached" });
+    const tabs = [page, second];
+    const counts = () => Promise.all(tabs.map(async (tab) => (await chimes(tab)).length));
+    const heard = async () => (await counts()).reduce((sum, count) => sum + count, 0);
+    // Check continuously through the safety-net deadline, not just at the first note.
+    // Time is the behaviour here: a duplicate must not arrive later in this turn.
+    const noSecondChime = async (beforeCounts: number[]) => {
+      await Promise.all(tabs.map(async (tab, index) => {
+        const expected = (await chimes(tab)).length;
+        assert.ok(expected <= beforeCounts[index]! + 2, "at most one new chime per tab");
+        const deadline = await tab.evaluate(() => performance.now() + 1_200);
+        await tab.waitForFunction(({ expected, deadline }) => {
+          const recording = window as unknown as { chimes: number[]; chimeEnd: number };
+          if (recording.chimes.length > expected) throw new Error("a second chime arrived");
+          return performance.now() >= deadline && performance.now() / 1000 >= recording.chimeEnd;
+        }, { expected, deadline }, { timeout: 5_000 });
+      }));
+    };
+    await Promise.all(tabs.map(quiet));
+    await report(thirdPane, "working");
+    await Promise.all(tabs.map((tab) => seen(thirdPane, "working", tab)));
+    const bothBefore = await counts();
+    await report(thirdPane, "blocked");
+    await Promise.all(tabs.map((tab) => seen(thirdPane, "blocked", tab)));
+    // Either tab may win: subscribe to the audio recorder in both.
+    await Promise.any(tabs.map((tab, index) => tab.waitForFunction((count) =>
+      (window as unknown as { chimes: number[] }).chimes.length > count, bothBefore[index], { timeout: 5_000 })));
+    await noSecondChime(bothBefore);
+    const delta = (await heard()) - bothBefore.reduce((sum, count) => sum + count, 0);
+    console.log(`two-tab shared alert: ${delta / 2} chimes (${delta} notes)`);
+    assert.equal(delta, 2, "exactly one chime across two tabs for the same alert");
+    console.log("PASS two open tabs chime once for a shared alert");
+
+    await report(otherPane, "working");
+    await Promise.all(tabs.map((tab) => seen(otherPane, "working", tab)));
+    const frontBefore = await counts();
+    assert.equal(await page.evaluate(() => document.visibilityState), "visible");
+    await report(otherPane, "blocked");
+    await Promise.all(tabs.map((tab) => seen(otherPane, "blocked", tab)));
+    await second.waitForFunction((count) => (window as unknown as { chimes: number[] }).chimes.length >= count + 2, frontBefore[1]);
+    await noSecondChime(frontBefore);
+    assert.deepEqual(await counts(), [frontBefore[0], frontBefore[1]! + 2], "tab B tells a question in front of tab A");
+    console.log("PASS the other tab tells a question in front of the first tab");
+
+    // The winning tab's confirmation comes a second late: the other tab waits for it instead of
+    // chiming the same question again.
+    await Promise.all(tabs.map(quiet));
+    await report(thirdPane, "working");
+    await Promise.all(tabs.map((tab) => seen(thirdPane, "working", tab)));
+    await Promise.all(tabs.map((tab) => tab.evaluate(() => { (window as unknown as { slowChimed: boolean }).slowChimed = true; })));
+    const slowBefore = await counts();
+    await report(thirdPane, "blocked");
+    await Promise.all(tabs.map((tab) => seen(thirdPane, "blocked", tab)));
+    await Promise.any(tabs.map((tab, index) => tab.waitForFunction((count) =>
+      (window as unknown as { chimes: number[] }).chimes.length > count, slowBefore[index], { timeout: 5_000 })));
+    await noSecondChime(slowBefore);
+    assert.equal((await heard()) - slowBefore.reduce((sum, count) => sum + count, 0), 2, "one chime when the winner confirms late");
+    console.log("PASS a winner that confirms late is not chimed over by the other tab");
+
     assert.deepEqual(errors, []);
   } finally {
     await context.close();
@@ -187,3 +272,23 @@ export async function checkAlertSound(browser: Browser, origin: string): Promise
     rmSync(root, { recursive: true, force: true });
   }
 }
+
+// Also runnable on its own through `bun run check run bun scripts/alert-sound-regression.ts`.
+if (import.meta.main) {
+  const { chromium } = await import("playwright-core");
+  const { createServer } = await import("../server/index.ts");
+  const { UsageService } = await import("../server/usage.ts");
+  const stateDir = mkdtempSync(join(tmpdir(), "herdr-web-ui-sound-server-"));
+  const server = createServer({ port: 0, hostname: "127.0.0.1", token: "", stateDir, usage: new UsageService(undefined, []) });
+  const browser = await chromium.launch({
+    executablePath: process.env.CHROME_PATH ?? "/opt/google/chrome/chrome",
+    headless: true, args: ["--no-sandbox", "--accept-lang=en-US"],
+  });
+  try {
+    await checkAlertSound(browser, `http://127.0.0.1:${server.port}`);
+  } finally {
+    await browser.close();
+    server.stop();
+    rmSync(stateDir, { recursive: true, force: true });
+  }
+}

src/App.tsx

@@ -54,7 +54,8 @@ import { useScreenWakeLock } from "./lib/wakeLock.ts";
 import { watchDrawerSwipe } from "./lib/edgeSwipe.ts";
 import { Droplet } from "./components/Droplet.tsx";
 import { dropletAllows, endedTurn, seedStatuses, showDroplet, trackTurn, type DropletKind } from "./lib/droplet.ts";
-import { playAlertSound, unlockAlertSound, type AlertSoundKind } from "./lib/alertSound.ts";
+import { canPlayAlertSound, playAlertSound, unlockAlertSound, type AlertSoundKind } from "./lib/alertSound.ts";
+import { createAlertTurnPlayer } from "./lib/alertTurns.ts";
 
 const APP_TITLE = "herdr web ui";
 const POLL_MS = 5000;
@@ -349,11 +350,19 @@ export function App() {
 
   // The alert sound (lib/alertSound.ts): heard also while the tab is hidden and a Focus silences
   // system notifications; never for the pane open in front of the user.
+  const alertTurns = useRef<ReturnType<typeof createAlertTurnPlayer> | null>(null);
+  useEffect(() => {
+    const player = createAlertTurnPlayer({
+      play: (kind) => alertsOnRef.current && alertSoundRef.current && playAlertSound(kind),
+    });
+    alertTurns.current = player;
+    return () => { player.dispose(); alertTurns.current = null; };
+  }, []);
   const chime = useCallback((machine: Machine, pane: HerdrPane, kind: AlertSoundKind) => {
-    if (!alertsOnRef.current || !alertSoundRef.current) return;
+    if (!alertsOnRef.current || !alertSoundRef.current || !canPlayAlertSound()) return;
     const open = selectionRef.current;
     if (document.visibilityState === "visible" && open.machineId === machine.id && open.paneId === pane.pane_id && !drawerOpenRef.current) return;
-    playAlertSound(kind);
+    alertTurns.current?.chime(JSON.stringify([machine.id, pane.pane_id, kind]), kind);
   }, []);
 
   // a page plays audio only after a tap or key on it: each one lets the next chime play. A mouse

src/lib/alertSound.test.ts

@@ -34,7 +34,10 @@ const LENGTH = 0.16 + 0.3;
 
 const saved = (globalThis as { AudioContext?: unknown }).AudioContext;
 Object.assign(globalThis, { AudioContext: FakeAudioContext });
-afterAll(() => { Object.assign(globalThis, { AudioContext: saved }); });
+afterAll(() => {
+  for (const audio of made) audio.state = "closed";
+  Object.assign(globalThis, { AudioContext: saved });
+});
 
 /** Lets the chime of the tab's last test end, and forgets what it played. */
 function quiet(): FakeAudioContext {
@@ -131,6 +134,28 @@ describe("one chime at a time", () => {
     expect(started).toEqual([...CHIME_NOTES.done, ...CHIME_NOTES.done]);
   });
 
+  it("confirms a queued question only after its audio clock reaches its start", async () => {
+    const audio = made.at(-1)!;
+    playAlertSound("done");
+    const pending = playAlertSound("blocked");
+    expect(pending).toBeInstanceOf(Promise);
+    let confirmed = false;
+    void Promise.resolve(pending).then(() => { confirmed = true; });
+    await Promise.resolve();
+    expect(confirmed).toBe(false);
+    audio.currentTime += LENGTH;
+    expect(await pending).toBe(true);
+  });
+
+  it("does not confirm a queued question if audio was interrupted before starting", async () => {
+    const audio = made.at(-1)!;
+    playAlertSound("done");
+    const pending = playAlertSound("blocked");
+    audio.state = "interrupted";
+    expect(await pending).toBe(false);
+    await unlockAlertSound();
+  });
+
   it("previews the chime in Settings, after the chime that sounds", () => {
     const now = made.at(-1)!.currentTime;
     previewAlertSound();

src/lib/alertSound.ts

@@ -10,9 +10,9 @@
  * One chime sounds at a time in a tab. Alerts that come together - several panes finishing at
  * once - would otherwise sound over each other. An alert that comes while a chime sounds is
  * already told by it, except a question after a finish: the question's chime starts where the
- * finish's ends, so a question is never lost. Open tabs of the app do not take turns: each
- * chimes for an alert they all hear, since a tab cannot tell which alert another tab's chime
- * was for, and staying quiet on a guess could leave a question told by no tab.
+ * finish's ends, so a question is never lost. `alertTurns.ts` coordinates eligible tabs by
+ * machine, pane and kind. An alert deferred to another tab stays pending until that tab
+ * confirms it chimed, or a local deadline/withdrawal lets this tab tell it instead.
  */
 
 export type AlertSoundKind = "blocked" | "done";
@@ -29,7 +29,7 @@ const PEAK_GAIN = 0.25;
 let context: AudioContext | null = null;
 // this tab's chimes that have not ended, on its context's clock: when the last of them ends,
 // and when the last question's does (a preview queued behind a question does not move that)
-let sounding: { audio: AudioContext; until: number; question: number } | null = null;
+let sounding: { audio: AudioContext; until: number; question: number; questionStart: number } | null = null;
 
 function contextClass(): typeof AudioContext | undefined {
   return globalThis.AudioContext ?? (globalThis as { webkitAudioContext?: typeof AudioContext }).webkitAudioContext;
@@ -53,10 +53,25 @@ export async function unlockAlertSound(): Promise<boolean> {
   return context.state === "running";
 }
 
-/** An alert: chimes, unless a chime this tab is playing already tells it. */
-export function playAlertSound(kind: AlertSoundKind): void {
+export function canPlayAlertSound(): boolean {
+  return context?.state === "running";
+}
+
+/** Confirms only audio that started, not a question scheduled behind a finish or preview. */
+export function playAlertSound(kind: AlertSoundKind): boolean | Promise<boolean> {
   const audio = context;
-  if (audio?.state === "running") chime(audio, kind);
+  if (audio?.state !== "running") return false;
+  const start = chime(audio, kind);
+  if (start <= audio.currentTime) return true;
+  return new Promise((resolve) => {
+    const check = () => {
+      if (audio.state !== "running" || context !== audio) { resolve(false); return; }
+      const remaining = start - audio.currentTime;
+      if (remaining <= 0) resolve(true);
+      else setTimeout(check, Math.ceil(remaining * 1_000));
+    };
+    check();
+  });
 }
 
 /** Settings' preview of the chime: always played, after a chime of this tab that still sounds. */
@@ -66,11 +81,11 @@ export function previewAlertSound(): void {
 }
 
 /** Plays `kind` unless this tab's last chime still sounds; a preview then starts where it ends. */
-function chime(audio: AudioContext, kind: AlertSoundKind, preview = false): void {
+function chime(audio: AudioContext, kind: AlertSoundKind, preview = false): number {
   const now = audio.currentTime;
   const current = sounding !== null && sounding.audio === audio && sounding.until > now ? sounding : null;
   // already told by a chime that sounds or waits its turn: a finish by any, a question by a question's
-  if (current && !preview && (kind === "done" || current.question > now)) return;
+  if (current && !preview && (kind === "done" || current.question > now)) return kind === "blocked" ? Math.max(now, current.questionStart) : now;
   const start = current ? current.until : now;
   const notes = CHIME_NOTES[kind];
   notes.forEach((frequency, index) => {
@@ -88,5 +103,10 @@ function chime(audio: AudioContext, kind: AlertSoundKind, preview = false): void
     oscillator.stop(at + NOTE_LENGTH_S);
   });
   const until = start + (notes.length - 1) * NOTE_GAP_S + NOTE_LENGTH_S;
-  sounding = { audio, until, question: kind === "blocked" && !preview ? until : current?.question ?? 0 };
+  sounding = {
+    audio, until,
+    question: kind === "blocked" && !preview ? until : current?.question ?? 0,
+    questionStart: kind === "blocked" && !preview ? start : current?.questionStart ?? 0,
+  };
+  return start;
 }