diff --git a/protoc-gen-rust-mcp/src/field_mask.rs b/protoc-gen-rust-mcp/src/field_mask.rs new file mode 100644 index 0000000..482ee87 --- /dev/null +++ b/protoc-gen-rust-mcp/src/field_mask.rs @@ -0,0 +1,605 @@ +//! Generates per-message JSON rewrite functions that translate a +//! `google.protobuf.FieldMask` field's JSON between pbjson's own +//! `{"paths": [...]}` encoding and protojson's single comma-joined +//! lowerCamel-path string (`schema.rs`'s `WKT_FIELD_MASK` case documents +//! why the schema says `string`; #20 is the issue this implements). +//! `server.rs`'s generated handler runs a request message's `..._fm_in` +//! rewrite on the raw JSON arguments after schema validation and before +//! the pbjson decode, and a response message's `..._fm_out` rewrite on +//! the pbjson-encoded response before `serde_json::to_string`. +//! +//! Only messages that transitively contain a FieldMask field (through +//! singular, repeated or map-valued message fields, however deeply +//! nested, including through a reference cycle) get a rewrite function; +//! every other method's generated handler is unchanged, byte for byte, +//! from before this module existed (no field in `example.proto` is a +//! FieldMask, so the golden-file and parity tests pin that this is a +//! no-op for the committed example). +//! +//! Functions and the two shared helpers are named with the owning +//! service's own `snake_case` name as a prefix (like the existing +//! `..._INPUT_SCHEMA`/`..._VALIDATOR` constants in `server.rs`), because +//! the generated `.mcp.rs` is `include!`d directly into one +//! module: two services in the same file must not emit colliding +//! top-level item names, even if (unusually) they share a message type +//! that needs a FieldMask rewrite. + +use std::collections::BTreeSet; +use std::fmt::Write as _; + +use heck::ToSnakeCase; +use prost_reflect::{Cardinality, FieldDescriptor, Kind, MessageDescriptor}; + +/// Full name of `google.protobuf.FieldMask`, duplicated from `schema.rs`'s +/// own (private) `WKT_FIELD_MASK`: the two modules care about this name +/// for different reasons (schema shape vs. JSON rewrite), not worth +/// sharing a `pub(crate)` item for one string literal. +const FIELD_MASK_FULL_NAME: &str = "google.protobuf.FieldMask"; + +/// Which way a rewrite function converts a FieldMask field's JSON. +#[derive(Clone, Copy, PartialEq, Eq)] +enum Direction { + /// protojson's string -> pbjson's `{"paths": [...]}`: run on tool call + /// arguments, after schema validation and before the pbjson decode. + In, + /// pbjson's `{"paths": [...]}` -> protojson's string: run on a + /// pbjson-encoded response, before `serde_json::to_string`. + Out, +} + +impl Direction { + fn suffix(self) -> &'static str { + match self { + Direction::In => "fm_in", + Direction::Out => "fm_out", + } + } +} + +/// Returns whether `message` contains a `google.protobuf.FieldMask` field +/// anywhere in its structure: directly, or nested inside another message +/// field (singular, repeated or map-valued), however deep, including +/// through a reference cycle. A message that only reaches a FieldMask by +/// looping back through itself is correctly treated as *not* containing +/// one: that loop never bottoms out in finitely many steps, so no real +/// value of that type can ever actually hold a FieldMask by that path +/// alone. +pub fn message_needs_rewrite(message: &MessageDescriptor) -> bool { + let mut visiting = BTreeSet::new(); + transitively_has_field_mask(message, &mut visiting) +} + +fn transitively_has_field_mask( + message: &MessageDescriptor, + visiting: &mut BTreeSet, +) -> bool { + let full_name = message.full_name().to_string(); + if visiting.contains(&full_name) { + return false; + } + visiting.insert(full_name.clone()); + let result = message + .fields() + .any(|field| field_has_field_mask(&field, visiting)); + visiting.remove(&full_name); + result +} + +/// The `Kind` a field's *value* actually carries: for a map field, that's +/// its synthetic entry message's value field (number 2); for every other +/// field (repeated or not), it's the field's own kind. +fn value_kind(field: &FieldDescriptor) -> Kind { + if field.is_map() { + let Kind::Message(entry) = field.kind() else { + unreachable!("a map field's kind is always its synthetic entry message"); + }; + entry.map_entry_value_field().kind() + } else { + field.kind() + } +} + +fn field_has_field_mask(field: &FieldDescriptor, visiting: &mut BTreeSet) -> bool { + match value_kind(field) { + Kind::Message(m) if m.full_name() == FIELD_MASK_FULL_NAME => true, + Kind::Message(m) => transitively_has_field_mask(&m, visiting), + _ => false, + } +} + +/// Accumulates the per-message rewrite functions (and the two shared +/// helpers) needed for one service's generated code, memoizing by message +/// full name + direction so a message reached from more than one method +/// (or through more than one field) gets exactly one function per +/// direction, however many times it's referenced. +pub struct FieldMaskCodegen { + service_snake: String, + emitted: BTreeSet<(String, &'static str)>, + helpers_emitted: bool, + functions: String, +} + +impl FieldMaskCodegen { + pub fn new(service_name: &str) -> Self { + Self { + service_snake: service_name.to_snake_case(), + emitted: BTreeSet::new(), + helpers_emitted: false, + functions: String::new(), + } + } + + /// If `message` (an RPC's request message) transitively contains a + /// FieldMask, ensures its `..._fm_in` function (and every function it + /// calls) is in [`functions`](Self::functions), and returns its name. + /// Returns `None` if `message` has no FieldMask anywhere in it, in + /// which case the caller emits no rewrite call at all. + pub fn ensure_in(&mut self, message: &MessageDescriptor) -> Option { + self.ensure(message, Direction::In) + } + + /// The `..._fm_out` counterpart of [`ensure_in`](Self::ensure_in), for + /// an RPC's response message. + pub fn ensure_out(&mut self, message: &MessageDescriptor) -> Option { + self.ensure(message, Direction::Out) + } + + /// Every function definition accumulated so far (the two shared + /// helpers, if any message needed them, followed by one function per + /// `(message, direction)` pair actually used), in the order `ensure` + /// first reached each one (depth-first). + pub fn functions(&self) -> &str { + &self.functions + } + + fn ensure(&mut self, message: &MessageDescriptor, direction: Direction) -> Option { + if !message_needs_rewrite(message) { + return None; + } + let fn_name = self.fn_name(message, direction); + let key = (message.full_name().to_string(), direction.suffix()); + if self.emitted.contains(&key) { + return Some(fn_name); + } + // Mark emitted before recursing into nested fields: a reference + // cycle (a recursive message with a FieldMask elsewhere in the + // cycle) then finds itself already "emitted" on the way back + // around and just calls the function being built, instead of + // recursing forever. Plain function calls don't need forward + // declarations at file scope, so it's fine that the function + // body for `fn_name` hasn't actually been appended to + // `self.functions` yet when a nested call site refers to it. + self.emitted.insert(key); + self.ensure_helpers(); + + let mut body = String::new(); + let _ = writeln!( + body, + "fn {fn_name}(v: &mut ::serde_json::Value) {{\n \ + let ::std::option::Option::Some(obj) = v.as_object_mut() else {{\n return;\n }};" + ); + for field in message.fields() { + self.emit_field(&mut body, &field, direction); + } + body.push_str("}\n\n"); + + self.functions.push_str(&body); + Some(fn_name) + } + + /// Appends one field's rewrite, if any, to `body`: a direct call to + /// the shared `..._fm_str_to_obj`/`..._fm_obj_to_str` helper for a + /// FieldMask-typed field, or a call to another message's own + /// `ensure`-built function for a message-typed field that itself + /// transitively contains a FieldMask (nothing is emitted for a field + /// that is neither), across the three shapes a field can have: + /// singular, repeated (a JSON array) and map (a JSON object keyed by + /// the map's string key). + fn emit_field(&mut self, body: &mut String, field: &FieldDescriptor, direction: Direction) { + let json_name = field.json_name(); + let helper = match direction { + Direction::In => format!("{}_fm_str_to_obj", self.service_snake), + Direction::Out => format!("{}_fm_obj_to_str", self.service_snake), + }; + + let is_map = field.is_map(); + let is_repeated = !is_map && field.cardinality() == Cardinality::Repeated; + + let call_name: Option = match value_kind(field) { + Kind::Message(m) if m.full_name() == FIELD_MASK_FULL_NAME => Some(helper), + Kind::Message(m) => self.ensure(&m, direction), + _ => None, + }; + let Some(call_name) = call_name else { + return; + }; + + if is_map { + let _ = writeln!( + body, + " if let ::std::option::Option::Some(::serde_json::Value::Object(map)) = obj.get_mut({json_name:?}) {{\n \ + for v in map.values_mut() {{\n {call_name}(v);\n }}\n \ + }}" + ); + } else if is_repeated { + let _ = writeln!( + body, + " if let ::std::option::Option::Some(::serde_json::Value::Array(items)) = obj.get_mut({json_name:?}) {{\n \ + for v in items {{\n {call_name}(v);\n }}\n \ + }}" + ); + } else { + let _ = writeln!( + body, + " if let ::std::option::Option::Some(f) = obj.get_mut({json_name:?}) {{\n {call_name}(f);\n }}" + ); + } + } + + fn fn_name(&self, message: &MessageDescriptor, direction: Direction) -> String { + format!( + "{}_{}_{}", + self.service_snake, + direction.suffix(), + message.full_name().to_snake_case() + ) + } + + fn ensure_helpers(&mut self) { + if self.helpers_emitted { + return; + } + self.helpers_emitted = true; + self.functions + .push_str(&field_mask_helpers(&self.service_snake)); + } +} + +/// The two shared helpers every `..._fm_in`/`..._fm_out` function calls +/// for an actual FieldMask-typed field (as opposed to a nested message +/// field, which instead calls that message's own `ensure`-built +/// function), plus the two path-segment case converters they're built +/// on. Emitted once per service (prefixed with its own `snake_case` name, +/// like every other item this module emits, to avoid colliding with +/// another service's copy in the same generated file). +fn field_mask_helpers(service_snake: &str) -> String { + const TEMPLATE: &str = "\ +/// Converts a FieldMask field's protojson string (comma-joined lowerCamel +/// paths, e.g. `\"fooBar,a.bC\"`) into pbjson's own `{\"paths\": [...]}` +/// shape (snake_case paths), so the pbjson-generated `Deserialize` can +/// decode it. A no-op if `v` is not a string (schema validation already +/// rejected that before this runs). An empty string means no paths, per +/// protojson's `unmarshalFieldMask`. +fn SERVICE_fm_str_to_obj(v: &mut ::serde_json::Value) { + let ::serde_json::Value::String(s) = v else { + return; + }; + let paths: ::std::vec::Vec<::serde_json::Value> = s + .split(',') + .filter(|p| !p.is_empty()) + .map(|p| { + ::serde_json::Value::String( + p.split('.') + .map(SERVICE_fm_lower_camel_to_snake) + .collect::<::std::vec::Vec<_>>() + .join(\".\"), + ) + }) + .collect(); + *v = ::serde_json::json!({ \"paths\": paths }); +} + +/// The reverse of [`SERVICE_fm_str_to_obj`]: pbjson's `{\"paths\": [...]}` +/// shape back into protojson's comma-joined lowerCamel string. A +/// zero-valued FieldMask (no paths) pbjson-encodes as `{}` (a `\"paths\"` +/// array is only present when non-empty, the usual pbjson/protojson +/// zero-value-omission rule), which this maps to `\"\"`, not left as an +/// empty JSON object: a no-op here would otherwise send a tool caller an +/// `{}` object for what the schema promises is always a string. A no-op +/// only if `v` is not even a JSON object (unreachable for a real +/// FieldMask field, since schema validation on the way in and this +/// generator's own pbjson encoding on the way out both guarantee that +/// shape in practice). +fn SERVICE_fm_obj_to_str(v: &mut ::serde_json::Value) { + let ::std::option::Option::Some(obj) = v.as_object() else { + return; + }; + let paths = match obj.get(\"paths\") { + ::std::option::Option::Some(::serde_json::Value::Array(paths)) => paths.as_slice(), + _ => &[], + }; + let joined = paths + .iter() + .filter_map(|p| p.as_str()) + .map(|p| { + p.split('.') + .map(SERVICE_fm_snake_to_lower_camel) + .collect::<::std::vec::Vec<_>>() + .join(\".\") + }) + .collect::<::std::vec::Vec<_>>() + .join(\",\"); + *v = ::serde_json::Value::String(joined); +} + +/// Converts a single lowerCamel path segment to snake_case, protojson's +/// own rule (`strs.JSONSnakeCase` in `google.golang.org/protobuf`): each +/// ASCII uppercase letter becomes `_` followed by its lowercase form. +fn SERVICE_fm_lower_camel_to_snake(s: &str) -> ::std::string::String { + let mut out = ::std::string::String::with_capacity(s.len()); + for c in s.chars() { + if c.is_ascii_uppercase() { + out.push('_'); + out.push(c.to_ascii_lowercase()); + } else { + out.push(c); + } + } + out +} + +/// Converts a single snake_case path segment to lowerCamel, the reverse +/// of [`SERVICE_fm_lower_camel_to_snake`] (`strs.JSONCamelCase`): each +/// `_x` becomes `X`, and any other underscore is dropped. +fn SERVICE_fm_snake_to_lower_camel(s: &str) -> ::std::string::String { + let mut out = ::std::string::String::with_capacity(s.len()); + let mut was_underscore = false; + for c in s.chars() { + if c != '_' { + if was_underscore && c.is_ascii_lowercase() { + out.push(c.to_ascii_uppercase()); + } else { + out.push(c); + } + } + was_underscore = c == '_'; + } + out +} + +"; + TEMPLATE.replace("SERVICE", service_snake) +} + +#[cfg(test)] +mod tests { + use super::*; + use prost_types::field_descriptor_proto::{Label, Type}; + use prost_types::{DescriptorProto, FieldDescriptorProto, FileDescriptorProto}; + + fn field( + name: &str, + number: i32, + r#type: Type, + type_name: Option<&str>, + ) -> FieldDescriptorProto { + FieldDescriptorProto { + name: Some(name.to_string()), + number: Some(number), + label: Some(Label::Optional as i32), + r#type: Some(r#type as i32), + type_name: type_name.map(|s| s.to_string()), + ..Default::default() + } + } + + fn repeated_field( + name: &str, + number: i32, + r#type: Type, + type_name: Option<&str>, + ) -> FieldDescriptorProto { + FieldDescriptorProto { + label: Some(Label::Repeated as i32), + ..field(name, number, r#type, type_name) + } + } + + /// A `map` field's synthetic entry message + /// (`MapEntry.key`/`MapEntry.value`, the `map_entry = true` option), + /// used by [`pool`] to build `fmtest.WithMaskMap.masks`. + fn map_entry_message( + name: &str, + value_type: Type, + value_type_name: Option<&str>, + ) -> DescriptorProto { + DescriptorProto { + name: Some(name.to_string()), + field: vec![ + field("key", 1, Type::String, None), + field("value", 2, value_type, value_type_name), + ], + options: Some(prost_types::MessageOptions { + map_entry: Some(true), + ..Default::default() + }), + ..Default::default() + } + } + + /// Builds a pool with: + /// - `fmtest.Leaf`: no FieldMask anywhere (just a string field). + /// - `fmtest.WithMask`: a direct FieldMask field. + /// - `fmtest.Nested`: a message field pointing at `WithMask`. + /// - `fmtest.Cyclic`: a self-referential field and nothing else (no + /// FieldMask reachable, even though it's recursive). + /// - `fmtest.WithMaskMap`: a `map` field. + fn pool() -> prost_reflect::DescriptorPool { + let leaf = DescriptorProto { + name: Some("Leaf".to_string()), + field: vec![field("name", 1, Type::String, None)], + ..Default::default() + }; + let with_mask = DescriptorProto { + name: Some("WithMask".to_string()), + field: vec![field( + "mask", + 1, + Type::Message, + Some(".google.protobuf.FieldMask"), + )], + ..Default::default() + }; + let nested = DescriptorProto { + name: Some("Nested".to_string()), + field: vec![field("inner", 1, Type::Message, Some(".fmtest.WithMask"))], + ..Default::default() + }; + let cyclic = DescriptorProto { + name: Some("Cyclic".to_string()), + field: vec![field("child", 1, Type::Message, Some(".fmtest.Cyclic"))], + ..Default::default() + }; + let masks_entry = map_entry_message( + "MasksEntry", + Type::Message, + Some(".google.protobuf.FieldMask"), + ); + let with_mask_map = DescriptorProto { + name: Some("WithMaskMap".to_string()), + field: vec![{ + let mut f = field( + "masks", + 1, + Type::Message, + Some(".fmtest.WithMaskMap.MasksEntry"), + ); + f.label = Some(Label::Repeated as i32); + f + }], + nested_type: vec![masks_entry], + ..Default::default() + }; + let field_mask = DescriptorProto { + name: Some("FieldMask".to_string()), + field: vec![repeated_field("paths", 1, Type::String, None)], + ..Default::default() + }; + let field_mask_file = FileDescriptorProto { + name: Some("google/protobuf/field_mask.proto".to_string()), + package: Some("google.protobuf".to_string()), + message_type: vec![field_mask], + syntax: Some("proto3".to_string()), + ..Default::default() + }; + let file = FileDescriptorProto { + name: Some("fmtest.proto".to_string()), + package: Some("fmtest".to_string()), + dependency: vec!["google/protobuf/field_mask.proto".to_string()], + message_type: vec![leaf, with_mask, nested, cyclic, with_mask_map], + syntax: Some("proto3".to_string()), + ..Default::default() + }; + prost_reflect::DescriptorPool::from_file_descriptor_set(prost_types::FileDescriptorSet { + file: vec![field_mask_file, file], + }) + .expect("building descriptor pool") + } + + #[test] + fn message_without_field_mask_does_not_need_rewrite() { + let pool = pool(); + let leaf = pool.get_message_by_name("fmtest.Leaf").unwrap(); + assert!(!message_needs_rewrite(&leaf)); + } + + #[test] + fn message_with_direct_field_mask_needs_rewrite() { + let pool = pool(); + let with_mask = pool.get_message_by_name("fmtest.WithMask").unwrap(); + assert!(message_needs_rewrite(&with_mask)); + } + + #[test] + fn message_with_nested_field_mask_needs_rewrite() { + let pool = pool(); + let nested = pool.get_message_by_name("fmtest.Nested").unwrap(); + assert!(message_needs_rewrite(&nested)); + } + + #[test] + fn purely_cyclic_message_without_field_mask_does_not_need_rewrite() { + let pool = pool(); + let cyclic = pool.get_message_by_name("fmtest.Cyclic").unwrap(); + assert!(!message_needs_rewrite(&cyclic)); + } + + #[test] + fn message_with_map_valued_field_mask_needs_rewrite() { + let pool = pool(); + let with_mask_map = pool.get_message_by_name("fmtest.WithMaskMap").unwrap(); + assert!(message_needs_rewrite(&with_mask_map)); + } + + /// A map field's rewrite iterates its values (not its entries or + /// keys), calling the shared `..._fm_str_to_obj` helper directly on + /// each one, since a `map`'s value is a FieldMask + /// itself, not a message that in turn contains one. + #[test] + fn map_valued_field_mask_rewrite_iterates_values() { + let pool = pool(); + let with_mask_map = pool.get_message_by_name("fmtest.WithMaskMap").unwrap(); + let mut codegen = FieldMaskCodegen::new("MyService"); + let fn_name = codegen.ensure_in(&with_mask_map).expect("needs rewrite"); + let functions = codegen.functions(); + assert!(functions.contains(&format!("fn {fn_name}"))); + assert!(functions.contains("obj.get_mut(\"masks\")")); + assert!(functions.contains("map.values_mut()")); + assert!(functions.contains("my_service_fm_str_to_obj(v)")); + } + + #[test] + fn ensure_returns_none_for_message_without_field_mask() { + let pool = pool(); + let leaf = pool.get_message_by_name("fmtest.Leaf").unwrap(); + let mut codegen = FieldMaskCodegen::new("MyService"); + assert_eq!(codegen.ensure_in(&leaf), None); + assert_eq!(codegen.functions(), ""); + } + + #[test] + fn ensure_is_memoized_by_message_and_direction() { + let pool = pool(); + let with_mask = pool.get_message_by_name("fmtest.WithMask").unwrap(); + let mut codegen = FieldMaskCodegen::new("MyService"); + let first = codegen.ensure_in(&with_mask).expect("needs rewrite"); + let second = codegen.ensure_in(&with_mask).expect("needs rewrite"); + assert_eq!(first, second); + // Only emitted once despite being requested twice: the helpers + // plus exactly one function definition for WithMask's "in". + assert_eq!( + codegen + .functions() + .matches("fn my_service_fm_in_fmtest_with_mask") + .count(), + 1 + ); + } + + #[test] + fn nested_message_rewrite_calls_the_inner_messages_function() { + let pool = pool(); + let nested = pool.get_message_by_name("fmtest.Nested").unwrap(); + let mut codegen = FieldMaskCodegen::new("MyService"); + let fn_name = codegen.ensure_in(&nested).expect("needs rewrite"); + assert_eq!(fn_name, "my_service_fm_in_fmtest_nested"); + let functions = codegen.functions(); + assert!(functions.contains("fn my_service_fm_in_fmtest_nested")); + assert!(functions.contains("fn my_service_fm_in_fmtest_with_mask")); + assert!(functions.contains("my_service_fm_in_fmtest_with_mask(f)")); + } + + #[test] + fn lower_camel_to_snake_matches_protojson() { + let helpers = field_mask_helpers("svc"); + // Spot-check the generated helper source compiles the expected + // logic by re-implementing it here isn't useful (it's the same + // code either way); instead this pins the exact rule in prose so + // a future edit to the template can't silently drift from + // protojson's strs.JSONSnakeCase/JSONCamelCase without a second + // regression test (handler-level, in server.rs's own tests) + // failing too. + assert!(helpers.contains("is_ascii_uppercase")); + assert!(helpers.contains("is_ascii_lowercase")); + } +} diff --git a/protoc-gen-rust-mcp/src/lib.rs b/protoc-gen-rust-mcp/src/lib.rs index ef44ce8..d1d7e1f 100644 --- a/protoc-gen-rust-mcp/src/lib.rs +++ b/protoc-gen-rust-mcp/src/lib.rs @@ -6,6 +6,7 @@ //! drives this library over stdin/stdout. pub mod comments; +pub mod field_mask; pub mod generator; pub mod header; pub mod rust_literal; diff --git a/protoc-gen-rust-mcp/src/schema.rs b/protoc-gen-rust-mcp/src/schema.rs index b83d558..38ce173 100644 --- a/protoc-gen-rust-mcp/src/schema.rs +++ b/protoc-gen-rust-mcp/src/schema.rs @@ -5,12 +5,20 @@ //! rule, reproduced here where it affects this port. //! //! The schema describes exactly what the pbjson-generated serde (this -//! repo's counterpart of Go's `protojson`) accepts for the message. The two -//! mappings agree on every rule below, with two deliberate exceptions -//! documented on [`any_schema`] and [`well_known_type_schema`]'s -//! `google.protobuf.FieldMask` case: pbjson does not special-case -//! `Any` or `FieldMask` the way `protojson` does, and instead encodes both -//! as the plain message they are on the wire. +//! repo's counterpart of Go's `protojson`) accepts for the message, with +//! one deliberate exception documented on [`any_schema`]: pbjson does not +//! special-case `Any` the way `protojson` does, and instead encodes it as +//! the plain message it is on the wire. +//! +//! `google.protobuf.FieldMask` is a second well-known type pbjson does not +//! special-case (see [`well_known_type_schema`]'s `WKT_FIELD_MASK` case), +//! but unlike `Any` this module's schema for it (`{"type": "string"}`) +//! matches `protojson`, not pbjson's own serde (which would need +//! `{"type": "object", "properties": {"paths": ...}}` to describe what it +//! actually accepts): per #20's decision, `server.rs`'s generated code +//! rewrites a FieldMask field's JSON between schema validation and pbjson +//! decode/encode, so the schema here describes what a caller actually +//! sees and sends, not pbjson's own serde in isolation. //! //! A third, narrower difference (documented on [`float_schema`]) is in //! float/double **output**, not input: pbjson serializes non-finite values @@ -334,22 +342,24 @@ fn well_known_type_schema(full_name: &str) -> Option { json!({}) } WKT_LIST_VALUE => json!({"type": "array"}), - // **pbjson/protojson difference:** protojson encodes a FieldMask as - // a single comma-joined string of its paths (e.g. `"a,b.c"`). - // pbjson-types has no special case for FieldMask (unlike Timestamp, - // Duration, Struct, Value, ListValue and the wrappers, each of - // which has a hand-written `Serialize`/`Deserialize` impl in - // `pbjson-types`' source; see its `build.rs`'s `exclude` list, - // which FieldMask is conspicuously absent from): the generated - // serde instead treats it as the plain one-field message it is on - // the wire, serializing it as `{"paths": ["a", "b.c"]}`. - // `tests::schema_test_message`'s `updateMask` case pins down the - // current pbjson behavior this is based on. - WKT_FIELD_MASK => json!({ - "type": "object", - "properties": {"paths": {"type": "array", "items": {"type": "string"}}}, - "additionalProperties": false, - }), + // **Matches protojson, not pbjson's own serde.** pbjson-types has + // no special case for FieldMask (unlike Timestamp, Duration, + // Struct, Value, ListValue and the wrappers, each of which has a + // hand-written `Serialize`/`Deserialize` impl in `pbjson-types`' + // source; see its `build.rs`'s `exclude` list, which FieldMask is + // conspicuously absent from): the generated serde instead treats + // it as the plain one-field message it is on the wire, + // serializing it as `{"paths": ["a", "b_c"]}`. protojson instead + // encodes a FieldMask as a single comma-joined string of + // lowerCamel paths (e.g. `"a,bC"`), per #20's decision to match + // Go rather than pbjson here: the schema says `string`, and + // `server.rs`'s generated per-message `..._fm_in`/`..._fm_out` + // functions rewrite the JSON between schema validation and + // pbjson decode (arguments), and between pbjson encode and + // `to_string` (results), so this is the schema users actually see + // and the wire format callers actually get, even though pbjson's + // own serde never produces or accepts this shape directly. + WKT_FIELD_MASK => json!({"type": "string"}), WKT_EMPTY => json!({"type": "object", "additionalProperties": false}), WKT_ANY => any_schema(), WKT_DOUBLE_VALUE | WKT_FLOAT_VALUE => float_schema(), @@ -704,14 +714,7 @@ mod tests { ("metadata", json!({"type": "object"})), ("anyValue", json!({})), ("anyList", json!({"type": "array"})), - ( - "updateMask", - json!({ - "type": "object", - "properties": {"paths": {"type": "array", "items": {"type": "string"}}}, - "additionalProperties": false, - }), - ), + ("updateMask", json!({"type": "string"})), ( "nothing", json!({"type": "object", "additionalProperties": false}), @@ -877,6 +880,15 @@ mod tests { /// and checks that each one, pbjson-serialized, validates against the /// schema generated for `SchemaTestMessage`, per the issue's /// property-test acceptance criterion. + /// + /// `random_schema_test_message` deliberately never sets `update_mask`: + /// pbjson's own serde for `FieldMask` (`{"paths": [...]}`) no longer + /// matches this module's `{"type": "string"}` schema for it (#20): + /// only `server.rs`'s generated per-message `..._fm_in`/`..._fm_out` + /// rewrites, not pbjson's serde in isolation, turn one into the + /// other, so validating a raw pbjson-serialized `update_mask` against + /// this schema here would fail by design. `server.rs`'s own tests + /// cover the FieldMask rewrite end to end. #[test] fn schema_validates_populated_messages() { let msg = load_message("SchemaTestMessage"); @@ -1028,9 +1040,8 @@ mod tests { }, ], }); - m.update_mask = Some(pbjson_types::FieldMask { - paths: vec!["name".to_string(), "root.value".to_string()], - }); + // update_mask is deliberately left unset: see this function's + // caller, schema_validates_populated_messages, for why. m.nothing = Some(pbjson_types::Empty {}); m.score_wrapper = Some(pbjson_types::DoubleValue { value: rng.random(), diff --git a/protoc-gen-rust-mcp/src/server.rs b/protoc-gen-rust-mcp/src/server.rs index c6cda4b..d232871 100644 --- a/protoc-gen-rust-mcp/src/server.rs +++ b/protoc-gen-rust-mcp/src/server.rs @@ -91,6 +91,7 @@ use prost_reflect::{MessageDescriptor, MethodDescriptor, ServiceDescriptor}; use serde_json::Value; use crate::comments::{camel_to_space, process_comment_to_string}; +use crate::field_mask::FieldMaskCodegen; use crate::rust_literal::rust_string_literal; use crate::schema::message_input_schema; use crate::source_info::leading_comments; @@ -157,6 +158,14 @@ pub fn generate_service(service: &ServiceDescriptor) -> String { out.push('\n'); } + // Per-message FieldMask JSON rewrite functions (#20): populated below + // as method_handler_fn asks for a request/response message's + // ..._fm_in/..._fm_out function (memoized, so a message type shared + // across methods only gets one function per direction). Emits + // nothing for a service with no FieldMask field anywhere in its + // methods' messages, same as before this existed. + let mut field_mask_codegen = FieldMaskCodegen::new(service_name); + let _ = write!( out, "/// A registered tool's handler: takes the raw tool call arguments\n\ @@ -221,7 +230,12 @@ pub fn generate_service(service: &ServiceDescriptor) -> String { for method in &unary_methods { out.push_str(&method_tool_fn(service_name, method)); out.push('\n'); - out.push_str(&method_handler_fn(service_name, &client_path, method)); + out.push_str(&method_handler_fn( + service_name, + &client_path, + method, + &mut field_mask_codegen, + )); out.push('\n'); out.push_str(&method_public_handler_fn(method)); out.push('\n'); @@ -327,6 +341,11 @@ pub fn generate_service(service: &ServiceDescriptor) -> String { }}\n" ); + if !field_mask_codegen.functions().is_empty() { + out.push('\n'); + out.push_str(field_mask_codegen.functions()); + } + out } @@ -437,24 +456,80 @@ fn validator_constant(service_name: &str, method: &MethodDescriptor) -> String { /// 1. Validates them against the method's `..._VALIDATOR`, returning a /// tool error listing every failing instance path and reason (backend /// not called) on failure. -/// 2. Decodes them into the request message with +/// 2. If the request message transitively contains a FieldMask field +/// (#20), rewrites every such field's JSON in place from protojson's +/// comma-joined lowerCamel string to pbjson's own `{"paths": [...]}` +/// shape, via the request message's `..._fm_in` function +/// ([`FieldMaskCodegen::ensure_in`]). A no-op (this step is skipped +/// entirely) for a request with no FieldMask anywhere in it. +/// 3. Decodes them into the request message with /// `serde_json::from_value` (the pbjson-generated `Deserialize`), /// returning a tool error on failure (backend not called). -/// 3. Calls the tonic client, returning a tool error formatted like +/// 4. Calls the tonic client, returning a tool error formatted like /// grpc-go's `Status.Error()` on a `tonic::Status`. -/// 4. Encodes the response with `serde_json::to_string` (the -/// pbjson-generated `Serialize`) and returns it as a successful tool -/// result. +/// 5. Encodes the response with `serde_json::to_value` (the +/// pbjson-generated `Serialize`), and, symmetrically with step 2, runs +/// the response message's `..._fm_out` function on the result if it +/// transitively contains a FieldMask, then `serde_json::to_string`s +/// it and returns that as a successful tool result. /// /// Named `call_` (not `_handler`, which is the public /// wrapper in [`method_public_handler_fn`]) because `register_default_tools` /// builds its closures from this one directly, cloning `self.client` once /// per registration rather than once per call. -fn method_handler_fn(service_name: &str, client_path: &str, method: &MethodDescriptor) -> String { +fn method_handler_fn( + service_name: &str, + client_path: &str, + method: &MethodDescriptor, + field_mask_codegen: &mut FieldMaskCodegen, +) -> String { let method_name = method.name(); let fn_name = method_fn_name(method); let validator_const = validator_const_name(service_name, method); let request_path = rust_message_path(&method.input()); + + // `None` for a request/response message with no FieldMask anywhere in + // it. In that (overwhelmingly common) case, the handler body below is + // byte-for-byte identical to what this generator produced before #20 + // (the golden file and example.proto's parity fixture, neither of + // which has a FieldMask field, pin that): the `args`/`response value` + // bindings stay immutable and go straight from validate to decode, or + // from the tonic call to `serde_json::to_string`, with no extra + // rewrite step or `let mut` spliced in. + let fm_in_fn = field_mask_codegen.ensure_in(&method.input()); + let fm_out_fn = field_mask_codegen.ensure_out(&method.output()); + + let args_binding = if fm_in_fn.is_some() { + "mut args" + } else { + "args" + }; + let fm_in_call = fm_in_fn + .map(|fn_name| format!("\u{20} {fn_name}(&mut args);\n")) + .unwrap_or_default(); + + let response_encode = match fm_out_fn { + None => "\u{20} Ok(response) => match ::serde_json::to_string(response.get_ref()) {\n\ + \u{20} Ok(json) => ::rmcp::model::CallToolResult::success(vec![::rmcp::model::ContentBlock::text(json)]),\n\ + \u{20} Err(e) => ::rmcp::model::CallToolResult::error(vec![::rmcp::model::ContentBlock::text(\n\ + \u{20} ::std::format!(\"encoding response: {e}\"),\n\ + \u{20} )]),\n\ + \u{20} },\n".to_string(), + Some(fm_out_fn) => format!( + "\u{20} Ok(response) => match ::serde_json::to_value(response.get_ref()) {{\n\ + \u{20} Ok(mut value) => {{\n\ + \u{20} {fm_out_fn}(&mut value);\n\ + \u{20} ::rmcp::model::CallToolResult::success(vec![::rmcp::model::ContentBlock::text(\n\ + \u{20} value.to_string(),\n\ + \u{20} )])\n\ + \u{20} }}\n\ + \u{20} Err(e) => ::rmcp::model::CallToolResult::error(vec![::rmcp::model::ContentBlock::text(\n\ + \u{20} ::std::format!(\"encoding response: {{e}}\"),\n\ + \u{20} )]),\n\ + \u{20} }},\n" + ), + }; + format!( " /// Validates, decodes, calls the backend for, and encodes the\n\ \u{20} /// response of, a `{method_name}` tool call: see this module's doc\n\ @@ -463,7 +538,7 @@ fn method_handler_fn(service_name: &str, client_path: &str, method: &MethodDescr \u{20} mut client: {client_path},\n\ \u{20} args: ::std::option::Option<::rmcp::model::JsonObject>,\n\ \u{20} ) -> ::rmcp::model::CallToolResult {{\n\ - \u{20} let args = ::serde_json::Value::Object(args.unwrap_or_default());\n\ + \u{20} let {args_binding} = ::serde_json::Value::Object(args.unwrap_or_default());\n\ \u{20} let errors: ::std::vec::Vec<::std::string::String> = {validator_const}\n\ \u{20} .iter_errors(&args)\n\ \u{20} .map(|e| ::std::format!(\"{{}}: {{e}}\", e.instance_path()))\n\ @@ -473,6 +548,7 @@ fn method_handler_fn(service_name: &str, client_path: &str, method: &MethodDescr \u{20} ::std::format!(\"invalid arguments: {{}}\", errors.join(\"; \")),\n\ \u{20} )]);\n\ \u{20} }}\n\ + {fm_in_call}\ \u{20} let request: {request_path} = match ::serde_json::from_value(args) {{\n\ \u{20} Ok(r) => r,\n\ \u{20} Err(e) => {{\n\ @@ -482,12 +558,7 @@ fn method_handler_fn(service_name: &str, client_path: &str, method: &MethodDescr \u{20} }}\n\ \u{20} }};\n\ \u{20} match client.{fn_name}(::tonic::Request::new(request)).await {{\n\ - \u{20} Ok(response) => match ::serde_json::to_string(response.get_ref()) {{\n\ - \u{20} Ok(json) => ::rmcp::model::CallToolResult::success(vec![::rmcp::model::ContentBlock::text(json)]),\n\ - \u{20} Err(e) => ::rmcp::model::CallToolResult::error(vec![::rmcp::model::ContentBlock::text(\n\ - \u{20} ::std::format!(\"encoding response: {{e}}\"),\n\ - \u{20} )]),\n\ - \u{20} }},\n\ + {response_encode}\ \u{20} // grpc-go's Status.Error() format, which Go's generated server\n\ \u{20} // returns verbatim as the tool error text.\n\ \u{20} Err(status) => ::rmcp::model::CallToolResult::error(vec![::rmcp::model::ContentBlock::text(\n\ diff --git a/protoc-gen-rust-mcp/tests/server_integration.rs b/protoc-gen-rust-mcp/tests/server_integration.rs index 042dd36..41258c2 100644 --- a/protoc-gen-rust-mcp/tests/server_integration.rs +++ b/protoc-gen-rust-mcp/tests/server_integration.rs @@ -85,6 +85,25 @@ impl VibeFixtureService for FakeVibeBackend { "not used by this test; VibeFixtureService.StreamVibe produces no tool", )) } + + /// Echoes every FieldMask it received straight back in the matching + /// response field, so a test can inspect exactly what paths the + /// generated `..._fm_in` rewrite decoded the request's top-level, + /// nested and repeated masks into (via the response alone, with no + /// shared mutable state across what may be parallel test runs), and + /// that the generated `..._fm_out` rewrite then runs on all three + /// shapes again on the way back out (#20). + async fn update_vibe( + &self, + request: tonic::Request, + ) -> Result, tonic::Status> { + let req = request.into_inner(); + Ok(tonic::Response::new(mcpgen::UpdateVibeResponse { + applied_mask: req.update_mask, + details: req.details, + extra_masks: req.extra_masks, + })) + } } /// An `OtherFixtureService` backend: `DoOther` echoes its input, used by @@ -179,9 +198,9 @@ where } /// `register_default_tools()` registers one tool per unary RPC (`SetVibe`, -/// `GetVibe`), by its proto method name, skipping the server-streaming -/// `StreamVibe` RPC: the issue's "a streaming RPC in the test data -/// produces no tool" criterion. +/// `GetVibe`, `UpdateVibe`), by its proto method name, skipping the +/// server-streaming `StreamVibe` RPC: the issue's "a streaming RPC in the +/// test data produces no tool" criterion. #[tokio::test] async fn register_default_tools_registers_only_unary_methods() { let mut server = mcpgen::VibeFixtureServiceMcpServer::new(lazy_vibe_client()); @@ -195,7 +214,14 @@ async fn register_default_tools_registers_only_unary_methods() { let tools = client.list_all_tools().await.expect("list_all_tools"); let mut names: Vec = tools.iter().map(|t| t.name.to_string()).collect(); names.sort_unstable(); - assert_eq!(names, vec!["GetVibe".to_string(), "SetVibe".to_string()]); + assert_eq!( + names, + vec![ + "GetVibe".to_string(), + "SetVibe".to_string(), + "UpdateVibe".to_string(), + ] + ); } /// Calling a tool that isn't registered returns rmcp's normal "tool not @@ -320,6 +346,7 @@ async fn one_server_can_serve_tools_from_two_services() { "DoOther".to_string(), "GetVibe".to_string(), "SetVibe".to_string(), + "UpdateVibe".to_string(), ] ); @@ -497,6 +524,159 @@ async fn handler_unknown_field_is_rejected_without_calling_backend() { ); } +/// The `UpdateVibe` tool's input schema maps every FieldMask field (a +/// top-level one, one nested inside a message field, and a repeated one) +/// to `{"type": "string"}`, matching Go's protojson-compatible schema for +/// FieldMask (#20's first acceptance criterion), not pbjson's own +/// `{"paths": [...]}` shape. +#[tokio::test] +async fn update_vibe_schema_maps_field_mask_to_string() { + let schema = + mcpgen::VibeFixtureServiceMcpServer::::update_vibe_tool() + .schema_as_json_value(); + assert_eq!(schema["properties"]["updateMask"]["type"], "string"); + assert_eq!( + schema["properties"]["details"]["properties"]["detailMask"]["type"], + "string" + ); + assert_eq!( + schema["properties"]["extraMasks"]["items"]["type"], + "string" + ); +} + +/// `{"updateMask": "vibe,someField"}` reaches the backend as paths +/// `["vibe", "some_field"]`, and a mask nested inside a message field +/// (`details.detailMask`) and a repeated FieldMask field (`extraMasks`) +/// are converted the same way: the generated `..._fm_in` rewrite handles +/// all three shapes (#20's second and third acceptance criteria). +/// `FakeVibeBackend::update_vibe` echoes every mask it receives straight +/// back in the matching response field, so the exact paths the backend +/// received are visible here (via `..._fm_out` converting them back to +/// protojson strings) without needing shared mutable state to inspect the +/// backend's own request. +#[tokio::test] +async fn update_vibe_decodes_top_level_nested_and_repeated_field_masks() { + let mut server = mcpgen::VibeFixtureServiceMcpServer::new(serve_vibe_backend().await); + server.register_default_tools(); + let client = serve(server).await; + + let mut args = JsonObject::new(); + args.insert( + "updateMask".to_string(), + serde_json::json!("vibe,someField"), + ); + args.insert( + "details".to_string(), + serde_json::json!({"detailMask": "a.bC"}), + ); + args.insert( + "extraMasks".to_string(), + serde_json::json!(["x,yZ", "oneTwo"]), + ); + let result = client + .call_tool(CallToolRequestParams::new("UpdateVibe").with_arguments(args)) + .await + .expect("call_tool"); + assert_eq!(result.is_error, Some(false), "{result:?}"); + let rmcp::model::ContentBlock::Text(text) = &result.content[0] else { + panic!("expected text content"); + }; + // The backend received paths ["vibe", "some_field"] for the top-level + // mask (proven by the snake_case paths pbjson encoded on the way + // out), ["a", "b_c"] for the nested one, and the two repeated masks + // split and converted the same way; `..._fm_out` then converts every + // one of them straight back to protojson's lowerCamel comma-joined + // form, which is exactly the original input string for each (proving + // the round trip is lossless for these inputs). + let value: serde_json::Value = serde_json::from_str(&text.text).expect("valid JSON"); + assert_eq!(value["appliedMask"], "vibe,someField"); + assert_eq!(value["details"]["detailMask"], "a.bC"); + assert_eq!(value["extraMasks"], serde_json::json!(["x,yZ", "oneTwo"])); +} + +/// A FieldMask in a response comes back as a single comma-joined +/// lowerCamel string, not pbjson's own `{"paths": [...]}` shape: the +/// generated `..._fm_out` rewrite runs on the pbjson-encoded result +/// (#20's second acceptance criterion, result side). +#[tokio::test] +async fn update_vibe_result_encodes_field_mask_as_comma_joined_string() { + let mut server = mcpgen::VibeFixtureServiceMcpServer::new(serve_vibe_backend().await); + server.register_default_tools(); + let client = serve(server).await; + + let mut args = JsonObject::new(); + args.insert( + "updateMask".to_string(), + serde_json::json!("vibe,someField"), + ); + let result = client + .call_tool(CallToolRequestParams::new("UpdateVibe").with_arguments(args)) + .await + .expect("call_tool"); + assert_eq!(result.is_error, Some(false), "{result:?}"); + let rmcp::model::ContentBlock::Text(text) = &result.content[0] else { + panic!("expected text content"); + }; + assert_eq!(text.text, r#"{"appliedMask":"vibe,someField"}"#); +} + +/// An empty string means no paths (protojson's rule, mirrored by the +/// generated `..._fm_str_to_obj` helper): `updateMask: ""` decodes into a +/// *present* FieldMask with zero paths (not one empty-string path), which +/// pbjson then encodes as `{}` (a message field has presence in proto3, +/// so a present-but-empty FieldMask is not omitted the way an absent one +/// would be); `..._fm_out` converts that `{}` to `""` rather than leaving +/// it as an empty object, matching what Go's protojson would also emit +/// for a present, zero-path FieldMask (`strings.Join(nil, ",")` is `""`). +#[tokio::test] +async fn update_vibe_empty_mask_string_means_no_paths() { + let mut server = mcpgen::VibeFixtureServiceMcpServer::new(serve_vibe_backend().await); + server.register_default_tools(); + let client = serve(server).await; + + let mut args = JsonObject::new(); + args.insert("updateMask".to_string(), serde_json::json!("")); + let result = client + .call_tool(CallToolRequestParams::new("UpdateVibe").with_arguments(args)) + .await + .expect("call_tool"); + assert_eq!(result.is_error, Some(false), "{result:?}"); + + let rmcp::model::ContentBlock::Text(text) = &result.content[0] else { + panic!("expected text content"); + }; + assert_eq!(text.text, r#"{"appliedMask":""}"#); +} + +/// A non-string `updateMask` fails schema validation before decoding +/// (the schema says `{"type": "string"}`, not pbjson's own object +/// shape), and the backend is never called. +#[tokio::test] +async fn update_vibe_wrong_argument_type_is_rejected_without_calling_backend() { + let mut server = mcpgen::VibeFixtureServiceMcpServer::new(lazy_vibe_client()); + server.register_default_tools(); + let client = serve(server).await; + + let mut args = JsonObject::new(); + args.insert( + "updateMask".to_string(), + serde_json::json!({"paths": ["vibe"]}), + ); + let result = client + .call_tool(CallToolRequestParams::new("UpdateVibe").with_arguments(args)) + .await + .expect("call_tool"); + assert_eq!(result.is_error, Some(true)); + let rmcp::model::ContentBlock::Text(text) = &result.content[0] else { + panic!("expected text content"); + }; + assert_eq!( + text.text, + r#"invalid arguments: /updateMask: {"paths":["vibe"]} is not of type "string""# + ); +} + /// A freshly constructed server has no tools registered yet (the issue's /// "no tools registered yet, like Go"). #[tokio::test] diff --git a/test-fixtures/build.rs b/test-fixtures/build.rs index a9a7be0..e3c44f3 100644 --- a/test-fixtures/build.rs +++ b/test-fixtures/build.rs @@ -74,7 +74,19 @@ fn generate_mcpgen_fixture() -> Result<(), Box> { let fds = protox::compile(["fixture.proto"], [&proto_dir])?; - tonic_prost_build::configure().compile_fds(fds.clone())?; + // Route google.protobuf well-known types (fixture.proto imports + // FieldMask, for #20's UpdateVibe RPC) to pbjson-types, same as + // schema.proto's own config above: tonic-prost-build's default of + // prost-types has no Serialize/Deserialize impls, which + // pbjson_build::Builder::build below needs to generate FieldMask's + // own (unused directly, since #20 rewrites its JSON before pbjson + // ever sees it) serde impl, and which this fixture's generated + // mcpgen.mcp.rs needs for UpdateVibeRequest/Response to compile at + // all (prost-types has no Serialize/Deserialize either way). + tonic_prost_build::configure() + .compile_well_known_types(true) + .extern_path(".google.protobuf", "::pbjson_types") + .compile_fds(fds.clone())?; let descriptor_bytes = { use protox::prost::Message; diff --git a/test-fixtures/testdata/mcpgen/fixture.proto b/test-fixtures/testdata/mcpgen/fixture.proto index 5a5be40..cca0c56 100644 --- a/test-fixtures/testdata/mcpgen/fixture.proto +++ b/test-fixtures/testdata/mcpgen/fixture.proto @@ -2,6 +2,8 @@ syntax = "proto3"; package mcpgen; +import "google/protobuf/field_mask.proto"; + // The request to set the vibe. message SetVibeRequest { // The vibe to set. @@ -41,6 +43,36 @@ message OtherResponse { string value = 1; } +// A message nested inside UpdateVibeRequest's own nested field, holding a +// FieldMask: used to test that a FieldMask reached through a nested +// message field (not just a top-level one) is rewritten too (#20). +message UpdateVibeDetails { + google.protobuf.FieldMask detail_mask = 1; +} + +// The request for UpdateVibe (#20): a top-level FieldMask, one nested +// inside a message field, and a repeated FieldMask, to test every shape +// the generated ..._fm_in rewrite must handle. +message UpdateVibeRequest { + // A top-level FieldMask field. + google.protobuf.FieldMask update_mask = 1; + // A FieldMask nested inside a message field. + UpdateVibeDetails details = 2; + // A repeated FieldMask field. + repeated google.protobuf.FieldMask extra_masks = 3; +} + +// The response for UpdateVibe (#20): echoes every FieldMask shape from +// the request back, so a test can check the generated ..._fm_in rewrite +// decoded the backend's paths correctly (via the response alone, instead +// of needing shared mutable state to inspect what the backend received) +// and that ..._fm_out runs on all three shapes in a result too. +message UpdateVibeResponse { + google.protobuf.FieldMask applied_mask = 1; + UpdateVibeDetails details = 2; + repeated google.protobuf.FieldMask extra_masks = 3; +} + // A service with two unary RPCs and one server-streaming RPC, used to // test that streaming RPCs produce no tool. service VibeFixtureService { @@ -50,6 +82,9 @@ service VibeFixtureService { rpc GetVibe(GetVibeRequest) returns (GetVibeResponse) {} // Streams vibes; should not produce an MCP tool. rpc StreamVibe(StreamVibeRequest) returns (stream StreamVibeResponse) {} + // Updates the vibe using a FieldMask (#20): request has a top-level, + // nested and repeated FieldMask; response echoes one back. + rpc UpdateVibe(UpdateVibeRequest) returns (UpdateVibeResponse) {} } // A second service, used to test that one MCP server can serve tools from