-
Notifications
You must be signed in to change notification settings - Fork 1k
feat(agui): propagate userId from forwardedProps to RuntimeContext #2277
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -79,6 +79,7 @@ public class AguiAgentAdapter { | |
| public static final String RUNTIME_CONTEXT_CONTEXT_KEY = "agui.context"; | ||
| public static final String RUNTIME_CONTEXT_STATE_KEY = "agui.state"; | ||
| public static final String RUNTIME_CONTEXT_FORWARDED_PROPS_KEY = "agui.forwardedProps"; | ||
| public static final String FORWARDED_PROP_USER_ID_KEY = "userId"; | ||
|
|
||
| private final Agent agent; | ||
| private final AguiAdapterConfig config; | ||
|
|
@@ -162,6 +163,7 @@ public Flux<AguiEvent> run(RunAgentInput input) { | |
| private RuntimeContext buildRuntimeContext(RunAgentInput input) { | ||
| return RuntimeContext.builder() | ||
| .sessionId(input.getThreadId()) | ||
| .userId(normalizeUserId(input.getForwardedProp(FORWARDED_PROP_USER_ID_KEY))) | ||
| .put(RunAgentInput.class, input) | ||
| .put(RUNTIME_CONTEXT_THREAD_ID_KEY, input.getThreadId()) | ||
| .put(RUNTIME_CONTEXT_RUN_ID_KEY, input.getRunId()) | ||
|
|
@@ -173,6 +175,18 @@ private RuntimeContext buildRuntimeContext(RunAgentInput input) { | |
| .build(); | ||
| } | ||
|
|
||
| private static String normalizeUserId(Object value) { | ||
| if (value == null) { | ||
| return null; | ||
| } | ||
| String userId = value.toString(); | ||
| if (userId == null) { | ||
| return null; | ||
|
Comment on lines
+183
to
+184
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] Dead code: private static String normalizeUserId(Object value) {
if (value == null) {
return null;
}
String userId = value.toString().trim();
return userId.isEmpty() ? null : userId;
}
Comment on lines
+183
to
+184
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] Dead code: private static String normalizeUserId(Object value) {
if (value == null) {
return null;
}
String userId = value.toString().trim();
return userId.isEmpty() ? null : userId;
} |
||
| } | ||
| userId = userId.trim(); | ||
| return userId.isEmpty() ? null : userId; | ||
| } | ||
|
|
||
| private ToolInjection injectFrontendTools(RunAgentInput input) { | ||
| if (!input.hasTools()) { | ||
| return ToolInjection.empty(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,6 +70,71 @@ void setUp() { | |
| adapter = new AguiAgentAdapter(mockAgent, AguiAdapterConfig.defaultConfig()); | ||
| } | ||
|
|
||
| @Test | ||
| void testRunSetsUserIdFromForwardedPropsIntoRuntimeContext() { | ||
| ArgumentCaptor<RuntimeContext> contextCaptor = | ||
| ArgumentCaptor.forClass(RuntimeContext.class); | ||
| when(mockAgent.stream(anyList(), any(StreamOptions.class), contextCaptor.capture())) | ||
| .thenReturn(Flux.empty()); | ||
|
|
||
| RunAgentInput input = | ||
| RunAgentInput.builder() | ||
| .threadId("thread-user") | ||
| .runId("run-user") | ||
| .messages(List.of(AguiMessage.userMessage("msg-1", "Hello"))) | ||
| .forwardedProps( | ||
| Map.of(AguiAgentAdapter.FORWARDED_PROP_USER_ID_KEY, "user-123")) | ||
| .build(); | ||
|
|
||
| adapter.run(input).collectList().block(); | ||
|
|
||
| RuntimeContext context = contextCaptor.getValue(); | ||
| assertEquals("user-123", context.getUserId()); | ||
| assertSame(input.getForwardedProps(), context.get("agui.forwardedProps")); | ||
| } | ||
|
|
||
| @Test | ||
| void testRunConvertsNonStringUserIdFromForwardedProps() { | ||
| ArgumentCaptor<RuntimeContext> contextCaptor = | ||
| ArgumentCaptor.forClass(RuntimeContext.class); | ||
| when(mockAgent.stream(anyList(), any(StreamOptions.class), contextCaptor.capture())) | ||
| .thenReturn(Flux.empty()); | ||
|
|
||
| RunAgentInput input = | ||
| RunAgentInput.builder() | ||
| .threadId("thread-user-long") | ||
| .runId("run-user-long") | ||
| .messages(List.of(AguiMessage.userMessage("msg-1", "Hello"))) | ||
| .forwardedProps(Map.of(AguiAgentAdapter.FORWARDED_PROP_USER_ID_KEY, 12345L)) | ||
| .build(); | ||
|
|
||
| adapter.run(input).collectList().block(); | ||
|
|
||
| RuntimeContext context = contextCaptor.getValue(); | ||
| assertEquals("12345", context.getUserId()); | ||
| } | ||
|
|
||
| @Test | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] The @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.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] The @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. |
||
| void testRunKeepsUserIdNullWhenForwardedPropsMissingUserId() { | ||
| ArgumentCaptor<RuntimeContext> contextCaptor = | ||
| ArgumentCaptor.forClass(RuntimeContext.class); | ||
| when(mockAgent.stream(anyList(), any(StreamOptions.class), contextCaptor.capture())) | ||
| .thenReturn(Flux.empty()); | ||
|
|
||
| RunAgentInput input = | ||
| RunAgentInput.builder() | ||
| .threadId("thread-no-user") | ||
| .runId("run-no-user") | ||
| .messages(List.of(AguiMessage.userMessage("msg-1", "Hello"))) | ||
| .forwardedProps(Map.of("tenant", "demo")) | ||
| .build(); | ||
|
|
||
| adapter.run(input).collectList().block(); | ||
|
|
||
| RuntimeContext context = contextCaptor.getValue(); | ||
| assertNull(context.getUserId()); | ||
| } | ||
|
|
||
| @Test | ||
| void testRunInjectsRunInputIntoRuntimeContext() { | ||
| ArgumentCaptor<RuntimeContext> contextCaptor = | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这样直接获取是否会存在身份伪造的风险?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
您说得很有道理,我完全认同RunAgentInput的参数确实需要校验,因为它们直接来自用户。不过我在想,是否可以把校验逻辑放在adapter之外的其他层来处理,这样职责会更清晰一些。
另外关于userId,我也觉得放在后台接口层从header或其他上下文中获取,再手动设置到RunAgentInput中,可能比让前端直接传递更合适,也更安全。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jujn
I'd be happy to. I'll continue to follow up in #2044.