feat: Forward MCP tools to delegate sessions

[buger]

Summary

Subagents created via delegation now inherit MCP configuration from their parent, allowing them to access MCP-provided tools. This matches the pattern used for other tools like bash and allowedTools.

Details

• Added enableMcp, mcpConfig, and mcpConfigPath parameters to the delegate function signature
• Forward these parameters from delegateTool to the delegate() function
• Each subagent creates its own MCPXmlBridge instance, maintaining isolation while enabling full MCP functionality in delegates
• Updated tests to include new parameters in expectations

Test Results

All 113 delegate-related tests pass.

🤖 Generated with Claude Code

[probelabs]

PR Overview: Forward MCP Tools to Delegate Sessions

Summary

This PR enables Model Context Protocol (MCP) tool inheritance in delegated subagent sessions. When a parent agent creates a delegate subagent, the subagent now inherits the parent's MCP configuration, allowing it to access MCP-provided tools. This matches the existing pattern used for other tools like bash and allowedTools.

Files Changed

• npm/src/delegate.js (+11, -2): Added enableMcp, mcpConfig, and mcpConfigPath parameters to the delegate() function signature and forwards them to the subagent's ProbeAgent constructor
• npm/src/tools/vercel.js (+5, -2): Updated delegateTool to accept and forward the new MCP parameters
• npm/tests/delegate-config.test.js (+7, -1): Updated test expectations to include new MCP parameters
• npm/tests/delegate-integration.test.js (+7, -1): Updated integration test expectations to include new MCP parameters

Architecture & Impact Assessment

What This PR Accomplishes

Subagents created via delegation now inherit MCP configuration from their parent agent, enabling:

• Full MCP tool access in delegate sessions
• Isolated MCPXmlBridge instances per subagent (no shared state)
• Consistent configuration inheritance pattern across all tool types

Key Technical Changes

1. Parameter Forwarding Chain:

   • delegateTool in vercel.js now accepts enableMcp, mcpConfig, and mcpConfigPath options
   • These parameters flow through to the delegate() function
   • The delegate() function passes them to the subagent's ProbeAgent constructor

2. Isolation Model:

   • Each subagent creates its own MCPXmlBridge instance
   • MCP connections are not shared between parent and child agents
   • This prevents resource conflicts and maintains clean session boundaries

3. Configuration Priority (as implemented in ProbeAgent.initializeMCP):

   • Explicit mcpConfig object (highest priority)
   • mcpConfigPath pointing to configuration file
   • Auto-discovery from standard locations (if no explicit config)

Affected System Components

graph TD
    A[Parent Agent] -->|delegateTool call| B[delegate function]
    B -->|creates| C[Subagent ProbeAgent]
    A -->|forwards MCP config| B
    B -->|passes MCP params| C
    C -->|initializeMCP| D[MCPXmlBridge]
    D -->|connects to| E[MCP Servers]
    C -->|can now use| F[MCP Tools]
    
    style A fill:#e1f5ff
    style C fill:#fff4e1
    style D fill:#f0f0f0

Loading

Component Relationships

graph LR
    subgraph Parent[Parent Agent Context]
        P1[enableMcp flag]
        P2[mcpConfig object]
        P3[mcpConfigPath string]
    end
    
    subgraph Delegate[Delegation Layer]
        D1[delegateTool]
        D2[delegate function]
    end
    
    subgraph Subagent[Subagent Context]
        S1[ProbeAgent instance]
        S2[MCPXmlBridge instance]
        S3[MCP Tool Access]
    end
    
    Parent --> D1
    D1 --> D2
    D2 --> S1
    S1 --> S2
    S2 --> S3
    
    style Parent fill:#e3f2fd
    style Subagent fill:#f3e5f5

Loading

Scope Discovery & Context Expansion

Direct Impact

• Delegation System: Core delegation functionality now supports MCP
• MCP Integration: MCP tools are now available in all delegate sessions
• Test Suite: 113 delegate-related tests updated and passing

Related Components (Inferred)

Based on the codebase structure, these areas are related but not directly modified:

1. ProbeAgent Initialization (npm/src/agent/ProbeAgent.js):

   • initializeMCP() method handles MCP setup
   • Already supports the three parameters being forwarded
   • No changes needed - this PR leverages existing capability

2. MCP Configuration (npm/src/agent/mcp/config.js):

   • Handles loading MCP configs from files or environment
   • Supports multiple configuration sources
   • Already integrated with ProbeAgent

3. MCP XML Bridge (npm/src/agent/mcp/xmlBridge.js):

   • Manages MCP tool connections and XML translation
   • Each subagent gets its own instance (isolated)

4. Tool System (npm/src/agent/tools.js):

   • Creates delegateTool with config options
   • Passes options through to delegate function

Potential Follow-up Areas

• Documentation updates for MCP + delegation usage
• Examples showing MCP tools in delegate sessions
• Performance testing with multiple concurrent MCP-enabled delegates

Testing

All 113 delegate-related tests pass, including:

• Configuration parameter forwarding
• Integration tests with full delegate workflow
• Test expectations updated to verify new parameters are passed correctly

Labels

• Type: feature - Adds new capability (MCP in delegates)
• Review Effort: 2 - Low complexity, follows existing patterns, well-tested

Metadata

• Review Effort: 2 / 5
• Primary Label: feature

---

Powered by Visor from Probelabs

Last updated: 2026-02-02T12:29:12.169Z | Triggered by: pr_opened | Commit: 807185c

💡 TIP: You can chat with Visor using /visor ask <your question>

[probelabs]

Security Issues (8)

Severity	Location	Issue
🔴 Critical	npm/src/agent/mcp/config.js:267	parseEnabledServers spreads serverConfig without validation, allowing injection of arbitrary commands through the 'command' and 'args' properties. An attacker could inject malicious commands. 💡 Suggestion Validate and sanitize command and args properties before using them. Ensure they are strings and don't contain shell metacharacters if passed to shell. Consider using a whitelist of allowed commands. 🔧 Suggested Fix // In parseEnabledServers, add validation after spreading serverConfig: const server = { name, ...serverConfig }; // Validate command if present (stdio transport) if (server.command) { if (typeof server.command !== 'string') { console.error([MCP ERROR] Server ${name} command must be a string); continue; } // Check for shell injection attempts if (/[;&|$()]/.test(server.command)) { console.error([MCP ERROR] Server ${name} command contains shell metacharacters`); continue; } } // Validate args if present if (server.args) { if (!Array.isArray(server.args)) { console.error([MCP ERROR] Server ${name} args must be an array); continue; } for (const arg of server.args) { if (typeof arg !== 'string') { console.error([MCP ERROR] Server ${name} args must be strings); continue; } } }
🔴 Critical	npm/src/agent/mcp/config.js:195	mergeWithEnvironment accepts COMMAND and ARGS from environment variables without validation, allowing command injection through environment variables. 💡 Suggestion Validate and sanitize command and args from environment variables before using them. Check for shell metacharacters and ensure they are safe. 🔧 Suggested Fix // In mergeWithEnvironment, add validation for COMMAND and ARGS: case 'COMMAND': // Validate command for shell injection if (/[;&|`$()]/.test(value)) { console.error(`[MCP ERROR] Server ${normalizedName} command contains shell metacharacters, ignoring`); break; } config.mcpServers[normalizedName].command = value; break; case 'ARGS': // Validate args are safe strings const args = value.split(',').map(arg => arg.trim()); for (const arg of args) { if (/[;&|`$()]/.test(arg)) { console.error(`[MCP ERROR] Server ${normalizedName} args contain shell metacharacters, ignoring`); break; } } config.mcpServers[normalizedName].args = args; break;
🔴 Critical	npm/src/agent/mcp/client.js:58	createTransport passes command and args directly to StdioClientTransport without validation, allowing command injection if the config contains malicious values. 💡 Suggestion Add validation for command and args before creating the transport. Ensure they don't contain shell metacharacters. 🔧 Suggested Fix // In createTransport, add validation for stdio transport: function createTransport(serverConfig) { const { transport, command, args, url, env } = serverConfig; switch (transport) { case 'stdio': // Validate command for shell injection if (!command || typeof command !== 'string') { throw new Error('Stdio transport requires a valid command string'); } if (/[;&|$()]/.test(command)) { throw new Error('Command contains shell metacharacters'); } // Validate args if (args) { if (!Array.isArray(args)) { throw new Error('Args must be an array'); } for (const arg of args) { if (typeof arg !== 'string') { throw new Error('Args must be strings'); } if (/[;&|$()]/.test(arg)) { throw new Error('Args contain shell metacharacters'); } } } return new StdioClientTransport({ command, args: args || [], env: env ? { ...process.env, ...env } : undefined });
🟠 Error	npm/src/delegate.js:217	mcpConfigPath parameter is passed through without validation, allowing potential path traversal attacks. An attacker could provide paths like '../../../etc/passwd' to read arbitrary files. 💡 Suggestion Validate and sanitize mcpConfigPath before use. Reject paths containing '..' or ensure the path is within allowed directories. Consider using path.resolve() and checking if the normalized path is within expected boundaries. 🔧 Suggested Fix // Add validation at the start of delegate function: if (mcpConfigPath && typeof mcpConfigPath === 'string') { const normalizedPath = path.resolve(mcpConfigPath); // Reject paths with traversal attempts if (mcpConfigPath.includes('..') || mcpConfigPath.includes('~')) { throw new Error('Invalid mcpConfigPath: path traversal not allowed'); } mcpConfigPath = normalizedPath; }
🟠 Error	npm/src/delegate.js:217	mcpConfig object is passed through without validation, allowing potential prototype pollution or malicious configuration injection. An attacker could inject dangerous properties like 'command' to execute arbitrary commands. 💡 Suggestion Validate mcpConfig structure before passing to ProbeAgent. Ensure it conforms to expected schema and doesn't contain dangerous properties. Consider deep-cloning and sanitizing the object. 🔧 Suggested Fix // Add validation before creating ProbeAgent: if (mcpConfig && typeof mcpConfig === 'object') { // Validate structure if (!mcpConfig.mcpServers || typeof mcpConfig.mcpServers !== 'object') { throw new Error('Invalid mcpConfig: must contain mcpServers object'); } // Deep clone to prevent prototype pollution mcpConfig = JSON.parse(JSON.stringify(mcpConfig)); // Validate each server config for (const [name, server] of Object.entries(mcpConfig.mcpServers)) { if (server.command && typeof server.command !== 'string') { throw new Error(`Invalid mcpConfig: server ${name} has invalid command`); } } }
🟠 Error	npm/src/tools/vercel.js:472	mcpConfigPath parameter is accepted from delegate tool options without validation, allowing path traversal attacks through the delegate tool interface. 💡 Suggestion Add validation for mcpConfigPath in delegateTool options before passing to delegate function. Reject paths containing '..' or normalize and validate the path. 🔧 Suggested Fix // Add validation in delegateTool: const { debug = false, timeout = 300, cwd, allowedFolders, enableBash = false, bashConfig, architectureFileName, enableMcp = false, mcpConfig = null, mcpConfigPath = null } = options; // Validate mcpConfigPath if provided if (mcpConfigPath && typeof mcpConfigPath === 'string') { if (mcpConfigPath.includes('..') || mcpConfigPath.includes('~')) { throw new Error('Invalid mcpConfigPath: path traversal not allowed'); } }
🟠 Error	npm/src/tools/vercel.js:472	mcpConfig object is accepted from delegate tool options without validation, allowing potential prototype pollution or malicious configuration injection through the tool interface. 💡 Suggestion Validate mcpConfig structure in delegateTool options before use. Ensure it conforms to expected schema and doesn't contain dangerous properties. 🔧 Suggested Fix // Add validation in delegateTool after destructuring: if (mcpConfig && typeof mcpConfig === 'object') { if (!mcpConfig.mcpServers || typeof mcpConfig.mcpServers !== 'object') { throw new Error('Invalid mcpConfig: must contain mcpServers object'); } }
🟠 Error	npm/src/agent/mcp/config.js:114	loadMCPConfigurationFromPath accepts any file path without validation, allowing path traversal attacks. An attacker could read arbitrary files by providing paths like '../../../etc/passwd'. 💡 Suggestion Add path validation to reject paths with traversal attempts or ensure the resolved path is within expected directories. Consider maintaining an allowlist of safe directories. 🔧 Suggested Fix function loadMCPConfigurationFromPath(configPath) { if (!configPath) { throw new Error('Config path is required'); } // Reject path traversal attempts if (configPath.includes('..') || configPath.includes('~')) { throw new Error(Invalid config path: path traversal not allowed); } // Normalize and validate the path const normalizedPath = path.resolve(configPath); if (!existsSync(normalizedPath)) { throw new Error(MCP configuration file not found: ${normalizedPath}); } try { const content = readFileSync(normalizedPath, 'utf8'); const config = JSON.parse(content); if (process.env.DEBUG === '1' || process.env.DEBUG_MCP === '1') { console.error(`[MCP DEBUG] Loaded configuration from: ${normalizedPath}`); } // Merge with environment variable overrides return mergeWithEnvironment(config); } catch (error) { throw new Error(Failed to parse MCP config from ${normalizedPath}: ${error.message}); } }

Architecture Issues (4)

Severity	Location	Issue
🟡 Warning	npm/src/delegate.js:189-191	The new MCP parameters (enableMcp, mcpConfig, mcpConfigPath) are added at the end of the function signature, breaking the established pattern where related configuration options are grouped together. This creates inconsistency with how other tool configurations (enableBash, bashConfig, architectureFileName) are organized. 💡 Suggestion Group the MCP parameters with other tool-related configurations. Consider placing enableMcp, mcpConfig, and mcpConfigPath after enableBash/bashConfig and before searchDelegate to maintain logical grouping of tool configurations. 🔧 Suggested Fix Move enableMcp, mcpConfig, mcpConfigPath parameters to be positioned after bashConfig and before architectureFileName to group all tool-related configurations together.
🟡 Warning	npm/src/tools/vercel.js:472	The delegateTool function destructures MCP parameters at the end of the options object, inconsistent with the delegate function's parameter order. This creates a mismatch between where parameters are defined in delegate() and where they're extracted in delegateTool(). 💡 Suggestion Reorder the destructured parameters in delegateTool to match the order in the delegate function signature, or establish a consistent pattern for parameter ordering across both functions. 🔧 Suggested Fix Reorder destructured parameters: { debug, timeout, cwd, allowedFolders, enableBash, bashConfig, enableMcp, mcpConfig, mcpConfigPath, architectureFileName, searchDelegate }
🟡 Warning	npm/src/delegate.js:276-281	The MCP properties are added at the end of the ProbeAgent constructor call, breaking the established pattern where related configurations are grouped. The existing code groups enableBash/bashConfig together, but MCP parameters are placed after enableTasks. 💡 Suggestion Group enableMcp, mcpConfig, and mcpConfigPath with other tool configurations (after bashConfig and before architectureFileName) to maintain consistency with how tool features are organized. 🔧 Suggested Fix Reorder properties: place enableMcp, mcpConfig, mcpConfigPath after bashConfig and before architectureFileName to group all tool-related configurations together.
🟡 Warning	npm/src/tools/vercel.js:561-564	The MCP parameters are passed to delegate() after searchDelegate, inconsistent with the delegate function's parameter order. This creates a mismatch between the function signature and how it's called. 💡 Suggestion Reorder the parameters passed to delegate() to match the function signature order, ensuring consistency between definition and usage. 🔧 Suggested Fix Reorder delegate call parameters to match function signature: place enableMcp, mcpConfig, mcpConfigPath after searchDelegate and before the closing brace.

Performance Issues (4)

Severity	Location	Issue
🟢 Info	npm/src/tools/vercel.js:472	The delegateTool function destructures 3 new MCP-related parameters (enableMcp, mcpConfig, mcpConfigPath) on every delegation call. While minimal overhead, this adds to the parameter passing cost for each delegation. 💡 Suggestion This is acceptable overhead for the feature. However, consider using an options object pattern to reduce parameter count and improve maintainability if more parameters are added in the future.
🟢 Info	npm/src/delegate.js:211-217	The delegate function adds 3 new optional parameters with default values (enableMcp=false, mcpConfig=null, mcpConfigPath=null). These are initialized on every function call even when MCP is not used. 💡 Suggestion Minimal impact. The default value assignment is negligible. This is acceptable for the feature's functionality.
🟡 Warning	npm/src/delegate.js:276-283	Each delegated subagent creates its own MCPXmlBridge instance and initializes MCP connections independently. This can cause significant performance overhead when multiple delegations occur, as each subagent will: (1) Load MCP configuration from disk or parse config objects, (2) Establish new connections to all MCP servers, (3) Fetch and register tools from each server. With multiple MCP servers using stdio transport, this could spawn multiple subprocesses per delegation. 💡 Suggestion Consider implementing MCP connection pooling or sharing the parent's MCP bridge with subagents. Options: (1) Pass the parent's mcpBridge instance to subagents with a reference flag, (2) Implement a singleton MCP manager that shares connections across delegations, (3) Add a configuration option to disable MCP in subagents when not needed, (4) Cache MCP tool definitions and reuse them without reconnection.
🟡 Warning	npm/src/delegate.js:276-283	Subagents create new MCPXmlBridge instances but the code doesn't show explicit cleanup of MCP connections when the subagent completes. MCP connections (especially stdio-based servers) spawn child processes that may not be properly cleaned up, leading to resource leaks. 💡 Suggestion Ensure MCP connections are properly closed when subagent completes. Add cleanup in the delegate function's finally block or implement a destructor pattern. Verify that mcpBridge.close() or equivalent is called to terminate stdio processes and release network connections.

Quality Issues (8)

Severity	Location	Issue
🟢 Info	npm/src/delegate.js:276	JSDoc comment says 'subagent creates own MCPXmlBridge' but doesn't explain the implications: each subagent creates a separate MCP instance with its own connections, which could lead to resource exhaustion with many delegations. 💡 Suggestion Expand documentation to warn about resource implications: 'Each subagent creates its own MCPXmlBridge instance with separate MCP server connections. Be mindful of delegation limits to avoid exhausting MCP server connections.'
🟡 Warning	npm/tests/delegate-config.test.js:49	Test expectations include hardcoded default values (enableMcp: false, mcpConfig: null, mcpConfigPath: null) without verification that these are the correct defaults. The test merely asserts what the code currently does rather than validating requirements. 💡 Suggestion Add a comment explaining why these defaults are expected, or extract them as named constants to make the test intent clearer. Consider adding a separate test that verifies MCP is disabled by default for security reasons.
🟡 Warning	npm/tests/delegate-integration.test.js:92	Test expectations include hardcoded default values (enableMcp: false, mcpConfig: null, mcpConfigPath: null) without verification that these are the correct defaults. The test merely asserts what the code currently does rather than validating requirements. 💡 Suggestion Add a comment explaining why these defaults are expected, or extract them as named constants to make the test intent clearer. Consider adding a separate test that verifies MCP is disabled by default for security reasons.
🟡 Warning	npm/tests/delegate-config.test.js:49	No tests verify that MCP parameters are properly validated or that invalid MCP configurations are handled correctly. Missing tests for: null/undefined enableMcp, invalid mcpConfig objects, non-existent mcpConfigPath. 💡 Suggestion Add negative test cases: 1) Test with enableMcp=true but invalid mcpConfig, 2) Test with enableMcp=true but non-existent mcpConfigPath, 3) Test with mcpConfig as invalid type (string, array), 4) Verify error messages are informative.
🟡 Warning	npm/tests/delegate-integration.test.js:92	No tests verify that MCP parameters are properly validated or that invalid MCP configurations are handled correctly. Missing tests for: null/undefined enableMcp, invalid mcpConfig objects, non-existent mcpConfigPath. 💡 Suggestion Add negative test cases: 1) Test with enableMcp=true but invalid mcpConfig, 2) Test with enableMcp=true but non-existent mcpConfigPath, 3) Test with mcpConfig as invalid type (string, array), 4) Verify error messages are informative.
🟡 Warning	npm/src/delegate.js:211	No validation for new MCP parameters (enableMcp, mcpConfig, mcpConfigPath). The function accepts these parameters without type checking or validation before passing to ProbeAgent constructor. 💡 Suggestion Add parameter validation similar to existing pattern: validate enableMcp is boolean, mcpConfig is object or null, mcpConfigPath is string or null. This would catch configuration errors early and provide clear error messages.
🟡 Warning	npm/src/tools/vercel.js:469	No validation for new MCP parameters (enableMcp, mcpConfig, mcpConfigPath) in delegateTool before passing to delegate function. Invalid types would propagate and cause unclear errors. 💡 Suggestion Add parameter validation in the execute function: validate enableMcp is boolean, mcpConfig is object or null, mcpConfigPath is string or null. Follow the existing validation pattern for other parameters.
🟡 Warning	npm/tests/delegate-config.test.js:49	No test verifies that MCP parameters are actually passed through to the subagent. The test only checks that delegate() is called with the parameters, not that ProbeAgent receives them correctly. 💡 Suggestion Add an integration test that: 1) Creates delegateTool with enableMcp=true and mcpConfig, 2) Mocks ProbeAgent to verify it receives the MCP parameters, 3) Confirms the subagent has MCP enabled. This would validate the full parameter flow.

---

Powered by Visor from Probelabs

Last updated: 2026-02-02T12:29:15.075Z | Triggered by: pr_opened | Commit: 807185c

💡 TIP: You can chat with Visor using /visor ask <your question>