Address dev symlink review feedback

Author: NgoQuocViet2001

src/specify_cli/agents.py

@@ -668,13 +668,9 @@ def _write_registered_output(
             return
 
         rel_output = Path(f"{output_name}{extension}")
-        cache_file = (
-            source_dir
-            / ".specify-dev"
-            / "agent-commands"
-            / agent_name
-            / rel_output
-        )
+        cache_root = source_dir / ".specify-dev" / "agent-commands" / agent_name
+        cache_file = cache_root / rel_output
+        CommandRegistrar._ensure_inside(cache_file, cache_root)
         cache_file.parent.mkdir(parents=True, exist_ok=True)
         cache_file.write_text(content, encoding="utf-8")
 

src/specify_cli/extensions.py

@@ -906,9 +906,18 @@ def _register_extension_skills(
             # Check if skill already exists before creating the directory
             skill_subdir = skills_dir / skill_name
             skill_file = skill_subdir / "SKILL.md"
-            if skill_file.exists():
-                # Do not overwrite user-customized skills
-                continue
+            cache_root = extension_dir / ".specify-dev" / "extension-skills"
+            cache_file = cache_root / skill_name / "SKILL.md"
+            CommandRegistrar._ensure_inside(cache_file, cache_root)
+            if skill_file.exists() or skill_file.is_symlink():
+                # Do not overwrite user-customized skills, but allow dev-mode
+                # symlinks that point back to this extension's generated cache
+                # to be refreshed on a subsequent dev install.
+                if not (
+                    link_outputs
+                    and self._is_expected_dev_symlink(skill_file, cache_file)
+                ):
+                    continue
 
             # Create skill directory; track whether we created it so we can clean
             # up safely if reading the source file subsequently fails.
@@ -961,13 +970,6 @@ def _register_extension_skills(
                 )
 
             if link_outputs:
-                cache_file = (
-                    extension_dir
-                    / ".specify-dev"
-                    / "extension-skills"
-                    / skill_name
-                    / "SKILL.md"
-                )
                 cache_file.parent.mkdir(parents=True, exist_ok=True)
                 cache_file.write_text(skill_content, encoding="utf-8")
                 try:
@@ -985,6 +987,17 @@ def _register_extension_skills(
 
         return written
 
+    @staticmethod
+    def _is_expected_dev_symlink(skill_file: Path, cache_file: Path) -> bool:
+        """Return True when an existing skill file links to its dev cache."""
+        if not skill_file.is_symlink():
+            return False
+
+        try:
+            return skill_file.resolve(strict=False) == cache_file.resolve(strict=False)
+        except OSError:
+            return False
+
     def _unregister_extension_skills(
         self,
         skill_names: List[str],

tests/test_extension_skills.py

@@ -11,6 +11,7 @@
 """
 
 import json
+import os
 import pytest
 import tempfile
 import shutil
@@ -116,6 +117,18 @@ def _create_extension_dir(temp_dir: Path, ext_id: str = "test-ext") -> Path:
     return ext_dir
 
 
+def _can_create_symlink(temp_dir: Path) -> bool:
+    """Return True when the current platform/user can create file symlinks."""
+    target = temp_dir / "symlink-target.txt"
+    link = temp_dir / "symlink-link.txt"
+    target.write_text("ok", encoding="utf-8")
+    try:
+        os.symlink(target, link)
+    except OSError:
+        return False
+    return link.is_symlink()
+
+
 # ===== Fixtures =====
 
 @pytest.fixture
@@ -316,6 +329,46 @@ def test_existing_skill_not_overwritten(self, skills_project, extension_dir):
         # The pre-existing one should NOT be in registered_skills (it was skipped)
         assert "speckit-test-ext-hello" not in metadata["registered_skills"]
 
+    def test_dev_skill_symlink_refreshes_existing_cache(
+        self, skills_project, extension_dir, temp_dir
+    ):
+        """Dev-mode skill symlinks should refresh rendered cache content."""
+        if not _can_create_symlink(temp_dir):
+            pytest.skip("Current platform/user cannot create symlinks")
+
+        project_dir, skills_dir = skills_project
+        manager = ExtensionManager(project_dir)
+        manifest = ExtensionManifest(extension_dir / "extension.yml")
+
+        manager._register_extension_skills(
+            manifest,
+            extension_dir,
+            link_outputs=True,
+        )
+
+        skill_file = skills_dir / "speckit-test-ext-hello" / "SKILL.md"
+        assert skill_file.is_symlink()
+        assert "Run this to say hello." in skill_file.read_text(encoding="utf-8")
+
+        (extension_dir / "commands" / "hello.md").write_text(
+            "---\n"
+            "description: \"Updated test hello command\"\n"
+            "---\n"
+            "\n"
+            "# Hello Command\n"
+            "\n"
+            "Run this updated hello.\n"
+        )
+
+        written = manager._register_extension_skills(
+            manifest,
+            extension_dir,
+            link_outputs=True,
+        )
+
+        assert "speckit-test-ext-hello" in written
+        assert "Run this updated hello." in skill_file.read_text(encoding="utf-8")
+
     def test_registered_skills_in_registry(self, skills_project, extension_dir):
         """Registry should contain registered_skills list."""
         project_dir, skills_dir = skills_project

tests/test_extensions.py

@@ -1799,6 +1799,33 @@ def raise_symlink_error(target, link):
             / "speckit.test-ext.hello.agent.md"
         ).exists()
 
+    def test_dev_register_commands_rejects_cache_path_traversal(self, temp_dir):
+        """Dev-mode cache writes must stay inside the agent cache root."""
+        from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar
+
+        source_dir = temp_dir / "extension"
+        source_dir.mkdir()
+        commands_dir = temp_dir / "commands"
+        commands_dir.mkdir()
+
+        with pytest.raises(ValueError, match="escapes directory"):
+            AgentCommandRegistrar._write_registered_output(
+                commands_dir / "safe.md",
+                "content",
+                source_dir,
+                "copilot",
+                "../escaped",
+                ".md",
+                True,
+            )
+
+        assert not (
+            source_dir
+            / ".specify-dev"
+            / "agent-commands"
+            / "escaped.md"
+        ).exists()
+
     def test_copilot_companion_prompt_created(self, extension_dir, project_dir):
         """Test that companion .prompt.md files are created in .github/prompts/."""
         agents_dir = project_dir / ".github" / "agents"