Phillip Tarrant
f60c47a85f
Implement 6 new agent tools — write_file, make_dir, delete_file, str_replace, patch_apply, run_command — bringing the agent from read-only observer to active code modifier. All write/shell operations are gated through the existing permissions service. Also fix a bug where qwen3.5 thinking mode produces reasoning tokens but no content after tool results, causing the agent to silently exit. The loop now detects reasoning-only responses, retries twice, then injects a nudge message to break the model out of its thinking loop. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
210 lines
8.1 KiB
Python
210 lines
8.1 KiB
Python
"""Tests for the tool framework and core tools (Phase 4)."""
|
|
|
|
from pathlib import Path
|
|
from unittest.mock import patch
|
|
|
|
import pytest
|
|
|
|
from app.models.config import AppConfig, PermissionsConfig, load_config
|
|
from app.models.tool_call import ToolResultStatus
|
|
from app.services.permissions import PermissionsService
|
|
from app.tools.base import BaseTool
|
|
from app.tools.filesystem import ListDirTool, ReadFileTool
|
|
from app.tools.registry import ToolRegistry, create_default_registry
|
|
from app.tools.search import FindFilesTool, GrepFilesTool
|
|
|
|
|
|
@pytest.fixture
|
|
def config() -> AppConfig:
|
|
return load_config()
|
|
|
|
|
|
@pytest.fixture
|
|
def workspace(config: AppConfig) -> Path:
|
|
return config.agent.workspace_root
|
|
|
|
|
|
# --- BaseTool ---
|
|
|
|
|
|
class TestBaseTool:
|
|
def test_run_with_invalid_args_returns_error(self, workspace: Path, config: AppConfig) -> None:
|
|
tool = ReadFileTool(workspace, config)
|
|
# missing required 'file_path'
|
|
result = tool.run("tc-1", {})
|
|
assert result.status == ToolResultStatus.ERROR
|
|
assert "Invalid arguments" in (result.error or "")
|
|
|
|
def test_get_openai_schema_structure(self, workspace: Path, config: AppConfig) -> None:
|
|
tool = ReadFileTool(workspace, config)
|
|
schema = tool.get_openai_schema()
|
|
assert schema["type"] == "function"
|
|
assert schema["function"]["name"] == "read_file"
|
|
assert "parameters" in schema["function"]
|
|
assert schema["function"]["parameters"]["type"] == "object"
|
|
|
|
|
|
# --- PermissionsService ---
|
|
|
|
|
|
class TestPermissionsService:
|
|
def test_deny_list_blocks(self) -> None:
|
|
svc = PermissionsService(PermissionsConfig(deny=["dangerous_tool"]))
|
|
assert svc.check("dangerous_tool") is False
|
|
|
|
def test_auto_approve_allows(self) -> None:
|
|
svc = PermissionsService(PermissionsConfig(auto_approve=["read_file"]))
|
|
assert svc.check("read_file") is True
|
|
|
|
@patch("app.services.permissions.Confirm.ask", return_value=True)
|
|
def test_prompt_user_approved(self, mock_ask: object) -> None:
|
|
svc = PermissionsService(PermissionsConfig(prompt_user=["write_file"]))
|
|
assert svc.check("write_file") is True
|
|
|
|
@patch("app.services.permissions.Confirm.ask", return_value=False)
|
|
def test_prompt_user_denied(self, mock_ask: object) -> None:
|
|
svc = PermissionsService(PermissionsConfig(prompt_user=["write_file"]))
|
|
assert svc.check("write_file") is False
|
|
|
|
@patch("app.services.permissions.Confirm.ask", return_value=False)
|
|
def test_unlisted_tool_prompts(self, mock_ask: object) -> None:
|
|
svc = PermissionsService(PermissionsConfig())
|
|
assert svc.check("unknown_tool") is False
|
|
|
|
|
|
# --- ToolRegistry ---
|
|
|
|
|
|
class TestToolRegistry:
|
|
def test_register_and_get(self, workspace: Path, config: AppConfig) -> None:
|
|
registry = ToolRegistry()
|
|
tool = ReadFileTool(workspace, config)
|
|
registry.register(tool)
|
|
assert registry.get("read_file") is tool
|
|
|
|
def test_duplicate_raises(self, workspace: Path, config: AppConfig) -> None:
|
|
registry = ToolRegistry()
|
|
tool = ReadFileTool(workspace, config)
|
|
registry.register(tool)
|
|
with pytest.raises(ValueError, match="Duplicate"):
|
|
registry.register(tool)
|
|
|
|
def test_get_missing_returns_none(self) -> None:
|
|
registry = ToolRegistry()
|
|
assert registry.get("nonexistent") is None
|
|
|
|
def test_create_default_registry(self, workspace: Path, config: AppConfig) -> None:
|
|
registry = create_default_registry(workspace, config)
|
|
names = set(registry.get_all().keys())
|
|
assert names == {
|
|
"read_file", "list_dir", "grep_files", "find_files",
|
|
"write_file", "make_dir", "delete_file",
|
|
"str_replace", "patch_apply",
|
|
"run_command",
|
|
"finish",
|
|
}
|
|
|
|
def test_schema_export(self, workspace: Path, config: AppConfig) -> None:
|
|
registry = create_default_registry(workspace, config)
|
|
schemas = registry.get_openai_tools_schema()
|
|
assert len(schemas) == 11
|
|
assert all(s["type"] == "function" for s in schemas)
|
|
|
|
|
|
# --- ReadFileTool ---
|
|
|
|
|
|
class TestReadFileTool:
|
|
def test_read_existing_file(self, workspace: Path, config: AppConfig) -> None:
|
|
tool = ReadFileTool(workspace, config)
|
|
result = tool.run("tc-1", {"file_path": "config/config.yaml"})
|
|
assert result.status == ToolResultStatus.SUCCESS
|
|
assert "llm:" in result.output
|
|
|
|
def test_read_nonexistent_file(self, workspace: Path, config: AppConfig) -> None:
|
|
tool = ReadFileTool(workspace, config)
|
|
result = tool.run("tc-2", {"file_path": "nonexistent.txt"})
|
|
assert result.status == ToolResultStatus.ERROR
|
|
assert "not found" in (result.error or "").lower()
|
|
|
|
def test_path_traversal_blocked(self, workspace: Path, config: AppConfig) -> None:
|
|
tool = ReadFileTool(workspace, config)
|
|
result = tool.run("tc-3", {"file_path": "../../etc/passwd"})
|
|
assert result.status == ToolResultStatus.ERROR
|
|
assert "outside" in (result.error or "").lower()
|
|
|
|
|
|
# --- ListDirTool ---
|
|
|
|
|
|
class TestListDirTool:
|
|
def test_list_root(self, workspace: Path, config: AppConfig) -> None:
|
|
tool = ListDirTool(workspace, config)
|
|
result = tool.run("tc-1", {})
|
|
assert result.status == ToolResultStatus.SUCCESS
|
|
assert "app/" in result.output
|
|
|
|
def test_list_subdir(self, workspace: Path, config: AppConfig) -> None:
|
|
tool = ListDirTool(workspace, config)
|
|
result = tool.run("tc-2", {"directory_path": "app/tools"})
|
|
assert result.status == ToolResultStatus.SUCCESS
|
|
assert "base.py" in result.output
|
|
|
|
def test_list_nonexistent_dir(self, workspace: Path, config: AppConfig) -> None:
|
|
tool = ListDirTool(workspace, config)
|
|
result = tool.run("tc-3", {"directory_path": "nonexistent_dir"})
|
|
assert result.status == ToolResultStatus.ERROR
|
|
|
|
def test_list_recursive(self, workspace: Path, config: AppConfig) -> None:
|
|
tool = ListDirTool(workspace, config)
|
|
result = tool.run("tc-4", {"directory_path": "config", "recursive": True})
|
|
assert result.status == ToolResultStatus.SUCCESS
|
|
assert "config.yaml" in result.output
|
|
|
|
|
|
# --- GrepFilesTool ---
|
|
|
|
|
|
class TestGrepFilesTool:
|
|
def test_grep_finds_matches(self, workspace: Path, config: AppConfig) -> None:
|
|
tool = GrepFilesTool(workspace, config)
|
|
result = tool.run("tc-1", {"pattern": "BaseTool", "path": "app/tools", "file_pattern": "*.py"})
|
|
assert result.status == ToolResultStatus.SUCCESS
|
|
assert "BaseTool" in result.output
|
|
|
|
def test_grep_no_matches(self, workspace: Path, config: AppConfig) -> None:
|
|
tool = GrepFilesTool(workspace, config)
|
|
# Search only in config/ to avoid matching the test file itself
|
|
result = tool.run("tc-2", {"pattern": "zzz_will_never_match_zzz", "path": "config"})
|
|
assert result.status == ToolResultStatus.SUCCESS
|
|
assert "No matches" in result.output
|
|
|
|
def test_grep_path_traversal_blocked(self, workspace: Path, config: AppConfig) -> None:
|
|
tool = GrepFilesTool(workspace, config)
|
|
result = tool.run("tc-3", {"pattern": "root", "path": "../../etc"})
|
|
assert result.status == ToolResultStatus.ERROR
|
|
|
|
|
|
# --- FindFilesTool ---
|
|
|
|
|
|
class TestFindFilesTool:
|
|
def test_find_files(self, workspace: Path, config: AppConfig) -> None:
|
|
tool = FindFilesTool(workspace, config)
|
|
result = tool.run("tc-1", {"pattern": "*.py", "path": "app/tools"})
|
|
assert result.status == ToolResultStatus.SUCCESS
|
|
assert "base.py" in result.output
|
|
|
|
def test_find_no_results(self, workspace: Path, config: AppConfig) -> None:
|
|
tool = FindFilesTool(workspace, config)
|
|
result = tool.run("tc-2", {"pattern": "*.nonexistent_ext"})
|
|
assert result.status == ToolResultStatus.SUCCESS
|
|
assert "No files found" in result.output
|
|
|
|
def test_find_relative_paths(self, workspace: Path, config: AppConfig) -> None:
|
|
tool = FindFilesTool(workspace, config)
|
|
result = tool.run("tc-3", {"pattern": "config.yaml"})
|
|
assert result.status == ToolResultStatus.SUCCESS
|
|
# Should be relative, not absolute
|
|
assert not result.output.startswith("/")
|