🪲 fix: drop orphan edges on both endpoints, not just to

Addresses the P1 codex finding on #12740: `filterOrphanedEdges`
previously only removed edges whose `to` referenced a skipped agent.
Edges whose `from` was a skipped agent — the symmetric case in a
bidirectional graph like `A <-> B` where `B` is deleted or the user
lacks VIEW on it — leaked through to `createRun` and re-triggered
`Found edge ending at unknown node <id>` at StateGraph compile time.

The filter now drops an edge if either endpoint references a skipped
id, and the existing `to`-only test cases were updated to reflect the
stricter behavior. Adds a bidirectional-graph regression test in
`discovery.spec.ts`.
This commit is contained in:
Danny Avila 2026-04-18 11:10:16 -04:00
parent 5cc74e3870
commit 7418dd83b5
3 changed files with 64 additions and 16 deletions

View file

@ -202,6 +202,40 @@ describe('discoverConnectedAgents', () => {
expect(result.edges.map((e) => e.to)).toEqual(['B']);
});
it('drops edges whose `from` references a skipped agent (bidirectional graph)', async () => {
// Orchestrator A with bidirectional handoffs A<->B. B is inaccessible,
// so `A -> B` and `B -> A` must both be filtered — otherwise `B -> A`
// leaks into createRun and triggers `Found edge ending at unknown node`.
const edges: GraphEdge[] = [
{ from: 'A', to: 'B', edgeType: 'handoff' },
{ from: 'B', to: 'A', edgeType: 'handoff' },
];
const primaryConfig = makeConfig('A', edges);
const getAgent = jest.fn(async () => makeAgent('B', []));
const checkPermission = jest.fn().mockResolvedValue(false);
const result = await discoverConnectedAgents(
{
req: makeReq(),
res: makeRes(),
primaryConfig,
allowedProviders: new Set(),
modelsConfig: { openai: ['gpt-4o'] },
loadTools: jest.fn(),
},
{
getAgent,
checkPermission,
logViolation: jest.fn(),
db: {} as never,
},
);
expect(result.skippedAgentIds.has('B')).toBe(true);
expect(result.edges).toHaveLength(0);
});
it('does not re-initialize agents that are already known (dedup)', async () => {
const edges: GraphEdge[] = [
{ from: 'A', to: 'B', edgeType: 'handoff' },

View file

@ -131,25 +131,35 @@ describe('edges utilities', () => {
expect(filterOrphanedEdges(edges, skipped)).toEqual(edges);
});
it('should filter out edges with orphaned to targets', () => {
it('should filter out edges with orphaned to targets and orphaned from sources', () => {
const skipped = new Set(['agent_b']);
const result = filterOrphanedEdges(edges, skipped);
// Only filters edges where `to` is in skipped set
// agent_a -> agent_b (filtered - to=agent_b is skipped)
// agent_a -> agent_c (kept - to=agent_c not skipped)
// agent_b -> agent_d (kept - to=agent_d not skipped, from is not checked)
expect(result).toHaveLength(2);
expect(result.map((e) => e.to)).toEqual(['agent_c', 'agent_d']);
// agent_a -> agent_c (kept - neither endpoint skipped)
// agent_b -> agent_d (filtered - from=agent_b is skipped; a source-side
// orphan would otherwise leave the graph referencing an absent agent)
expect(result).toHaveLength(1);
expect(result[0].to).toBe('agent_c');
});
it('should filter out multiple orphaned edges', () => {
const skipped = new Set(['agent_b', 'agent_c']);
const result = filterOrphanedEdges(edges, skipped);
// agent_a -> agent_b (filtered)
// agent_a -> agent_c (filtered)
// agent_b -> agent_d (kept - to=agent_d not skipped)
// agent_a -> agent_b (filtered, to)
// agent_a -> agent_c (filtered, to)
// agent_b -> agent_d (filtered, from)
expect(result).toHaveLength(0);
});
it('should filter edges whose `from` is an array containing a skipped id', () => {
const edgesWithArrayFrom: GraphEdge[] = [
{ from: ['agent_a', 'agent_b'], to: 'agent_d', edgeType: 'handoff' },
{ from: 'agent_a', to: 'agent_d', edgeType: 'handoff' },
];
const skipped = new Set(['agent_b']);
const result = filterOrphanedEdges(edgesWithArrayFrom, skipped);
expect(result).toHaveLength(1);
expect(result[0].to).toBe('agent_d');
expect(result[0].from).toBe('agent_a');
});
it('should handle array to values', () => {

View file

@ -30,17 +30,21 @@ export function getEdgeParticipants(edge: GraphEdge): string[] {
}
/**
* Filters out edges that reference non-existent (orphaned) agents.
* Only filters based on the 'to' field since those are the handoff targets.
* Filters out edges that reference non-existent (orphaned) agents on
* either endpoint. Any edge whose `from` OR `to` references a skipped
* agent id is dropped — leaving a source-side orphan in place would
* still fail graph compilation with `Found edge ending at unknown node`
* because `agents` no longer contains the source.
*/
export function filterOrphanedEdges(edges: GraphEdge[], skippedAgentIds: Set<string>): GraphEdge[] {
if (!edges || skippedAgentIds.size === 0) {
return edges;
}
return edges.filter((edge) => {
const toIds = Array.isArray(edge.to) ? edge.to : [edge.to];
return !toIds.some((id) => typeof id === 'string' && skippedAgentIds.has(id));
});
const referencesSkipped = (value: string | string[]): boolean => {
const ids = Array.isArray(value) ? value : [value];
return ids.some((id) => typeof id === 'string' && skippedAgentIds.has(id));
};
return edges.filter((edge) => !referencesSkipped(edge.from) && !referencesSkipped(edge.to));
}
/** Collects all unique agent IDs referenced across an array of edges. */