diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/localAgentSessionsController.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/localAgentSessionsController.ts index 1c774c1ee581b9..7f81cfe2a9b90b 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/localAgentSessionsController.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/localAgentSessionsController.ts @@ -132,17 +132,30 @@ export class LocalAgentsSessionsController extends Disposable implements IChatSe } private async tryUpdateLiveSessionItem(model: IChatModel): Promise { - const existing = this._items.get(model.sessionResource); - if (!existing) { + // Cheap gate first: a model that has no requests is never listable, so + // skip building the full detail (which awaits potentially expensive + // diff stats via `chatModelToChatDetail`) on every model change. + if (!model.hasRequests) { return; } - const updated = new LocalChatSessionItem(await chatModelToChatDetail(model), model); - if (existing.isEqual(updated)) { + // Build the item from the current model state. `toChatSessionItem` + // applies the same qualification rules as the full refresh (e.g. a + // session only becomes listable once it has requests). Doing this here + // also covers the case where a brand-new session becomes listable after + // its first request is sent: previously we bailed out when no item + // existed yet, so such sessions were missed until a manual refresh. + const updated = this.toChatSessionItem(await chatModelToChatDetail(model)); + if (!updated) { + return; + } + + const existing = this._items.get(model.sessionResource); + if (existing && existing.isEqual(updated)) { return; } - this._items.set(existing.resource, updated); + this._items.set(updated.resource, updated); this._onDidChangeChatSessionItems.fire({ addedOrUpdated: [updated] }); } diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/localAgentSessionsController.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/localAgentSessionsController.test.ts index 84996d098c8981..bb2352d4a2ded7 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/localAgentSessionsController.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/localAgentSessionsController.test.ts @@ -18,7 +18,7 @@ import { IChatService, ResponseModelState } from '../../../common/chatService/ch import { chatModelToChatDetail } from '../../../common/chatService/chatServiceImpl.js'; import { ChatSessionStatus, IChatSessionItem, IChatSessionsService, localChatSessionType } from '../../../common/chatSessionsService.js'; import { ChatEditingSessionState, ModifiedFileEntryState } from '../../../common/editing/chatEditingService.js'; -import { IChatChangedRequestEvent, IChatChangeEvent, IChatModel, IChatRequestModel, IChatResponseModel } from '../../../common/model/chatModel.js'; +import { IChatAddRequestEvent, IChatChangedRequestEvent, IChatChangeEvent, IChatModel, IChatRequestModel, IChatResponseModel } from '../../../common/model/chatModel.js'; import { LocalChatSessionUri } from '../../../common/model/chatUri.js'; import { MockChatService } from '../../common/chatService/mockChatService.js'; import { MockChatSessionsService } from '../../common/mockChatSessionsService.js'; @@ -39,6 +39,7 @@ function createTestTiming(options?: { interface MockChatModel extends IChatModel { setCustomTitle(title: string): void; setRequestInProgress(inProgress: boolean): void; + setHasRequests(hasRequests: boolean): void; } function createMockChatModel(options: { @@ -63,25 +64,28 @@ function createMockChatModel(options: { }): MockChatModel { const requests: IChatRequestModel[] = []; - if (options.hasRequests !== false) { - const mockResponse: Partial = { - isComplete: options.lastResponseComplete ?? true, - isCanceled: options.lastResponseCanceled ?? false, - result: options.lastResponseHasError ? { errorDetails: { message: 'error' } } : undefined, - timestamp: options.lastResponseTimestamp ?? Date.now(), - completedAt: options.lastResponseCompletedAt, - response: { - value: [], - getMarkdown: () => '', - getFinalResponse: () => '', - toString: () => options.customTitle ? '' : 'Test response content' - } - }; + const mockResponse: Partial = { + isComplete: options.lastResponseComplete ?? true, + isCanceled: options.lastResponseCanceled ?? false, + result: options.lastResponseHasError ? { errorDetails: { message: 'error' } } : undefined, + timestamp: options.lastResponseTimestamp ?? Date.now(), + completedAt: options.lastResponseCompletedAt, + response: { + value: [], + getMarkdown: () => '', + getFinalResponse: () => '', + toString: () => options.customTitle ? '' : 'Test response content' + } + }; + + const mockRequest = { + id: 'request-1', + response: mockResponse as IChatResponseModel + } as IChatRequestModel; - requests.push({ - id: 'request-1', - response: mockResponse as IChatResponseModel - } as IChatRequestModel); + let hasRequests = options.hasRequests !== false; + if (hasRequests) { + requests.push(mockRequest); } const editingSessionEntries = options.editingSession?.entries.map(entry => ({ @@ -106,7 +110,9 @@ function createMockChatModel(options: { return title; }, sessionResource: options.sessionResource, - hasRequests: options.hasRequests !== false, + get hasRequests() { + return hasRequests; + }, timestamp: options.timestamp ?? Date.now(), timing: createTestTiming({ created: options.timestamp }), requestInProgress, @@ -127,6 +133,17 @@ function createMockChatModel(options: { requestInProgress.set(inProgress, undefined); _onDidChange.fire({ kind: 'changedRequest' } as IChatChangedRequestEvent); }, + setHasRequests: (value: boolean) => { + if (hasRequests === value) { + return; + } + hasRequests = value; + requests.length = 0; + if (value) { + requests.push(mockRequest); + _onDidChange.fire({ kind: 'addRequest', request: mockRequest } as IChatAddRequestEvent); + } + }, } as Partial as MockChatModel; } @@ -603,7 +620,10 @@ suite('LocalAgentsSessionsController', () => { mockModel.setRequestInProgress(true); await onDidChangeChatSessionItems; - assert.strictEqual(changeEventCount, 3); + // The session is now added as soon as the model is seen with + // requests (via the model-change path), so the subsequent + // refresh dedupes instead of firing a separate add event. + assert.strictEqual(changeEventCount, 2); }); }); @@ -673,6 +693,37 @@ suite('LocalAgentsSessionsController', () => { }); }); + test('should add a new session once it gains its first request (without manual refresh)', async () => { + return runWithFakedTimers({}, async () => { + const controller = createController(); + + // Brand-new session: a model exists but has no requests yet, so + // it should not be listed initially. + const sessionResource = LocalChatSessionUri.forSession('new-empty-session'); + const mockModel = createMockChatModel({ + sessionResource, + hasRequests: false + }); + + mockChatService.addSession(mockModel); + await controller.refresh(CancellationToken.None); + assert.strictEqual(controller.items.length, 0, 'session without requests should not be listed'); + + const fired: { addedOrUpdated?: readonly IChatSessionItem[]; removed?: readonly URI[] }[] = []; + disposables.add(controller.onDidChangeChatSessionItems(delta => fired.push(delta))); + + // User sends the first request: the session becomes listable. + const onChange = Event.toPromise(controller.onDidChangeChatSessionItems); + mockChatService.setLiveSessionItems([await chatModelToChatDetail(mockModel)]); + mockModel.setHasRequests(true); + await onChange; + + assert.strictEqual(controller.items.length, 1, 'session should be listed after first request'); + const addedResources = fired.flatMap(d => d.addedOrUpdated ?? []).map(i => i.resource.toString()); + assert.ok(addedResources.includes(sessionResource.toString()), 'new session should be reported via addedOrUpdated'); + }); + }); + test('should clean up model listeners when model is removed via chatModels observable', async () => { return runWithFakedTimers({}, async () => { const controller = createController();