fix(core): 异常终止时保存 ReActAgent 状态

[hanydd]

AgentScope-Java Version

2.0.3-SNAPSHOT

Description

背景

ReActAgent 目前只在正常结束和用户 interrupt 后保存状态。模型、工具或 middleware 报错，以及流被取消时，本轮已经写入内存的用户消息和已完成上下文不会保存到 AgentStateStore，Agent 重建后会回退到旧状态。

另外，如果 reasoning 已经产生 tool call，但在 acting 写入结果前异常结束，直接保存会留下没有对应结果的 PENDING tool call，影响下一轮恢复。应用层因此需要额外实现异常保存和状态修复逻辑。

改动

• 在普通和 structured-output 执行报错或取消时，尽力保存当前已提交的 AgentState，且不覆盖原始异常。
• 保存前只移除本轮没有结果的 PENDING tool call，保留文本、thinking、ALLOWED 和 ASKING 状态。
• 复用现有状态保存和冲突处理逻辑，保持原有 interrupt 流程不变。
• 补充普通及 structured-output 报错、取消、未完成 tool call 和保存失败场景的测试。

测试

mvn -pl agentscope-core test

2298 tests passed，8 skipped。

Checklist

• Code has been formatted with mvn spotless:apply
• All tests are passing (mvn test)
• Javadoc comments are complete and follow project conventions
• Related documentation has been updated (no documentation change required)
• Code is ready for review

[codecov]

Codecov Report

❌ Patch coverage is 71.66667% with 17 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...e/src/main/java/io/agentscope/core/ReActAgent.java	71.66%	10 Missing and 7 partials ⚠️

📢 Thoughts on this report? Let us know!

[AgentScopeJavaBot]

🤖 AI Review

This PR adds best-effort state persistence when ReActAgent terminates abnormally — via unhandled errors or Reactor subscription cancellation. The refactoring of saveStateToSession into a reusable persistState() is clean, and the new dropUncommittedToolCallsFromCurrentCall() correctly bounds its work to the current call's context window (using loadedContextSize). The InterruptedException exclusion is correct since the existing interrupt path already handles state repair. Tests cover the key scenarios well.

One notable symmetry gap exists: the structured-output call paths (doNativeStructuredCall, doFallbackStructuredCall) also invoke scope.doCallInner(msgs) but were not updated with the same doOnCancel / onErrorResume operators, leaving them vulnerable to the same state-loss problem this PR fixes.

[AgentScopeJavaBot]

[major] Symmetry gap: The error/cancellation state-persistence operators (doOnCancel + onErrorResume) are only applied in this doCall(List<Msg>) path. The structured-output paths — doNativeStructuredCall (~line 1310) and doFallbackStructuredCall (~line 1350) — also call scope.doCallInner(msgs) but lack equivalent protection. If a structured-output call errors or gets cancelled, state will be lost — the exact same problem this PR fixes.

Consider applying the same operators to both structured-output paths, e.g.:

return scope.doCallInner(msgs)
        .doOnCancel(() -> checkpointAbnormalState(scope))
        .onErrorResume(error -> saveStateAfterError(scope, error))
        .flatMap(result -> { /* existing logic */ });

[AgentScopeJavaBot]

✅ Verified as addressed in 1770366: doStructuredCall now wraps both native and fallback execution paths with checkpointOnAbnormalTermination (line 1282), applying doOnCancel + onErrorResume symmetrically.

[AgentScopeJavaBot]

[nit] Minor: when error.getCause() is null, null instanceof InterruptedException safely evaluates to false, so this is correct as-is. However, for clarity, an explicit null-check could make the intent more obvious to readers:

if (error instanceof InterruptedException
        || (error.getCause() != null && error.getCause() instanceof InterruptedException)) {

This is purely a readability nit — no functional issue.

[AgentScopeJavaBot]

[praise] Good defensive design: wrapping both dropUncommittedToolCallsFromCurrentCall() and persistState() in separate try-catch blocks ensures that a failure in state repair doesn't prevent the best-effort save, and a save failure is properly logged without masking the original error.

[AgentScopeJavaBot]

✅ Verified as addressed in 1770366: checkpointAbnormalState (lines 519-531) still uses two separate try-catch blocks for repair and save as praised.

[AgentScopeJavaBot]

[praise] Clean implementation: iterating backward from the end of context and bounding by loadedContextSize ensures only current-call PENDING tool calls are dropped, preserving historical context integrity. The early return when retained.size() == message.getContent().size() is a nice short-circuit optimization.

[AgentScopeJavaBot]

✅ Verified as addressed in 1770366: dropUncommittedToolCallsFromCurrentCall (lines 2125-2158) retains the praised backward iteration, loadedContextSize bound, and early-return optimization.

[AgentScopeJavaBot]

🤖 AI Review

This PR adds best-effort state persistence when ReActAgent terminates abnormally — via unhandled errors or Reactor subscription cancellation. The refactoring of saveStateToSession into a reusable persistState() is clean, and the new dropUncommittedToolCallsFromCurrentCall() correctly bounds its work to the current call's context window (using loadedContextSize). The InterruptedException exclusion is correct since the existing interrupt path already handles state repair. Tests cover the key scenarios well.

One notable symmetry gap exists: the structured-output call paths (doNativeStructuredCall, doFallbackStructuredCall) also invoke scope.doCallInner(msgs) but were not updated with the same doOnCancel / onErrorResume operators, leaving them vulnerable to the same state-loss problem this PR fixes.

[AgentScopeJavaBot]

[major] Symmetry gap: The error/cancellation state-persistence operators (doOnCancel + onErrorResume) are only applied in this doCall(List<Msg>) path. The structured-output paths — doNativeStructuredCall (~line 1310) and doFallbackStructuredCall (~line 1350) — also call scope.doCallInner(msgs) but lack equivalent protection. If a structured-output call errors or gets cancelled, state will be lost — the exact same problem this PR fixes.

Consider applying the same operators to both structured-output paths, e.g.:

return scope.doCallInner(msgs)
        .doOnCancel(() -> checkpointAbnormalState(scope))
        .onErrorResume(error -> saveStateAfterError(scope, error))
        .flatMap(result -> { /* existing logic */ });

[AgentScopeJavaBot]

✅ Verified as addressed in 1770366: doStructuredCall now wraps both native and fallback execution paths with checkpointOnAbnormalTermination (line 1282), applying doOnCancel + onErrorResume symmetrically.

[AgentScopeJavaBot]

[nit] Minor: when error.getCause() is null, null instanceof InterruptedException safely evaluates to false, so this is correct as-is. However, for clarity, an explicit null-check could make the intent more obvious to readers:

if (error instanceof InterruptedException
        || (error.getCause() != null && error.getCause() instanceof InterruptedException)) {

This is purely a readability nit — no functional issue.

[AgentScopeJavaBot]

[praise] Good defensive design: wrapping both dropUncommittedToolCallsFromCurrentCall() and persistState() in separate try-catch blocks ensures that a failure in state repair doesn't prevent the best-effort save, and a save failure is properly logged without masking the original error.

[AgentScopeJavaBot]

✅ Verified as addressed in 1770366: checkpointAbnormalState (lines 519-531) still uses two separate try-catch blocks for repair and save as praised.

[AgentScopeJavaBot]

[praise] Clean implementation: iterating backward from the end of context and bounding by loadedContextSize ensures only current-call PENDING tool calls are dropped, preserving historical context integrity. The early return when retained.size() == message.getContent().size() is a nice short-circuit optimization.

[AgentScopeJavaBot]

✅ Verified as addressed in 1770366: dropUncommittedToolCallsFromCurrentCall (lines 2125-2158) retains the praised backward iteration, loadedContextSize bound, and early-return optimization.

[AgentScopeJavaBot]

All blocking review comments have been addressed. 2 non-blocking suggestion(s) (minor/nit) remain open and may be addressed in a follow-up.

[hanydd]

Closing in favor of #2820. After #2799 merged the error-persistence behavior, #2820 narrows the follow-up to Reactor cancellation and removing current-call PENDING tool calls before an abnormal save.