mirror of
https://github.com/microsoft/vscode.git
synced 2026-08-16 11:17:35 +01:00
agentHost: defer interactive MCP client registration
Keep automatic MCP authentication probes non-interactive so unsupported dynamic client registration is only surfaced after the user chooses Authenticate. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -77,6 +77,7 @@ Then read the relevant spec for the area you are changing (see table below). If
|
||||
- **Every untitled-session-title fallback must be quick-chat aware**: an untitled session's title observable is `''`, so a hardcoded `localize(…, "New Session")` fallback shows "New Session" even for a quick chat (whose composer says "New Chat"). Route **all** such fallbacks through the shared `getUntitledSessionTitle(isQuickChat)` helper (`services/sessions/common/session.ts`, boolean param so each caller controls reader-tracked `.read(reader)` vs `.get()`). There are ≥5 sites — titlebar (`sessionsTitleBarWidget`), session header (×2: title + rename placeholder), list-row hover (`sessionHoverContent`), sessions picker (`sessionsActions`) — keep them on the helper; never hardcode "New Session". (The Cmd+N *action* title stays "New Session" — that action creates a session, unrelated to a session's own title.)
|
||||
- **`NeedsInput` is still an active turn for live turn UI**: agent-host tool and input confirmations intentionally transition a running chat from `InProgress` to `NeedsInput` without ending `activeTurn`. Live status surfaces such as the chat input pills must use `isActiveSessionStatus` so they do not disappear until the next output returns the chat to `InProgress`.
|
||||
- **Agent-host-only exclusions for built-in client tools belong in `ClientToolSetsContribution`, not the global tool registration**: `AgentHostActiveClientService.getClientTools` advertises enabled members of every non-deprecated tool set, including extension-contributed sets. Omit an unsupported built-in tool from the client tool sets so normal Copilot chat can continue using it; do not treat this contribution as the sole Agent Host allowlist.
|
||||
- **Non-interactive MCP authentication probes must not create dynamic authentication providers**: Provider creation can prompt for manual client registration when dynamic registration is unsupported. With `allowInteraction: false`, only inspect existing providers and sessions; defer metadata discovery and provider creation until the user invokes the `mcpAuthenticationRequired` action.
|
||||
|
||||
## Capturing Feedback (meta-rule)
|
||||
|
||||
|
||||
@@ -279,7 +279,7 @@ export async function resolveMcpServerAuthentication(
|
||||
const scopes = options.scopes;
|
||||
for (const authorizationServer of protectedResource.authorization_servers ?? []) {
|
||||
const authorizationServerUri = URI.parse(authorizationServer);
|
||||
const providerId = await getOrCreateProviderForMcpResource(authorizationServerUri, protectedResource, authenticationService, logService, options.logPrefix);
|
||||
const providerId = await getOrCreateProviderForMcpResource(authorizationServerUri, protectedResource, authenticationService, logService, options.logPrefix, options.allowInteraction);
|
||||
if (!providerId) {
|
||||
continue;
|
||||
}
|
||||
@@ -317,10 +317,11 @@ async function getOrCreateProviderForMcpResource(
|
||||
authenticationService: IAuthenticationService,
|
||||
logService: ILogService,
|
||||
logPrefix: string,
|
||||
allowCreation: boolean,
|
||||
): Promise<string | undefined> {
|
||||
const resourceUri = URI.parse(protectedResource.resource);
|
||||
const existing = await authenticationService.getOrActivateProviderIdForServer(authorizationServer, resourceUri);
|
||||
if (existing) {
|
||||
if (existing || !allowCreation) {
|
||||
return existing;
|
||||
}
|
||||
|
||||
|
||||
@@ -237,6 +237,41 @@ suite('resolveMcpServerAuthentication', () => {
|
||||
requestedScopes: [['notifications']],
|
||||
});
|
||||
});
|
||||
|
||||
test('does not attempt dynamic provider creation without user interaction', async () => {
|
||||
const warnings: string[] = [];
|
||||
const logService = new class extends NullLogService {
|
||||
override warn(message: string): void {
|
||||
warnings.push(message);
|
||||
}
|
||||
}();
|
||||
const instantiationService = disposables.add(new TestInstantiationService());
|
||||
instantiationService.stub(IAuthenticationService, createMockAuthService({}));
|
||||
instantiationService.stub(IAuthenticationMcpAccessService, {});
|
||||
instantiationService.stub(IAuthenticationMcpService, {
|
||||
getAccountPreference: () => undefined,
|
||||
});
|
||||
instantiationService.stub(IAuthenticationMcpUsageService, {});
|
||||
instantiationService.stub(ILogService, logService);
|
||||
|
||||
const result = await instantiationService.invokeFunction(resolveMcpServerAuthentication, {
|
||||
resource: 'https://mcp.example.com',
|
||||
authorization_servers: ['not-a-valid-authorization-server'],
|
||||
}, {
|
||||
allowInteraction: false,
|
||||
logPrefix: '[AgentHost]',
|
||||
mcpServerId: 'server-id',
|
||||
mcpServerName: 'Example',
|
||||
mcpServerUrl: 'https://mcp.example.com',
|
||||
scopes: [],
|
||||
authenticate: async () => { },
|
||||
});
|
||||
|
||||
assert.deepStrictEqual({ result, warnings }, {
|
||||
result: false,
|
||||
warnings: [],
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
suite('authenticateProtectedResources', () => {
|
||||
|
||||
Reference in New Issue
Block a user