[bug-fix] preset-constitution-not-installed: resolver-aware seeding + post-install reseed#3298
Draft
github-actions[bot] wants to merge 1 commit into
Draft
Conversation
…d 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>
|
|
||
| try: | ||
| shutil.copy2(resolved, memory_path) | ||
| except OSError: |
| resolved_template = PresetResolver(project_path).resolve( | ||
| "constitution-template", "template" | ||
| ) | ||
| except Exception: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug fix — preset-constitution-not-installed
Proposed fix for issue #3272, applying the remediation from the bug assessment.
Verdict: valid · Severity: high
Summary
ensure_constitution_from_templatepreviously hardcoded a copy from the core template, bypassingPresetResolverentirely and always seeding.specify/memory/constitution.mdwith the generic upstream placeholder. This change makes constitution seeding preset-aware in two places (init flow andpreset add), so a preset that provides a ratifiedconstitution-templateautomatically lands in the project's memory file.Changes
src/specify_cli/commands/init.pyensure_constitution_from_templatenow resolves viaPresetResolver; call moved after preset installation blocksrc/specify_cli/presets/__init__.py_CONSTITUTION_PLACEHOLDER_TOKENSconstant and_maybe_reseed_constitutionmethod; called at end ofinstall_from_directorytests/test_presets.pyTestConstitutionReseedclassTests Added or Updated
TestConstitutionReseed::test_ensure_constitution_uses_resolver_when_preset_installed— verifiesensure_constitution_from_templatepicks up the preset's constitution-template viaPresetResolverTestConstitutionReseed::test_ensure_constitution_falls_back_to_core_without_preset— verifies fallback to core template when no preset overrides itTestConstitutionReseed::test_ensure_constitution_skips_existing_file— verifies existing memory files are never overwrittenTestConstitutionReseed::test_install_from_directory_reseeds_generic_constitution— verifiesinstall_from_directoryre-seeds a generic (placeholder-containing) memory constitution from the presetTestConstitutionReseed::test_install_from_directory_does_not_overwrite_authored_constitution— verifies authored constitutions (no placeholder tokens) are left untouchedTestConstitutionReseed::test_install_from_directory_skips_reseed_without_constitution_template— verifies presets that don't provideconstitution-templatedon't touch the memory fileTestConstitutionReseed::test_self_test_preset_reseeds_generic_constitution_on_install— end-to-end test using the self-test presetLocal Verification
python3 -c "import ast; ast.parse(open(f).read())"on all three files → OKuv/venvinstalled at workflow time); verified by code inspection and AST parsing.Deviations from Assessment
None. The implementation follows all three parts of the preferred remediation exactly:
PresetResolverused inensure_constitution_from_template✅install_from_directoryvia_maybe_reseed_constitution✅The guard condition uses both
[PROJECT_NAME]and[PRINCIPLE_1_NAME]as placeholder tokens, as recommended in the assessment's Risks & Considerations.Risks & Review Notes
[PROJECT_NAME]or[PRINCIPLE_1_NAME]as literal text. The conservative dual-token check (any(token in content for token in ...)) reduces but cannot eliminate this risk — a single stray[PROJECT_NAME]in an otherwise complete constitution would still trigger a re-seed.ensure_constitution_from_templateafter the preset block does not affect init runs without a--presetargument (no-op change for the common case).PresetResolveris now imported lazily insideensure_constitution_from_templatewith atry/except Exceptionguard, keeping the fallback to direct core-path lookup for environments where the preset system is unavailable.Refs #3272 · cc
@PechiSW