Skip to content

Commit 7f64380

Browse files
Copilotmnriem
andauthored
fix: Address code review feedback - edge case in bash/ps1 composition, improve default strategy test
Agent-Logs-Url: https://github.com/github/spec-kit/sessions/c285c51b-f00b-480a-9eb2-ae70e3cbcb72 Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>
1 parent fd29a72 commit 7f64380

File tree

3 files changed

+10
-1
lines changed

3 files changed

+10
-1
lines changed

scripts/bash/common.sh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,7 @@ except Exception:
468468

469469
# Compose bottom-up: start from lowest priority
470470
local content=""
471+
local has_base=false
471472
local started=false
472473
local i
473474
for (( i=count-1; i>=0; i-- )); do
@@ -479,9 +480,12 @@ except Exception:
479480
if [ "$started" = false ]; then
480481
if [ "$strat" = "replace" ]; then
481482
content="$layer_content"
483+
has_base=true
482484
fi
483485
# Keep consuming replace layers from the bottom until we hit a non-replace
484486
if [ "$strat" != "replace" ]; then
487+
# No base content to compose onto
488+
[ "$has_base" = false ] && return 1
485489
started=true
486490
case "$strat" in
487491
prepend) content="${layer_content}\n\n${content}" ;;

scripts/powershell/common.ps1

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,8 @@ except Exception:
423423
$content = $layerContent
424424
}
425425
if ($strat -ne 'replace') {
426+
# No base content to compose onto
427+
if ($null -eq $content) { return $null }
426428
$started = $true
427429
switch ($strat) {
428430
'prepend' { $content = "$layerContent`n`n$content" }

tests/test_presets.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2918,8 +2918,11 @@ def test_valid_wrap_strategy(self, temp_dir, valid_pack_data):
29182918
assert manifest.templates[0]["strategy"] == "wrap"
29192919

29202920
def test_default_strategy_is_replace(self, pack_dir):
2921-
"""Test that omitting strategy defaults to replace."""
2921+
"""Test that omitting strategy defaults to replace (key is absent)."""
29222922
manifest = PresetManifest(pack_dir / "preset.yml")
2923+
# Strategy key should not be present in the manifest data
2924+
assert "strategy" not in manifest.templates[0]
2925+
# But consumers should treat missing strategy as "replace"
29232926
assert manifest.templates[0].get("strategy", "replace") == "replace"
29242927

29252928
def test_invalid_strategy_rejected(self, temp_dir, valid_pack_data):

0 commit comments

Comments
 (0)