agentHost: dedupe no-op root state actions

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;
 		}

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());