Skip to content

Commit 41a2d30

Browse files
marcelsafinCopilot
andcommitted
fix: keep legacy cleanup project-local
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent dec29f8 commit 41a2d30

4 files changed

Lines changed: 42 additions & 13 deletions

File tree

src/specify_cli/extensions/__init__.py

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1290,15 +1290,10 @@ def _unregister_extension_skills(
12901290
Called during extension removal to clean up skill files that
12911291
were created by ``_register_extension_skills()``.
12921292
1293-
When *skills_dir* is omitted, this is a genuinely unscoped removal
1294-
(e.g. full extension removal): ``registered_skills`` is a single
1295-
flat list covering mirrors created under *every* agent this
1296-
extension was ever activated under, not just the currently active
1297-
one, so we always scan all known agent skills directories rather
1298-
than narrowing to whichever agent happens to be active right now.
1299-
Each candidate directory is verified against the SKILL.md
1300-
``metadata.source`` field before removal to avoid accidentally
1301-
deleting user-created skills with the same name.
1293+
When *skills_dir* is omitted, project-local agent skill directories
1294+
are scanned. Home-scoped outputs require explicit agent provenance:
1295+
the legacy flat registry and ``metadata.source`` marker do not
1296+
identify which project created a global skill.
13021297
13031298
Args:
13041299
skill_names: List of skill names to remove.
@@ -1391,6 +1386,10 @@ def _unregister_extension_skills(
13911386
skills_candidate,
13921387
trusted_root,
13931388
) in self._extension_skill_candidate_dirs().items():
1389+
# ponytail: flat provenance cannot safely own global output;
1390+
# include home paths once registry entries identify project/agent.
1391+
if trusted_root != Path(os.path.abspath(self.project_root)):
1392+
continue
13941393
if not skills_candidate.is_dir():
13951394
continue
13961395
# Reject the candidate directory itself (any path

src/specify_cli/presets/__init__.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2523,7 +2523,7 @@ def _infer_legacy_skill_provenance(
25232523
Copilot — permanently orphaning Copilot's override on later
25242524
removal.
25252525
2526-
Every *configured* integration's skills directory is probed (via
2526+
Every project-local configured integration's skills directory is probed (via
25272527
the same safe, symlink-validated helpers used for
25282528
restore/removal), not only agents whose registrar config is
25292529
statically ``/SKILL.md``-only: a command-backed agent (e.g.
@@ -2569,6 +2569,12 @@ def _infer_legacy_skill_provenance(
25692569
skills_dir = self._safe_skills_dir_for_agent(agent_name)
25702570
if skills_dir is None:
25712571
continue
2572+
# ponytail: legacy markers lack project ownership for global
2573+
# outputs; include home paths once provenance records the project.
2574+
if not Path(os.path.abspath(skills_dir)).is_relative_to(
2575+
Path(os.path.abspath(self.project_root))
2576+
):
2577+
continue
25722578
dir_to_agents.setdefault(skills_dir, []).append(agent_name)
25732579

25742580
marker = f"preset:{pack_id}"

tests/test_extension_skills.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2023,10 +2023,10 @@ def test_remove_while_second_agent_still_in_skills_mode_cleans_up_first_agent_mi
20232023
"directory (#2948)"
20242024
)
20252025

2026-
def test_unscoped_cleanup_removes_hermes_home_skill(
2026+
def test_unscoped_cleanup_preserves_unqualified_hermes_home_skill(
20272027
self, project_dir, temp_dir, monkeypatch
20282028
):
2029-
"""Fallback cleanup must include configured home-relative outputs."""
2029+
"""Flat registry provenance cannot own another project's home output."""
20302030
home = temp_dir / "home"
20312031
monkeypatch.setattr(Path, "home", lambda: home)
20322032
skill_name = "speckit-hermes-cleanup-hello"
@@ -2046,7 +2046,7 @@ def test_unscoped_cleanup_removes_hermes_home_skill(
20462046
[skill_name], "hermes-cleanup"
20472047
)
20482048

2049-
assert not skill_dir.exists()
2049+
assert skill_dir.exists()
20502050

20512051
def test_unregister_agent_artifacts_stays_scoped_when_agent_dir_absent(
20522052
self, project_dir, temp_dir

tests/test_presets.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6438,6 +6438,30 @@ def test_infer_legacy_skill_provenance_skips_invalid_utf8(
64386438
["speckit-specify"], "some-pack", "claude"
64396439
) == {"claude": ["speckit-specify"]}
64406440

6441+
def test_infer_legacy_skill_provenance_excludes_home_outputs(
6442+
self, project_dir, temp_dir, monkeypatch
6443+
):
6444+
home = temp_dir / "home"
6445+
monkeypatch.setattr(Path, "home", lambda: home)
6446+
skill_dir = home / ".hermes" / "skills" / "speckit-specify"
6447+
skill_dir.mkdir(parents=True)
6448+
(skill_dir / "SKILL.md").write_text(
6449+
"---\n"
6450+
"metadata:\n"
6451+
" source: preset:some-pack\n"
6452+
"---\n\n"
6453+
"Other project\n",
6454+
encoding="utf-8",
6455+
)
6456+
6457+
manager = PresetManager(project_dir)
6458+
inferred = manager._infer_legacy_skill_provenance(
6459+
["speckit-specify"], "some-pack", "claude"
6460+
)
6461+
6462+
assert inferred == {"claude": ["speckit-specify"]}
6463+
assert skill_dir.exists()
6464+
64416465
def test_remove_infers_legacy_flat_list_provenance_without_prior_rescaffold(
64426466
self, project_dir, temp_dir
64436467
):

0 commit comments

Comments
 (0)