diff --git a/src/vs/platform/agentHost/node/agentHostStateManager.ts b/src/vs/platform/agentHost/node/agentHostStateManager.ts index 1fac09a2499b1..9767d928fbcfb 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,6 +334,22 @@ export class AgentHostStateManager extends Disposable { let resultingState: unknown = undefined; // Apply to state if (isRootAction(action)) { + // `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) + : equals({ ...current, ...patch }, current); + if (isNoOp) { + return this._rootState; + } + } this._rootState = rootReducer(this._rootState, action as RootAction, this._log); resultingState = 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 64c0a5bf3e51e..262604dc04f4d 100644 --- a/src/vs/platform/agentHost/test/node/agentHostStateManager.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostStateManager.test.ts @@ -128,6 +128,50 @@ 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); + assert.strictEqual(manager.serverSeq, 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); + 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. + manager.dispatchServerAction({ + type: ActionType.RootConfigChanged, + 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({ + type: ActionType.RootConfigChanged, + config: { 'my.setting': 'value-b' }, + }); + assert.strictEqual(envelopes.length, 3); + assert.strictEqual(manager.serverSeq, 3); + }); + test('removeSession clears state without notification', () => { manager.createSession(makeSessionSummary());