feat(msgraph): batch Teams account→id resolution via startsWith advanced query by saurav-err-137 · Pull Request #438 · hmchangw/chat

saurav-err-137 · 2026-07-02T01:46:43Z

Summary

Follow-up to #378, addressing review feedback on how the Teams presence sync resolves accounts to Azure object IDs. Rebased on latest main; two logically-distinct commits.

1. `feat(msgraph): batch account->id resolution via startsWith advanced query`

Replaces the per-account GetUserByPrincipalName(account@domain) lookup with a batched, domain-agnostic GetUsersByAccounts:

OR-combines startsWith(userPrincipalName,'account@') across the cache-missing accounts into chunked $filter queries (ConsistencyLevel: eventual + $count for the advanced query).
Each account contributes both a lower- and upper-cased startsWith clause rather than relying on Graph case-insensitivity (accounts/query halved to stay within the clause budget).
Results map back by UPN local-part, case-insensitively, first match wins.
Drops the hardcoded TEAMS_EMAIL_DOMAIN (accounts may live under different domains), avoids per-account round-trips, and never enumerates the whole tenant.

2. `refactor(msgraph): move user lookup off Client onto DirectoryReader`

room-service depends on msgraph.Client only for CreateOnlineMeeting, but the user-lookup method sat on that same interface — forcing room-service's test fakes to change whenever it changed. Moved GetUsersByAccounts onto its own DirectoryReader interface + NewDirectoryClient constructor (mirroring the existing PresenceReader), so Client is meetings-only again and room-service no longer depends on the user-lookup surface. No behavior change.

Why

The prior approach hardcoded a single email domain and did one Graph round-trip per account.
startsWith needs the advanced-query headers, and Graph caps $filter complexity — hence chunking (≤15 clauses/query).
The permanent presence:idmap:azure cache means steady-state runs make ~zero Graph calls; only genuinely new accounts are looked up.

Testing

make lint — 0 issues
make sast — gosec / govulncheck / semgrep all PASS
Unit: pkg/msgraph (batch/chunk/cased-variants/empty), room-service, user-presence-service, .../sync
Integration tests compile under -tags integration (run on CI)

Each commit builds and passes tests independently.

🤖 Generated with Claude Code

https://claude.ai/code/session_011xT3cu9rqxJyNUzYsAv6sL

Generated by Claude Code

Summary by CodeRabbit

New Features
- Teams in-call presence is now synced using a broader, account-based lookup that works without relying on a fixed email domain.
- Presence resolution now fills missing user mappings in larger batches, improving coverage for synced users.
Bug Fixes
- Improved matching for user accounts with different letter casing.
- Reduced failures when resolving presence by falling back more gracefully if directory lookup errors occur.

…uery Replace the per-account GetUserByPrincipalName(account@domain) lookup with a batched, domain-agnostic GetUsersByAccounts: OR-combine startsWith(userPrincipalName,'account@') across the cache-missing accounts into chunked $filter queries (ConsistencyLevel: eventual + $count for the advanced query). Each account contributes both a lower- and upper-cased startsWith clause rather than relying on Graph case-insensitivity (accounts per query halved to stay within the clause budget); results map back by UPN local-part case-insensitively (first match wins). Drops the hardcoded TEAMS_EMAIL_DOMAIN (accounts may live under different domains), avoids per-account round-trips, and never enumerates the whole tenant. Follow-up to #378. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011xT3cu9rqxJyNUzYsAv6sL

room-service depends on msgraph.Client only for CreateOnlineMeeting, but the user-lookup method sat on that same interface, forcing room-service's fakes to change whenever it changed. Move GetUsersByAccounts onto its own DirectoryReader interface + NewDirectoryClient constructor (mirroring PresenceReader), so Client is meetings-only again and room-service no longer depends on the user-lookup surface. The sync uses NewDirectoryClient; no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011xT3cu9rqxJyNUzYsAv6sL

coderabbitai · 2026-07-02T01:47:01Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bcd3e052-802d-455c-a4d6-5116c07dcd5a

📥 Commits

Reviewing files that changed from the base of the PR and between 4f7f57a and f614ce5.

📒 Files selected for processing (10)

docs/superpowers/specs/2026-06-22-teams-presence-in-call-sync-design.md
pkg/msgraph/msgraph.go
pkg/msgraph/msgraph_test.go
room-service/handler_teams_test.go
user-presence-service/sync/deploy/docker-compose.yml
user-presence-service/sync/main.go
user-presence-service/sync/mock_test.go
user-presence-service/sync/reconcile.go
user-presence-service/sync/reconcile_test.go
user-presence-service/sync/store.go

💤 Files with no reviewable changes (2)

user-presence-service/sync/deploy/docker-compose.yml
room-service/handler_teams_test.go

📝 Walkthrough

Walkthrough

Replaces the single-user Graph lookup (GetUserByPrincipalName) with a batched, domain-agnostic user resolver (GetUsersByAccounts) via a new DirectoryReader interface in pkg/msgraph, updates user-presence-service sync reconciliation and config to use it, removes stale stub methods, updates mocks/tests, and revises the design spec accordingly.

Changes

Batched directory lookup

Layer / File(s)	Summary
DirectoryReader interface and GetUsersByAccounts implementation `pkg/msgraph/msgraph.go`, `pkg/msgraph/msgraph_test.go`	Adds `DirectoryReader` interface, `NewDirectoryClient` constructor, and a chunked, case-variant `startsWith` batched Graph query replacing `GetUserByPrincipalName`; tests cover auth headers, filter contents, chunking, and empty input.
Sync reconciliation resolves IDs via batched lookup `user-presence-service/sync/reconcile.go`, `store.go`, `main.go`, `mock_test.go`, `reconcile_test.go`, `deploy/docker-compose.yml`	Adds `localPart` helper and rewrites `fetchIDs` to resolve missing accounts via `GetUsersByAccounts`, matching case-insensitively by UPN local-part; removes `EmailDomain` config/env var and updates wiring, mocks, and tests.
Stub cleanup and design doc updates `room-service/handler_teams_test.go`, `docs/superpowers/specs/2026-06-22-teams-presence-in-call-sync-design.md`	Removes obsolete `GetUserByPrincipalName` stub methods from test doubles and updates the design spec to describe the batched, domain-agnostic resolution approach.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Reconciler
  participant Cache as Valkey Cache
  participant DirectoryReader
  participant GraphAPI

  Reconciler->>Cache: lookup presence:idmap:azure for active accounts
  Cache-->>Reconciler: cached IDs, missing accounts
  Reconciler->>DirectoryReader: GetUsersByAccounts(missing accounts)
  DirectoryReader->>DirectoryReader: chunk accounts, build startsWith filters
  DirectoryReader->>GraphAPI: GET /users?$filter=...&ConsistencyLevel=eventual
  GraphAPI-->>DirectoryReader: value[] GraphUser
  DirectoryReader-->>Reconciler: []GraphUser
  Reconciler->>Reconciler: match by localPart (case-insensitive, first match wins)
  Reconciler->>Cache: persist resolved account -> azureObjectID

Possibly related PRs

hmchangw/chat#378: Directly refactors the Graph directory lookup used by the Teams in-call sync (replacing GetUserByPrincipalName with a batched GetUsersByAccounts/DirectoryReader).

Suggested labels: ready

Suggested reviewers: mliu33

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: batching Teams account-to-ID resolution with a startsWith Graph query.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch claude/dreamy-dirac-jr5k0u

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

claude added 2 commits July 2, 2026 01:43

saurav-err-137 added the ready label Jul 2, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(msgraph): batch Teams account→id resolution via startsWith advanced query#438

feat(msgraph): batch Teams account→id resolution via startsWith advanced query#438
saurav-err-137 wants to merge 2 commits into
mainfrom
claude/dreamy-dirac-jr5k0u

saurav-err-137 commented Jul 2, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 2, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

saurav-err-137 commented Jul 2, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

1. feat(msgraph): batch account->id resolution via startsWith advanced query

2. refactor(msgraph): move user lookup off Client onto DirectoryReader

Why

Testing

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

saurav-err-137 commented Jul 2, 2026 •

edited by coderabbitai Bot

Loading

1. `feat(msgraph): batch account->id resolution via startsWith advanced query`

2. `refactor(msgraph): move user lookup off Client onto DirectoryReader`

coderabbitai Bot commented Jul 2, 2026 •

edited

Loading