feat(core): add an optional error field to TOOL_CALL_RESULT - #2549
Open
mxmzb wants to merge 12 commits into
Open
feat(core): add an optional error field to TOOL_CALL_RESULT#2549mxmzb wants to merge 12 commits into
mxmzb wants to merge 12 commits into
Conversation
Contributor
Python Preview PackagesVersion
Install with uvAdd the TestPyPI index to your [[tool.uv.index]]
name = "testpypi"
url = "https://test.pypi.org/simple/"
explicit = trueThen install the packages you need: # Core SDK
uv add 'ag-ui-protocol==0.0.0.dev1788201581' --index testpypi
# Integrations (each already depends on the matching ag-ui-protocol preview)
uv add 'ag-ui-langgraph==0.0.0.dev1788201581' --index testpypi
uv add 'ag-ui-crewai==0.0.0.dev1788201581' --index testpypi
# NOTE: ag-ui-agent-spec depends on pyagentspec (git-only, not on PyPI).
# You will need to install pyagentspec separately from its git repo.
uv add 'ag-ui-agent-spec==0.0.0.dev1788201581' --index testpypi
uv add 'ag_ui_adk==0.0.0.dev1788201581' --index testpypi
uv add 'ag_ui_strands==0.0.0.dev1788201581' --index testpypiInstall with pippip install \
--index-url https://test.pypi.org/simple/ \
--extra-index-url https://pypi.org/simple/ \
ag-ui-protocol==0.0.0.dev1788201581
Commit: f9caa8a |
@ag-ui/a2a-middleware
@ag-ui/a2ui-middleware
@ag-ui/event-throttle-middleware
@ag-ui/mcp-apps-middleware
@ag-ui/mcp-middleware
@ag-ui/a2a
@ag-ui/adk
@ag-ui/ag2
@ag-ui/agno
@ag-ui/aws-strands
@ag-ui/claude-agent-sdk
@ag-ui/claude-managed-agents
@ag-ui/crewai
@ag-ui/langchain
@ag-ui/langgraph
@ag-ui/llamaindex
@ag-ui/mastra
@ag-ui/pydantic-ai
@ag-ui/vercel-ai-sdk
@ag-ui/watsonx
@ag-ui/a2ui-toolkit
create-ag-ui-app
@ag-ui/client
@ag-ui/core
@ag-ui/encoder
@ag-ui/proto
commit: |
mxmzb
force-pushed
the
feat/tool-call-result-error
branch
from
August 26, 2026 23:04
7ff441c to
fca733a
Compare
ToolMessage carries an optional `error`. ToolCallResultEvent does not, so a UI rendering a live stream cannot show a failed tool call as failed — it has to wait for the MESSAGES_SNAPSHOT that carries the finished message, which arrives after the run and too late to be useful. Reported by S&P Global. Adds the field to TypeScript, Python and .NET as the event-side twin of ToolMessage.error, matching its naming, optionality and doc style. Purely additive: an absent error behaves exactly as today, and the tests assert that a stream produced before this field existed re-serializes byte-identically. No null tolerance, matching every field added since PNI-199 — absent is the only spelling and an explicit null fails the parse. Two changes beyond the schema, because the schema edit alone leaves the field incoherent: - @ag-ui/client threads `error` onto the ToolMessage that defaultApplyEvents accumulates. Without it the message built from the stream and the one in the MESSAGES_SNAPSHOT disagree about whether the call failed. - sdks/fixtures/null-omission.json gains tool_call_result_with_error, holding all three SDKs to the same wire text for the present case, as the existing tool_call_result_without_role already does for the absent one. The TypeScript suite carries a compile-time assertion alongside the runtime ones. BaseEventSchema passes unknown keys through, so deleting the field from the schema leaves five of six runtime cases green — the value survives parsing as an unrecognized key. Python has the same trap via extra="allow" and is guarded by asserting on model_fields. Both verified by deleting the field and watching the suites fail. Out of scope, deliberately: protobuf (TOOL_CALL_RESULT is not in the schema at all, tracked as PNI-214), producers actually populating the field, and the community SDKs, which are already behind core on subagentRunId. Refs PNI-362. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NAPvqoeqhoEhtxChZHMFoE
`sdks/fixtures/README.md` requires the `stream` array to read as one
plausible stream of events, top to bottom, so a given id means one thing
for the whole file. `tool_call_result_with_error` broke that: it used
`messageId: "msg_3"` and `toolCallId: "tc_2"`, and both are re-bound
further down the same array.
- `msg_3` is bound by `messages_snapshot_with_sparse_messages` to a tool
message carrying a SUCCESSFUL result (`{"hits":2}` for `tc_1`), so the
same message id is both a timeout failure and a success.
- `tc_2` is first minted by `assistant_message_tool_call_without_encrypted_value`
inside `msg_5`'s assistant `toolCalls` — i.e. the tool call is STARTED
after the case that reports it timing out.
Rebind the case to `msg_7` / `tc_3`, the lowest numbers unused anywhere
else in the file. Both now occur in exactly one case, so nothing later
reinterprets them and the failure reads as its own tool call.
No other case is touched. All three fixture consumers stay green:
@ag-ui/encoder 40/40, python 180/180, AGUI.Abstractions.UnitTests 323/323.
The `error` field on TOOL_CALL_RESULT is a wire-shape change, and
.github/skills/agui-cross-sdk-parity/SKILL.md is explicit that a JSON-shape
change may not land without a Compatibility fixture. The parent commit shipped
the field in all three SDKs and the shared sdks/fixtures/null-omission.json
case, but tool-call-events.json — the .NET side's guard against TypeScript wire
drift — still only held the pre-error TOOL_CALL_RESULT shape. Nothing in the
.NET tree asserted that a TS payload carrying `error` deserializes with it.
The new entry is produced by the TypeScript reference, not hand-written:
createToolCallResultEvent from packages/core/src/event-factories.ts, serialized
the way @ag-ui/encoder's encodeSSE does. The same generator first re-parsed all
seven existing entries through EventSchemas and confirmed each is byte-identical
to what TS emits, so the file as a whole is still TS-producer output.
Appended, never inserted: ToolCallEventsCompatibilityTest indexes the array
positionally (_fixtures[0]..[6]), so inserting anywhere else would silently
shift every existing assertion onto the wrong fixture. There is no hard-coded
count; the one loop-based test enumerates the array.
The absent half gets an assertion too — the pre-error fixture must deserialize
with Error null rather than empty — mirroring the pairing the TypeScript and
Python suites already use.
Verified the asset actually bites, twice. Deleting the Error property (and its
PublicAPI.Unshipped.txt lines, which fail the build first) breaks the build at
ToolCallEventsCompatibilityTest.cs lines 84 and 96 — both new assertions.
Renaming [JsonPropertyName("error")] to "errorDetail", which still compiles,
fails ToolCallResult_WithError_DeserializesFromTypeScriptPayload at runtime with
Expected "SearchTimeout: ..." / Actual null — so the fixture catches wire drift,
not merely the property's existence.
324 tests pass in tests/AGUI.Abstractions.UnitTests on net10.0.
Refs PNI-362.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NAPvqoeqhoEhtxChZHMFoE
The error field was added to the JS and Python event reference only, leaving the language-agnostic protocol table and the .NET event table listing a ToolCallResult that no longer matches the SDKs. Add the field to both missing surfaces, and state the semantics the schema alone does not carry: failure is signalled by presence rather than truthiness (an empty string is preserved and still means failed), content and error are independent, and absence does not yet prove success because no producer populates the field. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NAPvqoeqhoEhtxChZHMFoE
`defaultApplyEvents` receives events that have not necessarily been through `EventSchemas.parse` — `event as ToolCallResultEvent` is an assertion, not validation. The nullish guard `error != null` therefore let a non-string `error` (e.g. `error: 42` from an unvalidated producer) onto the accumulated `ToolMessage`, where it survived into `messages` and only failed `RunAgentInputSchema` a full turn later with "Expected string, received number" at `messages.N.error` — far from the event that caused it. Guard with `typeof error === "string"`, matching the `typeof x === "string"` convention already used elsewhere in this reducer, rather than adding a validation layer. An empty string is still carried: `""` is a value the producer deliberately sent, and dropping it would turn a badly reported failure into a success. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NAPvqoeqhoEhtxChZHMFoE
Four claims in the comments, doc-comments and docs tables were false or
overstated. Wording only; no behaviour changes.
- Not purely additive. BaseEventSchema is `.passthrough()` and the Python
models use extra="allow", so an `error` key of any shape used to survive
parsing as an unrecognized key. Typing it as a string means a stream carrying
`error: {code, message}` — or, in TypeScript, `error: null` — now fails the
parse. The narrowing is intended; the comments now say it is a narrowing.
- "An explicit null fails the parse" holds in TypeScript only. Python coerces
JSON null to None and .NET's `string? Error` deserializes it to null, as they
do for every optional field on those models (`subagent_run_id` and `role`
included). Scoped the claim rather than restating it universally.
- "Byte-identically / key-for-key" overstated the assertions. `toEqual` and
Python dict equality ignore key order, and `toEqual` also ignores
undefined-valued keys. Weakened the wording to "the same keys and values",
keeping the assertions as they are: key order is a serializer detail, not
part of the wire contract.
- "Absent means the call succeeded" overclaimed. Nothing populates the field
yet, so absence still covers real failures. The .NET XML doc additionally
promised a consumer could render the failure from the live stream, but
EventStreamConverter never reads Error — corrected to say the value is
reachable only off the event itself.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NAPvqoeqhoEhtxChZHMFoE
… is broken The tests added with the field did not test what they claimed. Measured by mutation, not by reading: * Deleting `error: z.string().optional()` from ToolCallResultEventSchema left 8 of the 9 new core cases green. BaseEventSchema is `.passthrough()`, so the value survives `.parse()` as an unrecognized key. Of the file's three compile-time annotations only one — `const read: string | undefined` — fired under `tsc`, and the comment credited all three. * The shared fixture case `tool_call_result_with_error` passed with the field deleted in BOTH TypeScript (passthrough) and Python (`extra="allow"`). Only .NET was a genuine harness, because System.Text.Json drops unknown keys. * The client assertion used `(toolMessage as any).error` and `Object.keys(toolMessage as object)`; both discard the ToolMessage narrowing, so removing `error` from ToolMessage in @ag-ui/core still typechecked and left the suite green. `as any` is also banned by the repo test policy. * `defaultApplyEvents` holds the only falsy-sensitive guard (`error != null`) and nothing pinned it: mutating it to `error &&` kept all four suites green while silently dropping a reported failure. Core, Python and .NET each pin the empty-string case; the client did not. * .NET `Deserialize_RoundTripsError` round-tripped through its own serializer, so it passed under a renamed wire key. `Serialize_OmitsError_WhenNull` never referenced `Error` at all, so its assertion was inevitable. What now carries each claim: * core: a runtime assertion on `ToolCallResultEventSchema.shape.error` — the TypeScript analogue of Python's `model_fields` check, and the only runtime fact a passthrough schema cannot fake — plus an annotated `string | undefined` local in every case, so each one fails `tsc` when the field is deleted. Deleting it now yields 12 typecheck errors and 2 failing cases, against 1 before. The stale comment is corrected, and it records that the exported `ToolCallResultEventProps` alias cannot carry a guard at all: `EventProps` is `Omit<z.input<Schema>, "type">`, and `Omit` over a passthrough input type collapses to a bare index signature. * fixtures: both the TypeScript and Python harnesses now assert that every key a case expects on the wire is a DECLARED field of the resolved variant — Zod shape and `model_fields` respectively — so a case can no longer pass on a field the SDK does not have. This covers all 36 cases, not just the new one. * client: type-predicate narrowing to ToolMessage instead of `as any`, annotated reads, a shared helper for the three cases, and a new empty-string case that fails under the `error &&` mutation. * dotnet: `Deserialize_RoundTripsError` asserts the "error" key on the serialized JSON before round-tripping, and `Serialize_OmitsError_WhenNull` pairs the omission with a populated sibling. Both now fail under a renamed wire key, where both passed before. No production source changed; the runtime behaviour is untouched. Honest limitation: a passthrough schema genuinely cannot reject an unknown key at runtime, so most individual core parse-and-read assertions still cannot fail on their own. The schema-shape assertion and the per-case compile-time annotations carry them, and the comment now says exactly that instead of implying the runtime asserts do the work. Refs PNI-362. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NAPvqoeqhoEhtxChZHMFoE
The note claimed the preceding case pins the absent half of the contract. That case tests round-trip fidelity for an event carrying no error at all, not null omission, so the claim overstated it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NAPvqoeqhoEhtxChZHMFoE
The eight places that stated this field's contract did not agree with each other, and the rule they stated could not be implemented in two of the three SDKs. Failure is now defined on the VALUE, not on presence: a call is reported as failed when `error` is a non-null string, empty string included. Presence is not a testable predicate — Python and .NET deserialize an absent key and an explicit JSON null to the same null, and TypeScript rejects the null outright — so `event.error !== undefined` was unimplementable advice that also disagreed with the shipped client, which guards on `typeof error === "string"`. Each SDK page now gives its own language's idiom; the protocol page states it neutrally. Also documents, on the user-facing pages, that typing this key NARROWS the wire: an `error` of another shape used to be ignored (carried through as an unrecognized key in TypeScript and Python, dropped in .NET) and is now a hard parse failure that ends the run, so a producer already sending a structured `error` must change. Corrects "every SDK preserves it" — the community Go, Java, Kotlin and Dart SDKs have no such field, and Go's strict decoder rejects the key outright. Drops the "no producer populates it yet" census from all seven places it had spread to (three code comments, one XML doc that ships to NuGet IntelliSense, three doc pages) in favour of one non-rotting statement on the protocol page. Resolves the contradiction between the .NET docs page and the XML doc, and stops the XML doc describing AGUI.Client behaviour from inside AGUI.Abstractions: the type doc now states only what is true of the type, and the consumer guidance lives on the docs page. Fixes the fixture note's claim that TypeScript runs a reflection-driven sweep over every optional field — it does not; its null-omission coverage comes from `JSON.stringify` dropping `undefined`. No runtime behaviour changes and no test files touched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NAPvqoeqhoEhtxChZHMFoE
…merge artifacts Adds the two tests the branch was missing: the reducer must warn when it drops a non-string `error`, and the streamed ToolMessage must agree with a later MESSAGES_SNAPSHOT about whether the call failed — the invariant the branch exists for, previously asserted only in a comment. Clears three merge artifacts: the verbatim duplicate of the empty-string test that re-inlined the setup `toolMessageFromResult` exists to remove; the comment claiming the reducer "guards on `error != null`" (replaced in 8df405598) and that this is "the only case in any SDK" (the Python SDK pins the same spelling); and the two `Object.keys(toolMessage as object)` casts that threw away the narrowing this file calls load-bearing. The type-predicate-and-throw narrowing is now a shared `expectToolMessage` helper used everywhere. Fixes the helper JSDoc, which described a `result` wire field that does not exist. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NAPvqoeqhoEhtxChZHMFoE
…ons able to fail Every item below was reproduced as a mutation first, fixed, then re-mutated to show the assertion now fails. - The fixture harnesses' declared-field guards were top-level only. Deleting the nested ToolMessage.tool_call_id or RunAgentInput.forwarded_props left the whole Python null-omission file green. Python's walk is now recursive, following the validated model tree and stopping at opaque JSON payloads. TypeScript needs no recursion — .passthrough() is used once, on BaseEventSchema, so a nested undeclared key is stripped and already fails the round-trip — so its docstring now says that, and a new test pins the asymmetry instead of asserting it in prose. - The `cases.length > 15` floor could not notice a deletion: removing the tool_call_result_with_error case outright left all three suites green. All three now pin the count exactly. - Python pinned neither behaviour its own source comment advertises: widening the annotation to Optional[Any] left all 181 tests green. Adds tests for a rejected non-string error and for an explicit null validating and re-emitting as omission. - .NET pinned neither either. Adds the observed behaviour for both: an explicit null reads back as absent and is dropped on re-serialization, and a non-string error is refused by the reader with a JsonException naming $.error. - Two assertions in the .NET compatibility test could not fail. Assert.Equal(string.Empty, typed.Content) passed with Content's JSON property renamed, so a control now feeds the same fixture a sentinel; the all-fixtures loop asserted only Type, so it now compares every property the fixture carries. Positional _fixtures[N] indexing is untouched. - Two core test comments each claimed to be "the one" runtime case that fails on deletion. Measured: exactly two fail. Both comments now say so and name each other. - The shared fixture's tool_call_result_with_error case used tc_3, which no TOOL_CALL_START opened. Adds the opening case ahead of it. - The Python module docstring said "Three tests" while indexing three of four classes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NAPvqoeqhoEhtxChZHMFoE
The `typeof error === "string"` guard discarded a non-string `error` in silence, leaving a ToolMessage byte-identical to the one a successful call produces — a reported failure read back as a success with nothing in the logs. Warn on the drop, in the same shape every other malformed-input path in this reducer uses. The comment justifying the guard was also false: it claimed a non-string would "only fail `RunAgentInputSchema` a full turn later", but no production code parses that schema anywhere. The real consequence is that the value reaches every consumer of `agent.messages` unchallenged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NAPvqoeqhoEhtxChZHMFoE
mxmzb
force-pushed
the
feat/tool-call-result-error
branch
from
August 31, 2026 18:39
fca733a to
b10a831
Compare
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
1 participant
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.
The gap
ToolMessagecarries an optionalerror.ToolCallResultEventdoes not.A UI rendering a live stream therefore cannot show a failed tool call as failed. It has to wait for
the
MESSAGES_SNAPSHOTthat carries the finishedToolMessage— which arrives after the run, toolate to be useful for anything the user is watching happen.
Reported by S&P Global, who hit exactly this in their frontend integration.
What this adds
An optional
errorstring onTOOL_CALL_RESULT, in TypeScript, Python and .NET — the event-sidetwin of
ToolMessage.error.error: z.string().optional()onToolCallResultEventSchemaerror: Optional[str] = NoneonToolCallResultEventpublic string? ErroronToolCallResultEvent, recorded inPublicAPI.Unshipped.txtPlus
@ag-ui/clientthreading the value onto theToolMessagethatdefaultApplyEventsaccumulates — without which the message built from the stream and the one in the
MESSAGES_SNAPSHOTdisagree about whether the call failed, which is the confusion the field exists to remove. There is
now a test that streams both and asserts they agree.
The contract
A tool call is reported as failed when
erroris a non-null string. The empty string counts — aproducer that sends
""chose to send it.Test the type, not the truthiness, and not presence:
if (event.error)reads""as a success.explicit JSON null both land as the language's null, so the contract is stated on the value.
Each SDK page gives its own idiom and names the wrong one. A producer with no failure to report
omits the key; writing an explicit JSON null instead is not allowed.
Compatibility — this narrows the wire
Additive for a producer that never writes
error. A tightening for one that does.BaseEventSchemais.passthrough()and the Python models useextra="allow", so anerrorkeyof any other shape previously survived. It is now typed:
EventSchemas.parsethrows and the run ends. Also rejects an explicitnull.ValidationError.JsonExceptionout of the SSE reader. (Previously .NET dropped such a key silentlyrather than carrying it, so its old behaviour differed from the other two.)
A producer already sending a structured
erroron this event must flatten it to a string or move itto another key. This narrowing is the intended reading of the no-null-tolerance rule for fields
added since PNI-199, but it is a tightening, and the schema comments and docs now say so.
The null rule is enforced in TypeScript only. Zod rejects an explicit null; Python coerces it to
Noneand .NET deserializes it tonull. That is pre-existing behaviour of every optional field onthose models —
subagent_run_idandrolebehave identically — and this change neither introducesnor fixes it. The guarantee is that no producer writes null, not that every SDK refuses one.
Coverage is not uniform. The community Go, Java, Kotlin and Dart SDKs have no
errorfield onthis event, and Go's strict decoder (
DisallowUnknownFields) rejects the key outright rather thanignoring it — as it already does for the pre-existing
subagentRunId.Deliberately not in this PR
TOOL_CALL_RESULTis not in the proto schema at all — a documented parityboundary, tracked as PNI-214.
erroryet, so an absenterrorstill means the producer saidnothing about failure. LangGraph, the MCP middleware and the Claude managed-agents integration all
have an error signal today and drop it; wiring them is the follow-up that makes this useful.
EventStreamConvertermaps this event to aFunctionResultContentbuilt from
ToolCallIdandContentonly. On .NET the value is reachable off the event itself(or via
ChatResponseUpdate.RawRepresentation); the XML doc and docs page say exactly that ratherthan promising a live-stream render. Mapping a string onto an
Exception-shaped API is a designdecision, not a mechanical one.
subagentRunId.Review
Two full review rounds, 30 reviewer agents on an unbiased prompt, 9 isolated fix agents. What it
changed, beyond the wording above:
@ag-ui/clientguard narrows on the value's type, not on nullishness — the event reachingthat reducer has not necessarily been parsed, and a serialized exception object is the natural
shape a Python or LangChain producer emits. A non-string is dropped with a warning; silently
discarding it would leave a
ToolMessagebyte-identical to a successful one.originally produced one type error and left the shared cross-language fixture case fully green in
two of three SDKs; it now produces twelve type errors and three runtime failures. The
fixture harnesses assert that every expected wire key is a declared field, and each SDK asserts
an exact case count so a deleted case is noticed.
the TypeScript factory rather than hand-written.
Verification
tsc --noEmitand lint cleanAGUI.Abstractions.UnitTests)Pre-existing and unrelated, verified on a clean
origin/maincheckout:AGUI.Client.UnitTestsandAGUI.Hosting.AspNetCore.IntegrationTestsdo not compile (CS0122;SignAssemblyis always true,which kills the
InternalsVisibleTocondition). Filed as follow-up.Refs PNI-362.