From fe18b3838781ae74c5a93d637a94a04b416bf12d Mon Sep 17 00:00:00 2001 From: King52HerTz Date: Mon, 28 Sep 2026 13:45:19 +0800 Subject: [PATCH 1/2] fix(core): report the tool return value on ToolResultEndEvent ToolEmitter documents that emitted chunks do not affect what the LLM receives; only the tool method's return value does. The event stream disagrees: once a tool has emitted chunks, ReActAgent records its id in chunkedToolIds and emitToolResultDelta then skips the return value, and ToolResultEndEvent carries no result either. A consumer reconstructing the tool message from the stream therefore gets the concatenated progress text. For AG-UI that means TOOL_CALL_RESULT holds progress text while the model saw the return value, so the two histories diverge: a client resending full history feeds progress text back as the tool result, anything emitted only for the UI leaks into it, and the UI shows the wrong outcome. Put the return value on the end event as an explicit field and let AguiStreamContext prefer it over the delta buffer. It is not stored under AgentEvent.getMetadata(), which passes the tool result's own metadata through unchanged; ReActAgentNewLoopE2ETest asserts that map exactly, so adding a framework key there would be a contract change for unrelated consumers. Producers that report no return value keep the previous behaviour. --- .../java/io/agentscope/core/ReActAgent.java | 28 +++++++++- .../core/event/ToolResultEndEvent.java | 40 ++++++++++++++- .../adapter/strategy/AguiStreamContext.java | 31 ++++++++--- .../strategy/ToolResultEventConverter.java | 2 +- .../agui/adapter/AguiAgentAdapterV2Test.java | 51 +++++++++++++++++++ 5 files changed, 141 insertions(+), 11 deletions(-) diff --git a/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java b/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java index 5c35401474..763d692f83 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java +++ b/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java @@ -3342,7 +3342,10 @@ private Flux runToolBatch( .getId(), entry.getKey() .getName(), - state) + state, + finalToolResultText( + entry + .getValue())) .withMetadata( entry.getValue() .getMetadata())); @@ -3499,6 +3502,29 @@ private void emitToolResultDelta( } } + /** + * Join the text blocks of a tool method's return value, reported on {@link + * ToolResultEndEvent#getFinalResultText()}. + * + *

Progress chunks emitted through {@link io.agentscope.core.tool.ToolEmitter} travel the + * event stream as deltas but are deliberately not what the model receives, so a consumer + * rebuilding the tool message from deltas needs the return value from this event instead. + * + * @return joined text, or {@code null} when the result has no non-empty text blocks + */ + private String finalToolResultText(ToolResultBlock result) { + if (result == null || result.getOutput() == null) { + return null; + } + String text = + result.getOutput().stream() + .filter(TextBlock.class::isInstance) + .map(block -> ((TextBlock) block).getText()) + .filter(value -> value != null && !value.isEmpty()) + .collect(Collectors.joining()); + return text.isEmpty() ? null : text; + } + private ToolResultState determineToolResultState(ToolResultBlock result) { if (result.isSuspended()) { return ToolResultState.RUNNING; diff --git a/agentscope-core/src/main/java/io/agentscope/core/event/ToolResultEndEvent.java b/agentscope-core/src/main/java/io/agentscope/core/event/ToolResultEndEvent.java index b3ff00dd50..7f19e77654 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/event/ToolResultEndEvent.java +++ b/agentscope-core/src/main/java/io/agentscope/core/event/ToolResultEndEvent.java @@ -27,6 +27,18 @@ public class ToolResultEndEvent extends AgentEvent { private final String toolCallName; private final ToolResultState state; + /** + * The tool method's return value as text, or {@code null} when the producer did not report one. + * + *

A tool that streams progress through {@link io.agentscope.core.tool.ToolEmitter} pushes + * those chunks onto the event stream as deltas, but per that emitter's contract progress is not + * what the model receives — only the return value is. A consumer rebuilding the tool message + * from the delta stream alone would persist progress text as the result, so the authoritative + * value travels here. It deliberately does not ride on {@link AgentEvent#getMetadata()}, which + * carries the tool result's own metadata through unchanged. + */ + private final String finalResultText; + @JsonCreator public ToolResultEndEvent( @JsonProperty("id") String id, @@ -35,12 +47,14 @@ public ToolResultEndEvent( @JsonProperty("toolCallId") String toolCallId, @JsonProperty("toolCallName") String toolCallName, @JsonProperty("state") ToolResultState state, - @JsonProperty("metadata") Map metadata) { + @JsonProperty("metadata") Map metadata, + @JsonProperty("finalResultText") String finalResultText) { super(id, createdAt); this.replyId = replyId; this.toolCallId = toolCallId; this.toolCallName = toolCallName; this.state = state; + this.finalResultText = finalResultText; this.withMetadata(metadata); } @@ -54,15 +68,29 @@ public ToolResultEndEvent( String toolCallId, String toolCallName, ToolResultState state) { - this(id, createdAt, replyId, toolCallId, toolCallName, state, null); + this(id, createdAt, replyId, toolCallId, toolCallName, state, null, null); } public ToolResultEndEvent( String replyId, String toolCallId, String toolCallName, ToolResultState state) { + this(replyId, toolCallId, toolCallName, state, null); + } + + /** + * As {@link #ToolResultEndEvent(String, String, String, ToolResultState)} additionally reporting + * the tool method's return value. + */ + public ToolResultEndEvent( + String replyId, + String toolCallId, + String toolCallName, + ToolResultState state, + String finalResultText) { this.replyId = replyId; this.toolCallId = toolCallId; this.toolCallName = toolCallName; this.state = state; + this.finalResultText = finalResultText; } @Override @@ -85,4 +113,12 @@ public String getToolCallName() { public ToolResultState getState() { return state; } + + /** + * The tool method's return value as text, or {@code null} when the producer did not report one + * (for example when the result carries no text blocks). + */ + public String getFinalResultText() { + return finalResultText; + } } diff --git a/agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/main/java/io/agentscope/core/agui/adapter/strategy/AguiStreamContext.java b/agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/main/java/io/agentscope/core/agui/adapter/strategy/AguiStreamContext.java index a646489683..2dcee7dae6 100644 --- a/agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/main/java/io/agentscope/core/agui/adapter/strategy/AguiStreamContext.java +++ b/agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/main/java/io/agentscope/core/agui/adapter/strategy/AguiStreamContext.java @@ -282,21 +282,38 @@ public void appendToolResultData(String toolCallId, ContentBlock data) { } public void endToolResult(String replyId, String toolCallId) { + endToolResult(replyId, toolCallId, null); + } + + /** + * Close out a tool call and emit its {@code TOOL_CALL_RESULT}. + * + *

{@code finalResultText} is the tool method's return value as reported by {@link + * io.agentscope.core.event.ToolResultEndEvent}. It wins over the buffered deltas when present: + * a tool that streamed progress through {@code ToolEmitter} has progress text in the buffer, and + * per that emitter's contract progress is not what the model received. Persisting the buffer as + * the result would feed progress text back to the model on the next history replay, and would + * leak anything emitted only for the UI. + * + * @param replyId the enclosing reply id + * @param toolCallId the tool call being closed + * @param finalResultText the return value, or {@code null} to fall back to buffered deltas + */ + public void endToolResult(String replyId, String toolCallId, String finalResultText) { if (!hasKnownToolCall(toolCallId, "ToolResultEndEvent")) { return; } if (startedToolCalls.contains(toolCallId) && endedToolCalls.add(toolCallId)) { emit(new AguiEvent.ToolCallEnd(threadId, runId, toolCallId)); } - StringBuilder content = toolResultContent.remove(toolCallId); + StringBuilder buffered = toolResultContent.remove(toolCallId); + String content = + finalResultText != null + ? finalResultText + : buffered != null && !buffered.isEmpty() ? buffered.toString() : null; emit( new AguiEvent.ToolCallResult( - threadId, - runId, - toolCallId, - content != null && !content.isEmpty() ? content.toString() : null, - "tool", - replyId + ":" + toolCallId)); + threadId, runId, toolCallId, content, "tool", replyId + ":" + toolCallId)); } public void markToolCallSuspended(String toolCallId) { diff --git a/agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/main/java/io/agentscope/core/agui/adapter/strategy/ToolResultEventConverter.java b/agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/main/java/io/agentscope/core/agui/adapter/strategy/ToolResultEventConverter.java index 3a7d37ac32..58c874446e 100644 --- a/agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/main/java/io/agentscope/core/agui/adapter/strategy/ToolResultEventConverter.java +++ b/agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/main/java/io/agentscope/core/agui/adapter/strategy/ToolResultEventConverter.java @@ -48,7 +48,7 @@ public void convert(AgentEvent event, AguiStreamContext context) { context.markToolCallSuspended(end.getToolCallId()); return; } - context.endToolResult(end.getReplyId(), end.getToolCallId()); + context.endToolResult(end.getReplyId(), end.getToolCallId(), end.getFinalResultText()); } } } diff --git a/agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/test/java/io/agentscope/core/agui/adapter/AguiAgentAdapterV2Test.java b/agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/test/java/io/agentscope/core/agui/adapter/AguiAgentAdapterV2Test.java index 177da4950c..39d0e7f3a2 100644 --- a/agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/test/java/io/agentscope/core/agui/adapter/AguiAgentAdapterV2Test.java +++ b/agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/test/java/io/agentscope/core/agui/adapter/AguiAgentAdapterV2Test.java @@ -755,6 +755,57 @@ void testToolResultDeltasAreAggregatedIntoToolCallResult() { assertEquals("reply-tool:tool-1", result.messageId()); } + @Test + void testEmittedProgressChunksDoNotBecomeTheToolResult() { + // ToolEmitter streams progress as deltas, but per its own contract only the tool + // method's return value reaches the model. TOOL_CALL_RESULT is what a client stores as + // the tool message, so progress text must not end up in it. + ToolResultEndEvent end = + new ToolResultEndEvent( + "reply-emitter", "tool-1", "enroll", null, "fingerprint enrolled"); + + List events = + runReActEvents( + new ToolCallStartEvent("reply-emitter", "tool-1", "enroll"), + new ToolCallEndEvent("reply-emitter", "tool-1", "enroll"), + new ToolResultStartEvent("reply-emitter", "tool-1", "enroll"), + new ToolResultTextDeltaEvent( + "reply-emitter", "tool-1", "enroll", "press once"), + new ToolResultTextDeltaEvent( + "reply-emitter", "tool-1", "enroll", "press again"), + end); + + assertEquals( + "fingerprint enrolled", + firstToolCallResult(events).content(), + "the return value replaces the progress buffer, matching what the model saw"); + } + + @Test + void testToolResultWithoutFinalTextMetadataStillUsesDeltaBuffer() { + List events = + runReActEvents( + new ToolCallStartEvent("reply-legacy", "tool-1", "lookup"), + new ToolCallEndEvent("reply-legacy", "tool-1", "lookup"), + new ToolResultStartEvent("reply-legacy", "tool-1", "lookup"), + new ToolResultTextDeltaEvent( + "reply-legacy", "tool-1", "lookup", "partial"), + new ToolResultEndEvent("reply-legacy", "tool-1", "lookup", null)); + + assertEquals( + "partial", + firstToolCallResult(events).content(), + "producers that do not report a return value keep the previous behaviour"); + } + + private static AguiEvent.ToolCallResult firstToolCallResult(List events) { + return events.stream() + .filter(AguiEvent.ToolCallResult.class::isInstance) + .map(AguiEvent.ToolCallResult.class::cast) + .findFirst() + .orElseThrow(); + } + @Test void testParallelToolResultsUsePerToolMessageIds() { List messageIds = From 0e94997e14b124f06978dbd217cdd24118dec484 Mon Sep 17 00:00:00 2001 From: King52HerTz Date: Wed, 7 Oct 2026 02:20:00 +0800 Subject: [PATCH 2/2] fix(core): close the null-vs-empty gap and align both result consumers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up for #3331. null on getFinalResultText() meant two different things: "the producer reports nothing" and "the tool returned content with no text in it". The second read as the first, so an image-only or blank return value fell back to the delta buffer and put progress text straight back into TOOL_CALL_RESULT — the leak this field exists to close. A result that carries content now reports its joined text even when that join is empty; only a result with no content blocks at all stays unreported. finalResultText joins text blocks, while an image or structured part reaches a client as ToolResultDataDeltaEvent, so replacing the whole buffer dropped it. AguiStreamContext now keeps the non-text fragments beside the buffer and re-appends them after the return value. The fallback path is untouched, including the arrival-order rendering the existing tests pin. service-dataplane was the second first-party consumer and still persisted the buffer, so the two disagreed about what a tool returned. SessionEventMapper applies the same rule with the same fallback, reuses the shared payload cap, and re-appends non-text fragments the way it already glued them. Also closes the two Info points: JSON round-trip for the new @JsonCreator property (the value survives polymorphic decode, "" decodes as "" rather than null, and a payload written before the field existed decodes to null), and the ReActAgent population path end to end — with an assertion that the progress chunk really reached the stream, so it cannot pass by the tool never emitting. Checked by mutation: reverting the empty-vs-null rule fails one test, never populating the field fails two, dropping the re-append in the AG-UI adapter fails two, reverting the dataplane rule fails two. Full reactor mvn test green, spotless:check green. --- .../java/io/agentscope/core/ReActAgent.java | 22 ++-- .../core/event/ToolResultEndEvent.java | 21 ++-- .../core/agent/ReActAgentNewLoopE2ETest.java | 101 ++++++++++++++++++ .../core/event/AgentEventStreamTest.java | 57 ++++++++++ .../adapter/strategy/AguiStreamContext.java | 40 ++++++- .../agui/adapter/AguiAgentAdapterV2Test.java | 77 +++++++++++++ .../web/managed/SessionEventMapper.java | 58 +++++++++- .../web/managed/SessionEventMapperTest.java | 64 +++++++++++ 8 files changed, 418 insertions(+), 22 deletions(-) diff --git a/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java b/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java index 763d692f83..ee128d2934 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java +++ b/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java @@ -3510,19 +3510,23 @@ private void emitToolResultDelta( * event stream as deltas but are deliberately not what the model receives, so a consumer * rebuilding the tool message from deltas needs the return value from this event instead. * - * @return joined text, or {@code null} when the result has no non-empty text blocks + *

An empty join is reported as {@code ""}, not {@code null}: a tool that returns an image + * or a blank string did produce a return value, and a consumer that fell back to the delta + * buffer for it would persist progress text as the result — the exact leak this field exists + * to close. Only a result we know nothing about is left unreported. + * + * @return joined text (possibly empty), or {@code null} when there are no content blocks to + * report from */ private String finalToolResultText(ToolResultBlock result) { - if (result == null || result.getOutput() == null) { + if (result == null || result.getOutput() == null || result.getOutput().isEmpty()) { return null; } - String text = - result.getOutput().stream() - .filter(TextBlock.class::isInstance) - .map(block -> ((TextBlock) block).getText()) - .filter(value -> value != null && !value.isEmpty()) - .collect(Collectors.joining()); - return text.isEmpty() ? null : text; + return result.getOutput().stream() + .filter(TextBlock.class::isInstance) + .map(block -> ((TextBlock) block).getText()) + .filter(value -> value != null && !value.isEmpty()) + .collect(Collectors.joining()); } private ToolResultState determineToolResultState(ToolResultBlock result) { diff --git a/agentscope-core/src/main/java/io/agentscope/core/event/ToolResultEndEvent.java b/agentscope-core/src/main/java/io/agentscope/core/event/ToolResultEndEvent.java index 7f19e77654..9da90992e5 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/event/ToolResultEndEvent.java +++ b/agentscope-core/src/main/java/io/agentscope/core/event/ToolResultEndEvent.java @@ -28,14 +28,15 @@ public class ToolResultEndEvent extends AgentEvent { private final ToolResultState state; /** - * The tool method's return value as text, or {@code null} when the producer did not report one. + * The tool method's return value as text, or {@code null} when the producer reports nothing. * - *

A tool that streams progress through {@link io.agentscope.core.tool.ToolEmitter} pushes - * those chunks onto the event stream as deltas, but per that emitter's contract progress is not - * what the model receives — only the return value is. A consumer rebuilding the tool message - * from the delta stream alone would persist progress text as the result, so the authoritative - * value travels here. It deliberately does not ride on {@link AgentEvent#getMetadata()}, which - * carries the tool result's own metadata through unchanged. + *

{@code ""} is a reported value, not an absent one: it means the tool returned content with + * no text in it (an image, or a blank result), and a consumer must not fall back to its delta + * buffer for that case. + * + *

This deliberately does not ride on {@link AgentEvent#getMetadata()}, which passes the tool + * result's own metadata through unchanged — {@code ReActAgentNewLoopE2ETest} asserts that map + * exactly, so a framework key in it would be a contract change for unrelated consumers. */ private final String finalResultText; @@ -115,8 +116,10 @@ public ToolResultState getState() { } /** - * The tool method's return value as text, or {@code null} when the producer did not report one - * (for example when the result carries no text blocks). + * The tool method's return value as text. + * + * @return the return value (possibly empty), or {@code null} when the producer reports nothing, + * which leaves consumers falling back to the delta stream */ public String getFinalResultText() { return finalResultText; diff --git a/agentscope-core/src/test/java/io/agentscope/core/agent/ReActAgentNewLoopE2ETest.java b/agentscope-core/src/test/java/io/agentscope/core/agent/ReActAgentNewLoopE2ETest.java index 2220f562b7..926d818896 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/agent/ReActAgentNewLoopE2ETest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/agent/ReActAgentNewLoopE2ETest.java @@ -29,12 +29,14 @@ import io.agentscope.core.event.ToolResultEndEvent; import io.agentscope.core.event.ToolResultTextDeltaEvent; import io.agentscope.core.message.ContentBlock; +import io.agentscope.core.message.ImageBlock; import io.agentscope.core.message.Msg; import io.agentscope.core.message.MsgRole; import io.agentscope.core.message.TextBlock; import io.agentscope.core.message.ToolResultBlock; import io.agentscope.core.message.ToolResultState; import io.agentscope.core.message.ToolUseBlock; +import io.agentscope.core.message.URLSource; import io.agentscope.core.middleware.ActingInput; import io.agentscope.core.middleware.AgentInput; import io.agentscope.core.middleware.MiddlewareBase; @@ -416,4 +418,103 @@ void streamEventsRestoresEmitterWhenActingContextLosesEventKeys() { assertTrue(events.get(0) instanceof AgentStartEvent); assertTrue(events.get(events.size() - 1) instanceof AgentEndEvent); } + + @Test + void toolResultEndEventReportsTheReturnValueWhenTheToolAlsoStreamedProgress() { + List events = runWithProgressTool(ToolResultBlock.text("fingerprint enrolled")); + + // Without this the test would still pass if the tool never emitted anything, and the field + // under test would be trivially correct — the progress chunk is the leak being closed. + assertTrue( + events.stream() + .filter(ToolResultTextDeltaEvent.class::isInstance) + .map(e -> ((ToolResultTextDeltaEvent) e).getDelta()) + .anyMatch("scanning 3 dirs"::equals), + "the progress chunk has to reach the stream"); + assertEquals( + "fingerprint enrolled", + lastToolResultEnd(events).getFinalResultText(), + "the model received the return value, so that is what the end event reports"); + } + + @Test + void imageOnlyReturnValueIsReportedAsEmptyRatherThanAbsent() { + // "" and null mean different things downstream: null tells a consumer to fall back to the + // deltas it buffered, which would feed progress text back as the tool result on replay. + List events = + runWithProgressTool( + ToolResultBlock.of( + ImageBlock.builder() + .source(new URLSource("https://example.com/chart.png")) + .build())); + + assertEquals("", lastToolResultEnd(events).getFinalResultText()); + } + + private List runWithProgressTool(ToolResultBlock returnValue) { + ScriptedModel model = + new ScriptedModel( + List.of( + () -> Flux.just(toolUseResponse("c1", "enroll", "alpha")), + () -> Flux.just(textResponse("done")))); + Toolkit tk = new Toolkit(); + tk.registerAgentTool(new ProgressTool("enroll", returnValue)); + + List events = + ReActAgent.builder() + .name("asst") + .sysPrompt("you are helpful") + .model(model) + .toolkit(tk) + .build() + .streamEvents( + List.of( + Msg.builder() + .role(MsgRole.USER) + .textContent("run enroll") + .build())) + .collectList() + .block(); + assertNotNull(events); + return events; + } + + private static ToolResultEndEvent lastToolResultEnd(List events) { + return events.stream() + .filter(ToolResultEndEvent.class::isInstance) + .map(ToolResultEndEvent.class::cast) + .reduce((first, second) -> second) + .orElseThrow(); + } + + /** Streams one progress chunk, then returns {@code returnValue}: the {@code ToolEmitter} deal. */ + private static final class ProgressTool extends ToolBase { + private final ToolResultBlock returnValue; + + ProgressTool(String name, ToolResultBlock returnValue) { + super( + name, + "streams progress then returns", + AlwaysAllowTool.schema(), + true, + true, + false, + null, + false, + false); + this.returnValue = returnValue; + } + + @Override + public Mono checkPermissions( + Map input, PermissionContextState ctx) { + return Mono.just(PermissionDecision.allow("ok")); + } + + @Override + public Mono callAsync(ToolCallParam param) { + param.getEmitter().emit(ToolResultBlock.text("scanning 3 dirs")); + return Mono.just(returnValue); + } + } } diff --git a/agentscope-core/src/test/java/io/agentscope/core/event/AgentEventStreamTest.java b/agentscope-core/src/test/java/io/agentscope/core/event/AgentEventStreamTest.java index 5adf5976fe..8e7b9ec128 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/event/AgentEventStreamTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/event/AgentEventStreamTest.java @@ -16,7 +16,9 @@ package io.agentscope.core.event; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -190,6 +192,61 @@ void toolResultStartEventEmptyMetadata() throws Exception { deserialized.getMetadata() == null || deserialized.getMetadata().isEmpty(), "ToolResultStartEvent should not carry result metadata before execution"); } + + @Test + @DisplayName("finalResultText survives the polymorphic round trip") + void finalResultTextSurvivesRoundTrip() throws Exception { + ToolResultEndEvent original = + new ToolResultEndEvent( + "reply-1", + "tc-1", + "search", + ToolResultState.SUCCESS, + "fingerprint enrolled"); + + String json = mapper.writeValueAsString(original); + assertTrue( + json.contains("\"finalResultText\":\"fingerprint enrolled\""), + "the new property has to be on the wire, not just in the object: " + json); + + ToolResultEndEvent back = + assertInstanceOf( + ToolResultEndEvent.class, mapper.readValue(json, AgentEvent.class)); + assertEquals("fingerprint enrolled", back.getFinalResultText()); + } + + @Test + @DisplayName("an empty return value decodes as empty, not as absent") + void emptyFinalResultTextIsNotLossy() throws Exception { + // "" carries meaning: the tool did return content, it just holds no text. Read back as + // null it would send consumers falling back to the delta buffer, re-opening the leak. + ToolResultEndEvent original = + new ToolResultEndEvent( + "reply-1", "tc-1", "search", ToolResultState.SUCCESS, ""); + + ToolResultEndEvent back = + assertInstanceOf( + ToolResultEndEvent.class, + mapper.readValue( + mapper.writeValueAsString(original), AgentEvent.class)); + assertEquals("", back.getFinalResultText()); + } + + @Test + @DisplayName("a payload written before the field existed decodes to null") + void absentFinalResultTextDecodesToNull() throws Exception { + // Sessions persisted by an older build are reloaded through this same creator. + ToolResultEndEvent back = + assertInstanceOf( + ToolResultEndEvent.class, + mapper.readValue( + "{\"type\":\"TOOL_RESULT_END\",\"replyId\":\"r\"," + + "\"toolCallId\":\"t\",\"toolCallName\":\"n\"," + + "\"state\":\"success\"}", + AgentEvent.class)); + assertEquals("r", back.getReplyId()); + assertNull(back.getFinalResultText()); + } } @Nested diff --git a/agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/main/java/io/agentscope/core/agui/adapter/strategy/AguiStreamContext.java b/agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/main/java/io/agentscope/core/agui/adapter/strategy/AguiStreamContext.java index 2dcee7dae6..2318cf8b7c 100644 --- a/agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/main/java/io/agentscope/core/agui/adapter/strategy/AguiStreamContext.java +++ b/agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/main/java/io/agentscope/core/agui/adapter/strategy/AguiStreamContext.java @@ -57,6 +57,13 @@ public class AguiStreamContext { private String currentTextMessageId; private String currentReasoningMessageId; private final Map toolResultContent = new LinkedHashMap<>(); + + /** + * Non-text result blocks carried by the delta stream, kept beside {@link #toolResultContent} so + * an authoritative return value can replace the progress text without erasing them. + */ + private final Map> toolResultDataBlocks = new LinkedHashMap<>(); + private final Map pendingInterrupts = new LinkedHashMap<>(); private final Set warnedMissingToolCallIdOperations = new LinkedHashSet<>(); private final TokenUsageAccumulator tokenUsageAccumulator = new TokenUsageAccumulator(); @@ -278,7 +285,11 @@ public void appendToolResultData(String toolCallId, ContentBlock data) { if (!buffer.isEmpty()) { buffer.append("\n"); } - buffer.append(serialize(data)); + String fragment = serialize(data); + buffer.append(fragment); + toolResultDataBlocks + .computeIfAbsent(toolCallId, ignored -> new ArrayList<>()) + .add(fragment); } public void endToolResult(String replyId, String toolCallId) { @@ -307,20 +318,45 @@ public void endToolResult(String replyId, String toolCallId, String finalResultT emit(new AguiEvent.ToolCallEnd(threadId, runId, toolCallId)); } StringBuilder buffered = toolResultContent.remove(toolCallId); + List dataBlocks = toolResultDataBlocks.remove(toolCallId); String content = finalResultText != null - ? finalResultText + ? appendDataBlocks(finalResultText, dataBlocks) : buffered != null && !buffered.isEmpty() ? buffered.toString() : null; emit( new AguiEvent.ToolCallResult( threadId, runId, toolCallId, content, "tool", replyId + ":" + toolCallId)); } + /** + * Keep the non-text blocks the stream carried while the authoritative return value replaces the + * buffered text. + * + *

{@code ToolResultEndEvent#getFinalResultText()} joins text blocks only, so a multimodal + * result — an image, a structured part — travels as {@code ToolResultDataDeltaEvent}s. Swapping + * the whole buffer for the return value would silently drop those parts and leave a client + * rendering result parts with an empty message. + */ + private static String appendDataBlocks(String text, List dataBlocks) { + if (dataBlocks == null || dataBlocks.isEmpty()) { + return text; + } + StringBuilder out = new StringBuilder(text); + for (String block : dataBlocks) { + if (!out.isEmpty()) { + out.append("\n"); + } + out.append(block); + } + return out.toString(); + } + public void markToolCallSuspended(String toolCallId) { if (!hasKnownToolCall(toolCallId, "ToolResultEndEvent")) { return; } toolResultContent.remove(toolCallId); + toolResultDataBlocks.remove(toolCallId); } public void addInterrupt(AguiEvent.Interrupt interrupt) { diff --git a/agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/test/java/io/agentscope/core/agui/adapter/AguiAgentAdapterV2Test.java b/agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/test/java/io/agentscope/core/agui/adapter/AguiAgentAdapterV2Test.java index 39d0e7f3a2..7a135462ad 100644 --- a/agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/test/java/io/agentscope/core/agui/adapter/AguiAgentAdapterV2Test.java +++ b/agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/test/java/io/agentscope/core/agui/adapter/AguiAgentAdapterV2Test.java @@ -69,11 +69,13 @@ import io.agentscope.core.message.AssistantMessage; import io.agentscope.core.message.ContentBlock; import io.agentscope.core.message.GenerateReason; +import io.agentscope.core.message.ImageBlock; import io.agentscope.core.message.Msg; import io.agentscope.core.message.TextBlock; import io.agentscope.core.message.ToolResultBlock; import io.agentscope.core.message.ToolResultState; import io.agentscope.core.message.ToolUseBlock; +import io.agentscope.core.message.URLSource; import io.agentscope.core.model.ChatUsage; import io.agentscope.core.model.ToolSchema; import io.agentscope.core.tool.SchemaOnlyTool; @@ -875,6 +877,81 @@ void testToolResultTextAndDataDeltasAreJoinedInArrivalOrder() { assertEquals("hel\nstructuredlo", result.content()); } + @Test + void testAuthoritativeResultKeepsTheNonTextPartsFromTheStream() { + // getFinalResultText() joins text blocks only. An image the tool returned travels as a + // data delta, so treating the return value as the whole result would drop it and leave + // a + // client that renders result parts with less than the model saw. + ImageBlock image = + ImageBlock.builder() + .source(URLSource.builder().url("https://example.com/cat.png").build()) + .build(); + List events = + runReActEvents( + new ToolCallStartEvent("reply-tool", "tool-1", "lookup"), + new ToolCallEndEvent("reply-tool", "tool-1", "lookup"), + new ToolResultStartEvent("reply-tool", "tool-1", "lookup"), + new ToolResultDataDeltaEvent("reply-tool", "tool-1", "lookup", image), + new ToolResultEndEvent( + "reply-tool", "tool-1", "lookup", null, "3 rows found")); + + String content = firstToolCallResult(events).content(); + assertTrue(content.startsWith("3 rows found"), content); + assertTrue( + content.contains("https://example.com/cat.png"), + "the image part survives the switch to the return value: " + content); + } + + @Test + void testImageOnlyResultDoesNotFallBackToProgressText() { + // A tool that streams progress and returns only an image has no text in its return + // value, + // so the field is "" — reported, but empty. Reading that as "nothing reported" would + // put + // the progress chunks back into TOOL_CALL_RESULT, which is the leak this PR exists for. + ImageBlock image = + ImageBlock.builder() + .source( + URLSource.builder() + .url("https://example.com/chart.png") + .build()) + .build(); + List events = + runReActEvents( + new ToolCallStartEvent("reply-image", "tool-1", "render"), + new ToolCallEndEvent("reply-image", "tool-1", "render"), + new ToolResultStartEvent("reply-image", "tool-1", "render"), + new ToolResultTextDeltaEvent( + "reply-image", "tool-1", "render", "drawing axes"), + new ToolResultDataDeltaEvent("reply-image", "tool-1", "render", image), + new ToolResultEndEvent("reply-image", "tool-1", "render", null, "")); + + String content = firstToolCallResult(events).content(); + assertFalse( + content.contains("drawing axes"), "progress text must not leak: " + content); + assertTrue( + content.contains("https://example.com/chart.png"), + "the image is the result: " + content); + } + + @Test + void testBlankReturnValueReplacesProgressTextWithNothing() { + List events = + runReActEvents( + new ToolCallStartEvent("reply-blank", "tool-1", "cleanup"), + new ToolCallEndEvent("reply-blank", "tool-1", "cleanup"), + new ToolResultStartEvent("reply-blank", "tool-1", "cleanup"), + new ToolResultTextDeltaEvent( + "reply-blank", "tool-1", "cleanup", "removing 3 files"), + new ToolResultEndEvent("reply-blank", "tool-1", "cleanup", null, "")); + + assertEquals( + "", + firstToolCallResult(events).content(), + "an empty return value is reported as empty, not as the progress buffer"); + } + @Test void testToolResultEndWithoutContentStillEmitsNullResult() { List events = diff --git a/agentscope-service/service-dataplane/src/main/java/io/agentscope/builder/web/managed/SessionEventMapper.java b/agentscope-service/service-dataplane/src/main/java/io/agentscope/builder/web/managed/SessionEventMapper.java index 83e9a60c7e..d4adbc9fe7 100644 --- a/agentscope-service/service-dataplane/src/main/java/io/agentscope/builder/web/managed/SessionEventMapper.java +++ b/agentscope-service/service-dataplane/src/main/java/io/agentscope/builder/web/managed/SessionEventMapper.java @@ -33,6 +33,7 @@ import io.agentscope.core.event.ToolResultTextDeltaEvent; import io.agentscope.core.message.ContentBlock; import io.agentscope.core.message.TextBlock; +import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -222,7 +223,7 @@ private MappingResult mapEvent(AgentEvent event, PreviewIds previewIds) { if (fragment == null || fragment.isEmpty()) { return MappingResult.empty(); } - previewIds.appendToolResultText( + previewIds.appendToolResultData( dataDelta.getToolCallId(), dataDelta.getToolCallName(), fragment); return MappingResult.empty(); } @@ -239,7 +240,9 @@ private MappingResult mapEvent(AgentEvent event, PreviewIds previewIds) { if (toolResult.getState() != null) { payload.put("state", toolResult.getState().name()); } - String output = buf.outputText(); + String output = + buf.applyFinalResultText( + toolResult.getFinalResultText(), MAX_TOOL_PAYLOAD_CHARS); payload.put("output", output); payload.put("text", output); payload.put("content", List.of(Map.of("type", "text", "text", output))); @@ -361,6 +364,25 @@ public void appendToolResultText(String toolCallId, String toolName, String delt buf.appendOutput(delta, MAX_TOOL_PAYLOAD_CHARS); } + /** + * Accumulate a non-text result block. The fragment still feeds the buffered output exactly the + * way a text delta does — that is what gets persisted when the producer reports no return + * value — and is additionally remembered so {@link + * ToolBuffers.ToolResultBuffer#applyFinalResultText} can swap the progress text for the return + * value without dropping the block. + */ + public void appendToolResultData(String toolCallId, String toolName, String fragment) { + ToolBuffers.ToolResultBuffer buf = + toolResults.computeIfAbsent( + key(toolCallId), + ignored -> new ToolBuffers.ToolResultBuffer(newEventId(), toolName)); + if (toolName != null) { + buf.setToolName(toolName); + } + buf.addDataFragment(fragment); + buf.appendOutput(fragment, MAX_TOOL_PAYLOAD_CHARS); + } + public ToolBuffers.ToolResultBuffer finishToolResult(String toolCallId, String toolName) { ToolBuffers.ToolResultBuffer buf = toolResults.computeIfAbsent( @@ -467,6 +489,7 @@ void appendInput(String delta, int maxChars) { static final class ToolResultBuffer { private final String eventId; private final StringBuilder output = new StringBuilder(); + private final List dataFragments = new ArrayList<>(); private String toolName; private boolean truncated; private int originalOutputSize; @@ -496,6 +519,37 @@ void setToolName(String toolName) { this.toolName = toolName; } + void addDataFragment(String fragment) { + dataFragments.add(fragment); + } + + /** + * Replace the accumulated stream text with the tool's authoritative return value. + * + *

Progress chunks and the return value both arrive as text deltas on this path, so a + * buffer that reached here through a streaming tool holds progress text — which is exactly + * what the AG-UI adapter stopped persisting. {@code finalResultText} joins text blocks + * only, so the non-text blocks are re-appended after it and the shared {@code maxChars} + * cap still decides truncation for whatever ends up persisted. + * + * @param finalResultText the return value, or {@code null} when the producer reports + * nothing, which leaves the buffered text untouched as before + * @return the text to persist + */ + String applyFinalResultText(String finalResultText, int maxChars) { + if (finalResultText == null) { + return output.toString(); + } + output.setLength(0); + truncated = false; + originalOutputSize = 0; + appendOutput(finalResultText, maxChars); + for (String fragment : dataFragments) { + appendOutput(fragment, maxChars); + } + return output.toString(); + } + void appendOutput(String delta, int maxChars) { originalOutputSize += delta.length(); if (output.length() >= maxChars) { diff --git a/agentscope-service/service-dataplane/src/test/java/io/agentscope/builder/web/managed/SessionEventMapperTest.java b/agentscope-service/service-dataplane/src/test/java/io/agentscope/builder/web/managed/SessionEventMapperTest.java index d45e26ec7b..73dbeac67b 100644 --- a/agentscope-service/service-dataplane/src/test/java/io/agentscope/builder/web/managed/SessionEventMapperTest.java +++ b/agentscope-service/service-dataplane/src/test/java/io/agentscope/builder/web/managed/SessionEventMapperTest.java @@ -26,11 +26,13 @@ import io.agentscope.core.event.ToolCallDeltaEvent; import io.agentscope.core.event.ToolCallEndEvent; import io.agentscope.core.event.ToolCallStartEvent; +import io.agentscope.core.event.ToolResultDataDeltaEvent; import io.agentscope.core.event.ToolResultEndEvent; import io.agentscope.core.event.ToolResultTextDeltaEvent; import io.agentscope.core.message.GenerateReason; import io.agentscope.core.message.Msg; import io.agentscope.core.message.MsgRole; +import io.agentscope.core.message.TextBlock; import io.agentscope.core.message.ToolResultState; import java.util.Map; import org.junit.jupiter.api.BeforeEach; @@ -209,4 +211,66 @@ void toolResultAccumulatesOutputAndPersistsOnEnd() { assertThat(persisted.payload().get("text")).isEqualTo("file1\nfile2\n"); assertThat(persisted.eventId()).startsWith("evt_"); } + + @Test + void toolResultPersistsTheReturnValueRatherThanTheProgressText() { + // The AG-UI adapter already prefers ToolResultEndEvent.getFinalResultText(); this consumer + // persisted the accumulated deltas, so the two disagreed about what the tool returned. + mapper.map( + new ToolResultTextDeltaEvent("r", "tool-1", "bash", "scanning 3 dirs\n"), + previewIds); + mapper.map( + new ToolResultTextDeltaEvent("r", "tool-1", "bash", "still scanning\n"), + previewIds); + + SessionEventMapper.MappingResult end = + mapper.map( + new ToolResultEndEvent( + "r", "tool-1", "bash", ToolResultState.SUCCESS, "42 files"), + previewIds); + + Map payload = end.persisted().orElseThrow().payload(); + assertThat(payload.get("output")).isEqualTo("42 files"); + assertThat(payload.get("text")).isEqualTo("42 files"); + assertThat(String.valueOf(payload.get("content"))).doesNotContain("scanning"); + } + + @Test + void toolResultKeepsNonTextFragmentsWhenTheReturnValueArrives() { + mapper.map( + new ToolResultDataDeltaEvent( + "r", + "tool-1", + "chart", + TextBlock.builder().text("chart-part-json").build()), + previewIds); + mapper.map( + new ToolResultTextDeltaEvent("r", "tool-1", "chart", "drawing axes\n"), previewIds); + + SessionEventMapper.MappingResult end = + mapper.map( + new ToolResultEndEvent( + "r", "tool-1", "chart", ToolResultState.SUCCESS, "rendered"), + previewIds); + + String output = String.valueOf(end.persisted().orElseThrow().payload().get("output")); + assertThat(output).startsWith("rendered").contains("chart-part-json"); + assertThat(output).doesNotContain("drawing axes"); + } + + @Test + void toolResultWithoutReturnValueStillPersistsTheBufferedDeltas() { + mapper.map(new ToolResultTextDeltaEvent("r", "tool-1", "bash", "file1\n"), previewIds); + mapper.map( + new ToolResultDataDeltaEvent( + "r", "tool-1", "bash", TextBlock.builder().text("part").build()), + previewIds); + + SessionEventMapper.MappingResult end = + mapper.map( + new ToolResultEndEvent("r", "tool-1", "bash", ToolResultState.SUCCESS), + previewIds); + + assertThat(end.persisted().orElseThrow().payload().get("output")).isEqualTo("file1\npart"); + } }