Skip to content

Commit abdc25a

Browse files
authored
Merge pull request #311129 from microsoft/connor4312/eager-activeclient
agentHost: adopt eager activeClient announcement
2 parents de172fb + 835f4c6 commit abdc25a

13 files changed

Lines changed: 258 additions & 33 deletions

File tree

src/vs/platform/agentHost/browser/remoteAgentHostProtocolClient.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ export class RemoteAgentHostProtocolClient extends Disposable implements IAgentC
188188
model: config?.model,
189189
workingDirectory: config?.workingDirectory ? fromAgentHostUri(config.workingDirectory).toString() : undefined,
190190
config: config?.config,
191+
activeClient: config?.activeClient,
191192
});
192193
return session;
193194
}

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { createDecorator } from '../../instantiation/common/instantiation.js';
1212
import type { ISyncedCustomization } from './agentPluginManager.js';
1313
import type { IAgentSubscription } from './state/agentSubscription.js';
1414
import type { ICreateTerminalParams, IResolveSessionConfigResult, ISessionConfigCompletionsResult } from './state/protocol/commands.js';
15-
import { IProtectedResourceMetadata, type IConfigSchema, type IFileEdit, type IModelSelection, type IToolDefinition } from './state/protocol/state.js';
15+
import { IProtectedResourceMetadata, type IConfigSchema, type IFileEdit, type IModelSelection, type ISessionActiveClient, type IToolDefinition } from './state/protocol/state.js';
1616
import type { IActionEnvelope, INotification, ISessionAction, ITerminalAction } from './state/sessionActions.js';
1717
import type { IResourceCopyParams, IResourceCopyResult, IResourceDeleteParams, IResourceDeleteResult, IResourceListResult, IResourceMoveParams, IResourceMoveResult, IResourceReadResult, IResourceWriteParams, IResourceWriteResult, IStateSnapshot } from './state/sessionProtocol.js';
1818
import { AttachmentType, ComponentToState, SessionInputResponseKind, SessionStatus, StateComponents, type ICustomizationRef, type IPendingMessage, type IRootState, type ISessionInputAnswer, type ISessionInputRequest, type IToolCallResult, type IToolResultContent, type PolicyState, type StringOrMarkdown } from './state/sessionState.js';
@@ -125,6 +125,14 @@ export interface IAgentCreateSessionConfig {
125125
readonly session?: URI;
126126
readonly workingDirectory?: URI;
127127
readonly config?: Record<string, string>;
128+
/**
129+
* Eagerly claim the active client role for the new session. When provided,
130+
* the server initializes the session with this client as the active
131+
* client, equivalent to dispatching a `session/activeClientChanged`
132+
* action immediately after creation. The `clientId` MUST match the
133+
* connection's own `clientId`.
134+
*/
135+
readonly activeClient?: ISessionActiveClient;
128136
/** Fork from an existing session at a specific turn. */
129137
readonly fork?: {
130138
readonly session: URI;
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
ab467b2
1+
0947b17

src/vs/platform/agentHost/common/state/protocol/commands.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
// allow-any-unicode-comment-file
77
// DO NOT EDIT -- auto-generated by scripts/sync-agent-host-protocol.ts
88

9-
import type { URI, ISnapshot, ISessionConfigSchema, ISessionSummary, IModelSelection, ITurn, ITerminalClaim } from './state.js';
9+
import type { URI, ISnapshot, ISessionConfigSchema, ISessionSummary, IModelSelection, ITurn, ITerminalClaim, ISessionActiveClient } from './state.js';
1010
import type { IActionEnvelope, IStateAction } from './actions.js';
1111

1212
export type { IConfigPropertySchema, IConfigSchema, ISessionConfigPropertySchema, ISessionConfigSchema } from './state.js';
@@ -198,6 +198,15 @@ export interface ICreateSessionParams {
198198
* Keys and values correspond to the schema returned by the server.
199199
*/
200200
config?: Record<string, string>;
201+
/**
202+
* Eagerly claim the active client role for the new session.
203+
*
204+
* When provided, the server initializes the session with this client as the
205+
* active client, equivalent to dispatching a `session/activeClientChanged`
206+
* action immediately after creation. The `clientId` MUST match the
207+
* `clientId` the creating client supplied in `initialize`.
208+
*/
209+
activeClient?: ISessionActiveClient;
201210
}
202211

203212
// ─── disposeSession ──────────────────────────────────────────────────────────

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,7 @@ export class AgentService extends Disposable implements IAgentService {
263263
const state = this._stateManager.createSession(summary);
264264
state.config = sessionConfig;
265265
state.turns = sourceTurns;
266+
state.activeClient = config.activeClient;
266267
} else {
267268
// Create empty state for new sessions
268269
const summary: ISessionSummary = {
@@ -278,6 +279,7 @@ export class AgentService extends Disposable implements IAgentService {
278279
};
279280
const state = this._stateManager.createSession(summary);
280281
state.config = sessionConfig;
282+
state.activeClient = config?.activeClient;
281283
}
282284
// Persist initial config values so a subsequent `restoreSession` can
283285
// re-hydrate them. We persist the full resolved values (not just the

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,15 @@ export class CopilotAgent extends Disposable implements IAgent {
495495

496496
const sessionId = config?.session ? AgentSession.id(config.session) : generateUuid();
497497
const sessionUri = AgentSession.uri(this.id, sessionId);
498+
let seededActiveClient = false;
499+
if (config?.activeClient) {
500+
const ac = this._getOrCreateActiveClient(sessionUri);
501+
seededActiveClient = true;
502+
ac.updateTools(config.activeClient.clientId, config.activeClient.tools);
503+
if (config.activeClient.customizations !== undefined) {
504+
await this._plugins.sync(config.activeClient.clientId, config.activeClient.customizations);
505+
}
506+
}
498507
const activeClient = this._activeClients.get(sessionUri);
499508
const snapshot = activeClient ? await activeClient.snapshot() : undefined;
500509
const workingDirectory = await this._resolveSessionWorkingDirectory(config, sessionId);
@@ -518,6 +527,9 @@ export class CopilotAgent extends Disposable implements IAgent {
518527
agentSession = this._createAgentSession(factory, sessionId, shellManager, snapshot);
519528
await agentSession.initializeSession();
520529
} catch (error) {
530+
if (seededActiveClient) {
531+
this._activeClients.delete(sessionUri);
532+
}
521533
await this._removeCreatedWorktree(sessionId);
522534
throw error;
523535
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
isJsonRpcNotification,
2424
isJsonRpcRequest,
2525
JSON_RPC_INTERNAL_ERROR,
26+
JsonRpcErrorCodes,
2627
ProtocolError,
2728
type IAhpServerNotification,
2829
type IInitializeParams,
@@ -349,6 +350,11 @@ export class ProtocolServerHandler extends Disposable {
349350
}
350351
fork = { session: URI.parse(params.fork.session), turnIndex, turnId: params.fork.turnId };
351352
}
353+
// If the client eagerly claimed the active client role, validate
354+
// the clientId matches the connection before forwarding.
355+
if (params.activeClient && params.activeClient.clientId !== _client.clientId) {
356+
throw new ProtocolError(JsonRpcErrorCodes.InvalidParams, `createSession.activeClient.clientId must match the connection's clientId`);
357+
}
352358
try {
353359
createdSession = await this._agentService.createSession({
354360
provider: params.provider,
@@ -357,6 +363,7 @@ export class ProtocolServerHandler extends Disposable {
357363
session: URI.parse(params.session),
358364
fork,
359365
config: params.config,
366+
activeClient: params.activeClient,
360367
});
361368
} catch (err) {
362369
if (err instanceof ProtocolError) {

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

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { AgentSession } from '../../common/agentService.js';
1919
import { ISessionDatabase, ISessionDataService } from '../../common/sessionDataService.js';
2020
import { SessionDatabase } from '../../node/sessionDatabase.js';
2121
import { ActionType, IActionEnvelope } from '../../common/state/sessionActions.js';
22-
import { ResponsePartKind, SessionLifecycle, ToolCallConfirmationReason, ToolCallStatus, ToolResultContentType, TurnState, buildSubagentSessionUri, type IMarkdownResponsePart, type IToolCallCompletedState, type IToolCallResponsePart } from '../../common/state/sessionState.js';
22+
import { ISessionActiveClient, ResponsePartKind, SessionLifecycle, ToolCallConfirmationReason, ToolCallStatus, ToolResultContentType, TurnState, buildSubagentSessionUri, type IMarkdownResponsePart, type IToolCallCompletedState, type IToolCallResponsePart } from '../../common/state/sessionState.js';
2323
import { IProductService } from '../../../product/common/productService.js';
2424
import { AgentService } from '../../node/agentService.js';
2525
import { MockAgent } from './mockAgent.js';
@@ -248,6 +248,36 @@ suite('AgentService (node dispatcher)', () => {
248248

249249
assert.deepStrictEqual(service.stateManager.getSessionState(session.toString())?.config?.values, config);
250250
});
251+
252+
test('seeds activeClient into the initial session state when provided', async () => {
253+
service.registerProvider(copilotAgent);
254+
255+
const envelopes: IActionEnvelope[] = [];
256+
disposables.add(service.onDidAction(env => envelopes.push(env)));
257+
258+
const activeClient: ISessionActiveClient = {
259+
clientId: 'client-eager',
260+
tools: [{ name: 't1', description: 'd', inputSchema: { type: 'object' } }],
261+
customizations: [{ uri: 'file:///plugin-a', displayName: 'A' }],
262+
};
263+
const session = await service.createSession({ provider: 'copilot', activeClient });
264+
265+
assert.deepStrictEqual({
266+
activeClient: service.stateManager.getSessionState(session.toString())?.activeClient,
267+
dispatchedActiveClientChanged: envelopes.some(e => e.action.type === ActionType.SessionActiveClientChanged),
268+
}, {
269+
activeClient,
270+
dispatchedActiveClientChanged: false,
271+
});
272+
});
273+
274+
test('omits activeClient from the initial session state when not provided', async () => {
275+
service.registerProvider(copilotAgent);
276+
277+
const session = await service.createSession({ provider: 'copilot' });
278+
279+
assert.strictEqual(service.stateManager.getSessionState(session.toString())?.activeClient, undefined);
280+
});
251281
});
252282

253283
// ---- authenticate ---------------------------------------------------

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

Lines changed: 72 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,14 +188,14 @@ class TestableCopilotAgent extends CopilotAgent {
188188
}
189189
}
190190

191-
function createTestAgentContext(disposables: Pick<DisposableStore, 'add'>, options?: { sessionDataService?: ISessionDataService; copilotClient?: ICopilotClient; gitService?: TestAgentHostGitService; environmentServiceRegistration?: 'native' | 'none' }): { agent: CopilotAgent; instantiationService: IInstantiationService } {
191+
function createTestAgentContext(disposables: Pick<DisposableStore, 'add'>, options?: { sessionDataService?: ISessionDataService; copilotClient?: ICopilotClient; gitService?: TestAgentHostGitService; environmentServiceRegistration?: 'native' | 'none'; pluginManager?: IAgentPluginManager }): { agent: CopilotAgent; instantiationService: IInstantiationService } {
192192
const services = new ServiceCollection();
193193
const logService = new NullLogService();
194194
const fileService = disposables.add(new FileService(logService));
195195
services.set(ILogService, logService);
196196
services.set(IFileService, fileService);
197197
services.set(ISessionDataService, options?.sessionDataService ?? createNullSessionDataService());
198-
services.set(IAgentPluginManager, new TestAgentPluginManager());
198+
services.set(IAgentPluginManager, options?.pluginManager ?? new TestAgentPluginManager());
199199
services.set(IAgentHostGitService, options?.gitService ?? new TestAgentHostGitService());
200200
services.set(IAgentHostTerminalManager, new TestAgentHostTerminalManager());
201201
if (options?.environmentServiceRegistration !== 'none') {
@@ -213,7 +213,7 @@ function createTestAgentContext(disposables: Pick<DisposableStore, 'add'>, optio
213213
return { agent, instantiationService };
214214
}
215215

216-
function createTestAgent(disposables: Pick<DisposableStore, 'add'>, options?: { sessionDataService?: ISessionDataService; copilotClient?: ICopilotClient; gitService?: TestAgentHostGitService; environmentServiceRegistration?: 'native' | 'none' }): CopilotAgent {
216+
function createTestAgent(disposables: Pick<DisposableStore, 'add'>, options?: { sessionDataService?: ISessionDataService; copilotClient?: ICopilotClient; gitService?: TestAgentHostGitService; environmentServiceRegistration?: 'native' | 'none'; pluginManager?: IAgentPluginManager }): CopilotAgent {
217217
return createTestAgentContext(disposables, options).agent;
218218
}
219219

@@ -386,6 +386,75 @@ suite('CopilotAgent', () => {
386386
}
387387
});
388388

389+
suite('createSession activeClient eager-claim', () => {
390+
391+
class SpyingPluginManager extends TestAgentPluginManager {
392+
public readonly calls: { clientId: string; customizations: ICustomizationRef[] }[] = [];
393+
394+
override async syncCustomizations(clientId: string, customizations: ICustomizationRef[], _progress?: (status: ISessionCustomization[]) => void): Promise<ISyncedCustomization[]> {
395+
this.calls.push({ clientId, customizations: [...customizations] });
396+
return [];
397+
}
398+
}
399+
400+
test('createSession seeds activeClient tools and syncs customizations', async () => {
401+
const sessionDataService = disposables.add(new TestSessionDataService());
402+
const client = new TestCopilotClient([]);
403+
const pluginManager = new SpyingPluginManager();
404+
// Fail fast inside the SDK factory so we don't need to wire up a
405+
// real raw session. The seeding of activeClient and the plugin
406+
// sync both happen before `client.createSession` is invoked.
407+
client.createSession = async () => { throw new Error('sentinel'); };
408+
409+
const agent = createTestAgent(disposables, { sessionDataService, copilotClient: client, pluginManager });
410+
try {
411+
await agent.authenticate('https://api.github.com', 'token');
412+
413+
const customizations: ICustomizationRef[] = [{ uri: 'file:///plugin-a', displayName: 'Plugin A' }];
414+
await assert.rejects(
415+
agent.createSession({
416+
session: AgentSession.uri('copilot', 'test-session'),
417+
workingDirectory: URI.file('/workspace'),
418+
activeClient: {
419+
clientId: 'client-1',
420+
tools: [{ name: 't1', description: 'd', inputSchema: { type: 'object' } }],
421+
customizations,
422+
},
423+
}),
424+
(err: Error) => /sentinel/.test(err.message),
425+
);
426+
427+
assert.deepStrictEqual(pluginManager.calls, [{ clientId: 'client-1', customizations }]);
428+
} finally {
429+
await disposeAgent(agent);
430+
}
431+
});
432+
433+
test('createSession without activeClient does not sync customizations', async () => {
434+
const sessionDataService = disposables.add(new TestSessionDataService());
435+
const client = new TestCopilotClient([]);
436+
const pluginManager = new SpyingPluginManager();
437+
client.createSession = async () => { throw new Error('sentinel'); };
438+
439+
const agent = createTestAgent(disposables, { sessionDataService, copilotClient: client, pluginManager });
440+
try {
441+
await agent.authenticate('https://api.github.com', 'token');
442+
443+
await assert.rejects(
444+
agent.createSession({
445+
session: AgentSession.uri('copilot', 'test-session-2'),
446+
workingDirectory: URI.file('/workspace'),
447+
}),
448+
(err: Error) => /sentinel/.test(err.message),
449+
);
450+
451+
assert.deepStrictEqual(pluginManager.calls, []);
452+
} finally {
453+
await disposeAgent(agent);
454+
}
455+
});
456+
});
457+
389458
suite('worktree announcement', () => {
390459
// Drives a real session through worktree creation (calling the
391460
// agent's _resolveSessionWorkingDirectory via a test seam so we don't

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ class MockAgentService implements IAgentService {
7272
readonly browsedUris: URI[] = [];
7373
readonly browseErrors = new Map<string, Error>();
7474
readonly listedSessions: IAgentSessionMetadata[] = [];
75+
readonly createSessionConfigs: (IAgentCreateSessionConfig | undefined)[] = [];
7576

7677
private readonly _onDidAction = new Emitter<import('../../common/state/sessionActions.js').IActionEnvelope>();
7778
readonly onDidAction = this._onDidAction.event;
@@ -91,6 +92,7 @@ class MockAgentService implements IAgentService {
9192
this._stateManager.dispatchClientAction(action, origin);
9293
}
9394
async createSession(config?: IAgentCreateSessionConfig): Promise<URI> {
95+
this.createSessionConfigs.push(config);
9496
const session = config?.session ?? URI.parse('copilot:///new-session');
9597
this._stateManager.createSession({
9698
resource: session.toString(),
@@ -577,4 +579,62 @@ suite('ProtocolServerHandler', () => {
577579
transport2.simulateClose();
578580
assert.deepStrictEqual(counts, [1, 1, 0]);
579581
});
582+
583+
// ---- createSession activeClient -------------------------------------
584+
585+
suite('createSession activeClient', () => {
586+
587+
test('forwards activeClient to the agent service', async () => {
588+
const newSession = URI.parse('copilot:///eager-session').toString();
589+
590+
const transport = connectClient('client-1');
591+
transport.sent.length = 0;
592+
593+
const responsePromise = waitForResponse(transport, 2);
594+
transport.simulateMessage(request(2, 'createSession', {
595+
session: newSession,
596+
provider: 'copilot',
597+
activeClient: {
598+
clientId: 'client-1',
599+
tools: [{ name: 't1', description: 'd', inputSchema: { type: 'object' } }],
600+
customizations: [{ uri: 'file:///plugin-a', displayName: 'A' }],
601+
},
602+
}));
603+
const resp = await responsePromise as { result?: unknown; error?: unknown };
604+
605+
assert.strictEqual(resp.error, undefined, 'createSession should succeed');
606+
const config = agentService.createSessionConfigs.at(-1);
607+
assert.deepStrictEqual({
608+
clientId: config?.activeClient?.clientId,
609+
toolName: config?.activeClient?.tools[0]?.name,
610+
customizationUri: config?.activeClient?.customizations?.[0].uri,
611+
}, {
612+
clientId: 'client-1',
613+
toolName: 't1',
614+
customizationUri: 'file:///plugin-a',
615+
});
616+
});
617+
618+
test('rejects createSession when activeClient.clientId mismatches', async () => {
619+
const newSession = URI.parse('copilot:///mismatch-session').toString();
620+
621+
const transport = connectClient('client-1');
622+
transport.sent.length = 0;
623+
624+
const responsePromise = waitForResponse(transport, 2);
625+
transport.simulateMessage(request(2, 'createSession', {
626+
session: newSession,
627+
provider: 'copilot',
628+
activeClient: {
629+
clientId: 'other-client',
630+
tools: [],
631+
},
632+
}));
633+
const resp = await responsePromise as { result?: unknown; error?: { code: number; message: string } };
634+
635+
assert.ok(resp.error, 'response should be an error');
636+
assert.strictEqual(resp.result, undefined);
637+
assert.strictEqual(agentService.createSessionConfigs.length, 0, 'agent service should not have been called');
638+
});
639+
});
580640
});

0 commit comments

Comments
 (0)