sessions: Support read state with legacy Agent Hosts (#339704)

* sessions: support read state with legacy Agent Hosts Treat missing per-chat read state from pre-0.10 Agent Hosts as unknown, preserve the client's conservative state across refreshes, and translate read updates to the legacy session-level action. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * sessions: fix legacy chat read fallback Update the requested chat optimistically on pre-0.10 hosts and always dispatch retryable session-level read actions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * sessions: use valid protocol version in remote tests Use the generated current AHP version in remote provider mocks so capability checks exercise a valid negotiated SemVer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit 7d2e8e2469)
2026-10-07 18:12:40 +01:00 · 2026-10-05 15:48:19 +02:00
1 parent 05136d8e5a
commit 94f0b81620
@@ -1141,7 +1141,7 @@ class AdditionalChat extends Disposable {
 		}, () => observable.get());
 	}

-	update(summary: ChatSummary): void {
+	update(summary: ChatSummary, supportsChatReadState: boolean = true): void {
 		const modifiedAt = new Date(summary.modifiedAt);
 		transaction(tx => {
 			this._title.set(summary.title || localize('newChatTab', "New Chat"), tx);
@@ -1152,7 +1152,9 @@ class AdditionalChat extends Disposable {
 			this._lastTurnEnd.set(modifiedAt, tx);
 			this._interactivity.set(toChatInteractivity(summary.interactivity), tx);
 			this._isArchived.set(isSessionStatusArchived(summary.status), tx);
-			this._isRead.set(isSessionStatusRead(summary.status), tx);
+			if (supportsChatReadState) {
+				this._isRead.set(isSessionStatusRead(summary.status), tx);
+			}
 			this._setChangesSummary(summary.changes, tx);
 		});
 	}
@@ -1199,8 +1201,8 @@ class AdditionalChat extends Disposable {
 		this._title.set(title || localize('newChatTab', "New Chat"), undefined);
 	}

-	setRead(isRead: boolean): void {
-		this._isRead.set(isRead, undefined);
+	setRead(isRead: boolean, tx?: ITransaction): void {
+		this._isRead.set(isRead, tx);
 	}

 	/** Present as `Untitled` until the first request is sent so the view shows the composer. */
@@ -1366,6 +1368,7 @@ export class AgentHostSessionAdapter extends Disposable implements ISession {
 	private _lastCatalogState: SessionState | undefined;
 	private readonly _chatCatalogCapabilitiesObserver = this._register(new MutableDisposable());
 	private readonly _supportsChatArchive: IObservable<boolean>;
+	private readonly _supportsChatReadState: IObservable<boolean>;
 	private readonly _rawId: string;
 	private readonly _resourceScheme: string;

@@ -1684,6 +1687,12 @@ export class AgentHostSessionAdapter extends Disposable implements ISession {
 			const initializeResult = connection?.initializeResult.read(reader);
 			return !!initializeResult && isActionKnownToVersion({ type: ActionType.ChatIsArchivedChanged, isArchived: false }, initializeResult.protocolVersion);
 		});
+		this._supportsChatReadState = derived(this, reader => {
+			this._options.connectionStatus?.read(reader);
+			const connection = this._options.getConnection();
+			const initializeResult = connection?.initializeResult.read(reader);
+			return !!initializeResult && isActionKnownToVersion({ type: ActionType.ChatIsReadChanged, isRead: true }, initializeResult.protocolVersion);
+		});
 		this.capabilities = derivedOpts<ISessionCapabilities>({ owner: this, equalsFn: structuralEquals }, reader => {
 			const agentCapabilities = this._options.agentCapabilities.read(reader)?.get(this.agentProvider);
 			this._options.connectionStatus?.read(reader);
@@ -1778,11 +1787,14 @@ export class AgentHostSessionAdapter extends Disposable implements ISession {
 			.map(chat => parseChatUri(chat.chat)?.chatId)
 			.filter(chatId => chatId !== undefined);
 		const hasMultipleChats = peerIds.length > 0;
+		const supportsChatReadState = this._supportsChatReadState.get();
 		this._hasMultipleChats.set(hasMultipleChats, tx);
 		if (!hasMultipleChats) {
 			this._defaultChatIsReadOverride.set(undefined, tx);
 		} else if (defaultChat?.isRead !== undefined) {
 			this._defaultChatIsReadOverride.set(defaultChat.isRead, tx);
+		} else if (!supportsChatReadState && this._defaultChatIsReadOverride.get() === undefined) {
+			this._defaultChatIsReadOverride.set(false, tx);
 		}
 		const survivingPeers = new Set(peerIds);
 		const chatOutputResources = new ResourceSet(chats.map(chat => chat.chat));
@@ -1813,7 +1825,7 @@ export class AgentHostSessionAdapter extends Disposable implements ISession {
 					status: withSessionStatusFlag(
 						withSessionStatusFlag(ProtocolSessionStatus.Idle, ProtocolSessionStatus.IsArchived, chat.archived === true),
 						ProtocolSessionStatus.IsRead,
-						chat.isRead ?? true,
+						chat.isRead ?? supportsChatReadState,
 					),
 					origin: chat.origin,
 					interactivity: chat.interactivity,
@@ -1845,6 +1857,10 @@ export class AgentHostSessionAdapter extends Disposable implements ISession {
 		return !arrayEquals(previousChats, nextChats) || sessionReadChanged;
 	}

+	supportsChatReadState(): boolean {
+		return this._supportsChatReadState.get();
+	}
+
 	hasUnreadChat(): boolean {
 		return this._aggregateChatResources.size > 1
 			&& this._chatsObs.get().some(chat => this._aggregateChatResources.has(chat.resource) && !chat.isRead.get());
@@ -1987,7 +2003,12 @@ export class AgentHostSessionAdapter extends Disposable implements ISession {
 		// Multiple chats: the default chat must show its own status, not the
 		// session aggregate which may have been promoted by a running peer chat.
 		this._defaultChatStatusOverride.set(defaultSummary ? mapProtocolStatus(defaultSummary.status) : undefined, undefined);
-		this._defaultChatIsReadOverride.set(defaultSummary ? isSessionStatusRead(defaultSummary.status) : undefined, undefined);
+		const supportsChatReadState = this._supportsChatReadState.get();
+		if (supportsChatReadState) {
+			this._defaultChatIsReadOverride.set(defaultSummary ? isSessionStatusRead(defaultSummary.status) : undefined, undefined);
+		} else if (this._defaultChatIsReadOverride.get() === undefined) {
+			this._defaultChatIsReadOverride.set(false, undefined);
+		}

 		const ordered: IChat[] = [];
 		for (const summary of state.chats) {
@@ -2004,7 +2025,7 @@ export class AgentHostSessionAdapter extends Disposable implements ISession {
 				entry = this._createAdditionalChat(chatId, summary);
 				this._additionalChats.set(chatId, entry);
 			} else {
-				entry.update(summary);
+				entry.update(summary, supportsChatReadState);
 			}
 			ordered.push(entry.chat);
 		}
@@ -2023,11 +2044,11 @@ export class AgentHostSessionAdapter extends Disposable implements ISession {
 		this._markSessionUnreadForUnreadChat();
 	}

-	setDefaultChatRead(isRead: boolean): boolean {
+	setDefaultChatRead(isRead: boolean, tx?: ITransaction): boolean {
 		if (!this._hasMultipleChats.get() || this._defaultChatIsReadOverride.get() === isRead) {
 			return false;
 		}
-		this._defaultChatIsReadOverride.set(isRead, undefined);
+		this._defaultChatIsReadOverride.set(isRead, tx);
 		return true;
 	}

@@ -2090,25 +2111,40 @@ export class AgentHostSessionAdapter extends Disposable implements ISession {
 			: this.resource;
 	}

-	setChatRead(chatResource: URI, isRead: boolean): boolean {
+	setChatRead(chatResource: URI, isRead: boolean, tx?: ITransaction): boolean {
 		if (isEqual(chatResource, this._defaultChat.resource)) {
 			if (this._hasMultipleChats.get()) {
-				this.setDefaultChatRead(isRead);
+				this.setDefaultChatRead(isRead, tx);
 			} else {
-				this.isRead.set(isRead, undefined);
+				this.isRead.set(isRead, tx);
 			}
-			this._markSessionUnreadForUnreadChat();
+			this._markSessionUnreadForUnreadChat(tx);
 			return true;
 		}
 		const chat = chatResource.fragment ? this._additionalChats.get(chatResource.fragment) : undefined;
 		if (!chat) {
 			return false;
 		}
-		chat.setRead(isRead);
-		this._markSessionUnreadForUnreadChat();
+		chat.setRead(isRead, tx);
+		this._markSessionUnreadForUnreadChat(tx);
 		return true;
 	}

+	setLegacySessionReadState(isRead: boolean, tx?: ITransaction): boolean {
+		let didChange = this.isRead.get() !== isRead;
+		for (const chat of this._chatsObs.get()) {
+			if (this._aggregateChatResources.size > 0 && !this._aggregateChatResources.has(chat.resource)) {
+				continue;
+			}
+			if (chat.isRead.get() !== isRead) {
+				this.setChatRead(chat.resource, isRead, tx);
+				didChange = true;
+			}
+		}
+		this.isRead.set(isRead, tx);
+		return didChange;
+	}
+
 	getBackendChatResource(chatResource: URI): URI | undefined {
 		if (isEqual(chatResource, this._defaultChat.resource)) {
 			return this._defaultChatBackendUri.get();
@@ -6289,6 +6325,10 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement
 		const rawId = this._rawIdFromChatId(sessionId);
 		const cached = rawId ? this._sessionCache.get(rawId) : undefined;
 		if (cached && rawId) {
+			if (!cached.supportsChatReadState()) {
+				this._setLegacySessionReadState(cached, isRead);
+				return;
+			}
 			if (isRead) {
 				for (const chat of cached.getUnreadAggregateChats()) {
 					await this.setChatReadState(sessionId, chat.resource, true);
@@ -6316,11 +6356,26 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement
 		const connection = this.connection;
 		const initializeResult = connection?.initializeResult.get();
 		const action = { type: ActionType.ChatIsReadChanged as const, isRead };
-		if (!cached || !connection || !initializeResult || !isActionKnownToVersion(action, initializeResult.protocolVersion)) {
+		if (!cached || !connection || !initializeResult) {
 			return false;
 		}
 		const backendChatResource = this.getBackendChatResource(chatResource);
-		if (!backendChatResource || !cached.setChatRead(chatResource, isRead)) {
+		if (!backendChatResource) {
+			throw new Error(localize('chatNotFound', "The chat could not be found."));
+		}
+		if (!isActionKnownToVersion(action, initializeResult.protocolVersion)) {
+			const chatChanged = cached.chats.get().some(chat => isEqual(chat.resource, chatResource) && chat.isRead.get() !== isRead);
+			if (!cached.setChatRead(chatResource, isRead)) {
+				throw new Error(localize('chatNotFound', "The chat could not be found."));
+			}
+			if (chatChanged) {
+				this._cacheDirty = true;
+				this._onDidChangeSessions.fire({ added: [], removed: [], changed: [cached] });
+			}
+			this._setLegacySessionReadState(cached, isRead);
+			return true;
+		}
+		if (!cached.setChatRead(chatResource, isRead)) {
 			throw new Error(localize('chatNotFound', "The chat could not be found."));
 		}
 		this._cacheDirty = true;
@@ -6329,6 +6384,17 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement
 		return true;
 	}

+	private _setLegacySessionReadState(cached: AgentHostSessionAdapter, isRead: boolean): void {
+		if (cached.setLegacySessionReadState(isRead)) {
+			this._cacheDirty = true;
+			this._onDidChangeSessions.fire({ added: [], removed: [], changed: [cached] });
+		}
+		const connection = this.connection;
+		if (connection) {
+			connection.dispatch(cached.backendUri.toString(), { type: ActionType.SessionIsReadChanged as const, isRead });
+		}
+	}
+
 	async deleteSession(sessionId: string): Promise<void> {
 		await this.deleteSessions([sessionId]);
 	}
@@ -8015,6 +8081,13 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement
 		const rawId = AgentSession.id(session);
 		const cached = this._sessionCache.get(rawId);
 		if (cached) {
+			if (!cached.supportsChatReadState()) {
+				if (cached.setLegacySessionReadState(isRead)) {
+					this._cacheDirty = true;
+					this._onDidChangeSessions.fire({ added: [], removed: [], changed: [cached] });
+				}
+				return;
+			}
 			const effectiveIsRead = isRead && !cached.hasUnreadChat();
 			const sessionChanged = cached.isRead.get() !== effectiveIsRead;
 			if (!sessionChanged) {
@@ -8052,10 +8125,18 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement
 					didChange = true;
 				}

-				const isRead = !!(changes.status & ProtocolSessionStatus.IsRead) && !cached.hasUnreadChat();
-				if (isRead !== cached.isRead.get()) {
-					cached.isRead.set(isRead, tx);
-					didChange = true;
+				const isRead = !!(changes.status & ProtocolSessionStatus.IsRead);
+				if (!cached.supportsChatReadState()) {
+					if (cached.setLegacySessionReadState(isRead, tx)) {
+						this._cacheDirty = true;
+						didChange = true;
+					}
+				} else {
+					const effectiveIsRead = isRead && !cached.hasUnreadChat();
+					if (effectiveIsRead !== cached.isRead.get()) {
+						cached.isRead.set(effectiveIsRead, tx);
+						didChange = true;
+					}
 				}
 			}

@@ -8841,31 +8841,71 @@ suite('LocalAgentHostSessionsProvider', () => {
 			});
 		});

-		test('chat read state reports protocol rejection so callers can retry after initialization', async () => {
-			agentHost.initializeResult.set({ ...agentHost.initializeResult.get(), protocolVersion: '0.8.0' }, undefined);
-			const provider = createProvider(disposables, agentHost);
-			const session = setupMultiChatSession(provider, 'chat-read-retry');
-			const sessionUri = AgentSession.uri('copilotcli', 'chat-read-retry').toString();
+		test('legacy hosts preserve client read state and use the session read action', async () => {
+			agentHost.initializeResult.set({ ...agentHost.initializeResult.get(), protocolVersion: '0.9.0' }, undefined);
+			const sessionUri = AgentSession.uri('copilotcli', 'legacy-chat-read').toString();
 			const defaultChat = buildDefaultChatUri(sessionUri);
-			agentHost.setSessionState('chat-read-retry', 'copilotcli', makeState([
-				makeChatSummary(defaultChat, ''),
-			], { defaultChat }));
-			const main = session.mainChat.get();
+			const peerChat = buildChatUri(sessionUri, 'peer-1');
+			const hiddenChat = buildChatUri(sessionUri, 'hidden');
+			agentHost.addSession(createSession('legacy-chat-read', {
+				status: ProtocolSessionStatus.Idle | ProtocolSessionStatus.IsRead,
+				chats: [
+					{ chat: URI.parse(defaultChat), kind: 'default', summary: '' },
+					{ chat: URI.parse(peerChat), kind: 'peer', summary: 'Peer' },
+					{ chat: URI.parse(hiddenChat), kind: 'peer', summary: 'Hidden', interactivity: ProtocolChatInteractivity.Hidden },
+				],
+			}));
+			const provider = createProvider(disposables, agentHost);
+			provider.getSessions();
+			await timeout(0);
+			const session = provider.getSessions().find(session => AgentSession.id(session.resource) === 'legacy-chat-read');
+			assert.ok(session);
+			const initial = {
+				session: session.isRead.get(),
+				chats: session.chats.get().map(chat => chat.isRead.get()),
+			};

-			const rejected = await provider.setChatReadState(session.sessionId, main.resource, true);
-			agentHost.initializeResult.set({ ...agentHost.initializeResult.get(), protocolVersion: PROTOCOL_VERSION }, undefined);
-			const accepted = await provider.setChatReadState(session.sessionId, main.resource, true);
+			const accepted = await provider.setChatReadState(session.sessionId, session.chats.get()[2].resource, true);
+			const retried = await provider.setChatReadState(session.sessionId, session.chats.get()[2].resource, true);
+			fireSessionSummaryChanged(agentHost, 'legacy-chat-read', {
+				chats: [
+					{ resource: defaultChat, title: '' },
+					{ resource: peerChat, title: 'Peer' },
+					{ resource: hiddenChat, title: 'Hidden', interactivity: ProtocolChatInteractivity.Hidden },
+				],
+				defaultChat,
+			});

 			assert.deepStrictEqual({
-				rejected,
+				initial,
 				accepted,
-				isRead: main.isRead.get(),
-				actions: agentHost.dispatchedActions.filter(dispatch => dispatch.action.type === ActionType.ChatIsReadChanged).length,
+				retried,
+				afterRefresh: {
+					session: session.isRead.get(),
+					chats: session.chats.get().map(chat => chat.isRead.get()),
+				},
+				actions: agentHost.dispatchedActions.map(dispatch => ({
+					channel: dispatch.channel,
+					type: dispatch.action.type,
+					isRead: dispatch.action.type === ActionType.SessionIsReadChanged ? dispatch.action.isRead : undefined,
+				})),
 			}, {
-				rejected: false,
+				initial: { session: false, chats: [false, false, false] },
 				accepted: true,
-				isRead: true,
-				actions: 1,
+				retried: true,
+				afterRefresh: { session: true, chats: [true, true, true] },
+				actions: [
+					{
+						channel: sessionUri,
+						type: ActionType.SessionIsReadChanged,
+						isRead: true,
+					},
+					{
+						channel: sessionUri,
+						type: ActionType.SessionIsReadChanged,
+						isRead: true,
+					},
+				],
 			});
 		});

@@ -28,6 +28,7 @@ import { AgentHostTransportFailureReason } from '../../../../../../platform/agen
 import { SessionArtifactType, withSessionArtifacts } from '../../../../../../platform/agentHost/common/sessionArtifacts.js';
 import type { ResolveSessionConfigResult } from '../../../../../../platform/agentHost/common/state/protocol/commands.js';
 import { ChangesetStatus, CustomizationType, MessageKind, ResponsePartKind, SessionLifecycle, ToolCallConfirmationReason, ToolCallStatus, ToolResultContentType, TurnState, type AgentCustomization, type AgentInfo, type AutomationState, type ChangesetFile, type ChangesetState, type ChatState, type RootState, type SessionConfigState, type SessionState } from '../../../../../../platform/agentHost/common/state/protocol/state.js';
+import { PROTOCOL_VERSION } from '../../../../../../platform/agentHost/common/state/protocol/version/registry.js';
 import { ActionType, NotificationType, type ActionEnvelope, type IRootConfigChangedAction, type SessionAction, type TerminalAction, type INotification, type ClientAnnotationsAction } from '../../../../../../platform/agentHost/common/state/sessionActions.js';
 import { buildChatUri, buildDefaultChatUri, createChatState, isAhpAutomationCatalogChannel, SessionStatus as ProtocolSessionStatus, StateComponents, withSessionExternal } from '../../../../../../platform/agentHost/common/state/sessionState.js';
 import type { IAgentSubscription } from '../../../../../../platform/agentHost/common/state/agentSubscription.js';
@@ -84,7 +85,7 @@ class MockAgentConnection extends mock<IAgentConnection>() {
 	private _rootStateValue: RootState = { agents: [{ provider: 'copilotcli', displayName: 'Copilot', description: '', models: [] } as AgentInfo] };
 	override readonly rootState: IAgentSubscription<RootState>;
 	override readonly initializeResult = constObservable({
-		protocolVersion: '1',
+		protocolVersion: PROTOCOL_VERSION,
 		serverSeq: 0,
 		snapshots: [],
 		automations: { create: {}, schedules: {}, runCancellation: {} },
@@ -1301,7 +1302,7 @@ suite('RemoteAgentHostSessionsProvider', () => {
 			const handle = '00000000-0000-4000-8000-000000000001';
 			const hostConnection = new class extends MockAgentConnection {
 				override readonly initializeResult = constObservable({
-					protocolVersion: '1', serverSeq: 0, snapshots: [],
+					protocolVersion: PROTOCOL_VERSION, serverSeq: 0, snapshots: [],
 					automations: { create: {}, schedules: {}, runCancellation: {} },
 					_meta: getAgentHostExtensionInitializeResultMeta(),
 				});
@@ -3462,7 +3463,7 @@ suite('CloudSandboxSessionsProvider external sessions', () => {
 		const importedSessions: URI[] = [];
 		const connection = disposables.add(new class extends MockAgentConnection {
 			override readonly initializeResult = constObservable({
-				protocolVersion: '1', serverSeq: 0, snapshots: [],
+				protocolVersion: PROTOCOL_VERSION, serverSeq: 0, snapshots: [],
 				automations: { create: {}, schedules: {}, runCancellation: {} },
 				_meta: getAgentHostExtensionInitializeResultMeta(true, false, false, true),
 			});