mirror of
https://github.com/moltbot/moltbot.git
synced 2026-05-13 15:47:28 +00:00
refactor: use fs-safe for staged package swaps
This commit is contained in:
@@ -46,6 +46,10 @@ function normalizeComparablePath(filePath: string): string {
|
||||
return path.join(comparableParent, basename);
|
||||
}
|
||||
|
||||
function createFsError(code: string, message = code): NodeJS.ErrnoException {
|
||||
return Object.assign(new Error(message), { code });
|
||||
}
|
||||
|
||||
async function rebindInstallBasePath(params: {
|
||||
installBaseDir: string;
|
||||
preservedDir: string;
|
||||
@@ -207,6 +211,49 @@ describe("installPackageDir", () => {
|
||||
await expect(fs.readdir(backupRoot)).resolves.toHaveLength(0);
|
||||
});
|
||||
|
||||
it("publishes the staged install through the copy fallback when rename crosses devices", async () => {
|
||||
await fixtureRootTracker.setup();
|
||||
const fixtureRoot = await fixtureRootTracker.make("case");
|
||||
const sourceDir = path.join(fixtureRoot, "source");
|
||||
const installBaseDir = path.join(fixtureRoot, "plugins");
|
||||
const targetDir = path.join(installBaseDir, "demo");
|
||||
await fs.mkdir(sourceDir, { recursive: true });
|
||||
await fs.writeFile(path.join(sourceDir, "marker.txt"), "new");
|
||||
|
||||
const realRename = fs.rename.bind(fs);
|
||||
let exdevMoves = 0;
|
||||
vi.spyOn(fs, "rename").mockImplementation(async (...args: Parameters<typeof fs.rename>) => {
|
||||
const [from, to] = args;
|
||||
const fromPath = String(from);
|
||||
if (
|
||||
exdevMoves === 0 &&
|
||||
path.basename(fromPath).startsWith(".openclaw-install-stage-") &&
|
||||
normalizeComparablePath(String(to)) === normalizeComparablePath(targetDir)
|
||||
) {
|
||||
exdevMoves += 1;
|
||||
throw createFsError("EXDEV", "cross-device link not permitted");
|
||||
}
|
||||
return await realRename(...args);
|
||||
});
|
||||
|
||||
const result = await installPackageDir({
|
||||
sourceDir,
|
||||
targetDir,
|
||||
mode: "install",
|
||||
timeoutMs: 1_000,
|
||||
copyErrorPrefix: "failed to copy plugin",
|
||||
hasDeps: false,
|
||||
depsLogMessage: "Installing deps…",
|
||||
});
|
||||
|
||||
expect(result).toEqual({ ok: true });
|
||||
expect(exdevMoves).toBe(1);
|
||||
await expect(fs.readFile(path.join(targetDir, "marker.txt"), "utf8")).resolves.toBe("new");
|
||||
await expect(
|
||||
listMatchingDirs(installBaseDir, ".openclaw-install-stage-"),
|
||||
).resolves.toHaveLength(0);
|
||||
});
|
||||
|
||||
it("aborts without outside writes when the install base is rebound before publish", async () => {
|
||||
await fixtureRootTracker.setup();
|
||||
const fixtureRoot = await fixtureRootTracker.make("case");
|
||||
@@ -255,25 +302,36 @@ describe("installPackageDir", () => {
|
||||
await createReboundInstallFixture({ fixtureRoot, withExistingInstall: true });
|
||||
|
||||
const warnings: string[] = [];
|
||||
const result = await withInstallBaseReboundOnRealpathCall({
|
||||
installBaseDir,
|
||||
preservedDir: preservedInstallRoot,
|
||||
outsideTarget: outsideInstallRoot,
|
||||
rebindAtCall: 8,
|
||||
run: async () =>
|
||||
await installPackageDir({
|
||||
sourceDir,
|
||||
targetDir,
|
||||
mode: "update",
|
||||
timeoutMs: 1_000,
|
||||
copyErrorPrefix: "failed to copy plugin",
|
||||
hasDeps: false,
|
||||
depsLogMessage: "Installing deps…",
|
||||
logger: { warn: (message) => warnings.push(message) },
|
||||
}),
|
||||
const installBasePath = normalizeComparablePath(installBaseDir);
|
||||
const realStat = fs.stat.bind(fs);
|
||||
let installBaseStatCalls = 0;
|
||||
vi.spyOn(fs, "stat").mockImplementation(async (...args: Parameters<typeof fs.stat>) => {
|
||||
if (normalizeComparablePath(String(args[0])) === installBasePath) {
|
||||
installBaseStatCalls += 1;
|
||||
if (installBaseStatCalls === 3) {
|
||||
await rebindInstallBasePath({
|
||||
installBaseDir,
|
||||
preservedDir: preservedInstallRoot,
|
||||
outsideTarget: outsideInstallRoot,
|
||||
});
|
||||
}
|
||||
}
|
||||
return await realStat(...args);
|
||||
});
|
||||
|
||||
const result = await installPackageDir({
|
||||
sourceDir,
|
||||
targetDir,
|
||||
mode: "update",
|
||||
timeoutMs: 1_000,
|
||||
copyErrorPrefix: "failed to copy plugin",
|
||||
hasDeps: false,
|
||||
depsLogMessage: "Installing deps…",
|
||||
logger: { warn: (message) => warnings.push(message) },
|
||||
});
|
||||
|
||||
expect(result).toEqual({ ok: true });
|
||||
expect(installBaseStatCalls).toBe(3);
|
||||
expect(warnings).toContain(
|
||||
"Install base directory changed before backup cleanup; leaving backup in place.",
|
||||
);
|
||||
|
||||
@@ -4,6 +4,7 @@ import { runCommandWithTimeout } from "../process/exec.js";
|
||||
import { pathExists } from "./fs-safe.js";
|
||||
import { assertCanonicalPathWithinBase } from "./install-safe-path.js";
|
||||
import { tryReadJson, writeJson } from "./json-files.js";
|
||||
import { movePathWithCopyFallback } from "./replace-file.js";
|
||||
import { createSafeNpmInstallArgs, createSafeNpmInstallEnv } from "./safe-package-install.js";
|
||||
|
||||
const INSTALL_BASE_CHANGED_ERROR_MESSAGE = "install base directory changed during install";
|
||||
@@ -210,7 +211,11 @@ export async function installPackageDir(params: {
|
||||
if (!backupDir) {
|
||||
return;
|
||||
}
|
||||
await fs.rename(backupDir, canonicalTargetDir).catch(() => undefined);
|
||||
await movePathWithCopyFallback({
|
||||
from: backupDir,
|
||||
sourceHardlinks: "reject",
|
||||
to: canonicalTargetDir,
|
||||
}).catch(() => undefined);
|
||||
backupDir = null;
|
||||
};
|
||||
|
||||
@@ -293,7 +298,11 @@ export async function installPackageDir(params: {
|
||||
installBaseDir,
|
||||
expectedRealPath: installBaseRealPath,
|
||||
});
|
||||
await fs.rename(canonicalTargetDir, backupDir);
|
||||
await movePathWithCopyFallback({
|
||||
from: canonicalTargetDir,
|
||||
sourceHardlinks: "reject",
|
||||
to: backupDir,
|
||||
});
|
||||
} catch (err) {
|
||||
return await fail(`${params.copyErrorPrefix}: ${String(err)}`, err);
|
||||
}
|
||||
@@ -304,7 +313,11 @@ export async function installPackageDir(params: {
|
||||
installBaseDir,
|
||||
expectedRealPath: installBaseRealPath,
|
||||
});
|
||||
await fs.rename(stageDir, canonicalTargetDir);
|
||||
await movePathWithCopyFallback({
|
||||
from: stageDir,
|
||||
sourceHardlinks: "reject",
|
||||
to: canonicalTargetDir,
|
||||
});
|
||||
stageDir = null;
|
||||
} catch (err) {
|
||||
return await fail(`${params.copyErrorPrefix}: ${String(err)}`, err);
|
||||
|
||||
@@ -29,6 +29,10 @@ function createNpmTarget(globalRoot: string): ResolvedGlobalInstallTarget {
|
||||
};
|
||||
}
|
||||
|
||||
function createFsError(code: string, message = code): NodeJS.ErrnoException {
|
||||
return Object.assign(new Error(message), { code });
|
||||
}
|
||||
|
||||
function createPnpmTarget(globalRoot: string): ResolvedGlobalInstallTarget {
|
||||
return {
|
||||
manager: "pnpm",
|
||||
@@ -124,6 +128,71 @@ describe("runGlobalPackageUpdateSteps", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("swaps staged npm package roots through the copy fallback when rename crosses devices", async () => {
|
||||
await withTempDir({ prefix: "openclaw-package-update-exdev-" }, async (base) => {
|
||||
const prefix = path.join(base, "prefix");
|
||||
const globalRoot = path.join(prefix, "lib", "node_modules");
|
||||
const packageRoot = path.join(globalRoot, "openclaw");
|
||||
|
||||
const realRename = fs.rename.bind(fs);
|
||||
let exdevMoves = 0;
|
||||
const renameSpy = vi
|
||||
.spyOn(fs, "rename")
|
||||
.mockImplementation(async (...args: Parameters<typeof fs.rename>) => {
|
||||
const [from, to] = args;
|
||||
const fromPath = String(from);
|
||||
if (
|
||||
exdevMoves === 0 &&
|
||||
fromPath.includes(`${path.sep}.openclaw-update-stage-`) &&
|
||||
path.basename(fromPath) === "openclaw" &&
|
||||
String(to) === packageRoot
|
||||
) {
|
||||
exdevMoves += 1;
|
||||
throw createFsError("EXDEV", "cross-device link not permitted");
|
||||
}
|
||||
return await realRename(...args);
|
||||
});
|
||||
|
||||
try {
|
||||
const result = await runGlobalPackageUpdateSteps({
|
||||
installTarget: createNpmTarget(globalRoot),
|
||||
installSpec: "openclaw@2.0.0",
|
||||
packageName: "openclaw",
|
||||
packageRoot,
|
||||
runCommand: createRootRunner(globalRoot),
|
||||
runStep: async ({ name, argv, cwd }) => {
|
||||
const prefixIndex = argv.indexOf("--prefix");
|
||||
const stagePrefix = argv[prefixIndex + 1];
|
||||
if (!stagePrefix) {
|
||||
throw new Error("missing staged prefix");
|
||||
}
|
||||
await writePackageRoot(
|
||||
path.join(stagePrefix, "lib", "node_modules", "openclaw"),
|
||||
"2.0.0",
|
||||
);
|
||||
return {
|
||||
name,
|
||||
command: argv.join(" "),
|
||||
cwd: cwd ?? process.cwd(),
|
||||
durationMs: 1,
|
||||
exitCode: 0,
|
||||
};
|
||||
},
|
||||
timeoutMs: 1000,
|
||||
});
|
||||
|
||||
expect(result.failedStep).toBeNull();
|
||||
expect(result.afterVersion).toBe("2.0.0");
|
||||
expect(exdevMoves).toBe(1);
|
||||
await expect(
|
||||
fs.readFile(path.join(packageRoot, "package.json"), "utf8"),
|
||||
).resolves.toContain('"version":"2.0.0"');
|
||||
} finally {
|
||||
renameSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it("stages pnpm-detected updates through npm when the global root has npm prefix layout", async () => {
|
||||
await withTempDir({ prefix: "openclaw-package-update-pnpm-staged-" }, async (base) => {
|
||||
const prefix = path.join(base, "prefix");
|
||||
|
||||
@@ -2,6 +2,7 @@ import fs from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { pathExists } from "./fs-safe.js";
|
||||
import { readPackageVersion } from "./package-json.js";
|
||||
import { movePathWithCopyFallback } from "./replace-file.js";
|
||||
import {
|
||||
collectInstalledGlobalPackageErrors,
|
||||
globalInstallArgs,
|
||||
@@ -283,10 +284,18 @@ async function swapStagedNpmInstall(params: {
|
||||
try {
|
||||
await fs.mkdir(targetLayout.globalRoot, { recursive: true });
|
||||
if (await pathExists(targetPackageRoot)) {
|
||||
await fs.rename(targetPackageRoot, backupRoot);
|
||||
await movePathWithCopyFallback({
|
||||
from: targetPackageRoot,
|
||||
sourceHardlinks: "reject",
|
||||
to: backupRoot,
|
||||
});
|
||||
movedExisting = true;
|
||||
}
|
||||
await fs.rename(params.stage.packageRoot, targetPackageRoot);
|
||||
await movePathWithCopyFallback({
|
||||
from: params.stage.packageRoot,
|
||||
sourceHardlinks: "reject",
|
||||
to: targetPackageRoot,
|
||||
});
|
||||
movedStaged = true;
|
||||
await replaceNpmBinShims({
|
||||
stageLayout: params.stage.layout,
|
||||
@@ -312,7 +321,11 @@ async function swapStagedNpmInstall(params: {
|
||||
await removePathBestEffort(targetPackageRoot);
|
||||
}
|
||||
if (movedExisting) {
|
||||
await fs.rename(backupRoot, targetPackageRoot).catch(() => undefined);
|
||||
await movePathWithCopyFallback({
|
||||
from: backupRoot,
|
||||
sourceHardlinks: "reject",
|
||||
to: targetPackageRoot,
|
||||
}).catch(() => undefined);
|
||||
}
|
||||
return {
|
||||
name: "global install swap",
|
||||
|
||||
Reference in New Issue
Block a user