From 3793b453c93b674fb15ec85c852ad62c5b7bd82b Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Sat, 21 Mar 2026 12:50:04 +0800 Subject: [PATCH 1/8] fix(codex): skip legacy prompts and fallback when bundled skills missing --- src/specify_cli/__init__.py | 64 ++++++++++++++++++++++++++++++------- tests/test_ai_skills.py | 18 +++++------ 2 files changed, 61 insertions(+), 21 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index d2bf63eeb..3ab4c1d6c 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -948,7 +948,19 @@ def download_template_from_github(ai_assistant: str, download_dir: Path, *, scri } return zip_path, metadata -def download_and_extract_template(project_path: Path, ai_assistant: str, script_type: str, is_current_dir: bool = False, *, verbose: bool = True, tracker: StepTracker | None = None, client: httpx.Client = None, debug: bool = False, github_token: str = None) -> Path: +def download_and_extract_template( + project_path: Path, + ai_assistant: str, + script_type: str, + is_current_dir: bool = False, + *, + skip_legacy_codex_prompts: bool = False, + verbose: bool = True, + tracker: StepTracker | None = None, + client: httpx.Client = None, + debug: bool = False, + github_token: str = None, +) -> Path: """Download the latest release and extract it to create a new project. Returns project_path. Uses tracker if provided (with keys: fetch, download, extract, cleanup) """ @@ -1019,6 +1031,10 @@ def download_and_extract_template(project_path: Path, ai_assistant: str, script_ console.print("[cyan]Found nested directory structure[/cyan]") for item in source_dir.iterdir(): + # Codex skills mode should not materialize legacy prompt files + # from older template archives. + if skip_legacy_codex_prompts and ai_assistant == "codex" and item.name == ".codex": + continue dest_path = project_path / item.name if item.is_dir(): if dest_path.exists(): @@ -1069,6 +1085,11 @@ def download_and_extract_template(project_path: Path, ai_assistant: str, script_ elif verbose: console.print("[cyan]Flattened nested directory structure[/cyan]") + if skip_legacy_codex_prompts and ai_assistant == "codex": + legacy_codex_dir = project_path / ".codex" + if legacy_codex_dir.is_dir(): + shutil.rmtree(legacy_codex_dir, ignore_errors=True) + except Exception as e: if tracker: tracker.error("extract", str(e)) @@ -1994,7 +2015,18 @@ def init( if use_github: with httpx.Client(verify=local_ssl_context) as local_client: - download_and_extract_template(project_path, selected_ai, selected_script, here, verbose=False, tracker=tracker, client=local_client, debug=debug, github_token=github_token) + download_and_extract_template( + project_path, + selected_ai, + selected_script, + here, + skip_legacy_codex_prompts=(selected_ai == "codex" and ai_skills), + verbose=False, + tracker=tracker, + client=local_client, + debug=debug, + github_token=github_token, + ) else: scaffold_ok = scaffold_from_core_pack(project_path, selected_ai, selected_script, here, tracker=tracker) if not scaffold_ok: @@ -2013,7 +2045,6 @@ def init( if not here and project_path.exists(): shutil.rmtree(project_path) raise typer.Exit(1) - # For generic agent, rename placeholder directory to user-specified path if selected_ai == "generic" and ai_commands_dir: placeholder_dir = project_path / ".speckit" / "commands" @@ -2033,16 +2064,25 @@ def init( if ai_skills: if selected_ai in NATIVE_SKILLS_AGENTS: skills_dir = _get_skills_dir(project_path, selected_ai) - if not _has_bundled_skills(project_path, selected_ai): - raise RuntimeError( - f"Expected bundled agent skills in {skills_dir.relative_to(project_path)}, " - "but none were found. Re-run with an up-to-date template." - ) - if tracker: - tracker.start("ai-skills") - tracker.complete("ai-skills", f"bundled skills → {skills_dir.relative_to(project_path)}") + bundled_found = _has_bundled_skills(project_path, selected_ai) + if bundled_found: + if tracker: + tracker.start("ai-skills") + tracker.complete("ai-skills", f"bundled skills → {skills_dir.relative_to(project_path)}") + else: + console.print(f"[green]✓[/green] Using bundled agent skills in {skills_dir.relative_to(project_path)}/") else: - console.print(f"[green]✓[/green] Using bundled agent skills in {skills_dir.relative_to(project_path)}/") + # Compatibility fallback: convert command templates to skills + # when an older template archive does not include native skills. + # This keeps `specify init --here --ai codex --ai-skills` usable + # in repos that already contain unrelated skills under .agents/skills. + fallback_ok = install_ai_skills(project_path, selected_ai, tracker=tracker) + if not fallback_ok: + raise RuntimeError( + f"Expected bundled agent skills in {skills_dir.relative_to(project_path)}, " + "but none were found and fallback conversion failed. " + "Re-run with an up-to-date template." + ) else: skills_ok = install_ai_skills(project_path, selected_ai, tracker=tracker) diff --git a/tests/test_ai_skills.py b/tests/test_ai_skills.py index 0bccd48d4..e6284bfe8 100644 --- a/tests/test_ai_skills.py +++ b/tests/test_ai_skills.py @@ -720,8 +720,8 @@ def fake_download(project_path, *args, **kwargs): mock_skills.assert_not_called() assert (target / ".agents" / "skills" / "speckit-specify" / "SKILL.md").exists() - def test_codex_native_skills_missing_fails_clearly(self, tmp_path): - """Codex native skills init should fail if bundled skills are missing.""" + def test_codex_native_skills_missing_falls_back_then_fails_cleanly(self, tmp_path): + """Codex should attempt fallback conversion when bundled skills are missing.""" from typer.testing import CliRunner runner = CliRunner() @@ -730,7 +730,7 @@ def test_codex_native_skills_missing_fails_clearly(self, tmp_path): with patch("specify_cli.download_and_extract_template", lambda *args, **kwargs: None), \ patch("specify_cli.ensure_executable_scripts"), \ patch("specify_cli.ensure_constitution_from_template"), \ - patch("specify_cli.install_ai_skills") as mock_skills, \ + patch("specify_cli.install_ai_skills", return_value=False) as mock_skills, \ patch("specify_cli.is_git_repo", return_value=False), \ patch("specify_cli.shutil.which", return_value="/usr/bin/codex"): result = runner.invoke( @@ -739,11 +739,12 @@ def test_codex_native_skills_missing_fails_clearly(self, tmp_path): ) assert result.exit_code == 1 - mock_skills.assert_not_called() + mock_skills.assert_called_once() assert "Expected bundled agent skills" in result.output + assert "fallback conversion failed" in result.output def test_codex_native_skills_ignores_non_speckit_skill_dirs(self, tmp_path): - """Non-spec-kit SKILL.md files should not satisfy Codex bundled-skills validation.""" + """Non-spec-kit SKILL.md files should trigger fallback conversion, not hard-fail.""" from typer.testing import CliRunner runner = CliRunner() @@ -757,7 +758,7 @@ def fake_download(project_path, *args, **kwargs): with patch("specify_cli.download_and_extract_template", side_effect=fake_download), \ patch("specify_cli.ensure_executable_scripts"), \ patch("specify_cli.ensure_constitution_from_template"), \ - patch("specify_cli.install_ai_skills") as mock_skills, \ + patch("specify_cli.install_ai_skills", return_value=True) as mock_skills, \ patch("specify_cli.is_git_repo", return_value=False), \ patch("specify_cli.shutil.which", return_value="/usr/bin/codex"): result = runner.invoke( @@ -765,9 +766,8 @@ def fake_download(project_path, *args, **kwargs): ["init", str(target), "--ai", "codex", "--ai-skills", "--script", "sh", "--no-git"], ) - assert result.exit_code == 1 - mock_skills.assert_not_called() - assert "Expected bundled agent skills" in result.output + assert result.exit_code == 0 + mock_skills.assert_called_once() def test_commands_preserved_when_skills_fail(self, tmp_path): """If skills fail, commands should NOT be removed (safety net).""" From 94ec86e8676e8ee871885547ddb3dfafa1058ee4 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Sat, 21 Mar 2026 13:04:34 +0800 Subject: [PATCH 2/8] fix(skills): allow native fallback to overwrite existing SKILL.md --- src/specify_cli/__init__.py | 28 +++++++++++++++++++++------- tests/test_ai_skills.py | 17 +++++++++++++++++ 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 3ab4c1d6c..dfa127679 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1520,18 +1520,26 @@ def _get_skills_dir(project_path: Path, selected_ai: str) -> Path: return project_path / DEFAULT_SKILLS_DIR -def install_ai_skills(project_path: Path, selected_ai: str, tracker: StepTracker | None = None) -> bool: +def install_ai_skills( + project_path: Path, + selected_ai: str, + tracker: StepTracker | None = None, + *, + overwrite_existing: bool = False, +) -> bool: """Install Prompt.MD files from templates/commands/ as agent skills. Skills are written to the agent-specific skills directory following the `agentskills.io `_ specification. - Installation is additive — existing files are never removed and prompt - command files in the agent's commands directory are left untouched. + Installation is additive by default — existing files are never removed and + prompt command files in the agent's commands directory are left untouched. Args: project_path: Target project directory. selected_ai: AI assistant key from ``AGENT_CONFIG``. tracker: Optional progress tracker. + overwrite_existing: When True, overwrite existing ``SKILL.md`` files + generated by previous runs. Defaults to False. Returns: ``True`` if at least one skill was installed or all skills were @@ -1661,9 +1669,10 @@ def install_ai_skills(project_path: Path, selected_ai: str, tracker: StepTracker skill_file = skill_dir / "SKILL.md" if skill_file.exists(): - # Do not overwrite user-customized skills on re-runs - skipped_count += 1 - continue + if not overwrite_existing: + # Default behavior: do not overwrite user-customized skills on re-runs + skipped_count += 1 + continue skill_file.write_text(skill_content, encoding="utf-8") installed_count += 1 @@ -2076,7 +2085,12 @@ def init( # when an older template archive does not include native skills. # This keeps `specify init --here --ai codex --ai-skills` usable # in repos that already contain unrelated skills under .agents/skills. - fallback_ok = install_ai_skills(project_path, selected_ai, tracker=tracker) + fallback_ok = install_ai_skills( + project_path, + selected_ai, + tracker=tracker, + overwrite_existing=True, + ) if not fallback_ok: raise RuntimeError( f"Expected bundled agent skills in {skills_dir.relative_to(project_path)}, " diff --git a/tests/test_ai_skills.py b/tests/test_ai_skills.py index e6284bfe8..ab97f0b0f 100644 --- a/tests/test_ai_skills.py +++ b/tests/test_ai_skills.py @@ -740,6 +740,7 @@ def test_codex_native_skills_missing_falls_back_then_fails_cleanly(self, tmp_pat assert result.exit_code == 1 mock_skills.assert_called_once() + assert mock_skills.call_args.kwargs.get("overwrite_existing") is True assert "Expected bundled agent skills" in result.output assert "fallback conversion failed" in result.output @@ -768,6 +769,7 @@ def fake_download(project_path, *args, **kwargs): assert result.exit_code == 0 mock_skills.assert_called_once() + assert mock_skills.call_args.kwargs.get("overwrite_existing") is True def test_commands_preserved_when_skills_fail(self, tmp_path): """If skills fail, commands should NOT be removed (safety net).""" @@ -859,6 +861,21 @@ def test_fresh_install_writes_all_skills(self, project_dir, templates_dir): # All 4 templates should produce skills (specify, plan, tasks, empty_fm) assert len(skill_dirs) == 4 + def test_existing_skill_overwritten_when_enabled(self, project_dir, templates_dir): + """When overwrite_existing=True, pre-existing SKILL.md should be replaced.""" + skill_dir = project_dir / ".claude" / "skills" / "speckit-specify" + skill_dir.mkdir(parents=True) + custom_content = "# My Custom Specify Skill\nUser-modified content\n" + skill_file = skill_dir / "SKILL.md" + skill_file.write_text(custom_content) + + result = install_ai_skills(project_dir, "claude", overwrite_existing=True) + + assert result is True + updated_content = skill_file.read_text() + assert updated_content != custom_content + assert "name: speckit-specify" in updated_content + # ===== SKILL_DESCRIPTIONS Coverage Tests ===== From 9736af918da7da52e90f7422c2d06c9f7c118296 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Sat, 21 Mar 2026 13:14:22 +0800 Subject: [PATCH 3/8] fix(codex): defer legacy .codex cleanup until after skills fallback --- src/specify_cli/__init__.py | 17 ++++++----- tests/test_ai_skills.py | 56 +++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index dfa127679..d7e1cd3bf 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1031,10 +1031,6 @@ def download_and_extract_template( console.print("[cyan]Found nested directory structure[/cyan]") for item in source_dir.iterdir(): - # Codex skills mode should not materialize legacy prompt files - # from older template archives. - if skip_legacy_codex_prompts and ai_assistant == "codex" and item.name == ".codex": - continue dest_path = project_path / item.name if item.is_dir(): if dest_path.exists(): @@ -1085,11 +1081,6 @@ def download_and_extract_template( elif verbose: console.print("[cyan]Flattened nested directory structure[/cyan]") - if skip_legacy_codex_prompts and ai_assistant == "codex": - legacy_codex_dir = project_path / ".codex" - if legacy_codex_dir.is_dir(): - shutil.rmtree(legacy_codex_dir, ignore_errors=True) - except Exception as e: if tracker: tracker.error("extract", str(e)) @@ -2121,6 +2112,14 @@ def init( # so leaving stale commands is non-fatal. console.print("[yellow]Warning: could not remove extracted commands directory[/yellow]") + # In Codex skills mode, remove legacy prompt layout after skills + # installation/fallback completes so prompt artifacts are not left + # behind in the project tree. + if selected_ai == "codex" and ai_skills: + legacy_codex_dir = project_path / ".codex" + if legacy_codex_dir.is_dir(): + shutil.rmtree(legacy_codex_dir, ignore_errors=True) + if not no_git: tracker.start("git") if is_git_repo(project_path): diff --git a/tests/test_ai_skills.py b/tests/test_ai_skills.py index ab97f0b0f..1f9307f3d 100644 --- a/tests/test_ai_skills.py +++ b/tests/test_ai_skills.py @@ -770,6 +770,62 @@ def fake_download(project_path, *args, **kwargs): assert result.exit_code == 0 mock_skills.assert_called_once() assert mock_skills.call_args.kwargs.get("overwrite_existing") is True + assert not (target / ".codex").exists() + + def test_codex_ai_skills_removes_legacy_codex_dir_after_fallback(self, tmp_path): + """Codex skills init should not leave legacy .codex prompt directory behind.""" + from typer.testing import CliRunner + + runner = CliRunner() + target = tmp_path / "codex-cleanup-new" + + def fake_download(project_path, *args, **kwargs): + prompts_dir = project_path / ".codex" / "prompts" + prompts_dir.mkdir(parents=True, exist_ok=True) + (prompts_dir / "speckit.specify.md").write_text("---\ndescription: Legacy prompt\n---\n\nBody.\n") + + with patch("specify_cli.download_and_extract_template", side_effect=fake_download), \ + patch("specify_cli.ensure_executable_scripts"), \ + patch("specify_cli.ensure_constitution_from_template"), \ + patch("specify_cli.install_ai_skills", return_value=True), \ + patch("specify_cli.is_git_repo", return_value=False), \ + patch("specify_cli.shutil.which", return_value="/usr/bin/codex"): + result = runner.invoke( + app, + ["init", str(target), "--ai", "codex", "--ai-skills", "--script", "sh", "--no-git"], + ) + + assert result.exit_code == 0 + assert not (target / ".codex").exists() + + def test_codex_ai_skills_here_mode_removes_legacy_codex_dir(self, tmp_path, monkeypatch): + """Codex --here skills init should not leave legacy .codex prompt directory behind.""" + from typer.testing import CliRunner + + runner = CliRunner() + target = tmp_path / "codex-cleanup-here" + target.mkdir() + monkeypatch.chdir(target) + + def fake_download(project_path, *args, **kwargs): + prompts_dir = project_path / ".codex" / "prompts" + prompts_dir.mkdir(parents=True, exist_ok=True) + (prompts_dir / "speckit.specify.md").write_text("---\ndescription: Legacy prompt\n---\n\nBody.\n") + + with patch("specify_cli.download_and_extract_template", side_effect=fake_download), \ + patch("specify_cli.ensure_executable_scripts"), \ + patch("specify_cli.ensure_constitution_from_template"), \ + patch("specify_cli.install_ai_skills", return_value=True), \ + patch("specify_cli.is_git_repo", return_value=True), \ + patch("specify_cli.shutil.which", return_value="/usr/bin/codex"): + result = runner.invoke( + app, + ["init", "--here", "--ai", "codex", "--ai-skills", "--script", "sh", "--no-git"], + input="y\n", + ) + + assert result.exit_code == 0 + assert not (target / ".codex").exists() def test_commands_preserved_when_skills_fail(self, tmp_path): """If skills fail, commands should NOT be removed (safety net).""" From 979a30e25772769852e87845e0b3a6df039af7d3 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Sat, 21 Mar 2026 13:18:14 +0800 Subject: [PATCH 4/8] fix(codex): preserve existing .codex while skipping legacy prompt extraction --- src/specify_cli/__init__.py | 12 ++++------ tests/test_ai_skills.py | 46 ++++++++----------------------------- 2 files changed, 13 insertions(+), 45 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index d7e1cd3bf..054267dae 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1031,6 +1031,10 @@ def download_and_extract_template( console.print("[cyan]Found nested directory structure[/cyan]") for item in source_dir.iterdir(): + # In Codex skills mode, do not materialize legacy prompt + # templates under .codex/prompts. + if skip_legacy_codex_prompts and ai_assistant == "codex" and item.name == ".codex": + continue dest_path = project_path / item.name if item.is_dir(): if dest_path.exists(): @@ -2112,14 +2116,6 @@ def init( # so leaving stale commands is non-fatal. console.print("[yellow]Warning: could not remove extracted commands directory[/yellow]") - # In Codex skills mode, remove legacy prompt layout after skills - # installation/fallback completes so prompt artifacts are not left - # behind in the project tree. - if selected_ai == "codex" and ai_skills: - legacy_codex_dir = project_path / ".codex" - if legacy_codex_dir.is_dir(): - shutil.rmtree(legacy_codex_dir, ignore_errors=True) - if not no_git: tracker.start("git") if is_git_repo(project_path): diff --git a/tests/test_ai_skills.py b/tests/test_ai_skills.py index 1f9307f3d..0e73d143e 100644 --- a/tests/test_ai_skills.py +++ b/tests/test_ai_skills.py @@ -770,49 +770,20 @@ def fake_download(project_path, *args, **kwargs): assert result.exit_code == 0 mock_skills.assert_called_once() assert mock_skills.call_args.kwargs.get("overwrite_existing") is True - assert not (target / ".codex").exists() - def test_codex_ai_skills_removes_legacy_codex_dir_after_fallback(self, tmp_path): - """Codex skills init should not leave legacy .codex prompt directory behind.""" + def test_codex_ai_skills_here_mode_preserves_existing_codex_dir(self, tmp_path, monkeypatch): + """Codex --here skills init should not delete a pre-existing .codex directory.""" from typer.testing import CliRunner runner = CliRunner() - target = tmp_path / "codex-cleanup-new" - - def fake_download(project_path, *args, **kwargs): - prompts_dir = project_path / ".codex" / "prompts" - prompts_dir.mkdir(parents=True, exist_ok=True) - (prompts_dir / "speckit.specify.md").write_text("---\ndescription: Legacy prompt\n---\n\nBody.\n") - - with patch("specify_cli.download_and_extract_template", side_effect=fake_download), \ - patch("specify_cli.ensure_executable_scripts"), \ - patch("specify_cli.ensure_constitution_from_template"), \ - patch("specify_cli.install_ai_skills", return_value=True), \ - patch("specify_cli.is_git_repo", return_value=False), \ - patch("specify_cli.shutil.which", return_value="/usr/bin/codex"): - result = runner.invoke( - app, - ["init", str(target), "--ai", "codex", "--ai-skills", "--script", "sh", "--no-git"], - ) - - assert result.exit_code == 0 - assert not (target / ".codex").exists() - - def test_codex_ai_skills_here_mode_removes_legacy_codex_dir(self, tmp_path, monkeypatch): - """Codex --here skills init should not leave legacy .codex prompt directory behind.""" - from typer.testing import CliRunner - - runner = CliRunner() - target = tmp_path / "codex-cleanup-here" + target = tmp_path / "codex-preserve-here" target.mkdir() + existing_prompts = target / ".codex" / "prompts" + existing_prompts.mkdir(parents=True) + (existing_prompts / "custom.md").write_text("custom") monkeypatch.chdir(target) - def fake_download(project_path, *args, **kwargs): - prompts_dir = project_path / ".codex" / "prompts" - prompts_dir.mkdir(parents=True, exist_ok=True) - (prompts_dir / "speckit.specify.md").write_text("---\ndescription: Legacy prompt\n---\n\nBody.\n") - - with patch("specify_cli.download_and_extract_template", side_effect=fake_download), \ + with patch("specify_cli.download_and_extract_template", return_value=target), \ patch("specify_cli.ensure_executable_scripts"), \ patch("specify_cli.ensure_constitution_from_template"), \ patch("specify_cli.install_ai_skills", return_value=True), \ @@ -825,7 +796,8 @@ def fake_download(project_path, *args, **kwargs): ) assert result.exit_code == 0 - assert not (target / ".codex").exists() + assert (target / ".codex").exists() + assert (existing_prompts / "custom.md").exists() def test_commands_preserved_when_skills_fail(self, tmp_path): """If skills fail, commands should NOT be removed (safety net).""" From 28b480148e73b79ae31a75490ac0f5b53fafcfe0 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Sat, 21 Mar 2026 13:25:58 +0800 Subject: [PATCH 5/8] docs(skills): clarify overwrite_existing behavior --- src/specify_cli/__init__.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 054267dae..ff4e73ef4 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1533,8 +1533,9 @@ def install_ai_skills( project_path: Target project directory. selected_ai: AI assistant key from ``AGENT_CONFIG``. tracker: Optional progress tracker. - overwrite_existing: When True, overwrite existing ``SKILL.md`` files - generated by previous runs. Defaults to False. + overwrite_existing: When True, overwrite any existing ``SKILL.md`` file + in the target skills directory (including user-authored content). + Defaults to False. Returns: ``True`` if at least one skill was installed or all skills were From d98e275bb025f9615c50d237206569c13a7b66f4 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Sat, 21 Mar 2026 13:48:36 +0800 Subject: [PATCH 6/8] test(codex): cover fresh-dir suppression of legacy .codex layout --- src/specify_cli/__init__.py | 7 +++++++ tests/test_ai_skills.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index ff4e73ef4..d253f851b 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1085,6 +1085,13 @@ def download_and_extract_template( elif verbose: console.print("[cyan]Flattened nested directory structure[/cyan]") + # For fresh-directory Codex skills init, suppress legacy + # prompt-based layout extracted from older archives. + if skip_legacy_codex_prompts and ai_assistant == "codex": + legacy_codex_dir = project_path / ".codex" + if legacy_codex_dir.is_dir(): + shutil.rmtree(legacy_codex_dir, ignore_errors=True) + except Exception as e: if tracker: tracker.error("extract", str(e)) diff --git a/tests/test_ai_skills.py b/tests/test_ai_skills.py index 0e73d143e..361b0bc51 100644 --- a/tests/test_ai_skills.py +++ b/tests/test_ai_skills.py @@ -11,6 +11,7 @@ """ import re +import zipfile import pytest import tempfile import shutil @@ -799,6 +800,36 @@ def test_codex_ai_skills_here_mode_preserves_existing_codex_dir(self, tmp_path, assert (target / ".codex").exists() assert (existing_prompts / "custom.md").exists() + def test_codex_ai_skills_fresh_dir_does_not_create_codex_dir(self, tmp_path): + """Fresh-directory Codex skills init should not leave legacy .codex from archive.""" + target = tmp_path / "fresh-codex-proj" + archive = tmp_path / "codex-template.zip" + + with zipfile.ZipFile(archive, "w") as zf: + zf.writestr("template-root/.codex/prompts/speckit.specify.md", "legacy") + zf.writestr("template-root/.specify/templates/constitution-template.md", "constitution") + + fake_meta = { + "filename": archive.name, + "size": archive.stat().st_size, + "release": "vtest", + "asset_url": "https://example.invalid/template.zip", + } + + with patch("specify_cli.download_template_from_github", return_value=(archive, fake_meta)): + specify_cli.download_and_extract_template( + target, + "codex", + "sh", + is_current_dir=False, + skip_legacy_codex_prompts=True, + verbose=False, + ) + + assert target.exists() + assert (target / ".specify").exists() + assert not (target / ".codex").exists() + def test_commands_preserved_when_skills_fail(self, tmp_path): """If skills fail, commands should NOT be removed (safety net).""" from typer.testing import CliRunner From 9bec94dbb8fe76d9a0e982f7b9f4c306362aa0d4 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Sat, 21 Mar 2026 14:29:58 +0800 Subject: [PATCH 7/8] docs(codex): clarify skip_legacy_codex_prompts suppresses full .codex dir --- src/specify_cli/__init__.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index d253f851b..7d613139d 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -963,6 +963,11 @@ def download_and_extract_template( ) -> Path: """Download the latest release and extract it to create a new project. Returns project_path. Uses tracker if provided (with keys: fetch, download, extract, cleanup) + + Note: + ``skip_legacy_codex_prompts`` suppresses the legacy top-level + ``.codex`` directory from older template archives in Codex skills mode. + The name is kept for backward compatibility with existing callers. """ current_dir = Path.cwd() @@ -1031,8 +1036,9 @@ def download_and_extract_template( console.print("[cyan]Found nested directory structure[/cyan]") for item in source_dir.iterdir(): - # In Codex skills mode, do not materialize legacy prompt - # templates under .codex/prompts. + # In Codex skills mode, do not materialize the legacy + # top-level .codex directory from older prompt-based + # template archives. if skip_legacy_codex_prompts and ai_assistant == "codex" and item.name == ".codex": continue dest_path = project_path / item.name @@ -1086,7 +1092,7 @@ def download_and_extract_template( console.print("[cyan]Flattened nested directory structure[/cyan]") # For fresh-directory Codex skills init, suppress legacy - # prompt-based layout extracted from older archives. + # top-level .codex layout extracted from older archives. if skip_legacy_codex_prompts and ai_assistant == "codex": legacy_codex_dir = project_path / ".codex" if legacy_codex_dir.is_dir(): From 80cea01fef1345dd5f50be74d5dc92a7124cf378 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Sat, 21 Mar 2026 15:23:04 +0800 Subject: [PATCH 8/8] security(init): validate zip member paths before extraction --- src/specify_cli/__init__.py | 15 +++++++++++++++ tests/test_ai_skills.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 7d613139d..1e4c296e6 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1007,6 +1007,19 @@ def download_and_extract_template( project_path.mkdir(parents=True) with zipfile.ZipFile(zip_path, 'r') as zip_ref: + def _validate_zip_members_within(root: Path) -> None: + """Validate all ZIP members stay within ``root`` (Zip Slip guard).""" + root_resolved = root.resolve() + for member in zip_ref.namelist(): + member_path = (root / member).resolve() + try: + member_path.relative_to(root_resolved) + except ValueError: + raise RuntimeError( + f"Unsafe path in ZIP archive: {member} " + "(potential path traversal)" + ) + zip_contents = zip_ref.namelist() if tracker: tracker.start("zip-list") @@ -1017,6 +1030,7 @@ def download_and_extract_template( if is_current_dir: with tempfile.TemporaryDirectory() as temp_dir: temp_path = Path(temp_dir) + _validate_zip_members_within(temp_path) zip_ref.extractall(temp_path) extracted_items = list(temp_path.iterdir()) @@ -1065,6 +1079,7 @@ def download_and_extract_template( if verbose and not tracker: console.print("[cyan]Template files merged into current directory[/cyan]") else: + _validate_zip_members_within(project_path) zip_ref.extractall(project_path) extracted_items = list(project_path.iterdir()) diff --git a/tests/test_ai_skills.py b/tests/test_ai_skills.py index 361b0bc51..cf2b6b7b9 100644 --- a/tests/test_ai_skills.py +++ b/tests/test_ai_skills.py @@ -16,6 +16,7 @@ import tempfile import shutil import yaml +import typer from pathlib import Path from unittest.mock import patch @@ -830,6 +831,39 @@ def test_codex_ai_skills_fresh_dir_does_not_create_codex_dir(self, tmp_path): assert (target / ".specify").exists() assert not (target / ".codex").exists() + @pytest.mark.parametrize("is_current_dir", [False, True]) + def test_download_and_extract_template_blocks_zip_path_traversal(self, tmp_path, monkeypatch, is_current_dir): + """Extraction should reject ZIP members escaping the target directory.""" + target = (tmp_path / "here-proj") if is_current_dir else (tmp_path / "new-proj") + if is_current_dir: + target.mkdir() + monkeypatch.chdir(target) + + archive = tmp_path / "malicious-template.zip" + with zipfile.ZipFile(archive, "w") as zf: + zf.writestr("../evil.txt", "pwned") + zf.writestr("template-root/.specify/templates/constitution-template.md", "constitution") + + fake_meta = { + "filename": archive.name, + "size": archive.stat().st_size, + "release": "vtest", + "asset_url": "https://example.invalid/template.zip", + } + + with patch("specify_cli.download_template_from_github", return_value=(archive, fake_meta)): + with pytest.raises(typer.Exit): + specify_cli.download_and_extract_template( + target, + "codex", + "sh", + is_current_dir=is_current_dir, + skip_legacy_codex_prompts=True, + verbose=False, + ) + + assert not (tmp_path / "evil.txt").exists() + def test_commands_preserved_when_skills_fail(self, tmp_path): """If skills fail, commands should NOT be removed (safety net).""" from typer.testing import CliRunner