Skip to content
Open
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
18 changes: 17 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,22 @@ class CodexIntegration(SkillsIntegration):

**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"`).

**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:

```python
class RovodevIntegration(SkillsIntegration):
key = "rovodev"
...

def _resolve_executable(self) -> str:
# cli_executable delegates here by default; override the fallback
# instead of self.key while still honoring SPECKIT_INTEGRATION_ROVODEV_EXECUTABLE.
env_name = f"SPECKIT_INTEGRATION_{self.key.upper().replace('-', '_')}_EXECUTABLE"
return os.environ.get(env_name, "").strip() or "acli"
```

`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.

### 3. Register it

In `src/specify_cli/integrations/__init__.py`, add one import and one `_register()` call inside `_register_builtins()`. Both lists are alphabetical:
Expand Down Expand Up @@ -502,7 +518,7 @@ Disclosure is **continuous**, not a one-time event. A single AI-disclosure parag

## Common Pitfalls

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.
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.
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.
3. **Incorrect `requires_cli` value**: Set to `True` only for agents that have a CLI tool; set to `False` for IDE-based agents.
4. **Wrong argument format**: Use `$ARGUMENTS` for Markdown agents, `{{args}}` for TOML agents.
Expand Down
33 changes: 11 additions & 22 deletions src/specify_cli/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,28 +104,17 @@ def check_tool(tool: str, tracker=None) -> bool:
Returns:
True if tool is found, False otherwise
"""
# Special handling for Claude CLI local installs
# See: https://github.com/github/spec-kit/issues/123
# See: https://github.com/github/spec-kit/issues/550
# Claude Code can be installed in two local paths:
# 1. ~/.claude/local/claude (after `claude migrate-installer`)
# 2. ~/.claude/local/node_modules/.bin/claude (npm-local install, e.g. via nvm)
# Neither path may be on the system PATH, so we check them explicitly.
if tool == "claude":
if CLAUDE_LOCAL_PATH.is_file() or CLAUDE_NPM_LOCAL_PATH.is_file():
if tracker:
tracker.complete(tool, "available")
return True

# Per-integration executable resolution.
if tool == "kiro-cli":
# Kiro currently supports both executable names. Prefer kiro-cli and
# accept kiro as a compatibility fallback.
found = shutil.which("kiro-cli") is not None or shutil.which("kiro") is not None
elif tool == "rovodev":
found = shutil.which("acli") is not None
else:
found = shutil.which(tool) is not None
# Integrations declare their own CLI-detection contract via
# `IntegrationBase.is_cli_available()` (see issue #2558), which
# subsumes what used to be hardcoded special cases here for Claude's
# non-PATH local installs, kiro-cli's dual executable names, and
# rovodev's `acli`-backed dispatch. Fall back to a plain `shutil.which`
# for tool names that are not registered integrations (e.g. "git",
# "code", "code-insiders").
from .integrations import get_integration

impl = get_integration(tool)
found = impl.is_cli_available() if impl is not None else shutil.which(tool) is not None

if tracker:
if found:
Expand Down
35 changes: 35 additions & 0 deletions src/specify_cli/integrations/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,41 @@ def _resolve_executable(self) -> str:
override = os.environ.get(env_name, "").strip()
return override if override else self.key

@property
def cli_executable(self) -> str:
"""Executable name used to detect this integration's CLI tool.

Defaults to whatever ``_resolve_executable()`` returns (the
``SPECKIT_INTEGRATION_<KEY>_EXECUTABLE`` override, else ``self.key``),
so integrations whose executable differs from their key only need to
override ``_resolve_executable()`` — as ``RovodevIntegration`` already
does for ``acli`` — to get correct CLI detection for free.

Integrations that need to accept more than one candidate binary name,
or check non-``PATH`` install locations, should override
``is_cli_available()`` instead of (or in addition to) this property.

See issue #2558.
"""
return self._resolve_executable()

def is_cli_available(self) -> bool:
"""Return whether this integration's CLI tool is installed.

The default implementation checks ``shutil.which(self.cli_executable)``.
Detection call sites (``check_tool()``, workflow command/prompt
dispatch) should call this instead of hardcoding
``shutil.which(self.key)`` or maintaining a per-agent special case.
Comment on lines +285 to +287

Override for agents whose detection can't be expressed as a single
executable name — e.g. ``KiroCliIntegration`` accepts either
``kiro-cli`` or the legacy ``kiro`` binary, and Claude Code also
checks non-``PATH`` local install locations.

See issue #2558.
"""
return shutil.which(self.cli_executable) is not None

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

Expand Down
17 changes: 17 additions & 0 deletions src/specify_cli/integrations/claude/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from typing import Any

from ..base import SkillsIntegration
from ... import _utils
from ..._utils import dump_frontmatter

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

def is_cli_available(self) -> bool:
"""Claude Code can be installed in two local paths that may not be
on the system ``PATH``:

1. ``~/.claude/local/claude`` (after ``claude migrate-installer``)
2. ``~/.claude/local/node_modules/.bin/claude`` (npm-local install,
e.g. via nvm)

Checked here (rather than a hardcoded special case in
``check_tool()``) so any future detection call site gets the same
behavior for free. See issues #123, #550, #2558.
"""
if _utils.CLAUDE_LOCAL_PATH.is_file() or _utils.CLAUDE_NPM_LOCAL_PATH.is_file():
return True
return super().is_cli_available()

@staticmethod
def inject_argument_hint(content: str, hint: str) -> str:
"""Insert ``argument-hint`` after the first ``description:`` in YAML frontmatter.
Expand Down
13 changes: 13 additions & 0 deletions src/specify_cli/integrations/kiro_cli/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
"""Kiro CLI integration."""

import shutil

from ..base import MarkdownIntegration


Expand Down Expand Up @@ -34,3 +36,14 @@ class KiroCliIntegration(MarkdownIntegration):
"args": _KIRO_ARG_FALLBACK,
"extension": ".md",
}

def is_cli_available(self) -> bool:
"""Kiro currently supports both executable names.
Prefer ``kiro-cli`` and accept the legacy ``kiro`` binary as a
compatibility fallback (see issue #2558).
"""
return (
shutil.which(self.cli_executable) is not None
or shutil.which("kiro") is not None
)
37 changes: 37 additions & 0 deletions tests/integrations/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import shlex
import sys
from types import SimpleNamespace
from unittest.mock import patch

import pytest

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


class TestCliExecutableDetection:
"""cli_executable / is_cli_available() — issue #2558."""

def test_cli_executable_defaults_to_key(self):
i = StubIntegration()
assert i.cli_executable == "stub"

def test_cli_executable_honors_executable_env_override(self, monkeypatch):
monkeypatch.setenv("SPECKIT_INTEGRATION_STUB_EXECUTABLE", "stub-bin")
i = StubIntegration()
assert i.cli_executable == "stub-bin"

def test_is_cli_available_true_when_executable_on_path(self):
i = StubIntegration()
with patch("specify_cli.integrations.base.shutil.which", return_value="/usr/bin/stub"):
assert i.is_cli_available() is True

def test_is_cli_available_false_when_not_on_path(self):
i = StubIntegration()
with patch("specify_cli.integrations.base.shutil.which", return_value=None):
assert i.is_cli_available() is False

def test_is_cli_available_uses_cli_executable_not_key(self, monkeypatch):
"""An integration overriding cli_executable should be detected by
that name, not by ``self.key`` (mirrors RovoDev: key='rovodev',
executable='acli')."""
monkeypatch.setenv("SPECKIT_INTEGRATION_STUB_EXECUTABLE", "stub-bin")
i = StubIntegration()

def fake_which(name):
return "/usr/bin/stub-bin" if name == "stub-bin" else None

with patch("specify_cli.integrations.base.shutil.which", side_effect=fake_which):
assert i.is_cli_available() is True


class TestMarkdownIntegration:
def test_is_subclass_of_base(self):
assert issubclass(MarkdownIntegration, IntegrationBase)
Expand Down