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] 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" + )