fix(hook): skip auto-patch only when ToolResultBlock IDs match pendin… Fixes #42#1409
fix(hook): skip auto-patch only when ToolResultBlock IDs match pendin… Fixes #42#1409sunwg2 wants to merge 3 commits into
Conversation
|
sunwg2 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
Fixes #42 |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
AgentScopeJavaBot
added
bug
test(hook): add recovery hook test for auto-patching stale tool result IDs
|
Run 'mvn spotless:apply' to fix these violations. |
Thank you for the feedback! I will run Also, I noticed you mentioned PR #1566 which seems to address I'll wait for your guidance. Thanks! |
AgentScopeJavaBot
left a comment
AgentScopeJavaBot
left a comment
There was a problem hiding this comment.
🤖 AI Review
This PR refines PendingToolRecoveryHook to use ID-level matching when deciding whether to skip auto-patching. Previously, any ToolResultBlock present in the input would suppress auto-recovery, even if the IDs were stale or unrelated to the current pending tool calls. Now, only ToolResultBlocks whose IDs match an active pending tool call ID will inhibit auto-patch. The logic change is correct and addresses a real edge case where stale tool results could block recovery.
| * <li>Only patches when pending tool calls exist AND user input does not contain any | ||
| * {@link ToolResultBlock} whose ID matches a current pending tool call ID</li> | ||
| * <li>If the input contains {@link ToolResultBlock}s but none of their IDs match the current | ||
| * pending IDs (i.e., stale or unrelated results), a warning is logged and the input is |
There was a problem hiding this comment.
[minor] Javadoc states: 'the input is passed through unmodified -- ReActAgent's doCall will then reject the invalid IDs with an IllegalStateException'. This is incorrect. After the warning, execution falls through to the auto-patch section which clears pending state. doCall subsequently runs normally without throwing. Suggested fix: rewrite to describe the actual flow.
| return Mono.just(event); | ||
| } | ||
| // Input contains ToolResultBlocks, but none of their IDs match the current | ||
| // pending tool call IDs (stale / unrelated results). Log a warning and |
There was a problem hiding this comment.
[minor] Inline comment says 'doCall will reject the invalid IDs with an IllegalStateException, which is the correct strict-mode behavior'. This contradicts the actual flow: the code falls through to patchPendingToolCalls, which clears the pending state. Remove the misleading sentence.
|
Closing due to inactivity. Feel free to reopen when ready. |
4 participants
…g tool calls
AgentScope-Java Version
[The version of AgentScope-Java you are working on, e.g. 1.0.12, check your pom.xml dependency version or run
mvn dependency:tree | grep agentscope-parent:pom(only mac/linux)]Description
[Please describe the background, purpose, changes made, and how to test this PR]
Checklist
Please check the following items before code is ready to be reviewed.
mvn spotless:applymvn test)