diff --git a/.github/skills/sessions/SKILL.md b/.github/skills/sessions/SKILL.md index e314bce7a91..202bc218180 100644 --- a/.github/skills/sessions/SKILL.md +++ b/.github/skills/sessions/SKILL.md @@ -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) diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostAuth.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostAuth.ts index 3bed9355e25..e635de18907 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostAuth.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostAuth.ts @@ -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 { const resourceUri = URI.parse(protectedResource.resource); const existing = await authenticationService.getOrActivateProviderIdForServer(authorizationServer, resourceUri); - if (existing) { + if (existing || !allowCreation) { return existing; } diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostAuth.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostAuth.test.ts index 6006cb00080..270cc06150c 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostAuth.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostAuth.test.ts @@ -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', () => {