refactor: align same-chat approval routing

docs/tools/exec-approvals.md

@@ -400,6 +400,9 @@ This shared text-command path uses the normal channel auth model for that conver
 originating chat can already send commands and receive replies, approval requests no longer need a
 separate channel-specific approval client just to stay pending.
 
+Discord and Telegram also support same-chat `/approve`, but those channels still use their
+resolved approver list for authorization even when the richer approval client is disabled.
+
 ### Rich approval clients
 
 Discord and Telegram can also act as richer exec approval clients with channel-specific config.
@@ -412,7 +415,9 @@ top of the shared same-chat `/approve` flow.
 
 Shared behavior:
 
-- only resolved approvers can approve or deny
+- Slack, Matrix, Microsoft Teams, and similar deliverable chats use the normal channel auth model
+  for same-chat `/approve`
+- for Discord and Telegram, 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
 - the originating chat can approve directly with `/approve` when that chat already supports commands and replies

extensions/discord/src/channel.ts

@@ -492,7 +492,10 @@ export const discordPlugin: ChannelPlugin<ResolvedDiscordAccount, DiscordProbe>
         },
       },
       execApprovals: {
-        getInitiatingSurfaceState: () => ({ kind: "enabled" }),
+        getInitiatingSurfaceState: ({ cfg, accountId }) =>
+          getDiscordExecApprovalApprovers({ cfg, accountId }).length > 0
+            ? { kind: "enabled" }
+            : { kind: "disabled" },
         shouldSuppressLocalPrompt: ({ cfg, accountId, payload }) =>
           shouldSuppressLocalDiscordExecApprovalPrompt({
             cfg,

extensions/telegram/src/channel.ts

@@ -52,6 +52,7 @@ import {
   shouldSuppressTelegramExecApprovalForwardingFallback,
 } from "./exec-approval-forwarding.js";
 import {
+  getTelegramExecApprovalApprovers,
   isTelegramExecApprovalClientEnabled,
   resolveTelegramExecApprovalTarget,
 } from "./exec-approvals.js";
@@ -457,7 +458,10 @@ export const telegramPlugin = createChatChannelPlugin({
       },
     },
     execApprovals: {
-      getInitiatingSurfaceState: () => ({ kind: "enabled" }),
+      getInitiatingSurfaceState: ({ cfg, accountId }) =>
+        getTelegramExecApprovalApprovers({ cfg, accountId }).length > 0
+          ? { kind: "enabled" }
+          : { kind: "disabled" },
       hasConfiguredDmRoute: ({ cfg }) => hasTelegramExecApprovalDmRoute(cfg),
       shouldSuppressForwardingFallback: (params) =>
         shouldSuppressTelegramExecApprovalForwardingFallback(params),

extensions/telegram/src/exec-approvals.test.ts

@@ -221,6 +221,11 @@ describe("telegram exec approvals", () => {
       expect(isTelegramExecApprovalAuthorizedSender({ cfg, senderId: "123" })).toBe(true);
     });
 
+    it("accepts explicit approvers even when the richer client is disabled", () => {
+      const cfg = buildConfig({ enabled: false, approvers: ["123"] });
+      expect(isTelegramExecApprovalAuthorizedSender({ cfg, senderId: "123" })).toBe(true);
+    });
+
     it("accepts active forwarded DM targets", () => {
       const cfg = {
         channels: { telegram: { botToken: "tok" } },

extensions/telegram/src/exec-approvals.ts

@@ -126,10 +126,7 @@ export function isTelegramExecApprovalAuthorizedSender(params: {
   accountId?: string | null;
   senderId?: string | null;
 }): boolean {
-  return (
-    (isTelegramExecApprovalClientEnabled(params) && isTelegramExecApprovalApprover(params)) ||
-    isTelegramExecApprovalTargetRecipient(params)
-  );
+  return isTelegramExecApprovalApprover(params) || isTelegramExecApprovalTargetRecipient(params);
 }
 
 export function resolveTelegramExecApprovalTarget(params: {

src/agents/bash-tools.exec.approval-id.test.ts

@@ -28,12 +28,14 @@ vi.mock("../infra/exec-obfuscation-detect.js", () => ({
 let callGatewayTool: typeof import("./tools/gateway.js").callGatewayTool;
 let createExecTool: typeof import("./bash-tools.exec.js").createExecTool;
 let detectCommandObfuscation: typeof import("../infra/exec-obfuscation-detect.js").detectCommandObfuscation;
+let getExecApprovalApproverDmNoticeText: typeof import("../infra/exec-approval-reply.js").getExecApprovalApproverDmNoticeText;
 
 async function loadExecApprovalModules() {
   vi.resetModules();
   ({ callGatewayTool } = await import("./tools/gateway.js"));
   ({ createExecTool } = await import("./bash-tools.exec.js"));
   ({ detectCommandObfuscation } = await import("../infra/exec-obfuscation-detect.js"));
+  ({ getExecApprovalApproverDmNoticeText } = await import("../infra/exec-approval-reply.js"));
 }
 
 function buildPreparedSystemRunPayload(rawInvokeParams: unknown) {
@@ -192,20 +194,6 @@ function mockPendingApprovalRegistration() {
   });
 }
 
-function expectApprovalUnavailableText(result: {
-  details: { status?: string };
-  content: Array<{ type?: string; text?: string }>;
-}) {
-  expect(result.details.status).toBe("approval-unavailable");
-  const text = result.content.find((part) => part.type === "text")?.text ?? "";
-  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:");
-  return text;
-}
-
 describe("exec approvals", () => {
   let previousHome: string | undefined;
   let previousUserProfile: string | undefined;
@@ -690,7 +678,7 @@ describe("exec approvals", () => {
     );
   });
 
-  it("returns an unavailable approval message instead of a local /approve prompt when discord exec approvals are disabled", async () => {
+  it("shows a local /approve prompt when discord exec approvals are disabled", async () => {
     await writeOpenClawConfig({
       channels: {
         discord: {
@@ -715,12 +703,13 @@ describe("exec approvals", () => {
       command: "npm view diver name version description",
     });
 
-    const text = expectApprovalUnavailableText(result);
-    expect(text).toContain("Discord does not support chat exec approvals.");
-    expect(text).toContain("Web UI or terminal UI");
+    expectPendingApprovalText(result, {
+      command: "npm view diver name version description",
+      host: "gateway",
+    });
   });
 
-  it("tells Telegram users that allowed approvers were DMed when Telegram approvals are disabled but Discord DM approvals are enabled", async () => {
+  it("keeps Telegram approvals in the initiating chat even when Discord DM approvals are also enabled", async () => {
     await writeOpenClawConfig(
       {
         channels: {
@@ -752,9 +741,12 @@ describe("exec approvals", () => {
       command: "npm view diver name version description",
     });
 
-    const text = expectApprovalUnavailableText(result);
-    expect(text).toContain("Telegram does not support chat exec approvals.");
-    expect(text).toContain("Web UI or terminal UI");
+    const details = expectPendingApprovalText(result, {
+      command: "npm view diver name version description",
+      host: "gateway",
+    });
+    expect(getResultText(result)).toContain(`/approve ${details.approvalSlug} allow-once`);
+    expect(getResultText(result)).not.toContain(getExecApprovalApproverDmNoticeText());
   });
 
   it("denies node obfuscated command when approval request times out", async () => {

src/auto-reply/reply/commands-approve.ts

@@ -1,10 +1,7 @@
 import { callGateway } from "../../gateway/call.js";
 import { ErrorCodes } from "../../gateway/protocol/index.js";
 import { logVerbose } from "../../globals.js";
-import {
-  isDiscordExecApprovalApprover,
-  isDiscordExecApprovalClientEnabled,
-} from "../../plugin-sdk/discord-surface.js";
+import { isDiscordExecApprovalApprover } from "../../plugin-sdk/discord-surface.js";
 import {
   isTelegramExecApprovalAuthorizedSender,
   isTelegramExecApprovalApprover,
@@ -130,8 +127,6 @@ export const handleApproveCommand: CommandHandler = async (params, allowTextComm
     return { shouldContinue: false, reply: { text: parsed.error } };
   }
   const isPluginId = parsed.id.startsWith("plugin:");
-  let discordExecApprovalDeniedReply: { shouldContinue: false; reply: { text: string } } | null =
-    null;
   let isTelegramExplicitApprover = false;
 
   if (params.command.channel === "telegram") {
@@ -158,19 +153,14 @@ export const handleApproveCommand: CommandHandler = async (params, allowTextComm
   }
 
   if (params.command.channel === "discord" && !isPluginId) {
-    const discordApproverContext = {
-      cfg: params.cfg,
-      accountId: params.ctx.AccountId,
-      senderId: params.command.senderId,
-    };
-    if (!isDiscordExecApprovalClientEnabled(discordApproverContext)) {
-      discordExecApprovalDeniedReply = {
-        shouldContinue: false,
-        reply: { text: "❌ Discord exec approvals are not enabled for this bot account." },
-      };
-    }
-    if (!discordExecApprovalDeniedReply && !isDiscordExecApprovalApprover(discordApproverContext)) {
-      discordExecApprovalDeniedReply = {
+    if (
+      !isDiscordExecApprovalApprover({
+        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 Discord." },
       };
@@ -227,25 +217,6 @@ export const handleApproveCommand: CommandHandler = async (params, allowTextComm
       };
     }
   } else {
-    if (discordExecApprovalDeniedReply) {
-      // Preserve the legacy unprefixed plugin fallback on Discord even when
-      // exec approvals are unavailable to this sender.
-      try {
-        await callApprovalMethod("plugin.approval.resolve");
-      } catch (pluginErr) {
-        if (isApprovalNotFoundError(pluginErr)) {
-          return discordExecApprovalDeniedReply;
-        }
-        return {
-          shouldContinue: false,
-          reply: { text: `❌ Failed to submit approval: ${String(pluginErr)}` },
-        };
-      }
-      return {
-        shouldContinue: false,
-        reply: { text: `✅ Approval ${parsed.decision} submitted for ${parsed.id}.` },
-      };
-    }
     try {
       await callApprovalMethod("exec.approval.resolve");
     } catch (err) {

src/auto-reply/reply/commands.test.ts

@@ -457,26 +457,29 @@ describe("/approve command", () => {
   it("requires configured Discord approvers for exec approvals", async () => {
     for (const testCase of [
       {
-        name: "discord approvals disabled",
+        name: "discord no approver policy",
         cfg: createDiscordApproveCfg(null),
         senderId: "123",
-        expectedText: "Discord exec approvals are not enabled",
-        setup: () =>
-          callGatewayMock.mockRejectedValue(
-            gatewayError("unknown or expired approval id", "APPROVAL_NOT_FOUND"),
-          ),
-        expectedGatewayCalls: 1,
+        expectedText: "not authorized to approve",
+        setup: undefined,
+        expectedGatewayCalls: 0,
       },
       {
         name: "discord non approver",
         cfg: createDiscordApproveCfg({ enabled: true, approvers: ["999"], target: "channel" }),
         senderId: "123",
         expectedText: "not authorized to approve",
-        setup: () =>
-          callGatewayMock.mockRejectedValue(
-            gatewayError("unknown or expired approval id", "APPROVAL_NOT_FOUND"),
-          ),
+        setup: undefined,
+        expectedGatewayCalls: 0,
+      },
+      {
+        name: "discord approver with rich client disabled",
+        cfg: createDiscordApproveCfg({ enabled: false, approvers: ["123"], target: "channel" }),
+        senderId: "123",
+        expectedText: "Approval allow-once submitted",
+        setup: () => callGatewayMock.mockResolvedValue({ ok: true }),
         expectedGatewayCalls: 1,
+        expectedMethod: "exec.approval.resolve",
       },
       {
         name: "discord approver",
@@ -485,10 +488,11 @@ describe("/approve command", () => {
         expectedText: "Approval allow-once submitted",
         setup: () => callGatewayMock.mockResolvedValue({ ok: true }),
         expectedGatewayCalls: 1,
+        expectedMethod: "exec.approval.resolve",
       },
     ] as const) {
       callGatewayMock.mockReset();
-      testCase.setup();
+      testCase.setup?.();
       const params = buildParams("/approve abc12345 allow-once", testCase.cfg, {
         Provider: "discord",
         Surface: "discord",
@@ -499,13 +503,10 @@ describe("/approve command", () => {
       expect(result.shouldContinue, testCase.name).toBe(false);
       expect(result.reply?.text, testCase.name).toContain(testCase.expectedText);
       expect(callGatewayMock, testCase.name).toHaveBeenCalledTimes(testCase.expectedGatewayCalls);
-      if (testCase.expectedGatewayCalls > 0) {
+      if ("expectedMethod" in testCase) {
         expect(callGatewayMock, testCase.name).toHaveBeenCalledWith(
           expect.objectContaining({
-            method:
-              testCase.name === "discord approver"
-                ? "exec.approval.resolve"
-                : "plugin.approval.resolve",
+            method: testCase.expectedMethod,
             params: { id: "abc12345", decision: "allow-once" },
           }),
         );
@@ -632,6 +633,19 @@ describe("/approve command", () => {
         expectedText: "not authorized to approve",
         expectGatewayCalls: 0,
       },
+      {
+        name: "telegram approver with rich client disabled",
+        cfg: createTelegramApproveCfg({ enabled: false, approvers: ["123"], target: "dm" }),
+        commandBody: "/approve abc12345 allow-once",
+        ctx: {
+          Provider: "telegram",
+          Surface: "telegram",
+          SenderId: "123",
+        },
+        setup: () => callGatewayMock.mockResolvedValue({ ok: true }),
+        expectedText: "Approval allow-once submitted",
+        expectGatewayCalls: 1,
+      },
       {
         name: "non approver",
         cfg: createTelegramApproveCfg({ enabled: true, approvers: ["999"], target: "dm" }),
@@ -654,6 +668,14 @@ describe("/approve command", () => {
       expect(result.shouldContinue, testCase.name).toBe(false);
       expect(result.reply?.text, testCase.name).toContain(testCase.expectedText);
       expect(callGatewayMock, testCase.name).toHaveBeenCalledTimes(testCase.expectGatewayCalls);
+      if (testCase.expectGatewayCalls > 0) {
+        expect(callGatewayMock, testCase.name).toHaveBeenCalledWith(
+          expect.objectContaining({
+            method: "exec.approval.resolve",
+            params: { id: "abc12345", decision: "allow-once" },
+          }),
+        );
+      }
     }
   });
 

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

@@ -191,6 +191,7 @@ export function createExecApprovalHandlers(
       const hasExecApprovalClients = context.hasExecApprovalClients?.(client?.connId) ?? false;
       const hasTurnSourceRoute = hasApprovalTurnSourceRoute({
         turnSourceChannel: record.request.turnSourceChannel,
+        turnSourceAccountId: record.request.turnSourceAccountId,
       });
       let forwarded = false;
       if (opts?.forwarder) {

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

@@ -124,6 +124,7 @@ export function createPluginApprovalHandlers(
       const hasApprovalClients = context.hasExecApprovalClients?.(client?.connId) ?? false;
       const hasTurnSourceRoute = hasApprovalTurnSourceRoute({
         turnSourceChannel: record.request.turnSourceChannel,
+        turnSourceAccountId: record.request.turnSourceAccountId,
       });
       if (!hasApprovalClients && !forwarded && !hasTurnSourceRoute) {
         manager.expire(record.id, "no-approval-route");

src/infra/approval-turn-source.test.ts

@@ -1,19 +1,52 @@
-import { describe, expect, it } from "vitest";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+const loadConfigMock = vi.hoisted(() => vi.fn());
+const resolveExecApprovalInitiatingSurfaceStateMock = vi.hoisted(() => vi.fn());
+
+vi.mock("../config/config.js", () => ({
+  loadConfig: () => loadConfigMock(),
+}));
+
+vi.mock("./exec-approval-surface.js", () => ({
+  resolveExecApprovalInitiatingSurfaceState: (...args: unknown[]) =>
+    resolveExecApprovalInitiatingSurfaceStateMock(...args),
+}));
+
 import { hasApprovalTurnSourceRoute } from "./approval-turn-source.js";
 
 describe("hasApprovalTurnSourceRoute", () => {
-  it("accepts operator UI turn sources", () => {
-    expect(hasApprovalTurnSourceRoute({ turnSourceChannel: "webchat" })).toBe(true);
-    expect(hasApprovalTurnSourceRoute({ turnSourceChannel: "tui" })).toBe(true);
+  beforeEach(() => {
+    loadConfigMock.mockReset();
+    resolveExecApprovalInitiatingSurfaceStateMock.mockReset();
+    loadConfigMock.mockReturnValue({ loaded: true });
   });
 
-  it("accepts deliverable chat channels", () => {
-    expect(hasApprovalTurnSourceRoute({ turnSourceChannel: "slack" })).toBe(true);
-    expect(hasApprovalTurnSourceRoute({ turnSourceChannel: "discord" })).toBe(true);
+  it("returns true when the initiating surface is enabled", () => {
+    resolveExecApprovalInitiatingSurfaceStateMock.mockReturnValue({ kind: "enabled" });
+
+    expect(
+      hasApprovalTurnSourceRoute({
+        turnSourceChannel: "slack",
+        turnSourceAccountId: "work",
+      }),
+    ).toBe(true);
+    expect(resolveExecApprovalInitiatingSurfaceStateMock).toHaveBeenCalledWith({
+      channel: "slack",
+      accountId: "work",
+      cfg: { loaded: true },
+    });
   });
 
-  it("rejects missing or unknown turn sources", () => {
-    expect(hasApprovalTurnSourceRoute({ turnSourceChannel: undefined })).toBe(false);
+  it("returns false when the initiating surface is disabled or unsupported", () => {
+    resolveExecApprovalInitiatingSurfaceStateMock.mockReturnValueOnce({ kind: "disabled" });
+    expect(hasApprovalTurnSourceRoute({ turnSourceChannel: "discord" })).toBe(false);
+
+    resolveExecApprovalInitiatingSurfaceStateMock.mockReturnValueOnce({ kind: "unsupported" });
     expect(hasApprovalTurnSourceRoute({ turnSourceChannel: "unknown-channel" })).toBe(false);
   });
+
+  it("returns false when there is no turn-source channel", () => {
+    expect(hasApprovalTurnSourceRoute({ turnSourceChannel: undefined })).toBe(false);
+    expect(resolveExecApprovalInitiatingSurfaceStateMock).not.toHaveBeenCalled();
+  });
 });

src/infra/approval-turn-source.ts

@@ -1,16 +1,18 @@
-import {
-  INTERNAL_MESSAGE_CHANNEL,
-  isDeliverableMessageChannel,
-  normalizeMessageChannel,
-} from "../utils/message-channel.js";
+import { loadConfig } from "../config/config.js";
+import { resolveExecApprovalInitiatingSurfaceState } from "./exec-approval-surface.js";
 
-export function hasApprovalTurnSourceRoute(params: { turnSourceChannel?: string | null }): boolean {
-  const channel = normalizeMessageChannel(params.turnSourceChannel);
-  if (!channel) {
+export function hasApprovalTurnSourceRoute(params: {
+  turnSourceChannel?: string | null;
+  turnSourceAccountId?: string | null;
+}): boolean {
+  if (!params.turnSourceChannel?.trim()) {
     return false;
   }
-  if (channel === INTERNAL_MESSAGE_CHANNEL || channel === "tui") {
-    return true;
-  }
-  return isDeliverableMessageChannel(channel);
+  return (
+    resolveExecApprovalInitiatingSurfaceState({
+      channel: params.turnSourceChannel,
+      accountId: params.turnSourceAccountId,
+      cfg: loadConfig(),
+    }).kind === "enabled"
+  );
 }