mirror of
https://github.com/Significant-Gravitas/AutoGPT.git
synced 2026-04-08 03:00:28 -04:00
fix(frontend/builder): address coderabbitai and sentry review feedback
- Validate required fields in parseGraphActions before emitting actions (coderabbitai: reject malformed payloads instead of coercing to "") - Gate chat seeding on isGraphLoaded to avoid seeding with empty graph when panel is opened before graph finishes loading (coderabbitai) - Deduplicate parsedActions in the hook to prevent duplicate React keys when AI suggests the same action twice (sentry) - Add tests for malformed action field validation
This commit is contained in:
@@ -16,9 +16,10 @@ import { useBuilderChatPanel } from "./useBuilderChatPanel";
|
||||
|
||||
interface Props {
|
||||
className?: string;
|
||||
isGraphLoaded?: boolean;
|
||||
}
|
||||
|
||||
export function BuilderChatPanel({ className }: Props) {
|
||||
export function BuilderChatPanel({ className, isGraphLoaded }: Props) {
|
||||
const {
|
||||
isOpen,
|
||||
handleToggle,
|
||||
@@ -31,7 +32,7 @@ export function BuilderChatPanel({ className }: Props) {
|
||||
nodes,
|
||||
parsedActions,
|
||||
handleApplyAction,
|
||||
} = useBuilderChatPanel();
|
||||
} = useBuilderChatPanel({ isGraphLoaded });
|
||||
|
||||
const [inputValue, setInputValue] = useState("");
|
||||
const messagesEndRef = useRef<HTMLDivElement>(null);
|
||||
|
||||
@@ -302,4 +302,16 @@ Here is a suggestion:
|
||||
const text = '```json\n{"key": "value"}\n```';
|
||||
expect(parseGraphActions(text)).toEqual([]);
|
||||
});
|
||||
|
||||
it("ignores update_node_input actions with missing required fields", () => {
|
||||
const text =
|
||||
'```json\n{"action": "update_node_input", "node_id": "1"}\n```';
|
||||
expect(parseGraphActions(text)).toEqual([]);
|
||||
});
|
||||
|
||||
it("ignores connect_nodes actions with empty handles", () => {
|
||||
const text =
|
||||
'```json\n{"action": "connect_nodes", "source": "1", "target": "2", "source_handle": "", "target_handle": "input"}\n```';
|
||||
expect(parseGraphActions(text)).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -62,19 +62,44 @@ export function parseGraphActions(text: string): GraphAction[] {
|
||||
}
|
||||
const obj = parsed as Record<string, unknown>;
|
||||
if (obj.action === "update_node_input") {
|
||||
const nodeId = obj.node_id;
|
||||
const key = obj.key;
|
||||
if (
|
||||
typeof nodeId !== "string" ||
|
||||
!nodeId ||
|
||||
typeof key !== "string" ||
|
||||
!key ||
|
||||
obj.value === undefined
|
||||
)
|
||||
continue;
|
||||
actions.push({
|
||||
type: "update_node_input",
|
||||
nodeId: String(obj.node_id ?? ""),
|
||||
key: String(obj.key ?? ""),
|
||||
nodeId,
|
||||
key,
|
||||
value: obj.value,
|
||||
});
|
||||
} else if (obj.action === "connect_nodes") {
|
||||
const source = obj.source;
|
||||
const target = obj.target;
|
||||
const sourceHandle = obj.source_handle;
|
||||
const targetHandle = obj.target_handle;
|
||||
if (
|
||||
typeof source !== "string" ||
|
||||
!source ||
|
||||
typeof target !== "string" ||
|
||||
!target ||
|
||||
typeof sourceHandle !== "string" ||
|
||||
!sourceHandle ||
|
||||
typeof targetHandle !== "string" ||
|
||||
!targetHandle
|
||||
)
|
||||
continue;
|
||||
actions.push({
|
||||
type: "connect_nodes",
|
||||
source: String(obj.source ?? ""),
|
||||
target: String(obj.target ?? ""),
|
||||
sourceHandle: String(obj.source_handle ?? ""),
|
||||
targetHandle: String(obj.target_handle ?? ""),
|
||||
source,
|
||||
target,
|
||||
sourceHandle,
|
||||
targetHandle,
|
||||
});
|
||||
}
|
||||
} catch {
|
||||
|
||||
@@ -15,7 +15,13 @@ import {
|
||||
|
||||
type SendMessageFn = ReturnType<typeof useChat>["sendMessage"];
|
||||
|
||||
export function useBuilderChatPanel() {
|
||||
interface UseBuilderChatPanelArgs {
|
||||
isGraphLoaded?: boolean;
|
||||
}
|
||||
|
||||
export function useBuilderChatPanel({
|
||||
isGraphLoaded = true,
|
||||
}: UseBuilderChatPanelArgs = {}) {
|
||||
const [isOpen, setIsOpen] = useState(false);
|
||||
const [sessionId, setSessionId] = useState<string | null>(null);
|
||||
const [isCreatingSession, setIsCreatingSession] = useState(false);
|
||||
@@ -92,13 +98,14 @@ export function useBuilderChatPanel() {
|
||||
sendMessageRef.current = sendMessage;
|
||||
|
||||
useEffect(() => {
|
||||
if (!sessionId || !transport || initializedRef.current) return;
|
||||
if (!sessionId || !transport || !isGraphLoaded || initializedRef.current)
|
||||
return;
|
||||
initializedRef.current = true;
|
||||
const summary = serializeGraphForChat(nodes, edges);
|
||||
sendMessageRef.current?.({
|
||||
text: `I'm building an agent in the AutoGPT flow builder. Here's the current graph:\n\n${summary}\n\nWhat does this agent do?`,
|
||||
});
|
||||
}, [sessionId, transport]);
|
||||
}, [sessionId, transport, isGraphLoaded]);
|
||||
|
||||
function handleToggle() {
|
||||
setIsOpen((o) => !o);
|
||||
@@ -136,7 +143,17 @@ export function useBuilderChatPanel() {
|
||||
)
|
||||
.map((p) => p.text)
|
||||
.join("");
|
||||
return parseGraphActions(text);
|
||||
const parsed = parseGraphActions(text);
|
||||
const seen = new Set<string>();
|
||||
return parsed.filter((action) => {
|
||||
const key =
|
||||
action.type === "update_node_input"
|
||||
? `${action.nodeId}:${action.key}`
|
||||
: `${action.source}->${action.target}`;
|
||||
if (seen.has(key)) return false;
|
||||
seen.add(key);
|
||||
return true;
|
||||
});
|
||||
}, [messages]);
|
||||
|
||||
return {
|
||||
|
||||
@@ -135,7 +135,7 @@ export const Flow = () => {
|
||||
executionId={flowExecutionID || undefined}
|
||||
graphId={flowID || undefined}
|
||||
/>
|
||||
<BuilderChatPanel />
|
||||
<BuilderChatPanel isGraphLoaded={isInitialLoadComplete} />
|
||||
</div>
|
||||
);
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user