fix(mcp): prevent LLM hallucination of optional tool parameters

3-layer defense against GPT-5.4 filling all optional MCP tool params
with fabricated values (e.g. api_key:"optional", proxyUrl:"http://example.com"):

Layer 1 — bridge_tool.go: expand placeholder detection to catch "optional",
"skip", example URLs; type-aware empty string handling (keep for string-typed,
strip for non-string); add propertyType() helper.

Layer 2 — schema_strict.go: OpenAI strict mode transform — optional props
become nullable unions, all props required, additionalProperties:false.
Constrained decoding prevents invalid output. Only enabled for first-party
OpenAI/Codex providers.

Layer 3 — systemprompt_sections.go: concrete WRONG/RIGHT examples in MCP
optional param instruction.

internal/mcp/bridge_tool.go

@@ -154,9 +154,8 @@ func inputSchemaToMap(schema mcpgo.ToolInputSchema) map[string]any {
 }
 
 // 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).
+// LLMs often send "", "optional", "null", or null for optional fields instead
+// of omitting them, causing MCP servers to reject invalid values.
 func (t *BridgeTool) stripEmptyOptionalArgs(args map[string]any) map[string]any {
 	if len(args) == 0 {
 		return args
@@ -167,35 +166,56 @@ func (t *BridgeTool) stripEmptyOptionalArgs(args map[string]any) map[string]any
 			cleaned[k] = v
 			continue
 		}
-		// Strip nil for optional fields.
+		// Strip nil/null for optional fields (also handles strict mode where model sends null).
 		if v == nil {
 			continue
 		}
-		// Strip empty strings and common LLM placeholder values for optional fields.
-		if s, ok := v.(string); ok && isPlaceholderValue(s) {
-			continue
+		if s, ok := v.(string); ok {
+			// Strip known placeholder values (e.g. "optional", "null", "http://example.com").
+			if isPlaceholderValue(s) {
+				continue
+			}
+			// Type-aware empty string: keep for string-typed params (user may want empty),
+			// strip for non-string params (empty string is never valid for number/boolean/UUID).
+			if s == "" && t.propertyType(k) != "string" {
+				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.
+// propertyType returns the JSON Schema "type" for a property, or "" if unknown.
+func (t *BridgeTool) propertyType(name string) string {
+	props, _ := t.inputSchema["properties"].(map[string]any)
+	if props == nil {
+		return ""
+	}
+	prop, _ := props[name].(map[string]any)
+	if prop == nil {
+		return ""
+	}
+	typ, _ := prop["type"].(string)
+	return typ
+}
+
+// isPlaceholderValue returns true for placeholder strings that LLMs commonly
+// generate when they don't intend to set an optional parameter.
+// Empty string ("") is NOT handled here — see stripEmptyOptionalArgs for type-aware handling.
 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__":
+		"optional", "skip", // LLMs copy these from schema descriptions
+		"__omit__", "__skip__", "__empty__",
+		"http://example.com", "https://example.com", // common hallucinated URLs
+		"http://localhost", "https://localhost":
 		return true
 	}
-	// Catch all-caps placeholder patterns like "SHOULD_NOT_BE_HERE", "DO_NOT_SEND", "NOT_SET".
 	if isAllCapsPlaceholder(s) {
 		return true
 	}

internal/mcp/bridge_tool_test.go

@@ -131,6 +131,110 @@ func TestBridgeToolNaming(t *testing.T) {
 	}
 }
 
+func TestIsPlaceholderValue(t *testing.T) {
+	// Should be detected as placeholder.
+	placeholders := []string{
+		"null", "None", "nil", "UNDEFINED", "n/a",
+		"optional", "Optional", "OPTIONAL",
+		"skip", "Skip",
+		"__OMIT__", "__skip__", "__EMPTY__",
+		"http://example.com", "https://example.com",
+		"http://localhost", "https://localhost",
+		"PLACEHOLDER", "NOT_SET", "DO_NOT_SEND",
+	}
+	for _, s := range placeholders {
+		if !isPlaceholderValue(s) {
+			t.Errorf("expected isPlaceholderValue(%q) = true", s)
+		}
+	}
+
+	// Should NOT be detected as placeholder (real values).
+	realValues := []string{
+		"", // empty string handled separately by type-aware check
+		"sk-abc123",
+		"my-proxy.example.com",
+		"https://api.reviewweb.site/v1",
+		"gpt-4o-mini",
+		"bullet",
+		"hello world",
+		"ab", // too short for all-caps check
+	}
+	for _, s := range realValues {
+		if isPlaceholderValue(s) {
+			t.Errorf("expected isPlaceholderValue(%q) = false", s)
+		}
+	}
+}
+
+func TestStripEmptyOptionalArgs(t *testing.T) {
+	bt := &BridgeTool{
+		requiredSet: map[string]bool{"url": true},
+		inputSchema: map[string]any{
+			"type": "object",
+			"properties": map[string]any{
+				"url":      map[string]any{"type": "string"},
+				"api_key":  map[string]any{"type": "string"},
+				"timeout":  map[string]any{"type": "number"},
+				"debug":    map[string]any{"type": "boolean"},
+				"keywords": map[string]any{"type": "string"},
+			},
+		},
+	}
+
+	args := map[string]any{
+		"url":      "https://example.com",
+		"api_key":  "optional",    // placeholder → strip
+		"timeout":  nil,           // nil → strip
+		"debug":    true,          // real boolean → keep
+		"keywords": "",            // empty string for string-typed → keep
+	}
+
+	cleaned := bt.stripEmptyOptionalArgs(args)
+
+	if cleaned["url"] != "https://example.com" {
+		t.Error("required param 'url' should be preserved")
+	}
+	if _, ok := cleaned["api_key"]; ok {
+		t.Error("placeholder 'optional' should be stripped for api_key")
+	}
+	if _, ok := cleaned["timeout"]; ok {
+		t.Error("nil should be stripped for timeout")
+	}
+	if cleaned["debug"] != true {
+		t.Error("real boolean value should be preserved")
+	}
+	if v, ok := cleaned["keywords"]; !ok || v != "" {
+		t.Error("empty string should be kept for string-typed optional param 'keywords'")
+	}
+}
+
+func TestStripEmptyOptionalArgs_EmptyStringNonString(t *testing.T) {
+	bt := &BridgeTool{
+		requiredSet: map[string]bool{},
+		inputSchema: map[string]any{
+			"type": "object",
+			"properties": map[string]any{
+				"timeout": map[string]any{"type": "number"},
+				"count":   map[string]any{"type": "integer"},
+			},
+		},
+	}
+
+	args := map[string]any{
+		"timeout": "",
+		"count":   "",
+	}
+
+	cleaned := bt.stripEmptyOptionalArgs(args)
+
+	if _, ok := cleaned["timeout"]; ok {
+		t.Error("empty string should be stripped for number-typed param")
+	}
+	if _, ok := cleaned["count"]; ok {
+		t.Error("empty string should be stripped for integer-typed param")
+	}
+}
+
 func TestEnsureMCPPrefix(t *testing.T) {
 	tests := []struct {
 		name       string

internal/providers/schema_cleaner.go

@@ -6,6 +6,12 @@ func CleanToolSchemas(providerName string, tools []ToolDefinition) []ToolDefinit
 	if len(tools) == 0 {
 		return tools
 	}
+	profile := profileForProvider(providerName)
+	var strictPtr *bool
+	if profile.StrictToolMode {
+		t := true
+		strictPtr = &t
+	}
 	cleaned := make([]ToolDefinition, len(tools))
 	for i, t := range tools {
 		cleaned[i] = ToolDefinition{
@@ -14,6 +20,7 @@ func CleanToolSchemas(providerName string, tools []ToolDefinition) []ToolDefinit
 				Name:        t.Function.Name,
 				Description: t.Function.Description,
 				Parameters:  NormalizeSchema(providerName, t.Function.Parameters),
+				Strict:      strictPtr,
 			},
 		}
 	}

internal/providers/schema_normalize.go

@@ -39,6 +39,9 @@ func NormalizeSchema(providerName string, schema map[string]any) map[string]any
 	if len(profile.StripKeys) > 0 {
 		result = stripKeys(result, profile.StripKeys, 0)
 	}
+	if profile.StrictToolMode {
+		result = applyStrictMode(result, 0)
+	}
 	return result
 }
 

internal/providers/schema_normalize_test.go

@@ -52,7 +52,8 @@ func TestResolveRefs_Circular(t *testing.T) {
 			},
 		},
 	}
-	result := NormalizeSchema("openai", schema)
+	// Use "anthropic" to test ref resolution in isolation (no strict mode transform).
+	result := NormalizeSchema("anthropic", schema)
 	node := prop(result, "node")
 	if node == nil {
 		t.Fatal("expected node property")
@@ -77,7 +78,8 @@ func TestResolveRefs_LegacyDefinitions(t *testing.T) {
 			"Item": map[string]any{"type": "string"},
 		},
 	}
-	result := NormalizeSchema("openai", schema)
+	// Use "anthropic" to test ref resolution in isolation (no strict mode transform).
+	result := NormalizeSchema("anthropic", schema)
 	item := prop(result, "item")
 	if item == nil || item["type"] != "string" {
 		t.Error("expected definitions/ ref resolved to string type")
@@ -410,6 +412,123 @@ func TestNormalizeSchema_DoesNotMutateOriginal(t *testing.T) {
 	}
 }
 
+// ---------------------------------------------------------------------------
+// Strict tool mode
+// ---------------------------------------------------------------------------
+
+func TestApplyStrictMode_Basic(t *testing.T) {
+	schema := map[string]any{
+		"type": "object",
+		"properties": map[string]any{
+			"url":     map[string]any{"type": "string", "description": "The URL"},
+			"api_key": map[string]any{"type": "string", "description": "API key"},
+			"timeout": map[string]any{"type": "number", "description": "Timeout"},
+		},
+		"required": []any{"url"},
+	}
+	result := NormalizeSchema("openai", schema)
+
+	// All properties should be required.
+	reqArr, _ := result["required"].([]any)
+	reqSet := make(map[string]bool, len(reqArr))
+	for _, r := range reqArr {
+		reqSet[r.(string)] = true
+	}
+	for _, name := range []string{"url", "api_key", "timeout"} {
+		if !reqSet[name] {
+			t.Errorf("expected %q in required array", name)
+		}
+	}
+
+	// additionalProperties should be false.
+	if result["additionalProperties"] != false {
+		t.Error("expected additionalProperties:false")
+	}
+
+	// Required param 'url' should keep original type.
+	urlProp := prop(result, "url")
+	if urlProp["type"] != "string" {
+		t.Errorf("expected url type:string, got %v", urlProp["type"])
+	}
+
+	// Optional params should be nullable.
+	apiKeyProp := prop(result, "api_key")
+	apiKeyType, ok := apiKeyProp["type"].([]any)
+	if !ok {
+		t.Fatalf("expected api_key type to be array, got %T: %v", apiKeyProp["type"], apiKeyProp["type"])
+	}
+	hasNull := false
+	for _, v := range apiKeyType {
+		if v == "null" {
+			hasNull = true
+		}
+	}
+	if !hasNull {
+		t.Error("expected api_key type to include 'null'")
+	}
+
+	timeoutProp := prop(result, "timeout")
+	timeoutType, ok := timeoutProp["type"].([]any)
+	if !ok {
+		t.Fatalf("expected timeout type to be array, got %T", timeoutProp["type"])
+	}
+	hasNull = false
+	for _, v := range timeoutType {
+		if v == "null" {
+			hasNull = true
+		}
+	}
+	if !hasNull {
+		t.Error("expected timeout type to include 'null'")
+	}
+}
+
+func TestApplyStrictMode_NestedObject(t *testing.T) {
+	schema := map[string]any{
+		"type": "object",
+		"properties": map[string]any{
+			"config": map[string]any{
+				"type": "object",
+				"properties": map[string]any{
+					"key": map[string]any{"type": "string"},
+				},
+			},
+		},
+		"required": []any{},
+	}
+	result := NormalizeSchema("openai", schema)
+
+	// Nested object should also have additionalProperties:false.
+	config := prop(result, "config")
+	if config == nil {
+		t.Fatal("expected config property")
+	}
+	if config["additionalProperties"] != false {
+		t.Error("expected nested object to have additionalProperties:false")
+	}
+}
+
+func TestApplyStrictMode_SkipsAnthropic(t *testing.T) {
+	schema := map[string]any{
+		"type": "object",
+		"properties": map[string]any{
+			"url":    map[string]any{"type": "string"},
+			"debug":  map[string]any{"type": "boolean"},
+		},
+		"required": []any{"url"},
+	}
+	result := NormalizeSchema("anthropic", schema)
+
+	// Anthropic should NOT get strict mode transforms.
+	if result["additionalProperties"] == false {
+		t.Error("Anthropic should not have additionalProperties:false")
+	}
+	debugProp := prop(result, "debug")
+	if debugProp["type"] != "boolean" {
+		t.Error("Anthropic should keep original type (no nullable transform)")
+	}
+}
+
 // ---------------------------------------------------------------------------
 // Helpers
 // ---------------------------------------------------------------------------

internal/providers/schema_profile.go

@@ -13,6 +13,7 @@ type SchemaProfile struct {
 	StripNullType     bool     // anyOf:[T, null] → T
 	RemoveTypeOnUnion bool     // strip "type" when anyOf/oneOf present (Gemini conflict)
 	StripKeys         []string // keys to recursively remove
+	StrictToolMode    bool     // OpenAI strict mode: optional→nullable, all props required, additionalProperties:false
 }
 
 // Provider-specific strip key lists.
@@ -59,7 +60,15 @@ func profileForProvider(name string) SchemaProfile {
 			InjectObjectType: true,
 			StripKeys:        xaiStripKeys,
 		}
-	default: // openai, codex, openrouter, deepseek, groq, dashscope, etc.
+	case isOpenAIStrict(name):
+		return SchemaProfile{
+			ResolveRefs:      true,
+			FlattenUnions:    true,
+			InjectObjectType: true,
+			StrictToolMode:   true,
+			StripKeys:        refOnlyStripKeys,
+		}
+	default: // openrouter, deepseek, groq, dashscope, bailian, minimax, etc.
 		return SchemaProfile{
 			ResolveRefs:      true,
 			FlattenUnions:    true,
@@ -69,6 +78,14 @@ func profileForProvider(name string) SchemaProfile {
 	}
 }
 
+// isOpenAIStrict returns true for providers known to support strict tool mode.
+// Only first-party OpenAI and Codex are safe — third-party proxies (OpenRouter,
+// DeepSeek, Groq, DashScope, etc.) may reject nullable unions or the strict flag.
+func isOpenAIStrict(name string) bool {
+	lower := strings.ToLower(name)
+	return lower == "openai" || lower == "codex"
+}
+
 // isGeminiName matches config names ("gemini", "gemini-flash") and
 // DB provider types ("gemini_native"). Uses Contains for robustness
 // with user-defined names (e.g. "my-gemini-proxy").

internal/providers/schema_strict.go

@@ -0,0 +1,116 @@
+package providers
+
+// schema_strict.go — OpenAI strict tool mode transform.
+// Converts optional properties to nullable unions, requires all properties,
+// and sets additionalProperties:false so constrained decoding can enforce schema compliance.
+
+// applyStrictMode transforms a tool's JSON Schema for OpenAI strict mode.
+// - Optional properties (not in "required") get their type changed to ["<type>", "null"]
+// - ALL property names are added to "required"
+// - "additionalProperties": false is set on object schemas
+// This is applied recursively to nested object schemas.
+func applyStrictMode(schema map[string]any, depth int) map[string]any {
+	if schema == nil || depth > maxSchemaDepth {
+		return schema
+	}
+
+	typ, _ := schema["type"].(string)
+	props, hasProps := schema["properties"].(map[string]any)
+
+	if typ != "object" || !hasProps {
+		return schema
+	}
+
+	// Build the set of currently required properties.
+	reqSet := make(map[string]bool)
+	if reqArr, ok := schema["required"].([]any); ok {
+		for _, r := range reqArr {
+			if s, ok := r.(string); ok {
+				reqSet[s] = true
+			}
+		}
+	}
+	if reqArr, ok := schema["required"].([]string); ok {
+		for _, s := range reqArr {
+			reqSet[s] = true
+		}
+	}
+
+	// Collect all property names for the new required array.
+	allRequired := make([]any, 0, len(props))
+
+	for name, prop := range props {
+		allRequired = append(allRequired, name)
+
+		pm, ok := prop.(map[string]any)
+		if !ok {
+			continue
+		}
+
+		// Recurse into nested objects first.
+		pm = applyStrictMode(pm, depth+1)
+		props[name] = pm
+
+		// Recurse into array items.
+		if items, ok := pm["items"].(map[string]any); ok {
+			pm["items"] = applyStrictMode(items, depth+1)
+		}
+
+		// Already required — no need to make nullable.
+		if reqSet[name] {
+			continue
+		}
+
+		// Make optional property nullable: type:"string" → type:["string","null"]
+		makeNullable(pm)
+	}
+
+	schema["properties"] = props
+	schema["required"] = allRequired
+	schema["additionalProperties"] = false
+
+	return schema
+}
+
+// makeNullable converts a property schema to accept null values.
+// - Simple type: "string" → ["string", "null"]
+// - Type array: ["string", "integer"] → ["string", "integer", "null"]
+// - anyOf/oneOf: appends {"type":"null"} variant
+// - No type: adds type:["null"] (fallback)
+func makeNullable(schema map[string]any) {
+	// Already has null variant in anyOf/oneOf — skip.
+	for _, key := range []string{"anyOf", "oneOf"} {
+		if variants, ok := schema[key].([]any); ok {
+			for _, v := range variants {
+				if m, ok := v.(map[string]any); ok && isNullSchema(m) {
+					return // already nullable
+				}
+			}
+			// Append null variant.
+			schema[key] = append(variants, map[string]any{"type": "null"})
+			return
+		}
+	}
+
+	typ, hasType := schema["type"]
+
+	switch t := typ.(type) {
+	case string:
+		if t == "null" {
+			return // already null
+		}
+		schema["type"] = []any{t, "null"}
+	case []any:
+		for _, v := range t {
+			if s, ok := v.(string); ok && s == "null" {
+				return // already has null
+			}
+		}
+		schema["type"] = append(t, "null")
+	default:
+		if !hasType {
+			// No type field — add nullable object as fallback.
+			schema["type"] = []any{"object", "null"}
+		}
+	}
+}

internal/providers/types.go

@@ -160,6 +160,7 @@ type ToolFunctionSchema struct {
 	Name        string         `json:"name"`
 	Description string         `json:"description"`
 	Parameters  map[string]any `json:"parameters"`
+	Strict      *bool          `json:"strict,omitempty"` // OpenAI strict mode — constrained decoding
 }
 
 // Usage tracks token consumption.