mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-22 14:41:34 +00:00
refactor: harden plugin install flow and main DM route pinning
This commit is contained in:
@@ -1,9 +1,5 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
findBundledPluginByNpmSpec,
|
||||
findBundledPluginByPluginId,
|
||||
resolveBundledPluginSources,
|
||||
} from "./bundled-sources.js";
|
||||
import { findBundledPluginSource, resolveBundledPluginSources } from "./bundled-sources.js";
|
||||
|
||||
const discoverOpenClawPluginsMock = vi.fn();
|
||||
const loadPluginManifestMock = vi.fn();
|
||||
@@ -91,8 +87,12 @@ describe("bundled plugin sources", () => {
|
||||
});
|
||||
loadPluginManifestMock.mockReturnValue({ ok: true, manifest: { id: "feishu" } });
|
||||
|
||||
const resolved = findBundledPluginByNpmSpec({ spec: "@openclaw/feishu" });
|
||||
const missing = findBundledPluginByNpmSpec({ spec: "@openclaw/not-found" });
|
||||
const resolved = findBundledPluginSource({
|
||||
lookup: { kind: "npmSpec", value: "@openclaw/feishu" },
|
||||
});
|
||||
const missing = findBundledPluginSource({
|
||||
lookup: { kind: "npmSpec", value: "@openclaw/not-found" },
|
||||
});
|
||||
|
||||
expect(resolved?.pluginId).toBe("feishu");
|
||||
expect(resolved?.localPath).toBe("/app/extensions/feishu");
|
||||
@@ -113,8 +113,12 @@ describe("bundled plugin sources", () => {
|
||||
});
|
||||
loadPluginManifestMock.mockReturnValue({ ok: true, manifest: { id: "diffs" } });
|
||||
|
||||
const resolved = findBundledPluginByPluginId({ pluginId: "diffs" });
|
||||
const missing = findBundledPluginByPluginId({ pluginId: "not-found" });
|
||||
const resolved = findBundledPluginSource({
|
||||
lookup: { kind: "pluginId", value: "diffs" },
|
||||
});
|
||||
const missing = findBundledPluginSource({
|
||||
lookup: { kind: "pluginId", value: "not-found" },
|
||||
});
|
||||
|
||||
expect(resolved?.pluginId).toBe("diffs");
|
||||
expect(resolved?.localPath).toBe("/app/extensions/diffs");
|
||||
|
||||
@@ -7,6 +7,10 @@ export type BundledPluginSource = {
|
||||
npmSpec?: string;
|
||||
};
|
||||
|
||||
export type BundledPluginLookup =
|
||||
| { kind: "npmSpec"; value: string }
|
||||
| { kind: "pluginId"; value: string };
|
||||
|
||||
export function resolveBundledPluginSources(params: {
|
||||
workspaceDir?: string;
|
||||
}): Map<string, BundledPluginSource> {
|
||||
@@ -41,38 +45,22 @@ export function resolveBundledPluginSources(params: {
|
||||
return bundled;
|
||||
}
|
||||
|
||||
export function findBundledPluginByNpmSpec(params: {
|
||||
spec: string;
|
||||
export function findBundledPluginSource(params: {
|
||||
lookup: BundledPluginLookup;
|
||||
workspaceDir?: string;
|
||||
}): BundledPluginSource | undefined {
|
||||
const targetSpec = params.spec.trim();
|
||||
if (!targetSpec) {
|
||||
const targetValue = params.lookup.value.trim();
|
||||
if (!targetValue) {
|
||||
return undefined;
|
||||
}
|
||||
const bundled = resolveBundledPluginSources({ workspaceDir: params.workspaceDir });
|
||||
if (params.lookup.kind === "pluginId") {
|
||||
return bundled.get(targetValue);
|
||||
}
|
||||
for (const source of bundled.values()) {
|
||||
if (source.npmSpec === targetSpec) {
|
||||
return source;
|
||||
}
|
||||
// Also match by plugin id so that e.g. `openclaw plugins install diffs`
|
||||
// resolves to the bundled @openclaw/diffs plugin when the unscoped npm
|
||||
// package `diffs` is not a valid OpenClaw plugin.
|
||||
// See: https://github.com/openclaw/openclaw/issues/32019
|
||||
if (source.pluginId === targetSpec) {
|
||||
if (source.npmSpec === targetValue) {
|
||||
return source;
|
||||
}
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
export function findBundledPluginByPluginId(params: {
|
||||
pluginId: string;
|
||||
workspaceDir?: string;
|
||||
}): BundledPluginSource | undefined {
|
||||
const targetPluginId = params.pluginId.trim();
|
||||
if (!targetPluginId) {
|
||||
return undefined;
|
||||
}
|
||||
const bundled = resolveBundledPluginSources({ workspaceDir: params.workspaceDir });
|
||||
return bundled.get(targetPluginId);
|
||||
}
|
||||
|
||||
@@ -8,7 +8,6 @@ import { expectSingleNpmPackIgnoreScriptsCall } from "../test-utils/exec-asserti
|
||||
import {
|
||||
expectInstallUsesIgnoreScripts,
|
||||
expectIntegrityDriftRejected,
|
||||
expectUnsupportedNpmSpec,
|
||||
mockNpmPackMetadataResult,
|
||||
} from "../test-utils/npm-spec-install-test-helpers.js";
|
||||
|
||||
@@ -20,6 +19,7 @@ let installPluginFromArchive: typeof import("./install.js").installPluginFromArc
|
||||
let installPluginFromDir: typeof import("./install.js").installPluginFromDir;
|
||||
let installPluginFromNpmSpec: typeof import("./install.js").installPluginFromNpmSpec;
|
||||
let installPluginFromPath: typeof import("./install.js").installPluginFromPath;
|
||||
let PLUGIN_INSTALL_ERROR_CODE: typeof import("./install.js").PLUGIN_INSTALL_ERROR_CODE;
|
||||
let runCommandWithTimeout: typeof import("../process/exec.js").runCommandWithTimeout;
|
||||
let suiteTempRoot = "";
|
||||
let tempDirCounter = 0;
|
||||
@@ -255,6 +255,7 @@ beforeAll(async () => {
|
||||
installPluginFromDir,
|
||||
installPluginFromNpmSpec,
|
||||
installPluginFromPath,
|
||||
PLUGIN_INSTALL_ERROR_CODE,
|
||||
} = await import("./install.js"));
|
||||
({ runCommandWithTimeout } = await import("../process/exec.js"));
|
||||
});
|
||||
@@ -372,6 +373,7 @@ describe("installPluginFromArchive", () => {
|
||||
return;
|
||||
}
|
||||
expect(result.error).toContain("openclaw.extensions");
|
||||
expect(result.code).toBe(PLUGIN_INSTALL_ERROR_CODE.MISSING_OPENCLAW_EXTENSIONS);
|
||||
});
|
||||
|
||||
it("rejects legacy plugin package shape when openclaw.extensions is missing", async () => {
|
||||
@@ -403,6 +405,7 @@ describe("installPluginFromArchive", () => {
|
||||
if (!result.ok) {
|
||||
expect(result.error).toContain("package.json missing openclaw.extensions");
|
||||
expect(result.error).toContain("update the plugin package");
|
||||
expect(result.code).toBe(PLUGIN_INSTALL_ERROR_CODE.MISSING_OPENCLAW_EXTENSIONS);
|
||||
return;
|
||||
}
|
||||
expect.unreachable("expected install to fail without openclaw.extensions");
|
||||
@@ -668,7 +671,12 @@ describe("installPluginFromNpmSpec", () => {
|
||||
});
|
||||
|
||||
it("rejects non-registry npm specs", async () => {
|
||||
await expectUnsupportedNpmSpec((spec) => installPluginFromNpmSpec({ spec }));
|
||||
const result = await installPluginFromNpmSpec({ spec: "github:evil/evil" });
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) {
|
||||
expect(result.error).toContain("unsupported npm spec");
|
||||
expect(result.code).toBe(PLUGIN_INSTALL_ERROR_CODE.INVALID_NPM_SPEC);
|
||||
}
|
||||
});
|
||||
|
||||
it("aborts when integrity drift callback rejects the fetched artifact", async () => {
|
||||
@@ -695,4 +703,25 @@ describe("installPluginFromNpmSpec", () => {
|
||||
actualIntegrity: "sha512-new",
|
||||
});
|
||||
});
|
||||
|
||||
it("classifies npm package-not-found errors with a stable error code", async () => {
|
||||
const run = vi.mocked(runCommandWithTimeout);
|
||||
run.mockResolvedValue({
|
||||
code: 1,
|
||||
stdout: "",
|
||||
stderr: "npm ERR! code E404\nnpm ERR! 404 Not Found - GET https://registry.npmjs.org/nope",
|
||||
signal: null,
|
||||
killed: false,
|
||||
termination: "exit",
|
||||
});
|
||||
|
||||
const result = await installPluginFromNpmSpec({
|
||||
spec: "@openclaw/not-found",
|
||||
logger: { info: () => {}, warn: () => {} },
|
||||
});
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) {
|
||||
expect(result.code).toBe(PLUGIN_INSTALL_ERROR_CODE.NPM_PACKAGE_NOT_FOUND);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -48,6 +48,17 @@ type PackageManifest = PluginPackageManifest & {
|
||||
const MISSING_EXTENSIONS_ERROR =
|
||||
'package.json missing openclaw.extensions; update the plugin package to include openclaw.extensions (for example ["./dist/index.js"]). See https://docs.openclaw.ai/help/troubleshooting#plugin-install-fails-with-missing-openclaw-extensions';
|
||||
|
||||
export const PLUGIN_INSTALL_ERROR_CODE = {
|
||||
INVALID_NPM_SPEC: "invalid_npm_spec",
|
||||
MISSING_OPENCLAW_EXTENSIONS: "missing_openclaw_extensions",
|
||||
EMPTY_OPENCLAW_EXTENSIONS: "empty_openclaw_extensions",
|
||||
NPM_PACKAGE_NOT_FOUND: "npm_package_not_found",
|
||||
PLUGIN_ID_MISMATCH: "plugin_id_mismatch",
|
||||
} as const;
|
||||
|
||||
export type PluginInstallErrorCode =
|
||||
(typeof PLUGIN_INSTALL_ERROR_CODE)[keyof typeof PLUGIN_INSTALL_ERROR_CODE];
|
||||
|
||||
export type InstallPluginResult =
|
||||
| {
|
||||
ok: true;
|
||||
@@ -59,7 +70,7 @@ export type InstallPluginResult =
|
||||
npmResolution?: NpmSpecResolution;
|
||||
integrityDrift?: NpmIntegrityDrift;
|
||||
}
|
||||
| { ok: false; error: string };
|
||||
| { ok: false; error: string; code?: PluginInstallErrorCode };
|
||||
|
||||
export type PluginNpmIntegrityDriftParams = {
|
||||
spec: string;
|
||||
@@ -86,15 +97,43 @@ function validatePluginId(pluginId: string): string | null {
|
||||
return null;
|
||||
}
|
||||
|
||||
function ensureOpenClawExtensions(params: { manifest: PackageManifest }): string[] {
|
||||
function ensureOpenClawExtensions(params: { manifest: PackageManifest }):
|
||||
| {
|
||||
ok: true;
|
||||
entries: string[];
|
||||
}
|
||||
| {
|
||||
ok: false;
|
||||
error: string;
|
||||
code: PluginInstallErrorCode;
|
||||
} {
|
||||
const resolved = resolvePackageExtensionEntries(params.manifest);
|
||||
if (resolved.status === "missing") {
|
||||
throw new Error(MISSING_EXTENSIONS_ERROR);
|
||||
return {
|
||||
ok: false,
|
||||
error: MISSING_EXTENSIONS_ERROR,
|
||||
code: PLUGIN_INSTALL_ERROR_CODE.MISSING_OPENCLAW_EXTENSIONS,
|
||||
};
|
||||
}
|
||||
if (resolved.status === "empty") {
|
||||
throw new Error("package.json openclaw.extensions is empty");
|
||||
return {
|
||||
ok: false,
|
||||
error: "package.json openclaw.extensions is empty",
|
||||
code: PLUGIN_INSTALL_ERROR_CODE.EMPTY_OPENCLAW_EXTENSIONS,
|
||||
};
|
||||
}
|
||||
return resolved.entries;
|
||||
return {
|
||||
ok: true,
|
||||
entries: resolved.entries,
|
||||
};
|
||||
}
|
||||
|
||||
function isNpmPackageNotFoundMessage(error: string): boolean {
|
||||
const normalized = error.trim();
|
||||
if (normalized.startsWith("Package not found on npm:")) {
|
||||
return true;
|
||||
}
|
||||
return /E404|404 not found|not in this registry/i.test(normalized);
|
||||
}
|
||||
|
||||
function buildFileInstallResult(pluginId: string, targetFile: string): InstallPluginResult {
|
||||
@@ -150,14 +189,17 @@ async function installPluginFromPackageDir(params: {
|
||||
return { ok: false, error: `invalid package.json: ${String(err)}` };
|
||||
}
|
||||
|
||||
let extensions: string[];
|
||||
try {
|
||||
extensions = ensureOpenClawExtensions({
|
||||
manifest,
|
||||
});
|
||||
} catch (err) {
|
||||
return { ok: false, error: String(err) };
|
||||
const extensionsResult = ensureOpenClawExtensions({
|
||||
manifest,
|
||||
});
|
||||
if (!extensionsResult.ok) {
|
||||
return {
|
||||
ok: false,
|
||||
error: extensionsResult.error,
|
||||
code: extensionsResult.code,
|
||||
};
|
||||
}
|
||||
const extensions = extensionsResult.entries;
|
||||
|
||||
const pkgName = typeof manifest.name === "string" ? manifest.name : "";
|
||||
const npmPluginId = pkgName ? unscopedPackageName(pkgName) : "plugin";
|
||||
@@ -181,6 +223,7 @@ async function installPluginFromPackageDir(params: {
|
||||
return {
|
||||
ok: false,
|
||||
error: `plugin id mismatch: expected ${params.expectedPluginId}, got ${pluginId}`,
|
||||
code: PLUGIN_INSTALL_ERROR_CODE.PLUGIN_ID_MISMATCH,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -436,7 +479,11 @@ export async function installPluginFromNpmSpec(params: {
|
||||
const spec = params.spec.trim();
|
||||
const specError = validateRegistryNpmSpec(spec);
|
||||
if (specError) {
|
||||
return { ok: false, error: specError };
|
||||
return {
|
||||
ok: false,
|
||||
error: specError,
|
||||
code: PLUGIN_INSTALL_ERROR_CODE.INVALID_NPM_SPEC,
|
||||
};
|
||||
}
|
||||
|
||||
logger.info?.(`Downloading ${spec}…`);
|
||||
@@ -459,7 +506,15 @@ export async function installPluginFromNpmSpec(params: {
|
||||
expectedPluginId,
|
||||
},
|
||||
});
|
||||
return finalizeNpmSpecArchiveInstall(flowResult);
|
||||
const finalized = finalizeNpmSpecArchiveInstall(flowResult);
|
||||
if (!finalized.ok && isNpmPackageNotFoundMessage(finalized.error)) {
|
||||
return {
|
||||
ok: false,
|
||||
error: finalized.error,
|
||||
code: PLUGIN_INSTALL_ERROR_CODE.NPM_PACKAGE_NOT_FOUND,
|
||||
};
|
||||
}
|
||||
return finalized;
|
||||
}
|
||||
|
||||
export async function installPluginFromPath(params: {
|
||||
|
||||
83
src/plugins/update.test.ts
Normal file
83
src/plugins/update.test.ts
Normal file
@@ -0,0 +1,83 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const installPluginFromNpmSpecMock = vi.fn();
|
||||
|
||||
vi.mock("./install.js", () => ({
|
||||
installPluginFromNpmSpec: (...args: unknown[]) => installPluginFromNpmSpecMock(...args),
|
||||
resolvePluginInstallDir: (pluginId: string) => `/tmp/${pluginId}`,
|
||||
PLUGIN_INSTALL_ERROR_CODE: {
|
||||
NPM_PACKAGE_NOT_FOUND: "npm_package_not_found",
|
||||
},
|
||||
}));
|
||||
|
||||
describe("updateNpmInstalledPlugins", () => {
|
||||
beforeEach(() => {
|
||||
installPluginFromNpmSpecMock.mockReset();
|
||||
});
|
||||
|
||||
it("formats package-not-found updates with a stable message", async () => {
|
||||
installPluginFromNpmSpecMock.mockResolvedValue({
|
||||
ok: false,
|
||||
code: "npm_package_not_found",
|
||||
error: "Package not found on npm: @openclaw/missing.",
|
||||
});
|
||||
|
||||
const { updateNpmInstalledPlugins } = await import("./update.js");
|
||||
const result = await updateNpmInstalledPlugins({
|
||||
config: {
|
||||
plugins: {
|
||||
installs: {
|
||||
missing: {
|
||||
source: "npm",
|
||||
spec: "@openclaw/missing",
|
||||
installPath: "/tmp/missing",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
pluginIds: ["missing"],
|
||||
dryRun: true,
|
||||
});
|
||||
|
||||
expect(result.outcomes).toEqual([
|
||||
{
|
||||
pluginId: "missing",
|
||||
status: "error",
|
||||
message: "Failed to check missing: npm package not found for @openclaw/missing.",
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("falls back to raw installer error for unknown error codes", async () => {
|
||||
installPluginFromNpmSpecMock.mockResolvedValue({
|
||||
ok: false,
|
||||
code: "invalid_npm_spec",
|
||||
error: "unsupported npm spec: github:evil/evil",
|
||||
});
|
||||
|
||||
const { updateNpmInstalledPlugins } = await import("./update.js");
|
||||
const result = await updateNpmInstalledPlugins({
|
||||
config: {
|
||||
plugins: {
|
||||
installs: {
|
||||
bad: {
|
||||
source: "npm",
|
||||
spec: "github:evil/evil",
|
||||
installPath: "/tmp/bad",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
pluginIds: ["bad"],
|
||||
dryRun: true,
|
||||
});
|
||||
|
||||
expect(result.outcomes).toEqual([
|
||||
{
|
||||
pluginId: "bad",
|
||||
status: "error",
|
||||
message: "Failed to check bad: unsupported npm spec: github:evil/evil",
|
||||
},
|
||||
]);
|
||||
});
|
||||
});
|
||||
@@ -5,7 +5,12 @@ import { openBoundaryFileSync } from "../infra/boundary-file-read.js";
|
||||
import type { UpdateChannel } from "../infra/update-channels.js";
|
||||
import { resolveUserPath } from "../utils.js";
|
||||
import { resolveBundledPluginSources } from "./bundled-sources.js";
|
||||
import { installPluginFromNpmSpec, resolvePluginInstallDir } from "./install.js";
|
||||
import {
|
||||
installPluginFromNpmSpec,
|
||||
PLUGIN_INSTALL_ERROR_CODE,
|
||||
type InstallPluginResult,
|
||||
resolvePluginInstallDir,
|
||||
} from "./install.js";
|
||||
import { buildNpmResolutionInstallFields, recordPluginInstall } from "./installs.js";
|
||||
|
||||
export type PluginUpdateLogger = {
|
||||
@@ -53,6 +58,18 @@ export type PluginChannelSyncResult = {
|
||||
summary: PluginChannelSyncSummary;
|
||||
};
|
||||
|
||||
function formatNpmInstallFailure(params: {
|
||||
pluginId: string;
|
||||
spec: string;
|
||||
phase: "check" | "update";
|
||||
result: Extract<InstallPluginResult, { ok: false }>;
|
||||
}): string {
|
||||
if (params.result.code === PLUGIN_INSTALL_ERROR_CODE.NPM_PACKAGE_NOT_FOUND) {
|
||||
return `Failed to ${params.phase} ${params.pluginId}: npm package not found for ${params.spec}.`;
|
||||
}
|
||||
return `Failed to ${params.phase} ${params.pluginId}: ${params.result.error}`;
|
||||
}
|
||||
|
||||
type InstallIntegrityDrift = {
|
||||
spec: string;
|
||||
expectedIntegrity: string;
|
||||
@@ -250,7 +267,12 @@ export async function updateNpmInstalledPlugins(params: {
|
||||
outcomes.push({
|
||||
pluginId,
|
||||
status: "error",
|
||||
message: `Failed to check ${pluginId}: ${probe.error}`,
|
||||
message: formatNpmInstallFailure({
|
||||
pluginId,
|
||||
spec: record.spec,
|
||||
phase: "check",
|
||||
result: probe,
|
||||
}),
|
||||
});
|
||||
continue;
|
||||
}
|
||||
@@ -304,7 +326,12 @@ export async function updateNpmInstalledPlugins(params: {
|
||||
outcomes.push({
|
||||
pluginId,
|
||||
status: "error",
|
||||
message: `Failed to update ${pluginId}: ${result.error}`,
|
||||
message: formatNpmInstallFailure({
|
||||
pluginId,
|
||||
spec: record.spec,
|
||||
phase: "update",
|
||||
result: result,
|
||||
}),
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user