mirror of
https://github.com/microsoft/vscode.git
synced 2026-07-13 15:35:20 +01:00
sessions: support background send for a new chat in an existing session (#322642)
* sessions: support background send for a new chat in an existing session Add background-send (Alt+Enter) support for a new chat created inside an existing multi-chat session, mirroring the new-session composer behavior. - Plumb a `background` flag and `forceNew` option through the new-chat composer, sessions service, and management service so a background send resets the composer before dispatching (avoiding a shared chat-session state race) and runs fire-and-forget without navigating the visible slot. - Keep tab order in the renderer: move in-composer `Untitled` chats to the end so a just-completed background chat never jumps last. - Agent host: present a freshly created peer chat as `Untitled` until its first request is sent (the host commits it eagerly as `Completed`), so the sessions view shows the composer that owns the Alt+Enter handler. Add a committed-chat branch to `sendRequest` and revert the chat to its real status once sent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * sessions: update remote provider not-found assertion Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * sessions: partition chat tabs instead of sort for stable status tracking Address CCR feedback: read each chat status exactly once inside the autorun so dependency tracking is complete, and preserve provider order by partition. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Sandeep Somavarapu
committed by
GitHub
GitHub
parent
c7741d853b
commit
ea3a084767
@@ -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)
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<void> {
|
||||
private async _send(query: string, attachedContext?: IChatRequestVariableEntry[], background?: boolean): Promise<void> {
|
||||
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);
|
||||
}
|
||||
|
||||
+104
-5
@@ -167,8 +167,9 @@ class AdditionalChat extends Disposable {
|
||||
private readonly _modelId: ISettableObservable<string | undefined>;
|
||||
private readonly _description: ISettableObservable<IMarkdownString | undefined>;
|
||||
private readonly _lastTurnEnd: ISettableObservable<Date | undefined>;
|
||||
private readonly _isNew: ISettableObservable<boolean>;
|
||||
|
||||
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<string | undefined>('chatModelId', summary.model ? `${resource.scheme}:${summary.model.id}` : undefined);
|
||||
this._description = observableValue<IMarkdownString | undefined>('chatDescription', summary.activity ? new MarkdownString().appendText(summary.activity) : undefined);
|
||||
this._lastTurnEnd = observableValue<Date | undefined>('chatLastTurnEnd', modifiedAt);
|
||||
this._isNew = observableValue<boolean>('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<readonly IChat[]>;
|
||||
/** Additional (non-default) peer chats keyed by chatId. */
|
||||
private readonly _additionalChats = this._register(new DisposableMap<string, AdditionalChat>());
|
||||
/** Chat ids that have not yet sent their first request (presented as `Untitled`). */
|
||||
private readonly _newChatIds = new Set<string>();
|
||||
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<ISession> {
|
||||
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<ISession> {
|
||||
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<ISession> {
|
||||
const connection = this.connection;
|
||||
if (!connection) {
|
||||
throw new Error(this._notConnectedSendErrorMessage());
|
||||
|
||||
+28
-1
@@ -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<void>({ 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/,
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
+1
-1
@@ -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/,
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
@@ -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<IChat | undefined> {
|
||||
async createNewChatInSession(session: ISession, options?: ICreateNewChatInSessionOptions): Promise<IChat | undefined> {
|
||||
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<void> {
|
||||
@@ -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<void> {
|
||||
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 {
|
||||
|
||||
@@ -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<void>;
|
||||
openNewChatInSession(session: ISession, options?: ICreateNewChatInSessionOptions): Promise<void>;
|
||||
|
||||
/**
|
||||
* 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<void> {
|
||||
async openNewChatInSession(session: ISession, options?: ICreateNewChatInSessionOptions): Promise<void> {
|
||||
this._cancelRestore();
|
||||
this._startOpenSession();
|
||||
const chat = await this.sessionsManagementService.createNewChatInSession(session);
|
||||
const chat = await this.sessionsManagementService.createNewChatInSession(session, options);
|
||||
if (!chat) {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -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<boolean>('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<IChat | undefined>;
|
||||
createNewChatInSession(session: ISession, options?: ICreateNewChatInSessionOptions): Promise<IChat | undefined>;
|
||||
|
||||
/**
|
||||
* 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<void>;
|
||||
|
||||
|
||||
@@ -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<ISession> {
|
||||
sentChatResource = chatResource;
|
||||
await new Promise<void>(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<IChat> {
|
||||
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' }));
|
||||
|
||||
Reference in New Issue
Block a user