mirror of
https://github.com/github/spec-kit.git
synced 2026-08-15 14:02:04 +02:00
chelsealongandGitHub
bd595cf838
* fix(claude): make argument-hint injection fold-aware for long descriptions ClaudeIntegration.inject_argument_hint spliced argument-hint: "..." as a raw text line right after the first line starting with "description:". When a description is long enough for the YAML dumper to fold it across indented continuation lines, that splice landed inside the scalar, producing invalid YAML (plain scalar) or silently absorbing the hint into the description string (quoted scalar). This reproduces #3991 for the case #3996 didn't cover: bundled core commands have no argument-hint in their source frontmatter, so the structural apply_argument_hint path is a no-op and this raw-text fallback is what actually runs. Skip every continuation line of the description scalar (anything more indented than the key itself) before inserting, so the new key always lands after the whole scalar ends rather than in the middle of it. Fixes #4044 * fix(claude): also skip unindented blank lines in description scalar PyYAML serializes an embedded paragraph break ("\n\n") inside a quoted description as unindented blank lines, not indented continuation lines. inject_argument_hint only skipped indented lines, so it still inserted argument-hint mid-scalar for multi-paragraph descriptions, reproducing the #4044 failure modes. Skip blank lines too, and add a regression test for the multi-paragraph case.
957 lines
40 KiB
Python
957 lines
40 KiB
Python
"""Tests for ClaudeIntegration."""
|
|
|
|
import json
|
|
import os
|
|
from pathlib import Path
|
|
from unittest.mock import patch
|
|
|
|
import yaml
|
|
|
|
from specify_cli.integrations import INTEGRATION_REGISTRY, get_integration
|
|
from specify_cli.integrations.base import IntegrationBase, SkillsIntegration
|
|
from specify_cli.integrations.claude import ARGUMENT_HINTS, FORK_CONTEXT_COMMANDS
|
|
from specify_cli.integrations.manifest import IntegrationManifest
|
|
|
|
|
|
class TestClaudeIntegration:
|
|
def test_registered(self):
|
|
assert "claude" in INTEGRATION_REGISTRY
|
|
assert get_integration("claude") is not None
|
|
|
|
def test_is_base_integration(self):
|
|
assert isinstance(get_integration("claude"), IntegrationBase)
|
|
|
|
def test_config_uses_skills(self):
|
|
integration = get_integration("claude")
|
|
assert integration.config["folder"] == ".claude/"
|
|
assert integration.config["commands_subdir"] == "skills"
|
|
|
|
def test_registrar_config_uses_skill_layout(self):
|
|
integration = get_integration("claude")
|
|
assert integration.registrar_config["dir"] == ".claude/skills"
|
|
assert integration.registrar_config["format"] == "markdown"
|
|
assert integration.registrar_config["args"] == "$ARGUMENTS"
|
|
assert integration.registrar_config["extension"] == "/SKILL.md"
|
|
|
|
def test_setup_creates_skill_files(self, tmp_path):
|
|
integration = get_integration("claude")
|
|
manifest = IntegrationManifest("claude", tmp_path)
|
|
created = integration.setup(tmp_path, manifest, script_type="sh")
|
|
|
|
skill_files = [path for path in created if path.name == "SKILL.md"]
|
|
assert skill_files
|
|
|
|
skills_dir = tmp_path / ".claude" / "skills"
|
|
assert skills_dir.is_dir()
|
|
|
|
plan_skill = skills_dir / "speckit-plan" / "SKILL.md"
|
|
assert plan_skill.exists()
|
|
|
|
content = plan_skill.read_text(encoding="utf-8")
|
|
assert "{SCRIPT}" not in content
|
|
assert "{ARGS}" not in content
|
|
assert "__AGENT__" not in content
|
|
assert "__SPECKIT_COMMAND_" not in content, "unprocessed __SPECKIT_COMMAND_*__"
|
|
assert "/speckit." not in content, "skills agent must use /speckit-<name> not /speckit.<name>"
|
|
|
|
parts = content.split("---", 2)
|
|
parsed = yaml.safe_load(parts[1])
|
|
assert parsed["name"] == "speckit-plan"
|
|
assert parsed["user-invocable"] is True
|
|
assert parsed["disable-model-invocation"] is False
|
|
assert parsed["metadata"]["source"] == "templates/commands/plan.md"
|
|
|
|
def test_render_skill_unicode(self):
|
|
"""Test rendering a skill preserves non-ASCII characters."""
|
|
integration = get_integration("claude")
|
|
rendered = integration._render_skill(
|
|
"constitution",
|
|
{"description": "Prüfe Konformität der Implementierung"},
|
|
"Body",
|
|
)
|
|
assert "Prüfe Konformität" in rendered
|
|
|
|
def test_setup_does_not_write_context_section(self, tmp_path):
|
|
"""The CLI no longer manages the agent context file — that is owned by
|
|
the opt-in agent-context extension. Setup must not create or touch it."""
|
|
integration = get_integration("claude")
|
|
manifest = IntegrationManifest("claude", tmp_path)
|
|
integration.setup(tmp_path, manifest, script_type="sh")
|
|
|
|
for path in tmp_path.rglob("*"):
|
|
if path.is_file():
|
|
text = path.read_text(encoding="utf-8", errors="ignore")
|
|
assert "<!-- SPECKIT START -->" not in text
|
|
|
|
def test_teardown_does_not_touch_existing_context_file(self, tmp_path):
|
|
"""A user-authored context file is left intact on teardown."""
|
|
integration = get_integration("claude")
|
|
ctx_path = tmp_path / "CLAUDE.md"
|
|
original = "# CLAUDE.md\n\nUser content.\n"
|
|
ctx_path.write_text(original, encoding="utf-8")
|
|
|
|
manifest = IntegrationManifest("claude", tmp_path)
|
|
integration.setup(tmp_path, manifest, script_type="sh")
|
|
integration.teardown(tmp_path, manifest)
|
|
|
|
assert ctx_path.read_text(encoding="utf-8") == original
|
|
|
|
def test_integration_flag_creates_skill_files_cli(self, tmp_path):
|
|
from typer.testing import CliRunner
|
|
from specify_cli import app
|
|
|
|
project = tmp_path / "claude-promote"
|
|
project.mkdir()
|
|
old_cwd = os.getcwd()
|
|
try:
|
|
os.chdir(project)
|
|
runner = CliRunner()
|
|
result = runner.invoke(
|
|
app,
|
|
[
|
|
"init",
|
|
"--here",
|
|
"--integration",
|
|
"claude",
|
|
"--script",
|
|
"sh",
|
|
"--ignore-agent-tools",
|
|
],
|
|
catch_exceptions=False,
|
|
)
|
|
finally:
|
|
os.chdir(old_cwd)
|
|
|
|
assert result.exit_code == 0, result.output
|
|
assert (project / ".claude" / "skills" / "speckit-plan" / "SKILL.md").exists()
|
|
assert not (project / ".claude" / "commands").exists()
|
|
|
|
init_options = json.loads(
|
|
(project / ".specify" / "init-options.json").read_text(encoding="utf-8")
|
|
)
|
|
assert init_options["ai"] == "claude"
|
|
assert init_options["ai_skills"] is True
|
|
assert init_options["integration"] == "claude"
|
|
|
|
def test_integration_flag_creates_skill_files(self, tmp_path):
|
|
from typer.testing import CliRunner
|
|
from specify_cli import app
|
|
|
|
project = tmp_path / "claude-integration"
|
|
project.mkdir()
|
|
old_cwd = os.getcwd()
|
|
try:
|
|
os.chdir(project)
|
|
runner = CliRunner()
|
|
result = runner.invoke(
|
|
app,
|
|
[
|
|
"init",
|
|
"--here",
|
|
"--integration",
|
|
"claude",
|
|
"--script",
|
|
"sh",
|
|
"--ignore-agent-tools",
|
|
],
|
|
catch_exceptions=False,
|
|
)
|
|
finally:
|
|
os.chdir(old_cwd)
|
|
|
|
assert result.exit_code == 0, result.output
|
|
assert (project / ".claude" / "skills" / "speckit-specify" / "SKILL.md").exists()
|
|
assert (project / ".specify" / "integrations" / "claude.manifest.json").exists()
|
|
|
|
def test_interactive_claude_selection_uses_integration_path(self, tmp_path):
|
|
from typer.testing import CliRunner
|
|
from specify_cli import app
|
|
|
|
project = tmp_path / "claude-interactive"
|
|
project.mkdir()
|
|
old_cwd = os.getcwd()
|
|
try:
|
|
os.chdir(project)
|
|
runner = CliRunner()
|
|
with (
|
|
patch("specify_cli.commands.init._stdin_is_interactive", return_value=True),
|
|
patch("specify_cli.commands.init.select_with_arrows", return_value="claude"),
|
|
):
|
|
result = runner.invoke(
|
|
app,
|
|
[
|
|
"init",
|
|
"--here",
|
|
"--script",
|
|
"sh",
|
|
"--ignore-agent-tools",
|
|
],
|
|
catch_exceptions=False,
|
|
)
|
|
finally:
|
|
os.chdir(old_cwd)
|
|
|
|
assert result.exit_code == 0, result.output
|
|
assert (project / ".specify" / "integration.json").exists()
|
|
assert (project / ".specify" / "integrations" / "claude.manifest.json").exists()
|
|
|
|
skill_file = project / ".claude" / "skills" / "speckit-plan" / "SKILL.md"
|
|
assert skill_file.exists()
|
|
skill_content = skill_file.read_text(encoding="utf-8")
|
|
assert "user-invocable: true" in skill_content
|
|
assert "disable-model-invocation: false" in skill_content
|
|
|
|
init_options = json.loads(
|
|
(project / ".specify" / "init-options.json").read_text(encoding="utf-8")
|
|
)
|
|
assert init_options["ai"] == "claude"
|
|
assert init_options["ai_skills"] is True
|
|
assert init_options["integration"] == "claude"
|
|
|
|
def test_claude_init_remains_usable_when_converter_fails(self, tmp_path):
|
|
"""Claude init should succeed even without install_skills."""
|
|
from typer.testing import CliRunner
|
|
from specify_cli import app
|
|
|
|
runner = CliRunner()
|
|
target = tmp_path / "fail-proj"
|
|
|
|
result = runner.invoke(
|
|
app,
|
|
["init", str(target), "--integration", "claude", "--script", "sh", "--ignore-agent-tools"],
|
|
)
|
|
|
|
assert result.exit_code == 0
|
|
assert (target / ".claude" / "skills" / "speckit-specify" / "SKILL.md").exists()
|
|
|
|
def test_claude_hooks_render_skill_invocation(self, tmp_path):
|
|
from specify_cli.extensions import HookExecutor
|
|
|
|
project = tmp_path / "claude-hooks"
|
|
project.mkdir()
|
|
init_options = project / ".specify" / "init-options.json"
|
|
init_options.parent.mkdir(parents=True, exist_ok=True)
|
|
init_options.write_text(json.dumps({"ai": "claude", "ai_skills": True}))
|
|
|
|
hook_executor = HookExecutor(project)
|
|
message = hook_executor.format_hook_message(
|
|
"before_plan",
|
|
[
|
|
{
|
|
"extension": "test-ext",
|
|
"command": "speckit.plan",
|
|
"optional": False,
|
|
}
|
|
],
|
|
)
|
|
|
|
assert "Executing: `/speckit-plan`" in message
|
|
assert "EXECUTE_COMMAND: speckit.plan" in message
|
|
assert "EXECUTE_COMMAND_INVOCATION: /speckit-plan" in message
|
|
|
|
def test_claude_preset_creates_new_skill_without_commands_dir(self, tmp_path):
|
|
from specify_cli import save_init_options
|
|
from specify_cli.presets import PresetManager
|
|
|
|
project = tmp_path / "claude-preset-skill"
|
|
project.mkdir()
|
|
save_init_options(project, {"ai": "claude", "ai_skills": True, "script": "sh"})
|
|
|
|
skills_dir = project / ".claude" / "skills"
|
|
skills_dir.mkdir(parents=True, exist_ok=True)
|
|
|
|
preset_dir = tmp_path / "claude-skill-command"
|
|
preset_dir.mkdir()
|
|
(preset_dir / "commands").mkdir()
|
|
(preset_dir / "commands" / "speckit.research.md").write_text(
|
|
"---\n"
|
|
"description: Research workflow\n"
|
|
"---\n\n"
|
|
"preset:claude-skill-command\n"
|
|
)
|
|
manifest_data = {
|
|
"schema_version": "1.0",
|
|
"preset": {
|
|
"id": "claude-skill-command",
|
|
"name": "Claude Skill Command",
|
|
"version": "1.0.0",
|
|
"description": "Test",
|
|
},
|
|
"requires": {"speckit_version": ">=0.1.0"},
|
|
"provides": {
|
|
"templates": [
|
|
{
|
|
"type": "command",
|
|
"name": "speckit.research",
|
|
"file": "commands/speckit.research.md",
|
|
}
|
|
]
|
|
},
|
|
}
|
|
with open(preset_dir / "preset.yml", "w") as f:
|
|
yaml.dump(manifest_data, f)
|
|
|
|
manager = PresetManager(project)
|
|
manager.install_from_directory(preset_dir, "0.1.5")
|
|
|
|
skill_file = skills_dir / "speckit-research" / "SKILL.md"
|
|
assert skill_file.exists()
|
|
content = skill_file.read_text(encoding="utf-8")
|
|
assert "preset:claude-skill-command" in content
|
|
assert "name: speckit-research" in content
|
|
assert "user-invocable: true" in content
|
|
assert "disable-model-invocation: false" in content
|
|
|
|
metadata = manager.registry.get("claude-skill-command")
|
|
assert "speckit-research" in metadata.get("registered_skills", {}).get("claude", [])
|
|
|
|
|
|
class TestClaudeArgumentHints:
|
|
"""Verify that argument-hint frontmatter is injected for Claude skills."""
|
|
|
|
def test_converge_has_no_argument_hint(self):
|
|
"""Converge should not advertise unsupported feature-name arguments."""
|
|
assert "converge" not in ARGUMENT_HINTS
|
|
|
|
def test_all_skills_have_hints(self, tmp_path):
|
|
"""Every skill with a configured hint must contain an argument-hint line."""
|
|
i = get_integration("claude")
|
|
m = IntegrationManifest("claude", tmp_path)
|
|
created = i.setup(tmp_path, m, script_type="sh")
|
|
skill_files = [f for f in created if f.name == "SKILL.md"]
|
|
assert len(skill_files) > 0
|
|
for f in skill_files:
|
|
stem = f.parent.name
|
|
if stem.startswith("speckit-"):
|
|
stem = stem[len("speckit-"):]
|
|
content = f.read_text(encoding="utf-8")
|
|
if stem in ARGUMENT_HINTS:
|
|
assert "argument-hint:" in content, (
|
|
f"{f.parent.name}/SKILL.md is missing argument-hint frontmatter"
|
|
)
|
|
else:
|
|
assert "argument-hint:" not in content, (
|
|
f"{f.parent.name}/SKILL.md unexpectedly has argument-hint frontmatter"
|
|
)
|
|
|
|
def test_hints_match_expected_values(self, tmp_path):
|
|
"""Each skill's argument-hint must match the expected text."""
|
|
i = get_integration("claude")
|
|
m = IntegrationManifest("claude", tmp_path)
|
|
created = i.setup(tmp_path, m, script_type="sh")
|
|
skill_files = [f for f in created if f.name == "SKILL.md"]
|
|
for f in skill_files:
|
|
# Extract stem: speckit-plan -> plan
|
|
stem = f.parent.name
|
|
if stem.startswith("speckit-"):
|
|
stem = stem[len("speckit-"):]
|
|
expected_hint = ARGUMENT_HINTS.get(stem)
|
|
content = f.read_text(encoding="utf-8")
|
|
if expected_hint is None:
|
|
assert "argument-hint:" not in content, (
|
|
f"{f.parent.name}/SKILL.md unexpectedly has argument-hint frontmatter"
|
|
)
|
|
else:
|
|
assert f'argument-hint: "{expected_hint}"' in content, (
|
|
f"{f.parent.name}/SKILL.md: expected hint '{expected_hint}' not found"
|
|
)
|
|
|
|
def test_hint_is_inside_frontmatter(self, tmp_path):
|
|
"""argument-hint must appear between the --- delimiters, not in the body."""
|
|
i = get_integration("claude")
|
|
m = IntegrationManifest("claude", tmp_path)
|
|
created = i.setup(tmp_path, m, script_type="sh")
|
|
skill_files = [f for f in created if f.name == "SKILL.md"]
|
|
for f in skill_files:
|
|
content = f.read_text(encoding="utf-8")
|
|
parts = content.split("---", 2)
|
|
assert len(parts) >= 3, f"No frontmatter in {f.parent.name}/SKILL.md"
|
|
frontmatter = parts[1]
|
|
body = parts[2]
|
|
stem = f.parent.name
|
|
if stem.startswith("speckit-"):
|
|
stem = stem[len("speckit-"):]
|
|
if stem in ARGUMENT_HINTS:
|
|
assert "argument-hint:" in frontmatter, (
|
|
f"{f.parent.name}/SKILL.md: argument-hint not in frontmatter section"
|
|
)
|
|
assert "argument-hint:" not in body, (
|
|
f"{f.parent.name}/SKILL.md: argument-hint leaked into body"
|
|
)
|
|
else:
|
|
assert "argument-hint:" not in content, (
|
|
f"{f.parent.name}/SKILL.md unexpectedly has argument-hint frontmatter"
|
|
)
|
|
|
|
def test_hint_appears_after_description(self, tmp_path):
|
|
"""argument-hint must immediately follow the description line."""
|
|
i = get_integration("claude")
|
|
m = IntegrationManifest("claude", tmp_path)
|
|
created = i.setup(tmp_path, m, script_type="sh")
|
|
skill_files = [f for f in created if f.name == "SKILL.md"]
|
|
for f in skill_files:
|
|
content = f.read_text(encoding="utf-8")
|
|
lines = content.splitlines()
|
|
stem = f.parent.name
|
|
if stem.startswith("speckit-"):
|
|
stem = stem[len("speckit-"):]
|
|
if stem not in ARGUMENT_HINTS:
|
|
assert "argument-hint:" not in content, (
|
|
f"{f.parent.name}/SKILL.md unexpectedly has argument-hint frontmatter"
|
|
)
|
|
continue
|
|
found_description = False
|
|
for idx, line in enumerate(lines):
|
|
if line.startswith("description:"):
|
|
found_description = True
|
|
assert idx + 1 < len(lines), (
|
|
f"{f.parent.name}/SKILL.md: description is last line"
|
|
)
|
|
assert lines[idx + 1].startswith("argument-hint:"), (
|
|
f"{f.parent.name}/SKILL.md: argument-hint does not follow description"
|
|
)
|
|
break
|
|
assert found_description, (
|
|
f"{f.parent.name}/SKILL.md: no description: line found in output"
|
|
)
|
|
|
|
def test_inject_argument_hint_only_in_frontmatter(self):
|
|
"""inject_argument_hint must not modify description: lines in the body."""
|
|
from specify_cli.integrations.claude import ClaudeIntegration
|
|
|
|
content = (
|
|
"---\n"
|
|
"description: My command\n"
|
|
"---\n"
|
|
"\n"
|
|
"description: this is body text\n"
|
|
)
|
|
result = ClaudeIntegration.inject_argument_hint(content, "Test hint")
|
|
lines = result.splitlines()
|
|
hint_count = sum(1 for ln in lines if ln.startswith("argument-hint:"))
|
|
assert hint_count == 1, (
|
|
f"Expected exactly 1 argument-hint line, found {hint_count}"
|
|
)
|
|
|
|
def test_inject_argument_hint_skips_if_already_present(self):
|
|
"""inject_argument_hint must not duplicate if argument-hint already exists."""
|
|
from specify_cli.integrations.claude import ClaudeIntegration
|
|
|
|
content = (
|
|
"---\n"
|
|
"description: My command\n"
|
|
'argument-hint: "Existing hint"\n'
|
|
"---\n"
|
|
"\n"
|
|
"Body text\n"
|
|
)
|
|
result = ClaudeIntegration.inject_argument_hint(content, "New hint")
|
|
assert result == content, "Content should be unchanged when hint already exists"
|
|
lines = result.splitlines()
|
|
hint_count = sum(1 for ln in lines if ln.startswith("argument-hint:"))
|
|
assert hint_count == 1
|
|
|
|
def test_inject_argument_hint_survives_folded_description(self):
|
|
"""A long description folded across lines must not corrupt the YAML (#4044).
|
|
|
|
A description long enough for the YAML dumper to fold it into a
|
|
multi-line plain scalar previously had ``argument-hint:`` spliced
|
|
into the *middle* of that scalar, producing invalid YAML.
|
|
"""
|
|
from specify_cli.integrations.claude import ClaudeIntegration
|
|
|
|
frontmatter = {
|
|
"name": "speckit-specify",
|
|
"description": (
|
|
"Create or update the feature specification from a natural "
|
|
"language feature description. Also accepts an issue URL "
|
|
"resolved via gh CLI (demo customization)."
|
|
),
|
|
"compatibility": "Requires spec-kit project structure with .specify/ directory",
|
|
}
|
|
frontmatter_text = yaml.safe_dump(
|
|
frontmatter, sort_keys=False, allow_unicode=True
|
|
).strip()
|
|
content = f"---\n{frontmatter_text}\n---\n\nBody text\n"
|
|
assert "\n " in content, "fixture description must actually fold across lines"
|
|
|
|
result = ClaudeIntegration.inject_argument_hint(content, "Describe the feature")
|
|
|
|
parsed = yaml.safe_load(result.split("---")[1])
|
|
assert parsed["argument-hint"] == "Describe the feature"
|
|
assert parsed["description"] == frontmatter["description"]
|
|
|
|
def test_inject_argument_hint_survives_quoted_folded_description(self):
|
|
"""A folded description forced into quotes must not absorb the hint (#4044)."""
|
|
from specify_cli.integrations.claude import ClaudeIntegration
|
|
|
|
frontmatter = {
|
|
"name": "speckit-specify",
|
|
"description": (
|
|
"Create or update the feature specification from a natural "
|
|
"language feature description. Also accepts a GitHub "
|
|
"issue/PR URL or #N reference resolved via gh CLI (demo)."
|
|
),
|
|
"compatibility": "Requires spec-kit project structure with .specify/ directory",
|
|
}
|
|
frontmatter_text = yaml.safe_dump(
|
|
frontmatter, sort_keys=False, allow_unicode=True
|
|
).strip()
|
|
content = f"---\n{frontmatter_text}\n---\n\nBody text\n"
|
|
assert "\n " in content, "fixture description must actually fold across lines"
|
|
|
|
result = ClaudeIntegration.inject_argument_hint(content, "Describe the feature")
|
|
|
|
parsed = yaml.safe_load(result.split("---")[1])
|
|
assert parsed["argument-hint"] == "Describe the feature"
|
|
assert parsed["description"] == frontmatter["description"]
|
|
|
|
def test_inject_argument_hint_survives_multi_paragraph_description(self):
|
|
"""A description with an embedded blank line must not absorb the hint.
|
|
|
|
PyYAML serializes an embedded ``\\n\\n`` inside a quoted scalar as
|
|
unindented blank lines, not indented ones, so a fix that only skips
|
|
indented continuation lines still fails on this case.
|
|
"""
|
|
from specify_cli.integrations.claude import ClaudeIntegration
|
|
|
|
frontmatter = {
|
|
"name": "speckit-specify",
|
|
"description": (
|
|
"First paragraph of a fairly long description that will "
|
|
"need to wrap across multiple lines when dumped by PyYAML."
|
|
"\n\n"
|
|
"Second paragraph continues the description after a blank "
|
|
"line separator to force embedded newlines in the scalar."
|
|
),
|
|
"compatibility": "Requires spec-kit project structure with .specify/ directory",
|
|
}
|
|
frontmatter_text = yaml.safe_dump(
|
|
frontmatter, sort_keys=False, allow_unicode=True
|
|
).strip()
|
|
content = f"---\n{frontmatter_text}\n---\n\nBody text\n"
|
|
assert "\n\n" in frontmatter_text, "fixture must produce a blank continuation line"
|
|
|
|
result = ClaudeIntegration.inject_argument_hint(content, "Describe the feature")
|
|
|
|
parsed = yaml.safe_load(result.split("---")[1])
|
|
assert parsed["argument-hint"] == "Describe the feature"
|
|
assert parsed["description"] == frontmatter["description"]
|
|
|
|
|
|
class TestClaudeDisableModelInvocation:
|
|
"""Verify disable-model-invocation is false for Claude skills."""
|
|
|
|
def test_setup_sets_disable_model_invocation_false(self, tmp_path):
|
|
"""Generated SKILL.md files must have disable-model-invocation: false."""
|
|
i = get_integration("claude")
|
|
m = IntegrationManifest("claude", tmp_path)
|
|
created = i.setup(tmp_path, m, script_type="sh")
|
|
skill_files = [f for f in created if f.name == "SKILL.md"]
|
|
assert len(skill_files) > 0
|
|
for f in skill_files:
|
|
content = f.read_text(encoding="utf-8")
|
|
parts = content.split("---", 2)
|
|
parsed = yaml.safe_load(parts[1])
|
|
assert parsed["disable-model-invocation"] is False, (
|
|
f"{f.parent.name}: expected disable-model-invocation: false"
|
|
)
|
|
|
|
def test_disable_model_invocation_not_true(self, tmp_path):
|
|
"""No Claude skill should have disable-model-invocation: true."""
|
|
i = get_integration("claude")
|
|
m = IntegrationManifest("claude", tmp_path)
|
|
created = i.setup(tmp_path, m, script_type="sh")
|
|
for f in created:
|
|
if f.name != "SKILL.md":
|
|
continue
|
|
content = f.read_text(encoding="utf-8")
|
|
assert "disable-model-invocation: true" not in content, (
|
|
f"{f.parent.name}: must not have disable-model-invocation: true"
|
|
)
|
|
|
|
def test_non_claude_agents_lack_disable_model_invocation(self, tmp_path):
|
|
"""Non-Claude skill agents should not get disable-model-invocation."""
|
|
from specify_cli.agents import CommandRegistrar
|
|
|
|
fm = CommandRegistrar.build_skill_frontmatter(
|
|
"codex", "speckit-plan", "desc", "templates/commands/plan.md"
|
|
)
|
|
assert "disable-model-invocation" not in fm
|
|
assert "user-invocable" not in fm
|
|
|
|
def test_skills_default_post_process_preserves_content_without_hooks(self, tmp_path):
|
|
"""SkillsIntegration agents without an override preserve non-hook content."""
|
|
# ``agy`` is a plain SkillsIntegration with no post-process override,
|
|
# so it stands in for the base-class default behavior.
|
|
agy = get_integration("agy")
|
|
if agy is None:
|
|
return # agy not registered in this build
|
|
content = "---\nname: test\n---\nBody"
|
|
assert agy.post_process_skill_content(content) == content
|
|
|
|
|
|
class TestClaudeForkContext:
|
|
"""Verify context: fork is injected only for commands listed in FORK_CONTEXT_COMMANDS."""
|
|
|
|
def test_no_commands_fork_by_default(self):
|
|
"""FORK_CONTEXT_COMMANDS is empty: no command opts into context: fork.
|
|
|
|
``analyze`` was removed (#3185) because its verbose report defeated the
|
|
purpose of forking and compounded context overhead across repeated runs.
|
|
"""
|
|
assert FORK_CONTEXT_COMMANDS == {}
|
|
|
|
def test_analyze_skill_does_not_fork(self, tmp_path):
|
|
"""speckit-analyze must run in the main session, not a forked subagent (#3185)."""
|
|
i = get_integration("claude")
|
|
m = IntegrationManifest("claude", tmp_path)
|
|
i.setup(tmp_path, m, script_type="sh")
|
|
analyze_skill = tmp_path / ".claude/skills/speckit-analyze/SKILL.md"
|
|
assert analyze_skill.exists()
|
|
content = analyze_skill.read_text(encoding="utf-8")
|
|
parts = content.split("---", 2)
|
|
parsed = yaml.safe_load(parts[1])
|
|
assert "context" not in parsed
|
|
assert "agent" not in parsed
|
|
|
|
def test_no_skills_fork(self, tmp_path):
|
|
"""Skills not in FORK_CONTEXT_COMMANDS must not get context: fork."""
|
|
i = get_integration("claude")
|
|
m = IntegrationManifest("claude", tmp_path)
|
|
created = i.setup(tmp_path, m, script_type="sh")
|
|
skill_files = [f for f in created if f.name == "SKILL.md"]
|
|
for f in skill_files:
|
|
stem = f.parent.name
|
|
if stem.startswith("speckit-"):
|
|
stem = stem[len("speckit-"):]
|
|
if stem in FORK_CONTEXT_COMMANDS:
|
|
continue
|
|
content = f.read_text(encoding="utf-8")
|
|
parts = content.split("---", 2)
|
|
parsed = yaml.safe_load(parts[1])
|
|
assert "context" not in parsed, (
|
|
f"{f.parent.name}: must not have context frontmatter"
|
|
)
|
|
assert "agent" not in parsed, (
|
|
f"{f.parent.name}: must not have agent frontmatter"
|
|
)
|
|
|
|
def test_post_process_no_fork_for_skills(self):
|
|
"""With FORK_CONTEXT_COMMANDS empty, post_process must not add context/agent."""
|
|
i = get_integration("claude")
|
|
for name in ("speckit-analyze", "speckit-plan"):
|
|
content = f'---\nname: "{name}"\ndescription: "x"\n---\n\nBody\n'
|
|
result = i.post_process_skill_content(content)
|
|
parsed = yaml.safe_load(result.split("---", 2)[1])
|
|
assert "context" not in parsed
|
|
assert "agent" not in parsed
|
|
|
|
def test_fork_mechanism_injects_when_configured(self, monkeypatch):
|
|
"""The injection mechanism still works for any command added to
|
|
FORK_CONTEXT_COMMANDS, even though none ships enabled by default."""
|
|
import specify_cli.integrations.claude as claude_mod
|
|
|
|
monkeypatch.setitem(
|
|
claude_mod.FORK_CONTEXT_COMMANDS,
|
|
"analyze",
|
|
{"context": "fork", "agent": "general-purpose"},
|
|
)
|
|
i = get_integration("claude")
|
|
content = '---\nname: "speckit-analyze"\ndescription: "x"\n---\n\nBody\n'
|
|
result = i.post_process_skill_content(content)
|
|
parts = result.split("---", 2)
|
|
parsed = yaml.safe_load(parts[1])
|
|
assert parsed.get("context") == "fork"
|
|
assert parsed.get("agent") == "general-purpose"
|
|
# Flags must land in the frontmatter, not the body.
|
|
assert "context: fork" in parts[1]
|
|
assert "context: fork" not in parts[2]
|
|
# Re-running must not duplicate the injected keys.
|
|
twice = i.post_process_skill_content(result)
|
|
assert result == twice
|
|
assert twice.count("context: fork") == 1
|
|
assert twice.count("agent: general-purpose") == 1
|
|
|
|
|
|
class TestClaudeHookCommandNote:
|
|
"""Verify dot-to-hyphen normalization note is injected in hook sections."""
|
|
|
|
def test_hook_note_injected_in_skills_with_hooks(self, tmp_path):
|
|
"""Skills that have hook sections should get the normalization note."""
|
|
i = get_integration("claude")
|
|
m = IntegrationManifest("claude", tmp_path)
|
|
i.setup(tmp_path, m, script_type="sh")
|
|
specify_skill = tmp_path / ".claude/skills/speckit-specify/SKILL.md"
|
|
assert specify_skill.exists()
|
|
content = specify_skill.read_text(encoding="utf-8")
|
|
# specify.md has hook sections
|
|
assert "replace dots" in content, (
|
|
"speckit-specify should have dot-to-hyphen hook note"
|
|
)
|
|
|
|
def test_hook_note_not_in_skills_without_hooks(self, tmp_path):
|
|
"""Skills without hook sections should not get the note."""
|
|
content = "---\nname: test\ndescription: test\n---\n\nNo hooks here.\n"
|
|
result = SkillsIntegration._inject_hook_command_note(content)
|
|
assert "replace dots" not in result
|
|
|
|
def test_hook_note_idempotent(self, tmp_path):
|
|
"""Injecting the note twice should not duplicate it."""
|
|
content = (
|
|
"---\nname: test\n---\n\n"
|
|
"- For each executable hook, output the following based on its flag:\n"
|
|
)
|
|
once = SkillsIntegration._inject_hook_command_note(content)
|
|
twice = SkillsIntegration._inject_hook_command_note(once)
|
|
assert once == twice, "Hook note injection should be idempotent"
|
|
|
|
def test_hook_note_fills_missing_repeated_instructions(self, tmp_path):
|
|
"""Already-noted hook sections should not suppress later sections."""
|
|
from specify_cli.integrations.base import _HOOK_COMMAND_NOTE
|
|
|
|
content = (
|
|
"---\nname: test\n---\n\n"
|
|
f"{_HOOK_COMMAND_NOTE}"
|
|
"- For each executable hook, output the following based on its flag:\n"
|
|
"\n"
|
|
" - For each executable hook, output the following based on its flag:\n"
|
|
)
|
|
result = SkillsIntegration._inject_hook_command_note(content)
|
|
assert result.count("replace dots (`.`) with hyphens") == 2
|
|
|
|
def test_hook_note_not_suppressed_by_unrelated_phrase(self, tmp_path):
|
|
"""Unrelated text should not trip the hook-note idempotence guard."""
|
|
content = (
|
|
"---\nname: test\n---\n\n"
|
|
"This paragraph says replace dots in a different context.\n"
|
|
"- For each executable hook, output the following based on its flag:\n"
|
|
)
|
|
result = SkillsIntegration._inject_hook_command_note(content)
|
|
assert "This paragraph says replace dots in a different context." in result
|
|
assert result.count("replace dots (`.`) with hyphens") == 1
|
|
|
|
def test_hook_note_preserves_indentation(self, tmp_path):
|
|
"""The injected note should match the indentation of the target line."""
|
|
content = (
|
|
"---\nname: test\n---\n\n"
|
|
" - For each executable hook, output the following\n"
|
|
)
|
|
result = SkillsIntegration._inject_hook_command_note(content)
|
|
lines = result.splitlines()
|
|
note_line = [line for line in lines if "replace dots" in line][0]
|
|
assert note_line.startswith(" "), "Note should preserve indentation"
|
|
|
|
def test_post_process_injects_all_claude_flags(self):
|
|
"""post_process_skill_content should inject all Claude-specific fields."""
|
|
i = get_integration("claude")
|
|
content = (
|
|
"---\nname: test\ndescription: test\n---\n\n"
|
|
"- For each executable hook, output the following\n"
|
|
)
|
|
result = i.post_process_skill_content(content)
|
|
assert "user-invocable: true" in result
|
|
assert "disable-model-invocation: false" in result
|
|
assert "replace dots" in result
|
|
|
|
|
|
class TestSpeckitManifestRecordsSkippedFiles:
|
|
"""Regression test for issue #2107.
|
|
|
|
``install_shared_infra`` must record every shared-infrastructure file
|
|
under ``.specify/`` in ``speckit.manifest.json``, including files that
|
|
were *skipped* because they already existed on disk and ``force=False``.
|
|
|
|
Before the fix, the skip branches in the scripts and templates loops
|
|
appended to ``skipped_files`` without calling ``manifest.record_existing``.
|
|
So when ``install_shared_infra`` ran with a fresh (or lost) manifest
|
|
against an already-populated ``.specify/`` tree, every file went down the
|
|
skip path, ``planned_copies`` and ``planned_templates`` stayed empty, and
|
|
``manifest.save()`` wrote an empty ``files`` field — leaving the
|
|
integration believing nothing was installed.
|
|
|
|
Reproduction (without the fix) using ``install_shared_infra`` directly:
|
|
|
|
install_shared_infra(p, "sh", ..., force=False) # 1st run → 10 files
|
|
(p / ".specify/integrations/speckit.manifest.json").unlink()
|
|
install_shared_infra(p, "sh", ..., force=False) # 2nd run → 0 files
|
|
# ^^ BUG: empty
|
|
"""
|
|
|
|
def _read_manifest_files(self, project_path: Path) -> dict:
|
|
manifest_path = (
|
|
project_path / ".specify" / "integrations" / "speckit.manifest.json"
|
|
)
|
|
assert manifest_path.exists(), (
|
|
f"speckit.manifest.json not written at {manifest_path}"
|
|
)
|
|
data = json.loads(manifest_path.read_text(encoding="utf-8"))
|
|
# ``IntegrationManifest.save`` serialises a ``files`` dict — assert
|
|
# the schema explicitly so a regression to a different key (e.g.
|
|
# the internal ``_files`` attribute name) fails loudly instead of
|
|
# being masked by a silent fallback.
|
|
assert isinstance(data, dict), (
|
|
f"manifest root is not a dict, got {type(data).__name__}"
|
|
)
|
|
assert "files" in data, (
|
|
f"manifest missing 'files' key, got keys: {sorted(data.keys())}"
|
|
)
|
|
files = data["files"]
|
|
assert isinstance(files, dict), (
|
|
f"manifest 'files' is not a dict, got {type(files).__name__}"
|
|
)
|
|
return files
|
|
|
|
def test_install_shared_infra_records_skipped_files(self, tmp_path):
|
|
"""With ``force=False`` and ``.specify/`` already populated, the
|
|
manifest must still record every file — the skip branches are not
|
|
allowed to drop files from the manifest."""
|
|
from rich.console import Console
|
|
from specify_cli.shared_infra import install_shared_infra
|
|
|
|
# Resolve the project's own packaged sources by walking up from this
|
|
# test file to the repo root (which contains ``scripts/`` and
|
|
# ``templates/`` that ``shared_scripts_source`` looks for).
|
|
repo_root = Path(__file__).resolve().parents[2]
|
|
console = Console(quiet=True)
|
|
|
|
# First run — fresh project, manifest gets populated normally.
|
|
install_shared_infra(
|
|
tmp_path,
|
|
"sh",
|
|
version="0.0.0",
|
|
core_pack=None,
|
|
repo_root=repo_root,
|
|
console=console,
|
|
force=False,
|
|
)
|
|
first_files = self._read_manifest_files(tmp_path)
|
|
assert first_files, "first install produced an empty manifest"
|
|
|
|
# Simulate a lost manifest while ``.specify/`` is still on disk
|
|
# (e.g. the manifest was deleted, corrupted, or the layout was
|
|
# extracted out-of-band).
|
|
manifest_path = (
|
|
tmp_path / ".specify" / "integrations" / "speckit.manifest.json"
|
|
)
|
|
manifest_path.unlink()
|
|
|
|
# Second run — every file already exists, so every iteration takes
|
|
# the skip branch. With the fix, those files are still recorded.
|
|
install_shared_infra(
|
|
tmp_path,
|
|
"sh",
|
|
version="0.0.0",
|
|
core_pack=None,
|
|
repo_root=repo_root,
|
|
console=console,
|
|
force=False,
|
|
)
|
|
second_files = self._read_manifest_files(tmp_path)
|
|
assert second_files, (
|
|
"speckit.manifest.json files dict is empty after install with "
|
|
"skipped files (issue #2107) — every file went down the skip "
|
|
"branch but none were recorded"
|
|
)
|
|
|
|
# The recovered manifest must cover everything the first run tracked.
|
|
missing = set(first_files) - set(second_files)
|
|
assert not missing, (
|
|
f"these files were tracked on the first install but missing after "
|
|
f"the skipped-files re-install: {sorted(missing)[:5]}"
|
|
)
|
|
|
|
def test_install_shared_infra_handles_directory_at_script_destination(
|
|
self, tmp_path
|
|
):
|
|
"""A non-file (directory) at a script's destination must NOT crash
|
|
``install_shared_infra`` and must NOT be recorded in the manifest —
|
|
the path still appears in the user-visible skipped-paths warning.
|
|
"""
|
|
from io import StringIO
|
|
from rich.console import Console
|
|
from specify_cli.shared_infra import install_shared_infra
|
|
|
|
repo_root = Path(__file__).resolve().parents[2]
|
|
output = StringIO()
|
|
console = Console(file=output, force_terminal=False, width=200)
|
|
|
|
# Pre-create the .specify/scripts/bash tree, then plant a directory
|
|
# where a script file is expected so the skip branch hits a
|
|
# non-regular-file path.
|
|
bash_dir = tmp_path / ".specify" / "scripts" / "bash"
|
|
bash_dir.mkdir(parents=True)
|
|
(bash_dir / "common.sh").mkdir() # collision: dir where file expected
|
|
|
|
# Must not crash.
|
|
install_shared_infra(
|
|
tmp_path,
|
|
"sh",
|
|
version="0.0.0",
|
|
core_pack=None,
|
|
repo_root=repo_root,
|
|
console=console,
|
|
force=False,
|
|
)
|
|
|
|
files = self._read_manifest_files(tmp_path)
|
|
assert ".specify/scripts/bash/common.sh" not in files, (
|
|
"directory at script dst must not be recorded in the manifest"
|
|
)
|
|
text = output.getvalue()
|
|
assert "common.sh" in text, (
|
|
"directory-at-script-dst path must surface in the skipped warning"
|
|
)
|
|
|
|
def test_install_shared_infra_handles_directory_at_template_destination(
|
|
self, tmp_path
|
|
):
|
|
"""Symmetric coverage for the templates loop: a directory at a
|
|
template's destination must NOT crash install nor be recorded."""
|
|
from io import StringIO
|
|
from rich.console import Console
|
|
from specify_cli.shared_infra import install_shared_infra
|
|
|
|
repo_root = Path(__file__).resolve().parents[2]
|
|
output = StringIO()
|
|
console = Console(file=output, force_terminal=False, width=200)
|
|
|
|
templates_dir = tmp_path / ".specify" / "templates"
|
|
templates_dir.mkdir(parents=True)
|
|
|
|
src_templates = repo_root / "templates"
|
|
real_template = next(
|
|
(
|
|
p.name
|
|
for p in src_templates.iterdir()
|
|
if p.is_file()
|
|
and not p.name.startswith(".")
|
|
and p.name != "vscode-settings.json"
|
|
),
|
|
None,
|
|
)
|
|
assert real_template, (
|
|
"no real template found in repo to collide against"
|
|
)
|
|
(templates_dir / real_template).mkdir() # collision
|
|
|
|
install_shared_infra(
|
|
tmp_path,
|
|
"sh",
|
|
version="0.0.0",
|
|
core_pack=None,
|
|
repo_root=repo_root,
|
|
console=console,
|
|
force=False,
|
|
)
|
|
|
|
files = self._read_manifest_files(tmp_path)
|
|
template_rel = f".specify/templates/{real_template}"
|
|
assert template_rel not in files, (
|
|
"directory at template dst must not be recorded in manifest"
|
|
)
|
|
text = output.getvalue()
|
|
assert real_template in text, (
|
|
"directory-at-template-dst path must surface in the skipped warning"
|
|
)
|