From cd731fb63eb40f70d62d4875f5de652f5c804425 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Tue, 23 Jun 2026 16:00:07 +0800 Subject: [PATCH] fix(plugins): avoid mutating retained rollback generations --- src/plugins/install.npm-spec.test.ts | 88 +++++++++++++++++++++++++++- src/plugins/install.ts | 28 ++++++++- 2 files changed, 112 insertions(+), 4 deletions(-) diff --git a/src/plugins/install.npm-spec.test.ts b/src/plugins/install.npm-spec.test.ts index 1fecb4230f1..be7e47d2f55 100644 --- a/src/plugins/install.npm-spec.test.ts +++ b/src/plugins/install.npm-spec.test.ts @@ -999,6 +999,89 @@ describe("installPluginFromNpmSpec", () => { await expect(newModule.default.runAttempt()).resolves.toEqual({ chunk: "new" }); }); + it("does not mutate a retained generation when an exact rollback reuses its artifact key", async () => { + const npmRoot = path.join(suiteTempRootTracker.makeTempDir(), "npm"); + const packageName = "@openclaw/codex"; + const install = async (version: string, options: { mode?: "update" }) => + installPluginFromNpmSpec({ + spec: `${packageName}@${version}`, + npmDir: npmRoot, + mode: options.mode, + logger: { info: () => {}, warn: () => {} }, + }); + + mockNpmViewAndInstall({ + spec: `${packageName}@2.0.0`, + packageName, + version: "2.0.0", + pluginId: "codex", + npmRoot, + integrity: "sha512-codex-v2", + shasum: "codexv2sha", + indexJs: `module.exports = { + version: "v2", + runAttempt: async () => (await import("./run-attempt-v2.js")).default, +};\n`, + extraDistFiles: { + "run-attempt-v2.js": "module.exports = { chunk: 'v2' };\n", + }, + expectedDependencySpec: "2.0.0", + }); + const first = await install("2.0.0", {}); + expect(first.ok).toBe(true); + if (!first.ok) { + return; + } + const retainedModule = await import( + pathToFileURL(path.join(first.targetDir, "dist", "index.js")).href + ); + const retainedPackageDir = first.targetDir; + + mockNpmViewAndInstall({ + spec: `${packageName}@3.0.0`, + packageName, + version: "3.0.0", + pluginId: "codex", + npmRoot, + integrity: "sha512-codex-v3", + shasum: "codexv3sha", + indexJs: "module.exports = { version: 'v3' };\n", + replaceExisting: true, + expectedDependencySpec: "3.0.0", + }); + const update = await install("3.0.0", { mode: "update" }); + expect(update.ok).toBe(true); + if (!update.ok) { + return; + } + await markRetainedManagedNpmInstall({ + packageDir: retainedPackageDir, + pluginId: "codex", + reason: "test-rollback-retention", + }); + + mockNpmViewAndInstall({ + spec: `${packageName}@2.0.0`, + packageName, + version: "2.0.0", + pluginId: "codex", + npmRoot, + integrity: "sha512-codex-v2", + shasum: "codexv2sha", + indexJs: "module.exports = { version: 'v2-rollback' };\n", + replaceExisting: true, + expectedDependencySpec: "2.0.0", + }); + const rollback = await install("2.0.0", { mode: "update" }); + expect(rollback.ok).toBe(true); + if (!rollback.ok) { + return; + } + expect(rollback.targetDir).not.toBe(retainedPackageDir); + await expect(retainedModule.default.runAttempt()).resolves.toEqual({ chunk: "v2" }); + expect(fs.existsSync(path.join(retainedPackageDir, "dist", "run-attempt-v2.js"))).toBe(true); + }); + it("installs into a fresh generation when the legacy npm target is retained", async () => { const npmRoot = path.join(suiteTempRootTracker.makeTempDir(), "npm"); const packageName = "@openclaw/codex"; @@ -1044,7 +1127,7 @@ describe("installPluginFromNpmSpec", () => { expect(fs.existsSync(legacyPackageDir)).toBe(true); }); - it("allows plain installs to reactivate an existing retained generation", async () => { + it("allocates a fresh generation when a plain install selects a retained artifact", async () => { const npmRoot = path.join(suiteTempRootTracker.makeTempDir(), "npm"); const packageName = "@openclaw/codex"; const legacyPackageDir = resolveTestPluginPackageDir(npmRoot, packageName); @@ -1086,7 +1169,8 @@ describe("installPluginFromNpmSpec", () => { if (!result.ok) { return; } - expect(result.targetDir).toBe(retainedGenerationPackageDir); + expect(result.targetDir).not.toBe(retainedGenerationPackageDir); + expect(hasRetainedManagedNpmInstallMarker(result.targetDir)).toBe(false); expect(hasRetainedManagedNpmInstallMarker(retainedGenerationPackageDir)).toBe(true); expect(hasRetainedManagedNpmInstallMarker(legacyPackageDir)).toBe(true); }); diff --git a/src/plugins/install.ts b/src/plugins/install.ts index 00d4d9a00a2..24d026859c0 100644 --- a/src/plugins/install.ts +++ b/src/plugins/install.ts @@ -1,5 +1,5 @@ // Installs plugins from package specs, local paths, and catalogs. -import { createHash } from "node:crypto"; +import { createHash, randomUUID } from "node:crypto"; import { constants as fsConstants, type Dirent } from "node:fs"; import fs from "node:fs/promises"; import os from "node:os"; @@ -1137,6 +1137,30 @@ function resolveManagedNpmRootForInstall(params: { }); } +function resolveManagedNpmInstallRoot(params: { + npmBaseDir: string; + packageName: string; + npmResolution: NpmSpecResolution; + useGeneration: boolean; +}): string { + const generationKey = resolveManagedNpmRootGenerationKey({ + packageName: params.packageName, + npmResolution: params.npmResolution, + }); + const npmRoot = resolveManagedNpmRootForInstall(params); + const installRoot = resolveManagedNpmRootPackageDir(npmRoot, params.packageName); + if (!params.useGeneration || !hasRetainedManagedNpmInstallMarker(installRoot)) { + return npmRoot; + } + // Never mutate a retained tree: an older process may still hold lazy imports + // rooted there. A fresh activation root keeps that module graph importable. + return resolvePluginNpmGenerationProjectDir({ + npmDir: params.npmBaseDir, + packageName: params.packageName, + generationKey: `${generationKey}\nactivation\n${randomUUID()}`, + }); +} + async function listManagedNpmPackageDirsForPackage(params: { runtime: Awaited>; npmBaseDir: string; @@ -1309,7 +1333,7 @@ async function installPluginFromManagedNpmRoot( packageName: params.packageName, requestedMode: mode, }); - const npmRoot = resolveManagedNpmRootForInstall({ + const npmRoot = resolveManagedNpmInstallRoot({ npmBaseDir, packageName: params.packageName, npmResolution: params.npmResolution,