mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 21:07:01 -06:00
fix(plugins): report installed dependency health authoritatively (#117029)
* fix(plugins): project missing dependency health consistently * fix(plugins): preserve leaf dependency-health boundaries * fix(plugins): retain runtime dependency failure guidance * test(plugins): reproduce failed dependency runtime imports * fix(plugins): narrow existing dependency diagnostics safely --------- Co-authored-by: Peter Steinberger <steipete@macos.shared>
This commit is contained in:
committed by
GitHub
parent
89394978e0
commit
90ae3c253f
@@ -26,6 +26,23 @@ export type PluginDependencyStatus = {
|
||||
optionalDependencies: PluginDependencyEntry[];
|
||||
};
|
||||
|
||||
type PluginDependencyHealthRegistry = {
|
||||
plugins: Array<{
|
||||
id: string;
|
||||
source: string;
|
||||
enabled: boolean;
|
||||
status: "loaded" | "disabled" | "error";
|
||||
error?: string;
|
||||
dependencyStatus?: PluginDependencyStatus;
|
||||
}>;
|
||||
diagnostics: Array<{
|
||||
level: "warn" | "error";
|
||||
message: string;
|
||||
pluginId?: string;
|
||||
source?: string;
|
||||
}>;
|
||||
};
|
||||
|
||||
function normalizeDependencyMap(raw: unknown): PluginDependencySpecMap {
|
||||
if (!raw || typeof raw !== "object" || Array.isArray(raw)) {
|
||||
return {};
|
||||
@@ -49,9 +66,14 @@ export function normalizePluginDependencySpecs(params: {
|
||||
dependencies: PluginDependencySpecMap;
|
||||
optionalDependencies: PluginDependencySpecMap;
|
||||
} {
|
||||
const dependencies = normalizeDependencyMap(params.dependencies);
|
||||
const optionalDependencies = normalizeDependencyMap(params.optionalDependencies);
|
||||
for (const name of Object.keys(optionalDependencies)) {
|
||||
delete dependencies[name];
|
||||
}
|
||||
return {
|
||||
dependencies: normalizeDependencyMap(params.dependencies),
|
||||
optionalDependencies: normalizeDependencyMap(params.optionalDependencies),
|
||||
dependencies,
|
||||
optionalDependencies,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -142,3 +164,46 @@ export function buildPluginDependencyStatus(params: {
|
||||
optionalDependencies,
|
||||
};
|
||||
}
|
||||
|
||||
/** Projects missing required dependencies consistently across cold plugin status surfaces. */
|
||||
export function projectPluginDependencyHealth<T extends PluginDependencyHealthRegistry>(
|
||||
registry: T,
|
||||
): T {
|
||||
const diagnostics = [...registry.diagnostics];
|
||||
const plugins = registry.plugins.map((plugin) => {
|
||||
const status = plugin.dependencyStatus;
|
||||
if (!plugin.enabled || status?.requiredInstalled !== false) {
|
||||
return plugin;
|
||||
}
|
||||
const message =
|
||||
`Plugin "${plugin.id}" cannot load because required dependencies are missing: ` +
|
||||
`${status.missing.join(", ")}. Install the plugin dependencies or reinstall/update the ` +
|
||||
"plugin, then restart the Gateway.";
|
||||
const existingDiagnosticIndex = diagnostics.findIndex(
|
||||
(entry) => entry.level === "error" && entry.pluginId === plugin.id,
|
||||
);
|
||||
if (existingDiagnosticIndex === -1) {
|
||||
diagnostics.push({ level: "error", pluginId: plugin.id, source: plugin.source, message });
|
||||
} else {
|
||||
const existingDiagnostic = diagnostics[existingDiagnosticIndex];
|
||||
if (existingDiagnostic && !existingDiagnostic.message.includes(message)) {
|
||||
diagnostics[existingDiagnosticIndex] = {
|
||||
...existingDiagnostic,
|
||||
message: `${existingDiagnostic.message}\n${message}`,
|
||||
};
|
||||
}
|
||||
}
|
||||
if (plugin.status === "error") {
|
||||
const existingError = plugin.error;
|
||||
return {
|
||||
...plugin,
|
||||
error:
|
||||
existingError && !existingError.includes(message)
|
||||
? `${existingError}\n${message}`
|
||||
: (existingError ?? message),
|
||||
};
|
||||
}
|
||||
return { ...plugin, status: "error" as const, error: message };
|
||||
});
|
||||
return { ...registry, plugins, diagnostics };
|
||||
}
|
||||
|
||||
@@ -9,7 +9,10 @@ import {
|
||||
} from "./plugin-registry.js";
|
||||
import { createEmptyPluginRegistry } from "./registry-empty.js";
|
||||
import type { PluginRecord, PluginRegistry } from "./registry-types.js";
|
||||
import { buildPluginDependencyStatus } from "./status-dependencies-core.js";
|
||||
import {
|
||||
buildPluginDependencyStatus,
|
||||
projectPluginDependencyHealth,
|
||||
} from "./status-dependencies-core.js";
|
||||
import type { PluginLogger } from "./types.js";
|
||||
|
||||
/** Control-plane plugin status shape used by `openclaw plugins status` style surfaces. */
|
||||
@@ -150,7 +153,7 @@ export function buildPluginRegistrySnapshotReport(
|
||||
workspaceDir: params?.workspaceDir,
|
||||
});
|
||||
const manifestByPluginId = metadataSnapshot.byPluginId;
|
||||
return {
|
||||
return projectPluginDependencyHealth({
|
||||
workspaceDir: params?.workspaceDir,
|
||||
...createEmptyPluginRegistry(),
|
||||
plugins: result.snapshot.plugins.map((plugin) =>
|
||||
@@ -159,5 +162,5 @@ export function buildPluginRegistrySnapshotReport(
|
||||
diagnostics: [...result.snapshot.diagnostics],
|
||||
registrySource: result.source,
|
||||
registryDiagnostics: result.diagnostics,
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
@@ -0,0 +1,144 @@
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
buildPluginDependencyStatus,
|
||||
projectPluginDependencyHealth,
|
||||
} from "./status-dependencies-core.js";
|
||||
import { buildPluginDiagnosticsReport, buildPluginSnapshotReport } from "./status.js";
|
||||
import { createPluginLoadResult, createPluginRecord } from "./status.test-fixtures.js";
|
||||
import {
|
||||
createColdPluginConfig,
|
||||
createColdPluginFixture,
|
||||
createColdPluginHermeticEnv,
|
||||
} from "./test-helpers/cold-plugin-fixtures.js";
|
||||
import { cleanupTrackedTempDirs, makeTrackedTempDir } from "./test-helpers/fs-fixtures.js";
|
||||
|
||||
const loaderState = vi.hoisted(() => ({
|
||||
registry: undefined as
|
||||
| ReturnType<typeof import("./status.test-fixtures.js").createPluginLoadResult>
|
||||
| undefined,
|
||||
}));
|
||||
|
||||
vi.mock("./loader.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("./loader.js")>()),
|
||||
loadOpenClawPlugins: () => loaderState.registry,
|
||||
}));
|
||||
|
||||
vi.mock("./runtime/metadata-registry-loader.js", async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import("./runtime/metadata-registry-loader.js")>()),
|
||||
loadPluginMetadataRegistrySnapshot: () => loaderState.registry,
|
||||
}));
|
||||
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
afterEach(() => {
|
||||
loaderState.registry = undefined;
|
||||
cleanupTrackedTempDirs(tempDirs);
|
||||
});
|
||||
|
||||
function createDependencyHealthRegistry(
|
||||
pluginId: string,
|
||||
overrides: Omit<Parameters<typeof createPluginRecord>[0], "id"> = {},
|
||||
) {
|
||||
return createPluginLoadResult({
|
||||
plugins: [
|
||||
createPluginRecord({
|
||||
id: pluginId,
|
||||
dependencyStatus: buildPluginDependencyStatus({
|
||||
dependencies: { "missing-runtime": "1.0.0" },
|
||||
}),
|
||||
...overrides,
|
||||
}),
|
||||
],
|
||||
});
|
||||
}
|
||||
|
||||
function createDependencyHealthFixture() {
|
||||
const rootDir = makeTrackedTempDir("openclaw-plugin-dependency-health", tempDirs);
|
||||
const pluginRoot = path.join(rootDir, "plugin");
|
||||
const bundledRoot = path.join(rootDir, "bundled");
|
||||
fs.mkdirSync(pluginRoot);
|
||||
fs.mkdirSync(bundledRoot);
|
||||
const fixture = createColdPluginFixture({
|
||||
rootDir: pluginRoot,
|
||||
pluginId: "missing-dependency-plugin",
|
||||
packageJson: {
|
||||
dependencies: { "missing-runtime": "1.0.0", "optional-runtime": "1.0.0" },
|
||||
optionalDependencies: { "optional-runtime": "2.0.0" },
|
||||
},
|
||||
});
|
||||
return {
|
||||
fixture,
|
||||
reportParams: {
|
||||
config: createColdPluginConfig(pluginRoot, fixture.pluginId),
|
||||
env: createColdPluginHermeticEnv(rootDir, { bundledPluginsDir: bundledRoot }),
|
||||
workspaceDir: rootDir,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe("plugin dependency health projection", () => {
|
||||
it.each([
|
||||
{ mode: "snapshot", load: buildPluginSnapshotReport },
|
||||
{ mode: "runtime", load: buildPluginDiagnosticsReport },
|
||||
])("surfaces missing required plugin dependencies in $mode inspections", ({ load }) => {
|
||||
const { fixture, reportParams } = createDependencyHealthFixture();
|
||||
loaderState.registry = createDependencyHealthRegistry(fixture.pluginId);
|
||||
|
||||
const report = load(reportParams);
|
||||
|
||||
expect(report.plugins[0]).toEqual(
|
||||
expect.objectContaining({
|
||||
status: "error",
|
||||
error: expect.stringContaining("missing-runtime"),
|
||||
}),
|
||||
);
|
||||
expect(report.diagnostics).toContainEqual(
|
||||
expect.objectContaining({
|
||||
level: "error",
|
||||
pluginId: "missing-dependency-plugin",
|
||||
message: expect.stringContaining("reinstall/update the plugin"),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("uses prepared manifest facts when runtime records omit dependency metadata", () => {
|
||||
const { fixture, reportParams } = createDependencyHealthFixture();
|
||||
loaderState.registry = createDependencyHealthRegistry(fixture.pluginId, {
|
||||
dependencyStatus: undefined,
|
||||
});
|
||||
|
||||
const report = buildPluginDiagnosticsReport(reportParams);
|
||||
|
||||
expect(report.plugins[0]?.dependencyStatus).toEqual(
|
||||
expect.objectContaining({
|
||||
requiredInstalled: false,
|
||||
missing: ["missing-runtime"],
|
||||
missingOptional: ["optional-runtime"],
|
||||
}),
|
||||
);
|
||||
expect(report.plugins[0]?.status).toBe("error");
|
||||
});
|
||||
|
||||
it("preserves an existing error diagnostic when dependency health also fails", () => {
|
||||
const registry = createDependencyHealthRegistry("existing-plugin-error", {
|
||||
status: "error",
|
||||
error: "Cannot find module 'missing-runtime'",
|
||||
});
|
||||
registry.diagnostics.push({
|
||||
level: "error",
|
||||
pluginId: "existing-plugin-error",
|
||||
message: "already recorded",
|
||||
});
|
||||
|
||||
const report = projectPluginDependencyHealth(registry);
|
||||
|
||||
expect(report.plugins[0]?.status).toBe("error");
|
||||
expect(report.plugins[0]?.error).toContain("Cannot find module 'missing-runtime'");
|
||||
expect(report.plugins[0]?.error).toContain("Install the plugin dependencies");
|
||||
expect(report.diagnostics).toHaveLength(1);
|
||||
expect(report.diagnostics[0]?.message).toContain("already recorded");
|
||||
expect(report.diagnostics[0]?.message).toContain("Install the plugin dependencies");
|
||||
});
|
||||
});
|
||||
@@ -5,7 +5,11 @@ import { afterEach, describe, expect, it } from "vitest";
|
||||
import { writePersistedInstalledPluginIndexSync } from "./installed-plugin-index-store.js";
|
||||
import { loadInstalledPluginIndex } from "./installed-plugin-index.js";
|
||||
import { refreshPluginRegistry } from "./plugin-registry.js";
|
||||
import { buildPluginRegistrySnapshotReport, buildPluginSnapshotReport } from "./status.js";
|
||||
import {
|
||||
buildPluginDiagnosticsReport,
|
||||
buildPluginRegistrySnapshotReport,
|
||||
buildPluginSnapshotReport,
|
||||
} from "./status.js";
|
||||
import {
|
||||
createColdPluginConfig,
|
||||
createColdPluginFixture,
|
||||
@@ -207,6 +211,18 @@ describe("buildPluginRegistrySnapshotReport", () => {
|
||||
});
|
||||
|
||||
const plugin = requirePlugin(report.plugins, "dependency-demo");
|
||||
expectFields(plugin, {
|
||||
status: "error",
|
||||
error:
|
||||
'Plugin "dependency-demo" cannot load because required dependencies are missing: missing-required. Install the plugin dependencies or reinstall/update the plugin, then restart the Gateway.',
|
||||
});
|
||||
expect(report.diagnostics).toContainEqual({
|
||||
level: "error",
|
||||
pluginId: "dependency-demo",
|
||||
source: fs.realpathSync(fixture.runtimeSource),
|
||||
message:
|
||||
'Plugin "dependency-demo" cannot load because required dependencies are missing: missing-required. Install the plugin dependencies or reinstall/update the plugin, then restart the Gateway.',
|
||||
});
|
||||
const dependencyStatus = requireRecord(plugin.dependencyStatus);
|
||||
expectFields(dependencyStatus, {
|
||||
hasDependencies: true,
|
||||
@@ -241,6 +257,101 @@ describe("buildPluginRegistrySnapshotReport", () => {
|
||||
expect(isColdPluginRuntimeLoaded(fixture)).toBe(false);
|
||||
});
|
||||
|
||||
it("honors npm optional dependency precedence without reporting a false required failure", () => {
|
||||
const fixture = createColdPluginFixture({
|
||||
rootDir: makeTempDir(),
|
||||
pluginId: "optional-dependency-demo",
|
||||
packageJson: {
|
||||
dependencies: { "optional-runtime": "1.0.0" },
|
||||
optionalDependencies: { "optional-runtime": "2.0.0" },
|
||||
},
|
||||
});
|
||||
|
||||
const report = buildPluginRegistrySnapshotReport({
|
||||
config: createColdPluginConfig(fixture.rootDir, fixture.pluginId),
|
||||
});
|
||||
const plugin = requirePlugin(report.plugins, fixture.pluginId);
|
||||
|
||||
expectFields(plugin, { status: "loaded" });
|
||||
expectFields(requireRecord(plugin.dependencyStatus), {
|
||||
requiredInstalled: true,
|
||||
optionalInstalled: false,
|
||||
missing: [],
|
||||
missingOptional: ["optional-runtime"],
|
||||
dependencies: [],
|
||||
});
|
||||
expect(report.diagnostics).not.toContainEqual(
|
||||
expect.objectContaining({ pluginId: fixture.pluginId, level: "error" }),
|
||||
);
|
||||
expect(isColdPluginRuntimeLoaded(fixture)).toBe(false);
|
||||
});
|
||||
|
||||
it("keeps disabled plugins with missing required dependencies diagnostic-free", () => {
|
||||
const fixture = createColdPluginFixture({
|
||||
rootDir: makeTempDir(),
|
||||
pluginId: "disabled-dependency-demo",
|
||||
packageJson: { dependencies: { "missing-required": "1.0.0" } },
|
||||
});
|
||||
|
||||
const report = buildPluginRegistrySnapshotReport({
|
||||
config: {
|
||||
plugins: {
|
||||
load: { paths: [fixture.rootDir] },
|
||||
entries: { [fixture.pluginId]: { enabled: false } },
|
||||
},
|
||||
},
|
||||
});
|
||||
const plugin = requirePlugin(report.plugins, fixture.pluginId);
|
||||
|
||||
expectFields(plugin, { enabled: false, status: "disabled" });
|
||||
expect(plugin.error).toBeUndefined();
|
||||
expect(requireRecord(plugin.dependencyStatus).missing).toEqual(["missing-required"]);
|
||||
expect(report.diagnostics).not.toContainEqual(
|
||||
expect.objectContaining({ pluginId: fixture.pluginId, level: "error" }),
|
||||
);
|
||||
expect(isColdPluginRuntimeLoaded(fixture)).toBe(false);
|
||||
});
|
||||
|
||||
it("preserves the real runtime import error while explaining missing dependencies", () => {
|
||||
const rootDir = makeTempDir();
|
||||
const bundledRoot = makeTempDir();
|
||||
const fixture = createColdPluginFixture({
|
||||
rootDir,
|
||||
pluginId: "failed-runtime-dependency-demo",
|
||||
packageJson: {
|
||||
dependencies: { "missing-runtime": "1.0.0", "optional-runtime": "1.0.0" },
|
||||
optionalDependencies: { "optional-runtime": "2.0.0" },
|
||||
},
|
||||
});
|
||||
fs.writeFileSync(
|
||||
fixture.runtimeSource,
|
||||
`require("node:fs").writeFileSync(${JSON.stringify(fixture.runtimeMarker)}, "loaded");\n` +
|
||||
'require("missing-runtime");\n',
|
||||
"utf8",
|
||||
);
|
||||
|
||||
const report = buildPluginDiagnosticsReport({
|
||||
config: createColdPluginConfig(rootDir, fixture.pluginId),
|
||||
workspaceDir: rootDir,
|
||||
env: createColdPluginHermeticEnv(rootDir, { bundledPluginsDir: bundledRoot }),
|
||||
logger: { info() {}, warn() {}, error() {}, debug() {} },
|
||||
});
|
||||
const plugin = requirePlugin(report.plugins, fixture.pluginId);
|
||||
const diagnostics = report.diagnostics.filter((entry) => entry.pluginId === fixture.pluginId);
|
||||
|
||||
expectFields(plugin, { status: "error" });
|
||||
expect(String(plugin.error)).toContain("Cannot find module");
|
||||
expect(String(plugin.error)).toContain("Install the plugin dependencies");
|
||||
expectFields(requireRecord(plugin.dependencyStatus), {
|
||||
missing: ["missing-runtime"],
|
||||
missingOptional: ["optional-runtime"],
|
||||
});
|
||||
expect(diagnostics).toHaveLength(1);
|
||||
expect(diagnostics[0]?.message).toContain("Cannot find module");
|
||||
expect(diagnostics[0]?.message).toContain("Install the plugin dependencies");
|
||||
expect(isColdPluginRuntimeLoaded(fixture)).toBe(true);
|
||||
});
|
||||
|
||||
it("replays persisted list metadata without importing plugin runtime", async () => {
|
||||
const fixture = createColdPluginFixture({
|
||||
rootDir: makeTempDir(),
|
||||
|
||||
+11
-6
@@ -32,7 +32,10 @@ import {
|
||||
resolvePluginRuntimeLoadContext,
|
||||
} from "./runtime/load-context.js";
|
||||
import { loadPluginMetadataRegistrySnapshot } from "./runtime/metadata-registry-loader.js";
|
||||
import { buildPluginDependencyStatus } from "./status-dependencies-core.js";
|
||||
import {
|
||||
buildPluginDependencyStatus,
|
||||
projectPluginDependencyHealth,
|
||||
} from "./status-dependencies-core.js";
|
||||
import type { PluginHookName, PluginLogger } from "./types.js";
|
||||
|
||||
export type PluginStatusReport = PluginRegistry & {
|
||||
@@ -234,6 +237,9 @@ function buildPluginReport(
|
||||
...baseContext,
|
||||
workspaceDir,
|
||||
};
|
||||
const manifestByPluginId =
|
||||
metadataSnapshot?.byPluginId ??
|
||||
new Map(context.manifestRegistry?.plugins.map((manifest) => [manifest.id, manifest]) ?? []);
|
||||
const config = context.config;
|
||||
|
||||
// Apply bundled-provider allowlist compat so that `plugins list` and `doctor`
|
||||
@@ -303,7 +309,7 @@ function buildPluginReport(
|
||||
...listImportedBundledPluginFacadeIds(),
|
||||
]);
|
||||
|
||||
return {
|
||||
return projectPluginDependencyHealth({
|
||||
workspaceDir,
|
||||
...registry,
|
||||
plugins: registry.plugins.map((plugin) =>
|
||||
@@ -314,13 +320,12 @@ function buildPluginReport(
|
||||
plugin.dependencyStatus ??
|
||||
buildPluginDependencyStatus({
|
||||
rootDir: plugin.rootDir,
|
||||
dependencies: metadataSnapshot?.byPluginId.get(plugin.id)?.packageDependencies,
|
||||
optionalDependencies: metadataSnapshot?.byPluginId.get(plugin.id)
|
||||
?.packageOptionalDependencies,
|
||||
dependencies: manifestByPluginId.get(plugin.id)?.packageDependencies,
|
||||
optionalDependencies: manifestByPluginId.get(plugin.id)?.packageOptionalDependencies,
|
||||
}),
|
||||
}),
|
||||
),
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
export function buildPluginSnapshotReport(params?: PluginReportParams): PluginStatusReport {
|
||||
|
||||
Reference in New Issue
Block a user