Skip to content

Commit 78ea28e

Browse files
authored
First-class tool names: (mcp.v1.tool_name) option + Gemini-safe mangling (#46)
* gen: force mangled tool names to start with a letter Providers disagree on the leading character of a function name: Gemini requires a letter or underscore, OpenAI and Anthropic also accept digits. The base-36 hash prefix of a mangled name can start with a digit, and Gemini then rejects the entire request with "Invalid function name" -- a single over-long proto method bricks every Gemini-backed agent using the server (live failure: morningstar-securities GetEquityResearchReport, 2026-06-10). Map a leading digit onto 'g'..'p'. Hashes already starting with a letter are unchanged, so only the names that were broken on Gemini change. * gen: add (mcp.v1.tool_name) option for first-class tool names Derived tool names are an encoding of proto full names, not names a model should read: 40+ characters of package prefix, hash-truncated when too long. This makes high-fidelity names a first-class feature: rpc GetEquityResearchReport(...) returns (...) { option (mcp.v1.tool_name) = "get_equity_research_report"; } The option affects only the MCP tool layer -- runtime.Tool.Name in generated registration code and in dynamic RegisterService. Generated Go identifiers stay proto-derived. Unset means the derived name, exactly as before. Guard rails: - the value must match ^[a-zA-Z_][a-zA-Z0-9_-]{0,63}$, the intersection of every provider's function-name rules; codegen fails on an invalid value. - duplicate tool names fail codegen across the whole generation run (one ToolNameRegistry per plugin invocation). Derived names are unique by construction, so duplicates can only come from the option. - the dynamic path falls back to the derived name on an invalid or colliding configured name: a gateway must keep serving, and derived names are always safe. With MCPs declaring their own short names, consumers that prefix tools (agent runtimes prefix "mcpname__") stop running into length mangling, and models get names worth reading. * gen: harden tool_name option per Codex review - A configured name squatting on a later method's derived name made the dynamic-path fallback recompute that same derived name and call AddTool twice with it. The fallback now de-collides with a numeric suffix; AddTool is never called twice with one name. - An explicitly authored (mcp.v1.tool_name) = "" was indistinguishable from an unset option and slipped past codegen validation. ConfiguredToolName now reports presence separately (HasExtension), so the generator rejects it. - Tighten validation to lowercase: ^[a-z_][a-z0-9_-]{0,63}$. Derived and mangled names always contain an UpperCamel "<Service>Service_" segment, so a lowercase-only configured name can never be mistaken for one by downstream de-mangling heuristics (aigw's toolname.Short keys on exactly that segment) -- configured names provably pass through them untouched. * gen: regenerate annotations.pb.go for the lowercase tool-name rule The proto source comment was tightened to ^[a-z_][a-z0-9_-]{0,63}$ but the generated file still carried the old uppercase-permitting doc -- the extension docs are the public API surface and must not contradict the validator. Caught by review. * gen: golden-test tool naming, scope dup guard per service Add testdata/tool_naming.proto to the golden generation test, pinning all three naming behaviors in emitted .pb.mcp.go output: a configured (mcp.v1.tool_name) emitted verbatim ("get_dashboard"), a derived name unchanged, and a >64-char name hash-truncated with a letter-leading prefix. Existing goldens are byte-identical, proving the default path did not move. The annotations proto is vendored into the testdata buf module (buf modules cannot depend on local sibling dirs); its Go output is suppressed so mcp/v1/annotations.proto registers exactly once (pkg/mcpv1). Re-scope the duplicate-name guard from per-run to per-service and drop ToolNameRegistry. A plugin run's file set is not a deployment unit: in a monorepo every MCP proto generates in one run, and two different MCPs may legitimately both name a tool "get_dashboard" -- they never share a server. The only statically decidable collision is within one service (one Register<Service>Handler call), which is where the guard now lives.
1 parent 954bf16 commit 78ea28e

26 files changed

Lines changed: 1720 additions & 6 deletions

README.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,22 @@ sqlv1mcp.RegisterSQLServiceHandler(s, clickhouseHandler, runtime.WithNamePrefix(
162162
// Tools: postgres_SQLService_Query, clickhouse_SQLService_Query, ...
163163
```
164164

165+
### Tool naming
166+
167+
By default a tool is named after the method's full proto name with dots replaced by underscores (`redpanda_mcps_sql_v1_SQLService_Query`). Names longer than 64 characters — the strictest provider limit — are truncated: a 10-character hash of the full name plus as much of the tail (the most specific part) as fits. The hash is forced to start with a letter because Gemini rejects function names that start with a digit.
168+
169+
To publish a method under a stable, human-chosen name instead, set the `(mcp.v1.tool_name)` method option:
170+
171+
```proto
172+
import "mcp/v1/annotations.proto";
173+
174+
rpc GetEquityResearchReport(GetEquityResearchReportRequest) returns (GetEquityResearchReportResponse) {
175+
option (mcp.v1.tool_name) = "get_equity_research_report";
176+
}
177+
```
178+
179+
The value must match `^[a-z_][a-z0-9_-]{0,63}$` — lowercase snake/kebab case within every provider's function-name rules; lowercase guarantees configured names pass untouched through consumers' de-mangling heuristics (they key on the UpperCamel `Service_` segment of derived names). Code generation fails on an invalid value and on duplicate tool names within a service (one service is one MCP server registration — different services may legitimately reuse a name). The dynamic registration path (`gen.RegisterService`) applies the same option; an invalid or colliding configured name there falls back to the derived name so a tool is never silently dropped.
180+
165181
## Migrating from mark3labs-only (pre-v0.2)
166182

167183
Generated code no longer imports `mark3labs/mcp-go` directly. It programs against the `runtime.MCPServer` interface, and you pick the MCP library via an adapter package.

justfile

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,11 @@ update-build: gazelle
3939

4040
# Generate proto code (outside Bazel)
4141
generate:
42+
cd proto && buf generate
4243
cd pkg/testdata && buf generate buf.build/googleapis/googleapis
4344
cd pkg/testdata && buf generate --include-imports --exclude-path buf/validate
4445
rm -rf pkg/testdata/gen/go/buf/
46+
rm -rf pkg/testdata/gen/go/mcp/
4547
cd pkg/testdata && buf build -o gen/descriptors.binpb --exclude-path buf/validate
4648
go run mvdan.cc/gofumpt@latest -l -w pkg/testdata/
4749

pkg/gen/BUILD.bazel

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,19 @@ go_library(
55
srcs = [
66
"register.go",
77
"schema.go",
8+
"toolname.go",
89
],
910
importpath = "github.com/redpanda-data/protoc-gen-go-mcp/pkg/gen",
1011
visibility = ["//visibility:public"],
1112
deps = [
13+
"//pkg/mcpv1",
1214
"//pkg/runtime",
1315
"@build_buf_gen_go_bufbuild_protovalidate_protocolbuffers_go//buf/validate",
1416
"@org_golang_google_genproto_googleapis_api//annotations",
1517
"@org_golang_google_protobuf//encoding/protojson",
1618
"@org_golang_google_protobuf//proto",
1719
"@org_golang_google_protobuf//reflect/protoreflect",
20+
"@org_golang_google_protobuf//types/descriptorpb",
1821
"@org_golang_google_protobuf//types/dynamicpb",
1922
],
2023
)
@@ -36,10 +39,12 @@ go_test(
3639
"schema_map_bug_test.go",
3740
"schema_recursive_test.go",
3841
"schema_test.go",
42+
"toolname_test.go",
3943
],
4044
data = glob(["testdata/**"]),
4145
embed = [":gen"],
4246
deps = [
47+
"//pkg/mcpv1",
4348
"//pkg/runtime",
4449
"//pkg/runtime/mark3labs",
4550
"//pkg/testdata/gen/go/testdata",

pkg/gen/register.go

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ func RegisterService(s runtime.MCPServer, sd protoreflect.ServiceDescriptor, han
7272
opts.NewMessage = DynamicNewMessage
7373
}
7474
schemaOpts := SchemaOptions{}
75+
seenNames := map[string]bool{}
7576

7677
for i := 0; i < sd.Methods().Len(); i++ {
7778
method := sd.Methods().Get(i)
@@ -86,11 +87,24 @@ func RegisterService(s runtime.MCPServer, sd protoreflect.ServiceDescriptor, han
8687
comment = opts.CommentProvider(method)
8788
}
8889

89-
// Generate tool schema
90-
toolName := MangleHeadIfTooLong(
91-
strings.ReplaceAll(string(method.FullName()), ".", "_"),
92-
64,
93-
)
90+
// Resolve the tool name: the (mcp.v1.tool_name) option when set
91+
// and valid, else the derived form. A configured name that
92+
// collides with one already registered for this service falls
93+
// back to the derived form, and if a configured name on an
94+
// earlier method squatted on THIS method's derived name, a
95+
// numeric suffix de-collides — AddTool is never called twice
96+
// with the same name and nothing is silently dropped.
97+
toolName := ToolNameForMethod(method)
98+
if seenNames[toolName] {
99+
toolName = MangleHeadIfTooLong(
100+
strings.ReplaceAll(string(method.FullName()), ".", "_"),
101+
64,
102+
)
103+
for base, i := toolName, 2; seenNames[toolName]; i++ {
104+
toolName = MangleHeadIfTooLong(fmt.Sprintf("%s_%d", base, i), 64)
105+
}
106+
}
107+
seenNames[toolName] = true
94108

95109
tool := runtime.Tool{
96110
Name: toolName,

pkg/gen/schema.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,16 @@ func MangleHeadIfTooLong(name string, maxLen int) string {
530530
if len(hashPrefix) > 10 {
531531
hashPrefix = hashPrefix[:10]
532532
}
533+
// Providers disagree on the leading character of a tool name: Gemini
534+
// requires a letter or underscore, while OpenAI and Anthropic also
535+
// accept digits. A base-36 hash can start with a digit, and a mangled
536+
// name leads with the hash -- Gemini then rejects the entire request
537+
// with "Invalid function name". Map a leading digit onto 'g'..'p' so
538+
// the name is valid on every provider; hashes that already start with
539+
// a letter are unchanged, keeping existing tool names stable.
540+
if c := hashPrefix[0]; c >= '0' && c <= '9' {
541+
hashPrefix = string('g'+(c-'0')) + hashPrefix[1:]
542+
}
533543
if maxLen <= len(hashPrefix) {
534544
return hashPrefix[:maxLen]
535545
}
@@ -544,7 +554,7 @@ func MangleHeadIfTooLong(name string, maxLen int) string {
544554
// ToolForMethod generates the MCP tool definition for a given RPC method
545555
// descriptor (input and output JSON schemas plus name and description).
546556
func ToolForMethod(method protoreflect.MethodDescriptor, comment string) runtime.Tool {
547-
toolName := MangleHeadIfTooLong(strings.ReplaceAll(string(method.FullName()), ".", "_"), 64)
557+
toolName := ToolNameForMethod(method)
548558
description := CleanComment(comment)
549559

550560
return runtime.Tool{

pkg/gen/schema_edge_cases_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,3 +327,57 @@ func TestMessageSchema_NoUnionKeywords(t *testing.T) {
327327
// Regular non-oneof field is still in properties.
328328
g.Expect(props).To(HaveKey("name"))
329329
}
330+
331+
// Mangled names must be valid tool names on every provider. Gemini is
332+
// the strictest: the name must start with a letter or an underscore —
333+
// a base-36 hash prefix that starts with a digit broke Gemini-backed
334+
// agents outright (live failure: morningstar-securities
335+
// GetEquityResearchReport, 2026-06-10).
336+
func TestMangleHeadIfTooLong_GeminiSafeLeadingChar(t *testing.T) {
337+
g := NewWithT(t)
338+
339+
// The exact name from the live failure: its hash starts with '5'.
340+
const morningstar = "redpanda_mcps_morningstar_securities_v1_MorningstarSecuritiesService_GetEquityResearchReport"
341+
got := MangleHeadIfTooLong(morningstar, 64)
342+
g.Expect(got).To(HaveLen(64))
343+
g.Expect(got).To(HaveSuffix("_MorningstarSecuritiesService_GetEquityResearchReport"))
344+
first := got[0]
345+
g.Expect(first >= 'a' && first <= 'z').To(BeTrue(),
346+
"mangled name %q must start with a letter (Gemini requirement)", got)
347+
348+
// Deterministic across calls, and the exact expected rename:
349+
// the old form was "5rl6rmshvl__..." — '5' maps to 'l'.
350+
g.Expect(got).To(Equal("lrl6rmshvl__MorningstarSecuritiesService_GetEquityResearchReport"))
351+
352+
// The github_read hash also led with a digit ('6' -> 'm'), so its
353+
// mangled name changes too — historically
354+
// "64ghux5adn_github_read_v1_GitHubReadService_GetAuthenticatedUser".
355+
const github = "redpanda_mcps_github_read_v1_GitHubReadService_GetAuthenticatedUser"
356+
g.Expect(MangleHeadIfTooLong(github, 64)).To(Equal(
357+
"m4ghux5adn_github_read_v1_GitHubReadService_GetAuthenticatedUser",
358+
))
359+
}
360+
361+
// Every mangled name, for any input, must satisfy the strictest
362+
// provider naming rule (Gemini): ^[a-zA-Z_][a-zA-Z0-9_.:-]*$.
363+
func TestMangleHeadIfTooLong_AlwaysProviderSafe(t *testing.T) {
364+
g := NewWithT(t)
365+
inputs := []string{
366+
strings.Repeat("x", 100),
367+
"0" + strings.Repeat("y", 100),
368+
"redpanda_mcps_morningstar_securities_v1_MorningstarSecuritiesService_GetEquityResearchReport",
369+
"redpanda_mcps_github_read_v1_GitHubReadService_GetAuthenticatedUser",
370+
"a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y.z.a.b.c.d.e.f.g.h",
371+
}
372+
for _, in := range inputs {
373+
for _, maxLen := range []int{1, 5, 10, 11, 20, 64} {
374+
got := MangleHeadIfTooLong(in, maxLen)
375+
if len(got) == 0 {
376+
continue
377+
}
378+
first := got[0]
379+
g.Expect((first >= 'a' && first <= 'z') || (first >= 'A' && first <= 'Z') || first == '_').To(BeTrue(),
380+
"MangleHeadIfTooLong(%q, %d) = %q starts with %q", in, maxLen, got, string(first))
381+
}
382+
}
383+
}

pkg/gen/toolname.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
package gen
2+
3+
import (
4+
"fmt"
5+
"regexp"
6+
"strings"
7+
8+
"google.golang.org/protobuf/proto"
9+
"google.golang.org/protobuf/reflect/protoreflect"
10+
"google.golang.org/protobuf/types/descriptorpb"
11+
12+
mcpv1 "github.com/redpanda-data/protoc-gen-go-mcp/pkg/mcpv1"
13+
)
14+
15+
// validToolName is lowercase snake/kebab case within every LLM
16+
// provider's function-name rules (Gemini requires a leading letter or
17+
// underscore, OpenAI restricts the charset, 64 is the strictest length
18+
// limit). Lowercase is enforced deliberately: derived/mangled names
19+
// always contain an UpperCamel "<Service>Service_" segment, so a
20+
// lowercase-only configured name can never be mistaken for one by
21+
// downstream de-mangling heuristics (e.g. aigw's toolname.Short) —
22+
// configured names provably pass through them untouched.
23+
var validToolName = regexp.MustCompile(`^[a-z_][a-z0-9_-]{0,63}$`)
24+
25+
// ValidateToolName reports whether a configured tool name is usable on
26+
// every LLM provider.
27+
func ValidateToolName(name string) error {
28+
if !validToolName.MatchString(name) {
29+
return fmt.Errorf("invalid tool name %q: must match %s (lowercase letter or underscore first; lowercase alphanumerics, underscores and dashes; max 64 chars)", name, validToolName)
30+
}
31+
return nil
32+
}
33+
34+
// ConfiguredToolName returns the (mcp.v1.tool_name) option on the
35+
// method and whether it is set at all. Presence is reported separately
36+
// from the value so an explicitly authored empty string is
37+
// distinguishable from an absent option — the generator must reject
38+
// the former. The value is returned as authored; callers decide how to
39+
// handle an invalid one (the generator fails the build, the runtime
40+
// registration path falls back to the derived name).
41+
func ConfiguredToolName(method protoreflect.MethodDescriptor) (string, bool) {
42+
opts, ok := method.Options().(*descriptorpb.MethodOptions)
43+
if !ok || opts == nil {
44+
return "", false
45+
}
46+
if !proto.HasExtension(opts, mcpv1.E_ToolName) {
47+
return "", false
48+
}
49+
name, _ := proto.GetExtension(opts, mcpv1.E_ToolName).(string)
50+
return name, true
51+
}
52+
53+
// ToolNameForMethod resolves the MCP tool name for a method: the
54+
// (mcp.v1.tool_name) option when set and valid, otherwise the derived
55+
// full-name-based form, hash-truncated to 64 chars. Invalid configured
56+
// names fall back to the derived form here so dynamic registration
57+
// never publishes a provider-rejected name; the generator additionally
58+
// fails code generation on them so they are caught when authored.
59+
func ToolNameForMethod(method protoreflect.MethodDescriptor) string {
60+
if name, ok := ConfiguredToolName(method); ok && ValidateToolName(name) == nil {
61+
return name
62+
}
63+
return MangleHeadIfTooLong(strings.ReplaceAll(string(method.FullName()), ".", "_"), 64)
64+
}

0 commit comments

Comments
 (0)