Skip to content

feat: protojson-compatible google.protobuf.FieldMask in tool args, results and schemas #20

Closed

Description

@erikist

Part of #2. Decision from @erikist (2026-10-09): make google.protobuf.FieldMask match Go's protojson. Leave Any pbjson-shaped and document it.

Why

pbjson-types 0.8 doesn't special-case FieldMask; its generated serde treats it as a plain message, {"paths": ["a", "b.c"]}. Go's protojson encodes it as one comma-joined string of lowerCamel paths ("a,b.c"), and Go's tool schema is {"type":"string"}. FieldMask is common in AIP-style Update* RPCs, so a tool for one of those currently has a different schema, and a different wire format, from Go's.

Do

  • Schema (feat: generate JSON Schema for tool inputs from proto descriptors (port from Go) #5's schema.rs): FieldMask → {"type":"string"}, as in Go.
  • Arguments: a FieldMask arrives as protojson's comma-joined lowerCamel string and decodes into the request's pbjson_types::FieldMask with snake_case paths, as protojson does.
  • Results: a FieldMask in a response is encoded as protojson's string.
  • Approach is your call; justify it in the PR. Options:
    • (a) The generator knows statically where FieldMask fields sit in each request and response message, including nested, repeated and map fields. It emits small JSON rewrites that run before the pbjson decode and after the pbjson encode.
    • (b) Point .google.protobuf.FieldMask at a protojson-compatible type via extern_path in the make generate / prost-serde configuration. Document what users must configure.
    • Option (a) needs nothing from users' build setup; prefer it unless it turns out unworkable.
  • Recursion: recursive messages must still work.
  • README: add a "Differences from protoc-gen-go-mcp" section (or tell docs: README parity with protoc-gen-go-mcp #13 what goes in it) covering:
    • Any is pbjson-shaped: {"typeUrl", "value"} with a base64 value.
    • Non-finite floats in results come out as null.
    • No editions support.

Acceptance criteria

  • The schema for a FieldMask field is JSON-equal to Go's.
  • An end-to-end or handler test sends {"updateMask":"vibe,someField"} and checks that the backend receives paths ["vibe","some_field"]. A response holding a FieldMask comes back as a comma-joined string.
  • Coverage includes a FieldMask nested inside a message field and one inside a repeated field.
  • The checks in AGENTS.md pass, and make generate and the golden file are current.

Activity

  1. erikist commented on Oct 10, 2026

    @erikist
    ContributorAuthor

    🤖 Factory: picked up by dispatcher; working on branch factory/20-feat-protojson-compatible-google-protobuf-fieldmask.

  2. erikist commented on Oct 10, 2026

    @erikist
    ContributorAuthor

    🔍 Guidance: a concrete design, so the next attempt can start writing code right away. The first attempt ran about 100 minutes with no commits. Use approach (a) from the issue; it needs nothing in users' build setup.

    1. Schema (schema.rs)

    In the well-known-type table, map google.protobuf.FieldMask to {"type":"string"}, as Go does. Update the test that pinned {"paths":[…]}.

    2. Generated JSON rewrites (server.rs)

    At codegen time, the generator has every message descriptor. For each request and response message type that transitively contains a FieldMask field, emit two private functions per message:

    // args: protojson string -> pbjson {"paths":[...]}; runs AFTER schema validation, BEFORE pbjson decode
    fn vibe_service_fm_in_examples_v1_update_vibe_request(v: &mut ::serde_json::Value) {
        let Some(obj) = v.as_object_mut() else { return };
        if let Some(f) = obj.get_mut("updateMask") { __fm_str_to_obj(f); }                 // FieldMask field
        if let Some(f) = obj.get_mut("inner") { vibe_service_fm_in_examples_v1_inner(f); }  // message field
        if let Some(::serde_json::Value::Array(items)) = obj.get_mut("masks") {             // repeated FieldMask
            for f in items { __fm_str_to_obj(f); }
        }
        // map<_, Msg>: iterate obj.get_mut(..).as_object_mut().values_mut()
    }
    // results: pbjson {"paths":[...]} -> protojson string; runs AFTER pbjson encode (serde_json::to_value, then to_string)
    fn vibe_service_fm_out_examples_v1_update_vibe_response(v: &mut ::serde_json::Value) { /* mirror image, using __fm_obj_to_str */ }
    • Key the property names by json_name. That's what the schema allows and what pbjson emits.
    • Emit each function once per message (memoize by full name). References between messages are plain calls, so recursive messages just recurse.
    • Emit nothing for messages with no FieldMask anywhere below them, and leave the handler unchanged for those RPCs. For example, nothing in example.proto should change, so the golden file and test: end-to-end test with a Go-parity fixture (#9) #26's parity fixture stay byte-identical.

    Plus two small helpers, once per generated file:

    fn __fm_str_to_obj(v: &mut Value) {        // "fooBar,a.bC" -> {"paths":["foo_bar","a.b_c"]}
        if let Value::String(s) = v {
            let paths: Vec<Value> = s.split(',').filter(|p| !p.is_empty())
                .map(|p| Value::String(p.split('.').map(lower_camel_to_snake).collect::<Vec<_>>().join("."))).collect();
            *v = serde_json::json!({ "paths": paths });
        }
    }
    fn __fm_obj_to_str(v: &mut Value) {        // {"paths":["foo_bar","a.b_c"]} -> "fooBar,a.bC"
        if let Some(Value::Array(paths)) = v.get("paths") { /* snake_to_lower_camel per segment, join(",") */ }
    }

    lower_camel_to_snake: each ASCII uppercase X becomes _x. snake_to_lower_camel: each _x becomes X. Those are protojson's rules. An empty string means no paths.

    3. Handler order

    validate (schema says string), then fm_in on the args Value, then pbjson decode, then the tonic call, then serde_json::to_value(resp), then fm_out, then to_string.

    4. Tests

    • Add to test-fixtures/testdata/mcpgen/fixture.proto (fixtures moved there in refactor: move test fixtures out of plugin's build.rs; format generated code (#22) #28) a service with:
      • an UpdateVibe(UpdateVibeRequest) RPC whose request has a top-level FieldMask, one nested in a message field, and a repeated FieldMask
      • a response that echoes a FieldMask
    • Assert:
      • the backend receives paths ["vibe","some_field"] for {"updateMask":"vibe,someField"}
      • nested and repeated masks are converted
      • the result comes back as "vibe,someField"
      • the schema for the field is {"type":"string"}, JSON-equal to Go's
    • Golden and parity tests must stay unchanged.

    Commit after each step (schema, then generator, then tests). A push resets the 2-hour limit.

  3. erikist commented on Oct 10, 2026

    @erikist
    ContributorAuthor

    🤖 Factory: opened PR #29, implementing the schema/generator/test plan from the design comment above (three commits: schema → generator → tests).

    • schema.rs: google.protobuf.FieldMask → {"type": "string"}.
    • server.rs + new field_mask.rs: per-message, memoized ..._fm_in/..._fm_out rewrite functions (keyed by json_name, handling singular/repeated/map-valued FieldMask fields and nested messages, including recursive ones) wired into the handler between schema validation/pbjson decode and pbjson encode/to_string. Methods with no FieldMask anywhere generate byte-identical handler code to before — example.proto's golden file and parity fixture are unchanged.
    • Tests: a new UpdateVibe RPC in test-fixtures/testdata/mcpgen/fixture.proto with top-level, nested and repeated FieldMask fields in the request and a response echoing them back, exercised end-to-end over a real tonic backend + MCP transport, plus field_mask.rs unit tests (including a map<string, FieldMask> case).

    Checks: cargo fmt --all --check, cargo clippy --workspace --all-targets -- -D warnings, cargo build --workspace, cargo test --workspace all pass; make generate + git status confirm the committed examples are unchanged. Self-reviewed via the reviewer subagent — no blocking findings.

    Also left a note on #13 (README parity) with what the "Differences from protoc-gen-go-mcp" section should cover, per this issue's README task.

    See PR #29 for full details, test evidence and two optional follow-up gaps noted by the reviewer.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    factory:in-progressAn sf-factory worker is on itfactory:readyReady for the sf-factory to pick up

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions