Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/specify_cli/integrations/agy/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ def build_exec_args(
output_json: bool = True,
) -> list[str] | None:
# agy does not support --model or JSON output; both params are ignored
return ["agy", "--print", prompt]
return [self._resolve_executable(), "--print", prompt]

def setup(
self,
Expand Down
28 changes: 25 additions & 3 deletions src/specify_cli/integrations/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,28 @@ def build_exec_args(
"""
return None

def _resolve_executable(self) -> str:
"""Return the executable for this integration's CLI tool.

Checks ``SPECKIT_INTEGRATION_<KEY>_EXECUTABLE`` first, allowing
operators to override the binary path without modifying the
integration configuration — useful when the tool is installed in
a non-standard location or a specific version must be pinned.
Hyphens in the integration key are replaced with underscores and
the key is uppercased so that, for example, ``kiro-cli`` maps to
``SPECKIT_INTEGRATION_KIRO_CLI_EXECUTABLE``.

Falls back to ``self.key`` when the env var is unset or
whitespace-only so existing behaviour is unchanged.

See issue #2596.
"""
env_name = (
f"SPECKIT_INTEGRATION_{self.key.upper().replace('-', '_')}_EXECUTABLE"
)
override = os.environ.get(env_name, "").strip()
return override if override else self.key

def _apply_extra_args_env_var(self, args: list[str]) -> None:
"""Append `SPECKIT_INTEGRATION_<KEY>_EXTRA_ARGS` env-var value to *args*.

Expand Down Expand Up @@ -895,7 +917,7 @@ def build_exec_args(
) -> list[str] | None:
if not self.config or not self.config.get("requires_cli"):
return None
args = [self.key, "-p", prompt]
args = [self._resolve_executable(), "-p", prompt]
self._apply_extra_args_env_var(args)
if model:
args.extend(["--model", model])
Expand Down Expand Up @@ -983,7 +1005,7 @@ def build_exec_args(
) -> list[str] | None:
if not self.config or not self.config.get("requires_cli"):
return None
args = [self.key, "-p", prompt]
args = [self._resolve_executable(), "-p", prompt]
self._apply_extra_args_env_var(args)
if model:
args.extend(["-m", model])
Expand Down Expand Up @@ -1402,7 +1424,7 @@ def build_exec_args(
) -> list[str] | None:
if not self.config or not self.config.get("requires_cli"):
return None
args = [self.key, "-p", prompt]
args = [self._resolve_executable(), "-p", prompt]
self._apply_extra_args_env_var(args)
if model:
args.extend(["--model", model])
Expand Down
8 changes: 3 additions & 5 deletions src/specify_cli/integrations/codex/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,9 @@ def build_exec_args(
output_json: bool = True,
) -> list[str] | None:
# Codex uses ``codex exec "prompt"`` for non-interactive mode.
# Use ``self.key`` so the executable name stays coupled to the
# env-var lookup (which also derives from ``self.key``), matching
# the pattern in Devin/Opencode and avoiding drift if the key
# ever changes.
args: list[str] = [self.key, "exec", prompt]
# Resolve argv[0] via the shared executable resolver so operators can
# override the binary with SPECKIT_INTEGRATION_CODEX_EXECUTABLE.
args: list[str] = [self._resolve_executable(), "exec", prompt]
self._apply_extra_args_env_var(args)
if model:
args.extend(["--model", model])
Expand Down
16 changes: 14 additions & 2 deletions src/specify_cli/integrations/copilot/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,18 @@ def options(cls) -> list[IntegrationOption]:
),
]

def _resolve_executable(self) -> str:
"""Return the Copilot CLI executable, respecting the env-var override.

Checks ``SPECKIT_INTEGRATION_COPILOT_EXECUTABLE`` first. Falls
back to the platform-specific default from ``_copilot_executable()``
(``copilot.cmd`` on Windows, ``copilot`` elsewhere) so that
existing behaviour is preserved when the env var is unset.
"""
env_name = "SPECKIT_INTEGRATION_COPILOT_EXECUTABLE"
override = os.environ.get(env_name, "").strip()
return override if override else _copilot_executable()

def build_exec_args(
self,
prompt: str,
Expand All @@ -148,7 +160,7 @@ def build_exec_args(
# Controlled by SPECKIT_COPILOT_ALLOW_ALL_TOOLS env var
# (default: enabled). The deprecated SPECKIT_ALLOW_ALL_TOOLS
# is also honoured as a fallback.
args = [_copilot_executable(), "-p", prompt]
args = [self._resolve_executable(), "-p", prompt]
self._apply_extra_args_env_var(args)
if _allow_all():
args.append("--yolo")
Expand Down Expand Up @@ -217,7 +229,7 @@ def dispatch_command(
agent_name = f"speckit.{stem}"
prompt = args or ""

cli_args = [_copilot_executable(), "-p", prompt]
cli_args = [self._resolve_executable(), "-p", prompt]
# Honour SPECKIT_INTEGRATION_COPILOT_EXTRA_ARGS for real workflow
# runs. `dispatch_command` builds cli_args inline rather than
# going through `build_exec_args`, so the hook must be invoked
Expand Down
2 changes: 1 addition & 1 deletion src/specify_cli/integrations/devin/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def build_exec_args(
stdout instead of structured JSON. ``requires_cli=True`` is
kept on the integration for tool detection.
"""
args = [self.key, "-p", prompt]
args = [self._resolve_executable(), "-p", prompt]
self._apply_extra_args_env_var(args)
if model:
args.extend(["--model", model])
Expand Down
2 changes: 1 addition & 1 deletion src/specify_cli/integrations/hermes/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ def build_exec_args(
mapping slash-command invocations to the appropriate skill-based
dispatch.
"""
args = [self.key, "chat", "-Q"]
args = [self._resolve_executable(), "chat", "-Q"]

if model:
args.extend(["-m", model])
Expand Down
2 changes: 1 addition & 1 deletion src/specify_cli/integrations/opencode/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def build_exec_args(
model: str | None = None,
output_json: bool = True,
) -> list[str] | None:
args = [self.key, "run"]
args = [self._resolve_executable(), "run"]
# Apply operator-injected extra args before the prompt-derived
# --command and the canonical --format/-m flags so Spec Kit's
# later appends remain authoritative under repeated-flag CLI
Expand Down
182 changes: 170 additions & 12 deletions tests/integrations/test_extra_args.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
"""Tests for the per-integration `SPECKIT_INTEGRATION_<KEY>_EXTRA_ARGS` env-var hook.
"""Tests for the per-integration `SPECKIT_INTEGRATION_<KEY>_EXTRA_ARGS` and
`SPECKIT_INTEGRATION_<KEY>_EXECUTABLE` env-var hooks.

The hook is implemented in `IntegrationBase._apply_extra_args_env_var`
and wired into every concrete `build_exec_args` —
`MarkdownIntegration`, `TomlIntegration`, `SkillsIntegration`, plus the
overrides in Codex, Devin, Opencode and Copilot. These tests cover both
the shared mechanism (via `SkillsIntegration` stubs near the top of the
file) and each override integration end-to-end (further down). See
issue #2595."""
The hooks are implemented in `IntegrationBase._apply_extra_args_env_var` and
`IntegrationBase._resolve_executable` and wired into every concrete
`build_exec_args` — `MarkdownIntegration`, `TomlIntegration`,
`SkillsIntegration`, plus override integrations.
These tests cover both the shared mechanisms (via `SkillsIntegration` stubs
near the top of the file) and override integrations end-to-end (further down).
See issues #2595 and #2596."""

import os

Expand Down Expand Up @@ -128,11 +129,12 @@ class _TomlAgentStub(TomlIntegration):

@pytest.fixture(autouse=True)
def _clean_extra_args_env(monkeypatch):
"""Strip any leaked SPECKIT_INTEGRATION_*_EXTRA_ARGS from the test
env so a developer's shell setting doesn't pollute results."""
"""Strip any leaked SPECKIT_INTEGRATION_*_EXTRA_ARGS and
SPECKIT_INTEGRATION_*_EXECUTABLE vars from the test env so a
developer's shell setting doesn't pollute results."""
for key in list(os.environ):
if key.startswith("SPECKIT_INTEGRATION_") and key.endswith(
"_EXTRA_ARGS"
if key.startswith("SPECKIT_INTEGRATION_") and (
key.endswith("_EXTRA_ARGS") or key.endswith("_EXECUTABLE")
):
monkeypatch.delenv(key, raising=False)

Expand Down Expand Up @@ -478,3 +480,159 @@ def test_codex_dispatch_command_includes_extra_args(monkeypatch):
assert capture.captured_args is not None
assert "--sandbox" in capture.captured_args
assert "read-only" in capture.captured_args


# ---------------------------------------------------------------------------
# SPECKIT_INTEGRATION_<KEY>_EXECUTABLE tests
#
# The `_resolve_executable()` method on `IntegrationBase` checks
# `SPECKIT_INTEGRATION_<KEY>_EXECUTABLE` and, when set, substitutes that
# value for `self.key` as the first token in argv. The tests below lock
# the behaviour across shared and override integration paths:
# - the shared SkillsIntegration/MarkdownIntegration/TomlIntegration bases,
# - representative override integrations,
# - the hyphen→underscore key normalisation, and
# - whitespace/unset no-op guarantee.
# ---------------------------------------------------------------------------


def test_executable_env_var_unset_uses_key():
"""Default: no override → executable is the integration key."""
args = _ClaudeStub().build_exec_args("p")
assert args[0] == "claude"


def test_executable_env_var_replaces_first_argv_token(monkeypatch):
"""Setting the env var substitutes the executable name in argv."""
monkeypatch.setenv("SPECKIT_INTEGRATION_CLAUDE_EXECUTABLE", "/opt/claude/bin/claude")
args = _ClaudeStub().build_exec_args("hello")
assert args[0] == "/opt/claude/bin/claude"
assert args[1:] == ["-p", "hello", "--output-format", "json"]


def test_executable_env_var_whitespace_only_falls_back_to_key(monkeypatch):
"""Whitespace-only value is treated as unset → falls back to self.key."""
monkeypatch.setenv("SPECKIT_INTEGRATION_CLAUDE_EXECUTABLE", " ")
args = _ClaudeStub().build_exec_args("p")
assert args[0] == "claude"


def test_executable_env_var_key_normalization_hyphen_to_underscore(monkeypatch):
"""`kiro-cli` key maps to `SPECKIT_INTEGRATION_KIRO_CLI_EXECUTABLE`."""
monkeypatch.setenv("SPECKIT_INTEGRATION_KIRO_CLI_EXECUTABLE", "/usr/local/bin/kiro-cli")
args = _KiroCliStub().build_exec_args("p")
assert args[0] == "/usr/local/bin/kiro-cli"


def test_executable_env_var_other_integration_ignored(monkeypatch):
"""`SPECKIT_INTEGRATION_GEMINI_EXECUTABLE` must NOT affect Claude."""
monkeypatch.setenv("SPECKIT_INTEGRATION_GEMINI_EXECUTABLE", "/custom/gemini")
args = _ClaudeStub().build_exec_args("p")
assert args[0] == "claude"


def test_executable_env_var_markdown_integration(monkeypatch):
"""MarkdownIntegration base honours the executable env var."""
monkeypatch.setenv("SPECKIT_INTEGRATION_MD_AGENT_EXECUTABLE", "/custom/md-agent")
args = _MarkdownAgentStub().build_exec_args("p")
assert args[0] == "/custom/md-agent"


def test_executable_env_var_toml_integration(monkeypatch):
"""TomlIntegration base honours the executable env var."""
monkeypatch.setenv("SPECKIT_INTEGRATION_TOML_AGENT_EXECUTABLE", "/custom/toml-agent")
args = _TomlAgentStub().build_exec_args("p")
assert args[0] == "/custom/toml-agent"


def test_executable_env_var_requires_cli_false_returns_none(monkeypatch):
"""`requires_cli: False` still returns None even when executable is set."""
monkeypatch.setenv("SPECKIT_INTEGRATION_NO_CLI_EXECUTABLE", "/custom/no-cli")
assert _NoCliStub().build_exec_args("p") is None


def test_executable_env_var_codex_integration(monkeypatch):
"""CodexIntegration honours the executable env var."""
from specify_cli.integrations.codex import CodexIntegration

monkeypatch.setenv("SPECKIT_INTEGRATION_CODEX_EXECUTABLE", "/opt/codex")
args = CodexIntegration().build_exec_args("p")
assert args[0] == "/opt/codex"
assert args[1] == "exec"


def test_executable_env_var_devin_integration(monkeypatch):
"""DevinIntegration honours the executable env var."""
from specify_cli.integrations.devin import DevinIntegration

monkeypatch.setenv("SPECKIT_INTEGRATION_DEVIN_EXECUTABLE", "/opt/devin")
args = DevinIntegration().build_exec_args("p")
assert args[0] == "/opt/devin"


def test_executable_env_var_opencode_integration(monkeypatch):
"""OpencodeIntegration honours the executable env var."""
from specify_cli.integrations.opencode import OpencodeIntegration

monkeypatch.setenv("SPECKIT_INTEGRATION_OPENCODE_EXECUTABLE", "/opt/opencode")
args = OpencodeIntegration().build_exec_args("p")
assert args[0] == "/opt/opencode"
assert args[1] == "run"


def test_executable_env_var_copilot_integration(monkeypatch):
"""CopilotIntegration honours the executable env var, overriding the
platform-specific default from `_copilot_executable()`."""
from specify_cli.integrations.copilot import CopilotIntegration

monkeypatch.setenv("SPECKIT_INTEGRATION_COPILOT_EXECUTABLE", "/opt/copilot")
monkeypatch.setenv("SPECKIT_COPILOT_ALLOW_ALL_TOOLS", "0")
args = CopilotIntegration().build_exec_args("p")
assert args[0] == "/opt/copilot"


def test_executable_env_var_copilot_unset_uses_platform_default(monkeypatch):
"""When `SPECKIT_INTEGRATION_COPILOT_EXECUTABLE` is unset, Copilot
falls back to the platform-specific default from `_copilot_executable()`."""
from specify_cli.integrations.copilot import CopilotIntegration, _copilot_executable

monkeypatch.setenv("SPECKIT_COPILOT_ALLOW_ALL_TOOLS", "0")
args = CopilotIntegration().build_exec_args("p")
assert args[0] == _copilot_executable()


def test_executable_env_var_copilot_dispatch_command(monkeypatch):
"""CopilotIntegration.dispatch_command honours the executable env var."""
import subprocess

from specify_cli.integrations.copilot import CopilotIntegration

capture = _RunCapture()
monkeypatch.setattr(subprocess, "run", capture)
monkeypatch.setenv("SPECKIT_INTEGRATION_COPILOT_EXECUTABLE", "/opt/copilot")
monkeypatch.setenv("SPECKIT_COPILOT_ALLOW_ALL_TOOLS", "0")

CopilotIntegration().dispatch_command("speckit.plan", args="body", stream=False)

assert capture.captured_args is not None
assert capture.captured_args[0] == "/opt/copilot"


def test_executable_and_extra_args_both_honoured(monkeypatch):
"""Both the executable override and extra args env vars can be set
simultaneously — they are independent hooks."""
monkeypatch.setenv("SPECKIT_INTEGRATION_CLAUDE_EXECUTABLE", "/opt/claude")
monkeypatch.setenv(
"SPECKIT_INTEGRATION_CLAUDE_EXTRA_ARGS", "--dangerously-skip-permissions"
)
args = _ClaudeStub().build_exec_args("hello", model="sonnet")
assert args == [
"/opt/claude",
"-p",
"hello",
"--dangerously-skip-permissions",
"--model",
"sonnet",
"--output-format",
"json",
]
Loading