Compare commits

...

5 Commits

Author SHA1 Message Date
Otto
e19b6e458f fix: always clear edge store when loading new graph
Moved setEdges([]) outside the graph?.links guard to prevent stale edges
from persisting when loading a graph with no links.
2026-02-15 05:59:56 +00:00
Otto
2be589c95f fix: prevent race condition where links load before nodes
Combined the separate useEffect hooks for adding nodes and links into
a single effect that ensures nodes are added first. Previously, links
could be processed before nodes existed in the store, causing all
connections to be silently filtered out.

Fixes: Sentry bug prediction about race condition on graph load
2026-02-15 05:49:59 +00:00
Otto
79748ca7fa fix: avoid history pollution in addLinks by bulk-adding edges
Changed addLinks to collect all valid edges first and add them in a single
set() call with one history push, instead of calling addEdge for each link
which would push to history for every edge individually.

Fixes: Sentry bug prediction about undo/redo history pollution
2026-02-14 19:40:08 +00:00
Otto
f3df841ea3 fix: correct docstring - pin validation is handled elsewhere 2026-02-14 13:07:52 +00:00
Otto
6cb794cbf8 fix(builder): auto-cleanup invalid/orphan edges during graph operations
Fixes graph desync issues where edge deletions don't persist, causing
stale connections that users cannot remove.

Frontend changes:
- edgeStore: Validate links during addLinks() - skip edges referencing
  non-existent nodes
- edgeStore: Filter invalid edges in getBackendLinks() before save
- useSaveGraph: Sync edge store with authoritative backend state after
  save to prevent desync

Backend changes:
- graph.py: Add prune_invalid_links() method that removes:
  - Links referencing non-existent nodes
  - Links with invalid block IDs
- Called during graph validation to auto-cleanup orphan edges

This ensures:
1. Invalid edges are filtered out when loading a graph
2. Invalid edges are not sent to backend during save
3. Frontend syncs with backend state after save
4. Backend cleans up any orphan edges that slip through

Closes: SECRT-1959
2026-02-14 12:42:23 +00:00
4 changed files with 144 additions and 14 deletions

View File

@@ -867,9 +867,67 @@ class GraphModel(Graph, GraphMeta):
return node_errors
@staticmethod
def prune_invalid_links(graph: BaseGraph) -> int:
"""
Remove invalid/orphan links from the graph.
This removes links that:
- Reference non-existent source or sink nodes
- Reference invalid block IDs
Note: Pin name validation is handled separately in _validate_graph_structure.
Returns the number of links pruned.
"""
node_map = {v.id: v for v in graph.nodes}
original_count = len(graph.links)
valid_links = []
for link in graph.links:
source_node = node_map.get(link.source_id)
sink_node = node_map.get(link.sink_id)
# Skip if either node doesn't exist
if not source_node or not sink_node:
logger.warning(
f"Pruning orphan link: source={link.source_id}, sink={link.sink_id} "
f"- node(s) not found"
)
continue
# Skip if source block doesn't exist
source_block = get_block(source_node.block_id)
if not source_block:
logger.warning(
f"Pruning link with invalid source block: {source_node.block_id}"
)
continue
# Skip if sink block doesn't exist
sink_block = get_block(sink_node.block_id)
if not sink_block:
logger.warning(
f"Pruning link with invalid sink block: {sink_node.block_id}"
)
continue
valid_links.append(link)
graph.links = valid_links
pruned_count = original_count - len(valid_links)
if pruned_count > 0:
logger.info(f"Pruned {pruned_count} invalid link(s) from graph {graph.id}")
return pruned_count
@staticmethod
def _validate_graph_structure(graph: BaseGraph):
"""Validate graph structure (links, connections, etc.)"""
# First, prune invalid links to clean up orphan edges
GraphModel.prune_invalid_links(graph)
node_map = {v.id: v for v in graph.nodes}
def is_static_output_block(nid: str) -> bool:

View File

@@ -133,22 +133,23 @@ export const useFlow = () => {
}
}, [availableGraphs, setAvailableSubGraphs]);
// adding nodes
// adding nodes and links together to avoid race condition
// Links depend on nodes existing, so we must add nodes first
useEffect(() => {
if (customNodes.length > 0) {
// Clear both stores to prevent stale data from previous graphs
useNodeStore.getState().setNodes([]);
useNodeStore.getState().clearResolutionState();
addNodes(customNodes);
}
}, [customNodes, addNodes]);
// adding links
useEffect(() => {
if (graph?.links) {
useEdgeStore.getState().setEdges([]);
addLinks(graph.links);
addNodes(customNodes);
// Only add links after nodes are in the store
if (graph?.links) {
addLinks(graph.links);
}
}
}, [graph?.links, addLinks]);
}, [customNodes, graph?.links, addNodes, addLinks]);
useEffect(() => {
if (customNodes.length > 0 && graph?.links) {

View File

@@ -13,6 +13,7 @@ import { Graph } from "@/app/api/__generated__/models/graph";
import { useNodeStore } from "../stores/nodeStore";
import { useEdgeStore } from "../stores/edgeStore";
import { graphsEquivalent } from "../components/NewControlPanel/NewSaveControl/helpers";
import { linkToCustomEdge } from "../components/helper";
import { useGraphStore } from "../stores/graphStore";
import { useShallow } from "zustand/react/shallow";
import {
@@ -21,6 +22,18 @@ import {
getTempFlowId,
} from "@/services/builder-draft/draft-service";
/**
* Sync the edge store with the authoritative backend state.
* This ensures the frontend matches what the backend accepted after save.
*/
function syncEdgesWithBackend(links: GraphModel["links"]) {
if (links !== undefined) {
// Replace all edges with the authoritative backend state
const newEdges = links.map(linkToCustomEdge);
useEdgeStore.getState().setEdges(newEdges);
}
}
export type SaveGraphOptions = {
showToast?: boolean;
onSuccess?: (graph: GraphModel) => void;
@@ -64,6 +77,9 @@ export const useSaveGraph = ({
flowVersion: data.version,
});
// Sync edge store with authoritative backend state
syncEdgesWithBackend(data.links);
const tempFlowId = getTempFlowId();
if (tempFlowId) {
await draftService.deleteDraft(tempFlowId);
@@ -101,6 +117,9 @@ export const useSaveGraph = ({
flowVersion: data.version,
});
// Sync edge store with authoritative backend state
syncEdgesWithBackend(data.links);
// Clear the draft for this flow after successful save
if (data.id) {
await draftService.deleteDraft(data.id);

View File

@@ -120,12 +120,64 @@ export const useEdgeStore = create<EdgeStore>((set, get) => ({
isOutputConnected: (nodeId, handle) =>
get().edges.some((e) => e.source === nodeId && e.sourceHandle === handle),
getBackendLinks: () => get().edges.map(customEdgeToLink),
getBackendLinks: () => {
// Filter out edges referencing non-existent nodes before converting to links
const nodeIds = new Set(useNodeStore.getState().nodes.map((n) => n.id));
const validEdges = get().edges.filter((edge) => {
const isValid = nodeIds.has(edge.source) && nodeIds.has(edge.target);
if (!isValid) {
console.warn(
`[EdgeStore] Filtering out invalid edge during save: source=${edge.source}, target=${edge.target}`,
);
}
return isValid;
});
return validEdges.map(customEdgeToLink);
},
addLinks: (links) => {
links.forEach((link) => {
get().addEdge(linkToCustomEdge(link));
});
// Get current node IDs to validate links
const nodeIds = new Set(useNodeStore.getState().nodes.map((n) => n.id));
// Convert and filter links in one pass, avoiding individual addEdge calls
// which would push to history for each edge (causing history pollution)
const newEdges: CustomEdge[] = [];
const existingEdges = get().edges;
for (const link of links) {
// Skip invalid links (orphan edges referencing non-existent nodes)
if (!nodeIds.has(link.source_id) || !nodeIds.has(link.sink_id)) {
console.warn(
`[EdgeStore] Skipping invalid link: source=${link.source_id}, sink=${link.sink_id} - node(s) not found`,
);
continue;
}
const edge = linkToCustomEdge(link);
// Skip if edge already exists
const exists = existingEdges.some(
(e) =>
e.source === edge.source &&
e.target === edge.target &&
e.sourceHandle === edge.sourceHandle &&
e.targetHandle === edge.targetHandle,
);
if (!exists) {
newEdges.push(edge);
}
}
if (newEdges.length > 0) {
// Bulk add all edges at once, pushing to history only once
const prevState = {
nodes: useNodeStore.getState().nodes,
edges: existingEdges,
};
set((state) => ({ edges: [...state.edges, ...newEdges] }));
useHistoryStore.getState().pushState(prevState);
}
},
getAllHandleIdsOfANode: (nodeId) =>