feat(migration): phase 2.1-2.3 — sandbox dispatch + thin slice tool wrappers

Phase 2.1 — sandbox dispatch helper:
- strix/tools/_sandbox_dispatch.py: post_to_sandbox() centralizes the
  host->container HTTP wire format. Connect=10s, read=150s timeouts mirror
  legacy executor.py. 50 MB response cap (C18) prevents OOM from a runaway
  tool. All errors surface as {"error": str} so the model can recover
  instead of the run dying.

Phase 2.2 — C6 lock-protected JSONL writes:
- strix/tools/notes/notes_actions.py: notes.jsonl appends are now wrapped
  in _notes_lock so concurrent agents can't interleave half-written lines.
  Regression test in test_notes_jsonl_concurrency.py verifies 1000 parallel
  writes produce exactly 1000 valid JSON lines.

Phase 2.3 — thin-slice SDK wrappers (think + todo + notes):
- strix/tools/_legacy_adapter.py: LegacyAgentStateAdapter shim — exposes
  just enough surface (.agent_id) for legacy tools that close over
  agent_state, sourced from ctx.context['agent_id'].
- strix/tools/thinking/thinking_sdk_tools.py: 1 tool (think).
- strix/tools/todo/todo_sdk_tools.py: 6 tools (create/list/update/done/
  pending/delete) with bulk-form preserved.
- strix/tools/notes/notes_sdk_tools.py: 5 tools (create/list/get/update/
  delete) with asyncio.to_thread around the lock-protected file I/O.

Tests: 22 new tests pass (10 sandbox dispatch + 2 concurrency + 10 SDK
local). Full suite still green.

Per-file ruff ignores added for SDK wrapper files: TC002 (RunContextWrapper
must be runtime-importable because the SDK calls get_type_hints() to
derive the JSON schema) and PLR0911 (sandbox dispatch's 10 short-circuit
returns are intentional, each a distinct documented failure mode).

Refs: PLAYBOOK.md §3.4, AUDIT_R3.md C6/C18.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
0xallam
2026-04-25 00:13:34 -07:00
parent 375389b8bc
commit 6e5d96af34
10 changed files with 1036 additions and 1 deletions
@@ -0,0 +1,79 @@
"""C6 regression test — concurrent notes JSONL writes must produce valid JSONL.
This test would fail before the C6 fix (AUDIT_R2 §1.1, applied in Phase 2.2):
the legacy ``_append_note_event`` opened the file and called ``f.write``
without holding ``_notes_lock``, so two threads writing simultaneously
could interleave bytes mid-line and corrupt the JSONL.
"""
from __future__ import annotations
import json
import threading
from collections.abc import Iterator
from pathlib import Path
from typing import Any
from unittest.mock import patch
import pytest
from strix.tools.notes.notes_actions import _append_note_event
@pytest.fixture
def notes_path(tmp_path: Path) -> Iterator[Path]:
"""Point ``_get_notes_jsonl_path`` at a tmp file for the test."""
target = tmp_path / "notes" / "notes.jsonl"
target.parent.mkdir(parents=True, exist_ok=True)
with patch(
"strix.tools.notes.notes_actions._get_notes_jsonl_path",
return_value=target,
):
yield target
def test_concurrent_note_writes_yield_valid_jsonl(notes_path: Path) -> None:
"""C6 fix: 50 threads x 20 events = 1000 lines, all valid JSON.
Without the lock, byte-level interleaving on the file produces
fragments like ``{"timesta{"timestamp"...`` that fail json.loads.
"""
def writer(thread_idx: int) -> None:
for i in range(20):
note: dict[str, Any] = {
"title": f"thread-{thread_idx}-note-{i}",
"content": "x" * 200, # non-trivial body to widen the race
"category": "general",
}
_append_note_event(
op="create",
note_id=f"t{thread_idx}-i{i}",
note=note,
)
threads = [threading.Thread(target=writer, args=(t,)) for t in range(50)]
for t in threads:
t.start()
for t in threads:
t.join()
lines = notes_path.read_text(encoding="utf-8").splitlines()
assert len(lines) == 1000, f"expected 1000 lines, got {len(lines)}"
for line in lines:
# raises if the line is malformed JSON
event = json.loads(line)
assert event["op"] == "create"
assert "note_id" in event
def test_single_writer_still_works(notes_path: Path) -> None:
"""Sanity: serial writes still produce a valid JSONL log."""
_append_note_event("create", "n1", {"title": "first"})
_append_note_event("update", "n1", {"title": "first updated"})
_append_note_event("delete", "n1")
events = [json.loads(line) for line in notes_path.read_text().splitlines()]
assert [e["op"] for e in events] == ["create", "update", "delete"]
assert all(e["note_id"] == "n1" for e in events)
+206
View File
@@ -0,0 +1,206 @@
"""Phase 2.1 smoke tests for the sandbox dispatch helper."""
from __future__ import annotations
from dataclasses import dataclass, field
from typing import Any
import httpx
import pytest
from strix.tools._sandbox_dispatch import post_to_sandbox
@dataclass
class _Ctx:
"""Stand-in for ``RunContextWrapper``. Only ``.context`` is touched."""
context: dict[str, Any] = field(default_factory=dict)
def _ok_ctx(**overrides: Any) -> _Ctx:
base: dict[str, Any] = {
"tool_server_host_port": 48081,
"sandbox_token": "test-bearer",
"agent_id": "agent-1",
}
base.update(overrides)
return _Ctx(context=base)
@pytest.mark.asyncio
async def test_missing_context_returns_error() -> None:
"""If ``ctx.context`` isn't a dict, return error — never raise."""
ctx = _Ctx(context="not a dict")
result = await post_to_sandbox(ctx, "browser_action", {"action": "launch"})
assert "error" in result
assert "context" in result["error"].lower()
@pytest.mark.asyncio
async def test_missing_port_or_token_returns_error() -> None:
ctx = _Ctx(context={"sandbox_token": "tok"}) # no port
result = await post_to_sandbox(ctx, "x", {})
assert "error" in result
assert "tool server port" in result["error"].lower()
@pytest.mark.asyncio
async def test_successful_response_returned_as_dict(
monkeypatch: pytest.MonkeyPatch,
) -> None:
captured: dict[str, Any] = {}
async def fake_post(
self: httpx.AsyncClient,
url: str,
*,
json: dict[str, Any] | None = None,
headers: dict[str, str] | None = None,
) -> httpx.Response:
captured["url"] = url
captured["json"] = json
captured["headers"] = headers
return httpx.Response(
status_code=200,
json={"result": "ok"},
request=httpx.Request("POST", url),
)
monkeypatch.setattr(httpx.AsyncClient, "post", fake_post)
result = await post_to_sandbox(_ok_ctx(), "terminal_execute", {"command": "ls"})
assert result == {"result": "ok"}
assert captured["url"] == "http://127.0.0.1:48081/execute"
assert captured["json"] == {
"agent_id": "agent-1",
"tool_name": "terminal_execute",
"kwargs": {"command": "ls"},
}
assert captured["headers"]["Authorization"] == "Bearer test-bearer"
@pytest.mark.asyncio
async def test_401_returns_auth_error(monkeypatch: pytest.MonkeyPatch) -> None:
async def fake_post(
self: httpx.AsyncClient,
url: str,
**_: Any,
) -> httpx.Response:
return httpx.Response(
status_code=401,
text="forbidden",
request=httpx.Request("POST", url),
)
monkeypatch.setattr(httpx.AsyncClient, "post", fake_post)
result = await post_to_sandbox(_ok_ctx(), "x", {})
assert "error" in result
assert "authorization" in result["error"].lower()
@pytest.mark.asyncio
async def test_5xx_returns_http_error(monkeypatch: pytest.MonkeyPatch) -> None:
async def fake_post(
self: httpx.AsyncClient,
url: str,
**_: Any,
) -> httpx.Response:
return httpx.Response(
status_code=503,
text="server fell over",
request=httpx.Request("POST", url),
)
monkeypatch.setattr(httpx.AsyncClient, "post", fake_post)
result = await post_to_sandbox(_ok_ctx(), "browser_action", {})
assert "error" in result
assert "503" in result["error"]
@pytest.mark.asyncio
async def test_timeout_returns_timeout_error(monkeypatch: pytest.MonkeyPatch) -> None:
async def fake_post(*_args: Any, **_kwargs: Any) -> httpx.Response:
raise httpx.ReadTimeout("read timeout")
monkeypatch.setattr(httpx.AsyncClient, "post", fake_post)
result = await post_to_sandbox(_ok_ctx(), "python_action", {})
assert "error" in result
assert "timed out" in result["error"].lower()
@pytest.mark.asyncio
async def test_connection_error_returns_error(monkeypatch: pytest.MonkeyPatch) -> None:
async def fake_post(*_args: Any, **_kwargs: Any) -> httpx.Response:
raise httpx.ConnectError("refused")
monkeypatch.setattr(httpx.AsyncClient, "post", fake_post)
result = await post_to_sandbox(_ok_ctx(), "x", {})
assert "error" in result
assert "connection" in result["error"].lower()
@pytest.mark.asyncio
async def test_response_too_large_capped(monkeypatch: pytest.MonkeyPatch) -> None:
"""C18 (AUDIT_R3): response > 50MB returns error, doesn't OOM."""
async def fake_post(
self: httpx.AsyncClient,
url: str,
**_: Any,
) -> httpx.Response:
# Construct a response with a >50MB body. Use a string trick —
# httpx.Response stores .content directly; we make it look huge.
big = b"x" * (51 * 1024 * 1024)
return httpx.Response(
status_code=200,
content=big,
request=httpx.Request("POST", url),
)
monkeypatch.setattr(httpx.AsyncClient, "post", fake_post)
result = await post_to_sandbox(_ok_ctx(), "browser_action", {})
assert "error" in result
assert "too large" in result["error"].lower()
@pytest.mark.asyncio
async def test_non_json_response_returns_error(
monkeypatch: pytest.MonkeyPatch,
) -> None:
async def fake_post(
self: httpx.AsyncClient,
url: str,
**_: Any,
) -> httpx.Response:
return httpx.Response(
status_code=200,
text="hello not json",
request=httpx.Request("POST", url),
)
monkeypatch.setattr(httpx.AsyncClient, "post", fake_post)
result = await post_to_sandbox(_ok_ctx(), "x", {})
assert "error" in result
assert "non-json" in result["error"].lower()
@pytest.mark.asyncio
async def test_non_object_json_returns_error(
monkeypatch: pytest.MonkeyPatch,
) -> None:
async def fake_post(
self: httpx.AsyncClient,
url: str,
**_: Any,
) -> httpx.Response:
return httpx.Response(
status_code=200,
json=["a", "list", "not", "an", "object"],
request=httpx.Request("POST", url),
)
monkeypatch.setattr(httpx.AsyncClient, "post", fake_post)
result = await post_to_sandbox(_ok_ctx(), "x", {})
assert "error" in result
assert "non-object" in result["error"].lower()
+250
View File
@@ -0,0 +1,250 @@
"""Phase 2.3 smoke tests for the simplest SDK-wrapped local tools.
Validates the wrapping pattern (legacy implementation in, JSON string out)
on three tool families: think (trivial), todo (in-memory + agent_state
adapter), notes (in-memory + JSONL persistence).
If this slice works end-to-end the same pattern carries the rest of the
local tools (reporting, web_search, file_edit, finish_scan, load_skill).
"""
from __future__ import annotations
import json
from collections.abc import Iterator
from dataclasses import dataclass, field
from pathlib import Path
from typing import Any
from unittest.mock import patch
import pytest
from agents.tool import FunctionTool
from strix.tools.notes import notes_actions as _notes_legacy
from strix.tools.notes.notes_sdk_tools import (
create_note,
delete_note,
get_note,
list_notes,
update_note,
)
from strix.tools.thinking.thinking_sdk_tools import think
from strix.tools.todo.todo_sdk_tools import (
create_todo,
delete_todo,
list_todos,
mark_todo_done,
mark_todo_pending,
update_todo,
)
@dataclass
class _Ctx:
"""Stand-in for ``RunContextWrapper``."""
context: dict[str, Any] = field(default_factory=dict)
def _ctx_for(agent_id: str = "test-agent") -> _Ctx:
return _Ctx(context={"agent_id": agent_id})
async def _invoke(tool: FunctionTool, ctx: _Ctx, **kwargs: Any) -> dict[str, Any]:
"""Invoke a function tool the way the SDK would and JSON-decode the result."""
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
# --- think ----------------------------------------------------------------
def test_think_is_a_function_tool() -> None:
assert isinstance(think, FunctionTool)
assert think.name == "think"
@pytest.mark.asyncio
async def test_think_records_thought() -> None:
ctx = _ctx_for()
thought = "planning my next move"
out = await _invoke(think, ctx, thought=thought)
assert out["success"] is True
assert f"{len(thought)} characters" in out["message"]
@pytest.mark.asyncio
async def test_think_rejects_empty() -> None:
ctx = _ctx_for()
out = await _invoke(think, ctx, thought=" ")
assert out["success"] is False
# --- todo -----------------------------------------------------------------
@pytest.fixture(autouse=True)
def _isolate_todo_storage() -> None:
"""Each test starts with an empty todo store so tests don't bleed."""
from strix.tools.todo import todo_actions
todo_actions._todos_storage.clear()
def test_todo_tools_are_function_tools() -> None:
for tool in (
create_todo,
list_todos,
update_todo,
mark_todo_done,
mark_todo_pending,
delete_todo,
):
assert isinstance(tool, FunctionTool)
@pytest.mark.asyncio
async def test_todo_lifecycle() -> None:
ctx = _ctx_for("agent-A")
# Create
created = await _invoke(create_todo, ctx, title="audit endpoint", priority="high")
assert created["success"] is True
assert created["count"] == 1
todo_id = created["created"][0]["todo_id"]
# List
listed = await _invoke(list_todos, ctx)
assert listed["success"] is True
assert any(t["todo_id"] == todo_id for t in listed["todos"])
# Update
updated = await _invoke(update_todo, ctx, todo_id=todo_id, status="in_progress")
assert updated["success"] is True
# Mark done
done = await _invoke(mark_todo_done, ctx, todo_id=todo_id)
assert done["success"] is True
# Reset to pending
pending = await _invoke(mark_todo_pending, ctx, todo_id=todo_id)
assert pending["success"] is True
# Delete
deleted = await _invoke(delete_todo, ctx, todo_id=todo_id)
assert deleted["success"] is True
@pytest.mark.asyncio
async def test_todos_are_per_agent_isolated() -> None:
"""Two agents should have independent todo stores."""
ctx_a = _ctx_for("agent-A")
ctx_b = _ctx_for("agent-B")
await _invoke(create_todo, ctx_a, title="A's task")
await _invoke(create_todo, ctx_b, title="B's task")
list_a = await _invoke(list_todos, ctx_a)
list_b = await _invoke(list_todos, ctx_b)
titles_a = [t["title"] for t in list_a["todos"]]
titles_b = [t["title"] for t in list_b["todos"]]
assert titles_a == ["A's task"]
assert titles_b == ["B's task"]
@pytest.mark.asyncio
async def test_create_todo_bulk_via_json_string() -> None:
ctx = _ctx_for()
out = await _invoke(
create_todo,
ctx,
todos=json.dumps(
[
{"title": "t1", "priority": "high"},
{"title": "t2", "priority": "low"},
],
),
)
assert out["success"] is True
assert out["count"] == 2
# --- notes ----------------------------------------------------------------
@pytest.fixture
def notes_run_dir(tmp_path: Path) -> Iterator[Path]:
"""Point the legacy notes module at a fresh run dir per test."""
run_dir = tmp_path / "strix_runs" / "test"
run_dir.mkdir(parents=True)
_notes_legacy._notes_storage.clear()
_notes_legacy._loaded_notes_run_dir = None
with patch.object(_notes_legacy, "_get_run_dir", return_value=run_dir):
yield run_dir
def test_notes_tools_are_function_tools() -> None:
for tool in (create_note, list_notes, get_note, update_note, delete_note):
assert isinstance(tool, FunctionTool)
@pytest.mark.asyncio
async def test_note_lifecycle(notes_run_dir: Path) -> None:
ctx = _ctx_for()
created = await _invoke(
create_note,
ctx,
title="SQLi at /login",
content="Form param `email` reflects into the WHERE clause.",
category="findings",
tags=["sqli", "auth"],
)
assert created["success"] is True
note_id = created["note_id"]
listed = await _invoke(list_notes, ctx, category="findings")
assert listed["success"] is True
assert listed["total_count"] == 1
fetched = await _invoke(get_note, ctx, note_id=note_id)
assert fetched["success"] is True
assert "WHERE clause" in fetched["note"]["content"]
updated = await _invoke(
update_note,
ctx,
note_id=note_id,
content="Confirmed boolean-blind SQLi.",
)
assert updated["success"] is True
deleted = await _invoke(delete_note, ctx, note_id=note_id)
assert deleted["success"] is True
@pytest.mark.asyncio
async def test_notes_jsonl_appended(notes_run_dir: Path) -> None:
"""Verify side effect: notes.jsonl receives one event per op."""
ctx = _ctx_for()
await _invoke(create_note, ctx, title="t", content="c", category="general")
jsonl = notes_run_dir / "notes" / "notes.jsonl"
assert jsonl.exists()
events = [json.loads(line) for line in jsonl.read_text().splitlines() if line]
assert events[0]["op"] == "create"
assert events[0]["note"]["title"] == "t"