fix(exec-approvals): harden forwarding target and resolve delivery paths

Co-authored-by: bubmiller <bubmiller@users.noreply.github.com>

CHANGELOG.md

@@ -36,6 +36,7 @@ Docs: https://docs.openclaw.ai
 - Channels/Group policy: fail closed when `groupPolicy: "allowlist"` is set without explicit `groups`, honor account-level `groupPolicy` overrides, and enforce `groupPolicy: "disabled"` as a hard group block. (#22215) Thanks @etereo.
 - Sandbox/Media: map container workspace paths (`/workspace/...` and `file:///workspace/...`) back to the host sandbox root for outbound media validation, preventing false deny errors for sandbox-generated local media. (#23083) Thanks @echo931.
 - Sandbox/Docker: apply custom bind mounts after workspace mounts and prioritize bind-source resolution on overlapping paths, so explicit workspace binds are no longer ignored. (#22669) Thanks @tasaankaeris.
+- Exec approvals/Forwarding: restore Discord text forwarding when component approvals are not configured, and carry request snapshots through resolve events so resolved notices still forward after cache misses/restarts. (#22988) Thanks @bubmiller.
 - Config/Memory: allow `"mistral"` in `agents.defaults.memorySearch.provider` and `agents.defaults.memorySearch.fallback` schema validation. (#14934) Thanks @ThomsenDrake.
 - Security/Feishu: enforce ID-only allowlist matching for DM/group sender authorization, normalize Feishu ID prefixes during checks, and ignore mutable display names so display-name collisions cannot satisfy allowlist entries. This ships in the next npm release. Thanks @jiseoung for reporting.
 - Feishu/Commands: in group chats, command authorization now falls back to top-level `channels.feishu.allowFrom` when per-group `allowFrom` is not set, so `/command` no longer gets blocked by an unintended empty allowlist. (#23756)

src/gateway/server-methods/exec-approval.ts

@@ -186,6 +186,7 @@ export function createExecApprovalHandlers(
         respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "invalid decision"));
         return;
       }
+      const snapshot = manager.getSnapshot(p.id);
       const resolvedBy = client?.connect?.client?.displayName ?? client?.connect?.client?.id;
       const ok = manager.resolve(p.id, decision, resolvedBy ?? null);
       if (!ok) {
@@ -194,11 +195,17 @@ export function createExecApprovalHandlers(
       }
       context.broadcast(
         "exec.approval.resolved",
-        { id: p.id, decision, resolvedBy, ts: Date.now() },
+        { id: p.id, decision, resolvedBy, ts: Date.now(), request: snapshot?.request },
         { dropIfSlow: true },
       );
       void opts?.forwarder
-        ?.handleResolved({ id: p.id, decision, resolvedBy, ts: Date.now() })
+        ?.handleResolved({
+          id: p.id,
+          decision,
+          resolvedBy,
+          ts: Date.now(),
+          request: snapshot?.request,
+        })
         .catch((err) => {
           context.logGateway?.error?.(`exec approvals: forward resolve failed: ${String(err)}`);
         });

src/infra/exec-approval-forwarder.test.ts

@@ -113,7 +113,7 @@ describe("exec approval forwarder", () => {
     expect(getFirstDeliveryText(deliver)).toContain("Command:\n```\necho `uname`\necho done\n```");
   });
 
-  it("skips discord forwarding targets", async () => {
+  it("forwards to discord when discord exec approvals handler is disabled", async () => {
     vi.useFakeTimers();
     const cfg = {
       approvals: { exec: { enabled: true, mode: "session" } },
@@ -126,9 +126,61 @@ describe("exec approval forwarder", () => {
 
     await forwarder.handleRequested(baseRequest);
 
+    expect(deliver).toHaveBeenCalledTimes(1);
+  });
+
+  it("skips discord forwarding when discord exec approvals handler is enabled", async () => {
+    vi.useFakeTimers();
+    const cfg = {
+      channels: {
+        discord: {
+          execApprovals: {
+            enabled: true,
+            approvers: ["123"],
+          },
+        },
+      },
+      approvals: { exec: { enabled: true, mode: "session" } },
+    } as OpenClawConfig;
+
+    const { deliver, forwarder } = createForwarder({
+      cfg,
+      resolveSessionTarget: () => ({ channel: "discord", to: "channel:123" }),
+    });
+
+    await forwarder.handleRequested(baseRequest);
+
     expect(deliver).not.toHaveBeenCalled();
   });
 
+  it("can forward resolved notices without pending cache when request payload is present", async () => {
+    vi.useFakeTimers();
+    const cfg = {
+      approvals: {
+        exec: {
+          enabled: true,
+          mode: "targets",
+          targets: [{ channel: "telegram", to: "123" }],
+        },
+      },
+    } as OpenClawConfig;
+    const { deliver, forwarder } = createForwarder({ cfg });
+
+    await forwarder.handleResolved({
+      id: "req-missing",
+      decision: "allow-once",
+      resolvedBy: "telegram:123",
+      ts: 2000,
+      request: {
+        command: "echo ok",
+        agentId: "main",
+        sessionKey: "agent:main:main",
+      },
+    });
+
+    expect(deliver).toHaveBeenCalledTimes(1);
+  });
+
   it("uses a longer fence when command already contains triple backticks", async () => {
     vi.useFakeTimers();
     const { deliver, forwarder } = createForwarder({ cfg: TARGETS_CFG });

src/infra/exec-approval-forwarder.ts

@@ -6,7 +6,7 @@ import type {
   ExecApprovalForwardTarget,
 } from "../config/types.approvals.js";
 import { createSubsystemLogger } from "../logging/subsystem.js";
-import { parseAgentSessionKey } from "../routing/session-key.js";
+import { normalizeAccountId, parseAgentSessionKey } from "../routing/session-key.js";
 import { isDeliverableMessageChannel, normalizeMessageChannel } from "../utils/message-channel.js";
 import type {
   ExecApprovalDecision,
@@ -98,10 +98,49 @@ function buildTargetKey(target: ExecApprovalForwardTarget): string {
   return [channel, target.to, accountId, threadId].join(":");
 }
 
-// Discord has component-based exec approvals; skip the text fallback there.
-function shouldSkipDiscordForwarding(target: ExecApprovalForwardTarget): boolean {
+function resolveChannelAccountConfig<T>(
+  accounts: Record<string, T> | undefined,
+  accountId?: string,
+): T | undefined {
+  if (!accounts || !accountId?.trim()) {
+    return undefined;
+  }
+  const normalized = normalizeAccountId(accountId);
+  const direct = accounts[normalized];
+  if (direct) {
+    return direct;
+  }
+  const fallbackKey = Object.keys(accounts).find(
+    (key) => key.toLowerCase() === normalized.toLowerCase(),
+  );
+  return fallbackKey ? accounts[fallbackKey] : undefined;
+}
+
+// Discord has component-based exec approvals; skip text fallback only when the
+// Discord-specific handler is enabled for the same target account.
+function shouldSkipDiscordForwarding(
+  target: ExecApprovalForwardTarget,
+  cfg: OpenClawConfig,
+): boolean {
   const channel = normalizeMessageChannel(target.channel) ?? target.channel;
-  return channel === "discord";
+  if (channel !== "discord") {
+    return false;
+  }
+  const discord = cfg.channels?.discord as
+    | {
+        execApprovals?: { enabled?: boolean; approvers?: Array<string | number> };
+        accounts?: Record<
+          string,
+          { execApprovals?: { enabled?: boolean; approvers?: Array<string | number> } }
+        >;
+      }
+    | undefined;
+  if (!discord) {
+    return false;
+  }
+  const account = resolveChannelAccountConfig(discord.accounts, target.accountId);
+  const execApprovals = account?.execApprovals ?? discord.execApprovals;
+  return Boolean(execApprovals?.enabled && (execApprovals.approvers?.length ?? 0) > 0);
 }
 
 function formatApprovalCommand(command: string): { inline: boolean; text: string } {
@@ -228,6 +267,48 @@ async function deliverToTargets(params: {
   await Promise.allSettled(deliveries);
 }
 
+function resolveForwardTargets(params: {
+  cfg: OpenClawConfig;
+  config?: ExecApprovalForwardingConfig;
+  request: ExecApprovalRequest;
+  resolveSessionTarget: (params: {
+    cfg: OpenClawConfig;
+    request: ExecApprovalRequest;
+  }) => ExecApprovalForwardTarget | null;
+}): ForwardTarget[] {
+  const mode = normalizeMode(params.config?.mode);
+  const targets: ForwardTarget[] = [];
+  const seen = new Set<string>();
+
+  if (mode === "session" || mode === "both") {
+    const sessionTarget = params.resolveSessionTarget({
+      cfg: params.cfg,
+      request: params.request,
+    });
+    if (sessionTarget) {
+      const key = buildTargetKey(sessionTarget);
+      if (!seen.has(key)) {
+        seen.add(key);
+        targets.push({ ...sessionTarget, source: "session" });
+      }
+    }
+  }
+
+  if (mode === "targets" || mode === "both") {
+    const explicitTargets = params.config?.targets ?? [];
+    for (const target of explicitTargets) {
+      const key = buildTargetKey(target);
+      if (seen.has(key)) {
+        continue;
+      }
+      seen.add(key);
+      targets.push({ ...target, source: "target" });
+    }
+  }
+
+  return targets;
+}
+
 export function createExecApprovalForwarder(
   deps: ExecApprovalForwarderDeps = {},
 ): ExecApprovalForwarder {
@@ -243,35 +324,12 @@ export function createExecApprovalForwarder(
     if (!shouldForward({ config, request })) {
       return;
     }
-
-    const mode = normalizeMode(config?.mode);
-    const targets: ForwardTarget[] = [];
-    const seen = new Set<string>();
-
-    if (mode === "session" || mode === "both") {
-      const sessionTarget = resolveSessionTarget({ cfg, request });
-      if (sessionTarget) {
-        const key = buildTargetKey(sessionTarget);
-        if (!seen.has(key)) {
-          seen.add(key);
-          targets.push({ ...sessionTarget, source: "session" });
-        }
-      }
-    }
-
-    if (mode === "targets" || mode === "both") {
-      const explicitTargets = config?.targets ?? [];
-      for (const target of explicitTargets) {
-        const key = buildTargetKey(target);
-        if (seen.has(key)) {
-          continue;
-        }
-        seen.add(key);
-        targets.push({ ...target, source: "target" });
-      }
-    }
-
-    const filteredTargets = targets.filter((target) => !shouldSkipDiscordForwarding(target));
+    const filteredTargets = resolveForwardTargets({
+      cfg,
+      config,
+      request,
+      resolveSessionTarget,
+    }).filter((target) => !shouldSkipDiscordForwarding(target, cfg));
 
     if (filteredTargets.length === 0) {
       return;
@@ -310,17 +368,37 @@ export function createExecApprovalForwarder(
 
   const handleResolved = async (resolved: ExecApprovalResolved) => {
     const entry = pending.get(resolved.id);
-    if (!entry) {
+    if (entry) {
+      if (entry.timeoutId) {
+        clearTimeout(entry.timeoutId);
+      }
+      pending.delete(resolved.id);
+    }
+    const cfg = getConfig();
+    let targets = entry?.targets;
+
+    if (!targets && resolved.request) {
+      const request: ExecApprovalRequest = {
+        id: resolved.id,
+        request: resolved.request,
+        createdAtMs: resolved.ts,
+        expiresAtMs: resolved.ts,
+      };
+      const config = cfg.approvals?.exec;
+      if (shouldForward({ config, request })) {
+        targets = resolveForwardTargets({
+          cfg,
+          config,
+          request,
+          resolveSessionTarget,
+        }).filter((target) => !shouldSkipDiscordForwarding(target, cfg));
+      }
+    }
+    if (!targets || targets.length === 0) {
       return;
     }
-    if (entry.timeoutId) {
-      clearTimeout(entry.timeoutId);
-    }
-    pending.delete(resolved.id);
-
-    const cfg = getConfig();
     const text = buildResolvedMessage(resolved);
-    await deliverToTargets({ cfg, targets: entry.targets, text, deliver });
+    await deliverToTargets({ cfg, targets, text, deliver });
   };
 
   const stop = () => {

src/infra/exec-approvals.ts

@@ -32,6 +32,7 @@ export type ExecApprovalResolved = {
   decision: ExecApprovalDecision;
   resolvedBy?: string | null;
   ts: number;
+  request?: ExecApprovalRequest["request"];
 };
 
 export type ExecApprovalsDefaults = {