feat(migration): phase 2.4 — wrap remaining local SDK tools
Five tool families ported to SDK function tools using the proven delegation pattern from Phase 2.3: - web_search (1 tool): asyncio.to_thread around the synchronous Perplexity request so the 300s API call doesn't block the SDK event loop. - file_edit (3 tools — str_replace_editor, list_files, search_files): these run *inside* the sandbox container in the legacy harness (sandbox_execution=True), so the SDK wrappers route through post_to_sandbox rather than importing the legacy module on the host (which pulls in openhands_aci, a sandbox-only dependency). - reporting (1 tool — create_vulnerability_report): asyncio.to_thread around the legacy function, which itself runs CVSS XML parsing, LLM-based dedup against existing findings, and tracer persistence. - load_skill (1 tool): legacy adapter passes ctx.context['agent_id'] through. The legacy implementation reaches into _agent_instances, a global Phase 3 will replace; until then the call degrades to a structured error rather than crashing. - finish_scan (1 tool): legacy adapter pattern. Validates non-empty fields, checks no other agents are still active (via legacy _agent_graph), persists the four executive sections through the global tracer. Tests: 12 new tests in test_sdk_remaining_local_tools.py — registration checks, web_search delegation + missing-key path, file_edit dispatch shape verification, vuln-report validation + delegation, load_skill adapter passthrough, finish_scan validation + delegation. The two finish_scan tests use a fixture that snapshots/clears the legacy _agent_graph['nodes'] dict so cross-test pollution from legacy multi-agent tests doesn't mask the validation path. Per-file ruff TC002 ignores added for the five new wrapper modules (same reason as Phase 2.3 — RunContextWrapper must be runtime-importable for SDK function_schema().get_type_hints()). Refs: PLAYBOOK.md §3.5. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
0xallam
@@ -0,0 +1,341 @@
|
||||
"""Phase 2.4 smoke tests for the remaining local SDK tool wrappers.
|
||||
|
||||
Covers: web_search, file_edit (str_replace_editor + list_files +
|
||||
search_files), reporting (create_vulnerability_report), load_skill,
|
||||
finish_scan.
|
||||
|
||||
Pattern matches test_sdk_local_tools.py — confirm registration as
|
||||
``FunctionTool``, exercise the legacy delegation path, verify that
|
||||
sandbox-bound tools route through ``post_to_sandbox``.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from collections.abc import Iterator
|
||||
from dataclasses import dataclass, field
|
||||
from typing import Any
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
from agents.tool import FunctionTool
|
||||
|
||||
from strix.tools.file_edit.file_edit_sdk_tools import (
|
||||
list_files,
|
||||
search_files,
|
||||
str_replace_editor,
|
||||
)
|
||||
from strix.tools.finish.finish_sdk_tool import finish_scan
|
||||
from strix.tools.load_skill.load_skill_sdk_tool import load_skill
|
||||
from strix.tools.reporting.reporting_sdk_tools import create_vulnerability_report
|
||||
from strix.tools.web_search.web_search_sdk_tool import web_search
|
||||
|
||||
|
||||
@dataclass
|
||||
class _Ctx:
|
||||
context: dict[str, Any] = field(default_factory=dict)
|
||||
|
||||
|
||||
def _ctx_for(agent_id: str = "test-agent") -> _Ctx:
|
||||
return _Ctx(
|
||||
context={
|
||||
"agent_id": agent_id,
|
||||
"tool_server_host_port": 12345,
|
||||
"sandbox_token": "test-token",
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
async def _invoke(tool: FunctionTool, ctx: _Ctx, **kwargs: Any) -> dict[str, Any]:
|
||||
from agents.tool_context import ToolContext
|
||||
|
||||
tool_ctx = ToolContext(
|
||||
context=ctx.context,
|
||||
usage=None,
|
||||
tool_name=tool.name,
|
||||
tool_call_id="test-call-id",
|
||||
tool_arguments=json.dumps(kwargs),
|
||||
)
|
||||
result = await tool.on_invoke_tool(tool_ctx, json.dumps(kwargs))
|
||||
assert isinstance(result, str)
|
||||
decoded = json.loads(result)
|
||||
assert isinstance(decoded, dict)
|
||||
return decoded
|
||||
|
||||
|
||||
def test_all_remaining_tools_are_function_tools() -> None:
|
||||
for tool in (
|
||||
web_search,
|
||||
str_replace_editor,
|
||||
list_files,
|
||||
search_files,
|
||||
create_vulnerability_report,
|
||||
load_skill,
|
||||
finish_scan,
|
||||
):
|
||||
assert isinstance(tool, FunctionTool)
|
||||
|
||||
|
||||
# --- web_search -----------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_web_search_no_api_key_returns_structured_error(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""The legacy function returns a structured error when the env var is
|
||||
missing — verify the wrapper passes that through verbatim."""
|
||||
monkeypatch.delenv("PERPLEXITY_API_KEY", raising=False)
|
||||
|
||||
out = await _invoke(web_search, _ctx_for(), query="anything")
|
||||
assert out["success"] is False
|
||||
assert "PERPLEXITY_API_KEY" in out["message"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_web_search_delegates_to_legacy(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Legacy ``web_search`` returns dict; wrapper JSON-encodes it."""
|
||||
monkeypatch.setenv("PERPLEXITY_API_KEY", "fake-key")
|
||||
|
||||
fake_result = {
|
||||
"success": True,
|
||||
"query": "xss techniques",
|
||||
"content": "Reflected XSS payload examples...",
|
||||
"message": "Web search completed successfully",
|
||||
}
|
||||
with patch(
|
||||
"strix.tools.web_search.web_search_sdk_tool._legacy.web_search",
|
||||
return_value=fake_result,
|
||||
) as legacy:
|
||||
out = await _invoke(web_search, _ctx_for(), query="xss techniques")
|
||||
|
||||
assert out == fake_result
|
||||
legacy.assert_called_once_with(query="xss techniques")
|
||||
|
||||
|
||||
# --- file_edit (sandbox-bound) -------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_str_replace_editor_routes_to_sandbox() -> None:
|
||||
"""file_edit tools must POST to the in-sandbox tool server, not run locally."""
|
||||
fake_response = {"result": {"content": "file viewed"}}
|
||||
with patch(
|
||||
"strix.tools.file_edit.file_edit_sdk_tools.post_to_sandbox",
|
||||
return_value=fake_response,
|
||||
) as dispatch:
|
||||
out = await _invoke(
|
||||
str_replace_editor,
|
||||
_ctx_for(),
|
||||
command="view",
|
||||
path="src/foo.py",
|
||||
)
|
||||
|
||||
assert out == fake_response
|
||||
assert dispatch.call_count == 1
|
||||
# post_to_sandbox is called positionally as (ctx, tool_name, kwargs).
|
||||
args, _ = dispatch.call_args
|
||||
assert args[1] == "str_replace_editor"
|
||||
assert args[2]["command"] == "view"
|
||||
assert args[2]["path"] == "src/foo.py"
|
||||
# All optional file-edit params are forwarded as None (parity with legacy schema).
|
||||
assert args[2]["file_text"] is None
|
||||
assert args[2]["old_str"] is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_list_files_routes_to_sandbox() -> None:
|
||||
fake_response = {"result": {"files": ["a.py"], "directories": []}}
|
||||
with patch(
|
||||
"strix.tools.file_edit.file_edit_sdk_tools.post_to_sandbox",
|
||||
return_value=fake_response,
|
||||
) as dispatch:
|
||||
out = await _invoke(list_files, _ctx_for(), path="src", recursive=True)
|
||||
|
||||
assert out == fake_response
|
||||
args, _ = dispatch.call_args
|
||||
assert args[1] == "list_files"
|
||||
assert args[2] == {"path": "src", "recursive": True}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_search_files_routes_to_sandbox() -> None:
|
||||
fake_response = {"result": {"output": "src/foo.py:1:match"}}
|
||||
with patch(
|
||||
"strix.tools.file_edit.file_edit_sdk_tools.post_to_sandbox",
|
||||
return_value=fake_response,
|
||||
) as dispatch:
|
||||
out = await _invoke(
|
||||
search_files,
|
||||
_ctx_for(),
|
||||
path="src",
|
||||
regex="TODO",
|
||||
file_pattern="*.py",
|
||||
)
|
||||
|
||||
assert out == fake_response
|
||||
args, _ = dispatch.call_args
|
||||
assert args[1] == "search_files"
|
||||
assert args[2] == {"path": "src", "regex": "TODO", "file_pattern": "*.py"}
|
||||
|
||||
|
||||
# --- reporting -----------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_create_vulnerability_report_validates_required_fields() -> None:
|
||||
"""Empty required fields should be rejected by the legacy validator."""
|
||||
out = await _invoke(
|
||||
create_vulnerability_report,
|
||||
_ctx_for(),
|
||||
title="", # empty -> validation error
|
||||
description="d",
|
||||
impact="i",
|
||||
target="t",
|
||||
technical_analysis="ta",
|
||||
poc_description="pd",
|
||||
poc_script_code="curl ...",
|
||||
remediation_steps="rs",
|
||||
cvss_breakdown="<attack_vector>N</attack_vector>",
|
||||
)
|
||||
assert out["success"] is False
|
||||
assert "errors" in out
|
||||
assert any("Title" in e for e in out["errors"])
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_create_vulnerability_report_delegates_to_legacy() -> None:
|
||||
"""Verify the wrapper passes all params through to the legacy function."""
|
||||
fake_result = {
|
||||
"success": True,
|
||||
"message": "Vulnerability report 'X' created successfully",
|
||||
"report_id": "abc123",
|
||||
"severity": "high",
|
||||
"cvss_score": 7.5,
|
||||
}
|
||||
with patch(
|
||||
"strix.tools.reporting.reporting_sdk_tools._legacy.create_vulnerability_report",
|
||||
return_value=fake_result,
|
||||
) as legacy:
|
||||
out = await _invoke(
|
||||
create_vulnerability_report,
|
||||
_ctx_for(),
|
||||
title="t",
|
||||
description="d",
|
||||
impact="i",
|
||||
target="tg",
|
||||
technical_analysis="ta",
|
||||
poc_description="pd",
|
||||
poc_script_code="pc",
|
||||
remediation_steps="rs",
|
||||
cvss_breakdown="<x/>",
|
||||
cve="CVE-2024-12345",
|
||||
)
|
||||
|
||||
assert out == fake_result
|
||||
kwargs = legacy.call_args.kwargs
|
||||
assert kwargs["title"] == "t"
|
||||
assert kwargs["cve"] == "CVE-2024-12345"
|
||||
# Optional params we didn't pass should still be forwarded as None.
|
||||
assert kwargs["endpoint"] is None
|
||||
assert kwargs["method"] is None
|
||||
|
||||
|
||||
# --- load_skill ----------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_load_skill_passes_adapter_with_agent_id() -> None:
|
||||
"""The wrapper must build the legacy-shaped adapter from ctx.context."""
|
||||
captured: dict[str, Any] = {}
|
||||
|
||||
def fake_legacy(*, agent_state: Any, skills: str) -> dict[str, Any]:
|
||||
captured["agent_id"] = agent_state.agent_id
|
||||
captured["skills"] = skills
|
||||
return {"success": True, "loaded_skills": ["recon"]}
|
||||
|
||||
with patch(
|
||||
"strix.tools.load_skill.load_skill_sdk_tool._legacy.load_skill",
|
||||
side_effect=fake_legacy,
|
||||
):
|
||||
out = await _invoke(load_skill, _ctx_for("agent-XYZ"), skills="recon")
|
||||
|
||||
assert out["success"] is True
|
||||
assert captured["agent_id"] == "agent-XYZ"
|
||||
assert captured["skills"] == "recon"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_load_skill_with_empty_input() -> None:
|
||||
"""End-to-end: empty skills string yields the legacy validation error."""
|
||||
out = await _invoke(load_skill, _ctx_for(), skills="")
|
||||
assert out["success"] is False
|
||||
assert "No skills" in out["error"]
|
||||
|
||||
|
||||
# --- finish_scan ---------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def isolated_agent_graph() -> Iterator[None]:
|
||||
"""Clear the legacy agent-graph globals so finish_scan sees an empty world.
|
||||
|
||||
The legacy ``_check_active_agents`` reads ``_agent_graph["nodes"]`` and
|
||||
returns an "agents still active" error if any non-self agent is in
|
||||
state ``running`` or ``stopping``. Tests in other modules (legacy
|
||||
multi-agent tests) populate this dict; without isolation they bleed
|
||||
into our validation tests and mask the field-validation path.
|
||||
"""
|
||||
from strix.tools.agents_graph import agents_graph_actions
|
||||
|
||||
saved_nodes = agents_graph_actions._agent_graph.get("nodes", {}).copy()
|
||||
agents_graph_actions._agent_graph["nodes"] = {}
|
||||
try:
|
||||
yield
|
||||
finally:
|
||||
agents_graph_actions._agent_graph["nodes"] = saved_nodes
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.usefixtures("isolated_agent_graph")
|
||||
async def test_finish_scan_validates_empty_fields() -> None:
|
||||
"""Legacy validation: every section must be non-empty."""
|
||||
out = await _invoke(
|
||||
finish_scan,
|
||||
_ctx_for(),
|
||||
executive_summary="",
|
||||
methodology="m",
|
||||
technical_analysis="ta",
|
||||
recommendations="r",
|
||||
)
|
||||
assert out["success"] is False
|
||||
assert any("Executive summary" in e for e in out["errors"])
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.usefixtures("isolated_agent_graph")
|
||||
async def test_finish_scan_delegates_to_legacy() -> None:
|
||||
"""Wrapper must pass the legacy adapter and the four sections through."""
|
||||
fake_result = {
|
||||
"success": True,
|
||||
"scan_completed": True,
|
||||
"message": "Scan completed successfully",
|
||||
"vulnerabilities_found": 3,
|
||||
}
|
||||
with patch(
|
||||
"strix.tools.finish.finish_sdk_tool._legacy.finish_scan",
|
||||
return_value=fake_result,
|
||||
) as legacy:
|
||||
out = await _invoke(
|
||||
finish_scan,
|
||||
_ctx_for("root-agent"),
|
||||
executive_summary="es",
|
||||
methodology="m",
|
||||
technical_analysis="ta",
|
||||
recommendations="r",
|
||||
)
|
||||
|
||||
assert out == fake_result
|
||||
kwargs = legacy.call_args.kwargs
|
||||
assert kwargs["executive_summary"] == "es"
|
||||
assert kwargs["agent_state"].agent_id == "root-agent"
|
||||
Reference in New Issue
Block a user