fix(audio): mute the host's voice when SayPi is the one speaking (#602)

src/audio/AudioModule.js

@@ -15,6 +15,8 @@ import { ChatbotService } from "../chatbots/ChatbotService.ts";
 import OffscreenAudioBridge from "./OffscreenAudioBridge.js";
 import { BrowserCompatibilityModule } from "../compat/BrowserCompatibilityModule.ts";
 import { createAudioRemovalObserverCallback } from "./audioElementRemoval.ts";
+import { findHostAudioElement, shouldMuteHostAudio } from "./hostAudio.ts";
+import { audioProviders } from "../tts/SpeechModel.ts";
 
 const INITIAL_PLAYBACK_BUFFER_TIMEOUT_MS = 5000;
 
@@ -26,6 +28,8 @@ export default class AudioModule {
 
     this.AUDIO_ELEMENT_ID = "saypi-audio-main";
     this.audioElement = null;
+    /** Are WE the voice? Set from audio:changeProvider; decides the host's mute. */
+    this.providerIsSayPi = false;
     this.mutationObserver = null;
     this.swapObserver = null;
 
@@ -250,6 +254,7 @@ export default class AudioModule {
     }
     this.decorateAudioElement(this.audioElement);
     this.registerRemovalListener();
+    this.applyHostAudioMute();
   }
 
   swapAudioElement(newAudioElement) {
@@ -263,7 +268,7 @@ export default class AudioModule {
       this.registerAudioPlaybackEvents(this.audioElement, this.audioOutputActor);
     }
     this.registerAudioPlaybackEvents(this.audioElement, this.voiceConverter);
-    
+
     // handle slow responses from pi.ai - since 2024-07 (Pi.ai only)
     if (ChatbotIdentifier.isChatbotType("pi")) {
       this.initializeSlowResponseHandler();
@@ -274,6 +279,29 @@ export default class AudioModule {
     }
     this.registerRemovalListener();
     this.registerLifecycleDebug();
+    // A fresh element arrives unmuted, and the host may already be playing
+    // through it, so re-assert the invariant rather than waiting for a
+    // loadstart we might have just missed (#602).
+    this.applyHostAudioMute();
+  }
+
+  /**
+   * Hold the host's audio silent while SayPi is the voice — an invariant, not a
+   * reaction to a `loadstart` we have to be attached in time to see (#602).
+   * Re-applied whenever the element changes or the provider does.
+   */
+  applyHostAudioMute() {
+    if (!this.audioElement) return;
+    const mute = shouldMuteHostAudio({
+      providerIsSayPi: this.providerIsSayPi,
+      playbackIsOffscreen: this.useOffscreenAudio,
+    });
+    if (this.audioElement.muted !== mute) {
+      logger.debug(
+        `[AudioModule] ${mute ? "Muting" : "Unmuting"} the host's audio element`
+      );
+      this.audioElement.muted = mute;
+    }
   }
 
   registerRemovalListener() {
@@ -290,7 +318,7 @@ export default class AudioModule {
             logger.debug("Audio element removed from the document");
             this.cleanupAudioElement(this.audioElement);
             this.audioElement = null;
-            this.listenForAudioElementSwap();
+            this.rebindAudioElement();
           }
         )
       );
@@ -348,6 +376,27 @@ export default class AudioModule {
     logger.debug("Cleaned up audio element");
   }
 
+  /**
+   * Take up whatever audio element the host has NOW, and only wait for one if
+   * there genuinely isn't one.
+   *
+   * The old recovery went straight to `listenForAudioElementSwap()`, whose
+   * observer fires for newly ADDED subtrees. A host that replaces its player
+   * rather than reusing it (pi.ai) has already inserted the replacement by the
+   * time we notice ours is gone, so nothing was ever added afterwards to react
+   * to: the observer waited forever, no `loadstart` reached the output machine,
+   * and the host's own voice played on top of ours (#602).
+   */
+  rebindAudioElement() {
+    const replacement = findHostAudioElement(document, this.AUDIO_ELEMENT_ID);
+    if (replacement) {
+      logger.debug("[AudioModule] Rebinding to the host's audio element");
+      this.swapAudioElement(replacement);
+      return;
+    }
+    this.listenForAudioElementSwap();
+  }
+
   listenForAudioElementSwap() {
     if (this.swapObserver) {
       this.swapObserver.disconnect();
@@ -510,6 +559,12 @@ export default class AudioModule {
       if (outputActor) {
         outputActor.send({ type: "changeProvider", ...detail });
       }
+      // Whether the host may be heard is decided by who is speaking, so it is
+      // settled here rather than at each playback event (#602). Matched by
+      // identity against the SayPi provider — `matches(source)` answers a
+      // different question (does this URL belong to it).
+      this.providerIsSayPi = detail?.provider === audioProviders.SayPi;
+      this.applyHostAudioMute();
     });
     EventBus.on("audio:changeVoice", (detail) => {
       if (outputActor) {

src/audio/hostAudio.ts

@@ -0,0 +1,55 @@
+/**
+ * Two decisions about the HOST's audio element, kept out of AudioModule.js so
+ * they can be tested (that module's constructor drags in the whole content-script
+ * bootstrap — the same reason `audioElementRemoval.ts` exists).
+ *
+ * Both exist to serve one rule: when a SayPi voice is selected for the active
+ * host, SayPi is the text-to-speech provider — the host's own audio is silenced
+ * and ours plays instead (#602).
+ */
+
+/**
+ * The element to bind to after losing the tracked one.
+ *
+ * The old recovery was to re-arm an observer that only fires for NEWLY ADDED
+ * subtrees. On a host that replaces its player rather than reusing it (pi.ai
+ * does), the replacement is already in the document by the time we look, so
+ * that observer waits forever and the binding is lost for the life of the page
+ * — with it, every `loadstart` the output machine needs in order to skip the
+ * host's voice.
+ *
+ * Prefers an element already carrying our id (we decorated it before), then any
+ * other. Returns null when there is genuinely nothing yet, which is the only
+ * case where waiting for an insertion is the right answer.
+ */
+export function findHostAudioElement(
+  root: Document | Element | null | undefined,
+  decoratedId: string
+): HTMLAudioElement | null {
+  if (!root) return null;
+  // Matched by property rather than by an interpolated `#id` selector: the id
+  // never needs escaping, and this can't throw on a hostile one.
+  const elements = [...root.querySelectorAll<HTMLAudioElement>("audio")];
+  return elements.find((el) => el.id === decoratedId) ?? elements[0] ?? null;
+}
+
+/**
+ * Whether the host's element should be held muted right now.
+ *
+ * An invariant rather than a reaction: `skipCurrent` only fires when we catch a
+ * `loadstart`, so a track the host started before we bound to it stays audible
+ * for its whole length. Muting is also gentler than pausing — the host's player
+ * keeps its own state, and its UI doesn't fight us over it.
+ *
+ * `playbackIsOffscreen` is the condition that makes this safe. Under the
+ * offscreen document (Chrome/Edge) our speech never touches the page's element,
+ * so muting it silences only the host. Where offscreen isn't available (Firefox,
+ * Safari) SayPi plays through that same element, and muting it would mute US —
+ * there, skip-on-loadstart remains the whole mechanism.
+ */
+export function shouldMuteHostAudio(state: {
+  providerIsSayPi: boolean;
+  playbackIsOffscreen: boolean;
+}): boolean {
+  return state.providerIsSayPi && state.playbackIsOffscreen;
+}

test/audio/audioElementRemoval.spec.ts

@@ -186,13 +186,45 @@ describe("AudioModule removal-observer wiring", () => {
     );
   });
 
-  it("releases the audio element and arms the swap listener on removal", () => {
+  it("releases the audio element and recovers the binding on removal", () => {
     const handler = source.match(
       /createAudioRemovalObserverCallback\([\s\S]*?\n {6}\);/
     )?.[0];
     expect(handler).toBeDefined();
     expect(handler).toMatch(/this\.cleanupAudioElement\(this\.audioElement\)/);
     expect(handler).toMatch(/this\.audioElement = null/);
-    expect(handler).toMatch(/this\.listenForAudioElementSwap\(\)/);
+    // Recovery goes through rebindAudioElement, which takes up a replacement
+    // that is ALREADY in the document and only falls back to waiting for an
+    // insertion when there is none. Going straight to the swap listener was
+    // #602: on a host that replaces its player, nothing is ever added
+    // afterwards, so the binding was lost for the life of the page.
+    expect(handler).toMatch(/this\.rebindAudioElement\(\)/);
+  });
+
+  it("rebinds to an existing element before falling back to waiting", () => {
+    const rebind = source.match(/rebindAudioElement\(\)\s*\{[\s\S]*?\n {2}\}/)?.[0];
+    expect(rebind).toBeDefined();
+    expect(rebind).toMatch(/findHostAudioElement\(/);
+    expect(rebind).toMatch(/this\.swapAudioElement\(/);
+    expect(rebind).toMatch(/this\.listenForAudioElementSwap\(\)/);
+    // The fallback must come after the take-it-now path, not instead of it.
+    expect(rebind!.indexOf("swapAudioElement")).toBeLessThan(
+      rebind!.indexOf("listenForAudioElementSwap")
+    );
+  });
+
+  it("holds the host's audio muted from the provider, not from playback events", () => {
+    // The mute is an invariant: re-asserted whenever the element changes and
+    // whenever the provider does, so a track the host started before we bound
+    // to it can't stay audible for its whole length (#602).
+    expect(source).toMatch(/applyHostAudioMute\(\)\s*\{/);
+    expect(source).toMatch(/shouldMuteHostAudio\(\{/);
+    const onProviderChange = source.match(
+      /EventBus\.on\("audio:changeProvider"[\s\S]*?\n {4}\}\);/
+    )?.[0];
+    expect(onProviderChange).toMatch(/providerIsSayPi = /);
+    expect(onProviderChange).toMatch(/this\.applyHostAudioMute\(\)/);
+    const swap = source.match(/swapAudioElement\(newAudioElement\)\s*\{[\s\S]*?\n {2}\}/)?.[0];
+    expect(swap).toMatch(/this\.applyHostAudioMute\(\)/);
   });
 });

test/audio/hostAudio.spec.ts

@@ -0,0 +1,61 @@
+import { describe, it, expect, beforeEach } from "vitest";
+import {
+  findHostAudioElement,
+  shouldMuteHostAudio,
+} from "../../src/audio/hostAudio";
+
+const ID = "saypi-audio-main";
+
+describe("finding the host's audio element again after losing it", () => {
+  beforeEach(() => {
+    document.body.innerHTML = "";
+  });
+
+  it("finds a replacement that is ALREADY in the document", () => {
+    // The #602 case: pi.ai replaces its player rather than reusing it, so by
+    // the time we notice ours is gone the new one has already been inserted —
+    // and an observer waiting for an insertion waits forever.
+    const replacement = document.createElement("audio");
+    document.body.appendChild(replacement);
+    expect(findHostAudioElement(document, ID)).toBe(replacement);
+  });
+
+  it("prefers the element we had already decorated", () => {
+    const stranger = document.createElement("audio");
+    const ours = document.createElement("audio");
+    ours.id = ID;
+    document.body.append(stranger, ours);
+    expect(findHostAudioElement(document, ID)).toBe(ours);
+  });
+
+  it("reports nothing when the host has no player yet — the one case worth waiting for", () => {
+    expect(findHostAudioElement(document, ID)).toBeNull();
+  });
+
+  it("never throws on a missing root", () => {
+    expect(findHostAudioElement(null, ID)).toBeNull();
+    expect(findHostAudioElement(undefined, ID)).toBeNull();
+  });
+});
+
+describe("holding the host's audio muted while SayPi is the provider", () => {
+  it("mutes the host when we are speaking from the offscreen document", () => {
+    expect(
+      shouldMuteHostAudio({ providerIsSayPi: true, playbackIsOffscreen: true })
+    ).toBe(true);
+  });
+
+  it("leaves the host audible when its own voice is the one selected", () => {
+    expect(
+      shouldMuteHostAudio({ providerIsSayPi: false, playbackIsOffscreen: true })
+    ).toBe(false);
+  });
+
+  it("never mutes the element WE play through (no offscreen document)", () => {
+    // Firefox/Safari: SayPi shares the page's audio element with the host, so
+    // muting it would mute us. Skip-on-loadstart stays the whole mechanism.
+    expect(
+      shouldMuteHostAudio({ providerIsSayPi: true, playbackIsOffscreen: false })
+    ).toBe(false);
+  });
+});