mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
fix: persist command registration before fallible skills phase on rescaffold
Fix remaining round-6 review findings on the active-only integration registration work (#2948): - register_enabled_presets_for_agent(): registered_commands and registered_skills were merged and persisted together in a single registry.update() call after both the commands and skills phases ran. If _register_skills() raised, the per-preset try/except swallowed it before that update() call was reached, even though _register_commands() had already written a real command file to disk. That file became untracked, so preset removal could no longer clean it up. install_from_directory() already persists registered_commands immediately after the commands phase, before starting the independently fallible skills phase; rescaffold now does the same. - test_presets.py: renamed a misleading claude_dir variable (pointing at Gemini's command directory) in test_composed_none_unregister_respects_active_agent to reuse the existing gemini_commands_dir variable already defined earlier in the same test. Added regression test test_rescaffold_persists_commands_before_fallible_skills_phase: simulates a skills-phase failure during rescaffold and asserts the command file already written to disk is still tracked in registered_commands. Verified all other round-6 findings (preset active-integration scoping, preset reconciliation/remove paths, skills-mode switching, override precedence during rescaffold, skill-subdirectory symlink safety) are already addressed by prior commits in this branch; re-checked each against current code before concluding no further change was needed. Targeted (tests/test_presets.py, tests/test_extensions.py: 689 passed) and full (3935 passed, 109 skipped) suites and ruff check on changed files pass clean. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -762,8 +762,6 @@ class PresetManager:
|
||||
# Isolate per-preset failures: one preset that fails to register
|
||||
# must not abort registration of the remaining enabled presets.
|
||||
try:
|
||||
updates: Dict[str, Any] = {}
|
||||
|
||||
registered_commands = self._register_commands(manifest, pack_dir)
|
||||
existing_commands = metadata.get("registered_commands", {})
|
||||
if not isinstance(existing_commands, dict):
|
||||
@@ -771,8 +769,13 @@ class PresetManager:
|
||||
merged_commands = copy.deepcopy(existing_commands)
|
||||
if registered_commands.get(agent_name):
|
||||
merged_commands[agent_name] = registered_commands[agent_name]
|
||||
# Persist the commands phase immediately, mirroring
|
||||
# install_from_directory(): _register_skills is an
|
||||
# independently fallible phase, and if it raises, the files
|
||||
# the commands phase already wrote to disk must still be
|
||||
# tracked so preset removal can clean them up (#2948).
|
||||
if merged_commands != existing_commands:
|
||||
updates["registered_commands"] = merged_commands
|
||||
self.registry.update(pack_id, {"registered_commands": merged_commands})
|
||||
|
||||
registered_skills = self._register_skills(manifest, pack_dir)
|
||||
existing_skills = self._normalize_registered_skills(
|
||||
@@ -782,10 +785,7 @@ class PresetManager:
|
||||
if registered_skills.get(agent_name):
|
||||
merged_skills[agent_name] = registered_skills[agent_name]
|
||||
if merged_skills != existing_skills:
|
||||
updates["registered_skills"] = merged_skills
|
||||
|
||||
if updates:
|
||||
self.registry.update(pack_id, updates)
|
||||
self.registry.update(pack_id, {"registered_skills": merged_skills})
|
||||
|
||||
for tmpl in manifest.templates:
|
||||
if tmpl.get("type") == "command":
|
||||
|
||||
Reference in New Issue
Block a user