From 1930f89d179f99dc95e7a36318ef08470482e6f8 Mon Sep 17 00:00:00 2001 From: Zhiyao Wen <75983327+ZhiyaoWen999@users.noreply.github.com> Date: Tue, 7 Jul 2026 23:51:50 +0800 Subject: [PATCH] fix extension-local script path rewriting (#3364) Co-authored-by: Zhiyao --- src/specify_cli/agents.py | 59 ++++++++++++++++++++----- src/specify_cli/extensions/__init__.py | 8 +++- tests/test_extension_skills.py | 61 ++++++++++++++++++++++++++ tests/test_extensions.py | 44 +++++++++++++++++++ 4 files changed, 158 insertions(+), 14 deletions(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 7864260a9..8610b4231 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -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//scripts/..." remain intact. + # Only rewrite top-level style references so existing generated paths + # like ".specify/extensions//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 diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index 5eea80c77..b84b83654 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -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( diff --git a/tests/test_extension_skills.py b/tests/test_extension_skills.py index 1cdf126a0..c2d9bb439 100644 --- a/tests/test_extension_skills.py +++ b/tests/test_extension_skills.py @@ -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 diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 2a3d02987..9dae5fefd 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -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