Skip to content

Commit 48a1f3a

Browse files
joshspicerCopilot
andcommitted
Refactor: PluginController self-manages host customizations
Move host customization management into PluginController so the IAgent interface no longer needs setHostCustomizations. PluginController now: - injects IAgentConfigurationService - subscribes to onDidRootConfigChange (new event) - reads + resolves customizations internally - fires onDidChange so AgentSideEffects can republish This removes the external poke from AgentSideEffects and the seed call from AgentService.registerProvider, addressing Connor's architectural feedback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1060c44 commit 48a1f3a

8 files changed

Lines changed: 85 additions & 68 deletions

File tree

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

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -483,16 +483,11 @@ export interface IAgent {
483483
getProtectedResources(): ProtectedResourceMetadata[];
484484

485485
/**
486-
* Updates the host-owned customization refs available to this agent.
487-
*
488-
* The list is a full replacement of the remote agent host's configured
489-
* plugins after scope resolution and persistence have been applied.
490-
*
491-
* Implementations can invoke `progress` whenever session-visible status
492-
* for the host customizations changes so callers can republish
493-
* `SessionCustomization` state.
486+
* Fires when the agent's host-owned customizations change
487+
* (loading state, resolution results, etc.), so infrastructure
488+
* can republish {@link AgentInfo} and session customization state.
494489
*/
495-
setHostCustomizations?(customizations: CustomizationRef[], progress?: () => void): void;
490+
readonly onDidCustomizationsChange?: Event<void>;
496491

497492
/**
498493
* Returns the host-owned customization refs this agent currently exposes.

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import * as fs from 'fs';
7+
import { Emitter, Event } from '../../../base/common/event.js';
78
import { Disposable } from '../../../base/common/lifecycle.js';
89
import { dirname } from '../../../base/common/path.js';
910
import { hasKey } from '../../../base/common/types.js';
@@ -36,6 +37,13 @@ export const IAgentConfigurationService = createDecorator<IAgentConfigurationSer
3637
export interface IAgentConfigurationService {
3738
readonly _serviceBrand: undefined;
3839

40+
/**
41+
* Fires whenever a {@link ActionType.RootConfigChanged} action is
42+
* processed by the state manager, signalling that callers should
43+
* re-read any root config values they depend on.
44+
*/
45+
readonly onDidRootConfigChange: Event<void>;
46+
3947
/**
4048
* Returns the effective value of `key` for `session`, walking the
4149
* `session → parent session → host` chain and returning the first
@@ -92,6 +100,9 @@ export class AgentConfigurationService extends Disposable implements IAgentConfi
92100
declare readonly _serviceBrand: undefined;
93101
private _rootConfigWrite = Promise.resolve();
94102

103+
private readonly _onDidRootConfigChange = this._register(new Emitter<void>());
104+
readonly onDidRootConfigChange: Event<void> = this._onDidRootConfigChange.event;
105+
95106
constructor(
96107
private readonly _stateManager: AgentHostStateManager,
97108
@ILogService private readonly _logService: ILogService,
@@ -110,6 +121,12 @@ export class AgentConfigurationService extends Disposable implements IAgentConfi
110121
},
111122
values: { ...existing?.values, ...this._loadPersistedRootConfig() },
112123
};
124+
125+
this._register(this._stateManager.onDidEmitEnvelope(envelope => {
126+
if (envelope.action.type === ActionType.RootConfigChanged) {
127+
this._onDidRootConfigChange.fire();
128+
}
129+
}));
113130
}
114131

115132
getEffectiveValue<D extends SchemaDefinition, K extends keyof D & string>(

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import { FileSystemProviderErrorCode, IFileService, toFileSystemProviderErrorCod
1414
import { InstantiationService } from '../../instantiation/common/instantiationService.js';
1515
import { ServiceCollection } from '../../instantiation/common/serviceCollection.js';
1616
import { ILogService } from '../../log/common/log.js';
17-
import { AgentHostConfigKey, agentHostCustomizationConfigSchema } from '../common/agentHostCustomizationConfig.js';
1817
import { AgentProvider, AgentSession, IAgent, IAgentCreateSessionConfig, IAgentMessageEvent, IAgentResolveSessionConfigParams, IAgentService, IAgentSessionConfigCompletionsParams, IAgentSessionMetadata, IAgentSubagentStartedEvent, IAgentToolCompleteEvent, IAgentToolStartEvent, AuthenticateParams, AuthenticateResult } from '../common/agentService.js';
1918
import { ISessionDataService } from '../common/sessionDataService.js';
2019
import { ActionType, ActionEnvelope, INotification, type IRootConfigChangedAction, type SessionAction, type TerminalAction } from '../common/state/sessionActions.js';
@@ -128,8 +127,6 @@ export class AgentService extends Disposable implements IAgentService {
128127
}
129128
this._logService.info(`Registering agent provider: ${provider.id}`);
130129
this._providers.set(provider.id, provider);
131-
const hostCustomizations = this._configurationService.getRootValue(agentHostCustomizationConfigSchema, AgentHostConfigKey.Customizations) ?? [];
132-
provider.setHostCustomizations?.([...hostCustomizations]);
133130
this._providerSubscriptions.add(this._sideEffects.registerProgressListener(provider));
134131
if (!this._defaultProvider) {
135132
this._defaultProvider = provider.id;

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

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import { URI } from '../../../base/common/uri.js';
1212
import { generateUuid } from '../../../base/common/uuid.js';
1313
import { ILogService } from '../../log/common/log.js';
1414
import { IInstantiationService } from '../../instantiation/common/instantiation.js';
15-
import { AgentHostConfigKey, agentHostCustomizationConfigSchema } from '../common/agentHostCustomizationConfig.js';
1615
import { IAgent, IAgentAttachment, IAgentProgressEvent, type IAgentToolCompleteEvent, type IAgentToolReadyEvent } from '../common/agentService.js';
1716
import { IDiffComputeService } from '../common/diffComputeService.js';
1817
import { ISessionDataService } from '../common/sessionDataService.js';
@@ -26,13 +25,11 @@ import {
2625
ToolResultContentType,
2726
buildSubagentSessionUri,
2827
getToolFileEdits,
29-
type CustomizationRef,
3028
type SessionState,
3129
type ToolResultContent,
3230
type URI as ProtocolURI,
3331
} from '../common/state/sessionState.js';
3432
import { AgentEventMapper } from './agentEventMapper.js';
35-
import { IAgentConfigurationService } from './agentConfigurationService.js';
3633
import { AgentHostStateManager } from './agentHostStateManager.js';
3734
import { NodeWorkerDiffComputeService } from './diffComputeService.js';
3835
import { computeSessionDiffs, type IIncrementalDiffOptions } from './sessionDiffAggregator.js';
@@ -78,8 +75,6 @@ export class AgentSideEffects extends Disposable {
7875
/** Serializes per-session diff computations to avoid races with stale previousDiffs. */
7976
private readonly _diffComputationSequencer = new SequencerByKey<string>();
8077
private _lastAgentInfos: readonly AgentInfo[] = [];
81-
/** Last customizations list passed to agents; compared before each {@link _applyHostCustomizationsToAgents} call to skip no-op updates. */
82-
private _lastAppliedCustomizations: readonly CustomizationRef[] = [];
8378
/** Per-session debounce timers for mid-turn diff computation. */
8479
private readonly _debouncedDiffTimers = this._register(new DisposableMap<string>());
8580
private static readonly _DIFF_DEBOUNCE_MS = 5000;
@@ -110,7 +105,6 @@ export class AgentSideEffects extends Disposable {
110105
private readonly _options: IAgentSideEffectsOptions,
111106
@IInstantiationService instantiationService: IInstantiationService,
112107
@ILogService private readonly _logService: ILogService,
113-
@IAgentConfigurationService private readonly _configurationService: IAgentConfigurationService,
114108
) {
115109
super();
116110
this._diffComputeService = this._register(new NodeWorkerDiffComputeService(this._logService));
@@ -153,20 +147,6 @@ export class AgentSideEffects extends Disposable {
153147
this._stateManager.dispatchServerAction({ type: ActionType.RootAgentsChanged, agents: infos });
154148
}
155149

156-
private _applyHostCustomizationsToAgents(): void {
157-
const customizations = this._configurationService.getRootValue(agentHostCustomizationConfigSchema, AgentHostConfigKey.Customizations) ?? [];
158-
if (equals(customizations, this._lastAppliedCustomizations)) {
159-
return;
160-
}
161-
this._lastAppliedCustomizations = customizations;
162-
for (const agent of this._options.agents.get()) {
163-
agent.setHostCustomizations?.([...customizations], () => {
164-
this._publishAgentInfos(this._options.agents.get());
165-
this._publishSessionCustomizationsForAgent(agent);
166-
});
167-
}
168-
}
169-
170150
private async _publishSessionCustomizations(agent: IAgent, session: ProtocolURI): Promise<void> {
171151
if (!agent.getSessionCustomizations) {
172152
return;
@@ -233,6 +213,12 @@ export class AgentSideEffects extends Disposable {
233213
disposables.add(agent.onDidSessionProgress(e => {
234214
this._handleAgentProgress(agent, agentMapper, e);
235215
}));
216+
if (agent.onDidCustomizationsChange) {
217+
disposables.add(agent.onDidCustomizationsChange(() => {
218+
this._publishAgentInfos(this._options.agents.get());
219+
this._publishSessionCustomizationsForAgent(agent);
220+
}));
221+
}
236222
return disposables;
237223
}
238224

@@ -764,7 +750,10 @@ export class AgentSideEffects extends Disposable {
764750
break;
765751
}
766752
case ActionType.RootConfigChanged: {
767-
this._applyHostCustomizationsToAgents();
753+
// Host customizations are self-managed by each agent's
754+
// PluginController via IAgentConfigurationService.onDidRootConfigChange.
755+
// Republish agent infos for non-customization schema changes
756+
// (e.g. permissions) and session customizations as a catchall.
768757
this._publishAgentInfos(this._options.agents.get());
769758
this._publishAllSessionCustomizations();
770759
break;

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

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { CopilotClient, ResumeSessionConfig, type CopilotClientOptions, type Ses
77
import { rgPath } from '@vscode/ripgrep';
88
import * as fs from 'fs/promises';
99
import { Limiter, SequencerByKey } from '../../../../base/common/async.js';
10-
import { Emitter } from '../../../../base/common/event.js';
10+
import { Emitter, Event } from '../../../../base/common/event.js';
1111
import { appendEscapedMarkdownInlineCode } from '../../../../base/common/htmlContent.js';
1212
import { Disposable, DisposableMap } from '../../../../base/common/lifecycle.js';
1313
import { ResourceMap } from '../../../../base/common/map.js';
@@ -22,7 +22,7 @@ import { IParsedPlugin, parsePlugin } from '../../../agentPlugins/common/pluginP
2222
import { IFileService } from '../../../files/common/files.js';
2323
import { IInstantiationService } from '../../../instantiation/common/instantiation.js';
2424
import { ILogService } from '../../../log/common/log.js';
25-
import { customizationMatchesDirectory } from '../../common/agentHostCustomizationConfig.js';
25+
import { customizationMatchesDirectory, AgentHostConfigKey, agentHostCustomizationConfigSchema } from '../../common/agentHostCustomizationConfig.js';
2626
import { IAgentPluginManager, ISyncedCustomization } from '../../common/agentPluginManager.js';
2727
import { AgentSession, IAgent, IAgentAttachment, IAgentCreateSessionConfig, IAgentCreateSessionResult, IAgentDescriptor, IAgentDeltaEvent, IAgentMessageEvent, IAgentModelInfo, IAgentProgressEvent, IAgentResolveSessionConfigParams, IAgentSessionConfigCompletionsParams, IAgentSessionMetadata, IAgentSessionProjectInfo, IAgentSubagentStartedEvent, IAgentToolCompleteEvent, IAgentToolStartEvent } from '../../common/agentService.js';
2828
import { AutoApproveLevel, ISchemaProperty, createSchema, platformSessionSchema, schemaProperty } from '../../common/agentHostSchema.js';
@@ -33,6 +33,7 @@ import { ProtectedResourceMetadata, SessionCustomizationSource, type ConfigSchem
3333
import { AHP_AUTH_REQUIRED, ProtocolError } from '../../common/state/sessionProtocol.js';
3434
import { CustomizationStatus, CustomizationRef, SessionInputResponseKind, type PendingMessage, type SessionInputAnswer, type ToolCallResult, type PolicyState } from '../../common/state/sessionState.js';
3535
import { IAgentHostGitService } from '../agentHostGitService.js';
36+
import { IAgentConfigurationService } from '../agentConfigurationService.js';
3637
import { IAgentHostTerminalManager } from '../agentHostTerminalManager.js';
3738
import { CopilotAgentSession, SessionWrapperFactory, type IActiveClientSnapshot } from './copilotAgentSession.js';
3839
import { ICopilotSessionContext, projectFromCopilotContext } from './copilotGitProject.js';
@@ -179,6 +180,7 @@ export class CopilotAgent extends Disposable implements IAgent {
179180
private readonly _sessionSequencer = new SequencerByKey<string>();
180181
private _shutdownPromise: Promise<void> | undefined;
181182
private readonly _plugins: PluginController;
183+
readonly onDidCustomizationsChange: Event<void>;
182184
/** Per-session active client state for tools + plugin snapshot tracking. */
183185
private readonly _activeClients = new ResourceMap<ActiveClient>();
184186

@@ -191,7 +193,8 @@ export class CopilotAgent extends Disposable implements IAgent {
191193
@IAgentHostTerminalManager private readonly _terminalManager: IAgentHostTerminalManager,
192194
) {
193195
super();
194-
this._plugins = this._instantiationService.createInstance(PluginController);
196+
this._plugins = this._register(this._instantiationService.createInstance(PluginController));
197+
this.onDidCustomizationsChange = this._plugins.onDidChange;
195198
}
196199

197200
protected _createCopilotClient(options: CopilotClientOptions): ICopilotClient {
@@ -228,10 +231,6 @@ export class CopilotAgent extends Disposable implements IAgent {
228231
return this._plugins.getSessionCustomizations(entry?.customizationDirectory ?? metadata?.customizationDirectory ?? metadata?.workingDirectory);
229232
}
230233

231-
setHostCustomizations(customizations: CustomizationRef[], progress?: () => void): void {
232-
this._plugins.setHostCustomizations(customizations, progress);
233-
}
234-
235234
async authenticate(resource: string, token: string): Promise<boolean> {
236235
if (resource !== 'https://api.github.com') {
237236
return false;
@@ -1216,20 +1215,33 @@ interface IResolvedCustomization {
12161215
readonly plugin?: IParsedPlugin;
12171216
}
12181217

1219-
class PluginController {
1218+
class PluginController extends Disposable {
1219+
private readonly _onDidChange = this._register(new Emitter<void>());
1220+
readonly onDidChange = this._onDidChange.event;
1221+
12201222
private readonly _enablement = new Map<string, boolean>();
12211223
private _clientCustomizations: readonly IResolvedCustomization[] = [];
12221224
private _hostCustomizations: readonly IResolvedCustomization[] = [];
12231225
private _clientSync: Promise<readonly IResolvedCustomization[]> = Promise.resolve([]);
12241226
private _hostSync: Promise<readonly IResolvedCustomization[]> = Promise.resolve([]);
12251227
private _clientRevision = 0;
12261228
private _hostRevision = 0;
1229+
private _lastAppliedRefs: readonly CustomizationRef[] = [];
12271230

12281231
constructor(
12291232
@IAgentPluginManager private readonly _pluginManager: IAgentPluginManager,
12301233
@ILogService private readonly _logService: ILogService,
12311234
@IFileService private readonly _fileService: IFileService,
1232-
) { }
1235+
@IAgentConfigurationService private readonly _configurationService: IAgentConfigurationService,
1236+
) {
1237+
super();
1238+
1239+
// Seed from current root config and subscribe to future changes.
1240+
this._applyHostCustomizations();
1241+
this._register(this._configurationService.onDidRootConfigChange(() => {
1242+
this._applyHostCustomizations();
1243+
}));
1244+
}
12331245

12341246
public getConfiguredHostCustomizations(): readonly CustomizationRef[] {
12351247
return this._hostCustomizations.map(item => item.customization.customization);
@@ -1276,7 +1288,18 @@ class PluginController {
12761288
this._enablement.set(pluginProtocolUri, enabled);
12771289
}
12781290

1279-
public setHostCustomizations(customizations: CustomizationRef[], progress?: () => void): void {
1291+
/**
1292+
* Reads the current host customizations from the root config and
1293+
* resolves them. Skips the update when the configured refs have not
1294+
* changed since the last application.
1295+
*/
1296+
private _applyHostCustomizations(): void {
1297+
const customizations = this._configurationService.getRootValue(agentHostCustomizationConfigSchema, AgentHostConfigKey.Customizations) ?? [];
1298+
if (equals(customizations, this._lastAppliedRefs)) {
1299+
return;
1300+
}
1301+
this._lastAppliedRefs = customizations;
1302+
12801303
const revision = ++this._hostRevision;
12811304
this._hostCustomizations = customizations.map(customization => ({
12821305
customization: {
@@ -1286,15 +1309,15 @@ class PluginController {
12861309
status: CustomizationStatus.Loading,
12871310
},
12881311
}));
1289-
progress?.();
1312+
this._onDidChange.fire();
12901313
this._hostSync = Promise.all(customizations.map(customization => this._resolveConfiguredCustomization(customization, SessionCustomizationSource.Host))).then(resolved => {
12911314
if (revision === this._hostRevision) {
12921315
this._hostCustomizations = resolved;
12931316
}
12941317
return resolved;
12951318
}).finally(() => {
12961319
if (revision === this._hostRevision) {
1297-
progress?.();
1320+
this._onDidChange.fire();
12981321
}
12991322
});
13001323
}

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

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { AgentSession } from '../../common/agentService.js';
2121
import { ISessionDatabase, ISessionDataService } from '../../common/sessionDataService.js';
2222
import { SessionDatabase } from '../../node/sessionDatabase.js';
2323
import { ActionType, ActionEnvelope } from '../../common/state/sessionActions.js';
24-
import { SessionActiveClient, ResponsePartKind, SessionLifecycle, ToolCallConfirmationReason, ToolCallStatus, ToolResultContentType, TurnState, buildSubagentSessionUri, type CustomizationRef, type MarkdownResponsePart, type ToolCallCompletedState, type ToolCallResponsePart } from '../../common/state/sessionState.js';
24+
import { SessionActiveClient, ResponsePartKind, SessionLifecycle, ToolCallConfirmationReason, ToolCallStatus, ToolResultContentType, TurnState, buildSubagentSessionUri, type MarkdownResponsePart, type ToolCallCompletedState, type ToolCallResponsePart } from '../../common/state/sessionState.js';
2525
import { IProductService } from '../../../product/common/productService.js';
2626
import { AgentService } from '../../node/agentService.js';
2727
import { MockAgent } from './mockAgent.js';
@@ -131,11 +131,6 @@ suite('AgentService (node dispatcher)', () => {
131131
const rootConfigResource = joinPath(tempDir, 'agent-host-config.json');
132132
const svc = disposables.add(new AgentService(new NullLogService(), fileService, createNullSessionDataService(), { _serviceBrand: undefined } as IProductService, rootConfigResource));
133133
const agent = new MockAgent('copilot');
134-
const hostCustomizationCalls: CustomizationRef[][] = [];
135-
agent.setHostCustomizations = customizations => {
136-
hostCustomizationCalls.push([...customizations]);
137-
agent.customizations = [...customizations];
138-
};
139134
disposables.add(toDisposable(() => agent.dispose()));
140135
svc.registerProvider(agent);
141136

@@ -147,26 +142,23 @@ suite('AgentService (node dispatcher)', () => {
147142

148143
let persisted = false;
149144
for (let attempt = 0; attempt < 20; attempt++) {
150-
if (hostCustomizationCalls.length > 1) {
151-
try {
152-
const parsed = JSON.parse(readFileSync(rootConfigResource.fsPath, 'utf8'));
153-
assert.deepStrictEqual(
154-
parsed.customizations,
155-
[customization],
156-
);
157-
persisted = true;
158-
break;
159-
} catch {
160-
// Wait for the serialized root-config write to complete.
161-
}
145+
try {
146+
const parsed = JSON.parse(readFileSync(rootConfigResource.fsPath, 'utf8'));
147+
assert.deepStrictEqual(
148+
parsed.customizations,
149+
[customization],
150+
);
151+
persisted = true;
152+
break;
153+
} catch {
154+
// Wait for the serialized root-config write to complete.
162155
}
163156
if (attempt === 19) {
164157
break;
165158
}
166159
await new Promise(resolve => setTimeout(resolve, 5));
167160
}
168161

169-
assert.deepStrictEqual(hostCustomizationCalls.at(-1), [customization]);
170162
assert.ok(persisted, 'should persist the root config change');
171163
} finally {
172164
rmSync(tempDir.fsPath, { recursive: true, force: true });

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -735,9 +735,7 @@ suite('AgentSideEffects', () => {
735735
test('republishes agent and session customizations for existing sessions', async () => {
736736
setupSession('file:///workspace');
737737
const customization = { uri: 'file:///plugin-a', displayName: 'Plugin A' };
738-
agent.setHostCustomizations = customizations => {
739-
agent.customizations = [...customizations];
740-
};
738+
agent.customizations = [customization];
741739
agent.getSessionCustomizations = async () => [{
742740
customization,
743741
source: SessionCustomizationSource.Host,

0 commit comments

Comments
 (0)