Repository navigation
.NET: Add TTL caching for MCP-listed skills - #9558
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 43a35206-f8c3-448c-a778-8cf9eaf19ecc
There was a problem hiding this comment.
🟢 Approval recommended
No blocking issues remain, and focused tests cover freshness, refresh failures, synchronization, cancellation, and disposal.
No open issues 🎉
What changed in this PR
Adds server-directed TTL caching to .NET MCP skill discovery, reducing repeated requests while refreshing expired catalogs on demand.
Changes:
- Parses per-page TTLs and synchronizes complete catalog refreshes.
- Integrates loader disposal and documents disabling outer caching layers.
- Adds deterministic coverage for expiration, pagination, concurrency, cancellation, and failed refreshes.
| File | Description |
|---|---|
| dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillsTests.Ttl.cs | Adds TTL and lifecycle tests. |
| dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillsTests.cs | Makes tests partial and disposes loaders. |
| dotnet/tests/Microsoft.Agents.AI.Mcp.UnitTests/Skills/Listed/AgentMcpListedSkillsSerializationTests.cs | Tests TTL deserialization and validation. |
| dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpSkillsListResult.cs | Maps ttlMs to a nullable duration. |
| dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsLoader.cs | Implements synchronized, TTL-aware catalog caching. |
| dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs | Documents disabling builder caching. |
| dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentMcpSkillsSource.cs | Documents TTL behavior and disposes the loader. |
Review effort: Balanced
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (1 commit(s)): c29de4e2c6b0
Model: gpt-5.6-sol
Overview
The loader measures each page's TTL from response receipt, refreshes complete catalogs under a single-flight gate, and publishes replacements only after all pages and manifests validate. The tests cover expiry boundaries, pagination, cancellation, failed refreshes, and preservation of previously returned manifests. However, the default builder cache prevents the new TTL behavior from running at all unless every caller opts out, and the new disposal path can race active discovery.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
2 verified findings remained after source verification (1 high, 1 medium) across 2 files. Details are attached to the affected lines below.
Affected areas: dotnet/src/Microsoft.Agents.AI.Mcp/Skills/AgentSkillsProviderBuilderMcpExtensions.cs, dotnet/src/Microsoft.Agents.AI.Mcp/Skills/Listed/McpListedSkillsLoader.cs
Code coverage overviewLanguages: C# C# / code-coverage/dotnet
View coverage of the 10 most impacted files.
ℹ️ Calculations represent line coverage. Results may change as calculations are updated. Updated |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 43a35206-f8c3-448c-a778-8cf9eaf19ecc
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 43a35206-f8c3-448c-a778-8cf9eaf19ecc
Motivation & Context
Honor server-provided TTLs for MCP-listed skills, avoiding repeated discovery while refreshing expired catalogs on demand.
Description & Review Guide
ttlMsfromskills/listresponses and cache skills while every discovery page remains fresh.DisableCaching()and add deterministic TTL, pagination, concurrency, cancellation, and disposal tests.Related Issue
N/A - no linked issue.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) - a workflow keeps the label and title prefix in sync automatically.