fix: address audit findings — SDK plumbing, TUI bus, dead code
Critical fixes: - ``StrixOrchestrationHooks.on_agent_start`` now finds the ``CaidoCapability`` via ``ctx.context['caido_capability']`` instead of ``agent.capabilities`` (we use plain ``Agent``, not ``SandboxAgent``, so the latter never existed). The session manager's bundle already exposes the capability; ``run_strix_scan`` threads it through ``make_agent_context`` and ``create_agent`` forwards it to children. - ``run_strix_scan`` registers the ``StrixTracingProcessor`` with the SDK's tracing provider via ``add_trace_processor`` so SDK trace spans hit ``run_dir/events.jsonl`` (was previously a parallel stream the SDK ignored). - ``on_llm_end`` now writes to ``Tracer.record_llm_usage`` in addition to ``bus.record_usage`` so the CLI/TUI stats panel sees real numbers instead of zeros. - ``run_strix_scan`` accepts an externally-built ``AgentMessageBus`` + an explicit ``model`` arg. The TUI pre-creates the bus so its stop and chat-input handlers can submit ``bus.send`` / ``bus.cancel_descendants`` coroutines onto the scan thread's loop via ``asyncio.run_coroutine_threadsafe`` — replacing the TODO-stub no-ops. - ``model`` config now propagates root → context → child agents in ``create_agent`` (was hardcoded fallback). Dead-code removal: - Deleted the ``load_skill`` tool entirely (host module, sandbox module, TUI renderer, tests). The legacy implementation reached into a global ``_agent_instances`` registry that no longer exists; the post-migration stub returned ``success=True`` without injecting anything — pure theater. Skills are still preloaded via the system prompt at scan-bring-up. - Dropped ``tenacity`` and ``xmltodict`` from ``[project.dependencies]`` — neither is imported anywhere post-migration. - Stripped the system prompt's "use the load_skill tool" lines. Tests: 278/278 passing. Removed two ``load_skill`` test cases and a ``test_tool_registration_modes::test_load_skill_import_...`` assertion that exercised the deleted module. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
0xallam
@@ -1,12 +1,7 @@
|
||||
"""Phase 2.4 smoke tests for the remaining local SDK tool wrappers.
|
||||
"""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``.
|
||||
search_files), reporting (create_vulnerability_report), finish_scan.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -25,7 +20,6 @@ from strix.tools.file_edit.tools import (
|
||||
str_replace_editor,
|
||||
)
|
||||
from strix.tools.finish.tool import finish_scan
|
||||
from strix.tools.load_skill.tool import load_skill
|
||||
from strix.tools.reporting.tool import create_vulnerability_report
|
||||
from strix.tools.web_search.tool import web_search
|
||||
|
||||
@@ -69,7 +63,6 @@ def test_all_remaining_tools_are_function_tools() -> None:
|
||||
list_files,
|
||||
search_files,
|
||||
create_vulnerability_report,
|
||||
load_skill,
|
||||
finish_scan,
|
||||
):
|
||||
assert isinstance(tool, FunctionTool)
|
||||
@@ -240,38 +233,6 @@ async def test_create_vulnerability_report_delegates_to_impl() -> 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.tool._impl.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 ---------------------------------------------------------
|
||||
|
||||
|
||||
|
||||
@@ -58,45 +58,5 @@ def test_sandbox_registers_sandbox_tools_but_not_non_sandbox_tools(
|
||||
assert "list_requests" in names
|
||||
assert "create_agent" not in names
|
||||
assert "finish_scan" not in names
|
||||
assert "load_skill" not in names
|
||||
assert "browser_action" not in names
|
||||
assert "web_search" not in names
|
||||
|
||||
|
||||
def test_load_skill_import_does_not_register_create_agent_in_sandbox(
|
||||
monkeypatch: Any,
|
||||
) -> None:
|
||||
monkeypatch.setenv("STRIX_SANDBOX_MODE", "true")
|
||||
monkeypatch.setenv("STRIX_DISABLE_BROWSER", "true")
|
||||
monkeypatch.delenv("PERPLEXITY_API_KEY", raising=False)
|
||||
monkeypatch.setattr(Config, "load", classmethod(_empty_config_load))
|
||||
|
||||
clear_registry()
|
||||
for name in list(sys.modules):
|
||||
if name == "strix.tools" or name.startswith("strix.tools."):
|
||||
sys.modules.pop(name, None)
|
||||
|
||||
load_skill_module = importlib.import_module("strix.tools.load_skill.load_skill_actions")
|
||||
registry = importlib.import_module("strix.tools.registry")
|
||||
|
||||
names_before = set(registry.get_tool_names())
|
||||
assert "load_skill" not in names_before
|
||||
assert "create_agent" not in names_before
|
||||
|
||||
state_type = type(
|
||||
"DummyState",
|
||||
(),
|
||||
{
|
||||
"agent_id": "agent_test",
|
||||
"context": {},
|
||||
"update_context": lambda self, key, value: self.context.__setitem__(key, value),
|
||||
},
|
||||
)
|
||||
result = load_skill_module.load_skill(state_type(), "nmap")
|
||||
|
||||
names_after = set(registry.get_tool_names())
|
||||
assert "create_agent" not in names_after
|
||||
# load_skill no longer reaches into the legacy _agent_instances global —
|
||||
# it returns ``success=True`` with the requested skills echoed back.
|
||||
assert result["success"] is True
|
||||
assert result["loaded_skills"] == ["nmap"]
|
||||
|
||||
Reference in New Issue
Block a user