From cd65d1d239b8c192810c6a56e20ab1b74c5b5295 Mon Sep 17 00:00:00 2001 From: konlue <2362586639@qq.com> Date: Wed, 14 Oct 2026 19:48:32 +0800 Subject: [PATCH] feat(agent): unify tool failure status contract and remove null-status error paths (T38) --- .../agent/interceptor/ToolCallResponse.java | 19 ++++- .../graph/agent/interceptor/ToolOutcome.java | 74 +++++++++++++++++++ .../toolerror/ToolErrorInterceptor.java | 2 +- .../toolretry/ToolRetryInterceptor.java | 2 +- .../ai/graph/agent/node/AgentToolNode.java | 8 +- .../ToolErrorInterceptorTest.java | 62 ++++++++++++++++ .../ToolRetryInterceptorUnitTest.java | 6 ++ .../AgentToolNodeFailureMetadataTest.java | 31 ++++++-- 8 files changed, 189 insertions(+), 15 deletions(-) create mode 100644 argi-agent-framework/src/main/java/io/github/agentic/ai/graph/agent/interceptor/ToolOutcome.java create mode 100644 argi-agent-framework/src/test/java/io/github/agentic/ai/graph/agent/interceptors/ToolErrorInterceptorTest.java diff --git a/argi-agent-framework/src/main/java/io/github/agentic/ai/graph/agent/interceptor/ToolCallResponse.java b/argi-agent-framework/src/main/java/io/github/agentic/ai/graph/agent/interceptor/ToolCallResponse.java index 180478e3b..ddb4e2b36 100644 --- a/argi-agent-framework/src/main/java/io/github/agentic/ai/graph/agent/interceptor/ToolCallResponse.java +++ b/argi-agent-framework/src/main/java/io/github/agentic/ai/graph/agent/interceptor/ToolCallResponse.java @@ -29,6 +29,9 @@ public class ToolCallResponse { public final static String SUCCESS_STATUS = "SUCCESS"; + /** Status of an error response; see {@link ToolOutcome} for the full outcome vocabulary. */ + public final static String ERROR_STATUS = "error"; + /** * Metadata key carrying the structured failure kind of an error response. Values are * one of {@link #FAILURE_KIND_TIMEOUT}, {@link #FAILURE_KIND_CANCELLED}, @@ -71,10 +74,18 @@ public ToolCallResponse(String result, String toolName, String toolCallId, Strin this.metadata = metadata != null ? new HashMap<>(metadata) : Collections.emptyMap(); } + /** + * Creates a response without an explicit status. Kept for backward compatibility; + * new code should use {@link #success(String, String, String)} or one of the + * {@code error} factories so the outcome is machine-readable (see {@link ToolOutcome}). + * @param toolCallId the tool call ID + * @param toolName the tool name + * @param result the response content + * @return a new ToolCallResponse with no status set + */ public static ToolCallResponse of(String toolCallId, String toolName, String result) { return new ToolCallResponse(result, toolName, toolCallId); } - /** *

* Rewrite the status field of the source code to SUCCESS, and then make a judgment in the tool interceptor @@ -96,7 +107,7 @@ public static ToolCallResponse success(String toolCallId, String toolName, Strin * @return a new error ToolCallResponse */ public static ToolCallResponse error(String toolCallId, String toolName, String errorMessage) { - return new ToolCallResponse("Error: " + errorMessage, toolName, toolCallId, "error", + return new ToolCallResponse("Error: " + errorMessage, toolName, toolCallId, ERROR_STATUS, Map.of("error", true, "errorMessage", errorMessage)); } @@ -132,7 +143,7 @@ public static ToolCallResponse error(String toolCallId, String toolName, String if (additionalMetadata != null) { metadata.putAll(additionalMetadata); } - return new ToolCallResponse("Error: " + errorMessage, toolName, toolCallId, "error", metadata); + return new ToolCallResponse("Error: " + errorMessage, toolName, toolCallId, ERROR_STATUS, metadata); } /** @@ -179,7 +190,7 @@ public Map getMetadata() { * @return true if this is an error response */ public boolean isError() { - return "error".equals(status); + return ERROR_STATUS.equals(status); } /** diff --git a/argi-agent-framework/src/main/java/io/github/agentic/ai/graph/agent/interceptor/ToolOutcome.java b/argi-agent-framework/src/main/java/io/github/agentic/ai/graph/agent/interceptor/ToolOutcome.java new file mode 100644 index 000000000..4a63569e5 --- /dev/null +++ b/argi-agent-framework/src/main/java/io/github/agentic/ai/graph/agent/interceptor/ToolOutcome.java @@ -0,0 +1,74 @@ +/* + * Copyright 2024-2026 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.github.agentic.ai.graph.agent.interceptor; + +/** + * Canonical outcome vocabulary for {@link ToolCallResponse#getStatus()}. + * + *

+ * The status field is historically a free-form {@link String}; this class constrains the + * values that new code may produce. Legacy values keep their existing string form + * ({@link #SUCCESS} equals {@link ToolCallResponse#SUCCESS_STATUS}, {@link #ERROR} equals + * the value used by the {@code error} factories) so that existing consumers comparing + * against {@link ToolCallResponse#SUCCESS_STATUS} or calling + * {@link ToolCallResponse#isError()} are unaffected. + *

+ * + *

+ * Fine-grained failure detail is carried in the response metadata (see + * {@link ToolCallResponse#FAILURE_KIND_METADATA_KEY failureKind}, + * {@link ToolCallResponse#EXCEPTION_TYPE_METADATA_KEY exceptionType} and the validation + * {@code errorCode}/{@code fieldPath} entries). The reserved values below + * ({@link #VALIDATION_DENIED}, {@link #AUTH_DENIED}, {@link #BUSINESS_FAILED}, + * {@link #TRANSIENT}, {@link #TIMEOUT}, {@link #CANCELLED}, {@link #UNCERTAIN}) are + * declared here so the vocabulary is defined once; wiring individual producers to them, + * business-failure mapping and a typed outcome field on the response are left to + * follow-up changes. + *

+ */ +public final class ToolOutcome { + + /** The tool call completed and returned a result. */ + public static final String SUCCESS = ToolCallResponse.SUCCESS_STATUS; + + /** The tool call failed; the generic failure status used by all error paths. */ + public static final String ERROR = "error"; + + /** The tool call was rejected before execution (e.g. argument validation failed). */ + public static final String VALIDATION_DENIED = "validation-denied"; + + /** The caller is not authorized to invoke the tool. */ + public static final String AUTH_DENIED = "auth-denied"; + + /** The tool ran and reported a business-level failure. */ + public static final String BUSINESS_FAILED = "business-failed"; + + /** The failure is transient and may succeed on retry. */ + public static final String TRANSIENT = "transient"; + + /** The tool did not finish within its deadline; its effect is unknown. */ + public static final String TIMEOUT = "timeout"; + + /** The tool execution was cancelled or interrupted. */ + public static final String CANCELLED = "cancelled"; + + /** The effect of the tool call could not be determined. */ + public static final String UNCERTAIN = "uncertain"; + + private ToolOutcome() { + } + +} diff --git a/argi-agent-framework/src/main/java/io/github/agentic/ai/graph/agent/interceptor/toolerror/ToolErrorInterceptor.java b/argi-agent-framework/src/main/java/io/github/agentic/ai/graph/agent/interceptor/toolerror/ToolErrorInterceptor.java index 5410292ea..a9ed941ed 100644 --- a/argi-agent-framework/src/main/java/io/github/agentic/ai/graph/agent/interceptor/toolerror/ToolErrorInterceptor.java +++ b/argi-agent-framework/src/main/java/io/github/agentic/ai/graph/agent/interceptor/toolerror/ToolErrorInterceptor.java @@ -27,7 +27,7 @@ public ToolCallResponse interceptToolCall(ToolCallRequest request, ToolCallHandl try { return handler.call(request); } catch (Exception e) { - return ToolCallResponse.of(request.getToolCallId(), request.getToolName(), + return ToolCallResponse.error(request.getToolCallId(), request.getToolName(), "Tool failed: " + e.getMessage()); } } diff --git a/argi-agent-framework/src/main/java/io/github/agentic/ai/graph/agent/interceptor/toolretry/ToolRetryInterceptor.java b/argi-agent-framework/src/main/java/io/github/agentic/ai/graph/agent/interceptor/toolretry/ToolRetryInterceptor.java index 7e3b6a2f2..00aca031e 100644 --- a/argi-agent-framework/src/main/java/io/github/agentic/ai/graph/agent/interceptor/toolretry/ToolRetryInterceptor.java +++ b/argi-agent-framework/src/main/java/io/github/agentic/ai/graph/agent/interceptor/toolretry/ToolRetryInterceptor.java @@ -133,7 +133,7 @@ public ToolCallResponse interceptToolCall(ToolCallRequest request, ToolCallHandl : "Tool call failed after " + maxAttempts + " attempts: " + lastException.getMessage(); log.error("Tool '{}' failed after {} attempts: {}", toolName, maxAttempts, lastException.getMessage()); - return ToolCallResponse.of(request.getToolCallId(), request.getToolName(), errorMessage); + return ToolCallResponse.error(request.getToolCallId(), request.getToolName(), errorMessage); } } diff --git a/argi-agent-framework/src/main/java/io/github/agentic/ai/graph/agent/node/AgentToolNode.java b/argi-agent-framework/src/main/java/io/github/agentic/ai/graph/agent/node/AgentToolNode.java index b3cd4a7da..c83d1f12e 100644 --- a/argi-agent-framework/src/main/java/io/github/agentic/ai/graph/agent/node/AgentToolNode.java +++ b/argi-agent-framework/src/main/java/io/github/agentic/ai/graph/agent/node/AgentToolNode.java @@ -993,9 +993,10 @@ private ToolCallResponse executeSyncTool(ToolCallback callback, ToolCallRequest /** * Builds the response for a {@link ToolExecutionException} whose message has already - * been produced by the configured {@link ToolExecutionExceptionProcessor}. The - * response keeps its historical shape (no status) and additionally carries the - * structured failure metadata so interceptors can tell what happened. + * been produced by the configured {@link ToolExecutionExceptionProcessor}. The response + * carries the error status (see {@link ToolOutcome}) plus the structured failure + * metadata so interceptors can tell what happened; historically it shipped without a + * status, which made the failure indistinguishable from a plain result upstream. */ private ToolCallResponse processedFailureResponse(ToolCallRequest request, String result, ToolExecutionException exception) { @@ -1003,6 +1004,7 @@ private ToolCallResponse processedFailureResponse(ToolCallRequest request, Strin .content(result) .toolName(request.getToolName()) .toolCallId(request.getToolCallId()) + .status(ToolCallResponse.ERROR_STATUS) .metadata(failureMetadata(exception)) .build(); } diff --git a/argi-agent-framework/src/test/java/io/github/agentic/ai/graph/agent/interceptors/ToolErrorInterceptorTest.java b/argi-agent-framework/src/test/java/io/github/agentic/ai/graph/agent/interceptors/ToolErrorInterceptorTest.java new file mode 100644 index 000000000..0e6bff58a --- /dev/null +++ b/argi-agent-framework/src/test/java/io/github/agentic/ai/graph/agent/interceptors/ToolErrorInterceptorTest.java @@ -0,0 +1,62 @@ +/* + * Copyright 2024-2026 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.github.agentic.ai.graph.agent.interceptors; + +import io.github.agentic.ai.graph.agent.interceptor.ToolCallRequest; +import io.github.agentic.ai.graph.agent.interceptor.ToolCallResponse; +import io.github.agentic.ai.graph.agent.interceptor.toolerror.ToolErrorInterceptor; +import org.junit.jupiter.api.Test; + +import java.util.HashMap; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Verifies that {@link ToolErrorInterceptor} swallows handler exceptions into an + * explicitly-shaped error response (status + error metadata) instead of a status-less + * response, and passes successful results through untouched. + */ +class ToolErrorInterceptorTest { + + private ToolCallRequest request() { + return new ToolCallRequest("t", "{}", "id-1", new HashMap<>()); + } + + @Test + void successfulCallPassesThrough() { + ToolCallResponse response = ToolErrorInterceptor.builder().build() + .interceptToolCall(request(), r -> ToolCallResponse.success(r.getToolCallId(), r.getToolName(), "ok")); + + assertThat(response.getStatus()).isEqualTo(ToolCallResponse.SUCCESS_STATUS); + assertThat(response.getResult()).isEqualTo("ok"); + } + + @Test + void handlerExceptionBecomesExplicitErrorResponse() { + ToolCallResponse response = ToolErrorInterceptor.builder().build() + .interceptToolCall(request(), r -> { + throw new IllegalStateException("backend down"); + }); + + assertThat(response.getStatus()).isEqualTo(ToolCallResponse.ERROR_STATUS); + assertThat(response.isError()).isTrue(); + assertThat(response.getResult()).contains("backend down"); + assertThat(response.getMetadata()) + .containsEntry("error", true) + .containsEntry("errorMessage", "Tool failed: backend down"); + } + +} diff --git a/argi-agent-framework/src/test/java/io/github/agentic/ai/graph/agent/interceptors/ToolRetryInterceptorUnitTest.java b/argi-agent-framework/src/test/java/io/github/agentic/ai/graph/agent/interceptors/ToolRetryInterceptorUnitTest.java index 68112f6b6..c5cc58361 100644 --- a/argi-agent-framework/src/test/java/io/github/agentic/ai/graph/agent/interceptors/ToolRetryInterceptorUnitTest.java +++ b/argi-agent-framework/src/test/java/io/github/agentic/ai/graph/agent/interceptors/ToolRetryInterceptorUnitTest.java @@ -24,6 +24,7 @@ import org.junit.jupiter.api.Test; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -55,6 +56,11 @@ void runsAllConfiguredAttempts() { }); assertEquals(3, calls.get()); assertTrue(response.getResult().contains("3 attempts")); + // Exhausted retries must surface as an explicit error, not a status-less response. + assertEquals(ToolCallResponse.ERROR_STATUS, response.getStatus()); + assertTrue(response.isError()); + assertThat(response.getMetadata()).containsEntry("error", true); + assertThat(response.getMetadata().get("errorMessage")).asString().contains("boom"); } @Test diff --git a/argi-agent-framework/src/test/java/io/github/agentic/ai/graph/agent/node/AgentToolNodeFailureMetadataTest.java b/argi-agent-framework/src/test/java/io/github/agentic/ai/graph/agent/node/AgentToolNodeFailureMetadataTest.java index 8e198b946..58d6cbd8c 100644 --- a/argi-agent-framework/src/test/java/io/github/agentic/ai/graph/agent/node/AgentToolNodeFailureMetadataTest.java +++ b/argi-agent-framework/src/test/java/io/github/agentic/ai/graph/agent/node/AgentToolNodeFailureMetadataTest.java @@ -134,8 +134,8 @@ void syncExceptionCarriesExecutionKindAndType() throws Exception { @Test @Timeout(value = 10, unit = TimeUnit.SECONDS) - @DisplayName("ToolExecutionException keeps its historical shape and reports the wrapped exception type") - void toolExecutionExceptionKeepsShapeAndReportsWrappedType() throws Exception { + @DisplayName("ToolExecutionException is reported as an error and carries the wrapped exception type") + void toolExecutionExceptionIsReportedAsErrorAndCarriesWrappedType() throws Exception { ToolDefinition definition = definition("wrappedTool"); ToolCallback wrapped = syncTool(definition, () -> { throw new ToolExecutionException(definition, new IllegalArgumentException("bad argument")); @@ -143,11 +143,30 @@ void toolExecutionExceptionKeepsShapeAndReportsWrappedType() throws Exception { ToolCallResponse response = runSingleTool(wrapped, "wrappedTool", false); - // Historical behaviour: processed by ToolExecutionExceptionProcessor, no status. - assertThat(response.getStatus()).isNull(); - assertThat(response.isError()).isFalse(); + // The processed message is still delivered as the result content, but the + // response is now explicitly an error instead of shipping without a status. + assertThat(response.getStatus()).isEqualTo(ToolCallResponse.ERROR_STATUS); + assertThat(response.isError()).isTrue(); assertThat(response.getResult()).contains("bad argument"); - // New: structured metadata points at the exception thrown by the tool itself. + // Structured metadata points at the exception thrown by the tool itself. + assertThat(response.getFailureKind()).contains(ToolCallResponse.FAILURE_KIND_EXECUTION); + assertThat(response.getExceptionType()).contains(IllegalArgumentException.class.getName()); + } + + @Test + @Timeout(value = 10, unit = TimeUnit.SECONDS) + @DisplayName("async ToolExecutionException is reported as an error with the same status contract") + void asyncToolExecutionExceptionIsReportedAsError() throws Exception { + ToolDefinition definition = definition("asyncWrappedTool"); + ToolCallback wrapped = asyncTool("asyncWrappedTool", Duration.ofSeconds(5), + () -> CompletableFuture.failedFuture( + new ToolExecutionException(definition, new IllegalArgumentException("bad async argument")))); + + ToolCallResponse response = runSingleTool(wrapped, "asyncWrappedTool", false); + + assertThat(response.getStatus()).isEqualTo(ToolCallResponse.ERROR_STATUS); + assertThat(response.isError()).isTrue(); + assertThat(response.getResult()).contains("bad async argument"); assertThat(response.getFailureKind()).contains(ToolCallResponse.FAILURE_KIND_EXECUTION); assertThat(response.getExceptionType()).contains(IllegalArgumentException.class.getName()); }