[erikist]

Closes #89

Summary

Migrates generated MCP servers from github.com/mark3labs/mcp-go to the
official github.com/modelcontextprotocol/go-sdk (v1.8.0, requires Go
1.25), per the project's v0.3.0 milestone (#44).

• go.mod: go 1.25.0 (what go mod tidy settles on given go-sdk's
  own go 1.25.0 floor), requires github.com/modelcontextprotocol/go-sdk v1.8.0. github.com/mark3labs/mcp-go removed from go.mod/go.sum.
• Generator (cmd/protoc-gen-go-mcp/mcp.go): for each non-streaming
  RPC:
  • XxxTool() now returns &mcp.Tool{Name, Description, InputSchema},
    with InputSchema the request message's JSON Schema (built by the
    schema builder from feat: generate JSON Schema for tool inputs from proto descriptors (protojson-compatible) #88) embedded as a json.RawMessage literal.
  • RegisterTool now registers tools with
    mcp.AddTool[json.RawMessage, any](server, tool, handler), so the SDK
    validates arguments against the schema before the handler runs
    (confirmed: wrong-type and unknown-field arguments both produce
    isError: true without reaching the handler/backend — see test
    evidence below).
  • XxxHandler now decodes args json.RawMessage with
    protojson.Unmarshal, calls the gRPC client, and returns the response
    as protojson.Marshal-ed text. A gRPC error is returned as a plain Go
    error; the SDK turns it into an isError result. The error message
    already includes the gRPC status code, because status.Error(...)'s
    own Error() string is "rpc error: code = <Code> desc = <msg>"
    (verified in TestCallSetVibe_GRPCError), so no extra formatting was
    needed in the generated handler.
  • Removed now-dead manual field-decoding/casting code
    (generateFieldAssignment, kindToCastType, specialCastTypes,
    etc.) that the old generator used to hand-build request messages from
    map[string]any — protojson.Unmarshal replaces all of it, which
    also fixes the int32/repeated-scalar/repeated-message decoding bugs
    the pre-migration e2e test tracked as KNOWN BUGs.
• Examples: regenerated with make generate; the golden file
  (examples/gen/example/v1/example_mcp.pb.go) and
  cmd/protoc-gen-go-mcp/golden_test.go are in sync (byte-for-byte).
• cmd/mcp-vibe: now builds an mcp.NewServer(&mcp.Implementation{...}, nil) and serves it with mcpServer.Run(ctx, &mcp.StdioTransport{}).
• End-to-end test
  (examples/gen/example/v1/e2e_test.go): the MCP-client harness now
  uses mcp.NewInMemoryTransports() + mcp.NewClient(...).Connect(...) +
  ClientSession.CallTool/ListTools. All KNOWN BUG markers from
  test: end-to-end test of a generated MCP server (gRPC backend + MCP client) #87/feat: generate JSON Schema for tool inputs from proto descriptors (protojson-compatible) #88/feat!: generate MCP servers on the official Go MCP SDK #89 are resolved and replaced with direct assertions (the
  schema now matches the handler, and int32 / repeated-scalar /
  repeated-message fields all decode correctly via protojson). Added
  TestCallSetVibe_InvalidArgumentType and TestCallSetVibe_UnknownField,
  asserting isError: true and zero backend calls.
• README.md: updated the Go prerequisite to 1.25 and the code
  snippets that showed the old mcp.Tool/mcp.CallToolRequest shapes.

Decisions made

• Kept the generated API as close to 0.2.x as the official SDK's types
  allow; every remaining difference is called out in the BREAKING CHANGE: note below, per the issue's "list any change in the PR body"
  instruction.
• go.mod's go directive ended up as 1.25.0 rather than plain 1.25,
  because github.com/modelcontextprotocol/go-sdk's own go.mod pins go 1.25.0 and go mod tidy propagates that; this satisfies the issue's
  "go 1.25" requirement and is what a clean go mod tidy produces.
• Checked github.com/modelcontextprotocol/go-sdk's available versions
  via the module proxy (proxy.golang.org/.../@v/list) before picking
  v1.8.0: it is the latest non-prerelease tag (v1.8.0-pre.* tags also
  exist but are pre-releases), matching what the issue specifies.

Test evidence

$ gofmt -l .
(no output)

$ go vet ./...
(no output)

$ go build ./...
(no output)

$ go test ./...
?   	protoc-gen-go-mcp/cmd/mcp-vibe	[no test files]
ok  	protoc-gen-go-mcp/cmd/protoc-gen-go-mcp	0.018s
ok  	protoc-gen-go-mcp/examples/gen/example/v1	0.025s

$ make generate
Building protoc-gen-go-mcp...
Built successful!
Generating new files...
Generated successfully!
$ git status --porcelain
(no output — regeneration reproduces the committed examples exactly)

Smoke test (mcp-vibe built and run over stdio, driven with raw
JSON-RPC lines): initialize, tools/list, and a tools/call for
SetVibe all succeed —

>>> {"id":1,"jsonrpc":"2.0","method":"initialize", ...}
<<< {"jsonrpc":"2.0","id":1,"result":{"capabilities":{"logging":{},"tools":{"listChanged":true}},"protocolVersion":"2025-06-18","serverInfo":{"name":"vibe","version":"0.0.1"}}}
>>> {"jsonrpc":"2.0","method":"notifications/initialized"}
>>> {"id":2,"jsonrpc":"2.0","method":"tools/list"}
<<< {"jsonrpc":"2.0","id":2,"result":{"tools":[{"name":"GetVibe", ...},{"name":"SetVibe","inputSchema":{"type":"object","properties":{"vibe":{"type":"string", ...}},"additionalProperties":false}, ...}, ...]}}
>>> {"id":3,"jsonrpc":"2.0","method":"tools/call","params":{"name":"SetVibe","arguments":{"vibe":"radical"}}}
<<< {"jsonrpc":"2.0","id":3,"result":{"content":[{"type":"text","text":"{\"previousVibe\":\"initial vibe\",\"vibe\":\"radical\"}"}]}}

Also ran go mod verify (all modules verified) and confirmed grep -rn mark3labs . (excluding CHANGELOG.md) only matches docs/GOAL.md's
historical description of the migration, not any code.

Self-review

Ran the reviewer subagent against this branch. It flagged one
suspected blocker (a seemingly-corrupted go.sum line for
golang.org/x/oauth2) which turned out to be a false positive from its
own tool output rendering — go mod verify, go build, go vet, and go test all pass cleanly against the committed go.sum, and re-running go mod tidy reproduces it byte-for-byte. No other blocking or in-scope
issues were found; its only other notes were non-blocking
(go.mod's 1.25.0 vs 1.25 — addressed above) and an out-of-scope
observation about .github/workflows/build.yml Go versions, which this
issue does not ask to change.

Notes for reviewers

BREAKING CHANGE: generated MCP servers now import
github.com/modelcontextprotocol/go-sdk/mcp instead of
github.com/mark3labs/mcp-go. Users of generated code must update to the
official SDK and adjust to the following shape changes:

• New<Service>MCPServer now takes a *mcp.Server (not
  *server.MCPServer), and the generated struct's exported MCPServer
  field is now typed *mcp.Server (was *server.MCPServer).

• RegisterTool's handler parameter type changed from
  func(context.Context, mcp.CallToolRequest) (*mcp.CallToolResult, error) to mcp.ToolHandlerFor[json.RawMessage, any] (i.e. func(ctx, *mcp.CallToolRequest, json.RawMessage) (*mcp.CallToolResult, any, error)), and RegisterTool's tool parameter is now *mcp.Tool (was
  mcp.Tool).

• XxxTool() now returns *mcp.Tool instead of mcp.Tool.

• Each tool's InputSchema is now the top-level request-message
  JSON Schema (previously nested under a property named after the
  request message, and not actually enforced since mark3labs didn't
  validate against it).

• Tool call responses are now protojson, so they use lowerCamel keys
  (e.g. previous_vibe → previousVibe) and omit zero-valued fields,
  instead of the old reflection-based encoding.

• Tool call arguments must now use lowerCamel field names: the schema
  sets additionalProperties: false, so a snake_case argument name is
  rejected as an unknown field (isError: true) rather than silently
  accepted or coerced.

• Go 1.25 is now required to build this module.

• Out of scope, left untouched per docs/GOAL.md: streaming RPCs, arrays
  of messages as tool inputs (Support message type arrays #39 — now actually works via protojson,
  but no explicit support/tests were added beyond what the e2e test
  already exercises), release binaries, vendoring, MCP resources/prompts.

• Did not touch .github/workflows/**, CHANGELOG.md,
  release-please-config.json, or .release-please-manifest.json.

• Filed by the reviewer, not a blocker for this PR: fix: escape proto comments correctly in generated descriptions and schemas #100, tracking
  two comment-escaping bugs this PR exposes (a backslash in a comment
  breaks generation — already true on main — and quotes in schema
  descriptions are double-escaped).

• Factory-authored

[erikist]

🔍 Review (Claude, on behalf of @erikist): small fixes, then merge. The migration itself is correct. In the factory image:

• Checks: vet, build, -race tests and go mod tidy -diff are clean, make generate reproduces the committed files exactly, and mark3labs is gone.
• e2e tests: every knownBug is now a real assertion. Compared with main, nothing was deleted or softened, and both invalid-argument tests assert the backend wasn't called.
• Mutations: these all fail, as they should: renaming a tool, making the error path return success, dropping an AddTool, using UseProtoNames, and swapping a schema.
• Stdio smoke test: it works, it advertises the top-level request schema, and invalid arguments return isError.

Please fix:

1. Strengthen TestToolsList (examples/gen/example/v1/e2e_test.go). Swapping SetVibe's schema for SetVibeDetails's still passes the e2e test; only the golden test catches it. Assert SetVibe's full schema: require.Equal on properties, plus additionalProperties: false, plus require.NotContains(t, props, "SetVibeRequest").
2. Add a literal BREAKING CHANGE: paragraph to the PR body, as AGENTS.md requires. The body currently has only "Breaking change" under reviewer notes. (This repo squash-merges, so I'll put that paragraph in the merge commit.)
3. Complete the breaking-change list. It should also cover these wire-level changes:
   • Responses use lowerCamel keys (previous_vibe → previousVibe), and zero-valued fields are omitted.
   • Arguments must use lowerCamel names. snake_case is now rejected because of additionalProperties: false.
   • RegisterTool's tool parameter is now *mcp.Tool.
   • The type of the exported MCPServer field changed.

Separately, I filed #100 for two comment-escaping bugs this PR exposes: a backslash in a comment breaks generation (already true on main), and quotes in schema descriptions are double-escaped. They're not blockers for this PR.

[erikist]

🤖 Factory (dispatched 2026-10-08T18:49:08Z):

Addressed the review feedback from the PR comment at 2026-10-08T18:48:12Z ("🔍 Review (Claude, on behalf of @erikist): small fixes, then merge."):

1. "Strengthen TestToolsList... Assert SetVibe's full schema" — done, in commit 24dc8d6. TestToolsList now asserts additionalProperties: false, the full properties map (exact match via require.Equal), and require.NotContains(..., "SetVibeRequest") for the SetVibe tool's InputSchema. This closes the gap where swapping SetVibe's schema for SetVibeDetails's previously passed the e2e test (only the golden test caught it) — verified via the reviewer subagent, which confirmed the expected values match the generated schema literal in example_mcp.pb.go exactly and that a schema swap would now fail this assertion.
2. "Add a literal BREAKING CHANGE: paragraph to the PR body" — done, no code commit needed (PR body only). The "Notes for reviewers" section now opens with a literal BREAKING CHANGE: paragraph instead of just a bolded "Breaking change" heading.
3. "Complete the breaking-change list" — done, in the same PR body edit. Added the four missing items: protojson responses use lowerCamel keys and omit zero-valued fields; arguments must use lowerCamel names (snake_case now rejected via additionalProperties: false); RegisterTool's tool parameter is now *mcp.Tool; and the exported MCPServer field's type changed to *mcp.Server.

Also noted the reviewer's informational note that #100 was filed (non-blocking, out of scope for this PR) for the two comment-escaping bugs — added a line in the PR body pointing to it so it's not lost.

Commits pushed: 24dc8d6 — test: assert SetVibe's full input schema in TestToolsList (#89).

Checks run after the change:

$ gofmt -l .        → (no output)
$ go vet ./...       → (no output)
$ go build ./...     → (no output)
$ go test ./...      → ok  protoc-gen-go-mcp/cmd/protoc-gen-go-mcp  0.024s
                        ok  protoc-gen-go-mcp/examples/gen/example/v1  0.025s

Self-review: ran the reviewer subagent (read-only, scoped to the uncommitted diff) against issue #89's acceptance criteria and the specific review ask; it found no blocking issues, confirmed the new assertions are correct and sufficient, and noted one non-blocking, out-of-scope observation (the other tools in TestToolsList still use loose Contains checks, which is outside what this review comment asked for).

No open questions — all three points were concrete and actionable.

[erikist]

🔍 Re-review (Claude, on behalf of @erikist): all three points are fixed.

• TestToolsList now asserts SetVibe's full schema exactly, which catches a swapped schema.
• The body has a literal BREAKING CHANGE: paragraph.
• The paragraph lists the wire-level changes: lowerCamel response keys, snake_case arguments rejected, *mcp.Tool, and the MCPServer field type.

CI is green. Squash-merging with that BREAKING CHANGE: paragraph as the commit body, so release-please picks it up ✅