Skip to content

Commit a77f7e6

Browse files
committed
fix: address fifth round of Copilot PR review feedback
- Reconcile all commands from full priority stack after install so install order doesn't determine the winning command file - Fix bash wrap substitution: use split-and-rejoin (%%/# parameter expansion) instead of //pattern/replacement to avoid & expansion corrupting content containing ampersands - Support python/py/py-3 fallbacks in PowerShell Resolve-TemplateContent via Get-Python3Command helper, matching Windows environments where python3 may not be available - Reconciliation skips skill-based agents to avoid overwriting properly formatted SKILL.md files written by _register_skills()
1 parent 3895d0d commit a77f7e6

File tree

3 files changed

+72
-11
lines changed

3 files changed

+72
-11
lines changed

scripts/bash/common.sh

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -542,15 +542,25 @@ except Exception:
542542
case "$strat" in
543543
prepend) content="$(printf '%s\n\n%s' "$layer_content" "$content")" ;;
544544
append) content="$(printf '%s\n\n%s' "$content" "$layer_content")" ;;
545-
wrap) content="${layer_content//\{CORE_TEMPLATE\}/$content}" ;;
545+
wrap)
546+
# Split on placeholder and rejoin with content to avoid
547+
# bash & expansion in parameter substitution replacement.
548+
local before="${layer_content%%\{CORE_TEMPLATE\}*}"
549+
local after="${layer_content#*\{CORE_TEMPLATE\}}"
550+
content="${before}${content}${after}"
551+
;;
546552
esac
547553
fi
548554
else
549555
case "$strat" in
550556
replace) content="$layer_content" ;;
551557
prepend) content="$(printf '%s\n\n%s' "$layer_content" "$content")" ;;
552558
append) content="$(printf '%s\n\n%s' "$content" "$layer_content")" ;;
553-
wrap) content="${layer_content//\{CORE_TEMPLATE\}/$content}" ;;
559+
wrap)
560+
local before="${layer_content%%\{CORE_TEMPLATE\}*}"
561+
local after="${layer_content#*\{CORE_TEMPLATE\}}"
562+
content="${before}${content}${after}"
563+
;;
554564
esac
555565
fi
556566
done

scripts/powershell/common.ps1

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,21 @@ function Test-DirHasFiles {
287287
}
288288
}
289289

290+
# Find a usable Python 3 executable (python3, python, or py -3).
291+
# Returns the command/arguments as an array, or $null if none found.
292+
function Get-Python3Command {
293+
if (Get-Command python3 -ErrorAction SilentlyContinue) { return @('python3') }
294+
if (Get-Command python -ErrorAction SilentlyContinue) {
295+
$ver = & python --version 2>&1
296+
if ($ver -match 'Python 3') { return @('python') }
297+
}
298+
if (Get-Command py -ErrorAction SilentlyContinue) {
299+
$ver = & py -3 --version 2>&1
300+
if ($ver -match 'Python 3') { return @('py', '-3') }
301+
}
302+
return $null
303+
}
304+
290305
# Resolve a template name to a file path using the priority stack:
291306
# 1. .specify/templates/overrides/
292307
# 2. .specify/presets/<preset-id>/templates/ (sorted by priority from .registry)
@@ -401,10 +416,11 @@ function Resolve-TemplateContent {
401416
$strategy = 'replace'
402417
$manifestFilePath = ''
403418
$manifest = Join-Path $presetsDir "$presetId/preset.yml"
404-
if ((Test-Path $manifest) -and (Get-Command python3 -ErrorAction SilentlyContinue)) {
419+
$pyCmd = Get-Python3Command
420+
if ((Test-Path $manifest) -and $pyCmd) {
405421
try {
406-
# Use python3 to parse YAML manifest for strategy and file path
407-
$stratResult = & python3 -c @"
422+
# Use Python to parse YAML manifest for strategy and file path
423+
$stratResult = & $pyCmd[0] @($pyCmd[1..99]) -c @"
408424
import sys
409425
try:
410426
import yaml

src/specify_cli/presets.py

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -646,8 +646,8 @@ def _reconcile_composed_commands(self, command_names: List[str]) -> None:
646646
continue
647647
for tmpl in manifest.templates:
648648
if tmpl.get("name") == cmd_name and tmpl.get("type") == "command":
649-
registrar.register_commands_for_all_agents(
650-
[tmpl], manifest.id, pack_dir, self.project_root
649+
self._register_for_non_skill_agents(
650+
registrar, [tmpl], manifest.id, pack_dir
651651
)
652652
registered = True
653653
break
@@ -681,9 +681,10 @@ def _reconcile_composed_commands(self, command_names: List[str]) -> None:
681681
composed_dir.mkdir(parents=True, exist_ok=True)
682682
composed_file = composed_dir / f"{cmd_name}.md"
683683
composed_file.write_text(composed, encoding="utf-8")
684-
registrar.register_commands_for_all_agents(
684+
self._register_for_non_skill_agents(
685+
registrar,
685686
[{**tmpl, "file": f".composed/{cmd_name}.md"}],
686-
manifest.id, pack_dir, self.project_root,
687+
manifest.id, pack_dir,
687688
)
688689
registered = True
689690
break
@@ -719,10 +720,36 @@ def _register_command_from_path(
719720
"type": "command",
720721
"file": cmd_path.name,
721722
}
722-
registrar.register_commands_for_all_agents(
723-
[cmd_tmpl], "reconciled", cmd_path.parent, self.project_root
723+
self._register_for_non_skill_agents(
724+
registrar, [cmd_tmpl], "reconciled", cmd_path.parent
724725
)
725726

727+
def _register_for_non_skill_agents(
728+
self,
729+
registrar: Any,
730+
commands: List[Dict[str, Any]],
731+
source_id: str,
732+
source_dir: Path,
733+
) -> None:
734+
"""Register commands for all non-skill agents.
735+
736+
Used during reconciliation to avoid overwriting properly formatted
737+
SKILL.md files that were written by _register_skills().
738+
"""
739+
registrar._ensure_configs()
740+
for agent_name, agent_config in registrar.AGENT_CONFIGS.items():
741+
if agent_config.get("extension") == "/SKILL.md":
742+
continue
743+
agent_dir = self.project_root / agent_config["dir"]
744+
if agent_dir.exists():
745+
try:
746+
registrar.register_commands(
747+
agent_name, commands, source_id,
748+
source_dir, self.project_root,
749+
)
750+
except (ValueError, Exception):
751+
continue
752+
726753
def _get_skills_dir(self) -> Optional[Path]:
727754
"""Return the active skills directory for preset skill overrides.
728755
@@ -1186,6 +1213,14 @@ def install_from_directory(
11861213
self.registry.remove(manifest.id)
11871214
raise
11881215

1216+
# Reconcile all affected commands from the full priority stack so that
1217+
# install order doesn't determine the winning command file.
1218+
cmd_names = [
1219+
t["name"] for t in manifest.templates if t.get("type") == "command"
1220+
]
1221+
if cmd_names:
1222+
self._reconcile_composed_commands(cmd_names)
1223+
11891224
return manifest
11901225

11911226
def install_from_zip(

0 commit comments

Comments
 (0)