mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: Integrate shell tool into harness agent (#6451)
* Integrate shell tool into AgentHarness * 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> * 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> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -543,3 +543,127 @@ 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)
|
||||
|
||||
|
||||
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(),
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user