Skip to content

fix(a2a-pb): ignore unknown fields in ProtoJSON decode (spec §5.7)#81

Draft
arkavo-com wants to merge 1 commit into
a2aproject:mainfrom
arkavo-ai:fix/protojson-ignore-unknown-fields
Draft

fix(a2a-pb): ignore unknown fields in ProtoJSON decode (spec §5.7)#81
arkavo-com wants to merge 1 commit into
a2aproject:mainfrom
arkavo-ai:fix/protojson-ignore-unknown-fields

Conversation

@arkavo-com

Copy link
Copy Markdown

Problem

Spec §5.7: "Implementations SHOULD ignore unrecognized fields in messages, allowing for forward compatibility as the protocol evolves." The pbjson-generated deserializers reject unknown fields, so any peer that sends a forward-compatible extra field (or a newer protocol revision) fails to decode.

Fix

One line in a2a-pb/build.rs: .ignore_unknown_fields() on the pbjson_build::Builder (supported by the pbjson-build 0.9.0 already in the workspace). Unknown fields now route to the generated __SkipField__ arm. All existing post-generation replacements (NULL_REPEATED_FIELD_REPLACEMENTS, map handling) still match — the generated serde code lives in OUT_DIR, so no committed generated files change.

ignore_unknown_enum_variants is deliberately not enabled: unknown enum string values still error, preserving strictness where the spec doesn't ask for leniency.

Heads-up for review: one existing test fixture changed

a2a-server's test_invalid_params used {"bogus": true} — a merely-unknown field — as its invalid-params fixture. Under §5.7 semantics that payload is now valid (decodes to a default request). The test now uses a genuine type mismatch ({"message": "not-an-object"}) with the same error-code assertion, and a new test_unknown_params_fields_are_ignored pins the tolerance end-to-end through the JSON-RPC server.

Found by / verified with

Cross-SDK interop testing (a2a-conformance, scenario edge/unknown-field-tolerance; FINDINGS.md finding 2). New round-trip tests in a2a-pb/tests/ (unknown top-level + nested fields accepted; unknown enum values still rejected); cargo test --workspace 450 passed / 0 failed (baseline 447/0); fmt + clippy clean.

🤖 Generated with Claude Code

Spec §5.7: "Implementations SHOULD ignore unrecognized fields in
messages, allowing for forward compatibility." The pbjson-generated
deserializers rejected any unknown field, so a peer sending
forward-compatible extra fields caused a local deserialize error on
otherwise valid results.

Enable pbjson_build's ignore_unknown_fields() so unknown fields are
skipped during deserialization. Unknown enum string values are still
rejected (ignore_unknown_enum_variants is intentionally not enabled).
The post-generation null-repeated-field patches apply unchanged. The
generated serde code lives in OUT_DIR (not the committed src/gen
sources), so the committed files are unaffected.

The server-side invalid-params test previously used a merely-unknown
field as its bad-params fixture; that is now valid per §5.7, so it uses
a genuine type mismatch instead, and a new test pins the unknown-field
tolerance end to end.

Found by cross-SDK interop testing (a2a-conformance).

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request configures the pbjson builder to ignore unrecognized fields in messages to ensure forward compatibility per Spec §5.7. It also adds tests to verify that unknown fields are ignored at both top-level and nested levels, while unknown enum string values are still rejected. Additionally, the JSON-RPC tests are updated to align with this new behavior. There are no review comments, so I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

88plug added a commit to 88plug/a2a-rs that referenced this pull request Jun 24, 2026
Adopt the WebSocket transport (new a2a-websocket crate: client + server +
common + errors + e2e tests) from upstream PR a2aproject#63, rebased
onto the current tree: brought in the self-contained crate + supporting
a2a/types + proto, regenerated the protobuf rather than merging generated
code, and merged the workspace deps.

Fixed 4 server tests whose unknown-field payloads ({"bogus":true}, etc.) no
longer error now that ProtoJSON ignores unknown fields per spec §5.7 (cherry
a2aproject#81) — switched them to malformed non-object payloads so they still exercise
the INVALID_PARAMS path. 105 unit + 8 integration tests pass; clippy clean.

Co-authored-by: amitami2 <amitami2@cisco.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: ss <andrew@88plug.com>
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