feat(agui): propagate userId from forwardedProps to RuntimeContext#2277
feat(agui): propagate userId from forwardedProps to RuntimeContext#2277jujn wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
AgentScopeJavaBot
left a comment
There was a problem hiding this comment.
🤖 AI Review
This is a clean, well-scoped change that propagates the userId field from AG-UI protocol's forwardedProps into RuntimeContext, enabling downstream agent code to identify the calling user. The implementation follows existing patterns in buildRuntimeContext, includes a defensive normalizeUserId helper that handles null/non-string/blank values, and ships with three focused unit tests covering the happy path, non-string type coercion, and missing-key scenarios. The code is correct and ready to merge with two minor nits below.
| if (userId == null) { | ||
| return null; |
There was a problem hiding this comment.
[nit] Dead code: Object.toString() is guaranteed by the Java Language Specification to never return null, so the if (userId == null) branch is unreachable. You can safely remove lines 183-185 and simplify to:
private static String normalizeUserId(Object value) {
if (value == null) {
return null;
}
String userId = value.toString().trim();
return userId.isEmpty() ? null : userId;
}| assertEquals("12345", context.getUserId()); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
[nit] The normalizeUserId method has explicit trim + empty-check logic, but no test exercises the blank/whitespace-only userId edge cases (e.g. "" or " "). Consider adding a test like:
@Test
void testRunNormalizesBlankUserIdToNull() {
// ... setup ...
RunAgentInput input = RunAgentInput.builder()
.threadId("t").runId("r")
.messages(List.of(AguiMessage.userMessage("m", "hi")))
.forwardedProps(Map.of(AguiAgentAdapter.FORWARDED_PROP_USER_ID_KEY, " "))
.build();
adapter.run(input).collectList().block();
assertNull(contextCaptor.getValue().getUserId());
}This would give full branch coverage for the normalization helper.
AgentScopeJavaBot
left a comment
There was a problem hiding this comment.
🤖 AI Review
This is a clean, well-scoped change that propagates the userId field from AG-UI protocol's forwardedProps into RuntimeContext, enabling downstream agent code to identify the calling user. The implementation follows existing patterns in buildRuntimeContext, includes a defensive normalizeUserId helper that handles null/non-string/blank values, and ships with three focused unit tests covering the happy path, non-string type coercion, and missing-key scenarios. The code is correct and ready to merge with two minor nits below.
| if (userId == null) { | ||
| return null; |
There was a problem hiding this comment.
[nit] Dead code: Object.toString() is guaranteed by the Java Language Specification to never return null, so the if (userId == null) branch is unreachable. You can safely remove lines 183-185 and simplify to:
private static String normalizeUserId(Object value) {
if (value == null) {
return null;
}
String userId = value.toString().trim();
return userId.isEmpty() ? null : userId;
}| assertEquals("12345", context.getUserId()); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
[nit] The normalizeUserId method has explicit trim + empty-check logic, but no test exercises the blank/whitespace-only userId edge cases (e.g. "" or " "). Consider adding a test like:
@Test
void testRunNormalizesBlankUserIdToNull() {
// ... setup ...
RunAgentInput input = RunAgentInput.builder()
.threadId("t").runId("r")
.messages(List.of(AguiMessage.userMessage("m", "hi")))
.forwardedProps(Map.of(AguiAgentAdapter.FORWARDED_PROP_USER_ID_KEY, " "))
.build();
adapter.run(input).collectList().block();
assertNull(contextCaptor.getValue().getUserId());
}This would give full branch coverage for the normalization helper.
|
@copilot review |
| private RuntimeContext buildRuntimeContext(RunAgentInput input) { | ||
| return RuntimeContext.builder() | ||
| .sessionId(input.getThreadId()) | ||
| .userId(normalizeUserId(input.getForwardedProp(FORWARDED_PROP_USER_ID_KEY))) |
There was a problem hiding this comment.
您说得很有道理,我完全认同RunAgentInput的参数确实需要校验,因为它们直接来自用户。不过我在想,是否可以把校验逻辑放在adapter之外的其他层来处理,这样职责会更清晰一些。
另外关于userId,我也觉得放在后台接口层从header或其他上下文中获取,再手动设置到RunAgentInput中,可能比让前端直接传递更合适,也更安全。
There was a problem hiding this comment.
@ningmeng0503
Can you take charge of this? You can wait until the event mechanism is upgraded to 2.0. I originally wanted to add a submission to your PR, but it can't be edited. You can continue in: #2044
oss-maintainer
left a comment
There was a problem hiding this comment.
Summary
This PR contains 115 lines of changes. Review in progress.
Automated review by github-manager-bot
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)