fix: native command arg menus follow-ups (#936) (thanks @thewilloftheshadow)

src/slack/monitor/slash.command-arg-menus.test.ts

@@ -0,0 +1,200 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+import { registerSlackMonitorSlashCommands } from "./slash.js";
+
+const dispatchMock = vi.fn();
+const readAllowFromStoreMock = vi.fn();
+const upsertPairingRequestMock = vi.fn();
+const resolveAgentRouteMock = vi.fn();
+
+vi.mock("../../auto-reply/reply/provider-dispatcher.js", () => ({
+  dispatchReplyWithDispatcher: (...args: unknown[]) => dispatchMock(...args),
+}));
+
+vi.mock("../../pairing/pairing-store.js", () => ({
+  readChannelAllowFromStore: (...args: unknown[]) => readAllowFromStoreMock(...args),
+  upsertChannelPairingRequest: (...args: unknown[]) => upsertPairingRequestMock(...args),
+}));
+
+vi.mock("../../routing/resolve-route.js", () => ({
+  resolveAgentRoute: (...args: unknown[]) => resolveAgentRouteMock(...args),
+}));
+
+vi.mock("../../agents/identity.js", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("../../agents/identity.js")>();
+  return {
+    ...actual,
+    resolveEffectiveMessagesConfig: () => ({ responsePrefix: "" }),
+  };
+});
+
+function encodeValue(parts: { command: string; arg: string; value: string; userId: string }) {
+  return [
+    "cmdarg",
+    encodeURIComponent(parts.command),
+    encodeURIComponent(parts.arg),
+    encodeURIComponent(parts.value),
+    encodeURIComponent(parts.userId),
+  ].join("|");
+}
+
+function createHarness() {
+  const commands = new Map<string, (args: unknown) => Promise<void>>();
+  const actions = new Map<string, (args: unknown) => Promise<void>>();
+
+  const postEphemeral = vi.fn().mockResolvedValue({ ok: true });
+  const app = {
+    client: { chat: { postEphemeral } },
+    command: (name: string, handler: (args: unknown) => Promise<void>) => {
+      commands.set(name, handler);
+    },
+    action: (id: string, handler: (args: unknown) => Promise<void>) => {
+      actions.set(id, handler);
+    },
+  };
+
+  const ctx = {
+    cfg: { commands: { native: true } },
+    runtime: {},
+    botToken: "bot-token",
+    botUserId: "bot",
+    teamId: "T1",
+    allowFrom: ["*"],
+    dmEnabled: true,
+    dmPolicy: "open",
+    groupDmEnabled: false,
+    groupDmChannels: [],
+    defaultRequireMention: true,
+    groupPolicy: "open",
+    useAccessGroups: false,
+    channelsConfig: undefined,
+    slashCommand: { enabled: true, name: "clawd", ephemeral: true, sessionPrefix: "slack:slash" },
+    textLimit: 4000,
+    app,
+    isChannelAllowed: () => true,
+    resolveChannelName: async () => ({ name: "dm", type: "im" }),
+    resolveUserName: async () => ({ name: "Ada" }),
+  } as unknown;
+
+  const account = { accountId: "acct", config: { commands: { native: true } } } as unknown;
+
+  return { commands, actions, postEphemeral, ctx, account };
+}
+
+beforeEach(() => {
+  dispatchMock.mockReset().mockResolvedValue({ counts: { final: 1, tool: 0, block: 0 } });
+  readAllowFromStoreMock.mockReset().mockResolvedValue([]);
+  upsertPairingRequestMock.mockReset().mockResolvedValue({ code: "PAIRCODE", created: true });
+  resolveAgentRouteMock.mockReset().mockReturnValue({
+    agentId: "main",
+    sessionKey: "session:1",
+    accountId: "acct",
+  });
+});
+
+describe("Slack native command argument menus", () => {
+  it("shows a button menu when required args are omitted", async () => {
+    const { commands, ctx, account } = createHarness();
+    registerSlackMonitorSlashCommands({ ctx: ctx as never, account: account as never });
+
+    const handler = commands.get("/cost");
+    if (!handler) throw new Error("Missing /cost handler");
+
+    const respond = vi.fn().mockResolvedValue(undefined);
+    const ack = vi.fn().mockResolvedValue(undefined);
+
+    await handler({
+      command: {
+        user_id: "U1",
+        user_name: "Ada",
+        channel_id: "C1",
+        channel_name: "directmessage",
+        text: "",
+        trigger_id: "t1",
+      },
+      ack,
+      respond,
+    });
+
+    expect(respond).toHaveBeenCalledTimes(1);
+    const payload = respond.mock.calls[0]?.[0] as { blocks?: Array<{ type: string }> };
+    expect(payload.blocks?.[0]?.type).toBe("section");
+    expect(payload.blocks?.[1]?.type).toBe("actions");
+  });
+
+  it("dispatches the command when a menu button is clicked", async () => {
+    const { actions, ctx, account } = createHarness();
+    registerSlackMonitorSlashCommands({ ctx: ctx as never, account: account as never });
+
+    const handler = actions.get("clawdbot_cmdarg");
+    if (!handler) throw new Error("Missing arg-menu action handler");
+
+    const respond = vi.fn().mockResolvedValue(undefined);
+    await handler({
+      ack: vi.fn().mockResolvedValue(undefined),
+      action: {
+        value: encodeValue({ command: "cost", arg: "mode", value: "on", userId: "U1" }),
+      },
+      body: {
+        user: { id: "U1", name: "Ada" },
+        channel: { id: "C1", name: "directmessage" },
+        trigger_id: "t1",
+      },
+      respond,
+    });
+
+    expect(dispatchMock).toHaveBeenCalledTimes(1);
+    const call = dispatchMock.mock.calls[0]?.[0] as { ctx?: { Body?: string } };
+    expect(call.ctx?.Body).toBe("/cost on");
+  });
+
+  it("rejects menu clicks from other users", async () => {
+    const { actions, ctx, account } = createHarness();
+    registerSlackMonitorSlashCommands({ ctx: ctx as never, account: account as never });
+
+    const handler = actions.get("clawdbot_cmdarg");
+    if (!handler) throw new Error("Missing arg-menu action handler");
+
+    const respond = vi.fn().mockResolvedValue(undefined);
+    await handler({
+      ack: vi.fn().mockResolvedValue(undefined),
+      action: {
+        value: encodeValue({ command: "cost", arg: "mode", value: "on", userId: "U1" }),
+      },
+      body: {
+        user: { id: "U2", name: "Eve" },
+        channel: { id: "C1", name: "directmessage" },
+        trigger_id: "t1",
+      },
+      respond,
+    });
+
+    expect(dispatchMock).not.toHaveBeenCalled();
+    expect(respond).toHaveBeenCalledWith({
+      text: "That menu is for another user.",
+      response_type: "ephemeral",
+    });
+  });
+
+  it("falls back to postEphemeral with token when respond is unavailable", async () => {
+    const { actions, postEphemeral, ctx, account } = createHarness();
+    registerSlackMonitorSlashCommands({ ctx: ctx as never, account: account as never });
+
+    const handler = actions.get("clawdbot_cmdarg");
+    if (!handler) throw new Error("Missing arg-menu action handler");
+
+    await handler({
+      ack: vi.fn().mockResolvedValue(undefined),
+      action: { value: "garbage" },
+      body: { user: { id: "U1" }, channel: { id: "C1" } },
+    });
+
+    expect(postEphemeral).toHaveBeenCalledWith(
+      expect.objectContaining({
+        token: "bot-token",
+        channel: "C1",
+        user: "U1",
+      }),
+    );
+  });
+});

src/slack/monitor/slash.ts

@@ -1,5 +1,4 @@
 import type { SlackActionMiddlewareArgs, SlackCommandMiddlewareArgs } from "@slack/bolt";
-import type { Block, KnownBlock } from "@slack/types";
 import type { ChatCommandDefinition, CommandArgs } from "../../auto-reply/commands-registry.js";
 import { resolveEffectiveMessagesConfig } from "../../agents/identity.js";
 import {
@@ -33,6 +32,8 @@ import type { SlackMonitorContext } from "./context.js";
 import { isSlackRoomAllowedByPolicy } from "./policy.js";
 import { deliverSlackSlashReplies } from "./replies.js";
 
+type SlackBlock = { type: string; [key: string]: unknown };
+
 const SLACK_COMMAND_ARG_ACTION_ID = "clawdbot_cmdarg";
 const SLACK_COMMAND_ARG_VALUE_PREFIX = "cmdarg";
 
@@ -68,7 +69,7 @@ function parseSlackCommandArgValue(raw?: string | null): {
 } | null {
   if (!raw) return null;
   const parts = raw.split("|");
-  if (parts.length < 5 || parts[0] !== SLACK_COMMAND_ARG_VALUE_PREFIX) return null;
+  if (parts.length !== 5 || parts[0] !== SLACK_COMMAND_ARG_VALUE_PREFIX) return null;
   const [, command, arg, value, userId] = parts;
   if (!command || !arg || !value || !userId) return null;
   return {
@@ -117,6 +118,9 @@ export function registerSlackMonitorSlashCommands(params: {
   const cfg = ctx.cfg;
   const runtime = ctx.runtime;
 
+  const supportsInteractiveArgMenus =
+    typeof (ctx.app as { action?: unknown }).action === "function";
+
   const slashCommand = resolveSlackSlashCommandConfig(
     ctx.slashCommand ?? account.config.slashCommand,
   );
@@ -266,7 +270,7 @@ export function registerSlackMonitorSlashCommands(params: {
         return;
       }
 
-      if (commandDefinition) {
+      if (commandDefinition && supportsInteractiveArgMenus) {
         const menu = resolveCommandArgMenu({
           command: commandDefinition,
           args: commandArgs,
@@ -432,19 +436,20 @@ export function registerSlackMonitorSlashCommands(params: {
     logVerbose("slack: slash commands disabled");
   }
 
-  ctx.app.action(SLACK_COMMAND_ARG_ACTION_ID, async (args: SlackActionMiddlewareArgs) => {
+  if (nativeCommands.length === 0 || !supportsInteractiveArgMenus) return;
+
+  (
+    ctx.app as unknown as { action: NonNullable<(typeof ctx.app & { action?: unknown })["action"]> }
+  ).action(SLACK_COMMAND_ARG_ACTION_ID, async (args: SlackActionMiddlewareArgs) => {
     const { ack, body, respond } = args;
     const action = args.action as { value?: string };
     await ack();
     const respondFn =
       respond ??
-      (async (payload: {
-        text: string;
-        blocks?: (Block | KnownBlock)[];
-        response_type?: string;
-      }) => {
+      (async (payload: { text: string; blocks?: SlackBlock[]; response_type?: string }) => {
         if (!body.channel?.id || !body.user?.id) return;
         await ctx.app.client.chat.postEphemeral({
+          token: ctx.botToken,
           channel: body.channel.id,
           user: body.user.id,
           text: payload.text,