fix(mcp): transform raw schema validation errors into helpful "Did you mean?" messages

docs/adr/29406-mcp-receiving-middleware-for-validation-error-messages.md

@@ -0,0 +1,88 @@
+# ADR-29406: MCP Receiving Middleware for Helpful Validation Error Messages
+
+**Date**: 2026-05-01
+**Status**: Draft
+**Deciders**: pelikhan
+
+---
+
+## Part 1 — Narrative (Human-Friendly)
+
+### Context
+
+The MCP server enforces strict JSON schema validation on tool call arguments using `additionalProperties: false`. When a caller passes an unknown parameter (e.g., `workflow-name` instead of `workflows`), the underlying `jsonschema-go` library produces a raw, internal error message such as `validating "arguments": validating root: unexpected additional properties ["workflow-name"]`. This opaque message is surfaced directly to the MCP client with no guidance on the correct parameter name, degrading the developer experience for all 10 registered tools. The MCP Go SDK provides a middleware hook (`AddReceivingMiddleware`) that can intercept tool results at the server boundary before they are returned to the caller.
+
+### Decision
+
+We will implement a generic MCP receiving middleware (`argumentValidationMiddleware`) that intercepts `CallToolResult` errors containing "unexpected additional properties" and replaces them with a human-readable message including the unknown parameter name, a "Did you mean?" suggestion derived from a longest-common-prefix fuzzy matcher, and a pointer to the tool's `--help` output. A hardcoded per-tool parameter registry (`mcpToolParams()`) maintains the list of valid parameter names for each tool, linked by comment to the source-of-truth `*Args` struct in each `register*Tool` function.
+
+### Alternatives Considered
+
+#### Alternative 1: Patch the upstream JSON schema or MCP SDK
+
+The `jsonschema-go` validation error format is controlled by the upstream library; customizing it would require forking or monkey-patching a third-party dependency. This was rejected because it introduces maintenance overhead for upstream changes and violates the project's policy of minimizing fork-based patches to dependencies.
+
+#### Alternative 2: Per-tool error handling at each registration site
+
+Each `register*Tool` function could wrap its handler to detect and reformat validation errors locally. While this avoids a central registry, it means duplicating the detection and formatting logic across ten tools and increases the risk of inconsistent messaging. The single middleware with a shared registry was preferred for uniformity and locality of change.
+
+#### Alternative 3: Levenshtein edit-distance fuzzy matching
+
+Full edit-distance similarity (e.g., via `golang.org/x/text` or a dedicated library) would handle transpositions and mid-string substitutions that longest-common-prefix cannot. It was considered but rejected for the typical case: MCP callers most often pass hyphen/underscore variants of a correct name (e.g., `workflow-name` for `workflows`), which normalization plus LCP handles reliably without an additional dependency.
+
+### Consequences
+
+#### Positive
+- All ten MCP tools immediately get helpful error messages without changes to their individual implementations.
+- Normalization (strip hyphens and underscores, lowercase) correctly resolves the most common error pattern — delimiter mismatch — before prefix comparison.
+- The middleware is transparent: non-validation errors and successful results pass through unchanged, so no existing behavior is affected.
+
+#### Negative
+- `mcpToolParams()` is a manually maintained registry; if a parameter is added or renamed in a `*Args` struct without updating this file, the suggestion will become stale or absent.
+- LCP-based similarity does not catch mid-string differences or transpositions (e.g., `workflwo` → `workflows`); callers who mistype in those ways receive no suggestion.
+
+#### Neutral
+- The middleware is registered once at server construction time, so its cost is one closure allocation; per-call overhead is limited to a single string comparison in the non-matching fast path.
+- A maintenance comment block in `mcpToolParams()` links each entry to its source-of-truth registration function to reduce the risk of the registry drifting.
+
+---
+
+## Part 2 — Normative Specification (RFC 2119)
+
+> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119).
+
+### Middleware Registration
+
+1. The MCP server **MUST** register `argumentValidationMiddleware` as a receiving middleware via `AddReceivingMiddleware` after all tools have been registered.
+2. The middleware **MUST** be initialized with the result of `mcpToolParams()` at server construction time.
+3. The middleware **MUST NOT** modify results for MCP methods other than `tools/call`.
+4. The middleware **MUST NOT** modify `CallToolResult` values where `IsError` is `false`.
+
+### Error Interception and Transformation
+
+1. The middleware **MUST** detect tool-call errors whose text content contains the substring `unexpected additional properties`.
+2. The middleware **MUST** extract all unknown parameter names from the error message using the `extractUnknownParams` parser.
+3. For each unknown parameter, the middleware **MUST** emit an `Unknown parameter '<name>'.` prefix in the replacement message.
+4. If a valid parameter whose normalized form shares a longest-common-prefix ratio ≥ 0.70 with the unknown parameter exists, the middleware **MUST** append `Did you mean '<suggestion>'?` to that parameter's line.
+5. If the tool name is known, the middleware **MUST** append a `Run 'agenticworkflows <tool> --help' for usage.` line.
+6. The replacement `CallToolResult` **MUST** preserve `IsError: true` and **MUST** contain exactly one `TextContent` element with the replacement message.
+
+### Parameter Registry (`mcpToolParams`)
+
+1. The `mcpToolParams()` registry **MUST** contain an entry for every tool registered in `createMCPServer`.
+2. Each entry **MUST** list all valid JSON parameter names derived from the corresponding `*Args` struct's `json` tags.
+3. Parameter lists **MUST** be kept in sorted order to produce deterministic suggestion output.
+4. When a tool's `*Args` struct gains, removes, or renames a parameter, the corresponding `mcpToolParams()` entry **MUST** be updated in the same commit.
+
+### Normalization
+
+1. Parameter name normalization **MUST** lowercase the name and remove all hyphen (`-`) and underscore (`_`) characters before comparison.
+2. An exact match after normalization **SHALL** be preferred over any prefix-score comparison and **MUST** return the matching valid parameter immediately.
+
+### Conformance
+
+An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance.
+
+---
+
+*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/25196075860) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*

pkg/cli/mcp_argument_validation.go

@@ -0,0 +1,289 @@
+package cli
+
+// This file implements an MCP receiving middleware that transforms raw JSON schema
+// "additional properties" validation errors into helpful, user-friendly messages
+// with "Did you mean?" suggestions.
+//
+// Background: When the MCP SDK validates tool arguments against the input schema
+// (which uses additionalProperties: false), it emits a raw message like:
+//
+//	validating "arguments": validating root: unexpected additional properties ["workflow-name"]
+//
+// This is surfaced directly to users, leaking internal validation details without
+// any guidance on the correct parameter name.  The middleware here intercepts
+// those tool-error results and replaces the message with a helpful alternative.
+
+import (
+	"context"
+	"fmt"
+	"regexp"
+	"sort"
+	"strings"
+
+	"github.com/modelcontextprotocol/go-sdk/mcp"
+)
+
+// toolParamEntry holds the valid parameter names for a single MCP tool.
+type toolParamEntry = []string
+
+// argumentValidationMiddleware returns a mcp.Middleware that intercepts tool-call
+// results containing "unexpected additional properties" validation errors and
+// replaces them with a helpful message that names the unknown parameter, suggests
+// a close match, and points to the tool's --help output.
+//
+// toolParams maps tool names to their list of valid JSON parameter names.  It is
+// provided by the caller as a hardcoded registry (see mcpToolParams).
+func argumentValidationMiddleware(toolParams map[string]toolParamEntry) mcp.Middleware {
+	return func(next mcp.MethodHandler) mcp.MethodHandler {
+		return func(ctx context.Context, method string, req mcp.Request) (mcp.Result, error) {
+			result, err := next(ctx, method, req)
+			if err != nil || method != "tools/call" {
+				return result, err
+			}
+
+			// Check whether the result is a tool error containing a schema
+			// "additional properties" validation message.
+			toolResult, ok := result.(*mcp.CallToolResult)
+			if !ok || !toolResult.IsError {
+				return result, err
+			}
+
+			// Extract the error text from the first TextContent element.
+			if len(toolResult.Content) == 0 {
+				return result, err
+			}
+			textContent, ok := toolResult.Content[0].(*mcp.TextContent)
+			if !ok {
+				return result, err
+			}
+			errMsg := textContent.Text
+
+			if !strings.Contains(errMsg, "unexpected additional properties") {
+				return result, err
+			}
+
+			// Parse the unknown parameter names from the error text.
+			unknownParams := extractUnknownParams(errMsg)
+			if len(unknownParams) == 0 {
+				return result, err
+			}
+
+			// Determine the tool name from the request so we can look up valid params.
+			toolName := extractMCPToolName(req)
+			validParams := toolParams[toolName]
+
+			// Build a helpful replacement message.
+			helpMsg := buildHelpfulParamError(toolName, unknownParams, validParams)
+
+			// Return a modified tool result with the helpful message, preserving IsError.
+			replaced := *toolResult
+			replaced.Content = []mcp.Content{&mcp.TextContent{Text: helpMsg}}
+			return &replaced, nil
+		}
+	}
+}
+
+// extractMCPToolName retrieves the tool name from a MCP Request by casting the
+// request params to *mcp.CallToolParamsRaw.  Returns an empty string if the cast
+// fails.
+func extractMCPToolName(req mcp.Request) string {
+	if p, ok := req.GetParams().(*mcp.CallToolParamsRaw); ok {
+		return p.Name
+	}
+	return ""
+}
+
+// extractUnknownParams parses the JSON-schema validation error string to extract
+// the list of unknown parameter names.
+//
+// The expected format (from jsonschema-go) is:
+//
+//	unexpected additional properties ["name1" "name2"]
+//
+// which uses %q-style quoting for a []string.
+var additionalPropsRE = regexp.MustCompile(`unexpected additional properties (.+)$`)
+var quotedStringRE = regexp.MustCompile(`"([^"]+)"`)
+
+func extractUnknownParams(errMsg string) []string {
+	m := additionalPropsRE.FindStringSubmatch(errMsg)
+	if m == nil {
+		return nil
+	}
+	raw := m[1]
+	matches := quotedStringRE.FindAllStringSubmatch(raw, -1)
+	var params []string
+	for _, sm := range matches {
+		if sm[1] != "" {
+			params = append(params, sm[1])
+		}
+	}
+	return params
+}
+
+// buildHelpfulParamError constructs a human-readable error message that:
+//   - names each unknown parameter
+//   - suggests the closest valid parameter (if a good match is found)
+//   - directs the user to the tool's --help output
+func buildHelpfulParamError(toolName string, unknownParams []string, validParams []string) string {
+	var sb strings.Builder
+
+	for i, param := range unknownParams {
+		if i > 0 {
+			sb.WriteString("\n")
+		}
+		sb.WriteString(fmt.Sprintf("Unknown parameter '%s'.", param))
+		if suggestion := findSimilarParam(param, validParams); suggestion != "" {
+			sb.WriteString(fmt.Sprintf(" Did you mean '%s'?", suggestion))
+		}
+	}
+
+	if toolName != "" {
+		sb.WriteString(fmt.Sprintf("\nRun 'agenticworkflows %s --help' for usage.", toolName))
+	}
+
+	return sb.String()
+}
+
+// findSimilarParam returns the valid parameter name most similar to unknown, or
+// an empty string if no parameter is close enough.
+//
+// Similarity is measured by the ratio of the longest-common-prefix length to the
+// shorter of the two normalized strings.  A threshold of 0.7 (70%) is required.
+//
+// Normalization: lowercase, hyphens and underscores removed.
+func findSimilarParam(unknown string, validParams []string) string {
+	if len(validParams) == 0 {
+		return ""
+	}
+
+	normUnknown := normalizeParamName(unknown)
+
+	type candidate struct {
+		name  string
+		score float64
+	}
+	var best candidate
+
+	for _, p := range validParams {
+		normP := normalizeParamName(p)
+
+		// Exact normalized match wins immediately.
+		if normP == normUnknown {
+			return p
+		}
+
+		lcp := longestCommonPrefixLen(normUnknown, normP)
+		shorter := min(len(normP), len(normUnknown))
+		if shorter == 0 {
+			continue
+		}
+		score := float64(lcp) / float64(shorter)
+		if score > best.score {
+			best = candidate{name: p, score: score}
+		}
+	}
+
+	const threshold = 0.7
+	if best.score >= threshold {
+		return best.name
+	}
+	return ""
+}
+
+// normalizeParamName lowercases name and removes hyphens and underscores, so
+// that "workflow-name", "workflow_name", and "workflowname" all compare equal.
+func normalizeParamName(name string) string {
+	name = strings.ToLower(name)
+	name = strings.ReplaceAll(name, "-", "")
+	name = strings.ReplaceAll(name, "_", "")
+	return name
+}
+
+// longestCommonPrefixLen returns the length of the longest common prefix of a
+// and b.
+func longestCommonPrefixLen(a, b string) int {
+	n := min(len(a), len(b))
+	for i := range n {
+		if a[i] != b[i] {
+			return i
+		}
+	}
+	return n
+}
+
+// mcpToolParams returns the registry of valid parameter names for every tool
+// registered in the MCP server.  This is called once during server construction
+// and the result is passed to argumentValidationMiddleware.
+//
+// The map key is the MCP tool name; the value is the sorted list of valid JSON
+// parameter names taken from the corresponding *Args struct json tags.
+//
+// MAINTENANCE: When tool parameters change, update the corresponding entry here
+// to match the json tags on the *Args struct in register*Tool functions:
+//   - status       → registerStatusTool   in mcp_tools_readonly.go  (statusArgs)
+//   - compile      → registerCompileTool  in mcp_tools_readonly.go  (compileArgs)
+//   - logs         → registerLogsTool     in mcp_tools_privileged.go (logsArgs)
+//   - audit        → registerAuditTool    in mcp_tools_privileged.go (auditArgs)
+//   - audit-diff   → registerAuditDiffTool in mcp_tools_privileged.go (auditDiffArgs)
+//   - checks       → registerChecksTool   in mcp_tools_readonly.go  (checksArgs)
+//   - mcp-inspect  → registerMCPInspectTool in mcp_tools_readonly.go (mcpInspectArgs)
+//   - add          → registerAddTool      in mcp_tools_management.go (addArgs)
+//   - update       → registerUpdateTool   in mcp_tools_management.go (updateArgs)
+//   - fix          → registerFixTool      in mcp_tools_management.go (fixArgs)
+func mcpToolParams() map[string]toolParamEntry {
+	params := map[string]toolParamEntry{
+		// statusArgs in mcp_tools_readonly.go
+		"status": {
+			"pattern",
+		},
+		// compileArgs in mcp_tools_readonly.go
+		"compile": {
+			"workflows", "strict", "zizmor", "poutine", "actionlint",
+			"runner-guard", "fix", "max_tokens",
+		},
+		// logsArgs in mcp_tools_privileged.go
+		"logs": {
+			"workflow_name", "count", "start_date", "end_date", "engine",
+			"firewall", "no_firewall", "filtered_integrity", "branch",
+			"after_run_id", "before_run_id", "timeout", "max_tokens", "artifacts",
+		},
+		// auditArgs in mcp_tools_privileged.go
+		"audit": {
+			"run_id_or_url", "run_ids_or_urls", "artifacts", "max_tokens",
+		},
+		// auditDiffArgs in mcp_tools_privileged.go
+		"audit-diff": {
+			"base_run_id", "compare_run_ids", "artifacts",
+		},
+		// checksArgs in mcp_tools_readonly.go
+		"checks": {
+			"pr_number", "repo",
+		},
+		// mcpInspectArgs in mcp_tools_readonly.go
+		"mcp-inspect": {
+			"workflow_file", "server", "tool",
+		},
+		// addArgs in mcp_tools_management.go
+		"add": {
+			"workflows", "number", "name",
+		},
+		// updateArgs in mcp_tools_management.go
+		"update": {
+			"workflows", "major", "force",
+		},
+		// fixArgs in mcp_tools_management.go
+		"fix": {
+			"workflows", "write", "list_codemods",
+		},
+	}
+
+	// Sort each list for deterministic output in suggestions.
+	for k, v := range params {
+		sorted := make([]string, len(v))
+		copy(sorted, v)
+		sort.Strings(sorted)
+		params[k] = sorted
+	}
+
+	return params
+}