fix(tool): mark ToolResultBlock.error() results with state=ERROR by logicwu0 · Pull Request #2186 · agentscope-ai/agentscope-java

logicwu0 · 2026-07-14T02:23:58Z

What

ToolResultBlock.error() returned a result whose state was left unset (the
constructor defaults it to RUNNING) with an "Error: " text prefix. But
ReActAgent.determineToolResultState() only classifies a result as ERROR
when either the structured state is a non-RUNNING value, or the output text
starts with "[ERROR]". Neither held, so failed tool calls fell through to the
default SUCCESS branch — the agent never saw the failure.

This affected every error() call site: MCP timeouts, tool-not-found, empty
results, and generic execution failures.

Reproduced in the issue with an MCP tool timing out after 3 retries:

WARN  Retrying tool call 'maps_weather' (attempt 2/2) due to: Tool execution timeout after PT30S
INFO  POST_ACTING | name=maps_weather, result_len=96, state=SUCCESS   <-- should be ERROR

Fix

Set state = ToolResultState.ERROR in ToolResultBlock.error(), so failures are
classified by structured state, decoupled from the fragile text-prefix heuristic.
This fixes all error paths at once. The output text ("Error: ...") is
unchanged, so existing behavior and assertions are preserved.

Tests

Extended shouldReturnErrorWhenToolThrows to assert the failing-tool response
carries state == ERROR.
Added errorResultCarriesErrorState as a focused regression guard.
Full agentscope-core suite: 2210 passed, 0 failed (9 skipped).

Closes #2157

ToolResultBlock.error() left state unset (defaulting to RUNNING) and produced an "Error: " text prefix. determineToolResultState() only recognizes a non-RUNNING state or an "[ERROR]" text prefix, so failed tool calls (MCP timeouts, tool-not-found, execution errors, ...) fell through to SUCCESS and the agent never saw the failure. Set state=ERROR in error() so failures are classified by structured state, independent of the output text. Fixes all error() call sites at once; the output text is unchanged. Closes agentscope-ai#2157

codecov · 2026-07-14T02:37:07Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

oss-maintainer

Summary

Critical bug fix — ToolResultBlock.error() was leaving state=null (defaulting to RUNNING), causing determineToolResultState() to misclassify failures as SUCCESS.

Findings

[Critical] ToolResultBlock.java:207 — Now explicitly sets state = ToolResultState.ERROR. This is the correct fix.
[Info] Test errorResultCarriesErrorState() added as regression guard. Good.

Verdict

Essential fix for tool error handling. LGTM.

Automated review by github-manager

AgentScopeJavaBot

🤖 AI Review

This is a clean, well-targeted bug fix for issue #2157. The root cause was a mismatch between ToolResultBlock.error() (which produced "Error: " text prefix with state defaulting to RUNNING) and ReActAgent.determineToolResultState() (which checks for non-RUNNING state or "[ERROR]" text prefix). Neither condition matched, so all tool failures — MCP timeouts, tool-not-found, execution errors — were silently classified as SUCCESS. The fix correctly sets state = ToolResultState.ERROR in the factory method, decoupling error identification from fragile text-prefix heuristics. The change is minimal (+21/-2), backward-compatible (text output unchanged), and includes both an extended integration assertion and a focused regression test. Full test suite reportedly green (2210 passed, 0 failed).

One minor follow-up consideration (outside this PR's scope): ReActAgent.buildErrorToolResult() (line ~1753) still constructs error results via the builder without setting state, relying on the "[ERROR]" text prefix heuristic. Applying the same structured-state approach there would improve consistency and allow eventual deprecation of the text-prefix fallback.

AgentScopeJavaBot · 2026-07-14T10:15:33Z

+        // determineToolResultState() misclassified failures as SUCCESS.
+        ToolResultBlock result = ToolResultBlock.error("boom");
+        assertEquals(ToolResultState.ERROR, result.getState());
    }


[praise] Excellent regression guard with a clear historical comment explaining why the bug existed (state defaulted to RUNNING, text prefix was "Error: " not "[ERROR]"). This level of context in a test comment is exactly what future maintainers need to understand the intent without digging through git history.

AgentScopeJavaBot · 2026-07-14T10:15:33Z

-     * @return ToolResultBlock with error output
+     * @return ToolResultBlock with error output and {@code state = ERROR}
     */
    public static ToolResultBlock error(String errorMessage) {


[praise] Clean fix — setting the structured state field directly in the factory method is the right approach. It fixes all error call sites (MCP timeouts, tool-not-found, generic failures) in one place and removes the dependency on the brittle "[ERROR]" text-prefix heuristic in determineToolResultState().

AgentScopeJavaBot

🤖 AI Review

This is a clean, well-targeted bug fix for issue #2157. The root cause was a mismatch between ToolResultBlock.error() (which produced "Error: " text prefix with state defaulting to RUNNING) and ReActAgent.determineToolResultState() (which checks for non-RUNNING state or "[ERROR]" text prefix). Neither condition matched, so all tool failures — MCP timeouts, tool-not-found, execution errors — were silently classified as SUCCESS. The fix correctly sets state = ToolResultState.ERROR in the factory method, decoupling error identification from fragile text-prefix heuristics. The change is minimal (+21/-2), backward-compatible (text output unchanged), and includes both an extended integration assertion and a focused regression test. Full test suite reportedly green (2210 passed, 0 failed).

One minor follow-up consideration (outside this PR's scope): ReActAgent.buildErrorToolResult() (line ~1753) still constructs error results via the builder without setting state, relying on the "[ERROR]" text prefix heuristic. Applying the same structured-state approach there would improve consistency and allow eventual deprecation of the text-prefix fallback.

AgentScopeJavaBot · 2026-07-14T10:16:56Z

+        // determineToolResultState() misclassified failures as SUCCESS.
+        ToolResultBlock result = ToolResultBlock.error("boom");
+        assertEquals(ToolResultState.ERROR, result.getState());
    }


[praise] Excellent regression guard with a clear historical comment explaining why the bug existed (state defaulted to RUNNING, text prefix was "Error: " not "[ERROR]"). This level of context in a test comment is exactly what future maintainers need to understand the intent without digging through git history.

AgentScopeJavaBot · 2026-07-14T10:16:56Z

-     * @return ToolResultBlock with error output
+     * @return ToolResultBlock with error output and {@code state = ERROR}
     */
    public static ToolResultBlock error(String errorMessage) {


[praise] Clean fix — setting the structured state field directly in the factory method is the right approach. It fixes all error call sites (MCP timeouts, tool-not-found, generic failures) in one place and removes the dependency on the brittle "[ERROR]" text-prefix heuristic in determineToolResultState().

logicwu0 requested a review from a team July 14, 2026 02:23

logicwu0 mentioned this pull request Jul 14, 2026

[Bug]:ToolResultBlock.error() 生成的错误前缀与 determineToolResultState() 检查的前缀不一致，导致工具超时/失败结果被标记为 SUCCESS #2157

Open

oss-maintainer approved these changes Jul 14, 2026

View reviewed changes

This was referenced Jul 14, 2026

fix toolResult Prefix Check #2174

Open

ToolResultBlock.error() can be emitted as ToolResultState.SUCCESS #2111

Open

AgentScopeJavaBot added bug Something isn't working area/core/tool Tool, skill, RAG abstractions labels Jul 14, 2026

AgentScopeJavaBot approved these changes Jul 14, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(tool): mark ToolResultBlock.error() results with state=ERROR#2186

fix(tool): mark ToolResultBlock.error() results with state=ERROR#2186
logicwu0 wants to merge 1 commit into
agentscope-ai:mainfrom
logicwu0:fix/2157-tool-error-state

logicwu0 commented Jul 14, 2026

Uh oh!

codecov Bot commented Jul 14, 2026

Uh oh!

oss-maintainer left a comment

Uh oh!

AgentScopeJavaBot left a comment

Uh oh!

AgentScopeJavaBot Jul 14, 2026

Uh oh!

AgentScopeJavaBot Jul 14, 2026

Uh oh!

AgentScopeJavaBot left a comment

Uh oh!

AgentScopeJavaBot Jul 14, 2026

Uh oh!

AgentScopeJavaBot Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

logicwu0 commented Jul 14, 2026

What

Fix

Tests

Uh oh!

codecov Bot commented Jul 14, 2026

Codecov Report

Uh oh!

oss-maintainer left a comment

Choose a reason for hiding this comment

Summary

Findings

Verdict

Uh oh!

AgentScopeJavaBot left a comment

Choose a reason for hiding this comment

Uh oh!

AgentScopeJavaBot Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

AgentScopeJavaBot Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

AgentScopeJavaBot left a comment

Choose a reason for hiding this comment

Uh oh!

AgentScopeJavaBot Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

AgentScopeJavaBot Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants