mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(plugins): prevent uninstalled npm plugins from coming back (#113902)
* fix(plugins): preserve retained uninstall files * test(plugins): track retention temp dirs * fix(plugins): retain rollback marker snapshots
This commit is contained in:
committed by
GitHub
parent
75e2595123
commit
0fe49c5731
@@ -749,6 +749,23 @@ function assertNpmPluginRemoved() {
|
||||
}
|
||||
}
|
||||
|
||||
function assertNpmPluginRetained() {
|
||||
const installPath = fs.readFileSync(scratchFile("plugins-npm-install-path.txt"), "utf8").trim();
|
||||
const dependencyPackagePath = fs
|
||||
.readFileSync(scratchFile("plugins-npm-dependency-path.txt"), "utf8")
|
||||
.trim();
|
||||
assertPluginRemoved({
|
||||
pluginId: "demo-plugin-npm",
|
||||
listFile: scratchFile("plugins-npm-retained.json"),
|
||||
});
|
||||
if (!fs.existsSync(installPath)) {
|
||||
throw new Error(`npm managed package was deleted by --keep-files: ${installPath}`);
|
||||
}
|
||||
if (!fs.existsSync(dependencyPackagePath)) {
|
||||
throw new Error(`npm managed dependency was deleted by --keep-files: ${dependencyPackagePath}`);
|
||||
}
|
||||
}
|
||||
|
||||
function assertInvalidOpenClawExtensionsRejected() {
|
||||
const pluginId = "demo-plugin-invalid-metadata";
|
||||
for (const expected of ["openclaw.extensions[1]", "non-empty string"]) {
|
||||
@@ -1025,6 +1042,7 @@ const commands = {
|
||||
"plugin-file-removed": assertPluginFileRemoved,
|
||||
"plugin-npm": assertNpmPlugin,
|
||||
"plugin-npm-update": assertNpmPluginUpdateUnchanged,
|
||||
"plugin-npm-retained": assertNpmPluginRetained,
|
||||
"plugin-npm-removed": assertNpmPluginRemoved,
|
||||
"invalid-openclaw-extensions": assertInvalidOpenClawExtensionsRejected,
|
||||
"bundle-disabled": assertClaudeBundleDisabled,
|
||||
|
||||
@@ -223,6 +223,16 @@ node scripts/e2e/lib/plugins/assertions.mjs plugin-npm
|
||||
openclaw_e2e_maybe_timeout "$OPENCLAW_PLUGINS_CLI_TIMEOUT" node "$OPENCLAW_ENTRY" plugins update demo-plugin-npm >"$OPENCLAW_PLUGINS_TMP_DIR/plugins-npm-update.log" 2>&1
|
||||
node scripts/e2e/lib/plugins/assertions.mjs plugin-npm-update
|
||||
|
||||
run_plugins_openclaw_logged uninstall-npm-retained plugins uninstall demo-plugin-npm --force --keep-files
|
||||
run_plugins_openclaw_capture "$OPENCLAW_PLUGINS_TMP_DIR/plugins-npm-retained.json" plugins list --json
|
||||
node scripts/e2e/lib/plugins/assertions.mjs plugin-npm-retained
|
||||
|
||||
run_plugins_openclaw_logged reinstall-npm plugins install "npm:@openclaw/demo-plugin-npm@0.0.1" --force
|
||||
run_plugins_openclaw_capture "$OPENCLAW_PLUGINS_TMP_DIR/plugins-npm.json" plugins list --json
|
||||
run_plugins_openclaw_capture "$OPENCLAW_PLUGINS_TMP_DIR/plugins-npm-inspect.json" plugins inspect demo-plugin-npm --runtime --json
|
||||
run_plugins_shell_logged exec-reinstalled-npm-plugin-cli 'node "$OPENCLAW_ENTRY" demo-npm ping >"$OPENCLAW_PLUGINS_TMP_DIR/plugins-npm-cli.txt"'
|
||||
node scripts/e2e/lib/plugins/assertions.mjs plugin-npm
|
||||
|
||||
run_plugins_openclaw_logged uninstall-npm plugins uninstall demo-plugin-npm --force
|
||||
run_plugins_openclaw_capture "$OPENCLAW_PLUGINS_TMP_DIR/plugins-npm-uninstalled.json" plugins list --json
|
||||
node scripts/e2e/lib/plugins/assertions.mjs plugin-npm-removed
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
import fs from "node:fs";
|
||||
import { expect, it, vi } from "vitest";
|
||||
import { RETAINED_MANAGED_NPM_KEEP_FILES_REASON } from "../plugins/managed-npm-retention-contract.js";
|
||||
import {
|
||||
hasRetainedManagedNpmInstallMarker,
|
||||
markRetainedManagedNpmInstall,
|
||||
} from "../plugins/managed-npm-retention.js";
|
||||
import { writeManagedNpmPlugin } from "../plugins/test-helpers/managed-npm-plugin.js";
|
||||
import { withOpenClawTestState } from "../test-utils/openclaw-test-state.js";
|
||||
import { cleanupRetainedPluginInstallGenerations } from "./server-retained-plugin-cleanup.js";
|
||||
|
||||
it("preserves package files retained by plugin uninstall", async () => {
|
||||
await withOpenClawTestState({ label: "gateway-retained-plugin-cleanup" }, async (state) => {
|
||||
const packageDir = writeManagedNpmPlugin({
|
||||
stateDir: state.stateDir,
|
||||
packageName: "@openclaw/kept-plugin",
|
||||
pluginId: "kept-plugin",
|
||||
version: "1.0.0",
|
||||
});
|
||||
await markRetainedManagedNpmInstall({
|
||||
packageDir,
|
||||
pluginId: "kept-plugin",
|
||||
reason: RETAINED_MANAGED_NPM_KEEP_FILES_REASON,
|
||||
});
|
||||
const log = { info: vi.fn(), warn: vi.fn() };
|
||||
|
||||
await cleanupRetainedPluginInstallGenerations({ log });
|
||||
|
||||
expect(fs.existsSync(packageDir)).toBe(true);
|
||||
expect(hasRetainedManagedNpmInstallMarker(packageDir)).toBe(true);
|
||||
expect(log.info).not.toHaveBeenCalled();
|
||||
expect(log.warn).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,141 @@
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { PluginInstallRecord } from "../config/types.plugins.js";
|
||||
import { withOpenClawTestState } from "../test-utils/openclaw-test-state.js";
|
||||
import { commitPluginInstallRecordsWithConfig } from "./install-record-commit.js";
|
||||
import { listRecoveredManagedNpmInstallCandidates } from "./installed-plugin-index-record-reader.js";
|
||||
import {
|
||||
cleanupRetainedManagedNpmInstallGenerations,
|
||||
hasRetainedManagedNpmInstallMarker,
|
||||
} from "./managed-npm-retention.js";
|
||||
import { writeManagedNpmPlugin } from "./test-helpers/managed-npm-plugin.js";
|
||||
|
||||
function npmRecord(packageName: string, installPath: string): PluginInstallRecord {
|
||||
return { source: "npm", spec: `${packageName}@1.0.0`, installPath };
|
||||
}
|
||||
|
||||
describe("retained managed npm record commits", () => {
|
||||
it("suppresses recovery when a retained install record is removed", async () => {
|
||||
await withOpenClawTestState({ label: "retained-record-removal" }, async (state) => {
|
||||
const packageName = "@openclaw/retained-demo";
|
||||
const installPath = writeManagedNpmPlugin({
|
||||
stateDir: state.stateDir,
|
||||
packageName,
|
||||
pluginId: "retained-demo",
|
||||
version: "1.0.0",
|
||||
});
|
||||
expect(
|
||||
listRecoveredManagedNpmInstallCandidates({ stateDir: state.stateDir }).map(
|
||||
(candidate) => candidate.pluginId,
|
||||
),
|
||||
).toContain("retained-demo");
|
||||
|
||||
await commitPluginInstallRecordsWithConfig({
|
||||
previousInstallRecords: { "retained-demo": npmRecord(packageName, installPath) },
|
||||
nextInstallRecords: {},
|
||||
nextConfig: {},
|
||||
});
|
||||
|
||||
expect(hasRetainedManagedNpmInstallMarker(installPath)).toBe(true);
|
||||
expect(
|
||||
listRecoveredManagedNpmInstallCandidates({ stateDir: state.stateDir }).map(
|
||||
(candidate) => candidate.pluginId,
|
||||
),
|
||||
).not.toContain("retained-demo");
|
||||
});
|
||||
});
|
||||
|
||||
it.each(["direct", "symlink"] as const)(
|
||||
"does not retire a package still used by a %s active install path",
|
||||
async (activePathKind) => {
|
||||
await withOpenClawTestState({ label: `retained-active-${activePathKind}` }, async (state) => {
|
||||
const packageName = "@openclaw/retained-active";
|
||||
const installPath = writeManagedNpmPlugin({
|
||||
stateDir: state.stateDir,
|
||||
packageName,
|
||||
pluginId: "retained-active",
|
||||
version: "1.0.0",
|
||||
});
|
||||
let activePath = installPath;
|
||||
if (activePathKind === "symlink") {
|
||||
activePath = state.statePath("active", "retained-active");
|
||||
fs.mkdirSync(path.dirname(activePath), { recursive: true });
|
||||
fs.symlinkSync(installPath, activePath, "dir");
|
||||
}
|
||||
|
||||
await commitPluginInstallRecordsWithConfig({
|
||||
previousInstallRecords: { "retained-active": npmRecord(packageName, installPath) },
|
||||
nextInstallRecords: {
|
||||
"active-alias": {
|
||||
source: "path",
|
||||
sourcePath: activePath,
|
||||
installPath: activePath,
|
||||
},
|
||||
},
|
||||
nextConfig: {},
|
||||
});
|
||||
|
||||
expect(hasRetainedManagedNpmInstallMarker(installPath)).toBe(false);
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it("does not retire a removed npm record outside the managed npm root", async () => {
|
||||
await withOpenClawTestState({ label: "retained-outside-root" }, async (state) => {
|
||||
const outsideRoot = state.path("outside-root");
|
||||
try {
|
||||
const packageName = "@openclaw/outside-retained";
|
||||
const installPath = writeManagedNpmPlugin({
|
||||
stateDir: outsideRoot,
|
||||
packageName,
|
||||
pluginId: "outside-retained",
|
||||
version: "1.0.0",
|
||||
});
|
||||
await commitPluginInstallRecordsWithConfig({
|
||||
previousInstallRecords: { "outside-retained": npmRecord(packageName, installPath) },
|
||||
nextInstallRecords: {},
|
||||
nextConfig: {},
|
||||
});
|
||||
expect(hasRetainedManagedNpmInstallMarker(installPath)).toBe(false);
|
||||
} finally {
|
||||
fs.rmSync(outsideRoot, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps npm-to-local source changes cleanup-eligible", async () => {
|
||||
await withOpenClawTestState({ label: "retained-source-change" }, async (state) => {
|
||||
const packageName = "@openclaw/moved-local";
|
||||
const installPath = writeManagedNpmPlugin({
|
||||
stateDir: state.stateDir,
|
||||
packageName,
|
||||
pluginId: "moved-local",
|
||||
version: "1.0.0",
|
||||
});
|
||||
const localInstallPath = state.statePath("extensions", "moved-local");
|
||||
fs.mkdirSync(localInstallPath, { recursive: true });
|
||||
|
||||
await commitPluginInstallRecordsWithConfig({
|
||||
previousInstallRecords: { "moved-local": npmRecord(packageName, installPath) },
|
||||
nextInstallRecords: {
|
||||
"moved-local": {
|
||||
source: "path",
|
||||
sourcePath: localInstallPath,
|
||||
installPath: localInstallPath,
|
||||
},
|
||||
},
|
||||
nextConfig: {},
|
||||
});
|
||||
|
||||
expect(hasRetainedManagedNpmInstallMarker(installPath)).toBe(true);
|
||||
await expect(
|
||||
cleanupRetainedManagedNpmInstallGenerations({
|
||||
activeInstallPaths: [localInstallPath],
|
||||
}),
|
||||
).resolves.toBe(1);
|
||||
expect(fs.existsSync(installPath)).toBe(false);
|
||||
expect(fs.existsSync(localInstallPath)).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -2,7 +2,8 @@
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { useAutoCleanupTempDirTracker } from "../../test/helpers/temp-dir.js";
|
||||
import {
|
||||
createPluginInstallRecordMap,
|
||||
getPluginInstallRecordMapEntry,
|
||||
@@ -11,11 +12,16 @@ import {
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import type { PluginInstallRecord } from "../config/types.plugins.js";
|
||||
import { withEnvAsync } from "../test-utils/env.js";
|
||||
import { listRecoveredManagedNpmInstallCandidates } from "./installed-plugin-index-record-reader.js";
|
||||
import type { InstalledPluginIndex } from "./installed-plugin-index.js";
|
||||
import {
|
||||
hasRetainedManagedNpmInstallMarker,
|
||||
markRetainedManagedNpmInstall,
|
||||
resolveRetainedManagedNpmInstallMarkerPath,
|
||||
} from "./managed-npm-retention.js";
|
||||
import { writeManagedNpmPlugin } from "./test-helpers/managed-npm-plugin.js";
|
||||
|
||||
const retentionTempDirs = useAutoCleanupTempDirTracker(afterEach);
|
||||
|
||||
const mocks = vi.hoisted(() => {
|
||||
const lease = {
|
||||
@@ -480,6 +486,49 @@ describe("commitConfigWithPendingPluginInstalls", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("removes a new retirement marker when the leased config commit rolls back", async () => {
|
||||
const stateDir = retentionTempDirs.make("openclaw-record-commit-");
|
||||
const installPath = writeManagedNpmPlugin({
|
||||
stateDir,
|
||||
packageName: "@openclaw/retained-rollback",
|
||||
pluginId: "retained-rollback",
|
||||
version: "1.0.0",
|
||||
});
|
||||
const previousInstallRecords: Record<string, PluginInstallRecord> = {
|
||||
"retained-rollback": {
|
||||
source: "npm",
|
||||
spec: "@openclaw/retained-rollback@1.0.0",
|
||||
installPath,
|
||||
},
|
||||
};
|
||||
mocks.replaceConfigFile.mockRejectedValueOnce(new Error("config changed"));
|
||||
|
||||
try {
|
||||
await withEnvAsync({ OPENCLAW_STATE_DIR: stateDir }, async () => {
|
||||
await expect(
|
||||
commitPluginInstallRecordsWithConfig({
|
||||
previousInstallRecords,
|
||||
nextInstallRecords: {},
|
||||
nextConfig: {},
|
||||
}),
|
||||
).rejects.toThrow("config changed");
|
||||
|
||||
expect(hasRetainedManagedNpmInstallMarker(installPath)).toBe(false);
|
||||
expect(
|
||||
listRecoveredManagedNpmInstallCandidates({ stateDir }).map(
|
||||
(candidate) => candidate.pluginId,
|
||||
),
|
||||
).toContain("retained-rollback");
|
||||
expect(mocks.restorePersistedInstalledPluginIndexIfCurrent).toHaveBeenCalledWith(null, 1, {
|
||||
filePath: mocks.lease.databasePath,
|
||||
lease: mocks.lease,
|
||||
});
|
||||
});
|
||||
} finally {
|
||||
fs.rmSync(stateDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("does not mark arbitrary npm paths outside the managed npm root", async () => {
|
||||
const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-record-commit-"));
|
||||
const outsideRoot = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-record-outside-"));
|
||||
@@ -805,6 +854,67 @@ describe("commitConfigWithPendingPluginInstalls", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("restores earlier active markers when clearing a later marker fails", async () => {
|
||||
const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-record-commit-"));
|
||||
const installPaths = ["codex", "voice-call"].map((pluginId) =>
|
||||
path.join(
|
||||
stateDir,
|
||||
"npm",
|
||||
"projects",
|
||||
`${pluginId}-v2`,
|
||||
"node_modules",
|
||||
"@openclaw",
|
||||
pluginId,
|
||||
),
|
||||
);
|
||||
for (const [index, installPath] of installPaths.entries()) {
|
||||
fs.mkdirSync(installPath, { recursive: true });
|
||||
await markRetainedManagedNpmInstall({
|
||||
packageDir: installPath,
|
||||
pluginId: index === 0 ? "codex" : "voice-call",
|
||||
retainedAt: "2026-04-25T00:00:00.000Z",
|
||||
reason: "test-retained-generation",
|
||||
});
|
||||
}
|
||||
const laterMarkerPath = resolveRetainedManagedNpmInstallMarkerPath(installPaths[1] ?? "");
|
||||
const realRm = fs.promises.rm.bind(fs.promises);
|
||||
const rmSpy = vi.spyOn(fs.promises, "rm").mockImplementation(async (target, options) => {
|
||||
if (String(target) === laterMarkerPath) {
|
||||
const error = new Error("marker clear failed") as NodeJS.ErrnoException;
|
||||
error.code = "EIO";
|
||||
throw error;
|
||||
}
|
||||
return await realRm(target, options);
|
||||
});
|
||||
|
||||
try {
|
||||
await expect(
|
||||
commitPluginInstallRecordsWithConfig({
|
||||
previousInstallRecords: {},
|
||||
nextInstallRecords: Object.fromEntries(
|
||||
installPaths.map((installPath, index) => {
|
||||
const pluginId = index === 0 ? "codex" : "voice-call";
|
||||
return [
|
||||
pluginId,
|
||||
{
|
||||
source: "npm",
|
||||
spec: `@openclaw/${pluginId}@2.0.0`,
|
||||
installPath,
|
||||
},
|
||||
];
|
||||
}),
|
||||
),
|
||||
nextConfig: {},
|
||||
}),
|
||||
).rejects.toThrow("marker clear failed");
|
||||
|
||||
expect(installPaths.every(hasRetainedManagedNpmInstallMarker)).toBe(true);
|
||||
} finally {
|
||||
rmSpy.mockRestore();
|
||||
fs.rmSync(stateDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("rolls back plugin index writes when the config write fails", async () => {
|
||||
const existingRecords: Record<string, PluginInstallRecord> = {
|
||||
existing: {
|
||||
|
||||
@@ -21,6 +21,7 @@ import {
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import type { PluginInstallRecord } from "../config/types.plugins.js";
|
||||
import { isPathInside } from "../infra/path-guards.js";
|
||||
import { resolveDefaultPluginNpmDir, resolvePluginNpmProjectsDir } from "./install-paths.js";
|
||||
import {
|
||||
loadInstalledPluginIndexInstallRecords,
|
||||
PLUGIN_INSTALLS_CONFIG_PATH,
|
||||
@@ -32,6 +33,7 @@ import {
|
||||
restorePersistedInstalledPluginIndexIfCurrent,
|
||||
type InstalledPluginIndexWriteReceipt,
|
||||
} from "./installed-plugin-index-store.js";
|
||||
import { RETAINED_MANAGED_NPM_KEEP_FILES_REASON } from "./managed-npm-retention-contract.js";
|
||||
import {
|
||||
clearRetainedManagedNpmInstallMarker,
|
||||
markRetainedManagedNpmInstall,
|
||||
@@ -131,9 +133,26 @@ function mergeAfterWrite(
|
||||
};
|
||||
}
|
||||
|
||||
function isMissingInstallPathError(error: unknown): boolean {
|
||||
const code = (error as NodeJS.ErrnoException).code;
|
||||
return code === "ENOENT" || code === "ENOTDIR";
|
||||
}
|
||||
|
||||
function resolveExistingInstallPath(installPath: string): string {
|
||||
const resolvedPath = path.resolve(installPath);
|
||||
try {
|
||||
return fs.realpathSync(resolvedPath);
|
||||
} catch (error) {
|
||||
if (isMissingInstallPathError(error)) {
|
||||
return resolvedPath;
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
function installPathsOverlap(left: string, right: string): boolean {
|
||||
const resolvedLeft = path.resolve(left);
|
||||
const resolvedRight = path.resolve(right);
|
||||
const resolvedLeft = resolveExistingInstallPath(left);
|
||||
const resolvedRight = resolveExistingInstallPath(right);
|
||||
return (
|
||||
resolvedLeft === resolvedRight ||
|
||||
isPathInside(resolvedLeft, resolvedRight) ||
|
||||
@@ -146,18 +165,52 @@ function resolveRetainedManagedNpmInstallMarkerTarget(params: {
|
||||
previousRecord?: PluginInstallRecord;
|
||||
nextRecord?: PluginInstallRecord;
|
||||
}): string | null {
|
||||
if (params.previousRecord?.source !== "npm" || params.nextRecord?.source !== "npm") {
|
||||
if (params.previousRecord?.source !== "npm") {
|
||||
return null;
|
||||
}
|
||||
const previousInstallPath = params.previousRecord.installPath?.trim();
|
||||
const nextInstallPath = params.nextRecord.installPath?.trim();
|
||||
if (!previousInstallPath || !nextInstallPath) {
|
||||
const nextInstallPath = params.nextRecord?.installPath?.trim();
|
||||
if (!previousInstallPath) {
|
||||
return null;
|
||||
}
|
||||
if (installPathsOverlap(previousInstallPath, nextInstallPath)) {
|
||||
if (
|
||||
params.nextRecord &&
|
||||
(!nextInstallPath || installPathsOverlap(previousInstallPath, nextInstallPath))
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (params.nextRecord?.source !== "npm") {
|
||||
const packageInfo = resolveRetainedManagedNpmInstallPackageInfo(previousInstallPath);
|
||||
if (!packageInfo) {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
const configuredNpmRoot = path.resolve(resolveDefaultPluginNpmDir());
|
||||
const npmRoot = fs.realpathSync(configuredNpmRoot);
|
||||
const configuredProjectRoot = path.resolve(packageInfo.projectRoot);
|
||||
const projectRoot = fs.realpathSync(configuredProjectRoot);
|
||||
const packageDir = fs.realpathSync(previousInstallPath);
|
||||
if (
|
||||
path.relative(configuredNpmRoot, configuredProjectRoot) !==
|
||||
path.relative(npmRoot, projectRoot) ||
|
||||
path.relative(configuredProjectRoot, path.resolve(previousInstallPath)) !==
|
||||
path.relative(projectRoot, packageDir)
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
if (projectRoot === npmRoot) {
|
||||
return previousInstallPath;
|
||||
}
|
||||
const projectsRoot = fs.realpathSync(resolvePluginNpmProjectsDir(npmRoot));
|
||||
return path.dirname(projectRoot) === projectsRoot ? previousInstallPath : null;
|
||||
} catch (error) {
|
||||
if (isMissingInstallPathError(error)) {
|
||||
return null;
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
const installs = createPluginInstallRecordMap<PluginInstallRecord>();
|
||||
setPluginInstallRecordMapEntry(installs, params.pluginId, params.previousRecord);
|
||||
const plan = planPluginUninstall({
|
||||
@@ -177,7 +230,7 @@ function resolveRetainedManagedNpmInstallMarkerTarget(params: {
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
if (installPathsOverlap(plan.directoryRemoval.target, nextInstallPath)) {
|
||||
if (nextInstallPath && installPathsOverlap(plan.directoryRemoval.target, nextInstallPath)) {
|
||||
return null;
|
||||
}
|
||||
return plan.directoryRemoval.target;
|
||||
@@ -206,17 +259,30 @@ function findReplacementNpmRecordForRemovedRecord(params: {
|
||||
return null;
|
||||
}
|
||||
|
||||
async function markRetainedReplacedManagedNpmInstallRecords(params: {
|
||||
async function markRetiredManagedNpmInstallRecords(params: {
|
||||
previousInstallRecords: Record<string, PluginInstallRecord>;
|
||||
nextInstallRecords: Record<string, PluginInstallRecord>;
|
||||
createdMarkerPaths: string[];
|
||||
}): Promise<void> {
|
||||
const markedPreviousPluginIds = new Set<string>();
|
||||
const markReplacement = async (
|
||||
const activeInstallPaths = Object.values(params.nextInstallRecords).flatMap((record) => {
|
||||
const installPath = record.installPath?.trim();
|
||||
return installPath ? [installPath] : [];
|
||||
});
|
||||
const markRetiredInstall = async (
|
||||
pluginId: string,
|
||||
previousRecord: PluginInstallRecord | undefined,
|
||||
nextRecord: PluginInstallRecord | undefined,
|
||||
) => {
|
||||
const previousInstallPath = previousRecord?.installPath?.trim();
|
||||
if (
|
||||
previousInstallPath &&
|
||||
activeInstallPaths.some((installPath) =>
|
||||
installPathsOverlap(previousInstallPath, installPath),
|
||||
)
|
||||
) {
|
||||
return;
|
||||
}
|
||||
const packageDir = resolveRetainedManagedNpmInstallMarkerTarget({
|
||||
pluginId,
|
||||
previousRecord,
|
||||
@@ -230,7 +296,12 @@ async function markRetainedReplacedManagedNpmInstallRecords(params: {
|
||||
const marked = await markRetainedManagedNpmInstall({
|
||||
packageDir,
|
||||
pluginId,
|
||||
reason: "replaced-by-managed-npm-generation-update",
|
||||
reason:
|
||||
nextRecord?.source === "npm"
|
||||
? "replaced-by-managed-npm-generation-update"
|
||||
: nextRecord
|
||||
? "replaced-by-plugin-source-change"
|
||||
: RETAINED_MANAGED_NPM_KEEP_FILES_REASON,
|
||||
});
|
||||
if (marked && !markerAlreadyExisted) {
|
||||
// Record each marker immediately so a later filesystem failure can roll it back.
|
||||
@@ -240,7 +311,7 @@ async function markRetainedReplacedManagedNpmInstallRecords(params: {
|
||||
};
|
||||
|
||||
for (const [pluginId, nextRecord] of Object.entries(params.nextInstallRecords)) {
|
||||
await markReplacement(
|
||||
await markRetiredInstall(
|
||||
pluginId,
|
||||
getPluginInstallRecordMapEntry(params.previousInstallRecords, pluginId),
|
||||
nextRecord,
|
||||
@@ -253,7 +324,7 @@ async function markRetainedReplacedManagedNpmInstallRecords(params: {
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
await markReplacement(
|
||||
await markRetiredInstall(
|
||||
pluginId,
|
||||
previousRecord,
|
||||
findReplacementNpmRecordForRemovedRecord({
|
||||
@@ -272,8 +343,8 @@ async function removeCreatedRetainedManagedNpmInstallMarkers(markerPaths: string
|
||||
|
||||
async function clearActiveRetainedManagedNpmInstallMarkers(
|
||||
nextInstallRecords: Record<string, PluginInstallRecord>,
|
||||
): Promise<Array<{ markerPath: string; contents: string }>> {
|
||||
const clearedMarkers: Array<{ markerPath: string; contents: string }> = [];
|
||||
clearedMarkers: Array<{ markerPath: string; contents: string }>,
|
||||
): Promise<void> {
|
||||
for (const record of Object.values(nextInstallRecords)) {
|
||||
if (record.source !== "npm" || !record.installPath?.trim()) {
|
||||
continue;
|
||||
@@ -295,10 +366,10 @@ async function clearActiveRetainedManagedNpmInstallMarkers(
|
||||
}
|
||||
const cleared = await clearRetainedManagedNpmInstallMarker(record.installPath);
|
||||
if (cleared) {
|
||||
// Record each cleared marker immediately so a later filesystem failure can roll it back.
|
||||
clearedMarkers.push({ markerPath, contents });
|
||||
}
|
||||
}
|
||||
return clearedMarkers;
|
||||
}
|
||||
|
||||
async function restoreClearedRetainedManagedNpmInstallMarkers(
|
||||
@@ -337,14 +408,15 @@ async function commitPluginInstallRecordsWithWriter(params: {
|
||||
lease,
|
||||
},
|
||||
);
|
||||
await markRetainedReplacedManagedNpmInstallRecords({
|
||||
await markRetiredManagedNpmInstallRecords({
|
||||
previousInstallRecords: prepared.previousInstallRecords,
|
||||
nextInstallRecords: prepared.nextInstallRecords,
|
||||
// Keep partial progress visible to the rollback path.
|
||||
createdMarkerPaths: retainedMarkerPaths,
|
||||
});
|
||||
clearedMarkerSnapshots.push(
|
||||
...(await clearActiveRetainedManagedNpmInstallMarkers(prepared.nextInstallRecords)),
|
||||
await clearActiveRetainedManagedNpmInstallMarkers(
|
||||
prepared.nextInstallRecords,
|
||||
clearedMarkerSnapshots,
|
||||
);
|
||||
const installRecordsChanged = !pluginInstallRecordMapsEqual(
|
||||
prepared.previousInstallRecords,
|
||||
|
||||
@@ -0,0 +1,2 @@
|
||||
/** Marker reason for packages preserved by an explicit `plugins uninstall --keep-files`. */
|
||||
export const RETAINED_MANAGED_NPM_KEEP_FILES_REASON = "removed-managed-npm-install-retained";
|
||||
@@ -1,8 +1,12 @@
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { useAutoCleanupTempDirTracker } from "../../test/helpers/temp-dir.js";
|
||||
import { resolvePluginNpmGenerationProjectDir } from "./install-paths.js";
|
||||
import { RETAINED_MANAGED_NPM_KEEP_FILES_REASON } from "./managed-npm-retention-contract.js";
|
||||
|
||||
const retentionTempDirs = useAutoCleanupTempDirTracker(afterEach);
|
||||
import {
|
||||
cleanupRetainedManagedNpmInstallGenerations,
|
||||
hasRetainedManagedNpmInstallMarker,
|
||||
@@ -72,4 +76,35 @@ describe("managed npm retention", () => {
|
||||
fs.rmSync(stateDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it.each(["project", "legacy"] as const)(
|
||||
"preserves %s packages retained by an explicit keep-files uninstall",
|
||||
async (layout) => {
|
||||
const stateDir = retentionTempDirs.make("openclaw-retention-");
|
||||
const npmDir = path.join(stateDir, "npm");
|
||||
const projectRoot =
|
||||
layout === "legacy"
|
||||
? npmDir
|
||||
: resolvePluginNpmGenerationProjectDir({
|
||||
npmDir,
|
||||
packageName: "@openclaw/kept-plugin",
|
||||
generationKey: "kept-plugin-v1",
|
||||
});
|
||||
const packageDir = path.join(projectRoot, "node_modules", "@openclaw", "kept-plugin");
|
||||
fs.mkdirSync(packageDir, { recursive: true });
|
||||
await markRetainedManagedNpmInstall({
|
||||
packageDir,
|
||||
pluginId: "kept-plugin",
|
||||
reason: RETAINED_MANAGED_NPM_KEEP_FILES_REASON,
|
||||
});
|
||||
|
||||
try {
|
||||
await expect(cleanupRetainedManagedNpmInstallGenerations({ npmDir })).resolves.toBe(0);
|
||||
expect(fs.existsSync(packageDir)).toBe(true);
|
||||
expect(hasRetainedManagedNpmInstallMarker(packageDir)).toBe(true);
|
||||
} finally {
|
||||
fs.rmSync(stateDir, { recursive: true, force: true });
|
||||
}
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
@@ -1,12 +1,23 @@
|
||||
// Marks retained managed npm package trees that should stay importable but not recoverable.
|
||||
// Marks managed npm packages excluded from recovery and classifies cleanup eligibility.
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import { isRecord } from "@openclaw/normalization-core/record-coerce";
|
||||
import { safePathSegmentHashed } from "../infra/install-safe-path.js";
|
||||
import { resolveDefaultPluginNpmDir, resolvePluginNpmProjectsDir } from "./install-paths.js";
|
||||
import { RETAINED_MANAGED_NPM_KEEP_FILES_REASON } from "./managed-npm-retention-contract.js";
|
||||
import { listManagedPluginNpmRootsSync } from "./npm-project-roots.js";
|
||||
|
||||
const RETAINED_MANAGED_NPM_INSTALL_MARKER_DIR = ".openclaw-retained-npm-installs";
|
||||
|
||||
function markerPreservesPackageFiles(markerPath: string): boolean {
|
||||
try {
|
||||
const marker: unknown = JSON.parse(fs.readFileSync(markerPath, "utf8"));
|
||||
return isRecord(marker) && marker.reason === RETAINED_MANAGED_NPM_KEEP_FILES_REASON;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
export function resolveRetainedManagedNpmInstallPackageInfo(packageDir: string): {
|
||||
packageName: string;
|
||||
projectRoot: string;
|
||||
@@ -150,6 +161,7 @@ async function cleanupRetainedLegacyNpmPackages(params: {
|
||||
for (const packageDir of listManagedNpmPackageDirs(params.npmRoot)) {
|
||||
if (
|
||||
!hasRetainedManagedNpmInstallMarker(packageDir) ||
|
||||
markerPreservesPackageFiles(resolveRetainedManagedNpmInstallMarkerPath(packageDir)) ||
|
||||
params.activeInstallPaths.some((installPath) => isPathEqualOrInside(packageDir, installPath))
|
||||
) {
|
||||
continue;
|
||||
@@ -205,6 +217,9 @@ export async function cleanupRetainedManagedNpmInstallGenerations(
|
||||
}
|
||||
if (
|
||||
markerEntries.length === 0 ||
|
||||
markerEntries.some((entry) =>
|
||||
markerPreservesPackageFiles(path.join(markerDir, entry.name)),
|
||||
) ||
|
||||
!isPathEqualOrInside(projectsDir, projectRoot) ||
|
||||
activeInstallPaths.some((installPath) => isPathEqualOrInside(projectRoot, installPath))
|
||||
) {
|
||||
|
||||
Reference in New Issue
Block a user