feat: generate tool handlers (validate, pbjson decode, tonic call, pbjson encode) (#7)

[erikist]

Closes #7

Summary

Replaces the "not implemented" stub tool handler body (left by #6) with
the real one, the counterpart of Go's generateHandler
(cmd/protoc-gen-go-mcp/mcp.go at 9e072f9) plus the argument validation
Go's mcp.AddTool does and the official Rust MCP SDK (rmcp) does not.

For each unary RPC's call_<method> associated function:

1. Validate the raw tool-call arguments (None treated as {})
   against a jsonschema::Validator compiled once per tool into a new
   package-level static <SERVICE>_<METHOD>_VALIDATOR: LazyLock, next to
   the existing ..._INPUT_SCHEMA constant it's built from. On failure,
   returns a tool error listing every failing instance path and reason
   ("invalid arguments: /vibe: 5 is not of type \"string\"; ..."), and
   the backend is not called.
2. Decode with serde_json::from_value::<Request> (the
   pbjson-generated Deserialize). A decode failure also becomes a tool
   error without calling the backend.
3. Call the tonic client (client.<method>(tonic::Request::new(request)),
   on a cloned client as before). A tonic::Status becomes a tool error
   formatted like grpc-go's Status.Error():
   "rpc error: code = NotFound desc = vibe not found" —
   tonic::Code's Debug impl (tonic 0.14.6) happens to render exactly
   grpc-go's CamelCase code names, so no separate name table was needed.
4. Encode the response with serde_json::to_string (the
   pbjson-generated Serialize) and return
   CallToolResult::success(vec![Content::text(json)]).

Generated API change: T's trait bounds

The generated <Service>McpServer<T>'s impl<T> block and its
rmcp::ServerHandler impl now bound T with exactly what the
tonic-generated client's own unary method needs to be callable
(tonic::client::GrpcService<tonic::body::Body> plus the associated-type
bounds on Error/ResponseBody/Future), instead of just `Clone + Send

• Sync + 'static. This is a public API change to every generated McpServer(it now requires a real gRPC-capableT, matching what the sibling tonic client already required); verified by compiling and running the generated handler against a real in-process tonic server (tonic::transport::Channelcontinues to satisfy these bounds, as doesInterceptedService`).

pbjson vs. protojson difference (result encoding only)

Flagged on #18's review: pbjson serializes a non-finite float/double as
JSON null, where protojson emits the string
"NaN"/"Infinity"/"-Infinity". This only affects tool results;
both pbjson's and protojson's generated deserializers accept all three
strings on input (pinned by schema.rs's existing
schema_validates_non_finite_floats test and this PR's
non_finite_floats_are_accepted_as_input). Documented in
server.rs's module doc comment. Making results match Go is a
nice-to-have per that review comment, not required by this issue.

Regenerated example

make generate (protoc 29.3, protoc-gen-prost 0.5.0, protoc-gen-tonic
0.5.0, protoc-gen-prost-serde 0.4.0, all from the factory image) against
examples/protos/example.proto; the golden test
(protoc-gen-rust-mcp/tests/golden.rs) passes against the regenerated,
committed examples.v1.mcp.rs.

Dependency added

jsonschema = { version = "0.58", default-features = false } (same
version protoc-gen-rust-mcp already uses as a dev-dependency for its own
schema tests) is now a runtime dependency of examples (the
generated code calls it), checked via cargo info jsonschema against the
latest 0.58.x release.

Decisions made

• Validator location/caching: one LazyLock<jsonschema::Validator>
  per tool, built from the schema constant already emitted for that tool
  (VALIDATOR next to INPUT_SCHEMA), rather than a shared validator
  keyed by tool name at runtime — keeps the per-tool cost at first use
  and needs no runtime lookup.
• Error text wording for schema/decode/encode failures
  ("invalid arguments: ...", "decoding arguments: ...",
  "encoding response: ...") isn't specified by Go beyond the gRPC-error
  format, which is pinned exactly; these three are this generator's own
  wording, pinned by tests.
• register_tool's requirement that T satisfy the new client
  bounds: no change needed there — register_tool/register_default_tools
  already live inside the impl<T> ... where <bounds> block, so this
  flows through automatically.

Test evidence

Commands run in this worktree (protoc 29.3, tools from the factory
image):

cargo fmt --all --check
cargo clippy --workspace --all-targets -- -D warnings
cargo build --workspace
cargo test --workspace
make generate && git status --porcelain   # confirms make generate reproduces the committed example

All four AGENTS.md checks pass clean (no warnings, no diff after
cargo fmt --all --check). cargo test --workspace: 49 + 51 + ... all
green, including:

• protoc-gen-rust-mcp's tests/golden.rs
  (generated_output_matches_committed_golden_file).
• protoc-gen-rust-mcp's tests/server_integration.rs: 12 tests,
  including 5 new ones exercising the real handler body against a real
  in-process tonic server built from tests/testdata/mcpgen/fixture.proto
  (success, gRPC NotFound → tool error text, wrong-type/unknown-field
  rejected without calling the backend, missing-arguments-as-{}), plus
  updated versions of the pre-existing override/composition tests that
  used to assert "not implemented" stub text.
• examples' new tests/handler.rs: 6 tests against the real, committed
  example VibeService server and a real in-process tonic backend,
  covering every bullet in the issue's acceptance criteria list
  (protojson-matching success JSON, gRPC NotFound in grpc-go format,
  wrong argument type, unknown field including the snake_case
  previous_vibe form, int64 as both number and string, enum by name,
  and NaN/Infinity/-Infinity floats as input).
• protoc-gen-rust-mcp's src/server.rs unit tests for the new
  rust_message_path helper (top-level and nested-message cases; no
  existing fixture proto has a nested message as an RPC's request/response
  type, so this needed a hand-built FileDescriptorProto).

Self-review: ran the reviewer subagent once; it found two stale
"stub handler" doc-comment references (one of which had leaked into the
committed generated file) and a module-doc comment that hadn't been
updated to mention this issue's own test coverage. Both fixed in a
follow-up commit, and the golden file/example were regenerated again
afterward. No blocking or in-scope findings remained.

Notes for reviewers

• The widened T bounds (see above) are the one generated-API-visible
  change beyond the handler bodies themselves; flagging per AGENTS.md's
  "document any change to [generated code's runtime dependencies]" rule.

• protoc-gen-rust-mcp/build.rs's fixture compilation now also runs
  pbjson-build for tests/testdata/mcpgen/fixture.proto (it previously
  only ran tonic-prost-build), since the generated handler body needs
  Serialize/Deserialize impls for the fixture's own types too, not
  just examples/'s committed ones.

• protoc-gen-rust-mcp's dev-dependency on tonic gained the server
  and router features (for Server::builder()/add_service/
  TcpIncoming), so server_integration.rs could run a real in-process
  gRPC server instead of asserting against a never-connecting client.
  examples' dev-dependency on tonic gained router for the same
  reason in handler.rs.

• No out-of-scope follow-ups identified.

• Factory-authored

[erikist]

🔍 Review: this matches the verified design. Mutation check: disabling the schema-validation branch in the generated example makes wrong_argument_type_is_rejected_without_calling_backend and unknown_field_including_snake_case_is_rejected_without_calling_backend fail, so validation is really covered. Three small changes:

1. cargo test -p examples doesn't compile on its own. examples/tests/handler.rs uses rmcp::RoleClient and ().serve(...) (rmcp's client feature), but examples' dependencies only enable server. It only works under --workspace, through feature unification with the plugin crate's dev-deps. Add rmcp with features = ["client"] to examples' [dev-dependencies], plus transport-io if the duplex needs it.
2. Pin the NaN result difference with a test, as asked on feat: generate tool handlers (validate, pbjson decode, tonic call, pbjson encode) #7: a backend response holding f64::NAN or INFINITY comes back as null in the result JSON. A module doc alone won't catch a pbjson change.
3. Add one case only the schema can catch. Today's wrong-type and unknown-field cases would also be rejected by pbjson's own decode, so they don't isolate validation from decoding. Add an enum given as a number ({"vibeScalar":{"vibeEnum":[1]}} for SetVibeDetails), which pbjson accepts and the schema rejects, and assert that the backend isn't called.

[erikist]

🤖 Factory (dispatched 2026-10-10T01:24:36Z):

Addressed all three points from the review comment (#25 (comment)):

1. cargo test -p examples didn't compile standalone. Added rmcp = { version = "3.5", default-features = false, features = ["client", "transport-io"] } to examples/Cargo.toml's [dev-dependencies] (same features protoc-gen-rust-mcp's own dev-deps already use for the equivalent in-memory-duplex test setup in server_integration.rs). Verified: cargo test -p examples now compiles and passes on its own, without --workspace.

2. Pinned the NaN/Infinity result divergence with a test, as asked on feat: generate tool handlers (validate, pbjson decode, tonic call, pbjson encode) #7: added non_finite_floats_in_result_are_encoded_as_json_null in examples/tests/handler.rs, which calls SetVibeArray with vibeDoubles: ["NaN", "Infinity", "-Infinity"] and asserts the result JSON is {"vibeArray":{"vibeDoubles":[null,null,null]}} — through the real handler, not pbjson in isolation. This needed FakeVibeService::set_vibe_array to actually echo its request (it previously returned unimplemented, since nothing exercised it).

3. Added the enum-as-number case that isolates schema validation from decoding: enum_as_number_is_rejected_by_schema_without_calling_backend sends {"vibeEnum": [1]} to SetVibeDetails (pbjson's generated Deserialize accepts an enum's numeric value via visit_i64/visit_u64, but the schema restricts enums to their name strings), using the never-connecting lazy_client() to prove the backend isn't called. Confirmed by running it that this is schema-only: pbjson's decoder alone would have accepted 1.

Also added one sentence to server.rs's module doc comment noting the NaN/Infinity divergence is now pinned by a test, not just documented.

Commit: 8d6d787 — test(examples): pin NaN-result and enum-as-number review follow-ups (#7).

Checks run (all clean):

cargo fmt --all --check                                  # clean
cargo clippy --workspace --all-targets -- -D warnings     # clean, no warnings
cargo build --workspace                                   # clean
cargo test --workspace                                    # all green, incl. examples' 8 handler.rs tests (was 6, +2 above)
make generate && git status --porcelain                   # no diff: committed example still matches

Self-review: ran the reviewer subagent once against this round's diff and the issue #7 acceptance criteria. No blocking findings. It noted two cosmetic/optional observations (uneven Cargo.toml comment style between the two dev-dep feature blocks; handler.rs's top doc comment wasn't updated to name the two new tests) which I'm leaving as-is — purely stylistic, not in scope. It also raised a question about the enum test asserting jsonschema's exact two-part error wording, which could be version-sensitive; I kept the exact-match style since every other validation-error test in this PR (wrong_argument_type_is_rejected..., unknown_field_including_snake_case_is_rejected...) already pins exact jsonschema wording the same way, so this stays consistent with the PR's existing pattern rather than introducing a different assertion style for one test.

No open questions beyond that. All three requested changes are done and verified; no other feedback was open (failing checks: none).