From 3055fafbf284d6d279ffd6e275d1fc80f7ea555b Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Sun, 7 Jun 2026 11:51:17 -0700 Subject: [PATCH 1/2] Show new local chat sessions in list without manual refresh A brand-new local session has no requests when its chat model is created, so the initial refresh() correctly excludes it. When the first request was sent, the per-model change listener (tryUpdateLiveSessionItem) bailed out early because the session was not yet in _items, so it was never added until something triggered a full refresh. This was effectively an event-ordering race between refresh() reading live items and the model's first-request event. Build the item from the current model state and add it when it becomes listable, regardless of whether an item already existed, while still deduping unchanged items. Update the affected event-count assertion and add a regression test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../localAgentSessionsController.ts | 16 +++- .../localAgentSessionsController.test.ts | 91 +++++++++++++++---- 2 files changed, 82 insertions(+), 25 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/localAgentSessionsController.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/localAgentSessionsController.ts index 1c774c1ee581b9..f4f6fc6f34913a 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/localAgentSessionsController.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/localAgentSessionsController.ts @@ -132,17 +132,23 @@ export class LocalAgentsSessionsController extends Disposable implements IChatSe } private async tryUpdateLiveSessionItem(model: IChatModel): Promise { - const existing = this._items.get(model.sessionResource); - if (!existing) { + // 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 updated = new LocalChatSessionItem(await chatModelToChatDetail(model), model); - if (existing.isEqual(updated)) { + 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..a63f31bcae9f40 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 @@ -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' } as IChatChangeEvent); + }, } 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(); From 1e721c22aa7f745bfdb34b6c6030b8c8dc55787f Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Sun, 7 Jun 2026 14:21:20 -0700 Subject: [PATCH 2/2] Address review: cheap hasRequests gate and correctly-shaped test event - Gate tryUpdateLiveSessionItem on model.hasRequests before building the full detail, avoiding the expensive diff-stats computation in chatModelToChatDetail on every model change for non-listable models. - Fire a correctly-shaped IChatAddRequestEvent (with request payload) from the test mock's setHasRequests helper, only when a request is added. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../browser/agentSessions/localAgentSessionsController.ts | 7 +++++++ .../agentSessions/localAgentSessionsController.test.ts | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/localAgentSessionsController.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/localAgentSessionsController.ts index f4f6fc6f34913a..7f81cfe2a9b90b 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/localAgentSessionsController.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/localAgentSessionsController.ts @@ -132,6 +132,13 @@ export class LocalAgentsSessionsController extends Disposable implements IChatSe } private async tryUpdateLiveSessionItem(model: IChatModel): Promise { + // 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; + } + // 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 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 a63f31bcae9f40..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'; @@ -141,8 +141,8 @@ function createMockChatModel(options: { requests.length = 0; if (value) { requests.push(mockRequest); + _onDidChange.fire({ kind: 'addRequest', request: mockRequest } as IChatAddRequestEvent); } - _onDidChange.fire({ kind: 'addRequest' } as IChatChangeEvent); }, } as Partial as MockChatModel; }