Repository navigation
fix(adk): flag the function response of a failed tool call with isError - #2937
Open
fiunchinho wants to merge 3 commits into
Open
fiunchinho wants to merge 3 commits into
fiunchinho wants to merge 3 commits into
Conversation
adk-go's MCP toolset turns a CallToolResult that carries isError into a
Go error, and the flow renders every tool error as {"error": text}. The
harness runtimes (harness/runtime/a2a) set isError beside a failed
result, so a client that shows a step as failed on that flag sees a
failed Go ADK tool as a successful step, with the error prose as its
output.
The on-tool-error callback now returns the same {"error": text} with
"isError": true. The model reads the text it read before; the A2A
function_response part carries the flag, as it does for the other
runtimes. A confirmation request is a pause, not a failure, and keeps
the plain shape; a rejected call did not run and is flagged.
Signed-off-by: Jose Armesto <github@armesto.net>
fiunchinho
marked this pull request as ready for review
September 24, 2026 21:47
This was referenced Sep 25, 2026
Signed-off-by: Jose Armesto <github@armesto.net>
github-actions
Bot
added
bug
|
This pull request has been marked as stale because of no activity in the last 15 days. It will be closed in the next 5 days unless it is tagged "no stalebot" or other activity occurs. |
moezdil
reviewed
Oct 16, 2026
| if errors.Is(err, tool.ErrConfirmationRequired) { | ||
| return nil, nil | ||
| } | ||
| return map[string]any{"error": err.Error(), "isError": true}, nil |
Contributor
There was a problem hiding this comment.
our ui already flags error responses, which client?
Author
There was a problem hiding this comment.
yeah, the kagent UI already handles this, this PR changes nothing there. What it does is give the Go ADK the same flag as the other two runtimes, so a client needs one check instead of two shapes (an error key for the Go ADK, isError for the rest). The error key alone is also a weak signal, since a tool that returns a map with an error key as a normal result renders as failed.
moezdil
reviewed
Oct 16, 2026
| return &toolErrorContext{StrictContextMock: adkagent.NewStrictContextMock(context.Background())} | ||
| } | ||
|
|
||
| func TestOnToolErrorCallback_FlagsAFailedCall(t *testing.T) { |
Contributor
There was a problem hiding this comment.
tests only call the callback, never the real flow
Author
There was a problem hiding this comment.
agreed, I added better tests now, PTAL
The tests called the on-tool-error callback and read its return value. They did not show that adk-go puts that value into the function response, which is what a client reads. They now run an agent through adk-go's runner with the tool callbacks createGoogleADKAgent wires, and read the function responses the flow emits: a failed call is flagged, a confirmation request keeps the plain shape, and the rejection of that request does not run the tool and is flagged. Signed-off-by: Jose Armesto <github@armesto.net>
github-actions
Bot
added
bug
moezdil
reviewed
Oct 19, 2026
| if errors.Is(err, tool.ErrConfirmationRequired) { | ||
| return nil, nil | ||
| } | ||
| return map[string]any{"error": err.Error(), "isError": true}, nil |
Contributor
There was a problem hiding this comment.
mcp app tools lose this error text in compaction
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
2 participants
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.
Problem
The runtimes do not agree on how a
function_responsesays that a tool call failed. The harness runtimes setisErrorbeside a failed result (go/harness/runtime/a2a/executor.go,toolResultPart), and the Python ADK passes the MCPisErrorthrough. The Go ADK sends only{"error": "<text>"}: adk-go'smcptoolsetturns an MCP result withisErrorinto a Go error, and the flow renders every tool error as{"error": text}(internal/llminternal/base_flow.go,callTool).So an A2A client needs two checks to tell a failed call from a successful one:
isErrorfor the harness and Python runtimes, theerrorkey for the Go ADK. The kagent UI does both (ToolCallCard.tsx), so it is not affected. A client that reads onlyisErrorshows a failed Go ADK call as a successful step. Seen with such a client: a Prometheus query the server rejected showed a green check, while the model's text quoted the parse error.Change
makeOnToolErrorCallbackreturns{"error": text, "isError": true}instead of leaving the response to adk-go's default{"error": text}. The model reads the same text as before; thefunction_responsepart carries the same flag on every runtime.tool.ErrConfirmationRequired) travels as an error too, but it is a pause, not a failure: it keeps the plain shape, which clients tell apart by its text.tool.ErrConfirmationRejected) did not run and is flagged.tool execution completedwithresult_keys=[error isError]. The error-leveltool execution failedline from the on-error callback is unchanged.Tests
cd go && go test -race ./adk/pkg/agent/...: the tests run an agent through adk-go's runner with the tool callbackscreateGoogleADKAgentwires, and read the function responses the flow emits. A failed call is flagged; a confirmation request keeps the plain shape and is followed by the confirmation request; the rejection of that request does not run the tool and is flagged.go test -race ./adk/...,gofmtandgo vetare clean.