diff --git a/src/commands/auth-choice.apply-helpers.test.ts b/src/commands/auth-choice.apply-helpers.test.ts index dac5d2579..9e5810791 100644 --- a/src/commands/auth-choice.apply-helpers.test.ts +++ b/src/commands/auth-choice.apply-helpers.test.ts @@ -64,6 +64,26 @@ async function ensureMinimaxApiKey(params: { }); } +async function ensureMinimaxApiKeyWithEnvRefPrompter(params: { + config?: Parameters[0]["config"]; + note: WizardPrompter["note"]; + select: WizardPrompter["select"]; + setCredential: Parameters[0]["setCredential"]; + text: WizardPrompter["text"]; +}) { + return await ensureApiKeyFromEnvOrPrompt({ + config: params.config ?? {}, + provider: "minimax", + envLabel: "MINIMAX_API_KEY", + promptMessage: "Enter key", + normalize: (value) => value.trim(), + validate: () => undefined, + prompter: createPrompter({ select: params.select, text: params.text, note: params.note }), + secretInputMode: "ref", + setCredential: params.setCredential, + }); +} + async function runEnsureMinimaxApiKeyFlow(params: { confirmResult: boolean; textResult: string }) { process.env.MINIMAX_API_KEY = "env-key"; delete process.env.MINIMAX_OAUTH_TOKEN; @@ -229,7 +249,7 @@ describe("ensureApiKeyFromEnvOrPrompt", () => { const note = vi.fn(async () => undefined); const setCredential = vi.fn(async () => undefined); - const result = await ensureApiKeyFromEnvOrPrompt({ + const result = await ensureMinimaxApiKeyWithEnvRefPrompter({ config: { secrets: { providers: { @@ -241,13 +261,9 @@ describe("ensureApiKeyFromEnvOrPrompt", () => { }, }, }, - provider: "minimax", - envLabel: "MINIMAX_API_KEY", - promptMessage: "Enter key", - normalize: (value) => value.trim(), - validate: () => undefined, - prompter: createPrompter({ select, text, note }), - secretInputMode: "ref", + select, + text, + note, setCredential, }); @@ -271,15 +287,11 @@ describe("ensureApiKeyFromEnvOrPrompt", () => { const note = vi.fn(async () => undefined); const setCredential = vi.fn(async () => undefined); - const result = await ensureApiKeyFromEnvOrPrompt({ + const result = await ensureMinimaxApiKeyWithEnvRefPrompter({ config: {}, - provider: "minimax", - envLabel: "MINIMAX_API_KEY", - promptMessage: "Enter key", - normalize: (value) => value.trim(), - validate: () => undefined, - prompter: createPrompter({ select, text, note }), - secretInputMode: "ref", + select, + text, + note, setCredential, }); diff --git a/src/commands/doctor-legacy-config.ts b/src/commands/doctor-legacy-config.ts index 4d8117bd8..50c9f38eb 100644 --- a/src/commands/doctor-legacy-config.ts +++ b/src/commands/doctor-legacy-config.ts @@ -1,6 +1,8 @@ import { shouldMoveSingleAccountChannelKey } from "../channels/plugins/setup-helpers.js"; import type { OpenClawConfig } from "../config/config.js"; import { + formatSlackStreamingBooleanMigrationMessage, + formatSlackStreamModeMigrationMessage, resolveDiscordPreviewStreamMode, resolveSlackNativeStreaming, resolveSlackStreamingMode, @@ -175,13 +177,11 @@ export function normalizeCompatibilityConfigValues(cfg: OpenClawConfig): { const { streamMode: _ignored, ...rest } = updated; updated = rest; changed = true; - changes.push( - `Moved ${params.pathPrefix}.streamMode → ${params.pathPrefix}.streaming (${resolvedStreaming}).`, - ); + changes.push(formatSlackStreamModeMigrationMessage(params.pathPrefix, resolvedStreaming)); } if (typeof legacyStreaming === "boolean") { changes.push( - `Moved ${params.pathPrefix}.streaming (boolean) → ${params.pathPrefix}.nativeStreaming (${resolvedNativeStreaming}).`, + formatSlackStreamingBooleanMigrationMessage(params.pathPrefix, resolvedNativeStreaming), ); } else if (typeof legacyStreaming === "string" && legacyStreaming !== resolvedStreaming) { changes.push( diff --git a/src/commands/doctor-state-migrations.test.ts b/src/commands/doctor-state-migrations.test.ts index d00fc6628..24bbb4e8e 100644 --- a/src/commands/doctor-state-migrations.test.ts +++ b/src/commands/doctor-state-migrations.test.ts @@ -20,6 +20,12 @@ async function makeTempRoot() { return root; } +async function makeRootWithEmptyCfg() { + const root = await makeTempRoot(); + const cfg: OpenClawConfig = {}; + return { root, cfg }; +} + afterEach(async () => { resetAutoMigrateLegacyStateForTest(); resetAutoMigrateLegacyStateDirForTest(); @@ -129,6 +135,26 @@ function expectTargetAlreadyExistsWarning(result: StateDirMigrationResult, targe ]); } +function expectUnmigratedWithoutWarnings(result: StateDirMigrationResult) { + expect(result.migrated).toBe(false); + expect(result.warnings).toEqual([]); +} + +function writeLegacyAgentFiles(root: string, files: Record) { + const legacyAgentDir = path.join(root, "agent"); + fs.mkdirSync(legacyAgentDir, { recursive: true }); + for (const [fileName, content] of Object.entries(files)) { + fs.writeFileSync(path.join(legacyAgentDir, fileName), content, "utf-8"); + } + return legacyAgentDir; +} + +function ensureCredentialsDir(root: string) { + const oauthDir = path.join(root, "credentials"); + fs.mkdirSync(oauthDir, { recursive: true }); + return oauthDir; +} + describe("doctor legacy state migrations", () => { it("migrates legacy sessions into agents//sessions", async () => { const root = await makeTempRoot(); @@ -177,23 +203,17 @@ describe("doctor legacy state migrations", () => { }); it("migrates legacy agent dir with conflict fallback", async () => { - const root = await makeTempRoot(); - const cfg: OpenClawConfig = {}; - - const legacyAgentDir = path.join(root, "agent"); - fs.mkdirSync(legacyAgentDir, { recursive: true }); - fs.writeFileSync(path.join(legacyAgentDir, "foo.txt"), "legacy", "utf-8"); - fs.writeFileSync(path.join(legacyAgentDir, "baz.txt"), "legacy2", "utf-8"); + const { root, cfg } = await makeRootWithEmptyCfg(); + writeLegacyAgentFiles(root, { + "foo.txt": "legacy", + "baz.txt": "legacy2", + }); const targetAgentDir = path.join(root, "agents", "main", "agent"); fs.mkdirSync(targetAgentDir, { recursive: true }); fs.writeFileSync(path.join(targetAgentDir, "foo.txt"), "new", "utf-8"); - const detected = await detectLegacyStateMigrations({ - cfg, - env: { OPENCLAW_STATE_DIR: root } as NodeJS.ProcessEnv, - }); - await runLegacyStateMigrations({ detected, now: () => 123 }); + await detectAndRunMigrations({ root, cfg, now: () => 123 }); expect(fs.readFileSync(path.join(targetAgentDir, "baz.txt"), "utf-8")).toBe("legacy2"); const backupDir = path.join(root, "agents", "main", "agent.legacy-123"); @@ -201,12 +221,8 @@ describe("doctor legacy state migrations", () => { }); it("auto-migrates legacy agent dir on startup", async () => { - const root = await makeTempRoot(); - const cfg: OpenClawConfig = {}; - - const legacyAgentDir = path.join(root, "agent"); - fs.mkdirSync(legacyAgentDir, { recursive: true }); - fs.writeFileSync(path.join(legacyAgentDir, "auth.json"), "{}", "utf-8"); + const { root, cfg } = await makeRootWithEmptyCfg(); + writeLegacyAgentFiles(root, { "auth.json": "{}" }); const { result, log } = await runAutoMigrateLegacyStateWithLog({ root, cfg }); @@ -217,8 +233,7 @@ describe("doctor legacy state migrations", () => { }); it("auto-migrates legacy sessions on startup", async () => { - const root = await makeTempRoot(); - const cfg: OpenClawConfig = {}; + const { root, cfg } = await makeRootWithEmptyCfg(); const legacySessionsDir = writeLegacySessionsFixture({ root, sessions: { @@ -245,20 +260,13 @@ describe("doctor legacy state migrations", () => { }); it("migrates legacy WhatsApp auth files without touching oauth.json", async () => { - const root = await makeTempRoot(); - const cfg: OpenClawConfig = {}; - - const oauthDir = path.join(root, "credentials"); - fs.mkdirSync(oauthDir, { recursive: true }); + const { root, cfg } = await makeRootWithEmptyCfg(); + const oauthDir = ensureCredentialsDir(root); fs.writeFileSync(path.join(oauthDir, "oauth.json"), "{}", "utf-8"); fs.writeFileSync(path.join(oauthDir, "creds.json"), "{}", "utf-8"); fs.writeFileSync(path.join(oauthDir, "session-abc.json"), "{}", "utf-8"); - const detected = await detectLegacyStateMigrations({ - cfg, - env: { OPENCLAW_STATE_DIR: root } as NodeJS.ProcessEnv, - }); - await runLegacyStateMigrations({ detected, now: () => 123 }); + await detectAndRunMigrations({ root, cfg, now: () => 123 }); const target = path.join(oauthDir, "whatsapp", "default"); expect(fs.existsSync(path.join(target, "creds.json"))).toBe(true); @@ -268,11 +276,8 @@ describe("doctor legacy state migrations", () => { }); it("migrates legacy Telegram pairing allowFrom store to account-scoped default file", async () => { - const root = await makeTempRoot(); - const cfg: OpenClawConfig = {}; - - const oauthDir = path.join(root, "credentials"); - fs.mkdirSync(oauthDir, { recursive: true }); + const { root, cfg } = await makeRootWithEmptyCfg(); + const oauthDir = ensureCredentialsDir(root); fs.writeFileSync( path.join(oauthDir, "telegram-allowFrom.json"), JSON.stringify( @@ -359,8 +364,7 @@ describe("doctor legacy state migrations", () => { }); it("canonicalizes legacy main keys inside the target sessions store", async () => { - const root = await makeTempRoot(); - const cfg: OpenClawConfig = {}; + const { root, cfg } = await makeRootWithEmptyCfg(); const targetDir = path.join(root, "agents", "main", "sessions"); writeJson5(path.join(targetDir, "sessions.json"), { main: { sessionId: "legacy", updatedAt: 10 }, @@ -415,8 +419,7 @@ describe("doctor legacy state migrations", () => { }); it("auto-migrates when only target sessions contain legacy keys", async () => { - const root = await makeTempRoot(); - const cfg: OpenClawConfig = {}; + const { root, cfg } = await makeRootWithEmptyCfg(); const targetDir = path.join(root, "agents", "main", "sessions"); writeJson5(path.join(targetDir, "sessions.json"), { main: { sessionId: "legacy", updatedAt: 10 }, @@ -469,9 +472,7 @@ describe("doctor legacy state migrations", () => { fs.symlinkSync(path.join(targetDir, "agent"), path.join(legacyDir, "agent"), DIR_LINK_TYPE); const result = await runStateDirMigration(root); - - expect(result.migrated).toBe(false); - expect(result.warnings).toEqual([]); + expectUnmigratedWithoutWarnings(result); }); it("warns when legacy state dir is empty and target already exists", async () => { @@ -504,9 +505,7 @@ describe("doctor legacy state migrations", () => { ); const result = await runStateDirMigration(root); - - expect(result.migrated).toBe(false); - expect(result.warnings).toEqual([]); + expectUnmigratedWithoutWarnings(result); }); it("warns when legacy state dir symlink points outside the target tree", async () => { diff --git a/src/commands/openai-codex-oauth.test.ts b/src/commands/openai-codex-oauth.test.ts index cae7fb794..b3b3846f9 100644 --- a/src/commands/openai-codex-oauth.test.ts +++ b/src/commands/openai-codex-oauth.test.ts @@ -43,6 +43,18 @@ function createRuntime(): RuntimeEnv { }; } +async function runCodexOAuth(params: { isRemote: boolean }) { + const { prompter, spin } = createPrompter(); + const runtime = createRuntime(); + const result = await loginOpenAICodexOAuth({ + prompter, + runtime, + isRemote: params.isRemote, + openUrl: async () => {}, + }); + return { result, prompter, spin, runtime }; +} + describe("loginOpenAICodexOAuth", () => { beforeEach(() => { vi.clearAllMocks(); @@ -64,14 +76,7 @@ describe("loginOpenAICodexOAuth", () => { }); mocks.loginOpenAICodex.mockResolvedValue(creds); - const { prompter, spin } = createPrompter(); - const runtime = createRuntime(); - const result = await loginOpenAICodexOAuth({ - prompter, - runtime, - isRemote: false, - openUrl: async () => {}, - }); + const { result, spin, runtime } = await runCodexOAuth({ isRemote: false }); expect(result).toEqual(creds); expect(mocks.loginOpenAICodex).toHaveBeenCalledOnce(); @@ -124,14 +129,7 @@ describe("loginOpenAICodexOAuth", () => { }); mocks.loginOpenAICodex.mockResolvedValue(creds); - const { prompter } = createPrompter(); - const runtime = createRuntime(); - const result = await loginOpenAICodexOAuth({ - prompter, - runtime, - isRemote: false, - openUrl: async () => {}, - }); + const { result, prompter, runtime } = await runCodexOAuth({ isRemote: false }); expect(result).toEqual(creds); expect(mocks.loginOpenAICodex).toHaveBeenCalledOnce(); diff --git a/src/config/discord-preview-streaming.ts b/src/config/discord-preview-streaming.ts index 900a03f7a..79d7f8fd9 100644 --- a/src/config/discord-preview-streaming.ts +++ b/src/config/discord-preview-streaming.ts @@ -142,3 +142,17 @@ export function resolveSlackNativeStreaming( } return true; } + +export function formatSlackStreamModeMigrationMessage( + pathPrefix: string, + resolvedStreaming: string, +): string { + return `Moved ${pathPrefix}.streamMode → ${pathPrefix}.streaming (${resolvedStreaming}).`; +} + +export function formatSlackStreamingBooleanMigrationMessage( + pathPrefix: string, + resolvedNativeStreaming: boolean, +): string { + return `Moved ${pathPrefix}.streaming (boolean) → ${pathPrefix}.nativeStreaming (${resolvedNativeStreaming}).`; +} diff --git a/src/config/legacy.migrations.part-1.ts b/src/config/legacy.migrations.part-1.ts index d1d077caf..fe814ac72 100644 --- a/src/config/legacy.migrations.part-1.ts +++ b/src/config/legacy.migrations.part-1.ts @@ -1,4 +1,6 @@ import { + formatSlackStreamingBooleanMigrationMessage, + formatSlackStreamModeMigrationMessage, resolveDiscordPreviewStreamMode, resolveSlackNativeStreaming, resolveSlackStreamingMode, @@ -357,13 +359,11 @@ export const LEGACY_CONFIG_MIGRATIONS_PART_1: LegacyConfigMigration[] = [ params.entry.nativeStreaming = resolvedNativeStreaming; if (hasLegacyStreamMode) { delete params.entry.streamMode; - changes.push( - `Moved ${params.pathPrefix}.streamMode → ${params.pathPrefix}.streaming (${resolvedStreaming}).`, - ); + changes.push(formatSlackStreamModeMigrationMessage(params.pathPrefix, resolvedStreaming)); } if (typeof legacyStreaming === "boolean") { changes.push( - `Moved ${params.pathPrefix}.streaming (boolean) → ${params.pathPrefix}.nativeStreaming (${resolvedNativeStreaming}).`, + formatSlackStreamingBooleanMigrationMessage(params.pathPrefix, resolvedNativeStreaming), ); } else if (typeof legacyNativeStreaming !== "boolean" && hasLegacyStreamMode) { changes.push(`Set ${params.pathPrefix}.nativeStreaming → ${resolvedNativeStreaming}.`); diff --git a/src/config/plugin-auto-enable.test.ts b/src/config/plugin-auto-enable.test.ts index ebe2a859f..52b2c9cc1 100644 --- a/src/config/plugin-auto-enable.test.ts +++ b/src/config/plugin-auto-enable.test.ts @@ -20,15 +20,55 @@ function makeRegistry(plugins: Array<{ id: string; channels: string[] }>): Plugi }; } +function makeApnChannelConfig() { + return { channels: { apn: { someKey: "value" } } }; +} + +function makeBluebubblesAndImessageChannels() { + return { + bluebubbles: { serverUrl: "http://localhost:1234", password: "x" }, + imessage: { cliPath: "/usr/local/bin/imsg" }, + }; +} + +function applyWithSlackConfig(extra?: { plugins?: { allow?: string[] } }) { + return applyPluginAutoEnable({ + config: { + channels: { slack: { botToken: "x" } }, + ...(extra?.plugins ? { plugins: extra.plugins } : {}), + }, + env: {}, + }); +} + +function applyWithApnChannelConfig(extra?: { + plugins?: { entries?: Record }; +}) { + return applyPluginAutoEnable({ + config: { + ...makeApnChannelConfig(), + ...(extra?.plugins ? { plugins: extra.plugins } : {}), + }, + env: {}, + manifestRegistry: makeRegistry([{ id: "apn-channel", channels: ["apn"] }]), + }); +} + +function applyWithBluebubblesImessageConfig(extra?: { + plugins?: { entries?: Record; deny?: string[] }; +}) { + return applyPluginAutoEnable({ + config: { + channels: makeBluebubblesAndImessageChannels(), + ...(extra?.plugins ? { plugins: extra.plugins } : {}), + }, + env: {}, + }); +} + describe("applyPluginAutoEnable", () => { it("auto-enables built-in channels and appends to existing allowlist", () => { - const result = applyPluginAutoEnable({ - config: { - channels: { slack: { botToken: "x" } }, - plugins: { allow: ["telegram"] }, - }, - env: {}, - }); + const result = applyWithSlackConfig({ plugins: { allow: ["telegram"] } }); expect(result.config.channels?.slack?.enabled).toBe(true); expect(result.config.plugins?.entries?.slack).toBeUndefined(); @@ -37,12 +77,7 @@ describe("applyPluginAutoEnable", () => { }); it("does not create plugins.allow when allowlist is unset", () => { - const result = applyPluginAutoEnable({ - config: { - channels: { slack: { botToken: "x" } }, - }, - env: {}, - }); + const result = applyWithSlackConfig(); expect(result.config.channels?.slack?.enabled).toBe(true); expect(result.config.plugins?.allow).toBeUndefined(); @@ -187,13 +222,7 @@ describe("applyPluginAutoEnable", () => { // Reproduces: https://github.com/openclaw/openclaw/issues/25261 // Plugin "apn-channel" declares channels: ["apn"]. Doctor must write // plugins.entries["apn-channel"], not plugins.entries["apn"]. - const result = applyPluginAutoEnable({ - config: { - channels: { apn: { someKey: "value" } }, - }, - env: {}, - manifestRegistry: makeRegistry([{ id: "apn-channel", channels: ["apn"] }]), - }); + const result = applyWithApnChannelConfig(); expect(result.config.plugins?.entries?.["apn-channel"]?.enabled).toBe(true); expect(result.config.plugins?.entries?.["apn"]).toBeUndefined(); @@ -201,26 +230,16 @@ describe("applyPluginAutoEnable", () => { }); it("does not double-enable when plugin is already enabled under its plugin id", () => { - const result = applyPluginAutoEnable({ - config: { - channels: { apn: { someKey: "value" } }, - plugins: { entries: { "apn-channel": { enabled: true } } }, - }, - env: {}, - manifestRegistry: makeRegistry([{ id: "apn-channel", channels: ["apn"] }]), + const result = applyWithApnChannelConfig({ + plugins: { entries: { "apn-channel": { enabled: true } } }, }); expect(result.changes).toEqual([]); }); it("respects explicit disable of the plugin by its plugin id", () => { - const result = applyPluginAutoEnable({ - config: { - channels: { apn: { someKey: "value" } }, - plugins: { entries: { "apn-channel": { enabled: false } } }, - }, - env: {}, - manifestRegistry: makeRegistry([{ id: "apn-channel", channels: ["apn"] }]), + const result = applyWithApnChannelConfig({ + plugins: { entries: { "apn-channel": { enabled: false } } }, }); expect(result.config.plugins?.entries?.["apn-channel"]?.enabled).toBe(false); @@ -243,15 +262,7 @@ describe("applyPluginAutoEnable", () => { describe("preferOver channel prioritization", () => { it("prefers bluebubbles: skips imessage auto-configure when both are configured", () => { - const result = applyPluginAutoEnable({ - config: { - channels: { - bluebubbles: { serverUrl: "http://localhost:1234", password: "x" }, - imessage: { cliPath: "/usr/local/bin/imsg" }, - }, - }, - env: {}, - }); + const result = applyWithBluebubblesImessageConfig(); expect(result.config.plugins?.entries?.bluebubbles?.enabled).toBe(true); expect(result.config.plugins?.entries?.imessage?.enabled).toBeUndefined(); @@ -262,15 +273,8 @@ describe("applyPluginAutoEnable", () => { }); it("keeps imessage enabled if already explicitly enabled (non-destructive)", () => { - const result = applyPluginAutoEnable({ - config: { - channels: { - bluebubbles: { serverUrl: "http://localhost:1234", password: "x" }, - imessage: { cliPath: "/usr/local/bin/imsg" }, - }, - plugins: { entries: { imessage: { enabled: true } } }, - }, - env: {}, + const result = applyWithBluebubblesImessageConfig({ + plugins: { entries: { imessage: { enabled: true } } }, }); expect(result.config.plugins?.entries?.bluebubbles?.enabled).toBe(true); @@ -278,15 +282,8 @@ describe("applyPluginAutoEnable", () => { }); it("allows imessage auto-configure when bluebubbles is explicitly disabled", () => { - const result = applyPluginAutoEnable({ - config: { - channels: { - bluebubbles: { serverUrl: "http://localhost:1234", password: "x" }, - imessage: { cliPath: "/usr/local/bin/imsg" }, - }, - plugins: { entries: { bluebubbles: { enabled: false } } }, - }, - env: {}, + const result = applyWithBluebubblesImessageConfig({ + plugins: { entries: { bluebubbles: { enabled: false } } }, }); expect(result.config.plugins?.entries?.bluebubbles?.enabled).toBe(false); @@ -295,15 +292,8 @@ describe("applyPluginAutoEnable", () => { }); it("allows imessage auto-configure when bluebubbles is in deny list", () => { - const result = applyPluginAutoEnable({ - config: { - channels: { - bluebubbles: { serverUrl: "http://localhost:1234", password: "x" }, - imessage: { cliPath: "/usr/local/bin/imsg" }, - }, - plugins: { deny: ["bluebubbles"] }, - }, - env: {}, + const result = applyWithBluebubblesImessageConfig({ + plugins: { deny: ["bluebubbles"] }, }); expect(result.config.plugins?.entries?.bluebubbles?.enabled).toBeUndefined(); diff --git a/src/config/sessions.test.ts b/src/config/sessions.test.ts index 7c77ffac2..2ea413302 100644 --- a/src/config/sessions.test.ts +++ b/src/config/sessions.test.ts @@ -684,12 +684,7 @@ describe("sessions", () => { }); const createDeferred = () => { - let resolve!: (value: T) => void; - let reject!: (reason?: unknown) => void; - const promise = new Promise((res, rej) => { - resolve = res; - reject = rej; - }); + const { promise, resolve, reject } = Promise.withResolvers(); return { promise, resolve, reject }; }; const firstStarted = createDeferred(); diff --git a/src/cron/isolated-agent.delivery.test-helpers.ts b/src/cron/isolated-agent.delivery.test-helpers.ts index 727737549..fe6dad727 100644 --- a/src/cron/isolated-agent.delivery.test-helpers.ts +++ b/src/cron/isolated-agent.delivery.test-helpers.ts @@ -1,4 +1,4 @@ -import { vi } from "vitest"; +import { expect, vi } from "vitest"; import { runEmbeddedPiAgent } from "../agents/pi-embedded.js"; import type { CliDeps } from "../cli/deps.js"; import { runCronIsolatedAgentTurn } from "./isolated-agent.js"; @@ -30,6 +30,20 @@ export function mockAgentPayloads( }); } +export function expectDirectTelegramDelivery( + deps: CliDeps, + params: { chatId: string; text: string; messageThreadId?: number }, +) { + expect(deps.sendMessageTelegram).toHaveBeenCalledTimes(1); + expect(deps.sendMessageTelegram).toHaveBeenCalledWith( + params.chatId, + params.text, + expect.objectContaining( + params.messageThreadId === undefined ? {} : { messageThreadId: params.messageThreadId }, + ), + ); +} + export async function runTelegramAnnounceTurn(params: { home: string; storePath: string; diff --git a/src/cron/isolated-agent.direct-delivery-forum-topics.test.ts b/src/cron/isolated-agent.direct-delivery-forum-topics.test.ts index d680a8fc7..7f7df2094 100644 --- a/src/cron/isolated-agent.direct-delivery-forum-topics.test.ts +++ b/src/cron/isolated-agent.direct-delivery-forum-topics.test.ts @@ -3,6 +3,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { runSubagentAnnounceFlow } from "../agents/subagent-announce.js"; import { createCliDeps, + expectDirectTelegramDelivery, mockAgentPayloads, runTelegramAnnounceTurn, } from "./isolated-agent.delivery.test-helpers.js"; @@ -30,14 +31,11 @@ describe("runCronIsolatedAgentTurn forum topic delivery", () => { expect(res.status).toBe("ok"); expect(res.delivered).toBe(true); expect(runSubagentAnnounceFlow).not.toHaveBeenCalled(); - expect(deps.sendMessageTelegram).toHaveBeenCalledTimes(1); - expect(deps.sendMessageTelegram).toHaveBeenCalledWith( - "123", - "forum message", - expect.objectContaining({ - messageThreadId: 42, - }), - ); + expectDirectTelegramDelivery(deps, { + chatId: "123", + text: "forum message", + messageThreadId: 42, + }); vi.clearAllMocks(); mockAgentPayloads([{ text: "plain message" }]); diff --git a/src/cron/isolated-agent.skips-delivery-without-whatsapp-recipient-besteffortdeliver-true.test.ts b/src/cron/isolated-agent.skips-delivery-without-whatsapp-recipient-besteffortdeliver-true.test.ts index 883c197bd..b82dc1f91 100644 --- a/src/cron/isolated-agent.skips-delivery-without-whatsapp-recipient-besteffortdeliver-true.test.ts +++ b/src/cron/isolated-agent.skips-delivery-without-whatsapp-recipient-besteffortdeliver-true.test.ts @@ -6,6 +6,7 @@ import { runSubagentAnnounceFlow } from "../agents/subagent-announce.js"; import type { CliDeps } from "../cli/deps.js"; import { createCliDeps, + expectDirectTelegramDelivery, mockAgentPayloads, runTelegramAnnounceTurn, } from "./isolated-agent.delivery.test-helpers.js"; @@ -262,14 +263,11 @@ describe("runCronIsolatedAgentTurn", () => { expect(res.status).toBe("ok"); expect(res.delivered).toBe(true); expect(runSubagentAnnounceFlow).not.toHaveBeenCalled(); - expect(deps.sendMessageTelegram).toHaveBeenCalledTimes(1); - expect(deps.sendMessageTelegram).toHaveBeenCalledWith( - "123", - "Final weather summary", - expect.objectContaining({ - messageThreadId: 42, - }), - ); + expectDirectTelegramDelivery(deps, { + chatId: "123", + text: "Final weather summary", + messageThreadId: 42, + }); }); }); diff --git a/src/cron/isolated-agent.subagent-model.test.ts b/src/cron/isolated-agent.subagent-model.test.ts index ea651f5d8..f9311a6ef 100644 --- a/src/cron/isolated-agent.subagent-model.test.ts +++ b/src/cron/isolated-agent.subagent-model.test.ts @@ -2,7 +2,7 @@ import "./isolated-agent.mocks.js"; import fs from "node:fs/promises"; import path from "node:path"; import { beforeEach, describe, expect, it, vi } from "vitest"; -import { withTempHome as withTempHomeBase } from "../../test/helpers/temp-home.js"; +import { withTempHome as withTempHomeHelper } from "../../test/helpers/temp-home.js"; import { loadModelCatalog } from "../agents/model-catalog.js"; import { runEmbeddedPiAgent } from "../agents/pi-embedded.js"; import type { CliDeps } from "../cli/deps.js"; @@ -11,7 +11,7 @@ import { runCronIsolatedAgentTurn } from "./isolated-agent.js"; import type { CronJob } from "./types.js"; async function withTempHome(fn: (home: string) => Promise): Promise { - return withTempHomeBase(fn, { prefix: "openclaw-cron-submodel-" }); + return withTempHomeHelper(fn, { prefix: "openclaw-cron-submodel-" }); } async function writeSessionStore(home: string) { diff --git a/src/cron/normalize.test.ts b/src/cron/normalize.test.ts index b75a23aca..6f34c85eb 100644 --- a/src/cron/normalize.test.ts +++ b/src/cron/normalize.test.ts @@ -20,32 +20,74 @@ function expectNormalizedAtSchedule(scheduleInput: Record) { expect(schedule.at).toBe(new Date(Date.parse("2026-01-12T18:00:00Z")).toISOString()); } +function expectAnnounceDeliveryTarget( + delivery: Record, + params: { channel: string; to: string }, +): void { + expect(delivery.mode).toBe("announce"); + expect(delivery.channel).toBe(params.channel); + expect(delivery.to).toBe(params.to); +} + +function expectPayloadDeliveryHintsCleared(payload: Record): void { + expect(payload.channel).toBeUndefined(); + expect(payload.deliver).toBeUndefined(); +} + +function normalizeIsolatedAgentTurnCreateJob(params: { + name: string; + payload?: Record; + delivery?: Record; +}): Record { + return normalizeCronJobCreate({ + name: params.name, + enabled: true, + schedule: { kind: "cron", expr: "* * * * *" }, + sessionTarget: "isolated", + wakeMode: "now", + payload: { + kind: "agentTurn", + message: "hi", + ...params.payload, + }, + ...(params.delivery ? { delivery: params.delivery } : {}), + }) as unknown as Record; +} + +function normalizeMainSystemEventCreateJob(params: { + name: string; + schedule: Record; +}): Record { + return normalizeCronJobCreate({ + name: params.name, + enabled: true, + schedule: params.schedule, + sessionTarget: "main", + wakeMode: "next-heartbeat", + payload: { + kind: "systemEvent", + text: "tick", + }, + }) as unknown as Record; +} + describe("normalizeCronJobCreate", () => { it("maps legacy payload.provider to payload.channel and strips provider", () => { - const normalized = normalizeCronJobCreate({ + const normalized = normalizeIsolatedAgentTurnCreateJob({ name: "legacy", - enabled: true, - schedule: { kind: "cron", expr: "* * * * *" }, - sessionTarget: "isolated", - wakeMode: "now", payload: { - kind: "agentTurn", - message: "hi", deliver: true, provider: " TeLeGrAm ", to: "7200373102", }, - }) as unknown as Record; + }); const payload = normalized.payload as Record; - expect(payload.channel).toBeUndefined(); - expect(payload.deliver).toBeUndefined(); + expectPayloadDeliveryHintsCleared(payload); expect("provider" in payload).toBe(false); const delivery = normalized.delivery as Record; - expect(delivery.mode).toBe("announce"); - expect(delivery.channel).toBe("telegram"); - expect(delivery.to).toBe("7200373102"); + expectAnnounceDeliveryTarget(delivery, { channel: "telegram", to: "7200373102" }); }); it("trims agentId and drops null", () => { @@ -105,29 +147,20 @@ describe("normalizeCronJobCreate", () => { }); it("canonicalizes payload.channel casing", () => { - const normalized = normalizeCronJobCreate({ + const normalized = normalizeIsolatedAgentTurnCreateJob({ name: "legacy provider", - enabled: true, - schedule: { kind: "cron", expr: "* * * * *" }, - sessionTarget: "isolated", - wakeMode: "now", payload: { - kind: "agentTurn", - message: "hi", deliver: true, channel: "Telegram", to: "7200373102", }, - }) as unknown as Record; + }); const payload = normalized.payload as Record; - expect(payload.channel).toBeUndefined(); - expect(payload.deliver).toBeUndefined(); + expectPayloadDeliveryHintsCleared(payload); const delivery = normalized.delivery as Record; - expect(delivery.mode).toBe("announce"); - expect(delivery.channel).toBe("telegram"); - expect(delivery.to).toBe("7200373102"); + expectAnnounceDeliveryTarget(delivery, { channel: "telegram", to: "7200373102" }); }); it("coerces ISO schedule.at to normalized ISO (UTC)", () => { @@ -139,17 +172,10 @@ describe("normalizeCronJobCreate", () => { }); it("migrates legacy schedule.cron into schedule.expr", () => { - const normalized = normalizeCronJobCreate({ + const normalized = normalizeMainSystemEventCreateJob({ name: "legacy-cron-field", - enabled: true, schedule: { kind: "cron", cron: "*/10 * * * *", tz: "UTC" }, - sessionTarget: "main", - wakeMode: "next-heartbeat", - payload: { - kind: "systemEvent", - text: "tick", - }, - }) as unknown as Record; + }); const schedule = normalized.schedule as Record; expect(schedule.kind).toBe("cron"); @@ -158,34 +184,20 @@ describe("normalizeCronJobCreate", () => { }); it("defaults cron stagger for recurring top-of-hour schedules", () => { - const normalized = normalizeCronJobCreate({ + const normalized = normalizeMainSystemEventCreateJob({ name: "hourly", - enabled: true, schedule: { kind: "cron", expr: "0 * * * *", tz: "UTC" }, - sessionTarget: "main", - wakeMode: "next-heartbeat", - payload: { - kind: "systemEvent", - text: "tick", - }, - }) as unknown as Record; + }); const schedule = normalized.schedule as Record; expect(schedule.staggerMs).toBe(DEFAULT_TOP_OF_HOUR_STAGGER_MS); }); it("preserves explicit exact cron schedule", () => { - const normalized = normalizeCronJobCreate({ + const normalized = normalizeMainSystemEventCreateJob({ name: "exact", - enabled: true, schedule: { kind: "cron", expr: "0 * * * *", tz: "UTC", staggerMs: 0 }, - sessionTarget: "main", - wakeMode: "next-heartbeat", - payload: { - kind: "systemEvent", - text: "tick", - }, - }) as unknown as Record; + }); const schedule = normalized.schedule as Record; expect(schedule.staggerMs).toBe(0); @@ -208,69 +220,43 @@ describe("normalizeCronJobCreate", () => { }); it("normalizes delivery mode and channel", () => { - const normalized = normalizeCronJobCreate({ + const normalized = normalizeIsolatedAgentTurnCreateJob({ name: "delivery", - enabled: true, - schedule: { kind: "cron", expr: "* * * * *" }, - sessionTarget: "isolated", - wakeMode: "now", - payload: { - kind: "agentTurn", - message: "hi", - }, delivery: { mode: " ANNOUNCE ", channel: " TeLeGrAm ", to: " 7200373102 ", }, - }) as unknown as Record; + }); const delivery = normalized.delivery as Record; - expect(delivery.mode).toBe("announce"); - expect(delivery.channel).toBe("telegram"); - expect(delivery.to).toBe("7200373102"); + expectAnnounceDeliveryTarget(delivery, { channel: "telegram", to: "7200373102" }); }); it("normalizes delivery accountId and strips blanks", () => { - const normalized = normalizeCronJobCreate({ + const normalized = normalizeIsolatedAgentTurnCreateJob({ name: "delivery account", - enabled: true, - schedule: { kind: "cron", expr: "* * * * *" }, - sessionTarget: "isolated", - wakeMode: "now", - payload: { - kind: "agentTurn", - message: "hi", - }, delivery: { mode: "announce", channel: "telegram", to: "-1003816714067", accountId: " coordinator ", }, - }) as unknown as Record; + }); const delivery = normalized.delivery as Record; expect(delivery.accountId).toBe("coordinator"); }); it("strips empty accountId from delivery", () => { - const normalized = normalizeCronJobCreate({ + const normalized = normalizeIsolatedAgentTurnCreateJob({ name: "empty account", - enabled: true, - schedule: { kind: "cron", expr: "* * * * *" }, - sessionTarget: "isolated", - wakeMode: "now", - payload: { - kind: "agentTurn", - message: "hi", - }, delivery: { mode: "announce", channel: "telegram", accountId: " ", }, - }) as unknown as Record; + }); const delivery = normalized.delivery as Record; expect("accountId" in delivery).toBe(false); @@ -296,15 +282,9 @@ describe("normalizeCronJobCreate", () => { }); it("defaults isolated agentTurn delivery to announce", () => { - const normalized = normalizeCronJobCreate({ + const normalized = normalizeIsolatedAgentTurnCreateJob({ name: "default-announce", - enabled: true, - schedule: { kind: "cron", expr: "* * * * *" }, - payload: { - kind: "agentTurn", - message: "hi", - }, - }) as unknown as Record; + }); const delivery = normalized.delivery as Record; expect(delivery.mode).toBe("announce"); @@ -326,9 +306,7 @@ describe("normalizeCronJobCreate", () => { }) as unknown as Record; const delivery = normalized.delivery as Record; - expect(delivery.mode).toBe("announce"); - expect(delivery.channel).toBe("telegram"); - expect(delivery.to).toBe("7200373102"); + expectAnnounceDeliveryTarget(delivery, { channel: "telegram", to: "7200373102" }); expect(delivery.bestEffort).toBe(true); }); diff --git a/src/cron/service.jobs.test.ts b/src/cron/service.jobs.test.ts index 9bd31726f..523f27102 100644 --- a/src/cron/service.jobs.test.ts +++ b/src/cron/service.jobs.test.ts @@ -4,6 +4,13 @@ import type { CronServiceState } from "./service/state.js"; import { DEFAULT_TOP_OF_HOUR_STAGGER_MS } from "./stagger.js"; import type { CronJob, CronJobPatch } from "./types.js"; +function expectCronStaggerMs(job: CronJob, expected: number): void { + expect(job.schedule.kind).toBe("cron"); + if (job.schedule.kind === "cron") { + expect(job.schedule.staggerMs).toBe(expected); + } +} + describe("applyJobPatch", () => { const createIsolatedAgentTurnJob = ( id: string, @@ -481,10 +488,7 @@ describe("cron stagger defaults", () => { payload: { kind: "systemEvent", text: "tick" }, }); - expect(job.schedule.kind).toBe("cron"); - if (job.schedule.kind === "cron") { - expect(job.schedule.staggerMs).toBe(DEFAULT_TOP_OF_HOUR_STAGGER_MS); - } + expectCronStaggerMs(job, DEFAULT_TOP_OF_HOUR_STAGGER_MS); }); it("keeps exact schedules when staggerMs is explicitly 0", () => { @@ -500,10 +504,7 @@ describe("cron stagger defaults", () => { payload: { kind: "systemEvent", text: "tick" }, }); - expect(job.schedule.kind).toBe("cron"); - if (job.schedule.kind === "cron") { - expect(job.schedule.staggerMs).toBe(0); - } + expectCronStaggerMs(job, 0); }); it("preserves existing stagger when editing cron expression without stagger", () => { diff --git a/src/cron/service.runs-one-shot-main-job-disables-it.test.ts b/src/cron/service.runs-one-shot-main-job-disables-it.test.ts index bcf5b919c..c36da9fd5 100644 --- a/src/cron/service.runs-one-shot-main-job-disables-it.test.ts +++ b/src/cron/service.runs-one-shot-main-job-disables-it.test.ts @@ -333,6 +333,20 @@ async function runIsolatedAnnounceJobAndWait(params: { return job; } +async function runIsolatedAnnounceScenario(params: { + cron: CronService; + events: ReturnType; + name: string; + status?: "ok" | "error"; +}) { + await runIsolatedAnnounceJobAndWait({ + cron: params.cron, + events: params.events, + name: params.name, + status: params.status ?? "ok", + }); +} + async function addWakeModeNowMainSystemEventJob( cron: CronService, options?: { name?: string; agentId?: string; sessionKey?: string }, @@ -349,6 +363,82 @@ async function addWakeModeNowMainSystemEventJob( }); } +async function addMainOneShotHelloJob( + cron: CronService, + params: { atMs: number; name: string; deleteAfterRun?: boolean }, +) { + return cron.add({ + name: params.name, + enabled: true, + ...(params.deleteAfterRun === undefined ? {} : { deleteAfterRun: params.deleteAfterRun }), + schedule: { kind: "at", at: new Date(params.atMs).toISOString() }, + sessionTarget: "main", + wakeMode: "now", + payload: { kind: "systemEvent", text: "hello" }, + }); +} + +function expectMainSystemEventPosted(enqueueSystemEvent: unknown, text: string) { + expect(enqueueSystemEvent).toHaveBeenCalledWith( + text, + expect.objectContaining({ agentId: undefined }), + ); +} + +async function stopCronAndCleanup(cron: CronService, store: { cleanup: () => Promise }) { + cron.stop(); + await store.cleanup(); +} + +function createStartedCronService( + storePath: string, + runIsolatedAgentJob?: CronServiceDeps["runIsolatedAgentJob"], +) { + return new CronService({ + storePath, + cronEnabled: true, + log: noopLogger, + enqueueSystemEvent: vi.fn(), + requestHeartbeatNow: vi.fn(), + runIsolatedAgentJob: runIsolatedAgentJob ?? vi.fn(async () => ({ status: "ok" as const })), + }); +} + +async function createMainOneShotJobHarness(params: { name: string; deleteAfterRun?: boolean }) { + const harness = await createMainOneShotHarness(); + const atMs = Date.parse("2025-12-13T00:00:02.000Z"); + const job = await addMainOneShotHelloJob(harness.cron, { + atMs, + name: params.name, + deleteAfterRun: params.deleteAfterRun, + }); + return { ...harness, atMs, job }; +} + +async function loadLegacyDeliveryMigrationByPayload(params: { + id: string; + payload: { provider?: string; channel?: string }; +}) { + const rawJob = createLegacyDeliveryMigrationJob(params); + return loadLegacyDeliveryMigration(rawJob); +} + +async function expectNoMainSummaryForIsolatedRun(params: { + runIsolatedAgentJob: CronServiceDeps["runIsolatedAgentJob"]; + name: string; +}) { + const { store, cron, enqueueSystemEvent, requestHeartbeatNow, events } = + await createIsolatedAnnounceHarness(params.runIsolatedAgentJob); + await runIsolatedAnnounceScenario({ + cron, + events, + name: params.name, + }); + expect(enqueueSystemEvent).not.toHaveBeenCalled(); + expect(requestHeartbeatNow).not.toHaveBeenCalled(); + await stopCronAndCleanup(cron, store); +} + function createLegacyDeliveryMigrationJob(options: { id: string; payload: { provider?: string; channel?: string }; @@ -378,14 +468,7 @@ async function loadLegacyDeliveryMigration(rawJob: Record) { const store = await makeStorePath(); writeStoreFile(store.storePath, { version: 1, jobs: [rawJob] }); - const cron = new CronService({ - storePath: store.storePath, - cronEnabled: true, - log: noopLogger, - enqueueSystemEvent: vi.fn(), - requestHeartbeatNow: vi.fn(), - runIsolatedAgentJob: vi.fn(async () => ({ status: "ok" as const })), - }); + const cron = createStartedCronService(store.storePath); await cron.start(); const jobs = await cron.list({ includeDisabled: true }); const job = jobs.find((j) => j.id === rawJob.id); @@ -394,18 +477,11 @@ async function loadLegacyDeliveryMigration(rawJob: Record) { describe("CronService", () => { it("runs a one-shot main job and disables it after success when requested", async () => { - const { store, cron, enqueueSystemEvent, requestHeartbeatNow, events } = - await createMainOneShotHarness(); - const atMs = Date.parse("2025-12-13T00:00:02.000Z"); - const job = await cron.add({ - name: "one-shot hello", - enabled: true, - deleteAfterRun: false, - schedule: { kind: "at", at: new Date(atMs).toISOString() }, - sessionTarget: "main", - wakeMode: "now", - payload: { kind: "systemEvent", text: "hello" }, - }); + const { store, cron, enqueueSystemEvent, requestHeartbeatNow, events, atMs, job } = + await createMainOneShotJobHarness({ + name: "one-shot hello", + deleteAfterRun: false, + }); expect(job.state.nextRunAtMs).toBe(atMs); @@ -416,29 +492,18 @@ describe("CronService", () => { const jobs = await cron.list({ includeDisabled: true }); const updated = jobs.find((j) => j.id === job.id); expect(updated?.enabled).toBe(false); - expect(enqueueSystemEvent).toHaveBeenCalledWith( - "hello", - expect.objectContaining({ agentId: undefined }), - ); + expectMainSystemEventPosted(enqueueSystemEvent, "hello"); expect(requestHeartbeatNow).toHaveBeenCalled(); await cron.list({ includeDisabled: true }); - cron.stop(); - await store.cleanup(); + await stopCronAndCleanup(cron, store); }); it("runs a one-shot job and deletes it after success by default", async () => { - const { store, cron, enqueueSystemEvent, requestHeartbeatNow, events } = - await createMainOneShotHarness(); - const atMs = Date.parse("2025-12-13T00:00:02.000Z"); - const job = await cron.add({ - name: "one-shot delete", - enabled: true, - schedule: { kind: "at", at: new Date(atMs).toISOString() }, - sessionTarget: "main", - wakeMode: "now", - payload: { kind: "systemEvent", text: "hello" }, - }); + const { store, cron, enqueueSystemEvent, requestHeartbeatNow, events, job } = + await createMainOneShotJobHarness({ + name: "one-shot delete", + }); vi.setSystemTime(new Date("2025-12-13T00:00:02.000Z")); await vi.runOnlyPendingTimersAsync(); @@ -446,14 +511,10 @@ describe("CronService", () => { const jobs = await cron.list({ includeDisabled: true }); expect(jobs.find((j) => j.id === job.id)).toBeUndefined(); - expect(enqueueSystemEvent).toHaveBeenCalledWith( - "hello", - expect.objectContaining({ agentId: undefined }), - ); + expectMainSystemEventPosted(enqueueSystemEvent, "hello"); expect(requestHeartbeatNow).toHaveBeenCalled(); - cron.stop(); - await store.cleanup(); + await stopCronAndCleanup(cron, store); }); it("wakeMode now waits for heartbeat completion when available", async () => { @@ -491,10 +552,7 @@ describe("CronService", () => { expect(runHeartbeatOnce).toHaveBeenCalledTimes(1); expect(requestHeartbeatNow).not.toHaveBeenCalled(); - expect(enqueueSystemEvent).toHaveBeenCalledWith( - "hello", - expect.objectContaining({ agentId: undefined }), - ); + expectMainSystemEventPosted(enqueueSystemEvent, "hello"); expect(job.state.runningAtMs).toBeTypeOf("number"); if (typeof resolveHeartbeat === "function") { @@ -505,8 +563,7 @@ describe("CronService", () => { expect(job.state.lastStatus).toBe("ok"); expect(job.state.lastDurationMs).toBeGreaterThan(0); - cron.stop(); - await store.cleanup(); + await stopCronAndCleanup(cron, store); }); it("rejects sessionTarget main for non-default agents at creation time", async () => { @@ -525,8 +582,7 @@ describe("CronService", () => { }), ).rejects.toThrow('cron: sessionTarget "main" is only valid for the default agent'); - cron.stop(); - await store.cleanup(); + await stopCronAndCleanup(cron, store); }); it("wakeMode now falls back to queued heartbeat when main lane stays busy", async () => { @@ -567,23 +623,18 @@ describe("CronService", () => { expect(job.state.lastError).toBeUndefined(); await cron.list({ includeDisabled: true }); - cron.stop(); - await store.cleanup(); + await stopCronAndCleanup(cron, store); }); it("runs an isolated job and posts summary to main", async () => { const runIsolatedAgentJob = vi.fn(async () => ({ status: "ok" as const, summary: "done" })); const { store, cron, enqueueSystemEvent, requestHeartbeatNow, events } = await createIsolatedAnnounceHarness(runIsolatedAgentJob); - await runIsolatedAnnounceJobAndWait({ cron, events, name: "weekly", status: "ok" }); + await runIsolatedAnnounceScenario({ cron, events, name: "weekly" }); expect(runIsolatedAgentJob).toHaveBeenCalledTimes(1); - expect(enqueueSystemEvent).toHaveBeenCalledWith( - "Cron: done", - expect.objectContaining({ agentId: undefined }), - ); + expectMainSystemEventPosted(enqueueSystemEvent, "Cron: done"); expect(requestHeartbeatNow).toHaveBeenCalled(); - cron.stop(); - await store.cleanup(); + await stopCronAndCleanup(cron, store); }); it("does not post isolated summary to main when run already delivered output", async () => { @@ -592,19 +643,11 @@ describe("CronService", () => { summary: "done", delivered: true, })); - const { store, cron, enqueueSystemEvent, requestHeartbeatNow, events } = - await createIsolatedAnnounceHarness(runIsolatedAgentJob); - await runIsolatedAnnounceJobAndWait({ - cron, - events, + await expectNoMainSummaryForIsolatedRun({ + runIsolatedAgentJob, name: "weekly delivered", - status: "ok", }); expect(runIsolatedAgentJob).toHaveBeenCalledTimes(1); - expect(enqueueSystemEvent).not.toHaveBeenCalled(); - expect(requestHeartbeatNow).not.toHaveBeenCalled(); - cron.stop(); - await store.cleanup(); }); it("does not post isolated summary to main when announce delivery was attempted", async () => { @@ -614,27 +657,18 @@ describe("CronService", () => { delivered: false, deliveryAttempted: true, })); - const { store, cron, enqueueSystemEvent, requestHeartbeatNow, events } = - await createIsolatedAnnounceHarness(runIsolatedAgentJob); - await runIsolatedAnnounceJobAndWait({ - cron, - events, + await expectNoMainSummaryForIsolatedRun({ + runIsolatedAgentJob, name: "weekly attempted", - status: "ok", }); expect(runIsolatedAgentJob).toHaveBeenCalledTimes(1); - expect(enqueueSystemEvent).not.toHaveBeenCalled(); - expect(requestHeartbeatNow).not.toHaveBeenCalled(); - cron.stop(); - await store.cleanup(); }); it("migrates legacy payload.provider to payload.channel on load", async () => { - const rawJob = createLegacyDeliveryMigrationJob({ + const { store, cron, job } = await loadLegacyDeliveryMigrationByPayload({ id: "legacy-1", payload: { provider: " TeLeGrAm " }, }); - const { store, cron, job } = await loadLegacyDeliveryMigration(rawJob); // Legacy delivery fields are migrated to the top-level delivery object const delivery = job?.delivery as unknown as Record; expect(delivery?.channel).toBe("telegram"); @@ -642,22 +676,19 @@ describe("CronService", () => { expect("provider" in payload).toBe(false); expect("channel" in payload).toBe(false); - cron.stop(); - await store.cleanup(); + await stopCronAndCleanup(cron, store); }); it("canonicalizes payload.channel casing on load", async () => { - const rawJob = createLegacyDeliveryMigrationJob({ + const { store, cron, job } = await loadLegacyDeliveryMigrationByPayload({ id: "legacy-2", payload: { channel: "Telegram" }, }); - const { store, cron, job } = await loadLegacyDeliveryMigration(rawJob); // Legacy delivery fields are migrated to the top-level delivery object const delivery = job?.delivery as unknown as Record; expect(delivery?.channel).toBe("telegram"); - cron.stop(); - await store.cleanup(); + await stopCronAndCleanup(cron, store); }); it("posts last output to main even when isolated job errors", async () => { @@ -675,13 +706,9 @@ describe("CronService", () => { status: "error", }); - expect(enqueueSystemEvent).toHaveBeenCalledWith( - "Cron (error): last output", - expect.objectContaining({ agentId: undefined }), - ); + expectMainSystemEventPosted(enqueueSystemEvent, "Cron (error): last output"); expect(requestHeartbeatNow).toHaveBeenCalled(); - cron.stop(); - await store.cleanup(); + await stopCronAndCleanup(cron, store); }); it("does not post fallback main summary for isolated delivery-target errors", async () => { @@ -702,24 +729,19 @@ describe("CronService", () => { expect(enqueueSystemEvent).not.toHaveBeenCalled(); expect(requestHeartbeatNow).not.toHaveBeenCalled(); - cron.stop(); - await store.cleanup(); + await stopCronAndCleanup(cron, store); }); it("rejects unsupported session/payload combinations", async () => { ensureDir(fixturesRoot); const store = await makeStorePath(); - const cron = new CronService({ - storePath: store.storePath, - cronEnabled: true, - log: noopLogger, - enqueueSystemEvent: vi.fn(), - requestHeartbeatNow: vi.fn(), - runIsolatedAgentJob: vi.fn(async (_params: { job: unknown; message: string }) => ({ - status: "ok", + const cron = createStartedCronService( + store.storePath, + vi.fn(async (_params: { job: unknown; message: string }) => ({ + status: "ok" as const, })) as unknown as CronServiceDeps["runIsolatedAgentJob"], - }); + ); await cron.start(); diff --git a/src/cron/service.store-migration.test.ts b/src/cron/service.store-migration.test.ts index 6322e11b2..52c9f571b 100644 --- a/src/cron/service.store-migration.test.ts +++ b/src/cron/service.store-migration.test.ts @@ -32,44 +32,61 @@ async function listJobById(cron: CronService, jobId: string) { return jobs.find((entry) => entry.id === jobId); } +async function startCronWithStoredJobs(jobs: Array>) { + const store = await makeStorePath(); + await fs.mkdir(path.dirname(store.storePath), { recursive: true }); + await fs.writeFile( + store.storePath, + JSON.stringify( + { + version: 1, + jobs, + }, + null, + 2, + ), + "utf-8", + ); + const cron = await createStartedCron(store.storePath).start(); + return { store, cron }; +} + +async function stopCronAndCleanup(cron: CronService, store: { cleanup: () => Promise }) { + cron.stop(); + await store.cleanup(); +} + +function createLegacyIsolatedAgentTurnJob( + overrides: Record, +): Record { + return { + enabled: true, + createdAtMs: Date.parse("2026-02-01T12:00:00.000Z"), + updatedAtMs: Date.parse("2026-02-05T12:00:00.000Z"), + schedule: { kind: "cron", expr: "0 23 * * *", tz: "UTC" }, + sessionTarget: "isolated", + wakeMode: "next-heartbeat", + payload: { kind: "agentTurn", message: "legacy payload fields" }, + ...overrides, + }; +} + describe("CronService store migrations", () => { it("migrates legacy top-level agentTurn fields and initializes missing state", async () => { - const store = await makeStorePath(); - await fs.mkdir(path.dirname(store.storePath), { recursive: true }); - await fs.writeFile( - store.storePath, - JSON.stringify( - { - version: 1, - jobs: [ - { - id: "legacy-agentturn-job", - name: "legacy agentturn", - enabled: true, - createdAtMs: Date.parse("2026-02-01T12:00:00.000Z"), - updatedAtMs: Date.parse("2026-02-05T12:00:00.000Z"), - schedule: { kind: "cron", expr: "0 23 * * *", tz: "UTC" }, - sessionTarget: "isolated", - wakeMode: "next-heartbeat", - model: "openrouter/deepseek/deepseek-r1", - thinking: "high", - timeoutSeconds: 120, - allowUnsafeExternalContent: true, - deliver: true, - channel: "telegram", - to: "12345", - bestEffortDeliver: true, - payload: { kind: "agentTurn", message: "legacy payload fields" }, - }, - ], - }, - null, - 2, - ), - "utf-8", - ); - - const cron = await createStartedCron(store.storePath).start(); + const { store, cron } = await startCronWithStoredJobs([ + createLegacyIsolatedAgentTurnJob({ + id: "legacy-agentturn-job", + name: "legacy agentturn", + model: "openrouter/deepseek/deepseek-r1", + thinking: "high", + timeoutSeconds: 120, + allowUnsafeExternalContent: true, + deliver: true, + channel: "telegram", + to: "12345", + bestEffortDeliver: true, + }), + ]); const status = await cron.status(); expect(status.enabled).toBe(true); @@ -106,40 +123,17 @@ describe("CronService store migrations", () => { expect(persistedJob?.to).toBeUndefined(); expect(persistedJob?.bestEffortDeliver).toBeUndefined(); - cron.stop(); - await store.cleanup(); + await stopCronAndCleanup(cron, store); }); it("preserves legacy timeoutSeconds=0 during top-level agentTurn field migration", async () => { - const store = await makeStorePath(); - await fs.mkdir(path.dirname(store.storePath), { recursive: true }); - await fs.writeFile( - store.storePath, - JSON.stringify( - { - version: 1, - jobs: [ - { - id: "legacy-agentturn-no-timeout", - name: "legacy no-timeout", - enabled: true, - createdAtMs: Date.parse("2026-02-01T12:00:00.000Z"), - updatedAtMs: Date.parse("2026-02-05T12:00:00.000Z"), - schedule: { kind: "cron", expr: "0 23 * * *", tz: "UTC" }, - sessionTarget: "isolated", - wakeMode: "next-heartbeat", - timeoutSeconds: 0, - payload: { kind: "agentTurn", message: "legacy payload fields" }, - }, - ], - }, - null, - 2, - ), - "utf-8", - ); - - const cron = await createStartedCron(store.storePath).start(); + const { store, cron } = await startCronWithStoredJobs([ + createLegacyIsolatedAgentTurnJob({ + id: "legacy-agentturn-no-timeout", + name: "legacy no-timeout", + timeoutSeconds: 0, + }), + ]); const job = await listJobById(cron, "legacy-agentturn-no-timeout"); expect(job).toBeDefined(); @@ -148,38 +142,22 @@ describe("CronService store migrations", () => { expect(job.payload.timeoutSeconds).toBe(0); } - cron.stop(); - await store.cleanup(); + await stopCronAndCleanup(cron, store); }); it("migrates legacy cron fields (jobId + schedule.cron) and defaults wakeMode", async () => { - const store = await makeStorePath(); - await fs.mkdir(path.dirname(store.storePath), { recursive: true }); - await fs.writeFile( - store.storePath, - JSON.stringify( - { - version: 1, - jobs: [ - { - jobId: "legacy-cron-field-job", - name: "legacy cron field", - enabled: true, - createdAtMs: Date.parse("2026-02-01T12:00:00.000Z"), - updatedAtMs: Date.parse("2026-02-05T12:00:00.000Z"), - schedule: { kind: "cron", cron: "*/5 * * * *", tz: "UTC" }, - payload: { kind: "systemEvent", text: "tick" }, - state: {}, - }, - ], - }, - null, - 2, - ), - "utf-8", - ); - - const cron = await createStartedCron(store.storePath).start(); + const { store, cron } = await startCronWithStoredJobs([ + { + jobId: "legacy-cron-field-job", + name: "legacy cron field", + enabled: true, + createdAtMs: Date.parse("2026-02-01T12:00:00.000Z"), + updatedAtMs: Date.parse("2026-02-05T12:00:00.000Z"), + schedule: { kind: "cron", cron: "*/5 * * * *", tz: "UTC" }, + payload: { kind: "systemEvent", text: "tick" }, + state: {}, + }, + ]); const job = await listJobById(cron, "legacy-cron-field-job"); expect(job).toBeDefined(); expect(job?.wakeMode).toBe("now"); @@ -202,7 +180,6 @@ describe("CronService store migrations", () => { expect(persistedSchedule?.cron).toBeUndefined(); expect(persistedSchedule?.expr).toBe("*/5 * * * *"); - cron.stop(); - await store.cleanup(); + await stopCronAndCleanup(cron, store); }); }); diff --git a/src/gateway/channel-health-monitor.test.ts b/src/gateway/channel-health-monitor.test.ts index 22f1e565f..becbdf82e 100644 --- a/src/gateway/channel-health-monitor.test.ts +++ b/src/gateway/channel-health-monitor.test.ts @@ -80,6 +80,56 @@ function managedStoppedAccount(lastError: string): Partial, +): Partial { + return { + running: true, + connected: true, + enabled: true, + configured: true, + ...overrides, + }; +} + +function createSlackSnapshotManager( + account: Partial, + overrides?: Partial, +): ChannelManager { + return createSnapshotManager( + { + slack: { + default: account, + }, + }, + overrides, + ); +} + +async function expectRestartedChannel( + manager: ChannelManager, + channel: ChannelId, + accountId = "default", +) { + const monitor = await startAndRunCheck(manager); + expect(manager.stopChannel).toHaveBeenCalledWith(channel, accountId); + expect(manager.startChannel).toHaveBeenCalledWith(channel, accountId); + monitor.stop(); +} + +async function expectNoRestart(manager: ChannelManager) { + const monitor = await startAndRunCheck(manager); + expect(manager.stopChannel).not.toHaveBeenCalled(); + expect(manager.startChannel).not.toHaveBeenCalled(); + monitor.stop(); +} + +async function expectNoStart(manager: ChannelManager) { + const monitor = await startAndRunCheck(manager); + expect(manager.startChannel).not.toHaveBeenCalled(); + monitor.stop(); +} + describe("channel-health-monitor", () => { beforeEach(() => { vi.useFakeTimers(); @@ -126,9 +176,7 @@ describe("channel-health-monitor", () => { }, }, }); - const monitor = await startAndRunCheck(manager); - expect(manager.startChannel).not.toHaveBeenCalled(); - monitor.stop(); + await expectNoStart(manager); }); it("skips unconfigured channels", async () => { @@ -137,9 +185,7 @@ describe("channel-health-monitor", () => { default: { running: false, enabled: true, configured: false }, }, }); - const monitor = await startAndRunCheck(manager); - expect(manager.startChannel).not.toHaveBeenCalled(); - monitor.stop(); + await expectNoStart(manager); }); it("skips manually stopped channels", async () => { @@ -151,9 +197,7 @@ describe("channel-health-monitor", () => { }, { isManuallyStopped: vi.fn(() => true) }, ); - const monitor = await startAndRunCheck(manager); - expect(manager.startChannel).not.toHaveBeenCalled(); - monitor.stop(); + await expectNoStart(manager); }); it("restarts a stuck channel (running but not connected)", async () => { @@ -312,98 +356,56 @@ describe("channel-health-monitor", () => { it("restarts a channel with no events past the stale threshold", async () => { const now = Date.now(); - const manager = createSnapshotManager({ - slack: { - default: { - running: true, - connected: true, - enabled: true, - configured: true, - lastStartAt: now - STALE_THRESHOLD - 60_000, - lastEventAt: now - STALE_THRESHOLD - 30_000, - }, - }, - }); - const monitor = await startAndRunCheck(manager); - expect(manager.stopChannel).toHaveBeenCalledWith("slack", "default"); - expect(manager.startChannel).toHaveBeenCalledWith("slack", "default"); - monitor.stop(); + const manager = createSlackSnapshotManager( + runningConnectedSlackAccount({ + lastStartAt: now - STALE_THRESHOLD - 60_000, + lastEventAt: now - STALE_THRESHOLD - 30_000, + }), + ); + await expectRestartedChannel(manager, "slack"); }); it("skips channels with recent events", async () => { const now = Date.now(); - const manager = createSnapshotManager({ - slack: { - default: { - running: true, - connected: true, - enabled: true, - configured: true, - lastStartAt: now - STALE_THRESHOLD - 60_000, - lastEventAt: now - 5_000, - }, - }, - }); - const monitor = await startAndRunCheck(manager); - expect(manager.stopChannel).not.toHaveBeenCalled(); - expect(manager.startChannel).not.toHaveBeenCalled(); - monitor.stop(); + const manager = createSlackSnapshotManager( + runningConnectedSlackAccount({ + lastStartAt: now - STALE_THRESHOLD - 60_000, + lastEventAt: now - 5_000, + }), + ); + await expectNoRestart(manager); }); it("skips channels still within the startup grace window for stale detection", async () => { const now = Date.now(); - const manager = createSnapshotManager({ - slack: { - default: { - running: true, - connected: true, - enabled: true, - configured: true, - lastStartAt: now - 5_000, - lastEventAt: null, - }, - }, - }); - const monitor = await startAndRunCheck(manager); - expect(manager.stopChannel).not.toHaveBeenCalled(); - expect(manager.startChannel).not.toHaveBeenCalled(); - monitor.stop(); + const manager = createSlackSnapshotManager( + runningConnectedSlackAccount({ + lastStartAt: now - 5_000, + lastEventAt: null, + }), + ); + await expectNoRestart(manager); }); it("restarts a channel that never received any event past the stale threshold", async () => { const now = Date.now(); - const manager = createSnapshotManager({ - slack: { - default: { - running: true, - connected: true, - enabled: true, - configured: true, - lastStartAt: now - STALE_THRESHOLD - 60_000, - }, - }, - }); - const monitor = await startAndRunCheck(manager); - expect(manager.stopChannel).toHaveBeenCalledWith("slack", "default"); - expect(manager.startChannel).toHaveBeenCalledWith("slack", "default"); - monitor.stop(); + const manager = createSlackSnapshotManager( + runningConnectedSlackAccount({ + lastStartAt: now - STALE_THRESHOLD - 60_000, + }), + ); + await expectRestartedChannel(manager, "slack"); }); it("respects custom staleEventThresholdMs", async () => { const customThreshold = 10 * 60_000; const now = Date.now(); - const manager = createSnapshotManager({ - slack: { - default: { - running: true, - connected: true, - enabled: true, - configured: true, - lastStartAt: now - customThreshold - 60_000, - lastEventAt: now - customThreshold - 30_000, - }, - }, - }); + const manager = createSlackSnapshotManager( + runningConnectedSlackAccount({ + lastStartAt: now - customThreshold - 60_000, + lastEventAt: now - customThreshold - 30_000, + }), + ); const monitor = await startAndRunCheck(manager, { staleEventThresholdMs: customThreshold, }); diff --git a/src/gateway/openai-http.test.ts b/src/gateway/openai-http.test.ts index 5195af6fb..c9d429521 100644 --- a/src/gateway/openai-http.test.ts +++ b/src/gateway/openai-http.test.ts @@ -136,6 +136,15 @@ describe("OpenAI-compatible HTTP API (e2e)", () => { } | undefined; const getFirstAgentMessage = () => getFirstAgentCall()?.message ?? ""; + const postSyncUserMessage = async (message: string) => { + const res = await postChatCompletions(port, { + stream: false, + model: "openclaw", + messages: [{ role: "user", content: message }], + }); + expect(res.status).toBe(200); + return (await res.json()) as Record; + }; try { { @@ -320,13 +329,7 @@ describe("OpenAI-compatible HTTP API (e2e)", () => { { mockAgentOnce([{ text: "hello" }]); - const res = await postChatCompletions(port, { - stream: false, - model: "openclaw", - messages: [{ role: "user", content: "hi" }], - }); - expect(res.status).toBe(200); - const json = (await res.json()) as Record; + const json = await postSyncUserMessage("hi"); expect(json.object).toBe("chat.completion"); expect(Array.isArray(json.choices)).toBe(true); const choice0 = (json.choices as Array>)[0] ?? {}; @@ -338,13 +341,7 @@ describe("OpenAI-compatible HTTP API (e2e)", () => { { agentCommand.mockClear(); agentCommand.mockResolvedValueOnce({ payloads: [{ text: "" }] } as never); - const res = await postChatCompletions(port, { - stream: false, - model: "openclaw", - messages: [{ role: "user", content: "hi" }], - }); - expect(res.status).toBe(200); - const json = (await res.json()) as Record; + const json = await postSyncUserMessage("hi"); const choice0 = (json.choices as Array>)[0] ?? {}; const msg = (choice0.message as Record | undefined) ?? {}; expect(msg.content).toBe("No response from OpenClaw."); diff --git a/src/gateway/openresponses-http.ts b/src/gateway/openresponses-http.ts index e392b47be..bea285299 100644 --- a/src/gateway/openresponses-http.ts +++ b/src/gateway/openresponses-http.ts @@ -191,6 +191,19 @@ function extractUsageFromResult(result: unknown): Usage { ); } +type PendingToolCall = { id: string; name: string; arguments: string }; + +function resolveStopReasonAndPendingToolCalls(meta: unknown): { + stopReason: string | undefined; + pendingToolCalls: PendingToolCall[] | undefined; +} { + if (!meta || typeof meta !== "object") { + return { stopReason: undefined, pendingToolCalls: undefined }; + } + const record = meta as { stopReason?: string; pendingToolCalls?: PendingToolCall[] }; + return { stopReason: record.stopReason, pendingToolCalls: record.pendingToolCalls }; +} + function createResponseResource(params: { id: string; model: string; @@ -467,13 +480,7 @@ export async function handleOpenResponsesHttpRequest( const payloads = (result as { payloads?: Array<{ text?: string }> } | null)?.payloads; const usage = extractUsageFromResult(result); const meta = (result as { meta?: unknown } | null)?.meta; - const stopReason = - meta && typeof meta === "object" ? (meta as { stopReason?: string }).stopReason : undefined; - const pendingToolCalls = - meta && typeof meta === "object" - ? (meta as { pendingToolCalls?: Array<{ id: string; name: string; arguments: string }> }) - .pendingToolCalls - : undefined; + const { stopReason, pendingToolCalls } = resolveStopReasonAndPendingToolCalls(meta); // If agent called a client tool, return function_call instead of text if (stopReason === "tool_calls" && pendingToolCalls && pendingToolCalls.length > 0) { @@ -709,18 +716,7 @@ export async function handleOpenResponsesHttpRequest( const resultAny = result as { payloads?: Array<{ text?: string }>; meta?: unknown }; const payloads = resultAny.payloads; const meta = resultAny.meta; - const stopReason = - meta && typeof meta === "object" - ? (meta as { stopReason?: string }).stopReason - : undefined; - const pendingToolCalls = - meta && typeof meta === "object" - ? ( - meta as { - pendingToolCalls?: Array<{ id: string; name: string; arguments: string }>; - } - ).pendingToolCalls - : undefined; + const { stopReason, pendingToolCalls } = resolveStopReasonAndPendingToolCalls(meta); // If agent called a client tool, emit function_call instead of text if (stopReason === "tool_calls" && pendingToolCalls && pendingToolCalls.length > 0) { diff --git a/src/gateway/server.auth.control-ui.suite.ts b/src/gateway/server.auth.control-ui.suite.ts index f754d0078..bbd00fede 100644 --- a/src/gateway/server.auth.control-ui.suite.ts +++ b/src/gateway/server.auth.control-ui.suite.ts @@ -91,6 +91,38 @@ export function registerControlUiAndPairingSuite(): void { expect(health.ok).toBe(true); }; + const seedApprovedOperatorReadPairing = async (params: { + identityPrefix: string; + clientId: string; + clientMode: string; + displayName: string; + platform: string; + }): Promise<{ identityPath: string; identity: { deviceId: string } }> => { + const { mkdtemp } = await import("node:fs/promises"); + const { tmpdir } = await import("node:os"); + const { join } = await import("node:path"); + const { loadOrCreateDeviceIdentity, publicKeyRawBase64UrlFromPem } = + await import("../infra/device-identity.js"); + const { approveDevicePairing, requestDevicePairing } = + await import("../infra/device-pairing.js"); + const identityDir = await mkdtemp(join(tmpdir(), params.identityPrefix)); + const identityPath = join(identityDir, "device.json"); + const identity = loadOrCreateDeviceIdentity(identityPath); + const devicePublicKey = publicKeyRawBase64UrlFromPem(identity.publicKeyPem); + const seeded = await requestDevicePairing({ + deviceId: identity.deviceId, + publicKey: devicePublicKey, + role: "operator", + scopes: ["operator.read"], + clientId: params.clientId, + clientMode: params.clientMode, + displayName: params.displayName, + platform: params.platform, + }); + await approveDevicePairing(seeded.request.requestId); + return { identityPath, identity: { deviceId: identity.deviceId } }; + }; + for (const tc of trustedProxyControlUiCases) { test(tc.name, async () => { await configureTrustedProxyControlUiAuth(); @@ -485,29 +517,15 @@ export function registerControlUiAndPairingSuite(): void { }); test("auto-approves loopback scope upgrades for control ui clients", async () => { - const { mkdtemp } = await import("node:fs/promises"); - const { tmpdir } = await import("node:os"); - const { join } = await import("node:path"); - const { loadOrCreateDeviceIdentity, publicKeyRawBase64UrlFromPem } = - await import("../infra/device-identity.js"); - const { approveDevicePairing, getPairedDevice, listDevicePairing, requestDevicePairing } = - await import("../infra/device-pairing.js"); + const { getPairedDevice, listDevicePairing } = await import("../infra/device-pairing.js"); const { server, ws, port, prevToken } = await startServerWithClient("secret"); - const identityDir = await mkdtemp(join(tmpdir(), "openclaw-device-token-scope-")); - const identityPath = join(identityDir, "device.json"); - const identity = loadOrCreateDeviceIdentity(identityPath); - const devicePublicKey = publicKeyRawBase64UrlFromPem(identity.publicKeyPem); - const seeded = await requestDevicePairing({ - deviceId: identity.deviceId, - publicKey: devicePublicKey, - role: "operator", - scopes: ["operator.read"], + const { identity, identityPath } = await seedApprovedOperatorReadPairing({ + identityPrefix: "openclaw-device-token-scope-", clientId: CONTROL_UI_CLIENT.id, clientMode: CONTROL_UI_CLIENT.mode, displayName: "loopback-control-ui-upgrade", platform: CONTROL_UI_CLIENT.platform, }); - await approveDevicePairing(seeded.request.requestId); ws.close(); @@ -740,30 +758,16 @@ export function registerControlUiAndPairingSuite(): void { }); test("auto-approves local scope upgrades even when paired metadata is legacy-shaped", async () => { - const { mkdtemp } = await import("node:fs/promises"); - const { tmpdir } = await import("node:os"); - const { join } = await import("node:path"); const { readJsonFile, resolvePairingPaths } = await import("../infra/pairing-files.js"); const { writeJsonAtomic } = await import("../infra/json-files.js"); - const { loadOrCreateDeviceIdentity, publicKeyRawBase64UrlFromPem } = - await import("../infra/device-identity.js"); - const { approveDevicePairing, getPairedDevice, listDevicePairing, requestDevicePairing } = - await import("../infra/device-pairing.js"); - const identityDir = await mkdtemp(join(tmpdir(), "openclaw-device-legacy-")); - const identityPath = join(identityDir, "device.json"); - const identity = loadOrCreateDeviceIdentity(identityPath); - const devicePublicKey = publicKeyRawBase64UrlFromPem(identity.publicKeyPem); - const seeded = await requestDevicePairing({ - deviceId: identity.deviceId, - publicKey: devicePublicKey, - role: "operator", - scopes: ["operator.read"], + const { getPairedDevice, listDevicePairing } = await import("../infra/device-pairing.js"); + const { identity, identityPath } = await seedApprovedOperatorReadPairing({ + identityPrefix: "openclaw-device-legacy-", clientId: TEST_OPERATOR_CLIENT.id, clientMode: TEST_OPERATOR_CLIENT.mode, displayName: "legacy-upgrade-test", platform: "test", }); - await approveDevicePairing(seeded.request.requestId); const { pairedPath } = resolvePairingPaths(undefined, "devices"); const paired = (await readJsonFile>>(pairedPath)) ?? {}; diff --git a/src/gateway/server.auth.default-token.suite.ts b/src/gateway/server.auth.default-token.suite.ts index 85227e058..8cc20f57a 100644 --- a/src/gateway/server.auth.default-token.suite.ts +++ b/src/gateway/server.auth.default-token.suite.ts @@ -67,6 +67,14 @@ export function registerDefaultAuthTokenSuite(): void { await new Promise((resolve) => ws.once("close", () => resolve())); } + async function expectStatusMissingScopeButHealthAvailable(ws: WebSocket): Promise { + const status = await rpcReq(ws, "status"); + expect(status.ok).toBe(false); + expect(status.error?.message).toContain("missing scope"); + const health = await rpcReq(ws, "health"); + expect(health.ok).toBe(true); + } + test("closes silent handshakes after timeout", async () => { vi.useRealTimers(); const prevHandshakeTimeout = process.env.OPENCLAW_TEST_HANDSHAKE_TIMEOUT_MS; @@ -198,11 +206,7 @@ export function registerDefaultAuthTokenSuite(): void { try { const res = await connectReq(ws, { scopes: [] }); expect(res.ok).toBe(true); - const status = await rpcReq(ws, "status"); - expect(status.ok).toBe(false); - expect(status.error?.message).toContain("missing scope"); - const health = await rpcReq(ws, "health"); - expect(health.ok).toBe(true); + await expectStatusMissingScopeButHealthAvailable(ws); } finally { ws.close(); } @@ -247,11 +251,7 @@ export function registerDefaultAuthTokenSuite(): void { expect(presenceScopes).toEqual([]); expect(presenceScopes).not.toContain("operator.admin"); - const status = await rpcReq(ws, "status"); - expect(status.ok).toBe(false); - expect(status.error?.message).toContain("missing scope"); - const health = await rpcReq(ws, "health"); - expect(health.ok).toBe(true); + await expectStatusMissingScopeButHealthAvailable(ws); ws.close(); }); diff --git a/src/gateway/server.auth.shared.ts b/src/gateway/server.auth.shared.ts index c50543edb..3f1f150fa 100644 --- a/src/gateway/server.auth.shared.ts +++ b/src/gateway/server.auth.shared.ts @@ -291,10 +291,22 @@ async function sendRawConnectReq( }>(ws, isConnectResMessage(params.id)); } -async function startRateLimitedTokenServerWithPairedDeviceToken() { +async function resolvePairedTokenForDeviceIdentityPath(deviceIdentityPath: string): Promise<{ + identity: { deviceId: string }; + deviceToken: string; +}> { const { loadOrCreateDeviceIdentity } = await import("../infra/device-identity.js"); const { getPairedDevice } = await import("../infra/device-pairing.js"); + const identity = loadOrCreateDeviceIdentity(deviceIdentityPath); + const paired = await getPairedDevice(identity.deviceId); + const deviceToken = paired?.tokens?.operator?.token; + expect(paired?.deviceId).toBe(identity.deviceId); + expect(deviceToken).toBeDefined(); + return { identity: { deviceId: identity.deviceId }, deviceToken: String(deviceToken ?? "") }; +} + +async function startRateLimitedTokenServerWithPairedDeviceToken() { testState.gatewayAuth = { mode: "token", token: "secret", @@ -309,12 +321,7 @@ async function startRateLimitedTokenServerWithPairedDeviceToken() { if (!initial.ok) { await approvePendingPairingIfNeeded(); } - - const identity = loadOrCreateDeviceIdentity(deviceIdentityPath); - const paired = await getPairedDevice(identity.deviceId); - const deviceToken = paired?.tokens?.operator?.token; - expect(paired?.deviceId).toBe(identity.deviceId); - expect(deviceToken).toBeDefined(); + const { deviceToken } = await resolvePairedTokenForDeviceIdentityPath(deviceIdentityPath); ws.close(); return { server, port, prevToken, deviceToken: String(deviceToken ?? ""), deviceIdentityPath }; @@ -331,24 +338,17 @@ async function ensurePairedDeviceTokenForCurrentIdentity(ws: WebSocket): Promise deviceToken: string; deviceIdentityPath: string; }> { - const { loadOrCreateDeviceIdentity } = await import("../infra/device-identity.js"); - const { getPairedDevice } = await import("../infra/device-pairing.js"); - const deviceIdentityPath = nextAuthIdentityPath("openclaw-auth-device"); const res = await connectReq(ws, { token: "secret", deviceIdentityPath }); if (!res.ok) { await approvePendingPairingIfNeeded(); } - - const identity = loadOrCreateDeviceIdentity(deviceIdentityPath); - const paired = await getPairedDevice(identity.deviceId); - const deviceToken = paired?.tokens?.operator?.token; - expect(paired?.deviceId).toBe(identity.deviceId); - expect(deviceToken).toBeDefined(); + const { identity, deviceToken } = + await resolvePairedTokenForDeviceIdentityPath(deviceIdentityPath); return { - identity: { deviceId: identity.deviceId }, - deviceToken: String(deviceToken ?? ""), + identity, + deviceToken, deviceIdentityPath, }; } diff --git a/src/gateway/server.hooks.test.ts b/src/gateway/server.hooks.test.ts index 9e1ee754c..ddad8bd6d 100644 --- a/src/gateway/server.hooks.test.ts +++ b/src/gateway/server.hooks.test.ts @@ -42,12 +42,24 @@ async function postHook( }); } +function setMainAndHooksAgents(): void { + testState.agentsConfig = { + list: [{ id: "main", default: true }, { id: "hooks" }], + }; +} + +function mockIsolatedRunOkOnce(): void { + cronIsolatedRun.mockClear(); + cronIsolatedRun.mockResolvedValueOnce({ + status: "ok", + summary: "done", + }); +} + describe("gateway server hooks", () => { test("handles auth, wake, and agent flows", async () => { testState.hooksConfig = { enabled: true, token: HOOK_TOKEN }; - testState.agentsConfig = { - list: [{ id: "main", default: true }, { id: "hooks" }], - }; + setMainAndHooksAgents(); await withGatewayServer(async ({ port }) => { const resNoAuth = await postHook(port, "/hooks/wake", { text: "Ping" }, { token: null }); expect(resNoAuth.status).toBe(401); @@ -58,22 +70,14 @@ describe("gateway server hooks", () => { expect(wakeEvents.some((e) => e.includes("Ping"))).toBe(true); drainSystemEvents(resolveMainKey()); - cronIsolatedRun.mockClear(); - cronIsolatedRun.mockResolvedValueOnce({ - status: "ok", - summary: "done", - }); + mockIsolatedRunOkOnce(); const resAgent = await postHook(port, "/hooks/agent", { message: "Do it", name: "Email" }); expect(resAgent.status).toBe(202); const agentEvents = await waitForSystemEvent(); expect(agentEvents.some((e) => e.includes("Hook Email: done"))).toBe(true); drainSystemEvents(resolveMainKey()); - cronIsolatedRun.mockClear(); - cronIsolatedRun.mockResolvedValueOnce({ - status: "ok", - summary: "done", - }); + mockIsolatedRunOkOnce(); const resAgentModel = await postHook(port, "/hooks/agent", { message: "Do it", name: "Email", @@ -87,11 +91,7 @@ describe("gateway server hooks", () => { expect(call?.job?.payload?.model).toBe("openai/gpt-4.1-mini"); drainSystemEvents(resolveMainKey()); - cronIsolatedRun.mockClear(); - cronIsolatedRun.mockResolvedValueOnce({ - status: "ok", - summary: "done", - }); + mockIsolatedRunOkOnce(); const resAgentWithId = await postHook(port, "/hooks/agent", { message: "Do it", name: "Email", @@ -105,11 +105,7 @@ describe("gateway server hooks", () => { expect(routedCall?.job?.agentId).toBe("hooks"); drainSystemEvents(resolveMainKey()); - cronIsolatedRun.mockClear(); - cronIsolatedRun.mockResolvedValueOnce({ - status: "ok", - summary: "done", - }); + mockIsolatedRunOkOnce(); const resAgentUnknown = await postHook(port, "/hooks/agent", { message: "Do it", name: "Email", @@ -243,15 +239,9 @@ describe("gateway server hooks", () => { allowRequestSessionKey: true, allowedSessionKeyPrefixes: ["hook:", "agent:"], }; - testState.agentsConfig = { - list: [{ id: "main", default: true }, { id: "hooks" }], - }; + setMainAndHooksAgents(); await withGatewayServer(async ({ port }) => { - cronIsolatedRun.mockClear(); - cronIsolatedRun.mockResolvedValueOnce({ - status: "ok", - summary: "done", - }); + mockIsolatedRunOkOnce(); const resAgent = await postHook(port, "/hooks/agent", { message: "Do it", @@ -285,15 +275,9 @@ describe("gateway server hooks", () => { }, ], }; - testState.agentsConfig = { - list: [{ id: "main", default: true }, { id: "hooks" }], - }; + setMainAndHooksAgents(); await withGatewayServer(async ({ port }) => { - cronIsolatedRun.mockClear(); - cronIsolatedRun.mockResolvedValueOnce({ - status: "ok", - summary: "done", - }); + mockIsolatedRunOkOnce(); const resNoAgent = await postHook(port, "/hooks/agent", { message: "No explicit agent" }); expect(resNoAgent.status).toBe(202); await waitForSystemEvent(); @@ -303,11 +287,7 @@ describe("gateway server hooks", () => { expect(noAgentCall?.job?.agentId).toBeUndefined(); drainSystemEvents(resolveMainKey()); - cronIsolatedRun.mockClear(); - cronIsolatedRun.mockResolvedValueOnce({ - status: "ok", - summary: "done", - }); + mockIsolatedRunOkOnce(); const resAllowed = await postHook(port, "/hooks/agent", { message: "Allowed", agentId: "hooks", diff --git a/src/gateway/server.plugin-http-auth.test.ts b/src/gateway/server.plugin-http-auth.test.ts index 71bd89ad4..46fdcacc5 100644 --- a/src/gateway/server.plugin-http-auth.test.ts +++ b/src/gateway/server.plugin-http-auth.test.ts @@ -18,10 +18,51 @@ import { withGatewayTempConfig, } from "./server-http.test-harness.js"; +type PluginRequestHandler = (req: IncomingMessage, res: ServerResponse) => Promise; + function canonicalizePluginPath(pathname: string): string { return canonicalizePathVariant(pathname); } +function respondJsonRoute(res: ServerResponse, route: string): true { + res.statusCode = 200; + res.setHeader("Content-Type", "application/json; charset=utf-8"); + res.end(JSON.stringify({ ok: true, route })); + return true; +} + +function createRootMountedControlUiOverrides(handlePluginRequest: PluginRequestHandler) { + return { + controlUiEnabled: true, + controlUiBasePath: "", + controlUiRoot: { kind: "missing" as const }, + handlePluginRequest, + }; +} + +const withRootMountedControlUiServer = (params: { + prefix: string; + handlePluginRequest: PluginRequestHandler; + run: Parameters[0]["run"]; +}) => + withPluginGatewayServer({ + prefix: params.prefix, + resolvedAuth: AUTH_NONE, + overrides: createRootMountedControlUiOverrides(params.handlePluginRequest), + run: params.run, + }); + +const withPluginGatewayServer = (params: Parameters[0]) => + withGatewayServer(params); + +function createProtectedPluginAuthOverrides(handlePluginRequest: PluginRequestHandler) { + return { + handlePluginRequest, + shouldEnforcePluginGatewayAuth: (pathContext: { pathname: string }) => + isProtectedPluginRoutePath(pathContext.pathname), + }; +} + describe("gateway plugin HTTP auth boundary", () => { test("applies default security headers and optional strict transport security", async () => { await withGatewayTempConfig("openclaw-plugin-http-security-headers-test-", async () => { @@ -179,16 +220,10 @@ describe("gateway plugin HTTP auth boundary", () => { const handlePluginRequest = vi.fn(async (req: IncomingMessage, res: ServerResponse) => { const pathname = new URL(req.url ?? "/", "http://localhost").pathname; if (pathname === "/plugin/routed") { - res.statusCode = 200; - res.setHeader("Content-Type", "application/json; charset=utf-8"); - res.end(JSON.stringify({ ok: true, route: "routed" })); - return true; + return respondJsonRoute(res, "routed"); } if (pathname === "/googlechat") { - res.statusCode = 200; - res.setHeader("Content-Type", "application/json; charset=utf-8"); - res.end(JSON.stringify({ ok: true, route: "wildcard-handler" })); - return true; + return respondJsonRoute(res, "wildcard-handler"); } return false; }); @@ -224,16 +259,10 @@ describe("gateway plugin HTTP auth boundary", () => { const handlePluginRequest = vi.fn(async (req: IncomingMessage, res: ServerResponse) => { const pathname = new URL(req.url ?? "/", "http://localhost").pathname; if (canonicalizePluginPath(pathname) === "/api/channels/nostr/default/profile") { - res.statusCode = 200; - res.setHeader("Content-Type", "application/json; charset=utf-8"); - res.end(JSON.stringify({ ok: true, route: "channel-default" })); - return true; + return respondJsonRoute(res, "channel-default"); } if (pathname === "/googlechat") { - res.statusCode = 200; - res.setHeader("Content-Type", "application/json; charset=utf-8"); - res.end(JSON.stringify({ ok: true, route: "wildcard-default" })); - return true; + return respondJsonRoute(res, "wildcard-default"); } return false; }); @@ -293,15 +322,9 @@ describe("gateway plugin HTTP auth boundary", () => { return false; }); - await withGatewayServer({ + await withRootMountedControlUiServer({ prefix: "openclaw-plugin-http-control-ui-precedence-test-", - resolvedAuth: AUTH_NONE, - overrides: { - controlUiEnabled: true, - controlUiBasePath: "", - controlUiRoot: { kind: "missing" }, - handlePluginRequest, - }, + handlePluginRequest, run: async (server) => { const response = await sendRequest(server, { path: "/plugins/diffs/view/demo-id/demo-token", @@ -326,15 +349,9 @@ describe("gateway plugin HTTP auth boundary", () => { return true; }); - await withGatewayServer({ + await withRootMountedControlUiServer({ prefix: "openclaw-plugin-http-control-ui-webhook-post-test-", - resolvedAuth: AUTH_NONE, - overrides: { - controlUiEnabled: true, - controlUiBasePath: "", - controlUiRoot: { kind: "missing" }, - handlePluginRequest, - }, + handlePluginRequest, run: async (server) => { const response = await sendRequest(server, { path: "/bluebubbles-webhook", @@ -360,15 +377,9 @@ describe("gateway plugin HTTP auth boundary", () => { return false; }); - await withGatewayServer({ + await withRootMountedControlUiServer({ prefix: "openclaw-plugin-http-control-ui-shadow-test-", - resolvedAuth: AUTH_NONE, - overrides: { - controlUiEnabled: true, - controlUiBasePath: "", - controlUiRoot: { kind: "missing" }, - handlePluginRequest, - }, + handlePluginRequest, run: async (server) => { const response = await sendRequest(server, { path: "/my-plugin/inbound" }); @@ -382,15 +393,9 @@ describe("gateway plugin HTTP auth boundary", () => { test("unmatched plugin paths fall through to control ui", async () => { const handlePluginRequest = vi.fn(async () => false); - await withGatewayServer({ + await withRootMountedControlUiServer({ prefix: "openclaw-plugin-http-control-ui-fallthrough-test-", - resolvedAuth: AUTH_NONE, - overrides: { - controlUiEnabled: true, - controlUiBasePath: "", - controlUiRoot: { kind: "missing" }, - handlePluginRequest, - }, + handlePluginRequest, run: async (server) => { const response = await sendRequest(server, { path: "/chat" }); @@ -404,14 +409,10 @@ describe("gateway plugin HTTP auth boundary", () => { test("requires gateway auth for canonicalized /api/channels variants", async () => { const handlePluginRequest = createCanonicalizedChannelPluginHandler(); - await withGatewayServer({ + await withPluginGatewayServer({ prefix: "openclaw-plugin-http-auth-canonicalized-test-", resolvedAuth: AUTH_TOKEN, - overrides: { - handlePluginRequest, - shouldEnforcePluginGatewayAuth: (pathContext) => - isProtectedPluginRoutePath(pathContext.pathname), - }, + overrides: createProtectedPluginAuthOverrides(handlePluginRequest), run: async (server) => { await expectUnauthorizedVariants({ server, variants: CANONICAL_UNAUTH_VARIANTS }); expect(handlePluginRequest).not.toHaveBeenCalled(); @@ -429,20 +430,15 @@ describe("gateway plugin HTTP auth boundary", () => { test("rejects unauthenticated plugin-channel fuzz corpus variants", async () => { const handlePluginRequest = createCanonicalizedChannelPluginHandler(); - await withGatewayServer({ + await withPluginGatewayServer({ prefix: "openclaw-plugin-http-auth-fuzz-corpus-test-", resolvedAuth: AUTH_TOKEN, - overrides: { - handlePluginRequest, - shouldEnforcePluginGatewayAuth: (pathContext) => - isProtectedPluginRoutePath(pathContext.pathname), - }, + overrides: createProtectedPluginAuthOverrides(handlePluginRequest), run: async (server) => { - for (const variant of buildChannelPathFuzzCorpus()) { - const response = await sendRequest(server, { path: variant.path }); - expect(response.res.statusCode, variant.label).toBe(401); - expect(response.getBody(), variant.label).toContain("Unauthorized"); - } + await expectUnauthorizedVariants({ + server, + variants: buildChannelPathFuzzCorpus(), + }); expect(handlePluginRequest).not.toHaveBeenCalled(); }, }); @@ -464,11 +460,7 @@ describe("gateway plugin HTTP auth boundary", () => { resolvedAuth: AUTH_TOKEN, overrides: { handlePluginRequest }, run: async (server) => { - for (const variant of encodedVariants) { - const response = await sendRequest(server, { path: variant.path }); - expect(response.res.statusCode, variant.label).toBe(401); - expect(response.getBody(), variant.label).toContain("Unauthorized"); - } + await expectUnauthorizedVariants({ server, variants: encodedVariants }); expect(handlePluginRequest).not.toHaveBeenCalled(); }, }); diff --git a/src/hooks/workspace.test.ts b/src/hooks/workspace.test.ts index 7d89919d5..00b7ddaa9 100644 --- a/src/hooks/workspace.test.ts +++ b/src/hooks/workspace.test.ts @@ -21,6 +21,34 @@ function writeHookPackageManifest(pkgDir: string, hooks: string[]): void { ); } +function setupHardlinkHookWorkspace(hookName: string): { + hooksRoot: string; + hookDir: string; + outsideDir: string; +} { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-hooks-workspace-hardlink-")); + const hooksRoot = path.join(root, "hooks"); + fs.mkdirSync(hooksRoot, { recursive: true }); + + const hookDir = path.join(hooksRoot, hookName); + const outsideDir = path.join(root, "outside"); + fs.mkdirSync(hookDir, { recursive: true }); + fs.mkdirSync(outsideDir, { recursive: true }); + return { hooksRoot, hookDir, outsideDir }; +} + +function tryCreateHardlinkOrSkip(createLink: () => void): boolean { + try { + createLink(); + return true; + } catch (err) { + if ((err as NodeJS.ErrnoException).code === "EXDEV") { + return false; + } + throw err; + } +} + describe("hooks workspace", () => { it("ignores package.json hook paths that traverse outside package directory", () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-hooks-workspace-")); @@ -88,27 +116,15 @@ describe("hooks workspace", () => { return; } - const root = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-hooks-workspace-hardlink-")); - const hooksRoot = path.join(root, "hooks"); - fs.mkdirSync(hooksRoot, { recursive: true }); - - const hookDir = path.join(hooksRoot, "hardlink-hook"); - const outsideDir = path.join(root, "outside"); - fs.mkdirSync(hookDir, { recursive: true }); - fs.mkdirSync(outsideDir, { recursive: true }); + const { hooksRoot, hookDir, outsideDir } = setupHardlinkHookWorkspace("hardlink-hook"); fs.writeFileSync(path.join(hookDir, "handler.js"), "export default async () => {};\n"); const outsideHookMd = path.join(outsideDir, "HOOK.md"); const linkedHookMd = path.join(hookDir, "HOOK.md"); fs.writeFileSync(linkedHookMd, "---\nname: hardlink-hook\n---\n"); fs.rmSync(linkedHookMd); fs.writeFileSync(outsideHookMd, "---\nname: outside\n---\n"); - try { - fs.linkSync(outsideHookMd, linkedHookMd); - } catch (err) { - if ((err as NodeJS.ErrnoException).code === "EXDEV") { - return; - } - throw err; + if (!tryCreateHardlinkOrSkip(() => fs.linkSync(outsideHookMd, linkedHookMd))) { + return; } const entries = loadHookEntriesFromDir({ dir: hooksRoot, source: "openclaw-workspace" }); @@ -121,25 +137,13 @@ describe("hooks workspace", () => { return; } - const root = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-hooks-workspace-hardlink-")); - const hooksRoot = path.join(root, "hooks"); - fs.mkdirSync(hooksRoot, { recursive: true }); - - const hookDir = path.join(hooksRoot, "hardlink-handler-hook"); - const outsideDir = path.join(root, "outside"); - fs.mkdirSync(hookDir, { recursive: true }); - fs.mkdirSync(outsideDir, { recursive: true }); + const { hooksRoot, hookDir, outsideDir } = setupHardlinkHookWorkspace("hardlink-handler-hook"); fs.writeFileSync(path.join(hookDir, "HOOK.md"), "---\nname: hardlink-handler-hook\n---\n"); const outsideHandler = path.join(outsideDir, "handler.js"); const linkedHandler = path.join(hookDir, "handler.js"); fs.writeFileSync(outsideHandler, "export default async () => {};\n"); - try { - fs.linkSync(outsideHandler, linkedHandler); - } catch (err) { - if ((err as NodeJS.ErrnoException).code === "EXDEV") { - return; - } - throw err; + if (!tryCreateHardlinkOrSkip(() => fs.linkSync(outsideHandler, linkedHandler))) { + return; } const entries = loadHookEntriesFromDir({ dir: hooksRoot, source: "openclaw-workspace" }); diff --git a/src/infra/exec-approvals.test.ts b/src/infra/exec-approvals.test.ts index 52f4b5ef9..57290c071 100644 --- a/src/infra/exec-approvals.test.ts +++ b/src/infra/exec-approvals.test.ts @@ -47,6 +47,22 @@ function analyzeEnvWrapperAllowlist(params: { argv: string[]; envPath: string; c return { analysis, allowlistEval }; } +function createPathExecutableFixture(params?: { executable?: string }): { + exeName: string; + exePath: string; + binDir: string; +} { + const dir = makeTempDir(); + const binDir = path.join(dir, "bin"); + fs.mkdirSync(binDir, { recursive: true }); + const baseName = params?.executable ?? "rg"; + const exeName = process.platform === "win32" ? `${baseName}.exe` : baseName; + const exePath = path.join(binDir, exeName); + fs.writeFileSync(exePath, ""); + fs.chmodSync(exePath, 0o755); + return { exeName, exePath, binDir }; +} + describe("exec approvals allowlist matching", () => { const baseResolution = { rawExecutable: "rg", @@ -221,19 +237,13 @@ describe("exec approvals command resolution", () => { { name: "PATH executable", setup: () => { - const dir = makeTempDir(); - const binDir = path.join(dir, "bin"); - fs.mkdirSync(binDir, { recursive: true }); - const exeName = process.platform === "win32" ? "rg.exe" : "rg"; - const exe = path.join(binDir, exeName); - fs.writeFileSync(exe, ""); - fs.chmodSync(exe, 0o755); + const fixture = createPathExecutableFixture(); return { command: "rg -n foo", cwd: undefined as string | undefined, - envPath: makePathEnv(binDir), - expectedPath: exe, - expectedExecutableName: exeName, + envPath: makePathEnv(fixture.binDir), + expectedPath: fixture.exePath, + expectedExecutableName: fixture.exeName, }; }, }, @@ -286,21 +296,15 @@ describe("exec approvals command resolution", () => { }); it("unwraps transparent env wrapper argv to resolve the effective executable", () => { - const dir = makeTempDir(); - const binDir = path.join(dir, "bin"); - fs.mkdirSync(binDir, { recursive: true }); - const exeName = process.platform === "win32" ? "rg.exe" : "rg"; - const exe = path.join(binDir, exeName); - fs.writeFileSync(exe, ""); - fs.chmodSync(exe, 0o755); + const fixture = createPathExecutableFixture(); const resolution = resolveCommandResolutionFromArgv( ["/usr/bin/env", "rg", "-n", "needle"], undefined, - makePathEnv(binDir), + makePathEnv(fixture.binDir), ); - expect(resolution?.resolvedPath).toBe(exe); - expect(resolution?.executableName).toBe(exeName); + expect(resolution?.resolvedPath).toBe(fixture.exePath); + expect(resolution?.executableName).toBe(fixture.exeName); }); it("blocks semantic env wrappers from allowlist/safeBins auto-resolution", () => { diff --git a/src/infra/outbound/deliver.test.ts b/src/infra/outbound/deliver.test.ts index 50be7afd5..17034a852 100644 --- a/src/infra/outbound/deliver.test.ts +++ b/src/infra/outbound/deliver.test.ts @@ -116,6 +116,18 @@ async function runChunkedWhatsAppDelivery(params?: { return { sendWhatsApp, results }; } +async function deliverSingleWhatsAppForHookTest(params?: { sessionKey?: string }) { + const sendWhatsApp = vi.fn().mockResolvedValue({ messageId: "w1", toJid: "jid" }); + await deliverOutboundPayloads({ + cfg: whatsappChunkConfig, + channel: "whatsapp", + to: "+1555", + payloads: [{ text: "hello" }], + deps: { sendWhatsApp }, + ...(params?.sessionKey ? { session: { key: params.sessionKey } } : {}), + }); +} + describe("deliverOutboundPayloads", () => { beforeEach(() => { setActivePluginRegistry(defaultRegistry); @@ -653,31 +665,14 @@ describe("deliverOutboundPayloads", () => { }); it("does not emit internal message:sent hook when neither mirror nor sessionKey is provided", async () => { - const sendWhatsApp = vi.fn().mockResolvedValue({ messageId: "w1", toJid: "jid" }); - - await deliverOutboundPayloads({ - cfg: whatsappChunkConfig, - channel: "whatsapp", - to: "+1555", - payloads: [{ text: "hello" }], - deps: { sendWhatsApp }, - }); + await deliverSingleWhatsAppForHookTest(); expect(internalHookMocks.createInternalHookEvent).not.toHaveBeenCalled(); expect(internalHookMocks.triggerInternalHook).not.toHaveBeenCalled(); }); it("emits internal message:sent hook when sessionKey is provided without mirror", async () => { - const sendWhatsApp = vi.fn().mockResolvedValue({ messageId: "w1", toJid: "jid" }); - - await deliverOutboundPayloads({ - cfg: whatsappChunkConfig, - channel: "whatsapp", - to: "+1555", - payloads: [{ text: "hello" }], - deps: { sendWhatsApp }, - session: { key: "agent:main:main" }, - }); + await deliverSingleWhatsAppForHookTest({ sessionKey: "agent:main:main" }); expect(internalHookMocks.createInternalHookEvent).toHaveBeenCalledTimes(1); expect(internalHookMocks.createInternalHookEvent).toHaveBeenCalledWith( diff --git a/src/infra/outbound/target-resolver.ts b/src/infra/outbound/target-resolver.ts index b3ac5ba43..06bd7d232 100644 --- a/src/infra/outbound/target-resolver.ts +++ b/src/infra/outbound/target-resolver.ts @@ -258,6 +258,14 @@ async function getDirectoryEntries(params: { preferLiveOnMiss?: boolean; }): Promise { const signature = buildTargetResolverSignature(params.channel); + const listParams = { + cfg: params.cfg, + channel: params.channel, + accountId: params.accountId, + kind: params.kind, + query: params.query, + runtime: params.runtime, + }; const cacheKey = buildDirectoryCacheKey({ channel: params.channel, accountId: params.accountId, @@ -270,12 +278,7 @@ async function getDirectoryEntries(params: { return cached; } const entries = await listDirectoryEntries({ - cfg: params.cfg, - channel: params.channel, - accountId: params.accountId, - kind: params.kind, - query: params.query, - runtime: params.runtime, + ...listParams, source: "cache", }); if (entries.length > 0 || !params.preferLiveOnMiss) { @@ -290,12 +293,7 @@ async function getDirectoryEntries(params: { signature, }); const liveEntries = await listDirectoryEntries({ - cfg: params.cfg, - channel: params.channel, - accountId: params.accountId, - kind: params.kind, - query: params.query, - runtime: params.runtime, + ...listParams, source: "live", }); directoryCache.set(liveKey, liveEntries, params.cfg); @@ -303,6 +301,24 @@ async function getDirectoryEntries(params: { return liveEntries; } +function buildNormalizedResolveResult(params: { + channel: ChannelId; + raw: string; + normalized: string; + kind: TargetResolveKind; +}): ResolveMessagingTargetResult { + const directTarget = preserveTargetCase(params.channel, params.raw, params.normalized); + return { + ok: true, + target: { + to: directTarget, + kind: params.kind, + display: stripTargetPrefixes(params.raw), + source: "normalized", + }, + }; +} + function pickAmbiguousMatch( entries: ChannelDirectoryEntry[], mode: ResolveAmbiguousMode, @@ -372,16 +388,12 @@ export async function resolveMessagingTarget(params: { return false; }; if (looksLikeTargetId()) { - const directTarget = preserveTargetCase(params.channel, raw, normalized); - return { - ok: true, - target: { - to: directTarget, - kind, - display: stripTargetPrefixes(raw), - source: "normalized", - }, - }; + return buildNormalizedResolveResult({ + channel: params.channel, + raw, + normalized, + kind, + }); } const query = stripTargetPrefixes(raw); const entries = await getDirectoryEntries({ @@ -434,16 +446,12 @@ export async function resolveMessagingTarget(params: { (params.channel === "bluebubbles" || params.channel === "imessage") && /^\+?\d{6,}$/.test(query) ) { - const directTarget = preserveTargetCase(params.channel, raw, normalized); - return { - ok: true, - target: { - to: directTarget, - kind, - display: stripTargetPrefixes(raw), - source: "normalized", - }, - }; + return buildNormalizedResolveResult({ + channel: params.channel, + raw, + normalized, + kind, + }); } return { diff --git a/src/infra/outbound/targets.channel-resolution.test.ts b/src/infra/outbound/targets.channel-resolution.test.ts index d426e98f4..e676a425b 100644 --- a/src/infra/outbound/targets.channel-resolution.test.ts +++ b/src/infra/outbound/targets.channel-resolution.test.ts @@ -11,7 +11,7 @@ function normalizeChannel(value?: string) { return value?.trim().toLowerCase() ?? undefined; } -function passthroughPluginAutoEnable(config: unknown) { +function applyPluginAutoEnableForTests(config: unknown) { return { config, changes: [] as unknown[] }; } @@ -36,14 +36,16 @@ vi.mock("../../agents/agent-scope.js", () => ({ resolveAgentWorkspaceDir: () => TEST_WORKSPACE_ROOT, })); -vi.mock("../../config/plugin-auto-enable.js", () => ({ - applyPluginAutoEnable: ({ config }: { config: unknown }) => passthroughPluginAutoEnable(config), -})); - vi.mock("../../plugins/loader.js", () => ({ loadOpenClawPlugins: mocks.loadOpenClawPlugins, })); +vi.mock("../../config/plugin-auto-enable.js", () => ({ + applyPluginAutoEnable(args: { config: unknown }) { + return applyPluginAutoEnableForTests(args.config); + }, +})); + import { setActivePluginRegistry } from "../../plugins/runtime.js"; import { createTestRegistry } from "../../test-utils/channel-plugins.js"; import { resolveOutboundTarget } from "./targets.js"; diff --git a/src/infra/outbound/targets.test.ts b/src/infra/outbound/targets.test.ts index 2e2f0cbfa..73f77aee8 100644 --- a/src/infra/outbound/targets.test.ts +++ b/src/infra/outbound/targets.test.ts @@ -5,6 +5,7 @@ import { resolveOutboundTarget, resolveSessionDeliveryTarget, } from "./targets.js"; +import type { SessionDeliveryTarget } from "./targets.js"; import { installResolveOutboundTargetPluginRegistryHooks, runResolveOutboundTargetCoreTests, @@ -14,15 +15,15 @@ runResolveOutboundTargetCoreTests(); describe("resolveOutboundTarget defaultTo config fallback", () => { installResolveOutboundTargetPluginRegistryHooks(); + const whatsappDefaultCfg: OpenClawConfig = { + channels: { whatsapp: { defaultTo: "+15551234567", allowFrom: ["*"] } }, + }; it("uses whatsapp defaultTo when no explicit target is provided", () => { - const cfg: OpenClawConfig = { - channels: { whatsapp: { defaultTo: "+15551234567", allowFrom: ["*"] } }, - }; const res = resolveOutboundTarget({ channel: "whatsapp", to: undefined, - cfg, + cfg: whatsappDefaultCfg, mode: "implicit", }); expect(res).toEqual({ ok: true, to: "+15551234567" }); @@ -42,13 +43,10 @@ describe("resolveOutboundTarget defaultTo config fallback", () => { }); it("explicit --reply-to overrides defaultTo", () => { - const cfg: OpenClawConfig = { - channels: { whatsapp: { defaultTo: "+15551234567", allowFrom: ["*"] } }, - }; const res = resolveOutboundTarget({ channel: "whatsapp", to: "+15559999999", - cfg, + cfg: whatsappDefaultCfg, mode: "explicit", }); expect(res).toEqual({ ok: true, to: "+15559999999" }); @@ -69,6 +67,41 @@ describe("resolveOutboundTarget defaultTo config fallback", () => { }); describe("resolveSessionDeliveryTarget", () => { + const expectImplicitRoute = ( + resolved: SessionDeliveryTarget, + params: { + channel?: SessionDeliveryTarget["channel"]; + to?: string; + lastChannel?: SessionDeliveryTarget["lastChannel"]; + lastTo?: string; + }, + ) => { + expect(resolved).toEqual({ + channel: params.channel, + to: params.to, + accountId: undefined, + threadId: undefined, + threadIdExplicit: false, + mode: "implicit", + lastChannel: params.lastChannel, + lastTo: params.lastTo, + lastAccountId: undefined, + lastThreadId: undefined, + }); + }; + + const expectTopicParsedFromExplicitTo = ( + entry: Parameters[0]["entry"], + ) => { + const resolved = resolveSessionDeliveryTarget({ + entry, + requestedChannel: "last", + explicitTo: "63448508:topic:1008013", + }); + expect(resolved.to).toBe("63448508"); + expect(resolved.threadId).toBe(1008013); + }; + it("derives implicit delivery from the last route", () => { const resolved = resolveSessionDeliveryTarget({ entry: { @@ -106,17 +139,11 @@ describe("resolveSessionDeliveryTarget", () => { requestedChannel: "telegram", }); - expect(resolved).toEqual({ + expectImplicitRoute(resolved, { channel: "telegram", to: undefined, - accountId: undefined, - threadId: undefined, - threadIdExplicit: false, - mode: "implicit", lastChannel: "whatsapp", lastTo: "+1555", - lastAccountId: undefined, - lastThreadId: undefined, }); }); @@ -132,17 +159,11 @@ describe("resolveSessionDeliveryTarget", () => { allowMismatchedLastTo: true, }); - expect(resolved).toEqual({ + expectImplicitRoute(resolved, { channel: "telegram", to: "+1555", - accountId: undefined, - threadId: undefined, - threadIdExplicit: false, - mode: "implicit", lastChannel: "whatsapp", lastTo: "+1555", - lastAccountId: undefined, - lastThreadId: undefined, }); }); @@ -207,49 +228,29 @@ describe("resolveSessionDeliveryTarget", () => { fallbackChannel: "slack", }); - expect(resolved).toEqual({ + expectImplicitRoute(resolved, { channel: "slack", to: undefined, - accountId: undefined, - threadId: undefined, - threadIdExplicit: false, - mode: "implicit", lastChannel: "whatsapp", lastTo: "+1555", - lastAccountId: undefined, - lastThreadId: undefined, }); }); it("parses :topic:NNN from explicitTo into threadId", () => { - const resolved = resolveSessionDeliveryTarget({ - entry: { - sessionId: "sess-topic", - updatedAt: 1, - lastChannel: "telegram", - lastTo: "63448508", - }, - requestedChannel: "last", - explicitTo: "63448508:topic:1008013", + expectTopicParsedFromExplicitTo({ + sessionId: "sess-topic", + updatedAt: 1, + lastChannel: "telegram", + lastTo: "63448508", }); - - expect(resolved.to).toBe("63448508"); - expect(resolved.threadId).toBe(1008013); }); it("parses :topic:NNN even when lastTo is absent", () => { - const resolved = resolveSessionDeliveryTarget({ - entry: { - sessionId: "sess-no-last", - updatedAt: 1, - lastChannel: "telegram", - }, - requestedChannel: "last", - explicitTo: "63448508:topic:1008013", + expectTopicParsedFromExplicitTo({ + sessionId: "sess-no-last", + updatedAt: 1, + lastChannel: "telegram", }); - - expect(resolved.to).toBe("63448508"); - expect(resolved.threadId).toBe(1008013); }); it("skips :topic: parsing for non-telegram channels", () => { @@ -365,18 +366,11 @@ describe("resolveSessionDeliveryTarget", () => { }); it("allows heartbeat delivery to Telegram direct chats by default", () => { - const cfg: OpenClawConfig = {}; - const resolved = resolveHeartbeatDeliveryTarget({ - cfg, - entry: { - sessionId: "sess-heartbeat-telegram-direct", - updatedAt: 1, - lastChannel: "telegram", - lastTo: "5232990709", - }, - heartbeat: { - target: "last", - }, + const resolved = resolveHeartbeatTarget({ + sessionId: "sess-heartbeat-telegram-direct", + updatedAt: 1, + lastChannel: "telegram", + lastTo: "5232990709", }); expect(resolved.channel).toBe("telegram"); @@ -384,20 +378,15 @@ describe("resolveSessionDeliveryTarget", () => { }); it("blocks heartbeat delivery to Telegram direct chats when directPolicy is block", () => { - const cfg: OpenClawConfig = {}; - const resolved = resolveHeartbeatDeliveryTarget({ - cfg, - entry: { + const resolved = resolveHeartbeatTarget( + { sessionId: "sess-heartbeat-telegram-direct", updatedAt: 1, lastChannel: "telegram", lastTo: "5232990709", }, - heartbeat: { - target: "last", - directPolicy: "block", - }, - }); + "block", + ); expect(resolved.channel).toBe("none"); expect(resolved.reason).toBe("dm-blocked"); diff --git a/src/infra/process-respawn.test.ts b/src/infra/process-respawn.test.ts index a496330ea..188b942eb 100644 --- a/src/infra/process-respawn.test.ts +++ b/src/infra/process-respawn.test.ts @@ -46,6 +46,19 @@ function clearSupervisorHints() { } } +function expectLaunchdKickstartSupervised(params?: { launchJobLabel?: string }) { + setPlatform("darwin"); + if (params?.launchJobLabel) { + process.env.LAUNCH_JOB_LABEL = params.launchJobLabel; + } + process.env.OPENCLAW_LAUNCHD_LABEL = "ai.openclaw.gateway"; + triggerOpenClawRestartMock.mockReturnValue({ ok: true, method: "launchctl" }); + const result = restartGatewayProcessWithFreshPid(); + expect(result.mode).toBe("supervised"); + expect(triggerOpenClawRestartMock).toHaveBeenCalledOnce(); + expect(spawnMock).not.toHaveBeenCalled(); +} + describe("restartGatewayProcessWithFreshPid", () => { it("returns disabled when OPENCLAW_NO_RESPAWN is set", () => { process.env.OPENCLAW_NO_RESPAWN = "1"; @@ -62,16 +75,7 @@ describe("restartGatewayProcessWithFreshPid", () => { }); it("runs launchd kickstart helper on macOS when launchd label is set", () => { - setPlatform("darwin"); - process.env.LAUNCH_JOB_LABEL = "ai.openclaw.gateway"; - process.env.OPENCLAW_LAUNCHD_LABEL = "ai.openclaw.gateway"; - triggerOpenClawRestartMock.mockReturnValue({ ok: true, method: "launchctl" }); - - const result = restartGatewayProcessWithFreshPid(); - - expect(result.mode).toBe("supervised"); - expect(triggerOpenClawRestartMock).toHaveBeenCalledOnce(); - expect(spawnMock).not.toHaveBeenCalled(); + expectLaunchdKickstartSupervised({ launchJobLabel: "ai.openclaw.gateway" }); }); it("returns failed when launchd kickstart helper fails", () => { @@ -124,13 +128,7 @@ describe("restartGatewayProcessWithFreshPid", () => { it("returns supervised when OPENCLAW_LAUNCHD_LABEL is set (stock launchd plist)", () => { clearSupervisorHints(); - setPlatform("darwin"); - process.env.OPENCLAW_LAUNCHD_LABEL = "ai.openclaw.gateway"; - triggerOpenClawRestartMock.mockReturnValue({ ok: true, method: "launchctl" }); - const result = restartGatewayProcessWithFreshPid(); - expect(result.mode).toBe("supervised"); - expect(triggerOpenClawRestartMock).toHaveBeenCalledOnce(); - expect(spawnMock).not.toHaveBeenCalled(); + expectLaunchdKickstartSupervised(); }); it("returns supervised when OPENCLAW_SYSTEMD_UNIT is set", () => { diff --git a/src/infra/shell-env.test.ts b/src/infra/shell-env.test.ts index 1696028b3..64be7f28f 100644 --- a/src/infra/shell-env.test.ts +++ b/src/infra/shell-env.test.ts @@ -31,15 +31,29 @@ describe("shell env fallback", () => { resetShellPathCacheForTests(); const env: NodeJS.ProcessEnv = { SHELL: shell }; const exec = vi.fn(() => Buffer.from("OPENAI_API_KEY=from-shell\0")); - const res = loadShellEnvFallback({ + const res = runShellEnvFallback({ enabled: true, env, expectedKeys: ["OPENAI_API_KEY"], - exec: exec as unknown as Parameters[0]["exec"], + exec, }); return { res, exec }; } + function runShellEnvFallback(params: { + enabled: boolean; + env: NodeJS.ProcessEnv; + expectedKeys: string[]; + exec: ReturnType; + }) { + return loadShellEnvFallback({ + enabled: params.enabled, + env: params.env, + expectedKeys: params.expectedKeys, + exec: params.exec as unknown as Parameters[0]["exec"], + }); + } + function makeUnsafeStartupEnv(): NodeJS.ProcessEnv { return { SHELL: "/bin/bash", @@ -76,6 +90,29 @@ describe("shell env fallback", () => { } } + function getShellPathTwiceWithExec(params: { + exec: ReturnType; + platform: NodeJS.Platform; + }) { + return getShellPathTwice({ + exec: params.exec as unknown as Parameters[0]["exec"], + platform: params.platform, + }); + } + + function probeShellPathWithFreshCache(params: { + exec: ReturnType; + platform: NodeJS.Platform; + }) { + resetShellPathCacheForTests(); + return getShellPathTwiceWithExec(params); + } + + function expectBinShFallbackExec(exec: ReturnType) { + expect(exec).toHaveBeenCalledTimes(1); + expect(exec).toHaveBeenCalledWith("/bin/sh", ["-l", "-c", "env -0"], expect.any(Object)); + } + it("is disabled by default", () => { expect(shouldEnableShellEnvFallback({} as NodeJS.ProcessEnv)).toBe(false); expect(shouldEnableShellEnvFallback({ OPENCLAW_LOAD_SHELL_ENV: "0" })).toBe(false); @@ -96,11 +133,11 @@ describe("shell env fallback", () => { const env: NodeJS.ProcessEnv = { OPENAI_API_KEY: "set" }; const exec = vi.fn(() => Buffer.from("")); - const res = loadShellEnvFallback({ + const res = runShellEnvFallback({ enabled: true, env, expectedKeys: ["OPENAI_API_KEY", "DISCORD_BOT_TOKEN"], - exec: exec as unknown as Parameters[0]["exec"], + exec, }); expect(res.ok).toBe(true); @@ -113,11 +150,11 @@ describe("shell env fallback", () => { const env: NodeJS.ProcessEnv = {}; const exec = vi.fn(() => Buffer.from("OPENAI_API_KEY=from-shell\0DISCORD_BOT_TOKEN=discord\0")); - const res1 = loadShellEnvFallback({ + const res1 = runShellEnvFallback({ enabled: true, env, expectedKeys: ["OPENAI_API_KEY", "DISCORD_BOT_TOKEN"], - exec: exec as unknown as Parameters[0]["exec"], + exec, }); expect(res1.ok).toBe(true); @@ -129,11 +166,11 @@ describe("shell env fallback", () => { const exec2 = vi.fn(() => Buffer.from("OPENAI_API_KEY=from-shell\0DISCORD_BOT_TOKEN=discord2\0"), ); - const res2 = loadShellEnvFallback({ + const res2 = runShellEnvFallback({ enabled: true, env, expectedKeys: ["OPENAI_API_KEY", "DISCORD_BOT_TOKEN"], - exec: exec2 as unknown as Parameters[0]["exec"], + exec: exec2, }); expect(res2.ok).toBe(true); @@ -143,11 +180,10 @@ describe("shell env fallback", () => { }); it("resolves PATH via login shell and caches it", () => { - resetShellPathCacheForTests(); const exec = vi.fn(() => Buffer.from("PATH=/usr/local/bin:/usr/bin\0HOME=/tmp\0")); - const { first, second } = getShellPathTwice({ - exec: exec as unknown as Parameters[0]["exec"], + const { first, second } = probeShellPathWithFreshCache({ + exec, platform: "linux", }); @@ -157,13 +193,12 @@ describe("shell env fallback", () => { }); it("returns null on shell env read failure and caches null", () => { - resetShellPathCacheForTests(); const exec = vi.fn(() => { throw new Error("exec failed"); }); - const { first, second } = getShellPathTwice({ - exec: exec as unknown as Parameters[0]["exec"], + const { first, second } = probeShellPathWithFreshCache({ + exec, platform: "linux", }); @@ -176,16 +211,14 @@ describe("shell env fallback", () => { const { res, exec } = runShellEnvFallbackForShell("zsh"); expect(res.ok).toBe(true); - expect(exec).toHaveBeenCalledTimes(1); - expect(exec).toHaveBeenCalledWith("/bin/sh", ["-l", "-c", "env -0"], expect.any(Object)); + expectBinShFallbackExec(exec); }); it("falls back to /bin/sh when SHELL points to an untrusted path", () => { const { res, exec } = runShellEnvFallbackForShell("/tmp/evil-shell"); expect(res.ok).toBe(true); - expect(exec).toHaveBeenCalledTimes(1); - expect(exec).toHaveBeenCalledWith("/bin/sh", ["-l", "-c", "env -0"], expect.any(Object)); + expectBinShFallbackExec(exec); }); it("falls back to /bin/sh when SHELL is absolute but not registered in /etc/shells", () => { @@ -193,8 +226,7 @@ describe("shell env fallback", () => { const { res, exec } = runShellEnvFallbackForShell("/opt/homebrew/bin/evil-shell"); expect(res.ok).toBe(true); - expect(exec).toHaveBeenCalledTimes(1); - expect(exec).toHaveBeenCalledWith("/bin/sh", ["-l", "-c", "env -0"], expect.any(Object)); + expectBinShFallbackExec(exec); }); }); @@ -220,11 +252,11 @@ describe("shell env fallback", () => { return Buffer.from("OPENAI_API_KEY=from-shell\0"); }); - const res = loadShellEnvFallback({ + const res = runShellEnvFallback({ enabled: true, env, expectedKeys: ["OPENAI_API_KEY"], - exec: exec as unknown as Parameters[0]["exec"], + exec, }); expect(res.ok).toBe(true); @@ -253,11 +285,10 @@ describe("shell env fallback", () => { }); it("returns null without invoking shell on win32", () => { - resetShellPathCacheForTests(); const exec = vi.fn(() => Buffer.from("PATH=/usr/local/bin:/usr/bin\0HOME=/tmp\0")); - const { first, second } = getShellPathTwice({ - exec: exec as unknown as Parameters[0]["exec"], + const { first, second } = probeShellPathWithFreshCache({ + exec, platform: "win32", }); diff --git a/src/infra/tmp-openclaw-dir.test.ts b/src/infra/tmp-openclaw-dir.test.ts index 4c0a68b90..890565138 100644 --- a/src/infra/tmp-openclaw-dir.test.ts +++ b/src/infra/tmp-openclaw-dir.test.ts @@ -23,6 +23,72 @@ function secureDirStat(uid = 501) { }; } +function makeDirStat(params?: { + isDirectory?: boolean; + isSymbolicLink?: boolean; + uid?: number; + mode?: number; +}) { + return { + isDirectory: () => params?.isDirectory ?? true, + isSymbolicLink: () => params?.isSymbolicLink ?? false, + uid: params?.uid ?? 501, + mode: params?.mode ?? 0o40700, + }; +} + +function readOnlyTmpAccessSync() { + return vi.fn((target: string) => { + if (target === "/tmp") { + throw new Error("read-only"); + } + }); +} + +function resolveWithReadOnlyTmpFallback(params: { + fallbackPath: string; + fallbackLstatSync: NonNullable; + chmodSync?: NonNullable; + warn?: NonNullable; +}) { + return resolvePreferredOpenClawTmpDir({ + accessSync: readOnlyTmpAccessSync(), + lstatSync: vi.fn((target: string) => { + if (target === POSIX_OPENCLAW_TMP_DIR) { + throw nodeErrorWithCode("ENOENT"); + } + if (target === params.fallbackPath) { + return params.fallbackLstatSync(target); + } + return secureDirStat(501); + }), + mkdirSync: vi.fn(), + chmodSync: params.chmodSync, + getuid: vi.fn(() => 501), + tmpdir: vi.fn(() => "/var/fallback"), + warn: params.warn, + }); +} + +function symlinkTmpDirLstat() { + return vi.fn(() => makeDirStat({ isSymbolicLink: true, mode: 0o120777 })); +} + +function expectFallsBackToOsTmpDir(params: { lstatSync: NonNullable }) { + const { resolved, tmpdir } = resolveWithMocks({ lstatSync: params.lstatSync }); + expect(resolved).toBe(fallbackTmp()); + expect(tmpdir).toHaveBeenCalled(); +} + +function missingThenSecureLstat(uid = 501) { + return vi + .fn>() + .mockImplementationOnce(() => { + throw nodeErrorWithCode("ENOENT"); + }) + .mockImplementationOnce(() => secureDirStat(uid)); +} + function resolveWithMocks(params: { lstatSync: NonNullable; fallbackLstatSync?: NonNullable; @@ -81,12 +147,7 @@ describe("resolvePreferredOpenClawTmpDir", () => { }); it("prefers /tmp/openclaw when it does not exist but /tmp is writable", () => { - const lstatSyncMock = vi - .fn>() - .mockImplementationOnce(() => { - throw nodeErrorWithCode("ENOENT"); - }) - .mockImplementationOnce(() => secureDirStat(501)); + const lstatSyncMock = missingThenSecureLstat(); const { resolved, accessSync, mkdirSync, tmpdir } = resolveWithMocks({ lstatSync: lstatSyncMock, @@ -99,12 +160,7 @@ describe("resolvePreferredOpenClawTmpDir", () => { }); it("falls back to os.tmpdir()/openclaw when /tmp/openclaw is not a directory", () => { - const lstatSync = vi.fn(() => ({ - isDirectory: () => false, - isSymbolicLink: () => false, - uid: 501, - mode: 0o100644, - })) as unknown as ReturnType & NonNullable; + const lstatSync = vi.fn(() => makeDirStat({ isDirectory: false, mode: 0o100644 })); const { resolved, tmpdir } = resolveWithMocks({ lstatSync }); expect(resolved).toBe(fallbackTmp()); @@ -130,59 +186,20 @@ describe("resolvePreferredOpenClawTmpDir", () => { }); it("falls back when /tmp/openclaw is a symlink", () => { - const lstatSync = vi.fn(() => ({ - isDirectory: () => true, - isSymbolicLink: () => true, - uid: 501, - mode: 0o120777, - })); - - const { resolved, tmpdir } = resolveWithMocks({ lstatSync }); - - expect(resolved).toBe(fallbackTmp()); - expect(tmpdir).toHaveBeenCalled(); + expectFallsBackToOsTmpDir({ lstatSync: symlinkTmpDirLstat() }); }); it("falls back when /tmp/openclaw is not owned by the current user", () => { - const lstatSync = vi.fn(() => ({ - isDirectory: () => true, - isSymbolicLink: () => false, - uid: 0, - mode: 0o40700, - })); - - const { resolved, tmpdir } = resolveWithMocks({ lstatSync }); - - expect(resolved).toBe(fallbackTmp()); - expect(tmpdir).toHaveBeenCalled(); + expectFallsBackToOsTmpDir({ lstatSync: vi.fn(() => makeDirStat({ uid: 0 })) }); }); it("falls back when /tmp/openclaw is group/other writable", () => { - const lstatSync = vi.fn(() => ({ - isDirectory: () => true, - isSymbolicLink: () => false, - uid: 501, - mode: 0o40777, - })); - const { resolved, tmpdir } = resolveWithMocks({ lstatSync }); - - expect(resolved).toBe(fallbackTmp()); - expect(tmpdir).toHaveBeenCalled(); + expectFallsBackToOsTmpDir({ lstatSync: vi.fn(() => makeDirStat({ mode: 0o40777 })) }); }); it("throws when fallback path is a symlink", () => { - const lstatSync = vi.fn(() => ({ - isDirectory: () => true, - isSymbolicLink: () => true, - uid: 501, - mode: 0o120777, - })); - const fallbackLstatSync = vi.fn(() => ({ - isDirectory: () => true, - isSymbolicLink: () => true, - uid: 501, - mode: 0o120777, - })); + const lstatSync = symlinkTmpDirLstat(); + const fallbackLstatSync = vi.fn(() => makeDirStat({ isSymbolicLink: true, mode: 0o120777 })); expect(() => resolveWithMocks({ @@ -193,18 +210,8 @@ describe("resolvePreferredOpenClawTmpDir", () => { }); it("creates fallback directory when missing, then validates ownership and mode", () => { - const lstatSync = vi.fn(() => ({ - isDirectory: () => true, - isSymbolicLink: () => true, - uid: 501, - mode: 0o120777, - })); - const fallbackLstatSync = vi - .fn>() - .mockImplementationOnce(() => { - throw nodeErrorWithCode("ENOENT"); - }) - .mockImplementationOnce(() => secureDirStat(501)); + const lstatSync = symlinkTmpDirLstat(); + const fallbackLstatSync = missingThenSecureLstat(); const { resolved, mkdirSync } = resolveWithMocks({ lstatSync, @@ -238,25 +245,15 @@ describe("resolvePreferredOpenClawTmpDir", () => { } }); - const resolved = resolvePreferredOpenClawTmpDir({ - accessSync: vi.fn((target: string) => { - if (target === "/tmp") { - throw new Error("read-only"); - } - }), - lstatSync: vi.fn((target: string) => { - if (target === POSIX_OPENCLAW_TMP_DIR) { - return lstatSync(target); - } + const resolved = resolveWithReadOnlyTmpFallback({ + fallbackPath, + fallbackLstatSync: vi.fn((target: string) => { if (target === fallbackPath) { return fallbackLstatSync(target); } - return secureDirStat(501); + return lstatSync(target); }), - mkdirSync: vi.fn(), chmodSync, - getuid: vi.fn(() => 501), - tmpdir: vi.fn(() => "/var/fallback"), warn: vi.fn(), }); @@ -274,30 +271,15 @@ describe("resolvePreferredOpenClawTmpDir", () => { }); const warn = vi.fn(); - const resolved = resolvePreferredOpenClawTmpDir({ - accessSync: vi.fn((target: string) => { - if (target === "/tmp") { - throw new Error("read-only"); - } - }), - lstatSync: vi.fn((target: string) => { - if (target === POSIX_OPENCLAW_TMP_DIR) { - throw nodeErrorWithCode("ENOENT"); - } - if (target === fallbackPath) { - return { - isDirectory: () => true, - isSymbolicLink: () => false, - uid: 501, - mode: fallbackMode, - }; - } - return secureDirStat(501); - }), - mkdirSync: vi.fn(), + const resolved = resolveWithReadOnlyTmpFallback({ + fallbackPath, + fallbackLstatSync: vi.fn(() => + makeDirStat({ + isSymbolicLink: false, + mode: fallbackMode, + }), + ), chmodSync, - getuid: vi.fn(() => 501), - tmpdir: vi.fn(() => "/var/fallback"), warn, }); diff --git a/src/markdown/ir.ts b/src/markdown/ir.ts index bab451bc3..c8b942ba4 100644 --- a/src/markdown/ir.ts +++ b/src/markdown/ir.ts @@ -400,6 +400,30 @@ function appendCellTextOnly(state: RenderState, cell: TableCell) { // Do not append styles - this is used for code blocks where inner styles would overlap } +function appendTableBulletValue( + state: RenderState, + params: { + header?: TableCell; + value?: TableCell; + columnIndex: number; + includeColumnFallback: boolean; + }, +) { + const { header, value, columnIndex, includeColumnFallback } = params; + if (!value?.text) { + return; + } + state.text += "• "; + if (header?.text) { + appendCell(state, header); + state.text += ": "; + } else if (includeColumnFallback) { + state.text += `Column ${columnIndex}: `; + } + appendCell(state, value); + state.text += "\n"; +} + function renderTableAsBullets(state: RenderState) { if (!state.table) { return; @@ -436,20 +460,12 @@ function renderTableAsBullets(state: RenderState) { // Add each column as a bullet point for (let i = 1; i < row.length; i++) { - const header = headers[i]; - const value = row[i]; - if (!value?.text) { - continue; - } - state.text += "• "; - if (header?.text) { - appendCell(state, header); - state.text += ": "; - } else { - state.text += `Column ${i}: `; - } - appendCell(state, value); - state.text += "\n"; + appendTableBulletValue(state, { + header: headers[i], + value: row[i], + columnIndex: i, + includeColumnFallback: true, + }); } state.text += "\n"; } @@ -457,18 +473,12 @@ function renderTableAsBullets(state: RenderState) { // Simple table: just list headers and values for (const row of rows) { for (let i = 0; i < row.length; i++) { - const header = headers[i]; - const value = row[i]; - if (!value?.text) { - continue; - } - state.text += "• "; - if (header?.text) { - appendCell(state, header); - state.text += ": "; - } - appendCell(state, value); - state.text += "\n"; + appendTableBulletValue(state, { + header: headers[i], + value: row[i], + columnIndex: i, + includeColumnFallback: false, + }); } state.text += "\n"; } @@ -813,6 +823,19 @@ function mergeStyleSpans(spans: MarkdownStyleSpan[]): MarkdownStyleSpan[] { return merged; } +function resolveSliceBounds( + span: { start: number; end: number }, + start: number, + end: number, +): { start: number; end: number } | null { + const sliceStart = Math.max(span.start, start); + const sliceEnd = Math.min(span.end, end); + if (sliceEnd <= sliceStart) { + return null; + } + return { start: sliceStart, end: sliceEnd }; +} + function sliceStyleSpans( spans: MarkdownStyleSpan[], start: number, @@ -823,15 +846,15 @@ function sliceStyleSpans( } const sliced: MarkdownStyleSpan[] = []; for (const span of spans) { - const sliceStart = Math.max(span.start, start); - const sliceEnd = Math.min(span.end, end); - if (sliceEnd > sliceStart) { - sliced.push({ - start: sliceStart - start, - end: sliceEnd - start, - style: span.style, - }); + const bounds = resolveSliceBounds(span, start, end); + if (!bounds) { + continue; } + sliced.push({ + start: bounds.start - start, + end: bounds.end - start, + style: span.style, + }); } return mergeStyleSpans(sliced); } @@ -842,15 +865,15 @@ function sliceLinkSpans(spans: MarkdownLinkSpan[], start: number, end: number): } const sliced: MarkdownLinkSpan[] = []; for (const span of spans) { - const sliceStart = Math.max(span.start, start); - const sliceEnd = Math.min(span.end, end); - if (sliceEnd > sliceStart) { - sliced.push({ - start: sliceStart - start, - end: sliceEnd - start, - href: span.href, - }); + const bounds = resolveSliceBounds(span, start, end); + if (!bounds) { + continue; } + sliced.push({ + start: bounds.start - start, + end: bounds.end - start, + href: span.href, + }); } return sliced; } diff --git a/src/media-understanding/runner.skip-tiny-audio.test.ts b/src/media-understanding/runner.skip-tiny-audio.test.ts index 6bc18991e..a4e5d2e2d 100644 --- a/src/media-understanding/runner.skip-tiny-audio.test.ts +++ b/src/media-understanding/runner.skip-tiny-audio.test.ts @@ -11,6 +11,7 @@ import { normalizeMediaAttachments, runCapability, } from "./runner.js"; +import type { AudioTranscriptionRequest } from "./types.js"; async function withAudioFixture(params: { filePrefix: string; @@ -47,6 +48,41 @@ async function withAudioFixture(params: { } } +const AUDIO_CAPABILITY_CFG = { + models: { + providers: { + openai: { + apiKey: "test-key", + models: [], + }, + }, + }, +} as unknown as OpenClawConfig; + +async function runAudioCapabilityWithTranscriber(params: { + ctx: MsgContext; + media: ReturnType; + cache: ReturnType; + transcribeAudio: (req: AudioTranscriptionRequest) => Promise<{ text: string; model: string }>; +}) { + const providerRegistry = buildProviderRegistry({ + openai: { + id: "openai", + capabilities: ["audio"], + transcribeAudio: params.transcribeAudio, + }, + }); + + return await runCapability({ + capability: "audio", + cfg: AUDIO_CAPABILITY_CFG, + ctx: params.ctx, + attachments: params.cache, + media: params.media, + providerRegistry, + }); +} + describe("runCapability skips tiny audio files", () => { it("skips audio transcription when file is smaller than MIN_AUDIO_FILE_BYTES", async () => { await withAudioFixture({ @@ -56,35 +92,14 @@ describe("runCapability skips tiny audio files", () => { fileContents: Buffer.alloc(100), // 100 bytes, way below 1024 run: async ({ ctx, media, cache }) => { let transcribeCalled = false; - const providerRegistry = buildProviderRegistry({ - openai: { - id: "openai", - capabilities: ["audio"], - transcribeAudio: async (req) => { - transcribeCalled = true; - return { text: "should not happen", model: req.model }; - }, - }, - }); - - const cfg = { - models: { - providers: { - openai: { - apiKey: "test-key", - models: [], - }, - }, - }, - } as unknown as OpenClawConfig; - - const result = await runCapability({ - capability: "audio", - cfg, + const result = await runAudioCapabilityWithTranscriber({ ctx, - attachments: cache, media, - providerRegistry, + cache, + transcribeAudio: async (req) => { + transcribeCalled = true; + return { text: "should not happen", model: req.model }; + }, }); // The provider should never be called @@ -109,35 +124,14 @@ describe("runCapability skips tiny audio files", () => { fileContents: Buffer.alloc(0), run: async ({ ctx, media, cache }) => { let transcribeCalled = false; - const providerRegistry = buildProviderRegistry({ - openai: { - id: "openai", - capabilities: ["audio"], - transcribeAudio: async () => { - transcribeCalled = true; - return { text: "nope", model: "whisper-1" }; - }, - }, - }); - - const cfg = { - models: { - providers: { - openai: { - apiKey: "test-key", - models: [], - }, - }, - }, - } as unknown as OpenClawConfig; - - const result = await runCapability({ - capability: "audio", - cfg, + const result = await runAudioCapabilityWithTranscriber({ ctx, - attachments: cache, media, - providerRegistry, + cache, + transcribeAudio: async () => { + transcribeCalled = true; + return { text: "nope", model: "whisper-1" }; + }, }); expect(transcribeCalled).toBe(false); @@ -154,35 +148,14 @@ describe("runCapability skips tiny audio files", () => { fileContents: Buffer.alloc(MIN_AUDIO_FILE_BYTES + 100), run: async ({ ctx, media, cache }) => { let transcribeCalled = false; - const providerRegistry = buildProviderRegistry({ - openai: { - id: "openai", - capabilities: ["audio"], - transcribeAudio: async (req) => { - transcribeCalled = true; - return { text: "hello world", model: req.model }; - }, - }, - }); - - const cfg = { - models: { - providers: { - openai: { - apiKey: "test-key", - models: [], - }, - }, - }, - } as unknown as OpenClawConfig; - - const result = await runCapability({ - capability: "audio", - cfg, + const result = await runAudioCapabilityWithTranscriber({ ctx, - attachments: cache, media, - providerRegistry, + cache, + transcribeAudio: async (req) => { + transcribeCalled = true; + return { text: "hello world", model: req.model }; + }, }); expect(transcribeCalled).toBe(true); diff --git a/src/memory/batch-voyage.ts b/src/memory/batch-voyage.ts index 35bd0d4e6..1835f9b05 100644 --- a/src/memory/batch-voyage.ts +++ b/src/memory/batch-voyage.ts @@ -36,6 +36,29 @@ export const VOYAGE_BATCH_ENDPOINT = EMBEDDING_BATCH_ENDPOINT; const VOYAGE_BATCH_COMPLETION_WINDOW = "12h"; const VOYAGE_BATCH_MAX_REQUESTS = 50000; +async function assertVoyageResponseOk(res: Response, context: string): Promise { + if (!res.ok) { + const text = await res.text(); + throw new Error(`${context}: ${res.status} ${text}`); + } +} + +function buildVoyageBatchRequest(params: { + client: VoyageEmbeddingClient; + path: string; + onResponse: (res: Response) => Promise; +}) { + const baseUrl = normalizeBatchBaseUrl(params.client); + return { + url: `${baseUrl}/${params.path}`, + ssrfPolicy: params.client.ssrfPolicy, + init: { + headers: buildBatchHeaders(params.client, { json: true }), + }, + onResponse: params.onResponse, + }; +} + async function submitVoyageBatch(params: { client: VoyageEmbeddingClient; requests: VoyageBatchRequest[]; @@ -74,21 +97,16 @@ async function fetchVoyageBatchStatus(params: { client: VoyageEmbeddingClient; batchId: string; }): Promise { - const baseUrl = normalizeBatchBaseUrl(params.client); - return await withRemoteHttpResponse({ - url: `${baseUrl}/batches/${params.batchId}`, - ssrfPolicy: params.client.ssrfPolicy, - init: { - headers: buildBatchHeaders(params.client, { json: true }), - }, - onResponse: async (res) => { - if (!res.ok) { - const text = await res.text(); - throw new Error(`voyage batch status failed: ${res.status} ${text}`); - } - return (await res.json()) as VoyageBatchStatus; - }, - }); + return await withRemoteHttpResponse( + buildVoyageBatchRequest({ + client: params.client, + path: `batches/${params.batchId}`, + onResponse: async (res) => { + await assertVoyageResponseOk(res, "voyage batch status failed"); + return (await res.json()) as VoyageBatchStatus; + }, + }), + ); } async function readVoyageBatchError(params: { @@ -96,30 +114,25 @@ async function readVoyageBatchError(params: { errorFileId: string; }): Promise { try { - const baseUrl = normalizeBatchBaseUrl(params.client); - return await withRemoteHttpResponse({ - url: `${baseUrl}/files/${params.errorFileId}/content`, - ssrfPolicy: params.client.ssrfPolicy, - init: { - headers: buildBatchHeaders(params.client, { json: true }), - }, - onResponse: async (res) => { - if (!res.ok) { + return await withRemoteHttpResponse( + buildVoyageBatchRequest({ + client: params.client, + path: `files/${params.errorFileId}/content`, + onResponse: async (res) => { + await assertVoyageResponseOk(res, "voyage batch error file content failed"); const text = await res.text(); - throw new Error(`voyage batch error file content failed: ${res.status} ${text}`); - } - const text = await res.text(); - if (!text.trim()) { - return undefined; - } - const lines = text - .split("\n") - .map((line) => line.trim()) - .filter(Boolean) - .map((line) => JSON.parse(line) as VoyageBatchOutputLine); - return extractBatchErrorMessage(lines); - }, - }); + if (!text.trim()) { + return undefined; + } + const lines = text + .split("\n") + .map((line) => line.trim()) + .filter(Boolean) + .map((line) => JSON.parse(line) as VoyageBatchOutputLine); + return extractBatchErrorMessage(lines); + }, + }), + ); } catch (err) { return formatUnavailableBatchError(err); } diff --git a/src/pairing/setup-code.ts b/src/pairing/setup-code.ts index d6b0ca2de..afeb447f4 100644 --- a/src/pairing/setup-code.ts +++ b/src/pairing/setup-code.ts @@ -1,11 +1,10 @@ import os from "node:os"; +import { resolveGatewayPort } from "../config/paths.js"; import type { OpenClawConfig } from "../config/types.js"; import { resolveGatewayBindUrl } from "../shared/gateway-bind-url.js"; import { isCarrierGradeNatIpv4Address, isRfc1918Ipv4Address } from "../shared/net/ip.js"; import { resolveTailnetHostWithRunner } from "../shared/tailscale-status.js"; -const DEFAULT_GATEWAY_PORT = 18789; - export type PairingSetupPayload = { url: string; token?: string; @@ -89,21 +88,6 @@ function normalizeUrl(raw: string, schemeFallback: "ws" | "wss"): string | null return `${schemeFallback}://${withoutPath}`; } -function resolveGatewayPort(cfg: OpenClawConfig, env: NodeJS.ProcessEnv): number { - const envRaw = env.OPENCLAW_GATEWAY_PORT?.trim() || env.CLAWDBOT_GATEWAY_PORT?.trim(); - if (envRaw) { - const parsed = Number.parseInt(envRaw, 10); - if (Number.isFinite(parsed) && parsed > 0) { - return parsed; - } - } - const configPort = cfg.gateway?.port; - if (typeof configPort === "number" && Number.isFinite(configPort) && configPort > 0) { - return configPort; - } - return DEFAULT_GATEWAY_PORT; -} - function resolveScheme( cfg: OpenClawConfig, opts?: { diff --git a/src/plugins/install.test.ts b/src/plugins/install.test.ts index c1078e05a..113483f87 100644 --- a/src/plugins/install.test.ts +++ b/src/plugins/install.test.ts @@ -555,6 +555,18 @@ describe("installPluginFromArchive", () => { }); describe("installPluginFromDir", () => { + function expectInstalledAsMemoryCognee( + result: Awaited>, + extensionsDir: string, + ) { + expect(result.ok).toBe(true); + if (!result.ok) { + return; + } + expect(result.pluginId).toBe("memory-cognee"); + expect(result.targetDir).toBe(path.join(extensionsDir, "memory-cognee")); + } + it("uses --ignore-scripts for dependency install", async () => { const { pluginDir, extensionsDir } = setupInstallPluginFromDirFixture(); @@ -617,12 +629,7 @@ describe("installPluginFromDir", () => { logger: { info: (msg: string) => infoMessages.push(msg), warn: () => {} }, }); - expect(res.ok).toBe(true); - if (!res.ok) { - return; - } - expect(res.pluginId).toBe("memory-cognee"); - expect(res.targetDir).toBe(path.join(extensionsDir, "memory-cognee")); + expectInstalledAsMemoryCognee(res, extensionsDir); expect( infoMessages.some((msg) => msg.includes( @@ -644,12 +651,7 @@ describe("installPluginFromDir", () => { logger: { info: () => {}, warn: () => {} }, }); - expect(res.ok).toBe(true); - if (!res.ok) { - return; - } - expect(res.pluginId).toBe("memory-cognee"); - expect(res.targetDir).toBe(path.join(extensionsDir, "memory-cognee")); + expectInstalledAsMemoryCognee(res, extensionsDir); }); }); diff --git a/src/plugins/install.ts b/src/plugins/install.ts index 155fb5a0a..6860568cd 100644 --- a/src/plugins/install.ts +++ b/src/plugins/install.ts @@ -147,6 +147,42 @@ function buildFileInstallResult(pluginId: string, targetFile: string): InstallPl }; } +type PackageInstallCommonParams = { + extensionsDir?: string; + timeoutMs?: number; + logger?: PluginInstallLogger; + mode?: "install" | "update"; + dryRun?: boolean; + expectedPluginId?: string; +}; + +type FileInstallCommonParams = Pick< + PackageInstallCommonParams, + "extensionsDir" | "logger" | "mode" | "dryRun" +>; + +function pickPackageInstallCommonParams( + params: PackageInstallCommonParams, +): PackageInstallCommonParams { + return { + extensionsDir: params.extensionsDir, + timeoutMs: params.timeoutMs, + logger: params.logger, + mode: params.mode, + dryRun: params.dryRun, + expectedPluginId: params.expectedPluginId, + }; +} + +function pickFileInstallCommonParams(params: FileInstallCommonParams): FileInstallCommonParams { + return { + extensionsDir: params.extensionsDir, + logger: params.logger, + mode: params.mode, + dryRun: params.dryRun, + }; +} + export function resolvePluginInstallDir(pluginId: string, extensionsDir?: string): string { const extensionsBase = extensionsDir ? resolveUserPath(extensionsDir) @@ -166,15 +202,11 @@ export function resolvePluginInstallDir(pluginId: string, extensionsDir?: string return targetDirResult.path; } -async function installPluginFromPackageDir(params: { - packageDir: string; - extensionsDir?: string; - timeoutMs?: number; - logger?: PluginInstallLogger; - mode?: "install" | "update"; - dryRun?: boolean; - expectedPluginId?: string; -}): Promise { +async function installPluginFromPackageDir( + params: { + packageDir: string; + } & PackageInstallCommonParams, +): Promise { const { logger, timeoutMs, mode, dryRun } = resolveTimedInstallModeOptions(params, defaultLogger); const manifestPath = path.join(params.packageDir, "package.json"); @@ -344,15 +376,11 @@ async function installPluginFromPackageDir(params: { }; } -export async function installPluginFromArchive(params: { - archivePath: string; - extensionsDir?: string; - timeoutMs?: number; - logger?: PluginInstallLogger; - mode?: "install" | "update"; - dryRun?: boolean; - expectedPluginId?: string; -}): Promise { +export async function installPluginFromArchive( + params: { + archivePath: string; + } & PackageInstallCommonParams, +): Promise { const logger = params.logger ?? defaultLogger; const timeoutMs = params.timeoutMs ?? 120_000; const mode = params.mode ?? "install"; @@ -370,25 +398,23 @@ export async function installPluginFromArchive(params: { onExtracted: async (packageDir) => await installPluginFromPackageDir({ packageDir, - extensionsDir: params.extensionsDir, - timeoutMs, - logger, - mode, - dryRun: params.dryRun, - expectedPluginId: params.expectedPluginId, + ...pickPackageInstallCommonParams({ + extensionsDir: params.extensionsDir, + timeoutMs, + logger, + mode, + dryRun: params.dryRun, + expectedPluginId: params.expectedPluginId, + }), }), }); } -export async function installPluginFromDir(params: { - dirPath: string; - extensionsDir?: string; - timeoutMs?: number; - logger?: PluginInstallLogger; - mode?: "install" | "update"; - dryRun?: boolean; - expectedPluginId?: string; -}): Promise { +export async function installPluginFromDir( + params: { + dirPath: string; + } & PackageInstallCommonParams, +): Promise { const dirPath = resolveUserPath(params.dirPath); if (!(await fileExists(dirPath))) { return { ok: false, error: `directory not found: ${dirPath}` }; @@ -400,12 +426,7 @@ export async function installPluginFromDir(params: { return await installPluginFromPackageDir({ packageDir: dirPath, - extensionsDir: params.extensionsDir, - timeoutMs: params.timeoutMs, - logger: params.logger, - mode: params.mode, - dryRun: params.dryRun, - expectedPluginId: params.expectedPluginId, + ...pickPackageInstallCommonParams(params), }); } @@ -517,30 +538,22 @@ export async function installPluginFromNpmSpec(params: { return finalized; } -export async function installPluginFromPath(params: { - path: string; - extensionsDir?: string; - timeoutMs?: number; - logger?: PluginInstallLogger; - mode?: "install" | "update"; - dryRun?: boolean; - expectedPluginId?: string; -}): Promise { +export async function installPluginFromPath( + params: { + path: string; + } & PackageInstallCommonParams, +): Promise { const pathResult = await resolveExistingInstallPath(params.path); if (!pathResult.ok) { return pathResult; } const { resolvedPath: resolved, stat } = pathResult; + const packageInstallOptions = pickPackageInstallCommonParams(params); if (stat.isDirectory()) { return await installPluginFromDir({ dirPath: resolved, - extensionsDir: params.extensionsDir, - timeoutMs: params.timeoutMs, - logger: params.logger, - mode: params.mode, - dryRun: params.dryRun, - expectedPluginId: params.expectedPluginId, + ...packageInstallOptions, }); } @@ -548,20 +561,12 @@ export async function installPluginFromPath(params: { if (archiveKind) { return await installPluginFromArchive({ archivePath: resolved, - extensionsDir: params.extensionsDir, - timeoutMs: params.timeoutMs, - logger: params.logger, - mode: params.mode, - dryRun: params.dryRun, - expectedPluginId: params.expectedPluginId, + ...packageInstallOptions, }); } return await installPluginFromFile({ filePath: resolved, - extensionsDir: params.extensionsDir, - logger: params.logger, - mode: params.mode, - dryRun: params.dryRun, + ...pickFileInstallCommonParams(params), }); } diff --git a/src/security/dm-policy-shared.test.ts b/src/security/dm-policy-shared.test.ts index 042b64600..da28a17ae 100644 --- a/src/security/dm-policy-shared.test.ts +++ b/src/security/dm-policy-shared.test.ts @@ -17,6 +17,43 @@ describe("security/dm-policy-shared", () => { hasControlCommand: true, } as const; + async function expectStoreReadSkipped(params: { + provider: string; + accountId: string; + dmPolicy?: "open" | "allowlist" | "pairing" | "disabled"; + shouldRead?: boolean; + }) { + let called = false; + const storeAllowFrom = await readStoreAllowFromForDmPolicy({ + provider: params.provider, + accountId: params.accountId, + ...(params.dmPolicy ? { dmPolicy: params.dmPolicy } : {}), + ...(params.shouldRead !== undefined ? { shouldRead: params.shouldRead } : {}), + readStore: async (_provider, _accountId) => { + called = true; + return ["should-not-be-read"]; + }, + }); + expect(called).toBe(false); + expect(storeAllowFrom).toEqual([]); + } + + function resolveCommandGate(overrides: { + isGroup: boolean; + isSenderAllowed: (allowFrom: string[]) => boolean; + groupPolicy?: "open" | "allowlist" | "disabled"; + }) { + return resolveDmGroupAccessWithCommandGate({ + dmPolicy: "pairing", + groupPolicy: overrides.groupPolicy ?? "allowlist", + allowFrom: ["owner"], + groupAllowFrom: ["group-owner"], + storeAllowFrom: ["paired-user"], + command: controlCommand, + ...overrides, + }); + } + it("normalizes config + store allow entries and counts distinct senders", async () => { const state = await resolveDmAllowState({ provider: "telegram", @@ -47,33 +84,19 @@ describe("security/dm-policy-shared", () => { }); it("skips pairing-store reads when dmPolicy is allowlist", async () => { - let called = false; - const storeAllowFrom = await readStoreAllowFromForDmPolicy({ + await expectStoreReadSkipped({ provider: "telegram", accountId: "default", dmPolicy: "allowlist", - readStore: async (_provider, _accountId) => { - called = true; - return ["should-not-be-read"]; - }, }); - expect(called).toBe(false); - expect(storeAllowFrom).toEqual([]); }); it("skips pairing-store reads when shouldRead=false", async () => { - let called = false; - const storeAllowFrom = await readStoreAllowFromForDmPolicy({ + await expectStoreReadSkipped({ provider: "slack", accountId: "default", shouldRead: false, - readStore: async (_provider, _accountId) => { - called = true; - return ["should-not-be-read"]; - }, }); - expect(called).toBe(false); - expect(storeAllowFrom).toEqual([]); }); it("builds effective DM/group allowlists from config + pairing store", () => { @@ -184,15 +207,9 @@ describe("security/dm-policy-shared", () => { }); it("resolves command gate with dm/group parity for groups", () => { - const resolved = resolveDmGroupAccessWithCommandGate({ + const resolved = resolveCommandGate({ isGroup: true, - dmPolicy: "pairing", - groupPolicy: "allowlist", - allowFrom: ["owner"], - groupAllowFrom: ["group-owner"], - storeAllowFrom: ["paired-user"], isSenderAllowed: (allowFrom) => allowFrom.includes("paired-user"), - command: controlCommand, }); expect(resolved.decision).toBe("block"); expect(resolved.reason).toBe("groupPolicy=allowlist (not allowlisted)"); @@ -216,15 +233,9 @@ describe("security/dm-policy-shared", () => { }); it("treats dm command authorization as dm access result", () => { - const resolved = resolveDmGroupAccessWithCommandGate({ + const resolved = resolveCommandGate({ isGroup: false, - dmPolicy: "pairing", - groupPolicy: "allowlist", - allowFrom: ["owner"], - groupAllowFrom: ["group-owner"], - storeAllowFrom: ["paired-user"], isSenderAllowed: (allowFrom) => allowFrom.includes("paired-user"), - command: controlCommand, }); expect(resolved.decision).toBe("allow"); expect(resolved.commandAuthorized).toBe(true); @@ -274,80 +285,83 @@ describe("security/dm-policy-shared", () => { "zalo", ] as const; + type ParityCase = { + name: string; + isGroup: boolean; + dmPolicy: "open" | "allowlist" | "pairing" | "disabled"; + groupPolicy: "open" | "allowlist" | "disabled"; + allowFrom: string[]; + groupAllowFrom: string[]; + storeAllowFrom: string[]; + isSenderAllowed: (allowFrom: string[]) => boolean; + expectedDecision: "allow" | "block" | "pairing"; + expectedReactionAllowed: boolean; + }; + + function createParityCase(overrides: Partial & Pick): ParityCase { + return { + name: overrides.name, + isGroup: false, + dmPolicy: "open", + groupPolicy: "allowlist", + allowFrom: [], + groupAllowFrom: [], + storeAllowFrom: [], + isSenderAllowed: () => false, + expectedDecision: "allow", + expectedReactionAllowed: true, + ...overrides, + }; + } + it("keeps message/reaction policy parity table across channels", () => { const cases = [ - { + createParityCase({ name: "dmPolicy=open", - isGroup: false, - dmPolicy: "open" as const, - groupPolicy: "allowlist" as const, - allowFrom: [] as string[], - groupAllowFrom: [] as string[], - storeAllowFrom: [] as string[], - isSenderAllowed: () => false, - expectedDecision: "allow" as const, + dmPolicy: "open", + expectedDecision: "allow", expectedReactionAllowed: true, - }, - { + }), + createParityCase({ name: "dmPolicy=disabled", - isGroup: false, - dmPolicy: "disabled" as const, - groupPolicy: "allowlist" as const, - allowFrom: [] as string[], - groupAllowFrom: [] as string[], - storeAllowFrom: [] as string[], - isSenderAllowed: () => false, - expectedDecision: "block" as const, + dmPolicy: "disabled", + expectedDecision: "block", expectedReactionAllowed: false, - }, - { + }), + createParityCase({ name: "dmPolicy=allowlist unauthorized", - isGroup: false, - dmPolicy: "allowlist" as const, - groupPolicy: "allowlist" as const, + dmPolicy: "allowlist", allowFrom: ["owner"], - groupAllowFrom: [] as string[], - storeAllowFrom: [] as string[], isSenderAllowed: () => false, - expectedDecision: "block" as const, + expectedDecision: "block", expectedReactionAllowed: false, - }, - { + }), + createParityCase({ name: "dmPolicy=allowlist authorized", - isGroup: false, - dmPolicy: "allowlist" as const, - groupPolicy: "allowlist" as const, + dmPolicy: "allowlist", allowFrom: ["owner"], - groupAllowFrom: [] as string[], - storeAllowFrom: [] as string[], isSenderAllowed: () => true, - expectedDecision: "allow" as const, + expectedDecision: "allow", expectedReactionAllowed: true, - }, - { + }), + createParityCase({ name: "dmPolicy=pairing unauthorized", - isGroup: false, - dmPolicy: "pairing" as const, - groupPolicy: "allowlist" as const, - allowFrom: [] as string[], - groupAllowFrom: [] as string[], - storeAllowFrom: [] as string[], + dmPolicy: "pairing", isSenderAllowed: () => false, - expectedDecision: "pairing" as const, + expectedDecision: "pairing", expectedReactionAllowed: false, - }, - { + }), + createParityCase({ name: "groupPolicy=allowlist rejects DM-paired sender not in explicit group list", isGroup: true, - dmPolicy: "pairing" as const, - groupPolicy: "allowlist" as const, - allowFrom: ["owner"] as string[], - groupAllowFrom: ["group-owner"] as string[], - storeAllowFrom: ["paired-user"] as string[], + dmPolicy: "pairing", + allowFrom: ["owner"], + groupAllowFrom: ["group-owner"], + storeAllowFrom: ["paired-user"], isSenderAllowed: (allowFrom: string[]) => allowFrom.includes("paired-user"), - expectedDecision: "block" as const, + expectedDecision: "block", expectedReactionAllowed: false, - }, + }), ]; for (const channel of channels) {