imageCarousel: address provenance, export format and reveal feedback

Exclude pasted cache references from source actions. Reuse image signature detection for resized attachment MIME and correct the suggested save extension. Suspend Explorer auto-reveal while closing the modal and selecting its source, restoring the previous setting even on failure.

Add regression coverage for actual GIF resizing, encoded formats, pasted references, export filenames and reveal failure paths.

Refs #334682

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: digitarald

src/vs/base/common/image.ts

@@ -24,24 +24,36 @@ export function readImageDimensions(buffer: VSBuffer): IImageDimensions | undefi
 	if (bytes.length < 12) {
 		return undefined;
 	}
+	switch (getImageMimeType(buffer)) {
+		case 'image/jpeg': return readJpegDimensions(bytes);
+		case 'image/png': return readPngDimensions(bytes);
+		case 'image/gif': return readGifDimensions(bytes);
+		case 'image/webp': return readWebPDimensions(bytes);
+		default: return undefined;
+	}
+}
+
+/** Detect JPEG, PNG, GIF or WebP from the encoded buffer's signature. */
+export function getImageMimeType(buffer: VSBuffer): string | undefined {
+	const bytes = buffer.buffer;
 	// JPEG: FF D8
 	if (bytes[0] === 0xFF && bytes[1] === 0xD8) {
-		return readJpegDimensions(bytes);
+		return 'image/jpeg';
 	}
 	// PNG: 89 50 4E 47 0D 0A 1A 0A
 	if (bytes[0] === 0x89 && bytes[1] === 0x50 && bytes[2] === 0x4E && bytes[3] === 0x47 &&
 		bytes[4] === 0x0D && bytes[5] === 0x0A && bytes[6] === 0x1A && bytes[7] === 0x0A) {
-		return readPngDimensions(bytes);
+		return 'image/png';
 	}
 	// GIF: "GIF87a" or "GIF89a"
 	if (bytes[0] === 0x47 && bytes[1] === 0x49 && bytes[2] === 0x46 && bytes[3] === 0x38 &&
 		(bytes[4] === 0x37 || bytes[4] === 0x39) && bytes[5] === 0x61) {
-		return readGifDimensions(bytes);
+		return 'image/gif';
 	}
 	// WebP: "RIFF" <size> "WEBP"
 	if (bytes[0] === 0x52 && bytes[1] === 0x49 && bytes[2] === 0x46 && bytes[3] === 0x46 &&
 		bytes[8] === 0x57 && bytes[9] === 0x45 && bytes[10] === 0x42 && bytes[11] === 0x50) {
-		return readWebPDimensions(bytes);
+		return 'image/webp';
 	}
 	return undefined;
 }

src/vs/base/test/common/image.test.ts

@@ -5,7 +5,7 @@
 
 import assert from 'assert';
 import { VSBuffer } from '../../common/buffer.js';
-import { readImageDimensions } from '../../common/image.js';
+import { getImageMimeType, readImageDimensions } from '../../common/image.js';
 import { ensureNoDisposablesAreLeakedInTestSuite } from './utils.js';
 
 function buf(...bytes: number[]): VSBuffer {
@@ -99,6 +99,24 @@ function makeWebPVp8x(width: number, height: number): VSBuffer {
 	);
 }
 
+suite('getImageMimeType', () => {
+	ensureNoDisposablesAreLeakedInTestSuite();
+
+	test('detects encoded formats and rejects incomplete or unrecognized signatures', () => {
+		const fixtures = [
+			makeJpeg(640, 480), makePng(640, 480), makeGif(640, 480),
+			makeWebPVp8(640, 480), makeWebPVp8l(640, 480), makeWebPVp8x(640, 480),
+			VSBuffer.alloc(0), buf(0xFF), makePng(1, 1).slice(0, 7),
+			makeGif(1, 1).slice(0, 5), makeWebPVp8(1, 1).slice(0, 11),
+			VSBuffer.fromString('<svg xmlns="http://www.w3.org/2000/svg"/>'),
+		];
+		assert.deepStrictEqual(fixtures.map(getImageMimeType), [
+			'image/jpeg', 'image/png', 'image/gif', 'image/webp', 'image/webp', 'image/webp',
+			undefined, undefined, undefined, undefined, undefined, undefined,
+		]);
+	});
+});
+
 suite('readImageDimensions', () => {
 	ensureNoDisposablesAreLeakedInTestSuite();
 

src/vs/workbench/contrib/chat/browser/chatImageCarouselService.ts

@@ -5,6 +5,7 @@
 
 import { renderAsPlaintext } from '../../../../base/browser/markdownRenderer.js';
 import { IMarkdownString } from '../../../../base/common/htmlContent.js';
+import { getImageMimeType } from '../../../../base/common/image.js';
 import { stripIcons } from '../../../../base/common/iconLabels.js';
 import { getMediaMime } from '../../../../base/common/mime.js';
 import { isEqual } from '../../../../base/common/resources.js';
@@ -277,7 +278,7 @@ export function buildSingleImageArgs(resource: URI, data: Uint8Array): ICarousel
 	} catch {
 		// keep raw segment if it isn't valid percent-encoding
 	}
-	const mimeType = getMediaMime(resource.path) ?? getMediaMime(name) ?? 'image/png';
+	const mimeType = getImageMimeType(VSBuffer.wrap(data)) ?? getMediaMime(resource.path) ?? getMediaMime(name) ?? 'image/png';
 	return { name, mimeType, data, title: name, sourceUri: getChatImageSourceUri(resource) };
 }
 

src/vs/workbench/contrib/chat/common/chatImageExtraction.ts

@@ -5,6 +5,7 @@
 
 import { decodeBase64, VSBuffer } from '../../../../base/common/buffer.js';
 import { IMarkdownString } from '../../../../base/common/htmlContent.js';
+import { getImageMimeType } from '../../../../base/common/image.js';
 import { getExtensionForMimeType, getMediaMime } from '../../../../base/common/mime.js';
 import { Schemas } from '../../../../base/common/network.js';
 import { URI } from '../../../../base/common/uri.js';
@@ -245,16 +246,17 @@ export function extractImagesFromChatVariables(
 		if (!buffer) {
 			continue;
 		}
-		const mimeType = variable.mimeType ?? getMediaMime(variable.name) ?? 'image/png';
+		const data = VSBuffer.wrap(buffer);
+		const mimeType = getImageMimeType(data) ?? variable.mimeType ?? getMediaMime(variable.name) ?? 'image/png';
 		const uri = variable.references?.[0]?.reference;
 		const imageUri = URI.isUri(uri) ? uri : URI.from({ scheme: 'data', path: `${variable.id}/${encodeURIComponent(variable.name)}` });
 		images.push({
 			id: imageUri.toString(),
 			uri: imageUri,
-			sourceUri: getChatImageSourceUri(imageUri),
+			sourceUri: variable.isPasted ? undefined : getChatImageSourceUri(imageUri),
 			name: variable.name,
 			mimeType,
-			data: VSBuffer.wrap(buffer),
+			data,
 			source: localize('chatImageExtraction.userAttachment', "Attachment"),
 			caption: undefined,
 		});

src/vs/workbench/contrib/chat/test/browser/chatImageCarouselService.test.ts

@@ -4,10 +4,11 @@
  *--------------------------------------------------------------------------------------------*/
 
 import assert from 'assert';
-import { VSBuffer } from '../../../../../base/common/buffer.js';
+import { decodeBase64, VSBuffer } from '../../../../../base/common/buffer.js';
 import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js';
 import { URI } from '../../../../../base/common/uri.js';
 import { buildCollectionArgs, buildSingleImageArgs, collectCarouselSections, findClickedImageIndex, ICarouselSection } from '../../browser/chatImageCarouselService.js';
+import { resizeImage } from '../../browser/chatImageUtils.js';
 import { IChatToolInvocationSerialized } from '../../common/chatService/chatService.js';
 import { ChatResponseResource } from '../../common/model/chatModel.js';
 import { IImageVariableEntry } from '../../common/attachments/chatVariableEntries.js';
@@ -177,6 +178,14 @@ suite('ChatImageCarouselService helpers', () => {
 	});
 
 	suite('buildSingleImageArgs', () => {
+		test('uses encoded MIME rather than the resource extension', () => {
+			const data = new Uint8Array([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]);
+			const args = buildSingleImageArgs(URI.file('/photos/image.gif'), data);
+			assert.deepStrictEqual({ name: args.name, mimeType: args.mimeType, data: args.data }, {
+				name: 'image.gif', mimeType: 'image/png', data,
+			});
+		});
+
 
 		test('keeps real source URIs but excludes synthetic and generated resources', () => {
 			const uris = [
@@ -226,6 +235,22 @@ suite('ChatImageCarouselService helpers', () => {
 	});
 
 	suite('collectCarouselSections', () => {
+		test('preserves resized GIF bytes with PNG MIME and excludes pasted cache sources', async () => {
+			const gif = decodeBase64('R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7');
+			const data = await resizeImage(gif.buffer, 'image/gif');
+			const sections = await collectCarouselSections([], async () => { throw new Error('Unexpected file read'); }, {
+				text: '',
+				attachments: [makeImageVariableEntry({
+					name: 'image.gif', mimeType: 'image/gif', value: data, isPasted: true,
+					references: [{ kind: 'reference', reference: URI.file('/cache/image.gif') }],
+				})],
+			});
+			const image = sections[0].images[0];
+			assert.deepStrictEqual({ mimeType: image.mimeType, sourceUri: image.sourceUri, data: [...image.data] }, {
+				mimeType: 'image/png', sourceUri: undefined, data: [...data],
+			});
+		});
+
 
 		test('preserves image identity and provenance in response, pending request and current input sections', async () => {
 			const uris = [

src/vs/workbench/contrib/chat/test/common/chatImageExtraction.test.ts

@@ -511,6 +511,30 @@ suite('extractImagesFromChatRequest', () => {
 		assert.deepStrictEqual([...result[0].data.buffer], [1, 2, 3]);
 	});
 
+	test('pasted attachments retain cache identities without exposing them as sources', () => {
+		const uri = URI.file('/cache/pasted-image.png');
+		const request = makeRequest([makeImageVariableEntry({
+			value: new Uint8Array([1, 2, 3]),
+			isPasted: true,
+			references: [{ kind: 'reference', reference: uri }],
+		})]);
+		assert.deepStrictEqual(extractImagesFromChatRequest(request).map(image => ({
+			id: image.id, uri: image.uri.toString(), sourceUri: image.sourceUri,
+		})), [{ id: uri.toString(), uri: uri.toString(), sourceUri: undefined }]);
+	});
+
+	test('uses encoded attachment MIME after resizing without changing bytes or identity', () => {
+		const data = new Uint8Array([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]);
+		const request = makeRequest(['gif', 'webp', 'jpeg'].map(extension => makeImageVariableEntry({
+			id: extension, name: `image.${extension}`, mimeType: `image/${extension}`, value: data,
+		})));
+		assert.deepStrictEqual(extractImagesFromChatRequest(request).map(image => ({
+			name: image.name, mimeType: image.mimeType, data: [...image.data.buffer],
+		})), ['gif', 'webp', 'jpeg'].map(extension => ({
+			name: `image.${extension}`, mimeType: 'image/png', data: [...data],
+		})));
+	});
+
 	test('extracts image attachment from ArrayBuffer', () => {
 		const request = makeRequest([
 			makeImageVariableEntry({ value: new Uint8Array([4, 5, 6]).buffer }),

src/vs/workbench/contrib/imageCarousel/browser/imageCarouselActions.ts

@@ -6,7 +6,7 @@
 import { triggerDownload } from '../../../../base/browser/dom.js';
 import { status } from '../../../../base/browser/ui/aria/aria.js';
 import { Codicon } from '../../../../base/common/codicons.js';
-import { getExtensionForMimeType } from '../../../../base/common/mime.js';
+import { getExtensionForMimeType, getMediaMime } from '../../../../base/common/mime.js';
 import { isWeb } from '../../../../base/common/platform.js';
 import { basename, extname } from '../../../../base/common/path.js';
 import { joinPath } from '../../../../base/common/resources.js';
@@ -25,6 +25,7 @@ import { resolveCommandsContext } from '../../../browser/parts/editor/editorComm
 import { IEditorGroupsService } from '../../../services/editor/common/editorGroupsService.js';
 import { IEditorService } from '../../../services/editor/common/editorService.js';
 import { IExplorerService } from '../../files/browser/files.js';
+import { ExplorerView } from '../../files/browser/views/explorerView.js';
 import { VIEW_ID } from '../../files/common/files.js';
 import { IViewsService } from '../../../services/views/common/viewsService.js';
 import { ImageCarouselEditor } from './imageCarouselEditor.js';
@@ -99,7 +100,7 @@ registerAction2(class extends ImageCarouselAction {
 		const dialogs = accessor.get(IFileDialogService);
 		const extension = getExtensionForMimeType(image.mimeType) ?? '';
 		const name = basename(image.name.replace(/\\/g, '/')) || `image${extension}`;
-		const filename = extname(name) ? name : `${name}${extension}`;
+		const filename = extension && getMediaMime(name) !== image.mimeType ? `${name.slice(0, name.length - extname(name).length)}${extension}` : name;
 		const data = await editor.getCurrentImageData();
 		if (isWeb) {
 			triggerDownload(data.buffer, filename);
@@ -155,12 +156,19 @@ registerAction2(class extends ImageCarouselAction {
 		}
 		const input = editor.input!;
 		const explorer = accessor.get(IExplorerService);
-		const view = await accessor.get(IViewsService).openView(explorer.getViewId() ?? VIEW_ID, false);
+		const view = await accessor.get(IViewsService).openView<ExplorerView>(explorer.getViewId() ?? VIEW_ID, false);
 		if (!view) {
 			throw new Error(localize('imageCarousel.explorerUnavailable', "The Explorer view is not available."));
 		}
-		await editor.group.closeEditor(input);
-		await explorer.select(resource, 'force');
-		view.focus();
+		const autoReveal = view.autoReveal;
+		view.autoReveal = false;
+		try {
+			await editor.group.closeEditor(input);
+			view.setExpanded(true);
+			await explorer.select(resource, 'force');
+			view.focus();
+		} finally {
+			view.autoReveal = autoReveal;
+		}
 	}
 });

src/vs/workbench/contrib/imageCarousel/test/browser/imageCarouselActions.test.ts

@@ -5,7 +5,7 @@
 
 import assert from 'assert';
 import sinon from 'sinon';
-import { VSBuffer } from '../../../../../base/common/buffer.js';
+import { decodeBase64, VSBuffer } from '../../../../../base/common/buffer.js';
 import { CancellationToken } from '../../../../../base/common/cancellation.js';
 import { URI } from '../../../../../base/common/uri.js';
 import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js';
@@ -26,7 +26,7 @@ import { ImageCarouselEditor } from '../../browser/imageCarouselEditor.js';
 import { ImageCarouselEditorInput } from '../../browser/imageCarouselEditorInput.js';
 import { ICarouselImage, ImageCarouselContextKeys, ImageCarouselContextMenu } from '../../browser/imageCarouselTypes.js';
 import { isWeb, OS } from '../../../../../base/common/platform.js';
-import { IExplorerService } from '../../../files/browser/files.js';
+import { IExplorerService, IExplorerView } from '../../../files/browser/files.js';
 import { IViewsService } from '../../../../services/views/common/viewsService.js';
 import { KeybindingsRegistry } from '../../../../../platform/keybinding/common/keybindingsRegistry.js';
 import { KeybindingResolver, ResultKind } from '../../../../../platform/keybinding/common/keybindingResolver.js';
@@ -178,23 +178,51 @@ suite('ImageCarouselActions', () => {
 		});
 	});
 
-	test('reveal uses the registered Explorer view in the Agents window', async () => {
-		const sourceUri = URI.file('/images/image.svg');
-		await load([{ id: 'image', name: 'Image', mimeType: 'image/svg+xml', data: svg, sourceUri }]);
-		const calls: string[] = [];
-		instantiation.stub(IExplorerService, {
-			getViewId: () => 'sessions.files.explorer',
-			select: async (resource: URI) => { calls.push(resource.toString()); },
+	for (const failureAt of [undefined, 'close', 'select']) {
+		test(`reveal suppresses auto-reveal and restores it after ${failureAt ?? 'success'}`, async () => {
+			const sourceUri = URI.file('/images/image.svg');
+			await load([{ id: 'image', name: 'Image', mimeType: 'image/svg+xml', data: svg, sourceUri }]);
+			const calls: string[] = [];
+			const failure = new Error('Reveal failed');
+			const view = {
+				autoReveal: 'focusNoScroll' as IExplorerView['autoReveal'],
+				setExpanded: (expanded: boolean) => { calls.push(`expanded:${expanded}`); },
+				focus: () => { calls.push(`focus:${view.autoReveal}`); },
+			};
+			instantiation.stub(IExplorerService, {
+				getViewId: () => 'sessions.files.explorer',
+				select: async (resource: URI, reveal: string) => {
+					calls.push(`select:${view.autoReveal}:${reveal}:${resource.toString()}`);
+					if (failureAt === 'select') {
+						throw failure;
+					}
+				},
+			});
+			instantiation.stub(IViewsService, {});
+			instantiation.stub(IViewsService, 'openView', async (id: string) => {
+				calls.push(id);
+				return view;
+			});
+			sinon.stub(group, 'closeEditor').callsFake(async () => {
+				calls.push(`close:${view.autoReveal}`);
+				if (failureAt === 'close') {
+					throw failure;
+				}
+				return true;
+			});
+			await run('imageCarousel.revealSource');
+			const expected = ['sessions.files.explorer', 'close:false'];
+			if (failureAt !== 'close') {
+				expected.push('expanded:true', `select:false:force:${sourceUri.toString()}`);
+				if (!failureAt) {
+					expected.push('focus:false');
+				}
+			}
+			assert.deepStrictEqual({ calls, errors, autoReveal: view.autoReveal }, {
+				calls: expected, errors: failureAt ? [failure] : [], autoReveal: 'focusNoScroll',
+			});
 		});
-		instantiation.stub(IViewsService, {});
-		instantiation.stub(IViewsService, 'openView', async (id: string) => {
-			calls.push(id);
-			return { focus: () => calls.push('focus') };
-		});
-		sinon.stub(group, 'closeEditor').callsFake(async () => { calls.push('close'); return true; });
-		await run('imageCarousel.revealSource');
-		assert.deepStrictEqual({ calls, errors }, { calls: ['sessions.files.explorer', 'close', sourceUri.toString(), 'focus'], errors: [] });
-	});
+	}
 
 	test('save preserves encoding, supplies an extension and respects cancellation', async function () {
 		if (isWeb) {
@@ -211,11 +239,31 @@ suite('ImageCarouselActions', () => {
 			writes.push(data.toString());
 			return { resource: target };
 		});
+
 		await run('imageCarousel.saveMediaAs');
 		cancel = true;
 		await run('imageCarousel.saveMediaAs');
 		assert.deepStrictEqual({ names: options.map(option => option.defaultUri?.path), writes, errors }, {
 			names: ['/saved/Image.svg', '/saved/Image.svg'], writes: [svg.toString()], errors: [],
 		});
 	});
+
+	test('save corrects an extension that disagrees with the encoded MIME', async function () {
+		if (isWeb) {
+			this.skip();
+		}
+		const png = decodeBase64($<HTMLCanvasElement>('canvas').toDataURL('image/png').split(',')[1]);
+		await load([{ id: 'image', name: 'image.gif', mimeType: 'image/png', data: png }]);
+		let filename: string | undefined;
+		instantiation.stub(IFileDialogService, 'defaultFilePath', async () => URI.file('/saved'));
+		instantiation.stub(IFileDialogService, 'showSaveDialog', async (options: ISaveDialogOptions) => {
+			filename = options.defaultUri?.path;
+			return URI.file('/saved/image.png');
+		});
+		const write = instantiation.stub(IFileService, 'writeFile', async (resource: URI) => ({ resource }));
+		await run('imageCarousel.saveMediaAs');
+		assert.deepStrictEqual({ filename, data: write.firstCall.args[1], errors }, {
+			filename: '/saved/image.png', data: png, errors: [],
+		});
+	});
 });