.NET: Isolate Foundry toolbox response cache - #8848
Roger Barreto (rogerbarreto) wants to merge 3 commits into
Conversation
Request-opened toolbox clients must not outlive the response context that created them. Keep startup cache reuse while disposing scoped clients on every response exit.
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The scoped lifecycle is consistently implemented and covered across concurrent, successful, and exceptional response paths.
Review effort: Balanced
Findings: None
What changed in this PR
Introduces response-scoped Foundry toolbox caching to prevent cross-request client reuse while preserving startup cache sharing.
Changes:
- Adds opaque response scopes with deterministic client cleanup.
- Separates startup and request-specific tools and consent state.
- Adds concurrency, isolation, reuse, failure, and disposal coverage.
| File | Description |
|---|---|
FoundryToolboxService.cs |
Implements scoped caches and disposal. |
AgentFrameworkResponseHandler.cs |
Manages toolbox scope across response streaming. |
HostedCallContext.cs |
Carries the response scope through async execution. |
FoundryToolboxServiceTests.cs |
Covers scoped reuse, isolation, and cleanup. |
FoundryToolboxResponseScopeTests.cs |
Adds end-to-end response lifecycle tests. |
FoundryToolboxMarkerScopingTests.cs |
Updates marker tests with request identities. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (1 commit(s)): a877c87e32ae
Model: gpt-5.6-sol-fast
Overview
The PR correctly separates startup-owned toolbox state from response-owned clients, uses opaque scope IDs, reapplies ambient scope across streaming yields, and disposes scoped resources on normal and exceptional response exits. The new per-response ownership model nevertheless permits unbounded process-wide client retention while responses remain active, serializes independent network opens behind a singleton semaphore, and can abandon resources after a disposal exception.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
3 verified findings remained after source verification (1 high, 2 medium) across 1 file. Details are attached to the affected lines below.
Affected areas: dotnet/src/Microsoft.Agents.AI.Foundry.Hosting/FoundryToolboxService.cs
Code Coverage OverviewLanguages: C# C# / code-coverage/dotnetThe overall line coverage in commit 9df5264 in the Show a line coverage summary of the most covered files.
Updated |
Allow independent response scopes to open concurrently. Cleanup attempts every scoped resource, preserves response outcomes, and aggregates failures only during service shutdown.
| return new FoundryToolboxService.ToolboxOpenResult( | ||
| new FoundryToolboxService.CachedToolbox( | ||
| Client: null, | ||
| new HttpClient(handler), |
| return new FoundryToolboxService.ToolboxOpenResult( | ||
| new FoundryToolboxService.CachedToolbox( | ||
| Client: null, | ||
| new HttpClient(handler), |
| new FoundryToolboxService.ToolboxOpenResult( | ||
| new FoundryToolboxService.CachedToolbox( | ||
| Client: null, | ||
| new HttpClient(handler), |
Motivation & Context
Foundry toolbox clients opened while serving a response were retained by the singleton toolbox service under the toolbox name. That lifetime allowed later responses to reuse client and tool state created under a different request context.
This change aligns request-opened toolbox ownership with the response lifetime while preserving safe reuse for clients opened during container startup.
Description & Review Guide
AgentFrameworkResponseHandler, the separation between startup and request caches inFoundryToolboxService, and cleanup across normal and exceptional exits.Related Issue
N/A. This change is tracked in a restricted incident record, so no GitHub issue is created.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and the title prefix in sync automatically.