Sync/upstream main 2026 08 07 - #189
Merged
Merged
Conversation
…main PiperOrigin-RevId: 958226953
…main PiperOrigin-RevId: 958283927
…t dataset ID This change removes the datasetId configuration and updates the default tableName from "events" to "agent_analytics". Change is needed to match python implementation. PiperOrigin-RevId: 958476351
`PartConverter` inferred the GenAI part type from the shape of an inbound `DataPart` payload: any map holding `name` + `args` became a `FunctionCall`, `name` + `response` a `FunctionResponse`, and likewise for executable code and code execution results. Conversion now requires the sender to label the part with the `adk_type` metadata key; unlabelled payloads are carried through as generic inline JSON. That stops a generic peer payload which merely happens to carry those keys from being silently reinterpreted as a control part, and matches the Python, Kotlin and Go converters, which have always keyed off the metadata alone. ADK-to-ADK traffic is unaffected, since the outbound helpers always set `adk_type`. Explicitly labelled parts are still converted - that path is required for relaying human-in-the-loop and long-running tool calls between agents - so this change alone does not prevent a peer from emitting a function call. The execution path behind it is closed by a follow-up change. PiperOrigin-RevId: 959607436
`ResponseConverter.getLongRunningToolIds` dereferenced `DataPart.getMetadata()` without a null check, but the A2A spec treats `metadata` as optional and deserializes an absent or null field to `null`. Any peer returning such a `DataPart` in a task artifact or status message triggered an NPE that aborted the entire response conversion and failed the local agent's turn. Skip the part when metadata is missing, and cover all three call sites with regression tests. Malformed rather than absent peer metadata can still abort conversion through `parseMetadata`. That is a different trigger with a different fix, tracked separately in b/542523886. PiperOrigin-RevId: 959608801
…lback A DataPart carrying no `adk_type` metadata is no longer inferred to be a function call. PartConverter serializes it into an inline `text/plain` JSON blob instead, so two tests that still asserted the old inferred behaviour failed at head with `NoSuchElementException` on an empty `functionCall()`. Update both to assert what the converter actually produces: no function call, and an inline JSON blob that still carries the original payload. A small `inlineJson` helper holds the blob-shape assertions and explains why the fallback happens. PiperOrigin-RevId: 959800614
`AgentExecutor#failedMessage` built the failure message sent back to the peer out of `Throwable#getMessage`. Exception text routinely contains absolute paths, class and module locations, configuration values and echoed request payloads, so a peer could map the host by provoking errors. It now logs the throwable under a short opaque id and sends only a fixed summary plus that id. It also took over the logging that used to happen at the call site, so the id in the response is the id in the log. Set `ADK_DEBUG_ERRORS=1` to get the detail back when debugging locally. PiperOrigin-RevId: 959895666
…writing into the caller's Content Merge google#1378 **Please ensure you have read the [contribution guide](./CONTRIBUTING.md) before creating a pull request.** ### Link to Issue or Description of Change **1. Link to an existing issue (if applicable):** - Closes: [google#1377](google#1377) **2. Or, if no issue exists, describe the change:** **Problem:** With `saveInputBlobsAsArtifacts(true)`, `Runner.appendNewMessageToSession` replaces each inline blob with a placeholder by writing into the parts list of the `Content` the caller passed to `runAsync` ([`Runner.java:376-382`](https://github.com/google/adk-java/blob/main/core/src/main/java/com/google/adk/runner/Runner.java#L376)). That list is never copied on the way in, so for an immutable one the `set` throws `UnsupportedOperationException` — before the model is called, and before anything is appended: | Construction | Backing list | Result | |---|---|---| | `builder().parts(List.of(text, blob))` | caller's `List.of`, uncopied | **throws** | | `builder().parts(textBuilder, blobBuilder)` | genai's own `ImmutableList` | **throws** | | `Content.fromParts(text, blob)` | `Arrays.asList` | works | **Solution:** Rewrite a **copy** of the parts list and build a new `Content` for the event. The loop moves into a helper returning both the prepared message and the saves it scheduled: ```java private record OffloadedMessage(Content message, Completable artifactSaves) { ... } private OffloadedMessage offloadInputBlobs(Session session, Content message, String invocationId) { // The runner directly saves the artifacts (if applicable) in the user message and replaces // the artifact data with a file name placeholder. List<Part> parts = new ArrayList<>(message.parts().get()); ... return new OffloadedMessage( message.toBuilder().parts(ImmutableList.copyOf(parts)).build(), saveArtifactsFlow); } ``` so `appendNewMessageToSession` becomes one decision: ```java OffloadedMessage offloaded = this.artifactService != null && saveInputBlobsAsArtifacts ? offloadInputBlobs(session, newMessage, invocationContext.invocationId()) : OffloadedMessage.unchanged(newMessage); ``` The functional change is one line — `new ArrayList<>(...)`, which always accepts `set`. Documented behaviour is unchanged: the persisted message still carries the placeholder, the blob is still saved, and the model still never sees it. ### Testing Plan **Unit Tests:** - [x] I have added or updated unit tests for my change. - [x] All unit tests pass locally. ```text Tests run: 94, Failures: 0, Errors: 0, Skipped: 0 # with the fix Tests run: 94, Failures: 5 # reverting only Runner.java ``` Nine tests added. Five fail without the fix (regression tests); four pass in both states (parity tests pinning behaviour that must not change): | Test | Without fix | |---|---| | `..._immutablePartsList_savesArtifactAndCompletes` | **fails** | | `..._partBuilderPartsList_savesArtifactAndCompletes` | **fails** | | `..._appendedEventReplacesBlobWithPlaceholder` | **fails** | | `..._doesNotModifyCallerMessage` | **fails** | | `..._storesBlobVerbatim` | **fails** | | `..._fromPartsConstruction_savesArtifactAndCompletes` | passes | | `..._disabled_keepsBlobAndSavesNothing` | passes | | `..._textOnlyMessage_passesThroughUnchanged` | passes | | `..._disabledWithTextOnlyMessage_passesThroughUnchanged` | passes | The last two cover ordinary traffic: a text-only prompt with the option on (the runner still walks the parts and, after the fix, copies them), and the default path with the option off (the rewrite is skipped entirely and the caller's message passes through uncopied). **Manual End-to-End (E2E) Tests:** A demo drives one real agent turn per row through `InMemoryRunner` (no custom wiring) against `gemini-2.5-flash`, varying only the construction and the flag, and reads back both the artifact store and the session afterwards. _Before:_ ```text Run A (immutable List.of) threw UnsupportedOperationException model calls 0 nothing appended Run B (Part.Builder varargs) threw UnsupportedOperationException model calls 0 nothing appended Run C (Content.fromParts) completed, blob offloaded, placeholder appended Run D (immutable, flag off) completed, blob reaches the model, no artifact BUG REPRODUCED ``` _After (same demo):_ ```text Run A (immutable List.of) completed model calls 1 payload back verbatim placeholder appended Run B (Part.Builder varargs) completed model calls 1 payload back verbatim placeholder appended Run C (Content.fromParts) unchanged Run D (immutable, flag off) unchanged Run E (text only, flag on) completed, nothing stored, text through untouched Run F (text only, flag off) completed, nothing stored, text through untouched FIXED ``` On every row where the offload ran, the payload came back out of storage verbatim, the model was never shown the blob, and the model **was** shown the placeholder — so this is not merely "stopped crashing". ### Checklist - [x] I have read the [CONTRIBUTING.md](./CONTRIBUTING.md) document. - [x] My pull request contains a single commit. - [x] I have performed a self-review of my own code. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have added tests that prove my fix is effective or that my feature works. - [x] New and existing unit tests pass locally with my changes. - [x] I have manually tested my changes end-to-end. - [ ] Any dependent changes have been merged and published in downstream modules. COPYBARA_INTEGRATE_REVIEW=google#1378 from svetanis:fix/runner-content-mutation d5f28e6 PiperOrigin-RevId: 959996970
PiperOrigin-RevId: 960131629
`ResponseConverter.parseMetadata` rethrew every deserialization failure as `IllegalArgumentException`, which propagated out of `taskToEvent`, `messageToEvent` and `handleTaskUpdate`. Because `adk_grounding_metadata`, `adk_usage_metadata`, `adk_custom_metadata` and `adk_error_code` are all peer-controlled, a single unparseable value from a remote agent could break the caller's turn. Log at WARN and drop the offending field instead, so auxiliary telemetry cannot take down the whole conversion. On the Java side these keys are read-only: nothing in ADK Java writes them, so a parse failure is always peer data, never our own serialization. This matches the drop semantics in ADK Kotlin (`LegacyA2aConverters.kt` logs WARN and returns null) and ADK Python (`to_adk_event._extract_genai_metadata` logs and returns None); Java was the outlier. The warning here omits the parser message, which quotes the peer's bytes, so it follows Python rather than Kotlin, which still attaches the exception. PiperOrigin-RevId: 960211499
PiperOrigin-RevId: 960227883
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.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
2. Or, if no issue exists, describe the change:
If applicable, please follow the issue templates to provide as much detail as
possible.
Problem:
A clear and concise description of what the problem is.
Solution:
A clear and concise description of what you want to happen and why you choose
this solution.
Testing Plan
Please describe the tests that you ran to verify your changes. This is required
for all PRs that are not small documentation or typo fixes.
Unit Tests:
Please include a summary of passed java test results.
Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any
necessary setup or configuration. Please provide logs or screenshots to help
reviewers better understand the fix.
Checklist
Additional context
Add any other context or screenshots about the feature request here.