fix(claude): stop the voice menu self-closing on a real click (#494)

src/chatbots/ClaudeVoiceMenu.ts

@@ -725,13 +725,19 @@ export class ClaudeVoiceMenu extends VoiceSelector {
   }
 
   /**
-   * The dropdown's single render path: rebuild the trigger button + menu
+   * The dropdown's single render path: paint the trigger button + menu
    * content from the given catalog and stored voice. The stored voice arrives
    * WITH the catalog (gather-then-render), so the trigger label is right on
-   * the first paint and curateShortlist pins the current voice synchronously
-   * — the old read-selection-from-the-button-label block and the async
-   * getVoice fallback (with its pin writeback + conditional re-populate) are
-   * gone.
+   * the first paint and curateShortlist pins the current voice synchronously.
+   *
+   * The trigger and menu container are STABLE NODES: created once, then
+   * updated/refilled in place. Rebuilding them here is not safe — every open
+   * runs this via refreshMenu, whose fetches resolve from warm in-memory
+   * caches in microtasks, i.e. (for a real, trusted click) BETWEEN the
+   * trigger's click listener and the document-level outside-click listener.
+   * A rebuild detaches the click's event.target, so the outside-click check
+   * `this.element.contains(event.target)` reads the opening click as
+   * "outside" and closes the menu in the same tick it opened (#494).
    */
   protected override renderMenu(
     voices: SpeechSynthesisVoiceRemote[],
@@ -763,21 +769,27 @@ export class ClaudeVoiceMenu extends VoiceSelector {
     );
     this.showTier = curated.tiersCoexist;
 
-    // Comprehensive cleanup to prevent duplicates
-    this.cleanupExistingElements(voiceSelector);
-
     // Check if we have any voices available besides "Voice off"
     const noVoicesAvailable = voices.length === 0;
+    const requiresSignIn = this.ttsRequiresSignIn(noVoicesAvailable);
+
+    // First render creates the nodes; every later render reuses them.
+    // (Duplicate defense — cleanupExistingElements — runs once, from the
+    // constructor, where a fresh instance may inherit a prior instance's
+    // leftovers.)
+    if (!this.menuButton.isConnected) {
+      this.menuButton = this.createVoiceButton(storedVoice, requiresSignIn);
+      voiceSelector.appendChild(this.menuButton);
+    }
+    this.updateSignInAffordance(requiresSignIn);
 
-    // Recreate the menu button and content from scratch
-    this.menuButton = this.createVoiceButton(
-      storedVoice,
-      this.ttsRequiresSignIn(noVoicesAvailable)
-    );
-    voiceSelector.appendChild(this.menuButton);
-
-    this.menuContent = this.createVoiceMenu();
-    voiceSelector.appendChild(this.menuContent);
+    if (!this.menuContent.isConnected) {
+      this.menuContent = this.createVoiceMenu();
+      voiceSelector.appendChild(this.menuContent);
+    }
+    // Refill in place — the container keeps its identity and, when open, its
+    // place in document.body and its visibility.
+    this.menuContent.replaceChildren();
 
     // Add "Voice off" option with appropriate messaging
     const voiceOffItem = this.createMenuItem(null, noVoicesAvailable);
@@ -817,6 +829,22 @@ export class ClaudeVoiceMenu extends VoiceSelector {
     this.updateSelectedVoice(storedVoice);
   }
 
+  /**
+   * In-place counterpart of createVoiceButton's requiresSignIn branch, for
+   * re-renders that reuse the trigger node (see that branch for the
+   * accessibility rationale).
+   */
+  private updateSignInAffordance(requiresSignIn: boolean): void {
+    this.menuButton.classList.toggle("saypi-voice-unavailable", requiresSignIn);
+    if (requiresSignIn) {
+      this.menuButton.setAttribute("aria-label", getMessage("signInForTTS"));
+      this.menuButton.setAttribute("title", getMessage("signInForTTS"));
+    } else {
+      this.menuButton.removeAttribute("aria-label");
+      this.menuButton.removeAttribute("title");
+    }
+  }
+
   /**
    * The muted final row linking to the full voice catalog in the extension
    * settings (Voices tab). Always rendered — it is the menu's path to the

test/chatbots/ClaudeVoiceMenu-open-race.spec.ts

@@ -0,0 +1,116 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+
+// ConfigModule reads injected env at import time; stub it (mirrors other specs).
+vi.mock("../../src/ConfigModule", () => ({
+  config: {
+    appServerUrl: "https://app.example.com",
+    apiServerUrl: "https://api.saypi.ai",
+    GA_MEASUREMENT_ID: "x",
+    GA_API_SECRET: "x",
+    GA_ENDPOINT: "x",
+  },
+}));
+
+vi.mock("../../src/JwtManager", () => ({
+  getJwtManagerSync: () => ({
+    isAuthenticated: () => true,
+    getClaims: () => ({ ttsQuotaRemaining: 1000 }),
+  }),
+}));
+
+vi.mock("../../src/popup/popupopener", () => ({
+  openSettings: vi.fn(),
+}));
+
+import { ClaudeVoiceMenu } from "../../src/chatbots/ClaudeVoiceMenu";
+import { claudeMockVoices } from "../data/Voices";
+
+// Bypass the heavy constructor; renderMenu only needs prototype methods
+// plus the fields it reads (pattern from ClaudeVoiceMenu-curation.spec.ts).
+function makeMenu(): any {
+  const menu = Object.create(ClaudeVoiceMenu.prototype);
+  menu.chatbot = {} as any;
+  menu.userPreferences = {
+    getVoice: vi.fn(async () => null),
+    setVoice: vi.fn(async () => {}),
+    unsetVoice: vi.fn(async () => {}),
+  };
+  menu.element = document.createElement("div");
+  menu.element.id = "claude-voice-selector";
+  document.body.appendChild(menu.element);
+  menu.menuButton = document.createElement("button");
+  menu.menuContent = document.createElement("div");
+  return menu;
+}
+
+beforeEach(() => {
+  document.body.innerHTML = "";
+});
+
+// Regression guard for the open-click self-close race (#494, introduced by
+// #492): every open runs refreshMenu → renderMenu. With warm in-memory
+// caches, that resolves in microtasks — which a REAL (trusted) click lets
+// run BETWEEN the trigger's own click listener and the document-level
+// outside-click listener. If renderMenu rebuilds the trigger, the click's
+// event.target is a detached node by the time the document listener checks
+// `this.element.contains(event.target)`, so it closes the menu in the same
+// tick it opened. The invariant that makes the race harmless: renderMenu
+// updates the existing trigger/menu nodes in place — it never replaces them.
+describe("ClaudeVoiceMenu re-render node stability (open-click race)", () => {
+  it("keeps the same trigger button node across re-renders", () => {
+    const menu = makeMenu();
+    menu.renderMenu(claudeMockVoices, null);
+    const trigger = menu.element.querySelector(
+      "button[data-voice-active]"
+    ) as HTMLButtonElement;
+    expect(trigger).not.toBeNull();
+
+    // A second render — exactly what an open-click triggers via refreshMenu.
+    menu.renderMenu(claudeMockVoices, null);
+
+    expect(menu.menuButton).toBe(trigger);
+    expect(trigger.isConnected).toBe(true);
+    expect(
+      menu.element.querySelectorAll("button[data-voice-active]").length
+    ).toBe(1);
+  });
+
+  it("keeps the same menu container node, and an open menu stays open in body", () => {
+    const menu = makeMenu();
+    menu.renderMenu(claudeMockVoices, null);
+    const content = menu.menuContent as HTMLDivElement;
+
+    // Simulate the menu being open: parked in document.body, visible
+    // (positionMenuAboveButton's arrangement).
+    document.body.appendChild(content);
+    content.style.display = "block";
+
+    // A re-render lands while open (mid-click rebuild, or an auth/preference
+    // refresh). It must refill the SAME node, not tear it down.
+    const lucy = claudeMockVoices.find((v) => v.name === "Lucy")!;
+    menu.renderMenu(claudeMockVoices, lucy);
+
+    expect(menu.menuContent).toBe(content);
+    expect(content.parentElement).toBe(document.body);
+    expect(content.style.display).toBe("block");
+    // …and the refill really happened: the stored voice is now checked.
+    const lucyRow = content.querySelector(
+      "[data-voice-name='Lucy'] .checkmark-container"
+    );
+    expect(lucyRow?.innerHTML).toContain("svg");
+  });
+
+  it("updates the trigger label and sign-in affordance in place", () => {
+    const menu = makeMenu();
+    menu.renderMenu(claudeMockVoices, null);
+    const trigger = menu.element.querySelector(
+      "button[data-voice-active]"
+    ) as HTMLButtonElement;
+
+    const lucy = claudeMockVoices.find((v) => v.name === "Lucy")!;
+    menu.renderMenu(claudeMockVoices, lucy);
+    expect(menu.element.querySelector(".voice-name")?.textContent).toBe("Lucy");
+    expect(trigger.getAttribute("data-voice-active")).toBe("true");
+    expect(trigger.classList.contains("saypi-voice-unavailable")).toBe(false);
+  });
+});