Skip to content

Commit dd11548

Browse files
dhruv-15-03Copilot
andcommitted
Add cli_executable property to IntegrationBase for agents whose executable differs from their key
Closes #2558. - Add cli_executable property and is_cli_available() method to IntegrationBase. cli_executable delegates to the existing _resolve_executable() hook (key, or SPECKIT_INTEGRATION_<KEY>_EXECUTABLE override); is_cli_available() defaults to shutil.which(cli_executable). - Refactor check_tool() in _utils.py to delegate to get_integration(tool).is_cli_available() when the tool is a registered integration, removing the hardcoded claude/kiro-cli/rovodev special cases. Falls back to a plain shutil.which(tool) for unregistered tools (git, code, etc). - Override is_cli_available() in ClaudeIntegration (checks local install paths before falling back to PATH) and KiroCliIntegration (accepts both kiro-cli and legacy kiro binary names). RovoDev needs no change: its existing _resolve_executable() override already returns �cli, so cli_executable picks it up automatically. - Scope note: the two workflow-step CLI-dispatch sites (workflows/steps/command, workflows/steps/prompt) are intentionally left unchanged in this PR. They already handle differing executables via an existing impl.key / exec_args[0] fallback, and tests/test_workflows.py has 15+ test cases patching shutil.which at those specific module paths; migrating them to is_cli_available() is deferred to a fast-follow to avoid unrelated test churn here. - Document the new override mechanism in AGENTS.md, with the pitfall fully explained (RovoDev's key=rovodev vs executable=acli). - Add TestCliExecutableDetection to tests/integrations/test_base.py covering default resolution, env-var override, and is_cli_available() true/false paths. All 9 existing tests/test_check_tool.py cases and 160+ integration tests pass unmodified. Assisted-by: GitHub Copilot (model: claude-sonnet-5, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent c0fe0e4 commit dd11548

6 files changed

Lines changed: 130 additions & 23 deletions

File tree

‎AGENTS.md‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,22 @@ class CodexIntegration(SkillsIntegration):
151151

152152
**Key design rule:** For CLI-based integrations (`requires_cli: True`), `key` must be the actual executable name (e.g., `"cursor-agent"` not `"cursor"`). This ensures `shutil.which(key)` works for CLI-tool checks without special-case mappings. IDE-based integrations (`requires_cli: False`) should use their canonical identifier (e.g., `"kilocode"`, `"copilot"`).
153153

154+
**When `key` and the CLI executable genuinely differ** (issue [#2558](https://github.com/github/spec-kit/issues/2558)): sometimes the executable name legitimately cannot match `key` — e.g. RovoDev's `key` is `"rovodev"` but it's invoked via the `acli` binary (`acli rovodev …`). Don't hack around this — override `IntegrationBase`'s `cli_executable` property (or its underlying `_resolve_executable()` hook) instead:
155+
156+
```python
157+
class RovodevIntegration(SkillsIntegration):
158+
key = "rovodev"
159+
...
160+
161+
def _resolve_executable(self) -> str:
162+
# cli_executable delegates here by default; override the fallback
163+
# instead of self.key while still honoring SPECKIT_INTEGRATION_ROVODEV_EXECUTABLE.
164+
env_name = f"SPECKIT_INTEGRATION_{self.key.upper().replace('-', '_')}_EXECUTABLE"
165+
return os.environ.get(env_name, "").strip() or "acli"
166+
```
167+
168+
`check_tool()` in `_utils.py` detects installed CLIs by calling `get_integration(tool).is_cli_available()`, which defaults to `shutil.which(self.cli_executable) is not None`. Override `is_cli_available()` directly (rather than just `cli_executable`) when detection needs more than a single PATH lookup — e.g. `ClaudeIntegration` also checks local install paths (`~/.claude/local/claude`, npm-local), and `KiroCliIntegration` accepts both `kiro-cli` and the legacy `kiro` binary name. Tools with no registered integration (e.g. `"git"`) fall back to a plain `shutil.which(tool)` check.
169+
154170
### 3. Register it
155171

156172
In `src/specify_cli/integrations/__init__.py`, add one import and one `_register()` call inside `_register_builtins()`. Both lists are alphabetical:
@@ -502,7 +518,7 @@ Disclosure is **continuous**, not a one-time event. A single AI-disclosure parag
502518
503519
## Common Pitfalls
504520
505-
1. **Using shorthand keys for CLI-based integrations**: For CLI-based integrations (`requires_cli: True`), the `key` must match the executable name (e.g., `"cursor-agent"` not `"cursor"`). `shutil.which(key)` is used for CLI tool checks — mismatches require special-case mappings. IDE-based integrations (`requires_cli: False`) are not subject to this constraint.
521+
1. **Using shorthand keys for CLI-based integrations**: For CLI-based integrations (`requires_cli: True`), the `key` must match the executable name (e.g., `"cursor-agent"` not `"cursor"`) whenever possible. `shutil.which(key)` is used for CLI tool checks by default. If the executable genuinely can't match `key` (e.g. RovoDev's `key="rovodev"` but binary is `acli`), override `cli_executable` / `_resolve_executable()` — or `is_cli_available()` for multi-path detection — instead of adding a hardcoded special case to `check_tool()` (see [#2558](https://github.com/github/spec-kit/issues/2558)). IDE-based integrations (`requires_cli: False`) are not subject to this constraint.
506522
2. **Reintroducing context handling into the CLI**: The opt-in `agent-context` extension owns everything about context files — including the per-agent default mapping in `agent-context-defaults.json`. Integration classes must **not** declare a `context_file`, and no CLI code should read, write, resolve, or migrate context files. All context-file logic lives in `.specify/extensions/agent-context/` and its bundled scripts.
507523
3. **Incorrect `requires_cli` value**: Set to `True` only for agents that have a CLI tool; set to `False` for IDE-based agents.
508524
4. **Wrong argument format**: Use `$ARGUMENTS` for Markdown agents, `{{args}}` for TOML agents.

‎src/specify_cli/_utils.py‎

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -104,28 +104,17 @@ def check_tool(tool: str, tracker=None) -> bool:
104104
Returns:
105105
True if tool is found, False otherwise
106106
"""
107-
# Special handling for Claude CLI local installs
108-
# See: https://github.com/github/spec-kit/issues/123
109-
# See: https://github.com/github/spec-kit/issues/550
110-
# Claude Code can be installed in two local paths:
111-
# 1. ~/.claude/local/claude (after `claude migrate-installer`)
112-
# 2. ~/.claude/local/node_modules/.bin/claude (npm-local install, e.g. via nvm)
113-
# Neither path may be on the system PATH, so we check them explicitly.
114-
if tool == "claude":
115-
if CLAUDE_LOCAL_PATH.is_file() or CLAUDE_NPM_LOCAL_PATH.is_file():
116-
if tracker:
117-
tracker.complete(tool, "available")
118-
return True
119-
120-
# Per-integration executable resolution.
121-
if tool == "kiro-cli":
122-
# Kiro currently supports both executable names. Prefer kiro-cli and
123-
# accept kiro as a compatibility fallback.
124-
found = shutil.which("kiro-cli") is not None or shutil.which("kiro") is not None
125-
elif tool == "rovodev":
126-
found = shutil.which("acli") is not None
127-
else:
128-
found = shutil.which(tool) is not None
107+
# Integrations declare their own CLI-detection contract via
108+
# `IntegrationBase.is_cli_available()` (see issue #2558), which
109+
# subsumes what used to be hardcoded special cases here for Claude's
110+
# non-PATH local installs, kiro-cli's dual executable names, and
111+
# rovodev's `acli`-backed dispatch. Fall back to a plain `shutil.which`
112+
# for tool names that are not registered integrations (e.g. "git",
113+
# "code", "code-insiders").
114+
from .integrations import get_integration
115+
116+
impl = get_integration(tool)
117+
found = impl.is_cli_available() if impl is not None else shutil.which(tool) is not None
129118

130119
if tracker:
131120
if found:

‎src/specify_cli/integrations/base.py‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,41 @@ def _resolve_executable(self) -> str:
260260
override = os.environ.get(env_name, "").strip()
261261
return override if override else self.key
262262

263+
@property
264+
def cli_executable(self) -> str:
265+
"""Executable name used to detect this integration's CLI tool.
266+
267+
Defaults to whatever ``_resolve_executable()`` returns (the
268+
``SPECKIT_INTEGRATION_<KEY>_EXECUTABLE`` override, else ``self.key``),
269+
so integrations whose executable differs from their key only need to
270+
override ``_resolve_executable()`` — as ``RovodevIntegration`` already
271+
does for ``acli`` — to get correct CLI detection for free.
272+
273+
Integrations that need to accept more than one candidate binary name,
274+
or check non-``PATH`` install locations, should override
275+
``is_cli_available()`` instead of (or in addition to) this property.
276+
277+
See issue #2558.
278+
"""
279+
return self._resolve_executable()
280+
281+
def is_cli_available(self) -> bool:
282+
"""Return whether this integration's CLI tool is installed.
283+
284+
The default implementation checks ``shutil.which(self.cli_executable)``.
285+
Detection call sites (``check_tool()``, workflow command/prompt
286+
dispatch) should call this instead of hardcoding
287+
``shutil.which(self.key)`` or maintaining a per-agent special case.
288+
289+
Override for agents whose detection can't be expressed as a single
290+
executable name — e.g. ``KiroCliIntegration`` accepts either
291+
``kiro-cli`` or the legacy ``kiro`` binary, and Claude Code also
292+
checks non-``PATH`` local install locations.
293+
294+
See issue #2558.
295+
"""
296+
return shutil.which(self.cli_executable) is not None
297+
263298
def _apply_extra_args_env_var(self, args: list[str]) -> None:
264299
"""Append `SPECKIT_INTEGRATION_<KEY>_EXTRA_ARGS` env-var value to *args*.
265300

‎src/specify_cli/integrations/claude/__init__.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from typing import Any
66

77
from ..base import SkillsIntegration
8+
from ... import _utils
89
from ..._utils import dump_frontmatter
910

1011
# Mapping of command template stem → argument-hint text shown inline
@@ -54,6 +55,22 @@ class ClaudeIntegration(SkillsIntegration):
5455
}
5556
multi_install_safe = True
5657

58+
def is_cli_available(self) -> bool:
59+
"""Claude Code can be installed in two local paths that may not be
60+
on the system ``PATH``:
61+
62+
1. ``~/.claude/local/claude`` (after ``claude migrate-installer``)
63+
2. ``~/.claude/local/node_modules/.bin/claude`` (npm-local install,
64+
e.g. via nvm)
65+
66+
Checked here (rather than a hardcoded special case in
67+
``check_tool()``) so any future detection call site gets the same
68+
behavior for free. See issues #123, #550, #2558.
69+
"""
70+
if _utils.CLAUDE_LOCAL_PATH.is_file() or _utils.CLAUDE_NPM_LOCAL_PATH.is_file():
71+
return True
72+
return super().is_cli_available()
73+
5774
@staticmethod
5875
def inject_argument_hint(content: str, hint: str) -> str:
5976
"""Insert ``argument-hint`` after the first ``description:`` in YAML frontmatter.

‎src/specify_cli/integrations/kiro_cli/__init__.py‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
"""Kiro CLI integration."""
22

3+
import shutil
4+
35
from ..base import MarkdownIntegration
46

57

@@ -34,3 +36,14 @@ class KiroCliIntegration(MarkdownIntegration):
3436
"args": _KIRO_ARG_FALLBACK,
3537
"extension": ".md",
3638
}
39+
40+
def is_cli_available(self) -> bool:
41+
"""Kiro currently supports both executable names.
42+
43+
Prefer ``kiro-cli`` and accept the legacy ``kiro`` binary as a
44+
compatibility fallback (see issue #2558).
45+
"""
46+
return (
47+
shutil.which(self.cli_executable) is not None
48+
or shutil.which("kiro") is not None
49+
)

‎tests/integrations/test_base.py‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import shlex
44
import sys
55
from types import SimpleNamespace
6+
from unittest.mock import patch
67

78
import pytest
89

@@ -97,6 +98,42 @@ def test_uninstall_delegates_to_teardown(self, tmp_path):
9798
assert skipped == []
9899

99100

101+
class TestCliExecutableDetection:
102+
"""cli_executable / is_cli_available() — issue #2558."""
103+
104+
def test_cli_executable_defaults_to_key(self):
105+
i = StubIntegration()
106+
assert i.cli_executable == "stub"
107+
108+
def test_cli_executable_honors_executable_env_override(self, monkeypatch):
109+
monkeypatch.setenv("SPECKIT_INTEGRATION_STUB_EXECUTABLE", "stub-bin")
110+
i = StubIntegration()
111+
assert i.cli_executable == "stub-bin"
112+
113+
def test_is_cli_available_true_when_executable_on_path(self):
114+
i = StubIntegration()
115+
with patch("specify_cli.integrations.base.shutil.which", return_value="/usr/bin/stub"):
116+
assert i.is_cli_available() is True
117+
118+
def test_is_cli_available_false_when_not_on_path(self):
119+
i = StubIntegration()
120+
with patch("specify_cli.integrations.base.shutil.which", return_value=None):
121+
assert i.is_cli_available() is False
122+
123+
def test_is_cli_available_uses_cli_executable_not_key(self, monkeypatch):
124+
"""An integration overriding cli_executable should be detected by
125+
that name, not by ``self.key`` (mirrors RovoDev: key='rovodev',
126+
executable='acli')."""
127+
monkeypatch.setenv("SPECKIT_INTEGRATION_STUB_EXECUTABLE", "stub-bin")
128+
i = StubIntegration()
129+
130+
def fake_which(name):
131+
return "/usr/bin/stub-bin" if name == "stub-bin" else None
132+
133+
with patch("specify_cli.integrations.base.shutil.which", side_effect=fake_which):
134+
assert i.is_cli_available() is True
135+
136+
100137
class TestMarkdownIntegration:
101138
def test_is_subclass_of_base(self):
102139
assert issubclass(MarkdownIntegration, IntegrationBase)

0 commit comments

Comments
 (0)