Skip to content

feat(mcp): advertise native args object on call_tool_* variants #379

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Dumbris merged 1 commit into from
Apr 11, 2026
+198 −121
Merged

feat(mcp): advertise native args object on call_tool_* variants #379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 67 additions & 59 deletions internal/server/mcp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,70 @@ func (p *MCPProxyServer) emitActivityInternalToolCall(internalToolName, targetSe
}
}

// buildCallToolVariantTool constructs the mcp.Tool definition for a single
// call_tool_* variant (read/write/destructive). Factored out so both
// registerTools and schema tests exercise the same builder.
func buildCallToolVariantTool(variant string) mcp.Tool {
var description, title, nameExample, sensitivityDesc, reasonDesc string
var opts []mcp.ToolOption

switch variant {
case contracts.ToolVariantRead:
description = "Execute a READ-ONLY tool. WORKFLOW: 1) Call retrieve_tools first to find tools, 2) Use the exact 'name' field from results. DECISION RULE: Use this when the tool name contains: search, query, list, get, fetch, find, check, view, read, show, describe, lookup, retrieve, browse, explore, discover, scan, inspect, analyze, examine, validate, verify. Examples: search_files, get_user, list_repositories, query_database, find_issues, check_status. This is the DEFAULT choice when unsure - most tools are read-only."
title = "Call Tool (Read)"
nameExample = "github:get_user"
sensitivityDesc = "Classify data being accessed: public, internal, private, or unknown. Helps track sensitive data access patterns."
reasonDesc = "Why is this tool being called? Provide context like 'User asked to check status' or 'Gathering data for report'."
opts = []mcp.ToolOption{mcp.WithReadOnlyHintAnnotation(true)}
case contracts.ToolVariantWrite:
description = "Execute a STATE-MODIFYING tool. WORKFLOW: 1) Call retrieve_tools first to find tools, 2) Use the exact 'name' field from results. DECISION RULE: Use this when the tool name contains: create, update, modify, add, set, send, edit, change, write, post, put, patch, insert, upload, submit, assign, configure, enable, register, subscribe, publish, move, copy, rename, merge. Examples: create_issue, update_file, send_message, add_comment, set_status, edit_page. Use only when explicitly modifying state."
title = "Call Tool (Write)"
nameExample = "github:create_issue"
sensitivityDesc = "Classify data being modified: public, internal, private, or unknown. Helps track sensitive data changes."
reasonDesc = "Why is this modification needed? Provide context like 'User requested update' or 'Fixing reported issue'."
opts = []mcp.ToolOption{mcp.WithDestructiveHintAnnotation(false)}
case contracts.ToolVariantDestructive:
description = "Execute a DESTRUCTIVE tool. WORKFLOW: 1) Call retrieve_tools first to find tools, 2) Use the exact 'name' field from results. DECISION RULE: Use this when the tool name contains: delete, remove, drop, revoke, disable, destroy, purge, reset, clear, unsubscribe, cancel, terminate, close, archive, ban, block, disconnect, kill, wipe, truncate, force, hard. Examples: delete_repo, remove_user, drop_table, revoke_access, clear_cache, terminate_session. Use for irreversible or high-impact operations."
title = "Call Tool (Destructive)"
nameExample = "github:delete_repo"
sensitivityDesc = "Classify data being deleted: public, internal, private, or unknown. Important for tracking destructive operations on sensitive data."
reasonDesc = "Why is this deletion needed? Provide justification like 'User confirmed cleanup' or 'Removing obsolete data'."
opts = []mcp.ToolOption{mcp.WithDestructiveHintAnnotation(true)}
default:
description = "Execute a tool."
title = "Call Tool"
nameExample = "server:tool"
sensitivityDesc = "Classify data sensitivity."
reasonDesc = "Why is this tool being called?"
}

allOpts := []mcp.ToolOption{
mcp.WithDescription(description),
mcp.WithTitleAnnotation(title),
}
allOpts = append(allOpts, opts...)
allOpts = append(allOpts,
mcp.WithString("name",
mcp.Required(),
mcp.Description(fmt.Sprintf("Tool name in format 'server:tool' (e.g., '%s'). CRITICAL: You MUST use exact names from retrieve_tools results - do NOT guess or invent server names. Unknown servers will fail.", nameExample)),
),
mcp.WithObject("args",
mcp.Description("Arguments to pass to the upstream tool as a native JSON object. Refer to the tool's inputSchema from retrieve_tools for required parameters. Example: {\"path\": \"src/index.ts\", \"limit\": 20}. This is the preferred parameter — it eliminates JSON escaping overhead. Use 'args_json' only if your client cannot produce nested JSON objects."),
),
mcp.WithString("args_json",
mcp.Description("Legacy: arguments as a pre-serialized JSON string. Prefer the 'args' parameter instead — it accepts a native JSON object and eliminates escaping overhead. If both are provided, 'args_json' wins for backward compatibility."),
),
mcp.WithString("intent_data_sensitivity",
mcp.Description(sensitivityDesc),
),
mcp.WithString("intent_reason",
mcp.Description(reasonDesc),
),
)

return mcp.NewTool(variant, allOpts...)
}

// registerTools registers all proxy tools with the MCP server
func (p *MCPProxyServer) registerTools(_ bool) {
// retrieve_tools - THE PRIMARY TOOL FOR DISCOVERING TOOLS - Enhanced with clear instructions
Expand Down Expand Up @@ -416,69 +480,13 @@ func (p *MCPProxyServer) registerTools(_ bool) {
// Intent-based tool variants (Spec 018)
// These replace the legacy call_tool with three operation-specific variants
// that enable granular IDE permission control and require explicit intent declaration.

// call_tool_read - Read-only operations
// NOTE: Intent parameters are flattened (not nested objects) for Gemini 3 Pro compatibility
callToolReadTool := mcp.NewTool(contracts.ToolVariantRead,
mcp.WithDescription("Execute a READ-ONLY tool. WORKFLOW: 1) Call retrieve_tools first to find tools, 2) Use the exact 'name' field from results. DECISION RULE: Use this when the tool name contains: search, query, list, get, fetch, find, check, view, read, show, describe, lookup, retrieve, browse, explore, discover, scan, inspect, analyze, examine, validate, verify. Examples: search_files, get_user, list_repositories, query_database, find_issues, check_status. This is the DEFAULT choice when unsure - most tools are read-only."),
mcp.WithTitleAnnotation("Call Tool (Read)"),
mcp.WithReadOnlyHintAnnotation(true),
mcp.WithString("name",
mcp.Required(),
mcp.Description("Tool name in format 'server:tool' (e.g., 'github:get_user'). CRITICAL: You MUST use exact names from retrieve_tools results - do NOT guess or invent server names. Unknown servers will fail."),
),
mcp.WithString("args_json",
mcp.Description("Arguments to pass to the tool as JSON string. Refer to the tool's inputSchema from retrieve_tools for required parameters."),
),
mcp.WithString("intent_data_sensitivity",
mcp.Description("Classify data being accessed: public, internal, private, or unknown. Helps track sensitive data access patterns."),
),
mcp.WithString("intent_reason",
mcp.Description("Why is this tool being called? Provide context like 'User asked to check status' or 'Gathering data for report'."),
),
)
callToolReadTool := buildCallToolVariantTool(contracts.ToolVariantRead)
p.server.AddTool(callToolReadTool, p.handleCallToolRead)

// call_tool_write - State-modifying operations
callToolWriteTool := mcp.NewTool(contracts.ToolVariantWrite,
mcp.WithDescription("Execute a STATE-MODIFYING tool. WORKFLOW: 1) Call retrieve_tools first to find tools, 2) Use the exact 'name' field from results. DECISION RULE: Use this when the tool name contains: create, update, modify, add, set, send, edit, change, write, post, put, patch, insert, upload, submit, assign, configure, enable, register, subscribe, publish, move, copy, rename, merge. Examples: create_issue, update_file, send_message, add_comment, set_status, edit_page. Use only when explicitly modifying state."),
mcp.WithTitleAnnotation("Call Tool (Write)"),
mcp.WithDestructiveHintAnnotation(false),
mcp.WithString("name",
mcp.Required(),
mcp.Description("Tool name in format 'server:tool' (e.g., 'github:create_issue'). CRITICAL: You MUST use exact names from retrieve_tools results - do NOT guess or invent server names. Unknown servers will fail."),
),
mcp.WithString("args_json",
mcp.Description("Arguments to pass to the tool as JSON string. Refer to the tool's inputSchema from retrieve_tools for required parameters."),
),
mcp.WithString("intent_data_sensitivity",
mcp.Description("Classify data being modified: public, internal, private, or unknown. Helps track sensitive data changes."),
),
mcp.WithString("intent_reason",
mcp.Description("Why is this modification needed? Provide context like 'User requested update' or 'Fixing reported issue'."),
),
)
callToolWriteTool := buildCallToolVariantTool(contracts.ToolVariantWrite)
p.server.AddTool(callToolWriteTool, p.handleCallToolWrite)

// call_tool_destructive - Irreversible operations
callToolDestructiveTool := mcp.NewTool(contracts.ToolVariantDestructive,
mcp.WithDescription("Execute a DESTRUCTIVE tool. WORKFLOW: 1) Call retrieve_tools first to find tools, 2) Use the exact 'name' field from results. DECISION RULE: Use this when the tool name contains: delete, remove, drop, revoke, disable, destroy, purge, reset, clear, unsubscribe, cancel, terminate, close, archive, ban, block, disconnect, kill, wipe, truncate, force, hard. Examples: delete_repo, remove_user, drop_table, revoke_access, clear_cache, terminate_session. Use for irreversible or high-impact operations."),
mcp.WithTitleAnnotation("Call Tool (Destructive)"),
mcp.WithDestructiveHintAnnotation(true),
mcp.WithString("name",
mcp.Required(),
mcp.Description("Tool name in format 'server:tool' (e.g., 'github:delete_repo'). CRITICAL: You MUST use exact names from retrieve_tools results - do NOT guess or invent server names. Unknown servers will fail."),
),
mcp.WithString("args_json",
mcp.Description("Arguments to pass to the tool as JSON string. Refer to the tool's inputSchema from retrieve_tools for required parameters."),
),
mcp.WithString("intent_data_sensitivity",
mcp.Description("Classify data being deleted: public, internal, private, or unknown. Important for tracking destructive operations on sensitive data."),
),
mcp.WithString("intent_reason",
mcp.Description("Why is this deletion needed? Provide justification like 'User confirmed cleanup' or 'Removing obsolete data'."),
),
)
callToolDestructiveTool := buildCallToolVariantTool(contracts.ToolVariantDestructive)
p.server.AddTool(callToolDestructiveTool, p.handleCallToolDestructive)

// read_cache - Access paginated data when responses are truncated
Expand Down
125 changes: 125 additions & 0 deletions internal/server/mcp_call_tool_args_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
package server

import (
"context"
"testing"

"github.com/mark3labs/mcp-go/mcp"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap"

"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
"github.com/smart-mcp-proxy/mcpproxy-go/internal/contracts"
"github.com/smart-mcp-proxy/mcpproxy-go/internal/secret"
"github.com/smart-mcp-proxy/mcpproxy-go/internal/upstream"
)

// TestCallToolVariantSchemaAdvertisesArgsObject verifies that the
// call_tool_read/write/destructive tool schemas advertise a native
// `args` object parameter alongside the legacy `args_json` string.
// This eliminates the escaping overhead when LLM clients produce
// tool arguments (see mcp-proxy-shim README for background).
func TestCallToolVariantSchemaAdvertisesArgsObject(t *testing.T) {
variants := []string{
contracts.ToolVariantRead,
contracts.ToolVariantWrite,
contracts.ToolVariantDestructive,
}

for _, variant := range variants {
t.Run(variant, func(t *testing.T) {
tool := buildCallToolVariantTool(variant)

// Legacy string path must remain advertised for backward compat.
argsJSONProp, ok := tool.InputSchema.Properties["args_json"].(map[string]any)
require.True(t, ok, "schema must still advertise 'args_json' (backward compat)")
assert.Equal(t, "string", argsJSONProp["type"])

// New native object path must be advertised.
argsProp, ok := tool.InputSchema.Properties["args"].(map[string]any)
require.True(t, ok, "schema must advertise 'args' as an object property")
assert.Equal(t, "object", argsProp["type"])

// `args` must not be required (args_json is still accepted).
for _, req := range tool.InputSchema.Required {
assert.NotEqual(t, "args", req, "'args' must not be required")
}
})
}
}

// TestHandleCallToolVariantAcceptsArgsObject verifies the handler extracts a
// native `args` object from the request and routes past the argument-parsing
// phase without emitting "Invalid args_json format" — i.e. the native object
// path actually works, not just the schema.
func TestHandleCallToolVariantAcceptsArgsObject(t *testing.T) {
mockProxy := &MCPProxyServer{
upstreamManager: upstream.NewManager(zap.NewNop(), config.DefaultConfig(), nil, secret.NewResolver(), nil),
logger: zap.NewNop(),
config: &config.Config{},
}
ctx := context.Background()

request := mcp.CallToolRequest{}
request.Params.Name = contracts.ToolVariantRead
request.Params.Arguments = map[string]any{
"name": "non-existent-server:some_tool",
"args": map[string]any{
"files": []any{
map[string]any{"path": "src/index.ts", "head": 20},
},
},
}

result, err := mockProxy.handleCallToolVariant(ctx, request, contracts.ToolVariantRead)
require.NoError(t, err)
require.NotNil(t, result)
require.True(t, result.IsError, "non-existent server should yield an error result")

// The error must NOT be an argument-parsing error. The handler may still
// fail later in the dispatch pipeline (because this mock proxy has no
// real runtime), but it must get past argument extraction — proving the
// native object path was accepted.
require.Len(t, result.Content, 1)
textContent, ok := result.Content[0].(mcp.TextContent)
require.True(t, ok, "expected TextContent")
errMsg := textContent.Text
assert.NotContains(t, errMsg, "Invalid args_json format",
"handler should not attempt args_json parsing when only 'args' object is provided")
assert.NotContains(t, errMsg, "Missing required parameter",
"required parameters are present — 'args' object must be accepted")
}

// TestHandleCallToolVariantArgsJSONWinsWhenBothProvided verifies backward
// compatibility: when a client sends BOTH args_json and args, args_json takes
// precedence. This preserves existing behavior so migration is gradual.
func TestHandleCallToolVariantArgsJSONWinsWhenBothProvided(t *testing.T) {
mockProxy := &MCPProxyServer{
upstreamManager: upstream.NewManager(zap.NewNop(), config.DefaultConfig(), nil, secret.NewResolver(), nil),
logger: zap.NewNop(),
config: &config.Config{},
}
ctx := context.Background()

// Malformed args_json + well-formed args → must surface args_json parse
// error, proving args_json took precedence.
request := mcp.CallToolRequest{}
request.Params.Name = contracts.ToolVariantRead
request.Params.Arguments = map[string]any{
"name": "non-existent-server:some_tool",
"args_json": "not valid json",
"args": map[string]any{"ok": true},
}

result, err := mockProxy.handleCallToolVariant(ctx, request, contracts.ToolVariantRead)
require.NoError(t, err)
require.NotNil(t, result)
require.True(t, result.IsError)

require.Len(t, result.Content, 1)
textContent, ok := result.Content[0].(mcp.TextContent)
require.True(t, ok)
assert.Contains(t, textContent.Text, "Invalid args_json format",
"args_json must take precedence when both are provided (backward compat)")
}
68 changes: 6 additions & 62 deletions internal/server/mcp_routing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,75 +342,19 @@ func (p *MCPProxyServer) buildCallToolModeTools() []mcpserver.ServerTool {
Handler: p.handleRetrieveToolsForMode(config.RoutingModeRetrieveTools),
})

// call_tool_read
callToolReadTool := mcp.NewTool(contracts.ToolVariantRead,
mcp.WithDescription("Execute a READ-ONLY tool. WORKFLOW: 1) Call retrieve_tools first to find tools, 2) Use the exact 'name' field from results. Use this for: search, query, list, get, fetch, find, check, view, read, show, describe, lookup, retrieve, browse, explore, discover, scan, inspect, analyze, examine, validate, verify. This is the DEFAULT choice when unsure."),
mcp.WithTitleAnnotation("Call Tool (Read)"),
mcp.WithReadOnlyHintAnnotation(true),
mcp.WithString("name",
mcp.Required(),
mcp.Description("Tool name in format 'server:tool' (e.g., 'github:get_user'). Use exact names from retrieve_tools results."),
),
mcp.WithString("args_json",
mcp.Description("Arguments as JSON string. Refer to the tool's inputSchema from retrieve_tools."),
),
mcp.WithString("intent_data_sensitivity",
mcp.Description("Classify data: public, internal, private, or unknown."),
),
mcp.WithString("intent_reason",
mcp.Description("Why is this tool being called? Provide context."),
),
)
// call_tool_read / call_tool_write / call_tool_destructive — all three
// built from the shared helper in mcp.go so schema stays in sync across
// the default and retrieve_tools routing modes.
tools = append(tools, mcpserver.ServerTool{
Tool: callToolReadTool,
Tool: buildCallToolVariantTool(contracts.ToolVariantRead),
Handler: p.handleCallToolRead,
})

// call_tool_write
callToolWriteTool := mcp.NewTool(contracts.ToolVariantWrite,
mcp.WithDescription("Execute a STATE-MODIFYING tool. WORKFLOW: 1) Call retrieve_tools first to find tools, 2) Use the exact 'name' field from results. Use this for: create, update, modify, add, set, send, edit, change, write, post, put, patch, insert, upload, submit. Use only when explicitly modifying state."),
mcp.WithTitleAnnotation("Call Tool (Write)"),
mcp.WithDestructiveHintAnnotation(false),
mcp.WithString("name",
mcp.Required(),
mcp.Description("Tool name in format 'server:tool' (e.g., 'github:create_issue'). Use exact names from retrieve_tools results."),
),
mcp.WithString("args_json",
mcp.Description("Arguments as JSON string. Refer to the tool's inputSchema from retrieve_tools."),
),
mcp.WithString("intent_data_sensitivity",
mcp.Description("Classify data: public, internal, private, or unknown."),
),
mcp.WithString("intent_reason",
mcp.Description("Why is this modification needed? Provide context."),
),
)
tools = append(tools, mcpserver.ServerTool{
Tool: callToolWriteTool,
Tool: buildCallToolVariantTool(contracts.ToolVariantWrite),
Handler: p.handleCallToolWrite,
})

// call_tool_destructive
callToolDestructiveTool := mcp.NewTool(contracts.ToolVariantDestructive,
mcp.WithDescription("Execute a DESTRUCTIVE tool. WORKFLOW: 1) Call retrieve_tools first to find tools, 2) Use the exact 'name' field from results. Use this for: delete, remove, drop, revoke, disable, destroy, purge, reset, clear, terminate. Use for irreversible or high-impact operations."),
mcp.WithTitleAnnotation("Call Tool (Destructive)"),
mcp.WithDestructiveHintAnnotation(true),
mcp.WithString("name",
mcp.Required(),
mcp.Description("Tool name in format 'server:tool' (e.g., 'github:delete_repo'). Use exact names from retrieve_tools results."),
),
mcp.WithString("args_json",
mcp.Description("Arguments as JSON string. Refer to the tool's inputSchema from retrieve_tools."),
),
mcp.WithString("intent_data_sensitivity",
mcp.Description("Classify data: public, internal, private, or unknown."),
),
mcp.WithString("intent_reason",
mcp.Description("Why is this deletion needed? Provide justification."),
),
)
tools = append(tools, mcpserver.ServerTool{
Tool: callToolDestructiveTool,
Tool: buildCallToolVariantTool(contracts.ToolVariantDestructive),
Handler: p.handleCallToolDestructive,
})

Expand Down
Loading