diff --git a/python/packages/foundry/agent_framework_foundry/_chat_client.py b/python/packages/foundry/agent_framework_foundry/_chat_client.py index 30dbc443065..f54d28d1a3f 100644 --- a/python/packages/foundry/agent_framework_foundry/_chat_client.py +++ b/python/packages/foundry/agent_framework_foundry/_chat_client.py @@ -355,6 +355,12 @@ def _check_model_presence(self, options: dict[str, Any]) -> None: raise ValueError("model must be a non-empty string") options["model"] = self.model + @override + def _shell_call_targets_local_executor(self, item: Any) -> bool: + # Foundry omits the local environment marker, so a registered executor owns every + # shell call that is not explicitly marked hosted. + return getattr(getattr(item, "environment", None), "type", None) in (None, "local") + @override def _prepare_tools_for_openai( self, diff --git a/python/packages/foundry/tests/foundry/test_foundry_chat_client.py b/python/packages/foundry/tests/foundry/test_foundry_chat_client.py index a4660da378c..ad9b323a6fe 100644 --- a/python/packages/foundry/tests/foundry/test_foundry_chat_client.py +++ b/python/packages/foundry/tests/foundry/test_foundry_chat_client.py @@ -43,6 +43,7 @@ from azure.core.pipeline.transport import HttpRequest, HttpResponse, HttpTransport from azure.identity import AzureCliCredential from openai import AsyncOpenAI, BadRequestError, DefaultAsyncHttpxClient +from openai.types.responses import ResponseFunctionShellToolCall, ResponseFunctionShellToolCallOutput from pydantic import BaseModel from pytest import param @@ -2224,3 +2225,279 @@ async def test_shared_chat_client_keeps_concurrent_agent_runs_isolated( assert first_session.service_session_id == "response-first" assert second_session.service_session_id == "response-second" assert transport.max_active_requests == 2 + + +def _make_foundry_client() -> FoundryChatClient: + project_client = MagicMock() + project_client.get_openai_client.return_value = _make_mock_openai_client() + return FoundryChatClient(project_client=project_client, model="test-model") + + +def test_foundry_shell_call_without_environment_uses_registered_local_executor() -> None: + """Foundry omits the local environment marker, so a registered executor still runs the call.""" + client = _make_foundry_client() + + def local_exec(command: str) -> str: + return command + + local_shell_tool = FoundryChatClient.get_shell_tool(func=local_exec) + shell_call = ResponseFunctionShellToolCall.model_validate({ + "id": "shell-item-1", + "type": "shell_call", + "call_id": "shell-call-1", + "action": {"commands": ["python --version"], "timeout_ms": 30000}, + "status": "completed", + }) + assert shell_call.environment is None + mock_response = MagicMock( + id="resp-1", + model="test-model", + created_at=1000000000, + metadata={}, + output_parsed=None, + output=[shell_call], + usage=None, + conversation=None, + status="completed", + incomplete=None, + ) + + response = client._parse_response_from_openai(mock_response, options={"tools": [local_shell_tool]}) + + call_content = response.messages[0].contents[0] + assert call_content.type == "function_call" + assert call_content.call_id == "shell-call-1" + assert call_content.name == local_shell_tool.name + assert call_content.parse_arguments() == {"command": "python --version"} + + +def test_foundry_shell_call_with_container_environment_remains_hosted() -> None: + """An explicit hosted container environment stays informational even with a local tool.""" + client = _make_foundry_client() + + def local_exec(command: str) -> str: + return command + + local_shell_tool = FoundryChatClient.get_shell_tool(func=local_exec) + shell_call = ResponseFunctionShellToolCall.model_validate({ + "id": "shell-item-1", + "type": "shell_call", + "call_id": "shell-call-1", + "action": {"commands": ["ls -la"], "timeout_ms": 30000}, + "environment": {"type": "container_reference", "container_id": "container-1"}, + "status": "completed", + }) + mock_response = MagicMock( + id="resp-1", + model="test-model", + created_at=1000000000, + metadata={}, + output_parsed=None, + output=[shell_call], + usage=None, + conversation=None, + status="completed", + incomplete=None, + ) + + response = client._parse_response_from_openai(mock_response, options={"tools": [local_shell_tool]}) + + call_content = response.messages[0].contents[0] + assert call_content.type == "shell_tool_call" + assert call_content.call_id == "shell-call-1" + assert call_content.commands == ["ls -la"] + + +def test_foundry_streaming_shell_call_without_environment_emits_command() -> None: + """The streaming path also routes an env-less Foundry shell call to the local executor.""" + client = _make_foundry_client() + + def local_exec(command: str) -> str: + return command + + local_shell_tool = FoundryChatClient.get_shell_tool(func=local_exec, approval_mode="never_require") + shell_call = ResponseFunctionShellToolCall.model_validate({ + "id": "shell-item-1", + "type": "shell_call", + "call_id": "shell-call-1", + "action": {"commands": ["python --version"], "timeout_ms": 30000}, + "status": "completed", + }) + done_event = MagicMock(type="response.output_item.done", item=shell_call) + + update = client._parse_chunk_from_openai(done_event, options={"tools": [local_shell_tool]}, function_call_ids={}) + + call_content = update.contents[0] + assert call_content.type == "function_call" + assert call_content.call_id == "shell-call-1" + assert call_content.name == local_shell_tool.name + assert call_content.parse_arguments() == {"command": "python --version"} + + +async def test_foundry_shell_call_without_environment_executes_and_replays_output() -> None: + """Regression for #8663: the env-less Foundry shell call runs and its output is sent back.""" + executed_commands: list[str] = [] + + def local_exec(command: str) -> str: + executed_commands.append(command) + return "Python 3.13.0" + + local_shell_tool = FoundryChatClient.get_shell_tool(func=local_exec, approval_mode="never_require") + shell_call = ResponseFunctionShellToolCall.model_validate({ + "id": "shell-item-1", + "type": "shell_call", + "call_id": "shell-call-1", + "action": {"commands": ["python --version"], "timeout_ms": 30000}, + "status": "completed", + }) + mock_response1 = MagicMock( + id="resp-1", + model="test-model", + created_at=1000000000, + metadata={}, + output_parsed=None, + output=[shell_call], + usage=None, + conversation=None, + status="completed", + finish_reason="tool_calls", + incomplete=None, + ) + text_item = MagicMock(type="message") + text_content = MagicMock(type="output_text", text="Python 3.13.0", annotations=[], logprobs=None) + text_item.content = [text_content] + mock_response2 = MagicMock( + id="resp-2", + model="test-model", + created_at=1000000001, + metadata={}, + output_parsed=None, + output=[text_item], + usage=None, + conversation=None, + status="completed", + finish_reason="stop", + incomplete=None, + ) + + mock_openai_client = _make_mock_openai_client() + mock_openai_client.responses.with_raw_response.create.side_effect = [ + _as_raw(mock_response1), + _as_raw(mock_response2), + ] + project_client = MagicMock() + project_client.get_openai_client.return_value = mock_openai_client + client = FoundryChatClient(project_client=project_client, model="test-model") + + await client.get_response( + messages=[Message(role="user", contents=["What Python version is available?"])], + options={"tools": [local_shell_tool], "store": False}, + ) + + assert executed_commands == ["python --version"] + assert mock_openai_client.responses.with_raw_response.create.call_count == 2 + second_call_input = mock_openai_client.responses.with_raw_response.create.call_args_list[1].kwargs["input"] + shell_outputs = [item for item in second_call_input if item.get("type") == "shell_call_output"] + assert len(shell_outputs) == 1 + assert shell_outputs[0]["call_id"] == "shell-call-1" + assert shell_outputs[0]["output"][0]["stdout"] == "Python 3.13.0" + + +def test_foundry_shell_call_with_matching_output_stays_hosted() -> None: + """An env-less shell call the provider already ran (matching output present) is not re-run locally.""" + client = _make_foundry_client() + + def local_exec(command: str) -> str: + return command + + local_shell_tool = FoundryChatClient.get_shell_tool(func=local_exec, approval_mode="never_require") + shell_call = ResponseFunctionShellToolCall.model_validate({ + "id": "shell-item-1", + "type": "shell_call", + "call_id": "shell-call-1", + "action": {"commands": ["pwd"], "timeout_ms": 30000}, + "status": "completed", + }) + shell_output = ResponseFunctionShellToolCallOutput.model_validate({ + "id": "shell-output-1", + "type": "shell_call_output", + "call_id": "shell-call-1", + "output": [{"stdout": "/workspace", "stderr": "", "outcome": {"type": "exit", "exit_code": 0}}], + "status": "completed", + }) + mock_response = MagicMock( + id="resp-1", + model="test-model", + created_at=1000000000, + metadata={}, + output_parsed=None, + output=[shell_call, shell_output], + usage=None, + conversation=None, + status="completed", + incomplete=None, + ) + + response = client._parse_response_from_openai(mock_response, options={"tools": [local_shell_tool]}) + + call_content = response.messages[0].contents[0] + assert call_content.type == "shell_tool_call" + assert call_content.call_id == "shell-call-1" + assert all(content.type != "function_call" for content in response.messages[0].contents) + + +async def test_foundry_shell_call_with_matching_output_is_not_reexecuted() -> None: + """Regression for the mixed call/output case: a provider-run shell call is not executed locally.""" + executed_commands: list[str] = [] + + def local_exec(command: str) -> str: + executed_commands.append(command) + return "should not run" + + local_shell_tool = FoundryChatClient.get_shell_tool(func=local_exec, approval_mode="never_require") + shell_call = ResponseFunctionShellToolCall.model_validate({ + "id": "shell-item-1", + "type": "shell_call", + "call_id": "shell-call-1", + "action": {"commands": ["pwd"], "timeout_ms": 30000}, + "status": "completed", + }) + shell_output = ResponseFunctionShellToolCallOutput.model_validate({ + "id": "shell-output-1", + "type": "shell_call_output", + "call_id": "shell-call-1", + "output": [{"stdout": "/workspace", "stderr": "", "outcome": {"type": "exit", "exit_code": 0}}], + "status": "completed", + }) + text_item = MagicMock(type="message") + text_content = MagicMock(type="output_text", text="/workspace", annotations=[], logprobs=None) + text_item.content = [text_content] + mock_response1 = MagicMock( + id="resp-1", + model="test-model", + created_at=1000000000, + metadata={}, + output_parsed=None, + output=[shell_call, shell_output, text_item], + usage=None, + conversation=None, + status="completed", + finish_reason="stop", + incomplete=None, + ) + + mock_openai_client = _make_mock_openai_client() + mock_openai_client.responses.with_raw_response.create.side_effect = [_as_raw(mock_response1)] + project_client = MagicMock() + project_client.get_openai_client.return_value = mock_openai_client + client = FoundryChatClient(project_client=project_client, model="test-model") + + await client.get_response( + messages=[Message(role="user", contents=["What directory are we in?"])], + options={"tools": [local_shell_tool], "store": False}, + ) + + # The provider already produced the output, so the local executor must not run and no second + # request (with a duplicate output for the same call_id) is issued. + assert executed_commands == [] + assert mock_openai_client.responses.with_raw_response.create.call_count == 1 diff --git a/python/packages/openai/agent_framework_openai/_chat_client.py b/python/packages/openai/agent_framework_openai/_chat_client.py index e1686b071a4..818b6d1fd94 100644 --- a/python/packages/openai/agent_framework_openai/_chat_client.py +++ b/python/packages/openai/agent_framework_openai/_chat_client.py @@ -2455,7 +2455,33 @@ def _image_generation_item_to_contents(self, item: Any) -> list[Content]: ), ] - def _shell_item_to_contents(self, item: Any, local_shell_tool_name: str | None) -> list[Content]: + def _shell_call_targets_local_executor(self, item: Any) -> bool: + """Whether a ``shell_call`` item should run against a registered local executor. + + The OpenAI Responses service echoes ``environment.type == "local"`` for local shell + calls and container types for hosted ones, so the marker disambiguates mixed + hosted/local responses. Providers that omit the marker override this. + """ + return getattr(getattr(item, "environment", None), "type", None) == "local" + + @staticmethod + def _shell_output_call_ids(items: Sequence[Any]) -> set[str]: + """Call IDs the provider already emitted a ``shell_call_output`` for in this response.""" + call_ids: set[str] = set() + for item in items: + if getattr(item, "type", None) != "shell_call_output": + continue + call_id = getattr(item, "call_id", None) + if isinstance(call_id, str) and call_id: + call_ids.add(call_id) + return call_ids + + def _shell_item_to_contents( + self, + item: Any, + local_shell_tool_name: str | None, + provider_executed_call_ids: set[str] | None = None, + ) -> list[Content]: """Convert a shell output item into framework ``Content`` objects. Handles ``shell_call``, ``local_shell_call``, and ``shell_call_output`` items. @@ -2464,6 +2490,10 @@ def _shell_item_to_contents(self, item: Any, local_shell_tool_name: str | None) populated ``action`` (commands are not available on the earlier ``response.output_item.added`` event, and there are no shell-specific streaming delta events). + + ``provider_executed_call_ids`` are the call IDs the provider already returned a + ``shell_call_output`` for in the same response; such a ``shell_call`` is never routed to a + local executor, since re-running it would duplicate the provider's output on replay. """ contents: list[Content] = [] item_type = getattr(item, "type", None) @@ -2483,10 +2513,13 @@ def _shell_item_to_contents(self, item: Any, local_shell_tool_name: str | None) ) shell_timeout_ms = getattr(action, "timeout_ms", None) shell_max_output = getattr(action, "max_output_length", None) - is_local_environment = getattr(getattr(item, "environment", None), "type", None) == "local" + is_local_environment = self._shell_call_targets_local_executor(item) + # A matching provider output means the service already ran this call; never re-execute it locally. + provider_already_executed = bool(provider_executed_call_ids) and shell_call_id in provider_executed_call_ids if ( local_shell_tool_name and is_local_environment + and not provider_already_executed and shell_call_id and shell_call_item_id and shell_commands @@ -3214,6 +3247,7 @@ def _parse_response_from_openai( response_outputs = response.output # type: ignore[reportUnknownMemberType] except AttributeError: response_outputs = [] + provider_executed_shell_call_ids = self._shell_output_call_ids(response_outputs) # type: ignore[reportUnknownArgumentType] for item in response_outputs: # type: ignore[reportUnknownVariableType] match item.type: # types: @@ -3473,7 +3507,9 @@ def _parse_response_from_openai( case "image_generation_call": # ResponseOutputImageGenerationCall contents.extend(self._image_generation_item_to_contents(item)) case "shell_call" | "local_shell_call" | "shell_call_output": - contents.extend(self._shell_item_to_contents(item, local_shell_tool_name)) + contents.extend( + self._shell_item_to_contents(item, local_shell_tool_name, provider_executed_shell_call_ids) + ) case _: logger.debug("Unparsed output of type: %s: %s", item.type, item) self._mark_completed_computer_calls(contents) @@ -4079,7 +4115,9 @@ def _get_ann_value(key: str) -> Any: contents.append(self._parse_search_tool_result_content(done_item)) elif getattr(done_item, "type", None) in ("shell_call", "local_shell_call", "shell_call_output"): # Shell items are parsed here (not on `response.output_item.added`) because the - # command/output is only populated on the completed item. + # command/output is only populated on the completed item. The full item list + # isn't available mid-stream, so the provider-output guard applies only to the + # non-streaming path; a delegated shell call never streams its own output. contents.extend(self._shell_item_to_contents(done_item, local_shell_tool_name)) elif getattr(done_item, "type", None) == "computer_call": contents.append(self._parse_computer_tool_call_content(done_item))