diff --git a/.github/skills/sessions/SKILL.md b/.github/skills/sessions/SKILL.md index bf9c29e7426..f9a4a7d233a 100644 --- a/.github/skills/sessions/SKILL.md +++ b/.github/skills/sessions/SKILL.md @@ -46,6 +46,7 @@ Then read the relevant spec for the area you are changing (see table below). If - **Scrollable transcript surfaces must use workbench scrollbars**: Don't make Agents/voice transcript regions scrollable with native `overflow-y: auto` on the content node. Wrap transcript content in `DomScrollableElement`/list widgets so scrollbars match VS Code theming and remain usable in narrow auxiliary-window layouts. - **Background-sending a multi-chat composer must reset the composer *before* dispatching the send, not concurrently**: in `NewChatInSessionWidget._send`, creating the replacement untitled chat (`openNewChatInSession({ forceNew: true })` → `provider.createNewChat`) and the fire-and-forget background `sendRequest` both reach into shared chat-session state (`acquireOrLoadSession` / `getOrCreateChatSession`) for chats in the **same group**. Running them concurrently (send first, reset second) raced and left the sent chat stuck spinning with its message never dispatched, plus a second empty "New Chat" tab. Fully `await` the composer reset first, then fire the background send so it runs on its own. - **Chat tab order belongs in the renderer, not the providers**: providers report chats in unstable orders — the agent host re-sorts its `state.chats` catalog when a chat finishes a turn (moving the just-completed one last). Don't try to fix a "tab jumps to the end on completion" bug inside one provider (it won't cover the others). Instead keep the provider's order in the renderer's rebuild autorun (`chatCompositeBar.ts`) and only move in-composer `Untitled` chats to the end. +- **A new chat must report `SessionStatus.Untitled` until its first request is sent, regardless of how the provider creates it**: `sessionView.ts` only shows the new-chat composer (which owns the Alt+Enter background-send handler) when `activeChat.status === Untitled`. The agent host commits a new peer chat *eagerly*, so its host status is `Completed` — surfacing the standard chat widget and breaking background send. Gate the chat's presented status on a provider-side `isNew` flag (`AdditionalChat.markNew`/`markSent`, set in `createNewChat` and cleared in `sendRequest`'s committed-chat branch), not on the host-reported status. ## Capturing Feedback (meta-rule) diff --git a/src/vs/sessions/SESSIONS.md b/src/vs/sessions/SESSIONS.md index c1fb75eab63..0338bf9110b 100644 --- a/src/vs/sessions/SESSIONS.md +++ b/src/vs/sessions/SESSIONS.md @@ -203,7 +203,11 @@ Sessions produce file changes organized into **`ISessionChangeset`** groups — ``` Follow-up messages to an existing chat go through `SessionsManagementService.sendRequest(session, chat, options)`. The view makes -the sent chat the active chat by reacting to the send events. +the sent chat the active chat by reacting to the send events. When +`options.background` is set, the send is **fire-and-forget** and skips the +`onWillSendRequest` notification, so the view's send-follow never navigates the +visible slot into the sent chat — see *Adding a Chat to an Existing Session* +below. Explicit user-initiated "new session" gestures (Ctrl/Cmd+N, the **New** button, the mobile titlebar "+" button, and the sessions quick picker's "New Session" @@ -230,10 +234,10 @@ longer referenced by `_pendingNewSession`. `background` lives on the management-layer `ISendRequestOptions` (which extends the provider's send-request options). Providers do not interpret the flag; it is -purely a management/UI concern. In the new-session composer the gesture is -**Alt+Enter** (or **Alt-click** the Send button); plain Enter / click sends in -the foreground. The background gesture is only offered for the new-session -composer, not when sending a new chat within an existing session. +purely a management/UI concern. The gesture is **Alt+Enter** (or **Alt-click** +the Send button); plain Enter / click sends in the foreground. It is offered both +by the new-session composer and by the new-chat-in-session composer (see *Adding +a Chat to an Existing Session* below). For callers outside the new-session composer, `createAndSendNewChatRequest(folderUri, options, createOptions?)` creates a fresh @@ -268,6 +272,36 @@ config). For the local agent host provider this is enabled for the → Returns the new IChat ``` +The **new-chat-in-session composer** (`NewChatInSessionWidget`) is shown when the +active chat is `Untitled` (`openNewChatInSession` creates/reuses an untitled chat +and makes it active). Sending from it calls +`sendRequest(session, untitledChat, options)`. Plain Enter / click sends in the +**foreground** (the view follows the send and navigates into the now-running +chat). **Alt+Enter** / **Alt-click** sends in the **background**: the widget first +resets the composer to a fresh untitled chat via +`openNewChatInSession(session, { forceNew: true })`, then the management service +runs the send fire-and-forget without firing `onWillSendRequest` (so the view's +send-follow never navigates into it). `forceNew` skips the reuse-untitled lookup +so a genuinely new chat is created rather than re-binding the composer to the +chat being sent. The user stays in the composer to start another parallel +conversation while the sent chat appears in the session's chat list once it +commits. + +The reset is sequenced **before** the send on purpose. Creating the replacement +chat (`provider.createNewChat`) and dispatching the send both reach into shared +chat-session state (`acquireOrLoadSession` / `getOrCreateChatSession`) for chats +in the **same group**. Running them concurrently raced and left the sent chat +stuck spinning with its message never dispatched. Fully awaiting the composer +reset before firing the background send keeps the send running on its own. + +Tab order in the chat composite bar is **stabilised by the renderer**, not by +the providers. The rebuild autorun (in `browser/parts/chatCompositeBar.ts`) +keeps each provider's reported chat order but moves any in-composer `Untitled` +chat to the end. This is provider-agnostic on purpose: the agent host re-sorts +its `state.chats` catalog when a chat finishes a turn (moving the just-completed +chat to the end) — pinning the untitled composer chat last keeps a +just-completed background chat from visibly jumping past it in the tab strip. + On the host, `AgentHostStateManager` keeps an authoritative multi-chat catalog per session: `addChat`/`removeChat` create/delete a per-chat `ChatState` and dispatch `SessionChatAdded`/`SessionChatRemoved`; the default chat (whose diff --git a/src/vs/sessions/browser/parts/chatCompositeBar.ts b/src/vs/sessions/browser/parts/chatCompositeBar.ts index b8317206aec..0917e58aedc 100644 --- a/src/vs/sessions/browser/parts/chatCompositeBar.ts +++ b/src/vs/sessions/browser/parts/chatCompositeBar.ts @@ -165,7 +165,17 @@ export class ChatCompositeBar extends Disposable { const mainChat = session.mainChat.read(reader); const activeChatUri = session.activeChat.read(reader)?.resource.toString() ?? ''; const mainChatUri = mainChat.resource.toString(); - this._rebuildTabs(chats, activeChatUri, mainChatUri); + // Keep the provider's order, but move untitled (in-composer) chats + // to the end so a just-completed background chat never jumps last. + // Partition so each chat's status is read exactly once (tracked) and + // relative order is preserved by construction. + const committed: IChat[] = []; + const untitled: IChat[] = []; + for (const chat of chats) { + (chat.status.read(reader) === SessionStatus.Untitled ? untitled : committed).push(chat); + } + const orderedChats = untitled.length === 0 ? chats : [...committed, ...untitled]; + this._rebuildTabs(orderedChats, activeChatUri, mainChatUri); if (shown) { return; diff --git a/src/vs/sessions/contrib/chat/browser/newChatInSessionWidget.ts b/src/vs/sessions/contrib/chat/browser/newChatInSessionWidget.ts index a149295ab6a..a675ee2f858 100644 --- a/src/vs/sessions/contrib/chat/browser/newChatInSessionWidget.ts +++ b/src/vs/sessions/contrib/chat/browser/newChatInSessionWidget.ts @@ -77,12 +77,13 @@ export class NewChatInSessionWidget extends Disposable { this._newChatInput = this._register(this.instantiationService.createInstance(NewChatInputWidget, { session: this._session, getContextFolderUri: () => this._getContextFolderUri(), - sendRequest: async ({ query, attachments }) => this._send(query, attachments), + sendRequest: async ({ query, attachments, background }) => this._send(query, attachments, background), canSendRequest, loading, historyKey: constObservable(undefined), // no persisted history for the new-chat-in-session view minEditorHeight: 64, placeholder: localize('newChatInSessionPlaceholder', 'Ask a follow-up question or start a new topic within this session...'), + supportsBackground: true, })); } @@ -155,14 +156,21 @@ export class NewChatInSessionWidget extends Disposable { // --- Send --- - private async _send(query: string, attachedContext?: IChatRequestVariableEntry[]): Promise { + private async _send(query: string, attachedContext?: IChatRequestVariableEntry[], background?: boolean): Promise { const activeSession = this._session.get(); if (!activeSession) { return; } const activeChat = activeSession.activeChat.get(); try { - await this.sessionsManagementService.sendRequest(activeSession, activeChat, { query, attachedContext }); + // Reset the composer before dispatching the send: both touch shared + // chat-session state for chats in the same group, and running them + // concurrently races and leaves the sent chat stuck spinning. + if (background) { + await this.sessionsService.openNewChatInSession(activeSession, { forceNew: true }); + } + + await this.sessionsManagementService.sendRequest(activeSession, activeChat, { query, attachedContext, background }); } catch (e) { this.logService.error('Failed to send secondary chat request:', e); } diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index cfcb6ffd6f9..2bcf61c3ae4 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -167,8 +167,9 @@ class AdditionalChat extends Disposable { private readonly _modelId: ISettableObservable; private readonly _description: ISettableObservable; private readonly _lastTurnEnd: ISettableObservable; + private readonly _isNew: ISettableObservable; - constructor(resource: URI, summary: ChatSummary) { + constructor(resource: URI, summary: ChatSummary, isNew: boolean = false) { super(); const modifiedAt = summary.modifiedAt ? new Date(summary.modifiedAt) : new Date(); this._title = observableValue('chatTitle', summary.title || localize('newChatTab', "New Chat")); @@ -177,12 +178,13 @@ class AdditionalChat extends Disposable { this._modelId = observableValue('chatModelId', summary.model ? `${resource.scheme}:${summary.model.id}` : undefined); this._description = observableValue('chatDescription', summary.activity ? new MarkdownString().appendText(summary.activity) : undefined); this._lastTurnEnd = observableValue('chatLastTurnEnd', modifiedAt); + this._isNew = observableValue('chatIsNew', isNew); this.chat = { resource, createdAt: modifiedAt, title: this._title, updatedAt: this._updatedAt, - status: this._status, + status: derived(reader => this._isNew.read(reader) ? SessionStatus.Untitled : this._status.read(reader)), changes: constObservable([]), checkpoints: observableValue(this, undefined), modelId: this._modelId, @@ -210,6 +212,16 @@ class AdditionalChat extends Disposable { setTitle(title: string): void { this._title.set(title || localize('newChatTab', "New Chat"), undefined); } + + /** Present as `Untitled` until the first request is sent so the view shows the composer. */ + markNew(): void { + this._isNew.set(true, undefined); + } + + /** Clear the `new` presentation after the first request is sent. */ + markSent(): void { + this._isNew.set(false, undefined); + } } /** @@ -265,6 +277,8 @@ export class AgentHostSessionAdapter extends Disposable implements ISession { private readonly _chatsObs: ISettableObservable; /** Additional (non-default) peer chats keyed by chatId. */ private readonly _additionalChats = this._register(new DisposableMap()); + /** Chat ids that have not yet sent their first request (presented as `Untitled`). */ + private readonly _newChatIds = new Set(); private readonly _rawId: string; private readonly _resourceScheme: string; @@ -558,7 +572,19 @@ export class AgentHostSessionAdapter extends Disposable implements ISession { private _createAdditionalChat(chatId: string, summary: ChatSummary): AdditionalChat { const resource = URI.from({ scheme: this._resourceScheme, path: `/${this._rawId}`, fragment: chatId }); - return new AdditionalChat(resource, summary); + return new AdditionalChat(resource, summary, this._newChatIds.has(chatId)); + } + + /** Mark a peer chat new so it shows as `Untitled` until its first request. */ + markChatAsNew(chatId: string): void { + this._newChatIds.add(chatId); + this._additionalChats.get(chatId)?.markNew(); + } + + /** Clear the `new` flag after the chat's first request is sent. */ + markChatAsSent(chatId: string): void { + this._newChatIds.delete(chatId); + this._additionalChats.get(chatId)?.markSent(); } /** Optimistically set the default chat tab title (independent of the session title). */ @@ -2468,6 +2494,9 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement const newChatId = generateUuid(); const chatUri = URI.parse(buildChatUri(sessionUri, newChatId)); + // Show as `Untitled` until the first request; the host commits it below. + cached.markChatAsNew(newChatId); + // Keep the session-state subscription alive so the `chatAdded` it emits // flows into `_applyChatCatalogFromState` and updates `cached.chats`. this._keepSessionStateAlive(cached.sessionId); @@ -2486,10 +2515,80 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement async sendRequest(chatId: string, chatResource: URI, options: ISendRequestOptions): Promise { const newSession = this._getNewSession(chatId); - if (!newSession) { - throw new Error(`Session '${chatId}' not found or not a new session`); + if (newSession) { + return this._sendNewSessionRequest(newSession, chatId, chatResource, options); + } + return this._sendCommittedChatRequest(chatId, chatResource, options); + } + + /** Send the first request for an already-committed peer chat, then clear its `new` flag. */ + private async _sendCommittedChatRequest(chatId: string, chatResource: URI, options: ISendRequestOptions): Promise { + const rawId = this._rawIdFromChatId(chatId); + const cached = rawId ? this._sessionCache.get(rawId) : undefined; + if (!cached) { + throw new Error(`Session '${chatId}' not found`); } + const { query, attachedContext } = options; + const sessionType = chatResource.scheme; + const contribution = this._chatSessionsService.getChatSessionContribution(sessionType); + + const selectedModelId = cached.modelId.get(); + const selectedAgentUri = cached.mode.get()?.id; + + const sendOptions: IChatSendRequestOptions = { + location: ChatAgentLocation.Chat, + userSelectedModelId: selectedModelId, + modeInfo: selectedAgentUri ? { + kind: ChatModeKind.Agent, + isBuiltin: false, + modeInstructions: { + uri: URI.parse(selectedAgentUri), + name: '', + content: '', + toolReferences: [], + }, + telemetryModeId: 'custom', + applyCodeBlockSuggestionId: undefined, + permissionLevel: undefined, + } : { + kind: ChatModeKind.Agent, + isBuiltin: true, + modeInstructions: undefined, + telemetryModeId: 'agent', + applyCodeBlockSuggestionId: undefined, + permissionLevel: undefined, + }, + agentIdSilent: contribution?.type, + attachedContext, + }; + + const modelRef = await this._chatService.acquireOrLoadSession(chatResource, ChatAgentLocation.Chat, CancellationToken.None); + if (modelRef) { + if (selectedModelId) { + const languageModel = this._languageModelsService.lookupLanguageModel(selectedModelId); + if (languageModel) { + modelRef.object.inputModel.setState({ selectedModel: { identifier: selectedModelId, metadata: languageModel } }); + } + } + if (selectedAgentUri) { + modelRef.object.inputModel.setState({ mode: { id: selectedAgentUri, kind: ChatModeKind.Agent } }); + } + modelRef.dispose(); + } + + const result = await this._chatService.sendRequest(chatResource, query, sendOptions); + if (result.kind === 'rejected') { + throw new Error(`[${this.id}] sendRequest rejected: ${result.reason}`); + } + + // First request sent: revert to the host-reported status. + cached.markChatAsSent(chatResource.fragment); + + return cached; + } + + private async _sendNewSessionRequest(newSession: NewSession, chatId: string, chatResource: URI, options: ISendRequestOptions): Promise { const connection = this.connection; if (!connection) { throw new Error(this._notConnectedSendErrorMessage()); diff --git a/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts index 448dba16170..32685892060 100644 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts @@ -35,6 +35,7 @@ import { IActiveSession } from '../../../../../services/sessions/common/sessions import { ISessionsService } from '../../../../../services/sessions/browser/sessionsService.js'; import { IAgentHostActiveClientService } from '../../../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentHostActiveClientService.js'; import { LocalAgentHostSessionsProvider } from '../../browser/localAgentHostSessionsProvider.js'; +import { AgentHostSessionAdapter } from '../../browser/baseAgentHostSessionsProvider.js'; import { CHANGESET_UPDATE_THROTTLE_MS } from '../../browser/agentHostChangesetConstants.js'; import { ILabelService } from '../../../../../../platform/label/common/label.js'; import { ILogService, NullLogService } from '../../../../../../platform/log/common/log.js'; @@ -1809,6 +1810,32 @@ suite('LocalAgentHostSessionsProvider', () => { }); }); + test('a new peer chat is presented as Untitled until its first request is sent', () => { + const provider = createProvider(disposables, agentHost); + const session = setupMultiChatSession(provider, 'multi-new'); + const sessionUri = AgentSession.uri('copilotcli', 'multi-new').toString(); + const defaultChat = buildDefaultChatUri(sessionUri); + const peerChat = buildChatUri(sessionUri, 'peer-1'); + + // Host commits the peer chat eagerly (Completed); mark it new first. + (session as AgentHostSessionAdapter).markChatAsNew('peer-1'); + agentHost.setSessionState('multi-new', 'copilotcli', makeState('multi-new', [ + makeChatSummary(defaultChat, ''), + makeChatSummary(peerChat, 'Peer'), + ], { defaultChat })); + + const peer = () => session.chats.get().find(c => c.resource.fragment === 'peer-1'); + const whileNew = peer()!.status.get(); + + (session as AgentHostSessionAdapter).markChatAsSent('peer-1'); + const afterSent = peer()!.status.get(); + + assert.deepStrictEqual({ whileNew, afterSent }, { + whileNew: SessionStatus.Untitled, + afterSent: SessionStatus.Completed, + }); + }); + test('deleteChat prompts for confirmation and disposes the peer chat when confirmed', () => runWithFakedTimers({ useFakeTimers: true }, async () => { const provider = createProvider(disposables, agentHost, undefined, { confirmDelete: true }); const session = setupMultiChatSession(provider, 'multi-del'); @@ -2156,7 +2183,7 @@ suite('LocalAgentHostSessionsProvider', () => { const provider = createProvider(disposables, agentHost); await assert.rejects( () => provider.sendRequest('nonexistent', URI.parse('untitled:chat'), { query: 'test' }), - /not found or not a new session/, + /not found/, ); }); diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts index 47acddd7d16..7b5cb87a485 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts @@ -932,7 +932,7 @@ suite('RemoteAgentHostSessionsProvider', () => { const provider = createProvider(disposables, connection); await assert.rejects( () => provider.sendRequest('nonexistent', URI.parse('untitled:chat'), { query: 'test' }), - /not found or not a new session/, + /not found/, ); }); diff --git a/src/vs/sessions/services/sessions/browser/sessionsManagementService.ts b/src/vs/sessions/services/sessions/browser/sessionsManagementService.ts index 4043d7a0317..d573f2924dd 100644 --- a/src/vs/sessions/services/sessions/browser/sessionsManagementService.ts +++ b/src/vs/sessions/services/sessions/browser/sessionsManagementService.ts @@ -12,7 +12,7 @@ import { IChatService } from '../../../../workbench/contrib/chat/common/chatServ import { ChatAgentLocation } from '../../../../workbench/contrib/chat/common/constants.js'; import { IChatWidgetHistoryService } from '../../../../workbench/contrib/chat/common/widget/chatWidgetHistoryService.js'; import { IUriIdentityService } from '../../../../platform/uriIdentity/common/uriIdentity.js'; -import { ICreateNewSessionOptions, IProviderSessionType, ISendRequestOptions, ISendRequestSentEvent, ISessionsChangeEvent, ISessionsManagementService } from '../common/sessionsManagement.js'; +import { ICreateNewChatInSessionOptions, ICreateNewSessionOptions, IProviderSessionType, ISendRequestOptions, ISendRequestSentEvent, ISessionsChangeEvent, ISessionsManagementService } from '../common/sessionsManagement.js'; import { ISessionsProvidersChangeEvent, ISessionsProvidersService } from './sessionsProvidersService.js'; import { ISessionChangeEvent, ISessionsProvider } from '../common/sessionsProvider.js'; import { IChat, ISession, ISessionWorkspace, SessionStatus, ISessionType } from '../common/session.js'; @@ -318,15 +318,22 @@ export class SessionsManagementService extends Disposable implements ISessionsMa return session; } - async createNewChatInSession(session: ISession): Promise { + async createNewChatInSession(session: ISession, options?: ICreateNewChatInSessionOptions): Promise { const provider = this._getProvider(session); if (!provider) { this.logService.warn(`[SessionsManagement] createNewChatInSession: provider '${session.providerId}' not found`); return undefined; } - // Reuse an existing untitled chat if one exists, otherwise create a new one. - const existingUntitled = session.chats.get().find(c => c.status.get() === SessionStatus.Untitled); - return existingUntitled ?? await provider.createNewChat(session.sessionId); + // `forceNew` skips reuse so callers can reset the composer right after + // sending a chat (which may still transiently report `Untitled`). + if (!options?.forceNew) { + const existingUntitled = session.chats.get().find(c => c.status.get() === SessionStatus.Untitled); + if (existingUntitled) { + return existingUntitled; + } + } + const created = await provider.createNewChat(session.sessionId); + return created; } async sendNewChatRequest(session: ISession, options: ISendRequestOptions): Promise { @@ -342,11 +349,9 @@ export class SessionsManagementService extends Disposable implements ISessionsMa } if (options.background) { - // Run the send fire-and-forget so the composer can reset and reseed - // immediately while the request commits. If the commit fails, the - // graduating draft is stranded (no longer the current new session), - // so dispose it through its provider to release the eager backend - // session. Safe no-op if the provider already graduated/removed it. + // Fire-and-forget so the composer can reset immediately. On commit + // failure the graduating draft is stranded, so dispose it through + // its provider (no-op if already graduated/removed). this._sendNewChatRequestInBackground(provider, session, options).catch(e => { provider.deleteNewSession(session.sessionId); this.logService.error('[SessionsManagement] Failed to send background request:', e); @@ -454,17 +459,27 @@ export class SessionsManagementService extends Disposable implements ISessionsMa // so dispose it to release its eager backend session. this.discardNewSession(); + const provider = this._getProvider(session); + if (!provider) { + throw new Error(`Sessions provider '${session.providerId}' not found`); + } + + if (options.background) { + // Fire-and-forget so the composer can reset immediately. Unlike the + // foreground path this skips `_onWillSendRequest` so the view's + // send-follow does not navigate the visible slot into the sent chat. + this._sendRequestInBackground(provider, session, chat, options).catch(e => { + this.logService.error('[SessionsManagement] Failed to send background request:', e); + }); + return; + } + // Notify listeners that a send is starting. Listeners (e.g., telemetry) // can use this to prewarm caches whose result is consumed when // `onDidSendRequest` fires below. The view service observes the will/did // send pair to keep the sent chat active in the visible slot. this._onWillSendRequest.fire(session); - const provider = this._getProvider(session); - if (!provider) { - throw new Error(`Sessions provider '${session.providerId}' not found`); - } - const chatResourceKey = chat.resource.toString(); this._pendingSendChatResources.add(chatResourceKey); let updatedSession: ISession; @@ -480,6 +495,28 @@ export class SessionsManagementService extends Disposable implements ISessionsMa this._onDidSendRequest.fire({ session: updatedSession, chat, isNewSession: false, isNewChat: true, options }); } + /** + * Send a request for an existing chat in the background: commit the send via + * the provider and—on success—fire {@link _onDidSendRequest}. Unlike the + * foreground {@link sendRequest} path this does not fire + * {@link _onWillSendRequest}, so the view's send-follow never navigates the + * visible slot into the sent chat. Errors are propagated to the caller. + */ + private async _sendRequestInBackground(provider: ISessionsProvider, session: ISession, chat: IChat, options: ISendRequestOptions): Promise { + const chatResourceKey = chat.resource.toString(); + this._pendingSendChatResources.add(chatResourceKey); + let updatedSession: ISession; + try { + updatedSession = await provider.sendRequest(session.sessionId, chat.resource, options); + } finally { + this._pendingSendChatResources.delete(chatResourceKey); + } + if (this._store.isDisposed) { + return; + } + this._onDidSendRequest.fire({ session: updatedSession, chat, isNewSession: false, isNewChat: true, options }); + } + // -- Session Actions -- private _getProvider(session: ISession): ISessionsProvider | undefined { diff --git a/src/vs/sessions/services/sessions/browser/sessionsService.ts b/src/vs/sessions/services/sessions/browser/sessionsService.ts index 628b3211f8e..f917ebd4fe6 100644 --- a/src/vs/sessions/services/sessions/browser/sessionsService.ts +++ b/src/vs/sessions/services/sessions/browser/sessionsService.ts @@ -16,7 +16,7 @@ import { ILogService } from '../../../../platform/log/common/log.js'; import { IStorageService, StorageScope, StorageTarget } from '../../../../platform/storage/common/storage.js'; import { IUriIdentityService } from '../../../../platform/uriIdentity/common/uriIdentity.js'; import { IChat, ISession, SessionStatus } from '../common/session.js'; -import { IActiveSession, ICreateNewSessionOptions, IRecentlyOpenedSessions, ISessionsChangeEvent, ISessionsManagementService, IToggleSessionStickinessEvent, ActiveSessionSupportsMultiChatContext } from '../common/sessionsManagement.js'; +import { IActiveSession, ICreateNewChatInSessionOptions, ICreateNewSessionOptions, IRecentlyOpenedSessions, ISessionsChangeEvent, ISessionsManagementService, IToggleSessionStickinessEvent, ActiveSessionSupportsMultiChatContext } from '../common/sessionsManagement.js'; import { ISessionsProvidersService } from './sessionsProvidersService.js'; import { SessionsNavigation } from './sessionNavigation.js'; import { SessionsRecencyHistory } from './sessionsRecencyHistory.js'; @@ -150,9 +150,11 @@ export interface ISessionsService { /** * Switch to the new-chat-in-session view. * Adds a new chat to the session via the provider, makes it the active chat, - * and shows a rich input for composing a message. + * and shows a rich input for composing a message. Pass + * {@link ICreateNewChatInSessionOptions.forceNew} to always create a fresh + * chat (e.g. when resetting the composer right after a background send). */ - openNewChatInSession(session: ISession): Promise; + openNewChatInSession(session: ISession, options?: ICreateNewChatInSessionOptions): Promise; /** * Discard the pending new session and clear the active session, returning @@ -669,10 +671,10 @@ export class SessionsService extends Disposable implements ISessionsService { return newSession ?? undefined; } - async openNewChatInSession(session: ISession): Promise { + async openNewChatInSession(session: ISession, options?: ICreateNewChatInSessionOptions): Promise { this._cancelRestore(); this._startOpenSession(); - const chat = await this.sessionsManagementService.createNewChatInSession(session); + const chat = await this.sessionsManagementService.createNewChatInSession(session, options); if (!chat) { return; } diff --git a/src/vs/sessions/services/sessions/common/sessionsManagement.ts b/src/vs/sessions/services/sessions/common/sessionsManagement.ts index fb889e5b333..34a4cafd631 100644 --- a/src/vs/sessions/services/sessions/common/sessionsManagement.ts +++ b/src/vs/sessions/services/sessions/common/sessionsManagement.ts @@ -20,9 +20,11 @@ import { ISendRequestOptions as ISessionsProviderSendRequestOptions } from './se */ export interface ISendRequestOptions extends ISessionsProviderSendRequestOptions { /** - * Start the session without navigating into it: the new-session composer - * stays put and the started session shows up in the sessions list. Only - * honored by {@link ISessionsManagementService.sendNewChatRequest}. + * Start the session without navigating into it: the composer stays put and + * the started session/chat shows up in the sessions list. Honored by + * {@link ISessionsManagementService.sendNewChatRequest} (new sessions) and + * {@link ISessionsManagementService.sendRequest} (a new chat within an + * existing session). */ readonly background?: boolean; } @@ -58,6 +60,18 @@ export interface ICreateNewSessionOptions { export const ActiveSessionSupportsMultiChatContext = new RawContextKey('activeSessionSupportsMultiChat', false, localize('activeSessionSupportsMultiChat', "Whether the active session supports multiple chats")); +/** + * Options for {@link ISessionsManagementService.createNewChatInSession}. + */ +export interface ICreateNewChatInSessionOptions { + /** + * Always create a fresh chat instead of reusing an existing untitled one. + * Used to reset the composer right after a background send, where the + * just-sent chat may still transiently report `Untitled`. + */ + readonly forceNew?: boolean; +} + /** * Event fired when sessions change within a provider. */ @@ -225,10 +239,12 @@ export interface ISessionsManagementService { /** * Create (or reuse an existing untitled) chat in the given session via its - * provider so it can be shown as the new-chat-in-session view. Returns the - * chat, or `undefined` when the provider could not be resolved. + * provider so it can be shown as the new-chat-in-session view. Pass + * {@link ICreateNewChatInSessionOptions.forceNew} to always create a fresh + * chat. Returns the chat, or `undefined` when the provider could not be + * resolved. */ - createNewChatInSession(session: ISession): Promise; + createNewChatInSession(session: ISession, options?: ICreateNewChatInSessionOptions): Promise; /** * Discard the in-progress new session, disposing it through its provider to @@ -266,6 +282,10 @@ export interface ISessionsManagementService { /** * Send a request for an existing chat within a session. + * + * When {@link ISendRequestOptions.background} is set, the send runs + * fire-and-forget and the view is not navigated into the sent chat, so the + * caller can keep composing (e.g. start a parallel conversation). */ sendRequest(session: ISession, chat: IChat, options: ISendRequestOptions): Promise; diff --git a/src/vs/sessions/services/sessions/test/browser/sessionsManagementService.test.ts b/src/vs/sessions/services/sessions/test/browser/sessionsManagementService.test.ts index 27703941243..53430ddc0da 100644 --- a/src/vs/sessions/services/sessions/test/browser/sessionsManagementService.test.ts +++ b/src/vs/sessions/services/sessions/test/browser/sessionsManagementService.test.ts @@ -759,6 +759,46 @@ suite('SessionsManagementService', () => { completeSendRequest?.(); }); + test('sendRequest with background is fire-and-forget and does not fire onWillSendRequest', async () => { + const chat: IChat = { ...stubChat, resource: URI.parse('test:///chat'), status: constObservable(SessionStatus.Untitled) }; + const session = stubSession({ + sessionId: 's1', + providerId: 'test', + chats: constObservable([chat]), + mainChat: constObservable(chat), + }); + let completeSendRequest: (() => void) | undefined; + let sentChatResource: URI | undefined; + const provider = new class extends TestSessionsProvider { + override async sendRequest(_sessionId: string, chatResource: URI, _options: ISendRequestOptions): Promise { + sentChatResource = chatResource; + await new Promise(resolve => { + completeSendRequest = resolve; + }); + return session; + } + }(session); + const { service } = createSessionsManagementService(session, disposables, provider); + + let willSendCount = 0; + disposables.add(service.onWillSendRequest(() => willSendCount++)); + + // The background send is fire-and-forget (it resolves before the + // provider commits) and never fires `onWillSendRequest`, so the view's + // send-follow cannot navigate into the sent chat. + await service.sendRequest(session, chat, { query: 'hi', background: true }); + + assert.deepStrictEqual({ + sentChatResource: sentChatResource?.toString(), + willSendCount, + }, { + sentChatResource: chat.resource.toString(), + willSendCount: 0, + }); + + completeSendRequest?.(); + }); + test('createAndSendNewChatRequest sends without changing the active view', async () => { const chat: IChat = { ...stubChat, resource: URI.parse('test:///chat') }; const session = stubSession({ @@ -1011,6 +1051,31 @@ suite('SessionsManagementService', () => { }); }); + test('forceNew creates a fresh chat even when an untitled one exists', async () => { + const untitledChat: IChat = { ...stubChat, resource: URI.parse('test:///untitled'), status: constObservable(SessionStatus.Untitled) }; + const createdChat: IChat = { ...stubChat, resource: URI.parse('test:///created') }; + const session = stubSession({ sessionId: 'force-new', providerId: 'test', chats: constObservable([untitledChat]) }); + let createNewChatCalls = 0; + const provider = new class extends TestSessionsProvider { + constructor() { super(session); } + override async createNewChat(): Promise { + createNewChatCalls++; + return createdChat; + } + }; + const { service } = createSessionsManagementService(session, disposables, provider); + + const result = await service.createNewChatInSession(session, { forceNew: true }); + + assert.deepStrictEqual({ + result: result?.resource.toString(), + createNewChatCalls, + }, { + result: createdChat.resource.toString(), + createNewChatCalls: 1, + }); + }); + test('returns undefined when the provider is not found', async () => { const session = stubSession({ sessionId: 'orphan', providerId: 'missing-provider' }); const provider = new TestSessionsProvider(stubSession({ sessionId: 'other', providerId: 'test' }));