From 1cf7be0cddda4d656280b2a480a335074ee4b25e Mon Sep 17 00:00:00 2001
From: Ross Cadogan <ross.cadogan@gmail.com>
Date: Thu, 6 Aug 2026 21:32:07 +0100
Subject: [PATCH] fix(audio): mute the host's voice when SayPi is the one
 speaking (#602)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

With a SayPi voice selected on pi.ai, both voices played at once — ours and
Pi's own over the top. Only visible since #597; before that we produced no
audio, so the missing mute looked like the whole bug.

AudioOutputMachine already owns the decision: on the audio element's
loadstart it sees a source belonging to neither the selected voice nor the
active provider, and skips it. It never got the chance, because AudioModule
had no element bound at all.

Measured live on pi.ai: the tracked element is removed, cleanupAudioElement
nulls it, and recovery re-arms listenForAudioElementSwap — an observer that
only fires for newly ADDED subtrees. pi.ai replaces its player rather than
reusing it, so the replacement was already in the document and nothing was
ever added afterwards to react to. The observer waited forever, no loadstart
reached the machine, and the page ended up with two undecorated <audio>
elements, one of them audibly playing pi.ai/api/chat/voice.

Two changes, both in service of one rule — when a SayPi voice is selected,
SayPi is the TTS provider, so the host is silent and we speak:

  - recovery takes up an element that is already there, and only waits for an
    insertion when there genuinely is none;
  - the mute is held as an invariant from the active provider rather than
    reacted to per playback event, so a track the host started before we
    bound to it can't stay audible for its whole length. Gated on offscreen
    playback: where there is no offscreen document (Firefox, Safari) SayPi
    shares that element and muting it would mute us, so skip-on-loadstart
    remains the whole mechanism there.

The decisions live in src/audio/hostAudio.ts because AudioModule.js can't be
imported by a test — the same reason audioElementRemoval.ts exists — and the
wiring is pinned by the source-scanning guards beside it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KgkAqzyjN5xtxR5fKxRPR2
---
 src/audio/AudioModule.js               | 59 ++++++++++++++++++++++++-
 src/audio/hostAudio.ts                 | 55 +++++++++++++++++++++++
 test/audio/audioElementRemoval.spec.ts | 36 ++++++++++++++-
 test/audio/hostAudio.spec.ts           | 61 ++++++++++++++++++++++++++
 4 files changed, 207 insertions(+), 4 deletions(-)
 create mode 100644 src/audio/hostAudio.ts
 create mode 100644 test/audio/hostAudio.spec.ts

diff --git a/src/audio/AudioModule.js b/src/audio/AudioModule.js
index ad51fd8138..838d9f0f02 100644
--- a/src/audio/AudioModule.js
+++ b/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) {
diff --git a/src/audio/hostAudio.ts b/src/audio/hostAudio.ts
new file mode 100644
index 0000000000..0b393d3f23
--- /dev/null
+++ b/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;
+}
diff --git a/test/audio/audioElementRemoval.spec.ts b/test/audio/audioElementRemoval.spec.ts
index a9f6bdafaa..a334a20a8d 100644
--- a/test/audio/audioElementRemoval.spec.ts
+++ b/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\(\)/);
   });
 });
diff --git a/test/audio/hostAudio.spec.ts b/test/audio/hostAudio.spec.ts
new file mode 100644
index 0000000000..f6d4f4e862
--- /dev/null
+++ b/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);
+  });
+});