fix(agent): checkpoint state on stream cancellation - #2461
Open
Niooooo wants to merge 1 commit into
Open
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
oss-maintainer
approved these changes
Jul 28, 2026
oss-maintainer
left a comment
Collaborator
There was a problem hiding this comment.
Review: PR #2461 — checkpoint state on stream cancellation
Verdict: Approve ✅
Summary
Fixes a real data-loss scenario: when streamEvents() is cancelled (e.g., SSE disconnect), the normal completion-based persistence path is bypassed, leaving intermediate agent state unrecoverable. This PR adds a cancellation-triggered checkpoint mechanism.
Changes Analyzed
| File | Assessment |
|---|---|
AgentBase.java (+103/-10) |
Core cancellation hook — captures callScope via AtomicReference, hooks into reactive cancellation signals, triggers onAgentExecutionCancelled(). Session serialization gate properly held until checkpoint completes. |
ReActAgent.java (+15) |
Clean override — delegates to saveStateToSession(), errors logged but swallowed to avoid breaking the cancellation flow. |
ReActAgentCancellationCheckpointTest.java (+416) |
Thorough coverage: normal cancel, full state recovery, missing store, persistence failure, same-session ordering, cross-session isolation. |
context.md (EN/ZH) |
Concise documentation of cancellation checkpoint behavior. |
Strengths
- Correct concurrency model — Same-session calls serialized behind the cancellation checkpoint; cross-session calls proceed independently.
- Best-effort semantics — Checkpoint failures logged but don't propagate, preventing cascade failures.
- No duplicate persistence — Normal completion path preserved; cancellation hook only fires on actual cancel signals.
- Comprehensive testing — 5 test scenarios with proper
CountDownLatchsynchronization for timing-sensitive behavior. - Clean abstraction —
onAgentExecutionCancelled(Object callScope)is a protected hook allowing other agent implementations to opt in.
Minor Observations (non-blocking)
- The
callScopeAtomicReferencepattern is correct but slightly unusual — a brief inline comment explaining the data flow would help future maintainers. - Consider whether
doFinallywithSignalType.CANCELis more idiomatic than the currentdoOnCancel+doFinallycombination.
Neither observation warrants changes. Solid implementation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AgentScope-Java Version
2.0.1-SNAPSHOT
Description
Fixes #1997.
Background
When a
streamEvents(...)subscription is cancelled, such as after an SSEdisconnect, the normal completion-based persistence path is not reached. The
live per-call
AgentStatemay therefore be cleaned up without a timelycheckpoint, making intermediate context, summary, task, plan, tool, and
subagent state unrecoverable.
Changes
cancellation signal.
CallExecutionand session persistence path so thecorrect
(userId, sessionId, agent_state)is saved.persistence terminates, preventing a newer call from loading stale state or
being overwritten by an older delayed save.
gate release safe when persistence fails.
AgentStateStoreis configured.normal completion, missing stores, persistence failures, same-session
ordering, and cross-session isolation.
context documentation.
Testing
ReActAgentCancellationCheckpointTest: 5/5 passed.DangerousPathBypassTest: 9/9 passed when rerun with the required Windowssymbolic-link privilege.
GracefulShutdownTest: 46/46 passed in isolation.clean verifycompleted successfully across all 82 reactormodules, with the Windows privilege-dependent and shared-state
order-sensitive test classes verified separately as listed above.
Checklist
Please check the following items before code is ready to be reviewed.
mvn test)