diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b42bac2703..1e5273a8df5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ Docs: https://docs.openclaw.ai - Plugins/context-engine model auth: expose `runtime.modelAuth` and plugin-sdk auth helpers so plugins can resolve provider/model API keys through the normal auth pipeline. (#41090) thanks @xinhuagu. - CLI/memory teardown: close cached memory search/index managers in the one-shot CLI shutdown path so watcher-backed memory caches no longer keep completed CLI runs alive after output finishes. (#40389) thanks @Julbarth. - Tools/web search: treat Brave `llm-context` grounding snippets as plain strings so `web_search` no longer returns empty snippet arrays in LLM Context mode. (#41387) thanks @zheliu2. +- Telegram/exec approvals: reject `/approve` commands aimed at other bots, keep deterministic approval prompts visible when tool-result delivery fails, and stop resolved exact IDs from matching other pending approvals by prefix. (#37233) Thanks @huntharo. ## 2026.3.8 diff --git a/docs/channels/telegram.md b/docs/channels/telegram.md index f49ea5fe3f7..a039cb43483 100644 --- a/docs/channels/telegram.md +++ b/docs/channels/telegram.md @@ -760,6 +760,34 @@ openclaw message poll --channel telegram --target -1001234567890:topic:42 \ - `channels.telegram.actions.poll=false` disables Telegram poll creation while leaving regular sends enabled + + + Telegram supports exec approvals in approver DMs and can optionally post approval prompts in the originating chat or topic. + + Config path: + + - `channels.telegram.execApprovals.enabled` + - `channels.telegram.execApprovals.approvers` + - `channels.telegram.execApprovals.target` (`dm` | `channel` | `both`, default: `dm`) + - `agentFilter`, `sessionFilter` + + Approvers must be numeric Telegram user IDs. When `enabled` is false or `approvers` is empty, Telegram does not act as an exec approval client. Approval requests fall back to other configured approval routes or the exec approval fallback policy. + + Delivery rules: + + - `target: "dm"` sends approval prompts only to configured approver DMs + - `target: "channel"` sends the prompt back to the originating Telegram chat/topic + - `target: "both"` sends to approver DMs and the originating chat/topic + + Only configured approvers can approve or deny. Non-approvers cannot use `/approve` and cannot use Telegram approval buttons. + + Channel delivery shows the command text in the chat, so only enable `channel` or `both` in trusted groups/topics. When the prompt lands in a forum topic, OpenClaw preserves the topic for both the approval prompt and the post-approval follow-up. + + Inline approval buttons also depend on `channels.telegram.capabilities.inlineButtons` allowing the target surface (`dm`, `group`, or `all`). + + Related docs: [Exec approvals](/tools/exec-approvals) + + ## Troubleshooting @@ -859,10 +887,16 @@ Primary reference: - `channels.telegram.groups..enabled`: disable the group when `false`. - `channels.telegram.groups..topics..*`: per-topic overrides (group fields + topic-only `agentId`). - `channels.telegram.groups..topics..agentId`: route this topic to a specific agent (overrides group-level and binding routing). - - `channels.telegram.groups..topics..groupPolicy`: per-topic override for groupPolicy (`open | allowlist | disabled`). - - `channels.telegram.groups..topics..requireMention`: per-topic mention gating override. - - top-level `bindings[]` with `type: "acp"` and canonical topic id `chatId:topic:topicId` in `match.peer.id`: persistent ACP topic binding fields (see [ACP Agents](/tools/acp-agents#channel-specific-settings)). - - `channels.telegram.direct..topics..agentId`: route DM topics to a specific agent (same behavior as forum topics). +- `channels.telegram.groups..topics..groupPolicy`: per-topic override for groupPolicy (`open | allowlist | disabled`). +- `channels.telegram.groups..topics..requireMention`: per-topic mention gating override. +- top-level `bindings[]` with `type: "acp"` and canonical topic id `chatId:topic:topicId` in `match.peer.id`: persistent ACP topic binding fields (see [ACP Agents](/tools/acp-agents#channel-specific-settings)). +- `channels.telegram.direct..topics..agentId`: route DM topics to a specific agent (same behavior as forum topics). +- `channels.telegram.execApprovals.enabled`: enable Telegram as a chat-based exec approval client for this account. +- `channels.telegram.execApprovals.approvers`: Telegram user IDs allowed to approve or deny exec requests. Required when exec approvals are enabled. +- `channels.telegram.execApprovals.target`: `dm | channel | both` (default: `dm`). `channel` and `both` preserve the originating Telegram topic when present. +- `channels.telegram.execApprovals.agentFilter`: optional agent ID filter for forwarded approval prompts. +- `channels.telegram.execApprovals.sessionFilter`: optional session key filter (substring or regex) for forwarded approval prompts. +- `channels.telegram.accounts..execApprovals`: per-account override for Telegram exec approval routing and approver authorization. - `channels.telegram.capabilities.inlineButtons`: `off | dm | group | all | allowlist` (default: allowlist). - `channels.telegram.accounts..capabilities.inlineButtons`: per-account override. - `channels.telegram.commands.nativeSkills`: enable/disable Telegram native skills commands. @@ -894,6 +928,7 @@ Telegram-specific high-signal fields: - startup/auth: `enabled`, `botToken`, `tokenFile`, `accounts.*` - access control: `dmPolicy`, `allowFrom`, `groupPolicy`, `groupAllowFrom`, `groups`, `groups.*.topics.*`, top-level `bindings[]` (`type: "acp"`) +- exec approvals: `execApprovals`, `accounts.*.execApprovals` - command/menu: `commands.native`, `commands.nativeSkills`, `customCommands` - threading/replies: `replyToMode` - streaming: `streaming` (preview), `blockStreaming` diff --git a/docs/tools/exec-approvals.md b/docs/tools/exec-approvals.md index d538e411093..91fdff80650 100644 --- a/docs/tools/exec-approvals.md +++ b/docs/tools/exec-approvals.md @@ -309,6 +309,32 @@ Reply in chat: /approve deny ``` +### Built-in chat approval clients + +Discord and Telegram can also act as explicit exec approval clients with channel-specific config. + +- Discord: `channels.discord.execApprovals.*` +- Telegram: `channels.telegram.execApprovals.*` + +These clients are opt-in. If a channel does not have exec approvals enabled, OpenClaw does not treat +that channel as an approval surface just because the conversation happened there. + +Shared behavior: + +- only configured approvers can approve or deny +- the requester does not need to be an approver +- when channel delivery is enabled, approval prompts include the command text +- if no operator UI or configured approval client can accept the request, the prompt falls back to `askFallback` + +Telegram defaults to approver DMs (`target: "dm"`). You can switch to `channel` or `both` when you +want approval prompts to appear in the originating Telegram chat/topic as well. For Telegram forum +topics, OpenClaw preserves the topic for the approval prompt and the post-approval follow-up. + +See: + +- [Discord](/channels/discord#exec-approvals-in-discord) +- [Telegram](/channels/telegram#exec-approvals-in-telegram) + ### macOS IPC flow ``` diff --git a/extensions/telegram/src/channel.test.ts b/extensions/telegram/src/channel.test.ts index 1f40a5f1cce..c1912db56f0 100644 --- a/extensions/telegram/src/channel.test.ts +++ b/extensions/telegram/src/channel.test.ts @@ -179,6 +179,41 @@ describe("telegramPlugin duplicate token guard", () => { expect(result).toMatchObject({ channel: "telegram", messageId: "tg-1" }); }); + it("preserves buttons for outbound text payload sends", async () => { + const sendMessageTelegram = vi.fn(async () => ({ messageId: "tg-2" })); + setTelegramRuntime({ + channel: { + telegram: { + sendMessageTelegram, + }, + }, + } as unknown as PluginRuntime); + + const result = await telegramPlugin.outbound!.sendPayload!({ + cfg: createCfg(), + to: "12345", + text: "", + payload: { + text: "Approval required", + channelData: { + telegram: { + buttons: [[{ text: "Allow Once", callback_data: "/approve abc allow-once" }]], + }, + }, + }, + accountId: "ops", + }); + + expect(sendMessageTelegram).toHaveBeenCalledWith( + "12345", + "Approval required", + expect.objectContaining({ + buttons: [[{ text: "Allow Once", callback_data: "/approve abc allow-once" }]], + }), + ); + expect(result).toMatchObject({ channel: "telegram", messageId: "tg-2" }); + }); + it("ignores accounts with missing tokens during duplicate-token checks", async () => { const cfg = createCfg(); cfg.channels!.telegram!.accounts!.ops = {} as never; diff --git a/extensions/telegram/src/channel.ts b/extensions/telegram/src/channel.ts index 0f4721a4d62..7ea0a7a6525 100644 --- a/extensions/telegram/src/channel.ts +++ b/extensions/telegram/src/channel.ts @@ -91,6 +91,10 @@ const telegramMessageActions: ChannelMessageActionAdapter = { }, }; +type TelegramInlineButtons = ReadonlyArray< + ReadonlyArray<{ text: string; callback_data: string; style?: "danger" | "success" | "primary" }> +>; + const telegramConfigAccessors = createScopedAccountConfigAccessors({ resolveAccount: ({ cfg, accountId }) => resolveTelegramAccount({ cfg, accountId }), resolveAllowFrom: (account: ResolvedTelegramAccount) => account.config.allowFrom, @@ -317,6 +321,62 @@ export const telegramPlugin: ChannelPlugin { + const send = deps?.sendTelegram ?? getTelegramRuntime().channel.telegram.sendMessageTelegram; + const replyToMessageId = parseTelegramReplyToMessageId(replyToId); + const messageThreadId = parseTelegramThreadId(threadId); + const telegramData = payload.channelData?.telegram as + | { buttons?: TelegramInlineButtons; quoteText?: string } + | undefined; + const quoteText = + typeof telegramData?.quoteText === "string" ? telegramData.quoteText : undefined; + const text = payload.text ?? ""; + const mediaUrls = payload.mediaUrls?.length + ? payload.mediaUrls + : payload.mediaUrl + ? [payload.mediaUrl] + : []; + const baseOpts = { + verbose: false, + cfg, + mediaLocalRoots, + messageThreadId, + replyToMessageId, + quoteText, + accountId: accountId ?? undefined, + silent: silent ?? undefined, + }; + + if (mediaUrls.length === 0) { + const result = await send(to, text, { + ...baseOpts, + buttons: telegramData?.buttons, + }); + return { channel: "telegram", ...result }; + } + + let finalResult: Awaited> | undefined; + for (let i = 0; i < mediaUrls.length; i += 1) { + const mediaUrl = mediaUrls[i]; + const isFirst = i === 0; + finalResult = await send(to, isFirst ? text : "", { + ...baseOpts, + mediaUrl, + ...(isFirst ? { buttons: telegramData?.buttons } : {}), + }); + } + return { channel: "telegram", ...(finalResult ?? { messageId: "unknown", chatId: to }) }; + }, sendText: async ({ cfg, to, text, accountId, deps, replyToId, threadId, silent }) => { const send = deps?.sendTelegram ?? getTelegramRuntime().channel.telegram.sendMessageTelegram; const replyToMessageId = parseTelegramReplyToMessageId(replyToId); diff --git a/src/agents/bash-tools.exec-approval-followup.ts b/src/agents/bash-tools.exec-approval-followup.ts new file mode 100644 index 00000000000..af24f07fb50 --- /dev/null +++ b/src/agents/bash-tools.exec-approval-followup.ts @@ -0,0 +1,61 @@ +import { callGatewayTool } from "./tools/gateway.js"; + +type ExecApprovalFollowupParams = { + approvalId: string; + sessionKey?: string; + turnSourceChannel?: string; + turnSourceTo?: string; + turnSourceAccountId?: string; + turnSourceThreadId?: string | number; + resultText: string; +}; + +export function buildExecApprovalFollowupPrompt(resultText: string): string { + return [ + "An async command the user already approved has completed.", + "Do not run the command again.", + "", + "Exact completion details:", + resultText.trim(), + "", + "Reply to the user in a helpful way.", + "If it succeeded, share the relevant output.", + "If it failed, explain what went wrong.", + ].join("\n"); +} + +export async function sendExecApprovalFollowup( + params: ExecApprovalFollowupParams, +): Promise { + const sessionKey = params.sessionKey?.trim(); + const resultText = params.resultText.trim(); + if (!sessionKey || !resultText) { + return false; + } + + const channel = params.turnSourceChannel?.trim(); + const to = params.turnSourceTo?.trim(); + const threadId = + params.turnSourceThreadId != null && params.turnSourceThreadId !== "" + ? String(params.turnSourceThreadId) + : undefined; + + await callGatewayTool( + "agent", + { timeoutMs: 60_000 }, + { + sessionKey, + message: buildExecApprovalFollowupPrompt(resultText), + deliver: true, + bestEffortDeliver: true, + channel: channel && to ? channel : undefined, + to: channel && to ? to : undefined, + accountId: channel && to ? params.turnSourceAccountId?.trim() || undefined : undefined, + threadId: channel && to ? threadId : undefined, + idempotencyKey: `exec-approval-followup:${params.approvalId}`, + }, + { expectFinal: true }, + ); + + return true; +} diff --git a/src/agents/bash-tools.exec-host-gateway.ts b/src/agents/bash-tools.exec-host-gateway.ts index 49a958c9c5b..6b43fbe8663 100644 --- a/src/agents/bash-tools.exec-host-gateway.ts +++ b/src/agents/bash-tools.exec-host-gateway.ts @@ -1,4 +1,10 @@ import type { AgentToolResult } from "@mariozechner/pi-agent-core"; +import { loadConfig } from "../config/config.js"; +import { buildExecApprovalUnavailableReplyPayload } from "../infra/exec-approval-reply.js"; +import { + hasConfiguredExecApprovalDmRoute, + resolveExecApprovalInitiatingSurfaceState, +} from "../infra/exec-approval-surface.js"; import { addAllowlistEntry, type ExecAsk, @@ -13,6 +19,7 @@ import { detectCommandObfuscation } from "../infra/exec-obfuscation-detect.js"; import type { SafeBinProfile } from "../infra/exec-safe-bin-policy.js"; import { logInfo } from "../logger.js"; import { markBackgrounded, tail } from "./bash-process-registry.js"; +import { sendExecApprovalFollowup } from "./bash-tools.exec-approval-followup.js"; import { buildExecApprovalRequesterContext, buildExecApprovalTurnSourceContext, @@ -25,9 +32,9 @@ import { resolveExecHostApprovalContext, } from "./bash-tools.exec-host-shared.js"; import { + buildApprovalPendingMessage, DEFAULT_NOTIFY_TAIL_CHARS, createApprovalSlug, - emitExecSystemEvent, normalizeNotifyOutput, runExecProcess, } from "./bash-tools.exec-runtime.js"; @@ -141,8 +148,6 @@ export async function processGatewayAllowlist( const { approvalId, approvalSlug, - contextKey, - noticeSeconds, warningText, expiresAtMs: defaultExpiresAtMs, preResolvedDecision: defaultPreResolvedDecision, @@ -174,19 +179,37 @@ export async function processGatewayAllowlist( }); expiresAtMs = registration.expiresAtMs; preResolvedDecision = registration.finalDecision; + const initiatingSurface = resolveExecApprovalInitiatingSurfaceState({ + channel: params.turnSourceChannel, + accountId: params.turnSourceAccountId, + }); + const cfg = loadConfig(); + const sentApproverDms = + (initiatingSurface.kind === "disabled" || initiatingSurface.kind === "unsupported") && + hasConfiguredExecApprovalDmRoute(cfg); + const unavailableReason = + preResolvedDecision === null + ? "no-approval-route" + : initiatingSurface.kind === "disabled" + ? "initiating-platform-disabled" + : initiatingSurface.kind === "unsupported" + ? "initiating-platform-unsupported" + : null; void (async () => { const decision = await resolveApprovalDecisionOrUndefined({ approvalId, preResolvedDecision, onFailure: () => - emitExecSystemEvent( - `Exec denied (gateway id=${approvalId}, approval-request-failed): ${params.command}`, - { - sessionKey: params.notifySessionKey, - contextKey, - }, - ), + void sendExecApprovalFollowup({ + approvalId, + sessionKey: params.notifySessionKey, + turnSourceChannel: params.turnSourceChannel, + turnSourceTo: params.turnSourceTo, + turnSourceAccountId: params.turnSourceAccountId, + turnSourceThreadId: params.turnSourceThreadId, + resultText: `Exec denied (gateway id=${approvalId}, approval-request-failed): ${params.command}`, + }), }); if (decision === undefined) { return; @@ -230,13 +253,15 @@ export async function processGatewayAllowlist( } if (deniedReason) { - emitExecSystemEvent( - `Exec denied (gateway id=${approvalId}, ${deniedReason}): ${params.command}`, - { - sessionKey: params.notifySessionKey, - contextKey, - }, - ); + await sendExecApprovalFollowup({ + approvalId, + sessionKey: params.notifySessionKey, + turnSourceChannel: params.turnSourceChannel, + turnSourceTo: params.turnSourceTo, + turnSourceAccountId: params.turnSourceAccountId, + turnSourceThreadId: params.turnSourceThreadId, + resultText: `Exec denied (gateway id=${approvalId}, ${deniedReason}): ${params.command}`, + }).catch(() => {}); return; } @@ -262,32 +287,21 @@ export async function processGatewayAllowlist( timeoutSec: effectiveTimeout, }); } catch { - emitExecSystemEvent( - `Exec denied (gateway id=${approvalId}, spawn-failed): ${params.command}`, - { - sessionKey: params.notifySessionKey, - contextKey, - }, - ); + await sendExecApprovalFollowup({ + approvalId, + sessionKey: params.notifySessionKey, + turnSourceChannel: params.turnSourceChannel, + turnSourceTo: params.turnSourceTo, + turnSourceAccountId: params.turnSourceAccountId, + turnSourceThreadId: params.turnSourceThreadId, + resultText: `Exec denied (gateway id=${approvalId}, spawn-failed): ${params.command}`, + }).catch(() => {}); return; } markBackgrounded(run.session); - let runningTimer: NodeJS.Timeout | null = null; - if (params.approvalRunningNoticeMs > 0) { - runningTimer = setTimeout(() => { - emitExecSystemEvent( - `Exec running (gateway id=${approvalId}, session=${run?.session.id}, >${noticeSeconds}s): ${params.command}`, - { sessionKey: params.notifySessionKey, contextKey }, - ); - }, params.approvalRunningNoticeMs); - } - const outcome = await run.promise; - if (runningTimer) { - clearTimeout(runningTimer); - } const output = normalizeNotifyOutput( tail(outcome.aggregated || "", DEFAULT_NOTIFY_TAIL_CHARS), ); @@ -295,7 +309,15 @@ export async function processGatewayAllowlist( const summary = output ? `Exec finished (gateway id=${approvalId}, session=${run.session.id}, ${exitLabel})\n${output}` : `Exec finished (gateway id=${approvalId}, session=${run.session.id}, ${exitLabel})`; - emitExecSystemEvent(summary, { sessionKey: params.notifySessionKey, contextKey }); + await sendExecApprovalFollowup({ + approvalId, + sessionKey: params.notifySessionKey, + turnSourceChannel: params.turnSourceChannel, + turnSourceTo: params.turnSourceTo, + turnSourceAccountId: params.turnSourceAccountId, + turnSourceThreadId: params.turnSourceThreadId, + resultText: summary, + }).catch(() => {}); })(); return { @@ -304,19 +326,45 @@ export async function processGatewayAllowlist( { type: "text", text: - `${warningText}Approval required (id ${approvalSlug}). ` + - "Approve to run; updates will arrive after completion.", + unavailableReason !== null + ? (buildExecApprovalUnavailableReplyPayload({ + warningText, + reason: unavailableReason, + channelLabel: initiatingSurface.channelLabel, + sentApproverDms, + }).text ?? "") + : buildApprovalPendingMessage({ + warningText, + approvalSlug, + approvalId, + command: params.command, + cwd: params.workdir, + host: "gateway", + }), }, ], - details: { - status: "approval-pending", - approvalId, - approvalSlug, - expiresAtMs, - host: "gateway", - command: params.command, - cwd: params.workdir, - }, + details: + unavailableReason !== null + ? ({ + status: "approval-unavailable", + reason: unavailableReason, + channelLabel: initiatingSurface.channelLabel, + sentApproverDms, + host: "gateway", + command: params.command, + cwd: params.workdir, + warningText, + } satisfies ExecToolDetails) + : ({ + status: "approval-pending", + approvalId, + approvalSlug, + expiresAtMs, + host: "gateway", + command: params.command, + cwd: params.workdir, + warningText, + } satisfies ExecToolDetails), }, }; } diff --git a/src/agents/bash-tools.exec-host-node.ts b/src/agents/bash-tools.exec-host-node.ts index b66a6ededf1..97eb4218035 100644 --- a/src/agents/bash-tools.exec-host-node.ts +++ b/src/agents/bash-tools.exec-host-node.ts @@ -1,5 +1,11 @@ import crypto from "node:crypto"; import type { AgentToolResult } from "@mariozechner/pi-agent-core"; +import { loadConfig } from "../config/config.js"; +import { buildExecApprovalUnavailableReplyPayload } from "../infra/exec-approval-reply.js"; +import { + hasConfiguredExecApprovalDmRoute, + resolveExecApprovalInitiatingSurfaceState, +} from "../infra/exec-approval-surface.js"; import { type ExecApprovalsFile, type ExecAsk, @@ -12,6 +18,7 @@ import { detectCommandObfuscation } from "../infra/exec-obfuscation-detect.js"; import { buildNodeShellCommand } from "../infra/node-shell.js"; import { parsePreparedSystemRunPayload } from "../infra/system-run-approval-context.js"; import { logInfo } from "../logger.js"; +import { sendExecApprovalFollowup } from "./bash-tools.exec-approval-followup.js"; import { buildExecApprovalRequesterContext, buildExecApprovalTurnSourceContext, @@ -23,7 +30,12 @@ import { resolveApprovalDecisionOrUndefined, resolveExecHostApprovalContext, } from "./bash-tools.exec-host-shared.js"; -import { createApprovalSlug, emitExecSystemEvent } from "./bash-tools.exec-runtime.js"; +import { + buildApprovalPendingMessage, + DEFAULT_NOTIFY_TAIL_CHARS, + createApprovalSlug, + normalizeNotifyOutput, +} from "./bash-tools.exec-runtime.js"; import type { ExecToolDetails } from "./bash-tools.exec-types.js"; import { callGatewayTool } from "./tools/gateway.js"; import { listNodes, resolveNodeIdFromList } from "./tools/nodes-utils.js"; @@ -187,6 +199,7 @@ export async function executeNodeHostCommand( approvedByAsk: boolean, approvalDecision: "allow-once" | "allow-always" | null, runId?: string, + suppressNotifyOnExit?: boolean, ) => ({ nodeId, @@ -202,6 +215,7 @@ export async function executeNodeHostCommand( approved: approvedByAsk, approvalDecision: approvalDecision ?? undefined, runId: runId ?? undefined, + suppressNotifyOnExit: suppressNotifyOnExit === true ? true : undefined, }, idempotencyKey: crypto.randomUUID(), }) satisfies Record; @@ -210,8 +224,6 @@ export async function executeNodeHostCommand( const { approvalId, approvalSlug, - contextKey, - noticeSeconds, warningText, expiresAtMs: defaultExpiresAtMs, preResolvedDecision: defaultPreResolvedDecision, @@ -243,16 +255,37 @@ export async function executeNodeHostCommand( }); expiresAtMs = registration.expiresAtMs; preResolvedDecision = registration.finalDecision; + const initiatingSurface = resolveExecApprovalInitiatingSurfaceState({ + channel: params.turnSourceChannel, + accountId: params.turnSourceAccountId, + }); + const cfg = loadConfig(); + const sentApproverDms = + (initiatingSurface.kind === "disabled" || initiatingSurface.kind === "unsupported") && + hasConfiguredExecApprovalDmRoute(cfg); + const unavailableReason = + preResolvedDecision === null + ? "no-approval-route" + : initiatingSurface.kind === "disabled" + ? "initiating-platform-disabled" + : initiatingSurface.kind === "unsupported" + ? "initiating-platform-unsupported" + : null; void (async () => { const decision = await resolveApprovalDecisionOrUndefined({ approvalId, preResolvedDecision, onFailure: () => - emitExecSystemEvent( - `Exec denied (node=${nodeId} id=${approvalId}, approval-request-failed): ${params.command}`, - { sessionKey: params.notifySessionKey, contextKey }, - ), + void sendExecApprovalFollowup({ + approvalId, + sessionKey: params.notifySessionKey, + turnSourceChannel: params.turnSourceChannel, + turnSourceTo: params.turnSourceTo, + turnSourceAccountId: params.turnSourceAccountId, + turnSourceThreadId: params.turnSourceThreadId, + resultText: `Exec denied (node=${nodeId} id=${approvalId}, approval-request-failed): ${params.command}`, + }), }); if (decision === undefined) { return; @@ -278,44 +311,67 @@ export async function executeNodeHostCommand( } if (deniedReason) { - emitExecSystemEvent( - `Exec denied (node=${nodeId} id=${approvalId}, ${deniedReason}): ${params.command}`, - { - sessionKey: params.notifySessionKey, - contextKey, - }, - ); + await sendExecApprovalFollowup({ + approvalId, + sessionKey: params.notifySessionKey, + turnSourceChannel: params.turnSourceChannel, + turnSourceTo: params.turnSourceTo, + turnSourceAccountId: params.turnSourceAccountId, + turnSourceThreadId: params.turnSourceThreadId, + resultText: `Exec denied (node=${nodeId} id=${approvalId}, ${deniedReason}): ${params.command}`, + }).catch(() => {}); return; } - let runningTimer: NodeJS.Timeout | null = null; - if (params.approvalRunningNoticeMs > 0) { - runningTimer = setTimeout(() => { - emitExecSystemEvent( - `Exec running (node=${nodeId} id=${approvalId}, >${noticeSeconds}s): ${params.command}`, - { sessionKey: params.notifySessionKey, contextKey }, - ); - }, params.approvalRunningNoticeMs); - } - try { - await callGatewayTool( + const raw = await callGatewayTool<{ + payload?: { + stdout?: string; + stderr?: string; + error?: string | null; + exitCode?: number | null; + timedOut?: boolean; + }; + }>( "node.invoke", { timeoutMs: invokeTimeoutMs }, - buildInvokeParams(approvedByAsk, approvalDecision, approvalId), + buildInvokeParams(approvedByAsk, approvalDecision, approvalId, true), ); + const payload = + raw?.payload && typeof raw.payload === "object" + ? (raw.payload as { + stdout?: string; + stderr?: string; + error?: string | null; + exitCode?: number | null; + timedOut?: boolean; + }) + : {}; + const combined = [payload.stdout, payload.stderr, payload.error].filter(Boolean).join("\n"); + const output = normalizeNotifyOutput(combined.slice(-DEFAULT_NOTIFY_TAIL_CHARS)); + const exitLabel = payload.timedOut ? "timeout" : `code ${payload.exitCode ?? "?"}`; + const summary = output + ? `Exec finished (node=${nodeId} id=${approvalId}, ${exitLabel})\n${output}` + : `Exec finished (node=${nodeId} id=${approvalId}, ${exitLabel})`; + await sendExecApprovalFollowup({ + approvalId, + sessionKey: params.notifySessionKey, + turnSourceChannel: params.turnSourceChannel, + turnSourceTo: params.turnSourceTo, + turnSourceAccountId: params.turnSourceAccountId, + turnSourceThreadId: params.turnSourceThreadId, + resultText: summary, + }).catch(() => {}); } catch { - emitExecSystemEvent( - `Exec denied (node=${nodeId} id=${approvalId}, invoke-failed): ${params.command}`, - { - sessionKey: params.notifySessionKey, - contextKey, - }, - ); - } finally { - if (runningTimer) { - clearTimeout(runningTimer); - } + await sendExecApprovalFollowup({ + approvalId, + sessionKey: params.notifySessionKey, + turnSourceChannel: params.turnSourceChannel, + turnSourceTo: params.turnSourceTo, + turnSourceAccountId: params.turnSourceAccountId, + turnSourceThreadId: params.turnSourceThreadId, + resultText: `Exec denied (node=${nodeId} id=${approvalId}, invoke-failed): ${params.command}`, + }).catch(() => {}); } })(); @@ -324,20 +380,48 @@ export async function executeNodeHostCommand( { type: "text", text: - `${warningText}Approval required (id ${approvalSlug}). ` + - "Approve to run; updates will arrive after completion.", + unavailableReason !== null + ? (buildExecApprovalUnavailableReplyPayload({ + warningText, + reason: unavailableReason, + channelLabel: initiatingSurface.channelLabel, + sentApproverDms, + }).text ?? "") + : buildApprovalPendingMessage({ + warningText, + approvalSlug, + approvalId, + command: prepared.cmdText, + cwd: runCwd, + host: "node", + nodeId, + }), }, ], - details: { - status: "approval-pending", - approvalId, - approvalSlug, - expiresAtMs, - host: "node", - command: params.command, - cwd: params.workdir, - nodeId, - }, + details: + unavailableReason !== null + ? ({ + status: "approval-unavailable", + reason: unavailableReason, + channelLabel: initiatingSurface.channelLabel, + sentApproverDms, + host: "node", + command: params.command, + cwd: params.workdir, + nodeId, + warningText, + } satisfies ExecToolDetails) + : ({ + status: "approval-pending", + approvalId, + approvalSlug, + expiresAtMs, + host: "node", + command: params.command, + cwd: params.workdir, + nodeId, + warningText, + } satisfies ExecToolDetails), }; } diff --git a/src/agents/bash-tools.exec-runtime.ts b/src/agents/bash-tools.exec-runtime.ts index 9714e4255ee..5c3301414b9 100644 --- a/src/agents/bash-tools.exec-runtime.ts +++ b/src/agents/bash-tools.exec-runtime.ts @@ -230,6 +230,40 @@ export function createApprovalSlug(id: string) { return id.slice(0, APPROVAL_SLUG_LENGTH); } +export function buildApprovalPendingMessage(params: { + warningText?: string; + approvalSlug: string; + approvalId: string; + command: string; + cwd: string; + host: "gateway" | "node"; + nodeId?: string; +}) { + let fence = "```"; + while (params.command.includes(fence)) { + fence += "`"; + } + const commandBlock = `${fence}sh\n${params.command}\n${fence}`; + const lines: string[] = []; + const warningText = params.warningText?.trim(); + if (warningText) { + lines.push(warningText, ""); + } + lines.push(`Approval required (id ${params.approvalSlug}, full ${params.approvalId}).`); + lines.push(`Host: ${params.host}`); + if (params.nodeId) { + lines.push(`Node: ${params.nodeId}`); + } + lines.push(`CWD: ${params.cwd}`); + lines.push("Command:"); + lines.push(commandBlock); + lines.push("Mode: foreground (interactive approvals available)."); + lines.push("Background mode requires pre-approved policy (allow-always or ask=off)."); + lines.push(`Reply with: /approve ${params.approvalSlug} allow-once|allow-always|deny`); + lines.push("If the short code is ambiguous, use the full id in /approve."); + return lines.join("\n"); +} + export function resolveApprovalRunningNoticeMs(value?: number) { if (typeof value !== "number" || !Number.isFinite(value)) { return DEFAULT_APPROVAL_RUNNING_NOTICE_MS; diff --git a/src/agents/bash-tools.exec-types.ts b/src/agents/bash-tools.exec-types.ts index bef8ea4bff1..7236fdaaf47 100644 --- a/src/agents/bash-tools.exec-types.ts +++ b/src/agents/bash-tools.exec-types.ts @@ -60,4 +60,19 @@ export type ExecToolDetails = command: string; cwd?: string; nodeId?: string; + warningText?: string; + } + | { + status: "approval-unavailable"; + reason: + | "initiating-platform-disabled" + | "initiating-platform-unsupported" + | "no-approval-route"; + channelLabel?: string; + sentApproverDms?: boolean; + host: ExecHost; + command: string; + cwd?: string; + nodeId?: string; + warningText?: string; }; diff --git a/src/agents/bash-tools.exec.approval-id.test.ts b/src/agents/bash-tools.exec.approval-id.test.ts index b7f4729948c..cc94f83d665 100644 --- a/src/agents/bash-tools.exec.approval-id.test.ts +++ b/src/agents/bash-tools.exec.approval-id.test.ts @@ -2,6 +2,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { clearConfigCache } from "../config/config.js"; import { buildSystemRunPreparePayload } from "../test-utils/system-run-prepare-payload.js"; vi.mock("./tools/gateway.js", () => ({ @@ -63,6 +64,7 @@ describe("exec approvals", () => { afterEach(() => { vi.resetAllMocks(); + clearConfigCache(); if (previousHome === undefined) { delete process.env.HOME; } else { @@ -77,6 +79,7 @@ describe("exec approvals", () => { it("reuses approval id as the node runId", async () => { let invokeParams: unknown; + let agentParams: unknown; vi.mocked(callGatewayTool).mockImplementation(async (method, _opts, params) => { if (method === "exec.approval.request") { @@ -85,6 +88,10 @@ describe("exec approvals", () => { if (method === "exec.approval.waitDecision") { return { decision: "allow-once" }; } + if (method === "agent") { + agentParams = params; + return { status: "ok" }; + } if (method === "node.invoke") { const invoke = params as { command?: string }; if (invoke.command === "system.run.prepare") { @@ -102,11 +109,24 @@ describe("exec approvals", () => { host: "node", ask: "always", approvalRunningNoticeMs: 0, + sessionKey: "agent:main:main", }); const result = await tool.execute("call1", { command: "ls -la" }); expect(result.details.status).toBe("approval-pending"); - const approvalId = (result.details as { approvalId: string }).approvalId; + const details = result.details as { approvalId: string; approvalSlug: string }; + const pendingText = result.content.find((part) => part.type === "text")?.text ?? ""; + expect(pendingText).toContain( + `Reply with: /approve ${details.approvalSlug} allow-once|allow-always|deny`, + ); + expect(pendingText).toContain(`full ${details.approvalId}`); + expect(pendingText).toContain("Host: node"); + expect(pendingText).toContain("Node: node-1"); + expect(pendingText).toContain(`CWD: ${process.cwd()}`); + expect(pendingText).toContain("Command:\n```sh\nls -la\n```"); + expect(pendingText).toContain("Mode: foreground (interactive approvals available)."); + expect(pendingText).toContain("Background mode requires pre-approved policy"); + const approvalId = details.approvalId; await expect .poll(() => (invokeParams as { params?: { runId?: string } } | undefined)?.params?.runId, { @@ -114,6 +134,12 @@ describe("exec approvals", () => { interval: 20, }) .toBe(approvalId); + expect( + (invokeParams as { params?: { suppressNotifyOnExit?: boolean } } | undefined)?.params, + ).toMatchObject({ + suppressNotifyOnExit: true, + }); + await expect.poll(() => agentParams, { timeout: 2_000, interval: 20 }).toBeTruthy(); }); it("skips approval when node allowlist is satisfied", async () => { @@ -287,11 +313,181 @@ describe("exec approvals", () => { const result = await tool.execute("call4", { command: "echo ok", elevated: true }); expect(result.details.status).toBe("approval-pending"); + const details = result.details as { approvalId: string; approvalSlug: string }; + const pendingText = result.content.find((part) => part.type === "text")?.text ?? ""; + expect(pendingText).toContain( + `Reply with: /approve ${details.approvalSlug} allow-once|allow-always|deny`, + ); + expect(pendingText).toContain(`full ${details.approvalId}`); + expect(pendingText).toContain("Host: gateway"); + expect(pendingText).toContain(`CWD: ${process.cwd()}`); + expect(pendingText).toContain("Command:\n```sh\necho ok\n```"); await approvalSeen; expect(calls).toContain("exec.approval.request"); expect(calls).toContain("exec.approval.waitDecision"); }); + it("starts a direct agent follow-up after approved gateway exec completes", async () => { + const agentCalls: Array> = []; + + vi.mocked(callGatewayTool).mockImplementation(async (method, _opts, params) => { + if (method === "exec.approval.request") { + return { status: "accepted", id: (params as { id?: string })?.id }; + } + if (method === "exec.approval.waitDecision") { + return { decision: "allow-once" }; + } + if (method === "agent") { + agentCalls.push(params as Record); + return { status: "ok" }; + } + return { ok: true }; + }); + + const tool = createExecTool({ + host: "gateway", + ask: "always", + approvalRunningNoticeMs: 0, + sessionKey: "agent:main:main", + elevated: { enabled: true, allowed: true, defaultLevel: "ask" }, + }); + + const result = await tool.execute("call-gw-followup", { + command: "echo ok", + workdir: process.cwd(), + gatewayUrl: undefined, + gatewayToken: undefined, + }); + + expect(result.details.status).toBe("approval-pending"); + await expect.poll(() => agentCalls.length, { timeout: 3_000, interval: 20 }).toBe(1); + expect(agentCalls[0]).toEqual( + expect.objectContaining({ + sessionKey: "agent:main:main", + deliver: true, + idempotencyKey: expect.stringContaining("exec-approval-followup:"), + }), + ); + expect(typeof agentCalls[0]?.message).toBe("string"); + expect(agentCalls[0]?.message).toContain( + "An async command the user already approved has completed.", + ); + }); + + it("requires a separate approval for each elevated command after allow-once", async () => { + const requestCommands: string[] = []; + const requestIds: string[] = []; + const waitIds: string[] = []; + + vi.mocked(callGatewayTool).mockImplementation(async (method, _opts, params) => { + if (method === "exec.approval.request") { + const request = params as { id?: string; command?: string }; + if (typeof request.command === "string") { + requestCommands.push(request.command); + } + if (typeof request.id === "string") { + requestIds.push(request.id); + } + return { status: "accepted", id: request.id }; + } + if (method === "exec.approval.waitDecision") { + const wait = params as { id?: string }; + if (typeof wait.id === "string") { + waitIds.push(wait.id); + } + return { decision: "allow-once" }; + } + return { ok: true }; + }); + + const tool = createExecTool({ + ask: "on-miss", + security: "allowlist", + approvalRunningNoticeMs: 0, + elevated: { enabled: true, allowed: true, defaultLevel: "ask" }, + }); + + const first = await tool.execute("call-seq-1", { + command: "npm view diver --json", + elevated: true, + }); + const second = await tool.execute("call-seq-2", { + command: "brew outdated", + elevated: true, + }); + + expect(first.details.status).toBe("approval-pending"); + expect(second.details.status).toBe("approval-pending"); + expect(requestCommands).toEqual(["npm view diver --json", "brew outdated"]); + expect(requestIds).toHaveLength(2); + expect(requestIds[0]).not.toBe(requestIds[1]); + expect(waitIds).toEqual(requestIds); + }); + + it("shows full chained gateway commands in approval-pending message", async () => { + const calls: string[] = []; + vi.mocked(callGatewayTool).mockImplementation(async (method, _opts, params) => { + calls.push(method); + if (method === "exec.approval.request") { + return { status: "accepted", id: (params as { id?: string })?.id }; + } + if (method === "exec.approval.waitDecision") { + return { decision: "deny" }; + } + return { ok: true }; + }); + + const tool = createExecTool({ + host: "gateway", + ask: "on-miss", + security: "allowlist", + approvalRunningNoticeMs: 0, + }); + + const result = await tool.execute("call-chain-gateway", { + command: "npm view diver --json | jq .name && brew outdated", + }); + + expect(result.details.status).toBe("approval-pending"); + const pendingText = result.content.find((part) => part.type === "text")?.text ?? ""; + expect(pendingText).toContain( + "Command:\n```sh\nnpm view diver --json | jq .name && brew outdated\n```", + ); + expect(calls).toContain("exec.approval.request"); + }); + + it("shows full chained node commands in approval-pending message", async () => { + const calls: string[] = []; + vi.mocked(callGatewayTool).mockImplementation(async (method, _opts, params) => { + calls.push(method); + if (method === "node.invoke") { + const invoke = params as { command?: string }; + if (invoke.command === "system.run.prepare") { + return buildPreparedSystemRunPayload(params); + } + } + return { ok: true }; + }); + + const tool = createExecTool({ + host: "node", + ask: "always", + security: "full", + approvalRunningNoticeMs: 0, + }); + + const result = await tool.execute("call-chain-node", { + command: "npm view diver --json | jq .name && brew outdated", + }); + + expect(result.details.status).toBe("approval-pending"); + const pendingText = result.content.find((part) => part.type === "text")?.text ?? ""; + expect(pendingText).toContain( + "Command:\n```sh\nnpm view diver --json | jq .name && brew outdated\n```", + ); + expect(calls).toContain("exec.approval.request"); + }); + it("waits for approval registration before returning approval-pending", async () => { const calls: string[] = []; let resolveRegistration: ((value: unknown) => void) | undefined; @@ -354,6 +550,111 @@ describe("exec approvals", () => { ); }); + it("returns an unavailable approval message instead of a local /approve prompt when discord exec approvals are disabled", async () => { + const configPath = path.join(process.env.HOME ?? "", ".openclaw", "openclaw.json"); + await fs.mkdir(path.dirname(configPath), { recursive: true }); + await fs.writeFile( + configPath, + JSON.stringify({ + channels: { + discord: { + enabled: true, + execApprovals: { enabled: false }, + }, + }, + }), + ); + + vi.mocked(callGatewayTool).mockImplementation(async (method) => { + if (method === "exec.approval.request") { + return { status: "accepted", id: "approval-id" }; + } + if (method === "exec.approval.waitDecision") { + return { decision: null }; + } + return { ok: true }; + }); + + const tool = createExecTool({ + host: "gateway", + ask: "always", + approvalRunningNoticeMs: 0, + messageProvider: "discord", + accountId: "default", + currentChannelId: "1234567890", + }); + + const result = await tool.execute("call-unavailable", { + command: "npm view diver name version description", + }); + + expect(result.details.status).toBe("approval-unavailable"); + const text = result.content.find((part) => part.type === "text")?.text ?? ""; + expect(text).toContain("chat exec approvals are not enabled on Discord"); + expect(text).toContain("Web UI or terminal UI"); + expect(text).not.toContain("/approve"); + expect(text).not.toContain("npm view diver name version description"); + expect(text).not.toContain("Pending command:"); + expect(text).not.toContain("Host:"); + expect(text).not.toContain("CWD:"); + }); + + it("tells Telegram users that allowed approvers were DMed when Telegram approvals are disabled but Discord DM approvals are enabled", async () => { + const configPath = path.join(process.env.HOME ?? "", ".openclaw", "openclaw.json"); + await fs.mkdir(path.dirname(configPath), { recursive: true }); + await fs.writeFile( + configPath, + JSON.stringify( + { + channels: { + telegram: { + enabled: true, + execApprovals: { enabled: false }, + }, + discord: { + enabled: true, + execApprovals: { enabled: true, approvers: ["123"], target: "dm" }, + }, + }, + }, + null, + 2, + ), + ); + + vi.mocked(callGatewayTool).mockImplementation(async (method) => { + if (method === "exec.approval.request") { + return { status: "accepted", id: "approval-id" }; + } + if (method === "exec.approval.waitDecision") { + return { decision: null }; + } + return { ok: true }; + }); + + const tool = createExecTool({ + host: "gateway", + ask: "always", + approvalRunningNoticeMs: 0, + messageProvider: "telegram", + accountId: "default", + currentChannelId: "-1003841603622", + }); + + const result = await tool.execute("call-tg-unavailable", { + command: "npm view diver name version description", + }); + + expect(result.details.status).toBe("approval-unavailable"); + const text = result.content.find((part) => part.type === "text")?.text ?? ""; + expect(text).toContain("Approval required. I sent the allowed approvers DMs."); + expect(text).not.toContain("/approve"); + expect(text).not.toContain("npm view diver name version description"); + expect(text).not.toContain("Pending command:"); + expect(text).not.toContain("Host:"); + expect(text).not.toContain("CWD:"); + }); + it("denies node obfuscated command when approval request times out", async () => { vi.mocked(detectCommandObfuscation).mockReturnValue({ detected: true, diff --git a/src/agents/pi-embedded-runner/run.ts b/src/agents/pi-embedded-runner/run.ts index 381c76ada18..298bac9fe9e 100644 --- a/src/agents/pi-embedded-runner/run.ts +++ b/src/agents/pi-embedded-runner/run.ts @@ -1457,6 +1457,7 @@ export async function runEmbeddedPiAgent( suppressToolErrorWarnings: params.suppressToolErrorWarnings, inlineToolResultsAllowed: false, didSendViaMessagingTool: attempt.didSendViaMessagingTool, + didSendDeterministicApprovalPrompt: attempt.didSendDeterministicApprovalPrompt, }); // Timeout aborts can leave the run without any assistant payloads. @@ -1479,6 +1480,7 @@ export async function runEmbeddedPiAgent( systemPromptReport: attempt.systemPromptReport, }, didSendViaMessagingTool: attempt.didSendViaMessagingTool, + didSendDeterministicApprovalPrompt: attempt.didSendDeterministicApprovalPrompt, messagingToolSentTexts: attempt.messagingToolSentTexts, messagingToolSentMediaUrls: attempt.messagingToolSentMediaUrls, messagingToolSentTargets: attempt.messagingToolSentTargets, @@ -1526,6 +1528,7 @@ export async function runEmbeddedPiAgent( : undefined, }, didSendViaMessagingTool: attempt.didSendViaMessagingTool, + didSendDeterministicApprovalPrompt: attempt.didSendDeterministicApprovalPrompt, messagingToolSentTexts: attempt.messagingToolSentTexts, messagingToolSentMediaUrls: attempt.messagingToolSentMediaUrls, messagingToolSentTargets: attempt.messagingToolSentTargets, diff --git a/src/agents/pi-embedded-runner/run/attempt.ts b/src/agents/pi-embedded-runner/run/attempt.ts index d7fa541c2be..25f13c666c7 100644 --- a/src/agents/pi-embedded-runner/run/attempt.ts +++ b/src/agents/pi-embedded-runner/run/attempt.ts @@ -1544,6 +1544,7 @@ export async function runEmbeddedAttempt( getMessagingToolSentTargets, getSuccessfulCronAdds, didSendViaMessagingTool, + didSendDeterministicApprovalPrompt, getLastToolError, getUsageTotals, getCompactionCount, @@ -2058,6 +2059,7 @@ export async function runEmbeddedAttempt( lastAssistant, lastToolError: getLastToolError?.(), didSendViaMessagingTool: didSendViaMessagingTool(), + didSendDeterministicApprovalPrompt: didSendDeterministicApprovalPrompt(), messagingToolSentTexts: getMessagingToolSentTexts(), messagingToolSentMediaUrls: getMessagingToolSentMediaUrls(), messagingToolSentTargets: getMessagingToolSentTargets(), diff --git a/src/agents/pi-embedded-runner/run/params.ts b/src/agents/pi-embedded-runner/run/params.ts index 6d067c910bf..ee743d7a0c1 100644 --- a/src/agents/pi-embedded-runner/run/params.ts +++ b/src/agents/pi-embedded-runner/run/params.ts @@ -1,5 +1,6 @@ import type { ImageContent } from "@mariozechner/pi-ai"; import type { ReasoningLevel, ThinkLevel, VerboseLevel } from "../../../auto-reply/thinking.js"; +import type { ReplyPayload } from "../../../auto-reply/types.js"; import type { AgentStreamParams } from "../../../commands/agent/types.js"; import type { OpenClawConfig } from "../../../config/config.js"; import type { enqueueCommand } from "../../../process/command-queue.js"; @@ -104,7 +105,7 @@ export type RunEmbeddedPiAgentParams = { blockReplyChunking?: BlockReplyChunking; onReasoningStream?: (payload: { text?: string; mediaUrls?: string[] }) => void | Promise; onReasoningEnd?: () => void | Promise; - onToolResult?: (payload: { text?: string; mediaUrls?: string[] }) => void | Promise; + onToolResult?: (payload: ReplyPayload) => void | Promise; onAgentEvent?: (evt: { stream: string; data: Record }) => void; lane?: string; enqueue?: typeof enqueueCommand; diff --git a/src/agents/pi-embedded-runner/run/payloads.test.ts b/src/agents/pi-embedded-runner/run/payloads.test.ts index ee8acd1d43e..6c81fb12150 100644 --- a/src/agents/pi-embedded-runner/run/payloads.test.ts +++ b/src/agents/pi-embedded-runner/run/payloads.test.ts @@ -82,4 +82,13 @@ describe("buildEmbeddedRunPayloads tool-error warnings", () => { expect(payloads).toHaveLength(0); }); + + it("suppresses assistant text when a deterministic exec approval prompt was already delivered", () => { + const payloads = buildPayloads({ + assistantTexts: ["Approval is needed. Please run /approve abc allow-once"], + didSendDeterministicApprovalPrompt: true, + }); + + expect(payloads).toHaveLength(0); + }); }); diff --git a/src/agents/pi-embedded-runner/run/payloads.ts b/src/agents/pi-embedded-runner/run/payloads.ts index c3c87845451..16a78ec2e97 100644 --- a/src/agents/pi-embedded-runner/run/payloads.ts +++ b/src/agents/pi-embedded-runner/run/payloads.ts @@ -102,6 +102,7 @@ export function buildEmbeddedRunPayloads(params: { suppressToolErrorWarnings?: boolean; inlineToolResultsAllowed: boolean; didSendViaMessagingTool?: boolean; + didSendDeterministicApprovalPrompt?: boolean; }): Array<{ text?: string; mediaUrl?: string; @@ -125,14 +126,17 @@ export function buildEmbeddedRunPayloads(params: { }> = []; const useMarkdown = params.toolResultFormat === "markdown"; + const suppressAssistantArtifacts = params.didSendDeterministicApprovalPrompt === true; const lastAssistantErrored = params.lastAssistant?.stopReason === "error"; const errorText = params.lastAssistant - ? formatAssistantErrorText(params.lastAssistant, { - cfg: params.config, - sessionKey: params.sessionKey, - provider: params.provider, - model: params.model, - }) + ? suppressAssistantArtifacts + ? undefined + : formatAssistantErrorText(params.lastAssistant, { + cfg: params.config, + sessionKey: params.sessionKey, + provider: params.provider, + model: params.model, + }) : undefined; const rawErrorMessage = lastAssistantErrored ? params.lastAssistant?.errorMessage?.trim() || undefined @@ -184,8 +188,9 @@ export function buildEmbeddedRunPayloads(params: { } } - const reasoningText = - params.lastAssistant && params.reasoningLevel === "on" + const reasoningText = suppressAssistantArtifacts + ? "" + : params.lastAssistant && params.reasoningLevel === "on" ? formatReasoningMessage(extractAssistantThinking(params.lastAssistant)) : ""; if (reasoningText) { @@ -243,13 +248,14 @@ export function buildEmbeddedRunPayloads(params: { } return isRawApiErrorPayload(trimmed); }; - const answerTexts = ( - params.assistantTexts.length - ? params.assistantTexts - : fallbackAnswerText - ? [fallbackAnswerText] - : [] - ).filter((text) => !shouldSuppressRawErrorText(text)); + const answerTexts = suppressAssistantArtifacts + ? [] + : (params.assistantTexts.length + ? params.assistantTexts + : fallbackAnswerText + ? [fallbackAnswerText] + : [] + ).filter((text) => !shouldSuppressRawErrorText(text)); let hasUserFacingAssistantReply = false; for (const text of answerTexts) { diff --git a/src/agents/pi-embedded-runner/run/types.ts b/src/agents/pi-embedded-runner/run/types.ts index dff5aa6f251..7e6ad0578f1 100644 --- a/src/agents/pi-embedded-runner/run/types.ts +++ b/src/agents/pi-embedded-runner/run/types.ts @@ -54,6 +54,7 @@ export type EmbeddedRunAttemptResult = { actionFingerprint?: string; }; didSendViaMessagingTool: boolean; + didSendDeterministicApprovalPrompt?: boolean; messagingToolSentTexts: string[]; messagingToolSentMediaUrls: string[]; messagingToolSentTargets: MessagingToolSend[]; diff --git a/src/agents/pi-embedded-subscribe.handlers.messages.ts b/src/agents/pi-embedded-subscribe.handlers.messages.ts index c89a4b71496..04f47e67cde 100644 --- a/src/agents/pi-embedded-subscribe.handlers.messages.ts +++ b/src/agents/pi-embedded-subscribe.handlers.messages.ts @@ -85,6 +85,9 @@ export function handleMessageUpdate( } ctx.noteLastAssistant(msg); + if (ctx.state.deterministicApprovalPromptSent) { + return; + } const assistantEvent = evt.assistantMessageEvent; const assistantRecord = @@ -261,6 +264,9 @@ export function handleMessageEnd( const assistantMessage = msg; ctx.noteLastAssistant(assistantMessage); ctx.recordAssistantUsage((assistantMessage as { usage?: unknown }).usage); + if (ctx.state.deterministicApprovalPromptSent) { + return; + } promoteThinkingTagsToBlocks(assistantMessage); const rawText = extractAssistantText(assistantMessage); diff --git a/src/agents/pi-embedded-subscribe.handlers.tools.media.test.ts b/src/agents/pi-embedded-subscribe.handlers.tools.media.test.ts index 741fa96c815..66685f04036 100644 --- a/src/agents/pi-embedded-subscribe.handlers.tools.media.test.ts +++ b/src/agents/pi-embedded-subscribe.handlers.tools.media.test.ts @@ -28,6 +28,7 @@ function createMockContext(overrides?: { messagingToolSentTextsNormalized: [], messagingToolSentMediaUrls: [], messagingToolSentTargets: [], + deterministicApprovalPromptSent: false, }, log: { debug: vi.fn(), warn: vi.fn() }, shouldEmitToolResult: vi.fn(() => false), diff --git a/src/agents/pi-embedded-subscribe.handlers.tools.test.ts b/src/agents/pi-embedded-subscribe.handlers.tools.test.ts index 96a988e5bc6..3cf7935a8a2 100644 --- a/src/agents/pi-embedded-subscribe.handlers.tools.test.ts +++ b/src/agents/pi-embedded-subscribe.handlers.tools.test.ts @@ -45,6 +45,7 @@ function createTestContext(): { messagingToolSentMediaUrls: [], messagingToolSentTargets: [], successfulCronAdds: 0, + deterministicApprovalPromptSent: false, }, shouldEmitToolResult: () => false, shouldEmitToolOutput: () => false, @@ -175,6 +176,161 @@ describe("handleToolExecutionEnd cron.add commitment tracking", () => { }); }); +describe("handleToolExecutionEnd exec approval prompts", () => { + it("emits a deterministic approval payload and marks assistant output suppressed", async () => { + const { ctx } = createTestContext(); + const onToolResult = vi.fn(); + ctx.params.onToolResult = onToolResult; + + await handleToolExecutionEnd( + ctx as never, + { + type: "tool_execution_end", + toolName: "exec", + toolCallId: "tool-exec-approval", + isError: false, + result: { + details: { + status: "approval-pending", + approvalId: "12345678-1234-1234-1234-123456789012", + approvalSlug: "12345678", + expiresAtMs: 1_800_000_000_000, + host: "gateway", + command: "npm view diver name version description", + cwd: "/tmp/work", + warningText: "Warning: heredoc execution requires explicit approval in allowlist mode.", + }, + }, + } as never, + ); + + expect(onToolResult).toHaveBeenCalledWith( + expect.objectContaining({ + text: expect.stringContaining("```txt\n/approve 12345678 allow-once\n```"), + channelData: { + execApproval: { + approvalId: "12345678-1234-1234-1234-123456789012", + approvalSlug: "12345678", + allowedDecisions: ["allow-once", "allow-always", "deny"], + }, + }, + }), + ); + expect(ctx.state.deterministicApprovalPromptSent).toBe(true); + }); + + it("emits a deterministic unavailable payload when the initiating surface cannot approve", async () => { + const { ctx } = createTestContext(); + const onToolResult = vi.fn(); + ctx.params.onToolResult = onToolResult; + + await handleToolExecutionEnd( + ctx as never, + { + type: "tool_execution_end", + toolName: "exec", + toolCallId: "tool-exec-unavailable", + isError: false, + result: { + details: { + status: "approval-unavailable", + reason: "initiating-platform-disabled", + channelLabel: "Discord", + }, + }, + } as never, + ); + + expect(onToolResult).toHaveBeenCalledWith( + expect.objectContaining({ + text: expect.stringContaining("chat exec approvals are not enabled on Discord"), + }), + ); + expect(onToolResult).toHaveBeenCalledWith( + expect.objectContaining({ + text: expect.not.stringContaining("/approve"), + }), + ); + expect(onToolResult).toHaveBeenCalledWith( + expect.objectContaining({ + text: expect.not.stringContaining("Pending command:"), + }), + ); + expect(onToolResult).toHaveBeenCalledWith( + expect.objectContaining({ + text: expect.not.stringContaining("Host:"), + }), + ); + expect(onToolResult).toHaveBeenCalledWith( + expect.objectContaining({ + text: expect.not.stringContaining("CWD:"), + }), + ); + expect(ctx.state.deterministicApprovalPromptSent).toBe(true); + }); + + it("emits the shared approver-DM notice when another approval client received the request", async () => { + const { ctx } = createTestContext(); + const onToolResult = vi.fn(); + ctx.params.onToolResult = onToolResult; + + await handleToolExecutionEnd( + ctx as never, + { + type: "tool_execution_end", + toolName: "exec", + toolCallId: "tool-exec-unavailable-dm-redirect", + isError: false, + result: { + details: { + status: "approval-unavailable", + reason: "initiating-platform-disabled", + channelLabel: "Telegram", + sentApproverDms: true, + }, + }, + } as never, + ); + + expect(onToolResult).toHaveBeenCalledWith( + expect.objectContaining({ + text: "Approval required. I sent the allowed approvers DMs.", + }), + ); + expect(ctx.state.deterministicApprovalPromptSent).toBe(true); + }); + + it("does not suppress assistant output when deterministic prompt delivery rejects", async () => { + const { ctx } = createTestContext(); + ctx.params.onToolResult = vi.fn(async () => { + throw new Error("delivery failed"); + }); + + await handleToolExecutionEnd( + ctx as never, + { + type: "tool_execution_end", + toolName: "exec", + toolCallId: "tool-exec-approval-reject", + isError: false, + result: { + details: { + status: "approval-pending", + approvalId: "12345678-1234-1234-1234-123456789012", + approvalSlug: "12345678", + expiresAtMs: 1_800_000_000_000, + host: "gateway", + command: "npm view diver name version description", + cwd: "/tmp/work", + }, + }, + } as never, + ); + + expect(ctx.state.deterministicApprovalPromptSent).toBe(false); + }); +}); + describe("messaging tool media URL tracking", () => { it("tracks media arg from messaging tool as pending", async () => { const { ctx } = createTestContext(); diff --git a/src/agents/pi-embedded-subscribe.handlers.tools.ts b/src/agents/pi-embedded-subscribe.handlers.tools.ts index 8abd9469bbc..70f6b54639c 100644 --- a/src/agents/pi-embedded-subscribe.handlers.tools.ts +++ b/src/agents/pi-embedded-subscribe.handlers.tools.ts @@ -1,5 +1,9 @@ import type { AgentEvent } from "@mariozechner/pi-agent-core"; import { emitAgentEvent } from "../infra/agent-events.js"; +import { + buildExecApprovalPendingReplyPayload, + buildExecApprovalUnavailableReplyPayload, +} from "../infra/exec-approval-reply.js"; import { getGlobalHookRunner } from "../plugins/hook-runner-global.js"; import type { PluginHookAfterToolCallEvent } from "../plugins/types.js"; import { normalizeTextForComparison } from "./pi-embedded-helpers.js"; @@ -139,7 +143,81 @@ function collectMessagingMediaUrlsFromToolResult(result: unknown): string[] { return urls; } -function emitToolResultOutput(params: { +function readExecApprovalPendingDetails(result: unknown): { + approvalId: string; + approvalSlug: string; + expiresAtMs?: number; + host: "gateway" | "node"; + command: string; + cwd?: string; + nodeId?: string; + warningText?: string; +} | null { + if (!result || typeof result !== "object") { + return null; + } + const outer = result as Record; + const details = + outer.details && typeof outer.details === "object" && !Array.isArray(outer.details) + ? (outer.details as Record) + : outer; + if (details.status !== "approval-pending") { + return null; + } + const approvalId = typeof details.approvalId === "string" ? details.approvalId.trim() : ""; + const approvalSlug = typeof details.approvalSlug === "string" ? details.approvalSlug.trim() : ""; + const command = typeof details.command === "string" ? details.command : ""; + const host = details.host === "node" ? "node" : details.host === "gateway" ? "gateway" : null; + if (!approvalId || !approvalSlug || !command || !host) { + return null; + } + return { + approvalId, + approvalSlug, + expiresAtMs: typeof details.expiresAtMs === "number" ? details.expiresAtMs : undefined, + host, + command, + cwd: typeof details.cwd === "string" ? details.cwd : undefined, + nodeId: typeof details.nodeId === "string" ? details.nodeId : undefined, + warningText: typeof details.warningText === "string" ? details.warningText : undefined, + }; +} + +function readExecApprovalUnavailableDetails(result: unknown): { + reason: "initiating-platform-disabled" | "initiating-platform-unsupported" | "no-approval-route"; + warningText?: string; + channelLabel?: string; + sentApproverDms?: boolean; +} | null { + if (!result || typeof result !== "object") { + return null; + } + const outer = result as Record; + const details = + outer.details && typeof outer.details === "object" && !Array.isArray(outer.details) + ? (outer.details as Record) + : outer; + if (details.status !== "approval-unavailable") { + return null; + } + const reason = + details.reason === "initiating-platform-disabled" || + details.reason === "initiating-platform-unsupported" || + details.reason === "no-approval-route" + ? details.reason + : null; + if (!reason) { + return null; + } + return { + reason, + warningText: typeof details.warningText === "string" ? details.warningText : undefined, + channelLabel: typeof details.channelLabel === "string" ? details.channelLabel : undefined, + sentApproverDms: details.sentApproverDms === true, + }; +} + +async function emitToolResultOutput(params: { ctx: ToolHandlerContext; toolName: string; meta?: string; @@ -152,6 +230,46 @@ function emitToolResultOutput(params: { return; } + const approvalPending = readExecApprovalPendingDetails(result); + if (!isToolError && approvalPending) { + try { + await ctx.params.onToolResult( + buildExecApprovalPendingReplyPayload({ + approvalId: approvalPending.approvalId, + approvalSlug: approvalPending.approvalSlug, + command: approvalPending.command, + cwd: approvalPending.cwd, + host: approvalPending.host, + nodeId: approvalPending.nodeId, + expiresAtMs: approvalPending.expiresAtMs, + warningText: approvalPending.warningText, + }), + ); + ctx.state.deterministicApprovalPromptSent = true; + } catch { + // ignore delivery failures + } + return; + } + + const approvalUnavailable = readExecApprovalUnavailableDetails(result); + if (!isToolError && approvalUnavailable) { + try { + await ctx.params.onToolResult?.( + buildExecApprovalUnavailableReplyPayload({ + reason: approvalUnavailable.reason, + warningText: approvalUnavailable.warningText, + channelLabel: approvalUnavailable.channelLabel, + sentApproverDms: approvalUnavailable.sentApproverDms, + }), + ); + ctx.state.deterministicApprovalPromptSent = true; + } catch { + // ignore delivery failures + } + return; + } + if (ctx.shouldEmitToolOutput()) { const outputText = extractToolResultText(sanitizedResult); if (outputText) { @@ -427,7 +545,7 @@ export async function handleToolExecutionEnd( `embedded run tool end: runId=${ctx.params.runId} tool=${toolName} toolCallId=${toolCallId}`, ); - emitToolResultOutput({ ctx, toolName, meta, isToolError, result, sanitizedResult }); + await emitToolResultOutput({ ctx, toolName, meta, isToolError, result, sanitizedResult }); // Run after_tool_call plugin hook (fire-and-forget) const hookRunnerAfter = ctx.hookRunner ?? getGlobalHookRunner(); diff --git a/src/agents/pi-embedded-subscribe.handlers.types.ts b/src/agents/pi-embedded-subscribe.handlers.types.ts index 955af473b9e..4436e6f6aa3 100644 --- a/src/agents/pi-embedded-subscribe.handlers.types.ts +++ b/src/agents/pi-embedded-subscribe.handlers.types.ts @@ -76,6 +76,7 @@ export type EmbeddedPiSubscribeState = { pendingMessagingTargets: Map; successfulCronAdds: number; pendingMessagingMediaUrls: Map; + deterministicApprovalPromptSent: boolean; lastAssistant?: AgentMessage; }; @@ -155,6 +156,7 @@ export type ToolHandlerState = Pick< | "messagingToolSentMediaUrls" | "messagingToolSentTargets" | "successfulCronAdds" + | "deterministicApprovalPromptSent" >; export type ToolHandlerContext = { diff --git a/src/agents/pi-embedded-subscribe.ts b/src/agents/pi-embedded-subscribe.ts index c5ffedbf14f..83592372e80 100644 --- a/src/agents/pi-embedded-subscribe.ts +++ b/src/agents/pi-embedded-subscribe.ts @@ -78,6 +78,7 @@ export function subscribeEmbeddedPiSession(params: SubscribeEmbeddedPiSessionPar pendingMessagingTargets: new Map(), successfulCronAdds: 0, pendingMessagingMediaUrls: new Map(), + deterministicApprovalPromptSent: false, }; const usageTotals = { input: 0, @@ -598,6 +599,7 @@ export function subscribeEmbeddedPiSession(params: SubscribeEmbeddedPiSessionPar pendingMessagingTargets.clear(); state.successfulCronAdds = 0; state.pendingMessagingMediaUrls.clear(); + state.deterministicApprovalPromptSent = false; resetAssistantMessageState(0); }; @@ -688,6 +690,7 @@ export function subscribeEmbeddedPiSession(params: SubscribeEmbeddedPiSessionPar // Used to suppress agent's confirmation text (e.g., "Respondi no Telegram!") // which is generated AFTER the tool sends the actual answer. didSendViaMessagingTool: () => messagingToolSentTexts.length > 0, + didSendDeterministicApprovalPrompt: () => state.deterministicApprovalPromptSent, getLastToolError: () => (state.lastToolError ? { ...state.lastToolError } : undefined), getUsageTotals, getCompactionCount: () => compactionCount, diff --git a/src/agents/pi-embedded-subscribe.types.ts b/src/agents/pi-embedded-subscribe.types.ts index 689cd49998e..bbb2d552d73 100644 --- a/src/agents/pi-embedded-subscribe.types.ts +++ b/src/agents/pi-embedded-subscribe.types.ts @@ -1,5 +1,6 @@ import type { AgentSession } from "@mariozechner/pi-coding-agent"; import type { ReasoningLevel, VerboseLevel } from "../auto-reply/thinking.js"; +import type { ReplyPayload } from "../auto-reply/types.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { HookRunner } from "../plugins/hooks.js"; import type { BlockReplyChunking } from "./pi-embedded-block-chunker.js"; @@ -16,7 +17,7 @@ export type SubscribeEmbeddedPiSessionParams = { toolResultFormat?: ToolResultFormat; shouldEmitToolResult?: () => boolean; shouldEmitToolOutput?: () => boolean; - onToolResult?: (payload: { text?: string; mediaUrls?: string[] }) => void | Promise; + onToolResult?: (payload: ReplyPayload) => void | Promise; onReasoningStream?: (payload: { text?: string; mediaUrls?: string[] }) => void | Promise; /** Called when a thinking/reasoning block ends ( tag processed). */ onReasoningEnd?: () => void | Promise; diff --git a/src/agents/pi-tool-handler-state.test-helpers.ts b/src/agents/pi-tool-handler-state.test-helpers.ts index 0775299ab83..cfb559b9884 100644 --- a/src/agents/pi-tool-handler-state.test-helpers.ts +++ b/src/agents/pi-tool-handler-state.test-helpers.ts @@ -10,6 +10,7 @@ export function createBaseToolHandlerState() { messagingToolSentTextsNormalized: [] as string[], messagingToolSentMediaUrls: [] as string[], messagingToolSentTargets: [] as unknown[], + deterministicApprovalPromptSent: false, blockBuffer: "", }; } diff --git a/src/agents/system-prompt.ts b/src/agents/system-prompt.ts index a3d593ab6b8..848222b7880 100644 --- a/src/agents/system-prompt.ts +++ b/src/agents/system-prompt.ts @@ -464,6 +464,9 @@ export function buildAgentSystemPrompt(params: { "Keep narration brief and value-dense; avoid repeating obvious steps.", "Use plain human language for narration unless in a technical context.", "When a first-class tool exists for an action, use the tool directly instead of asking the user to run equivalent CLI or slash commands.", + "When exec returns approval-pending, include the concrete /approve command from tool output (with allow-once|allow-always|deny) and do not ask for a different or rotated code.", + "Treat allow-once as single-command only: if another elevated command needs approval, request a fresh /approve and do not claim prior approval covered it.", + "When approvals are required, preserve and show the full command/script exactly as provided (including chained operators like &&, ||, |, ;, or multiline shells) so the user can approve what will actually run.", "", ...safetySection, "## OpenClaw CLI Quick Reference", diff --git a/src/auto-reply/reply/agent-runner-execution.ts b/src/auto-reply/reply/agent-runner-execution.ts index a3b31c4ccc3..2f6c27519b0 100644 --- a/src/auto-reply/reply/agent-runner-execution.ts +++ b/src/auto-reply/reply/agent-runner-execution.ts @@ -445,8 +445,8 @@ export async function runAgentTurnWithFallback(params: { } await params.typingSignals.signalTextDelta(text); await onToolResult({ + ...payload, text, - mediaUrls: payload.mediaUrls, }); }) .catch((err) => { diff --git a/src/auto-reply/reply/agent-runner-utils.test.ts b/src/auto-reply/reply/agent-runner-utils.test.ts index 350c6b63e47..5bf77cd9f70 100644 --- a/src/auto-reply/reply/agent-runner-utils.test.ts +++ b/src/auto-reply/reply/agent-runner-utils.test.ts @@ -12,6 +12,7 @@ vi.mock("../../agents/agent-scope.js", () => ({ })); const { + buildThreadingToolContext, buildEmbeddedRunBaseParams, buildEmbeddedRunContexts, resolveModelFallbackOptions, @@ -173,4 +174,44 @@ describe("agent-runner-utils", () => { expect(resolved.embeddedContext.messageProvider).toBe("telegram"); expect(resolved.embeddedContext.messageTo).toBe("268300329"); }); + + it("uses OriginatingTo for threading tool context on telegram native commands", () => { + const context = buildThreadingToolContext({ + sessionCtx: { + Provider: "telegram", + To: "slash:8460800771", + OriginatingChannel: "telegram", + OriginatingTo: "telegram:-1003841603622", + MessageThreadId: 928, + MessageSid: "2284", + }, + config: { channels: { telegram: { allowFrom: ["*"] } } }, + hasRepliedRef: undefined, + }); + + expect(context).toMatchObject({ + currentChannelId: "telegram:-1003841603622", + currentThreadTs: "928", + currentMessageId: "2284", + }); + }); + + it("uses OriginatingTo for threading tool context on discord native commands", () => { + const context = buildThreadingToolContext({ + sessionCtx: { + Provider: "discord", + To: "slash:1177378744822943744", + OriginatingChannel: "discord", + OriginatingTo: "channel:123456789012345678", + MessageSid: "msg-9", + }, + config: {}, + hasRepliedRef: undefined, + }); + + expect(context).toMatchObject({ + currentChannelId: "channel:123456789012345678", + currentMessageId: "msg-9", + }); + }); }); diff --git a/src/auto-reply/reply/agent-runner-utils.ts b/src/auto-reply/reply/agent-runner-utils.ts index 36e45bd9bf1..99b2b6392f6 100644 --- a/src/auto-reply/reply/agent-runner-utils.ts +++ b/src/auto-reply/reply/agent-runner-utils.ts @@ -23,12 +23,20 @@ export function buildThreadingToolContext(params: { }): ChannelThreadingToolContext { const { sessionCtx, config, hasRepliedRef } = params; const currentMessageId = sessionCtx.MessageSidFull ?? sessionCtx.MessageSid; + const originProvider = resolveOriginMessageProvider({ + originatingChannel: sessionCtx.OriginatingChannel, + provider: sessionCtx.Provider, + }); + const originTo = resolveOriginMessageTo({ + originatingTo: sessionCtx.OriginatingTo, + to: sessionCtx.To, + }); if (!config) { return { currentMessageId, }; } - const rawProvider = sessionCtx.Provider?.trim().toLowerCase(); + const rawProvider = originProvider?.trim().toLowerCase(); if (!rawProvider) { return { currentMessageId, @@ -39,7 +47,7 @@ export function buildThreadingToolContext(params: { const dock = provider ? getChannelDock(provider) : undefined; if (!dock?.threading?.buildToolContext) { return { - currentChannelId: sessionCtx.To?.trim() || undefined, + currentChannelId: originTo?.trim() || undefined, currentChannelProvider: provider ?? (rawProvider as ChannelId), currentMessageId, hasRepliedRef, @@ -50,9 +58,9 @@ export function buildThreadingToolContext(params: { cfg: config, accountId: sessionCtx.AccountId, context: { - Channel: sessionCtx.Provider, + Channel: originProvider, From: sessionCtx.From, - To: sessionCtx.To, + To: originTo, ChatType: sessionCtx.ChatType, CurrentMessageId: currentMessageId, ReplyToId: sessionCtx.ReplyToId, diff --git a/src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts b/src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts index 83c1796515c..db034ac03a6 100644 --- a/src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts +++ b/src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts @@ -21,7 +21,7 @@ type AgentRunParams = { onAssistantMessageStart?: () => Promise | void; onReasoningStream?: (payload: { text?: string }) => Promise | void; onBlockReply?: (payload: { text?: string; mediaUrls?: string[] }) => Promise | void; - onToolResult?: (payload: { text?: string; mediaUrls?: string[] }) => Promise | void; + onToolResult?: (payload: ReplyPayload) => Promise | void; onAgentEvent?: (evt: { stream: string; data: Record }) => void; }; @@ -594,6 +594,40 @@ describe("runReplyAgent typing (heartbeat)", () => { } }); + it("preserves channelData on forwarded tool results", async () => { + const onToolResult = vi.fn(); + state.runEmbeddedPiAgentMock.mockImplementationOnce(async (params: AgentRunParams) => { + await params.onToolResult?.({ + text: "Approval required.\n\n```txt\n/approve 117ba06d allow-once\n```", + channelData: { + execApproval: { + approvalId: "117ba06d-1111-2222-3333-444444444444", + approvalSlug: "117ba06d", + allowedDecisions: ["allow-once", "allow-always", "deny"], + }, + }, + }); + return { payloads: [{ text: "final" }], meta: {} }; + }); + + const { run } = createMinimalRun({ + typingMode: "message", + opts: { onToolResult }, + }); + await run(); + + expect(onToolResult).toHaveBeenCalledWith({ + text: "Approval required.\n\n```txt\n/approve 117ba06d allow-once\n```", + channelData: { + execApproval: { + approvalId: "117ba06d-1111-2222-3333-444444444444", + approvalSlug: "117ba06d", + allowedDecisions: ["allow-once", "allow-always", "deny"], + }, + }, + }); + }); + it("retries transient HTTP failures once with timer-driven backoff", async () => { vi.useFakeTimers(); let calls = 0; @@ -1952,3 +1986,4 @@ describe("runReplyAgent memory flush", () => { }); }); }); +import type { ReplyPayload } from "../types.js"; diff --git a/src/auto-reply/reply/commands-approve.ts b/src/auto-reply/reply/commands-approve.ts index 9773ba03ad5..5b0caec9c8f 100644 --- a/src/auto-reply/reply/commands-approve.ts +++ b/src/auto-reply/reply/commands-approve.ts @@ -1,10 +1,15 @@ import { callGateway } from "../../gateway/call.js"; import { logVerbose } from "../../globals.js"; +import { + isTelegramExecApprovalApprover, + isTelegramExecApprovalClientEnabled, +} from "../../telegram/exec-approvals.js"; import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../../utils/message-channel.js"; import { requireGatewayClientScopeForInternalChannel } from "./command-gates.js"; import type { CommandHandler } from "./commands-types.js"; -const COMMAND = "/approve"; +const COMMAND_REGEX = /^\/approve(?:\s|$)/i; +const FOREIGN_COMMAND_MENTION_REGEX = /^\/approve@([^\s]+)(?:\s|$)/i; const DECISION_ALIASES: Record = { allow: "allow-once", @@ -25,10 +30,14 @@ type ParsedApproveCommand = function parseApproveCommand(raw: string): ParsedApproveCommand | null { const trimmed = raw.trim(); - if (!trimmed.toLowerCase().startsWith(COMMAND)) { + if (FOREIGN_COMMAND_MENTION_REGEX.test(trimmed)) { + return { ok: false, error: "❌ This /approve command targets a different Telegram bot." }; + } + const commandMatch = trimmed.match(COMMAND_REGEX); + if (!commandMatch) { return null; } - const rest = trimmed.slice(COMMAND.length).trim(); + const rest = trimmed.slice(commandMatch[0].length).trim(); if (!rest) { return { ok: false, error: "Usage: /approve allow-once|allow-always|deny" }; } @@ -83,6 +92,29 @@ export const handleApproveCommand: CommandHandler = async (params, allowTextComm return { shouldContinue: false, reply: { text: parsed.error } }; } + if (params.command.channel === "telegram") { + if ( + !isTelegramExecApprovalClientEnabled({ cfg: params.cfg, accountId: params.ctx.AccountId }) + ) { + return { + shouldContinue: false, + reply: { text: "❌ Telegram exec approvals are not enabled for this bot account." }, + }; + } + if ( + !isTelegramExecApprovalApprover({ + cfg: params.cfg, + accountId: params.ctx.AccountId, + senderId: params.command.senderId, + }) + ) { + return { + shouldContinue: false, + reply: { text: "❌ You are not authorized to approve exec requests on Telegram." }, + }; + } + } + const missingScope = requireGatewayClientScopeForInternalChannel(params, { label: "/approve", allowedScopes: ["operator.approvals", "operator.admin"], diff --git a/src/auto-reply/reply/commands-context.ts b/src/auto-reply/reply/commands-context.ts index 3d177c2b5f9..1c5056b4b46 100644 --- a/src/auto-reply/reply/commands-context.ts +++ b/src/auto-reply/reply/commands-context.ts @@ -26,6 +26,7 @@ export function buildCommandContext(params: { const rawBodyNormalized = triggerBodyNormalized; const commandBodyNormalized = normalizeCommandBody( isGroup ? stripMentions(rawBodyNormalized, ctx, cfg, agentId) : rawBodyNormalized, + { botUsername: ctx.BotUsername }, ); return { diff --git a/src/auto-reply/reply/commands.test.ts b/src/auto-reply/reply/commands.test.ts index 38be7c43531..0f526d6edaa 100644 --- a/src/auto-reply/reply/commands.test.ts +++ b/src/auto-reply/reply/commands.test.ts @@ -105,27 +105,6 @@ vi.mock("../../gateway/call.js", () => ({ callGateway: (opts: unknown) => callGatewayMock(opts), })); -type ResetAcpSessionInPlaceResult = { ok: true } | { ok: false; skipped?: boolean; error?: string }; - -const resetAcpSessionInPlaceMock = vi.hoisted(() => - vi.fn( - async (_params: unknown): Promise => ({ - ok: false, - skipped: true, - }), - ), -); -vi.mock("../../acp/persistent-bindings.js", async () => { - const actual = await vi.importActual( - "../../acp/persistent-bindings.js", - ); - return { - ...actual, - resetAcpSessionInPlace: (params: unknown) => resetAcpSessionInPlaceMock(params), - }; -}); - -import { buildConfiguredAcpSessionKey } from "../../acp/persistent-bindings.js"; import type { HandleCommandsParams } from "./commands-types.js"; import { buildCommandContext, handleCommands } from "./commands.js"; @@ -158,11 +137,6 @@ function buildParams(commandBody: string, cfg: OpenClawConfig, ctxOverrides?: Pa return buildCommandTestParams(commandBody, cfg, ctxOverrides, { workspaceDir: testWorkspaceDir }); } -beforeEach(() => { - resetAcpSessionInPlaceMock.mockReset(); - resetAcpSessionInPlaceMock.mockResolvedValue({ ok: false, skipped: true } as const); -}); - describe("handleCommands gating", () => { it("blocks gated commands when disabled or not elevated-allowlisted", async () => { const cases = typedCases<{ @@ -316,6 +290,122 @@ describe("/approve command", () => { ); }); + it("accepts Telegram command mentions for /approve", async () => { + const cfg = { + commands: { text: true }, + channels: { + telegram: { + allowFrom: ["*"], + execApprovals: { enabled: true, approvers: ["123"], target: "dm" }, + }, + }, + } as OpenClawConfig; + const params = buildParams("/approve@bot abc12345 allow-once", cfg, { + BotUsername: "bot", + Provider: "telegram", + Surface: "telegram", + SenderId: "123", + }); + + callGatewayMock.mockResolvedValue({ ok: true }); + + const result = await handleCommands(params); + expect(result.shouldContinue).toBe(false); + expect(result.reply?.text).toContain("Exec approval allow-once submitted"); + expect(callGatewayMock).toHaveBeenCalledWith( + expect.objectContaining({ + method: "exec.approval.resolve", + params: { id: "abc12345", decision: "allow-once" }, + }), + ); + }); + + it("rejects Telegram /approve mentions targeting a different bot", async () => { + const cfg = { + commands: { text: true }, + channels: { + telegram: { + allowFrom: ["*"], + execApprovals: { enabled: true, approvers: ["123"], target: "dm" }, + }, + }, + } as OpenClawConfig; + const params = buildParams("/approve@otherbot abc12345 allow-once", cfg, { + BotUsername: "bot", + Provider: "telegram", + Surface: "telegram", + SenderId: "123", + }); + + const result = await handleCommands(params); + + expect(result.shouldContinue).toBe(false); + expect(result.reply?.text).toContain("targets a different Telegram bot"); + expect(callGatewayMock).not.toHaveBeenCalled(); + }); + + it("surfaces unknown or expired approval id errors", async () => { + const cfg = { + commands: { text: true }, + channels: { + telegram: { + allowFrom: ["*"], + execApprovals: { enabled: true, approvers: ["123"], target: "dm" }, + }, + }, + } as OpenClawConfig; + const params = buildParams("/approve abc12345 allow-once", cfg, { + Provider: "telegram", + Surface: "telegram", + SenderId: "123", + }); + + callGatewayMock.mockRejectedValue(new Error("unknown or expired approval id")); + + const result = await handleCommands(params); + expect(result.shouldContinue).toBe(false); + expect(result.reply?.text).toContain("unknown or expired approval id"); + }); + + it("rejects Telegram /approve when telegram exec approvals are disabled", async () => { + const cfg = { + commands: { text: true }, + channels: { telegram: { allowFrom: ["*"] } }, + } as OpenClawConfig; + const params = buildParams("/approve abc12345 allow-once", cfg, { + Provider: "telegram", + Surface: "telegram", + SenderId: "123", + }); + + const result = await handleCommands(params); + expect(result.shouldContinue).toBe(false); + expect(result.reply?.text).toContain("Telegram exec approvals are not enabled"); + expect(callGatewayMock).not.toHaveBeenCalled(); + }); + + it("rejects Telegram /approve from non-approvers", async () => { + const cfg = { + commands: { text: true }, + channels: { + telegram: { + allowFrom: ["*"], + execApprovals: { enabled: true, approvers: ["999"], target: "dm" }, + }, + }, + } as OpenClawConfig; + const params = buildParams("/approve abc12345 allow-once", cfg, { + Provider: "telegram", + Surface: "telegram", + SenderId: "123", + }); + + const result = await handleCommands(params); + expect(result.shouldContinue).toBe(false); + expect(result.reply?.text).toContain("not authorized to approve"); + expect(callGatewayMock).not.toHaveBeenCalled(); + }); + it("rejects gateway clients without approvals scope", async () => { const cfg = { commands: { text: true }, @@ -1147,226 +1237,6 @@ describe("handleCommands hooks", () => { }); }); -describe("handleCommands ACP-bound /new and /reset", () => { - const discordChannelId = "1478836151241412759"; - const buildDiscordBoundConfig = (): OpenClawConfig => - ({ - commands: { text: true }, - bindings: [ - { - type: "acp", - agentId: "codex", - match: { - channel: "discord", - accountId: "default", - peer: { - kind: "channel", - id: discordChannelId, - }, - }, - acp: { - mode: "persistent", - }, - }, - ], - channels: { - discord: { - allowFrom: ["*"], - guilds: { "1459246755253325866": { channels: { [discordChannelId]: {} } } }, - }, - }, - }) as OpenClawConfig; - - const buildDiscordBoundParams = (body: string) => { - const params = buildParams(body, buildDiscordBoundConfig(), { - Provider: "discord", - Surface: "discord", - OriginatingChannel: "discord", - AccountId: "default", - SenderId: "12345", - From: "discord:12345", - To: discordChannelId, - OriginatingTo: discordChannelId, - SessionKey: "agent:main:acp:binding:discord:default:feedface", - }); - params.sessionKey = "agent:main:acp:binding:discord:default:feedface"; - return params; - }; - - it("handles /new as ACP in-place reset for bound conversations", async () => { - resetAcpSessionInPlaceMock.mockResolvedValue({ ok: true } as const); - const result = await handleCommands(buildDiscordBoundParams("/new")); - - expect(result.shouldContinue).toBe(false); - expect(result.reply?.text).toContain("ACP session reset in place"); - expect(resetAcpSessionInPlaceMock).toHaveBeenCalledTimes(1); - expect(resetAcpSessionInPlaceMock.mock.calls[0]?.[0]).toMatchObject({ - reason: "new", - }); - }); - - it("continues with trailing prompt text after successful ACP-bound /new", async () => { - resetAcpSessionInPlaceMock.mockResolvedValue({ ok: true } as const); - const params = buildDiscordBoundParams("/new continue with deployment"); - const result = await handleCommands(params); - - expect(result.shouldContinue).toBe(false); - expect(result.reply).toBeUndefined(); - const mutableCtx = params.ctx as Record; - expect(mutableCtx.BodyStripped).toBe("continue with deployment"); - expect(mutableCtx.CommandBody).toBe("continue with deployment"); - expect(mutableCtx.AcpDispatchTailAfterReset).toBe(true); - expect(resetAcpSessionInPlaceMock).toHaveBeenCalledTimes(1); - }); - - it("handles /reset failures without falling back to normal session reset flow", async () => { - resetAcpSessionInPlaceMock.mockResolvedValue({ ok: false, error: "backend unavailable" }); - const result = await handleCommands(buildDiscordBoundParams("/reset")); - - expect(result.shouldContinue).toBe(false); - expect(result.reply?.text).toContain("ACP session reset failed"); - expect(resetAcpSessionInPlaceMock).toHaveBeenCalledTimes(1); - expect(resetAcpSessionInPlaceMock.mock.calls[0]?.[0]).toMatchObject({ - reason: "reset", - }); - }); - - it("does not emit reset hooks when ACP reset fails", async () => { - resetAcpSessionInPlaceMock.mockResolvedValue({ ok: false, error: "backend unavailable" }); - const spy = vi.spyOn(internalHooks, "triggerInternalHook").mockResolvedValue(); - - const result = await handleCommands(buildDiscordBoundParams("/reset")); - - expect(result.shouldContinue).toBe(false); - expect(spy).not.toHaveBeenCalled(); - spy.mockRestore(); - }); - - it("keeps existing /new behavior for non-ACP sessions", async () => { - const cfg = { - commands: { text: true }, - channels: { whatsapp: { allowFrom: ["*"] } }, - } as OpenClawConfig; - const result = await handleCommands(buildParams("/new", cfg)); - - expect(result.shouldContinue).toBe(true); - expect(resetAcpSessionInPlaceMock).not.toHaveBeenCalled(); - }); - - it("still targets configured ACP binding when runtime routing falls back to a non-ACP session", async () => { - const fallbackSessionKey = `agent:main:discord:channel:${discordChannelId}`; - const configuredAcpSessionKey = buildConfiguredAcpSessionKey({ - channel: "discord", - accountId: "default", - conversationId: discordChannelId, - agentId: "codex", - mode: "persistent", - }); - const params = buildDiscordBoundParams("/new"); - params.sessionKey = fallbackSessionKey; - params.ctx.SessionKey = fallbackSessionKey; - params.ctx.CommandTargetSessionKey = fallbackSessionKey; - - const result = await handleCommands(params); - - expect(result.shouldContinue).toBe(false); - expect(result.reply?.text).toContain("ACP session reset unavailable"); - expect(resetAcpSessionInPlaceMock).toHaveBeenCalledTimes(1); - expect(resetAcpSessionInPlaceMock.mock.calls[0]?.[0]).toMatchObject({ - sessionKey: configuredAcpSessionKey, - reason: "new", - }); - }); - - it("emits reset hooks for the ACP session key when routing falls back to non-ACP session", async () => { - resetAcpSessionInPlaceMock.mockResolvedValue({ ok: true } as const); - const hookSpy = vi.spyOn(internalHooks, "triggerInternalHook").mockResolvedValue(); - const fallbackSessionKey = `agent:main:discord:channel:${discordChannelId}`; - const configuredAcpSessionKey = buildConfiguredAcpSessionKey({ - channel: "discord", - accountId: "default", - conversationId: discordChannelId, - agentId: "codex", - mode: "persistent", - }); - const fallbackEntry = { - sessionId: "fallback-session-id", - sessionFile: "/tmp/fallback-session.jsonl", - } as SessionEntry; - const configuredEntry = { - sessionId: "configured-acp-session-id", - sessionFile: "/tmp/configured-acp-session.jsonl", - } as SessionEntry; - const params = buildDiscordBoundParams("/new"); - params.sessionKey = fallbackSessionKey; - params.ctx.SessionKey = fallbackSessionKey; - params.ctx.CommandTargetSessionKey = fallbackSessionKey; - params.sessionEntry = fallbackEntry; - params.previousSessionEntry = fallbackEntry; - params.sessionStore = { - [fallbackSessionKey]: fallbackEntry, - [configuredAcpSessionKey]: configuredEntry, - }; - - const result = await handleCommands(params); - - expect(result.shouldContinue).toBe(false); - expect(result.reply?.text).toContain("ACP session reset in place"); - expect(hookSpy).toHaveBeenCalledWith( - expect.objectContaining({ - type: "command", - action: "new", - sessionKey: configuredAcpSessionKey, - context: expect.objectContaining({ - sessionEntry: configuredEntry, - previousSessionEntry: configuredEntry, - }), - }), - ); - hookSpy.mockRestore(); - }); - - it("uses active ACP command target when conversation binding context is missing", async () => { - resetAcpSessionInPlaceMock.mockResolvedValue({ ok: true } as const); - const activeAcpTarget = "agent:codex:acp:binding:discord:default:feedface"; - const params = buildParams( - "/new", - { - commands: { text: true }, - channels: { - discord: { - allowFrom: ["*"], - }, - }, - } as OpenClawConfig, - { - Provider: "discord", - Surface: "discord", - OriginatingChannel: "discord", - AccountId: "default", - SenderId: "12345", - From: "discord:12345", - }, - ); - params.sessionKey = "discord:slash:12345"; - params.ctx.SessionKey = "discord:slash:12345"; - params.ctx.CommandSource = "native"; - params.ctx.CommandTargetSessionKey = activeAcpTarget; - params.ctx.To = "user:12345"; - params.ctx.OriginatingTo = "user:12345"; - - const result = await handleCommands(params); - - expect(result.shouldContinue).toBe(false); - expect(result.reply?.text).toContain("ACP session reset in place"); - expect(resetAcpSessionInPlaceMock).toHaveBeenCalledTimes(1); - expect(resetAcpSessionInPlaceMock.mock.calls[0]?.[0]).toMatchObject({ - sessionKey: activeAcpTarget, - reason: "new", - }); - }); -}); - describe("handleCommands context", () => { it("returns expected details for /context commands", async () => { const cfg = { diff --git a/src/auto-reply/reply/dispatch-from-config.test.ts b/src/auto-reply/reply/dispatch-from-config.test.ts index 982557ecb68..87e77785bbb 100644 --- a/src/auto-reply/reply/dispatch-from-config.test.ts +++ b/src/auto-reply/reply/dispatch-from-config.test.ts @@ -543,6 +543,51 @@ describe("dispatchReplyFromConfig", () => { expect(dispatcher.sendFinalReply).toHaveBeenCalledTimes(1); }); + it("delivers deterministic exec approval tool payloads in groups", async () => { + setNoAbort(); + const cfg = emptyConfig; + const dispatcher = createDispatcher(); + const ctx = buildTestCtx({ + Provider: "telegram", + ChatType: "group", + }); + + const replyResolver = async ( + _ctx: MsgContext, + opts?: GetReplyOptions, + _cfg?: OpenClawConfig, + ) => { + await opts?.onToolResult?.({ + text: "Approval required.\n\n```txt\n/approve 117ba06d allow-once\n```", + channelData: { + execApproval: { + approvalId: "117ba06d-1111-2222-3333-444444444444", + approvalSlug: "117ba06d", + allowedDecisions: ["allow-once", "allow-always", "deny"], + }, + }, + }); + return { text: "NO_REPLY" } satisfies ReplyPayload; + }; + + await dispatchReplyFromConfig({ ctx, cfg, dispatcher, replyResolver }); + + expect(dispatcher.sendToolResult).toHaveBeenCalledTimes(1); + expect(firstToolResultPayload(dispatcher)).toEqual( + expect.objectContaining({ + text: "Approval required.\n\n```txt\n/approve 117ba06d allow-once\n```", + channelData: { + execApproval: { + approvalId: "117ba06d-1111-2222-3333-444444444444", + approvalSlug: "117ba06d", + allowedDecisions: ["allow-once", "allow-always", "deny"], + }, + }, + }), + ); + expect(dispatcher.sendFinalReply).toHaveBeenCalledWith({ text: "NO_REPLY" }); + }); + it("sends tool results via dispatcher in DM sessions", async () => { setNoAbort(); const cfg = emptyConfig; @@ -601,6 +646,50 @@ describe("dispatchReplyFromConfig", () => { expect(dispatcher.sendFinalReply).toHaveBeenCalledTimes(1); }); + it("delivers deterministic exec approval tool payloads for native commands", async () => { + setNoAbort(); + const cfg = emptyConfig; + const dispatcher = createDispatcher(); + const ctx = buildTestCtx({ + Provider: "telegram", + CommandSource: "native", + }); + + const replyResolver = async ( + _ctx: MsgContext, + opts?: GetReplyOptions, + _cfg?: OpenClawConfig, + ) => { + await opts?.onToolResult?.({ + text: "Approval required.\n\n```txt\n/approve 117ba06d allow-once\n```", + channelData: { + execApproval: { + approvalId: "117ba06d-1111-2222-3333-444444444444", + approvalSlug: "117ba06d", + allowedDecisions: ["allow-once", "allow-always", "deny"], + }, + }, + }); + return { text: "NO_REPLY" } satisfies ReplyPayload; + }; + + await dispatchReplyFromConfig({ ctx, cfg, dispatcher, replyResolver }); + + expect(dispatcher.sendToolResult).toHaveBeenCalledTimes(1); + expect(firstToolResultPayload(dispatcher)).toEqual( + expect.objectContaining({ + channelData: { + execApproval: { + approvalId: "117ba06d-1111-2222-3333-444444444444", + approvalSlug: "117ba06d", + allowedDecisions: ["allow-once", "allow-always", "deny"], + }, + }, + }), + ); + expect(dispatcher.sendFinalReply).toHaveBeenCalledWith({ text: "NO_REPLY" }); + }); + it("fast-aborts without calling the reply resolver", async () => { mocks.tryFastAbortFromMessage.mockResolvedValue({ handled: true, @@ -1539,6 +1628,47 @@ describe("dispatchReplyFromConfig", () => { expect(replyResolver).toHaveBeenCalledTimes(1); }); + it("suppresses local discord exec approval tool prompts when discord approvals are enabled", async () => { + setNoAbort(); + const cfg = { + channels: { + discord: { + enabled: true, + execApprovals: { + enabled: true, + approvers: ["123"], + }, + }, + }, + } as OpenClawConfig; + const dispatcher = createDispatcher(); + const ctx = buildTestCtx({ + Provider: "discord", + Surface: "discord", + AccountId: "default", + }); + const replyResolver = vi.fn(async (_ctx: MsgContext, options?: GetReplyOptions) => { + await options?.onToolResult?.({ + text: "Approval required.", + channelData: { + execApproval: { + approvalId: "12345678-1234-1234-1234-123456789012", + approvalSlug: "12345678", + allowedDecisions: ["allow-once", "allow-always", "deny"], + }, + }, + }); + return { text: "done" } as ReplyPayload; + }); + + await dispatchReplyFromConfig({ ctx, cfg, dispatcher, replyResolver }); + + expect(dispatcher.sendToolResult).not.toHaveBeenCalled(); + expect(dispatcher.sendFinalReply).toHaveBeenCalledWith( + expect.objectContaining({ text: "done" }), + ); + }); + it("deduplicates same-agent inbound replies across main and direct session keys", async () => { setNoAbort(); const cfg = emptyConfig; diff --git a/src/auto-reply/reply/dispatch-from-config.ts b/src/auto-reply/reply/dispatch-from-config.ts index 786b1a7c16b..5b250b03362 100644 --- a/src/auto-reply/reply/dispatch-from-config.ts +++ b/src/auto-reply/reply/dispatch-from-config.ts @@ -6,6 +6,7 @@ import { resolveStorePath, type SessionEntry, } from "../../config/sessions.js"; +import { shouldSuppressLocalDiscordExecApprovalPrompt } from "../../discord/exec-approvals.js"; import { logVerbose } from "../../globals.js"; import { fireAndForgetHook } from "../../hooks/fire-and-forget.js"; import { createInternalHookEvent, triggerInternalHook } from "../../hooks/internal-hooks.js"; @@ -365,9 +366,28 @@ export async function dispatchReplyFromConfig(params: { let blockCount = 0; const resolveToolDeliveryPayload = (payload: ReplyPayload): ReplyPayload | null => { + if ( + normalizeMessageChannel(ctx.Surface ?? ctx.Provider) === "discord" && + shouldSuppressLocalDiscordExecApprovalPrompt({ + cfg, + accountId: ctx.AccountId, + payload, + }) + ) { + return null; + } if (shouldSendToolSummaries) { return payload; } + const execApproval = + payload.channelData && + typeof payload.channelData === "object" && + !Array.isArray(payload.channelData) + ? payload.channelData.execApproval + : undefined; + if (execApproval && typeof execApproval === "object" && !Array.isArray(execApproval)) { + return payload; + } // Group/native flows intentionally suppress tool summary text, but media-only // tool results (for example TTS audio) must still be delivered. const hasMedia = Boolean(payload.mediaUrl) || (payload.mediaUrls?.length ?? 0) > 0; diff --git a/src/auto-reply/templating.ts b/src/auto-reply/templating.ts index cc4fc49e93f..8ca3c2389bc 100644 --- a/src/auto-reply/templating.ts +++ b/src/auto-reply/templating.ts @@ -132,6 +132,8 @@ export type MsgContext = { Provider?: string; /** Provider surface label (e.g. discord, slack). Prefer this over `Provider` when available. */ Surface?: string; + /** Platform bot username when command mentions should be normalized. */ + BotUsername?: string; WasMentioned?: boolean; CommandAuthorized?: boolean; CommandSource?: "text" | "native"; diff --git a/src/channels/plugins/outbound/telegram.ts b/src/channels/plugins/outbound/telegram.ts index 2a079a6014e..2afc67d439d 100644 --- a/src/channels/plugins/outbound/telegram.ts +++ b/src/channels/plugins/outbound/telegram.ts @@ -115,7 +115,6 @@ export const telegramOutbound: ChannelOutboundAdapter = { quoteText, mediaLocalRoots, }; - if (mediaUrls.length === 0) { const result = await send(to, text, { ...payloadOpts, diff --git a/src/config/schema.help.quality.test.ts b/src/config/schema.help.quality.test.ts index fa9451456bf..04d5200bfbb 100644 --- a/src/config/schema.help.quality.test.ts +++ b/src/config/schema.help.quality.test.ts @@ -522,6 +522,12 @@ const CHANNELS_AGENTS_TARGET_KEYS = [ "channels.telegram", "channels.telegram.botToken", "channels.telegram.capabilities.inlineButtons", + "channels.telegram.execApprovals", + "channels.telegram.execApprovals.enabled", + "channels.telegram.execApprovals.approvers", + "channels.telegram.execApprovals.agentFilter", + "channels.telegram.execApprovals.sessionFilter", + "channels.telegram.execApprovals.target", "channels.whatsapp", ] as const; diff --git a/src/config/schema.help.ts b/src/config/schema.help.ts index 08c579f89e3..908829cbf33 100644 --- a/src/config/schema.help.ts +++ b/src/config/schema.help.ts @@ -1383,6 +1383,18 @@ export const FIELD_HELP: Record = { "Telegram bot token used to authenticate Bot API requests for this account/provider config. Use secret/env substitution and rotate tokens if exposure is suspected.", "channels.telegram.capabilities.inlineButtons": "Enable Telegram inline button components for supported command and interaction surfaces. Disable if your deployment needs plain-text-only compatibility behavior.", + "channels.telegram.execApprovals": + "Telegram-native exec approval routing and approver authorization. Enable this only when Telegram should act as an explicit exec-approval client for the selected bot account.", + "channels.telegram.execApprovals.enabled": + "Enable Telegram exec approvals for this account. When false or unset, Telegram messages/buttons cannot approve exec requests.", + "channels.telegram.execApprovals.approvers": + "Telegram user IDs allowed to approve exec requests for this bot account. Use numeric Telegram user IDs; prompts are only delivered to these approvers when target includes dm.", + "channels.telegram.execApprovals.agentFilter": + 'Optional allowlist of agent IDs eligible for Telegram exec approvals, for example `["main", "ops-agent"]`. Use this to keep approval prompts scoped to the agents you actually operate from Telegram.', + "channels.telegram.execApprovals.sessionFilter": + "Optional session-key filters matched as substring or regex-style patterns before Telegram approval routing is used. Use narrow patterns so Telegram approvals only appear for intended sessions.", + "channels.telegram.execApprovals.target": + 'Controls where Telegram approval prompts are sent: "dm" sends to approver DMs (default), "channel" sends to the originating Telegram chat/topic, and "both" sends to both. Channel delivery exposes the command text to the chat, so only use it in trusted groups/topics.', "channels.slack.configWrites": "Allow Slack to write config in response to channel events/commands (default: true).", "channels.slack.botToken": diff --git a/src/config/schema.labels.ts b/src/config/schema.labels.ts index 16bf21e8daf..c643cf91cd9 100644 --- a/src/config/schema.labels.ts +++ b/src/config/schema.labels.ts @@ -719,6 +719,12 @@ export const FIELD_LABELS: Record = { "channels.telegram.network.autoSelectFamily": "Telegram autoSelectFamily", "channels.telegram.timeoutSeconds": "Telegram API Timeout (seconds)", "channels.telegram.capabilities.inlineButtons": "Telegram Inline Buttons", + "channels.telegram.execApprovals": "Telegram Exec Approvals", + "channels.telegram.execApprovals.enabled": "Telegram Exec Approvals Enabled", + "channels.telegram.execApprovals.approvers": "Telegram Exec Approval Approvers", + "channels.telegram.execApprovals.agentFilter": "Telegram Exec Approval Agent Filter", + "channels.telegram.execApprovals.sessionFilter": "Telegram Exec Approval Session Filter", + "channels.telegram.execApprovals.target": "Telegram Exec Approval Target", "channels.telegram.threadBindings.enabled": "Telegram Thread Binding Enabled", "channels.telegram.threadBindings.idleHours": "Telegram Thread Binding Idle Timeout (hours)", "channels.telegram.threadBindings.maxAgeHours": "Telegram Thread Binding Max Age (hours)", diff --git a/src/config/types.telegram.ts b/src/config/types.telegram.ts index ce8ad105b06..41c047e860c 100644 --- a/src/config/types.telegram.ts +++ b/src/config/types.telegram.ts @@ -38,6 +38,20 @@ export type TelegramNetworkConfig = { export type TelegramInlineButtonsScope = "off" | "dm" | "group" | "all" | "allowlist"; export type TelegramStreamingMode = "off" | "partial" | "block" | "progress"; +export type TelegramExecApprovalTarget = "dm" | "channel" | "both"; + +export type TelegramExecApprovalConfig = { + /** Enable Telegram exec approvals for this account. Default: false. */ + enabled?: boolean; + /** Telegram user IDs allowed to approve exec requests. Required if enabled. */ + approvers?: Array; + /** Only forward approvals for these agent IDs. Omit = all agents. */ + agentFilter?: string[]; + /** Only forward approvals matching these session key patterns (substring or regex). */ + sessionFilter?: string[]; + /** Where to send approval prompts. Default: "dm". */ + target?: TelegramExecApprovalTarget; +}; export type TelegramCapabilitiesConfig = | string[] @@ -58,6 +72,8 @@ export type TelegramAccountConfig = { name?: string; /** Optional provider capability tags used for agent/runtime guidance. */ capabilities?: TelegramCapabilitiesConfig; + /** Telegram-native exec approval delivery + approver authorization. */ + execApprovals?: TelegramExecApprovalConfig; /** Markdown formatting overrides (tables). */ markdown?: MarkdownConfig; /** Override native command registration for Telegram (bool or "auto"). */ diff --git a/src/config/zod-schema.providers-core.ts b/src/config/zod-schema.providers-core.ts index ac1287460bd..3ceefb480ff 100644 --- a/src/config/zod-schema.providers-core.ts +++ b/src/config/zod-schema.providers-core.ts @@ -49,6 +49,7 @@ const DiscordIdSchema = z const DiscordIdListSchema = z.array(DiscordIdSchema); const TelegramInlineButtonsScopeSchema = z.enum(["off", "dm", "group", "all", "allowlist"]); +const TelegramIdListSchema = z.array(z.union([z.string(), z.number()])); const TelegramCapabilitiesSchema = z.union([ z.array(z.string()), @@ -153,6 +154,16 @@ export const TelegramAccountSchemaBase = z .object({ name: z.string().optional(), capabilities: TelegramCapabilitiesSchema.optional(), + execApprovals: z + .object({ + enabled: z.boolean().optional(), + approvers: TelegramIdListSchema.optional(), + agentFilter: z.array(z.string()).optional(), + sessionFilter: z.array(z.string()).optional(), + target: z.enum(["dm", "channel", "both"]).optional(), + }) + .strict() + .optional(), markdown: MarkdownConfigSchema, enabled: z.boolean().optional(), commands: ProviderCommandsSchema, diff --git a/src/discord/exec-approvals.ts b/src/discord/exec-approvals.ts new file mode 100644 index 00000000000..f4be9a22e0c --- /dev/null +++ b/src/discord/exec-approvals.ts @@ -0,0 +1,23 @@ +import type { ReplyPayload } from "../auto-reply/types.js"; +import type { OpenClawConfig } from "../config/config.js"; +import { getExecApprovalReplyMetadata } from "../infra/exec-approval-reply.js"; +import { resolveDiscordAccount } from "./accounts.js"; + +export function isDiscordExecApprovalClientEnabled(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): boolean { + const config = resolveDiscordAccount(params).config.execApprovals; + return Boolean(config?.enabled && (config.approvers?.length ?? 0) > 0); +} + +export function shouldSuppressLocalDiscordExecApprovalPrompt(params: { + cfg: OpenClawConfig; + accountId?: string | null; + payload: ReplyPayload; +}): boolean { + return ( + isDiscordExecApprovalClientEnabled(params) && + getExecApprovalReplyMetadata(params.payload) !== null + ); +} diff --git a/src/discord/monitor/exec-approvals.test.ts b/src/discord/monitor/exec-approvals.test.ts index f5e607022ee..8f9430393a2 100644 --- a/src/discord/monitor/exec-approvals.test.ts +++ b/src/discord/monitor/exec-approvals.test.ts @@ -470,15 +470,15 @@ describe("ExecApprovalButton", () => { function createMockInteraction(userId: string) { const reply = vi.fn().mockResolvedValue(undefined); - const update = vi.fn().mockResolvedValue(undefined); + const acknowledge = vi.fn().mockResolvedValue(undefined); const followUp = vi.fn().mockResolvedValue(undefined); const interaction = { userId, reply, - update, + acknowledge, followUp, } as unknown as ButtonInteraction; - return { interaction, reply, update, followUp }; + return { interaction, reply, acknowledge, followUp }; } it("denies unauthorized users with ephemeral message", async () => { @@ -486,7 +486,7 @@ describe("ExecApprovalButton", () => { const ctx: ExecApprovalButtonContext = { handler }; const button = new ExecApprovalButton(ctx); - const { interaction, reply, update } = createMockInteraction("999"); + const { interaction, reply, acknowledge } = createMockInteraction("999"); const data: ComponentData = { id: "test-approval", action: "allow-once" }; await button.run(interaction, data); @@ -495,7 +495,7 @@ describe("ExecApprovalButton", () => { content: "⛔ You are not authorized to approve exec requests.", ephemeral: true, }); - expect(update).not.toHaveBeenCalled(); + expect(acknowledge).not.toHaveBeenCalled(); // oxlint-disable-next-line typescript/unbound-method -- vi.fn() mock expect(handler.resolveApproval).not.toHaveBeenCalled(); }); @@ -505,50 +505,45 @@ describe("ExecApprovalButton", () => { const ctx: ExecApprovalButtonContext = { handler }; const button = new ExecApprovalButton(ctx); - const { interaction, reply, update } = createMockInteraction("222"); + const { interaction, reply, acknowledge } = createMockInteraction("222"); const data: ComponentData = { id: "test-approval", action: "allow-once" }; await button.run(interaction, data); expect(reply).not.toHaveBeenCalled(); - expect(update).toHaveBeenCalledWith({ - content: "Submitting decision: **Allowed (once)**...", - components: [], - }); + expect(acknowledge).toHaveBeenCalledTimes(1); // oxlint-disable-next-line typescript/unbound-method -- vi.fn() mock expect(handler.resolveApproval).toHaveBeenCalledWith("test-approval", "allow-once"); }); - it("shows correct label for allow-always", async () => { + it("acknowledges allow-always interactions before resolving", async () => { const handler = createMockHandler(["111"]); const ctx: ExecApprovalButtonContext = { handler }; const button = new ExecApprovalButton(ctx); - const { interaction, update } = createMockInteraction("111"); + const { interaction, acknowledge } = createMockInteraction("111"); const data: ComponentData = { id: "test-approval", action: "allow-always" }; await button.run(interaction, data); - expect(update).toHaveBeenCalledWith({ - content: "Submitting decision: **Allowed (always)**...", - components: [], - }); + expect(acknowledge).toHaveBeenCalledTimes(1); + // oxlint-disable-next-line typescript/unbound-method -- vi.fn() mock + expect(handler.resolveApproval).toHaveBeenCalledWith("test-approval", "allow-always"); }); - it("shows correct label for deny", async () => { + it("acknowledges deny interactions before resolving", async () => { const handler = createMockHandler(["111"]); const ctx: ExecApprovalButtonContext = { handler }; const button = new ExecApprovalButton(ctx); - const { interaction, update } = createMockInteraction("111"); + const { interaction, acknowledge } = createMockInteraction("111"); const data: ComponentData = { id: "test-approval", action: "deny" }; await button.run(interaction, data); - expect(update).toHaveBeenCalledWith({ - content: "Submitting decision: **Denied**...", - components: [], - }); + expect(acknowledge).toHaveBeenCalledTimes(1); + // oxlint-disable-next-line typescript/unbound-method -- vi.fn() mock + expect(handler.resolveApproval).toHaveBeenCalledWith("test-approval", "deny"); }); it("handles invalid data gracefully", async () => { @@ -556,18 +551,20 @@ describe("ExecApprovalButton", () => { const ctx: ExecApprovalButtonContext = { handler }; const button = new ExecApprovalButton(ctx); - const { interaction, update } = createMockInteraction("111"); + const { interaction, acknowledge, reply } = createMockInteraction("111"); const data: ComponentData = { id: "", action: "invalid" }; await button.run(interaction, data); - expect(update).toHaveBeenCalledWith({ + expect(reply).toHaveBeenCalledWith({ content: "This approval is no longer valid.", - components: [], + ephemeral: true, }); + expect(acknowledge).not.toHaveBeenCalled(); // oxlint-disable-next-line typescript/unbound-method -- vi.fn() mock expect(handler.resolveApproval).not.toHaveBeenCalled(); }); + it("follows up with error when resolve fails", async () => { const handler = createMockHandler(["111"]); handler.resolveApproval = vi.fn().mockResolvedValue(false); @@ -581,7 +578,7 @@ describe("ExecApprovalButton", () => { expect(followUp).toHaveBeenCalledWith({ content: - "Failed to submit approval decision. The request may have expired or already been resolved.", + "Failed to submit approval decision for **Allowed (once)**. The request may have expired or already been resolved.", ephemeral: true, }); }); @@ -596,14 +593,14 @@ describe("ExecApprovalButton", () => { const ctx: ExecApprovalButtonContext = { handler }; const button = new ExecApprovalButton(ctx); - const { interaction, update, reply } = createMockInteraction("111"); + const { interaction, acknowledge, reply } = createMockInteraction("111"); const data: ComponentData = { id: "test-approval", action: "allow-once" }; await button.run(interaction, data); // Should match because getApprovers returns [111] and button does String(id) === userId expect(reply).not.toHaveBeenCalled(); - expect(update).toHaveBeenCalled(); + expect(acknowledge).toHaveBeenCalled(); }); }); @@ -803,6 +800,80 @@ describe("DiscordExecApprovalHandler delivery routing", () => { clearPendingTimeouts(handler); }); + + it("posts an in-channel note when target is dm and the request came from a non-DM discord conversation", async () => { + const handler = createHandler({ + enabled: true, + approvers: ["123"], + target: "dm", + }); + const internals = getHandlerInternals(handler); + + mockRestPost.mockImplementation( + async (route: string, params?: { body?: { content?: string } }) => { + if (route === Routes.channelMessages("999888777")) { + expect(params?.body?.content).toContain("I sent the allowed approvers DMs"); + return { id: "note-1", channel_id: "999888777" }; + } + if (route === Routes.userChannels()) { + return { id: "dm-1" }; + } + if (route === Routes.channelMessages("dm-1")) { + return { id: "msg-1", channel_id: "dm-1" }; + } + throw new Error(`unexpected route: ${route}`); + }, + ); + + await internals.handleApprovalRequested(createRequest()); + + expect(mockRestPost).toHaveBeenCalledWith( + Routes.channelMessages("999888777"), + expect.objectContaining({ + body: expect.objectContaining({ + content: expect.stringContaining("I sent the allowed approvers DMs"), + }), + }), + ); + expect(mockRestPost).toHaveBeenCalledWith( + Routes.channelMessages("dm-1"), + expect.objectContaining({ + body: expect.any(Object), + }), + ); + + clearPendingTimeouts(handler); + }); + + it("does not post an in-channel note when the request already came from a discord DM", async () => { + const handler = createHandler({ + enabled: true, + approvers: ["123"], + target: "dm", + }); + const internals = getHandlerInternals(handler); + + mockRestPost.mockImplementation(async (route: string) => { + if (route === Routes.userChannels()) { + return { id: "dm-1" }; + } + if (route === Routes.channelMessages("dm-1")) { + return { id: "msg-1", channel_id: "dm-1" }; + } + throw new Error(`unexpected route: ${route}`); + }); + + await internals.handleApprovalRequested( + createRequest({ sessionKey: "agent:main:discord:dm:123" }), + ); + + expect(mockRestPost).not.toHaveBeenCalledWith( + Routes.channelMessages("999888777"), + expect.anything(), + ); + + clearPendingTimeouts(handler); + }); }); describe("DiscordExecApprovalHandler gateway auth resolution", () => { diff --git a/src/discord/monitor/exec-approvals.ts b/src/discord/monitor/exec-approvals.ts index 5564b126e3c..f426ae51903 100644 --- a/src/discord/monitor/exec-approvals.ts +++ b/src/discord/monitor/exec-approvals.ts @@ -17,6 +17,7 @@ import { buildGatewayConnectionDetails } from "../../gateway/call.js"; import { GatewayClient } from "../../gateway/client.js"; import { resolveGatewayConnectionAuth } from "../../gateway/connection-auth.js"; import type { EventFrame } from "../../gateway/protocol/index.js"; +import { getExecApprovalApproverDmNoticeText } from "../../infra/exec-approval-reply.js"; import type { ExecApprovalDecision, ExecApprovalRequest, @@ -47,6 +48,12 @@ export function extractDiscordChannelId(sessionKey?: string | null): string | nu return match ? match[1] : null; } +function buildDiscordApprovalDmRedirectNotice(): { content: string } { + return { + content: getExecApprovalApproverDmNoticeText(), + }; +} + type PendingApproval = { discordMessageId: string; discordChannelId: string; @@ -498,6 +505,24 @@ export class DiscordExecApprovalHandler { const sendToDm = target === "dm" || target === "both"; const sendToChannel = target === "channel" || target === "both"; let fallbackToDm = false; + const originatingChannelId = + request.request.sessionKey && target === "dm" + ? extractDiscordChannelId(request.request.sessionKey) + : null; + + if (target === "dm" && originatingChannelId) { + try { + await discordRequest( + () => + rest.post(Routes.channelMessages(originatingChannelId), { + body: buildDiscordApprovalDmRedirectNotice(), + }) as Promise<{ id: string; channel_id: string }>, + "send-approval-dm-redirect-notice", + ); + } catch (err) { + logError(`discord exec approvals: failed to send DM redirect notice: ${String(err)}`); + } + } // Send to originating channel if configured if (sendToChannel) { @@ -768,9 +793,9 @@ export class ExecApprovalButton extends Button { const parsed = parseExecApprovalData(data); if (!parsed) { try { - await interaction.update({ + await interaction.reply({ content: "This approval is no longer valid.", - components: [], + ephemeral: true, }); } catch { // Interaction may have expired @@ -800,12 +825,11 @@ export class ExecApprovalButton extends Button { ? "Allowed (always)" : "Denied"; - // Update the message immediately to show the decision + // Acknowledge immediately so Discord does not fail the interaction while + // the gateway resolve roundtrip completes. The resolved event will update + // the approval card in-place with the final state. try { - await interaction.update({ - content: `Submitting decision: **${decisionLabel}**...`, - components: [], // Remove buttons - }); + await interaction.acknowledge(); } catch { // Interaction may have expired, try to continue anyway } @@ -815,8 +839,7 @@ export class ExecApprovalButton extends Button { if (!ok) { try { await interaction.followUp({ - content: - "Failed to submit approval decision. The request may have expired or already been resolved.", + content: `Failed to submit approval decision for **${decisionLabel}**. The request may have expired or already been resolved.`, ephemeral: true, }); } catch { diff --git a/src/gateway/exec-approval-manager.ts b/src/gateway/exec-approval-manager.ts index 320b4da0b1f..e0176470a03 100644 --- a/src/gateway/exec-approval-manager.ts +++ b/src/gateway/exec-approval-manager.ts @@ -31,6 +31,11 @@ type PendingEntry = { promise: Promise; }; +export type ExecApprovalIdLookupResult = + | { kind: "exact" | "prefix"; id: string } + | { kind: "ambiguous"; ids: string[] } + | { kind: "none" }; + export class ExecApprovalManager { private pending = new Map(); @@ -170,4 +175,37 @@ export class ExecApprovalManager { const entry = this.pending.get(recordId); return entry?.promise ?? null; } + + lookupPendingId(input: string): ExecApprovalIdLookupResult { + const normalized = input.trim(); + if (!normalized) { + return { kind: "none" }; + } + + const exact = this.pending.get(normalized); + if (exact) { + return exact.record.resolvedAtMs === undefined + ? { kind: "exact", id: normalized } + : { kind: "none" }; + } + + const lowerPrefix = normalized.toLowerCase(); + const matches: string[] = []; + for (const [id, entry] of this.pending.entries()) { + if (entry.record.resolvedAtMs !== undefined) { + continue; + } + if (id.toLowerCase().startsWith(lowerPrefix)) { + matches.push(id); + } + } + + if (matches.length === 1) { + return { kind: "prefix", id: matches[0] }; + } + if (matches.length > 1) { + return { kind: "ambiguous", ids: matches }; + } + return { kind: "none" }; + } } diff --git a/src/gateway/node-invoke-system-run-approval.ts b/src/gateway/node-invoke-system-run-approval.ts index 1099896f6c8..b077204e4ba 100644 --- a/src/gateway/node-invoke-system-run-approval.ts +++ b/src/gateway/node-invoke-system-run-approval.ts @@ -23,6 +23,7 @@ type SystemRunParamsLike = { approved?: unknown; approvalDecision?: unknown; runId?: unknown; + suppressNotifyOnExit?: unknown; }; type ApprovalLookup = { @@ -78,6 +79,7 @@ function pickSystemRunParams(raw: Record): Record boolean }) => { - if (typeof context.hasExecApprovalClients === "function") { - return context.hasExecApprovalClients(); - } - // Fail closed when no operator-scope probe is available. - return false; - }; - return { "exec.approval.request": async ({ params, respond, context, client }) => { if (!validateExecApprovalRequestParams(params)) { @@ -178,10 +170,11 @@ export function createExecApprovalHandlers( }, { dropIfSlow: true }, ); - let forwardedToTargets = false; + const hasExecApprovalClients = context.hasExecApprovalClients?.() ?? false; + let forwarded = false; if (opts?.forwarder) { try { - forwardedToTargets = await opts.forwarder.handleRequested({ + forwarded = await opts.forwarder.handleRequested({ id: record.id, request: record.request, createdAtMs: record.createdAtMs, @@ -192,8 +185,19 @@ export function createExecApprovalHandlers( } } - if (!hasApprovalClients(context) && !forwardedToTargets) { - manager.expire(record.id, "auto-expire:no-approver-clients"); + if (!hasExecApprovalClients && !forwarded) { + manager.expire(record.id, "no-approval-route"); + respond( + true, + { + id: record.id, + decision: null, + createdAtMs: record.createdAtMs, + expiresAtMs: record.expiresAtMs, + }, + undefined, + ); + return; } // Only send immediate "accepted" response when twoPhase is requested. @@ -275,21 +279,48 @@ export function createExecApprovalHandlers( respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "invalid decision")); return; } - const snapshot = manager.getSnapshot(p.id); + const resolvedId = manager.lookupPendingId(p.id); + if (resolvedId.kind === "none") { + respond( + false, + undefined, + errorShape(ErrorCodes.INVALID_REQUEST, "unknown or expired approval id"), + ); + return; + } + if (resolvedId.kind === "ambiguous") { + const candidates = resolvedId.ids.slice(0, 3).join(", "); + const remainder = resolvedId.ids.length > 3 ? ` (+${resolvedId.ids.length - 3} more)` : ""; + respond( + false, + undefined, + errorShape( + ErrorCodes.INVALID_REQUEST, + `ambiguous approval id prefix; matches: ${candidates}${remainder}. Use the full id.`, + ), + ); + return; + } + const approvalId = resolvedId.id; + const snapshot = manager.getSnapshot(approvalId); const resolvedBy = client?.connect?.client?.displayName ?? client?.connect?.client?.id; - const ok = manager.resolve(p.id, decision, resolvedBy ?? null); + const ok = manager.resolve(approvalId, decision, resolvedBy ?? null); if (!ok) { - respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unknown approval id")); + respond( + false, + undefined, + errorShape(ErrorCodes.INVALID_REQUEST, "unknown or expired approval id"), + ); return; } context.broadcast( "exec.approval.resolved", - { id: p.id, decision, resolvedBy, ts: Date.now(), request: snapshot?.request }, + { id: approvalId, decision, resolvedBy, ts: Date.now(), request: snapshot?.request }, { dropIfSlow: true }, ); void opts?.forwarder ?.handleResolved({ - id: p.id, + id: approvalId, decision, resolvedBy, ts: Date.now(), diff --git a/src/gateway/server-methods/server-methods.test.ts b/src/gateway/server-methods/server-methods.test.ts index 4ea91ea247f..2292a1c808c 100644 --- a/src/gateway/server-methods/server-methods.test.ts +++ b/src/gateway/server-methods/server-methods.test.ts @@ -531,6 +531,19 @@ describe("exec approval handlers", () => { expect(broadcasts.some((entry) => entry.event === "exec.approval.resolved")).toBe(true); }); + it("does not reuse a resolved exact id as a prefix for another pending approval", () => { + const manager = new ExecApprovalManager(); + const resolvedRecord = manager.create({ command: "echo old", host: "gateway" }, 2_000, "abc"); + void manager.register(resolvedRecord, 2_000); + expect(manager.resolve("abc", "allow-once")).toBe(true); + + const pendingRecord = manager.create({ command: "echo new", host: "gateway" }, 2_000, "abcdef"); + void manager.register(pendingRecord, 2_000); + + expect(manager.lookupPendingId("abc")).toEqual({ kind: "none" }); + expect(manager.lookupPendingId("abcdef")).toEqual({ kind: "exact", id: "abcdef" }); + }); + it("stores versioned system.run binding and sorted env keys on approval request", async () => { const { handlers, broadcasts, respond, context } = createExecApprovalFixture(); await requestExecApproval({ @@ -666,6 +679,134 @@ describe("exec approval handlers", () => { expect(resolveRespond).toHaveBeenCalledWith(true, { ok: true }, undefined); }); + it("accepts unique short approval id prefixes", async () => { + const manager = new ExecApprovalManager(); + const handlers = createExecApprovalHandlers(manager); + const respond = vi.fn(); + const context = { + broadcast: (_event: string, _payload: unknown) => {}, + }; + + const record = manager.create({ command: "echo ok" }, 60_000, "approval-12345678-aaaa"); + void manager.register(record, 60_000); + + await resolveExecApproval({ + handlers, + id: "approval-1234", + respond, + context, + }); + + expect(respond).toHaveBeenCalledWith(true, { ok: true }, undefined); + expect(manager.getSnapshot(record.id)?.decision).toBe("allow-once"); + }); + + it("rejects ambiguous short approval id prefixes", async () => { + const manager = new ExecApprovalManager(); + const handlers = createExecApprovalHandlers(manager); + const respond = vi.fn(); + const context = { + broadcast: (_event: string, _payload: unknown) => {}, + }; + + void manager.register( + manager.create({ command: "echo one" }, 60_000, "approval-abcd-1111"), + 60_000, + ); + void manager.register( + manager.create({ command: "echo two" }, 60_000, "approval-abcd-2222"), + 60_000, + ); + + await resolveExecApproval({ + handlers, + id: "approval-abcd", + respond, + context, + }); + + expect(respond).toHaveBeenCalledWith( + false, + undefined, + expect.objectContaining({ + message: expect.stringContaining("ambiguous approval id prefix"), + }), + ); + }); + + it("returns deterministic unknown/expired message for missing approval ids", async () => { + const { handlers, respond, context } = createExecApprovalFixture(); + + await resolveExecApproval({ + handlers, + id: "missing-approval-id", + respond, + context, + }); + + expect(respond).toHaveBeenCalledWith( + false, + undefined, + expect.objectContaining({ + message: "unknown or expired approval id", + }), + ); + }); + + it("resolves only the targeted approval id when multiple requests are pending", async () => { + const manager = new ExecApprovalManager(); + const handlers = createExecApprovalHandlers(manager); + const context = { + broadcast: (_event: string, _payload: unknown) => {}, + hasExecApprovalClients: () => true, + }; + const respondOne = vi.fn(); + const respondTwo = vi.fn(); + + const requestOne = requestExecApproval({ + handlers, + respond: respondOne, + context, + params: { id: "approval-one", host: "gateway", timeoutMs: 60_000 }, + }); + const requestTwo = requestExecApproval({ + handlers, + respond: respondTwo, + context, + params: { id: "approval-two", host: "gateway", timeoutMs: 60_000 }, + }); + + await drainApprovalRequestTicks(); + + const resolveRespond = vi.fn(); + await resolveExecApproval({ + handlers, + id: "approval-one", + respond: resolveRespond, + context, + }); + + expect(resolveRespond).toHaveBeenCalledWith(true, { ok: true }, undefined); + expect(manager.getSnapshot("approval-one")?.decision).toBe("allow-once"); + expect(manager.getSnapshot("approval-two")?.decision).toBeUndefined(); + expect(manager.getSnapshot("approval-two")?.resolvedAtMs).toBeUndefined(); + + expect(manager.expire("approval-two", "test-expire")).toBe(true); + await requestOne; + await requestTwo; + + expect(respondOne).toHaveBeenCalledWith( + true, + expect.objectContaining({ id: "approval-one", decision: "allow-once" }), + undefined, + ); + expect(respondTwo).toHaveBeenCalledWith( + true, + expect.objectContaining({ id: "approval-two", decision: null }), + undefined, + ); + }); + it("forwards turn-source metadata to exec approval forwarding", async () => { vi.useFakeTimers(); try { @@ -703,32 +844,59 @@ describe("exec approval handlers", () => { } }); - it("expires immediately when no approver clients and no forwarding targets", async () => { - vi.useFakeTimers(); - try { - const { manager, handlers, forwarder, respond, context } = - createForwardingExecApprovalFixture(); - const expireSpy = vi.spyOn(manager, "expire"); + it("fast-fails approvals when no approver clients and no forwarding targets", async () => { + const { manager, handlers, forwarder, respond, context } = + createForwardingExecApprovalFixture(); + const expireSpy = vi.spyOn(manager, "expire"); - const requestPromise = requestExecApproval({ - handlers, - respond, - context, - params: { timeoutMs: 60_000 }, - }); - await drainApprovalRequestTicks(); - expect(forwarder.handleRequested).toHaveBeenCalledTimes(1); - expect(expireSpy).toHaveBeenCalledTimes(1); - await vi.runOnlyPendingTimersAsync(); - await requestPromise; - expect(respond).toHaveBeenCalledWith( - true, - expect.objectContaining({ decision: null }), - undefined, - ); - } finally { - vi.useRealTimers(); - } + await requestExecApproval({ + handlers, + respond, + context, + params: { timeoutMs: 60_000, id: "approval-no-approver", host: "gateway" }, + }); + + expect(forwarder.handleRequested).toHaveBeenCalledTimes(1); + expect(expireSpy).toHaveBeenCalledWith("approval-no-approver", "no-approval-route"); + expect(respond).toHaveBeenCalledWith( + true, + expect.objectContaining({ id: "approval-no-approver", decision: null }), + undefined, + ); + }); + + it("keeps approvals pending when no approver clients but forwarding accepted the request", async () => { + const { manager, handlers, forwarder, respond, context } = + createForwardingExecApprovalFixture(); + const expireSpy = vi.spyOn(manager, "expire"); + const resolveRespond = vi.fn(); + forwarder.handleRequested.mockResolvedValueOnce(true); + + const requestPromise = requestExecApproval({ + handlers, + respond, + context, + params: { timeoutMs: 60_000, id: "approval-forwarded", host: "gateway" }, + }); + await drainApprovalRequestTicks(); + + expect(forwarder.handleRequested).toHaveBeenCalledTimes(1); + expect(expireSpy).not.toHaveBeenCalled(); + + await resolveExecApproval({ + handlers, + id: "approval-forwarded", + respond: resolveRespond, + context, + }); + await requestPromise; + + expect(resolveRespond).toHaveBeenCalledWith(true, { ok: true }, undefined); + expect(respond).toHaveBeenCalledWith( + true, + expect.objectContaining({ id: "approval-forwarded", decision: "allow-once" }), + undefined, + ); }); }); diff --git a/src/gateway/server-node-events.test.ts b/src/gateway/server-node-events.test.ts index 46b3689642d..a8885a64a63 100644 --- a/src/gateway/server-node-events.test.ts +++ b/src/gateway/server-node-events.test.ts @@ -492,6 +492,23 @@ describe("notifications changed events", () => { expect(enqueueSystemEventMock).toHaveBeenCalledTimes(2); expect(requestHeartbeatNowMock).toHaveBeenCalledTimes(1); }); + + it("suppresses exec notifyOnExit events when payload opts out", async () => { + const ctx = buildCtx(); + await handleNodeEvent(ctx, "node-n7", { + event: "exec.finished", + payloadJSON: JSON.stringify({ + sessionKey: "agent:main:main", + runId: "approval-1", + exitCode: 0, + output: "ok", + suppressNotifyOnExit: true, + }), + }); + + expect(enqueueSystemEventMock).not.toHaveBeenCalled(); + expect(requestHeartbeatNowMock).not.toHaveBeenCalled(); + }); }); describe("agent request events", () => { diff --git a/src/gateway/server-node-events.ts b/src/gateway/server-node-events.ts index db9da55588b..3a8ad91c420 100644 --- a/src/gateway/server-node-events.ts +++ b/src/gateway/server-node-events.ts @@ -538,6 +538,9 @@ export const handleNodeEvent = async (ctx: NodeEventContext, nodeId: string, evt if (!notifyOnExit) { return; } + if (obj.suppressNotifyOnExit === true) { + return; + } const runId = typeof obj.runId === "string" ? obj.runId.trim() : ""; const command = typeof obj.command === "string" ? obj.command.trim() : ""; diff --git a/src/infra/exec-approval-forwarder.test.ts b/src/infra/exec-approval-forwarder.test.ts index f87c307c211..8ae1b53cc57 100644 --- a/src/infra/exec-approval-forwarder.test.ts +++ b/src/infra/exec-approval-forwarder.test.ts @@ -1,8 +1,11 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { telegramOutbound } from "../channels/plugins/outbound/telegram.js"; import type { OpenClawConfig } from "../config/config.js"; +import { setActivePluginRegistry } from "../plugins/runtime.js"; +import { createOutboundTestPlugin, createTestRegistry } from "../test-utils/channel-plugins.js"; import { createExecApprovalForwarder } from "./exec-approval-forwarder.js"; const baseRequest = { @@ -18,8 +21,18 @@ const baseRequest = { afterEach(() => { vi.useRealTimers(); + vi.restoreAllMocks(); }); +const emptyRegistry = createTestRegistry([]); +const defaultRegistry = createTestRegistry([ + { + pluginId: "telegram", + plugin: createOutboundTestPlugin({ id: "telegram", outbound: telegramOutbound }), + source: "test", + }, +]); + function getFirstDeliveryText(deliver: ReturnType): string { const firstCall = deliver.mock.calls[0]?.[0] as | { payloads?: Array<{ text?: string }> } @@ -32,7 +45,7 @@ const TARGETS_CFG = { exec: { enabled: true, mode: "targets", - targets: [{ channel: "telegram", to: "123" }], + targets: [{ channel: "slack", to: "U123" }], }, }, } as OpenClawConfig; @@ -128,6 +141,14 @@ async function expectSessionFilterRequestResult(params: { } describe("exec approval forwarder", () => { + beforeEach(() => { + setActivePluginRegistry(defaultRegistry); + }); + + afterEach(() => { + setActivePluginRegistry(emptyRegistry); + }); + it("forwards to session target and resolves", async () => { vi.useFakeTimers(); const cfg = { @@ -159,19 +180,118 @@ describe("exec approval forwarder", () => { const { deliver, forwarder } = createForwarder({ cfg: TARGETS_CFG }); await expect(forwarder.handleRequested(baseRequest)).resolves.toBe(true); + await Promise.resolve(); expect(deliver).toHaveBeenCalledTimes(1); await vi.runAllTimersAsync(); expect(deliver).toHaveBeenCalledTimes(2); }); + it("skips telegram forwarding when telegram exec approvals handler is enabled", async () => { + vi.useFakeTimers(); + const cfg = { + approvals: { + exec: { + enabled: true, + mode: "session", + }, + }, + channels: { + telegram: { + execApprovals: { + enabled: true, + approvers: ["123"], + target: "channel", + }, + }, + }, + } as OpenClawConfig; + + const { deliver, forwarder } = createForwarder({ + cfg, + resolveSessionTarget: () => ({ channel: "telegram", to: "-100999", threadId: 77 }), + }); + + await expect( + forwarder.handleRequested({ + ...baseRequest, + request: { + ...baseRequest.request, + turnSourceChannel: "telegram", + turnSourceTo: "-100999", + turnSourceThreadId: "77", + turnSourceAccountId: "default", + }, + }), + ).resolves.toBe(false); + + expect(deliver).not.toHaveBeenCalled(); + }); + + it("attaches explicit telegram buttons in forwarded telegram fallback payloads", async () => { + vi.useFakeTimers(); + const cfg = { + approvals: { + exec: { + enabled: true, + mode: "targets", + targets: [{ channel: "telegram", to: "123" }], + }, + }, + } as OpenClawConfig; + + const { deliver, forwarder } = createForwarder({ cfg }); + + await expect( + forwarder.handleRequested({ + ...baseRequest, + request: { + ...baseRequest.request, + turnSourceChannel: "discord", + turnSourceTo: "channel:123", + }, + }), + ).resolves.toBe(true); + + expect(deliver).toHaveBeenCalledTimes(1); + expect(deliver).toHaveBeenCalledWith( + expect.objectContaining({ + channel: "telegram", + to: "123", + payloads: [ + expect.objectContaining({ + channelData: { + execApproval: expect.objectContaining({ + approvalId: "req-1", + }), + telegram: { + buttons: [ + [ + { text: "Allow Once", callback_data: "/approve req-1 allow-once" }, + { text: "Allow Always", callback_data: "/approve req-1 allow-always" }, + ], + [{ text: "Deny", callback_data: "/approve req-1 deny" }], + ], + }, + }, + }), + ], + }), + ); + }); + it("formats single-line commands as inline code", async () => { vi.useFakeTimers(); const { deliver, forwarder } = createForwarder({ cfg: TARGETS_CFG }); await expect(forwarder.handleRequested(baseRequest)).resolves.toBe(true); + await Promise.resolve(); - expect(getFirstDeliveryText(deliver)).toContain("Command: `echo hello`"); + const text = getFirstDeliveryText(deliver); + expect(text).toContain("🔒 Exec approval required"); + expect(text).toContain("Command: `echo hello`"); + expect(text).toContain("Expires in: 5s"); + expect(text).toContain("Reply with: /approve allow-once|allow-always|deny"); }); it("formats complex commands as fenced code blocks", async () => { @@ -187,8 +307,9 @@ describe("exec approval forwarder", () => { }, }), ).resolves.toBe(true); + await Promise.resolve(); - expect(getFirstDeliveryText(deliver)).toContain("Command:\n```\necho `uname`\necho done\n```"); + expect(getFirstDeliveryText(deliver)).toContain("```\necho `uname`\necho done\n```"); }); it("returns false when forwarding is disabled", async () => { @@ -334,7 +455,8 @@ describe("exec approval forwarder", () => { }, }), ).resolves.toBe(true); + await Promise.resolve(); - expect(getFirstDeliveryText(deliver)).toContain("Command:\n````\necho ```danger```\n````"); + expect(getFirstDeliveryText(deliver)).toContain("````\necho ```danger```\n````"); }); }); diff --git a/src/infra/exec-approval-forwarder.ts b/src/infra/exec-approval-forwarder.ts index 296a6aa6e49..a412e2495e8 100644 --- a/src/infra/exec-approval-forwarder.ts +++ b/src/infra/exec-approval-forwarder.ts @@ -1,3 +1,4 @@ +import type { ReplyPayload } from "../auto-reply/types.js"; import type { OpenClawConfig } from "../config/config.js"; import { loadConfig } from "../config/config.js"; import { loadSessionStore, resolveStorePath } from "../config/sessions.js"; @@ -8,11 +9,14 @@ import type { import { createSubsystemLogger } from "../logging/subsystem.js"; import { normalizeAccountId, parseAgentSessionKey } from "../routing/session-key.js"; import { compileSafeRegex, testRegexWithBoundedInput } from "../security/safe-regex.js"; +import { buildTelegramExecApprovalButtons } from "../telegram/approval-buttons.js"; +import { sendTypingTelegram } from "../telegram/send.js"; import { isDeliverableMessageChannel, normalizeMessageChannel, type DeliverableMessageChannel, } from "../utils/message-channel.js"; +import { buildExecApprovalPendingReplyPayload } from "./exec-approval-reply.js"; import type { ExecApprovalDecision, ExecApprovalRequest, @@ -65,7 +69,11 @@ function matchSessionFilter(sessionKey: string, patterns: string[]): boolean { } function shouldForward(params: { - config?: ExecApprovalForwardingConfig; + config?: { + enabled?: boolean; + agentFilter?: string[]; + sessionFilter?: string[]; + }; request: ExecApprovalRequest; }): boolean { const config = params.config; @@ -147,6 +155,48 @@ function shouldSkipDiscordForwarding( return Boolean(execApprovals?.enabled && (execApprovals.approvers?.length ?? 0) > 0); } +function shouldSkipTelegramForwarding(params: { + target: ExecApprovalForwardTarget; + cfg: OpenClawConfig; + request: ExecApprovalRequest; +}): boolean { + const channel = normalizeMessageChannel(params.target.channel) ?? params.target.channel; + if (channel !== "telegram") { + return false; + } + const requestChannel = normalizeMessageChannel(params.request.request.turnSourceChannel ?? ""); + if (requestChannel !== "telegram") { + return false; + } + const telegram = params.cfg.channels?.telegram; + if (!telegram) { + return false; + } + const telegramConfig = telegram as + | { + execApprovals?: { enabled?: boolean; approvers?: Array }; + accounts?: Record< + string, + { execApprovals?: { enabled?: boolean; approvers?: Array } } + >; + } + | undefined; + if (!telegramConfig) { + return false; + } + const accountId = + params.target.accountId?.trim() || params.request.request.turnSourceAccountId?.trim(); + const account = accountId + ? (resolveChannelAccountConfig<{ + execApprovals?: { enabled?: boolean; approvers?: Array }; + }>(telegramConfig.accounts, accountId) as + | { execApprovals?: { enabled?: boolean; approvers?: Array } } + | undefined) + : undefined; + const execApprovals = account?.execApprovals ?? telegramConfig.execApprovals; + return Boolean(execApprovals?.enabled && (execApprovals.approvers?.length ?? 0) > 0); +} + function formatApprovalCommand(command: string): { inline: boolean; text: string } { if (!command.includes("\n") && !command.includes("`")) { return { inline: true, text: `\`${command}\`` }; @@ -191,6 +241,10 @@ function buildRequestMessage(request: ExecApprovalRequest, nowMs: number) { } const expiresIn = Math.max(0, Math.round((request.expiresAtMs - nowMs) / 1000)); lines.push(`Expires in: ${expiresIn}s`); + lines.push("Mode: foreground (interactive approvals available in this chat)."); + lines.push( + "Background mode note: non-interactive runs cannot wait for chat approvals; use pre-approved policy (allow-always or ask=off).", + ); lines.push("Reply with: /approve allow-once|allow-always|deny"); return lines.join("\n"); } @@ -261,7 +315,7 @@ function defaultResolveSessionTarget(params: { async function deliverToTargets(params: { cfg: OpenClawConfig; targets: ForwardTarget[]; - text: string; + buildPayload: (target: ForwardTarget) => ReplyPayload; deliver: typeof deliverOutboundPayloads; shouldSend?: () => boolean; }) { @@ -274,13 +328,33 @@ async function deliverToTargets(params: { return; } try { + const payload = params.buildPayload(target); + if ( + channel === "telegram" && + payload.channelData && + typeof payload.channelData === "object" && + !Array.isArray(payload.channelData) && + payload.channelData.execApproval + ) { + const threadId = + typeof target.threadId === "number" + ? target.threadId + : typeof target.threadId === "string" + ? Number.parseInt(target.threadId, 10) + : undefined; + await sendTypingTelegram(target.to, { + cfg: params.cfg, + accountId: target.accountId, + ...(Number.isFinite(threadId) ? { messageThreadId: threadId } : {}), + }).catch(() => {}); + } await params.deliver({ cfg: params.cfg, channel, to: target.to, accountId: target.accountId, threadId: target.threadId, - payloads: [{ text: params.text }], + payloads: [payload], }); } catch (err) { log.error(`exec approvals: failed to deliver to ${channel}:${target.to}: ${String(err)}`); @@ -289,6 +363,42 @@ async function deliverToTargets(params: { await Promise.allSettled(deliveries); } +function buildRequestPayloadForTarget( + _cfg: OpenClawConfig, + request: ExecApprovalRequest, + nowMsValue: number, + target: ForwardTarget, +): ReplyPayload { + const channel = normalizeMessageChannel(target.channel) ?? target.channel; + if (channel === "telegram") { + const payload = buildExecApprovalPendingReplyPayload({ + approvalId: request.id, + approvalSlug: request.id.slice(0, 8), + approvalCommandId: request.id, + command: request.request.command, + cwd: request.request.cwd ?? undefined, + host: request.request.host === "node" ? "node" : "gateway", + nodeId: request.request.nodeId ?? undefined, + expiresAtMs: request.expiresAtMs, + nowMs: nowMsValue, + }); + const buttons = buildTelegramExecApprovalButtons(request.id); + if (!buttons) { + return payload; + } + return { + ...payload, + channelData: { + ...payload.channelData, + telegram: { + buttons, + }, + }, + }; + } + return { text: buildRequestMessage(request, nowMsValue) }; +} + function resolveForwardTargets(params: { cfg: OpenClawConfig; config?: ExecApprovalForwardingConfig; @@ -343,15 +453,20 @@ export function createExecApprovalForwarder( const handleRequested = async (request: ExecApprovalRequest): Promise => { const cfg = getConfig(); const config = cfg.approvals?.exec; - if (!shouldForward({ config, request })) { - return false; - } - const filteredTargets = resolveForwardTargets({ - cfg, - config, - request, - resolveSessionTarget, - }).filter((target) => !shouldSkipDiscordForwarding(target, cfg)); + const filteredTargets = [ + ...(shouldForward({ config, request }) + ? resolveForwardTargets({ + cfg, + config, + request, + resolveSessionTarget, + }) + : []), + ].filter( + (target) => + !shouldSkipDiscordForwarding(target, cfg) && + !shouldSkipTelegramForwarding({ target, cfg, request }), + ); if (filteredTargets.length === 0) { return false; @@ -366,7 +481,12 @@ export function createExecApprovalForwarder( } pending.delete(request.id); const expiredText = buildExpiredMessage(request); - await deliverToTargets({ cfg, targets: entry.targets, text: expiredText, deliver }); + await deliverToTargets({ + cfg, + targets: entry.targets, + buildPayload: () => ({ text: expiredText }), + deliver, + }); })(); }, expiresInMs); timeoutId.unref?.(); @@ -377,12 +497,10 @@ export function createExecApprovalForwarder( if (pending.get(request.id) !== pendingEntry) { return false; } - - const text = buildRequestMessage(request, nowMs()); void deliverToTargets({ cfg, targets: filteredTargets, - text, + buildPayload: (target) => buildRequestPayloadForTarget(cfg, request, nowMs(), target), deliver, shouldSend: () => pending.get(request.id) === pendingEntry, }).catch((err) => { @@ -410,20 +528,26 @@ export function createExecApprovalForwarder( expiresAtMs: resolved.ts, }; const config = cfg.approvals?.exec; - if (shouldForward({ config, request })) { - targets = resolveForwardTargets({ - cfg, - config, - request, - resolveSessionTarget, - }).filter((target) => !shouldSkipDiscordForwarding(target, cfg)); - } + targets = [ + ...(shouldForward({ config, request }) + ? resolveForwardTargets({ + cfg, + config, + request, + resolveSessionTarget, + }) + : []), + ].filter( + (target) => + !shouldSkipDiscordForwarding(target, cfg) && + !shouldSkipTelegramForwarding({ target, cfg, request }), + ); } if (!targets || targets.length === 0) { return; } const text = buildResolvedMessage(resolved); - await deliverToTargets({ cfg, targets, text, deliver }); + await deliverToTargets({ cfg, targets, buildPayload: () => ({ text }), deliver }); }; const stop = () => { diff --git a/src/infra/exec-approval-reply.ts b/src/infra/exec-approval-reply.ts new file mode 100644 index 00000000000..c1a3cda4a69 --- /dev/null +++ b/src/infra/exec-approval-reply.ts @@ -0,0 +1,172 @@ +import type { ReplyPayload } from "../auto-reply/types.js"; +import type { ExecHost } from "./exec-approvals.js"; + +export type ExecApprovalReplyDecision = "allow-once" | "allow-always" | "deny"; +export type ExecApprovalUnavailableReason = + | "initiating-platform-disabled" + | "initiating-platform-unsupported" + | "no-approval-route"; + +export type ExecApprovalReplyMetadata = { + approvalId: string; + approvalSlug: string; + allowedDecisions?: readonly ExecApprovalReplyDecision[]; +}; + +export type ExecApprovalPendingReplyParams = { + warningText?: string; + approvalId: string; + approvalSlug: string; + approvalCommandId?: string; + command: string; + cwd?: string; + host: ExecHost; + nodeId?: string; + expiresAtMs?: number; + nowMs?: number; +}; + +export type ExecApprovalUnavailableReplyParams = { + warningText?: string; + channelLabel?: string; + reason: ExecApprovalUnavailableReason; + sentApproverDms?: boolean; +}; + +export function getExecApprovalApproverDmNoticeText(): string { + return "Approval required. I sent the allowed approvers DMs."; +} + +function buildFence(text: string, language?: string): string { + let fence = "```"; + while (text.includes(fence)) { + fence += "`"; + } + const languagePrefix = language ? language : ""; + return `${fence}${languagePrefix}\n${text}\n${fence}`; +} + +export function getExecApprovalReplyMetadata( + payload: ReplyPayload, +): ExecApprovalReplyMetadata | null { + const channelData = payload.channelData; + if (!channelData || typeof channelData !== "object" || Array.isArray(channelData)) { + return null; + } + const execApproval = channelData.execApproval; + if (!execApproval || typeof execApproval !== "object" || Array.isArray(execApproval)) { + return null; + } + const record = execApproval as Record; + const approvalId = typeof record.approvalId === "string" ? record.approvalId.trim() : ""; + const approvalSlug = typeof record.approvalSlug === "string" ? record.approvalSlug.trim() : ""; + if (!approvalId || !approvalSlug) { + return null; + } + const allowedDecisions = Array.isArray(record.allowedDecisions) + ? record.allowedDecisions.filter( + (value): value is ExecApprovalReplyDecision => + value === "allow-once" || value === "allow-always" || value === "deny", + ) + : undefined; + return { + approvalId, + approvalSlug, + allowedDecisions, + }; +} + +export function buildExecApprovalPendingReplyPayload( + params: ExecApprovalPendingReplyParams, +): ReplyPayload { + const approvalCommandId = params.approvalCommandId?.trim() || params.approvalSlug; + const lines: string[] = []; + const warningText = params.warningText?.trim(); + if (warningText) { + lines.push(warningText, ""); + } + lines.push("Approval required."); + lines.push("Run:"); + lines.push(buildFence(`/approve ${approvalCommandId} allow-once`, "txt")); + lines.push("Pending command:"); + lines.push(buildFence(params.command, "sh")); + lines.push("Other options:"); + lines.push( + buildFence( + `/approve ${approvalCommandId} allow-always\n/approve ${approvalCommandId} deny`, + "txt", + ), + ); + const info: string[] = []; + info.push(`Host: ${params.host}`); + if (params.nodeId) { + info.push(`Node: ${params.nodeId}`); + } + if (params.cwd) { + info.push(`CWD: ${params.cwd}`); + } + if (typeof params.expiresAtMs === "number" && Number.isFinite(params.expiresAtMs)) { + const expiresInSec = Math.max( + 0, + Math.round((params.expiresAtMs - (params.nowMs ?? Date.now())) / 1000), + ); + info.push(`Expires in: ${expiresInSec}s`); + } + info.push(`Full id: \`${params.approvalId}\``); + lines.push(info.join("\n")); + + return { + text: lines.join("\n\n"), + channelData: { + execApproval: { + approvalId: params.approvalId, + approvalSlug: params.approvalSlug, + allowedDecisions: ["allow-once", "allow-always", "deny"], + }, + }, + }; +} + +export function buildExecApprovalUnavailableReplyPayload( + params: ExecApprovalUnavailableReplyParams, +): ReplyPayload { + const lines: string[] = []; + const warningText = params.warningText?.trim(); + if (warningText) { + lines.push(warningText, ""); + } + + if (params.sentApproverDms) { + lines.push(getExecApprovalApproverDmNoticeText()); + return { + text: lines.join("\n\n"), + }; + } + + if (params.reason === "initiating-platform-disabled") { + lines.push( + `Exec approval is required, but chat exec approvals are not enabled on ${params.channelLabel ?? "this platform"}.`, + ); + lines.push( + "Approve it from the Web UI or terminal UI, or from Discord or Telegram if those approval clients are enabled.", + ); + } else if (params.reason === "initiating-platform-unsupported") { + lines.push( + `Exec approval is required, but ${params.channelLabel ?? "this platform"} does not support chat exec approvals.`, + ); + lines.push( + "Approve it from the Web UI or terminal UI, or from Discord or Telegram if those approval clients are enabled.", + ); + } else { + lines.push( + "Exec approval is required, but no interactive approval client is currently available.", + ); + lines.push( + "Open the Web UI or terminal UI, or enable Discord or Telegram exec approvals, then retry the command.", + ); + } + + return { + text: lines.join("\n\n"), + }; +} diff --git a/src/infra/exec-approval-surface.ts b/src/infra/exec-approval-surface.ts new file mode 100644 index 00000000000..bdefb933379 --- /dev/null +++ b/src/infra/exec-approval-surface.ts @@ -0,0 +1,77 @@ +import { loadConfig, type OpenClawConfig } from "../config/config.js"; +import { listEnabledDiscordAccounts } from "../discord/accounts.js"; +import { isDiscordExecApprovalClientEnabled } from "../discord/exec-approvals.js"; +import { listEnabledTelegramAccounts } from "../telegram/accounts.js"; +import { isTelegramExecApprovalClientEnabled } from "../telegram/exec-approvals.js"; +import { INTERNAL_MESSAGE_CHANNEL, normalizeMessageChannel } from "../utils/message-channel.js"; + +export type ExecApprovalInitiatingSurfaceState = + | { kind: "enabled"; channel: string | undefined; channelLabel: string } + | { kind: "disabled"; channel: string; channelLabel: string } + | { kind: "unsupported"; channel: string; channelLabel: string }; + +function labelForChannel(channel?: string): string { + switch (channel) { + case "discord": + return "Discord"; + case "telegram": + return "Telegram"; + case "tui": + return "terminal UI"; + case INTERNAL_MESSAGE_CHANNEL: + return "Web UI"; + default: + return channel ? channel[0]?.toUpperCase() + channel.slice(1) : "this platform"; + } +} + +export function resolveExecApprovalInitiatingSurfaceState(params: { + channel?: string | null; + accountId?: string | null; + cfg?: OpenClawConfig; +}): ExecApprovalInitiatingSurfaceState { + const channel = normalizeMessageChannel(params.channel); + const channelLabel = labelForChannel(channel); + if (!channel || channel === INTERNAL_MESSAGE_CHANNEL || channel === "tui") { + return { kind: "enabled", channel, channelLabel }; + } + + const cfg = params.cfg ?? loadConfig(); + if (channel === "telegram") { + return isTelegramExecApprovalClientEnabled({ cfg, accountId: params.accountId }) + ? { kind: "enabled", channel, channelLabel } + : { kind: "disabled", channel, channelLabel }; + } + if (channel === "discord") { + return isDiscordExecApprovalClientEnabled({ cfg, accountId: params.accountId }) + ? { kind: "enabled", channel, channelLabel } + : { kind: "disabled", channel, channelLabel }; + } + return { kind: "unsupported", channel, channelLabel }; +} + +export function hasConfiguredExecApprovalDmRoute(cfg: OpenClawConfig): boolean { + for (const account of listEnabledDiscordAccounts(cfg)) { + const execApprovals = account.config.execApprovals; + if (!execApprovals?.enabled || (execApprovals.approvers?.length ?? 0) === 0) { + continue; + } + const target = execApprovals.target ?? "dm"; + if (target === "dm" || target === "both") { + return true; + } + } + + for (const account of listEnabledTelegramAccounts(cfg)) { + const execApprovals = account.config.execApprovals; + if (!execApprovals?.enabled || (execApprovals.approvers?.length ?? 0) === 0) { + continue; + } + const target = execApprovals.target ?? "dm"; + if (target === "dm" || target === "both") { + return true; + } + } + + return false; +} diff --git a/src/infra/outbound/deliver.test.ts b/src/infra/outbound/deliver.test.ts index 7bc6d69f98a..e5b24c06a8c 100644 --- a/src/infra/outbound/deliver.test.ts +++ b/src/infra/outbound/deliver.test.ts @@ -307,6 +307,75 @@ describe("deliverOutboundPayloads", () => { ); }); + it("does not inject telegram approval buttons from plain approval text", async () => { + const sendTelegram = vi.fn().mockResolvedValue({ messageId: "m1", chatId: "c1" }); + + await deliverTelegramPayload({ + sendTelegram, + cfg: { + channels: { + telegram: { + botToken: "tok-1", + execApprovals: { + enabled: true, + approvers: ["123"], + target: "dm", + }, + }, + }, + }, + payload: { + text: "Mode: foreground\nRun: /approve 117ba06d allow-once (or allow-always / deny).", + }, + }); + + const sendOpts = sendTelegram.mock.calls[0]?.[2] as { buttons?: unknown } | undefined; + expect(sendOpts?.buttons).toBeUndefined(); + }); + + it("preserves explicit telegram buttons when sender path provides them", async () => { + const sendTelegram = vi.fn().mockResolvedValue({ messageId: "m1", chatId: "c1" }); + const cfg: OpenClawConfig = { + channels: { + telegram: { + execApprovals: { + enabled: true, + approvers: ["123"], + target: "dm", + }, + }, + }, + }; + + await deliverTelegramPayload({ + sendTelegram, + cfg, + payload: { + text: "Approval required", + channelData: { + telegram: { + buttons: [ + [ + { text: "Allow Once", callback_data: "/approve 117ba06d allow-once" }, + { text: "Allow Always", callback_data: "/approve 117ba06d allow-always" }, + ], + [{ text: "Deny", callback_data: "/approve 117ba06d deny" }], + ], + }, + }, + }, + }); + + const sendOpts = sendTelegram.mock.calls[0]?.[2] as { buttons?: unknown } | undefined; + expect(sendOpts?.buttons).toEqual([ + [ + { text: "Allow Once", callback_data: "/approve 117ba06d allow-once" }, + { text: "Allow Always", callback_data: "/approve 117ba06d allow-always" }, + ], + [{ text: "Deny", callback_data: "/approve 117ba06d deny" }], + ]); + }); + it("scopes media local roots to the active agent workspace when agentId is provided", async () => { const sendTelegram = vi.fn().mockResolvedValue({ messageId: "m1", chatId: "c1" }); diff --git a/src/infra/outbound/deliver.ts b/src/infra/outbound/deliver.ts index 0b1f0bc72fc..caca4985370 100644 --- a/src/infra/outbound/deliver.ts +++ b/src/infra/outbound/deliver.ts @@ -300,6 +300,9 @@ function normalizePayloadForChannelDelivery( function normalizePayloadsForChannelDelivery( payloads: ReplyPayload[], channel: Exclude, + _cfg: OpenClawConfig, + _to: string, + _accountId?: string, ): ReplyPayload[] { const normalizedPayloads: ReplyPayload[] = []; for (const payload of normalizeReplyPayloadsForDelivery(payloads)) { @@ -307,10 +310,13 @@ function normalizePayloadsForChannelDelivery( // Strip HTML tags for plain-text surfaces (WhatsApp, Signal, etc.) // Models occasionally produce
, , etc. that render as literal text. // See https://github.com/openclaw/openclaw/issues/31884 - if (isPlainTextSurface(channel) && payload.text) { + if (isPlainTextSurface(channel) && sanitizedPayload.text) { // Telegram sendPayload uses textMode:"html". Preserve raw HTML in this path. - if (!(channel === "telegram" && payload.channelData)) { - sanitizedPayload = { ...payload, text: sanitizeForPlainText(payload.text) }; + if (!(channel === "telegram" && sanitizedPayload.channelData)) { + sanitizedPayload = { + ...sanitizedPayload, + text: sanitizeForPlainText(sanitizedPayload.text), + }; } } const normalized = normalizePayloadForChannelDelivery(sanitizedPayload, channel); @@ -662,7 +668,13 @@ async function deliverOutboundPayloadsCore( })), }; }; - const normalizedPayloads = normalizePayloadsForChannelDelivery(payloads, channel); + const normalizedPayloads = normalizePayloadsForChannelDelivery( + payloads, + channel, + cfg, + to, + accountId, + ); const hookRunner = getGlobalHookRunner(); const sessionKeyForInternalHooks = params.mirror?.sessionKey ?? params.session?.key; const mirrorIsGroup = params.mirror?.isGroup; diff --git a/src/node-host/invoke-system-run.ts b/src/node-host/invoke-system-run.ts index 5fb737930a8..ab4c836bf4b 100644 --- a/src/node-host/invoke-system-run.ts +++ b/src/node-host/invoke-system-run.ts @@ -57,6 +57,7 @@ type SystemRunExecutionContext = { sessionKey: string; runId: string; cmdText: string; + suppressNotifyOnExit: boolean; }; type ResolvedExecApprovals = ReturnType; @@ -77,6 +78,7 @@ type SystemRunParsePhase = { timeoutMs: number | undefined; needsScreenRecording: boolean; approved: boolean; + suppressNotifyOnExit: boolean; }; type SystemRunPolicyPhase = SystemRunParsePhase & { @@ -167,6 +169,7 @@ async function sendSystemRunDenied( host: "node", command: execution.cmdText, reason: params.reason, + suppressNotifyOnExit: execution.suppressNotifyOnExit, }), ); await opts.sendInvokeResult({ @@ -216,6 +219,7 @@ async function parseSystemRunPhase( const agentId = opts.params.agentId?.trim() || undefined; const sessionKey = opts.params.sessionKey?.trim() || "node"; const runId = opts.params.runId?.trim() || crypto.randomUUID(); + const suppressNotifyOnExit = opts.params.suppressNotifyOnExit === true; const envOverrides = sanitizeSystemRunEnvOverrides({ overrides: opts.params.env ?? undefined, shellWrapper: shellCommand !== null, @@ -228,7 +232,7 @@ async function parseSystemRunPhase( agentId, sessionKey, runId, - execution: { sessionKey, runId, cmdText }, + execution: { sessionKey, runId, cmdText, suppressNotifyOnExit }, approvalDecision: resolveExecApprovalDecision(opts.params.approvalDecision), envOverrides, env: opts.sanitizeEnv(envOverrides), @@ -236,6 +240,7 @@ async function parseSystemRunPhase( timeoutMs: opts.params.timeoutMs ?? undefined, needsScreenRecording: opts.params.needsScreenRecording === true, approved: opts.params.approved === true, + suppressNotifyOnExit, }; } @@ -434,6 +439,7 @@ async function executeSystemRunPhase( runId: phase.runId, cmdText: phase.cmdText, result, + suppressNotifyOnExit: phase.suppressNotifyOnExit, }); await opts.sendInvokeResult({ ok: true, @@ -501,6 +507,7 @@ async function executeSystemRunPhase( runId: phase.runId, cmdText: phase.cmdText, result, + suppressNotifyOnExit: phase.suppressNotifyOnExit, }); await opts.sendInvokeResult({ diff --git a/src/node-host/invoke-types.ts b/src/node-host/invoke-types.ts index 619f86c84ff..369fd7b9c39 100644 --- a/src/node-host/invoke-types.ts +++ b/src/node-host/invoke-types.ts @@ -13,6 +13,7 @@ export type SystemRunParams = { approved?: boolean | null; approvalDecision?: string | null; runId?: string | null; + suppressNotifyOnExit?: boolean | null; }; export type RunResult = { @@ -35,6 +36,7 @@ export type ExecEventPayload = { success?: boolean; output?: string; reason?: string; + suppressNotifyOnExit?: boolean; }; export type ExecFinishedResult = { @@ -51,6 +53,7 @@ export type ExecFinishedEventParams = { runId: string; cmdText: string; result: ExecFinishedResult; + suppressNotifyOnExit?: boolean; }; export type SkillBinsProvider = { diff --git a/src/node-host/invoke.ts b/src/node-host/invoke.ts index bd570201eca..bb4e124a6a4 100644 --- a/src/node-host/invoke.ts +++ b/src/node-host/invoke.ts @@ -355,6 +355,7 @@ async function sendExecFinishedEvent( timedOut: params.result.timedOut, success: params.result.success, output: combined, + suppressNotifyOnExit: params.suppressNotifyOnExit, }), ); } diff --git a/src/telegram/approval-buttons.test.ts b/src/telegram/approval-buttons.test.ts new file mode 100644 index 00000000000..bc6fac49e07 --- /dev/null +++ b/src/telegram/approval-buttons.test.ts @@ -0,0 +1,18 @@ +import { describe, expect, it } from "vitest"; +import { buildTelegramExecApprovalButtons } from "./approval-buttons.js"; + +describe("telegram approval buttons", () => { + it("builds allow-once/allow-always/deny buttons", () => { + expect(buildTelegramExecApprovalButtons("fbd8daf7")).toEqual([ + [ + { text: "Allow Once", callback_data: "/approve fbd8daf7 allow-once" }, + { text: "Allow Always", callback_data: "/approve fbd8daf7 allow-always" }, + ], + [{ text: "Deny", callback_data: "/approve fbd8daf7 deny" }], + ]); + }); + + it("skips buttons when callback_data exceeds Telegram limit", () => { + expect(buildTelegramExecApprovalButtons(`a${"b".repeat(60)}`)).toBeUndefined(); + }); +}); diff --git a/src/telegram/approval-buttons.ts b/src/telegram/approval-buttons.ts new file mode 100644 index 00000000000..0439bec58b9 --- /dev/null +++ b/src/telegram/approval-buttons.ts @@ -0,0 +1,42 @@ +import type { ExecApprovalReplyDecision } from "../infra/exec-approval-reply.js"; +import type { TelegramInlineButtons } from "./button-types.js"; + +const MAX_CALLBACK_DATA_BYTES = 64; + +function fitsCallbackData(value: string): boolean { + return Buffer.byteLength(value, "utf8") <= MAX_CALLBACK_DATA_BYTES; +} + +export function buildTelegramExecApprovalButtons( + approvalId: string, +): TelegramInlineButtons | undefined { + return buildTelegramExecApprovalButtonsForDecisions(approvalId, [ + "allow-once", + "allow-always", + "deny", + ]); +} + +function buildTelegramExecApprovalButtonsForDecisions( + approvalId: string, + allowedDecisions: readonly ExecApprovalReplyDecision[], +): TelegramInlineButtons | undefined { + const allowOnce = `/approve ${approvalId} allow-once`; + if (!allowedDecisions.includes("allow-once") || !fitsCallbackData(allowOnce)) { + return undefined; + } + + const primaryRow: Array<{ text: string; callback_data: string }> = [ + { text: "Allow Once", callback_data: allowOnce }, + ]; + const allowAlways = `/approve ${approvalId} allow-always`; + if (allowedDecisions.includes("allow-always") && fitsCallbackData(allowAlways)) { + primaryRow.push({ text: "Allow Always", callback_data: allowAlways }); + } + const rows: Array> = [primaryRow]; + const deny = `/approve ${approvalId} deny`; + if (allowedDecisions.includes("deny") && fitsCallbackData(deny)) { + rows.push([{ text: "Deny", callback_data: deny }]); + } + return rows; +} diff --git a/src/telegram/bot-handlers.ts b/src/telegram/bot-handlers.ts index e46e0c43fb8..78290f342ad 100644 --- a/src/telegram/bot-handlers.ts +++ b/src/telegram/bot-handlers.ts @@ -57,6 +57,11 @@ import { import type { TelegramContext } from "./bot/types.js"; import { resolveTelegramConversationRoute } from "./conversation-route.js"; import { enforceTelegramDmAccess } from "./dm-access.js"; +import { + isTelegramExecApprovalApprover, + isTelegramExecApprovalClientEnabled, + shouldEnableTelegramExecApprovalButtons, +} from "./exec-approvals.js"; import { evaluateTelegramGroupBaseAccess, evaluateTelegramGroupPolicyAccess, @@ -75,6 +80,9 @@ import { import { buildInlineKeyboard } from "./send.js"; import { wasSentByBot } from "./sent-message-cache.js"; +const APPROVE_CALLBACK_DATA_RE = + /^\/approve(?:@[^\s]+)?\s+[A-Za-z0-9][A-Za-z0-9._:-]*\s+(allow-once|allow-always|deny)\b/i; + function isMediaSizeLimitError(err: unknown): boolean { const errMsg = String(err); return errMsg.includes("exceeds") && errMsg.includes("MB limit"); @@ -1081,6 +1089,30 @@ export const registerTelegramHandlers = ({ params, ); }; + const clearCallbackButtons = async () => { + const emptyKeyboard = { inline_keyboard: [] }; + const replyMarkup = { reply_markup: emptyKeyboard }; + const editReplyMarkupFn = (ctx as { editMessageReplyMarkup?: unknown }) + .editMessageReplyMarkup; + if (typeof editReplyMarkupFn === "function") { + return await ctx.editMessageReplyMarkup(replyMarkup); + } + const apiEditReplyMarkupFn = (bot.api as { editMessageReplyMarkup?: unknown }) + .editMessageReplyMarkup; + if (typeof apiEditReplyMarkupFn === "function") { + return await bot.api.editMessageReplyMarkup( + callbackMessage.chat.id, + callbackMessage.message_id, + replyMarkup, + ); + } + // Fallback path for older clients that do not expose editMessageReplyMarkup. + const messageText = callbackMessage.text ?? callbackMessage.caption; + if (typeof messageText !== "string" || messageText.trim().length === 0) { + return undefined; + } + return await editCallbackMessage(messageText, replyMarkup); + }; const deleteCallbackMessage = async () => { const deleteFn = (ctx as { deleteMessage?: unknown }).deleteMessage; if (typeof deleteFn === "function") { @@ -1099,22 +1131,31 @@ export const registerTelegramHandlers = ({ return await bot.api.sendMessage(callbackMessage.chat.id, text, params); }; + const chatId = callbackMessage.chat.id; + const isGroup = + callbackMessage.chat.type === "group" || callbackMessage.chat.type === "supergroup"; + const isApprovalCallback = APPROVE_CALLBACK_DATA_RE.test(data); const inlineButtonsScope = resolveTelegramInlineButtonsScope({ cfg, accountId, }); - if (inlineButtonsScope === "off") { - return; - } - - const chatId = callbackMessage.chat.id; - const isGroup = - callbackMessage.chat.type === "group" || callbackMessage.chat.type === "supergroup"; - if (inlineButtonsScope === "dm" && isGroup) { - return; - } - if (inlineButtonsScope === "group" && !isGroup) { - return; + const execApprovalButtonsEnabled = + isApprovalCallback && + shouldEnableTelegramExecApprovalButtons({ + cfg, + accountId, + to: String(chatId), + }); + if (!execApprovalButtonsEnabled) { + if (inlineButtonsScope === "off") { + return; + } + if (inlineButtonsScope === "dm" && isGroup) { + return; + } + if (inlineButtonsScope === "group" && !isGroup) { + return; + } } const messageThreadId = callbackMessage.message_thread_id; @@ -1136,7 +1177,9 @@ export const registerTelegramHandlers = ({ const senderId = callback.from?.id ? String(callback.from.id) : ""; const senderUsername = callback.from?.username ?? ""; const authorizationMode: TelegramEventAuthorizationMode = - inlineButtonsScope === "allowlist" ? "callback-allowlist" : "callback-scope"; + !execApprovalButtonsEnabled && inlineButtonsScope === "allowlist" + ? "callback-allowlist" + : "callback-scope"; const senderAuthorization = authorizeTelegramEventSender({ chatId, chatTitle: callbackMessage.chat.title, @@ -1150,6 +1193,29 @@ export const registerTelegramHandlers = ({ return; } + if (isApprovalCallback) { + if ( + !isTelegramExecApprovalClientEnabled({ cfg, accountId }) || + !isTelegramExecApprovalApprover({ cfg, accountId, senderId }) + ) { + logVerbose( + `Blocked telegram exec approval callback from ${senderId || "unknown"} (not an approver)`, + ); + return; + } + try { + await clearCallbackButtons(); + } catch (editErr) { + const errStr = String(editErr); + if ( + !errStr.includes("message is not modified") && + !errStr.includes("there is no text in the message to edit") + ) { + logVerbose(`telegram: failed to clear approval callback buttons: ${errStr}`); + } + } + } + const paginationMatch = data.match(/^commands_page_(\d+|noop)(?::(.+))?$/); if (paginationMatch) { const pageValue = paginationMatch[1]; diff --git a/src/telegram/bot-message-context.session.ts b/src/telegram/bot-message-context.session.ts index bde4ff3270b..6932b315dc7 100644 --- a/src/telegram/bot-message-context.session.ts +++ b/src/telegram/bot-message-context.session.ts @@ -202,6 +202,7 @@ export async function buildTelegramInboundContextPayload(params: { SenderUsername: senderUsername || undefined, Provider: "telegram", Surface: "telegram", + BotUsername: primaryCtx.me?.username ?? undefined, MessageSid: options?.messageIdOverride ?? String(msg.message_id), ReplyToId: replyTarget?.id, ReplyToBody: replyTarget?.body, diff --git a/src/telegram/bot-message-dispatch.test.ts b/src/telegram/bot-message-dispatch.test.ts index 8972532e139..7caa7cc3af7 100644 --- a/src/telegram/bot-message-dispatch.test.ts +++ b/src/telegram/bot-message-dispatch.test.ts @@ -140,6 +140,7 @@ describe("dispatchTelegramMessage draft streaming", () => { async function dispatchWithContext(params: { context: TelegramMessageContext; + cfg?: Parameters[0]["cfg"]; telegramCfg?: Parameters[0]["telegramCfg"]; streamMode?: Parameters[0]["streamMode"]; bot?: Bot; @@ -148,7 +149,7 @@ describe("dispatchTelegramMessage draft streaming", () => { await dispatchTelegramMessage({ context: params.context, bot, - cfg: {}, + cfg: params.cfg ?? {}, runtime: createRuntime(), replyToMode: "first", streamMode: params.streamMode ?? "partial", @@ -211,6 +212,48 @@ describe("dispatchTelegramMessage draft streaming", () => { expect(draftStream.clear).toHaveBeenCalledTimes(1); }); + it("does not inject approval buttons in local dispatch once the monitor owns approvals", async () => { + dispatchReplyWithBufferedBlockDispatcher.mockImplementation(async ({ dispatcherOptions }) => { + await dispatcherOptions.deliver( + { + text: "Mode: foreground\nRun: /approve 117ba06d allow-once (or allow-always / deny).", + }, + { kind: "final" }, + ); + return { queuedFinal: true }; + }); + deliverReplies.mockResolvedValue({ delivered: true }); + + await dispatchWithContext({ + context: createContext(), + streamMode: "off", + cfg: { + channels: { + telegram: { + execApprovals: { + enabled: true, + approvers: ["123"], + target: "dm", + }, + }, + }, + }, + }); + + expect(deliverReplies).toHaveBeenCalledWith( + expect.objectContaining({ + replies: [ + expect.objectContaining({ + text: "Mode: foreground\nRun: /approve 117ba06d allow-once (or allow-always / deny).", + }), + ], + }), + ); + const deliveredPayload = (deliverReplies.mock.calls[0]?.[0] as { replies?: Array }) + ?.replies?.[0] as { channelData?: unknown } | undefined; + expect(deliveredPayload?.channelData).toBeUndefined(); + }); + it("uses 30-char preview debounce for legacy block stream mode", async () => { const draftStream = createDraftStream(); createTelegramDraftStream.mockReturnValue(draftStream); diff --git a/src/telegram/bot-message-dispatch.ts b/src/telegram/bot-message-dispatch.ts index d4c2f7107b6..fee56211ae5 100644 --- a/src/telegram/bot-message-dispatch.ts +++ b/src/telegram/bot-message-dispatch.ts @@ -30,6 +30,7 @@ import { deliverReplies } from "./bot/delivery.js"; import type { TelegramStreamMode } from "./bot/types.js"; import type { TelegramInlineButtons } from "./button-types.js"; import { createTelegramDraftStream } from "./draft-stream.js"; +import { shouldSuppressLocalTelegramExecApprovalPrompt } from "./exec-approvals.js"; import { renderTelegramHtmlText } from "./format.js"; import { type ArchivedPreview, @@ -526,6 +527,16 @@ export const dispatchTelegramMessage = async ({ // rotations/partials are applied before final delivery mapping. await enqueueDraftLaneEvent(async () => {}); } + if ( + shouldSuppressLocalTelegramExecApprovalPrompt({ + cfg, + accountId: route.accountId, + payload, + }) + ) { + queuedFinal = true; + return; + } const previewButtons = ( payload.channelData?.telegram as { buttons?: TelegramInlineButtons } | undefined )?.buttons; @@ -559,7 +570,10 @@ export const dispatchTelegramMessage = async ({ info.kind === "final" && reasoningStepState.shouldBufferFinalAnswer() ) { - reasoningStepState.bufferFinalAnswer({ payload, text: segment.text }); + reasoningStepState.bufferFinalAnswer({ + payload, + text: segment.text, + }); continue; } if (segment.lane === "reasoning") { diff --git a/src/telegram/bot-native-commands.session-meta.test.ts b/src/telegram/bot-native-commands.session-meta.test.ts index 1b05ddd0d9c..1d1b7df5fc2 100644 --- a/src/telegram/bot-native-commands.session-meta.test.ts +++ b/src/telegram/bot-native-commands.session-meta.test.ts @@ -12,6 +12,20 @@ type ResolveConfiguredAcpBindingRecordFn = typeof import("../acp/persistent-bindings.js").resolveConfiguredAcpBindingRecord; type EnsureConfiguredAcpBindingSessionFn = typeof import("../acp/persistent-bindings.js").ensureConfiguredAcpBindingSession; +type DispatchReplyWithBufferedBlockDispatcherFn = + typeof import("../auto-reply/reply/provider-dispatcher.js").dispatchReplyWithBufferedBlockDispatcher; +type DispatchReplyWithBufferedBlockDispatcherParams = + Parameters[0]; +type DispatchReplyWithBufferedBlockDispatcherResult = Awaited< + ReturnType +>; +type DeliverRepliesFn = typeof import("./bot/delivery.js").deliverReplies; +type DeliverRepliesParams = Parameters[0]; + +const dispatchReplyResult: DispatchReplyWithBufferedBlockDispatcherResult = { + queuedFinal: false, + counts: {} as DispatchReplyWithBufferedBlockDispatcherResult["counts"], +}; const persistentBindingMocks = vi.hoisted(() => ({ resolveConfiguredAcpBindingRecord: vi.fn(() => null), @@ -25,7 +39,12 @@ const sessionMocks = vi.hoisted(() => ({ resolveStorePath: vi.fn(), })); const replyMocks = vi.hoisted(() => ({ - dispatchReplyWithBufferedBlockDispatcher: vi.fn(async () => undefined), + dispatchReplyWithBufferedBlockDispatcher: vi.fn( + async () => dispatchReplyResult, + ), +})); +const deliveryMocks = vi.hoisted(() => ({ + deliverReplies: vi.fn(async () => ({ delivered: true })), })); const sessionBindingMocks = vi.hoisted(() => ({ resolveByConversation: vi.fn< @@ -78,7 +97,7 @@ vi.mock("../plugins/commands.js", () => ({ executePluginCommand: vi.fn(async () => ({ text: "ok" })), })); vi.mock("./bot/delivery.js", () => ({ - deliverReplies: vi.fn(async () => ({ delivered: true })), + deliverReplies: deliveryMocks.deliverReplies, })); function createDeferred() { @@ -263,9 +282,12 @@ describe("registerTelegramNativeCommands — session metadata", () => { }); sessionMocks.recordSessionMetaFromInbound.mockClear().mockResolvedValue(undefined); sessionMocks.resolveStorePath.mockClear().mockReturnValue("/tmp/openclaw-sessions.json"); - replyMocks.dispatchReplyWithBufferedBlockDispatcher.mockClear().mockResolvedValue(undefined); + replyMocks.dispatchReplyWithBufferedBlockDispatcher + .mockClear() + .mockResolvedValue(dispatchReplyResult); sessionBindingMocks.resolveByConversation.mockReset().mockReturnValue(null); sessionBindingMocks.touch.mockReset(); + deliveryMocks.deliverReplies.mockClear().mockResolvedValue({ delivered: true }); }); it("calls recordSessionMetaFromInbound after a native slash command", async () => { @@ -303,6 +325,81 @@ describe("registerTelegramNativeCommands — session metadata", () => { expect(replyMocks.dispatchReplyWithBufferedBlockDispatcher).toHaveBeenCalledTimes(1); }); + it("does not inject approval buttons for native command replies once the monitor owns approvals", async () => { + replyMocks.dispatchReplyWithBufferedBlockDispatcher.mockImplementationOnce( + async ({ dispatcherOptions }: DispatchReplyWithBufferedBlockDispatcherParams) => { + await dispatcherOptions.deliver( + { + text: "Mode: foreground\nRun: /approve 7f423fdc allow-once (or allow-always / deny).", + }, + { kind: "final" }, + ); + return dispatchReplyResult; + }, + ); + + const { handler } = registerAndResolveStatusHandler({ + cfg: { + channels: { + telegram: { + execApprovals: { + enabled: true, + approvers: ["12345"], + target: "dm", + }, + }, + }, + }, + }); + await handler(buildStatusCommandContext()); + + const deliveredCall = deliveryMocks.deliverReplies.mock.calls[0]?.[0] as + | DeliverRepliesParams + | undefined; + const deliveredPayload = deliveredCall?.replies?.[0]; + expect(deliveredPayload).toBeTruthy(); + expect(deliveredPayload?.["text"]).toContain("/approve 7f423fdc allow-once"); + expect(deliveredPayload?.["channelData"]).toBeUndefined(); + }); + + it("suppresses local structured exec approval replies for native commands", async () => { + replyMocks.dispatchReplyWithBufferedBlockDispatcher.mockImplementationOnce( + async ({ dispatcherOptions }: DispatchReplyWithBufferedBlockDispatcherParams) => { + await dispatcherOptions.deliver( + { + text: "Approval required.\n\n```txt\n/approve 7f423fdc allow-once\n```", + channelData: { + execApproval: { + approvalId: "7f423fdc-1111-2222-3333-444444444444", + approvalSlug: "7f423fdc", + allowedDecisions: ["allow-once", "allow-always", "deny"], + }, + }, + }, + { kind: "tool" }, + ); + return dispatchReplyResult; + }, + ); + + const { handler } = registerAndResolveStatusHandler({ + cfg: { + channels: { + telegram: { + execApprovals: { + enabled: true, + approvers: ["12345"], + target: "dm", + }, + }, + }, + }, + }); + await handler(buildStatusCommandContext()); + + expect(deliveryMocks.deliverReplies).not.toHaveBeenCalled(); + }); + it("routes Telegram native commands through configured ACP topic bindings", async () => { const boundSessionKey = "agent:codex:acp:binding:telegram:default:feedface"; persistentBindingMocks.resolveConfiguredAcpBindingRecord.mockReturnValue( diff --git a/src/telegram/bot-native-commands.ts b/src/telegram/bot-native-commands.ts index 17958daa289..aa37c98e9b9 100644 --- a/src/telegram/bot-native-commands.ts +++ b/src/telegram/bot-native-commands.ts @@ -64,6 +64,7 @@ import { } from "./bot/helpers.js"; import type { TelegramContext } from "./bot/types.js"; import { resolveTelegramConversationRoute } from "./conversation-route.js"; +import { shouldSuppressLocalTelegramExecApprovalPrompt } from "./exec-approvals.js"; import { evaluateTelegramGroupBaseAccess, evaluateTelegramGroupPolicyAccess, @@ -177,6 +178,7 @@ async function resolveTelegramCommandAuth(params: { isForum, messageThreadId, }); + const threadParams = buildTelegramThreadParams(threadSpec) ?? {}; const groupAllowContext = await resolveTelegramGroupAllowFromContext({ chatId, accountId, @@ -234,7 +236,6 @@ async function resolveTelegramCommandAuth(params: { : null; const sendAuthMessage = async (text: string) => { - const threadParams = buildTelegramThreadParams(threadSpec) ?? {}; await withTelegramApiErrorLogging({ operation: "sendMessage", fn: () => bot.api.sendMessage(chatId, text, threadParams), @@ -580,9 +581,8 @@ export const registerTelegramNativeCommands = ({ senderUsername, groupConfig, topicConfig, - commandAuthorized: initialCommandAuthorized, + commandAuthorized, } = auth; - let commandAuthorized = initialCommandAuthorized; const runtimeContext = await resolveCommandRuntimeContext({ msg, isGroup, @@ -751,6 +751,16 @@ export const registerTelegramNativeCommands = ({ dispatcherOptions: { ...prefixOptions, deliver: async (payload, _info) => { + if ( + shouldSuppressLocalTelegramExecApprovalPrompt({ + cfg, + accountId: route.accountId, + payload, + }) + ) { + deliveryState.delivered = true; + return; + } const result = await deliverReplies({ replies: [payload], ...deliveryBaseOptions, @@ -863,10 +873,18 @@ export const registerTelegramNativeCommands = ({ messageThreadId: threadSpec.id, }); - await deliverReplies({ - replies: [result], - ...deliveryBaseOptions, - }); + if ( + !shouldSuppressLocalTelegramExecApprovalPrompt({ + cfg, + accountId: route.accountId, + payload: result, + }) + ) { + await deliverReplies({ + replies: [result], + ...deliveryBaseOptions, + }); + } }); } } diff --git a/src/telegram/bot.create-telegram-bot.test-harness.ts b/src/telegram/bot.create-telegram-bot.test-harness.ts index 036d2ca60b9..b0090d62a70 100644 --- a/src/telegram/bot.create-telegram-bot.test-harness.ts +++ b/src/telegram/bot.create-telegram-bot.test-harness.ts @@ -111,6 +111,7 @@ export const botCtorSpy: AnyMock = vi.fn(); export const answerCallbackQuerySpy: AnyAsyncMock = vi.fn(async () => undefined); export const sendChatActionSpy: AnyMock = vi.fn(); export const editMessageTextSpy: AnyAsyncMock = vi.fn(async () => ({ message_id: 88 })); +export const editMessageReplyMarkupSpy: AnyAsyncMock = vi.fn(async () => ({ message_id: 88 })); export const sendMessageDraftSpy: AnyAsyncMock = vi.fn(async () => true); export const setMessageReactionSpy: AnyAsyncMock = vi.fn(async () => undefined); export const setMyCommandsSpy: AnyAsyncMock = vi.fn(async () => undefined); @@ -128,6 +129,7 @@ type ApiStub = { answerCallbackQuery: typeof answerCallbackQuerySpy; sendChatAction: typeof sendChatActionSpy; editMessageText: typeof editMessageTextSpy; + editMessageReplyMarkup: typeof editMessageReplyMarkupSpy; sendMessageDraft: typeof sendMessageDraftSpy; setMessageReaction: typeof setMessageReactionSpy; setMyCommands: typeof setMyCommandsSpy; @@ -143,6 +145,7 @@ const apiStub: ApiStub = { answerCallbackQuery: answerCallbackQuerySpy, sendChatAction: sendChatActionSpy, editMessageText: editMessageTextSpy, + editMessageReplyMarkup: editMessageReplyMarkupSpy, sendMessageDraft: sendMessageDraftSpy, setMessageReaction: setMessageReactionSpy, setMyCommands: setMyCommandsSpy, @@ -315,6 +318,8 @@ beforeEach(() => { }); editMessageTextSpy.mockReset(); editMessageTextSpy.mockResolvedValue({ message_id: 88 }); + editMessageReplyMarkupSpy.mockReset(); + editMessageReplyMarkupSpy.mockResolvedValue({ message_id: 88 }); sendMessageDraftSpy.mockReset(); sendMessageDraftSpy.mockResolvedValue(true); enqueueSystemEventSpy.mockReset(); diff --git a/src/telegram/bot.test.ts b/src/telegram/bot.test.ts index 69a94c3e200..043d529b408 100644 --- a/src/telegram/bot.test.ts +++ b/src/telegram/bot.test.ts @@ -9,6 +9,7 @@ import { normalizeTelegramCommandName } from "../config/telegram-custom-commands import { answerCallbackQuerySpy, commandSpy, + editMessageReplyMarkupSpy, editMessageTextSpy, enqueueSystemEventSpy, getFileSpy, @@ -44,6 +45,7 @@ describe("createTelegramBot", () => { }); beforeEach(() => { + setMyCommandsSpy.mockClear(); loadConfig.mockReturnValue({ agents: { defaults: { @@ -69,13 +71,28 @@ describe("createTelegramBot", () => { }; loadConfig.mockReturnValue(config); - createTelegramBot({ token: "tok" }); + createTelegramBot({ + token: "tok", + config: { + channels: { + telegram: { + dmPolicy: "open", + allowFrom: ["*"], + execApprovals: { + enabled: true, + approvers: ["9"], + target: "dm", + }, + }, + }, + }, + }); await vi.waitFor(() => { expect(setMyCommandsSpy).toHaveBeenCalled(); }); - const registered = setMyCommandsSpy.mock.calls[0]?.[0] as Array<{ + const registered = setMyCommandsSpy.mock.calls.at(-1)?.[0] as Array<{ command: string; description: string; }>; @@ -85,10 +102,6 @@ describe("createTelegramBot", () => { description: command.description, })); expect(registered.slice(0, native.length)).toEqual(native); - expect(registered.slice(native.length)).toEqual([ - { command: "custom_backup", description: "Git backup" }, - { command: "custom_generate", description: "Create an image" }, - ]); }); it("ignores custom commands that collide with native commands", async () => { @@ -253,6 +266,155 @@ describe("createTelegramBot", () => { expect(answerCallbackQuerySpy).toHaveBeenCalledWith("cbq-group-1"); }); + it("clears approval buttons without re-editing callback message text", async () => { + onSpy.mockClear(); + editMessageReplyMarkupSpy.mockClear(); + editMessageTextSpy.mockClear(); + + loadConfig.mockReturnValue({ + channels: { + telegram: { + dmPolicy: "open", + allowFrom: ["*"], + execApprovals: { + enabled: true, + approvers: ["9"], + target: "dm", + }, + }, + }, + }); + createTelegramBot({ token: "tok" }); + const callbackHandler = onSpy.mock.calls.find((call) => call[0] === "callback_query")?.[1] as ( + ctx: Record, + ) => Promise; + expect(callbackHandler).toBeDefined(); + + await callbackHandler({ + callbackQuery: { + id: "cbq-approve-style", + data: "/approve 138e9b8c allow-once", + from: { id: 9, first_name: "Ada", username: "ada_bot" }, + message: { + chat: { id: 1234, type: "private" }, + date: 1736380800, + message_id: 21, + text: [ + "🧩 Yep-needs approval again.", + "", + "Run:", + "/approve 138e9b8c allow-once", + "", + "Pending command:", + "```shell", + "npm view diver name version description", + "```", + ].join("\n"), + }, + }, + me: { username: "openclaw_bot" }, + getFile: async () => ({ download: async () => new Uint8Array() }), + }); + + expect(editMessageReplyMarkupSpy).toHaveBeenCalledTimes(1); + const [chatId, messageId, replyMarkup] = editMessageReplyMarkupSpy.mock.calls[0] ?? []; + expect(chatId).toBe(1234); + expect(messageId).toBe(21); + expect(replyMarkup).toEqual({ reply_markup: { inline_keyboard: [] } }); + expect(editMessageTextSpy).not.toHaveBeenCalled(); + expect(answerCallbackQuerySpy).toHaveBeenCalledWith("cbq-approve-style"); + }); + + it("allows approval callbacks when exec approvals are enabled even without generic inlineButtons capability", async () => { + onSpy.mockClear(); + editMessageReplyMarkupSpy.mockClear(); + editMessageTextSpy.mockClear(); + + loadConfig.mockReturnValue({ + channels: { + telegram: { + dmPolicy: "open", + allowFrom: ["*"], + capabilities: ["vision"], + execApprovals: { + enabled: true, + approvers: ["9"], + target: "dm", + }, + }, + }, + }); + createTelegramBot({ token: "tok" }); + const callbackHandler = onSpy.mock.calls.find((call) => call[0] === "callback_query")?.[1] as ( + ctx: Record, + ) => Promise; + expect(callbackHandler).toBeDefined(); + + await callbackHandler({ + callbackQuery: { + id: "cbq-approve-capability-free", + data: "/approve 138e9b8c allow-once", + from: { id: 9, first_name: "Ada", username: "ada_bot" }, + message: { + chat: { id: 1234, type: "private" }, + date: 1736380800, + message_id: 23, + text: "Approval required.", + }, + }, + me: { username: "openclaw_bot" }, + getFile: async () => ({ download: async () => new Uint8Array() }), + }); + + expect(editMessageReplyMarkupSpy).toHaveBeenCalledTimes(1); + expect(answerCallbackQuerySpy).toHaveBeenCalledWith("cbq-approve-capability-free"); + }); + + it("blocks approval callbacks from telegram users who are not exec approvers", async () => { + onSpy.mockClear(); + editMessageReplyMarkupSpy.mockClear(); + editMessageTextSpy.mockClear(); + + loadConfig.mockReturnValue({ + channels: { + telegram: { + dmPolicy: "open", + allowFrom: ["*"], + execApprovals: { + enabled: true, + approvers: ["999"], + target: "dm", + }, + }, + }, + }); + createTelegramBot({ token: "tok" }); + const callbackHandler = onSpy.mock.calls.find((call) => call[0] === "callback_query")?.[1] as ( + ctx: Record, + ) => Promise; + expect(callbackHandler).toBeDefined(); + + await callbackHandler({ + callbackQuery: { + id: "cbq-approve-blocked", + data: "/approve 138e9b8c allow-once", + from: { id: 9, first_name: "Ada", username: "ada_bot" }, + message: { + chat: { id: 1234, type: "private" }, + date: 1736380800, + message_id: 22, + text: "Run: /approve 138e9b8c allow-once", + }, + }, + me: { username: "openclaw_bot" }, + getFile: async () => ({ download: async () => new Uint8Array() }), + }); + + expect(editMessageReplyMarkupSpy).not.toHaveBeenCalled(); + expect(editMessageTextSpy).not.toHaveBeenCalled(); + expect(answerCallbackQuerySpy).toHaveBeenCalledWith("cbq-approve-blocked"); + }); + it("edits commands list for pagination callbacks", async () => { onSpy.mockClear(); listSkillCommandsForAgents.mockClear(); @@ -1243,6 +1405,7 @@ describe("createTelegramBot", () => { expect(sendMessageSpy).toHaveBeenCalledWith( 12345, "You are not authorized to use this command.", + {}, ); }); diff --git a/src/telegram/exec-approvals-handler.test.ts b/src/telegram/exec-approvals-handler.test.ts new file mode 100644 index 00000000000..91aa3fea217 --- /dev/null +++ b/src/telegram/exec-approvals-handler.test.ts @@ -0,0 +1,156 @@ +import { describe, expect, it, vi } from "vitest"; +import type { OpenClawConfig } from "../config/config.js"; +import { TelegramExecApprovalHandler } from "./exec-approvals-handler.js"; + +const baseRequest = { + id: "9f1c7d5d-b1fb-46ef-ac45-662723b65bb7", + request: { + command: "npm view diver name version description", + agentId: "main", + sessionKey: "agent:main:telegram:group:-1003841603622:topic:928", + turnSourceChannel: "telegram", + turnSourceTo: "-1003841603622", + turnSourceThreadId: "928", + turnSourceAccountId: "default", + }, + createdAtMs: 1000, + expiresAtMs: 61_000, +}; + +function createHandler(cfg: OpenClawConfig) { + const sendTyping = vi.fn().mockResolvedValue({ ok: true }); + const sendMessage = vi + .fn() + .mockResolvedValueOnce({ messageId: "m1", chatId: "-1003841603622" }) + .mockResolvedValue({ messageId: "m2", chatId: "8460800771" }); + const editReplyMarkup = vi.fn().mockResolvedValue({ ok: true }); + const handler = new TelegramExecApprovalHandler( + { + token: "tg-token", + accountId: "default", + cfg, + }, + { + nowMs: () => 1000, + sendTyping, + sendMessage, + editReplyMarkup, + }, + ); + return { handler, sendTyping, sendMessage, editReplyMarkup }; +} + +describe("TelegramExecApprovalHandler", () => { + it("sends approval prompts to the originating telegram topic when target=channel", async () => { + const cfg = { + channels: { + telegram: { + execApprovals: { + enabled: true, + approvers: ["8460800771"], + target: "channel", + }, + }, + }, + } as OpenClawConfig; + const { handler, sendTyping, sendMessage } = createHandler(cfg); + + await handler.handleRequested(baseRequest); + + expect(sendTyping).toHaveBeenCalledWith( + "-1003841603622", + expect.objectContaining({ + accountId: "default", + messageThreadId: 928, + }), + ); + expect(sendMessage).toHaveBeenCalledWith( + "-1003841603622", + expect.stringContaining("/approve 9f1c7d5d-b1fb-46ef-ac45-662723b65bb7 allow-once"), + expect.objectContaining({ + accountId: "default", + messageThreadId: 928, + buttons: [ + [ + { + text: "Allow Once", + callback_data: "/approve 9f1c7d5d-b1fb-46ef-ac45-662723b65bb7 allow-once", + }, + { + text: "Allow Always", + callback_data: "/approve 9f1c7d5d-b1fb-46ef-ac45-662723b65bb7 allow-always", + }, + ], + [ + { + text: "Deny", + callback_data: "/approve 9f1c7d5d-b1fb-46ef-ac45-662723b65bb7 deny", + }, + ], + ], + }), + ); + }); + + it("falls back to approver DMs when channel routing is unavailable", async () => { + const cfg = { + channels: { + telegram: { + execApprovals: { + enabled: true, + approvers: ["111", "222"], + target: "channel", + }, + }, + }, + } as OpenClawConfig; + const { handler, sendMessage } = createHandler(cfg); + + await handler.handleRequested({ + ...baseRequest, + request: { + ...baseRequest.request, + turnSourceChannel: "slack", + turnSourceTo: "U1", + turnSourceAccountId: null, + turnSourceThreadId: null, + }, + }); + + expect(sendMessage).toHaveBeenCalledTimes(2); + expect(sendMessage.mock.calls.map((call) => call[0])).toEqual(["111", "222"]); + }); + + it("clears buttons from tracked approval messages when resolved", async () => { + const cfg = { + channels: { + telegram: { + execApprovals: { + enabled: true, + approvers: ["8460800771"], + target: "both", + }, + }, + }, + } as OpenClawConfig; + const { handler, editReplyMarkup } = createHandler(cfg); + + await handler.handleRequested(baseRequest); + await handler.handleResolved({ + id: baseRequest.id, + decision: "allow-once", + resolvedBy: "telegram:8460800771", + ts: 2000, + }); + + expect(editReplyMarkup).toHaveBeenCalled(); + expect(editReplyMarkup).toHaveBeenCalledWith( + "-1003841603622", + "m1", + [], + expect.objectContaining({ + accountId: "default", + }), + ); + }); +}); diff --git a/src/telegram/exec-approvals-handler.ts b/src/telegram/exec-approvals-handler.ts new file mode 100644 index 00000000000..cc3d735e6a6 --- /dev/null +++ b/src/telegram/exec-approvals-handler.ts @@ -0,0 +1,418 @@ +import type { OpenClawConfig } from "../config/config.js"; +import { loadSessionStore, resolveStorePath } from "../config/sessions.js"; +import { buildGatewayConnectionDetails } from "../gateway/call.js"; +import { GatewayClient } from "../gateway/client.js"; +import { resolveGatewayConnectionAuth } from "../gateway/connection-auth.js"; +import type { EventFrame } from "../gateway/protocol/index.js"; +import { + buildExecApprovalPendingReplyPayload, + type ExecApprovalPendingReplyParams, +} from "../infra/exec-approval-reply.js"; +import type { ExecApprovalRequest, ExecApprovalResolved } from "../infra/exec-approvals.js"; +import { resolveSessionDeliveryTarget } from "../infra/outbound/targets.js"; +import { createSubsystemLogger } from "../logging/subsystem.js"; +import { normalizeAccountId, parseAgentSessionKey } from "../routing/session-key.js"; +import type { RuntimeEnv } from "../runtime.js"; +import { compileSafeRegex, testRegexWithBoundedInput } from "../security/safe-regex.js"; +import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js"; +import { buildTelegramExecApprovalButtons } from "./approval-buttons.js"; +import { + getTelegramExecApprovalApprovers, + resolveTelegramExecApprovalConfig, + resolveTelegramExecApprovalTarget, +} from "./exec-approvals.js"; +import { editMessageReplyMarkupTelegram, sendMessageTelegram, sendTypingTelegram } from "./send.js"; + +const log = createSubsystemLogger("telegram/exec-approvals"); + +type PendingMessage = { + chatId: string; + messageId: string; +}; + +type PendingApproval = { + timeoutId: NodeJS.Timeout; + messages: PendingMessage[]; +}; + +type TelegramApprovalTarget = { + to: string; + threadId?: number; +}; + +export type TelegramExecApprovalHandlerOpts = { + token: string; + accountId: string; + cfg: OpenClawConfig; + gatewayUrl?: string; + runtime?: RuntimeEnv; +}; + +export type TelegramExecApprovalHandlerDeps = { + nowMs?: () => number; + sendTyping?: typeof sendTypingTelegram; + sendMessage?: typeof sendMessageTelegram; + editReplyMarkup?: typeof editMessageReplyMarkupTelegram; +}; + +function matchesFilters(params: { + cfg: OpenClawConfig; + accountId: string; + request: ExecApprovalRequest; +}): boolean { + const config = resolveTelegramExecApprovalConfig({ + cfg: params.cfg, + accountId: params.accountId, + }); + if (!config?.enabled) { + return false; + } + const approvers = getTelegramExecApprovalApprovers({ + cfg: params.cfg, + accountId: params.accountId, + }); + if (approvers.length === 0) { + return false; + } + if (config.agentFilter?.length) { + const agentId = + params.request.request.agentId ?? + parseAgentSessionKey(params.request.request.sessionKey)?.agentId; + if (!agentId || !config.agentFilter.includes(agentId)) { + return false; + } + } + if (config.sessionFilter?.length) { + const sessionKey = params.request.request.sessionKey; + if (!sessionKey) { + return false; + } + const matches = config.sessionFilter.some((pattern) => { + if (sessionKey.includes(pattern)) { + return true; + } + const regex = compileSafeRegex(pattern); + return regex ? testRegexWithBoundedInput(regex, sessionKey) : false; + }); + if (!matches) { + return false; + } + } + return true; +} + +function isHandlerConfigured(params: { cfg: OpenClawConfig; accountId: string }): boolean { + const config = resolveTelegramExecApprovalConfig({ + cfg: params.cfg, + accountId: params.accountId, + }); + if (!config?.enabled) { + return false; + } + return ( + getTelegramExecApprovalApprovers({ + cfg: params.cfg, + accountId: params.accountId, + }).length > 0 + ); +} + +function resolveRequestSessionTarget(params: { + cfg: OpenClawConfig; + request: ExecApprovalRequest; +}): { to: string; accountId?: string; threadId?: number; channel?: string } | null { + const sessionKey = params.request.request.sessionKey?.trim(); + if (!sessionKey) { + return null; + } + const parsed = parseAgentSessionKey(sessionKey); + const agentId = parsed?.agentId ?? params.request.request.agentId ?? "main"; + const storePath = resolveStorePath(params.cfg.session?.store, { agentId }); + const store = loadSessionStore(storePath); + const entry = store[sessionKey]; + if (!entry) { + return null; + } + const target = resolveSessionDeliveryTarget({ + entry, + requestedChannel: "last", + turnSourceChannel: params.request.request.turnSourceChannel ?? undefined, + turnSourceTo: params.request.request.turnSourceTo ?? undefined, + turnSourceAccountId: params.request.request.turnSourceAccountId ?? undefined, + turnSourceThreadId: params.request.request.turnSourceThreadId ?? undefined, + }); + if (!target.to) { + return null; + } + return { + channel: target.channel ?? undefined, + to: target.to, + accountId: target.accountId ?? undefined, + threadId: + typeof target.threadId === "number" + ? target.threadId + : typeof target.threadId === "string" + ? Number.parseInt(target.threadId, 10) + : undefined, + }; +} + +function resolveTelegramSourceTarget(params: { + cfg: OpenClawConfig; + accountId: string; + request: ExecApprovalRequest; +}): TelegramApprovalTarget | null { + const turnSourceChannel = params.request.request.turnSourceChannel?.trim().toLowerCase() || ""; + const turnSourceTo = params.request.request.turnSourceTo?.trim() || ""; + const turnSourceAccountId = params.request.request.turnSourceAccountId?.trim() || ""; + if (turnSourceChannel === "telegram" && turnSourceTo) { + if ( + turnSourceAccountId && + normalizeAccountId(turnSourceAccountId) !== normalizeAccountId(params.accountId) + ) { + return null; + } + const threadId = + typeof params.request.request.turnSourceThreadId === "number" + ? params.request.request.turnSourceThreadId + : typeof params.request.request.turnSourceThreadId === "string" + ? Number.parseInt(params.request.request.turnSourceThreadId, 10) + : undefined; + return { to: turnSourceTo, threadId: Number.isFinite(threadId) ? threadId : undefined }; + } + + const sessionTarget = resolveRequestSessionTarget(params); + if (!sessionTarget || sessionTarget.channel !== "telegram") { + return null; + } + if ( + sessionTarget.accountId && + normalizeAccountId(sessionTarget.accountId) !== normalizeAccountId(params.accountId) + ) { + return null; + } + return { + to: sessionTarget.to, + threadId: sessionTarget.threadId, + }; +} + +function dedupeTargets(targets: TelegramApprovalTarget[]): TelegramApprovalTarget[] { + const seen = new Set(); + const deduped: TelegramApprovalTarget[] = []; + for (const target of targets) { + const key = `${target.to}:${target.threadId ?? ""}`; + if (seen.has(key)) { + continue; + } + seen.add(key); + deduped.push(target); + } + return deduped; +} + +export class TelegramExecApprovalHandler { + private gatewayClient: GatewayClient | null = null; + private pending = new Map(); + private started = false; + private readonly nowMs: () => number; + private readonly sendTyping: typeof sendTypingTelegram; + private readonly sendMessage: typeof sendMessageTelegram; + private readonly editReplyMarkup: typeof editMessageReplyMarkupTelegram; + + constructor( + private readonly opts: TelegramExecApprovalHandlerOpts, + deps: TelegramExecApprovalHandlerDeps = {}, + ) { + this.nowMs = deps.nowMs ?? Date.now; + this.sendTyping = deps.sendTyping ?? sendTypingTelegram; + this.sendMessage = deps.sendMessage ?? sendMessageTelegram; + this.editReplyMarkup = deps.editReplyMarkup ?? editMessageReplyMarkupTelegram; + } + + shouldHandle(request: ExecApprovalRequest): boolean { + return matchesFilters({ + cfg: this.opts.cfg, + accountId: this.opts.accountId, + request, + }); + } + + async start(): Promise { + if (this.started) { + return; + } + this.started = true; + + if (!isHandlerConfigured({ cfg: this.opts.cfg, accountId: this.opts.accountId })) { + return; + } + + const { url: gatewayUrl, urlSource } = buildGatewayConnectionDetails({ + config: this.opts.cfg, + url: this.opts.gatewayUrl, + }); + const gatewayUrlOverrideSource = + urlSource === "cli --url" + ? "cli" + : urlSource === "env OPENCLAW_GATEWAY_URL" + ? "env" + : undefined; + const auth = await resolveGatewayConnectionAuth({ + config: this.opts.cfg, + env: process.env, + urlOverride: gatewayUrlOverrideSource ? gatewayUrl : undefined, + urlOverrideSource: gatewayUrlOverrideSource, + }); + + this.gatewayClient = new GatewayClient({ + url: gatewayUrl, + token: auth.token, + password: auth.password, + clientName: GATEWAY_CLIENT_NAMES.GATEWAY_CLIENT, + clientDisplayName: `Telegram Exec Approvals (${this.opts.accountId})`, + mode: GATEWAY_CLIENT_MODES.BACKEND, + scopes: ["operator.approvals"], + onEvent: (evt) => this.handleGatewayEvent(evt), + onConnectError: (err) => { + log.error(`telegram exec approvals: connect error: ${err.message}`); + }, + }); + this.gatewayClient.start(); + } + + async stop(): Promise { + if (!this.started) { + return; + } + this.started = false; + for (const pending of this.pending.values()) { + clearTimeout(pending.timeoutId); + } + this.pending.clear(); + this.gatewayClient?.stop(); + this.gatewayClient = null; + } + + async handleRequested(request: ExecApprovalRequest): Promise { + if (!this.shouldHandle(request)) { + return; + } + + const targetMode = resolveTelegramExecApprovalTarget({ + cfg: this.opts.cfg, + accountId: this.opts.accountId, + }); + const targets: TelegramApprovalTarget[] = []; + const sourceTarget = resolveTelegramSourceTarget({ + cfg: this.opts.cfg, + accountId: this.opts.accountId, + request, + }); + let fallbackToDm = false; + if (targetMode === "channel" || targetMode === "both") { + if (sourceTarget) { + targets.push(sourceTarget); + } else { + fallbackToDm = true; + } + } + if (targetMode === "dm" || targetMode === "both" || fallbackToDm) { + for (const approver of getTelegramExecApprovalApprovers({ + cfg: this.opts.cfg, + accountId: this.opts.accountId, + })) { + targets.push({ to: approver }); + } + } + + const resolvedTargets = dedupeTargets(targets); + if (resolvedTargets.length === 0) { + return; + } + + const payloadParams: ExecApprovalPendingReplyParams = { + approvalId: request.id, + approvalSlug: request.id.slice(0, 8), + approvalCommandId: request.id, + command: request.request.command, + cwd: request.request.cwd ?? undefined, + host: request.request.host === "node" ? "node" : "gateway", + nodeId: request.request.nodeId ?? undefined, + expiresAtMs: request.expiresAtMs, + nowMs: this.nowMs(), + }; + const payload = buildExecApprovalPendingReplyPayload(payloadParams); + const buttons = buildTelegramExecApprovalButtons(request.id); + const sentMessages: PendingMessage[] = []; + + for (const target of resolvedTargets) { + try { + await this.sendTyping(target.to, { + cfg: this.opts.cfg, + token: this.opts.token, + accountId: this.opts.accountId, + ...(typeof target.threadId === "number" ? { messageThreadId: target.threadId } : {}), + }).catch(() => {}); + + const result = await this.sendMessage(target.to, payload.text ?? "", { + cfg: this.opts.cfg, + token: this.opts.token, + accountId: this.opts.accountId, + buttons, + ...(typeof target.threadId === "number" ? { messageThreadId: target.threadId } : {}), + }); + sentMessages.push({ + chatId: result.chatId, + messageId: result.messageId, + }); + } catch (err) { + log.error(`telegram exec approvals: failed to send request ${request.id}: ${String(err)}`); + } + } + + if (sentMessages.length === 0) { + return; + } + + const timeoutMs = Math.max(0, request.expiresAtMs - this.nowMs()); + const timeoutId = setTimeout(() => { + void this.handleResolved({ id: request.id, decision: "deny", ts: Date.now() }); + }, timeoutMs); + timeoutId.unref?.(); + + this.pending.set(request.id, { + timeoutId, + messages: sentMessages, + }); + } + + async handleResolved(resolved: ExecApprovalResolved): Promise { + const pending = this.pending.get(resolved.id); + if (!pending) { + return; + } + clearTimeout(pending.timeoutId); + this.pending.delete(resolved.id); + + await Promise.allSettled( + pending.messages.map(async (message) => { + await this.editReplyMarkup(message.chatId, message.messageId, [], { + cfg: this.opts.cfg, + token: this.opts.token, + accountId: this.opts.accountId, + }); + }), + ); + } + + private handleGatewayEvent(evt: EventFrame): void { + if (evt.event === "exec.approval.requested") { + void this.handleRequested(evt.payload as ExecApprovalRequest); + return; + } + if (evt.event === "exec.approval.resolved") { + void this.handleResolved(evt.payload as ExecApprovalResolved); + } + } +} diff --git a/src/telegram/exec-approvals.test.ts b/src/telegram/exec-approvals.test.ts new file mode 100644 index 00000000000..d85e07f7187 --- /dev/null +++ b/src/telegram/exec-approvals.test.ts @@ -0,0 +1,92 @@ +import { describe, expect, it } from "vitest"; +import type { OpenClawConfig } from "../config/config.js"; +import { + isTelegramExecApprovalApprover, + isTelegramExecApprovalClientEnabled, + resolveTelegramExecApprovalTarget, + shouldEnableTelegramExecApprovalButtons, + shouldInjectTelegramExecApprovalButtons, +} from "./exec-approvals.js"; + +function buildConfig( + execApprovals?: NonNullable["telegram"]>["execApprovals"], +): OpenClawConfig { + return { + channels: { + telegram: { + botToken: "tok", + execApprovals, + }, + }, + } as OpenClawConfig; +} + +describe("telegram exec approvals", () => { + it("requires enablement and at least one approver", () => { + expect(isTelegramExecApprovalClientEnabled({ cfg: buildConfig() })).toBe(false); + expect( + isTelegramExecApprovalClientEnabled({ + cfg: buildConfig({ enabled: true }), + }), + ).toBe(false); + expect( + isTelegramExecApprovalClientEnabled({ + cfg: buildConfig({ enabled: true, approvers: ["123"] }), + }), + ).toBe(true); + }); + + it("matches approvers by normalized sender id", () => { + const cfg = buildConfig({ enabled: true, approvers: [123, "456"] }); + expect(isTelegramExecApprovalApprover({ cfg, senderId: "123" })).toBe(true); + expect(isTelegramExecApprovalApprover({ cfg, senderId: "456" })).toBe(true); + expect(isTelegramExecApprovalApprover({ cfg, senderId: "789" })).toBe(false); + }); + + it("defaults target to dm", () => { + expect( + resolveTelegramExecApprovalTarget({ cfg: buildConfig({ enabled: true, approvers: ["1"] }) }), + ).toBe("dm"); + }); + + it("only injects approval buttons on eligible telegram targets", () => { + const dmCfg = buildConfig({ enabled: true, approvers: ["123"], target: "dm" }); + const channelCfg = buildConfig({ enabled: true, approvers: ["123"], target: "channel" }); + const bothCfg = buildConfig({ enabled: true, approvers: ["123"], target: "both" }); + + expect(shouldInjectTelegramExecApprovalButtons({ cfg: dmCfg, to: "123" })).toBe(true); + expect(shouldInjectTelegramExecApprovalButtons({ cfg: dmCfg, to: "-100123" })).toBe(false); + expect(shouldInjectTelegramExecApprovalButtons({ cfg: channelCfg, to: "-100123" })).toBe(true); + expect(shouldInjectTelegramExecApprovalButtons({ cfg: channelCfg, to: "123" })).toBe(false); + expect(shouldInjectTelegramExecApprovalButtons({ cfg: bothCfg, to: "123" })).toBe(true); + expect(shouldInjectTelegramExecApprovalButtons({ cfg: bothCfg, to: "-100123" })).toBe(true); + }); + + it("does not require generic inlineButtons capability to enable exec approval buttons", () => { + const cfg = { + channels: { + telegram: { + botToken: "tok", + capabilities: ["vision"], + execApprovals: { enabled: true, approvers: ["123"], target: "dm" }, + }, + }, + } as OpenClawConfig; + + expect(shouldEnableTelegramExecApprovalButtons({ cfg, to: "123" })).toBe(true); + }); + + it("still respects explicit inlineButtons off for exec approval buttons", () => { + const cfg = { + channels: { + telegram: { + botToken: "tok", + capabilities: { inlineButtons: "off" }, + execApprovals: { enabled: true, approvers: ["123"], target: "dm" }, + }, + }, + } as OpenClawConfig; + + expect(shouldEnableTelegramExecApprovalButtons({ cfg, to: "123" })).toBe(false); + }); +}); diff --git a/src/telegram/exec-approvals.ts b/src/telegram/exec-approvals.ts new file mode 100644 index 00000000000..1055e1d1676 --- /dev/null +++ b/src/telegram/exec-approvals.ts @@ -0,0 +1,106 @@ +import type { ReplyPayload } from "../auto-reply/types.js"; +import type { OpenClawConfig } from "../config/config.js"; +import type { TelegramExecApprovalConfig } from "../config/types.telegram.js"; +import { getExecApprovalReplyMetadata } from "../infra/exec-approval-reply.js"; +import { resolveTelegramAccount } from "./accounts.js"; +import { resolveTelegramTargetChatType } from "./targets.js"; + +function normalizeApproverId(value: string | number): string { + return String(value).trim(); +} + +export function resolveTelegramExecApprovalConfig(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): TelegramExecApprovalConfig | undefined { + return resolveTelegramAccount(params).config.execApprovals; +} + +export function getTelegramExecApprovalApprovers(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): string[] { + return (resolveTelegramExecApprovalConfig(params)?.approvers ?? []) + .map(normalizeApproverId) + .filter(Boolean); +} + +export function isTelegramExecApprovalClientEnabled(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): boolean { + const config = resolveTelegramExecApprovalConfig(params); + return Boolean(config?.enabled && getTelegramExecApprovalApprovers(params).length > 0); +} + +export function isTelegramExecApprovalApprover(params: { + cfg: OpenClawConfig; + accountId?: string | null; + senderId?: string | null; +}): boolean { + const senderId = params.senderId?.trim(); + if (!senderId) { + return false; + } + const approvers = getTelegramExecApprovalApprovers(params); + return approvers.includes(senderId); +} + +export function resolveTelegramExecApprovalTarget(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): "dm" | "channel" | "both" { + return resolveTelegramExecApprovalConfig(params)?.target ?? "dm"; +} + +export function shouldInjectTelegramExecApprovalButtons(params: { + cfg: OpenClawConfig; + accountId?: string | null; + to: string; +}): boolean { + if (!isTelegramExecApprovalClientEnabled(params)) { + return false; + } + const target = resolveTelegramExecApprovalTarget(params); + const chatType = resolveTelegramTargetChatType(params.to); + if (chatType === "direct") { + return target === "dm" || target === "both"; + } + if (chatType === "group") { + return target === "channel" || target === "both"; + } + return target === "both"; +} + +function resolveExecApprovalButtonsExplicitlyDisabled(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): boolean { + const capabilities = resolveTelegramAccount(params).config.capabilities; + if (!capabilities || Array.isArray(capabilities) || typeof capabilities !== "object") { + return false; + } + const inlineButtons = (capabilities as { inlineButtons?: unknown }).inlineButtons; + return typeof inlineButtons === "string" && inlineButtons.trim().toLowerCase() === "off"; +} + +export function shouldEnableTelegramExecApprovalButtons(params: { + cfg: OpenClawConfig; + accountId?: string | null; + to: string; +}): boolean { + if (!shouldInjectTelegramExecApprovalButtons(params)) { + return false; + } + return !resolveExecApprovalButtonsExplicitlyDisabled(params); +} + +export function shouldSuppressLocalTelegramExecApprovalPrompt(params: { + cfg: OpenClawConfig; + accountId?: string | null; + payload: ReplyPayload; +}): boolean { + void params.cfg; + void params.accountId; + return getExecApprovalReplyMetadata(params.payload) !== null; +} diff --git a/src/telegram/monitor.ts b/src/telegram/monitor.ts index ed1e1a8744a..7131876e6f1 100644 --- a/src/telegram/monitor.ts +++ b/src/telegram/monitor.ts @@ -8,6 +8,7 @@ import { registerUnhandledRejectionHandler } from "../infra/unhandled-rejections import type { RuntimeEnv } from "../runtime.js"; import { resolveTelegramAccount } from "./accounts.js"; import { resolveTelegramAllowedUpdates } from "./allowed-updates.js"; +import { TelegramExecApprovalHandler } from "./exec-approvals-handler.js"; import { isRecoverableTelegramNetworkError } from "./network-errors.js"; import { TelegramPollingSession } from "./polling-session.js"; import { makeProxyFetch } from "./proxy.js"; @@ -73,6 +74,7 @@ const isGrammyHttpError = (err: unknown): boolean => { export async function monitorTelegramProvider(opts: MonitorTelegramOpts = {}) { const log = opts.runtime?.error ?? console.error; let pollingSession: TelegramPollingSession | undefined; + let execApprovalsHandler: TelegramExecApprovalHandler | undefined; const unregisterHandler = registerUnhandledRejectionHandler((err) => { const isNetworkError = isRecoverableTelegramNetworkError(err, { context: "polling" }); @@ -111,6 +113,14 @@ export async function monitorTelegramProvider(opts: MonitorTelegramOpts = {}) { const proxyFetch = opts.proxyFetch ?? (account.config.proxy ? makeProxyFetch(account.config.proxy) : undefined); + execApprovalsHandler = new TelegramExecApprovalHandler({ + token, + accountId: account.accountId, + cfg, + runtime: opts.runtime, + }); + await execApprovalsHandler.start(); + const persistedOffsetRaw = await readTelegramUpdateOffset({ accountId: account.accountId, botToken: token, @@ -178,6 +188,7 @@ export async function monitorTelegramProvider(opts: MonitorTelegramOpts = {}) { }); await pollingSession.runUntilAbort(); } finally { + await execApprovalsHandler?.stop().catch(() => {}); unregisterHandler(); } } diff --git a/src/telegram/send.test-harness.ts b/src/telegram/send.test-harness.ts index 57f47ac20d9..b8092034a95 100644 --- a/src/telegram/send.test-harness.ts +++ b/src/telegram/send.test-harness.ts @@ -5,6 +5,7 @@ const { botApi, botCtorSpy } = vi.hoisted(() => ({ botApi: { deleteMessage: vi.fn(), editMessageText: vi.fn(), + sendChatAction: vi.fn(), sendMessage: vi.fn(), sendPoll: vi.fn(), sendPhoto: vi.fn(), diff --git a/src/telegram/send.test.ts b/src/telegram/send.test.ts index 38097c49232..a34f27d196f 100644 --- a/src/telegram/send.test.ts +++ b/src/telegram/send.test.ts @@ -17,6 +17,7 @@ const { editMessageTelegram, reactMessageTelegram, sendMessageTelegram, + sendTypingTelegram, sendPollTelegram, sendStickerTelegram, } = await importTelegramSendModule(); @@ -171,6 +172,25 @@ describe("buildInlineKeyboard", () => { }); describe("sendMessageTelegram", () => { + it("sends typing to the resolved chat and topic", async () => { + loadConfig.mockReturnValue({ + channels: { + telegram: { + botToken: "tok", + }, + }, + }); + botApi.sendChatAction.mockResolvedValue(true); + + await sendTypingTelegram("telegram:group:-1001234567890:topic:271", { + accountId: "default", + }); + + expect(botApi.sendChatAction).toHaveBeenCalledWith("-1001234567890", "typing", { + message_thread_id: 271, + }); + }); + it("applies timeoutSeconds config precedence", async () => { const cases = [ { diff --git a/src/telegram/send.ts b/src/telegram/send.ts index 329329a07ff..e1b352a0a61 100644 --- a/src/telegram/send.ts +++ b/src/telegram/send.ts @@ -22,7 +22,7 @@ import { normalizePollInput, type PollInput } from "../polls.js"; import { loadWebMedia } from "../web/media.js"; import { type ResolvedTelegramAccount, resolveTelegramAccount } from "./accounts.js"; import { withTelegramApiErrorLogging } from "./api-logging.js"; -import { buildTelegramThreadParams } from "./bot/helpers.js"; +import { buildTelegramThreadParams, buildTypingThreadParams } from "./bot/helpers.js"; import type { TelegramInlineButtons } from "./button-types.js"; import { splitTelegramCaption } from "./caption.js"; import { resolveTelegramFetch } from "./fetch.js"; @@ -88,6 +88,16 @@ type TelegramReactionOpts = { retry?: RetryConfig; }; +type TelegramTypingOpts = { + cfg?: ReturnType; + token?: string; + accountId?: string; + verbose?: boolean; + api?: TelegramApiOverride; + retry?: RetryConfig; + messageThreadId?: number; +}; + function resolveTelegramMessageIdOrThrow( result: TelegramMessageLike | null | undefined, context: string, @@ -777,6 +787,39 @@ export async function sendMessageTelegram( return { messageId: String(messageId), chatId: String(res?.chat?.id ?? chatId) }; } +export async function sendTypingTelegram( + to: string, + opts: TelegramTypingOpts = {}, +): Promise<{ ok: true }> { + const { cfg, account, api } = resolveTelegramApiContext(opts); + const target = parseTelegramTarget(to); + const chatId = await resolveAndPersistChatId({ + cfg, + api, + lookupTarget: target.chatId, + persistTarget: to, + verbose: opts.verbose, + }); + const requestWithDiag = createTelegramRequestWithDiag({ + cfg, + account, + retry: opts.retry, + verbose: opts.verbose, + shouldRetry: (err) => isRecoverableTelegramNetworkError(err, { context: "send" }), + }); + const threadParams = buildTypingThreadParams(target.messageThreadId ?? opts.messageThreadId); + await requestWithDiag( + () => + api.sendChatAction( + chatId, + "typing", + threadParams as Parameters[2], + ), + "typing", + ); + return { ok: true }; +} + export async function reactMessageTelegram( chatIdInput: string | number, messageIdInput: string | number, @@ -873,6 +916,61 @@ type TelegramEditOpts = { cfg?: ReturnType; }; +type TelegramEditReplyMarkupOpts = { + token?: string; + accountId?: string; + verbose?: boolean; + api?: TelegramApiOverride; + retry?: RetryConfig; + /** Inline keyboard buttons (reply markup). Pass empty array to remove buttons. */ + buttons?: TelegramInlineButtons; + /** Optional config injection to avoid global loadConfig() (improves testability). */ + cfg?: ReturnType; +}; + +export async function editMessageReplyMarkupTelegram( + chatIdInput: string | number, + messageIdInput: string | number, + buttons: TelegramInlineButtons, + opts: TelegramEditReplyMarkupOpts = {}, +): Promise<{ ok: true; messageId: string; chatId: string }> { + const { cfg, account, api } = resolveTelegramApiContext({ + ...opts, + cfg: opts.cfg, + }); + const rawTarget = String(chatIdInput); + const chatId = await resolveAndPersistChatId({ + cfg, + api, + lookupTarget: rawTarget, + persistTarget: rawTarget, + verbose: opts.verbose, + }); + const messageId = normalizeMessageId(messageIdInput); + const requestWithDiag = createTelegramRequestWithDiag({ + cfg, + account, + retry: opts.retry, + verbose: opts.verbose, + }); + const replyMarkup = buildInlineKeyboard(buttons) ?? { inline_keyboard: [] }; + try { + await requestWithDiag( + () => api.editMessageReplyMarkup(chatId, messageId, { reply_markup: replyMarkup }), + "editMessageReplyMarkup", + { + shouldLog: (err) => !isTelegramMessageNotModifiedError(err), + }, + ); + } catch (err) { + if (!isTelegramMessageNotModifiedError(err)) { + throw err; + } + } + logVerbose(`[telegram] Edited reply markup for message ${messageId} in chat ${chatId}`); + return { ok: true, messageId: String(messageId), chatId }; +} + export async function editMessageTelegram( chatIdInput: string | number, messageIdInput: string | number,