From 8e486dffc927a8acdd6f5de63856ea08b4533e9b Mon Sep 17 00:00:00 2001 From: Tak Hoffman <781889+Takhoffman@users.noreply.github.com> Date: Sun, 8 Mar 2026 13:04:41 -0500 Subject: [PATCH] fix(plugins): align duplicate warning with skipped copy --- src/plugins/manifest-registry.test.ts | 46 +++++++++++++++++++++++++++ src/plugins/manifest-registry.ts | 11 ++++--- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/src/plugins/manifest-registry.test.ts b/src/plugins/manifest-registry.test.ts index 3a40b104024..531d999c6e1 100644 --- a/src/plugins/manifest-registry.test.ts +++ b/src/plugins/manifest-registry.test.ts @@ -47,6 +47,17 @@ function countDuplicateWarnings(registry: ReturnType, +): 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" }): { rootDir: string; linked: boolean; @@ -183,6 +194,9 @@ describe("loadPluginManifestRegistry", () => { // 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", () => { @@ -210,6 +224,38 @@ describe("loadPluginManifestRegistry", () => { 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", () => { diff --git a/src/plugins/manifest-registry.ts b/src/plugins/manifest-registry.ts index 9dddd7d833b..305d14a14a7 100644 --- a/src/plugins/manifest-registry.ts +++ b/src/plugins/manifest-registry.ts @@ -241,18 +241,19 @@ export function loadPluginManifestRegistry(params: { } continue; } + const candidateWins = + GENUINE_DUPLICATE_RANK[candidate.origin] < GENUINE_DUPLICATE_RANK[existing.candidate.origin]; + const skippedCandidate = candidateWins ? existing.candidate : candidate; diagnostics.push({ level: "warn", pluginId: manifest.id, - source: candidate.source, - message: `duplicate plugin id detected; skipping duplicate from ${candidate.source}`, + source: skippedCandidate.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 ( - GENUINE_DUPLICATE_RANK[candidate.origin] < GENUINE_DUPLICATE_RANK[existing.candidate.origin] - ) { + if (candidateWins) { records[existing.recordIndex] = buildRecord({ manifest, candidate,