diff --git a/frontend/src/components/LozengeFlyContext.test.tsx b/frontend/src/components/LozengeFlyContext.test.tsx index f9c7ebc..7d72d8a 100644 --- a/frontend/src/components/LozengeFlyContext.test.tsx +++ b/frontend/src/components/LozengeFlyContext.test.tsx @@ -956,3 +956,289 @@ describe("FlyingLozengeClone initial non-flying render", () => { }); }); }); + +// ─── Bug 137: Race condition on rapid pipeline updates ──────────────────── + +describe("Bug 137: no animation actions lost during rapid pipeline updates", () => { + beforeEach(() => { + vi.useFakeTimers(); + Element.prototype.getBoundingClientRect = vi.fn().mockReturnValue({ + left: 100, + top: 50, + right: 180, + bottom: 70, + width: 80, + height: 20, + x: 100, + y: 50, + toJSON: () => ({}), + }); + vi.spyOn(window, "requestAnimationFrame").mockImplementation((cb) => { + cb(0); + return 0; + }); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + it("rapid agent swap: first timeout does not prematurely reveal slot lozenge", async () => { + const empty = makePipeline(); + const withCoder1 = makePipeline({ + current: [ + { + story_id: "137_rapid_swap", + name: "Rapid Swap", + error: null, + agent: { agent_name: "coder-1", model: "sonnet", status: "running" }, + }, + ], + }); + const withCoder2 = makePipeline({ + current: [ + { + story_id: "137_rapid_swap", + name: "Rapid Swap", + error: null, + agent: { agent_name: "coder-2", model: "haiku", status: "running" }, + }, + ], + }); + + const { rerender } = render( + + + + + , + ); + + // First update: assign coder-1 → fly-in animation #1 starts + await act(async () => { + rerender( + + + + + , + ); + }); + + // Slot should be hidden (fly-in in progress) + const lozenge = screen.getByTestId("slot-lozenge-137_rapid_swap"); + expect(lozenge.style.opacity).toBe("0"); + + // Rapid swap at 200ms: coder-1 → coder-2 (before first animation's 500ms timeout) + await act(async () => { + vi.advanceTimersByTime(200); + }); + + await act(async () => { + rerender( + + + + + , + ); + }); + + // Slot should still be hidden (new fly-in for coder-2 is in progress) + expect(lozenge.style.opacity).toBe("0"); + + // At 300ms after first animation started (500ms total from start), + // the FIRST animation's timeout fires. It must NOT reveal the slot. + await act(async () => { + vi.advanceTimersByTime(300); + }); + + // BUG: Without fix, the first timeout clears pendingFlyIns for this story, + // revealing the slot while coder-2's fly-in is still in progress. + expect(lozenge.style.opacity).toBe("0"); + }); + + it("slot lozenge reveals correctly after the LAST animation completes", async () => { + const empty = makePipeline(); + const withCoder1 = makePipeline({ + current: [ + { + story_id: "137_reveal_last", + name: "Reveal Last", + error: null, + agent: { agent_name: "coder-1", model: null, status: "running" }, + }, + ], + }); + const withCoder2 = makePipeline({ + current: [ + { + story_id: "137_reveal_last", + name: "Reveal Last", + error: null, + agent: { agent_name: "coder-2", model: null, status: "running" }, + }, + ], + }); + + const { rerender } = render( + + + + + , + ); + + // First animation + await act(async () => { + rerender( + + + + + , + ); + }); + + // Swap at 200ms + await act(async () => { + vi.advanceTimersByTime(200); + }); + + await act(async () => { + rerender( + + + + + , + ); + }); + + const lozenge = screen.getByTestId("slot-lozenge-137_reveal_last"); + + // After the second animation's full 500ms, slot should reveal + await act(async () => { + vi.advanceTimersByTime(600); + }); + + expect(lozenge.style.opacity).toBe("1"); + }); +}); + +describe("Bug 137: animations remain functional through sustained agent activity", () => { + beforeEach(() => { + vi.useFakeTimers(); + Element.prototype.getBoundingClientRect = vi.fn().mockReturnValue({ + left: 100, + top: 50, + right: 180, + bottom: 70, + width: 80, + height: 20, + x: 100, + y: 50, + toJSON: () => ({}), + }); + vi.spyOn(window, "requestAnimationFrame").mockImplementation((cb) => { + cb(0); + return 0; + }); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + it("fly-in still works after multiple rapid swaps have completed", async () => { + const empty = makePipeline(); + const makeWith = (agentName: string) => + makePipeline({ + current: [ + { + story_id: "137_sustained", + name: "Sustained", + error: null, + agent: { agent_name: agentName, model: null, status: "running" }, + }, + ], + }); + + const { rerender } = render( + + + + + + , + ); + + // Rapid-fire: assign coder-1, then swap to coder-2 at 100ms + const p1 = makeWith("coder-1"); + await act(async () => { + rerender( + + + + + + , + ); + }); + + await act(async () => { + vi.advanceTimersByTime(100); + }); + + const p2 = makeWith("coder-2"); + await act(async () => { + rerender( + + + + + + , + ); + }); + + // Let all animations complete + await act(async () => { + vi.advanceTimersByTime(1000); + }); + + const lozenge = screen.getByTestId("slot-lozenge-137_sustained"); + expect(lozenge.style.opacity).toBe("1"); + + // Now assign coder-3 — a fresh fly-in should still work + const p3 = makeWith("coder-3"); + await act(async () => { + rerender( + + + + + + , + ); + }); + + // Slot should be hidden again for the new fly-in + expect(lozenge.style.opacity).toBe("0"); + + // A flying clone should exist + const clone = document.body.querySelector( + '[data-testid^="flying-lozenge-fly-in"]', + ); + expect(clone).not.toBeNull(); + + // After animation completes, slot reveals + await act(async () => { + vi.advanceTimersByTime(600); + }); + + expect(lozenge.style.opacity).toBe("1"); + }); +}); diff --git a/frontend/src/components/LozengeFlyContext.tsx b/frontend/src/components/LozengeFlyContext.tsx index 386dc49..a6bc279 100644 --- a/frontend/src/components/LozengeFlyContext.tsx +++ b/frontend/src/components/LozengeFlyContext.tsx @@ -101,6 +101,11 @@ export function LozengeFlyProvider({ const pendingFlyInActionsRef = useRef([]); const pendingFlyOutActionsRef = useRef([]); + // Track the active animation ID per story/agent so stale timeouts + // from superseded animations don't prematurely clear state. + const activeFlyInPerStory = useRef>(new Map()); + const activeFlyOutPerAgent = useRef>(new Map()); + const [pendingFlyIns, setPendingFlyIns] = useState>( new Set(), ); @@ -258,6 +263,7 @@ export function LozengeFlyProvider({ const rosterRect = rosterEl.getBoundingClientRect(); const id = `fly-in-${action.agentName}-${action.storyId}-${Date.now()}`; + activeFlyInPerStory.current.set(action.storyId, id); setFlyingLozenges((prev) => [ ...prev, @@ -282,14 +288,19 @@ export function LozengeFlyProvider({ }); }); - // After the transition completes, remove clone and reveal slot lozenge + // After the transition completes, remove clone and reveal slot lozenge. + // Only clear pendingFlyIns if this is still the active animation for + // this story — a newer animation may have superseded this one. setTimeout(() => { setFlyingLozenges((prev) => prev.filter((l) => l.id !== id)); - setPendingFlyIns((prev) => { - const next = new Set(prev); - next.delete(action.storyId); - return next; - }); + if (activeFlyInPerStory.current.get(action.storyId) === id) { + activeFlyInPerStory.current.delete(action.storyId); + setPendingFlyIns((prev) => { + const next = new Set(prev); + next.delete(action.storyId); + return next; + }); + } }, 500); } @@ -307,6 +318,7 @@ export function LozengeFlyProvider({ const rosterRect = rosterEl?.getBoundingClientRect(); const id = `fly-out-${action.agentName}-${action.storyId}-${Date.now()}`; + activeFlyOutPerAgent.current.set(action.agentName, id); setFlyingLozenges((prev) => [ ...prev, @@ -330,14 +342,18 @@ export function LozengeFlyProvider({ }); }); + // Only reveal the roster badge if this is still the active fly-out + // for this agent — a newer fly-out may have superseded this one. setTimeout(() => { setFlyingLozenges((prev) => prev.filter((l) => l.id !== id)); - // Reveal the roster badge now that the clone has landed. - setFlyingOutAgents((prev) => { - const next = new Set(prev); - next.delete(action.agentName); - return next; - }); + if (activeFlyOutPerAgent.current.get(action.agentName) === id) { + activeFlyOutPerAgent.current.delete(action.agentName); + setFlyingOutAgents((prev) => { + const next = new Set(prev); + next.delete(action.agentName); + return next; + }); + } }, 500); } }, [pipeline]);