mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
fix: write Codex dev skills as files (#2988)
* fix: write Codex dev skills as files * fix: route codex dev symlink policy through metadata * fix: replace codex dev symlinks on refresh * fix: migrate codex dev skill symlinks * fix: avoid inactive shared skill dev symlinks * fix: preserve unrelated dev skill symlinks
This commit is contained in:
@@ -37,6 +37,8 @@ def _build_agent_configs() -> dict[str, Any]:
|
|||||||
# when register_commands() resolves __SPECKIT_COMMAND_*__ tokens.
|
# when register_commands() resolves __SPECKIT_COMMAND_*__ tokens.
|
||||||
if "invoke_separator" not in config:
|
if "invoke_separator" not in config:
|
||||||
config["invoke_separator"] = integration.invoke_separator
|
config["invoke_separator"] = integration.invoke_separator
|
||||||
|
if integration.dev_no_symlink:
|
||||||
|
config["dev_no_symlink"] = True
|
||||||
configs[key] = config
|
configs[key] = config
|
||||||
return configs
|
return configs
|
||||||
|
|
||||||
@@ -714,6 +716,7 @@ class CommandRegistrar:
|
|||||||
output_name,
|
output_name,
|
||||||
agent_config["extension"],
|
agent_config["extension"],
|
||||||
link_outputs,
|
link_outputs,
|
||||||
|
agent_config,
|
||||||
)
|
)
|
||||||
|
|
||||||
if agent_name == "copilot":
|
if agent_name == "copilot":
|
||||||
@@ -788,6 +791,7 @@ class CommandRegistrar:
|
|||||||
alias_output_name,
|
alias_output_name,
|
||||||
agent_config["extension"],
|
agent_config["extension"],
|
||||||
link_outputs,
|
link_outputs,
|
||||||
|
agent_config,
|
||||||
)
|
)
|
||||||
if agent_name == "copilot":
|
if agent_name == "copilot":
|
||||||
self.write_copilot_prompt(project_root, alias)
|
self.write_copilot_prompt(project_root, alias)
|
||||||
@@ -804,9 +808,12 @@ class CommandRegistrar:
|
|||||||
output_name: str,
|
output_name: str,
|
||||||
extension: str,
|
extension: str,
|
||||||
link_outputs: bool,
|
link_outputs: bool,
|
||||||
|
agent_config: dict[str, Any] | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Write a rendered agent artifact, optionally as a dev-mode symlink."""
|
"""Write a rendered agent artifact, optionally as a dev-mode symlink."""
|
||||||
if not link_outputs:
|
if not link_outputs or (agent_config or {}).get("dev_no_symlink"):
|
||||||
|
if dest_file.is_symlink():
|
||||||
|
dest_file.unlink()
|
||||||
dest_file.write_text(content, encoding="utf-8")
|
dest_file.write_text(content, encoding="utf-8")
|
||||||
return
|
return
|
||||||
|
|
||||||
@@ -927,6 +934,16 @@ class CommandRegistrar:
|
|||||||
self._active_skills_agent(project_root)
|
self._active_skills_agent(project_root)
|
||||||
if create_missing_active_skills_dir else None
|
if create_missing_active_skills_dir else None
|
||||||
)
|
)
|
||||||
|
active_skills_dir: Optional[Path] = None
|
||||||
|
if active_skills_agent:
|
||||||
|
active_skills_config = self.AGENT_CONFIGS.get(active_skills_agent)
|
||||||
|
if (
|
||||||
|
active_skills_config
|
||||||
|
and active_skills_config.get("extension") == "/SKILL.md"
|
||||||
|
):
|
||||||
|
active_skills_dir = self._resolve_agent_dir(
|
||||||
|
active_skills_agent, active_skills_config, project_root,
|
||||||
|
)
|
||||||
active_created_skills_dir: Optional[Path] = None
|
active_created_skills_dir: Optional[Path] = None
|
||||||
for agent_name, agent_config in self.AGENT_CONFIGS.items():
|
for agent_name, agent_config in self.AGENT_CONFIGS.items():
|
||||||
active_skills_output = (
|
active_skills_output = (
|
||||||
@@ -958,6 +975,14 @@ class CommandRegistrar:
|
|||||||
agent_dir = self._resolve_agent_dir(
|
agent_dir = self._resolve_agent_dir(
|
||||||
agent_name, agent_config, project_root,
|
agent_name, agent_config, project_root,
|
||||||
)
|
)
|
||||||
|
shares_active_skills_dir = (
|
||||||
|
active_skills_dir is not None
|
||||||
|
and agent_name != active_skills_agent
|
||||||
|
and agent_config.get("extension") == "/SKILL.md"
|
||||||
|
and self._same_lexical_path(agent_dir, active_skills_dir)
|
||||||
|
)
|
||||||
|
if shares_active_skills_dir:
|
||||||
|
continue
|
||||||
|
|
||||||
agent_dir_existed = agent_dir.is_dir()
|
agent_dir_existed = agent_dir.is_dir()
|
||||||
register_missing_active_skills_agent = (
|
register_missing_active_skills_agent = (
|
||||||
|
|||||||
@@ -997,6 +997,7 @@ class ExtensionManager:
|
|||||||
if not isinstance(selected_ai, str) or not selected_ai:
|
if not isinstance(selected_ai, str) or not selected_ai:
|
||||||
return []
|
return []
|
||||||
registrar = CommandRegistrar()
|
registrar = CommandRegistrar()
|
||||||
|
agent_config = registrar.AGENT_CONFIGS.get(selected_ai, {})
|
||||||
integration = get_integration(selected_ai)
|
integration = get_integration(selected_ai)
|
||||||
|
|
||||||
for cmd_info in manifest.commands:
|
for cmd_info in manifest.commands:
|
||||||
@@ -1030,15 +1031,16 @@ class ExtensionManager:
|
|||||||
skill_file = skill_subdir / "SKILL.md"
|
skill_file = skill_subdir / "SKILL.md"
|
||||||
cache_root = extension_dir / ".specify-dev" / "extension-skills"
|
cache_root = extension_dir / ".specify-dev" / "extension-skills"
|
||||||
cache_file = cache_root / skill_name / "SKILL.md"
|
cache_file = cache_root / skill_name / "SKILL.md"
|
||||||
|
use_dev_symlink = link_outputs and not agent_config.get("dev_no_symlink")
|
||||||
CommandRegistrar._ensure_inside(cache_file, cache_root)
|
CommandRegistrar._ensure_inside(cache_file, cache_root)
|
||||||
if skill_file.exists() or skill_file.is_symlink():
|
if skill_file.exists() or skill_file.is_symlink():
|
||||||
|
is_expected_dev_symlink = self._is_expected_dev_symlink(
|
||||||
|
skill_file, cache_file
|
||||||
|
)
|
||||||
# Do not overwrite user-customized skills, but allow dev-mode
|
# Do not overwrite user-customized skills, but allow dev-mode
|
||||||
# symlinks that point back to this extension's generated cache
|
# symlinks that point back to this extension's generated cache
|
||||||
# to be refreshed on a subsequent dev install.
|
# to be refreshed on a subsequent dev install.
|
||||||
if not (
|
if not is_expected_dev_symlink:
|
||||||
link_outputs
|
|
||||||
and self._is_expected_dev_symlink(skill_file, cache_file)
|
|
||||||
):
|
|
||||||
continue
|
continue
|
||||||
|
|
||||||
# Create skill directory; track whether we created it so we can clean
|
# Create skill directory; track whether we created it so we can clean
|
||||||
@@ -1093,7 +1095,7 @@ class ExtensionManager:
|
|||||||
):
|
):
|
||||||
skill_content = integration.post_process_skill_content(skill_content)
|
skill_content = integration.post_process_skill_content(skill_content)
|
||||||
|
|
||||||
if link_outputs:
|
if use_dev_symlink:
|
||||||
try:
|
try:
|
||||||
cache_file.parent.mkdir(parents=True, exist_ok=True)
|
cache_file.parent.mkdir(parents=True, exist_ok=True)
|
||||||
cache_file.write_text(skill_content, encoding="utf-8")
|
cache_file.write_text(skill_content, encoding="utf-8")
|
||||||
@@ -1106,6 +1108,8 @@ class ExtensionManager:
|
|||||||
skill_file.unlink()
|
skill_file.unlink()
|
||||||
skill_file.write_text(skill_content, encoding="utf-8")
|
skill_file.write_text(skill_content, encoding="utf-8")
|
||||||
else:
|
else:
|
||||||
|
if skill_file.is_symlink():
|
||||||
|
skill_file.unlink()
|
||||||
skill_file.write_text(skill_content, encoding="utf-8")
|
skill_file.write_text(skill_content, encoding="utf-8")
|
||||||
written.append(skill_name)
|
written.append(skill_name)
|
||||||
|
|
||||||
|
|||||||
@@ -119,6 +119,9 @@ class IntegrationBase(ABC):
|
|||||||
invoke_separator: str = "."
|
invoke_separator: str = "."
|
||||||
"""Separator used in slash-command invocations (``"."`` → ``/speckit.plan``)."""
|
"""Separator used in slash-command invocations (``"."`` → ``/speckit.plan``)."""
|
||||||
|
|
||||||
|
dev_no_symlink: bool = False
|
||||||
|
"""Whether dev-mode registration should write files instead of symlinks."""
|
||||||
|
|
||||||
multi_install_safe: bool = False
|
multi_install_safe: bool = False
|
||||||
"""Whether this integration is declared safe to install alongside others.
|
"""Whether this integration is declared safe to install alongside others.
|
||||||
|
|
||||||
|
|||||||
@@ -27,6 +27,7 @@ class CodexIntegration(SkillsIntegration):
|
|||||||
"extension": "/SKILL.md",
|
"extension": "/SKILL.md",
|
||||||
}
|
}
|
||||||
context_file = "AGENTS.md"
|
context_file = "AGENTS.md"
|
||||||
|
dev_no_symlink = True
|
||||||
multi_install_safe = True
|
multi_install_safe = True
|
||||||
|
|
||||||
def build_exec_args(
|
def build_exec_args(
|
||||||
|
|||||||
@@ -360,6 +360,12 @@ class TestAgentConfigConsistency:
|
|||||||
"expected '-' (propagated from SkillsIntegration.invoke_separator)"
|
"expected '-' (propagated from SkillsIntegration.invoke_separator)"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_codex_dev_no_symlink_policy_in_agent_config(self):
|
||||||
|
"""Codex dev installs must expose the no-symlink policy as metadata."""
|
||||||
|
cfg = CommandRegistrar.AGENT_CONFIGS
|
||||||
|
|
||||||
|
assert cfg["codex"].get("dev_no_symlink") is True
|
||||||
|
|
||||||
def test_skills_agent_command_token_resolves_with_hyphen(self, tmp_path):
|
def test_skills_agent_command_token_resolves_with_hyphen(self, tmp_path):
|
||||||
"""__SPECKIT_COMMAND_*__ tokens in extension commands resolve to /speckit-<cmd>
|
"""__SPECKIT_COMMAND_*__ tokens in extension commands resolve to /speckit-<cmd>
|
||||||
when registered for a skills-based agent (e.g. claude).
|
when registered for a skills-based agent (e.g. claude).
|
||||||
|
|||||||
@@ -573,6 +573,84 @@ class TestExtensionSkillRegistration:
|
|||||||
assert "speckit-test-ext-hello" in written
|
assert "speckit-test-ext-hello" in written
|
||||||
assert "Run this updated hello." in skill_file.read_text(encoding="utf-8")
|
assert "Run this updated hello." in skill_file.read_text(encoding="utf-8")
|
||||||
|
|
||||||
|
def test_codex_dev_skill_registration_replaces_existing_dev_symlink(
|
||||||
|
self, project_dir, extension_dir, temp_dir
|
||||||
|
):
|
||||||
|
"""Codex dev skill registration should migrate prior dev symlinks to files."""
|
||||||
|
if not _can_create_symlink(temp_dir):
|
||||||
|
pytest.skip("Current platform/user cannot create symlinks")
|
||||||
|
|
||||||
|
_create_init_options(project_dir, ai="codex", ai_skills=True)
|
||||||
|
skills_dir = _create_skills_dir(project_dir, ai="codex")
|
||||||
|
manager = ExtensionManager(project_dir)
|
||||||
|
manifest = ExtensionManifest(extension_dir / "extension.yml")
|
||||||
|
|
||||||
|
skill_file = skills_dir / "speckit-test-ext-hello" / "SKILL.md"
|
||||||
|
skill_file.parent.mkdir(parents=True, exist_ok=True)
|
||||||
|
cache_file = (
|
||||||
|
extension_dir
|
||||||
|
/ ".specify-dev"
|
||||||
|
/ "extension-skills"
|
||||||
|
/ "speckit-test-ext-hello"
|
||||||
|
/ "SKILL.md"
|
||||||
|
)
|
||||||
|
cache_file.parent.mkdir(parents=True)
|
||||||
|
cache_file.write_text("old linked content", encoding="utf-8")
|
||||||
|
os.symlink(os.path.relpath(cache_file, skill_file.parent), skill_file)
|
||||||
|
|
||||||
|
written = manager._register_extension_skills(
|
||||||
|
manifest,
|
||||||
|
extension_dir,
|
||||||
|
link_outputs=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert "speckit-test-ext-hello" in written
|
||||||
|
assert skill_file.exists()
|
||||||
|
assert not skill_file.is_symlink()
|
||||||
|
assert "Run this to say hello." in skill_file.read_text(encoding="utf-8")
|
||||||
|
assert cache_file.read_text(encoding="utf-8") == "old linked content"
|
||||||
|
|
||||||
|
def test_codex_dev_skill_registration_preserves_unrelated_symlink(
|
||||||
|
self, project_dir, extension_dir, temp_dir
|
||||||
|
):
|
||||||
|
"""Codex dev registration should not overwrite user-owned symlinks."""
|
||||||
|
if not _can_create_symlink(temp_dir):
|
||||||
|
pytest.skip("Current platform/user cannot create symlinks")
|
||||||
|
|
||||||
|
_create_init_options(project_dir, ai="codex", ai_skills=True)
|
||||||
|
skills_dir = _create_skills_dir(project_dir, ai="codex")
|
||||||
|
manager = ExtensionManager(project_dir)
|
||||||
|
manifest = ExtensionManifest(extension_dir / "extension.yml")
|
||||||
|
|
||||||
|
skill_file = skills_dir / "speckit-test-ext-hello" / "SKILL.md"
|
||||||
|
skill_file.parent.mkdir(parents=True, exist_ok=True)
|
||||||
|
unrelated_cache_file = (
|
||||||
|
temp_dir
|
||||||
|
/ "other-extension"
|
||||||
|
/ ".specify-dev"
|
||||||
|
/ "extension-skills"
|
||||||
|
/ "speckit-test-ext-hello"
|
||||||
|
/ "SKILL.md"
|
||||||
|
)
|
||||||
|
unrelated_cache_file.parent.mkdir(parents=True)
|
||||||
|
unrelated_cache_file.write_text("user-owned linked content", encoding="utf-8")
|
||||||
|
os.symlink(
|
||||||
|
os.path.relpath(unrelated_cache_file, skill_file.parent), skill_file
|
||||||
|
)
|
||||||
|
|
||||||
|
written = manager._register_extension_skills(
|
||||||
|
manifest,
|
||||||
|
extension_dir,
|
||||||
|
link_outputs=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert "speckit-test-ext-hello" not in written
|
||||||
|
assert skill_file.is_symlink()
|
||||||
|
assert skill_file.resolve(strict=True) == unrelated_cache_file.resolve()
|
||||||
|
assert unrelated_cache_file.read_text(encoding="utf-8") == (
|
||||||
|
"user-owned linked content"
|
||||||
|
)
|
||||||
|
|
||||||
def test_dev_skill_registration_falls_back_to_copy_when_symlink_fails(
|
def test_dev_skill_registration_falls_back_to_copy_when_symlink_fails(
|
||||||
self, skills_project, extension_dir, monkeypatch
|
self, skills_project, extension_dir, monkeypatch
|
||||||
):
|
):
|
||||||
|
|||||||
@@ -2248,6 +2248,50 @@ Run {SCRIPT}
|
|||||||
assert target.is_file()
|
assert target.is_file()
|
||||||
assert "Extension: test-ext" in cmd_file.read_text(encoding="utf-8")
|
assert "Extension: test-ext" in cmd_file.read_text(encoding="utf-8")
|
||||||
|
|
||||||
|
def test_dev_register_commands_replaces_codex_dev_symlink(
|
||||||
|
self, extension_dir, project_dir, temp_dir
|
||||||
|
):
|
||||||
|
"""Codex dev registration should replace prior symlinks with real files."""
|
||||||
|
if not can_create_symlink(temp_dir):
|
||||||
|
pytest.skip("Current platform/user cannot create symlinks")
|
||||||
|
|
||||||
|
skill_file = (
|
||||||
|
project_dir
|
||||||
|
/ ".agents"
|
||||||
|
/ "skills"
|
||||||
|
/ "speckit-test-ext-hello"
|
||||||
|
/ "SKILL.md"
|
||||||
|
)
|
||||||
|
skill_file.parent.mkdir(parents=True)
|
||||||
|
cache_file = (
|
||||||
|
extension_dir
|
||||||
|
/ ".specify-dev"
|
||||||
|
/ "agent-commands"
|
||||||
|
/ "codex"
|
||||||
|
/ "speckit-test-ext-hello"
|
||||||
|
/ "SKILL.md"
|
||||||
|
)
|
||||||
|
cache_file.parent.mkdir(parents=True)
|
||||||
|
cache_file.write_text("old linked content", encoding="utf-8")
|
||||||
|
os.symlink(os.path.relpath(cache_file, skill_file.parent), skill_file)
|
||||||
|
|
||||||
|
manifest = ExtensionManifest(extension_dir / "extension.yml")
|
||||||
|
registrar = CommandRegistrar()
|
||||||
|
registrar.register_commands_for_agent(
|
||||||
|
"codex",
|
||||||
|
manifest,
|
||||||
|
extension_dir,
|
||||||
|
project_dir,
|
||||||
|
link_outputs=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert skill_file.exists()
|
||||||
|
assert not skill_file.is_symlink()
|
||||||
|
assert "name: speckit-test-ext-hello" in skill_file.read_text(
|
||||||
|
encoding="utf-8"
|
||||||
|
)
|
||||||
|
assert cache_file.read_text(encoding="utf-8") == "old linked content"
|
||||||
|
|
||||||
def test_dev_register_commands_falls_back_to_copy_when_symlink_fails(
|
def test_dev_register_commands_falls_back_to_copy_when_symlink_fails(
|
||||||
self, extension_dir, project_dir, monkeypatch
|
self, extension_dir, project_dir, monkeypatch
|
||||||
):
|
):
|
||||||
@@ -4874,6 +4918,93 @@ class TestExtensionAddCLI:
|
|||||||
else:
|
else:
|
||||||
assert not agent_file.is_symlink()
|
assert not agent_file.is_symlink()
|
||||||
|
|
||||||
|
def test_add_dev_writes_codex_skills_as_files(self, extension_dir, project_dir):
|
||||||
|
"""Codex dev skills should be written as files so Codex can load them."""
|
||||||
|
from typer.testing import CliRunner
|
||||||
|
from unittest.mock import patch
|
||||||
|
from specify_cli import app
|
||||||
|
|
||||||
|
init_options = project_dir / ".specify" / "init-options.json"
|
||||||
|
init_options.write_text(
|
||||||
|
json.dumps({"ai": "codex", "ai_skills": True}), encoding="utf-8"
|
||||||
|
)
|
||||||
|
|
||||||
|
runner = CliRunner()
|
||||||
|
with patch.object(Path, "cwd", return_value=project_dir):
|
||||||
|
result = runner.invoke(
|
||||||
|
app,
|
||||||
|
["extension", "add", str(extension_dir), "--dev"],
|
||||||
|
catch_exceptions=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result.exit_code == 0, result.output
|
||||||
|
|
||||||
|
skill_file = (
|
||||||
|
project_dir
|
||||||
|
/ ".agents"
|
||||||
|
/ "skills"
|
||||||
|
/ "speckit-test-ext-hello"
|
||||||
|
/ "SKILL.md"
|
||||||
|
)
|
||||||
|
assert skill_file.exists()
|
||||||
|
assert not skill_file.is_symlink()
|
||||||
|
|
||||||
|
content = skill_file.read_text(encoding="utf-8")
|
||||||
|
assert "name: speckit-test-ext-hello" in content
|
||||||
|
assert "metadata:" in content
|
||||||
|
assert "source: test-ext:commands/hello.md" in content
|
||||||
|
|
||||||
|
def test_add_dev_replaces_existing_codex_skill_symlink(
|
||||||
|
self, extension_dir, project_dir, temp_dir
|
||||||
|
):
|
||||||
|
"""Codex dev installs should migrate expected dev symlinks to files."""
|
||||||
|
if not can_create_symlink(temp_dir):
|
||||||
|
pytest.skip("Current platform/user cannot create symlinks")
|
||||||
|
|
||||||
|
from typer.testing import CliRunner
|
||||||
|
from unittest.mock import patch
|
||||||
|
from specify_cli import app
|
||||||
|
|
||||||
|
init_options = project_dir / ".specify" / "init-options.json"
|
||||||
|
init_options.write_text(
|
||||||
|
json.dumps({"ai": "codex", "ai_skills": True}), encoding="utf-8"
|
||||||
|
)
|
||||||
|
|
||||||
|
skill_file = (
|
||||||
|
project_dir
|
||||||
|
/ ".agents"
|
||||||
|
/ "skills"
|
||||||
|
/ "speckit-test-ext-hello"
|
||||||
|
/ "SKILL.md"
|
||||||
|
)
|
||||||
|
skill_file.parent.mkdir(parents=True)
|
||||||
|
cache_file = (
|
||||||
|
extension_dir
|
||||||
|
/ ".specify-dev"
|
||||||
|
/ "extension-skills"
|
||||||
|
/ "speckit-test-ext-hello"
|
||||||
|
/ "SKILL.md"
|
||||||
|
)
|
||||||
|
cache_file.parent.mkdir(parents=True)
|
||||||
|
cache_file.write_text("old linked content", encoding="utf-8")
|
||||||
|
os.symlink(os.path.relpath(cache_file, skill_file.parent), skill_file)
|
||||||
|
|
||||||
|
runner = CliRunner()
|
||||||
|
with patch.object(Path, "cwd", return_value=project_dir):
|
||||||
|
result = runner.invoke(
|
||||||
|
app,
|
||||||
|
["extension", "add", str(extension_dir), "--dev"],
|
||||||
|
catch_exceptions=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result.exit_code == 0, result.output
|
||||||
|
assert skill_file.exists()
|
||||||
|
assert not skill_file.is_symlink()
|
||||||
|
content = skill_file.read_text(encoding="utf-8")
|
||||||
|
assert "name: speckit-test-ext-hello" in content
|
||||||
|
assert "source: test-ext:commands/hello.md" in content
|
||||||
|
assert cache_file.read_text(encoding="utf-8") == "old linked content"
|
||||||
|
|
||||||
def test_add_dev_falls_back_to_copy_when_windows_symlinks_unavailable(
|
def test_add_dev_falls_back_to_copy_when_windows_symlinks_unavailable(
|
||||||
self, extension_dir, project_dir, monkeypatch
|
self, extension_dir, project_dir, monkeypatch
|
||||||
):
|
):
|
||||||
|
|||||||
Reference in New Issue
Block a user