From d2cbd551b79bada9df625876eb0c0066d9e2f9d8 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 19 Apr 2026 23:20:43 -0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=A4=9D=20fix:=20Load=20Handoff=20Agents?= =?UTF-8?q?=20for=20Agents=20API=20(#12740)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🀝 fix: load handoff sub-agents on OpenAI-compat endpoints (#12726) Extracts the BFS discovery + ACL-gated initialization of handoff sub-agents into a shared `discoverConnectedAgents` helper in `@librechat/api` and wires it into the OpenAI-compatible `/v1/chat/completions` and Open Responses `/v1/responses` controllers. These endpoints previously only passed the primary agent config to `createRun` while keeping `primaryConfig.edges` intact, which forced `MultiAgentGraph` into multi-agent mode without loading the referenced sub-agents and caused StateGraph to throw "Found edge ending at unknown node ". The discovery helper also filters orphaned edges (deleted sub-agents or those the caller lacks VIEW permission on), so API users see the same graceful fallback the chat UI already had. * πŸ§ͺ fix: use ServerRequest in discovery spec helpers CI `tsc --noEmit -p packages/api/tsconfig.json` caught that the test helpers typed `req` as `express.Request`, which is not assignable to `DiscoverConnectedAgentsParams.req` (typed as `ServerRequest` whose `user` is `IUser`). Local jest passed because ts-jest is transpile-only, but the CI typecheck uses the full compiler. * πŸͺ² 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 ` 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`. * πŸ”’ fix: enforce REMOTE_AGENT ACL on handoff sub-agents for API routes Addresses the second P1 codex finding on #12740: the OpenAI-compat `/v1/chat/completions` and Open Responses `/v1/responses` routes gate the primary agent on `REMOTE_AGENT` (via `createCheckRemoteAgentAccess`), but `discoverConnectedAgents` was checking handoff sub-agents against the looser in-app `AGENT` resource type. That allowed a remote caller who could reach the orchestrator but had only in-app visibility on a sub-agent to invoke it via the API β€” bypassing the remote-sharing boundary. Adds an optional `resourceType` param to `discoverConnectedAgents` (defaulting to `AGENT` for the chat UI path) and passes `ResourceType.REMOTE_AGENT` from both API controllers so every discovered sub-agent clears the same sharing boundary enforced at route entry. * 🧯 fix: enforce allowedProviders for discovered sub-agents Addresses the third P1 codex finding on #12740: `discoverConnectedAgents` forwarded the caller's `endpointOption` verbatim into `initializeAgent`, but on the OpenAI-compat routes that option's `endpoint` is the primary agent's provider (e.g. `openai`), not `agents`. `initializeAgent` only enforces `allowedProviders` when `isAgentsEndpoint(endpointOption.endpoint)` is true, so handoff sub-agents silently bypassed the provider allowlist configured under `endpoints.agents.allowedProviders`. Override `endpointOption.endpoint` to `EModelEndpoint.agents` for every per-sub-agent init call. The primary agent still uses the caller's endpointOption as before β€” this only affects the BFS-loaded handoff targets. Regression test asserts the override. * βœ‚οΈ fix: prune unreachable sub-agents after orphan-edge filtering Addresses the fourth P1 codex finding on #12740: BFS eagerly initializes every sub-agent referenced in the primary's edge scan, but once `filterOrphanedEdges` drops edges whose endpoints were skipped, some of those sub-agents end up disconnected from the primary. In an `A -> B -> C` graph (edges stored directly on A) where B is skipped (missing or no VIEW), both edges are filtered, but C was already loaded and would still be passed to `createRun` β€” which flips into multi-agent mode on `agents.length > 1` and turns C into an unintended parallel start node. After filtering edges, compute the set of agent ids reachable from the primary through the surviving edge set and prune `agentConfigs` to that set. Two regression tests added: one for the pruning case, one that confirms agents connected via surviving edges are still kept. * πŸ” fix: don't seed initialize.js agentConfigs from the pre-pruning callback Addresses the fifth P1 codex finding on #12740: `onAgentInitialized` fires during BFS, BEFORE the helper prunes agents that become disconnected once `filterOrphanedEdges` runs. Writing the sub-agent straight into the outer `agentConfigs` there and then only additively merging the pruned `discoveredConfigs` left stranded entries in the outer map, and `AgentClient` would still hand them to `createRun` as extra parallel start nodes (the exact failure mode the pass-4 prune was meant to eliminate for the API controllers). Drop the `agentConfigs.set` from the callback and replace the additive merge with a direct copy from `discoveredConfigs`, which is now the single authoritative source of what the run should see. The per-agent tool context map is still populated during BFS β€” stale entries there are harmless because they're only read by closure inside `ON_TOOL_EXECUTE` and are unreachable once the agent is not in `agentConfigs`. * πŸ”¬ fix: address audit findings on discovery helper Resolves findings from a comprehensive external audit of #12740. **Finding 1 (CRITICAL) β€” stale edges survive the reachability prune.** The pass-4 prune removed unreachable agents from `agentConfigs` but left matching edges in the return value. In an `A -> B -> C -> D` graph (all edges stored on A) where B is skipped, `filterOrphanedEdges` drops A->B and B->C but keeps C->D (neither endpoint is skipped). The caller then sees `agentConfigs` without C/D but `edges` still references them, flipping `createRun` into multi-agent mode with mismatched agents/edges β€” the exact crash this PR is supposed to fix. Now filter the edge list to the reachable set in the same pass, so the returned shape is self-consistent: every edge endpoint is either the primary id or a key of `agentConfigs`. New regression test covers A->B->C->D with B skipped. **Finding 2 (MAJOR) β€” unconditional `getModelsConfig` on every API request.** The OpenAI-compat and Responses controllers called `getModelsConfig(req)` and `discoverConnectedAgents` even when the primary agent had no edges (the common single-agent API case). Gate both behind `primaryConfig.edges?.length > 0` so single-agent runs don't pay that cost. **Finding 5 (MINOR) β€” silent mutation of caller's `primaryConfig.userMCPAuthMap`.** The helper aliased that object and then `Object.assign`'d sub-agent entries into it, changing the caller's config in-place. Shallow-clone up front so the returned merged map is the only destination. **Finding 7 (NIT) β€” dead `?? []` coalescing.** `filterOrphanedEdges` always returns a concrete array, so the `discoveredEdges ?? []` fallback was never reached. Simplified the `primaryConfig.edges = …` assignment. Also adds a test that verifies `primaryConfig.userMCPAuthMap` is not mutated in-place. * 🧹 chore: address audit NITs on discovery helper Addresses two NIT findings from the post-fix audit: **F1** β€” the shallow clone on `primaryConfig.userMCPAuthMap` was only applied on the primary side; the `else` branch (hit when the primary had no MCP auth and the first sub-agent seeds the map) assigned the sub-agent's `config.userMCPAuthMap` directly, so a later sub-agent's `Object.assign` mutated the first one's map in place. Harmless in practice (per-request ephemeral objects) but asymmetric. Clone in the else branch too. Test added. **F2** β€” `initialize.js` had a defensive `if (agentConfigs.size > 0 && !edges) edges = []` normalizer. Pre-existing dead code: the helper now always returns a concrete array from `filteredEdges.filter(...)`. Removed for clarity. * πŸ•Έ 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. * πŸ”€ fix: match SDK OR semantics for multi-source edge reachability Reverts the all-sources-required reachability gate from 4982f1c3b and replaces it with an any-source-reachable model, which matches how `@librechat/agents`'s `MultiAgentGraph.createWorkflow` actually wires multi-source edges at runtime (per-source `builder.addEdge(source, destination)`). With the previous `every` gate, a legitimate handoff edge `{ from: ['A', 'B'], to: 'C' }` where B had no incoming path was pruned along with C, regressing OR-semantics routing that the SDK would otherwise handle correctly. New behavior: 1. Reachability: an edge advances when ANY of its `from` endpoints is already reachable. Fixed-point iteration over `filteredEdges`. 2. Edge filter: keep an edge when it has at least one reachable source AND all destinations are reachable (a missing destination would still crash `StateGraph.compile` with `Found edge ending at unknown node`). 3. Agent prune: keep agents that are reachable OR referenced on any endpoint of a surviving edge. The second clause preserves co-sources in multi-source edges (B in `{ from: ['A','B'], to: 'C' }` when nothing else reaches B) so the SDK's per-source `addEdge` β€” and the `validateEdgeAgents` safety-net I added to the SDK in #111 β€” still finds B as a node. The pass-audit A->B->C->D regression test continues to pass: with B skipped, `filterOrphanedEdges` drops both B-adjacent edges, reachability never expands past A, C->D has no reachable source so it gets filtered, and C/D are pruned because they're neither reachable nor referenced. * βœ‚οΈ fix: strip skipped co-members from multi-source/multi-dest edges Addresses codex pass-9 P2 on #12740. `filterOrphanedEdges` previously dropped an edge whenever any `from` id was skipped, which was correct for scalar edges but over-aggressive for multi-source ones: the agents SDK adds one `builder.addEdge(source, destination)` per source, so `{ from: ['A','B'], to: 'C' }` with B skipped still has a valid `A -> C` route that was being thrown away. Now sanitize each endpoint: - Scalar skipped β†’ drop the whole edge (no route survives). - Array with some skipped β†’ strip the skipped ids, keep the edge with the surviving members. If the array empties out, drop the edge. Symmetric handling for `to` covers multi-destination fan-out when one co-destination is skipped. Tests updated/added: - `strips skipped co-sources from multi-source edges…` - `strips skipped co-destinations from multi-destination edges` - `drops multi-member edges only when every member on a side is skipped` - Discovery-side: `preserves valid routes when one co-source of a multi-source edge is skipped` asserts the end-to-end behavior β€” skipped co-source B gets stripped from the edge, A->C routing survives, and C remains in `agentConfigs`. * πŸ”“ fix: respect SHARE-on-AGENT fallback for handoff ACL on API routes Addresses codex pass-10 P1 on #12740. The API controllers were handing `discoverConnectedAgents` a raw `PermissionService.checkPermission` call against `ResourceType.REMOTE_AGENT`, but the route-level middleware (`createCheckRemoteAgentAccess`) authorizes the primary agent via `getRemoteAgentPermissions`, which first consults the AGENT ACL and treats owners with the SHARE bit as remotely authorized even without an explicit REMOTE_AGENT grant. The mismatch meant a user could open the primary via `/v1/chat/completions` or `/v1/responses`, but their own owned handoff sub-agents were silently skipped β€” breaking multi-agent handoffs for the common "owner runs their own multi-agent orchestrator" case. Both controllers now pass `discoverConnectedAgents` a `checkPermission` wrapper that delegates to `getRemoteAgentPermissions` (with `getEffectivePermissions` injected from `PermissionService`) and compares the returned bitmask against the required permission via `hasPermissions`. Sub-agents are now authorized by the exact same rules the route middleware applies to the primary. * 🌱 fix: preserve user-defined parallel-start branches Addresses codex pass-11 P2 on #12740. The post-filter reachability prune seeded only from `primaryConfig.id`, which killed `MultiAgentGraph`'s legitimate multi-start pattern β€” a user-defined edge like `X -> Y` where X has no incoming path (X is an intentional parallel starting node, run alongside the primary) was being dropped because neither X nor Y was reachable from the primary. Reconcile the tension with pass-4 ("prune accidental orphans when an intermediate is skipped") by using pre-filter reachability as the signal: - An agent that WAS reachable from the primary via the original (pre-filter) edges but loses that path when `filterOrphanedEdges` runs is an accidental orphan (a skipped hop broke the chain) β€” prune. - An agent that was NEVER reachable from the primary, even pre-filter, is an intentional parallel start β€” seed it into post-filter reachability so its component survives. Surviving-edge endpoint references still keep an agent (co-sources in multi-source edges). New test `preserves user-defined parallel-start branches disconnected from the primary` covers the pass-11 scenario; the existing `A->B->C->D, B skipped` regression test continues to pass because C/D were pre-filter reachable through B and lose that reachability after filtering. * 🎯 fix: tighten parallel-start seed criterion to 'no pre-filter incoming edge' Addresses codex pass-12 P1 on #12740. The pass-11 seed heuristic β€” 'agent is in `agentConfigs` but was not pre-filter reachable from the primary' β€” was too permissive. A downstream agent like Y in `X -> Y` where X gets skipped (missing / no VIEW) was never pre-filter reachable from the primary either, so the old rule promoted Y to a parallel start node and discovery returned `agents: [primary, Y]` with no connecting edge. The SDK then ran Y as an unintended parallel root β€” exactly the orphan behavior pass-4 wanted to prevent. Tighter criterion: seed a post-filter reachability root only when the agent had NO incoming edge in the pre-filter graph. That matches `MultiAgentGraph.analyzeGraph`'s "no-incoming-edge" definition of a start node applied to the user's original declared topology, so: - `A -> B` plus a user-defined `X -> Y` parallel branch: X has no incoming pre-filter β†’ seeded β†’ X and Y both survive. - `A -> B` plus `X -> Y` with X skipped: Y had an incoming pre-filter (`X -> Y`) β†’ NOT seeded β†’ Y is pruned as the orphan it is. - `A -> B -> C` with B skipped: C had an incoming pre-filter (`B -> C`) β†’ NOT seeded β†’ C is pruned. New test `does not promote a downstream orphan to a parallel start when its only upstream is skipped` locks in the pass-12 scenario. The pass-11 `preserves user-defined parallel-start branches` test continues to hold. * πŸ“ fix: don't enforce AGENT-only file ACL on REMOTE_AGENT API callers Addresses codex pass-13 P1 on #12740. When I refactored the API controllers' DB-method bundle, I inadvertently started forwarding `filterFilesByAgentAccess` into `initializeAgent`. That helper calls `checkPermission` with `resourceType: ResourceType.AGENT`, but these routes authorize callers through `REMOTE_AGENT` (via `getRemoteAgentPermissions`). A user granted `REMOTE_AGENT_VIEWER` on a shared agent but lacking direct `AGENT_VIEW` could invoke the agent yet all its owner-attached context files would get silently filtered out β€” breaking `file_search`/context retrieval for remote consumers. Drop `filterFilesByAgentAccess` from the OpenAI-compat and Responses controllers' `dbMethods` (and remove the now-unused import). The chat UI's `initialize.js` keeps it since that path legitimately authorizes at the AGENT level. No functional change inside the helper β€” passing `undefined` simply tells `primeResources` to skip the per-file ACL filter, restoring the pre-refactor API behavior. * πŸͺ“ fix: strip unreachable co-sources from surviving multi-source edges Addresses codex pass-14 P1 on #12740. The earlier pass-8 fix kept any agent referenced as an endpoint of a surviving edge (via a `referencedByEdge` fallback) to avoid the SDK's `validateEdgeAgents` failing on missing nodes. But that fallback propped up unreachable co-sources too: with `[A -> C, X -> B, [B,C] -> D]` and X skipped, `X -> B` gets filtered, the `[B,C] -> D` fan-in survives because C is reachable, and B stays in `agentConfigs` solely because the fan-in still lists it. `MultiAgentGraph.analyzeGraph` then sees B with no incoming edge and runs it as an unintended parallel root. Sanitize surviving edges instead: for a kept edge whose `from` is an array, filter out any co-source that isn't reachable. The SDK's per-source `addEdge` fires independently, so dropping an unreachable co-source doesn't invalidate the remaining routes β€” in the scenario above `[B,C] -> D` becomes `[C] -> D`, every endpoint of every surviving edge is now reachable, and the agent prune collapses to a strict `reachable.has(agentId)` check. No more referenced-by-edge fallback. Regression test added: `strips unreachable co-sources from surviving multi-source edges (no stray parallel root)` β€” asserts B is absent from every surviving edge endpoint and the fan-in's `from` is just `['C']`. All 22 prior discovery tests still pass unchanged. --- .../agents/__tests__/openai.spec.js | 21 + .../agents/__tests__/responses.unit.spec.js | 21 + api/server/controllers/agents/openai.js | 151 ++- api/server/controllers/agents/responses.js | 167 ++- .../services/Endpoints/agents/initialize.js | 178 +--- packages/api/src/agents/discovery.spec.ts | 999 ++++++++++++++++++ packages/api/src/agents/discovery.ts | 450 ++++++++ packages/api/src/agents/edges.spec.ts | 58 +- packages/api/src/agents/edges.ts | 75 +- packages/api/src/agents/index.ts | 1 + packages/api/src/agents/validation.ts | 13 +- 11 files changed, 1932 insertions(+), 202 deletions(-) create mode 100644 packages/api/src/agents/discovery.spec.ts create mode 100644 packages/api/src/agents/discovery.ts diff --git a/api/server/controllers/agents/__tests__/openai.spec.js b/api/server/controllers/agents/__tests__/openai.spec.js index c959be6cf4..f55b798f14 100644 --- a/api/server/controllers/agents/__tests__/openai.spec.js +++ b/api/server/controllers/agents/__tests__/openai.spec.js @@ -46,9 +46,11 @@ jest.mock('@librechat/api', () => ({ .fn() .mockReturnValue({ request: { model: 'agent-123', messages: [], stream: false } }), initializeAgent: jest.fn().mockResolvedValue({ + id: 'agent-123', model: 'gpt-4', model_parameters: {}, toolRegistry: {}, + edges: [], }), getBalanceConfig: mockGetBalanceConfig, createErrorResponse: jest.fn(), @@ -72,6 +74,24 @@ jest.mock('@librechat/api', () => ({ resolveRecursionLimit: jest.fn().mockReturnValue(50), createToolExecuteHandler: jest.fn().mockReturnValue({ handle: jest.fn() }), isChatCompletionValidationFailure: jest.fn().mockReturnValue(false), + discoverConnectedAgents: jest.fn().mockResolvedValue({ + agentConfigs: new Map(), + edges: [], + skippedAgentIds: new Set(), + userMCPAuthMap: undefined, + }), +})); + +jest.mock('~/server/controllers/ModelController', () => ({ + getModelsConfig: jest.fn().mockResolvedValue({}), +})); + +jest.mock('~/server/services/Files/permissions', () => ({ + filterFilesByAgentAccess: jest.fn(), +})); + +jest.mock('~/cache', () => ({ + logViolation: jest.fn(), })); jest.mock('~/server/services/ToolService', () => ({ @@ -91,6 +111,7 @@ jest.mock('~/server/controllers/agents/callbacks', () => ({ jest.mock('~/server/services/PermissionService', () => ({ findAccessibleResources: jest.fn().mockResolvedValue([]), + checkPermission: jest.fn().mockResolvedValue(true), })); const mockUpdateBalance = jest.fn().mockResolvedValue({}); diff --git a/api/server/controllers/agents/__tests__/responses.unit.spec.js b/api/server/controllers/agents/__tests__/responses.unit.spec.js index 26f5f5d30b..1a6fa6053f 100644 --- a/api/server/controllers/agents/__tests__/responses.unit.spec.js +++ b/api/server/controllers/agents/__tests__/responses.unit.spec.js @@ -43,9 +43,17 @@ jest.mock('@librechat/api', () => ({ buildToolSet: jest.fn().mockReturnValue(new Set()), createSafeUser: jest.fn().mockReturnValue({ id: 'user-123' }), initializeAgent: jest.fn().mockResolvedValue({ + id: 'agent-123', model: 'claude-3', model_parameters: {}, toolRegistry: {}, + edges: [], + }), + discoverConnectedAgents: jest.fn().mockResolvedValue({ + agentConfigs: new Map(), + edges: [], + skippedAgentIds: new Set(), + userMCPAuthMap: undefined, }), getBalanceConfig: mockGetBalanceConfig, getTransactionsConfig: mockGetTransactionsConfig, @@ -121,6 +129,19 @@ jest.mock('~/server/controllers/agents/callbacks', () => { jest.mock('~/server/services/PermissionService', () => ({ findAccessibleResources: jest.fn().mockResolvedValue([]), + checkPermission: jest.fn().mockResolvedValue(true), +})); + +jest.mock('~/server/controllers/ModelController', () => ({ + getModelsConfig: jest.fn().mockResolvedValue({}), +})); + +jest.mock('~/server/services/Files/permissions', () => ({ + filterFilesByAgentAccess: jest.fn(), +})); + +jest.mock('~/cache', () => ({ + logViolation: jest.fn(), })); const mockUpdateBalance = jest.fn().mockResolvedValue({}); diff --git a/api/server/controllers/agents/openai.js b/api/server/controllers/agents/openai.js index 9fa3af82c3..6b07993715 100644 --- a/api/server/controllers/agents/openai.js +++ b/api/server/controllers/agents/openai.js @@ -1,7 +1,12 @@ const { nanoid } = require('nanoid'); const { logger } = require('@librechat/data-schemas'); const { Callback, ToolEndHandler, formatAgentMessages } = require('@librechat/agents'); -const { EModelEndpoint, ResourceType, PermissionBits } = require('librechat-data-provider'); +const { + EModelEndpoint, + ResourceType, + PermissionBits, + hasPermissions, +} = require('librechat-data-provider'); const { writeSSE, createRun, @@ -21,6 +26,8 @@ const { createOpenAIStreamTracker, createOpenAIContentAggregator, isChatCompletionValidationFailure, + discoverConnectedAgents, + getRemoteAgentPermissions, } = require('@librechat/api'); const { buildSummarizationHandlers, @@ -29,7 +36,12 @@ const { agentLogHandlerObj, } = require('~/server/controllers/agents/callbacks'); const { loadAgentTools, loadToolsForExecution } = require('~/server/services/ToolService'); -const { findAccessibleResources } = require('~/server/services/PermissionService'); +const { + findAccessibleResources, + getEffectivePermissions, +} = require('~/server/services/PermissionService'); +const { getModelsConfig } = require('~/server/controllers/ModelController'); +const { logViolation } = require('~/cache'); const db = require('~/models'); /** @@ -207,6 +219,24 @@ const OpenAIChatCompletionController = async (req, res) => { model_parameters: agent.model_parameters ?? {}, }; + // `filterFilesByAgentAccess` is intentionally omitted: it calls + // `checkPermission` with `resourceType: AGENT`, but this route + // authorizes callers through `REMOTE_AGENT` (via + // `getRemoteAgentPermissions`), so including it would silently drop + // owner-attached context files for any remote user who has + // `REMOTE_AGENT_VIEWER` but not direct `AGENT_VIEW`. + const dbMethods = { + getConvoFiles: db.getConvoFiles, + getFiles: db.getFiles, + getUserKey: db.getUserKey, + getMessages: db.getMessages, + updateFilesUsage: db.updateFilesUsage, + getUserKeyValues: db.getUserKeyValues, + getUserCodeFiles: db.getUserCodeFiles, + getToolFilesByIds: db.getToolFilesByIds, + getCodeGeneratedFiles: db.getCodeGeneratedFiles, + }; + const primaryConfig = await initializeAgent( { req, @@ -220,19 +250,92 @@ const OpenAIChatCompletionController = async (req, res) => { allowedProviders, isInitialAgent: true, }, - { - getConvoFiles: db.getConvoFiles, - getFiles: db.getFiles, - getUserKey: db.getUserKey, - getMessages: db.getMessages, - updateFilesUsage: db.updateFilesUsage, - getUserKeyValues: db.getUserKeyValues, - getUserCodeFiles: db.getUserCodeFiles, - getToolFilesByIds: db.getToolFilesByIds, - getCodeGeneratedFiles: db.getCodeGeneratedFiles, - }, + dbMethods, ); + /** + * Per-agent tool-execution context map, keyed by agentId. + * Needed so the ON_TOOL_EXECUTE callback routes each sub-agent's tool calls + * to the correct toolRegistry / userMCPAuthMap / tool_resources. + * @type {Map>, + * tool_resources?: object, + * actionsEnabled?: boolean, + * }>} + */ + const agentToolContexts = new Map(); + agentToolContexts.set(primaryConfig.id, { + agent, + toolRegistry: primaryConfig.toolRegistry, + userMCPAuthMap: primaryConfig.userMCPAuthMap, + tool_resources: primaryConfig.tool_resources, + actionsEnabled: primaryConfig.actionsEnabled, + }); + + // Only run BFS discovery (and pay `getModelsConfig` upfront) when the + // primary has edges to follow β€” the common API case is single-agent. + let handoffAgentConfigs = new Map(); + let discoveredEdges = []; + let discoveredMCPAuthMap; + if (primaryConfig.edges?.length) { + const modelsConfig = await getModelsConfig(req); + ({ + agentConfigs: handoffAgentConfigs, + edges: discoveredEdges, + userMCPAuthMap: discoveredMCPAuthMap, + } = await discoverConnectedAgents( + { + req, + res, + primaryConfig, + endpointOption, + allowedProviders, + modelsConfig, + loadTools, + requestFiles: [], + conversationId, + parentMessageId, + // The route enforces REMOTE_AGENT on the primary; every discovered + // sub-agent must clear the same sharing boundary, not the looser + // in-app AGENT one. + resourceType: ResourceType.REMOTE_AGENT, + }, + { + getAgent: db.getAgent, + // Use `getRemoteAgentPermissions` so sub-agent authorization + // matches what the route's `createCheckRemoteAgentAccess` + // middleware does for the primary: AGENT owners with the SHARE + // bit are treated as remotely authorized even without an + // explicit REMOTE_AGENT grant. + checkPermission: async ({ userId, role, resourceId, requiredPermission }) => { + const permissions = await getRemoteAgentPermissions( + { getEffectivePermissions }, + userId, + role, + resourceId, + ); + return hasPermissions(permissions, requiredPermission); + }, + logViolation, + db: dbMethods, + onAgentInitialized: (agentId, handoffAgent, config) => { + agentToolContexts.set(agentId, { + agent: handoffAgent, + toolRegistry: config.toolRegistry, + userMCPAuthMap: config.userMCPAuthMap, + tool_resources: config.tool_resources, + actionsEnabled: config.actionsEnabled, + }); + }, + initializeAgent, + }, + )); + } + + primaryConfig.edges = discoveredEdges; + // Determine if streaming is enabled (check both request and agent config) const streamingDisabled = !!primaryConfig.model_parameters?.disableStreaming; const isStreaming = request.stream === true && !streamingDisabled; @@ -270,17 +373,18 @@ const OpenAIChatCompletionController = async (req, res) => { const toolEndCallback = createToolEndCallback({ req, res, artifactPromises, streamId: null }); const toolExecuteOptions = { - loadTools: async (toolNames) => { + loadTools: async (toolNames, agentId) => { + const ctx = agentToolContexts.get(agentId) ?? agentToolContexts.get(primaryConfig.id) ?? {}; return loadToolsForExecution({ req, res, - agent, toolNames, + agent: ctx.agent ?? agent, signal: abortController.signal, - toolRegistry: primaryConfig.toolRegistry, - userMCPAuthMap: primaryConfig.userMCPAuthMap, - tool_resources: primaryConfig.tool_resources, - actionsEnabled: primaryConfig.actionsEnabled, + toolRegistry: ctx.toolRegistry, + userMCPAuthMap: ctx.userMCPAuthMap, + tool_resources: ctx.tool_resources, + actionsEnabled: ctx.actionsEnabled, }); }, toolEndCallback, @@ -467,11 +571,14 @@ const OpenAIChatCompletionController = async (req, res) => { // Create and run the agent const userId = req.user?.id ?? 'api-user'; - // Extract userMCPAuthMap from primaryConfig (needed for MCP tool connections) - const userMCPAuthMap = primaryConfig.userMCPAuthMap; + // Extract merged userMCPAuthMap (needed for MCP tool connections across + // the primary and any discovered handoff sub-agents) + const userMCPAuthMap = discoveredMCPAuthMap ?? primaryConfig.userMCPAuthMap; + + const runAgents = [primaryConfig, ...handoffAgentConfigs.values()]; const run = await createRun({ - agents: [primaryConfig], + agents: runAgents, messages: formattedMessages, indexTokenCountMap, initialSummary, diff --git a/api/server/controllers/agents/responses.js b/api/server/controllers/agents/responses.js index 7abddf5e2f..beade07414 100644 --- a/api/server/controllers/agents/responses.js +++ b/api/server/controllers/agents/responses.js @@ -2,7 +2,12 @@ const { nanoid } = require('nanoid'); const { v4: uuidv4 } = require('uuid'); const { logger } = require('@librechat/data-schemas'); const { Callback, ToolEndHandler, formatAgentMessages } = require('@librechat/agents'); -const { EModelEndpoint, ResourceType, PermissionBits } = require('librechat-data-provider'); +const { + EModelEndpoint, + ResourceType, + PermissionBits, + hasPermissions, +} = require('librechat-data-provider'); const { createRun, buildToolSet, @@ -12,6 +17,8 @@ const { recordCollectedUsage, getTransactionsConfig, createToolExecuteHandler, + discoverConnectedAgents, + getRemoteAgentPermissions, // Responses API writeDone, buildResponse, @@ -38,7 +45,12 @@ const { agentLogHandlerObj, } = require('~/server/controllers/agents/callbacks'); const { loadAgentTools, loadToolsForExecution } = require('~/server/services/ToolService'); -const { findAccessibleResources } = require('~/server/services/PermissionService'); +const { + findAccessibleResources, + getEffectivePermissions, +} = require('~/server/services/PermissionService'); +const { getModelsConfig } = require('~/server/controllers/ModelController'); +const { logViolation } = require('~/cache'); const db = require('~/models'); /** @type {import('@librechat/api').AppConfig | null} */ @@ -345,6 +357,24 @@ const createResponse = async (req, res) => { model_parameters: agent.model_parameters ?? {}, }; + // `filterFilesByAgentAccess` is intentionally omitted: it calls + // `checkPermission` with `resourceType: AGENT`, but this route + // authorizes callers through `REMOTE_AGENT` (via + // `getRemoteAgentPermissions`), so including it would silently drop + // owner-attached context files for any remote user who has + // `REMOTE_AGENT_VIEWER` but not direct `AGENT_VIEW`. + const dbMethods = { + getConvoFiles: db.getConvoFiles, + getFiles: db.getFiles, + getUserKey: db.getUserKey, + getMessages: db.getMessages, + updateFilesUsage: db.updateFilesUsage, + getUserKeyValues: db.getUserKeyValues, + getUserCodeFiles: db.getUserCodeFiles, + getToolFilesByIds: db.getToolFilesByIds, + getCodeGeneratedFiles: db.getCodeGeneratedFiles, + }; + const primaryConfig = await initializeAgent( { req, @@ -358,19 +388,94 @@ const createResponse = async (req, res) => { allowedProviders, isInitialAgent: true, }, - { - getConvoFiles: db.getConvoFiles, - getFiles: db.getFiles, - getUserKey: db.getUserKey, - getMessages: db.getMessages, - updateFilesUsage: db.updateFilesUsage, - getUserKeyValues: db.getUserKeyValues, - getUserCodeFiles: db.getUserCodeFiles, - getToolFilesByIds: db.getToolFilesByIds, - getCodeGeneratedFiles: db.getCodeGeneratedFiles, - }, + dbMethods, ); + /** + * Per-agent tool-execution context map, keyed by agentId. Ensures the + * ON_TOOL_EXECUTE callback routes each sub-agent's tool calls to the + * correct toolRegistry / userMCPAuthMap / tool_resources. + * @type {Map>, + * tool_resources?: object, + * actionsEnabled?: boolean, + * }>} + */ + const agentToolContexts = new Map(); + agentToolContexts.set(primaryConfig.id, { + agent, + toolRegistry: primaryConfig.toolRegistry, + userMCPAuthMap: primaryConfig.userMCPAuthMap, + tool_resources: primaryConfig.tool_resources, + actionsEnabled: primaryConfig.actionsEnabled, + }); + + // Only run BFS discovery (and pay `getModelsConfig` upfront) when the + // primary has edges to follow β€” the common API case is single-agent. + let handoffAgentConfigs = new Map(); + let discoveredEdges = []; + let discoveredMCPAuthMap; + if (primaryConfig.edges?.length) { + const modelsConfig = await getModelsConfig(req); + ({ + agentConfigs: handoffAgentConfigs, + edges: discoveredEdges, + userMCPAuthMap: discoveredMCPAuthMap, + } = await discoverConnectedAgents( + { + req, + res, + primaryConfig, + endpointOption, + allowedProviders, + modelsConfig, + loadTools, + requestFiles: [], + conversationId, + parentMessageId, + // The route enforces REMOTE_AGENT on the primary; every discovered + // sub-agent must clear the same sharing boundary, not the looser + // in-app AGENT one. + resourceType: ResourceType.REMOTE_AGENT, + }, + { + getAgent: db.getAgent, + // Use `getRemoteAgentPermissions` so sub-agent authorization + // matches what the route's `createCheckRemoteAgentAccess` + // middleware does for the primary: AGENT owners with the SHARE + // bit are treated as remotely authorized even without an + // explicit REMOTE_AGENT grant. + checkPermission: async ({ userId, role, resourceId, requiredPermission }) => { + const permissions = await getRemoteAgentPermissions( + { getEffectivePermissions }, + userId, + role, + resourceId, + ); + return hasPermissions(permissions, requiredPermission); + }, + logViolation, + db: dbMethods, + onAgentInitialized: (agentId, handoffAgent, config) => { + agentToolContexts.set(agentId, { + agent: handoffAgent, + toolRegistry: config.toolRegistry, + userMCPAuthMap: config.userMCPAuthMap, + tool_resources: config.tool_resources, + actionsEnabled: config.actionsEnabled, + }); + }, + initializeAgent, + }, + )); + } + + primaryConfig.edges = discoveredEdges; + const runAgents = [primaryConfig, ...handoffAgentConfigs.values()]; + const mergedMCPAuthMap = discoveredMCPAuthMap ?? primaryConfig.userMCPAuthMap; + // Determine if streaming is enabled (check both request and agent config) const streamingDisabled = !!primaryConfig.model_parameters?.disableStreaming; const actuallyStreaming = isStreaming && !streamingDisabled; @@ -436,17 +541,19 @@ const createResponse = async (req, res) => { // Create tool execute options for event-driven tool execution const toolExecuteOptions = { - loadTools: async (toolNames) => { + loadTools: async (toolNames, agentId) => { + const ctx = + agentToolContexts.get(agentId) ?? agentToolContexts.get(primaryConfig.id) ?? {}; return loadToolsForExecution({ req, res, - agent, toolNames, + agent: ctx.agent ?? agent, signal: abortController.signal, - toolRegistry: primaryConfig.toolRegistry, - userMCPAuthMap: primaryConfig.userMCPAuthMap, - tool_resources: primaryConfig.tool_resources, - actionsEnabled: primaryConfig.actionsEnabled, + toolRegistry: ctx.toolRegistry, + userMCPAuthMap: ctx.userMCPAuthMap, + tool_resources: ctx.tool_resources, + actionsEnabled: ctx.actionsEnabled, }); }, toolEndCallback, @@ -483,10 +590,10 @@ const createResponse = async (req, res) => { // Create and run the agent const userId = req.user?.id ?? 'api-user'; - const userMCPAuthMap = primaryConfig.userMCPAuthMap; + const userMCPAuthMap = mergedMCPAuthMap; const run = await createRun({ - agents: [primaryConfig], + agents: runAgents, messages: formattedMessages, indexTokenCountMap, initialSummary, @@ -601,17 +708,19 @@ const createResponse = async (req, res) => { const toolEndCallback = createToolEndCallback({ req, res, artifactPromises, streamId: null }); const toolExecuteOptions = { - loadTools: async (toolNames) => { + loadTools: async (toolNames, agentId) => { + const ctx = + agentToolContexts.get(agentId) ?? agentToolContexts.get(primaryConfig.id) ?? {}; return loadToolsForExecution({ req, res, - agent, toolNames, + agent: ctx.agent ?? agent, signal: abortController.signal, - toolRegistry: primaryConfig.toolRegistry, - userMCPAuthMap: primaryConfig.userMCPAuthMap, - tool_resources: primaryConfig.tool_resources, - actionsEnabled: primaryConfig.actionsEnabled, + toolRegistry: ctx.toolRegistry, + userMCPAuthMap: ctx.userMCPAuthMap, + tool_resources: ctx.tool_resources, + actionsEnabled: ctx.actionsEnabled, }); }, toolEndCallback, @@ -646,10 +755,10 @@ const createResponse = async (req, res) => { }; const userId = req.user?.id ?? 'api-user'; - const userMCPAuthMap = primaryConfig.userMCPAuthMap; + const userMCPAuthMap = mergedMCPAuthMap; const run = await createRun({ - agents: [primaryConfig], + agents: runAgents, messages: formattedMessages, indexTokenCountMap, initialSummary, diff --git a/api/server/services/Endpoints/agents/initialize.js b/api/server/services/Endpoints/agents/initialize.js index 69767e191c..549e9047b5 100644 --- a/api/server/services/Endpoints/agents/initialize.js +++ b/api/server/services/Endpoints/agents/initialize.js @@ -3,15 +3,11 @@ const { createContentAggregator } = require('@librechat/agents'); const { initializeAgent, validateAgentModel, - createEdgeCollector, - filterOrphanedEdges, GenerationJobManager, getCustomEndpointConfig, - createSequentialChainEdges, + discoverConnectedAgents, } = require('@librechat/api'); const { - ResourceType, - PermissionBits, EModelEndpoint, isAgentsEndpoint, getResponseSender, @@ -230,63 +226,29 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { actionsEnabled: primaryConfig.actionsEnabled, }); - const agent_ids = primaryConfig.agent_ids; - let userMCPAuthMap = primaryConfig.userMCPAuthMap; - - /** @type {Set} Track agents that failed to load (orphaned references) */ - const skippedAgentIds = new Set(); - - async function processAgent(agentId) { - const agent = await db.getAgent({ id: agentId }); - if (!agent) { - logger.warn( - `[processAgent] Handoff agent ${agentId} not found, skipping (orphaned reference)`, - ); - skippedAgentIds.add(agentId); - return null; - } - - const hasAccess = await checkPermission({ - userId: req.user.id, - role: req.user.role, - resourceType: ResourceType.AGENT, - resourceId: agent._id, - requiredPermission: PermissionBits.VIEW, - }); - - if (!hasAccess) { - logger.warn( - `[processAgent] User ${req.user.id} lacks VIEW access to handoff agent ${agentId}, skipping`, - ); - skippedAgentIds.add(agentId); - return null; - } - - const validationResult = await validateAgentModel({ + const { + agentConfigs: discoveredConfigs, + edges: discoveredEdges, + userMCPAuthMap: discoveredMCPAuthMap, + } = await discoverConnectedAgents( + { req, res, - agent, + primaryConfig, + agent_ids: primaryConfig.agent_ids, + endpointOption, + allowedProviders, modelsConfig, + loadTools, + requestFiles, + conversationId, + parentMessageId, + }, + { + getAgent: db.getAgent, + checkPermission, logViolation, - }); - - if (!validationResult.isValid) { - throw new Error(validationResult.error?.message); - } - - const config = await initializeAgent( - { - req, - res, - agent, - loadTools, - requestFiles, - conversationId, - parentMessageId, - endpointOption, - allowedProviders, - }, - { + db: { getFiles: db.getFiles, getUserKey: db.getUserKey, getMessages: db.getMessages, @@ -298,71 +260,38 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { getCodeGeneratedFiles: db.getCodeGeneratedFiles, filterFilesByAgentAccess, }, - ); - - if (userMCPAuthMap != null) { - Object.assign(userMCPAuthMap, config.userMCPAuthMap ?? {}); - } else { - userMCPAuthMap = config.userMCPAuthMap; - } - - /** Store handoff agent's tool context for ON_TOOL_EXECUTE callback */ - agentToolContexts.set(agentId, { - agent, - toolRegistry: config.toolRegistry, - userMCPAuthMap: config.userMCPAuthMap, - tool_resources: config.tool_resources, - actionsEnabled: config.actionsEnabled, - }); - - agentConfigs.set(agentId, config); - return agent; - } - - const checkAgentInit = (agentId) => agentId === primaryConfig.id || agentConfigs.has(agentId); - - // Graph topology discovery for recursive agent handoffs (BFS) - const { edgeMap, agentsToProcess, collectEdges } = createEdgeCollector( - checkAgentInit, - skippedAgentIds, + // The callback fires during BFS, before the helper prunes agents + // whose edges end up filtered. Don't populate `agentConfigs` here β€” + // `discoveredConfigs` (returned below) is the authoritative pruned + // set. The per-agent tool context map is OK to keep populated even + // for pruned ids: it's only read by closure in ON_TOOL_EXECUTE, + // stale entries are unreachable at runtime. + onAgentInitialized: (agentId, agent, config) => { + agentToolContexts.set(agentId, { + agent, + toolRegistry: config.toolRegistry, + userMCPAuthMap: config.userMCPAuthMap, + tool_resources: config.tool_resources, + actionsEnabled: config.actionsEnabled, + }); + }, + // Pass through the `@librechat/api` exports so that tests which + // `jest.mock('@librechat/api')` can override the initializer/validator. + initializeAgent, + validateAgentModel, + }, ); - // Seed with primary agent's edges - collectEdges(primaryConfig.edges); - - // BFS to load and merge all connected agents (enables transitive handoffs: A->B->C) - while (agentsToProcess.size > 0) { - const agentId = agentsToProcess.values().next().value; - agentsToProcess.delete(agentId); - try { - const agent = await processAgent(agentId); - if (agent?.edges?.length) { - collectEdges(agent.edges); - } - } catch (err) { - logger.error(`[initializeClient] Error processing agent ${agentId}:`, err); - skippedAgentIds.add(agentId); - } + // Copy the pruned discovery result into the outer map. Anything the + // helper dropped (skipped or unreachable after edge filtering) is + // intentionally absent. `processAddedConvo` below may still add more + // entries for parallel multi-convo execution. + for (const [agentId, config] of discoveredConfigs) { + agentConfigs.set(agentId, config); } - /** @deprecated Agent Chain */ - if (agent_ids?.length) { - for (const agentId of agent_ids) { - if (checkAgentInit(agentId)) { - continue; - } - try { - await processAgent(agentId); - } catch (err) { - logger.error(`[initializeClient] Error processing chain agent ${agentId}:`, err); - skippedAgentIds.add(agentId); - } - } - const chain = await createSequentialChainEdges([primaryConfig.id].concat(agent_ids), '{convo}'); - collectEdges(chain); - } - - let edges = Array.from(edgeMap.values()); + let userMCPAuthMap = discoveredMCPAuthMap; + let edges = discoveredEdges; /** Multi-Convo: Process addedConvo for parallel agent execution */ const { userMCPAuthMap: updatedMCPAuthMap } = await processAddedConvo({ @@ -399,15 +328,8 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { }); } - // Ensure edges is an array when we have multiple agents (multi-agent mode) - // MultiAgentGraph.categorizeEdges requires edges to be iterable - if (agentConfigs.size > 0 && !edges) { - edges = []; - } - - // Filter out edges referencing non-existent agents (orphaned references) - edges = filterOrphanedEdges(edges, skippedAgentIds); - + // `discoverConnectedAgents` always returns a concrete array, so no + // further normalization is needed before handing this to `createRun`. primaryConfig.edges = edges; let endpointConfig = appConfig.endpoints?.[primaryConfig.endpoint]; diff --git a/packages/api/src/agents/discovery.spec.ts b/packages/api/src/agents/discovery.spec.ts new file mode 100644 index 0000000000..5b86cd194e --- /dev/null +++ b/packages/api/src/agents/discovery.spec.ts @@ -0,0 +1,999 @@ +import { EModelEndpoint } from 'librechat-data-provider'; +import type { Agent, GraphEdge } from 'librechat-data-provider'; +import type { Response } from 'express'; +import type { ServerRequest } from '~/types'; +import type { InitializedAgent } from './initialize'; + +jest.mock('@librechat/data-schemas', () => ({ + logger: { + debug: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + }, +})); + +const mockInitializeAgent = jest.fn(); +jest.mock('./initialize', () => ({ + initializeAgent: (...args: unknown[]) => mockInitializeAgent(...args), +})); + +const mockValidateAgentModel = jest.fn(); +jest.mock('./validation', () => ({ + validateAgentModel: (...args: unknown[]) => mockValidateAgentModel(...args), +})); + +import { discoverConnectedAgents } from './discovery'; + +const makeReq = (userId = 'u1', role = 'USER'): ServerRequest => + ({ + user: { id: userId, role }, + }) as unknown as ServerRequest; + +const makeRes = (): Response => ({}) as unknown as Response; + +const makeAgent = (id: string, edges: GraphEdge[] = []): Agent => + ({ + id, + _id: `mongo-${id}`, + provider: 'openai', + model: 'gpt-4o', + edges, + }) as unknown as Agent; + +const makeConfig = (id: string, edges: GraphEdge[] = []): InitializedAgent => + ({ + id, + provider: 'openai', + model: 'gpt-4o', + edges, + tools: [], + }) as unknown as InitializedAgent; + +describe('discoverConnectedAgents', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockValidateAgentModel.mockResolvedValue({ isValid: true }); + mockInitializeAgent.mockImplementation(async ({ agent }: { agent: Agent }) => + makeConfig(agent.id), + ); + }); + + it('returns empty configs and no edges for a single agent with no edges', async () => { + const primaryConfig = makeConfig('A'); + const result = await discoverConnectedAgents( + { + req: makeReq(), + res: makeRes(), + primaryConfig, + allowedProviders: new Set(), + modelsConfig: { openai: ['gpt-4o'] }, + loadTools: jest.fn(), + }, + { + getAgent: jest.fn(), + checkPermission: jest.fn(), + logViolation: jest.fn(), + db: {} as never, + }, + ); + + expect(result.agentConfigs.size).toBe(0); + expect(result.edges).toEqual([]); + expect(result.skippedAgentIds.size).toBe(0); + }); + + it('discovers handoff targets via BFS across a multi-hop chain A -> B -> C', async () => { + const edgesAB: GraphEdge[] = [{ from: 'A', to: 'B', edgeType: 'handoff' }]; + const edgesBC: GraphEdge[] = [{ from: 'B', to: 'C', edgeType: 'handoff' }]; + const primaryConfig = makeConfig('A', edgesAB); + + const agents: Record = { + B: makeAgent('B', edgesBC), + C: makeAgent('C', []), + }; + const getAgent = jest.fn(async ({ id }: { id: string }) => agents[id] ?? null); + const checkPermission = jest.fn().mockResolvedValue(true); + + const onAgentInitialized = jest.fn(); + + 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, + onAgentInitialized, + }, + ); + + expect(getAgent).toHaveBeenCalledWith({ id: 'B' }); + expect(getAgent).toHaveBeenCalledWith({ id: 'C' }); + expect(result.agentConfigs.size).toBe(2); + expect(result.agentConfigs.has('B')).toBe(true); + expect(result.agentConfigs.has('C')).toBe(true); + expect(result.edges).toHaveLength(2); + expect(onAgentInitialized).toHaveBeenCalledTimes(2); + expect(onAgentInitialized.mock.calls.map((c) => c[0])).toEqual(['B', 'C']); + }); + + it('skips orphaned agents and filters out edges pointing at them', async () => { + const edges: GraphEdge[] = [ + { from: 'A', to: 'B', edgeType: 'handoff' }, + { from: 'A', to: 'MISSING', edgeType: 'handoff' }, + ]; + const primaryConfig = makeConfig('A', edges); + + const getAgent = jest.fn(async ({ id }: { id: string }) => + id === 'B' ? makeAgent('B', []) : null, + ); + const checkPermission = jest.fn().mockResolvedValue(true); + const onAgentSkipped = jest.fn(); + + 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, + onAgentSkipped, + }, + ); + + expect(result.agentConfigs.has('B')).toBe(true); + expect(result.agentConfigs.has('MISSING')).toBe(false); + expect(result.skippedAgentIds.has('MISSING')).toBe(true); + expect(result.edges).toHaveLength(1); + expect(result.edges[0].to).toBe('B'); + expect(onAgentSkipped).toHaveBeenCalledWith('MISSING'); + }); + + it('skips sub-agents the user lacks VIEW permission on and filters their edges', async () => { + const edges: GraphEdge[] = [ + { from: 'A', to: 'B', edgeType: 'handoff' }, + { from: 'A', to: 'FORBIDDEN', edgeType: 'handoff' }, + ]; + const primaryConfig = makeConfig('A', edges); + + const agentMap: Record = { + B: makeAgent('B', []), + FORBIDDEN: makeAgent('FORBIDDEN', []), + }; + const getAgent = jest.fn(async ({ id }: { id: string }) => agentMap[id] ?? null); + const checkPermission = jest.fn( + async ({ resourceId }: { resourceId: unknown }) => resourceId !== 'mongo-FORBIDDEN', + ); + + 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('FORBIDDEN')).toBe(false); + expect(result.skippedAgentIds.has('FORBIDDEN')).toBe(true); + expect(result.edges.map((e) => e.to)).toEqual(['B']); + }); + + it('forces `endpoint: agents` on sub-agent init so allowedProviders is always enforced', async () => { + const primaryConfig = makeConfig('A', [{ from: 'A', to: 'B', edgeType: 'handoff' }]); + const getAgent = jest.fn(async () => makeAgent('B', [])); + const checkPermission = jest.fn().mockResolvedValue(true); + + await discoverConnectedAgents( + { + req: makeReq(), + res: makeRes(), + primaryConfig, + // Caller passes a non-agents endpoint (e.g. the OpenAI-compat + // controllers do this β€” endpoint = primary provider) + endpointOption: { endpoint: EModelEndpoint.openAI, model_parameters: {} }, + allowedProviders: new Set(['openai']), + modelsConfig: { openai: ['gpt-4o'] }, + loadTools: jest.fn(), + }, + { + getAgent, + checkPermission, + logViolation: jest.fn(), + db: {} as never, + }, + ); + + // The helper must override `endpoint` to 'agents' so + // `isAgentsEndpoint` is true and the allowedProviders guard in + // `initializeAgent` actually runs for sub-agents. + expect(mockInitializeAgent).toHaveBeenCalled(); + const initArgs = mockInitializeAgent.mock.calls[0][0]; + expect(initArgs.endpointOption.endpoint).toBe(EModelEndpoint.agents); + }); + + it('passes the configured resourceType (e.g. REMOTE_AGENT) to checkPermission', async () => { + const primaryConfig = makeConfig('A', [{ from: 'A', to: 'B', edgeType: 'handoff' }]); + const getAgent = jest.fn(async () => makeAgent('B', [])); + const checkPermission = jest.fn().mockResolvedValue(true); + + await discoverConnectedAgents( + { + req: makeReq(), + res: makeRes(), + primaryConfig, + allowedProviders: new Set(), + modelsConfig: { openai: ['gpt-4o'] }, + loadTools: jest.fn(), + resourceType: 'REMOTE_AGENT', + }, + { + getAgent, + checkPermission, + logViolation: jest.fn(), + db: {} as never, + }, + ); + + expect(checkPermission).toHaveBeenCalledWith( + expect.objectContaining({ resourceType: 'REMOTE_AGENT' }), + ); + }); + + it('prunes sub-agents disconnected from the primary after edge filtering', async () => { + // Graph: primary A has edges [A->B, B->C] stored directly on A. + // BFS loads B and C before permission is checked. B is skipped, and + // both edges reference B so they are filtered out. C was initialized + // but is now unreachable from A through any surviving edge β€” it must + // NOT be returned (otherwise it becomes a stray start node when + // createRun sees `agents.length > 1`). + const edges: GraphEdge[] = [ + { from: 'A', to: 'B', edgeType: 'handoff' }, + { from: 'B', to: 'C', edgeType: 'handoff' }, + ]; + const primaryConfig = makeConfig('A', edges); + + const agentMap: Record = { + B: makeAgent('B', []), + C: makeAgent('C', []), + }; + const getAgent = jest.fn(async ({ id }: { id: string }) => agentMap[id] ?? null); + // B fails permission, C passes. + const checkPermission = jest.fn( + async ({ resourceId }: { resourceId: unknown }) => resourceId !== 'mongo-B', + ); + + 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); + // C is unreachable once B's edges are filtered β†’ must be pruned. + expect(result.agentConfigs.has('C')).toBe(false); + expect(result.agentConfigs.size).toBe(0); + }); + + it('preserves valid routes when one co-source of a multi-source edge is skipped', async () => { + // Primary A has edge `{ from: ['A','B'], to: 'C' }`, but B lacks + // VIEW permission. The SDK treats each source independently, so the + // `A -> C` route is still valid. Discovery must strip B from the + // edge's sources rather than dropping the whole edge. + const edges: GraphEdge[] = [{ from: ['A', 'B'], to: 'C', edgeType: 'handoff' }]; + const primaryConfig = makeConfig('A', edges); + + const agentMap: Record = { + B: makeAgent('B', []), + C: makeAgent('C', []), + }; + const getAgent = jest.fn(async ({ id }: { id: string }) => agentMap[id] ?? null); + // Only B is forbidden. + const checkPermission = jest.fn( + async ({ resourceId }: { resourceId: unknown }) => resourceId !== 'mongo-B', + ); + + 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.agentConfigs.has('C')).toBe(true); + expect(result.agentConfigs.has('B')).toBe(false); + expect(result.edges).toHaveLength(1); + expect(result.edges[0].from).toEqual(['A']); + expect(result.edges[0].to).toBe('C'); + }); + + it('advances through a multi-source edge on ANY reachable source (SDK OR semantics)', async () => { + // Primary A has a single edge `{from: ['A','B'], to: 'C'}`. B loads + // successfully but has no incoming path from A. The agents SDK adds + // one LangGraph edge per `from` source (see + // `agents/src/graphs/MultiAgentGraph.ts`), so `A -> C` is a real + // routing regardless of B's reachability. Discovery must preserve it: + // - C reachable via A + // - edge kept (A source reachable, C dest reachable) + // - B kept in agentConfigs because it's still referenced by the + // surviving edge; otherwise the SDK's `validateEdgeAgents` + // safety-net rejects the graph for a missing `from` endpoint. + const edges: GraphEdge[] = [{ from: ['A', 'B'], to: 'C', edgeType: 'handoff' }]; + const primaryConfig = makeConfig('A', edges); + + const agentMap: Record = { + 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, + }, + ); + + expect(result.agentConfigs.has('B')).toBe(true); + expect(result.agentConfigs.has('C')).toBe(true); + expect(result.edges).toHaveLength(1); + expect(result.edges[0].from).toEqual(['A', 'B']); + expect(result.edges[0].to).toBe('C'); + }); + + 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 = { + 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('strips unreachable co-sources from surviving multi-source edges (no stray parallel root)', async () => { + // Primary has `[A -> C, X -> B, [B,C] -> D]`. X is skipped, so + // `X -> B` is filtered and B loses its only upstream. The fan-in + // edge `[B,C] -> D` still has C as a reachable source, so it + // survives β€” but discovery must strip B from that edge's `from` + // list and prune B from `agentConfigs`. Leaving B behind would + // make it an incoming-less agent that `MultiAgentGraph.analyzeGraph` + // runs as an unintended parallel root. + const edges: GraphEdge[] = [ + { from: 'A', to: 'C', edgeType: 'handoff' }, + { from: 'X', to: 'B', edgeType: 'handoff' }, + { from: ['B', 'C'], to: 'D', edgeType: 'direct' }, + ]; + const primaryConfig = makeConfig('A', edges); + + const agentMap: Record = { + B: makeAgent('B', []), + C: makeAgent('C', []), + D: makeAgent('D', []), + X: makeAgent('X', []), + }; + const getAgent = jest.fn(async ({ id }: { id: string }) => agentMap[id] ?? null); + // X is forbidden; B, C, D pass. + const checkPermission = jest.fn( + async ({ resourceId }: { resourceId: unknown }) => resourceId !== 'mongo-X', + ); + + 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('X')).toBe(true); + expect(result.agentConfigs.has('B')).toBe(false); + expect(result.agentConfigs.has('C')).toBe(true); + expect(result.agentConfigs.has('D')).toBe(true); + + // Every returned edge must reference only reachable agents β€” no B + // anywhere, not in a co-source slot and not as a node the SDK would + // need to register. + const endpoints = result.edges.flatMap((edge) => + [edge.from, edge.to].flatMap((v) => (Array.isArray(v) ? v : [v])), + ); + expect(endpoints).not.toContain('B'); + // The fan-in edge survived (C is a reachable co-source) but now + // lists only C as the source. + const fanInEdge = result.edges.find((edge) => { + const to = Array.isArray(edge.to) ? edge.to : [edge.to]; + return to.includes('D'); + }); + expect(fanInEdge).toBeDefined(); + expect(fanInEdge!.from).toEqual(['C']); + }); + + it('does not promote a downstream orphan to a parallel start when its only upstream is skipped', async () => { + // Primary A has `[A -> B, X -> Y]`. X is skipped (no VIEW), Y loads. + // Y had an incoming edge pre-filter (`X -> Y`), so it's a downstream + // agent β€” not a legitimate parallel starting node. When X is + // skipped, Y becomes a stranded orphan and must be pruned; the + // weaker "not pre-filter reachable from primary" rule would have + // incorrectly treated Y as an intentional parallel start. + const edges: GraphEdge[] = [ + { from: 'A', to: 'B', edgeType: 'handoff' }, + { from: 'X', to: 'Y', edgeType: 'direct' }, + ]; + const primaryConfig = makeConfig('A', edges); + + const agentMap: Record = { + B: makeAgent('B', []), + X: makeAgent('X', []), + Y: makeAgent('Y', []), + }; + const getAgent = jest.fn(async ({ id }: { id: string }) => agentMap[id] ?? null); + // X is forbidden β€” should be skipped. + const checkPermission = jest.fn( + async ({ resourceId }: { resourceId: unknown }) => resourceId !== 'mongo-X', + ); + + 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('X')).toBe(true); + expect(result.agentConfigs.has('B')).toBe(true); + expect(result.agentConfigs.has('Y')).toBe(false); + expect(result.edges).toHaveLength(1); + expect(result.edges[0].to).toBe('B'); + }); + + it('preserves user-defined parallel-start branches disconnected from the primary', async () => { + // Primary A has edges `[A -> B, X -> Y]`. The `X -> Y` branch is + // intentionally disconnected from A β€” `MultiAgentGraph.analyzeGraph` + // treats X as a starting node (no incoming edges) and runs it in + // parallel with A. Discovery must keep this branch intact. + const edges: GraphEdge[] = [ + { from: 'A', to: 'B', edgeType: 'handoff' }, + { from: 'X', to: 'Y', edgeType: 'direct' }, + ]; + const primaryConfig = makeConfig('A', edges); + + const agentMap: Record = { + B: makeAgent('B', []), + X: makeAgent('X', []), + Y: makeAgent('Y', []), + }; + 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('X')).toBe(true); + expect(result.agentConfigs.has('Y')).toBe(true); + expect(result.edges).toHaveLength(2); + expect(result.edges.map((e) => e.to).sort()).toEqual(['B', 'Y']); + }); + + 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, + // `filterOrphanedEdges` removes A->B (to=B) and B->C (from=B) β€” but + // `C->D` has no skipped endpoint and would otherwise survive, + // referencing agents that the reachability pass then prunes from + // `agentConfigs`. That combination is the exact shape that re-raises + // the "Found edge ending at unknown node" crash in `createRun`. + const edges: GraphEdge[] = [ + { from: 'A', to: 'B', edgeType: 'handoff' }, + { from: 'B', to: 'C', edgeType: 'handoff' }, + { from: 'C', to: 'D', edgeType: 'handoff' }, + ]; + const primaryConfig = makeConfig('A', edges); + + const agentMap: Record = { + B: makeAgent('B', []), + C: makeAgent('C', []), + D: makeAgent('D', []), + }; + const getAgent = jest.fn(async ({ id }: { id: string }) => agentMap[id] ?? null); + const checkPermission = jest.fn( + async ({ resourceId }: { resourceId: unknown }) => resourceId !== 'mongo-B', + ); + + 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.agentConfigs.has('C')).toBe(false); + expect(result.agentConfigs.has('D')).toBe(false); + // Every returned edge must reference only agents present in + // `agentConfigs` or the primary β€” no stranded `C->D` survives. + for (const edge of result.edges) { + const endpoints = [edge.from, edge.to].flatMap((v) => (Array.isArray(v) ? v : [v])); + for (const id of endpoints) { + expect(id === primaryConfig.id || result.agentConfigs.has(id)).toBe(true); + } + } + expect(result.edges).toHaveLength(0); + }); + + it('does not mutate a sub-agent userMCPAuthMap when merging later sub-agents', async () => { + // Primary has no MCP auth; first sub-agent (B) seeds `userMCPAuthMap`, + // and the second sub-agent (C) merges into it. Make sure B's own + // `config.userMCPAuthMap` object is not mutated by C's merge. + const bAuth = { serverB: { token: 'b' } }; + const cAuth = { serverC: { token: 'c' } }; + const primaryConfig = makeConfig('A', [ + { from: 'A', to: 'B', edgeType: 'handoff' }, + { from: 'A', to: 'C', edgeType: 'handoff' }, + ]); + primaryConfig.userMCPAuthMap = undefined; + + const agentMap: Record = { + B: makeAgent('B', []), + C: makeAgent('C', []), + }; + const authById: Record>> = { + B: bAuth, + C: cAuth, + }; + + const getAgent = jest.fn(async ({ id }: { id: string }) => agentMap[id] ?? null); + const checkPermission = jest.fn().mockResolvedValue(true); + mockInitializeAgent.mockImplementation(async ({ agent }: { agent: Agent }) => { + const cfg = makeConfig(agent.id); + cfg.userMCPAuthMap = authById[agent.id]; + return cfg; + }); + + 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, + }, + ); + + // B's own map must not have picked up C's entry. + expect(bAuth).toEqual({ serverB: { token: 'b' } }); + expect(cAuth).toEqual({ serverC: { token: 'c' } }); + // The returned merged map should still contain both. + expect(result.userMCPAuthMap).toEqual({ + serverB: { token: 'b' }, + serverC: { token: 'c' }, + }); + }); + + it('does not mutate the caller-supplied primaryConfig.userMCPAuthMap', async () => { + const primaryAuth = { serverA: { token: 'primary' } }; + const primaryConfig = makeConfig('A', [{ from: 'A', to: 'B', edgeType: 'handoff' }]); + primaryConfig.userMCPAuthMap = primaryAuth; + + const getAgent = jest.fn(async () => makeAgent('B', [])); + const checkPermission = jest.fn().mockResolvedValue(true); + + mockInitializeAgent.mockImplementation(async ({ agent }: { agent: Agent }) => { + const cfg = makeConfig(agent.id); + cfg.userMCPAuthMap = { serverB: { token: 'secondary' } }; + return cfg; + }); + + 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, + }, + ); + + // Caller's map is unchanged β€” only the returned merged map has the + // sub-agent entries. + expect(primaryConfig.userMCPAuthMap).toBe(primaryAuth); + expect(primaryConfig.userMCPAuthMap).toEqual({ serverA: { token: 'primary' } }); + expect(result.userMCPAuthMap).toEqual({ + serverA: { token: 'primary' }, + serverB: { token: 'secondary' }, + }); + expect(result.userMCPAuthMap).not.toBe(primaryAuth); + }); + + it('keeps sub-agents that remain reachable via surviving edges', async () => { + // Graph: A -> [B, C]; B is skipped, C is kept. C must remain because + // A -> C survives filtering. + const edges: GraphEdge[] = [ + { from: 'A', to: 'B', edgeType: 'handoff' }, + { from: 'A', to: 'C', edgeType: 'handoff' }, + ]; + const primaryConfig = makeConfig('A', edges); + + const agentMap: Record = { + B: makeAgent('B', []), + C: makeAgent('C', []), + }; + const getAgent = jest.fn(async ({ id }: { id: string }) => agentMap[id] ?? null); + const checkPermission = jest.fn( + async ({ resourceId }: { resourceId: unknown }) => resourceId !== 'mongo-B', + ); + + 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('C')).toBe(true); + expect(result.agentConfigs.has('B')).toBe(false); + expect(result.edges).toHaveLength(1); + expect(result.edges[0].to).toBe('C'); + }); + + 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' }, + { from: 'B', to: 'A', edgeType: 'handoff' }, + { from: 'A', to: 'B', edgeType: 'handoff' }, + ]; + const primaryConfig = makeConfig('A', edges); + + const getAgent = jest.fn(async ({ id }: { id: string }) => + id === 'B' ? makeAgent('B', [{ from: 'B', to: 'A', edgeType: 'handoff' }]) : 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, + }, + ); + + // B should be fetched once; A is the primary and is never re-fetched + expect(getAgent).toHaveBeenCalledTimes(1); + expect(result.agentConfigs.size).toBe(1); + expect(result.agentConfigs.has('B')).toBe(true); + // Deduped edges: A->B and B->A (not the duplicate A->B) + expect(result.edges).toHaveLength(2); + }); + + it('processes legacy agent_ids chain and adds sequential chain edges', async () => { + const primaryConfig = makeConfig('A', []); + + const agentMap: Record = { + 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, + agent_ids: ['B', 'C'], + allowedProviders: new Set(), + modelsConfig: { openai: ['gpt-4o'] }, + loadTools: jest.fn(), + }, + { + getAgent, + checkPermission, + logViolation: jest.fn(), + db: {} as never, + }, + ); + + expect(result.agentConfigs.size).toBe(2); + expect(result.agentConfigs.has('B')).toBe(true); + expect(result.agentConfigs.has('C')).toBe(true); + // Sequential chain: A -> B -> C (2 direct edges) + expect(result.edges).toHaveLength(2); + expect(result.edges.every((e) => e.edgeType === 'direct')).toBe(true); + }); + + it('merges userMCPAuthMap across primary and sub-agents', async () => { + const primaryConfig = makeConfig('A', [{ from: 'A', to: 'B', edgeType: 'handoff' }]); + primaryConfig.userMCPAuthMap = { serverA: { token: 'primary' } }; + + const getAgent = jest.fn(async () => makeAgent('B', [])); + const checkPermission = jest.fn().mockResolvedValue(true); + + mockInitializeAgent.mockImplementation(async ({ agent }: { agent: Agent }) => { + const cfg = makeConfig(agent.id); + cfg.userMCPAuthMap = { serverB: { token: 'secondary' } }; + return cfg; + }); + + 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.userMCPAuthMap).toEqual({ + serverA: { token: 'primary' }, + serverB: { token: 'secondary' }, + }); + }); + + it('throws when a sub-agent model validation fails', async () => { + mockValidateAgentModel.mockResolvedValueOnce({ + isValid: false, + error: { message: 'bad model' }, + }); + + const primaryConfig = makeConfig('A', [{ from: 'A', to: 'B', edgeType: 'handoff' }]); + + const getAgent = jest.fn(async () => makeAgent('B', [])); + 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, + }, + ); + + // Validation error is caught inside the BFS and the agent is skipped + expect(result.skippedAgentIds.has('B')).toBe(true); + expect(result.agentConfigs.has('B')).toBe(false); + }); + + it('skips when request has no authenticated user', async () => { + const primaryConfig = makeConfig('A', [{ from: 'A', to: 'B', edgeType: 'handoff' }]); + + const getAgent = jest.fn(async () => makeAgent('B', [])); + const checkPermission = jest.fn(); + + const result = await discoverConnectedAgents( + { + req: { user: undefined } as unknown as ServerRequest, + res: makeRes(), + primaryConfig, + allowedProviders: new Set(), + modelsConfig: { openai: ['gpt-4o'] }, + loadTools: jest.fn(), + }, + { + getAgent, + checkPermission, + logViolation: jest.fn(), + db: {} as never, + }, + ); + + expect(checkPermission).not.toHaveBeenCalled(); + expect(result.skippedAgentIds.has('B')).toBe(true); + expect(result.edges).toHaveLength(0); + }); +}); diff --git a/packages/api/src/agents/discovery.ts b/packages/api/src/agents/discovery.ts new file mode 100644 index 0000000000..e773c7be50 --- /dev/null +++ b/packages/api/src/agents/discovery.ts @@ -0,0 +1,450 @@ +import { logger } from '@librechat/data-schemas'; +import { ResourceType, PermissionBits, EModelEndpoint } from 'librechat-data-provider'; +import type { Agent, GraphEdge, TModelsConfig, TEndpointOption } from 'librechat-data-provider'; +import type { Response as ServerResponse } from 'express'; +import type { ServerRequest } from '~/types'; +import type { + InitializedAgent, + InitializeAgentParams, + InitializeAgentDbMethods, +} from './initialize'; +import type { ValidateAgentModelParams } from './validation'; +import { createEdgeCollector, filterOrphanedEdges } from './edges'; +import { createSequentialChainEdges } from './chain'; +import { validateAgentModel as defaultValidateAgentModel } from './validation'; +import { initializeAgent as defaultInitializeAgent } from './initialize'; + +/** + * Callback invoked after a sub-agent is successfully initialized. + * Used by callers that need to track per-agent tool context (e.g., for + * the ON_TOOL_EXECUTE event handler closure). + */ +export type OnAgentInitializedCallback = ( + agentId: string, + agent: Agent, + config: InitializedAgent, +) => void; + +/** + * Minimal permission check signature used to verify VIEW access on a + * candidate sub-agent before it is loaded into the run. + */ +export type CheckAgentPermission = (params: { + userId: string; + role?: string; + resourceType: string; + resourceId: unknown; + requiredPermission: number; +}) => Promise; + +export interface DiscoverConnectedAgentsParams { + req: ServerRequest; + res: ServerResponse; + /** The already-initialized primary agent config (starting point for BFS). */ + primaryConfig: InitializedAgent; + /** + * Optional legacy chain: agent IDs to append as sequential direct edges. + * Used by the deprecated Agent Chain feature. + */ + agent_ids?: string[]; + endpointOption?: Partial; + allowedProviders: Set; + modelsConfig: TModelsConfig; + loadTools: InitializeAgentParams['loadTools']; + requestFiles?: InitializeAgentParams['requestFiles']; + conversationId?: string | null; + parentMessageId?: string | null; + /** + * ResourceType to check each sub-agent's access against. Defaults to + * `AGENT` for the in-app chat flow. Callers whose entry-point gates on + * a different resource type (e.g. the OpenAI-compat controllers gate on + * `REMOTE_AGENT`) must pass the matching resource type so sub-agents + * don't bypass the same sharing boundary enforced at the route. + */ + resourceType?: string; +} + +export interface DiscoverConnectedAgentsDeps { + /** Fetch an agent by string id from the database. */ + getAgent: (filter: { id: string }) => Promise; + /** Permission check (typically a wrapper around PermissionService.checkPermission). */ + checkPermission: CheckAgentPermission; + /** Violation logger passed through to validateAgentModel. */ + logViolation: ValidateAgentModelParams['logViolation']; + /** DB methods consumed by initializeAgent for each sub-agent. */ + db: InitializeAgentDbMethods; + /** Optional callback invoked after each sub-agent is initialized. */ + onAgentInitialized?: OnAgentInitializedCallback; + /** Optional callback invoked when an agent id is skipped (missing or no access). */ + onAgentSkipped?: (agentId: string) => void; + /** + * Optional override for `initializeAgent`. Exists primarily so JS callers + * can inject their test doubles via `jest.mock('@librechat/api')` β€” since + * this module's own direct import would otherwise bypass that mock. + */ + initializeAgent?: typeof defaultInitializeAgent; + /** Optional override for `validateAgentModel` (same DI rationale). */ + validateAgentModel?: typeof defaultValidateAgentModel; +} + +export interface DiscoverConnectedAgentsResult { + /** Map of agentId -> initialized config for every discovered sub-agent. */ + agentConfigs: Map; + /** Deduplicated, orphan-filtered edges across the primary and sub-agents. */ + edges: GraphEdge[]; + /** Agent ids that were requested but could not be loaded (missing or no access). */ + skippedAgentIds: Set; + /** Merged MCP auth map from the primary and all sub-agents. */ + userMCPAuthMap?: Record>; +} + +/** + * Discovers and initializes all agents reachable from `primaryConfig.edges` + * via BFS. This is the shared graph-topology discovery logic that enables + * multi-agent handoffs (A -> B -> C) in both the primary chat flow and the + * OpenAI-compatible / Responses API controllers. + * + * Skips agents the caller cannot load (missing from DB or lacking VIEW + * permission) and filters out orphaned edges so `createRun` never sees an + * edge pointing at a missing node β€” which would otherwise trigger a + * `Found edge ending at unknown node` validation error from StateGraph. + */ +export async function discoverConnectedAgents( + params: DiscoverConnectedAgentsParams, + deps: DiscoverConnectedAgentsDeps, +): Promise { + const { + req, + res, + primaryConfig, + agent_ids, + endpointOption, + allowedProviders, + modelsConfig, + loadTools, + requestFiles, + conversationId, + parentMessageId, + resourceType = ResourceType.AGENT, + } = params; + + const { + getAgent, + checkPermission, + logViolation, + db, + onAgentInitialized, + onAgentSkipped, + initializeAgent = defaultInitializeAgent, + validateAgentModel = defaultValidateAgentModel, + } = deps; + + const agentConfigs = new Map(); + const skippedAgentIds = new Set(); + // Shallow-clone so the sub-agent merges below don't silently mutate + // `primaryConfig.userMCPAuthMap` on the caller's object. + let userMCPAuthMap: Record> | undefined = + primaryConfig.userMCPAuthMap ? { ...primaryConfig.userMCPAuthMap } : undefined; + + const markSkipped = (agentId: string): void => { + skippedAgentIds.add(agentId); + onAgentSkipped?.(agentId); + }; + + const processAgent = async (agentId: string): Promise => { + const agent = await getAgent({ id: agentId }); + if (!agent) { + logger.warn( + `[discoverConnectedAgents] Handoff agent ${agentId} not found, skipping (orphaned reference)`, + ); + markSkipped(agentId); + return null; + } + + const userId = req.user?.id; + if (!userId) { + logger.warn( + `[discoverConnectedAgents] No authenticated user on request, skipping handoff agent ${agentId}`, + ); + markSkipped(agentId); + return null; + } + + const hasAccess = await checkPermission({ + userId, + role: req.user?.role, + resourceType, + resourceId: agent._id, + requiredPermission: PermissionBits.VIEW, + }); + + if (!hasAccess) { + logger.warn( + `[discoverConnectedAgents] User ${userId} lacks VIEW access to handoff agent ${agentId}, skipping`, + ); + markSkipped(agentId); + return null; + } + + const validation = await validateAgentModel({ + req, + res, + agent, + modelsConfig, + logViolation, + }); + + if (!validation.isValid) { + throw new Error(validation.error?.message); + } + + /** + * Force `endpoint: agents` on the per-sub-agent init call so + * `initializeAgent`'s `isAgentsEndpoint`-gated `allowedProviders` + * check always fires for handoff sub-agents, regardless of which + * endpoint the caller entered through. Without this, the OpenAI- + * compat routes (whose `endpointOption.endpoint` is the primary + * provider, not `agents`) would silently bypass the provider + * allowlist configured under `endpoints.agents.allowedProviders`. + */ + const subAgentEndpointOption: Partial = { + ...(endpointOption ?? {}), + endpoint: EModelEndpoint.agents, + }; + + const config = await initializeAgent( + { + req, + res, + agent, + loadTools, + requestFiles, + conversationId, + parentMessageId, + endpointOption: subAgentEndpointOption, + allowedProviders, + }, + db, + ); + + if (userMCPAuthMap != null) { + Object.assign(userMCPAuthMap, config.userMCPAuthMap ?? {}); + } else if (config.userMCPAuthMap) { + // Clone so subsequent sub-agent merges don't mutate the first + // sub-agent's own `config.userMCPAuthMap` in place β€” symmetric with + // the shallow clone applied to the primary's map above. + userMCPAuthMap = { ...config.userMCPAuthMap }; + } + + agentConfigs.set(agentId, config); + onAgentInitialized?.(agentId, agent, config); + return agent; + }; + + const checkAgentInit = (agentId: string): boolean => + agentId === primaryConfig.id || agentConfigs.has(agentId); + + const { edgeMap, agentsToProcess, collectEdges } = createEdgeCollector( + checkAgentInit, + skippedAgentIds, + ); + + collectEdges(primaryConfig.edges); + + while (agentsToProcess.size > 0) { + const agentId = agentsToProcess.values().next().value as string; + agentsToProcess.delete(agentId); + try { + const agent = await processAgent(agentId); + if (agent?.edges?.length) { + collectEdges(agent.edges); + } + } catch (err) { + logger.error(`[discoverConnectedAgents] Error processing agent ${agentId}:`, err); + markSkipped(agentId); + } + } + + /** @deprecated Agent Chain β€” sequential direct-edge fallback */ + if (agent_ids?.length) { + for (const agentId of agent_ids) { + if (checkAgentInit(agentId)) { + continue; + } + try { + await processAgent(agentId); + } catch (err) { + logger.error(`[discoverConnectedAgents] Error processing chain agent ${agentId}:`, err); + markSkipped(agentId); + } + } + /** + * `createSequentialChainEdges` is typed against `@librechat/agents`'s + * `GraphEdge` (which uses `BaseMessage` from `@langchain/core`) whereas + * `collectEdges` uses the `librechat-data-provider` variant (structural + * `BaseMessage`). The produced chain edges are structurally identical and + * only carry `edgeType`, `from`, `to`, `prompt`, `excludeResults` β€” + * interchangeable for edge collection purposes. + */ + const chain = await createSequentialChainEdges([primaryConfig.id].concat(agent_ids), '{convo}'); + collectEdges(chain as unknown as GraphEdge[]); + } + + const preFilterEdges = Array.from(edgeMap.values()); + const filteredEdges = filterOrphanedEdges(preFilterEdges, skippedAgentIds); + + /** + * Keep discovery's reachability model aligned with the agents SDK's + * runtime semantics. `MultiAgentGraph.createWorkflow` adds one + * LangGraph edge per `from` source, so a multi-source edge + * `{ from: ['A', 'B'], to: 'C' }` is really `A -> C` OR `B -> C` β€” + * either source firing routes to `C`. Reachability therefore advances + * through an edge whenever ANY of its sources is already reachable. + * + * Two semantics to reconcile when pruning after orphan-filter: + * + * 1. Accidental orphans β€” agents loaded via BFS from the primary's + * edges that lost their only path when an intermediate agent was + * skipped (e.g. `A -> B -> C` with B skipped leaves C stranded). + * These should be pruned; leaving them flips `createRun` into + * multi-agent mode with a disconnected C and the SDK runs C as an + * unintended parallel root. + * + * 2. Intentional multi-start branches β€” agents referenced by edges the + * user explicitly defined without wiring them to the primary + * (e.g. `A -> B` plus `X -> Y` as two independent starting + * branches). The SDK's `MultiAgentGraph.analyzeGraph` treats + * `no-incoming-edge` agents as start nodes, so these run in + * parallel with the primary by design. These must be preserved. + * + * Distinguish the two by asking: did the agent have any incoming edge + * in the user's original (pre-filter) graph? If yes, it was wired as + * a downstream step, and losing that wiring post-filter makes it an + * accidental orphan β€” prune. If no, the user declared it a start + * node; seed it so the SDK's `analyzeGraph` behavior of running + * incoming-less agents in parallel is preserved. + * + * "No incoming edge pre-filter" is stricter than "not reachable from + * primary pre-filter": a downstream agent like Y in `X -> Y` where X + * is skipped was never reachable from the primary pre-filter either, + * but it's still an orphan (its upstream X would have routed to it). + * The incoming-edge test catches that case correctly. + * + * - Post-filter reachability is seeded with the primary AND every + * agent in `agentConfigs` that had no pre-filter incoming edge + * (legitimate parallel start). + * - Agents whose pre-filter incoming edges got filtered out lose + * reachability and get pruned. + * - Surviving edges are filtered to the post-filter reachable set + * so no stale edge references a pruned agent. + * - Agents referenced as an endpoint in a surviving edge are always + * kept (a multi-source edge co-source like B in + * `{ from: ['A','B'], to: 'C' }` where nothing reaches B still + * needs B present for the SDK's per-source `addEdge` to compile). + */ + const anyReachable = (value: string | string[], reachableSet: Set): boolean => { + const ids = Array.isArray(value) ? value : [value]; + return ids.some((id) => typeof id === 'string' && reachableSet.has(id)); + }; + const allReachable = (value: string | string[], reachableSet: Set): boolean => { + const ids = Array.isArray(value) ? value : [value]; + return ids.every((id) => typeof id !== 'string' || reachableSet.has(id)); + }; + const expandReachable = (seeds: Set, edgeList: GraphEdge[]): Set => { + const result = new Set(seeds); + let changed = true; + while (changed) { + changed = false; + for (const edge of edgeList) { + if (!anyReachable(edge.from, result)) { + continue; + } + const dests = Array.isArray(edge.to) ? edge.to : [edge.to]; + for (const dest of dests) { + if (typeof dest === 'string' && !result.has(dest)) { + result.add(dest); + changed = true; + } + } + } + } + return result; + }; + + // A legitimate parallel-start agent is one that has NO incoming edge + // in the pre-filter graph β€” the user declared it as a starting node. + // "Not reachable from primary pre-filter" is too permissive: a + // downstream agent whose only upstream got skipped (`X -> Y` with X + // skipped but Y loaded) would qualify under that weaker rule and be + // promoted to a parallel root even though it's actually a stranded + // orphan. Using "no incoming edge in pre-filter" tightens the criterion + // to match the SDK's `analyzeGraph` definition of a start node applied + // to the user's ORIGINAL graph topology, before any orphan filtering. + const hadIncomingEdgePreFilter = new Set(); + for (const edge of preFilterEdges) { + const dests = Array.isArray(edge.to) ? edge.to : [edge.to]; + for (const dest of dests) { + if (typeof dest === 'string') { + hadIncomingEdgePreFilter.add(dest); + } + } + } + + const postFilterSeeds = new Set([primaryConfig.id]); + for (const agentId of agentConfigs.keys()) { + if (!hadIncomingEdgePreFilter.has(agentId)) { + postFilterSeeds.add(agentId); + } + } + + const reachable = expandReachable(postFilterSeeds, filteredEdges); + + /** + * Filter + sanitize edges: + * - Keep an edge if at least one `from` source is reachable AND every + * `to` destination is reachable (a missing destination would still + * crash `StateGraph.compile` with `Found edge ending at unknown + * node`). + * - For kept edges with an array `from`, strip out unreachable + * co-sources. The SDK's per-source `addEdge` fires independently + * (each source becomes its own `addEdge(source, dest)` call), so + * losing an unreachable co-source doesn't invalidate the routes + * through the surviving ones. Leaving the dead co-source in the + * array was propping up agents that `reachable` had already + * excluded β€” in `MultiAgentGraph.analyzeGraph` they'd then show up + * as incoming-less nodes and execute as unintended parallel roots. + * + * After sanitization every endpoint in every surviving edge is + * guaranteed to be in `reachable`, which lets the agent prune below + * collapse to a strict reachability check. + */ + const edges: GraphEdge[] = []; + for (const edge of filteredEdges) { + if (!anyReachable(edge.from, reachable) || !allReachable(edge.to, reachable)) { + continue; + } + if (!Array.isArray(edge.from)) { + edges.push(edge); + continue; + } + const reachableSources = edge.from.filter((s) => typeof s !== 'string' || reachable.has(s)); + if (reachableSources.length === edge.from.length) { + edges.push(edge); + } else { + edges.push({ ...edge, from: reachableSources }); + } + } + + for (const agentId of [...agentConfigs.keys()]) { + if (!reachable.has(agentId)) { + agentConfigs.delete(agentId); + } + } + + return { + agentConfigs, + edges, + skippedAgentIds, + userMCPAuthMap, + }; +} diff --git a/packages/api/src/agents/edges.spec.ts b/packages/api/src/agents/edges.spec.ts index b23f00f63f..447c65fdb5 100644 --- a/packages/api/src/agents/edges.spec.ts +++ b/packages/api/src/agents/edges.spec.ts @@ -131,36 +131,66 @@ 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) - expect(result).toHaveLength(1); - expect(result[0].to).toBe('agent_d'); + // agent_a -> agent_b (filtered, to) + // agent_a -> agent_c (filtered, to) + // agent_b -> agent_d (filtered, from) + expect(result).toHaveLength(0); }); - it('should handle array to values', () => { + it('strips skipped co-sources from multi-source edges instead of dropping the whole edge', () => { + // The SDK adds one `builder.addEdge(source, dest)` per source, so + // `agent_a -> agent_d` is a valid route even when `agent_b` can't + // be loaded. Keep the edge with the surviving sources. + 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(2); + expect(result[0].from).toEqual(['agent_a']); + expect(result[0].to).toBe('agent_d'); + expect(result[1].from).toBe('agent_a'); + }); + + it('strips skipped co-destinations from multi-destination edges', () => { const edgesWithArray: GraphEdge[] = [ { from: 'agent_a', to: ['agent_b', 'agent_c'], edgeType: 'handoff' }, { from: 'agent_a', to: ['agent_d'], edgeType: 'handoff' }, ]; const skipped = new Set(['agent_b']); const result = filterOrphanedEdges(edgesWithArray, skipped); - expect(result).toHaveLength(1); - expect(result[0].to).toEqual(['agent_d']); + // First edge: `['agent_b','agent_c']` β†’ `['agent_c']` β€” kept. + // Second edge: unchanged. + expect(result).toHaveLength(2); + expect(result[0].to).toEqual(['agent_c']); + expect(result[1].to).toEqual(['agent_d']); + }); + + it('drops multi-member edges only when every member on a side is skipped', () => { + const edgesSide: GraphEdge[] = [ + // All sources skipped β†’ drop. + { from: ['agent_b', 'agent_c'], to: 'agent_a', edgeType: 'handoff' }, + // All destinations skipped β†’ drop. + { from: 'agent_a', to: ['agent_b', 'agent_c'], edgeType: 'handoff' }, + ]; + const skipped = new Set(['agent_b', 'agent_c']); + const result = filterOrphanedEdges(edgesSide, skipped); + expect(result).toHaveLength(0); }); it('should return original edges array when edges is null/undefined', () => { diff --git a/packages/api/src/agents/edges.ts b/packages/api/src/agents/edges.ts index 9a36105b74..9bc3705c60 100644 --- a/packages/api/src/agents/edges.ts +++ b/packages/api/src/agents/edges.ts @@ -30,17 +30,80 @@ 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. + * Drops skipped agent ids from an edge endpoint. + * + * Scalars: returns the value unchanged, or `null` if the skipped set + * contains it (the edge has lost its only endpoint on that side and + * must be dropped by the caller). + * + * Arrays: returns a new array with skipped ids removed; if every id was + * skipped, returns `null`. A single-element result stays an array β€” the + * SDK normalizes both shapes internally, so preserving array-ness is + * simpler than switching representation. + * + * Returns the original reference when nothing changed, so callers can + * cheaply detect and skip a re-spread. + */ +function sanitizeEndpoint( + value: string | string[], + skippedAgentIds: Set, +): string | string[] | null { + if (Array.isArray(value)) { + const kept: string[] = []; + let removed = false; + for (const id of value) { + if (typeof id === 'string' && skippedAgentIds.has(id)) { + removed = true; + continue; + } + kept.push(id); + } + if (kept.length === 0) { + return null; + } + return removed ? kept : value; + } + if (typeof value === 'string' && skippedAgentIds.has(value)) { + return null; + } + return value; +} + +/** + * Drops orphaned agents from edge endpoints, returning only the edges + * that still have at least one valid source and one valid destination. + * + * For multi-source / multi-destination edges, orphaned ids are stripped + * from the array rather than dropping the whole edge: `{ from: ['A','B'], + * to: 'C' }` with B skipped becomes `{ from: ['A'], to: 'C' }`. This + * matches the agents SDK's `MultiAgentGraph.createWorkflow`, which adds + * one LangGraph edge per source/destination pair (`builder.addEdge(src, + * dest)`), so losing one co-source (or one co-destination) doesn't + * invalidate the routes through the remaining members. + * + * An edge is dropped only when it has no surviving source OR no + * surviving destination β€” the SDK's per-source `addEdge` can't run if + * either side is empty, and any orphaned id left in place would still + * trigger `Found edge ending at unknown node` at compile time. */ export function filterOrphanedEdges(edges: GraphEdge[], skippedAgentIds: Set): 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 result: GraphEdge[] = []; + for (const edge of edges) { + const from = sanitizeEndpoint(edge.from, skippedAgentIds); + const to = sanitizeEndpoint(edge.to, skippedAgentIds); + if (from === null || to === null) { + continue; + } + if (from === edge.from && to === edge.to) { + result.push(edge); + } else { + result.push({ ...edge, from, to }); + } + } + return result; } /** Collects all unique agent IDs referenced across an array of edges. */ diff --git a/packages/api/src/agents/index.ts b/packages/api/src/agents/index.ts index 53f7f60a93..336eaea479 100644 --- a/packages/api/src/agents/index.ts +++ b/packages/api/src/agents/index.ts @@ -3,6 +3,7 @@ export * from './chain'; export * from './client'; export * from './config'; export * from './context'; +export * from './discovery'; export * from './edges'; export * from './handlers'; export * from './initialize'; diff --git a/packages/api/src/agents/validation.ts b/packages/api/src/agents/validation.ts index ee37c26f44..a59153c597 100644 --- a/packages/api/src/agents/validation.ts +++ b/packages/api/src/agents/validation.ts @@ -3,6 +3,13 @@ import { ViolationTypes, ErrorTypes } from 'librechat-data-provider'; import type { Agent, TModelsConfig } from 'librechat-data-provider'; import type { Request, Response } from 'express'; +/** + * Permissive Request alias used by {@link validateAgentModel}. Accepts either + * the default Express `Request` or the project-specific `ServerRequest` + * (see `~/types/http`), whose `params` type is widened to `unknown`. + */ +type LooseRequest = Request; + /** Avatar schema shared between create and update */ export const agentAvatarSchema = z.object({ filepath: z.string(), @@ -96,13 +103,13 @@ export const agentUpdateSchema = agentBaseSchema.extend({ model: z.string().nullable().optional(), }); -interface ValidateAgentModelParams { - req: Request; +export interface ValidateAgentModelParams { + req: LooseRequest; res: Response; agent: Agent; modelsConfig: TModelsConfig; logViolation: ( - req: Request, + req: LooseRequest, res: Response, type: string, errorMessage: Record,