🤖 fix: prevent double-compact during force compaction

src/browser/components/AIView.tsx

@@ -44,6 +44,7 @@ import { executeCompaction } from "@/browser/utils/chatCommands";
 import { useProviderOptions } from "@/browser/hooks/useProviderOptions";
 import { useAutoCompactionSettings } from "../hooks/useAutoCompactionSettings";
 import { useSendMessageOptions } from "@/browser/hooks/useSendMessageOptions";
+import { useForceCompaction } from "@/browser/hooks/useForceCompaction";
 import { useAPI } from "@/browser/contexts/API";
 
 interface AIViewProps {
@@ -139,9 +140,6 @@ const AIViewInner: React.FC<AIViewProps> = ({
     undefined
   );
 
-  // Track if we've already triggered force compaction for this stream
-  const forceCompactionTriggeredRef = useRef<string | null>(null);
-
   // Extract state from workspace state
   const { messages, canInterrupt, isCompacting, loading, currentModel } = workspaceState;
 
@@ -158,41 +156,24 @@ const AIViewInner: React.FC<AIViewProps> = ({
   // Show warning when: shouldShowWarning flag is true AND not currently compacting
   const shouldShowCompactionWarning = !isCompacting && autoCompactionResult.shouldShowWarning;
 
-  // Force compaction when live usage shows we're about to hit context limit
-  useEffect(() => {
-    if (
-      !autoCompactionResult.shouldForceCompact ||
-      !canInterrupt ||
-      isCompacting ||
-      forceCompactionTriggeredRef.current === activeStreamMessageId
-    ) {
-      return;
-    }
-
-    forceCompactionTriggeredRef.current = activeStreamMessageId ?? null;
+  // Handle force compaction callback - memoized to avoid effect re-runs
+  const handleForceCompaction = useCallback(() => {
     if (!api) return;
     void executeCompaction({
       api,
       workspaceId,
       sendMessageOptions: pendingSendOptions,
       continueMessage: { text: "Continue with the current task" },
     });
-  }, [
-    autoCompactionResult.shouldForceCompact,
+  }, [api, workspaceId, pendingSendOptions]);
+
+  // Force compaction when live usage shows we're about to hit context limit
+  useForceCompaction({
+    shouldForceCompact: autoCompactionResult.shouldForceCompact,
     canInterrupt,
     isCompacting,
-    activeStreamMessageId,
-    workspaceId,
-    pendingSendOptions,
-    api,
-  ]);
-
-  // Reset force compaction trigger when stream ends
-  useEffect(() => {
-    if (!canInterrupt) {
-      forceCompactionTriggeredRef.current = null;
-    }
-  }, [canInterrupt]);
+    onTrigger: handleForceCompaction,
+  });
 
   // Auto-retry state - minimal setter for keybinds and message sent handler
   // RetryBarrier manages its own state, but we need this for interrupt keybind

src/browser/hooks/useForceCompaction.test.ts

@@ -0,0 +1,261 @@
+/**
+ * Tests for useForceCompaction hook
+ *
+ * Verifies the force compaction behavior when usage exceeds the context limit.
+ * The key invariant: force compaction should only trigger ONCE until usage drops below threshold.
+ *
+ * Bug being tested (now fixed):
+ * When compaction completes and continues with a follow-up message, the guard was reset
+ * because canInterrupt changed, allowing a second compaction to trigger immediately.
+ */
+
+import { describe, test, expect, mock, beforeEach, afterEach, type Mock } from "bun:test";
+import { renderHook, act, cleanup } from "@testing-library/react";
+import { GlobalWindow } from "happy-dom";
+import { useForceCompaction, type ForceCompactionParams } from "./useForceCompaction";
+
+describe("useForceCompaction", () => {
+  let onTrigger: Mock<() => void>;
+
+  beforeEach(() => {
+    // Set up DOM environment for React
+    globalThis.window = new GlobalWindow() as unknown as Window & typeof globalThis;
+    globalThis.document = globalThis.window.document;
+
+    onTrigger = mock(() => undefined);
+  });
+
+  afterEach(() => {
+    cleanup();
+    globalThis.window = undefined as unknown as Window & typeof globalThis;
+    globalThis.document = undefined as unknown as Document;
+  });
+
+  test("triggers compaction when shouldForceCompact and canInterrupt are true", () => {
+    renderHook(() =>
+      useForceCompaction({
+        shouldForceCompact: true,
+        canInterrupt: true,
+        isCompacting: false,
+        onTrigger,
+      })
+    );
+
+    expect(onTrigger).toHaveBeenCalledTimes(1);
+  });
+
+  test("does not trigger when shouldForceCompact is false", () => {
+    renderHook(() =>
+      useForceCompaction({
+        shouldForceCompact: false,
+        canInterrupt: true,
+        isCompacting: false,
+        onTrigger,
+      })
+    );
+
+    expect(onTrigger).not.toHaveBeenCalled();
+  });
+
+  test("does not trigger when canInterrupt is false", () => {
+    renderHook(() =>
+      useForceCompaction({
+        shouldForceCompact: true,
+        canInterrupt: false,
+        isCompacting: false,
+        onTrigger,
+      })
+    );
+
+    expect(onTrigger).not.toHaveBeenCalled();
+  });
+
+  test("does not trigger when isCompacting is true", () => {
+    renderHook(() =>
+      useForceCompaction({
+        shouldForceCompact: true,
+        canInterrupt: true,
+        isCompacting: true,
+        onTrigger,
+      })
+    );
+
+    expect(onTrigger).not.toHaveBeenCalled();
+  });
+
+  test("does NOT trigger double compaction when continue message starts after compaction", () => {
+    // This is the key test for the bug fix!
+    //
+    // Timeline being tested:
+    // 1. Stream running, usage high: shouldForceCompact=true → compaction triggers
+    // 2. Compaction runs, completes: canInterrupt becomes false briefly
+    // 3. Continue message starts: canInterrupt=true again, isCompacting=false
+    // 4. BUG (before fix): second compaction would trigger because guard was reset
+    // 5. FIX: guard stays set until shouldForceCompact becomes false
+
+    // Phase 1: Initial streaming state exceeding threshold
+    const { rerender } = renderHook((props: ForceCompactionParams) => useForceCompaction(props), {
+      initialProps: {
+        shouldForceCompact: true,
+        canInterrupt: true,
+        isCompacting: false,
+        onTrigger,
+      },
+    });
+
+    // First compaction should trigger
+    expect(onTrigger).toHaveBeenCalledTimes(1);
+
+    // Phase 2: Compaction completes, stream ends briefly
+    act(() => {
+      rerender({
+        shouldForceCompact: true, // Usage still high (compaction summary not yet reflected)
+        canInterrupt: false, // Stream ended
+        isCompacting: false,
+        onTrigger,
+      });
+    });
+
+    // Still only 1 call (canInterrupt is false)
+    expect(onTrigger).toHaveBeenCalledTimes(1);
+
+    // Phase 3: Continue message stream starts - THIS IS WHERE THE BUG WOULD OCCUR
+    act(() => {
+      rerender({
+        shouldForceCompact: true, // Usage still high - would trigger second compaction without fix
+        canInterrupt: true, // New stream started
+        isCompacting: false,
+        onTrigger,
+      });
+    });
+
+    // Key assertion: still only 1 call, NOT 2!
+    expect(onTrigger).toHaveBeenCalledTimes(1);
+  });
+
+  test("allows new compaction after usage drops below threshold", () => {
+    // Phase 1: First compaction triggers
+    const { rerender } = renderHook((props: ForceCompactionParams) => useForceCompaction(props), {
+      initialProps: {
+        shouldForceCompact: true,
+        canInterrupt: true,
+        isCompacting: false,
+        onTrigger,
+      },
+    });
+
+    expect(onTrigger).toHaveBeenCalledTimes(1);
+
+    // Phase 2: Compaction succeeds, usage drops below threshold
+    act(() => {
+      rerender({
+        shouldForceCompact: false, // Usage dropped!
+        canInterrupt: false,
+        isCompacting: false,
+        onTrigger,
+      });
+    });
+
+    // Still only 1 call
+    expect(onTrigger).toHaveBeenCalledTimes(1);
+
+    // Phase 3: Much later, usage builds up again and exceeds threshold
+    act(() => {
+      rerender({
+        shouldForceCompact: true, // Usage exceeded again
+        canInterrupt: true,
+        isCompacting: false,
+        onTrigger,
+      });
+    });
+
+    // New compaction should be allowed since usage dropped and rose again
+    expect(onTrigger).toHaveBeenCalledTimes(2);
+  });
+
+  test("does not re-trigger on prop changes while guard is active", () => {
+    // Ensure multiple re-renders with same conditions don't trigger multiple times
+    const { rerender } = renderHook((props: ForceCompactionParams) => useForceCompaction(props), {
+      initialProps: {
+        shouldForceCompact: true,
+        canInterrupt: true,
+        isCompacting: false,
+        onTrigger,
+      },
+    });
+
+    expect(onTrigger).toHaveBeenCalledTimes(1);
+
+    // Re-render with same props (simulating React re-render)
+    act(() => {
+      rerender({
+        shouldForceCompact: true,
+        canInterrupt: true,
+        isCompacting: false,
+        onTrigger,
+      });
+    });
+
+    // Still only 1 call
+    expect(onTrigger).toHaveBeenCalledTimes(1);
+
+    // Re-render again
+    act(() => {
+      rerender({
+        shouldForceCompact: true,
+        canInterrupt: true,
+        isCompacting: false,
+        onTrigger,
+      });
+    });
+
+    // Still only 1 call
+    expect(onTrigger).toHaveBeenCalledTimes(1);
+  });
+
+  test("maintains guard across rapid prop changes", () => {
+    // Simulate rapid prop changes that might occur in React concurrent mode
+    const { rerender } = renderHook((props: ForceCompactionParams) => useForceCompaction(props), {
+      initialProps: {
+        shouldForceCompact: true,
+        canInterrupt: true,
+        isCompacting: false,
+        onTrigger,
+      },
+    });
+
+    // First trigger
+    expect(onTrigger).toHaveBeenCalledTimes(1);
+
+    // Rapid transitions simulating complex UI updates
+    act(() => {
+      rerender({
+        shouldForceCompact: true,
+        canInterrupt: false, // Brief interruption
+        isCompacting: false,
+        onTrigger,
+      });
+    });
+
+    act(() => {
+      rerender({
+        shouldForceCompact: true,
+        canInterrupt: true, // Back to interruptible
+        isCompacting: true, // Now actually compacting
+        onTrigger,
+      });
+    });
+
+    act(() => {
+      rerender({
+        shouldForceCompact: true,
+        canInterrupt: true,
+        isCompacting: false, // Compaction done
+        onTrigger,
+      });
+    });
+
+    // Still only the initial trigger - guard prevented all re-triggers
+    expect(onTrigger).toHaveBeenCalledTimes(1);
+  });
+});

src/browser/hooks/useForceCompaction.ts

@@ -0,0 +1,57 @@
+/**
+ * Force compaction hook - manages automatic compaction when context limit is approached
+ *
+ * Triggers compaction when:
+ * - shouldForceCompact is true (usage exceeds threshold + buffer)
+ * - canInterrupt is true (there's an active stream to interrupt)
+ * - isCompacting is false (not already compacting)
+ * - Haven't already triggered force compaction this cycle
+ *
+ * The key invariant: force compaction triggers ONCE per threshold breach.
+ * The guard only resets when shouldForceCompact becomes false (successful compaction reduced context).
+ */
+
+import { useEffect, useRef } from "react";
+
+export interface ForceCompactionParams {
+  shouldForceCompact: boolean;
+  canInterrupt: boolean;
+  isCompacting: boolean;
+  onTrigger: () => void;
+}
+
+/**
+ * Hook to manage force compaction triggering
+ *
+ * @returns Whether force compaction has been triggered (for testing/debugging)
+ */
+export function useForceCompaction(params: ForceCompactionParams): boolean {
+  const { shouldForceCompact, canInterrupt, isCompacting, onTrigger } = params;
+
+  // Track if we've already triggered force compaction (reset when usage drops)
+  const forceCompactionTriggeredRef = useRef<boolean>(false);
+
+  // Force compaction when live usage shows we're about to hit context limit
+  useEffect(() => {
+    if (
+      !shouldForceCompact ||
+      !canInterrupt ||
+      isCompacting ||
+      forceCompactionTriggeredRef.current
+    ) {
+      return;
+    }
+
+    forceCompactionTriggeredRef.current = true;
+    onTrigger();
+  }, [shouldForceCompact, canInterrupt, isCompacting, onTrigger]);
+
+  // Reset force compaction trigger when usage drops below threshold (successful compaction)
+  useEffect(() => {
+    if (!shouldForceCompact) {
+      forceCompactionTriggeredRef.current = false;
+    }
+  }, [shouldForceCompact]);
+
+  return forceCompactionTriggeredRef.current;
+}