From 067888a608dfe26110fcd6b80cd904e888c01936 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 28 Apr 2026 01:17:39 +0100 Subject: [PATCH] fix: surface npm plugin install errors --- CHANGELOG.md | 1 + src/infra/install-package-dir.test.ts | 52 +++++++++++++++++++++++++-- src/infra/install-package-dir.ts | 5 ++- src/test-utils/exec-assertions.ts | 2 +- 4 files changed, 56 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2eaf0114e70..60b46a90743 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ Docs: https://docs.openclaw.ai ### Fixes - Memory/LanceDB: call OpenAI-compatible embedding endpoints through the raw SDK transport without sending `encoding_format`, then normalize float-array or base64 responses so providers such as ZhiPu and DashScope no longer fail recall with wrong vector dimensions or rejected parameters. Fixes #63655. Thanks @kinthaiofficial. +- Plugins/install: run dependency installs with npm error-level logging instead of silent mode so failed plugin or hook installs surface actionable npm errors such as EUNSUPPORTEDPROTOCOL instead of `npm install failed:` with no detail. (#73093) Thanks @sanctrl. - Memory/LanceDB: bound memory recall embedding queries with a new `recallMaxChars` setting, prefer the latest user message over channel prompt metadata during auto-recall, and document the knob so small Ollama embedding models avoid context-length failures. Fixes #56780. Thanks @rungmc357 and @zak-collaborator. - CLI/skills: resolve workspace-backed skills commands from `--agent`, then the current agent workspace, before falling back to the default agent, so multi-agent ClawHub installs, updates, and status checks stay scoped to the active workspace. Fixes #56161; carries forward #72726. Thanks @langbowang and @luyao618. - Plugin SDK: fall back from partial bundled plugin directory overrides to package source public surfaces while preserving `OPENCLAW_DISABLE_BUNDLED_PLUGINS` as a hard disable. (#72817) Thanks @serkonyc. diff --git a/src/infra/install-package-dir.test.ts b/src/infra/install-package-dir.test.ts index 540bcaf0090..7b97f621424 100644 --- a/src/infra/install-package-dir.test.ts +++ b/src/infra/install-package-dir.test.ts @@ -325,7 +325,7 @@ describe("installPackageDir", () => { expect(result).toEqual({ ok: true }); expect(vi.mocked(runCommandWithTimeout)).toHaveBeenCalledWith( - ["npm", "install", "--omit=dev", "--silent", "--ignore-scripts"], + ["npm", "install", "--omit=dev", "--loglevel=error", "--ignore-scripts"], expect.objectContaining({ cwd: expect.stringContaining(".openclaw-install-stage-"), }), @@ -433,7 +433,7 @@ describe("installPackageDir", () => { expect(result).toEqual({ ok: true }); expect(vi.mocked(runCommandWithTimeout)).toHaveBeenCalledWith( - ["npm", "install", "--omit=dev", "--silent", "--ignore-scripts"], + ["npm", "install", "--omit=dev", "--loglevel=error", "--ignore-scripts"], expect.objectContaining({ env: expect.objectContaining({ npm_config_global: "false", @@ -455,4 +455,52 @@ describe("installPackageDir", () => { }), ); }); + + it("surfaces npm stderr when dependency install fails", async () => { + await fixtureRootTracker.setup(); + const fixtureRoot = await fixtureRootTracker.make("case"); + const sourceDir = path.join(fixtureRoot, "source"); + const targetDir = path.join(fixtureRoot, "plugins", "demo"); + await fs.mkdir(sourceDir, { recursive: true }); + await fs.writeFile( + path.join(sourceDir, "package.json"), + JSON.stringify({ + name: "demo-plugin", + version: "1.0.0", + dependencies: { + bad: "workspace:^", + }, + }), + "utf-8", + ); + + // Mirrors the Blacksmith repro: npm 11 preserved this stderr with + // `--loglevel=error`, while `--silent` returned empty output. + vi.mocked(runCommandWithTimeout).mockResolvedValue({ + stdout: "", + stderr: + 'npm error code EUNSUPPORTEDPROTOCOL\nnpm error Unsupported URL Type "workspace:": workspace:^\n', + code: 1, + signal: null, + killed: false, + termination: "exit", + }); + + const result = await installPackageDir({ + sourceDir, + targetDir, + mode: "install", + timeoutMs: 1_000, + copyErrorPrefix: "failed to copy plugin", + hasDeps: true, + depsLogMessage: "Installing deps…", + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toContain("npm install failed:"); + expect(result.error).toContain("EUNSUPPORTEDPROTOCOL"); + expect(result.error).toContain("workspace:"); + } + }); }); diff --git a/src/infra/install-package-dir.ts b/src/infra/install-package-dir.ts index 6adf27d69f0..e2af4c11541 100644 --- a/src/infra/install-package-dir.ts +++ b/src/infra/install-package-dir.ts @@ -258,7 +258,10 @@ export async function installPackageDir(params: { return await runCommandWithTimeout( // Plugins install into isolated directories, so omitting peer deps can strip // runtime requirements that npm would otherwise materialize for the package. - ["npm", "install", "--omit=dev", "--silent", "--ignore-scripts"], + // Verified on Blacksmith Ubuntu/Node 24/npm 11: `--silent` can make npm fail + // with empty stdout/stderr for bad specs like `workspace:^`; `--loglevel=error` + // stays quiet on success while preserving the actionable npm failure text. + ["npm", "install", "--omit=dev", "--loglevel=error", "--ignore-scripts"], { timeoutMs: Math.max(params.timeoutMs, 300_000), cwd: stageDir, diff --git a/src/test-utils/exec-assertions.ts b/src/test-utils/exec-assertions.ts index 6e9149725ef..98092e50e7a 100644 --- a/src/test-utils/exec-assertions.ts +++ b/src/test-utils/exec-assertions.ts @@ -28,7 +28,7 @@ export function expectSingleNpmInstallIgnoreScriptsCall(params: { throw new Error("expected npm install call"); } const [argv, opts] = first; - expect(argv).toEqual(["npm", "install", "--omit=dev", "--silent", "--ignore-scripts"]); + expect(argv).toEqual(["npm", "install", "--omit=dev", "--loglevel=error", "--ignore-scripts"]); expect(opts?.cwd).toBeTruthy(); const cwd = String(opts?.cwd); const expectedTargetDir = params.expectedTargetDir;