Skip to content

Commit bb50af4

Browse files
author
Nadir Dzhilkibaev
committed
fix(frontend): validate edge source/target before adding connection
1 parent ce17f02 commit bb50af4

2 files changed

Lines changed: 271 additions & 3 deletions

File tree

src/frontend/src/utils/__tests__/cleanEdges.test.ts

Lines changed: 248 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,24 @@ interface EdgeType {
9797
const filterHiddenFieldsEdges = (
9898
_edge: EdgeType,
9999
edges: EdgeType[],
100-
_targetNode: AllNodeType,
101-
) => edges;
100+
targetNode: AllNodeType,
101+
) => {
102+
// MCP components dynamically add tool parameters after connecting to an external
103+
// MCP server. Before the schema loads, dynamic fields may have show: false.
104+
// Removing edges in that transient state causes false positives.
105+
const isMcpComponent = targetNode.data.type === "MCPTools";
106+
const targetHandle = _edge.data?.targetHandle as TargetHandleType | undefined;
107+
if (!targetHandle) return edges;
108+
const fieldName = targetHandle.fieldName;
109+
const nodeTemplates = targetNode.data.node.template as Record<
110+
string,
111+
{ show?: boolean }
112+
>;
113+
if (nodeTemplates[fieldName!]?.show === false && !isMcpComponent) {
114+
return edges.filter((e) => e.id !== _edge.id);
115+
}
116+
return edges;
117+
};
102118

103119
// Reimplement cleanEdges function for testing (matches the fix in reactflowUtils.ts)
104120
function cleanEdges(nodes: AllNodeType[], edges: EdgeType[]) {
@@ -189,6 +205,16 @@ function cleanEdges(nodes: AllNodeType[], edges: EdgeType[]) {
189205
: rawInputTypes;
190206
const hasProxy = template[field!]?.proxy;
191207
const isToolMode = template[field!]?.tool_mode;
208+
// MCP components dynamically add tool parameters after connecting to an external
209+
// MCP server. When the flow loads, cleanEdges runs before the server has
210+
// responded with the tool's schema, so the template field may not exist yet
211+
// or may lack input_types. Removing the edge in that state is a false positive.
212+
const isMcpComponent = targetNode.data.type === "MCPTools";
213+
const fieldNotInTemplate = template[field!] === undefined;
214+
const fieldHasNoInputTypes =
215+
isMcpComponent && !fieldNotInTemplate && !rawInputTypes?.length;
216+
const isMcpWithPendingParams =
217+
isMcpComponent && (fieldNotInTemplate || fieldHasNoInputTypes);
192218

193219
let id: TargetHandleType | SourceHandleType;
194220

@@ -230,7 +256,9 @@ function cleanEdges(nodes: AllNodeType[], edges: EdgeType[]) {
230256
);
231257
const isLoopInput = targetOutput?.allows_loop === true;
232258

259+
// Skip edge removal for MCP components whose parameters haven't loaded yet
233260
if (
261+
!isMcpWithPendingParams &&
234262
(scapedJSONStringfy(id) !== targetHandle ||
235263
(targetNode.data.node?.tool_mode && isToolMode)) &&
236264
!isLoopInput
@@ -901,4 +929,222 @@ describe("cleanEdges", () => {
901929
expect(resultBasic2.edges.length).toBe(0);
902930
});
903931
});
932+
933+
describe("MCP Tools edge validation", () => {
934+
it("should preserve edge to MCP component when tool parameter field not yet in template", () => {
935+
// Scenario: Parser connected to MCP Tools, but MCP server hasn't loaded tool schema yet
936+
const parserNode: AllNodeType = {
937+
id: "Parser-1",
938+
type: "genericNode",
939+
data: {
940+
id: "Parser-1",
941+
type: "Parser",
942+
selected_output: "output",
943+
node: {
944+
display_name: "Parser",
945+
template: {},
946+
outputs: [
947+
{
948+
name: "output",
949+
display_name: "Output",
950+
types: ["Message"],
951+
selected: "Message",
952+
},
953+
],
954+
},
955+
},
956+
};
957+
958+
const mcpNode: AllNodeType = {
959+
id: "MCPTools-1",
960+
type: "genericNode",
961+
data: {
962+
id: "MCPTools-1",
963+
type: "MCPTools",
964+
node: {
965+
display_name: "MCP Tools",
966+
template: {
967+
// MCP server config fields exist
968+
mcp_server: { type: "str", input_types: ["str"] },
969+
// But tool-specific parameter "query" is NOT in template yet
970+
// (schema hasn't loaded from MCP server)
971+
},
972+
outputs: [],
973+
},
974+
},
975+
};
976+
977+
// Edge from Parser output to MCP "query" parameter
978+
const sourceHandle = scapedJSONStringfy({
979+
id: "Parser-1",
980+
name: "output",
981+
output_types: ["Message"],
982+
dataType: "Parser",
983+
});
984+
985+
const targetHandle = scapedJSONStringfy({
986+
type: "str",
987+
fieldName: "query",
988+
id: "MCPTools-1",
989+
inputTypes: ["str"],
990+
});
991+
992+
const edge: EdgeType = {
993+
id: "edge-1",
994+
source: "Parser-1",
995+
target: "MCPTools-1",
996+
sourceHandle,
997+
targetHandle,
998+
};
999+
1000+
const result = cleanEdges([parserNode, mcpNode], [edge]);
1001+
1002+
// Edge should be preserved because MCP component parameters are pending
1003+
expect(result.edges.length).toBe(1);
1004+
expect(result.brokenEdges.length).toBe(0);
1005+
});
1006+
1007+
it("should preserve edge to MCP component when field exists but input_types not loaded", () => {
1008+
// Scenario: Field exists in template but input_types array is empty (loading in progress)
1009+
const parserNode: AllNodeType = {
1010+
id: "Parser-1",
1011+
type: "genericNode",
1012+
data: {
1013+
id: "Parser-1",
1014+
type: "Parser",
1015+
selected_output: "output",
1016+
node: {
1017+
display_name: "Parser",
1018+
template: {},
1019+
outputs: [
1020+
{
1021+
name: "output",
1022+
display_name: "Output",
1023+
types: ["Message"],
1024+
selected: "Message",
1025+
},
1026+
],
1027+
},
1028+
},
1029+
};
1030+
1031+
const mcpNode: AllNodeType = {
1032+
id: "MCPTools-1",
1033+
type: "genericNode",
1034+
data: {
1035+
id: "MCPTools-1",
1036+
type: "MCPTools",
1037+
node: {
1038+
display_name: "MCP Tools",
1039+
template: {
1040+
mcp_server: { type: "str", input_types: ["str"] },
1041+
query: {
1042+
type: "str",
1043+
input_types: [], // Empty - still loading
1044+
show: true,
1045+
},
1046+
},
1047+
outputs: [],
1048+
},
1049+
},
1050+
};
1051+
1052+
const sourceHandle = scapedJSONStringfy({
1053+
id: "Parser-1",
1054+
name: "output",
1055+
output_types: ["Message"],
1056+
dataType: "Parser",
1057+
});
1058+
1059+
const targetHandle = scapedJSONStringfy({
1060+
type: "str",
1061+
fieldName: "query",
1062+
id: "MCPTools-1",
1063+
inputTypes: ["str"],
1064+
});
1065+
1066+
const edge: EdgeType = {
1067+
id: "edge-1",
1068+
source: "Parser-1",
1069+
target: "MCPTools-1",
1070+
sourceHandle,
1071+
targetHandle,
1072+
};
1073+
1074+
const result = cleanEdges([parserNode, mcpNode], [edge]);
1075+
1076+
// Edge should be preserved while parameters are loading
1077+
expect(result.edges.length).toBe(1);
1078+
expect(result.brokenEdges.length).toBe(0);
1079+
});
1080+
1081+
it("should still remove broken edges for non-MCP components", () => {
1082+
// Ensure normal validation still works for other component types
1083+
const sourceNode: AllNodeType = {
1084+
id: "Source-1",
1085+
type: "genericNode",
1086+
data: {
1087+
id: "Source-1",
1088+
type: "TextInput",
1089+
selected_output: "output",
1090+
node: {
1091+
display_name: "Text Input",
1092+
template: {},
1093+
outputs: [
1094+
{
1095+
name: "output",
1096+
display_name: "Output",
1097+
types: ["str"],
1098+
selected: "str",
1099+
},
1100+
],
1101+
},
1102+
},
1103+
};
1104+
1105+
const targetNode: AllNodeType = {
1106+
id: "Target-1",
1107+
type: "genericNode",
1108+
data: {
1109+
id: "Target-1",
1110+
type: "SomeComponent", // NOT MCPTools
1111+
node: {
1112+
display_name: "Some Component",
1113+
template: {
1114+
// Field doesn't exist
1115+
},
1116+
outputs: [],
1117+
},
1118+
},
1119+
};
1120+
1121+
const sourceHandle = scapedJSONStringfy({
1122+
id: "Source-1",
1123+
name: "output",
1124+
output_types: ["str"],
1125+
dataType: "TextInput",
1126+
});
1127+
1128+
const targetHandle = scapedJSONStringfy({
1129+
type: "str",
1130+
fieldName: "missing_field",
1131+
id: "Target-1",
1132+
inputTypes: ["str"],
1133+
});
1134+
1135+
const edge: EdgeType = {
1136+
id: "edge-1",
1137+
source: "Source-1",
1138+
target: "Target-1",
1139+
sourceHandle,
1140+
targetHandle,
1141+
};
1142+
1143+
const result = cleanEdges([sourceNode, targetNode], [edge]);
1144+
1145+
// Edge should be removed because this is NOT an MCP component
1146+
expect(result.edges.length).toBe(0);
1147+
expect(result.brokenEdges.length).toBe(1);
1148+
});
1149+
});
9041150
});

src/frontend/src/utils/reactflowUtils.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,18 @@ export function cleanEdges(nodes: AllNodeType[], edges: EdgeType[]) {
150150
const hasProxy = targetNode.data.node!.template[field]?.proxy;
151151
const isToolMode = targetNode.data.node!.template[field]?.tool_mode;
152152
const isAdvanced = targetNode.data.node!.template[field]?.advanced;
153+
// MCP components dynamically add tool parameters after connecting to an external
154+
// MCP server. When the flow loads, cleanEdges runs before the server has
155+
// responded with the tool's schema, so the template field may not exist yet
156+
// or may lack input_types. Removing the edge in that state is a false
157+
// positive — the edge is valid once the parameters arrive.
158+
const isMcpComponent = targetNode.data.type === "MCPTools";
159+
const fieldNotInTemplate =
160+
targetNode.data.node!.template[field] === undefined;
161+
const fieldHasNoInputTypes =
162+
isMcpComponent && !fieldNotInTemplate && !rawInputTypes?.length;
163+
const isMcpWithPendingParams =
164+
isMcpComponent && (fieldNotInTemplate || fieldHasNoInputTypes);
153165

154166
if (
155167
!field &&
@@ -196,7 +208,11 @@ export function cleanEdges(nodes: AllNodeType[], edges: EdgeType[]) {
196208
expectedTargetHandle,
197209
targetHandle,
198210
);
211+
// Skip edge removal for MCP components whose parameters haven't loaded yet
212+
// from the external MCP server. The field/template mismatch is transient —
213+
// once the schema arrives, the edge will validate correctly on the next pass.
199214
if (
215+
!isMcpWithPendingParams &&
200216
(!targetHandlesMatchResult ||
201217
(targetNode.data.node?.tool_mode && isToolMode) ||
202218
isAdvanced) &&
@@ -371,8 +387,14 @@ export function filterHiddenFieldsEdges(
371387
const fieldName = targetHandle.fieldName;
372388
const nodeTemplates = targetNode.data.node!.template;
373389

390+
// MCP components dynamically add tool parameters after connecting to an external
391+
// MCP server. Before the schema loads, dynamic fields may have show: false or
392+
// not exist at all. Removing edges in that transient state causes "connections
393+
// were removed because they were invalid" false positives.
394+
const isMcpComponent = targetNode.data.type === "MCPTools";
395+
374396
// Only check the specific field the edge is connected to
375-
if (nodeTemplates[fieldName]?.show === false) {
397+
if (nodeTemplates[fieldName]?.show === false && !isMcpComponent) {
376398
newEdges = newEdges.filter((e) => e.id !== edge.id);
377399
}
378400
}

0 commit comments

Comments
 (0)