Skip to content

feat(mcp): expose proto-annotated gRPC methods as MCP tools (Phase 4)#617

Closed
lakhansamani wants to merge 1 commit into
feat/grpc-rest-surfacefrom
feat/mcp-server
Closed

feat(mcp): expose proto-annotated gRPC methods as MCP tools (Phase 4)#617
lakhansamani wants to merge 1 commit into
feat/grpc-rest-surfacefrom
feat/mcp-server

Conversation

@lakhansamani
Copy link
Copy Markdown
Contributor

Stacked on top of #616 (which is stacked on #615, on #614). Review and merge after Phase 2 is in.

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`)

  • `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 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 (config + service + grpc) and serves MCP over stdio.
  • Wire-up with Claude Code: `claude mcp add authorizer -- /path/to/authorizer mcp --client-id=... --database-type=sqlite --database-url=auth.db`
  • Stderr-only logging so JSON-RPC framing on stdout isn't corrupted.

Today's surface (from proto annotations already in place)

MCP tool Backed by Status
`get_meta` `MetaService.GetMeta` Real, end-to-end working
`get_user` `UserService.GetUser` Stub (`codes.Unimplemented`)
`get_current_session` `SessionService.GetCurrentSession` Stub
`list_my_permissions` `AuthzService.ListMyPermissions` 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.

Test plan

  • `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 (70s) still green — no regression
  • `go build ./...` clean
  • CI green on this stacked PR

🤖 Generated with Claude Code

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>
Copy link
Copy Markdown
Contributor Author

@lakhansamani lakhansamani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.NewClientpassthrough:///bufconn recipe) and side-steps any concrete-client-type assumptions about generated code.
  • Stderr-only logging in cmd/mcp.go is exactly right for stdio framing.
  • Dynamicpb dispatch works. grpc.ClientConn.Invoke(ctx, method, args, reply any, ...) uses the proto codec, which marshals any proto.Message (including dynamicpb.Message). No need for generated client stubs — confirmed against grpc-go v1.81.

Issues — blocking

  1. schemaForMessage is not cycle-safe. It recurses unconditionally through MessageKind fields. Today nothing reachable from an exposed tool is cyclic, but the next tool exposure that includes authorizer.common.v1.AppData (and through it google.protobuf.Struct → Value → ListValue → repeated Value) will recurse infinitely and stack-overflow at process start. CreateUserRequest already has AppData; if anyone flips exposed: true on it, boot crashes. Fix: pass a visited map[protoreflect.FullName]bool down through schemaForMessage/schemaForField and emit {\"type\":\"object\"} (or a $ref-style placeholder) on revisit.

  2. No auth metadata is propagated on dispatch. registerTool does conn.Invoke(ctx, ...) with the bare MCP request context — no Authorization header, no cookies. The moment GetUser/GetCurrentSession/ListMyPermissions become real (they're stubs today), they will run as unauthenticated and transport.MetaFromGRPC will see empty headers. Either (a) document loudly that all MCP-exposed RPCs must work without caller identity (which is at odds with users/me semantics), or (b) wire a mechanism — env var, CLI flag, or MCP initialize-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 for me-style RPCs.

Issues — important

  1. Dead code in mcpToolFromMethod. Lines 87–89 (opts, ok := m.Options().(*descriptorOptionsCarrier); _ = opts; _ = ok) do nothing — m.Options() returns *descriptorpb.MethodOptions, never *descriptorOptionsCarrier. The descriptorOptionsCarrier struct 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
  2. JSON Schema is missing several proto signals the MCP host expects. All dormant today because get_meta has an empty input, but they'll surface as soon as a real input message is exposed:

    • No required array. proto3 has no required, but buf.validate.field.required = true is the project's convention (see UpdateUserRequest.user). Reading (buf.validate.field).required and populating Required would let MCP hosts pre-validate.
    • Enums emit type: string with no enum array. The host can't suggest valid values to the model. Trivial fix: walk f.Enum().Values() and emit the names.
    • Oneofs are flattened into N independent optional fields. CreateSessionRequest.grant becomes 4 sibling fields with no exclusivity hint. JSON Schema spells this with oneOf/anyOf — at minimum, skip the oneof on the first pass and emit a TODO so the LLM doesn't happily set both password and magic_link.
    • No field-level descriptions. The method-level leading comment is captured, but f.ParentFile().SourceLocations().ByDescriptor(f).LeadingComments would surface field-level docs too — same as the method-level wiring already does.
    • OutputSchema is not set. schemaForMessage(b.OutputDescriptor) would be a one-liner and lets clients validate structured_content.
  3. 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-RPC Internal error (or whatever the SDK does with non-jsonrpc Go errors) instead of the structured CallToolResult{IsError: true, Content: [TextContent{...}]} pattern MCP recommends. The LLM gets no actionable message. Convert gRPC status.Status into CallToolResult{IsError: true, Content: [&mcp.TextContent{Text: st.Message()}]} and return nil error — the host can then surface the failure back to the model coherently.

  4. cmd/mcp.go boots a Service with Storage/Token/MemoryStore/Email/SMS all nil. True today (MetaHandler only 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 in cmd/mcp.go acknowledges 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 same runRoot init path conditionally, or make mcp.New fail-fast when a discovered binding's handler needs a subsystem the service doesn't have (harder — would need annotation metadata).

Issues — nit

  1. string(req.Params.Arguments) != \"null\" is technically right because json.RawMessage is bytewise — but leading/trailing whitespace (e.g. \" null\") would slip through and fail downstream with a confusing protojson error. Prefer bytes.Equal(bytes.TrimSpace(req.Params.Arguments), []byte(\"null\")) (or just let protojson fail — its error message is fine).
  2. MethodOptions.Deprecated isn't checked; a method marked deprecated = true will still appear as an MCP tool. Probably fine but worth a one-line skip.
  3. Test re-uses mcpSrv.MCPServer() directly and never calls a cleanup that closes gwConn/lis. Two leaks per test invocation (goroutine + listener). Server.cleanup() is unexported — either export it or have mcp.New accept a context.Context that owns the bufconn goroutine.
  4. Tool name collision. Two services with the same method name (or two annotations with the same tool_name override) silently overwrite via mcp.Server.AddTool's "replace existing" semantics. A duplicate-detection check in Scan would catch this at boot.
  5. fmt.Sprintf(\"/%s/%s\", svcName, m.Name()) would be clearer constructed via protoreflect's m.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 assert schemaForMessage returns 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_user and assert what the MCP client sees. This is the actual error contract today.
  • Destructive flag wiring. No tool with destructive: true is currently exposed, so the tool.Annotations.DestructiveHint branch is uncovered. Trivial to fake.
  • JSON arg edge cases. arguments: null, arguments: \" null\", missing field, extra field (DiscardUnknown: true is set — verify), malformed JSON.
  • Scanner edge cases. A method with no mcp_tool annotation (skipped), exposed: false (skipped), tool_name override (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.

@lakhansamani
Copy link
Copy Markdown
Contributor Author

Superseded by #620, which consolidates this stack into a single PR against main. All blocking review findings from this PR were addressed in #620; see its body for the per-finding traceback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant