fix(mcp): stop stripping all-caps business identifiers from tool arguments

isAllCapsPlaceholder() classified ANY string of >=3 chars made only of A-Z and
underscore as an LLM placeholder, and stripEmptyOptionalArgs() then removed it
from the outbound CallTool arguments. Real business identifiers match that shape:

  bank codes      BKASH, NAGAD, ROCKET, VCB
  currency codes  BDT, VND, USD, PKR, MMK
  enum values     SUCCESS, PENDING, ALL

Impact is silent and severe: dropping an optional filter turns a filtered query
into an unfiltered one. The MCP server still answers 200 with the full result set,
so nothing errors — the model reports totals for the entire dataset as if they were
the filtered result, with full confidence.

Observed in production: asking "how many BKASH proxies are active" returned 6,610
(all banks) instead of 1,263. Three different bank codes returned byte-identical
results, which also drove the model into a retry loop until the loop guard fired.

Diagnosis was slow because the removal is invisible: the outbound log records
args_len BEFORE stripping, and stripEmptyOptionalArgs logs nothing at all — while
normalizeArgsForSchema right below it does emit mcp.tool.args.coerced. Confirmed
the argument never reaches the server by echoing the raw JSON-RPC arguments from
RequestContext[CallToolRequestParams] on the server side.

Changes:
- isAllCapsPlaceholder: require multi-word (contains "_") or membership in a known
  single-word placeholder list. Keeps the original intent — the doc comment's own
  examples (SHOULD_NOT_BE_HERE, DO_NOT_SEND, NOT_APPLICABLE) are all multi-word —
  while letting business codes through.
- stripEmptyOptionalArgs: emit slog.Warn("mcp.tool.args.stripped") whenever an
  argument is dropped, so this class of bug is one grep away instead of a
  multi-hour investigation.
- Tests: guard both layers against regression.

Verified: go build ./... clean; go test ./internal/mcp/ passes.

internal/mcp/bridge_tool.go

@@ -370,12 +370,18 @@ func (t *BridgeTool) stripEmptyOptionalArgs(args map[string]any) map[string]any
 		}
 		if s, ok := v.(string); ok {
 			// Strip known placeholder values (e.g. "optional", "null", "http://example.com").
+			// Logged: dropping an argument silently changes query semantics (a filtered
+			// call becomes unfiltered) and is otherwise invisible in traces.
 			if isPlaceholderValue(s) {
+				slog.Warn("mcp.tool.args.stripped",
+					"tool", t.registeredName, "arg", k, "value", s, "reason", "placeholder")
 				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" {
+				slog.Warn("mcp.tool.args.stripped",
+					"tool", t.registeredName, "arg", k, "reason", "empty_string_non_string_param")
 				continue
 			}
 		}
@@ -441,17 +447,40 @@ func detectLogicalErrorPayload(text string) (string, bool) {
 
 // isAllCapsPlaceholder detects LLM-generated all-caps placeholder strings
 // like "SHOULD_NOT_BE_HERE", "DO_NOT_SEND", "NOT_APPLICABLE", "PLACEHOLDER".
+//
+// NOTE: being all-caps is NOT sufficient. Real business identifiers are commonly
+// all-caps: bank codes (BKASH, NAGAD, ROCKET), currency codes (BDT, VND, USD),
+// country codes, ticker symbols, enum values (SUCCESS, PENDING). Silently dropping
+// those turns a filtered query into an unfiltered one — the MCP server still returns
+// 200 with the full result set, so neither the model nor the operator notices, and
+// the model then reports a wrong number with full confidence.
+//
+// Only treat a value as a placeholder when it is multi-word (contains "_") or matches
+// a known single-word placeholder.
 func isAllCapsPlaceholder(s string) bool {
 	trimmed := strings.TrimSpace(s)
 	if len(trimmed) < 3 {
 		return false
 	}
+	hasUnderscore := false
 	for _, r := range trimmed {
-		if r != '_' && (r < 'A' || r > 'Z') {
+		if r == '_' {
+			hasUnderscore = true
+			continue
+		}
+		if r < 'A' || r > 'Z' {
 			return false
 		}
 	}
-	return true
+	if hasUnderscore {
+		return true
+	}
+	switch trimmed {
+	case "PLACEHOLDER", "TODO", "TBD", "XXX", "NONE", "NULL", "UNKNOWN",
+		"VALUE", "STRING", "EXAMPLE", "CHANGEME", "REPLACE", "OMIT", "SKIP":
+		return true
+	}
+	return false
 }
 
 // wrapMCPContent wraps MCP tool results as external/untrusted content.

internal/mcp/bridge_tool_test.go

@@ -483,3 +483,66 @@ func TestEnsureMCPPrefix(t *testing.T) {
 		})
 	}
 }
+
+// TestIsAllCapsPlaceholder_KeepsBusinessCodes guards a production incident:
+// all-caps business identifiers (bank codes, currency codes, enum values) were
+// classified as LLM placeholders and silently dropped from tool arguments, so a
+// filtered query silently became an unfiltered one and the model reported totals
+// for the whole dataset as if they were the filtered result.
+func TestIsAllCapsPlaceholder_KeepsBusinessCodes(t *testing.T) {
+	keep := []string{
+		"BKASH", "NAGAD", "ROCKET", "ROCKETC", // bank codes
+		"BDT", "VND", "USD", "PKR", "MMK", // currency codes
+		"SUCCESS", "PENDING", "ALL", // enum values
+		"VCB", "SPACEPROXY",
+	}
+	for _, s := range keep {
+		if isAllCapsPlaceholder(s) {
+			t.Errorf("business identifier %q must NOT be treated as a placeholder", s)
+		}
+	}
+
+	drop := []string{
+		"SHOULD_NOT_BE_HERE", "DO_NOT_SEND", "NOT_APPLICABLE", // multi-word
+		"PLACEHOLDER", "TODO", "TBD", "XXX", "CHANGEME", // known single-word
+	}
+	for _, s := range drop {
+		if !isAllCapsPlaceholder(s) {
+			t.Errorf("placeholder %q must be detected", s)
+		}
+	}
+}
+
+// TestStripEmptyOptionalArgs_KeepsAllCapsBusinessCode is the end-to-end guard for
+// the same incident, at the layer that actually mutates the outbound arguments.
+func TestStripEmptyOptionalArgs_KeepsAllCapsBusinessCode(t *testing.T) {
+	bt := &BridgeTool{
+		requiredSet: map[string]bool{"merchant_id": true},
+		inputSchema: map[string]any{
+			"type": "object",
+			"properties": map[string]any{
+				"merchant_id": map[string]any{"type": "string"},
+				"bank_code":   map[string]any{"type": "string"},
+				"currency":    map[string]any{"type": "string"},
+				"note":        map[string]any{"type": "string"},
+			},
+		},
+	}
+
+	cleaned := bt.stripEmptyOptionalArgs(map[string]any{
+		"merchant_id": "CPS_C1",
+		"bank_code":   "BKASH",              // optional all-caps business code → keep
+		"currency":    "BDT",                // optional all-caps business code → keep
+		"note":        "SHOULD_NOT_BE_HERE", // real placeholder → strip
+	})
+
+	if cleaned["bank_code"] != "BKASH" {
+		t.Errorf("bank_code must survive, got %v", cleaned["bank_code"])
+	}
+	if cleaned["currency"] != "BDT" {
+		t.Errorf("currency must survive, got %v", cleaned["currency"])
+	}
+	if _, ok := cleaned["note"]; ok {
+		t.Error("multi-word all-caps placeholder should still be stripped")
+	}
+}