Skip to content

perf: cache Azure CLI Graph token across publish command Graph API calls#267

Merged
sellakumaran merged 3 commits intomicrosoft:mainfrom
pratapladhani:perf/publish-graph-token-caching
Feb 17, 2026
Merged

perf: cache Azure CLI Graph token across publish command Graph API calls#267
sellakumaran merged 3 commits intomicrosoft:mainfrom
pratapladhani:perf/publish-graph-token-caching

Conversation

@pratapladhani
Copy link
Contributor

Summary

Optimizes the publish command by caching Azure CLI Graph tokens in memory and eliminating redundant service principal lookups.

Fixes #265

Problem

Every Graph API call in the publish command spawns two az subprocesses (az account show + az account get-access-token). A typical publish with MOS prerequisites check triggers ~11 Graph API calls = ~22 subprocess spawns, adding ~45-60 seconds of overhead.

Additionally, CheckMosPrerequisitesAsync in PublishHelpers.cs performs redundant service principal lookups: Check 3 (admin consent verification) re-looks up the same 3 resource app SP IDs that Check 1 (SP verification) already found.

Changes

1. Token caching in GraphApiService.EnsureGraphHeadersAsync()

  • Added in-memory token cache with 5-minute TTL for the Azure CLI code path
  • Token is acquired once and reused across all Graph calls within the same command execution
  • Cache is tenant-aware -- switching tenants triggers a fresh acquisition
  • New fields: _cachedAzCliToken, _cachedAzCliTenantId, _cachedAzCliTokenExpiry

2. Redundant SP lookup elimination in PublishHelpers.CheckMosPrerequisitesAsync()

  • Check 1 caches SP IDs in a dictionary as it verifies them
  • Check 3 reads from the dictionary instead of re-calling LookupServicePrincipalByAppIdAsync

Impact

Metric Before After Reduction
az subprocess spawns per publish ~22 2 91%
Graph API calls in CheckMosPrerequisitesAsync ~10 ~7 30%

Due Diligence

Security review

  • Cache is in-memory only -- never persisted to disk, no file I/O
  • Token already exists in process memory (on HttpClient.DefaultRequestHeaders.Authorization) regardless of caching
  • Process exit clears all cached state -- no cross-session leakage
  • 5-minute TTL is conservative vs the token's ~60-minute Azure CLI validity
  • Contrast with PR fix: Add persistent MSAL token cache to reduce repeated WAM login prompts #261 (MSAL persistent cache) which correctly uses disk encryption -- different concern

Conflict analysis (4 open PRs checked)

PR Files Overlap?
#266 (exit codes) PublishCommand.cs None
#263 (MCP protocol) MockToolingServer/Server.cs None
#261 (MSAL caching) MsalBrowserCredential.cs, Directory.Packages.props None (different auth path)
#257 (endpoint name) CleanupCommand.cs, EndpointHelper.cs None

Thread safety

  • CLI is single-threaded (sequential command execution) -- no concurrent access to cache fields
  • Cache fields are private instance members, not static shared state

Cross-platform

  • No platform-specific code introduced -- works on Windows, macOS, Linux
  • Uses DateTimeOffset.UtcNow for timezone-safe expiry

Edge cases considered

  • Different tenant ID: Cache invalidated, fresh token acquired (tested)
  • Token acquisition failure: Not cached, error propagated unchanged
  • Cache expiry: Falls through to fresh acquisition naturally
  • Existing behavior preserved: CheckServicePrincipalCreationPrivilegesAsync bypasses EnsureGraphHeadersAsync and is unaffected

Testing

  • 4 new unit tests covering: same-tenant reuse, different-tenant isolation, mixed GET/POST operations, cache duration constant
  • Full test suite: 954 passed, 0 failed, 17 skipped
  • Real-world validation: a365 publish --verbose confirmed 1x token acquisition + 10x cache hits

Before (verbose output)

[DEBUG] Acquiring Graph API access token for tenant ...  (x11)
[DEBUG] Executing: az account show                       (x11)
[DEBUG] Executing: az account get-access-token ...       (x11)

After (verbose output)

[DEBUG] Acquiring Graph API access token for tenant ...  (x1)
[DEBUG] Executing: az account show                       (x1)
[DEBUG] Executing: az account get-access-token ...       (x1)
[DEBUG] Cached Azure CLI Graph token for 5 minutes
[DEBUG] Using cached Azure CLI Graph token (expires in 5.0 minutes)  (x10)

Add in-memory token caching to GraphApiService.EnsureGraphHeadersAsync()
for the Azure CLI code path. Previously, every Graph API call spawned two
'az' subprocesses (account show + get-access-token), resulting in ~22
subprocess spawns per publish command. With caching, the token is acquired
once and reused for 5 minutes.

Also eliminate redundant service principal lookups in
PublishHelpers.CheckMosPrerequisitesAsync() - Check 3 (admin consent
verification) previously re-looked up the same 3 resource app SP IDs that
Check 1 already found. Now uses a dictionary to pass SP IDs from Check 1
to Check 3.

Net result: ~22 subprocess spawns reduced to 2 (91% reduction).

Fixes microsoft#265
Copilot AI review requested due to automatic review settings February 16, 2026 17:28
@pratapladhani pratapladhani requested review from a team as code owners February 16, 2026 17:28
Copy link
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

Improves a365 publish performance by avoiding repeated Azure CLI (az) subprocess token acquisitions for Graph API calls, and by reusing previously resolved service principal IDs during MOS prerequisites checks.

Changes:

  • Add an in-memory, tenant-aware Azure CLI Graph token cache (5-minute TTL) in GraphApiService.EnsureGraphHeadersAsync().
  • Eliminate redundant service principal lookups in PublishHelpers.CheckMosPrerequisitesAsync() by caching SP object IDs from the initial verification step.
  • Add unit tests validating token reuse behavior across multiple Graph calls.

Reviewed changes

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

File Description
src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Services/GraphApiServiceTokenCacheTests.cs New tests covering Azure CLI token cache behavior across Graph GET/POST calls and tenants.
src/Microsoft.Agents.A365.DevTools.Cli/Services/GraphApiService.cs Adds tenant-aware in-memory caching for Azure CLI Graph tokens in EnsureGraphHeadersAsync().
src/Microsoft.Agents.A365.DevTools.Cli/Helpers/PublishHelpers.cs Reuses cached resource SP IDs to avoid duplicate Graph lookups during MOS prerequisites checks.

sellakumaran and others added 2 commits February 17, 2026 11:28
Applies code review improvements from pr-267-review.yaml:

- CR-267-001 (MEDIUM): Add cache expiry test
  * Added internal property CachedAzCliTokenExpiry to GraphApiService for testability
  * Added test GraphGetAsync_ExpiredCache_AcquiresNewToken to validate expired cache behavior

- CR-267-002 (MEDIUM): Clear cache on token acquisition failure
  * Added cache clearing logic (_cachedAzCliToken, _cachedAzCliTenantId, _cachedAzCliTokenExpiry)
    when GetGraphAccessTokenAsync returns null/empty to ensure clean state on failure

- CR-267-003 (LOW): HttpResponseMessage disposal verified
  * Verified TestHttpMessageHandler.Dispose() properly disposes queued responses (no changes needed)

- CR-267-004 (LOW): Cache duration constant placement
  * Kept AzCliTokenCacheDuration in GraphApiService as internal constant (no changes needed per review)

- CR-267-005 (INFO): Improve log message clarity
  * Updated PublishHelpers.cs Check 3 log from LogDebug "configuration needed" to
    LogWarning "unexpected - should have been cached by Check 1" to indicate unexpected condition

All tests passing: 954 passed (including new cache expiry test), 0 failed.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…e tests

Fixes inline review comments from GitHub Copilot:

1. HttpResponseMessage disposal (Comments on lines 61, 93):
   - Wrapped all tests in try-finally blocks to ensure TestHttpMessageHandler.Dispose() is called
   - This ensures queued HttpResponseMessage instances are properly disposed after each test
   - Prevents resource leaks during test execution

2. Missing assertion for subprocess reduction (Comment on line 77):
   - Added assertion to verify 'az account show' is called exactly once
   - This validates the full subprocess reduction claim (not just get-access-token)
   - Tests now verify both account show and get-access-token call counts

All 5 tests passing with proper resource cleanup.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings February 17, 2026 19:46
@sellakumaran sellakumaran enabled auto-merge (squash) February 17, 2026 19:47
Copy link
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 3 out of 3 changed files in this pull request and generated 4 comments.

@sellakumaran sellakumaran merged commit e83fd65 into microsoft:main Feb 17, 2026
10 checks passed
@pratapladhani pratapladhani deleted the perf/publish-graph-token-caching branch February 18, 2026 05:43
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.

Publish command: GraphApiService acquires a new token for every Graph API call (no caching in Azure CLI path)

3 participants

Comments