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 <noreply@anthropic.com> * test(cli): align skill summary assertions with client and path labels --------- Co-authored-by: wjl8636 <wjl8636@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: morluto <76467478+morluto@users.noreply.github.com>
This commit is contained in:
parent
d005bd5dcd
commit
ed93abed02
|
|
@ -627,11 +627,16 @@ const removeManagedSkills = async (
|
|||
platform: NodeJS.Platform,
|
||||
writeBlocker?: (path: string) => string | undefined,
|
||||
): Promise<UninstallItem> => {
|
||||
// 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");
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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) =>
|
||||
|
|
|
|||
Loading…
Reference in New Issue