Skip to content

.NET: Simplify ClientHeadersScope to rely on AsyncLocal natural restoration#5676

Open
rogerbarreto wants to merge 3 commits intomicrosoft:mainfrom
rogerbarreto:feature/clientheaders-asynclocal-simplification
Open

.NET: Simplify ClientHeadersScope to rely on AsyncLocal natural restoration#5676
rogerbarreto wants to merge 3 commits intomicrosoft:mainfrom
rogerbarreto:feature/clientheaders-asynclocal-simplification

Conversation

@rogerbarreto
Copy link
Copy Markdown
Member

Summary

Simplifies ClientHeadersScope by dropping the explicit using/Dispose pattern and relying on the natural AsyncLocal<T> restoration that happens when an awaited async method returns.

What changes

  • ClientHeadersScope collapses to a single Current { get; set; } property over an AsyncLocal<IReadOnlyDictionary<string, string>?>. Drops Push, the Scope struct, and Dispose. XML doc explains the AsyncLocal natural-restoration semantics so the design intent is self-documenting.
  • ClientHeadersAgent uses a direct ClientHeadersScope.Current = snapshot before delegating in both RunCoreAsync and RunCoreStreamingAsync. Drops the local RunAsyncCoreAsync helper and the snapshot-passed-as-parameter dance.
  • Test 10 renamed to ClientHeadersScope_IsAsyncLocalIsolatedAndAutoRestoresAsync; keeps the parallel-isolation assertion and adds a "caller sees null after the awaited probe returns" assertion.
  • Test 12 switches from using ClientHeadersScope.Push(...) to direct Current = ... with try/finally for test isolation.

Why

AsyncLocal<T> mutations made inside an awaited async method are visible within that method (and all of its awaits) but do not propagate back to the caller after the method returns. Since ClientHeadersAgent is the only caller of the scope and both call sites are async methods awaited by their callers, the explicit using/Dispose pattern was doing work the runtime already does for us.

The snapshot deep-copy in TrySnapshot stays. It defends against caller mutating the source Dictionary mid-run, which is independent of the AsyncLocal restoration mechanism.

Verification

  • Foundry build clean (0 errors, 0 warnings).
  • 401 Foundry unit tests pass.
  • CI-parity dotnet format --verify-no-changes clean on the changed project.

Wesley pointed out (with a clean demo) that AsyncLocal<T> mutations made
inside an awaited async method do not leak back to the caller after the
method returns - the runtime restores the caller's view automatically.

ClientHeadersAgent.RunCoreAsync and RunCoreStreamingAsync are the only
callers of the scope, both are async methods awaited by their callers,
so the explicit using/Dispose pattern was doing work the runtime already
does for us.

* ClientHeadersScope collapsed to a single Current { get; set; } property
  over an AsyncLocal<IReadOnlyDictionary<string,string>?>. Drops Push,
  the Scope struct, and Dispose. XML doc explains the AsyncLocal natural-
  restoration semantics so the design intent is self-documenting.
* ClientHeadersAgent uses a direct ClientHeadersScope.Current = snapshot
  before delegating. Drops the local RunAsyncCoreAsync helper and the
  snapshot-passed-as-parameter dance.
* Test 10 renamed to ClientHeadersScope_IsAsyncLocalIsolatedAndAutoRestoresAsync;
  drops the LIFO claim, keeps the parallel-isolation assertion, and adds
  a Wesley-style 'set inside async, caller sees null on return' assertion.
* Test 12 switches from using ClientHeadersScope.Push to direct
  Current = ... with try/finally for test isolation.

Snapshot deep-copy in TrySnapshot stays - it defends against caller
mutating the source Dictionary mid-run, which is independent of the
AsyncLocal restoration mechanism.
Copilot AI review requested due to automatic review settings May 6, 2026 15:24
@moonbox3 moonbox3 added the .NET label May 6, 2026
@github-actions github-actions Bot changed the title Simplify ClientHeadersScope to rely on AsyncLocal natural restoration .NET: Simplify ClientHeadersScope to rely on AsyncLocal natural restoration May 6, 2026
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Automated Code Review

Reviewers: 2 | Confidence: 95% | Result: All clear

Reviewed: Test Coverage, Design Approach


Automated review by rogerbarreto's agents

@rogerbarreto rogerbarreto added this pull request to the merge queue May 6, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 6, 2026
@rogerbarreto rogerbarreto added this pull request to the merge queue May 7, 2026
@rogerbarreto rogerbarreto self-assigned this May 7, 2026
@rogerbarreto rogerbarreto moved this to In Review in Agent Framework May 7, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 7, 2026
@rogerbarreto rogerbarreto enabled auto-merge May 7, 2026 18:18
@rogerbarreto rogerbarreto added this pull request to the merge queue May 7, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 7, 2026
@rogerbarreto rogerbarreto enabled auto-merge May 7, 2026 21:01
@rogerbarreto rogerbarreto added this pull request to the merge queue May 7, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

4 participants