Skip to content

fix: prevent _buildNativeTools crash when MCP tools are in toolImplementations#470

Merged
buger merged 1 commit intomainfrom
fix/mcp-tools-build-native-crash
Mar 4, 2026
Merged

fix: prevent _buildNativeTools crash when MCP tools are in toolImplementations#470
buger merged 1 commit intomainfrom
fix/mcp-tools-build-native-crash

Conversation

@buger
Copy link
Collaborator

@buger buger commented Mar 3, 2026

Summary

  • Fix TypeError: Cannot destructure property 'schema' of null when MCP tools are present in toolImplementations
  • Add null check before destructuring _getToolSchemaAndDescription() return value
  • Add 2 regression tests covering single and multiple MCP tools in toolImplementations

Closes #469

Root cause

After initializeMCP(), MCP tools get merged into this.toolImplementations. When _buildNativeTools() iterates all entries, it calls _getToolSchemaAndDescription(toolName) which returns null for MCP tool names (e.g. __tools___slack-send-dm). The destructuring const { schema, description } = null throws a TypeError before the if (schema && description) guard can execute.

Fix

// Before (crashes):
const { schema, description } = this._getToolSchemaAndDescription(toolName);

// After (safe):
const toolInfo = this._getToolSchemaAndDescription(toolName);
if (!toolInfo) continue;
const { schema, description } = toolInfo;

MCP tools are already handled separately via mcpBridge.getVercelTools() further down in the same method, so skipping them in the toolImplementations loop is correct.

Test plan

  • 2 new tests in mcp-message-history.test.js reproducing the exact crash scenario
  • All 106 test suites pass (2621 tests)

🤖 Generated with Claude Code

…entations (#469)

After initializeMCP() merges MCP tools into toolImplementations,
_buildNativeTools iterates all entries and calls _getToolSchemaAndDescription
which returns null for MCP tools. Destructuring null throws a TypeError.

Fix: check for null return before destructuring. MCP tools in
toolImplementations are safely skipped since they're already handled
separately via mcpBridge.getVercelTools().

Closes #469

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@probelabs
Copy link
Contributor

probelabs bot commented Mar 3, 2026

PR Overview: Fix _buildNativeTools crash when MCP tools are in toolImplementations

Summary

This PR fixes a TypeError: Cannot destructure property 'schema' of null crash that occurs in _buildNativeTools() when MCP tools have been merged into toolImplementations.


Files Changed Analysis

File Status Additions Deletions
npm/src/agent/ProbeAgent.js Modified 5 1
npm/tests/unit/mcp-message-history.test.js Modified 53 0

Total: 58 additions, 1 deletion across 2 files.


Architecture & Impact Assessment

What This PR Accomplishes

  • Prevents a runtime crash when _buildNativeTools() iterates over toolImplementations containing MCP tools
  • MCP tools (with names like __tools___slack-send-dm) are merged into toolImplementations during initializeMCP() but have no schema available via _getToolSchemaAndDescription()

Root Cause

The crash occurred because:

  1. After initializeMCP(), MCP tools are merged into this.toolImplementations (line ~714)
  2. _buildNativeTools() iterates ALL entries in toolImplementations (line ~1839)
  3. _getToolSchemaAndDescription(toolName) returns null for MCP tool names
  4. The destructuring const { schema, description } = null throws TypeError before the if (schema && description) guard can execute

Key Technical Change

// Before (crashes):
const { schema, description } = this._getToolSchemaAndDescription(toolName);

// After (safe):
const toolInfo = this._getToolSchemaAndDescription(toolName);
if (!toolInfo) continue;
const { schema, description } = toolInfo;

MCP tools are correctly handled separately via mcpBridge.getVercelTools() later in the same method, so skipping them in the toolImplementations loop is the correct behavior.

Affected System Components

flowchart TD
    A[initializeMCP] -->|merges MCP tools| B[toolImplementations]
    B -->|iterates all entries| C[_buildNativeTools]
    C -->|calls| D[_getToolSchemaAndDescription]
    D -->|returns null for MCP tools| E{null check}
    E -->|skip| F[Continue loop]
    C -->|later| G[mcpBridge.getVercelTools]
    G -->|provides| H[MCP tools in Vercel format]
Loading

Scope Discovery & Context Expansion

Related Code Paths

  • ProbeAgent.initialize() (~line 704-724): Merges MCP tools into toolImplementations
  • ProbeAgent.getSystemMessage() (~line 2916-2922): Also merges MCP tools (lazy init path)
  • ProbeAgent._buildNativeTools() (~line 1637-1947): The fixed method
  • mcpBridge.getVercelTools(): Provides properly formatted MCP tools

Test Coverage

  • 2 new regression tests in mcp-message-history.test.js:
    1. Single MCP tool in toolImplementations (reproduces exact crash scenario)
    2. Multiple MCP tools in toolImplementations
  • All 106 test suites pass (2621 tests)

Potential Related Areas to Verify

  • DSL runtime MCP integration (npm/src/agent/dsl/)
  • createExecutePlanTool MCP handling (npm/src/tools/executePlan.js)
  • MCP bridge implementations (npm/src/agent/mcp/)

Labels

  • tags.review-effort: 1 (trivial - single null check fix with clear tests)
  • tags.label: bug
Metadata
  • Review Effort: 1 / 5
  • Primary Label: bug

Powered by Visor from Probelabs

Last updated: 2026-03-03T21:29:24.669Z | Triggered by: pr_opened | Commit: 5706d02

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

@probelabs
Copy link
Contributor

probelabs bot commented Mar 3, 2026

✅ Security Check Passed

No security issues found – changes LGTM.

✅ Architecture Check Passed

No architecture issues found – changes LGTM.

✅ Security Check Passed

No security issues found – changes LGTM.

\n\n

✅ Architecture Check Passed

No architecture issues found – changes LGTM.

\n\n

✅ Performance Check Passed

No performance issues found – changes LGTM.


Powered by Visor from Probelabs

Last updated: 2026-03-03T21:22:53.910Z | Triggered by: pr_opened | Commit: 5706d02

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

@buger buger merged commit 66803b1 into main Mar 4, 2026
16 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: _buildNativeTools crashes with TypeError when MCP tools are present in toolImplementations

1 participant