From 781295c14b450a4f7aef7ef5b2b543c8a26f6eda Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 24 Mar 2026 15:47:44 +0000 Subject: [PATCH] refactor: dedupe test and script helpers --- ...age-handler.preflight.acp-bindings.test.ts | 211 +++--- .../monitor/monitor.agent-components.test.ts | 44 +- .../discord/src/monitor/monitor.test.ts | 240 +++---- .../native-command.plugin-dispatch.test.ts | 171 +++-- .../src/matrix/sdk/crypto-bootstrap.test.ts | 228 +++--- .../src/matrix/sdk/crypto-facade.test.ts | 144 ++-- .../src/matrix/sdk/recovery-key-store.test.ts | 248 +++---- .../matrix/sdk/verification-manager.test.ts | 197 +++--- extensions/zalo/src/channel.directory.test.ts | 37 +- .../zalo/src/monitor.image.polling.test.ts | 172 +---- .../src/monitor.pairing.lifecycle.test.ts | 119 ++-- .../src/monitor.reply-once.lifecycle.test.ts | 122 ++-- extensions/zalo/src/monitor.webhook.test.ts | 141 +--- scripts/check-architecture-smells.mjs | 106 ++- .../check-extension-plugin-sdk-boundary.mjs | 78 +-- scripts/check-ingress-agent-owner-context.mjs | 22 +- scripts/check-no-random-messaging-tmp.mjs | 37 +- scripts/check-no-raw-channel-fetch.mjs | 18 +- scripts/check-no-register-http-handler.mjs | 18 +- ...check-plugin-extension-import-boundary.mjs | 184 ++--- scripts/check-plugin-sdk-subpath-exports.mjs | 34 +- .../check-web-search-provider-boundaries.mjs | 72 +- scripts/generate-bundled-plugin-metadata.mjs | 82 +-- ...enerate-bundled-provider-auth-env-vars.mjs | 78 +-- scripts/lib/arg-utils.mjs | 169 +++++ scripts/lib/bundled-plugin-source-utils.mjs | 51 ++ scripts/lib/generated-output-utils.mjs | 45 ++ scripts/lib/guard-inventory-utils.mjs | 128 ++++ scripts/lib/ts-guard-utils.mjs | 15 + scripts/lib/vitest-report-cli-utils.mjs | 31 + scripts/test-find-thread-candidates.mjs | 146 ++-- scripts/test-hotspots.mjs | 53 +- scripts/test-perf-budget.mjs | 69 +- scripts/test-runner-manifest.mjs | 46 +- scripts/test-update-memory-hotspots.mjs | 82 +-- scripts/test-update-timings.mjs | 66 +- ...mpt.spawn-workspace.context-engine.test.ts | 231 +++---- .../transcript-rewrite.test.ts | 324 +++------ .../reply/directive-handling.model.test.ts | 364 ++++------ src/infra/outbound/outbound-session.test.ts | 24 + src/infra/outbound/outbound.test.ts | 647 ------------------ src/infra/outbound/payloads.test.ts | 11 + test/architecture-smells.test.ts | 22 +- test/extension-plugin-sdk-boundary.test.ts | 22 +- test/helpers/captured-io.ts | 20 + .../extensions/discord-component-runtime.ts | 85 +++ test/helpers/extensions/zalo-lifecycle.ts | 157 ++++- test/helpers/pattern-file.ts | 23 + test/plugin-extension-import-boundary.test.ts | 22 +- test/vitest-extensions-config.test.ts | 23 +- test/vitest-unit-config.test.ts | 25 +- test/web-search-provider-boundary.test.ts | 22 +- vitest.channels.config.ts | 16 +- vitest.extensions.config.ts | 16 +- vitest.pattern-file.ts | 20 + vitest.unit.config.ts | 21 +- 56 files changed, 2277 insertions(+), 3522 deletions(-) create mode 100644 scripts/lib/arg-utils.mjs create mode 100644 scripts/lib/bundled-plugin-source-utils.mjs create mode 100644 scripts/lib/generated-output-utils.mjs create mode 100644 scripts/lib/guard-inventory-utils.mjs create mode 100644 scripts/lib/vitest-report-cli-utils.mjs create mode 100644 test/helpers/captured-io.ts create mode 100644 test/helpers/extensions/discord-component-runtime.ts create mode 100644 test/helpers/pattern-file.ts create mode 100644 vitest.pattern-file.ts diff --git a/extensions/discord/src/monitor/message-handler.preflight.acp-bindings.test.ts b/extensions/discord/src/monitor/message-handler.preflight.acp-bindings.test.ts index 982b9589b22..8a1e7b13d43 100644 --- a/extensions/discord/src/monitor/message-handler.preflight.acp-bindings.test.ts +++ b/extensions/discord/src/monitor/message-handler.preflight.acp-bindings.test.ts @@ -166,6 +166,62 @@ function createBasePreflightParams(overrides?: Record) { } satisfies Parameters[0]; } +function createAllowedGuildEntries(requireMention = false) { + return { + [GUILD_ID]: { + id: GUILD_ID, + channels: { + [CHANNEL_ID]: { + allow: true, + enabled: true, + requireMention, + }, + }, + }, + }; +} + +function createHydratedGuildClient(restPayload: Record) { + const restGet = vi.fn(async () => restPayload); + const client = { + ...createGuildTextClient(CHANNEL_ID), + rest: { + get: restGet, + }, + } as unknown as Parameters[0]["client"]; + return { client, restGet }; +} + +async function runRestHydrationPreflight(params: { + messageId: string; + restPayload: Record; +}) { + const message = createDiscordMessage({ + id: params.messageId, + channelId: CHANNEL_ID, + content: "", + author: { + id: "user-1", + bot: false, + username: "alice", + }, + }); + const { client, restGet } = createHydratedGuildClient(params.restPayload); + const result = await preflightDiscordMessage( + createBasePreflightParams({ + client, + data: createGuildEvent({ + channelId: CHANNEL_ID, + guildId: GUILD_ID, + author: message.author, + message, + }), + guildEntries: createAllowedGuildEntries(false), + }), + ); + return { result, restGet }; +} + describe("preflightDiscordMessage configured ACP bindings", () => { beforeEach(() => { sessionBindingTesting.resetSessionBindingAdaptersForTests(); @@ -242,18 +298,7 @@ describe("preflightDiscordMessage configured ACP bindings", () => { author: message.author, message, }), - guildEntries: { - [GUILD_ID]: { - id: GUILD_ID, - channels: { - [CHANNEL_ID]: { - allow: true, - enabled: true, - requireMention: true, - }, - }, - }, - }, + guildEntries: createAllowedGuildEntries(true), }), ); @@ -263,59 +308,22 @@ describe("preflightDiscordMessage configured ACP bindings", () => { }); it("hydrates empty guild message payloads from REST before ensuring configured ACP bindings", async () => { - const message = createDiscordMessage({ - id: "m-rest", - channelId: CHANNEL_ID, - content: "", - author: { - id: "user-1", - bot: false, - username: "alice", + const { result, restGet } = await runRestHydrationPreflight({ + messageId: "m-rest", + restPayload: { + id: "m-rest", + content: "hello from rest", + attachments: [], + embeds: [], + mentions: [], + mention_roles: [], + mention_everyone: false, + author: { + id: "user-1", + username: "alice", + }, }, }); - const restGet = vi.fn(async () => ({ - id: "m-rest", - content: "hello from rest", - attachments: [], - embeds: [], - mentions: [], - mention_roles: [], - mention_everyone: false, - author: { - id: "user-1", - username: "alice", - }, - })); - const client = { - ...createGuildTextClient(CHANNEL_ID), - rest: { - get: restGet, - }, - } as unknown as Parameters[0]["client"]; - - const result = await preflightDiscordMessage( - createBasePreflightParams({ - client, - data: createGuildEvent({ - channelId: CHANNEL_ID, - guildId: GUILD_ID, - author: message.author, - message, - }), - guildEntries: { - [GUILD_ID]: { - id: GUILD_ID, - channels: { - [CHANNEL_ID]: { - allow: true, - enabled: true, - requireMention: false, - }, - }, - }, - }, - }), - ); expect(restGet).toHaveBeenCalledTimes(1); expect(result?.messageText).toBe("hello from rest"); @@ -324,65 +332,28 @@ describe("preflightDiscordMessage configured ACP bindings", () => { }); it("hydrates sticker-only guild message payloads from REST before ensuring configured ACP bindings", async () => { - const message = createDiscordMessage({ - id: "m-rest-sticker", - channelId: CHANNEL_ID, - content: "", - author: { - id: "user-1", - bot: false, - username: "alice", + const { result, restGet } = await runRestHydrationPreflight({ + messageId: "m-rest-sticker", + restPayload: { + id: "m-rest-sticker", + content: "", + attachments: [], + embeds: [], + mentions: [], + mention_roles: [], + mention_everyone: false, + sticker_items: [ + { + id: "sticker-1", + name: "wave", + }, + ], + author: { + id: "user-1", + username: "alice", + }, }, }); - const restGet = vi.fn(async () => ({ - id: "m-rest-sticker", - content: "", - attachments: [], - embeds: [], - mentions: [], - mention_roles: [], - mention_everyone: false, - sticker_items: [ - { - id: "sticker-1", - name: "wave", - }, - ], - author: { - id: "user-1", - username: "alice", - }, - })); - const client = { - ...createGuildTextClient(CHANNEL_ID), - rest: { - get: restGet, - }, - } as unknown as Parameters[0]["client"]; - - const result = await preflightDiscordMessage( - createBasePreflightParams({ - client, - data: createGuildEvent({ - channelId: CHANNEL_ID, - guildId: GUILD_ID, - author: message.author, - message, - }), - guildEntries: { - [GUILD_ID]: { - id: GUILD_ID, - channels: { - [CHANNEL_ID]: { - allow: true, - enabled: true, - requireMention: false, - }, - }, - }, - }, - }), - ); expect(restGet).toHaveBeenCalledTimes(1); expect(result?.messageText).toBe(" (1 sticker)"); diff --git a/extensions/discord/src/monitor/monitor.agent-components.test.ts b/extensions/discord/src/monitor/monitor.agent-components.test.ts index 4ac1fedcdb2..40555a58407 100644 --- a/extensions/discord/src/monitor/monitor.agent-components.test.ts +++ b/extensions/discord/src/monitor/monitor.agent-components.test.ts @@ -4,44 +4,13 @@ import type { DiscordAccountConfig } from "openclaw/plugin-sdk/config-runtime"; import { buildAgentSessionKey } from "openclaw/plugin-sdk/routing"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { peekSystemEvents, resetSystemEventsForTest } from "../../../../src/infra/system-events.ts"; +import { + readAllowFromStoreMock, + resetDiscordComponentRuntimeMocks, + upsertPairingRequestMock, +} from "../../../../test/helpers/extensions/discord-component-runtime.js"; import { createAgentComponentButton, createAgentSelectMenu } from "./agent-components.js"; -const readAllowFromStoreMock = vi.hoisted(() => vi.fn()); -const upsertPairingRequestMock = vi.hoisted(() => vi.fn()); - -vi.mock("openclaw/plugin-sdk/security-runtime", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - readStoreAllowFromForDmPolicy: async (params: { - provider: string; - accountId: string; - dmPolicy?: string | null; - shouldRead?: boolean | null; - }) => { - if (params.shouldRead === false || params.dmPolicy === "allowlist") { - return []; - } - return await readAllowFromStoreMock(params.provider, params.accountId); - }, - }; -}); - -vi.mock("openclaw/plugin-sdk/conversation-runtime", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - upsertChannelPairingRequest: (...args: unknown[]) => upsertPairingRequestMock(...args), - }; -}); -vi.mock("openclaw/plugin-sdk/conversation-runtime.js", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - upsertChannelPairingRequest: (...args: unknown[]) => upsertPairingRequestMock(...args), - }; -}); - describe("agent components", () => { const defaultDmSessionKey = buildAgentSessionKey({ agentId: "main", @@ -89,8 +58,7 @@ describe("agent components", () => { }; beforeEach(() => { - readAllowFromStoreMock.mockClear().mockResolvedValue([]); - upsertPairingRequestMock.mockClear().mockResolvedValue({ code: "PAIRCODE", created: true }); + resetDiscordComponentRuntimeMocks(); resetSystemEventsForTest(); }); diff --git a/extensions/discord/src/monitor/monitor.test.ts b/extensions/discord/src/monitor/monitor.test.ts index fa9e961dd76..05c8325dd3c 100644 --- a/extensions/discord/src/monitor/monitor.test.ts +++ b/extensions/discord/src/monitor/monitor.test.ts @@ -11,6 +11,14 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime"; import type { DiscordAccountConfig } from "openclaw/plugin-sdk/config-runtime"; import { buildPluginBindingApprovalCustomId } from "openclaw/plugin-sdk/conversation-runtime"; import { beforeEach, describe, expect, it, vi } from "vitest"; +import { + buildPluginBindingResolvedTextMock, + readAllowFromStoreMock, + recordInboundSessionMock, + resetDiscordComponentRuntimeMocks, + resolvePluginConversationBindingApprovalMock, + upsertPairingRequestMock, +} from "../../../../test/helpers/extensions/discord-component-runtime.js"; import { clearDiscordComponentEntries, registerDiscordComponentEntries, @@ -45,75 +53,25 @@ import { resolveDiscordReplyDeliveryPlan, } from "./threading.js"; -const readAllowFromStoreMock = vi.hoisted(() => vi.fn()); -const upsertPairingRequestMock = vi.hoisted(() => vi.fn()); const enqueueSystemEventMock = vi.hoisted(() => vi.fn()); const dispatchReplyMock = vi.hoisted(() => vi.fn()); -const recordInboundSessionMock = vi.hoisted(() => vi.fn()); const readSessionUpdatedAtMock = vi.hoisted(() => vi.fn()); const resolveStorePathMock = vi.hoisted(() => vi.fn()); const dispatchPluginInteractiveHandlerMock = vi.hoisted(() => vi.fn()); -const resolvePluginConversationBindingApprovalMock = vi.hoisted(() => vi.fn()); -const buildPluginBindingResolvedTextMock = vi.hoisted(() => vi.fn()); let lastDispatchCtx: Record | undefined; -vi.mock("openclaw/plugin-sdk/security-runtime", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - readStoreAllowFromForDmPolicy: async (params: { - provider: string; - accountId: string; - dmPolicy?: string | null; - shouldRead?: boolean | null; - }) => { - if (params.shouldRead === false || params.dmPolicy === "allowlist") { - return []; - } - return await readAllowFromStoreMock(params.provider, params.accountId); - }, - }; -}); - -vi.mock("openclaw/plugin-sdk/conversation-runtime", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - upsertChannelPairingRequest: (...args: unknown[]) => upsertPairingRequestMock(...args), - resolvePluginConversationBindingApproval: (...args: unknown[]) => - resolvePluginConversationBindingApprovalMock(...args), - buildPluginBindingResolvedText: (...args: unknown[]) => - buildPluginBindingResolvedTextMock(...args), - recordInboundSession: (...args: unknown[]) => recordInboundSessionMock(...args), - }; -}); -vi.mock("openclaw/plugin-sdk/conversation-runtime.js", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - upsertChannelPairingRequest: (...args: unknown[]) => upsertPairingRequestMock(...args), - resolvePluginConversationBindingApproval: (...args: unknown[]) => - resolvePluginConversationBindingApprovalMock(...args), - buildPluginBindingResolvedText: (...args: unknown[]) => - buildPluginBindingResolvedTextMock(...args), - recordInboundSession: (...args: unknown[]) => recordInboundSessionMock(...args), - }; -}); - -vi.mock("openclaw/plugin-sdk/infra-runtime", async (importOriginal) => { - const actual = await importOriginal(); +async function createInfraRuntimeMock( + importOriginal: () => Promise, +) { + const actual = await importOriginal(); return { ...actual, enqueueSystemEvent: (...args: unknown[]) => enqueueSystemEventMock(...args), }; -}); -vi.mock("openclaw/plugin-sdk/infra-runtime.js", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - enqueueSystemEvent: (...args: unknown[]) => enqueueSystemEventMock(...args), - }; -}); +} + +vi.mock("openclaw/plugin-sdk/infra-runtime", createInfraRuntimeMock); +vi.mock("openclaw/plugin-sdk/infra-runtime.js", createInfraRuntimeMock); vi.mock("openclaw/plugin-sdk/reply-runtime", async (importOriginal) => { const actual = await importOriginal(); @@ -297,6 +255,58 @@ describe("discord component interactions", () => { ...overrides, }); + const createGuildPluginButton = (allowFrom: string[]) => + createDiscordComponentButton( + createComponentContext({ + cfg: { + commands: { useAccessGroups: true }, + channels: { discord: { replyToMode: "first" } }, + } as OpenClawConfig, + allowFrom, + }), + ); + + const createGuildPluginButtonInteraction = (interactionId: string) => + createComponentButtonInteraction({ + rawData: { + channel_id: "guild-channel", + guild_id: "guild-1", + id: interactionId, + member: { roles: [] }, + } as unknown as ButtonInteraction["rawData"], + guild: { id: "guild-1", name: "Test Guild" } as unknown as ButtonInteraction["guild"], + }); + + async function expectPluginGuildInteractionAuth(params: { + allowFrom: string[]; + interactionId: string; + isAuthorizedSender: boolean; + }) { + registerDiscordComponentEntries({ + entries: [createButtonEntry({ callbackData: "codex:approve" })], + modals: [], + }); + dispatchPluginInteractiveHandlerMock.mockResolvedValue({ + matched: true, + handled: true, + duplicate: false, + }); + + const button = createGuildPluginButton(params.allowFrom); + const { interaction } = createGuildPluginButtonInteraction(params.interactionId); + + await button.run(interaction, { cid: "btn_1" } as ComponentData); + + expect(dispatchPluginInteractiveHandlerMock).toHaveBeenCalledWith( + expect.objectContaining({ + ctx: expect.objectContaining({ + auth: { isAuthorizedSender: params.isAuthorizedSender }, + }), + }), + ); + expect(dispatchReplyMock).not.toHaveBeenCalled(); + } + beforeEach(() => { editDiscordComponentMessageMock = vi .spyOn(sendComponents, "editDiscordComponentMessage") @@ -305,9 +315,8 @@ describe("discord component interactions", () => { channelId: "dm-channel", }); clearDiscordComponentEntries(); + resetDiscordComponentRuntimeMocks(); lastDispatchCtx = undefined; - readAllowFromStoreMock.mockClear().mockResolvedValue([]); - upsertPairingRequestMock.mockClear().mockResolvedValue({ code: "PAIRCODE", created: true }); enqueueSystemEventMock.mockClear(); dispatchReplyMock.mockClear().mockImplementation(async (params: DispatchParams) => { lastDispatchCtx = params.ctx; @@ -321,33 +330,6 @@ describe("discord component interactions", () => { handled: false, duplicate: false, }); - resolvePluginConversationBindingApprovalMock.mockReset().mockResolvedValue({ - status: "approved", - binding: { - bindingId: "binding-1", - pluginId: "openclaw-codex-app-server", - pluginName: "OpenClaw App Server", - pluginRoot: "/plugins/codex", - channel: "discord", - accountId: "default", - conversationId: "user:123456789", - boundAt: Date.now(), - }, - request: { - id: "approval-1", - pluginId: "openclaw-codex-app-server", - pluginName: "OpenClaw App Server", - pluginRoot: "/plugins/codex", - requestedAt: Date.now(), - conversation: { - channel: "discord", - accountId: "default", - conversationId: "user:123456789", - }, - }, - decision: "allow-once", - }); - buildPluginBindingResolvedTextMock.mockReset().mockReturnValue("Binding approved."); }); it("routes button clicks with reply references", async () => { @@ -552,87 +534,19 @@ describe("discord component interactions", () => { }); it("passes false auth to plugin Discord interactions for non-allowlisted guild users", async () => { - registerDiscordComponentEntries({ - entries: [createButtonEntry({ callbackData: "codex:approve" })], - modals: [], + await expectPluginGuildInteractionAuth({ + allowFrom: ["owner-1"], + interactionId: "interaction-guild-plugin-1", + isAuthorizedSender: false, }); - dispatchPluginInteractiveHandlerMock.mockResolvedValue({ - matched: true, - handled: true, - duplicate: false, - }); - - const button = createDiscordComponentButton( - createComponentContext({ - cfg: { - commands: { useAccessGroups: true }, - channels: { discord: { replyToMode: "first" } }, - } as OpenClawConfig, - allowFrom: ["owner-1"], - }), - ); - const { interaction } = createComponentButtonInteraction({ - rawData: { - channel_id: "guild-channel", - guild_id: "guild-1", - id: "interaction-guild-plugin-1", - member: { roles: [] }, - } as unknown as ButtonInteraction["rawData"], - guild: { id: "guild-1", name: "Test Guild" } as unknown as ButtonInteraction["guild"], - }); - - await button.run(interaction, { cid: "btn_1" } as ComponentData); - - expect(dispatchPluginInteractiveHandlerMock).toHaveBeenCalledWith( - expect.objectContaining({ - ctx: expect.objectContaining({ - auth: { isAuthorizedSender: false }, - }), - }), - ); - expect(dispatchReplyMock).not.toHaveBeenCalled(); }); it("passes true auth to plugin Discord interactions for allowlisted guild users", async () => { - registerDiscordComponentEntries({ - entries: [createButtonEntry({ callbackData: "codex:approve" })], - modals: [], + await expectPluginGuildInteractionAuth({ + allowFrom: ["123456789"], + interactionId: "interaction-guild-plugin-2", + isAuthorizedSender: true, }); - dispatchPluginInteractiveHandlerMock.mockResolvedValue({ - matched: true, - handled: true, - duplicate: false, - }); - - const button = createDiscordComponentButton( - createComponentContext({ - cfg: { - commands: { useAccessGroups: true }, - channels: { discord: { replyToMode: "first" } }, - } as OpenClawConfig, - allowFrom: ["123456789"], - }), - ); - const { interaction } = createComponentButtonInteraction({ - rawData: { - channel_id: "guild-channel", - guild_id: "guild-1", - id: "interaction-guild-plugin-2", - member: { roles: [] }, - } as unknown as ButtonInteraction["rawData"], - guild: { id: "guild-1", name: "Test Guild" } as unknown as ButtonInteraction["guild"], - }); - - await button.run(interaction, { cid: "btn_1" } as ComponentData); - - expect(dispatchPluginInteractiveHandlerMock).toHaveBeenCalledWith( - expect.objectContaining({ - ctx: expect.objectContaining({ - auth: { isAuthorizedSender: true }, - }), - }), - ); - expect(dispatchReplyMock).not.toHaveBeenCalled(); }); it("routes plugin Discord interactions in group DMs by channel id instead of sender id", async () => { diff --git a/extensions/discord/src/monitor/native-command.plugin-dispatch.test.ts b/extensions/discord/src/monitor/native-command.plugin-dispatch.test.ts index 7d1c5f471c8..c2f53db9d4f 100644 --- a/extensions/discord/src/monitor/native-command.plugin-dispatch.test.ts +++ b/extensions/discord/src/monitor/native-command.plugin-dispatch.test.ts @@ -81,6 +81,79 @@ function createConfig(): OpenClawConfig { } as OpenClawConfig; } +function createConfiguredAcpBinding(params: { + channelId: string; + peerKind: "channel" | "direct"; + agentId?: string; +}) { + return { + type: "acp", + agentId: params.agentId ?? "codex", + match: { + channel: "discord", + accountId: "default", + peer: { kind: params.peerKind, id: params.channelId }, + }, + acp: { + mode: "persistent", + }, + } as const; +} + +function createConfiguredAcpCase(params: { + channelType: ChannelType; + channelId: string; + peerKind: "channel" | "direct"; + guildId?: string; + guildName?: string; + includeChannelAccess?: boolean; + agentId?: string; +}) { + return { + cfg: { + commands: { + useAccessGroups: false, + }, + ...(params.includeChannelAccess === false + ? {} + : params.channelType === ChannelType.DM + ? { + channels: { + discord: { + dm: { enabled: true, policy: "open" }, + }, + }, + } + : { + channels: { + discord: { + guilds: { + [params.guildId!]: { + channels: { + [params.channelId]: { allow: true, requireMention: false }, + }, + }, + }, + }, + }, + }), + bindings: [ + createConfiguredAcpBinding({ + channelId: params.channelId, + peerKind: params.peerKind, + agentId: params.agentId, + }), + ], + } as OpenClawConfig, + interaction: createInteraction({ + channelType: params.channelType, + channelId: params.channelId, + guildId: params.guildId, + guildName: params.guildName, + }), + }; +} + async function loadCreateDiscordNativeCommand() { vi.resetModules(); return (await import("./native-command.js")).createDiscordNativeCommand; @@ -370,42 +443,11 @@ describe("Discord native plugin command dispatch", () => { }); it("routes native slash commands through configured ACP Discord channel bindings", async () => { - const guildId = "1459246755253325866"; - const channelId = "1478836151241412759"; - const cfg = { - commands: { - useAccessGroups: false, - }, - channels: { - discord: { - guilds: { - [guildId]: { - channels: { - [channelId]: { allow: true, requireMention: false }, - }, - }, - }, - }, - }, - bindings: [ - { - type: "acp", - agentId: "codex", - match: { - channel: "discord", - accountId: "default", - peer: { kind: "channel", id: channelId }, - }, - acp: { - mode: "persistent", - }, - }, - ], - } as OpenClawConfig; - const interaction = createInteraction({ + const { cfg, interaction } = createConfiguredAcpCase({ channelType: ChannelType.GuildText, - channelId, - guildId, + channelId: "1478836151241412759", + peerKind: "channel", + guildId: "1459246755253325866", guildName: "Ops", }); @@ -471,34 +513,10 @@ describe("Discord native plugin command dispatch", () => { }); it("routes Discord DM native slash commands through configured ACP bindings", async () => { - const channelId = "dm-1"; - const cfg = { - commands: { - useAccessGroups: false, - }, - bindings: [ - { - type: "acp", - agentId: "codex", - match: { - channel: "discord", - accountId: "default", - peer: { kind: "direct", id: channelId }, - }, - acp: { - mode: "persistent", - }, - }, - ], - channels: { - discord: { - dm: { enabled: true, policy: "open" }, - }, - }, - } as OpenClawConfig; - const interaction = createInteraction({ + const { cfg, interaction } = createConfiguredAcpCase({ channelType: ChannelType.DM, - channelId, + channelId: "dm-1", + peerKind: "direct", }); await expectBoundStatusCommandDispatch({ @@ -509,32 +527,13 @@ describe("Discord native plugin command dispatch", () => { }); it("allows recovery commands through configured ACP bindings even when ensure fails", async () => { - const guildId = "1459246755253325866"; - const channelId = "1479098716916023408"; - const cfg = { - commands: { - useAccessGroups: false, - }, - bindings: [ - { - type: "acp", - agentId: "codex", - match: { - channel: "discord", - accountId: "default", - peer: { kind: "channel", id: channelId }, - }, - acp: { - mode: "persistent", - }, - }, - ], - } as OpenClawConfig; - const interaction = createInteraction({ + const { cfg, interaction } = createConfiguredAcpCase({ channelType: ChannelType.GuildText, - channelId, - guildId, + channelId: "1479098716916023408", + peerKind: "channel", + guildId: "1459246755253325866", guildName: "Ops", + includeChannelAccess: false, }); ensureConfiguredBindingRouteReadyMock.mockResolvedValue({ ok: false, diff --git a/extensions/matrix/src/matrix/sdk/crypto-bootstrap.test.ts b/extensions/matrix/src/matrix/sdk/crypto-bootstrap.test.ts index 7e8a3b537c7..37678d9e343 100644 --- a/extensions/matrix/src/matrix/sdk/crypto-bootstrap.test.ts +++ b/extensions/matrix/src/matrix/sdk/crypto-bootstrap.test.ts @@ -29,6 +29,97 @@ function createCryptoApi(overrides?: Partial): MatrixC }; } +function createVerifiedDeviceStatus(overrides?: { + localVerified?: boolean; + crossSigningVerified?: boolean; + signedByOwner?: boolean; +}) { + return { + isVerified: () => true, + localVerified: overrides?.localVerified ?? true, + crossSigningVerified: overrides?.crossSigningVerified ?? true, + signedByOwner: overrides?.signedByOwner ?? true, + }; +} + +function createBootstrapperHarness( + cryptoOverrides?: Partial, + depsOverrides?: Partial>, +) { + const deps = { + ...createBootstrapperDeps(), + ...depsOverrides, + }; + const crypto = createCryptoApi(cryptoOverrides); + const bootstrapper = new MatrixCryptoBootstrapper( + deps as unknown as MatrixCryptoBootstrapperDeps, + ); + return { deps, crypto, bootstrapper }; +} + +async function runExplicitSecretStorageRepairScenario(firstError: string) { + const bootstrapCrossSigning = vi + .fn<() => Promise>() + .mockRejectedValueOnce(new Error(firstError)) + .mockResolvedValueOnce(undefined); + const { deps, crypto, bootstrapper } = createBootstrapperHarness({ + bootstrapCrossSigning, + isCrossSigningReady: vi.fn(async () => true), + userHasCrossSigningKeys: vi.fn(async () => true), + getDeviceVerificationStatus: vi.fn(async () => createVerifiedDeviceStatus()), + }); + + await bootstrapper.bootstrap(crypto, { + strict: true, + allowSecretStorageRecreateWithoutRecoveryKey: true, + allowAutomaticCrossSigningReset: false, + }); + + return { deps, crypto, bootstrapCrossSigning }; +} + +function expectSecretStorageRepairRetry( + deps: ReturnType, + crypto: MatrixCryptoBootstrapApi, + bootstrapCrossSigning: ReturnType, +) { + expect(deps.recoveryKeyStore.bootstrapSecretStorageWithRecoveryKey).toHaveBeenCalledWith(crypto, { + allowSecretStorageRecreateWithoutRecoveryKey: true, + forceNewSecretStorage: true, + }); + expect(bootstrapCrossSigning).toHaveBeenCalledTimes(2); +} + +async function bootstrapWithVerificationRequestListener(overrides?: { + deps?: Partial>; + crypto?: Partial; +}) { + const listeners = new Map void>(); + const { deps, bootstrapper, crypto } = createBootstrapperHarness( + { + getDeviceVerificationStatus: vi.fn(async () => ({ + isVerified: () => true, + })), + on: vi.fn((eventName: string, listener: (...args: unknown[]) => void) => { + listeners.set(eventName, listener); + }), + ...overrides?.crypto, + }, + overrides?.deps, + ); + + await bootstrapper.bootstrap(crypto); + const listener = Array.from(listeners.entries()).find(([eventName]) => + eventName.toLowerCase().includes("verificationrequest"), + )?.[1]; + expect(listener).toBeTypeOf("function"); + + return { + deps, + listener, + }; +} + describe("MatrixCryptoBootstrapper", () => { beforeEach(() => { vi.restoreAllMocks(); @@ -159,40 +250,11 @@ describe("MatrixCryptoBootstrapper", () => { }); it("recreates secret storage and retries cross-signing when explicit bootstrap hits a stale server key", async () => { - const deps = createBootstrapperDeps(); - const bootstrapCrossSigning = vi - .fn<() => Promise>() - .mockRejectedValueOnce(new Error("getSecretStorageKey callback returned falsey")) - .mockResolvedValueOnce(undefined); - const crypto = createCryptoApi({ - bootstrapCrossSigning, - isCrossSigningReady: vi.fn(async () => true), - userHasCrossSigningKeys: vi.fn(async () => true), - getDeviceVerificationStatus: vi.fn(async () => ({ - isVerified: () => true, - localVerified: true, - crossSigningVerified: true, - signedByOwner: true, - })), - }); - const bootstrapper = new MatrixCryptoBootstrapper( - deps as unknown as MatrixCryptoBootstrapperDeps, + const { deps, crypto, bootstrapCrossSigning } = await runExplicitSecretStorageRepairScenario( + "getSecretStorageKey callback returned falsey", ); - await bootstrapper.bootstrap(crypto, { - strict: true, - allowSecretStorageRecreateWithoutRecoveryKey: true, - allowAutomaticCrossSigningReset: false, - }); - - expect(deps.recoveryKeyStore.bootstrapSecretStorageWithRecoveryKey).toHaveBeenCalledWith( - crypto, - { - allowSecretStorageRecreateWithoutRecoveryKey: true, - forceNewSecretStorage: true, - }, - ); - expect(bootstrapCrossSigning).toHaveBeenCalledTimes(2); + expectSecretStorageRepairRetry(deps, crypto, bootstrapCrossSigning); expect(bootstrapCrossSigning).toHaveBeenNthCalledWith( 1, expect.objectContaining({ @@ -208,40 +270,11 @@ describe("MatrixCryptoBootstrapper", () => { }); it("recreates secret storage and retries cross-signing when explicit bootstrap hits bad MAC", async () => { - const deps = createBootstrapperDeps(); - const bootstrapCrossSigning = vi - .fn<() => Promise>() - .mockRejectedValueOnce(new Error("Error decrypting secret m.cross_signing.master: bad MAC")) - .mockResolvedValueOnce(undefined); - const crypto = createCryptoApi({ - bootstrapCrossSigning, - isCrossSigningReady: vi.fn(async () => true), - userHasCrossSigningKeys: vi.fn(async () => true), - getDeviceVerificationStatus: vi.fn(async () => ({ - isVerified: () => true, - localVerified: true, - crossSigningVerified: true, - signedByOwner: true, - })), - }); - const bootstrapper = new MatrixCryptoBootstrapper( - deps as unknown as MatrixCryptoBootstrapperDeps, + const { deps, crypto, bootstrapCrossSigning } = await runExplicitSecretStorageRepairScenario( + "Error decrypting secret m.cross_signing.master: bad MAC", ); - await bootstrapper.bootstrap(crypto, { - strict: true, - allowSecretStorageRecreateWithoutRecoveryKey: true, - allowAutomaticCrossSigningReset: false, - }); - - expect(deps.recoveryKeyStore.bootstrapSecretStorageWithRecoveryKey).toHaveBeenCalledWith( - crypto, - { - allowSecretStorageRecreateWithoutRecoveryKey: true, - forceNewSecretStorage: true, - }, - ); - expect(bootstrapCrossSigning).toHaveBeenCalledTimes(2); + expectSecretStorageRepairRetry(deps, crypto, bootstrapCrossSigning); }); it("fails in strict mode when cross-signing keys are still unpublished", async () => { @@ -264,9 +297,8 @@ describe("MatrixCryptoBootstrapper", () => { }); it("uses password UIA fallback when null and dummy auth fail", async () => { - const deps = createBootstrapperDeps(); const bootstrapCrossSigning = vi.fn(async () => {}); - const crypto = createCryptoApi({ + const { bootstrapper, crypto } = createBootstrapperHarness({ bootstrapCrossSigning, isCrossSigningReady: vi.fn(async () => true), userHasCrossSigningKeys: vi.fn(async () => true), @@ -274,9 +306,6 @@ describe("MatrixCryptoBootstrapper", () => { isVerified: () => true, })), }); - const bootstrapper = new MatrixCryptoBootstrapper( - deps as unknown as MatrixCryptoBootstrapperDeps, - ); await bootstrapper.bootstrap(crypto); @@ -321,12 +350,11 @@ describe("MatrixCryptoBootstrapper", () => { }); it("resets cross-signing when first bootstrap attempt throws", async () => { - const deps = createBootstrapperDeps(); const bootstrapCrossSigning = vi .fn<() => Promise>() .mockRejectedValueOnce(new Error("first attempt failed")) .mockResolvedValueOnce(undefined); - const crypto = createCryptoApi({ + const { bootstrapper, crypto } = createBootstrapperHarness({ bootstrapCrossSigning, isCrossSigningReady: vi.fn(async () => true), userHasCrossSigningKeys: vi.fn(async () => true), @@ -334,9 +362,6 @@ describe("MatrixCryptoBootstrapper", () => { isVerified: () => true, })), }); - const bootstrapper = new MatrixCryptoBootstrapper( - deps as unknown as MatrixCryptoBootstrapperDeps, - ); await bootstrapper.bootstrap(crypto); @@ -418,32 +443,13 @@ describe("MatrixCryptoBootstrapper", () => { }); it("tracks incoming verification requests from other users", async () => { - const deps = createBootstrapperDeps(); - const listeners = new Map void>(); - const crypto = createCryptoApi({ - getDeviceVerificationStatus: vi.fn(async () => ({ - isVerified: () => true, - })), - on: vi.fn((eventName: string, listener: (...args: unknown[]) => void) => { - listeners.set(eventName, listener); - }), - }); - const bootstrapper = new MatrixCryptoBootstrapper( - deps as unknown as MatrixCryptoBootstrapperDeps, - ); - - await bootstrapper.bootstrap(crypto); - + const { deps, listener } = await bootstrapWithVerificationRequestListener(); const verificationRequest = { otherUserId: "@alice:example.org", isSelfVerification: false, initiatedByMe: false, accept: vi.fn(async () => {}), }; - const listener = Array.from(listeners.entries()).find(([eventName]) => - eventName.toLowerCase().includes("verificationrequest"), - )?.[1]; - expect(listener).toBeTypeOf("function"); await listener?.(verificationRequest); expect(deps.verificationManager.trackVerificationRequest).toHaveBeenCalledWith( @@ -453,24 +459,20 @@ describe("MatrixCryptoBootstrapper", () => { }); it("does not touch request state when tracking summary throws", async () => { - const deps = createBootstrapperDeps(); - deps.verificationManager.trackVerificationRequest = vi.fn(() => { - throw new Error("summary failure"); + const { listener } = await bootstrapWithVerificationRequestListener({ + deps: { + verificationManager: { + trackVerificationRequest: vi.fn(() => { + throw new Error("summary failure"); + }), + }, + }, + crypto: { + getDeviceVerificationStatus: vi.fn(async () => ({ + isVerified: () => true, + })), + }, }); - const listeners = new Map void>(); - const crypto = createCryptoApi({ - getDeviceVerificationStatus: vi.fn(async () => ({ - isVerified: () => true, - })), - on: vi.fn((eventName: string, listener: (...args: unknown[]) => void) => { - listeners.set(eventName, listener); - }), - }); - const bootstrapper = new MatrixCryptoBootstrapper( - deps as unknown as MatrixCryptoBootstrapperDeps, - ); - - await bootstrapper.bootstrap(crypto); const verificationRequest = { otherUserId: "@alice:example.org", @@ -478,10 +480,6 @@ describe("MatrixCryptoBootstrapper", () => { initiatedByMe: false, accept: vi.fn(async () => {}), }; - const listener = Array.from(listeners.entries()).find(([eventName]) => - eventName.toLowerCase().includes("verificationrequest"), - )?.[1]; - expect(listener).toBeTypeOf("function"); await listener?.(verificationRequest); expect(verificationRequest.accept).not.toHaveBeenCalled(); diff --git a/extensions/matrix/src/matrix/sdk/crypto-facade.test.ts b/extensions/matrix/src/matrix/sdk/crypto-facade.test.ts index 6d7bca7c38f..b081f27f4a4 100644 --- a/extensions/matrix/src/matrix/sdk/crypto-facade.test.ts +++ b/extensions/matrix/src/matrix/sdk/crypto-facade.test.ts @@ -3,35 +3,69 @@ import { createMatrixCryptoFacade } from "./crypto-facade.js"; import type { MatrixRecoveryKeyStore } from "./recovery-key-store.js"; import type { MatrixVerificationManager } from "./verification-manager.js"; +type MatrixCryptoFacadeDeps = Parameters[0]; + +function createVerificationManagerMock( + overrides: Partial = {}, +): MatrixVerificationManager { + return { + requestOwnUserVerification: vi.fn(async () => null), + listVerifications: vi.fn(async () => []), + ensureVerificationDmTracked: vi.fn(async () => null), + requestVerification: vi.fn(), + acceptVerification: vi.fn(), + cancelVerification: vi.fn(), + startVerification: vi.fn(), + generateVerificationQr: vi.fn(), + scanVerificationQr: vi.fn(), + confirmVerificationSas: vi.fn(), + mismatchVerificationSas: vi.fn(), + confirmVerificationReciprocateQr: vi.fn(), + getVerificationSas: vi.fn(), + ...overrides, + } as unknown as MatrixVerificationManager; +} + +function createRecoveryKeyStoreMock( + summary: ReturnType = null, +): MatrixRecoveryKeyStore { + return { + getRecoveryKeySummary: vi.fn(() => summary), + } as unknown as MatrixRecoveryKeyStore; +} + +function createFacadeHarness(params?: { + client?: Partial; + verificationManager?: Partial; + recoveryKeySummary?: ReturnType; + getRoomStateEvent?: MatrixCryptoFacadeDeps["getRoomStateEvent"]; + downloadContent?: MatrixCryptoFacadeDeps["downloadContent"]; +}) { + const getRoomStateEvent: MatrixCryptoFacadeDeps["getRoomStateEvent"] = + params?.getRoomStateEvent ?? (async () => ({})); + const downloadContent: MatrixCryptoFacadeDeps["downloadContent"] = + params?.downloadContent ?? (async () => Buffer.alloc(0)); + const facade = createMatrixCryptoFacade({ + client: { + getRoom: params?.client?.getRoom ?? (() => null), + getCrypto: params?.client?.getCrypto ?? (() => undefined), + }, + verificationManager: createVerificationManagerMock(params?.verificationManager), + recoveryKeyStore: createRecoveryKeyStoreMock(params?.recoveryKeySummary ?? null), + getRoomStateEvent, + downloadContent, + }); + return { facade, getRoomStateEvent, downloadContent }; +} + describe("createMatrixCryptoFacade", () => { it("detects encrypted rooms from cached room state", async () => { - const facade = createMatrixCryptoFacade({ + const { facade } = createFacadeHarness({ client: { getRoom: () => ({ hasEncryptionStateEvent: () => true, }), - getCrypto: () => undefined, }, - verificationManager: { - requestOwnUserVerification: vi.fn(), - listVerifications: vi.fn(async () => []), - ensureVerificationDmTracked: vi.fn(async () => null), - requestVerification: vi.fn(), - acceptVerification: vi.fn(), - cancelVerification: vi.fn(), - startVerification: vi.fn(), - generateVerificationQr: vi.fn(), - scanVerificationQr: vi.fn(), - confirmVerificationSas: vi.fn(), - mismatchVerificationSas: vi.fn(), - confirmVerificationReciprocateQr: vi.fn(), - getVerificationSas: vi.fn(), - } as unknown as MatrixVerificationManager, - recoveryKeyStore: { - getRecoveryKeySummary: vi.fn(() => null), - } as unknown as MatrixRecoveryKeyStore, - getRoomStateEvent: vi.fn(async () => ({ algorithm: "m.megolm.v1.aes-sha2" })), - downloadContent: vi.fn(async () => Buffer.alloc(0)), }); await expect(facade.isRoomEncrypted("!room:example.org")).resolves.toBe(true); @@ -41,33 +75,13 @@ describe("createMatrixCryptoFacade", () => { const getRoomStateEvent = vi.fn(async () => ({ algorithm: "m.megolm.v1.aes-sha2", })); - const facade = createMatrixCryptoFacade({ + const { facade } = createFacadeHarness({ client: { getRoom: () => ({ hasEncryptionStateEvent: () => false, }), - getCrypto: () => undefined, }, - verificationManager: { - requestOwnUserVerification: vi.fn(), - listVerifications: vi.fn(async () => []), - ensureVerificationDmTracked: vi.fn(async () => null), - requestVerification: vi.fn(), - acceptVerification: vi.fn(), - cancelVerification: vi.fn(), - startVerification: vi.fn(), - generateVerificationQr: vi.fn(), - scanVerificationQr: vi.fn(), - confirmVerificationSas: vi.fn(), - mismatchVerificationSas: vi.fn(), - confirmVerificationReciprocateQr: vi.fn(), - getVerificationSas: vi.fn(), - } as unknown as MatrixVerificationManager, - recoveryKeyStore: { - getRecoveryKeySummary: vi.fn(() => null), - } as unknown as MatrixRecoveryKeyStore, getRoomStateEvent, - downloadContent: vi.fn(async () => Buffer.alloc(0)), }); await expect(facade.isRoomEncrypted("!room:example.org")).resolves.toBe(true); @@ -92,31 +106,15 @@ describe("createMatrixCryptoFacade", () => { createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(), })); - const facade = createMatrixCryptoFacade({ + const { facade } = createFacadeHarness({ client: { getRoom: () => null, getCrypto: () => crypto, }, verificationManager: { - requestOwnUserVerification: vi.fn(async () => null), - listVerifications: vi.fn(async () => []), - ensureVerificationDmTracked: vi.fn(async () => null), requestVerification, - acceptVerification: vi.fn(), - cancelVerification: vi.fn(), - startVerification: vi.fn(), - generateVerificationQr: vi.fn(), - scanVerificationQr: vi.fn(), - confirmVerificationSas: vi.fn(), - mismatchVerificationSas: vi.fn(), - confirmVerificationReciprocateQr: vi.fn(), - getVerificationSas: vi.fn(), - } as unknown as MatrixVerificationManager, - recoveryKeyStore: { - getRecoveryKeySummary: vi.fn(() => ({ keyId: "KEY" })), - } as unknown as MatrixRecoveryKeyStore, - getRoomStateEvent: vi.fn(async () => ({})), - downloadContent: vi.fn(async () => Buffer.alloc(0)), + }, + recoveryKeySummary: { keyId: "KEY" }, }); const result = await facade.requestVerification({ @@ -174,32 +172,14 @@ describe("createMatrixCryptoFacade", () => { requestOwnUserVerification: vi.fn(async () => null), findVerificationRequestDMInProgress: vi.fn(() => request), }; - const facade = createMatrixCryptoFacade({ + const { facade } = createFacadeHarness({ client: { getRoom: () => null, getCrypto: () => crypto, }, verificationManager: { trackVerificationRequest, - requestOwnUserVerification: vi.fn(async () => null), - listVerifications: vi.fn(async () => []), - ensureVerificationDmTracked: vi.fn(async () => null), - requestVerification: vi.fn(), - acceptVerification: vi.fn(), - cancelVerification: vi.fn(), - startVerification: vi.fn(), - generateVerificationQr: vi.fn(), - scanVerificationQr: vi.fn(), - confirmVerificationSas: vi.fn(), - mismatchVerificationSas: vi.fn(), - confirmVerificationReciprocateQr: vi.fn(), - getVerificationSas: vi.fn(), - } as unknown as MatrixVerificationManager, - recoveryKeyStore: { - getRecoveryKeySummary: vi.fn(() => null), - } as unknown as MatrixRecoveryKeyStore, - getRoomStateEvent: vi.fn(async () => ({})), - downloadContent: vi.fn(async () => Buffer.alloc(0)), + }, }); const summary = await facade.ensureVerificationDmTracked({ diff --git a/extensions/matrix/src/matrix/sdk/recovery-key-store.test.ts b/extensions/matrix/src/matrix/sdk/recovery-key-store.test.ts index 79d41b0e36b..3a3c850a9cf 100644 --- a/extensions/matrix/src/matrix/sdk/recovery-key-store.test.ts +++ b/extensions/matrix/src/matrix/sdk/recovery-key-store.test.ts @@ -4,13 +4,85 @@ import path from "node:path"; import { encodeRecoveryKey } from "matrix-js-sdk/lib/crypto-api/recovery-key.js"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { MatrixRecoveryKeyStore } from "./recovery-key-store.js"; -import type { MatrixCryptoBootstrapApi } from "./types.js"; +import type { MatrixCryptoBootstrapApi, MatrixSecretStorageStatus } from "./types.js"; function createTempRecoveryKeyPath(): string { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "matrix-recovery-key-store-")); return path.join(dir, "recovery-key.json"); } +function createGeneratedRecoveryKey(params: { + keyId: string; + name: string; + bytes: number[]; + encodedPrivateKey: string; +}) { + return { + keyId: params.keyId, + keyInfo: { name: params.name }, + privateKey: new Uint8Array(params.bytes), + encodedPrivateKey: params.encodedPrivateKey, + }; +} + +function createBootstrapSecretStorageMock(errorMessage?: string) { + return vi.fn( + async (opts?: { + setupNewSecretStorage?: boolean; + createSecretStorageKey?: () => Promise; + }) => { + if (opts?.setupNewSecretStorage || !errorMessage) { + await opts?.createSecretStorageKey?.(); + return; + } + throw new Error(errorMessage); + }, + ); +} + +function createRecoveryKeyCrypto(params: { + bootstrapSecretStorage: ReturnType; + createRecoveryKeyFromPassphrase: ReturnType; + status: MatrixSecretStorageStatus; +}): MatrixCryptoBootstrapApi { + return { + on: vi.fn(), + bootstrapCrossSigning: vi.fn(async () => {}), + bootstrapSecretStorage: params.bootstrapSecretStorage, + createRecoveryKeyFromPassphrase: params.createRecoveryKeyFromPassphrase, + getSecretStorageStatus: vi.fn(async () => params.status), + requestOwnUserVerification: vi.fn(async () => null), + } as unknown as MatrixCryptoBootstrapApi; +} + +async function runSecretStorageBootstrapScenario(params: { + generated: ReturnType; + status: MatrixSecretStorageStatus; + allowSecretStorageRecreateWithoutRecoveryKey?: boolean; + firstBootstrapError?: string; +}) { + const recoveryKeyPath = createTempRecoveryKeyPath(); + const store = new MatrixRecoveryKeyStore(recoveryKeyPath); + const createRecoveryKeyFromPassphrase = vi.fn(async () => params.generated); + const bootstrapSecretStorage = createBootstrapSecretStorageMock(params.firstBootstrapError); + const crypto = createRecoveryKeyCrypto({ + bootstrapSecretStorage, + createRecoveryKeyFromPassphrase, + status: params.status, + }); + + await store.bootstrapSecretStorageWithRecoveryKey(crypto, { + allowSecretStorageRecreateWithoutRecoveryKey: + params.allowSecretStorageRecreateWithoutRecoveryKey ?? false, + }); + + return { + store, + createRecoveryKeyFromPassphrase, + bootstrapSecretStorage, + }; +} + describe("MatrixRecoveryKeyStore", () => { beforeEach(() => { vi.restoreAllMocks(); @@ -65,30 +137,16 @@ describe("MatrixRecoveryKeyStore", () => { }); it("creates and persists a recovery key when secret storage is missing", async () => { - const recoveryKeyPath = createTempRecoveryKeyPath(); - const store = new MatrixRecoveryKeyStore(recoveryKeyPath); - const generated = { - keyId: "GENERATED", - keyInfo: { name: "generated" }, - privateKey: new Uint8Array([5, 6, 7, 8]), - encodedPrivateKey: "encoded-generated-key", // pragma: allowlist secret - }; - const createRecoveryKeyFromPassphrase = vi.fn(async () => generated); - const bootstrapSecretStorage = vi.fn( - async (opts?: { createSecretStorageKey?: () => Promise }) => { - await opts?.createSecretStorageKey?.(); - }, - ); - const crypto = { - on: vi.fn(), - bootstrapCrossSigning: vi.fn(async () => {}), - bootstrapSecretStorage, - createRecoveryKeyFromPassphrase, - getSecretStorageStatus: vi.fn(async () => ({ ready: false, defaultKeyId: null })), - requestOwnUserVerification: vi.fn(async () => null), - } as unknown as MatrixCryptoBootstrapApi; - - await store.bootstrapSecretStorageWithRecoveryKey(crypto); + const { store, createRecoveryKeyFromPassphrase, bootstrapSecretStorage } = + await runSecretStorageBootstrapScenario({ + generated: createGeneratedRecoveryKey({ + keyId: "GENERATED", + name: "generated", + bytes: [5, 6, 7, 8], + encodedPrivateKey: "encoded-generated-key", // pragma: allowlist secret + }), + status: { ready: false, defaultKeyId: null }, + }); expect(createRecoveryKeyFromPassphrase).toHaveBeenCalledTimes(1); expect(bootstrapSecretStorage).toHaveBeenCalledWith( @@ -138,30 +196,16 @@ describe("MatrixRecoveryKeyStore", () => { }); it("recreates secret storage when default key exists but is not usable locally", async () => { - const recoveryKeyPath = createTempRecoveryKeyPath(); - const store = new MatrixRecoveryKeyStore(recoveryKeyPath); - const generated = { - keyId: "RECOVERED", - keyInfo: { name: "recovered" }, - privateKey: new Uint8Array([1, 1, 2, 3]), - encodedPrivateKey: "encoded-recovered-key", // pragma: allowlist secret - }; - const createRecoveryKeyFromPassphrase = vi.fn(async () => generated); - const bootstrapSecretStorage = vi.fn( - async (opts?: { createSecretStorageKey?: () => Promise }) => { - await opts?.createSecretStorageKey?.(); - }, - ); - const crypto = { - on: vi.fn(), - bootstrapCrossSigning: vi.fn(async () => {}), - bootstrapSecretStorage, - createRecoveryKeyFromPassphrase, - getSecretStorageStatus: vi.fn(async () => ({ ready: false, defaultKeyId: "LEGACY" })), - requestOwnUserVerification: vi.fn(async () => null), - } as unknown as MatrixCryptoBootstrapApi; - - await store.bootstrapSecretStorageWithRecoveryKey(crypto); + const { store, createRecoveryKeyFromPassphrase, bootstrapSecretStorage } = + await runSecretStorageBootstrapScenario({ + generated: createGeneratedRecoveryKey({ + keyId: "RECOVERED", + name: "recovered", + bytes: [1, 1, 2, 3], + encodedPrivateKey: "encoded-recovered-key", // pragma: allowlist secret + }), + status: { ready: false, defaultKeyId: "LEGACY" }, + }); expect(createRecoveryKeyFromPassphrase).toHaveBeenCalledTimes(1); expect(bootstrapSecretStorage).toHaveBeenCalledWith( @@ -176,43 +220,22 @@ describe("MatrixRecoveryKeyStore", () => { }); it("recreates secret storage during explicit bootstrap when the server key exists but no local recovery key is available", async () => { - const recoveryKeyPath = createTempRecoveryKeyPath(); - const store = new MatrixRecoveryKeyStore(recoveryKeyPath); - const generated = { - keyId: "REPAIRED", - keyInfo: { name: "repaired" }, - privateKey: new Uint8Array([7, 7, 8, 9]), - encodedPrivateKey: "encoded-repaired-key", // pragma: allowlist secret - }; - const createRecoveryKeyFromPassphrase = vi.fn(async () => generated); - const bootstrapSecretStorage = vi.fn( - async (opts?: { - setupNewSecretStorage?: boolean; - createSecretStorageKey?: () => Promise; - }) => { - if (opts?.setupNewSecretStorage) { - await opts.createSecretStorageKey?.(); - return; - } - throw new Error("getSecretStorageKey callback returned falsey"); - }, - ); - const crypto = { - on: vi.fn(), - bootstrapCrossSigning: vi.fn(async () => {}), - bootstrapSecretStorage, - createRecoveryKeyFromPassphrase, - getSecretStorageStatus: vi.fn(async () => ({ - ready: true, - defaultKeyId: "LEGACY", - secretStorageKeyValidityMap: { LEGACY: true }, - })), - requestOwnUserVerification: vi.fn(async () => null), - } as unknown as MatrixCryptoBootstrapApi; - - await store.bootstrapSecretStorageWithRecoveryKey(crypto, { - allowSecretStorageRecreateWithoutRecoveryKey: true, - }); + const { store, createRecoveryKeyFromPassphrase, bootstrapSecretStorage } = + await runSecretStorageBootstrapScenario({ + generated: createGeneratedRecoveryKey({ + keyId: "REPAIRED", + name: "repaired", + bytes: [7, 7, 8, 9], + encodedPrivateKey: "encoded-repaired-key", // pragma: allowlist secret + }), + status: { + ready: true, + defaultKeyId: "LEGACY", + secretStorageKeyValidityMap: { LEGACY: true }, + }, + allowSecretStorageRecreateWithoutRecoveryKey: true, + firstBootstrapError: "getSecretStorageKey callback returned falsey", + }); expect(createRecoveryKeyFromPassphrase).toHaveBeenCalledTimes(1); expect(bootstrapSecretStorage).toHaveBeenCalledTimes(2); @@ -228,43 +251,22 @@ describe("MatrixRecoveryKeyStore", () => { }); it("recreates secret storage during explicit bootstrap when decrypting a stored secret fails with bad MAC", async () => { - const recoveryKeyPath = createTempRecoveryKeyPath(); - const store = new MatrixRecoveryKeyStore(recoveryKeyPath); - const generated = { - keyId: "REPAIRED", - keyInfo: { name: "repaired" }, - privateKey: new Uint8Array([7, 7, 8, 9]), - encodedPrivateKey: "encoded-repaired-key", // pragma: allowlist secret - }; - const createRecoveryKeyFromPassphrase = vi.fn(async () => generated); - const bootstrapSecretStorage = vi.fn( - async (opts?: { - setupNewSecretStorage?: boolean; - createSecretStorageKey?: () => Promise; - }) => { - if (opts?.setupNewSecretStorage) { - await opts.createSecretStorageKey?.(); - return; - } - throw new Error("Error decrypting secret m.cross_signing.master: bad MAC"); - }, - ); - const crypto = { - on: vi.fn(), - bootstrapCrossSigning: vi.fn(async () => {}), - bootstrapSecretStorage, - createRecoveryKeyFromPassphrase, - getSecretStorageStatus: vi.fn(async () => ({ - ready: true, - defaultKeyId: "LEGACY", - secretStorageKeyValidityMap: { LEGACY: true }, - })), - requestOwnUserVerification: vi.fn(async () => null), - } as unknown as MatrixCryptoBootstrapApi; - - await store.bootstrapSecretStorageWithRecoveryKey(crypto, { - allowSecretStorageRecreateWithoutRecoveryKey: true, - }); + const { createRecoveryKeyFromPassphrase, bootstrapSecretStorage } = + await runSecretStorageBootstrapScenario({ + generated: createGeneratedRecoveryKey({ + keyId: "REPAIRED", + name: "repaired", + bytes: [7, 7, 8, 9], + encodedPrivateKey: "encoded-repaired-key", // pragma: allowlist secret + }), + status: { + ready: true, + defaultKeyId: "LEGACY", + secretStorageKeyValidityMap: { LEGACY: true }, + }, + allowSecretStorageRecreateWithoutRecoveryKey: true, + firstBootstrapError: "Error decrypting secret m.cross_signing.master: bad MAC", + }); expect(createRecoveryKeyFromPassphrase).toHaveBeenCalledTimes(1); expect(bootstrapSecretStorage).toHaveBeenCalledTimes(2); diff --git a/extensions/matrix/src/matrix/sdk/verification-manager.test.ts b/extensions/matrix/src/matrix/sdk/verification-manager.test.ts index c9dfa068d69..d9313cf1286 100644 --- a/extensions/matrix/src/matrix/sdk/verification-manager.test.ts +++ b/extensions/matrix/src/matrix/sdk/verification-manager.test.ts @@ -86,6 +86,65 @@ class MockVerificationRequest extends EventEmitter implements MatrixVerification generateQRCode = vi.fn(async () => new Uint8ClampedArray([1, 2, 3])); } +function createSasVerifierFixture(params: { + decimal: [number, number, number]; + emoji: [string, string][]; + verifyImpl?: () => Promise; +}) { + const confirm = vi.fn(async () => {}); + const mismatch = vi.fn(); + const cancel = vi.fn(); + const verify = vi.fn(params.verifyImpl ?? (async () => {})); + return { + confirm, + mismatch, + verify, + verifier: new MockVerifier( + { + sas: { + decimal: params.decimal, + emoji: params.emoji, + }, + confirm, + mismatch, + cancel, + }, + null, + verify, + ), + }; +} + +function createReadyRequestWithoutVerifier(params: { + transactionId: string; + isSelfVerification: boolean; + verifier: MatrixVerifierLike; +}) { + const request = new MockVerificationRequest({ + transactionId: params.transactionId, + initiatedByMe: false, + isSelfVerification: params.isSelfVerification, + verifier: undefined, + }); + request.startVerification = vi.fn(async (_method: string) => { + request.phase = VerificationPhase.Started; + request.verifier = params.verifier; + return params.verifier; + }); + return request; +} + +function expectTrackedSas( + manager: MatrixVerificationManager, + trackedId: string, + decimal: [number, number, number], +) { + const summary = manager.listVerifications().find((item) => item.id === trackedId); + expect(summary?.hasSas).toBe(true); + expect(summary?.sas?.decimal).toEqual(decimal); + expect(manager.getVerificationSas(trackedId).decimal).toEqual(decimal); +} + describe("MatrixVerificationManager", () => { it("handles rust verification requests whose methods getter throws", () => { const manager = new MatrixVerificationManager(); @@ -173,24 +232,14 @@ describe("MatrixVerificationManager", () => { }); it("auto-starts an incoming verifier exposed via request change events", async () => { - const verify = vi.fn(async () => {}); - const verifier = new MockVerifier( - { - sas: { - decimal: [6158, 1986, 3513], - emoji: [ - ["gift", "Gift"], - ["globe", "Globe"], - ["horse", "Horse"], - ], - }, - confirm: vi.fn(async () => {}), - mismatch: vi.fn(), - cancel: vi.fn(), - }, - null, - verify, - ); + const { verifier, verify } = createSasVerifierFixture({ + decimal: [6158, 1986, 3513], + emoji: [ + ["gift", "Gift"], + ["globe", "Globe"], + ["horse", "Horse"], + ], + }); const request = new MockVerificationRequest({ transactionId: "txn-incoming-change", verifier: undefined, @@ -204,31 +253,18 @@ describe("MatrixVerificationManager", () => { await vi.waitFor(() => { expect(verify).toHaveBeenCalledTimes(1); }); - const summary = manager.listVerifications().find((item) => item.id === tracked.id); - expect(summary?.hasSas).toBe(true); - expect(summary?.sas?.decimal).toEqual([6158, 1986, 3513]); - expect(manager.getVerificationSas(tracked.id).decimal).toEqual([6158, 1986, 3513]); + expectTrackedSas(manager, tracked.id, [6158, 1986, 3513]); }); it("emits summary updates when SAS becomes available", async () => { - const verify = vi.fn(async () => {}); - const verifier = new MockVerifier( - { - sas: { - decimal: [6158, 1986, 3513], - emoji: [ - ["gift", "Gift"], - ["globe", "Globe"], - ["horse", "Horse"], - ], - }, - confirm: vi.fn(async () => {}), - mismatch: vi.fn(), - cancel: vi.fn(), - }, - null, - verify, - ); + const { verifier } = createSasVerifierFixture({ + decimal: [6158, 1986, 3513], + emoji: [ + ["gift", "Gift"], + ["globe", "Globe"], + ["horse", "Horse"], + ], + }); const request = new MockVerificationRequest({ transactionId: "txn-summary-listener", roomId: "!dm:example.org", @@ -257,34 +293,18 @@ describe("MatrixVerificationManager", () => { }); it("does not auto-start non-self inbound SAS when request becomes ready without a verifier", async () => { - const verify = vi.fn(async () => {}); - const verifier = new MockVerifier( - { - sas: { - decimal: [1234, 5678, 9012], - emoji: [ - ["gift", "Gift"], - ["rocket", "Rocket"], - ["butterfly", "Butterfly"], - ], - }, - confirm: vi.fn(async () => {}), - mismatch: vi.fn(), - cancel: vi.fn(), - }, - null, - verify, - ); - const request = new MockVerificationRequest({ - transactionId: "txn-no-auto-start-dm-sas", - initiatedByMe: false, - isSelfVerification: false, - verifier: undefined, + const { verifier, verify } = createSasVerifierFixture({ + decimal: [1234, 5678, 9012], + emoji: [ + ["gift", "Gift"], + ["rocket", "Rocket"], + ["butterfly", "Butterfly"], + ], }); - request.startVerification = vi.fn(async (_method: string) => { - request.phase = VerificationPhase.Started; - request.verifier = verifier; - return verifier; + const request = createReadyRequestWithoutVerifier({ + transactionId: "txn-no-auto-start-dm-sas", + isSelfVerification: false, + verifier, }); const manager = new MatrixVerificationManager(); const tracked = manager.trackVerificationRequest(request); @@ -303,34 +323,18 @@ describe("MatrixVerificationManager", () => { }); it("auto-starts self verification SAS when request becomes ready without a verifier", async () => { - const verify = vi.fn(async () => {}); - const verifier = new MockVerifier( - { - sas: { - decimal: [1234, 5678, 9012], - emoji: [ - ["gift", "Gift"], - ["rocket", "Rocket"], - ["butterfly", "Butterfly"], - ], - }, - confirm: vi.fn(async () => {}), - mismatch: vi.fn(), - cancel: vi.fn(), - }, - null, - verify, - ); - const request = new MockVerificationRequest({ - transactionId: "txn-auto-start-self-sas", - initiatedByMe: false, - isSelfVerification: true, - verifier: undefined, + const { verifier, verify } = createSasVerifierFixture({ + decimal: [1234, 5678, 9012], + emoji: [ + ["gift", "Gift"], + ["rocket", "Rocket"], + ["butterfly", "Butterfly"], + ], }); - request.startVerification = vi.fn(async (_method: string) => { - request.phase = VerificationPhase.Started; - request.verifier = verifier; - return verifier; + const request = createReadyRequestWithoutVerifier({ + transactionId: "txn-auto-start-self-sas", + isSelfVerification: true, + verifier, }); const manager = new MatrixVerificationManager(); const tracked = manager.trackVerificationRequest(request); @@ -344,10 +348,7 @@ describe("MatrixVerificationManager", () => { await vi.waitFor(() => { expect(verify).toHaveBeenCalledTimes(1); }); - const summary = manager.listVerifications().find((item) => item.id === tracked.id); - expect(summary?.hasSas).toBe(true); - expect(summary?.sas?.decimal).toEqual([1234, 5678, 9012]); - expect(manager.getVerificationSas(tracked.id).decimal).toEqual([1234, 5678, 9012]); + expectTrackedSas(manager, tracked.id, [1234, 5678, 9012]); }); it("auto-accepts incoming verification requests only once per transaction", async () => { diff --git a/extensions/zalo/src/channel.directory.test.ts b/extensions/zalo/src/channel.directory.test.ts index 813a573374b..e6254dd6be7 100644 --- a/extensions/zalo/src/channel.directory.test.ts +++ b/extensions/zalo/src/channel.directory.test.ts @@ -8,18 +8,17 @@ import { zaloPlugin } from "./channel.js"; describe("zalo directory", () => { const runtimeEnv = createDirectoryTestRuntime() as RuntimeEnv; + const directory = expectDirectorySurface(zaloPlugin.directory); - it("lists peers from allowFrom", async () => { + async function expectPeersFromAllowFrom(allowFrom: string[]) { const cfg = { channels: { zalo: { - allowFrom: ["zalo:123", "zl:234", "345"], + allowFrom, }, }, } as unknown as OpenClawConfig; - const directory = expectDirectorySurface(zaloPlugin.directory); - await expect( directory.listPeers({ cfg, @@ -45,34 +44,14 @@ describe("zalo directory", () => { runtime: runtimeEnv, }), ).resolves.toEqual([]); + } + + it("lists peers from allowFrom", async () => { + await expectPeersFromAllowFrom(["zalo:123", "zl:234", "345"]); }); it("normalizes spaced zalo prefixes in allowFrom and pairing entries", async () => { - const cfg = { - channels: { - zalo: { - allowFrom: [" zalo:123 ", " zl:234 ", " 345 "], - }, - }, - } as unknown as OpenClawConfig; - - const directory = expectDirectorySurface(zaloPlugin.directory); - - await expect( - directory.listPeers({ - cfg, - accountId: undefined, - query: undefined, - limit: undefined, - runtime: runtimeEnv, - }), - ).resolves.toEqual( - expect.arrayContaining([ - { kind: "user", id: "123" }, - { kind: "user", id: "234" }, - { kind: "user", id: "345" }, - ]), - ); + await expectPeersFromAllowFrom([" zalo:123 ", " zl:234 ", " 345 "]); expect(zaloPlugin.pairing?.normalizeAllowEntry?.(" zalo:123 ")).toBe("123"); expect(zaloPlugin.messaging?.normalizeTarget?.(" zl:234 ")).toBe("234"); diff --git a/extensions/zalo/src/monitor.image.polling.test.ts b/extensions/zalo/src/monitor.image.polling.test.ts index 84ffd5c1cac..6fc856ef4ac 100644 --- a/extensions/zalo/src/monitor.image.polling.test.ts +++ b/extensions/zalo/src/monitor.image.polling.test.ts @@ -1,169 +1,63 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { createPluginRuntimeMock } from "../../../test/helpers/extensions/plugin-runtime-mock.js"; import { createRuntimeEnv } from "../../../test/helpers/extensions/runtime-env.js"; -import type { OpenClawConfig, PluginRuntime } from "../runtime-api.js"; -import type { ResolvedZaloAccount } from "./accounts.js"; - -const getWebhookInfoMock = vi.hoisted(() => vi.fn(async () => ({ ok: true, result: { url: "" } }))); -const deleteWebhookMock = vi.hoisted(() => vi.fn(async () => ({ ok: true, result: { url: "" } }))); -const setWebhookMock = vi.hoisted(() => vi.fn(async () => ({ ok: true, result: { url: "" } }))); -const getUpdatesMock = vi.hoisted(() => vi.fn(() => new Promise(() => {}))); -const getZaloRuntimeMock = vi.hoisted(() => vi.fn()); - -vi.mock("./api.js", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - deleteWebhook: deleteWebhookMock, - getUpdates: getUpdatesMock, - getWebhookInfo: getWebhookInfoMock, - setWebhook: setWebhookMock, - }; -}); - -vi.mock("./runtime.js", () => ({ - getZaloRuntime: getZaloRuntimeMock, -})); - -const TEST_ACCOUNT: ResolvedZaloAccount = { - accountId: "default", - enabled: true, - token: "zalo-token", // pragma: allowlist secret - tokenSource: "config", - config: { - dmPolicy: "open", - }, -}; - -const TEST_CONFIG = { - channels: { - zalo: { - enabled: true, - accounts: { - default: { - enabled: true, - dmPolicy: "open", - }, - }, - }, - }, -} as OpenClawConfig; +import { + createImageLifecycleCore, + createImageUpdate, + createLifecycleMonitorSetup, + expectImageLifecycleDelivery, + getUpdatesMock, + getZaloRuntimeMock, + resetLifecycleTestState, +} from "../../../test/helpers/extensions/zalo-lifecycle.js"; describe("Zalo polling image handling", () => { - const finalizeInboundContextMock = vi.fn((ctx: Record) => ctx); - const recordInboundSessionMock = vi.fn(async () => undefined); - const fetchRemoteMediaMock = vi.fn(async () => ({ - buffer: Buffer.from("image-bytes"), - contentType: "image/jpeg", - })); - const saveMediaBufferMock = vi.fn(async () => ({ - path: "/tmp/zalo-photo.jpg", - contentType: "image/jpeg", - })); + const { + core, + finalizeInboundContextMock, + recordInboundSessionMock, + fetchRemoteMediaMock, + saveMediaBufferMock, + } = createImageLifecycleCore(); beforeEach(() => { - vi.clearAllMocks(); - - getZaloRuntimeMock.mockReturnValue( - createPluginRuntimeMock({ - channel: { - media: { - fetchRemoteMedia: - fetchRemoteMediaMock as unknown as PluginRuntime["channel"]["media"]["fetchRemoteMedia"], - saveMediaBuffer: - saveMediaBufferMock as unknown as PluginRuntime["channel"]["media"]["saveMediaBuffer"], - }, - reply: { - finalizeInboundContext: - finalizeInboundContextMock as unknown as PluginRuntime["channel"]["reply"]["finalizeInboundContext"], - dispatchReplyWithBufferedBlockDispatcher: vi.fn( - async () => undefined, - ) as unknown as PluginRuntime["channel"]["reply"]["dispatchReplyWithBufferedBlockDispatcher"], - }, - session: { - recordInboundSession: - recordInboundSessionMock as unknown as PluginRuntime["channel"]["session"]["recordInboundSession"], - }, - commands: { - shouldComputeCommandAuthorized: vi.fn( - () => false, - ) as unknown as PluginRuntime["channel"]["commands"]["shouldComputeCommandAuthorized"], - resolveCommandAuthorizedFromAuthorizers: vi.fn( - () => false, - ) as unknown as PluginRuntime["channel"]["commands"]["resolveCommandAuthorizedFromAuthorizers"], - isControlCommandMessage: vi.fn( - () => false, - ) as unknown as PluginRuntime["channel"]["commands"]["isControlCommandMessage"], - }, - }, - }), - ); + resetLifecycleTestState(); + getZaloRuntimeMock.mockReturnValue(core); }); afterEach(() => { - vi.clearAllMocks(); + resetLifecycleTestState(); }); it("downloads inbound image media from photo_url and preserves display_name", async () => { getUpdatesMock .mockResolvedValueOnce({ ok: true, - result: { - event_name: "message.image.received", - message: { - chat: { - id: "chat-123", - chat_type: "PRIVATE" as const, - }, - message_id: "msg-123", - date: 1774084566880, - message_type: "CHAT_PHOTO", - from: { - id: "user-123", - is_bot: false, - display_name: "Test User", - }, - photo_url: "https://example.com/test-image.jpg", - caption: "", - }, - }, + result: createImageUpdate({ date: 1774084566880 }), }) .mockImplementation(() => new Promise(() => {})); const { monitorZaloProvider } = await import("./monitor.js"); const abort = new AbortController(); const runtime = createRuntimeEnv(); + const { account, config } = createLifecycleMonitorSetup({ + accountId: "default", + dmPolicy: "open", + }); const run = monitorZaloProvider({ token: "zalo-token", // pragma: allowlist secret - account: TEST_ACCOUNT, - config: TEST_CONFIG, + account, + config, runtime, abortSignal: abort.signal, }); - await vi.waitFor(() => - expect(fetchRemoteMediaMock).toHaveBeenCalledWith({ - url: "https://example.com/test-image.jpg", - maxBytes: 5 * 1024 * 1024, - }), - ); - expect(saveMediaBufferMock).toHaveBeenCalledTimes(1); - expect(finalizeInboundContextMock).toHaveBeenCalledWith( - expect.objectContaining({ - SenderName: "Test User", - MediaPath: "/tmp/zalo-photo.jpg", - MediaType: "image/jpeg", - }), - ); - expect(recordInboundSessionMock).toHaveBeenCalledWith( - expect.objectContaining({ - ctx: expect.objectContaining({ - SenderName: "Test User", - MediaPath: "/tmp/zalo-photo.jpg", - MediaType: "image/jpeg", - }), - }), - ); + await vi.waitFor(() => expect(fetchRemoteMediaMock).toHaveBeenCalledTimes(1)); + expectImageLifecycleDelivery({ + fetchRemoteMediaMock, + saveMediaBufferMock, + finalizeInboundContextMock, + recordInboundSessionMock, + }); abort.abort(); await run; diff --git a/extensions/zalo/src/monitor.pairing.lifecycle.test.ts b/extensions/zalo/src/monitor.pairing.lifecycle.test.ts index b9c3f563c93..4d55519a886 100644 --- a/extensions/zalo/src/monitor.pairing.lifecycle.test.ts +++ b/extensions/zalo/src/monitor.pairing.lifecycle.test.ts @@ -1,85 +1,66 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { createPluginRuntimeMock } from "../../../test/helpers/extensions/plugin-runtime-mock.js"; import { - createLifecycleAccount, - createLifecycleConfig, + createLifecycleMonitorSetup, createTextUpdate, - getZaloRuntimeMock, - postWebhookUpdate, + postWebhookReplay, resetLifecycleTestState, + setLifecycleRuntimeCore, sendMessageMock, settleAsyncWork, startWebhookLifecycleMonitor, } from "../../../test/helpers/extensions/zalo-lifecycle.js"; import { withServer } from "../../../test/helpers/http-test-server.js"; -import type { PluginRuntime } from "../runtime-api.js"; describe("Zalo pairing lifecycle", () => { const readAllowFromStoreMock = vi.fn(async () => [] as string[]); const upsertPairingRequestMock = vi.fn(async () => ({ code: "PAIRCODE", created: true })); + beforeEach(() => { resetLifecycleTestState(); - - getZaloRuntimeMock.mockReturnValue( - createPluginRuntimeMock({ - channel: { - pairing: { - readAllowFromStore: - readAllowFromStoreMock as unknown as PluginRuntime["channel"]["pairing"]["readAllowFromStore"], - upsertPairingRequest: - upsertPairingRequestMock as unknown as PluginRuntime["channel"]["pairing"]["upsertPairingRequest"], - }, - commands: { - shouldComputeCommandAuthorized: vi.fn(() => false), - resolveCommandAuthorizedFromAuthorizers: vi.fn(() => false), - }, - }, - }), - ); + setLifecycleRuntimeCore({ + pairing: { + readAllowFromStore: readAllowFromStoreMock, + upsertPairingRequest: upsertPairingRequestMock, + }, + commands: { + shouldComputeCommandAuthorized: vi.fn(() => false), + resolveCommandAuthorizedFromAuthorizers: vi.fn(() => false), + }, + }); }); afterEach(() => { resetLifecycleTestState(); }); - it("emits one pairing reply across duplicate webhook replay and scopes reads and writes to accountId", async () => { - const { abort, route, run } = await startWebhookLifecycleMonitor({ - account: createLifecycleAccount({ - accountId: "acct-zalo-pairing", - dmPolicy: "pairing", - allowFrom: [], - }), - config: createLifecycleConfig({ - accountId: "acct-zalo-pairing", - dmPolicy: "pairing", - allowFrom: [], - }), + function createPairingMonitorSetup() { + return createLifecycleMonitorSetup({ + accountId: "acct-zalo-pairing", + dmPolicy: "pairing", + allowFrom: [], }); + } + + it("emits one pairing reply across duplicate webhook replay and scopes reads and writes to accountId", async () => { + const { abort, route, run } = await startWebhookLifecycleMonitor(createPairingMonitorSetup()); await withServer( (req, res) => route.handler(req, res), async (baseUrl) => { - const payload = createTextUpdate({ - messageId: `zalo-pairing-${Date.now()}`, - userId: "user-unauthorized", - userName: "Unauthorized User", - chatId: "dm-pairing-1", - }); - const first = await postWebhookUpdate({ + const { first, replay } = await postWebhookReplay({ baseUrl, path: "/hooks/zalo", secret: "supersecret", - payload, - }); - const second = await postWebhookUpdate({ - baseUrl, - path: "/hooks/zalo", - secret: "supersecret", - payload, + payload: createTextUpdate({ + messageId: `zalo-pairing-${Date.now()}`, + userId: "user-unauthorized", + userName: "Unauthorized User", + chatId: "dm-pairing-1", + }), }); expect(first.status).toBe(200); - expect(second.status).toBe(200); + expect(replay.status).toBe(200); await settleAsyncWork(); }, ); @@ -116,40 +97,24 @@ describe("Zalo pairing lifecycle", () => { it("does not emit a second pairing reply when replay arrives after the first send fails", async () => { sendMessageMock.mockRejectedValueOnce(new Error("pairing send failed")); - const { abort, route, run, runtime } = await startWebhookLifecycleMonitor({ - account: createLifecycleAccount({ - accountId: "acct-zalo-pairing", - dmPolicy: "pairing", - allowFrom: [], - }), - config: createLifecycleConfig({ - accountId: "acct-zalo-pairing", - dmPolicy: "pairing", - allowFrom: [], - }), - }); + const { abort, route, run, runtime } = await startWebhookLifecycleMonitor( + createPairingMonitorSetup(), + ); await withServer( (req, res) => route.handler(req, res), async (baseUrl) => { - const payload = createTextUpdate({ - messageId: `zalo-pairing-retry-${Date.now()}`, - userId: "user-unauthorized", - userName: "Unauthorized User", - chatId: "dm-pairing-1", - }); - const first = await postWebhookUpdate({ + const { first, replay } = await postWebhookReplay({ baseUrl, path: "/hooks/zalo", secret: "supersecret", - payload, - }); - await settleAsyncWork(); - const replay = await postWebhookUpdate({ - baseUrl, - path: "/hooks/zalo", - secret: "supersecret", - payload, + payload: createTextUpdate({ + messageId: `zalo-pairing-retry-${Date.now()}`, + userId: "user-unauthorized", + userName: "Unauthorized User", + chatId: "dm-pairing-1", + }), + settleBeforeReplay: true, }); expect(first.status).toBe(200); diff --git a/extensions/zalo/src/monitor.reply-once.lifecycle.test.ts b/extensions/zalo/src/monitor.reply-once.lifecycle.test.ts index 9b1820832c0..3e95c65db53 100644 --- a/extensions/zalo/src/monitor.reply-once.lifecycle.test.ts +++ b/extensions/zalo/src/monitor.reply-once.lifecycle.test.ts @@ -1,12 +1,10 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { createPluginRuntimeMock } from "../../../test/helpers/extensions/plugin-runtime-mock.js"; import { - createLifecycleAccount, - createLifecycleConfig, + createLifecycleMonitorSetup, createTextUpdate, - getZaloRuntimeMock, - postWebhookUpdate, + postWebhookReplay, resetLifecycleTestState, + setLifecycleRuntimeCore, sendMessageMock, settleAsyncWork, startWebhookLifecycleMonitor, @@ -29,33 +27,35 @@ describe("Zalo reply-once lifecycle", () => { beforeEach(() => { resetLifecycleTestState(); - - getZaloRuntimeMock.mockReturnValue( - createPluginRuntimeMock({ - channel: { - routing: { - resolveAgentRoute: - resolveAgentRouteMock as unknown as PluginRuntime["channel"]["routing"]["resolveAgentRoute"], - }, - reply: { - finalizeInboundContext: - finalizeInboundContextMock as unknown as PluginRuntime["channel"]["reply"]["finalizeInboundContext"], - dispatchReplyWithBufferedBlockDispatcher: - dispatchReplyWithBufferedBlockDispatcherMock as unknown as PluginRuntime["channel"]["reply"]["dispatchReplyWithBufferedBlockDispatcher"], - }, - session: { - recordInboundSession: - recordInboundSessionMock as unknown as PluginRuntime["channel"]["session"]["recordInboundSession"], - }, - }, - }), - ); + setLifecycleRuntimeCore({ + routing: { + resolveAgentRoute: + resolveAgentRouteMock as unknown as PluginRuntime["channel"]["routing"]["resolveAgentRoute"], + }, + reply: { + finalizeInboundContext: + finalizeInboundContextMock as unknown as PluginRuntime["channel"]["reply"]["finalizeInboundContext"], + dispatchReplyWithBufferedBlockDispatcher: + dispatchReplyWithBufferedBlockDispatcherMock as unknown as PluginRuntime["channel"]["reply"]["dispatchReplyWithBufferedBlockDispatcher"], + }, + session: { + recordInboundSession: + recordInboundSessionMock as unknown as PluginRuntime["channel"]["session"]["recordInboundSession"], + }, + }); }); afterEach(() => { resetLifecycleTestState(); }); + function createReplyOnceMonitorSetup() { + return createLifecycleMonitorSetup({ + accountId: "acct-zalo-lifecycle", + dmPolicy: "open", + }); + } + it("routes one accepted webhook event to one visible reply across duplicate replay", async () => { dispatchReplyWithBufferedBlockDispatcherMock.mockImplementation( async ({ dispatcherOptions }) => { @@ -63,41 +63,25 @@ describe("Zalo reply-once lifecycle", () => { }, ); - const { abort, route, run } = await startWebhookLifecycleMonitor({ - account: createLifecycleAccount({ - accountId: "acct-zalo-lifecycle", - dmPolicy: "open", - }), - config: createLifecycleConfig({ - accountId: "acct-zalo-lifecycle", - dmPolicy: "open", - }), - }); + const { abort, route, run } = await startWebhookLifecycleMonitor(createReplyOnceMonitorSetup()); await withServer( (req, res) => route.handler(req, res), async (baseUrl) => { - const payload = createTextUpdate({ - messageId: `zalo-replay-${Date.now()}`, - userId: "user-1", - userName: "User One", - chatId: "dm-chat-1", - }); - const first = await postWebhookUpdate({ + const { first, replay } = await postWebhookReplay({ baseUrl, path: "/hooks/zalo", secret: "supersecret", - payload, - }); - const second = await postWebhookUpdate({ - baseUrl, - path: "/hooks/zalo", - secret: "supersecret", - payload, + payload: createTextUpdate({ + messageId: `zalo-replay-${Date.now()}`, + userId: "user-1", + userName: "User One", + chatId: "dm-chat-1", + }), }); expect(first.status).toBe(200); - expect(second.status).toBe(200); + expect(replay.status).toBe(200); await settleAsyncWork(); }, ); @@ -144,38 +128,24 @@ describe("Zalo reply-once lifecycle", () => { }, ); - const { abort, route, run, runtime } = await startWebhookLifecycleMonitor({ - account: createLifecycleAccount({ - accountId: "acct-zalo-lifecycle", - dmPolicy: "open", - }), - config: createLifecycleConfig({ - accountId: "acct-zalo-lifecycle", - dmPolicy: "open", - }), - }); + const { abort, route, run, runtime } = await startWebhookLifecycleMonitor( + createReplyOnceMonitorSetup(), + ); await withServer( (req, res) => route.handler(req, res), async (baseUrl) => { - const payload = createTextUpdate({ - messageId: `zalo-retry-${Date.now()}`, - userId: "user-1", - userName: "User One", - chatId: "dm-chat-1", - }); - const first = await postWebhookUpdate({ + const { first, replay } = await postWebhookReplay({ baseUrl, path: "/hooks/zalo", secret: "supersecret", - payload, - }); - await settleAsyncWork(); - const replay = await postWebhookUpdate({ - baseUrl, - path: "/hooks/zalo", - secret: "supersecret", - payload, + payload: createTextUpdate({ + messageId: `zalo-retry-${Date.now()}`, + userId: "user-1", + userName: "User One", + chatId: "dm-chat-1", + }), + settleBeforeReplay: true, }); expect(first.status).toBe(200); diff --git a/extensions/zalo/src/monitor.webhook.test.ts b/extensions/zalo/src/monitor.webhook.test.ts index d1cca9bb2e9..15b1cd96b7b 100644 --- a/extensions/zalo/src/monitor.webhook.test.ts +++ b/extensions/zalo/src/monitor.webhook.test.ts @@ -2,7 +2,13 @@ import type { RequestListener } from "node:http"; import { afterEach, describe, expect, it, vi } from "vitest"; import { createEmptyPluginRegistry } from "../../../src/plugins/registry.js"; import { setActivePluginRegistry } from "../../../src/plugins/runtime.js"; -import { createPluginRuntimeMock } from "../../../test/helpers/extensions/plugin-runtime-mock.js"; +import { + createImageLifecycleCore, + createImageUpdate, + createTextUpdate, + expectImageLifecycleDelivery, + postWebhookReplay, +} from "../../../test/helpers/extensions/zalo-lifecycle.js"; import { withServer } from "../../../test/helpers/http-test-server.js"; import type { OpenClawConfig, PluginRuntime } from "../runtime-api.js"; import { @@ -206,39 +212,25 @@ describe("handleZaloWebhookRequest", () => { it("deduplicates webhook replay by event_name + message_id", async () => { const sink = vi.fn(); const unregister = registerTarget({ path: "/hook-replay", statusSink: sink }); - - const payload = { - event_name: "message.text.received", - message: { - from: { id: "123" }, - chat: { id: "123", chat_type: "PRIVATE" }, - message_id: "msg-replay-1", - date: Math.floor(Date.now() / 1000), - text: "hello", - }, - }; + const payload = createTextUpdate({ + messageId: "msg-replay-1", + userId: "123", + userName: "", + chatId: "123", + text: "hello", + }); try { await withServer(webhookRequestHandler, async (baseUrl) => { - const first = await fetch(`${baseUrl}/hook-replay`, { - method: "POST", - headers: { - "x-bot-api-secret-token": "secret", - "content-type": "application/json", - }, - body: JSON.stringify(payload), - }); - const second = await fetch(`${baseUrl}/hook-replay`, { - method: "POST", - headers: { - "x-bot-api-secret-token": "secret", - "content-type": "application/json", - }, - body: JSON.stringify(payload), + const { first, replay } = await postWebhookReplay({ + baseUrl, + path: "/hook-replay", + secret: "secret", + payload, }); expect(first.status).toBe(200); - expect(second.status).toBe(200); + expect(replay.status).toBe(200); expect(sink).toHaveBeenCalledTimes(1); }); } finally { @@ -247,48 +239,13 @@ describe("handleZaloWebhookRequest", () => { }); it("downloads inbound image media from webhook photo_url and preserves display_name", async () => { - const finalizeInboundContextMock = vi.fn((ctx: Record) => ctx); - const recordInboundSessionMock = vi.fn(async () => undefined); - const fetchRemoteMediaMock = vi.fn(async () => ({ - buffer: Buffer.from("image-bytes"), - contentType: "image/jpeg", - })); - const saveMediaBufferMock = vi.fn(async () => ({ - path: "/tmp/zalo-photo.jpg", - contentType: "image/jpeg", - })); - const core = createPluginRuntimeMock({ - channel: { - media: { - fetchRemoteMedia: - fetchRemoteMediaMock as unknown as PluginRuntime["channel"]["media"]["fetchRemoteMedia"], - saveMediaBuffer: - saveMediaBufferMock as unknown as PluginRuntime["channel"]["media"]["saveMediaBuffer"], - }, - reply: { - finalizeInboundContext: - finalizeInboundContextMock as unknown as PluginRuntime["channel"]["reply"]["finalizeInboundContext"], - dispatchReplyWithBufferedBlockDispatcher: vi.fn( - async () => undefined, - ) as unknown as PluginRuntime["channel"]["reply"]["dispatchReplyWithBufferedBlockDispatcher"], - }, - session: { - recordInboundSession: - recordInboundSessionMock as unknown as PluginRuntime["channel"]["session"]["recordInboundSession"], - }, - commands: { - shouldComputeCommandAuthorized: vi.fn( - () => false, - ) as unknown as PluginRuntime["channel"]["commands"]["shouldComputeCommandAuthorized"], - resolveCommandAuthorizedFromAuthorizers: vi.fn( - () => false, - ) as unknown as PluginRuntime["channel"]["commands"]["resolveCommandAuthorizedFromAuthorizers"], - isControlCommandMessage: vi.fn( - () => false, - ) as unknown as PluginRuntime["channel"]["commands"]["isControlCommandMessage"], - }, - }, - }); + const { + core, + finalizeInboundContextMock, + recordInboundSessionMock, + fetchRemoteMediaMock, + saveMediaBufferMock, + } = createImageLifecycleCore(); const unregister = registerTarget({ path: "/hook-image", core, @@ -299,19 +256,7 @@ describe("handleZaloWebhookRequest", () => { }, }, }); - - const payload = { - event_name: "message.image.received", - message: { - date: 1774086023728, - chat: { chat_type: "PRIVATE", id: "chat-123" }, - caption: "", - message_id: "msg-123", - message_type: "CHAT_PHOTO", - from: { id: "user-123", is_bot: false, display_name: "Test User" }, - photo_url: "https://example.com/test-image.jpg", - }, - }; + const payload = createImageUpdate(); try { await withServer(webhookRequestHandler, async (baseUrl) => { @@ -330,29 +275,13 @@ describe("handleZaloWebhookRequest", () => { unregister(); } - await vi.waitFor(() => - expect(fetchRemoteMediaMock).toHaveBeenCalledWith({ - url: "https://example.com/test-image.jpg", - maxBytes: 5 * 1024 * 1024, - }), - ); - expect(saveMediaBufferMock).toHaveBeenCalledTimes(1); - expect(finalizeInboundContextMock).toHaveBeenCalledWith( - expect.objectContaining({ - SenderName: "Test User", - MediaPath: "/tmp/zalo-photo.jpg", - MediaType: "image/jpeg", - }), - ); - expect(recordInboundSessionMock).toHaveBeenCalledWith( - expect.objectContaining({ - ctx: expect.objectContaining({ - SenderName: "Test User", - MediaPath: "/tmp/zalo-photo.jpg", - MediaType: "image/jpeg", - }), - }), - ); + await vi.waitFor(() => expect(fetchRemoteMediaMock).toHaveBeenCalledTimes(1)); + expectImageLifecycleDelivery({ + fetchRemoteMediaMock, + saveMediaBufferMock, + finalizeInboundContextMock, + recordInboundSessionMock, + }); }); it("returns 429 when per-path request rate exceeds threshold", async () => { diff --git a/scripts/check-architecture-smells.mjs b/scripts/check-architecture-smells.mjs index 1fb74428793..78d84490ef7 100644 --- a/scripts/check-architecture-smells.mjs +++ b/scripts/check-architecture-smells.mjs @@ -1,9 +1,15 @@ #!/usr/bin/env node -import { promises as fs } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; import ts from "typescript"; +import { + collectTypeScriptInventory, + normalizeRepoPath, + resolveRepoSpecifier, + visitModuleSpecifiers, + writeLine, +} from "./lib/guard-inventory-utils.mjs"; import { collectTypeScriptFilesFromRoots, resolveSourceRoots, @@ -14,10 +20,6 @@ import { const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const scanRoots = resolveSourceRoots(repoRoot, ["src/plugin-sdk", "src/plugins/runtime"]); -function normalizePath(filePath) { - return path.relative(repoRoot, filePath).split(path.sep).join("/"); -} - function compareEntries(left, right) { return ( left.category.localeCompare(right.category) || @@ -29,58 +31,41 @@ function compareEntries(left, right) { ); } -function resolveSpecifier(specifier, importerFile) { - if (specifier.startsWith(".")) { - return normalizePath(path.resolve(path.dirname(importerFile), specifier)); - } - if (specifier.startsWith("/")) { - return normalizePath(specifier); - } - return null; -} - function pushEntry(entries, entry) { entries.push(entry); } function scanPluginSdkExtensionFacadeSmells(sourceFile, filePath) { - const relativeFile = normalizePath(filePath); + const relativeFile = normalizeRepoPath(repoRoot, filePath); if (!relativeFile.startsWith("src/plugin-sdk/")) { return []; } const entries = []; - function visit(node) { - if ( - ts.isExportDeclaration(node) && - node.moduleSpecifier && - ts.isStringLiteral(node.moduleSpecifier) - ) { - const specifier = node.moduleSpecifier.text; - const resolvedPath = resolveSpecifier(specifier, filePath); - if (resolvedPath?.startsWith("extensions/")) { - pushEntry(entries, { - category: "plugin-sdk-extension-facade", - file: relativeFile, - line: toLine(sourceFile, node.moduleSpecifier), - kind: "export", - specifier, - resolvedPath, - reason: "plugin-sdk public surface re-exports extension-owned implementation", - }); - } + visitModuleSpecifiers(ts, sourceFile, ({ kind, specifier, specifierNode }) => { + if (kind !== "export") { + return; } - - ts.forEachChild(node, visit); - } - - visit(sourceFile); + const resolvedPath = resolveRepoSpecifier(repoRoot, specifier, filePath); + if (!resolvedPath?.startsWith("extensions/")) { + return; + } + pushEntry(entries, { + category: "plugin-sdk-extension-facade", + file: relativeFile, + line: toLine(sourceFile, specifierNode), + kind, + specifier, + resolvedPath, + reason: "plugin-sdk public surface re-exports extension-owned implementation", + }); + }); return entries; } function scanRuntimeTypeImplementationSmells(sourceFile, filePath) { - const relativeFile = normalizePath(filePath); + const relativeFile = normalizeRepoPath(repoRoot, filePath); if (!/^src\/plugins\/runtime\/types(?:-[^/]+)?\.ts$/.test(relativeFile)) { return []; } @@ -94,7 +79,7 @@ function scanRuntimeTypeImplementationSmells(sourceFile, filePath) { ts.isStringLiteral(node.argument.literal) ) { const specifier = node.argument.literal.text; - const resolvedPath = resolveSpecifier(specifier, filePath); + const resolvedPath = resolveRepoSpecifier(repoRoot, specifier, filePath); if ( resolvedPath && (/^src\/plugins\/runtime\/runtime-[^/]+\.ts$/.test(resolvedPath) || @@ -120,7 +105,7 @@ function scanRuntimeTypeImplementationSmells(sourceFile, filePath) { } function scanRuntimeServiceLocatorSmells(sourceFile, filePath) { - const relativeFile = normalizePath(filePath); + const relativeFile = normalizeRepoPath(repoRoot, filePath); if ( !relativeFile.startsWith("src/plugin-sdk/") && !relativeFile.startsWith("src/plugins/runtime/") @@ -210,25 +195,20 @@ function scanRuntimeServiceLocatorSmells(sourceFile, filePath) { export async function collectArchitectureSmells() { const files = (await collectTypeScriptFilesFromRoots(scanRoots)).toSorted((left, right) => - normalizePath(left).localeCompare(normalizePath(right)), + normalizeRepoPath(repoRoot, left).localeCompare(normalizeRepoPath(repoRoot, right)), ); - - const inventory = []; - for (const filePath of files) { - const source = await fs.readFile(filePath, "utf8"); - const sourceFile = ts.createSourceFile( - filePath, - source, - ts.ScriptTarget.Latest, - true, - ts.ScriptKind.TS, - ); - inventory.push(...scanPluginSdkExtensionFacadeSmells(sourceFile, filePath)); - inventory.push(...scanRuntimeTypeImplementationSmells(sourceFile, filePath)); - inventory.push(...scanRuntimeServiceLocatorSmells(sourceFile, filePath)); - } - - return inventory.toSorted(compareEntries); + return await collectTypeScriptInventory({ + ts, + files, + compareEntries, + collectEntries(sourceFile, filePath) { + return [ + ...scanPluginSdkExtensionFacadeSmells(sourceFile, filePath), + ...scanRuntimeTypeImplementationSmells(sourceFile, filePath), + ...scanRuntimeServiceLocatorSmells(sourceFile, filePath), + ]; + }, + }); } function formatInventoryHuman(inventory) { @@ -256,10 +236,6 @@ function formatInventoryHuman(inventory) { return lines.join("\n"); } -function writeLine(stream, text) { - stream.write(`${text}\n`); -} - export async function runArchitectureSmellsCheck(argv = process.argv.slice(2), io) { const streams = io ?? { stdout: process.stdout, stderr: process.stderr }; const json = argv.includes("--json"); diff --git a/scripts/check-extension-plugin-sdk-boundary.mjs b/scripts/check-extension-plugin-sdk-boundary.mjs index 41461f16450..c235c7a9823 100644 --- a/scripts/check-extension-plugin-sdk-boundary.mjs +++ b/scripts/check-extension-plugin-sdk-boundary.mjs @@ -4,6 +4,14 @@ import { promises as fs } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; import ts from "typescript"; +import { + diffInventoryEntries, + normalizeRepoPath, + resolveRepoSpecifier, + visitModuleSpecifiers, + writeLine, +} from "./lib/guard-inventory-utils.mjs"; +import { toLine } from "./lib/ts-guard-utils.mjs"; const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const extensionsRoot = path.join(repoRoot, "extensions"); @@ -47,10 +55,6 @@ const ruleTextByMode = { "Rule: production extensions/** must not use relative imports that escape their own extension package root", }; -function normalizePath(filePath) { - return path.relative(repoRoot, filePath).split(path.sep).join("/"); -} - function isCodeFile(fileName) { return /\.(ts|tsx|mts|cts|js|jsx|mjs|cjs)$/.test(fileName); } @@ -79,7 +83,7 @@ async function collectExtensionSourceFiles(rootDir) { if (!entry.isFile() || !isCodeFile(entry.name)) { continue; } - const relativePath = normalizePath(fullPath); + const relativePath = normalizeRepoPath(repoRoot, fullPath); if (isTestLikeFile(relativePath)) { continue; } @@ -87,7 +91,9 @@ async function collectExtensionSourceFiles(rootDir) { } } await walk(rootDir); - return out.toSorted((left, right) => normalizePath(left).localeCompare(normalizePath(right))); + return out.toSorted((left, right) => + normalizeRepoPath(repoRoot, left).localeCompare(normalizeRepoPath(repoRoot, right)), + ); } async function collectParsedExtensionSourceFiles() { @@ -118,22 +124,8 @@ async function collectParsedExtensionSourceFiles() { return await parsedExtensionSourceFilesPromise; } -function toLine(sourceFile, node) { - return sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)).line + 1; -} - -function resolveSpecifier(specifier, importerFile) { - if (specifier.startsWith(".")) { - return normalizePath(path.resolve(path.dirname(importerFile), specifier)); - } - if (specifier.startsWith("/")) { - return normalizePath(specifier); - } - return null; -} - function resolveExtensionRoot(filePath) { - const relativePath = normalizePath(filePath); + const relativePath = normalizeRepoPath(repoRoot, filePath); const segments = relativePath.split("/"); if (segments[0] !== "extensions" || !segments[1]) { return null; @@ -200,11 +192,12 @@ function collectEntriesByModeFromSourceFile(sourceFile, filePath) { "relative-outside-package": [], }; const extensionRoot = resolveExtensionRoot(filePath); + const relativeFile = normalizeRepoPath(repoRoot, filePath); function push(kind, specifierNode, specifier) { - const resolvedPath = resolveSpecifier(specifier, filePath); + const resolvedPath = resolveRepoSpecifier(repoRoot, specifier, filePath); const baseEntry = { - file: normalizePath(filePath), + file: relativeFile, line: toLine(sourceFile, specifierNode), kind, specifier, @@ -231,27 +224,9 @@ function collectEntriesByModeFromSourceFile(sourceFile, filePath) { } } - function visit(node) { - if (ts.isImportDeclaration(node) && ts.isStringLiteral(node.moduleSpecifier)) { - push("import", node.moduleSpecifier, node.moduleSpecifier.text); - } else if ( - ts.isExportDeclaration(node) && - node.moduleSpecifier && - ts.isStringLiteral(node.moduleSpecifier) - ) { - push("export", node.moduleSpecifier, node.moduleSpecifier.text); - } else if ( - ts.isCallExpression(node) && - node.expression.kind === ts.SyntaxKind.ImportKeyword && - node.arguments.length === 1 && - ts.isStringLiteral(node.arguments[0]) - ) { - push("dynamic-import", node.arguments[0], node.arguments[0].text); - } - ts.forEachChild(node, visit); - } - - visit(sourceFile); + visitModuleSpecifiers(ts, sourceFile, ({ kind, specifier, specifierNode }) => { + push(kind, specifierNode, specifier); + }); return entriesByMode; } @@ -303,16 +278,7 @@ export async function readExpectedInventory(mode) { } export function diffInventory(expected, actual) { - const expectedKeys = new Set(expected.map((entry) => JSON.stringify(entry))); - const actualKeys = new Set(actual.map((entry) => JSON.stringify(entry))); - return { - missing: expected - .filter((entry) => !actualKeys.has(JSON.stringify(entry))) - .toSorted(compareEntries), - unexpected: actual - .filter((entry) => !expectedKeys.has(JSON.stringify(entry))) - .toSorted(compareEntries), - }; + return diffInventoryEntries(expected, actual, compareEntries); } function formatInventoryHuman(mode, inventory) { @@ -335,10 +301,6 @@ function formatInventoryHuman(mode, inventory) { return lines.join("\n"); } -function writeLine(stream, text) { - stream.write(`${text}\n`); -} - export async function runExtensionPluginSdkBoundaryCheck(argv = process.argv.slice(2), io) { const streams = io ?? { stdout: process.stdout, stderr: process.stderr }; const json = argv.includes("--json"); diff --git a/scripts/check-ingress-agent-owner-context.mjs b/scripts/check-ingress-agent-owner-context.mjs index da9da112c6b..d01026ad105 100644 --- a/scripts/check-ingress-agent-owner-context.mjs +++ b/scripts/check-ingress-agent-owner-context.mjs @@ -3,7 +3,11 @@ import path from "node:path"; import ts from "typescript"; import { runCallsiteGuard } from "./lib/callsite-guard.mjs"; -import { runAsScript, toLine, unwrapExpression } from "./lib/ts-guard-utils.mjs"; +import { + collectCallExpressionLines, + runAsScript, + unwrapExpression, +} from "./lib/ts-guard-utils.mjs"; const sourceRoots = ["src/gateway", "extensions/discord/src/voice"]; const enforcedFiles = new Set([ @@ -16,18 +20,10 @@ const enforcedFiles = new Set([ export function findLegacyAgentCommandCallLines(content, fileName = "source.ts") { const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.Latest, true); - const lines = []; - const visit = (node) => { - if (ts.isCallExpression(node)) { - const callee = unwrapExpression(node.expression); - if (ts.isIdentifier(callee) && callee.text === "agentCommand") { - lines.push(toLine(sourceFile, callee)); - } - } - ts.forEachChild(node, visit); - }; - visit(sourceFile); - return lines; + return collectCallExpressionLines(ts, sourceFile, (node) => { + const callee = unwrapExpression(node.expression); + return ts.isIdentifier(callee) && callee.text === "agentCommand" ? callee : null; + }); } export async function main() { diff --git a/scripts/check-no-random-messaging-tmp.mjs b/scripts/check-no-random-messaging-tmp.mjs index ae5469d6deb..30cf1509f29 100644 --- a/scripts/check-no-random-messaging-tmp.mjs +++ b/scripts/check-no-random-messaging-tmp.mjs @@ -2,7 +2,11 @@ import ts from "typescript"; import { runCallsiteGuard } from "./lib/callsite-guard.mjs"; -import { runAsScript, toLine, unwrapExpression } from "./lib/ts-guard-utils.mjs"; +import { + collectCallExpressionLines, + runAsScript, + unwrapExpression, +} from "./lib/ts-guard-utils.mjs"; const sourceRoots = [ "src/channels", @@ -50,27 +54,18 @@ function collectOsTmpdirImports(sourceFile) { export function findMessagingTmpdirCallLines(content, fileName = "source.ts") { const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.Latest, true); const { osNamespaceOrDefault, namedTmpdir } = collectOsTmpdirImports(sourceFile); - const lines = []; - - const visit = (node) => { - if (ts.isCallExpression(node)) { - const callee = unwrapExpression(node.expression); - if ( - ts.isPropertyAccessExpression(callee) && - callee.name.text === "tmpdir" && - ts.isIdentifier(callee.expression) && - osNamespaceOrDefault.has(callee.expression.text) - ) { - lines.push(toLine(sourceFile, callee)); - } else if (ts.isIdentifier(callee) && namedTmpdir.has(callee.text)) { - lines.push(toLine(sourceFile, callee)); - } + return collectCallExpressionLines(ts, sourceFile, (node) => { + const callee = unwrapExpression(node.expression); + if ( + ts.isPropertyAccessExpression(callee) && + callee.name.text === "tmpdir" && + ts.isIdentifier(callee.expression) && + osNamespaceOrDefault.has(callee.expression.text) + ) { + return callee; } - ts.forEachChild(node, visit); - }; - - visit(sourceFile); - return lines; + return ts.isIdentifier(callee) && namedTmpdir.has(callee.text) ? callee : null; + }); } export async function main() { diff --git a/scripts/check-no-raw-channel-fetch.mjs b/scripts/check-no-raw-channel-fetch.mjs index 57adb600c81..18d801906b1 100644 --- a/scripts/check-no-raw-channel-fetch.mjs +++ b/scripts/check-no-raw-channel-fetch.mjs @@ -2,7 +2,11 @@ import ts from "typescript"; import { runCallsiteGuard } from "./lib/callsite-guard.mjs"; -import { runAsScript, toLine, unwrapExpression } from "./lib/ts-guard-utils.mjs"; +import { + collectCallExpressionLines, + runAsScript, + unwrapExpression, +} from "./lib/ts-guard-utils.mjs"; const sourceRoots = ["src/channels", "src/routing", "src/line", "extensions"]; @@ -65,15 +69,9 @@ function isRawFetchCall(expression) { export function findRawFetchCallLines(content, fileName = "source.ts") { const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.Latest, true); - const lines = []; - const visit = (node) => { - if (ts.isCallExpression(node) && isRawFetchCall(node.expression)) { - lines.push(toLine(sourceFile, node.expression)); - } - ts.forEachChild(node, visit); - }; - visit(sourceFile); - return lines; + return collectCallExpressionLines(ts, sourceFile, (node) => + isRawFetchCall(node.expression) ? node.expression : null, + ); } export async function main() { diff --git a/scripts/check-no-register-http-handler.mjs b/scripts/check-no-register-http-handler.mjs index 0884295be2d..ed949b7328e 100644 --- a/scripts/check-no-register-http-handler.mjs +++ b/scripts/check-no-register-http-handler.mjs @@ -2,7 +2,11 @@ import ts from "typescript"; import { runCallsiteGuard } from "./lib/callsite-guard.mjs"; -import { runAsScript, toLine, unwrapExpression } from "./lib/ts-guard-utils.mjs"; +import { + collectCallExpressionLines, + runAsScript, + unwrapExpression, +} from "./lib/ts-guard-utils.mjs"; const sourceRoots = ["src", "extensions"]; @@ -13,15 +17,9 @@ function isDeprecatedRegisterHttpHandlerCall(expression) { export function findDeprecatedRegisterHttpHandlerLines(content, fileName = "source.ts") { const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.Latest, true); - const lines = []; - const visit = (node) => { - if (ts.isCallExpression(node) && isDeprecatedRegisterHttpHandlerCall(node.expression)) { - lines.push(toLine(sourceFile, node.expression)); - } - ts.forEachChild(node, visit); - }; - visit(sourceFile); - return lines; + return collectCallExpressionLines(ts, sourceFile, (node) => + isDeprecatedRegisterHttpHandlerCall(node.expression) ? node.expression : null, + ); } export async function main() { diff --git a/scripts/check-plugin-extension-import-boundary.mjs b/scripts/check-plugin-extension-import-boundary.mjs index 9c33bcbc320..9bd802287a1 100644 --- a/scripts/check-plugin-extension-import-boundary.mjs +++ b/scripts/check-plugin-extension-import-boundary.mjs @@ -4,6 +4,14 @@ import { promises as fs } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; import ts from "typescript"; +import { + collectTypeScriptInventory, + diffInventoryEntries, + normalizeRepoPath, + runBaselineInventoryCheck, + resolveRepoSpecifier, + visitModuleSpecifiers, +} from "./lib/guard-inventory-utils.mjs"; import { collectTypeScriptFilesFromRoots, resolveSourceRoots, @@ -39,10 +47,6 @@ const bundledWebSearchPluginIds = new Set([ "xai", ]); -function normalizePath(filePath) { - return path.relative(repoRoot, filePath).split(path.sep).join("/"); -} - function compareEntries(left, right) { return ( left.file.localeCompare(right.file) || @@ -53,16 +57,6 @@ function compareEntries(left, right) { ); } -function resolveSpecifier(specifier, importerFile) { - if (specifier.startsWith(".")) { - return normalizePath(path.resolve(path.dirname(importerFile), specifier)); - } - if (specifier.startsWith("/")) { - return normalizePath(specifier); - } - return null; -} - function classifyResolvedExtensionReason(kind, resolvedPath) { const verb = kind === "export" @@ -85,67 +79,27 @@ function pushEntry(entries, entry) { function scanImportBoundaryViolations(sourceFile, filePath) { const entries = []; + const relativeFile = normalizeRepoPath(repoRoot, filePath); - function visit(node) { - if (ts.isImportDeclaration(node) && ts.isStringLiteral(node.moduleSpecifier)) { - const specifier = node.moduleSpecifier.text; - const resolvedPath = resolveSpecifier(specifier, filePath); - if (resolvedPath?.startsWith("extensions/")) { - pushEntry(entries, { - file: normalizePath(filePath), - line: toLine(sourceFile, node.moduleSpecifier), - kind: "import", - specifier, - resolvedPath, - reason: classifyResolvedExtensionReason("import", resolvedPath), - }); - } - } else if ( - ts.isExportDeclaration(node) && - node.moduleSpecifier && - ts.isStringLiteral(node.moduleSpecifier) - ) { - const specifier = node.moduleSpecifier.text; - const resolvedPath = resolveSpecifier(specifier, filePath); - if (resolvedPath?.startsWith("extensions/")) { - pushEntry(entries, { - file: normalizePath(filePath), - line: toLine(sourceFile, node.moduleSpecifier), - kind: "export", - specifier, - resolvedPath, - reason: classifyResolvedExtensionReason("export", resolvedPath), - }); - } - } else if ( - ts.isCallExpression(node) && - node.expression.kind === ts.SyntaxKind.ImportKeyword && - node.arguments.length === 1 && - ts.isStringLiteral(node.arguments[0]) - ) { - const specifier = node.arguments[0].text; - const resolvedPath = resolveSpecifier(specifier, filePath); - if (resolvedPath?.startsWith("extensions/")) { - pushEntry(entries, { - file: normalizePath(filePath), - line: toLine(sourceFile, node.arguments[0]), - kind: "dynamic-import", - specifier, - resolvedPath, - reason: classifyResolvedExtensionReason("dynamic-import", resolvedPath), - }); - } + visitModuleSpecifiers(ts, sourceFile, ({ kind, specifier, specifierNode }) => { + const resolvedPath = resolveRepoSpecifier(repoRoot, specifier, filePath); + if (!resolvedPath?.startsWith("extensions/")) { + return; } - - ts.forEachChild(node, visit); - } - - visit(sourceFile); + pushEntry(entries, { + file: relativeFile, + line: toLine(sourceFile, specifierNode), + kind, + specifier, + resolvedPath, + reason: classifyResolvedExtensionReason(kind, resolvedPath), + }); + }); return entries; } function scanWebSearchRegistrySmells(sourceFile, filePath) { - const relativeFile = normalizePath(filePath); + const relativeFile = normalizeRepoPath(repoRoot, filePath); if (relativeFile !== "src/plugins/web-search-providers.ts") { return []; } @@ -195,7 +149,7 @@ function scanWebSearchRegistrySmells(sourceFile, filePath) { } function shouldSkipFile(filePath) { - const relativeFile = normalizePath(filePath); + const relativeFile = normalizeRepoPath(repoRoot, filePath); return ( relativeFile === "src/plugins/bundled-web-search-registry.ts" || relativeFile.startsWith("src/plugins/contracts/") || @@ -211,23 +165,20 @@ export async function collectPluginExtensionImportBoundaryInventory() { cachedInventoryPromise = (async () => { const files = (await collectTypeScriptFilesFromRoots(scanRoots)) .filter((filePath) => !shouldSkipFile(filePath)) - .toSorted((left, right) => normalizePath(left).localeCompare(normalizePath(right))); - - const inventory = []; - for (const filePath of files) { - const source = await fs.readFile(filePath, "utf8"); - const sourceFile = ts.createSourceFile( - filePath, - source, - ts.ScriptTarget.Latest, - true, - ts.ScriptKind.TS, + .toSorted((left, right) => + normalizeRepoPath(repoRoot, left).localeCompare(normalizeRepoPath(repoRoot, right)), ); - inventory.push(...scanImportBoundaryViolations(sourceFile, filePath)); - inventory.push(...scanWebSearchRegistrySmells(sourceFile, filePath)); - } - - return inventory.toSorted(compareEntries); + return await collectTypeScriptInventory({ + ts, + files, + compareEntries, + collectEntries(sourceFile, filePath) { + return [ + ...scanImportBoundaryViolations(sourceFile, filePath), + ...scanWebSearchRegistrySmells(sourceFile, filePath), + ]; + }, + }); })(); try { @@ -255,16 +206,7 @@ export async function readExpectedInventory() { } export function diffInventory(expected, actual) { - const expectedKeys = new Set(expected.map((entry) => JSON.stringify(entry))); - const actualKeys = new Set(actual.map((entry) => JSON.stringify(entry))); - return { - missing: expected - .filter((entry) => !actualKeys.has(JSON.stringify(entry))) - .toSorted(compareEntries), - unexpected: actual - .filter((entry) => !expectedKeys.has(JSON.stringify(entry))) - .toSorted(compareEntries), - }; + return diffInventoryEntries(expected, actual, compareEntries); } function formatInventoryHuman(inventory) { @@ -293,48 +235,16 @@ function formatEntry(entry) { return `${entry.file}:${entry.line} [${entry.kind}] ${entry.reason} (${entry.specifier} -> ${entry.resolvedPath})`; } -function writeLine(stream, text) { - stream.write(`${text}\n`); -} - export async function runPluginExtensionImportBoundaryCheck(argv = process.argv.slice(2), io) { - const streams = io ?? { stdout: process.stdout, stderr: process.stderr }; - const json = argv.includes("--json"); - const actual = await collectPluginExtensionImportBoundaryInventory(); - const expected = await readExpectedInventory(); - const { missing, unexpected } = diffInventory(expected, actual); - const matchesBaseline = missing.length === 0 && unexpected.length === 0; - - if (json) { - writeLine(streams.stdout, JSON.stringify(actual, null, 2)); - } else { - writeLine(streams.stdout, formatInventoryHuman(actual)); - writeLine( - streams.stdout, - matchesBaseline - ? `Baseline matches (${actual.length} entries).` - : `Baseline mismatch (${unexpected.length} unexpected, ${missing.length} missing).`, - ); - if (!matchesBaseline) { - if (unexpected.length > 0) { - writeLine(streams.stderr, "Unexpected entries:"); - for (const entry of unexpected) { - writeLine(streams.stderr, `- ${formatEntry(entry)}`); - } - } - if (missing.length > 0) { - writeLine(streams.stderr, "Missing baseline entries:"); - for (const entry of missing) { - writeLine(streams.stderr, `- ${formatEntry(entry)}`); - } - } - } - } - - if (!matchesBaseline) { - return 1; - } - return 0; + return await runBaselineInventoryCheck({ + argv, + io, + collectActual: collectPluginExtensionImportBoundaryInventory, + readExpected: readExpectedInventory, + diffInventory, + formatInventoryHuman, + formatEntry, + }); } export async function main(argv = process.argv.slice(2), io) { diff --git a/scripts/check-plugin-sdk-subpath-exports.mjs b/scripts/check-plugin-sdk-subpath-exports.mjs index 07094e18a3b..d494844ce13 100644 --- a/scripts/check-plugin-sdk-subpath-exports.mjs +++ b/scripts/check-plugin-sdk-subpath-exports.mjs @@ -4,6 +4,7 @@ import { readFileSync } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; import ts from "typescript"; +import { normalizeRepoPath, visitModuleSpecifiers } from "./lib/guard-inventory-utils.mjs"; import { collectTypeScriptFilesFromRoots, resolveSourceRoots, @@ -29,10 +30,6 @@ function readEntrypoints() { return new Set(entrypoints.filter((entry) => entry !== "index")); } -function normalizePath(filePath) { - return path.relative(repoRoot, filePath).split(path.sep).join("/"); -} - function parsePluginSdkSubpath(specifier) { if (!specifier.startsWith("openclaw/plugin-sdk/")) { return null; @@ -55,7 +52,8 @@ async function collectViolations() { const entrypoints = readEntrypoints(); const exports = readPackageExports(); const files = (await collectTypeScriptFilesFromRoots(scanRoots, { includeTests: true })).toSorted( - (left, right) => normalizePath(left).localeCompare(normalizePath(right)), + (left, right) => + normalizeRepoPath(repoRoot, left).localeCompare(normalizeRepoPath(repoRoot, right)), ); const violations = []; @@ -87,7 +85,7 @@ async function collectViolations() { } violations.push({ - file: normalizePath(filePath), + file: normalizeRepoPath(repoRoot, filePath), line: toLine(sourceFile, specifierNode), kind, specifier, @@ -96,27 +94,9 @@ async function collectViolations() { }); } - function visit(node) { - if (ts.isImportDeclaration(node) && ts.isStringLiteral(node.moduleSpecifier)) { - push("import", node.moduleSpecifier, node.moduleSpecifier.text); - } else if ( - ts.isExportDeclaration(node) && - node.moduleSpecifier && - ts.isStringLiteral(node.moduleSpecifier) - ) { - push("export", node.moduleSpecifier, node.moduleSpecifier.text); - } else if ( - ts.isCallExpression(node) && - node.expression.kind === ts.SyntaxKind.ImportKeyword && - node.arguments.length === 1 && - ts.isStringLiteral(node.arguments[0]) - ) { - push("dynamic-import", node.arguments[0], node.arguments[0].text); - } - ts.forEachChild(node, visit); - } - - visit(sourceFile); + visitModuleSpecifiers(ts, sourceFile, ({ kind, specifier, specifierNode }) => { + push(kind, specifierNode, specifier); + }); } return violations.toSorted(compareEntries); diff --git a/scripts/check-web-search-provider-boundaries.mjs b/scripts/check-web-search-provider-boundaries.mjs index 914e58bc964..9a23b381bd8 100644 --- a/scripts/check-web-search-provider-boundaries.mjs +++ b/scripts/check-web-search-provider-boundaries.mjs @@ -3,6 +3,11 @@ import { promises as fs } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; +import { + diffInventoryEntries, + normalizeRepoPath, + runBaselineInventoryCheck, +} from "./lib/guard-inventory-utils.mjs"; import { runAsScript } from "./lib/ts-guard-utils.mjs"; const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); @@ -62,10 +67,6 @@ const ignoredFiles = new Set([ let webSearchProviderInventoryPromise; -function normalizeRelativePath(filePath) { - return path.relative(repoRoot, filePath).split(path.sep).join("/"); -} - async function walkFiles(rootDir) { const out = []; let entries = []; @@ -195,11 +196,11 @@ export async function collectWebSearchProviderBoundaryInventory() { ) .flat() .toSorted((left, right) => - normalizeRelativePath(left).localeCompare(normalizeRelativePath(right)), + normalizeRepoPath(repoRoot, left).localeCompare(normalizeRepoPath(repoRoot, right)), ); for (const filePath of files) { - const relativeFile = normalizeRelativePath(filePath); + const relativeFile = normalizeRepoPath(repoRoot, filePath); if (ignoredFiles.has(relativeFile) || relativeFile.includes(".test.")) { continue; } @@ -232,14 +233,7 @@ export async function readExpectedInventory() { } export function diffInventory(expected, actual) { - const expectedKeys = new Set(expected.map((entry) => JSON.stringify(entry))); - const actualKeys = new Set(actual.map((entry) => JSON.stringify(entry))); - const missing = expected.filter((entry) => !actualKeys.has(JSON.stringify(entry))); - const unexpected = actual.filter((entry) => !expectedKeys.has(JSON.stringify(entry))); - return { - missing: missing.toSorted(compareInventoryEntries), - unexpected: unexpected.toSorted(compareInventoryEntries), - }; + return diffInventoryEntries(expected, actual, compareInventoryEntries); } function formatInventoryHuman(inventory) { @@ -262,48 +256,16 @@ function formatEntry(entry) { return `${entry.provider} ${entry.file}:${entry.line} ${entry.reason}`; } -function writeLine(stream, text) { - stream.write(`${text}\n`); -} - export async function runWebSearchProviderBoundaryCheck(argv = process.argv.slice(2), io) { - const streams = io ?? { stdout: process.stdout, stderr: process.stderr }; - const json = argv.includes("--json"); - const actual = await collectWebSearchProviderBoundaryInventory(); - const expected = await readExpectedInventory(); - const { missing, unexpected } = diffInventory(expected, actual); - const matchesBaseline = missing.length === 0 && unexpected.length === 0; - - if (json) { - writeLine(streams.stdout, JSON.stringify(actual, null, 2)); - } else { - writeLine(streams.stdout, formatInventoryHuman(actual)); - writeLine( - streams.stdout, - matchesBaseline - ? `Baseline matches (${actual.length} entries).` - : `Baseline mismatch (${unexpected.length} unexpected, ${missing.length} missing).`, - ); - if (!matchesBaseline) { - if (unexpected.length > 0) { - writeLine(streams.stderr, "Unexpected entries:"); - for (const entry of unexpected) { - writeLine(streams.stderr, `- ${formatEntry(entry)}`); - } - } - if (missing.length > 0) { - writeLine(streams.stderr, "Missing baseline entries:"); - for (const entry of missing) { - writeLine(streams.stderr, `- ${formatEntry(entry)}`); - } - } - } - } - - if (!matchesBaseline) { - return 1; - } - return 0; + return await runBaselineInventoryCheck({ + argv, + io, + collectActual: collectWebSearchProviderBoundaryInventory, + readExpected: readExpectedInventory, + diffInventory, + formatInventoryHuman, + formatEntry, + }); } export async function main(argv = process.argv.slice(2), io) { diff --git a/scripts/generate-bundled-plugin-metadata.mjs b/scripts/generate-bundled-plugin-metadata.mjs index e214c5a7061..4fea8dd62f8 100644 --- a/scripts/generate-bundled-plugin-metadata.mjs +++ b/scripts/generate-bundled-plugin-metadata.mjs @@ -1,8 +1,7 @@ -import fs from "node:fs"; import path from "node:path"; -import { pathToFileURL } from "node:url"; +import { collectBundledPluginSources } from "./lib/bundled-plugin-source-utils.mjs"; import { formatGeneratedModule } from "./lib/format-generated-module.mjs"; -import { writeTextFileIfChanged } from "./runtime-postbuild-shared.mjs"; +import { reportGeneratedOutputCli, writeGeneratedOutput } from "./lib/generated-output-utils.mjs"; const GENERATED_BY = "scripts/generate-bundled-plugin-metadata.mjs"; const DEFAULT_OUTPUT_PATH = "src/plugins/bundled-plugin-metadata.generated.ts"; @@ -16,14 +15,6 @@ const CANONICAL_PACKAGE_ID_ALIASES = { "vllm-provider": "vllm", }; -function readIfExists(filePath) { - try { - return fs.readFileSync(filePath, "utf8"); - } catch { - return null; - } -} - function rewriteEntryToBuiltPath(entry) { if (typeof entry !== "string" || entry.trim().length === 0) { return undefined; @@ -136,30 +127,14 @@ function formatTypeScriptModule(source, { outputPath }) { export function collectBundledPluginMetadata(params = {}) { const repoRoot = path.resolve(params.repoRoot ?? process.cwd()); - const extensionsRoot = path.join(repoRoot, "extensions"); - if (!fs.existsSync(extensionsRoot)) { - return []; - } - const entries = []; - for (const dirent of fs.readdirSync(extensionsRoot, { withFileTypes: true })) { - if (!dirent.isDirectory()) { - continue; - } - - const pluginDir = path.join(extensionsRoot, dirent.name); - const manifestPath = path.join(pluginDir, "openclaw.plugin.json"); - const packageJsonPath = path.join(pluginDir, "package.json"); - if (!fs.existsSync(manifestPath) || !fs.existsSync(packageJsonPath)) { - continue; - } - - const manifest = normalizePluginManifest(JSON.parse(fs.readFileSync(manifestPath, "utf8"))); + for (const source of collectBundledPluginSources({ repoRoot, requirePackageJson: true })) { + const manifest = normalizePluginManifest(source.manifest); if (!manifest) { continue; } - const packageJson = JSON.parse(fs.readFileSync(packageJsonPath, "utf8")); + const packageJson = source.packageJson; const packageManifest = normalizePackageManifest(packageJson); const extensions = Array.isArray(packageManifest?.extensions) ? packageManifest.extensions.filter((entry) => typeof entry === "string" && entry.trim()) @@ -183,7 +158,7 @@ export function collectBundledPluginMetadata(params = {}) { : undefined; entries.push({ - dirName: dirent.name, + dirName: source.dirName, idHint: deriveIdHint({ filePath: sourceEntry, packageName: typeof packageJson.name === "string" ? packageJson.name : undefined, @@ -225,39 +200,16 @@ export function writeBundledPluginMetadataModule(params = {}) { renderBundledPluginMetadataModule(collectBundledPluginMetadata({ repoRoot })), { outputPath }, ); - const current = readIfExists(outputPath); - const changed = current !== next; - - if (params.check) { - return { - changed, - wrote: false, - outputPath, - }; - } - - return { - changed, - wrote: writeTextFileIfChanged(outputPath, next), - outputPath, - }; -} - -if (import.meta.url === pathToFileURL(process.argv[1] ?? "").href) { - const result = writeBundledPluginMetadataModule({ - check: process.argv.includes("--check"), + return writeGeneratedOutput({ + repoRoot, + outputPath: params.outputPath ?? DEFAULT_OUTPUT_PATH, + next, + check: params.check, }); - - if (result.changed) { - if (process.argv.includes("--check")) { - console.error( - `[bundled-plugin-metadata] stale generated output at ${path.relative(process.cwd(), result.outputPath)}`, - ); - process.exitCode = 1; - } else { - console.log( - `[bundled-plugin-metadata] wrote ${path.relative(process.cwd(), result.outputPath)}`, - ); - } - } } + +reportGeneratedOutputCli({ + importMetaUrl: import.meta.url, + label: "bundled-plugin-metadata", + run: ({ check }) => writeBundledPluginMetadataModule({ check }), +}); diff --git a/scripts/generate-bundled-provider-auth-env-vars.mjs b/scripts/generate-bundled-provider-auth-env-vars.mjs index ebcd29360e8..d4c3a279a31 100644 --- a/scripts/generate-bundled-provider-auth-env-vars.mjs +++ b/scripts/generate-bundled-provider-auth-env-vars.mjs @@ -1,19 +1,10 @@ -import fs from "node:fs"; import path from "node:path"; -import { pathToFileURL } from "node:url"; -import { writeTextFileIfChanged } from "./runtime-postbuild-shared.mjs"; +import { collectBundledPluginSources } from "./lib/bundled-plugin-source-utils.mjs"; +import { reportGeneratedOutputCli, writeGeneratedOutput } from "./lib/generated-output-utils.mjs"; const GENERATED_BY = "scripts/generate-bundled-provider-auth-env-vars.mjs"; const DEFAULT_OUTPUT_PATH = "src/plugins/bundled-provider-auth-env-vars.generated.ts"; -function readIfExists(filePath) { - try { - return fs.readFileSync(filePath, "utf8"); - } catch { - return null; - } -} - function normalizeProviderAuthEnvVars(providerAuthEnvVars) { if ( !providerAuthEnvVars || @@ -40,25 +31,10 @@ function normalizeProviderAuthEnvVars(providerAuthEnvVars) { export function collectBundledProviderAuthEnvVars(params = {}) { const repoRoot = path.resolve(params.repoRoot ?? process.cwd()); - const extensionsRoot = path.join(repoRoot, "extensions"); - if (!fs.existsSync(extensionsRoot)) { - return {}; - } - const entries = new Map(); - for (const dirent of fs.readdirSync(extensionsRoot, { withFileTypes: true })) { - if (!dirent.isDirectory()) { - continue; - } - - const manifestPath = path.join(extensionsRoot, dirent.name, "openclaw.plugin.json"); - if (!fs.existsSync(manifestPath)) { - continue; - } - - const manifest = JSON.parse(fs.readFileSync(manifestPath, "utf8")); + for (const source of collectBundledPluginSources({ repoRoot })) { for (const [providerId, envVars] of normalizeProviderAuthEnvVars( - manifest.providerAuthEnvVars, + source.manifest.providerAuthEnvVars, )) { entries.set(providerId, envVars); } @@ -89,43 +65,19 @@ ${renderedEntries} export function writeBundledProviderAuthEnvVarModule(params = {}) { const repoRoot = path.resolve(params.repoRoot ?? process.cwd()); - const outputPath = path.resolve(repoRoot, params.outputPath ?? DEFAULT_OUTPUT_PATH); const next = renderBundledProviderAuthEnvVarModule( collectBundledProviderAuthEnvVars({ repoRoot }), ); - const current = readIfExists(outputPath); - const changed = current !== next; - - if (params.check) { - return { - changed, - wrote: false, - outputPath, - }; - } - - return { - changed, - wrote: writeTextFileIfChanged(outputPath, next), - outputPath, - }; -} - -if (import.meta.url === pathToFileURL(process.argv[1] ?? "").href) { - const result = writeBundledProviderAuthEnvVarModule({ - check: process.argv.includes("--check"), + return writeGeneratedOutput({ + repoRoot, + outputPath: params.outputPath ?? DEFAULT_OUTPUT_PATH, + next, + check: params.check, }); - - if (result.changed) { - if (process.argv.includes("--check")) { - console.error( - `[bundled-provider-auth-env-vars] stale generated output at ${path.relative(process.cwd(), result.outputPath)}`, - ); - process.exitCode = 1; - } else { - console.log( - `[bundled-provider-auth-env-vars] wrote ${path.relative(process.cwd(), result.outputPath)}`, - ); - } - } } + +reportGeneratedOutputCli({ + importMetaUrl: import.meta.url, + label: "bundled-provider-auth-env-vars", + run: ({ check }) => writeBundledProviderAuthEnvVarModule({ check }), +}); diff --git a/scripts/lib/arg-utils.mjs b/scripts/lib/arg-utils.mjs new file mode 100644 index 00000000000..cb3c38a1da6 --- /dev/null +++ b/scripts/lib/arg-utils.mjs @@ -0,0 +1,169 @@ +export function readEnvNumber(name, env = process.env) { + const raw = env[name]?.trim(); + if (!raw) { + return null; + } + const parsed = Number.parseFloat(raw); + return Number.isFinite(parsed) ? parsed : null; +} + +export function consumeStringFlag(argv, index, flag, currentValue) { + if (argv[index] !== flag) { + return null; + } + return { + nextIndex: index + 1, + value: argv[index + 1] ?? currentValue, + }; +} + +export function consumeStringListFlag(argv, index, flag) { + if (argv[index] !== flag) { + return null; + } + const value = argv[index + 1]; + return { + nextIndex: index + 1, + value: typeof value === "string" && value.length > 0 ? value : null, + }; +} + +export function consumeIntFlag(argv, index, flag, currentValue, options = {}) { + if (argv[index] !== flag) { + return null; + } + const parsed = Number.parseInt(argv[index + 1] ?? "", 10); + const min = options.min ?? Number.NEGATIVE_INFINITY; + return { + nextIndex: index + 1, + value: Number.isFinite(parsed) && parsed >= min ? parsed : currentValue, + }; +} + +export function consumeFloatFlag(argv, index, flag, currentValue, options = {}) { + if (argv[index] !== flag) { + return null; + } + const parsed = Number.parseFloat(argv[index + 1] ?? ""); + const min = options.min ?? Number.NEGATIVE_INFINITY; + const includeMin = options.includeMin ?? true; + const isValid = Number.isFinite(parsed) && (includeMin ? parsed >= min : parsed > min); + return { + nextIndex: index + 1, + value: isValid ? parsed : currentValue, + }; +} + +export function stringFlag(flag, key) { + return { + consume(argv, index, args) { + const option = consumeStringFlag(argv, index, flag, args[key]); + if (!option) { + return null; + } + return { + nextIndex: option.nextIndex, + apply(target) { + target[key] = option.value; + }, + }; + }, + }; +} + +export function stringListFlag(flag, key) { + return { + consume(argv, index) { + const option = consumeStringListFlag(argv, index, flag); + if (!option) { + return null; + } + return { + nextIndex: option.nextIndex, + apply(target) { + if (option.value) { + target[key].push(option.value); + } + }, + }; + }, + }; +} + +function createAssignedValueFlag(consumeOption) { + return { + consume(argv, index, args) { + const option = consumeOption(argv, index, args); + if (!option) { + return null; + } + return { + nextIndex: option.nextIndex, + apply(target) { + target[option.key] = option.value; + }, + }; + }, + }; +} + +export function intFlag(flag, key, options) { + return createAssignedValueFlag((argv, index, args) => { + const option = consumeIntFlag(argv, index, flag, args[key], options); + return option ? { ...option, key } : null; + }); +} + +export function floatFlag(flag, key, options) { + return createAssignedValueFlag((argv, index, args) => { + const option = consumeFloatFlag(argv, index, flag, args[key], options); + return option ? { ...option, key } : null; + }); +} + +export function booleanFlag(flag, key, value = true) { + return { + consume(argv, index) { + if (argv[index] !== flag) { + return null; + } + return { + nextIndex: index, + apply(target) { + target[key] = value; + }, + }; + }, + }; +} + +export function parseFlagArgs(argv, args, specs, options = {}) { + for (let i = 0; i < argv.length; i += 1) { + const arg = argv[i]; + if (arg === "--" && options.ignoreDoubleDash) { + continue; + } + let handled = false; + for (const spec of specs) { + const option = spec.consume(argv, i, args); + if (!option) { + continue; + } + option.apply(args); + i = option.nextIndex; + handled = true; + break; + } + if (handled) { + continue; + } + const fallbackResult = options.onUnhandledArg?.(arg, args); + if (fallbackResult === "handled") { + continue; + } + if (!options.allowUnknownOptions && arg.startsWith("-")) { + throw new Error(`Unknown option: ${arg}`); + } + } + return args; +} diff --git a/scripts/lib/bundled-plugin-source-utils.mjs b/scripts/lib/bundled-plugin-source-utils.mjs new file mode 100644 index 00000000000..89907863cf1 --- /dev/null +++ b/scripts/lib/bundled-plugin-source-utils.mjs @@ -0,0 +1,51 @@ +import fs from "node:fs"; +import path from "node:path"; + +export function readIfExists(filePath) { + try { + return fs.readFileSync(filePath, "utf8"); + } catch { + return null; + } +} + +export function collectBundledPluginSources(params = {}) { + const repoRoot = path.resolve(params.repoRoot ?? process.cwd()); + const extensionsRoot = path.join(repoRoot, "extensions"); + if (!fs.existsSync(extensionsRoot)) { + return []; + } + + const requirePackageJson = params.requirePackageJson === true; + const entries = []; + for (const dirent of fs.readdirSync(extensionsRoot, { withFileTypes: true })) { + if (!dirent.isDirectory()) { + continue; + } + + const pluginDir = path.join(extensionsRoot, dirent.name); + const manifestPath = path.join(pluginDir, "openclaw.plugin.json"); + const packageJsonPath = path.join(pluginDir, "package.json"); + if (!fs.existsSync(manifestPath)) { + continue; + } + if (requirePackageJson && !fs.existsSync(packageJsonPath)) { + continue; + } + + entries.push({ + dirName: dirent.name, + pluginDir, + manifestPath, + manifest: JSON.parse(fs.readFileSync(manifestPath, "utf8")), + ...(fs.existsSync(packageJsonPath) + ? { + packageJsonPath, + packageJson: JSON.parse(fs.readFileSync(packageJsonPath, "utf8")), + } + : {}), + }); + } + + return entries.toSorted((left, right) => left.dirName.localeCompare(right.dirName)); +} diff --git a/scripts/lib/generated-output-utils.mjs b/scripts/lib/generated-output-utils.mjs new file mode 100644 index 00000000000..0daf0ba0790 --- /dev/null +++ b/scripts/lib/generated-output-utils.mjs @@ -0,0 +1,45 @@ +import path from "node:path"; +import { pathToFileURL } from "node:url"; +import { writeTextFileIfChanged } from "../runtime-postbuild-shared.mjs"; +import { readIfExists } from "./bundled-plugin-source-utils.mjs"; + +export function writeGeneratedOutput(params) { + const outputPath = path.resolve(params.repoRoot, params.outputPath); + const current = readIfExists(outputPath); + const changed = current !== params.next; + + if (params.check) { + return { + changed, + wrote: false, + outputPath, + }; + } + + return { + changed, + wrote: writeTextFileIfChanged(outputPath, params.next), + outputPath, + }; +} + +export function reportGeneratedOutputCli(params) { + if (params.importMetaUrl !== pathToFileURL(process.argv[1] ?? "").href) { + return; + } + + const check = process.argv.includes("--check"); + const result = params.run({ check }); + if (!result.changed) { + return; + } + + const relativeOutputPath = path.relative(process.cwd(), result.outputPath); + if (check) { + console.error(`[${params.label}] stale generated output at ${relativeOutputPath}`); + process.exitCode = 1; + return; + } + + console.log(`[${params.label}] wrote ${relativeOutputPath}`); +} diff --git a/scripts/lib/guard-inventory-utils.mjs b/scripts/lib/guard-inventory-utils.mjs new file mode 100644 index 00000000000..9be27412bdf --- /dev/null +++ b/scripts/lib/guard-inventory-utils.mjs @@ -0,0 +1,128 @@ +import { promises as fs } from "node:fs"; +import path from "node:path"; + +export function normalizeRepoPath(repoRoot, filePath) { + return path.relative(repoRoot, filePath).split(path.sep).join("/"); +} + +export function resolveRepoSpecifier(repoRoot, specifier, importerFile) { + if (specifier.startsWith(".")) { + return normalizeRepoPath(repoRoot, path.resolve(path.dirname(importerFile), specifier)); + } + if (specifier.startsWith("/")) { + return normalizeRepoPath(repoRoot, specifier); + } + return null; +} + +export function visitModuleSpecifiers(ts, sourceFile, visit) { + function walk(node) { + if (ts.isImportDeclaration(node) && ts.isStringLiteral(node.moduleSpecifier)) { + visit({ + kind: "import", + node, + specifier: node.moduleSpecifier.text, + specifierNode: node.moduleSpecifier, + }); + } else if ( + ts.isExportDeclaration(node) && + node.moduleSpecifier && + ts.isStringLiteral(node.moduleSpecifier) + ) { + visit({ + kind: "export", + node, + specifier: node.moduleSpecifier.text, + specifierNode: node.moduleSpecifier, + }); + } else if ( + ts.isCallExpression(node) && + node.expression.kind === ts.SyntaxKind.ImportKeyword && + node.arguments.length === 1 && + ts.isStringLiteral(node.arguments[0]) + ) { + visit({ + kind: "dynamic-import", + node, + specifier: node.arguments[0].text, + specifierNode: node.arguments[0], + }); + } + + ts.forEachChild(node, walk); + } + + walk(sourceFile); +} + +export function diffInventoryEntries(expected, actual, compareEntries) { + const expectedKeys = new Set(expected.map((entry) => JSON.stringify(entry))); + const actualKeys = new Set(actual.map((entry) => JSON.stringify(entry))); + return { + missing: expected + .filter((entry) => !actualKeys.has(JSON.stringify(entry))) + .toSorted(compareEntries), + unexpected: actual + .filter((entry) => !expectedKeys.has(JSON.stringify(entry))) + .toSorted(compareEntries), + }; +} + +export function writeLine(stream, text) { + stream.write(`${text}\n`); +} + +export async function collectTypeScriptInventory(params) { + const inventory = []; + + for (const filePath of params.files) { + const source = await fs.readFile(filePath, "utf8"); + const sourceFile = params.ts.createSourceFile( + filePath, + source, + params.ts.ScriptTarget.Latest, + true, + params.scriptKind ?? params.ts.ScriptKind.TS, + ); + inventory.push(...params.collectEntries(sourceFile, filePath)); + } + + return inventory.toSorted(params.compareEntries); +} + +export async function runBaselineInventoryCheck(params) { + const streams = params.io ?? { stdout: process.stdout, stderr: process.stderr }; + const json = params.argv.includes("--json"); + const actual = await params.collectActual(); + const expected = await params.readExpected(); + const { missing, unexpected } = params.diffInventory(expected, actual); + const matchesBaseline = missing.length === 0 && unexpected.length === 0; + + if (json) { + writeLine(streams.stdout, JSON.stringify(actual, null, 2)); + } else { + writeLine(streams.stdout, params.formatInventoryHuman(actual)); + writeLine( + streams.stdout, + matchesBaseline + ? `Baseline matches (${actual.length} entries).` + : `Baseline mismatch (${unexpected.length} unexpected, ${missing.length} missing).`, + ); + if (!matchesBaseline) { + if (unexpected.length > 0) { + writeLine(streams.stderr, "Unexpected entries:"); + for (const entry of unexpected) { + writeLine(streams.stderr, `- ${params.formatEntry(entry)}`); + } + } + if (missing.length > 0) { + writeLine(streams.stderr, "Missing baseline entries:"); + for (const entry of missing) { + writeLine(streams.stderr, `- ${params.formatEntry(entry)}`); + } + } + } + } + + return matchesBaseline ? 0 : 1; +} diff --git a/scripts/lib/ts-guard-utils.mjs b/scripts/lib/ts-guard-utils.mjs index e9130e4ae22..8dd8e397233 100644 --- a/scripts/lib/ts-guard-utils.mjs +++ b/scripts/lib/ts-guard-utils.mjs @@ -148,6 +148,21 @@ export function unwrapExpression(expression) { } } +export function collectCallExpressionLines(ts, sourceFile, resolveLineNode) { + const lines = []; + const visit = (node) => { + if (ts.isCallExpression(node)) { + const lineNode = resolveLineNode(node); + if (lineNode) { + lines.push(toLine(sourceFile, lineNode)); + } + } + ts.forEachChild(node, visit); + }; + visit(sourceFile); + return lines; +} + export function isDirectExecution(importMetaUrl) { const entry = process.argv[1]; if (!entry) { diff --git a/scripts/lib/vitest-report-cli-utils.mjs b/scripts/lib/vitest-report-cli-utils.mjs new file mode 100644 index 00000000000..b3e1807ba26 --- /dev/null +++ b/scripts/lib/vitest-report-cli-utils.mjs @@ -0,0 +1,31 @@ +import { readJsonFile, runVitestJsonReport } from "../test-report-utils.mjs"; +import { intFlag, parseFlagArgs, stringFlag } from "./arg-utils.mjs"; + +export function parseVitestReportArgs(argv, defaults) { + return parseFlagArgs( + argv, + { + config: defaults.config, + limit: defaults.limit, + reportPath: defaults.reportPath ?? "", + }, + [ + stringFlag("--config", "config"), + intFlag("--limit", "limit", { min: 1 }), + stringFlag("--report", "reportPath"), + ], + ); +} + +export function loadVitestReportFromArgs(args, prefix) { + const reportPath = runVitestJsonReport({ + config: args.config, + reportPath: args.reportPath, + prefix, + }); + return readJsonFile(reportPath); +} + +export function formatMs(value, digits = 1) { + return `${value.toFixed(digits)}ms`; +} diff --git a/scripts/test-find-thread-candidates.mjs b/scripts/test-find-thread-candidates.mjs index d1d63e56c26..d2c6a2f4c64 100644 --- a/scripts/test-find-thread-candidates.mjs +++ b/scripts/test-find-thread-candidates.mjs @@ -1,81 +1,49 @@ import { spawnSync } from "node:child_process"; import path from "node:path"; import { pathToFileURL } from "node:url"; +import { + booleanFlag, + floatFlag, + intFlag, + parseFlagArgs, + readEnvNumber, + stringFlag, +} from "./lib/arg-utils.mjs"; +import { formatMs } from "./lib/vitest-report-cli-utils.mjs"; import { loadTestRunnerBehavior, loadUnitTimingManifest } from "./test-runner-manifest.mjs"; -function readEnvNumber(name) { - const raw = process.env[name]?.trim(); - if (!raw) { - return null; - } - const parsed = Number.parseFloat(raw); - return Number.isFinite(parsed) ? parsed : null; -} - export function parseArgs(argv) { - const args = { - config: "vitest.unit.config.ts", - limit: Number.isFinite(readEnvNumber("OPENCLAW_TEST_THREAD_CANDIDATE_LIMIT")) - ? Math.max(1, Math.floor(readEnvNumber("OPENCLAW_TEST_THREAD_CANDIDATE_LIMIT"))) - : 20, - minDurationMs: readEnvNumber("OPENCLAW_TEST_THREAD_CANDIDATE_MIN_DURATION_MS") ?? 250, - minGainMs: readEnvNumber("OPENCLAW_TEST_THREAD_CANDIDATE_MIN_GAIN_MS") ?? 100, - minGainPct: readEnvNumber("OPENCLAW_TEST_THREAD_CANDIDATE_MIN_GAIN_PCT") ?? 10, - json: false, - files: [], - }; - for (let i = 0; i < argv.length; i += 1) { - const arg = argv[i]; - if (arg === "--") { - continue; - } - if (arg === "--config") { - args.config = argv[i + 1] ?? args.config; - i += 1; - continue; - } - if (arg === "--limit") { - const parsed = Number.parseInt(argv[i + 1] ?? "", 10); - if (Number.isFinite(parsed) && parsed > 0) { - args.limit = parsed; - } - i += 1; - continue; - } - if (arg === "--min-duration-ms") { - const parsed = Number.parseFloat(argv[i + 1] ?? ""); - if (Number.isFinite(parsed) && parsed >= 0) { - args.minDurationMs = parsed; - } - i += 1; - continue; - } - if (arg === "--min-gain-ms") { - const parsed = Number.parseFloat(argv[i + 1] ?? ""); - if (Number.isFinite(parsed) && parsed >= 0) { - args.minGainMs = parsed; - } - i += 1; - continue; - } - if (arg === "--min-gain-pct") { - const parsed = Number.parseFloat(argv[i + 1] ?? ""); - if (Number.isFinite(parsed) && parsed > 0) { - args.minGainPct = parsed; - } - i += 1; - continue; - } - if (arg === "--json") { - args.json = true; - continue; - } - if (arg.startsWith("-")) { - throw new Error(`Unknown option: ${arg}`); - } - args.files.push(arg); - } - return args; + const envLimit = readEnvNumber("OPENCLAW_TEST_THREAD_CANDIDATE_LIMIT"); + return parseFlagArgs( + argv, + { + config: "vitest.unit.config.ts", + limit: Number.isFinite(envLimit) ? Math.max(1, Math.floor(envLimit)) : 20, + minDurationMs: readEnvNumber("OPENCLAW_TEST_THREAD_CANDIDATE_MIN_DURATION_MS") ?? 250, + minGainMs: readEnvNumber("OPENCLAW_TEST_THREAD_CANDIDATE_MIN_GAIN_MS") ?? 100, + minGainPct: readEnvNumber("OPENCLAW_TEST_THREAD_CANDIDATE_MIN_GAIN_PCT") ?? 10, + json: false, + files: [], + }, + [ + stringFlag("--config", "config"), + intFlag("--limit", "limit", { min: 1 }), + floatFlag("--min-duration-ms", "minDurationMs", { min: 0 }), + floatFlag("--min-gain-ms", "minGainMs", { min: 0 }), + floatFlag("--min-gain-pct", "minGainPct", { min: 0, includeMin: false }), + booleanFlag("--json", "json"), + ], + { + ignoreDoubleDash: true, + onUnhandledArg(arg, args) { + if (arg.startsWith("-")) { + throw new Error(`Unknown option: ${arg}`); + } + args.files.push(arg); + return "handled"; + }, + }, + ); } export function getExistingThreadCandidateExclusions(behavior) { @@ -131,10 +99,6 @@ export function summarizeThreadBenchmark({ file, forks, threads, minGainMs, minG }; } -function formatMs(ms) { - return `${ms.toFixed(0)}ms`; -} - function benchmarkFile({ config, file, pool }) { const startedAt = process.hrtime.bigint(); const run = spawnSync("pnpm", ["vitest", "run", "--config", config, `--pool=${pool}`, file], { @@ -203,6 +167,7 @@ async function main() { console.log( `[test-find-thread-candidates] tested=${String(results.length)} minGain=${formatMs( opts.minGainMs, + 0, )} minGainPct=${String(opts.minGainPct)}%`, ); for (const result of results) { @@ -214,30 +179,17 @@ async function main() { ? "threads-failed" : "skip"; console.log( - `${status.padEnd(14, " ")} ${result.file} forks=${formatMs(result.forks.elapsedMs)} threads=${formatMs( - result.threads.elapsedMs, - )} gain=${formatMs(result.gainMs)} (${result.gainPct.toFixed(1)}%)`, + `${status.padEnd(14, " ")} ${result.file} forks=${formatMs( + result.forks.elapsedMs, + 0, + )} threads=${formatMs(result.threads.elapsedMs, 0)} gain=${formatMs(result.gainMs, 0)} (${result.gainPct.toFixed(1)}%)`, ); - if (result.threads.exitCode !== 0) { - const firstErrorLine = - result.threads.stderr - .split(/\r?\n/u) - .find( - (line) => line.includes("Error") || line.includes("TypeError") || line.includes("FAIL"), - ) ?? "threads failed"; - console.log(` ${firstErrorLine}`); - } } } -const isMain = - process.argv[1] && pathToFileURL(path.resolve(process.argv[1])).href === import.meta.url; - -if (isMain) { - try { - await main(); - } catch (error) { - console.error(error instanceof Error ? error.message : String(error)); +if (process.argv[1] && pathToFileURL(path.resolve(process.argv[1])).href === import.meta.url) { + main().catch((error) => { + console.error(error); process.exit(1); - } + }); } diff --git a/scripts/test-hotspots.mjs b/scripts/test-hotspots.mjs index 66d56c11cd8..cb51c385ccb 100644 --- a/scripts/test-hotspots.mjs +++ b/scripts/test-hotspots.mjs @@ -1,50 +1,15 @@ import { - collectVitestFileDurations, - readJsonFile, - runVitestJsonReport, -} from "./test-report-utils.mjs"; + formatMs, + loadVitestReportFromArgs, + parseVitestReportArgs, +} from "./lib/vitest-report-cli-utils.mjs"; +import { collectVitestFileDurations } from "./test-report-utils.mjs"; -function parseArgs(argv) { - const args = { - config: "vitest.unit.config.ts", - limit: 20, - reportPath: "", - }; - for (let i = 0; i < argv.length; i += 1) { - const arg = argv[i]; - if (arg === "--config") { - args.config = argv[i + 1] ?? args.config; - i += 1; - continue; - } - if (arg === "--limit") { - const parsed = Number.parseInt(argv[i + 1] ?? "", 10); - if (Number.isFinite(parsed) && parsed > 0) { - args.limit = parsed; - } - i += 1; - continue; - } - if (arg === "--report") { - args.reportPath = argv[i + 1] ?? ""; - i += 1; - continue; - } - } - return args; -} - -function formatMs(value) { - return `${value.toFixed(1)}ms`; -} - -const opts = parseArgs(process.argv.slice(2)); -const reportPath = runVitestJsonReport({ - config: opts.config, - reportPath: opts.reportPath, - prefix: "openclaw-vitest-hotspots", +const opts = parseVitestReportArgs(process.argv.slice(2), { + config: "vitest.unit.config.ts", + limit: 20, }); -const report = readJsonFile(reportPath); +const report = loadVitestReportFromArgs(opts, "openclaw-vitest-hotspots"); const fileResults = collectVitestFileDurations(report).toSorted( (a, b) => b.durationMs - a.durationMs, ); diff --git a/scripts/test-perf-budget.mjs b/scripts/test-perf-budget.mjs index 49aead698c4..1e6228776c0 100644 --- a/scripts/test-perf-budget.mjs +++ b/scripts/test-perf-budget.mjs @@ -1,58 +1,23 @@ +import { floatFlag, parseFlagArgs, readEnvNumber, stringFlag } from "./lib/arg-utils.mjs"; +import { formatMs } from "./lib/vitest-report-cli-utils.mjs"; import { readJsonFile, runVitestJsonReport } from "./test-report-utils.mjs"; -function readEnvNumber(name) { - const raw = process.env[name]?.trim(); - if (!raw) { - return null; - } - const parsed = Number.parseFloat(raw); - return Number.isFinite(parsed) ? parsed : null; -} - function parseArgs(argv) { - const args = { - config: "vitest.unit.config.ts", - maxWallMs: readEnvNumber("OPENCLAW_TEST_PERF_MAX_WALL_MS"), - baselineWallMs: readEnvNumber("OPENCLAW_TEST_PERF_BASELINE_WALL_MS"), - maxRegressionPct: readEnvNumber("OPENCLAW_TEST_PERF_MAX_REGRESSION_PCT") ?? 10, - }; - for (let i = 0; i < argv.length; i += 1) { - const arg = argv[i]; - if (arg === "--config") { - args.config = argv[i + 1] ?? args.config; - i += 1; - continue; - } - if (arg === "--max-wall-ms") { - const parsed = Number.parseFloat(argv[i + 1] ?? ""); - if (Number.isFinite(parsed)) { - args.maxWallMs = parsed; - } - i += 1; - continue; - } - if (arg === "--baseline-wall-ms") { - const parsed = Number.parseFloat(argv[i + 1] ?? ""); - if (Number.isFinite(parsed)) { - args.baselineWallMs = parsed; - } - i += 1; - continue; - } - if (arg === "--max-regression-pct") { - const parsed = Number.parseFloat(argv[i + 1] ?? ""); - if (Number.isFinite(parsed)) { - args.maxRegressionPct = parsed; - } - i += 1; - continue; - } - } - return args; -} - -function formatMs(ms) { - return `${ms.toFixed(1)}ms`; + return parseFlagArgs( + argv, + { + config: "vitest.unit.config.ts", + maxWallMs: readEnvNumber("OPENCLAW_TEST_PERF_MAX_WALL_MS"), + baselineWallMs: readEnvNumber("OPENCLAW_TEST_PERF_BASELINE_WALL_MS"), + maxRegressionPct: readEnvNumber("OPENCLAW_TEST_PERF_MAX_REGRESSION_PCT") ?? 10, + }, + [ + stringFlag("--config", "config"), + floatFlag("--max-wall-ms", "maxWallMs"), + floatFlag("--baseline-wall-ms", "baselineWallMs"), + floatFlag("--max-regression-pct", "maxRegressionPct"), + ], + ); } const opts = parseArgs(process.argv.slice(2)); diff --git a/scripts/test-runner-manifest.mjs b/scripts/test-runner-manifest.mjs index f33b4193c24..11ceab6b787 100644 --- a/scripts/test-runner-manifest.mjs +++ b/scripts/test-runner-manifest.mjs @@ -261,24 +261,14 @@ export function packFilesByDuration(files, bucketCount, estimateDurationMs) { return []; } - const buckets = Array.from({ length: Math.min(normalizedBucketCount, files.length) }, () => ({ - totalMs: 0, - files: [], - })); - - const sortedFiles = [...files].toSorted((left, right) => { - return estimateDurationMs(right) - estimateDurationMs(left); - }); - - for (const file of sortedFiles) { - const bucket = buckets.reduce((lightest, current) => - current.totalMs < lightest.totalMs ? current : lightest, - ); - bucket.files.push(file); - bucket.totalMs += estimateDurationMs(file); - } - - return buckets.map((bucket) => bucket.files).filter((bucket) => bucket.length > 0); + return packFilesIntoDurationBuckets( + files, + Array.from({ length: Math.min(normalizedBucketCount, files.length) }, () => ({ + totalMs: 0, + files: [], + })), + estimateDurationMs, + ).filter((bucket) => bucket.length > 0); } export function packFilesByDurationWithBaseLoads( @@ -292,14 +282,20 @@ export function packFilesByDurationWithBaseLoads( return []; } - const buckets = Array.from({ length: normalizedBucketCount }, (_, index) => ({ - totalMs: - Number.isFinite(baseLoadsMs[index]) && baseLoadsMs[index] >= 0 - ? Math.round(baseLoadsMs[index]) - : 0, - files: [], - })); + return packFilesIntoDurationBuckets( + files, + Array.from({ length: normalizedBucketCount }, (_, index) => ({ + totalMs: + Number.isFinite(baseLoadsMs[index]) && baseLoadsMs[index] >= 0 + ? Math.round(baseLoadsMs[index]) + : 0, + files: [], + })), + estimateDurationMs, + ); +} +function packFilesIntoDurationBuckets(files, buckets, estimateDurationMs) { const sortedFiles = [...files].toSorted((left, right) => { return estimateDurationMs(right) - estimateDurationMs(left); }); diff --git a/scripts/test-update-memory-hotspots.mjs b/scripts/test-update-memory-hotspots.mjs index ecc38266941..564af648d4d 100644 --- a/scripts/test-update-memory-hotspots.mjs +++ b/scripts/test-update-memory-hotspots.mjs @@ -1,71 +1,33 @@ import fs from "node:fs"; import path from "node:path"; +import { intFlag, parseFlagArgs, stringFlag, stringListFlag } from "./lib/arg-utils.mjs"; import { parseMemoryTraceSummaryLines } from "./test-parallel-memory.mjs"; import { normalizeTrackedRepoPath, tryReadJsonFile, writeJsonFile } from "./test-report-utils.mjs"; import { unitMemoryHotspotManifestPath } from "./test-runner-manifest.mjs"; import { matchesHotspotSummaryLane } from "./test-update-memory-hotspots-utils.mjs"; function parseArgs(argv) { - const args = { - config: "vitest.unit.config.ts", - out: unitMemoryHotspotManifestPath, - lane: "unit-fast", - lanePrefixes: [], - logs: [], - minDeltaKb: 256 * 1024, - limit: 64, - }; - for (let i = 0; i < argv.length; i += 1) { - const arg = argv[i]; - if (arg === "--config") { - args.config = argv[i + 1] ?? args.config; - i += 1; - continue; - } - if (arg === "--out") { - args.out = argv[i + 1] ?? args.out; - i += 1; - continue; - } - if (arg === "--lane") { - args.lane = argv[i + 1] ?? args.lane; - i += 1; - continue; - } - if (arg === "--lane-prefix") { - const lanePrefix = argv[i + 1]; - if (typeof lanePrefix === "string" && lanePrefix.length > 0) { - args.lanePrefixes.push(lanePrefix); - } - i += 1; - continue; - } - if (arg === "--log") { - const logPath = argv[i + 1]; - if (typeof logPath === "string" && logPath.length > 0) { - args.logs.push(logPath); - } - i += 1; - continue; - } - if (arg === "--min-delta-kb") { - const parsed = Number.parseInt(argv[i + 1] ?? "", 10); - if (Number.isFinite(parsed) && parsed > 0) { - args.minDeltaKb = parsed; - } - i += 1; - continue; - } - if (arg === "--limit") { - const parsed = Number.parseInt(argv[i + 1] ?? "", 10); - if (Number.isFinite(parsed) && parsed > 0) { - args.limit = parsed; - } - i += 1; - continue; - } - } - return args; + return parseFlagArgs( + argv, + { + config: "vitest.unit.config.ts", + out: unitMemoryHotspotManifestPath, + lane: "unit-fast", + lanePrefixes: [], + logs: [], + minDeltaKb: 256 * 1024, + limit: 64, + }, + [ + stringFlag("--config", "config"), + stringFlag("--out", "out"), + stringFlag("--lane", "lane"), + stringListFlag("--lane-prefix", "lanePrefixes"), + stringListFlag("--log", "logs"), + intFlag("--min-delta-kb", "minDeltaKb", { min: 1 }), + intFlag("--limit", "limit", { min: 1 }), + ], + ); } function mergeHotspotEntry(aggregated, file, value) { diff --git a/scripts/test-update-timings.mjs b/scripts/test-update-timings.mjs index b3fa815c6d9..c39b3a0d450 100644 --- a/scripts/test-update-timings.mjs +++ b/scripts/test-update-timings.mjs @@ -1,64 +1,30 @@ +import { intFlag, parseFlagArgs, stringFlag } from "./lib/arg-utils.mjs"; +import { loadVitestReportFromArgs, parseVitestReportArgs } from "./lib/vitest-report-cli-utils.mjs"; import { collectVitestFileDurations, normalizeTrackedRepoPath, - readJsonFile, - runVitestJsonReport, writeJsonFile, } from "./test-report-utils.mjs"; import { unitTimingManifestPath } from "./test-runner-manifest.mjs"; function parseArgs(argv) { - const args = { - config: "vitest.unit.config.ts", - out: unitTimingManifestPath, - reportPath: "", - limit: 256, - defaultDurationMs: 250, - }; - for (let i = 0; i < argv.length; i += 1) { - const arg = argv[i]; - if (arg === "--config") { - args.config = argv[i + 1] ?? args.config; - i += 1; - continue; - } - if (arg === "--out") { - args.out = argv[i + 1] ?? args.out; - i += 1; - continue; - } - if (arg === "--report") { - args.reportPath = argv[i + 1] ?? ""; - i += 1; - continue; - } - if (arg === "--limit") { - const parsed = Number.parseInt(argv[i + 1] ?? "", 10); - if (Number.isFinite(parsed) && parsed > 0) { - args.limit = parsed; - } - i += 1; - continue; - } - if (arg === "--default-duration-ms") { - const parsed = Number.parseInt(argv[i + 1] ?? "", 10); - if (Number.isFinite(parsed) && parsed > 0) { - args.defaultDurationMs = parsed; - } - i += 1; - continue; - } - } - return args; + return parseFlagArgs( + argv, + { + ...parseVitestReportArgs(argv, { + config: "vitest.unit.config.ts", + limit: 256, + reportPath: "", + }), + out: unitTimingManifestPath, + defaultDurationMs: 250, + }, + [stringFlag("--out", "out"), intFlag("--default-duration-ms", "defaultDurationMs", { min: 1 })], + ); } const opts = parseArgs(process.argv.slice(2)); -const reportPath = runVitestJsonReport({ - config: opts.config, - reportPath: opts.reportPath, - prefix: "openclaw-vitest-timings", -}); -const report = readJsonFile(reportPath); +const report = loadVitestReportFromArgs(opts, "openclaw-vitest-timings"); const files = Object.fromEntries( collectVitestFileDurations(report, normalizeTrackedRepoPath) .toSorted((a, b) => b.durationMs - a.durationMs) diff --git a/src/agents/pi-embedded-runner/run/attempt.spawn-workspace.context-engine.test.ts b/src/agents/pi-embedded-runner/run/attempt.spawn-workspace.context-engine.test.ts index 6bb54eaf4e5..80ad7a78056 100644 --- a/src/agents/pi-embedded-runner/run/attempt.spawn-workspace.context-engine.test.ts +++ b/src/agents/pi-embedded-runner/run/attempt.spawn-workspace.context-engine.test.ts @@ -13,6 +13,10 @@ import { } from "./attempt.spawn-workspace.test-support.js"; const hoisted = getHoisted(); +const embeddedSessionId = "embedded-session"; +const sessionFile = "/tmp/session.jsonl"; +const seedMessage = { role: "user", content: "seed", timestamp: 1 } as AgentMessage; +const doneMessage = { role: "assistant", content: "done", timestamp: 2 } as unknown as AgentMessage; function createTestContextEngine(params: Partial): AttemptContextEngine { return { @@ -31,6 +35,65 @@ function createTestContextEngine(params: Partial): Attempt } as AttemptContextEngine; } +async function runBootstrap( + sessionKey: string, + contextEngine: AttemptContextEngine, + overrides: Partial[0]> = {}, +) { + await runAttemptContextEngineBootstrap({ + hadSessionFile: true, + contextEngine, + sessionId: embeddedSessionId, + sessionKey, + sessionFile, + sessionManager: hoisted.sessionManager, + runtimeContext: {}, + runMaintenance: hoisted.runContextEngineMaintenanceMock, + warn: () => {}, + ...overrides, + }); +} + +async function runAssemble( + sessionKey: string, + contextEngine: AttemptContextEngine, + overrides: Partial[0]> = {}, +) { + await assembleAttemptContextEngine({ + contextEngine, + sessionId: embeddedSessionId, + sessionKey, + messages: [seedMessage], + tokenBudget: 2048, + modelId: "gpt-test", + ...overrides, + }); +} + +async function finalizeTurn( + sessionKey: string, + contextEngine: AttemptContextEngine, + overrides: Partial[0]> = {}, +) { + await finalizeAttemptContextEngineTurn({ + contextEngine, + promptError: false, + aborted: false, + yieldAborted: false, + sessionIdUsed: embeddedSessionId, + sessionKey, + sessionFile, + messagesSnapshot: [doneMessage], + prePromptMessageCount: 0, + tokenBudget: 2048, + runtimeContext: {}, + runMaintenance: hoisted.runContextEngineMaintenanceMock, + sessionManager: hoisted.sessionManager, + warn: () => {}, + ...overrides, + }); +} + describe("runEmbeddedAttempt context engine sessionKey forwarding", () => { const sessionKey = "agent:main:discord:channel:test-ctx-engine"; @@ -47,43 +110,9 @@ describe("runEmbeddedAttempt context engine sessionKey forwarding", () => { afterTurn, }); - await runAttemptContextEngineBootstrap({ - hadSessionFile: true, - contextEngine, - sessionId: "embedded-session", - sessionKey, - sessionFile: "/tmp/session.jsonl", - sessionManager: hoisted.sessionManager, - runtimeContext: {}, - runMaintenance: hoisted.runContextEngineMaintenanceMock, - warn: () => {}, - }); - await assembleAttemptContextEngine({ - contextEngine, - sessionId: "embedded-session", - sessionKey, - messages: [{ role: "user", content: "seed", timestamp: 1 } as AgentMessage], - tokenBudget: 2048, - modelId: "gpt-test", - }); - await finalizeAttemptContextEngineTurn({ - contextEngine, - promptError: false, - aborted: false, - yieldAborted: false, - sessionIdUsed: "embedded-session", - sessionKey, - sessionFile: "/tmp/session.jsonl", - messagesSnapshot: [ - { role: "assistant", content: "done", timestamp: 2 } as unknown as AgentMessage, - ], - prePromptMessageCount: 0, - tokenBudget: 2048, - runtimeContext: {}, - runMaintenance: hoisted.runContextEngineMaintenanceMock, - sessionManager: hoisted.sessionManager, - warn: () => {}, - }); + await runBootstrap(sessionKey, contextEngine); + await runAssemble(sessionKey, contextEngine); + await finalizeTurn(sessionKey, contextEngine); expectCalledWithSessionKey(bootstrap, sessionKey); expectCalledWithSessionKey(assemble, sessionKey); @@ -94,25 +123,8 @@ describe("runEmbeddedAttempt context engine sessionKey forwarding", () => { const { bootstrap, assemble } = createContextEngineBootstrapAndAssemble(); const contextEngine = createTestContextEngine({ bootstrap, assemble }); - await runAttemptContextEngineBootstrap({ - hadSessionFile: true, - contextEngine, - sessionId: "embedded-session", - sessionKey, - sessionFile: "/tmp/session.jsonl", - sessionManager: hoisted.sessionManager, - runtimeContext: {}, - runMaintenance: hoisted.runContextEngineMaintenanceMock, - warn: () => {}, - }); - await assembleAttemptContextEngine({ - contextEngine, - sessionId: "embedded-session", - sessionKey, - messages: [{ role: "user", content: "seed", timestamp: 1 } as AgentMessage], - tokenBudget: 2048, - modelId: "gpt-test", - }); + await runBootstrap(sessionKey, contextEngine); + await runAssemble(sessionKey, contextEngine); expect(assemble).toHaveBeenCalledWith( expect.objectContaining({ @@ -127,28 +139,9 @@ describe("runEmbeddedAttempt context engine sessionKey forwarding", () => { async (_params: { sessionKey?: string; messages: AgentMessage[] }) => ({ ingestedCount: 1 }), ); - await finalizeAttemptContextEngineTurn({ - contextEngine: createTestContextEngine({ - bootstrap, - assemble, - ingestBatch, - }), - promptError: false, - aborted: false, - yieldAborted: false, - sessionIdUsed: "embedded-session", - sessionKey, - sessionFile: "/tmp/session.jsonl", - messagesSnapshot: [ - { role: "user", content: "seed", timestamp: 1 } as AgentMessage, - { role: "assistant", content: "done", timestamp: 2 } as unknown as AgentMessage, - ], + await finalizeTurn(sessionKey, createTestContextEngine({ bootstrap, assemble, ingestBatch }), { + messagesSnapshot: [seedMessage, doneMessage], prePromptMessageCount: 1, - tokenBudget: 2048, - runtimeContext: {}, - runMaintenance: hoisted.runContextEngineMaintenanceMock, - sessionManager: hoisted.sessionManager, - warn: () => {}, }); expectCalledWithSessionKey(ingestBatch, sessionKey); @@ -160,28 +153,9 @@ describe("runEmbeddedAttempt context engine sessionKey forwarding", () => { ingested: true, })); - await finalizeAttemptContextEngineTurn({ - contextEngine: createTestContextEngine({ - bootstrap, - assemble, - ingest, - }), - promptError: false, - aborted: false, - yieldAborted: false, - sessionIdUsed: "embedded-session", - sessionKey, - sessionFile: "/tmp/session.jsonl", - messagesSnapshot: [ - { role: "user", content: "seed", timestamp: 1 } as AgentMessage, - { role: "assistant", content: "done", timestamp: 2 } as unknown as AgentMessage, - ], + await finalizeTurn(sessionKey, createTestContextEngine({ bootstrap, assemble, ingest }), { + messagesSnapshot: [seedMessage, doneMessage], prePromptMessageCount: 1, - tokenBudget: 2048, - runtimeContext: {}, - runMaintenance: hoisted.runContextEngineMaintenanceMock, - sessionManager: hoisted.sessionManager, - warn: () => {}, }); expect(ingest).toHaveBeenCalled(); @@ -199,28 +173,7 @@ describe("runEmbeddedAttempt context engine sessionKey forwarding", () => { throw new Error("afterTurn failed"); }); - await finalizeAttemptContextEngineTurn({ - contextEngine: createTestContextEngine({ - bootstrap, - assemble, - afterTurn, - }), - promptError: false, - aborted: false, - yieldAborted: false, - sessionIdUsed: "embedded-session", - sessionKey, - sessionFile: "/tmp/session.jsonl", - messagesSnapshot: [ - { role: "assistant", content: "done", timestamp: 2 } as unknown as AgentMessage, - ], - prePromptMessageCount: 0, - tokenBudget: 2048, - runtimeContext: {}, - runMaintenance: hoisted.runContextEngineMaintenanceMock, - sessionManager: hoisted.sessionManager, - warn: () => {}, - }); + await finalizeTurn(sessionKey, createTestContextEngine({ bootstrap, assemble, afterTurn })); expect(afterTurn).toHaveBeenCalled(); expect(hoisted.runContextEngineMaintenanceMock).not.toHaveBeenCalledWith( @@ -231,9 +184,9 @@ describe("runEmbeddedAttempt context engine sessionKey forwarding", () => { it("runs startup maintenance for existing sessions even without bootstrap()", async () => { const { assemble } = createContextEngineBootstrapAndAssemble(); - await runAttemptContextEngineBootstrap({ - hadSessionFile: true, - contextEngine: createTestContextEngine({ + await runBootstrap( + sessionKey, + createTestContextEngine({ assemble, maintain: async () => ({ changed: false, @@ -242,14 +195,7 @@ describe("runEmbeddedAttempt context engine sessionKey forwarding", () => { reason: "test maintenance", }), }), - sessionId: "embedded-session", - sessionKey, - sessionFile: "/tmp/session.jsonl", - sessionManager: hoisted.sessionManager, - runtimeContext: {}, - runMaintenance: hoisted.runContextEngineMaintenanceMock, - warn: () => {}, - }); + ); expect(hoisted.runContextEngineMaintenanceMock).toHaveBeenCalledWith( expect.objectContaining({ reason: "bootstrap" }), @@ -262,28 +208,9 @@ describe("runEmbeddedAttempt context engine sessionKey forwarding", () => { throw new Error("ingestBatch failed"); }); - await finalizeAttemptContextEngineTurn({ - contextEngine: createTestContextEngine({ - bootstrap, - assemble, - ingestBatch, - }), - promptError: false, - aborted: false, - yieldAborted: false, - sessionIdUsed: "embedded-session", - sessionKey, - sessionFile: "/tmp/session.jsonl", - messagesSnapshot: [ - { role: "user", content: "seed", timestamp: 1 } as AgentMessage, - { role: "assistant", content: "done", timestamp: 2 } as unknown as AgentMessage, - ], + await finalizeTurn(sessionKey, createTestContextEngine({ bootstrap, assemble, ingestBatch }), { + messagesSnapshot: [seedMessage, doneMessage], prePromptMessageCount: 1, - tokenBudget: 2048, - runtimeContext: {}, - runMaintenance: hoisted.runContextEngineMaintenanceMock, - sessionManager: hoisted.sessionManager, - warn: () => {}, }); expect(ingestBatch).toHaveBeenCalled(); diff --git a/src/agents/pi-embedded-runner/transcript-rewrite.test.ts b/src/agents/pi-embedded-runner/transcript-rewrite.test.ts index d919e7af476..021475eb9c5 100644 --- a/src/agents/pi-embedded-runner/transcript-rewrite.test.ts +++ b/src/agents/pi-embedded-runner/transcript-rewrite.test.ts @@ -40,6 +40,102 @@ function getBranchMessages(sessionManager: SessionManager): AgentMessage[] { .map((entry) => entry.message); } +function appendSessionMessages( + sessionManager: SessionManager, + messages: AppendMessage[], +): string[] { + return messages.map((message) => sessionManager.appendMessage(message)); +} + +function createTextContent(text: string) { + return [{ type: "text", text }]; +} + +function createReadRewriteSession(options?: { tailAssistantText?: string }) { + const sessionManager = SessionManager.inMemory(); + const entryIds = appendSessionMessages(sessionManager, [ + asAppendMessage({ + role: "user", + content: "read file", + timestamp: 1, + }), + asAppendMessage({ + role: "assistant", + content: [{ type: "toolCall", id: "call_1", name: "read", arguments: {} }], + timestamp: 2, + }), + asAppendMessage({ + role: "toolResult", + toolCallId: "call_1", + toolName: "read", + content: createTextContent("x".repeat(8_000)), + isError: false, + timestamp: 3, + }), + asAppendMessage({ + role: "assistant", + content: createTextContent(options?.tailAssistantText ?? "summarized"), + timestamp: 4, + }), + ]); + return { + sessionManager, + toolResultEntryId: entryIds[2], + tailAssistantEntryId: entryIds[3], + }; +} + +function createExecRewriteSession() { + const sessionManager = SessionManager.inMemory(); + const entryIds = appendSessionMessages(sessionManager, [ + asAppendMessage({ + role: "user", + content: "run tool", + timestamp: 1, + }), + asAppendMessage({ + role: "toolResult", + toolCallId: "call_1", + toolName: "exec", + content: createTextContent("before rewrite"), + isError: false, + timestamp: 2, + }), + asAppendMessage({ + role: "assistant", + content: createTextContent("summarized"), + timestamp: 3, + }), + ]); + return { + sessionManager, + toolResultEntryId: entryIds[1], + }; +} + +function createToolResultReplacement(toolName: string, text: string, timestamp: number) { + return { + role: "toolResult", + toolCallId: "call_1", + toolName, + content: createTextContent(text), + isError: false, + timestamp, + } as AgentMessage; +} + +function findAssistantEntryByText(sessionManager: SessionManager, text: string) { + return sessionManager + .getBranch() + .find( + (entry) => + entry.type === "message" && + entry.message.role === "assistant" && + Array.isArray(entry.message.content) && + entry.message.content.some((part) => part.type === "text" && part.text === text), + ); +} + beforeEach(async () => { acquireSessionWriteLockMock.mockClear(); acquireSessionWriteLockReleaseMock.mockClear(); @@ -48,57 +144,14 @@ beforeEach(async () => { describe("rewriteTranscriptEntriesInSessionManager", () => { it("branches from the first replaced message and re-appends the remaining suffix", () => { - const sessionManager = SessionManager.inMemory(); - sessionManager.appendMessage( - asAppendMessage({ - role: "user", - content: "read file", - timestamp: 1, - }), - ); - sessionManager.appendMessage( - asAppendMessage({ - role: "assistant", - content: [{ type: "toolCall", id: "call_1", name: "read", arguments: {} }], - timestamp: 2, - }), - ); - sessionManager.appendMessage( - asAppendMessage({ - role: "toolResult", - toolCallId: "call_1", - toolName: "read", - content: [{ type: "text", text: "x".repeat(8_000) }], - isError: false, - timestamp: 3, - }), - ); - sessionManager.appendMessage( - asAppendMessage({ - role: "assistant", - content: [{ type: "text", text: "summarized" }], - timestamp: 4, - }), - ); - - const toolResultEntry = sessionManager - .getBranch() - .find((entry) => entry.type === "message" && entry.message.role === "toolResult"); - expect(toolResultEntry).toBeDefined(); + const { sessionManager, toolResultEntryId } = createReadRewriteSession(); const result = rewriteTranscriptEntriesInSessionManager({ sessionManager, replacements: [ { - entryId: toolResultEntry!.id, - message: { - role: "toolResult", - toolCallId: "call_1", - toolName: "read", - content: [{ type: "text", text: "[externalized file_123]" }], - isError: false, - timestamp: 3, - }, + entryId: toolResultEntryId, + message: createToolResultReplacement("read", "[externalized file_123]", 3), }, ], }); @@ -123,48 +176,8 @@ describe("rewriteTranscriptEntriesInSessionManager", () => { }); it("preserves active-branch labels after rewritten entries are re-appended", () => { - const sessionManager = SessionManager.inMemory(); - sessionManager.appendMessage( - asAppendMessage({ - role: "user", - content: "read file", - timestamp: 1, - }), - ); - sessionManager.appendMessage( - asAppendMessage({ - role: "assistant", - content: [{ type: "toolCall", id: "call_1", name: "read", arguments: {} }], - timestamp: 2, - }), - ); - const toolResultEntryId = sessionManager.appendMessage( - asAppendMessage({ - role: "toolResult", - toolCallId: "call_1", - toolName: "read", - content: [{ type: "text", text: "x".repeat(8_000) }], - isError: false, - timestamp: 3, - }), - ); - sessionManager.appendMessage( - asAppendMessage({ - role: "assistant", - content: [{ type: "text", text: "summarized" }], - timestamp: 4, - }), - ); - - const summaryEntry = sessionManager - .getBranch() - .find( - (entry) => - entry.type === "message" && - entry.message.role === "assistant" && - Array.isArray(entry.message.content) && - entry.message.content.some((part) => part.type === "text" && part.text === "summarized"), - ); + const { sessionManager, toolResultEntryId } = createReadRewriteSession(); + const summaryEntry = findAssistantEntryByText(sessionManager, "summarized"); expect(summaryEntry).toBeDefined(); sessionManager.appendLabelChange(summaryEntry!.id, "bookmark"); @@ -173,66 +186,24 @@ describe("rewriteTranscriptEntriesInSessionManager", () => { replacements: [ { entryId: toolResultEntryId, - message: { - role: "toolResult", - toolCallId: "call_1", - toolName: "read", - content: [{ type: "text", text: "[externalized file_123]" }], - isError: false, - timestamp: 3, - }, + message: createToolResultReplacement("read", "[externalized file_123]", 3), }, ], }); expect(result.changed).toBe(true); - const rewrittenSummaryEntry = sessionManager - .getBranch() - .find( - (entry) => - entry.type === "message" && - entry.message.role === "assistant" && - Array.isArray(entry.message.content) && - entry.message.content.some((part) => part.type === "text" && part.text === "summarized"), - ); + const rewrittenSummaryEntry = findAssistantEntryByText(sessionManager, "summarized"); expect(rewrittenSummaryEntry).toBeDefined(); expect(sessionManager.getLabel(rewrittenSummaryEntry!.id)).toBe("bookmark"); expect(sessionManager.getBranch().some((entry) => entry.type === "label")).toBe(true); }); it("remaps compaction keep markers when rewritten entries change ids", () => { - const sessionManager = SessionManager.inMemory(); - sessionManager.appendMessage( - asAppendMessage({ - role: "user", - content: "read file", - timestamp: 1, - }), - ); - sessionManager.appendMessage( - asAppendMessage({ - role: "assistant", - content: [{ type: "toolCall", id: "call_1", name: "read", arguments: {} }], - timestamp: 2, - }), - ); - const toolResultEntryId = sessionManager.appendMessage( - asAppendMessage({ - role: "toolResult", - toolCallId: "call_1", - toolName: "read", - content: [{ type: "text", text: "x".repeat(8_000) }], - isError: false, - timestamp: 3, - }), - ); - const keptAssistantEntryId = sessionManager.appendMessage( - asAppendMessage({ - role: "assistant", - content: [{ type: "text", text: "keep me" }], - timestamp: 4, - }), - ); + const { + sessionManager, + toolResultEntryId, + tailAssistantEntryId: keptAssistantEntryId, + } = createReadRewriteSession({ tailAssistantText: "keep me" }); sessionManager.appendCompaction("summary", keptAssistantEntryId, 123); const result = rewriteTranscriptEntriesInSessionManager({ @@ -240,14 +211,7 @@ describe("rewriteTranscriptEntriesInSessionManager", () => { replacements: [ { entryId: toolResultEntryId, - message: { - role: "toolResult", - toolCallId: "call_1", - toolName: "read", - content: [{ type: "text", text: "[externalized file_123]" }], - isError: false, - timestamp: 3, - }, + message: createToolResultReplacement("read", "[externalized file_123]", 3), }, ], }); @@ -270,31 +234,7 @@ describe("rewriteTranscriptEntriesInSessionManager", () => { }); it("bypasses persistence hooks when replaying rewritten messages", () => { - const sessionManager = SessionManager.inMemory(); - sessionManager.appendMessage( - asAppendMessage({ - role: "user", - content: "run tool", - timestamp: 1, - }), - ); - const toolResultEntryId = sessionManager.appendMessage( - asAppendMessage({ - role: "toolResult", - toolCallId: "call_1", - toolName: "exec", - content: [{ type: "text", text: "before rewrite" }], - isError: false, - timestamp: 2, - }), - ); - sessionManager.appendMessage( - asAppendMessage({ - role: "assistant", - content: [{ type: "text", text: "summarized" }], - timestamp: 3, - }), - ); + const { sessionManager, toolResultEntryId } = createExecRewriteSession(); installSessionToolResultGuard(sessionManager, { transformToolResultForPersistence: (message) => ({ ...(message as Extract), @@ -309,14 +249,7 @@ describe("rewriteTranscriptEntriesInSessionManager", () => { replacements: [ { entryId: toolResultEntryId, - message: { - role: "toolResult", - toolCallId: "call_1", - toolName: "exec", - content: [{ type: "text", text: "[exact replacement]" }], - isError: false, - timestamp: 2, - }, + message: createToolResultReplacement("exec", "[exact replacement]", 2), }, ], }); @@ -341,29 +274,7 @@ describe("rewriteTranscriptEntriesInSessionManager", () => { describe("rewriteTranscriptEntriesInSessionFile", () => { it("emits transcript updates when the active branch changes", async () => { const sessionFile = "/tmp/session.jsonl"; - const sessionManager = SessionManager.inMemory(); - sessionManager.appendMessage( - asAppendMessage({ - role: "user", - content: "run tool", - timestamp: 1, - }), - ); - sessionManager.appendMessage( - asAppendMessage({ - role: "toolResult", - toolCallId: "call_1", - toolName: "exec", - content: [{ type: "text", text: "y".repeat(6_000) }], - isError: false, - timestamp: 2, - }), - ); - - const toolResultEntry = sessionManager - .getBranch() - .find((entry) => entry.type === "message" && entry.message.role === "toolResult"); - expect(toolResultEntry).toBeDefined(); + const { sessionManager, toolResultEntryId } = createExecRewriteSession(); const openSpy = vi .spyOn(SessionManager, "open") @@ -378,15 +289,8 @@ describe("rewriteTranscriptEntriesInSessionFile", () => { request: { replacements: [ { - entryId: toolResultEntry!.id, - message: { - role: "toolResult", - toolCallId: "call_1", - toolName: "exec", - content: [{ type: "text", text: "[file_ref:file_abc]" }], - isError: false, - timestamp: 2, - }, + entryId: toolResultEntryId, + message: createToolResultReplacement("exec", "[file_ref:file_abc]", 2), }, ], }, diff --git a/src/auto-reply/reply/directive-handling.model.test.ts b/src/auto-reply/reply/directive-handling.model.test.ts index 192f5566031..9af1b444ccc 100644 --- a/src/auto-reply/reply/directive-handling.model.test.ts +++ b/src/auto-reply/reply/directive-handling.model.test.ts @@ -34,6 +34,9 @@ vi.mock("../../infra/system-events.js", () => ({ })); const TEST_AGENT_DIR = "/tmp/agent"; +const OPENAI_DATE_PROFILE_ID = "20251001"; + +type ApiKeyProfile = { type: "api_key"; provider: string; key: string }; function baseAliasIndex(): ModelAliasIndex { return { byAlias: new Map(), byKey: new Map() }; @@ -46,6 +49,14 @@ function baseConfig(): OpenClawConfig { } as unknown as OpenClawConfig; } +function createSessionEntry(overrides?: Partial): SessionEntry { + return { + sessionId: "s1", + updatedAt: Date.now(), + ...overrides, + }; +} + beforeEach(() => { clearRuntimeAuthProfileStoreSnapshots(); replaceRuntimeAuthProfileStoreSnapshots([ @@ -60,9 +71,7 @@ afterEach(() => { clearRuntimeAuthProfileStoreSnapshots(); }); -function setAuthProfiles( - profiles: Record, -) { +function setAuthProfiles(profiles: Record) { replaceRuntimeAuthProfileStoreSnapshots([ { agentDir: TEST_AGENT_DIR, @@ -71,6 +80,23 @@ function setAuthProfiles( ]); } +function createDateAuthProfiles(provider: string, id = OPENAI_DATE_PROFILE_ID) { + return { + [id]: { + type: "api_key", + provider, + key: "sk-test", + }, + } satisfies Record; +} + +function createGptAliasIndex(): ModelAliasIndex { + return { + byAlias: new Map([["gpt", { alias: "gpt", ref: { provider: "openai", model: "gpt-4o" } }]]), + byKey: new Map([["openai/gpt-4o", ["gpt"]]]), + }; +} + function resolveModelSelectionForCommand(params: { command: string; allowedModelKeys: Set; @@ -89,6 +115,48 @@ function resolveModelSelectionForCommand(params: { }); } +async function persistModelDirectiveForTest(params: { + command: string; + profiles?: Record; + aliasIndex?: ModelAliasIndex; + allowedModelKeys: string[]; + sessionEntry?: SessionEntry; + provider?: string; + model?: string; + initialModelLabel?: string; +}) { + if (params.profiles) { + setAuthProfiles(params.profiles); + } + const directives = parseInlineDirectives(params.command); + const cfg = baseConfig(); + const sessionEntry = params.sessionEntry ?? createSessionEntry(); + const persisted = await persistInlineDirectives({ + directives, + effectiveModelDirective: directives.rawModelDirective, + cfg, + agentDir: TEST_AGENT_DIR, + sessionEntry, + sessionStore: { "agent:main:dm:1": sessionEntry }, + sessionKey: "agent:main:dm:1", + storePath: undefined, + elevatedEnabled: false, + elevatedAllowed: false, + defaultProvider: "anthropic", + defaultModel: "claude-opus-4-5", + aliasIndex: params.aliasIndex ?? baseAliasIndex(), + allowedModelKeys: new Set(params.allowedModelKeys), + provider: params.provider ?? "anthropic", + model: params.model ?? "claude-opus-4-5", + initialModelLabel: + params.initialModelLabel ?? + `${params.provider ?? "anthropic"}/${params.model ?? "claude-opus-4-5"}`, + formatModelSwitchEvent: (label) => label, + agentCfg: cfg.agents?.defaults, + }); + return { persisted, sessionEntry }; +} + async function resolveModelInfoReply( overrides: Partial[0]> = {}, ) { @@ -215,16 +283,10 @@ describe("/model chat UX", () => { }); it("treats @YYYYMMDD as a profile override when that profile exists for the resolved provider", () => { - setAuthProfiles({ - "20251001": { - type: "api_key", - provider: "openai", - key: "sk-test", - }, - }); + setAuthProfiles(createDateAuthProfiles("openai")); const resolved = resolveModelSelectionForCommand({ - command: "/model openai/gpt-4o@20251001", + command: `/model openai/gpt-4o@${OPENAI_DATE_PROFILE_ID}`, allowedModelKeys: new Set(["openai/gpt-4o"]), allowedModelCatalog: [], }); @@ -235,30 +297,19 @@ describe("/model chat UX", () => { model: "gpt-4o", isDefault: false, }); - expect(resolved.profileOverride).toBe("20251001"); + expect(resolved.profileOverride).toBe(OPENAI_DATE_PROFILE_ID); }); it("supports alias selections with numeric auth-profile overrides", () => { - setAuthProfiles({ - "20251001": { - type: "api_key", - provider: "openai", - key: "sk-test", - }, - }); - - const aliasIndex: ModelAliasIndex = { - byAlias: new Map([["gpt", { alias: "gpt", ref: { provider: "openai", model: "gpt-4o" } }]]), - byKey: new Map([["openai/gpt-4o", ["gpt"]]]), - }; + setAuthProfiles(createDateAuthProfiles("openai")); const resolved = resolveModelSelectionFromDirective({ - directives: parseInlineDirectives("/model gpt@20251001"), + directives: parseInlineDirectives(`/model gpt@${OPENAI_DATE_PROFILE_ID}`), cfg: { commands: { text: true } } as unknown as OpenClawConfig, - agentDir: "/tmp/agent", + agentDir: TEST_AGENT_DIR, defaultProvider: "anthropic", defaultModel: "claude-opus-4-5", - aliasIndex, + aliasIndex: createGptAliasIndex(), allowedModelKeys: new Set(["openai/gpt-4o"]), allowedModelCatalog: [], provider: "anthropic", @@ -271,20 +322,14 @@ describe("/model chat UX", () => { isDefault: false, alias: "gpt", }); - expect(resolved.profileOverride).toBe("20251001"); + expect(resolved.profileOverride).toBe(OPENAI_DATE_PROFILE_ID); }); it("supports providerless allowlist selections with numeric auth-profile overrides", () => { - setAuthProfiles({ - "20251001": { - type: "api_key", - provider: "openai", - key: "sk-test", - }, - }); + setAuthProfiles(createDateAuthProfiles("openai")); const resolved = resolveModelSelectionForCommand({ - command: "/model gpt-4o@20251001", + command: `/model gpt-4o@${OPENAI_DATE_PROFILE_ID}`, allowedModelKeys: new Set(["openai/gpt-4o"]), allowedModelCatalog: [], }); @@ -295,258 +340,103 @@ describe("/model chat UX", () => { model: "gpt-4o", isDefault: false, }); - expect(resolved.profileOverride).toBe("20251001"); + expect(resolved.profileOverride).toBe(OPENAI_DATE_PROFILE_ID); }); it("keeps @YYYYMMDD as part of the model when the stored numeric profile is for another provider", () => { - setAuthProfiles({ - "20251001": { - type: "api_key", - provider: "anthropic", - key: "sk-test", - }, - }); + setAuthProfiles(createDateAuthProfiles("anthropic")); const resolved = resolveModelSelectionForCommand({ - command: "/model custom/vertex-ai_claude-haiku-4-5@20251001", - allowedModelKeys: new Set(["custom/vertex-ai_claude-haiku-4-5@20251001"]), + command: `/model custom/vertex-ai_claude-haiku-4-5@${OPENAI_DATE_PROFILE_ID}`, + allowedModelKeys: new Set([`custom/vertex-ai_claude-haiku-4-5@${OPENAI_DATE_PROFILE_ID}`]), allowedModelCatalog: [], }); expect(resolved.errorText).toBeUndefined(); expect(resolved.modelSelection).toEqual({ provider: "custom", - model: "vertex-ai_claude-haiku-4-5@20251001", + model: `vertex-ai_claude-haiku-4-5@${OPENAI_DATE_PROFILE_ID}`, isDefault: false, }); expect(resolved.profileOverride).toBeUndefined(); }); it("persists inferred numeric auth-profile overrides for mixed-content messages", async () => { - setAuthProfiles({ - "20251001": { - type: "api_key", - provider: "openai", - key: "sk-test", - }, - }); - - const directives = parseInlineDirectives("/model openai/gpt-4o@20251001 hello"); - const sessionEntry = { - sessionId: "s1", - updatedAt: Date.now(), - } as SessionEntry; - const sessionStore = { "agent:main:dm:1": sessionEntry }; - - await persistInlineDirectives({ - directives, - effectiveModelDirective: directives.rawModelDirective, - cfg: baseConfig(), - agentDir: TEST_AGENT_DIR, - sessionEntry, - sessionStore, - sessionKey: "agent:main:dm:1", - storePath: undefined, - elevatedEnabled: false, - elevatedAllowed: false, - defaultProvider: "anthropic", - defaultModel: "claude-opus-4-5", - aliasIndex: baseAliasIndex(), - allowedModelKeys: new Set(["openai/gpt-4o", "openai/gpt-4o@20251001"]), - provider: "anthropic", - model: "claude-opus-4-5", - initialModelLabel: "anthropic/claude-opus-4-5", - formatModelSwitchEvent: (label) => label, - agentCfg: baseConfig().agents?.defaults, + const { sessionEntry } = await persistModelDirectiveForTest({ + command: `/model openai/gpt-4o@${OPENAI_DATE_PROFILE_ID} hello`, + profiles: createDateAuthProfiles("openai"), + allowedModelKeys: ["openai/gpt-4o", `openai/gpt-4o@${OPENAI_DATE_PROFILE_ID}`], }); expect(sessionEntry.providerOverride).toBe("openai"); expect(sessionEntry.modelOverride).toBe("gpt-4o"); - expect(sessionEntry.authProfileOverride).toBe("20251001"); + expect(sessionEntry.authProfileOverride).toBe(OPENAI_DATE_PROFILE_ID); }); it("persists alias-based numeric auth-profile overrides for mixed-content messages", async () => { - setAuthProfiles({ - "20251001": { - type: "api_key", - provider: "openai", - key: "sk-test", - }, - }); - - const aliasIndex: ModelAliasIndex = { - byAlias: new Map([["gpt", { alias: "gpt", ref: { provider: "openai", model: "gpt-4o" } }]]), - byKey: new Map([["openai/gpt-4o", ["gpt"]]]), - }; - const directives = parseInlineDirectives("/model gpt@20251001 hello"); - const sessionEntry = { - sessionId: "s1", - updatedAt: Date.now(), - } as SessionEntry; - const sessionStore = { "agent:main:dm:1": sessionEntry }; - - await persistInlineDirectives({ - directives, - effectiveModelDirective: directives.rawModelDirective, - cfg: baseConfig(), - agentDir: TEST_AGENT_DIR, - sessionEntry, - sessionStore, - sessionKey: "agent:main:dm:1", - storePath: undefined, - elevatedEnabled: false, - elevatedAllowed: false, - defaultProvider: "anthropic", - defaultModel: "claude-opus-4-5", - aliasIndex, - allowedModelKeys: new Set(["openai/gpt-4o"]), - provider: "anthropic", - model: "claude-opus-4-5", - initialModelLabel: "anthropic/claude-opus-4-5", - formatModelSwitchEvent: (label) => label, - agentCfg: baseConfig().agents?.defaults, + const { sessionEntry } = await persistModelDirectiveForTest({ + command: `/model gpt@${OPENAI_DATE_PROFILE_ID} hello`, + profiles: createDateAuthProfiles("openai"), + aliasIndex: createGptAliasIndex(), + allowedModelKeys: ["openai/gpt-4o"], }); expect(sessionEntry.providerOverride).toBe("openai"); expect(sessionEntry.modelOverride).toBe("gpt-4o"); - expect(sessionEntry.authProfileOverride).toBe("20251001"); + expect(sessionEntry.authProfileOverride).toBe(OPENAI_DATE_PROFILE_ID); }); it("persists providerless numeric auth-profile overrides for mixed-content messages", async () => { - setAuthProfiles({ - "20251001": { - type: "api_key", - provider: "openai", - key: "sk-test", - }, - }); - - const directives = parseInlineDirectives("/model gpt-4o@20251001 hello"); - const sessionEntry = { - sessionId: "s1", - updatedAt: Date.now(), - } as SessionEntry; - const sessionStore = { "agent:main:dm:1": sessionEntry }; - - await persistInlineDirectives({ - directives, - effectiveModelDirective: directives.rawModelDirective, - cfg: baseConfig(), - agentDir: TEST_AGENT_DIR, - sessionEntry, - sessionStore, - sessionKey: "agent:main:dm:1", - storePath: undefined, - elevatedEnabled: false, - elevatedAllowed: false, - defaultProvider: "anthropic", - defaultModel: "claude-opus-4-5", - aliasIndex: baseAliasIndex(), - allowedModelKeys: new Set(["openai/gpt-4o"]), - provider: "anthropic", - model: "claude-opus-4-5", - initialModelLabel: "anthropic/claude-opus-4-5", - formatModelSwitchEvent: (label) => label, - agentCfg: baseConfig().agents?.defaults, + const { sessionEntry } = await persistModelDirectiveForTest({ + command: `/model gpt-4o@${OPENAI_DATE_PROFILE_ID} hello`, + profiles: createDateAuthProfiles("openai"), + allowedModelKeys: ["openai/gpt-4o"], }); expect(sessionEntry.providerOverride).toBe("openai"); expect(sessionEntry.modelOverride).toBe("gpt-4o"); - expect(sessionEntry.authProfileOverride).toBe("20251001"); + expect(sessionEntry.authProfileOverride).toBe(OPENAI_DATE_PROFILE_ID); }); it("persists explicit auth profiles after @YYYYMMDD version suffixes in mixed-content messages", async () => { - setAuthProfiles({ - work: { - type: "api_key", - provider: "custom", - key: "sk-test", + const { sessionEntry } = await persistModelDirectiveForTest({ + command: `/model custom/vertex-ai_claude-haiku-4-5@${OPENAI_DATE_PROFILE_ID}@work hello`, + profiles: { + work: { + type: "api_key", + provider: "custom", + key: "sk-test", + }, }, - }); - - const directives = parseInlineDirectives( - "/model custom/vertex-ai_claude-haiku-4-5@20251001@work hello", - ); - const sessionEntry = { - sessionId: "s1", - updatedAt: Date.now(), - } as SessionEntry; - const sessionStore = { "agent:main:dm:1": sessionEntry }; - - await persistInlineDirectives({ - directives, - effectiveModelDirective: directives.rawModelDirective, - cfg: baseConfig(), - agentDir: TEST_AGENT_DIR, - sessionEntry, - sessionStore, - sessionKey: "agent:main:dm:1", - storePath: undefined, - elevatedEnabled: false, - elevatedAllowed: false, - defaultProvider: "anthropic", - defaultModel: "claude-opus-4-5", - aliasIndex: baseAliasIndex(), - allowedModelKeys: new Set(["custom/vertex-ai_claude-haiku-4-5@20251001"]), - provider: "anthropic", - model: "claude-opus-4-5", - initialModelLabel: "anthropic/claude-opus-4-5", - formatModelSwitchEvent: (label) => label, - agentCfg: baseConfig().agents?.defaults, + allowedModelKeys: [`custom/vertex-ai_claude-haiku-4-5@${OPENAI_DATE_PROFILE_ID}`], }); expect(sessionEntry.providerOverride).toBe("custom"); - expect(sessionEntry.modelOverride).toBe("vertex-ai_claude-haiku-4-5@20251001"); + expect(sessionEntry.modelOverride).toBe(`vertex-ai_claude-haiku-4-5@${OPENAI_DATE_PROFILE_ID}`); expect(sessionEntry.authProfileOverride).toBe("work"); }); it("ignores invalid mixed-content model directives during persistence", async () => { - setAuthProfiles({ - "20251001": { - type: "api_key", - provider: "openai", - key: "sk-test", - }, - }); - - const directives = parseInlineDirectives("/model 99 hello"); - const sessionEntry = { - sessionId: "s1", - updatedAt: Date.now(), - providerOverride: "openai", - modelOverride: "gpt-4o", - authProfileOverride: "20251001", - authProfileOverrideSource: "user", - } as SessionEntry; - const sessionStore = { "agent:main:dm:1": sessionEntry }; - - const persisted = await persistInlineDirectives({ - directives, - effectiveModelDirective: directives.rawModelDirective, - cfg: baseConfig(), - agentDir: TEST_AGENT_DIR, - sessionEntry, - sessionStore, - sessionKey: "agent:main:dm:1", - storePath: undefined, - elevatedEnabled: false, - elevatedAllowed: false, - defaultProvider: "anthropic", - defaultModel: "claude-opus-4-5", - aliasIndex: baseAliasIndex(), - allowedModelKeys: new Set(["openai/gpt-4o"]), + const { persisted, sessionEntry } = await persistModelDirectiveForTest({ + command: "/model 99 hello", + profiles: createDateAuthProfiles("openai"), + allowedModelKeys: ["openai/gpt-4o"], + sessionEntry: createSessionEntry({ + providerOverride: "openai", + modelOverride: "gpt-4o", + authProfileOverride: OPENAI_DATE_PROFILE_ID, + authProfileOverrideSource: "user", + }), provider: "openai", model: "gpt-4o", initialModelLabel: "openai/gpt-4o", - formatModelSwitchEvent: (label) => label, - agentCfg: baseConfig().agents?.defaults, }); expect(persisted.provider).toBe("openai"); expect(persisted.model).toBe("gpt-4o"); expect(sessionEntry.providerOverride).toBe("openai"); expect(sessionEntry.modelOverride).toBe("gpt-4o"); - expect(sessionEntry.authProfileOverride).toBe("20251001"); + expect(sessionEntry.authProfileOverride).toBe(OPENAI_DATE_PROFILE_ID); expect(sessionEntry.authProfileOverrideSource).toBe("user"); }); }); @@ -562,14 +452,6 @@ describe("handleDirectiveOnly model persist behavior (fixes #1435)", () => { type HandleParams = Parameters[0]; - function createSessionEntry(overrides?: Partial): SessionEntry { - return { - sessionId: "s1", - updatedAt: Date.now(), - ...overrides, - }; - } - function createHandleParams(overrides: Partial): HandleParams { const entryOverride = overrides.sessionEntry; const storeOverride = overrides.sessionStore; diff --git a/src/infra/outbound/outbound-session.test.ts b/src/infra/outbound/outbound-session.test.ts index 7a45f938bf8..a51e7e9d91d 100644 --- a/src/infra/outbound/outbound-session.test.ts +++ b/src/infra/outbound/outbound-session.test.ts @@ -265,6 +265,30 @@ describe("resolveOutboundSessionRoute", () => { chatType: "direct", }, }, + { + name: "Slack user DM target", + cfg: perChannelPeerCfg, + channel: "slack", + target: "user:U12345ABC", + expected: { + sessionKey: "agent:main:slack:direct:u12345abc", + from: "slack:U12345ABC", + to: "user:U12345ABC", + chatType: "direct", + }, + }, + { + name: "Slack channel target without thread", + cfg: baseConfig, + channel: "slack", + target: "channel:C999XYZ", + expected: { + sessionKey: "agent:main:slack:channel:c999xyz", + from: "slack:channel:C999XYZ", + to: "channel:C999XYZ", + chatType: "channel", + }, + }, ]; for (const testCase of cases) { diff --git a/src/infra/outbound/outbound.test.ts b/src/infra/outbound/outbound.test.ts index 9de658e2b7e..d91439efe89 100644 --- a/src/infra/outbound/outbound.test.ts +++ b/src/infra/outbound/outbound.test.ts @@ -1,5 +1,4 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import type { ReplyPayload } from "../../auto-reply/types.js"; import { setDefaultChannelPluginRegistryForTests } from "../../commands/channel-test-helpers.js"; import type { OpenClawConfig } from "../../config/config.js"; import { setActivePluginRegistry } from "../../plugins/runtime.js"; @@ -8,22 +7,11 @@ import { typedCases } from "../../test-utils/typed-cases.js"; import { DirectoryCache } from "./directory-cache.js"; import { buildOutboundResultEnvelope } from "./envelope.js"; import type { OutboundDeliveryJson } from "./format.js"; -import { - buildOutboundDeliveryJson, - formatGatewaySummary, - formatOutboundDeliverySummary, -} from "./format.js"; import { applyCrossContextDecoration, buildCrossContextDecoration, enforceCrossContextPolicy, } from "./outbound-policy.js"; -import { resolveOutboundSessionRoute } from "./outbound-session.js"; -import { - formatOutboundPayloadLog, - normalizeOutboundPayloads, - normalizeOutboundPayloadsForJson, -} from "./payloads.js"; import { runResolveOutboundTargetCoreTests } from "./targets.shared-test.js"; beforeEach(() => { @@ -149,132 +137,6 @@ describe("buildOutboundResultEnvelope", () => { }); }); -describe("formatOutboundDeliverySummary", () => { - it("formats fallback and channel-specific detail variants", () => { - const cases = [ - { - name: "fallback telegram", - channel: "telegram" as const, - result: undefined, - expected: "✅ Sent via Telegram. Message ID: unknown", - }, - { - name: "fallback imessage", - channel: "imessage" as const, - result: undefined, - expected: "✅ Sent via iMessage. Message ID: unknown", - }, - { - name: "telegram with chat detail", - channel: "telegram" as const, - result: { - channel: "telegram" as const, - messageId: "m1", - chatId: "c1", - }, - expected: "✅ Sent via Telegram. Message ID: m1 (chat c1)", - }, - { - name: "discord with channel detail", - channel: "discord" as const, - result: { - channel: "discord" as const, - messageId: "d1", - channelId: "chan", - }, - expected: "✅ Sent via Discord. Message ID: d1 (channel chan)", - }, - ]; - - for (const testCase of cases) { - expect(formatOutboundDeliverySummary(testCase.channel, testCase.result), testCase.name).toBe( - testCase.expected, - ); - } - }); -}); - -describe("buildOutboundDeliveryJson", () => { - it("builds direct delivery payloads across provider-specific fields", () => { - const cases = [ - { - name: "telegram direct payload", - input: { - channel: "telegram" as const, - to: "123", - result: { channel: "telegram" as const, messageId: "m1", chatId: "c1" }, - mediaUrl: "https://example.com/a.png", - }, - expected: { - channel: "telegram", - via: "direct", - to: "123", - messageId: "m1", - mediaUrl: "https://example.com/a.png", - chatId: "c1", - }, - }, - { - name: "whatsapp metadata", - input: { - channel: "whatsapp" as const, - to: "+1", - result: { channel: "whatsapp" as const, messageId: "w1", toJid: "jid" }, - }, - expected: { - channel: "whatsapp", - via: "direct", - to: "+1", - messageId: "w1", - mediaUrl: null, - toJid: "jid", - }, - }, - { - name: "signal timestamp", - input: { - channel: "signal" as const, - to: "+1", - result: { channel: "signal" as const, messageId: "s1", timestamp: 123 }, - }, - expected: { - channel: "signal", - via: "direct", - to: "+1", - messageId: "s1", - mediaUrl: null, - timestamp: 123, - }, - }, - ]; - - for (const testCase of cases) { - expect(buildOutboundDeliveryJson(testCase.input), testCase.name).toEqual(testCase.expected); - } - }); -}); - -describe("formatGatewaySummary", () => { - it("formats default and custom gateway action summaries", () => { - const cases = [ - { - name: "default send action", - input: { channel: "whatsapp", messageId: "m1" }, - expected: "✅ Sent via gateway (whatsapp). Message ID: m1", - }, - { - name: "custom action", - input: { action: "Poll sent", channel: "discord", messageId: "p1" }, - expected: "✅ Poll sent via gateway (discord). Message ID: p1", - }, - ]; - - for (const testCase of cases) { - expect(formatGatewaySummary(testCase.input), testCase.name).toBe(testCase.expected); - } - }); -}); - const slackConfig = { channels: { slack: { @@ -336,513 +198,4 @@ describe("outbound policy", () => { }); }); -describe("resolveOutboundSessionRoute", () => { - beforeEach(() => { - setDefaultChannelPluginRegistryForTests(); - }); - - const baseConfig = {} as OpenClawConfig; - - it("resolves provider-specific session routes", async () => { - const perChannelPeerCfg = { session: { dmScope: "per-channel-peer" } } as OpenClawConfig; - const identityLinksCfg = { - session: { - dmScope: "per-peer", - identityLinks: { - alice: ["discord:123"], - }, - }, - } as OpenClawConfig; - const slackMpimCfg = { - channels: { - slack: { - dm: { - groupChannels: ["G123"], - }, - }, - }, - } as OpenClawConfig; - const cases: Array<{ - name: string; - cfg: OpenClawConfig; - channel: string; - target: string; - replyToId?: string; - threadId?: string; - expected: { - sessionKey: string; - from?: string; - to?: string; - threadId?: string | number; - chatType?: "channel" | "direct" | "group"; - }; - }> = [ - { - name: "WhatsApp group jid", - cfg: baseConfig, - channel: "whatsapp", - target: "120363040000000000@g.us", - expected: { - sessionKey: "agent:main:whatsapp:group:120363040000000000@g.us", - from: "120363040000000000@g.us", - to: "120363040000000000@g.us", - chatType: "group", - }, - }, - { - name: "Matrix room target", - cfg: baseConfig, - channel: "matrix", - target: "room:!ops:matrix.example", - expected: { - sessionKey: "agent:main:matrix:channel:!ops:matrix.example", - from: "matrix:channel:!ops:matrix.example", - to: "room:!ops:matrix.example", - chatType: "channel", - }, - }, - { - name: "MSTeams conversation target", - cfg: baseConfig, - channel: "msteams", - target: "conversation:19:meeting_abc@thread.tacv2", - expected: { - sessionKey: "agent:main:msteams:channel:19:meeting_abc@thread.tacv2", - from: "msteams:channel:19:meeting_abc@thread.tacv2", - to: "conversation:19:meeting_abc@thread.tacv2", - chatType: "channel", - }, - }, - { - name: "Slack thread", - cfg: baseConfig, - channel: "slack", - target: "channel:C123", - replyToId: "456", - expected: { - sessionKey: "agent:main:slack:channel:c123:thread:456", - from: "slack:channel:C123", - to: "channel:C123", - threadId: "456", - }, - }, - { - name: "Telegram topic group", - cfg: baseConfig, - channel: "telegram", - target: "-100123456:topic:42", - expected: { - sessionKey: "agent:main:telegram:group:-100123456:topic:42", - from: "telegram:group:-100123456:topic:42", - to: "telegram:-100123456", - threadId: 42, - }, - }, - { - name: "Telegram DM with topic", - cfg: perChannelPeerCfg, - channel: "telegram", - target: "123456789:topic:99", - expected: { - sessionKey: "agent:main:telegram:direct:123456789:thread:99", - from: "telegram:123456789:topic:99", - to: "telegram:123456789", - threadId: 99, - chatType: "direct", - }, - }, - { - name: "Telegram unresolved username DM", - cfg: perChannelPeerCfg, - channel: "telegram", - target: "@alice", - expected: { - sessionKey: "agent:main:telegram:direct:@alice", - chatType: "direct", - }, - }, - { - name: "Telegram DM scoped threadId fallback", - cfg: perChannelPeerCfg, - channel: "telegram", - target: "12345", - threadId: "12345:99", - expected: { - sessionKey: "agent:main:telegram:direct:12345:thread:99", - from: "telegram:12345:topic:99", - to: "telegram:12345", - threadId: 99, - chatType: "direct", - }, - }, - { - name: "identity-links per-peer", - cfg: identityLinksCfg, - channel: "discord", - target: "user:123", - expected: { - sessionKey: "agent:main:direct:alice", - }, - }, - { - name: "Nextcloud Talk room target", - cfg: baseConfig, - channel: "nextcloud-talk", - target: "room:opsroom42", - expected: { - sessionKey: "agent:main:nextcloud-talk:group:opsroom42", - from: "nextcloud-talk:room:opsroom42", - to: "nextcloud-talk:opsroom42", - chatType: "group", - }, - }, - { - name: "BlueBubbles chat_* prefix stripping", - cfg: baseConfig, - channel: "bluebubbles", - target: "chat_guid:ABC123", - expected: { - sessionKey: "agent:main:bluebubbles:group:abc123", - from: "group:ABC123", - }, - }, - { - name: "Zalo direct target", - cfg: perChannelPeerCfg, - channel: "zalo", - target: "zl:123456", - expected: { - sessionKey: "agent:main:zalo:direct:123456", - from: "zalo:123456", - to: "zalo:123456", - chatType: "direct", - }, - }, - { - name: "Zalo Personal DM target", - cfg: perChannelPeerCfg, - channel: "zalouser", - target: "123456", - expected: { - sessionKey: "agent:main:zalouser:direct:123456", - chatType: "direct", - }, - }, - { - name: "Nostr prefixed target", - cfg: perChannelPeerCfg, - channel: "nostr", - target: "nostr:npub1example", - expected: { - sessionKey: "agent:main:nostr:direct:npub1example", - from: "nostr:npub1example", - to: "nostr:npub1example", - chatType: "direct", - }, - }, - { - name: "Tlon group target", - cfg: baseConfig, - channel: "tlon", - target: "group:~zod/main", - expected: { - sessionKey: "agent:main:tlon:group:chat/~zod/main", - from: "tlon:group:chat/~zod/main", - to: "tlon:chat/~zod/main", - chatType: "group", - }, - }, - { - name: "Slack mpim allowlist -> group key", - cfg: slackMpimCfg, - channel: "slack", - target: "channel:G123", - expected: { - sessionKey: "agent:main:slack:group:g123", - from: "slack:group:G123", - }, - }, - { - name: "Feishu explicit group prefix keeps group routing", - cfg: baseConfig, - channel: "feishu", - target: "group:oc_group_chat", - expected: { - sessionKey: "agent:main:feishu:group:oc_group_chat", - from: "feishu:group:oc_group_chat", - to: "oc_group_chat", - chatType: "group", - }, - }, - { - name: "Feishu explicit dm prefix keeps direct routing", - cfg: perChannelPeerCfg, - channel: "feishu", - target: "dm:oc_dm_chat", - expected: { - sessionKey: "agent:main:feishu:direct:oc_dm_chat", - from: "feishu:oc_dm_chat", - to: "oc_dm_chat", - chatType: "direct", - }, - }, - { - name: "Feishu bare oc_ target defaults to direct routing", - cfg: perChannelPeerCfg, - channel: "feishu", - target: "oc_ambiguous_chat", - expected: { - sessionKey: "agent:main:feishu:direct:oc_ambiguous_chat", - from: "feishu:oc_ambiguous_chat", - to: "oc_ambiguous_chat", - chatType: "direct", - }, - }, - { - name: "Slack user DM target", - cfg: perChannelPeerCfg, - channel: "slack", - target: "user:U12345ABC", - expected: { - sessionKey: "agent:main:slack:direct:u12345abc", - from: "slack:U12345ABC", - to: "user:U12345ABC", - chatType: "direct", - }, - }, - { - name: "Slack channel target without thread", - cfg: baseConfig, - channel: "slack", - target: "channel:C999XYZ", - expected: { - sessionKey: "agent:main:slack:channel:c999xyz", - from: "slack:channel:C999XYZ", - to: "channel:C999XYZ", - chatType: "channel", - }, - }, - ]; - - for (const testCase of cases) { - const route = await resolveOutboundSessionRoute({ - cfg: testCase.cfg, - channel: testCase.channel, - agentId: "main", - target: testCase.target, - replyToId: testCase.replyToId, - threadId: testCase.threadId, - }); - expect(route?.sessionKey, testCase.name).toBe(testCase.expected.sessionKey); - if (testCase.expected.from !== undefined) { - expect(route?.from, testCase.name).toBe(testCase.expected.from); - } - if (testCase.expected.to !== undefined) { - expect(route?.to, testCase.name).toBe(testCase.expected.to); - } - if (testCase.expected.threadId !== undefined) { - expect(route?.threadId, testCase.name).toBe(testCase.expected.threadId); - } - if (testCase.expected.chatType !== undefined) { - expect(route?.chatType, testCase.name).toBe(testCase.expected.chatType); - } - } - }); - - it("uses resolved Discord user targets to route bare numeric ids as DMs", async () => { - const route = await resolveOutboundSessionRoute({ - cfg: { session: { dmScope: "per-channel-peer" } } as OpenClawConfig, - channel: "discord", - agentId: "main", - target: "123", - resolvedTarget: { - to: "user:123", - kind: "user", - source: "directory", - }, - }); - - expect(route).toMatchObject({ - sessionKey: "agent:main:discord:direct:123", - from: "discord:123", - to: "user:123", - chatType: "direct", - }); - }); - - it("uses resolved Mattermost user targets to route bare ids as DMs", async () => { - const userId = "dthcxgoxhifn3pwh65cut3ud3w"; - const route = await resolveOutboundSessionRoute({ - cfg: { session: { dmScope: "per-channel-peer" } } as OpenClawConfig, - channel: "mattermost", - agentId: "main", - target: userId, - resolvedTarget: { - to: `user:${userId}`, - kind: "user", - source: "directory", - }, - }); - - expect(route).toMatchObject({ - sessionKey: `agent:main:mattermost:direct:${userId}`, - from: `mattermost:${userId}`, - to: `user:${userId}`, - chatType: "direct", - }); - }); - - it("rejects bare numeric Discord targets when the caller has no kind hint", async () => { - await expect( - resolveOutboundSessionRoute({ - cfg: { session: { dmScope: "per-channel-peer" } } as OpenClawConfig, - channel: "discord", - agentId: "main", - target: "123", - }), - ).rejects.toThrow(/Ambiguous Discord recipient/); - }); -}); - -describe("normalizeOutboundPayloadsForJson", () => { - it("normalizes payloads for JSON output", () => { - const cases = typedCases<{ - input: Parameters[0]; - expected: ReturnType; - }>([ - { - input: [ - { text: "hi" }, - { text: "photo", mediaUrl: "https://x.test/a.jpg", audioAsVoice: true }, - { text: "multi", mediaUrls: ["https://x.test/1.png"] }, - ], - expected: [ - { - text: "hi", - mediaUrl: null, - mediaUrls: undefined, - audioAsVoice: undefined, - channelData: undefined, - }, - { - text: "photo", - mediaUrl: "https://x.test/a.jpg", - mediaUrls: ["https://x.test/a.jpg"], - audioAsVoice: true, - channelData: undefined, - }, - { - text: "multi", - mediaUrl: null, - mediaUrls: ["https://x.test/1.png"], - audioAsVoice: undefined, - channelData: undefined, - }, - ], - }, - { - input: [ - { - text: "MEDIA:https://x.test/a.png\nMEDIA:https://x.test/b.png", - }, - ], - expected: [ - { - text: "", - mediaUrl: null, - mediaUrls: ["https://x.test/a.png", "https://x.test/b.png"], - audioAsVoice: undefined, - channelData: undefined, - }, - ], - }, - ]); - - for (const testCase of cases) { - const input: ReplyPayload[] = testCase.input.map((payload) => - "mediaUrls" in payload - ? ({ - ...payload, - mediaUrls: payload.mediaUrls ? [...payload.mediaUrls] : undefined, - } as ReplyPayload) - : ({ ...payload } as ReplyPayload), - ); - expect(normalizeOutboundPayloadsForJson(input)).toEqual(testCase.expected); - } - }); - - it("suppresses reasoning payloads", () => { - const normalized = normalizeOutboundPayloadsForJson([ - { text: "Reasoning:\n_step_", isReasoning: true }, - { text: "final answer" }, - ]); - expect(normalized).toEqual([ - { text: "final answer", mediaUrl: null, mediaUrls: undefined, audioAsVoice: undefined }, - ]); - }); -}); - -describe("normalizeOutboundPayloads", () => { - it("keeps channelData-only payloads", () => { - const channelData = { line: { flexMessage: { altText: "Card", contents: {} } } }; - const normalized = normalizeOutboundPayloads([{ channelData }]); - expect(normalized).toEqual([{ text: "", mediaUrls: [], channelData }]); - }); - - it("suppresses reasoning payloads", () => { - const normalized = normalizeOutboundPayloads([ - { text: "Reasoning:\n_step_", isReasoning: true }, - { text: "final answer" }, - ]); - expect(normalized).toEqual([{ text: "final answer", mediaUrls: [] }]); - }); - - it("formats BTW replies prominently for external delivery", () => { - const normalized = normalizeOutboundPayloads([ - { - text: "323", - btw: { question: "what is 17 * 19?" }, - }, - ]); - expect(normalized).toEqual([{ text: "BTW\nQuestion: what is 17 * 19?\n\n323", mediaUrls: [] }]); - }); -}); - -describe("formatOutboundPayloadLog", () => { - it("formats text+media and media-only logs", () => { - const cases = typedCases<{ - name: string; - input: Parameters[0]; - expected: string; - }>([ - { - name: "text with media lines", - input: { - text: "hello ", - mediaUrls: ["https://x.test/a.png", "https://x.test/b.png"], - }, - expected: "hello\nMEDIA:https://x.test/a.png\nMEDIA:https://x.test/b.png", - }, - { - name: "media only", - input: { - text: "", - mediaUrls: ["https://x.test/a.png"], - }, - expected: "MEDIA:https://x.test/a.png", - }, - ]); - - for (const testCase of cases) { - expect( - formatOutboundPayloadLog({ - ...testCase.input, - mediaUrls: [...testCase.input.mediaUrls], - }), - testCase.name, - ).toBe(testCase.expected); - } - }); -}); - runResolveOutboundTargetCoreTests(); diff --git a/src/infra/outbound/payloads.test.ts b/src/infra/outbound/payloads.test.ts index 3aaf1a2f61e..7faae74042b 100644 --- a/src/infra/outbound/payloads.test.ts +++ b/src/infra/outbound/payloads.test.ts @@ -169,6 +169,17 @@ describe("normalizeOutboundPayloads", () => { ]), ).toEqual([{ text: "final answer", mediaUrls: [] }]); }); + + it("formats BTW replies prominently for external delivery", () => { + expect( + normalizeOutboundPayloads([ + { + text: "323", + btw: { question: "what is 17 * 19?" }, + }, + ]), + ).toEqual([{ text: "BTW\nQuestion: what is 17 * 19?\n\n323", mediaUrls: [] }]); + }); }); describe("formatOutboundPayloadLog", () => { diff --git a/test/architecture-smells.test.ts b/test/architecture-smells.test.ts index c64a4cdc363..7a612e53672 100644 --- a/test/architecture-smells.test.ts +++ b/test/architecture-smells.test.ts @@ -1,26 +1,6 @@ import { describe, expect, it } from "vitest"; import { collectArchitectureSmells, main } from "../scripts/check-architecture-smells.mjs"; - -function createCapturedIo() { - let stdout = ""; - let stderr = ""; - return { - io: { - stdout: { - write(chunk) { - stdout += String(chunk); - }, - }, - stderr: { - write(chunk) { - stderr += String(chunk); - }, - }, - }, - readStdout: () => stdout, - readStderr: () => stderr, - }; -} +import { createCapturedIo } from "./helpers/captured-io.js"; describe("architecture smell inventory", () => { it("produces stable sorted output", async () => { diff --git a/test/extension-plugin-sdk-boundary.test.ts b/test/extension-plugin-sdk-boundary.test.ts index 1c8fc866516..f7acd5f28fd 100644 --- a/test/extension-plugin-sdk-boundary.test.ts +++ b/test/extension-plugin-sdk-boundary.test.ts @@ -3,6 +3,7 @@ import { collectExtensionPluginSdkBoundaryInventory, main, } from "../scripts/check-extension-plugin-sdk-boundary.mjs"; +import { createCapturedIo } from "./helpers/captured-io.js"; const srcOutsideInventoryPromise = collectExtensionPluginSdkBoundaryInventory("src-outside-plugin-sdk"); @@ -27,27 +28,6 @@ async function getJsonOutput( }; } -function createCapturedIo() { - let stdout = ""; - let stderr = ""; - return { - io: { - stdout: { - write(chunk) { - stdout += String(chunk); - }, - }, - stderr: { - write(chunk) { - stderr += String(chunk); - }, - }, - }, - readStdout: () => stdout, - readStderr: () => stderr, - }; -} - describe("extension src outside plugin-sdk boundary inventory", () => { it("stays empty and sorted", async () => { const inventory = await srcOutsideInventoryPromise; diff --git a/test/helpers/captured-io.ts b/test/helpers/captured-io.ts new file mode 100644 index 00000000000..62abef11c02 --- /dev/null +++ b/test/helpers/captured-io.ts @@ -0,0 +1,20 @@ +export function createCapturedIo() { + let stdout = ""; + let stderr = ""; + return { + io: { + stdout: { + write(chunk: unknown) { + stdout += String(chunk); + }, + }, + stderr: { + write(chunk: unknown) { + stderr += String(chunk); + }, + }, + }, + readStdout: () => stdout, + readStderr: () => stderr, + }; +} diff --git a/test/helpers/extensions/discord-component-runtime.ts b/test/helpers/extensions/discord-component-runtime.ts new file mode 100644 index 00000000000..a5191d915d4 --- /dev/null +++ b/test/helpers/extensions/discord-component-runtime.ts @@ -0,0 +1,85 @@ +import { vi } from "vitest"; + +const runtimeMocks = vi.hoisted(() => ({ + readAllowFromStoreMock: vi.fn(), + upsertPairingRequestMock: vi.fn(), + recordInboundSessionMock: vi.fn(), + resolvePluginConversationBindingApprovalMock: vi.fn(), + buildPluginBindingResolvedTextMock: vi.fn(), +})); + +export const readAllowFromStoreMock = runtimeMocks.readAllowFromStoreMock; +export const upsertPairingRequestMock = runtimeMocks.upsertPairingRequestMock; +export const recordInboundSessionMock = runtimeMocks.recordInboundSessionMock; +export const resolvePluginConversationBindingApprovalMock = + runtimeMocks.resolvePluginConversationBindingApprovalMock; +export const buildPluginBindingResolvedTextMock = runtimeMocks.buildPluginBindingResolvedTextMock; + +async function createConversationRuntimeMock( + importOriginal: () => Promise, +) { + const actual = await importOriginal(); + return { + ...actual, + upsertChannelPairingRequest: (...args: unknown[]) => upsertPairingRequestMock(...args), + resolvePluginConversationBindingApproval: (...args: unknown[]) => + resolvePluginConversationBindingApprovalMock(...args), + buildPluginBindingResolvedText: (...args: unknown[]) => + buildPluginBindingResolvedTextMock(...args), + recordInboundSession: (...args: unknown[]) => recordInboundSessionMock(...args), + }; +} + +vi.mock("openclaw/plugin-sdk/security-runtime", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + readStoreAllowFromForDmPolicy: async (params: { + provider: string; + accountId: string; + dmPolicy?: string | null; + shouldRead?: boolean | null; + }) => { + if (params.shouldRead === false || params.dmPolicy === "allowlist") { + return []; + } + return await readAllowFromStoreMock(params.provider, params.accountId); + }, + }; +}); + +vi.mock("openclaw/plugin-sdk/conversation-runtime", createConversationRuntimeMock); +vi.mock("openclaw/plugin-sdk/conversation-runtime.js", createConversationRuntimeMock); + +export function resetDiscordComponentRuntimeMocks() { + readAllowFromStoreMock.mockClear().mockResolvedValue([]); + upsertPairingRequestMock.mockClear().mockResolvedValue({ code: "PAIRCODE", created: true }); + recordInboundSessionMock.mockClear().mockResolvedValue(undefined); + resolvePluginConversationBindingApprovalMock.mockReset().mockResolvedValue({ + status: "approved", + binding: { + bindingId: "binding-1", + pluginId: "openclaw-codex-app-server", + pluginName: "OpenClaw App Server", + pluginRoot: "/plugins/codex", + channel: "discord", + accountId: "default", + conversationId: "user:123456789", + boundAt: Date.now(), + }, + request: { + id: "approval-1", + pluginId: "openclaw-codex-app-server", + pluginName: "OpenClaw App Server", + pluginRoot: "/plugins/codex", + requestedAt: Date.now(), + conversation: { + channel: "discord", + accountId: "default", + conversationId: "user:123456789", + }, + }, + decision: "allow-once", + }); + buildPluginBindingResolvedTextMock.mockReset().mockReturnValue("Binding approved."); +} diff --git a/test/helpers/extensions/zalo-lifecycle.ts b/test/helpers/extensions/zalo-lifecycle.ts index ac61fee103f..7134febd129 100644 --- a/test/helpers/extensions/zalo-lifecycle.ts +++ b/test/helpers/extensions/zalo-lifecycle.ts @@ -1,14 +1,16 @@ -import { vi } from "vitest"; +import { expect, vi } from "vitest"; import type { ResolvedZaloAccount } from "../../../extensions/zalo/src/accounts.js"; import { clearZaloWebhookSecurityStateForTest, monitorZaloProvider, } from "../../../extensions/zalo/src/monitor.js"; +import type { PluginRuntime } from "../../../extensions/zalo/src/runtime-api.js"; import type { OpenClawConfig } from "../../../extensions/zalo/src/runtime-api.js"; import { normalizeSecretInputString } from "../../../extensions/zalo/src/secret-input.js"; import { createEmptyPluginRegistry } from "../../../src/plugins/registry.js"; import { setActivePluginRegistry } from "../../../src/plugins/runtime.js"; import { withServer } from "../http-test-server.js"; +import { createPluginRuntimeMock } from "./plugin-runtime-mock.js"; import { createRuntimeEnv } from "./runtime-env.js"; export { withServer }; @@ -110,6 +112,19 @@ export function createLifecycleAccount(params: { } as ResolvedZaloAccount; } +export function createLifecycleMonitorSetup(params: { + accountId: string; + dmPolicy: "open" | "pairing"; + allowFrom?: string[]; + webhookUrl?: string; + webhookSecret?: string; +}) { + return { + account: createLifecycleAccount(params), + config: createLifecycleConfig(params), + }; +} + export function createTextUpdate(params: { messageId: string; userId: string; @@ -129,6 +144,131 @@ export function createTextUpdate(params: { }; } +export function createImageUpdate(params?: { + messageId?: string; + userId?: string; + displayName?: string; + chatId?: string; + photoUrl?: string; + date?: number; +}) { + return { + event_name: "message.image.received", + message: { + date: params?.date ?? 1774086023728, + chat: { chat_type: "PRIVATE" as const, id: params?.chatId ?? "chat-123" }, + caption: "", + message_id: params?.messageId ?? "msg-123", + message_type: "CHAT_PHOTO", + from: { + id: params?.userId ?? "user-123", + is_bot: false, + display_name: params?.displayName ?? "Test User", + }, + photo_url: params?.photoUrl ?? "https://example.com/test-image.jpg", + }, + }; +} + +export function setLifecycleRuntimeCore( + channel: NonNullable[0]>["channel"]>, +) { + getZaloRuntimeMock.mockReturnValue( + createPluginRuntimeMock({ + channel, + }), + ); +} + +export function createImageLifecycleCore() { + const finalizeInboundContextMock = vi.fn((ctx: Record) => ctx); + const recordInboundSessionMock = vi.fn(async () => undefined); + const fetchRemoteMediaMock = vi.fn(async () => ({ + buffer: Buffer.from("image-bytes"), + contentType: "image/jpeg", + })); + const saveMediaBufferMock = vi.fn(async () => ({ + path: "/tmp/zalo-photo.jpg", + contentType: "image/jpeg", + })); + const core = createPluginRuntimeMock({ + channel: { + media: { + fetchRemoteMedia: + fetchRemoteMediaMock as unknown as PluginRuntime["channel"]["media"]["fetchRemoteMedia"], + saveMediaBuffer: + saveMediaBufferMock as unknown as PluginRuntime["channel"]["media"]["saveMediaBuffer"], + }, + reply: { + finalizeInboundContext: + finalizeInboundContextMock as unknown as PluginRuntime["channel"]["reply"]["finalizeInboundContext"], + dispatchReplyWithBufferedBlockDispatcher: vi.fn( + async () => undefined, + ) as unknown as PluginRuntime["channel"]["reply"]["dispatchReplyWithBufferedBlockDispatcher"], + }, + session: { + recordInboundSession: + recordInboundSessionMock as unknown as PluginRuntime["channel"]["session"]["recordInboundSession"], + }, + commands: { + shouldComputeCommandAuthorized: vi.fn( + () => false, + ) as unknown as PluginRuntime["channel"]["commands"]["shouldComputeCommandAuthorized"], + resolveCommandAuthorizedFromAuthorizers: vi.fn( + () => false, + ) as unknown as PluginRuntime["channel"]["commands"]["resolveCommandAuthorizedFromAuthorizers"], + isControlCommandMessage: vi.fn( + () => false, + ) as unknown as PluginRuntime["channel"]["commands"]["isControlCommandMessage"], + }, + }, + }); + return { + core, + finalizeInboundContextMock, + recordInboundSessionMock, + fetchRemoteMediaMock, + saveMediaBufferMock, + }; +} + +export function expectImageLifecycleDelivery(params: { + fetchRemoteMediaMock: ReturnType; + saveMediaBufferMock: ReturnType; + finalizeInboundContextMock: ReturnType; + recordInboundSessionMock: ReturnType; + photoUrl?: string; + senderName?: string; + mediaPath?: string; + mediaType?: string; +}) { + const photoUrl = params.photoUrl ?? "https://example.com/test-image.jpg"; + const senderName = params.senderName ?? "Test User"; + const mediaPath = params.mediaPath ?? "/tmp/zalo-photo.jpg"; + const mediaType = params.mediaType ?? "image/jpeg"; + expect(params.fetchRemoteMediaMock).toHaveBeenCalledWith({ + url: photoUrl, + maxBytes: 5 * 1024 * 1024, + }); + expect(params.saveMediaBufferMock).toHaveBeenCalledTimes(1); + expect(params.finalizeInboundContextMock).toHaveBeenCalledWith( + expect.objectContaining({ + SenderName: senderName, + MediaPath: mediaPath, + MediaType: mediaType, + }), + ); + expect(params.recordInboundSessionMock).toHaveBeenCalledWith( + expect.objectContaining({ + ctx: expect.objectContaining({ + SenderName: senderName, + MediaPath: mediaPath, + MediaType: mediaType, + }), + }), + ); +} + export async function settleAsyncWork(): Promise { for (let i = 0; i < 6; i += 1) { await Promise.resolve(); @@ -152,6 +292,21 @@ export async function postWebhookUpdate(params: { }); } +export async function postWebhookReplay(params: { + baseUrl: string; + path: string; + secret: string; + payload: Record; + settleBeforeReplay?: boolean; +}) { + const first = await postWebhookUpdate(params); + if (params.settleBeforeReplay) { + await settleAsyncWork(); + } + const replay = await postWebhookUpdate(params); + return { first, replay }; +} + export async function startWebhookLifecycleMonitor(params: { account: ResolvedZaloAccount; config: OpenClawConfig; diff --git a/test/helpers/pattern-file.ts b/test/helpers/pattern-file.ts new file mode 100644 index 00000000000..4244ca7b275 --- /dev/null +++ b/test/helpers/pattern-file.ts @@ -0,0 +1,23 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +export function createPatternFileHelper(prefix: string) { + const tempDirs = new Set(); + + return { + cleanup() { + for (const dir of tempDirs) { + fs.rmSync(dir, { recursive: true, force: true }); + } + tempDirs.clear(); + }, + writePatternFile(basename: string, value: unknown) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + tempDirs.add(dir); + const filePath = path.join(dir, basename); + fs.writeFileSync(filePath, `${JSON.stringify(value)}\n`, "utf8"); + return filePath; + }, + }; +} diff --git a/test/plugin-extension-import-boundary.test.ts b/test/plugin-extension-import-boundary.test.ts index 45c6d3620a9..226854c64bd 100644 --- a/test/plugin-extension-import-boundary.test.ts +++ b/test/plugin-extension-import-boundary.test.ts @@ -6,6 +6,7 @@ import { diffInventory, main, } from "../scripts/check-plugin-extension-import-boundary.mjs"; +import { createCapturedIo } from "./helpers/captured-io.js"; const repoRoot = process.cwd(); const baselinePath = path.join( @@ -16,27 +17,6 @@ const baselinePath = path.join( ); const baseline = JSON.parse(readFileSync(baselinePath, "utf8")); -function createCapturedIo() { - let stdout = ""; - let stderr = ""; - return { - io: { - stdout: { - write(chunk) { - stdout += String(chunk); - }, - }, - stderr: { - write(chunk) { - stderr += String(chunk); - }, - }, - }, - readStdout: () => stdout, - readStderr: () => stderr, - }; -} - describe("plugin extension import boundary inventory", () => { it("keeps dedicated web-search registry shims out of the remaining inventory", async () => { const inventory = await collectPluginExtensionImportBoundaryInventory(); diff --git a/test/vitest-extensions-config.test.ts b/test/vitest-extensions-config.test.ts index 8cc3ded11ad..f9228f04466 100644 --- a/test/vitest-extensions-config.test.ts +++ b/test/vitest-extensions-config.test.ts @@ -1,33 +1,20 @@ -import fs from "node:fs"; -import os from "node:os"; -import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { loadIncludePatternsFromEnv } from "../vitest.extensions.config.ts"; +import { createPatternFileHelper } from "./helpers/pattern-file.js"; -const tempDirs = new Set(); +const patternFiles = createPatternFileHelper("openclaw-vitest-extensions-config-"); afterEach(() => { - for (const dir of tempDirs) { - fs.rmSync(dir, { recursive: true, force: true }); - } - tempDirs.clear(); + patternFiles.cleanup(); }); -const writePatternFile = (basename: string, value: unknown) => { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-vitest-extensions-config-")); - tempDirs.add(dir); - const filePath = path.join(dir, basename); - fs.writeFileSync(filePath, `${JSON.stringify(value)}\n`, "utf8"); - return filePath; -}; - describe("extensions vitest include patterns", () => { it("returns null when no include file is configured", () => { expect(loadIncludePatternsFromEnv({})).toBeNull(); }); it("loads include patterns from a JSON file", () => { - const filePath = writePatternFile("include.json", [ + const filePath = patternFiles.writePatternFile("include.json", [ "extensions/feishu/index.test.ts", 42, "", @@ -42,7 +29,7 @@ describe("extensions vitest include patterns", () => { }); it("throws when the configured file is not a JSON array", () => { - const filePath = writePatternFile("include.json", { + const filePath = patternFiles.writePatternFile("include.json", { include: ["extensions/feishu/index.test.ts"], }); diff --git a/test/vitest-unit-config.test.ts b/test/vitest-unit-config.test.ts index fa5189fae81..3f67c203125 100644 --- a/test/vitest-unit-config.test.ts +++ b/test/vitest-unit-config.test.ts @@ -1,37 +1,24 @@ -import fs from "node:fs"; -import os from "node:os"; -import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { createUnitVitestConfig, loadExtraExcludePatternsFromEnv, loadIncludePatternsFromEnv, } from "../vitest.unit.config.ts"; +import { createPatternFileHelper } from "./helpers/pattern-file.js"; -const tempDirs = new Set(); +const patternFiles = createPatternFileHelper("openclaw-vitest-unit-config-"); afterEach(() => { - for (const dir of tempDirs) { - fs.rmSync(dir, { recursive: true, force: true }); - } - tempDirs.clear(); + patternFiles.cleanup(); }); -const writePatternFile = (basename: string, value: unknown) => { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-vitest-unit-config-")); - tempDirs.add(dir); - const filePath = path.join(dir, basename); - fs.writeFileSync(filePath, `${JSON.stringify(value)}\n`, "utf8"); - return filePath; -}; - describe("loadIncludePatternsFromEnv", () => { it("returns null when no include file is configured", () => { expect(loadIncludePatternsFromEnv({})).toBeNull(); }); it("loads include patterns from a JSON file", () => { - const filePath = writePatternFile("include.json", [ + const filePath = patternFiles.writePatternFile("include.json", [ "src/infra/update-runner.test.ts", 42, "", @@ -52,7 +39,7 @@ describe("loadExtraExcludePatternsFromEnv", () => { }); it("loads extra exclude patterns from a JSON file", () => { - const filePath = writePatternFile("extra-exclude.json", [ + const filePath = patternFiles.writePatternFile("extra-exclude.json", [ "src/infra/update-runner.test.ts", 42, "", @@ -67,7 +54,7 @@ describe("loadExtraExcludePatternsFromEnv", () => { }); it("throws when the configured file is not a JSON array", () => { - const filePath = writePatternFile("extra-exclude.json", { + const filePath = patternFiles.writePatternFile("extra-exclude.json", { exclude: ["src/infra/update-runner.test.ts"], }); diff --git a/test/web-search-provider-boundary.test.ts b/test/web-search-provider-boundary.test.ts index 4d0ee25d49f..6aa0a1bbb8a 100644 --- a/test/web-search-provider-boundary.test.ts +++ b/test/web-search-provider-boundary.test.ts @@ -3,31 +3,11 @@ import { collectWebSearchProviderBoundaryInventory, main, } from "../scripts/check-web-search-provider-boundaries.mjs"; +import { createCapturedIo } from "./helpers/captured-io.js"; const inventoryPromise = collectWebSearchProviderBoundaryInventory(); const jsonOutputPromise = getJsonOutput(); -function createCapturedIo() { - let stdout = ""; - let stderr = ""; - return { - io: { - stdout: { - write(chunk) { - stdout += String(chunk); - }, - }, - stderr: { - write(chunk) { - stderr += String(chunk); - }, - }, - }, - readStdout: () => stdout, - readStderr: () => stderr, - }; -} - async function getJsonOutput() { const captured = createCapturedIo(); const exitCode = await main(["--json"], captured.io); diff --git a/vitest.channels.config.ts b/vitest.channels.config.ts index 2997e7aa837..91ed8f9b76a 100644 --- a/vitest.channels.config.ts +++ b/vitest.channels.config.ts @@ -1,23 +1,11 @@ -import fs from "node:fs"; import { channelTestInclude } from "./vitest.channel-paths.mjs"; +import { loadPatternListFromEnv } from "./vitest.pattern-file.ts"; import { createScopedVitestConfig } from "./vitest.scoped-config.ts"; -function loadPatternListFile(filePath: string, label: string): string[] { - const parsed = JSON.parse(fs.readFileSync(filePath, "utf8")) as unknown; - if (!Array.isArray(parsed)) { - throw new TypeError(`${label} must point to a JSON array: ${filePath}`); - } - return parsed.filter((value): value is string => typeof value === "string" && value.length > 0); -} - export function loadIncludePatternsFromEnv( env: Record = process.env, ): string[] | null { - const includeFile = env.OPENCLAW_VITEST_INCLUDE_FILE?.trim(); - if (!includeFile) { - return null; - } - return loadPatternListFile(includeFile, "OPENCLAW_VITEST_INCLUDE_FILE"); + return loadPatternListFromEnv("OPENCLAW_VITEST_INCLUDE_FILE", env); } export function createChannelsVitestConfig(env?: Record) { diff --git a/vitest.extensions.config.ts b/vitest.extensions.config.ts index a9a25f8c8e9..acdaf337308 100644 --- a/vitest.extensions.config.ts +++ b/vitest.extensions.config.ts @@ -1,23 +1,11 @@ -import fs from "node:fs"; import { channelTestExclude } from "./vitest.channel-paths.mjs"; +import { loadPatternListFromEnv } from "./vitest.pattern-file.ts"; import { createScopedVitestConfig } from "./vitest.scoped-config.ts"; -function loadPatternListFile(filePath: string, label: string): string[] { - const parsed = JSON.parse(fs.readFileSync(filePath, "utf8")) as unknown; - if (!Array.isArray(parsed)) { - throw new TypeError(`${label} must point to a JSON array: ${filePath}`); - } - return parsed.filter((value): value is string => typeof value === "string" && value.length > 0); -} - export function loadIncludePatternsFromEnv( env: Record = process.env, ): string[] | null { - const includeFile = env.OPENCLAW_VITEST_INCLUDE_FILE?.trim(); - if (!includeFile) { - return null; - } - return loadPatternListFile(includeFile, "OPENCLAW_VITEST_INCLUDE_FILE"); + return loadPatternListFromEnv("OPENCLAW_VITEST_INCLUDE_FILE", env); } export function createExtensionsVitestConfig( diff --git a/vitest.pattern-file.ts b/vitest.pattern-file.ts new file mode 100644 index 00000000000..456fc1d1c24 --- /dev/null +++ b/vitest.pattern-file.ts @@ -0,0 +1,20 @@ +import fs from "node:fs"; + +export function loadPatternListFile(filePath: string, label: string): string[] { + const parsed = JSON.parse(fs.readFileSync(filePath, "utf8")) as unknown; + if (!Array.isArray(parsed)) { + throw new TypeError(`${label} must point to a JSON array: ${filePath}`); + } + return parsed.filter((value): value is string => typeof value === "string" && value.length > 0); +} + +export function loadPatternListFromEnv( + envKey: string, + env: Record = process.env, +): string[] | null { + const filePath = env[envKey]?.trim(); + if (!filePath) { + return null; + } + return loadPatternListFile(filePath, envKey); +} diff --git a/vitest.unit.config.ts b/vitest.unit.config.ts index 86d805740df..7f904fef96a 100644 --- a/vitest.unit.config.ts +++ b/vitest.unit.config.ts @@ -1,6 +1,6 @@ -import fs from "node:fs"; import { defineConfig } from "vitest/config"; import baseConfig from "./vitest.config.ts"; +import { loadPatternListFromEnv } from "./vitest.pattern-file.ts"; import { resolveVitestIsolation } from "./vitest.scoped-config.ts"; import { unitTestAdditionalExcludePatterns, @@ -10,32 +10,17 @@ import { const base = baseConfig as unknown as Record; const baseTest = (baseConfig as { test?: { include?: string[]; exclude?: string[] } }).test ?? {}; const exclude = baseTest.exclude ?? []; -function loadPatternListFile(filePath: string, label: string): string[] { - const parsed = JSON.parse(fs.readFileSync(filePath, "utf8")) as unknown; - if (!Array.isArray(parsed)) { - throw new TypeError(`${label} must point to a JSON array: ${filePath}`); - } - return parsed.filter((value): value is string => typeof value === "string" && value.length > 0); -} export function loadIncludePatternsFromEnv( env: Record = process.env, ): string[] | null { - const includeFile = env.OPENCLAW_VITEST_INCLUDE_FILE?.trim(); - if (!includeFile) { - return null; - } - return loadPatternListFile(includeFile, "OPENCLAW_VITEST_INCLUDE_FILE"); + return loadPatternListFromEnv("OPENCLAW_VITEST_INCLUDE_FILE", env); } export function loadExtraExcludePatternsFromEnv( env: Record = process.env, ): string[] { - const extraExcludeFile = env.OPENCLAW_VITEST_EXTRA_EXCLUDE_FILE?.trim(); - if (!extraExcludeFile) { - return []; - } - return loadPatternListFile(extraExcludeFile, "OPENCLAW_VITEST_EXTRA_EXCLUDE_FILE"); + return loadPatternListFromEnv("OPENCLAW_VITEST_EXTRA_EXCLUDE_FILE", env) ?? []; } export function createUnitVitestConfig(env: Record = process.env) {