diff --git a/.release-please-manifest.json b/.release-please-manifest.json index 7588679c0..f2c43d597 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -1,3 +1,3 @@ { - ".": "1.7.0" + ".": "1.7.1" } diff --git a/AGENTS.md b/AGENTS.md deleted file mode 100644 index 5d33d2172..000000000 --- a/AGENTS.md +++ /dev/null @@ -1,3 +0,0 @@ -# AGENTS.md - -Validate changes by running `./mvnw test`. diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f717049c..a7cace1a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,18 @@ # Changelog +## [1.7.1](https://github.com/google/adk-java/compare/v1.7.0...v1.7.1) (2026-07-28) + + +### Bug Fixes + +* **codeexecutors:** add opt-in strict sandbox to ContainerCodeExecutor ([8049f7e](https://github.com/google/adk-java/commit/8049f7e5362ca654bf3706ea465f8d1021ee0346)) +* **core:** fallback to name when Agent description is missing ([233b83b](https://github.com/google/adk-java/commit/233b83bcacc39f7b6a1204a7644a1a5f13557a20)) +* **events:** accumulate endOfAgent in EventActions.merge to preserve parallel stop requests ([03b04fa](https://github.com/google/adk-java/commit/03b04fa2b17b8b9fc508add4d1013e83a4975cfe)) +* **mcp:** honor stdioServerParams in McpToolset.fromConfig ([cf71d7b](https://github.com/google/adk-java/commit/cf71d7bb07398d6fabd3a3ade24f31db98f9f36e)) +* preserve all parallel function calls on the live (BIDI) connection ([edc330d](https://github.com/google/adk-java/commit/edc330d760d8194058610907e717f13425717d8b)) +* **sessions:** apply afterTimestamp and numRecentEvents together in VertexAiSessionService ([24a4588](https://github.com/google/adk-java/commit/24a4588004228d6117d9ab4a45ce93be4c952d3c)) +* **sessions:** apply numRecentEvents and afterTimestamp together in InMemorySessionService ([4d19f7d](https://github.com/google/adk-java/commit/4d19f7d92becff955de12e2a58bc6bb23f14492d)) + ## [1.7.0](https://github.com/google/adk-java/compare/v1.6.0...v1.7.0) (2026-07-17) diff --git a/README.md b/README.md index 2a0a88415..4e96ca89e 100644 --- a/README.md +++ b/README.md @@ -50,13 +50,13 @@ If you're using Maven, add the following to your dependencies: com.google.adk google-adk - 1.7.0 + 1.7.1 com.google.adk google-adk-dev - 1.7.0 + 1.7.1 ``` diff --git a/a2a/README.md b/a2a/README.md index 82f1a13ae..139fac1e3 100644 --- a/a2a/README.md +++ b/a2a/README.md @@ -18,6 +18,7 @@ projects that demonstrate how to expose that runtime over HTTP. library and exposes the JSON-RPC endpoint. ### High‑Level Picture + ```mermaid graph LR classDef client fill:#E8F0FE,stroke:#1A73E8,color:#202124; @@ -114,45 +115,52 @@ transport-agnostic `a2a/src/...` tree described above. All commands below assume you are in `google_adk`. 1. **Start the Spring webservice sample** (run in its own terminal) - ```bash - lsof -ti :8081 | xargs -r kill - ./mvnw -f contrib/samples/a2a_remote/pom.xml spring-boot:run \ + + ```bash + lsof -ti :8081 | xargs -r kill + ./mvnw -f contrib/samples/a2a_remote/pom.xml spring-boot:run \ -Dspring-boot.run.arguments=--server.port=8081 - ``` + ``` - Background option: - ```bash - nohup env GOOGLE_GENAI_USE_VERTEXAI=FALSE \ + Background option: + + ```bash + nohup env GOOGLE_GENAI_USE_VERTEXAI=FALSE \ GOOGLE_API_KEY=your_api_key \ ./mvnw -f contrib/samples/a2a_remote/pom.xml spring-boot:run \ -Dspring-boot.run.arguments=--server.port=8081 \ > /tmp/a2a_webservice.log 2>&1 & echo $! - ``` - The log can be found at /tmp/a2a_webservice.log. + ``` + + The log can be found at /tmp/a2a_webservice.log. 2. **Run the basic client sample (`a2a_basic`)** (from another terminal) - ```bash - GOOGLE_GENAI_USE_VERTEXAI=FALSE \ - GOOGLE_API_KEY=your_api_key \ - ./mvnw -f contrib/samples/a2a_basic/pom.xml exec:java \ + + ```bash + GOOGLE_GENAI_USE_VERTEXAI=FALSE \ + GOOGLE_API_KEY=your_api_key \ + ./mvnw -f contrib/samples/a2a_basic/pom.xml exec:java \ -Dexec.args="http://localhost:8081/a2a/remote" - ``` + ``` + + The client logs the outbound JSON-RPC payload and shows the remote agent’s + reply (for example, `4 is not a prime number.`). - The client logs the outbound JSON-RPC payload and shows the remote agent’s - reply (for example, `4 is not a prime number.`). + > The first run downloads dependencies from Maven Central. Configure a + > mirror in `~/.m2/settings.xml` if your environment restricts outbound + > traffic. - > The first run downloads dependencies from Maven Central. Configure a - > mirror in `~/.m2/settings.xml` if your environment restricts outbound traffic. + Background option: - Background option: - ```bash - nohup env GOOGLE_GENAI_USE_VERTEXAI=FALSE \ + ```bash + nohup env GOOGLE_GENAI_USE_VERTEXAI=FALSE \ GOOGLE_API_KEY=your_api_key \ ./mvnw -f contrib/samples/a2a_basic/pom.xml exec:java \ -Dexec.args="http://localhost:8081/a2a/remote" \ > /tmp/a2a_basic.log 2>&1 & echo $! - ``` - Tail `/tmp/a2a_basic.log` to observe subsequent turns. + ``` + + Tail `/tmp/a2a_basic.log` to observe subsequent turns. To build the runtime, Spring webservice, and both samples together, activate the opt-in Maven profile: @@ -204,7 +212,7 @@ Sample response: "args": { "nums": [6] }, "name": "checkPrime" }, - "metadata": { "type": "function_call" }, + "metadata": { "adk_type": "function_call" }, "kind": "data" }, { @@ -212,7 +220,7 @@ Sample response: "response": { "result": "No prime numbers found." }, "name": "checkPrime" }, - "metadata": { "type": "function_response" }, + "metadata": { "adk_type": "function_response" }, "kind": "data" }, { diff --git a/a2a/pom.xml b/a2a/pom.xml index 494c17c15..e14e2f7dc 100644 --- a/a2a/pom.xml +++ b/a2a/pom.xml @@ -5,7 +5,7 @@ com.google.adk google-adk-parent - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT google-adk-a2a diff --git a/a2a/src/main/java/com/google/adk/a2a/converters/PartConverter.java b/a2a/src/main/java/com/google/adk/a2a/converters/PartConverter.java index 94cf51524..a905081b0 100644 --- a/a2a/src/main/java/com/google/adk/a2a/converters/PartConverter.java +++ b/a2a/src/main/java/com/google/adk/a2a/converters/PartConverter.java @@ -154,8 +154,7 @@ private static com.google.genai.types.Part convertDataPartToGenAiPart(DataPart d String metadataType = metadata.getOrDefault(A2AMetadataKey.TYPE.getType(), "").toString(); - if ((data.containsKey(NAME_KEY) && data.containsKey(ARGS_KEY)) - || metadataType.equals(A2ADataPartMetadataType.FUNCTION_CALL.getType())) { + if (metadataType.equals(A2ADataPartMetadataType.FUNCTION_CALL.getType())) { String functionName = String.valueOf(data.getOrDefault(NAME_KEY, "")); String functionId = String.valueOf(data.getOrDefault(ID_KEY, "")); Map args = coerceToMap(data.get(ARGS_KEY)); @@ -169,8 +168,7 @@ private static com.google.genai.types.Part convertDataPartToGenAiPart(DataPart d return builder.build(); } - if ((data.containsKey(NAME_KEY) && data.containsKey(RESPONSE_KEY)) - || metadataType.equals(A2ADataPartMetadataType.FUNCTION_RESPONSE.getType())) { + if (metadataType.equals(A2ADataPartMetadataType.FUNCTION_RESPONSE.getType())) { String functionName = String.valueOf(data.getOrDefault(NAME_KEY, "")); String functionId = String.valueOf(data.getOrDefault(ID_KEY, "")); Map response = coerceToMap(data.get(RESPONSE_KEY)); @@ -188,8 +186,7 @@ private static com.google.genai.types.Part convertDataPartToGenAiPart(DataPart d return builder.build(); } - if ((data.containsKey(CODE_KEY) && data.containsKey(LANGUAGE_KEY)) - || metadataType.equals(A2ADataPartMetadataType.EXECUTABLE_CODE.getType())) { + if (metadataType.equals(A2ADataPartMetadataType.EXECUTABLE_CODE.getType())) { String code = String.valueOf(data.getOrDefault(CODE_KEY, "")); String language = String.valueOf( @@ -204,8 +201,7 @@ private static com.google.genai.types.Part convertDataPartToGenAiPart(DataPart d return builder.build(); } - if ((data.containsKey(OUTCOME_KEY) && data.containsKey(OUTPUT_KEY)) - || metadataType.equals(A2ADataPartMetadataType.CODE_EXECUTION_RESULT.getType())) { + if (metadataType.equals(A2ADataPartMetadataType.CODE_EXECUTION_RESULT.getType())) { String outcome = String.valueOf(data.getOrDefault(OUTCOME_KEY, Outcome.Known.OUTCOME_OK).toString()); String output = String.valueOf(data.getOrDefault(OUTPUT_KEY, "")); @@ -222,6 +218,8 @@ private static com.google.genai.types.Part convertDataPartToGenAiPart(DataPart d return builder.build(); } + logIfUnlabelledControlPayload(data, metadataType); + try { String json = objectMapper.writeValueAsString(dataPart); String wrappedJson = A2A_DATA_PART_START_TAG + json + A2A_DATA_PART_END_TAG; @@ -239,6 +237,37 @@ private static com.google.genai.types.Part convertDataPartToGenAiPart(DataPart d } } + /** + * Warns when a DataPart carries a payload shaped like a control part but no {@code adk_type} + * label, so it is about to be carried through as generic data. + * + *

Conversion used to be inferred from this shape. A sender still relying on that - typically a + * non-ADK peer - now silently gets an inline JSON blob instead of a function call or response, so + * name the cause rather than leaving someone to bisect the converter. + */ + private static void logIfUnlabelledControlPayload(Map data, String metadataType) { + if (!metadataType.isEmpty() || !logger.isWarnEnabled()) { + return; + } + String inferredType = null; + if (data.containsKey(NAME_KEY) && data.containsKey(ARGS_KEY)) { + inferredType = A2ADataPartMetadataType.FUNCTION_CALL.getType(); + } else if (data.containsKey(NAME_KEY) && data.containsKey(RESPONSE_KEY)) { + inferredType = A2ADataPartMetadataType.FUNCTION_RESPONSE.getType(); + } else if (data.containsKey(CODE_KEY) && data.containsKey(LANGUAGE_KEY)) { + inferredType = A2ADataPartMetadataType.EXECUTABLE_CODE.getType(); + } else if (data.containsKey(OUTCOME_KEY) && data.containsKey(OUTPUT_KEY)) { + inferredType = A2ADataPartMetadataType.CODE_EXECUTION_RESULT.getType(); + } + if (inferredType != null) { + logger.warn( + "A2A DataPart looks like a '{}' but carries no '{}' metadata; treating it as generic" + + " data. Senders must label control parts explicitly.", + inferredType, + A2AMetadataKey.TYPE.getType()); + } + } + /** * Converts an A2A Message to a Google GenAI Content object. * diff --git a/a2a/src/main/java/com/google/adk/a2a/converters/ResponseConverter.java b/a2a/src/main/java/com/google/adk/a2a/converters/ResponseConverter.java index b5733e9a9..412763fa2 100644 --- a/a2a/src/main/java/com/google/adk/a2a/converters/ResponseConverter.java +++ b/a2a/src/main/java/com/google/adk/a2a/converters/ResponseConverter.java @@ -20,7 +20,7 @@ import static com.google.common.collect.Streams.zip; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.JavaType; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.adk.agents.InvocationContext; import com.google.adk.events.Event; @@ -61,6 +61,8 @@ public final class ResponseConverter { private static final ObjectMapper objectMapper = new ObjectMapper(); private static final Logger logger = LoggerFactory.getLogger(ResponseConverter.class); + private static final JavaType CUSTOM_METADATA_LIST_TYPE = + objectMapper.getTypeFactory().constructCollectionType(List.class, CustomMetadata.class); private static final ImmutableSet PENDING_STATES = ImmutableSet.of(TaskState.WORKING, TaskState.SUBMITTED); @@ -71,6 +73,8 @@ private ResponseConverter() {} * empty optional if the event should be ignored (e.g. if the event is not a final update for * TaskArtifactUpdateEvent or if the message is empty for TaskStatusUpdateEvent). * + *

Unparseable ADK metadata is logged and dropped; the rest of the event is still converted. + * * @throws IllegalArgumentException if the event type is not supported. */ public static Optional clientEventToEvent( @@ -93,6 +97,11 @@ private static boolean isPartial(@Nullable Map metadata) { return Objects.equals(metadata.getOrDefault(A2AMetadataKey.PARTIAL.getType(), false), true); } + private static boolean isLongRunning(@Nullable Map metadata) { + return metadata != null + && Objects.equals(metadata.get(A2AMetadataKey.IS_LONG_RUNNING.getType()), true); + } + /** * Converts a A2A {@link TaskUpdateEvent} to an ADK {@link Event}, if applicable. Returns null if * the event is not a final update for TaskArtifactUpdateEvent or if the message is empty for @@ -184,7 +193,11 @@ public static Event messageToFailedEvent(Message message, InvocationContext invo return builder.build(); } - /** Converts an A2A message back to ADK events. */ + /** + * Converts an A2A message back to ADK events. + * + *

Unparseable ADK metadata is logged and dropped; the rest of the event is still converted. + */ public static Event messageToEvent(Message message, InvocationContext invocationContext) { return updateEventMetadata( remoteAgentEventBuilder(invocationContext) @@ -214,6 +227,8 @@ public static Event messageToEvent( * Converts an A2A {@link Task} to an ADK {@link Event}. If the artifacts are present, the last * artifact is used. If not, the status message is used. If not, the last history message is used. * If none of these are present, an empty event is returned. + * + *

Unparseable ADK metadata is logged and dropped; the rest of the event is still converted. */ public static Event taskToEvent(Task task, InvocationContext invocationContext) { ImmutableList.Builder genaiParts = ImmutableList.builder(); @@ -268,9 +283,8 @@ private static ImmutableSet getLongRunningToolIds( if (!(part instanceof DataPart dataPart)) { return Optional.empty(); } - Object isLongRunning = - dataPart.getMetadata().get(A2AMetadataKey.IS_LONG_RUNNING.getType()); - if (!Objects.equals(isLongRunning, true)) { + // A2A peers may omit metadata entirely, which deserializes to null. + if (!isLongRunning(dataPart.getMetadata())) { return Optional.empty(); } if (convertedPart.functionCall().isEmpty()) { @@ -296,13 +310,13 @@ private static Event updateEventMetadata( clientMetadata = ImmutableMap.of(); } Event.Builder eventBuilder = event.toBuilder(); - Object groundingMetadata = clientMetadata.get(A2AMetadataKey.GROUNDING_METADATA.getType()); - // if groundingMetadata is null, parseMetadata will return null as well. - eventBuilder.groundingMetadata(parseMetadata(groundingMetadata, GroundingMetadata.class)); - Object usageMetadata = clientMetadata.get(A2AMetadataKey.USAGE_METADATA.getType()); - // if usageMetadata is null, parseMetadata will return null as well. + eventBuilder.groundingMetadata( + parseMetadata(clientMetadata, A2AMetadataKey.GROUNDING_METADATA, GroundingMetadata.class)); eventBuilder.usageMetadata( - parseMetadata(usageMetadata, GenerateContentResponseUsageMetadata.class)); + parseMetadata( + clientMetadata, + A2AMetadataKey.USAGE_METADATA, + GenerateContentResponseUsageMetadata.class)); ImmutableList.Builder customMetadataList = ImmutableList.builder(); customMetadataList @@ -316,32 +330,35 @@ private static Event updateEventMetadata( .key(AdkMetadataKey.CONTEXT_ID.getType()) .stringValue(contextId) .build()); - Object customMetadata = clientMetadata.get(A2AMetadataKey.CUSTOM_METADATA.getType()); - if (customMetadata != null) { - customMetadataList.addAll( - parseMetadata(customMetadata, new TypeReference>() {})); + List parsedCustomMetadata = + parseMetadata(clientMetadata, A2AMetadataKey.CUSTOM_METADATA, CUSTOM_METADATA_LIST_TYPE); + if (parsedCustomMetadata != null) { + customMetadataList.addAll(parsedCustomMetadata); } eventBuilder.customMetadata(customMetadataList.build()); - Object errorCode = clientMetadata.get(A2AMetadataKey.ERROR_CODE.getType()); - eventBuilder.errorCode(parseMetadata(errorCode, FinishReason.class)); + eventBuilder.errorCode( + parseMetadata(clientMetadata, A2AMetadataKey.ERROR_CODE, FinishReason.class)); return eventBuilder.build(); } - private static @Nullable T parseMetadata(@Nullable Object metadata, Class type) { - try { - if (metadata instanceof String jsonString) { - return objectMapper.readValue(jsonString, type); - } else { - return objectMapper.convertValue(metadata, type); - } - } catch (IllegalArgumentException | JsonProcessingException e) { - throw new IllegalArgumentException("Failed to parse metadata of type " + type, e); - } + /** + * Reads {@code key} out of the peer-supplied {@code clientMetadata} and deserializes it. + * + *

Returns null when the key is absent, and also when its value cannot be parsed: metadata is + * peer-controlled, so a malformed value is logged and dropped rather than failing the whole + * conversion. + */ + private static @Nullable T parseMetadata( + Map clientMetadata, A2AMetadataKey key, Class type) { + return parseMetadata(clientMetadata, key, objectMapper.getTypeFactory().constructType(type)); } - private static @Nullable T parseMetadata(@Nullable Object metadata, TypeReference type) { + /** Overload of {@link #parseMetadata(Map, A2AMetadataKey, Class)} for generic target types. */ + private static @Nullable T parseMetadata( + Map clientMetadata, A2AMetadataKey key, JavaType type) { + Object metadata = clientMetadata.get(key.getType()); try { if (metadata instanceof String jsonString) { return objectMapper.readValue(jsonString, type); @@ -349,10 +366,27 @@ private static Event updateEventMetadata( return objectMapper.convertValue(metadata, type); } } catch (IllegalArgumentException | JsonProcessingException e) { - throw new IllegalArgumentException("Failed to parse metadata of type " + type.getType(), e); + logDroppedMetadata(key, e); + return null; } } + /** + * Reports a dropped metadata value. + * + *

The parser's message quotes the peer's bytes, so the warning carries only the key and the + * exception type. A peer that streams malformed metadata would otherwise be able to write + * arbitrary content and a stack trace into the log on every event. The full exception is + * available at debug level. + */ + private static void logDroppedMetadata(A2AMetadataKey key, Exception e) { + logger.warn( + "Dropping unparseable A2A metadata for key {} ({})", + key.getType(), + e.getClass().getSimpleName()); + logger.debug("Unparseable A2A metadata for key {}", key.getType(), e); + } + private static Event emptyEvent(InvocationContext invocationContext) { Event.Builder builder = Event.builder() diff --git a/a2a/src/main/java/com/google/adk/a2a/executor/AgentExecutor.java b/a2a/src/main/java/com/google/adk/a2a/executor/AgentExecutor.java index 57a0d9db2..618888c4c 100644 --- a/a2a/src/main/java/com/google/adk/a2a/executor/AgentExecutor.java +++ b/a2a/src/main/java/com/google/adk/a2a/executor/AgentExecutor.java @@ -29,6 +29,7 @@ import com.google.adk.runner.Runner; import com.google.adk.sessions.BaseSessionService; import com.google.adk.sessions.Session; +import com.google.common.base.Ascii; import com.google.common.collect.ImmutableList; import com.google.errorprone.annotations.CanIgnoreReturnValue; import com.google.genai.types.Content; @@ -66,6 +67,18 @@ public class AgentExecutor implements io.a2a.server.agentexecution.AgentExecutor private static final Logger logger = LoggerFactory.getLogger(AgentExecutor.class); private static final String USER_ID_PREFIX = "A2A_USER_"; private static final String A2A_METADATA_KEY = "a2a_metadata"; + + /** + * Env var that puts exception text back into the failure message sent to the peer. + * + *

Off by default, and meant for local debugging only: enabling it on a network-reachable + * deployment restores the disclosure {@link #failedMessage} exists to prevent. + */ + private static final String DEBUG_ERRORS_ENV_VAR = "ADK_DEBUG_ERRORS"; + + /** Length of the correlation id, matching {@code new_error_id()} in adk-python. */ + private static final int ERROR_ID_LENGTH = 12; + private final Map activeTasks = new ConcurrentHashMap<>(); private final Runner.Builder runnerBuilder; private final AgentExecutorConfig agentExecutorConfig; @@ -232,13 +245,7 @@ public void execute(RequestContext ctx, EventQueue eventQueue) { .ignoreElements() .materialize() .flatMapCompletable( - notification -> { - Throwable error = notification.getError(); - if (error != null) { - logger.error("Runner failed to execute", error); - } - return handleExecutionEnd(ctx, error, eventQueue); - }) + notification -> handleExecutionEnd(ctx, notification.getError(), eventQueue)) .doFinally(() -> cleanupTask(ctx.getTaskId())) .subscribe( () -> {}, @@ -250,7 +257,16 @@ public void execute(RequestContext ctx, EventQueue eventQueue) { private Completable handleExecutionEnd( RequestContext ctx, Throwable error, EventQueue eventQueue) { TaskState state = error != null ? TaskState.FAILED : TaskState.COMPLETED; - Message message = error != null ? failedMessage(ctx, error) : null; + Message message = null; + if (error != null) { + // The peer is not trusted with the throwable: exception text routinely names absolute + // filesystem paths, class and module locations, configuration values and echoed request + // payloads, none of which the caller needs and all of which are useful reconnaissance. It is + // logged here in full under a short opaque id; the peer gets only that id. + String errorId = newErrorId(); + logger.error("Runner failed to execute [error_id={}]", errorId, error); + message = failedMessage(ctx, error, errorId); + } TaskStatusUpdateEvent initialEvent = new TaskStatusUpdateEvent.Builder() .taskId(ctx.getTaskId()) @@ -304,16 +320,66 @@ private Maybe prepareSession( })); } - private static Message failedMessage(RequestContext context, Throwable e) { + /** + * Builds the failure message handed back to the remote peer. + * + *

It carries {@code errorId} rather than {@code e.getMessage()}, so an operator handed the id + * can find the real stack trace in the log while the peer learns nothing about the host. Set + * {@code ADK_DEBUG_ERRORS=1} to put the exception text back into the response while debugging + * locally. + */ + private static Message failedMessage(RequestContext context, Throwable e, String errorId) { return new Message.Builder() .messageId(UUID.randomUUID().toString()) .contextId(context.getContextId()) .taskId(context.getTaskId()) .role(Message.Role.AGENT) - .parts(ImmutableList.of(new TextPart(e.getMessage()))) + .parts(ImmutableList.of(new TextPart(failureText(e, errorId, debugErrorsEnabled())))) .build(); } + /** + * Returns a short opaque id tying the peer's failure message to the logged throwable. + * + *

{@value #ERROR_ID_LENGTH} hex characters, the same shape as {@code new_error_id()} in + * adk-python, so an operator sees the same kind of id whichever runtime produced it. + */ + private static String newErrorId() { + return UUID.randomUUID().toString().replace("-", "").substring(0, ERROR_ID_LENGTH); + } + + /** + * Returns the failure text that is safe to hand to the remote peer. + * + * @param includeDetail whether to append the exception type and message; see {@link + * #DEBUG_ERRORS_ENV_VAR}. + */ + static String failureText(Throwable e, String errorId, boolean includeDetail) { + String text = "Agent execution failed. (error_id: " + errorId + ")"; + if (includeDetail) { + text = text + ": " + e.getClass().getName() + ": " + e.getMessage(); + } + return text; + } + + private static boolean debugErrorsEnabled() { + return debugErrorsEnabled(System.getenv(DEBUG_ERRORS_ENV_VAR)); + } + + /** + * Returns whether {@code value}, as read from {@link #DEBUG_ERRORS_ENV_VAR}, turns the detail + * back on. Unset or unrecognized means off, matching {@code is_env_enabled} in adk-python. + * + *

Split from the env lookup so both outcomes are testable: {@code System.getenv} cannot be set + * from a test in-process. + */ + static boolean debugErrorsEnabled(String value) { + if (value == null) { + return false; + } + return value.equals("1") || Ascii.equalsIgnoreCase(value, "true"); + } + // Processor that will process all events related to the one runner invocation. private static class EventProcessor { private final String runArtifactId; diff --git a/a2a/src/test/java/com/google/adk/a2a/converters/PartConverterTest.java b/a2a/src/test/java/com/google/adk/a2a/converters/PartConverterTest.java index 0242cdedf..03622c287 100644 --- a/a2a/src/test/java/com/google/adk/a2a/converters/PartConverterTest.java +++ b/a2a/src/test/java/com/google/adk/a2a/converters/PartConverterTest.java @@ -159,18 +159,28 @@ public void toGenaiPart_withDataPartFunctionCall_returnsGenaiFunctionCallPart() } @Test - public void toGenaiPart_withDataPartFunctionCallByNameAndArgs_returnsGenaiFunctionCallPart() { + public void toGenaiPart_withUnlabelledFunctionCallShapedDataPart_doesNotBuildFunctionCall() { ImmutableMap data = - ImmutableMap.of("name", "func", "id", "1", "args", ImmutableMap.of("param", "value")); + ImmutableMap.of("name", "local_tool", "id", "1", "args", ImmutableMap.of("param", "value")); DataPart dataPart = new DataPart(data, null); Part result = PartConverter.toGenaiPart(dataPart); - assertThat(result.functionCall()).isPresent(); - FunctionCall functionCall = result.functionCall().get(); - assertThat(functionCall.name()).hasValue("func"); - assertThat(functionCall.id()).hasValue("1"); - assertThat(functionCall.args()).hasValue(ImmutableMap.of("param", "value")); + assertThat(result.functionCall()).isEmpty(); + assertThat(result.inlineData()).isPresent(); + } + + @Test + public void toGenaiPart_withUnrelatedMetadataTypeAndFunctionCallShape_doesNotBuildFunctionCall() { + ImmutableMap data = + ImmutableMap.of("name", "local_tool", "id", "1", "args", ImmutableMap.of("param", "value")); + DataPart dataPart = + new DataPart(data, ImmutableMap.of(A2AMetadataKey.TYPE.getType(), "something_else")); + + Part result = PartConverter.toGenaiPart(dataPart); + + assertThat(result.functionCall()).isEmpty(); + assertThat(result.inlineData()).isPresent(); } @Test @@ -194,19 +204,95 @@ public void toGenaiPart_withDataPartFunctionResponse_returnsGenaiFunctionRespons } @Test - public void - toGenaiPart_withDataPartFunctionResponseByNameAndResponse_returnsGenaiFunctionResponsePart() { + public void toGenaiPart_withUnlabelledFunctionResponseShapedDataPart_doesNotBuildResponse() { ImmutableMap data = ImmutableMap.of("name", "func", "id", "1", "response", ImmutableMap.of("result", "value")); DataPart dataPart = new DataPart(data, null); Part result = PartConverter.toGenaiPart(dataPart); + assertThat(result.functionResponse()).isEmpty(); + assertThat(result.inlineData()).isPresent(); + } + + // The four positive cases below deliberately use the literal wire strings rather than the enum + // constants. Inbound conversion and the outbound createDataPartFrom* helpers read the same enum, + // so an enum-based assertion moves in lockstep with the converter and could never fail. These + // literals are the contract shared with the Python, Kotlin and Go converters, which is what a + // typo would actually break. + @Test + public void toGenaiPart_withLabelledExecutableCode_returnsGenaiExecutableCodePart() { + DataPart dataPart = + new DataPart( + ImmutableMap.of("code", "print(1)", "language", "PYTHON"), + ImmutableMap.of("adk_type", "executable_code")); + + Part result = PartConverter.toGenaiPart(dataPart); + + assertThat(result.executableCode()).isPresent(); + assertThat(result.executableCode().get().code()).hasValue("print(1)"); + } + + @Test + public void toGenaiPart_withLabelledCodeExecutionResult_returnsGenaiCodeExecutionResultPart() { + DataPart dataPart = + new DataPart( + ImmutableMap.of("outcome", "OUTCOME_OK", "output", "done"), + ImmutableMap.of("adk_type", "code_execution_result")); + + Part result = PartConverter.toGenaiPart(dataPart); + + assertThat(result.codeExecutionResult()).isPresent(); + assertThat(result.codeExecutionResult().get().output()).hasValue("done"); + } + + @Test + public void toGenaiPart_withLabelledFunctionCall_returnsGenaiFunctionCallPart() { + DataPart dataPart = + new DataPart( + ImmutableMap.of("name", "func", "id", "1", "args", ImmutableMap.of("param", "value")), + ImmutableMap.of("adk_type", "function_call")); + + Part result = PartConverter.toGenaiPart(dataPart); + + assertThat(result.functionCall()).isPresent(); + assertThat(result.functionCall().get().name()).hasValue("func"); + } + + @Test + public void toGenaiPart_withLabelledFunctionResponse_returnsGenaiFunctionResponsePart() { + DataPart dataPart = + new DataPart( + ImmutableMap.of( + "name", "func", "id", "1", "response", ImmutableMap.of("result", "value")), + ImmutableMap.of("adk_type", "function_response")); + + Part result = PartConverter.toGenaiPart(dataPart); + assertThat(result.functionResponse()).isPresent(); - FunctionResponse functionResponse = result.functionResponse().get(); - assertThat(functionResponse.name()).hasValue("func"); - assertThat(functionResponse.id()).hasValue("1"); - assertThat(functionResponse.response()).hasValue(ImmutableMap.of("result", "value")); + assertThat(result.functionResponse().get().name()).hasValue("func"); + } + + @Test + public void toGenaiPart_withUnlabelledExecutableCodeShapedDataPart_doesNotBuildExecutableCode() { + ImmutableMap data = ImmutableMap.of("code", "print(1)", "language", "PYTHON"); + DataPart dataPart = new DataPart(data, null); + + Part result = PartConverter.toGenaiPart(dataPart); + + assertThat(result.executableCode()).isEmpty(); + assertThat(result.inlineData()).isPresent(); + } + + @Test + public void toGenaiPart_withUnlabelledCodeResultShapedDataPart_doesNotBuildCodeResult() { + ImmutableMap data = ImmutableMap.of("outcome", "OUTCOME_OK", "output", "done"); + DataPart dataPart = new DataPart(data, null); + + Part result = PartConverter.toGenaiPart(dataPart); + + assertThat(result.codeExecutionResult()).isEmpty(); + assertThat(result.inlineData()).isPresent(); } @Test @@ -378,7 +464,7 @@ public void fromGenaiPart_withFunctionResponsePart_returnsDataPart() { @Test public void toGenaiPart_dataPartWithEmptyStringCoercedToEmptyMap() { ImmutableMap data = ImmutableMap.of("name", "func", "id", "1", "args", ""); - DataPart dataPart = new DataPart(data, null); + DataPart dataPart = new DataPart(data, functionCallMetadata()); Part result = PartConverter.toGenaiPart(dataPart); @@ -389,7 +475,7 @@ public void toGenaiPart_dataPartWithEmptyStringCoercedToEmptyMap() { @Test public void toGenaiPart_dataPartWithNonMapCoercedToMap() { ImmutableMap data = ImmutableMap.of("name", "func", "id", "1", "args", 123); - DataPart dataPart = new DataPart(data, null); + DataPart dataPart = new DataPart(data, functionCallMetadata()); Part result = PartConverter.toGenaiPart(dataPart); @@ -463,4 +549,9 @@ public void fromGenaiPart_withDataPartInlineDataAndMetadata_returnsDataPartWithM assertThat(dataPart.getMetadata()) .containsExactly("metaKey", "metaValue", "partMetaKey", "partMetaValue"); } + + private static ImmutableMap functionCallMetadata() { + return ImmutableMap.of( + A2AMetadataKey.TYPE.getType(), A2ADataPartMetadataType.FUNCTION_CALL.getType()); + } } diff --git a/a2a/src/test/java/com/google/adk/a2a/converters/ResponseConverterTest.java b/a2a/src/test/java/com/google/adk/a2a/converters/ResponseConverterTest.java index 20f2d3c10..9b854b616 100644 --- a/a2a/src/test/java/com/google/adk/a2a/converters/ResponseConverterTest.java +++ b/a2a/src/test/java/com/google/adk/a2a/converters/ResponseConverterTest.java @@ -17,8 +17,8 @@ package com.google.adk.a2a.converters; import static com.google.common.truth.Truth.assertThat; +import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.stream.Collectors.joining; -import static org.junit.Assert.assertThrows; import com.google.adk.agents.BaseAgent; import com.google.adk.agents.InvocationContext; @@ -47,6 +47,7 @@ import io.a2a.spec.TaskStatusUpdateEvent; import io.a2a.spec.TextPart; import io.reactivex.rxjava3.core.Flowable; +import java.util.List; import java.util.Optional; import org.junit.Before; import org.junit.Test; @@ -212,6 +213,92 @@ public void taskToEvent_withCustomMetadata_returnsEvent() { .inOrder(); } + @Test + public void taskToEvent_withMalformedMetadata_dropsFieldsAndConverts() { + Message statusMessage = + new Message.Builder() + .role(Message.Role.AGENT) + .parts(ImmutableList.of(new TextPart("Status message"))) + .build(); + TaskStatus status = new TaskStatus(TaskState.WORKING, statusMessage, null); + Task task = + testTask() + .status(status) + .artifacts(null) + .metadata( + ImmutableMap.of( + A2AMetadataKey.GROUNDING_METADATA.getType(), "not-valid-json", + A2AMetadataKey.USAGE_METADATA.getType(), "not-valid-json", + A2AMetadataKey.CUSTOM_METADATA.getType(), "not-valid-json", + A2AMetadataKey.ERROR_CODE.getType(), "not-valid-json")) + .build(); + + Event event = ResponseConverter.taskToEvent(task, invocationContext); + + assertThat(event.content().get().parts().get().get(0).text()).hasValue("Status message"); + assertThat(event.groundingMetadata()).isEmpty(); + assertThat(event.usageMetadata()).isEmpty(); + assertThat(event.errorCode()).isEmpty(); + assertThat(event.customMetadata().get()) + .containsExactly( + CustomMetadata.builder().key("a2a:task_id").stringValue("task-1").build(), + CustomMetadata.builder().key("a2a:context_id").stringValue("context-1").build()); + } + + @Test + public void taskToEvent_withUnrecognizedMetadataField_dropsField() { + Message statusMessage = + new Message.Builder() + .role(Message.Role.AGENT) + .parts(ImmutableList.of(new TextPart("Status message"))) + .build(); + TaskStatus status = new TaskStatus(TaskState.WORKING, statusMessage, null); + Task task = + testTask() + .status(status) + .artifacts(null) + .metadata( + ImmutableMap.of( + // A nested object takes the convertValue branch rather than readValue. The + // genai builders reject unknown fields, so snake_case fails to convert. + A2AMetadataKey.GROUNDING_METADATA.getType(), + ImmutableMap.of("web_search_queries", ImmutableList.of("test-query")))) + .build(); + + Event event = ResponseConverter.taskToEvent(task, invocationContext); + + assertThat(event.groundingMetadata()).isEmpty(); + assertThat(event.content().get().parts().get().get(0).text()).hasValue("Status message"); + } + + @Test + public void taskToEvent_withOneMalformedMetadataField_keepsTheValidFields() { + GroundingMetadata groundingMetadata = + GroundingMetadata.builder().webSearchQueries("test-query").build(); + Message statusMessage = + new Message.Builder() + .role(Message.Role.AGENT) + .parts(ImmutableList.of(new TextPart("Status message"))) + .build(); + TaskStatus status = new TaskStatus(TaskState.WORKING, statusMessage, null); + Task task = + testTask() + .status(status) + .artifacts(null) + .metadata( + ImmutableMap.of( + A2AMetadataKey.GROUNDING_METADATA.getType(), + groundingMetadata.toJson(), + A2AMetadataKey.USAGE_METADATA.getType(), + "not-valid-json")) + .build(); + + Event event = ResponseConverter.taskToEvent(task, invocationContext); + + assertThat(event.groundingMetadata()).hasValue(groundingMetadata); + assertThat(event.usageMetadata()).isEmpty(); + } + @Test public void messageToEvent_withMissingTaskId_returnsEvent() { Message a2aMessage = @@ -272,6 +359,95 @@ public void taskToEvent_withInputRequired_parsesLongRunningToolIds() { assertThat(event.longRunningToolIds().get()).containsExactly("call_123", "msg_123"); } + @Test + public void taskToEvent_withDataPartWithoutMetadata_fallsBackToInlineJson() { + DataPart dataPart = + new DataPart( + ImmutableMap.of("name", "myTool", "id", "call_123", "args", ImmutableMap.of())); + DataPart statusDataPart = + new DataPart( + ImmutableMap.of("name", "messageTool", "id", "msg_123", "args", ImmutableMap.of())); + Message statusMessage = + new Message.Builder() + .role(Message.Role.AGENT) + .parts(ImmutableList.of(statusDataPart)) + .build(); + TaskStatus status = new TaskStatus(TaskState.INPUT_REQUIRED, statusMessage, null); + Artifact artifact = + new Artifact.Builder().artifactId("artifact-1").parts(ImmutableList.of(dataPart)).build(); + Task task = testTask().status(status).artifacts(ImmutableList.of(artifact)).build(); + + Event event = ResponseConverter.taskToEvent(task, invocationContext); + + assertThat(event.longRunningToolIds().get()).isEmpty(); + List parts = event.content().get().parts().get(); + assertThat(parts).hasSize(2); + assertThat(parts.get(0).functionCall()).isEmpty(); + assertThat(inlineJson(parts.get(0))).contains("call_123"); + assertThat(parts.get(1).functionCall()).isEmpty(); + assertThat(inlineJson(parts.get(1))).contains("msg_123"); + } + + @Test + public void artifactToEvent_withDataPartWithoutMetadata_fallsBackToInlineJson() { + DataPart dataPart = + new DataPart( + ImmutableMap.of("name", "myTool", "id", "call_123", "args", ImmutableMap.of())); + Artifact artifact = + new Artifact.Builder().artifactId("artifact-1").parts(ImmutableList.of(dataPart)).build(); + + Event event = ResponseConverter.artifactToEvent(artifact, invocationContext); + + assertThat(event.longRunningToolIds().get()).isEmpty(); + List parts = event.content().get().parts().get(); + assertThat(parts).hasSize(1); + assertThat(parts.get(0).functionCall()).isEmpty(); + assertThat(inlineJson(parts.get(0))).contains("call_123"); + } + + /** + * {@return the wrapped JSON payload of a part that {@link PartConverter} carried through as + * generic data} + * + *

A DataPart with no {@code adk_type} metadata is not converted into a function call, even + * when its data is shaped like one; it is serialized into an inline JSON blob instead. + */ + private static String inlineJson(com.google.genai.types.Part part) { + assertThat(part.inlineData()).isPresent(); + assertThat(part.inlineData().get().mimeType()).hasValue("text/plain"); + return new String(part.inlineData().get().data().get(), UTF_8); + } + + @Test + public void taskToEvent_withMixedMetadataParts_keepsLongRunningId() { + DataPart noMetadataPart = + new DataPart( + ImmutableMap.of("name", "plainTool", "id", "call_plain", "args", ImmutableMap.of())); + DataPart longRunningPart = + new DataPart( + ImmutableMap.of("name", "lrTool", "id", "call_lr", "args", ImmutableMap.of()), + ImmutableMap.of( + A2AMetadataKey.TYPE.getType(), + "function_call", + A2AMetadataKey.IS_LONG_RUNNING.getType(), + true)); + Artifact artifact = + new Artifact.Builder() + .artifactId("artifact-1") + .parts(ImmutableList.of(noMetadataPart, longRunningPart)) + .build(); + Task task = + testTask() + .status(new TaskStatus(TaskState.INPUT_REQUIRED, null, null)) + .artifacts(ImmutableList.of(artifact)) + .build(); + + Event event = ResponseConverter.taskToEvent(task, invocationContext); + + assertThat(event.longRunningToolIds().get()).containsExactly("call_lr"); + assertThat(event.content().get().parts().get()).hasSize(2); + } + @Test public void taskToEvent_withFailedState_setsErrorCode() { Message statusMessage = @@ -452,7 +628,7 @@ public void clientEventToEvent_withFailedTaskStatusUpdateEvent_returnsErrorEvent } @Test - public void taskToEvent_withInvalidMetadata_throwsException() { + public void taskToEvent_withInvalidMetadata_dropsFieldInsteadOfThrowing() { Message statusMessage = new Message.Builder() .role(Message.Role.AGENT) @@ -467,12 +643,10 @@ public void taskToEvent_withInvalidMetadata_throwsException() { ImmutableMap.of(A2AMetadataKey.GROUNDING_METADATA.getType(), "{ invalid json ]")) .build(); - IllegalArgumentException exception = - assertThrows( - IllegalArgumentException.class, - () -> ResponseConverter.taskToEvent(task, invocationContext)); - assertThat(exception).hasMessageThat().contains("Failed to parse metadata"); - assertThat(exception).hasMessageThat().contains("GroundingMetadata"); + Event event = ResponseConverter.taskToEvent(task, invocationContext); + + assertThat(event.groundingMetadata()).isEmpty(); + assertThat(event.content().get().parts().get().get(0).text()).hasValue("Status message"); } @Test diff --git a/a2a/src/test/java/com/google/adk/a2a/executor/AgentExecutorTest.java b/a2a/src/test/java/com/google/adk/a2a/executor/AgentExecutorTest.java index 99b12286e..68cb196f1 100644 --- a/a2a/src/test/java/com/google/adk/a2a/executor/AgentExecutorTest.java +++ b/a2a/src/test/java/com/google/adk/a2a/executor/AgentExecutorTest.java @@ -63,6 +63,10 @@ @RunWith(JUnit4.class) public final class AgentExecutorTest { + /** A throwable message shaped like the ones that leak host detail. */ + private static final String SECRET_ERROR = + "Runner error: /home/victim/.config/adk/credentials.json (No such file)"; + private EventQueue eventQueue; private List enqueuedEvents; private TestAgent testAgent; @@ -137,7 +141,7 @@ public void createAgentExecutor_noAgentExecutorConfig_throwsException() { public void execute_withBeforeExecuteCallback_cancelsExecutionOnError() { // If callback returns error, execution should stop/fail. Callbacks.BeforeExecuteCallback callback = - ctx -> Single.error(new RuntimeException("Cancelled")); + ctx -> Single.error(new RuntimeException(SECRET_ERROR)); AgentExecutorConfig config = AgentExecutorConfig.builder().beforeExecuteCallback(callback).build(); @@ -162,7 +166,10 @@ public void execute_withBeforeExecuteCallback_cancelsExecutionOnError() { assertThat(statusEvent.getStatus().state().toString()).isEqualTo("FAILED"); assertThat(statusEvent.getStatus().message().getParts().get(0)).isInstanceOf(TextPart.class); TextPart textPart = (TextPart) statusEvent.getStatus().message().getParts().get(0); - assertThat(textPart.getText()).contains("Cancelled"); + // The remote peer gets a correlation id for the logged throwable, not its + // message -- see AgentExecutor#failedMessage. + assertThat(textPart.getText()).startsWith("Agent execution failed. (error_id: "); + assertThat(textPart.getText()).doesNotContain(SECRET_ERROR); } @Test @@ -289,7 +296,7 @@ public void execute_withAfterExecuteCallback_modifiesStatus() { @Test public void execute_runnerFails_registersFailedEvent() { - testAgent.setEventsToEmit(Flowable.error(new RuntimeException("Runner error"))); + testAgent.setEventsToEmit(Flowable.error(new RuntimeException(SECRET_ERROR))); AgentExecutor executor = new AgentExecutor.Builder() .agentExecutorConfig(AgentExecutorConfig.builder().build()) @@ -316,7 +323,48 @@ public void execute_runnerFails_registersFailedEvent() { assertThat(statusEvent.getStatus().state()).isEqualTo(TaskState.FAILED); assertThat(statusEvent.getStatus().message().getParts().get(0)).isInstanceOf(TextPart.class); TextPart textPart = (TextPart) statusEvent.getStatus().message().getParts().get(0); - assertThat(textPart.getText()).isEqualTo("Runner error"); + // A runner failure is reported to the peer as a correlation id only: the + // throwable's message names host paths and is for the server log. The id is + // 12 hex characters, the shape adk-python emits. + assertThat(textPart.getText()) + .matches("Agent execution failed\\. \\(error_id: [0-9a-f]{12}\\)"); + assertThat(textPart.getText()).doesNotContain(SECRET_ERROR); + assertThat(textPart.getText()).doesNotContain("/home/victim"); + } + + @Test + public void failureText_withoutDebug_carriesOnlyTheCorrelationId() { + String text = AgentExecutor.failureText(new RuntimeException(SECRET_ERROR), "abc-123", false); + + assertThat(text).isEqualTo("Agent execution failed. (error_id: abc-123)"); + } + + @Test + public void failureText_withDebug_carriesTheThrowableDetail() { + // ADK_DEBUG_ERRORS=1 is the documented opt-in for local debugging. + String text = AgentExecutor.failureText(new RuntimeException(SECRET_ERROR), "abc-123", true); + + assertThat(text).startsWith("Agent execution failed. (error_id: abc-123): "); + assertThat(text).contains("java.lang.RuntimeException"); + assertThat(text).contains(SECRET_ERROR); + } + + @Test + public void debugErrorsEnabled_recognizesTheDocumentedValues() { + assertThat(AgentExecutor.debugErrorsEnabled("1")).isTrue(); + assertThat(AgentExecutor.debugErrorsEnabled("true")).isTrue(); + assertThat(AgentExecutor.debugErrorsEnabled("TRUE")).isTrue(); + assertThat(AgentExecutor.debugErrorsEnabled("True")).isTrue(); + } + + @Test + public void debugErrorsEnabled_defaultsToOff() { + // Anything else leaves the redaction in place, including an unset variable. + assertThat(AgentExecutor.debugErrorsEnabled(null)).isFalse(); + assertThat(AgentExecutor.debugErrorsEnabled("")).isFalse(); + assertThat(AgentExecutor.debugErrorsEnabled("0")).isFalse(); + assertThat(AgentExecutor.debugErrorsEnabled("false")).isFalse(); + assertThat(AgentExecutor.debugErrorsEnabled("yes")).isFalse(); } @Test diff --git a/contrib/firestore-session-service/pom.xml b/contrib/firestore-session-service/pom.xml index 6f5442fd5..2f6f2e8e3 100644 --- a/contrib/firestore-session-service/pom.xml +++ b/contrib/firestore-session-service/pom.xml @@ -20,7 +20,7 @@ com.google.adk google-adk-parent - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT ../../pom.xml diff --git a/contrib/langchain4j/pom.xml b/contrib/langchain4j/pom.xml index 7ae9c7ad3..a609bacec 100644 --- a/contrib/langchain4j/pom.xml +++ b/contrib/langchain4j/pom.xml @@ -20,7 +20,7 @@ com.google.adk google-adk-parent - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT ../../pom.xml diff --git a/contrib/planners/pom.xml b/contrib/planners/pom.xml index 45fc8194e..55df8381a 100644 --- a/contrib/planners/pom.xml +++ b/contrib/planners/pom.xml @@ -20,7 +20,7 @@ com.google.adk google-adk-parent - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT ../../pom.xml diff --git a/contrib/samples/a2a_basic/pom.xml b/contrib/samples/a2a_basic/pom.xml index 855acf77e..36ea15bf7 100644 --- a/contrib/samples/a2a_basic/pom.xml +++ b/contrib/samples/a2a_basic/pom.xml @@ -5,7 +5,7 @@ com.google.adk google-adk-samples - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT .. diff --git a/contrib/samples/a2a_server/pom.xml b/contrib/samples/a2a_server/pom.xml index e98ae957b..f1b762a5b 100644 --- a/contrib/samples/a2a_server/pom.xml +++ b/contrib/samples/a2a_server/pom.xml @@ -5,7 +5,7 @@ com.google.adk google-adk-samples - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT .. diff --git a/contrib/samples/configagent/pom.xml b/contrib/samples/configagent/pom.xml index eae0a9542..a2674b357 100644 --- a/contrib/samples/configagent/pom.xml +++ b/contrib/samples/configagent/pom.xml @@ -5,7 +5,7 @@ com.google.adk google-adk-samples - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT .. diff --git a/contrib/samples/github/adkprtriaging/pom.xml b/contrib/samples/github/adkprtriaging/pom.xml index 8bafd9e33..d58918d96 100644 --- a/contrib/samples/github/adkprtriaging/pom.xml +++ b/contrib/samples/github/adkprtriaging/pom.xml @@ -20,7 +20,7 @@ com.google.adk google-adk-samples - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT ../.. diff --git a/contrib/samples/github/adkreleasedocs/pom.xml b/contrib/samples/github/adkreleasedocs/pom.xml index 5904a597c..fc9a39f2e 100644 --- a/contrib/samples/github/adkreleasedocs/pom.xml +++ b/contrib/samples/github/adkreleasedocs/pom.xml @@ -20,7 +20,7 @@ com.google.adk google-adk-samples - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT ../.. diff --git a/contrib/samples/github/adkspam/pom.xml b/contrib/samples/github/adkspam/pom.xml index 9e073518a..e78be4b54 100644 --- a/contrib/samples/github/adkspam/pom.xml +++ b/contrib/samples/github/adkspam/pom.xml @@ -20,7 +20,7 @@ com.google.adk google-adk-samples - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT ../.. diff --git a/contrib/samples/github/adkstale/pom.xml b/contrib/samples/github/adkstale/pom.xml index 970603818..154a54b9d 100644 --- a/contrib/samples/github/adkstale/pom.xml +++ b/contrib/samples/github/adkstale/pom.xml @@ -20,7 +20,7 @@ com.google.adk google-adk-samples - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT ../.. diff --git a/contrib/samples/github/adktriaging/pom.xml b/contrib/samples/github/adktriaging/pom.xml index 605548f99..550b2a04a 100644 --- a/contrib/samples/github/adktriaging/pom.xml +++ b/contrib/samples/github/adktriaging/pom.xml @@ -20,7 +20,7 @@ com.google.adk google-adk-samples - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT ../.. diff --git a/contrib/samples/github/githubtools/pom.xml b/contrib/samples/github/githubtools/pom.xml index d3704d964..409918e81 100644 --- a/contrib/samples/github/githubtools/pom.xml +++ b/contrib/samples/github/githubtools/pom.xml @@ -20,7 +20,7 @@ com.google.adk google-adk-samples - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT ../.. diff --git a/contrib/samples/helloworld/pom.xml b/contrib/samples/helloworld/pom.xml index 9edcfe9a1..334cb51b6 100644 --- a/contrib/samples/helloworld/pom.xml +++ b/contrib/samples/helloworld/pom.xml @@ -20,7 +20,7 @@ com.google.adk google-adk-samples - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT .. diff --git a/contrib/samples/mcpfilesystem/pom.xml b/contrib/samples/mcpfilesystem/pom.xml index ae20c5fa3..c3f277a1b 100644 --- a/contrib/samples/mcpfilesystem/pom.xml +++ b/contrib/samples/mcpfilesystem/pom.xml @@ -20,7 +20,7 @@ com.google.adk google-adk-parent - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT ../../.. diff --git a/contrib/samples/pom.xml b/contrib/samples/pom.xml index 514ad2b92..a926d3b85 100644 --- a/contrib/samples/pom.xml +++ b/contrib/samples/pom.xml @@ -5,7 +5,7 @@ com.google.adk google-adk-parent - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT ../.. diff --git a/contrib/sarvam-ai/pom.xml b/contrib/sarvam-ai/pom.xml index 5da7fd312..b8ad5fdbf 100644 --- a/contrib/sarvam-ai/pom.xml +++ b/contrib/sarvam-ai/pom.xml @@ -20,7 +20,7 @@ com.google.adk google-adk-parent - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT ../../pom.xml diff --git a/contrib/spring-ai/pom.xml b/contrib/spring-ai/pom.xml index 253a72f20..976806a99 100644 --- a/contrib/spring-ai/pom.xml +++ b/contrib/spring-ai/pom.xml @@ -20,7 +20,7 @@ com.google.adk google-adk-parent - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT ../../pom.xml diff --git a/core/pom.xml b/core/pom.xml index c069739e8..80141addc 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -20,7 +20,7 @@ com.google.adk google-adk-parent - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT google-adk diff --git a/core/src/main/java/com/google/adk/Version.java b/core/src/main/java/com/google/adk/Version.java index a890d6c61..48ceca967 100644 --- a/core/src/main/java/com/google/adk/Version.java +++ b/core/src/main/java/com/google/adk/Version.java @@ -22,7 +22,7 @@ */ public final class Version { // Don't touch this, release-please should keep it up to date. - public static final String JAVA_ADK_VERSION = "1.7.0"; // x-release-please-released-version + public static final String JAVA_ADK_VERSION = "1.7.1"; // x-release-please-released-version private Version() {} } diff --git a/core/src/main/java/com/google/adk/flows/llmflows/RequestConfirmationLlmRequestProcessor.java b/core/src/main/java/com/google/adk/flows/llmflows/RequestConfirmationLlmRequestProcessor.java index a93eb3cb4..6f73a0a8d 100644 --- a/core/src/main/java/com/google/adk/flows/llmflows/RequestConfirmationLlmRequestProcessor.java +++ b/core/src/main/java/com/google/adk/flows/llmflows/RequestConfirmationLlmRequestProcessor.java @@ -43,6 +43,7 @@ import io.reactivex.rxjava3.core.Single; import java.util.Collection; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -75,6 +76,28 @@ public Single processRequest( int finalConfirmationEventIndex = confirmationResult.get().eventIndex(); ImmutableMap requestConfirmationFunctionResponses = confirmationResult.get().responses(); + String agentName = invocationContext.agent().name(); + ImmutableMap functionCallsById = + functionCallsById(events, agentName); + ImmutableSet confirmationRequestedIds = confirmationRequestedIds(events); + // A tool has been confirmed, but it might already have been executed by a subsequent processor + // or in a subsequent turn: such calls have a function response after the user confirmation + // event. This is applied before the resumability check rather than after, because + // findMostRecentConfirmations re-matches the same stale user event on every later LLM call, so + // a settled confirmation would otherwise be re-examined - and re-logged - for the rest of the + // session. + // + // Only responses this agent produced count. A peer event landing after the approval that + // reuses the pending call's ID would otherwise convince this scan the tool had already run, + // silently dropping the approval - and it short-circuits before the resumability check, so + // that would not even leave a log line. + ImmutableSet alreadyResumedIds = + events.subList(finalConfirmationEventIndex + 1, events.size()).stream() + .filter(event -> Objects.equals(event.author(), agentName)) + .flatMap(event -> event.functionResponses().stream()) + .map(FunctionResponse::id) + .flatMap(Optional::stream) + .collect(toImmutableSet()); // Search backwards from the event before confirmation for the corresponding // request_confirmation function calls emitted by the model. @@ -83,6 +106,13 @@ public Single processRequest( if (event.functionCalls().isEmpty()) { continue; } + // Only this agent can ask this agent's user for confirmation. Function call parts also reach + // the session from an A2A peer response - ResponseConverter turns one into a model-role event + // authored by the local RemoteA2AAgent - and honouring a confirmation call from there would + // let the peer choose which local tool runs. + if (!Objects.equals(event.author(), agentName)) { + continue; + } Map toolsToResumeWithConfirmation = new HashMap<>(); Map toolsToResumeWithArgs = new HashMap<>(); @@ -95,6 +125,11 @@ public Single processRequest( .forEach( fc -> getOriginalFunctionCall(fc) + .filter(ofc -> !alreadyResumedIds.contains(ofc.id().get())) + .filter( + ofc -> + isResumableFunctionCall( + ofc, functionCallsById, confirmationRequestedIds, agentName)) .ifPresent( ofc -> { toolsToResumeWithConfirmation.put( @@ -103,23 +138,6 @@ public Single processRequest( toolsToResumeWithArgs.put(ofc.id().get(), ofc); })); - if (toolsToResumeWithConfirmation.isEmpty()) { - continue; - } - - // If a tool has been confirmed, it might have been executed by a subsequent - // processor, or in a subsequent turn. We identify tools that have already been - // executed by checking for function responses with matching IDs in events that - // occurred *after* the user confirmation event. - ImmutableSet alreadyConfirmedIds = - events.subList(finalConfirmationEventIndex + 1, events.size()).stream() - .flatMap(e -> e.functionResponses().stream()) - .map(FunctionResponse::id) - .flatMap(Optional::stream) - .collect(toImmutableSet()); - toolsToResumeWithConfirmation.keySet().removeAll(alreadyConfirmedIds); - toolsToResumeWithArgs.keySet().removeAll(alreadyConfirmedIds); - // If all confirmed tools in this event have already been processed, continue // searching in older events. if (toolsToResumeWithConfirmation.isEmpty()) { @@ -173,6 +191,126 @@ private static Optional findMostRecentConfirmations( return Optional.empty(); } + /** + * Indexes the tool function calls in session history by ID, keeping the most recent one per ID. + * + *

Confirmation calls are excluded: a confirmation resumes a real tool call, never another + * confirmation. + * + *

Collisions resolve last-wins, so a re-issue of an ID by {@code agentName} supersedes an + * earlier one - except that a foreign author may never displace a call {@code agentName} emitted. + * IDs are not globally unique and anyone can put an event in the session, so without that + * precedence a peer could reuse the ID of a call this agent is waiting on, shadow it, and have + * the author check in {@code isResumableFunctionCall} reject the legitimate confirmation + * - turning that check into a way for a peer to veto any pending tool call. + */ + private static ImmutableMap functionCallsById( + ImmutableList events, String agentName) { + Map byId = new LinkedHashMap<>(); + for (Event event : events) { + for (FunctionCall functionCall : event.functionCalls()) { + if (functionCall.id().isEmpty() + || Objects.equals( + functionCall.name().orElse(null), REQUEST_CONFIRMATION_FUNCTION_CALL_NAME)) { + continue; + } + String id = functionCall.id().get(); + AuthoredFunctionCall existing = byId.get(id); + if (existing == null + || Objects.equals(event.author(), agentName) + || !Objects.equals(existing.author(), agentName)) { + byId.put(id, new AuthoredFunctionCall(event.author(), functionCall)); + } + } + } + return ImmutableMap.copyOf(byId); + } + + /** + * Collects the IDs of function calls that a tool actually asked the user to confirm. + * + *

Covers both ways a confirmation is requested: a tool calling {@link + * com.google.adk.tools.ToolContext#requestConfirmation}, and a {@link + * com.google.adk.tools.FunctionTool} created with {@code requireConfirmation}, which routes + * through the same call. Accumulates over all events rather than keeping one event per ID: + * re-executing a confirmed tool emits a second function response with the same ID and no + * requested confirmations, which would otherwise shadow the original request. + */ + private static ImmutableSet confirmationRequestedIds(ImmutableList events) { + ImmutableSet.Builder ids = ImmutableSet.builder(); + for (Event event : events) { + Map requested = event.actions().requestedToolConfirmations(); + if (requested.isEmpty()) { + continue; + } + for (FunctionResponse functionResponse : event.functionResponses()) { + functionResponse.id().filter(requested::containsKey).ifPresent(ids::add); + } + } + return ids.build(); + } + + /** + * Returns whether {@code originalFunctionCall} faithfully reproduces a tool call {@code + * agentName} emitted and was genuinely awaiting confirmation. + * + *

The resumed call is read out of the {@code originalFunctionCall} argument of an {@code + * adk_request_confirmation} call found in session history, and function call parts reach the + * session from places other than the local model - notably an A2A peer response, which {@code + * ResponseConverter} turns into a model-role event. Resuming such a call unchecked would let + * whoever authored that event pick both the tool and its arguments, so only resume a call that + * matches one this agent emitted, by ID, author, name and arguments, and that a tool actually + * asked to have confirmed. + */ + private static boolean isResumableFunctionCall( + FunctionCall originalFunctionCall, + ImmutableMap functionCallsById, + ImmutableSet confirmationRequestedIds, + String agentName) { + String id = originalFunctionCall.id().get(); + AuthoredFunctionCall emitted = functionCallsById.get(id); + if (emitted == null) { + logger.warn( + "Ignoring tool confirmation for function call ID {}: no such function call in the session" + + " history.", + id); + return false; + } + if (!Objects.equals(emitted.author(), agentName)) { + // Another agent emitted the call; leave it for that agent's own processor. + logger.debug( + "Skipping tool confirmation for function call ID {}: emitted by {}, not by {}.", + id, + emitted.author(), + agentName); + return false; + } + if (!Objects.equals(emitted.functionCall().name(), originalFunctionCall.name())) { + logger.warn( + "Ignoring tool confirmation for function call ID {}: tool name does not match the" + + " function call this agent emitted.", + id); + return false; + } + if (!Objects.equals( + emitted.functionCall().args().orElse(ImmutableMap.of()), + originalFunctionCall.args().orElse(ImmutableMap.of()))) { + logger.warn( + "Ignoring tool confirmation for function call ID {}: arguments do not match the function" + + " call this agent emitted.", + id); + return false; + } + if (!confirmationRequestedIds.contains(id)) { + logger.warn( + "Ignoring tool confirmation for function call ID {}: no tool requested confirmation for" + + " it.", + id); + return false; + } + return true; + } + private Optional getOriginalFunctionCall(FunctionCall functionCall) { if (!functionCall.args().orElse(ImmutableMap.of()).containsKey(ORIGINAL_FUNCTION_CALL)) { return Optional.empty(); @@ -252,4 +390,7 @@ private static Optional> maybeCreateToolConf private record ConfirmationResult( ImmutableMap responses, int eventIndex) {} + + /** A tool function call from session history, together with the author of its event. */ + private record AuthoredFunctionCall(String author, FunctionCall functionCall) {} } diff --git a/core/src/main/java/com/google/adk/plugins/agentanalytics/BigQueryLoggerConfig.java b/core/src/main/java/com/google/adk/plugins/agentanalytics/BigQueryLoggerConfig.java index ec05c2616..301ead886 100644 --- a/core/src/main/java/com/google/adk/plugins/agentanalytics/BigQueryLoggerConfig.java +++ b/core/src/main/java/com/google/adk/plugins/agentanalytics/BigQueryLoggerConfig.java @@ -126,8 +126,7 @@ public static Builder builder() { .enabled(true) .maxContentLength(500 * 1024) .location("us") // Default location. - .datasetId("agent_analytics") - .tableName("events") + .tableName("agent_events") .clusteringFields(ImmutableList.of("event_type", "agent", "user_id")) .logMultiModalContent(true) .gcsBucketName("") diff --git a/core/src/main/java/com/google/adk/runner/Runner.java b/core/src/main/java/com/google/adk/runner/Runner.java index e3c30c79b..c9e56c4cb 100644 --- a/core/src/main/java/com/google/adk/runner/Runner.java +++ b/core/src/main/java/com/google/adk/runner/Runner.java @@ -347,6 +347,9 @@ public Completable close() { /** * Appends a new user message to the session history with optional state delta. * + *

{@code newMessage} is never modified; when inline blobs are saved as artifacts, the appended + * event carries a copy in which the blob data is replaced by placeholders. + * * @throws IllegalArgumentException if message has no parts. */ private Single appendNewMessageToSession( @@ -357,12 +360,16 @@ private Single appendNewMessageToSession( @Nullable Map stateDelta) { checkArgument(newMessage.parts().isPresent(), "No parts in the new_message."); + Content messageToAppend = newMessage; Completable saveArtifactsFlow = Completable.complete(); if (this.artifactService != null && saveInputBlobsAsArtifacts) { // The runner directly saves the artifacts (if applicable) in the user message and replaces - // the artifact data with a file name placeholder. - for (int i = 0; i < newMessage.parts().get().size(); i++) { - Part part = newMessage.parts().get().get(i); + // the artifact data with a file name placeholder. The rewrite happens on a copy of the parts + // list: the caller's list may be immutable, and the caller does not expect the message it + // passed to runAsync to be modified. + List parts = new ArrayList<>(newMessage.parts().get()); + for (int i = 0; i < parts.size(); i++) { + Part part = parts.get(i); if (part.inlineData().isEmpty()) { continue; } @@ -373,14 +380,11 @@ private Single appendNewMessageToSession( .saveArtifact(this.appName, session.userId(), session.id(), fileName, part) .ignoreElement()); - newMessage - .parts() - .get() - .set( - i, - Part.fromText( - "Uploaded file: " + fileName + ". It has been saved to the artifacts")); + parts.set( + i, + Part.fromText("Uploaded file: " + fileName + ". It has been saved to the artifacts")); } + messageToAppend = newMessage.toBuilder().parts(ImmutableList.copyOf(parts)).build(); } // Appends only. We do not yield the event because it's not from the model. Event.Builder eventBuilder = @@ -388,7 +392,7 @@ private Single appendNewMessageToSession( .id(Event.generateEventId()) .invocationId(invocationContext.invocationId()) .author("user") - .content(newMessage); + .content(messageToAppend); // Add state delta if provided if (stateDelta != null && !stateDelta.isEmpty()) { diff --git a/core/src/test/java/com/google/adk/flows/llmflows/RequestConfirmationLlmRequestProcessorTest.java b/core/src/test/java/com/google/adk/flows/llmflows/RequestConfirmationLlmRequestProcessorTest.java index 55adeb39e..c8da89026 100644 --- a/core/src/test/java/com/google/adk/flows/llmflows/RequestConfirmationLlmRequestProcessorTest.java +++ b/core/src/test/java/com/google/adk/flows/llmflows/RequestConfirmationLlmRequestProcessorTest.java @@ -25,6 +25,8 @@ import com.google.adk.agents.InvocationContext; import com.google.adk.agents.LlmAgent; import com.google.adk.events.Event; +import com.google.adk.events.EventActions; +import com.google.adk.events.ToolConfirmation; import com.google.adk.models.LlmRequest; import com.google.adk.plugins.PluginManager; import com.google.adk.sessions.InMemorySessionService; @@ -44,6 +46,7 @@ @RunWith(JUnit4.class) public class RequestConfirmationLlmRequestProcessorTest { + private static final String AGENT_NAME = "test agent"; private static final String ECHO_TOOL_NAME = "echo_tool"; private static final String ORIGINAL_FUNCTION_CALL_ID = "original_fc_id"; private static final ImmutableMap ORIGINAL_FUNCTION_CALL_ARGS = @@ -60,15 +63,53 @@ public class RequestConfirmationLlmRequestProcessorTest { "args", Optional.of(ORIGINAL_FUNCTION_CALL_ARGS))); private static final FunctionCall FUNCTION_CALL = - FunctionCall.builder().id(FUNCTION_CALL_ID).name(ECHO_TOOL_NAME).args(ARGS).build(); + FunctionCall.builder() + .id(FUNCTION_CALL_ID) + .name(REQUEST_CONFIRMATION_FUNCTION_CALL_NAME) + .args(ARGS) + .build(); private static final InMemorySessionService sessionService = new InMemorySessionService(); - private static final Event REQUEST_CONFIRMATION_EVENT = + /** The tool call the agent itself emitted, which the confirmation later resumes. */ + private static final Event ORIGINAL_FUNCTION_CALL_EVENT = + functionCallEvent( + AGENT_NAME, + FunctionCall.builder() + .id(ORIGINAL_FUNCTION_CALL_ID) + .name(ECHO_TOOL_NAME) + .args(ORIGINAL_FUNCTION_CALL_ARGS) + .build()); + + /** + * The tool's own response asking for the call to be confirmed. This is what {@link + * com.google.adk.tools.ToolContext#requestConfirmation} produces, and what a {@code + * requireConfirmation} FunctionTool routes through. + */ + private static final Event CONFIRMATION_REQUESTED_EVENT = Event.builder() - .author("model") - .content(Content.fromParts(Part.builder().functionCall(FUNCTION_CALL).build())) + .author(AGENT_NAME) + .content( + Content.fromParts( + Part.builder() + .functionResponse( + FunctionResponse.builder() + .id(ORIGINAL_FUNCTION_CALL_ID) + .name(ECHO_TOOL_NAME) + .response(ImmutableMap.of("error", "requires confirmation")) + .build()) + .build())) + .actions( + EventActions.builder() + .requestedToolConfirmations( + ImmutableMap.of( + ORIGINAL_FUNCTION_CALL_ID, + ToolConfirmation.builder().hint("please confirm").build())) + .build()) .build(); + private static final Event REQUEST_CONFIRMATION_EVENT = + functionCallEvent(AGENT_NAME, FUNCTION_CALL); + private static final Event USER_CONFIRMATION_EVENT = Event.builder() .author("user") @@ -84,16 +125,21 @@ public class RequestConfirmationLlmRequestProcessorTest { .build())) .build(); + /** The full, legitimate lead-up to a user confirmation. */ + private static final ImmutableList CONFIRMED_CALL_EVENTS = + ImmutableList.of( + ORIGINAL_FUNCTION_CALL_EVENT, + CONFIRMATION_REQUESTED_EVENT, + REQUEST_CONFIRMATION_EVENT, + USER_CONFIRMATION_EVENT); + private static final RequestConfirmationLlmRequestProcessor processor = new RequestConfirmationLlmRequestProcessor(); @Test public void runAsync_withConfirmation_callsOriginalFunction() { LlmAgent agent = createAgentWithEchoTool(); - Session session = - Session.builder("session_id") - .events(ImmutableList.of(REQUEST_CONFIRMATION_EVENT, USER_CONFIRMATION_EVENT)) - .build(); + Session session = Session.builder("session_id").events(CONFIRMED_CALL_EVENTS).build(); InvocationContext context = buildInvocationContext(agent, session); @@ -113,9 +159,11 @@ public void runAsync_withConfirmation_callsOriginalFunction() { @Test public void runAsync_withConfirmationAndToolAlreadyCalled_doesNotCallOriginalFunction() { LlmAgent agent = createAgentWithEchoTool(); + // Authored by the agent, matching Functions.java:740 which builds real tool response events + // with invocationContext.agent().name(). Event toolResponseEvent = Event.builder() - .author("model") + .author(AGENT_NAME) .content( Content.fromParts( Part.builder() @@ -130,8 +178,10 @@ public void runAsync_withConfirmationAndToolAlreadyCalled_doesNotCallOriginalFun Session session = Session.builder("session_id") .events( - ImmutableList.of( - REQUEST_CONFIRMATION_EVENT, USER_CONFIRMATION_EVENT, toolResponseEvent)) + ImmutableList.builder() + .addAll(CONFIRMED_CALL_EVENTS) + .add(toolResponseEvent) + .build()) .build(); InvocationContext context = buildInvocationContext(agent, session); @@ -172,6 +222,206 @@ public void runAsync_noUserConfirmationEvent_empty() { .isEmpty(); } + @Test + public void runAsync_peerReusesPendingCallId_stillCallsOriginalFunction() { + // A peer must not be able to veto a pending confirmation by reusing the ID of a call this + // agent is waiting on. The history index resolves collisions last-wins, so without author + // precedence the peer's entry shadows the agent's, the author check rejects the legitimate + // confirmation, and the user's approval silently does nothing. + LlmAgent agent = createAgentWithEchoTool(); + Event peerNoise = + functionCallEvent( + "remote_a2a_agent", + FunctionCall.builder() + .id(ORIGINAL_FUNCTION_CALL_ID) + .name("peer_noise") + .args(ImmutableMap.of("x", "y")) + .build()); + Session session = + Session.builder("session_id") + .events( + ImmutableList.of( + ORIGINAL_FUNCTION_CALL_EVENT, + CONFIRMATION_REQUESTED_EVENT, + REQUEST_CONFIRMATION_EVENT, + peerNoise, + USER_CONFIRMATION_EVENT)) + .build(); + + assertThat(resumedEvents(agent, session)).hasSize(1); + } + + @Test + public void runAsync_peerFakesExecutedResponse_stillCallsOriginalFunction() { + // The already-resumed scan must only count responses this agent produced. Otherwise a peer + // event landing after the approval, carrying a response that reuses the pending call's ID, + // convinces the processor the tool already ran. That short-circuits before the resumability + // check, so the approval is dropped with no diagnostics at all. + LlmAgent agent = createAgentWithEchoTool(); + Event peerResponse = + Event.builder() + .author("remote_a2a_agent") + .content( + Content.fromParts( + Part.builder() + .functionResponse( + FunctionResponse.builder() + .id(ORIGINAL_FUNCTION_CALL_ID) + .name("peer_noise") + .response(ImmutableMap.of("status", "whatever")) + .build()) + .build())) + .build(); + Session session = + Session.builder("session_id") + .events( + ImmutableList.builder() + .addAll(CONFIRMED_CALL_EVENTS) + .add(peerResponse) + .build()) + .build(); + + assertThat(resumedEvents(agent, session)).hasSize(1); + } + + @Test + public void runAsync_originalCallNotInHistory_doesNotCallOriginalFunction() { + LlmAgent agent = createAgentWithEchoTool(); + Session session = + Session.builder("session_id") + .events(ImmutableList.of(REQUEST_CONFIRMATION_EVENT, USER_CONFIRMATION_EVENT)) + .build(); + + assertThat(resumedEvents(agent, session)).isEmpty(); + } + + @Test + public void runAsync_originalCallEmittedByAnotherAgent_doesNotCallOriginalFunction() { + // The original call is in history and matches by name and args, but a different agent emitted + // it. Only the emitting agent's own processor may resume it. + LlmAgent agent = createAgentWithEchoTool(); + Session session = + Session.builder("session_id") + .events( + replacingFirst( + CONFIRMED_CALL_EVENTS, + functionCallEvent( + "remote_a2a_agent", + FunctionCall.builder() + .id(ORIGINAL_FUNCTION_CALL_ID) + .name(ECHO_TOOL_NAME) + .args(ORIGINAL_FUNCTION_CALL_ARGS) + .build()))) + .build(); + + assertThat(resumedEvents(agent, session)).isEmpty(); + } + + @Test + public void runAsync_confirmationCallFromAnotherAuthor_doesNotCallOriginalFunction() { + // Everything is legitimate except the event carrying the adk_request_confirmation call, which + // an A2A peer injected through RemoteA2AAgent. It must not resume a local tool. + LlmAgent agent = createAgentWithEchoTool(); + Session session = + Session.builder("session_id") + .events( + ImmutableList.of( + ORIGINAL_FUNCTION_CALL_EVENT, + CONFIRMATION_REQUESTED_EVENT, + functionCallEvent("remote_a2a_agent", FUNCTION_CALL), + USER_CONFIRMATION_EVENT)) + .build(); + + assertThat(resumedEvents(agent, session)).isEmpty(); + } + + @Test + public void runAsync_toolNeverRequestedConfirmation_doesNotCallOriginalFunction() { + // Replaying a call that ran without ever asking for confirmation must not re-run it. + LlmAgent agent = createAgentWithEchoTool(); + Session session = + Session.builder("session_id") + .events( + ImmutableList.of( + ORIGINAL_FUNCTION_CALL_EVENT, + REQUEST_CONFIRMATION_EVENT, + USER_CONFIRMATION_EVENT)) + .build(); + + assertThat(resumedEvents(agent, session)).isEmpty(); + } + + @Test + public void runAsync_confirmationWithMismatchedToolName_doesNotCallOriginalFunction() { + LlmAgent agent = createAgentWithEchoTool(); + Session session = + Session.builder("session_id") + .events( + replacingFirst( + CONFIRMED_CALL_EVENTS, + functionCallEvent( + AGENT_NAME, + FunctionCall.builder() + .id(ORIGINAL_FUNCTION_CALL_ID) + .name("some_other_tool") + .args(ORIGINAL_FUNCTION_CALL_ARGS) + .build()))) + .build(); + + assertThat(resumedEvents(agent, session)).isEmpty(); + } + + @Test + public void runAsync_confirmationWithMismatchedArgs_doesNotCallOriginalFunction() { + LlmAgent agent = createAgentWithEchoTool(); + Session session = + Session.builder("session_id") + .events( + replacingFirst( + CONFIRMED_CALL_EVENTS, + functionCallEvent( + AGENT_NAME, + FunctionCall.builder() + .id(ORIGINAL_FUNCTION_CALL_ID) + .name(ECHO_TOOL_NAME) + .args(ImmutableMap.of("say", "something else")) + .build()))) + .build(); + + assertThat(resumedEvents(agent, session)).isEmpty(); + } + + @Test + public void testAgentNameMatchesFixtures() { + // The fixtures hard-code the author, so catch a rename in TestUtils rather than silently + // turning every negative test into a false pass. + assertThat(createAgentWithEchoTool().name()).isEqualTo(AGENT_NAME); + } + + private static ImmutableList resumedEvents(LlmAgent agent, Session session) { + return ImmutableList.copyOf( + processor + .processRequest(buildInvocationContext(agent, session), LlmRequest.builder().build()) + .blockingGet() + .events()); + } + + /** Returns {@code events} with its first element swapped for {@code replacement}. */ + private static ImmutableList replacingFirst( + ImmutableList events, Event replacement) { + return ImmutableList.builder() + .add(replacement) + .addAll(events.subList(1, events.size())) + .build(); + } + + private static Event functionCallEvent(String author, FunctionCall functionCall) { + return Event.builder() + .author(author) + .content(Content.fromParts(Part.builder().functionCall(functionCall).build())) + .build(); + } + private static InvocationContext buildInvocationContext(LlmAgent agent, Session session) { return InvocationContext.builder() .pluginManager(new PluginManager()) diff --git a/core/src/test/java/com/google/adk/plugins/agentanalytics/BigQueryLoggerConfigTest.java b/core/src/test/java/com/google/adk/plugins/agentanalytics/BigQueryLoggerConfigTest.java index 2a4844627..58e1c5565 100644 --- a/core/src/test/java/com/google/adk/plugins/agentanalytics/BigQueryLoggerConfigTest.java +++ b/core/src/test/java/com/google/adk/plugins/agentanalytics/BigQueryLoggerConfigTest.java @@ -16,6 +16,7 @@ package com.google.adk.plugins.agentanalytics; +import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; @@ -27,7 +28,7 @@ public class BigQueryLoggerConfigTest { private static BigQueryLoggerConfig.Builder validBuilder() { - return BigQueryLoggerConfig.builder().projectId("test-project"); + return BigQueryLoggerConfig.builder().projectId("test-project").datasetId("test-dataset"); } @Test @@ -38,6 +39,18 @@ public void build_validConfig_succeeds() { assertEquals(10000, config.queueMaxSize()); } + @Test + public void build_defaults_matchCrossLanguageContract() { + BigQueryLoggerConfig config = validBuilder().build(); + assertThat(config.tableName()).isEqualTo("agent_events"); + } + + @Test + public void build_missingDatasetId_throws() { + BigQueryLoggerConfig.Builder builder = BigQueryLoggerConfig.builder().projectId("test-project"); + assertThrows(IllegalStateException.class, () -> builder.build()); + } + @Test public void build_nonPositiveBatchSize_throws() { BigQueryLoggerConfig.Builder builder = validBuilder().batchSize(0); diff --git a/core/src/test/java/com/google/adk/runner/RunnerTest.java b/core/src/test/java/com/google/adk/runner/RunnerTest.java index 38485a5a7..a2e90a1f7 100644 --- a/core/src/test/java/com/google/adk/runner/RunnerTest.java +++ b/core/src/test/java/com/google/adk/runner/RunnerTest.java @@ -23,6 +23,7 @@ import static com.google.adk.testing.TestUtils.createTestLlm; import static com.google.adk.testing.TestUtils.createTextLlmResponse; import static com.google.adk.testing.TestUtils.simplifyEvents; +import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.truth.Truth.assertThat; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Arrays.stream; @@ -50,6 +51,7 @@ import com.google.adk.apps.App; import com.google.adk.apps.ResumabilityConfig; import com.google.adk.artifacts.BaseArtifactService; +import com.google.adk.artifacts.InMemoryArtifactService; import com.google.adk.events.Event; import com.google.adk.flows.llmflows.Functions; import com.google.adk.models.LlmRequest; @@ -3042,6 +3044,275 @@ public void runner_executesSaveArtifactFlow() { assertThat(simplifyEvents(events.values())).containsExactly("test agent: from llm"); } + private static final String BLOB_MIME_TYPE = "example/octet-stream"; + private static final String BLOB_PAYLOAD = "blob payload"; + private static final String PLACEHOLDER_FORMAT = + "Uploaded file: %s. It has been saved to the artifacts"; + + private static Part blobPart() { + return Part.fromBytes(BLOB_PAYLOAD.getBytes(UTF_8), BLOB_MIME_TYPE); + } + + /** The text the runner substitutes for the blob it offloaded to {@code fileName}. */ + private static String placeholderFor(String fileName) { + return PLACEHOLDER_FORMAT.formatted(fileName); + } + + /** + * A message whose parts list is immutable: {@code Content.Builder.parts(List)} stores the + * caller's list without copying it. + */ + private static Content immutablePartsMessage() { + return Content.builder() + .role("user") + .parts(ImmutableList.of(Part.fromText("hello"), blobPart())) + .build(); + } + + /** A message whose parts list genai itself collected into an {@code ImmutableList}. */ + private static Content partBuilderPartsMessage() { + return Content.builder() + .role("user") + .parts(Part.fromText("hello").toBuilder(), blobPart().toBuilder()) + .build(); + } + + /** + * A message whose parts list accepts {@code set}. Used where the assertion is that the runner + * leaves the caller's message alone: with an immutable list the runner could not have modified it + * either way, so only a mutable one distinguishes copying from rewriting in place. + */ + private static Content mutablePartsMessage() { + return Content.builder() + .role("user") + .parts(new ArrayList<>(ImmutableList.of(Part.fromText("hello"), blobPart()))) + .build(); + } + + /** + * A message carrying two blobs, at part indices 1 and 2. The runner names each artifact after the + * index of the part it came from, so only a message with more than one blob distinguishes that + * from a running counter. + */ + private static Content twoBlobsMessage() { + return Content.builder() + .role("user") + .parts(ImmutableList.of(Part.fromText("hello"), blobPart(), blobPart())) + .build(); + } + + private static RunConfig saveInputBlobs(boolean enabled) { + return RunConfig.builder().saveInputBlobsAsArtifacts(enabled).build(); + } + + /** + * Points {@link #runner} at a runner backed by a fresh {@link InMemoryArtifactService}, with a + * fresh {@link #session} on it. What the service stored is read back with {@link #artifactNames} + * and {@link Runner#artifactService()}. + */ + private void useRunnerWithArtifactService() { + this.runner = + Runner.builder() + .app(App.builder().name("test").rootAgent(agent).build()) + .artifactService(new InMemoryArtifactService()) + .build(); + this.session = this.runner.sessionService().createSession("test", "user").blockingGet(); + } + + /** The names of the artifacts saved for {@link #session}. */ + private ImmutableList artifactNames() { + return ImmutableList.copyOf( + runner + .artifactService() + .listArtifactKeys("test", "user", session.id()) + .blockingGet() + .filenames()); + } + + /** The name of the single saved artifact whose file name ends in {@code suffix}. */ + private String artifactNameEndingIn(String suffix) { + ImmutableList matches = + artifactNames().stream().filter(name -> name.endsWith(suffix)).collect(toImmutableList()); + assertThat(matches).hasSize(1); + return matches.get(0); + } + + /** The user message that was actually appended to the session. */ + private Content appendedUserMessage() { + Session stored = + runner + .sessionService() + .getSession("test", "user", session.id(), Optional.empty()) + .blockingGet(); + return stored.events().stream() + .filter(event -> event.author().equals("user")) + .findFirst() + .flatMap(Event::content) + .orElseThrow(() -> new AssertionError("No user message was appended to the session.")); + } + + /** The parts of the user message that was actually appended to the session. */ + private List appendedUserParts() { + return appendedUserMessage() + .parts() + .orElseThrow(() -> new AssertionError("The appended user message has no parts.")); + } + + /** Asserts the run reached the model and emitted the agent's reply. */ + private static void assertAgentReplied(TestSubscriber events) { + events.assertComplete(); + assertThat(simplifyEvents(events.values())).containsExactly("test agent: from llm"); + } + + @Test + public void saveInputBlobsAsArtifacts_immutablePartsList_savesArtifactAndCompletes() { + useRunnerWithArtifactService(); + + var events = + runner.runAsync("user", session.id(), immutablePartsMessage(), saveInputBlobs(true)).test(); + + assertAgentReplied(events); + assertThat(artifactNames()).hasSize(1); + } + + @Test + public void saveInputBlobsAsArtifacts_partBuilderPartsList_savesArtifactAndCompletes() { + useRunnerWithArtifactService(); + + var events = + runner + .runAsync("user", session.id(), partBuilderPartsMessage(), saveInputBlobs(true)) + .test(); + + assertAgentReplied(events); + assertThat(artifactNames()).hasSize(1); + } + + @Test + public void saveInputBlobsAsArtifacts_doesNotModifyCallerMessage() { + useRunnerWithArtifactService(); + Content callerMessage = mutablePartsMessage(); + + var events = runner.runAsync("user", session.id(), callerMessage, saveInputBlobs(true)).test(); + + assertAgentReplied(events); + assertThat(artifactNames()).hasSize(1); + assertThat(callerMessage.parts().get().get(1).inlineData()).isPresent(); + assertThat(callerMessage.parts().get().get(1).text()).isEmpty(); + } + + @Test + public void saveInputBlobsAsArtifacts_appendedEventReplacesBlobWithPlaceholder() { + useRunnerWithArtifactService(); + + var events = + runner.runAsync("user", session.id(), immutablePartsMessage(), saveInputBlobs(true)).test(); + + assertAgentReplied(events); + // The appended message is a copy of the caller's, so the role has to survive the copy. + assertThat(appendedUserMessage().role()).hasValue("user"); + List appended = appendedUserParts(); + assertThat(appended).hasSize(2); + assertThat(appended.get(0).text()).hasValue("hello"); + assertThat(appended.get(1).inlineData()).isEmpty(); + assertThat(appended.get(1).text()).hasValue(placeholderFor(artifactNames().get(0))); + } + + @Test + public void saveInputBlobsAsArtifacts_twoBlobs_namesEachArtifactAfterItsPartIndex() { + useRunnerWithArtifactService(); + + var events = + runner.runAsync("user", session.id(), twoBlobsMessage(), saveInputBlobs(true)).test(); + + assertAgentReplied(events); + assertThat(artifactNames()).hasSize(2); + List appended = appendedUserParts(); + assertThat(appended).hasSize(3); + assertThat(appended.get(1).text()).hasValue(placeholderFor(artifactNameEndingIn("_1"))); + assertThat(appended.get(2).text()).hasValue(placeholderFor(artifactNameEndingIn("_2"))); + } + + @Test + public void saveInputBlobsAsArtifacts_storesBlobVerbatim() { + useRunnerWithArtifactService(); + + var events = + runner.runAsync("user", session.id(), immutablePartsMessage(), saveInputBlobs(true)).test(); + + assertAgentReplied(events); + assertThat(artifactNames()).hasSize(1); + Part stored = + runner + .artifactService() + .loadArtifact("test", "user", session.id(), artifactNames().get(0)) + .blockingGet(); + assertThat(new String(stored.inlineData().get().data().get(), UTF_8)).isEqualTo(BLOB_PAYLOAD); + assertThat(stored.inlineData().get().mimeType()).hasValue(BLOB_MIME_TYPE); + } + + @Test + public void saveInputBlobsAsArtifacts_textOnlyMessage_passesThroughUnchanged() { + useRunnerWithArtifactService(); + Content callerMessage = Content.fromParts(Part.fromText("hello")); + + var events = runner.runAsync("user", session.id(), callerMessage, saveInputBlobs(true)).test(); + + assertAgentReplied(events); + assertThat(artifactNames()).isEmpty(); + List appended = appendedUserParts(); + assertThat(appended).hasSize(1); + assertThat(appended.get(0).text()).hasValue("hello"); + assertThat(callerMessage.parts().get().get(0).text()).hasValue("hello"); + } + + @Test + public void saveInputBlobsAsArtifacts_disabledWithTextOnlyMessage_passesThroughUnchanged() { + // The default path for every ordinary agent call: no blob, and the option at its default false. + // The runner must not touch the message at all. + useRunnerWithArtifactService(); + Content callerMessage = Content.fromParts(Part.fromText("hello")); + + var events = runner.runAsync("user", session.id(), callerMessage, saveInputBlobs(false)).test(); + + assertAgentReplied(events); + assertThat(artifactNames()).isEmpty(); + List appended = appendedUserParts(); + assertThat(appended).hasSize(1); + assertThat(appended.get(0).text()).hasValue("hello"); + assertThat(callerMessage.parts().get().get(0).text()).hasValue("hello"); + } + + @Test + public void saveInputBlobsAsArtifacts_disabled_keepsBlobAndSavesNothing() { + useRunnerWithArtifactService(); + + var events = + runner + .runAsync("user", session.id(), immutablePartsMessage(), saveInputBlobs(false)) + .test(); + + assertAgentReplied(events); + assertThat(artifactNames()).isEmpty(); + assertThat(appendedUserParts().get(1).inlineData()).isPresent(); + } + + @Test + public void saveInputBlobsAsArtifacts_fromPartsConstruction_savesArtifactAndCompletes() { + useRunnerWithArtifactService(); + Content fromPartsMessage = Content.fromParts(Part.fromText("hello"), blobPart()); + + var events = + runner.runAsync("user", session.id(), fromPartsMessage, saveInputBlobs(true)).test(); + + assertAgentReplied(events); + assertThat(artifactNames()).hasSize(1); + List appended = appendedUserParts(); + assertThat(appended).hasSize(2); + assertThat(appended.get(1).inlineData()).isEmpty(); + assertThat(appended.get(1).text()).hasValue(placeholderFor(artifactNames().get(0))); + } + @Test public void runAsync_partialEvent_streamedButNotPassedToSessionService() { // The model streams a partial event followed by the final aggregated event in one turn. diff --git a/dev/pom.xml b/dev/pom.xml index 0dc29f8ac..7386e8eb3 100644 --- a/dev/pom.xml +++ b/dev/pom.xml @@ -18,7 +18,7 @@ com.google.adk google-adk-parent - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT google-adk-dev diff --git a/maven_plugin/examples/custom_tools/pom.xml b/maven_plugin/examples/custom_tools/pom.xml index 82d736187..2d9427378 100644 --- a/maven_plugin/examples/custom_tools/pom.xml +++ b/maven_plugin/examples/custom_tools/pom.xml @@ -4,7 +4,7 @@ com.example custom-tools-example - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT jar ADK Custom Tools Example diff --git a/maven_plugin/examples/simple-agent/pom.xml b/maven_plugin/examples/simple-agent/pom.xml index 50ecdd03d..a42ed255f 100644 --- a/maven_plugin/examples/simple-agent/pom.xml +++ b/maven_plugin/examples/simple-agent/pom.xml @@ -4,7 +4,7 @@ com.example simple-adk-agent - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT jar Simple ADK Agent Example diff --git a/maven_plugin/pom.xml b/maven_plugin/pom.xml index 9469fa581..552388ccb 100644 --- a/maven_plugin/pom.xml +++ b/maven_plugin/pom.xml @@ -5,7 +5,7 @@ com.google.adk google-adk-parent - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT ../pom.xml diff --git a/pom.xml b/pom.xml index 52475d377..31fe8a93f 100644 --- a/pom.xml +++ b/pom.xml @@ -17,7 +17,7 @@ com.google.adk google-adk-parent - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT pom Google Agent Development Kit Maven Parent POM diff --git a/tutorials/city-time-weather/pom.xml b/tutorials/city-time-weather/pom.xml index aaf1e9640..c17c86e86 100644 --- a/tutorials/city-time-weather/pom.xml +++ b/tutorials/city-time-weather/pom.xml @@ -20,7 +20,7 @@ com.google.adk google-adk-parent - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT ../../pom.xml diff --git a/tutorials/live-audio-single-agent/pom.xml b/tutorials/live-audio-single-agent/pom.xml index 49440394e..dfb7cd407 100644 --- a/tutorials/live-audio-single-agent/pom.xml +++ b/tutorials/live-audio-single-agent/pom.xml @@ -20,7 +20,7 @@ com.google.adk google-adk-parent - 1.7.1-SNAPSHOT + 1.7.2-SNAPSHOT ../../pom.xml