diff --git a/src/auto-reply/reply/agent-runner-payloads.test.ts b/src/auto-reply/reply/agent-runner-payloads.test.ts index 26f23d7a42c..db237848e3c 100644 --- a/src/auto-reply/reply/agent-runner-payloads.test.ts +++ b/src/auto-reply/reply/agent-runner-payloads.test.ts @@ -9,6 +9,20 @@ const baseParams = { replyToMode: "off" as const, }; +async function expectSameTargetRepliesSuppressed(params: { provider: string; to: string }) { + const { replyPayloads } = await buildReplyPayloads({ + ...baseParams, + payloads: [{ text: "hello world!" }], + messageProvider: "heartbeat", + originatingChannel: "feishu", + originatingTo: "ou_abc123", + messagingToolSentTexts: ["different message"], + messagingToolSentTargets: [{ tool: "message", provider: params.provider, to: params.to }], + }); + + expect(replyPayloads).toHaveLength(0); +} + describe("buildReplyPayloads media filter integration", () => { it("strips media URL from payload when in messagingToolSentMediaUrls", async () => { const { replyPayloads } = await buildReplyPayloads({ @@ -142,31 +156,11 @@ describe("buildReplyPayloads media filter integration", () => { }); it("suppresses same-target replies when message tool target provider is generic", async () => { - const { replyPayloads } = await buildReplyPayloads({ - ...baseParams, - payloads: [{ text: "hello world!" }], - messageProvider: "heartbeat", - originatingChannel: "feishu", - originatingTo: "ou_abc123", - messagingToolSentTexts: ["different message"], - messagingToolSentTargets: [{ tool: "message", provider: "message", to: "ou_abc123" }], - }); - - expect(replyPayloads).toHaveLength(0); + await expectSameTargetRepliesSuppressed({ provider: "message", to: "ou_abc123" }); }); it("suppresses same-target replies when target provider is channel alias", async () => { - const { replyPayloads } = await buildReplyPayloads({ - ...baseParams, - payloads: [{ text: "hello world!" }], - messageProvider: "heartbeat", - originatingChannel: "feishu", - originatingTo: "ou_abc123", - messagingToolSentTexts: ["different message"], - messagingToolSentTargets: [{ tool: "message", provider: "lark", to: "ou_abc123" }], - }); - - expect(replyPayloads).toHaveLength(0); + await expectSameTargetRepliesSuppressed({ provider: "lark", to: "ou_abc123" }); }); it("drops all final payloads when block pipeline streamed successfully", async () => { diff --git a/src/auto-reply/reply/agent-runner.media-paths.test.ts b/src/auto-reply/reply/agent-runner.media-paths.test.ts index f5658287aff..a759c539bdc 100644 --- a/src/auto-reply/reply/agent-runner.media-paths.test.ts +++ b/src/auto-reply/reply/agent-runner.media-paths.test.ts @@ -2,7 +2,7 @@ import path from "node:path"; import { beforeEach, describe, expect, it, vi } from "vitest"; import type { TemplateContext } from "../templating.js"; import type { FollowupRun, QueueSettings } from "./queue.js"; -import { createMockTypingController } from "./test-helpers.js"; +import { createMockFollowupRun, createMockTypingController } from "./test-helpers.js"; const runEmbeddedPiAgentMock = vi.fn(); const runWithModelFallbackMock = vi.fn(); @@ -72,32 +72,15 @@ describe("runReplyAgent media path normalization", () => { const result = await runReplyAgent({ commandBody: "generate", - followupRun: { + followupRun: createMockFollowupRun({ prompt: "generate", - enqueuedAt: Date.now(), run: { agentId: "main", agentDir: "/tmp/agent", - sessionId: "session", - sessionKey: "main", messageProvider: "telegram", - sessionFile: "/tmp/session.jsonl", workspaceDir: "/tmp/workspace", - config: {}, - provider: "anthropic", - model: "claude", - thinkLevel: "low", - verboseLevel: "off", - elevatedLevel: "off", - bashElevated: { - enabled: false, - allowed: false, - defaultLevel: "off", - }, - timeoutMs: 1_000, - blockReplyBreak: "message_end", }, - } as unknown as FollowupRun, + }) as unknown as FollowupRun, queueKey: "main", resolvedQueue: { mode: "interrupt" } as QueueSettings, shouldSteer: false, diff --git a/src/auto-reply/reply/commands-acp/context.ts b/src/auto-reply/reply/commands-acp/context.ts index fd90175f38a..84acb828015 100644 --- a/src/auto-reply/reply/commands-acp/context.ts +++ b/src/auto-reply/reply/commands-acp/context.ts @@ -5,8 +5,8 @@ import { } from "../../../acp/conversation-id.js"; import { DISCORD_THREAD_BINDING_CHANNEL } from "../../../channels/thread-bindings-policy.js"; import { resolveConversationIdFromTargets } from "../../../infra/outbound/conversation-id.js"; -import { parseAgentSessionKey } from "../../../routing/session-key.js"; import type { HandleCommandsParams } from "../commands-types.js"; +import { parseDiscordParentChannelFromSessionKey } from "../discord-parent-channel.js"; import { resolveTelegramConversationId } from "../telegram-context.js"; export function resolveAcpCommandChannel(params: HandleCommandsParams): string { @@ -64,19 +64,6 @@ export function resolveAcpCommandConversationId(params: HandleCommandsParams): s }); } -function parseDiscordParentChannelFromSessionKey(raw: unknown): string | undefined { - const sessionKey = normalizeConversationText(raw); - if (!sessionKey) { - return undefined; - } - const scoped = parseAgentSessionKey(sessionKey)?.rest ?? sessionKey.toLowerCase(); - const match = scoped.match(/(?:^|:)channel:([^:]+)$/); - if (!match?.[1]) { - return undefined; - } - return match[1]; -} - function parseDiscordParentChannelFromContext(raw: unknown): string | undefined { const parentId = normalizeConversationText(raw); if (!parentId) { diff --git a/src/auto-reply/reply/commands-allowlist.ts b/src/auto-reply/reply/commands-allowlist.ts index ffba3bf2505..fcecb0b31f3 100644 --- a/src/auto-reply/reply/commands-allowlist.ts +++ b/src/auto-reply/reply/commands-allowlist.ts @@ -1,10 +1,5 @@ import { getChannelDock } from "../../channels/dock.js"; -import { - authorizeConfigWrite, - canBypassConfigWritePolicy, - formatConfigWriteDeniedMessage, - resolveExplicitConfigWriteTarget, -} from "../../channels/plugins/config-writes.js"; +import { resolveExplicitConfigWriteTarget } from "../../channels/plugins/config-writes.js"; import { listPairingChannels } from "../../channels/plugins/pairing.js"; import type { ChannelId } from "../../channels/plugins/types.js"; import { normalizeChannelId } from "../../channels/registry.js"; @@ -36,6 +31,7 @@ import { resolveTelegramAccount } from "../../telegram/accounts.js"; import { resolveWhatsAppAccount } from "../../web/accounts.js"; import { rejectUnauthorizedCommand, requireCommandFlagEnabled } from "./command-gates.js"; import type { CommandHandler } from "./commands-types.js"; +import { resolveConfigWriteDeniedText } from "./config-write-authorization.js"; type AllowlistScope = "dm" | "group" | "all"; type AllowlistAction = "list" | "add" | "remove"; @@ -628,20 +624,19 @@ export const handleAllowlistCommand: CommandHandler = async (params, allowTextCo accountId: normalizedAccountId, writeTarget, } = resolveAccountTarget(parsedConfig, channelId, accountId); - const writeAuth = authorizeConfigWrite({ + const deniedText = resolveConfigWriteDeniedText({ cfg: params.cfg, - origin: { channelId, accountId: params.ctx.AccountId }, + channel: params.command.channel, + channelId, + accountId: params.ctx.AccountId, + gatewayClientScopes: params.ctx.GatewayClientScopes, target: writeTarget, - allowBypass: canBypassConfigWritePolicy({ - channel: params.command.channel, - gatewayClientScopes: params.ctx.GatewayClientScopes, - }), }); - if (!writeAuth.allowed) { + if (deniedText) { return { shouldContinue: false, reply: { - text: formatConfigWriteDeniedMessage({ result: writeAuth, fallbackChannelId: channelId }), + text: deniedText, }, }; } diff --git a/src/auto-reply/reply/commands-config.ts b/src/auto-reply/reply/commands-config.ts index 96b5a5d9be5..b40032758d3 100644 --- a/src/auto-reply/reply/commands-config.ts +++ b/src/auto-reply/reply/commands-config.ts @@ -1,9 +1,4 @@ -import { - authorizeConfigWrite, - canBypassConfigWritePolicy, - formatConfigWriteDeniedMessage, - resolveConfigWriteTargetFromPath, -} from "../../channels/plugins/config-writes.js"; +import { resolveConfigWriteTargetFromPath } from "../../channels/plugins/config-writes.js"; import { normalizeChannelId } from "../../channels/registry.js"; import { getConfigValueAtPath, @@ -31,6 +26,7 @@ import { } from "./command-gates.js"; import type { CommandHandler } from "./commands-types.js"; import { parseConfigCommand } from "./config-commands.js"; +import { resolveConfigWriteDeniedText } from "./config-write-authorization.js"; import { parseDebugCommand } from "./debug-commands.js"; export const handleConfigCommand: CommandHandler = async (params, allowTextCommands) => { @@ -84,20 +80,19 @@ export const handleConfigCommand: CommandHandler = async (params, allowTextComma } parsedWritePath = parsedPath.path; const channelId = params.command.channelId ?? normalizeChannelId(params.command.channel); - const writeAuth = authorizeConfigWrite({ + const deniedText = resolveConfigWriteDeniedText({ cfg: params.cfg, - origin: { channelId, accountId: params.ctx.AccountId }, + channel: params.command.channel, + channelId, + accountId: params.ctx.AccountId, + gatewayClientScopes: params.ctx.GatewayClientScopes, target: resolveConfigWriteTargetFromPath(parsedWritePath), - allowBypass: canBypassConfigWritePolicy({ - channel: params.command.channel, - gatewayClientScopes: params.ctx.GatewayClientScopes, - }), }); - if (!writeAuth.allowed) { + if (deniedText) { return { shouldContinue: false, reply: { - text: formatConfigWriteDeniedMessage({ result: writeAuth, fallbackChannelId: channelId }), + text: deniedText, }, }; } diff --git a/src/auto-reply/reply/config-write-authorization.ts b/src/auto-reply/reply/config-write-authorization.ts new file mode 100644 index 00000000000..a2c2142709f --- /dev/null +++ b/src/auto-reply/reply/config-write-authorization.ts @@ -0,0 +1,33 @@ +import { + authorizeConfigWrite, + canBypassConfigWritePolicy, + formatConfigWriteDeniedMessage, +} from "../../channels/plugins/config-writes.js"; +import type { ChannelId } from "../../channels/plugins/types.js"; +import type { OpenClawConfig } from "../../config/config.js"; + +export function resolveConfigWriteDeniedText(params: { + cfg: OpenClawConfig; + channel?: string | null; + channelId: ChannelId | null; + accountId?: string; + gatewayClientScopes?: string[]; + target: Parameters[0]["target"]; +}): string | null { + const writeAuth = authorizeConfigWrite({ + cfg: params.cfg, + origin: { channelId: params.channelId, accountId: params.accountId }, + target: params.target, + allowBypass: canBypassConfigWritePolicy({ + channel: params.channel ?? "", + gatewayClientScopes: params.gatewayClientScopes, + }), + }); + if (writeAuth.allowed) { + return null; + } + return formatConfigWriteDeniedMessage({ + result: writeAuth, + fallbackChannelId: params.channelId, + }); +} diff --git a/src/auto-reply/reply/discord-parent-channel.ts b/src/auto-reply/reply/discord-parent-channel.ts new file mode 100644 index 00000000000..877c4593ea7 --- /dev/null +++ b/src/auto-reply/reply/discord-parent-channel.ts @@ -0,0 +1,15 @@ +import { normalizeConversationText } from "../../acp/conversation-id.js"; +import { parseAgentSessionKey } from "../../routing/session-key.js"; + +export function parseDiscordParentChannelFromSessionKey(raw: unknown): string | undefined { + const sessionKey = normalizeConversationText(raw); + if (!sessionKey) { + return undefined; + } + const scoped = parseAgentSessionKey(sessionKey)?.rest ?? sessionKey.toLowerCase(); + const match = scoped.match(/(?:^|:)channel:([^:]+)$/); + if (!match?.[1]) { + return undefined; + } + return match[1]; +} diff --git a/src/auto-reply/reply/followup-runner.test.ts b/src/auto-reply/reply/followup-runner.test.ts index a02ce0b2038..8d12e815685 100644 --- a/src/auto-reply/reply/followup-runner.test.ts +++ b/src/auto-reply/reply/followup-runner.test.ts @@ -4,7 +4,7 @@ import path from "node:path"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { loadSessionStore, saveSessionStore, type SessionEntry } from "../../config/sessions.js"; import type { FollowupRun } from "./queue.js"; -import { createMockTypingController } from "./test-helpers.js"; +import { createMockFollowupRun, createMockTypingController } from "./test-helpers.js"; const runEmbeddedPiAgentMock = vi.fn(); const routeReplyMock = vi.fn(); @@ -50,47 +50,12 @@ beforeEach(() => { }); const baseQueuedRun = (messageProvider = "whatsapp"): FollowupRun => - ({ - prompt: "hello", - summaryLine: "hello", - enqueuedAt: Date.now(), - originatingTo: "channel:C1", - run: { - sessionId: "session", - sessionKey: "main", - messageProvider, - agentAccountId: "primary", - sessionFile: "/tmp/session.jsonl", - workspaceDir: "/tmp", - config: {}, - skillsSnapshot: {}, - provider: "anthropic", - model: "claude", - thinkLevel: "low", - verboseLevel: "off", - elevatedLevel: "off", - bashElevated: { - enabled: false, - allowed: false, - defaultLevel: "off", - }, - timeoutMs: 1_000, - blockReplyBreak: "message_end", - }, - }) as FollowupRun; + createMockFollowupRun({ run: { messageProvider } }); function createQueuedRun( overrides: Partial> & { run?: Partial } = {}, ): FollowupRun { - const base = baseQueuedRun(); - return { - ...base, - ...overrides, - run: { - ...base.run, - ...overrides.run, - }, - }; + return createMockFollowupRun(overrides); } function mockCompactionRun(params: { diff --git a/src/auto-reply/reply/session.ts b/src/auto-reply/reply/session.ts index 85e6754025f..a2c0b1c7cf4 100644 --- a/src/auto-reply/reply/session.ts +++ b/src/auto-reply/reply/session.ts @@ -34,11 +34,12 @@ import { resolveConversationIdFromTargets } from "../../infra/outbound/conversat import { deliverSessionMaintenanceWarning } from "../../infra/session-maintenance-warning.js"; import { createSubsystemLogger } from "../../logging/subsystem.js"; import { getGlobalHookRunner } from "../../plugins/hook-runner-global.js"; -import { normalizeMainKey, parseAgentSessionKey } from "../../routing/session-key.js"; +import { normalizeMainKey } from "../../routing/session-key.js"; import { normalizeSessionDeliveryFields } from "../../utils/delivery-context.js"; import { resolveCommandAuthorization } from "../command-auth.js"; import type { MsgContext, TemplateContext } from "../templating.js"; import { resolveEffectiveResetTargetSessionKey } from "./acp-reset-target.js"; +import { parseDiscordParentChannelFromSessionKey } from "./discord-parent-channel.js"; import { normalizeInboundTextNewlines } from "./inbound-text.js"; import { stripMentions, stripStructuralPrefixes } from "./mentions.js"; import { @@ -70,19 +71,6 @@ export type SessionInitResult = { triggerBodyNormalized: string; }; -function parseDiscordParentChannelFromSessionKey(raw: unknown): string | undefined { - const sessionKey = normalizeConversationText(raw); - if (!sessionKey) { - return undefined; - } - const scoped = parseAgentSessionKey(sessionKey)?.rest ?? sessionKey.toLowerCase(); - const match = scoped.match(/(?:^|:)channel:([^:]+)$/); - if (!match?.[1]) { - return undefined; - } - return match[1]; -} - function resolveAcpResetBindingContext(ctx: MsgContext): { channel: string; accountId: string; diff --git a/src/auto-reply/reply/test-helpers.ts b/src/auto-reply/reply/test-helpers.ts index 4c30ae6756a..d92bf481f42 100644 --- a/src/auto-reply/reply/test-helpers.ts +++ b/src/auto-reply/reply/test-helpers.ts @@ -1,4 +1,5 @@ import { vi } from "vitest"; +import type { FollowupRun } from "./queue.js"; import type { TypingController } from "./typing.js"; export function createMockTypingController( @@ -16,3 +17,44 @@ export function createMockTypingController( ...overrides, }; } + +export function createMockFollowupRun( + overrides: Partial> & { run?: Partial } = {}, +): FollowupRun { + const base: FollowupRun = { + prompt: "hello", + summaryLine: "hello", + enqueuedAt: Date.now(), + originatingTo: "channel:C1", + run: { + sessionId: "session", + sessionKey: "main", + messageProvider: "whatsapp", + agentAccountId: "primary", + sessionFile: "/tmp/session.jsonl", + workspaceDir: "/tmp", + config: {}, + skillsSnapshot: {}, + provider: "anthropic", + model: "claude", + thinkLevel: "low", + verboseLevel: "off", + elevatedLevel: "off", + bashElevated: { + enabled: false, + allowed: false, + defaultLevel: "off", + }, + timeoutMs: 1_000, + blockReplyBreak: "message_end", + }, + }; + return { + ...base, + ...overrides, + run: { + ...base.run, + ...overrides.run, + }, + }; +}