From 5d4288f5711af038ebda1e7d257dfc217908b63e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Fr=C3=B6mbgen?= <23717573+mfroembgen@users.noreply.github.com> Date: Fri, 11 Sep 2026 15:35:12 +0200 Subject: [PATCH 1/2] fix(uninstall): preserve unmanaged skills with an empty inventory --- CHANGELOG.md | 4 + src/apm_cli/commands/uninstall/engine.py | 4 +- .../test_mcp_only_lockfile_lifecycle.py | 73 +++++++++++++++++++ 3 files changed, 80 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ef4721095..3db937af79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Preserve unmanaged Claude and Kiro skills when uninstalling an MCP-only package whose lockfile has no deployed files. — by @mfroembgen (fixes #2946) + ### Added - gh-aw's shared APM import now supports `token-source: github-token`; after consumers re-vendor the workflow, its read-only current-repository identity can fetch same-repository private packages, while `cascade` remains the default and cross-repository packages still require a dedicated token or GitHub App. (#2706) diff --git a/src/apm_cli/commands/uninstall/engine.py b/src/apm_cli/commands/uninstall/engine.py index 4349dace8d..ebcf81fe81 100644 --- a/src/apm_cli/commands/uninstall/engine.py +++ b/src/apm_cli/commands/uninstall/engine.py @@ -1190,7 +1190,9 @@ def _sync_integrations_after_uninstall( authorized_targets.append(target) target_survivor_plan.append((dep_ref, pkg_info, authorized_targets)) - sync_managed = all_deployed_files if all_deployed_files else None + # An empty lockfile inventory still authorizes no file removal. Only + # installations without a lockfile may need legacy orphan detection. + sync_managed = all_deployed_files if lockfile is not None or all_deployed_files else None if sync_managed is not None: # Partition against default KNOWN_TARGETS for legacy/project-scope # paths, then merge with resolved targets for user-scope paths. diff --git a/tests/integration/test_mcp_only_lockfile_lifecycle.py b/tests/integration/test_mcp_only_lockfile_lifecycle.py index 440b742d63..91dedcd3f6 100644 --- a/tests/integration/test_mcp_only_lockfile_lifecycle.py +++ b/tests/integration/test_mcp_only_lockfile_lifecycle.py @@ -8,6 +8,7 @@ import pytest +from apm_cli.deps.lockfile import LockFile from apm_cli.utils.yaml_io import dump_yaml, load_yaml from tests.utils.apm_lifecycle_runner import ApmLifecycleRunner, CommandResult from tests.utils.artifact_snapshot import ArtifactSnapshot, assert_unchanged @@ -144,6 +145,78 @@ def test_mcp_only_install_audit_and_repeat_are_byte_identical( assert_unchanged(first_cache, ArtifactSnapshot.capture(isolated.cache_root)) +@pytest.mark.parametrize("with_skill", (False, True), ids=("mcp-only", "mcp-and-skill")) +def test_uninstall_preserves_unmanaged_skills( + tmp_path: Path, + apm_binary_path: Path, + with_skill: bool, +) -> None: + """Uninstall preserves personal skills while removing owned resources (#2946).""" + isolated = IsolatedApmEnvironment.create( + tmp_path / "unmanaged-skills", base_env=dict(os.environ) + ) + environment = isolated.subprocess_env() + project = LocalPackageFactory(isolated.work_root).create( + "consumer", targets=("claude", "kiro", "codex") + ) + packages = LocalPackageFactory(isolated.package_root) + package = packages.create("mcp-package", mcp_dependencies=(_MCP_DEPENDENCY,)) + if with_skill: + packages.add_skill( + package, + "owned-skill", + "---\nname: owned-skill\ndescription: APM-managed skill.\n---\nOwned skill.\n", + ) + skill_roots = [project.root / target / "skills" for target in (".claude", ".kiro", ".agents")] + for skill_root in skill_roots: + personal = skill_root / "personal" + personal.mkdir(parents=True) + (personal / "SKILL.md").write_text( + "---\nname: personal\ndescription: Personal unmanaged skill.\n---\nPreserve me.\n", + encoding="utf-8", + ) + (personal / "reference.txt").write_text("Personal reference material.\n", encoding="utf-8") + before = [ArtifactSnapshot.capture(root) for root in skill_roots] + personal_before = [ArtifactSnapshot.capture(root / "personal") for root in skill_roots] + runner = ApmLifecycleRunner((str(apm_binary_path),), timeout_seconds=60) + + installed = runner.run( + ("install", str(package.root), "--target", "claude,kiro,codex", "--no-policy"), + scenario_id="unmanaged-skills-install-mcp-only", + cwd=project.root, + env=environment, + ) + _assert_success(installed) + lockfile = LockFile.read(project.root / "apm.lock.yaml") + assert lockfile is not None + assert lockfile.dependencies + deployed_files = {path for dep in lockfile.dependencies.values() for path in dep.deployed_files} + assert bool(deployed_files) == with_skill + for path in deployed_files: + assert (project.root / path).exists() + assert lockfile.mcp_servers + mcp_config = project.root / ".mcp.json" + assert "fixture-mcp" in json.loads(mcp_config.read_text(encoding="utf-8"))["mcpServers"] + for expected in personal_before: + assert_unchanged(expected, ArtifactSnapshot.capture(expected.root)) + + removed = runner.run( + ("uninstall", str(package.root)), + scenario_id="unmanaged-skills-uninstall-mcp-only", + cwd=project.root, + env=environment, + ) + _assert_success(removed) + for expected, root in zip(before, skill_roots, strict=True): + assert_unchanged(expected, ArtifactSnapshot.capture(root)) + assert not (project.root / "apm.lock.yaml").exists() + assert not (project.root / "apm_modules" / "_local" / package.name).exists() + assert not load_yaml(project.manifest_path).get("dependencies", {}).get("apm", []) + assert "fixture-mcp" not in json.loads(mcp_config.read_text(encoding="utf-8"))["mcpServers"] + for path in deployed_files: + assert not (project.root / path).exists() + + @pytest.mark.parametrize( "install_suffix", ( From 6c33f17945dead225f4a25615b594dce501332e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Fr=C3=B6mbgen?= <23717573+mfroembgen@users.noreply.github.com> Date: Fri, 11 Sep 2026 15:35:44 +0200 Subject: [PATCH 2/2] docs(changelog): link unmanaged skill cleanup fix --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3db937af79..bcd26eb873 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Preserve unmanaged Claude and Kiro skills when uninstalling an MCP-only package whose lockfile has no deployed files. — by @mfroembgen (fixes #2946) +- Preserve unmanaged Claude and Kiro skills when uninstalling an MCP-only package whose lockfile has no deployed files. — by @mfroembgen (#2947) ### Added