mirror of
https://github.com/github/spec-kit.git
synced 2026-07-08 07:04:44 +08:00
fix extension-local script path rewriting (#3364)
Co-authored-by: Zhiyao <zhiyao@ZhiyaodeMacBook-Air.local>
This commit is contained in:
@@ -148,7 +148,9 @@ class CommandRegistrar:
|
||||
)
|
||||
return f"---\n{yaml_str}---\n"
|
||||
|
||||
def _adjust_script_paths(self, frontmatter: dict) -> dict:
|
||||
def _adjust_script_paths(
|
||||
self, frontmatter: dict, extension_id: Optional[str] = None
|
||||
) -> dict:
|
||||
"""Normalize script paths in frontmatter to generated project locations.
|
||||
|
||||
Rewrites known repo-relative and top-level script paths under the
|
||||
@@ -158,6 +160,7 @@ class CommandRegistrar:
|
||||
|
||||
Args:
|
||||
frontmatter: Frontmatter dictionary
|
||||
extension_id: Extension id when rendering extension-owned commands.
|
||||
|
||||
Returns:
|
||||
Modified frontmatter with normalized project paths
|
||||
@@ -168,11 +171,15 @@ class CommandRegistrar:
|
||||
if isinstance(scripts, dict):
|
||||
for key, script_path in scripts.items():
|
||||
if isinstance(script_path, str):
|
||||
scripts[key] = self.rewrite_project_relative_paths(script_path)
|
||||
scripts[key] = self.rewrite_project_relative_paths(
|
||||
script_path, extension_id=extension_id
|
||||
)
|
||||
return frontmatter
|
||||
|
||||
@staticmethod
|
||||
def rewrite_project_relative_paths(text: str) -> str:
|
||||
def rewrite_project_relative_paths(
|
||||
text: str, extension_id: Optional[str] = None
|
||||
) -> str:
|
||||
"""Rewrite repo-relative paths to their generated project locations."""
|
||||
if not isinstance(text, str) or not text:
|
||||
return text
|
||||
@@ -184,10 +191,18 @@ class CommandRegistrar:
|
||||
):
|
||||
text = text.replace(old, new)
|
||||
|
||||
# Only rewrite top-level style references so extension-local paths like
|
||||
# ".specify/extensions/<ext>/scripts/..." remain intact.
|
||||
# Only rewrite top-level style references so existing generated paths
|
||||
# like ".specify/extensions/<ext>/scripts/..." remain intact. When
|
||||
# rendering extension commands, top-level "scripts/" is extension-local.
|
||||
scripts_replacement = (
|
||||
f".specify/extensions/{extension_id}/scripts/"
|
||||
if extension_id
|
||||
else ".specify/scripts/"
|
||||
)
|
||||
text = re.sub(r'(^|[\s`"\'(])(?:\.?/)?memory/', r"\1.specify/memory/", text)
|
||||
text = re.sub(r'(^|[\s`"\'(])(?:\.?/)?scripts/', r"\1.specify/scripts/", text)
|
||||
text = re.sub(
|
||||
r'(^|[\s`"\'(])(?:\.?/)?scripts/', rf"\1{scripts_replacement}", text
|
||||
)
|
||||
text = re.sub(
|
||||
r'(^|[\s`"\'(])(?:\.?/)?templates/', r"\1.specify/templates/", text
|
||||
)
|
||||
@@ -312,6 +327,7 @@ class CommandRegistrar:
|
||||
source_id: str,
|
||||
source_file: str,
|
||||
project_root: Path,
|
||||
extension_id: Optional[str] = None,
|
||||
) -> str:
|
||||
"""Render a command override as a SKILL.md file.
|
||||
|
||||
@@ -331,7 +347,7 @@ class CommandRegistrar:
|
||||
agent_config = self.AGENT_CONFIGS.get(agent_name, {})
|
||||
if agent_config.get("extension") == "/SKILL.md":
|
||||
body = self.resolve_skill_placeholders(
|
||||
agent_name, frontmatter, body, project_root
|
||||
agent_name, frontmatter, body, project_root, extension_id=extension_id
|
||||
)
|
||||
|
||||
description = frontmatter.get(
|
||||
@@ -393,7 +409,11 @@ class CommandRegistrar:
|
||||
|
||||
@staticmethod
|
||||
def resolve_skill_placeholders(
|
||||
agent_name: str, frontmatter: dict, body: str, project_root: Path
|
||||
agent_name: str,
|
||||
frontmatter: dict,
|
||||
body: str,
|
||||
project_root: Path,
|
||||
extension_id: Optional[str] = None,
|
||||
) -> str:
|
||||
"""Resolve script placeholders for skills-backed agents."""
|
||||
if not isinstance(frontmatter, dict):
|
||||
@@ -433,7 +453,9 @@ class CommandRegistrar:
|
||||
|
||||
body = body.replace("{ARGS}", "$ARGUMENTS").replace("__AGENT__", agent_name)
|
||||
|
||||
return CommandRegistrar.rewrite_project_relative_paths(body)
|
||||
return CommandRegistrar.rewrite_project_relative_paths(
|
||||
body, extension_id=extension_id
|
||||
)
|
||||
|
||||
def _convert_argument_placeholder(
|
||||
self, content: str, from_placeholder: str, to_placeholder: str
|
||||
@@ -528,6 +550,7 @@ class CommandRegistrar:
|
||||
context_note: str = None,
|
||||
_resolved_dir: Path = None,
|
||||
link_outputs: bool = False,
|
||||
extension_id: Optional[str] = None,
|
||||
) -> List[str]:
|
||||
"""Register commands for a specific agent.
|
||||
|
||||
@@ -545,6 +568,7 @@ class CommandRegistrar:
|
||||
link_outputs: If True, write rendered output to a source-local
|
||||
dev cache and symlink the agent command file to it. Falls back
|
||||
to a normal file write when symlinks are unavailable.
|
||||
extension_id: Extension id when rendering extension-owned commands.
|
||||
|
||||
Returns:
|
||||
List of registered command names
|
||||
@@ -614,7 +638,9 @@ class CommandRegistrar:
|
||||
frontmatter[key] = core_frontmatter[key]
|
||||
frontmatter.pop("strategy", None)
|
||||
|
||||
frontmatter = self._adjust_script_paths(frontmatter)
|
||||
frontmatter = self._adjust_script_paths(
|
||||
frontmatter, extension_id=extension_id
|
||||
)
|
||||
|
||||
for key in agent_config.get("strip_frontmatter_keys", []):
|
||||
frontmatter.pop(key, None)
|
||||
@@ -653,10 +679,11 @@ class CommandRegistrar:
|
||||
source_id,
|
||||
cmd_file,
|
||||
project_root,
|
||||
extension_id=extension_id,
|
||||
)
|
||||
elif agent_config["format"] == "markdown":
|
||||
body = self.resolve_skill_placeholders(
|
||||
agent_name, frontmatter, body, project_root
|
||||
agent_name, frontmatter, body, project_root, extension_id=extension_id
|
||||
)
|
||||
body = self._convert_argument_placeholder(
|
||||
body, "$ARGUMENTS", agent_config["args"]
|
||||
@@ -666,7 +693,7 @@ class CommandRegistrar:
|
||||
)
|
||||
elif agent_config["format"] == "toml":
|
||||
body = self.resolve_skill_placeholders(
|
||||
agent_name, frontmatter, body, project_root
|
||||
agent_name, frontmatter, body, project_root, extension_id=extension_id
|
||||
)
|
||||
body = self._convert_argument_placeholder(
|
||||
body, "$ARGUMENTS", agent_config["args"]
|
||||
@@ -721,6 +748,7 @@ class CommandRegistrar:
|
||||
source_id,
|
||||
cmd_file,
|
||||
project_root,
|
||||
extension_id=extension_id,
|
||||
)
|
||||
elif agent_config["format"] == "markdown":
|
||||
alias_output = self.render_markdown_command(
|
||||
@@ -750,6 +778,7 @@ class CommandRegistrar:
|
||||
source_id,
|
||||
cmd_file,
|
||||
project_root,
|
||||
extension_id=extension_id,
|
||||
)
|
||||
|
||||
alias_file = (
|
||||
@@ -881,6 +910,7 @@ class CommandRegistrar:
|
||||
context_note: str = None,
|
||||
link_outputs: bool = False,
|
||||
create_missing_active_skills_dir: bool = False,
|
||||
extension_id: Optional[str] = None,
|
||||
) -> Dict[str, List[str]]:
|
||||
"""Register commands for all detected agents in the project.
|
||||
|
||||
@@ -897,6 +927,7 @@ class CommandRegistrar:
|
||||
Recovery requires active skills mode (or Kimi's existing native
|
||||
skills directory) and is skipped when safe resolution or
|
||||
creation fails.
|
||||
extension_id: Extension id when rendering extension-owned commands.
|
||||
|
||||
Returns:
|
||||
Dictionary mapping agent names to list of registered commands
|
||||
@@ -999,6 +1030,7 @@ class CommandRegistrar:
|
||||
context_note=context_note,
|
||||
_resolved_dir=agent_dir,
|
||||
link_outputs=link_outputs,
|
||||
extension_id=extension_id,
|
||||
)
|
||||
if registered:
|
||||
results[agent_name] = registered
|
||||
@@ -1023,6 +1055,7 @@ class CommandRegistrar:
|
||||
project_root: Path,
|
||||
context_note: Optional[str] = None,
|
||||
link_outputs: bool = False,
|
||||
extension_id: Optional[str] = None,
|
||||
) -> Dict[str, List[str]]:
|
||||
"""Register commands for all non-skill agents in the project.
|
||||
|
||||
@@ -1038,6 +1071,7 @@ class CommandRegistrar:
|
||||
context_note: Custom context comment for markdown output
|
||||
link_outputs: If True, create dev-mode symlinks for rendered
|
||||
command files when supported by the OS.
|
||||
extension_id: Extension id when rendering extension-owned commands.
|
||||
|
||||
Returns:
|
||||
Dictionary mapping agent names to list of registered commands
|
||||
@@ -1066,6 +1100,7 @@ class CommandRegistrar:
|
||||
context_note=context_note,
|
||||
_resolved_dir=agent_dir,
|
||||
link_outputs=link_outputs,
|
||||
extension_id=extension_id,
|
||||
)
|
||||
if registered:
|
||||
results[agent_name] = registered
|
||||
|
||||
@@ -1075,9 +1075,11 @@ class ExtensionManager:
|
||||
pass # best-effort cleanup
|
||||
continue
|
||||
frontmatter, body = registrar.parse_frontmatter(content)
|
||||
frontmatter = registrar._adjust_script_paths(frontmatter)
|
||||
frontmatter = registrar._adjust_script_paths(
|
||||
frontmatter, extension_id=manifest.id
|
||||
)
|
||||
body = registrar.resolve_skill_placeholders(
|
||||
selected_ai, frontmatter, body, self.project_root
|
||||
selected_ai, frontmatter, body, self.project_root, extension_id=manifest.id
|
||||
)
|
||||
|
||||
original_desc = frontmatter.get("description", "")
|
||||
@@ -1958,6 +1960,7 @@ class CommandRegistrar:
|
||||
project_root,
|
||||
context_note=context_note,
|
||||
link_outputs=link_outputs,
|
||||
extension_id=manifest.id,
|
||||
)
|
||||
|
||||
def register_commands_for_all_agents(
|
||||
@@ -1978,6 +1981,7 @@ class CommandRegistrar:
|
||||
context_note=context_note,
|
||||
link_outputs=link_outputs,
|
||||
create_missing_active_skills_dir=create_missing_active_skills_dir,
|
||||
extension_id=manifest.id,
|
||||
)
|
||||
|
||||
def unregister_commands(
|
||||
|
||||
@@ -850,6 +850,67 @@ class TestExtensionSkillRegistration:
|
||||
assert ".specify/templates/checklist.md" in content
|
||||
assert ".specify/memory/constitution.md" in content
|
||||
|
||||
def test_skill_registration_uses_extension_local_script_paths(self, project_dir, temp_dir):
|
||||
"""Auto-registered skills should not rewrite extension scripts into core scripts."""
|
||||
_create_init_options(project_dir, ai="claude", ai_skills=True)
|
||||
skills_dir = _create_skills_dir(project_dir, ai="claude")
|
||||
|
||||
ext_dir = temp_dir / "scripted-ext"
|
||||
ext_dir.mkdir()
|
||||
manifest_data = {
|
||||
"schema_version": "1.0",
|
||||
"extension": {
|
||||
"id": "scripted-ext",
|
||||
"name": "Scripted Extension",
|
||||
"version": "1.0.0",
|
||||
"description": "Test",
|
||||
},
|
||||
"requires": {"speckit_version": ">=0.1.0"},
|
||||
"provides": {
|
||||
"commands": [
|
||||
{
|
||||
"name": "speckit.scripted-ext.check",
|
||||
"file": "commands/check.md",
|
||||
"description": "Scripted check command",
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
with open(ext_dir / "extension.yml", "w") as f:
|
||||
yaml.safe_dump(manifest_data, f)
|
||||
|
||||
(ext_dir / "commands").mkdir()
|
||||
(ext_dir / "scripts" / "bash").mkdir(parents=True)
|
||||
(ext_dir / "scripts" / "bash" / "resolve-skill.sh").write_text(
|
||||
"#!/usr/bin/env bash\n"
|
||||
)
|
||||
(ext_dir / "scripts" / "bash" / "ensure-skills.sh").write_text(
|
||||
"#!/usr/bin/env bash\n"
|
||||
)
|
||||
(ext_dir / "commands" / "check.md").write_text(
|
||||
"---\n"
|
||||
"description: Scripted check command\n"
|
||||
"scripts:\n"
|
||||
' sh: scripts/bash/resolve-skill.sh "{ARGS}"\n'
|
||||
"---\n\n"
|
||||
"Run {SCRIPT}\n"
|
||||
"Then run scripts/bash/ensure-skills.sh.\n"
|
||||
)
|
||||
|
||||
manager = ExtensionManager(project_dir)
|
||||
manager.install_from_directory(ext_dir, "0.1.0", register_commands=False)
|
||||
|
||||
content = (skills_dir / "speckit-scripted-ext-check" / "SKILL.md").read_text()
|
||||
assert "{SCRIPT}" not in content
|
||||
assert "{ARGS}" not in content
|
||||
assert (
|
||||
'.specify/extensions/scripted-ext/scripts/bash/resolve-skill.sh "$ARGUMENTS"'
|
||||
in content
|
||||
)
|
||||
assert ".specify/extensions/scripted-ext/scripts/bash/ensure-skills.sh" in content
|
||||
assert ".specify/scripts/bash/resolve-skill.sh" not in content
|
||||
assert ".specify/scripts/bash/ensure-skills.sh" not in content
|
||||
|
||||
def test_missing_command_file_skipped(self, skills_project, temp_dir):
|
||||
"""Commands with missing source files should be skipped gracefully."""
|
||||
project_dir, skills_dir = skills_project
|
||||
|
||||
@@ -1147,10 +1147,12 @@ class TestExtensionManager:
|
||||
context_note=None,
|
||||
link_outputs=False,
|
||||
create_missing_active_skills_dir=False,
|
||||
extension_id=None,
|
||||
):
|
||||
captured["create_missing_active_skills_dir"] = (
|
||||
create_missing_active_skills_dir
|
||||
)
|
||||
captured["extension_id"] = extension_id
|
||||
return {}
|
||||
|
||||
monkeypatch.setattr(
|
||||
@@ -1164,6 +1166,7 @@ class TestExtensionManager:
|
||||
registrar.register_commands_for_all_agents(manifest, extension_dir, project_dir)
|
||||
|
||||
assert captured["create_missing_active_skills_dir"] is False
|
||||
assert captured["extension_id"] == manifest.id
|
||||
|
||||
def test_install_duplicate(self, extension_dir, project_dir):
|
||||
"""Test installing already installed extension."""
|
||||
@@ -1695,6 +1698,29 @@ $ARGUMENTS
|
||||
assert adjusted["scripts"]["sh"] == ".specify/extensions/test-ext/scripts/setup.sh {ARGS}"
|
||||
assert adjusted["scripts"]["ps"] == ".specify/scripts/powershell/setup-plan.ps1 {ARGS}"
|
||||
|
||||
def test_adjust_script_paths_rewrites_extension_top_level_scripts(self):
|
||||
"""Extension command-local scripts should resolve under the installed extension."""
|
||||
from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar
|
||||
|
||||
registrar = AgentCommandRegistrar()
|
||||
original = {
|
||||
"scripts": {
|
||||
"sh": "scripts/bash/resolve-skill.sh {ARGS}",
|
||||
"ps": "../../scripts/powershell/setup-plan.ps1 -Json",
|
||||
}
|
||||
}
|
||||
|
||||
adjusted = registrar._adjust_script_paths(original, extension_id="test-ext")
|
||||
|
||||
assert (
|
||||
adjusted["scripts"]["sh"]
|
||||
== ".specify/extensions/test-ext/scripts/bash/resolve-skill.sh {ARGS}"
|
||||
)
|
||||
assert (
|
||||
adjusted["scripts"]["ps"]
|
||||
== ".specify/scripts/powershell/setup-plan.ps1 -Json"
|
||||
)
|
||||
|
||||
def test_rewrite_project_relative_paths_preserves_extension_local_body_paths(self):
|
||||
"""Body rewrites should preserve extension-local assets while fixing top-level refs."""
|
||||
from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar
|
||||
@@ -1709,6 +1735,24 @@ $ARGUMENTS
|
||||
assert ".specify/extensions/test-ext/templates/spec.md" in rewritten
|
||||
assert ".specify/scripts/bash/setup-plan.sh" in rewritten
|
||||
|
||||
def test_rewrite_project_relative_paths_uses_extension_context_for_scripts(self):
|
||||
"""Extension source bodies treat top-level scripts/ as extension-local."""
|
||||
from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar
|
||||
|
||||
body = (
|
||||
"Run scripts/bash/ensure-skills.sh\n"
|
||||
"Fallback ../../scripts/bash/setup-plan.sh\n"
|
||||
"Read templates/checklist.md\n"
|
||||
)
|
||||
|
||||
rewritten = AgentCommandRegistrar.rewrite_project_relative_paths(
|
||||
body, extension_id="test-ext"
|
||||
)
|
||||
|
||||
assert ".specify/extensions/test-ext/scripts/bash/ensure-skills.sh" in rewritten
|
||||
assert ".specify/scripts/bash/setup-plan.sh" in rewritten
|
||||
assert ".specify/templates/checklist.md" in rewritten
|
||||
|
||||
def test_render_toml_command_handles_embedded_triple_double_quotes(self):
|
||||
"""TOML renderer should stay valid when body includes triple double-quotes."""
|
||||
from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar
|
||||
|
||||
Reference in New Issue
Block a user