refactor(core): dedupe infra, media, pairing, and plugin helpers

2026-03-12 15:30:39 +00:00 · 2026-03-02 21:31:18 +00:00
parent 91dd89313a
commit 34daed1d1e
11 changed files with 301 additions and 340 deletions
--- 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<void>;
+}): Promise<void> {
+  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<void>;
+}): Promise<void> {
+  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<void>;
+}): Promise<void> {
+  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));
--- 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" });
--- 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", () => {
--- 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,
--- 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(
      {
--- 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<string, unknown>) => ({
+    ok: true,
+    content: action,
+  }));
+}
+
 describe("handleSlackMessageAction", () => {
  it("maps download-file to the internal downloadFile action", async () => {
-    const invoke = vi.fn(async (action: Record<string, unknown>) => ({
-      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<string, unknown>) => ({
-      ok: true,
-      content: action,
-    }));
+    const invoke = createInvokeSpy();

    await handleSlackMessageAction({
      providerId: "slack",
--- a/src/plugins/discovery.test.ts
+++ b/src/plugins/discovery.test.ts
@@ -26,6 +26,15 @@ async function withStateDir<T>(stateDir: string, fn: () => Promise<T>) {
  );
 }

+async function discoverWithStateDir(
+  stateDir: string,
+  params: Parameters<typeof discoverOpenClawPlugins>[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);
--- 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);
  });
 });
--- 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;
    }

--- 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"]);
--- 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");
  });