From f572e5a5cc53ad60142054a62fc41fe64c68c9bf Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Thu, 27 Aug 2026 11:54:28 -0700 Subject: [PATCH 1/2] fix(uninstall): edit the Windows user PATH via the registry and broadcast best-effort [Environment]::SetEnvironmentVariable('Path', ..., 'User') writes HKCU\Environment and then broadcasts WM_SETTINGCHANGE synchronously to every top-level window. On a machine with a hung GUI process that broadcast can outlast the uninstaller's 15 s command timeout, so PowerShell was killed (exit 124) and the uninstaller refused to remove the CLI even though the PATH edit had completed. The remove and restore scripts now read the user Path unexpanded from the registry, preserve its value kind, write it back through Microsoft.Win32.Registry, and only then broadcast WM_SETTINGCHANGE via SendMessageTimeout with SMTO_ABORTIFHUNG and a 1 s per-window timeout inside try/catch, exiting 0 explicitly so the broadcast can never affect the exit code. The exit-3 not-present contract and the JSON rollback echo are unchanged. A genuine registry failure still pushes an error and preserves the CLI; a timeout is now reported as such. Because the rollback echo is written only after a successful write, a timeout that arrives after the echo is treated as a completed edit with a warning instead of a failure. --- packages/server/uninstall.test.ts | 174 +++++++++++++++++++++++++++++- packages/server/uninstall.ts | 101 ++++++++++++++--- 2 files changed, 256 insertions(+), 19 deletions(-) diff --git a/packages/server/uninstall.test.ts b/packages/server/uninstall.test.ts index 696da6545..4b70e1cab 100644 --- a/packages/server/uninstall.test.ts +++ b/packages/server/uninstall.test.ts @@ -21,6 +21,8 @@ import { formatPurgeWarning, runPlannotatorUninstall, type UninstallEnvironment, + WINDOWS_PATH_RESTORE_SCRIPT, + WINDOWS_PATH_SCRIPT, WINDOWS_SELF_DELETE_SCRIPT, } from "./uninstall"; @@ -78,6 +80,85 @@ describe("Windows self-delete worker", () => { }); }); +describe("Windows PATH scripts", () => { + const scripts = { + remove: WINDOWS_PATH_SCRIPT, + restore: WINDOWS_PATH_RESTORE_SCRIPT, + } as const; + + test("edit the registry directly and never call the blocking .NET setter", () => { + // SetEnvironmentVariable('Path', ..., 'User') broadcasts WM_SETTINGCHANGE + // synchronously to every window and can stall past the 15 s command + // timeout on a machine with a hung GUI process (the CI smoke flake). + for (const script of Object.values(scripts)) { + expect(script).not.toContain("SetEnvironmentVariable"); + expect(script).toContain("Microsoft.Win32.Registry"); + // The broadcast that replaces it must be bounded (SMTO_ABORTIFHUNG) and + // must not be able to reach the exit code. + expect(script).toContain("SendMessageTimeout"); + expect(script).toMatch(/'Environment',0x2,\d+,\[ref\]\$r\)\}catch\{\}; exit 0$/); + } + // The value is read unexpanded and written back with its own kind so + // %VARS% in unrelated entries survive. + expect(scripts.remove).toContain("DoNotExpandEnvironmentNames"); + expect(scripts.remove).toContain("$k.SetValue('Path',$n,$kind)"); + // The rollback echo must come after the write and before the broadcast. + const writeIndex = scripts.remove.indexOf("$k.SetValue("); + const echoIndex = scripts.remove.indexOf("Write-Output (ConvertTo-Json"); + const broadcastIndex = scripts.remove.indexOf("SendMessageTimeout("); + expect(writeIndex).toBeGreaterThan(-1); + expect(echoIndex).toBeGreaterThan(writeIndex); + expect(broadcastIndex).toBeGreaterThan(echoIndex); + }); + + test("stay single-quoted so they survive -Command argv quoting", () => { + // Both scripts travel as one argv element to powershell.exe; a literal + // double quote would be re-escaped by the spawn layer and break parsing. + for (const script of Object.values(scripts)) { + expect(script).not.toContain('"'); + } + }); + + const powershell = + Bun.which("pwsh") || + Bun.which("pwsh.exe") || + Bun.which("powershell.exe") || + process.env.PLANNOTATOR_TEST_POWERSHELL || + null; + + test.skipIf(!powershell)( + "parse cleanly in a real PowerShell (no registry access)", + async () => { + // Parser.ParseInput only parses; nothing is executed, so this touches + // neither the registry nor the environment. Runs on Windows CI and on + // any dev box with pwsh. + for (const [name, script] of Object.entries(scripts)) { + const proc = Bun.spawn( + [ + powershell!, + "-NoProfile", + "-NonInteractive", + "-Command", + "$errors=$null; [void][System.Management.Automation.Language.Parser]::ParseInput($env:PLANNOTATOR_TEST_SCRIPT,[ref]$null,[ref]$errors); if($errors.Count -gt 0){$errors | ForEach-Object { Write-Output $_.Message }; exit 1}; exit 0", + ], + { + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + env: { ...process.env, PLANNOTATOR_TEST_SCRIPT: script }, + }, + ); + const [stdout, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + proc.exited, + ]); + expect(`${name}: ${stdout.trim()}`).toBe(`${name}: `); + expect(exitCode).toBe(0); + } + }, + ); +}); + function createFixture( overrides: Partial = {}, ): Fixture { @@ -1541,12 +1622,101 @@ describe("host and platform integrations", () => { expect(result.ok).toBe(false); expect(result.errors).toContain( - `Could not remove ${dirname(currentExe)} from the Windows user PATH.`, + `Could not remove ${dirname(currentExe)} from the Windows user PATH (exit 1).`, + ); + expect(existsSync(currentExe)).toBe(true); + expect(fixture.scheduledDeletes).toEqual([]); + }); + + test("proceeds without a PATH error when the entry is not present (exit 3)", async () => { + const fixture = createFixture(); + const localAppData = join(fixture.homeDir, "AppData", "Local"); + const currentExe = join(localAppData, "plannotator", "plannotator.exe"); + writeText(currentExe); + + const result = await runPlannotatorUninstall( + { purge: false, dryRun: false }, + { + ...fixture.environment, + platform: "win32", + execPath: currentExe, + env: { LOCALAPPDATA: localAppData }, + which: () => "C:\\Windows\\powershell.exe", + runCommand: async () => ({ exitCode: 3, timedOut: false }), + }, + ); + + expect(result.ok).toBe(true); + expect(result.errors).toEqual([]); + expect(result.removed).not.toContain( + `Windows user PATH entry ${dirname(currentExe)}`, + ); + expect(fixture.scheduledDeletes).toEqual([ + { target: currentExe, parent: dirname(currentExe) }, + ]); + }); + + test("reports a timed-out PATH edit as a timeout and keeps the CLI", async () => { + const fixture = createFixture(); + const localAppData = join(fixture.homeDir, "AppData", "Local"); + const currentExe = join(localAppData, "plannotator", "plannotator.exe"); + writeText(currentExe); + + const result = await runPlannotatorUninstall( + { purge: false, dryRun: false }, + { + ...fixture.environment, + platform: "win32", + execPath: currentExe, + env: { LOCALAPPDATA: localAppData }, + which: () => "C:\\Windows\\powershell.exe", + // Killed before the script echoed anything: the edit is unproven. + runCommand: async () => ({ exitCode: 124, timedOut: true, stdout: "" }), + }, + ); + + expect(result.ok).toBe(false); + expect(result.errors).toContain( + `Could not remove ${dirname(currentExe)} from the Windows user PATH (command timed out).`, ); expect(existsSync(currentExe)).toBe(true); expect(fixture.scheduledDeletes).toEqual([]); }); + test("treats a timeout after the rollback echo as a completed PATH edit", async () => { + const fixture = createFixture(); + const localAppData = join(fixture.homeDir, "AppData", "Local"); + const currentExe = join(localAppData, "plannotator", "plannotator.exe"); + writeText(currentExe); + const originalPath = `C:\\Before;${dirname(currentExe)};C:\\After;;`; + + const result = await runPlannotatorUninstall( + { purge: false, dryRun: false }, + { + ...fixture.environment, + platform: "win32", + execPath: currentExe, + env: { LOCALAPPDATA: localAppData }, + which: () => "C:\\Windows\\powershell.exe", + // The script echoes the original PATH only after the registry write, + // so an echo followed by a kill means only the broadcast stalled. + runCommand: async () => ({ + exitCode: 124, + timedOut: true, + stdout: `${JSON.stringify(originalPath)}\n`, + }), + }, + ); + + const pathLabel = `Windows user PATH entry ${dirname(currentExe)}`; + expect(result.ok).toBe(true); + expect(result.removed).toContain(pathLabel); + expect(result.warnings.some((w) => w.includes("timed out"))).toBe(true); + expect(fixture.scheduledDeletes).toEqual([ + { target: currentExe, parent: dirname(currentExe) }, + ]); + }); + test("restores Windows PATH when scheduling self-delete fails", async () => { const fixture = createFixture(); const localAppData = join(fixture.homeDir, "AppData", "Local"); @@ -1615,7 +1785,7 @@ describe("host and platform integrations", () => { expect(existsSync(currentExe)).toBe(true); expect(result.removed).toContain(pathLabel); expect(result.errors).toContain( - `Could not restore ${dirname(currentExe)} to the Windows user PATH after self-delete scheduling failed.`, + `Could not restore ${dirname(currentExe)} to the Windows user PATH after self-delete scheduling failed (exit 1).`, ); expect(result.warnings).toContain( `The Plannotator CLI remains at ${currentExe}, but its Windows PATH entry could not be restored. Run that full path to retry, then restore PATH manually if needed.`, diff --git a/packages/server/uninstall.ts b/packages/server/uninstall.ts index 8a8f6af16..f605c18bf 100644 --- a/packages/server/uninstall.ts +++ b/packages/server/uninstall.ts @@ -108,22 +108,76 @@ const PURGE_OWNED_TOP_LEVEL = [ "guide-schema.json", ] as const; -const WINDOWS_PATH_SCRIPT = [ +/** + * Best-effort WM_SETTINGCHANGE broadcast that runs AFTER the registry write. + * + * `[Environment]::SetEnvironmentVariable(..., 'User')` performs this broadcast + * itself, synchronously, per top-level window and without SMTO_ABORTIFHUNG, so + * one orphaned or hung GUI process can stall it past the uninstaller's 15 s + * command timeout and turn a completed PATH edit into a killed PowerShell and + * exit 124. Here the broadcast is decoupled from the edit: SMTO_ABORTIFHUNG + * (0x2) skips windows Windows already considers hung, the per-window timeout + * is short, the whole thing is wrapped in try/catch, and the script exits 0 + * explicitly so nothing about the broadcast can reach the exit code. + * + * The DllImport attribute needs double quotes, which the `-Command` one-liners + * avoid on purpose (the script travels as a single argv element on Windows), + * so they are assembled from `[char]34` at runtime. + */ +const WINDOWS_PATH_BROADCAST_STATEMENTS = [ + "$q=[char]34", + "$sig='[DllImport('+$q+'user32.dll'+$q+',CharSet=CharSet.Unicode)] public static extern IntPtr SendMessageTimeout(IntPtr hWnd,uint msg,UIntPtr wParam,string lParam,uint flags,uint timeout,out UIntPtr result);'", + "try{Add-Type -Namespace Plannotator -Name PathBroadcast -MemberDefinition $sig; $r=[UIntPtr]::Zero; [void][Plannotator.PathBroadcast]::SendMessageTimeout([IntPtr]0xffff,0x1A,[UIntPtr]::Zero,'Environment',0x2,1000,[ref]$r)}catch{}", + "exit 0", +] as const; + +/** + * Removes exactly one entry from the HKCU user PATH through the registry API. + * + * The value is read unexpanded (`DoNotExpandEnvironmentNames`) and written back + * with its original kind (REG_EXPAND_SZ on most systems), so `%VARS%` in other + * entries survive byte for byte. Exit 3 means "not present or unchanged"; on + * success the ORIGINAL value is echoed as one JSON string on stdout so the + * caller can roll back. The echo is written before the broadcast, so stdout + * carrying that JSON proves the registry write completed. + * + * @internal Exported only so the PowerShell syntax can be regression-tested. + */ +export const WINDOWS_PATH_SCRIPT = [ "$ErrorActionPreference='Stop'", - "$p=[Environment]::GetEnvironmentVariable('Path','User')", + "$k=[Microsoft.Win32.Registry]::CurrentUser.OpenSubKey('Environment',$true)", + "if($null -eq $k){exit 3}", + "$p=$k.GetValue('Path',$null,[Microsoft.Win32.RegistryValueOptions]::DoNotExpandEnvironmentNames)", "if($null -eq $p){exit 3}", + "$p=[string]$p", + "$kind=$k.GetValueKind('Path')", "$t=$env:PLANNOTATOR_UNINSTALL_PATH.Trim().TrimEnd('\\')", "$kept=@($p -split ';' | Where-Object { $_.Trim().TrimEnd('\\') -ine $t })", "$n=$kept -join ';'", "if($n -eq $p){exit 3}", - "[Environment]::SetEnvironmentVariable('Path',$n,'User')", + "$k.SetValue('Path',$n,$kind)", + "$k.Close()", "Write-Output (ConvertTo-Json -Compress -InputObject $p)", + ...WINDOWS_PATH_BROADCAST_STATEMENTS, ].join("; "); -const WINDOWS_PATH_RESTORE_SCRIPT = [ +/** + * Writes the echoed original PATH back with the kind the value currently has + * (the kind the removal preserved), falling back to REG_EXPAND_SZ when the + * value is gone entirely. Same decoupled broadcast as the removal. + * + * @internal Exported only so the PowerShell syntax can be regression-tested. + */ +export const WINDOWS_PATH_RESTORE_SCRIPT = [ "$ErrorActionPreference='Stop'", "$original=$env:PLANNOTATOR_UNINSTALL_ORIGINAL_PATH", - "[Environment]::SetEnvironmentVariable('Path',$original,'User')", + "$k=[Microsoft.Win32.Registry]::CurrentUser.OpenSubKey('Environment',$true)", + "if($null -eq $k){$k=[Microsoft.Win32.Registry]::CurrentUser.CreateSubKey('Environment')}", + "$kind=[Microsoft.Win32.RegistryValueKind]::ExpandString", + "try{$kind=$k.GetValueKind('Path')}catch{}", + "$k.SetValue('Path',$original,$kind)", + "$k.Close()", + ...WINDOWS_PATH_BROADCAST_STATEMENTS, ].join("; "); /** @internal Exported only so the Windows worker syntax can be regression-tested. */ @@ -888,28 +942,41 @@ async function removeWindowsPathEntry( ["-NoProfile", "-NonInteractive", "-Command", WINDOWS_PATH_SCRIPT], { PLANNOTATOR_UNINSTALL_PATH: paths.windowsInstallDir }, ); - if (result.exitCode === 0) { + // The script echoes the original PATH only after the registry write + // succeeded, so a parseable echo proves the edit happened even when the + // process was later killed while broadcasting the change. + const echoedOriginalPath = parseEchoedWindowsPath(result.stdout); + if (result.exitCode === 0 || (result.timedOut && echoedOriginalPath !== null)) { state.removed.push(label); - try { - const originalPath: unknown = JSON.parse(result.stdout?.trim() ?? ""); - if (typeof originalPath !== "string") throw new Error("not a string"); - return originalPath; - } catch { - state.errors.push( - `Removed ${paths.windowsInstallDir} from the Windows user PATH but could not capture the original PATH for safe rollback.`, - ); + if (result.timedOut) { state.warnings.push( - `The Plannotator CLI remains at ${environment.execPath}, but its Windows PATH entry was removed without a usable backup. Run that full path to retry, then restore PATH manually if needed.`, + `Removed ${paths.windowsInstallDir} from the Windows user PATH, but notifying open windows of the change timed out; new terminals pick up the change after you sign in again.`, ); } + if (echoedOriginalPath !== null) return echoedOriginalPath; + state.errors.push( + `Removed ${paths.windowsInstallDir} from the Windows user PATH but could not capture the original PATH for safe rollback.`, + ); + state.warnings.push( + `The Plannotator CLI remains at ${environment.execPath}, but its Windows PATH entry was removed without a usable backup. Run that full path to retry, then restore PATH manually if needed.`, + ); } else if (result.exitCode !== 3) { state.errors.push( - `Could not remove ${paths.windowsInstallDir} from the Windows user PATH.`, + `Could not remove ${paths.windowsInstallDir} from the Windows user PATH (${result.timedOut ? "command timed out" : `exit ${result.exitCode}`}).`, ); } return null; } +function parseEchoedWindowsPath(stdout: string | undefined): string | null { + try { + const originalPath: unknown = JSON.parse(stdout?.trim() ?? ""); + return typeof originalPath === "string" ? originalPath : null; + } catch { + return null; + } +} + async function removeBinaries( request: UninstallRequest, environment: UninstallEnvironment, @@ -1000,7 +1067,7 @@ async function restoreWindowsPathEntry( ); if (result.exitCode !== 0) { state.errors.push( - `Could not restore ${paths.windowsInstallDir} to the Windows user PATH after self-delete scheduling failed.`, + `Could not restore ${paths.windowsInstallDir} to the Windows user PATH after self-delete scheduling failed (${result.timedOut ? "command timed out" : `exit ${result.exitCode}`}).`, ); state.warnings.push( `The Plannotator CLI remains at ${environment.execPath}, but its Windows PATH entry could not be restored. Run that full path to retry, then restore PATH manually if needed.`, From 522df0dfb77485ee3f6f3e53457b4a8e27cc3d8f Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Thu, 27 Aug 2026 12:09:46 -0700 Subject: [PATCH 2/2] fix(uninstall): trust the completed-write echo however the PowerShell process ends The remove script prints the rollback JSON, and the restore script now prints a PLANNOTATOR_PATH_RESTORED sentinel, only after the registry write. A native fault inside Add-Type or SendMessageTimeout cannot be caught by try/catch and ends the process with an NTSTATUS exit code, so both callers now treat a parseable echo as proof of the completed edit whether the process timed out or exited non-zero for another reason, and warn about the broadcast instead of misreporting a finished edit as a failure (which would also lose the rollback value). A non-zero exit with no echo still fails closed. --- packages/server/uninstall.test.ts | 154 ++++++++++++++++++++++++++++-- packages/server/uninstall.ts | 33 +++++-- 2 files changed, 174 insertions(+), 13 deletions(-) diff --git a/packages/server/uninstall.test.ts b/packages/server/uninstall.test.ts index 4b70e1cab..13f144f87 100644 --- a/packages/server/uninstall.test.ts +++ b/packages/server/uninstall.test.ts @@ -102,13 +102,23 @@ describe("Windows PATH scripts", () => { // %VARS% in unrelated entries survive. expect(scripts.remove).toContain("DoNotExpandEnvironmentNames"); expect(scripts.remove).toContain("$k.SetValue('Path',$n,$kind)"); - // The rollback echo must come after the write and before the broadcast. - const writeIndex = scripts.remove.indexOf("$k.SetValue("); - const echoIndex = scripts.remove.indexOf("Write-Output (ConvertTo-Json"); - const broadcastIndex = scripts.remove.indexOf("SendMessageTimeout("); - expect(writeIndex).toBeGreaterThan(-1); - expect(echoIndex).toBeGreaterThan(writeIndex); - expect(broadcastIndex).toBeGreaterThan(echoIndex); + // The completed-write echo must come after the write and before the + // broadcast in BOTH scripts: it is what lets the caller trust a write + // whose process was killed or faulted while broadcasting. + const echoes = { + remove: "Write-Output (ConvertTo-Json", + restore: "Write-Output 'PLANNOTATOR_PATH_RESTORED'", + } as const; + for (const [name, script] of Object.entries(scripts) as Array< + [keyof typeof scripts, string] + >) { + const writeIndex = script.indexOf("$k.SetValue("); + const echoIndex = script.indexOf(echoes[name]); + const broadcastIndex = script.indexOf("SendMessageTimeout("); + expect(`${name}: ${writeIndex}`).not.toBe(`${name}: -1`); + expect(echoIndex).toBeGreaterThan(writeIndex); + expect(broadcastIndex).toBeGreaterThan(echoIndex); + } }); test("stay single-quoted so they survive -Command argv quoting", () => { @@ -1717,6 +1727,136 @@ describe("host and platform integrations", () => { ]); }); + test("treats a non-zero exit after the rollback echo as a completed PATH edit", async () => { + const fixture = createFixture(); + const localAppData = join(fixture.homeDir, "AppData", "Local"); + const currentExe = join(localAppData, "plannotator", "plannotator.exe"); + writeText(currentExe); + const originalPath = `C:\\Before;${dirname(currentExe)};C:\\After;;`; + + const result = await runPlannotatorUninstall( + { purge: false, dryRun: false }, + { + ...fixture.environment, + platform: "win32", + execPath: currentExe, + env: { LOCALAPPDATA: localAppData }, + which: () => "C:\\Windows\\powershell.exe", + // A native fault inside Add-Type / SendMessageTimeout is not + // catchable and ends the process with an NTSTATUS code; the echo + // already on stdout still proves the write completed. + runCommand: async () => ({ + exitCode: -1073741819, + timedOut: false, + stdout: `${JSON.stringify(originalPath)}\n`, + }), + }, + ); + + const pathLabel = `Windows user PATH entry ${dirname(currentExe)}`; + expect(result.ok).toBe(true); + expect(result.errors).toEqual([]); + expect(result.removed).toContain(pathLabel); + expect(result.warnings.some((w) => w.includes("exit -1073741819"))).toBe(true); + expect(fixture.scheduledDeletes).toEqual([ + { target: currentExe, parent: dirname(currentExe) }, + ]); + }); + + test("treats a restore that printed its sentinel as completed however the process ended", async () => { + for (const ending of [ + { exitCode: 124, timedOut: true, needle: "timed out" }, + { exitCode: -1073741819, timedOut: false, needle: "exit -1073741819" }, + ]) { + const fixture = createFixture(); + const localAppData = join(fixture.homeDir, "AppData", "Local"); + const currentExe = join(localAppData, "plannotator", "plannotator.exe"); + writeText(currentExe); + let commandCount = 0; + + const result = await runPlannotatorUninstall( + { purge: false, dryRun: false }, + { + ...fixture.environment, + platform: "win32", + execPath: currentExe, + env: { LOCALAPPDATA: localAppData }, + which: () => "C:\\Windows\\powershell.exe", + runCommand: async () => { + commandCount += 1; + if (commandCount === 1) { + return { + exitCode: 0, + timedOut: false, + stdout: JSON.stringify( + `C:\\Before;${dirname(currentExe)};C:\\After;;`, + ), + }; + } + return { + exitCode: ending.exitCode, + timedOut: ending.timedOut, + stdout: "PLANNOTATOR_PATH_RESTORED\r\n", + }; + }, + scheduleWindowsSelfDelete: async () => false, + }, + ); + + const pathLabel = `Windows user PATH entry ${dirname(currentExe)}`; + expect(result.ok).toBe(false); + expect(existsSync(currentExe)).toBe(true); + expect(result.errors.some((e) => e.includes("Could not restore"))).toBe(false); + expect(result.removed).not.toContain(pathLabel); + expect(result.preserved).toContain(`${pathLabel} (restored for retry)`); + expect( + result.warnings.some( + (w) => w.startsWith("Restored ") && w.includes(ending.needle), + ), + ).toBe(true); + } + }); + + test("reports a restore that never printed its sentinel as failed", async () => { + const fixture = createFixture(); + const localAppData = join(fixture.homeDir, "AppData", "Local"); + const currentExe = join(localAppData, "plannotator", "plannotator.exe"); + writeText(currentExe); + let commandCount = 0; + + const result = await runPlannotatorUninstall( + { purge: false, dryRun: false }, + { + ...fixture.environment, + platform: "win32", + execPath: currentExe, + env: { LOCALAPPDATA: localAppData }, + which: () => "C:\\Windows\\powershell.exe", + runCommand: async () => { + commandCount += 1; + if (commandCount === 1) { + return { + exitCode: 0, + timedOut: false, + stdout: JSON.stringify( + `C:\\Before;${dirname(currentExe)};C:\\After;;`, + ), + }; + } + return { exitCode: 124, timedOut: true, stdout: "" }; + }, + scheduleWindowsSelfDelete: async () => false, + }, + ); + + const pathLabel = `Windows user PATH entry ${dirname(currentExe)}`; + expect(result.ok).toBe(false); + expect(result.removed).toContain(pathLabel); + expect(result.errors).toContain( + `Could not restore ${dirname(currentExe)} to the Windows user PATH after self-delete scheduling failed (command timed out).`, + ); + }); + test("restores Windows PATH when scheduling self-delete fails", async () => { const fixture = createFixture(); const localAppData = join(fixture.homeDir, "AppData", "Local"); diff --git a/packages/server/uninstall.ts b/packages/server/uninstall.ts index f605c18bf..46193c553 100644 --- a/packages/server/uninstall.ts +++ b/packages/server/uninstall.ts @@ -161,6 +161,13 @@ export const WINDOWS_PATH_SCRIPT = [ ...WINDOWS_PATH_BROADCAST_STATEMENTS, ].join("; "); +/** + * Line the restore script prints right after its registry write and before the + * broadcast, so the caller can tell a completed restore from one that never + * reached the write even when the process was killed or died afterwards. + */ +const WINDOWS_PATH_RESTORED_SENTINEL = "PLANNOTATOR_PATH_RESTORED"; + /** * Writes the echoed original PATH back with the kind the value currently has * (the kind the removal preserved), falling back to REG_EXPAND_SZ when the @@ -177,6 +184,7 @@ export const WINDOWS_PATH_RESTORE_SCRIPT = [ "try{$kind=$k.GetValueKind('Path')}catch{}", "$k.SetValue('Path',$original,$kind)", "$k.Close()", + `Write-Output '${WINDOWS_PATH_RESTORED_SENTINEL}'`, ...WINDOWS_PATH_BROADCAST_STATEMENTS, ].join("; "); @@ -943,14 +951,16 @@ async function removeWindowsPathEntry( { PLANNOTATOR_UNINSTALL_PATH: paths.windowsInstallDir }, ); // The script echoes the original PATH only after the registry write - // succeeded, so a parseable echo proves the edit happened even when the - // process was later killed while broadcasting the change. + // succeeded, so a parseable echo proves the edit happened however the + // process ended afterwards: killed by the timeout while broadcasting, or + // taken down by a native fault inside Add-Type / SendMessageTimeout that no + // try/catch can intercept and that exits with an NTSTATUS code. const echoedOriginalPath = parseEchoedWindowsPath(result.stdout); - if (result.exitCode === 0 || (result.timedOut && echoedOriginalPath !== null)) { + if (result.exitCode === 0 || echoedOriginalPath !== null) { state.removed.push(label); - if (result.timedOut) { + if (result.exitCode !== 0) { state.warnings.push( - `Removed ${paths.windowsInstallDir} from the Windows user PATH, but notifying open windows of the change timed out; new terminals pick up the change after you sign in again.`, + `Removed ${paths.windowsInstallDir} from the Windows user PATH, but notifying open windows of the change ${result.timedOut ? "timed out" : `failed (exit ${result.exitCode})`}; new terminals pick up the change after you sign in again.`, ); } if (echoedOriginalPath !== null) return echoedOriginalPath; @@ -1065,7 +1075,13 @@ async function restoreWindowsPathEntry( ["-NoProfile", "-NonInteractive", "-Command", WINDOWS_PATH_RESTORE_SCRIPT], { PLANNOTATOR_UNINSTALL_ORIGINAL_PATH: originalPath }, ); - if (result.exitCode !== 0) { + // Same proof as the removal: the sentinel is printed only after the + // registry write, so its presence means the restore completed even if the + // broadcast then timed out or faulted. + const restored = (result.stdout ?? "") + .split(/\r?\n/) + .some((line) => line.trim() === WINDOWS_PATH_RESTORED_SENTINEL); + if (result.exitCode !== 0 && !restored) { state.errors.push( `Could not restore ${paths.windowsInstallDir} to the Windows user PATH after self-delete scheduling failed (${result.timedOut ? "command timed out" : `exit ${result.exitCode}`}).`, ); @@ -1074,6 +1090,11 @@ async function restoreWindowsPathEntry( ); return; } + if (result.exitCode !== 0) { + state.warnings.push( + `Restored ${paths.windowsInstallDir} to the Windows user PATH, but notifying open windows of the change ${result.timedOut ? "timed out" : `failed (exit ${result.exitCode})`}; new terminals pick up the change after you sign in again.`, + ); + } const label = `Windows user PATH entry ${paths.windowsInstallDir}`; const removedIndex = state.removed.indexOf(label);