[cherry-pick] agentHost: bind SDK customization operations to the ver… · microsoft/vscode@c53625e · GitHub
Skip to content

Commit c53625e

Browse files
authored
[cherry-pick] agentHost: bind SDK customization operations to the verified policy session (#342629)
Cherry-pick of #342613 from `main`. <!-- Thank you for submitting a Pull Request. Please: * Read our Pull Request guidelines: https://github.com/microsoft/vscode/wiki/How-to-Contribute#pull-requests * Associate an issue with the Pull Request. * Ensure that the code is up-to-date with the `main` branch. * Include a description of the proposed changes and how to test them. --> ## Problem While a Copilot chat is open, the customization marketplace can't load installation receipts. The renderer logs this many times per second: ``` [AgentHostCustomizationMarketplace] Unable to load session-bound SDK installation receipts The SDK could not list installed MCP servers. (refused: policy-context-unavailable) ``` ## Root cause `CopilotCustomizationInstallations` used the open chat's SDK session as `policySessionId` for `list`, `recover`, `search`, and install/uninstall planning. It used the hidden policy session only when no chat was open. The Copilot runtime admits installation and inventory requests only when the policy session's GitHub credential is **verified**: a github.com login backed by a resolved Copilot user record (`mcp_installation` `authority()` → `installation_account_fingerprint()`). A credential passed at `createSession` (`gitHubToken` / `gitHubTokenProvider`) is never verified. Only `session.rpc.gitHubAuth.setCredentials` verifies it. A standalone SDK repro against runtimes 1.0.92-0 through 1.0.100-1 showed `refused: policy-context-unavailable` with the createSession token, even after a chat turn, and `listed` right after `setCredentials` on the same session. #341648 added `setCredentials` to the hidden policy session. Chat sessions get it only when the token changes while they're live. Chats created after the last token change, and provider-backed chats, are never verified. ## Fix Search, inventory, recovery, and install/uninstall planning now always use the hidden policy session (`getOrCreatePolicySessionId`). None of these operations needs the chat's identity: - **Receipts:** the runtime scopes them by Copilot home plus account and host fingerprints, not by session or working directory, so results are the same. - **Feature flags and managed settings:** the hidden session has the same catalog-installation feature flags and managed settings as chat sessions. - **Confirmation:** the client-level `installationConfirmationHandler` matches on the `policySessionId` that planned the operation. Plan and apply now always use the same hidden session, so matching is unchanged. - **Cursors and selections:** they stay keyed by `policySessionId`. They're now invalidated when the hidden session is replaced (auth change, client restart, idle disposal) rather than when you switch chats. The existing re-search recovery path handles that. The visible-session getter and the no-factory path are removed. That path produced "Start a Copilot agent session before managing SDK installations." and `unavailable: session`. The only production caller (`CopilotAgent`) always provides the hidden-session factory, so only tests could reach it. The factory is now required. `releasePolicySession` and the 10-minute idle disposal are unchanged. `AI_CUSTOMIZATIONS.md` is updated. ## Tests - `copilotCustomizationInstallations.test.ts`: tests that relied on a fixed chat session ID now use a `TestPolicySessions` helper that records created sessions. A new test checks that search, list, recover, and prepare all bind to one hidden session. - `copilotAgent.test.ts`: a new test opens a live chat and checks that `listCustomizationInstallations` goes to the hidden session, which received `setCredentials`, and not to the chat's session. ## Validation - `npm run typecheck-client`: passed - `npm run eslint -- <changed files>`: passed - `./scripts/test.sh --run src/vs/platform/agentHost/test/node/copilotCustomizationInstallations.test.ts --run src/vs/platform/agentHost/test/node/copilotSessionLauncher.test.ts --run src/vs/platform/agentHost/test/node/copilotAgent.test.ts`: 1012 passing, 1 pending, 1 failing. The failure is `isolates peer runtimes, preserves local adoption identity and refuses revoked prepared work`, which also fails on unmodified `main` (`e1a22a1ad40`).
2 parents 92ebaa7 + 19b75d1 commit c53625e

5 files changed

Lines changed: 353 additions & 98 deletions

File tree

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

Lines changed: 10 additions & 9 deletions

‎src/vs/platform/agentHost/node/copilot/customizations/copilotCustomizationInstallations.ts‎

Lines changed: 34 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,12 @@ class PendingCustomizationInstallation extends Disposable {
155155
readonly id: string,
156156
readonly client: ICopilotCustomizationInstallationClient,
157157
readonly sdkOperationId: string,
158-
readonly policySessionId: string,
158+
/**
159+
* The hidden policy session the SDK planned this operation in and must
160+
* apply it in, or `undefined` for configuration-only operations that do
161+
* not use a policy session.
162+
*/
163+
readonly policySessionId: string | undefined,
159164
readonly review: IAgentCustomizationInstallationReview,
160165
readonly validatesConfirmation: (request: InstallationConfirmationRequest) => boolean,
161166
readonly apply: () => Promise<ISdkResult | undefined>,
@@ -186,27 +191,31 @@ export class CopilotCustomizationInstallations extends Disposable {
186191
return operation?.approved && operation.validatesConfirmation(request) ? 'confirm' : 'cancel';
187192
};
188193

194+
/**
195+
* @param createPolicySession Creates the hidden policy session that binds
196+
* every SDK catalog, inventory, and installation request. The runtime admits
197+
* these requests only when the policy session's GitHub credential was
198+
* verified through `gitHubAuth.setCredentials`; a credential supplied at
199+
* session creation is never verified. Chat sessions are only verified if a
200+
* token change reaches them while live, so they are not guaranteed to have a
201+
* verified credential and must not be used as the policy session.
202+
*/
189203
constructor(
190204
private readonly getClient: () => Promise<ICopilotCustomizationInstallationClient>,
191-
private readonly getPolicySessionId: (session: URI) => string | undefined,
192-
private readonly createPolicySession?: (client: ICopilotCustomizationInstallationClient, sessionId: string) => Promise<ICopilotCustomizationPolicySession>,
205+
private readonly createPolicySession: (client: ICopilotCustomizationInstallationClient, sessionId: string) => Promise<ICopilotCustomizationPolicySession>,
193206
) {
194207
super();
195208
}
196209

197-
async search(session: URI, request: IAgentCustomizationMarketplaceSearchRequest): Promise<IAgentCustomizationMarketplaceSearchResult> {
210+
async search(request: IAgentCustomizationMarketplaceSearchRequest): Promise<IAgentCustomizationMarketplaceSearchResult> {
198211
if (!request.query.trim()) {
199212
if (request.mediaType && request.mediaType !== copilotPluginMediaType) {
200213
return { kind: 'page', items: [] };
201214
}
202215
return this.browseFeaturedPlugins(await this.getClient(), request);
203216
}
204-
const visiblePolicySessionId = this.getPolicySessionId(session);
205-
if (!visiblePolicySessionId && !this.createPolicySession) {
206-
return { kind: 'unavailable', reason: 'session' };
207-
}
208217
const client = await this.getClient();
209-
const policySessionId = visiblePolicySessionId ?? await this.getOrCreatePolicySessionId(client);
218+
const policySessionId = await this.getOrCreatePolicySessionId(client);
210219
const retainedCursor = request.cursor ? this.catalogCursors.deleteAndLeak(request.cursor) : undefined;
211220
if (request.cursor && (!retainedCursor || retainedCursor.client !== client || retainedCursor.policySessionId !== policySessionId)) {
212221
retainedCursor?.dispose();
@@ -291,10 +300,9 @@ export class CopilotCustomizationInstallations extends Disposable {
291300
};
292301
}
293302

294-
async list(session: URI): Promise<readonly IAgentCustomizationInstallation[]> {
295-
const visiblePolicySessionId = this.getVisiblePolicySessionId(session);
303+
async list(): Promise<readonly IAgentCustomizationInstallation[]> {
296304
const client = await this.getClient();
297-
const policySessionId = visiblePolicySessionId ?? await this.getOrCreatePolicySessionId(client);
305+
const policySessionId = await this.getOrCreatePolicySessionId(client);
298306
const [listedSkills, listedMcp, configured] = await Promise.all([
299307
client.rpc.skills.installations.list({ contract: skillInstallationContract, policySessionId }),
300308
client.rpc.mcp.installations.list({ contract: mcpInstallationContract, policySessionId }),
@@ -314,10 +322,9 @@ export class CopilotCustomizationInstallations extends Disposable {
314322
];
315323
}
316324

317-
async recover(session: URI): Promise<readonly IAgentCustomizationInstallation[]> {
318-
const visiblePolicySessionId = this.getVisiblePolicySessionId(session);
325+
async recover(): Promise<readonly IAgentCustomizationInstallation[]> {
319326
const client = await this.getClient();
320-
const policySessionId = visiblePolicySessionId ?? await this.getOrCreatePolicySessionId(client);
327+
const policySessionId = await this.getOrCreatePolicySessionId(client);
321328
const [skills, mcp, configured] = await Promise.all([
322329
client.rpc.skills.installations.recover({ contract: skillInstallationContract, policySessionId }),
323330
client.rpc.mcp.installations.recover({ contract: mcpInstallationContract, policySessionId }),
@@ -336,10 +343,9 @@ export class CopilotCustomizationInstallations extends Disposable {
336343
await client.rpc.mcp.config.add({ name, config: toCopilotMcpServerConfiguration(config) });
337344
}
338345

339-
async prepare(session: URI, request: IAgentCustomizationInstallationRequest | { readonly installationId: string }): Promise<IAgentCustomizationInstallationReview> {
340-
const visiblePolicySessionId = this.getVisiblePolicySessionId(session);
346+
async prepare(request: IAgentCustomizationInstallationRequest | { readonly installationId: string }): Promise<IAgentCustomizationInstallationReview> {
341347
const client = await this.getClient();
342-
const policySessionId = visiblePolicySessionId ?? await this.getOrCreatePolicySessionId(client);
348+
const policySessionId = await this.getOrCreatePolicySessionId(client);
343349
return hasKey(request, { installationId: true })
344350
? this.prepareUninstall(client, policySessionId, request.installationId)
345351
: this.prepareInstall(client, policySessionId, request);
@@ -350,7 +356,10 @@ export class CopilotCustomizationInstallations extends Disposable {
350356
if (!operation) {
351357
throw new Error(localize('copilot.customizationInstallation.expired', "This customization installation review expired. Start the operation again."));
352358
}
353-
if (operation.client !== await this.getClient()) {
359+
const client = await this.getClient();
360+
// A token change, telemetry revocation, or idle timeout deletes the hidden
361+
// policy session; the SDK cannot apply a plan bound to a deleted session.
362+
if (operation.client !== client || (operation.policySessionId !== undefined && operation.policySessionId !== this.policySession?.sessionId)) {
354363
this.operations.deleteAndDispose(operationId);
355364
throw new Error(localize('copilot.customizationInstallation.runtimeChanged', "The customization runtime restarted. Start the operation again."));
356365
}
@@ -513,7 +522,7 @@ export class CopilotCustomizationInstallations extends Disposable {
513522
}
514523
const configuration = planned.plan.configurationChanges[planned.plan.transportChoices.indexOf(choice)];
515524
if (source.kind === 'card') {
516-
return this.prepareMcpCardConfiguration(client, policySessionId, request, planned.plan, choice, configuration);
525+
return this.prepareMcpCardConfiguration(client, request, planned.plan, choice, configuration);
517526
}
518527
const prepared = await client.rpc.mcp.prepareInstall({
519528
contract: mcpInstallationContract,
@@ -573,7 +582,6 @@ export class CopilotCustomizationInstallations extends Disposable {
573582
/** Provisional direct-card fallback until https://github.com/github/copilot-agent-runtime/issues/26495 is available. */
574583
private prepareMcpCardConfiguration(
575584
client: ICopilotCustomizationInstallationClient,
576-
policySessionId: string,
577585
request: IAgentCustomizationInstallationRequest,
578586
plan: Extract<Awaited<ReturnType<CopilotClient['rpc']['mcp']['planInstall']>>, { readonly kind: 'planned' }>['plan'],
579587
choice: Extract<typeof plan.transportChoices[number], { readonly installMethod: 'remote' }>,
@@ -607,7 +615,7 @@ export class CopilotCustomizationInstallations extends Disposable {
607615
operationId,
608616
client,
609617
generateUuid(),
610-
policySessionId,
618+
undefined,
611619
review,
612620
() => false,
613621
async () => {
@@ -634,7 +642,7 @@ export class CopilotCustomizationInstallations extends Disposable {
634642
if (!Object.hasOwn(configured.servers, serverName)) {
635643
throw new Error(localize('copilot.customizationInstallation.notFound', "The SDK no longer owns this customization installation."));
636644
}
637-
return this.prepareConfiguredMcpUninstall(client, policySessionId, serverName);
645+
return this.prepareConfiguredMcpUninstall(client, serverName);
638646
}
639647
const inventory = await this.listWithClient(client, policySessionId);
640648
const installation = inventory.find(candidate => candidate.installationId === installationId);
@@ -712,7 +720,7 @@ export class CopilotCustomizationInstallations extends Disposable {
712720
return review;
713721
}
714722

715-
private prepareConfiguredMcpUninstall(client: ICopilotCustomizationInstallationClient, policySessionId: string, serverName: string): IAgentCustomizationInstallationReview {
723+
private prepareConfiguredMcpUninstall(client: ICopilotCustomizationInstallationClient, serverName: string): IAgentCustomizationInstallationReview {
716724
const operationId = generateUuid();
717725
const review: IAgentCustomizationInstallationReview = {
718726
operationId,
@@ -730,7 +738,7 @@ export class CopilotCustomizationInstallations extends Disposable {
730738
operationId,
731739
client,
732740
generateUuid(),
733-
policySessionId,
741+
undefined,
734742
review,
735743
() => false,
736744
async () => {
@@ -909,17 +917,6 @@ export class CopilotCustomizationInstallations extends Disposable {
909917
this.operations.set(operation.id, operation);
910918
}
911919

912-
private getVisiblePolicySessionId(session: URI): string | undefined {
913-
const policySessionId = this.getPolicySessionId(session);
914-
if (policySessionId) {
915-
return policySessionId;
916-
}
917-
if (!this.createPolicySession) {
918-
throw new Error(localize('copilot.customizationInstallation.sessionRequired', "Start a Copilot agent session before managing SDK installations."));
919-
}
920-
return undefined;
921-
}
922-
923920
private async getOrCreatePolicySessionId(client: ICopilotCustomizationInstallationClient): Promise<string> {
924921
let current = this.policySession;
925922
if (current?.client === client) {
@@ -934,7 +931,7 @@ export class CopilotCustomizationInstallations extends Disposable {
934931
const generation = ++this.policySessionGeneration;
935932
const sessionId = generateUuid();
936933
this.policySessionIds.add(sessionId);
937-
creating = this.createPolicySession!(client, sessionId).then(async created => {
934+
creating = this.createPolicySession(client, sessionId).then(async created => {
938935
if (created.sessionId !== sessionId || generation !== this.policySessionGeneration) {
939936
try {
940937
await created.dispose();

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

Lines changed: 64 additions & 0 deletions

0 commit comments

Comments
 (0)