Compare commits
5 Commits
main
...
fix/plugin
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
a3d08bb3d2 | ||
|
|
8e486dffc9 | ||
|
|
bacebb1cf1 | ||
|
|
c306c6e1fb | ||
|
|
dd64501262 |
@ -36,16 +36,17 @@ const renderGatewayPortHealthDiagnostics = vi.fn(() => ["diag: unhealthy port"])
|
|||||||
const renderRestartDiagnostics = vi.fn(() => ["diag: unhealthy runtime"]);
|
const renderRestartDiagnostics = vi.fn(() => ["diag: unhealthy runtime"]);
|
||||||
const resolveGatewayPort = vi.fn(() => 18789);
|
const resolveGatewayPort = vi.fn(() => 18789);
|
||||||
const findGatewayPidsOnPortSync = vi.fn<(port: number) => number[]>(() => []);
|
const findGatewayPidsOnPortSync = vi.fn<(port: number) => number[]>(() => []);
|
||||||
const probeGateway = vi.fn<
|
const probeGateway =
|
||||||
(opts: {
|
vi.fn<
|
||||||
url: string;
|
(opts: {
|
||||||
auth?: { token?: string; password?: string };
|
url: string;
|
||||||
timeoutMs: number;
|
auth?: { token?: string; password?: string };
|
||||||
}) => Promise<{
|
timeoutMs: number;
|
||||||
ok: boolean;
|
}) => Promise<{
|
||||||
configSnapshot: unknown;
|
ok: boolean;
|
||||||
}>
|
configSnapshot: unknown;
|
||||||
>();
|
}>
|
||||||
|
>();
|
||||||
const isRestartEnabled = vi.fn<(config?: { commands?: unknown }) => boolean>(() => true);
|
const isRestartEnabled = vi.fn<(config?: { commands?: unknown }) => boolean>(() => true);
|
||||||
const loadConfig = vi.fn(() => ({}));
|
const loadConfig = vi.fn(() => ({}));
|
||||||
|
|
||||||
|
|||||||
@ -1093,10 +1093,11 @@ describe("loadOpenClawPlugins", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
const entries = registry.plugins.filter((entry) => entry.id === "shadow");
|
const entries = registry.plugins.filter((entry) => entry.id === "shadow");
|
||||||
const loaded = entries.find((entry) => entry.status === "loaded");
|
// Manifest registry deduplicates genuine duplicates (same id, different paths),
|
||||||
const overridden = entries.find((entry) => entry.status === "disabled");
|
// so only the higher-precedence record survives to the loader.
|
||||||
expect(loaded?.origin).toBe("config");
|
expect(entries).toHaveLength(1);
|
||||||
expect(overridden?.origin).toBe("bundled");
|
expect(entries[0]?.status).toBe("loaded");
|
||||||
|
expect(entries[0]?.origin).toBe("config");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("prefers bundled plugin over auto-discovered global duplicate ids", () => {
|
it("prefers bundled plugin over auto-discovered global duplicate ids", () => {
|
||||||
@ -1133,11 +1134,11 @@ describe("loadOpenClawPlugins", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
const entries = registry.plugins.filter((entry) => entry.id === "feishu");
|
const entries = registry.plugins.filter((entry) => entry.id === "feishu");
|
||||||
const loaded = entries.find((entry) => entry.status === "loaded");
|
// Manifest registry deduplicates genuine duplicates (same id, different paths),
|
||||||
const overridden = entries.find((entry) => entry.status === "disabled");
|
// keeping the bundled copy over the auto-discovered global copy.
|
||||||
expect(loaded?.origin).toBe("bundled");
|
expect(entries).toHaveLength(1);
|
||||||
expect(overridden?.origin).toBe("global");
|
expect(entries[0]?.status).toBe("loaded");
|
||||||
expect(overridden?.error).toContain("overridden by bundled plugin");
|
expect(entries[0]?.origin).toBe("bundled");
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@ -47,6 +47,17 @@ function countDuplicateWarnings(registry: ReturnType<typeof loadPluginManifestRe
|
|||||||
).length;
|
).length;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function getDuplicateWarningMessages(
|
||||||
|
registry: ReturnType<typeof loadPluginManifestRegistry>,
|
||||||
|
): string[] {
|
||||||
|
return registry.diagnostics
|
||||||
|
.filter(
|
||||||
|
(diagnostic) =>
|
||||||
|
diagnostic.level === "warn" && diagnostic.message?.includes("duplicate plugin id"),
|
||||||
|
)
|
||||||
|
.map((diagnostic) => diagnostic.message);
|
||||||
|
}
|
||||||
|
|
||||||
function prepareLinkedManifestFixture(params: { id: string; mode: "symlink" | "hardlink" }): {
|
function prepareLinkedManifestFixture(params: { id: string; mode: "symlink" | "hardlink" }): {
|
||||||
rootDir: string;
|
rootDir: string;
|
||||||
linked: boolean;
|
linked: boolean;
|
||||||
@ -150,7 +161,101 @@ describe("loadPluginManifestRegistry", () => {
|
|||||||
}),
|
}),
|
||||||
];
|
];
|
||||||
|
|
||||||
expect(countDuplicateWarnings(loadRegistry(candidates))).toBe(1);
|
const registry = loadRegistry(candidates);
|
||||||
|
expect(countDuplicateWarnings(registry)).toBe(1);
|
||||||
|
// Only one record should be kept to prevent Gateway instability
|
||||||
|
expect(registry.plugins.filter((p) => p.id === "test-plugin")).toHaveLength(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps higher-precedence origin when genuine duplicate is detected from different paths", () => {
|
||||||
|
const bundledDir = makeTempDir();
|
||||||
|
const globalDir = makeTempDir();
|
||||||
|
const manifest = { id: "feishu-dup", configSchema: { type: "object" } };
|
||||||
|
writeManifest(bundledDir, manifest);
|
||||||
|
writeManifest(globalDir, manifest);
|
||||||
|
|
||||||
|
// Bundled discovered first, then global (npm-installed copy)
|
||||||
|
const candidates: PluginCandidate[] = [
|
||||||
|
createPluginCandidate({
|
||||||
|
idHint: "feishu-dup",
|
||||||
|
rootDir: bundledDir,
|
||||||
|
origin: "bundled",
|
||||||
|
}),
|
||||||
|
createPluginCandidate({
|
||||||
|
idHint: "feishu-dup",
|
||||||
|
rootDir: globalDir,
|
||||||
|
origin: "global",
|
||||||
|
}),
|
||||||
|
];
|
||||||
|
|
||||||
|
const registry = loadRegistry(candidates);
|
||||||
|
expect(countDuplicateWarnings(registry)).toBe(1);
|
||||||
|
expect(registry.plugins.filter((p) => p.id === "feishu-dup")).toHaveLength(1);
|
||||||
|
// Bundled has higher precedence than global for genuine duplicates
|
||||||
|
// (bundled ships with the binary and is the known-good version)
|
||||||
|
expect(registry.plugins[0]?.origin).toBe("bundled");
|
||||||
|
expect(getDuplicateWarningMessages(registry)).toContain(
|
||||||
|
`duplicate plugin id detected; skipping duplicate from ${candidates[1].source}`,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps existing record when genuine duplicate has lower precedence", () => {
|
||||||
|
const configDir = makeTempDir();
|
||||||
|
const globalDir = makeTempDir();
|
||||||
|
const manifest = { id: "lower-prec", configSchema: { type: "object" } };
|
||||||
|
writeManifest(configDir, manifest);
|
||||||
|
writeManifest(globalDir, manifest);
|
||||||
|
|
||||||
|
const candidates: PluginCandidate[] = [
|
||||||
|
createPluginCandidate({
|
||||||
|
idHint: "lower-prec",
|
||||||
|
rootDir: configDir,
|
||||||
|
origin: "config",
|
||||||
|
}),
|
||||||
|
createPluginCandidate({
|
||||||
|
idHint: "lower-prec",
|
||||||
|
rootDir: globalDir,
|
||||||
|
origin: "global",
|
||||||
|
}),
|
||||||
|
];
|
||||||
|
|
||||||
|
const registry = loadRegistry(candidates);
|
||||||
|
expect(countDuplicateWarnings(registry)).toBe(1);
|
||||||
|
expect(registry.plugins.filter((p) => p.id === "lower-prec")).toHaveLength(1);
|
||||||
|
// Config has higher precedence, should be kept
|
||||||
|
expect(registry.plugins[0]?.origin).toBe("config");
|
||||||
|
expect(getDuplicateWarningMessages(registry)).toContain(
|
||||||
|
`duplicate plugin id detected; skipping duplicate from ${candidates[1].source}`,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("warns with existing source when higher-precedence duplicate replaces it", () => {
|
||||||
|
const globalDir = makeTempDir();
|
||||||
|
const bundledDir = makeTempDir();
|
||||||
|
const manifest = { id: "replace-prec", configSchema: { type: "object" } };
|
||||||
|
writeManifest(globalDir, manifest);
|
||||||
|
writeManifest(bundledDir, manifest);
|
||||||
|
|
||||||
|
const candidates: PluginCandidate[] = [
|
||||||
|
createPluginCandidate({
|
||||||
|
idHint: "replace-prec",
|
||||||
|
rootDir: globalDir,
|
||||||
|
origin: "global",
|
||||||
|
}),
|
||||||
|
createPluginCandidate({
|
||||||
|
idHint: "replace-prec",
|
||||||
|
rootDir: bundledDir,
|
||||||
|
origin: "bundled",
|
||||||
|
}),
|
||||||
|
];
|
||||||
|
|
||||||
|
const registry = loadRegistry(candidates);
|
||||||
|
expect(countDuplicateWarnings(registry)).toBe(1);
|
||||||
|
expect(registry.plugins.filter((p) => p.id === "replace-prec")).toHaveLength(1);
|
||||||
|
expect(registry.plugins[0]?.origin).toBe("bundled");
|
||||||
|
expect(getDuplicateWarningMessages(registry)).toContain(
|
||||||
|
`duplicate plugin id detected; skipping duplicate from ${candidates[0].source}`,
|
||||||
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("suppresses duplicate warning when candidates share the same physical directory via symlink", () => {
|
it("suppresses duplicate warning when candidates share the same physical directory via symlink", () => {
|
||||||
|
|||||||
@ -12,7 +12,8 @@ type SeenIdEntry = {
|
|||||||
recordIndex: number;
|
recordIndex: number;
|
||||||
};
|
};
|
||||||
|
|
||||||
// Precedence: config > workspace > global > bundled
|
// Same-path dedup precedence: config > workspace > global > bundled
|
||||||
|
// Used when the same physical directory is discovered via multiple routes.
|
||||||
const PLUGIN_ORIGIN_RANK: Readonly<Record<PluginOrigin, number>> = {
|
const PLUGIN_ORIGIN_RANK: Readonly<Record<PluginOrigin, number>> = {
|
||||||
config: 0,
|
config: 0,
|
||||||
workspace: 1,
|
workspace: 1,
|
||||||
@ -20,6 +21,17 @@ const PLUGIN_ORIGIN_RANK: Readonly<Record<PluginOrigin, number>> = {
|
|||||||
bundled: 3,
|
bundled: 3,
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// Genuine-duplicate precedence: config > bundled > workspace > global
|
||||||
|
// Used when genuinely different copies of a plugin (same id, different paths)
|
||||||
|
// exist. Bundled plugins ship with the binary and are the known-good version,
|
||||||
|
// so they take precedence over auto-discovered global/npm-installed copies.
|
||||||
|
const GENUINE_DUPLICATE_RANK: Readonly<Record<PluginOrigin, number>> = {
|
||||||
|
config: 0,
|
||||||
|
bundled: 1,
|
||||||
|
workspace: 2,
|
||||||
|
global: 3,
|
||||||
|
};
|
||||||
|
|
||||||
export type PluginManifestRecord = {
|
export type PluginManifestRecord = {
|
||||||
id: string;
|
id: string;
|
||||||
name?: string;
|
name?: string;
|
||||||
@ -229,12 +241,30 @@ export function loadPluginManifestRegistry(params: {
|
|||||||
}
|
}
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
const candidateWins =
|
||||||
|
GENUINE_DUPLICATE_RANK[candidate.origin] <
|
||||||
|
GENUINE_DUPLICATE_RANK[existing.candidate.origin];
|
||||||
|
const skippedCandidate = candidateWins ? existing.candidate : candidate;
|
||||||
diagnostics.push({
|
diagnostics.push({
|
||||||
level: "warn",
|
level: "warn",
|
||||||
pluginId: manifest.id,
|
pluginId: manifest.id,
|
||||||
source: candidate.source,
|
source: skippedCandidate.source,
|
||||||
message: `duplicate plugin id detected; later plugin may be overridden (${candidate.source})`,
|
message: `duplicate plugin id detected; skipping duplicate from ${skippedCandidate.source}`,
|
||||||
});
|
});
|
||||||
|
// Genuine duplicate from a different physical path: apply
|
||||||
|
// bundled-first precedence to avoid registering two records with
|
||||||
|
// the same plugin id (which causes Gateway instability).
|
||||||
|
if (candidateWins) {
|
||||||
|
records[existing.recordIndex] = buildRecord({
|
||||||
|
manifest,
|
||||||
|
candidate,
|
||||||
|
manifestPath: manifestRes.manifestPath,
|
||||||
|
schemaCacheKey,
|
||||||
|
configSchema,
|
||||||
|
});
|
||||||
|
seenIds.set(manifest.id, { candidate, recordIndex: existing.recordIndex });
|
||||||
|
}
|
||||||
|
continue;
|
||||||
} else {
|
} else {
|
||||||
seenIds.set(manifest.id, { candidate, recordIndex: records.length });
|
seenIds.set(manifest.id, { candidate, recordIndex: records.length });
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user