From 1f6981bd0627159ca8082dc8a69c1ce1ae0bb45a Mon Sep 17 00:00:00 2001 From: majdyz Date: Thu, 9 Apr 2026 01:36:17 +0700 Subject: [PATCH] fix(frontend/builder): fix chat panel undo bypassing global history store Use setNodes/setEdges directly in undo restore closures instead of updateNodeData/removeEdge which push to the history store. This prevents the global Ctrl+Z from re-applying changes that the user already undid via the chat panel's own undo button. Also removes unused removeEdge selector from the hook. --- .../__tests__/useBuilderChatPanel.test.ts | 64 +++++++++++++------ .../BuilderChatPanel/useBuilderChatPanel.ts | 22 ++++--- 2 files changed, 59 insertions(+), 27 deletions(-) diff --git a/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/__tests__/useBuilderChatPanel.test.ts b/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/__tests__/useBuilderChatPanel.test.ts index 56a9833ce1..93babeb6d9 100644 --- a/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/__tests__/useBuilderChatPanel.test.ts +++ b/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/__tests__/useBuilderChatPanel.test.ts @@ -11,25 +11,42 @@ vi.mock("zustand/react/shallow", () => ({ const mockNodes: unknown[] = []; const mockEdges: unknown[] = []; const mockUpdateNodeData = vi.fn(); +const mockSetNodes = vi.fn(); const mockAddEdge = vi.fn(); +const mockSetEdges = vi.fn(); const mockRemoveEdge = vi.fn(); -vi.mock("../../../stores/nodeStore", () => ({ - useNodeStore: (selector: (s: unknown) => unknown) => +vi.mock("../../../stores/nodeStore", () => { + const useNodeStore = (selector: (s: unknown) => unknown) => selector({ nodes: mockNodes, updateNodeData: mockUpdateNodeData, - }), -})); + setNodes: mockSetNodes, + }); + useNodeStore.getState = () => ({ + nodes: mockNodes, + updateNodeData: mockUpdateNodeData, + setNodes: mockSetNodes, + }); + return { useNodeStore }; +}); -vi.mock("../../../stores/edgeStore", () => ({ - useEdgeStore: (selector: (s: unknown) => unknown) => +vi.mock("../../../stores/edgeStore", () => { + const useEdgeStore = (selector: (s: unknown) => unknown) => selector({ edges: mockEdges, addEdge: mockAddEdge, + setEdges: mockSetEdges, removeEdge: mockRemoveEdge, - }), -})); + }); + useEdgeStore.getState = () => ({ + edges: mockEdges, + addEdge: mockAddEdge, + setEdges: mockSetEdges, + removeEdge: mockRemoveEdge, + }); + return { useEdgeStore }; +}); const mockPostV2CreateSession = vi.fn(); vi.mock("@/app/api/__generated__/endpoints/chat/chat", () => ({ @@ -96,7 +113,9 @@ beforeEach(() => { mockChatMessages = []; mockChatStatus = "ready"; mockUpdateNodeData.mockClear(); + mockSetNodes.mockClear(); mockAddEdge.mockClear(); + mockSetEdges.mockClear(); mockRemoveEdge.mockClear(); mockPostV2CreateSession.mockClear(); mockInvalidateQueries.mockClear(); @@ -579,12 +598,12 @@ describe("useBuilderChatPanel – handleApplyAction", () => { }); describe("useBuilderChatPanel – undo", () => { - it("restores previous hardcodedValues after undo", () => { - const prevValues = { existing: "original" }; - mockNodes.push({ + it("restores previous node state after undo using setNodes (bypasses history store)", () => { + const initialNode = { id: "node-undo", - data: { hardcodedValues: prevValues }, - }); + data: { hardcodedValues: { existing: "original" } }, + }; + mockNodes.push(initialNode); const { result } = renderHook(() => useBuilderChatPanel()); @@ -599,13 +618,18 @@ describe("useBuilderChatPanel – undo", () => { expect(result.current.undoStack).toHaveLength(1); + // Clear call history so we can verify undo only uses setNodes (not updateNodeData) + mockUpdateNodeData.mockClear(); + mockSetNodes.mockClear(); + act(() => { result.current.handleUndoLastAction(); }); - expect(mockUpdateNodeData).toHaveBeenLastCalledWith("node-undo", { - hardcodedValues: prevValues, - }); + // setNodes is called with the captured snapshot to bypass the global history store + expect(mockSetNodes).toHaveBeenCalledWith([initialNode]); + // updateNodeData must NOT be called during undo to avoid pushing to history store + expect(mockUpdateNodeData).not.toHaveBeenCalled(); expect(result.current.undoStack).toHaveLength(0); }); @@ -632,7 +656,9 @@ describe("useBuilderChatPanel – undo", () => { expect(result.current.appliedActionKeys.size).toBe(0); }); - it("connect_nodes: removes edge via removeEdge after undo", () => { + it("connect_nodes: restores edges via setEdges after undo (bypasses history store)", () => { + const initialEdge = { id: "existing-edge", source: "a", target: "b" }; + mockEdges.push(initialEdge); mockNodes.push({ id: "src", data: {} }, { id: "tgt", data: {} }); const { result } = renderHook(() => useBuilderChatPanel()); @@ -654,7 +680,9 @@ describe("useBuilderChatPanel – undo", () => { result.current.handleUndoLastAction(); }); - expect(mockRemoveEdge).toHaveBeenCalledWith("src:out->tgt:in"); + // setEdges is called with the captured snapshot to bypass the global history store + expect(mockSetEdges).toHaveBeenCalledWith([initialEdge]); + expect(mockRemoveEdge).not.toHaveBeenCalled(); expect(result.current.undoStack).toHaveLength(0); expect(result.current.appliedActionKeys.size).toBe(0); }); diff --git a/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/useBuilderChatPanel.ts b/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/useBuilderChatPanel.ts index 86e28a5b5a..64582e9213 100644 --- a/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/useBuilderChatPanel.ts +++ b/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/useBuilderChatPanel.ts @@ -65,8 +65,9 @@ export function useBuilderChatPanel({ const nodes = useNodeStore(useShallow((s) => s.nodes)); const edges = useEdgeStore(useShallow((s) => s.edges)); const updateNodeData = useNodeStore(useShallow((s) => s.updateNodeData)); + const setNodes = useNodeStore(useShallow((s) => s.setNodes)); const addEdge = useEdgeStore(useShallow((s) => s.addEdge)); - const removeEdge = useEdgeStore(useShallow((s) => s.removeEdge)); + const setEdges = useEdgeStore(useShallow((s) => s.setEdges)); // Reset session and seed-sent guard when the user navigates to a different // graph so the new graph's context is sent to the AI on next open. @@ -266,19 +267,18 @@ export function useBuilderChatPanel({ }); return; } - // Deep-clone before mutating so sequential applies to the same node - // each capture an independent snapshot — without this, the reference - // would point to the same object after mutation. - const prevHardcoded = structuredClone(node.data.hardcodedValues); + // Capture a full nodes snapshot before mutating. The restore function + // uses setNodes (not updateNodeData) to bypass the history store — + // otherwise the global Ctrl+Z undo would conflict with the chat panel's + // own undo stack by re-applying the just-undone change. + const prevNodes = useNodeStore.getState().nodes; const key = getActionKey(action); setUndoStack((prev) => [ ...prev, { actionKey: key, restore: () => { - updateNodeData(action.nodeId, { - hardcodedValues: prevHardcoded, - }); + setNodes(prevNodes); setAppliedActionKeys((keys) => { const next = new Set(keys); next.delete(key); @@ -324,13 +324,17 @@ export function useBuilderChatPanel({ return; } const edgeId = `${action.source}:${action.sourceHandle}->${action.target}:${action.targetHandle}`; + // Capture a full edges snapshot before mutating. The restore function + // uses setEdges (not removeEdge) to bypass the history store — + // otherwise the global Ctrl+Z undo would re-add the just-removed edge. + const prevEdges = useEdgeStore.getState().edges; const key = getActionKey(action); setUndoStack((prev) => [ ...prev, { actionKey: key, restore: () => { - removeEdge(edgeId); + setEdges(prevEdges); setAppliedActionKeys((keys) => { const next = new Set(keys); next.delete(key);