fix: tighten commands output + telegram pagination (#2504)

Co-authored-by: hougangdev <hougangdev@users.noreply.github.com>
2026-01-27 02:35:09 -05:00
parent 97440eaf52
commit cc1782b105
6 changed files with 263 additions and 110 deletions
--- a/src/auto-reply/reply/commands-info.test.ts
+++ b/src/auto-reply/reply/commands-info.test.ts
@@ -0,0 +1,13 @@
+import { describe, expect, it } from "vitest";
+import { buildCommandsPaginationKeyboard } from "./commands-info.js";
+
+describe("buildCommandsPaginationKeyboard", () => {
+  it("adds agent id to callback data when provided", () => {
+    const keyboard = buildCommandsPaginationKeyboard(2, 3, "agent-main");
+    expect(keyboard[0]).toEqual([
+      { text: "◀ Prev", callback_data: "commands_page_1:agent-main" },
+      { text: "2/3", callback_data: "commands_page_noop:agent-main" },
+      { text: "Next ▶", callback_data: "commands_page_3:agent-main" },
+    ]);
+  });
+});
--- a/src/auto-reply/reply/commands-info.ts
+++ b/src/auto-reply/reply/commands-info.ts
@@ -1,5 +1,5 @@
 import { logVerbose } from "../../globals.js";
-import { listSkillCommandsForWorkspace } from "../skill-commands.js";
+import { listSkillCommandsForAgents } from "../skill-commands.js";
 import {
  buildCommandsMessage,
  buildCommandsMessagePaginated,
@@ -35,20 +35,18 @@ export const handleCommandsListCommand: CommandHandler = async (params, allowTex
  }
  const skillCommands =
    params.skillCommands ??
-    listSkillCommandsForWorkspace({
-      workspaceDir: params.workspaceDir,
+    listSkillCommandsForAgents({
      cfg: params.cfg,
+      agentIds: params.agentId ? [params.agentId] : undefined,
    });
  const surface = params.ctx.Surface;

-  // For Telegram, return paginated result with inline buttons
  if (surface === "telegram") {
    const result = buildCommandsMessagePaginated(params.cfg, skillCommands, {
      page: 1,
      surface,
    });

-    // Build inline keyboard for pagination if there are multiple pages
    if (result.totalPages > 1) {
      return {
        shouldContinue: false,
@@ -56,7 +54,11 @@ export const handleCommandsListCommand: CommandHandler = async (params, allowTex
          text: result.text,
          channelData: {
            telegram: {
-              buttons: buildCommandsPaginationKeyboard(result.currentPage, result.totalPages),
+              buttons: buildCommandsPaginationKeyboard(
+                result.currentPage,
+                result.totalPages,
+                params.agentId,
+              ),
            },
          },
        },
@@ -78,17 +80,28 @@ export const handleCommandsListCommand: CommandHandler = async (params, allowTex
 export function buildCommandsPaginationKeyboard(
  currentPage: number,
  totalPages: number,
+  agentId?: string,
 ): Array<Array<{ text: string; callback_data: string }>> {
  const buttons: Array<{ text: string; callback_data: string }> = [];
+  const suffix = agentId ? `:${agentId}` : "";

  if (currentPage > 1) {
-    buttons.push({ text: "◀ Prev", callback_data: `commands_page_${currentPage - 1}` });
+    buttons.push({
+      text: "◀ Prev",
+      callback_data: `commands_page_${currentPage - 1}${suffix}`,
+    });
  }

-  buttons.push({ text: `${currentPage}/${totalPages}`, callback_data: "commands_page_noop" });
+  buttons.push({
+    text: `${currentPage}/${totalPages}`,
+    callback_data: `commands_page_noop${suffix}`,
+  });

  if (currentPage < totalPages) {
-    buttons.push({ text: "Next ▶", callback_data: `commands_page_${currentPage + 1}` });
+    buttons.push({
+      text: "Next ▶",
+      callback_data: `commands_page_${currentPage + 1}${suffix}`,
+    });
  }

  return [buttons];
--- a/src/auto-reply/status.test.ts
+++ b/src/auto-reply/status.test.ts
@@ -4,7 +4,20 @@ import { afterEach, describe, expect, it, vi } from "vitest";
 import { normalizeTestText } from "../../test/helpers/normalize-text.js";
 import { withTempHome } from "../../test/helpers/temp-home.js";
 import type { ClawdbotConfig } from "../config/config.js";
-import { buildCommandsMessage, buildHelpMessage, buildStatusMessage } from "./status.js";
+import {
+  buildCommandsMessage,
+  buildCommandsMessagePaginated,
+  buildHelpMessage,
+  buildStatusMessage,
+} from "./status.js";
+
+const { listPluginCommands } = vi.hoisted(() => ({
+  listPluginCommands: vi.fn(() => []),
+}));
+
+vi.mock("../plugins/commands.js", () => ({
+  listPluginCommands,
+}));

 afterEach(() => {
  vi.restoreAllMocks();
@@ -400,6 +413,8 @@ describe("buildCommandsMessage", () => {
    const text = buildCommandsMessage({
      commands: { config: false, debug: false },
    } as ClawdbotConfig);
+    expect(text).toContain("ℹ️ Slash commands");
+    expect(text).toContain("Status");
    expect(text).toContain("/commands - List all slash commands.");
    expect(text).toContain("/skill - Run a skill by name.");
    expect(text).toContain("/think (/thinking, /t) - Set thinking level.");
@@ -436,3 +451,33 @@ describe("buildHelpMessage", () => {
    expect(text).not.toContain("/debug");
  });
 });
+
+describe("buildCommandsMessagePaginated", () => {
+  it("formats telegram output with pages", () => {
+    const result = buildCommandsMessagePaginated(
+      {
+        commands: { config: false, debug: false },
+      } as ClawdbotConfig,
+      undefined,
+      { surface: "telegram", page: 1 },
+    );
+    expect(result.text).toContain("ℹ️ Commands (1/");
+    expect(result.text).toContain("Session");
+    expect(result.text).toContain("/stop - Stop the current run.");
+  });
+
+  it("includes plugin commands in the paginated list", () => {
+    listPluginCommands.mockReturnValue([
+      { name: "plugin_cmd", description: "Plugin command", pluginId: "demo-plugin" },
+    ]);
+    const result = buildCommandsMessagePaginated(
+      {
+        commands: { config: false, debug: false },
+      } as ClawdbotConfig,
+      undefined,
+      { surface: "telegram", page: 99 },
+    );
+    expect(result.text).toContain("Plugins");
+    expect(result.text).toContain("/plugin_cmd (demo-plugin) - Plugin command");
+  });
+});
--- a/src/auto-reply/status.ts
+++ b/src/auto-reply/status.ts
@@ -34,9 +34,9 @@ import {
  listChatCommandsForConfig,
  type ChatCommandDefinition,
 } from "./commands-registry.js";
-import type { CommandCategory } from "./commands-registry.types.js";
 import { listPluginCommands } from "../plugins/commands.js";
 import type { SkillCommandSpec } from "../agents/skills.js";
+import type { CommandCategory } from "./commands-registry.types.js";
 import type { ElevatedLevel, ReasoningLevel, ThinkLevel, VerboseLevel } from "./thinking.js";
 import type { MediaUnderstandingDecision } from "../media-understanding/types.js";

@@ -471,12 +471,10 @@ function groupCommandsByCategory(
 export function buildHelpMessage(cfg?: ClawdbotConfig): string {
  const lines = ["ℹ️ Help", ""];

-  // Session commands - quick shortcuts
  lines.push("Session");
  lines.push("  /new  |  /reset  |  /compact [instructions]  |  /stop");
  lines.push("");

-  // Options - most commonly used
  const optionParts = ["/think <level>", "/model <id>", "/verbose on|off"];
  if (cfg?.commands?.config === true) optionParts.push("/config");
  if (cfg?.commands?.debug === true) optionParts.push("/debug");
@@ -484,12 +482,10 @@ export function buildHelpMessage(cfg?: ClawdbotConfig): string {
  lines.push(`  ${optionParts.join("  |  ")}`);
  lines.push("");

-  // Status commands
  lines.push("Status");
  lines.push("  /status  |  /whoami  |  /context");
  lines.push("");

-  // Skills
  lines.push("Skills");
  lines.push("  /skill <name> [input]");

@@ -534,6 +530,54 @@ function formatCommandEntry(command: ChatCommandDefinition): string {
  return `${primary}${aliasLabel}${scopeLabel} - ${command.description}`;
 }

+type CommandsListItem = {
+  label: string;
+  text: string;
+};
+
+function buildCommandItems(
+  commands: ChatCommandDefinition[],
+  pluginCommands: ReturnType<typeof listPluginCommands>,
+): CommandsListItem[] {
+  const grouped = groupCommandsByCategory(commands);
+  const items: CommandsListItem[] = [];
+
+  for (const category of CATEGORY_ORDER) {
+    const categoryCommands = grouped.get(category) ?? [];
+    if (categoryCommands.length === 0) continue;
+    const label = CATEGORY_LABELS[category];
+    for (const command of categoryCommands) {
+      items.push({ label, text: formatCommandEntry(command) });
+    }
+  }
+
+  for (const command of pluginCommands) {
+    const pluginLabel = command.pluginId ? ` (${command.pluginId})` : "";
+    items.push({
+      label: "Plugins",
+      text: `/${command.name}${pluginLabel} - ${command.description}`,
+    });
+  }
+
+  return items;
+}
+
+function formatCommandList(items: CommandsListItem[]): string {
+  const lines: string[] = [];
+  let currentLabel: string | null = null;
+
+  for (const item of items) {
+    if (item.label !== currentLabel) {
+      if (lines.length > 0) lines.push("");
+      lines.push(item.label);
+      currentLabel = item.label;
+    }
+    lines.push(`  ${item.text}`);
+  }
+
+  return lines.join("\n");
+}
+
 export function buildCommandsMessage(
  cfg?: ClawdbotConfig,
  skillCommands?: SkillCommandSpec[],
@@ -556,31 +600,11 @@ export function buildCommandsMessagePaginated(
    ? listChatCommandsForConfig(cfg, { skillCommands })
    : listChatCommands({ skillCommands });
  const pluginCommands = listPluginCommands();
+  const items = buildCommandItems(commands, pluginCommands);

-  // For non-Telegram surfaces, show grouped list without pagination
  if (!isTelegram) {
-    const grouped = groupCommandsByCategory(commands);
    const lines = ["ℹ️ Slash commands", ""];
-
-    for (const category of CATEGORY_ORDER) {
-      const categoryCommands = grouped.get(category) ?? [];
-      if (categoryCommands.length === 0) continue;
-
-      lines.push(`${CATEGORY_LABELS[category]}`);
-      for (const command of categoryCommands) {
-        lines.push(`  ${formatCommandEntry(command)}`);
-      }
-      lines.push("");
-    }
-
-    if (pluginCommands.length > 0) {
-      lines.push("Plugins");
-      for (const command of pluginCommands) {
-        const pluginLabel = command.pluginId ? ` (${command.pluginId})` : "";
-        lines.push(`  /${command.name}${pluginLabel} - ${command.description}`);
-      }
-    }
-
+    lines.push(formatCommandList(items));
    return {
      text: lines.join("\n").trim(),
      totalPages: 1,
@@ -590,87 +614,18 @@ export function buildCommandsMessagePaginated(
    };
  }

-  // For Telegram, use pagination
-  const grouped = groupCommandsByCategory(commands);
-
-  // Flatten commands with category headers for pagination
-  type PageItem =
-    | { type: "header"; category: CommandCategory }
-    | { type: "command"; command: ChatCommandDefinition };
-  const items: PageItem[] = [];
-
-  for (const category of CATEGORY_ORDER) {
-    const categoryCommands = grouped.get(category) ?? [];
-    if (categoryCommands.length === 0) continue;
-    items.push({ type: "header", category });
-    for (const command of categoryCommands) {
-      items.push({ type: "command", command });
-    }
-  }
-
-  // Add plugin commands
-  if (pluginCommands.length > 0) {
-    items.push({ type: "header", category: "tools" }); // Reuse tools category for plugins header indicator
-  }
-
-  // Calculate pages based on command count (headers don't count toward limit)
-  const commandItems = items.filter((item) => item.type === "command");
-  const totalCommands = commandItems.length + pluginCommands.length;
+  const totalCommands = items.length;
  const totalPages = Math.max(1, Math.ceil(totalCommands / COMMANDS_PER_PAGE));
  const currentPage = Math.min(page, totalPages);
  const startIndex = (currentPage - 1) * COMMANDS_PER_PAGE;
  const endIndex = startIndex + COMMANDS_PER_PAGE;
+  const pageItems = items.slice(startIndex, endIndex);

-  // Build page content
  const lines = [`ℹ️ Commands (${currentPage}/${totalPages})`, ""];
-
-  let commandIndex = 0;
-  let currentCategory: CommandCategory | null = null;
-  let pageCommandCount = 0;
-
-  for (const item of items) {
-    if (pageCommandCount >= COMMANDS_PER_PAGE) break;
-
-    if (item.type === "header") {
-      currentCategory = item.category;
-      continue;
-    }
-
-    if (commandIndex >= startIndex && commandIndex < endIndex) {
-      // Add category header if this is the first command of a category on this page
-      if (
-        (currentCategory && pageCommandCount === 0) ||
-        items[items.indexOf(item) - 1]?.type === "header"
-      ) {
-        if (currentCategory) {
-          if (pageCommandCount > 0) lines.push("");
-          lines.push(CATEGORY_LABELS[currentCategory]);
-        }
-      }
-      lines.push(`  ${formatCommandEntry(item.command)}`);
-      pageCommandCount++;
-    }
-    commandIndex++;
-  }
-
-  // Add plugin commands if they fall within this page range
-  const pluginStartIndex = commandItems.length;
-  for (let i = 0; i < pluginCommands.length && pageCommandCount < COMMANDS_PER_PAGE; i++) {
-    const pluginIndex = pluginStartIndex + i;
-    if (pluginIndex >= startIndex && pluginIndex < endIndex) {
-      if (i === 0 || pluginIndex === startIndex) {
-        if (pageCommandCount > 0) lines.push("");
-        lines.push("Plugins");
-      }
-      const command = pluginCommands[i];
-      const pluginLabel = command.pluginId ? ` (${command.pluginId})` : "";
-      lines.push(`  /${command.name}${pluginLabel} - ${command.description}`);
-      pageCommandCount++;
-    }
-  }
+  lines.push(formatCommandList(pageItems));

  return {
-    text: lines.join("\n"),
+    text: lines.join("\n").trim(),
    totalPages,
    currentPage,
    hasNext: currentPage < totalPages,
--- a/src/telegram/bot-handlers.ts
+++ b/src/telegram/bot-handlers.ts
@@ -7,6 +7,7 @@ import {
 import { buildCommandsPaginationKeyboard } from "../auto-reply/reply/commands-info.js";
 import { buildCommandsMessagePaginated } from "../auto-reply/status.js";
 import { listSkillCommandsForAgents } from "../auto-reply/skill-commands.js";
+import { resolveDefaultAgentId } from "../agents/agent-scope.js";
 import { loadConfig } from "../config/config.js";
 import { writeConfigFile } from "../config/io.js";
 import { danger, logVerbose, warn } from "../globals.js";
@@ -368,6 +369,47 @@ export const registerTelegramHandlers = ({
        }
      }

+      const paginationMatch = data.match(/^commands_page_(\d+|noop)(?::(.+))?$/);
+      if (paginationMatch) {
+        const pageValue = paginationMatch[1];
+        if (pageValue === "noop") return;
+
+        const page = Number.parseInt(pageValue, 10);
+        if (Number.isNaN(page) || page < 1) return;
+
+        const agentId = paginationMatch[2]?.trim() || resolveDefaultAgentId(cfg) || undefined;
+        const skillCommands = listSkillCommandsForAgents({
+          cfg,
+          agentIds: agentId ? [agentId] : undefined,
+        });
+        const result = buildCommandsMessagePaginated(cfg, skillCommands, {
+          page,
+          surface: "telegram",
+        });
+
+        const keyboard =
+          result.totalPages > 1
+            ? buildInlineKeyboard(
+                buildCommandsPaginationKeyboard(result.currentPage, result.totalPages, agentId),
+              )
+            : undefined;
+
+        try {
+          await bot.api.editMessageText(
+            callbackMessage.chat.id,
+            callbackMessage.message_id,
+            result.text,
+            keyboard ? { reply_markup: keyboard } : undefined,
+          );
+        } catch (editErr) {
+          const errStr = String(editErr);
+          if (!errStr.includes("message is not modified")) {
+            throw editErr;
+          }
+        }
+        return;
+      }
+
      const syntheticMessage: TelegramMessage = {
        ...callbackMessage,
        from: callback.from,
--- a/src/telegram/bot.test.ts
+++ b/src/telegram/bot.test.ts
@@ -93,6 +93,7 @@ const commandSpy = vi.fn();
 const botCtorSpy = vi.fn();
 const answerCallbackQuerySpy = vi.fn(async () => undefined);
 const sendChatActionSpy = vi.fn();
+const editMessageTextSpy = vi.fn(async () => ({ message_id: 88 }));
 const setMessageReactionSpy = vi.fn(async () => undefined);
 const setMyCommandsSpy = vi.fn(async () => undefined);
 const sendMessageSpy = vi.fn(async () => ({ message_id: 77 }));
@@ -102,6 +103,7 @@ type ApiStub = {
  config: { use: (arg: unknown) => void };
  answerCallbackQuery: typeof answerCallbackQuerySpy;
  sendChatAction: typeof sendChatActionSpy;
+  editMessageText: typeof editMessageTextSpy;
  setMessageReaction: typeof setMessageReactionSpy;
  setMyCommands: typeof setMyCommandsSpy;
  sendMessage: typeof sendMessageSpy;
@@ -112,6 +114,7 @@ const apiStub: ApiStub = {
  config: { use: useSpy },
  answerCallbackQuery: answerCallbackQuerySpy,
  sendChatAction: sendChatActionSpy,
+  editMessageText: editMessageTextSpy,
  setMessageReaction: setMessageReactionSpy,
  setMyCommands: setMyCommandsSpy,
  sendMessage: sendMessageSpy,
@@ -192,6 +195,7 @@ describe("createTelegramBot", () => {
    sendPhotoSpy.mockReset();
    setMessageReactionSpy.mockReset();
    answerCallbackQuerySpy.mockReset();
+    editMessageTextSpy.mockReset();
    setMyCommandsSpy.mockReset();
    wasSentByBot.mockReset();
    middlewareUseSpy.mockReset();
@@ -424,6 +428,87 @@ describe("createTelegramBot", () => {
    expect(answerCallbackQuerySpy).toHaveBeenCalledWith("cbq-2");
  });

+  it("edits commands list for pagination callbacks", async () => {
+    onSpy.mockReset();
+    listSkillCommandsForAgents.mockReset();
+
+    createTelegramBot({ token: "tok" });
+    const callbackHandler = onSpy.mock.calls.find((call) => call[0] === "callback_query")?.[1] as (
+      ctx: Record<string, unknown>,
+    ) => Promise<void>;
+    expect(callbackHandler).toBeDefined();
+
+    await callbackHandler({
+      callbackQuery: {
+        id: "cbq-3",
+        data: "commands_page_2:main",
+        from: { id: 9, first_name: "Ada", username: "ada_bot" },
+        message: {
+          chat: { id: 1234, type: "private" },
+          date: 1736380800,
+          message_id: 12,
+        },
+      },
+      me: { username: "clawdbot_bot" },
+      getFile: async () => ({ download: async () => new Uint8Array() }),
+    });
+
+    expect(listSkillCommandsForAgents).toHaveBeenCalledWith({
+      cfg: expect.any(Object),
+      agentIds: ["main"],
+    });
+    expect(editMessageTextSpy).toHaveBeenCalledTimes(1);
+    const [chatId, messageId, text, params] = editMessageTextSpy.mock.calls[0] ?? [];
+    expect(chatId).toBe(1234);
+    expect(messageId).toBe(12);
+    expect(String(text)).toContain("ℹ️ Commands");
+    expect(params).toEqual(
+      expect.objectContaining({
+        reply_markup: expect.any(Object),
+      }),
+    );
+  });
+
+  it("blocks pagination callbacks when allowlist rejects sender", async () => {
+    onSpy.mockReset();
+    editMessageTextSpy.mockReset();
+
+    createTelegramBot({
+      token: "tok",
+      config: {
+        channels: {
+          telegram: {
+            dmPolicy: "pairing",
+            capabilities: { inlineButtons: "allowlist" },
+            allowFrom: [],
+          },
+        },
+      },
+    });
+    const callbackHandler = onSpy.mock.calls.find((call) => call[0] === "callback_query")?.[1] as (
+      ctx: Record<string, unknown>,
+    ) => Promise<void>;
+    expect(callbackHandler).toBeDefined();
+
+    await callbackHandler({
+      callbackQuery: {
+        id: "cbq-4",
+        data: "commands_page_2",
+        from: { id: 9, first_name: "Ada", username: "ada_bot" },
+        message: {
+          chat: { id: 1234, type: "private" },
+          date: 1736380800,
+          message_id: 13,
+        },
+      },
+      me: { username: "clawdbot_bot" },
+      getFile: async () => ({ download: async () => new Uint8Array() }),
+    });
+
+    expect(editMessageTextSpy).not.toHaveBeenCalled();
+    expect(answerCallbackQuerySpy).toHaveBeenCalledWith("cbq-4");
+  });
+
  it("wraps inbound message with Telegram envelope", async () => {
    const originalTz = process.env.TZ;
    process.env.TZ = "Europe/Vienna";