fix: harden npm update staging

2026-05-06 11:30:43 +00:00 · 2026-04-27 14:24:22 +01:00
parent 6985c6751c
commit b0127b9f1f
5 changed files with 267 additions and 113 deletions
--- a/src/cli/update-cli/progress.ts
+++ b/src/cli/update-cli/progress.ts
@@ -30,6 +30,9 @@ const STEP_LABELS: Record<string, string> = {
  "git rev-parse HEAD (after)": "Verifying update",
  "global update": "Updating via package manager",
  "global update (omit optional)": "Retrying update without optional deps",
+  "global install stage": "Preparing staged package install",
+  "global install verify": "Verifying global package",
+  "global install swap": "Activating global package",
  "global install": "Installing global package",
 };

--- a/src/infra/package-update-steps.test.ts
+++ b/src/infra/package-update-steps.test.ts
@@ -164,4 +164,33 @@ describe("runGlobalPackageUpdateSteps", () => {
      );
    });
  });
+
+  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");
+      const globalRoot = path.join(prefix, "lib", "node_modules");
+      const packageRoot = path.join(globalRoot, "openclaw");
+      await writePackageRoot(packageRoot, "1.0.0");
+
+      let stagePrefix: string | undefined;
+      await expect(
+        runGlobalPackageUpdateSteps({
+          installTarget: createNpmTarget(globalRoot),
+          installSpec: "openclaw@2.0.0",
+          packageName: "openclaw",
+          packageRoot,
+          runCommand: createRootRunner(globalRoot),
+          runStep: async ({ argv }) => {
+            const prefixIndex = argv.indexOf("--prefix");
+            stagePrefix = argv[prefixIndex + 1];
+            throw new Error("install crashed");
+          },
+          timeoutMs: 1000,
+        }),
+      ).rejects.toThrow("install crashed");
+
+      expect(stagePrefix).toBeDefined();
+      await expect(fs.access(stagePrefix ?? "")).rejects.toMatchObject({ code: "ENOENT" });
+    });
+  });
 });
--- a/src/infra/package-update-steps.ts
+++ b/src/infra/package-update-steps.ts
@@ -71,6 +71,39 @@ async function createStagedNpmInstall(
  };
 }

+async function prepareStagedNpmInstall(
+  installTarget: ResolvedGlobalInstallTarget,
+  packageName: string,
+): Promise<{
+  stagedInstall: StagedNpmInstall | null;
+  failedStep: PackageUpdateStepResult | null;
+}> {
+  const startedAt = Date.now();
+  try {
+    return {
+      stagedInstall: await createStagedNpmInstall(installTarget, packageName),
+      failedStep: null,
+    };
+  } catch (err) {
+    const targetLayout =
+      installTarget.manager === "npm"
+        ? resolveNpmGlobalPrefixLayoutFromGlobalRoot(installTarget.globalRoot)
+        : null;
+    return {
+      stagedInstall: null,
+      failedStep: {
+        name: "global install stage",
+        command: "prepare staged npm install",
+        cwd: targetLayout?.prefix ?? installTarget.globalRoot ?? process.cwd(),
+        durationMs: Date.now() - startedAt,
+        exitCode: 1,
+        stdoutTail: null,
+        stderrTail: formatError(err),
+      },
+    };
+  }
+}
+
 async function cleanupStagedNpmInstall(stage: StagedNpmInstall | null): Promise<void> {
  if (!stage) {
    return;
@@ -215,118 +248,147 @@ export async function runGlobalPackageUpdateSteps(params: {
 }> {
  const installCwd = params.installCwd === undefined ? {} : { cwd: params.installCwd };
  const installEnv = params.env === undefined ? {} : { env: params.env };
-  let stagedInstall = await createStagedNpmInstall(params.installTarget, params.packageName);
-  const updateStep = await params.runStep({
-    name: "global update",
-    argv: globalInstallArgs(
-      params.installTarget,
-      params.installSpec,
-      undefined,
-      stagedInstall?.prefix,
-    ),
-    ...installCwd,
-    ...installEnv,
-    timeoutMs: params.timeoutMs,
-  });
+  let stagedInstall: StagedNpmInstall | null = null;

-  const steps = [updateStep];
-  let finalInstallStep = updateStep;
-  if (updateStep.exitCode !== 0) {
-    await cleanupStagedNpmInstall(stagedInstall);
-    stagedInstall = await createStagedNpmInstall(params.installTarget, params.packageName);
-    const fallbackArgv = globalInstallFallbackArgs(
-      params.installTarget,
-      params.installSpec,
-      undefined,
-      stagedInstall?.prefix,
-    );
-    if (fallbackArgv) {
-      const fallbackStep = await params.runStep({
-        name: "global update (omit optional)",
-        argv: fallbackArgv,
-        ...installCwd,
-        ...installEnv,
-        timeoutMs: params.timeoutMs,
-      });
-      steps.push(fallbackStep);
-      finalInstallStep = fallbackStep;
-    } else {
+  try {
+    const preparedInstall = await prepareStagedNpmInstall(params.installTarget, params.packageName);
+    stagedInstall = preparedInstall.stagedInstall;
+    if (preparedInstall.failedStep) {
+      return {
+        steps: [preparedInstall.failedStep],
+        verifiedPackageRoot: params.packageRoot ?? null,
+        afterVersion: null,
+        failedStep: preparedInstall.failedStep,
+      };
+    }
+
+    const updateStep = await params.runStep({
+      name: "global update",
+      argv: globalInstallArgs(
+        params.installTarget,
+        params.installSpec,
+        undefined,
+        stagedInstall?.prefix,
+      ),
+      ...installCwd,
+      ...installEnv,
+      timeoutMs: params.timeoutMs,
+    });
+
+    const steps = [updateStep];
+    let finalInstallStep = updateStep;
+    if (updateStep.exitCode !== 0) {
      await cleanupStagedNpmInstall(stagedInstall);
      stagedInstall = null;
-    }
-  }
+      const preparedFallbackInstall = await prepareStagedNpmInstall(
+        params.installTarget,
+        params.packageName,
+      );
+      stagedInstall = preparedFallbackInstall.stagedInstall;
+      if (preparedFallbackInstall.failedStep) {
+        steps.push(preparedFallbackInstall.failedStep);
+        return {
+          steps,
+          verifiedPackageRoot: params.packageRoot ?? null,
+          afterVersion: null,
+          failedStep: preparedFallbackInstall.failedStep,
+        };
+      }

-  let verifiedPackageRoot =
-    stagedInstall?.packageRoot ??
-    (
-      await resolveGlobalInstallTarget({
-        manager: params.installTarget,
-        runCommand: params.runCommand,
-        timeoutMs: params.timeoutMs,
-      })
-    ).packageRoot ??
-    params.packageRoot ??
-    null;
-
-  let afterVersion: string | null = null;
-  if (finalInstallStep.exitCode === 0 && verifiedPackageRoot) {
-    afterVersion = await readPackageVersion(verifiedPackageRoot);
-    const expectedVersion = resolveExpectedInstalledVersionFromSpec(
-      params.packageName,
-      params.installSpec,
-    );
-    const verificationErrors = await collectInstalledGlobalPackageErrors({
-      packageRoot: verifiedPackageRoot,
-      expectedVersion,
-    });
-    if (verificationErrors.length > 0) {
-      steps.push({
-        name: "global install verify",
-        command: `verify ${verifiedPackageRoot}`,
-        cwd: verifiedPackageRoot,
-        durationMs: 0,
-        exitCode: 1,
-        stderrTail: verificationErrors.join("\n"),
-        stdoutTail: null,
-      });
-    }
-
-    if (stagedInstall && verificationErrors.length === 0) {
-      const swapStep = await swapStagedNpmInstall({
-        stage: stagedInstall,
-        installTarget: params.installTarget,
-        packageName: params.packageName,
-      });
-      steps.push(swapStep);
-      if (swapStep.exitCode === 0) {
-        verifiedPackageRoot = params.installTarget.packageRoot ?? verifiedPackageRoot;
+      const fallbackArgv = globalInstallFallbackArgs(
+        params.installTarget,
+        params.installSpec,
+        undefined,
+        stagedInstall?.prefix,
+      );
+      if (fallbackArgv) {
+        const fallbackStep = await params.runStep({
+          name: "global update (omit optional)",
+          argv: fallbackArgv,
+          ...installCwd,
+          ...installEnv,
+          timeoutMs: params.timeoutMs,
+        });
+        steps.push(fallbackStep);
+        finalInstallStep = fallbackStep;
+      } else {
+        await cleanupStagedNpmInstall(stagedInstall);
+        stagedInstall = null;
      }
    }

-    const failedVerifyOrSwap = steps.find(
-      (step) =>
-        (step.name === "global install verify" || step.name === "global install swap") &&
-        step.exitCode !== 0,
-    );
-    const postVerifyStep = failedVerifyOrSwap
-      ? null
-      : await params.postVerifyStep?.(verifiedPackageRoot);
-    if (postVerifyStep) {
-      steps.push(postVerifyStep);
+    let verifiedPackageRoot =
+      stagedInstall?.packageRoot ??
+      (
+        await resolveGlobalInstallTarget({
+          manager: params.installTarget,
+          runCommand: params.runCommand,
+          timeoutMs: params.timeoutMs,
+        })
+      ).packageRoot ??
+      params.packageRoot ??
+      null;
+
+    let afterVersion: string | null = null;
+    if (finalInstallStep.exitCode === 0 && verifiedPackageRoot) {
+      afterVersion = await readPackageVersion(verifiedPackageRoot);
+      const expectedVersion = resolveExpectedInstalledVersionFromSpec(
+        params.packageName,
+        params.installSpec,
+      );
+      const verificationErrors = await collectInstalledGlobalPackageErrors({
+        packageRoot: verifiedPackageRoot,
+        expectedVersion,
+      });
+      if (verificationErrors.length > 0) {
+        steps.push({
+          name: "global install verify",
+          command: `verify ${verifiedPackageRoot}`,
+          cwd: verifiedPackageRoot,
+          durationMs: 0,
+          exitCode: 1,
+          stderrTail: verificationErrors.join("\n"),
+          stdoutTail: null,
+        });
+      }
+
+      if (stagedInstall && verificationErrors.length === 0) {
+        const swapStep = await swapStagedNpmInstall({
+          stage: stagedInstall,
+          installTarget: params.installTarget,
+          packageName: params.packageName,
+        });
+        steps.push(swapStep);
+        if (swapStep.exitCode === 0) {
+          verifiedPackageRoot = params.installTarget.packageRoot ?? verifiedPackageRoot;
+        }
+      }
+
+      const failedVerifyOrSwap = steps.find(
+        (step) =>
+          (step.name === "global install verify" || step.name === "global install swap") &&
+          step.exitCode !== 0,
+      );
+      const postVerifyStep = failedVerifyOrSwap
+        ? null
+        : await params.postVerifyStep?.(verifiedPackageRoot);
+      if (postVerifyStep) {
+        steps.push(postVerifyStep);
+      }
    }
+
+    const failedStep =
+      finalInstallStep.exitCode !== 0
+        ? finalInstallStep
+        : (steps.find((step) => step !== updateStep && step.exitCode !== 0) ?? null);
+
+    return {
+      steps,
+      verifiedPackageRoot,
+      afterVersion,
+      failedStep,
+    };
+  } finally {
+    await cleanupStagedNpmInstall(stagedInstall);
  }
-
-  await cleanupStagedNpmInstall(stagedInstall);
-
-  const failedStep =
-    finalInstallStep.exitCode !== 0
-      ? finalInstallStep
-      : (steps.find((step) => step !== updateStep && step.exitCode !== 0) ?? null);
-
-  return {
-    steps,
-    verifiedPackageRoot,
-    afterVersion,
-    failedStep,
-  };
 }
--- a/src/infra/update-runner.test.ts
+++ b/src/infra/update-runner.test.ts
@@ -247,6 +247,7 @@ describe("runGatewayUpdate", () => {
  }

  async function writeGlobalPackageVersion(pkgRoot: string, version = "2.0.0") {
+    await fs.mkdir(pkgRoot, { recursive: true });
    await fs.writeFile(
      path.join(pkgRoot, "package.json"),
      JSON.stringify({ name: "openclaw", version }),
@@ -1358,7 +1359,11 @@ describe("runGatewayUpdate", () => {
    npmRootOutput?: string;
    installCommand: string;
    gitRootMode?: "not-git" | "missing";
-    onInstall?: (options?: { env?: NodeJS.ProcessEnv }) => Promise<void>;
+    onInstall?: (options?: {
+      env?: NodeJS.ProcessEnv;
+      installPrefix?: string;
+      packageRoot?: string;
+    }) => Promise<void>;
  }) => {
    const calls: string[] = [];
    const runCommand = async (argv: string[], options?: { env?: NodeJS.ProcessEnv }) => {
@@ -1383,6 +1388,26 @@ describe("runGatewayUpdate", () => {
        await params.onInstall?.(options);
        return { stdout: "ok", stderr: "", code: 0 };
      }
+      const prefixIndex = argv.indexOf("--prefix");
+      const installPrefix = prefixIndex >= 0 ? argv[prefixIndex + 1] : undefined;
+      if (installPrefix) {
+        const normalizedInstallCommand = [
+          ...argv.slice(0, prefixIndex),
+          ...argv.slice(prefixIndex + 2),
+        ].join(" ");
+        if (normalizedInstallCommand === params.installCommand) {
+          const packageRoot =
+            process.platform === "win32"
+              ? path.join(installPrefix, "node_modules", "openclaw")
+              : path.join(installPrefix, "lib", "node_modules", "openclaw");
+          await params.onInstall?.({
+            ...options,
+            installPrefix,
+            packageRoot,
+          });
+          return { stdout: "ok", stderr: "", code: 0 };
+        }
+      }
      return { stdout: "", stderr: "", code: 0 };
    };
    return { calls, runCommand };
@@ -1575,16 +1600,18 @@ describe("runGatewayUpdate", () => {
      installCommand: "npm i -g openclaw@latest --no-fund --no-audit --loglevel=error",
      onInstall: async (options) => {
        installEnv = options?.env;
-        await writeGlobalPackageVersion(pkgRoot);
+        await writeGlobalPackageVersion(options?.packageRoot ?? pkgRoot);
      },
    });

-    await withEnvAsync({ LOCALAPPDATA: localAppData }, async () => {
-      const result = await runWithCommand(runCommand, { cwd: pkgRoot });
-      expect(result.status).toBe("ok");
-    });
-
-    platformSpy.mockRestore();
+    try {
+      await withEnvAsync({ LOCALAPPDATA: localAppData }, async () => {
+        const result = await runWithCommand(runCommand, { cwd: pkgRoot });
+        expect(result.status).toBe("ok");
+      });
+    } finally {
+      platformSpy.mockRestore();
+    }

    const mergedPath = installEnv?.Path ?? installEnv?.PATH ?? "";
    expect(mergedPath.split(path.delimiter).slice(0, 2)).toEqual([
@@ -1595,6 +1622,37 @@ describe("runGatewayUpdate", () => {
    expect(installEnv?.NODE_LLAMA_CPP_SKIP_DOWNLOAD).toBe("1");
  });

+  it("reports staged npm swap failures as global install failures", async () => {
+    const prefix = path.join(tempDir, "npm-prefix");
+    const nodeModules = path.join(prefix, "lib", "node_modules");
+    const pkgRoot = path.join(nodeModules, "openclaw");
+    await seedGlobalPackageRoot(pkgRoot);
+    await fs.writeFile(path.join(prefix, "bin"), "not a directory", "utf-8");
+
+    const { runCommand } = createGlobalInstallHarness({
+      pkgRoot,
+      npmRootOutput: nodeModules,
+      installCommand: "npm i -g openclaw@latest --no-fund --no-audit --loglevel=error",
+      onInstall: async (options) => {
+        await writeGlobalPackageVersion(options?.packageRoot ?? pkgRoot);
+        if (options?.installPrefix) {
+          const binDir = path.join(options.installPrefix, "bin");
+          await fs.mkdir(binDir, { recursive: true });
+          await fs.writeFile(path.join(binDir, "openclaw"), "#!/bin/sh\n", "utf-8");
+        }
+      },
+    });
+
+    const result = await runWithCommand(runCommand, { cwd: pkgRoot });
+
+    expect(result.status).toBe("error");
+    expect(result.reason).toBe("global-install-failed");
+    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"',
+    );
+  });
+
  it("uses OPENCLAW_UPDATE_PACKAGE_SPEC for global package updates", async () => {
    const { nodeModules, pkgRoot } = await createGlobalPackageFixture(tempDir);
    const expectedInstallCommand =
--- a/src/infra/update-runner.ts
+++ b/src/infra/update-runner.ts
@@ -565,7 +565,9 @@ function normalizeFallbackFailureReason(stepName: string): NonNullable<UpdateRun
  switch (stepName) {
    case "global update":
    case "global update (omit optional)":
+    case "global install stage":
    case "global install verify":
+    case "global install swap":
      return "global-install-failed";
    case "openclaw doctor":
      return "doctor-failed";