mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-22 01:53:33 +00:00
🕸 fix: require all sources reachable when traversing fan-in edges
Addresses the seventh P1 codex finding on #12740: the reachability BFS advanced through an edge as soon as any of its `from` endpoints matched the current frontier node (`sources.includes(current)`), but the subsequent edge filter required ALL sources to be reachable (`every`). The two-semantics mismatch let a fan-in edge like `{from: ['A','B'], to: 'C'}` mark C reachable purely via A even when B had no path from the primary, then drop the edge itself at filter time. Result: C survived in `agentConfigs` with no surviving edge connecting it to A, so `createRun` flipped into multi-agent mode on `agents.length > 1` and C ran as an unintended parallel root. Replace the BFS with a fixed-point iteration keyed on the same all-sources-reachable predicate used by the filter, so traversal and filtering stay aligned and multi-source edges only fire once every source is in the reachable set. Two regression tests added: - `{from: ['A','B'], to: 'C'}` with B having no incoming path — asserts neither B nor C leak into the result. - `A -> B`, `A -> C`, `['B','C'] -> D` — asserts the fan-in edge fires and D becomes reachable once both B and C are.
This commit is contained in:
parent
6450aa8751
commit
4982f1c3b0
2 changed files with 107 additions and 11 deletions
|
|
@ -311,6 +311,89 @@ describe('discoverConnectedAgents', () => {
|
||||||
expect(result.agentConfigs.size).toBe(0);
|
expect(result.agentConfigs.size).toBe(0);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('requires all sources to be reachable before advancing through a multi-source edge', async () => {
|
||||||
|
// Primary A has a single fan-in edge `{from: ['A','B'], to: 'C'}`.
|
||||||
|
// B loads successfully but has no incoming path from A, so the edge
|
||||||
|
// cannot legitimately fire. C must NOT be marked reachable (and thus
|
||||||
|
// must NOT end up in `agentConfigs`) — otherwise `createRun` sees
|
||||||
|
// `[primaryConfig, C]`, flips into multi-agent mode, and runs C as an
|
||||||
|
// unintended root because no edge connects A to C on its own.
|
||||||
|
const edges: GraphEdge[] = [{ from: ['A', 'B'], to: 'C', edgeType: 'direct' }];
|
||||||
|
const primaryConfig = makeConfig('A', edges);
|
||||||
|
|
||||||
|
const agentMap: Record<string, Agent> = {
|
||||||
|
B: makeAgent('B', []),
|
||||||
|
C: makeAgent('C', []),
|
||||||
|
};
|
||||||
|
const getAgent = jest.fn(async ({ id }: { id: string }) => agentMap[id] ?? null);
|
||||||
|
const checkPermission = jest.fn().mockResolvedValue(true);
|
||||||
|
|
||||||
|
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,
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
// Neither B nor C is reachable from A through the surviving edge set
|
||||||
|
// (the fan-in edge requires B, which has no incoming path).
|
||||||
|
expect(result.agentConfigs.has('B')).toBe(false);
|
||||||
|
expect(result.agentConfigs.has('C')).toBe(false);
|
||||||
|
expect(result.edges).toHaveLength(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('advances through a multi-source edge once all sources are reachable', async () => {
|
||||||
|
// Two independent paths converge: `A -> B` and `A -> C` both reach
|
||||||
|
// the fan-in edge `['B','C'] -> D`. Once both B and C are reachable,
|
||||||
|
// D should become reachable in a subsequent fixed-point iteration.
|
||||||
|
const edges: GraphEdge[] = [
|
||||||
|
{ from: 'A', to: 'B', edgeType: 'direct' },
|
||||||
|
{ from: 'A', to: 'C', edgeType: 'direct' },
|
||||||
|
{ from: ['B', 'C'], to: 'D', edgeType: 'direct' },
|
||||||
|
];
|
||||||
|
const primaryConfig = makeConfig('A', edges);
|
||||||
|
|
||||||
|
const agentMap: Record<string, Agent> = {
|
||||||
|
B: makeAgent('B', []),
|
||||||
|
C: makeAgent('C', []),
|
||||||
|
D: makeAgent('D', []),
|
||||||
|
};
|
||||||
|
const getAgent = jest.fn(async ({ id }: { id: string }) => agentMap[id] ?? null);
|
||||||
|
const checkPermission = jest.fn().mockResolvedValue(true);
|
||||||
|
|
||||||
|
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.agentConfigs.has('B')).toBe(true);
|
||||||
|
expect(result.agentConfigs.has('C')).toBe(true);
|
||||||
|
expect(result.agentConfigs.has('D')).toBe(true);
|
||||||
|
expect(result.edges).toHaveLength(3);
|
||||||
|
});
|
||||||
|
|
||||||
it('prunes surviving-but-unreachable edges from the return value (A->B->C->D, B skipped)', async () => {
|
it('prunes surviving-but-unreachable edges from the return value (A->B->C->D, B skipped)', async () => {
|
||||||
// All three edges are stored on the primary A. When B is skipped,
|
// All three edges are stored on the primary A. When B is skipped,
|
||||||
// `filterOrphanedEdges` removes A->B (to=B) and B->C (from=B) — but
|
// `filterOrphanedEdges` removes A->B (to=B) and B->C (from=B) — but
|
||||||
|
|
|
||||||
|
|
@ -316,20 +316,36 @@ export async function discoverConnectedAgents(
|
||||||
* every agent id referenced by an edge is guaranteed to be either
|
* every agent id referenced by an edge is guaranteed to be either
|
||||||
* `primaryConfig.id` or a key of `agentConfigs`.
|
* `primaryConfig.id` or a key of `agentConfigs`.
|
||||||
*/
|
*/
|
||||||
|
const edgeEndpointIsReachable = (
|
||||||
|
value: string | string[],
|
||||||
|
reachableSet: Set<string>,
|
||||||
|
): boolean => {
|
||||||
|
const ids = Array.isArray(value) ? value : [value];
|
||||||
|
return ids.every((id) => typeof id !== 'string' || reachableSet.has(id));
|
||||||
|
};
|
||||||
|
|
||||||
|
// Fixed-point reachability: an edge only advances reachability when ALL
|
||||||
|
// of its `from` endpoints are already reachable. Matches the edge-filter
|
||||||
|
// semantics below and handles multi-source / fan-in edges correctly:
|
||||||
|
// `{ from: ['A', 'B'], to: 'C' }` can only fire when both A and B reach
|
||||||
|
// it, so C shouldn't be marked reachable just because A is in the source
|
||||||
|
// list. The previous `sources.includes(current)` BFS over-approximated
|
||||||
|
// reachability for fan-in edges and left C in `agentConfigs` while the
|
||||||
|
// edge itself got dropped — `createRun` then saw `agents.length > 1`
|
||||||
|
// with a disconnected C and ran it as an unintended parallel root.
|
||||||
const reachable = new Set<string>([primaryConfig.id]);
|
const reachable = new Set<string>([primaryConfig.id]);
|
||||||
const frontier: string[] = [primaryConfig.id];
|
let changed = true;
|
||||||
while (frontier.length > 0) {
|
while (changed) {
|
||||||
const current = frontier.pop() as string;
|
changed = false;
|
||||||
for (const edge of filteredEdges) {
|
for (const edge of filteredEdges) {
|
||||||
const sources = Array.isArray(edge.from) ? edge.from : [edge.from];
|
if (!edgeEndpointIsReachable(edge.from, reachable)) {
|
||||||
if (!sources.includes(current)) {
|
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
const dests = Array.isArray(edge.to) ? edge.to : [edge.to];
|
const dests = Array.isArray(edge.to) ? edge.to : [edge.to];
|
||||||
for (const dest of dests) {
|
for (const dest of dests) {
|
||||||
if (typeof dest === 'string' && !reachable.has(dest)) {
|
if (typeof dest === 'string' && !reachable.has(dest)) {
|
||||||
reachable.add(dest);
|
reachable.add(dest);
|
||||||
frontier.push(dest);
|
changed = true;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -341,12 +357,9 @@ export async function discoverConnectedAgents(
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const edgeEndpointIsReachable = (value: string | string[]): boolean => {
|
|
||||||
const ids = Array.isArray(value) ? value : [value];
|
|
||||||
return ids.every((id) => typeof id !== 'string' || reachable.has(id));
|
|
||||||
};
|
|
||||||
const edges = filteredEdges.filter(
|
const edges = filteredEdges.filter(
|
||||||
(edge) => edgeEndpointIsReachable(edge.from) && edgeEndpointIsReachable(edge.to),
|
(edge) =>
|
||||||
|
edgeEndpointIsReachable(edge.from, reachable) && edgeEndpointIsReachable(edge.to, reachable),
|
||||||
);
|
);
|
||||||
|
|
||||||
return {
|
return {
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue