From ca4e8a7dc8efee8dd3daee5d79c8003d746ba893 Mon Sep 17 00:00:00 2001 From: pick-cat Date: Mon, 6 Jul 2026 09:12:51 +0800 Subject: [PATCH] fix(skills): apply command description limits per channel (#99593) * fix(skills): keep command spec descriptions UTF-16 safe at the truncation cut * chore: fix oxlint no-unnecessary-type-assertion in test Co-Authored-By: Claude Opus 4.7 * chore: fix Skill type completeness in test fixture Co-Authored-By: Claude Opus 4.7 * fix(skills): prove UTF-16-safe command truncation * fix(skills): apply description limits per channel --------- Co-authored-by: Claude Opus 4.7 Co-authored-by: Peter Steinberger --- .../monitor/native-command.options.test.ts | 18 +++---- .../src/monitor/native-command.options.ts | 3 +- .../src/mattermost/slash-commands.test.ts | 34 +++++++++++- .../src/mattermost/slash-commands.ts | 32 +++++++++-- src/skills/discovery/command-specs.test.ts | 54 ++++++++++++++++++- src/skills/discovery/command-specs.ts | 13 +---- src/skills/loading/skills.test.ts | 5 +- 7 files changed, 126 insertions(+), 33 deletions(-) diff --git a/extensions/discord/src/monitor/native-command.options.test.ts b/extensions/discord/src/monitor/native-command.options.test.ts index f7e9b5c544da..c130a2177df0 100644 --- a/extensions/discord/src/monitor/native-command.options.test.ts +++ b/extensions/discord/src/monitor/native-command.options.test.ts @@ -580,8 +580,8 @@ describe("createDiscordNativeCommand option wiring", () => { expect(respond).toHaveBeenCalledWith([]); }); - it("truncates Discord command and option descriptions to Discord's limit", () => { - const longDescription = "x".repeat(140); + it("truncates Discord command and option descriptions on a UTF-16 boundary", () => { + const longDescription = `${"x".repeat(99)}😀 trailing`; const cfg = {} as OpenClawConfig; const discordConfig = {} as NonNullable["discord"]; const command = createDiscordNativeCommand({ @@ -606,14 +606,12 @@ describe("createDiscordNativeCommand option wiring", () => { threadBindings: createNoopThreadBindingManager("default"), }); - expect(command.description).toHaveLength(100); - expect(command.description).toBe("x".repeat(100)); - expect(requireOption(command, "input").description).toHaveLength(100); - expect(requireOption(command, "input").description).toBe("x".repeat(100)); + expect(command.description).toBe("x".repeat(99)); + expect(requireOption(command, "input").description).toBe("x".repeat(99)); }); - it("serializes localized command descriptions", () => { - const longDescription = "k".repeat(140); + it("serializes localized command descriptions on a UTF-16 boundary", () => { + const longDescription = `${"k".repeat(99)}😀 trailing`; const command = createDiscordNativeCommand({ command: { name: "localized", @@ -634,14 +632,14 @@ describe("createDiscordNativeCommand option wiring", () => { expect(command.descriptionLocalizations).toEqual({ ko: "현지화된 설명", - "en-GB": "k".repeat(100), + "en-GB": "k".repeat(99), }); expect(command.serialize()).toEqual({ name: "localized", description: "Default description", description_localizations: { ko: "현지화된 설명", - "en-GB": "k".repeat(100), + "en-GB": "k".repeat(99), }, type: ApplicationCommandType.ChatInput, integration_types: [0, 1], diff --git a/extensions/discord/src/monitor/native-command.options.ts b/extensions/discord/src/monitor/native-command.options.ts index 1d0a639bfe5f..0e85df2df521 100644 --- a/extensions/discord/src/monitor/native-command.options.ts +++ b/extensions/discord/src/monitor/native-command.options.ts @@ -8,6 +8,7 @@ import { } from "openclaw/plugin-sdk/native-command-registry"; import { createSubsystemLogger } from "openclaw/plugin-sdk/runtime-env"; import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/string-coerce-runtime"; +import { truncateUtf16Safe } from "openclaw/plugin-sdk/text-utility-runtime"; import type { AutocompleteInteraction, CommandOptions } from "../internal/discord.js"; const log = createSubsystemLogger("discord/native-command"); @@ -27,7 +28,7 @@ export function truncateDiscordCommandDescription(params: { log.warn( `discord: truncating native command description (${label}) from ${value.length} to ${DISCORD_COMMAND_DESCRIPTION_MAX}: ${JSON.stringify(value)}`, ); - return value.slice(0, DISCORD_COMMAND_DESCRIPTION_MAX); + return truncateUtf16Safe(value, DISCORD_COMMAND_DESCRIPTION_MAX); } export function truncateDiscordCommandDescriptionLocalizations(params: { diff --git a/extensions/mattermost/src/mattermost/slash-commands.test.ts b/extensions/mattermost/src/mattermost/slash-commands.test.ts index 182fce74dc48..1078c3b6c021 100644 --- a/extensions/mattermost/src/mattermost/slash-commands.test.ts +++ b/extensions/mattermost/src/mattermost/slash-commands.test.ts @@ -14,6 +14,7 @@ import { describe("slash-commands", () => { async function registerSingleStatusCommand( requestImpl: (path: string, init?: RequestInit) => Promise, + description = "status", ) { const client: MattermostClient = { baseUrl: "https://chat.example.com", @@ -30,7 +31,7 @@ describe("slash-commands", () => { commands: [ { trigger: "oc_status", - description: "status", + description, autoComplete: true, }, ], @@ -162,6 +163,37 @@ describe("slash-commands", () => { expect(request).toHaveBeenCalledTimes(1); }); + it("truncates command descriptions to Mattermost's UTF-8 byte limit", async () => { + const description = `${"x".repeat(127)}😀 trailing`; + const request = vi.fn(async (path: string, init?: RequestInit) => { + if (path.startsWith("/commands?team_id=")) { + return []; + } + if (path === "/commands" && init?.method === "POST") { + const body = JSON.parse(typeof init.body === "string" ? init.body : "{}"); + expect(body.description).toBe("x".repeat(127)); + expect(body.auto_complete_desc).toBe("x".repeat(127)); + expect(Buffer.byteLength(body.description, "utf8")).toBeLessThanOrEqual(128); + return { + id: "cmd-1", + token: "tok-1", + team_id: "team-1", + creator_id: "bot-user", + trigger: "oc_status", + method: MATTERMOST_SLASH_POST_METHOD, + url: "http://gateway/callback", + auto_complete: true, + }; + } + throw new Error(`unexpected request path: ${path}`); + }); + + const result = await registerSingleStatusCommand(request, description); + + expect(result).toHaveLength(1); + expect(request).toHaveBeenCalledTimes(2); + }); + it("skips foreign command trigger collisions instead of mutating non-owned commands", async () => { const request = vi.fn(async (path: string, init?: { method?: string }) => { if (path.startsWith("/commands?team_id=")) { diff --git a/extensions/mattermost/src/mattermost/slash-commands.ts b/extensions/mattermost/src/mattermost/slash-commands.ts index 9536c4f1b2ea..d6caf101fe3d 100644 --- a/extensions/mattermost/src/mattermost/slash-commands.ts +++ b/extensions/mattermost/src/mattermost/slash-commands.ts @@ -5,6 +5,26 @@ import type { MattermostClient } from "./client.js"; // ─── Types ─────────────────────────────────────────────────────────────────── export const MATTERMOST_SLASH_POST_METHOD = "P"; +const MATTERMOST_COMMAND_DESCRIPTION_MAX_BYTES = 128; + +// Mattermost rejects command descriptions above 128 UTF-8 bytes. Keep portable +// descriptions intact until this API boundary so other channels retain their text. +function truncateMattermostCommandDescription(description: string): string { + if (Buffer.byteLength(description, "utf8") <= MATTERMOST_COMMAND_DESCRIPTION_MAX_BYTES) { + return description; + } + let bytes = 0; + let end = 0; + for (const char of description) { + const charBytes = Buffer.byteLength(char, "utf8"); + if (bytes + charBytes > MATTERMOST_COMMAND_DESCRIPTION_MAX_BYTES) { + break; + } + bytes += charBytes; + end += char.length; + } + return description.slice(0, end); +} export type MattermostSlashCommandConfig = { /** Enable native slash commands. "auto" resolves to false for now (opt-in). */ @@ -177,7 +197,8 @@ export const DEFAULT_COMMAND_SPECS: MattermostCommandSpec[] = [ originalName: "queue", description: "Adjust active-run queue behavior", autoComplete: true, - autoCompleteHint: "[steer|followup|collect|interrupt] [debounce:2s] [cap:N] [drop:old|new|summarize]", + autoCompleteHint: + "[steer|followup|collect|interrupt] [debounce:2s] [cap:N] [drop:old|new|summarize]", }, ]; @@ -290,6 +311,7 @@ export async function registerSlashCommands(params: { const registered: MattermostRegisteredCommand[] = []; for (const spec of commands) { + const description = truncateMattermostCommandDescription(spec.description); const existingForTrigger = existingByTrigger.get(spec.trigger) ?? []; const ownedCommands = existingForTrigger.filter( (cmd) => cmd.creator_id?.trim() === normalizedCreatorUserId, @@ -344,9 +366,9 @@ export async function registerSlashCommands(params: { trigger: spec.trigger, method: MATTERMOST_SLASH_POST_METHOD, url: callbackUrl, - description: spec.description, + description, auto_complete: spec.autoComplete, - auto_complete_desc: spec.description, + auto_complete_desc: description, auto_complete_hint: spec.autoCompleteHint, }); registered.push({ @@ -383,9 +405,9 @@ export async function registerSlashCommands(params: { trigger: spec.trigger, method: MATTERMOST_SLASH_POST_METHOD, url: callbackUrl, - description: spec.description, + description, auto_complete: spec.autoComplete, - auto_complete_desc: spec.description, + auto_complete_desc: description, auto_complete_hint: spec.autoCompleteHint, }); log?.(`mattermost: registered command /${spec.trigger} (id=${created.id})`); diff --git a/src/skills/discovery/command-specs.test.ts b/src/skills/discovery/command-specs.test.ts index 9f97618292f3..348bbe4c8897 100644 --- a/src/skills/discovery/command-specs.test.ts +++ b/src/skills/discovery/command-specs.test.ts @@ -1,11 +1,21 @@ // Command spec tests cover skill-provided command metadata and filtering. -import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { createFixtureSkillEntry } from "../test-support/test-helpers.js"; import type { SkillEntry } from "../types.js"; import { buildWorkspaceSkillCommandSpecs } from "./command-specs.js"; +const bundleCommandState = vi.hoisted(() => ({ + entries: [] as Array<{ + pluginId: string; + rawName: string; + description: string; + promptTemplate: string; + sourceFilePath: string; + }>, +})); + vi.mock("../../plugins/bundle-commands.js", () => ({ - loadEnabledClaudeBundleCommands: () => [], + loadEnabledClaudeBundleCommands: () => bundleCommandState.entries, })); vi.mock("../loading/workspace.js", () => ({ @@ -13,6 +23,10 @@ vi.mock("../loading/workspace.js", () => ({ loadVisibleWorkspaceSkillEntries: () => [], })); +afterEach(() => { + bundleCommandState.entries = []; +}); + describe("buildWorkspaceSkillCommandSpecs", () => { it("uses shared user-invocable skill exposure policy", () => { const specs = buildWorkspaceSkillCommandSpecs("/workspace", { @@ -36,4 +50,40 @@ describe("buildWorkspaceSkillCommandSpecs", () => { expect(specs.map((spec) => spec.skillName)).toEqual(["visible"]); }); + + it("preserves workspace skill descriptions for provider-specific limits", () => { + const prefix = "a".repeat(98); + const entry = createFixtureSkillEntry("emoji-skill"); + entry.skill.description = `${prefix}😀 extra text beyond the limit`; + + const specs = buildWorkspaceSkillCommandSpecs("/workspace", { + entries: [entry], + }); + + expect(specs[0]?.description).toBe(entry.skill.description); + }); + + it("preserves bundle command descriptions for provider-specific limits", () => { + const prefix = "a".repeat(98); + const description = `${prefix}😀 extra text beyond the limit`; + bundleCommandState.entries = [ + { + pluginId: "bundle-plugin", + rawName: "bundle-emoji", + description, + promptTemplate: "Run the bundled command.", + sourceFilePath: "/plugins/bundle-plugin/commands/bundle-emoji.md", + }, + ]; + + const specs = buildWorkspaceSkillCommandSpecs("/workspace", { + entries: [], + }); + + expect(specs[0]).toMatchObject({ + skillName: "bundle-emoji", + description, + promptTemplate: "Run the bundled command.", + }); + }); }); diff --git a/src/skills/discovery/command-specs.ts b/src/skills/discovery/command-specs.ts index 1f57c73a3299..8d27dadd9d75 100644 --- a/src/skills/discovery/command-specs.ts +++ b/src/skills/discovery/command-specs.ts @@ -19,7 +19,6 @@ const skillsLogger = createSubsystemLogger("skills"); const skillCommandDebugOnce = new Set(); const SKILL_COMMAND_MAX_LENGTH = 32; const SKILL_COMMAND_FALLBACK = "skill"; -const SKILL_COMMAND_DESCRIPTION_MAX_LENGTH = 100; // De-duplicate noisy skill command diagnostics across large workspace scans. function debugSkillCommandOnce( @@ -128,11 +127,7 @@ export function buildWorkspaceSkillCommandSpecs( ); } used.add(normalizeLowercaseStringOrEmpty(unique)); - const rawDescription = entry.skill.description?.trim() || rawName; - const description = - rawDescription.length > SKILL_COMMAND_DESCRIPTION_MAX_LENGTH - ? rawDescription.slice(0, SKILL_COMMAND_DESCRIPTION_MAX_LENGTH - 1) + "…" - : rawDescription; + const description = entry.skill.description?.trim() || rawName; const dispatch = (() => { const kindRaw = normalizeLowercaseStringOrEmpty( entry.frontmatter?.["command-dispatch"] ?? entry.frontmatter?.["command_dispatch"] ?? "", @@ -201,14 +196,10 @@ export function buildWorkspaceSkillCommandSpecs( ); } used.add(normalizeLowercaseStringOrEmpty(unique)); - const description = - entry.description.length > SKILL_COMMAND_DESCRIPTION_MAX_LENGTH - ? entry.description.slice(0, SKILL_COMMAND_DESCRIPTION_MAX_LENGTH - 1) + "…" - : entry.description; specs.push({ name: unique, skillName: entry.rawName, - description, + description: entry.description, promptTemplate: entry.promptTemplate, sourceFilePath: entry.sourceFilePath, }); diff --git a/src/skills/loading/skills.test.ts b/src/skills/loading/skills.test.ts index e93d6c51c3ab..a28c281a703d 100644 --- a/src/skills/loading/skills.test.ts +++ b/src/skills/loading/skills.test.ts @@ -213,7 +213,7 @@ describe("buildWorkspaceSkillCommandSpecs", () => { expect(commands.find((entry) => entry.skillName === "hidden-skill")).toBeUndefined(); }); - it("truncates descriptions and preserves tool-dispatch metadata", async () => { + it("preserves descriptions and tool-dispatch metadata", async () => { const workspaceDir = await makeWorkspace(); const longDescription = "This is a very long description that exceeds Discord's 100 character limit for slash command descriptions and should be truncated"; @@ -243,8 +243,7 @@ describe("buildWorkspaceSkillCommandSpecs", () => { const shortCmd = commands.find((entry) => entry.skillName === "short-desc"); const cmd = commands.find((entry) => entry.skillName === "tool-dispatch"); - expect(longCmd?.description.length).toBeLessThanOrEqual(100); - expect(longCmd?.description.endsWith("…")).toBe(true); + expect(longCmd?.description).toBe(longDescription); expect(shortCmd?.description).toBe("Short description"); expect(cmd?.dispatch).toEqual({ kind: "tool", toolName: "sessions_send", argMode: "raw" }); expect(cmd?.skillSource).toBe("workspace");