fix(clownfish): address review for ghcrawl-207039-agentic-merge (1)

openclaw-clownfish[bot] · vincentkoc · commit bfa867e28c02 · 2026-04-28T23:15:12.000-07:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -606,7 +606,7 @@ Docs: https://docs.openclaw.ai
 - Onboarding/GitHub Copilot: add manifest-owned `--github-copilot-token` support for non-interactive setup, including env fallback, tokenRef storage in ref mode, saved-profile reuse, and current Copilot default-model wiring. Refs #50002 and supersedes #50003. Thanks @scottgl9.
 - Gateway/install: add a validated `--wrapper`/`OPENCLAW_WRAPPER` service install path that persists executable LaunchAgent/systemd wrappers across forced reinstalls, updates, and doctor repairs instead of falling back to raw node/bun `ProgramArguments`. Fixes #69400. (#72445) Thanks @willtmc.
 - Plugins: fail plugin registration when loader-owned acceptance gates reject missing hook names or memory-only capability registration from non-memory plugins, surfacing the issue through plugin status and doctor instead of silently dropping the registration. Fixes #72459. Thanks @amknight.
-- Plugins: suppress false duplicate plugin-id warnings when the same id is discovered across different plugin origins, while preserving same-origin distinct-root diagnostics after same-root origin promotion. Fixes #42099; carries forward the focused #42192 approach and review feedback. Thanks @macarronesc.
+- Plugins: suppress only same-directory duplicate plugin-id false positives while keeping distinct-root cross-origin duplicate diagnostics visible. Fixes #42099; carries forward the focused #42192 approach and review feedback. Thanks @macarronesc.
 - macOS Gateway: write launchd services with a state-dir `WorkingDirectory`, use a durable state-dir temp path instead of freezing macOS session `TMPDIR`, create that temp directory before bootstrap, and label abort-shaped launchd exits as `SIGABRT/abort` in status output. Fixes #53679 and #70223; refs #71848. Thanks @dlturock, @stammi922, and @palladius.
 - Control UI/update: make `Update now` require a real gateway process replacement, report skipped/error update outcomes with stable reasons, and verify the running gateway version after restart so global installs cannot silently keep old code in memory. Fixes #62492; addresses #64892 and #63562. Thanks @IAMSamuelRodda.
 - Exec approvals: accept runtime-owned `source: "allow-always"` and `commandText` allowlist metadata in gateway and node approval-set payloads so Control UI round-trips no longer fail with `unexpected property 'source'`. Fixes #60000; carries forward #60064. Thanks @sd1471123, @sharkqwy, and @luoyanglang.
diff --git a/src/plugins/manifest-registry.test.ts b/src/plugins/manifest-registry.test.ts
@@ -372,7 +372,7 @@ describe("loadPluginManifestRegistry", () => {
     );
   });
 
-  it("lets config-loaded plugins replace bundled duplicates without cross-origin warnings", () => {
+  it("warns when config-loaded plugins replace bundled distinct-root duplicates", () => {
     const bundledDir = makeTempDir();
     const configDir = makeTempDir();
     const manifest = { id: "config-shadow", configSchema: { type: "object" } };
@@ -392,12 +392,15 @@ describe("loadPluginManifestRegistry", () => {
       }),
     ]);
 
-    expect(countDuplicateWarnings(registry)).toBe(0);
+    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("keeps explicit installed globals as the effective duplicate winner without cross-origin warnings", () => {
+  it("warns while keeping explicit installed globals as the effective duplicate winner", () => {
     const bundledDir = makeTempDir();
     const globalDir = makeTempDir();
     const manifest = { id: "zalouser", configSchema: { type: "object" } };
@@ -425,12 +428,16 @@ describe("loadPluginManifestRegistry", () => {
       ],
     });
 
-    expect(countDuplicateWarnings(registry)).toBe(0);
+    expect(countDuplicateWarnings(registry)).toBe(1);
     expect(registry.plugins).toHaveLength(1);
     expect(registry.plugins[0]?.origin).toBe("global");
+    expectRegistryDiagnosticContains(
+      registry,
+      "bundled plugin will be overridden by global plugin",
+    );
   });
 
-  it("suppresses duplicate warnings when the installed global is discovered before bundled", () => {
+  it("warns when the installed global is discovered before bundled", () => {
     const bundledDir = makeTempDir();
     const globalDir = makeTempDir();
     const manifest = { id: "zalouser", configSchema: { type: "object" } };
@@ -458,9 +465,13 @@ describe("loadPluginManifestRegistry", () => {
       ],
     });
 
-    expect(countDuplicateWarnings(registry)).toBe(0);
+    expect(countDuplicateWarnings(registry)).toBe(1);
     expect(registry.plugins).toHaveLength(1);
     expect(registry.plugins[0]?.origin).toBe("global");
+    expectRegistryDiagnosticContains(
+      registry,
+      "bundled plugin will be overridden by global plugin",
+    );
   });
 
   it("preserves provider auth env metadata from plugin manifests", () => {
@@ -1505,24 +1516,26 @@ describe("loadPluginManifestRegistry", () => {
 
   it.each([
     {
-      name: "keeps bundled plugins as the duplicate winner for auto-discovered globals",
+      name: "reports bundled plugins as the duplicate winner for auto-discovered globals",
       registry: () =>
         createDuplicateCandidateRegistry({
           pluginId: "feishu",
           duplicateOrigin: "global",
         }),
+      expectedMessage: "global plugin will be overridden by bundled plugin",
     },
     {
-      name: "keeps bundled plugins as the duplicate winner for workspace duplicates",
+      name: "reports bundled plugins as the duplicate winner for workspace duplicates",
       registry: () =>
         createDuplicateCandidateRegistry({
           pluginId: "shadowed",
           duplicateOrigin: "workspace",
         }),
+      expectedMessage: "workspace plugin will be overridden by bundled plugin",
     },
-  ] as const)("$name", ({ registry: buildRegistry }) => {
+  ] as const)("$name", ({ registry: buildRegistry, expectedMessage }) => {
     const registry = buildRegistry();
-    expect(countDuplicateWarnings(registry)).toBe(0);
+    expectRegistryDiagnosticContains(registry, expectedMessage);
     expect(registry.plugins).toHaveLength(1);
     expect(registry.plugins[0]?.origin).toBe("bundled");
   });
diff --git a/src/plugins/manifest-registry.ts b/src/plugins/manifest-registry.ts
@@ -744,11 +744,21 @@ export function loadPluginManifestRegistry(
         installRecords: getInstallRecords(),
       });
       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 });
         pushManifestCompatibilityDiagnostics({ record, diagnostics });
       }
+      if (winnerCandidate.origin !== overriddenCandidate.origin) {
+        diagnostics.push({
+          level: "warn",
+          pluginId: manifest.id,
+          source: overriddenCandidate.source,
+          message: `duplicate plugin id detected; ${overriddenCandidate.origin} plugin will be overridden by ${winnerCandidate.origin} plugin (${winnerCandidate.source})`,
+        });
+      }
       continue;
     }
 
