feat(cli): improve slash command menu - require explicit arg handling, execute on selection, show aliases

Author: brandonkachen

cli/src/chat.tsx

@@ -700,31 +700,7 @@ export const Chat = ({
     })
   })
 
-  // Click handlers for suggestion menu items
-  const handleSlashItemClick = useCallback(
-    (index: number) => {
-      const selected = slashMatches[index]
-      if (!selected || slashContext.startIndex < 0) return
-      const before = inputValue.slice(0, slashContext.startIndex)
-      const after = inputValue.slice(
-        slashContext.startIndex + 1 + slashContext.query.length,
-      )
-      const replacement = `/${selected.id} `
-      setInputValue({
-        text: before + replacement + after,
-        cursorPosition: before.length + replacement.length,
-        lastEditDueToNav: false,
-      })
-      setSlashSelectedIndex(0)
-    },
-    [
-      slashMatches,
-      slashContext,
-      inputValue,
-      setInputValue,
-      setSlashSelectedIndex,
-    ],
-  )
+  // handleSlashItemClick is defined later after openPublishMode is available
 
   const handleMentionItemClick = useCallback(
     (index: number) => {
@@ -803,6 +779,26 @@ export const Chat = ({
 
   const publishMutation = usePublishMutation()
 
+  // Click handler for slash menu items (defined here after openPublishMode is available)
+  const handleSlashItemClick = useCallback(
+    async (index: number) => {
+      const selected = slashMatches[index]
+      if (!selected) return
+      // Execute the selected slash command immediately
+      const commandString = `/${selected.id}`
+      setSlashSelectedIndex(0)
+      const result = await onSubmitPrompt(commandString, agentMode)
+      if (result?.openFeedbackMode) {
+        saveCurrentInput('', 0)
+        openFeedbackForMessage(null)
+      }
+      if (result?.openPublishMode) {
+        openPublishMode()
+      }
+    },
+    [slashMatches, setSlashSelectedIndex, onSubmitPrompt, agentMode, saveCurrentInput, openFeedbackForMessage, openPublishMode],
+  )
+
   const inputValueRef = useRef(inputValue)
   const cursorPositionRef = useRef(cursorPosition)
   useEffect(() => {
@@ -1032,26 +1028,48 @@ export const Chat = ({
       },
       onSlashMenuDown: () => setSlashSelectedIndex((prev) => prev + 1),
       onSlashMenuUp: () => setSlashSelectedIndex((prev) => prev - 1),
-      onSlashMenuTab: () =>
-        setSlashSelectedIndex((prev) => (prev + 1) % slashMatches.length),
+      onSlashMenuTab: async () => {
+        // If only one match, execute it immediately (unless the command opts out)
+        if (slashMatches.length === 1) {
+          const selected = slashMatches[0]
+          if (!selected) return
+          // Don't auto-execute commands that opt out (e.g. /exit)
+          if (selected.noTabAutoExecute) {
+            return
+          }
+          const commandString = `/${selected.id}`
+          setSlashSelectedIndex(0)
+          const result = await onSubmitPrompt(commandString, agentMode)
+          if (result?.openFeedbackMode) {
+            saveCurrentInput('', 0)
+            openFeedbackForMessage(null)
+          }
+          if (result?.openPublishMode) {
+            openPublishMode()
+          }
+          return
+        }
+        // Otherwise cycle through options
+        setSlashSelectedIndex((prev) => (prev + 1) % slashMatches.length)
+      },
       onSlashMenuShiftTab: () =>
         setSlashSelectedIndex(
           (prev) => (slashMatches.length + prev - 1) % slashMatches.length,
         ),
-      onSlashMenuSelect: () => {
+      onSlashMenuSelect: async () => {
         const selected = slashMatches[slashSelectedIndex] || slashMatches[0]
-        if (!selected || slashContext.startIndex < 0) return
-        const before = inputValue.slice(0, slashContext.startIndex)
-        const after = inputValue.slice(
-          slashContext.startIndex + 1 + slashContext.query.length,
-        )
-        const replacement = `/${selected.id} `
-        setInputValue({
-          text: before + replacement + after,
-          cursorPosition: before.length + replacement.length,
-          lastEditDueToNav: false,
-        })
+        if (!selected) return
+        // Execute the selected slash command immediately
+        const commandString = `/${selected.id}`
         setSlashSelectedIndex(0)
+        const result = await onSubmitPrompt(commandString, agentMode)
+        if (result?.openFeedbackMode) {
+          saveCurrentInput('', 0)
+          openFeedbackForMessage(null)
+        }
+        if (result?.openPublishMode) {
+          openPublishMode()
+        }
       },
       onMentionMenuDown: () => setAgentSelectedIndex((prev) => prev + 1),
       onMentionMenuUp: () => setAgentSelectedIndex((prev) => prev - 1),
@@ -1169,8 +1187,8 @@ export const Chat = ({
       setSlashSelectedIndex,
       slashMatches,
       slashSelectedIndex,
-      slashContext,
-      inputValue,
+      onSubmitPrompt,
+      agentMode,
       setAgentSelectedIndex,
       agentMatches,
       fileMatches,

cli/src/commands/__tests__/command-args.test.ts

@@ -0,0 +1,192 @@
+import { describe, test, expect, mock, beforeEach } from 'bun:test'
+
+import {
+  COMMAND_REGISTRY,
+  findCommand,
+  defineCommand,
+  defineCommandWithArgs,
+} from '../command-registry'
+
+import type { RouterParams } from '../command-registry'
+
+/**
+ * Tests for the command factory pattern.
+ * 
+ * The factory pattern ensures commands handle arguments correctly:
+ * - defineCommand: creates commands that reject arguments automatically
+ * - defineCommandWithArgs: creates commands that receive and handle arguments
+ */
+describe('command factory pattern', () => {
+  const createMockParams = (
+    overrides: Partial<RouterParams> = {},
+  ): RouterParams => ({
+    abortControllerRef: { current: null },
+    agentMode: 'DEFAULT',
+    inputRef: { current: null },
+    inputValue: '/test',
+    isChainInProgressRef: { current: false },
+    isStreaming: false,
+    logoutMutation: {} as any,
+    streamMessageIdRef: { current: null },
+    addToQueue: mock(() => {}),
+    clearMessages: mock(() => {}),
+    saveToHistory: mock(() => {}),
+    scrollToLatest: mock(() => {}),
+    sendMessage: mock(async () => {}),
+    setCanProcessQueue: mock(() => {}),
+    setInputFocused: mock(() => {}),
+    setInputValue: mock(() => {}),
+    setIsAuthenticated: mock(() => {}),
+    setMessages: mock(() => {}),
+    setUser: mock(() => {}),
+    stopStreaming: mock(() => {}),
+    ...overrides,
+  })
+
+  describe('defineCommand (no args)', () => {
+    test('creates a command that calls handler when no args provided', () => {
+      const handler = mock(() => {})
+      const cmd = defineCommand({
+        name: 'test',
+        handler,
+      })
+
+      const params = createMockParams()
+      cmd.handler(params, '')
+
+      expect(handler).toHaveBeenCalledWith(params)
+    })
+
+    test('creates a command that rejects args with error message', () => {
+      const handler = mock(() => {})
+      const setMessages = mock(() => {})
+      const cmd = defineCommand({
+        name: 'test',
+        handler,
+      })
+
+      const params = createMockParams({ setMessages })
+      cmd.handler(params, 'unexpected args')
+
+      // Handler should NOT be called
+      expect(handler).not.toHaveBeenCalled()
+      // Error message should be shown
+      expect(setMessages).toHaveBeenCalled()
+    })
+
+    test('sets aliases correctly', () => {
+      const cmd = defineCommand({
+        name: 'test',
+        aliases: ['t', 'tst'],
+        handler: () => {},
+      })
+
+      expect(cmd.aliases).toEqual(['t', 'tst'])
+    })
+
+    test('defaults to empty aliases when not provided', () => {
+      const cmd = defineCommand({
+        name: 'test',
+        handler: () => {},
+      })
+
+      expect(cmd.aliases).toEqual([])
+    })
+
+    test('truncates long args in error message', () => {
+      const handler = mock(() => {})
+      let capturedMessage = ''
+      const setMessages = mock((fn: any) => {
+        const messages = fn([])
+        capturedMessage = messages[1]?.content || ''
+      })
+      const cmd = defineCommand({
+        name: 'test',
+        handler,
+      })
+
+      const longArgs = 'a'.repeat(100)
+      const params = createMockParams({ setMessages })
+      cmd.handler(params, longArgs)
+
+      expect(capturedMessage).toContain('...')
+      expect(capturedMessage.length).toBeLessThan(longArgs.length + 100)
+    })
+  })
+
+  describe('defineCommandWithArgs', () => {
+    test('creates a command that passes args to handler', () => {
+      const handler = mock(() => {})
+      const cmd = defineCommandWithArgs({
+        name: 'test',
+        handler,
+      })
+
+      const params = createMockParams()
+      cmd.handler(params, 'some args')
+
+      expect(handler).toHaveBeenCalledWith(params, 'some args')
+    })
+
+    test('creates a command that passes empty args to handler', () => {
+      const handler = mock(() => {})
+      const cmd = defineCommandWithArgs({
+        name: 'test',
+        handler,
+      })
+
+      const params = createMockParams()
+      cmd.handler(params, '')
+
+      expect(handler).toHaveBeenCalledWith(params, '')
+    })
+
+    test('sets aliases correctly', () => {
+      const cmd = defineCommandWithArgs({
+        name: 'test',
+        aliases: ['t', 'tst'],
+        handler: () => {},
+      })
+
+      expect(cmd.aliases).toEqual(['t', 'tst'])
+    })
+  })
+
+  describe('COMMAND_REGISTRY commands', () => {
+    // Derive command categories from the acceptsArgs flag (set by factories)
+    const noArgsCommands = COMMAND_REGISTRY.filter((cmd) => !cmd.acceptsArgs)
+    const withArgsCommands = COMMAND_REGISTRY.filter((cmd) => cmd.acceptsArgs)
+
+    test('there are commands that reject args', () => {
+      expect(noArgsCommands.length).toBeGreaterThan(0)
+    })
+
+    test('there are commands that accept args', () => {
+      expect(withArgsCommands.length).toBeGreaterThan(0)
+    })
+
+    for (const cmd of noArgsCommands) {
+      test(`/${cmd.name} rejects arguments (acceptsArgs=false)`, () => {
+        const setMessages = mock(() => {})
+        const params = createMockParams({
+          inputValue: `/${cmd.name} extra args`,
+          setMessages,
+        })
+
+        cmd.handler(params, 'extra args')
+
+        // Should show error message
+        expect(setMessages).toHaveBeenCalled()
+      })
+    }
+
+    for (const cmd of withArgsCommands) {
+      test(`/${cmd.name} accepts arguments (acceptsArgs=true)`, () => {
+        // Verify the command has acceptsArgs=true
+        expect(cmd.acceptsArgs).toBe(true)
+        // The actual behavior is tested in specific command tests
+        expect(typeof cmd.handler).toBe('function')
+      })
+    }
+  })
+})

cli/src/commands/__tests__/command-sync.test.ts

@@ -0,0 +1,61 @@
+import { describe, test, expect } from 'bun:test'
+
+import { COMMAND_REGISTRY } from '../command-registry'
+import { SLASH_COMMANDS } from '../../data/slash-commands'
+
+/**
+ * These tests ensure that SLASH_COMMANDS (UI metadata) and COMMAND_REGISTRY (execution)
+ * stay in sync. They serve different purposes but should reference the same commands.
+ */
+describe('SLASH_COMMANDS and COMMAND_REGISTRY sync', () => {
+  // Commands that are intentionally hidden from the autocomplete menu
+  // These exist in COMMAND_REGISTRY but not in SLASH_COMMANDS
+  const HIDDEN_COMMANDS = ['login']
+
+  test('every SLASH_COMMAND has a corresponding COMMAND_REGISTRY entry', () => {
+    for (const slashCmd of SLASH_COMMANDS) {
+      const registryCmd = COMMAND_REGISTRY.find((c) => c.name === slashCmd.id)
+      expect(
+        registryCmd,
+        `SLASH_COMMAND "${slashCmd.id}" has no matching COMMAND_REGISTRY entry`,
+      ).toBeDefined()
+    }
+  })
+
+  test('every non-hidden COMMAND_REGISTRY entry has a corresponding SLASH_COMMAND', () => {
+    for (const registryCmd of COMMAND_REGISTRY) {
+      if (HIDDEN_COMMANDS.includes(registryCmd.name)) continue
+
+      const slashCmd = SLASH_COMMANDS.find((s) => s.id === registryCmd.name)
+      expect(
+        slashCmd,
+        `COMMAND_REGISTRY "${registryCmd.name}" has no matching SLASH_COMMAND entry. ` +
+          `If this command should be hidden from autocomplete, add it to HIDDEN_COMMANDS.`,
+      ).toBeDefined()
+    }
+  })
+
+  test('aliases match between SLASH_COMMANDS and COMMAND_REGISTRY', () => {
+    for (const slashCmd of SLASH_COMMANDS) {
+      const registryCmd = COMMAND_REGISTRY.find((c) => c.name === slashCmd.id)
+      if (!registryCmd) continue
+
+      const slashAliases = slashCmd.aliases ?? []
+      const registryAliases = registryCmd.aliases
+
+      expect(
+        slashAliases.sort(),
+        `Aliases mismatch for "${slashCmd.id}"`,
+      ).toEqual(registryAliases.sort())
+    }
+  })
+
+  test('exit command exists with its aliases and noTabAutoExecute flag', () => {
+    const exitCmd = SLASH_COMMANDS.find((cmd) => cmd.id === 'exit')
+    expect(exitCmd).toBeDefined()
+    expect(exitCmd?.aliases).toContain('quit')
+    expect(exitCmd?.aliases).toContain('q')
+    // /exit opts out of Tab auto-execute (too dangerous)
+    expect(exitCmd?.noTabAutoExecute).toBe(true)
+  })
+})