Skip to content

Conversation

@icecrasher321
Copy link
Collaborator

Summary

Need to exclude serverUrl from mcp tool call params.

Type of Change

  • Bug fix

Testing

Tested manually with @aadamgough

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Dec 31, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Review Updated (UTC)
docs Skipped Skipped Dec 31, 2025 7:37pm

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 31, 2025

Greptile Summary

This PR fixes a bug where serverUrl was being incorrectly passed as a tool argument to MCP tools. The serverUrl is a metadata field used for validation purposes (detecting when a server's URL has changed), but it should not be forwarded to the actual MCP tool execution.

The fix adds serverUrl to the MCP_SYSTEM_PARAMETERS set, which filters out internal/system parameters when extracting tool arguments. This ensures only the actual tool-specific parameters are sent to the MCP server.

Changes:

  • Added serverUrl to the MCP_SYSTEM_PARAMETERS set on line 124

Context:

  • serverUrl is stored alongside MCP tool references in workflow blocks (per StoredMcpToolReference type)
  • It's used by the validation logic in tool-validation.ts to detect when a server's URL changes
  • The executeMcpTool function filters parameters using this set on line 998 to extract only actual tool arguments
  • Without this fix, serverUrl would be incorrectly included in the tool execution payload

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The change is a simple, targeted bug fix that adds a single entry to a parameter filtering set. The fix is consistent with the existing pattern of filtering system/metadata parameters from MCP tool arguments. The change has been manually tested and aligns with the documented types where serverUrl is defined as an optional metadata field in StoredMcpToolReference. No breaking changes or side effects expected.
  • No files require special attention

Important Files Changed

Filename Overview
apps/sim/tools/index.ts Added serverUrl to the MCP_SYSTEM_PARAMETERS set to prevent it from being passed as a tool argument

Sequence Diagram

sequenceDiagram
    participant UI as MCP Tools List UI
    participant Executor as executeTool()
    participant McpHandler as executeMcpTool()
    participant Filter as MCP_SYSTEM_PARAMETERS
    participant MCPServer as MCP Server

    UI->>Executor: Call with params {serverId, serverUrl, toolName, serverName, ...args}
    Executor->>McpHandler: Forward MCP tool request
    
    Note over McpHandler,Filter: Extract tool arguments
    McpHandler->>Filter: Filter out system parameters
    Filter-->>McpHandler: Returns only tool-specific args
    
    Note over Filter: BEFORE FIX: serverUrl was included ❌<br/>AFTER FIX: serverUrl is filtered out ✅
    
    McpHandler->>MCPServer: Execute tool with filtered args
    MCPServer-->>McpHandler: Return execution result
    McpHandler-->>Executor: Return ToolResponse
    Executor-->>UI: Display result
Loading

@icecrasher321 icecrasher321 merged commit 4301342 into staging Dec 31, 2025
11 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.

2 participants