fix: restore npm shims on swap failure

This commit is contained in:
Shakker
2026-04-27 14:41:38 +01:00
parent 2186080963
commit ca444af891
3 changed files with 139 additions and 15 deletions

View File

@@ -159,6 +159,8 @@ describe("runGlobalPackageUpdateSteps", () => {
expect(result.steps.at(-1)?.stderrTail).toContain(
"expected installed version 2.0.0, found 1.5.0",
);
expect(result.verifiedPackageRoot).toBe(packageRoot);
expect(result.afterVersion).toBe("1.0.0");
expect(postVerifyStep).not.toHaveBeenCalled();
await expect(fs.readFile(path.join(packageRoot, "package.json"), "utf8")).resolves.toContain(
'"version":"1.0.0"',
@@ -166,6 +168,60 @@ describe("runGlobalPackageUpdateSteps", () => {
});
});
it.runIf(process.platform !== "win32")(
"restores the existing bin shim when staged shim replacement fails",
async () => {
await withTempDir({ prefix: "openclaw-package-update-shim-rollback-" }, async (base) => {
const prefix = path.join(base, "prefix");
const globalRoot = path.join(prefix, "lib", "node_modules");
const packageRoot = path.join(globalRoot, "openclaw");
const targetShim = path.join(prefix, "bin", "openclaw");
await writePackageRoot(packageRoot, "1.0.0");
await fs.mkdir(path.dirname(targetShim), { recursive: true });
await fs.writeFile(targetShim, "old shim\n", "utf8");
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",
);
const stagedShim = path.join(stagePrefix, "bin", "openclaw");
await fs.mkdir(path.dirname(stagedShim), { recursive: true });
await fs.writeFile(stagedShim, "new shim\n", "utf8");
await fs.chmod(stagedShim, 0);
return {
name,
command: argv.join(" "),
cwd: cwd ?? process.cwd(),
durationMs: 1,
exitCode: 0,
};
},
timeoutMs: 1000,
});
expect(result.failedStep?.name).toBe("global install swap");
expect(result.verifiedPackageRoot).toBe(packageRoot);
expect(result.afterVersion).toBe("1.0.0");
await expect(
fs.readFile(path.join(packageRoot, "package.json"), "utf8"),
).resolves.toContain('"version":"1.0.0"');
await expect(fs.readFile(targetShim, "utf8")).resolves.toBe("old shim\n");
});
},
);
it("cleans the staged npm prefix when the install command throws", async () => {
await withTempDir({ prefix: "openclaw-package-update-cleanup-" }, async (base) => {
const prefix = path.join(base, "prefix");

View File

@@ -38,6 +38,15 @@ type StagedNpmInstall = {
packageRoot: string;
};
type NpmBinShimBackup = {
backupDir: string;
targetBinDir: string;
entries: Array<{
name: string;
hadExisting: boolean;
}>;
};
function formatError(err: unknown): string {
return err instanceof Error ? err.message : String(err);
}
@@ -51,6 +60,17 @@ async function pathExists(targetPath: string): Promise<boolean> {
}
}
async function readPackageVersionIfPresent(packageRoot: string | null): Promise<string | null> {
if (!packageRoot) {
return null;
}
try {
return await readPackageVersion(packageRoot);
} catch {
return null;
}
}
async function createStagedNpmInstall(
installTarget: ResolvedGlobalInstallTarget,
packageName: string,
@@ -152,12 +172,47 @@ async function replaceNpmBinShims(params: {
return;
}
await fs.mkdir(params.targetLayout.binDir, { recursive: true });
for (const entry of shimEntries) {
await copyPathEntry(
path.join(params.stageLayout.binDir, entry),
path.join(params.targetLayout.binDir, entry),
);
const backup: NpmBinShimBackup = {
backupDir: await fs.mkdtemp(
path.join(params.targetLayout.globalRoot, ".openclaw-shim-backup-"),
),
targetBinDir: params.targetLayout.binDir,
entries: [],
};
try {
await fs.mkdir(params.targetLayout.binDir, { recursive: true });
for (const entry of shimEntries) {
const destination = path.join(params.targetLayout.binDir, entry);
const hadExisting = await pathExists(destination);
backup.entries.push({ name: entry, hadExisting });
if (hadExisting) {
await copyPathEntry(destination, path.join(backup.backupDir, entry));
}
}
for (const entry of shimEntries) {
await copyPathEntry(
path.join(params.stageLayout.binDir, entry),
path.join(params.targetLayout.binDir, entry),
);
}
} catch (err) {
await restoreNpmBinShimBackup(backup);
throw err;
} finally {
await fs.rm(backup.backupDir, { recursive: true, force: true }).catch(() => undefined);
}
}
async function restoreNpmBinShimBackup(backup: NpmBinShimBackup): Promise<void> {
await fs.mkdir(backup.targetBinDir, { recursive: true });
for (const entry of backup.entries) {
const destination = path.join(backup.targetBinDir, entry.name);
await fs.rm(destination, { recursive: true, force: true }).catch(() => undefined);
if (entry.hadExisting) {
await copyPathEntry(path.join(backup.backupDir, entry.name), destination);
}
}
}
@@ -318,8 +373,9 @@ export async function runGlobalPackageUpdateSteps(params: {
}
}
let verifiedPackageRoot =
stagedInstall?.packageRoot ??
const livePackageRoot =
params.installTarget.packageRoot ??
params.packageRoot ??
(
await resolveGlobalInstallTarget({
manager: params.installTarget,
@@ -327,25 +383,29 @@ export async function runGlobalPackageUpdateSteps(params: {
timeoutMs: params.timeoutMs,
})
).packageRoot ??
params.packageRoot ??
null;
const verificationPackageRoot = stagedInstall?.packageRoot ?? livePackageRoot;
let verifiedPackageRoot = livePackageRoot ?? verificationPackageRoot;
let afterVersion: string | null = null;
if (finalInstallStep.exitCode === 0 && verifiedPackageRoot) {
afterVersion = await readPackageVersion(verifiedPackageRoot);
if (finalInstallStep.exitCode === 0 && verificationPackageRoot) {
const candidateVersion = await readPackageVersion(verificationPackageRoot);
if (!stagedInstall) {
afterVersion = candidateVersion;
}
const expectedVersion = resolveExpectedInstalledVersionFromSpec(
params.packageName,
params.installSpec,
);
const verificationErrors = await collectInstalledGlobalPackageErrors({
packageRoot: verifiedPackageRoot,
packageRoot: verificationPackageRoot,
expectedVersion,
});
if (verificationErrors.length > 0) {
steps.push({
name: "global install verify",
command: `verify ${verifiedPackageRoot}`,
cwd: verifiedPackageRoot,
command: `verify ${verificationPackageRoot}`,
cwd: verificationPackageRoot,
durationMs: 0,
exitCode: 1,
stderrTail: verificationErrors.join("\n"),
@@ -362,6 +422,7 @@ export async function runGlobalPackageUpdateSteps(params: {
steps.push(swapStep);
if (swapStep.exitCode === 0) {
verifiedPackageRoot = params.installTarget.packageRoot ?? verifiedPackageRoot;
afterVersion = candidateVersion;
}
}
@@ -372,10 +433,15 @@ export async function runGlobalPackageUpdateSteps(params: {
);
const postVerifyStep = failedVerifyOrSwap
? null
: await params.postVerifyStep?.(verifiedPackageRoot);
: verifiedPackageRoot
? await params.postVerifyStep?.(verifiedPackageRoot)
: null;
if (postVerifyStep) {
steps.push(postVerifyStep);
}
if (failedVerifyOrSwap && stagedInstall) {
afterVersion = await readPackageVersionIfPresent(livePackageRoot);
}
}
const failedStep =

View File

@@ -1647,6 +1647,8 @@ describe("runGatewayUpdate", () => {
expect(result.status).toBe("error");
expect(result.reason).toBe("global-install-failed");
expect(result.root).toBe(pkgRoot);
expect(result.after?.version).toBe("1.0.0");
expect(result.steps.at(-1)?.name).toBe("global install swap");
await expect(fs.readFile(path.join(pkgRoot, "package.json"), "utf-8")).resolves.toContain(
'"version":"1.0.0"',