From 0487a9a7fd2b08b4a3f52588de7921862f27c1b8 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 1 Jul 2026 18:12:25 +0000 Subject: [PATCH 1/4] Fix preset-constitution-not-installed: use PresetResolver in constitution setup Apply the remediation from the bug assessment on issue #3272. Changes: 1. Modify ensure_constitution_from_template (init.py) to resolve the constitution-template through the preset priority stack via PresetResolver, instead of hardcoding the core template path. This ensures a preset's replacement constitution-template is used when seeding .specify/memory/constitution.md. 2. Reorder init flow: move ensure_constitution_from_template to after the preset installation block so that 'specify init --preset' seeds the memory file from the already-resolved template stack, not from the generic template that existed before the preset arrived. 3. Add _maybe_reseed_constitution to PresetManager (presets/__init__.py): a post-install hook that re-seeds .specify/memory/constitution.md from the preset's constitution-template during 'specify preset add' on an existing project, but only when the memory file still contains generic placeholder tokens ([PROJECT_NAME] or [PRINCIPLE_1_NAME]). Legitimately authored constitutions (no placeholder tokens) are never overwritten. 4. Add regression tests covering both code paths (TestConstitutionReseedOnPresetInstall and TestEnsureConstitutionFromTemplate in tests/test_presets.py). Refs #3272 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/commands/init.py | 28 ++++- src/specify_cli/presets/__init__.py | 48 ++++++++ tests/test_presets.py | 185 ++++++++++++++++++++++++++++ 3 files changed, 255 insertions(+), 6 deletions(-) diff --git a/src/specify_cli/commands/init.py b/src/specify_cli/commands/init.py index dd815b8c5d..fc0ad126cb 100644 --- a/src/specify_cli/commands/init.py +++ b/src/specify_cli/commands/init.py @@ -33,11 +33,12 @@ def _stdin_is_interactive() -> bool: def ensure_constitution_from_template( project_path: Path, tracker: StepTracker | None = None ) -> None: - """Copy constitution template to memory if it doesn't exist.""" + """Copy constitution template to memory if it doesn't exist. + + Resolves the template through the preset priority stack so that a preset's + replacement constitution-template is used instead of the generic core file. + """ memory_constitution = project_path / ".specify" / "memory" / "constitution.md" - template_constitution = ( - project_path / ".specify" / "templates" / "constitution-template.md" - ) if memory_constitution.exists(): if tracker: @@ -45,6 +46,21 @@ def ensure_constitution_from_template( tracker.skip("constitution", "existing file preserved") return + # Resolve through the preset priority stack so preset replacements are honoured. + template_constitution: Path | None = None + try: + from ..presets import PresetResolver as _PresetResolver + template_constitution = _PresetResolver(project_path).resolve( + "constitution-template", "template" + ) + except Exception: + template_constitution = None + + if template_constitution is None: + template_constitution = ( + project_path / ".specify" / "templates" / "constitution-template.md" + ) + if not template_constitution.exists(): if tracker: tracker.add("constitution", "Constitution setup") @@ -447,8 +463,6 @@ def init( "shared-infra", f"scripts ({selected_script}) + templates" ) - ensure_constitution_from_template(project_path, tracker=tracker) - try: bundled_wf = _locate_bundled_workflow("speckit") if bundled_wf: @@ -576,6 +590,8 @@ def init( continuing="Continuing without the optional preset.", ) + ensure_constitution_from_template(project_path, tracker=tracker) + tracker.complete("final", "project ready") except (typer.Exit, SystemExit): raise diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 863b6ef7dc..948ce685d9 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -1615,8 +1615,56 @@ def install_from_directory( stacklevel=2, ) + # Re-seed constitution memory if the preset provides a replacement + # constitution-template and the existing memory file is still a generic placeholder. + self._maybe_reseed_constitution(manifest) + return manifest + def _maybe_reseed_constitution(self, manifest: "PresetManifest") -> None: + """Re-seed .specify/memory/constitution.md from a preset's constitution-template. + + Only re-seeds when: + 1. The preset provides a ``constitution-template`` template entry. + 2. The memory file exists and still contains generic placeholder tokens + (``[PROJECT_NAME]`` or ``[PRINCIPLE_1_NAME]``), meaning it has not + been legitimately authored yet. + + This handles ``specify preset add`` on an existing project so the + preset's ratified constitution lands in memory without requiring + ``ensure_constitution_from_template`` to run again. + """ + has_constitution_template = any( + t.get("type") == "template" and t.get("name") == "constitution-template" + for t in manifest.templates + ) + if not has_constitution_template: + return + + memory_constitution = self.project_root / ".specify" / "memory" / "constitution.md" + if not memory_constitution.exists(): + # Will be seeded by ensure_constitution_from_template later; nothing to do here. + return + + try: + content = memory_constitution.read_text(encoding="utf-8") + except OSError: + return + + # Skip re-seed if the file has been legitimately authored (no placeholder tokens). + if "[PROJECT_NAME]" not in content and "[PRINCIPLE_1_NAME]" not in content: + return + + resolver = PresetResolver(self.project_root) + resolved = resolver.resolve("constitution-template", "template") + if resolved is None or not resolved.exists(): + return + + try: + shutil.copy2(resolved, memory_constitution) + except OSError: + pass # best-effort; don't fail preset installation for this + def install_from_zip( self, zip_path: Path, diff --git a/tests/test_presets.py b/tests/test_presets.py index 054018b7a0..44b83bd15b 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -6149,3 +6149,188 @@ def fake_open(url, timeout=None, extra_headers=None): ) assert resolved == "https://ghes.example/api/v3/repos/o/r/releases/assets/9" assert captured == ["https://ghes.example/api/v3/repos/o/r/releases/tags/v2"] + + +# ============================================================================= +# Tests for preset constitution re-seed (issue #3272) +# ============================================================================= + + +class TestConstitutionReseedOnPresetInstall: + """Tests for _maybe_reseed_constitution and the post-install re-seed hook. + + Verifies that install_from_directory re-seeds .specify/memory/constitution.md + from the preset's constitution-template when the memory file still contains + generic placeholder tokens, and does NOT overwrite legitimately authored ones. + """ + + def test_install_reseeds_generic_constitution_memory(self, project_dir): + """install_from_directory re-seeds memory/constitution.md when it still + contains the generic [PROJECT_NAME] placeholder and the preset provides a + constitution-template replacement.""" + memory_dir = project_dir / ".specify" / "memory" + memory_dir.mkdir(parents=True, exist_ok=True) + memory_constitution = memory_dir / "constitution.md" + memory_constitution.write_text( + "# [PROJECT_NAME] Constitution\n\n## Core Principles\n\n" + "### [PRINCIPLE_1_NAME]\n[PRINCIPLE_1_DESCRIPTION]\n" + ) + + manager = PresetManager(project_dir) + install_self_test_preset(manager) + + result = memory_constitution.read_text() + assert "preset:self-test" in result, "Expected preset's constitution content after re-seed" + assert "[PROJECT_NAME]" not in result + + def test_install_reseeds_on_principle_placeholder(self, project_dir): + """Re-seeds when [PRINCIPLE_1_NAME] is present even if [PROJECT_NAME] is absent.""" + memory_dir = project_dir / ".specify" / "memory" + memory_dir.mkdir(parents=True, exist_ok=True) + memory_constitution = memory_dir / "constitution.md" + memory_constitution.write_text( + "# My Company Constitution\n\n### [PRINCIPLE_1_NAME]\n[PRINCIPLE_1_DESCRIPTION]\n" + ) + + manager = PresetManager(project_dir) + install_self_test_preset(manager) + + result = memory_constitution.read_text() + assert "preset:self-test" in result + + def test_install_does_not_overwrite_authored_constitution(self, project_dir): + """install_from_directory must NOT overwrite a legitimately authored + constitution (one that no longer contains placeholder tokens).""" + authored_content = "# Acme Corp Constitution\n\n## Values\n\n1. Build great things.\n" + + memory_dir = project_dir / ".specify" / "memory" + memory_dir.mkdir(parents=True, exist_ok=True) + memory_constitution = memory_dir / "constitution.md" + memory_constitution.write_text(authored_content) + + manager = PresetManager(project_dir) + install_self_test_preset(manager) + + assert memory_constitution.read_text() == authored_content, ( + "install_from_directory must not overwrite an authored constitution" + ) + + def test_install_skips_reseed_when_no_constitution_template(self, project_dir, temp_dir): + """install_from_directory does not touch memory/constitution.md when the + preset provides no constitution-template entry.""" + pack_data = { + "schema_version": "1.0", + "preset": { + "id": "no-constitution", + "name": "No Constitution Preset", + "version": "1.0.0", + "description": "A preset without a constitution-template override", + "author": "Test", + "license": "MIT", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [ + { + "type": "template", + "name": "spec-template", + "file": "templates/spec-template.md", + "replaces": "spec-template", + } + ] + }, + } + pack_dir = temp_dir / "no-constitution" + pack_dir.mkdir() + (pack_dir / "preset.yml").write_text(yaml.dump(pack_data)) + (pack_dir / "templates").mkdir() + (pack_dir / "templates" / "spec-template.md").write_text("# Custom Spec\n") + + placeholder_content = "# [PROJECT_NAME] Constitution\n" + memory_dir = project_dir / ".specify" / "memory" + memory_dir.mkdir(parents=True, exist_ok=True) + memory_constitution = memory_dir / "constitution.md" + memory_constitution.write_text(placeholder_content) + + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.5") + + assert memory_constitution.read_text() == placeholder_content, ( + "Memory constitution must be unchanged when preset has no constitution-template" + ) + + def test_install_skips_reseed_when_memory_absent(self, project_dir): + """install_from_directory must not create memory/constitution.md — that + responsibility belongs to ensure_constitution_from_template.""" + manager = PresetManager(project_dir) + install_self_test_preset(manager) + + memory_constitution = project_dir / ".specify" / "memory" / "constitution.md" + assert not memory_constitution.exists(), ( + "install_from_directory must not create memory/constitution.md" + ) + + +class TestEnsureConstitutionFromTemplate: + """Tests for ensure_constitution_from_template using the preset resolver. + + Verifies that the function seeds .specify/memory/constitution.md from the + highest-priority resolved constitution-template, picking up preset overrides + instead of always copying the generic core file. + """ + + def test_uses_preset_constitution_when_available(self, project_dir): + """ensure_constitution_from_template seeds from the preset's constitution-template + rather than the generic core file when a replacement preset is installed.""" + from specify_cli.commands.init import ensure_constitution_from_template + + manager = PresetManager(project_dir) + install_self_test_preset(manager) + + # Provide a core template with generic content so we can confirm the + # function chose the preset's version over the core one. + core_tpl = project_dir / ".specify" / "templates" / "constitution-template.md" + core_tpl.write_text("# [PROJECT_NAME] Constitution\n\n[PRINCIPLE_1_NAME]\n") + + ensure_constitution_from_template(project_dir) + + memory_constitution = project_dir / ".specify" / "memory" / "constitution.md" + assert memory_constitution.exists() + content = memory_constitution.read_text() + assert "preset:self-test" in content, ( + "Expected preset's constitution, got core template" + ) + assert "[PROJECT_NAME]" not in content + + def test_falls_back_to_core_template_without_preset(self, project_dir): + """ensure_constitution_from_template falls back to the core template when + no preset overrides constitution-template.""" + from specify_cli.commands.init import ensure_constitution_from_template + + core_tpl = project_dir / ".specify" / "templates" / "constitution-template.md" + core_tpl.write_text("# [PROJECT_NAME] Constitution\n# Core template marker\n") + + ensure_constitution_from_template(project_dir) + + memory_constitution = project_dir / ".specify" / "memory" / "constitution.md" + assert memory_constitution.exists() + assert "Core template marker" in memory_constitution.read_text() + + def test_skips_when_memory_already_exists(self, project_dir): + """ensure_constitution_from_template is a no-op when memory/constitution.md + already exists, even if a preset replacement is available.""" + from specify_cli.commands.init import ensure_constitution_from_template + + memory_dir = project_dir / ".specify" / "memory" + memory_dir.mkdir(parents=True, exist_ok=True) + existing = memory_dir / "constitution.md" + existing.write_text("# Existing authored constitution\n") + + manager = PresetManager(project_dir) + install_self_test_preset(manager) + + ensure_constitution_from_template(project_dir) + + assert existing.read_text() == "# Existing authored constitution\n", ( + "ensure_constitution_from_template must not overwrite existing memory file" + ) From e601a6ed2bf1c71690325e458b31e0e4b549ddb5 Mon Sep 17 00:00:00 2001 From: Ben Buttigieg <70525+BenBtg@users.noreply.github.com> Date: Wed, 15 Jul 2026 15:07:42 +0100 Subject: [PATCH 2/4] Harden preset constitution resolution Use manifest-aware composed content, atomic safe writes, and conservative generic-template matching for constitution seeding and re-seeding. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 49891a32-bec4-462c-a7f2-6d6ec4eefcdb --- src/specify_cli/commands/init.py | 34 ++++--- src/specify_cli/presets/__init__.py | 87 ++++++++++++++++-- tests/integrations/test_cli.py | 51 ++++++++++ tests/test_presets.py | 138 ++++++++++++++++++++++++++-- 4 files changed, 278 insertions(+), 32 deletions(-) diff --git a/src/specify_cli/commands/init.py b/src/specify_cli/commands/init.py index fc0ad126cb..a3dfad9acc 100644 --- a/src/specify_cli/commands/init.py +++ b/src/specify_cli/commands/init.py @@ -24,6 +24,7 @@ ) from .._console import StepTracker, console, select_with_arrows, show_banner from .._utils import check_tool +from ..shared_infra import _ensure_safe_shared_directory, _write_shared_text def _stdin_is_interactive() -> bool: @@ -46,30 +47,35 @@ def ensure_constitution_from_template( tracker.skip("constitution", "existing file preserved") return - # Resolve through the preset priority stack so preset replacements are honoured. - template_constitution: Path | None = None + from ..presets import PresetResolver + try: - from ..presets import PresetResolver as _PresetResolver - template_constitution = _PresetResolver(project_path).resolve( + constitution_content = PresetResolver(project_path).resolve_content( "constitution-template", "template" ) - except Exception: - template_constitution = None - - if template_constitution is None: - template_constitution = ( - project_path / ".specify" / "templates" / "constitution-template.md" - ) + except (OSError, UnicodeError, ValueError) as exc: + if tracker: + tracker.add("constitution", "Constitution setup") + tracker.error("constitution", str(exc)) + else: + console.print( + f"[yellow]Warning: Could not resolve constitution template: {exc}[/yellow]" + ) + return - if not template_constitution.exists(): + if constitution_content is None: if tracker: tracker.add("constitution", "Constitution setup") tracker.error("constitution", "template not found") return try: - memory_constitution.parent.mkdir(parents=True, exist_ok=True) - shutil.copy2(template_constitution, memory_constitution) + _ensure_safe_shared_directory( + project_path, + memory_constitution.parent, + context="constitution memory directory", + ) + _write_shared_text(project_path, memory_constitution, constitution_content) if tracker: tracker.add("constitution", "Constitution setup") tracker.complete("constitution", "copied from template") diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 948ce685d9..fb84d84e63 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -31,7 +31,11 @@ from .._init_options import is_ai_skills_enabled from ..integrations.base import IntegrationBase from .._utils import dump_frontmatter, version_satisfies -from ..shared_infra import verify_archive_sha256 +from ..shared_infra import ( + _ensure_safe_shared_destination, + _write_shared_text, + verify_archive_sha256, +) def _substitute_core_template( @@ -1642,28 +1646,93 @@ def _maybe_reseed_constitution(self, manifest: "PresetManifest") -> None: return memory_constitution = self.project_root / ".specify" / "memory" / "constitution.md" + try: + _ensure_safe_shared_destination( + self.project_root, + memory_constitution, + parent_must_exist=False, + ) + except ValueError as exc: + import warnings + warnings.warn( + f"Could not inspect constitution after installing {manifest.id}: " + f"{exc}. The existing constitution was preserved.", + stacklevel=2, + ) + return + if not memory_constitution.exists(): # Will be seeded by ensure_constitution_from_template later; nothing to do here. return try: content = memory_constitution.read_text(encoding="utf-8") - except OSError: + except (OSError, UnicodeError) as exc: + import warnings + warnings.warn( + f"Could not inspect constitution after installing {manifest.id}: " + f"{exc}. The existing constitution was preserved.", + stacklevel=2, + ) return - # Skip re-seed if the file has been legitimately authored (no placeholder tokens). + # A remaining placeholder can occur in partially authored content, so only + # replace a file that still exactly matches the installed generic template. if "[PROJECT_NAME]" not in content and "[PRINCIPLE_1_NAME]" not in content: return - resolver = PresetResolver(self.project_root) - resolved = resolver.resolve("constitution-template", "template") - if resolved is None or not resolved.exists(): + core_constitution = ( + self.project_root / ".specify" / "templates" / "constitution-template.md" + ) + try: + _ensure_safe_shared_destination( + self.project_root, core_constitution + ) + core_content = core_constitution.read_text(encoding="utf-8") + except (OSError, UnicodeError, ValueError) as exc: + import warnings + warnings.warn( + f"Could not inspect core constitution after installing {manifest.id}: " + f"{exc}. The existing constitution was preserved.", + stacklevel=2, + ) + return + + if content != core_content: return try: - shutil.copy2(resolved, memory_constitution) - except OSError: - pass # best-effort; don't fail preset installation for this + resolved_content = PresetResolver(self.project_root).resolve_content( + "constitution-template", "template" + ) + except ( + OSError, + UnicodeError, + ValueError, + PresetValidationError, + ) as exc: + import warnings + warnings.warn( + f"Could not resolve constitution template after installing {manifest.id}: " + f"{exc}. The existing constitution was preserved.", + stacklevel=2, + ) + return + + if resolved_content is None: + return + + try: + _write_shared_text( + self.project_root, memory_constitution, resolved_content + ) + except (OSError, ValueError) as exc: + import warnings + warnings.warn( + f"Could not re-seed constitution after installing {manifest.id}: " + f"{exc}. The existing constitution was preserved.", + stacklevel=2, + ) def install_from_zip( self, diff --git a/tests/integrations/test_cli.py b/tests/integrations/test_cli.py index 25d4a7c16a..77e5b6cfb1 100644 --- a/tests/integrations/test_cli.py +++ b/tests/integrations/test_cli.py @@ -167,6 +167,57 @@ def fail_install(self, path, version): assert "Continuing without the optional preset" in normalized assert "Project ready" in normalized + def test_init_with_local_preset_seeds_manifest_constitution(self, tmp_path): + from typer.testing import CliRunner + from specify_cli import app + + preset_dir = tmp_path / "constitution-preset" + (preset_dir / "organization").mkdir(parents=True) + preset_content = "# Ratified Organization Constitution\n" + (preset_dir / "organization" / "ratified.md").write_text(preset_content) + (preset_dir / "preset.yml").write_text( + yaml.safe_dump({ + "schema_version": "1.0", + "preset": { + "id": "constitution-preset", + "name": "Constitution Preset", + "version": "1.0.0", + "description": "Provides a ratified constitution", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [{ + "type": "template", + "name": "constitution-template", + "file": "organization/ratified.md", + "strategy": "replace", + }] + }, + }) + ) + project = tmp_path / "init-with-preset" + + result = CliRunner().invoke( + app, + [ + "init", + str(project), + "--integration", + "copilot", + "--script", + "sh", + "--ignore-agent-tools", + "--preset", + str(preset_dir), + ], + catch_exceptions=False, + ) + + assert result.exit_code == 0, result.output + assert ( + project / ".specify" / "memory" / "constitution.md" + ).read_text() == preset_content + def test_integration_claude_here_preserves_preexisting_commands(self, tmp_path): from typer.testing import CliRunner from specify_cli import app diff --git a/tests/test_presets.py b/tests/test_presets.py index 44b83bd15b..979952f824 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -6168,13 +6168,17 @@ def test_install_reseeds_generic_constitution_memory(self, project_dir): """install_from_directory re-seeds memory/constitution.md when it still contains the generic [PROJECT_NAME] placeholder and the preset provides a constitution-template replacement.""" - memory_dir = project_dir / ".specify" / "memory" - memory_dir.mkdir(parents=True, exist_ok=True) - memory_constitution = memory_dir / "constitution.md" - memory_constitution.write_text( + generic_content = ( "# [PROJECT_NAME] Constitution\n\n## Core Principles\n\n" "### [PRINCIPLE_1_NAME]\n[PRINCIPLE_1_DESCRIPTION]\n" ) + ( + project_dir / ".specify" / "templates" / "constitution-template.md" + ).write_text(generic_content) + memory_dir = project_dir / ".specify" / "memory" + memory_dir.mkdir(parents=True, exist_ok=True) + memory_constitution = memory_dir / "constitution.md" + memory_constitution.write_text(generic_content) manager = PresetManager(project_dir) install_self_test_preset(manager) @@ -6183,20 +6187,23 @@ def test_install_reseeds_generic_constitution_memory(self, project_dir): assert "preset:self-test" in result, "Expected preset's constitution content after re-seed" assert "[PROJECT_NAME]" not in result - def test_install_reseeds_on_principle_placeholder(self, project_dir): - """Re-seeds when [PRINCIPLE_1_NAME] is present even if [PROJECT_NAME] is absent.""" + def test_install_preserves_partially_authored_constitution(self, project_dir): + """A remaining placeholder does not make partially authored content generic.""" memory_dir = project_dir / ".specify" / "memory" memory_dir.mkdir(parents=True, exist_ok=True) memory_constitution = memory_dir / "constitution.md" - memory_constitution.write_text( + partial_content = ( "# My Company Constitution\n\n### [PRINCIPLE_1_NAME]\n[PRINCIPLE_1_DESCRIPTION]\n" ) + memory_constitution.write_text(partial_content) + ( + project_dir / ".specify" / "templates" / "constitution-template.md" + ).write_text("# [PROJECT_NAME] Constitution\n") manager = PresetManager(project_dir) install_self_test_preset(manager) - result = memory_constitution.read_text() - assert "preset:self-test" in result + assert memory_constitution.read_text() == partial_content def test_install_does_not_overwrite_authored_constitution(self, project_dir): """install_from_directory must NOT overwrite a legitimately authored @@ -6270,6 +6277,119 @@ def test_install_skips_reseed_when_memory_absent(self, project_dir): "install_from_directory must not create memory/constitution.md" ) + def test_install_composes_constitution_strategy(self, project_dir, temp_dir): + """Re-seeding uses the effective composed template, not a raw preset fragment.""" + core_content = "# [PROJECT_NAME] Constitution\n" + (project_dir / ".specify" / "templates" / "constitution-template.md").write_text( + core_content + ) + memory_constitution = project_dir / ".specify" / "memory" / "constitution.md" + memory_constitution.parent.mkdir(parents=True) + memory_constitution.write_text(core_content) + + pack_dir = temp_dir / "append-constitution" + (pack_dir / "fragments").mkdir(parents=True) + (pack_dir / "fragments" / "governance.md").write_text("\n## Governance\nBe clear.\n") + (pack_dir / "preset.yml").write_text( + yaml.safe_dump({ + "schema_version": "1.0", + "preset": { + "id": "append-constitution", + "name": "Append Constitution", + "version": "1.0.0", + "description": "Appends governance rules", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [{ + "type": "template", + "name": "constitution-template", + "file": "fragments/governance.md", + "strategy": "append", + }] + }, + }) + ) + + PresetManager(project_dir).install_from_directory(pack_dir, "0.1.5") + + composed = memory_constitution.read_text() + assert composed.startswith(core_content) + assert composed.index("## Governance") > composed.index("[PROJECT_NAME]") + assert "Be clear." in composed + + def test_install_preserves_symlinked_constitution(self, project_dir): + """Re-seeding must not follow a memory-file symlink outside the project.""" + outside = project_dir.parent / "outside-constitution.md" + original = "# [PROJECT_NAME] Outside\n" + outside.write_text(original) + memory_constitution = project_dir / ".specify" / "memory" / "constitution.md" + memory_constitution.parent.mkdir(parents=True) + memory_constitution.symlink_to(outside) + + with pytest.warns(UserWarning, match="Could not inspect constitution"): + install_self_test_preset(PresetManager(project_dir)) + + assert outside.read_text() == original + assert memory_constitution.is_symlink() + + def test_install_preserves_non_utf8_constitution(self, project_dir): + """A decoding failure must not turn a completed preset install into an error.""" + memory_constitution = project_dir / ".specify" / "memory" / "constitution.md" + memory_constitution.parent.mkdir(parents=True) + original = b"\xff\xfe" + memory_constitution.write_bytes(original) + + with pytest.warns(UserWarning, match="Could not inspect constitution"): + manifest = install_self_test_preset(PresetManager(project_dir)) + + assert manifest.id == "self-test" + assert memory_constitution.read_bytes() == original + + def test_install_warns_when_constitution_composition_is_invalid( + self, project_dir, temp_dir + ): + """Invalid composition does not turn a completed preset install into failure.""" + generic_content = "# [PROJECT_NAME] Constitution\n" + ( + project_dir / ".specify" / "templates" / "constitution-template.md" + ).write_text(generic_content) + memory_constitution = project_dir / ".specify" / "memory" / "constitution.md" + memory_constitution.parent.mkdir(parents=True) + memory_constitution.write_text(generic_content) + + pack_dir = temp_dir / "invalid-wrap" + (pack_dir / "templates").mkdir(parents=True) + (pack_dir / "templates" / "wrapper.md").write_text("# Missing placeholder\n") + (pack_dir / "preset.yml").write_text( + yaml.safe_dump({ + "schema_version": "1.0", + "preset": { + "id": "invalid-wrap", + "name": "Invalid Wrap", + "version": "1.0.0", + "description": "Invalid constitution wrapper", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [{ + "type": "template", + "name": "constitution-template", + "file": "templates/wrapper.md", + "strategy": "wrap", + }] + }, + }) + ) + + with pytest.warns(UserWarning, match="Could not resolve constitution template"): + manifest = PresetManager(project_dir).install_from_directory( + pack_dir, "0.1.5" + ) + + assert manifest.id == "invalid-wrap" + assert memory_constitution.read_text() == generic_content + class TestEnsureConstitutionFromTemplate: """Tests for ensure_constitution_from_template using the preset resolver. From 84b8f7c78101a3624d24528911e3f00f41c5ffce Mon Sep 17 00:00:00 2001 From: Ben Buttigieg <70525+BenBtg@users.noreply.github.com> Date: Wed, 15 Jul 2026 17:23:50 +0100 Subject: [PATCH 3/4] Limit preset CLI change to regression test Remove accidental whole-file Ruff formatting introduced during conflict resolution so the PR contains only the intended end-to-end test. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 49891a32-bec4-462c-a7f2-6d6ec4eefcdb --- tests/integrations/test_cli.py | 634 ++++++++++----------------------- 1 file changed, 198 insertions(+), 436 deletions(-) diff --git a/tests/integrations/test_cli.py b/tests/integrations/test_cli.py index 551c4a2ed4..a428968c05 100644 --- a/tests/integrations/test_cli.py +++ b/tests/integrations/test_cli.py @@ -26,10 +26,7 @@ class TestCliDiagnosticFormatting: def test_cli_error_detail_flattens_newlines(self): import specify_cli - assert ( - specify_cli._cli_error_detail(RuntimeError("line one\nline two")) - == "line one line two" - ) + assert specify_cli._cli_error_detail(RuntimeError("line one\nline two")) == "line one line two" def test_cli_error_detail_handles_empty_message(self): import specify_cli @@ -49,42 +46,25 @@ class TestInitIntegrationFlag: def test_unknown_integration_rejected(self, tmp_path): from typer.testing import CliRunner from specify_cli import app - runner = CliRunner() - result = runner.invoke( - app, - [ - "init", - str(tmp_path / "test-project"), - "--integration", - "nonexistent", - ], - ) + result = runner.invoke(app, [ + "init", str(tmp_path / "test-project"), "--integration", "nonexistent", + ]) assert result.exit_code != 0 assert "Unknown integration" in result.output def test_integration_copilot_creates_files(self, tmp_path): from typer.testing import CliRunner from specify_cli import app - runner = CliRunner() project = tmp_path / "int-test" project.mkdir() old_cwd = os.getcwd() try: os.chdir(project) - result = runner.invoke( - app, - [ - "init", - "--here", - "--integration", - "copilot", - "--script", - "sh", - ], - catch_exceptions=False, - ) + result = runner.invoke(app, [ + "init", "--here", "--integration", "copilot", "--script", "sh", + ], catch_exceptions=False) finally: os.chdir(old_cwd) assert result.exit_code == 0, f"init failed: {result.output}" @@ -92,40 +72,24 @@ def test_integration_copilot_creates_files(self, tmp_path): assert (project / ".github" / "prompts" / "speckit.plan.prompt.md").exists() assert (project / ".specify" / "scripts" / "bash" / "common.sh").exists() - data = json.loads( - (project / ".specify" / "integration.json").read_text(encoding="utf-8") - ) + data = json.loads((project / ".specify" / "integration.json").read_text(encoding="utf-8")) assert data["integration"] == "copilot" - opts = json.loads( - (project / ".specify" / "init-options.json").read_text(encoding="utf-8") - ) + opts = json.loads((project / ".specify" / "init-options.json").read_text(encoding="utf-8")) assert opts["integration"] == "copilot" # init must not leave any legacy agent-context keys in init-options.json assert "context_file" not in opts # agent-context is fully opt-in: init must not install it or write its config - ext_cfg_path = ( - project - / ".specify" - / "extensions" - / "agent-context" - / "agent-context-config.yml" - ) - assert not ext_cfg_path.exists(), ( - "init must not create the agent-context extension config" - ) + ext_cfg_path = project / ".specify" / "extensions" / "agent-context" / "agent-context-config.yml" + assert not ext_cfg_path.exists(), "init must not create the agent-context extension config" - assert ( - project / ".specify" / "integrations" / "copilot.manifest.json" - ).exists() + assert (project / ".specify" / "integrations" / "copilot.manifest.json").exists() # init must not create or manage the agent context file assert not (project / ".github" / "copilot-instructions.md").exists() - shared_manifest = ( - project / ".specify" / "integrations" / "speckit.manifest.json" - ) + shared_manifest = project / ".specify" / "integrations" / "speckit.manifest.json" assert shared_manifest.exists() def test_noninteractive_init_defaults_to_copilot(self, tmp_path, monkeypatch): @@ -134,40 +98,24 @@ def test_noninteractive_init_defaults_to_copilot(self, tmp_path, monkeypatch): import specify_cli def fail_select(*_args, **_kwargs): - raise AssertionError( - "non-interactive init should not open the integration picker" - ) + raise AssertionError("non-interactive init should not open the integration picker") monkeypatch.setattr(specify_cli, "select_with_arrows", fail_select) runner = CliRunner() project = tmp_path / "noninteractive" - result = runner.invoke( - app, - [ - "init", - str(project), - "--script", - "sh", - "--ignore-agent-tools", - ], - catch_exceptions=False, - ) + result = runner.invoke(app, [ + "init", str(project), "--script", "sh", "--ignore-agent-tools", + ], catch_exceptions=False) assert result.exit_code == 0, result.output - assert ( - f"defaulting to '{specify_cli.DEFAULT_INIT_INTEGRATION}'" in result.output - ) + assert f"defaulting to '{specify_cli.DEFAULT_INIT_INTEGRATION}'" in result.output assert (project / ".github" / "agents" / "speckit.plan.agent.md").exists() - data = json.loads( - (project / ".specify" / "integration.json").read_text(encoding="utf-8") - ) + data = json.loads((project / ".specify" / "integration.json").read_text(encoding="utf-8")) assert data["integration"] == specify_cli.DEFAULT_INIT_INTEGRATION - def test_init_here_nonempty_noninteractive_errors_with_force_guidance( - self, tmp_path - ): + def test_init_here_nonempty_noninteractive_errors_with_force_guidance(self, tmp_path): """`init --here` on a non-empty directory with no confirmation input (empty stdin) must fail fast with guidance to use --force, instead of the bare 'Aborted.' from an EOF on typer.confirm. CliRunner with no `input=` provides @@ -182,19 +130,9 @@ def test_init_here_nonempty_noninteractive_errors_with_force_guidance( old_cwd = os.getcwd() try: os.chdir(project) - result = CliRunner().invoke( - app, - [ - "init", - "--here", - "--integration", - "copilot", - "--script", - "sh", - "--ignore-agent-tools", - ], - catch_exceptions=False, - ) + result = CliRunner().invoke(app, [ + "init", "--here", "--integration", "copilot", "--script", "sh", "--ignore-agent-tools", + ], catch_exceptions=False) finally: os.chdir(old_cwd) @@ -223,19 +161,9 @@ def test_init_here_interactive_cancel_exits_zero(self, tmp_path, monkeypatch): try: os.chdir(project) # No input → typer.confirm raises Abort (stands in for Ctrl+C). - result = CliRunner().invoke( - app, - [ - "init", - "--here", - "--integration", - "copilot", - "--script", - "sh", - "--ignore-agent-tools", - ], - catch_exceptions=False, - ) + result = CliRunner().invoke(app, [ + "init", "--here", "--integration", "copilot", "--script", "sh", "--ignore-agent-tools", + ], catch_exceptions=False) finally: os.chdir(old_cwd) @@ -247,25 +175,15 @@ def test_init_here_interactive_cancel_exits_zero(self, tmp_path, monkeypatch): def test_integration_copilot_auto_promotes(self, tmp_path): from typer.testing import CliRunner from specify_cli import app - project = tmp_path / "promote-test" project.mkdir() old_cwd = os.getcwd() try: os.chdir(project) runner = CliRunner() - result = runner.invoke( - app, - [ - "init", - "--here", - "--integration", - "copilot", - "--script", - "sh", - ], - catch_exceptions=False, - ) + result = runner.invoke(app, [ + "init", "--here", "--integration", "copilot", "--script", "sh", + ], catch_exceptions=False) finally: os.chdir(old_cwd) assert result.exit_code == 0 @@ -315,28 +233,24 @@ def test_init_with_local_preset_seeds_manifest_constitution(self, tmp_path): preset_content = "# Ratified Organization Constitution\n" (preset_dir / "organization" / "ratified.md").write_text(preset_content) (preset_dir / "preset.yml").write_text( - yaml.safe_dump( - { - "schema_version": "1.0", - "preset": { - "id": "constitution-preset", - "name": "Constitution Preset", - "version": "1.0.0", - "description": "Provides a ratified constitution", - }, - "requires": {"speckit_version": ">=0.1.0"}, - "provides": { - "templates": [ - { - "type": "template", - "name": "constitution-template", - "file": "organization/ratified.md", - "strategy": "replace", - } - ] - }, - } - ) + yaml.safe_dump({ + "schema_version": "1.0", + "preset": { + "id": "constitution-preset", + "name": "Constitution Preset", + "version": "1.0.0", + "description": "Provides a ratified constitution", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [{ + "type": "template", + "name": "constitution-template", + "file": "organization/ratified.md", + "strategy": "replace", + }] + }, + }) ) project = tmp_path / "init-with-preset" @@ -378,20 +292,9 @@ def test_integration_claude_here_preserves_preexisting_commands(self, tmp_path): try: os.chdir(project) runner = CliRunner() - result = runner.invoke( - app, - [ - "init", - "--here", - "--force", - "--integration", - "claude", - "--script", - "sh", - "--ignore-agent-tools", - ], - catch_exceptions=False, - ) + result = runner.invoke(app, [ + "init", "--here", "--force", "--integration", "claude", "--script", "sh", "--ignore-agent-tools", + ], catch_exceptions=False) finally: os.chdir(old_cwd) @@ -420,17 +323,13 @@ def test_shared_infra_skips_existing_files_without_force(self, tmp_path): 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" - ) + (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 + 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() @@ -454,17 +353,13 @@ def test_shared_infra_overwrites_existing_files_with_force(self, tmp_path): 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" - ) + (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 + 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() @@ -478,7 +373,9 @@ def test_shared_infra_installs_python_scripts_for_py(self, tmp_path): _install_shared_infra(project, "py") - assert (project / ".specify" / "scripts" / "python" / "common.py").exists() + assert ( + project / ".specify" / "scripts" / "python" / "common.py" + ).exists() def test_shared_infra_removes_stale_managed_script(self, tmp_path): """A managed script the core no longer ships (e.g. the legacy @@ -496,9 +393,7 @@ def test_shared_infra_removes_stale_managed_script(self, tmp_path): # Legacy orphan the current bundle no longer ships, recorded in the # manifest as a managed file (hash matches on disk) — a pre-refactor install. stale_rel = ".specify/scripts/bash/update-agent-context.sh" - (scripts_dir / "update-agent-context.sh").write_text( - "# legacy orphan\n", encoding="utf-8" - ) + (scripts_dir / "update-agent-context.sh").write_text("# legacy orphan\n", encoding="utf-8") manifest = IntegrationManifest("speckit", project, version="test") manifest.record_existing(stale_rel) manifest.save() @@ -566,9 +461,7 @@ def test_shared_infra_prunes_orphan_manifest_entry_when_file_absent(self, tmp_pa refreshed = IntegrationManifest.load("speckit", project) assert stale_rel not in refreshed.files - def test_shared_infra_empty_script_source_keeps_tracked_scripts( - self, tmp_path, monkeypatch - ): + def test_shared_infra_empty_script_source_keeps_tracked_scripts(self, tmp_path, monkeypatch): """If the bundle's script source dir exists but is empty, stale-cleanup must NOT run (no source files seen → can't tell what's obsolete): a previously-tracked script is preserved, never mass-deleted (#3076 review).""" @@ -578,9 +471,7 @@ def test_shared_infra_empty_script_source_keeps_tracked_scripts( # Point the script source at an empty ``bash/`` directory. empty_src = tmp_path / "empty-bundle" / "scripts" (empty_src / "bash").mkdir(parents=True) - monkeypatch.setattr( - shared_infra, "shared_scripts_source", lambda **kw: empty_src - ) + monkeypatch.setattr(shared_infra, "shared_scripts_source", lambda **kw: empty_src) project = tmp_path / "empty-source" project.mkdir() @@ -625,13 +516,11 @@ def test_shared_infra_stale_cleanup_ignores_unsafe_manifest_keys(self, tmp_path) # — stale-cleanup would consider it managed and unlink the target. traversal_key = ".specify/scripts/bash/../keep-me.sh" (manifest_dir / "speckit.manifest.json").write_text( - json.dumps( - { - "integration": "speckit", - "version": "test", - "files": {traversal_key: hashlib.sha256(victim_bytes).hexdigest()}, - } - ), + json.dumps({ + "integration": "speckit", + "version": "test", + "files": {traversal_key: hashlib.sha256(victim_bytes).hexdigest()}, + }), encoding="utf-8", ) @@ -767,9 +656,7 @@ def test_shared_infra_buckets_symlinked_script_destination(self, tmp_path, capsy assert outside.read_text(encoding="utf-8") == "# outside\n" @pytest.mark.skipif(not hasattr(os, "symlink"), reason="symlinks are unavailable") - def test_shared_infra_buckets_symlinked_template_destination( - self, tmp_path, capsys - ): + def test_shared_infra_buckets_symlinked_template_destination(self, tmp_path, capsys): """Symlinked template destinations are bucketed with a warning; the symlink target is preserved.""" from specify_cli import _install_shared_infra @@ -810,9 +697,7 @@ def test_shared_template_refresh_refuses_symlinked_destination(self, tmp_path): assert outside.read_text(encoding="utf-8") == "# outside\n" @pytest.mark.skipif(not hasattr(os, "symlink"), reason="symlinks are unavailable") - def test_shared_infra_refuses_symlinked_specify_directory_before_mkdir( - self, tmp_path - ): + def test_shared_infra_refuses_symlinked_specify_directory_before_mkdir(self, tmp_path): """Shared infra installs must not follow a symlinked .specify directory.""" from specify_cli import _install_shared_infra @@ -899,9 +784,7 @@ def test_shared_template_refresh_preflights_before_writing(self, tmp_path): assert outside.read_text(encoding="utf-8") == "# outside\n" @pytest.mark.skipif(not hasattr(os, "symlink"), reason="symlinks are unavailable") - def test_shared_infra_install_buckets_unsafe_destinations_and_continues( - self, tmp_path - ): + def test_shared_infra_install_buckets_unsafe_destinations_and_continues(self, tmp_path): """Symlinked destinations are bucketed with a warning; safe destinations in the same install still complete.""" from specify_cli.shared_infra import install_shared_infra @@ -973,9 +856,7 @@ def test_shared_infra_skip_warning_uses_posix_paths(self, tmp_path): templates_dest = project / ".specify" / "templates" templates_dest.mkdir(parents=True) - (templates_dest / "plan-template.md").write_text( - "# existing template\n", encoding="utf-8" - ) + (templates_dest / "plan-template.md").write_text("# existing template\n", encoding="utf-8") core_pack = tmp_path / "core-pack" nested_src = core_pack / "scripts" / "bash" / "nested" @@ -984,9 +865,7 @@ def test_shared_infra_skip_warning_uses_posix_paths(self, tmp_path): templates_src = core_pack / "templates" templates_src.mkdir(parents=True) - (templates_src / "plan-template.md").write_text( - "# bundled template\n", encoding="utf-8" - ) + (templates_src / "plan-template.md").write_text("# bundled template\n", encoding="utf-8") buffer = io.StringIO() install_shared_infra( @@ -1003,9 +882,7 @@ def test_shared_infra_skip_warning_uses_posix_paths(self, tmp_path): assert ".specify/scripts/bash/nested/deep.sh" in output assert ".specify/templates/plan-template.md" in output - @pytest.mark.skipif( - os.name == "nt", reason="POSIX mode bits are not stable on Windows" - ) + @pytest.mark.skipif(os.name == "nt", reason="POSIX mode bits are not stable on Windows") def test_shared_template_writes_are_not_world_writable(self, tmp_path): """Shared template writes use a safe default mode instead of chmod 666.""" from specify_cli.shared_infra import install_shared_infra @@ -1065,19 +942,11 @@ def test_init_here_force_overwrites_shared_infra(self, tmp_path): try: os.chdir(project) runner = CliRunner() - result = runner.invoke( - app, - [ - "init", - "--here", - "--force", - "--integration", - "copilot", - "--script", - "sh", - ], - catch_exceptions=False, - ) + result = runner.invoke(app, [ + "init", "--here", "--force", + "--integration", "copilot", + "--script", "sh", + ], catch_exceptions=False) finally: os.chdir(old_cwd) @@ -1103,19 +972,11 @@ def test_init_here_without_force_preserves_shared_infra(self, tmp_path): try: os.chdir(project) runner = CliRunner() - result = runner.invoke( - app, - [ - "init", - "--here", - "--integration", - "copilot", - "--script", - "sh", - ], - input="y\n", - catch_exceptions=False, - ) + result = runner.invoke(app, [ + "init", "--here", + "--integration", "copilot", + "--script", "sh", + ], input="y\n", catch_exceptions=False) finally: os.chdir(old_cwd) @@ -1141,19 +1002,10 @@ def test_force_merges_into_existing_dir(self, tmp_path): marker.write_text("keep me", encoding="utf-8") runner = CliRunner() - result = runner.invoke( - app, - [ - "init", - str(target), - "--integration", - "copilot", - "--force", - "--script", - "sh", - ], - catch_exceptions=False, - ) + result = runner.invoke(app, [ + "init", str(target), "--integration", "copilot", "--force", + "--script", "sh", + ], catch_exceptions=False) assert result.exit_code == 0, f"init --force failed: {result.output}" @@ -1173,18 +1025,10 @@ def test_without_force_errors_on_existing_dir(self, tmp_path): target.mkdir() runner = CliRunner() - result = runner.invoke( - app, - [ - "init", - str(target), - "--integration", - "copilot", - "--script", - "sh", - ], - catch_exceptions=False, - ) + result = runner.invoke(app, [ + "init", str(target), "--integration", "copilot", + "--script", "sh", + ], catch_exceptions=False) assert result.exit_code == 1 assert "already exists" in _normalize_cli_output(result.output) @@ -1204,19 +1048,10 @@ def test_git_extension_not_auto_installed(self, tmp_path): try: os.chdir(project) runner = CliRunner() - result = runner.invoke( - app, - [ - "init", - "--here", - "--integration", - "claude", - "--script", - "sh", - "--ignore-agent-tools", - ], - catch_exceptions=False, - ) + result = runner.invoke(app, [ + "init", "--here", "--integration", "claude", "--script", "sh", + "--ignore-agent-tools", + ], catch_exceptions=False) finally: os.chdir(old_cwd) @@ -1237,27 +1072,15 @@ def test_no_git_flag_is_rejected(self, tmp_path): try: os.chdir(project) runner = CliRunner() - result = runner.invoke( - app, - [ - "init", - "--here", - "--integration", - "claude", - "--script", - "sh", - "--no-git", - "--ignore-agent-tools", - ], - ) + result = runner.invoke(app, [ + "init", "--here", "--integration", "claude", "--script", "sh", + "--no-git", "--ignore-agent-tools", + ]) finally: os.chdir(old_cwd) assert result.exit_code != 0, "--no-git should be rejected as an unknown option" - assert ( - "No such option" in result.output - or "no such option" in result.output.lower() - ) + assert "No such option" in result.output or "no such option" in result.output.lower() def test_git_extension_commands_not_registered_by_default(self, tmp_path): """Git extension commands are NOT registered with the agent during default init.""" @@ -1270,19 +1093,10 @@ def test_git_extension_commands_not_registered_by_default(self, tmp_path): try: os.chdir(project) runner = CliRunner() - result = runner.invoke( - app, - [ - "init", - "--here", - "--integration", - "claude", - "--script", - "sh", - "--ignore-agent-tools", - ], - catch_exceptions=False, - ) + result = runner.invoke(app, [ + "init", "--here", "--integration", "claude", "--script", "sh", + "--ignore-agent-tools", + ], catch_exceptions=False) finally: os.chdir(old_cwd) @@ -1291,12 +1105,8 @@ def test_git_extension_commands_not_registered_by_default(self, tmp_path): # Git extension skill commands should NOT be present claude_skills = project / ".claude" / "skills" assert claude_skills.exists(), "Claude skills directory was not created" - git_skills = [ - f for f in claude_skills.iterdir() if f.name.startswith("speckit-git-") - ] - assert len(git_skills) == 0, ( - "git extension commands should not be registered by default" - ) + git_skills = [f for f in claude_skills.iterdir() if f.name.startswith("speckit-git-")] + assert len(git_skills) == 0, "git extension commands should not be registered by default" class TestSharedInfraCommandRefs: @@ -1331,9 +1141,7 @@ def test_dot_separator_in_page_templates(self, tmp_path): plan = project / ".specify" / "templates" / "plan-template.md" assert plan.exists() content = plan.read_text(encoding="utf-8") - assert "__SPECKIT_COMMAND_" not in content, ( - "unresolved placeholder in plan-template.md" - ) + assert "__SPECKIT_COMMAND_" not in content, "unresolved placeholder in plan-template.md" assert "/speckit.plan" in content checklist = project / ".specify" / "templates" / "checklist-template.md" @@ -1354,13 +1162,9 @@ def test_hyphen_separator_in_page_templates(self, tmp_path): plan = project / ".specify" / "templates" / "plan-template.md" assert plan.exists() content = plan.read_text(encoding="utf-8") - assert "__SPECKIT_COMMAND_" not in content, ( - "unresolved placeholder in plan-template.md" - ) + assert "__SPECKIT_COMMAND_" not in content, "unresolved placeholder in plan-template.md" assert "/speckit-plan" in content - assert "/speckit.plan" not in content, ( - "dot-notation leaked into skills page template" - ) + assert "/speckit.plan" not in content, "dot-notation leaked into skills page template" tasks = project / ".specify" / "templates" / "tasks-template.md" content = tasks.read_text(encoding="utf-8") @@ -1417,19 +1221,12 @@ def test_full_init_claude_resolves_page_templates(self, tmp_path): old_cwd = os.getcwd() try: os.chdir(tmp_path) - result = runner.invoke( - app, - [ - "init", - str(project), - "--integration", - "claude", - "--script", - "sh", - "--ignore-agent-tools", - ], - catch_exceptions=False, - ) + result = runner.invoke(app, [ + "init", str(project), + "--integration", "claude", + "--script", "sh", + "--ignore-agent-tools", + ], catch_exceptions=False) finally: os.chdir(old_cwd) @@ -1454,19 +1251,12 @@ def test_full_init_copilot_resolves_page_templates(self, tmp_path): old_cwd = os.getcwd() try: os.chdir(tmp_path) - result = runner.invoke( - app, - [ - "init", - str(project), - "--integration", - "copilot", - "--script", - "sh", - "--ignore-agent-tools", - ], - catch_exceptions=False, - ) + result = runner.invoke(app, [ + "init", str(project), + "--integration", "copilot", + "--script", "sh", + "--ignore-agent-tools", + ], catch_exceptions=False) finally: os.chdir(old_cwd) @@ -1491,21 +1281,13 @@ def test_full_init_copilot_skills_resolves_page_templates(self, tmp_path): old_cwd = os.getcwd() try: os.chdir(tmp_path) - result = runner.invoke( - app, - [ - "init", - str(project), - "--integration", - "copilot", - "--integration-options", - "--skills", - "--script", - "sh", - "--ignore-agent-tools", - ], - catch_exceptions=False, - ) + result = runner.invoke(app, [ + "init", str(project), + "--integration", "copilot", + "--integration-options", "--skills", + "--script", "sh", + "--ignore-agent-tools", + ], catch_exceptions=False) finally: os.chdir(old_cwd) @@ -1514,9 +1296,7 @@ def test_full_init_copilot_skills_resolves_page_templates(self, tmp_path): plan = project / ".specify" / "templates" / "plan-template.md" content = plan.read_text(encoding="utf-8") assert "/speckit-plan" in content, "Copilot --skills should use /speckit-plan" - assert "/speckit.plan" not in content, ( - "dot-notation leaked into Copilot skills page template" - ) + assert "/speckit.plan" not in content, "dot-notation leaked into Copilot skills page template" assert "__SPECKIT_COMMAND_" not in content script_content = self._combined_script_content(project, "sh") @@ -1565,9 +1345,7 @@ def _patch_catalog(self, monkeypatch, integrations=None): """Return a stubbed `_get_merged_integrations` that yields *integrations*.""" from specify_cli.integrations.catalog import IntegrationCatalog - data = list( - integrations if integrations is not None else self.FAKE_INTEGRATIONS - ) + data = list(integrations if integrations is not None else self.FAKE_INTEGRATIONS) def fake_merged(self, force_refresh=False): return data @@ -1712,18 +1490,13 @@ def test_integration_switch_cleanup_warning_reports_phase_and_targets( def fail_cleanup(self, integration_key): raise OSError("cleanup exploded") - monkeypatch.setattr( - ExtensionManager, "unregister_agent_artifacts", fail_cleanup - ) + monkeypatch.setattr(ExtensionManager, "unregister_agent_artifacts", fail_cleanup) result = self._invoke(["integration", "switch", "claude"], project) normalized = _normalize_cli_output(result.output) assert result.exit_code == 0, result.output - assert ( - "Failed to clean up extension artifacts for integration 'copilot'" - in normalized - ) + assert "Failed to clean up extension artifacts for integration 'copilot'" in normalized assert "cleanup exploded" in normalized assert "Switched to integration" in normalized @@ -1757,7 +1530,9 @@ def test_primary_integration_commands_require_specify_project(self, tmp_path): for command in commands: result = self._invoke(command, project) - failure_context = f"command={command!r}, exit_code={result.exit_code}, output={result.output!r}" + failure_context = ( + f"command={command!r}, exit_code={result.exit_code}, output={result.output!r}" + ) assert result.exit_code == 1, failure_context assert "Not a Spec Kit project" in result.output, failure_context @@ -1792,14 +1567,7 @@ def test_project_scoped_commands_require_specify_directory(self, tmp_path): ["preset", "enable", "demo"], ["preset", "disable", "demo"], ["preset", "catalog", "list"], - [ - "preset", - "catalog", - "add", - "https://example.com/catalog.yml", - "--name", - "demo", - ], + ["preset", "catalog", "add", "https://example.com/catalog.yml", "--name", "demo"], ["preset", "catalog", "remove", "demo"], ["extension", "list"], ["extension", "add", "demo"], @@ -1811,14 +1579,7 @@ def test_project_scoped_commands_require_specify_directory(self, tmp_path): ["extension", "disable", "demo"], ["extension", "set-priority", "demo", "5"], ["extension", "catalog", "list"], - [ - "extension", - "catalog", - "add", - "https://example.com/catalog.yml", - "--name", - "demo", - ], + ["extension", "catalog", "add", "https://example.com/catalog.yml", "--name", "demo"], ["extension", "catalog", "remove", "demo"], ["workflow", "run", "demo"], ["workflow", "resume", "demo"], @@ -1835,24 +1596,20 @@ def test_project_scoped_commands_require_specify_directory(self, tmp_path): for command in commands: result = self._invoke(command, project) - failure_context = f"command={command!r}, exit_code={result.exit_code}, output={result.output!r}" + failure_context = ( + f"command={command!r}, exit_code={result.exit_code}, output={result.output!r}" + ) assert result.exit_code == 1, failure_context assert "Not a Spec Kit project" in result.output, failure_context def test_catalog_config_output_uses_posix_paths(self, tmp_path): project = self._make_project(tmp_path) - preset_add = self._invoke( - [ - "preset", - "catalog", - "add", - "https://example.com/preset-catalog.yml", - "--name", - "demo-presets", - ], - project, - ) + preset_add = self._invoke([ + "preset", "catalog", "add", + "https://example.com/preset-catalog.yml", + "--name", "demo-presets", + ], project) assert preset_add.exit_code == 0, preset_add.output assert "Config saved to .specify/preset-catalogs.yml" in preset_add.output @@ -1860,17 +1617,11 @@ def test_catalog_config_output_uses_posix_paths(self, tmp_path): assert preset_list.exit_code == 0, preset_list.output assert "Config: .specify/preset-catalogs.yml" in preset_list.output - extension_add = self._invoke( - [ - "extension", - "catalog", - "add", - "https://example.com/extension-catalog.yml", - "--name", - "demo-extensions", - ], - project, - ) + extension_add = self._invoke([ + "extension", "catalog", "add", + "https://example.com/extension-catalog.yml", + "--name", "demo-extensions", + ], project) assert extension_add.exit_code == 0, extension_add.output assert "Config saved to .specify/extension-catalogs.yml" in extension_add.output @@ -1883,17 +1634,11 @@ def test_extension_catalog_add_rejects_non_mapping_config_root(self, tmp_path): cfg_path = project / ".specify" / "extension-catalogs.yml" cfg_path.write_text("- not\n- a\n- mapping\n", encoding="utf-8") - result = self._invoke( - [ - "extension", - "catalog", - "add", - "https://example.com/extension-catalog.yml", - "--name", - "demo-extensions", - ], - project, - ) + result = self._invoke([ + "extension", "catalog", "add", + "https://example.com/extension-catalog.yml", + "--name", "demo-extensions", + ], project) assert result.exit_code == 1, result.output output = _normalize_cli_output(result.output) @@ -1918,17 +1663,11 @@ def test_extension_catalog_add_escapes_catalog_name_markup(self, tmp_path): project = self._make_project(tmp_path) catalog_name = "[red]demo[/red]" - result = self._invoke( - [ - "extension", - "catalog", - "add", - "https://example.com/extension-catalog.yml", - "--name", - catalog_name, - ], - project, - ) + result = self._invoke([ + "extension", "catalog", "add", + "https://example.com/extension-catalog.yml", + "--name", catalog_name, + ], project) assert result.exit_code == 0, result.output output = _normalize_cli_output(result.output) @@ -2043,7 +1782,9 @@ def test_search_filters_by_author(self, tmp_path, monkeypatch): def test_search_no_match_hint(self, tmp_path, monkeypatch): project = self._make_project(tmp_path) self._patch_catalog(monkeypatch) - result = self._invoke(["integration", "search", "--tag", "nope"], project) + result = self._invoke( + ["integration", "search", "--tag", "nope"], project + ) assert result.exit_code == 0, result.output assert "No integrations found" in result.output assert "specify integration search" in result.output @@ -2061,7 +1802,9 @@ def test_search_marks_discovery_only_entry(self, tmp_path, monkeypatch): def test_info_found(self, tmp_path, monkeypatch): project = self._make_project(tmp_path) self._patch_catalog(monkeypatch) - result = self._invoke(["integration", "info", "stellar-agent"], project) + result = self._invoke( + ["integration", "info", "stellar-agent"], project + ) assert result.exit_code == 0, result.output assert "Stellar Agent" in result.output assert "stellar-agent" in result.output @@ -2070,7 +1813,9 @@ def test_info_found(self, tmp_path, monkeypatch): def test_info_not_found(self, tmp_path, monkeypatch): project = self._make_project(tmp_path) self._patch_catalog(monkeypatch) - result = self._invoke(["integration", "info", "does-not-exist"], project) + result = self._invoke( + ["integration", "info", "does-not-exist"], project + ) assert result.exit_code == 1 assert "not found" in result.output @@ -2106,7 +1851,9 @@ def test_search_local_config_error_shows_local_config_tip( assert "~/.specify/integration-catalogs.yml" in normalized_output assert "temporarily unavailable" not in normalized_output - def test_search_invalid_env_catalog_url_shows_env_tip(self, tmp_path, monkeypatch): + def test_search_invalid_env_catalog_url_shows_env_tip( + self, tmp_path, monkeypatch + ): project = self._make_project(tmp_path) monkeypatch.setenv( "SPECKIT_INTEGRATION_CATALOG_URL", @@ -2116,9 +1863,7 @@ def test_search_invalid_env_catalog_url_shows_env_tip(self, tmp_path, monkeypatc result = self._invoke(["integration", "search"], project) normalized_output = _normalize_cli_output(result.output) assert result.exit_code == 1, result.output - assert ( - "SPECKIT_INTEGRATION_CATALOG_URL environment variable" in normalized_output - ) + assert "SPECKIT_INTEGRATION_CATALOG_URL environment variable" in normalized_output assert "unset it to use the configured catalog files" in normalized_output assert ".specify/integration-catalogs.yml" in normalized_output assert "~/.specify/integration-catalogs.yml" in normalized_output @@ -2162,7 +1907,9 @@ def test_info_unknown_with_local_config_error_shows_local_config_tip( invalid_yaml = "catalogs:\n - [bad\n" cfg.write_text(invalid_yaml, encoding="utf-8") - result = self._invoke(["integration", "info", "definitely-not-real"], project) + result = self._invoke( + ["integration", "info", "definitely-not-real"], project + ) normalized_output = _normalize_cli_output(result.output) assert result.exit_code == 1, result.output assert "configuration file path shown above" in normalized_output @@ -2179,7 +1926,9 @@ def test_info_unknown_with_invalid_env_catalog_url_shows_env_tip( "http://insecure.example.com/catalog.json", ) - result = self._invoke(["integration", "info", "definitely-not-real"], project) + result = self._invoke( + ["integration", "info", "definitely-not-real"], project + ) normalized_output = _normalize_cli_output(result.output) assert result.exit_code == 1, result.output assert "SPECKIT_INTEGRATION_CATALOG_URL" in normalized_output @@ -2236,7 +1985,9 @@ def test_catalog_add_then_remove_roundtrip(self, tmp_path, monkeypatch): assert "default" not in list_result.output assert "community" not in list_result.output - remove_result = self._invoke(["integration", "catalog", "remove", "0"], project) + remove_result = self._invoke( + ["integration", "catalog", "remove", "0"], project + ) assert remove_result.exit_code == 0, remove_result.output assert "'mine' removed" in remove_result.output @@ -2340,7 +2091,6 @@ def test_catalog_add_strips_whitespace_in_success_output_and_storage( cfg_path = project / ".specify" / "integration-catalogs.yml" import yaml as _yaml - data = _yaml.safe_load(cfg_path.read_text(encoding="utf-8")) urls = [c["url"] for c in data["catalogs"]] assert clean_url in urls @@ -2363,9 +2113,13 @@ def test_catalog_add_rejects_invalid_url(self, tmp_path, monkeypatch): def test_catalog_add_rejects_duplicate(self, tmp_path, monkeypatch): project = self._make_project(tmp_path) url = "https://dup.example.com/catalog.json" - first = self._invoke(["integration", "catalog", "add", url], project) + first = self._invoke( + ["integration", "catalog", "add", url], project + ) assert first.exit_code == 0, first.output - second = self._invoke(["integration", "catalog", "add", url], project) + second = self._invoke( + ["integration", "catalog", "add", url], project + ) assert second.exit_code == 1 assert "already configured" in second.output @@ -2381,17 +2135,23 @@ def test_catalog_remove_out_of_range(self, tmp_path, monkeypatch): ], project, ) - result = self._invoke(["integration", "catalog", "remove", "9"], project) + result = self._invoke( + ["integration", "catalog", "remove", "9"], project + ) assert result.exit_code == 1 assert "out of range" in result.output def test_catalog_remove_without_config(self, tmp_path, monkeypatch): project = self._make_project(tmp_path) - result = self._invoke(["integration", "catalog", "remove", "0"], project) + result = self._invoke( + ["integration", "catalog", "remove", "0"], project + ) assert result.exit_code == 1 assert "No catalog config" in result.output - def test_catalog_remove_final_entry_restores_defaults(self, tmp_path, monkeypatch): + def test_catalog_remove_final_entry_restores_defaults( + self, tmp_path, monkeypatch + ): """End-to-end: add → remove-last-entry → list should not error. Regression for the flow where a user adds a catalog, removes it, then @@ -2417,7 +2177,9 @@ def test_catalog_remove_final_entry_restores_defaults(self, tmp_path, monkeypatc ) assert add.exit_code == 0, add.output - remove = self._invoke(["integration", "catalog", "remove", "0"], project) + remove = self._invoke( + ["integration", "catalog", "remove", "0"], project + ) assert remove.exit_code == 0, remove.output assert "'only' removed" in remove.output From e934b85b8df7071cc3fd57042c376b475bf57f52 Mon Sep 17 00:00:00 2001 From: Ben Buttigieg <70525+BenBtg@users.noreply.github.com> Date: Wed, 15 Jul 2026 17:25:17 +0100 Subject: [PATCH 4/4] Make preset init test depend on init ordering Disable preset-install lifecycle seeding in the regression test so it fails unless init materializes the constitution after registering the preset. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 49891a32-bec4-462c-a7f2-6d6ec4eefcdb --- tests/integrations/test_cli.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/integrations/test_cli.py b/tests/integrations/test_cli.py index a428968c05..6899a5105a 100644 --- a/tests/integrations/test_cli.py +++ b/tests/integrations/test_cli.py @@ -224,9 +224,18 @@ def fail_install(self, path, version): assert "Continuing without the optional preset" in normalized assert "Project ready" in normalized - def test_init_with_local_preset_seeds_manifest_constitution(self, tmp_path): + def test_init_with_local_preset_seeds_manifest_constitution( + self, tmp_path, monkeypatch + ): from typer.testing import CliRunner from specify_cli import app + from specify_cli.presets import PresetManager + + monkeypatch.setattr( + PresetManager, + "_seed_constitution_from_preset", + lambda *_args, **_kwargs: None, + ) preset_dir = tmp_path / "constitution-preset" (preset_dir / "organization").mkdir(parents=True)