feat(HITL): Emit UserConfirmResultEvent when resuming permission HITL - #2511
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! |
d41ced1 to
ca2d354
Compare
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
zouyx
left a comment
zouyx
left a comment
There was a problem hiding this comment.
LGTM. Reviewed confirmation validation, replyId correlation, and partial-confirmation behavior. ReActAgentHitlTest passes locally; CI is green.
zouyx
left a comment
zouyx
left a comment
There was a problem hiding this comment.
Documentation requirement: this PR changes the observable streamEvents contract by emitting UserConfirmResultEvent on resume and correlating it with the earlier RequireUserConfirmEvent replyId, but it contains no docs/ update. The current English and Chinese message-and-event docs still call UserConfirmResultEvent an input event and do not document getReplyId correlation. Please update both language versions, ideally alongside the HITL flow documentation, and add this behavior to the PR checklist before merging.
| } | ||
| normalized.add(result); | ||
| } | ||
| return List.copyOf(normalized); |
| * correlation metadata there lets the next call recover it from session state. | ||
| */ | ||
| private void persistPendingConfirmRequest(String replyId) { | ||
| Msg lastAssistant = findLastAssistantMsg(); |
There was a problem hiding this comment.
这个是在 actingStream 里面调的，只能重新 findLastAssistantMsg 一次了
| * HITL request. | ||
| */ | ||
| private void clearPendingConfirmRequest() { | ||
| Msg lastAssistant = findLastAssistantMsg(); |
There was a problem hiding this comment.
甚至我看不太懂，这个方法和上面方法的区别是什么？多了个 if？
| return ""; | ||
| } | ||
| Object raw = confirmRequestMsg.getMetadata().get(Msg.METADATA_CONFIRM_REQUEST_REPLY_ID); | ||
| return raw instanceof String s ? s : ""; |
There was a problem hiding this comment.
Non-blocking: when resuming a persisted ASKING state created before this change, METADATA_CONFIRM_REQUEST_REPLY_ID will be absent, so UserConfirmResultEvent#getReplyId() returns an empty string. The resume itself still works, and the original reply ID cannot be recovered, so this behavior seems reasonable. However, the documentation currently guarantees that the ID always matches the earlier RequireUserConfirmEvent. Could we document that replyId may be empty for legacy or manually restored states that do not contain the new correlation metadata?
There was a problem hiding this comment.
可以先 approve 吗？因为明天可能要发版，其它的PR需要等这个合入
Summary
UserConfirmResultEventwhenReActAgentacceptsConfirmResultpayloads while resuming permission HITL.replyIdon the live assistant message so the result event can correlate with the earlierRequireUserConfirmEvent.ConfirmResults fully cover all currentASKINGtool calls before emitting the event or mutating context.Checklist
Please check the following items before code is ready to be reviewed.
mvn spotless:applymvn test)