Merge branch 'main' into joh/private-fields-async-fix

Author: jrieken

build/gulpfile.vscode.ts

@@ -393,8 +393,7 @@ function packageTask(platform: string, arch: string, sourceFolderName: string, d
 			}));
 
 
-		const isInsiderOrExploration = quality === 'insider' || quality === 'exploration';
-		const embedded = isInsiderOrExploration
+		const embedded = quality
 			? (product as typeof product & { embedded?: EmbeddedProductInfo }).embedded
 			: undefined;
 
@@ -556,8 +555,8 @@ function packageTask(platform: string, arch: string, sourceFolderName: string, d
 				'**',
 				'!LICENSE',
 				'!version',
-				...(platform === 'darwin' && !isInsiderOrExploration ? ['!**/Contents/Applications', '!**/Contents/Applications/**'] : []),
-				...(platform === 'win32' && !isInsiderOrExploration ? ['!**/electron_proxy.exe'] : []),
+				...(platform === 'darwin' && !quality ? ['!**/Contents/Applications', '!**/Contents/Applications/**'] : []),
+				...(platform === 'win32' && !quality ? ['!**/electron_proxy.exe'] : []),
 			], { dot: true }));
 
 		if (platform === 'linux') {

build/gulpfile.vscode.win32.ts

@@ -112,8 +112,7 @@ function buildWin32Setup(arch: string, target: string): task.CallbackTask {
 			Quality: quality
 		};
 
-		const isInsiderOrExploration = quality === 'insider' || quality === 'exploration';
-		const embedded = isInsiderOrExploration
+		const embedded = quality
 			? (product as typeof product & { embedded?: EmbeddedProductInfo }).embedded
 			: undefined;
 

extensions/copilot/src/extension/prompt/node/pseudoStartStopConversationCallback.ts

@@ -32,6 +32,10 @@ export class PseudoStopStartResponseProcessor implements IResponseProcessor {
 	private nonReportedDeltas: IResponseDelta[] = [];
 	private thinkingActive: boolean = false;
 
+	private static readonly _toolStreamThrottleMs = 100;
+	private readonly _lastToolStreamUpdate = new Map<string, number>();
+	private readonly _pendingToolStreamUpdates = new Map<string, { id: string; arguments: string | undefined }>();
+
 	constructor(
 		private readonly stopStartMappings: readonly StartStopMapping[],
 		private readonly processNonReportedDelta: ((deltas: IResponseDelta[]) => string[]) | undefined,
@@ -43,14 +47,34 @@ export class PseudoStopStartResponseProcessor implements IResponseProcessor {
 	}
 
 	async doProcessResponse(responseStream: AsyncIterable<IResponsePart>, progress: ChatResponseStream, token: CancellationToken): Promise<void> {
-		for await (const { delta } of responseStream) {
+		try {
+			for await (const { delta } of responseStream) {
+				if (token.isCancellationRequested) {
+					return;
+				}
+				this.applyDelta(delta, progress);
+			}
+		} finally {
 			if (token.isCancellationRequested) {
-				return;
+				this._clearPendingToolStreamUpdates();
+			} else {
+				this._flushPendingToolStreamUpdates(progress);
 			}
-			this.applyDelta(delta, progress);
 		}
 	}
 
+	private _clearPendingToolStreamUpdates(): void {
+		this._pendingToolStreamUpdates.clear();
+		this._lastToolStreamUpdate.clear();
+	}
+
+	private _flushPendingToolStreamUpdates(progress: ChatResponseStream): void {
+		for (const update of this._pendingToolStreamUpdates.values()) {
+			progress.updateToolInvocation(update.id, { partialInput: tryParsePartialToolInput(update.arguments) });
+		}
+		this._clearPendingToolStreamUpdates();
+	}
+
 	protected applyDeltaToProgress(delta: IResponseDelta, progress: ChatResponseStream) {
 		if (delta.thinking) {
 			// Don't send parts that are only encrypted content
@@ -79,11 +103,20 @@ export class PseudoStopStartResponseProcessor implements IResponseProcessor {
 		}
 
 		if (delta.copilotToolCallStreamUpdates?.length) {
+			const now = Date.now();
 			for (const update of delta.copilotToolCallStreamUpdates) {
 				if (!update.name) {
 					continue;
 				}
-				progress.updateToolInvocation(update.id ?? '', { partialInput: tryParsePartialToolInput(update.arguments) });
+				const toolId = update.id ?? '';
+				const lastUpdate = this._lastToolStreamUpdate.get(toolId) ?? 0;
+				if (now - lastUpdate >= PseudoStopStartResponseProcessor._toolStreamThrottleMs) {
+					this._lastToolStreamUpdate.set(toolId, now);
+					this._pendingToolStreamUpdates.delete(toolId);
+					progress.updateToolInvocation(toolId, { partialInput: tryParsePartialToolInput(update.arguments) });
+				} else {
+					this._pendingToolStreamUpdates.set(toolId, { id: toolId, arguments: update.arguments });
+				}
 			}
 		}
 	}
@@ -176,6 +209,7 @@ export class PseudoStopStartResponseProcessor implements IResponseProcessor {
 			this.currentStartStop = undefined;
 			this.nonReportedDeltas = [];
 			this.thinkingActive = false;
+			this._clearPendingToolStreamUpdates();
 			if (delta.retryReason === 'network_error' || delta.retryReason === 'server_error') {
 				progress.clearToPreviousToolInvocation(ChatResponseClearToPreviousToolInvocationReason.NoReason);
 			} else if (delta.retryReason === FilterReason.Copyright) {

extensions/copilot/src/extension/test/node/pseudoStartStopConversationCallback.spec.ts

@@ -6,13 +6,14 @@
 import assert from 'assert';
 import * as sinon from 'sinon';
 import { afterEach, beforeEach, suite, test } from 'vitest';
-import type { ChatVulnerability } from 'vscode';
+import type { ChatToolInvocationStreamData, ChatVulnerability } from 'vscode';
 import { IResponsePart } from '../../../platform/chat/common/chatMLFetcher';
 import { IResponseDelta } from '../../../platform/networking/common/fetch';
 import { createPlatformServices } from '../../../platform/test/node/services';
+import { ChatResponseStreamImpl } from '../../../util/common/chatResponseStreamImpl';
 import { SpyChatResponseStream } from '../../../util/common/test/mockChatResponseStream';
 import { AsyncIterableSource } from '../../../util/vs/base/common/async';
-import { CancellationToken } from '../../../util/vs/base/common/cancellation';
+import { CancellationToken, CancellationTokenSource } from '../../../util/vs/base/common/cancellation';
 import { IInstantiationService } from '../../../util/vs/platform/instantiation/common/instantiation';
 import { ChatResponseMarkdownPart, ChatResponseMarkdownWithVulnerabilitiesPart } from '../../../vscodeTypes';
 import { PseudoStopStartResponseProcessor } from '../../prompt/node/pseudoStartStopConversationCallback';
@@ -169,3 +170,159 @@ suite('Post Report Conversation Callback', () => {
 
 	afterEach(() => sinon.restore());
 });
+
+suite('Tool stream throttling', () => {
+	let clock: sinon.SinonFakeTimers;
+	let updateCalls: { toolCallId: string; streamData: ChatToolInvocationStreamData }[];
+	let stream: ChatResponseStreamImpl;
+
+	beforeEach(() => {
+		clock = sinon.useFakeTimers({ now: 1000, toFake: ['Date'] });
+		updateCalls = [];
+		stream = new ChatResponseStreamImpl(
+			() => { },
+			() => { },
+			undefined,
+			undefined,
+			(toolCallId, streamData) => updateCalls.push({ toolCallId, streamData }),
+		);
+	});
+
+	afterEach(() => {
+		clock.restore();
+		sinon.restore();
+	});
+
+	test('first update is emitted immediately', async () => {
+		const responseSource = new AsyncIterableSource<IResponsePart>();
+		const processor = new PseudoStopStartResponseProcessor([], undefined);
+
+		responseSource.emitOne({ delta: { text: '', copilotToolCallStreamUpdates: [{ id: 'tool1', name: 'myTool', arguments: '{"a":1}' }] } });
+		responseSource.resolve();
+
+		await processor.doProcessResponse(responseSource.asyncIterable, stream, CancellationToken.None);
+
+		assert.strictEqual(updateCalls.length, 1);
+		assert.strictEqual(updateCalls[0].toolCallId, 'tool1');
+	});
+
+	test('rapid updates within throttle window are throttled', async () => {
+		const responseSource = new AsyncIterableSource<IResponsePart>();
+		const processor = new PseudoStopStartResponseProcessor([], undefined);
+
+		// First update goes through immediately
+		responseSource.emitOne({ delta: { text: '', copilotToolCallStreamUpdates: [{ id: 'tool1', name: 'myTool', arguments: '{"a":1}' }] } });
+		// These arrive within the 100ms throttle window — should be buffered
+		responseSource.emitOne({ delta: { text: '', copilotToolCallStreamUpdates: [{ id: 'tool1', name: 'myTool', arguments: '{"a":2}' }] } });
+		responseSource.emitOne({ delta: { text: '', copilotToolCallStreamUpdates: [{ id: 'tool1', name: 'myTool', arguments: '{"a":3}' }] } });
+		responseSource.resolve();
+
+		await processor.doProcessResponse(responseSource.asyncIterable, stream, CancellationToken.None);
+
+		// 1 immediate + 1 flush of the last buffered update = 2 total
+		assert.strictEqual(updateCalls.length, 2);
+		assert.strictEqual(updateCalls[0].toolCallId, 'tool1');
+		assert.deepStrictEqual(updateCalls[1].streamData.partialInput, { a: 3 });
+	});
+
+	test('update after throttle window elapses is emitted immediately', async () => {
+		const responseSource = new AsyncIterableSource<IResponsePart>();
+		const processor = new PseudoStopStartResponseProcessor([], undefined);
+
+		responseSource.emitOne({ delta: { text: '', copilotToolCallStreamUpdates: [{ id: 'tool1', name: 'myTool', arguments: '{"a":1}' }] } });
+		clock.tick(100);
+		responseSource.emitOne({ delta: { text: '', copilotToolCallStreamUpdates: [{ id: 'tool1', name: 'myTool', arguments: '{"a":2}' }] } });
+		responseSource.resolve();
+
+		await processor.doProcessResponse(responseSource.asyncIterable, stream, CancellationToken.None);
+
+		// Both emitted immediately (no pending flush needed)
+		assert.strictEqual(updateCalls.length, 2);
+		assert.deepStrictEqual(updateCalls[0].streamData.partialInput, { a: 1 });
+		assert.deepStrictEqual(updateCalls[1].streamData.partialInput, { a: 2 });
+	});
+
+	test('different tool IDs are throttled independently', async () => {
+		const responseSource = new AsyncIterableSource<IResponsePart>();
+		const processor = new PseudoStopStartResponseProcessor([], undefined);
+
+		responseSource.emitOne({ delta: { text: '', copilotToolCallStreamUpdates: [{ id: 'tool1', name: 'myTool', arguments: '{"a":1}' }] } });
+		responseSource.emitOne({ delta: { text: '', copilotToolCallStreamUpdates: [{ id: 'tool2', name: 'myTool', arguments: '{"b":1}' }] } });
+		// These are within the throttle window for their respective tools
+		responseSource.emitOne({ delta: { text: '', copilotToolCallStreamUpdates: [{ id: 'tool1', name: 'myTool', arguments: '{"a":2}' }] } });
+		responseSource.emitOne({ delta: { text: '', copilotToolCallStreamUpdates: [{ id: 'tool2', name: 'myTool', arguments: '{"b":2}' }] } });
+		responseSource.resolve();
+
+		await processor.doProcessResponse(responseSource.asyncIterable, stream, CancellationToken.None);
+
+		// 2 immediate (one per tool) + 2 flushed (one per tool) = 4
+		assert.strictEqual(updateCalls.length, 4);
+	});
+
+	test('pending updates are not flushed on cancellation', async () => {
+		const cts = new CancellationTokenSource();
+		const responseSource = new AsyncIterableSource<IResponsePart>();
+		const processor = new PseudoStopStartResponseProcessor([], undefined);
+
+		// Start processing, then emit items so the for-await loop consumes them
+		const promise = processor.doProcessResponse(responseSource.asyncIterable, stream, cts.token);
+
+		responseSource.emitOne({ delta: { text: '', copilotToolCallStreamUpdates: [{ id: 'tool1', name: 'myTool', arguments: '{"a":1}' }] } });
+		await new Promise(r => setTimeout(r, 0));
+		responseSource.emitOne({ delta: { text: '', copilotToolCallStreamUpdates: [{ id: 'tool1', name: 'myTool', arguments: '{"a":2}' }] } });
+		await new Promise(r => setTimeout(r, 0));
+
+		// Cancel after items are processed but before stream ends
+		cts.cancel();
+		responseSource.resolve();
+
+		await promise;
+
+		// Only the first immediate update — buffered update should NOT be flushed
+		assert.strictEqual(updateCalls.length, 1);
+		assert.strictEqual(updateCalls[0].toolCallId, 'tool1');
+	});
+
+	test('retry clears pending throttle state', async () => {
+		const responseSource = new AsyncIterableSource<IResponsePart>();
+		const clearCalls: number[] = [];
+		const clearStream = new ChatResponseStreamImpl(
+			() => { },
+			() => clearCalls.push(1),
+			undefined,
+			undefined,
+			(toolCallId, streamData) => updateCalls.push({ toolCallId, streamData }),
+		);
+		const processor = new PseudoStopStartResponseProcessor([], undefined);
+
+		// Buffer a pending update
+		responseSource.emitOne({ delta: { text: '', copilotToolCallStreamUpdates: [{ id: 'tool1', name: 'myTool', arguments: '{"a":1}' }] } });
+		responseSource.emitOne({ delta: { text: '', copilotToolCallStreamUpdates: [{ id: 'tool1', name: 'myTool', arguments: '{"a":2}' }] } });
+		// Retry clears everything
+		responseSource.emitOne({ delta: { text: '', retryReason: 'network_error' } });
+		// New update after retry should go through immediately
+		clock.tick(100);
+		responseSource.emitOne({ delta: { text: '', copilotToolCallStreamUpdates: [{ id: 'tool1', name: 'myTool', arguments: '{"a":3}' }] } });
+		responseSource.resolve();
+
+		await processor.doProcessResponse(responseSource.asyncIterable, clearStream, CancellationToken.None);
+
+		// 1 immediate before retry + 1 immediate after retry = 2
+		// The buffered {"a":2} should have been cleared by retry, not flushed
+		assert.strictEqual(updateCalls.length, 2);
+		assert.deepStrictEqual(updateCalls[0].streamData.partialInput, { a: 1 });
+		assert.deepStrictEqual(updateCalls[1].streamData.partialInput, { a: 3 });
+	});
+
+	test('updates without name are skipped', async () => {
+		const responseSource = new AsyncIterableSource<IResponsePart>();
+		const processor = new PseudoStopStartResponseProcessor([], undefined);
+
+		responseSource.emitOne({ delta: { text: '', copilotToolCallStreamUpdates: [{ id: 'tool1', name: undefined as any, arguments: '{"a":1}' }] } });
+		responseSource.resolve();
+
+		await processor.doProcessResponse(responseSource.asyncIterable, stream, CancellationToken.None);
+
+		assert.strictEqual(updateCalls.length, 0);
+	});
+});

extensions/copilot/src/platform/telemetry/common/baseTelemetryService.ts

@@ -8,13 +8,13 @@ import { ICopilotTokenStore } from '../../authentication/common/copilotTokenStor
 import { ICAPIClientService } from '../../endpoint/common/capiClient';
 import { BaseGHTelemetrySender } from './ghTelemetrySender';
 import { BaseMsftTelemetrySender } from './msftTelemetrySender';
-import { ITelemetryService, TelemetryDestination, TelemetryEventMeasurements, TelemetryEventProperties } from './telemetry';
+import { ITelemetryService, TelemetryDestination, TelemetryEventMeasurements, TelemetryEventProperties, TelemetryTrustedValue } from './telemetry';
 
 export class BaseTelemetryService implements ITelemetryService {
 	declare readonly _serviceBrand: undefined;
 	// Properties that are applied to all telemetry events (currently only used by the exp service
 	// TODO @lramos15 extend further to include more
-	private _sharedProperties: Record<string, string> = {};
+	private _sharedProperties: Record<string, string | TelemetryTrustedValue<string>> = {};
 	private _originalExpAssignments: string | undefined;
 	private _additionalExpAssignments: string[] = [];
 	private _disposables: IDisposable[] = [];
@@ -143,7 +143,7 @@ export class BaseTelemetryService implements ITelemetryService {
 			}
 		}
 		this._capiClientService.abExpContext = value;
-		this._sharedProperties['abexp.assignmentcontext'] = value;
+		this._sharedProperties['abexp.assignmentcontext'] = new TelemetryTrustedValue(value);
 	}
 
 	setSharedProperty(name: string, value: string): void {
@@ -165,9 +165,11 @@ export class BaseTelemetryService implements ITelemetryService {
 	}
 
 	postEvent(eventName: string, props: Map<string, string>): void {
-		for (const [key, value] of Object.entries(this._sharedProperties)) {
-			props.set(key, value);
-		}
-		this._microsoftTelemetrySender.postEvent(eventName, props);
+		const properties: Record<string, string | TelemetryTrustedValue<string>> = {
+			...Object.fromEntries(props),
+			...this._sharedProperties
+		};
+		this._microsoftTelemetrySender.sendInternalTelemetryEvent(eventName, properties);
+		this._microsoftTelemetrySender.sendTelemetryEvent(eventName, properties);
 	}
 }

extensions/copilot/src/platform/telemetry/common/msftTelemetrySender.ts

@@ -36,23 +36,6 @@ export class BaseMsftTelemetrySender implements IMSFTTelemetrySender {
 		this._disposables.add(copilotTokenStore.onDidStoreUpdate(() => this.processToken(copilotTokenStore.copilotToken)));
 	}
 
-	/**
-	 * **NOTE**: Do not call directly
-	 * This is just used by the experimentation service to log events to the scorecards
-	 * @param eventName
-	 * @param props
-	 */
-	postEvent(eventName: string, props: Map<string, string>): void {
-		const event: Record<string, string> = {};
-		for (const [key, value] of props) {
-			event[key] = value;
-		}
-		if (this._isInternal) {
-			this.sendInternalTelemetryEvent(eventName, event);
-		}
-		this.sendTelemetryEvent(eventName, event);
-	}
-
 	/**
 	 * Sends a telemetry event regarding internal Microsoft staff only. Will be dropped if telemetry level is below Usage
 	 * @param eventName The name of the event to send
@@ -61,7 +44,7 @@ export class BaseMsftTelemetrySender implements IMSFTTelemetrySender {
 	 * @returns
 	 */
 	sendInternalTelemetryEvent(eventName: string, properties?: TelemetryEventProperties, measurements?: TelemetryEventMeasurements): void {
-		if (!this._internalTelemetryReporter) {
+		if (!this._internalTelemetryReporter || !this._isInternal) {
 			return;
 		}
 		properties = { ...properties, 'common.tid': this._tid, 'common.userName': this._username ?? 'undefined' };

extensions/copilot/src/platform/telemetry/test/node/telemetry.spec.ts

@@ -126,7 +126,7 @@ suite('GitHub Telemetry Sender', function () {
 	const commonTelemetryData = {
 		properties: {
 			copilot_build: new TelemetryTrustedValue('1'),
-			copilot_buildType: new TelemetryTrustedValue('dev'),
+			copilot_buildType: new TelemetryTrustedValue(!!process.env.BUILD_SOURCEVERSION ? 'prod' : 'dev'),
 			copilot_trackingId: new TelemetryTrustedValue('testId'),
 			editor_plugin_version: new TelemetryTrustedValue('simulation-tests-plugin/2'),
 			client_machineid: new TelemetryTrustedValue('test-machine'),

src/vs/code/electron-main/app.ts

@@ -1354,7 +1354,7 @@ export class CodeApplication extends Disposable {
 		const args = this.environmentMainService.args;
 
 		// Handle agents window first based on context
-		if ((process as INodeProcess).isEmbeddedApp || (args['agents'] && this.productService.quality !== 'stable')) {
+		if ((process as INodeProcess).isEmbeddedApp || args['agents']) {
 			return windowsMainService.openAgentsWindow({
 				context,
 				cli: args,
@@ -1736,7 +1736,7 @@ export class CodeApplication extends Disposable {
 	}
 
 	private registerEmbeddedAppWithLaunchServices(): void {
-		if (!isMacintosh || (process as INodeProcess).isEmbeddedApp || !this.productService.embedded?.nameShort || this.productService.quality === 'stable') {
+		if (!isMacintosh || (process as INodeProcess).isEmbeddedApp || !this.productService.embedded?.nameShort) {
 			return;
 		}