Python: make FoundryToolbox.as_skills_provider() disable_caching effective

python/packages/core/AGENTS.md

@@ -91,7 +91,7 @@ agent_framework/
 - **`SkillScriptRunner`** - Protocol for file-based script execution. Any callable matching `(skill, script, args) -> Any` satisfies it. Code-defined scripts do not use a runner.
 - **`SkillScriptArgumentParser`** - Public type alias for an optional callable `(raw args: dict | list[str] | str | None) -> dict | None` that converts the raw `args` value before an `InlineSkillScript` runs (applied before the inline list-args guard). It is an opt-in customization hook (port of .NET PR #6498) that lets callers support backends sending tool-call arguments in a non-conforming shape (e.g. vLLM JSON strings). The output is constrained to a `dict` (named keyword arguments) or `None`, because inline scripts bind arguments by keyword name. Supply it via the `argument_parser=` constructor arg on `InlineSkillScript`, `InlineSkill` (default for scripts added via `@skill.script`), or `ClassSkill` (default for scripts discovered via `@ClassSkill.script`). When `None` (the default), the raw value is used unchanged. File-based scripts are unaffected (their runner owns arg handling).
 - **`SkillsProvider`** - Context provider (extends `ContextProvider`) that discovers file-based skills from `SKILL.md` files and/or accepts code-defined `Skill` instances. Follows progressive disclosure: advertise → load → read resources / run scripts. By default all three tools it exposes (`load_skill`, `read_skill_resource`, `run_skill_script`) are registered with `approval_mode="always_require"`, so every skill operation needs approval. To run unattended, pass one of the static auto-approval rules to `ToolApprovalMiddleware` (via `auto_approval_rules`): `SkillsProvider.read_only_tools_auto_approval_rule` approves only the read-only tools (`load_skill`, `read_skill_resource`) while still prompting for `run_skill_script`, and `SkillsProvider.all_tools_auto_approval_rule` approves every skill tool including script execution. Both rules reject any call carrying a `server_label` so they stay scoped to this provider's local tools and never auto-approve a same-named hosted tool. Alternatively, for trusted skills, the constructor / `from_paths` kwargs `disable_load_skill_approval`, `disable_read_skill_resource_approval`, and `disable_run_skill_script_approval` (all default `False`) opt individual tools out of approval entirely by registering them with `approval_mode="never_require"` (the auto-approval rules only apply to tools that still require approval). The tool names are also exposed as class constants (`LOAD_SKILL_TOOL_NAME`, `READ_SKILL_RESOURCE_TOOL_NAME`, `RUN_SKILL_SCRIPT_TOOL_NAME`).
-- **`SkillsSource` decorators** - Skill sources are composable: `SkillsSource` is the abstract base, with concrete sources (`InMemorySkillsSource`, `FileSkillsSource`, `MCPSkillsSource`) and decorators that wrap an inner source — `AggregatingSkillsSource` (concatenate several sources), `FilteringSkillsSource` (predicate filter), `DeduplicatingSkillsSource` (first-wins by name), and `CachingSkillsSource` (cache the inner source's skills list). `DelegatingSkillsSource` is the abstract base for decorators. **`get_skills` takes a `SkillsSourceContext`**: every source/decorator implements `async def get_skills(self, context: SkillsSourceContext) -> list[Skill]` and forwards `context` to inner sources. `SkillsSourceContext` (frozen) carries the invoking `agent` (`SupportsAgentRun`) and optional `session` (`AgentSession | None`); `SkillsProvider` builds it from `before_run`'s `agent`/`session` and passes it into the pipeline. `FilteringSkillsSource`'s predicate is context-aware: `Callable[[Skill, SkillsSourceContext], bool]` (port of .NET #6797). **Default caching is applied only to the built-in, context-independent leaf sources**: for the `Skill` / sequence-of-skills / `from_paths` constructors, `SkillsProvider` builds `DeduplicatingSkillsSource(CachingSkillsSource(<file|in-memory leaf>))` so expensive filesystem/network discovery runs once. A **caller-supplied `SkillsSource` is used as-is — never auto-wrapped in caching or deduplication** — because auto-caching a context-aware caller source in a single shared bucket would replay the first invocation's skills for later `SkillsSourceContext`s and leak skills across agents/tenants (matches .NET, whose custom-source constructor also adds no caching/dedup). Callers who want caching on a custom pipeline compose `CachingSkillsSource(inner, cache_isolation_key_selector=...)` themselves. `disable_caching=True` only affects the built-in leaf caching (it has no effect on a caller-supplied source, which is never cached). `CachingSkillsSource` shares a single in-flight fetch across concurrent callers (per cache key) and does not update its cache on a failed fetch, so the next call retries (an initial failure leaves the cache empty; a refresh failure keeps the previously cached list). By default all callers share one cache bucket; pass `cache_isolation_key_selector=Callable[[SkillsSourceContext], str | None]` to cache separately per key (e.g. per agent name) for context-aware inner sources — the key should be low-cardinality and stable, and returning `None` (or leaving the selector `None`) uses the shared bucket. By default a cached list never expires; pass `refresh_interval=timedelta(...)` (port of .NET `CachingAgentSkillsSourceOptions.RefreshInterval`) to treat a cached list as stale once it is older than the interval so the next call re-queries the inner source (useful when an inner source such as `MCPSkillsSource` changes over the process lifetime; a zero/negative interval makes every result immediately stale, and a failed refresh keeps the prior list and retries). Freshness is measured with a monotonic clock (`time.monotonic()`). `SkillsProvider.__init__` / `from_paths` expose a `cache_refresh_interval` kwarg that is threaded into the built-in `CachingSkillsSource` (it has no effect on a caller-supplied source or when `disable_caching=True`).
+- **`SkillsSource` decorators** - Skill sources are composable: `SkillsSource` is the abstract base, with concrete sources (`InMemorySkillsSource`, `FileSkillsSource`, `MCPSkillsSource`) and decorators that wrap an inner source — `AggregatingSkillsSource` (concatenate several sources), `FilteringSkillsSource` (predicate filter), `DeduplicatingSkillsSource` (first-wins by name), and `CachingSkillsSource` (cache the inner source's skills list). `DelegatingSkillsSource` is the abstract base for decorators. **`get_skills` takes a `SkillsSourceContext`**: every source/decorator implements `async def get_skills(self, context: SkillsSourceContext) -> list[Skill]` and forwards `context` to inner sources. `SkillsSourceContext` (frozen) carries the invoking `agent` (`SupportsAgentRun`) and optional `session` (`AgentSession | None`); `SkillsProvider` builds it from `before_run`'s `agent`/`session` and passes it into the pipeline. `FilteringSkillsSource`'s predicate is context-aware: `Callable[[Skill, SkillsSourceContext], bool]` (port of .NET #6797). **Default caching is applied only to the built-in, context-independent leaf sources**: for the `Skill` / sequence-of-skills / `from_paths` constructors, `SkillsProvider` builds `DeduplicatingSkillsSource(CachingSkillsSource(<file|in-memory leaf>))` so expensive filesystem/network discovery runs once. A **caller-supplied `SkillsSource` is used as-is — never auto-wrapped in caching or deduplication** — because auto-caching a context-aware caller source in a single shared bucket would replay the first invocation's skills for later `SkillsSourceContext`s and leak skills across agents/tenants (matches .NET, whose custom-source constructor also adds no caching/dedup). Callers who want caching on a custom pipeline compose `CachingSkillsSource(inner, cache_isolation_key_selector=...)` themselves. `disable_caching=True` only affects the built-in leaf caching (it has no effect on a caller-supplied source, which is never cached). `CachingSkillsSource` shares a single in-flight fetch across concurrent callers (per cache key) and does not update its cache on a failed fetch, so the next call retries (an initial failure leaves the cache empty; a refresh failure keeps the previously cached list). By default all callers share one cache bucket; pass `cache_isolation_key_selector=Callable[[SkillsSourceContext], str | None]` to cache separately per key (e.g. per agent name) for context-aware inner sources — the key should be low-cardinality and stable, and returning `None` (or leaving the selector `None`) uses the shared bucket. By default a cached list never expires; pass `refresh_interval=timedelta(...)` (port of .NET `CachingAgentSkillsSourceOptions.RefreshInterval`) to treat a cached list as stale once it is older than the interval so the next call re-queries the inner source (useful when an inner source such as `MCPSkillsSource` changes over the process lifetime; a zero/negative interval makes every result immediately stale, and a failed refresh keeps the prior list and retries). Freshness is measured with a monotonic clock (`time.monotonic()`). `SkillsProvider.__init__` / `from_paths` expose a `cache_refresh_interval` kwarg that is threaded into the built-in `CachingSkillsSource` (it has no effect on a caller-supplied source or when `disable_caching=True`). **`MCPSkillsSource` and `MCPSkill` accept exactly one of `client` (a fixed `ClientSession`) or `session_provider` (`Callable[[], ClientSession]`, resolved on every fetch); providing both/neither raises `ValueError`.** Use `session_provider` when the underlying session may be swapped over time — e.g. a reconnecting `MCPTool`/`FoundryToolbox` whose `session` is replaced on reconnect — so cached `MCPSkill`s keep fetching against the live session instead of a closed one (`MCPSkillsSource` forwards its provider to every `MCPSkill` it creates). A fixed `client` is safe only when the session outlives the skills.
 
 ### Model Context Protocol (`_mcp.py`)
 

python/packages/core/agent_framework/_skills.py

@@ -4054,6 +4054,38 @@ def _parse_mcp_skill_index(text: str) -> _McpSkillIndex:
     return _McpSkillIndex(schema=raw.get("$schema"), skills=entries)
 
 
+def _resolve_mcp_session_provider(
+    client: ClientSession | None,
+    session_provider: Callable[[], ClientSession] | None,
+) -> Callable[[], ClientSession]:
+    """Normalize the two MCP session inputs into a single session resolver.
+
+    Callers supply **exactly one** of a fixed ``client`` or a
+    ``session_provider`` callable. A fixed client is wrapped in a provider that
+    always returns it; a provider is used as-is so the session is resolved on
+    every call (reconnect-safe for sources whose underlying session is replaced
+    over time, e.g. a reconnecting :class:`~agent_framework.MCPTool`).
+
+    Args:
+        client: A fixed MCP client session, or ``None``.
+        session_provider: A callable returning the current MCP client session,
+            or ``None``.
+
+    Returns:
+        A callable that returns the MCP client session to use.
+
+    Raises:
+        ValueError: If both or neither of *client* and *session_provider* are
+            provided.
+    """
+    if (client is None) == (session_provider is None):
+        raise ValueError("Provide exactly one of 'client' or 'session_provider'.")
+    if session_provider is not None:
+        return session_provider
+    resolved = client
+    return lambda: cast("ClientSession", resolved)
+
+
 @experimental(feature_id=ExperimentalFeature.MCP_SKILLS)
 class MCPSkillResource(SkillResource):
     """A :class:`SkillResource` backed by content fetched from an MCP server.
@@ -4116,21 +4148,39 @@ def __init__(
         self,
         frontmatter: SkillFrontmatter,
         skill_md_uri: str,
-        client: ClientSession,
+        client: ClientSession | None = None,
+        *,
+        session_provider: Callable[[], ClientSession] | None = None,
     ) -> None:
         """Initialize an MCPSkill.
 
+        Provide **exactly one** of *client* or *session_provider*.
+
         Args:
             frontmatter: The parsed frontmatter metadata for this skill.
             skill_md_uri: The full MCP resource URI of the ``SKILL.md`` resource
                 (e.g. ``skill://unit-converter/SKILL.md``). The skill's root URI
                 is derived by stripping the trailing ``SKILL.md`` segment.
-            client: The MCP client session used to fetch resources on demand.
+            client: A fixed MCP client session used to fetch resources on demand.
+                Use this when the session outlives the skill (e.g. a caller-owned
+                long-lived session).
+
+        Keyword Args:
+            session_provider: A callable returning the current MCP client session,
+                resolved on every fetch. Prefer this over *client* when the
+                underlying session may be replaced over the skill's lifetime (for
+                example, a reconnecting :class:`~agent_framework.MCPTool` whose
+                ``session`` is swapped on reconnect), so a cached skill keeps
+                using the live session instead of a closed one.
+
+        Raises:
+            ValueError: If both or neither of *client* and *session_provider* are
+                provided.
         """
         self._frontmatter = frontmatter
         self._skill_md_uri = skill_md_uri
         self._skill_root_uri = self._compute_skill_root_uri(skill_md_uri)
-        self._client = client
+        self._session_provider = _resolve_mcp_session_provider(client, session_provider)
         self._content: str | None = None
 
     @property
@@ -4154,7 +4204,7 @@ async def get_content(self) -> str:
         if self._content is not None:
             return self._content
 
-        result = await self._client.read_resource(_mcp_any_url(self._skill_md_uri))
+        result = await self._session_provider().read_resource(_mcp_any_url(self._skill_md_uri))
         text = _mcp_join_text(result)
         if not text:
             raise ValueError(f"The MCP server returned no text content for SKILL.md resource '{self._skill_md_uri}'.")
@@ -4184,7 +4234,7 @@ async def get_resource(self, name: str) -> SkillResource | None:
 
         uri = self._skill_root_uri + normalized
         try:
-            result = await self._client.read_resource(_mcp_any_url(uri))
+            result = await self._session_provider().read_resource(_mcp_any_url(uri))
         except Exception as ex:
             if _is_mcp_resource_not_found(ex):
                 logger.debug("MCP resource '%s' not available: %s", uri, ex)
@@ -4276,14 +4326,36 @@ class MCPSkillsSource(SkillsSource):
     _INDEX_URI: Final[str] = "skill://index.json"
     _SKILL_MD_TYPE: Final[str] = "skill-md"
 
-    def __init__(self, client: ClientSession) -> None:
+    def __init__(
+        self,
+        client: ClientSession | None = None,
+        *,
+        session_provider: Callable[[], ClientSession] | None = None,
+    ) -> None:
         """Initialize an MCPSkillsSource.
 
+        Provide **exactly one** of *client* or *session_provider*.
+
         Args:
-            client: An MCP client session connected to a server that
-                exposes Agent Skills resources.
+            client: A fixed MCP client session connected to a server that exposes
+                Agent Skills resources. Use this when the session outlives the
+                source (e.g. a caller-owned long-lived session).
+
+        Keyword Args:
+            session_provider: A callable returning the current MCP client session,
+                resolved on every discovery and on each skill's on-demand fetch.
+                Prefer this over *client* when the underlying session may be
+                replaced over time (for example, a reconnecting
+                :class:`~agent_framework.MCPTool` whose ``session`` is swapped on
+                reconnect), so cached skills keep using the live session. The
+                provider is forwarded to every :class:`MCPSkill` this source
+                creates.
+
+        Raises:
+            ValueError: If both or neither of *client* and *session_provider* are
+                provided.
         """
-        self._client = client
+        self._session_provider = _resolve_mcp_session_provider(client, session_provider)
 
     async def get_skills(self, context: SkillsSourceContext) -> list[Skill]:
         """Discover and return skills from the MCP server.
@@ -4327,7 +4399,7 @@ async def _try_read_index(self) -> _McpSkillIndex | None:
             absent, empty, or malformed.
         """
         try:
-            result = await self._client.read_resource(_mcp_any_url(self._INDEX_URI))
+            result = await self._session_provider().read_resource(_mcp_any_url(self._INDEX_URI))
         except Exception as ex:
             if _is_mcp_resource_not_found(ex):
                 logger.debug("No skill://index.json resource available on MCP server: %s", ex)
@@ -4382,7 +4454,7 @@ def _try_create_skill(self, entry: _McpSkillIndexEntry) -> MCPSkill | None:
             logger.debug("Skipping entry '%s': invalid metadata: %s", entry.name, ex)
             return None
 
-        return MCPSkill(frontmatter=fm, skill_md_uri=entry.url, client=self._client)
+        return MCPSkill(frontmatter=fm, skill_md_uri=entry.url, session_provider=self._session_provider)
 
 
 # endregion