test: add proto3 JSON conformance spec for the generated codecs - #644
Open
ishan-1010 wants to merge 1 commit into
Open
test: add proto3 JSON conformance spec for the generated codecs#644ishan-1010 wants to merge 1 commit into
ishan-1010 wants to merge 1 commit into
Conversation
Round-tripping a shared corpus through this SDK and a2a-python turned up four places where the generated codecs disagree with proto3 JSON. Covers all four as it.fails, plus five passing cases pinning behaviour that is already correct. No source changes. Refs a2aproject#640, a2aproject#641, a2aproject#642, a2aproject#643
🧪 Code CoverageNo coverage changes. Generated by coverage-comment.yml |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
What
Adds
test/proto_json_conformance.spec.ts, covering how the generated codecs handleproto3 JSON. Ten cases: five that pass today and pin behaviour that's already correct,
and five marked
it.failsfor the divergences in #640, #641, #642 and #643.No source changes. This documents current behaviour and gives the fixes a regression net
to land against.
Why
These messages go over the wire to SDKs generated from the same
.protowith a differenttoolchain, so the JSON has to agree. I built a differential harness that round-trips a
shared corpus through this SDK and
a2a-pythonand diffs the output, and it turned upfour disagreements:
"UNRECOGNIZED", whicha2a-pythonrejectsTimestampis an unvalidated string and is never normalized to UTCPart.dataset tonulldrops the content oneofEach is filed separately with a full repro. This PR is just the tests.
On
it.failsit.failsasserts that the body fails, so the suite is green today and goes red themoment a bug is fixed. That's deliberate: the fix can't land without flipping the marker,
so none of these can be quietly resolved and forgotten. I checked the inversion is strict
with a control test whose body passes, and vitest reports it as a failure.
The five passing cases are there as controls. If a change breaks known-good behaviour like
bytes or non-ASCII round-tripping, those go red on their own.
Where the fixes should go
All four live in
src/types/pb/a2a.ts, which is generated and markedDO NOT EDIT, and Icouldn't find a regeneration path in the repo. Two of them, the enum sentinel and the
oneof leniency, are
ts-protobehaviour rather than anything configurable, so I didn'twant to guess. Happy to send the fixes once you've said whether you'd prefer them patched
into the generated output, handled in a normalization layer at the transport boundary, or
raised upstream.
Verification
Reproduced against
@a2a-js/sdk1.0.1 from npm as well asmainat1c6eb32, so none ofthis is a main-only artifact. Python side pinned at
a2a-pythoncff6727.Parts of this were put together with AI assistance. Every case was verified by hand
against both SDKs before filing.
CONTRIBUTINGGuide.Refs #640, #641, #642, #643