Skip to content

Commit 506d9c3

Browse files
committed
fix: address ninth round of Copilot PR review feedback
- Add top-replace short-circuit in PowerShell Resolve-TemplateContent: return top layer directly when its strategy is replace, matching Python and bash resolver behavior - Reconcile skills after install: call _reconcile_skills() for affected command names so SKILL.md files reflect the actual priority winner - Promote _collect_all_layers to public collect_all_layers: the method is used by the CLI (preset resolve command), so it should be a public API rather than a private helper
1 parent 3906ea6 commit 506d9c3

File tree

4 files changed

+19
-10
lines changed

4 files changed

+19
-10
lines changed

scripts/powershell/common.ps1

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,12 @@ except Exception:
498498

499499
if ($layerPaths.Count -eq 0) { return $null }
500500

501+
# If the top (highest-priority) layer is replace, it wins entirely —
502+
# lower layers are irrelevant regardless of their strategies.
503+
if ($layerStrategies[0] -eq 'replace') {
504+
return (Get-Content $layerPaths[0] -Raw)
505+
}
506+
501507
# Check if any layer uses a non-replace strategy
502508
$hasComposition = $false
503509
foreach ($s in $layerStrategies) {

src/specify_cli/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2596,7 +2596,7 @@ def preset_resolve(
25962596
raise typer.Exit(1)
25972597

25982598
resolver = PresetResolver(project_root)
2599-
layers = resolver._collect_all_layers(template_name)
2599+
layers = resolver.collect_all_layers(template_name)
26002600
result = resolver.resolve_with_source(template_name)
26012601

26022602
if layers:

src/specify_cli/presets.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,7 @@ def _reconcile_composed_commands(self, command_names: List[str]) -> None:
624624
registrar = CommandRegistrar()
625625

626626
for cmd_name in command_names:
627-
layers = resolver._collect_all_layers(cmd_name, "command")
627+
layers = resolver.collect_all_layers(cmd_name, "command")
628628
if not layers:
629629
continue
630630

@@ -770,7 +770,7 @@ def _reconcile_skills(self, command_names: List[str]) -> None:
770770

771771
resolver = PresetResolver(self.project_root)
772772
for cmd_name in command_names:
773-
layers = resolver._collect_all_layers(cmd_name, "command")
773+
layers = resolver.collect_all_layers(cmd_name, "command")
774774
if not layers:
775775
continue
776776

@@ -1258,6 +1258,9 @@ def install_from_directory(
12581258
]
12591259
if cmd_names:
12601260
self._reconcile_composed_commands(cmd_names)
1261+
# Also reconcile skills so SKILL.md files reflect the actual
1262+
# winning command layer, not just the last-installed preset.
1263+
self._reconcile_skills(cmd_names)
12611264

12621265
return manifest
12631266

@@ -2162,7 +2165,7 @@ def resolve_with_source(
21622165

21632166
return {"path": resolved_str, "source": "core"}
21642167

2165-
def _collect_all_layers(
2168+
def collect_all_layers(
21662169
self,
21672170
template_name: str,
21682171
template_type: str = "template",
@@ -2318,7 +2321,7 @@ def resolve_content(
23182321
Returns:
23192322
Composed content string, or None if not found
23202323
"""
2321-
layers = self._collect_all_layers(template_name, template_type)
2324+
layers = self.collect_all_layers(template_name, template_type)
23222325
if not layers:
23232326
return None
23242327

tests/test_presets.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3541,7 +3541,7 @@ class TestCollectAllLayers:
35413541
def test_single_core_layer(self, project_dir):
35423542
"""Test collecting layers with only core template."""
35433543
resolver = PresetResolver(project_dir)
3544-
layers = resolver._collect_all_layers("spec-template")
3544+
layers = resolver.collect_all_layers("spec-template")
35453545
assert len(layers) == 1
35463546
assert layers[0]["source"] == "core"
35473547
assert layers[0]["strategy"] == "replace"
@@ -3554,7 +3554,7 @@ def test_layers_include_presets(self, project_dir, temp_dir, valid_pack_data):
35543554
manager.install_from_directory(pack_dir, "0.1.5")
35553555

35563556
resolver = PresetResolver(project_dir)
3557-
layers = resolver._collect_all_layers("spec-template")
3557+
layers = resolver.collect_all_layers("spec-template")
35583558
assert len(layers) == 2
35593559
# Highest priority first
35603560
assert "test-pack" in layers[0]["source"]
@@ -3575,7 +3575,7 @@ def test_layers_order_matches_priority(self, project_dir, temp_dir, valid_pack_d
35753575
manager.install_from_directory(p, "0.1.5", priority=prio)
35763576

35773577
resolver = PresetResolver(project_dir)
3578-
layers = resolver._collect_all_layers("spec-template")
3578+
layers = resolver.collect_all_layers("spec-template")
35793579
assert len(layers) == 3 # pack-hi, pack-lo, core
35803580
assert "pack-hi" in layers[0]["source"]
35813581
assert "pack-lo" in layers[1]["source"]
@@ -3604,7 +3604,7 @@ def test_layers_read_strategy_from_manifest(self, project_dir, temp_dir, valid_p
36043604
manager.install_from_directory(pack_dir, "0.1.5")
36053605

36063606
resolver = PresetResolver(project_dir)
3607-
layers = resolver._collect_all_layers("spec-template")
3607+
layers = resolver.collect_all_layers("spec-template")
36083608
# Preset layer should have strategy=append
36093609
assert layers[0]["strategy"] == "append"
36103610
# Core layer should be replace
@@ -3675,7 +3675,7 @@ def test_remove_restores_lower_priority_command(
36753675
# The low-priority preset's command should still be present
36763676
# in the resolution stack
36773677
resolver = PresetResolver(project_dir)
3678-
layers = resolver._collect_all_layers("speckit.specify", "command")
3678+
layers = resolver.collect_all_layers("speckit.specify", "command")
36793679
assert len(layers) >= 1
36803680
assert "lo-preset" in layers[0]["source"]
36813681

0 commit comments

Comments
 (0)