From e1c99cd24c00ff288f983db7ad59c660cb930df5 Mon Sep 17 00:00:00 2001
From: Hongming Wang <hongmingwang.rabbit@users.noreply.github.com>
Date: Mon, 4 May 2026 03:18:00 -0700
Subject: [PATCH] Pin the visibility gate behavior, not just cadence
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Self-review on PR #2723 caught a coverage gap: the existing
"visibility gate" describe block actually tested cadence (10s/30s
timing), not the gate itself. If a refactor dropped the
`if (!visible) return` line, the cadence test would still pass
because the effect would still fire every 30s — the regression would
silently ship.

New test renders with comms-returning mock so the panel renders, clicks
the close button, advances 60s, asserts no further fetches occur.

Discipline-verified: removed `if (!visible) return` from the source,
test fails as expected. Restored, test passes.

Same failure mode as PR #434 (test asserted broken behavior) — pin
what you claim to fix, not the easy substring.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 .../__tests__/CommunicationOverlay.test.tsx   | 59 ++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/canvas/src/components/__tests__/CommunicationOverlay.test.tsx b/canvas/src/components/__tests__/CommunicationOverlay.test.tsx
index 1612f8eb..3bed0076 100644
--- a/canvas/src/components/__tests__/CommunicationOverlay.test.tsx
+++ b/canvas/src/components/__tests__/CommunicationOverlay.test.tsx
@@ -99,7 +99,7 @@ describe("CommunicationOverlay — fan-out cap", () => {
   });
 });
 
-describe("CommunicationOverlay — visibility gate", () => {
+describe("CommunicationOverlay — cadence", () => {
   it("uses 30s interval cadence (was 10s pre-fix)", async () => {
     await act(async () => {
       render(<CommunicationOverlay />);
@@ -119,3 +119,60 @@ describe("CommunicationOverlay — visibility gate", () => {
     expect(mockGet).toHaveBeenCalledTimes(6); // +3 from second tick
   });
 });
+
+describe("CommunicationOverlay — visibility gate", () => {
+  // The visibility gate is the dial that drops collapsed-panel polling
+  // to ZERO. The cadence test above can't catch its removal — if a
+  // refactor dropped `if (!visible) return`, the cadence test would
+  // still pass because the effect would still fire every 30s.
+  //
+  // Direct probe: render with comms-returning mock so the panel
+  // actually renders (close button only exists in the expanded panel,
+  // not the collapsed button-state). Click close, advance the clock,
+  // assert no further fetches.
+  it("stops polling after the user collapses the panel", async () => {
+    // Mock returns one a2a_send so comms.length > 0 → panel renders →
+    // close button accessible.
+    mockGet.mockResolvedValue([
+      {
+        id: "act-1",
+        workspace_id: "ws-1",
+        activity_type: "a2a_send",
+        source_id: "ws-1",
+        target_id: "ws-2",
+        summary: "test",
+        status: "completed",
+        duration_ms: 100,
+        created_at: new Date().toISOString(),
+      },
+    ]);
+
+    const { getByLabelText } = await act(async () => {
+      return render(<CommunicationOverlay />);
+    });
+    // Drain pending microtasks (resolves the await in fetchComms) so
+    // setComms lands and the panel renders. Don't advance time — that
+    // would fire the next interval tick and pollute the assertion.
+    await act(async () => {
+      await Promise.resolve();
+      await Promise.resolve();
+      await Promise.resolve();
+    });
+    // Initial mount polled 3 workspaces.
+    expect(mockGet).toHaveBeenCalledTimes(3);
+    mockGet.mockClear();
+
+    // Click the close button. Synchronous getByLabelText avoids
+    // findBy's internal setTimeout (deadlocks under useFakeTimers).
+    const closeBtn = getByLabelText("Close communications panel");
+    await act(async () => {
+      fireEvent.click(closeBtn);
+    });
+
+    // Advance well past the 30s cadence — gate should suppress the tick.
+    await act(async () => {
+      vi.advanceTimersByTime(60_000);
+    });
+    expect(mockGet).not.toHaveBeenCalled();
+  });
+});