From 2ec92799c8ae9c6dbc6cee7931ef29954dc63418 Mon Sep 17 00:00:00 2001 From: murashit Date: Mon, 25 May 2026 10:38:46 +0900 Subject: [PATCH] Enforce slash command argument contracts --- src/features/chat/slash-commands.ts | 38 ++++++------ .../chat/composer/slash-commands.test.ts | 60 ++++++++++++++++--- 2 files changed, 73 insertions(+), 25 deletions(-) diff --git a/src/features/chat/slash-commands.ts b/src/features/chat/slash-commands.ts index 053a81a2..d504da28 100644 --- a/src/features/chat/slash-commands.ts +++ b/src/features/chat/slash-commands.ts @@ -3,7 +3,7 @@ import type { Thread } from "../../generated/app-server/v2/Thread"; import type { UserInput } from "../../generated/app-server/v2/UserInput"; import { getThreadTitle } from "../../domain/threads/model"; import type { ReferencedThreadDisplay } from "../../domain/threads/reference"; -import { slashCommandHelpRows, type SlashCommandName } from "./composer/slash-commands"; +import { slashCommandDefinition, slashCommandHelpRows, type SlashCommandName } from "./composer/slash-commands"; import type { DisplayDetailSection, DisplayDetailMetaRow } from "./display/types"; import { modelOverrideMessage, @@ -54,6 +54,12 @@ export async function executeSlashCommand( args: string, context: SlashCommandExecutionContext, ): Promise { + const argumentError = validateSlashCommandArguments(command, args); + if (argumentError) { + context.addSystemMessage(argumentError); + return; + } + if (command === "new") { await context.startNewThread(); if (args) return { sendText: args }; @@ -73,7 +79,7 @@ export async function executeSlashCommand( if (command === "refer") { const parsed = parseReferArgs(args); if (!parsed) { - context.addSystemMessage("Usage: /refer "); + context.addSystemMessage(usageError(command, "requires a thread and a message")); return; } const thread = resolveThreadArgument(parsed.threadQuery, context.listedThreads); @@ -91,10 +97,6 @@ export async function executeSlashCommand( } if (command === "fork") { - if (args) { - context.addSystemMessage(`Unsupported slash command arguments: ${args}`); - return; - } if (!context.activeThreadId) { context.addSystemMessage("No active thread to fork."); return; @@ -104,10 +106,6 @@ export async function executeSlashCommand( } if (command === "rollback") { - if (args) { - context.addSystemMessage(`Unsupported slash command arguments: ${args}`); - return; - } if (!context.activeThreadId) { context.addSystemMessage("No active thread to roll back."); return; @@ -186,10 +184,6 @@ export async function executeSlashCommand( } if (command === "mcp") { - if (args) { - context.addSystemMessage(`Unsupported slash command arguments: ${args}`); - return; - } context.addStructuredSystemMessage("MCP servers", detailsFromLines(await context.mcpStatusLines())); return; } @@ -213,7 +207,7 @@ export async function executeSlashCommand( return; } if (args) { - context.addSystemMessage(`Unsupported effort: ${args}`); + context.addSystemMessage(`Unsupported effort: ${args}. Usage: ${slashCommandDefinition(command).usage}`); return; } context.addStructuredSystemMessage("Reasoning effort", detailsFromLines(context.effortStatusLines())); @@ -221,10 +215,18 @@ export async function executeSlashCommand( } context.addStructuredSystemMessage("Available slash commands", [{ rows: slashCommandHelpRows() }]); +} - if (args) { - context.addSystemMessage(`Unsupported slash command arguments: ${args}`); - } +function validateSlashCommandArguments(command: SlashCommandName, args: string): string | null { + const definition = slashCommandDefinition(command); + if (definition.argsKind === "none" && args) return usageError(command, "does not take arguments"); + if (definition.argsKind === "threadAndMessage" && !parseReferArgs(args)) return usageError(command, "requires a thread and a message"); + return null; +} + +function usageError(command: SlashCommandName, message: string): string { + const definition = slashCommandDefinition(command); + return `${definition.command} ${message}. Usage: ${definition.usage}`; } function detailsFromLines(lines: string[]): DisplayDetailSection[] { diff --git a/tests/features/chat/composer/slash-commands.test.ts b/tests/features/chat/composer/slash-commands.test.ts index e3e26157..82861cc1 100644 --- a/tests/features/chat/composer/slash-commands.test.ts +++ b/tests/features/chat/composer/slash-commands.test.ts @@ -131,7 +131,7 @@ describe("slash commands", () => { await executeSlashCommand("refer", "thread-2", ctx); expect(ctx.referThread).not.toHaveBeenCalled(); - expect(ctx.addSystemMessage).toHaveBeenCalledWith("Usage: /refer "); + expect(ctx.addSystemMessage).toHaveBeenCalledWith("/refer requires a thread and a message. Usage: /refer "); }); it("rejects /refer for the active thread", async () => { @@ -160,7 +160,7 @@ describe("slash commands", () => { await executeSlashCommand("fork", "anything", ctx); expect(ctx.forkThread).not.toHaveBeenCalled(); - expect(ctx.addSystemMessage).toHaveBeenCalledWith("Unsupported slash command arguments: anything"); + expect(ctx.addSystemMessage).toHaveBeenCalledWith("/fork does not take arguments. Usage: /fork"); }); it("rolls back the active thread for /rollback", async () => { @@ -195,7 +195,7 @@ describe("slash commands", () => { await executeSlashCommand("rollback", "2", ctx); expect(ctx.rollbackThread).not.toHaveBeenCalled(); - expect(ctx.addSystemMessage).toHaveBeenCalledWith("Unsupported slash command arguments: 2"); + expect(ctx.addSystemMessage).toHaveBeenCalledWith("/rollback does not take arguments. Usage: /rollback"); }); it("toggles Plan mode without sending text for bare /plan", async () => { @@ -234,13 +234,13 @@ describe("slash commands", () => { expect(result).toEqual({ sendText: "この依頼からお願いします" }); }); - it("keeps /compact behavior unchanged", async () => { + it("rejects /compact arguments", async () => { const ctx = context(); await executeSlashCommand("compact", "ignored for now", ctx); - expect(ctx.compactThread).toHaveBeenCalledWith("thread-1"); - expect(ctx.addSystemMessage).toHaveBeenCalledWith("Compaction requested."); + expect(ctx.compactThread).not.toHaveBeenCalled(); + expect(ctx.addSystemMessage).toHaveBeenCalledWith("/compact does not take arguments. Usage: /compact"); }); it("archives the active thread for bare /archive", async () => { @@ -305,6 +305,15 @@ describe("slash commands", () => { expect(ctx.addStructuredSystemMessage).toHaveBeenCalledWith("Available slash commands", [{ rows: slashCommandHelpRows() }]); }); + it("rejects /help arguments", async () => { + const ctx = context(); + + await executeSlashCommand("help", "anything", ctx); + + expect(ctx.addStructuredSystemMessage).not.toHaveBeenCalled(); + expect(ctx.addSystemMessage).toHaveBeenCalledWith("/help does not take arguments. Usage: /help"); + }); + it("shows status as a structured system result", async () => { const ctx = context({ statusSummaryLines: () => ["Session status", "Session: thread-1", "Usage limits", "5h: 42%"] }); @@ -322,6 +331,15 @@ describe("slash commands", () => { ]); }); + it("rejects /status arguments", async () => { + const ctx = context(); + + await executeSlashCommand("status", "anything", ctx); + + expect(ctx.addStructuredSystemMessage).not.toHaveBeenCalled(); + expect(ctx.addSystemMessage).toHaveBeenCalledWith("/status does not take arguments. Usage: /status"); + }); + it("shows doctor diagnostics as shared structured sections", async () => { const details = [{ title: "Process", rows: [{ key: "connection", value: "connected" }] }]; const ctx = context({ connectionDiagnosticDetails: () => details }); @@ -332,6 +350,16 @@ describe("slash commands", () => { expect(ctx.addStructuredSystemMessage).toHaveBeenCalledWith("Connection diagnostics", details); }); + it("rejects /doctor arguments", async () => { + const details = [{ title: "Process", rows: [{ key: "connection", value: "connected" }] }]; + const ctx = context({ connectionDiagnosticDetails: () => details }); + + await executeSlashCommand("doctor", "anything", ctx); + + expect(ctx.addStructuredSystemMessage).not.toHaveBeenCalled(); + expect(ctx.addSystemMessage).toHaveBeenCalledWith("/doctor does not take arguments. Usage: /doctor"); + }); + it("documents that /plan can take a message", () => { expect(slashCommandHelpLines().find((line) => line.startsWith("/plan"))).toBe( "/plan [message] - Toggle Plan mode, optionally with a message.", @@ -365,6 +393,15 @@ describe("slash commands", () => { ); }); + it("rejects unsupported reasoning effort with usage", async () => { + const ctx = context(); + + await executeSlashCommand("effort", "extreme", ctx); + + expect(ctx.setRequestedReasoningEffort).not.toHaveBeenCalled(); + expect(ctx.addSystemMessage).toHaveBeenCalledWith("Unsupported effort: extreme. Usage: /effort [effort|default]"); + }); + it("shows MCP server status", async () => { const ctx = context(); @@ -380,7 +417,16 @@ describe("slash commands", () => { await executeSlashCommand("mcp", "enable github", ctx); expect(ctx.mcpStatusLines).not.toHaveBeenCalled(); - expect(ctx.addSystemMessage).toHaveBeenCalledWith("Unsupported slash command arguments: enable github"); + expect(ctx.addSystemMessage).toHaveBeenCalledWith("/mcp does not take arguments. Usage: /mcp"); + }); + + it("rejects /fast arguments before toggling", async () => { + const ctx = context(); + + await executeSlashCommand("fast", "now", ctx); + + expect(ctx.toggleFastMode).not.toHaveBeenCalled(); + expect(ctx.addSystemMessage).toHaveBeenCalledWith("/fast does not take arguments. Usage: /fast"); }); it("documents MCP status", () => {