From ed93abed02e03d5423fd85c1f547121f406a06b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=B8=87=E5=AE=B6=E8=89=AF?= <102887115+wjl8636@users.noreply.github.com> Date: Tue, 13 Oct 2026 23:32:31 +0800 Subject: [PATCH] fix(cli): label each uninstall skill root by its client (#2197) * fix(cli): label each uninstall skill root by its client rea uninstall reported every managed skill destination under the shared "skill" label (with only Claude Code special-cased), so a retained or failed directory could not be told apart from the others. Use the destination's client identifier so the aggregated detail names each root: "shared: retained (...)", "claude_code: removed (...)", "antigravity: retained (...)". Retained and failed entries now point to the specific directory that needs attention. Fixes #2111 Co-Authored-By: Claude Fable 5 * test(cli): align skill summary assertions with client and path labels --------- Co-authored-by: wjl8636 Co-authored-by: Claude Fable 5 Co-authored-by: morluto <76467478+morluto@users.noreply.github.com> --- src/application/Uninstall.ts | 13 +++-- .../cli/clientProfileLifecycle.test.ts | 7 ++- .../filesystem/clientLifecycle.test.ts | 49 ++++++++++++++++--- 3 files changed, 55 insertions(+), 14 deletions(-) diff --git a/src/application/Uninstall.ts b/src/application/Uninstall.ts index a4561c45e..eac36c73d 100644 --- a/src/application/Uninstall.ts +++ b/src/application/Uninstall.ts @@ -627,11 +627,16 @@ const removeManagedSkills = async ( platform: NodeJS.Platform, writeBlocker?: (path: string) => string | undefined, ): Promise => { - // Every destination shares one summary item, so each result is labeled by - // the directory it refers to. + // Every destination shares one summary item; preserve both client and path. const results = await Promise.all( - skillDestinations(home, undefined, environment, platform).map(({ path }) => - removeManagedSkill(path, path, fileSystem, writeBlocker), + skillDestinations(home, undefined, environment, platform).map( + ({ client, path }) => + removeManagedSkill( + path, + `${client} (${path})`, + fileSystem, + writeBlocker, + ), ), ); const failed = results.find(({ status }) => status === "failed"); diff --git a/tests/boundary/cli/clientProfileLifecycle.test.ts b/tests/boundary/cli/clientProfileLifecycle.test.ts index 4fd566fd7..8a71b669d 100644 --- a/tests/boundary/cli/clientProfileLifecycle.test.ts +++ b/tests/boundary/cli/clientProfileLifecycle.test.ts @@ -287,9 +287,12 @@ cliTest("uninstall names each retained skill directory", async ({ cli }) => { }; const skill = report.items?.find(({ name }) => name === "skill"); expect(skill?.status).toBe("retained"); - for (const directory of [shared, claude]) { + for (const [client, directory] of [ + ["shared", shared], + ["claude_code", claude], + ]) { expect(skill?.detail).toContain( - `${directory}: retained (Skill manifest is not REA-owned.)`, + `${client} (${directory}): retained (Skill manifest is not REA-owned.)`, ); expect(await readFile(join(directory, "SKILL.md"), "utf8")).toBe( "---\nname: unrelated\n---\n", diff --git a/tests/boundary/filesystem/clientLifecycle.test.ts b/tests/boundary/filesystem/clientLifecycle.test.ts index 63ca0667a..24f655507 100644 --- a/tests/boundary/filesystem/clientLifecycle.test.ts +++ b/tests/boundary/filesystem/clientLifecycle.test.ts @@ -1895,14 +1895,11 @@ describe("client configuration filesystem failure reporting", () => { remove: () => Promise.reject(new Error("SECRET removal failure")), }), ); - expect(result.items).toContainEqual( - expect.objectContaining({ - name: "skill", - status: "failed", - detail: expect.stringContaining( - `Managed file could not be removed: ${join(skillRoot, "SKILL.md")}`, - ), - }), + const skillItem = result.items.find(({ name }) => name === "skill"); + expect(skillItem?.status).toBe("failed"); + expect(skillItem?.detail).toContain(`shared (${skillRoot}): failed (`); + expect(skillItem?.detail).toContain( + `Managed file could not be removed: ${join(skillRoot, "SKILL.md")}`, ); expect(JSON.stringify(result)).not.toContain("SECRET"); expect(await readFile(join(skillRoot, "SKILL.md"), "utf8")).toBe( @@ -1944,6 +1941,42 @@ describe("client configuration filesystem failure reporting", () => { }); }); +describe("skill root client identifier labels", () => { + it("labels each retained or removed skill root by its client identifier", async () => { + const home = await createTestTempDirectory("rea-uninstall-skill-labels-"); + roots.push(home); + const customClaudeDirectory = join(home, "claude-config"); + const sharedRoot = join(home, ".agents/skills/reverse-engineer-anything"); + const claudeRoot = join( + customClaudeDirectory, + "skills/reverse-engineer-anything", + ); + await mkdir(sharedRoot, { recursive: true }); + await mkdir(claudeRoot, { recursive: true }); + await writeFile( + join(sharedRoot, "SKILL.md"), + "---\nname: another-skill\n---\n# Not REA-owned\n", + ); + await writeFile(join(claudeRoot, "SKILL.md"), ownedSkillManifest); + + const skillItem = await systemUninstallHost(home, testFileSystem, { + CLAUDE_CONFIG_DIR: customClaudeDirectory, + }).removeSkill(); + + expect(skillItem.name).toBe("skill"); + expect(skillItem.detail).toContain(`shared (${sharedRoot}): retained (`); + expect(skillItem.detail).toContain( + `claude_code (${claudeRoot}): removed (`, + ); + await expect( + readFile(join(claudeRoot, "SKILL.md"), "utf8"), + ).rejects.toMatchObject({ code: "ENOENT" }); + expect(await readFile(join(sharedRoot, "SKILL.md"), "utf8")).toContain( + "another-skill", + ); + }); +}); + const testFileSystem: UninstallFileSystem = { readText: (path) => readFile(path, "utf8"), copy: (source, destination) =>