diff --git a/python/packages/core/AGENTS.md b/python/packages/core/AGENTS.md index 18540ee21fd..810ea2f5af1 100644 --- a/python/packages/core/AGENTS.md +++ b/python/packages/core/AGENTS.md @@ -170,8 +170,8 @@ The vector store API is experimental under the shared `VECTOR_STORES` feature ID - **`Skill`** - Abstract base for a skill definition bundling instructions (`content`) with frontmatter metadata, resources, and scripts. Concrete subclasses (`InlineSkill`, `FileSkill`, `ClassSkill`) accept a `frontmatter=SkillFrontmatter(...)` argument carrying the spec fields. Adding new spec fields is done in one place — on `SkillFrontmatter` — keeping the subclass constructors stable. - **`SkillFrontmatter`** - L1 discovery metadata for a skill (`name`, `description`, `license`, `compatibility`, `allowed_tools`, `metadata`). All fields are mutable plain attributes; the constructor validates `name`, `description`, and `compatibility` against the spec but post-construction assignments are not re-validated. Spec fields are reachable on every skill via `skill.frontmatter`. - **`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. +- **`SkillScript`** - An executable script attached to a skill; inline scripts run in-process, while file-based scripts are delegated to a runner. Inline callbacks can declare a `FunctionInvocationContext` parameter (hidden from the schema) to access host values separately through `ctx.kwargs`. Custom `run()` overrides opt in with the same annotation; unannotated overrides retain runtime keyword forwarding. +- **`SkillScriptRunner`** - Protocol for file-based script execution. Any callable matching `(skill, script, args) -> Any` satisfies it. Code-defined scripts do not use a runner. Runners can opt into context injection with an additional keyword-bindable `FunctionInvocationContext` parameter that has a default value. - **`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`). - **`FileSkillsSource`** - `SkillsSource` that discovers file-based skills by scanning configured root paths for `SKILL.md`. The **configured root paths define the trust boundary** and are used as given (a root may itself be a symlink); everything discovered *below* a root is link-checked and fails closed. `_discover_skill_directories` rejects any entry that is a symbolic link, junction, or other reparse point (via the shared `agent_framework._filesystem.is_link_or_reparse_point` helper) before descending into it, and rejects a directory whose `SKILL.md` is itself such a link — otherwise a link planted under a root would be adopted as the skill root, and since every later guard treats the skill root as the boundary and only inspects segments below it, the link itself would never be inspected. Resource and script discovery apply the same rule per path segment via `_has_link_or_reparse_point_in_path`; source-discovered resources and scripts retain a `_SkillPathScope` pairing their configured root with their skill directory, and repeat containment, regular-file, and link checks immediately before reading or execution — scanning every segment from the configured root down to the file, so a skill directory (or any directory between it and the root) swapped for a link after discovery is rejected too. An `OSError` while inspecting an entry is treated as unsafe (skip / reject), never as "safe". diff --git a/python/packages/core/agent_framework/_skills.py b/python/packages/core/agent_framework/_skills.py index 2efce44a0be..71973281c25 100644 --- a/python/packages/core/agent_framework/_skills.py +++ b/python/packages/core/agent_framework/_skills.py @@ -60,17 +60,35 @@ from dataclasses import dataclass from datetime import timedelta from enum import Enum -from functools import lru_cache +from functools import lru_cache, partial from html import escape as xml_escape from pathlib import Path, PurePosixPath -from typing import IO, TYPE_CHECKING, Any, ClassVar, Final, Protocol, TypeAlias, TypeVar, cast, runtime_checkable +from types import UnionType +from typing import ( + IO, + TYPE_CHECKING, + Any, + ClassVar, + Final, + Protocol, + TypeAlias, + TypeVar, + Union, + cast, + get_args, + get_origin, + get_type_hints, + runtime_checkable, +) from urllib.parse import unquote import yaml +from typing_extensions import Sentinel from yaml.nodes import MappingNode, Node, ScalarNode from ._feature_stage import ExperimentalFeature, experimental from ._filesystem import _is_link_or_reparse_point # pyright: ignore[reportPrivateUsage] +from ._middleware import FunctionInvocationContext from ._sessions import ContextProvider from ._telemetry import FeatureIndex, mark_feature_used from ._tools import ApprovalMode, FunctionTool @@ -81,12 +99,14 @@ from pydantic import AnyUrl from ._agents import SupportsAgentRun - from ._middleware import FunctionInvocationContext from ._sessions import AgentSession, SessionContext from ._types import Content logger = logging.getLogger(__name__) +# Distinguish an omitted keyword from an explicit application ``context=None``. +_UNSET_CONTEXT: Any = Sentinel("_UNSET_CONTEXT") + # region Models @@ -382,6 +402,13 @@ def parameters_schema(self) -> dict[str, Any] | None: async def run(self, skill: Skill, args: dict[str, Any] | list[str] | None = None, **kwargs: Any) -> Any: """Run this script. + Implementations can receive the enclosing ``run_skill_script`` invocation + by declaring a keyword-bindable ``FunctionInvocationContext`` parameter + after ``skill`` and ``args``. Without that annotation, host runtime + values are passed as individual keyword arguments. The invocation's + ``kwargs`` contain host values; its ``arguments`` contain the outer + tool-call arguments, not just the script arguments. + Args: skill: The skill that owns this script. args: Optional arguments for the script, provided by the @@ -396,21 +423,104 @@ async def run(self, skill: Skill, args: dict[str, Any] | list[str] | None = None """ +def _find_context_parameter( + function: Callable[..., Any], signature: inspect.Signature, *, script_name: str +) -> str | None: + """Return the name of the callable's ``FunctionInvocationContext`` parameter, if any. + + Raises: + ValueError: If multiple context parameters are declared, or the context + parameter cannot be passed by keyword. + """ + try: + type_hints = get_type_hints(function, include_extras=True) + except (AttributeError, NameError, TypeError): + logger.debug("Could not resolve annotations for script '%s'; using signature annotations.", script_name) + type_hints = {} + + context_params: list[inspect.Parameter] = [] + for param in signature.parameters.values(): + annotation = type_hints.get(param.name, param.annotation) + + # Only unwrap unions such as ``FunctionInvocationContext | None``, not containers like ``list[...]``. + candidates = get_args(annotation) if get_origin(annotation) in (Union, UnionType) else (annotation,) + + # Recognize context types and simple forward references. + if any( + candidate is FunctionInvocationContext or candidate == "FunctionInvocationContext" + for candidate in candidates + ): + context_params.append(param) + + # Callbacks without a context parameter keep their existing behavior. + if not context_params: + return None + + # Only one parameter can receive the invocation context. + if len(context_params) > 1: + raise ValueError(f"Script '{script_name}' defines multiple FunctionInvocationContext parameters.") + + param = context_params[0] + + # Context injection binds by keyword, not by position. + if param.kind not in (inspect.Parameter.POSITIONAL_OR_KEYWORD, inspect.Parameter.KEYWORD_ONLY): + raise ValueError(f"Context parameter '{param.name}' on script '{script_name}' must accept a keyword.") + + return param.name + + +def _find_run_context_parameter(script_type: type[SkillScript]) -> str | None: + """Find an opted-in context parameter on a script's ``run`` method.""" + try: + signature = inspect.signature(script_type.run) + except (TypeError, ValueError): + logger.debug("Could not inspect run for script '%s'; calling without context.", script_type.__name__) + return None + + context_name = _find_context_parameter(script_type.run, signature, script_name=script_type.__name__) + if context_name is not None: + # Reject context parameters that replace skill or args. + try: + signature.bind(None, None, None, **{context_name: None}) + except TypeError as exc: + raise ValueError( + f"Context parameter '{context_name}' on script '{script_type.__name__}' " + f"must support run(skill, args, {context_name}=...) without replacing skill or args." + ) from exc + return context_name + + class InlineSkillScript(SkillScript): """A code-defined skill script backed by a callable. The callable is invoked directly in-process when the script is run. Parameters schema is lazily generated from the callable's signature. + Declare a ``FunctionInvocationContext`` parameter on the callable to receive + the enclosing tool invocation from :class:`SkillsProvider`. That parameter is + hidden from the schema, and runtime values are available only through + ``ctx.kwargs``; ordinary parameters and ``**kwargs`` receive only model + arguments. + Attributes: name: Script identifier. description: Optional human-readable summary, or ``None``. function: Callable that implements the script. Examples: + Receiving runtime context separately from model arguments: + .. code-block:: python - InlineSkillScript(name="analyze", function=analyze_data, description="Run analysis") + from agent_framework import FunctionInvocationContext + + + def analyze(query: str, *, ctx: FunctionInvocationContext) -> str: + tenant_id = ctx.kwargs["tenant_id"] + return f"Analyzing {query} for {tenant_id}" + + + InlineSkillScript(name="analyze", function=analyze) """ def __init__( @@ -427,6 +537,8 @@ def __init__( name: Identifier for this script (e.g. ``"analyze"``). description: Optional human-readable summary. function: Callable (sync or async) that implements the script. + Declare one keyword-bindable ``FunctionInvocationContext`` + parameter to receive runtime context. argument_parser: Optional callable that converts the raw ``args`` value into the named arguments passed to ``function`` before the script runs. When ``None`` (the @@ -434,6 +546,10 @@ def __init__( ``dict`` (or ``None``). Supply a parser to support backends that send arguments in a non-conforming shape (for example, vLLM-style JSON strings). + + Raises: + ValueError: If multiple context parameters are declared, or a + context parameter cannot be passed by keyword. """ super().__init__(name=name, description=description) @@ -442,10 +558,10 @@ def __init__( self._parameters_schema: dict[str, Any] | None = None self._parameters_schema_resolved: bool = False - # Precompute whether the function accepts **kwargs to avoid - # repeated inspect.signature() calls on every invocation. + # Precompute signature facts to avoid inspecting the function on every invocation. sig = inspect.signature(function) self._accepts_kwargs = any(p.kind == inspect.Parameter.VAR_KEYWORD for p in sig.parameters.values()) + self._context_parameter_name = _find_context_parameter(function, sig, script_name=name) @property def parameters_schema(self) -> dict[str, Any] | None: @@ -461,7 +577,14 @@ def parameters_schema(self) -> dict[str, Any] | None: self._parameters_schema_resolved = True return self._parameters_schema - async def run(self, skill: Skill, args: dict[str, Any] | list[str] | str | None = None, **kwargs: Any) -> Any: + async def run( + self, + skill: Skill, + args: dict[str, Any] | list[str] | str | None = None, + *, + context: FunctionInvocationContext | None = _UNSET_CONTEXT, + **kwargs: Any, + ) -> Any: """Run the script by invoking the callable in-process. When an ``argument_parser`` is configured, it is applied to @@ -477,6 +600,8 @@ async def run(self, skill: Skill, args: dict[str, Any] | list[str] | str | None be a ``dict`` or ``None``; a ``list`` raises :class:`TypeError` because inline scripts bind arguments by keyword name. + context: The enclosing invocation when supplied by the provider. + Other values are forwarded to the callback unchanged. **kwargs: Runtime keyword arguments forwarded only to script functions that accept ``**kwargs``. @@ -488,7 +613,48 @@ async def run(self, skill: Skill, args: dict[str, Any] | list[str] | str | None ``list``. A leftover ``str`` means no ``argument_parser`` converted it; a ``list`` is array-style and only supported for file-based scripts. + ValueError: If model arguments supply the injected context parameter. """ + call_args = self._parse_arguments(args) or {} + + # Unannotated overrides may forward application "context" values, even invocation objects. + invocation = ( + context + if isinstance(context, FunctionInvocationContext) and _find_run_context_parameter(type(self)) is not None + else None + ) + + runtime_kwargs: Mapping[str, Any] = {} + if invocation is None: + # Preserve explicit application "context" values as data. + if context is not _UNSET_CONTEXT: + kwargs = {"context": context, **kwargs} + elif self._context_parameter_name is None: + # Callback without context: pass host values as separate keywords. + runtime_kwargs = invocation.kwargs + elif self._context_parameter_name in call_args: + # Reject model arguments that impersonate runtime context. + raise ValueError( + f"Argument '{self._context_parameter_name}' for inline script '{self.name}' " + "is reserved for runtime context injection." + ) + else: + # Opted-in callback: inject without changing model arguments. + call_args = {**call_args, self._context_parameter_name: invocation} + + # Keep separate keyword expansions so duplicate model and host names still fail. + if self._accepts_kwargs: + result = self.function(**call_args, **runtime_kwargs, **kwargs) + else: + result = self.function(**call_args) + + # Support both synchronous and asynchronous callbacks. + if inspect.isawaitable(result): + return await result + + return result + + def _parse_arguments(self, args: dict[str, Any] | list[str] | str | None) -> dict[str, Any] | None: if self.argument_parser is not None: args = self.argument_parser(args) if isinstance(args, str): @@ -503,13 +669,7 @@ async def run(self, skill: Skill, args: dict[str, Any] | list[str] | str | None f"but received a list. Array-style arguments are only supported " f"for file-based scripts." ) - if self._accepts_kwargs: # ruff:ignore[if-else-block-instead-of-if-exp] - result = self.function(**(args or {}), **kwargs) - else: - result = self.function(**(args or {})) - if inspect.isawaitable(result): - return await result - return result + return args class FileSkillScript(SkillScript): @@ -518,6 +678,11 @@ class FileSkillScript(SkillScript): Represents a script file on disk that is delegated to a configured :class:`SkillScriptRunner` for execution. + Runners can declare a defaulted ``FunctionInvocationContext`` parameter + in addition to ``skill``, ``script``, and ``args``. :class:`SkillsProvider` + injects the enclosing invocation into that parameter without changing + script arguments or forwarding host values to subprocesses. + Attributes: name: Script identifier. description: Optional human-readable summary, or ``None``. @@ -547,6 +712,7 @@ def __init__( full_path: Absolute path to the script file. runner: Strategy for running file-based scripts. Required for execution; an error is raised from :meth:`run` if not provided. + See :class:`SkillScriptRunner` for optional context injection. skill_dir: Trusted skill directory used to revalidate the script before execution. trusted_root: Configured discovery root used to include the skill directory and intermediate directories in revalidation. Requires ``skill_dir``. When omitted, @@ -555,7 +721,9 @@ def __init__( Raises: ValueError: If ``full_path`` is empty or not an absolute path, if ``trusted_root`` is provided without ``skill_dir``, or if ``skill_dir`` - does not reside at or beneath ``trusted_root``. + does not reside at or beneath ``trusted_root``, or if the runner's + context parameter is not optional and keyword-bindable or replaces + one of its three existing arguments. """ super().__init__(name=name, description=description) @@ -576,6 +744,9 @@ def __init__( if skill_dir is not None else None ) + self._context_parameter_name = ( + self._find_runner_context_parameter(runner, script_name=name) if runner is not None else None + ) @property def parameters_schema(self) -> dict[str, Any] | None: @@ -587,13 +758,21 @@ def parameters_schema(self) -> dict[str, Any] | None: """ return {"type": "array", "items": {"type": "string"}} - async def run(self, skill: Skill, args: dict[str, Any] | list[str] | None = None, **kwargs: Any) -> Any: + async def run( + self, + skill: Skill, + args: dict[str, Any] | list[str] | None = None, + *, + context: FunctionInvocationContext | None = None, + **kwargs: Any, + ) -> Any: """Run the script by delegating to the configured runner. Args: skill: The skill that owns this script. Must be a :class:`FileSkill`. args: Optional arguments for the script. + context: Optional enclosing tool invocation, passed to context-aware runners. **kwargs: Additional runtime keyword arguments (unused). Returns: @@ -601,14 +780,19 @@ async def run(self, skill: Skill, args: dict[str, Any] | list[str] | None = None Raises: TypeError: If ``skill`` is not a :class:`FileSkill`. - ValueError: If no runner was provided. + ValueError: If no runner was provided or the discovered file is no longer valid. """ + # Require a file-backed skill for runner execution. if not isinstance(skill, FileSkill): raise TypeError( f"File-based script '{self.name}' requires a FileSkill but received '{type(skill).__name__}'." ) + + # File scripts need a runner to execute them, e.g. a subprocess runner. if self._runner is None: raise ValueError(f"Script '{self.name}' requires a runner. Provide a script_runner for file-based scripts.") + + # Recheck discovered paths: e.g. scripts/run.py may have been replaced by a symlink. if self._scope is not None: await asyncio.to_thread( FileSkillsSource._validate_file_path_for_use, # pyright: ignore[reportPrivateUsage] @@ -617,11 +801,73 @@ async def run(self, skill: Skill, args: dict[str, Any] | list[str] | None = None self.name, "Script", ) - result = self._runner(skill, self, args) + + # The protocol declares only three arguments; the runner's context slot was validated at construction. + runner = cast(Callable[..., Any], self._runner) + + # Unannotated overrides may forward application "context" values, even invocation objects. + runner_kwargs: dict[str, FunctionInvocationContext] = {} + if ( + self._context_parameter_name is not None + and isinstance(context, FunctionInvocationContext) + and _find_run_context_parameter(type(self)) is not None + ): + runner_kwargs[self._context_parameter_name] = context + + # E.g. runner(skill, self, ["input.txt"], ctx=context); host values stay in context.kwargs. + result = runner(skill, self, args, **runner_kwargs) + + # Support both synchronous and asynchronous runners. if inspect.isawaitable(result): return await result + return result + @staticmethod + def _find_runner_context_parameter(runner: SkillScriptRunner, *, script_name: str) -> str | None: + """Find a context parameter without changing the runner's three-argument contract.""" + try: + signature = inspect.signature(runner) + except (TypeError, ValueError): + logger.debug("Could not inspect runner for script '%s'; calling without context.", script_name) + return None + + function: Callable[..., Any] = runner + while isinstance(function, partial): + function = function.func + + # Callable objects declare parameter annotations on __call__. + if not inspect.isroutine(function): + function = function.__call__ + + context_name = _find_context_parameter(function, signature, script_name=script_name) + + # Keep context injection opt-in for existing runners. + if context_name is None: + return None + + parameter = signature.parameters[context_name] + positional_names = [ + name + for name, param in signature.parameters.items() + if param.kind in (inspect.Parameter.POSITIONAL_ONLY, inspect.Parameter.POSITIONAL_OR_KEYWORD) + ] + + # Preserve the runner's positional skill, script, and args parameters. + if context_name in positional_names[:3]: + raise ValueError( + f"Runner context parameter '{context_name}' for script '{script_name}' " + "must not replace the skill, script, or args parameters." + ) + + # Runners must remain callable without an injected context. + if parameter.default is inspect.Parameter.empty: + raise ValueError( + f"Runner context parameter '{context_name}' for script '{script_name}' must have a default value." + ) + + return context_name + class Skill(ABC): """Abstract base class for all agent skills. @@ -1393,8 +1639,11 @@ def script( When applied to a method on a :class:`ClassSkill` subclass, the method is automatically discovered and registered as an :class:`InlineSkillScript`. - The method's parameters (excluding ``self``) are used to generate a JSON + The method's parameters (excluding ``self`` and any injected + ``FunctionInvocationContext`` parameter) are used to generate a JSON schema, and the method is invoked in-process when the script is run. + Declare a ``FunctionInvocationContext`` parameter as described by + :class:`InlineSkillScript` to receive host context separately. Supports bare usage (``@ClassSkill.script``) and parameterized usage (``@ClassSkill.script(name="custom", description="...")``). @@ -1702,7 +1951,17 @@ class SkillScriptRunner(Protocol): are always executed **in-process** and do not use a script runner. Any callable (sync or async) matching the ``__call__`` signature - satisfies this protocol. + satisfies this protocol. To opt into context injection, add one keyword-bindable + parameter annotated with ``FunctionInvocationContext`` (or ``FunctionInvocationContext | None``). + It must have a default value and must not replace ``skill``, ``script``, or + ``args``. For example: + + .. code-block:: python + + from agent_framework import FunctionInvocationContext + + + def runner(skill, script, args=None, *, ctx: FunctionInvocationContext | None = None): ... """ def __call__( @@ -2482,7 +2741,8 @@ async def _load(skill_name: str) -> str: # kwargs, so a script's ``**kwargs`` can also hold model-supplied entries that are # not declared parameters. Resources take no model arguments, so a resource's # ``**kwargs`` is runtime-only. Do not treat a name appearing in a script's - # ``**kwargs`` as proof the host supplied it. + # ``**kwargs`` as proof the host supplied it. Inline scripts that declare a + # FunctionInvocationContext parameter receive host values only through it. async def _read_resource( ctx: FunctionInvocationContext, skill_name: str, @@ -2506,7 +2766,7 @@ async def _run_script( skill_name, script_name, args, - runtime_kwargs=ctx.kwargs, + context=ctx, ) return [ @@ -2636,12 +2896,13 @@ async def _run_skill_script( script_name: str, args: dict[str, Any] | list[str] | None = None, *, - runtime_kwargs: Mapping[str, Any] | None = None, + context: FunctionInvocationContext, ) -> Any: """Run a named script from a skill. Resolves the skill and script by name, then delegates execution - to :meth:`SkillScript.run`. + to :meth:`SkillScript.run`. Implementations can opt into receiving the + invocation with an annotated ``FunctionInvocationContext`` parameter. Args: skills: The skills to look up the skill from. @@ -2649,19 +2910,8 @@ async def _run_skill_script( script_name: The script name to look up (case-insensitive). args: Optional arguments for the script, provided by the agent/LLM. - runtime_kwargs: Runtime keyword arguments forwarded only to script - functions that accept ``**kwargs``. This parameter carries host - request context supplied via - ``agent.run(..., function_invocation_kwargs={"user_id": "123"})`` - and delivered through :class:`FunctionInvocationContext`; it is - never sourced from model-supplied tool arguments. Note that the - receiving script's own ``**kwargs`` is not runtime-only: these - values are expanded alongside any undeclared entries in *args*, - which the model supplies. Taking these as one mapping keeps this - helper's own parameter names (``skills``, ``skill_name``, - ``script_name``, ``args``) usable as runtime kwarg names; the - public :meth:`SkillScript.run` signature still expands them, so - ``skill`` and ``args`` remain reserved at that boundary. + context: The enclosing tool invocation, passed to an annotated + parameter or unpacked as host runtime keyword arguments. Returns: The script result. Returns a user-facing error string for @@ -2687,7 +2937,13 @@ async def _run_skill_script( return f"Error: Script '{script_name}' not found in skill '{skill_name}'." try: - return await script.run(skill, args, **(runtime_kwargs or {})) + context_name = _find_run_context_parameter(type(script)) + if context_name is not None: + # Opted-in run: pass the whole invocation. + return await script.run(skill, args, **{context_name: context}) + + # Unannotated run: pass host values individually. + return await script.run(skill, args, **context.kwargs) except Exception: logger.exception("Error running script '%s' in skill '%s'", script_name, skill_name) raise diff --git a/python/packages/core/tests/core/test_skills.py b/python/packages/core/tests/core/test_skills.py index f15d6d93038..3db8f7c27a1 100644 --- a/python/packages/core/tests/core/test_skills.py +++ b/python/packages/core/tests/core/test_skills.py @@ -4,13 +4,16 @@ from __future__ import annotations +import asyncio +import inspect import os import shutil from abc import ABC -from collections.abc import Sequence +from collections.abc import Callable, MutableSequence, Sequence from datetime import timedelta +from functools import partial, wraps from pathlib import Path -from typing import Any +from typing import Annotated, Any from unittest.mock import AsyncMock, patch import pytest @@ -21,12 +24,14 @@ CachingSkillsSource, ChatOptions, ChatResponse, + ChatResponseUpdate, ClassSkill, Content, DeduplicatingSkillsSource, FileSkill, FileSkillScript, FileSkillsSource, + FunctionTool, InlineSkill, InMemorySkillsSource, Message, @@ -1860,7 +1865,8 @@ def test_discover_skips_symlinked_script(self, tmp_path: Path) -> None: assert "scripts/safe.py" in discovered assert "scripts/leak.py" not in discovered - async def test_run_rejects_script_replaced_with_symlink(self, tmp_path: Path) -> None: + @pytest.mark.parametrize("context_aware", [False, True]) + async def test_run_rejects_script_replaced_with_symlink(self, tmp_path: Path, context_aware: bool) -> None: """A script replaced after discovery must be revalidated before its runner is invoked.""" skill_dir = _write_skill(tmp_path, "my-skill") script_path = skill_dir / "scripts" / "run.py" @@ -1870,7 +1876,13 @@ async def test_run_rejects_script_replaced_with_symlink(self, tmp_path: Path) -> outside_script.write_text("print('outside')", encoding="utf-8") runner_called = False - def runner(skill: Skill, script: SkillScript, args: dict[str, Any] | list[str] | None = None) -> None: + def runner( + skill: Skill, + script: SkillScript, + args: dict[str, Any] | list[str] | None = None, + *, + ctx: FunctionInvocationContext | None = None, + ) -> None: nonlocal runner_called runner_called = True @@ -1882,7 +1894,10 @@ def runner(skill: Skill, script: SkillScript, args: dict[str, Any] | list[str] | script_path.symlink_to(outside_script) with pytest.raises(ValueError, match="symbolic link or reparse point"): - await script.run(skill) + if context_aware: + await script.run(skill, context=_script_context()) + else: + await script.run(skill) assert runner_called is False async def test_discover_skips_symlinked_skill_directory(self, tmp_path: Path) -> None: @@ -4111,9 +4126,690 @@ def test_runner_none_by_default(self) -> None: assert script._runner is None +def _script_context(**runtime_kwargs: Any) -> FunctionInvocationContext: + """Build a ``run_skill_script`` invocation context carrying host runtime values.""" + return _invocation_context(FunctionTool(name="run_skill_script", func=lambda: None), **runtime_kwargs) + + +async def _run_through_provider( + script: SkillScript, + skill: InlineSkill | FileSkill, + args: Any = None, + *, + context: FunctionInvocationContext, +) -> Any: + if script not in skill._scripts: + skill._scripts.append(script) + return await SkillsProvider(skill)._run_skill_script( + [skill], skill.frontmatter.name, script.name, args, context=context + ) + + +async def _run_with_context( + function: Any, + args: Any = None, + *, + context: FunctionInvocationContext | None = None, + argument_parser: SkillScriptArgumentParser | None = None, +) -> Any: + """Run ``function`` as an inline script through the provider.""" + script = InlineSkillScript(name="analyze", function=function, argument_parser=argument_parser) + skill = InlineSkill(frontmatter=SkillFrontmatter(name="s", description="d"), instructions="c") + return await _run_through_provider(script, skill, args, context=context or _script_context()) + + +def _context_callback(value: str, *, ctx: FunctionInvocationContext) -> Any: + return value, ctx, {} + + +def _context_callback_with_kwargs(value: str, ctx: FunctionInvocationContext, **kwargs: Any) -> Any: + return value, ctx, kwargs + + +async def _async_context_callback(value: str, *, ctx: FunctionInvocationContext) -> Any: + return _context_callback(value, ctx=ctx) + + +async def _async_context_callback_with_kwargs(value: str, ctx: FunctionInvocationContext, **kwargs: Any) -> Any: + return _context_callback_with_kwargs(value, ctx, **kwargs) + + +def _inline_context_skill(argument_parser: SkillScriptArgumentParser) -> Skill: + skill = InlineSkill( + frontmatter=SkillFrontmatter(name="s", description="d"), instructions="Body", argument_parser=argument_parser + ) + + @skill.script + def analyze(value: str, *, ctx: FunctionInvocationContext) -> Any: + return value, ctx + + return skill + + +class _ContextClassSkill(ClassSkill): + @property + def instructions(self) -> str: + return "Body" + + @ClassSkill.script + def analyze(self, value: str, *, ctx: FunctionInvocationContext) -> Any: + return value, ctx + + +class TestInlineSkillScriptContext: + """Opt-in runtime context injection for inline callbacks.""" + + @pytest.mark.parametrize( + ("callback", "model_extras"), + [ + (_context_callback, {}), + (_async_context_callback, {}), + (_context_callback_with_kwargs, {"tenant_id": "model-tenant", "context": "model-context"}), + (_async_context_callback_with_kwargs, {"tenant_id": "model-tenant", "context": "model-context"}), + ], + ) + async def test_injects_context_without_merging_host_values( + self, callback: Any, model_extras: dict[str, Any] + ) -> None: + runtime_kwargs = { + "tenant_id": "host-tenant", + "value": "host", + "ctx": "host-ctx", + "skill": "host-skill", + "args": "host-args", + "context": "host-context", + } + context = _script_context(**runtime_kwargs) + args = {"value": "model", **model_extras} + + value, received_context, kwargs = await _run_with_context(callback, args, context=context) + + assert value == "model" + assert received_context is context + assert kwargs == model_extras + assert args == {"value": "model", **model_extras} + assert context.kwargs == runtime_kwargs + assert context.result is None + + @pytest.mark.parametrize( + "annotation", + [ + FunctionInvocationContext, + FunctionInvocationContext | None, + "FunctionInvocationContext", + "FunctionInvocationContext | None", + ], + ) + async def test_context_annotations_are_injected_and_hidden_from_schema(self, annotation: Any) -> None: + def callback(value: str, *, invocation: Any) -> Any: + return invocation + + callback.__annotations__["invocation"] = annotation + context = _script_context() + + schema = InlineSkillScript(name="analyze", function=callback).parameters_schema + assert schema is not None + assert set(schema["properties"]) == {"value"} + assert schema["required"] == ["value"] + assert await _run_with_context(callback, {"value": "model"}, context=context) is context + + @pytest.mark.parametrize( + "annotation", + [ + "FunctionInvocationContext", + "FunctionInvocationContext | None", + ], + ) + async def test_context_only_callback_has_no_model_parameters(self, annotation: str) -> None: + def callback(ctx: FunctionInvocationContext) -> Any: + return ctx + + callback.__annotations__["ctx"] = annotation + context = _script_context() + + assert InlineSkillScript(name="analyze", function=callback).parameters_schema is None + assert await _run_with_context(callback, context=context) is context + + @pytest.mark.parametrize( + "annotation", + [ + list[FunctionInvocationContext], + Callable[..., FunctionInvocationContext], + Annotated[FunctionInvocationContext, "runtime"], + "list[FunctionInvocationContext]", + "Annotated[FunctionInvocationContext, 'runtime']", + ], + ) + async def test_non_context_annotations_are_not_injected(self, annotation: Any) -> None: + def callback(value: Any) -> Any: + return value + + callback.__annotations__["value"] = annotation + + assert InlineSkillScript(name="analyze", function=callback)._context_parameter_name is None # pyright: ignore[reportPrivateUsage] + assert await _run_with_context(callback, {"value": "model"}) == "model" + + @pytest.mark.parametrize("use_parser", [False, True]) + async def test_script_arguments_cannot_supply_context(self, use_parser: bool) -> None: + def callback(*, ctx: FunctionInvocationContext) -> None: + pytest.fail("Injected context must not be supplied as a script argument") + + args = {"ctx": "model"} + with pytest.raises(ValueError, match="'ctx'.*reserved for runtime context injection"): + await _run_with_context( + callback, + {"raw": "value"} if use_parser else args, + argument_parser=(lambda _: args) if use_parser else None, + ) + + @pytest.mark.parametrize( + ("raw_args", "expected"), + [('{"value": "parsed"}', "parsed"), (["parsed"], "parsed"), ({"value": "parsed"}, "parsed"), (None, "default")], + ) + async def test_argument_parser_runs_once_before_injection(self, raw_args: Any, expected: str) -> None: + parser_calls: list[Any] = [] + + def parser(args: Any) -> dict[str, Any] | None: + parser_calls.append(args) + return None if args is None else {"value": "parsed"} + + def callback(value: str = "default", *, ctx: FunctionInvocationContext) -> str: + return value + + assert await _run_with_context(callback, raw_args, argument_parser=parser) == expected + assert parser_calls == [raw_args] + + @pytest.mark.parametrize("use_parser", [False, True]) + async def test_context_injection_leaves_reused_arguments_unchanged(self, use_parser: bool) -> None: + def callback(value: str, *, ctx: FunctionInvocationContext) -> Any: + return value, ctx + + args = {"value": "model"} + script = InlineSkillScript( + name="analyze", function=callback, argument_parser=(lambda _: args) if use_parser else None + ) + skill = InlineSkill(frontmatter=SkillFrontmatter(name="s", description="d"), instructions="c") + + for tenant in ("first", "second"): + context = _script_context(tenant_id=tenant) + value, received_context = await script.run(skill, args, context=context) + assert value == "model" + assert received_context is context + assert args == {"value": "model"} + + @pytest.mark.parametrize( + ("invalid_args", "message"), [("unparsed", "argument_parser"), (["array"], "requires keyword arguments")] + ) + @pytest.mark.parametrize("use_parser", [False, True]) + async def test_invalid_argument_shapes_are_rejected( + self, invalid_args: Any, message: str, use_parser: bool + ) -> None: + def callback(ctx: FunctionInvocationContext) -> None: + pytest.fail("Invalid arguments must not execute the script") + + with pytest.raises(TypeError, match=message): + await _run_with_context( + callback, + None if use_parser else invalid_args, + argument_parser=(lambda _: invalid_args) if use_parser else None, + ) + + async def test_direct_run_keeps_existing_behavior(self) -> None: + def callback(ctx: FunctionInvocationContext | None = None, **kwargs: Any) -> Any: + return ctx, kwargs + + script = InlineSkillScript(name="analyze", function=callback) + skill = InlineSkill(frontmatter=SkillFrontmatter(name="s", description="d"), instructions="c") + context = _script_context() + + assert await script.run(skill, {"value": "model"}, tenant_id="host") == ( + None, + {"value": "model", "tenant_id": "host"}, + ) + received_context, kwargs = await script.run(skill, {"ctx": context}, tenant_id="host") + assert received_context is context + assert kwargs == {"tenant_id": "host"} + + @pytest.mark.parametrize("value", [None, "customer information", False, 0, _script_context()]) + @pytest.mark.parametrize("through_provider", [False, True]) + async def test_forwarded_context_keyword_reaches_callback(self, value: Any, through_provider: bool) -> None: + class PlainScript(InlineSkillScript): + async def run(self, skill: Skill, args: Any = None, **kwargs: Any) -> Any: + return await super().run(skill, args, **kwargs) + + script = PlainScript(name="analyze", function=lambda **kwargs: kwargs) + skill = InlineSkill(frontmatter=SkillFrontmatter(name="s", description="d"), instructions="c") + if through_provider: + result = await _run_through_provider(script, skill, context=_script_context(context=value)) + else: + result = await script.run(skill, context=value) + assert result == {"context": value} + + async def test_forwarded_context_keyword_preserves_duplicate_argument_error(self) -> None: + class PlainScript(InlineSkillScript): + async def run(self, skill: Skill, args: Any = None, **kwargs: Any) -> Any: + return await super().run(skill, args, **kwargs) + + script = PlainScript(name="analyze", function=lambda **kwargs: pytest.fail("Callback should not run")) + skill = InlineSkill(frontmatter=SkillFrontmatter(name="s", description="d"), instructions="c") + with pytest.raises(TypeError, match="multiple values"): + await _run_through_provider(script, skill, {"context": "model"}, context=_script_context(context=None)) + + async def test_unannotated_context_name_does_not_opt_in(self) -> None: + def callback(ctx: str, **kwargs: Any) -> Any: + return ctx, kwargs + + schema = InlineSkillScript(name="analyze", function=callback).parameters_schema + assert schema is not None + assert set(schema["properties"]) == {"ctx"} + assert await _run_with_context(callback, {"ctx": "model"}, context=_script_context(tenant_id="host")) == ( + "model", + {"tenant_id": "host"}, + ) + + @pytest.mark.parametrize("reject", [False, True]) + async def test_annotated_callback_does_not_bypass_unannotated_run_override(self, reject: bool) -> None: + calls: list[str] = [] + failure = RuntimeError("Rejected by the run override") + runtime_kwargs = {"tenant_id": "host"} + + def callback(ctx: FunctionInvocationContext | None = None, **kwargs: Any) -> Any: + calls.append("callback") + assert ctx is None + return kwargs + + class CustomScript(InlineSkillScript): + async def run(self, skill: Skill, args: Any = None, **kwargs: Any) -> Any: + calls.append("override") + if reject: + raise failure + return await super().run(skill, args, **kwargs) + + class InheritedScript(CustomScript): + pass + + script = InheritedScript(name="analyze", function=callback) + skill = InlineSkill(frontmatter=SkillFrontmatter(name="s", description="d"), instructions="c") + context = _script_context(**runtime_kwargs) + if reject: + with pytest.raises(RuntimeError) as caught: + await _run_through_provider(script, skill, context=context) + assert caught.value is failure + assert calls == ["override"] + else: + assert await _run_through_provider(script, skill, context=context) == runtime_kwargs + assert calls == ["override", "callback"] + + @pytest.mark.parametrize("script_kind", ["inline", "file"]) + async def test_subclass_without_run_override_inherits_context_injection(self, script_kind: str) -> None: + class CustomInline(InlineSkillScript): + pass + + class CustomFile(FileSkillScript): + pass + + def callback(ctx: FunctionInvocationContext) -> Any: + return ctx + + def runner( + skill: FileSkill, script: FileSkillScript, args: Any = None, *, ctx: FunctionInvocationContext | None = None + ) -> Any: + return ctx + + script = ( + CustomInline(name="analyze", function=callback) + if script_kind == "inline" + else CustomFile(name="analyze", full_path=f"{_ABS}/run.py", runner=runner) + ) + skill = FileSkill(frontmatter=SkillFrontmatter(name="s", description="d"), content="c", path=_ABS) + context = _script_context() + assert await _run_through_provider(script, skill, context=context) is context + + @pytest.mark.parametrize("value", [None, "customer information", 0]) + async def test_file_run_ignores_non_invocation_context_values(self, value: Any) -> None: + def runner( + skill: FileSkill, script: FileSkillScript, args: Any = None, *, ctx: FunctionInvocationContext | None = None + ) -> Any: + return ctx + + script = FileSkillScript(name="run.py", full_path=f"{_ABS}/run.py", runner=runner) + skill = FileSkill(frontmatter=SkillFrontmatter(name="s", description="d"), content="Body", path=_ABS) + assert await script.run(skill, context=value) is None + + @pytest.mark.parametrize("script_type", [InlineSkillScript, FileSkillScript]) + def test_builtin_run_context_is_optional_and_keyword_only(self, script_type: type[SkillScript]) -> None: + parameter = inspect.signature(script_type.run).parameters["context"] + assert parameter.kind is inspect.Parameter.KEYWORD_ONLY + assert parameter.default is not inspect.Parameter.empty + with pytest.raises(TypeError): + inspect.signature(script_type.run).bind(None, None, None, _script_context()) + + def test_invalid_context_signatures_fail_at_registration(self) -> None: + def positional(ctx: FunctionInvocationContext, /) -> None: ... + + def varargs(*ctx: FunctionInvocationContext) -> None: ... + + def kwargs(**ctx: FunctionInvocationContext) -> None: ... + + def multiple(first: FunctionInvocationContext, *, second: FunctionInvocationContext) -> None: ... + + for function in (positional, varargs, kwargs): + with pytest.raises(ValueError, match="must accept a keyword"): + InlineSkillScript(name="invalid", function=function) + with pytest.raises(ValueError, match="multiple FunctionInvocationContext"): + InlineSkillScript(name="invalid", function=multiple) + + @pytest.mark.parametrize("is_async", [False, True]) + @pytest.mark.parametrize("failure_type", [TypeError, RuntimeError, asyncio.CancelledError]) + async def test_callback_failure_propagates_without_retry( + self, is_async: bool, failure_type: type[BaseException] + ) -> None: + calls: list[FunctionInvocationContext] = [] + failure = failure_type("script failed") + + def callback(ctx: FunctionInvocationContext) -> None: + calls.append(ctx) + raise failure + + async def async_callback(ctx: FunctionInvocationContext) -> None: + callback(ctx) + + context = _script_context() + with pytest.raises(failure_type) as caught: + await _run_with_context(async_callback if is_async else callback, context=context) + assert caught.value is failure + assert calls == [context] + + async def test_concurrent_invocations_keep_context_separate(self) -> None: + both_running = asyncio.Event() + arrived: list[FunctionInvocationContext] = [] + + async def callback(ctx: FunctionInvocationContext) -> Any: + arrived.append(ctx) + if len(arrived) == 2: + both_running.set() + await asyncio.wait_for(both_running.wait(), timeout=5) + return ctx + + script = InlineSkillScript(name="analyze", function=callback) + skill = InlineSkill(frontmatter=SkillFrontmatter(name="s", description="d"), instructions="c") + contexts = [_script_context(tenant_id="first"), _script_context(tenant_id="second")] + + results = await asyncio.gather(*(script.run(skill, context=context) for context in contexts)) + assert results[0] is contexts[0] + assert results[1] is contexts[1] + + @pytest.mark.parametrize( + "make_skill", + [ + _inline_context_skill, + lambda parser: _ContextClassSkill( + frontmatter=SkillFrontmatter(name="s", description="d"), argument_parser=parser + ), + ], + ids=["inline", "class"], + ) + async def test_decorated_scripts_inject_context_through_provider(self, make_skill: Any) -> None: + skill = make_skill(lambda args: {"value": args["input"]}) + provider = SkillsProvider(skill) + await _init_provider(provider) + run_tool = next(t for t in _ctx(provider)[2] if hasattr(t, "name") and t.name == "run_skill_script") + context = _invocation_context(run_tool, value="host") + + script = await skill.get_script("analyze") + assert script is not None + assert script.parameters_schema is not None + assert set(script.parameters_schema["properties"]) == {"value"} + content = await skill.get_content() + assert '"ctx"' not in content + assert '"value"' in content + + value, received_context = await run_tool.func( + context, skill_name="s", script_name="analyze", args={"input": "model"} + ) + assert value == "model" + assert received_context is context + + class TestSkillScriptRun: """Tests for SkillScript.run().""" + @pytest.mark.parametrize( + ("args", "result", "runtime_kwargs"), + [ + (None, None, {}), + ({"value": 1}, object(), {"user_id": "host"}), + (["--value", "1"], {"completed": True}, {"user_id": "host"}), + ], + ) + async def test_provider_delegates_to_unannotated_run( + self, args: dict[str, Any] | list[str] | None, result: Any, runtime_kwargs: dict[str, Any] + ) -> None: + calls: list[tuple[Skill, Any, dict[str, Any]]] = [] + + class PlainScript(SkillScript): + async def run(self, skill: Skill, args: Any = None, **kwargs: Any) -> Any: + calls.append((skill, args, kwargs)) + return result + + script = PlainScript(name="plain") + skill = InlineSkill(frontmatter=SkillFrontmatter(name="s", description="d"), instructions="c") + context = _invocation_context(FunctionTool(name="run_skill_script", func=lambda: None), **runtime_kwargs) + + assert await _run_through_provider(script, skill, args, context=context) is result + assert calls == [(skill, args, runtime_kwargs)] + assert calls[0][1] is args + assert context.kwargs == runtime_kwargs + assert context.result is None + + @pytest.mark.parametrize("script_kind", ["inline", "file"]) + async def test_provider_preserves_builtin_subclass_overrides(self, script_kind: str) -> None: + class CustomInline(InlineSkillScript): + async def run(self, skill: Skill, args: Any = None, **kwargs: Any) -> Any: + return "inline", args, kwargs + + class CustomFile(FileSkillScript): + async def run(self, skill: Skill, args: Any = None, **kwargs: Any) -> Any: + return "file", args, kwargs + + script = ( + CustomInline(name="custom", function=lambda: pytest.fail("The run override must be used")) + if script_kind == "inline" + else CustomFile(name="custom", full_path=f"{_ABS}/test/run.py") + ) + skill = InlineSkill(frontmatter=SkillFrontmatter(name="s", description="d"), instructions="c") + context = _invocation_context(FunctionTool(name="run_skill_script", func=lambda: None), user_id="host") + + assert await _run_through_provider(script, skill, {"value": 1}, context=context) == ( + script_kind, + {"value": 1}, + {"user_id": "host"}, + ) + + @pytest.mark.parametrize("script_kind", ["inline", "file"]) + async def test_annotated_run_override_can_delegate_to_parent(self, script_kind: str) -> None: + class CustomInline(InlineSkillScript): + async def run( + self, + skill: Skill, + args: Any = None, + *, + context: FunctionInvocationContext | None = None, + **kwargs: Any, + ) -> Any: + assert context is not None + assert kwargs == {} + return await super().run(skill, args, context=context) + + class CustomFile(FileSkillScript): + async def run( + self, + skill: Skill, + args: Any = None, + *, + context: FunctionInvocationContext | None = None, + **kwargs: Any, + ) -> Any: + assert context is not None + assert kwargs == {} + return await super().run(skill, args, context=context) + + def callback(value: str, ctx: FunctionInvocationContext) -> Any: + return value, ctx + + def runner( + skill: FileSkill, script: FileSkillScript, args: Any = None, ctx: FunctionInvocationContext | None = None + ) -> Any: + return args["value"], ctx + + script = ( + CustomInline(name="analyze", function=callback) + if script_kind == "inline" + else CustomFile(name="analyze", full_path=f"{_ABS}/run.py", runner=runner) + ) + skill = FileSkill(frontmatter=SkillFrontmatter(name="s", description="d"), content="c", path=_ABS) + context = _script_context(context=None, value="host") + value, received_context = await _run_through_provider(script, skill, {"value": "model"}, context=context) + assert value == "model" + assert received_context is context + + @pytest.mark.parametrize( + ("signature_kind", "message"), + [ + ("args_slot", "must support run"), + ("missing_required", "must support run"), + ("positional_only", "must accept a keyword"), + ("varargs", "must accept a keyword"), + ("var_keyword", "must accept a keyword"), + ("multiple", "multiple FunctionInvocationContext"), + ], + ) + async def test_invalid_run_context_signature_does_not_execute(self, signature_kind: str, message: str) -> None: + async def args_slot( + self: Any, skill: Skill, ctx: FunctionInvocationContext | None = None, **kwargs: Any + ) -> Any: + pytest.fail("An invalid run signature must not be called") + + async def missing_required( + self: Any, skill: Skill, args: Any, extra: Any, ctx: FunctionInvocationContext | None = None + ) -> Any: + pytest.fail("An invalid run signature must not be called") + + async def positional_only( + self: Any, skill: Skill, args: Any = None, ctx: FunctionInvocationContext | None = None, / + ) -> Any: + pytest.fail("An invalid run signature must not be called") + + async def varargs(self: Any, skill: Skill, args: Any = None, *ctx: FunctionInvocationContext) -> Any: + pytest.fail("An invalid run signature must not be called") + + async def var_keyword(self: Any, skill: Skill, args: Any = None, **ctx: FunctionInvocationContext) -> Any: + pytest.fail("An invalid run signature must not be called") + + async def multiple( + self: Any, + skill: Skill, + args: Any = None, + ctx: FunctionInvocationContext | None = None, + other: FunctionInvocationContext | None = None, + ) -> Any: + pytest.fail("An invalid run signature must not be called") + + methods = { + "args_slot": args_slot, + "missing_required": missing_required, + "positional_only": positional_only, + "varargs": varargs, + "var_keyword": var_keyword, + "multiple": multiple, + } + skill = InlineSkill(frontmatter=SkillFrontmatter(name="s", description="d"), instructions="c") + script = type("InvalidScript", (SkillScript,), {"run": methods[signature_kind]})(name="invalid") + with pytest.raises(ValueError, match=message): + await _run_through_provider(script, skill, context=_script_context()) + + async def test_uninspectable_run_is_called_without_context( + self, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture + ) -> None: + class PlainScript(SkillScript): + async def run(self, skill: Skill, args: Any = None, **kwargs: Any) -> Any: + return kwargs + + monkeypatch.setattr(PlainScript.run, "__signature__", object(), raising=False) + skill = InlineSkill(frontmatter=SkillFrontmatter(name="s", description="d"), instructions="c") + with caplog.at_level("DEBUG", logger="agent_framework._skills"): + result = await _run_through_provider( + PlainScript(name="plain"), skill, context=_script_context(tenant_id="host") + ) + assert result == {"tenant_id": "host"} + assert "calling without context" in caplog.text + + async def test_run_context_detected_when_other_annotations_are_unresolvable(self) -> None: + class ContextScript(SkillScript): + async def run(self, skill: Skill, args: Any = None, ctx: Any = None, **kwargs: Any) -> Any: + assert kwargs == {} + return ctx + + ContextScript.run.__annotations__["ctx"] = "FunctionInvocationContext" + ContextScript.run.__annotations__["skill"] = "UndefinedSkillType" + script = ContextScript(name="analyze") + skill = InlineSkill(frontmatter=SkillFrontmatter(name="s", description="d"), instructions="c") + context = _script_context(tenant_id="host") + assert await _run_through_provider(script, skill, context=context) is context + + @pytest.mark.parametrize( + "annotation", + [ + FunctionInvocationContext, + FunctionInvocationContext | None, + "FunctionInvocationContext", + "FunctionInvocationContext | None", + ], + ) + async def test_provider_detects_run_context_by_annotation(self, annotation: Any) -> None: + class ContextScript(SkillScript): + async def run(self, skill: Skill, args: Any = None, ctx: Any = None, **kwargs: Any) -> Any: + assert kwargs == {} + return ctx + + ContextScript.run.__annotations__["ctx"] = annotation + script = ContextScript(name="analyze") + skill = InlineSkill(frontmatter=SkillFrontmatter(name="s", description="d"), instructions="c") + context = _script_context(ctx="application value", context=None) + assert await _run_through_provider(script, skill, context=context) is context + + async def test_provider_uses_updated_run_signature(self, monkeypatch: pytest.MonkeyPatch) -> None: + class Script(SkillScript): + async def run(self, skill: Skill, args: Any = None, **kwargs: Any) -> Any: + return kwargs + + script = Script(name="analyze") + skill = InlineSkill(frontmatter=SkillFrontmatter(name="s", description="d"), instructions="c") + context = _script_context(tenant_id="host") + assert await _run_through_provider(script, skill, context=context) == {"tenant_id": "host"} + + async def context_run( + self: Any, skill: Skill, args: Any = None, ctx: FunctionInvocationContext | None = None, **kwargs: Any + ) -> Any: + assert kwargs == {} + return ctx + + monkeypatch.setattr(Script, "run", context_run) + assert await _run_through_provider(script, skill, context=context) is context + + async def test_run_with_invocation_retains_file_validation(self, tmp_path: Path) -> None: + def runner(skill: FileSkill, script: FileSkillScript, args: Any = None) -> None: + pytest.fail("Invalid files must be rejected before calling the runner") + + script = FileSkillScript( + name="missing.py", full_path=str(tmp_path / "missing.py"), skill_dir=str(tmp_path), runner=runner + ) + skill = FileSkill(frontmatter=SkillFrontmatter(name="s", description="d"), content="c", path=str(tmp_path)) + context = _invocation_context(FunctionTool(name="run_skill_script", func=lambda: None)) + + with pytest.raises(ValueError, match="not found"): + await script.run(skill, context=context) + async def test_run_code_defined_sync(self) -> None: def greet(name: str = "world") -> str: return f"hello {name}" @@ -4338,6 +5034,357 @@ def test_scripts_at_construction(self) -> None: # --------------------------------------------------------------------------- +class TestFileSkillScriptContext: + """Optional context injection without changing the runner protocol.""" + + @pytest.mark.parametrize( + "runner_kind", ["sync", "async", "awaitable", "object", "async_object", "method", "partial", "decorated"] + ) + @pytest.mark.parametrize("args", [None, {"ctx": "model", "tenant_id": "model"}, ["--tenant", "model"]]) + async def test_runner_receives_original_context_and_arguments(self, runner_kind: str, args: Any) -> None: + calls: list[tuple[FileSkill, FileSkillScript, Any, FunctionInvocationContext | None]] = [] + result = object() + + def runner( + skill: FileSkill, + script: FileSkillScript, + args: Any = None, + *, + ctx: FunctionInvocationContext | None = None, + marker: str = "default", + **kwargs: Any, + ) -> Any: + assert kwargs == {} + assert marker == ("bound" if runner_kind == "partial" else "default") + calls.append((skill, script, args, ctx)) + return result + + async def async_runner( + skill: FileSkill, script: FileSkillScript, args: Any = None, *, ctx: FunctionInvocationContext | None = None + ) -> Any: + return runner(skill, script, args, ctx=ctx) + + def awaitable_runner( + skill: FileSkill, script: FileSkillScript, args: Any = None, *, ctx: FunctionInvocationContext | None = None + ) -> Any: + return async_runner(skill, script, args, ctx=ctx) + + class Runner: + def __call__( + self, + skill: FileSkill, + script: FileSkillScript, + args: Any = None, + *, + ctx: FunctionInvocationContext | None = None, + ) -> Any: + return runner(skill, script, args, ctx=ctx) + + class AsyncRunner: + async def __call__( + self, + skill: FileSkill, + script: FileSkillScript, + args: Any = None, + *, + ctx: FunctionInvocationContext | None = None, + ) -> Any: + return runner(skill, script, args, ctx=ctx) + + @wraps(runner) + def decorated(*args: Any, **kwargs: Any) -> Any: + return runner(*args, **kwargs) + + runners: dict[str, SkillScriptRunner] = { + "sync": runner, + "async": async_runner, + "awaitable": awaitable_runner, + "object": Runner(), + "async_object": AsyncRunner(), + "method": Runner().__call__, + "partial": partial(runner, marker="bound"), + "decorated": decorated, + } + selected_runner = runners[runner_kind] + script = FileSkillScript(name="run.py", full_path=f"{_ABS}/run.py", runner=selected_runner) + skill = FileSkill(frontmatter=SkillFrontmatter(name="s", description="d"), content="Body", path=_ABS) + context = _script_context(tenant_id="host", skill="host", script="host", args="host", ctx="host") + arguments = {"skill_name": "s", "script_name": "run.py", "args": args} + context.arguments = arguments + + direct_result = selected_runner(skill, script, args) + if inspect.isawaitable(direct_result): + direct_result = await direct_result + assert direct_result is result + assert await script.run(skill, args, ctx=context) is result + assert await script.run(skill, args, context=context) is result + + assert len(calls) == 3 + for owner, received_script, received_args, _ in calls: + assert owner is skill + assert received_script is script + assert received_args is args + assert [call[3] for call in calls[:2]] == [None, None] + assert calls[2][3] is context + assert context.arguments is arguments + assert context.result is None + assert script.parameters_schema == {"type": "array", "items": {"type": "string"}} + + @pytest.mark.parametrize( + "annotation", + [ + FunctionInvocationContext, + "FunctionInvocationContext | None", + ], + ) + async def test_context_parameter_may_have_another_name_and_positional_default(self, annotation: Any) -> None: + def runner(skill: FileSkill, script: FileSkillScript, args: Any = None, invocation: Any = None) -> Any: + return invocation + + runner.__annotations__["invocation"] = annotation + script = FileSkillScript(name="run.py", full_path=f"{_ABS}/run.py", runner=runner) + skill = FileSkill(frontmatter=SkillFrontmatter(name="s", description="d"), content="Body", path=_ABS) + context = _script_context() + assert await script.run(skill) is None + assert await script.run(skill, context=context) is context + + @pytest.mark.parametrize( + "annotation", + [ + list[FunctionInvocationContext], + Callable[..., FunctionInvocationContext], + Annotated[FunctionInvocationContext, "runtime"], + ], + ) + async def test_runner_non_context_annotations_are_not_injected(self, annotation: Any) -> None: + def runner(skill: FileSkill, script: FileSkillScript, args: Any = None, *, invocation: Any = None) -> Any: + return invocation + + runner.__annotations__["invocation"] = annotation + script = FileSkillScript(name="run.py", full_path=f"{_ABS}/run.py", runner=runner) + skill = FileSkill(frontmatter=SkillFrontmatter(name="s", description="d"), content="Body", path=_ABS) + assert await script.run(skill, context=_script_context()) is None + + async def test_unannotated_runner_kwargs_do_not_opt_in(self) -> None: + def runner( + skill: FileSkill, script: FileSkillScript, args: Any = None, *, ctx: str = "default", **kwargs: Any + ) -> Any: + return ctx, kwargs + + script = FileSkillScript(name="run.py", full_path=f"{_ABS}/run.py", runner=runner) + skill = FileSkill(frontmatter=SkillFrontmatter(name="s", description="d"), content="Body", path=_ABS) + assert await script.run(skill, context=_script_context(ctx="host")) == ("default", {}) + + async def test_uninspectable_runner_is_called_without_context(self, caplog: pytest.LogCaptureFixture) -> None: + class Runner: + @property + def __signature__(self) -> Any: + raise ValueError("Signature unavailable") + + def __call__(self, skill: FileSkill, script: FileSkillScript, args: Any = None) -> Any: + return args + + with caplog.at_level("DEBUG", logger="agent_framework._skills"): + script = FileSkillScript(name="run.py", full_path=f"{_ABS}/run.py", runner=Runner()) + skill = FileSkill(frontmatter=SkillFrontmatter(name="s", description="d"), content="Body", path=_ABS) + args = ["input.txt"] + assert await script.run(skill, args, context=_script_context()) is args + assert "calling without context" in caplog.text + + @pytest.mark.parametrize("reject", [False, True]) + @pytest.mark.parametrize("application_context", [None, "customer information", _script_context()]) + async def test_context_runner_does_not_bypass_subclass_run(self, reject: bool, application_context: Any) -> None: + calls: list[str] = [] + failure = RuntimeError("Rejected by the run override") + runtime_kwargs = {"tenant_id": "host", "context": application_context} + + def runner( + skill: FileSkill, script: FileSkillScript, args: Any = None, *, ctx: FunctionInvocationContext | None = None + ) -> Any: + calls.append("runner") + assert ctx is None + return args + + class CustomScript(FileSkillScript): + async def run(self, skill: Skill, args: Any = None, **kwargs: Any) -> Any: + calls.append("override") + assert kwargs == runtime_kwargs + if reject: + raise failure + return await super().run(skill, args, **kwargs) + + script = CustomScript(name="run.py", full_path=f"{_ABS}/run.py", runner=runner) + skill = FileSkill(frontmatter=SkillFrontmatter(name="s", description="d"), content="Body", path=_ABS) + args = ["input.txt"] + context = _script_context(**runtime_kwargs) + if reject: + with pytest.raises(RuntimeError) as caught: + await _run_through_provider(script, skill, args, context=context) + assert caught.value is failure + assert calls == ["override"] + else: + assert await _run_through_provider(script, skill, args, context=context) is args + assert calls == ["override", "runner"] + + @pytest.mark.parametrize("failure_type", [TypeError, RuntimeError, asyncio.CancelledError]) + @pytest.mark.parametrize("is_async", [False, True]) + @pytest.mark.parametrize("context_aware", [False, True]) + async def test_runner_failure_propagates_without_retry( + self, failure_type: type[BaseException], is_async: bool, context_aware: bool + ) -> None: + calls: list[FunctionInvocationContext | None] = [] + failure = failure_type("Runner failed") + + def runner( + skill: FileSkill, script: FileSkillScript, args: Any = None, *, ctx: FunctionInvocationContext | None = None + ) -> None: + calls.append(ctx) + raise failure + + async def async_runner( + skill: FileSkill, script: FileSkillScript, args: Any = None, *, ctx: FunctionInvocationContext | None = None + ) -> None: + runner(skill, script, args, ctx=ctx) + + script = FileSkillScript(name="run.py", full_path=f"{_ABS}/run.py", runner=async_runner if is_async else runner) + skill = FileSkill(frontmatter=SkillFrontmatter(name="s", description="d"), content="Body", path=_ABS) + context = _script_context() + with pytest.raises(failure_type) as caught: + if context_aware: + await script.run(skill, context=context) + else: + await script.run(skill) + assert caught.value is failure + assert calls == [context if context_aware else None] + + @pytest.mark.parametrize( + ("invalid_case", "error_type", "message"), + [ + ("skill", TypeError, "requires a FileSkill"), + ("runner", ValueError, "requires a runner"), + ("file", ValueError, "not found"), + ], + ) + @pytest.mark.parametrize("context_aware", [False, True]) + async def test_validation_precedes_runner( + self, tmp_path: Path, invalid_case: str, error_type: type[Exception], message: str, context_aware: bool + ) -> None: + def runner( + skill: FileSkill, script: FileSkillScript, args: Any = None, *, ctx: FunctionInvocationContext | None = None + ) -> None: + pytest.fail("Invalid execution must not call the runner") + + script = FileSkillScript( + name="missing.py", + full_path=str(tmp_path / "missing.py"), + skill_dir=str(tmp_path), + runner=None if invalid_case == "runner" else runner, + ) + skill: Skill = FileSkill( + frontmatter=SkillFrontmatter(name="s", description="d"), content="Body", path=str(tmp_path) + ) + if invalid_case == "skill": + skill = InlineSkill(frontmatter=SkillFrontmatter(name="s", description="d"), instructions="Body") + with pytest.raises(error_type, match=message): + if context_aware: + await script.run(skill, context=_script_context()) + else: + await script.run(skill) + + @pytest.mark.parametrize("context_aware", [False, True]) + async def test_file_is_revalidated_on_every_run(self, tmp_path: Path, context_aware: bool) -> None: + calls: list[FunctionInvocationContext | None] = [] + + def runner( + skill: FileSkill, script: FileSkillScript, args: Any = None, *, ctx: FunctionInvocationContext | None = None + ) -> str: + calls.append(ctx) + return "executed" + + script_path = tmp_path / "run.py" + script_path.write_text("# Script executed by the runner\n", encoding="utf-8") + script = FileSkillScript(name="run.py", full_path=str(script_path), skill_dir=str(tmp_path), runner=runner) + skill = FileSkill(frontmatter=SkillFrontmatter(name="s", description="d"), content="Body", path=str(tmp_path)) + context = _script_context() + + if context_aware: + assert await script.run(skill, context=context) == "executed" + else: + assert await script.run(skill) == "executed" + + script_path.unlink() + with pytest.raises(ValueError, match="not found"): + if context_aware: + await script.run(skill, context=context) + else: + await script.run(skill) + + assert calls == [context if context_aware else None] + + def test_invalid_context_signatures_fail_at_registration(self) -> None: + def required(skill: Any, script: Any, args: Any = None, *, ctx: FunctionInvocationContext) -> None: ... + + def first(ctx: FunctionInvocationContext | None = None, script: Any = None, args: Any = None) -> None: ... + + def second(skill: Any, ctx: FunctionInvocationContext | None = None, args: Any = None) -> None: ... + + def third(skill: Any, script: Any, ctx: FunctionInvocationContext | None = None) -> None: ... + + def positional( + skill: Any, script: Any, args: Any = None, ctx: FunctionInvocationContext | None = None, / + ) -> None: ... + + def varargs(skill: Any, script: Any, args: Any = None, *ctx: FunctionInvocationContext) -> None: ... + + def kwargs(skill: Any, script: Any, args: Any = None, **ctx: FunctionInvocationContext) -> None: ... + + def multiple( + skill: Any, + script: Any, + args: Any = None, + *, + ctx: FunctionInvocationContext | None = None, + other: FunctionInvocationContext | None = None, + ) -> None: ... + + cases: list[tuple[Any, str]] = [ + (required, "must have a default"), + (first, "must not replace"), + (second, "must not replace"), + (third, "must not replace"), + (positional, "must accept a keyword"), + (varargs, "must accept a keyword"), + (kwargs, "must accept a keyword"), + (multiple, "multiple FunctionInvocationContext"), + ] + for runner, message in cases: + with pytest.raises(ValueError, match=message): + FileSkillScript(name="run.py", full_path=f"{_ABS}/run.py", runner=runner) + + async def test_concurrent_invocations_keep_runner_context_separate(self) -> None: + arrived: list[FunctionInvocationContext | None] = [] + both_running = asyncio.Event() + + async def runner( + skill: FileSkill, script: FileSkillScript, args: Any = None, *, ctx: FunctionInvocationContext | None = None + ) -> Any: + arrived.append(ctx) + if len(arrived) == 2: + both_running.set() + await asyncio.wait_for(both_running.wait(), timeout=5) + return ctx + + script = FileSkillScript(name="run.py", full_path=f"{_ABS}/run.py", runner=runner) + skill = FileSkill(frontmatter=SkillFrontmatter(name="s", description="d"), content="Body", path=_ABS) + contexts = [_script_context(tenant_id="first"), _script_context(tenant_id="second")] + results = await asyncio.gather(*(script.run(skill, context=context) for context in contexts)) + assert results[0] is contexts[0] + assert results[1] is contexts[1] + + def test_protocol_signature_is_unchanged(self) -> None: + assert list(inspect.signature(SkillScriptRunner.__call__).parameters) == ["self", "skill", "script", "args"] + + class TestSkillScriptRunnerProtocol: """Tests for the SkillScriptRunner protocol.""" @@ -4773,12 +5820,13 @@ def greet(name: str, **kwargs: Any) -> str: provider = SkillsProvider([skill]) await _init_provider(provider) + run_tool = next(t for t in _ctx(provider)[2] if hasattr(t, "name") and t.name == "run_skill_script") result = await provider._run_skill_script( _raw_skills(provider), "my-skill", "greet", args={"name": "Alice"}, - runtime_kwargs={"user_id": "u42"}, + context=_invocation_context(run_tool, user_id="u42"), ) assert result == "Hello Alice (user=u42)" @@ -4792,12 +5840,13 @@ async def fetch(url: str, **kwargs: Any) -> str: provider = SkillsProvider([skill]) await _init_provider(provider) + run_tool = next(t for t in _ctx(provider)[2] if hasattr(t, "name") and t.name == "run_skill_script") result = await provider._run_skill_script( _raw_skills(provider), "my-skill", "fetch", args={"url": "http://x"}, - runtime_kwargs={"auth_token": "abc"}, + context=_invocation_context(run_tool, auth_token="abc"), ) assert result == "fetched http://x with token=abc" @@ -4811,12 +5860,13 @@ def simple(query: str) -> str: provider = SkillsProvider([skill]) await _init_provider(provider) + run_tool = next(t for t in _ctx(provider)[2] if hasattr(t, "name") and t.name == "run_skill_script") result = await provider._run_skill_script( _raw_skills(provider), "my-skill", "simple", args={"query": "test"}, - runtime_kwargs={"user_id": "ignored"}, + context=_invocation_context(run_tool, user_id="ignored"), ) assert result == "result: test" @@ -4830,15 +5880,141 @@ def process(**kwargs: Any) -> str: provider = SkillsProvider([skill]) await _init_provider(provider) + run_tool = next(t for t in _ctx(provider)[2] if hasattr(t, "name") and t.name == "run_skill_script") with pytest.raises(TypeError): await provider._run_skill_script( _raw_skills(provider), "my-skill", "process", args={"mode": "llm-value"}, - runtime_kwargs={"mode": "runtime-value"}, + context=_invocation_context(run_tool, mode="runtime-value"), ) + async def test_run_skill_script_forwards_original_context_to_override(self) -> None: + calls: list[tuple[Skill, Any, FunctionInvocationContext]] = [] + result = object() + + class ContextScript(SkillScript): + async def run( + self, skill: Skill, args: Any = None, invocation: FunctionInvocationContext | None = None, **kwargs: Any + ) -> Any: + assert invocation is not None + assert kwargs == {} + calls.append((skill, args, invocation)) + return result + + skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") + skill._scripts.append(ContextScript(name="s1")) + provider = SkillsProvider([skill]) + await _init_provider(provider) + run_tool = next(t for t in _ctx(provider)[2] if hasattr(t, "name") and t.name == "run_skill_script") + args = {"value": "model", "context": "model-context"} + arguments = {"skill_name": "my-skill", "script_name": "s1", "args": args} + session = MockAgentSession() + tools: list[Any] = [run_tool] + context = FunctionInvocationContext( + function=run_tool, + arguments=arguments, + session=session, + tools=tools, + metadata={"marker": "outer-call"}, + kwargs={"skill": "host-skill", "args": "host-args", "context": "host-context"}, + ) + + assert await run_tool.func(context, **arguments) is result + assert len(calls) == 1 + assert calls[0][0] is skill + assert calls[0][1] is args + assert calls[0][2] is context + assert context.function is run_tool + assert context.arguments is arguments + assert context.session is session + assert context.tools is tools + assert context.metadata == {"marker": "outer-call"} + assert context.kwargs == {"skill": "host-skill", "args": "host-args", "context": "host-context"} + assert context.result is None + + @pytest.mark.parametrize("script_kind", ["kwargs", "context", "timeout", "context_kwargs"]) + @pytest.mark.parametrize("application_context", [None, "customer information"]) + async def test_run_skill_script_passes_runtime_kwargs_to_unannotated_overrides( + self, script_kind: str, application_context: str | None + ) -> None: + calls: list[tuple[Any, dict[str, Any]]] = [] + + class PlainScript(SkillScript): + async def run(self, skill: Skill, args: Any = None, **kwargs: Any) -> Any: + calls.append((args, kwargs)) + return "plain" + + class ApplicationContextScript(PlainScript): + async def run(self, skill: Skill, args: Any = None, context: str | None = "default", **kwargs: Any) -> Any: + return await super().run(skill, args, context=context, **kwargs) + + class TimeoutScript(PlainScript): + async def run(self, skill: Skill, args: Any = None, timeout: int = 30, **kwargs: Any) -> Any: + assert timeout == 30 + return await super().run(skill, args, **kwargs) + + class ContextKwargsScript(PlainScript): + async def run(self, skill: Skill, args: Any = None, **context: Any) -> Any: + return await super().run(skill, args, **context) + + helpers: dict[str, type[SkillScript]] = { + "kwargs": PlainScript, + "context": ApplicationContextScript, + "timeout": TimeoutScript, + "context_kwargs": ContextKwargsScript, + } + script = helpers[script_kind](name="s1") + skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") + skill._scripts.append(script) + provider = SkillsProvider([skill]) + await _init_provider(provider) + run_tool = next(t for t in _ctx(provider)[2] if hasattr(t, "name") and t.name == "run_skill_script") + + result = await provider._run_skill_script( + _raw_skills(provider), + "my-skill", + "s1", + args={"value": "model"}, + context=_invocation_context(run_tool, user_id="host", context=application_context), + ) + + assert result == "plain" + assert calls == [({"value": "model"}, {"user_id": "host", "context": application_context})] + + @pytest.mark.parametrize("context_override", [False, True]) + @pytest.mark.parametrize("failure_type", [TypeError, RuntimeError, asyncio.CancelledError]) + async def test_script_failure_propagates_once_through_context_dispatch( + self, context_override: bool, failure_type: type[BaseException], caplog: pytest.LogCaptureFixture + ) -> None: + calls: list[str] = [] + failure = failure_type("script failed") + + class FailingScript(SkillScript): + async def run( + self, skill: Skill, args: Any = None, ctx: FunctionInvocationContext | None = None, **kwargs: Any + ) -> Any: + if context_override: + calls.append("context") + else: + calls.append("plain") + raise failure + + skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") + skill._scripts.append(FailingScript(name="boom")) + provider = SkillsProvider([skill]) + await _init_provider(provider) + run_tool = next(t for t in _ctx(provider)[2] if hasattr(t, "name") and t.name == "run_skill_script") + + with pytest.raises(failure_type) as caught: + await run_tool.func(_invocation_context(run_tool), skill_name="my-skill", script_name="boom") + + assert caught.value is failure + assert calls == ["context" if context_override else "plain"] + if failure_type is not asyncio.CancelledError: + assert "Error running script 'boom' in skill 'my-skill'" in caplog.text + async def test_run_skill_script_error_on_missing_script(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") skill._scripts.append(InlineSkillScript(name="s1", function=lambda: None)) @@ -7774,6 +8950,211 @@ def _agent(client: MockBaseChatClient, skill: InlineSkill) -> Agent: ) return Agent[ChatOptions[None]](client=client, context_providers=[provider]) + @pytest.mark.parametrize( + "script_kind", + [ + "inline", + "async_inline", + "context_inline", + "async_context_inline", + "file", + "async_file", + "context_file", + "async_context_file", + "custom", + ], + ) + @pytest.mark.parametrize("streaming", [False, True]) + @pytest.mark.parametrize("approval", [None, True, False], ids=["no-approval", "approved", "rejected"]) + async def test_context_dispatch_preserves_execution_and_approval( + self, + chat_client_base: MockBaseChatClient, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + script_kind: str, + streaming: bool, + approval: bool | None, + ) -> None: + calls: list[tuple[Any, dict[str, Any]]] = [] + contexts: list[FunctionInvocationContext] = [] + model_calls: list[list[Message]] = [] + original_get_response = chat_client_base._inner_get_response + + def capture_model_call(*, messages: MutableSequence[Message], **kwargs: Any) -> Any: + model_calls.append([Message.from_dict(message.to_dict()) for message in messages]) + return original_get_response(messages=messages, **kwargs) + + monkeypatch.setattr(chat_client_base, "_inner_get_response", capture_model_call) + + def record(args: Any, kwargs: dict[str, Any]) -> str: + calls.append((args, kwargs)) + return "script completed" + + script_args: dict[str, Any] | list[str] + if script_kind in {"file", "async_file", "context_file", "async_context_file"}: + _write_skill(tmp_path, "test-skill", resources={"scripts/run.py": "# Executed by the runner\n"}) + + def runner(skill: FileSkill, script: FileSkillScript, args: Any = None) -> str: + assert skill.frontmatter.name == "test-skill" + assert script.name == "scripts/run.py" + return record(args, {}) + + async def async_runner(skill: FileSkill, script: FileSkillScript, args: Any = None) -> str: + return runner(skill, script, args) + + def context_runner( + skill: FileSkill, + script: FileSkillScript, + args: Any = None, + *, + ctx: FunctionInvocationContext | None = None, + ) -> str: + assert ctx is not None + contexts.append(ctx) + assert skill.frontmatter.name == "test-skill" + assert script.name == "scripts/run.py" + return record(args, ctx.kwargs) + + async def async_context_runner( + skill: FileSkill, + script: FileSkillScript, + args: Any = None, + *, + ctx: FunctionInvocationContext | None = None, + ) -> str: + return context_runner(skill, script, args, ctx=ctx) + + runners: dict[str, SkillScriptRunner] = { + "file": runner, + "async_file": async_runner, + "context_file": context_runner, + "async_context_file": async_context_runner, + } + provider = SkillsProvider.from_paths( + tmp_path, + script_runner=runners[script_kind], + disable_run_skill_script_approval=approval is None, + ) + script_name = "scripts/run.py" + script_args = ["--value", "1"] + else: + skill = InlineSkill( + frontmatter=SkillFrontmatter(name="test-skill", description="Test"), instructions="Body" + ) + if script_kind == "custom": + + class ContextScript(SkillScript): + async def run( + self, + skill: Skill, + args: Any = None, + ctx: FunctionInvocationContext | None = None, + **kwargs: Any, + ) -> Any: + assert ctx is not None + assert kwargs == {} + contexts.append(ctx) + return record(args, ctx.kwargs) + + skill._scripts.append(ContextScript(name="test-script")) + elif script_kind in {"context_inline", "async_context_inline"}: + + def context_callback(value: int, ctx: FunctionInvocationContext, **kwargs: Any) -> str: + contexts.append(ctx) + assert kwargs == {} + return record({"value": value}, ctx.kwargs) + + async def async_context_callback(value: int, ctx: FunctionInvocationContext, **kwargs: Any) -> str: + return context_callback(value, ctx, **kwargs) + + skill.script(name="test-script")( + async_context_callback if script_kind == "async_context_inline" else context_callback + ) + else: + + def callback(value: int, **kwargs: Any) -> str: + return record({"value": value}, kwargs) + + async def async_callback(value: int, **kwargs: Any) -> str: + return callback(value, **kwargs) + + skill.script(name="test-script")(async_callback if script_kind == "async_inline" else callback) + provider = SkillsProvider(skill, disable_run_skill_script_approval=approval is None) + script_name = "test-script" + script_args = {"value": 1} + + arguments = {"skill_name": "test-skill", "script_name": script_name, "args": script_args} + function_call = Content.from_function_call(call_id="script-call", name="run_skill_script", arguments=arguments) + if streaming: + chat_client_base.streaming_responses = [ + [ChatResponseUpdate(role="assistant", contents=[function_call])], + [ChatResponseUpdate(role="assistant", contents=[Content.from_text("done")])], + ] + else: + chat_client_base.run_responses = [ + ChatResponse(messages=[Message(role="assistant", contents=[function_call])]), + ChatResponse(messages=[Message(role="assistant", contents=["done"])]), + ] + agent = Agent[ChatOptions[None]](client=chat_client_base, context_providers=[provider]) + session = agent.create_session() + + async def run(messages: Any) -> Any: + if not streaming: + return await agent.run( + messages, session=session, function_invocation_kwargs={"tenant_id": "host-tenant"} + ) + response_stream = agent.run( + messages, session=session, stream=True, function_invocation_kwargs={"tenant_id": "host-tenant"} + ) + streamed_contents = [item async for update in response_stream for item in update.contents] + response = await response_stream.get_final_response() + final_contents = [item for message in response.messages for item in message.contents] + assert [item.type for item in streamed_contents] == [item.type for item in final_contents] + assert [ + (item.call_id, str(item.result)) for item in streamed_contents if item.type == "function_result" + ] == [(item.call_id, str(item.result)) for item in final_contents if item.type == "function_result"] + return response + + response = await run("Run the script") + if approval is not None: + assert calls == [] + assert contexts == [] + assert len(response.user_input_requests) == 1 + assert chat_client_base.call_count == 1 + response = await run(response.user_input_requests[0].to_function_approval_response(approval)) + + results = [item for message in response.messages for item in message.contents if item.type == "function_result"] + assert len(results) == 1 + assert results[0].call_id == "script-call" + assert chat_client_base.call_count == 2 + if approval is False: + assert calls == [] + assert contexts == [] + assert "script completed" not in str(results[0].result) + else: + expected_kwargs = ( + {} if script_kind in {"file", "async_file"} else {"tenant_id": "host-tenant", "session": session} + ) + assert calls == [(script_args, expected_kwargs)] + assert results[0].exception is None + assert "script completed" in str(results[0].result) + if script_kind in { + "custom", + "context_inline", + "async_context_inline", + "context_file", + "async_context_file", + }: + assert len(contexts) == 1 + assert contexts[0].session is session + assert contexts[0].arguments == arguments + assert contexts[0].function.name == "run_skill_script" + + model_contents = [item for message in model_calls[-1] for item in message.contents] + assert [item.call_id for item in model_contents if item.type == "function_call"] == ["script-call"] + assert [item.call_id for item in model_contents if item.type == "function_result"] == ["script-call"] + assert not any(item.type.startswith("function_approval_") for item in model_contents) + async def test_resource_receives_host_runtime_kwargs(self, chat_client_base: MockBaseChatClient) -> None: """Host ``function_invocation_kwargs`` must reach a callable resource. diff --git a/python/samples/02-agents/skills/README.md b/python/samples/02-agents/skills/README.md index df281c5e037..326c1c9ef6f 100644 --- a/python/samples/02-agents/skills/README.md +++ b/python/samples/02-agents/skills/README.md @@ -51,6 +51,28 @@ Skills can include executable scripts. How a script runs depends on how it was d The `script_runner` parameter on `SkillsProvider` is only applicable to **file-based** scripts. Code-defined scripts are always executed in-process regardless of this setting. See [file_based_skill](file_based_skill/) for an example using a `SkillScriptRunner` callable with a subprocess runner, and [code_defined_skill](code_defined_skill/) for in-process scripts that need no runner. +### Host Runtime Context + +Pass host values with `agent.run(..., function_invocation_kwargs={...})`. Scripts +should accept `FunctionInvocationContext` to read those values from +`ctx.kwargs`, separately from model-supplied script arguments. + +- [Code-defined skills](code_defined_skill/) and [class-based skills](class_based_skill/) + declare `*, ctx: FunctionInvocationContext` on their conversion callbacks to read + host-controlled rounding precision. The context parameter is hidden from the + script schema and cannot be supplied through script arguments. +- File-based runners that need host context declare + `*, ctx: FunctionInvocationContext | None = None`. + The runner's context parameter must have a default value. + Only model arguments are forwarded to the subprocess; context is not + automatically added to CLI arguments or environment variables. + +`SkillsProvider` injects the enclosing tool invocation through `run(...)`. Custom +`SkillScript` implementations opt in with a keyword-bindable +`FunctionInvocationContext` parameter, such as +`ctx: FunctionInvocationContext | None = None`. Unannotated overrides keep +receiving host values as individual keyword arguments. + ## Prerequisites All samples require: diff --git a/python/samples/02-agents/skills/class_based_skill/README.md b/python/samples/02-agents/skills/class_based_skill/README.md index c8d19c0d0d2..817ea0648f5 100644 --- a/python/samples/02-agents/skills/class_based_skill/README.md +++ b/python/samples/02-agents/skills/class_based_skill/README.md @@ -8,6 +8,7 @@ This sample demonstrates how to define **Agent Skills as Python classes** using - Bundling name, description, instructions, resources, and scripts into a single class - Using `@ClassSkill.resource` decorator for automatic resource discovery - Using `@ClassSkill.script` decorator for automatic script discovery +- Injecting `FunctionInvocationContext` into a script method to read host-controlled rounding precision - Lazy-loading and caching of resources and scripts - Registering class-based skills with `SkillsProvider` @@ -20,6 +21,19 @@ A `UnitConverterSkill` class that converts between common units. Defined in `cla - `conversion-table` — Static resource with factor table - `convert` — Script that performs `value × factor` conversion +## Runtime Context + +`main()` supplies `function_invocation_kwargs={"precision": 4}` to `agent.run()`. +The `convert_units` method declares `*, ctx: FunctionInvocationContext` and +reads `ctx.kwargs["precision"]`, while `value` and `factor` come from the model. +The context parameter is hidden from the script schema and cannot be supplied +through script arguments. Missing host precision raises an error. + +Context injection works for `@ClassSkill.script` methods just as it does for +`@skill.script` callbacks. `SkillsProvider` supplies the context automatically +during agent execution. See the +[recommended script pattern](../README.md#host-runtime-context). + ## Project Structure ``` diff --git a/python/samples/02-agents/skills/class_based_skill/class_based_skill.py b/python/samples/02-agents/skills/class_based_skill/class_based_skill.py index 9e59ce17e7d..cb31ae380bc 100644 --- a/python/samples/02-agents/skills/class_based_skill/class_based_skill.py +++ b/python/samples/02-agents/skills/class_based_skill/class_based_skill.py @@ -5,7 +5,14 @@ import os from textwrap import dedent -from agent_framework import Agent, ClassSkill, SkillFrontmatter, SkillsProvider, ToolApprovalMiddleware +from agent_framework import ( + Agent, + ClassSkill, + FunctionInvocationContext, + SkillFrontmatter, + SkillsProvider, + ToolApprovalMiddleware, +) from agent_framework.foundry import FoundryChatClient from azure.identity import AzureCliCredential from dotenv import load_dotenv @@ -22,6 +29,8 @@ - Subclassing ``ClassSkill`` to create a self-contained skill - Using ``@property`` + ``@ClassSkill.resource`` (bare) — name defaults to method name - Using ``@ClassSkill.script(name=..., description=...)`` — explicit name and description +- Reading host-controlled rounding precision through an injected + ``FunctionInvocationContext``, separately from model-supplied arguments - Lazy-loading and caching of resources and scripts """ @@ -86,17 +95,26 @@ def conversion_table(self) -> str: # 2. Explicit name — overrides the method name # 3. Explicit description — provides a description for the script @ClassSkill.script(name="convert", description="Multiplies a value by a conversion factor.") - def convert_units(self, value: float, factor: float) -> str: + def convert_units(self, value: float, factor: float, *, ctx: FunctionInvocationContext) -> str: """Convert a value using a multiplication factor: result = value × factor. + The model supplies ``value`` and ``factor``. The annotated ``ctx`` parameter + is injected by the provider and hidden from the script's parameter schema. + Args: value: The numeric value to convert. factor: Conversion factor from the conversion table. + ctx: Tool invocation context with host-controlled rounding precision. Returns: JSON string with the inputs and converted result. """ - result = round(value * factor, 4) + if "precision" not in ctx.kwargs: + raise RuntimeError( + "Expected host-supplied 'precision' in ctx.kwargs. " + "Pass it through agent.run(function_invocation_kwargs=...)." + ) + result = round(value * factor, ctx.kwargs["precision"]) return json.dumps({"value": value, "factor": factor, "result": result}) @@ -128,6 +146,7 @@ async def main() -> None: session = agent.create_session() response = await agent.run( "How many kilometers is a marathon (26.2 miles)? And how many pounds is 75 kilograms?", + function_invocation_kwargs={"precision": 4}, session=session, ) print(f"Agent: {response}\n") diff --git a/python/samples/02-agents/skills/code_defined_skill/README.md b/python/samples/02-agents/skills/code_defined_skill/README.md index 187442d17fe..1a8d9897e13 100644 --- a/python/samples/02-agents/skills/code_defined_skill/README.md +++ b/python/samples/02-agents/skills/code_defined_skill/README.md @@ -6,10 +6,23 @@ This sample demonstrates how to create **Agent Skills** in Python code, without 1. **Static Resources** — Pass inline content via the `resources` parameter when constructing a `Skill` 2. **Dynamic Resources** — Attach callable functions via the `@skill.resource` decorator that return content computed at runtime -3. **Dynamic Scripts** — Attach callable scripts via the `@skill.script` decorator (unit conversion via a single factor parameter) +3. **Dynamic Scripts** — Attach callable scripts via the `@skill.script` decorator, with an injected `FunctionInvocationContext` for host-controlled rounding precision All three can be combined with file-based skills in a single `SkillsProvider`. +## Runtime Context vs Model Arguments + +`main()` passes `function_invocation_kwargs={"precision": 2}` to `agent.run()`. +The `convert` script declares `*, ctx: FunctionInvocationContext` and reads +`ctx.kwargs["precision"]`. The model supplies only `value` and `factor`; `ctx` +is hidden from the script schema and cannot be supplied through script arguments. +Missing host precision raises an error rather than silently using a default. + +The dynamic resource receives host values through `**kwargs`; script callbacks +use the separate `ctx.kwargs` mapping. `SkillsProvider` supplies the context +automatically during agent execution. See the +[recommended script pattern](../README.md#host-runtime-context). + ## Project Structure ``` @@ -28,7 +41,7 @@ code_defined_skill/ Set the required environment variables in a `.env` file (see `python/.env.example`): - `FOUNDRY_PROJECT_ENDPOINT`: Your Microsoft Foundry project endpoint -- `AZURE_OPENAI_MODEL`: The name of your model deployment (defaults to `gpt-4o-mini`) +- `FOUNDRY_MODEL`: The name of your model deployment (defaults to `gpt-4o-mini`) ### Authentication diff --git a/python/samples/02-agents/skills/code_defined_skill/code_defined_skill.py b/python/samples/02-agents/skills/code_defined_skill/code_defined_skill.py index d6bb4b72ac0..21fbb3f6939 100644 --- a/python/samples/02-agents/skills/code_defined_skill/code_defined_skill.py +++ b/python/samples/02-agents/skills/code_defined_skill/code_defined_skill.py @@ -8,6 +8,7 @@ from agent_framework import ( Agent, + FunctionInvocationContext, InlineSkill, InlineSkillResource, SkillFrontmatter, @@ -38,13 +39,12 @@ Attach a callable script via the @skill.script decorator. Scripts are executable functions the agent can invoke directly in-process. -Resources and scripts that accept ``**kwargs`` also receive host-supplied -runtime context from ``agent.run(..., function_invocation_kwargs={...})``. -This sample passes ``precision`` that way. Scripts additionally receive the -model-supplied nested ``args`` dictionary: declared parameters bind by name, -and extra entries can enter the callback's ``**kwargs``. Unlike resource -callbacks, script callbacks therefore do not have a runtime-only ``**kwargs`` -mapping. +This sample passes host-controlled ``precision`` through +``agent.run(..., function_invocation_kwargs={"precision": 2})``. The resource +receives it through ``**kwargs``. The script declares a +``FunctionInvocationContext`` parameter and reads ``ctx.kwargs["precision"]``, +keeping host values separate from model-supplied ``value`` and ``factor``. +The injected context parameter is hidden from the script's parameter schema. Code-defined skills can be combined with file-based skills in a single SkillsProvider — see the mixed_skills sample. @@ -132,7 +132,7 @@ def conversion_policy(**kwargs: Any) -> Any: # 3. Dynamic Scripts — in-process callable function # --------------------------------------------------------------------------- @unit_converter_skill.script(name="convert", description="Convert a value: result = value × factor") -def convert_units(value: float, factor: float, **kwargs: Any) -> str: +def convert_units(value: float, factor: float, *, ctx: FunctionInvocationContext) -> str: """Convert a value using a multiplication factor: result = value × factor. The caller looks up the correct factor from the conversion-tables @@ -140,25 +140,25 @@ def convert_units(value: float, factor: float, **kwargs: Any) -> str: The model supplies ``value`` and ``factor`` through the script's nested ``args`` dictionary, while ``main()`` supplies ``precision`` through - ``function_invocation_kwargs``. Both dictionaries are expanded into this - callback, so additional nested ``args`` entries can also enter ``**kwargs``. - Checking for ``precision`` below verifies its presence, not its source. + ``function_invocation_kwargs``. Declaring ``ctx`` opts into context injection: + host values are available only in ``ctx.kwargs``, not merged into script + arguments. The model cannot supply ``ctx``. Args: value: The numeric value to convert. factor: Conversion factor from the conversion table. - **kwargs: Runtime keyword arguments from ``agent.run()`` and any extra - entries in the script's nested ``args`` dictionary. The ``precision`` - kwarg controls how many decimal places the result is rounded to. + ctx: Injected tool invocation context. Its host-supplied ``precision`` + value controls how many decimal places the result is rounded to. Returns: JSON string with the inputs and converted result. """ - if "precision" not in kwargs: + if "precision" not in ctx.kwargs: raise RuntimeError( - "This sample requires 'precision'. main() supplies it through agent.run(function_invocation_kwargs=...)." + "Expected host-supplied 'precision' in ctx.kwargs. " + "Pass it through agent.run(function_invocation_kwargs=...)." ) - precision = kwargs["precision"] + precision = ctx.kwargs["precision"] result = round(value * factor, precision) return json.dumps({"value": value, "factor": factor, "result": result})