Skip to content

Commit 116ab2c

Browse files
BenBtgCopilot
andcommitted
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
1 parent e03fb69 commit 116ab2c

9 files changed

Lines changed: 244 additions & 42 deletions

src/specify_cli/presets/__init__.py

Lines changed: 77 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -31,18 +31,50 @@
3131
from .._init_options import is_ai_skills_enabled
3232
from ..integrations.base import IntegrationBase
3333
from .._utils import dump_frontmatter, version_satisfies
34-
from ..shared_infra import verify_archive_sha256
34+
from ..shared_infra import (
35+
_ensure_safe_shared_destination,
36+
_ensure_safe_shared_directory,
37+
_write_shared_bytes,
38+
_write_shared_text,
39+
verify_archive_sha256,
40+
)
3541

3642

37-
# Tokens that mark an unmodified, generic constitution that has not yet been
38-
# authored. Used to decide whether seeding/re-seeding memory/constitution.md
39-
# from a preset-provided template is safe (i.e. won't clobber authored content).
40-
_CONSTITUTION_PLACEHOLDER_TOKENS = ("[PROJECT_NAME]", "[PRINCIPLE_1_NAME]")
43+
_CONSTITUTION_PROVENANCE_FILE = ".constitution-template.json"
4144

4245

43-
def _constitution_is_placeholder(content: str) -> bool:
44-
"""Return True if a constitution body is still the generic placeholder."""
45-
return any(token in content for token in _CONSTITUTION_PLACEHOLDER_TOKENS)
46+
def _content_sha256(content: bytes) -> str:
47+
return hashlib.sha256(content).hexdigest()
48+
49+
50+
def _constitution_is_generated(
51+
project_root: Path,
52+
memory_constitution: Path,
53+
layers: list[dict[str, Any]],
54+
) -> bool:
55+
"""Return whether the live constitution is an unchanged generated file."""
56+
_ensure_safe_shared_destination(project_root, memory_constitution)
57+
content = memory_constitution.read_bytes()
58+
provenance = memory_constitution.parent / _CONSTITUTION_PROVENANCE_FILE
59+
_ensure_safe_shared_destination(project_root, provenance)
60+
61+
if provenance.exists():
62+
try:
63+
metadata = json.loads(provenance.read_text(encoding="utf-8"))
64+
except (json.JSONDecodeError, UnicodeDecodeError):
65+
metadata = {}
66+
if (
67+
isinstance(metadata, dict)
68+
and metadata.get("sha256") == _content_sha256(content)
69+
):
70+
return True
71+
72+
# Older projects have no provenance sidecar. Only the exact known core
73+
# template is safe to treat as generated; placeholder substrings are not.
74+
for layer in layers:
75+
if layer["source"].startswith("core") and layer["path"].read_bytes() == content:
76+
return True
77+
return False
4678

4779

4880
def _materialize_constitution_template(
@@ -61,17 +93,33 @@ def _materialize_constitution_template(
6193
if not layers:
6294
return None
6395

64-
memory_constitution.parent.mkdir(parents=True, exist_ok=True)
6596
top_layer = layers[0]
6697
if top_layer["strategy"] == "replace":
67-
shutil.copy2(top_layer["path"], memory_constitution)
68-
return "copied"
69-
70-
composed_content = resolver.resolve_content("constitution-template", "template")
71-
if composed_content is None:
72-
return None
73-
memory_constitution.write_text(composed_content, encoding="utf-8")
74-
return "composed"
98+
content = top_layer["path"].read_bytes()
99+
result = "copied"
100+
else:
101+
composed_content = resolver.resolve_content("constitution-template", "template")
102+
if composed_content is None:
103+
return None
104+
content = composed_content.encode("utf-8")
105+
result = "composed"
106+
107+
_ensure_safe_shared_directory(project_root, memory_constitution.parent)
108+
_write_shared_bytes(project_root, memory_constitution, content)
109+
provenance = memory_constitution.parent / _CONSTITUTION_PROVENANCE_FILE
110+
_write_shared_text(
111+
project_root,
112+
provenance,
113+
json.dumps(
114+
{
115+
"sha256": _content_sha256(content),
116+
"source": top_layer["source"],
117+
},
118+
indent=2,
119+
)
120+
+ "\n",
121+
)
122+
return result
75123

76124

77125
def _substitute_core_template(
@@ -1660,8 +1708,8 @@ def install_from_directory(
16601708
# materialized to a live file rather than resolved on demand, so a
16611709
# preset that ships one (e.g. strategy: replace with a ratified
16621710
# constitution) must be propagated here. Guard against clobbering an
1663-
# already-authored constitution by only seeding when the memory file is
1664-
# missing or still contains generic placeholder tokens.
1711+
# already-authored constitution by only replacing a file whose recorded
1712+
# hash (or exact legacy core-template content) proves it was generated.
16651713
self._seed_constitution_from_preset(manifest)
16661714

16671715
return manifest
@@ -1671,7 +1719,7 @@ def _seed_constitution_from_preset(self, manifest: PresetManifest) -> None:
16711719
16721720
Only runs when the preset declares a ``type: template`` entry named
16731721
``constitution-template`` and the live memory file is either missing or
1674-
still the generic placeholder. Authored constitutions are never
1722+
is an unchanged generated file. Authored constitutions are never
16751723
overwritten.
16761724
"""
16771725
provides_constitution = any(
@@ -1684,22 +1732,21 @@ def _seed_constitution_from_preset(self, manifest: PresetManifest) -> None:
16841732
memory_constitution = (
16851733
self.project_root / ".specify" / "memory" / "constitution.md"
16861734
)
1687-
if memory_constitution.exists():
1688-
try:
1689-
existing = memory_constitution.read_text(encoding="utf-8")
1690-
except OSError:
1691-
return
1692-
if not _constitution_is_placeholder(existing):
1693-
# Legitimately authored constitution; leave it untouched.
1694-
return
1695-
16961735
try:
1736+
resolver = PresetResolver(self.project_root)
1737+
layers = resolver.collect_all_layers(
1738+
"constitution-template", "template"
1739+
)
1740+
if memory_constitution.exists() and not _constitution_is_generated(
1741+
self.project_root, memory_constitution, layers
1742+
):
1743+
return
16971744
result = _materialize_constitution_template(
16981745
self.project_root, memory_constitution
16991746
)
17001747
if result is None:
17011748
return
1702-
except OSError as exc:
1749+
except (OSError, UnicodeDecodeError, PresetValidationError, ValueError) as exc:
17031750
import warnings
17041751

17051752
warnings.warn(

tests/integrations/test_integration_base_markdown.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ def _expected_files(self, script_variant: str) -> list[str]:
253253
"spec-template.md", "tasks-template.md"]:
254254
files.append(f".specify/templates/{name}")
255255

256+
files.append(".specify/memory/.constitution-template.json")
256257
files.append(".specify/memory/constitution.md")
257258
# Bundled workflow
258259
files.append(".specify/workflows/speckit/workflow.yml")

tests/integrations/test_integration_base_skills.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,7 @@ def _expected_files(self, script_variant: str) -> list[str]:
399399
".specify/integration.json",
400400
f".specify/integrations/{self.KEY}.manifest.json",
401401
".specify/integrations/speckit.manifest.json",
402+
".specify/memory/.constitution-template.json",
402403
".specify/memory/constitution.md",
403404
]
404405
# Script variant

tests/integrations/test_integration_base_toml.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,7 @@ def _expected_files(self, script_variant: str) -> list[str]:
517517
]:
518518
files.append(f".specify/templates/{name}")
519519

520+
files.append(".specify/memory/.constitution-template.json")
520521
files.append(".specify/memory/constitution.md")
521522
# Bundled workflow
522523
files.append(".specify/workflows/speckit/workflow.yml")

tests/integrations/test_integration_base_yaml.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,7 @@ def _expected_files(self, script_variant: str) -> list[str]:
401401
]:
402402
files.append(f".specify/templates/{name}")
403403

404+
files.append(".specify/memory/.constitution-template.json")
404405
files.append(".specify/memory/constitution.md")
405406
# Bundled workflow
406407
files.append(".specify/workflows/speckit/workflow.yml")

tests/integrations/test_integration_cline.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ def _expected_files(self, script_variant: str) -> list[str]:
214214
]:
215215
files.append(f".specify/templates/{name}")
216216

217+
files.append(".specify/memory/.constitution-template.json")
217218
files.append(".specify/memory/constitution.md")
218219
# Bundled workflow
219220
files.append(".specify/workflows/speckit/workflow.yml")

tests/integrations/test_integration_copilot.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ def test_complete_file_inventory_sh(self, tmp_path):
252252
".specify/templates/plan-template.md",
253253
".specify/templates/spec-template.md",
254254
".specify/templates/tasks-template.md",
255+
".specify/memory/.constitution-template.json",
255256
".specify/memory/constitution.md",
256257
".specify/workflows/speckit/workflow.yml",
257258
".specify/workflows/workflow-registry.json",
@@ -313,6 +314,7 @@ def test_complete_file_inventory_ps(self, tmp_path):
313314
".specify/templates/plan-template.md",
314315
".specify/templates/spec-template.md",
315316
".specify/templates/tasks-template.md",
317+
".specify/memory/.constitution-template.json",
316318
".specify/memory/constitution.md",
317319
".specify/workflows/speckit/workflow.yml",
318320
".specify/workflows/workflow-registry.json",
@@ -724,6 +726,7 @@ def test_complete_file_inventory_skills_sh(self, tmp_path):
724726
".specify/templates/plan-template.md",
725727
".specify/templates/spec-template.md",
726728
".specify/templates/tasks-template.md",
729+
".specify/memory/.constitution-template.json",
727730
".specify/memory/constitution.md",
728731
# Bundled workflow
729732
".specify/workflows/speckit/workflow.yml",

tests/integrations/test_integration_generic.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,7 @@ def test_complete_file_inventory_sh(self, tmp_path):
286286
".specify/integration.json",
287287
".specify/integrations/generic.manifest.json",
288288
".specify/integrations/speckit.manifest.json",
289+
".specify/memory/.constitution-template.json",
289290
".specify/memory/constitution.md",
290291
".specify/scripts/bash/check-prerequisites.sh",
291292
".specify/scripts/bash/common.sh",
@@ -342,6 +343,7 @@ def test_complete_file_inventory_ps(self, tmp_path):
342343
".specify/integration.json",
343344
".specify/integrations/generic.manifest.json",
344345
".specify/integrations/speckit.manifest.json",
346+
".specify/memory/.constitution-template.json",
345347
".specify/memory/constitution.md",
346348
".specify/scripts/powershell/check-prerequisites.ps1",
347349
".specify/scripts/powershell/common.ps1",

0 commit comments

Comments
 (0)