Repository navigation
fix(core): emit ExternalExecutionResultEvent when external tool results resume - #2605
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes the external-tool resume loop by persisting a correlation replyId for external-execution pauses and emitting a correlated ExternalExecutionResultEvent when externally executed ToolResultBlocks are accepted on a later call(). It also refactors the permission-HITL resume path to validate + accept confirmations in a single helper and updates v2 docs accordingly. The PR title follows the conventional commit format.
Changes:
- Persist pending-request
replyIdmetadata on the paused assistant message for both confirmation HITL and external execution, and restore it during resume. - Emit
ExternalExecutionResultEvent(correlated byreplyId) after accepting resumed external tool results, then continue reasoning. - Add a test covering the full external execution resume flow and update v2 English/Chinese docs for the revised event semantics.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/v2/zh/docs/building-blocks/tool.md | Updates external-execution description to reflect resume via ToolResultBlock and correlated ExternalExecutionResultEvent. |
| docs/v2/zh/docs/building-blocks/message-and-event.md | Documents RequireExternalExecutionEvent/ExternalExecutionResultEvent fields and correlation behavior. |
| docs/v2/zh/docs/building-blocks/agent.md | Updates external-execution resume walkthrough to mention correlated result event emission before reasoning continues. |
| docs/v2/en/docs/building-blocks/tool.md | Same as zh: clarifies resume and correlated event emission semantics. |
| docs/v2/en/docs/building-blocks/message-and-event.md | Same as zh: documents methods and correlation for external execution events. |
| docs/v2/en/docs/building-blocks/agent.md | Same as zh: updates the resume walkthrough for correlated ExternalExecutionResultEvent. |
| agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java | Adds correlation metadata helpers, refactors confirm-resume validation/acceptance, and emits ExternalExecutionResultEvent on external-result resume. |
| agentscope-core/src/main/java/io/agentscope/core/message/Msg.java | Adds metadata key for external-execution pause replyId correlation. |
| agentscope-core/src/test/java/io/agentscope/core/agent/ReActAgentNewLoopReplyTest.java | Adds coverage for external execution resume emitting a correlated ExternalExecutionResultEvent and continuing model reasoning. |
Suppressed comments (1)
agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java:2235
validateAndAddToolResultsonly clears the external-executionreplyIdmetadata whenreplyIdis non-empty. If the metadata key exists but the value is missing/invalid, the key will remain on the assistant message and can cause stale correlation on later turns. Clear the metadata key after accepting results regardless, and only guard event emission.
String replyId =
resolvePendingRequestReplyId(Msg.METADATA_EXTERNAL_EXECUTION_REQUEST_REPLY_ID);
if (!replyId.isEmpty()) {
publishEvent(new ExternalExecutionResultEvent(replyId, results));
clearPendingRequestReplyId(Msg.METADATA_EXTERNAL_EXECUTION_REQUEST_REPLY_ID);
| String replyId = resolvePendingRequestReplyId(Msg.METADATA_CONFIRM_REQUEST_REPLY_ID); | ||
| if (!replyId.isEmpty()) { | ||
| publishEvent(new UserConfirmResultEvent(replyId, normalized)); | ||
| clearPendingRequestReplyId(Msg.METADATA_CONFIRM_REQUEST_REPLY_ID); | ||
| } |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
|
||
| state.contextMutable().addAll(msgs); | ||
| String replyId = | ||
| resolvePendingRequestReplyId(Msg.METADATA_EXTERNAL_EXECUTION_REQUEST_REPLY_ID); |
There was a problem hiding this comment.
我看 resolvePendingRequestReplyId 找最后一条 Msg,但上面有 addAll 过一次,会不会找到上面 add 进去的 msg?
这是对的吗?如果是对的,为啥不在 addAll 之前滤出来?
There was a problem hiding this comment.
这里应该是顺序无关的吧。state.contextMutable().addAll(msgs) 的 msgs 来自用户;resolvePendingRequestReplyId 找的是最后一条 Assistant 消息。
oss-maintainer
left a comment
There was a problem hiding this comment.
Summary
This PR modifies 9 files with 369 lines of diff.
Observations
- Files changed: 9
- Test coverage: ✅ Tests included
- CLA: ✅ Signed
Suggestions
- Please ensure all public API changes are backward compatible
- Consider adding unit tests for new logic paths
Automated review by github-manager-bot
Summary
Fix the external tool execution resume loop by emitting
ExternalExecutionResultEventwhen externally executedToolResultBlocks are provided back toReActAgent.The agent now persists the
replyIdfromRequireExternalExecutionEventon the pending assistant message, restores that correlation when matching tool results are accepted in a latercall(), emitsExternalExecutionResultEventwith the samereplyId, and then continues reasoning.This also simplifies the confirmation resume path by combining confirmation validation and acceptance into
validateAndAcceptConfirmResults.Changes
ExternalExecutionResultEventafter accepted external tool results are added to context.RequireExternalExecutionEventToolResultBlockresumeExternalExecutionResultEventChecklist
Please check the following items before code is ready to be reviewed.
mvn spotless:applymvn test)