mirror of https://github.com/openclaw/openclaw.git
fix(plugins): prefer explicit installs over bundled duplicates (#46722)
* fix(plugins): prefer explicit installs over bundled duplicates * test(feishu): mock structured card sends in outbound tests * fix(plugins): align duplicate diagnostics with loader precedence
This commit is contained in:
parent
e3b7ff2f1f
commit
f00db91590
|
|
@ -19,6 +19,7 @@ Docs: https://docs.openclaw.ai
|
|||
- Control UI/chat sessions: show human-readable labels in the grouped session dropdown again, keep unique scoped fallbacks when metadata is missing, and disambiguate duplicate labels only when needed. (#45130) thanks @luzhidong.
|
||||
- Configure/startup: move outbound send-deps resolution into a lightweight helper so `openclaw configure` no longer stalls after the banner while eagerly loading channel plugins. (#46301) thanks @scoootscooob.
|
||||
- Zalo Personal/group gating: stop reapplying `dmPolicy.allowFrom` as a sender gate for already-allowlisted groups when `groupAllowFrom` is unset, so any member of an allowed group can trigger replies while DMs stay restricted. (#40146)
|
||||
- Plugins/install precedence: keep bundled plugins ahead of auto-discovered globals by default, but let an explicitly installed plugin record win its own duplicate-id tie so installed channel plugins load from `~/.openclaw/extensions` after `openclaw plugins install`.
|
||||
|
||||
### Fixes
|
||||
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
|
|||
const sendMediaFeishuMock = vi.hoisted(() => vi.fn());
|
||||
const sendMessageFeishuMock = vi.hoisted(() => vi.fn());
|
||||
const sendMarkdownCardFeishuMock = vi.hoisted(() => vi.fn());
|
||||
const sendStructuredCardFeishuMock = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("./media.js", () => ({
|
||||
sendMediaFeishu: sendMediaFeishuMock,
|
||||
|
|
@ -14,6 +15,7 @@ vi.mock("./media.js", () => ({
|
|||
vi.mock("./send.js", () => ({
|
||||
sendMessageFeishu: sendMessageFeishuMock,
|
||||
sendMarkdownCardFeishu: sendMarkdownCardFeishuMock,
|
||||
sendStructuredCardFeishu: sendStructuredCardFeishuMock,
|
||||
}));
|
||||
|
||||
vi.mock("./runtime.js", () => ({
|
||||
|
|
@ -33,6 +35,7 @@ function resetOutboundMocks() {
|
|||
vi.clearAllMocks();
|
||||
sendMessageFeishuMock.mockResolvedValue({ messageId: "text_msg" });
|
||||
sendMarkdownCardFeishuMock.mockResolvedValue({ messageId: "card_msg" });
|
||||
sendStructuredCardFeishuMock.mockResolvedValue({ messageId: "card_msg" });
|
||||
sendMediaFeishuMock.mockResolvedValue({ messageId: "media_msg" });
|
||||
}
|
||||
|
||||
|
|
@ -132,7 +135,7 @@ describe("feishuOutbound.sendText local-image auto-convert", () => {
|
|||
accountId: "main",
|
||||
});
|
||||
|
||||
expect(sendMarkdownCardFeishuMock).toHaveBeenCalledWith(
|
||||
expect(sendStructuredCardFeishuMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
to: "chat_1",
|
||||
text: "| a | b |\n| - | - |",
|
||||
|
|
@ -207,7 +210,7 @@ describe("feishuOutbound.sendText replyToId forwarding", () => {
|
|||
);
|
||||
});
|
||||
|
||||
it("forwards replyToId to sendMarkdownCardFeishu when renderMode=card", async () => {
|
||||
it("forwards replyToId to sendStructuredCardFeishu when renderMode=card", async () => {
|
||||
await sendText({
|
||||
cfg: {
|
||||
channels: {
|
||||
|
|
@ -222,7 +225,7 @@ describe("feishuOutbound.sendText replyToId forwarding", () => {
|
|||
accountId: "main",
|
||||
});
|
||||
|
||||
expect(sendMarkdownCardFeishuMock).toHaveBeenCalledWith(
|
||||
expect(sendStructuredCardFeishuMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
replyToMessageId: "om_reply_target",
|
||||
}),
|
||||
|
|
|
|||
|
|
@ -1543,6 +1543,54 @@ describe("loadOpenClawPlugins", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("prefers an explicitly installed global plugin over a bundled duplicate", () => {
|
||||
const bundledDir = makeTempDir();
|
||||
writePlugin({
|
||||
id: "zalouser",
|
||||
body: `module.exports = { id: "zalouser", register() {} };`,
|
||||
dir: bundledDir,
|
||||
filename: "index.cjs",
|
||||
});
|
||||
process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = bundledDir;
|
||||
|
||||
const stateDir = makeTempDir();
|
||||
withEnv({ OPENCLAW_STATE_DIR: stateDir, CLAWDBOT_STATE_DIR: undefined }, () => {
|
||||
const globalDir = path.join(stateDir, "extensions", "zalouser");
|
||||
mkdirSafe(globalDir);
|
||||
writePlugin({
|
||||
id: "zalouser",
|
||||
body: `module.exports = { id: "zalouser", register() {} };`,
|
||||
dir: globalDir,
|
||||
filename: "index.cjs",
|
||||
});
|
||||
|
||||
const registry = loadOpenClawPlugins({
|
||||
cache: false,
|
||||
config: {
|
||||
plugins: {
|
||||
allow: ["zalouser"],
|
||||
installs: {
|
||||
zalouser: {
|
||||
source: "npm",
|
||||
installPath: globalDir,
|
||||
},
|
||||
},
|
||||
entries: {
|
||||
zalouser: { enabled: true },
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const entries = registry.plugins.filter((entry) => entry.id === "zalouser");
|
||||
const loaded = entries.find((entry) => entry.status === "loaded");
|
||||
const overridden = entries.find((entry) => entry.status === "disabled");
|
||||
expect(loaded?.origin).toBe("global");
|
||||
expect(overridden?.origin).toBe("bundled");
|
||||
expect(overridden?.error).toContain("overridden by global plugin");
|
||||
});
|
||||
});
|
||||
|
||||
it("warns when plugins.allow is empty and non-bundled plugins are discoverable", () => {
|
||||
useNoBundledPlugins();
|
||||
const plugin = writePlugin({
|
||||
|
|
|
|||
|
|
@ -453,6 +453,78 @@ function isTrackedByProvenance(params: {
|
|||
return matchesPathMatcher(params.index.loadPathMatcher, sourcePath);
|
||||
}
|
||||
|
||||
function matchesExplicitInstallRule(params: {
|
||||
pluginId: string;
|
||||
source: string;
|
||||
index: PluginProvenanceIndex;
|
||||
env: NodeJS.ProcessEnv;
|
||||
}): boolean {
|
||||
const sourcePath = resolveUserPath(params.source, params.env);
|
||||
const installRule = params.index.installRules.get(params.pluginId);
|
||||
if (!installRule || installRule.trackedWithoutPaths) {
|
||||
return false;
|
||||
}
|
||||
return matchesPathMatcher(installRule.matcher, sourcePath);
|
||||
}
|
||||
|
||||
function resolveCandidateDuplicateRank(params: {
|
||||
candidate: ReturnType<typeof discoverOpenClawPlugins>["candidates"][number];
|
||||
manifestByRoot: Map<string, ReturnType<typeof loadPluginManifestRegistry>["plugins"][number]>;
|
||||
provenance: PluginProvenanceIndex;
|
||||
env: NodeJS.ProcessEnv;
|
||||
}): number {
|
||||
const manifestRecord = params.manifestByRoot.get(params.candidate.rootDir);
|
||||
const pluginId = manifestRecord?.id;
|
||||
const isExplicitInstall =
|
||||
params.candidate.origin === "global" &&
|
||||
pluginId !== undefined &&
|
||||
matchesExplicitInstallRule({
|
||||
pluginId,
|
||||
source: params.candidate.source,
|
||||
index: params.provenance,
|
||||
env: params.env,
|
||||
});
|
||||
|
||||
switch (params.candidate.origin) {
|
||||
case "config":
|
||||
return 0;
|
||||
case "workspace":
|
||||
return 1;
|
||||
case "global":
|
||||
return isExplicitInstall ? 2 : 4;
|
||||
case "bundled":
|
||||
return 3;
|
||||
}
|
||||
}
|
||||
|
||||
function compareDuplicateCandidateOrder(params: {
|
||||
left: ReturnType<typeof discoverOpenClawPlugins>["candidates"][number];
|
||||
right: ReturnType<typeof discoverOpenClawPlugins>["candidates"][number];
|
||||
manifestByRoot: Map<string, ReturnType<typeof loadPluginManifestRegistry>["plugins"][number]>;
|
||||
provenance: PluginProvenanceIndex;
|
||||
env: NodeJS.ProcessEnv;
|
||||
}): number {
|
||||
const leftPluginId = params.manifestByRoot.get(params.left.rootDir)?.id;
|
||||
const rightPluginId = params.manifestByRoot.get(params.right.rootDir)?.id;
|
||||
if (!leftPluginId || leftPluginId !== rightPluginId) {
|
||||
return 0;
|
||||
}
|
||||
return (
|
||||
resolveCandidateDuplicateRank({
|
||||
candidate: params.left,
|
||||
manifestByRoot: params.manifestByRoot,
|
||||
provenance: params.provenance,
|
||||
env: params.env,
|
||||
}) -
|
||||
resolveCandidateDuplicateRank({
|
||||
candidate: params.right,
|
||||
manifestByRoot: params.manifestByRoot,
|
||||
provenance: params.provenance,
|
||||
env: params.env,
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
function warnWhenAllowlistIsOpen(params: {
|
||||
logger: PluginLogger;
|
||||
pluginsEnabled: boolean;
|
||||
|
|
@ -644,13 +716,22 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi
|
|||
const manifestByRoot = new Map(
|
||||
manifestRegistry.plugins.map((record) => [record.rootDir, record]),
|
||||
);
|
||||
const orderedCandidates = [...discovery.candidates].toSorted((left, right) => {
|
||||
return compareDuplicateCandidateOrder({
|
||||
left,
|
||||
right,
|
||||
manifestByRoot,
|
||||
provenance,
|
||||
env,
|
||||
});
|
||||
});
|
||||
|
||||
const seenIds = new Map<string, PluginRecord["origin"]>();
|
||||
const memorySlot = normalized.slots.memory;
|
||||
let selectedMemoryPluginId: string | null = null;
|
||||
let memorySlotMatched = false;
|
||||
|
||||
for (const candidate of discovery.candidates) {
|
||||
for (const candidate of orderedCandidates) {
|
||||
const manifestRecord = manifestByRoot.get(candidate.rootDir);
|
||||
if (!manifestRecord) {
|
||||
continue;
|
||||
|
|
|
|||
|
|
@ -155,6 +155,76 @@ describe("loadPluginManifestRegistry", () => {
|
|||
expect(countDuplicateWarnings(loadRegistry(candidates))).toBe(1);
|
||||
});
|
||||
|
||||
it("reports explicit installed globals as the effective duplicate winner", () => {
|
||||
const bundledDir = makeTempDir();
|
||||
const globalDir = makeTempDir();
|
||||
const manifest = { id: "zalouser", configSchema: { type: "object" } };
|
||||
writeManifest(bundledDir, manifest);
|
||||
writeManifest(globalDir, manifest);
|
||||
|
||||
const registry = loadPluginManifestRegistry({
|
||||
cache: false,
|
||||
config: {
|
||||
plugins: {
|
||||
installs: {
|
||||
zalouser: {
|
||||
source: "npm",
|
||||
installPath: globalDir,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
candidates: [
|
||||
createPluginCandidate({
|
||||
idHint: "zalouser",
|
||||
rootDir: bundledDir,
|
||||
origin: "bundled",
|
||||
}),
|
||||
createPluginCandidate({
|
||||
idHint: "zalouser",
|
||||
rootDir: globalDir,
|
||||
origin: "global",
|
||||
}),
|
||||
],
|
||||
});
|
||||
|
||||
expect(
|
||||
registry.diagnostics.some((diag) =>
|
||||
diag.message.includes("bundled plugin will be overridden by global plugin"),
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("reports bundled plugins as the duplicate winner for auto-discovered globals", () => {
|
||||
const bundledDir = makeTempDir();
|
||||
const globalDir = makeTempDir();
|
||||
const manifest = { id: "feishu", configSchema: { type: "object" } };
|
||||
writeManifest(bundledDir, manifest);
|
||||
writeManifest(globalDir, manifest);
|
||||
|
||||
const registry = loadPluginManifestRegistry({
|
||||
cache: false,
|
||||
candidates: [
|
||||
createPluginCandidate({
|
||||
idHint: "feishu",
|
||||
rootDir: bundledDir,
|
||||
origin: "bundled",
|
||||
}),
|
||||
createPluginCandidate({
|
||||
idHint: "feishu",
|
||||
rootDir: globalDir,
|
||||
origin: "global",
|
||||
}),
|
||||
],
|
||||
});
|
||||
|
||||
expect(
|
||||
registry.diagnostics.some((diag) =>
|
||||
diag.message.includes("global plugin will be overridden by bundled plugin"),
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("suppresses duplicate warning when candidates share the same physical directory via symlink", () => {
|
||||
const realDir = makeTempDir();
|
||||
const manifest = { id: "feishu", configSchema: { type: "object" } };
|
||||
|
|
|
|||
|
|
@ -1,9 +1,10 @@
|
|||
import fs from "node:fs";
|
||||
import type { OpenClawConfig } from "../config/config.js";
|
||||
import { resolveUserPath } from "../utils.js";
|
||||
import { normalizePluginsConfig, type NormalizedPluginsConfig } from "./config-state.js";
|
||||
import { discoverOpenClawPlugins, type PluginCandidate } from "./discovery.js";
|
||||
import { loadPluginManifest, type PluginManifest } from "./manifest.js";
|
||||
import { safeRealpathSync } from "./path-safety.js";
|
||||
import { isPathInside, safeRealpathSync } from "./path-safety.js";
|
||||
import { resolvePluginCacheInputs } from "./roots.js";
|
||||
import type { PluginConfigUiHint, PluginDiagnostic, PluginKind, PluginOrigin } from "./types.js";
|
||||
|
||||
|
|
@ -12,7 +13,7 @@ type SeenIdEntry = {
|
|||
recordIndex: number;
|
||||
};
|
||||
|
||||
// Precedence: config > workspace > global > bundled
|
||||
// Precedence: config > workspace > explicit-install global > bundled > auto-discovered global
|
||||
const PLUGIN_ORIGIN_RANK: Readonly<Record<PluginOrigin, number>> = {
|
||||
config: 0,
|
||||
workspace: 1,
|
||||
|
|
@ -135,6 +136,50 @@ function buildRecord(params: {
|
|||
};
|
||||
}
|
||||
|
||||
function matchesInstalledPluginRecord(params: {
|
||||
pluginId: string;
|
||||
candidate: PluginCandidate;
|
||||
config?: OpenClawConfig;
|
||||
env: NodeJS.ProcessEnv;
|
||||
}): boolean {
|
||||
if (params.candidate.origin !== "global") {
|
||||
return false;
|
||||
}
|
||||
const record = params.config?.plugins?.installs?.[params.pluginId];
|
||||
if (!record) {
|
||||
return false;
|
||||
}
|
||||
const candidateSource = resolveUserPath(params.candidate.source, params.env);
|
||||
const trackedPaths = [record.installPath, record.sourcePath]
|
||||
.filter((entry): entry is string => typeof entry === "string" && entry.trim().length > 0)
|
||||
.map((entry) => resolveUserPath(entry, params.env));
|
||||
if (trackedPaths.length === 0) {
|
||||
return false;
|
||||
}
|
||||
return trackedPaths.some((trackedPath) => {
|
||||
return candidateSource === trackedPath || isPathInside(trackedPath, candidateSource);
|
||||
});
|
||||
}
|
||||
|
||||
function resolveDuplicatePrecedenceRank(params: {
|
||||
pluginId: string;
|
||||
candidate: PluginCandidate;
|
||||
config?: OpenClawConfig;
|
||||
env: NodeJS.ProcessEnv;
|
||||
}): number {
|
||||
if (params.candidate.origin === "global") {
|
||||
return matchesInstalledPluginRecord({
|
||||
pluginId: params.pluginId,
|
||||
candidate: params.candidate,
|
||||
config: params.config,
|
||||
env: params.env,
|
||||
})
|
||||
? 2
|
||||
: 4;
|
||||
}
|
||||
return PLUGIN_ORIGIN_RANK[params.candidate.origin];
|
||||
}
|
||||
|
||||
export function loadPluginManifestRegistry(params: {
|
||||
config?: OpenClawConfig;
|
||||
workspaceDir?: string;
|
||||
|
|
@ -237,7 +282,21 @@ export function loadPluginManifestRegistry(params: {
|
|||
level: "warn",
|
||||
pluginId: manifest.id,
|
||||
source: candidate.source,
|
||||
message: `duplicate plugin id detected; later plugin may be overridden (${candidate.source})`,
|
||||
message:
|
||||
resolveDuplicatePrecedenceRank({
|
||||
pluginId: manifest.id,
|
||||
candidate,
|
||||
config,
|
||||
env,
|
||||
}) <
|
||||
resolveDuplicatePrecedenceRank({
|
||||
pluginId: manifest.id,
|
||||
candidate: existing.candidate,
|
||||
config,
|
||||
env,
|
||||
})
|
||||
? `duplicate plugin id detected; ${existing.candidate.origin} plugin will be overridden by ${candidate.origin} plugin (${candidate.source})`
|
||||
: `duplicate plugin id detected; ${candidate.origin} plugin will be overridden by ${existing.candidate.origin} plugin (${candidate.source})`,
|
||||
});
|
||||
} else {
|
||||
seenIds.set(manifest.id, { candidate, recordIndex: records.length });
|
||||
|
|
|
|||
Loading…
Reference in New Issue