From 19b797425353e66b5efb003bdf1afc287f57f742 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:31:35 +0000 Subject: [PATCH] Fix preset-constitution-not-installed: resolver-aware seeding + reseed hook Apply the remediation from the bug assessment on issue #3272. Three-part fix: 1. Make ensure_constitution_from_template resolver-aware (init.py): replace the hardcoded shutil.copy2 from the core template path with a call to PresetResolver(project_path).resolve('constitution-template', 'template'). The resolver walks the full preset priority stack so a preset-provided constitution-template is picked up automatically. Falls back to the core template when no preset overrides it, preserving existing behaviour for projects without presets. 2. Reorder the init flow (init.py): move the call to ensure_constitution_from_template from before the preset installation block to after it. For 'specify init --preset', the memory file is now seeded from the already-resolved template stack instead of from the generic core template that existed before the preset arrived. 3. Add guarded re-seed hook in install_from_directory (presets/__init__.py): _maybe_reseed_constitution is called at the end of install_from_directory and re-seeds .specify/memory/constitution.md from the preset's resolved constitution-template, but only when (a) the manifest provides a constitution-template entry AND (b) the current memory file still contains generic placeholder tokens ([PROJECT_NAME] or [PRINCIPLE_1_NAME]). Legitimately authored constitutions (no placeholder tokens) are never overwritten. Also adds a _CONSTITUTION_PLACEHOLDER_TOKENS module-level constant and seven regression tests covering: - ensure_constitution_from_template picks up a preset override via resolver - falls back to core template when no preset is installed - skips existing memory files - install_from_directory re-seeds a generic memory constitution - install_from_directory does not overwrite an authored constitution - install_from_directory skips re-seed for presets without constitution-template - self-test preset re-seeds a generic memory constitution on install 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 | 35 +++- src/specify_cli/presets/__init__.py | 51 ++++++ tests/test_presets.py | 237 ++++++++++++++++++++++++++++ 3 files changed, 315 insertions(+), 8 deletions(-) diff --git a/src/specify_cli/commands/init.py b/src/specify_cli/commands/init.py index dd815b8c5..60ea90c2d 100644 --- a/src/specify_cli/commands/init.py +++ b/src/specify_cli/commands/init.py @@ -33,11 +33,13 @@ 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 full preset priority stack so that a + preset-provided constitution-template is used when one is installed. + Falls back to the core template when no preset overrides it. + """ memory_constitution = project_path / ".specify" / "memory" / "constitution.md" - template_constitution = ( - project_path / ".specify" / "templates" / "constitution-template.md" - ) if memory_constitution.exists(): if tracker: @@ -45,7 +47,24 @@ def ensure_constitution_from_template( tracker.skip("constitution", "existing file preserved") return - if not template_constitution.exists(): + # Resolve through the preset priority stack so installed presets are honoured. + resolved_template: Path | None = None + try: + from ..presets import PresetResolver + resolved_template = PresetResolver(project_path).resolve( + "constitution-template", "template" + ) + except Exception: + pass + + if resolved_template is None: + # Direct fallback: core template path (handles bare test environments + # where the preset system is unavailable). + core_path = project_path / ".specify" / "templates" / "constitution-template.md" + if core_path.exists(): + resolved_template = core_path + + if resolved_template is None: if tracker: tracker.add("constitution", "Constitution setup") tracker.error("constitution", "template not found") @@ -53,7 +72,7 @@ def ensure_constitution_from_template( try: memory_constitution.parent.mkdir(parents=True, exist_ok=True) - shutil.copy2(template_constitution, memory_constitution) + shutil.copy2(resolved_template, memory_constitution) if tracker: tracker.add("constitution", "Constitution setup") tracker.complete("constitution", "copied from template") @@ -447,8 +466,6 @@ def register(app: typer.Typer) -> None: "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 +593,8 @@ def register(app: typer.Typer) -> None: 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 863b6ef7d..1c02ebf47 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -541,6 +541,12 @@ class PresetRegistry: return pack_id in packs +# Placeholder tokens present in the generic upstream constitution template. +# Used to detect whether a memory constitution has been legitimately authored +# or is still the unmodified placeholder shipped with Spec Kit. +_CONSTITUTION_PLACEHOLDER_TOKENS = ("[PROJECT_NAME]", "[PRINCIPLE_1_NAME]") + + class PresetManager: """Manages preset lifecycle: installation, removal, updates.""" @@ -1498,6 +1504,49 @@ class PresetManager: # No core or extension template — remove the skill entirely shutil.rmtree(skill_subdir) + def _maybe_reseed_constitution(self, manifest: PresetManifest) -> None: + """Re-seed the memory constitution from the resolved template if it is still generic. + + Acts only when all of the following hold: + 1. The manifest provides a ``constitution-template`` entry (any strategy). + 2. ``.specify/memory/constitution.md`` exists in the project. + 3. The memory file still contains generic placeholder tokens, meaning it + has not been legitimately authored by a user or agent. + + Legitimately authored constitutions (no placeholder tokens) are never + overwritten, so running ``specify preset add`` on a project whose + constitution has been filled in is always safe. + """ + has_constitution_template = any( + t.get("name") == "constitution-template" + for t in manifest.templates + if t.get("type") == "template" + ) + if not has_constitution_template: + return + + memory_path = self.project_root / ".specify" / "memory" / "constitution.md" + if not memory_path.exists(): + return + + try: + content = memory_path.read_text(encoding="utf-8") + except OSError: + return + + if not any(token in content for token in _CONSTITUTION_PLACEHOLDER_TOKENS): + return + + resolver = PresetResolver(self.project_root) + resolved = resolver.resolve("constitution-template", "template") + if resolved is None: + return + + try: + shutil.copy2(resolved, memory_path) + except OSError: + pass + def install_from_directory( self, source_dir: Path, @@ -1615,6 +1664,8 @@ class PresetManager: stacklevel=2, ) + self._maybe_reseed_constitution(manifest) + return manifest def install_from_zip( diff --git a/tests/test_presets.py b/tests/test_presets.py index 054018b7a..dbeaab056 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2863,6 +2863,243 @@ class TestSelfTestPreset: assert cmd_file.exists(), "Skill not registered despite extension being present" +# ===== Constitution Re-seed Tests ===== + + +class TestConstitutionReseed: + """Tests for preset-aware constitution seeding. + + Covers the two-part fix for issue #3272: + 1. ``ensure_constitution_from_template`` uses PresetResolver so a preset + constitution-template is picked up. + 2. ``install_from_directory`` re-seeds the memory constitution when adding + a preset to an existing project that still has the generic placeholder. + """ + + def _make_constitution_preset(self, temp_dir: Path, preset_id: str, content: str) -> Path: + """Build a minimal preset directory that provides a constitution-template.""" + preset_dir = temp_dir / preset_id + preset_dir.mkdir() + templates_dir = preset_dir / "templates" + templates_dir.mkdir() + (templates_dir / "constitution-template.md").write_text(content, encoding="utf-8") + manifest_data = { + "schema_version": "1.0", + "preset": { + "id": preset_id, + "name": preset_id.title(), + "version": "1.0.0", + "description": "Test preset with constitution template", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [ + { + "type": "template", + "name": "constitution-template", + "file": "templates/constitution-template.md", + "description": "Ratified constitution", + "replaces": "constitution-template", + "strategy": "replace", + } + ] + }, + } + with open(preset_dir / "preset.yml", "w") as f: + yaml.dump(manifest_data, f) + return preset_dir + + def test_ensure_constitution_uses_resolver_when_preset_installed( + self, project_dir: Path, temp_dir: Path + ) -> None: + """ensure_constitution_from_template seeds from a preset's constitution-template.""" + from specify_cli.commands.init import ensure_constitution_from_template + + # Create the core template (generic placeholder) + 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## [PRINCIPLE_1_NAME]\n", encoding="utf-8" + ) + + # Install a preset that provides a ratified constitution + preset_content = "# Ratified Constitution\n## Core Principle\nDo the right thing.\n" + preset_dir = self._make_constitution_preset(temp_dir, "org-constitution", preset_content) + manager = PresetManager(project_dir) + manager.install_from_directory(preset_dir, "0.1.0") + + # Ensure there is no existing memory constitution + memory_path = project_dir / ".specify" / "memory" / "constitution.md" + assert not memory_path.exists() + + # Call the function — it should pick up the preset's version + ensure_constitution_from_template(project_dir) + + assert memory_path.exists(), "memory constitution should have been created" + content = memory_path.read_text(encoding="utf-8") + assert "Ratified Constitution" in content, ( + "memory constitution should contain the preset's content, not the generic placeholder" + ) + assert "[PROJECT_NAME]" not in content + + def test_ensure_constitution_falls_back_to_core_without_preset( + self, project_dir: Path + ) -> None: + """ensure_constitution_from_template uses the core template when no preset overrides it.""" + from specify_cli.commands.init import ensure_constitution_from_template + + 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## [PRINCIPLE_1_NAME]\n", encoding="utf-8" + ) + + memory_path = project_dir / ".specify" / "memory" / "constitution.md" + assert not memory_path.exists() + + ensure_constitution_from_template(project_dir) + + assert memory_path.exists() + content = memory_path.read_text(encoding="utf-8") + assert "[PROJECT_NAME]" in content # fell back to generic core template + + def test_ensure_constitution_skips_existing_file(self, project_dir: Path) -> None: + """ensure_constitution_from_template does not overwrite an existing memory file.""" + from specify_cli.commands.init import ensure_constitution_from_template + + templates_dir = project_dir / ".specify" / "templates" + templates_dir.mkdir(parents=True, exist_ok=True) + (templates_dir / "constitution-template.md").write_text("# Template\n") + + memory_dir = project_dir / ".specify" / "memory" + memory_dir.mkdir(parents=True, exist_ok=True) + memory_path = memory_dir / "constitution.md" + memory_path.write_text("# My Authored Constitution\n", encoding="utf-8") + + ensure_constitution_from_template(project_dir) + + assert memory_path.read_text(encoding="utf-8") == "# My Authored Constitution\n" + + def test_install_from_directory_reseeds_generic_constitution( + self, project_dir: Path, temp_dir: Path + ) -> None: + """install_from_directory re-seeds a generic memory constitution from the preset.""" + # Seed the memory constitution with the generic placeholder + memory_dir = project_dir / ".specify" / "memory" + memory_dir.mkdir(parents=True, exist_ok=True) + memory_path = memory_dir / "constitution.md" + memory_path.write_text( + "# [PROJECT_NAME] Constitution\n## [PRINCIPLE_1_NAME]\n", encoding="utf-8" + ) + + preset_content = "# Ratified Constitution\n## Article I\nDo no harm.\n" + preset_dir = self._make_constitution_preset(temp_dir, "org-const2", preset_content) + + manager = PresetManager(project_dir) + manager.install_from_directory(preset_dir, "0.1.0") + + content = memory_path.read_text(encoding="utf-8") + assert "Ratified Constitution" in content, ( + "memory constitution should be re-seeded from the preset after install" + ) + assert "[PROJECT_NAME]" not in content + + def test_install_from_directory_does_not_overwrite_authored_constitution( + self, project_dir: Path, temp_dir: Path + ) -> None: + """install_from_directory leaves an authored constitution (no placeholders) intact.""" + # Seed the memory constitution with authored content (no placeholder tokens) + memory_dir = project_dir / ".specify" / "memory" + memory_dir.mkdir(parents=True, exist_ok=True) + memory_path = memory_dir / "constitution.md" + authored_content = "# Acme Corp Constitution\n## Core Principle\nInnovate responsibly.\n" + memory_path.write_text(authored_content, encoding="utf-8") + + preset_content = "# Preset Constitution\n## Article I\nPreset principle.\n" + preset_dir = self._make_constitution_preset(temp_dir, "org-const3", preset_content) + + manager = PresetManager(project_dir) + manager.install_from_directory(preset_dir, "0.1.0") + + assert memory_path.read_text(encoding="utf-8") == authored_content, ( + "authored constitution should not be overwritten by preset install" + ) + + def test_install_from_directory_skips_reseed_without_constitution_template( + self, project_dir: Path, temp_dir: Path + ) -> None: + """install_from_directory does not touch the memory file for presets lacking constitution-template.""" + memory_dir = project_dir / ".specify" / "memory" + memory_dir.mkdir(parents=True, exist_ok=True) + memory_path = memory_dir / "constitution.md" + memory_path.write_text( + "# [PROJECT_NAME] Constitution\n## [PRINCIPLE_1_NAME]\n", encoding="utf-8" + ) + original_content = memory_path.read_text(encoding="utf-8") + + # Preset that provides a spec-template only (no constitution-template) + preset_dir = temp_dir / "no-const-preset" + preset_dir.mkdir() + templates_dir = preset_dir / "templates" + templates_dir.mkdir() + (templates_dir / "spec-template.md").write_text("# Custom spec\n") + manifest_data = { + "schema_version": "1.0", + "preset": { + "id": "no-const-preset", + "name": "No Constitution Preset", + "version": "1.0.0", + "description": "Preset without constitution-template", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [ + { + "type": "template", + "name": "spec-template", + "file": "templates/spec-template.md", + "description": "Custom spec template", + "replaces": "spec-template", + } + ] + }, + } + with open(preset_dir / "preset.yml", "w") as f: + yaml.dump(manifest_data, f) + + manager = PresetManager(project_dir) + manager.install_from_directory(preset_dir, "0.1.0") + + assert memory_path.read_text(encoding="utf-8") == original_content + + def test_self_test_preset_reseeds_generic_constitution_on_install( + self, project_dir: Path + ) -> None: + """Installing the self-test preset re-seeds a generic memory constitution.""" + # Seed the memory constitution with the generic placeholder + memory_dir = project_dir / ".specify" / "memory" + memory_dir.mkdir(parents=True, exist_ok=True) + memory_path = memory_dir / "constitution.md" + memory_path.write_text( + "# [PROJECT_NAME] Constitution\n## [PRINCIPLE_1_NAME]\n", encoding="utf-8" + ) + + 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## [PRINCIPLE_1_NAME]\n" + ) + + manager = PresetManager(project_dir) + install_self_test_preset(manager) + + content = memory_path.read_text(encoding="utf-8") + assert "preset:self-test" in content, ( + "memory constitution should be re-seeded from the self-test preset" + ) + assert "[PROJECT_NAME]" not in content + + # ===== Init Options and Skills Tests =====