fix(plugins): suppress misleading hook-pack fallback on npm-root recovery failure

pingu · pingu · commit 93d00f311ec9 · 2026-05-25T00:25:53.000+08:00
Address review: the managed-npm-root recovery rebuild-failure error
("managed npm root recovery failed after quarantining…") did not match the
hook-pack-fallback skip predicate, so the misleading "Also not a valid hook
pack" message could still print on that path. Add the recovery-failure prefix
to shouldSkipHookPackFallbackAfterPluginInstallError, add a CLI regression
test for the rebuild-failure path, and document the managed-root no-lock
quarantine assumption.
diff --git a/src/cli/plugins-cli.install.test.ts b/src/cli/plugins-cli.install.test.ts
@@ -1159,6 +1159,28 @@ describe("plugins cli install", () => {
     expect(runtimeErrors.at(-1)).not.toContain("Also not a valid hook pack");
   });
 
+  it("does not fall back to hook pack when managed npm root recovery rebuild fails", async () => {
+    loadConfig.mockReturnValue({} as OpenClawConfig);
+    installPluginFromNpmSpec.mockResolvedValue({
+      ok: false,
+      error:
+        "managed npm root recovery failed after quarantining package-lock.json at /tmp/openclaw-state/extensions/.openclaw-managed-node_modules/_openclaw-quarantined-npm-roots/corrupt-demo: npm install failed during rebuild",
+    });
+    installHooksFromNpmSpec.mockResolvedValue({
+      ok: false,
+      error: "package.json missing openclaw.hooks",
+    });
+
+    await expect(runPluginsCommand(["plugins", "install", "npm:demo"])).rejects.toThrow(
+      "__exit__:1",
+    );
+
+    expect(installPluginFromClawHub).not.toHaveBeenCalled();
+    expect(installHooksFromNpmSpec).not.toHaveBeenCalled();
+    expect(runtimeErrors.at(-1)).toContain("managed npm root recovery failed");
+    expect(runtimeErrors.at(-1)).not.toContain("Also not a valid hook pack");
+  });
+
   it("adds a Git PATH hint when npm plugin dependency install cannot spawn git", async () => {
     loadConfig.mockReturnValue({} as OpenClawConfig);
     installPluginFromNpmSpec.mockResolvedValue({
diff --git a/src/cli/plugins-command-helpers.ts b/src/cli/plugins-command-helpers.ts
@@ -201,6 +201,7 @@ export function shouldSkipHookPackFallbackAfterPluginInstallError(pluginError: s
   return (
     pluginError.startsWith("npm install failed") ||
     pluginError.startsWith("npm peer dependency planning failed") ||
+    pluginError.startsWith("managed npm root recovery failed") ||
     pluginError.startsWith("Failed to stage npm pack archive in managed npm root")
   );
 }
diff --git a/src/infra/npm-managed-root.ts b/src/infra/npm-managed-root.ts
@@ -569,6 +569,7 @@ export async function quarantineManagedNpmRootForRebuild(params: {
   await fs.mkdir(params.npmRoot, { recursive: true });
   const quarantineParent = path.join(params.npmRoot, MANAGED_NPM_ROOT_QUARANTINE_DIR);
   await fs.mkdir(quarantineParent, { recursive: true });
+  // No process lock on the managed root; safe-by-construction: mkdtemp unique dir + rename ENOENT swallowed on re-quarantine.
   const quarantineDir = await fs.mkdtemp(path.join(quarantineParent, "corrupt-"));
   const movedArtifactNames: string[] = [];
   for (const artifactName of MANAGED_NPM_ROOT_REBUILD_ARTIFACT_NAMES) {

Original file line number	Diff line number	Diff line change
`@@ -201,6 +201,7 @@ export function shouldSkipHookPackFallbackAfterPluginInstallError(pluginError: s`
`201`	`201`	`return (`
`202`	`202`	`pluginError.startsWith("npm install failed") \|\|`
`203`	`203`	`pluginError.startsWith("npm peer dependency planning failed") \|\|`
	`204`	`+ pluginError.startsWith("managed npm root recovery failed") \|\|`
`204`	`205`	`pluginError.startsWith("Failed to stage npm pack archive in managed npm root")`
`205`	`206`	`);`
`206`	`207`	`}`