Skip to content

Commit 0d2dd42

Browse files
committed
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
1 parent 60f8d5b commit 0d2dd42

File tree

1 file changed

+109
-9
lines changed

1 file changed

+109
-9
lines changed

src/specify_cli/presets.py

Lines changed: 109 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1359,10 +1359,16 @@ def install_from_directory(
13591359
t["name"] for t in manifest.templates if t.get("type") == "command"
13601360
]
13611361
if cmd_names:
1362-
self._reconcile_composed_commands(cmd_names)
1363-
# Also reconcile skills so SKILL.md files reflect the actual
1364-
# winning command layer, not just the last-installed preset.
1365-
self._reconcile_skills(cmd_names)
1362+
try:
1363+
self._reconcile_composed_commands(cmd_names)
1364+
self._reconcile_skills(cmd_names)
1365+
except Exception as exc:
1366+
import warnings
1367+
warnings.warn(
1368+
f"Post-install reconciliation failed for {manifest.id}: {exc}. "
1369+
f"Agent command files may not reflect the current priority stack.",
1370+
stacklevel=2,
1371+
)
13661372

13671373
return manifest
13681374

@@ -1471,10 +1477,17 @@ def remove(self, pack_id: str) -> bool:
14711477
# Reconcile: if other presets still provide these commands,
14721478
# re-resolve from the remaining stack so the next layer takes effect.
14731479
if removed_cmd_names:
1474-
self._reconcile_composed_commands(list(removed_cmd_names))
1475-
# Also reconcile skills so SKILL.md files reflect the new winning
1476-
# command layer rather than being left absent or stale.
1477-
self._reconcile_skills(list(removed_cmd_names))
1480+
try:
1481+
self._reconcile_composed_commands(list(removed_cmd_names))
1482+
self._reconcile_skills(list(removed_cmd_names))
1483+
except Exception as exc:
1484+
import warnings
1485+
warnings.warn(
1486+
f"Post-removal reconciliation failed for {pack_id}: {exc}. "
1487+
f"Agent command files may be stale; reinstall affected presets "
1488+
f"or run 'specify preset add' to refresh.",
1489+
stacklevel=2,
1490+
)
14781491

14791492
return True
14801493

@@ -2551,9 +2564,69 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]:
25512564
"source": "core",
25522565
"strategy": "replace",
25532566
})
2567+
else:
2568+
# Priority 5: Bundled core_pack (wheel install) or repo-root
2569+
# templates (source-checkout), matching resolve()'s tier-5 fallback.
2570+
bundled = self._find_bundled_core(template_name, template_type, ext)
2571+
if bundled:
2572+
layers.append({
2573+
"path": bundled,
2574+
"source": "core (bundled)",
2575+
"strategy": "replace",
2576+
})
25542577

25552578
return layers
25562579

2580+
def _find_bundled_core(
2581+
self,
2582+
template_name: str,
2583+
template_type: str,
2584+
ext: str,
2585+
) -> Optional[Path]:
2586+
"""Find a core template from the bundled pack or source checkout.
2587+
2588+
Mirrors the tier-5 fallback logic in ``resolve()`` so that
2589+
``collect_all_layers()`` can locate base layers even when
2590+
``.specify/templates/`` doesn't contain the core file.
2591+
"""
2592+
try:
2593+
from specify_cli import _locate_core_pack
2594+
except ImportError:
2595+
return None
2596+
2597+
stem = self._core_stem(template_name)
2598+
names = [template_name]
2599+
if stem and stem != template_name:
2600+
names.append(stem)
2601+
2602+
core_pack = _locate_core_pack()
2603+
if core_pack is not None:
2604+
for name in names:
2605+
if template_type == "template":
2606+
c = core_pack / "templates" / f"{name}.md"
2607+
elif template_type == "command":
2608+
c = core_pack / "commands" / f"{name}.md"
2609+
elif template_type == "script":
2610+
c = core_pack / "scripts" / f"{name}{ext}"
2611+
else:
2612+
c = core_pack / f"{name}.md"
2613+
if c.exists():
2614+
return c
2615+
else:
2616+
repo_root = Path(__file__).parent.parent.parent
2617+
for name in names:
2618+
if template_type == "template":
2619+
c = repo_root / "templates" / f"{name}.md"
2620+
elif template_type == "command":
2621+
c = repo_root / "templates" / "commands" / f"{name}.md"
2622+
elif template_type == "script":
2623+
c = repo_root / "scripts" / f"{name}{ext}"
2624+
else:
2625+
c = repo_root / f"{name}.md"
2626+
if c.exists():
2627+
return c
2628+
return None
2629+
25572630
def resolve_content(
25582631
self,
25592632
template_name: str,
@@ -2613,6 +2686,7 @@ def resolve_content(
26132686
# layer's frontmatter will be reattached at the end.
26142687
is_command = template_type == "command"
26152688
top_frontmatter_text = None
2689+
base_frontmatter_text = None
26162690

26172691
def _split_frontmatter(text: str) -> tuple:
26182692
"""Return (frontmatter_block_with_fences, body) or (None, text).
@@ -2641,6 +2715,7 @@ def _split_frontmatter(text: str) -> tuple:
26412715
fm, body = _split_frontmatter(content)
26422716
if fm:
26432717
top_frontmatter_text = fm
2718+
base_frontmatter_text = fm
26442719
content = body
26452720

26462721
# Apply composition layers from bottom to top
@@ -2679,8 +2754,33 @@ def _split_frontmatter(text: str) -> tuple:
26792754
)
26802755
content = layer_content.replace(placeholder, content)
26812756

2682-
# Reattach the highest-priority frontmatter for commands
2757+
# Reattach the highest-priority frontmatter for commands,
2758+
# inheriting scripts/agent_scripts from the base if missing.
26832759
if is_command and top_frontmatter_text:
2760+
if base_frontmatter_text and base_frontmatter_text != top_frontmatter_text:
2761+
# Parse both frontmatters to check for missing keys
2762+
try:
2763+
from .agents import CommandRegistrar
2764+
_, _ = CommandRegistrar.parse_frontmatter("placeholder") # ensure import
2765+
base_fm, _ = CommandRegistrar.parse_frontmatter(
2766+
base_frontmatter_text + "\nbody"
2767+
)
2768+
top_fm, _ = CommandRegistrar.parse_frontmatter(
2769+
top_frontmatter_text + "\nbody"
2770+
)
2771+
inherited = False
2772+
for key in ("scripts", "agent_scripts"):
2773+
if key not in top_fm and key in base_fm:
2774+
top_fm[key] = base_fm[key]
2775+
inherited = True
2776+
if inherited:
2777+
top_frontmatter_text = (
2778+
"---\n"
2779+
+ yaml.safe_dump(top_fm, sort_keys=False).strip()
2780+
+ "\n---"
2781+
)
2782+
except Exception:
2783+
pass # best-effort; fall back to top frontmatter as-is
26842784
content = top_frontmatter_text + "\n\n" + content
26852785

26862786
return content

0 commit comments

Comments
 (0)