🤖 fix: stabilize unit tests for bun + window guards

Change-Id: If25e133840a3d7cd29c32a0970cabd432a0e2b2d
Signed-off-by: Thomas Kosiewski <tk@coder.com>

Author: ThomasK33

src/browser/components/AppLoader.auth.test.tsx

@@ -15,9 +15,16 @@ void mock.module("@/browser/contexts/API", () => ({
 }));
 
 void mock.module("@/browser/components/AuthTokenModal", () => ({
+  // Note: Module mocks leak between bun test files.
+  // Export all commonly-used symbols to avoid cross-test import errors.
   AuthTokenModal: (props: { error?: string | null }) => (
     <div data-testid="AuthTokenModalMock">{props.error ?? "no-error"}</div>
   ),
+  getStoredAuthToken: () => null,
+  // eslint-disable-next-line @typescript-eslint/no-empty-function
+  setStoredAuthToken: () => {},
+  // eslint-disable-next-line @typescript-eslint/no-empty-function
+  clearStoredAuthToken: () => {},
 }));
 
 import { AppLoader } from "./AppLoader";

src/browser/contexts/API.test.tsx

@@ -66,8 +66,13 @@ void mock.module("@orpc/client/message-port", () => ({
 }));
 
 void mock.module("@/browser/components/AuthTokenModal", () => ({
+  // Note: Module mocks leak between bun test files.
+  // Export all commonly-used symbols to avoid cross-test import errors.
+  AuthTokenModal: () => null,
   getStoredAuthToken: () => null,
   // eslint-disable-next-line @typescript-eslint/no-empty-function
+  setStoredAuthToken: () => {},
+  // eslint-disable-next-line @typescript-eslint/no-empty-function
   clearStoredAuthToken: () => {},
 }));
 

src/browser/hooks/useVoiceInput.ts

@@ -56,8 +56,12 @@ export interface UseVoiceInputResult {
  * We hide our voice UI on these devices to avoid redundancy with system dictation.
  */
 function hasTouchDictation(): boolean {
-  if (typeof window === "undefined") return false;
-  const hasTouch = "ontouchstart" in window || navigator.maxTouchPoints > 0;
+  if (typeof window === "undefined" || typeof navigator === "undefined") return false;
+
+  const maxTouchPoints =
+    typeof navigator.maxTouchPoints === "number" ? navigator.maxTouchPoints : 0;
+  const hasTouch = "ontouchstart" in window || maxTouchPoints > 0;
+
   // Touch-only check: most touch devices have native dictation.
   // We don't check screen size because iPads are large but still have dictation.
   return hasTouch;
@@ -66,7 +70,9 @@ function hasTouchDictation(): boolean {
 const HAS_TOUCH_DICTATION = hasTouchDictation();
 const HAS_MEDIA_RECORDER = typeof window !== "undefined" && typeof MediaRecorder !== "undefined";
 const HAS_GET_USER_MEDIA =
-  typeof window !== "undefined" && typeof navigator.mediaDevices?.getUserMedia === "function";
+  typeof window !== "undefined" &&
+  typeof navigator !== "undefined" &&
+  typeof navigator.mediaDevices?.getUserMedia === "function";
 
 // =============================================================================
 // Global Key State Tracking
@@ -79,7 +85,7 @@ const HAS_GET_USER_MEDIA =
  */
 let isSpaceCurrentlyHeld = false;
 
-if (typeof window !== "undefined") {
+if (typeof window !== "undefined" && typeof window.addEventListener === "function") {
   window.addEventListener(
     "keydown",
     (e) => {

src/browser/utils/RefreshController.test.ts

@@ -1,50 +1,51 @@
-import { describe, it, expect, beforeEach, afterEach, jest } from "@jest/globals";
+import { describe, it, expect, afterEach, jest, setSystemTime } from "bun:test";
 import { RefreshController } from "./RefreshController";
 
-describe("RefreshController", () => {
-  beforeEach(() => {
-    jest.useFakeTimers();
-  });
+async function sleep(ms: number): Promise<void> {
+  await new Promise((resolve) => setTimeout(resolve, ms));
+}
 
+describe("RefreshController", () => {
   afterEach(() => {
-    jest.useRealTimers();
+    // Some tests manipulate Date.now via setSystemTime(); always restore.
+    setSystemTime();
   });
 
-  it("rate-limits multiple schedule() calls (doesn't reset timer)", () => {
+  it("rate-limits multiple schedule() calls (doesn't reset timer)", async () => {
     const onRefresh = jest.fn<() => void>();
-    const controller = new RefreshController({ onRefresh, debounceMs: 100 });
+    const controller = new RefreshController({ onRefresh, debounceMs: 50 });
 
     controller.schedule();
-    jest.advanceTimersByTime(50);
+    await sleep(20);
     controller.schedule(); // Shouldn't reset timer
-    jest.advanceTimersByTime(50);
 
-    // Should fire at 100ms from first call, not 150ms
+    // Should fire at 50ms from first call, not 70ms
+    await sleep(40);
     expect(onRefresh).toHaveBeenCalledTimes(1);
 
     controller.dispose();
   });
 
-  it("coalesces calls during rate-limit window", () => {
+  it("coalesces calls during rate-limit window", async () => {
     const onRefresh = jest.fn<() => void>();
-    const controller = new RefreshController({ onRefresh, debounceMs: 100 });
+    const controller = new RefreshController({ onRefresh, debounceMs: 50 });
 
     controller.schedule();
     controller.schedule();
     controller.schedule();
 
     expect(onRefresh).not.toHaveBeenCalled();
 
-    jest.advanceTimersByTime(100);
+    await sleep(60);
 
     expect(onRefresh).toHaveBeenCalledTimes(1);
 
     controller.dispose();
   });
 
-  it("requestImmediate() bypasses rate-limit timer", () => {
+  it("requestImmediate() bypasses rate-limit timer", async () => {
     const onRefresh = jest.fn<() => void>();
-    const controller = new RefreshController({ onRefresh, debounceMs: 100 });
+    const controller = new RefreshController({ onRefresh, debounceMs: 50 });
 
     controller.schedule();
     expect(onRefresh).not.toHaveBeenCalled();
@@ -53,7 +54,7 @@ describe("RefreshController", () => {
     expect(onRefresh).toHaveBeenCalledTimes(1);
 
     // Original timer should be cleared
-    jest.advanceTimersByTime(100);
+    await sleep(60);
     expect(onRefresh).toHaveBeenCalledTimes(1);
 
     controller.dispose();
@@ -80,33 +81,45 @@ describe("RefreshController", () => {
   });
 
   it("schedule() during in-flight queues refresh for after completion", async () => {
-    let resolveRefresh: () => void;
+    setSystemTime(0);
+
+    const resolvers: Array<() => void> = [];
     const onRefresh = jest.fn(
       () =>
         new Promise<void>((resolve) => {
-          resolveRefresh = resolve;
+          resolvers.push(resolve);
         })
     );
 
-    const controller = new RefreshController({ onRefresh, debounceMs: 100 });
+    const controller = new RefreshController({ onRefresh, debounceMs: 50 });
 
     // Start first refresh
     controller.requestImmediate();
     expect(onRefresh).toHaveBeenCalledTimes(1);
+    expect(resolvers).toHaveLength(1);
 
     // schedule() while in-flight should queue, not start timer
     controller.schedule();
 
     // Complete the first refresh and let .finally() run
-    resolveRefresh!();
+    resolvers[0]();
     await Promise.resolve();
     await Promise.resolve(); // Extra tick for .finally()
 
-    // Should trigger follow-up refresh, but never more frequently than the minimum interval.
-    // First tick runs the post-flight setTimeout(0), then we wait out the min interval.
-    jest.advanceTimersByTime(0);
-    jest.advanceTimersByTime(500);
+    // Make the follow-up refresh eligible immediately (skip MIN_REFRESH_INTERVAL_MS wait)
+    setSystemTime(1000);
+
+    // Allow post-flight setTimeout(0) to run
+    await sleep(0);
+    await sleep(10);
+
     expect(onRefresh).toHaveBeenCalledTimes(2);
+    expect(resolvers).toHaveLength(2);
+
+    // Resolve the follow-up refresh promise to avoid leaving it in-flight.
+    resolvers[1]();
+    await Promise.resolve();
+    await Promise.resolve();
 
     controller.dispose();
   });
@@ -131,43 +144,43 @@ describe("RefreshController", () => {
     controller.dispose();
   });
 
-  it("dispose() cleans up debounce timer", () => {
+  it("dispose() cleans up debounce timer", async () => {
     const onRefresh = jest.fn<() => void>();
-    const controller = new RefreshController({ onRefresh, debounceMs: 100 });
+    const controller = new RefreshController({ onRefresh, debounceMs: 50 });
 
     controller.schedule();
     controller.dispose();
 
-    jest.advanceTimersByTime(100);
+    await sleep(60);
 
     expect(onRefresh).not.toHaveBeenCalled();
   });
 
-  it("does not refresh after dispose", () => {
+  it("does not refresh after dispose", async () => {
     const onRefresh = jest.fn<() => void>();
-    const controller = new RefreshController({ onRefresh, debounceMs: 100 });
+    const controller = new RefreshController({ onRefresh, debounceMs: 50 });
 
     controller.dispose();
     controller.schedule();
     controller.requestImmediate();
 
-    jest.advanceTimersByTime(100);
+    await sleep(60);
 
     expect(onRefresh).not.toHaveBeenCalled();
   });
 
-  it("requestImmediate() bypasses isPaused check (for manual refresh)", () => {
+  it("requestImmediate() bypasses isPaused check (for manual refresh)", async () => {
     const onRefresh = jest.fn<() => void>();
     const paused = true;
     const controller = new RefreshController({
       onRefresh,
-      debounceMs: 100,
+      debounceMs: 50,
       isPaused: () => paused,
     });
 
     // schedule() should be blocked by isPaused
     controller.schedule();
-    jest.advanceTimersByTime(100);
+    await sleep(60);
     expect(onRefresh).not.toHaveBeenCalled();
 
     // requestImmediate() should bypass isPaused (manual refresh)
@@ -177,18 +190,18 @@ describe("RefreshController", () => {
     controller.dispose();
   });
 
-  it("schedule() respects isPaused and flushes on notifyUnpaused", () => {
+  it("schedule() respects isPaused and flushes on notifyUnpaused", async () => {
     const onRefresh = jest.fn<() => void>();
     let paused = true;
     const controller = new RefreshController({
       onRefresh,
-      debounceMs: 100,
+      debounceMs: 50,
       isPaused: () => paused,
     });
 
     // schedule() should queue but not execute while paused
     controller.schedule();
-    jest.advanceTimersByTime(100);
+    await sleep(60);
     expect(onRefresh).not.toHaveBeenCalled();
 
     // Unpausing should flush the pending refresh
@@ -199,9 +212,12 @@ describe("RefreshController", () => {
     controller.dispose();
   });
 
-  it("lastRefreshInfo tracks trigger and timestamp", () => {
+  it("lastRefreshInfo tracks trigger and timestamp", async () => {
+    // This test needs to avoid MIN_REFRESH_INTERVAL_MS; use setSystemTime() to simulate time passing.
+    setSystemTime(0);
+
     const onRefresh = jest.fn<() => void>();
-    const controller = new RefreshController({ onRefresh, debounceMs: 100 });
+    const controller = new RefreshController({ onRefresh, debounceMs: 20 });
 
     expect(controller.lastRefreshInfo).toBeNull();
 
@@ -213,37 +229,44 @@ describe("RefreshController", () => {
     expect(controller.lastRefreshInfo!.timestamp).toBeGreaterThanOrEqual(beforeManual);
 
     // Scheduled refresh should record "scheduled" trigger
+    setSystemTime(1000);
     controller.schedule();
-    jest.advanceTimersByTime(500);
+    await sleep(30);
     expect(controller.lastRefreshInfo!.trigger).toBe("scheduled");
 
     // Priority refresh should record "priority" trigger
+    setSystemTime(2000);
     controller.schedulePriority();
-    jest.advanceTimersByTime(500);
+    await sleep(30);
     expect(controller.lastRefreshInfo!.trigger).toBe("priority");
 
     controller.dispose();
   });
 
-  it("onRefreshComplete callback is called with refresh info", () => {
+  it("onRefreshComplete callback is called with refresh info", async () => {
+    // This test needs to avoid MIN_REFRESH_INTERVAL_MS; use setSystemTime() to simulate time passing.
+    setSystemTime(0);
+
     const onRefresh = jest.fn<() => void>();
     const onRefreshComplete = jest.fn<(info: { trigger: string; timestamp: number }) => void>();
     const controller = new RefreshController({
       onRefresh,
       onRefreshComplete,
-      debounceMs: 100,
+      debounceMs: 20,
     });
 
     expect(onRefreshComplete).not.toHaveBeenCalled();
 
     controller.requestImmediate();
     expect(onRefreshComplete).toHaveBeenCalledTimes(1);
     expect(onRefreshComplete).toHaveBeenCalledWith(
+      // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
       expect.objectContaining({ trigger: "manual", timestamp: expect.any(Number) })
     );
 
+    setSystemTime(1000);
     controller.schedule();
-    jest.advanceTimersByTime(500);
+    await sleep(30);
     expect(onRefreshComplete).toHaveBeenCalledTimes(2);
     expect(onRefreshComplete).toHaveBeenLastCalledWith(
       expect.objectContaining({ trigger: "scheduled" })

src/node/services/tools/task.test.ts

@@ -31,7 +31,7 @@ describe("task tool", () => {
 
     const result: unknown = await Promise.resolve(
       tool.execute!(
-        { subagent_type: "explore", prompt: "do it", run_in_background: true },
+        { subagent_type: "explore", prompt: "do it", title: "Child task", run_in_background: true },
         mockToolCallOptions
       )
     );
@@ -63,7 +63,12 @@ describe("task tool", () => {
 
     const result: unknown = await Promise.resolve(
       tool.execute!(
-        { subagent_type: "explore", prompt: "do it", run_in_background: false },
+        {
+          subagent_type: "explore",
+          prompt: "do it",
+          title: "Child task",
+          run_in_background: false,
+        },
         mockToolCallOptions
       )
     );
@@ -95,7 +100,10 @@ describe("task tool", () => {
     let caught: unknown = null;
     try {
       await Promise.resolve(
-        tool.execute!({ subagent_type: "explore", prompt: "do it" }, mockToolCallOptions)
+        tool.execute!(
+          { subagent_type: "explore", prompt: "do it", title: "Child task" },
+          mockToolCallOptions
+        )
       );
     } catch (error: unknown) {
       caught = error;
@@ -131,7 +139,10 @@ describe("task tool", () => {
     let caught: unknown = null;
     try {
       await Promise.resolve(
-        tool.execute!({ subagent_type: "exec", prompt: "do it" }, mockToolCallOptions)
+        tool.execute!(
+          { subagent_type: "exec", prompt: "do it", title: "Child task" },
+          mockToolCallOptions
+        )
       );
     } catch (error: unknown) {
       caught = error;