From ece376a4dbfd464bf0b756567361e093e6b3fdb0 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Wed, 1 Jul 2026 00:18:39 +0200 Subject: [PATCH] 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:. 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> --- .github/skills/sessions/SKILL.md | 2 ++ .../agentHost/node/agentHostStateManager.ts | 14 ++++++-------- .../platform/agentHost/node/agentService.ts | 10 +++++++--- .../agentHost/test/node/agentService.test.ts | 19 +++++++++++++++++++ .../agentHost/AGENT_HOST_SESSIONS_PROVIDER.md | 3 ++- 5 files changed, 36 insertions(+), 12 deletions(-) diff --git a/.github/skills/sessions/SKILL.md b/.github/skills/sessions/SKILL.md index 7e1c331b836..1f77c316d37 100644 --- a/.github/skills/sessions/SKILL.md +++ b/.github/skills/sessions/SKILL.md @@ -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:`); 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: diff --git a/src/vs/platform/agentHost/node/agentHostStateManager.ts b/src/vs/platform/agentHost/node/agentHostStateManager.ts index 7af898c74f3..d918d70626e 100644 --- a/src/vs/platform/agentHost/node/agentHostStateManager.ts +++ b/src/vs/platform/agentHost/node/agentHostStateManager.ts @@ -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) { diff --git a/src/vs/platform/agentHost/node/agentService.ts b/src/vs/platform/agentHost/node/agentService.ts index 56b6dd10f77..db6fbe2aa38 100644 --- a/src/vs/platform/agentHost/node/agentService.ts +++ b/src/vs/platform/agentHost/node/agentService.ts @@ -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[] = []; // 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 { const ref = await this._sessionDataService.tryOpenDatabase?.(session); if (!ref) { diff --git a/src/vs/platform/agentHost/test/node/agentService.test.ts b/src/vs/platform/agentHost/test/node/agentService.test.ts index 3fc34422647..0ae4cfc837d 100644 --- a/src/vs/platform/agentHost/test/node/agentService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentService.test.ts @@ -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 { } diff --git a/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md b/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md index 7ddd94631df..78789e7154a 100644 --- a/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md +++ b/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md @@ -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:` 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`).