Add system dependencies support for tools
Tools can now declare system-level package dependencies (apt, brew, pacman, dnf) that get checked and optionally installed when the tool is installed or run. Features: - SystemDependency dataclass with short form (string) and long form (dict) - New system_deps.py module with platform detection and installation - `cmdforge system-deps <tool>` command to check/install system packages - `cmdforge check` now shows both CmdForge and system dependencies - `cmdforge registry install` prompts for system dep installation - GUI: System Dependencies section in Tool Builder with add/edit dialog - Runner warns about missing system deps before execution - Integration with project install (manifest and lock-based) Also includes: - Quote paths in wrapper scripts for spaces support - Tests for value type preservation in code steps - Unskip invalid tool name test Co-Authored-By: Claude Opus 4.5 <[email protected]>
This commit is contained in:
Claude Opus 4.5
+2
-1
@@ -110,13 +110,14 @@ class TestCreateCommand:
|
||||
captured = capsys.readouterr()
|
||||
assert 'exists' in captured.err.lower() or 'exists' in captured.out.lower()
|
||||
|
||||
@pytest.mark.skip(reason="CLI does not yet validate tool names - potential enhancement")
|
||||
def test_create_invalid_name(self, temp_tools_dir, capsys):
|
||||
"""Invalid tool name should fail."""
|
||||
with patch('sys.argv', ['cmdforge', 'create', 'invalid/name']):
|
||||
result = main()
|
||||
|
||||
assert result != 0
|
||||
captured = capsys.readouterr()
|
||||
assert 'invalid' in captured.out.lower() or 'invalid' in captured.err.lower()
|
||||
|
||||
|
||||
class TestDeleteCommand:
|
||||
|
||||
+56
-4
@@ -92,6 +92,21 @@ Line 3: {var1} again"""
|
||||
result = substitute_variables("Count: {n}", {"n": 42})
|
||||
assert result == "Count: 42"
|
||||
|
||||
def test_zero_value_preserved(self):
|
||||
"""Zero should be preserved, not converted to empty string."""
|
||||
result = substitute_variables("Count: {n}", {"n": 0})
|
||||
assert result == "Count: 0"
|
||||
|
||||
def test_false_value_preserved(self):
|
||||
"""False should be preserved, not converted to empty string."""
|
||||
result = substitute_variables("Enabled: {flag}", {"flag": False})
|
||||
assert result == "Enabled: False"
|
||||
|
||||
def test_true_value_preserved(self):
|
||||
"""True should be preserved as 'True'."""
|
||||
result = substitute_variables("Enabled: {flag}", {"flag": True})
|
||||
assert result == "Enabled: True"
|
||||
|
||||
|
||||
class TestExecutePromptStep:
|
||||
"""Tests for prompt step execution."""
|
||||
@@ -179,9 +194,10 @@ class TestExecuteCodeStep:
|
||||
outputs, success = execute_code_step(step, variables)
|
||||
|
||||
assert success is True
|
||||
assert outputs["a"] == "1"
|
||||
assert outputs["b"] == "2"
|
||||
assert outputs["c"] == "3"
|
||||
# Code step outputs preserve original types (integers here)
|
||||
assert outputs["a"] == 1
|
||||
assert outputs["b"] == 2
|
||||
assert outputs["c"] == 3
|
||||
|
||||
def test_code_uses_variables(self):
|
||||
step = CodeStep(
|
||||
@@ -242,7 +258,43 @@ class TestExecuteCodeStep:
|
||||
outputs, success = execute_code_step(step, variables)
|
||||
|
||||
assert success is True
|
||||
assert outputs["result"] == "3"
|
||||
# Code step outputs preserve original types (integer here)
|
||||
assert outputs["result"] == 3
|
||||
|
||||
def test_code_preserves_complex_types(self):
|
||||
"""Code step outputs preserve lists/dicts for code→code workflows."""
|
||||
step = CodeStep(
|
||||
code="data = [1, 2, 3]\ninfo = {'key': 'value'}",
|
||||
output_var="data, info"
|
||||
)
|
||||
variables = {"input": ""}
|
||||
|
||||
outputs, success = execute_code_step(step, variables)
|
||||
|
||||
assert success is True
|
||||
assert outputs["data"] == [1, 2, 3]
|
||||
assert outputs["info"] == {"key": "value"}
|
||||
|
||||
def test_code_to_code_workflow(self):
|
||||
"""Test that complex types can flow between code steps."""
|
||||
# First step creates a list
|
||||
step1 = CodeStep(
|
||||
code="numbers = [1, 2, 3, 4, 5]",
|
||||
output_var="numbers"
|
||||
)
|
||||
variables = {"input": ""}
|
||||
outputs1, _ = execute_code_step(step1, variables)
|
||||
variables.update(outputs1)
|
||||
|
||||
# Second step uses the list directly (no re-parsing needed)
|
||||
step2 = CodeStep(
|
||||
code="total = sum(numbers)",
|
||||
output_var="total"
|
||||
)
|
||||
outputs2, success = execute_code_step(step2, variables)
|
||||
|
||||
assert success is True
|
||||
assert outputs2["total"] == 15
|
||||
|
||||
|
||||
class TestRunTool:
|
||||
|
||||
@@ -0,0 +1,310 @@
|
||||
"""Tests for system dependency management."""
|
||||
|
||||
import shutil
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from cmdforge.tool import SystemDependency, Tool
|
||||
from cmdforge.system_deps import (
|
||||
detect_os, detect_package_manager, check_system_dep,
|
||||
get_install_command, get_install_command_string,
|
||||
check_and_report, get_binary_path
|
||||
)
|
||||
|
||||
|
||||
class TestSystemDependencyDataclass:
|
||||
"""Tests for SystemDependency dataclass."""
|
||||
|
||||
def test_short_form_roundtrip(self):
|
||||
"""Short form 'ffmpeg' serializes back to string."""
|
||||
dep = SystemDependency.from_dict("ffmpeg")
|
||||
assert dep.name == "ffmpeg"
|
||||
assert dep.to_dict() == "ffmpeg"
|
||||
|
||||
def test_long_form_roundtrip(self):
|
||||
"""Long form with all fields serializes correctly."""
|
||||
data = {
|
||||
"name": "ffmpeg",
|
||||
"description": "Audio/video processing",
|
||||
"binaries": ["ffplay", "ffmpeg"],
|
||||
"packages": {"apt": "ffmpeg", "brew": "ffmpeg"}
|
||||
}
|
||||
dep = SystemDependency.from_dict(data)
|
||||
result = dep.to_dict()
|
||||
|
||||
assert result["name"] == "ffmpeg"
|
||||
assert result["description"] == "Audio/video processing"
|
||||
assert result["binaries"] == ["ffplay", "ffmpeg"]
|
||||
assert result["packages"]["apt"] == "ffmpeg"
|
||||
|
||||
def test_long_form_minimal_becomes_short(self):
|
||||
"""Long form with only name becomes short form on serialize."""
|
||||
data = {"name": "curl"}
|
||||
dep = SystemDependency.from_dict(data)
|
||||
# Empty fields should serialize to short form
|
||||
dep._original_format = "short"
|
||||
assert dep.to_dict() == "curl"
|
||||
|
||||
def test_get_binaries_to_check_default(self):
|
||||
"""get_binaries_to_check returns [name] when binaries is empty."""
|
||||
dep = SystemDependency(name="curl")
|
||||
assert dep.get_binaries_to_check() == ["curl"]
|
||||
|
||||
def test_get_binaries_to_check_specified(self):
|
||||
"""get_binaries_to_check returns specified binaries."""
|
||||
dep = SystemDependency(name="ffmpeg", binaries=["ffplay", "ffmpeg"])
|
||||
assert dep.get_binaries_to_check() == ["ffplay", "ffmpeg"]
|
||||
|
||||
def test_get_package_name_default(self):
|
||||
"""get_package_name returns name when no override."""
|
||||
dep = SystemDependency(name="ffmpeg")
|
||||
assert dep.get_package_name("apt") == "ffmpeg"
|
||||
assert dep.get_package_name("brew") == "ffmpeg"
|
||||
|
||||
def test_get_package_name_override(self):
|
||||
"""get_package_name returns override when specified."""
|
||||
dep = SystemDependency(name="ffmpeg", packages={"brew": "ffmpeg-headless"})
|
||||
assert dep.get_package_name("apt") == "ffmpeg" # Default
|
||||
assert dep.get_package_name("brew") == "ffmpeg-headless" # Override
|
||||
|
||||
|
||||
class TestPlatformDetection:
|
||||
"""Tests for platform detection functions."""
|
||||
|
||||
@patch('platform.system')
|
||||
def test_detect_os_linux(self, mock_system):
|
||||
"""detect_os returns 'linux' for Linux."""
|
||||
mock_system.return_value = "Linux"
|
||||
assert detect_os() == "linux"
|
||||
|
||||
@patch('platform.system')
|
||||
def test_detect_os_darwin(self, mock_system):
|
||||
"""detect_os returns 'darwin' for macOS."""
|
||||
mock_system.return_value = "Darwin"
|
||||
assert detect_os() == "darwin"
|
||||
|
||||
@patch('platform.system')
|
||||
def test_detect_os_windows(self, mock_system):
|
||||
"""detect_os returns 'windows' for Windows."""
|
||||
mock_system.return_value = "Windows"
|
||||
assert detect_os() == "windows"
|
||||
|
||||
@patch('cmdforge.system_deps.detect_os')
|
||||
@patch('shutil.which')
|
||||
def test_detect_package_manager_apt(self, mock_which, mock_os):
|
||||
"""detect_package_manager finds apt on Linux."""
|
||||
mock_os.return_value = "linux"
|
||||
mock_which.side_effect = lambda x: "/usr/bin/apt-get" if x == "apt-get" else None
|
||||
assert detect_package_manager() == "apt"
|
||||
|
||||
@patch('cmdforge.system_deps.detect_os')
|
||||
@patch('shutil.which')
|
||||
def test_detect_package_manager_brew_macos(self, mock_which, mock_os):
|
||||
"""detect_package_manager finds brew on macOS."""
|
||||
mock_os.return_value = "darwin"
|
||||
mock_which.side_effect = lambda x: "/usr/local/bin/brew" if x == "brew" else None
|
||||
assert detect_package_manager() == "brew"
|
||||
|
||||
@patch('cmdforge.system_deps.detect_os')
|
||||
def test_detect_package_manager_windows_returns_none(self, mock_os):
|
||||
"""detect_package_manager returns None on Windows."""
|
||||
mock_os.return_value = "windows"
|
||||
assert detect_package_manager() is None
|
||||
|
||||
|
||||
class TestDependencyChecking:
|
||||
"""Tests for dependency checking functions."""
|
||||
|
||||
@patch('shutil.which')
|
||||
def test_check_system_dep_found(self, mock_which):
|
||||
"""check_system_dep returns True when binary exists."""
|
||||
mock_which.return_value = "/usr/bin/curl"
|
||||
dep = SystemDependency(name="curl")
|
||||
assert check_system_dep(dep) is True
|
||||
|
||||
@patch('shutil.which')
|
||||
def test_check_system_dep_not_found(self, mock_which):
|
||||
"""check_system_dep returns False when binary not found."""
|
||||
mock_which.return_value = None
|
||||
dep = SystemDependency(name="nonexistent-pkg")
|
||||
assert check_system_dep(dep) is False
|
||||
|
||||
@patch('shutil.which')
|
||||
def test_check_system_dep_any_binary_satisfied(self, mock_which):
|
||||
"""Dep is satisfied if ANY binary is found."""
|
||||
def which_side_effect(binary):
|
||||
return "/usr/bin/ffplay" if binary == "ffplay" else None
|
||||
|
||||
mock_which.side_effect = which_side_effect
|
||||
dep = SystemDependency(name="ffmpeg", binaries=["ffplay", "ffmpeg"])
|
||||
assert check_system_dep(dep) is True
|
||||
|
||||
@patch('shutil.which')
|
||||
def test_check_and_report(self, mock_which):
|
||||
"""check_and_report categorizes deps correctly."""
|
||||
def which_side_effect(binary):
|
||||
if binary in ["curl", "git"]:
|
||||
return f"/usr/bin/{binary}"
|
||||
return None
|
||||
|
||||
mock_which.side_effect = which_side_effect
|
||||
|
||||
deps = [
|
||||
SystemDependency(name="curl"),
|
||||
SystemDependency(name="git"),
|
||||
SystemDependency(name="nonexistent"),
|
||||
]
|
||||
|
||||
installed, missing = check_and_report(deps)
|
||||
|
||||
assert len(installed) == 2
|
||||
assert len(missing) == 1
|
||||
assert missing[0].name == "nonexistent"
|
||||
|
||||
|
||||
class TestInstallCommands:
|
||||
"""Tests for install command generation."""
|
||||
|
||||
def test_get_install_command_apt(self):
|
||||
"""get_install_command returns correct apt command."""
|
||||
dep = SystemDependency(name="ffmpeg")
|
||||
cmd = get_install_command(dep, "apt")
|
||||
assert cmd == ["sudo", "apt-get", "install", "-y", "ffmpeg"]
|
||||
|
||||
def test_get_install_command_brew(self):
|
||||
"""get_install_command returns correct brew command."""
|
||||
dep = SystemDependency(name="ffmpeg")
|
||||
cmd = get_install_command(dep, "brew")
|
||||
assert cmd == ["brew", "install", "ffmpeg"]
|
||||
|
||||
def test_get_install_command_pacman(self):
|
||||
"""get_install_command returns correct pacman command."""
|
||||
dep = SystemDependency(name="ffmpeg")
|
||||
cmd = get_install_command(dep, "pacman")
|
||||
assert cmd == ["sudo", "pacman", "-S", "--noconfirm", "ffmpeg"]
|
||||
|
||||
def test_get_install_command_dnf(self):
|
||||
"""get_install_command returns correct dnf command."""
|
||||
dep = SystemDependency(name="ffmpeg")
|
||||
cmd = get_install_command(dep, "dnf")
|
||||
assert cmd == ["sudo", "dnf", "install", "-y", "ffmpeg"]
|
||||
|
||||
def test_get_install_command_unknown_manager(self):
|
||||
"""get_install_command returns None for unknown manager."""
|
||||
dep = SystemDependency(name="ffmpeg")
|
||||
cmd = get_install_command(dep, "unknown")
|
||||
assert cmd is None
|
||||
|
||||
def test_install_command_no_shell_injection(self):
|
||||
"""Install command uses list, no shell injection possible."""
|
||||
# Even malicious input is safe because subprocess with list doesn't use shell
|
||||
dep = SystemDependency(name="test; rm -rf /")
|
||||
cmd = get_install_command(dep, "apt")
|
||||
assert cmd == ["sudo", "apt-get", "install", "-y", "test; rm -rf /"]
|
||||
# This is safe - the semicolon is passed as literal arg, not interpreted
|
||||
|
||||
def test_get_install_command_string(self):
|
||||
"""get_install_command_string returns readable string."""
|
||||
dep = SystemDependency(name="ffmpeg")
|
||||
cmd_str = get_install_command_string(dep, "apt")
|
||||
assert cmd_str == "sudo apt-get install -y ffmpeg"
|
||||
|
||||
|
||||
class TestToolIntegration:
|
||||
"""Tests for Tool integration with system dependencies."""
|
||||
|
||||
def test_tool_with_system_dependencies_loads(self):
|
||||
"""Tool config with system_dependencies parses correctly."""
|
||||
data = {
|
||||
"name": "test-tool",
|
||||
"description": "Test",
|
||||
"system_dependencies": [
|
||||
"curl",
|
||||
{
|
||||
"name": "ffmpeg",
|
||||
"description": "Audio processing",
|
||||
"binaries": ["ffplay"],
|
||||
}
|
||||
],
|
||||
"steps": [],
|
||||
"output": "{input}"
|
||||
}
|
||||
|
||||
tool = Tool.from_dict(data)
|
||||
|
||||
assert len(tool.system_dependencies) == 2
|
||||
assert tool.system_dependencies[0].name == "curl"
|
||||
assert tool.system_dependencies[1].name == "ffmpeg"
|
||||
assert tool.system_dependencies[1].binaries == ["ffplay"]
|
||||
|
||||
def test_tool_to_dict_includes_system_dependencies(self):
|
||||
"""Tool.to_dict includes system_dependencies when present."""
|
||||
tool = Tool(
|
||||
name="test-tool",
|
||||
description="Test",
|
||||
system_dependencies=[
|
||||
SystemDependency(name="curl", _original_format="short"),
|
||||
SystemDependency(
|
||||
name="ffmpeg",
|
||||
description="Audio",
|
||||
binaries=["ffplay"],
|
||||
_original_format="long"
|
||||
)
|
||||
]
|
||||
)
|
||||
|
||||
data = tool.to_dict()
|
||||
|
||||
assert "system_dependencies" in data
|
||||
assert len(data["system_dependencies"]) == 2
|
||||
assert data["system_dependencies"][0] == "curl" # Short form
|
||||
assert data["system_dependencies"][1]["name"] == "ffmpeg" # Long form
|
||||
|
||||
def test_tool_without_system_dependencies(self):
|
||||
"""Tool without system_dependencies works correctly."""
|
||||
data = {
|
||||
"name": "simple-tool",
|
||||
"description": "Test",
|
||||
"steps": [],
|
||||
"output": "{input}"
|
||||
}
|
||||
|
||||
tool = Tool.from_dict(data)
|
||||
assert tool.system_dependencies == []
|
||||
|
||||
# to_dict should not include empty system_dependencies
|
||||
result = tool.to_dict()
|
||||
assert "system_dependencies" not in result
|
||||
|
||||
|
||||
class TestRunnerIntegration:
|
||||
"""Tests for runner.py integration."""
|
||||
|
||||
@patch('cmdforge.system_deps.check_system_dep')
|
||||
def test_check_system_dependencies_function(self, mock_check):
|
||||
"""check_system_dependencies returns missing deps."""
|
||||
from cmdforge.runner import check_system_dependencies
|
||||
|
||||
mock_check.side_effect = lambda d: d.name != "missing-pkg"
|
||||
|
||||
tool = Tool(
|
||||
name="test-tool",
|
||||
system_dependencies=[
|
||||
SystemDependency(name="installed-pkg"),
|
||||
SystemDependency(name="missing-pkg"),
|
||||
]
|
||||
)
|
||||
|
||||
missing = check_system_dependencies(tool)
|
||||
|
||||
assert len(missing) == 1
|
||||
assert missing[0].name == "missing-pkg"
|
||||
|
||||
def test_check_system_dependencies_empty(self):
|
||||
"""check_system_dependencies returns empty for no deps."""
|
||||
from cmdforge.runner import check_system_dependencies
|
||||
|
||||
tool = Tool(name="test-tool")
|
||||
missing = check_system_dependencies(tool)
|
||||
assert missing == []
|
||||
Reference in New Issue
Block a user