diff --git a/src/specify_cli/_utils.py b/src/specify_cli/_utils.py index 300ca4ff58..141f012ea4 100644 --- a/src/specify_cli/_utils.py +++ b/src/specify_cli/_utils.py @@ -7,7 +7,9 @@ import shutil import stat import subprocess +import sys import tempfile +import warnings import yaml from pathlib import Path, PurePosixPath, PureWindowsPath from typing import Any @@ -19,7 +21,72 @@ DOCKER_AGENT_CHECK_TIMEOUT = 5 -def docker_agent_command(executable: str | None = None) -> list[str] | None: +def _docker_path_is_excluded(path: Path, roots: tuple[Path, ...]) -> bool: + if any(path.is_relative_to(root) for root in roots): + return True + # resolve() can retain a different spelling on case-insensitive filesystems. + for ancestor in (path, *path.parents): + for root in roots: + try: + if ancestor.samefile(root): + return True + except (FileNotFoundError, NotADirectoryError): + continue + return False + + +def _find_docker_executable( + name: str, *, project_root: Path | None = None +) -> str | None: + """Search absolute PATH entries outside launch and runtime directories.""" + roots = (Path.cwd().resolve(),) + if project_root is not None: + roots += (project_root.resolve(),) + names = [name] + if sys.platform == "win32": + # Python 3.11's which() does not expand PATHEXT for qualified names. + pathext = os.environ.get("PATHEXT") or ".COM;.EXE;.BAT;.CMD" + names = [ + name + extension + for extension in pathext.split(";") + if extension + ] + for entry in os.environ.get("PATH", os.defpath).split(os.pathsep): + directory = Path(entry) + if not directory.is_absolute(): + continue + try: + directory = directory.resolve() + if _docker_path_is_excluded(directory, roots): + continue + except (OSError, RuntimeError) as exc: + warnings.warn( + f"Skipping Docker executable search candidate {directory}: {exc}", + stacklevel=2, + ) + continue + # A qualified name bypasses Python 3.11's implicit Windows CWD search. + for filename in names: + candidate = directory / filename + try: + resolved = shutil.which(str(candidate)) + if resolved is not None: + executable = Path(resolved) + if executable.is_absolute() and not _docker_path_is_excluded( + executable.resolve(), roots + ): + return str(executable) + except (OSError, RuntimeError) as exc: + warnings.warn( + f"Skipping Docker executable search candidate {candidate}: {exc}", + stacklevel=2, + ) + return None + + +def docker_agent_command( + executable: str | None = None, *, project_root: Path | None = None +) -> list[str] | None: """Return a runnable Docker Agent command, or ``None`` if unavailable. Docker Agent is distributed either as the standalone ``docker-agent`` @@ -27,11 +94,11 @@ def docker_agent_command(executable: str | None = None) -> list[str] | None: is verified with a bounded, read-only version probe so a plain Docker CLI is not mistaken for an installed Docker Agent. """ - resolved_from_path = executable is None if executable is None: - if shutil.which("docker-agent"): - return ["docker-agent", "run"] - executable = shutil.which("docker") + standalone = _find_docker_executable("docker-agent", project_root=project_root) + if standalone is not None: + return [standalone, "run"] + executable = _find_docker_executable("docker", project_root=project_root) if executable is None: return None @@ -54,7 +121,7 @@ def docker_agent_command(executable: str | None = None) -> list[str] | None: return None if result.returncode != 0: return None - return ["docker", "agent", "run"] if resolved_from_path else run_command + return run_command def relative_extension_path_violation(value: Any) -> str | None: diff --git a/src/specify_cli/integrations/docker_agent/__init__.py b/src/specify_cli/integrations/docker_agent/__init__.py index 939e35555b..237d0c4ef8 100644 --- a/src/specify_cli/integrations/docker_agent/__init__.py +++ b/src/specify_cli/integrations/docker_agent/__init__.py @@ -50,20 +50,20 @@ class DockerAgentIntegration(SkillsIntegration): # ``agents..hooks``. Spec Kit does not edit that user-owned file, so # hooks are intentionally not exposed through the integration event system. - def _agent_command(self) -> list[str]: + def _agent_command(self, project_root: Path | None = None) -> list[str] | None: """Return the available Docker Agent command form.""" # The shared executable override supports both a standalone # ``docker-agent`` binary and the Docker CLI plugin form. executable = self._resolve_executable() - command = docker_agent_command( - None if executable == self.key else executable + # An explicit override can have the same spelling as the default key. + has_override = bool( + os.environ.get("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", "").strip() + ) + return docker_agent_command( + None if executable == self.key and not has_override else executable, + project_root=project_root, ) - if command is None: - # Preserve the normal executable-shaped argv for dispatch callers; - # preflight and the subprocess runner report the unavailable CLI. - return [executable, "run"] - return command @classmethod def options(cls) -> list[IntegrationOption]: @@ -128,7 +128,10 @@ def build_exec_args( "reference, for example ./agent.yaml" ) - args = [*self._agent_command(), "--exec"] + command = self._agent_command(project_root) + if command is None: + return None + args = [*command, "--exec"] args.extend(runtime_args or legacy_args) diff --git a/tests/integrations/test_integration_docker_agent.py b/tests/integrations/test_integration_docker_agent.py index 173f75efda..683db2815c 100644 --- a/tests/integrations/test_integration_docker_agent.py +++ b/tests/integrations/test_integration_docker_agent.py @@ -1,5 +1,9 @@ """Tests for the Docker Agent integration.""" +import os +import subprocess +from unittest.mock import Mock + import pytest from specify_cli.integrations.docker_agent import DockerAgentIntegration @@ -47,15 +51,15 @@ def test_extra_args_are_applied_to_build_exec_args(monkeypatch): "./agent.yaml --agent root --model openai/gpt-5", ) monkeypatch.setattr( - "shutil.which", - lambda name: "/usr/bin/docker" if name == "docker" else None, + "specify_cli._utils._find_docker_executable", + lambda name, **_kwargs: "/usr/bin/docker" if name == "docker" else None, ) monkeypatch.setattr("subprocess.run", lambda *args, **kwargs: type("Result", (), {"returncode": 0})()) args = DockerAgentIntegration().build_exec_args("prompt", output_json=False) assert args == [ - "docker", + "/usr/bin/docker", "agent", "run", "--exec", @@ -72,8 +76,8 @@ def test_extra_args_are_applied_to_build_exec_args(monkeypatch): def test_per_step_runtime_config_builds_ordered_argv(monkeypatch): monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", raising=False) monkeypatch.setattr( - "shutil.which", - lambda name: "/usr/bin/docker-agent" if name == "docker-agent" else None, + "specify_cli._utils._find_docker_executable", + lambda name, **_kwargs: "/usr/bin/docker-agent" if name == "docker-agent" else None, ) args = DockerAgentIntegration().build_exec_args( @@ -88,7 +92,7 @@ def test_per_step_runtime_config_builds_ordered_argv(monkeypatch): ) assert args == [ - "docker-agent", + "/usr/bin/docker-agent", "run", "--exec", "./agent.yaml", @@ -107,7 +111,10 @@ def test_per_step_args_ignore_legacy_extra_flags(monkeypatch): monkeypatch.setenv( "SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", "--hide-tool-results" ) - monkeypatch.setattr("shutil.which", lambda name: None) + monkeypatch.setattr( + "specify_cli._utils._find_docker_executable", + lambda name, **_kwargs: "/usr/bin/docker-agent", + ) args = DockerAgentIntegration().build_exec_args( "prompt", @@ -116,7 +123,7 @@ def test_per_step_args_ignore_legacy_extra_flags(monkeypatch): ) assert args == [ - "docker-agent", + "/usr/bin/docker-agent", "run", "--exec", "./agent.yaml", @@ -131,7 +138,10 @@ def test_per_step_args_override_legacy_extra_args(monkeypatch, legacy_value): "SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", legacy_value, ) - monkeypatch.setattr("shutil.which", lambda name: None) + monkeypatch.setattr( + "specify_cli._utils._find_docker_executable", + lambda name, **_kwargs: "/usr/bin/docker-agent", + ) args = DockerAgentIntegration().build_exec_args( "prompt", @@ -140,7 +150,7 @@ def test_per_step_args_override_legacy_extra_args(monkeypatch, legacy_value): ) assert args == [ - "docker-agent", + "/usr/bin/docker-agent", "run", "--exec", "./step.yaml", @@ -151,7 +161,10 @@ def test_per_step_args_override_legacy_extra_args(monkeypatch, legacy_value): def test_command_step_model_is_the_only_model_source(monkeypatch): monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", raising=False) - monkeypatch.setattr("shutil.which", lambda name: None) + monkeypatch.setattr( + "specify_cli._utils._find_docker_executable", + lambda name, **_kwargs: "/usr/bin/docker-agent", + ) args = DockerAgentIntegration().build_exec_args( "prompt", @@ -212,8 +225,8 @@ def test_prompt_is_passed_after_agent_config(monkeypatch): "SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", "./agent.yaml" ) monkeypatch.setattr( - "shutil.which", - lambda name: "/usr/bin/docker" if name == "docker" else None, + "specify_cli._utils._find_docker_executable", + lambda name, **_kwargs: "/usr/bin/docker" if name == "docker" else None, ) monkeypatch.setattr("subprocess.run", lambda *args, **kwargs: type("Result", (), {"returncode": 0})()) @@ -222,7 +235,7 @@ def test_prompt_is_passed_after_agent_config(monkeypatch): ) assert args == [ - "docker", + "/usr/bin/docker", "agent", "run", "--exec", @@ -234,11 +247,14 @@ def test_prompt_is_passed_after_agent_config(monkeypatch): def test_prompt_starting_with_flag_is_delimited(monkeypatch): monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", "./agent.yaml") - monkeypatch.setattr("shutil.which", lambda name: None) + monkeypatch.setattr( + "specify_cli._utils._find_docker_executable", + lambda name, **_kwargs: "/usr/bin/docker-agent", + ) args = DockerAgentIntegration().build_exec_args("--help", output_json=False) - assert args == ["docker-agent", "run", "--exec", "./agent.yaml", "--", "--help"] + assert args == ["/usr/bin/docker-agent", "run", "--exec", "./agent.yaml", "--", "--help"] def test_requires_agent_config(monkeypatch): @@ -257,22 +273,25 @@ def test_legacy_agent_config_cannot_resolve_to_empty_token_list(monkeypatch): def test_uses_standalone_executable(monkeypatch): monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", "./agent.yaml") monkeypatch.setattr( - "shutil.which", - lambda name: "/usr/bin/docker-agent" if name == "docker-agent" else None, + "specify_cli._utils._find_docker_executable", + lambda name, **_kwargs: "/usr/bin/docker-agent" if name == "docker-agent" else None, ) args = DockerAgentIntegration().build_exec_args("prompt", output_json=False) - assert args == ["docker-agent", "run", "--exec", "./agent.yaml", "--", "prompt"] + assert args == ["/usr/bin/docker-agent", "run", "--exec", "./agent.yaml", "--", "prompt"] def test_standalone_executable_has_priority(monkeypatch): monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", "./agent.yaml") - monkeypatch.setattr("shutil.which", lambda name: "/usr/bin/docker-agent") + monkeypatch.setattr( + "specify_cli._utils._find_docker_executable", + lambda name, **_kwargs: "/usr/bin/docker-agent", + ) args = DockerAgentIntegration().build_exec_args("prompt", output_json=False) - assert args == ["docker-agent", "run", "--exec", "./agent.yaml", "--", "prompt"] + assert args == ["/usr/bin/docker-agent", "run", "--exec", "./agent.yaml", "--", "prompt"] def test_executable_override(monkeypatch): @@ -300,3 +319,187 @@ def test_docker_executable_override_uses_agent_subcommand(monkeypatch): args = DockerAgentIntegration().build_exec_args("prompt", output_json=False) assert args == ["/opt/docker", "agent", "run", "--exec", "./agent.yaml", "--", "prompt"] + + +@pytest.mark.parametrize("stream", [False, True]) +def test_dispatch_rejects_checkout_local_executable(tmp_path, monkeypatch, stream): + executable = tmp_path / ("docker-agent.exe" if os.name == "nt" else "docker-agent") + executable.touch() + executable.chmod(0o755) + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("PATH", str(tmp_path)) + monkeypatch.setenv("PATHEXT", ".exe") + monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", raising=False) + run = Mock(return_value=subprocess.CompletedProcess([], 0, stdout="", stderr="")) + monkeypatch.setattr(subprocess, "run", run) + + with pytest.raises(NotImplementedError, match="does not support CLI dispatch"): + DockerAgentIntegration().dispatch_command( + "speckit.plan", + project_root=tmp_path, + stream=stream, + integration_args=["./agent.yaml"], + ) + + run.assert_not_called() + + +@pytest.mark.parametrize("override_kind", ["absolute", "bare"]) +def test_dispatch_preserves_explicit_checkout_local_override( + tmp_path, monkeypatch, override_kind +): + executable = tmp_path / ("docker-agent.exe" if os.name == "nt" else "docker-agent") + executable.touch() + executable.chmod(0o755) + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("PATH", str(tmp_path)) + monkeypatch.setenv("PATHEXT", ".exe") + override = str(executable) if override_kind == "absolute" else "docker-agent" + monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", override) + run = Mock(return_value=subprocess.CompletedProcess([], 0, stdout="done", stderr="")) + monkeypatch.setattr(subprocess, "run", run) + + result = DockerAgentIntegration().dispatch_command( + "speckit.plan", + project_root=tmp_path, + stream=False, + integration_args=["./agent.yaml"], + ) + + assert result == {"exit_code": 0, "stdout": "done", "stderr": ""} + assert run.call_args.args[0][0] == str(executable) + run.assert_called_once() + + +@pytest.mark.parametrize("override", ["docker-agent", " docker-agent "]) +def test_explicit_default_named_override_bypasses_automatic_discovery(monkeypatch, override): + monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", override) + finder = Mock(return_value=None) + monkeypatch.setattr("specify_cli._utils._find_docker_executable", finder) + run = Mock() + monkeypatch.setattr(subprocess, "run", run) + + args = DockerAgentIntegration().build_exec_args( + "prompt", output_json=False, integration_args=["./agent.yaml"] + ) + + assert args == ["docker-agent", "run", "--exec", "./agent.yaml", "--", "prompt"] + finder.assert_not_called() + run.assert_not_called() + + +@pytest.mark.parametrize("override", [None, "", " "]) +def test_unset_or_blank_override_still_uses_restricted_discovery(monkeypatch, override): + if override is None: + monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", raising=False) + else: + monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", override) + finder = Mock(return_value=None) + monkeypatch.setattr("specify_cli._utils._find_docker_executable", finder) + run = Mock() + monkeypatch.setattr(subprocess, "run", run) + + assert DockerAgentIntegration().build_exec_args( + "prompt", integration_args=["./agent.yaml"] + ) is None + assert [call.args[0] for call in finder.call_args_list] == ["docker-agent", "docker"] + run.assert_not_called() + + +def test_dispatch_stops_after_failed_plugin_probe(tmp_path, monkeypatch): + executable = tmp_path / ("docker.exe" if os.name == "nt" else "docker") + monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", raising=False) + monkeypatch.setattr( + "specify_cli._utils._find_docker_executable", + lambda name, **_kwargs: str(executable) if name == "docker" else None, + ) + run = Mock(return_value=subprocess.CompletedProcess([], 1)) + monkeypatch.setattr(subprocess, "run", run) + + with pytest.raises(NotImplementedError, match="does not support CLI dispatch"): + DockerAgentIntegration().dispatch_command( + "speckit.plan", + stream=False, + integration_args=["./agent.yaml"], + ) + + run.assert_called_once_with( + [str(executable), "agent", "version"], + capture_output=True, + check=False, + timeout=5, + ) + + +@pytest.mark.parametrize("name", ["docker", "docker-agent"]) +@pytest.mark.parametrize("stream", [False, True]) +def test_dispatch_excludes_runtime_project_from_another_cwd( + tmp_path, monkeypatch, name, stream +): + launch = tmp_path / "launch" + launch.mkdir() + project = tmp_path / "project" + local_bin = project / "bin" + local_bin.mkdir(parents=True) + executable = local_bin / (f"{name}.exe" if os.name == "nt" else name) + executable.touch() + executable.chmod(0o755) + monkeypatch.chdir(launch) + monkeypatch.setenv("PATH", str(local_bin)) + monkeypatch.setenv("PATHEXT", ".exe") + monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", raising=False) + run = Mock(return_value=subprocess.CompletedProcess([], 0, stdout="", stderr="")) + monkeypatch.setattr(subprocess, "run", run) + + with pytest.raises(NotImplementedError, match="does not support CLI dispatch"): + DockerAgentIntegration().dispatch_command( + "speckit.plan", + project_root=project, + stream=stream, + integration_args=["./agent.yaml"], + ) + + run.assert_not_called() + + +@pytest.mark.parametrize("name", ["docker", "docker-agent"]) +def test_dispatch_from_another_cwd_uses_external_install(tmp_path, monkeypatch, name): + launch = tmp_path / "launch" + launch.mkdir() + project = tmp_path / "project" + local_bin = project / "bin" + local_bin.mkdir(parents=True) + installed = tmp_path / "installed" + installed.mkdir() + filename = f"{name}.exe" if os.name == "nt" else name + for directory in (local_bin, installed): + executable = directory / filename + executable.touch() + executable.chmod(0o755) + monkeypatch.chdir(launch) + monkeypatch.setenv("PATH", os.pathsep.join([str(local_bin), str(installed)])) + monkeypatch.setenv("PATHEXT", ".exe") + monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", raising=False) + run = Mock(return_value=subprocess.CompletedProcess([], 0, stdout="done", stderr="")) + monkeypatch.setattr(subprocess, "run", run) + + result = DockerAgentIntegration().dispatch_command( + "speckit.plan", + project_root=project, + stream=False, + integration_args=["./agent.yaml"], + ) + + assert result == {"exit_code": 0, "stdout": "done", "stderr": ""} + assert run.call_args.args[0][0] == str(installed / filename) + assert run.call_args.kwargs["cwd"] == str(project) + assert run.call_count == (2 if name == "docker" else 1) + + +def test_unavailable_executable_cannot_build_dispatch_args(monkeypatch): + monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", raising=False) + monkeypatch.setenv("PATH", "") + + assert DockerAgentIntegration().build_exec_args( + "prompt", integration_args=["./agent.yaml"] + ) is None diff --git a/tests/specify_cli/test_check_tool.py b/tests/specify_cli/test_check_tool.py index 0d82e84564..76dbb0dcb0 100644 --- a/tests/specify_cli/test_check_tool.py +++ b/tests/specify_cli/test_check_tool.py @@ -117,11 +117,11 @@ def fake_which(name): def test_docker_agent_plugin_fallback(self): """docker-agent should detect a working Docker CLI plugin form.""" - def fake_which(name): + def fake_which(name, **_kwargs): return "/usr/bin/docker" if name == "docker" else None with ( - patch("shutil.which", side_effect=fake_which), + patch("specify_cli._utils._find_docker_executable", side_effect=fake_which), patch("subprocess.run") as run, ): run.return_value.returncode = 0 @@ -135,16 +135,16 @@ def fake_which(name): def test_docker_agent_missing(self): """docker-agent should be missing when neither form is installed.""" - with patch("shutil.which", return_value=None): + with patch("specify_cli._utils._find_docker_executable", return_value=None): assert check_tool("docker-agent") is False def test_docker_agent_plugin_missing(self): """Plain Docker CLI should not count as Docker Agent.""" - def fake_which(name): + def fake_which(name, **_kwargs): return "/usr/bin/docker" if name == "docker" else None with ( - patch("shutil.which", side_effect=fake_which), + patch("specify_cli._utils._find_docker_executable", side_effect=fake_which), patch("subprocess.run") as run, ): run.return_value.returncode = 1 diff --git a/tests/test_utils.py b/tests/test_utils.py index 567e0096aa..1e9eed7797 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,12 +1,25 @@ -"""Tests for specify_cli._utils.run_command.""" +"""Tests for command execution and Docker executable discovery.""" from __future__ import annotations +import errno import inspect +import os +import shutil +import subprocess +from unittest.mock import Mock import pytest -from specify_cli import run_command +from specify_cli import check_tool, run_command +from specify_cli._utils import docker_agent_command + + +def _symlink_or_skip(link, target, *, target_is_directory=False): + try: + link.symlink_to(target, target_is_directory=target_is_directory) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks are unavailable: {exc}") def test_run_command_has_no_shell_parameter(): @@ -19,3 +32,325 @@ def test_run_command_has_no_shell_parameter(): assert "shell" not in inspect.signature(run_command).parameters with pytest.raises(TypeError): run_command(["echo", "blocked"], shell=True) # type: ignore[call-arg] # noqa: S604 + + +@pytest.mark.parametrize("docker_installed", [False, True]) +def test_docker_probe_ignores_implicit_windows_current_directory( + tmp_path, monkeypatch, docker_installed +): + """Model Python 3.11's implicit Windows CWD search, even with a clean PATH.""" + project = tmp_path / "project" + project.mkdir() + installed = tmp_path / "installed" + installed.mkdir() + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", str(installed)) + monkeypatch.setenv("PATHEXT", ".EXE;.CMD") + monkeypatch.setattr("specify_cli._utils.sys.platform", "win32") + + def windows_which(command): + if command == "docker": + return str(project / "docker.exe") + if command == str(installed / "docker.EXE") and docker_installed: + return str(installed / "docker.exe") + return None + + monkeypatch.setattr(shutil, "which", windows_which) + run = Mock(return_value=subprocess.CompletedProcess([], 0)) + monkeypatch.setattr(subprocess, "run", run) + + assert check_tool("docker-agent") is docker_installed + if docker_installed: + run.assert_called_once_with( + [str(installed / "docker.exe"), "agent", "version"], + capture_output=True, + check=False, + timeout=5, + ) + else: + run.assert_not_called() + + +@pytest.mark.parametrize("name", ["docker", "docker-agent"]) +@pytest.mark.parametrize("entry", ["empty", "relative", "cwd", "child", "symlink"]) +def test_docker_discovery_skips_checkout_executables( + tmp_path, monkeypatch, name, entry +): + project = tmp_path / "project" + project.mkdir() + local_bin = project / "bin" + local_bin.mkdir() + installed = tmp_path / "installed" + installed.mkdir() + monkeypatch.setenv("PATHEXT", ".exe") + filename = f"{name}.exe" if os.name == "nt" else name + for directory in (project, local_bin, installed): + executable = directory / filename + executable.touch() + executable.chmod(0o755) + link = tmp_path / "linked-bin" + if entry == "symlink": + _symlink_or_skip(link, local_bin, target_is_directory=True) + entries = { + "empty": "", + "relative": "bin", + "cwd": str(project), + "child": str(local_bin), + "symlink": str(link), + } + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", os.pathsep.join([entries[entry], str(installed)])) + run = Mock(return_value=subprocess.CompletedProcess([], 0)) + monkeypatch.setattr(subprocess, "run", run) + + command = docker_agent_command() + + expected = str(installed / filename) + if name == "docker": + assert command == [expected, "agent", "run"] + run.assert_called_once_with( + [expected, "agent", "version"], + capture_output=True, + check=False, + timeout=5, + ) + else: + assert command == [expected, "run"] + run.assert_not_called() + + +@pytest.mark.parametrize("name", ["docker", "docker-agent"]) +def test_docker_discovery_rejects_executable_symlink_into_checkout( + tmp_path, monkeypatch, name +): + project = tmp_path / "project" + project.mkdir() + monkeypatch.setenv("PATHEXT", ".exe") + filename = f"{name}.exe" if os.name == "nt" else name + executable = project / filename + executable.touch() + executable.chmod(0o755) + installed = tmp_path / "installed" + installed.mkdir() + _symlink_or_skip(installed / filename, executable) + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", str(installed)) + run = Mock() + monkeypatch.setattr(subprocess, "run", run) + + assert docker_agent_command() is None + run.assert_not_called() + + +@pytest.mark.parametrize( + "failure", [1, OSError("unavailable"), subprocess.TimeoutExpired("docker", 5)] +) +def test_docker_plugin_probe_failures_are_unavailable(tmp_path, monkeypatch, failure): + project = tmp_path / "project" + project.mkdir() + installed = tmp_path / "installed" + installed.mkdir() + monkeypatch.setenv("PATHEXT", ".exe") + executable = installed / ("docker.exe" if os.name == "nt" else "docker") + executable.touch() + executable.chmod(0o755) + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", str(installed)) + run = Mock() + if isinstance(failure, Exception): + run.side_effect = failure + else: + run.return_value = subprocess.CompletedProcess([], failure) + monkeypatch.setattr(subprocess, "run", run) + + assert docker_agent_command() is None + run.assert_called_once() + + +def test_docker_discovery_with_empty_path_never_probes(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("PATH", "") + which = Mock() + run = Mock() + monkeypatch.setattr(shutil, "which", which) + monkeypatch.setattr(subprocess, "run", run) + + assert docker_agent_command() is None + which.assert_not_called() + run.assert_not_called() + + +def test_docker_discovery_uses_default_path_when_unset(tmp_path, monkeypatch): + project = tmp_path / "project" + project.mkdir() + installed = tmp_path / "installed" + installed.mkdir() + monkeypatch.setenv("PATHEXT", ".exe") + executable = installed / ("docker-agent.exe" if os.name == "nt" else "docker-agent") + executable.touch() + executable.chmod(0o755) + monkeypatch.chdir(project) + monkeypatch.delenv("PATH", raising=False) + monkeypatch.setattr(os, "defpath", str(installed)) + + assert docker_agent_command() == [str(executable), "run"] + + +@pytest.mark.parametrize("pathext", [None, ""]) +@pytest.mark.parametrize("name", ["docker", "docker-agent"]) +def test_docker_discovery_uses_default_windows_extensions( + tmp_path, monkeypatch, name, pathext +): + project = tmp_path / "project" + project.mkdir() + installed = tmp_path / "installed" + installed.mkdir() + executable = str(installed / f"{name}.EXE") + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", str(installed)) + if pathext is None: + monkeypatch.delenv("PATHEXT", raising=False) + else: + monkeypatch.setenv("PATHEXT", pathext) + monkeypatch.setattr("specify_cli._utils.sys.platform", "win32") + monkeypatch.setattr( + shutil, "which", lambda command: executable if command == executable else None + ) + run = Mock(return_value=subprocess.CompletedProcess([], 0)) + monkeypatch.setattr(subprocess, "run", run) + + expected = [executable, "agent", "run"] if name == "docker" else [executable, "run"] + assert docker_agent_command() == expected + if name == "docker": + run.assert_called_once() + else: + run.assert_not_called() + + +@pytest.mark.parametrize("name", ["docker", "docker-agent"]) +@pytest.mark.parametrize("entry", ["directory", "executable-symlink"]) +def test_docker_discovery_rejects_case_aliases_of_checkout( + tmp_path, monkeypatch, name, entry +): + project = tmp_path / "project" + project.mkdir() + alias = tmp_path / "PROJECT" + if not alias.exists() or not alias.samefile(project): + pytest.skip("requires a case-insensitive filesystem") + local_bin = project / "bin" + local_bin.mkdir() + filename = f"{name}.exe" if os.name == "nt" else name + executable = local_bin / filename + executable.touch() + executable.chmod(0o755) + search_directory = alias / "bin" + if entry == "executable-symlink": + search_directory = tmp_path / "installed" + search_directory.mkdir() + _symlink_or_skip(search_directory / filename, alias / "bin" / filename) + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", str(search_directory)) + monkeypatch.setenv("PATHEXT", ".exe") + run = Mock(return_value=subprocess.CompletedProcess([], 0)) + monkeypatch.setattr(subprocess, "run", run) + + assert docker_agent_command() is None + run.assert_not_called() + + +@pytest.mark.parametrize("name", ["docker", "docker-agent"]) +@pytest.mark.parametrize( + "stage", + ["directory-resolve", "directory-identity", "executable-resolve", "executable-identity", "which"], +) +@pytest.mark.parametrize( + "error", + [ + PermissionError("access denied"), + OSError(errno.ELOOP, "too many symbolic links"), + RuntimeError("Symlink loop from candidate"), + ], +) +def test_docker_discovery_skips_failed_candidates( + tmp_path, monkeypatch, name, stage, error +): + project = tmp_path / "project" + project.mkdir() + inaccessible = tmp_path / "inaccessible" + inaccessible.mkdir() + installed = tmp_path / "installed" + installed.mkdir() + filename = f"{name}.exe" if os.name == "nt" else name + for directory in (inaccessible, installed): + executable = directory / filename + executable.touch() + executable.chmod(0o755) + original_resolve = type(tmp_path).resolve + original_samefile = type(tmp_path).samefile + original_which = shutil.which + + def resolve(path, *args, **kwargs): + if ( + (stage == "directory-resolve" and path == inaccessible) + or (stage == "executable-resolve" and path == inaccessible / filename) + ): + raise error + return original_resolve(path, *args, **kwargs) + + def samefile(path, other): + if ( + (stage == "directory-identity" and path == inaccessible) + or (stage == "executable-identity" and path == inaccessible / filename) + ): + raise error + return original_samefile(path, other) + + def which(command): + if stage == "which" and command == str(inaccessible / filename): + raise error + return original_which(command) + + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", os.pathsep.join([str(inaccessible), str(installed)])) + monkeypatch.setenv("PATHEXT", ".exe") + monkeypatch.setattr(type(tmp_path), "resolve", resolve) + monkeypatch.setattr(type(tmp_path), "samefile", samefile) + monkeypatch.setattr(shutil, "which", which) + run = Mock(return_value=subprocess.CompletedProcess([], 0)) + monkeypatch.setattr(subprocess, "run", run) + + with pytest.warns(UserWarning, match="Skipping Docker executable search candidate"): + command = docker_agent_command() + + expected = str(installed / filename) + assert command == ([expected, "agent", "run"] if name == "docker" else [expected, "run"]) + if name == "docker": + assert run.call_args.args[0] == [expected, "agent", "version"] + run.assert_called_once() + else: + run.assert_not_called() + + +@pytest.mark.parametrize("name", ["docker", "docker-agent"]) +def test_docker_discovery_skips_cyclic_directory(tmp_path, monkeypatch, name): + project = tmp_path / "project" + project.mkdir() + loop = tmp_path / "loop" + _symlink_or_skip(loop, loop, target_is_directory=True) + installed = tmp_path / "installed" + installed.mkdir() + filename = f"{name}.exe" if os.name == "nt" else name + executable = installed / filename + executable.touch() + executable.chmod(0o755) + monkeypatch.chdir(project) + monkeypatch.setenv("PATH", os.pathsep.join([str(loop), str(installed)])) + monkeypatch.setenv("PATHEXT", ".exe") + run = Mock(return_value=subprocess.CompletedProcess([], 0)) + monkeypatch.setattr(subprocess, "run", run) + + with pytest.warns(UserWarning, match="Skipping Docker executable search candidate"): + command = docker_agent_command() + + expected = str(executable) + assert command == ([expected, "agent", "run"] if name == "docker" else [expected, "run"]) diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 2910f8b8ad..54ef832609 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -1859,6 +1859,10 @@ def fake_run(args, **kwargs): monkeypatch.delenv( "SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", raising=False ) + monkeypatch.setattr( + "specify_cli._utils._find_docker_executable", + lambda name, **_kwargs: "/usr/bin/docker-agent" if name == "docker-agent" else None, + ) monkeypatch.setattr( "shutil.which", lambda name: ( @@ -1957,6 +1961,10 @@ def fake_run(args, **kwargs): monkeypatch.delenv( "SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", raising=False ) + monkeypatch.setattr( + "specify_cli._utils._find_docker_executable", + lambda name, **_kwargs: "/usr/bin/docker-agent" if name == "docker-agent" else None, + ) monkeypatch.setattr( "shutil.which", lambda name: ( @@ -2429,6 +2437,31 @@ def test_dispatch_failure_returns_failed_status(self, tmp_path): class TestPromptStep: """Test the prompt step type.""" + def test_unavailable_docker_agent_returns_failed_result(self, tmp_path, monkeypatch): + from unittest.mock import Mock + from specify_cli.workflows.base import StepContext, StepStatus + from specify_cli.workflows.step.prompt import PromptStep + + monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", "./agent.yaml") + monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", raising=False) + monkeypatch.setenv("PATH", "") + run = Mock() + monkeypatch.setattr("subprocess.run", run) + + result = PromptStep().execute( + {"id": "review", "prompt": "Review changes", "integration": "docker-agent"}, + StepContext(project_root=str(tmp_path)), + ) + + assert result.status is StepStatus.FAILED + assert result.output["prompt"] == "Review changes" + assert result.output["integration"] == "docker-agent" + assert result.output["exit_code"] == 1 + assert result.output["dispatched"] is False + assert result.error is not None + assert "CLI not found or not installed" in result.error + run.assert_not_called() + def test_execute_basic(self): from unittest.mock import patch from specify_cli.workflows.step.prompt import PromptStep @@ -8332,6 +8365,40 @@ def test_execute_accepts_valid_and_null_dispatch_defaults( class TestContinueOnError: """Test the `continue_on_error` step-level field.""" + @pytest.mark.parametrize("continue_on_error", [False, True]) + def test_unavailable_docker_prompt_obeys_continue_on_error( + self, project_dir, monkeypatch, continue_on_error + ): + from specify_cli.workflows.base import RunStatus + from specify_cli.workflows.engine import WorkflowDefinition, WorkflowEngine + + monkeypatch.setenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", "./agent.yaml") + monkeypatch.delenv("SPECKIT_INTEGRATION_DOCKER_AGENT_EXECUTABLE", raising=False) + monkeypatch.setenv("PATH", "") + definition = WorkflowDefinition({ + "schema_version": "1.0", + "workflow": {"id": "missing-docker", "name": "Missing Docker", "version": "1.0.0"}, + "steps": [ + { + "id": "review", + "type": "prompt", + "prompt": "Review changes", + "integration": "docker-agent", + "continue_on_error": continue_on_error, + }, + {"id": "after", "type": "shell", "run": "echo did-run"}, + ], + }) + + state = WorkflowEngine(project_dir).execute(definition) + + expected_status = RunStatus.COMPLETED if continue_on_error else RunStatus.FAILED + assert state.status is expected_status + assert state.step_results["review"]["status"] == "failed" + assert state.step_results["review"]["output"]["dispatched"] is False + assert "CLI not found or not installed" in state.step_results["review"]["error"] + assert ("after" in state.step_results) is continue_on_error + def test_undeclared_failure_halts_run(self, project_dir): """Default behaviour (no `continue_on_error`): a failing step halts the workflow run with `status == StepStatus.FAILED`. @@ -11686,6 +11753,10 @@ def fake_run(args, **kwargs): monkeypatch.delenv( "SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", raising=False ) + monkeypatch.setattr( + "specify_cli._utils._find_docker_executable", + lambda name, **_kwargs: "/usr/bin/docker-agent" if name == "docker-agent" else None, + ) monkeypatch.setattr( "shutil.which", lambda name: ( @@ -11736,6 +11807,10 @@ def test_resume_does_not_mix_new_integration_with_old_runtime_config( monkeypatch.delenv( "SPECKIT_INTEGRATION_DOCKER_AGENT_EXTRA_ARGS", raising=False ) + monkeypatch.setattr( + "specify_cli._utils._find_docker_executable", + lambda name, **_kwargs: "/usr/bin/docker-agent" if name == "docker-agent" else None, + ) monkeypatch.setattr( "shutil.which", lambda name: (