From fc15fdc6c811bb034783027a5ab884c7ade7433d Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Tue, 21 Apr 2026 15:19:20 -0500 Subject: [PATCH 01/33] fix: rebase onto upstream/main, resolve conflicts with PR #2189 upstream/main merged PR #2189 (wrap-only strategy) which overlaps with our comprehensive composition strategies (prepend/append/wrap). Resolved conflicts keeping our implementation as source of truth: - README: keep our future considerations (composition is now fully implemented, not a future item) - presets.py: keep our composition architecture (_reconcile_composed_commands, collect_all_layers, resolve_content) while preserving #2189's _substitute_core_template which is used by agents.py for skill generation - tests: keep both test sets (our composition tests + #2189's wrap tests), removed TestReplayWrapsForCommand and TestInstallRemoveWrapLifecycle which test the superseded _replay_wraps_for_command API; our composition tests cover equivalent scenarios - Restored missing _unregister_commands call in remove() that was lost during #2189 merge --- presets/ARCHITECTURE.md | 18 + presets/README.md | 44 +- presets/scaffold/preset.yml | 28 + scripts/bash/common.sh | 233 ++++++- scripts/powershell/common.ps1 | 212 ++++++ src/specify_cli/__init__.py | 55 +- src/specify_cli/agents.py | 40 ++ src/specify_cli/presets.py | 888 +++++++++++++++++------- tests/test_presets.py | 1231 ++++++++++++++++++--------------- 9 files changed, 1893 insertions(+), 856 deletions(-) diff --git a/presets/ARCHITECTURE.md b/presets/ARCHITECTURE.md index d0e6547816..d7a45cb9ff 100644 --- a/presets/ARCHITECTURE.md +++ b/presets/ARCHITECTURE.md @@ -41,6 +41,24 @@ The resolution is implemented three times to ensure consistency: - **Bash**: `resolve_template()` in `scripts/bash/common.sh` - **PowerShell**: `Resolve-Template` in `scripts/powershell/common.ps1` +### Composition Strategies + +Templates, commands, and scripts support a `strategy` field that controls how a preset's content is combined with lower-priority content instead of fully replacing it: + +| Strategy | Description | Templates | Commands | Scripts | +|----------|-------------|-----------|----------|---------| +| `replace` (default) | Fully replaces lower-priority content | ✓ | ✓ | ✓ | +| `prepend` | Places content before lower-priority content (separated by a blank line) | ✓ | ✓ | — | +| `append` | Places content after lower-priority content (separated by a blank line) | ✓ | ✓ | — | +| `wrap` | Content contains `{CORE_TEMPLATE}` (templates/commands) or `$CORE_SCRIPT` (scripts) placeholder replaced with lower-priority content | ✓ | ✓ | ✓ | + +Composition is recursive — multiple composing presets chain. The `PresetResolver.resolve_content()` method walks the full priority stack bottom-up and applies each layer's strategy. + +Content resolution functions for composition: +- **Python**: `PresetResolver.resolve_content()` in `src/specify_cli/presets.py` +- **Bash**: `resolve_template_content()` in `scripts/bash/common.sh` +- **PowerShell**: `Resolve-TemplateContent` in `scripts/powershell/common.ps1` + ## Command Registration When a preset is installed with `type: "command"` entries, the `PresetManager` registers them into all detected agent directories using the shared `CommandRegistrar` from `src/specify_cli/agents.py`. diff --git a/presets/README.md b/presets/README.md index 72751b4bfb..7d7b9ae8a2 100644 --- a/presets/README.md +++ b/presets/README.md @@ -61,7 +61,37 @@ specify preset add healthcare-compliance --priority 5 # overrides enterprise-sa specify preset add pm-workflow --priority 1 # overrides everything ``` -Presets **override**, they don't merge. If two presets both provide `spec-template`, the one with the lowest priority number wins entirely. +Presets **override by default**, they don't merge. If two presets both provide `spec-template` with the default `replace` strategy, the one with the lowest priority number wins entirely. However, presets can use **composition strategies** to augment rather than replace content. + +### Composition Strategies + +Presets can declare a `strategy` per template to control how content is combined. The `name` field identifies which template to compose with in the priority stack, while `file` points to the actual content file (which can differ from the convention path `templates/.md`): + +```yaml +provides: + templates: + - type: "template" + name: "spec-template" + file: "templates/spec-addendum.md" + strategy: "append" # adds content after the core template +``` + +| Strategy | Description | +|----------|-------------| +| `replace` (default) | Fully replaces the lower-priority template | +| `prepend` | Places content **before** the resolved lower-priority template, separated by a blank line | +| `append` | Places content **after** the resolved lower-priority template, separated by a blank line | +| `wrap` | Content contains `{CORE_TEMPLATE}` placeholder (or `$CORE_SCRIPT` for scripts) replaced with the lower-priority content | + +**Supported combinations:** + +| Type | `replace` | `prepend` | `append` | `wrap` | +|------|-----------|-----------|----------|--------| +| **template** | ✓ (default) | ✓ | ✓ | ✓ | +| **command** | ✓ (default) | ✓ | ✓ | ✓ | +| **script** | ✓ (default) | — | — | ✓ | + +Multiple composing presets chain recursively. For example, a security preset with `prepend` and a compliance preset with `append` will produce: security header + core content + compliance footer. ## Catalog Management @@ -108,13 +138,5 @@ See [scaffold/](scaffold/) for a scaffold you can copy to create your own preset The following enhancements are under consideration for future releases: -- **Composition strategies** — Allow presets to declare a `strategy` per template instead of the default `replace`: - - | Type | `replace` | `prepend` | `append` | `wrap` | - |------|-----------|-----------|----------|--------| - | **template** | ✓ (default) | ✓ | ✓ | ✓ | - | **command** | ✓ (default) | ✓ | ✓ | ✓ | - | **script** | ✓ (default) | — | — | ✓ | - - For artifacts and commands (which are LLM directives), `wrap` injects preset content before and after the core template using a `{CORE_TEMPLATE}` placeholder (implemented). For scripts, `wrap` would run custom logic before/after the core script via a `$CORE_SCRIPT` variable (not yet implemented). -- **Script overrides** — Enable presets to provide alternative versions of core scripts (e.g. `create-new-feature.sh`) for workflow customization. A `strategy: "wrap"` option could allow presets to run custom logic before/after the core script without fully replacing it. +- **Structural merge strategies** — Parsing Markdown sections for per-section granularity (e.g., "replace only ## Security"). +- **Conflict detection** — `specify preset lint` / `specify preset doctor` for detecting composition conflicts. diff --git a/presets/scaffold/preset.yml b/presets/scaffold/preset.yml index 975a92a413..0180c3375a 100644 --- a/presets/scaffold/preset.yml +++ b/presets/scaffold/preset.yml @@ -32,6 +32,14 @@ provides: templates: # CUSTOMIZE: Define your template overrides # Templates are document scaffolds (spec-template.md, plan-template.md, etc.) + # + # Strategy options (optional, defaults to "replace"): + # replace - Fully replaces the lower-priority template (default) + # prepend - Places this content BEFORE the lower-priority template + # append - Places this content AFTER the lower-priority template + # wrap - Uses {CORE_TEMPLATE} placeholder, replaced with lower-priority content + # + # Note: Scripts only support "replace" and "wrap" strategies. - type: "template" name: "spec-template" file: "templates/spec-template.md" @@ -45,6 +53,26 @@ provides: # description: "Custom plan template" # replaces: "plan-template" + # COMPOSITION EXAMPLES: + # The `file` field points to the content file (can differ from the + # convention path `templates/.md`). The `name` field identifies + # which template to compose with in the priority stack. + # + # Append additional sections to an existing template: + # - type: "template" + # name: "spec-template" + # file: "templates/spec-addendum.md" + # description: "Add compliance section to spec template" + # strategy: "append" + # + # Wrap a command with preamble/sign-off: + # - type: "command" + # name: "speckit.specify" + # file: "commands/specify-wrapper.md" + # description: "Wrap specify command with compliance checks" + # strategy: "wrap" + # # In the wrapper file, use {CORE_TEMPLATE} where the original content goes + # OVERRIDE EXTENSION TEMPLATES: # Presets sit above extensions in the resolution stack, so you can # override templates provided by any installed extension. diff --git a/scripts/bash/common.sh b/scripts/bash/common.sh index b41d17dec3..14cf49a8ac 100644 --- a/scripts/bash/common.sh +++ b/scripts/bash/common.sh @@ -321,7 +321,8 @@ try: data = json.load(f) presets = data.get('presets', {}) for pid, meta in sorted(presets.items(), key=lambda x: x[1].get('priority', 10)): - print(pid) + if meta.get('enabled', True) is not False: + print(pid) except Exception: sys.exit(1) " 2>/dev/null); then @@ -373,3 +374,233 @@ except Exception: return 1 } +# Resolve a template name to composed content using composition strategies. +# Reads strategy metadata from preset manifests and composes content +# from multiple layers using prepend, append, or wrap strategies. +# +# Usage: CONTENT=$(resolve_template_content "template-name" "$REPO_ROOT") +# Returns composed content string on stdout; exit code 1 if not found. +resolve_template_content() { + local template_name="$1" + local repo_root="$2" + local base="$repo_root/.specify/templates" + + # Collect all layers (highest priority first) + local -a layer_paths=() + local -a layer_strategies=() + + # Priority 1: Project overrides (always "replace") + local override="$base/overrides/${template_name}.md" + if [ -f "$override" ]; then + layer_paths+=("$override") + layer_strategies+=("replace") + fi + + # Priority 2: Installed presets (sorted by priority from .registry) + local presets_dir="$repo_root/.specify/presets" + if [ -d "$presets_dir" ]; then + local registry_file="$presets_dir/.registry" + local sorted_presets="" + if [ -f "$registry_file" ] && command -v python3 >/dev/null 2>&1; then + if sorted_presets=$(SPECKIT_REGISTRY="$registry_file" python3 -c " +import json, sys, os +try: + with open(os.environ['SPECKIT_REGISTRY']) as f: + data = json.load(f) + presets = data.get('presets', {}) + for pid, meta in sorted(presets.items(), key=lambda x: x[1].get('priority', 10)): + if meta.get('enabled', True) is not False: + print(pid) +except Exception: + sys.exit(1) +" 2>/dev/null); then + if [ -n "$sorted_presets" ]; then + while IFS= read -r preset_id; do + # Read strategy and file path from preset manifest + local strategy="replace" + local manifest_file="" + local manifest="$presets_dir/$preset_id/preset.yml" + if [ -f "$manifest" ] && command -v python3 >/dev/null 2>&1; then + # Requires PyYAML; falls back to replace/convention if unavailable + local result + local py_stderr + py_stderr=$(mktemp) + result=$(SPECKIT_MANIFEST="$manifest" SPECKIT_TMPL="$template_name" python3 -c " +import sys, os +try: + import yaml +except ImportError: + print('yaml_missing', file=sys.stderr) + print('replace\t') + sys.exit(0) +try: + with open(os.environ['SPECKIT_MANIFEST']) as f: + data = yaml.safe_load(f) + for t in data.get('provides', {}).get('templates', []): + if t.get('name') == os.environ['SPECKIT_TMPL'] and t.get('type', 'template') == 'template': + print(t.get('strategy', 'replace') + '\t' + t.get('file', '')) + sys.exit(0) + print('replace\t') +except Exception: + print('replace\t') +" 2>"$py_stderr") + local parse_status=$? + if [ $parse_status -eq 0 ] && [ -n "$result" ]; then + IFS=$'\t' read -r strategy manifest_file <<< "$result" + strategy=$(printf '%s' "$strategy" | tr '[:upper:]' '[:lower:]') + fi + # Warn only when PyYAML is explicitly missing + if grep -q 'yaml_missing' "$py_stderr" 2>/dev/null; then + echo "Warning: PyYAML not available; composition strategies in $manifest may be ignored" >&2 + fi + rm -f "$py_stderr" + fi + # Try manifest file path first, then convention path + local candidate="" + if [ -n "$manifest_file" ]; then + local mf="$presets_dir/$preset_id/$manifest_file" + [ -f "$mf" ] && candidate="$mf" + fi + if [ -z "$candidate" ]; then + local cf="$presets_dir/$preset_id/templates/${template_name}.md" + [ -f "$cf" ] && candidate="$cf" + fi + if [ -n "$candidate" ]; then + layer_paths+=("$candidate") + layer_strategies+=("$strategy") + fi + done <<< "$sorted_presets" + fi + else + # python3 failed — fall back to unordered directory scan (replace only) + for preset in "$presets_dir"/*/; do + [ -d "$preset" ] || continue + local candidate="$preset/templates/${template_name}.md" + if [ -f "$candidate" ]; then + layer_paths+=("$candidate") + layer_strategies+=("replace") + fi + done + fi + else + # No python3 or registry — fall back to unordered directory scan (replace only) + for preset in "$presets_dir"/*/; do + [ -d "$preset" ] || continue + local candidate="$preset/templates/${template_name}.md" + if [ -f "$candidate" ]; then + layer_paths+=("$candidate") + layer_strategies+=("replace") + fi + done + fi + fi + + # Priority 3: Extension-provided templates (always "replace") + local ext_dir="$repo_root/.specify/extensions" + if [ -d "$ext_dir" ]; then + for ext in "$ext_dir"/*/; do + [ -d "$ext" ] || continue + case "$(basename "$ext")" in .*) continue;; esac + local candidate="$ext/templates/${template_name}.md" + if [ -f "$candidate" ]; then + layer_paths+=("$candidate") + layer_strategies+=("replace") + fi + done + fi + + # Priority 4: Core templates (always "replace") + local core="$base/${template_name}.md" + if [ -f "$core" ]; then + layer_paths+=("$core") + layer_strategies+=("replace") + fi + + local count=${#layer_paths[@]} + [ "$count" -eq 0 ] && return 1 + + # Check if any layer uses a non-replace strategy + local has_composition=false + for s in "${layer_strategies[@]}"; do + [ "$s" != "replace" ] && has_composition=true && break + done + + # If the top (highest-priority) layer is replace, it wins entirely — + # lower layers are irrelevant regardless of their strategies. + if [ "${layer_strategies[0]}" = "replace" ]; then + cat "${layer_paths[0]}" + return 0 + fi + + if [ "$has_composition" = false ]; then + cat "${layer_paths[0]}" + return 0 + fi + + # Compose bottom-up: start from lowest priority + local content="" + local has_base=false + local started=false + local i + for (( i=count-1; i>=0; i-- )); do + local path="${layer_paths[$i]}" + local strat="${layer_strategies[$i]}" + local layer_content + # Preserve trailing newlines: append sentinel, then strip it + layer_content=$(cat "$path"; printf x) + layer_content="${layer_content%x}" + + if [ "$started" = false ]; then + if [ "$strat" = "replace" ]; then + content="$layer_content" + has_base=true + fi + # Keep consuming replace layers from the bottom until we hit a non-replace + if [ "$strat" != "replace" ]; then + # No base content to compose onto + [ "$has_base" = false ] && return 1 + started=true + case "$strat" in + prepend) content="$(printf '%s\n\n%s' "$layer_content" "$content")" ;; + append) content="$(printf '%s\n\n%s' "$content" "$layer_content")" ;; + wrap) + # Validate placeholder exists + case "$layer_content" in + *'{CORE_TEMPLATE}'*) ;; + *) echo "Error: wrap strategy missing {CORE_TEMPLATE} placeholder" >&2; return 1 ;; + esac + # Replace all occurrences to match Python/PowerShell behavior + while [[ "$layer_content" == *'{CORE_TEMPLATE}'* ]]; do + local before="${layer_content%%\{CORE_TEMPLATE\}*}" + local after="${layer_content#*\{CORE_TEMPLATE\}}" + layer_content="${before}${content}${after}" + done + content="$layer_content" + ;; + esac + fi + else + case "$strat" in + replace) content="$layer_content" ;; + prepend) content="$(printf '%s\n\n%s' "$layer_content" "$content")" ;; + append) content="$(printf '%s\n\n%s' "$content" "$layer_content")" ;; + wrap) + case "$layer_content" in + *'{CORE_TEMPLATE}'*) ;; + *) echo "Error: wrap strategy missing {CORE_TEMPLATE} placeholder" >&2; return 1 ;; + esac + while [[ "$layer_content" == *'{CORE_TEMPLATE}'* ]]; do + local before="${layer_content%%\{CORE_TEMPLATE\}*}" + local after="${layer_content#*\{CORE_TEMPLATE\}}" + layer_content="${before}${content}${after}" + done + content="$layer_content" + ;; + esac + fi + done + + printf '%s' "$content" + return 0 +} + diff --git a/scripts/powershell/common.ps1 b/scripts/powershell/common.ps1 index 0d6544aaf4..52fc363b1f 100644 --- a/scripts/powershell/common.ps1 +++ b/scripts/powershell/common.ps1 @@ -287,6 +287,21 @@ function Test-DirHasFiles { } } +# Find a usable Python 3 executable (python3, python, or py -3). +# Returns the command/arguments as an array, or $null if none found. +function Get-Python3Command { + if (Get-Command python3 -ErrorAction SilentlyContinue) { return @('python3') } + if (Get-Command python -ErrorAction SilentlyContinue) { + $ver = & python --version 2>&1 + if ($ver -match 'Python 3') { return @('python') } + } + if (Get-Command py -ErrorAction SilentlyContinue) { + $ver = & py -3 --version 2>&1 + if ($ver -match 'Python 3') { return @('py', '-3') } + } + return $null +} + # Resolve a template name to a file path using the priority stack: # 1. .specify/templates/overrides/ # 2. .specify/presets//templates/ (sorted by priority from .registry) @@ -315,6 +330,7 @@ function Resolve-Template { $presets = $registryData.presets if ($presets) { $sortedPresets = $presets.PSObject.Properties | + Where-Object { $null -eq $_.Value.enabled -or $_.Value.enabled -ne $false } | Sort-Object { if ($null -ne $_.Value.priority) { $_.Value.priority } else { 10 } } | ForEach-Object { $_.Name } } @@ -354,3 +370,199 @@ function Resolve-Template { return $null } +# Resolve a template name to composed content using composition strategies. +# Reads strategy metadata from preset manifests and composes content +# from multiple layers using prepend, append, or wrap strategies. +function Resolve-TemplateContent { + param( + [Parameter(Mandatory=$true)][string]$TemplateName, + [Parameter(Mandatory=$true)][string]$RepoRoot + ) + + $base = Join-Path $RepoRoot '.specify/templates' + + # Collect all layers (highest priority first) + $layerPaths = @() + $layerStrategies = @() + + # Priority 1: Project overrides (always "replace") + $override = Join-Path $base "overrides/$TemplateName.md" + if (Test-Path $override) { + $layerPaths += $override + $layerStrategies += 'replace' + } + + # Priority 2: Installed presets (sorted by priority from .registry) + $presetsDir = Join-Path $RepoRoot '.specify/presets' + if (Test-Path $presetsDir) { + $registryFile = Join-Path $presetsDir '.registry' + $sortedPresets = @() + if (Test-Path $registryFile) { + try { + $registryData = Get-Content $registryFile -Raw | ConvertFrom-Json + $presets = $registryData.presets + if ($presets) { + $sortedPresets = $presets.PSObject.Properties | + Where-Object { $null -eq $_.Value.enabled -or $_.Value.enabled -ne $false } | + Sort-Object { if ($null -ne $_.Value.priority) { $_.Value.priority } else { 10 } } | + ForEach-Object { $_.Name } + } + } catch { + $sortedPresets = @() + } + } + + if ($sortedPresets.Count -gt 0) { + $pyCmd = Get-Python3Command + foreach ($presetId in $sortedPresets) { + # Read strategy and file path from preset manifest + $strategy = 'replace' + $manifestFilePath = '' + $manifest = Join-Path $presetsDir "$presetId/preset.yml" + if ((Test-Path $manifest) -and $pyCmd) { + try { + # Use Python to parse YAML manifest for strategy and file path + $pyArgs = if ($pyCmd.Count -gt 1) { $pyCmd[1..($pyCmd.Count-1)] } else { @() } + $pyStderrFile = [System.IO.Path]::GetTempFileName() + $stratResult = & $pyCmd[0] @pyArgs -c @" +import sys +try: + import yaml +except ImportError: + print('yaml_missing', file=sys.stderr) + print('replace\t') + sys.exit(0) +try: + with open(sys.argv[1]) as f: + data = yaml.safe_load(f) + for t in data.get('provides', {}).get('templates', []): + if t.get('name') == sys.argv[2] and t.get('type', 'template') == 'template': + print(t.get('strategy', 'replace') + '\t' + t.get('file', '')) + sys.exit(0) + print('replace\t') +except Exception: + print('replace\t') +"@ $manifest $TemplateName 2>$pyStderrFile + if ($stratResult) { + $parts = $stratResult.Trim() -split "`t", 2 + $strategy = $parts[0].ToLowerInvariant() + if ($parts.Count -gt 1 -and $parts[1]) { $manifestFilePath = $parts[1] } + } + # Warn only when PyYAML is explicitly missing + if ((Test-Path $pyStderrFile) -and (Get-Content $pyStderrFile -Raw -ErrorAction SilentlyContinue) -match 'yaml_missing') { + Write-Warning "PyYAML not available; composition strategies in $manifest may be ignored" + } + Remove-Item $pyStderrFile -Force -ErrorAction SilentlyContinue + } catch { + $strategy = 'replace' + if ($pyStderrFile) { Remove-Item $pyStderrFile -Force -ErrorAction SilentlyContinue } + } + } + # Try manifest file path first, then convention path + $candidate = $null + if ($manifestFilePath) { + $mf = Join-Path $presetsDir "$presetId/$manifestFilePath" + if (Test-Path $mf) { $candidate = $mf } + } + if (-not $candidate) { + $cf = Join-Path $presetsDir "$presetId/templates/$TemplateName.md" + if (Test-Path $cf) { $candidate = $cf } + } + if ($candidate) { + $layerPaths += $candidate + $layerStrategies += $strategy + } + } + } else { + # Fallback: alphabetical directory order (no registry or parse failure) + foreach ($preset in Get-ChildItem -Path $presetsDir -Directory -ErrorAction SilentlyContinue | Where-Object { $_.Name -notlike '.*' }) { + $candidate = Join-Path $preset.FullName "templates/$TemplateName.md" + if (Test-Path $candidate) { + $layerPaths += $candidate + $layerStrategies += 'replace' + } + } + } + } + + # Priority 3: Extension-provided templates (always "replace") + $extDir = Join-Path $RepoRoot '.specify/extensions' + if (Test-Path $extDir) { + foreach ($ext in Get-ChildItem -Path $extDir -Directory -ErrorAction SilentlyContinue | Where-Object { $_.Name -notlike '.*' } | Sort-Object Name) { + $candidate = Join-Path $ext.FullName "templates/$TemplateName.md" + if (Test-Path $candidate) { + $layerPaths += $candidate + $layerStrategies += 'replace' + } + } + } + + # Priority 4: Core templates (always "replace") + $core = Join-Path $base "$TemplateName.md" + if (Test-Path $core) { + $layerPaths += $core + $layerStrategies += 'replace' + } + + if ($layerPaths.Count -eq 0) { return $null } + + # If the top (highest-priority) layer is replace, it wins entirely — + # lower layers are irrelevant regardless of their strategies. + if ($layerStrategies[0] -eq 'replace') { + return (Get-Content $layerPaths[0] -Raw) + } + + # Check if any layer uses a non-replace strategy + $hasComposition = $false + foreach ($s in $layerStrategies) { + if ($s -ne 'replace') { $hasComposition = $true; break } + } + + if (-not $hasComposition) { + return (Get-Content $layerPaths[0] -Raw) + } + + # Compose bottom-up: start from lowest priority + $content = $null + $started = $false + for ($i = $layerPaths.Count - 1; $i -ge 0; $i--) { + $path = $layerPaths[$i] + $strat = $layerStrategies[$i] + $layerContent = Get-Content $path -Raw + + if (-not $started) { + if ($strat -eq 'replace') { + $content = $layerContent + } + if ($strat -ne 'replace') { + # No base content to compose onto + if ($null -eq $content) { return $null } + $started = $true + switch ($strat) { + 'prepend' { $content = "$layerContent`n`n$content" } + 'append' { $content = "$content`n`n$layerContent" } + 'wrap' { + if (-not $layerContent.Contains('{CORE_TEMPLATE}')) { + throw "Wrap strategy missing {CORE_TEMPLATE} placeholder" + } + $content = $layerContent.Replace('{CORE_TEMPLATE}', $content) + } + } + } + } else { + switch ($strat) { + 'replace' { $content = $layerContent } + 'prepend' { $content = "$layerContent`n`n$content" } + 'append' { $content = "$content`n`n$layerContent" } + 'wrap' { + if (-not $layerContent.Contains('{CORE_TEMPLATE}')) { + throw "Wrap strategy missing {CORE_TEMPLATE} placeholder" + } + $content = $layerContent.Replace('{CORE_TEMPLATE}', $content) + } + } + } + } + + return $content +} \ No newline at end of file diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 97cb993a96..6e672758d5 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -2595,14 +2595,55 @@ def preset_resolve( raise typer.Exit(1) resolver = PresetResolver(project_root) - result = resolver.resolve_with_source(template_name) - - if result: - console.print(f" [bold]{template_name}[/bold]: {result['path']}") - console.print(f" [dim](from: {result['source']})[/dim]") + layers = resolver.collect_all_layers(template_name) + + if layers: + # Use the highest-priority layer for display because the final output + # may be composed and may not map to resolve_with_source()'s single path. + display_layer = layers[0] + console.print(f" [bold]{template_name}[/bold]: {display_layer['path']}") + console.print(f" [dim](top layer from: {display_layer['source']})[/dim]") + + has_composition = ( + layers[0]["strategy"] != "replace" + and any(layer["strategy"] != "replace" for layer in layers) + ) + if has_composition: + # Verify composition is actually possible + try: + composed = resolver.resolve_content(template_name) + except Exception as exc: + composed = None + console.print(f" [yellow]Warning: composition error: {exc}[/yellow]") + if composed is None: + console.print(" [yellow]Warning: composition cannot produce output (no base layer with 'replace' strategy)[/yellow]") + else: + console.print(" [dim]Final output is composed from multiple preset layers; the path above is the highest-priority contributing layer.[/dim]") + console.print("\n [bold]Composition chain:[/bold]") + # Compute the effective base: the last consecutive replace layer + # from the bottom before the first non-replace (same logic as + # PresetResolver.resolve_content). + reversed_display = list(reversed(layers)) + effective_base_idx = 0 + for idx, lyr in enumerate(reversed_display): + if lyr["strategy"] == "replace": + effective_base_idx = idx + else: + break + for i, layer in enumerate(reversed_display): + strategy_label = layer["strategy"] + if strategy_label == "replace" and i == effective_base_idx: + strategy_label = "base" + console.print(f" {i + 1}. [{strategy_label}] {layer['source']} → {layer['path']}") else: - console.print(f" [yellow]{template_name}[/yellow]: not found") - console.print(" [dim]No template with this name exists in the resolution stack[/dim]") + # No layers found — fall back to resolve_with_source for non-composition cases + result = resolver.resolve_with_source(template_name) + if result: + console.print(f" [bold]{template_name}[/bold]: {result['path']}") + console.print(f" [dim](from: {result['source']})[/dim]") + else: + console.print(f" [yellow]{template_name}[/yellow]: not found") + console.print(" [dim]No template with this name exists in the resolution stack[/dim]") @preset_app.command("info") diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index c5e25a7085..2c77a86ad6 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -651,6 +651,46 @@ def register_commands_for_all_agents( return results + def register_commands_for_non_skill_agents( + self, + commands: List[Dict[str, Any]], + source_id: str, + source_dir: Path, + project_root: Path, + ) -> Dict[str, List[str]]: + """Register commands for all non-skill agents in the project. + + Like register_commands_for_all_agents but skips skill-based agents + (those with extension '/SKILL.md'). Used by reconciliation to avoid + overwriting properly formatted SKILL.md files. + + Args: + commands: List of command info dicts + source_id: Identifier of the source + source_dir: Directory containing command source files + project_root: Path to project root + + Returns: + Dictionary mapping agent names to list of registered commands + """ + results = {} + self._ensure_configs() + for agent_name, agent_config in self.AGENT_CONFIGS.items(): + if agent_config.get("extension") == "/SKILL.md": + continue + agent_dir = project_root / agent_config["dir"] + if agent_dir.exists(): + try: + registered = self.register_commands( + agent_name, commands, source_id, + source_dir, project_root, + ) + if registered: + results[agent_name] = registered + except ValueError: + continue + return results + def unregister_commands( self, registered_commands: Dict[str, List[str]], project_root: Path ) -> None: diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 5f28be7204..9100ff4003 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -109,6 +109,9 @@ class PresetCompatibilityError(PresetError): VALID_PRESET_TEMPLATE_TYPES = {"template", "command", "script"} +VALID_PRESET_STRATEGIES = {"replace", "prepend", "append", "wrap"} +# Scripts only support replace and wrap (prepend/append don't make semantic sense for executable code) +VALID_SCRIPT_STRATEGIES = {"replace", "wrap"} class PresetManifest: @@ -207,6 +210,28 @@ def _validate(self): "must be a relative path within the preset directory" ) + # Validate strategy field (optional, defaults to "replace") + strategy = tmpl.get("strategy", "replace") + if not isinstance(strategy, str): + raise PresetValidationError( + f"Invalid strategy value: must be a string, " + f"got {type(strategy).__name__}" + ) + strategy = strategy.lower() + # Persist normalized value so downstream code sees lowercase + if "strategy" in tmpl: + tmpl["strategy"] = strategy + if strategy not in VALID_PRESET_STRATEGIES: + raise PresetValidationError( + f"Invalid strategy '{strategy}': " + f"must be one of {sorted(VALID_PRESET_STRATEGIES)}" + ) + if tmpl["type"] == "script" and strategy not in VALID_SCRIPT_STRATEGIES: + raise PresetValidationError( + f"Invalid strategy '{strategy}' for script: " + f"scripts only support {sorted(VALID_SCRIPT_STRATEGIES)}" + ) + # Validate template name format if tmpl["type"] == "command": # Commands use dot notation (e.g. speckit.specify) @@ -558,6 +583,10 @@ def _register_commands( file, and writes it to every detected agent directory using the CommandRegistrar from the agents module. + When a command uses a composition strategy (prepend, append, wrap), + the content is composed with the lower-priority command before + registration. + Args: manifest: Preset manifest preset_dir: Installed preset directory @@ -587,6 +616,37 @@ def _register_commands( if not filtered: return {} + # Handle composition strategies: resolve composed content for non-replace commands + resolver = PresetResolver(self.project_root) + composed_dir = None + commands_to_register = [] + for cmd in filtered: + strategy = cmd.get("strategy", "replace") + if strategy != "replace": + # Resolve composed content using the full priority stack + composed = resolver.resolve_content(cmd["name"], "command") + if composed is not None: + # Write composed content to a temporary subdirectory + if composed_dir is None: + composed_dir = preset_dir / ".composed" + composed_dir.mkdir(parents=True, exist_ok=True) + composed_file = composed_dir / f"{cmd['name']}.md" + composed_file.write_text(composed, encoding="utf-8") + commands_to_register.append({ + **cmd, + "file": f".composed/{cmd['name']}.md", + }) + else: + raise PresetValidationError( + f"Command '{cmd['name']}' uses '{strategy}' strategy " + f"but no base command layer exists to compose onto. " + f"Ensure a lower-priority preset, extension, or core " + f"command provides this command before using " + f"composition strategies." + ) + else: + commands_to_register.append(cmd) + try: from .agents import CommandRegistrar except ImportError: @@ -594,7 +654,7 @@ def _register_commands( registrar = CommandRegistrar() return registrar.register_commands_for_all_agents( - filtered, manifest.id, preset_dir, self.project_root + commands_to_register, manifest.id, preset_dir, self.project_root ) def _unregister_commands(self, registered_commands: Dict[str, List[str]]) -> None: @@ -611,231 +671,208 @@ def _unregister_commands(self, registered_commands: Dict[str, List[str]]) -> Non registrar = CommandRegistrar() registrar.unregister_commands(registered_commands, self.project_root) - def _replay_wraps_for_command(self, cmd_name: str) -> None: - """Recompose and rewrite agent files for a wrap-strategy command. - - Collects all installed presets that declare cmd_name in their - wrap_commands registry field, sorts them so the highest-precedence - preset (lowest priority number) wraps outermost, then writes the - fully composed output to every agent directory. + def _reconcile_composed_commands(self, command_names: List[str]) -> None: + """Re-resolve and re-register composed commands from the full stack. - Called after every install and remove to keep agent files correct - regardless of installation order. + After install or remove, recompute the effective content for each + command name that participates in composition, and write the winning + content to the agent directories. This ensures command files always + reflect the current priority stack rather than depending on + install/remove order. Args: - cmd_name: Full command name (e.g. "speckit.specify") + command_names: List of command names to reconcile """ + if not command_names: + return + try: from .agents import CommandRegistrar except ImportError: return - # Collect enabled presets that wrap this command, sorted ascending - # (lowest priority number = highest precedence = outermost). - wrap_presets = [] - for pack_id, metadata in self.registry.list_by_priority(include_disabled=False): - if cmd_name not in metadata.get("wrap_commands", []): - continue - pack_dir = self.presets_dir / pack_id - if not pack_dir.is_dir(): - continue # corrupted state — skip - wrap_presets.append((pack_id, pack_dir)) + resolver = PresetResolver(self.project_root) + registrar = CommandRegistrar() - if not wrap_presets: - return + # Cache registry and manifests outside the loop to avoid + # repeated filesystem reads for each command name. + presets_by_priority = list(PresetRegistry(self.presets_dir).list_by_priority()) if self.presets_dir.exists() else [] - # Derive short name for core resolution fallback. - short_name = cmd_name - if short_name.startswith("speckit."): - short_name = short_name[len("speckit."):] + for cmd_name in command_names: + layers = resolver.collect_all_layers(cmd_name, "command") + if not layers: + continue - resolver = PresetResolver(self.project_root) - core_file = ( - resolver.resolve_core(cmd_name, "command") - or resolver.resolve_extension_command_via_manifest(cmd_name) - or ( - resolver.resolve_extension_command_via_manifest(short_name) - if short_name != cmd_name - else None + # If the top layer is replace, it wins entirely — lower layers + # are irrelevant regardless of their strategies. + top_is_replace = layers[0]["strategy"] == "replace" + has_composition = not top_is_replace and any( + layer["strategy"] != "replace" for layer in layers ) - or resolver.resolve_core(short_name, "command") - ) - if core_file is None: - return - - registrar = CommandRegistrar() - core_frontmatter, core_body = registrar.parse_frontmatter( - core_file.read_text(encoding="utf-8") - ) - replay_aliases: List[str] = [] - seen_aliases: set[str] = set() - - # Apply wraps innermost-first (reverse of ascending list). - accumulated_body = core_body - outermost_frontmatter = {} - outermost_pack_id = wrap_presets[0][0] # fallback; updated per contributing preset - for pack_id, pack_dir in reversed(wrap_presets): - manifest_path = pack_dir / "preset.yml" - cmd_file: Optional[Path] = None - if manifest_path.exists(): - try: - manifest = PresetManifest(manifest_path) - except (PresetValidationError, KeyError, TypeError, ValueError): - manifest = None - if manifest is not None: - for template in manifest.templates: - if template.get("type") != "command" or template.get("name") != cmd_name: - continue - file_rel = template.get("file") - if isinstance(file_rel, str): - rel_path = Path(file_rel) - if not rel_path.is_absolute(): - try: - preset_root = pack_dir.resolve() - candidate = (preset_root / rel_path).resolve() - candidate.relative_to(preset_root) - except (OSError, ValueError): - candidate = None - if candidate is not None: - cmd_file = candidate - aliases = template.get("aliases", []) - if not isinstance(aliases, list): - aliases = [] - for alias in aliases: - if isinstance(alias, str) and alias not in seen_aliases: - replay_aliases.append(alias) - seen_aliases.add(alias) + if not has_composition: + # Pure replace — the top layer wins. + top_layer = layers[0] + top_path = top_layer["path"] + # Try to find which preset owns this layer + registered = False + for pack_id, _meta in presets_by_priority: + pack_dir = self.presets_dir / pack_id + if top_path.is_relative_to(pack_dir): + manifest = resolver._get_manifest(pack_dir) + if manifest: + for tmpl in manifest.templates: + if tmpl.get("name") == cmd_name and tmpl.get("type") == "command": + self._register_for_non_skill_agents( + registrar, [tmpl], manifest.id, pack_dir + ) + registered = True + break break - if cmd_file is None: - cmd_file = pack_dir / "commands" / f"{cmd_name}.md" - if not cmd_file.exists(): - continue - wrap_fm, wrap_body = registrar.parse_frontmatter( - cmd_file.read_text(encoding="utf-8") - ) - accumulated_body = wrap_body.replace("{CORE_TEMPLATE}", accumulated_body) - outermost_frontmatter = wrap_fm # last iteration = outermost preset - outermost_pack_id = pack_id - - # Build final frontmatter: outermost preset wins; fall back to core for - # scripts/agent_scripts if the outermost preset does not define them. - final_frontmatter = dict(outermost_frontmatter) - final_frontmatter.pop("strategy", None) - for key in ("scripts", "agent_scripts"): - if key not in final_frontmatter and key in core_frontmatter: - final_frontmatter[key] = core_frontmatter[key] - - composed_content = ( - registrar.render_frontmatter(final_frontmatter) + "\n" + accumulated_body - ) - - self._replay_skill_override(cmd_name, composed_content, outermost_pack_id) - - with tempfile.TemporaryDirectory() as tmpdir: - tmp_path = Path(tmpdir) - cmd_dir = tmp_path / "commands" - cmd_dir.mkdir() - (cmd_dir / f"{cmd_name}.md").write_text(composed_content, encoding="utf-8") - registrar._ensure_configs() - for agent_name, agent_config in registrar.AGENT_CONFIGS.items(): - if agent_config.get("extension") == "/SKILL.md": - continue - agent_dir = self.project_root / agent_config["dir"] - if not agent_dir.exists(): - continue - try: - registrar.register_commands( - agent_name, - [{ - "name": cmd_name, - "file": f"commands/{cmd_name}.md", - "aliases": replay_aliases, - }], - f"preset:{outermost_pack_id}", - tmp_path, - self.project_root, + if not registered: + # Top layer is a non-preset source (extension, core, or + # project override). Register directly from the layer path. + # Extract stable source_id from display label + source = layers[0]["source"] + if source.startswith("extension:"): + # "extension:foo v1.0" → "foo" + source_id = source.split(":", 1)[1].split(" ", 1)[0] + else: + source_id = source + self._register_command_from_path( + registrar, cmd_name, top_path, + source_id=source_id, ) - except ValueError: + else: + # Composed command — resolve from full stack + composed = resolver.resolve_content(cmd_name, "command") + if composed is None: continue - def _replay_skill_override( + # Write to the highest-priority preset's .composed dir + registered = False + for pack_id, _meta in presets_by_priority: + pack_dir = self.presets_dir / pack_id + manifest = resolver._get_manifest(pack_dir) + if not manifest: + continue + for tmpl in manifest.templates: + if tmpl.get("name") == cmd_name and tmpl.get("type") == "command": + composed_dir = pack_dir / ".composed" + composed_dir.mkdir(parents=True, exist_ok=True) + composed_file = composed_dir / f"{cmd_name}.md" + composed_file.write_text(composed, encoding="utf-8") + self._register_for_non_skill_agents( + registrar, + [{**tmpl, "file": f".composed/{cmd_name}.md"}], + manifest.id, pack_dir, + ) + registered = True + break + else: + continue + break + if not registered: + # No preset owns this composed command — write to a + # shared .composed dir and register from the top layer. + shared_composed = self.presets_dir / ".composed" + shared_composed.mkdir(parents=True, exist_ok=True) + composed_file = shared_composed / f"{cmd_name}.md" + composed_file.write_text(composed, encoding="utf-8") + source = layers[0]["source"] + if source.startswith("extension:"): + source_id = source.split(":", 1)[1].split(" ", 1)[0] + else: + source_id = source + self._register_command_from_path( + registrar, cmd_name, composed_file, + source_id=source_id, + ) + + def _register_command_from_path( self, + registrar: Any, cmd_name: str, - composed_content: str, - outermost_pack_id: str, + cmd_path: Path, + source_id: str = "reconciled", ) -> None: - """Rewrite any active SKILL.md override for a replayed wrap command.""" - skills_dir = self._get_skills_dir() - if not skills_dir: - return - - from . import SKILL_DESCRIPTIONS, load_init_options - from .agents import CommandRegistrar - from .integrations import get_integration + """Register a single command from a file path (non-preset source). - init_opts = load_init_options(self.project_root) - if not isinstance(init_opts, dict): - init_opts = {} - selected_ai = init_opts.get("ai") - if not isinstance(selected_ai, str): - return + Used by reconciliation when the winning layer is an extension, + core template, or project override rather than a preset. - registrar = CommandRegistrar() - integration = get_integration(selected_ai) - agent_config = registrar.AGENT_CONFIGS.get(selected_ai, {}) - create_missing_skills = bool(init_opts.get("ai_skills")) and agent_config.get("extension") != "/SKILL.md" - - skill_name, legacy_skill_name = self._skill_names_for_command(cmd_name) - target_skill_names: List[str] = [] - if (skills_dir / skill_name).is_dir(): - target_skill_names.append(skill_name) - if legacy_skill_name != skill_name and (skills_dir / legacy_skill_name).is_dir(): - target_skill_names.append(legacy_skill_name) - if not target_skill_names and create_missing_skills: - missing_skill_dir = skills_dir / skill_name - if not missing_skill_dir.exists(): - target_skill_names.append(skill_name) - if not target_skill_names: + Args: + registrar: CommandRegistrar instance + cmd_name: Command name + cmd_path: Path to the command file + source_id: Source attribution for rendered output + """ + if not cmd_path.exists(): return - - raw_short_name = cmd_name - if raw_short_name.startswith("speckit."): - raw_short_name = raw_short_name[len("speckit."):] - short_name = raw_short_name.replace(".", "-") - skill_title = self._skill_title_from_command(cmd_name) - - frontmatter, body = registrar.parse_frontmatter(composed_content) - original_desc = frontmatter.get("description", "") - enhanced_desc = SKILL_DESCRIPTIONS.get( - short_name, - original_desc or f"Spec-kit workflow command: {short_name}", + cmd_tmpl = { + "name": cmd_name, + "type": "command", + "file": cmd_path.name, + } + self._register_for_non_skill_agents( + registrar, [cmd_tmpl], source_id, cmd_path.parent ) - body = registrar.resolve_skill_placeholders( - selected_ai, dict(frontmatter), body, self.project_root + + def _register_for_non_skill_agents( + self, + registrar: Any, + commands: List[Dict[str, Any]], + source_id: str, + source_dir: Path, + ) -> None: + """Register commands for non-skill agents during reconciliation. + + Skill-based agents (``/SKILL.md`` layout) are handled separately: + - On removal: ``_unregister_skills()`` restores from core/extension, + then ``_reconcile_skills()`` re-runs ``_register_skills()`` for the + next winning preset so SKILL.md files get proper frontmatter and + descriptions. + - On install: ``_register_skills()`` writes formatted SKILL.md, then + ``_reconcile_skills()`` ensures the actual priority winner is used. + + Writing raw command content to skill agents would produce invalid + SKILL.md files (missing skill frontmatter, descriptions, etc.). + """ + registrar.register_commands_for_non_skill_agents( + commands, source_id, source_dir, self.project_root ) - for target_skill_name in target_skill_names: - skill_subdir = skills_dir / target_skill_name - if skill_subdir.exists() and not skill_subdir.is_dir(): + def _reconcile_skills(self, command_names: List[str]) -> None: + """Re-register skills for commands whose winning layer changed. + + After a preset is removed, finds the next preset in the priority + stack that provides each command and re-runs skill registration + for that preset so SKILL.md files reflect the current winner. + + Args: + command_names: List of command names to reconcile skills for + """ + if not command_names: + return + + resolver = PresetResolver(self.project_root) + for cmd_name in command_names: + layers = resolver.collect_all_layers(cmd_name, "command") + if not layers: continue - skill_subdir.mkdir(parents=True, exist_ok=True) - frontmatter_data = registrar.build_skill_frontmatter( - selected_ai, - target_skill_name, - enhanced_desc, - f"preset:{outermost_pack_id}", - ) - frontmatter_text = yaml.safe_dump(frontmatter_data, sort_keys=False).strip() - skill_content = ( - f"---\n" - f"{frontmatter_text}\n" - f"---\n\n" - f"# Speckit {skill_title} Skill\n\n" - f"{body}\n" - ) - if integration is not None and hasattr(integration, "post_process_skill_content"): - skill_content = integration.post_process_skill_content(skill_content) - (skill_subdir / "SKILL.md").write_text(skill_content, encoding="utf-8") + + top_path = layers[0]["path"] + # Find the preset that owns the winning layer + for pack_id, _meta in PresetRegistry(self.presets_dir).list_by_priority(): + pack_dir = self.presets_dir / pack_id + if top_path.is_relative_to(pack_dir): + manifest_path = pack_dir / "preset.yml" + if manifest_path.exists(): + try: + manifest = PresetManifest(manifest_path) + except PresetValidationError: + break + self._register_skills(manifest, pack_dir) + break def _get_skills_dir(self) -> Optional[Path]: """Return the active skills directory for preset skill overrides. @@ -1016,6 +1053,12 @@ def _register_skills( if not source_file.exists(): continue + # Use composed content if available (written by _register_commands + # for commands with non-replace strategies), otherwise the original. + composed_file = preset_dir / ".composed" / f"{cmd_name}.md" + if composed_file.exists(): + source_file = composed_file + # Derive the short command name (e.g. "specify" from "speckit.specify") raw_short_name = cmd_name if raw_short_name.startswith("speckit."): @@ -1257,43 +1300,58 @@ def install_from_directory( shutil.copytree(source_dir, dest_dir) - # Register command overrides with AI agents - registered_commands = self._register_commands(manifest, dest_dir) - - # Update corresponding skills when --ai-skills was previously used - registered_skills = self._register_skills(manifest, dest_dir) - - # Detect wrap commands before registry.add() so a read failure doesn't - # leave a partially-committed registry entry. - wrap_commands = [] - try: - from .agents import CommandRegistrar as _CR - _registrar = _CR() - for cmd_tmpl in manifest.templates: - if cmd_tmpl.get("type") != "command": - continue - cmd_file = dest_dir / cmd_tmpl["file"] - if not cmd_file.exists(): - continue - cmd_fm, _ = _registrar.parse_frontmatter(cmd_file.read_text(encoding="utf-8")) - if cmd_fm.get("strategy") == "wrap": - wrap_commands.append(cmd_tmpl["name"]) - except ImportError: - pass - + # Pre-register the preset so that composition resolution can see it + # in the priority stack when resolving composed command content. self.registry.add(manifest.id, { "version": manifest.version, "source": "local", "manifest_hash": manifest.get_hash(), "enabled": True, "priority": priority, - "registered_commands": registered_commands, - "registered_skills": registered_skills, - "wrap_commands": wrap_commands, + "registered_commands": {}, + "registered_skills": [], }) - for cmd_name in wrap_commands: - self._replay_wraps_for_command(cmd_name) + registered_commands: Dict[str, List[str]] = {} + registered_skills: List[str] = [] + try: + # Register command overrides with AI agents and persist the result + # immediately so cleanup can recover even if installation stops + # before later phases complete. + registered_commands = self._register_commands(manifest, dest_dir) + self.registry.update(manifest.id, { + "registered_commands": registered_commands, + }) + + # Update corresponding skills when --ai-skills was previously used + # and persist that result as well. + registered_skills = self._register_skills(manifest, dest_dir) + self.registry.update(manifest.id, { + "registered_skills": registered_skills, + }) + except Exception: + # Roll back all side effects: unregister any commands/skills that + # were written (using local vars which capture partial progress), + # remove the copied preset dir, and drop the registry entry. + if registered_commands: + self._unregister_commands(registered_commands) + if registered_skills: + self._unregister_skills(registered_skills, dest_dir) + if dest_dir.exists(): + shutil.rmtree(dest_dir) + self.registry.remove(manifest.id) + raise + + # Reconcile all affected commands from the full priority stack so that + # install order doesn't determine the winning command file. + cmd_names = [ + t["name"] for t in manifest.templates if t.get("type") == "command" + ] + if cmd_names: + self._reconcile_composed_commands(cmd_names) + # Also reconcile skills so SKILL.md files reflect the actual + # winning command layer, not just the last-installed preset. + self._reconcile_skills(cmd_names) return manifest @@ -1369,16 +1427,16 @@ def remove(self, pack_id: str) -> bool: # Restore original skills when preset is removed registered_skills = metadata.get("registered_skills", []) if metadata else [] registered_commands = metadata.get("registered_commands", {}) if metadata else {} - wrap_commands = metadata.get("wrap_commands", []) if metadata else [] pack_dir = self.presets_dir / pack_id - # _unregister_skills must run before directory deletion (reads preset files) + # Collect ALL command names before filtering for reconciliation, + # so commands registered only for skill-based agents are also reconciled. + removed_cmd_names = set() + for cmd_names in registered_commands.values(): + removed_cmd_names.update(cmd_names) + if registered_skills: self._unregister_skills(registered_skills, pack_dir) - # When _unregister_skills has already handled skill-agent files, strip - # those entries from registered_commands to avoid double-deletion. - # (When registered_skills is empty, skill-agent entries in - # registered_commands are the only deletion path for those files.) try: from .agents import CommandRegistrar except ImportError: @@ -1390,43 +1448,22 @@ def remove(self, pack_id: str) -> bool: if CommandRegistrar.AGENT_CONFIGS.get(agent_name, {}).get("extension") != "/SKILL.md" } - # Delete the preset directory before mutating the registry so a - # filesystem failure cannot leave files on disk without a registry entry. + # Unregister non-skill command files from AI agents. + if registered_commands: + self._unregister_commands(registered_commands) + if pack_dir.exists(): shutil.rmtree(pack_dir) - # Remove from registry before replaying so _replay_wraps_for_command sees - # the post-removal registry state. self.registry.remove(pack_id) - # Separate wrap commands from non-wrap commands in registered_commands. - non_wrap_commands = { - agent_name: [c for c in cmd_names if c not in wrap_commands] - for agent_name, cmd_names in registered_commands.items() - } - non_wrap_commands = {k: v for k, v in non_wrap_commands.items() if v} - - # Unregister non-wrap command files from AI agents. - if non_wrap_commands: - self._unregister_commands(non_wrap_commands) - - # For each wrapped command, either re-compose remaining wraps or delete. - for cmd_name in wrap_commands: - remaining = [ - pid for pid, meta in self.registry.list().items() - if cmd_name in meta.get("wrap_commands", []) - ] - if remaining: - self._replay_wraps_for_command(cmd_name) - else: - # No wrap presets remain — delete the agent file entirely. - wrap_agent_commands = { - agent_name: [c for c in cmd_names if c == cmd_name] - for agent_name, cmd_names in registered_commands.items() - } - wrap_agent_commands = {k: v for k, v in wrap_agent_commands.items() if v} - if wrap_agent_commands: - self._unregister_commands(wrap_agent_commands) + # Reconcile: if other presets still provide these commands, + # re-resolve from the remaining stack so the next layer takes effect. + if removed_cmd_names: + self._reconcile_composed_commands(list(removed_cmd_names)) + # Also reconcile skills so SKILL.md files reflect the new winning + # command layer rather than being left absent or stale. + self._reconcile_skills(list(removed_cmd_names)) return True @@ -2036,6 +2073,21 @@ def __init__(self, project_root: Path): self.presets_dir = project_root / ".specify" / "presets" self.overrides_dir = self.templates_dir / "overrides" self.extensions_dir = project_root / ".specify" / "extensions" + self._manifest_cache: Dict[str, Optional["PresetManifest"]] = {} + + def _get_manifest(self, pack_dir: Path) -> Optional["PresetManifest"]: + """Get a cached preset manifest, parsing it on first access.""" + key = str(pack_dir) + if key not in self._manifest_cache: + manifest_path = pack_dir / "preset.yml" + if manifest_path.exists(): + try: + self._manifest_cache[key] = PresetManifest(manifest_path) + except PresetValidationError: + self._manifest_cache[key] = None + else: + self._manifest_cache[key] = None + return self._manifest_cache[key] def _get_all_extensions_by_priority(self) -> list[tuple[int, str, dict | None]]: """Build unified list of registered and unregistered extensions sorted by priority. @@ -2079,6 +2131,19 @@ def _get_all_extensions_by_priority(self) -> list[tuple[int, str, dict | None]]: all_extensions.sort(key=lambda x: (x[0], x[1])) return all_extensions + @staticmethod + def _core_stem(template_name: str) -> Optional[str]: + """Extract the stem for core command lookup. + + Commands use dot notation (e.g. ``speckit.specify``), but core + command files are named by stem (e.g. ``specify.md``). Returns + the stem if *template_name* follows the ``speckit.`` pattern, + or ``None`` otherwise. + """ + if template_name.startswith("speckit."): + return template_name[len("speckit."):] + return None + def resolve( self, template_name: str, @@ -2156,6 +2221,12 @@ def resolve( core = self.templates_dir / "commands" / f"{template_name}.md" if core.exists(): return core + # Fallback: speckit. → .md + stem = self._core_stem(template_name) + if stem: + core = self.templates_dir / "commands" / f"{stem}.md" + if core.exists(): + return core elif template_type == "script": core = self.templates_dir / "scripts" / f"{template_name}{ext}" if core.exists(): @@ -2317,3 +2388,288 @@ def resolve_with_source( continue return {"path": resolved_str, "source": "core"} + + def collect_all_layers( + self, + template_name: str, + template_type: str = "template", + ) -> List[Dict[str, Any]]: + """Collect all layers in the priority stack for a template. + + Returns layers from highest priority (checked first) to lowest priority. + Each layer is a dict with 'path', 'source', and 'strategy' keys. + + Args: + template_name: Template name (e.g., "spec-template") + template_type: Template type ("template", "command", or "script") + + Returns: + List of layer dicts ordered highest-to-lowest priority. + """ + if template_type == "template": + subdirs = ["templates", ""] + elif template_type == "command": + subdirs = ["commands"] + elif template_type == "script": + subdirs = ["scripts"] + else: + subdirs = [""] + + ext = ".md" + if template_type == "script": + ext = ".sh" + + layers: List[Dict[str, Any]] = [] + + def _find_in_subdirs(base_dir: Path) -> Optional[Path]: + for subdir in subdirs: + if subdir: + candidate = base_dir / subdir / f"{template_name}{ext}" + else: + candidate = base_dir / f"{template_name}{ext}" + if candidate.exists(): + return candidate + return None + + # Priority 1: Project-local overrides (always "replace" strategy) + if template_type == "script": + override = self.overrides_dir / "scripts" / f"{template_name}{ext}" + else: + override = self.overrides_dir / f"{template_name}{ext}" + if override.exists(): + layers.append({ + "path": override, + "source": "project override", + "strategy": "replace", + }) + + # Priority 2: Installed presets (sorted by priority — lower number = higher precedence) + if self.presets_dir.exists(): + registry = PresetRegistry(self.presets_dir) + for pack_id, metadata in registry.list_by_priority(): + pack_dir = self.presets_dir / pack_id + # Read strategy and manifest file path from preset manifest + strategy = "replace" + manifest_file_path = None + manifest = self._get_manifest(pack_dir) + if manifest: + for tmpl in manifest.templates: + if (tmpl.get("name") == template_name + and tmpl.get("type") == template_type): + strategy = tmpl.get("strategy", "replace") + manifest_file_path = tmpl.get("file") + break + # Use manifest file path if specified, otherwise convention-based lookup + candidate = None + if manifest_file_path: + manifest_candidate = pack_dir / manifest_file_path + if manifest_candidate.exists(): + candidate = manifest_candidate + if candidate is None: + candidate = _find_in_subdirs(pack_dir) + if candidate: + version = metadata.get("version", "?") if metadata else "?" + layers.append({ + "path": candidate, + "source": f"{pack_id} v{version}", + "strategy": strategy, + }) + + # Priority 3: Extension-provided templates (always "replace") + for _priority, ext_id, ext_meta in self._get_all_extensions_by_priority(): + ext_dir = self.extensions_dir / ext_id + if not ext_dir.is_dir(): + continue + # Try convention-based lookup first + candidate = _find_in_subdirs(ext_dir) + # If not found and this is a command, check extension manifest + if candidate is None and template_type == "command": + ext_manifest_path = ext_dir / "extension.yml" + if ext_manifest_path.exists(): + try: + from .extensions import ExtensionManifest, ValidationError as ExtValidationError + ext_manifest = ExtensionManifest(ext_manifest_path) + for cmd in ext_manifest.commands: + if cmd.get("name") == template_name: + cmd_file = cmd.get("file") + if cmd_file: + c = ext_dir / cmd_file + if c.exists(): + candidate = c + break + except (ExtValidationError, yaml.YAMLError): + # Invalid extension manifest — fall back to + # convention-based lookup (already attempted above). + pass + if candidate: + if ext_meta: + version = ext_meta.get("version", "?") + source = f"extension:{ext_id} v{version}" + else: + source = f"extension:{ext_id} (unregistered)" + layers.append({ + "path": candidate, + "source": source, + "strategy": "replace", + }) + + # Priority 4: Core templates (always "replace") + core = None + if template_type == "template": + c = self.templates_dir / f"{template_name}.md" + if c.exists(): + core = c + elif template_type == "command": + c = self.templates_dir / "commands" / f"{template_name}.md" + if c.exists(): + core = c + else: + # Fallback: speckit. → .md + stem = self._core_stem(template_name) + if stem: + c = self.templates_dir / "commands" / f"{stem}.md" + if c.exists(): + core = c + elif template_type == "script": + c = self.templates_dir / "scripts" / f"{template_name}{ext}" + if c.exists(): + core = c + if core: + layers.append({ + "path": core, + "source": "core", + "strategy": "replace", + }) + + return layers + + def resolve_content( + self, + template_name: str, + template_type: str = "template", + ) -> Optional[str]: + """Resolve a template name and return composed content. + + Walks the priority stack and composes content using strategies: + - replace (default): highest-priority content wins entirely + - prepend: content is placed before lower-priority content + - append: content is placed after lower-priority content + - wrap: content contains {CORE_TEMPLATE} placeholder replaced + with lower-priority content (or $CORE_SCRIPT for scripts) + + Composition is recursive — multiple composing presets chain. + + Args: + template_name: Template name (e.g., "spec-template") + template_type: Template type ("template", "command", or "script") + + Returns: + Composed content string, or None if not found + """ + layers = self.collect_all_layers(template_name, template_type) + if not layers: + return None + + # If the top (highest-priority) layer is replace, it wins entirely — + # lower layers are irrelevant regardless of their strategies. + if layers[0]["strategy"] == "replace": + return layers[0]["path"].read_text(encoding="utf-8") + + # Composition: build content bottom-up (lowest priority first) + # Start from the lowest-priority "replace" layer as the base, + # then apply composition layers on top. + # + # layers is ordered highest-priority first. We process in reverse. + reversed_layers = list(reversed(layers)) + + # Find the base content: the first "replace" layer from the bottom + content = None + start_idx = 0 + for i, layer in enumerate(reversed_layers): + if layer["strategy"] == "replace": + content = layer["path"].read_text(encoding="utf-8") + start_idx = i + 1 + else: + # Once we hit a non-replace layer, stop looking for base + break + + # If no base content found, there's nothing to compose onto + if content is None: + return None + + # For command composition, strip frontmatter from each layer to avoid + # leaking YAML metadata into the composed body. The highest-priority + # layer's frontmatter will be reattached at the end. + is_command = template_type == "command" + top_frontmatter_text = None + + def _split_frontmatter(text: str) -> tuple: + """Return (frontmatter_block_with_fences, body) or (None, text). + + Uses line-based fence detection (fence must be ``---`` on its + own line) to avoid false matches on ``---`` inside YAML values. + """ + lines = text.splitlines(keepends=True) + if not lines or lines[0].rstrip("\r\n") != "---": + return None, text + + fence_end = -1 + for i, line in enumerate(lines[1:], start=1): + if line.rstrip("\r\n") == "---": + fence_end = i + break + + if fence_end == -1: + return None, text + + fm_block = "".join(lines[:fence_end + 1]).rstrip("\r\n") + body = "".join(lines[fence_end + 1:]) + return fm_block, body + + if is_command: + fm, body = _split_frontmatter(content) + if fm: + top_frontmatter_text = fm + content = body + + # Apply composition layers from bottom to top + for layer in reversed_layers[start_idx:]: + layer_content = layer["path"].read_text(encoding="utf-8") + strategy = layer["strategy"] + + if is_command: + fm, layer_body = _split_frontmatter(layer_content) + layer_content = layer_body + # Track the highest-priority frontmatter seen; + # replace layers reset frontmatter (even to None) since + # they replace the entire command including metadata. + if strategy == "replace": + top_frontmatter_text = fm + elif fm: + top_frontmatter_text = fm + + if strategy == "replace": + content = layer_content + elif strategy == "prepend": + content = layer_content + "\n\n" + content + elif strategy == "append": + content = content + "\n\n" + layer_content + elif strategy == "wrap": + if template_type == "script": + placeholder = "$CORE_SCRIPT" + else: + placeholder = "{CORE_TEMPLATE}" + if placeholder not in layer_content: + raise PresetValidationError( + f"Wrap strategy in '{layer['source']}' is missing " + f"the {placeholder} placeholder. The wrapper must " + f"contain {placeholder} to indicate where the " + f"lower-priority content should be inserted." + ) + content = layer_content.replace(placeholder, content) + + # Reattach the highest-priority frontmatter for commands + if is_command and top_frontmatter_text: + content = top_frontmatter_text + "\n\n" + content + + return content diff --git a/tests/test_presets.py b/tests/test_presets.py index d913c3b195..b9a684e277 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -3666,647 +3666,736 @@ def _make_wrap_preset_dir( return preset_dir -class TestReplayWrapsForCommand: - """Tests for PresetManager._replay_wraps_for_command().""" - def test_replay_no_op_when_no_wrap_presets(self, project_dir): - """replay does nothing when no presets declare wrap_commands for the command.""" - manager = PresetManager(project_dir) - # Should not raise - manager._replay_wraps_for_command("speckit.specify") - - def test_replay_no_op_when_core_missing(self, project_dir, temp_dir): - """replay exits gracefully when resolve_core returns None.""" - from specify_cli.agents import CommandRegistrar - import copy - - preset_dir = _make_wrap_preset_dir(temp_dir, "preset-a", "speckit.nonexistent-cmd", "pre-a", "post-a") - installed = project_dir / ".specify" / "presets" / "preset-a" - import shutil as _shutil - _shutil.copytree(preset_dir, installed) +class TestCompositionStrategyValidation: + """Test strategy field validation in PresetManifest.""" - manager = PresetManager(project_dir) - manager.registry.add("preset-a", { - "version": "1.0.0", "source": "local", "enabled": True, - "priority": 10, "manifest_hash": "x", - "registered_commands": {}, "registered_skills": [], - "wrap_commands": ["speckit.nonexistent-cmd"], - }) + def test_valid_replace_strategy(self, temp_dir, valid_pack_data): + """Test that replace strategy is accepted.""" + valid_pack_data["provides"]["templates"][0]["strategy"] = "replace" + manifest_path = temp_dir / "preset.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_pack_data, f) + (temp_dir / "templates").mkdir(exist_ok=True) + (temp_dir / "templates" / "spec-template.md").write_text("test") + manifest = PresetManifest(manifest_path) + assert manifest.templates[0]["strategy"] == "replace" - agent_dir = project_dir / ".claude" / "commands" - agent_dir.mkdir(parents=True) + def test_valid_prepend_strategy(self, temp_dir, valid_pack_data): + """Test that prepend strategy is accepted for templates.""" + valid_pack_data["provides"]["templates"][0]["strategy"] = "prepend" + manifest_path = temp_dir / "preset.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_pack_data, f) + (temp_dir / "templates").mkdir(exist_ok=True) + (temp_dir / "templates" / "spec-template.md").write_text("test") + manifest = PresetManifest(manifest_path) + assert manifest.templates[0]["strategy"] == "prepend" - registrar = CommandRegistrar() - original = copy.deepcopy(registrar.AGENT_CONFIGS) - registrar.AGENT_CONFIGS["test-agent"] = { - "dir": str(agent_dir.relative_to(project_dir)), - "format": "markdown", "args": "$ARGUMENTS", - "extension": ".md", "strip_frontmatter_keys": [], - } - try: - # No core file exists for this command — replay should return without writing - manager._replay_wraps_for_command("speckit.nonexistent-cmd") - finally: - CommandRegistrar.AGENT_CONFIGS.clear() - CommandRegistrar.AGENT_CONFIGS.update(original) + def test_valid_append_strategy(self, temp_dir, valid_pack_data): + """Test that append strategy is accepted for templates.""" + valid_pack_data["provides"]["templates"][0]["strategy"] = "append" + manifest_path = temp_dir / "preset.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_pack_data, f) + (temp_dir / "templates").mkdir(exist_ok=True) + (temp_dir / "templates" / "spec-template.md").write_text("test") + manifest = PresetManifest(manifest_path) + assert manifest.templates[0]["strategy"] == "append" - assert not (agent_dir / "speckit.nonexistent-cmd.md").exists() + def test_valid_wrap_strategy(self, temp_dir, valid_pack_data): + """Test that wrap strategy is accepted for templates.""" + valid_pack_data["provides"]["templates"][0]["strategy"] = "wrap" + manifest_path = temp_dir / "preset.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_pack_data, f) + (temp_dir / "templates").mkdir(exist_ok=True) + (temp_dir / "templates" / "spec-template.md").write_text("test") + manifest = PresetManifest(manifest_path) + assert manifest.templates[0]["strategy"] == "wrap" - def test_replay_single_preset_writes_composed_output(self, project_dir, temp_dir): - """Single wrap preset: replay writes pre + core + post to agent dirs.""" - from specify_cli.agents import CommandRegistrar - import copy, shutil as _shutil + def test_default_strategy_is_replace(self, pack_dir): + """Test that omitting strategy defaults to replace (key is absent).""" + manifest = PresetManifest(pack_dir / "preset.yml") + # Strategy key should not be present in the manifest data + assert "strategy" not in manifest.templates[0] + # But consumers should treat missing strategy as "replace" + assert manifest.templates[0].get("strategy", "replace") == "replace" + + def test_invalid_strategy_rejected(self, temp_dir, valid_pack_data): + """Test that invalid strategy values are rejected.""" + valid_pack_data["provides"]["templates"][0]["strategy"] = "merge" + manifest_path = temp_dir / "preset.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_pack_data, f) + with pytest.raises(PresetValidationError, match="Invalid strategy"): + PresetManifest(manifest_path) - # Core template - core_dir = project_dir / ".specify" / "templates" / "commands" - core_dir.mkdir(parents=True, exist_ok=True) - (core_dir / "specify.md").write_text("---\ndescription: core\n---\n\ncore body\n") + def test_prepend_rejected_for_scripts(self, temp_dir, valid_pack_data): + """Test that prepend strategy is rejected for scripts.""" + valid_pack_data["provides"]["templates"] = [{ + "type": "script", + "name": "create-new-feature", + "file": "scripts/create-new-feature.sh", + "strategy": "prepend", + }] + manifest_path = temp_dir / "preset.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_pack_data, f) + with pytest.raises(PresetValidationError, match="Invalid strategy.*for script"): + PresetManifest(manifest_path) - # Install preset-a - preset_dir = _make_wrap_preset_dir(temp_dir, "preset-a", "speckit.specify", "pre-a", "post-a") - installed = project_dir / ".specify" / "presets" / "preset-a" - _shutil.copytree(preset_dir, installed) + def test_append_rejected_for_scripts(self, temp_dir, valid_pack_data): + """Test that append strategy is rejected for scripts.""" + valid_pack_data["provides"]["templates"] = [{ + "type": "script", + "name": "create-new-feature", + "file": "scripts/create-new-feature.sh", + "strategy": "append", + }] + manifest_path = temp_dir / "preset.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_pack_data, f) + with pytest.raises(PresetValidationError, match="Invalid strategy.*for script"): + PresetManifest(manifest_path) - manager = PresetManager(project_dir) - manager.registry.add("preset-a", { - "version": "1.0.0", "source": "local", "enabled": True, - "priority": 10, "manifest_hash": "x", - "registered_commands": {}, "registered_skills": [], - "wrap_commands": ["speckit.specify"], - }) + def test_wrap_accepted_for_scripts(self, temp_dir, valid_pack_data): + """Test that wrap strategy is accepted for scripts.""" + valid_pack_data["provides"]["templates"] = [{ + "type": "script", + "name": "create-new-feature", + "file": "scripts/create-new-feature.sh", + "strategy": "wrap", + }] + manifest_path = temp_dir / "preset.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_pack_data, f) + manifest = PresetManifest(manifest_path) + assert manifest.templates[0]["strategy"] == "wrap" + + def test_replace_accepted_for_scripts(self, temp_dir, valid_pack_data): + """Test that replace strategy is accepted for scripts.""" + valid_pack_data["provides"]["templates"] = [{ + "type": "script", + "name": "create-new-feature", + "file": "scripts/create-new-feature.sh", + "strategy": "replace", + }] + manifest_path = temp_dir / "preset.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_pack_data, f) + manifest = PresetManifest(manifest_path) + assert manifest.templates[0]["strategy"] == "replace" + + def test_prepend_accepted_for_commands(self, temp_dir, valid_pack_data): + """Test that prepend strategy is accepted for commands.""" + valid_pack_data["provides"]["templates"] = [{ + "type": "command", + "name": "speckit.specify", + "file": "commands/speckit.specify.md", + "strategy": "prepend", + }] + manifest_path = temp_dir / "preset.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_pack_data, f) + manifest = PresetManifest(manifest_path) + assert manifest.templates[0]["strategy"] == "prepend" - agent_dir = project_dir / ".claude" / "commands" - agent_dir.mkdir(parents=True) - registrar = CommandRegistrar() - original = copy.deepcopy(registrar.AGENT_CONFIGS) - registrar.AGENT_CONFIGS["test-agent"] = { - "dir": str(agent_dir.relative_to(project_dir)), - "format": "markdown", "args": "$ARGUMENTS", - "extension": ".md", "strip_frontmatter_keys": [], - } - try: - manager._replay_wraps_for_command("speckit.specify") - finally: - CommandRegistrar.AGENT_CONFIGS.clear() - CommandRegistrar.AGENT_CONFIGS.update(original) - written = (agent_dir / "speckit.specify.md").read_text() - assert "[pre-a]" in written - assert "core body" in written - assert "[post-a]" in written - assert "{CORE_TEMPLATE}" not in written - assert "strategy" not in written +class TestResolveContent: + """Test PresetResolver.resolve_content() composition.""" - def test_replay_uses_manifest_command_file_mapping(self, project_dir, temp_dir): - """Replay reads wrapper files from preset.yml instead of assuming command-name paths.""" - from specify_cli.agents import CommandRegistrar - import copy, shutil as _shutil + def test_resolve_content_core_template(self, project_dir): + """Test resolve_content returns core template when no composition.""" + resolver = PresetResolver(project_dir) + content = resolver.resolve_content("spec-template") + assert content is not None + assert "Core Spec Template" in content - core_dir = project_dir / ".specify" / "templates" / "commands" - core_dir.mkdir(parents=True, exist_ok=True) - (core_dir / "specify.md").write_text("---\ndescription: core\n---\n\nCORE\n") - - preset_dir = _make_wrap_preset_dir( - temp_dir, - "preset-a", - "speckit.specify", - "pre-a", - "post-a", - file_rel="commands/custom-wrapper.md", - ) - installed = project_dir / ".specify" / "presets" / "preset-a" - _shutil.copytree(preset_dir, installed) + def test_resolve_content_nonexistent(self, project_dir): + """Test resolve_content returns None for nonexistent template.""" + resolver = PresetResolver(project_dir) + content = resolver.resolve_content("nonexistent") + assert content is None + def test_resolve_content_replace_strategy(self, project_dir, temp_dir, valid_pack_data): + """Test resolve_content with default replace strategy.""" manager = PresetManager(project_dir) - manager.registry.add("preset-a", { - "version": "1.0.0", "source": "local", "enabled": True, - "priority": 10, "manifest_hash": "x", - "registered_commands": {}, "registered_skills": [], - "wrap_commands": ["speckit.specify"], - }) + manager.install_from_directory( + _create_pack(temp_dir, valid_pack_data, "replace-pack", + "# Replaced Content\n"), + "0.1.5" + ) - agent_dir = project_dir / ".claude" / "commands" - agent_dir.mkdir(parents=True) - registrar = CommandRegistrar() - original = copy.deepcopy(registrar.AGENT_CONFIGS) - registrar.AGENT_CONFIGS["test-agent"] = { - "dir": str(agent_dir.relative_to(project_dir)), - "format": "markdown", "args": "$ARGUMENTS", - "extension": ".md", "strip_frontmatter_keys": [], + resolver = PresetResolver(project_dir) + content = resolver.resolve_content("spec-template") + assert content is not None + assert "Replaced Content" in content + assert "Core Spec Template" not in content + + def test_resolve_content_append_strategy(self, project_dir, temp_dir, valid_pack_data): + """Test resolve_content with append strategy.""" + pack_data = {**valid_pack_data} + pack_data["preset"] = {**valid_pack_data["preset"], "id": "append-pack", "name": "Append"} + pack_data["provides"] = { + "templates": [{ + "type": "template", + "name": "spec-template", + "file": "templates/spec-template.md", + "strategy": "append", + }] } - try: - manager._replay_wraps_for_command("speckit.specify") - finally: - CommandRegistrar.AGENT_CONFIGS.clear() - CommandRegistrar.AGENT_CONFIGS.update(original) + pack_dir = temp_dir / "append-pack" + pack_dir.mkdir() + with open(pack_dir / "preset.yml", 'w') as f: + yaml.dump(pack_data, f) + (pack_dir / "templates").mkdir() + (pack_dir / "templates" / "spec-template.md").write_text("## Appended Section\n") - written = (agent_dir / "speckit.specify.md").read_text() - assert "[pre-a]" in written - assert "CORE" in written - assert "[post-a]" in written + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.5") - def test_replay_resolves_extension_core_via_manifest_mapping(self, project_dir, temp_dir): - """Replay finds extension core commands whose manifest file differs from command name.""" - from specify_cli.agents import CommandRegistrar - import copy, shutil as _shutil + resolver = PresetResolver(project_dir) + content = resolver.resolve_content("spec-template") + assert content is not None + assert "Core Spec Template" in content + assert "Appended Section" in content + # Core should come first, appended after + assert content.index("Core Spec Template") < content.index("Appended Section") + + def test_resolve_content_prepend_strategy(self, project_dir, temp_dir, valid_pack_data): + """Test resolve_content with prepend strategy.""" + pack_data = {**valid_pack_data} + pack_data["preset"] = {**valid_pack_data["preset"], "id": "prepend-pack", "name": "Prepend"} + pack_data["provides"] = { + "templates": [{ + "type": "template", + "name": "spec-template", + "file": "templates/spec-template.md", + "strategy": "prepend", + }] + } + pack_dir = temp_dir / "prepend-pack" + pack_dir.mkdir() + with open(pack_dir / "preset.yml", 'w') as f: + yaml.dump(pack_data, f) + (pack_dir / "templates").mkdir() + (pack_dir / "templates" / "spec-template.md").write_text("## Security Header\n") - ext_dir = project_dir / ".specify" / "extensions" / "selftest" - cmd_dir = ext_dir / "commands" - cmd_dir.mkdir(parents=True, exist_ok=True) - (cmd_dir / "selftest.md").write_text( - "---\ndescription: selftest core\n---\n\nEXTENSION-CORE\n" - ) - (ext_dir / "extension.yml").write_text( - "schema_version: '1.0'\n" - "extension:\n id: selftest\n name: Self-Test\n version: 1.0.0\n" - " description: test\n author: test\n repository: https://example.com\n" - " license: MIT\n" - "requires:\n speckit_version: '>=0.2.0'\n" - "provides:\n" - " commands:\n" - " - name: speckit.selftest.extension\n" - " file: commands/selftest.md\n" - " description: Selftest command\n" - ) + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.5") - preset_dir = _make_wrap_preset_dir( - temp_dir, "preset-a", "speckit.selftest.extension", "pre-a", "post-a" + resolver = PresetResolver(project_dir) + content = resolver.resolve_content("spec-template") + assert content is not None + assert "Security Header" in content + assert "Core Spec Template" in content + # Prepended content should come first + assert content.index("Security Header") < content.index("Core Spec Template") + + def test_resolve_content_wrap_strategy(self, project_dir, temp_dir, valid_pack_data): + """Test resolve_content with wrap strategy for templates.""" + pack_data = {**valid_pack_data} + pack_data["preset"] = {**valid_pack_data["preset"], "id": "wrap-pack", "name": "Wrap"} + pack_data["provides"] = { + "templates": [{ + "type": "template", + "name": "spec-template", + "file": "templates/spec-template.md", + "strategy": "wrap", + }] + } + pack_dir = temp_dir / "wrap-pack" + pack_dir.mkdir() + with open(pack_dir / "preset.yml", 'w') as f: + yaml.dump(pack_data, f) + (pack_dir / "templates").mkdir() + (pack_dir / "templates" / "spec-template.md").write_text( + "# Wrapper Start\n\n{CORE_TEMPLATE}\n\n# Wrapper End\n" ) - installed = project_dir / ".specify" / "presets" / "preset-a" - _shutil.copytree(preset_dir, installed) manager = PresetManager(project_dir) - manager.registry.add("preset-a", { - "version": "1.0.0", "source": "local", "enabled": True, - "priority": 10, "manifest_hash": "x", - "registered_commands": {}, "registered_skills": [], - "wrap_commands": ["speckit.selftest.extension"], - }) + manager.install_from_directory(pack_dir, "0.1.5") - agent_dir = project_dir / ".claude" / "commands" - agent_dir.mkdir(parents=True) - registrar = CommandRegistrar() - original = copy.deepcopy(registrar.AGENT_CONFIGS) - registrar.AGENT_CONFIGS["test-agent"] = { - "dir": str(agent_dir.relative_to(project_dir)), - "format": "markdown", "args": "$ARGUMENTS", - "extension": ".md", "strip_frontmatter_keys": [], + resolver = PresetResolver(project_dir) + content = resolver.resolve_content("spec-template") + assert content is not None + assert "Wrapper Start" in content + assert "Core Spec Template" in content + assert "Wrapper End" in content + # Wrapper should surround core + assert content.index("Wrapper Start") < content.index("Core Spec Template") + assert content.index("Core Spec Template") < content.index("Wrapper End") + + def test_resolve_content_wrap_strategy_script(self, project_dir, temp_dir, valid_pack_data): + """Test resolve_content with wrap strategy for scripts uses $CORE_SCRIPT.""" + # Create core script + scripts_dir = project_dir / ".specify" / "templates" / "scripts" + scripts_dir.mkdir(parents=True, exist_ok=True) + (scripts_dir / "test-script.sh").write_text("echo 'core script'\n") + + pack_data = {**valid_pack_data} + pack_data["preset"] = {**valid_pack_data["preset"], "id": "script-wrap", "name": "Script Wrap"} + pack_data["provides"] = { + "templates": [{ + "type": "script", + "name": "test-script", + "file": "scripts/test-script.sh", + "strategy": "wrap", + }] } - try: - manager._replay_wraps_for_command("speckit.selftest.extension") - finally: - CommandRegistrar.AGENT_CONFIGS.clear() - CommandRegistrar.AGENT_CONFIGS.update(original) - - written = (agent_dir / "speckit.selftest.extension.md").read_text() - assert "[pre-a]" in written - assert "EXTENSION-CORE" in written - assert "[post-a]" in written - - def test_replay_priority_order_lower_number_outermost(self, project_dir, temp_dir): - """Two wrap presets: lower priority number = outermost wrapper.""" - from specify_cli.agents import CommandRegistrar - import copy, shutil as _shutil - - core_dir = project_dir / ".specify" / "templates" / "commands" - core_dir.mkdir(parents=True, exist_ok=True) - (core_dir / "specify.md").write_text("---\ndescription: core\n---\n\nCORE\n") - - for pid in ("preset-outer", "preset-inner"): - src = _make_wrap_preset_dir(temp_dir, pid, "speckit.specify", f"pre-{pid}", f"post-{pid}") - _shutil.copytree(src, project_dir / ".specify" / "presets" / pid) + pack_dir = temp_dir / "script-wrap" + pack_dir.mkdir() + with open(pack_dir / "preset.yml", 'w') as f: + yaml.dump(pack_data, f) + (pack_dir / "scripts").mkdir() + (pack_dir / "scripts" / "test-script.sh").write_text( + "#!/bin/bash\necho 'before'\n$CORE_SCRIPT\necho 'after'\n" + ) manager = PresetManager(project_dir) - # preset-outer has priority 1 (highest precedence = outermost) - # preset-inner has priority 10 (lowest precedence = innermost) - for pid, pri in (("preset-outer", 1), ("preset-inner", 10)): - manager.registry.add(pid, { - "version": "1.0.0", "source": "local", "enabled": True, - "priority": pri, "manifest_hash": "x", - "registered_commands": {}, "registered_skills": [], - "wrap_commands": ["speckit.specify"], - }) + manager.install_from_directory(pack_dir, "0.1.5") - agent_dir = project_dir / ".claude" / "commands" - agent_dir.mkdir(parents=True) - registrar = CommandRegistrar() - original = copy.deepcopy(registrar.AGENT_CONFIGS) - registrar.AGENT_CONFIGS["test-agent"] = { - "dir": str(agent_dir.relative_to(project_dir)), - "format": "markdown", "args": "$ARGUMENTS", - "extension": ".md", "strip_frontmatter_keys": [], + resolver = PresetResolver(project_dir) + content = resolver.resolve_content("test-script", "script") + assert content is not None + assert "echo 'before'" in content + assert "echo 'core script'" in content + assert "echo 'after'" in content + + def test_resolve_content_multi_preset_chain(self, project_dir, temp_dir, valid_pack_data): + """Test multi-preset composition chain: prepend + append stacking.""" + # Create preset A (priority 1): prepend security header + pack_a_data = {**valid_pack_data} + pack_a_data["preset"] = {**valid_pack_data["preset"], "id": "preset-a", "name": "A"} + pack_a_data["provides"] = { + "templates": [{ + "type": "template", + "name": "spec-template", + "file": "templates/spec-template.md", + "strategy": "prepend", + }] } - try: - manager._replay_wraps_for_command("speckit.specify") - finally: - CommandRegistrar.AGENT_CONFIGS.clear() - CommandRegistrar.AGENT_CONFIGS.update(original) - - written = (agent_dir / "speckit.specify.md").read_text() - # Outermost (preset-outer, p=1) wraps everything; innermost (preset-inner, p=10) is next - outer_pre = written.index("[pre-preset-outer]") - inner_pre = written.index("[pre-preset-inner]") - core_pos = written.index("CORE") - inner_post = written.index("[post-preset-inner]") - outer_post = written.index("[post-preset-outer]") - assert outer_pre < inner_pre < core_pos < inner_post < outer_post - - def test_replay_install_order_independent(self, project_dir, temp_dir): - """Nesting order is determined by priority, not install order.""" - from specify_cli.agents import CommandRegistrar - import copy, shutil as _shutil - - core_dir = project_dir / ".specify" / "templates" / "commands" - core_dir.mkdir(parents=True, exist_ok=True) - (core_dir / "specify.md").write_text("---\ndescription: core\n---\n\nCORE\n") - - for pid in ("preset-a", "preset-b"): - src = _make_wrap_preset_dir(temp_dir, pid, "speckit.specify", f"pre-{pid}", f"post-{pid}") - _shutil.copytree(src, project_dir / ".specify" / "presets" / pid) + pack_a_dir = temp_dir / "preset-a" + pack_a_dir.mkdir() + with open(pack_a_dir / "preset.yml", 'w') as f: + yaml.dump(pack_a_data, f) + (pack_a_dir / "templates").mkdir() + (pack_a_dir / "templates" / "spec-template.md").write_text("## Security Header\n") + + # Create preset B (priority 2): append compliance footer + pack_b_data = {**valid_pack_data} + pack_b_data["preset"] = {**valid_pack_data["preset"], "id": "preset-b", "name": "B"} + pack_b_data["provides"] = { + "templates": [{ + "type": "template", + "name": "spec-template", + "file": "templates/spec-template.md", + "strategy": "append", + }] + } + pack_b_dir = temp_dir / "preset-b" + pack_b_dir.mkdir() + with open(pack_b_dir / "preset.yml", 'w') as f: + yaml.dump(pack_b_data, f) + (pack_b_dir / "templates").mkdir() + (pack_b_dir / "templates" / "spec-template.md").write_text("## Compliance Footer\n") manager = PresetManager(project_dir) - # preset-a priority=5 (outermost), preset-b priority=10 (innermost) - # Install in reverse order to verify install order doesn't affect nesting - for pid, pri in (("preset-b", 10), ("preset-a", 5)): - manager.registry.add(pid, { - "version": "1.0.0", "source": "local", "enabled": True, - "priority": pri, "manifest_hash": "x", - "registered_commands": {}, "registered_skills": [], - "wrap_commands": ["speckit.specify"], - }) + manager.install_from_directory(pack_a_dir, "0.1.5", priority=1) + manager.install_from_directory(pack_b_dir, "0.1.5", priority=2) - agent_dir = project_dir / ".claude" / "commands" - agent_dir.mkdir(parents=True) - registrar = CommandRegistrar() - original = copy.deepcopy(registrar.AGENT_CONFIGS) - registrar.AGENT_CONFIGS["test-agent"] = { - "dir": str(agent_dir.relative_to(project_dir)), - "format": "markdown", "args": "$ARGUMENTS", - "extension": ".md", "strip_frontmatter_keys": [], + resolver = PresetResolver(project_dir) + content = resolver.resolve_content("spec-template") + assert content is not None + # Result: + + + assert "Security Header" in content + assert "Core Spec Template" in content + assert "Compliance Footer" in content + assert content.index("Security Header") < content.index("Core Spec Template") + assert content.index("Core Spec Template") < content.index("Compliance Footer") + + def test_resolve_content_override_trumps_composition(self, project_dir, temp_dir, valid_pack_data): + """Test that project overrides trump composition (replace at top priority).""" + # Install a composing preset + pack_data = {**valid_pack_data} + pack_data["preset"] = {**valid_pack_data["preset"], "id": "append-pack", "name": "Append"} + pack_data["provides"] = { + "templates": [{ + "type": "template", + "name": "spec-template", + "file": "templates/spec-template.md", + "strategy": "append", + }] } - try: - manager._replay_wraps_for_command("speckit.specify") - finally: - CommandRegistrar.AGENT_CONFIGS.clear() - CommandRegistrar.AGENT_CONFIGS.update(original) - - written = (agent_dir / "speckit.specify.md").read_text() - a_pre = written.index("[pre-preset-a]") - b_pre = written.index("[pre-preset-b]") - core_pos = written.index("CORE") - b_post = written.index("[post-preset-b]") - a_post = written.index("[post-preset-a]") - # preset-a (p=5) is outermost regardless of install order - assert a_pre < b_pre < core_pos < b_post < a_post - - def test_replay_updates_skill_outputs(self, project_dir, temp_dir): - """Replay also rewrites SKILL.md-backed agent outputs.""" - import json - import shutil as _shutil - - core_dir = project_dir / ".specify" / "templates" / "commands" - core_dir.mkdir(parents=True, exist_ok=True) - (core_dir / "specify.md").write_text("---\ndescription: core\n---\n\nCORE\n") - - preset_dir = _make_wrap_preset_dir(temp_dir, "preset-a", "speckit.specify", "pre-a", "post-a") - _shutil.copytree(preset_dir, project_dir / ".specify" / "presets" / "preset-a") + pack_dir = temp_dir / "append-pack" + pack_dir.mkdir() + with open(pack_dir / "preset.yml", 'w') as f: + yaml.dump(pack_data, f) + (pack_dir / "templates").mkdir() + (pack_dir / "templates" / "spec-template.md").write_text("## Appended\n") manager = PresetManager(project_dir) - manager.registry.add("preset-a", { - "version": "1.0.0", "source": "local", "enabled": True, - "priority": 10, "manifest_hash": "x", - "registered_commands": {}, "registered_skills": [], - "wrap_commands": ["speckit.specify"], - }) - - skills_dir = project_dir / ".claude" / "skills" - skill_subdir = skills_dir / "speckit-specify" - skill_subdir.mkdir(parents=True) - (skill_subdir / "SKILL.md").write_text("---\nname: speckit-specify\n---\n\nold\n") - (project_dir / ".specify" / "init-options.json").write_text( - json.dumps({"ai": "claude", "ai_skills": True}) - ) - - manager._replay_wraps_for_command("speckit.specify") - - written = (skill_subdir / "SKILL.md").read_text() - assert "[pre-a]" in written - assert "CORE" in written - assert "[post-a]" in written - - def test_replay_applies_integration_post_processing_to_skill(self, project_dir, temp_dir): - """_replay_skill_override must call post_process_skill_content, matching _register_skills.""" - import json - import shutil as _shutil + manager.install_from_directory(pack_dir, "0.1.5") - core_dir = project_dir / ".specify" / "templates" / "commands" - core_dir.mkdir(parents=True, exist_ok=True) - (core_dir / "specify.md").write_text("---\ndescription: core\n---\n\nCORE\n") + # Create project override (replaces everything) + overrides_dir = project_dir / ".specify" / "templates" / "overrides" + overrides_dir.mkdir(parents=True) + (overrides_dir / "spec-template.md").write_text("# Override Only\n") - preset_dir = _make_wrap_preset_dir(temp_dir, "preset-a", "speckit.specify", "pre-a", "post-a") - _shutil.copytree(preset_dir, project_dir / ".specify" / "presets" / "preset-a") + resolver = PresetResolver(project_dir) + content = resolver.resolve_content("spec-template") + assert content is not None + assert "Override Only" in content + # Override replaces, so appended content should not be visible + assert "Core Spec Template" not in content + + def test_resolve_content_command_type(self, project_dir, temp_dir, valid_pack_data): + """Test resolve_content with command template type.""" + # Create core command using stem naming (matches real layout: plan.md, not speckit.plan.md) + commands_dir = project_dir / ".specify" / "templates" / "commands" + commands_dir.mkdir(parents=True, exist_ok=True) + (commands_dir / "plan.md").write_text("# Core Plan Command\n") + + pack_data = {**valid_pack_data} + pack_data["preset"] = {**valid_pack_data["preset"], "id": "cmd-append", "name": "CmdAppend"} + pack_data["provides"] = { + "templates": [{ + "type": "command", + "name": "speckit.plan", + "file": "commands/speckit.plan.md", + "strategy": "append", + }] + } + pack_dir = temp_dir / "cmd-append" + pack_dir.mkdir() + with open(pack_dir / "preset.yml", 'w') as f: + yaml.dump(pack_data, f) + (pack_dir / "commands").mkdir() + (pack_dir / "commands" / "speckit.plan.md").write_text("## Additional Instructions\n") manager = PresetManager(project_dir) - manager.registry.add("preset-a", { - "version": "1.0.0", "source": "local", "enabled": True, - "priority": 10, "manifest_hash": "x", - "registered_commands": {}, "registered_skills": [], - "wrap_commands": ["speckit.specify"], - }) + manager.install_from_directory(pack_dir, "0.1.5") - skills_dir = project_dir / ".claude" / "skills" - skill_subdir = skills_dir / "speckit-specify" - skill_subdir.mkdir(parents=True) - (skill_subdir / "SKILL.md").write_text("---\nname: speckit-specify\n---\n\nold\n") - (project_dir / ".specify" / "init-options.json").write_text( - json.dumps({"ai": "claude", "ai_skills": True}) + resolver = PresetResolver(project_dir) + content = resolver.resolve_content("speckit.plan", "command") + assert content is not None + assert "Core Plan Command" in content + assert "Additional Instructions" in content + + def test_resolve_content_command_frontmatter_stripping(self, project_dir, temp_dir, valid_pack_data): + """Test that command composition strips frontmatter from lower layers + and reattaches only the highest-priority frontmatter.""" + # Create core command with frontmatter + commands_dir = project_dir / ".specify" / "templates" / "commands" + commands_dir.mkdir(parents=True, exist_ok=True) + (commands_dir / "check.md").write_text( + "---\ndescription: Core check command\n---\nCore body content\n" ) - manager._replay_wraps_for_command("speckit.specify") - - # ClaudeIntegration.post_process_skill_content injects these flags. - # Their presence proves the integration hook ran during replay. - written = (skill_subdir / "SKILL.md").read_text() - assert "disable-model-invocation: false" in written, ( - "_replay_skill_override must call post_process_skill_content " - "(same as _register_skills)" + pack_data = {**valid_pack_data} + pack_data["preset"] = {**valid_pack_data["preset"], "id": "fm-test", "name": "FmTest"} + pack_data["provides"] = { + "templates": [{ + "type": "command", + "name": "speckit.check", + "file": "commands/speckit.check.md", + "strategy": "append", + }] + } + pack_dir = temp_dir / "fm-test" + pack_dir.mkdir() + with open(pack_dir / "preset.yml", 'w') as f: + yaml.dump(pack_data, f) + (pack_dir / "commands").mkdir() + (pack_dir / "commands" / "speckit.check.md").write_text( + "---\ndescription: Preset check override\n---\nPreset body content\n" ) + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.5") -class TestInstallRemoveWrapLifecycle: - """Tests for wrap_commands stored on install and replayed on remove.""" - - def _setup_agent(self, project_dir, registrar, agent_configs_dict): - """Register a test markdown agent and return its commands dir.""" - agent_dir = project_dir / ".claude" / "commands" - agent_dir.mkdir(parents=True, exist_ok=True) - agent_configs_dict["test-agent"] = { - "dir": str(agent_dir.relative_to(project_dir)), - "format": "markdown", "args": "$ARGUMENTS", - "extension": ".md", "strip_frontmatter_keys": [], + resolver = PresetResolver(project_dir) + content = resolver.resolve_content("speckit.check", "command") + assert content is not None + # Should have the preset (highest-priority) frontmatter + assert "Preset check override" in content + # Should have both bodies + assert "Core body content" in content + assert "Preset body content" in content + # Core frontmatter should NOT appear in the body + assert content.count("---") == 2 # only one frontmatter block (opening + closing) + + def test_resolve_content_blank_line_separator(self, project_dir, temp_dir, valid_pack_data): + """Test that prepend/append use blank line separator.""" + pack_data = {**valid_pack_data} + pack_data["preset"] = {**valid_pack_data["preset"], "id": "sep-test", "name": "SepTest"} + pack_data["provides"] = { + "templates": [{ + "type": "template", + "name": "spec-template", + "file": "templates/spec-template.md", + "strategy": "append", + }] } - return agent_dir + pack_dir = temp_dir / "sep-test" + pack_dir.mkdir() + with open(pack_dir / "preset.yml", 'w') as f: + yaml.dump(pack_data, f) + (pack_dir / "templates").mkdir() + (pack_dir / "templates" / "spec-template.md").write_text("appended") - def test_install_stores_wrap_commands_in_registry(self, project_dir, temp_dir): - """install_from_directory stores wrap_commands in the registry entry.""" - from specify_cli.agents import CommandRegistrar - import copy - - core_dir = project_dir / ".specify" / "templates" / "commands" - core_dir.mkdir(parents=True, exist_ok=True) - (core_dir / "specify.md").write_text("---\ndescription: core\n---\n\ncore\n") - - preset_src = _make_wrap_preset_dir(temp_dir, "preset-a", "speckit.specify", "pre", "post") + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.5") - registrar = CommandRegistrar() - original = copy.deepcopy(registrar.AGENT_CONFIGS) - agent_dir = project_dir / ".claude" / "commands" - agent_dir.mkdir(parents=True, exist_ok=True) - registrar.AGENT_CONFIGS["test-agent"] = { - "dir": str(agent_dir.relative_to(project_dir)), - "format": "markdown", "args": "$ARGUMENTS", - "extension": ".md", "strip_frontmatter_keys": [], + resolver = PresetResolver(project_dir) + content = resolver.resolve_content("spec-template") + # Should have blank line separator + assert "\n\n" in content + + def test_resolve_content_replace_over_wrap(self, project_dir, temp_dir, valid_pack_data): + """Top-priority replace layer should win even if a lower layer uses wrap.""" + # Install a low-priority wrap preset (with no placeholder — would fail if evaluated) + wrap_data = {**valid_pack_data} + wrap_data["preset"] = {**valid_pack_data["preset"], "id": "wrap-lo", "name": "WrapLo"} + wrap_data["provides"] = { + "templates": [{ + "type": "template", + "name": "spec-template", + "file": "templates/spec-template.md", + "strategy": "wrap", + }] } - try: - manager = PresetManager(project_dir) - manager.install_from_directory(preset_src, "0.1.0", priority=10) - finally: - CommandRegistrar.AGENT_CONFIGS.clear() - CommandRegistrar.AGENT_CONFIGS.update(original) - - meta = manager.registry.get("preset-a") - assert "wrap_commands" in meta - assert "speckit.specify" in meta["wrap_commands"] - - def test_install_replay_produces_correct_nested_output(self, project_dir, temp_dir): - """After installing two wrap presets, agent file contains correctly nested output.""" - from specify_cli.agents import CommandRegistrar - import copy, shutil as _shutil - - core_dir = project_dir / ".specify" / "templates" / "commands" - core_dir.mkdir(parents=True, exist_ok=True) - (core_dir / "specify.md").write_text("---\ndescription: core\n---\n\nCORE\n") + wrap_dir = temp_dir / "wrap-lo" + wrap_dir.mkdir() + with open(wrap_dir / "preset.yml", "w") as f: + yaml.dump(wrap_data, f) + (wrap_dir / "templates").mkdir() + # Intentionally missing {CORE_TEMPLATE} — would error if composition ran + (wrap_dir / "templates" / "spec-template.md").write_text("wrapper without placeholder") - registrar = CommandRegistrar() - original = copy.deepcopy(registrar.AGENT_CONFIGS) - agent_dir = project_dir / ".claude" / "commands" - agent_dir.mkdir(parents=True, exist_ok=True) - registrar.AGENT_CONFIGS["test-agent"] = { - "dir": str(agent_dir.relative_to(project_dir)), - "format": "markdown", "args": "$ARGUMENTS", - "extension": ".md", "strip_frontmatter_keys": [], + manager = PresetManager(project_dir) + manager.install_from_directory(wrap_dir, "0.1.5", priority=10) + + # Install a high-priority replace preset + rep_data = {**valid_pack_data} + rep_data["preset"] = {**valid_pack_data["preset"], "id": "rep-hi", "name": "RepHi"} + rep_data["provides"] = { + "templates": [{ + "type": "template", + "name": "spec-template", + "file": "templates/spec-template.md", + }] } - try: - manager = PresetManager(project_dir) - # Install outermost first (priority=5), then innermost (priority=10) - outer_src = _make_wrap_preset_dir(temp_dir, "preset-outer", "speckit.specify", "OUTER-PRE", "OUTER-POST") - # Rename to avoid id conflict with fixture - inner_src = _make_wrap_preset_dir(temp_dir, "preset-inner", "speckit.specify", "INNER-PRE", "INNER-POST") - manager.install_from_directory(outer_src, "0.1.0", priority=5) - manager.install_from_directory(inner_src, "0.1.0", priority=10) - finally: - CommandRegistrar.AGENT_CONFIGS.clear() - CommandRegistrar.AGENT_CONFIGS.update(original) - - written = (agent_dir / "speckit.specify.md").read_text() - outer_pre = written.index("OUTER-PRE") - inner_pre = written.index("INNER-PRE") - core_pos = written.index("CORE") - inner_post = written.index("INNER-POST") - outer_post = written.index("OUTER-POST") - assert outer_pre < inner_pre < core_pos < inner_post < outer_post - - def test_remove_replays_remaining_wraps(self, project_dir, temp_dir): - """Removing one wrap preset re-composes the remaining wraps correctly.""" - from specify_cli.agents import CommandRegistrar - import copy - - core_dir = project_dir / ".specify" / "templates" / "commands" - core_dir.mkdir(parents=True, exist_ok=True) - (core_dir / "specify.md").write_text("---\ndescription: core\n---\n\nCORE\n") + rep_dir = temp_dir / "rep-hi" + rep_dir.mkdir() + with open(rep_dir / "preset.yml", "w") as f: + yaml.dump(rep_data, f) + (rep_dir / "templates").mkdir() + (rep_dir / "templates" / "spec-template.md").write_text("# Replaced content\n") - registrar = CommandRegistrar() - original = copy.deepcopy(registrar.AGENT_CONFIGS) - agent_dir = project_dir / ".claude" / "commands" - agent_dir.mkdir(parents=True, exist_ok=True) - registrar.AGENT_CONFIGS["test-agent"] = { - "dir": str(agent_dir.relative_to(project_dir)), - "format": "markdown", "args": "$ARGUMENTS", - "extension": ".md", "strip_frontmatter_keys": [], - } - try: - manager = PresetManager(project_dir) - outer_src = _make_wrap_preset_dir(temp_dir, "preset-outer", "speckit.specify", "OUTER-PRE", "OUTER-POST") - inner_src = _make_wrap_preset_dir(temp_dir, "preset-inner", "speckit.specify", "INNER-PRE", "INNER-POST") - manager.install_from_directory(outer_src, "0.1.0", priority=5) - manager.install_from_directory(inner_src, "0.1.0", priority=10) - manager.remove("preset-outer") - finally: - CommandRegistrar.AGENT_CONFIGS.clear() - CommandRegistrar.AGENT_CONFIGS.update(original) + manager.install_from_directory(rep_dir, "0.1.5", priority=1) - written = (agent_dir / "speckit.specify.md").read_text() - # Only inner wrap remains — should be: INNER-PRE + CORE + INNER-POST, no OUTER - assert "INNER-PRE" in written - assert "CORE" in written - assert "INNER-POST" in written - assert "OUTER-PRE" not in written - assert "OUTER-POST" not in written - - def test_wrap_aliases_are_replayed_and_removed(self, project_dir, temp_dir): - """Replay preserves wrap aliases across install/remove lifecycle changes.""" - from specify_cli.agents import CommandRegistrar - import copy + resolver = PresetResolver(project_dir) + content = resolver.resolve_content("spec-template") + assert content == "# Replaced content\n" - core_dir = project_dir / ".specify" / "templates" / "commands" - core_dir.mkdir(parents=True, exist_ok=True) - (core_dir / "specify.md").write_text("---\ndescription: core\n---\n\nCORE\n") - registrar = CommandRegistrar() - original = copy.deepcopy(registrar.AGENT_CONFIGS) - agent_dir = project_dir / ".claude" / "commands" - agent_dir.mkdir(parents=True, exist_ok=True) - registrar.AGENT_CONFIGS["test-agent"] = { - "dir": str(agent_dir.relative_to(project_dir)), - "format": "markdown", "args": "$ARGUMENTS", - "extension": ".md", "strip_frontmatter_keys": [], - } - try: - manager = PresetManager(project_dir) - outer_src = _make_wrap_preset_dir( - temp_dir, - "preset-outer", - "speckit.specify", - "OUTER-PRE", - "OUTER-POST", - aliases=["speckit.alias"], - ) - inner_src = _make_wrap_preset_dir( - temp_dir, "preset-inner", "speckit.specify", "INNER-PRE", "INNER-POST" - ) - manager.install_from_directory(outer_src, "0.1.0", priority=5) - manager.install_from_directory(inner_src, "0.1.0", priority=10) - - alias_file = agent_dir / "speckit.alias.md" - written = alias_file.read_text() - assert "OUTER-PRE" in written - assert "INNER-PRE" in written - assert "INNER-POST" in written - assert "OUTER-POST" in written - - manager.remove("preset-inner") - written = alias_file.read_text() - assert "OUTER-PRE" in written - assert "OUTER-POST" in written - assert "INNER-PRE" not in written - assert "INNER-POST" not in written - - manager.remove("preset-outer") - finally: - CommandRegistrar.AGENT_CONFIGS.clear() - CommandRegistrar.AGENT_CONFIGS.update(original) +class TestCollectAllLayers: + """Test PresetResolver._collect_all_layers() method.""" - assert not (agent_dir / "speckit.alias.md").exists() + def test_single_core_layer(self, project_dir): + """Test collecting layers with only core template.""" + resolver = PresetResolver(project_dir) + layers = resolver.collect_all_layers("spec-template") + assert len(layers) == 1 + assert layers[0]["source"] == "core" + assert layers[0]["strategy"] == "replace" - def test_remove_last_wrap_preset_deletes_agent_file(self, project_dir, temp_dir): - """Removing the only wrap preset deletes the agent command file.""" - from specify_cli.agents import CommandRegistrar - import copy + def test_layers_include_presets(self, project_dir, temp_dir, valid_pack_data): + """Test that layers include installed preset.""" + manager = PresetManager(project_dir) + pack_dir = _create_pack(temp_dir, valid_pack_data, "test-pack", + "# From Pack\n") + manager.install_from_directory(pack_dir, "0.1.5") - core_dir = project_dir / ".specify" / "templates" / "commands" - core_dir.mkdir(parents=True, exist_ok=True) - (core_dir / "specify.md").write_text("---\ndescription: core\n---\n\nCORE\n") + resolver = PresetResolver(project_dir) + layers = resolver.collect_all_layers("spec-template") + assert len(layers) == 2 + # Highest priority first + assert "test-pack" in layers[0]["source"] + assert layers[1]["source"] == "core" + + def test_layers_order_matches_priority(self, project_dir, temp_dir, valid_pack_data): + """Test that layers are ordered by priority (highest first).""" + manager = PresetManager(project_dir) + for pid, prio in [("pack-lo", 10), ("pack-hi", 1)]: + d = {**valid_pack_data} + d["preset"] = {**valid_pack_data["preset"], "id": pid, "name": pid} + p = temp_dir / pid + p.mkdir() + with open(p / "preset.yml", 'w') as f: + yaml.dump(d, f) + (p / "templates").mkdir() + (p / "templates" / "spec-template.md").write_text(f"# {pid}\n") + manager.install_from_directory(p, "0.1.5", priority=prio) - registrar = CommandRegistrar() - original = copy.deepcopy(registrar.AGENT_CONFIGS) - agent_dir = project_dir / ".claude" / "commands" - agent_dir.mkdir(parents=True, exist_ok=True) - registrar.AGENT_CONFIGS["test-agent"] = { - "dir": str(agent_dir.relative_to(project_dir)), - "format": "markdown", "args": "$ARGUMENTS", - "extension": ".md", "strip_frontmatter_keys": [], + resolver = PresetResolver(project_dir) + layers = resolver.collect_all_layers("spec-template") + assert len(layers) == 3 # pack-hi, pack-lo, core + assert "pack-hi" in layers[0]["source"] + assert "pack-lo" in layers[1]["source"] + assert layers[2]["source"] == "core" + + def test_layers_read_strategy_from_manifest(self, project_dir, temp_dir, valid_pack_data): + """Test that layers read strategy from preset manifest.""" + pack_data = {**valid_pack_data} + pack_data["preset"] = {**valid_pack_data["preset"], "id": "strat-pack", "name": "Strat"} + pack_data["provides"] = { + "templates": [{ + "type": "template", + "name": "spec-template", + "file": "templates/spec-template.md", + "strategy": "append", + }] } - try: - manager = PresetManager(project_dir) - src = _make_wrap_preset_dir(temp_dir, "preset-only", "speckit.specify", "PRE", "POST") - manager.install_from_directory(src, "0.1.0", priority=10) - assert (agent_dir / "speckit.specify.md").exists() - manager.remove("preset-only") - finally: - CommandRegistrar.AGENT_CONFIGS.clear() - CommandRegistrar.AGENT_CONFIGS.update(original) + pack_dir = temp_dir / "strat-pack" + pack_dir.mkdir() + with open(pack_dir / "preset.yml", 'w') as f: + yaml.dump(pack_data, f) + (pack_dir / "templates").mkdir() + (pack_dir / "templates" / "spec-template.md").write_text("## Footer\n") - assert not (agent_dir / "speckit.specify.md").exists() - - def test_remove_keeps_registry_entry_when_directory_delete_fails(self, project_dir, monkeypatch): - """A failed preset directory delete must not leave files untracked by the registry.""" manager = PresetManager(project_dir) - pack_dir = manager.presets_dir / "preset-a" - pack_dir.mkdir(parents=True) - manager.registry.add("preset-a", { - "version": "1.0.0", "source": "local", "enabled": True, - "priority": 10, "manifest_hash": "x", - "registered_commands": {}, "registered_skills": [], - "wrap_commands": [], - }) + manager.install_from_directory(pack_dir, "0.1.5") - def fail_rmtree(_path): - raise OSError("locked") + resolver = PresetResolver(project_dir) + layers = resolver.collect_all_layers("spec-template") + # Preset layer should have strategy=append + assert layers[0]["strategy"] == "append" + # Core layer should be replace + assert layers[1]["strategy"] == "replace" - monkeypatch.setattr(shutil, "rmtree", fail_rmtree) - with pytest.raises(OSError, match="locked"): - manager.remove("preset-a") +class TestRemoveReconciliation: + """Test that removing a preset re-registers the next layer's command.""" - assert manager.registry.is_installed("preset-a") - assert pack_dir.exists() + def test_remove_restores_lower_priority_command( + self, project_dir, temp_dir, valid_pack_data + ): + """After removing the top-priority preset, the next preset's command + should be re-registered in agent directories.""" + manager = PresetManager(project_dir) - def test_non_wrap_commands_unaffected_by_wrap_lifecycle(self, project_dir, temp_dir): - """wrap_commands is empty for a preset with no strategy:wrap commands.""" - from specify_cli.agents import CommandRegistrar - import copy - import yaml as _yaml + # Create a gemini commands dir so reconciliation writes there + gemini_dir = project_dir / ".gemini" / "commands" + gemini_dir.mkdir(parents=True) - # Create a preset with a non-wrap command - preset_dir = temp_dir / "non-wrap-preset" - cmd_dir = preset_dir / "commands" - cmd_dir.mkdir(parents=True) - manifest = { - "schema_version": "1.0", - "preset": { - "id": "non-wrap-preset", "name": "Non-wrap", "version": "1.0.0", - "description": "no wrap", "author": "test", - "repository": "https://example.com", "license": "MIT", - }, - "requires": {"speckit_version": ">=0.1.0"}, - "provides": {"templates": [ - {"type": "command", "name": "speckit.specify", - "file": "commands/speckit.specify.md", "description": "override"}, - ]}, - "tags": [], + # Install a low-priority preset with a command + lo_data = {**valid_pack_data} + lo_data["preset"] = { + **valid_pack_data["preset"], + "id": "lo-preset", + "name": "Lo", } - (preset_dir / "preset.yml").write_text(_yaml.dump(manifest)) - (cmd_dir / "speckit.specify.md").write_text( - "---\ndescription: plain override\n---\n\nplain body\n" + lo_data["provides"] = { + "templates": [{ + "type": "command", + "name": "speckit.specify", + "file": "commands/speckit.specify.md", + }] + } + lo_dir = temp_dir / "lo-preset" + lo_dir.mkdir() + with open(lo_dir / "preset.yml", "w") as f: + yaml.dump(lo_data, f) + (lo_dir / "commands").mkdir() + (lo_dir / "commands" / "speckit.specify.md").write_text( + "---\ndescription: lo\n---\nLo content\n" ) - - registrar = CommandRegistrar() - original = copy.deepcopy(registrar.AGENT_CONFIGS) - agent_dir = project_dir / ".claude" / "commands" - agent_dir.mkdir(parents=True, exist_ok=True) - registrar.AGENT_CONFIGS["test-agent"] = { - "dir": str(agent_dir.relative_to(project_dir)), - "format": "markdown", "args": "$ARGUMENTS", - "extension": ".md", "strip_frontmatter_keys": [], + manager.install_from_directory(lo_dir, "0.1.5", priority=10) + + # Install a high-priority preset overriding the same command + hi_data = {**valid_pack_data} + hi_data["preset"] = { + **valid_pack_data["preset"], + "id": "hi-preset", + "name": "Hi", } - try: - manager = PresetManager(project_dir) - manager.install_from_directory(preset_dir, "0.1.0", priority=10) - finally: - CommandRegistrar.AGENT_CONFIGS.clear() - CommandRegistrar.AGENT_CONFIGS.update(original) + hi_data["provides"] = { + "templates": [{ + "type": "command", + "name": "speckit.specify", + "file": "commands/speckit.specify.md", + }] + } + hi_dir = temp_dir / "hi-preset" + hi_dir.mkdir() + with open(hi_dir / "preset.yml", "w") as f: + yaml.dump(hi_data, f) + (hi_dir / "commands").mkdir() + (hi_dir / "commands" / "speckit.specify.md").write_text( + "---\ndescription: hi\n---\nHi content\n" + ) + manager.install_from_directory(hi_dir, "0.1.5", priority=1) - meta = manager.registry.get("non-wrap-preset") - assert meta.get("wrap_commands", []) == [] - written = (agent_dir / "speckit.specify.md").read_text() - assert "plain body" in written + # Verify the hi-preset's content is active in agent dir + cmd_files = list(gemini_dir.glob("*specify*")) + assert cmd_files, "Command file should exist in gemini dir" + assert "Hi content" in cmd_files[0].read_text() + + # Remove the high-priority preset + manager.remove("hi-preset") + + # The low-priority preset's command should now be in the resolution stack + resolver = PresetResolver(project_dir) + layers = resolver.collect_all_layers("speckit.specify", "command") + assert len(layers) >= 1 + assert "lo-preset" in layers[0]["source"] + + # Verify on-disk agent command file switched to lo-preset content + cmd_files = list(gemini_dir.glob("*specify*")) + assert cmd_files, "Command file should still exist after removal" + assert "Lo content" in cmd_files[0].read_text() + + +def _create_pack(temp_dir, valid_pack_data, pack_id, content, + strategy="replace", template_type="template", + template_name="spec-template"): + """Helper to create a preset pack directory.""" + pack_data = {**valid_pack_data} + pack_data["preset"] = {**valid_pack_data["preset"], "id": pack_id, "name": pack_id} + + tmpl_entry = { + "type": template_type, + "name": template_name, + } + if template_type == "script": + tmpl_entry["file"] = f"scripts/{template_name}.sh" + elif template_type == "command": + tmpl_entry["file"] = f"commands/{template_name}.md" + else: + tmpl_entry["file"] = f"templates/{template_name}.md" + if strategy != "replace": + tmpl_entry["strategy"] = strategy + pack_data["provides"] = {"templates": [tmpl_entry]} + + pack_dir = temp_dir / pack_id + pack_dir.mkdir(exist_ok=True) + with open(pack_dir / "preset.yml", 'w') as f: + yaml.dump(pack_data, f) + + if template_type == "script": + subdir = pack_dir / "scripts" + subdir.mkdir(exist_ok=True) + (subdir / f"{template_name}.sh").write_text(content) + elif template_type == "command": + subdir = pack_dir / "commands" + subdir.mkdir(exist_ok=True) + (subdir / f"{template_name}.md").write_text(content) + else: + subdir = pack_dir / "templates" + subdir.mkdir(exist_ok=True) + (subdir / f"{template_name}.md").write_text(content) + + return pack_dir From 60f8d5bdeafc2ac4417f4774b84cd858cc729443 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Tue, 21 Apr 2026 17:12:28 -0500 Subject: [PATCH 02/33] fix: re-create skill directory in _reconcile_skills after removal After _unregister_skills removes a skill directory, _register_skills skips writing because the dir no longer passes the is_dir() check. Fix by ensuring the skill subdirectory exists before calling _register_skills so the next winning preset's content gets registered. Fixes the Claude E2E failure where removing a top-priority override preset left skill-based agents without any SKILL.md file. --- src/specify_cli/presets.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 9100ff4003..6eb37c7322 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -855,11 +855,22 @@ def _reconcile_skills(self, command_names: List[str]) -> None: return resolver = PresetResolver(self.project_root) + skills_dir = self._get_skills_dir() + for cmd_name in command_names: layers = resolver.collect_all_layers(cmd_name, "command") if not layers: continue + # Ensure skill directory exists so _register_skills can write to it. + # After _unregister_skills removes a skill dir, this re-creates it + # so the next winning preset's skill content can be registered. + if skills_dir: + skill_name, _ = self._skill_names_for_command(cmd_name) + skill_subdir = skills_dir / skill_name + if not skill_subdir.exists(): + skill_subdir.mkdir(parents=True, exist_ok=True) + top_path = layers[0]["path"] # Find the preset that owns the winning layer for pack_id, _meta in PresetRegistry(self.presets_dir).list_by_priority(): From 0d2dd42bd42a55cc6bac4be5a436b588e5140a49 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Tue, 21 Apr 2026 17:36:55 -0500 Subject: [PATCH 03/33] fix: address twenty-third round of Copilot PR review feedback - Protect reconciliation in remove(): wrap _reconcile_composed_commands and _reconcile_skills in try/except so failures emit a warning instead of leaving the project in an inconsistent state - Protect reconciliation in install(): same pattern for post-install reconciliation so partial installs don't lack cleanup - Inherit scripts/agent_scripts from base frontmatter: when composing commands, merge scripts and agent_scripts keys from the base command's frontmatter into the top layer's frontmatter if missing, preventing composed commands from losing required script references - Add tier-5 bundled core fallback to collect_all_layers(): check the bundled core_pack (wheel) or repo-root templates (source checkout) when .specify/templates/ doesn't contain the core file, matching resolve()'s tier-5 fallback so composition can always find a base layer --- src/specify_cli/presets.py | 118 ++++++++++++++++++++++++++++++++++--- 1 file changed, 109 insertions(+), 9 deletions(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 6eb37c7322..81f02696bf 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -1359,10 +1359,16 @@ def install_from_directory( t["name"] for t in manifest.templates if t.get("type") == "command" ] if cmd_names: - self._reconcile_composed_commands(cmd_names) - # Also reconcile skills so SKILL.md files reflect the actual - # winning command layer, not just the last-installed preset. - self._reconcile_skills(cmd_names) + try: + self._reconcile_composed_commands(cmd_names) + self._reconcile_skills(cmd_names) + except Exception as exc: + import warnings + warnings.warn( + f"Post-install reconciliation failed for {manifest.id}: {exc}. " + f"Agent command files may not reflect the current priority stack.", + stacklevel=2, + ) return manifest @@ -1471,10 +1477,17 @@ def remove(self, pack_id: str) -> bool: # Reconcile: if other presets still provide these commands, # re-resolve from the remaining stack so the next layer takes effect. if removed_cmd_names: - self._reconcile_composed_commands(list(removed_cmd_names)) - # Also reconcile skills so SKILL.md files reflect the new winning - # command layer rather than being left absent or stale. - self._reconcile_skills(list(removed_cmd_names)) + try: + self._reconcile_composed_commands(list(removed_cmd_names)) + self._reconcile_skills(list(removed_cmd_names)) + except Exception as exc: + import warnings + warnings.warn( + f"Post-removal reconciliation failed for {pack_id}: {exc}. " + f"Agent command files may be stale; reinstall affected presets " + f"or run 'specify preset add' to refresh.", + stacklevel=2, + ) return True @@ -2551,9 +2564,69 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: "source": "core", "strategy": "replace", }) + else: + # Priority 5: Bundled core_pack (wheel install) or repo-root + # templates (source-checkout), matching resolve()'s tier-5 fallback. + bundled = self._find_bundled_core(template_name, template_type, ext) + if bundled: + layers.append({ + "path": bundled, + "source": "core (bundled)", + "strategy": "replace", + }) return layers + def _find_bundled_core( + self, + template_name: str, + template_type: str, + ext: str, + ) -> Optional[Path]: + """Find a core template from the bundled pack or source checkout. + + Mirrors the tier-5 fallback logic in ``resolve()`` so that + ``collect_all_layers()`` can locate base layers even when + ``.specify/templates/`` doesn't contain the core file. + """ + try: + from specify_cli import _locate_core_pack + except ImportError: + return None + + stem = self._core_stem(template_name) + names = [template_name] + if stem and stem != template_name: + names.append(stem) + + core_pack = _locate_core_pack() + if core_pack is not None: + for name in names: + if template_type == "template": + c = core_pack / "templates" / f"{name}.md" + elif template_type == "command": + c = core_pack / "commands" / f"{name}.md" + elif template_type == "script": + c = core_pack / "scripts" / f"{name}{ext}" + else: + c = core_pack / f"{name}.md" + if c.exists(): + return c + else: + repo_root = Path(__file__).parent.parent.parent + for name in names: + if template_type == "template": + c = repo_root / "templates" / f"{name}.md" + elif template_type == "command": + c = repo_root / "templates" / "commands" / f"{name}.md" + elif template_type == "script": + c = repo_root / "scripts" / f"{name}{ext}" + else: + c = repo_root / f"{name}.md" + if c.exists(): + return c + return None + def resolve_content( self, template_name: str, @@ -2613,6 +2686,7 @@ def resolve_content( # layer's frontmatter will be reattached at the end. is_command = template_type == "command" top_frontmatter_text = None + base_frontmatter_text = None def _split_frontmatter(text: str) -> tuple: """Return (frontmatter_block_with_fences, body) or (None, text). @@ -2641,6 +2715,7 @@ def _split_frontmatter(text: str) -> tuple: fm, body = _split_frontmatter(content) if fm: top_frontmatter_text = fm + base_frontmatter_text = fm content = body # Apply composition layers from bottom to top @@ -2679,8 +2754,33 @@ def _split_frontmatter(text: str) -> tuple: ) content = layer_content.replace(placeholder, content) - # Reattach the highest-priority frontmatter for commands + # Reattach the highest-priority frontmatter for commands, + # inheriting scripts/agent_scripts from the base if missing. if is_command and top_frontmatter_text: + if base_frontmatter_text and base_frontmatter_text != top_frontmatter_text: + # Parse both frontmatters to check for missing keys + try: + from .agents import CommandRegistrar + _, _ = CommandRegistrar.parse_frontmatter("placeholder") # ensure import + base_fm, _ = CommandRegistrar.parse_frontmatter( + base_frontmatter_text + "\nbody" + ) + top_fm, _ = CommandRegistrar.parse_frontmatter( + top_frontmatter_text + "\nbody" + ) + inherited = False + for key in ("scripts", "agent_scripts"): + if key not in top_fm and key in base_fm: + top_fm[key] = base_fm[key] + inherited = True + if inherited: + top_frontmatter_text = ( + "---\n" + + yaml.safe_dump(top_fm, sort_keys=False).strip() + + "\n---" + ) + except Exception: + pass # best-effort; fall back to top frontmatter as-is content = top_frontmatter_text + "\n\n" + content return content From a6777870a14be07c66300f2cf0f6a91322463a72 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Tue, 21 Apr 2026 18:08:08 -0500 Subject: [PATCH 04/33] fix: address twenty-fourth round of Copilot PR review feedback - Use yaml.safe_load for frontmatter parsing in resolve_content instead of CommandRegistrar.parse_frontmatter which uses naive find('---',3); strip strategy key from final frontmatter to prevent leaking internal composition directives into rendered agent command files - Filter _reconcile_skills to specific commands: use _FilteredManifest wrapper so only the commands being reconciled get their skills updated, preventing accidental overwrites of other commands' skills that may be owned by higher-priority presets --- src/specify_cli/presets.py | 104 +++++++++++++++++++++++++------------ 1 file changed, 71 insertions(+), 33 deletions(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 81f02696bf..a4509b08ed 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -841,6 +841,27 @@ def _register_for_non_skill_agents( commands, source_id, source_dir, self.project_root ) + class _FilteredManifest: + """Wrapper that exposes only selected command templates from a manifest. + + Used by _reconcile_skills to avoid overwriting skills for commands + that aren't being reconciled. + """ + + def __init__(self, manifest: "PresetManifest", cmd_names: set): + self._manifest = manifest + self._cmd_names = cmd_names + + def __getattr__(self, name: str): + return getattr(self._manifest, name) + + @property + def templates(self) -> List[Dict[str, Any]]: + return [ + t for t in self._manifest.templates + if t.get("name") in self._cmd_names + ] + def _reconcile_skills(self, command_names: List[str]) -> None: """Re-register skills for commands whose winning layer changed. @@ -857,14 +878,16 @@ def _reconcile_skills(self, command_names: List[str]) -> None: resolver = PresetResolver(self.project_root) skills_dir = self._get_skills_dir() + # Group command names by winning preset to batch _register_skills calls + # while only registering skills for the specific commands being reconciled. + preset_cmds: Dict[str, List[str]] = {} + for cmd_name in command_names: layers = resolver.collect_all_layers(cmd_name, "command") if not layers: continue # Ensure skill directory exists so _register_skills can write to it. - # After _unregister_skills removes a skill dir, this re-creates it - # so the next winning preset's skill content can be registered. if skills_dir: skill_name, _ = self._skill_names_for_command(cmd_name) skill_subdir = skills_dir / skill_name @@ -876,15 +899,25 @@ def _reconcile_skills(self, command_names: List[str]) -> None: for pack_id, _meta in PresetRegistry(self.presets_dir).list_by_priority(): pack_dir = self.presets_dir / pack_id if top_path.is_relative_to(pack_dir): - manifest_path = pack_dir / "preset.yml" - if manifest_path.exists(): - try: - manifest = PresetManifest(manifest_path) - except PresetValidationError: - break - self._register_skills(manifest, pack_dir) + preset_cmds.setdefault(pack_id, []).append(cmd_name) break + # Register skills only for the specific commands being reconciled, + # not all commands in each winning preset's manifest. + for pack_id, cmds in preset_cmds.items(): + pack_dir = self.presets_dir / pack_id + manifest_path = pack_dir / "preset.yml" + if not manifest_path.exists(): + continue + try: + manifest = PresetManifest(manifest_path) + except PresetValidationError: + continue + # Filter manifest to only the commands being reconciled + cmds_set = set(cmds) + filtered_manifest = self._FilteredManifest(manifest, cmds_set) + self._register_skills(filtered_manifest, pack_dir) + def _get_skills_dir(self) -> Optional[Path]: """Return the active skills directory for preset skill overrides. @@ -2755,32 +2788,37 @@ def _split_frontmatter(text: str) -> tuple: content = layer_content.replace(placeholder, content) # Reattach the highest-priority frontmatter for commands, - # inheriting scripts/agent_scripts from the base if missing. + # inheriting scripts/agent_scripts from the base if missing + # and stripping the strategy key (internal-only, not for agent output). if is_command and top_frontmatter_text: - if base_frontmatter_text and base_frontmatter_text != top_frontmatter_text: - # Parse both frontmatters to check for missing keys + def _parse_fm_yaml(fm_block: str) -> dict: + """Parse YAML from a frontmatter block (with --- fences).""" + lines = fm_block.splitlines() + # Strip opening/closing --- fences + yaml_lines = [line for line in lines if line.strip() != "---"] try: - from .agents import CommandRegistrar - _, _ = CommandRegistrar.parse_frontmatter("placeholder") # ensure import - base_fm, _ = CommandRegistrar.parse_frontmatter( - base_frontmatter_text + "\nbody" - ) - top_fm, _ = CommandRegistrar.parse_frontmatter( - top_frontmatter_text + "\nbody" - ) - inherited = False - for key in ("scripts", "agent_scripts"): - if key not in top_fm and key in base_fm: - top_fm[key] = base_fm[key] - inherited = True - if inherited: - top_frontmatter_text = ( - "---\n" - + yaml.safe_dump(top_fm, sort_keys=False).strip() - + "\n---" - ) - except Exception: - pass # best-effort; fall back to top frontmatter as-is + return yaml.safe_load("\n".join(yaml_lines)) or {} + except yaml.YAMLError: + return {} + + top_fm = _parse_fm_yaml(top_frontmatter_text) + + # Inherit scripts/agent_scripts from base frontmatter if missing + if base_frontmatter_text and base_frontmatter_text != top_frontmatter_text: + base_fm = _parse_fm_yaml(base_frontmatter_text) + for key in ("scripts", "agent_scripts"): + if key not in top_fm and key in base_fm: + top_fm[key] = base_fm[key] + + # Strip strategy key — it's an internal composition directive, + # not meant for rendered agent command files + top_fm.pop("strategy", None) + + top_frontmatter_text = ( + "---\n" + + yaml.safe_dump(top_fm, sort_keys=False).strip() + + "\n---" + ) content = top_frontmatter_text + "\n\n" + content return content From 9d0e95b5b4fe15d21fbae6cff65937cd5edd4095 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 06:19:04 -0500 Subject: [PATCH 05/33] fix: address twenty-fifth round of Copilot PR review feedback - Support legacy command-frontmatter strategy: when preset.yml doesn't declare a strategy, check the command file's YAML frontmatter for strategy: wrap as a fallback so legacy wrap presets participate in composition and multi-preset chaining - Guard skill dir creation in _reconcile_skills: only re-create the skill directory if the skill was previously managed (listed in some preset's registered_skills), avoiding creation of new skill dirs that _register_skills would normally skip --- src/specify_cli/presets.py | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index a4509b08ed..d5f2224ea9 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -887,12 +887,23 @@ def _reconcile_skills(self, command_names: List[str]) -> None: if not layers: continue - # Ensure skill directory exists so _register_skills can write to it. + # Re-create the skill directory only if it was previously managed + # (i.e., listed in some preset's registered_skills). This avoids + # creating new skill dirs that _register_skills would normally skip. if skills_dir: skill_name, _ = self._skill_names_for_command(cmd_name) skill_subdir = skills_dir / skill_name if not skill_subdir.exists(): - skill_subdir.mkdir(parents=True, exist_ok=True) + # Check if any preset previously registered this skill + was_managed = False + for _pid, meta in PresetRegistry(self.presets_dir).list_by_priority(): + if not isinstance(meta, dict): + continue + if skill_name in meta.get("registered_skills", []): + was_managed = True + break + if was_managed: + skill_subdir.mkdir(parents=True, exist_ok=True) top_path = layers[0]["path"] # Find the preset that owns the winning layer @@ -2525,6 +2536,21 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: if candidate is None: candidate = _find_in_subdirs(pack_dir) if candidate: + # Legacy fallback: if manifest doesn't declare a strategy, + # check the command file's frontmatter for strategy: wrap + if strategy == "replace" and template_type == "command": + try: + cmd_content = candidate.read_text(encoding="utf-8") + if cmd_content.startswith("---"): + fm_yaml = cmd_content.split("---", 2) + if len(fm_yaml) >= 3: + fm_data = yaml.safe_load(fm_yaml[1]) + if isinstance(fm_data, dict): + fm_strategy = fm_data.get("strategy") + if isinstance(fm_strategy, str) and fm_strategy.lower() in VALID_PRESET_STRATEGIES: + strategy = fm_strategy.lower() + except (yaml.YAMLError, OSError): + pass version = metadata.get("version", "?") if metadata else "?" layers.append({ "path": candidate, From b968fc1b3e72e3bf7ebd75e7a351c4e18ce387af Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 06:25:56 -0500 Subject: [PATCH 06/33] fix: add explanatory comment to empty except in legacy frontmatter parsing --- src/specify_cli/presets.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index d5f2224ea9..abf0a36c2e 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -2550,6 +2550,8 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: if isinstance(fm_strategy, str) and fm_strategy.lower() in VALID_PRESET_STRATEGIES: strategy = fm_strategy.lower() except (yaml.YAMLError, OSError): + # Best-effort legacy frontmatter parsing: keep default + # strategy ("replace") when content is unreadable/invalid. pass version = metadata.get("version", "?") if metadata else "?" layers.append({ From df56d7a187a8f660fd051d920ecede4477063b6c Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 07:16:47 -0500 Subject: [PATCH 07/33] fix: address twenty-sixth round of Copilot PR review feedback - Unregister stale commands when composition fails: when resolve_content returns None during reconciliation (base layer removed), unregister the command from non-skill agents and emit a warning - Load extension aliases during reconciliation: _register_command_from_path now checks extension.yml for aliases when the winning layer is an extension, so alias files are restored after preset removal - Use line-based fence detection for legacy frontmatter strategy fallback: scan for --- on its own line instead of split('---',2) to avoid mis-parsing YAML values containing --- --- src/specify_cli/presets.py | 50 ++++++++++++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index abf0a36c2e..5847257445 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -746,6 +746,20 @@ def _reconcile_composed_commands(self, command_names: List[str]) -> None: # Composed command — resolve from full stack composed = resolver.resolve_content(cmd_name, "command") if composed is None: + # Composition no longer possible (e.g. base layer removed). + # Unregister any stale command file from non-skill agents. + import warnings + warnings.warn( + f"Cannot compose command '{cmd_name}': no base layer. " + f"Stale command files may remain.", + stacklevel=2, + ) + registrar._ensure_configs() + registrar.unregister_commands( + {agent: [cmd_name] for agent in registrar.AGENT_CONFIGS + if registrar.AGENT_CONFIGS[agent].get("extension") != "/SKILL.md"}, + self.project_root, + ) continue # Write to the highest-priority preset's .composed dir @@ -808,11 +822,31 @@ def _register_command_from_path( """ if not cmd_path.exists(): return - cmd_tmpl = { + cmd_tmpl: Dict[str, Any] = { "name": cmd_name, "type": "command", "file": cmd_path.name, } + # Load aliases from extension manifest when the winning layer is an extension + if source_id and not source_id.startswith("preset:"): + try: + from .extensions import ExtensionManifest + for ext_dir in self.extensions_dir.iterdir(): + if not ext_dir.is_dir(): + continue + if cmd_path.is_relative_to(ext_dir): + manifest_path = ext_dir / "extension.yml" + if manifest_path.exists(): + ext_manifest = ExtensionManifest(manifest_path) + for cmd in ext_manifest.commands: + if cmd.get("name") == cmd_name: + aliases = cmd.get("aliases", []) + if isinstance(aliases, list) and aliases: + cmd_tmpl["aliases"] = aliases + break + break + except Exception: + pass # best-effort alias loading self._register_for_non_skill_agents( registrar, [cmd_tmpl], source_id, cmd_path.parent ) @@ -2541,10 +2575,16 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: if strategy == "replace" and template_type == "command": try: cmd_content = candidate.read_text(encoding="utf-8") - if cmd_content.startswith("---"): - fm_yaml = cmd_content.split("---", 2) - if len(fm_yaml) >= 3: - fm_data = yaml.safe_load(fm_yaml[1]) + lines = cmd_content.splitlines(keepends=True) + if lines and lines[0].rstrip("\r\n") == "---": + fence_end = -1 + for fi, fline in enumerate(lines[1:], start=1): + if fline.rstrip("\r\n") == "---": + fence_end = fi + break + if fence_end > 0: + fm_text = "".join(lines[1:fence_end]) + fm_data = yaml.safe_load(fm_text) if isinstance(fm_data, dict): fm_strategy = fm_data.get("strategy") if isinstance(fm_strategy, str) and fm_strategy.lower() in VALID_PRESET_STRATEGIES: From 3799a5ec879c0c3f1b803efb6c55514b3f919098 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 08:31:57 -0500 Subject: [PATCH 08/33] fix: address twenty-seventh round of Copilot PR review feedback - Handle non-preset winners in _reconcile_skills: when the winning layer is core/extension/project-override, restore skills via _unregister_skills so skill-based agents stay consistent with the priority stack - Update base_frontmatter_text on replace layers: when a higher-priority replace layer occurs during composition, update both top and base frontmatter so scripts/agent_scripts inheritance reflects the effective base beneath the top composed layer --- src/specify_cli/presets.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 5847257445..40e6c69d3f 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -915,6 +915,7 @@ def _reconcile_skills(self, command_names: List[str]) -> None: # Group command names by winning preset to batch _register_skills calls # while only registering skills for the specific commands being reconciled. preset_cmds: Dict[str, List[str]] = {} + non_preset_skills: List[str] = [] for cmd_name in command_names: layers = resolver.collect_all_layers(cmd_name, "command") @@ -941,11 +942,23 @@ def _reconcile_skills(self, command_names: List[str]) -> None: top_path = layers[0]["path"] # Find the preset that owns the winning layer + found_preset = False for pack_id, _meta in PresetRegistry(self.presets_dir).list_by_priority(): pack_dir = self.presets_dir / pack_id if top_path.is_relative_to(pack_dir): preset_cmds.setdefault(pack_id, []).append(cmd_name) + found_preset = True break + if not found_preset: + # Winner is a non-preset source (core/extension/override). + # Restore skill from that source using _unregister_skills logic. + skill_name, _ = self._skill_names_for_command(cmd_name) + non_preset_skills.append(skill_name) + + # Restore skills for commands whose winner is non-preset + if non_preset_skills and skills_dir: + # Use a dummy preset_dir (won't be read for core/extension restore) + self._unregister_skills(non_preset_skills, self.presets_dir) # Register skills only for the specific commands being reconciled, # not all commands in each winning preset's manifest. @@ -2828,10 +2841,11 @@ def _split_frontmatter(text: str) -> tuple: fm, layer_body = _split_frontmatter(layer_content) layer_content = layer_body # Track the highest-priority frontmatter seen; - # replace layers reset frontmatter (even to None) since + # replace layers reset both top and base frontmatter since # they replace the entire command including metadata. if strategy == "replace": top_frontmatter_text = fm + base_frontmatter_text = fm elif fm: top_frontmatter_text = fm From a7b1660ce076307c68806267116cefac96089f3a Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 08:49:14 -0500 Subject: [PATCH 09/33] fix: address twenty-eighth round of Copilot PR review feedback - Parse only interior lines in _parse_fm_yaml: use lines[1:-1] instead of filtering all --- lines, preventing corruption when YAML values contain a line that is exactly --- - Omit empty frontmatter: skip re-rendering when top_fm is empty dict to avoid emitting ---/{}/--- for intentionally empty frontmatter - Update scaffold wrap comment: mention both {CORE_TEMPLATE} and $CORE_SCRIPT placeholders for templates/commands vs scripts - Clarify shell composition scope in ARCHITECTURE.md: note that bash/PS1 resolve_template_content only handles templates; command/script composition is handled by the Python resolver --- presets/ARCHITECTURE.md | 6 +++--- presets/scaffold/preset.yml | 3 ++- src/specify_cli/presets.py | 25 +++++++++++++++++-------- 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/presets/ARCHITECTURE.md b/presets/ARCHITECTURE.md index d7a45cb9ff..3a119cbd5f 100644 --- a/presets/ARCHITECTURE.md +++ b/presets/ARCHITECTURE.md @@ -55,9 +55,9 @@ Templates, commands, and scripts support a `strategy` field that controls how a Composition is recursive — multiple composing presets chain. The `PresetResolver.resolve_content()` method walks the full priority stack bottom-up and applies each layer's strategy. Content resolution functions for composition: -- **Python**: `PresetResolver.resolve_content()` in `src/specify_cli/presets.py` -- **Bash**: `resolve_template_content()` in `scripts/bash/common.sh` -- **PowerShell**: `Resolve-TemplateContent` in `scripts/powershell/common.ps1` +- **Python**: `PresetResolver.resolve_content()` in `src/specify_cli/presets.py` (templates, commands, and scripts) +- **Bash**: `resolve_template_content()` in `scripts/bash/common.sh` (templates only; command/script composition is handled by the Python resolver) +- **PowerShell**: `Resolve-TemplateContent` in `scripts/powershell/common.ps1` (templates only; command/script composition is handled by the Python resolver) ## Command Registration diff --git a/presets/scaffold/preset.yml b/presets/scaffold/preset.yml index 0180c3375a..65111ba9f3 100644 --- a/presets/scaffold/preset.yml +++ b/presets/scaffold/preset.yml @@ -37,7 +37,8 @@ provides: # replace - Fully replaces the lower-priority template (default) # prepend - Places this content BEFORE the lower-priority template # append - Places this content AFTER the lower-priority template - # wrap - Uses {CORE_TEMPLATE} placeholder, replaced with lower-priority content + # wrap - Uses {CORE_TEMPLATE} placeholder (templates/commands) or + # $CORE_SCRIPT placeholder (scripts), replaced with lower-priority content # # Note: Scripts only support "replace" and "wrap" strategies. - type: "template" diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 40e6c69d3f..ac22238d98 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -2876,8 +2876,11 @@ def _split_frontmatter(text: str) -> tuple: def _parse_fm_yaml(fm_block: str) -> dict: """Parse YAML from a frontmatter block (with --- fences).""" lines = fm_block.splitlines() - # Strip opening/closing --- fences - yaml_lines = [line for line in lines if line.strip() != "---"] + # Parse only interior lines (between --- fences) + if len(lines) >= 2: + yaml_lines = lines[1:-1] + else: + yaml_lines = [] try: return yaml.safe_load("\n".join(yaml_lines)) or {} except yaml.YAMLError: @@ -2896,11 +2899,17 @@ def _parse_fm_yaml(fm_block: str) -> dict: # not meant for rendered agent command files top_fm.pop("strategy", None) - top_frontmatter_text = ( - "---\n" - + yaml.safe_dump(top_fm, sort_keys=False).strip() - + "\n---" - ) - content = top_frontmatter_text + "\n\n" + content + if top_fm: + top_frontmatter_text = ( + "---\n" + + yaml.safe_dump(top_fm, sort_keys=False).strip() + + "\n---" + ) + else: + # Empty frontmatter — omit rather than emitting {} + top_frontmatter_text = None + + if top_frontmatter_text: + content = top_frontmatter_text + "\n\n" + content return content From 0a704886e640ad72b43adf1a05949896e73f2d38 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 09:08:01 -0500 Subject: [PATCH 10/33] fix: address twenty-ninth round of Copilot PR review feedback - Fix TestCollectAllLayers docstring: reference collect_all_layers() - Add default/unknown strategy handling in bash/PS1 composition: error on unrecognized strategy values instead of silently skipping - Fix comment: .composed/ is a persistent dir, not temporary - Fix comment: legacy fallback checks all valid strategies, not just wrap - Cache PresetRegistry in _reconcile_skills: build presets_by_priority once instead of constructing registry per-command --- scripts/bash/common.sh | 2 ++ scripts/powershell/common.ps1 | 2 ++ src/specify_cli/presets.py | 13 +++++++++---- tests/test_presets.py | 2 +- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/scripts/bash/common.sh b/scripts/bash/common.sh index 14cf49a8ac..97e8423aea 100644 --- a/scripts/bash/common.sh +++ b/scripts/bash/common.sh @@ -577,6 +577,7 @@ except Exception: done content="$layer_content" ;; + *) echo "Error: unknown strategy '$strat'" >&2; return 1 ;; esac fi else @@ -596,6 +597,7 @@ except Exception: done content="$layer_content" ;; + *) echo "Error: unknown strategy '$strat'" >&2; return 1 ;; esac fi done diff --git a/scripts/powershell/common.ps1 b/scripts/powershell/common.ps1 index 52fc363b1f..8ce0d65c11 100644 --- a/scripts/powershell/common.ps1 +++ b/scripts/powershell/common.ps1 @@ -547,6 +547,7 @@ except Exception: } $content = $layerContent.Replace('{CORE_TEMPLATE}', $content) } + default { throw "Unknown strategy: $strat" } } } } else { @@ -560,6 +561,7 @@ except Exception: } $content = $layerContent.Replace('{CORE_TEMPLATE}', $content) } + default { throw "Unknown strategy: $strat" } } } } diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index ac22238d98..7318ced447 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -626,7 +626,7 @@ def _register_commands( # Resolve composed content using the full priority stack composed = resolver.resolve_content(cmd["name"], "command") if composed is not None: - # Write composed content to a temporary subdirectory + # Write composed content to the preset's .composed directory if composed_dir is None: composed_dir = preset_dir / ".composed" composed_dir.mkdir(parents=True, exist_ok=True) @@ -912,6 +912,11 @@ def _reconcile_skills(self, command_names: List[str]) -> None: resolver = PresetResolver(self.project_root) skills_dir = self._get_skills_dir() + # Cache registry once to avoid repeated filesystem reads + presets_by_priority = list( + PresetRegistry(self.presets_dir).list_by_priority() + ) if self.presets_dir.exists() else [] + # Group command names by winning preset to batch _register_skills calls # while only registering skills for the specific commands being reconciled. preset_cmds: Dict[str, List[str]] = {} @@ -931,7 +936,7 @@ def _reconcile_skills(self, command_names: List[str]) -> None: if not skill_subdir.exists(): # Check if any preset previously registered this skill was_managed = False - for _pid, meta in PresetRegistry(self.presets_dir).list_by_priority(): + for _pid, meta in presets_by_priority: if not isinstance(meta, dict): continue if skill_name in meta.get("registered_skills", []): @@ -943,7 +948,7 @@ def _reconcile_skills(self, command_names: List[str]) -> None: top_path = layers[0]["path"] # Find the preset that owns the winning layer found_preset = False - for pack_id, _meta in PresetRegistry(self.presets_dir).list_by_priority(): + for pack_id, _meta in presets_by_priority: pack_dir = self.presets_dir / pack_id if top_path.is_relative_to(pack_dir): preset_cmds.setdefault(pack_id, []).append(cmd_name) @@ -2584,7 +2589,7 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: candidate = _find_in_subdirs(pack_dir) if candidate: # Legacy fallback: if manifest doesn't declare a strategy, - # check the command file's frontmatter for strategy: wrap + # check the command file's frontmatter for any valid strategy if strategy == "replace" and template_type == "command": try: cmd_content = candidate.read_text(encoding="utf-8") diff --git a/tests/test_presets.py b/tests/test_presets.py index b9a684e277..64bdc1f0b5 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -4202,7 +4202,7 @@ def test_resolve_content_replace_over_wrap(self, project_dir, temp_dir, valid_pa class TestCollectAllLayers: - """Test PresetResolver._collect_all_layers() method.""" + """Test PresetResolver.collect_all_layers() method.""" def test_single_core_layer(self, project_dir): """Test collecting layers with only core template.""" From 0ec81eb542e29af1fb884d09bdb37f0b71fbfe08 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 09:24:49 -0500 Subject: [PATCH 11/33] fix: address thirtieth round of Copilot PR review feedback - Guard legacy frontmatter fallback: only check command file frontmatter for strategy when the manifest entry doesn't explicitly include the strategy key, preventing override of manifest-declared strategies - Document rollback limitation: note that mid-registration failures may leave orphaned agent command files since partial progress isn't captured by the local vars --- src/specify_cli/presets.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 7318ced447..35b1c314ad 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -1437,9 +1437,12 @@ def install_from_directory( "registered_skills": registered_skills, }) except Exception: - # Roll back all side effects: unregister any commands/skills that - # were written (using local vars which capture partial progress), - # remove the copied preset dir, and drop the registry entry. + # Roll back all side effects. Note: if _register_commands or + # _register_skills raised mid-way (e.g. I/O error after writing + # some files), registered_commands/registered_skills may be empty + # and some agent command files could be orphaned. Removing dest_dir + # (which contains .composed/) and the registry entry ensures the + # preset system is consistent even if orphaned files remain. if registered_commands: self._unregister_commands(registered_commands) if registered_skills: @@ -2571,12 +2574,14 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: # Read strategy and manifest file path from preset manifest strategy = "replace" manifest_file_path = None + manifest_has_strategy = False manifest = self._get_manifest(pack_dir) if manifest: for tmpl in manifest.templates: if (tmpl.get("name") == template_name and tmpl.get("type") == template_type): strategy = tmpl.get("strategy", "replace") + manifest_has_strategy = "strategy" in tmpl manifest_file_path = tmpl.get("file") break # Use manifest file path if specified, otherwise convention-based lookup @@ -2588,9 +2593,11 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: if candidate is None: candidate = _find_in_subdirs(pack_dir) if candidate: - # Legacy fallback: if manifest doesn't declare a strategy, - # check the command file's frontmatter for any valid strategy - if strategy == "replace" and template_type == "command": + # Legacy fallback: if manifest doesn't explicitly declare a + # strategy, check the command file's frontmatter for any valid + # strategy. Skip when the manifest entry includes strategy key + # (even if it's "replace") to avoid overriding explicit declarations. + if not manifest_has_strategy and strategy == "replace" and template_type == "command": try: cmd_content = candidate.read_text(encoding="utf-8") lines = cmd_content.splitlines(keepends=True) From 655476b8edf97430aec22fb5117ba4cc03a94267 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 09:52:16 -0500 Subject: [PATCH 12/33] fix: handle project override skills and extension context in reconciliation --- src/specify_cli/presets.py | 84 +++++++++++++++++++++++++++++++------- 1 file changed, 69 insertions(+), 15 deletions(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 35b1c314ad..cd28817120 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -731,17 +731,29 @@ def _reconcile_composed_commands(self, command_names: List[str]) -> None: if not registered: # Top layer is a non-preset source (extension, core, or # project override). Register directly from the layer path. - # Extract stable source_id from display label source = layers[0]["source"] if source.startswith("extension:"): - # "extension:foo v1.0" → "foo" - source_id = source.split(":", 1)[1].split(" ", 1)[0] - else: - source_id = source - self._register_command_from_path( - registrar, cmd_name, top_path, - source_id=source_id, - ) + # Use extension's own registration to preserve context formatting + ext_id = source.split(":", 1)[1].split(" ", 1)[0] + ext_dir = self.project_root / ".specify" / "extensions" / ext_id + ext_manifest_path = ext_dir / "extension.yml" + if ext_manifest_path.exists(): + try: + from .extensions import ExtensionManifest + ext_manifest = ExtensionManifest(ext_manifest_path) + registrar.register_commands_for_non_skill_agents( + ext_manifest.commands, ext_id, ext_dir, + self.project_root, + ) + registered = True + except Exception: + pass + if not registered: + source_id = source.split(":", 1)[1].split(" ", 1)[0] if source.startswith("extension:") else source + self._register_command_from_path( + registrar, cmd_name, top_path, + source_id=source_id, + ) else: # Composed command — resolve from full stack composed = resolver.resolve_content(cmd_name, "command") @@ -831,7 +843,7 @@ def _register_command_from_path( if source_id and not source_id.startswith("preset:"): try: from .extensions import ExtensionManifest - for ext_dir in self.extensions_dir.iterdir(): + for ext_dir in (self.project_root / ".specify" / "extensions").iterdir(): if not ext_dir.is_dir(): continue if cmd_path.is_relative_to(ext_dir): @@ -956,14 +968,56 @@ def _reconcile_skills(self, command_names: List[str]) -> None: break if not found_preset: # Winner is a non-preset source (core/extension/override). - # Restore skill from that source using _unregister_skills logic. + # Track the winning layer path for skill restoration. skill_name, _ = self._skill_names_for_command(cmd_name) - non_preset_skills.append(skill_name) + non_preset_skills.append((skill_name, cmd_name, layers[0])) - # Restore skills for commands whose winner is non-preset + # Restore skills for commands whose winner is non-preset. + # Use _unregister_skills which restores from core/extension, but + # also handles project overrides by reading the winning layer directly. if non_preset_skills and skills_dir: - # Use a dummy preset_dir (won't be read for core/extension restore) - self._unregister_skills(non_preset_skills, self.presets_dir) + skill_names_only = [s[0] for s in non_preset_skills] + self._unregister_skills(skill_names_only, self.presets_dir) + # For project overrides, _unregister_skills restores from core. + # Re-write from the actual winning layer if it's an override. + for skill_name, cmd_name, top_layer in non_preset_skills: + if top_layer["source"] == "project override": + skill_subdir = skills_dir / skill_name + if skill_subdir.is_dir(): + skill_file = skill_subdir / "SKILL.md" + try: + from .agents import CommandRegistrar + from . import SKILL_DESCRIPTIONS, load_init_options + registrar = CommandRegistrar() + content = top_layer["path"].read_text(encoding="utf-8") + fm, body = registrar.parse_frontmatter(content) + short_name = cmd_name + if short_name.startswith("speckit."): + short_name = short_name[len("speckit."):] + desc = SKILL_DESCRIPTIONS.get( + short_name.replace(".", "-"), + fm.get("description", f"Command: {short_name}"), + ) + init_opts = load_init_options(self.project_root) + selected_ai = init_opts.get("ai") if isinstance(init_opts, dict) else "" + if isinstance(selected_ai, str): + body = registrar.resolve_skill_placeholders( + selected_ai, fm, body, self.project_root + ) + fm_data = registrar.build_skill_frontmatter( + selected_ai if isinstance(selected_ai, str) else "", + skill_name, desc, + f"override:{cmd_name}", + ) + fm_text = yaml.safe_dump(fm_data, sort_keys=False).strip() + skill_title = self._skill_title_from_command(cmd_name) + skill_content = ( + f"---\n{fm_text}\n---\n\n" + f"# Speckit {skill_title} Skill\n\n{body}\n" + ) + skill_file.write_text(skill_content, encoding="utf-8") + except Exception: + pass # best-effort override skill restoration # Register skills only for the specific commands being reconciled, # not all commands in each winning preset's manifest. From 95c6e62d5052cf4402e082c9f374c85164969346 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 10:03:30 -0500 Subject: [PATCH 13/33] fix: add comment to empty except in extension registration fallback --- src/specify_cli/presets.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index cd28817120..94d74ecdd9 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -747,6 +747,8 @@ def _reconcile_composed_commands(self, command_names: List[str]) -> None: ) registered = True except Exception: + # Extension registration failed; fall back to + # generic path-based registration below. pass if not registered: source_id = source.split(":", 1)[1].split(" ", 1)[0] if source.startswith("extension:") else source From 98698fa5cf2ae3d1a07001695f1d0695ab65466a Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 10:31:42 -0500 Subject: [PATCH 14/33] fix: filter extension commands in reconciliation and fix type annotation --- src/specify_cli/presets.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 94d74ecdd9..78236e95d1 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -741,11 +741,17 @@ def _reconcile_composed_commands(self, command_names: List[str]) -> None: try: from .extensions import ExtensionManifest ext_manifest = ExtensionManifest(ext_manifest_path) - registrar.register_commands_for_non_skill_agents( - ext_manifest.commands, ext_id, ext_dir, - self.project_root, - ) - registered = True + # Filter to only the command being reconciled + matching_cmds = [ + c for c in ext_manifest.commands + if c.get("name") == cmd_name + ] + if matching_cmds: + registrar.register_commands_for_non_skill_agents( + matching_cmds, ext_id, ext_dir, + self.project_root, + ) + registered = True except Exception: # Extension registration failed; fall back to # generic path-based registration below. @@ -934,7 +940,7 @@ def _reconcile_skills(self, command_names: List[str]) -> None: # Group command names by winning preset to batch _register_skills calls # while only registering skills for the specific commands being reconciled. preset_cmds: Dict[str, List[str]] = {} - non_preset_skills: List[str] = [] + non_preset_skills: List[tuple] = [] for cmd_name in command_names: layers = resolver.collect_all_layers(cmd_name, "command") From 0a562ace9e088c840b5b6a088e4b111c98cb57dd Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 12:46:02 -0500 Subject: [PATCH 15/33] fix: filter extension commands from post-install reconciliation Apply the same extension-installed check used in _register_commands to the reconciliation command list, preventing reconciliation from registering commands for extensions that are not installed. --- src/specify_cli/presets.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 78236e95d1..e5e85595fa 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -1516,9 +1516,20 @@ def install_from_directory( # Reconcile all affected commands from the full priority stack so that # install order doesn't determine the winning command file. - cmd_names = [ - t["name"] for t in manifest.templates if t.get("type") == "command" - ] + # Apply the same extension-installed filter as _register_commands to + # avoid reconciling extension commands when the extension isn't installed. + extensions_dir = self.project_root / ".specify" / "extensions" + cmd_names = [] + for t in manifest.templates: + if t.get("type") != "command": + continue + name = t["name"] + parts = name.split(".") + if len(parts) >= 3 and parts[0] == "speckit": + ext_id = parts[1] + if not (extensions_dir / ext_id).is_dir(): + continue + cmd_names.append(name) if cmd_names: try: self._reconcile_composed_commands(cmd_names) From 2437d7ae2eb0103f2d1df457357c4f496162e4ff Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 13:15:23 -0500 Subject: [PATCH 16/33] fix: skip convention fallback for explicit file paths and add stem fallback to tier-5 When a preset manifest provides an explicit file path that does not exist, skip the convention-based fallback to avoid masking typos. Also add speckit. to .md fallback in tier-5 bundled/source core lookup for consistency with tier-4. --- src/specify_cli/presets.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index e5e85595fa..58d729d0b8 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -2440,6 +2440,10 @@ def resolve( candidate = _core_pack / "templates" / f"{template_name}.md" elif template_type == "command": candidate = _core_pack / "commands" / f"{template_name}.md" + if not candidate.exists(): + stem = self._core_stem(template_name) + if stem: + candidate = _core_pack / "commands" / f"{stem}.md" elif template_type == "script": candidate = _core_pack / "scripts" / f"{template_name}{ext}" else: @@ -2453,6 +2457,10 @@ def resolve( candidate = repo_root / "templates" / f"{template_name}.md" elif template_type == "command": candidate = repo_root / "templates" / "commands" / f"{template_name}.md" + if not candidate.exists(): + stem = self._core_stem(template_name) + if stem: + candidate = repo_root / "templates" / "commands" / f"{stem}.md" elif template_type == "script": candidate = repo_root / "scripts" / f"{template_name}{ext}" else: @@ -2663,7 +2671,9 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: manifest_candidate = pack_dir / manifest_file_path if manifest_candidate.exists(): candidate = manifest_candidate - if candidate is None: + # Explicit file path that doesn't exist: skip convention + # fallback to avoid masking typos or picking up unintended files. + elif candidate is None: candidate = _find_in_subdirs(pack_dir) if candidate: # Legacy fallback: if manifest doesn't explicitly declare a From 697f056fb016a316392e9efa451b3ec9d460cc2d Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 13:30:47 -0500 Subject: [PATCH 17/33] fix: scan past non-replace layers to find base in resolve_content The base-finding scan now skips non-replace layers below a replace layer instead of stopping at the first non-replace. This fixes the case where a low-priority append/prepend layer sits below a replace that should serve as the base for composition. --- src/specify_cli/presets.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 58d729d0b8..db1cbeee49 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -2875,16 +2875,20 @@ def resolve_content( # layers is ordered highest-priority first. We process in reverse. reversed_layers = list(reversed(layers)) - # Find the base content: the first "replace" layer from the bottom + # Find the base content: scan bottom-up for the highest-priority + # "replace" layer that can serve as a base. Non-replace layers below + # the base are irrelevant (they have nothing to compose onto). content = None start_idx = 0 for i, layer in enumerate(reversed_layers): if layer["strategy"] == "replace": content = layer["path"].read_text(encoding="utf-8") start_idx = i + 1 - else: - # Once we hit a non-replace layer, stop looking for base + elif content is not None: + # Found a non-replace layer above a replace — stop scanning. + # The replace layer(s) form the base; composition starts here. break + # Non-replace layers below any replace are skipped (no base yet) # If no base content found, there's nothing to compose onto if content is None: From a1a0094c36f8f23293d1f7822d3da0074c0fe873 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 13:44:05 -0500 Subject: [PATCH 18/33] fix: add context_note to non-skill agent registration for extensions Add context_note parameter to register_commands_for_non_skill_agents and pass extension name/id during reconciliation so rendered command files preserve the extension-specific context markers. --- src/specify_cli/agents.py | 3 +++ src/specify_cli/presets.py | 2 ++ 2 files changed, 5 insertions(+) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 2c77a86ad6..2a0c627eb3 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -657,6 +657,7 @@ def register_commands_for_non_skill_agents( source_id: str, source_dir: Path, project_root: Path, + context_note: str = None, ) -> Dict[str, List[str]]: """Register commands for all non-skill agents in the project. @@ -669,6 +670,7 @@ def register_commands_for_non_skill_agents( source_id: Identifier of the source source_dir: Directory containing command source files project_root: Path to project root + context_note: Custom context comment for markdown output Returns: Dictionary mapping agent names to list of registered commands @@ -684,6 +686,7 @@ def register_commands_for_non_skill_agents( registered = self.register_commands( agent_name, commands, source_id, source_dir, project_root, + context_note=context_note, ) if registered: results[agent_name] = registered diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index db1cbeee49..77606cdfb4 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -747,9 +747,11 @@ def _reconcile_composed_commands(self, command_names: List[str]) -> None: if c.get("name") == cmd_name ] if matching_cmds: + ext_name = ext_manifest.data.get("extension", {}).get("name", ext_id) registrar.register_commands_for_non_skill_agents( matching_cmds, ext_id, ext_dir, self.project_root, + context_note=f"Extension: {ext_name} ({ext_id})", ) registered = True except Exception: From 75f64d98cb810da93c43858a1d014cbfd8ce6182 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 14:51:25 -0500 Subject: [PATCH 19/33] fix: Optional type, rollback safety, and override skill restoration - Fix context_note type to Optional[str] - Wrap shutil.rmtree in try/except during install rollback - Separate override-backed skills from core/extension in _reconcile_skills --- src/specify_cli/agents.py | 4 +- src/specify_cli/presets.py | 104 +++++++++++++++++++++---------------- 2 files changed, 60 insertions(+), 48 deletions(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 2a0c627eb3..515ec1e973 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -8,7 +8,7 @@ import os from pathlib import Path -from typing import Dict, List, Any +from typing import Dict, List, Any, Optional import platform import re @@ -657,7 +657,7 @@ def register_commands_for_non_skill_agents( source_id: str, source_dir: Path, project_root: Path, - context_note: str = None, + context_note: Optional[str] = None, ) -> Dict[str, List[str]]: """Register commands for all non-skill agents in the project. diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 77606cdfb4..e33ad3f533 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -983,51 +983,60 @@ def _reconcile_skills(self, command_names: List[str]) -> None: non_preset_skills.append((skill_name, cmd_name, layers[0])) # Restore skills for commands whose winner is non-preset. - # Use _unregister_skills which restores from core/extension, but - # also handles project overrides by reading the winning layer directly. if non_preset_skills and skills_dir: - skill_names_only = [s[0] for s in non_preset_skills] - self._unregister_skills(skill_names_only, self.presets_dir) - # For project overrides, _unregister_skills restores from core. - # Re-write from the actual winning layer if it's an override. - for skill_name, cmd_name, top_layer in non_preset_skills: - if top_layer["source"] == "project override": - skill_subdir = skills_dir / skill_name - if skill_subdir.is_dir(): - skill_file = skill_subdir / "SKILL.md" - try: - from .agents import CommandRegistrar - from . import SKILL_DESCRIPTIONS, load_init_options - registrar = CommandRegistrar() - content = top_layer["path"].read_text(encoding="utf-8") - fm, body = registrar.parse_frontmatter(content) - short_name = cmd_name - if short_name.startswith("speckit."): - short_name = short_name[len("speckit."):] - desc = SKILL_DESCRIPTIONS.get( - short_name.replace(".", "-"), - fm.get("description", f"Command: {short_name}"), - ) - init_opts = load_init_options(self.project_root) - selected_ai = init_opts.get("ai") if isinstance(init_opts, dict) else "" - if isinstance(selected_ai, str): - body = registrar.resolve_skill_placeholders( - selected_ai, fm, body, self.project_root - ) - fm_data = registrar.build_skill_frontmatter( - selected_ai if isinstance(selected_ai, str) else "", - skill_name, desc, - f"override:{cmd_name}", - ) - fm_text = yaml.safe_dump(fm_data, sort_keys=False).strip() - skill_title = self._skill_title_from_command(cmd_name) - skill_content = ( - f"---\n{fm_text}\n---\n\n" - f"# Speckit {skill_title} Skill\n\n{body}\n" - ) - skill_file.write_text(skill_content, encoding="utf-8") - except Exception: - pass # best-effort override skill restoration + # Separate override-backed skills from core/extension-backed ones. + # _unregister_skills can rmtree the skill dir, so overrides must + # be handled directly (create dir + write) without that call. + core_ext_skills = [] + override_skills = [] + for item in non_preset_skills: + if item[2]["source"] == "project override": + override_skills.append(item) + else: + core_ext_skills.append(item) + + if core_ext_skills: + self._unregister_skills( + [s[0] for s in core_ext_skills], self.presets_dir + ) + + for skill_name, cmd_name, top_layer in override_skills: + skill_subdir = skills_dir / skill_name + skill_subdir.mkdir(parents=True, exist_ok=True) + skill_file = skill_subdir / "SKILL.md" + try: + from .agents import CommandRegistrar + from . import SKILL_DESCRIPTIONS, load_init_options + registrar = CommandRegistrar() + content = top_layer["path"].read_text(encoding="utf-8") + fm, body = registrar.parse_frontmatter(content) + short_name = cmd_name + if short_name.startswith("speckit."): + short_name = short_name[len("speckit."):] + desc = SKILL_DESCRIPTIONS.get( + short_name.replace(".", "-"), + fm.get("description", f"Command: {short_name}"), + ) + init_opts = load_init_options(self.project_root) + selected_ai = init_opts.get("ai") if isinstance(init_opts, dict) else "" + if isinstance(selected_ai, str): + body = registrar.resolve_skill_placeholders( + selected_ai, fm, body, self.project_root + ) + fm_data = registrar.build_skill_frontmatter( + selected_ai if isinstance(selected_ai, str) else "", + skill_name, desc, + f"override:{cmd_name}", + ) + fm_text = yaml.safe_dump(fm_data, sort_keys=False).strip() + skill_title = self._skill_title_from_command(cmd_name) + skill_content = ( + f"---\n{fm_text}\n---\n\n" + f"# Speckit {skill_title} Skill\n\n{body}\n" + ) + skill_file.write_text(skill_content, encoding="utf-8") + except Exception: + pass # best-effort override skill restoration # Register skills only for the specific commands being reconciled, # not all commands in each winning preset's manifest. @@ -1511,8 +1520,11 @@ def install_from_directory( self._unregister_commands(registered_commands) if registered_skills: self._unregister_skills(registered_skills, dest_dir) - if dest_dir.exists(): - shutil.rmtree(dest_dir) + try: + if dest_dir.exists(): + shutil.rmtree(dest_dir) + except OSError: + pass # best-effort cleanup; don't mask the original error self.registry.remove(manifest.id) raise From d5875f73ecbe218f4a9dc97127bb3f6a11cd8c8f Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 15:53:51 -0500 Subject: [PATCH 20/33] fix: align bash/PS1 base-finding with Python resolver Rewrite bash and PowerShell composition loops to find the effective base replace layer first (scanning bottom-up, skipping non-replace layers below it), then compose only from the base upward. This prevents evaluation of irrelevant lower layers (e.g. a wrap with no placeholder below a replace) and matches resolve_content behavior. --- scripts/bash/common.sh | 92 +++++++++++++++-------------------- scripts/powershell/common.ps1 | 58 +++++++++------------- 2 files changed, 64 insertions(+), 86 deletions(-) diff --git a/scripts/bash/common.sh b/scripts/bash/common.sh index 97e8423aea..b3018db4ed 100644 --- a/scripts/bash/common.sh +++ b/scripts/bash/common.sh @@ -537,69 +537,57 @@ except Exception: return 0 fi - # Compose bottom-up: start from lowest priority + # Compose bottom-up: start from the effective base. + # Find the highest-priority replace layer that sits below composing layers. + # Skip non-replace layers below any replace (they have no base to compose onto). local content="" local has_base=false - local started=false + local base_idx=-1 local i for (( i=count-1; i>=0; i-- )); do + local strat="${layer_strategies[$i]}" + if [ "$strat" = "replace" ]; then + base_idx=$i + elif [ $base_idx -ge 0 ]; then + # Found a non-replace above a replace — this is where composition starts + break + fi + done + + if [ $base_idx -lt 0 ]; then + return 1 # no base layer found + fi + + # Read the base content; start composing from the layer above the base + content=$(cat "${layer_paths[$base_idx]}"; printf x) + content="${content%x}" + + for (( i=base_idx-1; i>=0; i-- )); do local path="${layer_paths[$i]}" local strat="${layer_strategies[$i]}" local layer_content - # Preserve trailing newlines: append sentinel, then strip it + # Preserve trailing newlines layer_content=$(cat "$path"; printf x) layer_content="${layer_content%x}" - if [ "$started" = false ]; then - if [ "$strat" = "replace" ]; then - content="$layer_content" - has_base=true - fi - # Keep consuming replace layers from the bottom until we hit a non-replace - if [ "$strat" != "replace" ]; then - # No base content to compose onto - [ "$has_base" = false ] && return 1 - started=true - case "$strat" in - prepend) content="$(printf '%s\n\n%s' "$layer_content" "$content")" ;; - append) content="$(printf '%s\n\n%s' "$content" "$layer_content")" ;; - wrap) - # Validate placeholder exists - case "$layer_content" in - *'{CORE_TEMPLATE}'*) ;; - *) echo "Error: wrap strategy missing {CORE_TEMPLATE} placeholder" >&2; return 1 ;; - esac - # Replace all occurrences to match Python/PowerShell behavior - while [[ "$layer_content" == *'{CORE_TEMPLATE}'* ]]; do - local before="${layer_content%%\{CORE_TEMPLATE\}*}" - local after="${layer_content#*\{CORE_TEMPLATE\}}" - layer_content="${before}${content}${after}" - done - content="$layer_content" - ;; - *) echo "Error: unknown strategy '$strat'" >&2; return 1 ;; + case "$strat" in + replace) content="$layer_content" ;; + prepend) content="$(printf '%s\n\n%s' "$layer_content" "$content")" ;; + append) content="$(printf '%s\n\n%s' "$content" "$layer_content")" ;; + wrap) + case "$layer_content" in + *'{CORE_TEMPLATE}'*) ;; + *) echo "Error: wrap strategy missing {CORE_TEMPLATE} placeholder" >&2; return 1 ;; esac - fi - else - case "$strat" in - replace) content="$layer_content" ;; - prepend) content="$(printf '%s\n\n%s' "$layer_content" "$content")" ;; - append) content="$(printf '%s\n\n%s' "$content" "$layer_content")" ;; - wrap) - case "$layer_content" in - *'{CORE_TEMPLATE}'*) ;; - *) echo "Error: wrap strategy missing {CORE_TEMPLATE} placeholder" >&2; return 1 ;; - esac - while [[ "$layer_content" == *'{CORE_TEMPLATE}'* ]]; do - local before="${layer_content%%\{CORE_TEMPLATE\}*}" - local after="${layer_content#*\{CORE_TEMPLATE\}}" - layer_content="${before}${content}${after}" - done - content="$layer_content" - ;; - *) echo "Error: unknown strategy '$strat'" >&2; return 1 ;; - esac - fi + while [[ "$layer_content" == *'{CORE_TEMPLATE}'* ]]; do + local before="${layer_content%%\{CORE_TEMPLATE\}*}" + local after="${layer_content#*\{CORE_TEMPLATE\}}" + layer_content="${before}${content}${after}" + done + content="$layer_content" + ;; + *) echo "Error: unknown strategy '$strat'" >&2; return 1 ;; + esac done printf '%s' "$content" diff --git a/scripts/powershell/common.ps1 b/scripts/powershell/common.ps1 index 8ce0d65c11..47950d5792 100644 --- a/scripts/powershell/common.ps1 +++ b/scripts/powershell/common.ps1 @@ -522,47 +522,37 @@ except Exception: return (Get-Content $layerPaths[0] -Raw) } - # Compose bottom-up: start from lowest priority - $content = $null - $started = $false + # Find the effective base: highest-priority replace below composing layers. + # Skip non-replace layers below any replace (they have no base to compose onto). + $baseIdx = -1 for ($i = $layerPaths.Count - 1; $i -ge 0; $i--) { + $strat = $layerStrategies[$i] + if ($strat -eq 'replace') { + $baseIdx = $i + } elseif ($baseIdx -ge 0) { + break # non-replace above a replace — composition starts here + } + } + if ($baseIdx -lt 0) { return $null } + + $content = Get-Content $layerPaths[$baseIdx] -Raw + + for ($i = $baseIdx - 1; $i -ge 0; $i--) { $path = $layerPaths[$i] $strat = $layerStrategies[$i] $layerContent = Get-Content $path -Raw - if (-not $started) { - if ($strat -eq 'replace') { - $content = $layerContent - } - if ($strat -ne 'replace') { - # No base content to compose onto - if ($null -eq $content) { return $null } - $started = $true - switch ($strat) { - 'prepend' { $content = "$layerContent`n`n$content" } - 'append' { $content = "$content`n`n$layerContent" } - 'wrap' { - if (-not $layerContent.Contains('{CORE_TEMPLATE}')) { - throw "Wrap strategy missing {CORE_TEMPLATE} placeholder" - } - $content = $layerContent.Replace('{CORE_TEMPLATE}', $content) - } - default { throw "Unknown strategy: $strat" } - } - } - } else { - switch ($strat) { - 'replace' { $content = $layerContent } - 'prepend' { $content = "$layerContent`n`n$content" } - 'append' { $content = "$content`n`n$layerContent" } - 'wrap' { - if (-not $layerContent.Contains('{CORE_TEMPLATE}')) { - throw "Wrap strategy missing {CORE_TEMPLATE} placeholder" - } - $content = $layerContent.Replace('{CORE_TEMPLATE}', $content) + switch ($strat) { + 'replace' { $content = $layerContent } + 'prepend' { $content = "$layerContent`n`n$content" } + 'append' { $content = "$content`n`n$layerContent" } + 'wrap' { + if (-not $layerContent.Contains('{CORE_TEMPLATE}')) { + throw "Wrap strategy missing {CORE_TEMPLATE} placeholder" } - default { throw "Unknown strategy: $strat" } + $content = $layerContent.Replace('{CORE_TEMPLATE}', $content) } + default { throw "Unknown strategy: $strat" } } } From 7217df4f9746c56246ae17ff08bf4b37f437e8a6 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 16:20:02 -0500 Subject: [PATCH 21/33] fix: PS1 no-python warning, integration hook for override skills, alias cleanup - Warn when no Python 3 found in PS1 and presets use composition strategies - Apply post_process_skill_content integration hook when restoring override-backed skills so agent-specific flags are preserved - Unregister command aliases alongside primary name when composition fails to prevent orphaned alias files --- scripts/powershell/common.ps1 | 10 ++++++++++ src/specify_cli/presets.py | 19 ++++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/scripts/powershell/common.ps1 b/scripts/powershell/common.ps1 index 47950d5792..0f7f197526 100644 --- a/scripts/powershell/common.ps1 +++ b/scripts/powershell/common.ps1 @@ -414,6 +414,16 @@ function Resolve-TemplateContent { if ($sortedPresets.Count -gt 0) { $pyCmd = Get-Python3Command + if (-not $pyCmd) { + # Check if any preset has strategy fields that would be ignored + foreach ($pid in $sortedPresets) { + $mf = Join-Path $presetsDir "$pid/preset.yml" + if ((Test-Path $mf) -and (Select-String -Path $mf -Pattern 'strategy:' -Quiet -ErrorAction SilentlyContinue)) { + Write-Warning "No Python 3 found; preset composition strategies will be ignored" + break + } + } + } foreach ($presetId in $sortedPresets) { # Read strategy and file path from preset manifest $strategy = 'replace' diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index e33ad3f533..3a948fb3ea 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -777,8 +777,20 @@ def _reconcile_composed_commands(self, command_names: List[str]) -> None: stacklevel=2, ) registrar._ensure_configs() + # Include aliases from the top layer's manifest + cmd_names_to_unregister = [cmd_name] + for _pid, _meta in presets_by_priority: + _pd = self.presets_dir / _pid + _m = resolver._get_manifest(_pd) + if _m: + for _t in _m.templates: + if _t.get("name") == cmd_name and _t.get("type") == "command": + for alias in _t.get("aliases", []): + if isinstance(alias, str): + cmd_names_to_unregister.append(alias) + break registrar.unregister_commands( - {agent: [cmd_name] for agent in registrar.AGENT_CONFIGS + {agent: cmd_names_to_unregister for agent in registrar.AGENT_CONFIGS if registrar.AGENT_CONFIGS[agent].get("extension") != "/SKILL.md"}, self.project_root, ) @@ -1034,6 +1046,11 @@ def _reconcile_skills(self, command_names: List[str]) -> None: f"---\n{fm_text}\n---\n\n" f"# Speckit {skill_title} Skill\n\n{body}\n" ) + # Apply integration post-processing (e.g. Claude flags) + from .integrations import get_integration + integration = get_integration(selected_ai) if isinstance(selected_ai, str) else None + if integration is not None and hasattr(integration, "post_process_skill_content"): + skill_content = integration.post_process_skill_content(skill_content) skill_file.write_text(skill_content, encoding="utf-8") except Exception: pass # best-effort override skill restoration From 06b1694c9c5024711c09765a7898dd5e3ebde09c Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 16:43:00 -0500 Subject: [PATCH 22/33] fix: include aliases in removed_cmd_names during preset removal Read aliases from preset manifest before deleting pack_dir so alias command files are included in unregistration and reconciliation. --- src/specify_cli/presets.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 3a948fb3ea..012c8a1402 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -1651,9 +1651,21 @@ def remove(self, pack_id: str) -> bool: # Collect ALL command names before filtering for reconciliation, # so commands registered only for skill-based agents are also reconciled. + # Also include aliases from the manifest (not tracked in registered_commands). removed_cmd_names = set() for cmd_names in registered_commands.values(): removed_cmd_names.update(cmd_names) + manifest_path = pack_dir / "preset.yml" + if manifest_path.exists(): + try: + manifest = PresetManifest(manifest_path) + for tmpl in manifest.templates: + if tmpl.get("type") == "command": + for alias in tmpl.get("aliases", []): + if isinstance(alias, str): + removed_cmd_names.add(alias) + except PresetValidationError: + pass if registered_skills: self._unregister_skills(registered_skills, pack_dir) From 4ba13ae73314bca751dd9a7518145534c5f5efaf Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 16:47:55 -0500 Subject: [PATCH 23/33] fix: add comment to empty except in alias extraction during removal --- src/specify_cli/presets.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 012c8a1402..8ed3565a65 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -1665,6 +1665,8 @@ def remove(self, pack_id: str) -> bool: if isinstance(alias, str): removed_cmd_names.add(alias) except PresetValidationError: + # Invalid manifest — skip alias extraction; primary command + # names from registered_commands are still unregistered. pass if registered_skills: From 9caf29cb7645cd33272f53a7c37c11eafed6ef7b Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 17:31:27 -0500 Subject: [PATCH 24/33] fix: scan top-down for effective base in all resolvers Change base-finding to scan from highest priority downward to find the nearest replace layer, then compose only layers above it. Prevents evaluation of irrelevant lower layers (e.g. a wrap without placeholder below a higher-priority replace) across Python, bash, and PowerShell. --- scripts/bash/common.sh | 17 ++++++----------- scripts/powershell/common.ps1 | 12 +++++------- src/specify_cli/presets.py | 30 +++++++++++++++--------------- 3 files changed, 26 insertions(+), 33 deletions(-) diff --git a/scripts/bash/common.sh b/scripts/bash/common.sh index b3018db4ed..562efeefde 100644 --- a/scripts/bash/common.sh +++ b/scripts/bash/common.sh @@ -537,19 +537,13 @@ except Exception: return 0 fi - # Compose bottom-up: start from the effective base. - # Find the highest-priority replace layer that sits below composing layers. - # Skip non-replace layers below any replace (they have no base to compose onto). - local content="" - local has_base=false + # Find the effective base: scan from highest priority (index 0) downward + # to find the nearest replace layer. Only compose layers above that base. local base_idx=-1 local i - for (( i=count-1; i>=0; i-- )); do - local strat="${layer_strategies[$i]}" - if [ "$strat" = "replace" ]; then + for (( i=0; i Date: Wed, 22 Apr 2026 17:53:08 -0500 Subject: [PATCH 25/33] fix: align CLI composition chain display with top-down base-finding Show only contributing layers (base and above) in preset resolve output, matching resolve_content top-down semantics. Layers below the effective base are omitted since they do not contribute. --- src/specify_cli/__init__.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 6e672758d5..7f744f74c7 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -2620,19 +2620,22 @@ def preset_resolve( else: console.print(" [dim]Final output is composed from multiple preset layers; the path above is the highest-priority contributing layer.[/dim]") console.print("\n [bold]Composition chain:[/bold]") - # Compute the effective base: the last consecutive replace layer - # from the bottom before the first non-replace (same logic as - # PresetResolver.resolve_content). - reversed_display = list(reversed(layers)) - effective_base_idx = 0 - for idx, lyr in enumerate(reversed_display): + # Compute the effective base: first replace layer scanning from + # highest priority (matching resolve_content top-down logic). + # Only show layers from the base upward (lower layers are ignored). + effective_base_idx = None + for idx, lyr in enumerate(layers): if lyr["strategy"] == "replace": effective_base_idx = idx - else: break - for i, layer in enumerate(reversed_display): + # Show only contributing layers (base and above) + if effective_base_idx is not None: + contributing = layers[:effective_base_idx + 1] + else: + contributing = layers + for i, layer in enumerate(reversed(contributing)): strategy_label = layer["strategy"] - if strategy_label == "replace" and i == effective_base_idx: + if strategy_label == "replace" and i == 0: strategy_label = "base" console.print(f" {i + 1}. [{strategy_label}] {layer['source']} → {layer['path']}") else: From f6a7cd8c9321da72c4b79ad2b8ce98a38797da75 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 18:10:16 -0500 Subject: [PATCH 26/33] fix: guard corrupted registry entries and make manifest authoritative - Add isinstance(meta, dict) guard in bash registry parsing so corrupted entries are skipped instead of breaking priority ordering - Only use convention-based file lookup when the manifest does not list the requested template, making preset.yml authoritative and preventing stray on-disk files from creating unintended layers --- scripts/bash/common.sh | 8 ++++---- src/specify_cli/presets.py | 9 +++++++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/scripts/bash/common.sh b/scripts/bash/common.sh index 562efeefde..068eb6d4e1 100644 --- a/scripts/bash/common.sh +++ b/scripts/bash/common.sh @@ -320,8 +320,8 @@ try: with open(os.environ['SPECKIT_REGISTRY']) as f: data = json.load(f) presets = data.get('presets', {}) - for pid, meta in sorted(presets.items(), key=lambda x: x[1].get('priority', 10)): - if meta.get('enabled', True) is not False: + 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 meta.get('enabled', True) is not False: print(pid) except Exception: sys.exit(1) @@ -408,8 +408,8 @@ try: with open(os.environ['SPECKIT_REGISTRY']) as f: data = json.load(f) presets = data.get('presets', {}) - for pid, meta in sorted(presets.items(), key=lambda x: x[1].get('priority', 10)): - if meta.get('enabled', True) is not False: + 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 meta.get('enabled', True) is not False: print(pid) except Exception: sys.exit(1) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 0d501ab3da..bf2de95d7f 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -2701,6 +2701,7 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: strategy = "replace" manifest_file_path = None manifest_has_strategy = False + manifest_found_entry = False manifest = self._get_manifest(pack_dir) if manifest: for tmpl in manifest.templates: @@ -2709,8 +2710,11 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: strategy = tmpl.get("strategy", "replace") manifest_has_strategy = "strategy" in tmpl manifest_file_path = tmpl.get("file") + manifest_found_entry = True break - # Use manifest file path if specified, otherwise convention-based lookup + # Use manifest file path if specified, otherwise convention-based + # lookup — but only when the manifest doesn't exist or doesn't + # list this template, so preset.yml stays authoritative. candidate = None if manifest_file_path: manifest_candidate = pack_dir / manifest_file_path @@ -2718,7 +2722,8 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: candidate = manifest_candidate # Explicit file path that doesn't exist: skip convention # fallback to avoid masking typos or picking up unintended files. - elif candidate is None: + elif not manifest_found_entry: + # Manifest doesn't list this template — check convention paths candidate = _find_in_subdirs(pack_dir) if candidate: # Legacy fallback: if manifest doesn't explicitly declare a From d912227a62b05e3983abba5370f455cf8b6fdb16 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 19:01:32 -0500 Subject: [PATCH 27/33] fix: align resolve() with manifest file paths and match extension context_note - Update resolve() preset tier to consult manifest file paths before convention-based lookup, matching collect_all_layers behavior - Use exact extension context_note format matching extensions.CommandRegistrar - Update test to declare template in manifest (authoritative manifest) --- src/specify_cli/presets.py | 31 ++++++++++++++++++++++--------- tests/test_presets.py | 25 ++++++++++++++++++------- 2 files changed, 40 insertions(+), 16 deletions(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index bf2de95d7f..bad09b6a42 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -747,11 +747,10 @@ def _reconcile_composed_commands(self, command_names: List[str]) -> None: if c.get("name") == cmd_name ] if matching_cmds: - ext_name = ext_manifest.data.get("extension", {}).get("name", ext_id) registrar.register_commands_for_non_skill_agents( matching_cmds, ext_id, ext_dir, self.project_root, - context_note=f"Extension: {ext_name} ({ext_id})", + context_note=f"\n\n\n", ) registered = True except Exception: @@ -2432,13 +2431,27 @@ def resolve( registry = PresetRegistry(self.presets_dir) for pack_id, _metadata in registry.list_by_priority(): pack_dir = self.presets_dir / pack_id - for subdir in subdirs: - if subdir: - candidate = pack_dir / subdir / f"{template_name}{ext}" - else: - candidate = pack_dir / f"{template_name}{ext}" - if candidate.exists(): - return candidate + # Check manifest file path first + manifest = self._get_manifest(pack_dir) + if manifest: + 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: + candidate = pack_dir / file_path + if candidate.exists(): + return candidate + break + # Convention-based fallback (only when manifest doesn't list this template) + else: + for subdir in subdirs: + if subdir: + candidate = pack_dir / subdir / f"{template_name}{ext}" + else: + candidate = pack_dir / f"{template_name}{ext}" + if candidate.exists(): + return candidate # Priority 3: Extension-provided templates (sorted by priority — lower number wins) for _priority, ext_id, _metadata in self._get_all_extensions_by_priority(): diff --git a/tests/test_presets.py b/tests/test_presets.py index 64bdc1f0b5..4e44e9ba53 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2886,17 +2886,28 @@ def test_enable_not_installed(self, project_dir): assert result.exit_code == 1, result.output assert "not installed" in result.output.lower() - def test_disabled_preset_excluded_from_resolution(self, project_dir, pack_dir): + def test_disabled_preset_excluded_from_resolution(self, project_dir, temp_dir, valid_pack_data): """Test that disabled presets are excluded from template resolution.""" - # Install preset with a template + # Install preset with a template declared in the manifest + pack_data = {**valid_pack_data} + pack_data["preset"] = {**valid_pack_data["preset"], "id": "test-pack", "name": "Test"} + pack_data["provides"] = { + "templates": [{ + "type": "template", + "name": "test-template", + "file": "templates/test-template.md", + }] + } + pack_dir = temp_dir / "test-pack" + pack_dir.mkdir(exist_ok=True) + with open(pack_dir / "preset.yml", "w") as f: + yaml.dump(pack_data, f) + (pack_dir / "templates").mkdir(exist_ok=True) + (pack_dir / "templates" / "test-template.md").write_text("# Template from test-pack") + manager = PresetManager(project_dir) manager.install_from_directory(pack_dir, "0.1.5") - # Create a template in the preset directory - preset_template = project_dir / ".specify" / "presets" / "test-pack" / "templates" / "test-template.md" - preset_template.parent.mkdir(parents=True, exist_ok=True) - preset_template.write_text("# Template from test-pack") - resolver = PresetResolver(project_dir) # Template should be found when enabled From fa5626b9ea13fb84c4b4eaf0e8bbb2e4f66438a9 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 22 Apr 2026 19:03:23 -0500 Subject: [PATCH 28/33] revert: restore resolve() convention-based behavior for backwards compatibility resolve() is the existing public API used by shell scripts and other callers. Changing it to manifest-authoritative breaks backward compat for presets that rely on convention-based file lookup. Only the new collect_all_layers/resolve_content path uses manifest-authoritative logic. --- src/specify_cli/presets.py | 28 +++++++--------------------- tests/test_presets.py | 25 +++++++------------------ 2 files changed, 14 insertions(+), 39 deletions(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index bad09b6a42..0224bba95e 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -2431,27 +2431,13 @@ def resolve( registry = PresetRegistry(self.presets_dir) for pack_id, _metadata in registry.list_by_priority(): pack_dir = self.presets_dir / pack_id - # Check manifest file path first - manifest = self._get_manifest(pack_dir) - if manifest: - 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: - candidate = pack_dir / file_path - if candidate.exists(): - return candidate - break - # Convention-based fallback (only when manifest doesn't list this template) - else: - for subdir in subdirs: - if subdir: - candidate = pack_dir / subdir / f"{template_name}{ext}" - else: - candidate = pack_dir / f"{template_name}{ext}" - if candidate.exists(): - return candidate + for subdir in subdirs: + if subdir: + candidate = pack_dir / subdir / f"{template_name}{ext}" + else: + candidate = pack_dir / f"{template_name}{ext}" + if candidate.exists(): + return candidate # Priority 3: Extension-provided templates (sorted by priority — lower number wins) for _priority, ext_id, _metadata in self._get_all_extensions_by_priority(): diff --git a/tests/test_presets.py b/tests/test_presets.py index 4e44e9ba53..64bdc1f0b5 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2886,28 +2886,17 @@ def test_enable_not_installed(self, project_dir): assert result.exit_code == 1, result.output assert "not installed" in result.output.lower() - def test_disabled_preset_excluded_from_resolution(self, project_dir, temp_dir, valid_pack_data): + def test_disabled_preset_excluded_from_resolution(self, project_dir, pack_dir): """Test that disabled presets are excluded from template resolution.""" - # Install preset with a template declared in the manifest - pack_data = {**valid_pack_data} - pack_data["preset"] = {**valid_pack_data["preset"], "id": "test-pack", "name": "Test"} - pack_data["provides"] = { - "templates": [{ - "type": "template", - "name": "test-template", - "file": "templates/test-template.md", - }] - } - pack_dir = temp_dir / "test-pack" - pack_dir.mkdir(exist_ok=True) - with open(pack_dir / "preset.yml", "w") as f: - yaml.dump(pack_data, f) - (pack_dir / "templates").mkdir(exist_ok=True) - (pack_dir / "templates" / "test-template.md").write_text("# Template from test-pack") - + # Install preset with a template manager = PresetManager(project_dir) manager.install_from_directory(pack_dir, "0.1.5") + # Create a template in the preset directory + preset_template = project_dir / ".specify" / "presets" / "test-pack" / "templates" / "test-template.md" + preset_template.parent.mkdir(parents=True, exist_ok=True) + preset_template.write_text("# Template from test-pack") + resolver = PresetResolver(project_dir) # Template should be found when enabled From 38382cb6c177fcb23d16b4479cd97120c82a92bd Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 23 Apr 2026 07:18:31 -0500 Subject: [PATCH 29/33] fix: only pre-compose when this preset is the top composing layer Skip composition in _register_commands when a higher-priority replace layer already wins for the command. Register the raw file instead and let reconciliation write the correct final content. --- src/specify_cli/presets.py | 50 +++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 0224bba95e..7e00143044 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -623,27 +623,37 @@ def _register_commands( for cmd in filtered: strategy = cmd.get("strategy", "replace") if strategy != "replace": - # Resolve composed content using the full priority stack - composed = resolver.resolve_content(cmd["name"], "command") - if composed is not None: - # Write composed content to the preset's .composed directory - if composed_dir is None: - composed_dir = preset_dir / ".composed" - composed_dir.mkdir(parents=True, exist_ok=True) - composed_file = composed_dir / f"{cmd['name']}.md" - composed_file.write_text(composed, encoding="utf-8") - commands_to_register.append({ - **cmd, - "file": f".composed/{cmd['name']}.md", - }) + # Only pre-compose if this preset is the top composing layer. + # If a higher-priority replace already wins, skip composition + # here — reconciliation will write the correct content. + layers = resolver.collect_all_layers(cmd["name"], "command") + top_layer_is_ours = ( + layers and layers[0]["path"].is_relative_to(preset_dir) + ) + if top_layer_is_ours: + composed = resolver.resolve_content(cmd["name"], "command") + if composed is not None: + if composed_dir is None: + composed_dir = preset_dir / ".composed" + composed_dir.mkdir(parents=True, exist_ok=True) + composed_file = composed_dir / f"{cmd['name']}.md" + composed_file.write_text(composed, encoding="utf-8") + commands_to_register.append({ + **cmd, + "file": f".composed/{cmd['name']}.md", + }) + else: + raise PresetValidationError( + f"Command '{cmd['name']}' uses '{strategy}' strategy " + f"but no base command layer exists to compose onto. " + f"Ensure a lower-priority preset, extension, or core " + f"command provides this command before using " + f"composition strategies." + ) else: - raise PresetValidationError( - f"Command '{cmd['name']}' uses '{strategy}' strategy " - f"but no base command layer exists to compose onto. " - f"Ensure a lower-priority preset, extension, or core " - f"command provides this command before using " - f"composition strategies." - ) + # Not the top layer — register raw file; reconciliation + # will overwrite with the correct composed/winning content. + commands_to_register.append(cmd) else: commands_to_register.append(cmd) From 7bb154df83b41d2972eb6cfb02dc792537669973 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 23 Apr 2026 08:32:43 -0500 Subject: [PATCH 30/33] fix: deduplicate PyYAML warnings and use self.registry in reconciliation - Emit PyYAML-missing warning once per function call in bash/PS1 instead of per-preset to avoid spamming stderr - Use self.registry.list_by_priority() in reconciliation methods instead of constructing new PresetRegistry instances to avoid redundant I/O and potential consistency issues --- scripts/bash/common.sh | 7 ++++--- scripts/powershell/common.ps1 | 7 ++++--- src/specify_cli/presets.py | 6 ++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/scripts/bash/common.sh b/scripts/bash/common.sh index 068eb6d4e1..36467f3836 100644 --- a/scripts/bash/common.sh +++ b/scripts/bash/common.sh @@ -415,6 +415,7 @@ except Exception: sys.exit(1) " 2>/dev/null); then if [ -n "$sorted_presets" ]; then + local yaml_warned=false while IFS= read -r preset_id; do # Read strategy and file path from preset manifest local strategy="replace" @@ -449,9 +450,9 @@ except Exception: IFS=$'\t' read -r strategy manifest_file <<< "$result" strategy=$(printf '%s' "$strategy" | tr '[:upper:]' '[:lower:]') fi - # Warn only when PyYAML is explicitly missing - if grep -q 'yaml_missing' "$py_stderr" 2>/dev/null; then - echo "Warning: PyYAML not available; composition strategies in $manifest may be ignored" >&2 + if [ "$yaml_warned" = false ] && grep -q 'yaml_missing' "$py_stderr" 2>/dev/null; then + echo "Warning: PyYAML not available; composition strategies may be ignored" >&2 + yaml_warned=true fi rm -f "$py_stderr" fi diff --git a/scripts/powershell/common.ps1 b/scripts/powershell/common.ps1 index 51beb4b64c..8b887f4545 100644 --- a/scripts/powershell/common.ps1 +++ b/scripts/powershell/common.ps1 @@ -424,6 +424,7 @@ function Resolve-TemplateContent { } } } + $yamlWarned = $false foreach ($presetId in $sortedPresets) { # Read strategy and file path from preset manifest $strategy = 'replace' @@ -458,9 +459,9 @@ except Exception: $strategy = $parts[0].ToLowerInvariant() if ($parts.Count -gt 1 -and $parts[1]) { $manifestFilePath = $parts[1] } } - # Warn only when PyYAML is explicitly missing - if ((Test-Path $pyStderrFile) -and (Get-Content $pyStderrFile -Raw -ErrorAction SilentlyContinue) -match 'yaml_missing') { - Write-Warning "PyYAML not available; composition strategies in $manifest may be ignored" + if (-not $yamlWarned -and (Test-Path $pyStderrFile) -and (Get-Content $pyStderrFile -Raw -ErrorAction SilentlyContinue) -match 'yaml_missing') { + Write-Warning "PyYAML not available; composition strategies may be ignored" + $yamlWarned = $true } Remove-Item $pyStderrFile -Force -ErrorAction SilentlyContinue } catch { diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 7e00143044..d2b98f5b05 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -706,7 +706,7 @@ def _reconcile_composed_commands(self, command_names: List[str]) -> None: # Cache registry and manifests outside the loop to avoid # repeated filesystem reads for each command name. - presets_by_priority = list(PresetRegistry(self.presets_dir).list_by_priority()) if self.presets_dir.exists() else [] + presets_by_priority = list(self.registry.list_by_priority()) for cmd_name in command_names: layers = resolver.collect_all_layers(cmd_name, "command") @@ -956,9 +956,7 @@ def _reconcile_skills(self, command_names: List[str]) -> None: skills_dir = self._get_skills_dir() # Cache registry once to avoid repeated filesystem reads - presets_by_priority = list( - PresetRegistry(self.presets_dir).list_by_priority() - ) if self.presets_dir.exists() else [] + presets_by_priority = list(self.registry.list_by_priority()) # Group command names by winning preset to batch _register_skills calls # while only registering skills for the specific commands being reconciled. From 6636549b28e6a20747cc9ee88e58228ec3b59026 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 23 Apr 2026 08:51:02 -0500 Subject: [PATCH 31/33] fix: document strategy handling consistency between layers and registrar Composed output already strips strategy from frontmatter (resolve_content pops it). Raw file registration preserves legacy frontmatter strategy for backward compat; reconciliation corrects the final state. --- src/specify_cli/presets.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index d2b98f5b05..cab0fe00a5 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -653,6 +653,9 @@ def _register_commands( else: # Not the top layer — register raw file; reconciliation # will overwrite with the correct composed/winning content. + # Note: CommandRegistrar may process frontmatter strategy: wrap + # from the raw file (legacy compat), but reconciliation runs + # immediately after install and corrects the final output. commands_to_register.append(cmd) else: commands_to_register.append(cmd) From 3456fad12178995687f646a1faa22fdccde09c1a Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 23 Apr 2026 09:11:58 -0500 Subject: [PATCH 32/33] fix: correct stale comments for alias tracking and base-finding algorithm --- src/specify_cli/presets.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index cab0fe00a5..ed33f992c3 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -1661,7 +1661,8 @@ def remove(self, pack_id: str) -> bool: # Collect ALL command names before filtering for reconciliation, # so commands registered only for skill-based agents are also reconciled. - # Also include aliases from the manifest (not tracked in registered_commands). + # Also include aliases from the manifest as a safety net for registries + # populated by older versions that may not track aliases. removed_cmd_names = set() for cmd_names in registered_commands.values(): removed_cmd_names.update(cmd_names) @@ -2928,9 +2929,9 @@ def resolve_content( if layers[0]["strategy"] == "replace": return layers[0]["path"].read_text(encoding="utf-8") - # Composition: build content bottom-up (lowest priority first) - # Start from the lowest-priority "replace" layer as the base, - # then apply composition layers on top. + # Composition: build content bottom-up from the effective base. + # The base is the nearest replace layer scanning from highest priority + # downward. Only layers above the base contribute to composition. # # layers is ordered highest-priority first. We process in reverse. reversed_layers = list(reversed(layers)) From dc09a931d5ebf6717cfb318fecf8fc1525e98b21 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 23 Apr 2026 09:42:25 -0500 Subject: [PATCH 33/33] security: validate manifest file paths in bash/PowerShell resolvers Reject absolute paths and parent directory traversal (..) in the manifest-declared file field before joining with the preset directory. Matches the Python-side validation in PresetManifest._validate(). --- scripts/bash/common.sh | 6 ++++++ scripts/powershell/common.ps1 | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/scripts/bash/common.sh b/scripts/bash/common.sh index 36467f3836..cad10bdb39 100644 --- a/scripts/bash/common.sh +++ b/scripts/bash/common.sh @@ -458,6 +458,12 @@ except Exception: fi # Try manifest file path first, then convention path local candidate="" + if [ -n "$manifest_file" ]; then + # Reject absolute paths and parent traversal + case "$manifest_file" in + /*|*../*|../*) manifest_file="" ;; + esac + fi if [ -n "$manifest_file" ]; then local mf="$presets_dir/$preset_id/$manifest_file" [ -f "$mf" ] && candidate="$mf" diff --git a/scripts/powershell/common.ps1 b/scripts/powershell/common.ps1 index 8b887f4545..d799e4f7e7 100644 --- a/scripts/powershell/common.ps1 +++ b/scripts/powershell/common.ps1 @@ -471,6 +471,12 @@ except Exception: } # Try manifest file path first, then convention path $candidate = $null + if ($manifestFilePath) { + # Reject absolute paths and parent traversal + if ([System.IO.Path]::IsPathRooted($manifestFilePath) -or $manifestFilePath -match '\.\.[\\/]') { + $manifestFilePath = '' + } + } if ($manifestFilePath) { $mf = Join-Path $presetsDir "$presetId/$manifestFilePath" if (Test-Path $mf) { $candidate = $mf }