Python: [BREAKING] Make all SkillsProvider tools require approval by default

python/packages/core/AGENTS.md

@@ -74,7 +74,7 @@ agent_framework/
 - **`SkillResource`** - Named supplementary content attached to a skill; holds either static `content` or a dynamic `function` (sync or async). Exactly one must be provided.
 - **`SkillScript`** - An executable script attached to a skill; holds either an inline `function` (code-defined, runs in-process) or a `path` to a file on disk (file-based, delegated to a runner). Exactly one must be provided.
 - **`SkillScriptRunner`** - Protocol for file-based script execution. Any callable matching `(skill, script, args) -> Any` satisfies it. Code-defined scripts do not use a runner.
-- **`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.
+- **`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. 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. The tool names are also exposed as class constants (`LOAD_SKILL_TOOL_NAME`, `READ_SKILL_RESOURCE_TOOL_NAME`, `RUN_SKILL_SCRIPT_TOOL_NAME`).
 
 ### Model Context Protocol (`_mcp.py`)
 

python/packages/core/agent_framework/_skills.py

@@ -67,6 +67,7 @@
 
     from ._agents import SupportsAgentRun
     from ._sessions import AgentSession, SessionContext
+    from ._types import Content
 
 logger = logging.getLogger(__name__)
 
@@ -1790,6 +1791,17 @@ class SkillsProvider(ContextProvider):
     and file-based resource reads are guarded against path traversal and
     symlink escape.  Only use skills from trusted sources.
 
+    **Tool approval:** every tool exposed by this provider
+    (``load_skill``, ``read_skill_resource``, and ``run_skill_script``) is
+    registered with ``approval_mode="always_require"``, so each skill operation
+    needs approval.  To run unattended, pass one of the static
+    auto-approval rules to :class:`~agent_framework.ToolApprovalMiddleware` (via
+    ``auto_approval_rules``):
+    :meth:`read_only_tools_auto_approval_rule` approves only the read-only tools
+    (``load_skill`` and ``read_skill_resource``) while still prompting for
+    ``run_skill_script``, and :meth:`all_tools_auto_approval_rule` approves every
+    skill tool including script execution.
+
     Examples:
         File-based factory (recommended for single-source file skills):
 
@@ -1833,16 +1845,102 @@ class SkillsProvider(ContextProvider):
 
     Attributes:
         DEFAULT_SOURCE_ID: Default value for the ``source_id`` used by this provider.
+        LOAD_SKILL_TOOL_NAME: Name of the tool that loads a skill.
+        READ_SKILL_RESOURCE_TOOL_NAME: Name of the tool that reads a skill resource.
+        RUN_SKILL_SCRIPT_TOOL_NAME: Name of the tool that runs a skill script.
     """
 
     DEFAULT_SOURCE_ID: ClassVar[str] = "agent_skills"
 
+    #: Name of the tool that loads the full content of a skill.
+    LOAD_SKILL_TOOL_NAME: ClassVar[str] = "load_skill"
+    #: Name of the tool that reads a resource associated with a skill.
+    READ_SKILL_RESOURCE_TOOL_NAME: ClassVar[str] = "read_skill_resource"
+    #: Name of the tool that runs a script associated with a skill.
+    RUN_SKILL_SCRIPT_TOOL_NAME: ClassVar[str] = "run_skill_script"
+
+    #: Names of the tools that only read (never execute scripts from) the skills source.
+    _READ_ONLY_TOOL_NAMES: ClassVar[frozenset[str]] = frozenset({
+        LOAD_SKILL_TOOL_NAME,
+        READ_SKILL_RESOURCE_TOOL_NAME,
+    })
+
+    #: Names of all tools exposed by this provider.
+    _ALL_TOOL_NAMES: ClassVar[frozenset[str]] = frozenset({
+        LOAD_SKILL_TOOL_NAME,
+        READ_SKILL_RESOURCE_TOOL_NAME,
+        RUN_SKILL_SCRIPT_TOOL_NAME,
+    })
+
+    @staticmethod
+    def _is_local_tool_call(function_call: Content) -> bool:
+        """Return whether a function call targets this provider's local tools.
+
+        Hosted-tool calls carry a ``server_label`` in their
+        ``additional_properties`` and are a separate server-scoped approval
+        boundary that must be passed through untouched (see
+        :func:`agent_framework._tools._is_hosted_tool_approval`). These rules
+        only ever auto-approve the provider's own local tools, so any call that
+        carries a ``server_label`` is rejected even if its name collides with a
+        skill tool name.
+        """
+        return not function_call.additional_properties.get("server_label")
+
+    @staticmethod
+    def read_only_tools_auto_approval_rule(function_call: Content) -> bool:
+        """Auto-approval rule that approves only the read-only skill tools.
+
+        The tools exposed by :class:`SkillsProvider` always require approval.
+        Pass this rule to :class:`~agent_framework.ToolApprovalMiddleware` (via
+        ``auto_approval_rules``) to automatically approve the tools that read
+        skill content (``load_skill`` and ``read_skill_resource``), while still
+        prompting for script execution (``run_skill_script``).
+
+        Hosted-tool calls (those carrying a ``server_label``) are never
+        auto-approved, even when their name matches a skill tool, so the rule
+        stays scoped to this provider's local tools.
+
+        Args:
+            function_call: The pending ``function_call`` content.
+
+        Returns:
+            ``True`` for read-only skill tools, ``False`` otherwise so that
+            subsequent rules continue to be evaluated.
+        """
+        return (
+            SkillsProvider._is_local_tool_call(function_call)
+            and function_call.name in SkillsProvider._READ_ONLY_TOOL_NAMES
+        )
+
+    @staticmethod
+    def all_tools_auto_approval_rule(function_call: Content) -> bool:
+        """Auto-approval rule that approves every skill tool.
+
+        The tools exposed by :class:`SkillsProvider` always require approval.
+        Pass this rule to :class:`~agent_framework.ToolApprovalMiddleware` (via
+        ``auto_approval_rules``) to automatically approve every skill tool,
+        including the script execution tool (``run_skill_script``).
+
+        Hosted-tool calls (those carrying a ``server_label``) are never
+        auto-approved, even when their name matches a skill tool, so the rule
+        stays scoped to this provider's local tools.
+
+        Args:
+            function_call: The pending ``function_call`` content.
+
+        Returns:
+            ``True`` for any skill tool, ``False`` otherwise so that subsequent
+            rules continue to be evaluated.
+        """
+        return (
+            SkillsProvider._is_local_tool_call(function_call) and function_call.name in SkillsProvider._ALL_TOOL_NAMES
+        )
+
     def __init__(
         self,
         source: SkillsSource | Sequence[Skill] | Skill,
         *,
         instruction_template: str | None = None,
-        require_script_approval: bool = False,
         disable_caching: bool = False,
         source_id: str | None = None,
     ) -> None:
@@ -1869,22 +1967,21 @@ def __init__(
                 When omitted, those instructions are simply not included in the
                 rendered prompt (the corresponding tools are still registered).
                 Uses a built-in template when ``None``.
-            require_script_approval: When ``True``, skill script execution
-                requires explicit user approval before running. Instead of
-                executing immediately, the agent pauses and returns a
-                ``function_approval_request`` via ``result.user_input_requests``.
-                The application should present the request to the user, then
-                call ``request.to_function_approval_response(approved=True)``
-                (or ``False`` to reject) and pass the response back with
-                ``agent.run(approval_response, session=session)``.
-                Rejected scripts are not executed and the agent is informed
-                the user declined. Defaults to ``False``.  See
-                ``samples/02-agents/skills/script_approval/script_approval.py``
-                for the full approval loop pattern.
             disable_caching: When ``True``, rebuilds tools and instructions
                 from the source on every invocation instead of caching
                 after the first build.  Defaults to ``False``.
             source_id: Unique identifier for this provider instance.
+
+        .. note::
+
+            All skill tools require approval. To approve them
+            automatically, pass :meth:`read_only_tools_auto_approval_rule` or
+            :meth:`all_tools_auto_approval_rule` to
+            :class:`~agent_framework.ToolApprovalMiddleware`. See
+            ``samples/02-agents/skills/skills_auto_approval/skills_auto_approval.py``
+            for the auto-approval pattern and
+            ``samples/02-agents/skills/script_approval/script_approval.py`` for
+            the manual approval loop.
         """
         super().__init__(source_id or self.DEFAULT_SOURCE_ID)
 
@@ -1903,7 +2000,6 @@ def __init__(
 
         self._source = source
         self._instruction_template = instruction_template
-        self._require_script_approval = require_script_approval
         self._disable_caching = disable_caching
 
         # Lazy-initialized via _get_or_create_context / _create_context
@@ -1921,7 +2017,6 @@ def from_paths(
         script_filter: Callable[[str, str], bool] | None = None,
         resource_filter: Callable[[str, str], bool] | None = None,
         instruction_template: str | None = None,
-        require_script_approval: bool = False,
         disable_caching: bool = False,
         source_id: str | None = None,
     ) -> _TSkillsProvider:
@@ -1957,25 +2052,20 @@ def from_paths(
             instruction_template: Custom system-prompt template for
                 advertising skills.  Must contain a ``{skills}`` placeholder.
                 Uses a built-in template when ``None``.
-            require_script_approval: When ``True``, skill script execution
-                requires explicit user approval before running. Instead of
-                executing immediately, the agent pauses and returns a
-                ``function_approval_request`` via ``result.user_input_requests``.
-                The application should present the request to the user, then
-                call ``request.to_function_approval_response(approved=True)``
-                (or ``False`` to reject) and pass the response back with
-                ``agent.run(approval_response, session=session)``.
-                Rejected scripts are not executed and the agent is informed
-                the user declined. Defaults to ``False``.  See
-                ``samples/02-agents/skills/script_approval/script_approval.py``
-                for the full approval loop pattern.
             disable_caching: When ``True``, rebuilds tools and instructions
                 from the source on every invocation instead of caching
                 after the first build.
             source_id: Unique identifier for this provider instance.
 
         Returns:
             A configured :class:`SkillsProvider`.
+
+        .. note::
+
+            All skill tools require approval. To approve them
+            automatically, pass :meth:`read_only_tools_auto_approval_rule` or
+            :meth:`all_tools_auto_approval_rule` to
+            :class:`~agent_framework.ToolApprovalMiddleware`.
         """
         source = DeduplicatingSkillsSource(
             FileSkillsSource(
@@ -1991,7 +2081,6 @@ def from_paths(
         return cls(
             source,
             instruction_template=instruction_template,
-            require_script_approval=require_script_approval,
             disable_caching=disable_caching,
             source_id=source_id,
         )
@@ -2083,10 +2172,7 @@ async def _create_context(self) -> tuple[Sequence[Skill], str | None, list[Funct
             skills=skills,
         )
 
-        tools = self._create_tools(
-            skills=skills,
-            require_script_approval=self._require_script_approval,
-        )
+        tools = self._create_tools(skills=skills)
 
         return skills, instructions, tools
 
@@ -2153,18 +2239,19 @@ async def before_run(
     def _create_tools(
         self,
         skills: Sequence[Skill],
-        require_script_approval: bool = False,
     ) -> list[FunctionTool]:
         """Create the tool definitions for skill interaction.
 
         Always includes ``load_skill``, ``read_skill_resource``, and
-        ``run_skill_script``.
+        ``run_skill_script``.  Every tool is registered with
+        ``approval_mode="always_require"`` so each skill operation needs
+        approval; use :meth:`read_only_tools_auto_approval_rule` or
+        :meth:`all_tools_auto_approval_rule` with
+        :class:`~agent_framework.ToolApprovalMiddleware` to approve them
+        automatically.
 
         Args:
             skills: The skills to bind to tool handlers.
-            require_script_approval: When ``True``, the
-                ``run_skill_script`` tool pauses for user approval
-                before each invocation.
 
         Returns:
             A list of :class:`FunctionTool` instances.
@@ -2183,9 +2270,10 @@ async def _run_script(
 
         return [
             FunctionTool(
-                name="load_skill",
+                name=self.LOAD_SKILL_TOOL_NAME,
                 description="Loads the full instructions for a specific skill.",
                 func=_load,
+                approval_mode="always_require",
                 input_model={
                     "type": "object",
                     "properties": {
@@ -2195,9 +2283,10 @@ async def _run_script(
                 },
             ),
             FunctionTool(
-                name="read_skill_resource",
+                name=self.READ_SKILL_RESOURCE_TOOL_NAME,
                 description=("Reads a resource associated with a skill, such as references, assets, or dynamic data."),
                 func=_read_resource,
+                approval_mode="always_require",
                 input_model={
                     "type": "object",
                     "properties": {
@@ -2211,10 +2300,10 @@ async def _run_script(
                 },
             ),
             FunctionTool(
-                name="run_skill_script",
+                name=self.RUN_SKILL_SCRIPT_TOOL_NAME,
                 description="Runs a script associated with a skill.",
                 func=_run_script,
-                approval_mode="always_require" if require_script_approval else "never_require",
+                approval_mode="always_require",
                 input_model={
                     "type": "object",
                     "properties": {