fix(agui): Suppress ReActAgent handshake events in AG-UI converters - #2639
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the AG-UI adapter’s default event-converter registry to explicitly suppress ReActAgent “handshake” events (permission confirm results and external-execution request/result events) so they don’t fall through to RAW events in the AG-UI stream.
Changes:
- Register
UserConfirmResultEventas a no-op inPermissionConfirmEventConverterto prevent default RAW emission. - Introduce
ExternalExecutionEventConverterto no-opRequireExternalExecutionEventandExternalExecutionResultEvent. - Extend test coverage to assert suppression behavior and confirm custom converters can still override the suppressed defaults.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/test/java/io/agentscope/core/agui/adapter/AguiAgentAdapterV2Test.java | Adds tests verifying handshake events are suppressed and that custom converters can override suppressed defaults. |
| agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/main/java/io/agentscope/core/agui/adapter/strategy/PermissionConfirmEventConverter.java | Registers UserConfirmResultEvent and no-ops it to avoid RAW fallback. |
| agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/main/java/io/agentscope/core/agui/adapter/strategy/ExternalExecutionEventConverter.java | Adds a dedicated no-op converter for external execution handshake events. |
| agentscope-extensions/agentscope-extensions-protocol/agentscope-extensions-agui/src/main/java/io/agentscope/core/agui/adapter/strategy/AgentEventConverterRegistry.java | Registers the new external execution converter in the built-in converter set. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
AgentScopeJavaBot
added
bug
AgentScopeJavaBot
left a comment
AgentScopeJavaBot
left a comment
There was a problem hiding this comment.
🤖 AI Review
This PR suppresses ReActAgent external-execution handshake events (RequireExternalExecutionEvent, ExternalExecutionResultEvent) and UserConfirmResultEvent from the default AG-UI event stream. Without this change, these events fall through to the RawAgentEventConverter and leak as RAW events — duplicating information already surfaced through the native RUN_FINISHED.outcome interrupt contract. The fix introduces a new no-op ExternalExecutionEventConverter and extends PermissionConfirmEventConverter to also absorb UserConfirmResultEvent. The approach is consistent with the existing converter architecture, well-tested, and correctly handles the custom-converter-override path.
| * {@code ToolUseBlock}, and tool-input validation reads {@code content} directly with no fallback to | ||
| * {@code input}; a null content would fail the resume with {@code argument "content" is null}. | ||
| * | ||
| * <p>{@link UserConfirmResultEvent} is intentionally registered here as a no-op.. |
There was a problem hiding this comment.
[nitpick] Double period typo in Javadoc: "is intentionally registered here as a no-op.." — remove the trailing extra ..
AgentScopeJavaBot
left a comment
AgentScopeJavaBot
left a comment
There was a problem hiding this comment.
🤖 AI Review
This PR suppresses ReActAgent external-execution handshake events (RequireExternalExecutionEvent, ExternalExecutionResultEvent) and UserConfirmResultEvent from the default AG-UI event stream. Without this change, these events fall through to the RawAgentEventConverter and leak as RAW events — duplicating information already surfaced through the native RUN_FINISHED.outcome interrupt contract. The fix introduces a new no-op ExternalExecutionEventConverter and extends PermissionConfirmEventConverter to also absorb UserConfirmResultEvent. The approach is consistent with the existing converter architecture, well-tested, and correctly handles the custom-converter-override path.
| * {@code ToolUseBlock}, and tool-input validation reads {@code content} directly with no fallback to | ||
| * {@code input}; a null content would fail the resume with {@code argument "content" is null}. | ||
| * | ||
| * <p>{@link UserConfirmResultEvent} is intentionally registered here as a no-op.. |
There was a problem hiding this comment.
[nitpick] Double period typo in Javadoc: "is intentionally registered here as a no-op.." — remove the trailing extra ..
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Summary
This PR suppresses ReActAgent handshake events from the default AG-UI event stream so they do not fall through to
RAWevents.Changes include:
UserConfirmResultEventinPermissionConfirmEventConverteras a no-op.ExternalExecutionEventConverterto no-opRequireExternalExecutionEventandExternalExecutionResultEvent.Checklist
Please check the following items before code is ready to be reviewed.
mvn spotless:applymvn test)