Skip to content

Commit 764960f

Browse files
vincentkocvincentkoc
committed
Plugins: preserve scoped ids and reserve bundled duplicates
1 parent f2649ba commit 764960f

File tree

6 files changed

+140
-41
lines changed

6 files changed

+140
-41
lines changed

‎src/plugins/install.test.ts‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ async function setupVoiceCallArchiveInstall(params: { outName: string; version:
157157
}
158158

159159
function expectPluginFiles(result: { targetDir: string }, stateDir: string, pluginId: string) {
160-
expect(result.targetDir).toBe(path.join(stateDir, "extensions", pluginId));
160+
expect(result.targetDir).toBe(path.join(stateDir, "extensions", pluginId.replaceAll("/", "__")));
161161
expect(fs.existsSync(path.join(result.targetDir, "package.json"))).toBe(true);
162162
expect(fs.existsSync(path.join(result.targetDir, "dist", "index.js"))).toBe(true);
163163
}
@@ -394,7 +394,7 @@ beforeEach(() => {
394394
});
395395

396396
describe("installPluginFromArchive", () => {
397-
it("installs into ~/.openclaw/extensions and uses unscoped id", async () => {
397+
it("installs into ~/.openclaw/extensions and preserves scoped package ids", async () => {
398398
const { stateDir, archivePath, extensionsDir } = await setupVoiceCallArchiveInstall({
399399
outName: "plugin.tgz",
400400
version: "0.0.1",
@@ -404,7 +404,7 @@ describe("installPluginFromArchive", () => {
404404
archivePath,
405405
extensionsDir,
406406
});
407-
expectSuccessfulArchiveInstall({ result, stateDir, pluginId: "voice-call" });
407+
expectSuccessfulArchiveInstall({ result, stateDir, pluginId: "@openclaw/voice-call" });
408408
});
409409

410410
it("rejects installing when plugin already exists", async () => {
@@ -443,7 +443,7 @@ describe("installPluginFromArchive", () => {
443443
archivePath,
444444
extensionsDir,
445445
});
446-
expectSuccessfulArchiveInstall({ result, stateDir, pluginId: "zipper" });
446+
expectSuccessfulArchiveInstall({ result, stateDir, pluginId: "@openclaw/zipper" });
447447
});
448448

449449
it("allows updates when mode is update", async () => {
@@ -615,16 +615,17 @@ describe("installPluginFromArchive", () => {
615615
});
616616

617617
describe("installPluginFromDir", () => {
618-
function expectInstalledAsMemoryCognee(
618+
function expectInstalledWithPluginId(
619619
result: Awaited<ReturnType<typeof installPluginFromDir>>,
620620
extensionsDir: string,
621+
pluginId: string,
621622
) {
622623
expect(result.ok).toBe(true);
623624
if (!result.ok) {
624625
return;
625626
}
626-
expect(result.pluginId).toBe("memory-cognee");
627-
expect(result.targetDir).toBe(path.join(extensionsDir, "memory-cognee"));
627+
expect(result.pluginId).toBe(pluginId);
628+
expect(result.targetDir).toBe(path.join(extensionsDir, pluginId.replaceAll("/", "__")));
628629
}
629630

630631
it("uses --ignore-scripts for dependency install", async () => {
@@ -689,29 +690,40 @@ describe("installPluginFromDir", () => {
689690
logger: { info: (msg: string) => infoMessages.push(msg), warn: () => {} },
690691
});
691692

692-
expectInstalledAsMemoryCognee(res, extensionsDir);
693+
expectInstalledWithPluginId(res, extensionsDir, "memory-cognee");
693694
expect(
694695
infoMessages.some((msg) =>
695696
msg.includes(
696-
'Plugin manifest id "memory-cognee" differs from npm package name "cognee-openclaw"',
697+
'Plugin manifest id "memory-cognee" differs from npm package name "@openclaw/cognee-openclaw"',
697698
),
698699
),
699700
).toBe(true);
700701
});
701702

702-
it("normalizes scoped manifest ids to unscoped install keys", async () => {
703+
it("preserves scoped manifest ids as install keys", async () => {
703704
const { pluginDir, extensionsDir } = setupManifestInstallFixture({
704705
manifestId: "@team/memory-cognee",
705706
});
706707

707708
const res = await installPluginFromDir({
708709
dirPath: pluginDir,
709710
extensionsDir,
710-
expectedPluginId: "memory-cognee",
711+
expectedPluginId: "@team/memory-cognee",
711712
logger: { info: () => {}, warn: () => {} },
712713
});
713714

714-
expectInstalledAsMemoryCognee(res, extensionsDir);
715+
expectInstalledWithPluginId(res, extensionsDir, "@team/memory-cognee");
716+
});
717+
718+
it("preserves scoped package names when no plugin manifest id is present", async () => {
719+
const { pluginDir, extensionsDir } = setupInstallPluginFromDirFixture();
720+
721+
const res = await installPluginFromDir({
722+
dirPath: pluginDir,
723+
extensionsDir,
724+
});
725+
726+
expectInstalledWithPluginId(res, extensionsDir, "@openclaw/test-plugin");
715727
});
716728
});
717729

‎src/plugins/install.ts‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,7 @@ import {
88
resolveTimedInstallModeOptions,
99
} from "../infra/install-mode-options.js";
1010
import { installPackageDir } from "../infra/install-package-dir.js";
11-
import {
12-
resolveSafeInstallDir,
13-
safeDirName,
14-
unscopedPackageName,
15-
} from "../infra/install-safe-path.js";
11+
import { resolveSafeInstallDir, safeDirName } from "../infra/install-safe-path.js";
1612
import {
1713
type NpmIntegrityDrift,
1814
type NpmSpecResolution,
@@ -85,13 +81,24 @@ function safeFileName(input: string): string {
8581
}
8682

8783
function validatePluginId(pluginId: string): string | null {
88-
if (!pluginId) {
84+
const trimmed = pluginId.trim();
85+
if (!trimmed) {
8986
return "invalid plugin name: missing";
9087
}
91-
if (pluginId === "." || pluginId === "..") {
88+
if (trimmed.includes("\\")) {
89+
return "invalid plugin name: path separators not allowed";
90+
}
91+
const segments = trimmed.split("/");
92+
if (segments.some((segment) => !segment)) {
93+
return "invalid plugin name: malformed scope";
94+
}
95+
if (segments.some((segment) => segment === "." || segment === "..")) {
9296
return "invalid plugin name: reserved path segment";
9397
}
94-
if (pluginId.includes("/") || pluginId.includes("\\")) {
98+
if (segments.length === 1) {
99+
return null;
100+
}
101+
if (segments.length !== 2 || !segments[0]?.startsWith("@")) {
95102
return "invalid plugin name: path separators not allowed";
96103
}
97104
return null;
@@ -233,8 +240,8 @@ async function installPluginFromPackageDir(
233240
}
234241
const extensions = extensionsResult.entries;
235242

236-
const pkgName = typeof manifest.name === "string" ? manifest.name : "";
237-
const npmPluginId = pkgName ? unscopedPackageName(pkgName) : "plugin";
243+
const pkgName = typeof manifest.name === "string" ? manifest.name.trim() : "";
244+
const npmPluginId = pkgName || "plugin";
238245

239246
// Prefer the canonical `id` from openclaw.plugin.json over the npm package name.
240247
// This avoids a latent key-mismatch bug: if the manifest id (e.g. "memory-cognee")
@@ -243,7 +250,7 @@ async function installPluginFromPackageDir(
243250
const ocManifestResult = loadPluginManifest(params.packageDir);
244251
const manifestPluginId =
245252
ocManifestResult.ok && ocManifestResult.manifest.id
246-
? unscopedPackageName(ocManifestResult.manifest.id)
253+
? ocManifestResult.manifest.id.trim()
247254
: undefined;
248255

249256
const pluginId = manifestPluginId ?? npmPluginId;

‎src/plugins/loader.test.ts‎

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1692,7 +1692,37 @@ describe("loadOpenClawPlugins", () => {
16921692
expect(workspacePlugin?.status).toBe("loaded");
16931693
});
16941694

1695-
it("lets an explicitly trusted workspace plugin shadow a bundled plugin with the same id", () => {
1695+
it("keeps scoped and unscoped plugin ids distinct", () => {
1696+
useNoBundledPlugins();
1697+
const scoped = writePlugin({
1698+
id: "@team/shadowed",
1699+
body: `module.exports = { id: "@team/shadowed", register() {} };`,
1700+
filename: "scoped.cjs",
1701+
});
1702+
const unscoped = writePlugin({
1703+
id: "shadowed",
1704+
body: `module.exports = { id: "shadowed", register() {} };`,
1705+
filename: "unscoped.cjs",
1706+
});
1707+
1708+
const registry = loadOpenClawPlugins({
1709+
cache: false,
1710+
config: {
1711+
plugins: {
1712+
load: { paths: [scoped.file, unscoped.file] },
1713+
allow: ["@team/shadowed", "shadowed"],
1714+
},
1715+
},
1716+
});
1717+
1718+
expect(registry.plugins.find((entry) => entry.id === "@team/shadowed")?.status).toBe("loaded");
1719+
expect(registry.plugins.find((entry) => entry.id === "shadowed")?.status).toBe("loaded");
1720+
expect(
1721+
registry.diagnostics.some((diag) => String(diag.message).includes("duplicate plugin id")),
1722+
).toBe(false);
1723+
});
1724+
1725+
it("keeps bundled plugins ahead of trusted workspace duplicates with the same id", () => {
16961726
const bundledDir = makeTempDir();
16971727
writePlugin({
16981728
id: "shadowed",
@@ -1719,15 +1749,19 @@ describe("loadOpenClawPlugins", () => {
17191749
plugins: {
17201750
enabled: true,
17211751
allow: ["shadowed"],
1752+
entries: {
1753+
shadowed: { enabled: true },
1754+
},
17221755
},
17231756
},
17241757
});
17251758

17261759
const entries = registry.plugins.filter((entry) => entry.id === "shadowed");
17271760
const loaded = entries.find((entry) => entry.status === "loaded");
17281761
const overridden = entries.find((entry) => entry.status === "disabled");
1729-
expect(loaded?.origin).toBe("workspace");
1730-
expect(overridden?.origin).toBe("bundled");
1762+
expect(loaded?.origin).toBe("bundled");
1763+
expect(overridden?.origin).toBe("workspace");
1764+
expect(overridden?.error).toContain("overridden by bundled plugin");
17311765
});
17321766

17331767
it("warns when loaded non-bundled plugin has no install/load-path provenance", () => {

‎src/plugins/loader.ts‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -485,16 +485,20 @@ function resolveCandidateDuplicateRank(params: {
485485
env: params.env,
486486
});
487487

488-
switch (params.candidate.origin) {
489-
case "config":
490-
return 0;
491-
case "workspace":
492-
return 1;
493-
case "global":
494-
return isExplicitInstall ? 2 : 4;
495-
case "bundled":
496-
return 3;
488+
if (params.candidate.origin === "config") {
489+
return 0;
490+
}
491+
if (params.candidate.origin === "global" && isExplicitInstall) {
492+
return 1;
493+
}
494+
if (params.candidate.origin === "bundled") {
495+
// Bundled plugin ids stay reserved unless the operator configured an override.
496+
return 2;
497+
}
498+
if (params.candidate.origin === "workspace") {
499+
return 3;
497500
}
501+
return 4;
498502
}
499503

500504
function compareDuplicateCandidateOrder(params: {

‎src/plugins/manifest-registry.test.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,36 @@ describe("loadPluginManifestRegistry", () => {
225225
).toBe(true);
226226
});
227227

228+
it("reports bundled plugins as the duplicate winner for workspace duplicates", () => {
229+
const bundledDir = makeTempDir();
230+
const workspaceDir = makeTempDir();
231+
const manifest = { id: "shadowed", configSchema: { type: "object" } };
232+
writeManifest(bundledDir, manifest);
233+
writeManifest(workspaceDir, manifest);
234+
235+
const registry = loadPluginManifestRegistry({
236+
cache: false,
237+
candidates: [
238+
createPluginCandidate({
239+
idHint: "shadowed",
240+
rootDir: bundledDir,
241+
origin: "bundled",
242+
}),
243+
createPluginCandidate({
244+
idHint: "shadowed",
245+
rootDir: workspaceDir,
246+
origin: "workspace",
247+
}),
248+
],
249+
});
250+
251+
expect(
252+
registry.diagnostics.some((diag) =>
253+
diag.message.includes("workspace plugin will be overridden by bundled plugin"),
254+
),
255+
).toBe(true);
256+
});
257+
228258
it("suppresses duplicate warning when candidates share the same physical directory via symlink", () => {
229259
const realDir = makeTempDir();
230260
const manifest = { id: "feishu", configSchema: { type: "object" } };

‎src/plugins/manifest-registry.ts‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ type SeenIdEntry = {
1313
recordIndex: number;
1414
};
1515

16-
// Precedence: config > workspace > explicit-install global > bundled > auto-discovered global
16+
// Canonicalize identical physical plugin roots with the most explicit source.
17+
// This only applies when multiple candidates resolve to the same on-disk plugin.
1718
const PLUGIN_ORIGIN_RANK: Readonly<Record<PluginOrigin, number>> = {
1819
config: 0,
1920
workspace: 1,
@@ -167,17 +168,28 @@ function resolveDuplicatePrecedenceRank(params: {
167168
config?: OpenClawConfig;
168169
env: NodeJS.ProcessEnv;
169170
}): number {
170-
if (params.candidate.origin === "global") {
171-
return matchesInstalledPluginRecord({
171+
if (params.candidate.origin === "config") {
172+
return 0;
173+
}
174+
if (
175+
params.candidate.origin === "global" &&
176+
matchesInstalledPluginRecord({
172177
pluginId: params.pluginId,
173178
candidate: params.candidate,
174179
config: params.config,
175180
env: params.env,
176181
})
177-
? 2
178-
: 4;
182+
) {
183+
return 1;
184+
}
185+
if (params.candidate.origin === "bundled") {
186+
// Bundled plugin ids are reserved unless the operator explicitly overrides them.
187+
return 2;
188+
}
189+
if (params.candidate.origin === "workspace") {
190+
return 3;
179191
}
180-
return PLUGIN_ORIGIN_RANK[params.candidate.origin];
192+
return 4;
181193
}
182194

183195
export function loadPluginManifestRegistry(params: {

0 commit comments

Comments
 (0)