feat(presets): resolve constitution templates at command time - #3984
Conversation
Gate install-time constitution materialization behind the constitution-sync preset while preserving one-time init seeding and authored-file safeguards. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7dbce70f-80c6-4e14-a30d-78cb358bcb84
There was a problem hiding this comment.
🟡 Not ready to approve
The referenced CLI command does not provide composed template content for /constitution to consume.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Moves constitution templates to runtime resolution while retaining optional materialization through constitution-sync.
Changes:
- Gates install-time constitution reconciliation behind
constitution-sync. - Updates
/constitutionguidance and lifecycle documentation. - Expands tests for default and opt-in behavior.
File summaries
| File | Description |
|---|---|
src/specify_cli/presets/__init__.py |
Gates constitution materialization. |
templates/commands/constitution.md |
Adds runtime-resolution workflow. |
tests/test_presets.py |
Covers lifecycle behavior. |
presets/README.md |
Documents default behavior. |
presets/ARCHITECTURE.md |
Describes constitution lifecycle. |
presets/constitution-sync/README.md |
Documents opt-in materialization. |
presets/constitution-sync/preset.yml |
Updates preset description. |
presets/catalog.json |
Updates catalog metadata. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Add a machine-readable preset resolve mode backed by PresetResolver.resolve_content and require the constitution command to consume it. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7dbce70f-80c6-4e14-a30d-78cb358bcb84
|
Addressed the command-time composition review in Posted on behalf of @mnriem by GitHub Copilot (model: GPT-5.6 Sol). |
There was a problem hiding this comment.
🟡 Not ready to approve
The new content endpoint permits path traversal through an unvalidated template name.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 1
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7dbce70f-80c6-4e14-a30d-78cb358bcb84
|
Posted on behalf of @mnriem by GitHub Copilot (model: GPT-5.6 Sol). Commit The change preserves the existing diagnostic |
There was a problem hiding this comment.
🟡 Not ready to approve
Runtime resolution has path-traversal risks and diverges from the canonical extension and convention stack.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (4)
scripts/python/common.py:343
- This extension tier is ordered alphabetically and includes every directory, ignoring
.specify/extensions/.registry. Consequently a disabled extension can still supplyconstitution-template, and registered extension priorities are not honored, so/constitutiondoes not actually use the same full stack asPresetResolver._get_all_extensions_by_priority(). All three runtime resolvers need registry-aware extension filtering and ordering.
extensions_dir = repo_root / ".specify" / "extensions"
try:
extension_dirs = sorted(
path
for path in extensions_dir.iterdir()
if path.is_dir() and not path.name.startswith(".")
scripts/python/common.py:267
- The canonical
PresetResolversearches bothtemplates/<name>.mdand the preset-root<name>.mdconvention, and the install-time constitution code explicitly recognizes both. This helper only checks thetemplates/convention, so/constitutionsilently falls through past a valid root-level preset template. Preserve both convention paths in every runtime resolver.
This issue also appears on line 338 of the same file.
manifest_path = preset_dir / "preset.yml"
conventional = preset_dir / "templates" / f"{template_name}.md"
scripts/bash/common.sh:612
- This fallback only checks
templates/$TemplateName.md, while the canonical resolver and constitution seeding also support a preset-root$TemplateName.md. Such a convention-based layer will be ignored at command time, making the Bash result differ from initialization. Add the root-level fallback and keep all script variants aligned.
if [ -z "$candidate" ] && [ "$manifest_declared" = false ]; then
local cf="$presets_dir/$preset_id/templates/${template_name}.md"
[ -f "$cf" ] && candidate="$cf"
scripts/powershell/common.ps1:594
- This convention fallback omits the supported preset-root
$TemplateName.mdlocation. A constitution seeded from that location byPresetResolvercan therefore resolve to a different lower layer on the next/constitutionrun. Check both convention locations consistently across script variants.
if (-not $candidate -and -not $manifestDeclared) {
$cf = Join-Path $presetsDir "$presetId/templates/$TemplateName.md"
if (Test-Path $cf) { $candidate = $cf }
- Files reviewed: 43/43 changed files
- Comments generated: 2
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Align runtime resolution across script variants, validate registry path components, and honor canonical extension ordering and convention paths. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Addressed the review feedback in commit Posted on behalf of @mnriem by GitHub Copilot (model: GPT-5.6 Sol). |
There was a problem hiding this comment.
🟡 Not ready to approve
Runtime priority ordering can diverge from the canonical resolver, and Python file generation changes line endings on Windows.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (6)
scripts/powershell/common.ps1:569
- The canonical registry sorts equal priorities alphabetically by preset ID, but this sort uses priority alone. Since priority 10 is the default, installing the same presets in different orders can produce a different winning/composed runtime template than
PresetResolver. AddNameas the secondary sort key.
Sort-Object { & $priorityFor $_ } |
scripts/python/setup_plan.py:71
Path.write_text()performs platform newline translation, so on Windows the Python variant changes LF template content to CRLF. The previousshutil.copy()and the Bash/PowerShell variants preserve the resolved content, making generatedplan.mddiffer by script type. Write encoded bytes to preserve parity.
paths.impl_plan.write_text(template_content, encoding="utf-8")
scripts/powershell/common.ps1:561
- Mixed priority representations are deliberately treated as a parse failure here, after which lines 578-582 scan every preset directory and ignore registry disabled state. Canonical registry resolution normalizes numeric strings, booleans, and invalid values instead; normalize each entry locally so one corrupted priority cannot activate disabled presets.
This issue also appears on line 569 of the same file.
if ($presetEntries.Count -gt 1) {
$allNumeric = $true
$allStrings = $true
foreach ($entry in $presetEntries) {
$priority = & $priorityFor $entry
scripts/python/create_new_feature.py:395
- Writing through text mode translates LF to CRLF on Windows, unlike the prior byte-preserving copy and the Bash/PowerShell implementations. This makes
spec.mddepend on the selected script variant; write UTF-8 bytes directly instead.
spec_file.write_text(template_content, encoding="utf-8")
scripts/python/common.py:223
- Preset priorities are sorted before normalization and without the canonical ID tie-breaker. Mixed values such as
"1"and2trigger the directory fallback, which re-enables disabled presets; equal priorities (the default for most installs) instead retain install order, so a different layer can win than inPresetRegistry.list_by_priority(). Sort by normalized priority and preset ID.
key=lambda kv: kv[1].get("priority", 10)
if isinstance(kv[1], dict)
else 10,
)
if (
scripts/bash/common.sh:608
- This raw priority sort neither normalizes registry values nor applies the canonical preset-ID tie-breaker. Mixed values can fall back to scanning every directory (including disabled presets), while equal default priorities retain install order and can select a different layer than
PresetRegistry.list_by_priority(). Sort by normalized priority and ID.
for pid, meta in sorted(presets.items(), key=lambda x: x[1].get('priority', 10) if isinstance(x[1], dict) else 10):
if isinstance(meta, dict) and bool(meta.get('enabled', True)) and re.fullmatch(r'[a-z0-9-]+', pid):
- Files reviewed: 43/43 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Normalize and tie-break preset priorities consistently across script variants, and preserve template bytes when Python materializes generated files. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Addressed review Posted on behalf of @mnriem by GitHub Copilot (model: GPT-5.6 Sol). |
There was a problem hiding this comment.
🟡 Not ready to approve
Python newline conversion breaks byte preservation, and irrelevant lower layers can incorrectly invalidate a winning override.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (2)
scripts/python/common.py:406
- A project override is an unconditional highest-priority
replace, but the resolver still parses every lower preset before returning it. Consequently, an unrelated malformed preset manifest—or missing PyYAML—makes/constitutionand other template consumers fail even though none of those layers can contribute. Short-circuit on the override in all three resolver variants (and similarly stop once the effective top layer isreplace) so lower irrelevant layers cannot invalidate a valid resolution.
if override.is_file():
layers.append((override, "replace"))
scripts/python/common.py:440
Path.read_text()performs universal-newline conversion, so a CRLF template is normalized to LF beforesetup_plan.pyorcreate_new_feature.pyre-encodes it withwrite_bytes(). This defeats the byte-preservation change and diverges from the Bash and PowerShell resolvers. Read and decode the raw bytes for every layer so existing line endings survive composition.
content = layers[base_index][0].read_text(encoding="utf-8")
for path, strategy in reversed(layers[:base_index]):
layer_content = path.read_text(encoding="utf-8")
- Files reviewed: 43/43 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Avoid parsing irrelevant lower layers once resolution reaches a replace base, and decode raw bytes so Python preserves source line endings. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Addressed review Posted on behalf of @mnriem by GitHub Copilot (model: GPT-5.6 Sol). |
There was a problem hiding this comment.
🟡 Not ready to approve
Runtime resolvers currently diverge from canonical extension conventions and cross-variant behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (5)
scripts/bash/common.sh:746
- This omits the root-level extension template convention supported by the canonical resolver (
src/specify_cli/presets/__init__.py:5035-5038,5091-5102). Projects with.specify/extensions/<id>/<template>.mdwill resolve that layer during init but Bash command-time resolution silently falls through to a different layer.
local ext="$ext_dir/$extension_id"
local candidate="$ext/templates/${template_name}.md"
if [ -f "$candidate" ]; then
layer_paths+=("$candidate")
layer_strategies+=("replace")
effective_base_found=true
break
fi
scripts/python/common.py:449
- Runtime composition only checks an extension's
templates/directory, but the canonical resolver treats bothtemplates/<name>.mdand root-level<name>.mdas valid template conventions (src/specify_cli/presets/__init__.py:5035-5038,5091-5102). A root-level extension template therefore wins during init/canonical resolution but is skipped by the new command-time resolver, causing/constitutionand the other migrated commands to use a lower layer instead.
candidate = extension_dir / "templates" / f"{template_name}.md"
if candidate.is_file():
layers.append((candidate, "replace"))
return compose_from_base()
scripts/powershell/common.ps1:671
- The canonical extension lookup supports both
templates/<name>.mdand root-level<name>.md(src/specify_cli/presets/__init__.py:5035-5038,5091-5102), while this runtime path checks only the former. PowerShell command-time resolution can therefore disagree with init and skip the effective extension layer.
foreach ($extensionId in Get-SortedExtensionIds -ExtensionsDir $extDir) {
$candidate = Join-Path $extDir "$extensionId/templates/$TemplateName.md"
if (Test-Path $candidate) {
$layerPaths += $candidate
$layerStrategies += 'replace'
$effectiveBaseFound = $true
break
}
scripts/bash/common.sh:465
- When no Python executable is available, this fallback emits every extension directory alphabetically and ignores
.registrypriorities and enabled state. The Bash resolver can consequently select content from a disabled extension or the wrong priority layer, contradicting the new cross-variant resolution contract. The fallback should either parse the registry with another available JSON tool or fail explicitly rather than silently changing semantics.
This issue also appears on line 739 of the same file.
local ext extension_id
for ext in "$ext_dir"/*/; do
[ -d "$ext" ] || continue
extension_id=$(basename "$ext")
case "$extension_id" in *[!a-z0-9-]*) continue ;; esac
printf '%s\n' "$extension_id"
done
scripts/python/check_prerequisites.py:230
--templateis ignored unless Python is also given--json, whereas the new Bash and PowerShell implementations resolve it before choosing output mode and fail if it is missing or malformed. Thus the same non-JSON invocation has different exit status across supported script variants. Either validate that--templaterequires JSON consistently or resolve it in Python before the output-mode branch.
if args.json_mode:
payload: dict[str, object] = {
"FEATURE_DIR": str(paths.feature_dir),
"AVAILABLE_DOCS": docs,
}
if args.template_name:
- Files reviewed: 43/43 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Support root-level extension templates across runtime resolvers, fail safely when Bash cannot parse an extension registry, and validate requested templates in every prerequisite output mode. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Addressed the latest review round in d6f426f:
Added parity regressions for each: malformed-manifest cases ( Validation: full suite ( Posted on behalf of @mnriem by GitHub Copilot (model: Claude Opus 4.8), acting autonomously. |
There was a problem hiding this comment.
🟡 Not ready to approve
Bash and Python still fail open when the extension registry path is a directory or broken symlink.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (2)
scripts/python/common.py:247
- A non-file registry still fails open here. If
.specify/extensions/.registryis a directory or broken symlink,is_file()is false, so every safe on-disk extension directory is treated as unregistered and its template can be served. This contradicts the new fail-closed behavior for corrupt/unreadable extension registries; detect an existing directory entry first and reject it unless it is a readable file.
if registry.is_file():
scripts/bash/common.sh:431
- This check also treats a directory or broken symlink at
.registryas if no registry existed, then ranks all extension directories as unregistered. The no-Python branch below does the same via-f, so a corrupt registry can bypass the intended fail-closed behavior and expose extension templates. Detect any filesystem entry at.registry(including symlinks), and reject it unless it is a readable regular file before either branch scans directories.
if registry.is_file():
- Files reviewed: 44/44 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
The Bash and Python resolvers used is_file()/`-f` to gate reading the extension `.registry`, which returns false for a directory or a broken symlink at that path. In those cases the resolvers treated the registry as absent and scanned every on-disk extension directory as unregistered and enabled — a fail-open path. Detect any filesystem entry at the registry path (including broken symlinks) and reject unless it is a readable regular file. PowerShell now rejects a non-leaf entry explicitly for parity. Adds directory- and broken-symlink parity regressions. Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3158e06f-95df-4e3a-843f-f159a35aa30c
|
Addressed review 4858629667 (fail-open on non-regular-file extension registry). The Bash and Python resolvers gated the extension Added directory- and broken-symlink parity regressions in Commit: f9732f9 Posted on behalf of @mnriem by GitHub Copilot (model: Claude Opus 4.8), acting autonomously. |
There was a problem hiding this comment.
🟡 Changes recommended
Canonical and PowerShell resolution still permit fail-open behavior for specific corrupt extension-registry states.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 44/44 changed files
- Comments generated: 2
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
…nd PowerShell Two remaining fail-open paths for an invalid extension registry: - The canonical PresetResolver enumerated extensions through ExtensionRegistry, whose _load() normalizes a corrupt or unreadable registry to an empty mapping. The directory scan then admitted every on-disk extension directory as unregistered-and-enabled, so a corrupt registry could still supply constitution content at init and through constitution-sync materialization. Add a non-invasive is_corrupt() probe (recovery behavior for install/enable/disable is unchanged) and raise from _get_all_extensions_by_priority() when the registry exists but is invalid. _load() now also recovers from OSError/UnicodeDecodeError so a directory or unreadable registry no longer crashes construction. - The PowerShell resolver gated the registry read with Test-Path, which returns false for a dangling symlink on Windows, letting a broken .registry symlink bypass the guard and enable every on-disk extension. Detect the entry via directory enumeration (which observes a broken symlink) and reject it unless it is a readable regular file. Adds canonical corrupt/directory-registry regressions and extends the broken-symlink parity test to PowerShell. Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3158e06f-95df-4e3a-843f-f159a35aa30c
|
Addressed review 4864849239 (two remaining fail-open registry states). Canonical PowerShell broken symlink: Added canonical corrupt- and directory-registry regressions in Commit: 1299c50 Posted on behalf of @mnriem by GitHub Copilot (model: Claude Opus 4.8), acting autonomously. |
There was a problem hiding this comment.
🟡 Changes recommended
Dangling registries can still fail open, and canonical resolution does not stop at the first effective replacement layer.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (1)
src/specify_cli/presets/init.py:5357
- The canonical resolver still collects every lower preset and then extensions after a project override or higher-priority
replacelayer has already won. As a result, init/constitution-syncmaterialization can fail on a corrupt lower extension registry (or inspect malformed lower layers), while all three runtime resolvers now return immediately at the first effective base. Stop canonical resolution before enumerating lower tiers so these paths retain the promised priority parity.
for pack_id, metadata in self._get_all_presets_by_priority():
pack_dir = self.presets_dir / pack_id
# Read strategy and manifest file path from preset manifest
strategy = "replace"
manifest_has_strategy = False
- Files reviewed: 45/45 changed files
- Comments generated: 1
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
…s_corrupt is_corrupt() gated on Path.exists(), which follows symlinks and returns False for a dangling .registry symlink — so the canonical PresetResolver treated it as an absent registry and fell back to scanning every on-disk extension directory as unregistered-and-enabled, reopening the fail-open path this guard closes. Detect lexical existence with os.path.lexists and require a regular file before parsing, so a broken symlink (or directory) is reported corrupt and resolution fails closed. Adds a canonical broken-symlink regression alongside the directory case. Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3158e06f-95df-4e3a-843f-f159a35aa30c
|
Addressed the three open Copilot review threads (all extension-registry fail-open follow-ups).
Added canonical corrupt-, directory-, and broken-symlink regressions and extended the broken-symlink parity test to PowerShell; each guard verified to fail on revert. Full suite: 6559 passed / 8 skipped; ruff, Latest commit: e453c50 Posted on behalf of @mnriem by GitHub Copilot (model: Claude Opus 4.8), acting autonomously. |
…iew-feedback # Conflicts: # src/specify_cli/extensions/__init__.py
There was a problem hiding this comment.
Review details
Suppressed comments (2)
src/specify_cli/presets/init.py:4982
collect_all_layers()calls this unconditionally after collecting overrides and presets, so a corrupt extension registry still aborts canonical constitution materialization even when a higher-priorityreplaceoverride/preset has already won. For example, init with an override plus a corrupt.specify/extensions/.registryreaches this exception and leavesmemory/constitution.mdunseeded, contrary to the new “stop at the first effective replace base” behavior implemented by the runtime scripts. Short-circuit canonical layer collection once the winning replace base is found, while retaining this fail-closed check when extension enumeration is actually needed.
return None, None
for tmpl in manifest.templates:
if tmpl.get("name") == template_name and tmpl.get("type") == template_type:
file_path = tmpl.get("file")
if file_path:
scripts/powershell/common.ps1:364
- PowerShell comparison operators are case-insensitive by default, so on a case-sensitive filesystem this treats an unrelated
.REGISTRYentry as.registry; the subsequent exact-pathTest-Pathfails and resolution rejects a state that Bash and Python treat as having no registry. Use a case-sensitive comparison to keep registry-path behavior consistent across variants.
Where-Object { $_.Name -eq '.registry' } |
- Files reviewed: 45/45 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
Updated the branch — Resolved in favor of upstream's narrower Full suite green after the merge: 6694 passed / 8 skipped; ruff clean. Latest commit: 86adcbe Posted on behalf of @mnriem by GitHub Copilot (model: Claude Opus 4.8), acting autonomously. |
There was a problem hiding this comment.
Review details
Suppressed comments (2)
src/specify_cli/presets/init.py:5012
collect_all_layers()always reaches this check even when it has already collected a project override or a preset withstrategy: replace. Consequently, a corrupt extension registry below that effective base makes canonicalPresetResolver.resolve_content()fail, while the new Bash/PowerShell/Python runtime resolvers return the winning replace layer without inspecting lower tiers. This breaks the stated first-replace short-circuit and affects init/constitution-sync; stop layer collection before enumerating extensions once an effective replace base has been found.
return None, None
def _extension_manifest_declared_template(
self, ext_dir: Path, template_name: str, template_type: str
scripts/python/common.py:368
- This runtime validator checks only that
nameandfileare strings, so entries with an invalid name or an absolute/parent-traversing file path pass even though canonicalPresetManifestrejects them (src/specify_cli/presets/__init__.py:425-469). For a matching unsafe file,_preset_template_layer()merely returnsNone, allowing resolution to succeed from a lower/core layer instead of rejecting the malformed manifest. Add the canonical name-format and normalized relative-path checks for every entry in all three runtime validators.
for field in ("type", "name", "file"):
if not isinstance(entry[field], str):
raise ValueError(f"manifest template {field} must be a string")
- Files reviewed: 45/45 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
Review details
Suppressed comments (6)
scripts/powershell/common.ps1:742
Encoding.UTF8replaces invalid template bytes instead of throwing, so a malformed winning layer is returned successfully with U+FFFD characters while the Bash and Python resolvers fail the read. Apply a strictUTF8Encoding(..., throwOnInvalidBytes: true)to every layer read in this function so template resolution fails closed consistently.
return [System.IO.File]::ReadAllText($layerPaths[0], [System.Text.Encoding]::UTF8)
scripts/powershell/common.ps1:550
- This UTF-8 decoder silently replaces malformed byte sequences. Consequently, a preset registry with invalid UTF-8 in unrelated metadata can still parse and honor its priorities in PowerShell, while Bash/Python reject it and fall back to directory ordering, potentially selecting a different preset. Decode with
throwOnInvalidBytesso registry normalization remains cross-language consistent.
$registryData = [System.IO.File]::ReadAllText($registryFile, [System.Text.Encoding]::UTF8) | ConvertFrom-Json
scripts/bash/common.sh:812
- This extension lookup bypasses
extension.yml, so a manifest-declared template whose filename is non-conventional is omitted (or a stale conventional file wins). That disagrees with the canonical extension resolution contract insrc/specify_cli/presets/__init__.py:5527-5541and its regression tests attests/test_presets.py:11398-11511. Consult the authoritative manifest entry before falling back to these paths.
local candidate="$ext/templates/${template_name}.md"
[ -f "$candidate" ] || candidate="$ext/${template_name}.md"
if [ -f "$candidate" ]; then
scripts/python/common.py:502
- Runtime resolution ignores
extension.ymland checks only the two conventional filenames. A registered extension that declaresconstitution-template(or another requested template) at a custom path therefore falls through to a lower layer, even though the canonical resolver treats that manifest entry as authoritative (src/specify_cli/presets/__init__.py:5527-5541; regression coverage attests/test_presets.py:11398-11430). Resolve the extension manifest entry before applying the convention fallback.
for extension_id in _sorted_extension_ids(extensions_dir):
extension_dir = extensions_dir / extension_id
candidate = _conventional_template(extension_dir, template_name)
if candidate is not None:
layers.append((candidate, "replace"))
scripts/powershell/common.ps1:721
- Only probing conventional paths means this resolver cannot consume extension templates declared at custom paths in
extension.yml, and it can select a stale conventional file despite the manifest. The canonical resolver explicitly makes extension manifests authoritative (src/specify_cli/presets/__init__.py:5527-5541; tests attests/test_presets.py:11398-11511). Resolve the manifest entry first, then use conventions only when it is undeclared.
$candidate = Join-Path $extDir "$extensionId/templates/$TemplateName.md"
if (-not (Test-Path $candidate)) {
$candidate = Join-Path $extDir "$extensionId/$TemplateName.md"
}
if (Test-Path $candidate) {
scripts/powershell/common.ps1:371
Encoding.UTF8uses replacement fallback, so invalid UTF-8 bytes inside otherwise valid registry JSON are converted to U+FFFD and can parse successfully. Bash and Python decode strictly and reject the same corrupt extension registry, so PowerShell can violate the promised fail-closed parity. Use a strict UTF-8 decoder that throws on invalid bytes.
This issue also appears in the following locations of the same file:
- line 550
- line 742
$data = [System.IO.File]::ReadAllText($registryFile, [System.Text.Encoding]::UTF8) | ConvertFrom-Json
- Files reviewed: 45/45 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Summary
constitution-templateat command time through the dedicated Bash, PowerShell, or Python resolver scriptprovides/templatessections or declaring an empty template list, consistent with the canonicalPresetManifestPresetResolver(used by init andconstitution-syncmaterialization) when a Bash template layer cannot be read, and when the extension registry is corrupt, unreadable, or not a readable regular file (a directory or broken symlink at the registry path, including a dangling.registrysymlink under Windows PowerShell), instead of returning successful empty/partial content or silently treating every on-disk extension directory as unregistered-and-enabledspecify preset resolvewhile rejecting empty or traversal-like segmentsreplacebase so malformed lower layers cannot invalidate a winning override or presetspec.mdandplan.mdconstitution-syncis enabled and document the lifecycleTesting
.venv/bin/python -m pytest tests/test_presets.py tests/test_resolve_template_python_parity.py tests/test_setup_tasks.py -q.venv/bin/python -m pytest tests/test_check_prerequisites_python_parity.py tests/test_setup_plan_python_parity.py tests/test_create_new_feature_python_parity.py tests/test_setup_tasks_python_parity.py -quvx ruff@0.15.0 check src testsCloses #3950
Authored by GitHub Copilot (model: Claude Opus 4.8), acting autonomously on behalf of @mnriem.