diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index 9480d4ebabd..74f5a3ac6b6 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -22183,6 +22183,73 @@ describe('Session', () => { expect(mockChatRecordingService.recordUserMessage).toHaveBeenCalled(); }); + it.each([ + ['skill', CommandKind.SKILL], + ['custom command', CommandKind.FILE], + ])( + 'keeps attachments when a %s expands into a model prompt', + async (_, kind) => { + vi.mocked( + nonInteractiveCliCommands.handleSlashCommand, + ).mockResolvedValueOnce({ + type: 'submit_prompt', + content: [{ text: 'Expanded skill prompt' }], + resolvedCommand: { + name: 'price-sheet', + kind, + }, + }); + + await session.prompt({ + sessionId: 'test-session-id', + prompt: [ + { type: 'text', text: '/price-sheet update these prices' }, + { type: 'image', mimeType: 'image/png', data: 'QUJD' }, + { + type: 'resource', + resource: { + uri: 'attachment:///notes.txt', + mimeType: 'text/plain', + text: 'hello', + }, + }, + ], + }); + + expect(firstSentMessage()).toEqual([ + { text: '@attachment:///notes.txt' }, + { inlineData: { mimeType: 'image/png', data: 'QUJD' } }, + { text: 'File: attachment:///notes.txt\nhello' }, + { text: 'Expanded skill prompt' }, + ]); + }, + ); + + it('does not forward attachments from built-in commands', async () => { + vi.mocked( + nonInteractiveCliCommands.handleSlashCommand, + ).mockResolvedValueOnce({ + type: 'submit_prompt', + content: [{ text: 'Expanded built-in prompt' }], + resolvedCommand: { + name: 'remember', + kind: CommandKind.BUILT_IN, + }, + }); + + await session.prompt({ + sessionId: 'test-session-id', + prompt: [ + { type: 'text', text: '/remember this' }, + { type: 'image', mimeType: 'image/png', data: 'QUJD' }, + ], + }); + + expect(firstSentMessage()).toEqual([ + { text: 'Expanded built-in prompt' }, + ]); + }); + it('preserves an expanded slash prompt cancelled before model send', async () => { const finishedSpy = vi .spyOn(core, 'logConversationFinishedEvent') diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index de4d07bf691..56773c97fe6 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -13510,14 +13510,24 @@ export class Session implements SessionContext { : undefined; switch (result.type) { - case 'submit_prompt': - // Command wants to submit a prompt to the model - // Convert PartListUnion to Part[] + case 'submit_prompt': { + const expandedPrompt = normalizePartList(result.content); + const attachmentBlocks = + result.resolvedCommand?.kind === CommandKind.BUILT_IN + ? [] + : originalPrompt.filter((block) => block.type !== 'text'); + const attachmentParts = + attachmentBlocks.length === 0 + ? [] + : await this.#resolvePrompt(attachmentBlocks, abortSignal, { + deferBridgeConversions: true, + }); return this.#applyBridgeConversionsIfNeeded( - normalizePartList(result.content), + [...attachmentParts, ...expandedPrompt], abortSignal, onFullTurnModel, ); + } case 'message': { if (result.messageType === 'error') { diff --git a/packages/web-shell/client/daemon/session/actions.test.ts b/packages/web-shell/client/daemon/session/actions.test.ts index ac0e436a1b4..b53fbc34f2b 100644 --- a/packages/web-shell/client/daemon/session/actions.test.ts +++ b/packages/web-shell/client/daemon/session/actions.test.ts @@ -2290,6 +2290,7 @@ describe('createDaemonSessionActions', () => { connection: { status: 'connected', workspaceCwd: '/workspace', + commands: [commandInfo('help', 'builtin-command')], capabilities: { v: 1, mode: 'http-bridge', @@ -2316,6 +2317,319 @@ describe('createDaemonSessionActions', () => { ); }); + it('does not upload attachments for built-in command aliases', async () => { + const session = createMockSession('session-a'); + const { actions } = createActionsHarness({ + session, + connection: { + status: 'connected', + workspaceCwd: '/workspace', + commands: [commandInfo('compress', 'builtin-command', ['summarize'])], + capabilities: { + v: 1, + mode: 'http-bridge', + features: ['session_attachments'], + modelServices: [], + }, + }, + }); + + await actions.submitPrompt('/summarize', { + images: [{ data: 'AQID', mimeType: 'image/png' }], + }); + + expect(session.uploadAttachment).not.toHaveBeenCalled(); + expect(session.submitPrompt).toHaveBeenCalledWith({ + prompt: [{ type: 'text', text: '/summarize' }], + }); + }); + + it.each(['/Compress', '/SUMMARIZE'])( + 'keeps attachments for unresolved wrong-case command %s', + async (text) => { + const session = createMockSession('session-a'); + const { actions } = createActionsHarness({ + session, + connection: { + status: 'connected', + workspaceCwd: '/workspace', + commands: [commandInfo('compress', 'builtin-command', ['summarize'])], + capabilities: { + v: 1, + mode: 'http-bridge', + features: ['session_attachments'], + modelServices: [], + }, + }, + }); + + await actions.submitPrompt(text, { + images: [{ data: 'AQID', mimeType: 'image/png' }], + }); + + expect(session.uploadAttachment).toHaveBeenCalledOnce(); + }, + ); + + it('prefers a primary command name over another command alias', async () => { + const session = createMockSession('session-a'); + const { actions } = createActionsHarness({ + session, + connection: { + status: 'connected', + workspaceCwd: '/workspace', + commands: [ + commandInfo('compress', 'builtin-command', ['summarize']), + commandInfo('summarize', 'skill-dir-command'), + ], + capabilities: { + v: 1, + mode: 'http-bridge', + features: ['session_attachments'], + modelServices: [], + }, + }, + }); + + await actions.submitPrompt('/summarize', { + images: [{ data: 'AQID', mimeType: 'image/png' }], + }); + + expect(session.uploadAttachment).toHaveBeenCalledOnce(); + }); + + it('recognizes built-in commands with whitespace after the slash', async () => { + const session = createMockSession('session-a'); + const { actions } = createActionsHarness({ + session, + connection: { + status: 'connected', + workspaceCwd: '/workspace', + commands: [commandInfo('compress', 'builtin-command')], + capabilities: { + v: 1, + mode: 'http-bridge', + features: ['session_attachments'], + modelServices: [], + }, + }, + }); + + await actions.submitPrompt('/ compress', { + images: [{ data: 'AQID', mimeType: 'image/png' }], + }); + + expect(session.uploadAttachment).not.toHaveBeenCalled(); + }); + + it('does not upload an unknown slash command before command metadata loads', async () => { + const session = createMockSession('session-a'); + const { actions } = createActionsHarness({ + session, + connection: { + status: 'connected', + workspaceCwd: '/workspace', + commands: [commandInfo('price-sheet', 'skill-dir-command')], + capabilities: { + v: 1, + mode: 'http-bridge', + features: ['session_attachments'], + modelServices: [], + }, + }, + }); + + await actions.submitPrompt('/remember this API shape', { + images: [{ data: 'AQID', mimeType: 'image/png' }], + }); + + expect(session.uploadAttachment).not.toHaveBeenCalled(); + }); + + it('does not upload a slash command before command metadata loads', async () => { + const session = createMockSession('session-a'); + const { actions } = createActionsHarness({ + session, + connection: { + status: 'connected', + workspaceCwd: '/workspace', + capabilities: { + v: 1, + mode: 'http-bridge', + features: ['session_attachments'], + modelServices: [], + }, + }, + }); + + await actions.submitPrompt('/remember this API shape', { + images: [{ data: 'AQID', mimeType: 'image/png' }], + }); + + expect(session.uploadAttachment).not.toHaveBeenCalled(); + }); + + it.each([ + '// stack trace from prod', + '/* crash note */', + '/var/log/app.log shows the crash', + '/var\\log\\app.log shows the crash', + ])('uploads attachments for non-command slash input %s', async (text) => { + const session = createMockSession('session-a'); + const { actions } = createActionsHarness({ + session, + connection: { + status: 'connected', + workspaceCwd: '/workspace', + capabilities: { + v: 1, + mode: 'http-bridge', + features: ['session_attachments'], + modelServices: [], + }, + }, + }); + + await actions.submitPrompt(text, { + images: [{ data: 'AQID', mimeType: 'image/png' }], + }); + + expect(session.uploadAttachment).toHaveBeenCalledOnce(); + expect(session.submitPrompt).toHaveBeenCalledWith({ + prompt: [ + { type: 'text', text }, + { + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 3, + }, + ], + }); + }); + + it('uploads attachments for unknown commands after metadata loads', async () => { + const session = createMockSession('session-a'); + const { actions } = createActionsHarness({ + session, + connection: { + status: 'connected', + workspaceCwd: '/workspace', + commands: [commandInfo('compress', 'builtin-command')], + capabilities: { + v: 1, + mode: 'http-bridge', + features: ['session_attachments'], + modelServices: [], + }, + }, + }); + + await actions.submitPrompt('/unlisted update', { + images: [{ data: 'AQID', mimeType: 'image/png' }], + }); + + expect(session.uploadAttachment).toHaveBeenCalledOnce(); + }); + + it('uploads attachments used by skill slash commands', async () => { + const session = createMockSession('session-a'); + const { actions, store } = createActionsHarness({ + session, + connection: { + status: 'connected', + workspaceCwd: '/workspace', + commands: [commandInfo('price-sheet', 'skill-dir-command')], + capabilities: { + v: 1, + mode: 'http-bridge', + features: ['session_attachments'], + modelServices: [], + }, + }, + }); + + await actions.submitPrompt('/price-sheet update these prices', { + images: [{ data: 'AQID', mimeType: 'image/png' }], + }); + + expect(session.uploadAttachment).toHaveBeenCalledWith( + expect.any(Blob), + 'image.png', + 'image/png', + undefined, + ); + expect(session.submitPrompt).toHaveBeenCalledWith({ + prompt: [ + { type: 'text', text: '/price-sheet update these prices' }, + { + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 3, + }, + ], + }); + expect(store.appendLocalUserMessage).toHaveBeenCalledWith( + '/price-sheet update these prices', + [{ data: 'AQID', mimeType: 'image/png' }], + undefined, + [], + ); + }); + + it.each([ + ['skill-dir-command', true], + ['builtin-command', false], + ] as const)( + 'classifies %s command attachments on the sendPrompt path', + async (source, shouldUpload) => { + const session = createMockSession('session-a'); + const { actions } = createActionsHarness({ + session, + connection: { + status: 'connected', + workspaceCwd: '/workspace', + commands: [commandInfo('price-sheet', source)], + capabilities: { + v: 1, + mode: 'http-bridge', + features: ['session_attachments'], + modelServices: [], + }, + }, + }); + + const prompt = actions.sendPrompt('/price-sheet update', { + images: [{ data: 'AQID', mimeType: 'image/png' }], + }); + await vi.waitFor(() => expect(session.submitPrompt).toHaveBeenCalled()); + + expect(session.uploadAttachment).toHaveBeenCalledTimes( + shouldUpload ? 1 : 0, + ); + expect(session.submitPrompt).toHaveBeenCalledWith( + { + prompt: shouldUpload + ? [ + { type: 'text', text: '/price-sheet update' }, + { + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 3, + }, + ] + : [{ type: 'text', text: '/price-sheet update' }], + }, + expect.any(AbortSignal), + ); + + await actions.cancel(); + await expect(prompt).resolves.toEqual({ stopReason: 'cancelled' }); + }, + ); + it('uploads text attachments and submits attachment references', async () => { const session = createMockSession('session-a'); const { actions, store } = createActionsHarness({ @@ -3912,11 +4226,13 @@ function createDeferred() { return { promise, resolve, reject }; } -function commandInfo(name: string) { +function commandInfo(name: string, source?: string, altNames?: string[]) { const raw = commandRaw(name); return { name, description: '', + ...(source ? { source } : {}), + ...(altNames ? { altNames } : {}), raw, }; } diff --git a/packages/web-shell/client/daemon/session/actions.ts b/packages/web-shell/client/daemon/session/actions.ts index 57f0bb404b6..04178abd012 100644 --- a/packages/web-shell/client/daemon/session/actions.ts +++ b/packages/web-shell/client/daemon/session/actions.ts @@ -416,6 +416,28 @@ export function createDaemonSessionActions({ ); } + function discardsSlashCommandAttachments(text: string): boolean { + const trimmed = text.trim(); + if (!trimmed.startsWith('/')) return false; + if (trimmed.startsWith('//') || trimmed.startsWith('/*')) return false; + const name = trimmed.slice(1).trim().split(/\s+/, 1)[0]; + if (!name) return false; + if (name.includes('/') || name.includes('\\')) return false; + // Keep this lexical gate and two-pass lookup aligned with the daemon's + // isSlashCommand/parseSlashCommand behavior. A built-in entry marks a + // fully loaded command snapshot; until then unresolved commands fail closed. + const connection = getConnection(); + const command = + connection.commands?.find((candidate) => candidate.name === name) ?? + connection.commands?.find((candidate) => + candidate.altNames?.includes(name), + ); + if (command) return command.source === 'builtin-command'; + return !connection.commands?.some( + (candidate) => candidate.source === 'builtin-command', + ); + } + async function promptContentWithUploadedAttachments( session: DaemonSessionClient, text: string, @@ -432,7 +454,7 @@ export function createDaemonSessionActions({ references: DaemonSessionAttachmentReference[]; fileReferences: DaemonSessionAttachmentReference[]; }> { - if (text.trimStart().startsWith('/')) { + if (discardsSlashCommandAttachments(text)) { return { content: toDaemonPromptContent(text), references: [], @@ -857,9 +879,9 @@ export function createDaemonSessionActions({ img.mimeType || img.mediaType || img.media_type || 'image/*', })); const normalizedFiles = normalizePromptFiles(options?.files); - const slashCommand = text.trimStart().startsWith('/'); - const displayedImages = slashCommand ? [] : normalizedImages; - const displayedFiles = slashCommand ? [] : normalizedFiles; + const discardAttachments = discardsSlashCommandAttachments(text); + const displayedImages = discardAttachments ? [] : normalizedImages; + const displayedFiles = discardAttachments ? [] : normalizedFiles; const inputAnnotations = options?.inputAnnotations && options.inputAnnotations.length > 0 ? options.inputAnnotations @@ -1019,9 +1041,9 @@ export function createDaemonSessionActions({ mimeType: img.mimeType || img.mediaType || img.media_type || 'image/*', })); const normalizedFiles = normalizePromptFiles(options?.files); - const slashCommand = text.trimStart().startsWith('/'); - const displayedImages = slashCommand ? [] : normalizedImages; - const displayedFiles = slashCommand ? [] : normalizedFiles; + const discardAttachments = discardsSlashCommandAttachments(text); + const displayedImages = discardAttachments ? [] : normalizedImages; + const displayedFiles = discardAttachments ? [] : normalizedFiles; const inputAnnotations = options?.inputAnnotations && options.inputAnnotations.length > 0 ? options.inputAnnotations diff --git a/packages/web-shell/client/daemon/session/mappers.test.ts b/packages/web-shell/client/daemon/session/mappers.test.ts index bd9b0058bf5..9ce0bb7c661 100644 --- a/packages/web-shell/client/daemon/session/mappers.test.ts +++ b/packages/web-shell/client/daemon/session/mappers.test.ts @@ -1176,6 +1176,31 @@ describe('updateConnectionFromDaemonEvent', () => { expect(next.skills).toEqual(['review']); }); + it('maps command aliases from available_commands_update metadata', () => { + const next = applyEvent( + { status: 'connected', workspaceCwd: '/workspace' }, + availableCommandsEvent( + [ + { + name: 'compress', + description: 'Compress context', + input: null, + _meta: { source: 'builtin-command', altNames: ['summarize'] }, + }, + ], + [], + ), + ); + + expect(next.commands).toEqual([ + expect.objectContaining({ + name: 'compress', + source: 'builtin-command', + altNames: ['summarize'], + }), + ]); + }); + it('reads nested availableSkills from the daemon wire shape', () => { const next = applyEvent( { status: 'connected', workspaceCwd: '/workspace' }, diff --git a/packages/web-shell/client/daemon/session/mappers.ts b/packages/web-shell/client/daemon/session/mappers.ts index 5618ed4abe1..5af18fc5e50 100644 --- a/packages/web-shell/client/daemon/session/mappers.ts +++ b/packages/web-shell/client/daemon/session/mappers.ts @@ -849,11 +849,17 @@ function mapAvailableCommandsUpdate( function mapCommandMeta( meta: Record | null | undefined, -): Pick { +): Pick { const record = meta ?? undefined; const source = getString(record, 'source'); + const altNames = Array.isArray(record?.['altNames']) + ? record['altNames'].filter( + (name): name is string => typeof name === 'string', + ) + : []; return { ...(source ? { source } : {}), + ...(altNames.length > 0 ? { altNames } : {}), }; } diff --git a/packages/web-shell/client/daemon/session/promptContent.ts b/packages/web-shell/client/daemon/session/promptContent.ts index e69df182767..b8ca43a7b03 100644 --- a/packages/web-shell/client/daemon/session/promptContent.ts +++ b/packages/web-shell/client/daemon/session/promptContent.ts @@ -44,8 +44,8 @@ export function toDaemonPromptContent( files: readonly DaemonPromptFile[] = [], ): PromptContentBlock[] { // Token lines keep a visible trace in the daemon-recorded transcript - // (which stores text blocks only). Skipped for slash commands: the - // daemon's slash path drops attachment blocks, so a token would dangle. + // (which stores text blocks only). Slash commands resolve attachment blocks + // separately from their expanded text, so they do not need token lines. const withTokens = withAttachmentTokens( text, files.map((file) => attachmentUriForName(file.name)), diff --git a/packages/web-shell/client/daemon/session/types.ts b/packages/web-shell/client/daemon/session/types.ts index d2c1ca96b67..d7a1ec5e124 100644 --- a/packages/web-shell/client/daemon/session/types.ts +++ b/packages/web-shell/client/daemon/session/types.ts @@ -322,6 +322,7 @@ export interface DaemonCommandInfo { argumentHint?: string; autoSubmit?: boolean; source?: string; + altNames?: string[]; raw: DaemonAvailableCommand; }