mirror of
https://github.com/github/spec-kit.git
synced 2026-03-17 19:03:08 +00:00
fix(ai-skills): exclude non-speckit copilot agent markdown from skill… (#1867)
* fix(ai-skills): exclude non-speckit copilot agent markdown from skill generation * Potential fix for pull request finding Fix missing `.agent` filename suffix Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Fix test assertion speckit.plan.md to speckit.plan.agent Fix test assertion speckit.plan.md to speckit.plan.agent * Fix filter glob based on review suggestions fix(ai-skills): normalize Copilot .agent template names and align template fallback filtering * Add template glob for fallback directory * GH Copilot Suggestions Clarify comment regarding Copilot's use of templates in tests. Add extra test assertion * fix(ai-skills): normalize Copilot .agent templates and preserve fallback behavior fix(ai-skills): handle Copilot .agent templates and fallback filtering Normalize Copilot command template names by stripping the .agent suffix when deriving skill names and metadata sources, so files like speckit.plan.agent.md produce speckit-plan and map to plan.md metadata. Also align Copilot template discovery with speckit.* filtering while preserving fallback to templates/commands/ when .github/agents contains only user-authored markdown files, and add regression coverage for both non-speckit agent exclusion and fallback behavior. * fix(ai-skills): ignore non-speckit markdown commands --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -1256,7 +1256,12 @@ def install_ai_skills(project_path: Path, selected_ai: str, tracker: StepTracker
|
||||
else:
|
||||
templates_dir = project_path / commands_subdir
|
||||
|
||||
if not templates_dir.exists() or not any(templates_dir.glob("*.md")):
|
||||
# Only consider speckit.*.md templates so that user-authored command
|
||||
# files (e.g. custom slash commands, agent files) coexisting in the
|
||||
# same commands directory are not incorrectly converted into skills.
|
||||
template_glob = "speckit.*.md"
|
||||
|
||||
if not templates_dir.exists() or not any(templates_dir.glob(template_glob)):
|
||||
# Fallback: try the repo-relative path (for running from source checkout)
|
||||
# This also covers agents whose extracted commands are in a different
|
||||
# format (e.g. gemini/tabnine use .toml, not .md).
|
||||
@@ -1264,15 +1269,16 @@ def install_ai_skills(project_path: Path, selected_ai: str, tracker: StepTracker
|
||||
fallback_dir = script_dir / "templates" / "commands"
|
||||
if fallback_dir.exists() and any(fallback_dir.glob("*.md")):
|
||||
templates_dir = fallback_dir
|
||||
template_glob = "*.md"
|
||||
|
||||
if not templates_dir.exists() or not any(templates_dir.glob("*.md")):
|
||||
if not templates_dir.exists() or not any(templates_dir.glob(template_glob)):
|
||||
if tracker:
|
||||
tracker.error("ai-skills", "command templates not found")
|
||||
else:
|
||||
console.print("[yellow]Warning: command templates not found, skipping skills installation[/yellow]")
|
||||
return False
|
||||
|
||||
command_files = sorted(templates_dir.glob("*.md"))
|
||||
command_files = sorted(templates_dir.glob(template_glob))
|
||||
if not command_files:
|
||||
if tracker:
|
||||
tracker.skip("ai-skills", "no command templates found")
|
||||
@@ -1311,11 +1317,14 @@ def install_ai_skills(project_path: Path, selected_ai: str, tracker: StepTracker
|
||||
body = content
|
||||
|
||||
command_name = command_file.stem
|
||||
# Normalize: extracted commands may be named "speckit.<cmd>.md";
|
||||
# strip the "speckit." prefix so skill names stay clean and
|
||||
# Normalize: extracted commands may be named "speckit.<cmd>.md"
|
||||
# or "speckit.<cmd>.agent.md"; strip the "speckit." prefix and
|
||||
# any trailing ".agent" suffix so skill names stay clean and
|
||||
# SKILL_DESCRIPTIONS lookups work.
|
||||
if command_name.startswith("speckit."):
|
||||
command_name = command_name[len("speckit."):]
|
||||
if command_name.endswith(".agent"):
|
||||
command_name = command_name[:-len(".agent")]
|
||||
# Kimi CLI discovers skills by directory name and invokes them as
|
||||
# /skill:<name> — use dot separator to match packaging convention.
|
||||
if selected_ai == "kimi":
|
||||
@@ -1340,6 +1349,8 @@ def install_ai_skills(project_path: Path, selected_ai: str, tracker: StepTracker
|
||||
source_name = command_file.name
|
||||
if source_name.startswith("speckit."):
|
||||
source_name = source_name[len("speckit."):]
|
||||
if source_name.endswith(".agent.md"):
|
||||
source_name = source_name[:-len(".agent.md")] + ".md"
|
||||
|
||||
frontmatter_data = {
|
||||
"name": skill_name,
|
||||
|
||||
@@ -62,7 +62,7 @@ def templates_dir(project_dir):
|
||||
tpl_root.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
# Template with valid YAML frontmatter
|
||||
(tpl_root / "specify.md").write_text(
|
||||
(tpl_root / "speckit.specify.md").write_text(
|
||||
"---\n"
|
||||
"description: Create or update the feature specification.\n"
|
||||
"handoffs:\n"
|
||||
@@ -79,7 +79,7 @@ def templates_dir(project_dir):
|
||||
)
|
||||
|
||||
# Template with minimal frontmatter
|
||||
(tpl_root / "plan.md").write_text(
|
||||
(tpl_root / "speckit.plan.md").write_text(
|
||||
"---\n"
|
||||
"description: Generate implementation plan.\n"
|
||||
"---\n"
|
||||
@@ -91,7 +91,7 @@ def templates_dir(project_dir):
|
||||
)
|
||||
|
||||
# Template with no frontmatter
|
||||
(tpl_root / "tasks.md").write_text(
|
||||
(tpl_root / "speckit.tasks.md").write_text(
|
||||
"# Tasks Command\n"
|
||||
"\n"
|
||||
"Body without frontmatter.\n",
|
||||
@@ -99,7 +99,7 @@ def templates_dir(project_dir):
|
||||
)
|
||||
|
||||
# Template with empty YAML frontmatter (yaml.safe_load returns None)
|
||||
(tpl_root / "empty_fm.md").write_text(
|
||||
(tpl_root / "speckit.empty_fm.md").write_text(
|
||||
"---\n"
|
||||
"---\n"
|
||||
"\n"
|
||||
@@ -337,7 +337,7 @@ class TestInstallAiSkills:
|
||||
cmds_dir = project_dir / ".claude" / "commands"
|
||||
cmds_dir.mkdir(parents=True)
|
||||
|
||||
(cmds_dir / "broken.md").write_text(
|
||||
(cmds_dir / "speckit.broken.md").write_text(
|
||||
"---\n"
|
||||
"description: [unclosed bracket\n"
|
||||
" invalid: yaml: content: here\n"
|
||||
@@ -430,9 +430,12 @@ class TestInstallAiSkills:
|
||||
|
||||
# Place .md templates in the agent's commands directory
|
||||
agent_folder = AGENT_CONFIG[agent_key]["folder"]
|
||||
cmds_dir = proj / agent_folder.rstrip("/") / "commands"
|
||||
commands_subdir = AGENT_CONFIG[agent_key].get("commands_subdir", "commands")
|
||||
cmds_dir = proj / agent_folder.rstrip("/") / commands_subdir
|
||||
cmds_dir.mkdir(parents=True)
|
||||
(cmds_dir / "specify.md").write_text(
|
||||
# Copilot uses speckit.*.agent.md templates; other agents use speckit.*.md
|
||||
fname = "speckit.specify.agent.md" if agent_key == "copilot" else "speckit.specify.md"
|
||||
(cmds_dir / fname).write_text(
|
||||
"---\ndescription: Test command\n---\n\n# Test\n\nBody.\n"
|
||||
)
|
||||
|
||||
@@ -448,7 +451,100 @@ class TestInstallAiSkills:
|
||||
assert expected_skill_name in skill_dirs
|
||||
assert (skills_dir / expected_skill_name / "SKILL.md").exists()
|
||||
|
||||
def test_copilot_ignores_non_speckit_agents(self, project_dir):
|
||||
"""Non-speckit markdown in .github/agents/ must not produce skills."""
|
||||
agents_dir = project_dir / ".github" / "agents"
|
||||
agents_dir.mkdir(parents=True, exist_ok=True)
|
||||
(agents_dir / "speckit.plan.agent.md").write_text(
|
||||
"---\ndescription: Generate implementation plan.\n---\n\n# Plan\n\nBody.\n"
|
||||
)
|
||||
(agents_dir / "my-custom-agent.agent.md").write_text(
|
||||
"---\ndescription: A user custom agent\n---\n\n# Custom\n\nBody.\n"
|
||||
)
|
||||
|
||||
result = install_ai_skills(project_dir, "copilot")
|
||||
|
||||
assert result is True
|
||||
skills_dir = _get_skills_dir(project_dir, "copilot")
|
||||
assert skills_dir.exists()
|
||||
skill_dirs = [d.name for d in skills_dir.iterdir() if d.is_dir()]
|
||||
assert "speckit-plan" in skill_dirs
|
||||
assert "speckit-my-custom-agent.agent" not in skill_dirs
|
||||
assert "speckit-my-custom-agent" not in skill_dirs
|
||||
|
||||
@pytest.mark.parametrize("agent_key,custom_file", [
|
||||
("claude", "review.md"),
|
||||
("cursor-agent", "deploy.md"),
|
||||
("qwen", "my-workflow.md"),
|
||||
])
|
||||
def test_non_speckit_commands_ignored_for_all_agents(self, temp_dir, agent_key, custom_file):
|
||||
"""User-authored command files must not produce skills for any agent."""
|
||||
proj = temp_dir / f"proj-{agent_key}"
|
||||
proj.mkdir()
|
||||
|
||||
agent_folder = AGENT_CONFIG[agent_key]["folder"]
|
||||
commands_subdir = AGENT_CONFIG[agent_key].get("commands_subdir", "commands")
|
||||
cmds_dir = proj / agent_folder.rstrip("/") / commands_subdir
|
||||
cmds_dir.mkdir(parents=True)
|
||||
(cmds_dir / "speckit.specify.md").write_text(
|
||||
"---\ndescription: Create spec.\n---\n\n# Specify\n\nBody.\n"
|
||||
)
|
||||
(cmds_dir / custom_file).write_text(
|
||||
"---\ndescription: User custom command\n---\n\n# Custom\n\nBody.\n"
|
||||
)
|
||||
|
||||
result = install_ai_skills(proj, agent_key)
|
||||
|
||||
assert result is True
|
||||
skills_dir = _get_skills_dir(proj, agent_key)
|
||||
skill_dirs = [d.name for d in skills_dir.iterdir() if d.is_dir()]
|
||||
assert "speckit-specify" in skill_dirs
|
||||
custom_stem = Path(custom_file).stem
|
||||
assert f"speckit-{custom_stem}" not in skill_dirs
|
||||
|
||||
def test_copilot_fallback_when_only_non_speckit_agents(self, project_dir):
|
||||
"""Fallback to templates/commands/ when .github/agents/ has no speckit.*.md files."""
|
||||
agents_dir = project_dir / ".github" / "agents"
|
||||
agents_dir.mkdir(parents=True, exist_ok=True)
|
||||
# Only a user-authored agent, no speckit.* templates
|
||||
(agents_dir / "my-custom-agent.agent.md").write_text(
|
||||
"---\ndescription: A user custom agent\n---\n\n# Custom\n\nBody.\n"
|
||||
)
|
||||
|
||||
result = install_ai_skills(project_dir, "copilot")
|
||||
|
||||
# Should succeed via fallback to templates/commands/
|
||||
assert result is True
|
||||
skills_dir = _get_skills_dir(project_dir, "copilot")
|
||||
assert skills_dir.exists()
|
||||
skill_dirs = [d.name for d in skills_dir.iterdir() if d.is_dir()]
|
||||
# Should have skills from fallback templates, not from the custom agent
|
||||
assert "speckit-plan" in skill_dirs
|
||||
assert not any("my-custom" in d for d in skill_dirs)
|
||||
|
||||
@pytest.mark.parametrize("agent_key", ["claude", "cursor-agent", "qwen"])
|
||||
def test_fallback_when_only_non_speckit_commands(self, temp_dir, agent_key):
|
||||
"""Fallback to templates/commands/ when agent dir has no speckit.*.md files."""
|
||||
proj = temp_dir / f"proj-{agent_key}"
|
||||
proj.mkdir()
|
||||
|
||||
agent_folder = AGENT_CONFIG[agent_key]["folder"]
|
||||
commands_subdir = AGENT_CONFIG[agent_key].get("commands_subdir", "commands")
|
||||
cmds_dir = proj / agent_folder.rstrip("/") / commands_subdir
|
||||
cmds_dir.mkdir(parents=True)
|
||||
# Only a user-authored command, no speckit.* templates
|
||||
(cmds_dir / "my-custom-command.md").write_text(
|
||||
"---\ndescription: User custom command\n---\n\n# Custom\n\nBody.\n"
|
||||
)
|
||||
|
||||
result = install_ai_skills(proj, agent_key)
|
||||
|
||||
# Should succeed via fallback to templates/commands/
|
||||
assert result is True
|
||||
skills_dir = _get_skills_dir(proj, agent_key)
|
||||
assert skills_dir.exists()
|
||||
skill_dirs = [d.name for d in skills_dir.iterdir() if d.is_dir()]
|
||||
assert not any("my-custom" in d for d in skill_dirs)
|
||||
|
||||
class TestCommandCoexistence:
|
||||
"""Verify install_ai_skills never touches command files.
|
||||
@@ -460,14 +556,16 @@ class TestCommandCoexistence:
|
||||
|
||||
def test_existing_commands_preserved_claude(self, project_dir, templates_dir, commands_dir_claude):
|
||||
"""install_ai_skills must NOT remove pre-existing .claude/commands files."""
|
||||
# Verify commands exist before
|
||||
assert len(list(commands_dir_claude.glob("speckit.*"))) == 3
|
||||
# Verify commands exist before (templates_dir adds 4 speckit.* files,
|
||||
# commands_dir_claude overlaps with 3 of them)
|
||||
before = list(commands_dir_claude.glob("speckit.*"))
|
||||
assert len(before) >= 3
|
||||
|
||||
install_ai_skills(project_dir, "claude")
|
||||
|
||||
# Commands must still be there — install_ai_skills never touches them
|
||||
remaining = list(commands_dir_claude.glob("speckit.*"))
|
||||
assert len(remaining) == 3
|
||||
assert len(remaining) == len(before)
|
||||
|
||||
def test_existing_commands_preserved_gemini(self, project_dir, templates_dir, commands_dir_gemini):
|
||||
"""install_ai_skills must NOT remove pre-existing .gemini/commands files."""
|
||||
|
||||
Reference in New Issue
Block a user