From 8cc05d0ff4b75ac36b03142e1a471e78b3ace2df Mon Sep 17 00:00:00 2001 From: tt-a1i <53142663+tt-a1i@users.noreply.github.com> Date: Sun, 21 Jun 2026 00:57:33 +0800 Subject: [PATCH] fix(cli): wire ACP model-invocable commands --- .../acp-integration/session/Session.test.ts | 2 + .../src/acp-integration/session/Session.ts | 14 +- .../cli/src/nonInteractiveCliCommands.test.ts | 150 ++++++++++++++++++ packages/cli/src/nonInteractiveCliCommands.ts | 141 ++++++++++------ 4 files changed, 254 insertions(+), 53 deletions(-) diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index e31a813213c..9f75374e83d 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -841,6 +841,7 @@ describe('Session', () => { mockConfig, expect.any(AbortSignal), 'acp', + mockSettings, ); expect(mockClient.sessionUpdate).toHaveBeenCalledWith({ sessionId: 'test-session-id', @@ -884,6 +885,7 @@ describe('Session', () => { mockConfig, expect.any(AbortSignal), 'acp', + mockSettings, ); expect(mockClient.sessionUpdate).toHaveBeenCalledWith({ sessionId: 'test-session-id', diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index 74d215f19bf..e1309d37c2b 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -531,8 +531,14 @@ export interface AvailableCommandsSnapshot { export async function buildAvailableCommandsSnapshot( config: Config, abortSignal: AbortSignal = AbortSignal.timeout(10_000), + settings?: LoadedSettings, ): Promise { - const slashCommands = await getAvailableCommands(config, abortSignal, 'acp'); + const slashCommands = await getAvailableCommands( + config, + abortSignal, + 'acp', + settings, + ); const availableCommands: AvailableCommand[] = slashCommands.map((cmd) => { const acceptsInput = @@ -2960,7 +2966,11 @@ export class Session implements SessionContext { async sendAvailableCommandsUpdate(): Promise { try { const { availableCommands, availableSkills, availableSkillDetails } = - await buildAvailableCommandsSnapshot(this.config); + await buildAvailableCommandsSnapshot( + this.config, + undefined, + this.settings, + ); const update: SessionUpdate = { sessionUpdate: 'available_commands_update', diff --git a/packages/cli/src/nonInteractiveCliCommands.test.ts b/packages/cli/src/nonInteractiveCliCommands.test.ts index 1a3f0099389..ddfce119634 100644 --- a/packages/cli/src/nonInteractiveCliCommands.test.ts +++ b/packages/cli/src/nonInteractiveCliCommands.test.ts @@ -713,19 +713,83 @@ describe('handleSlashCommand', () => { expect(result.type).toBe('no_command'); }); + + it('does not expose disabled model-invocable commands through SkillTool', async () => { + const modelInvocableCommand = { + name: 'custom', + description: 'Custom file command', + kind: CommandKind.FILE, + modelInvocable: true, + supportedModes: ['non_interactive'] as ExecutionMode[], + action: vi.fn().mockResolvedValue({ + type: 'submit_prompt', + content: 'Expanded prompt', + }), + }; + mockGetCommands.mockReturnValue([modelInvocableCommand]); + vi.mocked(mockConfig.getDisabledSlashCommands).mockReturnValue([ + 'custom', + ]); + mockCommandServiceCreate.mockImplementation( + async (_loaders, _signal, disabledNames?: ReadonlySet) => { + const commands = + disabledNames?.has('custom') === true + ? [] + : [modelInvocableCommand]; + return { + getCommands: () => commands, + getCommandsForMode: (mode: ExecutionMode) => + filterCommandsForMode(commands, mode), + getModelInvocableCommands: () => + commands.filter((command) => command.modelInvocable === true), + }; + }, + ); + + const result = await handleSlashCommand( + '/custom', + abortController, + mockConfig, + mockSettings, + ); + + expect(result.type).toBe('unsupported'); + if (result.type === 'unsupported') { + expect(result.reason).toContain('disabled'); + } + const provider = vi.mocked(mockConfig.setModelInvocableCommandsProvider) + .mock.calls[0]?.[0]; + expect(provider?.()).toEqual([]); + const executor = vi.mocked(mockConfig.setModelInvocableCommandsExecutor) + .mock.calls[0]?.[0]; + await expect(executor?.('custom')).resolves.toBeNull(); + }); }); }); describe('getAvailableCommands', () => { let mockConfig: Config; + let notifyConfigChanged: ReturnType; + let fireUserPromptExpansionEvent: ReturnType; beforeEach(() => { vi.clearAllMocks(); + mockGetCommandsForMode.mockImplementation((mode: ExecutionMode) => + filterCommandsForMode(mockGetCommands(), mode), + ); + mockGetModelInvocableCommands.mockImplementation(() => + mockGetCommands().filter( + (command: { modelInvocable?: boolean; hidden?: boolean }) => + !command.hidden && command.modelInvocable === true, + ), + ); mockCommandServiceCreate.mockResolvedValue({ getCommands: mockGetCommands, getCommandsForMode: mockGetCommandsForMode, getModelInvocableCommands: mockGetModelInvocableCommands, }); + notifyConfigChanged = vi.fn().mockResolvedValue(undefined); + fireUserPromptExpansionEvent = vi.fn().mockResolvedValue(undefined); mockConfig = { getExperimentalZedIntegration: vi.fn().mockReturnValue(false), @@ -735,6 +799,14 @@ describe('getAvailableCommands', () => { getFolderTrust: vi.fn().mockReturnValue(false), getProjectRoot: vi.fn().mockReturnValue('/test/project'), getDisabledSlashCommands: vi.fn().mockReturnValue([]), + getDisableAllHooks: vi.fn().mockReturnValue(false), + hasHooksForEvent: vi.fn().mockReturnValue(false), + getHookSystem: vi.fn().mockReturnValue({ + fireUserPromptExpansionEvent, + }), + setModelInvocableCommandsProvider: vi.fn(), + setModelInvocableCommandsExecutor: vi.fn(), + getSkillManager: vi.fn().mockReturnValue({ notifyConfigChanged }), storage: {}, } as unknown as Config; }); @@ -756,4 +828,82 @@ describe('getAvailableCommands', () => { expect(commands.map((command) => command.name)).toContain('export'); }); + + it('does not partially register model-invocable commands without settings', async () => { + mockGetCommands.mockReturnValue([ + { + name: 'expand-prompt', + description: 'Expand prompt', + kind: CommandKind.FILE, + modelInvocable: true, + supportedModes: ['acp'] as const, + }, + ]); + + await getAvailableCommands(mockConfig, new AbortController().signal, 'acp'); + + expect(mockConfig.setModelInvocableCommandsProvider).not.toHaveBeenCalled(); + expect(mockConfig.setModelInvocableCommandsExecutor).not.toHaveBeenCalled(); + expect(notifyConfigChanged).not.toHaveBeenCalled(); + }); + + it('registers model-invocable commands for ACP command snapshots', async () => { + const promptCommand = { + name: 'expand-prompt', + description: 'Fallback description', + modelDescription: 'Model-facing description', + kind: CommandKind.FILE, + modelInvocable: true, + supportedModes: ['acp'] as const, + action: vi.fn().mockResolvedValue({ + type: 'submit_prompt', + content: 'expanded prompt', + }), + }; + mockGetCommands.mockReturnValue([promptCommand]); + vi.mocked(mockConfig.hasHooksForEvent).mockReturnValue(true); + const expiredSnapshotSignal = new AbortController(); + expiredSnapshotSignal.abort(); + + await getAvailableCommands( + mockConfig, + expiredSnapshotSignal.signal, + 'acp', + { + system: { path: '', settings: {} }, + systemDefaults: { path: '', settings: {} }, + user: { path: '', settings: {} }, + workspace: { path: '', settings: {} }, + } as LoadedSettings, + ); + + const provider = vi.mocked(mockConfig.setModelInvocableCommandsProvider) + .mock.calls[0]?.[0]; + expect(provider?.()).toEqual([ + { + name: 'expand-prompt', + description: 'Model-facing description', + }, + ]); + + const executor = vi.mocked(mockConfig.setModelInvocableCommandsExecutor) + .mock.calls[0]?.[0]; + await expect(executor?.('expand-prompt', 'with args')).resolves.toBe( + 'expanded prompt', + ); + expect(fireUserPromptExpansionEvent).toHaveBeenCalledTimes(1); + expect(fireUserPromptExpansionEvent.mock.calls[0]?.[3].aborted).toBe(false); + expect(promptCommand.action).toHaveBeenCalledWith( + expect.objectContaining({ + executionMode: 'acp', + invocation: { + raw: '/expand-prompt with args', + name: 'expand-prompt', + args: 'with args', + }, + }), + 'with args', + ); + expect(notifyConfigChanged).toHaveBeenCalledTimes(1); + }); }); diff --git a/packages/cli/src/nonInteractiveCliCommands.ts b/packages/cli/src/nonInteractiveCliCommands.ts index 75abf9943c0..d9d5d4b860a 100644 --- a/packages/cli/src/nonInteractiveCliCommands.ts +++ b/packages/cli/src/nonInteractiveCliCommands.ts @@ -37,6 +37,8 @@ import { const debugLogger = createDebugLogger('NON_INTERACTIVE_COMMANDS'); +type CommandServiceInstance = Awaited>; + /** * Result of handling a slash command in non-interactive mode. * @@ -233,6 +235,72 @@ async function fireUserPromptExpansionHook( }; } +async function registerModelInvocableCommands( + commandService: CommandServiceInstance, + config: Config, + executionMode: ExecutionMode, + settings?: LoadedSettings, +): Promise { + if (!settings) { + return; + } + + config.setModelInvocableCommandsProvider(() => + commandService.getModelInvocableCommands().map((cmd) => ({ + name: cmd.name, + description: cmd.modelDescription ?? cmd.description, + })), + ); + + config.setModelInvocableCommandsExecutor( + async (name: string, args: string = '') => { + const commands = commandService.getModelInvocableCommands(); + const cmd = commands.find((c) => c.name === name); + if (!cmd?.action) return null; + const minimalContext = { + executionMode, + invocation: { + raw: args ? `/${name} ${args}` : `/${name}`, + name, + args, + }, + services: { config, settings, logger: null }, + } as unknown as CommandContext; + const result = await cmd.action(minimalContext, args); + if (!result || result.type !== 'submit_prompt') return null; + const hookSignal = new AbortController().signal; + const hookResult = await fireUserPromptExpansionHook( + config, + name, + args, + result.content, + hookSignal, + ); + if (hookResult.blockedResult) { + return hookResult.blockedResult.type === 'message' + ? { error: hookResult.blockedResult.content } + : null; + } + const content = hookResult.content; + if (typeof content === 'string') return content; + if (Array.isArray(content)) { + return content + .map((p) => + typeof p === 'string' ? p : ((p as { text?: string }).text ?? ''), + ) + .join(''); + } + return null; + }, + ); + + const skillManager = + typeof config.getSkillManager === 'function' + ? config.getSkillManager() + : null; + await skillManager?.notifyConfigChanged(); +} + /** * Processes a slash command in a non-interactive environment. * @@ -287,61 +355,25 @@ export const handleSlashCommand = async ( // fallback existence check below can distinguish a disabled command from a // truly unknown one. Without this, a disabled command would fall through to // `no_command` and be forwarded to the model as plain prompt text. - const commandService = await CommandService.create( + const allCommandService = await CommandService.create( allLoaders, abortController.signal, ); - // Register model-invocable commands provider so the startup snapshot and - // per-turn drain include these in non-interactive / ACP mode. - config.setModelInvocableCommandsProvider(() => - commandService.getModelInvocableCommands().map((cmd) => ({ - name: cmd.name, - description: cmd.modelDescription ?? cmd.description, - })), - ); - // Register executor so SkillTool can invoke model-invocable commands - // (e.g. MCP prompts) that are not file-based skills. - config.setModelInvocableCommandsExecutor( - async (name: string, args: string = '') => { - const commands = commandService.getModelInvocableCommands(); - const cmd = commands.find((c) => c.name === name); - if (!cmd?.action) return null; - const minimalContext = { - executionMode, - invocation: { - raw: args ? `/${name} ${args}` : `/${name}`, - name, - args, - }, - services: { config, settings, logger: null }, - } as unknown as CommandContext; - const result = await cmd.action(minimalContext, args); - if (!result || result.type !== 'submit_prompt') return null; - const hookResult = await fireUserPromptExpansionHook( - config, - name, - args, - result.content, - abortController.signal, - ); - if (hookResult.blockedResult) { - return hookResult.blockedResult.type === 'message' - ? { error: hookResult.blockedResult.content } - : null; - } - const content = hookResult.content; - if (typeof content === 'string') return content; - if (Array.isArray(content)) { - return content - .map((p) => - typeof p === 'string' ? p : ((p as { text?: string }).text ?? ''), - ) - .join(''); - } - return null; - }, + const commandService = + disabledNameSet.size > 0 + ? await CommandService.create( + allLoaders, + abortController.signal, + disabledNameSet, + ) + : allCommandService; + await registerModelInvocableCommands( + commandService, + config, + executionMode, + settings, ); - const allCommands = commandService.getCommands(); + const allCommands = allCommandService.getCommands(); const filteredCommands = commandService .getCommandsForMode(executionMode) .filter((cmd) => !isDisabled(cmd)); @@ -475,6 +507,7 @@ export const getAvailableCommands = async ( config: Config, abortSignal: AbortSignal, mode: ExecutionMode = 'acp', + settings?: LoadedSettings, ): Promise => { try { const loaders = [ @@ -493,6 +526,12 @@ export const getAvailableCommands = async ( ? new Set(disabledSlashCommands) : undefined, ); + await registerModelInvocableCommands( + commandService, + config, + mode, + settings, + ); return commandService.getCommandsForMode(mode) as SlashCommand[]; } catch (error) { // Handle errors gracefully - log and return empty array