diff --git a/canvas/src/components/A2ATopologyOverlay.tsx b/canvas/src/components/A2ATopologyOverlay.tsx index efd4f0ff..58f2d976 100644 --- a/canvas/src/components/A2ATopologyOverlay.tsx +++ b/canvas/src/components/A2ATopologyOverlay.tsx @@ -138,14 +138,37 @@ export function A2ATopologyOverlay() { // Stable Zustand action reference — safe to call inside effects const setA2AEdges = useCanvasStore((s) => s.setA2AEdges); - // Read the nodes array as a primitive ref; derive visible IDs outside the selector - const nodes = useCanvasStore((s) => s.nodes); + // Subscribe to a STABLE STRING KEY of visible workspace IDs, not the + // nodes array itself. Zustand returns a new array reference on every + // store update (status flips, position drags, peer-discovery writes, + // workspace-tab opens, etc.) — even when the set of visible IDs is + // unchanged. Selecting a sorted-CSV string makes Zustand's default + // shallow-equal short-circuit the re-render unless the actual ID set + // changes. + // + // Why this matters: previously visibleIds was useMemo'd on `nodes`, so + // the array reference recreated on every store mutation. fetchAndUpdate + // (useCallback'd on visibleIds) then recreated, the useEffect re-fired, + // it tore down the 60s setInterval and immediately re-ran the fan-out. + // With ~5 store updates/second from heartbeats + polling, the canvas + // hammered /workspaces//activity?type=delegation 5×N requests/sec + // until edge rate-limit kicked in with HTTP 429. The recursive React + // render trace in the original bug report (uE → ux → uE → ux ...) is + // the symptom of this re-render storm. + // + // The fix is purely the dependency-stability change here; the fetch + // logic is unchanged. + const visibleIdsKey = useCanvasStore((s) => + s.nodes + .filter((n) => !n.hidden) + .map((n) => n.id) + .sort() + .join(",") + ); - // IDs of visible (non-nested, non-hidden) workspace nodes. - // Recomputed only when the nodes array reference changes. const visibleIds = useMemo( - () => nodes.filter((n) => !n.hidden).map((n) => n.id), - [nodes] + () => (visibleIdsKey ? visibleIdsKey.split(",") : []), + [visibleIdsKey] ); // Fetch delegation activity for all visible workspaces and rebuild overlay edges. diff --git a/canvas/src/components/__tests__/A2ATopologyOverlay.test.tsx b/canvas/src/components/__tests__/A2ATopologyOverlay.test.tsx index 8d4dba00..6cdd19a7 100644 --- a/canvas/src/components/__tests__/A2ATopologyOverlay.test.tsx +++ b/canvas/src/components/__tests__/A2ATopologyOverlay.test.tsx @@ -296,4 +296,75 @@ describe("A2ATopologyOverlay component", () => { // setA2AEdges should still be called with an empty array expect(mockStoreState.setA2AEdges).toHaveBeenCalled(); }); + + // Regression for the 2026-05-04 render-loop incident: + // tenant heartbeats / status flips / peer-discovery writes mutated + // canvas store .nodes ~5x/sec. Previously visibleIds was useMemo'd on + // [nodes] so the array reference recreated on every store mutation, + // causing fetchAndUpdate to recreate, the useEffect to re-fire, and + // the 60-second polling fan-out to fire on EVERY store update. With + // 5 visible workspaces and 5 store updates/sec, the canvas hammered + // /workspaces//activity?type=delegation 25×/sec until edge rate + // -limit returned 429 (per browser console captured by user). + // + // Fix: select a stable string key (sorted CSV of IDs) from Zustand + // so the selector's shallow-equal short-circuit prevents re-renders + // when the actual ID set hasn't changed. + // + // This test verifies the fetch fires ONCE on mount + only re-fires + // when the visible ID set actually changes, NOT on every nodes[] + // reference change. + it("does not re-fetch when nodes[] reference changes but visible IDs are the same", async () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + mockGet.mockResolvedValue([] as any); + const { rerender } = render(); + await act(async () => { await Promise.resolve(); await Promise.resolve(); }); + + const callsAfterMount = mockGet.mock.calls.length; + // Sanity: 2 visible nodes (ws-a, ws-b) → 2 fan-out requests on mount + expect(callsAfterMount).toBe(2); + + // Simulate a store mutation that changes the nodes array reference + // (e.g. status flip on a node) WITHOUT changing the set of visible + // IDs. Pre-fix: this triggered a re-fetch storm. Post-fix: the + // sorted-CSV selector returns the same key, Zustand's shallow-equal + // short-circuits, useMemo keeps the same visibleIds, fetchAndUpdate + // keeps the same identity, useEffect does NOT re-fire. + mockStoreState.nodes = [ + { id: "ws-a", hidden: false, data: { newStatus: "online" } }, // mutated + { id: "ws-b", hidden: false, data: {} }, + { id: "ws-hidden", hidden: true, data: {} }, + ]; + rerender(); + await act(async () => { await Promise.resolve(); await Promise.resolve(); }); + + // No additional fetches should have fired. + expect(mockGet.mock.calls.length).toBe(callsAfterMount); + }); + + it("re-fetches when the visible ID set actually changes", async () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + mockGet.mockResolvedValue([] as any); + const { rerender } = render(); + await act(async () => { await Promise.resolve(); await Promise.resolve(); }); + + const callsAfterMount = mockGet.mock.calls.length; + expect(callsAfterMount).toBe(2); + + // Add a new visible workspace — the visible-ID-set actually changed. + mockStoreState.nodes = [ + { id: "ws-a", hidden: false, data: {} }, + { id: "ws-b", hidden: false, data: {} }, + { id: "ws-c", hidden: false, data: {} }, // NEW + { id: "ws-hidden", hidden: true, data: {} }, + ]; + rerender(); + await act(async () => { await Promise.resolve(); await Promise.resolve(); }); + + // Should have fetched the additional workspace + the existing two + // (the effect re-fires once with the new ID set). Total: 2 + 3 = 5. + expect(mockGet.mock.calls.length).toBe(callsAfterMount + 3); + const allPaths = mockGet.mock.calls.map(([p]) => p as string); + expect(allPaths.some((p) => p.includes("ws-c"))).toBe(true); + }); });