Skip to content

.NET: Rename and restructure MCP skills cache - #9586

Merged
SergeyMenshykh merged 1 commit into
feature-mcp-skills-nextfrom
sergeymenshykh-mcp-skills-snapshot
Oct 21, 2026
Merged

SergeyMenshykh merged 1 commit into
feature-mcp-skills-nextfrom
sergeymenshykh-mcp-skills-snapshot

Conversation

@SergeyMenshykh

@SergeyMenshykh SergeyMenshykh commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

Motivation & Context

Make the MCP skills cache easier to understand by naming its stored skill list and page lifetimes a snapshot, and configuring discovery once when the cache is created. This is a naming and structural refactor; discovery and caching logic are unchanged.

Description

  • Rename McpListedSkillsDiscoveryResult to McpListedSkillsSnapshot and use snapshot naming throughout the cache.
  • Move discoverSkills from GetOrCreateAsync to the McpListedSkillsCache constructor.
  • Update the loader, tests, and XML comments to match.
  • Preserve existing discovery and caching behavior.

Related Issue

N/A

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: eeaa072d-79cf-4092-8f3f-9931e6c1a4e0

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

Static review supports this behavior-preserving refactor with no blocking issues identified; tests were not run.

No open issues 🎉

What changed in this PR

Clarifies the .NET MCP skills cache structure while preserving discovery and caching behavior.

Changes:

  • Renames discovery results to snapshots.
  • Moves the discovery callback into the cache constructor.
  • Updates tests and adds null-callback validation coverage.
File Description
dotnet/​tests/​Microsoft.Agents.AI.Mcp.UnitTests/​Skills/​Listed/​McpListedSkillsCacheTests.cs Updates cache usage and tests constructor validation.
dotnet/​src/​Microsoft.Agents.AI.Mcp/​Skills/​Listed/​McpListedSkillsSnapshot.cs Introduces the renamed snapshot container.
dotnet/​src/​Microsoft.Agents.AI.Mcp/​Skills/​Listed/​McpListedSkillsLoader.cs Supplies discovery during cache construction.
dotnet/​src/​Microsoft.Agents.AI.Mcp/​Skills/​Listed/​McpListedSkillsDiscoveryResult.cs Removes the former container name.
dotnet/​src/​Microsoft.Agents.AI.Mcp/​Skills/​Listed/​McpListedSkillsCache.cs Stores the callback and adopts snapshot terminology.

Review effort: Balanced

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MAF Automated Review — Iteration 1

Result: No findings
Scope: full PR (1 commit(s)): c557f78ab2b9
Model: gpt-5.6-sol

Overview

This refactor consistently renames the cached discovery result as a snapshot and binds discovery once when each per-loader cache is constructed. Existing guardrails preserve serialized refresh, atomic snapshot publication, TTL semantics, cancellation behavior, and source-to-cache disposal ownership, with focused cache tests and broader end-to-end TTL tests covering those contracts. No publishable Critical, High, or Medium defect was established.

Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.

@github-code-quality

Copy link
Copy Markdown
Contributor

Code coverage overview

Languages: C#

C# / code-coverage/dotnet

feature-mcp-skills-n... 08851e6 sergeymenshykh-mcp-s... c557f78 +/-
86% 85% -1%
View coverage of the 10 most impacted files.
File feature-mcp-skills-n... 08851e6 sergeymenshykh-mcp-s... c557f78 +/-
/home/runner/wo...toryProvider.cs 95% 31% -64%
/home/runner/wo...SessionStore.cs 100% 53% -47%
/home/runner/wo...hellExecutor.cs 90% 73% -17%
/home/runner/wo...SkillsSource.cs 89% 83% -6%
/home/runner/wo...ctionVisitor.cs 81% 75% -6%
/home/runner/wo...CopilotAgent.cs 72% 76% +4%
/home/runner/wo...pListedSkill.cs 0% 93% +93%
/home/runner/wo...SkillsLoader.cs 0% 100% +100%
/home/runner/wo...dSkillsCache.cs 0% 100% +100%
/home/runner/wo...ueExtensions.cs 0% 100% +100%

ℹ️ Calculations represent line coverage. Results may change as calculations are updated.

@SergeyMenshykh
SergeyMenshykh merged commit 64c8df6 into feature-mcp-skills-next Oct 21, 2026
23 of 24 checks passed
@SergeyMenshykh
SergeyMenshykh deleted the sergeymenshykh-mcp-skills-snapshot branch October 21, 2026 19:48

This branch was successfully deployed

2 active deployments
github-app-auth — c557f78a Deployed Oct 21, 2026 by SergeyMenshykh via add_label #25873
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

.NET Usage: [Issues, PRs], Target: .Net

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants