Skip to content

Commit 753addc

Browse files
Copilotmnriem
andcommitted
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 a7f547d commit 753addc

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
@@ -481,6 +481,7 @@ except Exception:
481481

482482
# Compose bottom-up: start from lowest priority
483483
local content=""
484+
local has_base=false
484485
local started=false
485486
local i
486487
for (( i=count-1; i>=0; i-- )); do
@@ -492,9 +493,12 @@ except Exception:
492493
if [ "$started" = false ]; then
493494
if [ "$strat" = "replace" ]; then
494495
content="$layer_content"
496+
has_base=true
495497
fi
496498
# Keep consuming replace layers from the bottom until we hit a non-replace
497499
if [ "$strat" != "replace" ]; then
500+
# No base content to compose onto
501+
[ "$has_base" = false ] && return 1
498502
started=true
499503
case "$strat" in
500504
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
@@ -482,6 +482,8 @@ except Exception:
482482
$content = $layerContent
483483
}
484484
if ($strat -ne 'replace') {
485+
# No base content to compose onto
486+
if ($null -eq $content) { return $null }
485487
$started = $true
486488
switch ($strat) {
487489
'prepend' { $content = "$layerContent`n`n$content" }

tests/test_presets.py

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

30943094
def test_default_strategy_is_replace(self, pack_dir):
3095-
"""Test that omitting strategy defaults to replace."""
3095+
"""Test that omitting strategy defaults to replace (key is absent)."""
30963096
manifest = PresetManifest(pack_dir / "preset.yml")
3097+
# Strategy key should not be present in the manifest data
3098+
assert "strategy" not in manifest.templates[0]
3099+
# But consumers should treat missing strategy as "replace"
30973100
assert manifest.templates[0].get("strategy", "replace") == "replace"
30983101

30993102
def test_invalid_strategy_rejected(self, temp_dir, valid_pack_data):

0 commit comments

Comments
 (0)