From 5d98d2e6ecd7a53b41e2643dc7689c12118e0e1c Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 9 Aug 2026 04:40:50 -0700 Subject: [PATCH] fix(uninstall): preserve user data when gateway removal fails (#113887) * fix(uninstall): gate cleanup on service teardown * fix(daemon): scope strict task removal checks --- src/commands/cleanup-command.test-support.ts | 24 ++++- src/commands/reset.test.ts | 25 ++++++ src/commands/reset.ts | 15 ++-- src/commands/uninstall.test.ts | 94 ++++++++++++++++++++ src/commands/uninstall.ts | 26 +++--- src/daemon/launchd.test.ts | 22 +++++ src/daemon/launchd.ts | 14 ++- src/daemon/schtasks-install.ts | 32 ++++++- src/daemon/schtasks.install.test.ts | 14 +++ 9 files changed, 242 insertions(+), 24 deletions(-) diff --git a/src/commands/cleanup-command.test-support.ts b/src/commands/cleanup-command.test-support.ts index 2c4ba907b201..73105977dd80 100644 --- a/src/commands/cleanup-command.test-support.ts +++ b/src/commands/cleanup-command.test-support.ts @@ -10,6 +10,14 @@ export const prepareLegacyWorkspaceStateReset = vi.fn(); export const removeLegacyWorkspaceStateForReset = vi.fn(); export const removeStateAndLinkedPaths = vi.fn(); export const removeWorkspaceDirs = vi.fn(); +const gatewayServiceState = vi.hoisted(() => ({ + notLoadedText: "is not installed", + isLoaded: vi.fn(), + stop: vi.fn(), + uninstall: vi.fn(), +})); +export const gatewayService = gatewayServiceState; +const cleanupConfigState = vi.hoisted(() => ({ isNixMode: false })); vi.mock("../agents/workspace-legacy-state.js", () => ({ prepareLegacyWorkspaceStateReset, @@ -17,7 +25,13 @@ vi.mock("../agents/workspace-legacy-state.js", () => ({ })); vi.mock("../config/config.js", () => ({ - isNixMode: false, + get isNixMode() { + return cleanupConfigState.isNixMode; + }, +})); + +vi.mock("../daemon/service.js", () => ({ + resolveGatewayService: () => gatewayService, })); vi.mock("./cleanup-plan.js", () => ({ @@ -51,6 +65,14 @@ export function resetCleanupCommandMocks() { removeLegacyWorkspaceStateForReset.mockResolvedValue({ removedPaths: [], warnings: [] }); removeStateAndLinkedPaths.mockResolvedValue(true); removeWorkspaceDirs.mockResolvedValue(undefined); + gatewayService.isLoaded.mockReset().mockResolvedValue(true); + gatewayService.stop.mockReset().mockResolvedValue(undefined); + gatewayService.uninstall.mockReset().mockResolvedValue(undefined); + cleanupConfigState.isNixMode = false; +} + +export function setCleanupNixMode(value: boolean) { + cleanupConfigState.isNixMode = value; } export function silenceCleanupCommandRuntime(runtime: RuntimeEnv) { diff --git a/src/commands/reset.test.ts b/src/commands/reset.test.ts index a15cdb95aff2..9e747cfd0359 100644 --- a/src/commands/reset.test.ts +++ b/src/commands/reset.test.ts @@ -3,6 +3,7 @@ import { beforeAll, beforeEach, describe, expect, it } from "vitest"; import { cleanupCommandLogMessages, createCleanupCommandRuntime, + gatewayService, removeStateAndLinkedPaths, removeWorkspaceDirs, resetCleanupCommandMocks, @@ -22,6 +23,30 @@ describe("resetCommand", () => { silenceCleanupCommandRuntime(runtime); }); + it.each([ + { + failure: "inspection fails", + arrange: () => gatewayService.isLoaded.mockRejectedValue(new Error("inspection failed")), + }, + { + failure: "stop fails", + arrange: () => gatewayService.stop.mockRejectedValue(new Error("stop failed")), + }, + ])("preserves user data when gateway $failure", async ({ arrange }) => { + arrange(); + + await expect( + resetCommand(runtime, { + scope: "full", + yes: true, + nonInteractive: true, + }), + ).rejects.toMatchObject({ name: "ExitError", code: 1 }); + + expect(removeStateAndLinkedPaths).not.toHaveBeenCalled(); + expect(removeWorkspaceDirs).not.toHaveBeenCalled(); + }); + it("recommends creating a backup before state-destructive reset scopes", async () => { await resetCommand(runtime, { scope: "config+creds+sessions", diff --git a/src/commands/reset.ts b/src/commands/reset.ts index 00297d1c8288..05c79a4e9aff 100644 --- a/src/commands/reset.ts +++ b/src/commands/reset.ts @@ -32,11 +32,11 @@ type ResetOptions = { dryRun?: boolean; }; -async function stopGatewayIfRunning(runtime: RuntimeEnv) { +async function stopGatewayIfRunning(runtime: RuntimeEnv): Promise { if (isNixMode) { // Nix mode owns service lifecycle outside OpenClaw-managed launchd/systemd // installs, so reset should not try to stop a service it did not create. - return; + return true; } const service = resolveGatewayService(); let loaded; @@ -44,15 +44,17 @@ async function stopGatewayIfRunning(runtime: RuntimeEnv) { loaded = await service.isLoaded({ env: process.env }); } catch (err) { runtime.error(`Gateway service check failed: ${String(err)}`); - return; + return false; } if (!loaded) { - return; + return true; } try { await service.stop({ env: process.env, stdout: process.stdout }); + return true; } catch (err) { runtime.error(`Gateway stop failed: ${String(err)}`); + return false; } } @@ -130,8 +132,9 @@ export async function resetCommand(runtime: RuntimeEnv, opts: ResetOptions) { logBackupRecommendation(runtime); if (dryRun) { runtime.log("[dry-run] stop gateway service"); - } else { - await stopGatewayIfRunning(runtime); + } else if (!(await stopGatewayIfRunning(runtime))) { + runtime.exit(1); + return; } } diff --git a/src/commands/uninstall.test.ts b/src/commands/uninstall.test.ts index 1449b7792ff5..2ac5624861a8 100644 --- a/src/commands/uninstall.test.ts +++ b/src/commands/uninstall.test.ts @@ -3,11 +3,13 @@ import { beforeEach, describe, expect, it } from "vitest"; import { cleanupCommandLogMessages, createCleanupCommandRuntime, + gatewayService, prepareLegacyWorkspaceStateReset, removeLegacyWorkspaceStateForReset, removeStateAndLinkedPaths, removeWorkspaceDirs, resetCleanupCommandMocks, + setCleanupNixMode, silenceCleanupCommandRuntime, } from "./cleanup-command.test-support.js"; @@ -21,6 +23,98 @@ describe("uninstallCommand", () => { silenceCleanupCommandRuntime(runtime); }); + it.each([ + { + failure: "inspection fails", + arrange: () => gatewayService.isLoaded.mockRejectedValue(new Error("inspection failed")), + }, + { + failure: "stop fails", + arrange: () => gatewayService.stop.mockRejectedValue(new Error("stop failed")), + }, + { + failure: "service removal fails", + arrange: () => gatewayService.uninstall.mockRejectedValue(new Error("uninstall failed")), + }, + ])("preserves user data when gateway $failure", async ({ arrange }) => { + arrange(); + + await expect( + uninstallCommand(runtime, { + all: true, + yes: true, + nonInteractive: true, + }), + ).rejects.toMatchObject({ name: "ExitError", code: 1 }); + + expect(removeStateAndLinkedPaths).not.toHaveBeenCalled(); + expect(removeWorkspaceDirs).not.toHaveBeenCalled(); + expect(prepareLegacyWorkspaceStateReset).not.toHaveBeenCalled(); + expect(cleanupCommandLogMessages(runtime)).not.toContain( + "CLI still installed. Remove via npm/pnpm if desired.", + ); + }); + + it("preserves user data when Nix owns service lifecycle", async () => { + setCleanupNixMode(true); + + await expect( + uninstallCommand(runtime, { + all: true, + yes: true, + nonInteractive: true, + }), + ).rejects.toMatchObject({ name: "ExitError", code: 1 }); + + expect(gatewayService.isLoaded).not.toHaveBeenCalled(); + expect(gatewayService.stop).not.toHaveBeenCalled(); + expect(gatewayService.uninstall).not.toHaveBeenCalled(); + expect(removeStateAndLinkedPaths).not.toHaveBeenCalled(); + expect(removeWorkspaceDirs).not.toHaveBeenCalled(); + }); + + it("still removes service registration after a failed gateway stop", async () => { + gatewayService.stop.mockRejectedValue(new Error("listener still active")); + + await expect( + uninstallCommand(runtime, { + service: true, + yes: true, + nonInteractive: true, + }), + ).rejects.toMatchObject({ name: "ExitError", code: 1 }); + + expect(gatewayService.uninstall).toHaveBeenCalledOnce(); + }); + + it("removes requested data after successful gateway teardown", async () => { + await uninstallCommand(runtime, { + all: true, + yes: true, + nonInteractive: true, + }); + + expect(gatewayService.stop).toHaveBeenCalledOnce(); + expect(gatewayService.uninstall).toHaveBeenCalledOnce(); + expect(removeStateAndLinkedPaths).toHaveBeenCalledOnce(); + expect(removeWorkspaceDirs).toHaveBeenCalledOnce(); + }); + + it("removes an unloaded service definition before deleting user data", async () => { + gatewayService.isLoaded.mockResolvedValue(false); + + await uninstallCommand(runtime, { + all: true, + yes: true, + nonInteractive: true, + }); + + expect(gatewayService.stop).not.toHaveBeenCalled(); + expect(gatewayService.uninstall).toHaveBeenCalledOnce(); + expect(removeStateAndLinkedPaths).toHaveBeenCalledOnce(); + expect(removeWorkspaceDirs).toHaveBeenCalledOnce(); + }); + it("recommends creating a backup before removing state or workspaces", async () => { await uninstallCommand(runtime, { state: true, diff --git a/src/commands/uninstall.ts b/src/commands/uninstall.ts index 8b624b482418..6ba8f7844079 100644 --- a/src/commands/uninstall.ts +++ b/src/commands/uninstall.ts @@ -78,24 +78,27 @@ async function stopAndUninstallService(runtime: RuntimeEnv): Promise { } if (!loaded) { runtime.log(`Gateway service ${service.notLoadedText}.`); - return true; } - try { - await service.stop({ env: process.env, stdout: process.stdout }); - } catch (err) { - runtime.error( - `Gateway stop failed: ${formatErrorMessage(err)}. Run ${formatCliCommand("openclaw gateway status --deep")} before retrying uninstall.`, - ); + let stopped = true; + if (loaded) { + try { + await service.stop({ env: process.env, stdout: process.stdout }); + } catch (err) { + stopped = false; + runtime.error( + `Gateway stop failed: ${formatErrorMessage(err)}. Run ${formatCliCommand("openclaw gateway status --deep")} before retrying uninstall.`, + ); + } } try { await service.uninstall({ env: process.env, stdout: process.stdout }); - return true; } catch (err) { runtime.error( `Gateway uninstall failed: ${formatErrorMessage(err)}. Run ${formatCliCommand("openclaw gateway status --deep")} for the service state.`, ); return false; } + return stopped; } async function removeMacApp(runtime: RuntimeEnv, dryRun?: boolean) { @@ -188,8 +191,11 @@ export async function uninstallCommand(runtime: RuntimeEnv, opts: UninstallOptio if (scopes.has("service")) { if (dryRun) { runtime.log("[dry-run] remove gateway service"); - } else { - await stopAndUninstallService(runtime); + } else if (!(await stopAndUninstallService(runtime))) { + // Service removal may prevent relaunch even when runtime termination is + // uncertain; preserve mutable user data until teardown can be verified. + runtime.exit(1); + return; } } diff --git a/src/daemon/launchd.test.ts b/src/daemon/launchd.test.ts index 77ae33be5e96..baee09bac4a5 100644 --- a/src/daemon/launchd.test.ts +++ b/src/daemon/launchd.test.ts @@ -15,6 +15,7 @@ import { disableCurrentOpenClawUpdateLaunchdJob, disableOpenClawUpdateLaunchdJob, findStaleOpenClawUpdateLaunchdJobs, + isLaunchAgentLoaded, parkCurrentLaunchAgentForMaintenance, parseLaunchctlPrint, parseLaunchctlListOpenClawUpdateJobs, @@ -1445,6 +1446,15 @@ describe("launchd bootstrap repair", () => { }); describe("launchd uninstall", () => { + it("rejects an unrecognized launchctl inspection failure", async () => { + state.printError = "launchctl print permission denied"; + state.printFailuresRemaining = 1; + + await expect(isLaunchAgentLoaded({ env: createDefaultLaunchdEnv() })).rejects.toThrow( + "launchctl print failed: launchctl print permission denied", + ); + }); + it("refuses an in-band uninstall before bootout or plist removal", async () => { const env = createDefaultLaunchdEnv(); const plistPath = resolveLaunchAgentPlistPath(env); @@ -1478,6 +1488,18 @@ describe("launchd uninstall", () => { expect(state.files.has(plistPath)).toBe(true); }); + it("preserves the plist when launchctl cannot boot out the service", async () => { + const env = createDefaultLaunchdEnv(); + const plistPath = resolveLaunchAgentPlistPath(env); + state.files.set(plistPath, "RunAtLoad=true"); + state.bootoutError = "launchctl bootout permission denied"; + + await expect(uninstallLaunchAgent({ env, stdout: new PassThrough() })).rejects.toThrow( + "launchctl bootout failed: launchctl bootout permission denied", + ); + expect(state.files.has(plistPath)).toBe(true); + }); + it("reports inaccessible LaunchAgents instead of claiming they are missing", async () => { const env = createDefaultLaunchdEnv(); const plistPath = resolveLaunchAgentPlistPath(env); diff --git a/src/daemon/launchd.ts b/src/daemon/launchd.ts index e12a4277bdee..df2cc40f3d8c 100644 --- a/src/daemon/launchd.ts +++ b/src/daemon/launchd.ts @@ -820,7 +820,13 @@ export async function isLaunchAgentLoaded(args: GatewayServiceEnvArgs): Promise< const domain = resolveGuiDomain(); const label = resolveLaunchAgentLabel(args.env); const res = await execLaunchctl(["print", `${domain}/${label}`]); - return res.code === 0; + if (res.code === 0) { + return true; + } + if (isLaunchctlNotLoaded(res)) { + return false; + } + throw new Error(`launchctl print failed: ${formatLaunchctlResultDetail(res)}`); } export async function launchAgentPlistExists(env: GatewayServiceEnv): Promise { @@ -974,8 +980,10 @@ export async function uninstallLaunchAgent({ const domain = resolveGuiDomain(); const label = resolveLaunchAgentLabel(env); const plistPath = resolveLaunchAgentPlistPath(env); - await execLaunchctl(["bootout", domain, plistPath]); - await execLaunchctl(["unload", plistPath]); + const bootout = await execLaunchctl(["bootout", domain, plistPath]); + if (bootout.code !== 0 && !isLaunchctlNotLoaded(bootout)) { + throw new Error(`launchctl bootout failed: ${formatLaunchctlResultDetail(bootout)}`); + } try { await fs.lstat(plistPath); diff --git a/src/daemon/schtasks-install.ts b/src/daemon/schtasks-install.ts index f4efb85405e3..e6dbfe4ef16a 100644 --- a/src/daemon/schtasks-install.ts +++ b/src/daemon/schtasks-install.ts @@ -1,5 +1,6 @@ import fs from "node:fs/promises"; import path from "node:path"; +import { normalizeLowercaseStringOrEmpty } from "@openclaw/normalization-core/string-coerce"; import { uniqueStrings } from "@openclaw/normalization-core/string-normalization"; import { resolveGatewayServiceDescription } from "./constants.js"; import { formatLine, writeFormattedLines } from "./output.js"; @@ -37,6 +38,7 @@ import { isStartupEntryInstalled, launchFallbackTaskScript, removeStartupEntries, + probeScheduledTaskExists, resolveFallbackRuntime, waitForFallbackTakeoverRuntime, waitForScheduledTaskRunningEvidence, @@ -388,8 +390,23 @@ export async function uninstallScheduledTask({ }: GatewayServiceManageArgs): Promise { await assertSchtasksAvailable(); const taskName = resolveTaskName(env); - if (await isRegisteredScheduledTask(env).catch(() => false)) { - await execSchtasks(["/Delete", "/F", "/TN", taskName]); + const query = await execSchtasks(["/Query", "/TN", taskName]); + const queryDetail = normalizeLowercaseStringOrEmpty(query.stderr || query.stdout); + const exists = + query.code === 0 + ? true + : queryDetail.includes("cannot find the file") + ? false + : probeScheduledTaskExists(taskName); + if (exists === null) { + throw new Error(`Could not verify whether Scheduled Task ${taskName} exists.`); + } + if (exists) { + const deletion = await execSchtasks(["/Delete", "/F", "/TN", taskName]); + if (deletion.code !== 0) { + const detail = (deletion.stderr || deletion.stdout).trim() || "unknown error"; + throw new Error(`schtasks delete failed: ${detail}`); + } } await removeStartupEntries(env, stdout); @@ -406,12 +423,19 @@ export async function uninstallScheduledTask({ try { await fs.unlink(launcherPath); stdout.write(`${formatLine("Removed task launcher", launcherPath)}\n`); - } catch {} + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") { + throw error; + } + } } try { await fs.unlink(scriptPath); stdout.write(`${formatLine("Removed task script", scriptPath)}\n`); - } catch { + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") { + throw error; + } stdout.write(`Task script not found at ${scriptPath}\n`); } } diff --git a/src/daemon/schtasks.install.test.ts b/src/daemon/schtasks.install.test.ts index ac401b21b0dc..cd76151860fb 100644 --- a/src/daemon/schtasks.install.test.ts +++ b/src/daemon/schtasks.install.test.ts @@ -418,6 +418,20 @@ describe("installScheduledTask", () => { }); }); + it("preserves task scripts when Scheduled Task deletion fails", async () => { + await withUserProfileDir(async (_tmpDir, env) => { + schtasksResponses.push(okSchtasksResponse, okSchtasksResponse, accessDeniedResponse); + const scriptPath = resolveTaskScriptPath(env); + await fs.mkdir(path.dirname(scriptPath), { recursive: true }); + await fs.writeFile(scriptPath, "@echo off\n", "utf8"); + + await expect(uninstallScheduledTask({ env, stdout: new PassThrough() })).rejects.toThrow( + "schtasks delete failed: ERROR: Access is denied.", + ); + await expect(fs.access(scriptPath)).resolves.toBeUndefined(); + }); + }); + it("creates the Scheduled Task via XML with battery start/continue enabled (#59299)", async () => { await withUserProfileDir(async (_tmpDir, env) => { schtasksResponses.push(okSchtasksResponse, missingTaskResponse);