github/github-spec-kit
1
0
Fork 0
mirror of https://github.com/github/spec-kit.git synced 2026-08-03 06:26:30 +08:00
Code Issues Packages Projects Releases Wiki Activity

Keep unregister_agent_artifacts scoped to its agent when directory is absent

Browse Source 
ExtensionManager.unregister_agent_artifacts() converted its resolved
agent_skills_dir to None whenever that directory didn't exist, before
calling _unregister_extension_skills(). After 1d8f9e3, omitting
skills_dir means "genuinely unscoped removal": scan every configured
agent's directory, reserved for ExtensionManager.remove()'s full
project cleanup. Since unregister_agent_artifacts is agent-scoped (used
by switch to clean up the previous integration's artifacts), this
caused it to delete every other agent's live extension skill mirrors
whenever the target agent's own directory happened to be absent, e.g.
unregistering an agent that was never activated.

Fix: always pass the explicit, agent-scoped skills_dir, even when it
doesn't exist on disk, so the fast path is a safe no-op for an absent
directory instead of falling back to the all-agents scan. Registry
reconciliation (dropping removed names from the flat registered_skills
list) now only runs when the agent's directory actually exists, so an
absent directory can't be misread as "these names were removed
everywhere" and wipe tracking for mirrors that still legitimately live
under other agents' directories.

Added regression test:
- test_unregister_agent_artifacts_stays_scoped_when_agent_dir_absent

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
marcelsafin
2026-07-11 11:08:11 +02:00
parent 1d8f9e3989
commit 0ab9a5f3f4
2 changed files with 101 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

50
src/specify_cli/extensions/__init__.py
View File

@@ -1871,30 +1871,44 @@ class ExtensionManager:
metadata.get("registered_skills", [])
)
if registered_skills:
# Only pass the resolved skills_dir when it actually exists.
# Otherwise let _unregister_extension_skills fall back to
# scanning all known agent skills directories, which is useful
# for cleaning up stale entries created by earlier installs.
skills_dir = agent_skills_dir if agent_skills_dir.is_dir() else None
# Always pass the explicit, agent-scoped skills_dir — even
# when it doesn't currently exist on disk. This method must
# stay scoped to *this* agent only; omitting skills_dir (a
# bare ``None``) tells _unregister_extension_skills "this is
# a genuinely unscoped removal", which triggers its
# all-configured-agents fallback scan — reserved for
# ExtensionManager.remove()'s full project cleanup. If this
# agent's directory doesn't exist, there is nothing under it
# to clean up; the fast path below is a safe no-op in that
# case (every candidate skill_subdir.is_dir() check fails).
self._unregister_extension_skills(
registered_skills, ext_id, skills_dir=skills_dir
registered_skills, ext_id, skills_dir=agent_skills_dir
)
# Only reconcile registry state when cleanup was scoped to a
# specific existing directory. When skills_dir is None,
# _unregister_extension_skills falls back to scanning multiple
# candidate directories, so agent_skills_dir cannot be used to
# infer what was removed. When skills_dir is set,
# _unregister_extension_skills may intentionally skip deletion
# when ownership cannot be verified (e.g., corrupted/missing
# SKILL.md or mismatching metadata.source). Only drop registry
# entries for skill directories that were actually removed so
# future cleanup attempts can still find skipped ones.
if skills_dir is not None:
# Only reconcile registry state when this agent's directory
# actually exists. When it's absent, this agent never had
# any of these skills mirrored under its own directory in
# the first place, so there is nothing to conclude about
# global ``registered_skills`` tracking from that absence —
# other agents' directories may still legitimately hold
# live mirrors for these same names (the flat list is
# agent-agnostic). Recomputing "remaining" against an
# absent directory would incorrectly conclude every name
# was removed and drop them all from the registry, silently
# orphaning any still-live mirrors under other agents'
# directories from future cleanup/removal.
#
# When the directory does exist, _unregister_extension_skills
# may intentionally skip deletion when ownership cannot be
# verified (e.g., corrupted/missing SKILL.md or mismatching
# metadata.source). Only drop registry entries for skill
# directories that were actually removed so future cleanup
# attempts can still find skipped ones.
if agent_skills_dir.is_dir():
remaining_skills = [
skill_name
for skill_name in registered_skills
if (skills_dir / skill_name).is_dir()
if (agent_skills_dir / skill_name).is_dir()
]
if remaining_skills != registered_skills:
updates["registered_skills"] = remaining_skills

69
tests/test_extension_skills.py
View File

@@ -1510,6 +1510,75 @@ class TestExtensionSkillRegistration:
"directory (#2948)"
)
def test_unregister_agent_artifacts_stays_scoped_when_agent_dir_absent(
self, project_dir, temp_dir
):
"""``unregister_agent_artifacts`` must remain scoped to the given
agent even when that agent's own skills directory does not exist —
it must never fall through to the genuinely-unscoped, all-directory
removal semantics reserved for ``remove()``.
Auggie and Copilot are both activated in skills mode, each writing
its own mirror. ``unregister_agent_artifacts("cursor-agent")`` is
then called for a supported agent that was *never* activated, so
its skills directory does not exist on disk. Before this fix, the
caller converted the resolved-but-absent directory to ``None``
before calling ``_unregister_extension_skills`` — and after the
prior fix (1d8f9e3), omitting ``skills_dir`` means "scan and clean
up every configured agent's directory", not "this specific agent
has nothing to clean up". That silently deleted Auggie's and
Copilot's still-live mirrors while "cleaning up" an agent that was
never even active.
"""
_create_init_options(project_dir, ai="auggie", ai_skills=True)
manager = ExtensionManager(project_dir)
manager.install_from_directory(
_create_extension_dir(temp_dir, ext_id="scoped-unregister-ext"), "0.1.0",
register_commands=False,
)
manager.register_enabled_extensions_for_agent("auggie")
auggie_skills_dir = project_dir / ".augment" / "skills"
auggie_hello = auggie_skills_dir / "speckit-scoped-unregister-ext-hello" / "SKILL.md"
auggie_world = auggie_skills_dir / "speckit-scoped-unregister-ext-world" / "SKILL.md"
assert auggie_hello.exists() and auggie_world.exists(), (
"sanity: auggie's skills-mode activation should mirror both "
"extension commands as SKILL.md files"
)
_create_init_options(project_dir, ai="copilot", ai_skills=True)
manager.register_enabled_extensions_for_agent("copilot")
copilot_skills_dir = project_dir / ".github" / "skills"
copilot_hello = copilot_skills_dir / "speckit-scoped-unregister-ext-hello" / "SKILL.md"
copilot_world = copilot_skills_dir / "speckit-scoped-unregister-ext-world" / "SKILL.md"
assert copilot_hello.exists() and copilot_world.exists(), (
"sanity: copilot's skills-mode activation should also mirror "
"both extension commands"
)
cursor_skills_dir = project_dir / ".cursor" / "skills"
assert not cursor_skills_dir.exists(), (
"sanity: cursor-agent was never activated so it has no "
"skills directory on disk"
)
# Unregister artifacts for an agent that was never activated (its
# skills directory is absent). This must be a no-op with respect
# to other agents' live mirrors.
manager.unregister_agent_artifacts("cursor-agent")
assert auggie_hello.exists() and auggie_world.exists(), (
"unregistering a never-activated agent's artifacts must not "
"delete auggie's live mirror — an absent target directory "
"must not widen cleanup to every configured agent directory"
)
assert copilot_hello.exists() and copilot_world.exists(), (
"unregistering a never-activated agent's artifacts must not "
"delete copilot's live mirror — an absent target directory "
"must not widen cleanup to every configured agent directory"
)
def test_extension_owned_skill_names_rejects_symlinked_candidate_directory(
self, project_dir, temp_dir
):