From 9072bce0c3a6c91a43af53018bb30e6e7c0ee537 Mon Sep 17 00:00:00 2001 From: Todd Miller Date: Sun, 25 Oct 2026 23:42:01 -0700 Subject: [PATCH] fix(server): keep the previous plugin installation when an update replaces it A Git-source install is checked out under `~/.paseo/plugins///checkout`, and `updateSource` deleted that directory once the new install was active. Anything in it that was never pushed went with it: uncommitted files, and commits on a branch or a detached HEAD, which is how the checkout arrives. `removeVersion` now retires the outgoing version root as `.old-` instead of removing it. The copy kept by the previous update is reaped first, so one retired root sits beside the active one and they do not accumulate. Uninstalling still deletes the whole plugin root, kept copies included, and a failed preparation or activation still discards its staging candidate without touching the running installation. The retired root is logged at info level so the path is discoverable; the update result keeps its single `warning` field for the case where the old root could not be kept. Regression test: "keeps the previous Git installation and reaps it on the next update" installs from a Git repository, writes into the checkout without committing, applies a reviewed update, and asserts the retired checkout still holds that file. It fails on the previous code (`expected [] to have a length of 1`) and passes here. A second update asserts the older copy is reaped and the newer one is kept. Refs https://github.com/getpaseo/paseo/issues/7904 --- .../src/server/plugins/index.posix.test.ts | 65 +++++++++++++++++++ packages/server/src/server/plugins/index.ts | 5 +- .../src/server/plugins/managed-source.ts | 28 ++++++-- 3 files changed, 92 insertions(+), 6 deletions(-) diff --git a/packages/server/src/server/plugins/index.posix.test.ts b/packages/server/src/server/plugins/index.posix.test.ts index 71fe2b96e9..b75528dd4a 100644 --- a/packages/server/src/server/plugins/index.posix.test.ts +++ b/packages/server/src/server/plugins/index.posix.test.ts @@ -883,6 +883,71 @@ export default function contribute(server: PluginServerContext) { await service.stopAllPlugins(); }, 30_000); + it("keeps the previous Git installation and reaps it on the next update", async () => { + const home = await mkdtemp(path.join(tmpdir(), "paseo-plugin-home-")); + roots.push(home); + const repository = await mkdtemp(path.join(tmpdir(), "paseo-plugin-repository-")); + roots.push(repository); + await runGitCommand(["init", "-b", "main"], { cwd: repository }); + // Fixture commits must not run the developer's global hooks (core.hooksPath). + await runGitCommand(["config", "core.hooksPath", "/dev/null"], { cwd: repository }); + await runGitCommand(["config", "user.name", "Paseo Tests"], { cwd: repository }); + await runGitCommand(["config", "user.email", "paseo@example.test"], { cwd: repository }); + await writeFile( + path.join(repository, "paseo-plugin.json"), + JSON.stringify({ id: "kept-update" }), + ); + await writeFile( + path.join(repository, "index.server.ts"), + "export default function contribute(plugin: unknown) { void plugin; return () => undefined; }", + ); + await writeFile(path.join(repository, "marker.txt"), "first"); + await runGitCommand(["add", "-A"], { cwd: repository }); + await runGitCommand(["commit", "-m", "first"], { cwd: repository }); + + const service = bindTestSessionHost( + new PluginService(pino({ level: "silent" }), createStore(home), "0.4.0", { + managedSources: new ManagedPluginSources(home), + }), + ); + await service.start(); + const installed = await service.installSource({ source: pathToFileURL(repository).href }); + const pluginRoot = path.dirname(path.dirname(installed.path)); + expect(await readdir(pluginRoot)).toHaveLength(1); + + // Work that exists only inside the checkout: an update must not take it away. + await writeFile(path.join(installed.path, "marker.txt"), "uncommitted work"); + await writeFile(path.join(repository, "marker.txt"), "second"); + await runGitCommand(["add", "-A"], { cwd: repository }); + await runGitCommand(["commit", "-m", "second"], { cwd: repository }); + await applyReviewedUpdate(service, "kept-update"); + + const retired = (await readdir(pluginRoot)).filter((entry) => /\.old-\d+$/.test(entry)); + expect(retired).toHaveLength(1); + expect(await readdir(pluginRoot)).toHaveLength(2); + expect( + await readFile(path.join(pluginRoot, retired[0]!, "checkout", "marker.txt"), "utf8"), + ).toBe("uncommitted work"); + + // A second update replaces the kept copy with the one it just retired. + await writeFile(path.join(repository, "marker.txt"), "third"); + await runGitCommand(["add", "-A"], { cwd: repository }); + await runGitCommand(["commit", "-m", "third"], { cwd: repository }); + await applyReviewedUpdate(service, "kept-update"); + + const reaped = (await readdir(pluginRoot)).filter((entry) => /\.old-\d+$/.test(entry)); + expect(reaped).toHaveLength(1); + expect(await readdir(pluginRoot)).toHaveLength(2); + expect( + await readFile(path.join(pluginRoot, reaped[0]!, "checkout", "marker.txt"), "utf8"), + ).toBe("second"); + + // Uninstalling still clears the plugin root, kept copies included. + await service.removePlugin("kept-update"); + await expect(stat(pluginRoot)).rejects.toThrow(); + await service.stopAllPlugins(); + }, 30_000); + it("activates an update when the enabled plugin previously failed to start", async () => { const home = await mkdtemp(path.join(tmpdir(), "paseo-plugin-home-")); roots.push(home); diff --git a/packages/server/src/server/plugins/index.ts b/packages/server/src/server/plugins/index.ts index a5ecea8097..fe85beb498 100644 --- a/packages/server/src/server/plugins/index.ts +++ b/packages/server/src/server/plugins/index.ts @@ -749,9 +749,10 @@ export class PluginService { this.notify(pluginId); let warning: string | undefined; try { - await managedSources.removeVersion(pluginId, source.path); + const retired = await managedSources.retireVersion(pluginId, source.path); + this.logger.info({ pluginId, directory: retired }, "Kept the previous plugin installation"); } catch (error) { - warning = `Plugin updated, but previous installation cleanup failed: ${error instanceof Error ? error.message : String(error)}`; + warning = `Plugin updated, but the previous installation could not be kept: ${error instanceof Error ? error.message : String(error)}`; } return { id: pluginId, diff --git a/packages/server/src/server/plugins/managed-source.ts b/packages/server/src/server/plugins/managed-source.ts index 5373b7453a..dc47a453c2 100644 --- a/packages/server/src/server/plugins/managed-source.ts +++ b/packages/server/src/server/plugins/managed-source.ts @@ -5,7 +5,7 @@ import { import { resolveRegistryPlugin, type RegistryOptions } from "./managed-source/registry.js"; import { randomUUID } from "node:crypto"; import { existsSync, readFileSync } from "node:fs"; -import { mkdir, realpath, rename, rm } from "node:fs/promises"; +import { mkdir, readdir, realpath, rename, rm } from "node:fs/promises"; import path from "node:path"; import { isDeepStrictEqual } from "node:util"; import compareVersions from "semver/functions/compare.js"; @@ -26,6 +26,9 @@ import { acquireNpm, isNpmSource, readNpmArtifact, resolveNpm } from "./managed- const GIT_TIMEOUT_MS = 120_000; const GIT_ENV = { GIT_TERMINAL_PROMPT: "0" } as const; +/** A retired installation root keeps its version id and gains a timestamp: `.old-`. */ +const RETIRED_VERSION_SUFFIX = ".old-"; +const RETIRED_VERSION_PATTERN = /\.old-\d+$/; // COMPAT(plugin-source-record): v0.8.0; accept and strip pre-correction selector/revision/root copies until daemon floor supports artifact-derived metadata. const ManagedPluginRecordSchema = z.union([ z.object({ @@ -381,10 +384,27 @@ export class ManagedPluginSources { async discard(candidate: ManagedPluginCandidate): Promise { await rm(candidate.versionRoot, { recursive: true, force: true }); } - async removeVersion(pluginId: string, configuredPath: string): Promise { + /** + * Keeps the outgoing installation root instead of deleting it, so a Git checkout's uncommitted + * files and unpushed commits survive an update. The copy from the previous update is reaped + * first, which leaves exactly one retired root beside the active one. + */ + async retireVersion(pluginId: string, configuredPath: string): Promise { const location = this.locate(pluginId, configuredPath); - await assertRealContainment(this.root, path.dirname(location.versionRoot)); - await rm(location.versionRoot, { recursive: true, force: true }); + const pluginRoot = path.dirname(location.versionRoot); + await assertRealContainment(this.root, pluginRoot); + await this.reapRetiredVersions(pluginRoot); + const retired = `${location.versionRoot}${RETIRED_VERSION_SUFFIX}${Date.now()}`; + await rename(location.versionRoot, retired); + return retired; + } + private async reapRetiredVersions(pluginRoot: string): Promise { + for (const entry of await readdir(pluginRoot)) { + if (!RETIRED_VERSION_PATTERN.test(entry)) continue; + const retired = path.join(pluginRoot, entry); + await assertRealContainment(this.root, retired); + await rm(retired, { recursive: true, force: true }); + } } async remove(pluginId: string): Promise { if (!this.records[pluginId]) return;