From f68d608b26ce14eabe5c4fafce613fd222a5b855 Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 25 Jul 2026 21:34:50 -0400 Subject: [PATCH] fix(validate): accept hyphenated MCP tool names in allow/deny lists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mcpToolNamePattern rejected "-", but hyphenated tool names are the MCP ecosystem norm and match the spec's SHOULD pattern (^[a-zA-Z0-9_-]{1,128}$). Field case: an agent built with Calendly's registry ops crash-looped at startup — ERROR: mcp.servers[0].tools.allow[0]: invalid tool name "event_types-get_event_type" — because the allow-list carries the server's genuine tool names, which the platform cannot rename without breaking tools/call. The "__" namespace-separator reservation is unchanged (separate check, now pinned by its own test); a single "-" poses no conflict with the "__" runtime form. Length cap raised 64 → 128 to match the spec pattern. Co-Authored-By: Claude Fable 5 --- forge-core/validate/mcp_config.go | 13 +++++++++---- forge-core/validate/mcp_config_test.go | 22 +++++++++++++++++++--- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/forge-core/validate/mcp_config.go b/forge-core/validate/mcp_config.go index f0cce73..a331182 100644 --- a/forge-core/validate/mcp_config.go +++ b/forge-core/validate/mcp_config.go @@ -15,10 +15,15 @@ import ( // ≤31 chars. var mcpServerNamePattern = regexp.MustCompile(`^[a-z][a-z0-9-]{0,30}$`) -// MCP tool name format. Allowed in Tools.Allow / Tools.Deny. Names -// containing "__" are reserved for the namespaced runtime form -// "__"; here we only validate the bare tool name. -var mcpToolNamePattern = regexp.MustCompile(`^[a-zA-Z0-9_]{1,64}$`) +// MCP tool name format. Allowed in Tools.Allow / Tools.Deny. Matches the +// MCP spec's SHOULD pattern for tool names (`^[a-zA-Z0-9_-]{1,128}$`) — +// real servers expose hyphenated names (Calendly's users-get_current_user, +// event_types-list_event_types, …), and rejecting the hyphen bricked any +// agent at startup whose allow-list carried the server's genuine tool +// names. Names containing "__" are reserved for the namespaced runtime +// form "__" and rejected by a separate check; single "-" +// or "_" pose no conflict with that separator. +var mcpToolNamePattern = regexp.MustCompile(`^[a-zA-Z0-9_-]{1,128}$`) // knownMCPTransports is the closed set of accepted transport values. // Phase 1: HTTP only. Stdio is on the roadmap. diff --git a/forge-core/validate/mcp_config_test.go b/forge-core/validate/mcp_config_test.go index 40975e0..441c08e 100644 --- a/forge-core/validate/mcp_config_test.go +++ b/forge-core/validate/mcp_config_test.go @@ -234,12 +234,28 @@ func TestValidateMCPConfig(t *testing.T) { wantErrs: []string{"invalid tool name"}, }, { - name: "deny tool name with hyphen rejected", + // Hyphenated names are the MCP-ecosystem norm (Calendly: + // users-get_current_user, event_types-list_event_types, …) + // and match the spec's SHOULD pattern. Rejecting them bricked + // agents at startup whose allow-lists carried the server's + // real tool names. + name: "tool names with hyphens accepted", cfg: types.MCPConfig{Servers: []types.MCPServer{{ Name: "x", Transport: "http", URL: "http://x", - Tools: types.MCPToolFilter{Allow: []string{"good"}, Deny: []string{"bad-tool"}}, + Tools: types.MCPToolFilter{ + Allow: []string{"users-get_current_user", "scheduling_links-create_single_use_scheduling_link"}, + Deny: []string{"meetings-cancel_event"}, + }, }}}, - wantErrs: []string{"invalid tool name"}, + wantErrs: nil, + }, + { + name: "tool name with double underscore still rejected", + cfg: types.MCPConfig{Servers: []types.MCPServer{{ + Name: "x", Transport: "http", URL: "http://x", + Tools: types.MCPToolFilter{Allow: []string{"foo__bar"}}, + }}}, + wantErrs: []string{"reserved for MCP namespacing"}, }, { name: "tool in both allow and deny",