From 1ca01b738bb2f38303a058f16fd9811e8bed130d Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 30 Mar 2026 06:24:18 +0900 Subject: [PATCH] fix: stabilize exec approval approver routing --- CHANGELOG.md | 1 + docs/channels/discord.md | 8 ++- docs/channels/telegram.md | 12 ++-- docs/tools/exec-approvals.md | 4 +- extensions/discord/src/channel.ts | 6 +- extensions/discord/src/exec-approvals.test.ts | 70 +++++++++++++++++++ extensions/discord/src/exec-approvals.ts | 62 +++++++++++++++- .../src/monitor/exec-approvals.test.ts | 6 +- extensions/telegram/src/config-ui-hints.ts | 2 +- .../telegram/src/exec-approvals.test.ts | 24 ++++++- extensions/telegram/src/exec-approvals.ts | 41 ++++++++++- src/agents/bash-tools.exec-runtime.ts | 16 ++--- ...-embedded-subscribe.handlers.tools.test.ts | 2 +- src/config/types.discord.ts | 4 +- src/config/types.telegram.ts | 2 +- src/infra/exec-approval-forwarder.test.ts | 6 ++ src/infra/exec-approval-forwarder.ts | 4 +- src/infra/exec-approval-reply.test.ts | 17 ++++- src/infra/exec-approval-reply.ts | 36 +++++++--- src/infra/exec-approvals.ts | 2 +- 20 files changed, 278 insertions(+), 47 deletions(-) create mode 100644 extensions/discord/src/exec-approvals.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c358512b8a..b265461fa29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ Docs: https://docs.openclaw.ai - Control UI/slash commands: make `/steer` and `/redirect` work from the chat command palette with visible pending state for active-run `/steer`, correct redirected-run tracking, and a single canonical `/steer` entry in the command menu. (#54625) Thanks @fuller-stack-dev. - Exec/runtime: default implicit exec to `host=auto`, resolve that target to sandbox only when a sandbox runtime exists, keep explicit `host=sandbox` fail-closed without sandbox, and show `/exec` effective host state in runtime status/docs. - Exec: fail closed when the implicit sandbox host has no sandbox runtime, and stop denied async approval followups from reusing prior command output from the same session. (#56800) Thanks @scoootscooob. +- Exec/approvals: infer Discord and Telegram exec approvers from existing owner config when `execApprovals.approvers` is unset, extend the default approval window to 30 minutes, and clarify approval-unavailable guidance so approvals do not appear to silently disappear. - Exec/node: stop gateway-side workdir fallback from rewriting explicit `host=node` cwd values to the gateway filesystem, so remote node exec approval and runs keep using the intended node-local directory. (#50961) Thanks @openperf. - Plugins/ClawHub: sanitize temporary archive filenames for scoped package names and slash-containing skill slugs so `openclaw plugins install @scope/name` no longer fails with `ENOENT` during archive download. (#56452) Thanks @soimy. - Telegram/polling: keep the watchdog from aborting long-running reply delivery by treating recent non-polling API activity as bounded liveness instead of a hard stall. (#56343) Thanks @openperf. diff --git a/docs/channels/discord.md b/docs/channels/discord.md index e03fa3f38d6..d9ad0cbbcc6 100644 --- a/docs/channels/discord.md +++ b/docs/channels/discord.md @@ -948,11 +948,13 @@ Default slash command settings: Config path: - `channels.discord.execApprovals.enabled` - - `channels.discord.execApprovals.approvers` + - `channels.discord.execApprovals.approvers` (optional; falls back to owner IDs inferred from `allowFrom` and explicit DM `defaultTo` when possible) - `channels.discord.execApprovals.target` (`dm` | `channel` | `both`, default: `dm`) - `agentFilter`, `sessionFilter`, `cleanupAfterResolve` - When `target` is `channel` or `both`, the approval prompt is visible in the channel. Only configured approvers can use the buttons; other users receive an ephemeral denial. Approval prompts include the command text, so only enable channel delivery in trusted channels. If the channel ID cannot be derived from the session key, OpenClaw falls back to DM delivery. + Discord becomes an approval client when `enabled: true` and at least one approver can be resolved, either from `execApprovals.approvers` or from the account's existing owner config (`allowFrom`, legacy `dm.allowFrom`, or explicit DM `defaultTo`). + + When `target` is `channel` or `both`, the approval prompt is visible in the channel. Only resolved approvers can use the buttons; other users receive an ephemeral denial. Approval prompts include the command text, so only enable channel delivery in trusted channels. If the channel ID cannot be derived from the session key, OpenClaw falls back to DM delivery. Gateway auth for this handler uses the same shared credential resolution contract as other Gateway clients: @@ -961,7 +963,7 @@ Default slash command settings: - remote-mode support via `gateway.remote.*` when applicable - URL overrides are override-safe: CLI overrides do not reuse implicit credentials, and env overrides use env credentials only - If approvals fail with unknown approval IDs, verify approver list and feature enablement. + Exec approvals expire after 30 minutes by default. If approvals fail with unknown approval IDs, verify approver resolution and feature enablement. Related docs: [Exec approvals](/tools/exec-approvals) diff --git a/docs/channels/telegram.md b/docs/channels/telegram.md index 91ac3ec4b67..c984aeb4563 100644 --- a/docs/channels/telegram.md +++ b/docs/channels/telegram.md @@ -806,21 +806,21 @@ openclaw message poll --channel telegram --target -1001234567890:topic:42 \ Config path: - `channels.telegram.execApprovals.enabled` - - `channels.telegram.execApprovals.approvers` + - `channels.telegram.execApprovals.approvers` (optional; falls back to numeric owner IDs inferred from `allowFrom` and direct `defaultTo` when possible) - `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. + Approvers must be numeric Telegram user IDs. Telegram becomes an exec approval client when `enabled` is true and at least one approver can be resolved, either from `execApprovals.approvers` or from the account's numeric owner config (`allowFrom` and direct-message `defaultTo`). Approval requests otherwise 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: "dm"` sends approval prompts only to resolved 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. + Only resolved 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. + 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. Exec approvals expire after 30 minutes by default. Inline approval buttons also depend on `channels.telegram.capabilities.inlineButtons` allowing the target surface (`dm`, `group`, or `all`). @@ -932,7 +932,7 @@ Primary reference: - 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.approvers`: Telegram user IDs allowed to approve or deny exec requests. Optional when `channels.telegram.allowFrom` or a direct `channels.telegram.defaultTo` already identifies the owner. - `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. diff --git a/docs/tools/exec-approvals.md b/docs/tools/exec-approvals.md index 3914c1c0055..c0758d7b315 100644 --- a/docs/tools/exec-approvals.md +++ b/docs/tools/exec-approvals.md @@ -402,9 +402,11 @@ that channel as an approval surface just because the conversation happened there Shared behavior: -- only configured approvers can approve or deny +- only resolved approvers can approve or deny +- Discord and Telegram approvers can be explicit (`execApprovals.approvers`) or inferred from existing owner config (`allowFrom`, plus direct-message `defaultTo` where supported) - the requester does not need to be an approver - when channel delivery is enabled, approval prompts include the command text +- pending exec approvals expire after 30 minutes by default - 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 diff --git a/extensions/discord/src/channel.ts b/extensions/discord/src/channel.ts index b936f0b8066..0fa8e6b804b 100644 --- a/extensions/discord/src/channel.ts +++ b/extensions/discord/src/channel.ts @@ -40,6 +40,7 @@ import { listDiscordDirectoryPeersFromConfig, } from "./directory-config.js"; import { + getDiscordExecApprovalApprovers, isDiscordExecApprovalClientEnabled, shouldSuppressLocalDiscordExecApprovalPrompt, } from "./exec-approvals.js"; @@ -300,7 +301,10 @@ function buildDiscordCrossContextComponents(params: { function hasDiscordExecApprovalDmRoute(cfg: OpenClawConfig): boolean { return listDiscordAccountIds(cfg).some((accountId) => { const execApprovals = resolveDiscordAccount({ cfg, accountId }).config.execApprovals; - if (!execApprovals?.enabled || (execApprovals.approvers?.length ?? 0) === 0) { + if ( + !execApprovals?.enabled || + getDiscordExecApprovalApprovers({ cfg, accountId }).length === 0 + ) { return false; } const target = execApprovals.target ?? "dm"; diff --git a/extensions/discord/src/exec-approvals.test.ts b/extensions/discord/src/exec-approvals.test.ts new file mode 100644 index 00000000000..dd3a86ce59d --- /dev/null +++ b/extensions/discord/src/exec-approvals.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, it } from "vitest"; +import type { OpenClawConfig } from "../../../src/config/config.js"; +import { + getDiscordExecApprovalApprovers, + isDiscordExecApprovalApprover, + isDiscordExecApprovalClientEnabled, +} from "./exec-approvals.js"; + +function buildConfig( + execApprovals?: NonNullable["discord"]>["execApprovals"], + channelOverrides?: Partial["discord"]>>, +): OpenClawConfig { + return { + channels: { + discord: { + token: "discord-token", + ...channelOverrides, + execApprovals, + }, + }, + } as OpenClawConfig; +} + +describe("discord exec approvals", () => { + it("requires enablement and an explicit or inferred approver", () => { + expect(isDiscordExecApprovalClientEnabled({ cfg: buildConfig() })).toBe(false); + expect(isDiscordExecApprovalClientEnabled({ cfg: buildConfig({ enabled: true }) })).toBe(false); + expect( + isDiscordExecApprovalClientEnabled({ + cfg: buildConfig({ enabled: true }, { allowFrom: ["123"] }), + }), + ).toBe(true); + }); + + it("prefers explicit approvers when configured", () => { + const cfg = buildConfig( + { enabled: true, approvers: ["456"] }, + { allowFrom: ["123"], defaultTo: "user:789" }, + ); + + expect(getDiscordExecApprovalApprovers({ cfg })).toEqual(["456"]); + expect(isDiscordExecApprovalApprover({ cfg, senderId: "456" })).toBe(true); + expect(isDiscordExecApprovalApprover({ cfg, senderId: "123" })).toBe(false); + }); + + it("infers approvers from allowFrom, legacy dm.allowFrom, and explicit DM defaultTo", () => { + const cfg = buildConfig( + { enabled: true }, + { + allowFrom: ["123"], + dm: { allowFrom: ["456"] }, + defaultTo: "user:789", + }, + ); + + expect(getDiscordExecApprovalApprovers({ cfg })).toEqual(["123", "456", "789"]); + expect(isDiscordExecApprovalApprover({ cfg, senderId: "789" })).toBe(true); + }); + + it("ignores non-user default targets when inferring approvers", () => { + const cfg = buildConfig( + { enabled: true }, + { + defaultTo: "channel:123", + }, + ); + + expect(getDiscordExecApprovalApprovers({ cfg })).toEqual([]); + }); +}); diff --git a/extensions/discord/src/exec-approvals.ts b/extensions/discord/src/exec-approvals.ts index 532bdabe20d..fedf520be67 100644 --- a/extensions/discord/src/exec-approvals.ts +++ b/extensions/discord/src/exec-approvals.ts @@ -2,13 +2,70 @@ import { getExecApprovalReplyMetadata } from "openclaw/plugin-sdk/approval-runti import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime"; import type { ReplyPayload } from "openclaw/plugin-sdk/reply-runtime"; import { resolveDiscordAccount } from "./accounts.js"; +import { parseDiscordTarget } from "./targets.js"; + +function normalizeDiscordApproverId(value: string): string | undefined { + const trimmed = value.trim(); + if (!trimmed) { + return undefined; + } + if (/^\d+$/.test(trimmed)) { + return trimmed; + } + try { + const target = parseDiscordTarget(trimmed); + return target?.kind === "user" ? target.id : undefined; + } catch { + return undefined; + } +} + +function collectDiscordInferredApprovers(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): string[] { + const account = resolveDiscordAccount(params).config; + const inferred = new Set(); + for (const entry of [...(account.allowFrom ?? []), ...(account.dm?.allowFrom ?? [])]) { + const approverId = normalizeDiscordApproverId(String(entry)); + if (approverId) { + inferred.add(approverId); + } + } + const defaultTo = account.defaultTo?.trim(); + if (defaultTo) { + try { + const target = parseDiscordTarget(defaultTo); + if (target?.kind === "user") { + inferred.add(target.id); + } + } catch { + // Ignore ambiguous default targets; explicit approvers or allowFrom still work. + } + } + return [...inferred]; +} + +export function getDiscordExecApprovalApprovers(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): string[] { + const config = resolveDiscordAccount(params).config.execApprovals; + const explicit = (config?.approvers ?? []) + .map((entry) => normalizeDiscordApproverId(String(entry))) + .filter((entry): entry is string => Boolean(entry)); + if (explicit.length > 0) { + return [...new Set(explicit)]; + } + return collectDiscordInferredApprovers(params); +} 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); + return Boolean(config?.enabled && getDiscordExecApprovalApprovers(params).length > 0); } export function isDiscordExecApprovalApprover(params: { @@ -20,8 +77,7 @@ export function isDiscordExecApprovalApprover(params: { if (!senderId) { return false; } - const approvers = resolveDiscordAccount(params).config.execApprovals?.approvers ?? []; - return approvers.some((approverId) => String(approverId) === senderId); + return getDiscordExecApprovalApprovers(params).includes(senderId); } export function shouldSuppressLocalDiscordExecApprovalPrompt(params: { diff --git a/extensions/discord/src/monitor/exec-approvals.test.ts b/extensions/discord/src/monitor/exec-approvals.test.ts index 5c8cb233139..02928381cb6 100644 --- a/extensions/discord/src/monitor/exec-approvals.test.ts +++ b/extensions/discord/src/monitor/exec-approvals.test.ts @@ -757,7 +757,7 @@ describe("DiscordExecApprovalHandler plugin approvals", () => { const handler = createHandler({ enabled: true, approvers: ["123"] }); mockSuccessfulDmDelivery({ noteChannelId: "999888777", - expectedNoteText: "I sent the allowed approvers DMs", + expectedNoteText: "I sent approval DMs to the approvers for this account", throwOnUnexpectedRoute: true, }); @@ -1237,7 +1237,7 @@ describe("DiscordExecApprovalHandler delivery routing", () => { mockSuccessfulDmDelivery({ noteChannelId: "999888777", - expectedNoteText: "I sent the allowed approvers DMs", + expectedNoteText: "I sent approval DMs to the approvers for this account", throwOnUnexpectedRoute: true, }); @@ -1247,7 +1247,7 @@ describe("DiscordExecApprovalHandler delivery routing", () => { Routes.channelMessages("999888777"), expect.objectContaining({ body: expect.objectContaining({ - content: expect.stringContaining("I sent the allowed approvers DMs"), + content: expect.stringContaining("I sent approval DMs to the approvers for this account"), }), }), ); diff --git a/extensions/telegram/src/config-ui-hints.ts b/extensions/telegram/src/config-ui-hints.ts index a990fe62f1b..7d7be2990b9 100644 --- a/extensions/telegram/src/config-ui-hints.ts +++ b/extensions/telegram/src/config-ui-hints.ts @@ -91,7 +91,7 @@ export const telegramChannelConfigUiHints = { }, "execApprovals.approvers": { label: "Telegram Exec Approval Approvers", - help: "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.", + help: "Telegram user IDs allowed to approve exec requests for this bot account. Use numeric Telegram user IDs. If you leave this unset, OpenClaw falls back to numeric owner IDs inferred from channels.telegram.allowFrom and direct-message defaultTo when possible.", }, "execApprovals.agentFilter": { label: "Telegram Exec Approval Agent Filter", diff --git a/extensions/telegram/src/exec-approvals.test.ts b/extensions/telegram/src/exec-approvals.test.ts index 6bb38b8f8f4..f86f9ae6bdb 100644 --- a/extensions/telegram/src/exec-approvals.test.ts +++ b/extensions/telegram/src/exec-approvals.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import type { OpenClawConfig } from "../../../src/config/config.js"; import { + getTelegramExecApprovalApprovers, isTelegramExecApprovalAuthorizedSender, isTelegramExecApprovalApprover, isTelegramExecApprovalClientEnabled, @@ -12,11 +13,13 @@ import { function buildConfig( execApprovals?: NonNullable["telegram"]>["execApprovals"], + channelOverrides?: Partial["telegram"]>>, ): OpenClawConfig { return { channels: { telegram: { botToken: "tok", + ...channelOverrides, execApprovals, }, }, @@ -24,13 +27,18 @@ function buildConfig( } describe("telegram exec approvals", () => { - it("requires enablement and at least one approver", () => { + it("requires enablement and an explicit or inferred approver", () => { expect(isTelegramExecApprovalClientEnabled({ cfg: buildConfig() })).toBe(false); expect( isTelegramExecApprovalClientEnabled({ cfg: buildConfig({ enabled: true }), }), ).toBe(false); + expect( + isTelegramExecApprovalClientEnabled({ + cfg: buildConfig({ enabled: true }, { allowFrom: ["123"] }), + }), + ).toBe(true); expect( isTelegramExecApprovalClientEnabled({ cfg: buildConfig({ enabled: true, approvers: ["123"] }), @@ -45,6 +53,20 @@ describe("telegram exec approvals", () => { expect(isTelegramExecApprovalApprover({ cfg, senderId: "789" })).toBe(false); }); + it("infers approvers from allowFrom and direct defaultTo", () => { + const cfg = buildConfig( + { enabled: true }, + { + allowFrom: ["12345", "-100999", "@ignored"], + defaultTo: 67890, + }, + ); + + expect(getTelegramExecApprovalApprovers({ cfg })).toEqual(["12345", "67890"]); + expect(isTelegramExecApprovalApprover({ cfg, senderId: "12345" })).toBe(true); + expect(isTelegramExecApprovalApprover({ cfg, senderId: "67890" })).toBe(true); + }); + it("defaults target to dm", () => { expect( resolveTelegramExecApprovalTarget({ cfg: buildConfig({ enabled: true, approvers: ["1"] }) }), diff --git a/extensions/telegram/src/exec-approvals.ts b/extensions/telegram/src/exec-approvals.ts index a9daac8a6df..5efefb9f8d1 100644 --- a/extensions/telegram/src/exec-approvals.ts +++ b/extensions/telegram/src/exec-approvals.ts @@ -11,6 +11,37 @@ function normalizeApproverId(value: string | number): string { return String(value).trim(); } +function normalizeTelegramDirectApproverId(value: string | number): string | undefined { + const normalized = normalizeApproverId(value); + const chatId = normalizeTelegramChatId(normalized); + if (!chatId || chatId.startsWith("-")) { + return undefined; + } + return chatId; +} + +function collectTelegramInferredApprovers(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): string[] { + const account = resolveTelegramAccount(params).config; + const inferred = new Set(); + for (const entry of account.allowFrom ?? []) { + const approverId = normalizeTelegramDirectApproverId(entry); + if (approverId) { + inferred.add(approverId); + } + } + const defaultTo = account.defaultTo; + if (defaultTo !== undefined && defaultTo !== null) { + const approverId = normalizeTelegramDirectApproverId(defaultTo); + if (approverId) { + inferred.add(approverId); + } + } + return [...inferred]; +} + export function resolveTelegramExecApprovalConfig(params: { cfg: OpenClawConfig; accountId?: string | null; @@ -22,9 +53,13 @@ export function getTelegramExecApprovalApprovers(params: { cfg: OpenClawConfig; accountId?: string | null; }): string[] { - return (resolveTelegramExecApprovalConfig(params)?.approvers ?? []) - .map(normalizeApproverId) - .filter(Boolean); + const explicit = (resolveTelegramExecApprovalConfig(params)?.approvers ?? []) + .map(normalizeTelegramDirectApproverId) + .filter((entry): entry is string => Boolean(entry)); + if (explicit.length > 0) { + return [...new Set(explicit)]; + } + return collectTelegramInferredApprovers(params); } export function isTelegramExecApprovalClientEnabled(params: { diff --git a/src/agents/bash-tools.exec-runtime.ts b/src/agents/bash-tools.exec-runtime.ts index 4f1d0469afd..08ae6277e84 100644 --- a/src/agents/bash-tools.exec-runtime.ts +++ b/src/agents/bash-tools.exec-runtime.ts @@ -1,7 +1,11 @@ import path from "node:path"; import type { AgentToolResult } from "@mariozechner/pi-agent-core"; import { Type } from "@sinclair/typebox"; -import { type ExecHost, type ExecTarget } from "../infra/exec-approvals.js"; +import { + DEFAULT_EXEC_APPROVAL_TIMEOUT_MS, + type ExecHost, + type ExecTarget, +} from "../infra/exec-approvals.js"; import { requestHeartbeatNow } from "../infra/heartbeat-wake.js"; import { isDangerousHostEnvVarName } from "../infra/host-env-security.js"; import { findPathKey, mergePathPrepend } from "../infra/path-prepend.js"; @@ -111,8 +115,8 @@ export const DEFAULT_PATH = process.env.PATH ?? "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"; export const DEFAULT_NOTIFY_TAIL_CHARS = 400; const DEFAULT_NOTIFY_SNIPPET_CHARS = 180; -export const DEFAULT_APPROVAL_TIMEOUT_MS = 120_000; -export const DEFAULT_APPROVAL_REQUEST_TIMEOUT_MS = 130_000; +export const DEFAULT_APPROVAL_TIMEOUT_MS = DEFAULT_EXEC_APPROVAL_TIMEOUT_MS; +export const DEFAULT_APPROVAL_REQUEST_TIMEOUT_MS = DEFAULT_APPROVAL_TIMEOUT_MS + 10_000; const DEFAULT_APPROVAL_RUNNING_NOTICE_MS = 10_000; const APPROVAL_SLUG_LENGTH = 8; @@ -254,11 +258,7 @@ export function resolveExecTarget(params: { } const selectedTarget = requestedTarget ?? configuredTarget; const effectiveHost = - selectedTarget === "auto" - ? params.sandboxAvailable - ? "sandbox" - : "gateway" - : selectedTarget; + selectedTarget === "auto" ? (params.sandboxAvailable ? "sandbox" : "gateway") : selectedTarget; return { configuredTarget, requestedTarget, diff --git a/src/agents/pi-embedded-subscribe.handlers.tools.test.ts b/src/agents/pi-embedded-subscribe.handlers.tools.test.ts index c41dba3b9fb..86eb5fab649 100644 --- a/src/agents/pi-embedded-subscribe.handlers.tools.test.ts +++ b/src/agents/pi-embedded-subscribe.handlers.tools.test.ts @@ -356,7 +356,7 @@ describe("handleToolExecutionEnd exec approval prompts", () => { expect(onToolResult).toHaveBeenCalledWith( expect.objectContaining({ - text: "Approval required. I sent the allowed approvers DMs.", + text: "Approval required. I sent approval DMs to the approvers for this account.", }), ); expect(ctx.state.deterministicApprovalPromptSent).toBe(true); diff --git a/src/config/types.discord.ts b/src/config/types.discord.ts index d8ffa046662..2640c7bdfb1 100644 --- a/src/config/types.discord.ts +++ b/src/config/types.discord.ts @@ -142,7 +142,7 @@ export type DiscordVoiceConfig = { export type DiscordExecApprovalConfig = { /** Enable exec approval forwarding to Discord DMs. Default: false. */ enabled?: boolean; - /** Discord user IDs to receive approval prompts. Required if enabled. */ + /** Discord user IDs to receive approval prompts. Optional: falls back to owner IDs inferred from allowFrom/defaultTo when possible. */ approvers?: string[]; /** Only forward approvals for these agent IDs. Omit = all agents. */ agentFilter?: string[]; @@ -152,7 +152,7 @@ export type DiscordExecApprovalConfig = { cleanupAfterResolve?: boolean; /** Where to send approval prompts. "dm" sends to approver DMs (default), "channel" sends to the * originating Discord channel, "both" sends to both. When target is "channel" or "both", buttons - * are only usable by configured approvers; other users receive an ephemeral denial. */ + * are only usable by resolved approvers; other users receive an ephemeral denial. */ target?: "dm" | "channel" | "both"; }; diff --git a/src/config/types.telegram.ts b/src/config/types.telegram.ts index 2fb5bc91435..7c67d1e07b5 100644 --- a/src/config/types.telegram.ts +++ b/src/config/types.telegram.ts @@ -61,7 +61,7 @@ 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. */ + /** Telegram user IDs allowed to approve exec requests. Optional: falls back to numeric owner IDs inferred from allowFrom/defaultTo when possible. */ approvers?: Array; /** Only forward approvals for these agent IDs. Omit = all agents. */ agentFilter?: string[]; diff --git a/src/infra/exec-approval-forwarder.test.ts b/src/infra/exec-approval-forwarder.test.ts index 82feeec700c..776c81799c8 100644 --- a/src/infra/exec-approval-forwarder.test.ts +++ b/src/infra/exec-approval-forwarder.test.ts @@ -20,7 +20,12 @@ const baseRequest = { expiresAtMs: 6000, }; +const activeForwarders: Array> = []; + afterEach(() => { + for (const forwarder of activeForwarders.splice(0)) { + forwarder.stop(); + } vi.useRealTimers(); vi.restoreAllMocks(); }); @@ -115,6 +120,7 @@ function createForwarder(params: { deps.resolveSessionTarget = params.resolveSessionTarget; } const forwarder = createExecApprovalForwarder(deps); + activeForwarders.push(forwarder); return { deliver, forwarder }; } diff --git a/src/infra/exec-approval-forwarder.ts b/src/infra/exec-approval-forwarder.ts index f53d817f749..36b357bfdd8 100644 --- a/src/infra/exec-approval-forwarder.ts +++ b/src/infra/exec-approval-forwarder.ts @@ -16,6 +16,7 @@ import { type DeliverableMessageChannel, } from "../utils/message-channel.js"; import { resolveExecApprovalCommandDisplay } from "./exec-approval-command-display.js"; +import { formatExecApprovalExpiresIn } from "./exec-approval-reply.js"; import { resolveExecApprovalSessionTarget } from "./exec-approval-session-target.js"; import type { ExecApprovalRequest, ExecApprovalResolved } from "./exec-approvals.js"; import { deliverOutboundPayloads } from "./outbound/deliver.js"; @@ -178,8 +179,7 @@ function buildRequestMessage(request: ExecApprovalRequest, nowMs: number) { if (request.request.ask) { lines.push(`Ask: ${request.request.ask}`); } - const expiresIn = Math.max(0, Math.round((request.expiresAtMs - nowMs) / 1000)); - lines.push(`Expires in: ${expiresIn}s`); + lines.push(`Expires in: ${formatExecApprovalExpiresIn(request.expiresAtMs, nowMs)}`); 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).", diff --git a/src/infra/exec-approval-reply.test.ts b/src/infra/exec-approval-reply.test.ts index 459ae5116a6..39a3e82e596 100644 --- a/src/infra/exec-approval-reply.test.ts +++ b/src/infra/exec-approval-reply.test.ts @@ -46,7 +46,7 @@ describe("exec approval reply helpers", () => { it("returns the approver DM notice text", () => { expect(getExecApprovalApproverDmNoticeText()).toBe( - "Approval required. I sent the allowed approvers DMs.", + "Approval required. I sent approval DMs to the approvers for this account.", ); }); @@ -129,6 +129,19 @@ describe("exec approval reply helpers", () => { expect(payload.text).toContain("Expires in: 0s"); }); + it("formats longer approval windows in minutes", () => { + const payload = buildExecApprovalPendingReplyPayload({ + approvalId: "req-30m", + approvalSlug: "slug-30m", + command: "echo later", + host: "gateway", + expiresAtMs: 1_801_000, + nowMs: 1_000, + }); + + expect(payload.text).toContain("Expires in: 30m"); + }); + it("builds unavailable payloads for approver DMs", () => { expect( buildExecApprovalUnavailableReplyPayload({ @@ -137,7 +150,7 @@ describe("exec approval reply helpers", () => { sentApproverDms: true, }), ).toEqual({ - text: "Careful.\n\nApproval required. I sent the allowed approvers DMs.", + text: "Careful.\n\nApproval required. I sent approval DMs to the approvers for this account.", }); }); diff --git a/src/infra/exec-approval-reply.ts b/src/infra/exec-approval-reply.ts index 3c3eaffd896..5c3c2667a00 100644 --- a/src/infra/exec-approval-reply.ts +++ b/src/infra/exec-approval-reply.ts @@ -34,7 +34,29 @@ export type ExecApprovalUnavailableReplyParams = { }; export function getExecApprovalApproverDmNoticeText(): string { - return "Approval required. I sent the allowed approvers DMs."; + return "Approval required. I sent approval DMs to the approvers for this account."; +} + +export function formatExecApprovalExpiresIn(expiresAtMs: number, nowMs: number): string { + const totalSeconds = Math.max(0, Math.round((expiresAtMs - nowMs) / 1000)); + if (totalSeconds < 60) { + return `${totalSeconds}s`; + } + + const hours = Math.floor(totalSeconds / 3600); + const minutes = Math.floor((totalSeconds % 3600) / 60); + const seconds = totalSeconds % 60; + const parts: string[] = []; + if (hours > 0) { + parts.push(`${hours}h`); + } + if (minutes > 0) { + parts.push(`${minutes}m`); + } + if (hours === 0 && minutes < 5 && seconds > 0) { + parts.push(`${seconds}s`); + } + return parts.join(" "); } function buildFence(text: string, language?: string): string { @@ -106,11 +128,9 @@ export function buildExecApprovalPendingReplyPayload( 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: ${formatExecApprovalExpiresIn(params.expiresAtMs, params.nowMs ?? Date.now())}`, ); - info.push(`Expires in: ${expiresInSec}s`); } info.push(`Full id: \`${params.approvalId}\``); lines.push(info.join("\n")); @@ -148,21 +168,21 @@ export function buildExecApprovalUnavailableReplyPayload( `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.", + "Approve it from the Web UI or terminal UI, or enable Discord or Telegram exec approvals. If those accounts already know your owner ID via allowFrom, OpenClaw can infer approvers automatically.", ); } 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.", + "Approve it from the Web UI or terminal UI, or enable Discord or Telegram exec approvals. If those accounts already know your owner ID via allowFrom, OpenClaw can infer approvers automatically.", ); } 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.", + "Open the Web UI or terminal UI, or enable Discord or Telegram exec approvals, then retry the command. If those accounts already know your owner ID via allowFrom, you can usually leave execApprovals.approvers unset.", ); } diff --git a/src/infra/exec-approvals.ts b/src/infra/exec-approvals.ts index 1a78b732faa..d7d94d007e9 100644 --- a/src/infra/exec-approvals.ts +++ b/src/infra/exec-approvals.ts @@ -153,7 +153,7 @@ export type ExecApprovalsResolved = { }; // Keep CLI + gateway defaults in sync. -export const DEFAULT_EXEC_APPROVAL_TIMEOUT_MS = 120_000; +export const DEFAULT_EXEC_APPROVAL_TIMEOUT_MS = 1_800_000; const DEFAULT_SECURITY: ExecSecurity = "deny"; const DEFAULT_ASK: ExecAsk = "on-miss";