Skip to content

Fix GetCallerBaggagePairs: resolve userId across all channels#251

Open
fpfp100 wants to merge 3 commits intomainfrom
fix/userid-fallback-chain
Open

Fix GetCallerBaggagePairs: resolve userId across all channels#251
fpfp100 wants to merge 3 commits intomainfrom
fix/userid-fallback-chain

Conversation

@fpfp100
Copy link
Copy Markdown

@fpfp100 fpfp100 commented May 5, 2026

Summary

Channel behavior after fix

Field Teams Other channels (aad_object_id None) A2A (user) A2A (agent)
userId aad_object_id frm.id aad_object_id agentic_user_id

Test plan

  • Existing tests pass
  • New test: non-Teams channel — userId falls back to frm.id
  • New test: A2A — userId falls back to agentic_user_id
  • New test: precedence — aad_object_id wins when all set

🤖 Generated with Claude Code

userId was only set from aad_object_id, which is None on non-Teams
channels and A2A calls. Add fallback chain: aad_object_id → agentic_user_id → frm.id

Port of microsoft/Agent365-dotnet#246

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 5, 2026 00:54
@fpfp100 fpfp100 requested a review from a team as a code owner May 5, 2026 00:54
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 5, 2026

⚠️ Deprecation Warning: The deny-licenses option is deprecated for possible removal in the next major release. For more information, see issue 997.

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates observability hosting caller identity extraction so get_caller_pairs can still populate user.id when aad_object_id is absent, improving telemetry coverage for non-Teams and A2A scenarios.

Changes:

  • Changed caller ID resolution in get_caller_pairs to use aad_object_id -> agentic_user_id -> from.id.
  • Added regression tests for non-Teams fallback to from.id.
  • Added tests for A2A fallback and identifier precedence when multiple values are present.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
libraries/microsoft-agents-a365-observability-hosting/microsoft_agents_a365/observability/hosting/scope_helpers/utils.py Updates caller user.id derivation used by hosting scope helpers.
tests/observability/hosting/scope_helpers/test_scope_helper_utils.py Adds focused tests for fallback behavior and precedence.

jsl517 and others added 2 commits May 4, 2026 18:32
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 5, 2026 01:55
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

if not frm:
return
yield USER_ID_KEY, frm.aad_object_id
yield USER_ID_KEY, frm.aad_object_id or frm.agentic_user_id or frm.id
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fpfp100 review this

Copy link
Copy Markdown
Author

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.

from_account = ChannelAccount(
id="from-id-456",
name="Agent Caller",
agentic_user_id="a2a-agent-guid",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fpfp100 review this

Copy link
Copy Markdown
Author

@fpfp100 fpfp100 May 6, 2026

Choose a reason for hiding this comment

The 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>

if not frm:
return
yield USER_ID_KEY, frm.aad_object_id
yield USER_ID_KEY, frm.aad_object_id or frm.agentic_user_id or frm.id
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fpfp100 review this

from_account = ChannelAccount(
id="from-id-456",
name="Agent Caller",
agentic_user_id="a2a-agent-guid",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fpfp100 review this

@fpfp100
Copy link
Copy Markdown
Author

fpfp100 commented May 5, 2026

Re: Copilot comment on test_scope_helper_utils.py:106 — 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 presence of agentic_user_id with absent aad_object_id is the A2A condition — no additional marker is needed. This matches the .NET original (microsoft/Agent365-dotnet#246).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants