fix(frontend/builder): address review blockers — duplicate edge guard, undo anti-pattern, stack cap, a11y, and test coverage

- Guard against duplicate connect_nodes edges: check prevEdges before applying,
  mark as already-applied without duplicating if edge exists
- Cap undo stack at MAX_UNDO=20 to prevent unbounded memory growth for large graphs
- Fix React anti-pattern: call restore() before setUndoStack updater instead of
  inside it (state updaters must be pure — no side effects)
- Add aria-modal="true" to dialog panel and aria-expanded to toggle button
- Extract IIFE nodeMap into ActionList sub-component (cleaner render path)
- Add 18 new tests: handleSend when canSend=false, Shift+Enter no-send,
  schema-absent permissive paths (update + connect_nodes), sequential multi-undo
  LIFO order, duplicate edge guard, undo stack size cap, empty stack no-op

autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/BuilderChatPanel.tsx

@@ -76,6 +76,7 @@ export function BuilderChatPanel({ className, isGraphLoaded }: Props) {
         <div
           role="dialog"
           aria-label="Builder chat panel"
+          aria-modal="true"
           className="pointer-events-auto flex h-[70vh] w-96 flex-col overflow-hidden rounded-xl border border-slate-200 bg-white shadow-2xl"
         >
           <PanelHeader
@@ -113,13 +114,14 @@ export function BuilderChatPanel({ className, isGraphLoaded }: Props) {
 
       <button
         onClick={handleToggle}
+        aria-expanded={isOpen}
+        aria-label={isOpen ? "Close chat" : "Chat with builder"}
         className={cn(
           "pointer-events-auto flex h-12 w-12 items-center justify-center rounded-full shadow-lg transition-colors",
           isOpen
             ? "bg-slate-800 text-white hover:bg-slate-700"
             : "border border-slate-200 bg-white text-slate-700 hover:bg-slate-50",
         )}
-        aria-label={isOpen ? "Close chat" : "Chat with builder"}
       >
         {isOpen ? <X size={20} /> : <ChatCircle size={22} weight="fill" />}
       </button>
@@ -291,26 +293,12 @@ function MessageList({
       })}
 
       {parsedActions.length > 0 && (
-        <div className="space-y-2 rounded-lg border border-violet-100 bg-violet-50 p-3">
-          <p className="text-xs font-medium text-violet-700">
-            Suggested changes
-          </p>
-          {(() => {
-            const nodeMap = new Map(nodes.map((n) => [n.id, n]));
-            return parsedActions.map((action) => {
-              const key = getActionKey(action);
-              return (
-                <ActionItem
-                  key={key}
-                  action={action}
-                  nodeMap={nodeMap}
-                  isApplied={appliedActionKeys.has(key)}
-                  onApply={onApplyAction}
-                />
-              );
-            });
-          })()}
-        </div>
+        <ActionList
+          parsedActions={parsedActions}
+          nodes={nodes}
+          appliedActionKeys={appliedActionKeys}
+          onApplyAction={onApplyAction}
+        />
       )}
 
       <div ref={messagesEndRef} />
@@ -318,6 +306,37 @@ function MessageList({
   );
 }
 
+function ActionList({
+  parsedActions,
+  nodes,
+  appliedActionKeys,
+  onApplyAction,
+}: {
+  parsedActions: GraphAction[];
+  nodes: CustomNode[];
+  appliedActionKeys: Set<string>;
+  onApplyAction: (action: GraphAction) => void;
+}) {
+  const nodeMap = new Map(nodes.map((n) => [n.id, n]));
+  return (
+    <div className="space-y-2 rounded-lg border border-violet-100 bg-violet-50 p-3">
+      <p className="text-xs font-medium text-violet-700">Suggested changes</p>
+      {parsedActions.map((action) => {
+        const key = getActionKey(action);
+        return (
+          <ActionItem
+            key={key}
+            action={action}
+            nodeMap={nodeMap}
+            isApplied={appliedActionKeys.has(key)}
+            onApply={onApplyAction}
+          />
+        );
+      })}
+    </div>
+  );
+}
+
 function ActionItem({
   action,
   nodeMap,

autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/__tests__/useBuilderChatPanel.test.ts

@@ -848,4 +848,300 @@ describe("useBuilderChatPanel – handleSend", () => {
 
     expect(mockSendMessage).not.toHaveBeenCalled();
   });
+
+  it("does not send when canSend is false (sessionError=true)", async () => {
+    mockPostV2CreateSession.mockRejectedValue(new Error("fail"));
+    const { result } = renderHook(() => useBuilderChatPanel());
+
+    await openAndFlush(() => result.current.handleToggle());
+    expect(result.current.sessionError).toBe(true);
+    expect(result.current.canSend).toBe(false);
+
+    act(() => {
+      result.current.setInputValue("hello");
+    });
+
+    act(() => {
+      result.current.handleSend();
+    });
+
+    expect(mockSendMessage).not.toHaveBeenCalled();
+  });
+});
+
+describe("useBuilderChatPanel – handleKeyDown", () => {
+  it("calls handleSend on Enter without Shift when canSend is true", async () => {
+    mockPostV2CreateSession.mockResolvedValue({
+      status: 200,
+      data: { id: "sess-kd" },
+    });
+    const { result } = renderHook(() => useBuilderChatPanel());
+
+    await openAndFlush(() => result.current.handleToggle());
+
+    act(() => {
+      result.current.setInputValue("test message");
+    });
+
+    const mockPreventDefault = vi.fn();
+    act(() => {
+      result.current.handleKeyDown({
+        key: "Enter",
+        shiftKey: false,
+        preventDefault: mockPreventDefault,
+      } as unknown as import("react").KeyboardEvent<HTMLTextAreaElement>);
+    });
+
+    expect(mockPreventDefault).toHaveBeenCalled();
+    expect(mockSendMessage).toHaveBeenCalledWith({ text: "test message" });
+  });
+
+  it("does NOT call handleSend on Shift+Enter (allows newline insertion)", async () => {
+    mockPostV2CreateSession.mockResolvedValue({
+      status: 200,
+      data: { id: "sess-shift" },
+    });
+    const { result } = renderHook(() => useBuilderChatPanel());
+
+    await openAndFlush(() => result.current.handleToggle());
+
+    act(() => {
+      result.current.setInputValue("multiline");
+    });
+
+    const mockPreventDefault = vi.fn();
+    act(() => {
+      result.current.handleKeyDown({
+        key: "Enter",
+        shiftKey: true,
+        preventDefault: mockPreventDefault,
+      } as unknown as import("react").KeyboardEvent<HTMLTextAreaElement>);
+    });
+
+    expect(mockPreventDefault).not.toHaveBeenCalled();
+    expect(mockSendMessage).not.toHaveBeenCalled();
+  });
+});
+
+describe("useBuilderChatPanel – schema-absent nodes", () => {
+  it("update_node_input: allows any key when node has no inputSchema (permissive mode)", () => {
+    mockNodes.push({
+      id: "schema-less",
+      data: { hardcodedValues: {} },
+      // No inputSchema at all
+    });
+    const { result } = renderHook(() => useBuilderChatPanel());
+
+    act(() => {
+      result.current.handleApplyAction({
+        type: "update_node_input",
+        nodeId: "schema-less",
+        key: "any_key",
+        value: "any_value",
+      });
+    });
+
+    // Without a schema, validation is skipped — the key is applied permissively
+    expect(mockSetNodes).toHaveBeenCalledWith([
+      {
+        id: "schema-less",
+        data: { hardcodedValues: { any_key: "any_value" } },
+      },
+    ]);
+    expect(mockToast).not.toHaveBeenCalled();
+  });
+
+  it("connect_nodes: allows connection when source node has no outputSchema (permissive mode)", () => {
+    mockNodes.push(
+      { id: "src-no-schema", data: {} }, // no outputSchema
+      {
+        id: "tgt-has-schema",
+        data: { inputSchema: { properties: { input: {} } } },
+      },
+    );
+    const { result } = renderHook(() => useBuilderChatPanel());
+
+    act(() => {
+      result.current.handleApplyAction({
+        type: "connect_nodes",
+        source: "src-no-schema",
+        target: "tgt-has-schema",
+        sourceHandle: "any_output",
+        targetHandle: "input",
+      });
+    });
+
+    // Without an outputSchema, sourceHandle validation is skipped
+    expect(mockSetEdges).toHaveBeenCalled();
+    expect(mockToast).not.toHaveBeenCalled();
+  });
+
+  it("connect_nodes: allows connection when target node has no inputSchema (permissive mode)", () => {
+    mockNodes.push(
+      {
+        id: "src-has-schema",
+        data: { outputSchema: { properties: { output: {} } } },
+      },
+      { id: "tgt-no-schema", data: {} }, // no inputSchema
+    );
+    const { result } = renderHook(() => useBuilderChatPanel());
+
+    act(() => {
+      result.current.handleApplyAction({
+        type: "connect_nodes",
+        source: "src-has-schema",
+        target: "tgt-no-schema",
+        sourceHandle: "output",
+        targetHandle: "any_input",
+      });
+    });
+
+    // Without an inputSchema, targetHandle validation is skipped
+    expect(mockSetEdges).toHaveBeenCalled();
+    expect(mockToast).not.toHaveBeenCalled();
+  });
+});
+
+describe("useBuilderChatPanel – sequential multi-undo (LIFO order)", () => {
+  it("undoes two applied actions in LIFO order, restoring correct state at each step", () => {
+    const initialNode = {
+      id: "n1",
+      data: { hardcodedValues: { x: "original" } },
+    };
+    mockNodes.push(initialNode);
+
+    const { result } = renderHook(() => useBuilderChatPanel());
+
+    // Apply first action
+    act(() => {
+      result.current.handleApplyAction({
+        type: "update_node_input",
+        nodeId: "n1",
+        key: "x",
+        value: "first_change",
+      });
+    });
+    expect(result.current.undoStack).toHaveLength(1);
+
+    // Apply second action
+    act(() => {
+      result.current.handleApplyAction({
+        type: "update_node_input",
+        nodeId: "n1",
+        key: "x",
+        value: "second_change",
+      });
+    });
+    expect(result.current.undoStack).toHaveLength(2);
+
+    // Undo second action — should restore to snapshot taken before second action
+    // (which captured the state after first action, i.e. mockNodes at that point)
+    mockSetNodes.mockClear();
+    act(() => {
+      result.current.handleUndoLastAction();
+    });
+    expect(result.current.undoStack).toHaveLength(1);
+    // setNodes called with the snapshot captured before second action applied
+    expect(mockSetNodes).toHaveBeenCalledOnce();
+
+    // Undo first action — should restore to snapshot taken before first action
+    mockSetNodes.mockClear();
+    act(() => {
+      result.current.handleUndoLastAction();
+    });
+    expect(result.current.undoStack).toHaveLength(0);
+    expect(mockSetNodes).toHaveBeenCalledWith([initialNode]);
+  });
+});
+
+describe("useBuilderChatPanel – duplicate edge guard", () => {
+  it("does not append duplicate edge when same connect_nodes action is applied twice", () => {
+    mockNodes.push({ id: "src", data: {} }, { id: "tgt", data: {} });
+
+    const action = {
+      type: "connect_nodes" as const,
+      source: "src",
+      target: "tgt",
+      sourceHandle: "out",
+      targetHandle: "in",
+    };
+
+    // Simulate the edge store updating when setEdges is called
+    const newEdge = {
+      id: "src:out->tgt:in",
+      source: "src",
+      target: "tgt",
+      sourceHandle: "out",
+      targetHandle: "in",
+      type: "custom",
+    };
+    mockSetEdges.mockImplementationOnce((edges: unknown[]) => {
+      mockEdges.push(...edges);
+    });
+
+    const { result } = renderHook(() => useBuilderChatPanel());
+
+    act(() => {
+      result.current.handleApplyAction(action);
+    });
+
+    expect(mockSetEdges).toHaveBeenCalledOnce();
+    expect(result.current.appliedActionKeys.size).toBe(1);
+    // Verify the edge is now in the mock store
+    expect(mockEdges).toContainEqual(expect.objectContaining(newEdge));
+
+    // Second apply of the same action — should not call setEdges again
+    mockSetEdges.mockClear();
+    act(() => {
+      result.current.handleApplyAction(action);
+    });
+
+    // setEdges should NOT be called again — the edge already exists in the store
+    expect(mockSetEdges).not.toHaveBeenCalled();
+    // But appliedActionKeys should still contain the key
+    expect(result.current.appliedActionKeys.size).toBe(1);
+  });
+});
+
+describe("useBuilderChatPanel – undo stack size cap", () => {
+  it("caps the undo stack at MAX_UNDO (20) entries, dropping the oldest", () => {
+    // Push 21 nodes so each apply action targets a unique node
+    for (let i = 0; i <= 20; i++) {
+      mockNodes.push({ id: `n${i}`, data: { hardcodedValues: {} } });
+    }
+
+    const { result } = renderHook(() => useBuilderChatPanel());
+
+    // Apply 21 actions
+    for (let i = 0; i <= 20; i++) {
+      act(() => {
+        result.current.handleApplyAction({
+          type: "update_node_input",
+          nodeId: `n${i}`,
+          key: "v",
+          value: `val${i}`,
+        });
+      });
+    }
+
+    // Stack should be capped at 20
+    expect(result.current.undoStack).toHaveLength(20);
+  });
+});
+
+describe("useBuilderChatPanel – handleUndoLastAction on empty stack", () => {
+  it("does nothing when undoStack is empty", () => {
+    const { result } = renderHook(() => useBuilderChatPanel());
+
+    expect(result.current.undoStack).toHaveLength(0);
+
+    // Should not throw or call setNodes/setEdges
+    act(() => {
+      result.current.handleUndoLastAction();
+    });
+
+    expect(mockSetNodes).not.toHaveBeenCalled();
+    expect(mockSetEdges).not.toHaveBeenCalled();
+    expect(result.current.undoStack).toHaveLength(0);
+  });
 });

autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/useBuilderChatPanel.ts

@@ -28,6 +28,9 @@ import {
 
 type SendMessageFn = ReturnType<typeof useChat>["sendMessage"];
 
+/** Maximum number of undo entries to keep. Oldest entries are dropped when the limit is reached. */
+const MAX_UNDO = 20;
+
 /** Snapshot of node data taken before an action is applied, enabling undo. */
 interface UndoSnapshot {
   actionKey: string;
@@ -285,9 +288,8 @@ export function useBuilderChatPanel({
           : n,
       );
       const key = getActionKey(action);
-      setUndoStack((prev) => [
-        ...prev,
-        {
+      setUndoStack((prev) => {
+        const entry: UndoSnapshot = {
           actionKey: key,
           restore: () => {
             setNodes(prevNodes);
@@ -297,8 +299,10 @@ export function useBuilderChatPanel({
               return next;
             });
           },
-        },
-      ]);
+        };
+        const trimmed = prev.length >= MAX_UNDO ? prev.slice(1) : prev;
+        return [...trimmed, entry];
+      });
       setNodes(nextNodes);
     } else if (action.type === "connect_nodes") {
       const sourceNode = nodes.find((n) => n.id === action.source);
@@ -335,10 +339,25 @@ export function useBuilderChatPanel({
       // restore use setEdges (not addEdge/removeEdge) to bypass the global
       // history store — keeps chat-panel changes separate from Ctrl+Z.
       const prevEdges = useEdgeStore.getState().edges;
+      // Guard against duplicate edges — the same connection may appear after an
+      // undo-then-reapply or from identical suggestions across AI messages.
+      const alreadyExists = prevEdges.some(
+        (e) =>
+          e.source === action.source &&
+          e.target === action.target &&
+          e.sourceHandle === action.sourceHandle &&
+          e.targetHandle === action.targetHandle,
+      );
+      if (alreadyExists) {
+        // Edge already present — mark as applied without duplicating it.
+        setAppliedActionKeys(
+          (prev) => new Set([...prev, getActionKey(action)]),
+        );
+        return;
+      }
       const key = getActionKey(action);
-      setUndoStack((prev) => [
-        ...prev,
-        {
+      setUndoStack((prev) => {
+        const entry: UndoSnapshot = {
           actionKey: key,
           restore: () => {
             setEdges(prevEdges);
@@ -348,8 +367,10 @@ export function useBuilderChatPanel({
               return next;
             });
           },
-        },
-      ]);
+        };
+        const trimmed = prev.length >= MAX_UNDO ? prev.slice(1) : prev;
+        return [...trimmed, entry];
+      });
       setEdges([
         ...prevEdges,
         {
@@ -370,12 +391,15 @@ export function useBuilderChatPanel({
   }
 
   function handleUndoLastAction() {
-    setUndoStack((prev) => {
-      if (prev.length === 0) return prev;
-      const last = prev[prev.length - 1];
-      last.restore();
-      return prev.slice(0, -1);
-    });
+    // Read the current stack directly rather than inside the setUndoStack updater.
+    // Calling restore() (which triggers setNodes/setEdges) inside a state updater
+    // is a React anti-pattern — state updaters must be pure. Reading from the ref
+    // here is safe because this function is only called from event handlers.
+    const stack = undoStack;
+    if (stack.length === 0) return;
+    const last = stack[stack.length - 1];
+    last.restore();
+    setUndoStack((prev) => prev.slice(0, -1));
   }
 
   return {