From e08cca7d578833e287d23f702c70de062f48fcee Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Tue, 5 May 2026 10:02:00 -0700 Subject: [PATCH 1/5] Clean up agenthost icon handling Localized to the sessions provider --- .../browser/baseAgentHostSessionsProvider.ts | 35 +++++++++++++++---- .../services/sessions/common/session.ts | 21 ----------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/vs/sessions/contrib/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/agentHost/browser/baseAgentHostSessionsProvider.ts index b8161e1dcf5f0..49ab0c27c15fa 100644 --- a/src/vs/sessions/contrib/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -3,8 +3,9 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { raceTimeout, disposableTimeout } from '../../../../base/common/async.js'; +import { disposableTimeout, raceTimeout } from '../../../../base/common/async.js'; import { CancellationToken } from '../../../../base/common/cancellation.js'; +import { Codicon } from '../../../../base/common/codicons.js'; import { Emitter, Event } from '../../../../base/common/event.js'; import { IMarkdownString, MarkdownString } from '../../../../base/common/htmlContent.js'; import { Disposable, DisposableMap, DisposableStore, IDisposable, IReference, MutableDisposable } from '../../../../base/common/lifecycle.js'; @@ -15,25 +16,25 @@ import { URI } from '../../../../base/common/uri.js'; import { generateUuid } from '../../../../base/common/uuid.js'; import { localize } from '../../../../nls.js'; import { AgentSession, IAgentConnection, IAgentSessionMetadata } from '../../../../platform/agentHost/common/agentService.js'; -import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; import { KNOWN_AUTO_APPROVE_VALUES, SessionConfigKey } from '../../../../platform/agentHost/common/sessionConfigKeys.js'; import { ResolveSessionConfigResult } from '../../../../platform/agentHost/common/state/protocol/commands.js'; import { NotificationType } from '../../../../platform/agentHost/common/state/protocol/notifications.js'; -import { FileEdit, ModelSelection, RootConfigState, RootState, SessionState, SessionSummary, SessionStatus as ProtocolSessionStatus } from '../../../../platform/agentHost/common/state/protocol/state.js'; +import { FileEdit, ModelSelection, SessionStatus as ProtocolSessionStatus, RootConfigState, RootState, SessionState, SessionSummary } from '../../../../platform/agentHost/common/state/protocol/state.js'; import { ActionType, isSessionAction } from '../../../../platform/agentHost/common/state/sessionActions.js'; import { readSessionGitState, StateComponents, type ISessionGitState } from '../../../../platform/agentHost/common/state/sessionState.js'; +import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; +import { ILogService } from '../../../../platform/log/common/log.js'; import { ChatViewPaneTarget, IChatWidgetService } from '../../../../workbench/contrib/chat/browser/chat.js'; import { IChatSendRequestOptions, IChatService } from '../../../../workbench/contrib/chat/common/chatService/chatService.js'; import { IChatSessionFileChange, IChatSessionFileChange2, IChatSessionsService } from '../../../../workbench/contrib/chat/common/chatSessionsService.js'; import { ChatAgentLocation, ChatConfiguration, ChatModeKind } from '../../../../workbench/contrib/chat/common/constants.js'; import { ILanguageModelsService } from '../../../../workbench/contrib/chat/common/languageModels.js'; -import { ILogService } from '../../../../platform/log/common/log.js'; -import { diffsEqual, diffsToChanges, mapProtocolStatus } from './agentHostDiffs.js'; import { buildMutableConfigSchema, IAgentHostSessionsProvider, resolvedConfigsEqual } from '../../../common/agentHostSessionsProvider.js'; import { agentHostSessionWorkspaceKey } from '../../../common/agentHostSessionWorkspace.js'; import { isSessionConfigComplete } from '../../../common/sessionConfig.js'; -import { IChat, iconForAgentProvider, IGitHubInfo, ISession, ISessionChangeset, ISessionType, ISessionWorkspace, ISessionWorkspaceBrowseAction, SessionStatus, toSessionId } from '../../../services/sessions/common/session.js'; +import { CopilotCLISessionType, IChat, IGitHubInfo, ISession, ISessionChangeset, ISessionType, ISessionWorkspace, ISessionWorkspaceBrowseAction, SessionStatus, toSessionId } from '../../../services/sessions/common/session.js'; import { ISendRequestOptions, ISessionChangeEvent } from '../../../services/sessions/common/sessionsProvider.js'; +import { diffsEqual, diffsToChanges, mapProtocolStatus } from './agentHostDiffs.js'; // ============================================================================ // AgentHostSessionAdapter — shared adapter for local and remote sessions @@ -761,7 +762,7 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement const next = rootState.agents.map((agent): ISessionType => ({ id: agent.provider, label: this._formatSessionTypeLabel(agent.displayName?.trim() || agent.provider), - icon: iconForAgentProvider(agent.provider) ?? this.icon, + icon: this.iconForAgentProvider(agent.provider) ?? this.icon, })); const prev = this._sessionTypes; @@ -772,6 +773,26 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement this._onDidChangeSessionTypes.fire(); } + /** + * Returns the {@link ThemeIcon} associated with a known agent provider, or + * `undefined` when the provider is not recognised. + */ + private iconForAgentProvider(provider: string): ThemeIcon | undefined { + if (provider === CopilotCLISessionType.id) { + return CopilotCLISessionType.icon; + } + + if (provider.includes('claude')) { + return Codicon.claude; + } + + if (provider === 'openai' || provider.includes('codex')) { + return Codicon.openai; + } + + return undefined; + } + /** * Reconcile {@link _rootConfig} against {@link RootState.config}, firing * {@link onDidChangeRootConfig} only when schema or values actually change. diff --git a/src/vs/sessions/services/sessions/common/session.ts b/src/vs/sessions/services/sessions/common/session.ts index 846826f424588..1ee6d62fb2abc 100644 --- a/src/vs/sessions/services/sessions/common/session.ts +++ b/src/vs/sessions/services/sessions/common/session.ts @@ -51,27 +51,6 @@ export const ClaudeCodeSessionType: ISessionType = { icon: Codicon.claude, }; -/** - * Returns the {@link ThemeIcon} associated with a known agent provider, or - * `undefined` when the provider is not recognised. - * - * - Any provider whose ID contains `'copilot'` → {@link Codicon.copilot} - * - Any provider whose ID contains `'claude'` → {@link Codicon.claude} - * - `'openai'` or any provider whose ID contains `'codex'` → {@link Codicon.openai} - */ -export function iconForAgentProvider(provider: string): ThemeIcon | undefined { - if (provider.includes('copilot')) { - return Codicon.copilot; - } - if (provider.includes('claude')) { - return Codicon.claude; - } - if (provider === 'openai' || provider.includes('codex')) { - return Codicon.openai; - } - return undefined; -} - /** * Returns whether the given session type represents a workspace-backed * agent (e.g. Copilot CLI, Claude Code) that operates on a worktree or From 122bead33b57691edb0ad080fc0911cc27f3f706 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Tue, 5 May 2026 17:13:55 -0700 Subject: [PATCH 2/5] agentHost: dedupe no-op root state actions Skip envelope emission in AgentHostStateManager when a root action's reducer output deeply equals the previous root state. This prevents a feedback loop where a contribution re-publishes a root config value that already matches: the server would still fire a RootStateSubscription change, the contribution would react and re-dispatch, and so on. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../agentHost/node/agentHostStateManager.ts | 15 ++++++- .../test/node/agentHostStateManager.test.ts | 39 +++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/src/vs/platform/agentHost/node/agentHostStateManager.ts b/src/vs/platform/agentHost/node/agentHostStateManager.ts index 1fac09a2499b1..a7b2393cdcb97 100644 --- a/src/vs/platform/agentHost/node/agentHostStateManager.ts +++ b/src/vs/platform/agentHost/node/agentHostStateManager.ts @@ -6,6 +6,7 @@ import { RunOnceScheduler } from '../../../base/common/async.js'; import { Emitter, Event } from '../../../base/common/event.js'; import { Disposable } from '../../../base/common/lifecycle.js'; +import { equals } from '../../../base/common/objects.js'; import { ILogService } from '../../log/common/log.js'; import { ActionType, NotificationType, ActionEnvelope, ActionOrigin, INotification, IRootConfigChangedAction, SessionAction, RootAction, StateAction, TerminalAction, isRootAction, isSessionAction } from '../common/state/sessionActions.js'; import type { IStateSnapshot } from '../common/state/sessionProtocol.js'; @@ -333,8 +334,18 @@ export class AgentHostStateManager extends Disposable { let resultingState: unknown = undefined; // Apply to state if (isRootAction(action)) { - this._rootState = rootReducer(this._rootState, action as RootAction, this._log); - resultingState = this._rootState; + const prevRoot = this._rootState; + const nextRoot = rootReducer(prevRoot, action as RootAction, this._log); + // Skip emission entirely when the action is a no-op against the + // current root state. Otherwise downstream listeners (and clients + // observing rootState.onDidChange) treat every dispatched action as + // a change, which can spin into an infinite loop when something + // re-publishes a value that already matches. + if (equals(prevRoot, nextRoot)) { + return prevRoot; + } + this._rootState = nextRoot; + resultingState = nextRoot; } if (isSessionAction(action)) { diff --git a/src/vs/platform/agentHost/test/node/agentHostStateManager.test.ts b/src/vs/platform/agentHost/test/node/agentHostStateManager.test.ts index 64c0a5bf3e51e..83c712a878bf9 100644 --- a/src/vs/platform/agentHost/test/node/agentHostStateManager.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostStateManager.test.ts @@ -128,6 +128,45 @@ suite('AgentHostStateManager', () => { assert.deepStrictEqual(envelopes[0].origin, origin); }); + test('root action that does not change state is not emitted', () => { + const envelopes: ActionEnvelope[] = []; + disposables.add(manager.onDidEmitEnvelope(e => envelopes.push(e))); + + // First dispatch: introduces a new value, should emit. + manager.dispatchServerAction({ + type: ActionType.RootConfigChanged, + config: { 'my.setting': 'value-a' }, + }); + assert.strictEqual(envelopes.length, 1); + + // Second dispatch with the same value: should be deduped and not emit. + manager.dispatchServerAction({ + type: ActionType.RootConfigChanged, + config: { 'my.setting': 'value-a' }, + }); + assert.strictEqual(envelopes.length, 1); + + // Third dispatch with a deeply-equal but newly allocated object value: + // should also be deduped. + manager.dispatchServerAction({ + type: ActionType.RootConfigChanged, + config: { 'my.nested': { allow: ['x'], deny: [] } }, + }); + assert.strictEqual(envelopes.length, 2); + manager.dispatchServerAction({ + type: ActionType.RootConfigChanged, + config: { 'my.nested': { allow: ['x'], deny: [] } }, + }); + assert.strictEqual(envelopes.length, 2); + + // Real change still emits. + manager.dispatchServerAction({ + type: ActionType.RootConfigChanged, + config: { 'my.setting': 'value-b' }, + }); + assert.strictEqual(envelopes.length, 3); + }); + test('removeSession clears state without notification', () => { manager.createSession(makeSessionSummary()); From 865abe659d08793d2bf448f4e557e419c29fc8fe Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Tue, 5 May 2026 18:11:45 -0700 Subject: [PATCH 3/5] agentHost: narrow root no-op dedupe to RootConfigChanged Per review feedback, only RootConfigChanged is dispatched as a true no-op (its reducer spreads values even when the patch matches). Other root actions like RootActiveSessionsChanged fire frequently (every turn start/complete) and always carry a real value, so the deep-compare was wasted work for them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../agentHost/node/agentHostStateManager.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/vs/platform/agentHost/node/agentHostStateManager.ts b/src/vs/platform/agentHost/node/agentHostStateManager.ts index a7b2393cdcb97..ab33bae5d79d7 100644 --- a/src/vs/platform/agentHost/node/agentHostStateManager.ts +++ b/src/vs/platform/agentHost/node/agentHostStateManager.ts @@ -336,12 +336,14 @@ export class AgentHostStateManager extends Disposable { if (isRootAction(action)) { const prevRoot = this._rootState; const nextRoot = rootReducer(prevRoot, action as RootAction, this._log); - // Skip emission entirely when the action is a no-op against the - // current root state. Otherwise downstream listeners (and clients - // observing rootState.onDidChange) treat every dispatched action as - // a change, which can spin into an infinite loop when something - // re-publishes a value that already matches. - if (equals(prevRoot, nextRoot)) { + // `RootConfigChanged` is the one root action that can be a true + // no-op against current state (the reducer spreads values even + // when the patch matches), and re-emitting it would cause clients + // observing rootState.onDidChange to react and potentially + // re-dispatch in a loop. Other root actions always replace whole + // fields and are only dispatched on real changes, so we skip the + // deep-compare for them. + if (action.type === ActionType.RootConfigChanged && equals(prevRoot.config, nextRoot.config)) { return prevRoot; } this._rootState = nextRoot; From 50719b2aa9ffdc16d4e7c3f95372d00ae3d655b3 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Tue, 5 May 2026 18:19:49 -0700 Subject: [PATCH 4/5] agentHost: pre-check action patch to avoid no-op reducer allocation Instead of running the reducer and post-comparing output, inspect the RootConfigChanged action's patch against the current config values before calling rootReducer. This avoids allocating a new state object entirely for no-op dispatches, rather than allocating and discarding it. (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../agentHost/node/agentHostStateManager.ts | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/vs/platform/agentHost/node/agentHostStateManager.ts b/src/vs/platform/agentHost/node/agentHostStateManager.ts index ab33bae5d79d7..e4d6a6eee0472 100644 --- a/src/vs/platform/agentHost/node/agentHostStateManager.ts +++ b/src/vs/platform/agentHost/node/agentHostStateManager.ts @@ -334,20 +334,24 @@ export class AgentHostStateManager extends Disposable { let resultingState: unknown = undefined; // Apply to state if (isRootAction(action)) { - const prevRoot = this._rootState; - const nextRoot = rootReducer(prevRoot, action as RootAction, this._log); - // `RootConfigChanged` is the one root action that can be a true - // no-op against current state (the reducer spreads values even - // when the patch matches), and re-emitting it would cause clients - // observing rootState.onDidChange to react and potentially - // re-dispatch in a loop. Other root actions always replace whole - // fields and are only dispatched on real changes, so we skip the - // deep-compare for them. - if (action.type === ActionType.RootConfigChanged && equals(prevRoot.config, nextRoot.config)) { - return prevRoot; + // `RootConfigChanged` can be a true no-op: the reducer merges/replaces + // values even when the patch matches the current state, and re-emitting + // it would cause clients observing rootState.onDidChange to react and + // potentially re-dispatch in a loop. Check the action's own patch + // against current values before running the reducer so we avoid + // allocating a new state object at all. + if (action.type === ActionType.RootConfigChanged && this._rootState.config) { + const current = this._rootState.config.values; + const patch = action.config; + const isNoOp = action.replace + ? equals(current, patch) + : Object.keys(patch).every(k => equals(current[k], patch[k])); + if (isNoOp) { + return this._rootState; + } } - this._rootState = nextRoot; - resultingState = nextRoot; + this._rootState = rootReducer(this._rootState, action as RootAction, this._log); + resultingState = this._rootState; } if (isSessionAction(action)) { From 927f6bf35abbe525a1166089902f576d7e9f0c63 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Tue, 5 May 2026 18:41:55 -0700 Subject: [PATCH 5/5] agentHost: fix merge-mode no-op check and add serverSeq assertions - Merge-mode pre-check computes the merged result and deep-compares against current values, so a patch adding a new key (even with value undefined) is not mistakenly treated as a no-op - Test now asserts serverSeq does not advance on deduped (no-op) actions (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/vs/platform/agentHost/node/agentHostStateManager.ts | 2 +- .../agentHost/test/node/agentHostStateManager.test.ts | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/vs/platform/agentHost/node/agentHostStateManager.ts b/src/vs/platform/agentHost/node/agentHostStateManager.ts index e4d6a6eee0472..9767d928fbcfb 100644 --- a/src/vs/platform/agentHost/node/agentHostStateManager.ts +++ b/src/vs/platform/agentHost/node/agentHostStateManager.ts @@ -345,7 +345,7 @@ export class AgentHostStateManager extends Disposable { const patch = action.config; const isNoOp = action.replace ? equals(current, patch) - : Object.keys(patch).every(k => equals(current[k], patch[k])); + : equals({ ...current, ...patch }, current); if (isNoOp) { return this._rootState; } diff --git a/src/vs/platform/agentHost/test/node/agentHostStateManager.test.ts b/src/vs/platform/agentHost/test/node/agentHostStateManager.test.ts index 83c712a878bf9..262604dc04f4d 100644 --- a/src/vs/platform/agentHost/test/node/agentHostStateManager.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostStateManager.test.ts @@ -138,6 +138,7 @@ suite('AgentHostStateManager', () => { config: { 'my.setting': 'value-a' }, }); assert.strictEqual(envelopes.length, 1); + assert.strictEqual(manager.serverSeq, 1); // Second dispatch with the same value: should be deduped and not emit. manager.dispatchServerAction({ @@ -145,6 +146,7 @@ suite('AgentHostStateManager', () => { config: { 'my.setting': 'value-a' }, }); assert.strictEqual(envelopes.length, 1); + assert.strictEqual(manager.serverSeq, 1, 'serverSeq must not advance on a no-op'); // Third dispatch with a deeply-equal but newly allocated object value: // should also be deduped. @@ -153,11 +155,13 @@ suite('AgentHostStateManager', () => { config: { 'my.nested': { allow: ['x'], deny: [] } }, }); assert.strictEqual(envelopes.length, 2); + assert.strictEqual(manager.serverSeq, 2); manager.dispatchServerAction({ type: ActionType.RootConfigChanged, config: { 'my.nested': { allow: ['x'], deny: [] } }, }); assert.strictEqual(envelopes.length, 2); + assert.strictEqual(manager.serverSeq, 2, 'serverSeq must not advance on a no-op'); // Real change still emits. manager.dispatchServerAction({ @@ -165,6 +169,7 @@ suite('AgentHostStateManager', () => { config: { 'my.setting': 'value-b' }, }); assert.strictEqual(envelopes.length, 3); + assert.strictEqual(manager.serverSeq, 3); }); test('removeSession clears state without notification', () => {