From e2bdfcee75cd1c487ed4c3d2a2c5552cb226364d Mon Sep 17 00:00:00 2001
From: morluto <76467478+morluto@users.noreply.github.com>
Date: Wed, 11 Nov 2026 12:58:52 +0800
Subject: [PATCH] fix(uninstall): stop purging after skill removal failure
 (#9947)

---
 src/application/Uninstall.test.ts | 54 +++++++++++++++++++++++++++++++
 src/application/Uninstall.ts      | 22 +++++++------
 2 files changed, 66 insertions(+), 10 deletions(-)
 create mode 100644 src/application/Uninstall.test.ts

diff --git a/src/application/Uninstall.test.ts b/src/application/Uninstall.test.ts
new file mode 100644
index 0000000000..c04f88aaf4
--- /dev/null
+++ b/src/application/Uninstall.test.ts
@@ -0,0 +1,54 @@
+import { expect, it, vi } from "vitest";
+import { runUninstall, type UninstallHost } from "./Uninstall.js";
+
+it.each(["failed", "removed", "retained", "skipped"] as const)(
+  "honors managed-skill removal status before purging data: %s",
+  async (status) => {
+    const purgeData = vi.fn<UninstallHost["purgeData"]>(async () => [
+      { name: "cache", status: "removed", detail: "Removed fixture cache." },
+    ]);
+    const close = vi.fn<NonNullable<UninstallHost["close"]>>(
+      async () => undefined,
+    );
+    const host: UninstallHost = {
+      clients: async () => [],
+      inspectClient: async () => undefined,
+      removeClient: async () => ({
+        name: "client",
+        status: "skipped",
+        detail: "Absent.",
+      }),
+      removeSkill: async () => ({
+        name: "skill",
+        status,
+        detail: "Fixture skill outcome.",
+      }),
+      purgeData,
+      close,
+    };
+
+    const result = await runUninstall(true, host);
+
+    expect(close).toHaveBeenCalledOnce();
+    expect(result.status).toBe(status === "failed" ? "failed" : "complete");
+    expect(result.items).toContainEqual({
+      name: "skill",
+      status,
+      detail: "Fixture skill outcome.",
+    });
+    if (status === "failed") {
+      expect(purgeData).not.toHaveBeenCalled();
+      expect(result.items).toContainEqual(
+        expect.objectContaining({ name: "uninstall", status: "skipped" }),
+      );
+      expect(result.items.some(({ name }) => name === "cache")).toBe(false);
+    } else {
+      expect(purgeData).toHaveBeenCalledOnce();
+      expect(result.items).toContainEqual({
+        name: "cache",
+        status: "removed",
+        detail: "Removed fixture cache.",
+      });
+    }
+  },
+);
diff --git a/src/application/Uninstall.ts b/src/application/Uninstall.ts
index 4e7a966712..4bf31d2377 100644
--- a/src/application/Uninstall.ts
+++ b/src/application/Uninstall.ts
@@ -241,20 +241,22 @@ const removeAll = async (
   host: UninstallHost,
 ): Promise<UninstallItem[]> => {
   const items: UninstallItem[] = [];
+  const stopped = (): UninstallItem[] => [
+    ...items,
+    item(
+      "uninstall",
+      "skipped",
+      "Uninstall stopped at this failure; the items above report what changed. Repair the failed removal, then rerun uninstall.",
+    ),
+  ];
   for (const client of clients) {
     const removed = await host.removeClient(client);
     items.push({ ...removed, configuration_path: client.configPath });
-    if (removed.status === "failed")
-      return [
-        ...items,
-        item(
-          "uninstall",
-          "skipped",
-          "Uninstall stopped at this failure; the items above report what changed. Repair the failed configuration, then rerun uninstall.",
-        ),
-      ];
+    if (removed.status === "failed") return stopped();
   }
-  items.push(await host.removeSkill());
+  const skill = await host.removeSkill();
+  items.push(skill);
+  if (skill.status === "failed") return stopped();
   if (purgeData) items.push(...(await host.purgeData()));
   return items;
 };