fix(plugins): surface npm errors when plugin install fails [AI-assisted]

[sanctrl]

Summary

• Problem: openclaw plugins install <pkg> reports failures as literally npm install failed: with no detail, leaving users no way to diagnose registry 4xx, ERESOLVE, EUNSUPPORTEDPROTOCOL, network timeouts, etc.
• Why it matters: Every npm-related install failure surfaces as the same opaque message. Maintainers and end users have no actionable signal — the only way to recover the real error today is to reproduce the install manually with a different flag set.
• What changed: Replace --silent with --loglevel=error in the argv at src/infra/install-package-dir.ts:261. Single-token change. Updated the two existing argv assertions in install-package-dir.test.ts and added a new failure-path regression test that locks in the user-facing behavior.
• What did NOT change (scope boundary): Install staging, env-scrubbing (createNpmProjectInstallEnv), --ignore-scripts, --omit=dev, timeout handling, hidden-.npmrc dance — all preserved.

Change Type

• Bug fix

Scope

• Gateway / orchestration (plugins install pipeline)

Linked Issue/PR

• Closes #
• Related #
• This PR fixes a bug or regression

(No tracking issue — per CONTRIBUTING.md "Bugs & small fixes -> Open a PR!")

Root Cause

• Root cause: runCommandWithTimeout is invoked with --silent in the argv. --silent suppresses both stdout and stderr below the warn level — npm can exit non-zero but write nothing to either stream. The failure handler formats "npm install failed: " + (stderr || stdout).trim(), and with empty buffers that's the literal output.
• Missing detection / guardrail: No test exercised the failure path of installPackageDir's npm-install branch. The two existing tests assert only the spawned argv (success path).
• Contributing context: --silent was almost certainly originally added to suppress npm's info-level chatter in the user terminal — a reasonable UX goal that has the same surface-area effect as --loglevel=error but a much wider blast radius (kills error output too). Replacing with --loglevel=error preserves the quiet-stdout intent while letting genuine errors through.

Regression Test Plan

• Unit test — new test in src/infra/install-package-dir.test.ts: surfaces npm's stderr in the failure message instead of swallowing it.
• Target test or file: src/infra/install-package-dir.test.ts
• Scenario the test should lock in: runCommandWithTimeout returns { code: 1, stderr: "npm error code EUNSUPPORTEDPROTOCOL\nnpm error Unsupported URL Type \"workspace:\": workspace:^\n" } and the resulting installPackageDir failure message must contain both EUNSUPPORTEDPROTOCOL and workspace: (i.e. the real npm error has actually been surfaced, not swallowed).
• Why this is the smallest reliable guardrail: It mocks the exact contract between the spawn call and the failure formatter. A future commit that re-introduces a flag silencing npm's stderr (or a refactor that loses the (stderr || stdout) formatting) fails this test before merge.
• Existing test that already covers this: None. The existing tests assert success-path argv only.
• If no new test is added, why not: N/A — added.

User-visible / Behavior Changes

• Failed openclaw plugins install now prints the actual npm error after npm install failed: (e.g. EUNSUPPORTEDPROTOCOL, ERESOLVE, network timeout, registry 4xx) instead of an empty string.
• Successful openclaw plugins install is unchanged — --loglevel=error suppresses the same info-level chatter --silent did.

Diagram

Before:
[user runs `openclaw plugins install bad-pkg`]
  -> spawn("npm install --silent ...")
  -> npm exits 1, stdout="", stderr=""
  -> "npm install failed: " <-- nothing after the colon

After:
[user runs `openclaw plugins install bad-pkg`]
  -> spawn("npm install --loglevel=error ...")
  -> npm exits 1, stderr="npm error code EUNSUPPORTEDPROTOCOL ..."
  -> "npm install failed: npm error code EUNSUPPORTEDPROTOCOL ..."

Security Impact

• New permissions/capabilities? No
• Secrets/tokens handling changed? No
• New/changed network calls? No
• Command/tool execution surface changed? No (same npm subprocess, one different flag)
• Data access scope changed? No

Repro + Verification

Environment

• OS: Linux 6.1.0-44-cloud-amd64 (Debian on GCE)
• Runtime: Node v24.14.1, npm 11.11.0
• Integration/channel: @agentchatme/openclaw@0.6.2 (community plugin) via ClawHub source-linked
• Relevant config: stock root ~/.openclaw, no env overrides

Steps

1. openclaw plugins install @agentchatme/openclaw@0.6.2 (this version had a workspace:^ spec in its source-tree package.json that ClawHub source-linked builds shipped verbatim — npm rejects it with EUNSUPPORTEDPROTOCOL).
2. Observe the truncated error message.
3. (Manual recovery to confirm the real error existed) Inspect ~/.npm/_logs/<latest>.log to recover the swallowed stderr.

Expected (after this PR)

• Output ends with the real npm error: npm install failed: npm error code EUNSUPPORTEDPROTOCOL ...

Actual (before this PR)

• Output ends with npm install failed: and nothing after the colon. Recovering the real error requires reading ~/.npm/_logs/<latest>.log manually.

Evidence

• Failing test before / passing after — the new surfaces npm's stderr in the failure message instead of swallowing it test fails on main (asserts EUNSUPPORTEDPROTOCOL in the result error, which would be empty under --silent) and passes on this branch.

• Trace/log snippets — recovered npm log from a real failed install (the user-side reproducer) attached:

  15 verbose stack Error: Unsupported URL Type "workspace:": workspace:^
  16 error code EUNSUPPORTEDPROTOCOL
  17 error Unsupported URL Type "workspace:": workspace:^
  

  Today this content is in ~/.npm/_logs/<latest>.log only; after this PR, OpenClaw surfaces it in the install command's stderr too.

Human Verification

• Verified scenarios: ran pnpm exec vitest run src/infra/install-package-dir.test.ts locally — 8/8 tests pass on this branch (vs. 7/8 + 1 newly-added on main, which would fail without the fix).
• Edge cases checked: --loglevel=error semantics: npm install warnings are still emitted at warn and above, errors at error and above (npm docs default is warn, --silent is below error, error lets warn-level through if upstream changes). Did not need to verify the warning-suppression edge — --silent was the failure mode, not the warning level.
• What I did not verify: Did not run the full pnpm test matrix (the targeted vitest run covers the modified file). Did not run pnpm check (the umbrella lint/typecheck) — happy to address findings if CI surfaces any.

Review Conversations

• I replied to or resolved every bot review conversation I addressed in this PR.
• I left unresolved only the conversations that still need reviewer or maintainer judgment.

(Will fill in after submitting if any bot conversations land.)

Compatibility / Migration

• Backward compatible? Yes — change is internal to the install pipeline; no public API, config, or persisted format touched.
• Config/env changes? No
• Migration needed? No

Risks and Mitigations

• Risk: A user environment that depended on the empty-stdout behavior could now see error output it didn't before. Realistic? No — the only consumers of this output are humans reading their terminal.
  • Mitigation: None needed.
• Risk: --loglevel=error lets warning-level output through where --silent did not (different surface from errors).
  • Mitigation: npm's --loglevel=error actually only emits error-and-above; warnings are at warn level and would require --loglevel=warn. So this is strictly tighter than default and looser than --silent only in the error category — no unrelated chatter is unblocked.
• Risk: Some downstream parser of the OpenClaw error text might break if it depended on the empty npm install failed: prefix.
  • Mitigation: The prefix is unchanged; only the post-prefix content gains real information. Existing parsers continue to match.

AI-assisted

This PR was AI-assisted (Claude Code). The author reviewed every line, ran the targeted test suite, and confirmed the behavior matches the documented intent before submitting. Degree of testing: lightly tested — targeted vitest run on the modified file plus end-to-end manual verification on the GCE VM that originally hit the bug; full pnpm check / pnpm build not yet run from this machine due to memory pressure — happy to iterate on CI findings.

[greptile-apps]

Greptile Summary

This PR fixes openclaw plugins install surfacing a blank npm install failed: message on failure by replacing --silent with --loglevel=error in the npm invocation. The single-token change preserves suppression of npm's info-level chatter while letting genuine error output reach the existing stderr || stdout formatter at line 282. Two existing argv assertions are updated to match, and a well-targeted regression test directly verifies that an EUNSUPPORTEDPROTOCOL stderr payload propagates to the user-facing error string.

Confidence Score: 5/5

Safe to merge — minimal, well-scoped change with correct logic and good regression coverage.

Single-token flag swap with a clear semantic justification, updated tests, and a new regression test that locks in the corrected behavior. No P0 or P1 issues found.

No files require special attention.

_{Reviews (1): Last reviewed commit: "fix(plugins): surface npm errors when pl..." | Re-trigger Greptile}

[steipete]

Thanks for the focused fix. I verified the failure mode on Blacksmith Ubuntu / Node 24 / npm 11: npm install --silent can fail with empty stdout/stderr for a bad dependency spec like workspace:^, while --loglevel=error preserves the actionable EUNSUPPORTEDPROTOCOL stderr.

Landed this on main in 067888a with the same npm flag change in src/infra/install-package-dir.ts, updated shared argv assertions in src/test-utils/exec-assertions.ts, and added a regression test in src/infra/install-package-dir.test.ts so plugin/hook dependency failures keep surfacing npm stderr.

Validation run before push:

• pnpm test src/infra/install-package-dir.test.ts src/plugins/install.test.ts src/plugins/install.path.test.ts src/hooks/install.test.ts
• pnpm check:changelog-attributions
• git diff --check

Closing this PR as covered by the landed commit. Thanks @sanctrl.