From 931d4513773cb175d816474fbe45e2098dee4e56 Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Wed, 10 Jun 2026 13:15:49 +0000 Subject: [PATCH 1/3] Integrate shell tool into AgentHarness --- .../packages/core/agent_framework/__init__.py | 2 + .../packages/core/agent_framework/_clients.py | 30 +++ .../core/agent_framework/_harness/_agent.py | 59 +++++- .../packages/core/agent_framework/_skills.py | 10 +- .../core/tests/core/test_harness_agent.py | 92 +++++++++ .../core/tests/core/test_mcp_observability.py | 4 +- .../core/tests/core/test_mcp_skills.py | 190 ++++++++---------- python/samples/02-agents/harness/README.md | 23 +++ 8 files changed, 288 insertions(+), 122 deletions(-) diff --git a/python/packages/core/agent_framework/__init__.py b/python/packages/core/agent_framework/__init__.py index b287b4be579..635f18bf9ef 100644 --- a/python/packages/core/agent_framework/__init__.py +++ b/python/packages/core/agent_framework/__init__.py @@ -27,6 +27,7 @@ SupportsGetEmbeddings, SupportsImageGenerationTool, SupportsMCPTool, + SupportsShellTool, SupportsWebSearchTool, ) from ._compaction import ( @@ -496,6 +497,7 @@ "SupportsGetEmbeddings", "SupportsImageGenerationTool", "SupportsMCPTool", + "SupportsShellTool", "SupportsWebSearchTool", "SwitchCaseEdgeGroup", "SwitchCaseEdgeGroupCase", diff --git a/python/packages/core/agent_framework/_clients.py b/python/packages/core/agent_framework/_clients.py index 746427bffda..f0bd051980d 100644 --- a/python/packages/core/agent_framework/_clients.py +++ b/python/packages/core/agent_framework/_clients.py @@ -819,6 +819,36 @@ def get_file_search_tool(**kwargs: Any) -> Any: ... +@runtime_checkable +class SupportsShellTool(Protocol): + """Protocol for clients that support shell tools. + + This protocol enables runtime checking to determine if a client + supports executing shell commands. + + Examples: + .. code-block:: python + + from agent_framework import SupportsShellTool + + if isinstance(client, SupportsShellTool): + tool = client.get_shell_tool(func=shell.as_function()) + agent = ChatAgent(client, tools=[tool]) + """ + + @staticmethod + def get_shell_tool(**kwargs: Any) -> Any: + """Create a shell tool configuration. + + Keyword Args: + **kwargs: Provider-specific configuration options. + + Returns: + A tool configuration ready to pass to ChatAgent. + """ + ... + + # endregion diff --git a/python/packages/core/agent_framework/_harness/_agent.py b/python/packages/core/agent_framework/_harness/_agent.py index 5896f721418..987f617c37f 100644 --- a/python/packages/core/agent_framework/_harness/_agent.py +++ b/python/packages/core/agent_framework/_harness/_agent.py @@ -15,7 +15,7 @@ from typing import TYPE_CHECKING, Any from .._agents import Agent, SupportsAgentRun -from .._clients import SupportsWebSearchTool +from .._clients import SupportsShellTool, SupportsWebSearchTool from .._compaction import CompactionProvider, ContextWindowCompactionStrategy, ToolResultCompactionStrategy from .._feature_stage import ExperimentalFeature, experimental from .._sessions import ContextProvider, HistoryProvider, InMemoryHistoryProvider @@ -106,6 +106,7 @@ def _assemble_context_providers( skills_paths: Sequence[str] | None, background_agents: Sequence[SupportsAgentRun] | None, background_agents_instructions: str | None, + shell_context_provider: ContextProvider | None, extra_context_providers: Sequence[ContextProvider] | None, ) -> list[ContextProvider]: """Assemble the ordered list of context providers.""" @@ -137,6 +138,10 @@ def _assemble_context_providers( if background_agents: providers.append(BackgroundAgentsProvider(background_agents, instructions=background_agents_instructions)) + # Shell environment provider is opt-in: only added when a shell tool was wired. + if shell_context_provider is not None: + providers.append(shell_context_provider) + # Append any user-supplied additional providers. if extra_context_providers: providers.extend(extra_context_providers) @@ -144,6 +149,37 @@ def _assemble_context_providers( return providers +def _assemble_shell( + client: SupportsChatGetResponse[Any], + shell_executor: Any, + shell_environment_provider_options: Any, +) -> tuple[ToolTypes | None, ContextProvider | None]: + """Build the shell tool and environment provider when a shell executor is supplied. + + Returns a ``(tool, provider)`` tuple. Both are ``None`` when no shell executor is + provided, or when the client does not support shell tools (a warning is logged in the + latter case, since the environment provider is not useful without an execution path). + """ + if shell_executor is None: + return None, None + + if not isinstance(client, SupportsShellTool): + logger.warning( + "Shell tool not available: client %r does not implement SupportsShellTool. " + "Skipping the shell tool and environment provider.", + type(client).__name__, + ) + return None, None + + # Imported lazily: the shell types live in the separate agent-framework-tools package, + # which depends on core, so core cannot import them at module load time. + from agent_framework_tools.shell import ShellEnvironmentProvider + + shell_tool = client.get_shell_tool(func=shell_executor.as_function()) + shell_provider = ShellEnvironmentProvider(shell_executor, shell_environment_provider_options) + return shell_tool, shell_provider + + HARNESS_AGENT_PROVIDER_NAME = "microsoft.agent_framework.harness" @@ -174,6 +210,8 @@ def create_harness_agent( skills_paths: Sequence[str] | None = None, background_agents: Sequence[SupportsAgentRun] | None = None, background_agents_instructions: str | None = None, + shell_executor: Any = None, + shell_environment_provider_options: Any = None, disable_web_search: bool = False, otel_provider_name: str | None = None, context_providers: Sequence[ContextProvider] | None = None, @@ -270,6 +308,15 @@ def create_harness_agent( background_agents_instructions: Optional instruction override for the ``BackgroundAgentsProvider``. May include ``{background_agents}`` placeholder which will be replaced with the agent listing. + shell_executor: Optional shell tool that enables shell command execution. When + provided, the shell tool and a ``ShellEnvironmentProvider`` are automatically + added (provided the client supports shell tools; otherwise a warning is logged + and both are skipped). The object must expose ``as_function()`` and satisfy the + ``ShellExecutor`` protocol -- e.g. a ``LocalShellTool`` or ``DockerShellTool`` from + the ``agent-framework-tools`` package. The caller owns the executor's lifecycle. + shell_environment_provider_options: Optional ``ShellEnvironmentProviderOptions`` + (from ``agent-framework-tools``) used to customize the ``ShellEnvironmentProvider`` + environment probing and instructions. Only used when ``shell_executor`` is provided. disable_web_search: When True, skip automatic web search tool inclusion. When False (default), the web search tool is automatically added if the client implements SupportsWebSearchTool. A warning is logged if the client @@ -307,6 +354,13 @@ def create_harness_agent( tokenizer=tokenizer, ) + # Build the shell tool and environment provider (opt-in via shell_executor). + shell_tool, shell_provider = _assemble_shell( + client, + shell_executor, + shell_environment_provider_options, + ) + # Build context providers. assembled_providers = _assemble_context_providers( history_provider=resolved_history, @@ -321,6 +375,7 @@ def create_harness_agent( skills_paths=skills_paths, background_agents=background_agents, background_agents_instructions=background_agents_instructions, + shell_context_provider=shell_provider, extra_context_providers=context_providers, ) @@ -338,6 +393,8 @@ def create_harness_agent( "Set disable_web_search=True to suppress this warning.", type(client).__name__, ) + if shell_tool is not None: + assembled_tools.append(shell_tool) if tools is not None: if isinstance(tools, Sequence): assembled_tools.extend(tools) # pyright: ignore[reportUnknownArgumentType] diff --git a/python/packages/core/agent_framework/_skills.py b/python/packages/core/agent_framework/_skills.py index 97afe66cea9..91bdb619143 100644 --- a/python/packages/core/agent_framework/_skills.py +++ b/python/packages/core/agent_framework/_skills.py @@ -3516,9 +3516,7 @@ async def get_content(self) -> str: result = await self._client.read_resource(_mcp_any_url(self._skill_md_uri)) text = _mcp_join_text(result) if not text: - raise ValueError( - f"The MCP server returned no text content for SKILL.md resource '{self._skill_md_uri}'." - ) + raise ValueError(f"The MCP server returned no text content for SKILL.md resource '{self._skill_md_uri}'.") self._content = text return text @@ -3572,11 +3570,7 @@ def _validate_resource_name(name: str) -> str | None: or ``None`` if the name is unsafe. """ normalized = name.replace("\\", "/") - if ( - normalized.startswith("/") - or "://" in normalized - or any(seg == ".." for seg in normalized.split("/")) - ): + if normalized.startswith("/") or "://" in normalized or any(seg == ".." for seg in normalized.split("/")): logger.debug("Rejecting resource name with unsafe path components: %r", name) return None return normalized diff --git a/python/packages/core/tests/core/test_harness_agent.py b/python/packages/core/tests/core/test_harness_agent.py index 58ef3f5f2d0..1fa15ab3342 100644 --- a/python/packages/core/tests/core/test_harness_agent.py +++ b/python/packages/core/tests/core/test_harness_agent.py @@ -485,3 +485,95 @@ def test_create_harness_agent_empty_background_agents_list() -> None: ) providers = agent.context_providers or [] assert not any(isinstance(p, BackgroundAgentsProvider) for p in providers) + + +# --- Shell Tool Tests --- + + +class _FakeShellTool: + """Fake shell executor/tool exposing as_function().""" + + def as_function(self) -> str: + return "shell_fn" + + +class _FakeShellClient(_FakeChatClient): + """Fake client that supports the shell tool.""" + + def __init__(self) -> None: + self.shell_func: Any = None + + def get_shell_tool(self, *, func: Any = None, **kwargs: Any) -> str: + self.shell_func = func + return "shell_tool_instance" + + +def test_create_harness_agent_adds_shell_tool_and_provider() -> None: + """Shell tool and ShellEnvironmentProvider should be added when a shell executor is supplied.""" + from agent_framework_tools.shell import ShellEnvironmentProvider + + client = _FakeShellClient() + agent = create_harness_agent( + client=client, # type: ignore[arg-type] + max_context_window_tokens=128_000, + max_output_tokens=16_384, + disable_web_search=True, + shell_executor=_FakeShellTool(), + ) + tools = agent.default_options.get("tools", []) + assert "shell_tool_instance" in tools + assert client.shell_func == "shell_fn" + providers = agent.context_providers or [] + assert any(isinstance(p, ShellEnvironmentProvider) for p in providers) + + +def test_create_harness_agent_shell_passes_custom_options() -> None: + """Custom ShellEnvironmentProviderOptions should be forwarded to the provider.""" + from agent_framework_tools.shell import ShellEnvironmentProvider, ShellEnvironmentProviderOptions + + options = ShellEnvironmentProviderOptions(probe_tools=("git",)) + agent = create_harness_agent( + client=_FakeShellClient(), # type: ignore[arg-type] + max_context_window_tokens=128_000, + max_output_tokens=16_384, + disable_web_search=True, + shell_executor=_FakeShellTool(), + shell_environment_provider_options=options, + ) + providers = agent.context_providers or [] + provider = next(p for p in providers if isinstance(p, ShellEnvironmentProvider)) + assert provider._options is options + + +def test_create_harness_agent_shell_skipped_when_unsupported(caplog: pytest.LogCaptureFixture) -> None: + """When the client lacks get_shell_tool, both the tool and provider are skipped with a warning.""" + import logging + + from agent_framework_tools.shell import ShellEnvironmentProvider + + with caplog.at_level(logging.WARNING, logger="agent_framework._harness._agent"): + agent = create_harness_agent( + client=_FakeChatClient(), # type: ignore[arg-type] + max_context_window_tokens=128_000, + max_output_tokens=16_384, + disable_web_search=True, + shell_executor=_FakeShellTool(), + ) + assert any("SupportsShellTool" in msg for msg in caplog.messages) + providers = agent.context_providers or [] + assert not any(isinstance(p, ShellEnvironmentProvider) for p in providers) + assert "tools" not in agent.default_options or not agent.default_options.get("tools") + + +def test_create_harness_agent_no_shell_by_default() -> None: + """No shell tool or provider should be added when shell_executor is not provided.""" + from agent_framework_tools.shell import ShellEnvironmentProvider + + agent = create_harness_agent( + client=_FakeShellClient(), # type: ignore[arg-type] + max_context_window_tokens=128_000, + max_output_tokens=16_384, + disable_web_search=True, + ) + providers = agent.context_providers or [] + assert not any(isinstance(p, ShellEnvironmentProvider) for p in providers) diff --git a/python/packages/core/tests/core/test_mcp_observability.py b/python/packages/core/tests/core/test_mcp_observability.py index 226e9761205..8b17f3dc88c 100644 --- a/python/packages/core/tests/core/test_mcp_observability.py +++ b/python/packages/core/tests/core/test_mcp_observability.py @@ -281,9 +281,7 @@ async def test_mcp_prompts_get_creates_client_span(span_exporter: InMemorySpanEx async def test_mcp_prompts_get_mcp_error_sets_error_type(span_exporter: InMemorySpanExporter): """When session.get_prompt() raises McpError, the span should have error.type and ERROR status.""" tool = _make_connected_mcp_tool() - tool.session.get_prompt = AsyncMock( - side_effect=McpError(ErrorData(code=-32602, message="prompt not found")) - ) + tool.session.get_prompt = AsyncMock(side_effect=McpError(ErrorData(code=-32602, message="prompt not found"))) span_exporter.clear() with pytest.raises(ToolExecutionException): diff --git a/python/packages/core/tests/core/test_mcp_skills.py b/python/packages/core/tests/core/test_mcp_skills.py index 3e7c67662af..74993997d0e 100644 --- a/python/packages/core/tests/core/test_mcp_skills.py +++ b/python/packages/core/tests/core/test_mcp_skills.py @@ -35,26 +35,22 @@ Body content here. """ -SAMPLE_SKILL_INDEX = json.dumps( - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { - "name": "unit-converter", - "type": "skill-md", - "description": "Convert between common units.", - "url": "skill://unit-converter/SKILL.md", - } - ], - } -) +SAMPLE_SKILL_INDEX = json.dumps({ + "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", + "skills": [ + { + "name": "unit-converter", + "type": "skill-md", + "description": "Convert between common units.", + "url": "skill://unit-converter/SKILL.md", + } + ], +}) def _make_text_result(text: str, uri: str = "skill://test") -> ReadResourceResult: """Create a ReadResourceResult with a single TextResourceContents.""" - return ReadResourceResult( - contents=[TextResourceContents(uri=AnyUrl(uri), text=text, mimeType="text/markdown")] - ) + return ReadResourceResult(contents=[TextResourceContents(uri=AnyUrl(uri), text=text, mimeType="text/markdown")]) def _make_blob_result( @@ -230,12 +226,10 @@ async def test_get_content_raises_on_empty(self) -> None: @pytest.mark.asyncio async def test_get_resource_text(self) -> None: - client = _make_client( - **{ - "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), - "skill://unit-converter/references/checklist.md": _make_text_result("- check thing 1\n- check thing 2"), - } - ) + client = _make_client(**{ + "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), + "skill://unit-converter/references/checklist.md": _make_text_result("- check thing 1\n- check thing 2"), + }) from agent_framework import SkillFrontmatter fm = SkillFrontmatter(name="unit-converter", description="Convert between common units.") @@ -249,12 +243,10 @@ async def test_get_resource_text(self) -> None: @pytest.mark.asyncio async def test_get_resource_binary(self) -> None: data = bytes([0x01, 0x02, 0x03, 0x04]) - client = _make_client( - **{ - "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), - "skill://unit-converter/assets/icon.bin": _make_blob_result(data), - } - ) + client = _make_client(**{ + "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), + "skill://unit-converter/assets/icon.bin": _make_blob_result(data), + }) from agent_framework import SkillFrontmatter fm = SkillFrontmatter(name="unit-converter", description="Convert between common units.") @@ -345,12 +337,10 @@ class TestMCPSkillsSource: @pytest.mark.asyncio async def test_index_based_discovery_returns_skill(self) -> None: - client = _make_client( - **{ - "skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"), - "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), - } - ) + client = _make_client(**{ + "skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"), + "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), + }) source = MCPSkillsSource(client=client) skills = await source.get_skills() @@ -373,9 +363,7 @@ async def test_no_index_returns_empty(self) -> None: async def test_does_not_read_skill_md_during_discovery(self) -> None: # Index points to a skill, but SKILL.md is not registered on the server. # Discovery should succeed because it only reads the index. - client = _make_client( - **{"skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json")} - ) + client = _make_client(**{"skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json")}) source = MCPSkillsSource(client=client) skills = await source.get_skills() @@ -384,19 +372,17 @@ async def test_does_not_read_skill_md_during_discovery(self) -> None: @pytest.mark.asyncio async def test_invalid_name_is_skipped(self) -> None: - index_json = json.dumps( - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { - "name": "UnitConverter", # Invalid: uppercase - "type": "skill-md", - "description": "Convert between common units.", - "url": "skill://UnitConverter/SKILL.md", - } - ], - } - ) + index_json = json.dumps({ + "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", + "skills": [ + { + "name": "UnitConverter", # Invalid: uppercase + "type": "skill-md", + "description": "Convert between common units.", + "url": "skill://UnitConverter/SKILL.md", + } + ], + }) client = _make_client(**{"skill://index.json": _make_text_result(index_json, uri="skill://index.json")}) source = MCPSkillsSource(client=client) skills = await source.get_skills() @@ -404,18 +390,16 @@ async def test_invalid_name_is_skipped(self) -> None: @pytest.mark.asyncio async def test_missing_required_fields_is_skipped(self) -> None: - index_json = json.dumps( - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { - "name": "unit-converter", - "type": "skill-md", - # Missing description and url - } - ], - } - ) + index_json = json.dumps({ + "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", + "skills": [ + { + "name": "unit-converter", + "type": "skill-md", + # Missing description and url + } + ], + }) client = _make_client(**{"skill://index.json": _make_text_result(index_json, uri="skill://index.json")}) source = MCPSkillsSource(client=client) skills = await source.get_skills() @@ -423,19 +407,17 @@ async def test_missing_required_fields_is_skipped(self) -> None: @pytest.mark.asyncio async def test_unsupported_type_is_skipped(self) -> None: - index_json = json.dumps( - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { - "name": "some-skill", - "type": "archive", - "description": "Packaged skill.", - "url": "skill://some-skill.tar.gz", - } - ], - } - ) + index_json = json.dumps({ + "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", + "skills": [ + { + "name": "some-skill", + "type": "archive", + "description": "Packaged skill.", + "url": "skill://some-skill.tar.gz", + } + ], + }) client = _make_client(**{"skill://index.json": _make_text_result(index_json, uri="skill://index.json")}) source = MCPSkillsSource(client=client) skills = await source.get_skills() @@ -443,18 +425,16 @@ async def test_unsupported_type_is_skipped(self) -> None: @pytest.mark.asyncio async def test_template_type_is_skipped(self) -> None: - index_json = json.dumps( - { - "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", - "skills": [ - { - "type": "mcp-resource-template", - "description": "Per-product documentation skill", - "url": "skill://docs/{product}/SKILL.md", - } - ], - } - ) + index_json = json.dumps({ + "$schema": "https://schemas.agentskills.io/discovery/0.2.0/schema.json", + "skills": [ + { + "type": "mcp-resource-template", + "description": "Per-product documentation skill", + "url": "skill://docs/{product}/SKILL.md", + } + ], + }) client = _make_client(**{"skill://index.json": _make_text_result(index_json, uri="skill://index.json")}) source = MCPSkillsSource(client=client) skills = await source.get_skills() @@ -462,31 +442,25 @@ async def test_template_type_is_skipped(self) -> None: @pytest.mark.asyncio async def test_empty_index_returns_empty(self) -> None: - client = _make_client( - **{"skill://index.json": _make_text_result('{"skills": []}', uri="skill://index.json")} - ) + client = _make_client(**{"skill://index.json": _make_text_result('{"skills": []}', uri="skill://index.json")}) source = MCPSkillsSource(client=client) skills = await source.get_skills() assert skills == [] @pytest.mark.asyncio async def test_malformed_index_json_returns_empty(self) -> None: - client = _make_client( - **{"skill://index.json": _make_text_result("not valid json", uri="skill://index.json")} - ) + client = _make_client(**{"skill://index.json": _make_text_result("not valid json", uri="skill://index.json")}) source = MCPSkillsSource(client=client) skills = await source.get_skills() assert skills == [] @pytest.mark.asyncio async def test_sibling_text_resource(self) -> None: - client = _make_client( - **{ - "skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"), - "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), - "skill://unit-converter/references/checklist.md": _make_text_result("- check thing 1\n- check thing 2"), - } - ) + client = _make_client(**{ + "skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"), + "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), + "skill://unit-converter/references/checklist.md": _make_text_result("- check thing 1\n- check thing 2"), + }) source = MCPSkillsSource(client=client) skill = (await source.get_skills())[0] resource = await skill.get_resource("references/checklist.md") @@ -497,13 +471,11 @@ async def test_sibling_text_resource(self) -> None: @pytest.mark.asyncio async def test_sibling_binary_resource(self) -> None: data = bytes([0x01, 0x02, 0x03, 0x04]) - client = _make_client( - **{ - "skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"), - "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), - "skill://unit-converter/assets/icon.bin": _make_blob_result(data), - } - ) + client = _make_client(**{ + "skill://index.json": _make_text_result(SAMPLE_SKILL_INDEX, uri="skill://index.json"), + "skill://unit-converter/SKILL.md": _make_text_result(SAMPLE_SKILL_MD), + "skill://unit-converter/assets/icon.bin": _make_blob_result(data), + }) source = MCPSkillsSource(client=client) skill = (await source.get_skills())[0] resource = await skill.get_resource("assets/icon.bin") @@ -649,9 +621,7 @@ async def test_get_resource_generic_mcp_error_propagates(self) -> None: from agent_framework import SkillFrontmatter client = AsyncMock() - client.read_resource = AsyncMock( - side_effect=McpError(error=ErrorData(code=0, message="Handler error")) - ) + client.read_resource = AsyncMock(side_effect=McpError(error=ErrorData(code=0, message="Handler error"))) fm = SkillFrontmatter(name="test-skill", description="Test.") skill = MCPSkill(frontmatter=fm, skill_md_uri="skill://test/SKILL.md", client=client) with pytest.raises(McpError): diff --git a/python/samples/02-agents/harness/README.md b/python/samples/02-agents/harness/README.md index 3bf0f091102..835ccf6e85f 100644 --- a/python/samples/02-agents/harness/README.md +++ b/python/samples/02-agents/harness/README.md @@ -17,6 +17,7 @@ from a chat client. | AgentModeProvider | Plan/execute mode tracking | | MemoryContextProvider | File-based durable memory (when `memory_store` provided) | | SkillsProvider | File-based skill discovery and progressive loading | +| Shell tool | Shell command execution + environment probing (when `shell_executor` provided) | | OpenTelemetry | Built-in observability | Each feature can be disabled or customized via keyword arguments. @@ -81,3 +82,25 @@ agent = create_harness_agent( The `AgentModeProvider` enables a two-phase workflow: 1. **Plan mode** — Interactive: the agent asks questions, creates todos, gets approval 2. **Execute mode** — Autonomous: the agent works through todos independently + +### Shell Tool + +Pass a shell executor (e.g. `LocalShellTool` from `agent-framework-tools`) to enable shell +command execution plus automatic environment probing via a `ShellEnvironmentProvider`. The +tool is only wired when the chat client supports shell tools; otherwise a warning is logged +and the shell tool/provider are skipped. The caller owns the executor's lifecycle. + +```python +from agent_framework_tools.shell import LocalShellTool, ShellEnvironmentProviderOptions + +async with LocalShellTool(acknowledge_unsafe=True) as shell: + agent = create_harness_agent( + client=client, + max_context_window_tokens=128_000, + max_output_tokens=16_384, + shell_executor=shell, + # Optional: customize environment probing. + shell_environment_provider_options=ShellEnvironmentProviderOptions(probe_tools=("git", "python")), + ) +``` + From 851b32fe737b7ac9d9b08a9a4ec4db5457e458f7 Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Wed, 10 Jun 2026 13:56:02 +0000 Subject: [PATCH 2/3] Validate shell_executor exposes as_function() with a clear TypeError Addresses PR review feedback: a public factory should fail fast with an actionable error rather than a cryptic AttributeError when an incompatible shell_executor is supplied. Validation happens upfront, regardless of whether the client supports shell tools. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../core/agent_framework/_harness/_agent.py | 10 ++++++ .../core/tests/core/test_harness_agent.py | 32 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/python/packages/core/agent_framework/_harness/_agent.py b/python/packages/core/agent_framework/_harness/_agent.py index 987f617c37f..2ae64d641cf 100644 --- a/python/packages/core/agent_framework/_harness/_agent.py +++ b/python/packages/core/agent_framework/_harness/_agent.py @@ -159,10 +159,20 @@ def _assemble_shell( Returns a ``(tool, provider)`` tuple. Both are ``None`` when no shell executor is provided, or when the client does not support shell tools (a warning is logged in the latter case, since the environment provider is not useful without an execution path). + + Raises: + TypeError: If ``shell_executor`` does not expose a callable ``as_function()`` method. """ if shell_executor is None: return None, None + if not callable(getattr(shell_executor, "as_function", None)): + raise TypeError( + f"shell_executor must expose a callable 'as_function()' method " + f"(e.g. a LocalShellTool or DockerShellTool from agent-framework-tools), " + f"but got {type(shell_executor).__name__}." + ) + if not isinstance(client, SupportsShellTool): logger.warning( "Shell tool not available: client %r does not implement SupportsShellTool. " diff --git a/python/packages/core/tests/core/test_harness_agent.py b/python/packages/core/tests/core/test_harness_agent.py index 1fa15ab3342..e3598268e23 100644 --- a/python/packages/core/tests/core/test_harness_agent.py +++ b/python/packages/core/tests/core/test_harness_agent.py @@ -577,3 +577,35 @@ def test_create_harness_agent_no_shell_by_default() -> None: ) providers = agent.context_providers or [] assert not any(isinstance(p, ShellEnvironmentProvider) for p in providers) + + +def test_create_harness_agent_shell_executor_without_as_function_raises() -> None: + """A shell_executor lacking a callable as_function() should raise a clear TypeError.""" + + class _BadExecutor: + pass + + with pytest.raises(TypeError, match="as_function"): + create_harness_agent( + client=_FakeShellClient(), # type: ignore[arg-type] + max_context_window_tokens=128_000, + max_output_tokens=16_384, + disable_web_search=True, + shell_executor=_BadExecutor(), + ) + + +def test_create_harness_agent_shell_executor_validated_before_client_check() -> None: + """The as_function() contract is validated upfront, even when the client lacks shell support.""" + + class _BadExecutor: + pass + + with pytest.raises(TypeError, match="as_function"): + create_harness_agent( + client=_FakeChatClient(), # type: ignore[arg-type] + max_context_window_tokens=128_000, + max_output_tokens=16_384, + disable_web_search=True, + shell_executor=_BadExecutor(), + ) From b12823b50e66a337507371c403aca6e64911cfe9 Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Thu, 11 Jun 2026 18:03:07 +0000 Subject: [PATCH 3/3] Type shell harness params via TYPE_CHECKING import Addresses PR review feedback: type shell_executor and shell_environment_provider_options instead of Any, using a TYPE_CHECKING import from agent_framework_tools.shell. The import never executes at runtime, so there is no circular dependency, and the lazy runtime import of ShellEnvironmentProvider is retained. Since ShellExecutor is a protocol without as_function(), the validated getattr result is invoked directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../core/agent_framework/_harness/_agent.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/python/packages/core/agent_framework/_harness/_agent.py b/python/packages/core/agent_framework/_harness/_agent.py index 2ae64d641cf..295a43443ce 100644 --- a/python/packages/core/agent_framework/_harness/_agent.py +++ b/python/packages/core/agent_framework/_harness/_agent.py @@ -28,6 +28,8 @@ if TYPE_CHECKING: from collections.abc import Mapping + from agent_framework_tools.shell import ShellEnvironmentProviderOptions, ShellExecutor + from .._clients import SupportsChatGetResponse from .._compaction import CompactionStrategy, TokenizerProtocol from .._middleware import MiddlewareTypes @@ -151,8 +153,8 @@ def _assemble_context_providers( def _assemble_shell( client: SupportsChatGetResponse[Any], - shell_executor: Any, - shell_environment_provider_options: Any, + shell_executor: ShellExecutor | None, + shell_environment_provider_options: ShellEnvironmentProviderOptions | None, ) -> tuple[ToolTypes | None, ContextProvider | None]: """Build the shell tool and environment provider when a shell executor is supplied. @@ -166,7 +168,10 @@ def _assemble_shell( if shell_executor is None: return None, None - if not callable(getattr(shell_executor, "as_function", None)): + # ShellExecutor is a protocol without ``as_function()``, so the + # contract is validated at runtime: a shell tool such as LocalShellTool/DockerShellTool exposes it. + as_function = getattr(shell_executor, "as_function", None) + if not callable(as_function): raise TypeError( f"shell_executor must expose a callable 'as_function()' method " f"(e.g. a LocalShellTool or DockerShellTool from agent-framework-tools), " @@ -185,7 +190,7 @@ def _assemble_shell( # which depends on core, so core cannot import them at module load time. from agent_framework_tools.shell import ShellEnvironmentProvider - shell_tool = client.get_shell_tool(func=shell_executor.as_function()) + shell_tool = client.get_shell_tool(func=as_function()) shell_provider = ShellEnvironmentProvider(shell_executor, shell_environment_provider_options) return shell_tool, shell_provider @@ -220,8 +225,8 @@ def create_harness_agent( skills_paths: Sequence[str] | None = None, background_agents: Sequence[SupportsAgentRun] | None = None, background_agents_instructions: str | None = None, - shell_executor: Any = None, - shell_environment_provider_options: Any = None, + shell_executor: ShellExecutor | None = None, + shell_environment_provider_options: ShellEnvironmentProviderOptions | None = None, disable_web_search: bool = False, otel_provider_name: str | None = None, context_providers: Sequence[ContextProvider] | None = None,