mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
* feat: add Zed integration * fix: update integrations stats grid to 31 for consistency * fix: address Copilot review feedback - Remove non-actionable --skills flag from ZedIntegration (Zed is always skills-based, like Agy) - Align zed_skill_mode predicate with ai_skills for consistency across init output and hook rendering - Consolidate claude/cursor/zed slash-skill return blocks in _render_hook_invocation to reduce duplication - Override test_options_include_skills_flag for Zed (no --skills flag) * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix: address Copilot review round 2 - Make zed_skill_mode unconditional in hook rendering (Zed is always skills-based, no --skills option) - Add test_init_persists_ai_skills_for_zed that exercises the actual CLI init path and verifies HookExecutor renders /speckit-plan without manual init-options manipulation * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix: address copilot review feedback for zed integration - Update integration count from 31 to 33 in docs/index.md (32 integrations + Generic) - Make zed_skill_mode unconditional to match extensions.py behavior - Consolidate slash-skill integrations into a set for consistency - Move os import to module level in test_integration_zed.py * fix: refine slash-skill logic and ai-skills validation - Fix slash-skill integrations: Claude/Cursor require ai_skills=true; Zed/Agy/Devin are always skills - Allow --ai-skills with --integration (not just --ai) to fix validation error * fix: remove unused variables and update ai-skills help text - Add agy_skill_mode and devin_skill_mode variables to fix F841 lint error - Use all skill mode variables in the slash-skill conditional check - Update --ai-skills help text to reflect it works with --integration too * fix: add trae_skill_mode to hook invocation for consistency Trae is a SkillsIntegration like Zed/Agy/Devin, so it should also be treated as always-skills-based in hook invocation rendering. * fix: make Agy always skills-based for consistency AgyIntegration is a SkillsIntegration subclass with no --skills option, so it should be treated as always skills-based (like Zed, Devin, Trae). This aligns init.py skill mode detection with extensions.py hook rendering. * fix: gate agy_skill_mode and refactor _render_hook_invocation to use sets Addressed Copilot review comments: - Restored _is_skills_integration guard on agy_skill_mode in init.py to be defensive about runtime integration type. - Refactored _render_hook_invocation() in extensions.py to use always_slash/conditional_slash frozensets instead of individual per-agent booleans, eliminating unused variables (F841) and making it harder for conditions to drift between integrations. - Centralized slash-skill determination so adding a new unconditional slash-skill integration is a one-key addition. * fix: address latest Copilot review comments - Added copilot to CONDITIONAL_SLASH_AGENTS for consistent hook invocation rendering with init.py - Moved always_slash/conditional_slash frozensets to module scope to avoid per-call reallocation - Replaced manual os.chdir() with monkeypatch.chdir() in test - Overrode test_options_include_skills_flag for Zed (no --skills) * fix: address latest Copilot review comments - Removed redundant local import yaml in _register_extension_skills (yaml is already imported at module scope) - Split --ai-skills usage hint into two separate print statements for better readability - Changed integrations count from '33' to '30+' to avoid future drift * fix: re-add _is_skills_integration definition lost in merge The _is_skills_integration variable was accidentally dropped during the web UI merge resolution of upstream/main's removal of legacy --ai flags. Re-added the definition via isinstance(resolved_integration, SkillsIntegration) check so that skill-mode booleans work correctly. * fix: gate zed_skill_mode on _is_skills_integration for consistency Aligns zed_skill_mode with the other skills-based agents (codex, claude, cursor-agent, copilot) which all use _is_skills_integration gating. Since ZedIntegration extends SkillsIntegration, behavior is unchanged. * fix: remove unused claude_skill_mode and cursor_skill_mode locals in _render_hook_invocation These variables became unused after the refactor to ALWAYS_SLASH_AGENTS / CONDITIONAL_SLASH_AGENTS sets. Claude and Cursor-Agent are now handled by the CONDITIONAL_SLASH_AGENTS path, so the separate boolean locals are dead code. Fixes ruff F841 and addresses Copilot review feedback that was repeated across multiple review rounds. * fix: align agy/trae invocation format in init next-steps with hook rendering and build_command_invocation - Moved agy and trae from '-<name>' (dollar/Codex format) to '/speckit-<name>' (slash format) in _display_cmd() to match: - HookExecutor._render_hook_invocation() (ALWAYS_SLASH_AGENTS for trae, CONDITIONAL_SLASH_AGENTS for agy) - SkillsIntegration.build_command_invocation() (default: /speckit-<name>) - The '$' prefix is specific to Codex; all other skills agents use '/'. * fix: address Copilot review comments on hook invocation consistency - Add is_slash_skills_agent() helper to extensions.py to centralize the agent-to-invocation-format mapping, reducing drift risk between HookExecutor._render_hook_invocation() and init.py _display_cmd() - Use the shared helper in both locations; init.py now imports and delegates to is_slash_skills_agent() instead of maintaining its own per-agent boolean matrix - Fix test_hooks_render_skill_invocation to use ai_skills=False, proving Zed renders /speckit-<name> unconditionally - Add parameterized TestSlashSkillsSets covering all agents in ALWAYS_SLASH_AGENTS and CONDITIONAL_SLASH_AGENTS with ai_skills both true and false * fix: address Copilot review comments on type safety and test API - Make is_slash_skills_agent() accept str | None to match its call sites (init_options.get("ai") can return None) - Refactor TestSlashSkillsSets to use public execute_hook() API instead of private _render_hook_invocation() method * fix: address Copilot review comments on typing and naming clarity - Add from __future__ import annotations to extensions.py so PEP 604 unions (str | None) are safe regardless of Python version - Add clarifying _ai_skills_enabled local variable in init.py's _display_cmd() to make the semantic meaning explicit when passing it to is_slash_skills_agent() * fix: move invocation-style logic into shared _invocation_style module - Extract ALWAYS_SLASH_AGENTS, CONDITIONAL_SLASH_AGENTS, and is_slash_skills_agent() from extensions.py into new _invocation_style.py module, eliminating the awkward init.py -> extensions.py import dependency for invocation-style decision logic - Both HookExecutor._render_hook_invocation() and init.py _display_cmd() now import from the shared module instead of one subsystem importing from the other - Revert /SKILL.md change: the leading slash is semantically significant (path component vs filename suffix) * fix: add None guard before i.options() in test_options_include_skills_flag get_integration() returns IntegrationBase | None, so i.options() is a type error without a None check. * fix: override test_options_include_skills_flag for Zed (always skills, no --skills flag) Zed is always skills-based and doesn't expose a --skills option. Override the inherited base test to assert --skills is absent. * fix: rename test and skip inherited test_options_include_skills_flag for Zed - Skip inherited test_options_include_skills_flag (not applicable — Zed is always skills-based with no --skills flag) - Add test_options_do_not_include_skills_flag with correct name matching the assertion (--skills is absent) * fix: add defensive non-string check in is_slash_skills_agent Reject non-string values for selected_ai to prevent TypeError from set membership checks when persisted init-options contain corrupted data (e.g. list or dict instead of string). --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
290 lines
11 KiB
Python
290 lines
11 KiB
Python
"""Tests for INTEGRATION_REGISTRY — mechanics, completeness, and registrar alignment."""
|
|
|
|
import json
|
|
import os
|
|
from pathlib import PurePosixPath
|
|
|
|
import pytest
|
|
from typer.testing import CliRunner
|
|
|
|
from specify_cli import app
|
|
from specify_cli.integrations import (
|
|
INTEGRATION_REGISTRY,
|
|
_register,
|
|
get_integration,
|
|
)
|
|
from specify_cli.integrations.base import MarkdownIntegration
|
|
from .conftest import StubIntegration
|
|
|
|
|
|
# Every integration key that must be registered (Stage 2 + Stage 3 + Stage 4 + Stage 5).
|
|
ALL_INTEGRATION_KEYS = [
|
|
"copilot",
|
|
# Stage 3 — standard markdown integrations
|
|
"claude", "qwen", "opencode", "junie", "kilocode", "auggie",
|
|
"roo", "rovodev", "codebuddy", "qodercli", "amp", "shai", "bob", "trae",
|
|
"pi", "iflow", "kiro-cli", "windsurf", "vibe", "cursor-agent",
|
|
# Stage 4 — TOML integrations
|
|
"gemini", "tabnine",
|
|
# Stage 5 — skills, generic & option-driven integrations
|
|
"codex", "kimi", "agy", "zed", "generic",
|
|
]
|
|
|
|
|
|
def _multi_install_safe_keys() -> list[str]:
|
|
return sorted(
|
|
key
|
|
for key, integration in INTEGRATION_REGISTRY.items()
|
|
if integration.multi_install_safe
|
|
)
|
|
|
|
|
|
def _multi_install_safe_pairs() -> list[tuple[str, str]]:
|
|
safe_keys = _multi_install_safe_keys()
|
|
return [
|
|
(safe_keys[left], safe_keys[right])
|
|
for left in range(len(safe_keys))
|
|
for right in range(left + 1, len(safe_keys))
|
|
]
|
|
|
|
|
|
def _posix_path(value: str | None) -> str | None:
|
|
if not value:
|
|
return None
|
|
return PurePosixPath(value).as_posix()
|
|
|
|
|
|
def _integration_root_dir(key: str) -> str | None:
|
|
integration = INTEGRATION_REGISTRY[key]
|
|
cfg = integration.config if isinstance(integration.config, dict) else {}
|
|
return _posix_path(cfg.get("folder"))
|
|
|
|
|
|
def _integration_commands_dir(key: str) -> str | None:
|
|
integration = INTEGRATION_REGISTRY[key]
|
|
cfg = integration.config if isinstance(integration.config, dict) else {}
|
|
folder = cfg.get("folder")
|
|
if not folder:
|
|
return None
|
|
subdir = cfg.get("commands_subdir", "commands")
|
|
return (PurePosixPath(folder) / subdir).as_posix()
|
|
|
|
|
|
def _paths_overlap(first: str | None, second: str | None) -> bool:
|
|
if not first or not second:
|
|
return False
|
|
left = PurePosixPath(first)
|
|
right = PurePosixPath(second)
|
|
try:
|
|
left.relative_to(right)
|
|
return True
|
|
except ValueError:
|
|
pass
|
|
try:
|
|
right.relative_to(left)
|
|
return True
|
|
except ValueError:
|
|
return False
|
|
|
|
|
|
def _path_is_inside(path: str | None, directory: str | None) -> bool:
|
|
if not path or not directory:
|
|
return False
|
|
try:
|
|
PurePosixPath(path).relative_to(PurePosixPath(directory))
|
|
return True
|
|
except ValueError:
|
|
return False
|
|
|
|
|
|
class TestRegistry:
|
|
def test_registry_is_dict(self):
|
|
assert isinstance(INTEGRATION_REGISTRY, dict)
|
|
|
|
def test_register_and_get(self):
|
|
stub = StubIntegration()
|
|
_register(stub)
|
|
try:
|
|
assert get_integration("stub") is stub
|
|
finally:
|
|
INTEGRATION_REGISTRY.pop("stub", None)
|
|
|
|
def test_get_missing_returns_none(self):
|
|
assert get_integration("nonexistent-xyz") is None
|
|
|
|
def test_register_empty_key_raises(self):
|
|
class EmptyKey(MarkdownIntegration):
|
|
key = ""
|
|
with pytest.raises(ValueError, match="empty key"):
|
|
_register(EmptyKey())
|
|
|
|
def test_register_duplicate_raises(self):
|
|
stub = StubIntegration()
|
|
_register(stub)
|
|
try:
|
|
with pytest.raises(KeyError, match="already registered"):
|
|
_register(StubIntegration())
|
|
finally:
|
|
INTEGRATION_REGISTRY.pop("stub", None)
|
|
|
|
|
|
class TestRegistryCompleteness:
|
|
"""Every expected integration must be registered."""
|
|
|
|
@pytest.mark.parametrize("key", ALL_INTEGRATION_KEYS)
|
|
def test_key_registered(self, key):
|
|
assert key in INTEGRATION_REGISTRY, f"{key} missing from registry"
|
|
|
|
|
|
class TestRegistrarKeyAlignment:
|
|
"""Every integration key must have a matching AGENT_CONFIGS entry.
|
|
|
|
``generic`` is excluded because it has no fixed directory — its
|
|
output path comes from ``--commands-dir`` at runtime.
|
|
"""
|
|
|
|
@pytest.mark.parametrize(
|
|
"key",
|
|
[k for k in ALL_INTEGRATION_KEYS if k != "generic"],
|
|
)
|
|
def test_integration_key_in_registrar(self, key):
|
|
from specify_cli.agents import CommandRegistrar
|
|
assert key in CommandRegistrar.AGENT_CONFIGS, (
|
|
f"Integration '{key}' is registered but has no AGENT_CONFIGS entry"
|
|
)
|
|
|
|
def test_no_stale_cursor_shorthand(self):
|
|
"""The old 'cursor' shorthand must not appear in AGENT_CONFIGS."""
|
|
from specify_cli.agents import CommandRegistrar
|
|
assert "cursor" not in CommandRegistrar.AGENT_CONFIGS
|
|
|
|
|
|
class TestMultiInstallSafeContracts:
|
|
"""Declared safe integrations must stay isolated from each other."""
|
|
|
|
@pytest.mark.parametrize("key", _multi_install_safe_keys())
|
|
def test_safe_integrations_have_static_isolated_paths(self, key):
|
|
integration = INTEGRATION_REGISTRY[key]
|
|
|
|
assert _integration_root_dir(key), (
|
|
f"{key} is declared multi-install safe but has no static root directory"
|
|
)
|
|
assert _integration_commands_dir(key), (
|
|
f"{key} is declared multi-install safe but has no static commands directory"
|
|
)
|
|
assert integration.context_file, (
|
|
f"{key} is declared multi-install safe but has no context file"
|
|
)
|
|
|
|
@pytest.mark.parametrize(("first", "second"), _multi_install_safe_pairs())
|
|
def test_safe_integrations_have_distinct_agent_roots(self, first, second):
|
|
assert not _paths_overlap(_integration_root_dir(first), _integration_root_dir(second)), (
|
|
f"{first} and {second} are declared multi-install safe but have "
|
|
f"overlapping agent roots {_integration_root_dir(first)!r} and "
|
|
f"{_integration_root_dir(second)!r}"
|
|
)
|
|
|
|
@pytest.mark.parametrize(("first", "second"), _multi_install_safe_pairs())
|
|
def test_safe_integrations_have_distinct_command_dirs(self, first, second):
|
|
assert not _paths_overlap(_integration_commands_dir(first), _integration_commands_dir(second)), (
|
|
f"{first} and {second} are declared multi-install safe but have "
|
|
f"overlapping command directories {_integration_commands_dir(first)!r} and "
|
|
f"{_integration_commands_dir(second)!r}"
|
|
)
|
|
|
|
@pytest.mark.parametrize(("first", "second"), _multi_install_safe_pairs())
|
|
def test_safe_integrations_have_distinct_context_files(self, first, second):
|
|
first_context = _posix_path(INTEGRATION_REGISTRY[first].context_file)
|
|
second_context = _posix_path(INTEGRATION_REGISTRY[second].context_file)
|
|
|
|
assert first_context != second_context, (
|
|
f"{first} and {second} are declared multi-install safe but share "
|
|
f"context file {first_context!r}"
|
|
)
|
|
|
|
@pytest.mark.parametrize(("first", "second"), _multi_install_safe_pairs())
|
|
def test_safe_context_files_do_not_overlap_other_agent_roots(self, first, second):
|
|
first_context = _posix_path(INTEGRATION_REGISTRY[first].context_file)
|
|
second_context = _posix_path(INTEGRATION_REGISTRY[second].context_file)
|
|
|
|
assert not _path_is_inside(first_context, _integration_root_dir(second)), (
|
|
f"{first} context file {first_context!r} lives under {second} "
|
|
f"agent root {_integration_root_dir(second)!r}"
|
|
)
|
|
assert not _path_is_inside(second_context, _integration_root_dir(first)), (
|
|
f"{second} context file {second_context!r} lives under {first} "
|
|
f"agent root {_integration_root_dir(first)!r}"
|
|
)
|
|
|
|
@pytest.mark.parametrize(("first", "second"), _multi_install_safe_pairs())
|
|
def test_safe_context_files_do_not_overlap_other_command_dirs(self, first, second):
|
|
first_context = _posix_path(INTEGRATION_REGISTRY[first].context_file)
|
|
second_context = _posix_path(INTEGRATION_REGISTRY[second].context_file)
|
|
|
|
assert not _path_is_inside(first_context, _integration_commands_dir(second)), (
|
|
f"{first} context file {first_context!r} lives under {second} "
|
|
f"commands directory {_integration_commands_dir(second)!r}"
|
|
)
|
|
assert not _path_is_inside(second_context, _integration_commands_dir(first)), (
|
|
f"{second} context file {second_context!r} lives under {first} "
|
|
f"commands directory {_integration_commands_dir(first)!r}"
|
|
)
|
|
|
|
@pytest.mark.parametrize(("first", "second"), _multi_install_safe_pairs())
|
|
def test_safe_integrations_have_disjoint_manifests(
|
|
self,
|
|
tmp_path,
|
|
first,
|
|
second,
|
|
):
|
|
for initial, additional in ((first, second), (second, first)):
|
|
project_root = tmp_path / f"project-{initial}-{additional}"
|
|
project_root.mkdir()
|
|
runner = CliRunner()
|
|
|
|
original_cwd = os.getcwd()
|
|
try:
|
|
os.chdir(project_root)
|
|
init_result = runner.invoke(
|
|
app,
|
|
[
|
|
"init",
|
|
"--here",
|
|
"--integration",
|
|
initial,
|
|
"--script",
|
|
"sh",
|
|
"--ignore-agent-tools",
|
|
],
|
|
catch_exceptions=False,
|
|
)
|
|
assert init_result.exit_code == 0, init_result.output
|
|
|
|
install_result = runner.invoke(
|
|
app,
|
|
["integration", "install", additional, "--script", "sh"],
|
|
catch_exceptions=False,
|
|
)
|
|
assert install_result.exit_code == 0, install_result.output
|
|
finally:
|
|
os.chdir(original_cwd)
|
|
|
|
initial_manifest = json.loads(
|
|
(
|
|
project_root / ".specify" / "integrations" / f"{initial}.manifest.json"
|
|
).read_text(encoding="utf-8")
|
|
)
|
|
additional_manifest = json.loads(
|
|
(
|
|
project_root / ".specify" / "integrations" / f"{additional}.manifest.json"
|
|
).read_text(encoding="utf-8")
|
|
)
|
|
|
|
initial_files = set(initial_manifest.get("files", {}))
|
|
additional_files = set(additional_manifest.get("files", {}))
|
|
|
|
assert initial_files.isdisjoint(additional_files), (
|
|
f"{initial} and {additional} are declared multi-install safe but both manage "
|
|
f"these files: {sorted(initial_files & additional_files)}"
|
|
)
|