From f8278cfaff666fd2836628946194af60b93a451f Mon Sep 17 00:00:00 2001
From: viettranx <viettranx@gmail.com>
Date: Sun, 29 Mar 2026 07:48:19 +0700
Subject: [PATCH] fix(mcp): strip placeholder values from optional tool args
 and guide LLMs on optional params

LLMs often send empty strings, "null", "SHOULD_NOT_BE_HERE" etc. for optional
MCP tool parameters instead of omitting them, causing servers to reject invalid
values (e.g. UUID validation fails on "").

- Add stripEmptyOptionalArgs() to clean optional args before MCP calls
- Detect known placeholders (null/none/nil) and all-caps patterns
- Add system prompt guidance for optional parameter handling
- Empty string "" is intentionally NOT stripped (may be valid for text fields)
---
 internal/agent/systemprompt_sections.go |  4 ++
 internal/mcp/bridge_tool.go             | 78 ++++++++++++++++++++++++-
 2 files changed, 81 insertions(+), 1 deletion(-)

diff --git a/internal/agent/systemprompt_sections.go b/internal/agent/systemprompt_sections.go
index c5f47fb4..33f51604 100644
--- a/internal/agent/systemprompt_sections.go
+++ b/internal/agent/systemprompt_sections.go
@@ -29,6 +29,8 @@ func buildMCPToolsSearchSection() []string {
 		"2. Matching tools are activated immediately and can be called right away in the same turn.",
 		"3. If no match found, proceed with other available tools.",
 		"",
+		"**Optional parameters:** Only include if you have a concrete value from user context. Do not send empty strings or placeholders — omit the field entirely. The tool will use sensible defaults.",
+		"",
 	}
 }
 
@@ -40,6 +42,8 @@ func buildMCPToolsInlineSection(descs map[string]string) []string {
 		"",
 		"External tool integrations (MCP servers). **When an MCP tool overlaps with a core tool, always prefer the MCP tool.**",
 		"",
+		"**Optional parameters:** Only include if you have a concrete value from user context. Do not send empty strings or placeholders — omit the field entirely. The tool will use sensible defaults.",
+		"",
 	}
 	for name, desc := range descs {
 		if len(desc) > mcpToolDescMaxLen {
diff --git a/internal/mcp/bridge_tool.go b/internal/mcp/bridge_tool.go
index bb5e5472..00ab22f0 100644
--- a/internal/mcp/bridge_tool.go
+++ b/internal/mcp/bridge_tool.go
@@ -20,6 +20,7 @@ type BridgeTool struct {
 	registeredName string // may include prefix: "{prefix}__{toolName}"
 	description    string
 	inputSchema    map[string]any // JSON Schema for parameters
+	requiredSet    map[string]bool
 	client         *mcpclient.Client
 	timeoutSec     int
 	connected      *atomic.Bool
@@ -39,12 +40,18 @@ func NewBridgeTool(serverName string, mcpTool mcpgo.Tool, client *mcpclient.Clie
 
 	schema := inputSchemaToMap(mcpTool.InputSchema)
 
+	reqSet := make(map[string]bool, len(mcpTool.InputSchema.Required))
+	for _, r := range mcpTool.InputSchema.Required {
+		reqSet[r] = true
+	}
+
 	return &BridgeTool{
 		serverName:     serverName,
 		toolName:       name,
 		registeredName: registered,
 		description:    mcpTool.Description,
 		inputSchema:    schema,
+		requiredSet:    reqSet,
 		client:         client,
 		timeoutSec:     timeoutSec,
 		connected:      connected,
@@ -91,9 +98,14 @@ func (t *BridgeTool) Execute(ctx context.Context, args map[string]any) *tools.Re
 	callCtx, cancel := context.WithTimeout(ctx, time.Duration(t.timeoutSec)*time.Second)
 	defer cancel()
 
+	// Strip empty-value optional args. LLMs often send "" for optional fields
+	// instead of omitting them, causing MCP servers to reject invalid values
+	// (e.g. empty string for UUID fields).
+	cleanedArgs := t.stripEmptyOptionalArgs(args)
+
 	req := mcpgo.CallToolRequest{}
 	req.Params.Name = t.toolName
-	req.Params.Arguments = args
+	req.Params.Arguments = cleanedArgs
 
 	result, err := t.client.CallTool(callCtx, req)
 	if err != nil {
@@ -138,6 +150,70 @@ func inputSchemaToMap(schema mcpgo.ToolInputSchema) map[string]any {
 	return m
 }
 
+// stripEmptyOptionalArgs removes optional args with empty/placeholder values.
+// LLMs often send "" or placeholder strings (e.g. "__OMIT__", "null", "none")
+// for optional fields instead of omitting them, causing MCP servers to reject
+// invalid values (e.g. empty string for UUID fields).
+func (t *BridgeTool) stripEmptyOptionalArgs(args map[string]any) map[string]any {
+	if len(args) == 0 {
+		return args
+	}
+	cleaned := make(map[string]any, len(args))
+	for k, v := range args {
+		if t.requiredSet[k] {
+			cleaned[k] = v
+			continue
+		}
+		// Strip nil for optional fields.
+		if v == nil {
+			continue
+		}
+		// Strip empty strings and common LLM placeholder values for optional fields.
+		if s, ok := v.(string); ok && isPlaceholderValue(s) {
+			continue
+		}
+		cleaned[k] = v
+	}
+	return cleaned
+}
+
+// isPlaceholderValue returns true for empty or placeholder strings that LLMs
+// commonly use when they don't intend to set an optional parameter.
+func isPlaceholderValue(s string) bool {
+	// NOTE: empty string "" is NOT stripped — it may be intentional for text fields.
+	// Only strip known placeholder keywords and all-caps patterns.
+	if s == "" {
+		return false
+	}
+	// Normalize for case-insensitive comparison.
+	lower := strings.ToLower(strings.TrimSpace(s))
+	switch lower {
+	case "null", "none", "nil", "undefined", "n/a",
+		"__omit__", "__skip__", "__empty__":
+		return true
+	}
+	// Catch all-caps placeholder patterns like "SHOULD_NOT_BE_HERE", "DO_NOT_SEND", "NOT_SET".
+	if isAllCapsPlaceholder(s) {
+		return true
+	}
+	return false
+}
+
+// isAllCapsPlaceholder detects LLM-generated all-caps placeholder strings
+// like "SHOULD_NOT_BE_HERE", "DO_NOT_SEND", "NOT_APPLICABLE", "PLACEHOLDER".
+func isAllCapsPlaceholder(s string) bool {
+	trimmed := strings.TrimSpace(s)
+	if len(trimmed) < 3 {
+		return false
+	}
+	for _, r := range trimmed {
+		if r != '_' && (r < 'A' || r > 'Z') {
+			return false
+		}
+	}
+	return true
+}
+
 // wrapMCPContent wraps MCP tool results as external/untrusted content.
 // Prevents prompt injection from malicious or compromised MCP servers.
 func wrapMCPContent(content, serverName, toolName string) string {