Skip to content

Commit 74624e6

Browse files
TakhoffmanTakhoffman
authored
fix: prefer bundled channel plugins over npm duplicates (#40094)
* fix: prefer bundled channel plugins over npm duplicates * fix: tighten bundled plugin review follow-ups * fix: address check gate follow-ups * docs: add changelog for bundled plugin install fix * fix: align lifecycle test formatting with CI oxfmt
1 parent 6c9b49a commit 74624e6

File tree

9 files changed

+343
-50
lines changed

9 files changed

+343
-50
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ Docs: https://docs.openclaw.ai
1313

1414
### Fixes
1515

16+
- Plugins/channel onboarding: prefer bundled channel plugins over duplicate npm-installed copies during onboarding and release-channel sync, preventing bundled plugins from being shadowed by npm installs with the same plugin ID. (#40092)
1617
- macOS app/chat UI: route browser proxy through the local node browser service, preserve plain-text paste semantics, strip completed assistant trace/debug wrapper noise from transcripts, refresh permission state after returning from System Settings, and tolerate malformed cron rows in the macOS tab. (#39516) Thanks @Imhermes1.
1718
- Mattermost replies: keep `root_id` pinned to the existing thread root when an agent replies inside a thread, while still using reply-target threading for top-level posts. (#27744) thanks @hnykda.
1819
- Agents/failover: detect Amazon Bedrock `Too many tokens per day` quota errors as rate limits across fallback, cron retry, and memory embeddings while keeping context-window `too many tokens per request` errors out of the rate-limit lane. (#39377) Thanks @gambletan.

‎src/cli/plugin-install-plan.test.ts‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, expect, it, vi } from "vitest";
22
import { PLUGIN_INSTALL_ERROR_CODE } from "../plugins/install.js";
33
import {
4+
resolveBundledInstallPlanForCatalogEntry,
45
resolveBundledInstallPlanBeforeNpm,
56
resolveBundledInstallPlanForNpmFailure,
67
} from "./plugin-install-plan.js";
@@ -34,6 +35,53 @@ describe("plugin install plan helpers", () => {
3435
expect(result).toBeNull();
3536
});
3637

38+
it("prefers bundled catalog plugin by id before npm spec", () => {
39+
const findBundledSource = vi
40+
.fn()
41+
.mockImplementation(({ kind, value }: { kind: "pluginId" | "npmSpec"; value: string }) => {
42+
if (kind === "pluginId" && value === "voice-call") {
43+
return {
44+
pluginId: "voice-call",
45+
localPath: "/tmp/extensions/voice-call",
46+
npmSpec: "@openclaw/voice-call",
47+
};
48+
}
49+
return undefined;
50+
});
51+
52+
const result = resolveBundledInstallPlanForCatalogEntry({
53+
pluginId: "voice-call",
54+
npmSpec: "@openclaw/voice-call",
55+
findBundledSource,
56+
});
57+
58+
expect(findBundledSource).toHaveBeenCalledWith({ kind: "pluginId", value: "voice-call" });
59+
expect(result?.bundledSource.localPath).toBe("/tmp/extensions/voice-call");
60+
});
61+
62+
it("rejects npm-spec matches that resolve to a different plugin id", () => {
63+
const findBundledSource = vi
64+
.fn()
65+
.mockImplementation(({ kind }: { kind: "pluginId" | "npmSpec"; value: string }) => {
66+
if (kind === "npmSpec") {
67+
return {
68+
pluginId: "not-voice-call",
69+
localPath: "/tmp/extensions/not-voice-call",
70+
npmSpec: "@openclaw/voice-call",
71+
};
72+
}
73+
return undefined;
74+
});
75+
76+
const result = resolveBundledInstallPlanForCatalogEntry({
77+
pluginId: "voice-call",
78+
npmSpec: "@openclaw/voice-call",
79+
findBundledSource,
80+
});
81+
82+
expect(result).toBeNull();
83+
});
84+
3785
it("uses npm-spec bundled fallback only for package-not-found", () => {
3886
const findBundledSource = vi.fn().mockReturnValue({
3987
pluginId: "voice-call",

‎src/cli/plugin-install-plan.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,36 @@ function isBareNpmPackageName(spec: string): boolean {
1212
return /^[a-z0-9][a-z0-9-._~]*$/.test(trimmed);
1313
}
1414

15+
export function resolveBundledInstallPlanForCatalogEntry(params: {
16+
pluginId: string;
17+
npmSpec: string;
18+
findBundledSource: BundledLookup;
19+
}): { bundledSource: BundledPluginSource } | null {
20+
const pluginId = params.pluginId.trim();
21+
const npmSpec = params.npmSpec.trim();
22+
if (!pluginId || !npmSpec) {
23+
return null;
24+
}
25+
26+
const bundledById = params.findBundledSource({
27+
kind: "pluginId",
28+
value: pluginId,
29+
});
30+
if (bundledById?.pluginId === pluginId) {
31+
return { bundledSource: bundledById };
32+
}
33+
34+
const bundledBySpec = params.findBundledSource({
35+
kind: "npmSpec",
36+
value: npmSpec,
37+
});
38+
if (bundledBySpec?.pluginId === pluginId) {
39+
return { bundledSource: bundledBySpec };
40+
}
41+
42+
return null;
43+
}
44+
1545
export function resolveBundledInstallPlanBeforeNpm(params: {
1646
rawSpec: string;
1747
findBundledSource: BundledLookup;

‎src/commands/onboarding/plugin-install.test.ts‎

Lines changed: 78 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,50 @@
11
import path from "node:path";
22
import { beforeEach, describe, expect, it, vi } from "vitest";
33

4-
vi.mock("node:fs", () => ({
5-
default: {
6-
existsSync: vi.fn(),
7-
},
8-
}));
4+
vi.mock("node:fs", async (importOriginal) => {
5+
const actual = await importOriginal<typeof import("node:fs")>();
6+
const existsSync = vi.fn();
7+
return {
8+
...actual,
9+
existsSync,
10+
default: {
11+
...actual,
12+
existsSync,
13+
},
14+
};
15+
});
916

1017
const installPluginFromNpmSpec = vi.fn();
1118
vi.mock("../../plugins/install.js", () => ({
1219
installPluginFromNpmSpec: (...args: unknown[]) => installPluginFromNpmSpec(...args),
1320
}));
1421

22+
const resolveBundledPluginSources = vi.fn();
23+
vi.mock("../../plugins/bundled-sources.js", () => ({
24+
findBundledPluginSourceInMap: ({
25+
bundled,
26+
lookup,
27+
}: {
28+
bundled: ReadonlyMap<string, { pluginId: string; localPath: string; npmSpec?: string }>;
29+
lookup: { kind: "pluginId" | "npmSpec"; value: string };
30+
}) => {
31+
const targetValue = lookup.value.trim();
32+
if (!targetValue) {
33+
return undefined;
34+
}
35+
if (lookup.kind === "pluginId") {
36+
return bundled.get(targetValue);
37+
}
38+
for (const source of bundled.values()) {
39+
if (source.npmSpec === targetValue) {
40+
return source;
41+
}
42+
}
43+
return undefined;
44+
},
45+
resolveBundledPluginSources: (...args: unknown[]) => resolveBundledPluginSources(...args),
46+
}));
47+
1548
vi.mock("../../plugins/loader.js", () => ({
1649
loadOpenClawPlugins: vi.fn(),
1750
}));
@@ -41,6 +74,7 @@ const baseEntry: ChannelPluginCatalogEntry = {
4174

4275
beforeEach(() => {
4376
vi.clearAllMocks();
77+
resolveBundledPluginSources.mockReturnValue(new Map());
4478
});
4579

4680
function mockRepoLocalPathExists() {
@@ -136,6 +170,45 @@ describe("ensureOnboardingPluginInstalled", () => {
136170
expect(await runInitialValueForChannel("beta")).toBe("npm");
137171
});
138172

173+
it("defaults to bundled local path on beta channel when available", async () => {
174+
const runtime = makeRuntime();
175+
const select = vi.fn((async <T extends string>() => "skip" as T) as WizardPrompter["select"]);
176+
const prompter = makePrompter({ select: select as unknown as WizardPrompter["select"] });
177+
const cfg: OpenClawConfig = { update: { channel: "beta" } };
178+
vi.mocked(fs.existsSync).mockReturnValue(false);
179+
resolveBundledPluginSources.mockReturnValue(
180+
new Map([
181+
[
182+
"zalo",
183+
{
184+
pluginId: "zalo",
185+
localPath: "/opt/openclaw/extensions/zalo",
186+
npmSpec: "@openclaw/zalo",
187+
},
188+
],
189+
]),
190+
);
191+
192+
await ensureOnboardingPluginInstalled({
193+
cfg,
194+
entry: baseEntry,
195+
prompter,
196+
runtime,
197+
});
198+
199+
expect(select).toHaveBeenCalledWith(
200+
expect.objectContaining({
201+
initialValue: "local",
202+
options: expect.arrayContaining([
203+
expect.objectContaining({
204+
value: "local",
205+
hint: "/opt/openclaw/extensions/zalo",
206+
}),
207+
]),
208+
}),
209+
);
210+
});
211+
139212
it("falls back to local path after npm install failure", async () => {
140213
const runtime = makeRuntime();
141214
const note = vi.fn(async () => {});

‎src/commands/onboarding/plugin-install.ts‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,13 @@ import fs from "node:fs";
22
import path from "node:path";
33
import { resolveAgentWorkspaceDir, resolveDefaultAgentId } from "../../agents/agent-scope.js";
44
import type { ChannelPluginCatalogEntry } from "../../channels/plugins/catalog.js";
5+
import { resolveBundledInstallPlanForCatalogEntry } from "../../cli/plugin-install-plan.js";
56
import type { OpenClawConfig } from "../../config/config.js";
67
import { createSubsystemLogger } from "../../logging/subsystem.js";
8+
import {
9+
findBundledPluginSourceInMap,
10+
resolveBundledPluginSources,
11+
} from "../../plugins/bundled-sources.js";
712
import { enablePluginInConfig } from "../../plugins/enable.js";
813
import { installPluginFromNpmSpec } from "../../plugins/install.js";
914
import { buildNpmResolutionInstallFields, recordPluginInstall } from "../../plugins/installs.js";
@@ -107,8 +112,12 @@ function resolveInstallDefaultChoice(params: {
107112
cfg: OpenClawConfig;
108113
entry: ChannelPluginCatalogEntry;
109114
localPath?: string | null;
115+
bundledLocalPath?: string | null;
110116
}): InstallChoice {
111-
const { cfg, entry, localPath } = params;
117+
const { cfg, entry, localPath, bundledLocalPath } = params;
118+
if (bundledLocalPath) {
119+
return "local";
120+
}
112121
const updateChannel = cfg.update?.channel;
113122
if (updateChannel === "dev") {
114123
return localPath ? "local" : "npm";
@@ -136,11 +145,20 @@ export async function ensureOnboardingPluginInstalled(params: {
136145
const { entry, prompter, runtime, workspaceDir } = params;
137146
let next = params.cfg;
138147
const allowLocal = hasGitWorkspace(workspaceDir);
139-
const localPath = resolveLocalPath(entry, workspaceDir, allowLocal);
148+
const bundledSources = resolveBundledPluginSources({ workspaceDir });
149+
const bundledLocalPath =
150+
resolveBundledInstallPlanForCatalogEntry({
151+
pluginId: entry.id,
152+
npmSpec: entry.install.npmSpec,
153+
findBundledSource: (lookup) =>
154+
findBundledPluginSourceInMap({ bundled: bundledSources, lookup }),
155+
})?.bundledSource.localPath ?? null;
156+
const localPath = bundledLocalPath ?? resolveLocalPath(entry, workspaceDir, allowLocal);
140157
const defaultChoice = resolveInstallDefaultChoice({
141158
cfg: next,
142159
entry,
143160
localPath,
161+
bundledLocalPath,
144162
});
145163
const choice = await promptInstallChoice({
146164
entry,

‎src/plugins/bundled-sources.test.ts‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { beforeEach, describe, expect, it, vi } from "vitest";
2-
import { findBundledPluginSource, resolveBundledPluginSources } from "./bundled-sources.js";
2+
import {
3+
findBundledPluginSource,
4+
findBundledPluginSourceInMap,
5+
resolveBundledPluginSources,
6+
} from "./bundled-sources.js";
37

48
const discoverOpenClawPluginsMock = vi.fn();
59
const loadPluginManifestMock = vi.fn();
@@ -124,4 +128,34 @@ describe("bundled plugin sources", () => {
124128
expect(resolved?.localPath).toBe("/app/extensions/diffs");
125129
expect(missing).toBeUndefined();
126130
});
131+
132+
it("reuses a pre-resolved bundled map for repeated lookups", () => {
133+
const bundled = new Map([
134+
[
135+
"feishu",
136+
{
137+
pluginId: "feishu",
138+
localPath: "/app/extensions/feishu",
139+
npmSpec: "@openclaw/feishu",
140+
},
141+
],
142+
]);
143+
144+
expect(
145+
findBundledPluginSourceInMap({
146+
bundled,
147+
lookup: { kind: "pluginId", value: "feishu" },
148+
}),
149+
).toEqual({
150+
pluginId: "feishu",
151+
localPath: "/app/extensions/feishu",
152+
npmSpec: "@openclaw/feishu",
153+
});
154+
expect(
155+
findBundledPluginSourceInMap({
156+
bundled,
157+
lookup: { kind: "npmSpec", value: "@openclaw/feishu" },
158+
})?.pluginId,
159+
).toBe("feishu");
160+
});
127161
});

‎src/plugins/bundled-sources.ts‎

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,25 @@ export type BundledPluginLookup =
1111
| { kind: "npmSpec"; value: string }
1212
| { kind: "pluginId"; value: string };
1313

14+
export function findBundledPluginSourceInMap(params: {
15+
bundled: ReadonlyMap<string, BundledPluginSource>;
16+
lookup: BundledPluginLookup;
17+
}): BundledPluginSource | undefined {
18+
const targetValue = params.lookup.value.trim();
19+
if (!targetValue) {
20+
return undefined;
21+
}
22+
if (params.lookup.kind === "pluginId") {
23+
return params.bundled.get(targetValue);
24+
}
25+
for (const source of params.bundled.values()) {
26+
if (source.npmSpec === targetValue) {
27+
return source;
28+
}
29+
}
30+
return undefined;
31+
}
32+
1433
export function resolveBundledPluginSources(params: {
1534
workspaceDir?: string;
1635
}): Map<string, BundledPluginSource> {
@@ -49,18 +68,9 @@ export function findBundledPluginSource(params: {
4968
lookup: BundledPluginLookup;
5069
workspaceDir?: string;
5170
}): BundledPluginSource | undefined {
52-
const targetValue = params.lookup.value.trim();
53-
if (!targetValue) {
54-
return undefined;
55-
}
5671
const bundled = resolveBundledPluginSources({ workspaceDir: params.workspaceDir });
57-
if (params.lookup.kind === "pluginId") {
58-
return bundled.get(targetValue);
59-
}
60-
for (const source of bundled.values()) {
61-
if (source.npmSpec === targetValue) {
62-
return source;
63-
}
64-
}
65-
return undefined;
72+
return findBundledPluginSourceInMap({
73+
bundled,
74+
lookup: params.lookup,
75+
});
6676
}

0 commit comments

Comments
 (0)