diff --git a/src/channels/account-summary.ts b/src/channels/account-summary.ts index f4ff677a1c0..3e6db86c615 100644 --- a/src/channels/account-summary.ts +++ b/src/channels/account-summary.ts @@ -34,3 +34,38 @@ export function formatChannelAllowFrom(params: { } return params.allowFrom.map((entry) => String(entry).trim()).filter(Boolean); } + +function asRecord(value: unknown): Record | undefined { + if (!value || typeof value !== "object") { + return undefined; + } + return value as Record; +} + +export function resolveChannelAccountEnabled(params: { + plugin: ChannelPlugin; + account: unknown; + cfg: OpenClawConfig; +}): boolean { + if (params.plugin.config.isEnabled) { + return params.plugin.config.isEnabled(params.account, params.cfg); + } + const enabled = asRecord(params.account)?.enabled; + return enabled !== false; +} + +export async function resolveChannelAccountConfigured(params: { + plugin: ChannelPlugin; + account: unknown; + cfg: OpenClawConfig; + readAccountConfiguredField?: boolean; +}): Promise { + if (params.plugin.config.isConfigured) { + return await params.plugin.config.isConfigured(params.account, params.cfg); + } + if (params.readAccountConfiguredField) { + const configured = asRecord(params.account)?.configured; + return configured !== false; + } + return true; +} diff --git a/src/channels/plugins/actions/discord/handle-action.ts b/src/channels/plugins/actions/discord/handle-action.ts index 4c868c71efb..c0f3dc01ae2 100644 --- a/src/channels/plugins/actions/discord/handle-action.ts +++ b/src/channels/plugins/actions/discord/handle-action.ts @@ -4,6 +4,7 @@ import { readStringArrayParam, readStringParam, } from "../../../../agents/tools/common.js"; +import { readDiscordParentIdParam } from "../../../../agents/tools/discord-actions-shared.js"; import { handleDiscordAction } from "../../../../agents/tools/discord-actions.js"; import { resolveDiscordChannelId } from "../../../../discord/targets.js"; import type { ChannelMessageActionContext } from "../../types.js"; @@ -11,16 +12,6 @@ import { tryHandleDiscordMessageActionGuildAdmin } from "./handle-action.guild-a const providerId = "discord"; -function readParentIdParam(params: Record): string | null | undefined { - if (params.clearParent === true) { - return null; - } - if (params.parentId === null) { - return null; - } - return readStringParam(params, "parentId"); -} - export async function handleDiscordMessageAction( ctx: Pick< ChannelMessageActionContext, @@ -285,7 +276,7 @@ export async function handleDiscordMessageAction( const adminResult = await tryHandleDiscordMessageActionGuildAdmin({ ctx, resolveChannelId, - readParentIdParam, + readParentIdParam: readDiscordParentIdParam, }); if (adminResult !== undefined) { return adminResult; diff --git a/src/channels/session-envelope.ts b/src/channels/session-envelope.ts new file mode 100644 index 00000000000..e438028daec --- /dev/null +++ b/src/channels/session-envelope.ts @@ -0,0 +1,21 @@ +import { resolveEnvelopeFormatOptions } from "../auto-reply/envelope.js"; +import type { OpenClawConfig } from "../config/config.js"; +import { readSessionUpdatedAt, resolveStorePath } from "../config/sessions.js"; + +export function resolveInboundSessionEnvelopeContext(params: { + cfg: OpenClawConfig; + agentId: string; + sessionKey: string; +}) { + const storePath = resolveStorePath(params.cfg.session?.store, { + agentId: params.agentId, + }); + return { + storePath, + envelopeOptions: resolveEnvelopeFormatOptions(params.cfg), + previousTimestamp: readSessionUpdatedAt({ + storePath, + sessionKey: params.sessionKey, + }), + }; +} diff --git a/src/channels/session-meta.ts b/src/channels/session-meta.ts new file mode 100644 index 00000000000..29b2d77e046 --- /dev/null +++ b/src/channels/session-meta.ts @@ -0,0 +1,24 @@ +import type { MsgContext } from "../auto-reply/templating.js"; +import type { OpenClawConfig } from "../config/config.js"; +import { recordSessionMetaFromInbound, resolveStorePath } from "../config/sessions.js"; + +export async function recordInboundSessionMetaSafe(params: { + cfg: OpenClawConfig; + agentId: string; + sessionKey: string; + ctx: MsgContext; + onError?: (error: unknown) => void; +}): Promise { + const storePath = resolveStorePath(params.cfg.session?.store, { + agentId: params.agentId, + }); + try { + await recordSessionMetaFromInbound({ + storePath, + sessionKey: params.sessionKey, + ctx: params.ctx, + }); + } catch (err) { + params.onError?.(err); + } +} diff --git a/src/channels/targets.ts b/src/channels/targets.ts index 49ec74f3f6f..f9a0b015927 100644 --- a/src/channels/targets.ts +++ b/src/channels/targets.ts @@ -84,6 +84,52 @@ export function parseTargetPrefixes(params: { return undefined; } +export function parseAtUserTarget(params: { + raw: string; + pattern: RegExp; + errorMessage: string; +}): MessagingTarget | undefined { + if (!params.raw.startsWith("@")) { + return undefined; + } + const candidate = params.raw.slice(1).trim(); + const id = ensureTargetId({ + candidate, + pattern: params.pattern, + errorMessage: params.errorMessage, + }); + return buildMessagingTarget("user", id, params.raw); +} + +export function parseMentionPrefixOrAtUserTarget(params: { + raw: string; + mentionPattern: RegExp; + prefixes: Array<{ prefix: string; kind: MessagingTargetKind }>; + atUserPattern: RegExp; + atUserErrorMessage: string; +}): MessagingTarget | undefined { + const mentionTarget = parseTargetMention({ + raw: params.raw, + mentionPattern: params.mentionPattern, + kind: "user", + }); + if (mentionTarget) { + return mentionTarget; + } + const prefixedTarget = parseTargetPrefixes({ + raw: params.raw, + prefixes: params.prefixes, + }); + if (prefixedTarget) { + return prefixedTarget; + } + return parseAtUserTarget({ + raw: params.raw, + pattern: params.atUserPattern, + errorMessage: params.atUserErrorMessage, + }); +} + export function requireTargetKind(params: { platform: string; target: MessagingTarget | undefined; diff --git a/src/discord/monitor/dm-command-decision.test.ts b/src/discord/monitor/dm-command-decision.test.ts index 1847ec2e56e..2f87d8bb30b 100644 --- a/src/discord/monitor/dm-command-decision.test.ts +++ b/src/discord/monitor/dm-command-decision.test.ts @@ -12,16 +12,44 @@ function buildDmAccess(overrides: Partial): DiscordDmCom }; } +const TEST_ACCOUNT_ID = "default"; +const TEST_SENDER = { id: "123", tag: "alice#0001", name: "alice" }; + +function createDmDecisionHarness(params?: { pairingCreated?: boolean }) { + const onPairingCreated = vi.fn(async () => {}); + const onUnauthorized = vi.fn(async () => {}); + const upsertPairingRequest = vi.fn(async () => ({ + code: "PAIR-1", + created: params?.pairingCreated ?? true, + })); + return { onPairingCreated, onUnauthorized, upsertPairingRequest }; +} + +async function runPairingDecision(params?: { pairingCreated?: boolean }) { + const harness = createDmDecisionHarness({ pairingCreated: params?.pairingCreated }); + const allowed = await handleDiscordDmCommandDecision({ + dmAccess: buildDmAccess({ + decision: "pairing", + commandAuthorized: false, + allowMatch: { allowed: false }, + }), + accountId: TEST_ACCOUNT_ID, + sender: TEST_SENDER, + onPairingCreated: harness.onPairingCreated, + onUnauthorized: harness.onUnauthorized, + upsertPairingRequest: harness.upsertPairingRequest, + }); + return { allowed, ...harness }; +} + describe("handleDiscordDmCommandDecision", () => { it("returns true for allowed DM access", async () => { - const onPairingCreated = vi.fn(async () => {}); - const onUnauthorized = vi.fn(async () => {}); - const upsertPairingRequest = vi.fn(async () => ({ code: "PAIR-1", created: true })); + const { onPairingCreated, onUnauthorized, upsertPairingRequest } = createDmDecisionHarness(); const allowed = await handleDiscordDmCommandDecision({ dmAccess: buildDmAccess({ decision: "allow" }), - accountId: "default", - sender: { id: "123", tag: "alice#0001", name: "alice" }, + accountId: TEST_ACCOUNT_ID, + sender: TEST_SENDER, onPairingCreated, onUnauthorized, upsertPairingRequest, @@ -34,31 +62,17 @@ describe("handleDiscordDmCommandDecision", () => { }); it("creates pairing reply for new pairing requests", async () => { - const onPairingCreated = vi.fn(async () => {}); - const onUnauthorized = vi.fn(async () => {}); - const upsertPairingRequest = vi.fn(async () => ({ code: "PAIR-1", created: true })); - - const allowed = await handleDiscordDmCommandDecision({ - dmAccess: buildDmAccess({ - decision: "pairing", - commandAuthorized: false, - allowMatch: { allowed: false }, - }), - accountId: "default", - sender: { id: "123", tag: "alice#0001", name: "alice" }, - onPairingCreated, - onUnauthorized, - upsertPairingRequest, - }); + const { allowed, onPairingCreated, onUnauthorized, upsertPairingRequest } = + await runPairingDecision(); expect(allowed).toBe(false); expect(upsertPairingRequest).toHaveBeenCalledWith({ channel: "discord", id: "123", - accountId: "default", + accountId: TEST_ACCOUNT_ID, meta: { - tag: "alice#0001", - name: "alice", + tag: TEST_SENDER.tag, + name: TEST_SENDER.name, }, }); expect(onPairingCreated).toHaveBeenCalledWith("PAIR-1"); @@ -66,21 +80,8 @@ describe("handleDiscordDmCommandDecision", () => { }); it("skips pairing reply when pairing request already exists", async () => { - const onPairingCreated = vi.fn(async () => {}); - const onUnauthorized = vi.fn(async () => {}); - const upsertPairingRequest = vi.fn(async () => ({ code: "PAIR-1", created: false })); - - const allowed = await handleDiscordDmCommandDecision({ - dmAccess: buildDmAccess({ - decision: "pairing", - commandAuthorized: false, - allowMatch: { allowed: false }, - }), - accountId: "default", - sender: { id: "123", tag: "alice#0001", name: "alice" }, - onPairingCreated, - onUnauthorized, - upsertPairingRequest, + const { allowed, onPairingCreated, onUnauthorized } = await runPairingDecision({ + pairingCreated: false, }); expect(allowed).toBe(false); @@ -89,9 +90,7 @@ describe("handleDiscordDmCommandDecision", () => { }); it("runs unauthorized handler for blocked DM access", async () => { - const onPairingCreated = vi.fn(async () => {}); - const onUnauthorized = vi.fn(async () => {}); - const upsertPairingRequest = vi.fn(async () => ({ code: "PAIR-1", created: true })); + const { onPairingCreated, onUnauthorized, upsertPairingRequest } = createDmDecisionHarness(); const allowed = await handleDiscordDmCommandDecision({ dmAccess: buildDmAccess({ @@ -99,8 +98,8 @@ describe("handleDiscordDmCommandDecision", () => { commandAuthorized: false, allowMatch: { allowed: false }, }), - accountId: "default", - sender: { id: "123", tag: "alice#0001", name: "alice" }, + accountId: TEST_ACCOUNT_ID, + sender: TEST_SENDER, onPairingCreated, onUnauthorized, upsertPairingRequest, diff --git a/src/discord/monitor/listeners.ts b/src/discord/monitor/listeners.ts index 44e280ea962..0afd31c9258 100644 --- a/src/discord/monitor/listeners.ts +++ b/src/discord/monitor/listeners.ts @@ -374,7 +374,7 @@ async function handleDiscordReactionEvent(params: { channelType === ChannelType.PublicThread || channelType === ChannelType.PrivateThread || channelType === ChannelType.AnnouncementThread; - const ingressAccess = await authorizeDiscordReactionIngress({ + const reactionIngressBase: Omit = { accountId: params.accountId, user, isDirectMessage, @@ -391,7 +391,8 @@ async function handleDiscordReactionEvent(params: { groupPolicy: params.groupPolicy, allowNameMatching: params.allowNameMatching, guildInfo, - }); + }; + const ingressAccess = await authorizeDiscordReactionIngress(reactionIngressBase); if (!ingressAccess.allowed) { logVerbose(`discord reaction blocked sender=${user.id} (reason=${ingressAccess.reason})`); return; @@ -486,22 +487,7 @@ async function handleDiscordReactionEvent(params: { channelConfig: ReturnType, ) => await authorizeDiscordReactionIngress({ - accountId: params.accountId, - user, - isDirectMessage, - isGroupDm, - isGuildMessage, - channelId: data.channel_id, - channelName, - channelSlug, - dmEnabled: params.dmEnabled, - groupDmEnabled: params.groupDmEnabled, - groupDmChannels: params.groupDmChannels, - dmPolicy: params.dmPolicy, - allowFrom: params.allowFrom, - groupPolicy: params.groupPolicy, - allowNameMatching: params.allowNameMatching, - guildInfo, + ...reactionIngressBase, channelConfig, }); const authorizeThreadChannelAccess = async (channelInfo: { parentId?: string } | null) => { diff --git a/src/discord/monitor/message-handler.inbound-contract.test.ts b/src/discord/monitor/message-handler.inbound-contract.test.ts index 378f99c5210..b6a3c8f85f1 100644 --- a/src/discord/monitor/message-handler.inbound-contract.test.ts +++ b/src/discord/monitor/message-handler.inbound-contract.test.ts @@ -3,7 +3,10 @@ import { inboundCtxCapture as capture } from "../../../test/helpers/inbound-cont import { expectInboundContextContract } from "../../../test/helpers/inbound-contract.js"; import type { DiscordMessagePreflightContext } from "./message-handler.preflight.js"; import { processDiscordMessage } from "./message-handler.process.js"; -import { createBaseDiscordMessageContext } from "./message-handler.test-harness.js"; +import { + createBaseDiscordMessageContext, + createDiscordDirectMessageContextOverrides, +} from "./message-handler.test-harness.js"; describe("discord processDiscordMessage inbound contract", () => { it("passes a finalized MsgContext to dispatchInboundMessage", async () => { @@ -11,26 +14,7 @@ describe("discord processDiscordMessage inbound contract", () => { const messageCtx = await createBaseDiscordMessageContext({ cfg: { messages: {} }, ackReactionScope: "direct", - data: { guild: null }, - channelInfo: null, - channelName: undefined, - isGuildMessage: false, - isDirectMessage: true, - isGroupDm: false, - shouldRequireMention: false, - canDetectMention: false, - effectiveWasMentioned: false, - displayChannelSlug: "", - guildInfo: null, - guildSlug: "", - baseSessionKey: "agent:main:discord:direct:u1", - route: { - agentId: "main", - channel: "discord", - accountId: "default", - sessionKey: "agent:main:discord:direct:u1", - mainSessionKey: "agent:main:main", - }, + ...createDiscordDirectMessageContextOverrides(), }); await processDiscordMessage(messageCtx); diff --git a/src/discord/monitor/message-handler.process.test.ts b/src/discord/monitor/message-handler.process.test.ts index bce0325042a..1c0e8f029f0 100644 --- a/src/discord/monitor/message-handler.process.test.ts +++ b/src/discord/monitor/message-handler.process.test.ts @@ -1,6 +1,9 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { DEFAULT_EMOJIS } from "../../channels/status-reactions.js"; -import { createBaseDiscordMessageContext } from "./message-handler.test-harness.js"; +import { + createBaseDiscordMessageContext, + createDiscordDirectMessageContextOverrides, +} from "./message-handler.test-harness.js"; import { __testing as threadBindingTesting, createThreadBindingManager, @@ -295,18 +298,7 @@ describe("processDiscordMessage ack reactions", () => { describe("processDiscordMessage session routing", () => { it("stores DM lastRoute with user target for direct-session continuity", async () => { const ctx = await createBaseContext({ - data: { guild: null }, - channelInfo: null, - channelName: undefined, - isGuildMessage: false, - isDirectMessage: true, - isGroupDm: false, - shouldRequireMention: false, - canDetectMention: false, - effectiveWasMentioned: false, - displayChannelSlug: "", - guildInfo: null, - guildSlug: "", + ...createDiscordDirectMessageContextOverrides(), message: { id: "m1", channelId: "dm1", @@ -314,14 +306,6 @@ describe("processDiscordMessage session routing", () => { attachments: [], }, messageChannelId: "dm1", - baseSessionKey: "agent:main:discord:direct:u1", - route: { - agentId: "main", - channel: "discord", - accountId: "default", - sessionKey: "agent:main:discord:direct:u1", - mainSessionKey: "agent:main:main", - }, }); // oxlint-disable-next-line typescript/no-explicit-any diff --git a/src/discord/monitor/message-handler.test-harness.ts b/src/discord/monitor/message-handler.test-harness.ts index 1913fa8cf81..e62e2fc82da 100644 --- a/src/discord/monitor/message-handler.test-harness.ts +++ b/src/discord/monitor/message-handler.test-harness.ts @@ -72,3 +72,28 @@ export async function createBaseDiscordMessageContext( ...overrides, } as unknown as DiscordMessagePreflightContext; } + +export function createDiscordDirectMessageContextOverrides(): Record { + return { + data: { guild: null }, + channelInfo: null, + channelName: undefined, + isGuildMessage: false, + isDirectMessage: true, + isGroupDm: false, + shouldRequireMention: false, + canDetectMention: false, + effectiveWasMentioned: false, + displayChannelSlug: "", + guildInfo: null, + guildSlug: "", + baseSessionKey: "agent:main:discord:direct:u1", + route: { + agentId: "main", + channel: "discord", + accountId: "default", + sessionKey: "agent:main:discord:direct:u1", + mainSessionKey: "agent:main:main", + }, + }; +} diff --git a/src/discord/monitor/message-utils.test.ts b/src/discord/monitor/message-utils.test.ts index 152f76c8e3e..72ca2aea94d 100644 --- a/src/discord/monitor/message-utils.test.ts +++ b/src/discord/monitor/message-utils.test.ts @@ -30,6 +30,68 @@ function asMessage(payload: Record): Message { return payload as unknown as Message; } +function expectSinglePngDownload(params: { + result: unknown; + expectedUrl: string; + filePathHint: string; + expectedPath: string; + placeholder: "" | ""; +}) { + expect(fetchRemoteMedia).toHaveBeenCalledTimes(1); + expect(fetchRemoteMedia).toHaveBeenCalledWith({ + url: params.expectedUrl, + filePathHint: params.filePathHint, + maxBytes: 512, + fetchImpl: undefined, + ssrfPolicy: expect.objectContaining({ allowRfc2544BenchmarkRange: true }), + }); + expect(saveMediaBuffer).toHaveBeenCalledTimes(1); + expect(saveMediaBuffer).toHaveBeenCalledWith(expect.any(Buffer), "image/png", "inbound", 512); + expect(params.result).toEqual([ + { + path: params.expectedPath, + contentType: "image/png", + placeholder: params.placeholder, + }, + ]); +} + +function expectAttachmentImageFallback(params: { result: unknown; attachment: { url: string } }) { + expect(saveMediaBuffer).not.toHaveBeenCalled(); + expect(params.result).toEqual([ + { + path: params.attachment.url, + contentType: "image/png", + placeholder: "", + }, + ]); +} + +function asForwardedSnapshotMessage(params: { + content: string; + embeds: Array<{ title?: string; description?: string }>; +}) { + return asMessage({ + content: "", + rawData: { + message_snapshots: [ + { + message: { + content: params.content, + embeds: params.embeds, + attachments: [], + author: { + id: "u2", + username: "Bob", + discriminator: "0", + }, + }, + }, + ], + }, + }); +} + describe("resolveDiscordMessageChannelId", () => { it.each([ { @@ -157,14 +219,7 @@ describe("resolveForwardedMediaList", () => { 512, ); - expect(saveMediaBuffer).not.toHaveBeenCalled(); - expect(result).toEqual([ - { - path: attachment.url, - contentType: "image/png", - placeholder: "", - }, - ]); + expectAttachmentImageFallback({ result, attachment }); }); it("downloads forwarded stickers", async () => { @@ -191,23 +246,13 @@ describe("resolveForwardedMediaList", () => { 512, ); - expect(fetchRemoteMedia).toHaveBeenCalledTimes(1); - expect(fetchRemoteMedia).toHaveBeenCalledWith({ - url: "https://media.discordapp.net/stickers/sticker-1.png", + expectSinglePngDownload({ + result, + expectedUrl: "https://media.discordapp.net/stickers/sticker-1.png", filePathHint: "wave.png", - maxBytes: 512, - fetchImpl: undefined, - ssrfPolicy: expect.objectContaining({ allowRfc2544BenchmarkRange: true }), + expectedPath: "/tmp/sticker.png", + placeholder: "", }); - expect(saveMediaBuffer).toHaveBeenCalledTimes(1); - expect(saveMediaBuffer).toHaveBeenCalledWith(expect.any(Buffer), "image/png", "inbound", 512); - expect(result).toEqual([ - { - path: "/tmp/sticker.png", - contentType: "image/png", - placeholder: "", - }, - ]); }); it("returns empty when no snapshots are present", async () => { @@ -260,23 +305,13 @@ describe("resolveMediaList", () => { 512, ); - expect(fetchRemoteMedia).toHaveBeenCalledTimes(1); - expect(fetchRemoteMedia).toHaveBeenCalledWith({ - url: "https://media.discordapp.net/stickers/sticker-2.png", + expectSinglePngDownload({ + result, + expectedUrl: "https://media.discordapp.net/stickers/sticker-2.png", filePathHint: "hello.png", - maxBytes: 512, - fetchImpl: undefined, - ssrfPolicy: expect.objectContaining({ allowRfc2544BenchmarkRange: true }), + expectedPath: "/tmp/sticker-2.png", + placeholder: "", }); - expect(saveMediaBuffer).toHaveBeenCalledTimes(1); - expect(saveMediaBuffer).toHaveBeenCalledWith(expect.any(Buffer), "image/png", "inbound", 512); - expect(result).toEqual([ - { - path: "/tmp/sticker-2.png", - contentType: "image/png", - placeholder: "", - }, - ]); }); it("forwards fetchImpl to sticker downloads", async () => { @@ -324,14 +359,7 @@ describe("resolveMediaList", () => { 512, ); - expect(saveMediaBuffer).not.toHaveBeenCalled(); - expect(result).toEqual([ - { - path: attachment.url, - contentType: "image/png", - placeholder: "", - }, - ]); + expectAttachmentImageFallback({ result, attachment }); }); it("falls back to URL when saveMediaBuffer fails", async () => { @@ -471,24 +499,9 @@ describe("Discord media SSRF policy", () => { describe("resolveDiscordMessageText", () => { it("includes forwarded message snapshots in body text", () => { const text = resolveDiscordMessageText( - asMessage({ - content: "", - rawData: { - message_snapshots: [ - { - message: { - content: "forwarded hello", - embeds: [], - attachments: [], - author: { - id: "u2", - username: "Bob", - discriminator: "0", - }, - }, - }, - ], - }, + asForwardedSnapshotMessage({ + content: "forwarded hello", + embeds: [], }), { includeForwarded: true }, ); @@ -560,24 +573,9 @@ describe("resolveDiscordMessageText", () => { it("joins forwarded snapshot embed title and description when content is empty", () => { const text = resolveDiscordMessageText( - asMessage({ + asForwardedSnapshotMessage({ content: "", - rawData: { - message_snapshots: [ - { - message: { - content: "", - embeds: [{ title: "Forwarded title", description: "Forwarded details" }], - attachments: [], - author: { - id: "u2", - username: "Bob", - discriminator: "0", - }, - }, - }, - ], - }, + embeds: [{ title: "Forwarded title", description: "Forwarded details" }], }), { includeForwarded: true }, ); diff --git a/src/discord/monitor/provider.lifecycle.test.ts b/src/discord/monitor/provider.lifecycle.test.ts index da4a06d5b9c..22e8be6353f 100644 --- a/src/discord/monitor/provider.lifecycle.test.ts +++ b/src/discord/monitor/provider.lifecycle.test.ts @@ -122,6 +122,27 @@ describe("runDiscordGatewayLifecycle", () => { expect(params.releaseEarlyGatewayErrorGuard).toHaveBeenCalledTimes(1); } + function createGatewayHarness(params?: { + state?: { + sessionId?: string | null; + resumeGatewayUrl?: string | null; + sequence?: number | null; + }; + sequence?: number | null; + }) { + const emitter = new EventEmitter(); + const gateway = { + isConnected: false, + options: {}, + disconnect: vi.fn(), + connect: vi.fn(), + ...(params?.state ? { state: params.state } : {}), + ...(params?.sequence !== undefined ? { sequence: params.sequence } : {}), + emitter, + }; + return { emitter, gateway }; + } + it("cleans up thread bindings when exec approvals startup fails", async () => { const { runDiscordGatewayLifecycle } = await import("./provider.lifecycle.js"); const { lifecycleParams, start, stop, threadStop, releaseEarlyGatewayErrorGuard } = @@ -229,20 +250,14 @@ describe("runDiscordGatewayLifecycle", () => { vi.useFakeTimers(); try { const { runDiscordGatewayLifecycle } = await import("./provider.lifecycle.js"); - const emitter = new EventEmitter(); - const gateway = { - isConnected: false, - options: {}, - disconnect: vi.fn(), - connect: vi.fn(), + const { emitter, gateway } = createGatewayHarness({ state: { sessionId: "session-1", resumeGatewayUrl: "wss://gateway.discord.gg", sequence: 123, }, sequence: 123, - emitter, - }; + }); getDiscordGatewayEmitterMock.mockReturnValueOnce(emitter); waitForDiscordGatewayStopMock.mockImplementationOnce(async () => { emitter.emit("debug", "WebSocket connection opened"); @@ -260,9 +275,10 @@ describe("runDiscordGatewayLifecycle", () => { expect(gateway.connect).toHaveBeenNthCalledWith(1, true); expect(gateway.connect).toHaveBeenNthCalledWith(2, true); expect(gateway.connect).toHaveBeenNthCalledWith(3, false); - expect(gateway.state.sessionId).toBeNull(); - expect(gateway.state.resumeGatewayUrl).toBeNull(); - expect(gateway.state.sequence).toBeNull(); + expect(gateway.state).toBeDefined(); + expect(gateway.state?.sessionId).toBeNull(); + expect(gateway.state?.resumeGatewayUrl).toBeNull(); + expect(gateway.state?.sequence).toBeNull(); expect(gateway.sequence).toBeNull(); } finally { vi.useRealTimers(); @@ -273,20 +289,14 @@ describe("runDiscordGatewayLifecycle", () => { vi.useFakeTimers(); try { const { runDiscordGatewayLifecycle } = await import("./provider.lifecycle.js"); - const emitter = new EventEmitter(); - const gateway = { - isConnected: false, - options: {}, - disconnect: vi.fn(), - connect: vi.fn(), + const { emitter, gateway } = createGatewayHarness({ state: { sessionId: "session-2", resumeGatewayUrl: "wss://gateway.discord.gg", sequence: 456, }, sequence: 456, - emitter, - }; + }); getDiscordGatewayEmitterMock.mockReturnValueOnce(emitter); waitForDiscordGatewayStopMock.mockImplementationOnce(async () => { emitter.emit("debug", "WebSocket connection opened"); @@ -324,14 +334,7 @@ describe("runDiscordGatewayLifecycle", () => { vi.useFakeTimers(); try { const { runDiscordGatewayLifecycle } = await import("./provider.lifecycle.js"); - const emitter = new EventEmitter(); - const gateway = { - isConnected: false, - options: {}, - disconnect: vi.fn(), - connect: vi.fn(), - emitter, - }; + const { emitter, gateway } = createGatewayHarness(); getDiscordGatewayEmitterMock.mockReturnValueOnce(emitter); waitForDiscordGatewayStopMock.mockImplementationOnce( (waitParams: WaitForDiscordGatewayStopParams) => @@ -356,14 +359,7 @@ describe("runDiscordGatewayLifecycle", () => { vi.useFakeTimers(); try { const { runDiscordGatewayLifecycle } = await import("./provider.lifecycle.js"); - const emitter = new EventEmitter(); - const gateway = { - isConnected: false, - options: {}, - disconnect: vi.fn(), - connect: vi.fn(), - emitter, - }; + const { emitter, gateway } = createGatewayHarness(); getDiscordGatewayEmitterMock.mockReturnValueOnce(emitter); let resolveWait: (() => void) | undefined; waitForDiscordGatewayStopMock.mockImplementationOnce( diff --git a/src/discord/monitor/provider.ts b/src/discord/monitor/provider.ts index 016a18b77ba..b3420ca8e9f 100644 --- a/src/discord/monitor/provider.ts +++ b/src/discord/monitor/provider.ts @@ -14,6 +14,11 @@ import { resolveTextChunkLimit } from "../../auto-reply/chunk.js"; import { listNativeCommandSpecsForConfig } from "../../auto-reply/commands-registry.js"; import type { HistoryEntry } from "../../auto-reply/reply/history.js"; import { listSkillCommandsForAgents } from "../../auto-reply/skill-commands.js"; +import { + resolveThreadBindingIdleTimeoutMs, + resolveThreadBindingMaxAgeMs, + resolveThreadBindingsEnabled, +} from "../../channels/thread-bindings-policy.js"; import { isNativeCommandsExplicitlyDisabled, resolveNativeCommandsEnabled, @@ -110,59 +115,6 @@ function summarizeGuilds(entries?: Record) { return `${sample.join(", ")}${suffix}`; } -const DEFAULT_THREAD_BINDING_IDLE_HOURS = 24; -const DEFAULT_THREAD_BINDING_MAX_AGE_HOURS = 0; - -function normalizeThreadBindingHours(raw: unknown): number | undefined { - if (typeof raw !== "number" || !Number.isFinite(raw)) { - return undefined; - } - if (raw < 0) { - return undefined; - } - return raw; -} - -function resolveThreadBindingIdleTimeoutMs(params: { - channelIdleHoursRaw: unknown; - sessionIdleHoursRaw: unknown; -}): number { - const idleHours = - normalizeThreadBindingHours(params.channelIdleHoursRaw) ?? - normalizeThreadBindingHours(params.sessionIdleHoursRaw) ?? - DEFAULT_THREAD_BINDING_IDLE_HOURS; - return Math.floor(idleHours * 60 * 60 * 1000); -} - -function resolveThreadBindingMaxAgeMs(params: { - channelMaxAgeHoursRaw: unknown; - sessionMaxAgeHoursRaw: unknown; -}): number { - const maxAgeHours = - normalizeThreadBindingHours(params.channelMaxAgeHoursRaw) ?? - normalizeThreadBindingHours(params.sessionMaxAgeHoursRaw) ?? - DEFAULT_THREAD_BINDING_MAX_AGE_HOURS; - return Math.floor(maxAgeHours * 60 * 60 * 1000); -} - -function normalizeThreadBindingsEnabled(raw: unknown): boolean | undefined { - if (typeof raw !== "boolean") { - return undefined; - } - return raw; -} - -function resolveThreadBindingsEnabled(params: { - channelEnabledRaw: unknown; - sessionEnabledRaw: unknown; -}): boolean { - return ( - normalizeThreadBindingsEnabled(params.channelEnabledRaw) ?? - normalizeThreadBindingsEnabled(params.sessionEnabledRaw) ?? - true - ); -} - function formatThreadBindingDurationForConfigLabel(durationMs: number): string { const label = formatThreadBindingDurationLabel(durationMs); return label === "disabled" ? "off" : label; @@ -612,43 +564,26 @@ export async function monitorDiscordProvider(opts: MonitorDiscordOpts = {}) { client.listeners, new DiscordMessageListener(messageHandler, logger, trackInboundEvent), ); + const reactionListenerOptions = { + cfg, + accountId: account.accountId, + runtime, + botUserId, + dmEnabled, + groupDmEnabled, + groupDmChannels: groupDmChannels ?? [], + dmPolicy, + allowFrom: allowFrom ?? [], + groupPolicy, + allowNameMatching: isDangerousNameMatchingEnabled(discordCfg), + guildEntries, + logger, + onEvent: trackInboundEvent, + }; + registerDiscordListener(client.listeners, new DiscordReactionListener(reactionListenerOptions)); registerDiscordListener( client.listeners, - new DiscordReactionListener({ - cfg, - accountId: account.accountId, - runtime, - botUserId, - dmEnabled, - groupDmEnabled, - groupDmChannels: groupDmChannels ?? [], - dmPolicy, - allowFrom: allowFrom ?? [], - groupPolicy, - allowNameMatching: isDangerousNameMatchingEnabled(discordCfg), - guildEntries, - logger, - onEvent: trackInboundEvent, - }), - ); - registerDiscordListener( - client.listeners, - new DiscordReactionRemoveListener({ - cfg, - accountId: account.accountId, - runtime, - botUserId, - dmEnabled, - groupDmEnabled, - groupDmChannels: groupDmChannels ?? [], - dmPolicy, - allowFrom: allowFrom ?? [], - groupPolicy, - allowNameMatching: isDangerousNameMatchingEnabled(discordCfg), - guildEntries, - logger, - onEvent: trackInboundEvent, - }), + new DiscordReactionRemoveListener(reactionListenerOptions), ); if (discordCfg.intents?.presence) { diff --git a/src/discord/resolve-channels.test.ts b/src/discord/resolve-channels.test.ts index f0445a80086..39b46a53f33 100644 --- a/src/discord/resolve-channels.test.ts +++ b/src/discord/resolve-channels.test.ts @@ -4,6 +4,28 @@ import { resolveDiscordChannelAllowlist } from "./resolve-channels.js"; import { jsonResponse, urlToString } from "./test-http-helpers.js"; describe("resolveDiscordChannelAllowlist", () => { + async function resolveWithChannelLookup(params: { + guilds: Array<{ id: string; name: string }>; + channel: { id: string; name: string; guild_id: string; type: number }; + entry: string; + }) { + const fetcher = withFetchPreconnect(async (input: RequestInfo | URL) => { + const url = urlToString(input); + if (url.endsWith("/users/@me/guilds")) { + return jsonResponse(params.guilds); + } + if (url.endsWith(`/channels/${params.channel.id}`)) { + return jsonResponse(params.channel); + } + return new Response("not found", { status: 404 }); + }); + return resolveDiscordChannelAllowlist({ + token: "test", + entries: [params.entry], + fetcher, + }); + } + it("resolves guild/channel by name", async () => { const fetcher = withFetchPreconnect(async (input: RequestInfo | URL) => { const url = urlToString(input); @@ -54,21 +76,10 @@ describe("resolveDiscordChannelAllowlist", () => { }); it("resolves guildId/channelId entries via channel lookup", async () => { - const fetcher = withFetchPreconnect(async (input: RequestInfo | URL) => { - const url = urlToString(input); - if (url.endsWith("/users/@me/guilds")) { - return jsonResponse([{ id: "111", name: "Guild One" }]); - } - if (url.endsWith("/channels/222")) { - return jsonResponse({ id: "222", name: "general", guild_id: "111", type: 0 }); - } - return new Response("not found", { status: 404 }); - }); - - const res = await resolveDiscordChannelAllowlist({ - token: "test", - entries: ["111/222"], - fetcher, + const res = await resolveWithChannelLookup({ + guilds: [{ id: "111", name: "Guild One" }], + channel: { id: "222", name: "general", guild_id: "111", type: 0 }, + entry: "111/222", }); expect(res[0]).toMatchObject({ @@ -82,24 +93,13 @@ describe("resolveDiscordChannelAllowlist", () => { }); it("reports unresolved when channel id belongs to a different guild", async () => { - const fetcher = withFetchPreconnect(async (input: RequestInfo | URL) => { - const url = urlToString(input); - if (url.endsWith("/users/@me/guilds")) { - return jsonResponse([ - { id: "111", name: "Guild One" }, - { id: "333", name: "Guild Two" }, - ]); - } - if (url.endsWith("/channels/222")) { - return jsonResponse({ id: "222", name: "general", guild_id: "333", type: 0 }); - } - return new Response("not found", { status: 404 }); - }); - - const res = await resolveDiscordChannelAllowlist({ - token: "test", - entries: ["111/222"], - fetcher, + const res = await resolveWithChannelLookup({ + guilds: [ + { id: "111", name: "Guild One" }, + { id: "333", name: "Guild Two" }, + ], + channel: { id: "222", name: "general", guild_id: "333", type: 0 }, + entry: "111/222", }); expect(res[0]).toMatchObject({ diff --git a/src/discord/targets.ts b/src/discord/targets.ts index 6f8fd85039f..9ddbae388eb 100644 --- a/src/discord/targets.ts +++ b/src/discord/targets.ts @@ -1,9 +1,7 @@ import type { DirectoryConfigParams } from "../channels/plugins/directory-config.js"; import { buildMessagingTarget, - ensureTargetId, - parseTargetMention, - parseTargetPrefixes, + parseMentionPrefixOrAtUserTarget, requireTargetKind, type MessagingTarget, type MessagingTargetKind, @@ -25,33 +23,19 @@ export function parseDiscordTarget( if (!trimmed) { return undefined; } - const mentionTarget = parseTargetMention({ + const userTarget = parseMentionPrefixOrAtUserTarget({ raw: trimmed, mentionPattern: /^<@!?(\d+)>$/, - kind: "user", - }); - if (mentionTarget) { - return mentionTarget; - } - const prefixedTarget = parseTargetPrefixes({ - raw: trimmed, prefixes: [ { prefix: "user:", kind: "user" }, { prefix: "channel:", kind: "channel" }, { prefix: "discord:", kind: "user" }, ], + atUserPattern: /^\d+$/, + atUserErrorMessage: "Discord DMs require a user id (use user: or a <@id> mention)", }); - if (prefixedTarget) { - return prefixedTarget; - } - if (trimmed.startsWith("@")) { - const candidate = trimmed.slice(1).trim(); - const id = ensureTargetId({ - candidate, - pattern: /^\d+$/, - errorMessage: "Discord DMs require a user id (use user: or a <@id> mention)", - }); - return buildMessagingTarget("user", id, trimmed); + if (userTarget) { + return userTarget; } if (/^\d+$/.test(trimmed)) { if (options.defaultKind) { diff --git a/src/discord/voice/manager.e2e.test.ts b/src/discord/voice/manager.e2e.test.ts index ab13304b5e3..13c618ed361 100644 --- a/src/discord/voice/manager.e2e.test.ts +++ b/src/discord/voice/manager.e2e.test.ts @@ -124,6 +124,44 @@ describe("DiscordVoiceManager", () => { resolveAgentRouteMock.mockClear(); }); + const createManager = ( + discordConfig: ConstructorParameters< + typeof managerModule.DiscordVoiceManager + >[0]["discordConfig"] = {}, + ) => + new managerModule.DiscordVoiceManager({ + client: createClient() as never, + cfg: {}, + discordConfig, + accountId: "default", + runtime: createRuntime(), + }); + + const expectConnectedStatus = ( + manager: InstanceType, + channelId: string, + ) => { + expect(manager.status()).toEqual([ + { + ok: true, + message: `connected: guild g1 channel ${channelId}`, + guildId: "g1", + channelId, + }, + ]); + }; + + const emitDecryptFailure = (manager: InstanceType) => { + const entry = (manager as unknown as { sessions: Map }).sessions.get("g1"); + expect(entry).toBeDefined(); + ( + manager as unknown as { handleReceiveError: (e: unknown, err: unknown) => void } + ).handleReceiveError( + entry, + new Error("Failed to decrypt: DecryptionFailed(UnencryptedWhenPassthroughDisabled)"), + ); + }; + it("keeps the new session when an old disconnected handler fires", async () => { const oldConnection = createConnectionMock(); const newConnection = createConnectionMock(); @@ -135,13 +173,7 @@ describe("DiscordVoiceManager", () => { return undefined; }); - const manager = new managerModule.DiscordVoiceManager({ - client: createClient() as never, - cfg: {}, - discordConfig: {}, - accountId: "default", - runtime: createRuntime(), - }); + const manager = createManager(); await manager.join({ guildId: "g1", channelId: "c1" }); await manager.join({ guildId: "g1", channelId: "c2" }); @@ -150,14 +182,7 @@ describe("DiscordVoiceManager", () => { expect(oldDisconnected).toBeTypeOf("function"); await oldDisconnected?.(); - expect(manager.status()).toEqual([ - { - ok: true, - message: "connected: guild g1 channel c2", - guildId: "g1", - channelId: "c2", - }, - ]); + expectConnectedStatus(manager, "c2"); }); it("keeps the new session when an old destroyed handler fires", async () => { @@ -165,13 +190,7 @@ describe("DiscordVoiceManager", () => { const newConnection = createConnectionMock(); joinVoiceChannelMock.mockReturnValueOnce(oldConnection).mockReturnValueOnce(newConnection); - const manager = new managerModule.DiscordVoiceManager({ - client: createClient() as never, - cfg: {}, - discordConfig: {}, - accountId: "default", - runtime: createRuntime(), - }); + const manager = createManager(); await manager.join({ guildId: "g1", channelId: "c1" }); await manager.join({ guildId: "g1", channelId: "c2" }); @@ -180,26 +199,13 @@ describe("DiscordVoiceManager", () => { expect(oldDestroyed).toBeTypeOf("function"); oldDestroyed?.(); - expect(manager.status()).toEqual([ - { - ok: true, - message: "connected: guild g1 channel c2", - guildId: "g1", - channelId: "c2", - }, - ]); + expectConnectedStatus(manager, "c2"); }); it("removes voice listeners on leave", async () => { const connection = createConnectionMock(); joinVoiceChannelMock.mockReturnValueOnce(connection); - const manager = new managerModule.DiscordVoiceManager({ - client: createClient() as never, - cfg: {}, - discordConfig: {}, - accountId: "default", - runtime: createRuntime(), - }); + const manager = createManager(); await manager.join({ guildId: "g1", channelId: "c1" }); await manager.leave({ guildId: "g1" }); @@ -212,17 +218,11 @@ describe("DiscordVoiceManager", () => { }); it("passes DAVE options to joinVoiceChannel", async () => { - const manager = new managerModule.DiscordVoiceManager({ - client: createClient() as never, - cfg: {}, - discordConfig: { - voice: { - daveEncryption: false, - decryptionFailureTolerance: 8, - }, + const manager = createManager({ + voice: { + daveEncryption: false, + decryptionFailureTolerance: 8, }, - accountId: "default", - runtime: createRuntime(), }); await manager.join({ guildId: "g1", channelId: "c1" }); @@ -236,36 +236,13 @@ describe("DiscordVoiceManager", () => { }); it("attempts rejoin after repeated decrypt failures", async () => { - const manager = new managerModule.DiscordVoiceManager({ - client: createClient() as never, - cfg: {}, - discordConfig: {}, - accountId: "default", - runtime: createRuntime(), - }); + const manager = createManager(); await manager.join({ guildId: "g1", channelId: "c1" }); - const entry = (manager as unknown as { sessions: Map }).sessions.get("g1"); - expect(entry).toBeDefined(); - ( - manager as unknown as { handleReceiveError: (e: unknown, err: unknown) => void } - ).handleReceiveError( - entry, - new Error("Failed to decrypt: DecryptionFailed(UnencryptedWhenPassthroughDisabled)"), - ); - ( - manager as unknown as { handleReceiveError: (e: unknown, err: unknown) => void } - ).handleReceiveError( - entry, - new Error("Failed to decrypt: DecryptionFailed(UnencryptedWhenPassthroughDisabled)"), - ); - ( - manager as unknown as { handleReceiveError: (e: unknown, err: unknown) => void } - ).handleReceiveError( - entry, - new Error("Failed to decrypt: DecryptionFailed(UnencryptedWhenPassthroughDisabled)"), - ); + emitDecryptFailure(manager); + emitDecryptFailure(manager); + emitDecryptFailure(manager); await new Promise((resolve) => setTimeout(resolve, 0)); await new Promise((resolve) => setTimeout(resolve, 0)); diff --git a/src/gateway/node-invoke-system-run-approval-match.test.ts b/src/gateway/node-invoke-system-run-approval-match.test.ts index 4f6d5d84c52..33234c2fd8d 100644 --- a/src/gateway/node-invoke-system-run-approval-match.test.ts +++ b/src/gateway/node-invoke-system-run-approval-match.test.ts @@ -2,6 +2,23 @@ import { describe, expect, test } from "vitest"; import { buildSystemRunApprovalBinding } from "../infra/system-run-approval-binding.js"; import { evaluateSystemRunApprovalMatch } from "./node-invoke-system-run-approval-match.js"; +const defaultBinding = { + cwd: null, + agentId: null, + sessionKey: null, +}; + +function expectMismatch( + result: ReturnType, + code: "APPROVAL_REQUEST_MISMATCH" | "APPROVAL_ENV_BINDING_MISSING", +) { + expect(result.ok).toBe(false); + if (result.ok) { + throw new Error("unreachable"); + } + expect(result.code).toBe(code); +} + describe("evaluateSystemRunApprovalMatch", () => { test("rejects approvals that do not carry v1 binding", () => { const result = evaluateSystemRunApprovalMatch({ @@ -10,17 +27,9 @@ describe("evaluateSystemRunApprovalMatch", () => { host: "node", command: "echo SAFE", }, - binding: { - cwd: null, - agentId: null, - sessionKey: null, - }, + binding: defaultBinding, }); - expect(result.ok).toBe(false); - if (result.ok) { - throw new Error("unreachable"); - } - expect(result.code).toBe("APPROVAL_REQUEST_MISMATCH"); + expectMismatch(result, "APPROVAL_REQUEST_MISMATCH"); }); test("enforces exact argv binding in v1 object", () => { @@ -36,11 +45,7 @@ describe("evaluateSystemRunApprovalMatch", () => { sessionKey: null, }).binding, }, - binding: { - cwd: null, - agentId: null, - sessionKey: null, - }, + binding: defaultBinding, }); expect(result).toEqual({ ok: true }); }); @@ -58,17 +63,9 @@ describe("evaluateSystemRunApprovalMatch", () => { sessionKey: null, }).binding, }, - binding: { - cwd: null, - agentId: null, - sessionKey: null, - }, + binding: defaultBinding, }); - expect(result.ok).toBe(false); - if (result.ok) { - throw new Error("unreachable"); - } - expect(result.code).toBe("APPROVAL_REQUEST_MISMATCH"); + expectMismatch(result, "APPROVAL_REQUEST_MISMATCH"); }); test("rejects env overrides when v1 binding has no env hash", () => { @@ -85,17 +82,11 @@ describe("evaluateSystemRunApprovalMatch", () => { }).binding, }, binding: { - cwd: null, - agentId: null, - sessionKey: null, + ...defaultBinding, env: { GIT_EXTERNAL_DIFF: "/tmp/pwn.sh" }, }, }); - expect(result.ok).toBe(false); - if (result.ok) { - throw new Error("unreachable"); - } - expect(result.code).toBe("APPROVAL_ENV_BINDING_MISSING"); + expectMismatch(result, "APPROVAL_ENV_BINDING_MISSING"); }); test("accepts matching env hash with reordered keys", () => { @@ -113,9 +104,7 @@ describe("evaluateSystemRunApprovalMatch", () => { }).binding, }, binding: { - cwd: null, - agentId: null, - sessionKey: null, + ...defaultBinding, env: { SAFE_B: "2", SAFE_A: "1" }, }, }); @@ -129,17 +118,9 @@ describe("evaluateSystemRunApprovalMatch", () => { host: "gateway", command: "echo SAFE", }, - binding: { - cwd: null, - agentId: null, - sessionKey: null, - }, + binding: defaultBinding, }); - expect(result.ok).toBe(false); - if (result.ok) { - throw new Error("unreachable"); - } - expect(result.code).toBe("APPROVAL_REQUEST_MISMATCH"); + expectMismatch(result, "APPROVAL_REQUEST_MISMATCH"); }); test("uses v1 binding even when legacy command text diverges", () => { @@ -156,11 +137,7 @@ describe("evaluateSystemRunApprovalMatch", () => { sessionKey: null, }).binding, }, - binding: { - cwd: null, - agentId: null, - sessionKey: null, - }, + binding: defaultBinding, }); expect(result).toEqual({ ok: true }); }); diff --git a/src/gateway/node-invoke-system-run-approval.test.ts b/src/gateway/node-invoke-system-run-approval.test.ts index dfffe562170..63f750de889 100644 --- a/src/gateway/node-invoke-system-run-approval.test.ts +++ b/src/gateway/node-invoke-system-run-approval.test.ts @@ -78,6 +78,21 @@ describe("sanitizeSystemRunParamsForForwarding", () => { expect(params.approvalDecision).toBe("allow-once"); } + function expectRejectedForwardingResult( + result: ReturnType, + code: string, + messageSubstring?: string, + ) { + expect(result.ok).toBe(false); + if (result.ok) { + throw new Error("unreachable"); + } + if (messageSubstring) { + expect(result.message).toContain(messageSubstring); + } + expect(result.details?.code).toBe(code); + } + test("rejects cmd.exe /c trailing-arg mismatch against rawCommand", () => { const result = sanitizeSystemRunParamsForForwarding({ rawParams: { @@ -92,12 +107,11 @@ describe("sanitizeSystemRunParamsForForwarding", () => { execApprovalManager: manager(makeRecord("echo")), nowMs: now, }); - expect(result.ok).toBe(false); - if (result.ok) { - throw new Error("unreachable"); - } - expect(result.message).toContain("rawCommand does not match command"); - expect(result.details?.code).toBe("RAW_COMMAND_MISMATCH"); + expectRejectedForwardingResult( + result, + "RAW_COMMAND_MISMATCH", + "rawCommand does not match command", + ); }); test("accepts matching cmd.exe /c command text for approval binding", () => { @@ -139,12 +153,11 @@ describe("sanitizeSystemRunParamsForForwarding", () => { execApprovalManager: manager(makeRecord("echo SAFE")), nowMs: now, }); - expect(result.ok).toBe(false); - if (result.ok) { - throw new Error("unreachable"); - } - expect(result.message).toContain("approval id does not match request"); - expect(result.details?.code).toBe("APPROVAL_REQUEST_MISMATCH"); + expectRejectedForwardingResult( + result, + "APPROVAL_REQUEST_MISMATCH", + "approval id does not match request", + ); }); test("accepts env-assignment shell wrapper only when approval command matches full argv text", () => { @@ -184,12 +197,11 @@ describe("sanitizeSystemRunParamsForForwarding", () => { execApprovalManager: manager(makeRecord("runner")), nowMs: now, }); - expect(result.ok).toBe(false); - if (result.ok) { - throw new Error("unreachable"); - } - expect(result.message).toContain("approval id does not match request"); - expect(result.details?.code).toBe("APPROVAL_REQUEST_MISMATCH"); + expectRejectedForwardingResult( + result, + "APPROVAL_REQUEST_MISMATCH", + "approval id does not match request", + ); }); test("enforces commandArgv identity when approval includes argv binding", () => { @@ -205,12 +217,11 @@ describe("sanitizeSystemRunParamsForForwarding", () => { execApprovalManager: manager(makeRecord("echo SAFE", ["echo SAFE"])), nowMs: now, }); - expect(result.ok).toBe(false); - if (result.ok) { - throw new Error("unreachable"); - } - expect(result.message).toContain("approval id does not match request"); - expect(result.details?.code).toBe("APPROVAL_REQUEST_MISMATCH"); + expectRejectedForwardingResult( + result, + "APPROVAL_REQUEST_MISMATCH", + "approval id does not match request", + ); }); test("accepts matching commandArgv binding for trailing-space argv", () => { @@ -287,11 +298,7 @@ describe("sanitizeSystemRunParamsForForwarding", () => { execApprovalManager: manager(makeRecord("git diff", ["git", "diff"])), nowMs: now, }); - expect(result.ok).toBe(false); - if (result.ok) { - throw new Error("unreachable"); - } - expect(result.details?.code).toBe("APPROVAL_ENV_BINDING_MISSING"); + expectRejectedForwardingResult(result, "APPROVAL_ENV_BINDING_MISSING"); }); test("rejects env hash mismatch", () => { @@ -317,11 +324,7 @@ describe("sanitizeSystemRunParamsForForwarding", () => { execApprovalManager: manager(record), nowMs: now, }); - expect(result.ok).toBe(false); - if (result.ok) { - throw new Error("unreachable"); - } - expect(result.details?.code).toBe("APPROVAL_ENV_MISMATCH"); + expectRejectedForwardingResult(result, "APPROVAL_ENV_MISMATCH"); }); test("accepts matching env hash with reordered keys", () => { @@ -405,11 +408,7 @@ describe("sanitizeSystemRunParamsForForwarding", () => { execApprovalManager: approvalManager, nowMs: now, }); - expect(second.ok).toBe(false); - if (second.ok) { - throw new Error("unreachable"); - } - expect(second.details?.code).toBe("APPROVAL_REQUIRED"); + expectRejectedForwardingResult(second, "APPROVAL_REQUIRED"); }); test("rejects approval ids that do not bind a nodeId", () => { @@ -427,12 +426,7 @@ describe("sanitizeSystemRunParamsForForwarding", () => { execApprovalManager: manager(record), nowMs: now, }); - expect(result.ok).toBe(false); - if (result.ok) { - throw new Error("unreachable"); - } - expect(result.message).toContain("missing node binding"); - expect(result.details?.code).toBe("APPROVAL_NODE_BINDING_MISSING"); + expectRejectedForwardingResult(result, "APPROVAL_NODE_BINDING_MISSING", "missing node binding"); }); test("rejects approval ids replayed against a different nodeId", () => { @@ -448,11 +442,6 @@ describe("sanitizeSystemRunParamsForForwarding", () => { execApprovalManager: manager(makeRecord("echo SAFE")), nowMs: now, }); - expect(result.ok).toBe(false); - if (result.ok) { - throw new Error("unreachable"); - } - expect(result.message).toContain("not valid for this node"); - expect(result.details?.code).toBe("APPROVAL_NODE_MISMATCH"); + expectRejectedForwardingResult(result, "APPROVAL_NODE_MISMATCH", "not valid for this node"); }); }); diff --git a/src/gateway/server-methods/send.test.ts b/src/gateway/server-methods/send.test.ts index aa3a6593bd2..0220a4d6895 100644 --- a/src/gateway/server-methods/send.test.ts +++ b/src/gateway/server-methods/send.test.ts @@ -120,6 +120,21 @@ async function runPoll(params: Record) { return { respond }; } +function expectDeliverySessionMirror(params: { agentId: string; sessionKey: string }) { + expect(mocks.deliverOutboundPayloads).toHaveBeenCalledWith( + expect.objectContaining({ + session: expect.objectContaining({ + agentId: params.agentId, + key: params.sessionKey, + }), + mirror: expect.objectContaining({ + sessionKey: params.sessionKey, + agentId: params.agentId, + }), + }), + ); +} + function mockDeliverySuccess(messageId: string) { mocks.deliverOutboundPayloads.mockResolvedValue([{ messageId, channel: "slack" }]); } @@ -423,18 +438,10 @@ describe("gateway send mirroring", () => { idempotencyKey: "idem-session-agent", }); - expect(mocks.deliverOutboundPayloads).toHaveBeenCalledWith( - expect.objectContaining({ - session: expect.objectContaining({ - agentId: "work", - key: "agent:work:slack:channel:c1", - }), - mirror: expect.objectContaining({ - sessionKey: "agent:work:slack:channel:c1", - agentId: "work", - }), - }), - ); + expectDeliverySessionMirror({ + agentId: "work", + sessionKey: "agent:work:slack:channel:c1", + }); }); it("prefers explicit agentId over sessionKey agent for delivery and mirror", async () => { @@ -475,18 +482,10 @@ describe("gateway send mirroring", () => { idempotencyKey: "idem-agent-blank", }); - expect(mocks.deliverOutboundPayloads).toHaveBeenCalledWith( - expect.objectContaining({ - session: expect.objectContaining({ - agentId: "work", - key: "agent:work:slack:channel:c1", - }), - mirror: expect.objectContaining({ - sessionKey: "agent:work:slack:channel:c1", - agentId: "work", - }), - }), - ); + expectDeliverySessionMirror({ + agentId: "work", + sessionKey: "agent:work:slack:channel:c1", + }); }); it("forwards threadId to outbound delivery when provided", async () => { diff --git a/src/gateway/server.hooks.test.ts b/src/gateway/server.hooks.test.ts index 473b4e855aa..9e1ee754cb7 100644 --- a/src/gateway/server.hooks.test.ts +++ b/src/gateway/server.hooks.test.ts @@ -12,29 +12,47 @@ import { installGatewayTestHooks({ scope: "suite" }); const resolveMainKey = () => resolveMainSessionKeyFromConfig(); +const HOOK_TOKEN = "hook-secret"; + +function buildHookJsonHeaders(options?: { + token?: string | null; + headers?: Record; +}): Record { + const token = options?.token === undefined ? HOOK_TOKEN : options.token; + return { + "Content-Type": "application/json", + ...(token ? { Authorization: `Bearer ${token}` } : {}), + ...options?.headers, + }; +} + +async function postHook( + port: number, + path: string, + body: Record | string, + options?: { + token?: string | null; + headers?: Record; + }, +): Promise { + return fetch(`http://127.0.0.1:${port}${path}`, { + method: "POST", + headers: buildHookJsonHeaders(options), + body: typeof body === "string" ? body : JSON.stringify(body), + }); +} describe("gateway server hooks", () => { test("handles auth, wake, and agent flows", async () => { - testState.hooksConfig = { enabled: true, token: "hook-secret" }; + testState.hooksConfig = { enabled: true, token: HOOK_TOKEN }; testState.agentsConfig = { list: [{ id: "main", default: true }, { id: "hooks" }], }; await withGatewayServer(async ({ port }) => { - const resNoAuth = await fetch(`http://127.0.0.1:${port}/hooks/wake`, { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ text: "Ping" }), - }); + const resNoAuth = await postHook(port, "/hooks/wake", { text: "Ping" }, { token: null }); expect(resNoAuth.status).toBe(401); - const resWake = await fetch(`http://127.0.0.1:${port}/hooks/wake`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer hook-secret", - }, - body: JSON.stringify({ text: "Ping", mode: "next-heartbeat" }), - }); + const resWake = await postHook(port, "/hooks/wake", { text: "Ping", mode: "next-heartbeat" }); expect(resWake.status).toBe(200); const wakeEvents = await waitForSystemEvent(); expect(wakeEvents.some((e) => e.includes("Ping"))).toBe(true); @@ -45,14 +63,7 @@ describe("gateway server hooks", () => { status: "ok", summary: "done", }); - const resAgent = await fetch(`http://127.0.0.1:${port}/hooks/agent`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer hook-secret", - }, - body: JSON.stringify({ message: "Do it", name: "Email" }), - }); + 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); @@ -63,17 +74,10 @@ describe("gateway server hooks", () => { status: "ok", summary: "done", }); - const resAgentModel = await fetch(`http://127.0.0.1:${port}/hooks/agent`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer hook-secret", - }, - body: JSON.stringify({ - message: "Do it", - name: "Email", - model: "openai/gpt-4.1-mini", - }), + const resAgentModel = await postHook(port, "/hooks/agent", { + message: "Do it", + name: "Email", + model: "openai/gpt-4.1-mini", }); expect(resAgentModel.status).toBe(202); await waitForSystemEvent(); @@ -88,13 +92,10 @@ describe("gateway server hooks", () => { status: "ok", summary: "done", }); - const resAgentWithId = await fetch(`http://127.0.0.1:${port}/hooks/agent`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer hook-secret", - }, - body: JSON.stringify({ message: "Do it", name: "Email", agentId: "hooks" }), + const resAgentWithId = await postHook(port, "/hooks/agent", { + message: "Do it", + name: "Email", + agentId: "hooks", }); expect(resAgentWithId.status).toBe(202); await waitForSystemEvent(); @@ -109,13 +110,10 @@ describe("gateway server hooks", () => { status: "ok", summary: "done", }); - const resAgentUnknown = await fetch(`http://127.0.0.1:${port}/hooks/agent`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer hook-secret", - }, - body: JSON.stringify({ message: "Do it", name: "Email", agentId: "missing-agent" }), + const resAgentUnknown = await postHook(port, "/hooks/agent", { + message: "Do it", + name: "Email", + agentId: "missing-agent", }); expect(resAgentUnknown.status).toBe(202); await waitForSystemEvent(); @@ -125,32 +123,27 @@ describe("gateway server hooks", () => { expect(fallbackCall?.job?.agentId).toBe("main"); drainSystemEvents(resolveMainKey()); - const resQuery = await fetch(`http://127.0.0.1:${port}/hooks/wake?token=hook-secret`, { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ text: "Query auth" }), - }); + const resQuery = await postHook( + port, + "/hooks/wake?token=hook-secret", + { text: "Query auth" }, + { token: null }, + ); expect(resQuery.status).toBe(400); - const resBadChannel = await fetch(`http://127.0.0.1:${port}/hooks/agent`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer hook-secret", - }, - body: JSON.stringify({ message: "Nope", channel: "sms" }), + const resBadChannel = await postHook(port, "/hooks/agent", { + message: "Nope", + channel: "sms", }); expect(resBadChannel.status).toBe(400); expect(peekSystemEvents(resolveMainKey()).length).toBe(0); - const resHeader = await fetch(`http://127.0.0.1:${port}/hooks/wake`, { - method: "POST", - headers: { - "Content-Type": "application/json", - "x-openclaw-token": "hook-secret", - }, - body: JSON.stringify({ text: "Header auth" }), - }); + const resHeader = await postHook( + port, + "/hooks/wake", + { text: "Header auth" }, + { token: null, headers: { "x-openclaw-token": HOOK_TOKEN } }, + ); expect(resHeader.status).toBe(200); const headerEvents = await waitForSystemEvent(); expect(headerEvents.some((e) => e.includes("Header auth"))).toBe(true); @@ -162,51 +155,23 @@ describe("gateway server hooks", () => { }); expect(resGet.status).toBe(405); - const resBlankText = await fetch(`http://127.0.0.1:${port}/hooks/wake`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer hook-secret", - }, - body: JSON.stringify({ text: " " }), - }); + const resBlankText = await postHook(port, "/hooks/wake", { text: " " }); expect(resBlankText.status).toBe(400); - const resBlankMessage = await fetch(`http://127.0.0.1:${port}/hooks/agent`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer hook-secret", - }, - body: JSON.stringify({ message: " " }), - }); + const resBlankMessage = await postHook(port, "/hooks/agent", { message: " " }); expect(resBlankMessage.status).toBe(400); - const resBadJson = await fetch(`http://127.0.0.1:${port}/hooks/wake`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer hook-secret", - }, - body: "{", - }); + const resBadJson = await postHook(port, "/hooks/wake", "{"); expect(resBadJson.status).toBe(400); }); }); test("rejects request sessionKey unless hooks.allowRequestSessionKey is enabled", async () => { - testState.hooksConfig = { enabled: true, token: "hook-secret" }; + testState.hooksConfig = { enabled: true, token: HOOK_TOKEN }; await withGatewayServer(async ({ port }) => { - const denied = await fetch(`http://127.0.0.1:${port}/hooks/agent`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer hook-secret", - }, - body: JSON.stringify({ - message: "Do it", - sessionKey: "agent:main:dm:u99999", - }), + const denied = await postHook(port, "/hooks/agent", { + message: "Do it", + sessionKey: "agent:main:dm:u99999", }); expect(denied.status).toBe(400); const deniedBody = (await denied.json()) as { error?: string }; @@ -217,7 +182,7 @@ describe("gateway server hooks", () => { test("respects hooks session policy for request + mapping session keys", async () => { testState.hooksConfig = { enabled: true, - token: "hook-secret", + token: HOOK_TOKEN, allowRequestSessionKey: true, allowedSessionKeyPrefixes: ["hook:"], defaultSessionKey: "hook:ingress", @@ -240,14 +205,7 @@ describe("gateway server hooks", () => { cronIsolatedRun.mockClear(); cronIsolatedRun.mockResolvedValue({ status: "ok", summary: "done" }); - const defaultRoute = await fetch(`http://127.0.0.1:${port}/hooks/agent`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer hook-secret", - }, - body: JSON.stringify({ message: "No key" }), - }); + const defaultRoute = await postHook(port, "/hooks/agent", { message: "No key" }); expect(defaultRoute.status).toBe(202); await waitForSystemEvent(); const defaultCall = (cronIsolatedRun.mock.calls[0] as unknown[] | undefined)?.[0] as @@ -258,14 +216,7 @@ describe("gateway server hooks", () => { cronIsolatedRun.mockClear(); cronIsolatedRun.mockResolvedValue({ status: "ok", summary: "done" }); - const mappedOk = await fetch(`http://127.0.0.1:${port}/hooks/mapped-ok`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer hook-secret", - }, - body: JSON.stringify({ subject: "hello", id: "42" }), - }); + const mappedOk = await postHook(port, "/hooks/mapped-ok", { subject: "hello", id: "42" }); expect(mappedOk.status).toBe(202); await waitForSystemEvent(); const mappedCall = (cronIsolatedRun.mock.calls[0] as unknown[] | undefined)?.[0] as @@ -274,27 +225,13 @@ describe("gateway server hooks", () => { expect(mappedCall?.sessionKey).toBe("hook:mapped:42"); drainSystemEvents(resolveMainKey()); - const requestBadPrefix = await fetch(`http://127.0.0.1:${port}/hooks/agent`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer hook-secret", - }, - body: JSON.stringify({ - message: "Bad key", - sessionKey: "agent:main:main", - }), + const requestBadPrefix = await postHook(port, "/hooks/agent", { + message: "Bad key", + sessionKey: "agent:main:main", }); expect(requestBadPrefix.status).toBe(400); - const mappedBadPrefix = await fetch(`http://127.0.0.1:${port}/hooks/mapped-bad`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer hook-secret", - }, - body: JSON.stringify({ subject: "hello" }), - }); + const mappedBadPrefix = await postHook(port, "/hooks/mapped-bad", { subject: "hello" }); expect(mappedBadPrefix.status).toBe(400); }); }); @@ -302,7 +239,7 @@ describe("gateway server hooks", () => { test("normalizes duplicate target-agent prefixes before isolated dispatch", async () => { testState.hooksConfig = { enabled: true, - token: "hook-secret", + token: HOOK_TOKEN, allowRequestSessionKey: true, allowedSessionKeyPrefixes: ["hook:", "agent:"], }; @@ -316,18 +253,11 @@ describe("gateway server hooks", () => { summary: "done", }); - const resAgent = await fetch(`http://127.0.0.1:${port}/hooks/agent`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer hook-secret", - }, - body: JSON.stringify({ - message: "Do it", - name: "Email", - agentId: "hooks", - sessionKey: "agent:hooks:slack:channel:c123", - }), + const resAgent = await postHook(port, "/hooks/agent", { + message: "Do it", + name: "Email", + agentId: "hooks", + sessionKey: "agent:hooks:slack:channel:c123", }); expect(resAgent.status).toBe(202); await waitForSystemEvent(); @@ -344,7 +274,7 @@ describe("gateway server hooks", () => { test("enforces hooks.allowedAgentIds for explicit agent routing", async () => { testState.hooksConfig = { enabled: true, - token: "hook-secret", + token: HOOK_TOKEN, allowedAgentIds: ["hooks"], mappings: [ { @@ -364,14 +294,7 @@ describe("gateway server hooks", () => { status: "ok", summary: "done", }); - const resNoAgent = await fetch(`http://127.0.0.1:${port}/hooks/agent`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer hook-secret", - }, - body: JSON.stringify({ message: "No explicit agent" }), - }); + const resNoAgent = await postHook(port, "/hooks/agent", { message: "No explicit agent" }); expect(resNoAgent.status).toBe(202); await waitForSystemEvent(); const noAgentCall = (cronIsolatedRun.mock.calls[0] as unknown[] | undefined)?.[0] as { @@ -385,13 +308,9 @@ describe("gateway server hooks", () => { status: "ok", summary: "done", }); - const resAllowed = await fetch(`http://127.0.0.1:${port}/hooks/agent`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer hook-secret", - }, - body: JSON.stringify({ message: "Allowed", agentId: "hooks" }), + const resAllowed = await postHook(port, "/hooks/agent", { + message: "Allowed", + agentId: "hooks", }); expect(resAllowed.status).toBe(202); await waitForSystemEvent(); @@ -401,26 +320,15 @@ describe("gateway server hooks", () => { expect(allowedCall?.job?.agentId).toBe("hooks"); drainSystemEvents(resolveMainKey()); - const resDenied = await fetch(`http://127.0.0.1:${port}/hooks/agent`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer hook-secret", - }, - body: JSON.stringify({ message: "Denied", agentId: "main" }), + const resDenied = await postHook(port, "/hooks/agent", { + message: "Denied", + agentId: "main", }); expect(resDenied.status).toBe(400); const deniedBody = (await resDenied.json()) as { error?: string }; expect(deniedBody.error).toContain("hooks.allowedAgentIds"); - const resMappedDenied = await fetch(`http://127.0.0.1:${port}/hooks/mapped`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer hook-secret", - }, - body: JSON.stringify({ subject: "hello" }), - }); + const resMappedDenied = await postHook(port, "/hooks/mapped", { subject: "hello" }); expect(resMappedDenied.status).toBe(400); const mappedDeniedBody = (await resMappedDenied.json()) as { error?: string }; expect(mappedDeniedBody.error).toContain("hooks.allowedAgentIds"); @@ -431,20 +339,16 @@ describe("gateway server hooks", () => { test("denies explicit agentId when hooks.allowedAgentIds is empty", async () => { testState.hooksConfig = { enabled: true, - token: "hook-secret", + token: HOOK_TOKEN, allowedAgentIds: [], }; testState.agentsConfig = { list: [{ id: "main", default: true }, { id: "hooks" }], }; await withGatewayServer(async ({ port }) => { - const resDenied = await fetch(`http://127.0.0.1:${port}/hooks/agent`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer hook-secret", - }, - body: JSON.stringify({ message: "Denied", agentId: "hooks" }), + const resDenied = await postHook(port, "/hooks/agent", { + message: "Denied", + agentId: "hooks", }); expect(resDenied.status).toBe(400); const deniedBody = (await resDenied.json()) as { error?: string }; @@ -454,52 +358,34 @@ describe("gateway server hooks", () => { }); test("throttles repeated hook auth failures and resets after success", async () => { - testState.hooksConfig = { enabled: true, token: "hook-secret" }; + testState.hooksConfig = { enabled: true, token: HOOK_TOKEN }; await withGatewayServer(async ({ port }) => { - const firstFail = await fetch(`http://127.0.0.1:${port}/hooks/wake`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer wrong", - }, - body: JSON.stringify({ text: "blocked" }), - }); + const firstFail = await postHook( + port, + "/hooks/wake", + { text: "blocked" }, + { token: "wrong" }, + ); expect(firstFail.status).toBe(401); let throttled: Response | null = null; for (let i = 0; i < 20; i++) { - throttled = await fetch(`http://127.0.0.1:${port}/hooks/wake`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer wrong", - }, - body: JSON.stringify({ text: "blocked" }), - }); + throttled = await postHook(port, "/hooks/wake", { text: "blocked" }, { token: "wrong" }); } expect(throttled?.status).toBe(429); expect(throttled?.headers.get("retry-after")).toBeTruthy(); - const allowed = await fetch(`http://127.0.0.1:${port}/hooks/wake`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer hook-secret", - }, - body: JSON.stringify({ text: "auth reset" }), - }); + const allowed = await postHook(port, "/hooks/wake", { text: "auth reset" }); expect(allowed.status).toBe(200); await waitForSystemEvent(); drainSystemEvents(resolveMainKey()); - const failAfterSuccess = await fetch(`http://127.0.0.1:${port}/hooks/wake`, { - method: "POST", - headers: { - "Content-Type": "application/json", - Authorization: "Bearer wrong", - }, - body: JSON.stringify({ text: "blocked" }), - }); + const failAfterSuccess = await postHook( + port, + "/hooks/wake", + { text: "blocked" }, + { token: "wrong" }, + ); expect(failAfterSuccess.status).toBe(401); }); }); diff --git a/src/gateway/session-utils.test.ts b/src/gateway/session-utils.test.ts index e765210e207..ff090f2248f 100644 --- a/src/gateway/session-utils.test.ts +++ b/src/gateway/session-utils.test.ts @@ -399,17 +399,23 @@ describe("resolveSessionModelRef", () => { }); describe("resolveSessionModelIdentityRef", () => { + const resolveLegacyIdentityRef = ( + cfg: OpenClawConfig, + modelProvider: string | undefined = undefined, + ) => + resolveSessionModelIdentityRef(cfg, { + sessionId: "legacy-session", + updatedAt: Date.now(), + model: "claude-sonnet-4-6", + modelProvider, + }); + test("does not inherit default provider for unprefixed legacy runtime model", () => { const cfg = createModelDefaultsConfig({ primary: "google-gemini-cli/gemini-3-pro-preview", }); - const resolved = resolveSessionModelIdentityRef(cfg, { - sessionId: "legacy-session", - updatedAt: Date.now(), - model: "claude-sonnet-4-6", - modelProvider: undefined, - }); + const resolved = resolveLegacyIdentityRef(cfg); expect(resolved).toEqual({ model: "claude-sonnet-4-6" }); }); @@ -422,12 +428,7 @@ describe("resolveSessionModelIdentityRef", () => { }, }); - const resolved = resolveSessionModelIdentityRef(cfg, { - sessionId: "legacy-session", - updatedAt: Date.now(), - model: "claude-sonnet-4-6", - modelProvider: undefined, - }); + const resolved = resolveLegacyIdentityRef(cfg); expect(resolved).toEqual({ provider: "anthropic", model: "claude-sonnet-4-6" }); }); @@ -441,12 +442,7 @@ describe("resolveSessionModelIdentityRef", () => { }, }); - const resolved = resolveSessionModelIdentityRef(cfg, { - sessionId: "legacy-session", - updatedAt: Date.now(), - model: "claude-sonnet-4-6", - modelProvider: undefined, - }); + const resolved = resolveLegacyIdentityRef(cfg); expect(resolved).toEqual({ model: "claude-sonnet-4-6" }); }); diff --git a/src/imessage/monitor/inbound-processing.test.ts b/src/imessage/monitor/inbound-processing.test.ts index 5eb13e097b9..fab878a4cc7 100644 --- a/src/imessage/monitor/inbound-processing.test.ts +++ b/src/imessage/monitor/inbound-processing.test.ts @@ -61,13 +61,12 @@ describe("describeIMessageEchoDropLog", () => { describe("resolveIMessageInboundDecision command auth", () => { const cfg = {} as OpenClawConfig; - - it("does not auto-authorize DM commands in open mode without allowlists", () => { - const decision = resolveIMessageInboundDecision({ + const resolveDmCommandDecision = (params: { messageId: number; storeAllowFrom: string[] }) => + resolveIMessageInboundDecision({ cfg, accountId: "default", message: { - id: 100, + id: params.messageId, sender: "+15555550123", text: "/status", is_from_me: false, @@ -80,13 +79,19 @@ describe("resolveIMessageInboundDecision command auth", () => { groupAllowFrom: [], groupPolicy: "open", dmPolicy: "open", - storeAllowFrom: [], + storeAllowFrom: params.storeAllowFrom, historyLimit: 0, groupHistories: new Map(), echoCache: undefined, logVerbose: undefined, }); + it("does not auto-authorize DM commands in open mode without allowlists", () => { + const decision = resolveDmCommandDecision({ + messageId: 100, + storeAllowFrom: [], + }); + expect(decision.kind).toBe("dispatch"); if (decision.kind !== "dispatch") { return; @@ -95,28 +100,9 @@ describe("resolveIMessageInboundDecision command auth", () => { }); it("authorizes DM commands for senders in pairing-store allowlist", () => { - const decision = resolveIMessageInboundDecision({ - cfg, - accountId: "default", - message: { - id: 101, - sender: "+15555550123", - text: "/status", - is_from_me: false, - is_group: false, - }, - opts: undefined, - messageText: "/status", - bodyText: "/status", - allowFrom: [], - groupAllowFrom: [], - groupPolicy: "open", - dmPolicy: "open", + const decision = resolveDmCommandDecision({ + messageId: 101, storeAllowFrom: ["+15555550123"], - historyLimit: 0, - groupHistories: new Map(), - echoCache: undefined, - logVerbose: undefined, }); expect(decision.kind).toBe("dispatch"); diff --git a/src/line/bot-message-context.ts b/src/line/bot-message-context.ts index dd1da2ffbfe..255aa34bfc7 100644 --- a/src/line/bot-message-context.ts +++ b/src/line/bot-message-context.ts @@ -1,14 +1,10 @@ import type { MessageEvent, StickerEventMessage, EventSource, PostbackEvent } from "@line/bot-sdk"; -import { formatInboundEnvelope, resolveEnvelopeFormatOptions } from "../auto-reply/envelope.js"; +import { formatInboundEnvelope } from "../auto-reply/envelope.js"; import { finalizeInboundContext } from "../auto-reply/reply/inbound-context.js"; import { formatLocationText, toLocationContext } from "../channels/location.js"; +import { resolveInboundSessionEnvelopeContext } from "../channels/session-envelope.js"; import type { OpenClawConfig } from "../config/config.js"; -import { - readSessionUpdatedAt, - recordSessionMetaFromInbound, - resolveStorePath, - updateLastRoute, -} from "../config/sessions.js"; +import { recordSessionMetaFromInbound, updateLastRoute } from "../config/sessions.js"; import { logVerbose, shouldLogVerbose } from "../globals.js"; import { recordChannelActivity } from "../infra/channel-activity.js"; import { resolveAgentRoute } from "../routing/resolve-route.js"; @@ -243,12 +239,9 @@ async function finalizeLineInboundContext(params: { senderLabel, }); - const storePath = resolveStorePath(params.cfg.session?.store, { + const { storePath, envelopeOptions, previousTimestamp } = resolveInboundSessionEnvelopeContext({ + cfg: params.cfg, agentId: params.route.agentId, - }); - const envelopeOptions = resolveEnvelopeFormatOptions(params.cfg); - const previousTimestamp = readSessionUpdatedAt({ - storePath, sessionKey: params.route.sessionKey, }); diff --git a/src/plugin-sdk/inbound-envelope.ts b/src/plugin-sdk/inbound-envelope.ts index 69258432fc1..2a4ff0aaa06 100644 --- a/src/plugin-sdk/inbound-envelope.ts +++ b/src/plugin-sdk/inbound-envelope.ts @@ -8,6 +8,22 @@ type RoutePeerLike = { id: string | number; }; +type InboundEnvelopeFormatParams = { + channel: string; + from: string; + timestamp?: number; + previousTimestamp?: number; + envelope: TEnvelope; + body: string; +}; + +type InboundRouteResolveParams = { + cfg: TConfig; + channel: string; + accountId: string; + peer: TPeer; +}; + export function createInboundEnvelopeBuilder(params: { cfg: TConfig; route: RouteLike; @@ -15,14 +31,7 @@ export function createInboundEnvelopeBuilder(params: { resolveStorePath: (store: string | undefined, opts: { agentId: string }) => string; readSessionUpdatedAt: (params: { storePath: string; sessionKey: string }) => number | undefined; resolveEnvelopeFormatOptions: (cfg: TConfig) => TEnvelope; - formatAgentEnvelope: (params: { - channel: string; - from: string; - timestamp?: number; - previousTimestamp?: number; - envelope: TEnvelope; - body: string; - }) => string; + formatAgentEnvelope: (params: InboundEnvelopeFormatParams) => string; }) { const storePath = params.resolveStorePath(params.sessionStore, { agentId: params.route.agentId, @@ -55,24 +64,12 @@ export function resolveInboundRouteEnvelopeBuilder< channel: string; accountId: string; peer: TPeer; - resolveAgentRoute: (params: { - cfg: TConfig; - channel: string; - accountId: string; - peer: TPeer; - }) => TRoute; + resolveAgentRoute: (params: InboundRouteResolveParams) => TRoute; sessionStore?: string; resolveStorePath: (store: string | undefined, opts: { agentId: string }) => string; readSessionUpdatedAt: (params: { storePath: string; sessionKey: string }) => number | undefined; resolveEnvelopeFormatOptions: (cfg: TConfig) => TEnvelope; - formatAgentEnvelope: (params: { - channel: string; - from: string; - timestamp?: number; - previousTimestamp?: number; - envelope: TEnvelope; - body: string; - }) => string; + formatAgentEnvelope: (params: InboundEnvelopeFormatParams) => string; }): { route: TRoute; buildEnvelope: ReturnType>; @@ -102,12 +99,7 @@ type InboundRouteEnvelopeRuntime< TPeer extends RoutePeerLike, > = { routing: { - resolveAgentRoute: (params: { - cfg: TConfig; - channel: string; - accountId: string; - peer: TPeer; - }) => TRoute; + resolveAgentRoute: (params: InboundRouteResolveParams) => TRoute; }; session: { resolveStorePath: (store: string | undefined, opts: { agentId: string }) => string; @@ -115,14 +107,7 @@ type InboundRouteEnvelopeRuntime< }; reply: { resolveEnvelopeFormatOptions: (cfg: TConfig) => TEnvelope; - formatAgentEnvelope: (params: { - channel: string; - from: string; - timestamp?: number; - previousTimestamp?: number; - envelope: TEnvelope; - body: string; - }) => string; + formatAgentEnvelope: (params: InboundEnvelopeFormatParams) => string; }; }; diff --git a/src/plugin-sdk/webhook-targets.ts b/src/plugin-sdk/webhook-targets.ts index de90c398667..298b3d14974 100644 --- a/src/plugin-sdk/webhook-targets.ts +++ b/src/plugin-sdk/webhook-targets.ts @@ -112,6 +112,23 @@ export type WebhookTargetMatchResult = | { kind: "single"; target: T } | { kind: "ambiguous" }; +function updateMatchedWebhookTarget( + matched: T | undefined, + target: T, +): { ok: true; matched: T } | { ok: false; result: WebhookTargetMatchResult } { + if (matched) { + return { ok: false, result: { kind: "ambiguous" } }; + } + return { ok: true, matched: target }; +} + +function finalizeMatchedWebhookTarget(matched: T | undefined): WebhookTargetMatchResult { + if (!matched) { + return { kind: "none" }; + } + return { kind: "single", target: matched }; +} + export function resolveSingleWebhookTarget( targets: readonly T[], isMatch: (target: T) => boolean, @@ -121,15 +138,13 @@ export function resolveSingleWebhookTarget( if (!isMatch(target)) { continue; } - if (matched) { - return { kind: "ambiguous" }; + const updated = updateMatchedWebhookTarget(matched, target); + if (!updated.ok) { + return updated.result; } - matched = target; + matched = updated.matched; } - if (!matched) { - return { kind: "none" }; - } - return { kind: "single", target: matched }; + return finalizeMatchedWebhookTarget(matched); } export async function resolveSingleWebhookTargetAsync( @@ -141,15 +156,13 @@ export async function resolveSingleWebhookTargetAsync( if (!(await isMatch(target))) { continue; } - if (matched) { - return { kind: "ambiguous" }; + const updated = updateMatchedWebhookTarget(matched, target); + if (!updated.ok) { + return updated.result; } - matched = target; + matched = updated.matched; } - if (!matched) { - return { kind: "none" }; - } - return { kind: "single", target: matched }; + return finalizeMatchedWebhookTarget(matched); } export async function resolveWebhookTargetWithAuthOrReject(params: { diff --git a/src/security/dm-policy-shared.test.ts b/src/security/dm-policy-shared.test.ts index b68489222b0..c28225ab71f 100644 --- a/src/security/dm-policy-shared.test.ts +++ b/src/security/dm-policy-shared.test.ts @@ -10,6 +10,12 @@ import { } from "./dm-policy-shared.js"; describe("security/dm-policy-shared", () => { + const controlCommand = { + useAccessGroups: true, + allowTextCommands: true, + hasControlCommand: true, + } as const; + it("normalizes config + store allow entries and counts distinct senders", async () => { const state = await resolveDmAllowState({ provider: "telegram", @@ -148,11 +154,7 @@ describe("security/dm-policy-shared", () => { groupAllowFrom: ["group-owner"], storeAllowFrom: ["paired-user"], isSenderAllowed: (allowFrom) => allowFrom.includes("paired-user"), - command: { - useAccessGroups: true, - allowTextCommands: true, - hasControlCommand: true, - }, + command: controlCommand, }); expect(resolved.decision).toBe("block"); expect(resolved.reason).toBe("groupPolicy=allowlist (not allowlisted)"); @@ -169,11 +171,7 @@ describe("security/dm-policy-shared", () => { groupAllowFrom: [], storeAllowFrom: ["paired-user"], isSenderAllowed: (allowFrom) => allowFrom.includes("owner"), - command: { - useAccessGroups: true, - allowTextCommands: true, - hasControlCommand: true, - }, + command: controlCommand, }); expect(resolved.commandAuthorized).toBe(true); expect(resolved.shouldBlockControlCommand).toBe(false); @@ -188,11 +186,7 @@ describe("security/dm-policy-shared", () => { groupAllowFrom: ["group-owner"], storeAllowFrom: ["paired-user"], isSenderAllowed: (allowFrom) => allowFrom.includes("paired-user"), - command: { - useAccessGroups: true, - allowTextCommands: true, - hasControlCommand: true, - }, + command: controlCommand, }); expect(resolved.decision).toBe("allow"); expect(resolved.commandAuthorized).toBe(true); @@ -208,11 +202,7 @@ describe("security/dm-policy-shared", () => { groupAllowFrom: [], storeAllowFrom: [], isSenderAllowed: () => false, - command: { - useAccessGroups: true, - allowTextCommands: true, - hasControlCommand: true, - }, + command: controlCommand, }); expect(resolved.decision).toBe("allow"); expect(resolved.commandAuthorized).toBe(false); diff --git a/src/slack/monitor/message-handler.test.ts b/src/slack/monitor/message-handler.test.ts index f19f640ed6e..8453b9ce4b0 100644 --- a/src/slack/monitor/message-handler.test.ts +++ b/src/slack/monitor/message-handler.test.ts @@ -36,6 +36,18 @@ function createContext(overrides?: { } as Parameters[0]["ctx"]; } +function createHandlerWithTracker(overrides?: { + markMessageSeen?: (channel: string | undefined, ts: string | undefined) => boolean; +}) { + const trackEvent = vi.fn(); + const handler = createSlackMessageHandler({ + ctx: createContext(overrides), + account: { accountId: "default" } as Parameters[0]["account"], + trackEvent, + }); + return { handler, trackEvent }; +} + describe("createSlackMessageHandler", () => { beforeEach(() => { enqueueMock.mockClear(); @@ -68,14 +80,7 @@ describe("createSlackMessageHandler", () => { }); it("does not track duplicate messages that are already seen", async () => { - const trackEvent = vi.fn(); - const handler = createSlackMessageHandler({ - ctx: createContext({ markMessageSeen: () => true }), - account: { accountId: "default" } as Parameters< - typeof createSlackMessageHandler - >[0]["account"], - trackEvent, - }); + const { handler, trackEvent } = createHandlerWithTracker({ markMessageSeen: () => true }); await handler( { @@ -93,14 +98,7 @@ describe("createSlackMessageHandler", () => { }); it("tracks accepted non-duplicate messages", async () => { - const trackEvent = vi.fn(); - const handler = createSlackMessageHandler({ - ctx: createContext(), - account: { accountId: "default" } as Parameters< - typeof createSlackMessageHandler - >[0]["account"], - trackEvent, - }); + const { handler, trackEvent } = createHandlerWithTracker(); await handler( { diff --git a/src/slack/monitor/message-handler/prepare.test-helpers.ts b/src/slack/monitor/message-handler/prepare.test-helpers.ts new file mode 100644 index 00000000000..c80ea4b6ace --- /dev/null +++ b/src/slack/monitor/message-handler/prepare.test-helpers.ts @@ -0,0 +1,68 @@ +import type { App } from "@slack/bolt"; +import type { OpenClawConfig } from "../../../config/config.js"; +import type { RuntimeEnv } from "../../../runtime.js"; +import type { ResolvedSlackAccount } from "../../accounts.js"; +import { createSlackMonitorContext } from "../context.js"; + +export function createInboundSlackTestContext(params: { + cfg: OpenClawConfig; + appClient?: App["client"]; + defaultRequireMention?: boolean; + replyToMode?: "off" | "all" | "first"; + channelsConfig?: Record; +}) { + return createSlackMonitorContext({ + cfg: params.cfg, + accountId: "default", + botToken: "token", + app: { client: params.appClient ?? {} } as App, + runtime: {} as RuntimeEnv, + botUserId: "B1", + teamId: "T1", + apiAppId: "A1", + historyLimit: 0, + sessionScope: "per-sender", + mainKey: "main", + dmEnabled: true, + dmPolicy: "open", + allowFrom: [], + allowNameMatching: false, + groupDmEnabled: true, + groupDmChannels: [], + defaultRequireMention: params.defaultRequireMention ?? true, + channelsConfig: params.channelsConfig, + groupPolicy: "open", + useAccessGroups: false, + reactionMode: "off", + reactionAllowlist: [], + replyToMode: params.replyToMode ?? "off", + threadHistoryScope: "thread", + threadInheritParent: false, + slashCommand: { + enabled: false, + name: "openclaw", + sessionPrefix: "slack:slash", + ephemeral: true, + }, + textLimit: 4000, + ackReactionScope: "group-mentions", + mediaMaxBytes: 1024, + removeAckAfterReply: false, + }); +} + +export function createSlackTestAccount( + config: ResolvedSlackAccount["config"] = {}, +): ResolvedSlackAccount { + return { + accountId: "default", + enabled: true, + botTokenSource: "config", + appTokenSource: "config", + userTokenSource: "none", + config, + replyToMode: config.replyToMode, + replyToModeByChatType: config.replyToModeByChatType, + dm: config.dm, + }; +} diff --git a/src/slack/monitor/message-handler/prepare.thread-session-key.test.ts b/src/slack/monitor/message-handler/prepare.thread-session-key.test.ts index db2e2e6b5ab..5383311301d 100644 --- a/src/slack/monitor/message-handler/prepare.thread-session-key.test.ts +++ b/src/slack/monitor/message-handler/prepare.thread-session-key.test.ts @@ -1,64 +1,26 @@ import type { App } from "@slack/bolt"; import { describe, expect, it } from "vitest"; import type { OpenClawConfig } from "../../../config/config.js"; -import type { RuntimeEnv } from "../../../runtime.js"; import type { ResolvedSlackAccount } from "../../accounts.js"; import type { SlackMessageEvent } from "../../types.js"; -import { createSlackMonitorContext } from "../context.js"; import { prepareSlackMessage } from "./prepare.js"; +import { createInboundSlackTestContext, createSlackTestAccount } from "./prepare.test-helpers.js"; function buildCtx(overrides?: { replyToMode?: "all" | "first" | "off" }) { - return createSlackMonitorContext({ + const replyToMode = overrides?.replyToMode ?? "all"; + return createInboundSlackTestContext({ cfg: { channels: { - slack: { enabled: true, replyToMode: overrides?.replyToMode ?? "all" }, + slack: { enabled: true, replyToMode }, }, } as OpenClawConfig, - accountId: "default", - botToken: "token", - app: { client: {} } as App, - runtime: {} as RuntimeEnv, - botUserId: "B1", - teamId: "T1", - apiAppId: "A1", - historyLimit: 0, - sessionScope: "per-sender", - mainKey: "main", - dmEnabled: true, - dmPolicy: "open", - allowFrom: [], - groupDmEnabled: true, - groupDmChannels: [], + appClient: {} as App["client"], defaultRequireMention: false, - groupPolicy: "open", - allowNameMatching: false, - useAccessGroups: false, - reactionMode: "off", - reactionAllowlist: [], - replyToMode: overrides?.replyToMode ?? "all", - threadHistoryScope: "thread", - threadInheritParent: false, - slashCommand: { - enabled: false, - name: "openclaw", - sessionPrefix: "slack:slash", - ephemeral: true, - }, - textLimit: 4000, - ackReactionScope: "group-mentions", - mediaMaxBytes: 1024, - removeAckAfterReply: false, + replyToMode, }); } -const account: ResolvedSlackAccount = { - accountId: "default", - enabled: true, - botTokenSource: "config", - appTokenSource: "config", - userTokenSource: "none", - config: {}, -}; +const account: ResolvedSlackAccount = createSlackTestAccount(); describe("thread-level session keys", () => { it("uses thread-level session key for channel messages", async () => { diff --git a/src/slack/monitor/slash.ts b/src/slack/monitor/slash.ts index 104db52ec56..dcd379da680 100644 --- a/src/slack/monitor/slash.ts +++ b/src/slack/monitor/slash.ts @@ -510,11 +510,11 @@ export async function registerSlackMonitorSlashCommands(params: { const [ { resolveConversationLabel }, { createReplyPrefixOptions }, - { recordSessionMetaFromInbound, resolveStorePath }, + { recordInboundSessionMetaSafe }, ] = await Promise.all([ import("../../channels/conversation-label.js"), import("../../channels/reply-prefix.js"), - import("../../config/sessions.js"), + import("../../channels/session-meta.js"), ]); const route = resolveAgentRoute({ @@ -578,18 +578,14 @@ export async function registerSlackMonitorSlashCommands(params: { OriginatingTo: `user:${command.user_id}`, }); - const storePath = resolveStorePath(cfg.session?.store, { + await recordInboundSessionMetaSafe({ + cfg, agentId: route.agentId, + sessionKey: ctxPayload.SessionKey ?? route.sessionKey, + ctx: ctxPayload, + onError: (err) => + runtime.error?.(danger(`slack slash: failed updating session meta: ${String(err)}`)), }); - try { - await recordSessionMetaFromInbound({ - storePath, - sessionKey: ctxPayload.SessionKey ?? route.sessionKey, - ctx: ctxPayload, - }); - } catch (err) { - runtime.error?.(danger(`slack slash: failed updating session meta: ${String(err)}`)); - } const { onModelSelected, ...prefixOptions } = createReplyPrefixOptions({ cfg, diff --git a/src/slack/targets.ts b/src/slack/targets.ts index d12bc605ec4..e6bc69d8d24 100644 --- a/src/slack/targets.ts +++ b/src/slack/targets.ts @@ -1,8 +1,7 @@ import { buildMessagingTarget, ensureTargetId, - parseTargetMention, - parseTargetPrefixes, + parseMentionPrefixOrAtUserTarget, requireTargetKind, type MessagingTarget, type MessagingTargetKind, @@ -23,33 +22,19 @@ export function parseSlackTarget( if (!trimmed) { return undefined; } - const mentionTarget = parseTargetMention({ + const userTarget = parseMentionPrefixOrAtUserTarget({ raw: trimmed, mentionPattern: /^<@([A-Z0-9]+)>$/i, - kind: "user", - }); - if (mentionTarget) { - return mentionTarget; - } - const prefixedTarget = parseTargetPrefixes({ - raw: trimmed, prefixes: [ { prefix: "user:", kind: "user" }, { prefix: "channel:", kind: "channel" }, { prefix: "slack:", kind: "user" }, ], + atUserPattern: /^[A-Z0-9]+$/i, + atUserErrorMessage: "Slack DMs require a user id (use user: or <@id>)", }); - if (prefixedTarget) { - return prefixedTarget; - } - if (trimmed.startsWith("@")) { - const candidate = trimmed.slice(1).trim(); - const id = ensureTargetId({ - candidate, - pattern: /^[A-Z0-9]+$/i, - errorMessage: "Slack DMs require a user id (use user: or <@id>)", - }); - return buildMessagingTarget("user", id, trimmed); + if (userTarget) { + return userTarget; } if (trimmed.startsWith("#")) { const candidate = trimmed.slice(1).trim(); diff --git a/src/slack/threading-tool-context.test.ts b/src/slack/threading-tool-context.test.ts index c2054f1039c..c4be6ef2d77 100644 --- a/src/slack/threading-tool-context.test.ts +++ b/src/slack/threading-tool-context.test.ts @@ -4,6 +4,23 @@ import { buildSlackThreadingToolContext } from "./threading-tool-context.js"; const emptyCfg = {} as OpenClawConfig; +function resolveReplyToModeWithConfig(params: { + slackConfig: Record; + context: Record; +}) { + const cfg = { + channels: { + slack: params.slackConfig, + }, + } as OpenClawConfig; + const result = buildSlackThreadingToolContext({ + cfg, + accountId: null, + context: params.context as never, + }); + return result.replyToMode; +} + describe("buildSlackThreadingToolContext", () => { it("uses top-level replyToMode by default", () => { const cfg = { @@ -20,37 +37,27 @@ describe("buildSlackThreadingToolContext", () => { }); it("uses chat-type replyToMode overrides for direct messages when configured", () => { - const cfg = { - channels: { - slack: { + expect( + resolveReplyToModeWithConfig({ + slackConfig: { replyToMode: "off", replyToModeByChatType: { direct: "all" }, }, - }, - } as OpenClawConfig; - const result = buildSlackThreadingToolContext({ - cfg, - accountId: null, - context: { ChatType: "direct" }, - }); - expect(result.replyToMode).toBe("all"); + context: { ChatType: "direct" }, + }), + ).toBe("all"); }); it("uses top-level replyToMode for channels when no channel override is set", () => { - const cfg = { - channels: { - slack: { + expect( + resolveReplyToModeWithConfig({ + slackConfig: { replyToMode: "off", replyToModeByChatType: { direct: "all" }, }, - }, - } as OpenClawConfig; - const result = buildSlackThreadingToolContext({ - cfg, - accountId: null, - context: { ChatType: "channel" }, - }); - expect(result.replyToMode).toBe("off"); + context: { ChatType: "channel" }, + }), + ).toBe("off"); }); it("falls back to top-level when no chat-type override is set", () => { @@ -70,61 +77,46 @@ describe("buildSlackThreadingToolContext", () => { }); it("uses legacy dm.replyToMode for direct messages when no chat-type override exists", () => { - const cfg = { - channels: { - slack: { + expect( + resolveReplyToModeWithConfig({ + slackConfig: { replyToMode: "off", dm: { replyToMode: "all" }, }, - }, - } as OpenClawConfig; - const result = buildSlackThreadingToolContext({ - cfg, - accountId: null, - context: { ChatType: "direct" }, - }); - expect(result.replyToMode).toBe("all"); + context: { ChatType: "direct" }, + }), + ).toBe("all"); }); it("uses all mode when MessageThreadId is present", () => { - const cfg = { - channels: { - slack: { + expect( + resolveReplyToModeWithConfig({ + slackConfig: { replyToMode: "all", replyToModeByChatType: { direct: "off" }, }, - }, - } as OpenClawConfig; - const result = buildSlackThreadingToolContext({ - cfg, - accountId: null, - context: { - ChatType: "direct", - ThreadLabel: "thread-label", - MessageThreadId: "1771999998.834199", - }, - }); - expect(result.replyToMode).toBe("all"); + context: { + ChatType: "direct", + ThreadLabel: "thread-label", + MessageThreadId: "1771999998.834199", + }, + }), + ).toBe("all"); }); it("does not force all mode from ThreadLabel alone", () => { - const cfg = { - channels: { - slack: { + expect( + resolveReplyToModeWithConfig({ + slackConfig: { replyToMode: "all", replyToModeByChatType: { direct: "off" }, }, - }, - } as OpenClawConfig; - const result = buildSlackThreadingToolContext({ - cfg, - accountId: null, - context: { - ChatType: "direct", - ThreadLabel: "label-without-real-thread", - }, - }); - expect(result.replyToMode).toBe("off"); + context: { + ChatType: "direct", + ThreadLabel: "label-without-real-thread", + }, + }), + ).toBe("off"); }); it("keeps configured channel behavior when not in a thread", () => { diff --git a/src/slack/threading.test.ts b/src/slack/threading.test.ts index cc519683fb5..dc98f767966 100644 --- a/src/slack/threading.test.ts +++ b/src/slack/threading.test.ts @@ -2,6 +2,22 @@ import { describe, expect, it } from "vitest"; import { resolveSlackThreadContext, resolveSlackThreadTargets } from "./threading.js"; describe("resolveSlackThreadTargets", () => { + function expectAutoCreatedTopLevelThreadTsBehavior(replyToMode: "off" | "first") { + const { replyThreadTs, statusThreadTs, isThreadReply } = resolveSlackThreadTargets({ + replyToMode, + message: { + type: "message", + channel: "C1", + ts: "123", + thread_ts: "123", + }, + }); + + expect(isThreadReply).toBe(false); + expect(replyThreadTs).toBeUndefined(); + expect(statusThreadTs).toBeUndefined(); + } + it("threads replies when message is already threaded", () => { const { replyThreadTs, statusThreadTs } = resolveSlackThreadTargets({ replyToMode: "off", @@ -46,35 +62,11 @@ describe("resolveSlackThreadTargets", () => { }); it("does not treat auto-created top-level thread_ts as a real thread when mode is off", () => { - const { replyThreadTs, statusThreadTs, isThreadReply } = resolveSlackThreadTargets({ - replyToMode: "off", - message: { - type: "message", - channel: "C1", - ts: "123", - thread_ts: "123", - }, - }); - - expect(isThreadReply).toBe(false); - expect(replyThreadTs).toBeUndefined(); - expect(statusThreadTs).toBeUndefined(); + expectAutoCreatedTopLevelThreadTsBehavior("off"); }); it("keeps first-mode behavior for auto-created top-level thread_ts", () => { - const { replyThreadTs, statusThreadTs, isThreadReply } = resolveSlackThreadTargets({ - replyToMode: "first", - message: { - type: "message", - channel: "C1", - ts: "123", - thread_ts: "123", - }, - }); - - expect(isThreadReply).toBe(false); - expect(replyThreadTs).toBeUndefined(); - expect(statusThreadTs).toBeUndefined(); + expectAutoCreatedTopLevelThreadTsBehavior("first"); }); it("sets messageThreadId for top-level messages when replyToMode is all", () => { diff --git a/src/telegram/accounts.test.ts b/src/telegram/accounts.test.ts index 6c7f350ca43..b53c9ef6ded 100644 --- a/src/telegram/accounts.test.ts +++ b/src/telegram/accounts.test.ts @@ -215,6 +215,18 @@ describe("resolveTelegramAccount allowFrom precedence", () => { }); describe("resolveTelegramAccount groups inheritance (#30673)", () => { + const createMultiAccountGroupsConfig = (): OpenClawConfig => ({ + channels: { + telegram: { + groups: { "-100123": { requireMention: false } }, + accounts: { + default: { botToken: "123:default" }, + dev: { botToken: "456:dev" }, + }, + }, + }, + }); + it("inherits channel-level groups in single-account setup", () => { const resolved = resolveTelegramAccount({ cfg: { @@ -235,17 +247,7 @@ describe("resolveTelegramAccount groups inheritance (#30673)", () => { it("does NOT inherit channel-level groups to secondary account in multi-account setup", () => { const resolved = resolveTelegramAccount({ - cfg: { - channels: { - telegram: { - groups: { "-100123": { requireMention: false } }, - accounts: { - default: { botToken: "123:default" }, - dev: { botToken: "456:dev" }, - }, - }, - }, - }, + cfg: createMultiAccountGroupsConfig(), accountId: "dev", }); @@ -254,17 +256,7 @@ describe("resolveTelegramAccount groups inheritance (#30673)", () => { it("does NOT inherit channel-level groups to default account in multi-account setup", () => { const resolved = resolveTelegramAccount({ - cfg: { - channels: { - telegram: { - groups: { "-100123": { requireMention: false } }, - accounts: { - default: { botToken: "123:default" }, - dev: { botToken: "456:dev" }, - }, - }, - }, - }, + cfg: createMultiAccountGroupsConfig(), accountId: "default", }); diff --git a/src/telegram/bot-native-commands.ts b/src/telegram/bot-native-commands.ts index 0fd97d9dfe5..071a0c1fa37 100644 --- a/src/telegram/bot-native-commands.ts +++ b/src/telegram/bot-native-commands.ts @@ -14,10 +14,10 @@ import { dispatchReplyWithBufferedBlockDispatcher } from "../auto-reply/reply/pr import { listSkillCommandsForAgents } from "../auto-reply/skill-commands.js"; import { resolveCommandAuthorizedFromAuthorizers } from "../channels/command-gating.js"; import { createReplyPrefixOptions } from "../channels/reply-prefix.js"; +import { recordInboundSessionMetaSafe } from "../channels/session-meta.js"; import type { OpenClawConfig } from "../config/config.js"; import type { ChannelGroupPolicy } from "../config/group-policy.js"; import { resolveMarkdownTableMode } from "../config/markdown-tables.js"; -import { recordSessionMetaFromInbound, resolveStorePath } from "../config/sessions.js"; import { normalizeTelegramCommandName, resolveTelegramCustomCommands, @@ -618,18 +618,16 @@ export const registerTelegramNativeCommands = ({ OriginatingTo: `telegram:${chatId}`, }); - const storePath = resolveStorePath(cfg.session?.store, { + await recordInboundSessionMetaSafe({ + cfg, agentId: route.agentId, + sessionKey: ctxPayload.SessionKey ?? route.sessionKey, + ctx: ctxPayload, + onError: (err) => + runtime.error?.( + danger(`telegram slash: failed updating session meta: ${String(err)}`), + ), }); - try { - await recordSessionMetaFromInbound({ - storePath, - sessionKey: ctxPayload.SessionKey ?? route.sessionKey, - ctx: ctxPayload, - }); - } catch (err) { - runtime.error?.(danger(`telegram slash: failed updating session meta: ${String(err)}`)); - } const disableBlockStreaming = typeof telegramCfg.blockStreaming === "boolean" diff --git a/src/telegram/fetch.test.ts b/src/telegram/fetch.test.ts index 90da589f882..7019b4cb513 100644 --- a/src/telegram/fetch.test.ts +++ b/src/telegram/fetch.test.ts @@ -37,6 +37,15 @@ vi.mock("undici", () => ({ const originalFetch = globalThis.fetch; +function expectEnvProxyAgentConstructorCall(params: { nth: number; autoSelectFamily: boolean }) { + expect(EnvHttpProxyAgentCtor).toHaveBeenNthCalledWith(params.nth, { + connect: { + autoSelectFamily: params.autoSelectFamily, + autoSelectFamilyAttemptTimeout: 300, + }, + }); +} + afterEach(() => { resetTelegramFetchStateForTests(); setDefaultAutoSelectFamily.mockReset(); @@ -157,12 +166,7 @@ describe("resolveTelegramFetch", () => { resolveTelegramFetch(undefined, { network: { autoSelectFamily: true } }); expect(setGlobalDispatcher).toHaveBeenCalledTimes(1); - expect(EnvHttpProxyAgentCtor).toHaveBeenCalledWith({ - connect: { - autoSelectFamily: true, - autoSelectFamilyAttemptTimeout: 300, - }, - }); + expectEnvProxyAgentConstructorCall({ nth: 1, autoSelectFamily: true }); }); it("keeps an existing proxy-like global dispatcher", async () => { @@ -204,18 +208,8 @@ describe("resolveTelegramFetch", () => { resolveTelegramFetch(undefined, { network: { autoSelectFamily: false } }); expect(setGlobalDispatcher).toHaveBeenCalledTimes(2); - expect(EnvHttpProxyAgentCtor).toHaveBeenNthCalledWith(1, { - connect: { - autoSelectFamily: true, - autoSelectFamilyAttemptTimeout: 300, - }, - }); - expect(EnvHttpProxyAgentCtor).toHaveBeenNthCalledWith(2, { - connect: { - autoSelectFamily: false, - autoSelectFamilyAttemptTimeout: 300, - }, - }); + expectEnvProxyAgentConstructorCall({ nth: 1, autoSelectFamily: true }); + expectEnvProxyAgentConstructorCall({ nth: 2, autoSelectFamily: false }); }); it("retries once with ipv4 fallback when fetch fails with network timeout/unreachable", async () => { @@ -248,18 +242,8 @@ describe("resolveTelegramFetch", () => { expect(fetchMock).toHaveBeenCalledTimes(2); expect(setGlobalDispatcher).toHaveBeenCalledTimes(2); - expect(EnvHttpProxyAgentCtor).toHaveBeenNthCalledWith(1, { - connect: { - autoSelectFamily: true, - autoSelectFamilyAttemptTimeout: 300, - }, - }); - expect(EnvHttpProxyAgentCtor).toHaveBeenNthCalledWith(2, { - connect: { - autoSelectFamily: false, - autoSelectFamilyAttemptTimeout: 300, - }, - }); + expectEnvProxyAgentConstructorCall({ nth: 1, autoSelectFamily: true }); + expectEnvProxyAgentConstructorCall({ nth: 2, autoSelectFamily: false }); }); it("retries with ipv4 fallback once per request, not once per process", async () => { diff --git a/src/telegram/group-access.policy-access.test.ts b/src/telegram/group-access.policy-access.test.ts index 5edb85c15a6..5683732476c 100644 --- a/src/telegram/group-access.policy-access.test.ts +++ b/src/telegram/group-access.policy-access.test.ts @@ -22,29 +22,48 @@ const senderAllow = { invalidEntries: [], }; +type GroupAccessParams = Parameters[0]; + +const DEFAULT_GROUP_ACCESS_PARAMS: GroupAccessParams = { + isGroup: true, + chatId: "-100123456", + cfg: baseCfg, + telegramCfg: baseTelegramCfg, + effectiveGroupAllow: emptyAllow, + senderId: "999", + senderUsername: "user", + resolveGroupPolicy: () => ({ + allowlistEnabled: true, + allowed: true, + groupConfig: { requireMention: false }, + }), + enforcePolicy: true, + useTopicAndGroupOverrides: false, + enforceAllowlistAuthorization: true, + allowEmptyAllowlistEntries: false, + requireSenderForAllowlistAuthorization: true, + checkChatAllowlist: true, +}; + +function runAccess(overrides: Partial) { + return evaluateTelegramGroupPolicyAccess({ + ...DEFAULT_GROUP_ACCESS_PARAMS, + ...overrides, + resolveGroupPolicy: + overrides.resolveGroupPolicy ?? DEFAULT_GROUP_ACCESS_PARAMS.resolveGroupPolicy, + }); +} + describe("evaluateTelegramGroupPolicyAccess – chat allowlist vs sender allowlist ordering", () => { it("allows a group explicitly listed in groups config even when no allowFrom entries exist", () => { // Issue #30613: a group configured with a dedicated entry (groupConfig set) // should be allowed even without any allowFrom / groupAllowFrom entries. - const result = evaluateTelegramGroupPolicyAccess({ - isGroup: true, - chatId: "-100123456", - cfg: baseCfg, - telegramCfg: baseTelegramCfg, - effectiveGroupAllow: emptyAllow, - senderId: "999", - senderUsername: "user", + const result = runAccess({ resolveGroupPolicy: () => ({ allowlistEnabled: true, allowed: true, groupConfig: { requireMention: false }, // dedicated entry — not just wildcard }), - enforcePolicy: true, - useTopicAndGroupOverrides: false, - enforceAllowlistAuthorization: true, - allowEmptyAllowlistEntries: false, - requireSenderForAllowlistAuthorization: true, - checkChatAllowlist: true, }); expect(result).toEqual({ allowed: true, groupPolicy: "allowlist" }); @@ -52,25 +71,12 @@ describe("evaluateTelegramGroupPolicyAccess – chat allowlist vs sender allowli it("still blocks when only wildcard match and no allowFrom entries", () => { // groups: { "*": ... } with no allowFrom → wildcard does NOT bypass sender checks. - const result = evaluateTelegramGroupPolicyAccess({ - isGroup: true, - chatId: "-100123456", - cfg: baseCfg, - telegramCfg: baseTelegramCfg, - effectiveGroupAllow: emptyAllow, - senderId: "999", - senderUsername: "user", + const result = runAccess({ resolveGroupPolicy: () => ({ allowlistEnabled: true, allowed: true, groupConfig: undefined, // wildcard match only — no dedicated entry }), - enforcePolicy: true, - useTopicAndGroupOverrides: false, - enforceAllowlistAuthorization: true, - allowEmptyAllowlistEntries: false, - requireSenderForAllowlistAuthorization: true, - checkChatAllowlist: true, }); expect(result).toEqual({ @@ -81,24 +87,12 @@ describe("evaluateTelegramGroupPolicyAccess – chat allowlist vs sender allowli }); it("rejects a group NOT in groups config", () => { - const result = evaluateTelegramGroupPolicyAccess({ - isGroup: true, + const result = runAccess({ chatId: "-100999999", - cfg: baseCfg, - telegramCfg: baseTelegramCfg, - effectiveGroupAllow: emptyAllow, - senderId: "999", - senderUsername: "user", resolveGroupPolicy: () => ({ allowlistEnabled: true, allowed: false, }), - enforcePolicy: true, - useTopicAndGroupOverrides: false, - enforceAllowlistAuthorization: true, - allowEmptyAllowlistEntries: false, - requireSenderForAllowlistAuthorization: true, - checkChatAllowlist: true, }); expect(result).toEqual({ @@ -109,24 +103,12 @@ describe("evaluateTelegramGroupPolicyAccess – chat allowlist vs sender allowli }); it("still enforces sender allowlist when checkChatAllowlist is disabled", () => { - const result = evaluateTelegramGroupPolicyAccess({ - isGroup: true, - chatId: "-100123456", - cfg: baseCfg, - telegramCfg: baseTelegramCfg, - effectiveGroupAllow: emptyAllow, - senderId: "999", - senderUsername: "user", + const result = runAccess({ resolveGroupPolicy: () => ({ allowlistEnabled: true, allowed: true, groupConfig: { requireMention: false }, }), - enforcePolicy: true, - useTopicAndGroupOverrides: false, - enforceAllowlistAuthorization: true, - allowEmptyAllowlistEntries: false, - requireSenderForAllowlistAuthorization: true, checkChatAllowlist: false, }); @@ -138,11 +120,7 @@ describe("evaluateTelegramGroupPolicyAccess – chat allowlist vs sender allowli }); it("blocks unauthorized sender even when chat is explicitly allowed and sender entries exist", () => { - const result = evaluateTelegramGroupPolicyAccess({ - isGroup: true, - chatId: "-100123456", - cfg: baseCfg, - telegramCfg: baseTelegramCfg, + const result = runAccess({ effectiveGroupAllow: senderAllow, // entries: ["111"] senderId: "222", // not in senderAllow.entries senderUsername: "other", @@ -151,12 +129,6 @@ describe("evaluateTelegramGroupPolicyAccess – chat allowlist vs sender allowli allowed: true, groupConfig: { requireMention: false }, }), - enforcePolicy: true, - useTopicAndGroupOverrides: false, - enforceAllowlistAuthorization: true, - allowEmptyAllowlistEntries: false, - requireSenderForAllowlistAuthorization: true, - checkChatAllowlist: true, }); // Chat is explicitly allowed, but sender entries exist and sender is not in them. @@ -168,48 +140,24 @@ describe("evaluateTelegramGroupPolicyAccess – chat allowlist vs sender allowli }); it("allows when groupPolicy is open regardless of allowlist state", () => { - const result = evaluateTelegramGroupPolicyAccess({ - isGroup: true, - chatId: "-100123456", - cfg: baseCfg, + const result = runAccess({ telegramCfg: { groupPolicy: "open" } as unknown as TelegramAccountConfig, - effectiveGroupAllow: emptyAllow, - senderId: "999", - senderUsername: "user", resolveGroupPolicy: () => ({ allowlistEnabled: false, allowed: false, }), - enforcePolicy: true, - useTopicAndGroupOverrides: false, - enforceAllowlistAuthorization: true, - allowEmptyAllowlistEntries: false, - requireSenderForAllowlistAuthorization: true, - checkChatAllowlist: true, }); expect(result).toEqual({ allowed: true, groupPolicy: "open" }); }); it("rejects when groupPolicy is disabled", () => { - const result = evaluateTelegramGroupPolicyAccess({ - isGroup: true, - chatId: "-100123456", - cfg: baseCfg, + const result = runAccess({ telegramCfg: { groupPolicy: "disabled" } as unknown as TelegramAccountConfig, - effectiveGroupAllow: emptyAllow, - senderId: "999", - senderUsername: "user", resolveGroupPolicy: () => ({ allowlistEnabled: false, allowed: false, }), - enforcePolicy: true, - useTopicAndGroupOverrides: false, - enforceAllowlistAuthorization: true, - allowEmptyAllowlistEntries: false, - requireSenderForAllowlistAuthorization: true, - checkChatAllowlist: true, }); expect(result).toEqual({ @@ -220,49 +168,27 @@ describe("evaluateTelegramGroupPolicyAccess – chat allowlist vs sender allowli }); it("allows non-group messages without any checks", () => { - const result = evaluateTelegramGroupPolicyAccess({ + const result = runAccess({ isGroup: false, chatId: "12345", - cfg: baseCfg, - telegramCfg: baseTelegramCfg, - effectiveGroupAllow: emptyAllow, - senderId: "999", - senderUsername: "user", resolveGroupPolicy: () => ({ allowlistEnabled: true, allowed: false, }), - enforcePolicy: true, - useTopicAndGroupOverrides: false, - enforceAllowlistAuthorization: true, - allowEmptyAllowlistEntries: false, - requireSenderForAllowlistAuthorization: true, - checkChatAllowlist: true, }); expect(result).toEqual({ allowed: true, groupPolicy: "allowlist" }); }); it("allows authorized sender in wildcard-matched group with sender entries", () => { - const result = evaluateTelegramGroupPolicyAccess({ - isGroup: true, - chatId: "-100123456", - cfg: baseCfg, - telegramCfg: baseTelegramCfg, + const result = runAccess({ effectiveGroupAllow: senderAllow, // entries: ["111"] senderId: "111", // IS in senderAllow.entries - senderUsername: "user", resolveGroupPolicy: () => ({ allowlistEnabled: true, allowed: true, groupConfig: undefined, // wildcard only }), - enforcePolicy: true, - useTopicAndGroupOverrides: false, - enforceAllowlistAuthorization: true, - allowEmptyAllowlistEntries: false, - requireSenderForAllowlistAuthorization: true, - checkChatAllowlist: true, }); expect(result).toEqual({ allowed: true, groupPolicy: "allowlist" }); diff --git a/src/telegram/lane-delivery.ts b/src/telegram/lane-delivery.ts index b334c6ded41..5337badbacc 100644 --- a/src/telegram/lane-delivery.ts +++ b/src/telegram/lane-delivery.ts @@ -183,6 +183,23 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { lane, treatEditFailureAsDelivered, }); + const finalizePreview = ( + previewMessageId: number, + treatEditFailureAsDelivered: boolean, + ): boolean | Promise => { + const currentPreviewText = previewTextSnapshot ?? getLanePreviewText(lane); + const shouldSkipRegressive = shouldSkipRegressivePreviewUpdate({ + currentPreviewText, + text, + skipRegressive, + hadPreviewMessage, + }); + if (shouldSkipRegressive) { + params.markDelivered(); + return true; + } + return editPreview(previewMessageId, treatEditFailureAsDelivered); + }; if (!lane.stream) { return false; } @@ -199,18 +216,7 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { if (typeof previewMessageId !== "number") { return false; } - const currentPreviewText = previewTextSnapshot ?? getLanePreviewText(lane); - const shouldSkipRegressive = shouldSkipRegressivePreviewUpdate({ - currentPreviewText, - text, - skipRegressive, - hadPreviewMessage, - }); - if (shouldSkipRegressive) { - params.markDelivered(); - return true; - } - return editPreview(previewMessageId, true); + return finalizePreview(previewMessageId, true); } if (stopBeforeEdit) { await params.stopDraftLane(lane); @@ -222,18 +228,7 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { if (typeof previewMessageId !== "number") { return false; } - const currentPreviewText = previewTextSnapshot ?? getLanePreviewText(lane); - const shouldSkipRegressive = shouldSkipRegressivePreviewUpdate({ - currentPreviewText, - text, - skipRegressive, - hadPreviewMessage, - }); - if (shouldSkipRegressive) { - params.markDelivered(); - return true; - } - return editPreview(previewMessageId, false); + return finalizePreview(previewMessageId, false); }; const consumeArchivedAnswerPreviewForFinal = async ({ diff --git a/src/telegram/network-errors.ts b/src/telegram/network-errors.ts index 177ef00d646..f9b7061dd61 100644 --- a/src/telegram/network-errors.ts +++ b/src/telegram/network-errors.ts @@ -1,4 +1,9 @@ -import { extractErrorCode, formatErrorMessage } from "../infra/errors.js"; +import { + collectErrorGraphCandidates, + extractErrorCode, + formatErrorMessage, + readErrorName, +} from "../infra/errors.js"; const RECOVERABLE_ERROR_CODES = new Set([ "ECONNRESET", @@ -44,13 +49,6 @@ function normalizeCode(code?: string): string { return code?.trim().toUpperCase() ?? ""; } -function getErrorName(err: unknown): string { - if (!err || typeof err !== "object") { - return ""; - } - return "name" in err ? String(err.name) : ""; -} - function getErrorCode(err: unknown): string | undefined { const direct = extractErrorCode(err); if (direct) { @@ -69,50 +67,6 @@ function getErrorCode(err: unknown): string | undefined { return undefined; } -function collectErrorCandidates(err: unknown): unknown[] { - const queue = [err]; - const seen = new Set(); - const candidates: unknown[] = []; - - while (queue.length > 0) { - const current = queue.shift(); - if (current == null || seen.has(current)) { - continue; - } - seen.add(current); - candidates.push(current); - - if (typeof current === "object") { - const cause = (current as { cause?: unknown }).cause; - if (cause && !seen.has(cause)) { - queue.push(cause); - } - const reason = (current as { reason?: unknown }).reason; - if (reason && !seen.has(reason)) { - queue.push(reason); - } - const errors = (current as { errors?: unknown }).errors; - if (Array.isArray(errors)) { - for (const nested of errors) { - if (nested && !seen.has(nested)) { - queue.push(nested); - } - } - } - // Grammy's HttpError wraps the underlying error in .error (not .cause) - // Only follow .error for HttpError to avoid widening the search graph - if (getErrorName(current) === "HttpError") { - const wrappedError = (current as { error?: unknown }).error; - if (wrappedError && !seen.has(wrappedError)) { - queue.push(wrappedError); - } - } - } - } - - return candidates; -} - export type TelegramNetworkErrorContext = "polling" | "send" | "webhook" | "unknown"; export function isRecoverableTelegramNetworkError( @@ -127,13 +81,23 @@ export function isRecoverableTelegramNetworkError( ? options.allowMessageMatch : options.context !== "send"; - for (const candidate of collectErrorCandidates(err)) { + for (const candidate of collectErrorGraphCandidates(err, (current) => { + const nested: Array = [current.cause, current.reason]; + if (Array.isArray(current.errors)) { + nested.push(...current.errors); + } + // Grammy's HttpError wraps the underlying error in .error (not .cause). + if (readErrorName(current) === "HttpError") { + nested.push(current.error); + } + return nested; + })) { const code = normalizeCode(getErrorCode(candidate)); if (code && RECOVERABLE_ERROR_CODES.has(code)) { return true; } - const name = getErrorName(candidate); + const name = readErrorName(candidate); if (name && RECOVERABLE_ERROR_NAMES.has(name)) { return true; } diff --git a/src/terminal/restore.test.ts b/src/terminal/restore.test.ts index deaa8e74c0a..8fbd0560073 100644 --- a/src/terminal/restore.test.ts +++ b/src/terminal/restore.test.ts @@ -22,6 +22,20 @@ function configureTerminalIO(params: { (process.stdin as { isPaused?: () => boolean }).isPaused = params.isPaused; } +function setupPausedTTYStdin() { + const setRawMode = vi.fn(); + const resume = vi.fn(); + const isPaused = vi.fn(() => true); + configureTerminalIO({ + stdinIsTTY: true, + stdoutIsTTY: false, + setRawMode, + resume, + isPaused, + }); + return { setRawMode, resume }; +} + describe("restoreTerminalState", () => { const originalStdinIsTTY = process.stdin.isTTY; const originalStdoutIsTTY = process.stdout.isTTY; @@ -45,17 +59,7 @@ describe("restoreTerminalState", () => { }); it("does not resume paused stdin by default", () => { - const setRawMode = vi.fn(); - const resume = vi.fn(); - const isPaused = vi.fn(() => true); - - configureTerminalIO({ - stdinIsTTY: true, - stdoutIsTTY: false, - setRawMode, - resume, - isPaused, - }); + const { setRawMode, resume } = setupPausedTTYStdin(); restoreTerminalState("test"); @@ -64,17 +68,7 @@ describe("restoreTerminalState", () => { }); it("resumes paused stdin when resumeStdin is true", () => { - const setRawMode = vi.fn(); - const resume = vi.fn(); - const isPaused = vi.fn(() => true); - - configureTerminalIO({ - stdinIsTTY: true, - stdoutIsTTY: false, - setRawMode, - resume, - isPaused, - }); + const { setRawMode, resume } = setupPausedTTYStdin(); restoreTerminalState("test", { resumeStdinIfPaused: true }); diff --git a/src/terminal/table.test.ts b/src/terminal/table.test.ts index f8b34516ca9..bb6f2082fe3 100644 --- a/src/terminal/table.test.ts +++ b/src/terminal/table.test.ts @@ -48,44 +48,13 @@ describe("renderTable", () => { ], }); - const ESC = "\u001b"; - for (let i = 0; i < out.length; i += 1) { - if (out[i] !== ESC) { - continue; - } - - // SGR: ESC [ ... m - if (out[i + 1] === "[") { - let j = i + 2; - while (j < out.length) { - const ch = out[j]; - if (ch === "m") { - break; - } - if (ch && ch >= "0" && ch <= "9") { - j += 1; - continue; - } - if (ch === ";") { - j += 1; - continue; - } - break; - } - expect(out[j]).toBe("m"); - i = j; - continue; - } - - // OSC-8: ESC ] 8 ; ; ... ST (ST = ESC \) - if (out[i + 1] === "]" && out.slice(i + 2, i + 5) === "8;;") { - const st = out.indexOf(`${ESC}\\`, i + 5); - expect(st).toBeGreaterThanOrEqual(0); - i = st + 1; - continue; - } - - throw new Error(`Unexpected escape sequence at index ${i}`); + const ansiToken = new RegExp(String.raw`\u001b\[[0-9;]*m|\u001b\]8;;.*?\u001b\\`, "gs"); + let escapeIndex = out.indexOf("\u001b"); + while (escapeIndex >= 0) { + ansiToken.lastIndex = escapeIndex; + const match = ansiToken.exec(out); + expect(match?.index).toBe(escapeIndex); + escapeIndex = out.indexOf("\u001b", escapeIndex + 1); } }); diff --git a/src/tts/tts.ts b/src/tts/tts.ts index c11cfaf1d87..bd3399732ad 100644 --- a/src/tts/tts.ts +++ b/src/tts/tts.ts @@ -532,6 +532,13 @@ function formatTtsProviderError(provider: TtsProvider, err: unknown): string { return `${provider}: ${error.message}`; } +function buildTtsFailureResult(errors: string[]): { success: false; error: string } { + return { + success: false, + error: `TTS conversion failed: ${errors.join("; ") || "no providers available"}`, + }; +} + export async function textToSpeech(params: { text: string; cfg: OpenClawConfig; @@ -696,10 +703,7 @@ export async function textToSpeech(params: { } } - return { - success: false, - error: `TTS conversion failed: ${errors.join("; ") || "no providers available"}`, - }; + return buildTtsFailureResult(errors); } export async function textToSpeechTelephony(params: { @@ -785,10 +789,7 @@ export async function textToSpeechTelephony(params: { } } - return { - success: false, - error: `TTS conversion failed: ${errors.join("; ") || "no providers available"}`, - }; + return buildTtsFailureResult(errors); } export async function maybeApplyTtsToPayload(params: { diff --git a/src/web/auto-reply/monitor/process-message.inbound-contract.test.ts b/src/web/auto-reply/monitor/process-message.inbound-contract.test.ts index 8458487d8e9..945b1c23973 100644 --- a/src/web/auto-reply/monitor/process-message.inbound-contract.test.ts +++ b/src/web/auto-reply/monitor/process-message.inbound-contract.test.ts @@ -61,6 +61,28 @@ function makeProcessMessageArgs(params: { } as any; } +function createWhatsAppDirectStreamingArgs(params?: { + rememberSentText?: (text: string | undefined, opts: unknown) => void; +}) { + return makeProcessMessageArgs({ + routeSessionKey: "agent:main:whatsapp:direct:+1555", + groupHistoryKey: "+1555", + rememberSentText: params?.rememberSentText, + cfg: { + channels: { whatsapp: { blockStreaming: true } }, + messages: {}, + session: { store: sessionStorePath }, + } as unknown as ReturnType, + msg: { + id: "msg1", + from: "+1555", + to: "+2000", + chatType: "direct", + body: "hi", + }, + }); +} + vi.mock("../../../auto-reply/reply/provider-dispatcher.js", () => ({ // oxlint-disable-next-line typescript/no-explicit-any dispatchReplyWithBufferedBlockDispatcher: vi.fn(async (params: any) => { @@ -243,25 +265,7 @@ describe("web processMessage inbound contract", () => { it("suppresses non-final WhatsApp payload delivery", async () => { const rememberSentText = vi.fn(); - await processMessage( - makeProcessMessageArgs({ - routeSessionKey: "agent:main:whatsapp:direct:+1555", - groupHistoryKey: "+1555", - rememberSentText, - cfg: { - channels: { whatsapp: { blockStreaming: true } }, - messages: {}, - session: { store: sessionStorePath }, - } as unknown as ReturnType, - msg: { - id: "msg1", - from: "+1555", - to: "+2000", - chatType: "direct", - body: "hi", - }, - }), - ); + await processMessage(createWhatsAppDirectStreamingArgs({ rememberSentText })); // oxlint-disable-next-line typescript/no-explicit-any const deliver = (capturedDispatchParams as any)?.dispatcherOptions?.deliver as @@ -280,24 +284,7 @@ describe("web processMessage inbound contract", () => { }); it("forces disableBlockStreaming for WhatsApp dispatch", async () => { - await processMessage( - makeProcessMessageArgs({ - routeSessionKey: "agent:main:whatsapp:direct:+1555", - groupHistoryKey: "+1555", - cfg: { - channels: { whatsapp: { blockStreaming: true } }, - messages: {}, - session: { store: sessionStorePath }, - } as unknown as ReturnType, - msg: { - id: "msg1", - from: "+1555", - to: "+2000", - chatType: "direct", - body: "hi", - }, - }), - ); + await processMessage(createWhatsAppDirectStreamingArgs()); // oxlint-disable-next-line typescript/no-explicit-any const replyOptions = (capturedDispatchParams as any)?.replyOptions; diff --git a/src/web/auto-reply/monitor/process-message.ts b/src/web/auto-reply/monitor/process-message.ts index 2e49e9c7989..93a12ff073a 100644 --- a/src/web/auto-reply/monitor/process-message.ts +++ b/src/web/auto-reply/monitor/process-message.ts @@ -1,10 +1,7 @@ import { resolveIdentityNamePrefix } from "../../../agents/identity.js"; import { resolveChunkMode, resolveTextChunkLimit } from "../../../auto-reply/chunk.js"; import { shouldComputeCommandAuthorized } from "../../../auto-reply/command-detection.js"; -import { - formatInboundEnvelope, - resolveEnvelopeFormatOptions, -} from "../../../auto-reply/envelope.js"; +import { formatInboundEnvelope } from "../../../auto-reply/envelope.js"; import type { getReplyFromConfig } from "../../../auto-reply/reply.js"; import { buildHistoryContextFromEntries, @@ -15,13 +12,10 @@ import { dispatchReplyWithBufferedBlockDispatcher } from "../../../auto-reply/re import type { ReplyPayload } from "../../../auto-reply/types.js"; import { toLocationContext } from "../../../channels/location.js"; import { createReplyPrefixOptions } from "../../../channels/reply-prefix.js"; +import { resolveInboundSessionEnvelopeContext } from "../../../channels/session-envelope.js"; import type { loadConfig } from "../../../config/config.js"; import { resolveMarkdownTableMode } from "../../../config/markdown-tables.js"; -import { - readSessionUpdatedAt, - recordSessionMetaFromInbound, - resolveStorePath, -} from "../../../config/sessions.js"; +import { recordSessionMetaFromInbound } from "../../../config/sessions.js"; import { logVerbose, shouldLogVerbose } from "../../../globals.js"; import type { getChildLogger } from "../../../logging.js"; import { getAgentScopedMediaLocalRoots } from "../../../media/local-roots.js"; @@ -142,12 +136,9 @@ export async function processMessage(params: { suppressGroupHistoryClear?: boolean; }) { const conversationId = params.msg.conversationId ?? params.msg.from; - const storePath = resolveStorePath(params.cfg.session?.store, { + const { storePath, envelopeOptions, previousTimestamp } = resolveInboundSessionEnvelopeContext({ + cfg: params.cfg, agentId: params.route.agentId, - }); - const envelopeOptions = resolveEnvelopeFormatOptions(params.cfg); - const previousTimestamp = readSessionUpdatedAt({ - storePath, sessionKey: params.route.sessionKey, }); let combinedBody = buildInboundLine({