🦋 Changeset detected

Latest commit: 4046fa3

The changes in this PR will be included in the next version bump.

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

Claude finished @ShobhitPatra's task in 3m 51s —— View job

Review — PR #5036 (commit 4046fa3)

Verdict: solid fix, no blocking issues. The silent attachment data-loss is real (the old converter read only message.content and handled only text/image), the fix mirrors the adk/ag-ui sibling precedent correctly, and CI is fully green (build/tests, Code Quality, Changeset Semver, autofix.ci all pass; changeset ✓, api-surface snapshot regenerated ✓, no docs pages reference the changed export ✓).

The head commit is unchanged in substance since the earlier bot reviews (re-push of the same content), so this review focuses on adjudicating the open inline findings from greptile/cubic rather than re-describing the diff.

Adjudication of open bot findings

1. Non-base64 data URL falls into raw in the file case (greptile P1, cubic V3) — valid, but recommend fixing in the follow-up, not here.
conversions.ts:149-153: a file part with data: "data:text/plain,hello" fails parseDataUrl (requires ;base64,), fails httpUrlPattern, and lands in raw — which types.ts:23 documents as base64 bytes, so a receiver decodes garbage. Two mitigating facts the bot comments miss:

• It's unreachable via first-party adapters — core's attachment path builds data URLs with FileReader.readAsDataURL, which always emits ;base64,.
• It exactly mirrors the existing behavior of buildInputSource in packages/react-ag-ui/src/runtime/adapter/conversions.ts:158-174 (value: parsed?.data ?? value) and react-langchain's copy — all four copies of this helper share the flaw.

Since the previous review already flagged hoisting parseDataUrl/httpUrlPattern into @assistant-ui/core/internal as a follow-up (now the 4th byte-identical copy), the right move is to fix the fallthrough once in the shared helper rather than patch one copy and let the adapters diverge. If you prefer to fix it in this PR, a one-line guard before the raw fallback works: if (part.data.startsWith("data:")) return null; (or emit { url: part.data }, matching the image case). Fix this →

2. Zero-byte data URL fails to parse, .+ vs .* (cubic V1) — valid but marginal; same shared-copy situation.
data:<mime>;base64, (an empty file through readAsDataURL) fails the .+ payload match and takes the same corrupting fallthrough as #1. The .* suggestion is safe and trivial — but again the regex is byte-identical across four packages (this PR copied it faithfully from ag-ui), so fixing it only here creates drift. Bundle with #1 in the hoist follow-up, or fix both here together.

3. type: "data" parts dropped (greptile P1, cubic V2) — pre-existing gap, correctly out of scope; add to the follow-up.
The old code dropped everything except text/image too, so this PR doesn't regress anything — it's the same category as the audio gap the PR description already declares as follow-up. That said, the case is unusually cheap for A2A specifically: A2APart has a first-class data?: unknown payload (types.ts:25) and the inbound converter already handles it (conversions.ts:37-39), so outbound case "data": return { data: part.data } would restore round-trip symmetry. Worth naming data alongside audio in the follow-up note.

Suggested improvements

• The five near-identical object literals in the image/file cases (already flagged inline at conversions.ts:154) — a small buildPart(source, mediaType?, filename?) helper collapses them. One caution on the simplify agent's variant of this: react-langchain's merged form (raw: parsed?.data ?? part.data) would bake in the finding-Merge EditBar into Composer and revamp the Context API #1 fallthrough, so if you restructure, keep the data-URL-parse-failure path distinct.

Good practices

• Root-cause fix with sibling-adapter precedent cited and verified (adk, ag-ui), not a symptom patch.
• Removed a drifted hand-rolled duplicate of the converter in A2AThreadRuntimeCore.threadMessageToA2AMessage by delegating to the exported function.
• Both converter directions plus the runtime wire path tested; defensive ?? [] guards each covered by a dedicated runtime test.
• Append-only surface: structural input widening + optional trailing param; api-surface snapshot regenerated in-PR.
• Correctly dropped the invalid mediaType: "image/*" (a match pattern, not a MIME type) with the behavior change documented.

Simplify

The background simplify pass found no efficiency issues and two things worth noting beyond the inline comments already on the PR:

_{Reviews (1): Last reviewed commit: "fix(react-a2a): forward user attachments..." | Re-trigger Greptile}

noting the follow-up here so it's tracked with the PR: after this and #5039 merge, one small PR to finish outbound converter part coverage :

• audio parts (base64 + audio/ contentType → {raw, mediaType})
• data parts (structured payload → a2a data field) — the greptile/cubic finding
• route unparseable data: strings to url in the file branch, matching the image branch (also covers the zero-byte ;base64, case)
• tests for the fallthrough branches
• swap the local parseDataUrl/httpUrlPattern for the shared helpers from refactor: share parseDataUrl and httpUrlPattern from core internal #5039

kept out of this PR to stay on the reported bug.