mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
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>
This commit is contained in:
@@ -33,20 +33,19 @@ def _stdin_is_interactive() -> bool:
|
|||||||
def ensure_constitution_from_template(
|
def ensure_constitution_from_template(
|
||||||
project_path: Path, tracker: StepTracker | None = None
|
project_path: Path, tracker: StepTracker | None = 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
|
Resolution walks the full priority stack (project overrides → installed
|
||||||
presets → extensions → core) via :class:`PresetResolver`, so a preset that
|
presets → extensions → core) via :class:`PresetResolver`, so a preset that
|
||||||
ships a ``constitution-template`` (e.g. ``strategy: replace`` with a ratified
|
ships a ``constitution-template`` (e.g. ``strategy: replace`` with a ratified
|
||||||
constitution) seeds the memory file verbatim. When nothing overrides it, the
|
constitution) can seed the memory file. When nothing overrides it, the
|
||||||
resolver falls through to the core template, preserving legacy behavior.
|
resolver falls through to the core template.
|
||||||
"""
|
"""
|
||||||
from ..presets import PresetResolver
|
from ..presets import PresetResolver
|
||||||
|
|
||||||
memory_constitution = project_path / ".specify" / "memory" / "constitution.md"
|
memory_constitution = project_path / ".specify" / "memory" / "constitution.md"
|
||||||
template_constitution = PresetResolver(project_path).resolve(
|
resolver = PresetResolver(project_path)
|
||||||
"constitution-template", "template"
|
layers = resolver.collect_all_layers("constitution-template", "template")
|
||||||
)
|
|
||||||
|
|
||||||
if memory_constitution.exists():
|
if memory_constitution.exists():
|
||||||
if tracker:
|
if tracker:
|
||||||
@@ -54,7 +53,7 @@ def ensure_constitution_from_template(
|
|||||||
tracker.skip("constitution", "existing file preserved")
|
tracker.skip("constitution", "existing file preserved")
|
||||||
return
|
return
|
||||||
|
|
||||||
if template_constitution is None or not template_constitution.exists():
|
if not layers:
|
||||||
if tracker:
|
if tracker:
|
||||||
tracker.add("constitution", "Constitution setup")
|
tracker.add("constitution", "Constitution setup")
|
||||||
tracker.error("constitution", "template not found")
|
tracker.error("constitution", "template not found")
|
||||||
@@ -62,7 +61,16 @@ def ensure_constitution_from_template(
|
|||||||
|
|
||||||
try:
|
try:
|
||||||
memory_constitution.parent.mkdir(parents=True, exist_ok=True)
|
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:
|
if tracker:
|
||||||
tracker.add("constitution", "Constitution setup")
|
tracker.add("constitution", "Constitution setup")
|
||||||
tracker.complete("constitution", "copied from template")
|
tracker.complete("constitution", "copied from template")
|
||||||
|
|||||||
@@ -1664,15 +1664,23 @@ class PresetManager:
|
|||||||
# Legitimately authored constitution; leave it untouched.
|
# Legitimately authored constitution; leave it untouched.
|
||||||
return
|
return
|
||||||
|
|
||||||
resolved = PresetResolver(self.project_root).resolve(
|
resolver = PresetResolver(self.project_root)
|
||||||
"constitution-template", "template"
|
layers = resolver.collect_all_layers("constitution-template", "template")
|
||||||
)
|
if not layers:
|
||||||
if resolved is None or not resolved.exists():
|
|
||||||
return
|
return
|
||||||
|
|
||||||
try:
|
try:
|
||||||
memory_constitution.parent.mkdir(parents=True, exist_ok=True)
|
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:
|
except OSError as exc:
|
||||||
import warnings
|
import warnings
|
||||||
|
|
||||||
|
|||||||
@@ -2827,6 +2827,66 @@ class TestSelfTestPreset:
|
|||||||
assert result is not None
|
assert result is not None
|
||||||
assert "preset:self-test" in result.read_text()
|
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):
|
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."""
|
"""Test that extension command overrides are skipped if the extension isn't installed."""
|
||||||
claude_dir = project_dir / ".claude" / "skills"
|
claude_dir = project_dir / ".claude" / "skills"
|
||||||
@@ -6219,6 +6279,39 @@ class TestEnsureConstitutionResolverAware:
|
|||||||
"# [PROJECT_NAME] Constitution\n\n### [PRINCIPLE_1_NAME]\n"
|
"# [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):
|
def test_seeds_from_core_when_no_preset(self, project_dir):
|
||||||
from specify_cli.commands.init import ensure_constitution_from_template
|
from specify_cli.commands.init import ensure_constitution_from_template
|
||||||
|
|
||||||
@@ -6260,3 +6353,20 @@ class TestEnsureConstitutionResolverAware:
|
|||||||
ensure_constitution_from_template(project_dir)
|
ensure_constitution_from_template(project_dir)
|
||||||
|
|
||||||
assert memory.read_text() == authored
|
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
|
||||||
|
|||||||
Reference in New Issue
Block a user