fix(release): harden package update validation

steipete · steipete · commit 781c9b7ab08a · 2026-05-03T03:37:52.000+01:00
diff --git a/scripts/e2e/lib/kitchen-sink-plugin/assertions.mjs b/scripts/e2e/lib/kitchen-sink-plugin/assertions.mjs
@@ -142,7 +142,6 @@ function assertExpectedDiagnostics(surfaceMode, errorMessages) {
     "cli registration missing explicit commands metadata",
     "only bundled plugins can register Codex app-server extension factories",
     "only bundled plugins can register agent tool result middleware",
-    "agent event subscription registration requires id and handle",
     'compaction provider "kitchen-sink-compaction-provider" registration missing summarize',
     "context engine registration missing id",
     "control UI descriptor registration requires id, surface, label, and valid optional fields",
@@ -158,6 +157,10 @@ function assertExpectedDiagnostics(surfaceMode, errorMessages) {
     "session scheduler job registration requires unique id, sessionKey, and kind",
     "tool metadata registration missing toolName",
   ]);
+  const optionalErrorMessages = new Set([
+    "agent event subscription registration requires id and handle",
+  ]);
+  const allowedErrorMessages = new Set([...expectedErrorMessages, ...optionalErrorMessages]);
   if (!INVALID_PROBE_DIAGNOSTIC_SURFACE_MODES.has(surfaceMode)) {
     if (errorMessages.size > 0) {
       throw new Error(
@@ -167,7 +170,7 @@ function assertExpectedDiagnostics(surfaceMode, errorMessages) {
     return;
   }
   for (const message of errorMessages) {
-    if (!expectedErrorMessages.has(message)) {
+    if (!allowedErrorMessages.has(message)) {
       throw new Error(`unexpected kitchen-sink diagnostic error: ${message}`);
     }
   }
diff --git a/src/infra/package-update-steps.test.ts b/src/infra/package-update-steps.test.ts
@@ -115,6 +115,67 @@ describe("runGlobalPackageUpdateSteps", () => {
     });
   });
 
+  it("keeps a successful staged swap when old package cleanup hits a transient Windows native module error", async () => {
+    await withTempDir({ prefix: "openclaw-package-update-staged-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");
+
+      const realRm = fs.rm;
+      const rmSpy = vi.spyOn(fs, "rm").mockImplementation(async (target, options) => {
+        const targetPath = String(target);
+        if (
+          targetPath.includes(`${path.sep}.openclaw-`) &&
+          !targetPath.includes(".openclaw-update-stage-") &&
+          !targetPath.includes(".openclaw-shim-backup-")
+        ) {
+          throw Object.assign(new Error("EPERM: operation not permitted, unlink native.node"), {
+            code: "EPERM",
+          });
+        }
+        return realRm(target, options);
+      });
+
+      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");
+        await expect(
+          fs.readFile(path.join(packageRoot, "package.json"), "utf8"),
+        ).resolves.toContain('"version":"2.0.0"');
+      } finally {
+        rmSpy.mockRestore();
+      }
+    });
+  });
+
   it("does not run post-verify work when staged npm verification fails", async () => {
     await withTempDir({ prefix: "openclaw-package-update-verify-" }, async (base) => {
       const prefix = path.join(base, "prefix");
diff --git a/src/infra/package-update-steps.ts b/src/infra/package-update-steps.ts
@@ -60,6 +60,17 @@ async function pathExists(targetPath: string): Promise<boolean> {
   }
 }
 
+async function removePathBestEffort(targetPath: string): Promise<void> {
+  await fs
+    .rm(targetPath, {
+      recursive: true,
+      force: true,
+      maxRetries: process.platform === "win32" ? 5 : 2,
+      retryDelay: 100,
+    })
+    .catch(() => undefined);
+}
+
 async function readPackageVersionIfPresent(packageRoot: string | null): Promise<string | null> {
   if (!packageRoot) {
     return null;
@@ -129,12 +140,12 @@ async function cleanupStagedNpmInstall(stage: StagedNpmInstall | null): Promise<
   if (!stage) {
     return;
   }
-  await fs.rm(stage.prefix, { recursive: true, force: true }).catch(() => undefined);
+  await removePathBestEffort(stage.prefix);
 }
 
 async function copyPathEntry(source: string, destination: string): Promise<void> {
   const stat = await fs.lstat(source);
-  await fs.rm(destination, { recursive: true, force: true }).catch(() => undefined);
+  await removePathBestEffort(destination);
   if (stat.isSymbolicLink()) {
     await fs.symlink(await fs.readlink(source), destination);
     return;
@@ -201,15 +212,15 @@ async function replaceNpmBinShims(params: {
     await restoreNpmBinShimBackup(backup);
     throw err;
   } finally {
-    await fs.rm(backup.backupDir, { recursive: true, force: true }).catch(() => undefined);
+    await removePathBestEffort(backup.backupDir);
   }
 }
 
 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);
+    await removePathBestEffort(destination);
     if (entry.hadExisting) {
       await copyPathEntry(path.join(backup.backupDir, entry.name), destination);
     }
@@ -253,7 +264,7 @@ async function swapStagedNpmInstall(params: {
       packageName: params.packageName,
     });
     if (movedExisting) {
-      await fs.rm(backupRoot, { recursive: true, force: true });
+      await removePathBestEffort(backupRoot);
     }
     return {
       name: "global install swap",
@@ -268,7 +279,7 @@ async function swapStagedNpmInstall(params: {
     };
   } catch (err) {
     if (movedStaged) {
-      await fs.rm(targetPackageRoot, { recursive: true, force: true }).catch(() => undefined);
+      await removePathBestEffort(targetPackageRoot);
     }
     if (movedExisting) {
       await fs.rename(backupRoot, targetPackageRoot).catch(() => undefined);
