From 19be1246ac059a65cfdea9bae4f897eabf7f2c3c Mon Sep 17 00:00:00 2001 From: Ben Buttigieg <70525+BenBtg@users.noreply.github.com> Date: Tue, 30 Jun 2026 19:54:29 +0100 Subject: [PATCH 01/10] fix(presets): seed constitution from preset constitution-template (#3272) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The constitution is the only template materialized to a live file (.specify/memory/constitution.md) rather than resolved on demand, yet ensure_constitution_from_template hardcoded a copy from the core template and ignored PresetResolver. Combined with init seeding the constitution before preset installation, a preset's constitution-template (e.g. strategy: replace with a ratified constitution) could never go live. Changes: - ensure_constitution_from_template now resolves constitution-template through PresetResolver, so a preset/override/extension wins and core is the fallback. - init seeds the constitution after preset installation so init --preset uses the resolved stack. - install_from_directory re-seeds memory/constitution.md from the resolved preset template, guarded to only act when the memory file is missing or still contains generic placeholder tokens — authored constitutions are never overwritten. Covers preset add and install_from_zip. - Tests for preset seeding, placeholder re-seed, authored-constitution preservation, override resolution, and resolver-aware init seeding. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/commands/init.py | 24 ++++-- src/specify_cli/presets/__init__.py | 64 ++++++++++++++++ tests/test_presets.py | 111 ++++++++++++++++++++++++++++ 3 files changed, 193 insertions(+), 6 deletions(-) diff --git a/src/specify_cli/commands/init.py b/src/specify_cli/commands/init.py index dd815b8c5d..af4ae3e845 100644 --- a/src/specify_cli/commands/init.py +++ b/src/specify_cli/commands/init.py @@ -33,10 +33,19 @@ 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 the resolved constitution template to memory if it doesn't exist. + + Resolution walks the full priority stack (project overrides → installed + presets → extensions → core) via :class:`PresetResolver`, so a preset that + ships a ``constitution-template`` (e.g. ``strategy: replace`` with a ratified + constitution) seeds the memory file verbatim. When nothing overrides it, the + resolver falls through to the core template, preserving legacy behavior. + """ + from ..presets import PresetResolver + memory_constitution = project_path / ".specify" / "memory" / "constitution.md" - template_constitution = ( - project_path / ".specify" / "templates" / "constitution-template.md" + template_constitution = PresetResolver(project_path).resolve( + "constitution-template", "template" ) if memory_constitution.exists(): @@ -45,7 +54,7 @@ def ensure_constitution_from_template( tracker.skip("constitution", "existing file preserved") return - if not template_constitution.exists(): + if template_constitution is None or not template_constitution.exists(): if tracker: tracker.add("constitution", "Constitution setup") tracker.error("constitution", "template not found") @@ -447,8 +456,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 +583,11 @@ def init( continuing="Continuing without the optional preset.", ) + # Seed the constitution AFTER preset installation so that a + # preset-provided constitution-template (resolved via the + # priority stack) wins over the core template. + 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..b6ea849052 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -34,6 +34,17 @@ from ..shared_infra import verify_archive_sha256 +# Tokens that mark an unmodified, generic constitution that has not yet been +# authored. Used to decide whether seeding/re-seeding memory/constitution.md +# from a preset-provided template is safe (i.e. won't clobber authored content). +_CONSTITUTION_PLACEHOLDER_TOKENS = ("[PROJECT_NAME]", "[PRINCIPLE_1_NAME]") + + +def _constitution_is_placeholder(content: str) -> bool: + """Return True if a constitution body is still the generic placeholder.""" + return any(token in content for token in _CONSTITUTION_PLACEHOLDER_TOKENS) + + def _substitute_core_template( body: str, cmd_name: str, @@ -1615,8 +1626,61 @@ def install_from_directory( stacklevel=2, ) + # Seed/re-seed memory/constitution.md from a preset-provided + # constitution-template. The constitution is the only template that is + # materialized to a live file rather than resolved on demand, so a + # preset that ships one (e.g. strategy: replace with a ratified + # constitution) must be propagated here. Guard against clobbering an + # already-authored constitution by only seeding when the memory file is + # missing or still contains generic placeholder tokens. + self._seed_constitution_from_preset(manifest) + return manifest + def _seed_constitution_from_preset(self, manifest: PresetManifest) -> None: + """Seed memory/constitution.md from a preset constitution-template. + + Only runs when the preset declares a ``type: template`` entry named + ``constitution-template`` and the live memory file is either missing or + still the generic placeholder. Authored constitutions are never + overwritten. + """ + provides_constitution = any( + t.get("type") == "template" and t.get("name") == "constitution-template" + for t in manifest.templates + ) + if not provides_constitution: + return + + memory_constitution = ( + self.project_root / ".specify" / "memory" / "constitution.md" + ) + if memory_constitution.exists(): + try: + existing = memory_constitution.read_text(encoding="utf-8") + except OSError: + return + if not _constitution_is_placeholder(existing): + # Legitimately authored constitution; leave it untouched. + return + + resolved = PresetResolver(self.project_root).resolve( + "constitution-template", "template" + ) + if resolved is None or not resolved.exists(): + return + + try: + memory_constitution.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(resolved, memory_constitution) + except OSError as exc: + import warnings + + warnings.warn( + f"Failed to seed constitution from preset {manifest.id}: {exc}.", + stacklevel=2, + ) + def install_from_zip( self, zip_path: Path, diff --git a/tests/test_presets.py b/tests/test_presets.py index a4c78d9268..933756b753 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2778,6 +2778,55 @@ def test_self_test_no_commands_without_agent_dirs(self, project_dir): metadata = manager.registry.get("self-test") assert metadata["registered_commands"] == {} + def test_self_test_seeds_constitution_when_memory_absent(self, project_dir): + """Installing a preset seeds memory/constitution.md from its template.""" + manager = PresetManager(project_dir) + install_self_test_preset(manager) + + memory = project_dir / ".specify" / "memory" / "constitution.md" + assert memory.exists(), "constitution.md was not seeded from the preset" + assert "preset:self-test" in memory.read_text(), ( + "constitution.md was not seeded from the self-test preset template" + ) + + def test_self_test_reseeds_placeholder_constitution(self, project_dir): + """A placeholder memory constitution is re-seeded from the preset template.""" + memory = project_dir / ".specify" / "memory" / "constitution.md" + memory.parent.mkdir(parents=True, exist_ok=True) + memory.write_text("# [PROJECT_NAME] Constitution\n\n### [PRINCIPLE_1_NAME]\n") + + manager = PresetManager(project_dir) + install_self_test_preset(manager) + + content = memory.read_text() + assert "preset:self-test" in content, "placeholder constitution was not re-seeded" + assert "[PROJECT_NAME]" not in content + + def test_self_test_preserves_authored_constitution(self, project_dir): + """An authored (placeholder-free) constitution is never overwritten.""" + memory = project_dir / ".specify" / "memory" / "constitution.md" + memory.parent.mkdir(parents=True, exist_ok=True) + authored = "# Acme Constitution\n\n### I. Ship It\nAuthored by a human.\n" + memory.write_text(authored) + + manager = PresetManager(project_dir) + install_self_test_preset(manager) + + assert memory.read_text() == authored, "authored constitution was overwritten" + + def test_self_test_override_resolves_constitution_template(self, project_dir): + """The preset override of constitution-template resolves to the preset file.""" + templates_dir = project_dir / ".specify" / "templates" + (templates_dir / "constitution-template.md").write_text("# Core constitution\n") + + manager = PresetManager(project_dir) + install_self_test_preset(manager) + + resolver = PresetResolver(project_dir) + result = resolver.resolve("constitution-template", "template") + assert result is not None + assert "preset:self-test" in result.read_text() + def test_extension_command_skipped_when_extension_missing(self, project_dir, temp_dir): """Test that extension command overrides are skipped if the extension isn't installed.""" claude_dir = project_dir / ".claude" / "skills" @@ -6204,3 +6253,65 @@ 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"] + + +# ===== ensure_constitution_from_template resolver-awareness ===== + + +class TestEnsureConstitutionResolverAware: + """`ensure_constitution_from_template` must resolve through PresetResolver. + + The constitution is the only template materialized to a live file rather + than resolved on demand. These tests pin the regression from issue #3272: + a preset-provided ``constitution-template`` must seed memory, while the + core template is used when no preset overrides it. + """ + + def _core_constitution(self, project_dir): + templates_dir = project_dir / ".specify" / "templates" + templates_dir.mkdir(parents=True, exist_ok=True) + (templates_dir / "constitution-template.md").write_text( + "# [PROJECT_NAME] Constitution\n\n### [PRINCIPLE_1_NAME]\n" + ) + + def test_seeds_from_core_when_no_preset(self, project_dir): + from specify_cli.commands.init import ensure_constitution_from_template + + self._core_constitution(project_dir) + ensure_constitution_from_template(project_dir) + + memory = project_dir / ".specify" / "memory" / "constitution.md" + assert memory.exists() + assert "[PROJECT_NAME]" in memory.read_text() + + def test_seeds_from_preset_when_installed(self, project_dir): + from specify_cli.commands.init import ensure_constitution_from_template + + self._core_constitution(project_dir) + manager = PresetManager(project_dir) + install_self_test_preset(manager) + + # Remove the memory file seeded during install to test ensure() in + # isolation; it must re-seed from the preset, not the core template. + memory = project_dir / ".specify" / "memory" / "constitution.md" + memory.unlink() + + ensure_constitution_from_template(project_dir) + + assert memory.exists() + content = memory.read_text() + assert "preset:self-test" in content + assert "[PROJECT_NAME]" not in content + + def test_preserves_existing_memory(self, project_dir): + from specify_cli.commands.init import ensure_constitution_from_template + + self._core_constitution(project_dir) + memory = project_dir / ".specify" / "memory" / "constitution.md" + memory.parent.mkdir(parents=True, exist_ok=True) + authored = "# Acme Constitution\nAuthored.\n" + memory.write_text(authored) + + ensure_constitution_from_template(project_dir) + + assert memory.read_text() == authored From f84ecd9c980c1437d0adde4126e99d4acfcd201e Mon Sep 17 00:00:00 2001 From: Ben Buttigieg <70525+BenBtg@users.noreply.github.com> Date: Wed, 1 Jul 2026 19:30:34 +0100 Subject: [PATCH 02/10] fix(presets): compose constitution-template when seeding memory Take on review feedback from Copilot and gglachant: - constitution seeding previously copied the top layer file path verbatim even when the winning layer used a composing strategy (prepend/append/wrap), which could leave {CORE_TEMPLATE} unresolved. - both seeding paths now inspect resolver layers and only copy verbatim for replace; non-replace strategies materialize composed content via PresetResolver.resolve_content(). - add regression tests for wrap strategy composition in both PresetManager seeding and ensure_constitution_from_template. - add a drift-guard test pinning _CONSTITUTION_PLACEHOLDER_TOKENS to the placeholders in templates/constitution-template.md. Assisted-by: GitHub Copilot (model: GPT-5.3-Codex, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/commands/init.py | 24 ++++-- src/specify_cli/presets/__init__.py | 18 +++-- tests/test_presets.py | 110 ++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+), 13 deletions(-) diff --git a/src/specify_cli/commands/init.py b/src/specify_cli/commands/init.py index af4ae3e845..00ea68806b 100644 --- a/src/specify_cli/commands/init.py +++ b/src/specify_cli/commands/init.py @@ -33,20 +33,19 @@ def _stdin_is_interactive() -> bool: def ensure_constitution_from_template( project_path: Path, tracker: StepTracker | None = None ) -> None: - """Copy the resolved constitution template to memory if it doesn't exist. + """Materialize the resolved constitution template to memory if missing. Resolution walks the full priority stack (project overrides → installed presets → extensions → core) via :class:`PresetResolver`, so a preset that ships a ``constitution-template`` (e.g. ``strategy: replace`` with a ratified - constitution) seeds the memory file verbatim. When nothing overrides it, the - resolver falls through to the core template, preserving legacy behavior. + constitution) can seed the memory file. When nothing overrides it, the + resolver falls through to the core template. """ from ..presets import PresetResolver memory_constitution = project_path / ".specify" / "memory" / "constitution.md" - template_constitution = PresetResolver(project_path).resolve( - "constitution-template", "template" - ) + resolver = PresetResolver(project_path) + layers = resolver.collect_all_layers("constitution-template", "template") if memory_constitution.exists(): if tracker: @@ -54,7 +53,7 @@ def ensure_constitution_from_template( tracker.skip("constitution", "existing file preserved") return - if template_constitution is None or not template_constitution.exists(): + if not layers: if tracker: tracker.add("constitution", "Constitution setup") tracker.error("constitution", "template not found") @@ -62,7 +61,16 @@ def ensure_constitution_from_template( try: memory_constitution.parent.mkdir(parents=True, exist_ok=True) - shutil.copy2(template_constitution, memory_constitution) + top_layer = layers[0] + if top_layer["strategy"] == "replace": + shutil.copy2(top_layer["path"], memory_constitution) + else: + composed_content = resolver.resolve_content( + "constitution-template", "template" + ) + if composed_content is None: + raise FileNotFoundError("constitution template not found") + memory_constitution.write_text(composed_content, encoding="utf-8") 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 b6ea849052..184f3d6b8c 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -1664,15 +1664,23 @@ def _seed_constitution_from_preset(self, manifest: PresetManifest) -> None: # Legitimately authored constitution; leave it untouched. return - resolved = PresetResolver(self.project_root).resolve( - "constitution-template", "template" - ) - if resolved is None or not resolved.exists(): + resolver = PresetResolver(self.project_root) + layers = resolver.collect_all_layers("constitution-template", "template") + if not layers: return try: memory_constitution.parent.mkdir(parents=True, exist_ok=True) - shutil.copy2(resolved, memory_constitution) + top_layer = layers[0] + if top_layer["strategy"] == "replace": + shutil.copy2(top_layer["path"], memory_constitution) + else: + composed_content = resolver.resolve_content( + "constitution-template", "template" + ) + if composed_content is None: + return + memory_constitution.write_text(composed_content, encoding="utf-8") except OSError as exc: import warnings diff --git a/tests/test_presets.py b/tests/test_presets.py index 933756b753..20c40cdb52 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2827,6 +2827,66 @@ def test_self_test_override_resolves_constitution_template(self, project_dir): assert result is not None assert "preset:self-test" in result.read_text() + def test_constitution_seed_composes_wrap_strategy(self, project_dir, temp_dir): + """Seeding memory composes wrap constitution-template layers.""" + templates_dir = project_dir / ".specify" / "templates" + templates_dir.mkdir(parents=True, exist_ok=True) + (templates_dir / "constitution-template.md").write_text( + "# Core Constitution\n\n## Core Principle\n" + ) + + preset_dir = temp_dir / "constitution-wrap" + (preset_dir / "templates").mkdir(parents=True) + (preset_dir / "templates" / "constitution-template.md").write_text( + "# Wrapper Constitution\n\n{CORE_TEMPLATE}\n\n## Wrapper Footer\n" + ) + (preset_dir / "preset.yml").write_text( + yaml.dump( + { + "schema_version": "1.0", + "preset": { + "id": "constitution-wrap", + "name": "Constitution Wrap", + "version": "1.0.0", + "description": "Wrap constitution template for testing", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [ + { + "type": "template", + "name": "constitution-template", + "file": "templates/constitution-template.md", + "strategy": "wrap", + "description": "Wrapped constitution template", + } + ] + }, + } + ) + ) + + manager = PresetManager(project_dir) + manager.install_from_directory(preset_dir, "0.1.5") + + memory = project_dir / ".specify" / "memory" / "constitution.md" + content = memory.read_text() + assert "{CORE_TEMPLATE}" not in content + assert "# Wrapper Constitution" in content + assert "## Core Principle" in content + + def test_constitution_placeholder_tokens_are_pinned_to_core_template(self): + """Guard placeholder token drift between code and core template.""" + from specify_cli.presets import _CONSTITUTION_PLACEHOLDER_TOKENS + + expected_tokens = {"[PROJECT_NAME]", "[PRINCIPLE_1_NAME]"} + assert set(_CONSTITUTION_PLACEHOLDER_TOKENS) == expected_tokens + + core_template = Path(__file__).parent.parent / "templates" / "constitution-template.md" + content = core_template.read_text(encoding="utf-8") + for token in expected_tokens: + assert token in content + def test_extension_command_skipped_when_extension_missing(self, project_dir, temp_dir): """Test that extension command overrides are skipped if the extension isn't installed.""" claude_dir = project_dir / ".claude" / "skills" @@ -6274,6 +6334,39 @@ def _core_constitution(self, project_dir): "# [PROJECT_NAME] Constitution\n\n### [PRINCIPLE_1_NAME]\n" ) + def _wrap_constitution_preset(self, temp_dir): + preset_dir = temp_dir / "ensure-wrap-preset" + (preset_dir / "templates").mkdir(parents=True) + (preset_dir / "templates" / "constitution-template.md").write_text( + "# Ensure Wrapper\n\n{CORE_TEMPLATE}\n\n## Tail\n" + ) + (preset_dir / "preset.yml").write_text( + yaml.dump( + { + "schema_version": "1.0", + "preset": { + "id": "ensure-wrap", + "name": "Ensure Wrap", + "version": "1.0.0", + "description": "Wrap strategy for ensure() coverage", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [ + { + "type": "template", + "name": "constitution-template", + "file": "templates/constitution-template.md", + "strategy": "wrap", + "description": "Wrapped constitution", + } + ] + }, + } + ) + ) + return preset_dir + def test_seeds_from_core_when_no_preset(self, project_dir): from specify_cli.commands.init import ensure_constitution_from_template @@ -6315,3 +6408,20 @@ def test_preserves_existing_memory(self, project_dir): ensure_constitution_from_template(project_dir) assert memory.read_text() == authored + + def test_composes_wrap_strategy_when_ensuring(self, project_dir, temp_dir): + from specify_cli.commands.init import ensure_constitution_from_template + + self._core_constitution(project_dir) + manager = PresetManager(project_dir) + manager.install_from_directory(self._wrap_constitution_preset(temp_dir), "0.1.5") + + # Ensure we validate ensure() behavior directly. + memory = project_dir / ".specify" / "memory" / "constitution.md" + memory.unlink() + ensure_constitution_from_template(project_dir) + + content = memory.read_text() + assert "{CORE_TEMPLATE}" not in content + assert "# Ensure Wrapper" in content + assert "[PROJECT_NAME]" in content From ff8bda3005a3de63ea204364da3fcd31022c4ab6 Mon Sep 17 00:00:00 2001 From: Ben Buttigieg <70525+BenBtg@users.noreply.github.com> Date: Wed, 1 Jul 2026 19:53:21 +0100 Subject: [PATCH 03/10] refactor(presets): unify constitution template materialization Address latest Copilot feedback on the constitution seeding path: - moved resolver/layer I/O behind the existing-memory fast path in init - corrected tracker output for composed materialization - deduplicated materialization logic shared by init and preset install seeding into presets._materialize_constitution_template() Behavior is unchanged for replace strategies (copy verbatim) and remains composed for prepend/append/wrap via resolve_content(). Assisted-by: GitHub Copilot (model: GPT-5.3-Codex, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/commands/init.py | 35 ++++++++------------ src/specify_cli/presets/__init__.py | 50 ++++++++++++++++++++--------- 2 files changed, 47 insertions(+), 38 deletions(-) diff --git a/src/specify_cli/commands/init.py b/src/specify_cli/commands/init.py index 00ea68806b..8c39014ceb 100644 --- a/src/specify_cli/commands/init.py +++ b/src/specify_cli/commands/init.py @@ -3,7 +3,6 @@ from __future__ import annotations import os -import shutil import sys from pathlib import Path from typing import Any @@ -41,11 +40,9 @@ def ensure_constitution_from_template( constitution) can seed the memory file. When nothing overrides it, the resolver falls through to the core template. """ - from ..presets import PresetResolver + from ..presets import _materialize_constitution_template memory_constitution = project_path / ".specify" / "memory" / "constitution.md" - resolver = PresetResolver(project_path) - layers = resolver.collect_all_layers("constitution-template", "template") if memory_constitution.exists(): if tracker: @@ -53,27 +50,21 @@ def ensure_constitution_from_template( tracker.skip("constitution", "existing file preserved") return - if not layers: - if tracker: - tracker.add("constitution", "Constitution setup") - tracker.error("constitution", "template not found") - return - try: - memory_constitution.parent.mkdir(parents=True, exist_ok=True) - top_layer = layers[0] - if top_layer["strategy"] == "replace": - shutil.copy2(top_layer["path"], memory_constitution) - else: - composed_content = resolver.resolve_content( - "constitution-template", "template" - ) - if composed_content is None: - raise FileNotFoundError("constitution template not found") - memory_constitution.write_text(composed_content, encoding="utf-8") + materialization = _materialize_constitution_template( + project_path, memory_constitution + ) + if materialization is None: + if tracker: + tracker.add("constitution", "Constitution setup") + tracker.error("constitution", "template not found") + return if tracker: tracker.add("constitution", "Constitution setup") - tracker.complete("constitution", "copied from template") + if materialization == "copied": + tracker.complete("constitution", "copied from template") + else: + tracker.complete("constitution", "composed from template") else: console.print("[cyan]Initialized constitution from template[/cyan]") except Exception as e: diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 184f3d6b8c..fb596f9424 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -45,6 +45,35 @@ def _constitution_is_placeholder(content: str) -> bool: return any(token in content for token in _CONSTITUTION_PLACEHOLDER_TOKENS) +def _materialize_constitution_template( + project_root: Path, + memory_constitution: Path, +) -> str | None: + """Materialize constitution-template content into memory/constitution.md. + + Returns: + "copied" when the winning layer is ``replace`` and the source file is + copied verbatim; "composed" when a composing strategy is materialized + via ``resolve_content``; ``None`` when no constitution template resolves. + """ + resolver = PresetResolver(project_root) + layers = resolver.collect_all_layers("constitution-template", "template") + if not layers: + return None + + memory_constitution.parent.mkdir(parents=True, exist_ok=True) + top_layer = layers[0] + if top_layer["strategy"] == "replace": + shutil.copy2(top_layer["path"], memory_constitution) + return "copied" + + composed_content = resolver.resolve_content("constitution-template", "template") + if composed_content is None: + return None + memory_constitution.write_text(composed_content, encoding="utf-8") + return "composed" + + def _substitute_core_template( body: str, cmd_name: str, @@ -1664,23 +1693,12 @@ def _seed_constitution_from_preset(self, manifest: PresetManifest) -> None: # Legitimately authored constitution; leave it untouched. return - resolver = PresetResolver(self.project_root) - layers = resolver.collect_all_layers("constitution-template", "template") - if not layers: - return - try: - memory_constitution.parent.mkdir(parents=True, exist_ok=True) - top_layer = layers[0] - if top_layer["strategy"] == "replace": - shutil.copy2(top_layer["path"], memory_constitution) - else: - composed_content = resolver.resolve_content( - "constitution-template", "template" - ) - if composed_content is None: - return - memory_constitution.write_text(composed_content, encoding="utf-8") + result = _materialize_constitution_template( + self.project_root, memory_constitution + ) + if result is None: + return except OSError as exc: import warnings From e03fb69bbfe346ee777dd396c1c1e1e639a3c391 Mon Sep 17 00:00:00 2001 From: Ben Buttigieg <70525+BenBtg@users.noreply.github.com> Date: Mon, 13 Jul 2026 16:41:03 +0100 Subject: [PATCH 04/10] fix(init): restore shutil import The constitution materialization refactor removed the module import, but init still uses shutil.rmtree when cleaning up a failed new-project initialization. Restore the import so the required ruff check passes. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/commands/init.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/specify_cli/commands/init.py b/src/specify_cli/commands/init.py index 8c39014ceb..ed7cb7ce74 100644 --- a/src/specify_cli/commands/init.py +++ b/src/specify_cli/commands/init.py @@ -3,6 +3,7 @@ from __future__ import annotations import os +import shutil import sys from pathlib import Path from typing import Any From 116ab2c2b22cbdfdaa685911ab5ca52564f3fd80 Mon Sep 17 00:00:00 2001 From: Ben Buttigieg <70525+BenBtg@users.noreply.github.com> Date: Mon, 13 Jul 2026 17:55:41 +0100 Subject: [PATCH 05/10] fix(presets): harden constitution materialization Address the outstanding review batch for preset constitution seeding: - use checked atomic writes and reject symlinked memory paths - replace placeholder heuristics with hash/source provenance - rematerialize unchanged generated constitutions by resolver priority - preserve authored or edited constitutions, including placeholder mentions - warn non-fatally when post-install materialization cannot complete - retain exact core-template comparison for legacy projects without provenance Add focused provenance, priority, symlink, and failure-path coverage, and update integration inventories for the generated provenance sidecar. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1b2c095d-b45c-4d52-8d56-bd6121d96ab6 --- src/specify_cli/presets/__init__.py | 107 +++++++---- .../test_integration_base_markdown.py | 1 + .../test_integration_base_skills.py | 1 + .../test_integration_base_toml.py | 1 + .../test_integration_base_yaml.py | 1 + tests/integrations/test_integration_cline.py | 1 + .../integrations/test_integration_copilot.py | 3 + .../integrations/test_integration_generic.py | 2 + tests/test_presets.py | 169 ++++++++++++++++-- 9 files changed, 244 insertions(+), 42 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index fb596f9424..8b671c333d 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -31,18 +31,50 @@ 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, + _ensure_safe_shared_directory, + _write_shared_bytes, + _write_shared_text, + verify_archive_sha256, +) -# Tokens that mark an unmodified, generic constitution that has not yet been -# authored. Used to decide whether seeding/re-seeding memory/constitution.md -# from a preset-provided template is safe (i.e. won't clobber authored content). -_CONSTITUTION_PLACEHOLDER_TOKENS = ("[PROJECT_NAME]", "[PRINCIPLE_1_NAME]") +_CONSTITUTION_PROVENANCE_FILE = ".constitution-template.json" -def _constitution_is_placeholder(content: str) -> bool: - """Return True if a constitution body is still the generic placeholder.""" - return any(token in content for token in _CONSTITUTION_PLACEHOLDER_TOKENS) +def _content_sha256(content: bytes) -> str: + return hashlib.sha256(content).hexdigest() + + +def _constitution_is_generated( + project_root: Path, + memory_constitution: Path, + layers: list[dict[str, Any]], +) -> bool: + """Return whether the live constitution is an unchanged generated file.""" + _ensure_safe_shared_destination(project_root, memory_constitution) + content = memory_constitution.read_bytes() + provenance = memory_constitution.parent / _CONSTITUTION_PROVENANCE_FILE + _ensure_safe_shared_destination(project_root, provenance) + + if provenance.exists(): + try: + metadata = json.loads(provenance.read_text(encoding="utf-8")) + except (json.JSONDecodeError, UnicodeDecodeError): + metadata = {} + if ( + isinstance(metadata, dict) + and metadata.get("sha256") == _content_sha256(content) + ): + return True + + # Older projects have no provenance sidecar. Only the exact known core + # template is safe to treat as generated; placeholder substrings are not. + for layer in layers: + if layer["source"].startswith("core") and layer["path"].read_bytes() == content: + return True + return False def _materialize_constitution_template( @@ -61,17 +93,33 @@ def _materialize_constitution_template( if not layers: return None - memory_constitution.parent.mkdir(parents=True, exist_ok=True) top_layer = layers[0] if top_layer["strategy"] == "replace": - shutil.copy2(top_layer["path"], memory_constitution) - return "copied" - - composed_content = resolver.resolve_content("constitution-template", "template") - if composed_content is None: - return None - memory_constitution.write_text(composed_content, encoding="utf-8") - return "composed" + content = top_layer["path"].read_bytes() + result = "copied" + else: + composed_content = resolver.resolve_content("constitution-template", "template") + if composed_content is None: + return None + content = composed_content.encode("utf-8") + result = "composed" + + _ensure_safe_shared_directory(project_root, memory_constitution.parent) + _write_shared_bytes(project_root, memory_constitution, content) + provenance = memory_constitution.parent / _CONSTITUTION_PROVENANCE_FILE + _write_shared_text( + project_root, + provenance, + json.dumps( + { + "sha256": _content_sha256(content), + "source": top_layer["source"], + }, + indent=2, + ) + + "\n", + ) + return result def _substitute_core_template( @@ -1660,8 +1708,8 @@ def install_from_directory( # materialized to a live file rather than resolved on demand, so a # preset that ships one (e.g. strategy: replace with a ratified # constitution) must be propagated here. Guard against clobbering an - # already-authored constitution by only seeding when the memory file is - # missing or still contains generic placeholder tokens. + # already-authored constitution by only replacing a file whose recorded + # hash (or exact legacy core-template content) proves it was generated. self._seed_constitution_from_preset(manifest) return manifest @@ -1671,7 +1719,7 @@ def _seed_constitution_from_preset(self, manifest: PresetManifest) -> None: Only runs when the preset declares a ``type: template`` entry named ``constitution-template`` and the live memory file is either missing or - still the generic placeholder. Authored constitutions are never + is an unchanged generated file. Authored constitutions are never overwritten. """ provides_constitution = any( @@ -1684,22 +1732,21 @@ def _seed_constitution_from_preset(self, manifest: PresetManifest) -> None: memory_constitution = ( self.project_root / ".specify" / "memory" / "constitution.md" ) - if memory_constitution.exists(): - try: - existing = memory_constitution.read_text(encoding="utf-8") - except OSError: - return - if not _constitution_is_placeholder(existing): - # Legitimately authored constitution; leave it untouched. - return - try: + resolver = PresetResolver(self.project_root) + layers = resolver.collect_all_layers( + "constitution-template", "template" + ) + if memory_constitution.exists() and not _constitution_is_generated( + self.project_root, memory_constitution, layers + ): + return result = _materialize_constitution_template( self.project_root, memory_constitution ) if result is None: return - except OSError as exc: + except (OSError, UnicodeDecodeError, PresetValidationError, ValueError) as exc: import warnings warnings.warn( diff --git a/tests/integrations/test_integration_base_markdown.py b/tests/integrations/test_integration_base_markdown.py index 886dfb912f..aa906c440d 100644 --- a/tests/integrations/test_integration_base_markdown.py +++ b/tests/integrations/test_integration_base_markdown.py @@ -253,6 +253,7 @@ def _expected_files(self, script_variant: str) -> list[str]: "spec-template.md", "tasks-template.md"]: files.append(f".specify/templates/{name}") + files.append(".specify/memory/.constitution-template.json") files.append(".specify/memory/constitution.md") # Bundled workflow files.append(".specify/workflows/speckit/workflow.yml") diff --git a/tests/integrations/test_integration_base_skills.py b/tests/integrations/test_integration_base_skills.py index d88b786757..79c6d3fb50 100644 --- a/tests/integrations/test_integration_base_skills.py +++ b/tests/integrations/test_integration_base_skills.py @@ -399,6 +399,7 @@ def _expected_files(self, script_variant: str) -> list[str]: ".specify/integration.json", f".specify/integrations/{self.KEY}.manifest.json", ".specify/integrations/speckit.manifest.json", + ".specify/memory/.constitution-template.json", ".specify/memory/constitution.md", ] # Script variant diff --git a/tests/integrations/test_integration_base_toml.py b/tests/integrations/test_integration_base_toml.py index 37bad8a609..8a7344e4b2 100644 --- a/tests/integrations/test_integration_base_toml.py +++ b/tests/integrations/test_integration_base_toml.py @@ -517,6 +517,7 @@ def _expected_files(self, script_variant: str) -> list[str]: ]: files.append(f".specify/templates/{name}") + files.append(".specify/memory/.constitution-template.json") files.append(".specify/memory/constitution.md") # Bundled workflow files.append(".specify/workflows/speckit/workflow.yml") diff --git a/tests/integrations/test_integration_base_yaml.py b/tests/integrations/test_integration_base_yaml.py index 56bed09eb2..27c11ebeb3 100644 --- a/tests/integrations/test_integration_base_yaml.py +++ b/tests/integrations/test_integration_base_yaml.py @@ -401,6 +401,7 @@ def _expected_files(self, script_variant: str) -> list[str]: ]: files.append(f".specify/templates/{name}") + files.append(".specify/memory/.constitution-template.json") files.append(".specify/memory/constitution.md") # Bundled workflow files.append(".specify/workflows/speckit/workflow.yml") diff --git a/tests/integrations/test_integration_cline.py b/tests/integrations/test_integration_cline.py index 7475b5ad02..f1abdedc8a 100644 --- a/tests/integrations/test_integration_cline.py +++ b/tests/integrations/test_integration_cline.py @@ -214,6 +214,7 @@ def _expected_files(self, script_variant: str) -> list[str]: ]: files.append(f".specify/templates/{name}") + files.append(".specify/memory/.constitution-template.json") files.append(".specify/memory/constitution.md") # Bundled workflow files.append(".specify/workflows/speckit/workflow.yml") diff --git a/tests/integrations/test_integration_copilot.py b/tests/integrations/test_integration_copilot.py index 41261ba1cb..5b3a5712ad 100644 --- a/tests/integrations/test_integration_copilot.py +++ b/tests/integrations/test_integration_copilot.py @@ -252,6 +252,7 @@ def test_complete_file_inventory_sh(self, tmp_path): ".specify/templates/plan-template.md", ".specify/templates/spec-template.md", ".specify/templates/tasks-template.md", + ".specify/memory/.constitution-template.json", ".specify/memory/constitution.md", ".specify/workflows/speckit/workflow.yml", ".specify/workflows/workflow-registry.json", @@ -313,6 +314,7 @@ def test_complete_file_inventory_ps(self, tmp_path): ".specify/templates/plan-template.md", ".specify/templates/spec-template.md", ".specify/templates/tasks-template.md", + ".specify/memory/.constitution-template.json", ".specify/memory/constitution.md", ".specify/workflows/speckit/workflow.yml", ".specify/workflows/workflow-registry.json", @@ -724,6 +726,7 @@ def test_complete_file_inventory_skills_sh(self, tmp_path): ".specify/templates/plan-template.md", ".specify/templates/spec-template.md", ".specify/templates/tasks-template.md", + ".specify/memory/.constitution-template.json", ".specify/memory/constitution.md", # Bundled workflow ".specify/workflows/speckit/workflow.yml", diff --git a/tests/integrations/test_integration_generic.py b/tests/integrations/test_integration_generic.py index 1c5edc2efc..3f079c438d 100644 --- a/tests/integrations/test_integration_generic.py +++ b/tests/integrations/test_integration_generic.py @@ -286,6 +286,7 @@ def test_complete_file_inventory_sh(self, tmp_path): ".specify/integration.json", ".specify/integrations/generic.manifest.json", ".specify/integrations/speckit.manifest.json", + ".specify/memory/.constitution-template.json", ".specify/memory/constitution.md", ".specify/scripts/bash/check-prerequisites.sh", ".specify/scripts/bash/common.sh", @@ -342,6 +343,7 @@ def test_complete_file_inventory_ps(self, tmp_path): ".specify/integration.json", ".specify/integrations/generic.manifest.json", ".specify/integrations/speckit.manifest.json", + ".specify/memory/.constitution-template.json", ".specify/memory/constitution.md", ".specify/scripts/powershell/check-prerequisites.ps1", ".specify/scripts/powershell/common.ps1", diff --git a/tests/test_presets.py b/tests/test_presets.py index 20c40cdb52..b1f71fdbf0 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2789,11 +2789,15 @@ def test_self_test_seeds_constitution_when_memory_absent(self, project_dir): "constitution.md was not seeded from the self-test preset template" ) - def test_self_test_reseeds_placeholder_constitution(self, project_dir): - """A placeholder memory constitution is re-seeded from the preset template.""" + def test_self_test_reseeds_exact_core_constitution(self, project_dir): + """An unchanged core constitution is re-seeded from the preset template.""" + core = "# [PROJECT_NAME] Constitution\n\n### [PRINCIPLE_1_NAME]\n" + (project_dir / ".specify" / "templates" / "constitution-template.md").write_text( + core + ) memory = project_dir / ".specify" / "memory" / "constitution.md" memory.parent.mkdir(parents=True, exist_ok=True) - memory.write_text("# [PROJECT_NAME] Constitution\n\n### [PRINCIPLE_1_NAME]\n") + memory.write_text(core) manager = PresetManager(project_dir) install_self_test_preset(manager) @@ -2802,6 +2806,20 @@ def test_self_test_reseeds_placeholder_constitution(self, project_dir): assert "preset:self-test" in content, "placeholder constitution was not re-seeded" assert "[PROJECT_NAME]" not in content + def test_self_test_preserves_authored_constitution_with_placeholder( + self, project_dir + ): + """A placeholder mention does not establish generated provenance.""" + memory = project_dir / ".specify" / "memory" / "constitution.md" + memory.parent.mkdir(parents=True, exist_ok=True) + authored = "# Acme Constitution\n\nGuidance for [PROJECT_NAME].\n" + memory.write_text(authored) + + manager = PresetManager(project_dir) + install_self_test_preset(manager) + + assert memory.read_text() == authored + def test_self_test_preserves_authored_constitution(self, project_dir): """An authored (placeholder-free) constitution is never overwritten.""" memory = project_dir / ".specify" / "memory" / "constitution.md" @@ -2875,17 +2893,129 @@ def test_constitution_seed_composes_wrap_strategy(self, project_dir, temp_dir): assert "# Wrapper Constitution" in content assert "## Core Principle" in content - def test_constitution_placeholder_tokens_are_pinned_to_core_template(self): - """Guard placeholder token drift between code and core template.""" - from specify_cli.presets import _CONSTITUTION_PLACEHOLDER_TOKENS + def test_higher_priority_preset_reseeds_unchanged_generated_constitution( + self, project_dir, temp_dir + ): + """An unchanged generated constitution follows resolver priority.""" + manager = PresetManager(project_dir) + install_self_test_preset(manager) + + preset_dir = temp_dir / "higher-priority" + (preset_dir / "templates").mkdir(parents=True) + (preset_dir / "templates" / "constitution-template.md").write_text( + "# Higher Priority Constitution\n" + ) + (preset_dir / "preset.yml").write_text( + yaml.dump( + { + "schema_version": "1.0", + "preset": { + "id": "higher-priority", + "name": "Higher Priority", + "version": "1.0.0", + "description": "Higher-priority constitution", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [ + { + "type": "template", + "name": "constitution-template", + "file": "templates/constitution-template.md", + "strategy": "replace", + "description": "Higher-priority constitution", + } + ] + }, + } + ) + ) + + manager.install_from_directory(preset_dir, "0.1.5", priority=1) + + memory = project_dir / ".specify" / "memory" / "constitution.md" + assert memory.read_text() == "# Higher Priority Constitution\n" + + def test_constitution_seed_rejects_symlinked_memory_directory( + self, project_dir, temp_dir + ): + """Preset installation cannot seed through a symlinked memory directory.""" + outside = temp_dir / "outside" + outside.mkdir() + try: + (project_dir / ".specify" / "memory").symlink_to( + outside, target_is_directory=True + ) + except OSError: + pytest.skip("symlinks are unavailable") + + manager = PresetManager(project_dir) + with pytest.warns(UserWarning, match="symlinked"): + install_self_test_preset(manager) + + assert manager.registry.is_installed("self-test") + assert not (outside / "constitution.md").exists() + + def test_constitution_seed_rejects_dangling_destination_symlink( + self, project_dir, temp_dir + ): + """Preset installation cannot seed through a dangling destination symlink.""" + memory = project_dir / ".specify" / "memory" + memory.mkdir(parents=True) + outside = temp_dir / "outside-constitution.md" + try: + (memory / "constitution.md").symlink_to(outside) + except OSError: + pytest.skip("symlinks are unavailable") + + manager = PresetManager(project_dir) + with pytest.warns(UserWarning, match="symlinked"): + install_self_test_preset(manager) + + assert manager.registry.is_installed("self-test") + assert not outside.exists() + + def test_constitution_materialization_error_is_nonfatal( + self, project_dir, temp_dir + ): + """An invalid wrap warns without reporting an uninstalled preset.""" + preset_dir = temp_dir / "invalid-wrap" + (preset_dir / "templates").mkdir(parents=True) + (preset_dir / "templates" / "constitution-template.md").write_text( + "# Missing core placeholder\n" + ) + (preset_dir / "preset.yml").write_text( + yaml.dump( + { + "schema_version": "1.0", + "preset": { + "id": "invalid-wrap", + "name": "Invalid Wrap", + "version": "1.0.0", + "description": "Invalid wrapping constitution", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [ + { + "type": "template", + "name": "constitution-template", + "file": "templates/constitution-template.md", + "strategy": "wrap", + "description": "Invalid wrap", + } + ] + }, + } + ) + ) - expected_tokens = {"[PROJECT_NAME]", "[PRINCIPLE_1_NAME]"} - assert set(_CONSTITUTION_PLACEHOLDER_TOKENS) == expected_tokens + manager = PresetManager(project_dir) + with pytest.warns(UserWarning, match="Failed to seed constitution"): + manifest = manager.install_from_directory(preset_dir, "0.1.5") - core_template = Path(__file__).parent.parent / "templates" / "constitution-template.md" - content = core_template.read_text(encoding="utf-8") - for token in expected_tokens: - assert token in content + assert manifest.id == "invalid-wrap" + assert manager.registry.is_installed("invalid-wrap") def test_extension_command_skipped_when_extension_missing(self, project_dir, temp_dir): """Test that extension command overrides are skipped if the extension isn't installed.""" @@ -6376,6 +6506,7 @@ def test_seeds_from_core_when_no_preset(self, project_dir): memory = project_dir / ".specify" / "memory" / "constitution.md" assert memory.exists() assert "[PROJECT_NAME]" in memory.read_text() + assert (memory.parent / ".constitution-template.json").exists() def test_seeds_from_preset_when_installed(self, project_dir): from specify_cli.commands.init import ensure_constitution_from_template @@ -6409,6 +6540,20 @@ def test_preserves_existing_memory(self, project_dir): assert memory.read_text() == authored + def test_preserves_edited_generated_memory(self, project_dir): + from specify_cli.commands.init import ensure_constitution_from_template + + self._core_constitution(project_dir) + ensure_constitution_from_template(project_dir) + memory = project_dir / ".specify" / "memory" / "constitution.md" + authored = memory.read_text() + "\nAuthored amendment.\n" + memory.write_text(authored) + + manager = PresetManager(project_dir) + install_self_test_preset(manager) + + assert memory.read_text() == authored + def test_composes_wrap_strategy_when_ensuring(self, project_dir, temp_dir): from specify_cli.commands.init import ensure_constitution_from_template From 8685bccb2c3877a8d9dc083139af21dd9a52bb58 Mon Sep 17 00:00:00 2001 From: Ben Buttigieg <70525+BenBtg@users.noreply.github.com> Date: Tue, 14 Jul 2026 11:57:57 +0100 Subject: [PATCH 06/10] fix(presets): reconcile constitution after removal When the removed preset supplied constitution-template, rematerialize the winning remaining resolver layer only if provenance proves the live file is still generated and unchanged. Preserve edited constitutions and report post-removal reconciliation failures as non-fatal warnings. Add coverage for restoring the core layer, falling back from a removed higher-priority preset, and preserving edited generated content. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1b2c095d-b45c-4d52-8d56-bd6121d96ab6 --- src/specify_cli/presets/__init__.py | 48 +++++++++++++++++++---------- tests/test_presets.py | 26 ++++++++++++++-- 2 files changed, 56 insertions(+), 18 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 8b671c333d..b9c1a711ec 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -1729,23 +1729,8 @@ def _seed_constitution_from_preset(self, manifest: PresetManifest) -> None: if not provides_constitution: return - memory_constitution = ( - self.project_root / ".specify" / "memory" / "constitution.md" - ) try: - resolver = PresetResolver(self.project_root) - layers = resolver.collect_all_layers( - "constitution-template", "template" - ) - if memory_constitution.exists() and not _constitution_is_generated( - self.project_root, memory_constitution, layers - ): - return - result = _materialize_constitution_template( - self.project_root, memory_constitution - ) - if result is None: - return + self._reconcile_constitution() except (OSError, UnicodeDecodeError, PresetValidationError, ValueError) as exc: import warnings @@ -1754,6 +1739,19 @@ def _seed_constitution_from_preset(self, manifest: PresetManifest) -> None: stacklevel=2, ) + def _reconcile_constitution(self) -> None: + """Materialize the winning constitution layer when the live file is generated.""" + memory_constitution = ( + self.project_root / ".specify" / "memory" / "constitution.md" + ) + resolver = PresetResolver(self.project_root) + layers = resolver.collect_all_layers("constitution-template", "template") + if memory_constitution.exists() and not _constitution_is_generated( + self.project_root, memory_constitution, layers + ): + return + _materialize_constitution_template(self.project_root, memory_constitution) + def install_from_zip( self, zip_path: Path, @@ -1833,6 +1831,7 @@ def remove(self, pack_id: str) -> bool: # Also include aliases from the manifest as a safety net for registries # populated by older versions that may not track aliases. removed_cmd_names = set() + removed_constitution = False for cmd_names in registered_commands.values(): removed_cmd_names.update(cmd_names) manifest_path = pack_dir / "preset.yml" @@ -1840,6 +1839,11 @@ def remove(self, pack_id: str) -> bool: try: manifest = PresetManifest(manifest_path) for tmpl in manifest.templates: + if ( + tmpl.get("type") == "template" + and tmpl.get("name") == "constitution-template" + ): + removed_constitution = True if tmpl.get("type") == "command": for alias in tmpl.get("aliases", []): if isinstance(alias, str): @@ -1886,6 +1890,18 @@ def remove(self, pack_id: str) -> bool: stacklevel=2, ) + if removed_constitution: + try: + self._reconcile_constitution() + except (OSError, UnicodeDecodeError, PresetValidationError, ValueError) as exc: + import warnings + + warnings.warn( + f"Post-removal constitution reconciliation failed for {pack_id}: " + f"{exc}. The live constitution may be stale.", + stacklevel=2, + ) + return True def list_installed(self) -> List[Dict[str, Any]]: diff --git a/tests/test_presets.py b/tests/test_presets.py index b1f71fdbf0..4b6454b4bb 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2704,6 +2704,24 @@ def test_self_test_removal_restores_core(self, project_dir): assert result is not None assert result["source"] == "core" + memory = project_dir / ".specify" / "memory" / "constitution.md" + assert memory.read_text() == "# Core constitution-template\n" + + def test_self_test_removal_preserves_edited_constitution(self, project_dir): + """Removing a preset does not overwrite an edited generated constitution.""" + templates_dir = project_dir / ".specify" / "templates" + (templates_dir / "constitution-template.md").write_text("# Core Constitution\n") + + manager = PresetManager(project_dir) + install_self_test_preset(manager) + memory = project_dir / ".specify" / "memory" / "constitution.md" + edited = memory.read_text() + "\n## Authored amendment\n" + memory.write_text(edited) + + manager.remove("self-test") + + assert memory.read_text() == edited + def test_self_test_not_in_catalog(self): """Verify the self-test preset is NOT in the catalog (it's local-only).""" catalog_path = Path(__file__).parent.parent / "presets" / "catalog.json" @@ -2893,10 +2911,10 @@ def test_constitution_seed_composes_wrap_strategy(self, project_dir, temp_dir): assert "# Wrapper Constitution" in content assert "## Core Principle" in content - def test_higher_priority_preset_reseeds_unchanged_generated_constitution( + def test_constitution_follows_priority_when_winning_preset_removed( self, project_dir, temp_dir ): - """An unchanged generated constitution follows resolver priority.""" + """An unchanged generated constitution follows priority and fallback layers.""" manager = PresetManager(project_dir) install_self_test_preset(manager) @@ -2936,6 +2954,10 @@ def test_higher_priority_preset_reseeds_unchanged_generated_constitution( memory = project_dir / ".specify" / "memory" / "constitution.md" assert memory.read_text() == "# Higher Priority Constitution\n" + manager.remove("higher-priority") + + assert "preset:self-test" in memory.read_text() + def test_constitution_seed_rejects_symlinked_memory_directory( self, project_dir, temp_dir ): From d52098a7d278b4936799920652ae809d4d5ff3d1 Mon Sep 17 00:00:00 2001 From: Ben Buttigieg <70525+BenBtg@users.noreply.github.com> Date: Tue, 14 Jul 2026 18:21:27 +0100 Subject: [PATCH 07/10] fix(presets): tighten legacy constitution provenance Trust only the immutable bundled/source constitution template when migrating legacy projects without provenance. Do not infer core provenance from mutable project templates or preset source labels, including IDs beginning with core. Also detect convention-based constitution-template files before preset removal so unchanged generated constitutions reconcile to the next resolver layer. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1b2c095d-b45c-4d52-8d56-bd6121d96ab6 --- src/specify_cli/presets/__init__.py | 25 +++-- tests/test_presets.py | 149 +++++++++++++++++++++++++++- 2 files changed, 160 insertions(+), 14 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index b9c1a711ec..3888cce56b 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -50,7 +50,7 @@ def _content_sha256(content: bytes) -> str: def _constitution_is_generated( project_root: Path, memory_constitution: Path, - layers: list[dict[str, Any]], + resolver: "PresetResolver", ) -> bool: """Return whether the live constitution is an unchanged generated file.""" _ensure_safe_shared_destination(project_root, memory_constitution) @@ -69,12 +69,12 @@ def _constitution_is_generated( ): return True - # Older projects have no provenance sidecar. Only the exact known core - # template is safe to treat as generated; placeholder substrings are not. - for layer in layers: - if layer["source"].startswith("core") and layer["path"].read_bytes() == content: - return True - return False + # Older projects have no provenance sidecar. Only the immutable bundled or + # source-checkout core template is safe to treat as generated. + core = resolver._find_bundled_core( + "constitution-template", "template", ".md" + ) + return core is not None and core.read_bytes() == content def _materialize_constitution_template( @@ -1745,9 +1745,8 @@ def _reconcile_constitution(self) -> None: self.project_root / ".specify" / "memory" / "constitution.md" ) resolver = PresetResolver(self.project_root) - layers = resolver.collect_all_layers("constitution-template", "template") if memory_constitution.exists() and not _constitution_is_generated( - self.project_root, memory_constitution, layers + self.project_root, memory_constitution, resolver ): return _materialize_constitution_template(self.project_root, memory_constitution) @@ -1831,7 +1830,13 @@ def remove(self, pack_id: str) -> bool: # Also include aliases from the manifest as a safety net for registries # populated by older versions that may not track aliases. removed_cmd_names = set() - removed_constitution = False + removed_constitution = any( + path.exists() + for path in ( + pack_dir / "templates" / "constitution-template.md", + pack_dir / "constitution-template.md", + ) + ) for cmd_names in registered_commands.values(): removed_cmd_names.update(cmd_names) manifest_path = pack_dir / "preset.yml" diff --git a/tests/test_presets.py b/tests/test_presets.py index 4b6454b4bb..b1d0070274 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2597,6 +2597,40 @@ def install_self_test_preset(manager: PresetManager, speckit_version: str = "0.1 return manager.install_from_directory(SELF_TEST_PRESET_DIR, speckit_version) +def _make_convention_constitution_preset(temp_dir: Path) -> Path: + """Create a preset whose constitution is found by convention, not its manifest.""" + preset_dir = temp_dir / "convention-constitution" + (preset_dir / "templates").mkdir(parents=True) + (preset_dir / "templates" / "constitution-template.md").write_text( + "# Convention Constitution\n" + ) + (preset_dir / "templates" / "spec-template.md").write_text("# Spec\n") + (preset_dir / "preset.yml").write_text( + yaml.dump( + { + "schema_version": "1.0", + "preset": { + "id": "convention-constitution", + "name": "Convention Constitution", + "version": "1.0.0", + "description": "Convention-based constitution for testing", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [ + { + "type": "template", + "name": "spec-template", + "file": "templates/spec-template.md", + } + ] + }, + } + ) + ) + return preset_dir + + class TestSelfTestPreset: """Tests using the self-test preset that ships with the repo. @@ -2809,13 +2843,15 @@ def test_self_test_seeds_constitution_when_memory_absent(self, project_dir): def test_self_test_reseeds_exact_core_constitution(self, project_dir): """An unchanged core constitution is re-seeded from the preset template.""" - core = "# [PROJECT_NAME] Constitution\n\n### [PRINCIPLE_1_NAME]\n" - (project_dir / ".specify" / "templates" / "constitution-template.md").write_text( - core + resolver = PresetResolver(project_dir) + bundled_core = resolver._find_bundled_core( + "constitution-template", "template", ".md" ) + assert bundled_core is not None + core = bundled_core.read_bytes() memory = project_dir / ".specify" / "memory" / "constitution.md" memory.parent.mkdir(parents=True, exist_ok=True) - memory.write_text(core) + memory.write_bytes(core) manager = PresetManager(project_dir) install_self_test_preset(manager) @@ -2824,6 +2860,69 @@ def test_self_test_reseeds_exact_core_constitution(self, project_dir): assert "preset:self-test" in content, "placeholder constitution was not re-seeded" assert "[PROJECT_NAME]" not in content + def test_self_test_preserves_mutable_project_core_copy(self, project_dir): + """A project template copy does not establish generated provenance.""" + authored = "# Acme Organization Constitution\n\nOrganization policy.\n" + project_template = ( + project_dir / ".specify" / "templates" / "constitution-template.md" + ) + project_template.write_text(authored) + memory = project_dir / ".specify" / "memory" / "constitution.md" + memory.parent.mkdir(parents=True, exist_ok=True) + memory.write_text(authored) + + manager = PresetManager(project_dir) + install_self_test_preset(manager) + + assert memory.read_text() == authored + assert not (memory.parent / ".constitution-template.json").exists() + + def test_core_prefixed_preset_does_not_establish_generated_provenance( + self, project_dir, temp_dir + ): + """A preset ID beginning with core is not an immutable core source.""" + authored = "# Acme Organization Constitution\n\nOrganization policy.\n" + memory = project_dir / ".specify" / "memory" / "constitution.md" + memory.parent.mkdir(parents=True, exist_ok=True) + memory.write_text(authored) + + preset_dir = temp_dir / "core-company" + (preset_dir / "templates").mkdir(parents=True) + (preset_dir / "templates" / "constitution-template.md").write_text(authored) + (preset_dir / "preset.yml").write_text( + yaml.safe_dump( + { + "schema_version": "1.0", + "preset": { + "id": "core-company", + "name": "Core Company", + "version": "1.0.0", + "description": "Company constitution preset", + "author": "Test Author", + "repository": "https://github.com/test/core-company", + "license": "MIT", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [ + { + "type": "template", + "name": "constitution-template", + "file": "templates/constitution-template.md", + "description": "Company constitution", + "replaces": "constitution-template", + } + ] + }, + } + ) + ) + + PresetManager(project_dir).install_from_directory(preset_dir, "0.1.5") + + assert memory.read_text() == authored + assert not (memory.parent / ".constitution-template.json").exists() + def test_self_test_preserves_authored_constitution_with_placeholder( self, project_dir ): @@ -2958,6 +3057,48 @@ def test_constitution_follows_priority_when_winning_preset_removed( assert "preset:self-test" in memory.read_text() + def test_convention_constitution_removal_restores_remaining_layer( + self, project_dir, temp_dir + ): + """Removing a convention layer rematerializes the remaining resolver layer.""" + from specify_cli.commands.init import ensure_constitution_from_template + + manager = PresetManager(project_dir) + install_self_test_preset(manager) + manager.install_from_directory( + _make_convention_constitution_preset(temp_dir), "0.1.5", priority=1 + ) + + memory = project_dir / ".specify" / "memory" / "constitution.md" + memory.unlink() + ensure_constitution_from_template(project_dir) + assert memory.read_text() == "# Convention Constitution\n" + + manager.remove("convention-constitution") + + assert "preset:self-test" in memory.read_text() + + def test_convention_constitution_removal_preserves_edited_content( + self, project_dir, temp_dir + ): + """Removing a convention layer does not overwrite edited generated content.""" + from specify_cli.commands.init import ensure_constitution_from_template + + templates_dir = project_dir / ".specify" / "templates" + (templates_dir / "constitution-template.md").write_text("# Core Constitution\n") + manager = PresetManager(project_dir) + manager.install_from_directory( + _make_convention_constitution_preset(temp_dir), "0.1.5" + ) + ensure_constitution_from_template(project_dir) + memory = project_dir / ".specify" / "memory" / "constitution.md" + edited = memory.read_text() + "\n## Authored amendment\n" + memory.write_text(edited) + + manager.remove("convention-constitution") + + assert memory.read_text() == edited + def test_constitution_seed_rejects_symlinked_memory_directory( self, project_dir, temp_dir ): From 43e1f04825719bde4a3f4e6cec57bb8cab8ab2bd Mon Sep 17 00:00:00 2001 From: Ben Buttigieg <70525+BenBtg@users.noreply.github.com> Date: Tue, 14 Jul 2026 19:37:40 +0100 Subject: [PATCH 08/10] fix(presets): preserve files with invalid provenance Use immutable-core legacy migration only when the provenance sidecar is absent. If a sidecar is malformed or its hash does not match the live constitution, treat the file as edited and preserve it. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1b2c095d-b45c-4d52-8d56-bd6121d96ab6 --- src/specify_cli/presets/__init__.py | 7 +++---- tests/test_presets.py | 30 +++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 3888cce56b..66e3f16d1e 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -62,12 +62,11 @@ def _constitution_is_generated( try: metadata = json.loads(provenance.read_text(encoding="utf-8")) except (json.JSONDecodeError, UnicodeDecodeError): - metadata = {} - if ( + return False + return ( isinstance(metadata, dict) and metadata.get("sha256") == _content_sha256(content) - ): - return True + ) # Older projects have no provenance sidecar. Only the immutable bundled or # source-checkout core template is safe to treat as generated. diff --git a/tests/test_presets.py b/tests/test_presets.py index b1d0070274..f51d700db7 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2860,6 +2860,36 @@ def test_self_test_reseeds_exact_core_constitution(self, project_dir): assert "preset:self-test" in content, "placeholder constitution was not re-seeded" assert "[PROJECT_NAME]" not in content + @pytest.mark.parametrize( + "provenance_content", + [ + '{"sha256": "does-not-match", "source": "old-preset"}\n', + "{not valid json", + ], + ids=["hash-mismatch", "malformed"], + ) + def test_self_test_preserves_core_content_with_existing_invalid_provenance( + self, project_dir, provenance_content + ): + """A present invalid sidecar disables legacy core-template migration.""" + resolver = PresetResolver(project_dir) + bundled_core = resolver._find_bundled_core( + "constitution-template", "template", ".md" + ) + assert bundled_core is not None + memory = project_dir / ".specify" / "memory" / "constitution.md" + memory.parent.mkdir(parents=True, exist_ok=True) + memory.write_bytes(bundled_core.read_bytes()) + (memory.parent / ".constitution-template.json").write_text( + provenance_content + ) + original = memory.read_bytes() + + manager = PresetManager(project_dir) + install_self_test_preset(manager) + + assert memory.read_bytes() == original + def test_self_test_preserves_mutable_project_core_copy(self, project_dir): """A project template copy does not establish generated provenance.""" authored = "# Acme Organization Constitution\n\nOrganization policy.\n" From 0b9affd8bc1931825294f184864d6821d8d5b69f Mon Sep 17 00:00:00 2001 From: Ben Buttigieg <70525+BenBtg@users.noreply.github.com> Date: Tue, 14 Jul 2026 19:52:22 +0100 Subject: [PATCH 09/10] fix(presets): reconcile constitution on stack changes Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1b2c095d-b45c-4d52-8d56-bd6121d96ab6 --- src/specify_cli/presets/__init__.py | 26 +++++++++--- src/specify_cli/presets/_commands.py | 9 ++++ tests/test_presets.py | 62 ++++++++++++++++++++++++++-- 3 files changed, 87 insertions(+), 10 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 66e3f16d1e..ad4785c58d 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -1709,32 +1709,46 @@ def install_from_directory( # constitution) must be propagated here. Guard against clobbering an # already-authored constitution by only replacing a file whose recorded # hash (or exact legacy core-template content) proves it was generated. - self._seed_constitution_from_preset(manifest) + self._seed_constitution_from_preset(manifest, dest_dir) return manifest - def _seed_constitution_from_preset(self, manifest: PresetManifest) -> None: + def _seed_constitution_from_preset( + self, manifest: PresetManifest, preset_dir: Path + ) -> None: """Seed memory/constitution.md from a preset constitution-template. Only runs when the preset declares a ``type: template`` entry named - ``constitution-template`` and the live memory file is either missing or - is an unchanged generated file. Authored constitutions are never - overwritten. + ``constitution-template`` or provides one at a convention path, and the + live memory file is either missing or is an unchanged generated file. + Authored constitutions are never overwritten. """ provides_constitution = any( t.get("type") == "template" and t.get("name") == "constitution-template" for t in manifest.templates + ) or any( + (preset_dir / relative_path).is_file() + for relative_path in ( + "templates/constitution-template.md", + "constitution-template.md", + ) ) if not provides_constitution: return + self.reconcile_constitution( + f"Failed to seed constitution from preset {manifest.id}" + ) + + def reconcile_constitution(self, failure_context: str) -> None: + """Reconcile generated constitution content without failing a persisted change.""" try: self._reconcile_constitution() except (OSError, UnicodeDecodeError, PresetValidationError, ValueError) as exc: import warnings warnings.warn( - f"Failed to seed constitution from preset {manifest.id}: {exc}.", + f"{failure_context}: {exc}.", stacklevel=2, ) diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 0c5632358e..44402831e8 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -484,6 +484,9 @@ def preset_set_priority( # Update priority manager.registry.update(preset_id, {"priority": priority}) + manager.reconcile_constitution( + f"Failed to reconcile constitution after changing priority for preset {preset_id}" + ) console.print(f"[green]✓[/green] Preset '{preset_id}' priority changed: {old_priority} → {priority}") console.print("\n[dim]Lower priority = higher precedence in template resolution[/dim]") @@ -517,6 +520,9 @@ def preset_enable( # Enable the preset manager.registry.update(preset_id, {"enabled": True}) + manager.reconcile_constitution( + f"Failed to reconcile constitution after enabling preset {preset_id}" + ) console.print(f"[green]✓[/green] Preset '{preset_id}' enabled") console.print("\nTemplates from this preset will now be included in resolution.") @@ -551,6 +557,9 @@ def preset_disable( # Disable the preset manager.registry.update(preset_id, {"enabled": False}) + manager.reconcile_constitution( + f"Failed to reconcile constitution after disabling preset {preset_id}" + ) console.print(f"[green]✓[/green] Preset '{preset_id}' disabled") console.print("\nTemplates from this preset will be skipped during resolution.") diff --git a/tests/test_presets.py b/tests/test_presets.py index f51d700db7..05eee58044 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -3091,8 +3091,6 @@ def test_convention_constitution_removal_restores_remaining_layer( self, project_dir, temp_dir ): """Removing a convention layer rematerializes the remaining resolver layer.""" - from specify_cli.commands.init import ensure_constitution_from_template - manager = PresetManager(project_dir) install_self_test_preset(manager) manager.install_from_directory( @@ -3100,8 +3098,6 @@ def test_convention_constitution_removal_restores_remaining_layer( ) memory = project_dir / ".specify" / "memory" / "constitution.md" - memory.unlink() - ensure_constitution_from_template(project_dir) assert memory.read_text() == "# Convention Constitution\n" manager.remove("convention-constitution") @@ -4473,6 +4469,31 @@ def test_set_priority_changes_priority(self, project_dir, pack_dir): manager2 = PresetManager(project_dir) assert manager2.registry.get("test-pack")["priority"] == 5 + def test_set_priority_reconciles_generated_constitution( + self, project_dir, temp_dir + ): + """Changing priority rematerializes an unchanged generated constitution.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + manager = PresetManager(project_dir) + install_self_test_preset(manager) + manager.install_from_directory( + _make_convention_constitution_preset(temp_dir), "0.1.5", priority=20 + ) + memory = project_dir / ".specify" / "memory" / "constitution.md" + assert "preset:self-test" in memory.read_text() + + with patch.object(Path, "cwd", return_value=project_dir): + result = CliRunner().invoke( + app, + ["preset", "set-priority", "convention-constitution", "1"], + ) + + assert result.exit_code == 0, result.output + assert memory.read_text() == "# Convention Constitution\n" + def test_set_priority_same_value_no_change(self, project_dir, pack_dir): """Test set-priority with same value shows already set message.""" from typer.testing import CliRunner @@ -4693,6 +4714,39 @@ def test_enable_preset(self, project_dir, pack_dir): manager2 = PresetManager(project_dir) assert manager2.registry.get("test-pack")["enabled"] is True + def test_enable_disable_reconciles_generated_constitution( + self, project_dir, temp_dir + ): + """Enable and disable rematerialize the winning constitution layer.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + manager = PresetManager(project_dir) + install_self_test_preset(manager) + manager.install_from_directory( + _make_convention_constitution_preset(temp_dir), "0.1.5", priority=1 + ) + memory = project_dir / ".specify" / "memory" / "constitution.md" + assert memory.read_text() == "# Convention Constitution\n" + runner = CliRunner() + + with patch.object(Path, "cwd", return_value=project_dir): + disabled = runner.invoke( + app, ["preset", "disable", "convention-constitution"] + ) + + assert disabled.exit_code == 0, disabled.output + assert "preset:self-test" in memory.read_text() + + with patch.object(Path, "cwd", return_value=project_dir): + enabled = runner.invoke( + app, ["preset", "enable", "convention-constitution"] + ) + + assert enabled.exit_code == 0, enabled.output + assert memory.read_text() == "# Convention Constitution\n" + def test_disable_already_disabled(self, project_dir, pack_dir): """Test disable on already disabled preset shows warning.""" from typer.testing import CliRunner From 4b6b34f021d94db3f27f6a3e3c9ecaa5ffbe8e6e Mon Sep 17 00:00:00 2001 From: Ben Buttigieg <70525+BenBtg@users.noreply.github.com> Date: Tue, 14 Jul 2026 20:05:21 +0100 Subject: [PATCH 10/10] fix(presets): guard constitution reconciliation edges Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1b2c095d-b45c-4d52-8d56-bd6121d96ab6 --- src/specify_cli/presets/__init__.py | 48 +++++++++++++++++-- tests/test_presets.py | 72 +++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index ad4785c58d..c441bf6836 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -76,6 +76,29 @@ def _constitution_is_generated( return core is not None and core.read_bytes() == content +def _constitution_provenance_matches_preset( + project_root: Path, + memory_constitution: Path, + pack_id: str, + pack_version: str, +) -> bool: + """Return whether provenance identifies a preset as the materialized source.""" + provenance = memory_constitution.parent / _CONSTITUTION_PROVENANCE_FILE + if not provenance.parent.exists(): + return False + _ensure_safe_shared_destination(project_root, provenance) + if not provenance.exists(): + return False + try: + metadata = json.loads(provenance.read_text(encoding="utf-8")) + except (json.JSONDecodeError, UnicodeDecodeError): + return False + return ( + isinstance(metadata, dict) + and metadata.get("source") == f"{pack_id} v{pack_version}" + ) + + def _materialize_constitution_template( project_root: Path, memory_constitution: Path, @@ -1737,13 +1760,16 @@ def _seed_constitution_from_preset( return self.reconcile_constitution( - f"Failed to seed constitution from preset {manifest.id}" + f"Failed to seed constitution from preset {manifest.id}", + create_if_missing=True, ) - def reconcile_constitution(self, failure_context: str) -> None: + def reconcile_constitution( + self, failure_context: str, *, create_if_missing: bool = False + ) -> None: """Reconcile generated constitution content without failing a persisted change.""" try: - self._reconcile_constitution() + self._reconcile_constitution(create_if_missing=create_if_missing) except (OSError, UnicodeDecodeError, PresetValidationError, ValueError) as exc: import warnings @@ -1752,11 +1778,13 @@ def reconcile_constitution(self, failure_context: str) -> None: stacklevel=2, ) - def _reconcile_constitution(self) -> None: + def _reconcile_constitution(self, *, create_if_missing: bool = False) -> None: """Materialize the winning constitution layer when the live file is generated.""" memory_constitution = ( self.project_root / ".specify" / "memory" / "constitution.md" ) + if not memory_constitution.exists() and not create_if_missing: + return resolver = PresetResolver(self.project_root) if memory_constitution.exists() and not _constitution_is_generated( self.project_root, memory_constitution, resolver @@ -1850,6 +1878,18 @@ def remove(self, pack_id: str) -> bool: pack_dir / "constitution-template.md", ) ) + if metadata and isinstance(metadata.get("version"), str): + memory_constitution = ( + self.project_root / ".specify" / "memory" / "constitution.md" + ) + removed_constitution = removed_constitution or ( + _constitution_provenance_matches_preset( + self.project_root, + memory_constitution, + pack_id, + metadata["version"], + ) + ) for cmd_names in registered_commands.values(): removed_cmd_names.update(cmd_names) manifest_path = pack_dir / "preset.yml" diff --git a/tests/test_presets.py b/tests/test_presets.py index 05eee58044..70a15f746c 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -3125,6 +3125,56 @@ def test_convention_constitution_removal_preserves_edited_content( assert memory.read_text() == edited + def test_custom_constitution_removal_recovers_with_invalid_manifest( + self, project_dir, temp_dir + ): + """Provenance triggers fallback when a custom-path manifest is invalid.""" + manager = PresetManager(project_dir) + install_self_test_preset(manager) + + preset_dir = temp_dir / "custom-constitution" + (preset_dir / "policy").mkdir(parents=True) + (preset_dir / "policy" / "charter.md").write_text("# Custom Constitution\n") + (preset_dir / "preset.yml").write_text( + yaml.dump( + { + "schema_version": "1.0", + "preset": { + "id": "custom-constitution", + "name": "Custom Constitution", + "version": "1.0.0", + "description": "Custom-path constitution for testing", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [ + { + "type": "template", + "name": "constitution-template", + "file": "policy/charter.md", + } + ] + }, + } + ) + ) + manager.install_from_directory(preset_dir, "0.1.5", priority=1) + memory = project_dir / ".specify" / "memory" / "constitution.md" + assert memory.read_text() == "# Custom Constitution\n" + + installed_manifest = ( + project_dir + / ".specify" + / "presets" + / "custom-constitution" + / "preset.yml" + ) + installed_manifest.write_text("invalid: [") + + manager.remove("custom-constitution") + + assert "preset:self-test" in memory.read_text() + def test_constitution_seed_rejects_symlinked_memory_directory( self, project_dir, temp_dir ): @@ -4747,6 +4797,28 @@ def test_enable_disable_reconciles_generated_constitution( assert enabled.exit_code == 0, enabled.output assert memory.read_text() == "# Convention Constitution\n" + def test_stack_changes_do_not_create_missing_constitution( + self, project_dir, pack_dir + ): + """Stack changes for non-providers do not seed a missing constitution.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + PresetManager(project_dir).install_from_directory(pack_dir, "0.1.5") + memory = project_dir / ".specify" / "memory" / "constitution.md" + runner = CliRunner() + + for args in ( + ["preset", "set-priority", "test-pack", "5"], + ["preset", "disable", "test-pack"], + ["preset", "enable", "test-pack"], + ): + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke(app, args) + assert result.exit_code == 0, result.output + assert not memory.exists() + def test_disable_already_disabled(self, project_dir, pack_dir): """Test disable on already disabled preset shows warning.""" from typer.testing import CliRunner