mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
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>
This commit is contained in:
github-actions[bot]
committed by
GitHub
GitHub
parent
c34a505d1c
commit
19b7974253
@@ -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
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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 =====
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user