feat(mcp): expose proto-annotated gRPC methods as MCP tools (Phase 4)#617
feat(mcp): expose proto-annotated gRPC methods as MCP tools (Phase 4)#617lakhansamani wants to merge 1 commit into
Conversation
Adds an MCP server that auto-discovers tools from the proto layer: any
RPC with `(authorizer.common.v1.mcp_tool).exposed = true` becomes an MCP
tool, with its input schema derived from the request message descriptor
and its description sourced from the leading proto comment. No hand
registration; adding a new MCP tool is a one-annotation proto change.
How it works (internal/mcp):
- scanner.go walks grpc.Server.GetServiceInfo() + protoregistry.GlobalFiles,
reads the mcp_tool MethodOption on each method, builds a ToolBinding
list (tool name, description, destructive hint, full method, input/output
descriptors).
- schema.go renders proto request descriptors into JSON Schema for MCP
tool input declarations (string/int/bool/array/object/enum).
- server.go registers tools dynamically on a github.com/modelcontextprotocol/go-sdk
Server. Each tool's handler unmarshals JSON args into a dynamicpb.Message,
invokes the matching gRPC method over an in-process bufconn (same pattern
as the REST gateway), and renders the response as JSON.
CLI (cmd/mcp.go):
- `authorizer mcp` boots the minimal subsystems needed (config + service
+ grpc) and serves MCP over stdio — suitable for
`claude mcp add authorizer -- /path/to/authorizer mcp ...`.
- Stderr-only logging so JSON-RPC framing on stdout isn't corrupted.
Today's surface (from proto annotations already in place):
- get_meta → MetaService.GetMeta (real, end-to-end working)
- get_user → UserService.GetUser (stub: codes.Unimplemented)
- get_current_session → SessionService... (stub)
- list_my_permissions → AuthzService... (stub)
As each underlying handler migrates from internal/graphql into
internal/service in follow-up PRs, its MCP tool becomes functional
automatically — no MCP-side change required.
Tests:
- TestMCPListAndCallGetMeta: in-memory MCP client + server; verifies
tools/list returns 4 tools and tools/call get_meta returns the real
MetaService response with structured_content matching the proto shape.
- Full SQLite integration suite still green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lakhansamani
left a comment
There was a problem hiding this comment.
Principal-engineer review
The annotation-driven design here is genuinely good: one proto field, zero hand-registration, and the tool surface auto-grows as future PRs migrate handlers into internal/service. The bufconn dispatch mirrors the REST gateway pattern, which is a nice consistency win. End-to-end vertical slice works.
That said, a handful of issues are worth fixing before/after merge — some are dormant today (only get_meta is real), but the design hard-codes assumptions that will bite as more tools land.
Strengths
- Single source of truth. Adding a new MCP tool is a one-line proto annotation; the scanner picks it up at boot. The audit story is also clean —
git grep \"mcp_tool\"is the authoritative list. - Description from leading proto comment is a lovely touch — RPC docstrings now serve REST openapi and MCP tool hints.
- Reusing bufconn keeps the dispatch path uniform with the gateway (same
grpc.NewClient→passthrough:///bufconnrecipe) and side-steps any concrete-client-type assumptions about generated code. - Stderr-only logging in
cmd/mcp.gois exactly right for stdio framing. - Dynamicpb dispatch works.
grpc.ClientConn.Invoke(ctx, method, args, reply any, ...)uses the proto codec, which marshals anyproto.Message(includingdynamicpb.Message). No need for generated client stubs — confirmed against grpc-go v1.81.
Issues — blocking
-
schemaForMessageis not cycle-safe. It recurses unconditionally throughMessageKindfields. Today nothing reachable from an exposed tool is cyclic, but the next tool exposure that includesauthorizer.common.v1.AppData(and through itgoogle.protobuf.Struct → Value → ListValue → repeated Value) will recurse infinitely and stack-overflow at process start.CreateUserRequestalready hasAppData; if anyone flipsexposed: trueon it, boot crashes. Fix: pass avisited map[protoreflect.FullName]booldown throughschemaForMessage/schemaForFieldand emit{\"type\":\"object\"}(or a$ref-style placeholder) on revisit. -
No auth metadata is propagated on dispatch.
registerTooldoesconn.Invoke(ctx, ...)with the bare MCP request context — noAuthorizationheader, no cookies. The momentGetUser/GetCurrentSession/ListMyPermissionsbecome real (they're stubs today), they will run as unauthenticated andtransport.MetaFromGRPCwill see empty headers. Either (a) document loudly that all MCP-exposed RPCs must work without caller identity (which is at odds withusers/mesemantics), or (b) wire a mechanism — env var, CLI flag, or MCPinitialize-time client capability — to attach a static Bearer/cookie to outgoing gRPC metadata. As written, the three stub tools are essentially "identity-of-process" calls, which is the wrong trust model forme-style RPCs.
Issues — important
-
Dead code in
mcpToolFromMethod. Lines 87–89 (opts, ok := m.Options().(*descriptorOptionsCarrier); _ = opts; _ = ok) do nothing —m.Options()returns*descriptorpb.MethodOptions, never*descriptorOptionsCarrier. ThedescriptorOptionsCarrierstruct itself is unreferenced outside this failed assertion. Delete both. Also:proto.GetExtension(nil, ...)does not panic — it returns the default. The whole function reduces to:t, _ := proto.GetExtension(m.Options(), commonv1.E_McpTool).(*commonv1.McpTool) return t
-
JSON Schema is missing several proto signals the MCP host expects. All dormant today because
get_metahas an empty input, but they'll surface as soon as a real input message is exposed:- No
requiredarray. proto3 has norequired, butbuf.validate.field.required = trueis the project's convention (seeUpdateUserRequest.user). Reading(buf.validate.field).requiredand populatingRequiredwould let MCP hosts pre-validate. - Enums emit
type: stringwith noenumarray. The host can't suggest valid values to the model. Trivial fix: walkf.Enum().Values()and emit the names. - Oneofs are flattened into N independent optional fields.
CreateSessionRequest.grantbecomes 4 sibling fields with no exclusivity hint. JSON Schema spells this withoneOf/anyOf— at minimum, skip the oneof on the first pass and emit a TODO so the LLM doesn't happily set bothpasswordandmagic_link. - No field-level descriptions. The method-level leading comment is captured, but
f.ParentFile().SourceLocations().ByDescriptor(f).LeadingCommentswould surface field-level docs too — same as the method-level wiring already does. OutputSchemais not set.schemaForMessage(b.OutputDescriptor)would be a one-liner and lets clients validatestructured_content.
- No
-
Errors from the underlying gRPC method are surfaced as JSON-RPC errors, not tool-call errors. Today the 3 stubs return
codes.Unimplemented; the MCP client sees an opaque JSON-RPCInternal error(or whatever the SDK does with non-jsonrpc Go errors) instead of the structuredCallToolResult{IsError: true, Content: [TextContent{...}]}pattern MCP recommends. The LLM gets no actionable message. Convert gRPCstatus.StatusintoCallToolResult{IsError: true, Content: [&mcp.TextContent{Text: st.Message()}]}and returnnilerror — the host can then surface the failure back to the model coherently. -
cmd/mcp.goboots a Service withStorage/Token/MemoryStore/Email/SMSall nil. True today (MetaHandleronly needs config), but the moment a tool that touches storage or token gets exposed in a follow-up PR, the call nil-panics. The comment incmd/mcp.goacknowledges this ("each panics-on-nil call moved here would be caught by integration tests") but a runtime guard would be safer than a TODO: either wire the samerunRootinit path conditionally, or makemcp.Newfail-fast when a discovered binding's handler needs a subsystem the service doesn't have (harder — would need annotation metadata).
Issues — nit
string(req.Params.Arguments) != \"null\"is technically right becausejson.RawMessageis bytewise — but leading/trailing whitespace (e.g.\" null\") would slip through and fail downstream with a confusingprotojsonerror. Preferbytes.Equal(bytes.TrimSpace(req.Params.Arguments), []byte(\"null\"))(or just let protojson fail — its error message is fine).MethodOptions.Deprecatedisn't checked; a method markeddeprecated = truewill still appear as an MCP tool. Probably fine but worth a one-line skip.- Test re-uses
mcpSrv.MCPServer()directly and never calls a cleanup that closesgwConn/lis. Two leaks per test invocation (goroutine + listener).Server.cleanup()is unexported — either export it or havemcp.Newaccept acontext.Contextthat owns the bufconn goroutine. - Tool name collision. Two services with the same method name (or two annotations with the same
tool_nameoverride) silently overwrite viamcp.Server.AddTool's "replace existing" semantics. A duplicate-detection check inScanwould catch this at boot. fmt.Sprintf(\"/%s/%s\", svcName, m.Name())would be clearer constructed viaprotoreflect'sm.FullName()machinery, but pragmatically fine.
Test coverage gaps
The PR is one test deep — TestMCPListAndCallGetMeta happy-path. Worth adding before more tools land:
- Schema-generation table test covering: scalar field kinds (each integer width, float, bool, bytes, enum), repeated, map, nested message, well-known types (
Timestamp,FieldMask,Struct). Pin the expected JSON Schema. This is the spec of what the LLM sees and should be locked. - Recursion safety test — synthesize a descriptor that points to
google.protobuf.Struct(or a self-referential test proto) and assertschemaForMessagereturns instead of stack-overflowing. - Tool-list size. Test currently checks only "contains get_meta". A simple
require.Len(list.Tools, 4)would have caught any annotation regression in the 3 stub tools. - Stub Unimplemented path. Call
get_userand assert what the MCP client sees. This is the actual error contract today. - Destructive flag wiring. No tool with
destructive: trueis currently exposed, so thetool.Annotations.DestructiveHintbranch is uncovered. Trivial to fake. - JSON arg edge cases.
arguments: null,arguments: \" null\", missing field, extra field (DiscardUnknown: trueis set — verify), malformed JSON. - Scanner edge cases. A method with no
mcp_toolannotation (skipped),exposed: false(skipped),tool_nameoverride (used), and the health/reflection-service-not-in-protoregistry path (today asserted only by integration boot).
Recommendation
Approve in spirit for the vertical-slice but block on (1) and (2) — the recursion is a real crash waiting for the next annotation flip, and the unauthenticated-dispatch issue is the kind of thing that will quietly ship a bad default. (3)–(6) and the test gaps can land as a follow-up PR.
Summary
Adds an MCP server that auto-discovers tools from the proto layer: any RPC with `(authorizer.common.v1.mcp_tool).exposed = true` becomes an MCP tool, with its input schema derived from the request-message descriptor and its description sourced from the leading proto comment. No hand registration; adding a new MCP tool is a one-annotation proto change.
How it works (`internal/mcp`)
CLI (`cmd/mcp.go`)
Today's surface (from proto annotations already in place)
As each underlying handler migrates from `internal/graphql` into `internal/service` in follow-up PRs, its MCP tool becomes functional automatically — no MCP-side change required.
Test plan
🤖 Generated with Claude Code