From d4c5ab94dfe3c810cb724abbc6bf7adb125c7f22 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:27:04 +0900 Subject: [PATCH 1/2] fix(service): propagate native uninstall failures --- src/service.ts | 51 ++++++++++++++++++++++++++--------------- tests/uninstall.test.ts | 29 ++++++++++++++++++++++- 2 files changed, 61 insertions(+), 19 deletions(-) diff --git a/src/service.ts b/src/service.ts index 43d4d8ffe..91d1bb28e 100644 --- a/src/service.ts +++ b/src/service.ts @@ -33,7 +33,7 @@ import { type ElevatedSchtasksCreateAndRunExecution, type ElevatedSchtasksCreateAndRunResult, } from "./lib/windows-elevation"; -import { defaultWinswEntry, installWinswService, startWinswService, stopWinswService, statusWinswRaw, uninstallWinswService, winswStatusSummary, winswXmlPath, WINSW_SERVICE_ID, WINSW_SHA256, WINSW_VERSION } from "./lib/winsw"; +import { defaultWinswEntry, installWinswService, startWinswService, stopWinswService, statusWinswRaw, uninstallWinswService, winswStatusSummary, winswXmlPath, WINSW_SERVICE_ID, WINSW_SHA256, WINSW_VERSION, type WinswStatus } from "./lib/winsw"; import { hardenSecretDir, hardenSecretPath } from "./lib/windows-secret-acl"; import { windowsEnvIndirectBatchPathList, windowsEnvIndirectBatchValue } from "./lib/win-paths"; import { recordOwnedConfigPath } from "./lib/config-ownership"; @@ -2452,33 +2452,48 @@ function removeServiceInstallState(): void { } } +type UninstallServiceHooksForTests = { + platform: typeof process.platform; + assertEnvironment: () => void; + queryWindowsTask: () => string; + uninstallWindowsTask: () => void; + nativeStatus: () => WinswStatus; + uninstallNative: () => void; + removeInstallState: () => void; +}; + +let uninstallServiceHooksForTests: UninstallServiceHooksForTests | null = null; + +/** Test-only hooks for full-uninstall service removal. */ +export function setUninstallServiceHooksForTests(hooks: UninstallServiceHooksForTests | null): void { + uninstallServiceHooksForTests = hooks; +} + /** * Best-effort service removal for full uninstall. Unlike `ocx service uninstall`, this is quiet - * when no service exists and never exits the process just because the platform has no service - * manager. + * when no service exists or the platform has no service manager. An installed native Windows + * service that cannot be removed throws so the caller cannot erase state and report success. */ export function uninstallServiceIfInstalled(): boolean { - assertServiceEnvironmentMatchesInstall(); - if (process.platform === "darwin") { + const hooks = uninstallServiceHooksForTests; + (hooks?.assertEnvironment ?? assertServiceEnvironmentMatchesInstall)(); + const platform = hooks?.platform ?? process.platform; + if (platform === "darwin") { if (existsSync(plistPath())) { try { uninstallLaunchd(); removeServiceInstallState(); return true; } catch { return false; } } - } else if (process.platform === "win32") { + } else if (platform === "win32") { let removed = false; try { - const q = schtasks(["/query", "/tn", TASK]); - if (q.includes(TASK)) { uninstallWindows(); removed = true; } + const q = (hooks?.queryWindowsTask ?? (() => schtasks(["/query", "/tn", TASK])))(); + if (q.includes(TASK)) { (hooks?.uninstallWindowsTask ?? uninstallWindows)(); removed = true; } } catch { /* task not found */ } - if (statusWinswRaw() !== "nonexistent") { - try { - uninstallWinswService(); - removed = true; - } catch (err) { - console.warn(`⚠️ Failed to remove native service: ${err instanceof Error ? err.message : String(err)}. Check 'sc.exe query ${WINSW_SERVICE_ID}'.`); - } + if ((hooks?.nativeStatus ?? statusWinswRaw)() !== "nonexistent") { + (hooks?.uninstallNative ?? uninstallWinswService)(); + removed = true; } - if (removed) { removeServiceInstallState(); return true; } - } else if (process.platform === "linux" && existsSync(unitPath())) { + if (removed) { (hooks?.removeInstallState ?? removeServiceInstallState)(); return true; } + } else if (platform === "linux" && existsSync(unitPath())) { try { uninstallSystemd(); removeServiceInstallState(); return true; } catch { try { unlinkSync(unitPath()); removeServiceInstallState(); return true; } catch { return false; } } @@ -2873,4 +2888,4 @@ export async function serviceCommand(...args: (string | undefined)[]): Promise { } describe("full uninstall command", () => { + afterEach(() => setUninstallServiceHooksForTests(null)); + test("CLI exposes a one-shot local state cleanup command", async () => { const cli = await readText("src/cli/index.ts"); @@ -41,6 +47,27 @@ describe("full uninstall command", () => { expect(service).toContain("uninstallSystemd"); }); + test("native service removal failure propagates without deleting install state", () => { + const calls: string[] = []; + let stateRemovals = 0; + setUninstallServiceHooksForTests({ + platform: "win32", + assertEnvironment: () => {}, + queryWindowsTask: () => "opencodex-proxy", + uninstallWindowsTask: () => { calls.push("scheduler"); }, + nativeStatus: () => "started", + uninstallNative: () => { + calls.push("native"); + throw new Error("native removal failed"); + }, + removeInstallState: () => { stateRemovals++; }, + }); + + expect(() => uninstallServiceIfInstalled()).toThrow("native removal failed"); + expect(calls).toEqual(["scheduler", "native"]); + expect(stateRemovals).toBe(0); + }); + test("full uninstall kills the tracked proxy before deleting service assets", async () => { const cli = await readText("src/cli/index.ts"); const uninstallBody = cli.slice(cli.indexOf("async function handleUninstall()"), cli.indexOf("type HealthCheck")); From c84badac70a83d2d99103cb60e6f06eb94eb3ae7 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:43:58 +0900 Subject: [PATCH 2/2] fix(service): fail closed on scheduler cleanup --- src/service.ts | 17 +++++++++++------ tests/uninstall.test.ts | 18 +++++++++++++++++- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/service.ts b/src/service.ts index 91d1bb28e..856fa4558 100644 --- a/src/service.ts +++ b/src/service.ts @@ -2455,7 +2455,7 @@ function removeServiceInstallState(): void { type UninstallServiceHooksForTests = { platform: typeof process.platform; assertEnvironment: () => void; - queryWindowsTask: () => string; + probeWindowsTask: () => WindowsSchedulerTaskProbe; uninstallWindowsTask: () => void; nativeStatus: () => WinswStatus; uninstallNative: () => void; @@ -2472,7 +2472,8 @@ export function setUninstallServiceHooksForTests(hooks: UninstallServiceHooksFor /** * Best-effort service removal for full uninstall. Unlike `ocx service uninstall`, this is quiet * when no service exists or the platform has no service manager. An installed native Windows - * service that cannot be removed throws so the caller cannot erase state and report success. + * service or scheduler task that cannot be removed throws so the caller cannot erase state and + * report success. */ export function uninstallServiceIfInstalled(): boolean { const hooks = uninstallServiceHooksForTests; @@ -2484,10 +2485,14 @@ export function uninstallServiceIfInstalled(): boolean { } } else if (platform === "win32") { let removed = false; - try { - const q = (hooks?.queryWindowsTask ?? (() => schtasks(["/query", "/tn", TASK])))(); - if (q.includes(TASK)) { (hooks?.uninstallWindowsTask ?? uninstallWindows)(); removed = true; } - } catch { /* task not found */ } + const scheduler = (hooks?.probeWindowsTask ?? probeWindowsSchedulerTask)(); + if (scheduler.status === "unknown") { + throw new Error(`Could not determine Task Scheduler state: ${scheduler.detail}`); + } + if (scheduler.status === "present") { + (hooks?.uninstallWindowsTask ?? uninstallWindows)(); + removed = true; + } if ((hooks?.nativeStatus ?? statusWinswRaw)() !== "nonexistent") { (hooks?.uninstallNative ?? uninstallWinswService)(); removed = true; diff --git a/tests/uninstall.test.ts b/tests/uninstall.test.ts index c0f033681..8f9b9749b 100644 --- a/tests/uninstall.test.ts +++ b/tests/uninstall.test.ts @@ -53,7 +53,7 @@ describe("full uninstall command", () => { setUninstallServiceHooksForTests({ platform: "win32", assertEnvironment: () => {}, - queryWindowsTask: () => "opencodex-proxy", + probeWindowsTask: () => ({ status: "present" }), uninstallWindowsTask: () => { calls.push("scheduler"); }, nativeStatus: () => "started", uninstallNative: () => { @@ -68,6 +68,22 @@ describe("full uninstall command", () => { expect(stateRemovals).toBe(0); }); + test("scheduler removal failure propagates without deleting install state", () => { + let stateRemovals = 0; + setUninstallServiceHooksForTests({ + platform: "win32", + assertEnvironment: () => {}, + probeWindowsTask: () => ({ status: "present" }), + uninstallWindowsTask: () => { throw new Error("scheduler removal failed"); }, + nativeStatus: () => "nonexistent", + uninstallNative: () => {}, + removeInstallState: () => { stateRemovals++; }, + }); + + expect(() => uninstallServiceIfInstalled()).toThrow("scheduler removal failed"); + expect(stateRemovals).toBe(0); + }); + test("full uninstall kills the tracked proxy before deleting service assets", async () => { const cli = await readText("src/cli/index.ts"); const uninstallBody = cli.slice(cli.indexOf("async function handleUninstall()"), cli.indexOf("type HealthCheck"));