mirror of
https://github.com/microsoft/vscode.git
synced 2026-07-10 00:35:30 +01:00
sessions: persist main chat rename across restore for agent host (#323785)
* sessions: persist main chat rename across restore for agent host For agent host multichat sessions, renaming the default/main chat sets an independent title persisted on the host as customChatTitle:<defaultChatUri>. However restoreSession always re-seeded the default chat with an empty title (inherit the session title), so after a process restart or idle eviction the main chat tab reverted to the session title. Peer chats were already restored via _restorePeerChats. Read the persisted default-chat title on restore and seed it back through restoreSession/_ensureDefaultChat, mirroring the peer-chat restore path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * sessions: clarify _readPersistedChatTitle doc comment Addresses CCR feedback: the helper now reads the default chat's title too, not only peer chats. 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:
committed by
GitHub
parent
6ca57e4232
commit
ece376a4db
@@ -61,6 +61,8 @@ Then read the relevant spec for the area you are changing (see table below). If
|
||||
|
||||
Whenever the user flags a wrong pattern, rejects an approach, or gives design/rules feedback, **automatically add it** as a concise pitfall/learning to this `Common Pitfalls` section (or the most relevant spec doc) in the same change — without being asked again. Keep each entry 1–3 sentences: the anti-pattern, why it is wrong, and the preferred pattern.
|
||||
|
||||
- **Default/main chat title persistence differs per provider — don't unify them**: For the **agent host**, the default (main) chat title is independent of the session title (`AgentHostSessionAdapter._defaultChatTitleOverride`, persisted on the host as `customChatTitle:<defaultChatUri>`); it must be seeded back on restore via `restoreSession`/`_ensureDefaultChat` (mirroring `_restorePeerChats`), or it reverts to the session title after a process restart / idle eviction. For the **local chat sessions provider** (`localChatSessionsProvider`), the primary chat and the session intentionally **share one `_title` observable**, so renaming the first/main chat updates the session title live — this is by design; do not "fix" it to be independent. Only additional (non-primary) local chats have their own title.
|
||||
|
||||
## Validating Changes
|
||||
|
||||
You **must** run these checks before declaring work complete:
|
||||
|
||||
@@ -598,7 +598,7 @@ export class AgentHostStateManager extends Disposable {
|
||||
* notification because the session is already known to clients via
|
||||
* `listSessions`.
|
||||
*/
|
||||
restoreSession(summary: SessionSummary, turns: Turn[], options?: { readonly draft?: Message }): SessionState {
|
||||
restoreSession(summary: SessionSummary, turns: Turn[], options?: { readonly draft?: Message; readonly defaultChatTitle?: string }): SessionState {
|
||||
const key = summary.resource;
|
||||
const existing = this._sessionStates.get(key);
|
||||
if (existing) {
|
||||
@@ -611,7 +611,7 @@ export class AgentHostStateManager extends Disposable {
|
||||
lifecycle: SessionLifecycle.Ready,
|
||||
};
|
||||
this._sessionStates.set(key, this._newEntry(state, summary));
|
||||
this._ensureDefaultChat(key, summary, turns, options?.draft);
|
||||
this._ensureDefaultChat(key, summary, turns, options?.draft, options?.defaultChatTitle);
|
||||
this._summaryNotifier.announce(key, summary);
|
||||
|
||||
this._logService.trace(`[AgentHostStateManager] Restored session: ${key} (${turns.length} turns)`);
|
||||
@@ -631,13 +631,11 @@ export class AgentHostStateManager extends Disposable {
|
||||
* at creation/restore time, so the snapshot a client later receives on
|
||||
* subscribe already reflects the default chat.
|
||||
*/
|
||||
private _ensureDefaultChat(sessionKey: string, summary: SessionSummary, turns?: Turn[], draft?: Message): void {
|
||||
private _ensureDefaultChat(sessionKey: string, summary: SessionSummary, turns?: Turn[], draft?: Message, defaultChatTitle?: string): void {
|
||||
const chatUri = buildDefaultChatUri(sessionKey);
|
||||
// The default chat starts with an empty title so it inherits the session
|
||||
// title for display. It only gets its own title when renamed independently
|
||||
// (via a per-chat `SessionChatUpdated`). This keeps the session title and
|
||||
// the default chat tab title independent.
|
||||
const chatSummary: ChatSummary = { ...createDefaultChatSummary(summary, chatUri), title: '' };
|
||||
// Empty title means "inherit the session title"; a persisted independent
|
||||
// rename (`defaultChatTitle`) is seeded back here so it survives restore.
|
||||
const chatSummary: ChatSummary = { ...createDefaultChatSummary(summary, chatUri), title: defaultChatTitle ?? '' };
|
||||
this._chatStates.set(chatUri, { ...createChatState(chatSummary), turns: turns ?? [], draft });
|
||||
const entry = this._sessionStates.get(sessionKey);
|
||||
if (entry) {
|
||||
|
||||
@@ -1673,8 +1673,12 @@ export class AgentService extends Disposable implements IAgentService {
|
||||
_meta: sessionMetadata
|
||||
};
|
||||
|
||||
const defaultDraft = await this._getChatDraft(session, URI.parse(buildDefaultChatUri(sessionStr)));
|
||||
this._stateManager.restoreSession(summary, [...turns], { draft: defaultDraft });
|
||||
const defaultChatUri = URI.parse(buildDefaultChatUri(sessionStr));
|
||||
const [defaultDraft, defaultChatTitle] = await Promise.all([
|
||||
this._getChatDraft(session, defaultChatUri),
|
||||
this._readPersistedChatTitle(session, defaultChatUri),
|
||||
]);
|
||||
this._stateManager.restoreSession(summary, [...turns], { draft: defaultDraft, defaultChatTitle });
|
||||
|
||||
const promises: Promise<unknown>[] = [];
|
||||
// Eagerly register subagent child sessions discovered in the event log
|
||||
@@ -1798,7 +1802,7 @@ export class AgentService extends Disposable implements IAgentService {
|
||||
}
|
||||
}
|
||||
|
||||
/** Reads a peer chat's persisted custom title, if any. */
|
||||
/** Reads a chat's persisted custom title (default or peer chat), if any. */
|
||||
private async _readPersistedChatTitle(session: URI, chatUri: URI): Promise<string | undefined> {
|
||||
const ref = await this._sessionDataService.tryOpenDatabase?.(session);
|
||||
if (!ref) {
|
||||
|
||||
@@ -2501,6 +2501,25 @@ suite('AgentService (node dispatcher)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
test('restoreSession restores the default chat\'s independently-renamed title', async () => {
|
||||
const db = new TestSessionDatabase();
|
||||
const localService = disposables.add(new AgentService(new NullLogService(), fileService, createSessionDataService(db), { _serviceBrand: undefined } as IProductService, createNoopGitService()));
|
||||
localService.registerProvider(copilotAgent);
|
||||
await copilotAgent.createSession();
|
||||
const sessionResource = (await copilotAgent.listSessions())[0].session;
|
||||
copilotAgent.sessionMessages = [];
|
||||
|
||||
// The host persists an independent default-chat rename under this key;
|
||||
// restore must seed it back or the main chat tab reverts to the session title.
|
||||
const defaultChatUri = buildDefaultChatUri(sessionResource.toString());
|
||||
await db.setMetadata(`customChatTitle:${defaultChatUri}`, 'Renamed Default Chat');
|
||||
|
||||
await localService.restoreSession(sessionResource);
|
||||
|
||||
const state = localService.stateManager.getSessionState(sessionResource.toString());
|
||||
assert.strictEqual(state?.chats.find(c => c.resource === defaultChatUri)?.title, 'Renamed Default Chat');
|
||||
});
|
||||
|
||||
test('restoreSession preserves peer chat catalog order regardless of load timing', async () => {
|
||||
class MultiChatAgent extends MockAgent {
|
||||
async createChat(_session: URI, _chat: URI): Promise<void> { }
|
||||
|
||||
@@ -141,7 +141,8 @@ When restoring Copilot SDK history, `mapSessionEvents` best-effort reconstructs
|
||||
## CRUD & Stubbed Operations
|
||||
|
||||
- `archiveSession` / `unarchiveSession` / `deleteSession` — round-trip to the backend. `deleteSessions` is the batch variant (used when multiple sessions are selected): it disposes each backend session and emits a single removal change event. Sessions advertise `capabilities.supportsDelete`, so the shared sessions-list "Delete..." action (contributed by the sessions workbench, gated on `SessionSupportsDeleteContext`) confirms and invokes deletion — there is no provider-specific delete action.
|
||||
- `renameChat` — updates the session title.
|
||||
- `renameChat` — renames a single chat independently of the session title. For an additional peer chat it dispatches `SessionTitleChanged` on that chat's channel; for the default/main chat it dispatches on the default chat channel (`setDefaultChatTitle`). The host persists the new title under `customChatTitle:<chatUri>` and re-applies it on restore — the default chat's title is seeded back through `restoreSession`/`_ensureDefaultChat`, peer chats through `_restorePeerChats` — so an independently-renamed main/peer chat survives a process restart or idle eviction instead of reverting to the session title.
|
||||
- `renameSession` — updates the session-level title.
|
||||
- `deleteChat` — no-op (agent host sessions don't model individually deletable chats).
|
||||
- `forkChat(sessionId, sourceChat, turnId)` — multi-chat only. Mints a peer chat URI and calls `connection.createChat(sessionUri, chatUri, { fork: { source, turnId } })`, where `source` is the backend chat URI (a `chatId` fragment addresses a peer chat, otherwise the session's default chat). The host seeds the new chat with the forked history; the provider waits for it to surface in `cached.chats` and returns it. Routed from the **Fork Conversation** gesture via `ISessionsManagementService.forkChatInSession`; single-chat sessions instead fork into a new session (the workbench `AgentHostSessionHandler.forkSession`).
|
||||
|
||||
|
||||
Reference in New Issue
Block a user