mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
Fix unscoped extension-skill removal and legacy preset provenance on direct remove
- _unregister_extension_skills(): omitting skills_dir now always triggers the full multi-directory fallback scan instead of narrowing to the currently active agent's directory. Previously, remove() (the only caller that omits skills_dir) would resolve the active agent's dir and take the scoped fast path, orphaning a previously-active second agent's extension skill mirror during full removal. - PresetManager.remove(): infer legacy flat-list registered_skills provenance (reusing _infer_legacy_skill_provenance from the prior rescaffold fix) before invoking _unregister_skills, so a direct `preset remove` with no intervening rescaffold/switch also restores every previously-active agent's directory instead of only the currently active one. Added regression tests: - test_remove_while_second_agent_still_in_skills_mode_cleans_up_first_agent_mirror - test_remove_infers_legacy_flat_list_provenance_without_prior_rescaffold Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -1241,32 +1241,32 @@ class ExtensionManager:
|
||||
Called during extension removal to clean up skill files that
|
||||
were created by ``_register_extension_skills()``.
|
||||
|
||||
If *skills_dir* is not provided and ``_get_skills_dir()`` returns
|
||||
``None`` (e.g. the user removed init-options.json or toggled
|
||||
ai_skills after installation), we fall back to scanning all known
|
||||
agent skills directories so that orphaned skill directories are
|
||||
still cleaned up. In that case each candidate directory is
|
||||
verified against the SKILL.md ``metadata.source`` field before
|
||||
removal to avoid accidentally deleting user-created skills with
|
||||
the same name.
|
||||
When *skills_dir* is omitted, this is a genuinely unscoped removal
|
||||
(e.g. full extension removal): ``registered_skills`` is a single
|
||||
flat list covering mirrors created under *every* agent this
|
||||
extension was ever activated under, not just the currently active
|
||||
one, so we always scan all known agent skills directories rather
|
||||
than narrowing to whichever agent happens to be active right now.
|
||||
Each candidate directory is verified against the SKILL.md
|
||||
``metadata.source`` field before removal to avoid accidentally
|
||||
deleting user-created skills with the same name.
|
||||
|
||||
Args:
|
||||
skill_names: List of skill names to remove.
|
||||
extension_id: Extension ID used to verify ownership during
|
||||
fallback candidate scanning.
|
||||
skills_dir: Optional explicit skills directory to use instead
|
||||
of resolving via ``_get_skills_dir()``. Useful when the
|
||||
caller needs to target a specific agent's skills directory
|
||||
regardless of the currently-active agent in init-options.
|
||||
skills_dir: Optional explicit skills directory to scope
|
||||
cleanup to. Useful when the caller needs to target a
|
||||
specific agent's skills directory regardless of the
|
||||
currently-active agent in init-options. When omitted,
|
||||
every configured agent's skills directory is scanned
|
||||
instead of resolving just the currently active one.
|
||||
"""
|
||||
if not skill_names:
|
||||
return
|
||||
|
||||
from ..shared_infra import _validate_safe_shared_directory
|
||||
|
||||
if skills_dir is None:
|
||||
skills_dir = self._get_skills_dir()
|
||||
|
||||
if skills_dir:
|
||||
# Reject the candidate directory itself (any path component,
|
||||
# including the final one) if it's a symlink escaping the
|
||||
|
||||
@@ -2247,6 +2247,30 @@ class PresetManager:
|
||||
metadata = self.registry.get(pack_id)
|
||||
# Restore original skills when preset is removed
|
||||
registered_skills = metadata.get("registered_skills", []) if metadata else []
|
||||
if isinstance(registered_skills, list) and registered_skills:
|
||||
# Legacy flat-list registries predate per-agent provenance
|
||||
# tracking. Migration to the per-agent dict form previously
|
||||
# only happened during a rescaffold (register_enabled_presets_
|
||||
# for_agent); if the *first* post-upgrade operation is instead
|
||||
# `preset remove` (no intervening use/upgrade), the legacy
|
||||
# branch of _unregister_skills restores only the currently
|
||||
# active agent's directory, leaving this preset's overrides in
|
||||
# every previously active agent's directory orphaned. Infer
|
||||
# real per-agent ownership from the on-disk preset marker now,
|
||||
# while pack_id is still known, and hand the resulting mapping
|
||||
# through the same dict-based cleanup path already used for
|
||||
# non-legacy registries (#2948).
|
||||
from .. import load_init_options
|
||||
|
||||
init_opts = load_init_options(self.project_root)
|
||||
fallback_agent = init_opts.get("ai") if isinstance(init_opts, dict) else None
|
||||
if not isinstance(fallback_agent, str):
|
||||
fallback_agent = ""
|
||||
registered_skills = self._infer_legacy_skill_provenance(
|
||||
[name for name in registered_skills if isinstance(name, str)],
|
||||
pack_id,
|
||||
fallback_agent=fallback_agent,
|
||||
)
|
||||
registered_commands = metadata.get("registered_commands", {}) if metadata else {}
|
||||
pack_dir = self.presets_dir / pack_id
|
||||
|
||||
|
||||
Reference in New Issue
Block a user