From 12ebbdb2674b263661f8828b639c50481d4db529 Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Mon, 14 Sep 2026 21:07:48 +0100 Subject: [PATCH 1/3] Python: Propagate runtime arguments to skill callbacks Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../packages/core/agent_framework/_skills.py | 59 ++- .../packages/core/tests/core/test_skills.py | 433 +++++++++++++++++- .../code_defined_skill/code_defined_skill.py | 39 +- 3 files changed, 498 insertions(+), 33 deletions(-) diff --git a/python/packages/core/agent_framework/_skills.py b/python/packages/core/agent_framework/_skills.py index 4c14e5b07b5..9b67c55b679 100644 --- a/python/packages/core/agent_framework/_skills.py +++ b/python/packages/core/agent_framework/_skills.py @@ -75,6 +75,7 @@ from pydantic import AnyUrl from ._agents import SupportsAgentRun + from ._middleware import FunctionInvocationContext from ._sessions import AgentSession, SessionContext from ._types import Content @@ -2549,13 +2550,41 @@ def _create_tools( async def _load(skill_name: str) -> str: return await self._load_skill(skills, skill_name) - async def _read_resource(skill_name: str, resource_name: str, **kwargs: Any) -> Any: - return await self._read_skill_resource(skills, skill_name, resource_name, **kwargs) + # The runtime kwargs these handlers forward come from the host's + # ``agent.run(function_invocation_kwargs=...)``, never from the model. Two + # things keep the channels apart at this seam, and both are required. + # + # First, declaring ``ctx`` opts the handler into FunctionInvocationContext + # injection; without it the function-invocation layer has nowhere to put the + # host values and silently drops them. Second, the handlers take no ``**kwargs`` + # and the outer schemas below set ``additionalProperties: False``, so an + # undeclared top-level argument returned by the model is rejected during + # validation instead of binding into the runtime namespace. + # + # ``ctx`` is injected by name and is absent from the advertised schema, so it is + # never something the model can supply or override. + # + # This guarantee covers ``ctx.kwargs``, not everything a skill callback ends up + # seeing. ``args`` stays deliberately free-form because scripts declare their own + # parameters, and :meth:`InlineSkillScript.run` expands it alongside these runtime + # 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. + async def _read_resource( + ctx: FunctionInvocationContext, + skill_name: str, + resource_name: str, + ) -> Any: + return await self._read_skill_resource(skills, skill_name, resource_name, **ctx.kwargs) async def _run_script( - skill_name: str, script_name: str, args: dict[str, Any] | list[str] | None = None, **kwargs: Any + ctx: FunctionInvocationContext, + skill_name: str, + script_name: str, + args: dict[str, Any] | list[str] | None = None, ) -> Any: - return await self._run_skill_script(skills, skill_name, script_name, args, **kwargs) + return await self._run_skill_script(skills, skill_name, script_name, args, **ctx.kwargs) return [ FunctionTool( @@ -2569,6 +2598,7 @@ async def _run_script( "skill_name": {"type": "string", "description": "The name of the skill to load."}, }, "required": ["skill_name"], + "additionalProperties": False, }, ), FunctionTool( @@ -2586,6 +2616,7 @@ async def _run_script( }, }, "required": ["skill_name", "resource_name"], + "additionalProperties": False, }, ), FunctionTool( @@ -2637,6 +2668,9 @@ async def _run_script( }, }, "required": ["skill_name", "script_name"], + # Scripts declare their own parameters, so the nested ``args`` object + # above stays free-form; only the outer envelope is closed. + "additionalProperties": False, }, ), ] @@ -2692,8 +2726,14 @@ async def _run_skill_script( args: Optional arguments for the script, provided by the agent/LLM. **kwargs: Runtime keyword arguments forwarded only to script - functions that accept ``**kwargs`` (e.g. arguments passed via - ``agent.run(user_id="123")``). + 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. Returns: The script result. Returns a user-facing error string for @@ -2738,8 +2778,11 @@ async def _read_skill_resource( skill_name: The name of the owning skill. resource_name: The resource name to look up (case-insensitive). **kwargs: Runtime keyword arguments forwarded to resource functions - that accept ``**kwargs`` (e.g. arguments passed via - ``agent.run(user_id="123")``). + that accept ``**kwargs``. These carry host request context + supplied via + ``agent.run(..., function_invocation_kwargs={"user_id": "123"})`` + and delivered through :class:`FunctionInvocationContext`; they are + never sourced from model-supplied tool arguments. Returns: The resource content (any type). Returns a user-facing error diff --git a/python/packages/core/tests/core/test_skills.py b/python/packages/core/tests/core/test_skills.py index 94b98f2114e..35ece2b1771 100644 --- a/python/packages/core/tests/core/test_skills.py +++ b/python/packages/core/tests/core/test_skills.py @@ -16,8 +16,10 @@ import pytest from agent_framework import ( + Agent, AggregatingSkillsSource, CachingSkillsSource, + ChatResponse, ClassSkill, Content, DeduplicatingSkillsSource, @@ -26,6 +28,7 @@ FileSkillsSource, InlineSkill, InMemorySkillsSource, + Message, SessionContext, Skill, SkillFrontmatter, @@ -37,6 +40,7 @@ SkillsSource, SkillsSourceContext, ) +from agent_framework._middleware import FunctionInvocationContext from agent_framework._skills import ( DEFAULT_RESOURCE_EXTENSIONS, DEFAULT_SCRIPT_EXTENSIONS, @@ -49,7 +53,7 @@ _SkillPathScope, ) -from .conftest import MockAgent, MockAgentSession, create_junction_or_skip +from .conftest import MockAgent, MockAgentSession, MockBaseChatClient, create_junction_or_skip # Cross-platform absolute path prefix for tests _ABS = "C:\\skills" if os.name == "nt" else "/skills" @@ -80,6 +84,17 @@ async def _noop_script_runner(skill: Any, script: Any, args: Any = None) -> None return +def _invocation_context(tool: Any, **runtime_kwargs: Any) -> FunctionInvocationContext: + """Build the invocation context a skill tool handler receives at dispatch time. + + ``read_skill_resource`` and ``run_skill_script`` take host runtime values through + :class:`FunctionInvocationContext` rather than through model-supplied arguments, so + tests that call ``tool.func(...)`` directly must supply one. Pass runtime values as + keyword arguments to stand in for ``agent.run(function_invocation_kwargs=...)``. + """ + return FunctionInvocationContext(function=tool, arguments={}, kwargs=runtime_kwargs) + + class _CountingSkillsSource(SkillsSource): """Test source that records how many times ``get_skills`` is called.""" @@ -3676,7 +3691,12 @@ def my_function(key: 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 run_tool.func(skill_name="my-skill", script_name="s1", args={"key": "hello"}) + result = await run_tool.func( + _invocation_context(run_tool), + skill_name="my-skill", + script_name="s1", + args={"key": "hello"}, + ) assert result == "executed: hello" @@ -3723,7 +3743,7 @@ async def test_read_skill_resource_tool_returns_content(self) -> None: provider = SkillsProvider([skill]) await _init_provider(provider) read_tool = next(t for t in _ctx(provider)[2] if hasattr(t, "name") and t.name == "read_skill_resource") - result = await read_tool.func(skill_name="my-skill", resource_name="ref") + result = await read_tool.func(_invocation_context(read_tool), skill_name="my-skill", resource_name="ref") assert result == "reference data" async def test_file_skills_with_custom_runner(self, tmp_path: Path) -> None: @@ -3791,7 +3811,12 @@ def sync_runner(skill, script, args=None): ) 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 run_tool.func(skill_name="my-skill", script_name="scripts/run.py", args={"key": "val"}) + result = await run_tool.func( + _invocation_context(run_tool), + skill_name="my-skill", + script_name="scripts/run.py", + args={"key": "val"}, + ) assert result == "sync: scripts/run.py args={'key': 'val'}" async def test_file_skills_with_callback_runner(self, tmp_path: Path) -> None: @@ -3861,12 +3886,12 @@ async def test_file_script_error_without_runner(self) -> None: run_tool = next(t for t in _ctx(provider)[2] if hasattr(t, "name") and t.name == "run_skill_script") # Code script works - result = await run_tool.func(skill_name="my-skill", script_name="code-s") + result = await run_tool.func(_invocation_context(run_tool), skill_name="my-skill", script_name="code-s") assert result == "ok" # File script without runner propagates an error by default with pytest.raises(TypeError, match="requires a FileSkill"): - await run_tool.func(skill_name="my-skill", script_name="file-s") + await run_tool.func(_invocation_context(run_tool), skill_name="my-skill", script_name="file-s") async def test_async_code_script_runs_directly(self) -> None: async def async_func(x: int = 0) -> str: @@ -3878,7 +3903,12 @@ async def async_func(x: int = 0) -> 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 run_tool.func(skill_name="my-skill", script_name="s1", args={"x": 42}) + result = await run_tool.func( + _invocation_context(run_tool), + skill_name="my-skill", + script_name="s1", + args={"x": 42}, + ) assert result == "async: 42" async def test_code_script_returns_object(self) -> None: @@ -3893,7 +3923,7 @@ def returns_dict() -> dict: 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 run_tool.func(skill_name="my-skill", script_name="s1") + result = await run_tool.func(_invocation_context(run_tool), skill_name="my-skill", script_name="s1") assert result == {"status": "ok", "value": 42} async def test_code_script_returns_none(self) -> None: @@ -3904,7 +3934,7 @@ async def test_code_script_returns_none(self) -> None: 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 run_tool.func(skill_name="my-skill", script_name="s1") + result = await run_tool.func(_invocation_context(run_tool), skill_name="my-skill", script_name="s1") assert result is None async def test_script_with_path_errors_without_runner(self) -> None: @@ -3918,12 +3948,12 @@ async def test_script_with_path_errors_without_runner(self) -> None: run_tool = next(t for t in _ctx(provider)[2] if hasattr(t, "name") and t.name == "run_skill_script") # Code-only script still works - result = await run_tool.func(skill_name="my-skill", script_name="code-s") + result = await run_tool.func(_invocation_context(run_tool), skill_name="my-skill", script_name="code-s") assert result == "ok" # Path+function script without runner propagates an error by default with pytest.raises(TypeError, match="requires a FileSkill"): - await run_tool.func(skill_name="my-skill", script_name="path-s") + await run_tool.func(_invocation_context(run_tool), skill_name="my-skill", script_name="path-s") async def test_run_skill_script_error_on_missing_skill(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") @@ -3932,7 +3962,7 @@ async def test_run_skill_script_error_on_missing_skill(self) -> None: 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 run_tool.func(skill_name="nonexistent", script_name="s1") + result = await run_tool.func(_invocation_context(run_tool), skill_name="nonexistent", script_name="s1") assert "Error" in result assert "nonexistent" in result @@ -4003,7 +4033,7 @@ async def test_run_skill_script_error_on_missing_script(self) -> None: 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 run_tool.func(skill_name="my-skill", script_name="nonexistent") + result = await run_tool.func(_invocation_context(run_tool), skill_name="my-skill", script_name="nonexistent") assert "Error" in result assert "nonexistent" in result @@ -4015,10 +4045,10 @@ async def test_run_skill_script_error_on_empty_names(self) -> None: 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 run_tool.func(skill_name="", script_name="s1") + result = await run_tool.func(_invocation_context(run_tool), skill_name="", script_name="s1") assert "Error" in result - result = await run_tool.func(skill_name="my-skill", script_name="") + result = await run_tool.func(_invocation_context(run_tool), skill_name="my-skill", script_name="") assert "Error" in result async def test_instructions_include_script_runner_hints(self) -> None: @@ -4239,7 +4269,7 @@ def failing_script() -> str: 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(RuntimeError, match="Something went wrong"): - await run_tool.func(skill_name="my-skill", script_name="boom") + await run_tool.func(_invocation_context(run_tool), skill_name="my-skill", script_name="boom") async def test_custom_template_without_runner_placeholder_raises(self) -> None: """Providers accept custom templates without {runner_instructions}.""" @@ -6167,7 +6197,7 @@ async def source_runner(skill: Any, script: Any, args: Any = None) -> str: # The source-level runner should be discovered and used run_tool = next(t for t in _ctx(provider)[2] if hasattr(t, "name") and t.name == "run_skill_script") - result = await run_tool.func(skill_name="my-skill", script_name="scripts/run.py") + result = await run_tool.func(_invocation_context(run_tool), skill_name="my-skill", script_name="scripts/run.py") assert result == "source" assert call_log == ["source"] @@ -6648,7 +6678,12 @@ def runner(skill: Any, script: Any, args: Any = None) -> 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 run_tool.func(skill_name="my-skill", script_name="run.py", args=["input.docx", "--verbose"]) + result = await run_tool.func( + _invocation_context(run_tool), + skill_name="my-skill", + script_name="run.py", + args=["input.docx", "--verbose"], + ) assert result == "list_result" assert captured["args"] == ["input.docx", "--verbose"] @@ -6661,7 +6696,7 @@ async def test_run_skill_script_inline_with_list_args_propagates_error(self) -> 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, match="requires keyword arguments"): - await run_tool.func(skill_name="my-skill", script_name="s1", args=["arg1"]) + await run_tool.func(_invocation_context(run_tool), skill_name="my-skill", script_name="s1", args=["arg1"]) async def test_file_skill_content_includes_scripts_block(self) -> None: """FileSkill.content appends an block when scripts are present.""" @@ -6846,7 +6881,12 @@ def greet(name: str = "world") -> 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 run_tool.func(skill_name="my-skill", script_name="greet", args={"q": "Eve"}) + result = await run_tool.func( + _invocation_context(run_tool), + skill_name="my-skill", + script_name="greet", + args={"q": "Eve"}, + ) assert result == "hello Eve" async def test_inline_string_args_without_parser_raises(self) -> None: @@ -6855,3 +6895,356 @@ async def test_inline_string_args_without_parser_raises(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="s", description="d"), instructions="c") with pytest.raises(TypeError, match="argument_parser"): await script.run(skill, args='{"name": "Alice"}') + + +# --------------------------------------------------------------------------- +# Tests: runtime kwargs provenance (host context vs. model arguments) +# --------------------------------------------------------------------------- + + +def _function_call_response(*, call_id: str, name: str, arguments: str) -> ChatResponse: + """Build a chat response containing a single tool call.""" + return ChatResponse( + messages=[ + Message( + role="assistant", + contents=[Content.from_function_call(call_id=call_id, name=name, arguments=arguments)], + ) + ] + ) + + +class TestSkillsRuntimeKwargsProvenance: + """End-to-end tests that runtime kwargs reach skills from the host, not the model. + + The framework exposes two distinct argument channels for skill tools: + + * **Trusted** — host values supplied via ``agent.run(function_invocation_kwargs=...)``, + delivered through :class:`FunctionInvocationContext`. + * **Untrusted** — the arguments the model produced for the tool call, which are + constrained by the tool's advertised JSON schema. + + Runtime values such as ``tenant_id`` or ``auth_token`` must come only from the + trusted channel. These tests drive the real ``Agent.run`` dispatch path rather + than the provider's private helpers, so they cover schema validation and context + injection — the layers where the two channels are told apart. + """ + + @staticmethod + def _tenant_skill(calls: list[dict[str, Any]]) -> InlineSkill: + """Build a skill whose resource and script record the runtime kwargs they receive.""" + skill = InlineSkill( + frontmatter=SkillFrontmatter(name="tenant-data", description="Tenant-scoped data."), + instructions="Body", + ) + + @skill.resource(name="account-record", description="The current tenant's account record.") + def account_record(**kwargs: Any) -> Any: + calls.append(dict(kwargs)) + return f"record for {kwargs.get('tenant_id', 'MISSING')}" + + @skill.script(name="export-report", description="Export the current tenant's report.") + def export_report(report: str, **kwargs: Any) -> Any: + calls.append(dict(kwargs)) + return f"{report} for {kwargs.get('tenant_id', 'MISSING')}" + + return skill + + @staticmethod + def _agent(client: MockBaseChatClient, skill: InlineSkill) -> Agent: + """Build an agent exposing the skill's tools without approval prompts.""" + provider = SkillsProvider( + [skill], + disable_load_skill_approval=True, + disable_read_skill_resource_approval=True, + disable_run_skill_script_approval=True, + ) + return Agent(client=client, context_providers=[provider]) + + async def test_resource_receives_host_runtime_kwargs(self, chat_client_base: MockBaseChatClient) -> None: + """Host ``function_invocation_kwargs`` must reach a callable resource. + + ``SkillsProvider`` documents ``agent.run(function_invocation_kwargs=...)`` as the + channel for request-scoped values, so a resource that accepts ``**kwargs`` has to + observe them on the real dispatch path. + """ + calls: list[dict[str, Any]] = [] + skill = self._tenant_skill(calls) + + chat_client_base.run_responses = [ + _function_call_response( + call_id="call_1", + name="read_skill_resource", + arguments='{"skill_name": "tenant-data", "resource_name": "account-record"}', + ), + ChatResponse(messages=[Message(role="assistant", contents=["Done!"])]), + ] + + agent = self._agent(chat_client_base, skill) + session = agent.create_session() + await agent.run( + "Read the account record.", + function_invocation_kwargs={"tenant_id": "host-tenant"}, + session=session, + ) + + assert calls == [{"tenant_id": "host-tenant", "session": session}] + + async def test_script_receives_host_runtime_kwargs(self, chat_client_base: MockBaseChatClient) -> None: + """Host ``function_invocation_kwargs`` must reach a callable script.""" + calls: list[dict[str, Any]] = [] + skill = self._tenant_skill(calls) + + chat_client_base.run_responses = [ + _function_call_response( + call_id="call_1", + name="run_skill_script", + arguments=( + '{"skill_name": "tenant-data", "script_name": "export-report", "args": {"report": "summary"}}' + ), + ), + ChatResponse(messages=[Message(role="assistant", contents=["Done!"])]), + ] + + agent = self._agent(chat_client_base, skill) + session = agent.create_session() + await agent.run( + "Export the report.", + function_invocation_kwargs={"tenant_id": "host-tenant"}, + session=session, + ) + + assert calls == [{"tenant_id": "host-tenant", "session": session}] + + async def test_resource_rejects_undeclared_model_argument(self, chat_client_base: MockBaseChatClient) -> None: + """A model argument outside the advertised schema must not become a runtime kwarg. + + ``read_skill_resource`` advertises only ``skill_name`` and ``resource_name``. An + extra top-level property returned by the model must be rejected before the + resource runs, otherwise the model — not the host — chooses the tenant. + """ + calls: list[dict[str, Any]] = [] + skill = self._tenant_skill(calls) + + chat_client_base.run_responses = [ + _function_call_response( + call_id="call_1", + name="read_skill_resource", + arguments=( + '{"skill_name": "tenant-data", "resource_name": "account-record", "tenant_id": "model-tenant"}' + ), + ), + ChatResponse(messages=[Message(role="assistant", contents=["Done!"])]), + ] + + await self._agent(chat_client_base, skill).run( + "Read the account record.", + function_invocation_kwargs={"tenant_id": "host-tenant"}, + ) + + assert calls == [] + + async def test_script_rejects_undeclared_model_argument(self, chat_client_base: MockBaseChatClient) -> None: + """An undeclared top-level model argument must not reach a script as a runtime kwarg. + + Free-form script parameters belong inside the nested ``args`` property, which + stays permissive; the outer schema must not. + """ + calls: list[dict[str, Any]] = [] + skill = self._tenant_skill(calls) + + chat_client_base.run_responses = [ + _function_call_response( + call_id="call_1", + name="run_skill_script", + arguments=( + '{"skill_name": "tenant-data", "script_name": "export-report", ' + '"args": {"report": "summary"}, "tenant_id": "model-tenant"}' + ), + ), + ChatResponse(messages=[Message(role="assistant", contents=["Done!"])]), + ] + + await self._agent(chat_client_base, skill).run( + "Export the report.", + function_invocation_kwargs={"tenant_id": "host-tenant"}, + ) + + assert calls == [] + + async def test_script_kwargs_also_receive_undeclared_nested_args( + self, chat_client_base: MockBaseChatClient + ) -> None: + """Undeclared entries in the nested ``args`` object still reach a script's ``**kwargs``. + + This pins the deliberate limit of the outer ``additionalProperties: False`` + rule. Scripts declare their own parameters, so ``args`` stays free-form and + :meth:`InlineSkillScript.run` expands it alongside the host runtime kwargs. + A script therefore cannot treat a name in its ``**kwargs`` as host-supplied. + Closing ``args`` would be a deliberate behavior change and should fail here. + """ + calls: list[dict[str, Any]] = [] + skill = self._tenant_skill(calls) + + chat_client_base.run_responses = [ + _function_call_response( + call_id="call_1", + name="run_skill_script", + arguments=( + '{"skill_name": "tenant-data", "script_name": "export-report", ' + '"args": {"report": "summary", "region": "model-region"}}' + ), + ), + ChatResponse(messages=[Message(role="assistant", contents=["Done!"])]), + ] + + agent = self._agent(chat_client_base, skill) + session = agent.create_session() + await agent.run( + "Export the report.", + function_invocation_kwargs={"tenant_id": "host-tenant"}, + session=session, + ) + + assert calls == [{"tenant_id": "host-tenant", "region": "model-region", "session": session}] + + async def test_model_cannot_override_host_value_through_nested_args( + self, chat_client_base: MockBaseChatClient + ) -> None: + """A model value colliding with a host runtime kwarg must not win. + + ``args`` is free-form, so the model can name a key the host also supplies. + Binding both raises :class:`TypeError` in the script call, which the + function-invocation pipeline turns into a tool error. The point is that the + collision fails closed: the script never runs with the model's value. + """ + calls: list[dict[str, Any]] = [] + skill = self._tenant_skill(calls) + + chat_client_base.run_responses = [ + _function_call_response( + call_id="call_1", + name="run_skill_script", + arguments=( + '{"skill_name": "tenant-data", "script_name": "export-report", ' + '"args": {"report": "summary", "tenant_id": "model-tenant"}}' + ), + ), + ChatResponse(messages=[Message(role="assistant", contents=["Done!"])]), + ] + + response = await self._agent(chat_client_base, skill).run( + "Export the report.", + function_invocation_kwargs={"tenant_id": "host-tenant"}, + ) + + assert calls == [] + # Assert the tool was actually dispatched and failed on the collision, so this + # cannot pass for the unrelated reason that the call never reached the script. + exceptions = [ + content.exception + for message in response.messages + for content in message.contents + if content.type == "function_result" and content.exception + ] + assert any("multiple values for keyword argument 'tenant_id'" in str(exc) for exc in exceptions) + + async def test_host_runtime_kwargs_win_over_model_argument(self, chat_client_base: MockBaseChatClient) -> None: + """When the model supplies a colliding value, the host value must still be used. + + This is the cross-tenant case: the resource must never be invoked with the + model's tenant, whether the extra argument is rejected outright or ignored. + """ + calls: list[dict[str, Any]] = [] + skill = self._tenant_skill(calls) + + chat_client_base.run_responses = [ + _function_call_response( + call_id="call_1", + name="read_skill_resource", + arguments=( + '{"skill_name": "tenant-data", "resource_name": "account-record", "tenant_id": "model-tenant"}' + ), + ), + ChatResponse(messages=[Message(role="assistant", contents=["Done!"])]), + ] + + await self._agent(chat_client_base, skill).run( + "Read the account record.", + function_invocation_kwargs={"tenant_id": "host-tenant"}, + ) + + assert all(call.get("tenant_id") != "model-tenant" for call in calls) + + async def test_resource_without_host_kwargs_receives_session(self, chat_client_base: MockBaseChatClient) -> None: + """Without caller-supplied kwargs, only the framework session is forwarded.""" + calls: list[dict[str, Any]] = [] + skill = self._tenant_skill(calls) + + chat_client_base.run_responses = [ + _function_call_response( + call_id="call_1", + name="read_skill_resource", + arguments='{"skill_name": "tenant-data", "resource_name": "account-record"}', + ), + ChatResponse(messages=[Message(role="assistant", contents=["Done!"])]), + ] + + agent = self._agent(chat_client_base, skill) + session = agent.create_session() + await agent.run("Read the account record.", session=session) + + assert calls == [{"session": session}] + + @pytest.mark.parametrize( + "tool_name", + ["load_skill", "read_skill_resource", "run_skill_script"], + ) + async def test_outer_schema_forbids_additional_properties(self, tool_name: str) -> None: + """Each skill tool must close its outer schema to undeclared model arguments. + + ``_validate_arguments_against_schema`` rejects unexpected arguments only when + ``additionalProperties`` is explicitly ``False``, so omitting the flag silently + admits them into the handler's ``**kwargs``. + """ + skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") + skill._scripts.append(InlineSkillScript(name="s1", function=lambda: None)) + + provider = SkillsProvider([skill]) + await _init_provider(provider) + tool = next(t for t in _ctx(provider)[2] if hasattr(t, "name") and t.name == tool_name) + + assert tool.parameters().get("additionalProperties") is False + + async def test_script_args_property_still_allows_free_form_values(self) -> None: + """Closing the outer schema must not restrict the nested ``args`` property. + + Scripts declare their own parameters, so ``args`` intentionally stays open. + """ + skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") + skill._scripts.append(InlineSkillScript(name="s1", function=lambda: None)) + + provider = SkillsProvider([skill]) + await _init_provider(provider) + tool = next(t for t in _ctx(provider)[2] if hasattr(t, "name") and t.name == "run_skill_script") + + args_schema = tool.parameters()["properties"]["args"] + object_variant = next(v for v in args_schema["oneOf"] if v.get("type") == "object") + assert object_variant["additionalProperties"] is True + + async def test_resource_handler_declares_invocation_context(self) -> None: + """The provider's tool handlers must opt in to trusted context injection. + + ``FunctionTool`` forwards host runtime kwargs only to handlers that declare a + :class:`FunctionInvocationContext` parameter; without one the trusted values are + dropped before the handler runs. + """ + skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") + skill._scripts.append(InlineSkillScript(name="s1", function=lambda: None)) + + provider = SkillsProvider([skill]) + await _init_provider(provider) + tools = {t.name: t for t in _ctx(provider)[2] if hasattr(t, "name")} + + assert tools["read_skill_resource"]._context_parameter_name is not None + assert tools["run_skill_script"]._context_parameter_name is not None 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 c3589b4b2a5..d6bb4b72ac0 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 @@ -38,6 +38,14 @@ 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. + Code-defined skills can be combined with file-based skills in a single SkillsProvider — see the mixed_skills sample. """ @@ -95,12 +103,23 @@ def conversion_policy(**kwargs: Any) -> Any: When the resource function accepts ``**kwargs``, runtime keyword arguments passed to ``agent.run()`` are forwarded automatically. + These runtime values are *host-controlled request context*: they come only + from the application calling ``agent.run()``, never from the model. That + distinction matters for values that select authority — a tenant ID, a user + ID, or an auth token — so this resource treats a missing ``precision`` as a + bug rather than silently falling back to a default and masking it. + Args: **kwargs: Runtime keyword arguments from ``agent.run()``. For example, ``agent.run(..., function_invocation_kwargs={"precision": 2})`` makes ``kwargs["precision"]`` available here. """ - precision = kwargs.get("precision", 4) + if "precision" not in kwargs: + raise RuntimeError( + "Expected host-supplied 'precision' in runtime kwargs. Runtime context must reach " + "resources via agent.run(function_invocation_kwargs=...)." + ) + precision = kwargs["precision"] return dedent(f"""\ # Conversion Policy @@ -119,17 +138,27 @@ def convert_units(value: float, factor: float, **kwargs: Any) -> str: The caller looks up the correct factor from the conversion-tables resource and passes it here. + 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. + Args: value: The numeric value to convert. factor: Conversion factor from the conversion table. - **kwargs: Runtime keyword arguments from ``agent.run()``. - The ``precision`` kwarg controls how many decimal places - the result is rounded to (default 4). + **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. Returns: JSON string with the inputs and converted result. """ - precision = kwargs.get("precision", 4) + if "precision" not in kwargs: + raise RuntimeError( + "This sample requires 'precision'. main() supplies it through agent.run(function_invocation_kwargs=...)." + ) + precision = kwargs["precision"] result = round(value * factor, precision) return json.dumps({"value": value, "factor": factor, "result": result}) From 1b27ba22396d91b9422c532714fdebe2147e13aa Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Tue, 15 Sep 2026 12:36:22 +0100 Subject: [PATCH 2/3] Python: Pass runtime mappings through skill helpers Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../packages/core/agent_framework/_skills.py | 41 +++++-- .../packages/core/tests/core/test_skills.py | 111 ++++++++++++++++-- 2 files changed, 135 insertions(+), 17 deletions(-) diff --git a/python/packages/core/agent_framework/_skills.py b/python/packages/core/agent_framework/_skills.py index 9b67c55b679..e4101609d28 100644 --- a/python/packages/core/agent_framework/_skills.py +++ b/python/packages/core/agent_framework/_skills.py @@ -55,7 +55,7 @@ import time import zipfile from abc import ABC, abstractmethod -from collections.abc import Callable, Sequence +from collections.abc import Callable, Mapping, Sequence from dataclasses import dataclass from datetime import timedelta from enum import Enum @@ -2576,7 +2576,12 @@ async def _read_resource( skill_name: str, resource_name: str, ) -> Any: - return await self._read_skill_resource(skills, skill_name, resource_name, **ctx.kwargs) + return await self._read_skill_resource( + skills, + skill_name, + resource_name, + runtime_kwargs=ctx.kwargs, + ) async def _run_script( ctx: FunctionInvocationContext, @@ -2584,7 +2589,13 @@ async def _run_script( script_name: str, args: dict[str, Any] | list[str] | None = None, ) -> Any: - return await self._run_skill_script(skills, skill_name, script_name, args, **ctx.kwargs) + return await self._run_skill_script( + skills, + skill_name, + script_name, + args, + runtime_kwargs=ctx.kwargs, + ) return [ FunctionTool( @@ -2712,7 +2723,8 @@ async def _run_skill_script( skill_name: str, script_name: str, args: dict[str, Any] | list[str] | None = None, - **kwargs: Any, + *, + runtime_kwargs: Mapping[str, Any] | None = None, ) -> Any: """Run a named script from a skill. @@ -2725,7 +2737,7 @@ 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. - **kwargs: Runtime keyword arguments forwarded only to script + 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"})`` @@ -2733,7 +2745,11 @@ async def _run_skill_script( 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. + 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. Returns: The script result. Returns a user-facing error string for @@ -2759,13 +2775,18 @@ async def _run_skill_script( return f"Error: Script '{script_name}' not found in skill '{skill_name}'." try: - return await script.run(skill, args, **kwargs) + return await script.run(skill, args, **(runtime_kwargs or {})) except Exception: logger.exception("Error running script '%s' in skill '%s'", script_name, skill_name) raise async def _read_skill_resource( - self, skills: Sequence[Skill], skill_name: str, resource_name: str, **kwargs: Any + self, + skills: Sequence[Skill], + skill_name: str, + resource_name: str, + *, + runtime_kwargs: Mapping[str, Any] | None = None, ) -> Any: """Read a named resource from a skill. @@ -2777,7 +2798,7 @@ async def _read_skill_resource( skills: The skills to look up the skill from. skill_name: The name of the owning skill. resource_name: The resource name to look up (case-insensitive). - **kwargs: Runtime keyword arguments forwarded to resource functions + runtime_kwargs: Runtime keyword arguments forwarded to resource functions that accept ``**kwargs``. These carry host request context supplied via ``agent.run(..., function_invocation_kwargs={"user_id": "123"})`` @@ -2811,7 +2832,7 @@ async def _read_skill_resource( return f"Error: Resource '{resource_name}' not found in skill '{skill_name}'." try: - return await resource.read(**kwargs) + return await resource.read(**(runtime_kwargs or {})) except Exception: logger.exception("Failed to read resource '%s' from skill '%s'", resource_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 35ece2b1771..353dce10cf5 100644 --- a/python/packages/core/tests/core/test_skills.py +++ b/python/packages/core/tests/core/test_skills.py @@ -1558,7 +1558,10 @@ def get_user_config(**kwargs: Any) -> Any: provider = SkillsProvider([skill]) await _init_provider(provider) result = await provider._read_skill_resource( - _raw_skills(provider), "prog-skill", "get_user_config", user_id="user_123" + _raw_skills(provider), + "prog-skill", + "get_user_config", + runtime_kwargs={"user_id": "user_123"}, ) assert result == "config for user_123" @@ -1575,7 +1578,10 @@ async def get_user_data(**kwargs: Any) -> Any: provider = SkillsProvider([skill]) await _init_provider(provider) result = await provider._read_skill_resource( - _raw_skills(provider), "prog-skill", "get_user_data", auth_token="abc" + _raw_skills(provider), + "prog-skill", + "get_user_data", + runtime_kwargs={"auth_token": "abc"}, ) assert result == "data with token=abc" @@ -1592,7 +1598,10 @@ def static_resource() -> Any: provider = SkillsProvider([skill]) await _init_provider(provider) result = await provider._read_skill_resource( - _raw_skills(provider), "prog-skill", "static_resource", user_id="ignored" + _raw_skills(provider), + "prog-skill", + "static_resource", + runtime_kwargs={"user_id": "ignored"}, ) assert result == "static content" @@ -3977,7 +3986,11 @@ def greet(name: str, **kwargs: Any) -> str: provider = SkillsProvider([skill]) await _init_provider(provider) result = await provider._run_skill_script( - _raw_skills(provider), "my-skill", "greet", args={"name": "Alice"}, user_id="u42" + _raw_skills(provider), + "my-skill", + "greet", + args={"name": "Alice"}, + runtime_kwargs={"user_id": "u42"}, ) assert result == "Hello Alice (user=u42)" @@ -3992,7 +4005,11 @@ async def fetch(url: str, **kwargs: Any) -> str: provider = SkillsProvider([skill]) await _init_provider(provider) result = await provider._run_skill_script( - _raw_skills(provider), "my-skill", "fetch", args={"url": "http://x"}, auth_token="abc" + _raw_skills(provider), + "my-skill", + "fetch", + args={"url": "http://x"}, + runtime_kwargs={"auth_token": "abc"}, ) assert result == "fetched http://x with token=abc" @@ -4007,7 +4024,11 @@ def simple(query: str) -> str: provider = SkillsProvider([skill]) await _init_provider(provider) result = await provider._run_skill_script( - _raw_skills(provider), "my-skill", "simple", args={"query": "test"}, user_id="ignored" + _raw_skills(provider), + "my-skill", + "simple", + args={"query": "test"}, + runtime_kwargs={"user_id": "ignored"}, ) assert result == "result: test" @@ -4023,7 +4044,11 @@ def process(**kwargs: Any) -> str: await _init_provider(provider) with pytest.raises(TypeError): await provider._run_skill_script( - _raw_skills(provider), "my-skill", "process", args={"mode": "llm-value"}, mode="runtime-value" + _raw_skills(provider), + "my-skill", + "process", + args={"mode": "llm-value"}, + runtime_kwargs={"mode": "runtime-value"}, ) async def test_run_skill_script_error_on_missing_script(self) -> None: @@ -6990,6 +7015,38 @@ async def test_resource_receives_host_runtime_kwargs(self, chat_client_base: Moc assert calls == [{"tenant_id": "host-tenant", "session": session}] + async def test_resource_runtime_kwargs_do_not_collide_with_dispatcher_names( + self, chat_client_base: MockBaseChatClient + ) -> None: + """Private resource-dispatcher names must remain valid runtime kwarg names.""" + calls: list[dict[str, Any]] = [] + skill = self._tenant_skill(calls) + + chat_client_base.run_responses = [ + _function_call_response( + call_id="call_1", + name="read_skill_resource", + arguments='{"skill_name": "tenant-data", "resource_name": "account-record"}', + ), + ChatResponse(messages=[Message(role="assistant", contents=["Done!"])]), + ] + + runtime_kwargs = { + "skills": "runtime-skills", + "skill_name": "runtime-skill-name", + "resource_name": "runtime-resource-name", + "runtime_kwargs": "runtime-runtime-kwargs", + } + agent = self._agent(chat_client_base, skill) + session = agent.create_session() + await agent.run( + "Read the account record.", + function_invocation_kwargs=runtime_kwargs, + session=session, + ) + + assert calls == [{**runtime_kwargs, "session": session}] + async def test_script_receives_host_runtime_kwargs(self, chat_client_base: MockBaseChatClient) -> None: """Host ``function_invocation_kwargs`` must reach a callable script.""" calls: list[dict[str, Any]] = [] @@ -7016,6 +7073,46 @@ async def test_script_receives_host_runtime_kwargs(self, chat_client_base: MockB assert calls == [{"tenant_id": "host-tenant", "session": session}] + async def test_script_runtime_kwargs_do_not_collide_with_dispatcher_names( + self, chat_client_base: MockBaseChatClient + ) -> None: + """Private script-dispatcher names must remain valid runtime kwarg names. + + ``skill`` and ``args`` are deliberately absent: those are parameters of the + public :meth:`SkillScript.run` signature, which still expands runtime kwargs + and therefore still constrains those two names. Closing that gap would change + a public contract, so only the private dispatch chain is covered here. + """ + calls: list[dict[str, Any]] = [] + skill = self._tenant_skill(calls) + + chat_client_base.run_responses = [ + _function_call_response( + call_id="call_1", + name="run_skill_script", + arguments=( + '{"skill_name": "tenant-data", "script_name": "export-report", "args": {"report": "summary"}}' + ), + ), + ChatResponse(messages=[Message(role="assistant", contents=["Done!"])]), + ] + + runtime_kwargs = { + "skills": "runtime-skills", + "skill_name": "runtime-skill-name", + "script_name": "runtime-script-name", + "runtime_kwargs": "runtime-runtime-kwargs", + } + agent = self._agent(chat_client_base, skill) + session = agent.create_session() + await agent.run( + "Export the report.", + function_invocation_kwargs=runtime_kwargs, + session=session, + ) + + assert calls == [{**runtime_kwargs, "session": session}] + async def test_resource_rejects_undeclared_model_argument(self, chat_client_base: MockBaseChatClient) -> None: """A model argument outside the advertised schema must not become a runtime kwarg. From 716929715f47443b8c0477ebdd01e772fd64899e Mon Sep 17 00:00:00 2001 From: SergeyMenshykh Date: Tue, 15 Sep 2026 12:58:20 +0100 Subject: [PATCH 3/3] Python: Specify agent options type in skill tests Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- python/packages/core/tests/core/test_skills.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/packages/core/tests/core/test_skills.py b/python/packages/core/tests/core/test_skills.py index 353dce10cf5..bf6fc571b77 100644 --- a/python/packages/core/tests/core/test_skills.py +++ b/python/packages/core/tests/core/test_skills.py @@ -19,6 +19,7 @@ Agent, AggregatingSkillsSource, CachingSkillsSource, + ChatOptions, ChatResponse, ClassSkill, Content, @@ -6984,7 +6985,7 @@ def _agent(client: MockBaseChatClient, skill: InlineSkill) -> Agent: disable_read_skill_resource_approval=True, disable_run_skill_script_approval=True, ) - return Agent(client=client, context_providers=[provider]) + return Agent[ChatOptions[None]](client=client, context_providers=[provider]) async def test_resource_receives_host_runtime_kwargs(self, chat_client_base: MockBaseChatClient) -> None: """Host ``function_invocation_kwargs`` must reach a callable resource.