Compare commits

...

3 Commits

Author SHA1 Message Date
Manfred Riem
c8664f9f6a Potential fix for pull request finding 'Empty except'
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
2026-06-04 12:23:09 -05:00
copilot-swe-agent[bot]
fc9ce2cfec Changes before error encountered
Agent-Logs-Url: https://github.com/github/spec-kit/sessions/9bf72d24-ce5d-4f1b-8803-d75f9c366793
2026-06-04 16:47:21 +00:00
copilot-swe-agent[bot]
d24d3b18cf Initial plan 2026-06-04 16:35:06 +00:00
9 changed files with 251 additions and 42 deletions

View File

@@ -147,12 +147,12 @@ class CodexIntegration(SkillsIntegration):
| Field | Location | Purpose |
|---|---|---|
| `key` | Class attribute | Unique identifier; for CLI-based integrations (`requires_cli: True`), must match the CLI executable name |
| `key` | Class attribute | Unique identifier; for most CLI-based integrations this matches the executable name, but see `cli_executable` below for exceptions |
| `config` | Class attribute (dict) | Agent metadata: `name`, `folder`, `commands_subdir`, `install_url`, `requires_cli` |
| `registrar_config` | Class attribute (dict) | Command output config: `dir`, `format`, `args` placeholder, file `extension` |
| `context_file` | Class attribute (str or None) | Path to agent context/instructions file (e.g., `"CLAUDE.md"`, `".github/copilot-instructions.md"`) |
**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., `"windsurf"`, `"copilot"`).
**Key design rule:** For CLI-based integrations (`requires_cli: True`), `key` should generally match the CLI executable name so that the default `is_cli_available()` check works without any override. When the executable name differs from the key (e.g., RovoDev's key is `"rovodev"` but the binary is `"acli"`), override the `cli_executable` property or `is_cli_available()` method — see [§6 Optional overrides](#6-optional-overrides) below. IDE-based integrations (`requires_cli: False`) should use their canonical identifier (e.g., `"windsurf"`, `"copilot"`).
### 3. Register it
@@ -222,11 +222,37 @@ The base classes handle most work automatically. Override only when the agent de
| Override | When to use | Example |
|---|---|---|
| `cli_executable` | Binary name differs from `key` | RovoDev: key `"rovodev"`, binary `"acli"` → override returns `"acli"` |
| `is_cli_available()` | Multiple binary names or non-PATH installs | Claude checks `~/.claude/local/`; Kiro accepts both `kiro-cli` and `kiro` |
| `command_filename(template_name)` | Custom file naming or extension | Copilot → `speckit.{name}.agent.md` |
| `options()` | Integration-specific CLI flags via `--integration-options` | Codex → `--skills` flag, Copilot → `--skills` flag |
| `setup()` | Custom install logic (companion files, settings merge) | Copilot → `.agent.md` + `.prompt.md` + `.vscode/settings.json` (default) or `speckit-<name>/SKILL.md` (skills mode) |
| `teardown()` | Custom uninstall logic | Rarely needed; base handles manifest-tracked files |
**`cli_executable` property** — Return the binary name to look up on `PATH` for tool-availability checks. The default implementation returns `self.key`. Override when the executable name differs from the integration key:
```python
@property
def cli_executable(self) -> str:
return "acli" # e.g. RovoDev: key="rovodev", binary="acli"
```
**`is_cli_available()` method** — Return `True` if the integration's CLI tool is installed. The default implementation calls `shutil.which(self.cli_executable)`. Override for more complex detection:
```python
def is_cli_available(self) -> bool:
# Multiple binary names (Kiro):
return shutil.which("kiro-cli") is not None or shutil.which("kiro") is not None
# Non-PATH install locations (Claude):
import specify_cli._utils as _utils_mod
if _utils_mod.CLAUDE_LOCAL_PATH.is_file() or _utils_mod.CLAUDE_NPM_LOCAL_PATH.is_file():
return True
return shutil.which(self.cli_executable) is not None
```
`is_cli_available()` is used by `check_tool()` in `_utils.py` and by both `CommandStep` and `PromptStep` workflow steps to gate CLI dispatch. No hardcoded special cases should be added to those callers — encode detection logic in the integration class instead.
**Example — Copilot (fully custom `setup`):**
Copilot extends `IntegrationBase` directly because it creates `.agent.md` commands, companion `.prompt.md` files, and merges `.vscode/settings.json`. It also supports a `--skills` mode that scaffolds `speckit-<name>/SKILL.md` under `.github/skills/` using composition with an internal `_CopilotSkillsHelper`. See `src/specify_cli/integrations/copilot/__init__.py` for the full implementation.
@@ -436,7 +462,7 @@ When an issue exists, include its number immediately after the prefix — this i
## 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`), `key` should generally match the executable name. When it cannot (e.g., the binary name differs), override `cli_executable` or `is_cli_available()` on the integration class. Do **not** add special-case mappings to `check_tool()`, `CommandStep`, or `PromptStep`.
2. **Forgetting context configuration**: The bundled `agent-context` extension reads from `.specify/extensions/agent-context/agent-context-config.yml`. New integrations only need to set `context_file` on the class — markers and dispatcher scripts are managed centrally.
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.

View File

@@ -38,35 +38,44 @@ def run_command(cmd: list[str], check_return: bool = True, capture: bool = False
def check_tool(tool: str, tracker=None) -> bool:
"""Check if a tool is installed. Optionally update tracker.
For tools that correspond to a registered integration the check is
delegated to ``IntegrationBase.is_cli_available()`` so that each
integration can encode its own detection logic (e.g. multiple
binary names, non-PATH install locations). Unknown tools fall back
to a plain ``shutil.which`` look-up.
Args:
tool: Name of the tool to check
tool: Name of the tool to check (typically an integration key)
tracker: StepTracker | None to update with results
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
found: bool
# 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
# Delegate to the integration's is_cli_available() when the tool
# key matches a registered integration. This removes the need for
# hard-coded special cases here (e.g. Claude local paths, kiro dual
# binaries, rovodev/acli mismatch). See issue #2597.
try:
from specify_cli.integrations import get_integration
impl = get_integration(tool)
if impl is not None:
found = impl.is_cli_available()
if tracker:
if found:
tracker.complete(tool, "available")
else:
tracker.error(tool, "not found")
return found
except ImportError as exc:
# Integrations module is unavailable in this environment; fall back
# to PATH-based detection below for non-integration tools.
_ = exc
# Fallback for non-integration tools (e.g. "git").
found = shutil.which(tool) is not None
if tracker:
if found:

View File

@@ -162,6 +162,45 @@ class IntegrationBase(ABC):
"""
return None
@property
def cli_executable(self) -> str:
"""Executable name used for CLI availability detection.
Defaults to ``self.key``. Integrations whose CLI binary name
differs from the integration key should override this property.
For example, RovoDev's key is ``"rovodev"`` but the binary is
``"acli"``, so its override returns ``"acli"``.
This property is used by :meth:`is_cli_available` and by
``check_tool()`` when checking whether the integration's CLI
tool is installed. It intentionally does **not** honour the
``SPECKIT_INTEGRATION_<KEY>_EXECUTABLE`` env-var override — that
variable controls which binary is *executed* at runtime (see
:meth:`_resolve_executable`), whereas ``cli_executable`` names
the tool to *detect* on ``PATH``.
See issue #2597.
"""
return self.key
def is_cli_available(self) -> bool:
"""Return ``True`` if this integration's CLI tool is installed.
The default implementation checks ``shutil.which(self.cli_executable)``.
Integrations with non-standard install locations or multiple
possible binary names should override this method.
Examples of integrations that override this:
* **ClaudeIntegration** — also checks ``~/.claude/local/`` paths
that are not on ``PATH``.
* **KiroCliIntegration** — accepts both ``kiro-cli`` and the
legacy ``kiro`` binary name.
See issue #2597.
"""
return shutil.which(self.cli_executable) is not None
def _resolve_executable(self) -> str:
"""Return the executable for this integration's CLI tool.

View File

@@ -2,6 +2,7 @@
from __future__ import annotations
import shutil
from pathlib import Path
from typing import Any
@@ -45,6 +46,27 @@ class ClaudeIntegration(SkillsIntegration):
context_file = "CLAUDE.md"
multi_install_safe = True
def is_cli_available(self) -> bool:
"""Return ``True`` if the Claude Code CLI is installed.
Claude Code can be installed in multiple locations, not all of
which are on ``PATH``:
1. ``~/.claude/local/claude`` — ``claude migrate-installer``
2. ``~/.claude/local/node_modules/.bin/claude`` — npm-local install (nvm)
3. Anywhere on ``PATH`` — global npm install
See issues #123, #550, and #2597.
"""
import specify_cli._utils as _utils_mod
if (
_utils_mod.CLAUDE_LOCAL_PATH.is_file()
or _utils_mod.CLAUDE_NPM_LOCAL_PATH.is_file()
):
return True
return shutil.which(self.cli_executable) is not None
@staticmethod
def inject_argument_hint(content: str, hint: str) -> str:
"""Insert ``argument-hint`` after the first ``description:`` in YAML frontmatter.

View File

@@ -1,5 +1,7 @@
"""Kiro CLI integration."""
import shutil
from ..base import MarkdownIntegration
@@ -27,3 +29,17 @@ class KiroCliIntegration(MarkdownIntegration):
"extension": ".md",
}
context_file = "AGENTS.md"
def is_cli_available(self) -> bool:
"""Return ``True`` if the Kiro CLI is installed.
Kiro ships under two binary names: ``kiro-cli`` (preferred) and
the legacy ``kiro`` alias. Either name satisfies the availability
check so existing installations continue to work.
See issue #2597.
"""
return (
shutil.which("kiro-cli") is not None
or shutil.which("kiro") is not None
)

View File

@@ -43,6 +43,19 @@ class RovodevIntegration(SkillsIntegration):
# -- CLI dispatch ------------------------------------------------------
@property
def cli_executable(self) -> str:
"""Executable name for CLI availability detection (``acli``).
RovoDev is invoked as ``acli rovodev …`` — ``acli`` is the
host binary; ``rovodev`` is a sub-command. The integration key
is ``"rovodev"``, but the binary to detect on ``PATH`` is
``"acli"``.
See issue #2597.
"""
return "acli"
def _resolve_executable(self) -> str:
"""Return the binary to invoke (``acli``).

View File

@@ -2,7 +2,6 @@
from __future__ import annotations
import shutil
from pathlib import Path
from typing import Any
@@ -126,15 +125,10 @@ class CommandStep(StepBase):
if impl is None:
return None
# Build sample args for fallback executable detection when impl.key is not executable.
exec_args = impl.build_exec_args("test")
# Check if the CLI tool is actually installed.
# Try the integration key first (covers most agents), then fall back
# to exec_args[0] for agents whose executable differs.
cli_path = shutil.which(impl.key)
fallback_cli_path = shutil.which(exec_args[0]) if exec_args else None
if cli_path is None and fallback_cli_path is None:
# Check if the CLI tool is actually installed via the integration's
# own availability check (honours custom executables, dual binaries,
# and non-PATH install paths). See issue #2597.
if not impl.is_cli_available():
return None
project_root = Path(context.project_root) if context.project_root else None

View File

@@ -2,7 +2,6 @@
from __future__ import annotations
import shutil
from pathlib import Path
from typing import Any
@@ -116,12 +115,10 @@ class PromptStep(StepBase):
exec_args = impl.build_exec_args(prompt, model=model, output_json=False)
# Check if the CLI tool is actually installed.
# Try the integration key first (covers most agents), then fall back
# to exec_args[0] for agents whose executable differs.
cli_path = shutil.which(impl.key)
fallback_cli_path = shutil.which(exec_args[0]) if exec_args else None
if cli_path is None and fallback_cli_path is None:
# Check if the CLI tool is actually installed via the integration's
# own availability check (honours custom executables, dual binaries,
# and non-PATH install paths). See issue #2597.
if not impl.is_cli_available():
return None
# Prompt dispatch executes exec_args directly; require a non-empty argv.

View File

@@ -10,6 +10,7 @@ from unittest.mock import patch, MagicMock
from typer.testing import CliRunner
from specify_cli import app, check_tool
from specify_cli.integrations import get_integration
from tests.conftest import strip_ansi
@@ -121,6 +122,98 @@ class TestCheckToolOther:
assert check_tool("rovodev") is True
class TestIsCliAvailable:
"""Integration.is_cli_available() must encode correct detection logic."""
def test_rovodev_cli_executable_is_acli(self):
"""RovodevIntegration.cli_executable should return 'acli'."""
impl = get_integration("rovodev")
assert impl.cli_executable == "acli"
def test_rovodev_is_cli_available_uses_acli(self):
"""RovodevIntegration.is_cli_available() checks for 'acli', not 'rovodev'."""
impl = get_integration("rovodev")
with patch("shutil.which", side_effect=lambda name: "/usr/bin/acli" if name == "acli" else None):
assert impl.is_cli_available() is True
with patch("shutil.which", return_value=None):
assert impl.is_cli_available() is False
def test_kiro_is_cli_available_accepts_kiro_cli(self):
"""KiroCliIntegration.is_cli_available() returns True for 'kiro-cli' binary."""
impl = get_integration("kiro-cli")
with patch("shutil.which", side_effect=lambda name: "/usr/bin/kiro-cli" if name == "kiro-cli" else None):
assert impl.is_cli_available() is True
def test_kiro_is_cli_available_accepts_legacy_kiro(self):
"""KiroCliIntegration.is_cli_available() accepts the legacy 'kiro' binary."""
impl = get_integration("kiro-cli")
with patch("shutil.which", side_effect=lambda name: "/usr/bin/kiro" if name == "kiro" else None):
assert impl.is_cli_available() is True
def test_kiro_is_cli_available_false_when_neither(self):
"""KiroCliIntegration.is_cli_available() returns False when neither binary exists."""
impl = get_integration("kiro-cli")
with patch("shutil.which", return_value=None):
assert impl.is_cli_available() is False
def test_claude_is_cli_available_local_path(self, tmp_path):
"""ClaudeIntegration.is_cli_available() finds claude via local path."""
impl = get_integration("claude")
fake_claude = tmp_path / "claude"
fake_claude.touch()
fake_missing = tmp_path / "nonexistent" / "claude"
with patch("specify_cli._utils.CLAUDE_LOCAL_PATH", fake_claude), \
patch("specify_cli._utils.CLAUDE_NPM_LOCAL_PATH", fake_missing), \
patch("shutil.which", return_value=None):
assert impl.is_cli_available() is True
def test_claude_is_cli_available_npm_local_path(self, tmp_path):
"""ClaudeIntegration.is_cli_available() finds claude via npm-local path."""
impl = get_integration("claude")
fake_npm = tmp_path / "node_modules" / ".bin" / "claude"
fake_npm.parent.mkdir(parents=True)
fake_npm.touch()
fake_missing = tmp_path / "nonexistent" / "claude"
with patch("specify_cli._utils.CLAUDE_LOCAL_PATH", fake_missing), \
patch("specify_cli._utils.CLAUDE_NPM_LOCAL_PATH", fake_npm), \
patch("shutil.which", return_value=None):
assert impl.is_cli_available() is True
def test_claude_is_cli_available_path(self, tmp_path):
"""ClaudeIntegration.is_cli_available() finds claude via PATH."""
impl = get_integration("claude")
fake_missing = tmp_path / "nonexistent" / "claude"
with patch("specify_cli._utils.CLAUDE_LOCAL_PATH", fake_missing), \
patch("specify_cli._utils.CLAUDE_NPM_LOCAL_PATH", fake_missing), \
patch("shutil.which", return_value="/usr/local/bin/claude"):
assert impl.is_cli_available() is True
def test_claude_is_cli_available_not_found(self, tmp_path):
"""ClaudeIntegration.is_cli_available() returns False when not installed."""
impl = get_integration("claude")
fake_missing = tmp_path / "nonexistent" / "claude"
with patch("specify_cli._utils.CLAUDE_LOCAL_PATH", fake_missing), \
patch("specify_cli._utils.CLAUDE_NPM_LOCAL_PATH", fake_missing), \
patch("shutil.which", return_value=None):
assert impl.is_cli_available() is False
def test_default_integration_uses_key(self):
"""Integrations without an override use key as cli_executable."""
# Use a non-CLI integration to test the default; any MarkdownIntegration
# without a cli_executable override works.
impl = get_integration("gemini")
assert impl.cli_executable == impl.key
class TestCheckTip:
"""`specify check` should point users to the existing version check."""