From 0946e37523f93f1c2d0c93fb6682d3e74495e547 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 23 Apr 2026 01:22:09 +0100 Subject: [PATCH] fix(plugins): skip unchanged npm updates --- CHANGELOG.md | 1 + docs/cli/plugins.md | 9 + scripts/e2e/plugin-update-unchanged-docker.sh | 151 ++++++++++++++ src/cli/plugins-update-selection.test.ts | 20 ++ src/cli/plugins-update-selection.ts | 15 +- src/infra/install-source-utils.ts | 62 ++++++ src/plugins/update.test.ts | 194 +++++++++++++++++- src/plugins/update.ts | 58 ++++++ 8 files changed, 507 insertions(+), 3 deletions(-) create mode 100755 scripts/e2e/plugin-update-unchanged-docker.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 28e855153cf..d7f5db737f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ Docs: https://docs.openclaw.ai - Providers/Amazon Bedrock Mantle: refresh IAM-backed bearer tokens at runtime instead of baking discovery-time tokens into provider config, so long-lived Mantle sessions keep working after the initial token ages out. Thanks @wirjo. - Config/includes: write through single-file top-level includes for isolated OpenClaw-owned mutations, so `plugins install` and `plugins update` update an included `plugins.json5` file instead of flattening modular `$include` configs. Fixes #41050 and #66048. - Config/reload: plan gateway reloads from source-authored config instead of runtime-materialized snapshots, so plugin update writes no longer trigger false restarts from derived provider/plugin config paths. Fixes #68732. +- Plugins/update: skip npm plugin reinstall/config rewrites when the installed version and recorded artifact identity already match the registry target, and let bare npm package names resolve back to tracked install records. Fixes #46955 and #67957. - Agents/MCP: keep `mcp.servers` and bundle MCP tools available in Pi embedded `coding` and `messaging` sessions while preserving `minimal` profile and `tools.deny: ["bundle-mcp"]` opt-out behavior. Fixes #68875 and #68818. diff --git a/docs/cli/plugins.md b/docs/cli/plugins.md index b292be325e8..7c164e5781b 100644 --- a/docs/cli/plugins.md +++ b/docs/cli/plugins.md @@ -243,6 +243,15 @@ or exact version. OpenClaw resolves that package name back to the tracked plugin record, updates that installed plugin, and records the new npm spec for future id-based updates. +Passing the npm package name without a version or tag also resolves back to the +tracked plugin record. Use this when a plugin was pinned to an exact version and +you want to move it back to the registry's default release line. + +Before a live npm update, OpenClaw checks the installed package version against +the npm registry metadata. If the installed version and recorded artifact +identity already match the resolved target, the update is skipped without +downloading, reinstalling, or rewriting `openclaw.json`. + When a stored integrity hash exists and the fetched artifact hash changes, OpenClaw treats that as npm artifact drift. The interactive `openclaw plugins update` command prints the expected and actual hashes and asks diff --git a/scripts/e2e/plugin-update-unchanged-docker.sh b/scripts/e2e/plugin-update-unchanged-docker.sh new file mode 100755 index 00000000000..221e12e1e57 --- /dev/null +++ b/scripts/e2e/plugin-update-unchanged-docker.sh @@ -0,0 +1,151 @@ +#!/usr/bin/env bash +set -euo pipefail + +ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +source "$ROOT_DIR/scripts/lib/docker-e2e-logs.sh" + +IMAGE_NAME="${OPENCLAW_PLUGIN_UPDATE_E2E_IMAGE:-openclaw-plugin-update-e2e}" +SKIP_BUILD="${OPENCLAW_PLUGIN_UPDATE_E2E_SKIP_BUILD:-0}" + +if [ "$SKIP_BUILD" = "1" ]; then + echo "Reusing Docker image: $IMAGE_NAME" +else + echo "Building Docker image..." + run_logged plugin-update-build docker build -t "$IMAGE_NAME" -f "$ROOT_DIR/scripts/e2e/Dockerfile" "$ROOT_DIR" +fi + +echo "Running unchanged plugin update smoke..." +docker run --rm \ + -e COREPACK_ENABLE_DOWNLOAD_PROMPT=0 \ + -e OPENCLAW_SKIP_CHANNELS=1 \ + -e OPENCLAW_SKIP_PROVIDERS=1 \ + "$IMAGE_NAME" \ + bash -lc "set -euo pipefail +entry=dist/index.mjs +[ -f \"\$entry\" ] || entry=dist/index.js +export NPM_CONFIG_REGISTRY=http://127.0.0.1:4873 + +mkdir -p \"\$HOME/.openclaw/extensions/lossless-claw\" +cat > \"\$HOME/.openclaw/extensions/lossless-claw/package.json\" <<'JSON' +{ + \"name\": \"@example/lossless-claw\", + \"version\": \"0.9.0\" +} +JSON +cat > \"\$HOME/.openclaw/openclaw.json\" <<'JSON' +{ + \"plugins\": { + \"installs\": { + \"lossless-claw\": { + \"source\": \"npm\", + \"spec\": \"@example/lossless-claw@0.9.0\", + \"installPath\": \"~/.openclaw/extensions/lossless-claw\", + \"resolvedName\": \"@example/lossless-claw\", + \"resolvedVersion\": \"0.9.0\", + \"resolvedSpec\": \"@example/lossless-claw@0.9.0\", + \"integrity\": \"sha512-same\", + \"shasum\": \"same\" + } + } + } +} +JSON + +cat > /tmp/openclaw-e2e-registry.mjs <<'NODE' +import http from 'node:http'; + +const metadata = { + name: '@example/lossless-claw', + 'dist-tags': { latest: '0.9.0' }, + versions: { + '0.9.0': { + name: '@example/lossless-claw', + version: '0.9.0', + dist: { + integrity: 'sha512-same', + shasum: 'same', + tarball: 'http://127.0.0.1:4873/@example/lossless-claw/-/lossless-claw-0.9.0.tgz' + } + } + } +}; + +const server = http.createServer((req, res) => { + if (req.url === '/@example%2flossless-claw' || req.url === '/@example%2Flossless-claw') { + res.writeHead(200, { 'content-type': 'application/json' }); + res.end(JSON.stringify(metadata)); + return; + } + res.writeHead(404, { 'content-type': 'text/plain' }); + res.end('not found: ' + req.url); +}); + +server.listen(4873, '127.0.0.1'); +NODE +node /tmp/openclaw-e2e-registry.mjs >/tmp/openclaw-e2e-registry.log 2>&1 & +registry_pid=\$! +trap 'kill \"\$registry_pid\" >/dev/null 2>&1 || true' EXIT + +registry_ready=0 +for _ in \$(seq 1 50); do + if node --input-type=module -e ' + import http from \"node:http\"; + const req = http.get(\"http://127.0.0.1:4873/@example%2flossless-claw\", (res) => { + process.exit(res.statusCode === 200 ? 0 : 1); + }); + req.on(\"error\", () => process.exit(1)); + req.setTimeout(200, () => { + req.destroy(); + process.exit(1); + }); + '; then + registry_ready=1 + break + fi + sleep 0.1 +done +if [ \"\$registry_ready\" -ne 1 ]; then + echo \"Local npm metadata registry failed to start\" + cat /tmp/openclaw-e2e-registry.log || true + exit 1 +fi + +before_hash=\$(node --input-type=module -e ' + import crypto from \"node:crypto\"; + import fs from \"node:fs\"; + import os from \"node:os\"; + import path from \"node:path\"; + const file = path.join(os.homedir(), \".openclaw\", \"openclaw.json\"); + process.stdout.write(crypto.createHash(\"sha256\").update(fs.readFileSync(file)).digest(\"hex\")); +') + +node \"\$entry\" plugins update @example/lossless-claw > /tmp/plugin-update-output.log 2>&1 + +after_hash=\$(node --input-type=module -e ' + import crypto from \"node:crypto\"; + import fs from \"node:fs\"; + import os from \"node:os\"; + import path from \"node:path\"; + const file = path.join(os.homedir(), \".openclaw\", \"openclaw.json\"); + process.stdout.write(crypto.createHash(\"sha256\").update(fs.readFileSync(file)).digest(\"hex\")); +') + +if [ \"\$before_hash\" != \"\$after_hash\" ]; then + echo \"Config changed unexpectedly\" + cat /tmp/plugin-update-output.log + exit 1 +fi +if grep -q 'Downloading @example/lossless-claw' /tmp/plugin-update-output.log; then + echo \"Unexpected npm download/reinstall path\" + cat /tmp/plugin-update-output.log + exit 1 +fi +if ! grep -q 'lossless-claw is up to date (0.9.0).' /tmp/plugin-update-output.log; then + echo \"Expected up-to-date output missing\" + cat /tmp/plugin-update-output.log + exit 1 +fi +cat /tmp/plugin-update-output.log +" + +echo "Plugin update unchanged Docker E2E passed." diff --git a/src/cli/plugins-update-selection.test.ts b/src/cli/plugins-update-selection.test.ts index b33b67dee6f..722640c703b 100644 --- a/src/cli/plugins-update-selection.test.ts +++ b/src/cli/plugins-update-selection.test.ts @@ -92,4 +92,24 @@ describe("resolvePluginUpdateSelection", () => { pluginIds: ["openclaw-codex-app-server"], }); }); + + it("maps a bare scoped npm package update to the tracked plugin id", () => { + expect( + resolvePluginUpdateSelection({ + installs: { + "lossless-claw": createNpmInstall({ + spec: "@martian-engineering/lossless-claw@0.9.0", + installPath: "/tmp/lossless-claw", + resolvedName: "@martian-engineering/lossless-claw", + }), + }, + rawId: "@martian-engineering/lossless-claw", + }), + ).toEqual({ + pluginIds: ["lossless-claw"], + specOverrides: { + "lossless-claw": "@martian-engineering/lossless-claw", + }, + }); + }); }); diff --git a/src/cli/plugins-update-selection.ts b/src/cli/plugins-update-selection.ts index 17e7ee0ff87..38ebc0eff4b 100644 --- a/src/cli/plugins-update-selection.ts +++ b/src/cli/plugins-update-selection.ts @@ -18,11 +18,14 @@ export function resolvePluginUpdateSelection(params: { return { pluginIds: [] }; } - const parsedSpec = parseRegistryNpmSpec(params.rawId); - if (!parsedSpec || parsedSpec.selectorKind === "none") { + if (params.rawId in params.installs) { return { pluginIds: [params.rawId] }; } + const parsedSpec = parseRegistryNpmSpec(params.rawId); + if (!parsedSpec) { + return { pluginIds: [params.rawId] }; + } const matches = Object.entries(params.installs).filter(([, install]) => { return extractInstalledNpmPackageName(install) === parsedSpec.name; }); @@ -34,6 +37,14 @@ export function resolvePluginUpdateSelection(params: { if (!pluginId) { return { pluginIds: [params.rawId] }; } + if (parsedSpec.selectorKind === "none") { + return { + pluginIds: [pluginId], + specOverrides: { + [pluginId]: parsedSpec.raw, + }, + }; + } return { pluginIds: [pluginId], specOverrides: { diff --git a/src/infra/install-source-utils.ts b/src/infra/install-source-utils.ts index 9fba1924a15..39ff1d27e83 100644 --- a/src/infra/install-source-utils.ts +++ b/src/infra/install-source-utils.ts @@ -34,6 +34,68 @@ export function buildNpmResolutionFields(resolution?: NpmSpecResolution): NpmRes }; } +function normalizeNpmViewMetadata(value: unknown): NpmSpecResolution | null { + if (!value || typeof value !== "object") { + return null; + } + const rec = value as Record; + const name = toOptionalString(rec.name); + const version = toOptionalString(rec.version); + const resolvedSpec = name && version ? `${name}@${version}` : undefined; + const dist = + rec.dist && typeof rec.dist === "object" ? (rec.dist as Record) : {}; + return { + name, + version, + resolvedSpec, + integrity: toOptionalString(rec["dist.integrity"]) ?? toOptionalString(dist.integrity), + shasum: toOptionalString(rec["dist.shasum"]) ?? toOptionalString(dist.shasum), + }; +} + +export async function resolveNpmSpecMetadata(params: { spec: string; timeoutMs?: number }): Promise< + | { + ok: true; + metadata: NpmSpecResolution; + } + | { + ok: false; + error: string; + } +> { + const res = await runCommandWithTimeout( + ["npm", "view", params.spec, "name", "version", "dist.integrity", "dist.shasum", "--json"], + { + timeoutMs: Math.max(params.timeoutMs ?? 60_000, 60_000), + env: { + COREPACK_ENABLE_DOWNLOAD_PROMPT: "0", + NPM_CONFIG_IGNORE_SCRIPTS: "true", + }, + }, + ); + if (res.code !== 0) { + const raw = res.stderr.trim() || res.stdout.trim(); + if (/E404|is not in this registry/i.test(raw)) { + return { + ok: false, + error: `Package not found on npm: ${params.spec}. See https://docs.openclaw.ai/tools/plugin for installable plugins.`, + }; + } + return { ok: false, error: `npm view failed: ${raw}` }; + } + + try { + const parsed = JSON.parse(res.stdout.trim()) as unknown; + const metadata = normalizeNpmViewMetadata(parsed); + if (!metadata?.name || !metadata.version) { + return { ok: false, error: "npm view produced incomplete package metadata" }; + } + return { ok: true, metadata }; + } catch (err) { + return { ok: false, error: `npm view produced invalid JSON: ${String(err)}` }; + } +} + export type NpmIntegrityDrift = { expectedIntegrity: string; actualIntegrity: string; diff --git a/src/plugins/update.test.ts b/src/plugins/update.test.ts index a5908eb34bd..b9502fe79fc 100644 --- a/src/plugins/update.test.ts +++ b/src/plugins/update.test.ts @@ -1,4 +1,7 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { bundledPluginRootAt } from "../../test/helpers/bundled-plugin-paths.js"; import type { OpenClawConfig } from "../config/config.js"; import type { PluginNpmIntegrityDriftParams } from "./install.js"; @@ -13,6 +16,8 @@ const installPluginFromNpmSpecMock = vi.fn(); const installPluginFromMarketplaceMock = vi.fn(); const installPluginFromClawHubMock = vi.fn(); const resolveBundledPluginSourcesMock = vi.fn(); +const runCommandWithTimeoutMock = vi.fn(); +const tempDirs: string[] = []; vi.mock("./install.js", () => ({ installPluginFromNpmSpec: (...args: unknown[]) => installPluginFromNpmSpecMock(...args), @@ -34,6 +39,10 @@ vi.mock("./bundled-sources.js", () => ({ resolveBundledPluginSources: (...args: unknown[]) => resolveBundledPluginSourcesMock(...args), })); +vi.mock("../process/exec.js", () => ({ + runCommandWithTimeout: (...args: unknown[]) => runCommandWithTimeoutMock(...args), +})); + const { syncPluginsForUpdateChannel, updateNpmInstalledPlugins } = await import("./update.js"); function createSuccessfulNpmUpdateResult(params?: { @@ -168,6 +177,34 @@ function createCodexAppServerInstallConfig(params: { }; } +function createInstalledPackageDir(params: { name?: string; version: string }): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-plugin-update-test-")); + tempDirs.push(dir); + fs.writeFileSync( + path.join(dir, "package.json"), + JSON.stringify({ name: params.name ?? "test-plugin", version: params.version }, null, 2), + ); + return dir; +} + +function mockNpmViewMetadata(params: { + name: string; + version: string; + integrity?: string; + shasum?: string; +}) { + runCommandWithTimeoutMock.mockResolvedValueOnce({ + code: 0, + stdout: JSON.stringify({ + name: params.name, + version: params.version, + ...(params.integrity ? { "dist.integrity": params.integrity } : {}), + ...(params.shasum ? { "dist.shasum": params.shasum } : {}), + }), + stderr: "", + }); +} + function expectNpmUpdateCall(params: { spec: string; expectedIntegrity?: string; @@ -232,6 +269,13 @@ describe("updateNpmInstalledPlugins", () => { installPluginFromMarketplaceMock.mockReset(); installPluginFromClawHubMock.mockReset(); resolveBundledPluginSourcesMock.mockReset(); + runCommandWithTimeoutMock.mockReset(); + }); + + afterEach(() => { + for (const dir of tempDirs.splice(0)) { + fs.rmSync(dir, { recursive: true, force: true }); + } }); it.each([ @@ -305,6 +349,154 @@ describe("updateNpmInstalledPlugins", () => { }, ); + it("skips npm reinstall and config rewrite when the installed artifact is unchanged", async () => { + const installPath = createInstalledPackageDir({ + name: "@martian-engineering/lossless-claw", + version: "0.9.0", + }); + mockNpmViewMetadata({ + name: "@martian-engineering/lossless-claw", + version: "0.9.0", + integrity: "sha512-same", + shasum: "same", + }); + installPluginFromNpmSpecMock.mockRejectedValue(new Error("installer should not run")); + const config: OpenClawConfig = { + plugins: { + installs: { + "lossless-claw": { + source: "npm", + spec: "@martian-engineering/lossless-claw", + installPath, + resolvedName: "@martian-engineering/lossless-claw", + resolvedVersion: "0.9.0", + resolvedSpec: "@martian-engineering/lossless-claw@0.9.0", + integrity: "sha512-same", + shasum: "same", + }, + }, + }, + }; + + const result = await updateNpmInstalledPlugins({ + config, + pluginIds: ["lossless-claw"], + }); + + expect(runCommandWithTimeoutMock).toHaveBeenCalledWith( + [ + "npm", + "view", + "@martian-engineering/lossless-claw", + "name", + "version", + "dist.integrity", + "dist.shasum", + "--json", + ], + expect.any(Object), + ); + expect(installPluginFromNpmSpecMock).not.toHaveBeenCalled(); + expect(result.changed).toBe(false); + expect(result.config).toBe(config); + expect(result.outcomes).toEqual([ + { + pluginId: "lossless-claw", + status: "unchanged", + currentVersion: "0.9.0", + nextVersion: "0.9.0", + message: "lossless-claw is up to date (0.9.0).", + }, + ]); + }); + + it("falls through to npm reinstall when the recorded integrity differs", async () => { + const installPath = createInstalledPackageDir({ + name: "@martian-engineering/lossless-claw", + version: "0.9.0", + }); + mockNpmViewMetadata({ + name: "@martian-engineering/lossless-claw", + version: "0.9.0", + integrity: "sha512-new", + }); + installPluginFromNpmSpecMock.mockResolvedValue( + createSuccessfulNpmUpdateResult({ + pluginId: "lossless-claw", + targetDir: installPath, + version: "0.9.0", + npmResolution: { + name: "@martian-engineering/lossless-claw", + version: "0.9.0", + resolvedSpec: "@martian-engineering/lossless-claw@0.9.0", + }, + }), + ); + + const result = await updateNpmInstalledPlugins({ + config: { + plugins: { + installs: { + "lossless-claw": { + source: "npm", + spec: "@martian-engineering/lossless-claw", + installPath, + resolvedName: "@martian-engineering/lossless-claw", + resolvedVersion: "0.9.0", + resolvedSpec: "@martian-engineering/lossless-claw@0.9.0", + integrity: "sha512-old", + }, + }, + }, + }, + pluginIds: ["lossless-claw"], + }); + + expect(installPluginFromNpmSpecMock).toHaveBeenCalledTimes(1); + expect(result.changed).toBe(true); + expect(result.outcomes[0]).toMatchObject({ + pluginId: "lossless-claw", + status: "unchanged", + currentVersion: "0.9.0", + nextVersion: "0.9.0", + }); + }); + + it("falls through to npm reinstall when metadata probing fails", async () => { + const warn = vi.fn(); + const installPath = createInstalledPackageDir({ + name: "@martian-engineering/lossless-claw", + version: "0.9.0", + }); + runCommandWithTimeoutMock.mockResolvedValueOnce({ + code: 1, + stdout: "", + stderr: "registry timeout", + }); + installPluginFromNpmSpecMock.mockResolvedValue( + createSuccessfulNpmUpdateResult({ + pluginId: "lossless-claw", + targetDir: installPath, + version: "0.9.0", + }), + ); + + await updateNpmInstalledPlugins({ + config: createNpmInstallConfig({ + pluginId: "lossless-claw", + spec: "@martian-engineering/lossless-claw", + installPath, + }), + pluginIds: ["lossless-claw"], + logger: { warn }, + }); + + expect(warn).toHaveBeenCalledWith( + "Could not check lossless-claw before update; falling back to installer path: npm view failed: registry timeout", + ); + expect(installPluginFromNpmSpecMock).toHaveBeenCalledTimes(1); + }); + it("aborts exact pinned npm plugin updates on integrity drift by default", async () => { const warn = vi.fn(); installPluginFromNpmSpecMock.mockImplementation( diff --git a/src/plugins/update.ts b/src/plugins/update.ts index 151221c0e9d..c2662aa5382 100644 --- a/src/plugins/update.ts +++ b/src/plugins/update.ts @@ -1,4 +1,6 @@ import type { OpenClawConfig } from "../config/types.openclaw.js"; +import type { NpmSpecResolution } from "../infra/install-source-utils.js"; +import { resolveNpmSpecMetadata } from "../infra/install-source-utils.js"; import { expectedIntegrityForUpdate, readInstalledPackageVersion, @@ -104,6 +106,36 @@ type InstallIntegrityDrift = { }; }; +function stringFieldMatches(recorded: string | undefined, resolved: string | undefined): boolean { + return !recorded || (resolved !== undefined && recorded === resolved); +} + +function shouldSkipUnchangedNpmInstall(params: { + currentVersion?: string; + record: { + integrity?: string; + shasum?: string; + resolvedName?: string; + resolvedSpec?: string; + resolvedVersion?: string; + }; + metadata: NpmSpecResolution; +}): boolean { + if (!params.currentVersion || !params.metadata.version) { + return false; + } + if (params.currentVersion !== params.metadata.version) { + return false; + } + return ( + stringFieldMatches(params.record.integrity, params.metadata.integrity) && + stringFieldMatches(params.record.shasum, params.metadata.shasum) && + stringFieldMatches(params.record.resolvedName, params.metadata.name) && + stringFieldMatches(params.record.resolvedSpec, params.metadata.resolvedSpec) && + stringFieldMatches(params.record.resolvedVersion, params.metadata.version) + ); +} + function pathsEqual( left: string | undefined, right: string | undefined, @@ -348,6 +380,32 @@ export async function updateNpmInstalledPlugins(params: { } const currentVersion = await readInstalledPackageVersion(installPath); + if (!params.dryRun && record.source === "npm" && currentVersion) { + const metadataResult = await resolveNpmSpecMetadata({ spec: effectiveSpec! }); + if (metadataResult.ok) { + if ( + shouldSkipUnchangedNpmInstall({ + currentVersion, + record, + metadata: metadataResult.metadata, + }) + ) { + outcomes.push({ + pluginId, + status: "unchanged", + currentVersion, + nextVersion: metadataResult.metadata.version, + message: `${pluginId} is up to date (${currentVersion}).`, + }); + continue; + } + } else { + logger.warn?.( + `Could not check ${pluginId} before update; falling back to installer path: ${metadataResult.error}`, + ); + } + } + if (params.dryRun) { let probe: | Awaited>