mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 05:30:42 +00:00
fix(plugins): prefer higher-precedence manifests for duplicate plugin ids
Keep only the highest-precedence manifest when distinct discovered plugins share an id, while preserving the newer installed-global precedence behavior on main. Lower-precedence duplicates now warn against the ignored manifest source instead of loading as disabled plugin entries. Thanks @Tortes.
This commit is contained in:
@@ -13,6 +13,7 @@ Docs: https://docs.openclaw.ai
|
||||
### Fixes
|
||||
|
||||
- Plugins/memory: preserve the active memory capability when read-only snapshot plugin loads run, so status and provider discovery paths no longer wipe memory public artifacts. (#69219) Thanks @zeroaltitude.
|
||||
- Plugins: keep only the highest-precedence manifest when distinct discovered plugins share an id, so lower-precedence global or workspace duplicates no longer load beside bundled or config-selected plugins. (#41626) Thanks @Tortes.
|
||||
- fix(security): block MINIMAX_API_HOST workspace env injection and remove env-driven URL routing [AI-assisted]. (#67300) Thanks @pgondhi987.
|
||||
- Cron/delivery: treat explicit `delivery.mode: "none"` runs as not requested even if the runner reports `delivered: false`, so no-delivery cron jobs no longer persist false delivery failures or errors. (#69285) Thanks @matsuri1987.
|
||||
- Plugins/install: repair active and default-enabled bundled plugin runtime dependencies before import in packaged installs, so bundled Discord, WhatsApp, Slack, Telegram, and provider plugins work without putting their dependency trees in core.
|
||||
|
||||
@@ -429,13 +429,22 @@ function expectPluginSourcePrecedence(
|
||||
},
|
||||
) {
|
||||
const entries = registry.plugins.filter((entry) => entry.id === scenario.pluginId);
|
||||
const loaded = entries.find((entry) => entry.status === "loaded");
|
||||
const overridden = entries.find((entry) => entry.status === "disabled");
|
||||
expect(entries, scenario.label).toHaveLength(1);
|
||||
const loaded = entries[0];
|
||||
expect(loaded?.origin, scenario.label).toBe(scenario.expectedLoadedOrigin);
|
||||
expect(overridden?.origin, scenario.label).toBe(scenario.expectedDisabledOrigin);
|
||||
if (scenario.expectedDisabledError) {
|
||||
expect(overridden?.error, scenario.label).toContain(scenario.expectedDisabledError);
|
||||
}
|
||||
expect(loaded?.status, scenario.label).toBe("loaded");
|
||||
const expectedWarning =
|
||||
scenario.expectedDisabledError ??
|
||||
`${scenario.expectedDisabledOrigin} plugin will be overridden by ${scenario.expectedLoadedOrigin} plugin`;
|
||||
expect(
|
||||
registry.diagnostics.some(
|
||||
(diag) =>
|
||||
diag.level === "warn" &&
|
||||
diag.pluginId === scenario.pluginId &&
|
||||
diag.message.includes(expectedWarning),
|
||||
),
|
||||
scenario.label,
|
||||
).toBe(true);
|
||||
}
|
||||
|
||||
function expectPluginOriginAndStatus(params: {
|
||||
|
||||
@@ -310,7 +310,7 @@ afterEach(() => {
|
||||
});
|
||||
|
||||
describe("loadPluginManifestRegistry", () => {
|
||||
it("emits duplicate warning for truly distinct plugins with same id", () => {
|
||||
it("keeps only the higher-precedence plugin for truly distinct duplicates", () => {
|
||||
const dirA = makeTempDir();
|
||||
const dirB = makeTempDir();
|
||||
const manifest = { id: "test-plugin", configSchema: { type: "object" } };
|
||||
@@ -330,7 +330,42 @@ describe("loadPluginManifestRegistry", () => {
|
||||
}),
|
||||
];
|
||||
|
||||
expect(countDuplicateWarnings(loadRegistry(candidates))).toBe(1);
|
||||
const registry = loadRegistry(candidates);
|
||||
expect(countDuplicateWarnings(registry)).toBe(1);
|
||||
expect(registry.plugins).toHaveLength(1);
|
||||
expect(registry.plugins[0]?.origin).toBe("bundled");
|
||||
expectRegistryDiagnosticContains(
|
||||
registry,
|
||||
"global plugin will be overridden by bundled plugin",
|
||||
);
|
||||
});
|
||||
|
||||
it("lets config-loaded plugins replace bundled duplicates", () => {
|
||||
const bundledDir = makeTempDir();
|
||||
const configDir = makeTempDir();
|
||||
const manifest = { id: "config-shadow", configSchema: { type: "object" } };
|
||||
writeManifest(bundledDir, manifest);
|
||||
writeManifest(configDir, manifest);
|
||||
|
||||
const registry = loadRegistry([
|
||||
createPluginCandidate({
|
||||
idHint: "config-shadow",
|
||||
rootDir: bundledDir,
|
||||
origin: "bundled",
|
||||
}),
|
||||
createPluginCandidate({
|
||||
idHint: "config-shadow",
|
||||
rootDir: configDir,
|
||||
origin: "config",
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(countDuplicateWarnings(registry)).toBe(1);
|
||||
expect(registry.plugins).toHaveLength(1);
|
||||
expect(registry.plugins[0]?.origin).toBe("config");
|
||||
const warning = registry.diagnostics.find((diag) => diag.pluginId === "config-shadow");
|
||||
expect(warning?.source).toBe(path.join(bundledDir, "index.ts"));
|
||||
expect(warning?.message).toContain(path.join(configDir, "index.ts"));
|
||||
});
|
||||
|
||||
it("reports explicit installed globals as the effective duplicate winner", () => {
|
||||
@@ -371,6 +406,8 @@ describe("loadPluginManifestRegistry", () => {
|
||||
diag.message.includes("bundled plugin will be overridden by global plugin"),
|
||||
),
|
||||
).toBe(true);
|
||||
expect(registry.plugins).toHaveLength(1);
|
||||
expect(registry.plugins[0]?.origin).toBe("global");
|
||||
});
|
||||
|
||||
it("preserves provider auth env metadata from plugin manifests", () => {
|
||||
@@ -787,6 +824,8 @@ describe("loadPluginManifestRegistry", () => {
|
||||
] as const)("$name", ({ registry: buildRegistry, expectedMessage }) => {
|
||||
const registry = buildRegistry();
|
||||
expectRegistryDiagnosticContains(registry, expectedMessage);
|
||||
expect(registry.plugins).toHaveLength(1);
|
||||
expect(registry.plugins[0]?.origin).toBe("bundled");
|
||||
});
|
||||
|
||||
it("suppresses duplicate warning when candidates share the same physical directory via symlink", () => {
|
||||
|
||||
@@ -581,6 +581,20 @@ export function loadPluginManifestRegistry(
|
||||
: manifestRes.manifestPath;
|
||||
})();
|
||||
|
||||
const record = isBundleRecord
|
||||
? buildBundleRecord({
|
||||
manifest: manifest as Parameters<typeof buildBundleRecord>[0]["manifest"],
|
||||
candidate,
|
||||
manifestPath: manifestRes.manifestPath,
|
||||
})
|
||||
: buildRecord({
|
||||
manifest: manifest as PluginManifest,
|
||||
candidate,
|
||||
manifestPath: manifestRes.manifestPath,
|
||||
schemaCacheKey,
|
||||
configSchema,
|
||||
});
|
||||
|
||||
const existing = seenIds.get(manifest.id);
|
||||
if (existing) {
|
||||
// Check whether both candidates point to the same physical directory
|
||||
@@ -599,62 +613,42 @@ export function loadPluginManifestRegistry(
|
||||
// Prefer higher-precedence origins even if candidates are passed in
|
||||
// an unexpected order (config > workspace > global > bundled).
|
||||
if (PLUGIN_ORIGIN_RANK[candidate.origin] < PLUGIN_ORIGIN_RANK[existing.candidate.origin]) {
|
||||
records[existing.recordIndex] = isBundleRecord
|
||||
? buildBundleRecord({
|
||||
manifest: manifest as Parameters<typeof buildBundleRecord>[0]["manifest"],
|
||||
candidate,
|
||||
manifestPath: manifestRes.manifestPath,
|
||||
})
|
||||
: buildRecord({
|
||||
manifest: manifest as PluginManifest,
|
||||
candidate,
|
||||
manifestPath: manifestRes.manifestPath,
|
||||
schemaCacheKey,
|
||||
configSchema,
|
||||
});
|
||||
records[existing.recordIndex] = record;
|
||||
seenIds.set(manifest.id, { candidate, recordIndex: existing.recordIndex });
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
const candidateRank = resolveDuplicatePrecedenceRank({
|
||||
pluginId: manifest.id,
|
||||
candidate,
|
||||
config,
|
||||
env,
|
||||
});
|
||||
const existingRank = resolveDuplicatePrecedenceRank({
|
||||
pluginId: manifest.id,
|
||||
candidate: existing.candidate,
|
||||
config,
|
||||
env,
|
||||
});
|
||||
const candidateWins = candidateRank < existingRank;
|
||||
const winnerCandidate = candidateWins ? candidate : existing.candidate;
|
||||
const overriddenCandidate = candidateWins ? existing.candidate : candidate;
|
||||
if (candidateWins) {
|
||||
records[existing.recordIndex] = record;
|
||||
seenIds.set(manifest.id, { candidate, recordIndex: existing.recordIndex });
|
||||
}
|
||||
diagnostics.push({
|
||||
level: "warn",
|
||||
pluginId: manifest.id,
|
||||
source: 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})`,
|
||||
source: overriddenCandidate.source,
|
||||
message: `duplicate plugin id detected; ${overriddenCandidate.origin} plugin will be overridden by ${winnerCandidate.origin} plugin (${winnerCandidate.source})`,
|
||||
});
|
||||
} else {
|
||||
seenIds.set(manifest.id, { candidate, recordIndex: records.length });
|
||||
continue;
|
||||
}
|
||||
|
||||
records.push(
|
||||
isBundleRecord
|
||||
? buildBundleRecord({
|
||||
manifest: manifest as Parameters<typeof buildBundleRecord>[0]["manifest"],
|
||||
candidate,
|
||||
manifestPath: manifestRes.manifestPath,
|
||||
})
|
||||
: buildRecord({
|
||||
manifest: manifest as PluginManifest,
|
||||
candidate,
|
||||
manifestPath: manifestRes.manifestPath,
|
||||
schemaCacheKey,
|
||||
configSchema,
|
||||
}),
|
||||
);
|
||||
seenIds.set(manifest.id, { candidate, recordIndex: records.length });
|
||||
records.push(record);
|
||||
}
|
||||
|
||||
const registry = { plugins: records, diagnostics };
|
||||
|
||||
Reference in New Issue
Block a user