From d8a9298b98d634cdf0eacab2ba695c3acb59ea6a Mon Sep 17 00:00:00 2001 From: Sivakumar Mahalingam Date: Tue, 29 Sep 2026 19:27:38 +0400 Subject: [PATCH 1/5] Python: refresh agent-level run tools before each function-calling turn (#4024) --- .../specs/004-python-function-calling-loop.md | 2 +- .../packages/core/agent_framework/_agents.py | 91 ++-- .../packages/core/agent_framework/_tools.py | 41 ++ .../packages/core/tests/core/test_agents.py | 408 ++++++++++++++++++ 4 files changed, 511 insertions(+), 31 deletions(-) diff --git a/docs/specs/004-python-function-calling-loop.md b/docs/specs/004-python-function-calling-loop.md index bdf2fc8ac23..aeec8a07330 100644 --- a/docs/specs/004-python-function-calling-loop.md +++ b/docs/specs/004-python-function-calling-loop.md @@ -591,7 +591,7 @@ that manually replay messages own the equivalent rule: do not resend an approval | OpenAI hosted/local shell boundary | Hosted shell calls remain informational in streaming and non-streaming responses even when a local executor is configured; only valid explicit local-shell items or shell calls marked with a local environment can execute, local execution preserves its configured approval mode, stateless loops preserve the complete provider shell transcript or fail explicitly, and locally generated shell outputs are sent in every continuation mode. | `packages/openai/tests/openai/test_openai_chat_client.py::test_response_content_creation_with_shell_call_remains_hosted_with_local_tool`, `test_parse_chunk_from_openai_shell_call_done_remains_hosted`, `test_parse_chunk_from_openai_local_environment_shell_call_done_emits_command`, `test_mixed_shell_calls_only_invoke_explicit_local_shell_call`, `test_stateless_shell_transcript_without_provider_item_fails`, `test_prepare_messages_keeps_local_shell_output_under_storage`, `test_malformed_local_environment_shell_call_is_not_executable`, `test_response_content_creation_with_local_shell_call_maps_to_function_call`, `test_malformed_local_shell_call_is_not_executable`, `test_response_function_call_named_local_shell_is_informational`, `test_parse_chunk_function_call_named_local_shell_is_informational`, `test_local_shell_tool_requires_approval_before_function_loop_execution` | | Declaration-only call | The call is surfaced as user input and is not executed; streaming arguments appear once while finalized request metadata remains available. | `test_declaration_only_tool`, `test_streaming_declaration_only_tool_preserves_metadata_without_duplicate_arguments` | | Function invocation disabled | The client bypasses the invocation loop without losing invocation kwargs. | `test_function_invocation_config_enabled_false`, `test_function_invocation_config_enabled_false_preserves_invocation_kwargs`, `test_streaming_function_invocation_config_enabled_false` | -| Runtime tool changes | Added tools become available on the next iteration and retain approval behavior. | `test_add_tools_available_next_iteration`, `test_add_tools_with_approval_required_tool` | +| Runtime tool changes | Added tools become available on the next iteration and retain approval behavior. In an agent run, tools appended to the agent's tools, its MCP servers, or a run-level tools list are sent on the next model call in streaming and non-streaming execution; tools removed through the invocation context stay removed, and a duplicate name is skipped with a warning without aborting the run. | `test_add_tools_available_next_iteration`, `test_add_tools_with_approval_required_tool`, `packages/core/tests/core/test_agents.py::test_tool_appended_to_agent_tools_during_run_is_sent_on_next_request`, `test_tool_appended_to_run_tools_during_run_is_sent_on_next_request`, `test_mcp_server_added_to_agent_during_run_is_sent_on_next_request`, `test_tool_removed_with_context_during_run_is_not_restored_from_agent_tools`, `test_tool_with_duplicate_name_added_during_run_is_skipped`, `test_tools_list_identity_is_unchanged_across_iterations_when_nothing_is_added`, `test_same_tool_object_appended_twice_during_run_is_idempotent` | | In-run compaction summaries | Summaries inserted for tool groups excluded by in-run compaction are returned in the final non-streaming response transcript before the group they replace, so history loaded with `skip_excluded` keeps the summarized content; summaries of caller-owned input messages are not added. | `packages/core/tests/core/test_clients.py::test_function_loop_returns_compaction_summaries_in_final_response`, `test_function_loop_returns_compaction_summaries_when_iteration_budget_exhausted`, `test_function_loop_reconciles_nested_compaction_summaries`, `test_function_loop_returns_compacted_transcript_on_early_terminal_exit`, `packages/core/tests/core/test_agents.py::test_agent_run_returns_and_persists_compaction_summaries` | | Per-service-call provider context | When framework-managed history is loaded around each model call, tools and instructions contributed by `HistoryProvider.before_run` remain available to the model and function loop without duplication in streaming or non-streaming execution. | `packages/core/tests/core/test_agents.py::test_vector_history_search_tool_is_available_with_per_service_call_persistence` | diff --git a/python/packages/core/agent_framework/_agents.py b/python/packages/core/agent_framework/_agents.py index db264535aa1..7b476205d7b 100644 --- a/python/packages/core/agent_framework/_agents.py +++ b/python/packages/core/agent_framework/_agents.py @@ -1481,6 +1481,45 @@ def _extract_conversation_id_from_streaming_response( return None + def _run_tool_source_items(self, run_tools: Any) -> list[Any]: + """Return the items of the tool lists a run re-reads before each model call. + + These are the agent's tools, its MCP servers, and the run-level tools when they + were passed as a list. + """ + items: list[Any] = [*self.mcp_tools] + for source in (self.default_options.get("tools"), run_tools): + if isinstance(source, list): + items.extend(cast("list[Any]", source)) + return items + + async def _append_run_tools( + self, + final_tools: list[ToolTypes], + tools: Sequence[ToolTypes], + function_invocation_kwargs: Mapping[str, Any], + ) -> None: + """Append tools to a run's tool list, connecting MCP tools and adding their functions.""" + from ._mcp import MCPTool + + for tool in tools: + if not isinstance(tool, MCPTool): + _append_unique_tools(final_tools, [tool]) + continue + await tool._prepare_for_run(function_invocation_kwargs) # pyright: ignore[reportPrivateUsage] + if not tool.is_connected: + # The handshake and discovery requests are issued before any tool call, so the run's + # kwargs must reach header_provider here or those requests go out unauthenticated. + await self._async_exit_stack.enter_async_context(tool) + await tool._prepare_for_run(function_invocation_kwargs) # pyright: ignore[reportPrivateUsage] + _append_unique_tools( + final_tools, + tool.functions, + duplicate_error_message=( + "Tool names must be unique. Consider setting `tool_name_prefix` on the MCPTool." + ), + ) + async def _prepare_run_context( self, *, @@ -1503,6 +1542,9 @@ async def _prepare_run_context( # remaining options into the request and silently override the resolved list. tools_ = _select_run_level_tools(tools, opts) opts.pop("tools", None) + # Items already in the agent's and the run's tool lists when the run starts; anything + # appended to those lists later is added to the run before its next model call. + seen_tool_items = {id(item): item for item in self._run_tool_source_items(tools_)} input_messages = normalize_messages(messages) @@ -1585,11 +1627,13 @@ async def _prepare_run_context( } agent_name = self._get_agent_name() - from ._mcp import MCPTool - from ._tools import _PARENT_TOOL_APPROVAL_SOURCE_IDS_CONTEXT_KEY # pyright: ignore[reportPrivateUsage] + from ._tools import ( + _PARENT_TOOL_APPROVAL_SOURCE_IDS_CONTEXT_KEY, # pyright: ignore[reportPrivateUsage] + _REFRESH_RUN_TOOLS_KEY, # pyright: ignore[reportPrivateUsage] + FunctionInvocationLayer, + ) base_tools = _normalize_tools(chat_options.pop("tools", None)) - mcp_duplicate_message = "Tool names must be unique. Consider setting `tool_name_prefix` on the MCPTool." # Normalize tools normalized_tools = _normalize_tools(tools_) @@ -1602,34 +1646,18 @@ async def _prepare_run_context( # Resolve final tool list (configured tools + runtime provided tools + local MCP server tools) final_tools = list(base_tools) - for tool in normalized_tools: - if isinstance(tool, MCPTool): - await tool._prepare_for_run(additional_function_arguments) # pyright: ignore[reportPrivateUsage] - if not tool.is_connected: - # The handshake and discovery requests are issued before any tool call, so the run's - # kwargs must reach header_provider here or those requests go out unauthenticated. - await self._async_exit_stack.enter_async_context(tool) - await tool._prepare_for_run(additional_function_arguments) # pyright: ignore[reportPrivateUsage] - _append_unique_tools( - final_tools, - tool.functions, - duplicate_error_message=mcp_duplicate_message, - ) - else: - _append_unique_tools(final_tools, [tool]) + await self._append_run_tools(final_tools, [*normalized_tools, *self.mcp_tools], additional_function_arguments) + # MCP tools added later in the run are prepared with the same kwargs as those added above. + mcp_run_kwargs = dict(additional_function_arguments) - for mcp_server in self.mcp_tools: - await mcp_server._prepare_for_run(additional_function_arguments) # pyright: ignore[reportPrivateUsage] - if not mcp_server.is_connected: - await self._async_exit_stack.enter_async_context(mcp_server) - await mcp_server._prepare_for_run( # pyright: ignore[reportPrivateUsage] - additional_function_arguments - ) - _append_unique_tools( - final_tools, - mcp_server.functions, - duplicate_error_message=mcp_duplicate_message, - ) + async def refresh_run_tools() -> list[ToolTypes]: + new_items = { + id(item): item for item in self._run_tool_source_items(tools_) if id(item) not in seen_tool_items + } + seen_tool_items.update(new_items) + new_tools: list[ToolTypes] = [] + await self._append_run_tools(new_tools, _normalize_tools(list(new_items.values())), mcp_run_kwargs) + return new_tools additional_function_arguments[_PARENT_TOOL_APPROVAL_SOURCE_IDS_CONTEXT_KEY] = _tool_approval_source_ids( self.middleware @@ -1678,6 +1706,9 @@ async def _prepare_run_context( if active_session is not None: effective_client_kwargs["session"] = active_session effective_client_kwargs[_APPROVAL_SESSION_IS_AUTHORITATIVE_KEY] = not framework_created_session + effective_client_kwargs.pop(_REFRESH_RUN_TOOLS_KEY, None) + if isinstance(self.client, FunctionInvocationLayer): + effective_client_kwargs[_REFRESH_RUN_TOOLS_KEY] = refresh_run_tools per_service_call_history_middleware: PerServiceCallHistoryPersistingMiddleware | None = None if per_service_call_history_providers and active_session is not None: per_service_call_history_middleware = PerServiceCallHistoryPersistingMiddleware( diff --git a/python/packages/core/agent_framework/_tools.py b/python/packages/core/agent_framework/_tools.py index 4ab15a52f79..273a29e8534 100644 --- a/python/packages/core/agent_framework/_tools.py +++ b/python/packages/core/agent_framework/_tools.py @@ -106,6 +106,7 @@ def _generate_function_call_occurrence_id() -> str: SHELL_TOOL_KIND_VALUE: Final[str] = "shell" _TOOL_APPROVAL_STATE_KEY: Final[str] = "tool_approval" _APPROVAL_SESSION_IS_AUTHORITATIVE_KEY: Final[str] = "_approval_session_is_authoritative" +_REFRESH_RUN_TOOLS_KEY: Final[str] = "_refresh_run_tools" _PARENT_TOOL_APPROVAL_SOURCE_IDS_CONTEXT_KEY: Final[str] = "_parent_tool_approval_source_ids" @@ -4381,6 +4382,39 @@ def _prepare_messages_for_next_iteration(prepared_messages: list[Message], respo prepared_messages[:] = response.messages[-1:] +_RunToolsRefresher: TypeAlias = Callable[[], Awaitable[list[ToolTypes]]] + + +async def _refresh_run_tools(options: dict[str, Any], refresh_run_tools: _RunToolsRefresher | None) -> None: + """Add tools that joined the run's tool sources since the previous model call. + + Agent runs supply ``refresh_run_tools`` so a tool appended to the agent's or the run's + tool list while the run executes is sent on the next model call. The new tools are + merged into the run-local tools list in place, so the tool map and + ``FunctionInvocationContext.tools`` see them too. A resolution failure (for example a + duplicate tool name) is logged and the run continues with its current tools rather + than aborting mid-loop. + """ + options_tools = options.get("tools") + # Without a run-local tools list the run started with no tools, so no tool can have run yet. + if refresh_run_tools is None or not isinstance(options_tools, list): + return + live_tools = cast("list[ToolTypes]", options_tools) + try: + new_tools = await refresh_run_tools() + if not new_tools: + return + # Validate against a copy first so a duplicate name leaves the live list unchanged. + merged = _append_unique_tools(list(live_tools), new_tools) + except Exception: + logger.warning( + "Tools added during the run could not be made available to the model; they take effect from the next run.", + exc_info=True, + ) + return + live_tools[:] = merged + + @dataclass class _FunctionProcessingResult: """Control data produced while resolving or executing function calls.""" @@ -5028,6 +5062,7 @@ async def _get_response_with_function_invocation( budget_state: dict[str, Any], max_errors: int, middleware_pipeline: FunctionMiddlewarePipeline | None = None, + refresh_run_tools: _RunToolsRefresher | None = None, ) -> ChatResponse[Any]: """Run the non-streaming function invocation loop.""" from ._compaction import _reconcile_compaction_summaries # pyright: ignore[reportPrivateUsage] @@ -5107,6 +5142,7 @@ async def settle_approval_replay_calls(function_calls: Sequence[Content]) -> Non # Phase 2: alternate model turns and local execution until a terminal response or safety limit is reached. for attempt_idx in range(attempt_start, max_iterations): budget_state["attempt_count"] = attempt_idx + 1 + await _refresh_run_tools(options, refresh_run_tools) on_invalidated = _response_invalidation_cleanup(invocation_session, budget_state) response = cast( ChatResponse[Any], @@ -5253,6 +5289,7 @@ async def _stream_response_with_function_invocation( max_errors: int, middleware_pipeline: FunctionMiddlewarePipeline | None = None, invalidation_error: list[ResponseInvalidatedException], + refresh_run_tools: _RunToolsRefresher | None = None, ) -> AsyncIterable[ChatResponseUpdate]: """Run the streaming function invocation loop.""" from ._middleware import MiddlewareFailure @@ -5326,6 +5363,7 @@ async def settle_approval_replay_calls(function_calls: Sequence[Content]) -> Non # Phase 2: stream each model turn, finalize it, execute its calls, then advance the transcript. for attempt_idx in range(attempt_start, max_iterations): budget_state["attempt_count"] = attempt_idx + 1 + await _refresh_run_tools(options, refresh_run_tools) on_invalidated = _response_invalidation_cleanup(invocation_session, budget_state, invalidation_error) inner_stream = cast( "ResponseStream[ChatResponseUpdate, ChatResponse[Any]]", @@ -5633,6 +5671,7 @@ def get_response( approval_session_is_authoritative = ( request_kwargs.pop(_APPROVAL_SESSION_IS_AUTHORITATIVE_KEY, True) is not False ) + refresh_run_tools = cast("_RunToolsRefresher | None", request_kwargs.pop(_REFRESH_RUN_TOOLS_KEY, None)) if invocation_session is None and requires_session_state: invocation_session = _AgentSession() approval_session_is_authoritative = False @@ -5687,6 +5726,7 @@ def get_response( budget_state=budget_state, max_errors=max_errors, middleware_pipeline=function_middleware_pipeline, + refresh_run_tools=refresh_run_tools, ) response_format = mutable_options.get("response_format") @@ -5712,6 +5752,7 @@ def finalize_stream(updates: Sequence[ChatResponseUpdate]) -> ChatResponse[Any]: max_errors=max_errors, middleware_pipeline=function_middleware_pipeline, invalidation_error=invalidation_error, + refresh_run_tools=refresh_run_tools, ), finalizer=finalize_stream, ) diff --git a/python/packages/core/tests/core/test_agents.py b/python/packages/core/tests/core/test_agents.py index e0e79153288..04b09336471 100644 --- a/python/packages/core/tests/core/test_agents.py +++ b/python/packages/core/tests/core/test_agents.py @@ -4574,3 +4574,411 @@ async def test_psc_flag_on_storing_without_conversation_id_warns_every_call( # Two service calls -> the warning is emitted twice (one per call, not deduped). missing_id_warnings = [r for r in caplog.records if "returned no conversation_id" in r.message] assert len(missing_id_warnings) == 2 + + +# region Tools added to the agent's or run's tool list during a run + + +def _math_tool_script(chat_client_base: MockBaseChatClient, *, stream: bool) -> None: + """Script three model turns: call load_math_tools, then multiply, then answer.""" + turns = [ + [Content.from_function_call(call_id="call_1", name="load_math_tools", arguments="{}")], + [Content.from_function_call(call_id="call_2", name="multiply", arguments='{"a": 6, "b": 7}')], + [Content.from_text("42")], + ] + if stream: + chat_client_base.streaming_responses = [ + [ChatResponseUpdate(contents=contents, role="assistant", finish_reason="stop" if index == 2 else None)] + for index, contents in enumerate(turns) + ] + else: + chat_client_base.run_responses = [ + ChatResponse(messages=Message(role="assistant", contents=contents)) for contents in turns + ] + chat_client_base.function_invocation_configuration["max_iterations"] = 3 + + +@pytest.mark.parametrize("stream", [False, True]) +async def test_tool_appended_to_agent_tools_during_run_is_sent_on_next_request( + chat_client_base: MockBaseChatClient, + stream: bool, +) -> None: + executed: list[str] = [] + + @tool(name="multiply", approval_mode="never_require") + def multiply(a: int, b: int) -> int: + """Multiply two numbers.""" + executed.append("multiply") + return a * b + + @tool(name="load_math_tools", approval_mode="never_require") + def load_math_tools() -> str: + """Load the math tools.""" + agent.default_options["tools"].append(multiply) + return "Math tools loaded." + + agent = Agent(client=chat_client_base, tools=[load_math_tools]) + _math_tool_script(chat_client_base, stream=stream) + + requested_tool_names: list[list[str]] = [] + original_inner = chat_client_base._inner_get_response + + def capture_inner( + *, messages: MutableSequence[Message], stream: bool, options: dict[str, Any], **kwargs: Any + ) -> Any: + requested_tool_names.append([tool.name for tool in options.get("tools", [])]) + return original_inner(messages=messages, stream=stream, options=options, **kwargs) + + with patch.object(chat_client_base, "_inner_get_response", side_effect=capture_inner): + if stream: + text = "".join([update.text or "" async for update in agent.run("What is 6 times 7?", stream=True)]) + else: + text = (await agent.run("What is 6 times 7?")).text + + assert requested_tool_names == [ + ["load_math_tools"], + ["load_math_tools", "multiply"], + ["load_math_tools", "multiply"], + ] + assert executed == ["multiply"] + assert text == "42" + + +@pytest.mark.parametrize("stream", [False, True]) +async def test_tool_appended_to_run_tools_during_run_is_sent_on_next_request( + chat_client_base: MockBaseChatClient, + stream: bool, +) -> None: + executed: list[str] = [] + + @tool(name="multiply", approval_mode="never_require") + def multiply(a: int, b: int) -> int: + """Multiply two numbers.""" + executed.append("multiply") + return a * b + + @tool(name="load_math_tools", approval_mode="never_require") + def load_math_tools() -> str: + """Load the math tools.""" + run_tools.append(multiply) + return "Math tools loaded." + + run_tools: list[Any] = [load_math_tools] + agent = Agent(client=chat_client_base) + _math_tool_script(chat_client_base, stream=stream) + + requested_tool_names: list[list[str]] = [] + original_inner = chat_client_base._inner_get_response + + def capture_inner( + *, messages: MutableSequence[Message], stream: bool, options: dict[str, Any], **kwargs: Any + ) -> Any: + requested_tool_names.append([tool.name for tool in options.get("tools", [])]) + return original_inner(messages=messages, stream=stream, options=options, **kwargs) + + with patch.object(chat_client_base, "_inner_get_response", side_effect=capture_inner): + if stream: + text = "".join([ + update.text or "" async for update in agent.run("What is 6 times 7?", tools=run_tools, stream=True) + ]) + else: + text = (await agent.run("What is 6 times 7?", tools=run_tools)).text + + assert requested_tool_names == [ + ["load_math_tools"], + ["load_math_tools", "multiply"], + ["load_math_tools", "multiply"], + ] + assert executed == ["multiply"] + assert text == "42" + + +def _capture_requested_tool_names( + chat_client_base: MockBaseChatClient, requested_tool_names: list[list[str]], *, kwarg_names: list[str] | None = None +) -> Any: + original_inner = chat_client_base._inner_get_response + + def capture_inner( + *, messages: MutableSequence[Message], stream: bool, options: dict[str, Any], **kwargs: Any + ) -> Any: + requested_tool_names.append([tool.name for tool in options.get("tools", [])]) + if kwarg_names is not None: + kwarg_names.extend(kwargs) + return original_inner(messages=messages, stream=stream, options=options, **kwargs) + + return patch.object(chat_client_base, "_inner_get_response", side_effect=capture_inner) + + +async def test_tool_removed_with_context_during_run_is_not_restored_from_agent_tools( + chat_client_base: MockBaseChatClient, +) -> None: + @tool(name="get_weather", approval_mode="never_require") + def get_weather(location: str) -> str: + """Get the weather.""" + return "sunny" + + @tool(name="drop_weather", approval_mode="never_require") + def drop_weather(ctx: FunctionInvocationContext) -> str: + """Remove the weather tool.""" + ctx.remove_tools("get_weather") + return "Weather tool removed." + + agent = Agent(client=chat_client_base, tools=[drop_weather, get_weather]) + chat_client_base.run_responses = [ + ChatResponse( + messages=Message( + role="assistant", + contents=[Content.from_function_call(call_id="call_1", name="drop_weather", arguments="{}")], + ) + ), + ChatResponse(messages=Message(role="assistant", contents=["done"])), + ] + requested_tool_names: list[list[str]] = [] + + with _capture_requested_tool_names(chat_client_base, requested_tool_names): + await agent.run("Drop the weather tool.") + + assert requested_tool_names == [["drop_weather", "get_weather"], ["drop_weather"]] + assert [tool.name for tool in agent.default_options["tools"]] == ["drop_weather", "get_weather"] + + +@pytest.mark.parametrize("function_invocation_enabled", [True, False]) +async def test_run_tools_refresh_is_not_forwarded_to_chat_client( + chat_client_base: MockBaseChatClient, + function_invocation_enabled: bool, +) -> None: + from agent_framework._tools import _REFRESH_RUN_TOOLS_KEY + + @tool(name="lookup_weather", approval_mode="never_require") + def lookup_weather(location: str) -> str: + """Get the weather.""" + return "sunny" + + chat_client_base.function_invocation_configuration["enabled"] = function_invocation_enabled + agent = Agent(client=chat_client_base, tools=[lookup_weather]) + chat_client_base.run_responses = [ + ChatResponse( + messages=Message( + role="assistant", + contents=[ + Content.from_function_call( + call_id="call_1", name="lookup_weather", arguments='{"location": "Seattle"}' + ) + ], + ) + ), + ChatResponse(messages=Message(role="assistant", contents=["done"])), + ] + requested_tool_names: list[list[str]] = [] + kwarg_names: list[str] = [] + + with _capture_requested_tool_names(chat_client_base, requested_tool_names, kwarg_names=kwarg_names): + await agent.run("What's the weather in Seattle?") + + assert _REFRESH_RUN_TOOLS_KEY not in kwarg_names + expected_calls = 2 if function_invocation_enabled else 1 + assert requested_tool_names == [["lookup_weather"]] * expected_calls + + +async def test_mcp_server_added_to_agent_during_run_is_sent_on_next_request( + chat_client_base: MockBaseChatClient, +) -> None: + math_server = _ConnectedMCPTool("math", ["multiply"]) + + @tool(name="connect_math_server", approval_mode="never_require") + def connect_math_server() -> str: + """Connect the math MCP server.""" + agent.mcp_tools.append(math_server) + return "Math server connected." + + agent = Agent(client=chat_client_base, tools=[connect_math_server]) + chat_client_base.run_responses = [ + ChatResponse( + messages=Message( + role="assistant", + contents=[Content.from_function_call(call_id="call_1", name="connect_math_server", arguments="{}")], + ) + ), + ChatResponse(messages=Message(role="assistant", contents=["done"])), + ] + requested_tool_names: list[list[str]] = [] + + with _capture_requested_tool_names(chat_client_base, requested_tool_names): + await agent.run("Connect the math server.") + + assert requested_tool_names == [["connect_math_server"], ["connect_math_server", "multiply"]] + + +async def test_tool_with_duplicate_name_added_during_run_is_skipped( + chat_client_base: MockBaseChatClient, + caplog: pytest.LogCaptureFixture, +) -> None: + executed: list[str] = [] + + @tool(name="multiply", approval_mode="never_require") + def multiply(a: int, b: int) -> int: + """Multiply two numbers.""" + executed.append("original") + return a * b + + @tool(name="multiply", approval_mode="never_require") + def other_multiply(a: int, b: int) -> int: + """Multiply two numbers differently.""" + executed.append("other") + return a * b + + @tool(name="load_math_tools", approval_mode="never_require") + def load_math_tools() -> str: + """Load the math tools.""" + agent.default_options["tools"].append(other_multiply) + return "Math tools loaded." + + agent = Agent(client=chat_client_base, tools=[load_math_tools, multiply]) + _math_tool_script(chat_client_base, stream=False) + requested_tool_names: list[list[str]] = [] + + with ( + caplog.at_level(logging.WARNING, logger="agent_framework"), + _capture_requested_tool_names(chat_client_base, requested_tool_names), + ): + response = await agent.run("What is 6 times 7?") + + assert requested_tool_names == [["load_math_tools", "multiply"]] * 3 + assert executed == ["original"] + assert response.text == "42" + assert any("could not be made available to the model" in record.message for record in caplog.records) + + +@pytest.mark.parametrize("stream", [False, True]) +async def test_tools_list_identity_is_unchanged_across_iterations_when_nothing_is_added( + chat_client_base: MockBaseChatClient, + stream: bool, +) -> None: + """Regression: when no tool mutates any tool list, the run-local tools list sent to the + model is not replaced or reordered between iterations of the same run. + + This guards the no-op path in ``_refresh_run_tools``: with nothing new to merge, the + run-local list object identity (and its contents) must be exactly what it was before, + matching behavior prior to mid-run tool refresh. + """ + + @tool(name="lookup_weather", approval_mode="never_require") + def lookup_weather(location: str) -> str: + """Get the weather.""" + return "sunny" + + @tool(name="lookup_time", approval_mode="never_require") + def lookup_time(location: str) -> str: + """Get the time.""" + return "noon" + + agent = Agent(client=chat_client_base, tools=[lookup_weather]) + turns = [ + [Content.from_function_call(call_id="call_1", name="lookup_weather", arguments='{"location": "Seattle"}')], + [Content.from_function_call(call_id="call_2", name="lookup_time", arguments='{"location": "Boston"}')], + [Content.from_text("done")], + ] + if stream: + chat_client_base.streaming_responses = [ + [ChatResponseUpdate(contents=contents, role="assistant", finish_reason="stop" if index == 2 else None)] + for index, contents in enumerate(turns) + ] + else: + chat_client_base.run_responses = [ + ChatResponse(messages=Message(role="assistant", contents=contents)) for contents in turns + ] + chat_client_base.function_invocation_configuration["max_iterations"] = 3 + + requested_tool_lists: list[Any] = [] + original_inner = chat_client_base._inner_get_response + + def capture_inner( + *, messages: MutableSequence[Message], stream: bool, options: dict[str, Any], **kwargs: Any + ) -> Any: + requested_tool_lists.append(options.get("tools")) + return original_inner(messages=messages, stream=stream, options=options, **kwargs) + + with patch.object(chat_client_base, "_inner_get_response", side_effect=capture_inner): + if stream: + text = "".join([ + update.text or "" async for update in agent.run("weather?", tools=[lookup_time], stream=True) + ]) + else: + text = (await agent.run("weather?", tools=[lookup_time])).text + + assert text == "done" + assert len(requested_tool_lists) == 3 + # The exact same list object is reused for every model call in the run: nothing was + # added or removed, so the loop never replaces it with a new list. + assert all(tools_list is requested_tool_lists[0] for tools_list in requested_tool_lists) + assert [t.name for t in requested_tool_lists[0]] == ["lookup_weather", "lookup_time"] + + +async def test_same_tool_object_appended_twice_during_run_is_idempotent( + chat_client_base: MockBaseChatClient, + caplog: pytest.LogCaptureFixture, +) -> None: + """Appending the exact same tool object to the agent's tools twice, across two + different iterations, is a no-op the second time: no duplicate entry, no warning, + and no error, since it's the same object under an already-adopted name. + """ + executed: list[str] = [] + + @tool(name="multiply", approval_mode="never_require") + def multiply(a: int, b: int) -> int: + """Multiply two numbers.""" + executed.append("multiply") + return a * b + + @tool(name="add_multiply", approval_mode="never_require") + def add_multiply() -> str: + """Add the multiply tool.""" + agent.default_options["tools"].append(multiply) + return "added" + + @tool(name="add_multiply_again", approval_mode="never_require") + def add_multiply_again() -> str: + """Add the exact same multiply tool object a second time.""" + agent.default_options["tools"].append(multiply) + return "added again" + + agent = Agent(client=chat_client_base, tools=[add_multiply, add_multiply_again]) + chat_client_base.function_invocation_configuration["max_iterations"] = 4 + chat_client_base.run_responses = [ + ChatResponse( + messages=Message( + role="assistant", + contents=[Content.from_function_call(call_id="call_1", name="add_multiply", arguments="{}")], + ) + ), + ChatResponse( + messages=Message( + role="assistant", + contents=[Content.from_function_call(call_id="call_2", name="add_multiply_again", arguments="{}")], + ) + ), + ChatResponse( + messages=Message( + role="assistant", + contents=[Content.from_function_call(call_id="call_3", name="multiply", arguments='{"a": 2, "b": 3}')], + ) + ), + ChatResponse(messages=Message(role="assistant", contents=["done"])), + ] + requested_tool_names: list[list[str]] = [] + + with ( + caplog.at_level(logging.WARNING, logger="agent_framework"), + _capture_requested_tool_names(chat_client_base, requested_tool_names), + ): + response = await agent.run("Multiply 2 and 3.") + + assert requested_tool_names == [ + ["add_multiply", "add_multiply_again"], + ["add_multiply", "add_multiply_again", "multiply"], + ["add_multiply", "add_multiply_again", "multiply"], + ["add_multiply", "add_multiply_again", "multiply"], + ] + assert executed == ["multiply"] + assert response.text == "done" + assert not any("could not be made available to the model" in record.message for record in caplog.records) From ea6efddb529dd09a39dd846ad2a232439b68aa6f Mon Sep 17 00:00:00 2001 From: Sivakumar Mahalingam Date: Tue, 29 Sep 2026 19:52:51 +0400 Subject: [PATCH 2/5] Python: skip only the clashing tool when refreshing run tools mid-run --- .../specs/004-python-function-calling-loop.md | 2 +- .../packages/core/agent_framework/_agents.py | 75 ++++--- .../packages/core/agent_framework/_tools.py | 33 +++- .../packages/core/tests/core/test_agents.py | 184 ++++++++++++++++++ 4 files changed, 255 insertions(+), 39 deletions(-) diff --git a/docs/specs/004-python-function-calling-loop.md b/docs/specs/004-python-function-calling-loop.md index aeec8a07330..368973c06d0 100644 --- a/docs/specs/004-python-function-calling-loop.md +++ b/docs/specs/004-python-function-calling-loop.md @@ -591,7 +591,7 @@ that manually replay messages own the equivalent rule: do not resend an approval | OpenAI hosted/local shell boundary | Hosted shell calls remain informational in streaming and non-streaming responses even when a local executor is configured; only valid explicit local-shell items or shell calls marked with a local environment can execute, local execution preserves its configured approval mode, stateless loops preserve the complete provider shell transcript or fail explicitly, and locally generated shell outputs are sent in every continuation mode. | `packages/openai/tests/openai/test_openai_chat_client.py::test_response_content_creation_with_shell_call_remains_hosted_with_local_tool`, `test_parse_chunk_from_openai_shell_call_done_remains_hosted`, `test_parse_chunk_from_openai_local_environment_shell_call_done_emits_command`, `test_mixed_shell_calls_only_invoke_explicit_local_shell_call`, `test_stateless_shell_transcript_without_provider_item_fails`, `test_prepare_messages_keeps_local_shell_output_under_storage`, `test_malformed_local_environment_shell_call_is_not_executable`, `test_response_content_creation_with_local_shell_call_maps_to_function_call`, `test_malformed_local_shell_call_is_not_executable`, `test_response_function_call_named_local_shell_is_informational`, `test_parse_chunk_function_call_named_local_shell_is_informational`, `test_local_shell_tool_requires_approval_before_function_loop_execution` | | Declaration-only call | The call is surfaced as user input and is not executed; streaming arguments appear once while finalized request metadata remains available. | `test_declaration_only_tool`, `test_streaming_declaration_only_tool_preserves_metadata_without_duplicate_arguments` | | Function invocation disabled | The client bypasses the invocation loop without losing invocation kwargs. | `test_function_invocation_config_enabled_false`, `test_function_invocation_config_enabled_false_preserves_invocation_kwargs`, `test_streaming_function_invocation_config_enabled_false` | -| Runtime tool changes | Added tools become available on the next iteration and retain approval behavior. In an agent run, tools appended to the agent's tools, its MCP servers, or a run-level tools list are sent on the next model call in streaming and non-streaming execution; tools removed through the invocation context stay removed, and a duplicate name is skipped with a warning without aborting the run. | `test_add_tools_available_next_iteration`, `test_add_tools_with_approval_required_tool`, `packages/core/tests/core/test_agents.py::test_tool_appended_to_agent_tools_during_run_is_sent_on_next_request`, `test_tool_appended_to_run_tools_during_run_is_sent_on_next_request`, `test_mcp_server_added_to_agent_during_run_is_sent_on_next_request`, `test_tool_removed_with_context_during_run_is_not_restored_from_agent_tools`, `test_tool_with_duplicate_name_added_during_run_is_skipped`, `test_tools_list_identity_is_unchanged_across_iterations_when_nothing_is_added`, `test_same_tool_object_appended_twice_during_run_is_idempotent` | +| Runtime tool changes | Added tools become available on the next iteration and retain approval behavior. In an agent run, tools appended to the agent's tools, its MCP servers, or a run-level tools list are sent on the next model call in streaming and non-streaming execution; tools removed through the invocation context stay removed, and a tool whose name is already taken is skipped with a warning on its own, without dropping other tools added in the same turn or aborting the run. | `test_add_tools_available_next_iteration`, `test_add_tools_with_approval_required_tool`, `packages/core/tests/core/test_agents.py::test_tool_appended_to_agent_tools_during_run_is_sent_on_next_request`, `test_tool_appended_to_run_tools_during_run_is_sent_on_next_request`, `test_mcp_server_added_to_agent_during_run_is_sent_on_next_request`, `test_tool_removed_with_context_during_run_is_not_restored_from_agent_tools`, `test_tool_with_duplicate_name_added_during_run_is_skipped`, `test_tools_list_identity_is_unchanged_across_iterations_when_nothing_is_added`, `test_same_tool_object_appended_twice_during_run_is_idempotent`, `test_valid_tool_added_with_duplicate_in_same_turn_is_still_exposed`, `test_two_new_tools_with_same_name_in_one_batch_keep_the_first`, `test_mcp_server_added_during_run_with_partially_clashing_functions_exposes_the_rest` | | In-run compaction summaries | Summaries inserted for tool groups excluded by in-run compaction are returned in the final non-streaming response transcript before the group they replace, so history loaded with `skip_excluded` keeps the summarized content; summaries of caller-owned input messages are not added. | `packages/core/tests/core/test_clients.py::test_function_loop_returns_compaction_summaries_in_final_response`, `test_function_loop_returns_compaction_summaries_when_iteration_budget_exhausted`, `test_function_loop_reconciles_nested_compaction_summaries`, `test_function_loop_returns_compacted_transcript_on_early_terminal_exit`, `packages/core/tests/core/test_agents.py::test_agent_run_returns_and_persists_compaction_summaries` | | Per-service-call provider context | When framework-managed history is loaded around each model call, tools and instructions contributed by `HistoryProvider.before_run` remain available to the model and function loop without duplication in streaming or non-streaming execution. | `packages/core/tests/core/test_agents.py::test_vector_history_search_tool_is_available_with_per_service_call_persistence` | diff --git a/python/packages/core/agent_framework/_agents.py b/python/packages/core/agent_framework/_agents.py index 7b476205d7b..d341772ad5f 100644 --- a/python/packages/core/agent_framework/_agents.py +++ b/python/packages/core/agent_framework/_agents.py @@ -1493,32 +1493,23 @@ def _run_tool_source_items(self, run_tools: Any) -> list[Any]: items.extend(cast("list[Any]", source)) return items - async def _append_run_tools( + async def _resolve_run_tool( self, - final_tools: list[ToolTypes], - tools: Sequence[ToolTypes], + tool: ToolTypes, function_invocation_kwargs: Mapping[str, Any], - ) -> None: - """Append tools to a run's tool list, connecting MCP tools and adding their functions.""" + ) -> list[ToolTypes]: + """Resolve one normalized tool into the tools a run exposes, connecting MCP tools.""" from ._mcp import MCPTool - for tool in tools: - if not isinstance(tool, MCPTool): - _append_unique_tools(final_tools, [tool]) - continue + if not isinstance(tool, MCPTool): + return [tool] + await tool._prepare_for_run(function_invocation_kwargs) # pyright: ignore[reportPrivateUsage] + if not tool.is_connected: + # The handshake and discovery requests are issued before any tool call, so the run's + # kwargs must reach header_provider here or those requests go out unauthenticated. + await self._async_exit_stack.enter_async_context(tool) await tool._prepare_for_run(function_invocation_kwargs) # pyright: ignore[reportPrivateUsage] - if not tool.is_connected: - # The handshake and discovery requests are issued before any tool call, so the run's - # kwargs must reach header_provider here or those requests go out unauthenticated. - await self._async_exit_stack.enter_async_context(tool) - await tool._prepare_for_run(function_invocation_kwargs) # pyright: ignore[reportPrivateUsage] - _append_unique_tools( - final_tools, - tool.functions, - duplicate_error_message=( - "Tool names must be unique. Consider setting `tool_name_prefix` on the MCPTool." - ), - ) + return list(tool.functions) async def _prepare_run_context( self, @@ -1627,6 +1618,7 @@ async def _prepare_run_context( } agent_name = self._get_agent_name() + from ._mcp import MCPTool from ._tools import ( _PARENT_TOOL_APPROVAL_SOURCE_IDS_CONTEXT_KEY, # pyright: ignore[reportPrivateUsage] _REFRESH_RUN_TOOLS_KEY, # pyright: ignore[reportPrivateUsage] @@ -1646,18 +1638,43 @@ async def _prepare_run_context( # Resolve final tool list (configured tools + runtime provided tools + local MCP server tools) final_tools = list(base_tools) - await self._append_run_tools(final_tools, [*normalized_tools, *self.mcp_tools], additional_function_arguments) + for tool in [*normalized_tools, *self.mcp_tools]: + _append_unique_tools( + final_tools, + await self._resolve_run_tool(tool, additional_function_arguments), + duplicate_error_message=( + "Tool names must be unique. Consider setting `tool_name_prefix` on the MCPTool." + if isinstance(tool, MCPTool) + else None + ), + ) # MCP tools added later in the run are prepared with the same kwargs as those added above. mcp_run_kwargs = dict(additional_function_arguments) async def refresh_run_tools() -> list[ToolTypes]: - new_items = { - id(item): item for item in self._run_tool_source_items(tools_) if id(item) not in seen_tool_items - } - seen_tool_items.update(new_items) - new_tools: list[ToolTypes] = [] - await self._append_run_tools(new_tools, _normalize_tools(list(new_items.values())), mcp_run_kwargs) - return new_tools + # Candidates are returned without de-duplication; the function-invocation loop merges + # them one at a time and skips only the names that clash. + candidates: list[ToolTypes] = [] + for item in self._run_tool_source_items(tools_): + if id(item) in seen_tool_items: + continue + try: + resolved = [ + resolved_tool + for tool in _normalize_tools(item) + for resolved_tool in await self._resolve_run_tool(tool, mcp_run_kwargs) + ] + except Exception: + # Left unseen, so resolution is retried before the next model call. + logger.warning( + "Tool %r added during the run could not be resolved; retrying before the next model call.", + item, + exc_info=True, + ) + continue + seen_tool_items[id(item)] = item + candidates.extend(resolved) + return candidates additional_function_arguments[_PARENT_TOOL_APPROVAL_SOURCE_IDS_CONTEXT_KEY] = _tool_approval_source_ids( self.middleware diff --git a/python/packages/core/agent_framework/_tools.py b/python/packages/core/agent_framework/_tools.py index 273a29e8534..e5fcfc9514c 100644 --- a/python/packages/core/agent_framework/_tools.py +++ b/python/packages/core/agent_framework/_tools.py @@ -4390,10 +4390,10 @@ async def _refresh_run_tools(options: dict[str, Any], refresh_run_tools: _RunToo Agent runs supply ``refresh_run_tools`` so a tool appended to the agent's or the run's tool list while the run executes is sent on the next model call. The new tools are - merged into the run-local tools list in place, so the tool map and - ``FunctionInvocationContext.tools`` see them too. A resolution failure (for example a - duplicate tool name) is logged and the run continues with its current tools rather - than aborting mid-loop. + merged into the run-local tools list in place, one at a time, so the tool map and + ``FunctionInvocationContext.tools`` see them too. A tool whose name is already taken by + a different tool (in the run or earlier in the same batch) is logged and skipped on its + own; the other new tools are still added and the run never aborts mid-loop. """ options_tools = options.get("tools") # Without a run-local tools list the run started with no tools, so no tool can have run yet. @@ -4402,17 +4402,32 @@ async def _refresh_run_tools(options: dict[str, Any], refresh_run_tools: _RunToo live_tools = cast("list[ToolTypes]", options_tools) try: new_tools = await refresh_run_tools() - if not new_tools: - return - # Validate against a copy first so a duplicate name leaves the live list unchanged. - merged = _append_unique_tools(list(live_tools), new_tools) except Exception: logger.warning( "Tools added during the run could not be made available to the model; they take effect from the next run.", exc_info=True, ) return - live_tools[:] = merged + if not new_tools: + return + tools_by_name = {name: tool_item for tool_item in live_tools if (name := _get_tool_name(tool_item))} + for tool_item in new_tools: + name = _get_tool_name(tool_item) + if name is None: + live_tools.append(tool_item) + continue + existing_tool = tools_by_name.get(name) + if existing_tool is tool_item: + continue + if existing_tool is not None: + logger.warning( + "Tool '%s' added during the run could not be made available to the model: " + "a different tool with that name is already available.", + name, + ) + continue + tools_by_name[name] = tool_item + live_tools.append(tool_item) @dataclass diff --git a/python/packages/core/tests/core/test_agents.py b/python/packages/core/tests/core/test_agents.py index 04b09336471..4847251f161 100644 --- a/python/packages/core/tests/core/test_agents.py +++ b/python/packages/core/tests/core/test_agents.py @@ -4982,3 +4982,187 @@ def add_multiply_again() -> str: assert executed == ["multiply"] assert response.text == "done" assert not any("could not be made available to the model" in record.message for record in caplog.records) + + +def _script_turns(chat_client_base: MockBaseChatClient, turns: list[list[Content]], *, stream: bool) -> None: + """Script one model turn per entry; the last turn finishes the response.""" + if stream: + chat_client_base.streaming_responses = [ + [ + ChatResponseUpdate( + contents=contents, role="assistant", finish_reason="stop" if index == len(turns) - 1 else None + ) + ] + for index, contents in enumerate(turns) + ] + else: + chat_client_base.run_responses = [ + ChatResponse(messages=Message(role="assistant", contents=contents)) for contents in turns + ] + chat_client_base.function_invocation_configuration["max_iterations"] = len(turns) + + +async def _run_agent_text(agent: Agent[Any], prompt: str, *, stream: bool) -> str: + if stream: + return "".join([update.text or "" async for update in agent.run(prompt, stream=True)]) + return (await agent.run(prompt)).text + + +def _duplicate_tool_warnings(caplog: pytest.LogCaptureFixture) -> list[str]: + return [record.message for record in caplog.records if "could not be made available to the model" in record.message] + + +@pytest.mark.parametrize("stream", [False, True]) +async def test_valid_tool_added_with_duplicate_in_same_turn_is_still_exposed( + chat_client_base: MockBaseChatClient, + caplog: pytest.LogCaptureFixture, + stream: bool, +) -> None: + executed: list[str] = [] + + @tool(name="multiply", approval_mode="never_require") + def multiply(a: int, b: int) -> int: + """Multiply two numbers.""" + executed.append("multiply") + return a * b + + @tool(name="multiply", approval_mode="never_require") + def other_multiply(a: int, b: int) -> int: + """Multiply two numbers differently.""" + executed.append("other_multiply") + return a * b + + @tool(name="divide", approval_mode="never_require") + def divide(a: int, b: int) -> float: + """Divide two numbers.""" + executed.append("divide") + return a / b + + @tool(name="load_math_tools", approval_mode="never_require") + def load_math_tools() -> str: + """Load the math tools.""" + agent.default_options["tools"].extend([other_multiply, divide]) + return "Math tools loaded." + + agent = Agent(client=chat_client_base, tools=[load_math_tools, multiply]) + _script_turns( + chat_client_base, + [ + [Content.from_function_call(call_id="call_1", name="load_math_tools", arguments="{}")], + [Content.from_function_call(call_id="call_2", name="divide", arguments='{"a": 6, "b": 3}')], + [Content.from_text("2")], + ], + stream=stream, + ) + requested_tool_names: list[list[str]] = [] + + with ( + caplog.at_level(logging.WARNING, logger="agent_framework"), + _capture_requested_tool_names(chat_client_base, requested_tool_names), + ): + text = await _run_agent_text(agent, "What is 6 divided by 3?", stream=stream) + + assert requested_tool_names == [ + ["load_math_tools", "multiply"], + ["load_math_tools", "multiply", "divide"], + ["load_math_tools", "multiply", "divide"], + ] + assert executed == ["divide"] + assert text == "2" + assert len(_duplicate_tool_warnings(caplog)) == 1 + + +@pytest.mark.parametrize("stream", [False, True]) +async def test_two_new_tools_with_same_name_in_one_batch_keep_the_first( + chat_client_base: MockBaseChatClient, + caplog: pytest.LogCaptureFixture, + stream: bool, +) -> None: + executed: list[str] = [] + + @tool(name="add", approval_mode="never_require") + def first_add(a: int, b: int) -> int: + """Add two numbers.""" + executed.append("first_add") + return a + b + + @tool(name="add", approval_mode="never_require") + def second_add(a: int, b: int) -> int: + """Add two numbers differently.""" + executed.append("second_add") + return a + b + + @tool(name="load_math_tools", approval_mode="never_require") + def load_math_tools() -> str: + """Load the math tools.""" + agent.default_options["tools"].extend([first_add, second_add]) + return "Math tools loaded." + + agent = Agent(client=chat_client_base, tools=[load_math_tools]) + _script_turns( + chat_client_base, + [ + [Content.from_function_call(call_id="call_1", name="load_math_tools", arguments="{}")], + [Content.from_function_call(call_id="call_2", name="add", arguments='{"a": 2, "b": 3}')], + [Content.from_text("5")], + ], + stream=stream, + ) + requested_tool_names: list[list[str]] = [] + + with ( + caplog.at_level(logging.WARNING, logger="agent_framework"), + _capture_requested_tool_names(chat_client_base, requested_tool_names), + ): + text = await _run_agent_text(agent, "What is 2 plus 3?", stream=stream) + + assert requested_tool_names == [["load_math_tools"], ["load_math_tools", "add"], ["load_math_tools", "add"]] + assert executed == ["first_add"] + assert text == "5" + assert len(_duplicate_tool_warnings(caplog)) == 1 + + +@pytest.mark.parametrize("stream", [False, True]) +async def test_mcp_server_added_during_run_with_partially_clashing_functions_exposes_the_rest( + chat_client_base: MockBaseChatClient, + caplog: pytest.LogCaptureFixture, + stream: bool, +) -> None: + math_server = _ConnectedMCPTool("math", ["multiply", "divide", "subtract"]) + + @tool(name="multiply", approval_mode="never_require") + def multiply(a: int, b: int) -> int: + """Multiply two numbers.""" + return a * b + + @tool(name="connect_math_server", approval_mode="never_require") + def connect_math_server() -> str: + """Connect the math MCP server.""" + agent.mcp_tools.append(math_server) + return "Math server connected." + + agent = Agent(client=chat_client_base, tools=[connect_math_server, multiply]) + _script_turns( + chat_client_base, + [ + [Content.from_function_call(call_id="call_1", name="connect_math_server", arguments="{}")], + [Content.from_text("done")], + ], + stream=stream, + ) + requested_tool_names: list[list[str]] = [] + + with ( + caplog.at_level(logging.WARNING, logger="agent_framework"), + _capture_requested_tool_names(chat_client_base, requested_tool_names), + ): + text = await _run_agent_text(agent, "Connect the math server.", stream=stream) + + assert requested_tool_names == [ + ["connect_math_server", "multiply"], + ["connect_math_server", "multiply", "divide", "subtract"], + ] + assert text == "done" + warnings = _duplicate_tool_warnings(caplog) + assert len(warnings) == 1 + assert "'multiply'" in warnings[0] From 5bc6ddb1d380f226953c7c81eadbcab2aca0fb59 Mon Sep 17 00:00:00 2001 From: Sivakumar Mahalingam Date: Tue, 29 Sep 2026 20:15:50 +0400 Subject: [PATCH 3/5] Python: resolve mid-run tool refresh in run-start source order --- .../specs/004-python-function-calling-loop.md | 2 +- .../packages/core/agent_framework/_agents.py | 26 +++++--- .../packages/core/tests/core/test_agents.py | 62 +++++++++++++++++++ 3 files changed, 81 insertions(+), 9 deletions(-) diff --git a/docs/specs/004-python-function-calling-loop.md b/docs/specs/004-python-function-calling-loop.md index 368973c06d0..5428caedf55 100644 --- a/docs/specs/004-python-function-calling-loop.md +++ b/docs/specs/004-python-function-calling-loop.md @@ -591,7 +591,7 @@ that manually replay messages own the equivalent rule: do not resend an approval | OpenAI hosted/local shell boundary | Hosted shell calls remain informational in streaming and non-streaming responses even when a local executor is configured; only valid explicit local-shell items or shell calls marked with a local environment can execute, local execution preserves its configured approval mode, stateless loops preserve the complete provider shell transcript or fail explicitly, and locally generated shell outputs are sent in every continuation mode. | `packages/openai/tests/openai/test_openai_chat_client.py::test_response_content_creation_with_shell_call_remains_hosted_with_local_tool`, `test_parse_chunk_from_openai_shell_call_done_remains_hosted`, `test_parse_chunk_from_openai_local_environment_shell_call_done_emits_command`, `test_mixed_shell_calls_only_invoke_explicit_local_shell_call`, `test_stateless_shell_transcript_without_provider_item_fails`, `test_prepare_messages_keeps_local_shell_output_under_storage`, `test_malformed_local_environment_shell_call_is_not_executable`, `test_response_content_creation_with_local_shell_call_maps_to_function_call`, `test_malformed_local_shell_call_is_not_executable`, `test_response_function_call_named_local_shell_is_informational`, `test_parse_chunk_function_call_named_local_shell_is_informational`, `test_local_shell_tool_requires_approval_before_function_loop_execution` | | Declaration-only call | The call is surfaced as user input and is not executed; streaming arguments appear once while finalized request metadata remains available. | `test_declaration_only_tool`, `test_streaming_declaration_only_tool_preserves_metadata_without_duplicate_arguments` | | Function invocation disabled | The client bypasses the invocation loop without losing invocation kwargs. | `test_function_invocation_config_enabled_false`, `test_function_invocation_config_enabled_false_preserves_invocation_kwargs`, `test_streaming_function_invocation_config_enabled_false` | -| Runtime tool changes | Added tools become available on the next iteration and retain approval behavior. In an agent run, tools appended to the agent's tools, its MCP servers, or a run-level tools list are sent on the next model call in streaming and non-streaming execution; tools removed through the invocation context stay removed, and a tool whose name is already taken is skipped with a warning on its own, without dropping other tools added in the same turn or aborting the run. | `test_add_tools_available_next_iteration`, `test_add_tools_with_approval_required_tool`, `packages/core/tests/core/test_agents.py::test_tool_appended_to_agent_tools_during_run_is_sent_on_next_request`, `test_tool_appended_to_run_tools_during_run_is_sent_on_next_request`, `test_mcp_server_added_to_agent_during_run_is_sent_on_next_request`, `test_tool_removed_with_context_during_run_is_not_restored_from_agent_tools`, `test_tool_with_duplicate_name_added_during_run_is_skipped`, `test_tools_list_identity_is_unchanged_across_iterations_when_nothing_is_added`, `test_same_tool_object_appended_twice_during_run_is_idempotent`, `test_valid_tool_added_with_duplicate_in_same_turn_is_still_exposed`, `test_two_new_tools_with_same_name_in_one_batch_keep_the_first`, `test_mcp_server_added_during_run_with_partially_clashing_functions_exposes_the_rest` | +| Runtime tool changes | Added tools become available on the next iteration and retain approval behavior. In an agent run, tools appended to the agent's tools, its MCP servers, or a run-level tools list are sent on the next model call in streaming and non-streaming execution; tools removed through the invocation context stay removed, and a tool whose name is already taken is skipped with a warning on its own, without dropping other tools added in the same turn or aborting the run. | `test_add_tools_available_next_iteration`, `test_add_tools_with_approval_required_tool`, `packages/core/tests/core/test_agents.py::test_tool_appended_to_agent_tools_during_run_is_sent_on_next_request`, `test_tool_appended_to_run_tools_during_run_is_sent_on_next_request`, `test_mcp_server_added_to_agent_during_run_is_sent_on_next_request`, `test_tool_removed_with_context_during_run_is_not_restored_from_agent_tools`, `test_tool_with_duplicate_name_added_during_run_is_skipped`, `test_tools_list_identity_is_unchanged_across_iterations_when_nothing_is_added`, `test_same_tool_object_appended_twice_during_run_is_idempotent`, `test_valid_tool_added_with_duplicate_in_same_turn_is_still_exposed`, `test_two_new_tools_with_same_name_in_one_batch_keep_the_first`, `test_mcp_server_added_during_run_with_partially_clashing_functions_exposes_the_rest`, `test_local_tool_wins_over_mcp_function_appended_in_same_turn` | | In-run compaction summaries | Summaries inserted for tool groups excluded by in-run compaction are returned in the final non-streaming response transcript before the group they replace, so history loaded with `skip_excluded` keeps the summarized content; summaries of caller-owned input messages are not added. | `packages/core/tests/core/test_clients.py::test_function_loop_returns_compaction_summaries_in_final_response`, `test_function_loop_returns_compaction_summaries_when_iteration_budget_exhausted`, `test_function_loop_reconciles_nested_compaction_summaries`, `test_function_loop_returns_compacted_transcript_on_early_terminal_exit`, `packages/core/tests/core/test_agents.py::test_agent_run_returns_and_persists_compaction_summaries` | | Per-service-call provider context | When framework-managed history is loaded around each model call, tools and instructions contributed by `HistoryProvider.before_run` remain available to the model and function loop without duplication in streaming or non-streaming execution. | `packages/core/tests/core/test_agents.py::test_vector_history_search_tool_is_available_with_per_service_call_persistence` | diff --git a/python/packages/core/agent_framework/_agents.py b/python/packages/core/agent_framework/_agents.py index d341772ad5f..c91b9627a49 100644 --- a/python/packages/core/agent_framework/_agents.py +++ b/python/packages/core/agent_framework/_agents.py @@ -1481,14 +1481,22 @@ def _extract_conversation_id_from_streaming_response( return None + def _run_tool_sources(self, agent_tools: Any, run_tools: Any) -> list[Any]: + """Return a run's tool sources in precedence order: agent tools, run-level tools, MCP servers. + + When two sources provide a tool with the same name, the tool from the earlier source is + the one the run keeps. Run start and the mid-run refresh both resolve tools in this order. + """ + return [agent_tools, run_tools, self.mcp_tools] + def _run_tool_source_items(self, run_tools: Any) -> list[Any]: - """Return the items of the tool lists a run re-reads before each model call. + """Return the items of the tool lists a run re-reads before each model call, in precedence order. - These are the agent's tools, its MCP servers, and the run-level tools when they - were passed as a list. + These are the agent's tools, the run-level tools when they were passed as a list, and + the agent's MCP servers. """ - items: list[Any] = [*self.mcp_tools] - for source in (self.default_options.get("tools"), run_tools): + items: list[Any] = [] + for source in self._run_tool_sources(self.default_options.get("tools"), run_tools): if isinstance(source, list): items.extend(cast("list[Any]", source)) return items @@ -1636,9 +1644,11 @@ async def _prepare_run_context( ) additional_function_arguments = {**effective_function_invocation_kwargs, **existing_additional_args} - # Resolve final tool list (configured tools + runtime provided tools + local MCP server tools) - final_tools = list(base_tools) - for tool in [*normalized_tools, *self.mcp_tools]: + # Resolve final tool list in source precedence order. The agent's tools (including tools from + # context providers) are used as given; run-level tools and MCP servers are resolved after them. + agent_tools, *resolved_sources = self._run_tool_sources(base_tools, normalized_tools) + final_tools = list(agent_tools) + for tool in (tool for source in resolved_sources for tool in source): _append_unique_tools( final_tools, await self._resolve_run_tool(tool, additional_function_arguments), diff --git a/python/packages/core/tests/core/test_agents.py b/python/packages/core/tests/core/test_agents.py index 4847251f161..8bb69998a5f 100644 --- a/python/packages/core/tests/core/test_agents.py +++ b/python/packages/core/tests/core/test_agents.py @@ -5166,3 +5166,65 @@ def connect_math_server() -> str: warnings = _duplicate_tool_warnings(caplog) assert len(warnings) == 1 assert "'multiply'" in warnings[0] + + +@pytest.mark.parametrize("stream", [False, True]) +async def test_local_tool_wins_over_mcp_function_appended_in_same_turn( + chat_client_base: MockBaseChatClient, + caplog: pytest.LogCaptureFixture, + stream: bool, +) -> None: + """A mid-run refresh uses run-start source precedence: agent tools, run-level tools, then MCP servers.""" + executed: list[str] = [] + math_server = _ConnectedMCPTool("math", ["multiply", "divide"]) + + @tool(name="multiply", approval_mode="never_require") + def local_multiply(a: int, b: int) -> int: + """Multiply two numbers locally.""" + executed.append("local_multiply") + return a * b + + @tool(name="load_math_tools", approval_mode="never_require") + def load_math_tools() -> str: + """Load the math tools.""" + agent.mcp_tools.append(math_server) + agent.default_options["tools"].append(local_multiply) + return "Math tools loaded." + + agent = Agent(client=chat_client_base, tools=[load_math_tools]) + _script_turns( + chat_client_base, + [ + [Content.from_function_call(call_id="call_1", name="load_math_tools", arguments="{}")], + [Content.from_function_call(call_id="call_2", name="multiply", arguments='{"a": 2, "b": 3}')], + [Content.from_text("6")], + ], + stream=stream, + ) + requested_tools: list[list[tuple[str, str | None]]] = [] + original_inner = chat_client_base._inner_get_response + + def capture_inner( + *, messages: MutableSequence[Message], stream: bool, options: dict[str, Any], **kwargs: Any + ) -> Any: + requested_tools.append([(tool.name, tool.description) for tool in options.get("tools", [])]) + return original_inner(messages=messages, stream=stream, options=options, **kwargs) + + with ( + caplog.at_level(logging.WARNING, logger="agent_framework"), + patch.object(chat_client_base, "_inner_get_response", side_effect=capture_inner), + ): + text = await _run_agent_text(agent, "What is 2 times 3?", stream=stream) + + local_multiply_entry = ("multiply", "Multiply two numbers locally.") + assert requested_tools[1] == [ + ("load_math_tools", "Load the math tools."), + local_multiply_entry, + ("divide", "divide from math"), + ] + assert requested_tools[2] == requested_tools[1] + assert executed == ["local_multiply"] + assert text == "6" + warnings = _duplicate_tool_warnings(caplog) + assert len(warnings) == 1 + assert "'multiply'" in warnings[0] From 628f38137e7f0d59eb25d4c42231f5e3eab283a1 Mon Sep 17 00:00:00 2001 From: Sivakumar Mahalingam <36898056+sivakumar-mahalingam@users.noreply.github.com> Date: Tue, 29 Sep 2026 20:23:11 +0400 Subject: [PATCH 4/5] Update docstring for _run_tool_sources method Clarified docstring to specify discovery order of tool sources and behavior for duplicate names. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- python/packages/core/agent_framework/_agents.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/packages/core/agent_framework/_agents.py b/python/packages/core/agent_framework/_agents.py index c91b9627a49..752677d3e81 100644 --- a/python/packages/core/agent_framework/_agents.py +++ b/python/packages/core/agent_framework/_agents.py @@ -1482,10 +1482,10 @@ def _extract_conversation_id_from_streaming_response( return None def _run_tool_sources(self, agent_tools: Any, run_tools: Any) -> list[Any]: - """Return a run's tool sources in precedence order: agent tools, run-level tools, MCP servers. + """Return a run's tool sources in discovery order: agent tools, run-level tools, then MCP servers. - When two sources provide a tool with the same name, the tool from the earlier source is - the one the run keeps. Run start and the mid-run refresh both resolve tools in this order. + Earlier sources win duplicate names at run start and among items discovered in the same refresh. + A name already exposed by an earlier refresh remains bound to its existing tool. """ return [agent_tools, run_tools, self.mcp_tools] From 6ea3c5053f8e5966f60605835e48bfe7ebb7bcc9 Mon Sep 17 00:00:00 2001 From: Sivakumar Mahalingam Date: Tue, 29 Sep 2026 20:46:11 +0400 Subject: [PATCH 5/5] Python: skip unchanged tool sources when refreshing run tools --- .../specs/004-python-function-calling-loop.md | 2 +- .../packages/core/agent_framework/_agents.py | 74 ++++--- .../packages/core/tests/core/test_agents.py | 208 ++++++++++++++++++ 3 files changed, 254 insertions(+), 30 deletions(-) diff --git a/docs/specs/004-python-function-calling-loop.md b/docs/specs/004-python-function-calling-loop.md index 5428caedf55..21377906aa3 100644 --- a/docs/specs/004-python-function-calling-loop.md +++ b/docs/specs/004-python-function-calling-loop.md @@ -591,7 +591,7 @@ that manually replay messages own the equivalent rule: do not resend an approval | OpenAI hosted/local shell boundary | Hosted shell calls remain informational in streaming and non-streaming responses even when a local executor is configured; only valid explicit local-shell items or shell calls marked with a local environment can execute, local execution preserves its configured approval mode, stateless loops preserve the complete provider shell transcript or fail explicitly, and locally generated shell outputs are sent in every continuation mode. | `packages/openai/tests/openai/test_openai_chat_client.py::test_response_content_creation_with_shell_call_remains_hosted_with_local_tool`, `test_parse_chunk_from_openai_shell_call_done_remains_hosted`, `test_parse_chunk_from_openai_local_environment_shell_call_done_emits_command`, `test_mixed_shell_calls_only_invoke_explicit_local_shell_call`, `test_stateless_shell_transcript_without_provider_item_fails`, `test_prepare_messages_keeps_local_shell_output_under_storage`, `test_malformed_local_environment_shell_call_is_not_executable`, `test_response_content_creation_with_local_shell_call_maps_to_function_call`, `test_malformed_local_shell_call_is_not_executable`, `test_response_function_call_named_local_shell_is_informational`, `test_parse_chunk_function_call_named_local_shell_is_informational`, `test_local_shell_tool_requires_approval_before_function_loop_execution` | | Declaration-only call | The call is surfaced as user input and is not executed; streaming arguments appear once while finalized request metadata remains available. | `test_declaration_only_tool`, `test_streaming_declaration_only_tool_preserves_metadata_without_duplicate_arguments` | | Function invocation disabled | The client bypasses the invocation loop without losing invocation kwargs. | `test_function_invocation_config_enabled_false`, `test_function_invocation_config_enabled_false_preserves_invocation_kwargs`, `test_streaming_function_invocation_config_enabled_false` | -| Runtime tool changes | Added tools become available on the next iteration and retain approval behavior. In an agent run, tools appended to the agent's tools, its MCP servers, or a run-level tools list are sent on the next model call in streaming and non-streaming execution; tools removed through the invocation context stay removed, and a tool whose name is already taken is skipped with a warning on its own, without dropping other tools added in the same turn or aborting the run. | `test_add_tools_available_next_iteration`, `test_add_tools_with_approval_required_tool`, `packages/core/tests/core/test_agents.py::test_tool_appended_to_agent_tools_during_run_is_sent_on_next_request`, `test_tool_appended_to_run_tools_during_run_is_sent_on_next_request`, `test_mcp_server_added_to_agent_during_run_is_sent_on_next_request`, `test_tool_removed_with_context_during_run_is_not_restored_from_agent_tools`, `test_tool_with_duplicate_name_added_during_run_is_skipped`, `test_tools_list_identity_is_unchanged_across_iterations_when_nothing_is_added`, `test_same_tool_object_appended_twice_during_run_is_idempotent`, `test_valid_tool_added_with_duplicate_in_same_turn_is_still_exposed`, `test_two_new_tools_with_same_name_in_one_batch_keep_the_first`, `test_mcp_server_added_during_run_with_partially_clashing_functions_exposes_the_rest`, `test_local_tool_wins_over_mcp_function_appended_in_same_turn` | +| Runtime tool changes | Added tools become available on the next iteration and retain approval behavior. In an agent run, tools appended to the agent's tools, its MCP servers, or a run-level tools list are sent on the next model call in streaming and non-streaming execution; tools removed through the invocation context stay removed, and a tool whose name is already taken is skipped with a warning on its own, without dropping other tools added in the same turn or aborting the run. Tools added mid-run are resolved in run-start source order (agent tools, run-level tools, then MCP servers), so the earlier source keeps a clashing name; at run start the same clash raises instead. Before each model call a source list is re-scanned only when it was replaced or its length changed, so an unchanged source costs no scan and an in-place replacement that keeps the same length takes effect from the next run. | `test_add_tools_available_next_iteration`, `test_add_tools_with_approval_required_tool`, `packages/core/tests/core/test_agents.py::test_tool_appended_to_agent_tools_during_run_is_sent_on_next_request`, `test_tool_appended_to_run_tools_during_run_is_sent_on_next_request`, `test_mcp_server_added_to_agent_during_run_is_sent_on_next_request`, `test_tool_removed_with_context_during_run_is_not_restored_from_agent_tools`, `test_tool_with_duplicate_name_added_during_run_is_skipped`, `test_tools_list_identity_is_unchanged_across_iterations_when_nothing_is_added`, `test_same_tool_object_appended_twice_during_run_is_idempotent`, `test_valid_tool_added_with_duplicate_in_same_turn_is_still_exposed`, `test_two_new_tools_with_same_name_in_one_batch_keep_the_first`, `test_mcp_server_added_during_run_with_partially_clashing_functions_exposes_the_rest`, `test_local_tool_wins_over_mcp_function_appended_in_same_turn`, `test_tools_added_mid_run_match_run_start_tool_order`, `test_run_start_rejects_mcp_function_clashing_with_local_tool`, `test_refresh_does_not_scan_unchanged_tool_sources`, `test_disconnected_mcp_server_added_during_run_is_entered_discovered_and_closed` | | In-run compaction summaries | Summaries inserted for tool groups excluded by in-run compaction are returned in the final non-streaming response transcript before the group they replace, so history loaded with `skip_excluded` keeps the summarized content; summaries of caller-owned input messages are not added. | `packages/core/tests/core/test_clients.py::test_function_loop_returns_compaction_summaries_in_final_response`, `test_function_loop_returns_compaction_summaries_when_iteration_budget_exhausted`, `test_function_loop_reconciles_nested_compaction_summaries`, `test_function_loop_returns_compacted_transcript_on_early_terminal_exit`, `packages/core/tests/core/test_agents.py::test_agent_run_returns_and_persists_compaction_summaries` | | Per-service-call provider context | When framework-managed history is loaded around each model call, tools and instructions contributed by `HistoryProvider.before_run` remain available to the model and function loop without duplication in streaming or non-streaming execution. | `packages/core/tests/core/test_agents.py::test_vector_history_search_tool_is_available_with_per_service_call_persistence` | diff --git a/python/packages/core/agent_framework/_agents.py b/python/packages/core/agent_framework/_agents.py index 752677d3e81..5493de9e4dc 100644 --- a/python/packages/core/agent_framework/_agents.py +++ b/python/packages/core/agent_framework/_agents.py @@ -1489,17 +1489,17 @@ def _run_tool_sources(self, agent_tools: Any, run_tools: Any) -> list[Any]: """ return [agent_tools, run_tools, self.mcp_tools] - def _run_tool_source_items(self, run_tools: Any) -> list[Any]: - """Return the items of the tool lists a run re-reads before each model call, in precedence order. + def _run_tool_source_lists(self, run_tools: Any) -> list[list[Any] | None]: + """Return the tool lists a run re-reads before each model call, in precedence order. - These are the agent's tools, the run-level tools when they were passed as a list, and - the agent's MCP servers. + These are the agent's tools, the run-level tools, and the agent's MCP servers. A source + that is not a list (for example a tuple or a single run-level tool) cannot grow during + the run and is returned as ``None``. """ - items: list[Any] = [] - for source in self._run_tool_sources(self.default_options.get("tools"), run_tools): - if isinstance(source, list): - items.extend(cast("list[Any]", source)) - return items + return [ + cast("list[Any]", source) if isinstance(source, list) else None + for source in self._run_tool_sources(self.default_options.get("tools"), run_tools) + ] async def _resolve_run_tool( self, @@ -1542,8 +1542,14 @@ async def _prepare_run_context( tools_ = _select_run_level_tools(tools, opts) opts.pop("tools", None) # Items already in the agent's and the run's tool lists when the run starts; anything - # appended to those lists later is added to the run before its next model call. - seen_tool_items = {id(item): item for item in self._run_tool_source_items(tools_)} + # appended to those lists later is added to the run before its next model call. Each + # source's watermark (the list object and its length) lets an unchanged source be skipped + # without scanning its items. + start_source_lists = self._run_tool_source_lists(tools_) + seen_tool_items = {id(item): item for source in start_source_lists if source is not None for item in source} + source_watermarks: list[tuple[list[Any], int] | None] = [ + (source, len(source)) if source is not None else None for source in start_source_lists + ] input_messages = normalize_messages(messages) @@ -1665,25 +1671,35 @@ async def refresh_run_tools() -> list[ToolTypes]: # Candidates are returned without de-duplication; the function-invocation loop merges # them one at a time and skips only the names that clash. candidates: list[ToolTypes] = [] - for item in self._run_tool_source_items(tools_): - if id(item) in seen_tool_items: + for index, source in enumerate(self._run_tool_source_lists(tools_)): + watermark = source_watermarks[index] + # Same list object and length as last time: nothing was appended, so skip the scan. + if source is None or (watermark is not None and watermark[0] is source and watermark[1] == len(source)): continue - try: - resolved = [ - resolved_tool - for tool in _normalize_tools(item) - for resolved_tool in await self._resolve_run_tool(tool, mcp_run_kwargs) - ] - except Exception: - # Left unseen, so resolution is retried before the next model call. - logger.warning( - "Tool %r added during the run could not be resolved; retrying before the next model call.", - item, - exc_info=True, - ) - continue - seen_tool_items[id(item)] = item - candidates.extend(resolved) + source_resolved = True + for item in source: + if id(item) in seen_tool_items: + continue + try: + resolved = [ + resolved_tool + for tool in _normalize_tools(item) + for resolved_tool in await self._resolve_run_tool(tool, mcp_run_kwargs) + ] + except Exception: + # Left unseen, and the watermark is not advanced, so resolution is retried + # before the next model call. + logger.warning( + "Tool %r added during the run could not be resolved; retrying before the next model call.", + item, + exc_info=True, + ) + source_resolved = False + continue + seen_tool_items[id(item)] = item + candidates.extend(resolved) + if source_resolved: + source_watermarks[index] = (source, len(source)) return candidates additional_function_arguments[_PARENT_TOOL_APPROVAL_SOURCE_IDS_CONTEXT_KEY] = _tool_approval_source_ids( diff --git a/python/packages/core/tests/core/test_agents.py b/python/packages/core/tests/core/test_agents.py index 8bb69998a5f..6d152bd10de 100644 --- a/python/packages/core/tests/core/test_agents.py +++ b/python/packages/core/tests/core/test_agents.py @@ -5228,3 +5228,211 @@ def capture_inner( warnings = _duplicate_tool_warnings(caplog) assert len(warnings) == 1 assert "'multiply'" in warnings[0] + + +@pytest.mark.parametrize("stream", [False, True]) +async def test_tools_added_mid_run_match_run_start_tool_order( + chat_client_base: MockBaseChatClient, + stream: bool, +) -> None: + """Tools appended mid-run are exposed in the same order as when configured at run start.""" + + def make_tools() -> tuple[FunctionTool, _ConnectedMCPTool]: + @tool(name="add", approval_mode="never_require") + def local_add(a: int, b: int) -> int: + """Add two numbers locally.""" + return a + b + + return local_add, _ConnectedMCPTool("math", ["multiply", "divide"]) + + @tool(name="load_math_tools", approval_mode="never_require") + def load_math_tools() -> str: + """Load the math tools.""" + agent.mcp_tools.append(mid_run_server) + agent.default_options["tools"].append(mid_run_add) + return "Math tools loaded." + + def capture_tools(requested_tools: list[list[tuple[str, str | None]]]) -> Any: + original_inner = chat_client_base._inner_get_response + + def capture_inner( + *, messages: MutableSequence[Message], stream: bool, options: dict[str, Any], **kwargs: Any + ) -> Any: + requested_tools.append([(tool.name, tool.description) for tool in options.get("tools", [])]) + return original_inner(messages=messages, stream=stream, options=options, **kwargs) + + return patch.object(chat_client_base, "_inner_get_response", side_effect=capture_inner) + + run_start_add, run_start_server = make_tools() + run_start_agent = Agent(client=chat_client_base, tools=[load_math_tools, run_start_add, run_start_server]) + _script_turns(chat_client_base, [[Content.from_text("done")]], stream=stream) + run_start_tools: list[list[tuple[str, str | None]]] = [] + with capture_tools(run_start_tools): + await _run_agent_text(run_start_agent, "Hello.", stream=stream) + + mid_run_add, mid_run_server = make_tools() + agent = Agent(client=chat_client_base, tools=[load_math_tools]) + _script_turns( + chat_client_base, + [ + [Content.from_function_call(call_id="call_1", name="load_math_tools", arguments="{}")], + [Content.from_text("done")], + ], + stream=stream, + ) + mid_run_tools: list[list[tuple[str, str | None]]] = [] + with capture_tools(mid_run_tools): + await _run_agent_text(agent, "Load the math tools.", stream=stream) + + assert run_start_tools[0] == [ + ("load_math_tools", "Load the math tools."), + ("add", "Add two numbers locally."), + ("multiply", "multiply from math"), + ("divide", "divide from math"), + ] + assert mid_run_tools[1] == run_start_tools[0] + + +@pytest.mark.parametrize("stream", [False, True]) +async def test_run_start_rejects_mcp_function_clashing_with_local_tool( + chat_client_base: MockBaseChatClient, + stream: bool, +) -> None: + """At run start the MCP function is the rejected duplicate, the same one a mid-run refresh skips.""" + + @tool(name="multiply", approval_mode="never_require") + def local_multiply(a: int, b: int) -> int: + """Multiply two numbers locally.""" + return a * b + + agent = Agent(client=chat_client_base, tools=[local_multiply, _ConnectedMCPTool("math", ["multiply", "divide"])]) + _script_turns(chat_client_base, [[Content.from_text("done")]], stream=stream) + + with pytest.raises(ValueError, match=r"Duplicate tool name 'multiply'\..*tool_name_prefix.*MCPTool"): + await _run_agent_text(agent, "What is 2 times 3?", stream=stream) + + +class _IterationCountingList(list[Any]): + """List that counts how many times it is iterated.""" + + iterations = 0 + + def __iter__(self) -> Any: + self.iterations += 1 + return super().__iter__() + + +@pytest.mark.parametrize("stream", [False, True]) +async def test_refresh_does_not_scan_unchanged_tool_sources( + chat_client_base: MockBaseChatClient, + stream: bool, +) -> None: + @tool(name="lookup_weather", approval_mode="never_require") + def lookup_weather(location: str) -> str: + """Get the weather.""" + return "sunny" + + agent = Agent(client=chat_client_base, tools=[lookup_weather]) + agent_tools = _IterationCountingList(agent.default_options["tools"]) + agent.default_options["tools"] = agent_tools + run_tools = _IterationCountingList() + _script_turns( + chat_client_base, + [ + [Content.from_function_call(call_id="call_1", name="lookup_weather", arguments='{"location": "Seattle"}')], + [Content.from_function_call(call_id="call_2", name="lookup_weather", arguments='{"location": "Boston"}')], + [Content.from_text("done")], + ], + stream=stream, + ) + iterations_per_request: list[tuple[int, int]] = [] + original_inner = chat_client_base._inner_get_response + + def capture_inner( + *, messages: MutableSequence[Message], stream: bool, options: dict[str, Any], **kwargs: Any + ) -> Any: + iterations_per_request.append((agent_tools.iterations, run_tools.iterations)) + return original_inner(messages=messages, stream=stream, options=options, **kwargs) + + with patch.object(chat_client_base, "_inner_get_response", side_effect=capture_inner): + if stream: + text = "".join([update.text or "" async for update in agent.run("weather?", tools=run_tools, stream=True)]) + else: + text = (await agent.run("weather?", tools=run_tools)).text + + assert text == "done" + assert len(iterations_per_request) == 3 + # Run preparation iterates the sources; the refreshes before later model calls do not. + assert iterations_per_request[1] == iterations_per_request[0] + assert iterations_per_request[2] == iterations_per_request[0] + + +class _DiscoveringMCPTool(_ConnectedMCPTool): + """Disconnected MCP test double whose functions are discovered only when it is entered.""" + + def __init__(self, name: str, function_names: list[str], events: list[str]) -> None: + super().__init__(name=name, function_names=function_names) + self._undiscovered_functions = self._functions + self._functions = [] + self.is_connected = False + self.events = events + + async def __aenter__(self) -> "_DiscoveringMCPTool": + self.events.append(f"{self.name}.enter") + self._functions = self._undiscovered_functions + self.is_connected = True + return self + + async def __aexit__( + self, exc_type: type[BaseException] | None, exc_value: BaseException | None, traceback: Any + ) -> None: + self.events.append(f"{self.name}.exit") + self.is_connected = False + + +@pytest.mark.parametrize("stream", [False, True]) +async def test_disconnected_mcp_server_added_during_run_is_entered_discovered_and_closed( + chat_client_base: MockBaseChatClient, + stream: bool, +) -> None: + events: list[str] = [] + math_server = _DiscoveringMCPTool("math", ["multiply", "divide"], events) + + @tool(name="connect_math_server", approval_mode="never_require") + def connect_math_server() -> str: + """Connect the math MCP server.""" + agent.mcp_tools.append(math_server) + return "Math server connected." + + agent = Agent(client=chat_client_base, tools=[connect_math_server]) + _script_turns( + chat_client_base, + [ + [Content.from_function_call(call_id="call_1", name="connect_math_server", arguments="{}")], + [Content.from_text("done")], + ], + stream=stream, + ) + original_inner = chat_client_base._inner_get_response + + def capture_inner( + *, messages: MutableSequence[Message], stream: bool, options: dict[str, Any], **kwargs: Any + ) -> Any: + events.append("request: " + ", ".join(tool.name for tool in options.get("tools", []))) + return original_inner(messages=messages, stream=stream, options=options, **kwargs) + + with patch.object(chat_client_base, "_inner_get_response", side_effect=capture_inner): + text = await _run_agent_text(agent, "Connect the math server.", stream=stream) + + assert text == "done" + assert events == [ + "request: connect_math_server", + "math.enter", + "request: connect_math_server, multiply, divide", + ] + assert math_server.is_connected + + await agent.close() + + assert events[-1] == "math.exit" + assert not math_server.is_connected