-
Notifications
You must be signed in to change notification settings - Fork 15
Fix GetCallerBaggagePairs: resolve userId across all channels #251
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
base: main
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -83,6 +83,65 @@ def test_get_channel_pairs(): | |
| assert (CHANNEL_LINK_KEY, None) in result | ||
|
|
||
|
|
||
| def test_get_caller_pairs_non_teams_fallback_to_from_id(): | ||
| """Test userId falls back to from.id when aad_object_id is None (non-Teams channel).""" | ||
| from_account = ChannelAccount( | ||
| id="from-id-123", | ||
| name="Non-Teams User", | ||
| ) | ||
| activity = Activity(type="message", from_property=from_account) | ||
|
|
||
| result = list(get_caller_pairs(activity)) | ||
|
|
||
| assert (USER_ID_KEY, "from-id-123") in result | ||
| assert (USER_NAME_KEY, "Non-Teams User") in result | ||
| assert (USER_EMAIL_KEY, None) in result | ||
|
|
||
|
|
||
| def test_get_caller_pairs_a2a_fallback_to_agentic_user_id(): | ||
| """Test userId falls back to agentic_user_id for A2A calls (no aad_object_id).""" | ||
| from_account = ChannelAccount( | ||
| id="from-id-456", | ||
| name="Agent Caller", | ||
| agentic_user_id="a2a-agent-guid", | ||
|
Contributor
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. @fpfp100 review this
Author
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. The test is correct. In an A2A scenario, agentic_user_id is populated and aad_object_id is null — that is exactly what the test sets up. For human callers on non-Teams channels, agentic_user_id is null, so the fallback skips it and goes to frm.id (covered by test_get_caller_pairs_non_teams_fallback_to_from_id). The test tests the path the case when the presence of agentic_user_id but no aad_object_id. As for agentic_user_id is being used to set user email, for real user, agentic_user_id is null, user.email will not be set. It can be set manually. TurnContext has no éxplicit field for user email. For agentic user(agent) using host agent, agentic_user_id is upn. How to set user email is out of scope of this bug fix PR> |
||
| ) | ||
| activity = Activity(type="message", from_property=from_account) | ||
|
|
||
| result = list(get_caller_pairs(activity)) | ||
|
|
||
| assert (USER_ID_KEY, "a2a-agent-guid") in result | ||
| assert (USER_EMAIL_KEY, "a2a-agent-guid") in result | ||
|
|
||
|
|
||
| def test_get_caller_pairs_aad_object_id_wins_when_all_set(): | ||
| """Test aad_object_id takes precedence when all identifiers are present.""" | ||
| from_account = ChannelAccount( | ||
| id="from-id-789", | ||
| aad_object_id="aad-wins", | ||
| name="Full User", | ||
| agentic_user_id="agent-upn", | ||
| ) | ||
| activity = Activity(type="message", from_property=from_account) | ||
|
|
||
| result = list(get_caller_pairs(activity)) | ||
|
|
||
| assert (USER_ID_KEY, "aad-wins") in result | ||
| assert (USER_NAME_KEY, "Full User") in result | ||
| assert (USER_EMAIL_KEY, "agent-upn") in result | ||
|
|
||
|
|
||
| def test_get_caller_pairs_a2a_guid_agentic_user_id(): | ||
| """Test userId resolves to GUID AgenticUserId in A2A scenario.""" | ||
| from_account = ChannelAccount( | ||
| id="29:1sH5NArUwkWAX", | ||
| name="Agent Caller", | ||
| agentic_user_id="bef730f4-d6f5-4ffb-b759-26ffa449ed7e", | ||
| ) | ||
| activity = Activity(type="message", from_property=from_account) | ||
| result = list(get_caller_pairs(activity)) | ||
| assert (USER_ID_KEY, "bef730f4-d6f5-4ffb-b759-26ffa449ed7e") in result | ||
|
|
||
|
|
||
| def test_get_conversation_pairs(): | ||
| """Test get_conversation_pairs extracts conversation information.""" | ||
| conversation = ConversationAccount(id="conversation-123") | ||
|
|
||
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.
@fpfp100 review this
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.
It is not breaking. since for human call, agentic user id is null. it will be used only when it is presented and aad_object_id is not null.