Skip to content

Commit 86bd3e9

Browse files
authored
agent-host: stop redundant customization sync events (#317611)
* agent-host: stop redundant customization sync events Refactors customization syncing to publish state changes as SessionActions through the session state manager rather than via a per-call progress callback that emitted full lists each tick. Also avoids redundant `activeClientChanged` dispatches that were driving large loops of customization/file requests across the connection. - `setClientCustomizations` now takes the session URI and publishes `SessionCustomizationsChanged` once with the resolved list, followed by `SessionCustomizationUpdated` actions only when an individual customization's state actually changes. - `agentSideEffects` no longer wires its own progress callback; it lets the agent publish actions and forwards action signals into the state manager even when no turn is active so session-level state (customizations, title, config) stays consistent. - `agentHostSessionHandler` no longer claims the active-client role on session restore, and only re-dispatches `activeClientChanged` when the local view of `activeClient` actually differs from the session state. The pre-send hook ensures the active client is current before sending a message. - `agentCustomizationItemProvider` reads session customizations from the shared session subscription instead of caching them locally and re-firing on every customization update. - Adds tests covering the no-redundant-dispatch behavior and the new progress publishing flow. Fixes #316538 (Commit message generated by Copilot) * address Copilot review and fix provider tests - copilotAgent: construct SessionCustomizationUpdated explicitly (omit clientId) - mockAgent: always emit SessionCustomizationsChanged so clears propagate - agentCustomizationItemProvider: restore session customizations cache and handle both Changed/Updated actions - agentSideEffects.test: assert clear publishes one empty SessionCustomizationsChanged * drop local session-customization cache, read from session subscription Replaces the provider-local _sessionCustomizationsCache with a direct read from the central session subscription via getSubscriptionUnmanaged(StateComponents.Session). The session reducer already maintains 'customizations' from SessionCustomizationsChanged/Updated, so the local mirror only duplicated logic and would have missed state populated before the provider attached. Teaches MockAgentConnection to expose session subscriptions backed by fireAction so tests exercise the same code path. * build
1 parent 1661173 commit 86bd3e9

15 files changed

Lines changed: 360 additions & 101 deletions

File tree

src/vs/platform/agentHost/common/agentPluginManager.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,15 @@ export interface IAgentPluginManager {
4141
* Syncs a set of client-provided customization refs to local storage.
4242
*
4343
* Each ref is copied to a local directory, respecting nonce-based
44-
* caching. The optional {@link progress} callback fires as individual
45-
* customizations complete or fail, allowing callers to publish
46-
* incremental status updates.
44+
* caching. The optional {@link progress} callback fires with the single
45+
* customization that completed or failed, allowing callers to publish
46+
* targeted incremental status updates.
4747
*
4848
* Concurrent calls for the same plugin URI are serialized so that
4949
* overlapping syncs do not clobber each other.
5050
*
5151
* @returns Final status for every customization, with `pluginDir`
5252
* defined when the sync was successful.
5353
*/
54-
syncCustomizations(clientId: string, customizations: CustomizationRef[], progress?: (status: SessionCustomization[]) => void): Promise<ISyncedCustomization[]>;
54+
syncCustomizations(clientId: string, customizations: CustomizationRef[], progress?: (status: SessionCustomization) => void): Promise<ISyncedCustomization[]>;
5555
}

src/vs/platform/agentHost/common/agentService.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -644,13 +644,13 @@ export interface IAgent {
644644
onArchivedChanged?(session: URI, isArchived: boolean): Promise<void>;
645645

646646
/**
647-
* Receives client-provided customization refs and syncs them (e.g. copies
648-
* plugin files to local storage). Returns per-customization status with
649-
* local plugin directories.
647+
* Receives client-provided customization refs for a session and syncs them
648+
* (e.g. copies plugin files to local storage). The agent publishes
649+
* customization state actions as the sync progresses.
650650
*
651651
* The agent MAY defer a client restart until all active sessions are idle.
652652
*/
653-
setClientCustomizations(clientId: string, customizations: CustomizationRef[], progress?: (results: ISyncedCustomization[]) => void): Promise<ISyncedCustomization[]>;
653+
setClientCustomizations(session: URI, clientId: string, customizations: CustomizationRef[]): Promise<ISyncedCustomization[]>;
654654

655655
/**
656656
* Receives client-provided tool definitions to make available in a

src/vs/platform/agentHost/node/agentPluginManager.ts

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -68,32 +68,24 @@ export class AgentPluginManager implements IAgentPluginManager {
6868
async syncCustomizations(
6969
clientId: string,
7070
customizations: CustomizationRef[],
71-
progress?: (status: SessionCustomization[]) => void,
71+
progress?: (status: SessionCustomization) => void,
7272
): Promise<ISyncedCustomization[]> {
7373
await this._ensureCacheLoaded();
7474

75-
// Build initial loading status and fire it immediately via progress
76-
const statuses: SessionCustomization[] = customizations.map(c => ({
77-
customization: c,
78-
enabled: true,
79-
status: CustomizationStatus.Loading,
80-
}));
81-
progress?.([...statuses]);
82-
8375
// Sync each customization in parallel, serialized per URI
84-
const results = await Promise.all(customizations.map((ref, i) =>
76+
const results = await Promise.all(customizations.map(ref =>
8577
this._sequencer.queue(ref.uri, async (): Promise<ISyncedCustomization> => {
8678
try {
8779
const pluginDir = await this._syncPlugin(clientId, ref);
88-
statuses[i] = { customization: ref, enabled: true, status: CustomizationStatus.Loaded };
89-
progress?.([...statuses]);
90-
return { customization: statuses[i], pluginDir };
80+
const customization = { customization: ref, enabled: true, status: CustomizationStatus.Loaded };
81+
progress?.(customization);
82+
return { customization, pluginDir };
9183
} catch (err) {
9284
const message = err instanceof Error ? err.message : String(err);
9385
this._logService.error(`[AgentPluginManager] Failed to sync plugin ${ref.uri}: ${message}`);
94-
statuses[i] = { customization: ref, enabled: true, status: CustomizationStatus.Error, statusMessage: message };
95-
progress?.([...statuses]);
96-
return { customization: statuses[i] };
86+
const customization = { customization: ref, enabled: true, status: CustomizationStatus.Error, statusMessage: message };
87+
progress?.(customization);
88+
return { customization };
9789
}
9890
})
9991
));

src/vs/platform/agentHost/node/agentSideEffects.ts

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -332,14 +332,16 @@ export class AgentSideEffects extends Disposable {
332332
return;
333333
}
334334

335-
// No active turn on the session. Most signals are silently dropped,
336-
// but a `SessionTurnComplete` (idle) still needs to drive its
337-
// post-turn side effects — flushing pending diff computation,
338-
// recomputing diffs, and notifying the host. Tests routinely fire
339-
// `idle` without first dispatching the matching `SessionTurnStarted`
340-
// through the state manager.
341-
if (signal.kind === 'action' && signal.action.type === ActionType.SessionTurnComplete) {
342-
this._runTurnCompleteSideEffects(sessionKey, undefined);
335+
// No active turn on the session. Non-action signals are silently
336+
// dropped, but action signals can still target session-level state
337+
// such as customizations, title, or configuration. A turnComplete
338+
// action also drives post-turn side effects even when the matching
339+
// turnStarted was not observed by this side-effects instance.
340+
if (signal.kind === 'action') {
341+
this._stateManager.dispatchServerAction(sessionKey, signal.action);
342+
if (signal.action.type === ActionType.SessionTurnComplete) {
343+
this._runTurnCompleteSideEffects(sessionKey, undefined);
344+
}
343345
}
344346
}
345347

@@ -798,15 +800,7 @@ export class AgentSideEffects extends Disposable {
798800
agent.setClientTools(URI.parse(channel), clientId, action.activeClient?.tools ?? []);
799801

800802
const refs = action.activeClient?.customizations ?? [];
801-
agent.setClientCustomizations(
802-
clientId,
803-
refs,
804-
() => {
805-
this._publishSessionCustomizationsSoon(agent, channel);
806-
},
807-
).then(() => {
808-
this._publishSessionCustomizationsSoon(agent, channel);
809-
}).catch(err => {
803+
agent.setClientCustomizations(URI.parse(channel), clientId, refs).catch(err => {
810804
this._logService.error('[AgentSideEffects] setClientCustomizations failed', err);
811805
});
812806
break;
@@ -971,4 +965,3 @@ export class AgentSideEffects extends Disposable {
971965
super.dispose();
972966
}
973967
}
974-

src/vs/platform/agentHost/node/claude/claudeAgent.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1078,7 +1078,7 @@ export class ClaudeAgent extends Disposable implements IAgent {
10781078
// matching pending promise on the SDK session.
10791079
}
10801080

1081-
setClientCustomizations(_clientId: string, _customizations: CustomizationRef[], _progress?: (results: ISyncedCustomization[]) => void): Promise<ISyncedCustomization[]> {
1081+
setClientCustomizations(_session: URI, _clientId: string, _customizations: CustomizationRef[]): Promise<ISyncedCustomization[]> {
10821082
throw new Error('TODO: Phase 11');
10831083
}
10841084

src/vs/platform/agentHost/node/copilot/copilotAgent.ts

Lines changed: 41 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import { SessionConfigKey } from '../../common/sessionConfigKeys.js';
3030
import { ISessionDataService, SESSION_DB_FILENAME } from '../../common/sessionDataService.js';
3131
import type { ResolveSessionConfigResult, SessionConfigCompletionsResult } from '../../common/state/protocol/commands.js';
3232
import { ProtectedResourceMetadata, type ConfigSchema, type ModelSelection, type SessionCustomization, type ToolDefinition } from '../../common/state/protocol/state.js';
33+
import { ActionType, type SessionAction } from '../../common/state/sessionActions.js';
3334
import { AHP_AUTH_REQUIRED, ProtocolError } from '../../common/state/sessionProtocol.js';
3435
import { CustomizationRef, CustomizationStatus, ResponsePartKind, SessionInputResponseKind, parseSubagentSessionUri, type MessageAttachment, type PendingMessage, type PolicyState, type ResponsePart, type SessionInputAnswer, type ToolCallResult, type Turn } from '../../common/state/sessionState.js';
3536
import { IAgentConfigurationService } from '../agentConfigurationService.js';
@@ -310,14 +311,18 @@ export class CopilotAgent extends Disposable implements IAgent {
310311
}
311312

312313
async getSessionCustomizations(session: URI): Promise<readonly SessionCustomization[]> {
314+
return this._plugins.getSessionCustomizations(await this._getSessionCustomizationDirectory(session));
315+
}
316+
317+
private async _getSessionCustomizationDirectory(session: URI): Promise<URI | undefined> {
313318
const sessionId = AgentSession.id(session);
314319
const provisional = this._provisionalSessions.get(sessionId);
315320
if (provisional) {
316-
return this._plugins.getSessionCustomizations(provisional.workingDirectory);
321+
return provisional.workingDirectory;
317322
}
318323
const entry = this._sessions.get(sessionId);
319324
const metadata = entry ? undefined : await this._readSessionMetadata(session);
320-
return this._plugins.getSessionCustomizations(entry?.customizationDirectory ?? metadata?.customizationDirectory ?? metadata?.workingDirectory);
325+
return entry?.customizationDirectory ?? metadata?.customizationDirectory ?? metadata?.workingDirectory;
321326
}
322327

323328
async authenticate(resource: string, token: string): Promise<boolean> {
@@ -760,7 +765,7 @@ export class CopilotAgent extends Disposable implements IAgent {
760765
const ac = this._getOrCreateActiveClient(sessionUri);
761766
ac.updateTools(config.activeClient.clientId, config.activeClient.tools);
762767
if (config.activeClient.customizations !== undefined) {
763-
await this._plugins.sync(config.activeClient.clientId, config.activeClient.customizations);
768+
await this._plugins.sync(config.activeClient.clientId, config.activeClient.customizations, config.workingDirectory);
764769
}
765770
}
766771

@@ -940,8 +945,11 @@ export class CopilotAgent extends Disposable implements IAgent {
940945
return { items: branches.map(branch => ({ value: branch, label: branch })) };
941946
}
942947

943-
async setClientCustomizations(clientId: string, customizations: CustomizationRef[], progress?: (results: ISyncedCustomization[]) => void): Promise<ISyncedCustomization[]> {
944-
return this._plugins.sync(clientId, customizations, progress);
948+
async setClientCustomizations(session: URI, clientId: string, customizations: CustomizationRef[]): Promise<ISyncedCustomization[]> {
949+
const directory = await this._getSessionCustomizationDirectory(session);
950+
return this._plugins.sync(clientId, customizations, directory, action => {
951+
this._onDidSessionProgress.fire({ kind: 'action', session, action });
952+
});
945953
}
946954

947955
setClientTools(session: URI, clientId: string, tools: ToolDefinition[]): void {
@@ -1931,7 +1939,7 @@ class PluginController extends Disposable {
19311939
});
19321940
}
19331941

1934-
public sync(clientId: string, customizations: CustomizationRef[], progress?: (results: ISyncedCustomization[]) => void) {
1942+
public sync(clientId: string, customizations: CustomizationRef[], directory: URI | undefined, publish?: (action: SessionAction) => void) {
19351943
const revision = ++this._clientRevision;
19361944
this._clientCustomizations = customizations.map(customization => ({
19371945
customization: {
@@ -1941,7 +1949,29 @@ class PluginController extends Disposable {
19411949
status: CustomizationStatus.Loading,
19421950
},
19431951
}));
1944-
progress?.(this._clientCustomizations.map(item => ({ customization: this._applyEnablement(item.customization) })));
1952+
publish?.({
1953+
type: ActionType.SessionCustomizationsChanged,
1954+
customizations: [...this.getSessionCustomizations(directory)],
1955+
});
1956+
const published = new Map<string, SessionCustomization>();
1957+
for (const customization of this._clientCustomizations) {
1958+
const enabled = this._applyEnablement(customization.customization);
1959+
published.set(enabled.customization.uri, this._applyEnablement(customization.customization));
1960+
}
1961+
const publishUpdate = (item: IResolvedCustomization) => {
1962+
const customization = this._applyEnablement(item.customization);
1963+
if (equals(published.get(customization.customization.uri), customization)) {
1964+
return;
1965+
}
1966+
published.set(customization.customization.uri, { ...customization });
1967+
publish?.({
1968+
type: ActionType.SessionCustomizationUpdated,
1969+
customization: customization.customization,
1970+
enabled: customization.enabled,
1971+
status: customization.status,
1972+
statusMessage: customization.statusMessage,
1973+
});
1974+
};
19451975

19461976
const prev = this._clientSync;
19471977
const promise = this._clientSync = prev.catch(err => {
@@ -1952,18 +1982,15 @@ class PluginController extends Disposable {
19521982
return;
19531983
}
19541984

1955-
this._clientCustomizations = status.map(customization => ({
1956-
customization: {
1957-
...customization,
1958-
clientId,
1959-
},
1960-
}));
1961-
progress?.(this._clientCustomizations.map(item => ({ customization: this._applyEnablement(item.customization) })));
1985+
publishUpdate({ customization: { ...status, clientId } });
19621986
});
19631987

19641988
const resolved = await Promise.all(result.map(item => this._resolveSyncedCustomization(item, clientId)));
19651989
if (revision === this._clientRevision) {
19661990
this._clientCustomizations = resolved;
1991+
for (const item of resolved) {
1992+
publishUpdate(item);
1993+
}
19671994
}
19681995
return resolved;
19691996
});

src/vs/platform/agentHost/test/node/agentPluginManager.test.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -88,18 +88,15 @@ suite('AgentPluginManager', () => {
8888
assert.strictEqual(results[1].pluginDir, undefined);
8989
});
9090

91-
test('fires progress callback with loading, then loaded', async () => {
91+
test('fires progress callback with changed customization status', async () => {
9292
await seedPluginDir('prog', { 'index.js': 'content' });
9393

94-
const progressCalls: SessionCustomization[][] = [];
95-
await manager.syncCustomizations('test-client', [makeRef('prog', 'n1')], statuses => {
96-
progressCalls.push(statuses);
94+
const progressCalls: SessionCustomization[] = [];
95+
await manager.syncCustomizations('test-client', [makeRef('prog', 'n1')], status => {
96+
progressCalls.push(status);
9797
});
9898

99-
// At least two calls: initial loading + final loaded
100-
assert.ok(progressCalls.length >= 2, `expected at least 2 progress calls, got ${progressCalls.length}`);
101-
assert.strictEqual(progressCalls[0][0].status, CustomizationStatus.Loading);
102-
assert.strictEqual(progressCalls[progressCalls.length - 1][0].status, CustomizationStatus.Loaded);
99+
assert.deepStrictEqual(progressCalls.map(call => call.status), [CustomizationStatus.Loaded]);
103100
});
104101

105102
test('skips copy when nonce matches', async () => {

src/vs/platform/agentHost/test/node/agentSideEffects.test.ts

Lines changed: 88 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import { ISessionDataService } from '../../common/sessionDataService.js';
2323
import type { RootConfigChangedAction } from '../../common/state/protocol/actions.js';
2424
import { CustomizationStatus } from '../../common/state/protocol/state.js';
2525
import { ActionType, ActionEnvelope, SessionAction } from '../../common/state/sessionActions.js';
26-
import { buildSubagentSessionUri, MessageAttachmentKind, PendingMessageKind, ResponsePartKind, SessionStatus, ToolCallConfirmationReason, ToolCallStatus, ToolResultContentType } from '../../common/state/sessionState.js';
26+
import { buildSubagentSessionUri, MessageAttachmentKind, PendingMessageKind, ResponsePartKind, SessionStatus, ToolCallConfirmationReason, ToolCallStatus, ToolResultContentType, type SessionCustomization } from '../../common/state/sessionState.js';
2727
import { IProductService } from '../../../product/common/productService.js';
2828
import { ITelemetryService, TelemetryLevel } from '../../../telemetry/common/telemetry.js';
2929
import { NullTelemetryService } from '../../../telemetry/common/telemetryUtils.js';
@@ -864,7 +864,11 @@ suite('AgentSideEffects', () => {
864864

865865
suite('handleAction — session/activeClientChanged', () => {
866866

867-
test('calls setClientCustomizations and dispatches customizationsChanged', async () => {
867+
setup(() => {
868+
disposables.add(sideEffects.registerProgressListener(agent));
869+
});
870+
871+
test('calls setClientCustomizations and dispatches customizationsChanged once', async () => {
868872
setupSession();
869873
agent.getSessionCustomizations = async () => [
870874
{
@@ -908,7 +912,83 @@ suite('AgentSideEffects', () => {
908912

909913
const customizationActions = envelopes
910914
.filter(e => e.action.type === ActionType.SessionCustomizationsChanged);
911-
assert.ok(customizationActions.length >= 1, 'should dispatch at least one customizationsChanged');
915+
assert.strictEqual(customizationActions.length, 1, 'should dispatch one full customizationsChanged replacement');
916+
assert.strictEqual(
917+
envelopes.filter(e => e.action.type === ActionType.SessionCustomizationUpdated).length,
918+
0,
919+
'should not dispatch customizationUpdated when progress matches the final state',
920+
);
921+
});
922+
923+
test('dispatches customizationUpdated for sync progress after initial replacement', async () => {
924+
setupSession();
925+
const customization = { uri: 'file:///plugin-a', displayName: 'Plugin A' };
926+
let currentCustomizations: readonly SessionCustomization[] = [];
927+
agent.getSessionCustomizations = async () => currentCustomizations;
928+
agent.setClientCustomizations = async (session, clientId, customizations) => {
929+
agent.setClientCustomizationsCalls.push({ clientId, customizations });
930+
currentCustomizations = [{
931+
customization,
932+
enabled: true,
933+
status: CustomizationStatus.Loading,
934+
}];
935+
agent.fireProgress({
936+
kind: 'action',
937+
session,
938+
action: {
939+
type: ActionType.SessionCustomizationsChanged,
940+
customizations: [...currentCustomizations],
941+
},
942+
});
943+
await new Promise(resolve => setTimeout(resolve, 0));
944+
currentCustomizations = [{
945+
customization,
946+
enabled: true,
947+
status: CustomizationStatus.Loaded,
948+
}];
949+
agent.fireProgress({
950+
kind: 'action',
951+
session,
952+
action: {
953+
type: ActionType.SessionCustomizationUpdated,
954+
customization,
955+
enabled: true,
956+
status: CustomizationStatus.Loaded,
957+
},
958+
});
959+
return currentCustomizations.map(customization => ({ customization }));
960+
};
961+
962+
const envelopes: ActionEnvelope[] = [];
963+
disposables.add(stateManager.onDidEmitEnvelope(e => envelopes.push(e)));
964+
965+
sideEffects.handleAction(sessionUri.toString(), {
966+
type: ActionType.SessionActiveClientChanged,
967+
activeClient: {
968+
clientId: 'test-client',
969+
tools: [],
970+
customizations: [customization],
971+
},
972+
});
973+
await new Promise(resolve => setTimeout(resolve, 50));
974+
975+
const customizationsChanged = envelopes.filter(e => e.action.type === ActionType.SessionCustomizationsChanged);
976+
assert.strictEqual(customizationsChanged.length, 1);
977+
const firstCustomizationsChanged = customizationsChanged[0].action;
978+
assert.strictEqual(firstCustomizationsChanged.type, ActionType.SessionCustomizationsChanged);
979+
assert.deepStrictEqual(firstCustomizationsChanged.customizations, [{
980+
customization,
981+
enabled: true,
982+
status: CustomizationStatus.Loading,
983+
}]);
984+
985+
const customizationUpdated = envelopes.filter(e => e.action.type === ActionType.SessionCustomizationUpdated);
986+
assert.deepStrictEqual(customizationUpdated.map(e => e.action), [{
987+
type: ActionType.SessionCustomizationUpdated,
988+
customization,
989+
enabled: true,
990+
status: CustomizationStatus.Loaded,
991+
}]);
912992
});
913993

914994
test('clears client customizations when activeClient has no customizations', () => {
@@ -932,7 +1012,11 @@ suite('AgentSideEffects', () => {
9321012
}]);
9331013
const customizationActions = envelopes
9341014
.filter(e => e.action.type === ActionType.SessionCustomizationsChanged);
935-
assert.strictEqual(customizationActions.length, 0);
1015+
assert.strictEqual(customizationActions.length, 1);
1016+
assert.deepStrictEqual(customizationActions[0].action, {
1017+
type: ActionType.SessionCustomizationsChanged,
1018+
customizations: [],
1019+
});
9361020
});
9371021

9381022
test('clears client customizations when activeClient is null', () => {

0 commit comments

Comments
 (0)