From 0b1df3bf385bd266b842acf6d8f699f8d3bf9e38 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 16:18:07 -0500 Subject: [PATCH 1/2] fix: --force now overwrites shared infra files during init and upgrade _install_shared_infra() previously skipped all existing files under .specify/scripts/ and .specify/templates/, regardless of --force. This meant users could never receive upstream fixes to shared scripts or templates after initial project setup. Changes: - Add force parameter to _install_shared_infra(); when True, existing files are overwritten with the latest bundled versions - Wire force=True through specify init --here --force and specify integration upgrade --force call sites - Replace hidden logging.warning with visible console output listing skipped files and suggesting --force - Fix contradictory upgrade docs that claimed --force updated shared infra (it didn't) and warned about overwrites (they didn't happen) - Add 6 tests: unit tests for skip/overwrite/warning behavior, plus end-to-end CLI tests for both --force and non-force paths Fixes #2319 --- docs/upgrade.md | 15 ++-- src/specify_cli/__init__.py | 29 ++++--- tests/integrations/test_cli.py | 134 ++++++++++++++++++++++++++++++--- 3 files changed, 151 insertions(+), 27 deletions(-) diff --git a/docs/upgrade.md b/docs/upgrade.md index 020360d222..16f4b4b0c0 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -53,8 +53,8 @@ When Spec Kit releases new features (like new slash commands or updated template Running `specify init --here --force` will update: - ✅ **Slash command files** (`.claude/commands/`, `.github/prompts/`, etc.) -- ✅ **Script files** (`.specify/scripts/`) -- ✅ **Template files** (`.specify/templates/`) +- ✅ **Script files** (`.specify/scripts/`) — **only with `--force`**; without it, only missing files are added +- ✅ **Template files** (`.specify/templates/`) — **only with `--force`**; without it, only missing files are added - ✅ **Shared memory files** (`.specify/memory/`) - **⚠️ See warnings below** ### What stays safe? @@ -94,7 +94,9 @@ Template files will be merged with existing content and may overwrite existing f Proceed? [y/N] ``` -With `--force`, it skips the confirmation and proceeds immediately. +With `--force`, it skips the confirmation and proceeds immediately. It also **overwrites shared infrastructure files** (`.specify/scripts/` and `.specify/templates/`) with the latest versions from the installed Spec Kit release. + +Without `--force`, shared infrastructure files that already exist are skipped — the CLI will print a warning listing the skipped files so you know which ones were not updated. **Important: Your `specs/` directory is always safe.** The `--force` flag only affects template files (commands, scripts, templates, memory). Your feature specifications, plans, and tasks in `specs/` are never included in upgrade packages and cannot be overwritten. @@ -126,13 +128,14 @@ Or use git to restore it: git restore .specify/memory/constitution.md ``` -### 2. Custom template modifications +### 2. Custom script or template modifications -If you customized any templates in `.specify/templates/`, the upgrade will overwrite them. Back them up first: +If you customized files in `.specify/scripts/` or `.specify/templates/`, the `--force` flag will overwrite them. Back them up first: ```bash -# Back up custom templates +# Back up custom templates and scripts cp -r .specify/templates .specify/templates-backup +cp -r .specify/scripts .specify/scripts-backup # After upgrade, merge your changes back manually ``` diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 97cb993a96..b03384eb4a 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -714,12 +714,18 @@ def _install_shared_infra( project_path: Path, script_type: str, tracker: StepTracker | None = None, + force: bool = False, ) -> bool: """Install shared infrastructure files into *project_path*. Copies ``.specify/scripts/`` and ``.specify/templates/`` from the bundled core_pack or source checkout. Tracks all installed files in ``speckit.manifest.json``. + + When *force* is ``True``, existing files are overwritten with the + latest bundled versions. When ``False`` (default), only missing + files are added and existing ones are skipped. + Returns ``True`` on success. """ from .integrations.manifest import IntegrationManifest @@ -744,12 +750,11 @@ def _install_shared_infra( if variant_src.is_dir(): dest_variant = dest_scripts / variant_dir dest_variant.mkdir(parents=True, exist_ok=True) - # Merge without overwriting — only add files that don't exist yet for src_path in variant_src.rglob("*"): if src_path.is_file(): rel_path = src_path.relative_to(variant_src) dst_path = dest_variant / rel_path - if dst_path.exists(): + if dst_path.exists() and not force: skipped_files.append(str(dst_path.relative_to(project_path))) else: dst_path.parent.mkdir(parents=True, exist_ok=True) @@ -770,7 +775,7 @@ def _install_shared_infra( for f in templates_src.iterdir(): if f.is_file() and f.name != "vscode-settings.json" and not f.name.startswith("."): dst = dest_templates / f.name - if dst.exists(): + if dst.exists() and not force: skipped_files.append(str(dst.relative_to(project_path))) else: shutil.copy2(f, dst) @@ -778,10 +783,13 @@ def _install_shared_infra( manifest.record_existing(rel) if skipped_files: - import logging - logging.getLogger(__name__).warning( - "The following shared files already exist and were not overwritten:\n%s", - "\n".join(f" {f}" for f in skipped_files), + console.print( + f"[yellow]⚠[/yellow] {len(skipped_files)} shared infrastructure file(s) already exist and were not updated:" + ) + for f in skipped_files: + console.print(f" {f}") + console.print( + "Use [cyan]--force[/cyan] to overwrite with the latest versions." ) manifest.save() @@ -1272,7 +1280,7 @@ def init( # Install shared infrastructure (scripts, templates) tracker.start("shared-infra") - _install_shared_infra(project_path, selected_script, tracker=tracker) + _install_shared_infra(project_path, selected_script, tracker=tracker, force=force) tracker.complete("shared-infra", f"scripts ({selected_script}) + templates") ensure_constitution_from_template(project_path, tracker=tracker) @@ -2297,9 +2305,8 @@ def integration_upgrade( selected_script = _resolve_script_type(project_root, script) - # Ensure shared infrastructure is present (safe to run unconditionally; - # _install_shared_infra merges missing files without overwriting). - _install_shared_infra(project_root, selected_script) + # Ensure shared infrastructure is up to date; --force overwrites existing files. + _install_shared_infra(project_root, selected_script, force=force) if os.name != "nt": ensure_executable_scripts(project_root) diff --git a/tests/integrations/test_cli.py b/tests/integrations/test_cli.py index ff9386d626..c02924a620 100644 --- a/tests/integrations/test_cli.py +++ b/tests/integrations/test_cli.py @@ -173,13 +173,43 @@ def test_ai_claude_here_preserves_preexisting_commands(self, tmp_path): assert "speckit-specify" in command_file.read_text(encoding="utf-8") assert (project / ".claude" / "skills" / "speckit-plan" / "SKILL.md").exists() - def test_shared_infra_skips_existing_files(self, tmp_path): - """Pre-existing shared files are not overwritten by _install_shared_infra.""" - from typer.testing import CliRunner - from specify_cli import app + def test_shared_infra_skips_existing_files_without_force(self, tmp_path): + """Pre-existing shared files are not overwritten without --force.""" + from specify_cli import _install_shared_infra project = tmp_path / "skip-test" project.mkdir() + (project / ".specify").mkdir() + + # Pre-create a shared script with custom content + scripts_dir = project / ".specify" / "scripts" / "bash" + scripts_dir.mkdir(parents=True) + custom_content = "# user-modified common.sh\n" + (scripts_dir / "common.sh").write_text(custom_content, encoding="utf-8") + + # Pre-create a shared template with custom content + templates_dir = project / ".specify" / "templates" + templates_dir.mkdir(parents=True) + custom_template = "# user-modified spec-template\n" + (templates_dir / "spec-template.md").write_text(custom_template, encoding="utf-8") + + _install_shared_infra(project, "sh", force=False) + + # User's files should be preserved (not overwritten) + assert (scripts_dir / "common.sh").read_text(encoding="utf-8") == custom_content + assert (templates_dir / "spec-template.md").read_text(encoding="utf-8") == custom_template + + # Other shared files should still be installed + assert (scripts_dir / "setup-plan.sh").exists() + assert (templates_dir / "plan-template.md").exists() + + def test_shared_infra_overwrites_existing_files_with_force(self, tmp_path): + """Pre-existing shared files ARE overwritten when force=True.""" + from specify_cli import _install_shared_infra + + project = tmp_path / "force-test" + project.mkdir() + (project / ".specify").mkdir() # Pre-create a shared script with custom content scripts_dir = project / ".specify" / "scripts" / "bash" @@ -193,6 +223,64 @@ def test_shared_infra_skips_existing_files(self, tmp_path): custom_template = "# user-modified spec-template\n" (templates_dir / "spec-template.md").write_text(custom_template, encoding="utf-8") + _install_shared_infra(project, "sh", force=True) + + # Files should be overwritten with bundled versions + assert (scripts_dir / "common.sh").read_text(encoding="utf-8") != custom_content + assert (templates_dir / "spec-template.md").read_text(encoding="utf-8") != custom_template + + # Other shared files should also be installed + assert (scripts_dir / "setup-plan.sh").exists() + assert (templates_dir / "plan-template.md").exists() + + def test_shared_infra_skip_warning_displayed(self, tmp_path, capsys): + """Console warning is displayed when files are skipped.""" + from specify_cli import _install_shared_infra + + project = tmp_path / "warn-test" + project.mkdir() + (project / ".specify").mkdir() + + scripts_dir = project / ".specify" / "scripts" / "bash" + scripts_dir.mkdir(parents=True) + (scripts_dir / "common.sh").write_text("# custom\n", encoding="utf-8") + + _install_shared_infra(project, "sh", force=False) + + captured = capsys.readouterr() + assert "already exist and were not updated" in captured.out + assert "--force" in captured.out + + def test_shared_infra_no_warning_when_forced(self, tmp_path, capsys): + """No skip warning when force=True (all files overwritten).""" + from specify_cli import _install_shared_infra + + project = tmp_path / "no-warn-test" + project.mkdir() + (project / ".specify").mkdir() + + scripts_dir = project / ".specify" / "scripts" / "bash" + scripts_dir.mkdir(parents=True) + (scripts_dir / "common.sh").write_text("# custom\n", encoding="utf-8") + + _install_shared_infra(project, "sh", force=True) + + captured = capsys.readouterr() + assert "already exist and were not updated" not in captured.out + + def test_init_here_force_overwrites_shared_infra(self, tmp_path): + """E2E: specify init --here --force overwrites shared infra files.""" + from typer.testing import CliRunner + from specify_cli import app + + project = tmp_path / "e2e-force" + project.mkdir() + + scripts_dir = project / ".specify" / "scripts" / "bash" + scripts_dir.mkdir(parents=True) + custom_content = "# user-modified common.sh\n" + (scripts_dir / "common.sh").write_text(custom_content, encoding="utf-8") + old_cwd = os.getcwd() try: os.chdir(project) @@ -207,14 +295,40 @@ def test_shared_infra_skips_existing_files(self, tmp_path): os.chdir(old_cwd) assert result.exit_code == 0 + # --force should overwrite the custom file + assert (scripts_dir / "common.sh").read_text(encoding="utf-8") != custom_content - # User's files should be preserved - assert (scripts_dir / "common.sh").read_text(encoding="utf-8") == custom_content - assert (templates_dir / "spec-template.md").read_text(encoding="utf-8") == custom_template + def test_init_here_without_force_preserves_shared_infra(self, tmp_path): + """E2E: specify init --here (no --force) preserves existing shared infra files.""" + from typer.testing import CliRunner + from specify_cli import app - # Other shared files should still be installed - assert (scripts_dir / "setup-plan.sh").exists() - assert (templates_dir / "plan-template.md").exists() + project = tmp_path / "e2e-no-force" + project.mkdir() + + scripts_dir = project / ".specify" / "scripts" / "bash" + scripts_dir.mkdir(parents=True) + custom_content = "# user-modified common.sh\n" + (scripts_dir / "common.sh").write_text(custom_content, encoding="utf-8") + + old_cwd = os.getcwd() + try: + os.chdir(project) + runner = CliRunner() + result = runner.invoke(app, [ + "init", "--here", + "--integration", "copilot", + "--script", "sh", + "--no-git", + ], input="y\n", catch_exceptions=False) + finally: + os.chdir(old_cwd) + + assert result.exit_code == 0 + # Without --force, custom file should be preserved + assert (scripts_dir / "common.sh").read_text(encoding="utf-8") == custom_content + # Warning about skipped files should appear + assert "not updated" in result.output class TestForceExistingDirectory: From 0b3d55edddcb8343a496726ec9c26ed9777ae4cd Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 16:24:34 -0500 Subject: [PATCH 2/2] fix: improve skip warning to suggest specific commands Address review feedback: the generic '--force' suggestion was misleading when _install_shared_infra is called from integration install/switch (which don't have a --force for shared infra). Now points users to the specific commands that can refresh shared infra: 'specify init --here --force' or 'specify integration upgrade --force'. --- src/specify_cli/__init__.py | 4 +++- tests/integrations/test_cli.py | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index b03384eb4a..db84257ce0 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -789,7 +789,9 @@ def _install_shared_infra( for f in skipped_files: console.print(f" {f}") console.print( - "Use [cyan]--force[/cyan] to overwrite with the latest versions." + "To refresh shared infrastructure, run " + "[cyan]specify init --here --force[/cyan] or " + "[cyan]specify integration upgrade --force[/cyan]." ) manifest.save() diff --git a/tests/integrations/test_cli.py b/tests/integrations/test_cli.py index c02924a620..9672ab76bf 100644 --- a/tests/integrations/test_cli.py +++ b/tests/integrations/test_cli.py @@ -249,7 +249,10 @@ def test_shared_infra_skip_warning_displayed(self, tmp_path, capsys): captured = capsys.readouterr() assert "already exist and were not updated" in captured.out - assert "--force" in captured.out + assert "specify init --here --force" in captured.out + # Rich may wrap long lines; normalize whitespace for the second command + normalized = " ".join(captured.out.split()) + assert "specify integration upgrade --force" in normalized def test_shared_infra_no_warning_when_forced(self, tmp_path, capsys): """No skip warning when force=True (all files overwritten)."""