From 612069f255262602ad7e75d434c82311093ef2c3 Mon Sep 17 00:00:00 2001 From: eavanvalkenburg Date: Thu, 27 Aug 2026 16:44:59 +0200 Subject: [PATCH 1/2] Python: restore agent middleware API contract Remove the agent-hooks extra from the core package and restore sequence-only middleware inputs for agents and the harness.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- python/PACKAGE_STATUS.md | 5 +- .../core/agent_framework/_agent_hooks.py | 6 +-- .../packages/core/agent_framework/_agents.py | 29 ++++------- .../core/agent_framework/_harness/_agent.py | 9 ++-- .../core/agent_framework/_harness/_agent.pyi | 2 +- .../core/agent_framework/_middleware.py | 23 ++++----- .../core/agent_framework/observability.py | 8 +-- python/packages/core/pyproject.toml | 5 -- .../core/tests/core/test_agent_hooks.py | 17 +++---- .../core/tests/core/test_harness_agent.py | 19 ++++++- .../tests/core/test_middleware_with_agent.py | 49 ++++++++----------- python/pyproject.toml | 3 +- python/uv.lock | 6 +-- 13 files changed, 81 insertions(+), 100 deletions(-) diff --git a/python/PACKAGE_STATUS.md b/python/PACKAGE_STATUS.md index d14af5d0606..ff95b8368d2 100644 --- a/python/PACKAGE_STATUS.md +++ b/python/PACKAGE_STATUS.md @@ -70,9 +70,8 @@ listed below. `create_agent_hooks_middleware_from_emitter` from `agent_framework/_agent_hooks.py`, the AGENT-HOOKS-0.1 enforcement middleware bundle, and the `MiddlewareBundle` container from `agent_framework/_middleware.py` that both factories produce - (`MiddlewareBundle` itself needs no extra). Requires the opt-in - `agent-framework-core[agent-hooks]` extra (`agent-hooks-sdk`), which is deliberately - not part of `agent-framework-core[all]`. Known limitation: service-side (hosted) tool + (`MiddlewareBundle` itself needs no optional dependency). Requires installing the + `agent-hooks-sdk` package separately. Known limitation: service-side (hosted) tool execution never passes through the framework's function-invocation seam, so the `pre_tool_call`/`post_tool_call` points cannot intercept it; hosted tool calls and outputs are surfaced in the `post_model_call` content projection instead. diff --git a/python/packages/core/agent_framework/_agent_hooks.py b/python/packages/core/agent_framework/_agent_hooks.py index ea5ff016e68..ea27801f5aa 100644 --- a/python/packages/core/agent_framework/_agent_hooks.py +++ b/python/packages/core/agent_framework/_agent_hooks.py @@ -93,7 +93,7 @@ The ``agent-hooks-sdk`` dependency is optional: importing this module (and the lazy root exports) works without it, and the factories raise a descriptive ``ModuleNotFoundError`` -when the SDK is missing. Install it via ``pip install agent-framework-core[agent-hooks]``. +when the SDK is missing. Install it via ``pip install agent-hooks-sdk``. """ from __future__ import annotations @@ -162,7 +162,7 @@ _SDK_MISSING_MESSAGE = ( "The agent-hooks middleware requires the optional `agent-hooks-sdk` package. " - "Please install `agent-framework-core[agent-hooks]` (or `agent-hooks-sdk`)." + "Please install it with `pip install agent-hooks-sdk`." ) _TRIO_REQUIRED_MESSAGE = ( @@ -191,7 +191,7 @@ def _require_sdk() -> None: """Import the SDK surface this module uses at runtime, with a helpful install hint. Only a genuinely missing ``agent_hooks`` package is translated into the - install-the-extra message; anything else (a broken installation, an incompatible + SDK installation message; anything else (a broken installation, an incompatible SDK version missing symbols, a failing transitive import) propagates unchanged so real breakage is not masked as a missing extra. """ diff --git a/python/packages/core/agent_framework/_agents.py b/python/packages/core/agent_framework/_agents.py index f6224421343..179127d1781 100644 --- a/python/packages/core/agent_framework/_agents.py +++ b/python/packages/core/agent_framework/_agents.py @@ -439,7 +439,7 @@ def __init__( name: str | None = None, description: str | None = None, context_providers: Sequence[ContextProvider] | None = None, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, additional_properties: MutableMapping[str, Any] | None = None, ) -> None: """Initialize a BaseAgent instance. @@ -450,10 +450,8 @@ def __init__( name: The name of the agent, can be None. description: The description of the agent. context_providers: Context providers to include during agent invocation. - middleware: List of middleware, or a single middleware object (including a - ``MiddlewareBundle``) which is treated as a one-element list. The - constructor copies the sequence; assign to or mutate the - ``middleware`` attribute for post-construction changes. + middleware: List of middleware. The constructor copies the sequence; assign + to or mutate the ``middleware`` attribute for post-construction changes. additional_properties: Additional properties set on the agent. """ if id is None: @@ -462,12 +460,7 @@ def __init__( self.name = name self.description = description self.context_providers: list[ContextProvider] = list(context_providers or []) - # Canonicalize storage: the bare-source rule (a single middleware object or a - # MiddlewareBundle is one element) is owned by _as_middleware_list; storing a - # normalized list keeps the declared attribute type honest. - self.middleware: list[MiddlewareTypes] | None = ( - _as_middleware_list(middleware) if middleware is not None else None - ) + self.middleware: list[MiddlewareTypes] | None = list(middleware) if middleware is not None else None self.additional_properties: dict[str, Any] = cast(dict[str, Any], additional_properties or {}) def create_session(self, *, session_id: str | None = None) -> AgentSession: @@ -819,7 +812,7 @@ def __init__( tools: ToolTypes | Callable[..., Any] | Sequence[ToolTypes | Callable[..., Any]] | None = None, default_options: OptionsCoT | None = None, context_providers: Sequence[ContextProvider] | None = None, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, require_per_service_call_history_persistence: bool = False, compaction_strategy: CompactionStrategy | None = None, tokenizer: TokenizerProtocol | None = None, @@ -838,8 +831,6 @@ def __init__( description: A brief description of the agent's purpose. context_providers: Context providers to include during agent invocation. middleware: List of middleware to intercept agent and function invocations. - A single middleware object (including a ``MiddlewareBundle``) is - treated as a one-element list. require_per_service_call_history_persistence: When True (and a HistoryProvider is present), the provider always persists history via per-service-call middleware, regardless of whether the client stores history server-side. If the client does @@ -1808,7 +1799,7 @@ def run( *, stream: Literal[False] = ..., session: AgentSession | None = None, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, tools: ToolTypes | Callable[..., Any] | Sequence[ToolTypes | Callable[..., Any]] | None = None, options: ChatOptions[ResponseModelBoundT], compaction_strategy: CompactionStrategy | None = None, @@ -1824,7 +1815,7 @@ def run( *, stream: Literal[False] = ..., session: AgentSession | None = None, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, tools: ToolTypes | Callable[..., Any] | Sequence[ToolTypes | Callable[..., Any]] | None = None, options: OptionsCoT | ChatOptions[None] | None = None, compaction_strategy: CompactionStrategy | None = None, @@ -1840,7 +1831,7 @@ def run( *, stream: Literal[True], session: AgentSession | None = None, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, tools: ToolTypes | Callable[..., Any] | Sequence[ToolTypes | Callable[..., Any]] | None = None, options: OptionsCoT | ChatOptions[Any] | None = None, compaction_strategy: CompactionStrategy | None = None, @@ -1855,7 +1846,7 @@ def run( *, stream: bool = False, session: AgentSession | None = None, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, tools: ToolTypes | Callable[..., Any] | Sequence[ToolTypes | Callable[..., Any]] | None = None, options: OptionsCoT | ChatOptions[Any] | None = None, compaction_strategy: CompactionStrategy | None = None, @@ -1893,7 +1884,7 @@ def __init__( tools: ToolTypes | Callable[..., Any] | Sequence[ToolTypes | Callable[..., Any]] | None = None, default_options: OptionsCoT | None = None, context_providers: Sequence[ContextProvider] | None = None, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, require_per_service_call_history_persistence: bool = False, compaction_strategy: CompactionStrategy | None = None, tokenizer: TokenizerProtocol | None = None, diff --git a/python/packages/core/agent_framework/_harness/_agent.py b/python/packages/core/agent_framework/_harness/_agent.py index 8ad199069f4..45af3767e0f 100644 --- a/python/packages/core/agent_framework/_harness/_agent.py +++ b/python/packages/core/agent_framework/_harness/_agent.py @@ -339,7 +339,7 @@ def create_harness_agent( loop_max_iterations: int | None = DEFAULT_MAX_ITERATIONS, otel_provider_name: str | None = None, context_providers: Sequence[ContextProvider] | None = None, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, default_options: Mapping[str, Any] | None = None, ) -> Agent[OptionsCoT]: """Create a pre-configured agent with batteries included. @@ -655,11 +655,8 @@ def create_harness_agent( # Message injection is always on. It is a no-op when no messages are queued for the session, # so there is no opt-out. assembled_middleware.append(MessageInjectionMiddleware()) - # Bare-source normalization (a single middleware object or a MiddlewareBundle is - # one element) is owned by _as_middleware_list. - from .._middleware import _as_middleware_list # pyright: ignore[reportPrivateUsage] - - assembled_middleware.extend(_as_middleware_list(middleware)) + if middleware is not None: + assembled_middleware.extend(middleware) agent = Agent( client, diff --git a/python/packages/core/agent_framework/_harness/_agent.pyi b/python/packages/core/agent_framework/_harness/_agent.pyi index c9f4f318007..26a57288a74 100644 --- a/python/packages/core/agent_framework/_harness/_agent.pyi +++ b/python/packages/core/agent_framework/_harness/_agent.pyi @@ -87,6 +87,6 @@ def create_harness_agent( loop_max_iterations: int | None = DEFAULT_MAX_ITERATIONS, otel_provider_name: str | None = None, context_providers: Sequence[ContextProvider] | None = None, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, default_options: Mapping[str, Any] | None = None, ) -> Agent[OptionsCoT]: ... diff --git a/python/packages/core/agent_framework/_middleware.py b/python/packages/core/agent_framework/_middleware.py index 45a3dcbf823..dd3b67bdfb6 100644 --- a/python/packages/core/agent_framework/_middleware.py +++ b/python/packages/core/agent_framework/_middleware.py @@ -1425,7 +1425,7 @@ class AgentMiddlewareLayer: def __init__( self, *args: Any, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, **kwargs: Any, ) -> None: middleware_list = categorize_middleware(middleware) @@ -1456,7 +1456,7 @@ def run( *, stream: Literal[False] = ..., session: AgentSession | None = None, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, tools: ToolTypes | Callable[..., Any] | Sequence[ToolTypes | Callable[..., Any]] | None = None, options: ChatOptions[ResponseModelBoundT], compaction_strategy: CompactionStrategy | None = None, @@ -1472,7 +1472,7 @@ def run( *, stream: Literal[False] = ..., session: AgentSession | None = None, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, tools: ToolTypes | Callable[..., Any] | Sequence[ToolTypes | Callable[..., Any]] | None = None, options: ChatOptions[None] | None = None, compaction_strategy: CompactionStrategy | None = None, @@ -1488,7 +1488,7 @@ def run( *, stream: Literal[True], session: AgentSession | None = None, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, tools: ToolTypes | Callable[..., Any] | Sequence[ToolTypes | Callable[..., Any]] | None = None, options: ChatOptions[Any] | None = None, compaction_strategy: CompactionStrategy | None = None, @@ -1503,7 +1503,7 @@ def run( *, stream: bool = False, session: AgentSession | None = None, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, tools: ToolTypes | Callable[..., Any] | Sequence[ToolTypes | Callable[..., Any]] | None = None, options: ChatOptions[Any] | None = None, compaction_strategy: CompactionStrategy | None = None, @@ -1512,14 +1512,11 @@ def run( client_kwargs: Mapping[str, Any] | None = None, ) -> Awaitable[AgentResponse[Any]] | ResponseStream[AgentResponseUpdate, AgentResponse[Any]]: """MiddlewareTypes-enabled unified run method.""" - # Re-categorize self.middleware at runtime to support dynamic changes. The raw - # attribute is passed straight through: categorize_middleware owns the rule - # that a bare single source (one middleware object or a MiddlewareBundle - # assigned directly to the attribute) is one element — never silently dropped. - base_middleware_list = categorize_middleware( - cast("MiddlewareTypes | Sequence[MiddlewareTypes] | None", getattr(self, "middleware", None)) - ) - run_middleware_list = categorize_middleware(middleware) + # Copy the declared sequences so invalid singular values fail instead of becoming + # implicitly supported inputs. + base_middleware = cast("Sequence[MiddlewareTypes] | None", getattr(self, "middleware", None)) + base_middleware_list = categorize_middleware(list(base_middleware) if base_middleware is not None else None) + run_middleware_list = categorize_middleware(list(middleware) if middleware is not None else None) pipeline = self._get_agent_middleware_pipeline([*base_middleware_list["agent"], *run_middleware_list["agent"]]) # Combine base and run-level function/chat middleware for forwarding to chat client diff --git a/python/packages/core/agent_framework/observability.py b/python/packages/core/agent_framework/observability.py index acfe8019c3a..9cecdc57fca 100644 --- a/python/packages/core/agent_framework/observability.py +++ b/python/packages/core/agent_framework/observability.py @@ -2542,7 +2542,7 @@ def run( *, stream: Literal[False] = ..., session: AgentSession | None = None, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, tools: ToolTypes | Callable[..., Any] | Sequence[ToolTypes | Callable[..., Any]] | None = None, options: ChatOptions[ResponseModelBoundT], compaction_strategy: CompactionStrategy | None = None, @@ -2558,7 +2558,7 @@ def run( *, stream: Literal[False] = ..., session: AgentSession | None = None, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, tools: ToolTypes | Callable[..., Any] | Sequence[ToolTypes | Callable[..., Any]] | None = None, options: ChatOptions[None] | None = None, compaction_strategy: CompactionStrategy | None = None, @@ -2574,7 +2574,7 @@ def run( *, stream: Literal[True], session: AgentSession | None = None, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, tools: ToolTypes | Callable[..., Any] | Sequence[ToolTypes | Callable[..., Any]] | None = None, options: ChatOptions[Any] | None = None, compaction_strategy: CompactionStrategy | None = None, @@ -2589,7 +2589,7 @@ def run( *, stream: bool = False, session: AgentSession | None = None, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, tools: ToolTypes | Callable[..., Any] | Sequence[ToolTypes | Callable[..., Any]] | None = None, options: ChatOptions[Any] | None = None, compaction_strategy: CompactionStrategy | None = None, diff --git a/python/packages/core/pyproject.toml b/python/packages/core/pyproject.toml index d3b28c529d6..0b8e00c0602 100644 --- a/python/packages/core/pyproject.toml +++ b/python/packages/core/pyproject.toml @@ -31,11 +31,6 @@ dependencies = [ ] [project.optional-dependencies] -# Deliberately NOT part of `all`: the agent-hooks enforcement middleware is an -# explicitly opt-in experimental feature. -agent-hooks = [ - "agent-hooks-sdk>=0.1.0a4,<0.2", -] all = [ "mcp>=1.24.0,<2", "agent-framework-a2a", diff --git a/python/packages/core/tests/core/test_agent_hooks.py b/python/packages/core/tests/core/test_agent_hooks.py index ef6a7b3ed20..4d5815ac544 100644 --- a/python/packages/core/tests/core/test_agent_hooks.py +++ b/python/packages/core/tests/core/test_agent_hooks.py @@ -191,15 +191,12 @@ async def test_from_emitter_factory_requires_both_arguments() -> None: @requires_sdk -async def test_bare_bundle_at_construction_is_fully_enforced(chat_client_base: MockBaseChatClient) -> None: - # Passing the bundle bare (instead of inside a list) at construction must install - # it exactly like `middleware=[bundle]` — previously it was silently dropped and - # the run executed fully unhooked. +async def test_bundle_at_construction_is_fully_enforced(chat_client_base: MockBaseChatClient) -> None: records: list[InterceptionRecord] = [] guard = PointGuard("output", Verdict.deny(reason="egress_blocked")) agent = Agent( client=chat_client_base, - middleware=create_agent_hooks_middleware([guard], record_sink=records.append), + middleware=[create_agent_hooks_middleware([guard], record_sink=records.append)], ) with pytest.raises(InterceptionBlocked) as exc_info: @@ -2736,16 +2733,16 @@ def test_agent_hooks_middleware_importable_without_sdk(monkeypatch: pytest.Monke is agent_hooks_module.create_agent_hooks_middleware_from_emitter ) - with pytest.raises(ModuleNotFoundError, match=r"agent-framework-core\[agent-hooks\]"): + with pytest.raises(ModuleNotFoundError, match="pip install agent-hooks-sdk"): agent_framework.create_agent_hooks_middleware([cast("Any", object())]) - with pytest.raises(ModuleNotFoundError, match=r"agent-framework-core\[agent-hooks\]"): + with pytest.raises(ModuleNotFoundError, match="pip install agent-hooks-sdk"): agent_framework.create_agent_hooks_middleware_from_emitter(cast("Any", object()), cast("Any", object())) -def test_broken_sdk_installation_is_not_masked_as_missing_extra(monkeypatch: pytest.MonkeyPatch) -> None: +def test_broken_sdk_installation_is_not_masked_as_missing_sdk(monkeypatch: pytest.MonkeyPatch) -> None: # A transitively missing dependency (or any other breakage inside the SDK) must - # propagate unchanged — only a genuinely absent `agent_hooks` package gets the - # install-the-extra hint. + # propagate unchanged; only a genuinely absent `agent_hooks` package gets the + # SDK installation hint. _hide_agent_hooks( monkeypatch, error=ModuleNotFoundError("No module named 'some_native_dep'", name="some_native_dep") ) diff --git a/python/packages/core/tests/core/test_harness_agent.py b/python/packages/core/tests/core/test_harness_agent.py index b74902116ed..af751aa4045 100644 --- a/python/packages/core/tests/core/test_harness_agent.py +++ b/python/packages/core/tests/core/test_harness_agent.py @@ -6,7 +6,7 @@ import warnings from collections.abc import AsyncIterable, Awaitable, Mapping, Sequence from pathlib import Path -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, cast from unittest.mock import patch import pytest @@ -1114,6 +1114,23 @@ async def process(self, context: Any, call_next: Any) -> None: assert [type(mw) for mw in agent.middleware] == [MessageInjectionMiddleware, _CustomMiddleware] +def test_create_harness_agent_rejects_bare_user_middleware() -> None: + """Harness middleware must be supplied as a sequence.""" + from agent_framework import AgentMiddleware + + class _CustomMiddleware(AgentMiddleware): + async def process(self, context: Any, call_next: Any) -> None: + await call_next() + + with pytest.raises(TypeError): + create_harness_agent( + client=_FakeChatClient(), # type: ignore[arg-type] + max_context_window_tokens=128_000, + max_output_tokens=16_384, + middleware=cast("Any", _CustomMiddleware()), + ) + + def test_create_harness_agent_no_middleware_when_tool_approval_disabled_and_none() -> None: """Only the always-on message injection middleware remains when tool approval is disabled.""" from agent_framework import MessageInjectionMiddleware diff --git a/python/packages/core/tests/core/test_middleware_with_agent.py b/python/packages/core/tests/core/test_middleware_with_agent.py index ee2ba7b204d..4bb107d5495 100644 --- a/python/packages/core/tests/core/test_middleware_with_agent.py +++ b/python/packages/core/tests/core/test_middleware_with_agent.py @@ -72,50 +72,43 @@ async def process(self, context: AgentContext, call_next: Callable[[], Awaitable # Verify middleware execution order assert execution_order == ["agent_middleware_before", "agent_middleware_after"] - async def test_bare_middleware_at_construction_is_installed(self, client: SupportsChatGetResponse) -> None: - """A single middleware object passed bare (not in a list) at construction is installed. - - Construction-time middleware mirrors categorize_middleware's single-source - handling, matching the run-level ``middleware=`` behavior instead of silently - dropping the middleware. - """ - execution_order: list[str] = [] + def test_bare_middleware_at_construction_is_rejected(self, client: SupportsChatGetResponse) -> None: + """A single middleware object is not a valid construction-time middleware sequence.""" class TrackingAgentMiddleware(AgentMiddleware): async def process(self, context: AgentContext, call_next: Callable[[], Awaitable[None]]) -> None: - execution_order.append("before") await call_next() - execution_order.append("after") - agent = Agent(client=client, middleware=TrackingAgentMiddleware()) + with pytest.raises(TypeError): + Agent(client=client, middleware=cast("Any", TrackingAgentMiddleware())) - response = await agent.run([Message(role="user", contents=["test message"])]) + async def test_bare_middleware_assigned_to_attribute_is_rejected(self, client: SupportsChatGetResponse) -> None: + """A singular value assigned to ``agent.middleware`` is rejected when the agent runs.""" - assert response is not None - assert execution_order == ["before", "after"] + class TrackingAgentMiddleware(AgentMiddleware): + async def process(self, context: AgentContext, call_next: Callable[[], Awaitable[None]]) -> None: + await call_next() - async def test_bare_middleware_assigned_to_attribute_is_installed(self, client: SupportsChatGetResponse) -> None: - """A single middleware object assigned bare to ``agent.middleware`` executes. + agent = Agent(client=client) + agent.middleware = cast("Any", TrackingAgentMiddleware()) - categorize_middleware owns the bare-source rule (a non-sequence source is a - one-element list) and ``run()`` passes the raw attribute straight to it, so a - bare attribute assignment — which used to be silently ignored — now executes. - """ - execution_order: list[str] = [] + with pytest.raises(TypeError): + await agent.run([Message(role="user", contents=["test message"])]) + + async def test_bare_run_middleware_is_rejected(self, client: SupportsChatGetResponse) -> None: + """Per-run middleware must also be supplied as a sequence.""" class TrackingAgentMiddleware(AgentMiddleware): async def process(self, context: AgentContext, call_next: Callable[[], Awaitable[None]]) -> None: - execution_order.append("before") await call_next() - execution_order.append("after") agent = Agent(client=client) - agent.middleware = cast("Any", TrackingAgentMiddleware()) - - response = await agent.run([Message(role="user", contents=["test message"])]) - assert response is not None - assert execution_order == ["before", "after"] + with pytest.raises(TypeError): + await agent.run( + [Message(role="user", contents=["test message"])], + middleware=cast("Any", TrackingAgentMiddleware()), + ) async def test_class_based_function_middleware_with_chat_agent(self, client: "MockChatClient") -> None: """Test class-based function middleware with Agent.""" diff --git a/python/pyproject.toml b/python/pyproject.toml index 144126b8613..11cb932d5d9 100644 --- a/python/pyproject.toml +++ b/python/pyproject.toml @@ -52,8 +52,7 @@ dev = [ test = [ "azure-monitor-opentelemetry", "mcp[ws]", - # Optional SDK behind core's `agent-hooks` extra; declared here (like mcp[ws]) so - # isolated source checks (dependency-pyright) can resolve its API. + # Optional SDK used by the agent-hooks tests and isolated source checks. "agent-hooks-sdk>=0.1.0a4,<0.2", ] diff --git a/python/uv.lock b/python/uv.lock index 5aa816032c6..c6e5a8ed284 100644 --- a/python/uv.lock +++ b/python/uv.lock @@ -439,9 +439,6 @@ dependencies = [ ] [package.optional-dependencies] -agent-hooks = [ - { name = "agent-hooks-sdk" }, -] all = [ { name = "agent-framework-a2a" }, { name = "agent-framework-ag-ui" }, @@ -513,7 +510,6 @@ requires-dist = [ { name = "agent-framework-purview", marker = "extra == 'all'", editable = "packages/purview" }, { name = "agent-framework-redis", marker = "extra == 'all'", editable = "packages/redis" }, { name = "agent-framework-tools", marker = "extra == 'all'", editable = "packages/tools" }, - { name = "agent-hooks-sdk", marker = "extra == 'agent-hooks'", specifier = ">=0.1.0a4,<0.2" }, { name = "mcp", marker = "extra == 'all'", specifier = ">=1.24.0,<2" }, { name = "msgspec", specifier = ">=0.20.0,<0.22" }, { name = "opentelemetry-api", specifier = ">=1.39.0,<2" }, @@ -521,7 +517,7 @@ requires-dist = [ { name = "python-dotenv", specifier = ">=1,<2" }, { name = "typing-extensions", specifier = ">=4.15.0,<5" }, ] -provides-extras = ["agent-hooks", "all"] +provides-extras = ["all"] [package.metadata.requires-dev] dev = [{ name = "azure-ai-agentserver-core", specifier = ">=2.0.0b7,<3" }] From 28c633be23043fd38033b70f4a57e666d66fd3de Mon Sep 17 00:00:00 2001 From: eavanvalkenburg Date: Thu, 27 Aug 2026 17:04:32 +0200 Subject: [PATCH 2/2] Python: validate middleware sequences consistently Use one strict sequence validator across agent entry points and align client and Foundry factory signatures with the runtime contract. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ac1d6000-fc1a-4891-95f4-8d77d4420e31 --- .../packages/core/agent_framework/_agents.py | 5 +- .../packages/core/agent_framework/_clients.py | 2 +- .../core/agent_framework/_harness/_agent.py | 3 +- .../core/agent_framework/_middleware.py | 26 +++++++--- .../core/tests/core/test_harness_agent.py | 8 ++-- .../tests/core/test_middleware_with_agent.py | 47 +++++++++---------- .../foundry/agent_framework_foundry/_agent.py | 6 +-- 7 files changed, 56 insertions(+), 41 deletions(-) diff --git a/python/packages/core/agent_framework/_agents.py b/python/packages/core/agent_framework/_agents.py index 179127d1781..ae594af4915 100644 --- a/python/packages/core/agent_framework/_agents.py +++ b/python/packages/core/agent_framework/_agents.py @@ -31,6 +31,7 @@ FunctionInvocationContext, MiddlewareTypes, _as_middleware_list, # pyright: ignore[reportPrivateUsage] + _copy_middleware_sequence, # pyright: ignore[reportPrivateUsage] categorize_middleware, ) from ._serialization import SerializationMixin @@ -460,7 +461,9 @@ def __init__( self.name = name self.description = description self.context_providers: list[ContextProvider] = list(context_providers or []) - self.middleware: list[MiddlewareTypes] | None = list(middleware) if middleware is not None else None + self.middleware: list[MiddlewareTypes] | None = ( + _copy_middleware_sequence(middleware) if middleware is not None else None + ) self.additional_properties: dict[str, Any] = cast(dict[str, Any], additional_properties or {}) def create_session(self, *, session_id: str | None = None) -> AgentSession: diff --git a/python/packages/core/agent_framework/_clients.py b/python/packages/core/agent_framework/_clients.py index d4b758af26f..6119d3dcd3e 100644 --- a/python/packages/core/agent_framework/_clients.py +++ b/python/packages/core/agent_framework/_clients.py @@ -578,7 +578,7 @@ def as_agent( tools: ToolTypes | Callable[..., Any] | Sequence[ToolTypes | Callable[..., Any]] | None = None, default_options: OptionsCoT | Mapping[str, Any] | None = None, context_providers: Sequence[Any] | None = None, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, require_per_service_call_history_persistence: bool = False, compaction_strategy: CompactionStrategy | None = None, tokenizer: TokenizerProtocol | None = None, diff --git a/python/packages/core/agent_framework/_harness/_agent.py b/python/packages/core/agent_framework/_harness/_agent.py index fc8c15521e8..36c3fcbc2d8 100644 --- a/python/packages/core/agent_framework/_harness/_agent.py +++ b/python/packages/core/agent_framework/_harness/_agent.py @@ -20,6 +20,7 @@ from .._clients import SupportsShellTool, SupportsWebSearchTool from .._compaction import CompactionProvider, ContextWindowCompactionStrategy from .._feature_stage import ExperimentalFeature, warn_experimental_feature +from .._middleware import _copy_middleware_sequence # pyright: ignore[reportPrivateUsage] from .._sessions import ContextProvider, HistoryProvider, InMemoryHistoryProvider, MessageInjectionMiddleware from .._skills import SkillsProvider from .._telemetry import FeatureIndex, mark_feature_used @@ -670,7 +671,7 @@ def create_harness_agent( # so there is no opt-out. assembled_middleware.append(MessageInjectionMiddleware()) if middleware is not None: - assembled_middleware.extend(middleware) + assembled_middleware.extend(_copy_middleware_sequence(middleware)) agent = Agent( client, diff --git a/python/packages/core/agent_framework/_middleware.py b/python/packages/core/agent_framework/_middleware.py index dd3b67bdfb6..64c82104f5f 100644 --- a/python/packages/core/agent_framework/_middleware.py +++ b/python/packages/core/agent_framework/_middleware.py @@ -828,6 +828,15 @@ def __repr__(self) -> str: ) +def _copy_middleware_sequence(source: object | None) -> list[MiddlewareTypes]: + """Validate and copy a middleware sequence.""" + if source is None: + return [] + if isinstance(source, (str, bytes)) or not isinstance(source, Sequence): + raise TypeError("middleware must be a non-string sequence of middleware.") + return list(cast("Sequence[MiddlewareTypes]", source)) + + def agent_middleware(func: AgentMiddlewareCallable) -> AgentMiddlewareCallable: """Decorator to mark a function as agent middleware. @@ -1428,11 +1437,12 @@ def __init__( middleware: Sequence[MiddlewareTypes] | None = None, **kwargs: Any, ) -> None: - middleware_list = categorize_middleware(middleware) + middleware_sequence = _copy_middleware_sequence(middleware) if middleware is not None else None + middleware_list = categorize_middleware(middleware_sequence) self.agent_middleware = middleware_list["agent"] self._cached_agent_middleware_pipeline: AgentMiddlewarePipeline | None = None # Pass middleware to super so BaseAgent can store it for dynamic rebuild - super().__init__(*args, middleware=middleware, **kwargs) # type: ignore[call-arg] + super().__init__(*args, middleware=middleware_sequence, **kwargs) # type: ignore[call-arg] # Note: We intentionally don't extend client's middleware lists here. # Chat and function middleware is passed to the chat client at runtime via kwargs # in AgentMiddlewareLayer.run(), where it's properly combined with run-level middleware. @@ -1512,11 +1522,13 @@ def run( client_kwargs: Mapping[str, Any] | None = None, ) -> Awaitable[AgentResponse[Any]] | ResponseStream[AgentResponseUpdate, AgentResponse[Any]]: """MiddlewareTypes-enabled unified run method.""" - # Copy the declared sequences so invalid singular values fail instead of becoming - # implicitly supported inputs. - base_middleware = cast("Sequence[MiddlewareTypes] | None", getattr(self, "middleware", None)) - base_middleware_list = categorize_middleware(list(base_middleware) if base_middleware is not None else None) - run_middleware_list = categorize_middleware(list(middleware) if middleware is not None else None) + base_middleware = getattr(self, "middleware", None) + base_middleware_list = categorize_middleware( + _copy_middleware_sequence(base_middleware) if base_middleware is not None else None + ) + run_middleware_list = categorize_middleware( + _copy_middleware_sequence(middleware) if middleware is not None else None + ) pipeline = self._get_agent_middleware_pipeline([*base_middleware_list["agent"], *run_middleware_list["agent"]]) # Combine base and run-level function/chat middleware for forwarding to chat client diff --git a/python/packages/core/tests/core/test_harness_agent.py b/python/packages/core/tests/core/test_harness_agent.py index f6eecf12aa5..6535a6fc68f 100644 --- a/python/packages/core/tests/core/test_harness_agent.py +++ b/python/packages/core/tests/core/test_harness_agent.py @@ -1132,20 +1132,22 @@ async def process(self, context: Any, call_next: Any) -> None: assert [type(mw) for mw in agent.middleware] == [MessageInjectionMiddleware, _CustomMiddleware] -def test_create_harness_agent_rejects_bare_user_middleware() -> None: - """Harness middleware must be supplied as a sequence.""" +def test_create_harness_agent_rejects_generator_middleware() -> None: + """Harness middleware rejects iterables that are not sequences.""" from agent_framework import AgentMiddleware class _CustomMiddleware(AgentMiddleware): async def process(self, context: Any, call_next: Any) -> None: await call_next() + middleware = (item for item in [_CustomMiddleware()]) + with pytest.raises(TypeError): create_harness_agent( client=_FakeChatClient(), # type: ignore[arg-type] max_context_window_tokens=128_000, max_output_tokens=16_384, - middleware=cast("Any", _CustomMiddleware()), + middleware=cast("Any", middleware), ) diff --git a/python/packages/core/tests/core/test_middleware_with_agent.py b/python/packages/core/tests/core/test_middleware_with_agent.py index 4bb107d5495..3fcfe49400a 100644 --- a/python/packages/core/tests/core/test_middleware_with_agent.py +++ b/python/packages/core/tests/core/test_middleware_with_agent.py @@ -2,7 +2,7 @@ import asyncio import threading -from collections.abc import Awaitable, Callable +from collections.abc import Awaitable, Callable, Iterator from typing import Any, cast import pytest @@ -38,6 +38,14 @@ # region Agent Tests +class _IterableAgentMiddleware(AgentMiddleware): + def __iter__(self) -> Iterator[Any]: + return iter(()) + + async def process(self, context: AgentContext, call_next: Callable[[], Awaitable[None]]) -> None: + await call_next() + + class TestChatAgentClassBasedMiddleware: """Test cases for class-based middleware integration with Agent.""" @@ -72,42 +80,31 @@ async def process(self, context: AgentContext, call_next: Callable[[], Awaitable # Verify middleware execution order assert execution_order == ["agent_middleware_before", "agent_middleware_after"] - def test_bare_middleware_at_construction_is_rejected(self, client: SupportsChatGetResponse) -> None: - """A single middleware object is not a valid construction-time middleware sequence.""" - - class TrackingAgentMiddleware(AgentMiddleware): - async def process(self, context: AgentContext, call_next: Callable[[], Awaitable[None]]) -> None: - await call_next() - + @pytest.mark.parametrize("middleware", [_IterableAgentMiddleware(), "middleware", b"middleware"]) + def test_non_sequence_middleware_at_construction_is_rejected( + self, client: SupportsChatGetResponse, middleware: object + ) -> None: + """Construction rejects iterable middleware objects and string sequences.""" with pytest.raises(TypeError): - Agent(client=client, middleware=cast("Any", TrackingAgentMiddleware())) - - async def test_bare_middleware_assigned_to_attribute_is_rejected(self, client: SupportsChatGetResponse) -> None: - """A singular value assigned to ``agent.middleware`` is rejected when the agent runs.""" - - class TrackingAgentMiddleware(AgentMiddleware): - async def process(self, context: AgentContext, call_next: Callable[[], Awaitable[None]]) -> None: - await call_next() + Agent(client=client, middleware=cast("Any", middleware)) + async def test_iterable_middleware_assigned_to_attribute_is_rejected(self, client: SupportsChatGetResponse) -> None: + """An iterable middleware object assigned to ``agent.middleware`` is rejected.""" agent = Agent(client=client) - agent.middleware = cast("Any", TrackingAgentMiddleware()) + agent.middleware = cast("Any", _IterableAgentMiddleware()) with pytest.raises(TypeError): await agent.run([Message(role="user", contents=["test message"])]) - async def test_bare_run_middleware_is_rejected(self, client: SupportsChatGetResponse) -> None: - """Per-run middleware must also be supplied as a sequence.""" - - class TrackingAgentMiddleware(AgentMiddleware): - async def process(self, context: AgentContext, call_next: Callable[[], Awaitable[None]]) -> None: - await call_next() - + async def test_generator_run_middleware_is_rejected(self, client: SupportsChatGetResponse) -> None: + """Per-run middleware rejects iterables that are not sequences.""" agent = Agent(client=client) + middleware = (item for item in [_IterableAgentMiddleware()]) with pytest.raises(TypeError): await agent.run( [Message(role="user", contents=["test message"])], - middleware=cast("Any", TrackingAgentMiddleware()), + middleware=cast("Any", middleware), ) async def test_class_based_function_middleware_with_chat_agent(self, client: "MockChatClient") -> None: diff --git a/python/packages/foundry/agent_framework_foundry/_agent.py b/python/packages/foundry/agent_framework_foundry/_agent.py index 862c54a2a99..0407f9c48fa 100644 --- a/python/packages/foundry/agent_framework_foundry/_agent.py +++ b/python/packages/foundry/agent_framework_foundry/_agent.py @@ -299,7 +299,7 @@ def as_agent( tools: ToolTypes | Callable[..., Any] | Sequence[ToolTypes | Callable[..., Any]] | None = None, default_options: FoundryAgentOptionsT | Mapping[str, Any] | None = None, context_providers: Sequence[ContextProvider] | None = None, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, require_per_service_call_history_persistence: bool = False, function_invocation_configuration: FunctionInvocationConfiguration | None = None, compaction_strategy: CompactionStrategy | None = None, @@ -671,7 +671,7 @@ def __init__( default_headers: Mapping[str, str] | None = None, tools: FunctionTool | Callable[..., Any] | Sequence[FunctionTool | Callable[..., Any]] | None = None, context_providers: Sequence[ContextProvider] | None = None, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, client_type: type[RawFoundryAgentChatClient] | None = None, env_file_path: str | None = None, env_file_encoding: str | None = None, @@ -961,7 +961,7 @@ def __init__( default_headers: Mapping[str, str] | None = None, tools: FunctionTool | Callable[..., Any] | Sequence[FunctionTool | Callable[..., Any]] | None = None, context_providers: Sequence[ContextProvider] | None = None, - middleware: MiddlewareTypes | Sequence[MiddlewareTypes] | None = None, + middleware: Sequence[MiddlewareTypes] | None = None, client_type: type[RawFoundryAgentChatClient] | None = None, env_file_path: str | None = None, env_file_encoding: str | None = None,