Repository navigation
fix(a2a): preserve serialized structured results - #2172
Conversation
🦋 Changeset detectedLatest commit: a560d1e The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved after static review and independent review. Structured A2A results now retain the exact JSON representation that passed the byte limit through task storage and HTTP publication, including custom toJSON output. String text results and serialization failure handling remain intact. Hosted CI remains pending; no candidate code or tests executed.
c3e30d2 to
02aba47
Compare
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Package-split integration preserves the reviewed A2A JSON serialization fix: relocated client, executor and tests are byte-identical. Documentation paths and changeset targets match the split. Static and independent review; hosted CI pending; no candidate code or tests executed.
|
Hosted CI follow-up at 02aba47: Node 24 test job failed the realtime-react-native example test |
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
The test-only change replaces racy stdout request counting with IPC acknowledgment. A2A behavior is unchanged. The prior hosted test failure has not yet been verified cleared on this head. Static and independent review; hosted CI pending; no candidate code or tests executed.
|
Hosted CI at ce6124e: coverage job timed out after 5000 ms in scripts/released-api-contract.test.mjs:2659, |
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Base integration preserves the A2A implementation. The sole additional change gives the previously timing-out API-contract test 15 seconds without changing assertions. The prior hosted timeout has not yet been verified cleared on this head. Static and independent review; hosted CI pending. No candidate code or tests executed.
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Hosted CI update at 64bc380: all eight checks completed successfully, including Linux/macOS/Windows package jobs, Node 22 and 24.3.x tests, Windows sandbox paths, and coverage. Native job evidence. Approval remains current; closing CI follow-up for this head. Earlier-head failures are historical, not current failures. No candidate code/tests or CI reruns were executed locally. |
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved the integration of the independently reviewed child-tracing fix in #2179. Shared tracing changes match that reviewed change, and existing feature code remains intact.
Reviewed commit 48aa8a1. Static and independent review only; no candidate code or tests ran locally. Hosted native qualification remains pending.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved after static follow-up and independent review. The six-file increment integrates the reviewed child-tracing changes from #2179 at 44f6e78. Child creation context, including absent context, remains authoritative for later steps and direct/CodeMode approvals. Existing feature changes remain intact; no new actionable findings. Hosted native and Node CI remain pending. No candidate code or tests were executed locally.
…-owned-child-trace-context
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved at a560d1e. The response-encoding gate composes with checkBytes returning the serialized value, preserving structured result serialization and bounds. Shared tracing/child-admission changes match reviewed 1875314, and patch hashes match the manifests.
Current reported hosted checks: 5 SUCCESS. Coordinator and independent static review found no actionable integration regressions. No candidate code or local tests executed; no CI reruns requested.
Builds on merged #2171. Current head:
a560d1e0.This fixes structured result handling for REQ744.
A prompted agent can use a Zod transform to return a class with a
toJSON()method. The A2A executor checked the size of that JSON, then saved the original class instance. Cloning the task discarded the method. In a real HTTP exchange, the Python client received alocalfield even though the serializer producedanswer.The executor now builds the structured data part from the JSON that passed the byte limit. The task store and remote client receive the same value.
Verification
At
a560d1e0, all 46 A2A tests pass with no skips. Five normal development-package archives match the current source; all 132 native inputs and six products match shared Core691cbffb. Fresh installed ESM/CommonJS imports pass. Both SDKs served and called the other in six HTTP exchanges against the Python41f8b308wheel, covering text, custom JSON serialization and manual approval.The repository-required pair of independent reviews passed on this complete seven-file task diff. After that acceptance, the required full SDK script passed 9,549 tests with 116 skips across 368 passing test files. Source, component, repository and native-product identities stayed unchanged. Changeset validation, current-head human approval and hosted code/security reviews are complete.
Earlier qualification:
64bc3800, the repository build, runtime type check and all 83 adapter and recovery tests passed. The official Linux release from the shared tracing fix matched all 132 native build inputs and all five binary hashes. Shared tracing fix · Release build64bc3800and the Python wheel from5e05bca9. Both SDKs served and called the other in six HTTP exchanges covering text, structured output and manual approval. The Python client received{"answer":"js structured result"}from a prompted TypeScript agent with a custom serializer. CommonJS and ESM loaded both public adapter exports.ce6124ef, the full test profile passed 9,521 tests and skipped 116. The Node 22 and 24.3 CI jobs passed on that revision.