From 34daed1d1eae99513ea0680218531a58831584ad Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 2 Mar 2026 21:31:18 +0000 Subject: [PATCH] refactor(core): dedupe infra, media, pairing, and plugin helpers --- src/infra/fs-safe.test.ts | 221 ++++++++++-------- .../targets.channel-resolution.test.ts | 57 ++--- src/media-understanding/resolve.test.ts | 25 +- src/media-understanding/runner.entries.ts | 42 ++-- src/pairing/setup-code.test.ts | 20 +- src/plugin-sdk/slack-message-actions.test.ts | 17 +- src/plugins/discovery.test.ts | 17 +- src/plugins/http-registry.test.ts | 94 ++++---- src/plugins/loader.ts | 56 ++--- src/plugins/tools.optional.test.ts | 67 ++---- src/routing/resolve-route.test.ts | 25 +- 11 files changed, 301 insertions(+), 340 deletions(-) diff --git a/src/infra/fs-safe.test.ts b/src/infra/fs-safe.test.ts index cae7bd418bf..4ee2da1b210 100644 --- a/src/infra/fs-safe.test.ts +++ b/src/infra/fs-safe.test.ts @@ -24,6 +24,81 @@ afterEach(async () => { await tempDirs.cleanup(); }); +async function expectWriteOpenRaceIsBlocked(params: { + slotPath: string; + outsideDir: string; + runWrite: () => Promise; +}): Promise { + await withRealpathSymlinkRebindRace({ + shouldFlip: (realpathInput) => realpathInput.endsWith(path.join("slot", "target.txt")), + symlinkPath: params.slotPath, + symlinkTarget: params.outsideDir, + timing: "before-realpath", + run: async () => { + await expect(params.runWrite()).rejects.toMatchObject({ code: "outside-workspace" }); + }, + }); +} + +async function expectSymlinkWriteRaceRejectsOutside(params: { + slotPath: string; + outsideDir: string; + runWrite: (relativePath: string) => Promise; +}): Promise { + const relativePath = path.join("slot", "target.txt"); + await expectWriteOpenRaceIsBlocked({ + slotPath: params.slotPath, + outsideDir: params.outsideDir, + runWrite: async () => await params.runWrite(relativePath), + }); +} + +async function withOutsideHardlinkAlias(params: { + aliasPath: string; + run: (outsideFile: string) => Promise; +}): Promise { + const outside = await tempDirs.make("openclaw-fs-safe-outside-"); + const outsideFile = path.join(outside, "outside.txt"); + await fs.writeFile(outsideFile, "outside"); + try { + try { + await fs.link(outsideFile, params.aliasPath); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === "EXDEV") { + return; + } + throw err; + } + await params.run(outsideFile); + } finally { + await fs.rm(params.aliasPath, { force: true }); + await fs.rm(outsideFile, { force: true }); + } +} + +async function setupSymlinkWriteRaceFixture(options?: { seedInsideTarget?: boolean }): Promise<{ + root: string; + outside: string; + slot: string; + outsideTarget: string; +}> { + const root = await tempDirs.make("openclaw-fs-safe-root-"); + const inside = path.join(root, "inside"); + const outside = await tempDirs.make("openclaw-fs-safe-outside-"); + await fs.mkdir(inside, { recursive: true }); + if (options?.seedInsideTarget) { + await fs.writeFile(path.join(inside, "target.txt"), "inside"); + } + const outsideTarget = path.join(outside, "target.txt"); + await fs.writeFile(outsideTarget, "X".repeat(4096)); + const slot = path.join(root, "slot"); + await createRebindableDirectoryAlias({ + aliasPath: slot, + targetPath: inside, + }); + return { root, outside, slot, outsideTarget }; +} + describe("fs-safe", () => { it("reads a local file safely", async () => { const dir = await tempDirs.make("openclaw-fs-safe-"); @@ -147,29 +222,18 @@ describe("fs-safe", () => { it.runIf(process.platform !== "win32")("blocks hardlink aliases under root", async () => { const root = await tempDirs.make("openclaw-fs-safe-root-"); - const outside = await tempDirs.make("openclaw-fs-safe-outside-"); - const outsideFile = path.join(outside, "outside.txt"); const hardlinkPath = path.join(root, "link.txt"); - await fs.writeFile(outsideFile, "outside"); - try { - try { - await fs.link(outsideFile, hardlinkPath); - } catch (err) { - if ((err as NodeJS.ErrnoException).code === "EXDEV") { - return; - } - throw err; - } - await expect( - openFileWithinRoot({ - rootDir: root, - relativePath: "link.txt", - }), - ).rejects.toMatchObject({ code: "invalid-path" }); - } finally { - await fs.rm(hardlinkPath, { force: true }); - await fs.rm(outsideFile, { force: true }); - } + await withOutsideHardlinkAlias({ + aliasPath: hardlinkPath, + run: async () => { + await expect( + openFileWithinRoot({ + rootDir: root, + relativePath: "link.txt", + }), + ).rejects.toMatchObject({ code: "invalid-path" }); + }, + }); }); it("writes a file within root safely", async () => { @@ -245,99 +309,58 @@ describe("fs-safe", () => { it.runIf(process.platform !== "win32")("rejects writing through hardlink aliases", async () => { const root = await tempDirs.make("openclaw-fs-safe-root-"); - const outside = await tempDirs.make("openclaw-fs-safe-outside-"); - const outsideFile = path.join(outside, "outside.txt"); const hardlinkPath = path.join(root, "alias.txt"); - await fs.writeFile(outsideFile, "outside"); - try { - try { - await fs.link(outsideFile, hardlinkPath); - } catch (err) { - if ((err as NodeJS.ErrnoException).code === "EXDEV") { - return; - } - throw err; - } - await expect( - writeFileWithinRoot({ - rootDir: root, - relativePath: "alias.txt", - data: "pwned", - }), - ).rejects.toMatchObject({ code: "invalid-path" }); - await expect(fs.readFile(outsideFile, "utf8")).resolves.toBe("outside"); - } finally { - await fs.rm(hardlinkPath, { force: true }); - await fs.rm(outsideFile, { force: true }); - } - }); - - it("does not truncate out-of-root file when symlink retarget races write open", async () => { - const root = await tempDirs.make("openclaw-fs-safe-root-"); - const inside = path.join(root, "inside"); - const outside = await tempDirs.make("openclaw-fs-safe-outside-"); - await fs.mkdir(inside, { recursive: true }); - const insideTarget = path.join(inside, "target.txt"); - const outsideTarget = path.join(outside, "target.txt"); - await fs.writeFile(insideTarget, "inside"); - await fs.writeFile(outsideTarget, "X".repeat(4096)); - const slot = path.join(root, "slot"); - await createRebindableDirectoryAlias({ - aliasPath: slot, - targetPath: inside, - }); - - await withRealpathSymlinkRebindRace({ - shouldFlip: (realpathInput) => realpathInput.endsWith(path.join("slot", "target.txt")), - symlinkPath: slot, - symlinkTarget: outside, - timing: "before-realpath", - run: async () => { + await withOutsideHardlinkAlias({ + aliasPath: hardlinkPath, + run: async (outsideFile) => { await expect( writeFileWithinRoot({ rootDir: root, - relativePath: path.join("slot", "target.txt"), - data: "new-content", - mkdir: false, + relativePath: "alias.txt", + data: "pwned", }), - ).rejects.toMatchObject({ code: "outside-workspace" }); + ).rejects.toMatchObject({ code: "invalid-path" }); + await expect(fs.readFile(outsideFile, "utf8")).resolves.toBe("outside"); }, }); + }); + + it("does not truncate out-of-root file when symlink retarget races write open", async () => { + const { root, outside, slot, outsideTarget } = await setupSymlinkWriteRaceFixture({ + seedInsideTarget: true, + }); + + await expectSymlinkWriteRaceRejectsOutside({ + slotPath: slot, + outsideDir: outside, + runWrite: async (relativePath) => + await writeFileWithinRoot({ + rootDir: root, + relativePath, + data: "new-content", + mkdir: false, + }), + }); await expect(fs.readFile(outsideTarget, "utf8")).resolves.toBe("X".repeat(4096)); }); it("does not clobber out-of-root file when symlink retarget races write-from-path open", async () => { - const root = await tempDirs.make("openclaw-fs-safe-root-"); - const inside = path.join(root, "inside"); - const outside = await tempDirs.make("openclaw-fs-safe-outside-"); + const { root, outside, slot, outsideTarget } = await setupSymlinkWriteRaceFixture(); const sourceDir = await tempDirs.make("openclaw-fs-safe-source-"); const sourcePath = path.join(sourceDir, "source.txt"); await fs.writeFile(sourcePath, "new-content"); - await fs.mkdir(inside, { recursive: true }); - const outsideTarget = path.join(outside, "target.txt"); - await fs.writeFile(outsideTarget, "X".repeat(4096)); - const slot = path.join(root, "slot"); - await createRebindableDirectoryAlias({ - aliasPath: slot, - targetPath: inside, - }); - await withRealpathSymlinkRebindRace({ - shouldFlip: (realpathInput) => realpathInput.endsWith(path.join("slot", "target.txt")), - symlinkPath: slot, - symlinkTarget: outside, - timing: "before-realpath", - run: async () => { - await expect( - writeFileFromPathWithinRoot({ - rootDir: root, - relativePath: path.join("slot", "target.txt"), - sourcePath, - mkdir: false, - }), - ).rejects.toMatchObject({ code: "outside-workspace" }); - }, + await expectSymlinkWriteRaceRejectsOutside({ + slotPath: slot, + outsideDir: outside, + runWrite: async (relativePath) => + await writeFileFromPathWithinRoot({ + rootDir: root, + relativePath, + sourcePath, + mkdir: false, + }), }); await expect(fs.readFile(outsideTarget, "utf8")).resolves.toBe("X".repeat(4096)); diff --git a/src/infra/outbound/targets.channel-resolution.test.ts b/src/infra/outbound/targets.channel-resolution.test.ts index c1632071d13..d426e98f4fa 100644 --- a/src/infra/outbound/targets.channel-resolution.test.ts +++ b/src/infra/outbound/targets.channel-resolution.test.ts @@ -15,6 +15,17 @@ function passthroughPluginAutoEnable(config: unknown) { return { config, changes: [] as unknown[] }; } +function createTelegramPlugin() { + return { + id: "telegram", + meta: { label: "Telegram" }, + config: { + listAccountIds: () => [], + resolveAccount: () => ({}), + }, + }; +} + vi.mock("../../channels/plugins/index.js", () => ({ getChannelPlugin: mocks.getChannelPlugin, normalizeChannelId: normalizeChannel, @@ -39,6 +50,13 @@ import { resolveOutboundTarget } from "./targets.js"; describe("resolveOutboundTarget channel resolution", () => { let registrySeq = 0; + const resolveTelegramTarget = () => + resolveOutboundTarget({ + channel: "telegram", + to: "123456", + cfg: { channels: { telegram: { botToken: "test-token" } } }, + mode: "explicit", + }); beforeEach(() => { registrySeq += 1; @@ -48,39 +66,20 @@ describe("resolveOutboundTarget channel resolution", () => { }); it("recovers telegram plugin resolution so announce delivery does not fail with Unsupported channel: telegram", () => { - const telegramPlugin = { - id: "telegram", - meta: { label: "Telegram" }, - config: { - listAccountIds: () => [], - resolveAccount: () => ({}), - }, - }; + const telegramPlugin = createTelegramPlugin(); mocks.getChannelPlugin .mockReturnValueOnce(undefined) .mockReturnValueOnce(telegramPlugin) .mockReturnValue(telegramPlugin); - const result = resolveOutboundTarget({ - channel: "telegram", - to: "123456", - cfg: { channels: { telegram: { botToken: "test-token" } } }, - mode: "explicit", - }); + const result = resolveTelegramTarget(); expect(result).toEqual({ ok: true, to: "123456" }); expect(mocks.loadOpenClawPlugins).toHaveBeenCalledTimes(1); }); it("retries bootstrap on subsequent resolve when the first bootstrap attempt fails", () => { - const telegramPlugin = { - id: "telegram", - meta: { label: "Telegram" }, - config: { - listAccountIds: () => [], - resolveAccount: () => ({}), - }, - }; + const telegramPlugin = createTelegramPlugin(); mocks.getChannelPlugin .mockReturnValueOnce(undefined) .mockReturnValueOnce(undefined) @@ -93,18 +92,8 @@ describe("resolveOutboundTarget channel resolution", () => { }) .mockImplementation(() => undefined); - const first = resolveOutboundTarget({ - channel: "telegram", - to: "123456", - cfg: { channels: { telegram: { botToken: "test-token" } } }, - mode: "explicit", - }); - const second = resolveOutboundTarget({ - channel: "telegram", - to: "123456", - cfg: { channels: { telegram: { botToken: "test-token" } } }, - mode: "explicit", - }); + const first = resolveTelegramTarget(); + const second = resolveTelegramTarget(); expect(first.ok).toBe(false); expect(second).toEqual({ ok: true, to: "123456" }); diff --git a/src/media-understanding/resolve.test.ts b/src/media-understanding/resolve.test.ts index 90dba89cbf8..2184a3242a6 100644 --- a/src/media-understanding/resolve.test.ts +++ b/src/media-understanding/resolve.test.ts @@ -89,6 +89,21 @@ describe("resolveEntriesWithActiveFallback", () => { }); } + function expectResolvedProviders(params: { + cfg: OpenClawConfig; + capability: ResolveWithFallbackInput["capability"]; + config: ResolveWithFallbackInput["config"]; + providers: string[]; + }) { + const entries = resolveWithActiveFallback({ + cfg: params.cfg, + capability: params.capability, + config: params.config, + }); + expect(entries).toHaveLength(params.providers.length); + expect(entries.map((entry) => entry.provider)).toEqual(params.providers); + } + it("uses active model when enabled and no models are configured", () => { const cfg: OpenClawConfig = { tools: { @@ -98,13 +113,12 @@ describe("resolveEntriesWithActiveFallback", () => { }, }; - const entries = resolveWithActiveFallback({ + expectResolvedProviders({ cfg, capability: "audio", config: cfg.tools?.media?.audio, + providers: ["groq"], }); - expect(entries).toHaveLength(1); - expect(entries[0]?.provider).toBe("groq"); }); it("ignores active model when configured entries exist", () => { @@ -116,13 +130,12 @@ describe("resolveEntriesWithActiveFallback", () => { }, }; - const entries = resolveWithActiveFallback({ + expectResolvedProviders({ cfg, capability: "audio", config: cfg.tools?.media?.audio, + providers: ["openai"], }); - expect(entries).toHaveLength(1); - expect(entries[0]?.provider).toBe("openai"); }); it("skips active model when provider lacks capability", () => { diff --git a/src/media-understanding/runner.entries.ts b/src/media-understanding/runner.entries.ts index 36e6a89b438..dfd5ba32155 100644 --- a/src/media-understanding/runner.entries.ts +++ b/src/media-understanding/runner.entries.ts @@ -400,33 +400,21 @@ export async function runProviderEntry(params: { timeoutMs, }); const provider = getMediaUnderstandingProvider(providerId, params.providerRegistry); - const result = provider?.describeImage - ? await provider.describeImage({ - buffer: media.buffer, - fileName: media.fileName, - mime: media.mime, - model: modelId, - provider: providerId, - prompt, - timeoutMs, - profile: entry.profile, - preferredProfile: entry.preferredProfile, - agentDir: params.agentDir, - cfg: params.cfg, - }) - : await describeImageWithModel({ - buffer: media.buffer, - fileName: media.fileName, - mime: media.mime, - model: modelId, - provider: providerId, - prompt, - timeoutMs, - profile: entry.profile, - preferredProfile: entry.preferredProfile, - agentDir: params.agentDir, - cfg: params.cfg, - }); + const imageInput = { + buffer: media.buffer, + fileName: media.fileName, + mime: media.mime, + model: modelId, + provider: providerId, + prompt, + timeoutMs, + profile: entry.profile, + preferredProfile: entry.preferredProfile, + agentDir: params.agentDir, + cfg: params.cfg, + }; + const describeImage = provider?.describeImage ?? describeImageWithModel; + const result = await describeImage(imageInput); return { kind: "image.description", attachmentIndex: params.attachmentIndex, diff --git a/src/pairing/setup-code.test.ts b/src/pairing/setup-code.test.ts index abbe7fe3c2c..fefdfbe24a2 100644 --- a/src/pairing/setup-code.test.ts +++ b/src/pairing/setup-code.test.ts @@ -2,6 +2,14 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { encodePairingSetupCode, resolvePairingSetupFromConfig } from "./setup-code.js"; describe("pairing setup code", () => { + function createTailnetDnsRunner() { + return vi.fn(async () => ({ + code: 0, + stdout: '{"Self":{"DNSName":"mb-server.tailnet.ts.net."}}', + stderr: "", + })); + } + beforeEach(() => { vi.stubEnv("OPENCLAW_GATEWAY_TOKEN", ""); vi.stubEnv("CLAWDBOT_GATEWAY_TOKEN", ""); @@ -83,11 +91,7 @@ describe("pairing setup code", () => { }); it("uses tailscale serve DNS when available", async () => { - const runCommandWithTimeout = vi.fn(async () => ({ - code: 0, - stdout: '{"Self":{"DNSName":"mb-server.tailnet.ts.net."}}', - stderr: "", - })); + const runCommandWithTimeout = createTailnetDnsRunner(); const resolved = await resolvePairingSetupFromConfig( { @@ -114,11 +118,7 @@ describe("pairing setup code", () => { }); it("prefers gateway.remote.url over tailscale when requested", async () => { - const runCommandWithTimeout = vi.fn(async () => ({ - code: 0, - stdout: '{"Self":{"DNSName":"mb-server.tailnet.ts.net."}}', - stderr: "", - })); + const runCommandWithTimeout = createTailnetDnsRunner(); const resolved = await resolvePairingSetupFromConfig( { diff --git a/src/plugin-sdk/slack-message-actions.test.ts b/src/plugin-sdk/slack-message-actions.test.ts index 109b825fab9..9c098bffe76 100644 --- a/src/plugin-sdk/slack-message-actions.test.ts +++ b/src/plugin-sdk/slack-message-actions.test.ts @@ -1,12 +1,16 @@ import { describe, expect, it, vi } from "vitest"; import { handleSlackMessageAction } from "./slack-message-actions.js"; +function createInvokeSpy() { + return vi.fn(async (action: Record) => ({ + ok: true, + content: action, + })); +} + describe("handleSlackMessageAction", () => { it("maps download-file to the internal downloadFile action", async () => { - const invoke = vi.fn(async (action: Record) => ({ - ok: true, - content: action, - })); + const invoke = createInvokeSpy(); await handleSlackMessageAction({ providerId: "slack", @@ -34,10 +38,7 @@ describe("handleSlackMessageAction", () => { }); it("maps download-file target aliases to scope fields", async () => { - const invoke = vi.fn(async (action: Record) => ({ - ok: true, - content: action, - })); + const invoke = createInvokeSpy(); await handleSlackMessageAction({ providerId: "slack", diff --git a/src/plugins/discovery.test.ts b/src/plugins/discovery.test.ts index 806411c3a94..e896910268b 100644 --- a/src/plugins/discovery.test.ts +++ b/src/plugins/discovery.test.ts @@ -26,6 +26,15 @@ async function withStateDir(stateDir: string, fn: () => Promise) { ); } +async function discoverWithStateDir( + stateDir: string, + params: Parameters[0], +) { + return await withStateDir(stateDir, async () => { + return discoverOpenClawPlugins(params); + }); +} + function writePluginPackageManifest(params: { packageDir: string; packageName: string; @@ -197,9 +206,7 @@ describe("discoverOpenClawPlugins", () => { }); fs.writeFileSync(outside, "export default function () {}", "utf-8"); - const result = await withStateDir(stateDir, async () => { - return discoverOpenClawPlugins({}); - }); + const result = await discoverWithStateDir(stateDir, {}); expect(result.candidates).toHaveLength(0); expectEscapesPackageDiagnostic(result.diagnostics); @@ -225,9 +232,7 @@ describe("discoverOpenClawPlugins", () => { extensions: ["./linked/escape.ts"], }); - const { candidates, diagnostics } = await withStateDir(stateDir, async () => { - return discoverOpenClawPlugins({}); - }); + const { candidates, diagnostics } = await discoverWithStateDir(stateDir, {}); expect(candidates.some((candidate) => candidate.idHint === "pack")).toBe(false); expectEscapesPackageDiagnostic(diagnostics); diff --git a/src/plugins/http-registry.test.ts b/src/plugins/http-registry.test.ts index 73174d6385d..179ddadac5e 100644 --- a/src/plugins/http-registry.test.ts +++ b/src/plugins/http-registry.test.ts @@ -2,6 +2,41 @@ import { describe, expect, it, vi } from "vitest"; import { registerPluginHttpRoute } from "./http-registry.js"; import { createEmptyPluginRegistry } from "./registry.js"; +function expectRouteRegistrationDenied(params: { + replaceExisting: boolean; + expectedLogFragment: string; +}) { + const registry = createEmptyPluginRegistry(); + const logs: string[] = []; + + registerPluginHttpRoute({ + path: "/plugins/demo", + auth: "plugin", + handler: vi.fn(), + registry, + pluginId: "demo-a", + source: "demo-a-src", + log: (msg) => logs.push(msg), + }); + + const unregister = registerPluginHttpRoute({ + path: "/plugins/demo", + auth: "plugin", + ...(params.replaceExisting ? { replaceExisting: true } : {}), + handler: vi.fn(), + registry, + pluginId: "demo-b", + source: "demo-b-src", + log: (msg) => logs.push(msg), + }); + + expect(registry.httpRoutes).toHaveLength(1); + expect(logs.at(-1)).toContain(params.expectedLogFragment); + + unregister(); + expect(registry.httpRoutes).toHaveLength(1); +} + describe("registerPluginHttpRoute", () => { it("registers route and unregisters it", () => { const registry = createEmptyPluginRegistry(); @@ -84,65 +119,16 @@ describe("registerPluginHttpRoute", () => { }); it("rejects conflicting route registrations without replaceExisting", () => { - const registry = createEmptyPluginRegistry(); - const logs: string[] = []; - - registerPluginHttpRoute({ - path: "/plugins/demo", - auth: "plugin", - handler: vi.fn(), - registry, - pluginId: "demo-a", - source: "demo-a-src", - log: (msg) => logs.push(msg), + expectRouteRegistrationDenied({ + replaceExisting: false, + expectedLogFragment: "route conflict", }); - - const unregister = registerPluginHttpRoute({ - path: "/plugins/demo", - auth: "plugin", - handler: vi.fn(), - registry, - pluginId: "demo-b", - source: "demo-b-src", - log: (msg) => logs.push(msg), - }); - - expect(registry.httpRoutes).toHaveLength(1); - expect(logs.at(-1)).toContain("route conflict"); - - unregister(); - expect(registry.httpRoutes).toHaveLength(1); }); it("rejects route replacement when a different plugin owns the route", () => { - const registry = createEmptyPluginRegistry(); - const logs: string[] = []; - - registerPluginHttpRoute({ - path: "/plugins/demo", - auth: "plugin", - handler: vi.fn(), - registry, - pluginId: "demo-a", - source: "demo-a-src", - log: (msg) => logs.push(msg), - }); - - const unregister = registerPluginHttpRoute({ - path: "/plugins/demo", - auth: "plugin", + expectRouteRegistrationDenied({ replaceExisting: true, - handler: vi.fn(), - registry, - pluginId: "demo-b", - source: "demo-b-src", - log: (msg) => logs.push(msg), + expectedLogFragment: "route replacement denied", }); - - expect(registry.httpRoutes).toHaveLength(1); - expect(logs.at(-1)).toContain("route replacement denied"); - - unregister(); - expect(registry.httpRoutes).toHaveLength(1); }); }); diff --git a/src/plugins/loader.ts b/src/plugins/loader.ts index a52fdff9c3a..deb0fa02cd3 100644 --- a/src/plugins/loader.ts +++ b/src/plugins/loader.ts @@ -507,6 +507,18 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi record.kind = manifestRecord.kind; record.configUiHints = manifestRecord.configUiHints; record.configJsonSchema = manifestRecord.configSchema; + const pushPluginLoadError = (message: string) => { + record.status = "error"; + record.error = message; + registry.plugins.push(record); + seenIds.set(pluginId, candidate.origin); + registry.diagnostics.push({ + level: "error", + pluginId: record.id, + source: record.source, + message: record.error, + }); + }; if (!enableState.enabled) { record.status = "disabled"; @@ -517,16 +529,7 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi } if (!manifestRecord.configSchema) { - record.status = "error"; - record.error = "missing config schema"; - registry.plugins.push(record); - seenIds.set(pluginId, candidate.origin); - registry.diagnostics.push({ - level: "error", - pluginId: record.id, - source: record.source, - message: record.error, - }); + pushPluginLoadError("missing config schema"); continue; } @@ -541,16 +544,7 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi skipLexicalRootCheck: true, }); if (!opened.ok) { - record.status = "error"; - record.error = "plugin entry path escapes plugin root or fails alias checks"; - registry.plugins.push(record); - seenIds.set(pluginId, candidate.origin); - registry.diagnostics.push({ - level: "error", - pluginId: record.id, - source: record.source, - message: record.error, - }); + pushPluginLoadError("plugin entry path escapes plugin root or fails alias checks"); continue; } const safeSource = opened.path; @@ -634,16 +628,7 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi if (!validatedConfig.ok) { logger.error(`[plugins] ${record.id} invalid config: ${validatedConfig.errors?.join(", ")}`); - record.status = "error"; - record.error = `invalid config: ${validatedConfig.errors?.join(", ")}`; - registry.plugins.push(record); - seenIds.set(pluginId, candidate.origin); - registry.diagnostics.push({ - level: "error", - pluginId: record.id, - source: record.source, - message: record.error, - }); + pushPluginLoadError(`invalid config: ${validatedConfig.errors?.join(", ")}`); continue; } @@ -655,16 +640,7 @@ export function loadOpenClawPlugins(options: PluginLoadOptions = {}): PluginRegi if (typeof register !== "function") { logger.error(`[plugins] ${record.id} missing register/activate export`); - record.status = "error"; - record.error = "plugin export missing register/activate"; - registry.plugins.push(record); - seenIds.set(pluginId, candidate.origin); - registry.diagnostics.push({ - level: "error", - pluginId: record.id, - source: record.source, - message: record.error, - }); + pushPluginLoadError("plugin export missing register/activate"); continue; } diff --git a/src/plugins/tools.optional.test.ts b/src/plugins/tools.optional.test.ts index a3c4c2fb249..da2ba912ab7 100644 --- a/src/plugins/tools.optional.test.ts +++ b/src/plugins/tools.optional.test.ts @@ -71,64 +71,47 @@ function resolveWithConflictingCoreName(options?: { suppressNameConflicts?: bool }); } +function setOptionalDemoRegistry() { + setRegistry([ + { + pluginId: "optional-demo", + optional: true, + source: "/tmp/optional-demo.js", + factory: () => makeTool("optional_tool"), + }, + ]); +} + +function resolveOptionalDemoTools(toolAllowlist?: string[]) { + return resolvePluginTools({ + context: createContext() as never, + ...(toolAllowlist ? { toolAllowlist } : {}), + }); +} + describe("resolvePluginTools optional tools", () => { beforeEach(() => { loadOpenClawPluginsMock.mockClear(); }); it("skips optional tools without explicit allowlist", () => { - setRegistry([ - { - pluginId: "optional-demo", - optional: true, - source: "/tmp/optional-demo.js", - factory: () => makeTool("optional_tool"), - }, - ]); - - const tools = resolvePluginTools({ - context: createContext() as never, - }); + setOptionalDemoRegistry(); + const tools = resolveOptionalDemoTools(); expect(tools).toHaveLength(0); }); it("allows optional tools by tool name", () => { - setRegistry([ - { - pluginId: "optional-demo", - optional: true, - source: "/tmp/optional-demo.js", - factory: () => makeTool("optional_tool"), - }, - ]); - - const tools = resolvePluginTools({ - context: createContext() as never, - toolAllowlist: ["optional_tool"], - }); + setOptionalDemoRegistry(); + const tools = resolveOptionalDemoTools(["optional_tool"]); expect(tools.map((tool) => tool.name)).toEqual(["optional_tool"]); }); it("allows optional tools via plugin-scoped allowlist entries", () => { - setRegistry([ - { - pluginId: "optional-demo", - optional: true, - source: "/tmp/optional-demo.js", - factory: () => makeTool("optional_tool"), - }, - ]); - - const toolsByPlugin = resolvePluginTools({ - context: createContext() as never, - toolAllowlist: ["optional-demo"], - }); - const toolsByGroup = resolvePluginTools({ - context: createContext() as never, - toolAllowlist: ["group:plugins"], - }); + setOptionalDemoRegistry(); + const toolsByPlugin = resolveOptionalDemoTools(["optional-demo"]); + const toolsByGroup = resolveOptionalDemoTools(["group:plugins"]); expect(toolsByPlugin.map((tool) => tool.name)).toEqual(["optional_tool"]); expect(toolsByGroup.map((tool) => tool.name)).toEqual(["optional_tool"]); diff --git a/src/routing/resolve-route.test.ts b/src/routing/resolve-route.test.ts index a685baa5bc7..5d23303e3ca 100644 --- a/src/routing/resolve-route.test.ts +++ b/src/routing/resolve-route.test.ts @@ -4,6 +4,15 @@ import type { OpenClawConfig } from "../config/config.js"; import { resolveAgentRoute } from "./resolve-route.js"; describe("resolveAgentRoute", () => { + const resolveDiscordGuildRoute = (cfg: OpenClawConfig) => + resolveAgentRoute({ + cfg, + channel: "discord", + accountId: "default", + peer: { kind: "channel", id: "c1" }, + guildId: "g1", + }); + test("defaults to main/default when no bindings exist", () => { const cfg: OpenClawConfig = {}; const route = resolveAgentRoute({ @@ -123,13 +132,7 @@ describe("resolveAgentRoute", () => { }, ], }; - const route = resolveAgentRoute({ - cfg, - channel: "discord", - accountId: "default", - peer: { kind: "channel", id: "c1" }, - guildId: "g1", - }); + const route = resolveDiscordGuildRoute(cfg); expect(route.agentId).toBe("chan"); expect(route.sessionKey).toBe("agent:chan:discord:channel:c1"); expect(route.matchedBy).toBe("binding.peer"); @@ -163,13 +166,7 @@ describe("resolveAgentRoute", () => { }, ], }; - const route = resolveAgentRoute({ - cfg, - channel: "discord", - accountId: "default", - peer: { kind: "channel", id: "c1" }, - guildId: "g1", - }); + const route = resolveDiscordGuildRoute(cfg); expect(route.agentId).toBe("guild"); expect(route.matchedBy).toBe("binding.guild"); });