Skip to content

.NET: Clarify Agent Skills caching behavior - #8474

Merged
SergeyMenshykh merged 1 commit into
microsoft:mainfrom
SergeyMenshykh:sergeymenshykh-document-skill-cache-isolation
Sep 17, 2026
Merged

SergeyMenshykh merged 1 commit into
microsoft:mainfrom
SergeyMenshykh:sergeymenshykh-document-skill-cache-isolation

Conversation

@SergeyMenshykh

Copy link
Copy Markdown
Contributor

Motivation & Context

The Agent Skills builder caches resolved skills by default, but the scope of that cache was not stated consistently where custom sources are registered and providers are built. Clarifying the existing behavior helps callers choose the appropriate caching configuration for context-dependent sources.

Description & Review Guide

  • What are the major changes? Expanded the XML documentation for both custom-source overloads, Build(), and CacheIsolationKeySelector to describe the existing shared cache bucket and point to DisableCaching() and cache isolation keys.
  • What is the impact of these changes? Documentation only; there are no API or runtime behavior changes.
  • What do you want reviewers to focus on? Whether the guidance accurately describes the current builder and caching-options behavior without implying a recommended isolation-key cardinality.

Related Issue

N/A — no public issue is associated with this documentation clarification.

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 the title prefix in sync automatically.

Document the default shared cache behavior for custom skill sources and point callers to the existing caching controls.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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

The documentation accurately reflects the caching implementation and introduces no runtime changes.

Pull request overview

Clarifies the existing default cache scope and isolation requirements for .NET Agent Skills providers.

Changes:

  • Documents shared-cache behavior for custom sources and Build().
  • Explains when to use isolation keys or disable caching.
File summaries
File Description
CachingAgentSkillsSourceOptions.cs Clarifies cache isolation and security-boundary considerations.
AgentSkillsProviderBuilder.cs Documents default caching across source registration and provider construction.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

@SergeyMenshykh
SergeyMenshykh deployed to github-app-auth September 17, 2026 15:14 — with GitHub Actions Active

@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: Findings reported
Scope: full PR (1 commit(s)): 4e1fc1beb6c1
Model: gpt-5.6-sol-fast

Overview

The added XML documentation correctly surfaces the builder's default caching, the null-key shared bucket, and the available opt-out and isolation controls. The implementation and tests substantiate default reuse, per-key separation, retries after failures or cancellation, and provider-owned cache disposal. One uncommon but concrete isolation gap remains: the internal shared-bucket sentinel is an ordinary string that a non-null selector can return, so the new security-boundary guidance overstates the isolation guarantee for that value.

Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
1 verified finding remained after source verification (1 medium) across 1 file. Details are attached to the affected lines below.

Affected areas: dotnet/src/Microsoft.Agents.AI/Skills/Decorators/CachingAgentSkillsSourceOptions.cs

@SergeyMenshykh
SergeyMenshykh added this pull request to the merge queue Sep 17, 2026
Merged via the queue into microsoft:main with commit 81c47e2 Sep 17, 2026
38 of 39 checks passed
@github-project-automation github-project-automation Bot moved this from In Review to Done in Agent Framework Sep 17, 2026
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.

3 participants