From 53ae981106a3bb452c6e26317d65b6661fd78f76 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Wed, 19 Aug 2026 18:32:41 -0700 Subject: [PATCH 1/2] Ask the agent SDK about setup instead of guessing from the filesystem A developer who pays for Claude directly hit a GitHub Copilot sign-in wall, because session-type availability was inferred by sniffing for config files on disk. That guess was wrong in both directions: it gated users who had a working account, and it advertised agents to users who had none. Replace the inference with what the agent's own SDK reports, and make the SDK download an explicit choice rather than something that happens on startup. - Agents publish an SDK setup status (`notDownloaded` / `downloading` / `ready`) over the root config channel, plus the capabilities they offer for getting an account. The workbench derives "no account" from `ready` + zero models, so there is a single wire source per fact. - Agents declare capabilities only; every user-facing string is localized in the workbench via `vs/nls`. - The download is offered by a banner and performed on request. Consent is recorded per agent, so a later version bump re-downloads silently for that agent while a different agent still asks. - Background fetches stay invisible: only the explicit gesture registers download progress interest. - `AgentSdkSetupChannel` holds the nonce handling, in-flight latch and publish ordering once, so Claude and Codex differ only in their capability literals. Removes the filesystem-sniffing paths this replaces: `codexLocalAuth` and the "we discovered your existing configuration" notification, which asked users to sign in again after they had already declined the sign-in modal. Co-Authored-By: Claude Opus 5 --- .../agentHost/common/agentSdkSetup.ts | 162 ++++ .../node/agentSdkDownloadTelemetry.ts | 120 +++ .../agentHost/node/agentSdkDownloader.ts | 39 +- .../agentHost/node/agentSdkSetupChannel.ts | 121 +++ .../agentHost/node/claude/claudeAgent.ts | 129 ++- .../node/claude/claudeAgentSdkService.ts | 9 +- .../node/claude/claudeTransportMode.ts | 113 +-- .../agentHost/node/codex/codexAgent.ts | 130 ++- .../agentHost/node/codex/codexLocalAuth.ts | 70 -- .../test/common/agentSdkSetup.test.ts | 126 +++ .../node/agentSdkDownloadTelemetry.test.ts | 36 + .../test/node/agentSdkDownloader.test.ts | 73 +- .../test/node/claudeAgent.integrationTest.ts | 2 +- .../agentHost/test/node/claudeAgent.test.ts | 866 +++++++++++------- .../test/node/claudeSubagentResolver.test.ts | 2 +- .../test/node/claudeTransportMode.test.ts | 130 +-- .../test/node/codex/codexAgent.test.ts | 81 +- .../test/node/codex/codexLocalAuth.test.ts | 60 -- .../test/node/codex/codexModelRefresh.test.ts | 552 +++++++---- .../node/codex/codexSessionConfigKeys.test.ts | 3 +- .../node/codex/codexSessionTitleSpans.test.ts | 3 +- .../test/node/testAgentSdkDownloader.ts | 66 ++ src/vs/sessions/browser/sessionsAuthGate.ts | 35 - .../browser/mobile/mobileSessionTypePicker.ts | 4 +- .../contrib/chat/browser/sessionTypePicker.ts | 4 + .../test/browser/sessionTypePicker.test.ts | 2 + .../agentHostDiscoveredConfigNotification.ts | 162 ---- .../browser/localAgentHost.contribution.ts | 4 +- ...ntHostDiscoveredConfigNotification.test.ts | 140 --- .../test/browser/sessionsAuthGate.test.ts | 46 +- .../agentHost/agentHost.contribution.ts | 2 + .../agentHostSdkSetupNotification.ts | 339 +++++++ .../agentSessions/sessionTypeAvailability.ts | 30 +- .../delegationSessionPickerActionItem.ts | 4 +- .../input/sessionTargetPickerActionItem.ts | 6 + .../agentHostSdkSetupNotification.test.ts | 244 +++++ .../sessionTypeAvailability.test.ts | 33 +- .../sessionTargetPickerActionItem.test.ts | 86 +- .../agentHost/browser/agentSdkSetupService.ts | 247 +++++ .../agentHost/browser/codexAccountService.ts | 9 + .../test/browser/codexAccountService.test.ts | 2 + 41 files changed, 2930 insertions(+), 1362 deletions(-) create mode 100644 src/vs/platform/agentHost/common/agentSdkSetup.ts create mode 100644 src/vs/platform/agentHost/node/agentSdkDownloadTelemetry.ts create mode 100644 src/vs/platform/agentHost/node/agentSdkSetupChannel.ts delete mode 100644 src/vs/platform/agentHost/node/codex/codexLocalAuth.ts create mode 100644 src/vs/platform/agentHost/test/common/agentSdkSetup.test.ts create mode 100644 src/vs/platform/agentHost/test/node/agentSdkDownloadTelemetry.test.ts delete mode 100644 src/vs/platform/agentHost/test/node/codex/codexLocalAuth.test.ts create mode 100644 src/vs/platform/agentHost/test/node/testAgentSdkDownloader.ts delete mode 100644 src/vs/sessions/contrib/providers/agentHost/browser/agentHostDiscoveredConfigNotification.ts delete mode 100644 src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostDiscoveredConfigNotification.test.ts create mode 100644 src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSdkSetupNotification.ts create mode 100644 src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostSdkSetupNotification.test.ts create mode 100644 src/vs/workbench/services/agentHost/browser/agentSdkSetupService.ts diff --git a/src/vs/platform/agentHost/common/agentSdkSetup.ts b/src/vs/platform/agentHost/common/agentSdkSetup.ts new file mode 100644 index 0000000000000..953baebb2508e --- /dev/null +++ b/src/vs/platform/agentHost/common/agentSdkSetup.ts @@ -0,0 +1,162 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import type { RootState } from './state/protocol/state.js'; + +/** + * Private side-channel describing whether each agent's SDK is on disk yet, and + * what the user can do about it. Rides `publishRootTransientValues` rather than + * AHP proper, alongside `vscode.codexAccount`: the protocol files here are + * generated and version-pinned, so promoting this is a cross-repo change. + * + * One key per agent rather than one key holding a map — transient values are a + * shallow patch, so a shared key would let agents erase each other's entry. + */ +const AGENT_SDK_SETUP_STATUS_KEY_PREFIX = 'vscode.agentSdkSetup.status.'; + +export const AGENT_SDK_SETUP_DOWNLOAD_REQUEST_KEY = 'vscode.agentSdkSetup.downloadRequest'; + +export function agentSdkSetupStatusKey(agent: string): string { + return `${AGENT_SDK_SETUP_STATUS_KEY_PREFIX}${agent}`; +} + +/** + * Whether the agent's SDK can be loaded without a network fetch. + * + * Deliberately the *only* thing on the wire: account state is derivable from the + * model list, which already flows over AHP — `ready` plus zero models means "no + * account" — and publishing it too would be two sources for one truth. + */ +export type AgentSdkDownloadStatus = 'notDownloaded' | 'downloading' | 'ready'; + +/** + * What an agent declares about its own setup. Capabilities, never UI: no + * user-facing strings ride this channel, because localization belongs in the + * workbench. + */ +export interface IAgentSdkSetupInfo { + /** Agent/provider id, e.g. `'claude'`. */ + readonly agent: string; + readonly download: AgentSdkDownloadStatus; + /** + * Where the user goes to finish setup, for the agents whose setup happens + * outside the app (`claude login`, an exported API key). + */ + readonly setupDocsUrl?: string; + /** + * Display name of the provider this agent can sign in to in-app, e.g. + * `'ChatGPT'`; absent means it has no such flow. A proper noun the workbench + * cannot invent, so it crosses the wire like `displayName` does and is + * interpolated into a localized template rather than shown raw. + */ + readonly signInProviderName?: string; +} + +/** A request the workbench addresses to one agent, made unique so a repeat press is not swallowed. */ +export interface IAgentSdkSetupRequest { + readonly agent: string; + readonly request: string; +} + +export function isAgentSdkSetupRequestFor(value: unknown, agent: string): value is IAgentSdkSetupRequest { + if (!value || typeof value !== 'object') { + return false; + } + const request: Partial = value; + return request.agent === agent && typeof request.request === 'string' && request.request.length > 0; +} + +function readOne(value: unknown, agent: string): IAgentSdkSetupInfo | undefined { + if (!value || typeof value !== 'object') { + return undefined; + } + const info: Partial = value; + if (info.download !== 'notDownloaded' && info.download !== 'downloading' && info.download !== 'ready') { + return undefined; + } + return { + agent, + download: info.download, + setupDocsUrl: typeof info.setupDocsUrl === 'string' ? info.setupDocsUrl : undefined, + signInProviderName: typeof info.signInProviderName === 'string' && info.signInProviderName.length > 0 ? info.signInProviderName : undefined, + }; +} + +/** + * Every agent that has published a setup status, in root-state key order. Agents + * that have not published are absent rather than guessed at — there is no honest + * default for "we were never told". + */ +export function readAgentSdkSetupInfos(state: RootState | undefined): readonly IAgentSdkSetupInfo[] { + // The one sanctioned hop into the namespaced setup slots; every field read out + // of them is validated in `readOne`. + const meta = state?._meta; + const values = state?.config?.values; + const infos: IAgentSdkSetupInfo[] = []; + const seen = new Set(); + for (const bag of [values, meta]) { + for (const key of Object.keys(bag ?? {})) { + if (!key.startsWith(AGENT_SDK_SETUP_STATUS_KEY_PREFIX)) { + continue; + } + const agent = key.slice(AGENT_SDK_SETUP_STATUS_KEY_PREFIX.length); + if (!agent || seen.has(agent)) { + continue; + } + const info = readOne(bag?.[key], agent); + if (info) { + seen.add(agent); + infos.push(info); + } + } + } + return infos; +} + +/** + * The agents whose SDK the user has agreed to fetch, decoded from storage. A + * malformed or absent record reads as "nobody consented", which costs at worst + * one extra press of a button the user was about to press anyway. + */ +export function readConsentedSdkAgents(stored: string | undefined): ReadonlySet { + if (!stored) { + return new Set(); + } + try { + const parsed: unknown = JSON.parse(stored); + return new Set(Array.isArray(parsed) ? parsed.filter(agent => typeof agent === 'string') : []); + } catch { + return new Set(); + } +} + +export function writeConsentedSdkAgents(agents: ReadonlySet): string { + return JSON.stringify([...agents]); +} + +/** + * Which agents should be asked to fetch their SDK without being offered a + * button, given standing consent. The SDK version is pinned per build + * and the cache keyed by version, so every update invalidates it — daily on + * Insiders. Consent is to "this product downloads the Claude SDK", not to one + * tarball, so re-asking would nag people who already said yes. + * + * It does not carry to a *different* agent: the button says "we need to + * download the Codex Agent SDK", and pressing it is not permission to fetch + * Claude's. + * + * `alreadyRequested` stops a failing download retrying forever: a failed fetch + * republishes `notDownloaded`, and every status change re-runs this. A window is + * the retry unit. + */ +export function resolveConsentedSdkDownloads( + consentedAgents: ReadonlySet, + setups: readonly IAgentSdkSetupInfo[], + alreadyRequested: ReadonlySet, +): readonly string[] { + return setups + .filter(setup => setup.download === 'notDownloaded' && consentedAgents.has(setup.agent) && !alreadyRequested.has(setup.agent)) + .map(setup => setup.agent); +} diff --git a/src/vs/platform/agentHost/node/agentSdkDownloadTelemetry.ts b/src/vs/platform/agentHost/node/agentSdkDownloadTelemetry.ts new file mode 100644 index 0000000000000..8c7846b88d3aa --- /dev/null +++ b/src/vs/platform/agentHost/node/agentSdkDownloadTelemetry.ts @@ -0,0 +1,120 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { ILogService } from '../../log/common/log.js'; +import { ITelemetryService } from '../../telemetry/common/telemetry.js'; +import type { IAgentSdkDownloadProgress } from './agentSdkDownloader.js'; + +// #region Failure classification + +/** + * Coarse bucket for a failed SDK fetch. A closed set: the downloader's own error + * strings carry the CDN URL and the cache path, so the raw message can never be + * the reported reason. `notConfigured` and `unsupportedTarget` describe a build + * that cannot fetch this SDK at all, so a non-zero count is a signal in itself. + */ +export type AgentSdkDownloadFailureReason = + | 'cancelled' + | 'network' + | 'filesystem' + | 'extract' + | 'notConfigured' + | 'unsupportedTarget' + | 'unknown'; + +/** + * Order matters. Network before extraction, because an HTTP failure message + * embeds the tarball URL and would otherwise match an archive-shaped hint; + * filesystem errnos before network, because they are unambiguous where a bare + * `EACCES` from a proxy is not. + */ +const FAILURE_HINTS: readonly (readonly [AgentSdkDownloadFailureReason, readonly string[]])[] = [ + ['notConfigured', ['no `product.agentSdks', 'unknown placeholder']], + ['unsupportedTarget', ['no SDK target for this host']], + ['filesystem', ['ENOSPC', 'EACCES', 'EPERM', 'EROFS', 'EBUSY', 'EMFILE', 'ENAMETOOLONG', 'EXDEV']], + ['network', ['HTTP ', 'ENOTFOUND', 'EAI_AGAIN', 'ECONNRESET', 'ECONNREFUSED', 'ETIMEDOUT', 'EPROTO', 'ECONNABORTED', 'socket hang up', 'certificate', 'tunneling socket', 'getaddrinfo']], + ['extract', ['TAR_', 'zlib', 'gzip', 'unexpected end of file', 'incorrect header check', 'invalid entry']], +]; + +/** + * Bucket a downloader failure message. Substring matching, because the messages + * are assembled from Node errnos, `node-tar` diagnostics and our own wrappers — + * none of which carry a stable code by the time they arrive here. Anything + * unrecognised is `unknown` rather than guessed at: a rising `unknown` share is + * the signal to add a hint, which a neighbouring bucket would hide. + */ +export function classifyAgentSdkDownloadFailure(error: string | undefined): AgentSdkDownloadFailureReason { + if (!error) { + return 'unknown'; + } + // The downloader reports cancellation as this exact token, not as a message. + if (error === 'cancelled') { + return 'cancelled'; + } + const haystack = error.toLowerCase(); + for (const [reason, hints] of FAILURE_HINTS) { + if (hints.some(hint => haystack.includes(hint.toLowerCase()))) { + return reason; + } + } + return 'unknown'; +} + +// #endregion + +// #region Telemetry + +interface IAgentSdkDownloadEvent { + packageId: string; + phase: string; + failureReason: string; + explicitlyRequested: boolean; + durationMs: number; + receivedBytes: number; + totalBytes: number; +} + +type AgentSdkDownloadClassification = { + packageId: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Which agent SDK was being fetched, e.g. claude or codex.' }; + phase: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Whether the download started, completed, or failed.' }; + failureReason: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Coarse bucket for a failed download (cancelled, network, filesystem, extract, notConfigured, unsupportedTarget, unknown). Empty unless the phase is failed.' }; + explicitlyRequested: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Whether a user gesture asked for this download, as opposed to a quiet re-fetch under standing consent.' }; + durationMs: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; isMeasurement: true; comment: 'How long the download had been running when it reached this phase.' }; + receivedBytes: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; isMeasurement: true; comment: 'Bytes fetched by the time this phase was reached.' }; + totalBytes: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; isMeasurement: true; comment: 'Total size the server advertised, or zero when it did not.' }; + owner: 'TylerLeonhardt'; + comment: 'The middle of the agent SDK setup funnel: whether an offered download is actually attempted, and whether it works.'; +}; + +/** + * Report one endpoint of a download. Callers pass only terminal and `started` + * frames; the throttled `progress` frames are not counted. + * `explicitlyRequested` is the user-gesture-versus-standing-consent split costs + * are measured on. + */ +export function reportAgentSdkDownload( + telemetryService: ITelemetryService, + logService: ILogService, + progress: IAgentSdkDownloadProgress, + durationMs: number, +): void { + const failureReason = progress.phase === 'failed' ? classifyAgentSdkDownloadFailure(progress.error) : ''; + telemetryService.publicLog2('agentHost.agentSdkDownload', { + packageId: progress.packageId, + phase: progress.phase, + failureReason, + explicitlyRequested: progress.explicitlyRequested, + durationMs, + receivedBytes: progress.receivedBytes, + totalBytes: progress.totalBytes ?? 0, + }); + logService.info( + `[AgentSdkDownloader] ${progress.packageId}: ${progress.phase}` + + ` (explicit=${progress.explicitlyRequested}, bytes=${progress.receivedBytes}/${progress.totalBytes ?? 'unknown'}, ms=${durationMs}` + + `${failureReason ? `, reason=${failureReason}` : ''})`, + ); +} + +// #endregion diff --git a/src/vs/platform/agentHost/node/agentSdkDownloader.ts b/src/vs/platform/agentHost/node/agentSdkDownloader.ts index 0e4786cb61acd..1d59ac95cb129 100644 --- a/src/vs/platform/agentHost/node/agentSdkDownloader.ts +++ b/src/vs/platform/agentHost/node/agentSdkDownloader.ts @@ -21,7 +21,9 @@ import { createDecorator } from '../../instantiation/common/instantiation.js'; import { ILogService } from '../../log/common/log.js'; import { IProductService } from '../../product/common/productService.js'; import { IRequestService } from '../../request/common/request.js'; +import { ITelemetryService } from '../../telemetry/common/telemetry.js'; import { IRequestContext } from '../../../base/parts/request/common/request.js'; +import { reportAgentSdkDownload } from './agentSdkDownloadTelemetry.js'; // #region Per-package strategy @@ -273,6 +275,7 @@ export class AgentSdkDownloader extends Disposable implements IAgentSdkDownloade @IRequestService private readonly _requestService: IRequestService, @IFileService private readonly _fileService: IFileService, @ILogService private readonly _logService: ILogService, + @ITelemetryService private readonly _telemetryService: ITelemetryService, ) { super(); } @@ -320,9 +323,11 @@ export class AgentSdkDownloader extends Disposable implements IAgentSdkDownloade return override; } - // 2. Negative cache: a recent failure short-circuits without I/O. + // 2. Negative cache: a recent failure short-circuits without I/O. Not for a + // user who asked by hand, though — the latch exists to stop background retry + // storms, not to leave a Download button doing nothing for half a minute. const latched = this._failureLatch.get(pkg.id); - if (latched && latched.expiresAt > Date.now()) { + if (latched && latched.expiresAt > Date.now() && !this._explicitProgressInterest.has(pkg.id)) { throw latched.error; } @@ -380,8 +385,14 @@ export class AgentSdkDownloader extends Disposable implements IAgentSdkDownloade // that crashed mid-way never write it. See `_download` for why // the sentinel is written inside the tmp dir before the rename. if (await this._fileService.exists(sentinel)) { + // Logged, not counted: a cache hit happens on every SDK method call + // and would drown the download funnel. It matters here because "was + // the SDK already there?" is the first question asked of a log where + // no download was ever attempted. + this._logService.trace(`[AgentSdkDownloader] ${pkg.id}: cache hit at ${cacheDir}`); return cacheDir; } + this._logService.info(`[AgentSdkDownloader] ${pkg.id}: cache miss for version ${config.version} (${sdkTarget}); a download is required`); // Download (deduped across concurrent callers in the same process). // cacheDir is already unique per (pkg, version, sdkTarget) — within @@ -439,14 +450,14 @@ export class AgentSdkDownloader extends Disposable implements IAgentSdkDownloade const downloadId = generateUuid(); let lastReceived = 0; let lastTotal: number | undefined; - this._fireProgress(pkg, downloadId, 'started', 0, undefined); + this._fireProgress(pkg, downloadId, start, 'started', 0, undefined); try { const tarballPath = path.join(tmpDir, 'sdk.tgz'); await this._fetch(url, tarballPath, token, (receivedBytes, totalBytes) => { lastReceived = receivedBytes; lastTotal = totalBytes; - this._fireProgress(pkg, downloadId, 'progress', receivedBytes, totalBytes); + this._fireProgress(pkg, downloadId, start, 'progress', receivedBytes, totalBytes); }); await this._extractTarGz(tarballPath, tmpDir); await this._fileService.del(URI.file(tarballPath)); @@ -469,24 +480,24 @@ export class AgentSdkDownloader extends Disposable implements IAgentSdkDownloade } catch (err) { if (await this._handleRenameLoser(err, sentinel, tmpDirUri)) { this._logService.info(`[AgentSdkDownloader] ${pkg.id}: lost rename race, using existing cache`); - this._fireProgress(pkg, downloadId, 'completed', lastReceived, lastTotal); + this._fireProgress(pkg, downloadId, start, 'completed', lastReceived, lastTotal); return cacheDir; } throw err; } const elapsed = Math.round((Date.now() - start) / 1000); - this._logService.info(`[AgentSdkDownloader] ${pkg.id}: downloaded in ${elapsed}s`); - this._fireProgress(pkg, downloadId, 'completed', lastTotal ?? lastReceived, lastTotal); + this._logService.info(`[AgentSdkDownloader] ${pkg.id}: downloaded ${lastTotal ?? lastReceived} bytes in ${elapsed}s`); + this._fireProgress(pkg, downloadId, start, 'completed', lastTotal ?? lastReceived, lastTotal); return cacheDir; } catch (err) { await this._delIgnoringMissing(tmpDirUri); if (token.isCancellationRequested) { - this._fireProgress(pkg, downloadId, 'failed', lastReceived, lastTotal, 'cancelled'); + this._fireProgress(pkg, downloadId, start, 'failed', lastReceived, lastTotal, 'cancelled'); throw new CancellationError(); } const message = err instanceof Error ? err.message : String(err); - this._fireProgress(pkg, downloadId, 'failed', lastReceived, lastTotal, message); + this._fireProgress(pkg, downloadId, start, 'failed', lastReceived, lastTotal, message); throw new Error( `Failed to download ${pkg.id} SDK from ${url} ` + `(cache target: ${cacheDir}). ` + @@ -499,12 +510,13 @@ export class AgentSdkDownloader extends Disposable implements IAgentSdkDownloade private _fireProgress( pkg: IAgentSdkPackage, downloadId: string, + startedAt: number, phase: AgentSdkDownloadPhase, receivedBytes: number, totalBytes: number | undefined, error?: string, ): void { - this._onDidDownloadProgress.fire({ + const progress: IAgentSdkDownloadProgress = { downloadId, packageId: pkg.id, displayName: pkg.displayName, @@ -513,7 +525,12 @@ export class AgentSdkDownloader extends Disposable implements IAgentSdkDownloade totalBytes, explicitlyRequested: this._explicitProgressInterest.has(pkg.id), ...(error !== undefined ? { error } : {}), - }); + }; + this._onDidDownloadProgress.fire(progress); + // Endpoints only — the throttled `progress` frames would flood the funnel. + if (phase !== 'progress') { + reportAgentSdkDownload(this._telemetryService, this._logService, progress, Date.now() - startedAt); + } } private async _handleRenameLoser( diff --git a/src/vs/platform/agentHost/node/agentSdkSetupChannel.ts b/src/vs/platform/agentHost/node/agentSdkSetupChannel.ts new file mode 100644 index 0000000000000..98efa3127933a --- /dev/null +++ b/src/vs/platform/agentHost/node/agentSdkSetupChannel.ts @@ -0,0 +1,121 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Disposable } from '../../../base/common/lifecycle.js'; +import { ILogService } from '../../log/common/log.js'; +import { AGENT_SDK_SETUP_DOWNLOAD_REQUEST_KEY, AgentSdkDownloadStatus, IAgentSdkSetupInfo, agentSdkSetupStatusKey, isAgentSdkSetupRequestFor } from '../common/agentSdkSetup.js'; +import { IAgentConfigurationService } from './agentConfigurationService.js'; +import { IAgentSdkDownloader, IAgentSdkPackage } from './agentSdkDownloader.js'; + +/** The per-agent half of {@link AgentSdkSetupChannel}. */ +export interface IAgentSdkSetupChannelAgent { + /** Agent/provider id, which becomes {@link IAgentSdkSetupInfo.agent}. */ + readonly id: string; + readonly sdkPackage: IAgentSdkPackage; + + /** What this agent offers besides the download. Published verbatim. */ + readonly setupInfo: Omit; + + /** Whether the SDK can be loaded without a network fetch. */ + isSdkLocal(): Promise; + + /** Fetch the SDK. Only ever called for the explicit gesture. */ + downloadSdk(): Promise; + + /** Restart chat discovery, which defers itself while there is no SDK to read a catalog from. */ + restartChatDiscovery(): void; + + /** Re-enumerate models against the SDK that just landed. */ + refreshModels(): Promise; +} + +/** + * One agent's side of the SDK setup channel: publishes whether its SDK is on + * disk, and performs the download the workbench asks for. Every agent needs the + * same nonce handling, latching and publish ordering, so only the calls in + * {@link IAgentSdkSetupChannelAgent} differ. + */ +export class AgentSdkSetupChannel extends Disposable { + + /** Consumed request nonce, so a root-config change we caused isn't re-handled. */ + private _lastRequest: string | undefined; + + /** + * Latched while the *explicit* download runs. {@link IAgentSdkSetupChannelAgent.isSdkLocal} + * stays false throughout, so without this the channel could only ever report + * `notDownloaded` and the banner would keep offering a button for work already + * underway. Deliberately not a query on the downloader, which would also latch + * for background fetches — those are the ones the user never asked for and so + * must stay invisible. + */ + private _downloadInFlight = false; + + constructor( + private readonly _agent: IAgentSdkSetupChannelAgent, + private readonly _configurationService: IAgentConfigurationService, + private readonly _downloader: IAgentSdkDownloader, + private readonly _logService: ILogService, + ) { + super(); + // The workbench addresses the agent through the root config bag. The key is + // cleared as it is consumed so a later identical press still lands. + this._register(this._configurationService.onDidRootConfigChange(() => this._handleRequest())); + queueMicrotask(() => { void this.publish(); }); + } + + /** Publish the current status, paying for the is-local probe. */ + async publish(): Promise { + this.publishWith(await this._agent.isSdkLocal()); + } + + /** The synchronous half, for callers that have just paid for the probe. */ + publishWith(sdkIsLocal: boolean): void { + const download: AgentSdkDownloadStatus = this._downloadInFlight + ? 'downloading' + : sdkIsLocal ? 'ready' : 'notDownloaded'; + const info: Omit = { ...this._agent.setupInfo, download }; + this._configurationService.publishRootTransientValues?.({ [agentSdkSetupStatusKey(this._agent.id)]: info }); + } + + private _handleRequest(): void { + const request = this._configurationService.getRootConfigValues?.()[AGENT_SDK_SETUP_DOWNLOAD_REQUEST_KEY]; + if (!isAgentSdkSetupRequestFor(request, this._agent.id) || request.request === this._lastRequest) { + return; + } + this._lastRequest = request.request; + this._configurationService.updateRootConfig({ [AGENT_SDK_SETUP_DOWNLOAD_REQUEST_KEY]: undefined }); + void this._download(); + } + + /** + * The explicit download gesture. Acquiring progress interest is what makes the + * fetch visible: the downloader only emits frames for a session that asked or an + * explicitly-registered interest, and this download belongs to no session. + */ + private async _download(): Promise { + if (this._downloadInFlight) { + return; + } + const progressInterest = this._downloader.acquireDownloadProgressInterest(this._agent.sdkPackage); + this._downloadInFlight = true; + this.publishWith(false); + try { + this._logService.info(`[AgentSdkSetup] ${this._agent.id}: downloading the agent SDK at the user's request`); + await this._agent.downloadSdk(); + } catch (error) { + this._logService.error(error, `[AgentSdkSetup] ${this._agent.id}: agent SDK download failed`); + } finally { + this._downloadInFlight = false; + progressInterest.dispose(); + } + // Chat discovery deferred itself while there was no SDK to read the catalog + // from; this is the one moment that can change. + this._agent.restartChatDiscovery(); + // Second, not first: the refresh is what asks the fresh SDK about the account, + // so announcing `ready` ahead of it would show "no account found" to a user + // who has one for as long as enumeration takes. + await this._agent.refreshModels(); + } +} diff --git a/src/vs/platform/agentHost/node/claude/claudeAgent.ts b/src/vs/platform/agentHost/node/claude/claudeAgent.ts index 026930e22311c..e2ebc5496b692 100644 --- a/src/vs/platform/agentHost/node/claude/claudeAgent.ts +++ b/src/vs/platform/agentHost/node/claude/claudeAgent.ts @@ -20,6 +20,8 @@ import { INativeEnvironmentService } from '../../../environment/common/environme import { ILogService } from '../../../log/common/log.js'; import { IProductService } from '../../../product/common/productService.js'; import { IAgentPluginManager, ISyncedCustomization } from '../../common/agentPluginManager.js'; +import { IAgentSdkDownloader } from '../agentSdkDownloader.js'; +import { AgentSdkSetupChannel } from '../agentSdkSetupChannel.js'; import { decodeProviderData, encodeProviderData, type IPersistedChat } from '../agentChatBackings.js'; import { buildSideChatSourceContext, prepareSideChatPrompt, sliceSideChatTurns } from '../agentPeerChats.js'; import { AgentHostConfigKey, agentHostCustomizationConfigSchema } from '../../common/agentHostCustomizationConfig.js'; @@ -44,9 +46,9 @@ import { IAgentHostCheckpointService } from '../../common/agentHostCheckpointSer import { PendingRequestRegistry } from '../../common/pendingRequestRegistry.js'; import { projectFromCopilotContext } from '../copilot/copilotGitProject.js'; import { ICopilotApiService } from '../shared/copilotApiService.js'; -import { IClaudeAgentSdkService } from './claudeAgentSdkService.js'; +import { ClaudeSdkPackage, IClaudeAgentSdkService } from './claudeAgentSdkService.js'; import { buildModelEnumerationOptions } from './claudeSdkOptions.js'; -import { detectExistingClaudeSetup, resolveClaudeTransportMode, type ClaudeTransportMode } from './claudeTransportMode.js'; +import { isClaudeAccountSetUp, resolveClaudeTransportMode, type ClaudeTransportMode } from './claudeTransportMode.js'; import { mergeClaudeModelCatalogs, resolveClaudeSessionTransport } from './claudeModelSelection.js'; import { mapSessionMessagesToTurns, resolveForkAnchorUuid } from './claudeReplayMapper.js'; import { getSubagentTranscript } from './claudeSubagentResolver.js'; @@ -66,6 +68,9 @@ import { IAgentHostOTelService } from '../../common/otel/agentHostOTelService.js const USER_AGENT_PREFIX = 'vscode_claude_code'; +/** Where a user goes to establish Claude credentials; the workbench labels the button. */ +const CLAUDE_SETUP_DOCS_URL = 'https://docs.claude.com/en/docs/claude-code/setup'; + /** * Returns true if `m` is a Claude-family model that should be advertised * to clients picking a model for the Claude provider. @@ -605,6 +610,7 @@ export class ClaudeAgent extends Disposable implements IAgent { @ICopilotApiService private readonly _copilotApiService: ICopilotApiService, @IClaudeProxyService private readonly _claudeProxyService: IClaudeProxyService, @IClaudeAgentSdkService private readonly _sdkService: IClaudeAgentSdkService, + @IAgentSdkDownloader private readonly _agentSdkDownloader: IAgentSdkDownloader, @IAgentHostSessionTitleSignal private readonly _sessionTitleSignal: IAgentHostSessionTitleSignal, @IAgentHostOTelService private readonly _otelService: IAgentHostOTelService, @IAgentHostGitService private readonly _gitService: IAgentHostGitService, @@ -645,8 +651,28 @@ export class ClaudeAgent extends Disposable implements IAgent { // (see {@link _defaultTransportMode}), so a sign-in state change needs no // reactive re-resolve — the next session simply reads it live. queueMicrotask(() => { void this._startModelRefresh(); }); + + this._sdkSetupChannel = this._register(new AgentSdkSetupChannel({ + id: this.id, + sdkPackage: ClaudeSdkPackage, + // Every Claude credential — subscription or `ANTHROPIC_API_KEY` — is + // established outside the app, and the SDK exposes no login control + // request, so the docs link is the only route this agent can offer. + setupInfo: { setupDocsUrl: CLAUDE_SETUP_DOCS_URL }, + isSdkLocal: () => this._sdkService.canLoadWithoutDownload(), + downloadSdk: () => this._sdkService.ensureAvailable(), + restartChatDiscovery: () => this._restartChatDiscovery(), + refreshModels: () => this._startModelRefresh(), + }, this._configurationService, this._agentSdkDownloader, this._logService)); } + /** + * Publishes whether the SDK is on disk — and deliberately nothing about the + * account, which the workbench derives from the model list (`ready` + zero + * models → no account). Two wire sources for one truth could disagree. + */ + private readonly _sdkSetupChannel: AgentSdkSetupChannel; + /** * The fallback transport for a session whose model names no provider (model-less * or a bare/legacy id). Read on demand at materialize — never cached — from live @@ -657,19 +683,14 @@ export class ClaudeAgent extends Disposable implements IAgent { */ private _defaultTransportMode(): ClaudeTransportMode { const allowSignedOutWhenUsable = this._configurationService.getRootValue(agentHostCustomizationConfigSchema, AgentHostConfigKey.AllowSignedOutWhenUsable) === true; - return resolveClaudeTransportMode({ allowSignedOutWhenUsable, hasGitHubToken: this._proxyHandle !== undefined, hasExistingSetup: this._hasUsableNativeSetup() }); + return resolveClaudeTransportMode({ allowSignedOutWhenUsable, hasGitHubToken: this._proxyHandle !== undefined, hasExistingSetup: this._nativeAccountSetUp }); } /** - * Whether Claude can run without GitHub right now: the signed-out opt-in is on - * AND a BYO-Anthropic credential is discoverable (see - * {@link detectExistingClaudeSetup}). Backs both the advertised requirement and - * the model-less transport default so the two cannot disagree. + * The SDK's last answer to {@link isClaudeAccountSetUp}, kept current by + * {@link _refreshModels}. Starts `false`: unasked is not evidence of an account. */ - private _hasUsableNativeSetup(): boolean { - return this._configurationService.getRootValue(agentHostCustomizationConfigSchema, AgentHostConfigKey.AllowSignedOutWhenUsable) === true - && detectExistingClaudeSetup(this._environmentService.userHome.fsPath); - } + private _nativeAccountSetUp = false; // #region Descriptor + auth @@ -690,14 +711,14 @@ export class ClaudeAgent extends Disposable implements IAgent { } getProtectedResources(): ProtectedResourceMetadata[] { - // Kept in the list even when optional, never dropped: - // `authenticateProtectedResources` matches on `resource` and ignores - // `required`, so advertising it is what lets the host silently forward a - // token to an already-signed-in user — and acquire the proxy handle - // Copilot-routed models need — without forcing sign-in on anyone else. + // Always listed, always optional. Listing it is what lets the host forward a + // token to an already-signed-in user (matching ignores `required`); the + // unconditional `required: false` is what stops `resolveSignedOutWindowGate` + // walling off the whole Agents window before the user reaches a surface that + // could explain itself. const copilotResource = this._gitHubEndpointService.getCopilotResource(); return [ - this._hasUsableNativeSetup() ? { ...copilotResource, required: false } : copilotResource, + { ...copilotResource, required: false }, this._gitHubEndpointService.getRepoResource(), ]; } @@ -858,14 +879,14 @@ export class ClaudeAgent extends Disposable implements IAgent { /** * Enumerate both providers' catalogs in parallel and publish them as one * provider-qualified list via {@link mergeClaudeModelCatalogs}. Each source is - * optional — the proxy catalog needs a GitHub token, the native catalog needs a - * local Claude setup — so a source we can't attempt contributes an empty list - * rather than failing the whole refresh. {@link Promise.allSettled} tolerates - * one source erroring; only when *every* source we attempted fails do we keep - * the last known-good catalog instead of blanking, so a transient double - * failure never wipes the picker. + * optional — the proxy catalog needs a GitHub token, the native catalog needs the + * SDK on disk — so a source we can't attempt contributes an empty list rather + * than failing the whole refresh. {@link Promise.allSettled} tolerates one source + * erroring; only when *every* source we attempted fails do we keep the last + * known-good catalog instead of blanking, so a transient double failure never + * wipes the picker. * - * Gating the native half on {@link detectExistingClaudeSetup} is deliberate and + * Gating the native half on the SDK's own account report is deliberate and * load-bearing, not just an optimization. `supportedModels()` returns a *static* * list of models the SDK understands — it is not an entitlement or credential * check, and it answers even with no `ANTHROPIC_API_KEY`, no @@ -874,14 +895,27 @@ export class ClaudeAgent extends Disposable implements IAgent { * reads downstream as "usable without GitHub" and would hold the Agents window * open on an agent that fails on its first turn. An empty catalog is the honest * signal: it surfaces as "no models" (`SessionTypeAuthRequirement.Unusable`) - * rather than a sign-in prompt that would not help. + * rather than a sign-in prompt that would not help. The empty list is also what + * the window reads account state *from*, so it must never be a guess. + * + * The native attempt is skipped while the SDK is not on disk: asking it anything + * costs a multi-hundred-megabyte download, and that download is the user's + * explicit choice to make. */ private async _refreshModels(): Promise { const tokenAtStart = this._githubToken; - const hasNativeSetup = detectExistingClaudeSetup(this._environmentService.userHome.fsPath); + // True only for a dev override, a dev bare import, or an already-cached SDK. + const canAttemptNative = await this._sdkService.canLoadWithoutDownload(); + // Free republish: another path may have pulled the SDK down since we last + // looked (the download gesture, or a session the user started). + this._sdkSetupChannel.publishWith(canAttemptNative); + if (!canAttemptNative) { + // No SDK, so no evidence of an account — say so rather than retaining a stale `true`. + this._nativeAccountSetUp = false; + } const [proxyOutcome, nativeOutcome] = await Promise.allSettled([ tokenAtStart ? this._fetchProxyModels(tokenAtStart) : Promise.resolve([]), - hasNativeSetup ? this._fetchNativeModels() : Promise.resolve([]), + canAttemptNative ? this._fetchNativeModels() : Promise.resolve([]), ]); // Stale-write guard: a newer refresh superseded this one while we were // awaiting — the proxy token rotated (sign-in / sign-out). A merged write @@ -889,7 +923,7 @@ export class ClaudeAgent extends Disposable implements IAgent { if (this._githubToken !== tokenAtStart) { return; } - const attempted = (tokenAtStart ? 1 : 0) + (hasNativeSetup ? 1 : 0); + const attempted = (tokenAtStart ? 1 : 0) + (canAttemptNative ? 1 : 0); const failed = (proxyOutcome.status === 'rejected' ? 1 : 0) + (nativeOutcome.status === 'rejected' ? 1 : 0); if (attempted > 0 && failed === attempted) { // Every source we attempted failed — keep the last known-good catalog @@ -922,6 +956,11 @@ export class ClaudeAgent extends Disposable implements IAgent { * yields, so no turn runs and no session transcript is written (verified * Phase 19 E2E). Projected with no commercial metadata, minus the SDK's * {@link isSdkDefaultModel} alias row. + * + * `accountInfo()` rides the *same* query, so asking is effectively free — and it + * is the only honest source for "does this user have a Claude setup": a + * `claude login` credential lives in the login keychain, where nothing on the + * filesystem can see it. When it says no, the catalog is published empty. */ private async _fetchNativeModels(): Promise { // A prompt iterable that never yields: enumeration only needs the @@ -932,7 +971,14 @@ export class ClaudeAgent extends Disposable implements IAgent { const options = buildModelEnumerationOptions(); const query = await this._sdkService.query({ prompt: neverYieldingPrompt, options }); try { - const models = await query.supportedModels(); + const [account, models] = await Promise.all([query.accountInfo(), query.supportedModels()]); + const setUp = isClaudeAccountSetUp(account); + this._nativeAccountSetUp = setUp; + // Origin only — never the credential itself. + this._logService.info(`[Claude] Native account check: setUp=${setUp}, provider=${account.apiProvider ?? 'none'}, tokenSource=${account.tokenSource ?? 'absent'}, apiKeySource=${account.apiKeySource ?? 'absent'}`); + if (!setUp) { + return []; + } return models .filter(m => !isSdkDefaultModel(m)) .map(m => fromSdkModelInfo(m, this.id)); @@ -2047,10 +2093,11 @@ export class ClaudeAgent extends Disposable implements IAgent { } async listChatsToMigrate(): Promise { - try { - await this._sdkService.ensureAvailableForDiscovery(); - } catch (err) { - this._logService.warn('[Claude] SDK unavailable while listing chats to migrate', err); + // `undefined` is "can't enumerate yet", which is the honest answer while the + // SDK is absent: the catalog lives inside it, but fetching one is the user's + // call. {@link _restartChatDiscovery} revisits this once they make it. + if (!(await this._sdkService.canLoadWithoutDownload())) { + this._logService.info('[Claude] SDK not downloaded yet; deferring the migratable chat list'); return undefined; } const chats = await this._listClaudeCodeChats(); @@ -2067,7 +2114,13 @@ export class ClaudeAgent extends Disposable implements IAgent { private _startClaudeCodeChatDiscovery(): Promise { if (!this._claudeCodeChatDiscovery) { this._claudeCodeChatDiscovery = retry(async () => { - await this._sdkService.ensureAvailableForDiscovery(); + // Waits for the SDK rather than pulling it down — see + // {@link listChatsToMigrate}. Returning leaves the retry loop happy, + // since no amount of retrying will make the user press Download. + if (!(await this._sdkService.canLoadWithoutDownload())) { + this._logService.info('[Claude] SDK not downloaded yet; deferring chat discovery'); + return; + } if (!(await this._emitClaudeCodeChats())) { throw new Error('Claude chat catalog is not available'); } @@ -2077,6 +2130,14 @@ export class ClaudeAgent extends Disposable implements IAgent { return this._claudeCodeChatDiscovery; } + /** Runs discovery again for whoever is still subscribed, after it deferred for want of an SDK. */ + private _restartChatDiscovery(): void { + if (this._claudeCodeChatDiscovery) { + this._claudeCodeChatDiscovery = undefined; + void this._startClaudeCodeChatDiscovery(); + } + } + private async _emitClaudeCodeChats(): Promise { try { const chats = await this._listClaudeCodeChats(); diff --git a/src/vs/platform/agentHost/node/claude/claudeAgentSdkService.ts b/src/vs/platform/agentHost/node/claude/claudeAgentSdkService.ts index 46a2df473581e..d53bc976c3879 100644 --- a/src/vs/platform/agentHost/node/claude/claudeAgentSdkService.ts +++ b/src/vs/platform/agentHost/node/claude/claudeAgentSdkService.ts @@ -75,10 +75,11 @@ export interface IClaudeAgentSdkService { */ canLoadWithoutDownload(): Promise; /** - * Ensures the SDK is available for native chat discovery without loading - * the module. + * Downloads the SDK if it isn't local yet, without loading the module. This + * is the explicit gesture: background callers gate on + * {@link canLoadWithoutDownload} instead and do without. */ - ensureAvailableForDiscovery(): Promise; + ensureAvailable(): Promise; forkSession(sessionId: string, options?: ForkSessionOptions): Promise; deleteSession(sessionId: string, options?: SessionMutationOptions): Promise; @@ -178,7 +179,7 @@ export class ClaudeAgentSdkService implements IClaudeAgentSdkService { return this._downloader.isSdkResolvableWithoutDownload(ClaudeSdkPackage); } - async ensureAvailableForDiscovery(): Promise { + async ensureAvailable(): Promise { if (!(await this.canLoadWithoutDownload())) { await this._downloader.loadSdkRoot(ClaudeSdkPackage, CancellationToken.None); } diff --git a/src/vs/platform/agentHost/node/claude/claudeTransportMode.ts b/src/vs/platform/agentHost/node/claude/claudeTransportMode.ts index 0e165ddac3f3d..e594e31820121 100644 --- a/src/vs/platform/agentHost/node/claude/claudeTransportMode.ts +++ b/src/vs/platform/agentHost/node/claude/claudeTransportMode.ts @@ -3,12 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { readFileSync } from 'fs'; -import { parse as parseJSONC, type ParseError } from '../../../../base/common/json.js'; -import { join } from '../../../../base/common/path.js'; -import { isFalsyOrWhitespace } from '../../../../base/common/strings.js'; -import { isString } from '../../../../base/common/types.js'; -import { vObj, vOptionalProp, vUnknown, type ValidatorType } from '../../../../base/common/validation.js'; +import type { AccountInfo } from '@anthropic-ai/claude-agent-sdk'; /** * Resolved Claude host transport. `proxy` routes Anthropic traffic through the @@ -25,7 +20,7 @@ export interface IClaudeTransportModeInputs { readonly allowSignedOutWhenUsable: boolean; /** Whether a GitHub Copilot token has been captured (i.e. signed in). */ readonly hasGitHubToken: boolean; - /** Whether an existing local Claude setup was detected (see {@link detectExistingClaudeSetup}). */ + /** Whether the SDK reported a Claude setup usable on the user's own credentials (see {@link isClaudeAccountSetUp}). */ readonly hasExistingSetup: boolean; } @@ -47,16 +42,14 @@ export interface IClaudeTransportModeInputs { * forced to sign in. * * The result is **not** an input to the Agents window's sign-in gate, and - * resolving to `proxy` does not by itself make the session type "require - * GitHub". That answer is `getProtectedResources()`, which marks the Copilot - * resource `required: false` on the same `hasExistingSetup` fact used here — so - * the two agree by construction: a user with their own Anthropic credential is - * not forced to sign in, and one without (case 4) is. `resolveAgentAuthRequirement` - * then separates `None` from `Unusable` on the *model count*, since a - * `required: false` agent that cannot enumerate a single model must not hold the - * window open. The proxy fallback of case 4 only bites at use time, when a - * model-less/bare session actually materializes with no proxy handle and - * `_ensureAuthenticated` raises `AHP_AUTH_REQUIRED`. + * resolving to `proxy` does not by itself make the session type "require GitHub". + * `getProtectedResources()` marks the Copilot resource `required: false` + * unconditionally, so nothing decided here can raise a sign-in wall. What + * separates `None` from `Unusable` downstream is the *model count*, published + * from the same `accountInfo()` answer that feeds `hasExistingSetup` here — so + * the two cannot disagree about one user. The proxy fallback of case 4 only + * bites at use time, when a model-less session materializes with no proxy handle + * and `_ensureAuthenticated` raises `AHP_AUTH_REQUIRED`. * * There is deliberately no host-global setting to *prefer* a transport. Since * the picker offers both providers' models side by side, transport is downstream @@ -81,70 +74,30 @@ export function resolveClaudeTransportMode(inputs: IClaudeTransportModeInputs): } /** - * Validators for the `~/.claude/settings.json` sources that indicate a usable - * native setup, kept separate — and holding `unknown` rather than `vString()` — - * so one malformed entry reads as absent instead of voiding its siblings. - * {@link hasValue} is what decides usability. - */ -const claudeApiKeyHelperValidator = vObj({ - apiKeyHelper: vOptionalProp(vUnknown()), -}); - -const claudeSettingsEnvValidator = vObj({ - env: vOptionalProp(vObj({ - ANTHROPIC_API_KEY: vOptionalProp(vUnknown()), - ANTHROPIC_AUTH_TOKEN: vOptionalProp(vUnknown()), - ANTHROPIC_BASE_URL: vOptionalProp(vUnknown()), - CLAUDE_CODE_OAUTH_TOKEN: vOptionalProp(vUnknown()), - })), -}); - -/** - * The `env` shape both `process.env` and `~/.claude/settings.json` are probed - * for, derived from {@link claudeSettingsEnvValidator} so the two never drift. - */ -type ClaudeNativeEnv = NonNullable['env']>; - -/** - * Whether a local Claude setup exists that can run without GitHub: a recognized - * credential or endpoint key in `env` or `/.claude/settings.json`, or - * that file's `apiKeyHelper`. Each source is read independently, so a malformed - * value never masks a usable one. + * Whether the SDK's own account report describes a Claude setup that can serve + * requests on the user's own credentials — the single rule behind both the + * advertised requirement and the native model catalog. Only the SDK can answer + * honestly: a `claude login` credential lives in the macOS keychain, invisible + * to `process.env` and `~/.claude/settings.json` alike. + * + * The two branches must NOT be collapsed. `apiProvider` reports `'firstParty'` + * even for an empty home directory, so it is a presence signal for nobody — it + * is consulted only to spot a *third-party* backend (Bedrock, Vertex, a + * gateway), whose credential fields the SDK documents as absent because auth is + * external. Requiring a credential field there would lock every one of them out. + * + * Says *configured*, not *working*: verifying would cost a billable request per + * check, and the failure being fixed here is genuinely set-up users locked out. */ -export function detectExistingClaudeSetup(homeDir: string, env: NodeJS.ProcessEnv = process.env): boolean { - if (hasNativeClaudeEnv(env)) { - return true; +export function isClaudeAccountSetUp(account: AccountInfo | undefined): boolean { + if (!account) { + return false; } - const settings = readJsonFile(join(homeDir, '.claude', 'settings.json')); - return hasNativeClaudeEnv(claudeSettingsEnvValidator.validate(settings).content?.env) - || hasValue(claudeApiKeyHelperValidator.validate(settings).content?.apiKeyHelper); -} - -/** True when any recognized native-Claude key carries a usable value. */ -function hasNativeClaudeEnv(env: ClaudeNativeEnv | undefined): boolean { - return hasValue(env?.ANTHROPIC_API_KEY) - || hasValue(env?.ANTHROPIC_AUTH_TOKEN) - || hasValue(env?.ANTHROPIC_BASE_URL) - || hasValue(env?.CLAUDE_CODE_OAUTH_TOKEN); -} - -/** A setting counts only when it actually carries a value, never a blank leftover. */ -function hasValue(value: unknown): value is string { - return isString(value) && !isFalsyOrWhitespace(value); -} - -/** Parsed JSON, or `undefined` when the file is missing, unreadable or malformed. */ -function readJsonFile(path: string): unknown { - let text: string; - try { - text = readFileSync(path, 'utf8'); - } catch { - return undefined; + if (account.apiProvider !== undefined && account.apiProvider !== 'firstParty') { + return true; } - // The tolerant parser reports on `errors` rather than throwing, and salvages a - // partial result from broken input — so a truncated file has to be rejected - // here, or half a credential reads as a setup the CLI could not load either. - const errors: ParseError[] = []; - const parsed: unknown = parseJSONC(text, errors, { allowTrailingComma: true, allowEmptyContent: true }); - return errors.length === 0 ? parsed : undefined; + // `tokenSource` spells "no credential" as `'none'` rather than absence; + // `apiKeySource` has only ever been observed absent in that case. + return (account.tokenSource !== undefined && account.tokenSource !== 'none') + || account.apiKeySource !== undefined; } diff --git a/src/vs/platform/agentHost/node/codex/codexAgent.ts b/src/vs/platform/agentHost/node/codex/codexAgent.ts index 79f419b054ffe..2d6b9466075f4 100644 --- a/src/vs/platform/agentHost/node/codex/codexAgent.ts +++ b/src/vs/platform/agentHost/node/codex/codexAgent.ts @@ -25,6 +25,7 @@ import { createSchema, platformRootSchema, platformSessionSchema, schemaProperty import { createPricingMetaFromBilling, normalizeCAPIBilling } from '../../common/agentModelPricing.js'; import { CHATGPT_SUBSCRIPTION_MODEL_SOURCE_ID, createAgentModelSourceMeta } from '../../common/agentModelSource.js'; import { AgentHostConfigKey, agentHostCustomizationConfigSchema } from '../../common/agentHostCustomizationConfig.js'; +import { AgentSdkSetupChannel } from '../agentSdkSetupChannel.js'; import { CODEX_ACCOUNT_META_KEY, CODEX_ACCOUNT_SIGN_IN_REQUEST_KEY, CODEX_ACCOUNT_SIGN_OUT_REQUEST_KEY, type ICodexAccountInfo } from '../../common/codexAccount.js'; import { getReasoningEffortDescription, getReasoningEffortLabel, resolveDefaultReasoningEffort } from '../../common/reasoningEffort.js'; import { AgentSession, AgentSignal, CODEX_AGENT_PROVIDER_ID, IActiveClient, IAgent, IAgentChatConfigCompletionsParams, IAgentChatContext, IAgentChatDataChange, IAgentChatMetadata, IAgentChats, IAgentCreateChatForkSource, IAgentCreateChatResult, IAgentCreateChatOptions, IAgentDescriptor, IAgentDiscoveredChat, IAgentMaterializeChatEvent, IAgentModelInfo, IAgentResolveChatConfigParams, IAgentSpawnChatEvent, IMcpNotification, resolveAgentChatContext, resolveAgentHostInstructions, type AgentProvider, type AuthenticateParams } from '../../common/agent.js'; @@ -138,7 +139,6 @@ import type { ConfigReadResponse } from './protocol/generated/v2/ConfigReadRespo import type { ConfigWriteResponse } from './protocol/generated/v2/ConfigWriteResponse.js'; import { formatGuardianDenialNotification, summarizeGuardianReviewAction, toGuardianAssessmentEventJson } from './codexGuardianReview.js'; import { CODEX_COMPACT_SLASH_COMMAND } from '../codexCompactCommand.js'; -import { detectExistingCodexChatGPTSetup } from './codexLocalAuth.js'; const CLIENT_INFO = { name: 'vscode_agent_host', @@ -175,6 +175,16 @@ const CODEX_THINKING_LEVEL_KEY = 'thinkingLevel'; */ const USER_AGENT_PREFIX = 'vscode_codex'; +/** Where a user finishes setting Codex up outside the app; the workbench labels the button. */ +const CODEX_SETUP_DOCS_URL = 'https://learn.chatgpt.com/codex/auth'; + +/** + * The account the in-app sign-in signs into. A proper noun rather than a + * translatable string, so publishing it keeps user-facing text out of the host — + * the workbench interpolates it into its own localized sentence. + */ +const CODEX_SIGN_IN_PROVIDER_NAME = 'ChatGPT'; + const CODEX_REASONING_EFFORTS: readonly ReasoningEffort[] = ['minimal', 'low', 'medium', 'high']; /** @@ -1146,13 +1156,36 @@ export class CodexAgent extends Disposable implements IAgent { this._configurationService.updateRootConfig({ [CODEX_ACCOUNT_SIGN_OUT_REQUEST_KEY]: undefined }); void this._signOutOfChatGPT(); } - this._startModelRefreshForExistingChatGPTSetup(); + this._startModelRefreshWhenSdkIsLocal(); this._queueProviderConfigurationWrite(); })); void this._refreshProviderConfiguration(); - this._startModelRefreshForExistingChatGPTSetup(); + this._startModelRefreshWhenSdkIsLocal(); + this._sdkSetupChannel = this._register(new AgentSdkSetupChannel({ + id: this.id, + sdkPackage: CodexSdkPackage, + setupInfo: { + setupDocsUrl: CODEX_SETUP_DOCS_URL, + // ChatGPT sign-in is a control request the app server answers, so the + // banner can start it in-window. An API key still has to be established + // outside — hence the docs link alongside it. + signInProviderName: CODEX_SIGN_IN_PROVIDER_NAME, + }, + isSdkLocal: () => this._isSdkResolvableWithoutDownload(), + downloadSdk: async () => { await this._resolveSdkRoot(); }, + restartChatDiscovery: () => this._restartChatDiscovery(), + refreshModels: () => this.refreshModels(), + }, this._configurationService, this._agentSdkDownloader, this._logService)); } + /** + * Publishes whether the SDK is on disk — and nothing about the account, which + * the workbench derives from the model list already flowing over AHP (`ready` + * + zero models → no account). Distinct from {@link _publishAccountInfo}'s + * `vscode.codexAccount` channel, which drives the ChatGPT account menu. + */ + private readonly _sdkSetupChannel: AgentSdkSetupChannel; + private _setOpenAIAccountState(state: ICodexAccountState, _publish = true): void { this._openAIAccountState = state; if (state.status !== 'signedIn' || state.authType !== 'chatgpt') { @@ -1234,13 +1267,14 @@ export class CodexAgent extends Disposable implements IAgent { // #region Auth getProtectedResources(): ProtectedResourceMetadata[] { - // Keep the Copilot resource advertised even when optional so an existing - // token is still forwarded and Copilot-backed models remain additive. - // Without a usable ChatGPT setup, however, Copilot is the only available - // transport and must stay required so the workbench shows its auth gate. + // Always listed, always optional — matching Claude. Listing it is what lets + // the host forward a token to an already-signed-in user (matching ignores + // `required`); the unconditional `required: false` is what stops + // `resolveSignedOutWindowGate` walling off the whole Agents window before + // the user reaches a surface that could explain itself. const copilotResource = this._gitHubEndpointService.getCopilotResource(); return [ - this._hasExistingChatGPTSetup() ? { ...copilotResource, required: false } : copilotResource, + { ...copilotResource, required: false }, this._gitHubEndpointService.getRepoResource(), ]; } @@ -1706,38 +1740,27 @@ export class CodexAgent extends Disposable implements IAgent { this._models.set([...this._copilotModels, ...this._codexModels], undefined); } - private _hasExistingChatGPTSetup(): boolean { - const allowSignedOutWhenUsable = this._configurationService.getRootValue(agentHostCustomizationConfigSchema, AgentHostConfigKey.AllowSignedOutWhenUsable) === true; - if (!allowSignedOutWhenUsable) { - return false; - } - if (this._openAIAccountState.status === 'signedIn') { - return this._openAIAccountState.authType === 'chatgpt'; - } - if (this._openAIAccountState.status === 'unavailable') { - return this._openAIAccountState.requiresOpenaiAuth === false; - } - if (this._openAIAccountState.status === 'signedOut' || this._openAIAccountState.status === 'error') { - return false; - } - return detectExistingCodexChatGPTSetup( - this._environmentService.userHome.fsPath, - process.env, - process.env[AgentHostCodexAgentCodexHomeEnvVar], - ); - } - /** - * Match Claude native mode: once persisted credentials make the provider - * usable without GitHub, eagerly materialize the SDK and publish only the - * authoritative app-server model catalog. Until that finishes the provider - * remains present but unusable; no cached or synthetic model is advertised. + * Ask the app server for the authoritative catalog at startup, but only when + * asking is free — i.e. the SDK is already on disk. + * + * Replaces a `~/.codex/auth.json` sniff that was wrong in both directions: it + * missed API-key setups established through the environment, and claimed a + * setup from a stale token file. Only the app server can answer whether this + * user can run Codex without GitHub. Behind the flag, so a Copilot-only user + * still spawns nothing at startup. */ - private _startModelRefreshForExistingChatGPTSetup(): void { - if (!this._hasExistingChatGPTSetup() || this._codexModels.length > 0) { + private _startModelRefreshWhenSdkIsLocal(): void { + const allowSignedOutWhenUsable = this._configurationService.getRootValue(agentHostCustomizationConfigSchema, AgentHostConfigKey.AllowSignedOutWhenUsable) === true; + if (!allowSignedOutWhenUsable || this._codexModels.length > 0) { return; } - queueMicrotask(() => { void this.refreshModels(); }); + queueMicrotask(async () => { + if (this._store.isDisposed || !(await this._isSdkResolvableWithoutDownload())) { + return; + } + await this.refreshModels(); + }); } private async _refreshCopilotModels(): Promise { @@ -1794,7 +1817,15 @@ export class CodexAgent extends Disposable implements IAgent { private async _refreshCodexModels(): Promise { try { - if (this._connection.kind === 'idle' && !(await this._isSdkResolvableWithoutDownload()) && !this._hasExistingChatGPTSetup()) { + // A refresh must never be what pulls the SDK down — the download is an + // explicit gesture now — so with no local SDK this reports the honest + // empty catalog and the banner offers it. A live connection already + // proves the SDK is on disk, so it short-circuits the stat. + const sdkReady = this._connection.kind !== 'idle' || await this._isSdkResolvableWithoutDownload(); + // Also the freshest answer to "is the SDK here" — a download that landed + // elsewhere (a prewarm, another window) surfaces here. + this._sdkSetupChannel.publishWith(sdkReady); + if (!sdkReady) { this._codexModels = []; return; } @@ -5607,10 +5638,11 @@ export class CodexAgent extends Disposable implements IAgent { } async listChatsToMigrate(): Promise { - try { - await this._resolveSdkRoot(); - } catch (err) { - this._logService.warn(`[Codex] SDK unavailable while listing chats to migrate: ${err instanceof Error ? err.message : String(err)}`); + // `undefined` is "can't enumerate yet", which is the honest answer while the + // SDK is absent: the catalog lives inside it, but fetching one is the user's + // call. {@link _restartChatDiscovery} revisits this once they make it. + if (!(await this._isSdkResolvableWithoutDownload())) { + this._logService.info('[Codex] SDK not downloaded yet; deferring the migratable chat list'); return undefined; } const chats = await this._listCodexChats(); @@ -5627,7 +5659,13 @@ export class CodexAgent extends Disposable implements IAgent { private _startCodexChatDiscovery(): Promise { if (!this._codexChatDiscovery) { this._codexChatDiscovery = retry(async () => { - await this._resolveSdkRoot(); + // Waits for the SDK rather than pulling it down — see + // {@link listChatsToMigrate}. Returning leaves the retry loop happy, + // since no amount of retrying will make the user press Download. + if (!(await this._isSdkResolvableWithoutDownload())) { + this._logService.info('[Codex] SDK not downloaded yet; deferring chat discovery'); + return; + } if (!(await this._emitCodexChats())) { throw new Error('Codex chat catalog is not available'); } @@ -5637,6 +5675,14 @@ export class CodexAgent extends Disposable implements IAgent { return this._codexChatDiscovery; } + /** Runs discovery again for whoever is still subscribed, after it deferred for want of an SDK. */ + private _restartChatDiscovery(): void { + if (this._codexChatDiscovery) { + this._codexChatDiscovery = undefined; + void this._startCodexChatDiscovery(); + } + } + private async _emitCodexChats(): Promise { try { const chats = await this._listCodexChats(); diff --git a/src/vs/platform/agentHost/node/codex/codexLocalAuth.ts b/src/vs/platform/agentHost/node/codex/codexLocalAuth.ts deleted file mode 100644 index b04d0633b4772..0000000000000 --- a/src/vs/platform/agentHost/node/codex/codexLocalAuth.ts +++ /dev/null @@ -1,70 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import { readFileSync } from 'fs'; -import { join } from '../../../../base/common/path.js'; - -/** Resolve the same config directory as Codex without requiring its binary. */ -function resolveCodexHome(userHome: string, env: NodeJS.ProcessEnv, codexHome: string | undefined): string { - return codexHome || env.CODEX_HOME || join(userHome, '.codex'); -} - -function readJson(path: string): unknown { - try { - return JSON.parse(readFileSync(path, 'utf8')); - } catch { - return undefined; - } -} - -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} - -function isNonEmptyString(value: unknown): value is string { - return typeof value === 'string' && value.trim().length > 0; -} - -function hasChatGPTTokens(value: unknown): boolean { - return isRecord(value) && (isNonEmptyString(value.access_token) || isNonEmptyString(value.refresh_token)); -} - -/** - * Detect an existing persisted ChatGPT identity without starting or downloading - * Codex. This mirrors Codex's `AuthDotJson::resolved_mode` classification for - * the modes that `account/read` exposes as a ChatGPT account. API keys, - * Bedrock, headers, and Agent Identity deliberately do not count. - * - * Token expiry is not checked here: managed ChatGPT auth commonly has an - * expired access token alongside a refresh token, and app-server remains the - * authority that refreshes and validates it before the first request. - */ -export function detectExistingCodexChatGPTSetup(userHome: string, env: NodeJS.ProcessEnv = process.env, codexHome?: string): boolean { - const auth = readJson(join(resolveCodexHome(userHome, env, codexHome), 'auth.json')); - if (!isRecord(auth)) { - return false; - } - - const authMode = auth.auth_mode; - if (authMode === 'personalAccessToken') { - return isNonEmptyString(auth.personal_access_token); - } - if (authMode === 'chatgpt' || authMode === 'chatgptAuthTokens') { - return hasChatGPTTokens(auth.tokens); - } - if (authMode !== undefined) { - return false; - } - - // Legacy Codex auth files predate `auth_mode`: PAT wins first, then - // `OPENAI_API_KEY`, and otherwise token material means managed ChatGPT auth. - if (isNonEmptyString(auth.personal_access_token)) { - return true; - } - if (isNonEmptyString(auth.OPENAI_API_KEY) || auth.bedrock_api_key !== undefined || auth.agent_identity !== undefined) { - return false; - } - return hasChatGPTTokens(auth.tokens); -} diff --git a/src/vs/platform/agentHost/test/common/agentSdkSetup.test.ts b/src/vs/platform/agentHost/test/common/agentSdkSetup.test.ts new file mode 100644 index 0000000000000..581fd741f65d1 --- /dev/null +++ b/src/vs/platform/agentHost/test/common/agentSdkSetup.test.ts @@ -0,0 +1,126 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +import { agentSdkSetupStatusKey, isAgentSdkSetupRequestFor, readAgentSdkSetupInfos, readConsentedSdkAgents, resolveConsentedSdkDownloads, writeConsentedSdkAgents, type IAgentSdkSetupInfo } from '../../common/agentSdkSetup.js'; + +suite('Agent SDK setup channel', () => { + ensureNoDisposablesAreLeakedInTestSuite(); + + test('reads one entry per agent, from the transient meta bag', () => { + assert.deepStrictEqual(readAgentSdkSetupInfos({ + agents: [], + _meta: { + [agentSdkSetupStatusKey('claude')]: { download: 'ready', setupDocsUrl: 'https://example.test/claude' }, + [agentSdkSetupStatusKey('codex')]: { download: 'notDownloaded', signInProviderName: 'ChatGPT' }, + }, + }), [ + { agent: 'claude', download: 'ready', setupDocsUrl: 'https://example.test/claude', signInProviderName: undefined }, + { agent: 'codex', download: 'notDownloaded', setupDocsUrl: undefined, signInProviderName: 'ChatGPT' }, + ]); + }); + + test('a persisted config value wins over the transient meta bag for the same agent', () => { + assert.deepStrictEqual(readAgentSdkSetupInfos({ + agents: [], + config: { schema: { type: 'object', properties: {} }, values: { [agentSdkSetupStatusKey('claude')]: { download: 'ready' } } }, + _meta: { [agentSdkSetupStatusKey('claude')]: { download: 'notDownloaded' } }, + }), [ + { agent: 'claude', download: 'ready', setupDocsUrl: undefined, signInProviderName: undefined }, + ]); + }); + + test('an agent that never published is absent rather than guessed at', () => { + assert.deepStrictEqual(readAgentSdkSetupInfos({ agents: [] }), []); + assert.deepStrictEqual(readAgentSdkSetupInfos(undefined), []); + }); + + test('drops entries whose download status is not one we understand', () => { + assert.deepStrictEqual(readAgentSdkSetupInfos({ + agents: [], + _meta: { + [agentSdkSetupStatusKey('claude')]: { download: 'somethingElse' }, + [agentSdkSetupStatusKey('codex')]: 'not an object', + [agentSdkSetupStatusKey('')]: { download: 'ready' }, + 'vscode.codexAccount': { status: 'signedIn' }, + }, + }), []); + }); + + test('drops optional fields that are wrong-typed, or right-typed but useless', () => { + assert.deepStrictEqual(readAgentSdkSetupInfos({ + agents: [], + _meta: { + // An empty provider name would render a "Sign in to " button, so it is + // dropped like a wrong type rather than passed through. + [agentSdkSetupStatusKey('claude')]: { download: 'ready', setupDocsUrl: 42, signInProviderName: '' }, + }, + }), [ + { agent: 'claude', download: 'ready', setupDocsUrl: undefined, signInProviderName: undefined }, + ]); + }); + + test('a request is only for the agent it names, and only when it carries a nonce', () => { + assert.strictEqual(isAgentSdkSetupRequestFor({ agent: 'claude', request: 'abc' }, 'claude'), true); + assert.strictEqual(isAgentSdkSetupRequestFor({ agent: 'claude', request: 'abc' }, 'codex'), false); + assert.strictEqual(isAgentSdkSetupRequestFor({ agent: 'claude', request: '' }, 'claude'), false); + assert.strictEqual(isAgentSdkSetupRequestFor({ agent: 'claude' }, 'claude'), false); + assert.strictEqual(isAgentSdkSetupRequestFor(undefined, 'claude'), false); + // The key is cleared by writing `undefined`, which is what a consumed + // request looks like on the next change event. + assert.strictEqual(isAgentSdkSetupRequestFor('claude', 'claude'), false); + }); + + suite('standing consent', () => { + const claude: IAgentSdkSetupInfo = { agent: 'claude', download: 'notDownloaded' }; + const codex: IAgentSdkSetupInfo = { agent: 'codex', download: 'notDownloaded' }; + const none: ReadonlySet = new Set(); + const both: ReadonlySet = new Set(['claude', 'codex']); + + test('a consented user whose cache a version bump invalidated re-downloads with no gate', () => { + assert.deepStrictEqual(resolveConsentedSdkDownloads(both, [claude, codex], none), ['claude', 'codex']); + }); + + test('a user who never consented still sees the offer', () => { + assert.deepStrictEqual(resolveConsentedSdkDownloads(new Set(), [claude, codex], none), []); + }); + + test('consenting to one agent is not consent to fetch another', () => { + // The button that records this says "download the Codex Agent SDK". + assert.deepStrictEqual(resolveConsentedSdkDownloads(new Set(['codex']), [claude, codex], none), ['codex']); + }); + + test('an SDK already on disk, or already fetching, is left alone', () => { + assert.deepStrictEqual(resolveConsentedSdkDownloads(both, [ + { ...claude, download: 'ready' }, + { ...codex, download: 'downloading' }, + ], none), []); + }); + + test('a download that failed is not retried until the next window', () => { + // The failure republishes `notDownloaded`, and every status change re-runs + // this — without the guard that is an unbounded retry loop. + assert.deepStrictEqual(resolveConsentedSdkDownloads(both, [claude, codex], new Set(['claude'])), ['codex']); + }); + + test('the consent record survives a round trip, and a corrupt one consents to nobody', () => { + assert.deepStrictEqual({ + roundTrip: [...readConsentedSdkAgents(writeConsentedSdkAgents(both))], + absent: [...readConsentedSdkAgents(undefined)], + corrupt: [...readConsentedSdkAgents('{not json')], + wrongShape: [...readConsentedSdkAgents('{"claude":true}')], + // A stray non-string entry drops out rather than poisoning the set. + mixed: [...readConsentedSdkAgents('["claude",7]')], + }, { + roundTrip: ['claude', 'codex'], + absent: [], + corrupt: [], + wrongShape: [], + mixed: ['claude'], + }); + }); + }); +}); diff --git a/src/vs/platform/agentHost/test/node/agentSdkDownloadTelemetry.test.ts b/src/vs/platform/agentHost/test/node/agentSdkDownloadTelemetry.test.ts new file mode 100644 index 0000000000000..22d7529cfd6ea --- /dev/null +++ b/src/vs/platform/agentHost/test/node/agentSdkDownloadTelemetry.test.ts @@ -0,0 +1,36 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +import { classifyAgentSdkDownloadFailure, type AgentSdkDownloadFailureReason } from '../../node/agentSdkDownloadTelemetry.js'; + +// Reporting itself is exercised end-to-end against the real downloader in +// `agentSdkDownloader.test.ts`; only the classifier is worth a table here. +suite('Agent SDK download telemetry', () => { + ensureNoDisposablesAreLeakedInTestSuite(); + + suite('failure classification', () => { + const cases: readonly { readonly name: string; readonly error: string | undefined; readonly expected: AgentSdkDownloadFailureReason }[] = [ + { name: 'the downloader reports cancellation as a bare token, not a message', error: 'cancelled', expected: 'cancelled' }, + { name: 'an HTTP status is the network', error: 'HTTP 503 for https://cdn.example.test/claude-1.2.3.tgz', expected: 'network' }, + { name: 'so is a DNS or TLS failure', error: 'getaddrinfo ENOTFOUND cdn.example.test', expected: 'network' }, + { name: 'a full or read-only disk is not', error: `ENOSPC: no space left on device, write '/home/u/.cache/sdk.tgz'`, expected: 'filesystem' }, + { name: 'nor is a permission denied under the cache dir', error: `EACCES: permission denied, mkdir '/home/u/.cache'`, expected: 'filesystem' }, + { name: 'a corrupt archive is its own bucket', error: 'zlib: incorrect header check', expected: 'extract' }, + { name: 'a build with no SDK configured says so', error: 'no `product.agentSdks.claude` in this build', expected: 'notConfigured' }, + { name: 'and one with no artefact for this platform says that', error: 'no SDK target for this host (linux-riscv64)', expected: 'unsupportedTarget' }, + { name: 'an HTTP failure is not read as a corrupt tarball just because the URL ends in .tgz', error: 'HTTP 404 for https://cdn.example.test/sdk.tar.gz', expected: 'network' }, + { name: 'anything unrecognised stays unknown rather than being folded into a neighbour', error: 'something went wrong', expected: 'unknown' }, + { name: 'a failure with no message at all is unknown too', error: undefined, expected: 'unknown' }, + ]; + + for (const { name, error, expected } of cases) { + test(name, () => { + assert.strictEqual(classifyAgentSdkDownloadFailure(error), expected); + }); + } + }); +}); diff --git a/src/vs/platform/agentHost/test/node/agentSdkDownloader.test.ts b/src/vs/platform/agentHost/test/node/agentSdkDownloader.test.ts index 9df7962594660..b97ae40414277 100644 --- a/src/vs/platform/agentHost/test/node/agentSdkDownloader.test.ts +++ b/src/vs/platform/agentHost/test/node/agentSdkDownloader.test.ts @@ -18,6 +18,8 @@ import { FileService } from '../../../files/common/fileService.js'; import type { IFileService } from '../../../files/common/files.js'; import { DiskFileSystemProvider } from '../../../files/node/diskFileSystemProvider.js'; import { NullLogService } from '../../../log/common/log.js'; +import { NullTelemetryService, NullTelemetryServiceShape } from '../../../telemetry/common/telemetryUtils.js'; +import type { ITelemetryService } from '../../../telemetry/common/telemetry.js'; import { RequestService } from '../../../request/node/requestService.js'; import { AgentSdkDownloader, resolveSdkTarget, type IAgentSdkPackage, type IAgentSdkDownloadProgress } from '../../node/agentSdkDownloader.js'; import { ClaudeSdkPackage } from '../../node/claude/claudeAgentSdkService.js'; @@ -25,6 +27,14 @@ import { AgentHostClaudeSdkRootEnvVar } from '../../common/agentService.js'; import type { INativeEnvironmentService } from '../../../environment/common/environment.js'; import type { IProductService } from '../../../product/common/productService.js'; +class RecordingTelemetryService extends NullTelemetryServiceShape { + readonly events: { name: string; data: Record }[] = []; + + override publicLog2(eventName?: string, data?: Record): void { + this.events.push({ name: eventName ?? '', data: data ?? {} }); + } +} + interface ITestSdkDownloadFixture { tarballPath: string; innerFile: string; // path that should exist inside the extracted root @@ -234,7 +244,7 @@ suite('AgentSdkDownloader', () => { * explicitly. Pass `productConfig: null` to omit the agentSdks block * entirely (the "no product config" case). */ - function makeDownloader(productConfig?: { version?: string; urlTemplate?: string } | null) { + function makeDownloader(productConfig?: { version?: string; urlTemplate?: string } | null, telemetryService: ITelemetryService = NullTelemetryService) { const config = productConfig === null ? undefined : { version: productConfig?.version ?? '1.0.0', urlTemplate: productConfig?.urlTemplate ?? `http://127.0.0.1:${server.port}/sdk-{sdkTarget}.tgz`, @@ -245,6 +255,7 @@ suite('AgentSdkDownloader', () => { makeRequestService(disposables), makeFileService(disposables), new NullLogService(), + telemetryService, )); } @@ -306,8 +317,42 @@ suite('AgentSdkDownloader', () => { assert.strictEqual(completed.receivedBytes, tarballSize); }); + test('loadSdkRoot: counts only the endpoints of a download, with the time it took', async () => { + const telemetry = new RecordingTelemetryService(); + const downloader = makeDownloader(undefined, telemetry); + + await downloader.loadSdkRoot(ClaudeSdkPackage, newToken()); + + // The throttled `progress` frames drive the progress bar, not the funnel. + assert.deepStrictEqual(telemetry.events.map(event => [event.name, event.data.phase]), [ + ['agentHost.agentSdkDownload', 'started'], + ['agentHost.agentSdkDownload', 'completed'], + ]); + const completed = telemetry.events[1].data; + assert.strictEqual(completed.packageId, 'claude'); + assert.strictEqual(completed.failureReason, ''); + assert.strictEqual(completed.explicitlyRequested, false); + assert.ok(typeof completed.durationMs === 'number' && completed.durationMs >= 0); + }); + + test('loadSdkRoot: a failed download reports its bucket, never the raw cause', async () => { + const telemetry = new RecordingTelemetryService(); + // Port 1 on loopback refuses instantly, so this is a network failure with + // no bytes and no advertised total. + const downloader = makeDownloader({ urlTemplate: 'http://127.0.0.1:1/sdk-{sdkTarget}.tgz' }, telemetry); + + await assert.rejects(() => downloader.loadSdkRoot(ClaudeSdkPackage, newToken())); + + const failure = telemetry.events[telemetry.events.length - 1].data; + assert.strictEqual(failure.phase, 'failed'); + assert.strictEqual(failure.failureReason, 'network'); + assert.strictEqual(failure.totalBytes, 0, 'an unknown total is reported as zero rather than dropped'); + assert.ok(!JSON.stringify(failure).includes(userDataPath), 'the on-disk cache path must not reach telemetry'); + }); + test('loadSdkRoot: marks progress explicitly requested by a user-initiated flow', async () => { - const downloader = makeDownloader(); + const telemetry = new RecordingTelemetryService(); + const downloader = makeDownloader(undefined, telemetry); const samples: IAgentSdkDownloadProgress[] = []; disposables.add(downloader.onDidDownloadProgress(p => samples.push(p))); disposables.add(downloader.acquireDownloadProgressInterest(ClaudeSdkPackage)); @@ -316,6 +361,30 @@ suite('AgentSdkDownloader', () => { assert.ok(samples.length >= 2); assert.ok(samples.every(sample => sample.explicitlyRequested)); + // The same split reaches telemetry, which is what separates a button press + // from a quiet re-fetch under standing consent. + assert.ok(telemetry.events.every(event => event.data.explicitlyRequested === true)); + }); + + test('loadSdkRoot: a user asking by hand retries through the negative cache', async () => { + // Port 1 refuses instantly, so every attempt here fails. The latch rethrows + // the *same* error it stored, while a real attempt builds a new one — which + // is how this tells a short-circuit from a retry without a second server. + const downloader = makeDownloader({ urlTemplate: 'http://127.0.0.1:1/sdk-{sdkTarget}.tgz' }); + const failure = async () => { + try { + await downloader.loadSdkRoot(ClaudeSdkPackage, newToken()); + throw new Error('expected the download to fail'); + } catch (err) { + return err; + } + }; + + const first = await failure(); + assert.strictEqual(await failure(), first, 'a background caller is short-circuited by the latch'); + + disposables.add(downloader.acquireDownloadProgressInterest(ClaudeSdkPackage)); + assert.notStrictEqual(await failure(), first, 'an explicit request is a fresh mandate to try again'); }); test('loadSdkRoot: cache hit returns immediately without re-downloading', async () => { diff --git a/src/vs/platform/agentHost/test/node/claudeAgent.integrationTest.ts b/src/vs/platform/agentHost/test/node/claudeAgent.integrationTest.ts index 69166cf147cb4..1ba7a034e411f 100644 --- a/src/vs/platform/agentHost/test/node/claudeAgent.integrationTest.ts +++ b/src/vs/platform/agentHost/test/node/claudeAgent.integrationTest.ts @@ -405,7 +405,7 @@ class ProxyRoundTripSdkService implements IClaudeAgentSdkService { return true; } - async ensureAvailableForDiscovery(): Promise { } + async ensureAvailable(): Promise { } async getSessionInfo(_sessionId: string): Promise { return undefined; diff --git a/src/vs/platform/agentHost/test/node/claudeAgent.test.ts b/src/vs/platform/agentHost/test/node/claudeAgent.test.ts index 298c1648327a4..748fe18966afd 100644 --- a/src/vs/platform/agentHost/test/node/claudeAgent.test.ts +++ b/src/vs/platform/agentHost/test/node/claudeAgent.test.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import type Anthropic from '@anthropic-ai/sdk'; -import type { AgentInfo, ForkSessionOptions, ForkSessionResult, GetSessionMessagesOptions, McpSdkServerConfigWithInstance, McpServerStatus, ModelInfo, Options, PermissionMode, Query, SDKControlInterruptResponse, SDKMessage, SDKSessionInfo, SDKUserMessage, SdkMcpToolDefinition, SessionMessage, SessionMutationOptions, Settings, SlashCommand, WarmQuery } from '@anthropic-ai/claude-agent-sdk'; +import type { AccountInfo, AgentInfo, ForkSessionOptions, ForkSessionResult, GetSessionMessagesOptions, McpSdkServerConfigWithInstance, McpServerStatus, ModelInfo, Options, PermissionMode, Query, SDKControlInterruptResponse, SDKMessage, SDKSessionInfo, SDKUserMessage, SdkMcpToolDefinition, SessionMessage, SessionMutationOptions, Settings, SlashCommand, WarmQuery } from '@anthropic-ai/claude-agent-sdk'; import type { CallToolResult } from '@modelcontextprotocol/sdk/types.js'; import type { CCAModel } from '@vscode/copilot-api'; @@ -32,7 +32,6 @@ import { VSBuffer } from '../../../../base/common/buffer.js'; import { Emitter, Event } from '../../../../base/common/event.js'; import { DisposableStore, toDisposable } from '../../../../base/common/lifecycle.js'; import { URI } from '../../../../base/common/uri.js'; -import { join } from '../../../../base/common/path.js'; import { generateUuid, isUUID } from '../../../../base/common/uuid.js'; import { isCancellationError } from '../../../../base/common/errors.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; @@ -78,7 +77,9 @@ import { createClaudeInternalMcpServerCustomization } from '../../node/claude/cu import { ClaudeSessionMetadataStore } from '../../node/claude/claudeSessionMetadataStore.js'; import { ClaudeSessionConfigKey } from '../../common/claudeSessionConfigKeys.js'; import { ClaudeAgentSdkService, IClaudeAgentSdkService, IClaudeSdkBindings } from '../../node/claude/claudeAgentSdkService.js'; +import { AGENT_SDK_SETUP_DOWNLOAD_REQUEST_KEY, readAgentSdkSetupInfos } from '../../common/agentSdkSetup.js'; import { IAgentSdkDownloader } from '../../node/agentSdkDownloader.js'; +import { RecordingAgentSdkDownloader } from './testAgentSdkDownloader.js'; import { PendingRequestRegistry } from '../../common/pendingRequestRegistry.js'; import { IClaudeProxyCreditsReport, IClaudeProxyHandle, IClaudeProxyService } from '../../node/claude/claudeProxyService.js'; import { resolvePromptToContentBlocks } from '../../node/claude/claudePromptResolver.js'; @@ -508,6 +509,14 @@ class FakeClaudeAgentSdkService implements IClaudeAgentSdkService { supportedModelsCallCount = 0; readonly supportedModelsOptions: Options[] = []; + /** + * Programmable `accountInfo()` report. Defaults to the shape measured on a + * machine with nothing configured, so a test that does not opt in gets the + * honest "no account" answer; {@link NATIVE_ACCOUNT} is the opt-in. + */ + accountInfoResult: AccountInfo = { tokenSource: 'none', apiProvider: 'firstParty' }; + accountInfoCallCount = 0; + /** * Optional gate awaited by {@link FakeQuery.supportedModels} before it * resolves. Lets a test park the native half of a merged refresh mid-flight @@ -517,6 +526,13 @@ class FakeClaudeAgentSdkService implements IClaudeAgentSdkService { */ supportedModelsGate: Promise | undefined; + /** + * Programmable rejection for the native half of a merged refresh. Distinct + * from a *fulfilled* empty enumeration, which is an honest "no native models"; + * a rejection is "we could not find out". + */ + supportedModelsRejection: Error | undefined; + /** All warm queries produced by {@link startup}. Last entry is the most recent. */ readonly warmQueries: FakeWarmQuery[] = []; @@ -543,11 +559,24 @@ class FakeClaudeAgentSdkService implements IClaudeAgentSdkService { return this.canLoadWithoutDownloadResult; } - ensureAvailableForDiscoveryCalls = 0; - async ensureAvailableForDiscovery(): Promise { - this.ensureAvailableForDiscoveryCalls++; + ensureAvailableCalls = 0; + async ensureAvailable(): Promise { + this.ensureAvailableCalls++; + if (this.ensureAvailableRejection) { + throw this.ensureAvailableRejection; + } + // Deliberately does NOT flip {@link canLoadWithoutDownloadResult}: a real + // fetch takes seconds, so tests stage that flip themselves when they + // release the gate. + await this.ensureAvailableGate; } + /** Optional gate awaited by {@link ensureAvailable}, so a test can park a download mid-flight. */ + ensureAvailableGate: Promise | undefined; + + /** Programmable failure for an explicit download (dead CDN, disk full). */ + ensureAvailableRejection: Error | undefined; + /** * Programmable result for {@link canLoadWithoutDownload}. Defaults to * `true` (SDK already local). Set to `false` to simulate the cold-start @@ -874,6 +903,9 @@ class FakeQuery implements AsyncGenerator { } supportedModels(): Promise { this._sdk.supportedModelsCallCount++; + if (this._sdk.supportedModelsRejection) { + return Promise.reject(this._sdk.supportedModelsRejection); + } const gate = this._sdk.supportedModelsGate; return gate ? gate.then(() => this._sdk.supportedModelsResult) : Promise.resolve(this._sdk.supportedModelsResult); } @@ -903,7 +935,10 @@ class FakeQuery implements AsyncGenerator { error_count: 0, }) as never; } - accountInfo(): never { throw new Error('FakeQuery: accountInfo not modeled'); } + accountInfo(): Promise { + this._sdk.accountInfoCallCount++; + return Promise.resolve(this._sdk.accountInfoResult); + } rewindFiles(): never { throw new Error('FakeQuery: rewindFiles not modeled'); } readFile(): never { throw new Error('FakeQuery: readFile not modeled'); } seedReadState(): never { throw new Error('FakeQuery: seedReadState not modeled'); } @@ -1059,6 +1094,7 @@ interface ITestContext { readonly otelService: RecordingOTelService; readonly instantiationService: IInstantiationService; readonly fileService: IFileService; + readonly sdkDownloader: RecordingAgentSdkDownloader; } /** @@ -1083,12 +1119,17 @@ class CapturingLogService extends NullLogService { function createTestContext( disposables: Pick, - overrides?: { logService?: ILogService; database?: TestSessionDatabase; sessionDataService?: ISessionDataService; rootConfig?: Record; userHome?: URI; gitHubEndpointService?: IAgentHostGitHubEndpointService; checkpointService?: IAgentHostCheckpointService }, + overrides?: { logService?: ILogService; database?: TestSessionDatabase; sessionDataService?: ISessionDataService; rootConfig?: Record; userHome?: URI; gitHubEndpointService?: IAgentHostGitHubEndpointService; checkpointService?: IAgentHostCheckpointService; nativeAccount?: AccountInfo }, ): ITestContext { const proxy = new FakeClaudeProxyService(); const api = new FakeCopilotApiService(); api.models = async () => [...ALL_MODELS]; const sdk = new FakeClaudeAgentSdkService(); + // Staged before the agent is constructed: its ctor queues the first model + // refresh, which is what asks for the account. + if (overrides?.nativeAccount) { + sdk.accountInfoResult = overrides.nativeAccount; + } const sessionData = new RecordingSessionDataService( overrides?.sessionDataService ?? (overrides?.database @@ -1106,6 +1147,7 @@ function createTestContext( disposables.add(fileService.registerProvider(Schemas.file, disposables.add(new InMemoryFileSystemProvider()))); const otelService = new RecordingOTelService(); + const sdkDownloader = new RecordingAgentSdkDownloader(); const services = new ServiceCollection( [IFileService, fileService], [INativeEnvironmentService, { userHome: overrides?.userHome ?? URI.file('/mock-home') } as INativeEnvironmentService], @@ -1114,6 +1156,7 @@ function createTestContext( [IClaudeProxyService, proxy], [ISessionDataService, sessionData], [IClaudeAgentSdkService, sdk], + [IAgentSdkDownloader, sdkDownloader], [IAgentPluginManager, new FakeAgentPluginManager()], [IAgentHostGitService, createNoopGitService()], [IAgentHostCheckpointService, overrides?.checkpointService ?? NULL_CHECKPOINT_SERVICE], @@ -1167,7 +1210,7 @@ function createTestContext( chats.changeAgent = (chat, nextAgent, context) => changeAgent(chat, nextAgent, toChatContext(chat, context)); const getMessages = chats.getMessages.bind(agent.chats); chats.getMessages = (chat, context) => getMessages(chat, toChatContext(chat, context)); - return { agent, proxy, api, sdk, sessionData, stateManager, configService, otelService, instantiationService, fileService }; + return { agent, proxy, api, sdk, sessionData, stateManager, configService, otelService, instantiationService, fileService, sdkDownloader }; } /** Drains the microtask queue so awaited refresh writes settle. */ @@ -1176,21 +1219,12 @@ function tick(): Promise { } /** - * Run `body` against a temp `$HOME/.claude/settings.json` carrying an Anthropic - * key so {@link detectExistingClaudeSetup} reports a usable native setup, then - * always clean the directory up. Pair with `allowSignedOutWhenUsable` to make a - * signed-out agent resolve its model-less default to native. + * The SDK account report of a user signed in on their own credentials — the + * `claude login` / keychain case no filesystem check could ever see. Pass as + * `nativeAccount` to make an agent publish native models. */ -async function withNativeSetup(body: (userHome: URI) => Promise): Promise { - const userHome = URI.file(await fs.mkdtemp(`${os.tmpdir()}/claude-native-setup-`)); - await fs.mkdir(join(userHome.fsPath, '.claude'), { recursive: true }); - await fs.writeFile(join(userHome.fsPath, '.claude', 'settings.json'), JSON.stringify({ env: { ANTHROPIC_API_KEY: 'sk-ant-test-key' } }), 'utf8'); - try { - await body(userHome); - } finally { - await fs.rm(userHome.fsPath, { recursive: true, force: true }); - } -} +const NATIVE_ACCOUNT: AccountInfo = { tokenSource: 'ANTHROPIC_AUTH_TOKEN', apiProvider: 'firstParty' }; + /** * A two-turn source transcript (`u1`/`a1`, `u2`/`a2`) used by the Phase 6.5 @@ -1206,22 +1240,6 @@ function forkSourceMessages(sourceId: string): SessionMessage[] { ]; } -/** - * Stub for {@link IAgentSdkDownloader} consumed by tests that need a real - * `ClaudeAgentSdkService` constructor but override `_loadSdk` themselves — - * the downloader is therefore never actually called. - */ -function stubAgentSdkDownloader(): IAgentSdkDownloader { - return { - _serviceBrand: undefined, - onDidDownloadProgress: Event.None, - acquireDownloadProgressInterest: () => toDisposable(() => { }), - isAvailable: () => false, - isSdkResolvableWithoutDownload: async () => false, - loadSdkRoot: () => { throw new Error('test stub: downloader.loadSdkRoot should not be called'); }, - }; -} - /** * Foundational services every {@link ClaudeAgentSession} requires for its * customization disk scan: an in-memory {@link IFileService} (nothing is @@ -1331,7 +1349,9 @@ suite('ClaudeAgent', () => { resource_name: 'GitHub Copilot', authorization_servers: ['https://github.com/login/oauth'], scopes_supported: ['read:user', 'user:email'], - required: true, + // Shape check; the `required` flag itself is the subject of + // 'the Copilot resource is unconditionally optional […]'. + required: false, }, { resource: 'https://api.github.com/repos', resource_name: 'GitHub Repository', @@ -1390,51 +1410,48 @@ suite('ClaudeAgent', () => { }); test('signed-in probe flips inferred-native to proxy (allowSignedOutWhenUsable)', async () => { - // The fix for the startup catch-22: with the exp flag on and a local Claude - // setup present, a signed-OUT user resolves to native — which still - // advertises the Copilot resource as not-required so the host can probe. If - // the host then silently forwards a GitHub token (the user was signed in all - // along), the acquired proxy handle re-resolves the default (rule 2: signed - // in ⇒ proxy) and flips the transport to proxy, starting the proxy. Real - // detection is used against a real `~/.claude/settings.json` credential under - // a temp home. - await withNativeSetup(async userHome => { - const { agent, proxy } = createTestContext(disposables, { - rootConfig: { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }, - userHome, - }); - // Signed out at startup ⇒ native, Copilot advertised as not-required. - const before = { - resources: agent.getProtectedResources().map(r => ({ resource: r.resource, required: r.required })), - proxyStarts: proxy.startCalls.length, - }; + // The fix for the startup catch-22: with the exp flag on and the SDK + // reporting a Claude account, a signed-OUT user resolves to native — which + // still advertises the Copilot resource as not-required so the host can + // probe. If the host then silently forwards a GitHub token (the user was + // signed in all along), the acquired proxy handle re-resolves the default + // (rule 2: signed in ⇒ proxy) and flips the transport to proxy, starting + // the proxy. + const { agent, proxy } = createTestContext(disposables, { + rootConfig: { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }, + nativeAccount: NATIVE_ACCOUNT, + }); + // Signed out at startup ⇒ native, Copilot advertised as not-required. + const before = { + resources: agent.getProtectedResources().map(r => ({ resource: r.resource, required: r.required })), + proxyStarts: proxy.startCalls.length, + }; - // Host probe forwards a GitHub token (user was signed in) ⇒ flip to proxy. - await agent.authenticate('https://api.github.com', 'gh-token'); - await tick(); + // Host probe forwards a GitHub token (user was signed in) ⇒ flip to proxy. + await agent.authenticate('https://api.github.com', 'gh-token'); + await tick(); - assert.deepStrictEqual({ - before, - after: { - resources: agent.getProtectedResources().map(r => ({ resource: r.resource, required: r.required })), - proxyStarts: proxy.startCalls.length, - }, - }, { - before: { - resources: [ - { resource: 'https://api.github.com', required: false }, - { resource: 'https://api.github.com/repos', required: false }, - ], - proxyStarts: 0, - }, - after: { - resources: [ - { resource: 'https://api.github.com', required: false }, - { resource: 'https://api.github.com/repos', required: false }, - ], - proxyStarts: 1, - }, - }); + assert.deepStrictEqual({ + before, + after: { + resources: agent.getProtectedResources().map(r => ({ resource: r.resource, required: r.required })), + proxyStarts: proxy.startCalls.length, + }, + }, { + before: { + resources: [ + { resource: 'https://api.github.com', required: false }, + { resource: 'https://api.github.com/repos', required: false }, + ], + proxyStarts: 0, + }, + after: { + resources: [ + { resource: 'https://api.github.com', required: false }, + { resource: 'https://api.github.com/repos', required: false }, + ], + proxyStarts: 1, + }, }); }); @@ -1463,17 +1480,52 @@ suite('ClaudeAgent', () => { }); }); - test('keeps the last known-good models when a periodic refresh fails', async () => { - const { agent, api } = createTestContext(disposables); + test('keeps the last known-good models only when every attempted source fails', async () => { + // Retention is all-or-nothing across the merged catalog: a source that + // *answers* is authoritative for its own half. Asking the SDK on every + // refresh widened where that bites — a Copilot-only user used to skip the + // native half entirely, so a CAPI hiccup held their picker; now the native + // half answers "no account" and the merged write drops the stale rows. + const { agent, api, sdk } = createTestContext(disposables); api.models = async () => [...ALL_MODELS]; await agent.authenticate('https://api.github.com', 'tok'); await agent.refreshModels(); - const modelIds = agent.models.get().map(model => model.id); + const populated = agent.models.get().map(model => model.id); + + // Only the proxy fails; the native half answers honestly (no account, so no + // models) and that answer is published. + api.models = async () => { throw new Error('transient failure'); }; + await agent.refreshModels(); + const proxyOnlyFailed = agent.models.get().map(model => model.id); + // Now nothing can answer: the catalog is held rather than blanked again. + api.models = async () => [...ALL_MODELS]; + await agent.refreshModels(); + const republished = agent.models.get().map(model => model.id); api.models = async () => { throw new Error('transient failure'); }; + sdk.supportedModelsRejection = new Error('sdk subprocess died'); await agent.refreshModels(); - assert.deepStrictEqual(agent.models.get().map(model => model.id), modelIds); + assert.deepStrictEqual({ + populated, + proxyOnlyFailed, + republished, + bothFailed: agent.models.get().map(model => model.id), + }, { + populated: [ + toClaudeModelSelectionId(CLAUDE_PROVIDER_COPILOT, 'claude-opus-4.6'), + toClaudeModelSelectionId(CLAUDE_PROVIDER_COPILOT, 'claude-sonnet-4.6'), + ], + proxyOnlyFailed: [], + republished: [ + toClaudeModelSelectionId(CLAUDE_PROVIDER_COPILOT, 'claude-opus-4.6'), + toClaudeModelSelectionId(CLAUDE_PROVIDER_COPILOT, 'claude-sonnet-4.6'), + ], + bothFailed: [ + toClaudeModelSelectionId(CLAUDE_PROVIDER_COPILOT, 'claude-opus-4.6'), + toClaudeModelSelectionId(CLAUDE_PROVIDER_COPILOT, 'claude-sonnet-4.6'), + ], + }); }); test('clears models when enumeration for a replacement token fails', async () => { @@ -1499,73 +1551,64 @@ suite('ClaudeAgent', () => { // superseded account to drop, and blanking would close the // `allowSignedOutWhenUsable` gate mid-startup and force the sign-in dialog // on a user who is already signing in. - const userHome = URI.file(await fs.mkdtemp(`${os.tmpdir()}/claude-first-signin-`)); - await fs.mkdir(join(userHome.fsPath, '.claude'), { recursive: true }); - await fs.writeFile(join(userHome.fsPath, '.claude', 'settings.json'), JSON.stringify({ env: { ANTHROPIC_API_KEY: 'sk-ant-test-key' } }), 'utf8'); - try { - const { agent, api, sdk } = createTestContext(disposables, { userHome }); - sdk.supportedModelsResult = [ - { value: 'claude-sonnet-4-5-20250929', displayName: 'Claude Sonnet 4.5', description: '', supportedEffortLevels: ['high'] }, - ]; - // The constructor's bootstrap refresh publishes native-only (no token yet). - for (let i = 0; i < 100 && agent.models.get().length === 0; i++) { - await tick(); - } - const bootstrap = agent.models.get().map(model => model.name); + const { agent, api, sdk } = createTestContext(disposables, { nativeAccount: NATIVE_ACCOUNT }); + sdk.supportedModelsResult = [ + { value: 'claude-sonnet-4-5-20250929', displayName: 'Claude Sonnet 4.5', description: '', supportedEffortLevels: ['high'] }, + ]; + // The constructor's bootstrap refresh publishes native-only (no token yet). + for (let i = 0; i < 100 && agent.models.get().length === 0; i++) { + await tick(); + } + const bootstrap = agent.models.get().map(model => model.name); - // Hold the CAPI enumeration open so the post-sign-in refresh is still in - // flight when we sample the catalog — that pending window is exactly what - // the renderer saw as an empty (and therefore `Unusable`) agent. - const gate = new DeferredPromise(); - api.models = async () => { await gate.p; return [...ALL_MODELS]; }; - await agent.authenticate('https://api.github.com', 'tok'); - const whileEnumerating = agent.models.get().map(model => model.name); + // Hold the CAPI enumeration open so the post-sign-in refresh is still in + // flight when we sample the catalog — that pending window is exactly what + // the renderer saw as an empty (and therefore `Unusable`) agent. + const gate = new DeferredPromise(); + api.models = async () => { await gate.p; return [...ALL_MODELS]; }; + await agent.authenticate('https://api.github.com', 'tok'); + const whileEnumerating = agent.models.get().map(model => model.name); - gate.complete(); - await agent.refreshModels(); + gate.complete(); + await agent.refreshModels(); - assert.deepStrictEqual({ - bootstrap, - whileEnumerating, - merged: agent.models.get().map(model => model.name), - }, { - bootstrap: ['Claude Sonnet 4.5'], - whileEnumerating: ['Claude Sonnet 4.5'], - merged: ['Claude Opus 4.6', 'Claude Sonnet 4.6', 'Claude Sonnet 4.5'], - }); - } finally { - await fs.rm(userHome.fsPath, { recursive: true, force: true }); - } + assert.deepStrictEqual({ + bootstrap, + whileEnumerating, + merged: agent.models.get().map(model => model.name), + }, { + bootstrap: ['Claude Sonnet 4.5'], + whileEnumerating: ['Claude Sonnet 4.5'], + merged: ['Claude Opus 4.6', 'Claude Sonnet 4.6', 'Claude Sonnet 4.5'], + }); }); - test('signed out with a local setup: models populate from supportedModels() with no proxy start and no CAPI models() call', async () => { - // Native enumeration only runs when a credential is actually present, so - // give this a real `~/.claude/settings.json` under a temp home. Signed out, - // so the proxy half of the merged catalog contributes nothing. - await withNativeSetup(async userHome => { - const { agent, proxy, api, sdk } = createTestContext(disposables, { userHome }); - let capiModelsCalls = 0; - api.models = async () => { capiModelsCalls++; return []; }; - sdk.supportedModelsResult = [ - { value: 'claude-sonnet-4-5-20250929', displayName: 'Claude Sonnet 4.5', description: '', supportedEffortLevels: ['high'] }, - ]; - // The constructor kicks off an initial native refresh; `_fetchNativeModels` - // awaits a real `mkdtemp` before enumerating, so poll until it lands. - for (let i = 0; i < 100 && sdk.supportedModelsCallCount === 0; i++) { - await tick(); - } + test('signed out with an SDK-reported account: models populate from supportedModels() with no proxy start and no CAPI models() call', async () => { + // Native enumeration only publishes when the SDK's own account report says + // the user is set up, so hand it one. Signed out, so the proxy half of the + // merged catalog contributes nothing. + const { agent, proxy, api, sdk } = createTestContext(disposables, { nativeAccount: NATIVE_ACCOUNT }); + let capiModelsCalls = 0; + api.models = async () => { capiModelsCalls++; return []; }; + sdk.supportedModelsResult = [ + { value: 'claude-sonnet-4-5-20250929', displayName: 'Claude Sonnet 4.5', description: '', supportedEffortLevels: ['high'] }, + ]; + // The constructor kicks off an initial native refresh; `_fetchNativeModels` + // awaits a real `mkdtemp` before enumerating, so poll until it lands. + for (let i = 0; i < 100 && sdk.supportedModelsCallCount === 0; i++) { await tick(); - assert.deepStrictEqual({ - models: agent.models.get().map(m => ({ id: m.id, name: m.name })), - proxyStarts: proxy.startCalls.length, - supportedModelsCalls: sdk.supportedModelsCallCount, - capiModelsCalls, - }, { - models: [{ id: toClaudeModelSelectionId(CLAUDE_PROVIDER_ANTHROPIC, 'claude-sonnet-4-5-20250929'), name: 'Claude Sonnet 4.5' }], - proxyStarts: 0, - supportedModelsCalls: 1, - capiModelsCalls: 0, - }); + } + await tick(); + assert.deepStrictEqual({ + models: agent.models.get().map(m => ({ id: m.id, name: m.name })), + proxyStarts: proxy.startCalls.length, + supportedModelsCalls: sdk.supportedModelsCallCount, + capiModelsCalls, + }, { + models: [{ id: toClaudeModelSelectionId(CLAUDE_PROVIDER_ANTHROPIC, 'claude-sonnet-4-5-20250929'), name: 'Claude Sonnet 4.5' }], + proxyStarts: 0, + supportedModelsCalls: 1, + capiModelsCalls: 0, }); }); @@ -1575,62 +1618,62 @@ suite('ClaudeAgent', () => { // configured to use. Published next to the Copilot-routed models it reads // as a third, unrelated choice whose target is invisible, so it is // filtered out — the model it resolves to is already its own row. - await withNativeSetup(async userHome => { - const { agent, sdk } = createTestContext(disposables, { userHome }); - sdk.supportedModelsResult = [ - { value: 'default', resolvedModel: 'claude-sonnet-4-5-20250929', displayName: 'Default (recommended)', description: '' }, - { value: 'claude-sonnet-4-5-20250929', displayName: 'Claude Sonnet 4.5', description: '' }, - ]; - for (let i = 0; i < 100 && sdk.supportedModelsCallCount === 0; i++) { - await tick(); - } + const { agent, sdk } = createTestContext(disposables, { nativeAccount: NATIVE_ACCOUNT }); + sdk.supportedModelsResult = [ + { value: 'default', resolvedModel: 'claude-sonnet-4-5-20250929', displayName: 'Default (recommended)', description: '' }, + { value: 'claude-sonnet-4-5-20250929', displayName: 'Claude Sonnet 4.5', description: '' }, + ]; + for (let i = 0; i < 100 && sdk.supportedModelsCallCount === 0; i++) { await tick(); - assert.deepStrictEqual(agent.models.get().map(m => ({ id: m.id, name: m.name })), [ - { id: toClaudeModelSelectionId(CLAUDE_PROVIDER_ANTHROPIC, 'claude-sonnet-4-5-20250929'), name: 'Claude Sonnet 4.5' }, - ]); - }); + } + await tick(); + assert.deepStrictEqual(agent.models.get().map(m => ({ id: m.id, name: m.name })), [ + { id: toClaudeModelSelectionId(CLAUDE_PROVIDER_ANTHROPIC, 'claude-sonnet-4-5-20250929'), name: 'Claude Sonnet 4.5' }, + ]); }); - test('signed out without a credential publishes an empty catalog instead of the SDK static list', async () => { + test('an SDK account report of "nothing configured" publishes an empty catalog instead of the SDK static list', async () => { // `supportedModels()` answers even with no credentials (it is a static // catalog), so publishing it would advertise models that fail on first // use — and would make the type look usable-without-GitHub to the window - // gate. `/mock-home` has no `.claude` credential, so the native half is - // never attempted; signed out, neither is the proxy half. + // gate. The gate is `accountInfo()`, not the model list: both are asked + // (they are local, cheap calls against an already-present SDK) and the + // account report is what decides whether the models are published. const { agent, sdk } = createTestContext(disposables); sdk.supportedModelsResult = [ { value: 'claude-sonnet-4-5-20250929', displayName: 'Claude Sonnet 4.5', description: '' }, ]; - for (let i = 0; i < 20; i++) { + for (let i = 0; i < 100 && sdk.accountInfoCallCount === 0; i++) { await tick(); } + await tick(); assert.deepStrictEqual({ models: agent.models.get(), + accountInfoCalls: sdk.accountInfoCallCount, supportedModelsCalls: sdk.supportedModelsCallCount, }, { models: [], - supportedModelsCalls: 0, + accountInfoCalls: 1, + supportedModelsCalls: 1, }); }); test('native model enumeration closes the throwaway query (no leaked subprocess)', async () => { - await withNativeSetup(async userHome => { - const { sdk } = createTestContext(disposables, { userHome }); - sdk.supportedModelsResult = [ - { value: 'claude-sonnet-4-5-20250929', displayName: 'Claude Sonnet 4.5', description: '' }, - ]; - // The constructor kicks off the initial native enumeration; wait for it. - for (let i = 0; i < 100 && sdk.supportedModelsCallCount === 0; i++) { - await tick(); - } + const { sdk } = createTestContext(disposables, { nativeAccount: NATIVE_ACCOUNT }); + sdk.supportedModelsResult = [ + { value: 'claude-sonnet-4-5-20250929', displayName: 'Claude Sonnet 4.5', description: '' }, + ]; + // The constructor kicks off the initial native enumeration; wait for it. + for (let i = 0; i < 100 && sdk.supportedModelsCallCount === 0; i++) { await tick(); - assert.deepStrictEqual({ - queries: sdk.enumerationQueries.length, - closed: sdk.enumerationQueries[0]?.closeCount, - }, { - queries: 1, - closed: 1, - }); + } + await tick(); + assert.deepStrictEqual({ + queries: sdk.enumerationQueries.length, + closed: sdk.enumerationQueries[0]?.closeCount, + }, { + queries: 1, + closed: 1, }); }); @@ -1640,15 +1683,13 @@ suite('ClaudeAgent', () => { // so a session that later picks a Copilot-routed model has a started proxy // to run against — even though the model-less default // (`_defaultTransportMode`) was native right up to this call. - await withNativeSetup(async userHome => { - const { agent, proxy } = createTestContext(disposables, { - rootConfig: { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }, - userHome, - }); - const accepted = await agent.authenticate('https://api.github.com', 'tok'); - await tick(); - assert.deepStrictEqual({ accepted, proxyStarts: proxy.startCalls.length }, { accepted: true, proxyStarts: 1 }); + const { agent, proxy } = createTestContext(disposables, { + rootConfig: { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }, + nativeAccount: NATIVE_ACCOUNT, }); + const accepted = await agent.authenticate('https://api.github.com', 'tok'); + await tick(); + assert.deepStrictEqual({ accepted, proxyStarts: proxy.startCalls.length }, { accepted: true, proxyStarts: 1 }); }); test('a host-default transport flip no longer proactively demands auth (sign-in defers to first send)', async () => { @@ -1658,16 +1699,14 @@ suite('ClaudeAgent', () => { // `auth/required`. Sign-in for a Copilot-routed model defers to the first // send, where `_ensureAuthenticated` throws `AHP_AUTH_REQUIRED`. Signing in // is the surviving runtime flip lever (native default → proxy default). - await withNativeSetup(async userHome => { - const { agent } = createTestContext(disposables, { - rootConfig: { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }, - userHome, - }); - await agent.authenticate('https://api.github.com', 'tok'); - await tick(); - - assert.strictEqual((agent as IAgent).authenticationRequired, undefined); + const { agent } = createTestContext(disposables, { + rootConfig: { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }, + nativeAccount: NATIVE_ACCOUNT, }); + await agent.authenticate('https://api.github.com', 'tok'); + await tick(); + + assert.strictEqual((agent as IAgent).authenticationRequired, undefined); }); test('construction in proxy mode does not emit auth/required', async () => { @@ -2007,6 +2046,7 @@ suite('ClaudeAgent', () => { [IClaudeProxyService, proxy], [ISessionDataService, createNullSessionDataService()], [IClaudeAgentSdkService, new FakeClaudeAgentSdkService()], + [IAgentSdkDownloader, new RecordingAgentSdkDownloader()], [IAgentPluginManager, new FakeAgentPluginManager()], [IAgentHostGitService, createNoopGitService()], [IProductService, FakeProductService], @@ -2081,6 +2121,7 @@ suite('ClaudeAgent', () => { [IClaudeProxyService, proxy], [ISessionDataService, createNullSessionDataService()], [IClaudeAgentSdkService, new FakeClaudeAgentSdkService()], + [IAgentSdkDownloader, new RecordingAgentSdkDownloader()], [IAgentPluginManager, new FakeAgentPluginManager()], [IProductService, FakeProductService], [IAgentHostGitHubEndpointService, createTestGitHubEndpointService()], @@ -2152,6 +2193,7 @@ suite('ClaudeAgent', () => { [IClaudeProxyService, proxy], [ISessionDataService, createNullSessionDataService()], [IClaudeAgentSdkService, new FakeClaudeAgentSdkService()], + [IAgentSdkDownloader, new RecordingAgentSdkDownloader()], [IAgentPluginManager, new FakeAgentPluginManager()], [IProductService, FakeProductService], [IAgentHostGitHubEndpointService, createTestGitHubEndpointService()], @@ -4188,6 +4230,7 @@ suite('ClaudeAgent', () => { [IClaudeProxyService, proxy], [ISessionDataService, sessionData], [IClaudeAgentSdkService, sdk], + [IAgentSdkDownloader, new RecordingAgentSdkDownloader()], [IAgentPluginManager, new FakeAgentPluginManager()], [IAgentHostGitService, createNoopGitService()], [IAgentHostCheckpointService, NULL_CHECKPOINT_SERVICE], @@ -4894,7 +4937,6 @@ suite('ClaudeAgent', () => { }, }; const sdk = new FakeClaudeAgentSdkService(); - sdk.canLoadWithoutDownloadResult = false; sdk.sessionList = [ { sessionId: 'a', summary: 'Session A', lastModified: 1000, createdAt: 900 }, { sessionId: 'b', summary: 'Session B', lastModified: 2000, createdAt: 1900 }, @@ -4908,6 +4950,7 @@ suite('ClaudeAgent', () => { [IClaudeProxyService, new FakeClaudeProxyService()], [ISessionDataService, sessionData], [IClaudeAgentSdkService, sdk], + [IAgentSdkDownloader, new RecordingAgentSdkDownloader()], [IAgentPluginManager, new FakeAgentPluginManager()], [IProductService, FakeProductService], [IAgentHostGitHubEndpointService, createTestGitHubEndpointService()], @@ -4930,7 +4973,7 @@ suite('ClaudeAgent', () => { modifiedA: a?.modifiedTime, modifiedB: b?.modifiedTime, sdkCalls: sdk.listSessionsCallCount, - availabilityRequests: sdk.ensureAvailableForDiscoveryCalls, + availabilityRequests: sdk.ensureAvailableCalls, migrationChats: chatsToMigrate?.map(r => sessionIdOfChat(r.chat)), }, { count: 3, @@ -4940,7 +4983,7 @@ suite('ClaudeAgent', () => { modifiedA: 1000, modifiedB: 2000, sdkCalls: 2, - availabilityRequests: 1, + availabilityRequests: 0, migrationChats: ['a'], }); @@ -5019,6 +5062,7 @@ suite('ClaudeAgent', () => { [IClaudeProxyService, new FakeClaudeProxyService()], [ISessionDataService, sessionData], [IClaudeAgentSdkService, sdk], + [IAgentSdkDownloader, new RecordingAgentSdkDownloader()], [IAgentPluginManager, new FakeAgentPluginManager()], [IProductService, FakeProductService], [IAgentHostGitHubEndpointService, createTestGitHubEndpointService()], @@ -5061,6 +5105,7 @@ suite('ClaudeAgent', () => { [IClaudeProxyService, new FakeClaudeProxyService()], [ISessionDataService, createNullSessionDataService()], [IClaudeAgentSdkService, sdk], + [IAgentSdkDownloader, new RecordingAgentSdkDownloader()], [IAgentPluginManager, new FakeAgentPluginManager()], [IProductService, FakeProductService], [IAgentHostGitHubEndpointService, createTestGitHubEndpointService()], @@ -5110,6 +5155,7 @@ suite('ClaudeAgent', () => { [IClaudeProxyService, new FakeClaudeProxyService()], [ISessionDataService, sessionData], [IClaudeAgentSdkService, sdk], + [IAgentSdkDownloader, new RecordingAgentSdkDownloader()], [IAgentPluginManager, new FakeAgentPluginManager()], [IProductService, FakeProductService], [IAgentHostGitHubEndpointService, createTestGitHubEndpointService()], @@ -5184,7 +5230,7 @@ suite('ClaudeAgent', () => { }); }); - test('restore reads defer while cold discovery requests SDK availability', async () => { + test('neither restore nor cold discovery pulls the SDK down', async () => { // Regression: when a materialized Claude session is restored on // startup (the renderer subscribes to the last-active session), the // host's restore path calls `getChatMetadata` -> `getSessionInfo` @@ -5192,8 +5238,8 @@ suite('ClaudeAgent', () => { // Before the fix that eagerly triggered a cold SDK download (with no // progress interest registered, so no notification) purely from // preselecting/restoring Claude — the download must only start on the - // first user message. Discovery is different: it requests background SDK - // availability so native chats are retried without a new session. + // first user message. Discovery used to be exempt and fetch in the + // background; it no longer is, since the download is the user's call. const sdk = new FakeClaudeAgentSdkService(); sdk.canLoadWithoutDownloadResult = false; sdk.sessionList = [ @@ -5208,6 +5254,7 @@ suite('ClaudeAgent', () => { [IClaudeProxyService, new FakeClaudeProxyService()], [ISessionDataService, createNullSessionDataService()], [IClaudeAgentSdkService, sdk], + [IAgentSdkDownloader, new RecordingAgentSdkDownloader()], [IAgentPluginManager, new FakeAgentPluginManager()], [IProductService, FakeProductService], ); @@ -5226,19 +5273,19 @@ suite('ClaudeAgent', () => { assert.deepStrictEqual({ metadata, messages, - // Restore must never touch the SDK. Discovery alone asks the SDK - // service to begin availability work. + // Nothing reachable from restore or discovery may touch the SDK + // while it is absent, whether to read it or to fetch it. getSessionInfoCalls: sdk.getSessionInfoCalls, getSessionMessagesCalls: sdk.getSessionMessagesCalls, - availabilityRequests: sdk.ensureAvailableForDiscoveryCalls, + availabilityRequests: sdk.ensureAvailableCalls, discoveredChats, }, { metadata: undefined, messages: [], getSessionInfoCalls: [], getSessionMessagesCalls: [], - availabilityRequests: 1, - discoveredChats: [1], + availabilityRequests: 0, + discoveredChats: [], }); }); @@ -5307,7 +5354,7 @@ suite('ClaudeAgent', () => { const services = new ServiceCollection( [ILogService, new RecordingLogService()], - [IAgentSdkDownloader, stubAgentSdkDownloader()], + [IAgentSdkDownloader, new RecordingAgentSdkDownloader(false)], ); const inst = disposables.add(new InstantiationService(services)); const svc = inst.createInstance(TestableClaudeAgentSdkService); @@ -5394,7 +5441,7 @@ suite('ClaudeAgent', () => { const inst = disposables.add(new InstantiationService(new ServiceCollection( [ILogService, new NullLogService()], - [IAgentSdkDownloader, stubAgentSdkDownloader()], + [IAgentSdkDownloader, new RecordingAgentSdkDownloader(false)], ))); const svc = inst.createInstance(TestableClaudeAgentSdkService); @@ -5505,6 +5552,7 @@ suite('ClaudeAgent', () => { [IClaudeProxyService, new RecordingProxyService()], [ISessionDataService, createNullSessionDataService()], [IClaudeAgentSdkService, new FakeClaudeAgentSdkService()], + [IAgentSdkDownloader, new RecordingAgentSdkDownloader()], [IAgentPluginManager, new FakeAgentPluginManager()], [IAgentHostGitService, createNoopGitService()], [IProductService, FakeProductService], @@ -5560,6 +5608,7 @@ suite('ClaudeAgent', () => { [IClaudeProxyService, proxy], [ISessionDataService, sessionData], [IClaudeAgentSdkService, sdk], + [IAgentSdkDownloader, new RecordingAgentSdkDownloader()], [IAgentPluginManager, new FakeAgentPluginManager()], [IAgentHostGitService, createNoopGitService()], [IAgentHostCheckpointService, NULL_CHECKPOINT_SERVICE], @@ -6011,6 +6060,171 @@ suite('ClaudeAgent', () => { // #endregion }); +suite('ClaudeAgent — agent SDK setup channel', () => { + const disposables = ensureNoDisposablesAreLeakedInTestSuite(); + + /** What the workbench would read off root state right now. */ + function readSetup(ctx: ITestContext) { + return readAgentSdkSetupInfos(ctx.stateManager.rootState).find(setup => setup.agent === 'claude'); + } + + /** Addresses a download request at an agent the way `IAgentSdkSetupService` does. */ + function dispatchDownload(ctx: ITestContext, agent = 'claude', request = 'req-1'): void { + ctx.configService.updateRootConfig({ [AGENT_SDK_SETUP_DOWNLOAD_REQUEST_KEY]: { agent, request } }); + } + + /** Waits for the ctor's queued publish (and any refresh it chains) to settle. */ + async function settle(): Promise { + for (let i = 0; i < 20; i++) { + await tick(); + } + } + + test('an SDK already on disk publishes `ready` plus the docs URL the banner links to', async () => { + const ctx = createTestContext(disposables); + await settle(); + + assert.deepStrictEqual(readSetup(ctx), { + agent: 'claude', + download: 'ready', + setupDocsUrl: 'https://docs.claude.com/en/docs/claude-code/setup', + // No in-app sign-in: every Claude credential is established outside the + // app, so the banner can only point at the docs. + signInProviderName: undefined, + }); + }); + + test('a cold cache publishes `notDownloaded`, which is what turns the banner into an offer', async () => { + const ctx = createTestContext(disposables); + ctx.sdk.canLoadWithoutDownloadResult = false; + await ctx.agent.refreshModels(); + await settle(); + + assert.strictEqual(readSetup(ctx)?.download, 'notDownloaded'); + }); + + test('an explicit download fetches the SDK, holds progress interest for the fetch, and ends at `ready`', async () => { + const ctx = createTestContext(disposables); + ctx.sdk.canLoadWithoutDownloadResult = false; + let releaseDownload = () => { }; + ctx.sdk.ensureAvailableGate = new Promise(resolve => { + // Releasing the gate is the moment the SDK lands on disk. + releaseDownload = () => { ctx.sdk.canLoadWithoutDownloadResult = true; resolve(); }; + }); + await ctx.agent.refreshModels(); + await settle(); + + dispatchDownload(ctx); + await settle(); + const inFlight = { + download: readSetup(ctx)?.download, + interests: [...ctx.sdkDownloader.progressInterests], + held: ctx.sdkDownloader.heldProgressInterests, + fetches: ctx.sdk.ensureAvailableCalls, + }; + + releaseDownload(); + await settle(); + + assert.deepStrictEqual({ inFlight, after: readSetup(ctx)?.download, held: ctx.sdkDownloader.heldProgressInterests }, { + inFlight: { download: 'downloading', interests: ['claude'], held: 1, fetches: 1 }, + after: 'ready', + held: 0, + }); + }); + + test('the request key is cleared as it is consumed, so an identical later press still lands', async () => { + const ctx = createTestContext(disposables); + ctx.sdk.canLoadWithoutDownloadResult = false; + await ctx.agent.refreshModels(); + await settle(); + + dispatchDownload(ctx, 'claude', 'press-1'); + await settle(); + const consumed = ctx.configService.getRootConfigValues()[AGENT_SDK_SETUP_DOWNLOAD_REQUEST_KEY]; + + dispatchDownload(ctx, 'claude', 'press-2'); + await settle(); + + assert.deepStrictEqual({ consumed, fetches: ctx.sdk.ensureAvailableCalls }, { consumed: undefined, fetches: 2 }); + }); + + test('a request addressed to another agent is ignored', async () => { + const ctx = createTestContext(disposables); + ctx.sdk.canLoadWithoutDownloadResult = false; + await ctx.agent.refreshModels(); + await settle(); + + dispatchDownload(ctx, 'codex'); + await settle(); + + assert.deepStrictEqual({ + fetches: ctx.sdk.ensureAvailableCalls, + // Left in place for the agent it names, rather than consumed by this one. + key: ctx.configService.getRootConfigValues()[AGENT_SDK_SETUP_DOWNLOAD_REQUEST_KEY], + }, { + fetches: 0, + key: { agent: 'codex', request: 'req-1' }, + }); + }); + + test('a failed download releases the progress interest and stops claiming to be downloading', async () => { + const ctx = createTestContext(disposables); + ctx.sdk.canLoadWithoutDownloadResult = false; + ctx.sdk.ensureAvailableRejection = new Error('CDN unreachable'); + await ctx.agent.refreshModels(); + await settle(); + + dispatchDownload(ctx); + await settle(); + + assert.deepStrictEqual({ + download: readSetup(ctx)?.download, + held: ctx.sdkDownloader.heldProgressInterests, + }, { + download: 'notDownloaded', + held: 0, + }); + }); + + test('chat discovery waits for the SDK rather than fetching it, and runs again once it lands', async () => { + // The catalog of migratable Claude Code chats lives inside the SDK, so + // discovery used to fetch one at startup — hundreds of megabytes for a + // user still being asked whether they want it. + const ctx = createTestContext(disposables); + ctx.sdk.canLoadWithoutDownloadResult = false; + ctx.sdk.sessionList = [{ sessionId: 'from-claude-code', summary: 'An existing chat', lastModified: 1000, createdAt: 900 }]; + // Subscribing is what starts discovery. + const discovered: number[] = []; + disposables.add(ctx.agent.onDidDiscoverChats(chats => discovered.push(chats.length))); + await settle(); + const cold = { + discovered: [...discovered], + // `undefined` is "ask again later", as distinct from "nothing to migrate". + migratable: await ctx.agent.listChatsToMigrate(), + fetches: ctx.sdk.ensureAvailableCalls, + }; + + let landed = () => { }; + ctx.sdk.ensureAvailableGate = new Promise(resolve => { + landed = () => { ctx.sdk.canLoadWithoutDownloadResult = true; resolve(); }; + }); + dispatchDownload(ctx); + await settle(); + const inFlight = [...discovered]; + + landed(); + await settle(); + + assert.deepStrictEqual({ cold, inFlight, after: discovered, migratable: await ctx.agent.listChatsToMigrate() }, { + cold: { discovered: [], migratable: undefined, fetches: 0 }, + inFlight: [], + after: [1], + migratable: [], + }); + }); +}); + suite('ClaudeAgent — per-session provider', () => { const disposables = ensureNoDisposablesAreLeakedInTestSuite(); @@ -6202,56 +6416,59 @@ suite('ClaudeAgent — per-session provider', () => { // the host-global default flips underneath it. Otherwise signing into // Copilot mid-conversation would silently drag a running native // (BYO-Anthropic) session onto the proxy on its next rebind. - await withNativeSetup(async userHome => { - const ctx = createTestContext(disposables, { - rootConfig: { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }, - userHome, - }); + const ctx = createTestContext(disposables, { + rootConfig: { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }, + nativeAccount: NATIVE_ACCOUNT, + }); + // The host default only becomes native once the SDK has been *asked* about + // the account — that answer is what `_defaultTransportMode` reads. Await a + // full refresh, or this materializes on the proxy default and throws + // `AHP_AUTH_REQUIRED` while signed out. + await ctx.agent.refreshModels(); + + // Materialize a native session while signed out: turn-1 starts the + // subprocess (system_init) then crashes mid-stream, leaving it needing a + // warm rebind on the next send. + const created = await createSession(ctx.agent, { workingDirectories: [URI.file('/workspace')], model: { id: 'claude-sonnet-4-5-20250929' } }); + const sid = created.sdkSessionId; + ctx.sdk.nextQueryMessages = [makeSystemInitMessage(sid)]; + ctx.sdk.queryAdvance = async (i: number) => { if (i === 1) { throw new Error('subprocess crashed'); } }; + await assert.rejects( + ctx.agent.chats.sendMessage(defaultChatUri(created.session), 'hi', undefined, undefined, 'turn-1', undefined, undefined, chatContext(defaultChatUri(created.session))), + (err: Error) => err.message.includes('subprocess crashed'), + ); + ctx.sdk.queryAdvance = undefined; - // Materialize a native session while signed out: turn-1 starts the - // subprocess (system_init) then crashes mid-stream, leaving it needing a - // warm rebind on the next send. - const created = await createSession(ctx.agent, { workingDirectories: [URI.file('/workspace')], model: { id: 'claude-sonnet-4-5-20250929' } }); - const sid = created.sdkSessionId; - ctx.sdk.nextQueryMessages = [makeSystemInitMessage(sid)]; - ctx.sdk.queryAdvance = async (i: number) => { if (i === 1) { throw new Error('subprocess crashed'); } }; - await assert.rejects( - ctx.agent.chats.sendMessage(defaultChatUri(created.session), 'hi', undefined, undefined, 'turn-1', undefined, undefined, chatContext(defaultChatUri(created.session))), - (err: Error) => err.message.includes('subprocess crashed'), - ); - ctx.sdk.queryAdvance = undefined; - - // Sign into Copilot: this flips the host default native→proxy and - // acquires a proxy handle. - await ctx.agent.authenticate('https://api.github.com', 'tok'); - await tick(); + // Sign into Copilot: this flips the host default native→proxy and + // acquires a proxy handle. + await ctx.agent.authenticate('https://api.github.com', 'tok'); + await tick(); - // Positive control that the flip is live: a brand-new session now - // materializes on the proxy (carrying the per-session bearer token). - const fresh = await createSession(ctx.agent, { workingDirectories: [URI.file('/fresh')], model: { id: 'claude-opus-4.6' } }); - const freshSid = fresh.sdkSessionId; - ctx.sdk.nextQueryMessages = [makeSystemInitMessage(freshSid), makeResultSuccess(freshSid)]; - await ctx.agent.chats.sendMessage(defaultChatUri(fresh.session), 'hi', undefined, undefined, 'fresh-1', undefined, undefined, chatContext(defaultChatUri(fresh.session))); + // Positive control that the flip is live: a brand-new session now + // materializes on the proxy (carrying the per-session bearer token). + const fresh = await createSession(ctx.agent, { workingDirectories: [URI.file('/fresh')], model: { id: 'claude-opus-4.6' } }); + const freshSid = fresh.sdkSessionId; + ctx.sdk.nextQueryMessages = [makeSystemInitMessage(freshSid), makeResultSuccess(freshSid)]; + await ctx.agent.chats.sendMessage(defaultChatUri(fresh.session), 'hi', undefined, undefined, 'fresh-1', undefined, undefined, chatContext(defaultChatUri(fresh.session))); - // Recover the ORIGINAL session: the next send warm-rebuilds it (resume), - // and that rebuild must stay native despite the flipped host default. - ctx.sdk.nextQueryMessages = [makeSystemInitMessage(sid), makeResultSuccess(sid)]; - await ctx.agent.chats.sendMessage(defaultChatUri(created.session), 'recover', undefined, undefined, 'turn-2', undefined, undefined, chatContext(defaultChatUri(created.session))); + // Recover the ORIGINAL session: the next send warm-rebuilds it (resume), + // and that rebuild must stay native despite the flipped host default. + ctx.sdk.nextQueryMessages = [makeSystemInitMessage(sid), makeResultSuccess(sid)]; + await ctx.agent.chats.sendMessage(defaultChatUri(created.session), 'recover', undefined, undefined, 'turn-2', undefined, undefined, chatContext(defaultChatUri(created.session))); - assert.deepStrictEqual({ - originalMaterializeNative: proxyAuthTokenOf(ctx.sdk.capturedStartupOptions[0]) === undefined, - freshSessionProxy: proxyAuthTokenOf(ctx.sdk.capturedStartupOptions[1]) !== undefined, - rebuild: { - resume: ctx.sdk.capturedStartupOptions[2]?.resume, - stayedNative: proxyAuthTokenOf(ctx.sdk.capturedStartupOptions[2]) === undefined, - }, - totalStartups: ctx.sdk.startupCallCount, - }, { - originalMaterializeNative: true, - freshSessionProxy: true, - rebuild: { resume: sid, stayedNative: true }, - totalStartups: 3, - }); + assert.deepStrictEqual({ + originalMaterializeNative: proxyAuthTokenOf(ctx.sdk.capturedStartupOptions[0]) === undefined, + freshSessionProxy: proxyAuthTokenOf(ctx.sdk.capturedStartupOptions[1]) !== undefined, + rebuild: { + resume: ctx.sdk.capturedStartupOptions[2]?.resume, + stayedNative: proxyAuthTokenOf(ctx.sdk.capturedStartupOptions[2]) === undefined, + }, + totalStartups: ctx.sdk.startupCallCount, + }, { + originalMaterializeNative: true, + freshSessionProxy: true, + rebuild: { resume: sid, stayedNative: true }, + totalStartups: 3, }); }); @@ -6283,63 +6500,73 @@ suite('ClaudeAgent — per-session provider', () => { }); }); - test('the Copilot resource is optional only when the opt-in AND a BYO-Anthropic credential are both present', async () => { - // Full 2x2 so no single input can carry the result on its own: in - // particular `optInOnNoCredential` is the regression this guards — the - // requirement must survive the opt-in being on when the user has no - // Anthropic credential to run on. - const copilotRequired = (agent: ClaudeAgent) => - agent.getProtectedResources().find(r => r.resource === 'https://api.github.com')?.required; - const optIn = { rootConfig: { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true } }; - await withNativeSetup(async userHome => { - assert.deepStrictEqual({ - optInOnNoCredential: copilotRequired(createTestContext(disposables, { ...optIn }).agent), - optInOnWithCredential: copilotRequired(createTestContext(disposables, { ...optIn, userHome }).agent), - optInOffWithCredential: copilotRequired(createTestContext(disposables, { userHome }).agent), - optInOffNoCredential: copilotRequired(createTestContext(disposables).agent), - }, { - optInOnNoCredential: true, - optInOnWithCredential: false, - optInOffWithCredential: true, - optInOffNoCredential: true, + test('the Copilot resource is unconditionally optional, whatever the opt-in or the SDK account report says', async () => { + // The load-bearing assertion of the whole feature. `required: false` is what + // stops `resolveAgentAuthRequirement` answering `GitHub` for this session + // type; when *every* type answers `GitHub`, `resolveSignedOutWindowGate` + // puts a non-dismissible sign-in wall over the entire Agents window. + // + // The full 2x2 is asserted because the behavior it replaces was a 2x2 with + // three `true`s in it: neither the opt-in nor the account report may bring + // the requirement back. Even with no Claude account the type reads as + // `Unusable` rather than `GitHub`, which is what opens the window. The flag + // is absent by design — it gates this one level up, in + // `resolveSignedOutWindowGate`. + const advertisedRequirement = async (inputs: { optIn: boolean; account: boolean }) => { + const { agent } = createTestContext(disposables, { + ...(inputs.optIn ? { rootConfig: { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true } } : {}), + ...(inputs.account ? { nativeAccount: NATIVE_ACCOUNT } : {}), }); + // Let the account probe land. The old answer keyed on exactly this fact, + // so without the wait both halves of the matrix would be asserting the + // same pre-probe state and the test would pass vacuously. + await agent.refreshModels(); + return agent.getProtectedResources().find(r => r.resource === 'https://api.github.com')?.required; + }; + + assert.deepStrictEqual({ + optInOnWithAccount: await advertisedRequirement({ optIn: true, account: true }), + optInOffWithAccount: await advertisedRequirement({ optIn: false, account: true }), + optInOnNoAccount: await advertisedRequirement({ optIn: true, account: false }), + optInOffNoAccount: await advertisedRequirement({ optIn: false, account: false }), + }, { + optInOnWithAccount: false, + optInOffWithAccount: false, + optInOnNoAccount: false, + optInOffNoAccount: false, }); }); test('the Copilot resource is advertised, never dropped, so the silent token probe survives', async () => { // `authenticateProtectedResources` matches on `resource` and ignores // `required`, so dropping it would break sign-in forwarding. - await withNativeSetup(async userHome => { - const optional = createTestContext(disposables, { - rootConfig: { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }, - userHome, - }).agent.getProtectedResources(); - assert.deepStrictEqual(optional.map(r => ({ resource: r.resource, required: r.required })), [ - { resource: 'https://api.github.com', required: false }, - { resource: 'https://api.github.com/repos', required: false }, - ]); - }); + const optional = createTestContext(disposables, { + rootConfig: { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }, + nativeAccount: NATIVE_ACCOUNT, + }).agent.getProtectedResources(); + assert.deepStrictEqual(optional.map(r => ({ resource: r.resource, required: r.required })), [ + { resource: 'https://api.github.com', required: false }, + { resource: 'https://api.github.com/repos', required: false }, + ]); }); test('merged catalog lists both providers, each id provider-qualified', async () => { - await withNativeSetup(async userHome => { - const { agent, api, sdk } = createTestContext(disposables, { userHome }); - api.models = async () => [CLAUDE_OPUS]; - sdk.supportedModelsResult = [ - { value: 'claude-sonnet-4-5-20250929', displayName: 'Claude Sonnet 4.5', description: '' }, - ]; - await agent.authenticate('https://api.github.com', 'tok'); - await agent.refreshModels(); - await tick(); + const { agent, api, sdk } = createTestContext(disposables, { nativeAccount: NATIVE_ACCOUNT }); + api.models = async () => [CLAUDE_OPUS]; + sdk.supportedModelsResult = [ + { value: 'claude-sonnet-4-5-20250929', displayName: 'Claude Sonnet 4.5', description: '' }, + ]; + await agent.authenticate('https://api.github.com', 'tok'); + await agent.refreshModels(); + await tick(); - // Proxy first (preserves `models[0]`-is-default), then native; each id is - // rewritten to its provider-qualified form so the picked row carries its - // transport. - assert.deepStrictEqual(agent.models.get().map(m => ({ id: m.id, name: m.name })), [ - { id: toClaudeModelSelectionId(CLAUDE_PROVIDER_COPILOT, 'claude-opus-4.6'), name: 'Claude Opus 4.6' }, - { id: toClaudeModelSelectionId(CLAUDE_PROVIDER_ANTHROPIC, 'claude-sonnet-4-5-20250929'), name: 'Claude Sonnet 4.5' }, - ]); - }); + // Proxy first (preserves `models[0]`-is-default), then native; each id is + // rewritten to its provider-qualified form so the picked row carries its + // transport. + assert.deepStrictEqual(agent.models.get().map(m => ({ id: m.id, name: m.name })), [ + { id: toClaudeModelSelectionId(CLAUDE_PROVIDER_COPILOT, 'claude-opus-4.6'), name: 'Claude Opus 4.6' }, + { id: toClaudeModelSelectionId(CLAUDE_PROVIDER_ANTHROPIC, 'claude-sonnet-4-5-20250929'), name: 'Claude Sonnet 4.5' }, + ]); }); test('per-session transport gates on the picked model, not a global mode', async () => { @@ -6402,23 +6629,21 @@ suite('ClaudeAgent — per-session provider', () => { // unconditionally — a signed-out window with a native setup has no GitHub // token to trigger a proxy refresh, so without this it would never populate // its picker and dead-end. No `authenticate`, no explicit `refreshModels`. - await withNativeSetup(async userHome => { - const { agent, sdk } = createTestContext(disposables, { userHome }); - sdk.supportedModelsResult = [ - { value: 'claude-sonnet-4-5-20250929', displayName: 'Claude Sonnet 4.5', description: '' }, - ]; - // The constructor kicks off the initial merged enumeration; wait for it. - for (let i = 0; i < 100 && sdk.supportedModelsCallCount === 0; i++) { - await tick(); - } + const { agent, sdk } = createTestContext(disposables, { nativeAccount: NATIVE_ACCOUNT }); + sdk.supportedModelsResult = [ + { value: 'claude-sonnet-4-5-20250929', displayName: 'Claude Sonnet 4.5', description: '' }, + ]; + // The constructor kicks off the initial merged enumeration; wait for it. + for (let i = 0; i < 100 && sdk.supportedModelsCallCount === 0; i++) { await tick(); + } + await tick(); - // Signed out → the proxy half contributes nothing; only the native - // models appear, provider-qualified. - assert.deepStrictEqual(agent.models.get().map(m => m.id), [ - toClaudeModelSelectionId(CLAUDE_PROVIDER_ANTHROPIC, 'claude-sonnet-4-5-20250929'), - ]); - }); + // Signed out → the proxy half contributes nothing; only the native + // models appear, provider-qualified. + assert.deepStrictEqual(agent.models.get().map(m => m.id), [ + toClaudeModelSelectionId(CLAUDE_PROVIDER_ANTHROPIC, 'claude-sonnet-4-5-20250929'), + ]); }); test('a failing proxy start does not fail sign-in', async () => { @@ -6520,6 +6745,7 @@ suite('ClaudeAgentSession (Phase 7 §3.2)', () => { [ICopilotApiService, new FakeCopilotApiService()], [IAgentHostAuthenticationService, disposables.add(new FakeAgentHostAuthenticationService())], [IAgentHostGitHubEndpointService, createTestGitHubEndpointService()], + [IAgentSdkDownloader, new RecordingAgentSdkDownloader()], [IAgentPluginManager, new FakeAgentPluginManager()], [ISessionDataService, sessionData], ); @@ -8123,6 +8349,7 @@ suite('ClaudeAgent — Phase 11 customizations', () => { disposables.add(fileService.registerProvider(Schemas.file, disposables.add(new InMemoryFileSystemProvider()))); const otelService = new RecordingOTelService(); + const sdkDownloader = new RecordingAgentSdkDownloader(); const services = new ServiceCollection( [IFileService, fileService], [INativeEnvironmentService, { userHome: URI.file('/mock-home') } as INativeEnvironmentService], @@ -8131,6 +8358,7 @@ suite('ClaudeAgent — Phase 11 customizations', () => { [IClaudeProxyService, proxy], [ISessionDataService, sessionData], [IClaudeAgentSdkService, sdk], + [IAgentSdkDownloader, sdkDownloader], [IAgentPluginManager, pluginManager], [IAgentHostGitService, createNoopGitService()], [IAgentHostCheckpointService, NULL_CHECKPOINT_SERVICE], @@ -8176,7 +8404,7 @@ suite('ClaudeAgent — Phase 11 customizations', () => { ?? (chat.scheme === 'ahp-chat' ? URI.parse(parseRequiredSessionUriFromChatUri(chat.toString())) : chat); return sendMessage(chat, prompt, workingDirectoriesOrDirectory, attachments, turnId, senderClientId, clientType, { ...createAgentChatContext(stateManager, session, chat), ...explicit }); }; - return { agent, proxy, api, sdk, sessionData, stateManager, configService, otelService, instantiationService, fileService }; + return { agent, proxy, api, sdk, sessionData, stateManager, configService, otelService, instantiationService, fileService, sdkDownloader }; } function publishReducerCustomizations(stateManager: AgentHostStateManager, session: URI, customizations: readonly Customization[]): void { diff --git a/src/vs/platform/agentHost/test/node/claudeSubagentResolver.test.ts b/src/vs/platform/agentHost/test/node/claudeSubagentResolver.test.ts index 922ca21121bc4..6f63f4bebd1ae 100644 --- a/src/vs/platform/agentHost/test/node/claudeSubagentResolver.test.ts +++ b/src/vs/platform/agentHost/test/node/claudeSubagentResolver.test.ts @@ -45,7 +45,7 @@ class FakeSdkService implements IClaudeAgentSdkService { async listSessions(): Promise { return []; } async canLoadWithoutDownload(): Promise { return true; } - async ensureAvailableForDiscovery(): Promise { } + async ensureAvailable(): Promise { } async getSessionInfo(_id: string): Promise { return undefined; } async startup(_p: { options: Options; initializeTimeoutMs?: number }): Promise { throw new Error('not used'); } async query(_params: { prompt: string | AsyncIterable; options?: Options }): Promise { throw new Error('not used'); } diff --git a/src/vs/platform/agentHost/test/node/claudeTransportMode.test.ts b/src/vs/platform/agentHost/test/node/claudeTransportMode.test.ts index 537fd852b08ec..80bf009c10226 100644 --- a/src/vs/platform/agentHost/test/node/claudeTransportMode.test.ts +++ b/src/vs/platform/agentHost/test/node/claudeTransportMode.test.ts @@ -3,12 +3,10 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import type { AccountInfo } from '@anthropic-ai/claude-agent-sdk'; import assert from 'assert'; -import * as fs from 'fs'; -import * as os from 'os'; -import { join } from '../../../../base/common/path.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; -import { detectExistingClaudeSetup, resolveClaudeTransportMode } from '../../node/claude/claudeTransportMode.js'; +import { isClaudeAccountSetUp, resolveClaudeTransportMode } from '../../node/claude/claudeTransportMode.js'; suite('claudeTransportMode', () => { @@ -39,97 +37,39 @@ suite('claudeTransportMode', () => { }); }); - suite('detectExistingClaudeSetup', () => { - // The credential env is injected explicitly (never `process.env`), so the - // ambient machine's real credentials can't leak into the assertions and no - // global is mutated. The file source is exercised through a real temp home. - let homeDir: string; - - setup(async () => { - homeDir = await fs.promises.mkdtemp(join(os.tmpdir(), 'claude-setup-detect-')); - }); - - teardown(async () => { - await fs.promises.rm(homeDir, { recursive: true, force: true }); - }); - - function writeSettings(contents: string): void { - const dir = join(homeDir, '.claude'); - fs.mkdirSync(dir, { recursive: true }); - fs.writeFileSync(join(dir, 'settings.json'), contents, 'utf8'); - } - - test('detects each env-var credential (and ignores a blank value)', () => { - assert.deepStrictEqual({ - none: detectExistingClaudeSetup(homeDir, {}), - apiKey: detectExistingClaudeSetup(homeDir, { ANTHROPIC_API_KEY: 'sk-ant-api-x' }), - authToken: detectExistingClaudeSetup(homeDir, { ANTHROPIC_AUTH_TOKEN: 'sk-ant-auth-x' }), - baseUrl: detectExistingClaudeSetup(homeDir, { ANTHROPIC_BASE_URL: 'https://gateway.example/v1' }), - oauthToken: detectExistingClaudeSetup(homeDir, { CLAUDE_CODE_OAUTH_TOKEN: 'sk-ant-oat-x' }), - emptyValue: detectExistingClaudeSetup(homeDir, { ANTHROPIC_API_KEY: '' }), - whitespaceValue: detectExistingClaudeSetup(homeDir, { ANTHROPIC_API_KEY: ' ' }), - }, { none: false, apiKey: true, authToken: true, baseUrl: true, oauthToken: true, emptyValue: false, whitespaceValue: false }); - }); - - test('detects a credential in the settings.json env block (empty env injected)', () => { - const results: Record = {}; - writeSettings(JSON.stringify({ env: { ANTHROPIC_API_KEY: 'sk-ant-api-x' } })); - results.apiKey = detectExistingClaudeSetup(homeDir, {}); - writeSettings(JSON.stringify({ env: { ANTHROPIC_AUTH_TOKEN: 'sk-ant-auth-x' } })); - results.authToken = detectExistingClaudeSetup(homeDir, {}); - writeSettings(JSON.stringify({ env: { ANTHROPIC_BASE_URL: 'https://gateway.example/v1' } })); - results.baseUrl = detectExistingClaudeSetup(homeDir, {}); - writeSettings(JSON.stringify({ env: { CLAUDE_CODE_OAUTH_TOKEN: 'sk-ant-oat-x' } })); - results.oauthToken = detectExistingClaudeSetup(homeDir, {}); - writeSettings(JSON.stringify({ env: { ANTHROPIC_API_KEY: '' } })); - results.emptyValue = detectExistingClaudeSetup(homeDir, {}); - writeSettings(JSON.stringify({ env: { ANTHROPIC_API_KEY: ' ' } })); - results.whitespaceValue = detectExistingClaudeSetup(homeDir, {}); - writeSettings(JSON.stringify({ model: 'claude-sonnet-4-5' })); - results.noEnvBlock = detectExistingClaudeSetup(homeDir, {}); - writeSettings('not json'); - results.malformed = detectExistingClaudeSetup(homeDir, {}); - // The tolerant parser salvages a partial object from a truncated file - // rather than failing, so the credential it recovers must not count — - // the CLI reading the same file would not get one. - writeSettings('{ "env": { "ANTHROPIC_API_KEY": "sk-ant-api-x"'); - results.truncated = detectExistingClaudeSetup(homeDir, {}); - // Read with the same tolerant parser VS Code uses for every other - // hand-edited config, so comments and a trailing comma still resolve. - writeSettings('{\n\t// my key\n\t"env": { "ANTHROPIC_API_KEY": "sk-ant-api-x", },\n}'); - results.jsonc = detectExistingClaudeSetup(homeDir, {}); - - assert.deepStrictEqual(results, { apiKey: true, authToken: true, baseUrl: true, oauthToken: true, emptyValue: false, whitespaceValue: false, noEnvBlock: false, malformed: false, truncated: false, jsonc: true }); - }); - - test('detects the top-level apiKeyHelper alongside unrecognized settings', () => { - const results: Record = {}; - writeSettings(JSON.stringify({ apiKeyHelper: '/bin/mint-key.sh' })); - results.helper = detectExistingClaudeSetup(homeDir, {}); - // A real settings file carries keys the validator doesn't declare; they - // must be ignored rather than fail validation for the whole file. - writeSettings(JSON.stringify({ apiKeyHelper: '/bin/mint-key.sh', model: 'claude-sonnet-4-5', permissions: { allow: [] } })); - results.helperAmongOthers = detectExistingClaudeSetup(homeDir, {}); - writeSettings(JSON.stringify({ apiKeyHelper: '' })); - results.emptyValue = detectExistingClaudeSetup(homeDir, {}); - writeSettings(JSON.stringify({ apiKeyHelper: 42 })); - results.wrongType = detectExistingClaudeSetup(homeDir, {}); - - assert.deepStrictEqual(results, { helper: true, helperAmongOthers: true, emptyValue: false, wrongType: false }); - }); - - test('a malformed source never masks a usable one', () => { - const results: Record = {}; - writeSettings(JSON.stringify({ apiKeyHelper: '/bin/mint-key.sh', env: { ANTHROPIC_API_KEY: 42 } })); - results.helperWithMistypedEnvKey = detectExistingClaudeSetup(homeDir, {}); - writeSettings(JSON.stringify({ apiKeyHelper: 42, env: { ANTHROPIC_API_KEY: 'sk-ant-api-x' } })); - results.apiKeyWithMistypedHelper = detectExistingClaudeSetup(homeDir, {}); - writeSettings(JSON.stringify({ env: { ANTHROPIC_API_KEY: 'sk-ant-api-x', ANTHROPIC_BASE_URL: 8080 } })); - results.apiKeyWithMistypedSibling = detectExistingClaudeSetup(homeDir, {}); - writeSettings(JSON.stringify({ apiKeyHelper: '/bin/mint-key.sh', env: 'not an object' })); - results.helperWithNonObjectEnv = detectExistingClaudeSetup(homeDir, {}); - - assert.deepStrictEqual(results, { helperWithMistypedEnvKey: true, apiKeyWithMistypedHelper: true, apiKeyWithMistypedSibling: true, helperWithNonObjectEnv: true }); + suite('isClaudeAccountSetUp', () => { + // Every row is a shape observed from a real `accountInfo()` probe — the + // rule exists to match what the SDK actually reports. + const cases: readonly (readonly [name: string, account: AccountInfo | undefined, expected: boolean])[] = [ + // The SDK could not be asked at all (not downloaded, or the query + // failed). Publishing models we cannot back is the bug being fixed. + ['no report at all', undefined, false], + // Measured with an empty `HOME` and a stripped environment. The + // real-looking `apiProvider` here is exactly why it is not a presence + // signal — this user has nothing configured. + ['nothing configured', { tokenSource: 'none', apiProvider: 'firstParty' }, false], + // Same verdict without the provider field, so absence is not read as + // third-party. + ['nothing configured, no provider field', { tokenSource: 'none' }, false], + ['empty report', {}, false], + // `claude login` / `CLAUDE_CODE_OAUTH_TOKEN` — the keychain case no + // filesystem check could ever see. + ['oauth token', { tokenSource: 'ANTHROPIC_AUTH_TOKEN', apiProvider: 'firstParty' }, true], + // An API key reports through `apiKeySource` and leaves `tokenSource` + // at its `'none'` sentinel, so testing `tokenSource` alone misses it. + ['api key', { tokenSource: 'none', apiKeySource: 'ANTHROPIC_API_KEY', apiProvider: 'firstParty' }, true], + // The rows a later "simplification" silently breaks: for third-party + // backends the SDK documents the credential fields as absent, because + // auth is external (AWS creds, gcloud ADC). + ['third-party backend (bedrock)', { apiProvider: 'bedrock' }, true], + ['third-party backend (vertex)', { apiProvider: 'vertex' }, true], + ['enterprise gateway', { apiProvider: 'gateway' }, true], + ]; + + test('maps observed SDK account reports onto one set-up answer', () => { + assert.deepStrictEqual( + Object.fromEntries(cases.map(([name, account]) => [name, isClaudeAccountSetUp(account)])), + Object.fromEntries(cases.map(([name, , expected]) => [name, expected]))); }); }); }); diff --git a/src/vs/platform/agentHost/test/node/codex/codexAgent.test.ts b/src/vs/platform/agentHost/test/node/codex/codexAgent.test.ts index 893542b48e4fc..ad65982286323 100644 --- a/src/vs/platform/agentHost/test/node/codex/codexAgent.test.ts +++ b/src/vs/platform/agentHost/test/node/codex/codexAgent.test.ts @@ -379,23 +379,27 @@ suite('CodexAgent', () => { }); }); - test('cold native discovery waits for the SDK and emits through one deterministic path', async () => { - const sdkReady = new DeferredPromise(); + test('cold native discovery waits for the SDK rather than fetching it, and runs again once it lands', async () => { const onDidDiscoverChats = new Emitter(); const discoveredChats: number[] = []; const listener = onDidDiscoverChats.event(chats => discoveredChats.push(chats.length)); - const startDiscovery = (CodexAgent.prototype as unknown as { - _startCodexChatDiscovery(this: { - _codexChatDiscovery: Promise | undefined; - _resolveSdkRoot(): Promise; - _emitCodexChats(): Promise; - _logService: { warn(message: string): void }; - }): Promise; - })._startCodexChatDiscovery; - const harness = { - _logService: { warn: () => { } }, - _codexChatDiscovery: undefined as Promise | undefined, - _resolveSdkRoot: () => sdkReady.p, + type DiscoveryHarness = { + _codexChatDiscovery: Promise | undefined; + _isSdkResolvableWithoutDownload(): Promise; + _emitCodexChats(): Promise; + _startCodexChatDiscovery(): Promise; + _logService: { warn(message: string): void; info(message: string): void }; + }; + const discovery = CodexAgent.prototype as unknown as { + _startCodexChatDiscovery(this: DiscoveryHarness): Promise; + _restartChatDiscovery(this: DiscoveryHarness): void; + }; + let sdkIsLocal = false; + const harness: DiscoveryHarness = { + _logService: { warn: () => { }, info: () => { } }, + _codexChatDiscovery: undefined, + _isSdkResolvableWithoutDownload: async () => sdkIsLocal, + _startCodexChatDiscovery: () => discovery._startCodexChatDiscovery.call(harness), _emitCodexChats: async () => { onDidDiscoverChats.fire([{ chat: URI.parse('agenthost-chat://codex/session/default'), @@ -407,13 +411,15 @@ suite('CodexAgent', () => { }, }; - const discovery = startDiscovery.call(harness); - assert.deepStrictEqual(discoveredChats, []); + await discovery._startCodexChatDiscovery.call(harness); + const cold = [...discoveredChats]; - sdkReady.complete('/sdk-root'); - await discovery; + // What the explicit download does on its way out. + sdkIsLocal = true; + discovery._restartChatDiscovery.call(harness); + await harness._codexChatDiscovery; - assert.deepStrictEqual(discoveredChats, [1]); + assert.deepStrictEqual({ cold, after: discoveredChats }, { cold: [], after: [1] }); listener.dispose(); onDidDiscoverChats.dispose(); }); @@ -429,36 +435,31 @@ suite('CodexAgent', () => { ]; const listChatsToMigrate = (CodexAgent.prototype as unknown as { listChatsToMigrate(this: { - _resolveSdkRoot(): Promise; + _isSdkResolvableWithoutDownload(): Promise; _listCodexChats(): Promise; _isKnownCodexChat(chat: (typeof chats)[number]): Promise; - _logService: NullLogService; + _logService: { info(message: string): void }; }): Promise; }).listChatsToMigrate; - - const result = await listChatsToMigrate.call({ - _resolveSdkRoot: async () => '/sdk-root', + // Deferred while the SDK is absent: the catalog it reads lives inside one, + // and fetching it is the user's call. + let sdkIsLocal = false; + const harness = { + _logService: { info: () => { } }, + _isSdkResolvableWithoutDownload: async () => sdkIsLocal, _listCodexChats: async () => chats, - _isKnownCodexChat: async chat => { + _isKnownCodexChat: async (chat: (typeof chats)[number]) => { const id = AgentSession.id(URI.parse(parseRequiredSessionUriFromChatUri(chat.chat))); return id !== 'unknown-external'; }, - _logService: new NullLogService(), - }); + }; - assert.deepStrictEqual(result, chats.slice(0, 2)); - assert.deepStrictEqual(await listChatsToMigrate.call({ - _resolveSdkRoot: async () => '/sdk-root', - _listCodexChats: async () => [], - _isKnownCodexChat: async () => false, - _logService: new NullLogService(), - }), []); - assert.deepStrictEqual(await listChatsToMigrate.call({ - _resolveSdkRoot: async () => { throw new Error('SDK unavailable'); }, - _listCodexChats: async () => [], - _isKnownCodexChat: async () => false, - _logService: new NullLogService(), - }), undefined); + const cold = await listChatsToMigrate.call(harness); + sdkIsLocal = true; + const result = await listChatsToMigrate.call(harness); + const empty = await listChatsToMigrate.call({ ...harness, _listCodexChats: async () => [], _isKnownCodexChat: async () => false }); + + assert.deepStrictEqual({ cold, result, empty }, { cold: undefined, result: chats.slice(0, 2), empty: [] }); }); test('native discovery emits only unknown Codex chats as external', async () => { diff --git a/src/vs/platform/agentHost/test/node/codex/codexLocalAuth.test.ts b/src/vs/platform/agentHost/test/node/codex/codexLocalAuth.test.ts deleted file mode 100644 index d970e1e69e25c..0000000000000 --- a/src/vs/platform/agentHost/test/node/codex/codexLocalAuth.test.ts +++ /dev/null @@ -1,60 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import assert from 'assert'; -import { promises as fs } from 'fs'; -import os from 'os'; -import { join } from '../../../../../base/common/path.js'; -import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; -import { detectExistingCodexChatGPTSetup } from '../../../node/codex/codexLocalAuth.js'; - -suite('Codex local auth detection', () => { - - ensureNoDisposablesAreLeakedInTestSuite(); - - async function withCodexHome(run: (codexHome: string, env: NodeJS.ProcessEnv) => Promise): Promise { - const codexHome = await fs.mkdtemp(join(os.tmpdir(), 'vscode-codex-auth-test-')); - try { - await run(codexHome, { CODEX_HOME: codexHome }); - } finally { - await fs.rm(codexHome, { recursive: true, force: true }); - } - } - - test('recognizes persisted ChatGPT token modes', async () => withCodexHome(async (codexHome, env) => { - for (const auth of [ - { auth_mode: 'chatgpt', tokens: { access_token: 'access', refresh_token: 'refresh' } }, - { auth_mode: 'chatgptAuthTokens', tokens: { access_token: 'access' } }, - { auth_mode: 'personalAccessToken', personal_access_token: 'pat' }, - { tokens: { access_token: 'legacy-access' } }, - ]) { - await fs.writeFile(join(codexHome, 'auth.json'), JSON.stringify(auth)); - assert.strictEqual(detectExistingCodexChatGPTSetup('/unused', env), true); - } - })); - - test('rejects non-human and malformed auth states', async () => withCodexHome(async (codexHome, env) => { - for (const auth of [ - { auth_mode: 'apiKey', OPENAI_API_KEY: 'sk-test' }, - { auth_mode: 'bedrockApiKey', bedrock_api_key: { secret: 'secret' } }, - { auth_mode: 'agentIdentity', agent_identity: { token: 'token' } }, - { auth_mode: 'chatgpt', tokens: { access_token: '', refresh_token: '' } }, - { tokens: null }, - ]) { - await fs.writeFile(join(codexHome, 'auth.json'), JSON.stringify(auth)); - assert.strictEqual(detectExistingCodexChatGPTSetup('/unused', env), false); - } - await fs.writeFile(join(codexHome, 'auth.json'), '{'); - assert.strictEqual(detectExistingCodexChatGPTSetup('/unused', env), false); - })); - - test('honors an explicit Codex home override', async () => withCodexHome(async (codexHome, env) => { - await fs.writeFile(join(codexHome, 'auth.json'), JSON.stringify({ - auth_mode: 'personalAccessToken', - personal_access_token: 'pat', - })); - assert.strictEqual(detectExistingCodexChatGPTSetup('/unused', {}, codexHome), true); - })); -}); diff --git a/src/vs/platform/agentHost/test/node/codex/codexModelRefresh.test.ts b/src/vs/platform/agentHost/test/node/codex/codexModelRefresh.test.ts index 52ee076207d0d..e0aee4c565d58 100644 --- a/src/vs/platform/agentHost/test/node/codex/codexModelRefresh.test.ts +++ b/src/vs/platform/agentHost/test/node/codex/codexModelRefresh.test.ts @@ -5,10 +5,7 @@ import type { CCAModel } from '@vscode/copilot-api'; import assert from 'assert'; -import * as fs from 'fs'; -import * as os from 'os'; import { Event } from '../../../../../base/common/event.js'; -import { join } from '../../../../../base/common/path.js'; import type { DisposableStore } from '../../../../../base/common/lifecycle.js'; import { URI } from '../../../../../base/common/uri.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; @@ -22,7 +19,9 @@ import { IAgentHostCustomizationEnablementService } from '../../../node/agentHos import { AgentHostStateManager } from '../../../node/agentHostStateManager.js'; import { IAgentHostSessionTitleSignal } from '../../../node/agentHostSessionTitleSignal.js'; import { IAgentSdkDownloader } from '../../../node/agentSdkDownloader.js'; +import { RecordingAgentSdkDownloader } from '../testAgentSdkDownloader.js'; import { IAgentHostCheckpointService, NULL_CHECKPOINT_SERVICE } from '../../../common/agentHostCheckpointService.js'; +import { AGENT_SDK_SETUP_DOWNLOAD_REQUEST_KEY, readAgentSdkSetupInfos } from '../../../common/agentSdkSetup.js'; import { CodexAgent, toCodexModelSelectionId } from '../../../node/codex/codexAgent.js'; import { ICodexProxyService } from '../../../node/codex/codexProxyService.js'; import { ICopilotApiService } from '../../../node/shared/copilotApiService.js'; @@ -33,7 +32,20 @@ import { IAgentHostOTelService } from '../../../common/otel/agentHostOTelService import { AgentHostConfigKey } from '../../../common/agentHostCustomizationConfig.js'; import { createNoopCustomizationEnablementService } from '../testCustomizationEnablementService.js'; -function createAgent(disposables: Pick, models: () => Promise, rootConfig: Record = {}, userHome = '/tmp'): CodexAgent { +interface ITestAgentContext { + readonly agent: CodexAgent; + readonly stateManager: AgentHostStateManager; + readonly configurationService: AgentConfigurationService; + readonly sdkDownloader: RecordingAgentSdkDownloader; +} + +/** + * The downloader defaults to "SDK already on disk", which is what makes these + * tests deterministic — otherwise the answer depends on whether the machine + * running the suite has `@openai/codex` in `node_modules`. Tests wanting the + * cold case override `_isSdkResolvableWithoutDownload` directly. + */ +function createAgentContext(disposables: Pick, models: () => Promise, rootConfig: Record = {}, sdkDownloader = new RecordingAgentSdkDownloader()): ITestAgentContext { const instantiationService = new TestInstantiationService(); const logService = new NullLogService(); const stateManager = disposables.add(new AgentHostStateManager(logService)); @@ -45,242 +57,235 @@ function createAgent(disposables: Pick, models: () => Pr instantiationService.stub(IAgentConfigurationService, configurationService); instantiationService.stub(IAgentHostCustomizationEnablementService, createNoopCustomizationEnablementService()); instantiationService.stub(IAgentHostGitHubEndpointService, createTestGitHubEndpointService()); - instantiationService.stub(IAgentSdkDownloader, { - _serviceBrand: undefined, - isSdkResolvableWithoutDownload: () => new Promise(() => { }), - }); + instantiationService.stub(IAgentSdkDownloader, sdkDownloader); instantiationService.stub(IAgentHostCheckpointService, NULL_CHECKPOINT_SERVICE); instantiationService.stub(IAgentHostOTelService, { _serviceBrand: undefined, getNativeSdkTelemetryConfig: async () => undefined }); instantiationService.stub(IAgentHostSessionTitleSignal, { _serviceBrand: undefined, onDidChangeSessionTitle: Event.None }); instantiationService.stub(IProductService, { _serviceBrand: undefined, version: '1.0.0-test' } as IProductService); - instantiationService.stub(INativeEnvironmentService, { userHome: URI.file(userHome) }); + instantiationService.stub(INativeEnvironmentService, { userHome: URI.file('/tmp') }); instantiationService.stub(ILogService, logService); - return disposables.add(instantiationService.createInstance(CodexAgent)); + const agent = disposables.add(instantiationService.createInstance(CodexAgent)); + return { agent, stateManager, configurationService, sdkDownloader }; } -suite('CodexAgent model refresh', () => { +function createAgent(disposables: Pick, models: () => Promise, rootConfig: Record = {}, sdkDownloader = new RecordingAgentSdkDownloader()): CodexAgent { + return createAgentContext(disposables, models, rootConfig, sdkDownloader).agent; +} - const disposables = ensureNoDisposablesAreLeakedInTestSuite(); - const modelListResponse = { - data: [{ - id: 'gpt-5.6-sol', - model: 'gpt-5.6-sol', - upgrade: null, - upgradeInfo: null, - availabilityNux: null, - displayName: 'GPT-5.6-Sol', - description: 'Latest frontier agentic coding model.', - hidden: false, - supportedReasoningEfforts: [ - { reasoningEffort: 'low', description: 'Fast responses with lighter reasoning' }, - { reasoningEffort: 'medium', description: 'Balances speed and reasoning depth for everyday tasks' }, - { reasoningEffort: 'high', description: 'Greater reasoning depth for complex problems' }, - { reasoningEffort: 'xhigh', description: 'Extra high reasoning depth for complex problems' }, - { reasoningEffort: 'max', description: 'Maximum reasoning depth for the hardest problems' }, - { reasoningEffort: 'ultra', description: 'Maximum reasoning with automatic task delegation' }, - ], - defaultReasoningEffort: 'low', - inputModalities: ['text', 'image'], - supportsPersonality: true, - additionalSpeedTiers: [], - serviceTiers: [], - defaultServiceTier: null, - isDefault: true, - }], - nextCursor: null, +const modelListResponse = { + data: [{ + id: 'gpt-5.6-sol', + model: 'gpt-5.6-sol', + upgrade: null, + upgradeInfo: null, + availabilityNux: null, + displayName: 'GPT-5.6-Sol', + description: 'Latest frontier agentic coding model.', + hidden: false, + supportedReasoningEfforts: [ + { reasoningEffort: 'low', description: 'Fast responses with lighter reasoning' }, + { reasoningEffort: 'medium', description: 'Balances speed and reasoning depth for everyday tasks' }, + { reasoningEffort: 'high', description: 'Greater reasoning depth for complex problems' }, + { reasoningEffort: 'xhigh', description: 'Extra high reasoning depth for complex problems' }, + { reasoningEffort: 'max', description: 'Maximum reasoning depth for the hardest problems' }, + { reasoningEffort: 'ultra', description: 'Maximum reasoning with automatic task delegation' }, + ], + defaultReasoningEffort: 'low', + inputModalities: ['text', 'image'], + supportsPersonality: true, + additionalSpeedTiers: [], + serviceTiers: [], + defaultServiceTier: null, + isDefault: true, + }], + nextCursor: null, +}; + +/** + * @param requests records every method the agent asks for, so a test can assert + * on enumeration specifically — `config/read` shares this connection once the + * SDK is local, so a raw "did we connect" count conflates callers. + */ +function createChatGPTConnection(account: unknown = { type: 'chatgpt', email: 'person@example.com', planType: 'plus' }, requests: string[] = []) { + return { + kind: 'ready', + client: { + request: async (method: string) => { + requests.push(method); + if (method === 'account/read') { + return { account, requiresOpenaiAuth: true }; + } + if (method === 'config/read') { + return { config: { model_provider: 'openai' } }; + } + if (method === 'model/list') { + return modelListResponse; + } + throw new Error(`Unexpected request: ${method}`); + }, + }, + proxyHandle: { dispose() { } }, + child: { kill: () => true }, }; +} - function createChatGPTHome(): string { - const userHome = fs.mkdtempSync(join(os.tmpdir(), 'vscode-codex-agent-test-')); - const codexHome = join(userHome, '.codex'); - fs.mkdirSync(codexHome); - fs.writeFileSync(join(codexHome, 'auth.json'), JSON.stringify({ - auth_mode: 'chatgpt', - tokens: { access_token: 'access', refresh_token: 'refresh' }, - })); - return userHome; - } +suite('CodexAgent model refresh', () => { - function createChatGPTConnection(account: unknown = { type: 'chatgpt', email: 'person@example.com', planType: 'plus' }) { - return { - kind: 'ready', - client: { - request: async (method: string) => { - if (method === 'account/read') { - return { account, requiresOpenaiAuth: true }; - } - if (method === 'config/read') { - return { config: { model_provider: 'openai' } }; - } - if (method === 'model/list') { - return modelListResponse; - } - throw new Error(`Unexpected request: ${method}`); - }, - }, - proxyHandle: { dispose() { } }, - child: { kill: () => true }, - }; - } + const disposables = ensureNoDisposablesAreLeakedInTestSuite(); - test('eagerly enumerates authoritative ChatGPT models when existing auth is detected', async () => { - const userHome = createChatGPTHome(); - try { - const agent = createAgent(disposables, async () => [], { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }, userHome); - const connection = createChatGPTConnection(); - let resolveConnection!: () => void; - const connectionPromise = new Promise(resolve => { resolveConnection = () => resolve(connection as never); }); - let ensureConnectionCalls = 0; - agent['_isSdkResolvableWithoutDownload'] = async () => false; - agent['_ensureConnection'] = async () => { - ensureConnectionCalls++; - return connectionPromise; - }; + test('eagerly enumerates the authoritative catalog at startup when the SDK is already local', async () => { + const agent = createAgent(disposables, async () => [], { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }); + const requests: string[] = []; + let resolveConnection!: () => void; + const connectionPromise = new Promise(resolve => { resolveConnection = () => resolve(createChatGPTConnection(undefined, requests) as never); }); + let connectionRequested = false; + agent['_ensureConnection'] = async () => { + connectionRequested = true; + return connectionPromise; + }; - await new Promise(resolve => setTimeout(resolve, 0)); - assert.strictEqual(ensureConnectionCalls, 1); - assert.deepStrictEqual(agent.models.get(), []); + await new Promise(resolve => setTimeout(resolve, 0)); + assert.deepStrictEqual({ connectionRequested, models: agent.models.get() }, { connectionRequested: true, models: [] }); - resolveConnection(); - await agent.refreshModels(); + resolveConnection(); + await agent.refreshModels(); - assert.deepStrictEqual(agent.models.get().map(model => ({ provider: model.provider, id: model.id, name: model.name, meta: model._meta })), [{ + assert.deepStrictEqual({ + // One enumeration, not one per caller that happened to want the connection. + enumerations: requests.filter(method => method === 'model/list').length, + models: agent.models.get().map(model => ({ provider: model.provider, id: model.id, name: model.name, meta: model._meta })), + }, { + enumerations: 1, + models: [{ provider: 'chatgpt', id: toCodexModelSelectionId('openai', 'gpt-5.6-sol'), name: 'GPT-5.6-Sol', meta: { modelSourceId: 'chatgptSubscription' }, - }]); - } finally { - fs.rmSync(userHome, { recursive: true, force: true }); - } + }], + }); }); - test('does not enumerate ChatGPT models while signed-out use is disabled', async () => { - const userHome = createChatGPTHome(); - try { - const agent = createAgent(disposables, async () => [], {}, userHome); - let ensureConnectionCalls = 0; - agent['_isSdkResolvableWithoutDownload'] = async () => false; - agent['_ensureConnection'] = async () => { - ensureConnectionCalls++; - return createChatGPTConnection() as never; - }; + test('does not enumerate at startup while signed-out use is disabled', async () => { + const agent = createAgent(disposables, async () => [], {}); + const requests: string[] = []; + agent['_ensureConnection'] = async () => createChatGPTConnection(undefined, requests) as never; - await new Promise(resolve => setTimeout(resolve, 0)); + await new Promise(resolve => setTimeout(resolve, 0)); - assert.strictEqual(ensureConnectionCalls, 0); - assert.deepStrictEqual(agent.models.get(), []); - } finally { - fs.rmSync(userHome, { recursive: true, force: true }); - } + // Reading `config.toml` may still open a connection — that is unrelated to + // enumeration. What must not happen is asking about the account or catalog. + assert.deepStrictEqual({ + enumerationRequests: requests.filter(method => method === 'account/read' || method === 'model/list'), + models: agent.models.get(), + }, { + enumerationRequests: [], + models: [], + }); }); - test('requires Copilot unless signed-out use and persisted ChatGPT auth are both present', () => { - const userHome = createChatGPTHome(); - try { - const copilotRequired = (agent: CodexAgent) => agent.getProtectedResources()[0].required; - assert.deepStrictEqual({ - noChatGPTAuth: copilotRequired(createAgent(disposables, async () => [], { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true })), - chatGPTAuthEnabled: copilotRequired(createAgent(disposables, async () => [], { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }, userHome)), - chatGPTAuthDisabled: copilotRequired(createAgent(disposables, async () => [], {}, userHome)), - }, { - noChatGPTAuth: true, - chatGPTAuthEnabled: false, - chatGPTAuthDisabled: true, - }); - } finally { - fs.rmSync(userHome, { recursive: true, force: true }); - } + test('reports an empty catalog rather than downloading the SDK to enumerate', async () => { + const sdkDownloader = new RecordingAgentSdkDownloader(false); + const agent = createAgent(disposables, async () => [], { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }, sdkDownloader); + let ensureConnectionCalls = 0; + agent['_isSdkResolvableWithoutDownload'] = async () => false; + agent['_ensureConnection'] = async () => { + ensureConnectionCalls++; + return createChatGPTConnection() as never; + }; + + await agent.refreshModels(); + + // The download is an explicit gesture now, so a refresh that finds no local + // SDK reports the honest empty catalog and leaves the offer to the banner. + assert.deepStrictEqual({ + ensureConnectionCalls, + models: agent.models.get(), + downloads: sdkDownloader.progressInterests, + }, { + ensureConnectionCalls: 0, + models: [], + downloads: [], + }); }); - test('requires Copilot again after persisted ChatGPT auth is removed', async () => { - const userHome = createChatGPTHome(); - try { - const agent = createAgent(disposables, async () => [], { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }, userHome); - assert.strictEqual(agent.getProtectedResources()[0].required, false); - - fs.rmSync(join(userHome, '.codex', 'auth.json')); - agent['_connection'] = createChatGPTConnection(null) as never; - await agent.refreshModels(); - - assert.deepStrictEqual({ - copilotRequired: agent.getProtectedResources()[0].required, - models: agent.models.get(), - }, { - copilotRequired: true, - models: [], - }); - } finally { - fs.rmSync(userHome, { recursive: true, force: true }); - } + test('never requires Copilot, whatever the flag says and whatever the account turns out to be', async () => { + const copilotRequired = (agent: CodexAgent) => agent.getProtectedResources()[0].required; + const withoutSdk = createAgent(disposables, async () => [], { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }); + withoutSdk['_isSdkResolvableWithoutDownload'] = async () => false; + const withoutAccount = createAgent(disposables, async () => [], { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }); + withoutAccount['_connection'] = createChatGPTConnection(null) as never; + const withAccount = createAgent(disposables, async () => [], { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }); + withAccount['_connection'] = createChatGPTConnection() as never; + await Promise.all([withoutAccount.refreshModels(), withAccount.refreshModels()]); + + // `required: false` is unconditional: a `true` here from any of these + // combinations puts the whole Agents window behind a GitHub sign-in wall, + // because `resolveSignedOutWindowGate` forces sign-in only when *every* + // session type requires GitHub. + assert.deepStrictEqual({ + signedOutUseDisabled: copilotRequired(createAgent(disposables, async () => [], {})), + noLocalSdk: copilotRequired(withoutSdk), + noAccount: copilotRequired(withoutAccount), + chatGPTAccount: copilotRequired(withAccount), + }, { + signedOutUseDisabled: false, + noLocalSdk: false, + noAccount: false, + chatGPTAccount: false, + }); }); test('waits for an app-server already starting when signed-out use becomes enabled', async () => { - const userHome = createChatGPTHome(); - try { - const agent = createAgent(disposables, async () => [], {}, userHome); - const connection = createChatGPTConnection(); - let resolveConnection!: () => void; - agent['_connection'] = { kind: 'starting', promise: new Promise(resolve => { resolveConnection = () => resolve(connection as never); }) }; - - agent['_configurationService'].updateRootConfig({ [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }); - await new Promise(resolve => setTimeout(resolve, 0)); - assert.deepStrictEqual(agent.models.get(), []); + const agent = createAgent(disposables, async () => [], {}); + const connection = createChatGPTConnection(); + let resolveConnection!: () => void; + agent['_connection'] = { kind: 'starting', promise: new Promise(resolve => { resolveConnection = () => resolve(connection as never); }) }; - resolveConnection(); - await agent.refreshModels(); + agent['_configurationService'].updateRootConfig({ [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }); + await new Promise(resolve => setTimeout(resolve, 0)); + assert.deepStrictEqual(agent.models.get(), []); - assert.deepStrictEqual(agent.models.get().map(model => model.id), [toCodexModelSelectionId('openai', 'gpt-5.6-sol')]); - } finally { - fs.rmSync(userHome, { recursive: true, force: true }); - } + resolveConnection(); + await agent.refreshModels(); + + assert.deepStrictEqual(agent.models.get().map(model => model.id), [toCodexModelSelectionId('openai', 'gpt-5.6-sol')]); }); - test('does not publish ChatGPT models when detected credentials are invalid', async () => { - const userHome = createChatGPTHome(); - try { - const copilotModels = [{ id: 'copilot-model', name: 'Copilot Model', supported_endpoints: ['/responses'] }] as CCAModel[]; - const agent = createAgent(disposables, async () => copilotModels, { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }, userHome); - agent['_githubToken'] = 'token'; - agent['_connection'] = createChatGPTConnection(null) as never; - - await agent.refreshModels(); - - assert.deepStrictEqual({ - providers: agent.models.get().map(model => model.provider), - copilotRequired: agent.getProtectedResources()[0].required, - }, { - providers: ['copilot'], - copilotRequired: true, - }); - } finally { - fs.rmSync(userHome, { recursive: true, force: true }); - } + test('publishes no ChatGPT models when the app server reports no account', async () => { + const copilotModels = [{ id: 'copilot-model', name: 'Copilot Model', supported_endpoints: ['/responses'] }] as CCAModel[]; + const agent = createAgent(disposables, async () => copilotModels, { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }); + agent['_githubToken'] = 'token'; + agent['_connection'] = createChatGPTConnection(null) as never; + + await agent.refreshModels(); + + assert.deepStrictEqual({ + providers: agent.models.get().map(model => model.provider), + copilotRequired: agent.getProtectedResources()[0].required, + }, { + providers: ['copilot'], + copilotRequired: false, + }); }); test('does not publish a model when authoritative discovery fails', async () => { - const userHome = createChatGPTHome(); - try { - const agent = createAgent(disposables, async () => [], { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }, userHome); - agent['_connection'] = { - kind: 'ready', - client: { - request: async (method: string) => { - if (method === 'account/read') { - return { account: { type: 'chatgpt', email: null, planType: 'plus' }, requiresOpenaiAuth: true }; - } - throw new Error('model discovery failed'); - }, + const agent = createAgent(disposables, async () => [], { [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }); + agent['_connection'] = { + kind: 'ready', + client: { + request: async (method: string) => { + if (method === 'account/read') { + return { account: { type: 'chatgpt', email: null, planType: 'plus' }, requiresOpenaiAuth: true }; + } + throw new Error('model discovery failed'); }, - proxyHandle: { dispose() { } }, - child: { kill: () => true }, - } as never; - - await agent.refreshModels(); - assert.deepStrictEqual(agent.models.get(), []); - } finally { - fs.rmSync(userHome, { recursive: true, force: true }); - } + }, + proxyHandle: { dispose() { } }, + child: { kill: () => true }, + } as never; + + await agent.refreshModels(); + assert.deepStrictEqual(agent.models.get(), []); }); test('keeps the last known-good models when a periodic refresh fails', async () => { @@ -594,7 +599,9 @@ suite('CodexAgent model refresh', () => { await agent['_signOutOfChatGPT'](); assert.deepStrictEqual({ - requests, + // Scoped to the sign-out gesture: with the SDK local, the startup + // `config.toml` read lands on this same connection. + requests: requests.filter(method => method.startsWith('account/')), accountStatus: agent['_openAIAccountState'].status, }, { requests: ['account/logout', 'account/read'], @@ -659,3 +666,142 @@ suite('CodexAgent model refresh', () => { }); }); }); + +suite('CodexAgent — agent SDK setup channel', () => { + + const disposables = ensureNoDisposablesAreLeakedInTestSuite(); + + /** What the workbench would read off root state right now. */ + function readSetup(ctx: ITestAgentContext) { + return readAgentSdkSetupInfos(ctx.stateManager.rootState).find(setup => setup.agent === 'codex'); + } + + /** Addresses a download request at an agent the way `IAgentSdkSetupService` does. */ + function dispatchDownload(ctx: ITestAgentContext, agent = 'codex', request = 'req-1'): void { + ctx.configurationService.updateRootConfig({ [AGENT_SDK_SETUP_DOWNLOAD_REQUEST_KEY]: { agent, request } }); + } + + /** Waits for the ctor's queued publish (and any refresh it chains) to settle. */ + async function settle(): Promise { + for (let i = 0; i < 20; i++) { + await new Promise(resolve => setTimeout(resolve, 0)); + } + } + + /** + * A build that knows where to fetch the SDK from but has not yet — the state + * the banner's offer exists for. Both flags are set explicitly because + * `isAvailable` false would fall through to `resolveCodexDevSdkRoot()`. + */ + function createNotDownloaded(): RecordingAgentSdkDownloader { + const sdkDownloader = new RecordingAgentSdkDownloader(); + sdkDownloader.resolvableWithoutDownload = false; + return sdkDownloader; + } + + test('an SDK already on disk publishes `ready`, plus the docs URL and sign-in affordance the banner offers', async () => { + const ctx = createAgentContext(disposables, async () => []); + await settle(); + + assert.deepStrictEqual(readSetup(ctx), { + agent: 'codex', + download: 'ready', + setupDocsUrl: 'https://learn.chatgpt.com/codex/auth', + // Unlike Claude, ChatGPT sign-in is a control request the app server + // answers, so the banner can start it without the user leaving the window. + signInProviderName: 'ChatGPT', + }); + }); + + test('a cold cache publishes `notDownloaded`, which is what turns the banner into an offer', async () => { + const ctx = createAgentContext(disposables, async () => [], {}, createNotDownloaded()); + await settle(); + + assert.strictEqual(readSetup(ctx)?.download, 'notDownloaded'); + }); + + test('an explicit download fetches the SDK, holds progress interest for the fetch, and ends at `ready`', async () => { + const sdkDownloader = createNotDownloaded(); + let releaseDownload = () => { }; + const downloaded = new Promise(resolve => { + // Releasing the gate is the moment the SDK lands on disk. + releaseDownload = () => { sdkDownloader.resolvableWithoutDownload = true; resolve(); }; + }); + sdkDownloader.loadSdkRootResult = async () => { await downloaded; return '/tmp/codex-sdk'; }; + const ctx = createAgentContext(disposables, async () => [], {}, sdkDownloader); + // The refresh the download chains must not spawn a real app server. + ctx.agent['_ensureConnection'] = async () => { throw new Error('offline'); }; + await settle(); + + dispatchDownload(ctx); + await settle(); + const inFlight = { + download: readSetup(ctx)?.download, + interests: [...sdkDownloader.progressInterests], + held: sdkDownloader.heldProgressInterests, + }; + + releaseDownload(); + await settle(); + + assert.deepStrictEqual({ inFlight, after: readSetup(ctx)?.download, held: sdkDownloader.heldProgressInterests }, { + inFlight: { download: 'downloading', interests: ['codex'], held: 1 }, + after: 'ready', + held: 0, + }); + }); + + test('the request key is cleared as it is consumed, so an identical later press still lands', async () => { + const sdkDownloader = createNotDownloaded(); + let downloads = 0; + sdkDownloader.loadSdkRootResult = async () => { downloads++; return '/tmp/codex-sdk'; }; + const ctx = createAgentContext(disposables, async () => [], {}, sdkDownloader); + ctx.agent['_ensureConnection'] = async () => { throw new Error('offline'); }; + await settle(); + + dispatchDownload(ctx, 'codex', 'press-1'); + await settle(); + const consumed = ctx.configurationService.getRootConfigValues()[AGENT_SDK_SETUP_DOWNLOAD_REQUEST_KEY]; + + dispatchDownload(ctx, 'codex', 'press-2'); + await settle(); + + assert.deepStrictEqual({ consumed, downloads }, { consumed: undefined, downloads: 2 }); + }); + + test('a request addressed to another agent is ignored', async () => { + const sdkDownloader = createNotDownloaded(); + const ctx = createAgentContext(disposables, async () => [], {}, sdkDownloader); + await settle(); + + dispatchDownload(ctx, 'claude'); + await settle(); + + assert.deepStrictEqual({ + downloads: sdkDownloader.progressInterests, + // Left in place for the agent it names, rather than consumed by this one. + key: ctx.configurationService.getRootConfigValues()[AGENT_SDK_SETUP_DOWNLOAD_REQUEST_KEY], + }, { + downloads: [], + key: { agent: 'claude', request: 'req-1' }, + }); + }); + + test('a failed download releases the progress interest and stops claiming to be downloading', async () => { + const sdkDownloader = createNotDownloaded(); + sdkDownloader.loadSdkRootResult = async () => { throw new Error('CDN unreachable'); }; + const ctx = createAgentContext(disposables, async () => [], {}, sdkDownloader); + await settle(); + + dispatchDownload(ctx); + await settle(); + + assert.deepStrictEqual({ + download: readSetup(ctx)?.download, + held: sdkDownloader.heldProgressInterests, + }, { + download: 'notDownloaded', + held: 0, + }); + }); +}); diff --git a/src/vs/platform/agentHost/test/node/codex/codexSessionConfigKeys.test.ts b/src/vs/platform/agentHost/test/node/codex/codexSessionConfigKeys.test.ts index c4e6a27014573..ab59ebd65eadb 100644 --- a/src/vs/platform/agentHost/test/node/codex/codexSessionConfigKeys.test.ts +++ b/src/vs/platform/agentHost/test/node/codex/codexSessionConfigKeys.test.ts @@ -19,6 +19,7 @@ import { ICodexProxyService } from '../../../node/codex/codexProxyService.js'; import { IAgentConfigurationService } from '../../../node/agentConfigurationService.js'; import { IAgentHostCustomizationEnablementService } from '../../../node/agentHostCustomizationEnablementService.js'; import { IAgentSdkDownloader } from '../../../node/agentSdkDownloader.js'; +import { RecordingAgentSdkDownloader } from '../testAgentSdkDownloader.js'; import { IAgentHostCheckpointService, NULL_CHECKPOINT_SERVICE } from '../../../common/agentHostCheckpointService.js'; import { ICopilotApiService } from '../../../node/shared/copilotApiService.js'; import { SessionConfigKey } from '../../../common/sessionConfigKeys.js'; @@ -40,7 +41,7 @@ function createAgent(disposables: Pick): CodexAgent { getRootValue: () => undefined, }); instantiationService.stub(IAgentHostCustomizationEnablementService, createNoopCustomizationEnablementService()); - instantiationService.stub(IAgentSdkDownloader, { _serviceBrand: undefined }); + instantiationService.stub(IAgentSdkDownloader, new RecordingAgentSdkDownloader()); instantiationService.stub(IAgentHostCheckpointService, NULL_CHECKPOINT_SERVICE); instantiationService.stub(IAgentHostOTelService, { _serviceBrand: undefined, getNativeSdkTelemetryConfig: async () => undefined }); instantiationService.stub(IAgentHostSessionTitleSignal, { _serviceBrand: undefined, onDidChangeSessionTitle: Event.None }); diff --git a/src/vs/platform/agentHost/test/node/codex/codexSessionTitleSpans.test.ts b/src/vs/platform/agentHost/test/node/codex/codexSessionTitleSpans.test.ts index 88b2996b9ad51..8e6ddfc1a4b3e 100644 --- a/src/vs/platform/agentHost/test/node/codex/codexSessionTitleSpans.test.ts +++ b/src/vs/platform/agentHost/test/node/codex/codexSessionTitleSpans.test.ts @@ -23,6 +23,7 @@ import { IAgentHostCustomizationEnablementService } from '../../../node/agentHos import { AgentHostSessionTitleSignal, IAgentHostSessionTitleSignal } from '../../../node/agentHostSessionTitleSignal.js'; import { AgentHostStateManager } from '../../../node/agentHostStateManager.js'; import { IAgentSdkDownloader } from '../../../node/agentSdkDownloader.js'; +import { RecordingAgentSdkDownloader } from '../testAgentSdkDownloader.js'; import { CodexAgent } from '../../../node/codex/codexAgent.js'; import { ICodexProxyService } from '../../../node/codex/codexProxyService.js'; import { ICopilotApiService } from '../../../node/shared/copilotApiService.js'; @@ -71,7 +72,7 @@ function createTestContext(disposables: Pick): { stateMa getRootValue: () => undefined, }); instantiationService.stub(IAgentHostCustomizationEnablementService, createNoopCustomizationEnablementService()); - instantiationService.stub(IAgentSdkDownloader, { _serviceBrand: undefined }); + instantiationService.stub(IAgentSdkDownloader, new RecordingAgentSdkDownloader()); instantiationService.stub(IAgentHostCheckpointService, NULL_CHECKPOINT_SERVICE); instantiationService.stub(IAgentHostOTelService, otelService); instantiationService.stub(IAgentHostSessionTitleSignal, disposables.add(new AgentHostSessionTitleSignal(stateManager))); diff --git a/src/vs/platform/agentHost/test/node/testAgentSdkDownloader.ts b/src/vs/platform/agentHost/test/node/testAgentSdkDownloader.ts new file mode 100644 index 0000000000000..ecb6b3ff81c24 --- /dev/null +++ b/src/vs/platform/agentHost/test/node/testAgentSdkDownloader.ts @@ -0,0 +1,66 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Event } from '../../../../base/common/event.js'; +import { IDisposable, toDisposable } from '../../../../base/common/lifecycle.js'; +import { IAgentSdkDownloader, IAgentSdkPackage } from '../../node/agentSdkDownloader.js'; + +/** + * Downloader stub that records the interactions worth asserting on and refuses + * the rest loudly. + * + * {@link available} answers `isAvailable` — "this build knows where to fetch the + * SDK from". {@link resolvableWithoutDownload} answers the separate question of + * whether it is already on disk, and defaults to {@link available} because that + * is the common "SDK is here" case. Setting `available` true and + * `resolvableWithoutDownload` false is the state the setup banner exists for: a + * download is possible but has not happened yet. Neither ever falls through to + * an agent's dev fallback, which would read this repo's `node_modules` and make + * the answer depend on the machine. + * + * {@link loadSdkRootResult} is unset by default, so an unexpected cold download + * surfaces as a thrown error rather than a silently mocked success. Fetching is + * the downloader's own job and is covered by its own tests; what agents owe is + * the progress interest held for the duration of a user-requested download, + * which is what {@link heldProgressInterests} pins. + */ +export class RecordingAgentSdkDownloader implements IAgentSdkDownloader { + declare readonly _serviceBrand: undefined; + + readonly onDidDownloadProgress = Event.None; + + /** Package ids for progress interests taken, and how many are still held. */ + readonly progressInterests: string[] = []; + heldProgressInterests = 0; + + /** Whether the SDK is already on disk. Defaults to {@link available}. */ + resolvableWithoutDownload: boolean | undefined; + + /** What `loadSdkRoot` resolves to. Unset means "no download was expected here". */ + loadSdkRootResult: (() => Promise) | undefined; + + constructor(public available = true) { } + + acquireDownloadProgressInterest(pkg: IAgentSdkPackage): IDisposable { + this.progressInterests.push(pkg.id); + this.heldProgressInterests++; + return toDisposable(() => { this.heldProgressInterests--; }); + } + + isAvailable(): boolean { + return this.available; + } + + async isSdkResolvableWithoutDownload(): Promise { + return this.resolvableWithoutDownload ?? this.available; + } + + loadSdkRoot(pkg: IAgentSdkPackage): Promise { + if (!this.loadSdkRootResult) { + throw new Error(`test stub: unexpected SDK download for ${pkg.id}`); + } + return this.loadSdkRootResult(); + } +} diff --git a/src/vs/sessions/browser/sessionsAuthGate.ts b/src/vs/sessions/browser/sessionsAuthGate.ts index 1745a46bb8b75..ca553b8d88f38 100644 --- a/src/vs/sessions/browser/sessionsAuthGate.ts +++ b/src/vs/sessions/browser/sessionsAuthGate.ts @@ -114,38 +114,3 @@ export function observeAllowSignedOutWhenUsable(configurationService: IConfigura Event.filter(configurationService.onDidChangeConfiguration, e => e.affectsConfiguration(AgentHostAllowSignedOutWhenUsableSettingId)), () => isAllowSignedOutWhenUsableEnabled(configurationService)); } - -/** - * Inputs to the "discovered your existing configuration" nudge for a - * single agent-host session type. - */ -export interface IDiscoveredConfigNudgeContext { - /** Whether a GitHub account is currently signed in. */ - readonly signedIn: boolean; - /** The `chat.agentHost.allowSignedOutWhenUsable` experimentation opt-in. */ - readonly allowSignedOutWhenUsable: boolean; - /** - * Whether the agent's session type is usable without GitHub right now — i.e. - * its agent discovered an existing native configuration and is running in - * native mode rather than the Copilot proxy. - */ - readonly usableWithoutGitHub: boolean; - /** - * Whether the user has already dismissed this nudge, which silences it for - * good. Once muted, the nudge never shows again regardless of the other - * inputs. - */ - readonly muted: boolean; -} - -/** - * Decides whether to surface the discovered-config nudge for one agent-host - * session type: shown only to a signed-out user who has opted in, when that - * type is usable without GitHub right now — the agent found an existing native - * config, so we let them in and explain how to switch to a Copilot subscription - * instead. Signed-in users never see it; with the opt-in off, or once the user - * has muted it, it is always false. - */ -export function shouldShowDiscoveredConfigNudge(context: IDiscoveredConfigNudgeContext): boolean { - return !context.signedIn && context.allowSignedOutWhenUsable && context.usableWithoutGitHub && !context.muted; -} diff --git a/src/vs/sessions/contrib/chat/browser/mobile/mobileSessionTypePicker.ts b/src/vs/sessions/contrib/chat/browser/mobile/mobileSessionTypePicker.ts index 06c198ef04125..c0815437da19a 100644 --- a/src/vs/sessions/contrib/chat/browser/mobile/mobileSessionTypePicker.ts +++ b/src/vs/sessions/contrib/chat/browser/mobile/mobileSessionTypePicker.ts @@ -12,6 +12,7 @@ import { IChatSessionsService } from '../../../../../workbench/contrib/chat/comm import { ILanguageModelsService } from '../../../../../workbench/contrib/chat/common/languageModels.js'; import { getSessionTypeAvailability, getSessionTypeUnavailableLabel, SessionTypeAvailability } from '../../../../../workbench/contrib/chat/browser/agentSessions/sessionTypeAvailability.js'; import { IChatEntitlementService } from '../../../../../workbench/services/chat/common/chatEntitlementService.js'; +import { IChatInputNotificationService } from '../../../../../workbench/contrib/chat/browser/widget/input/chatInputNotificationService.js'; import { IProviderSessionType, ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; import { ISessionsProvidersService } from '../../../../services/sessions/browser/sessionsProvidersService.js'; import { ISession } from '../../../../services/sessions/common/session.js'; @@ -47,10 +48,11 @@ export class MobileSessionTypePicker extends SessionTypePicker { @IChatEntitlementService chatEntitlementService: IChatEntitlementService, @ILanguageModelsService languageModelsService: ILanguageModelsService, @IConfigurationService configurationService: IConfigurationService, + @IChatInputNotificationService chatInputNotificationService: IChatInputNotificationService, @IWorkbenchLayoutService private readonly layoutService: IWorkbenchLayoutService, @IContextKeyService contextKeyService: IContextKeyService, ) { - super(session, options, actionWidgetService, sessionsManagementService, _sessionsProvidersService, storageService, telemetryService, chatSessionsService, chatEntitlementService, languageModelsService, configurationService, contextKeyService); + super(session, options, actionWidgetService, sessionsManagementService, _sessionsProvidersService, storageService, telemetryService, chatSessionsService, chatEntitlementService, languageModelsService, configurationService, chatInputNotificationService, contextKeyService); } override render(container: HTMLElement, options?: { className?: string }): void { diff --git a/src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts b/src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts index 99023ab0df3cd..76f8116e367b1 100644 --- a/src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts +++ b/src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts @@ -26,6 +26,8 @@ import { ITelemetryService } from '../../../../platform/telemetry/common/telemet import { IChatSessionsService } from '../../../../workbench/contrib/chat/common/chatSessionsService.js'; import { ILanguageModelsService } from '../../../../workbench/contrib/chat/common/languageModels.js'; import { getSessionTypeAvailability, getSessionTypePickerAvailability, getSessionTypeUnavailableDescription, getSessionTypeUnavailableHover, SessionTypeAvailability } from '../../../../workbench/contrib/chat/browser/agentSessions/sessionTypeAvailability.js'; +import { hasAgentSdkSetupNotification } from '../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSdkSetupNotification.js'; +import { IChatInputNotificationService } from '../../../../workbench/contrib/chat/browser/widget/input/chatInputNotificationService.js'; import { IChatEntitlementService } from '../../../../workbench/services/chat/common/chatEntitlementService.js'; import { markOnboardingTarget } from '../../../../workbench/contrib/onboarding/browser/spotlight/onboardingTarget.js'; import { reportNewChatPickerClosed } from './newChatPickerTelemetry.js'; @@ -167,6 +169,7 @@ export class SessionTypePicker extends Disposable { @IChatEntitlementService protected readonly chatEntitlementService: IChatEntitlementService, @ILanguageModelsService protected readonly languageModelsService: ILanguageModelsService, @IConfigurationService protected readonly configurationService: IConfigurationService, + @IChatInputNotificationService protected readonly chatInputNotificationService: IChatInputNotificationService, @IContextKeyService contextKeyService: IContextKeyService, ) { super(); @@ -462,6 +465,7 @@ export class SessionTypePicker extends Disposable { modelTarget, getSessionTypeAvailability(this.chatSessionsService, this.chatEntitlementService, this.languageModelsService, modelTarget, allowSignedOutWhenUsable), allowSignedOutWhenUsable, + hasAgentSdkSetupNotification(this.chatInputNotificationService, modelTarget), ); const unavailable = availability !== SessionTypeAvailability.Available; const item: ISessionTypePickerItem = { diff --git a/src/vs/sessions/contrib/chat/test/browser/sessionTypePicker.test.ts b/src/vs/sessions/contrib/chat/test/browser/sessionTypePicker.test.ts index 633c3026b64bb..1355c07f504b7 100644 --- a/src/vs/sessions/contrib/chat/test/browser/sessionTypePicker.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/sessionTypePicker.test.ts @@ -12,6 +12,7 @@ import { URI } from '../../../../../base/common/uri.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; import { IActionWidgetService } from '../../../../../platform/actionWidget/browser/actionWidget.js'; import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; +import { IChatInputNotificationService } from '../../../../../workbench/contrib/chat/browser/widget/input/chatInputNotificationService.js'; import { TestConfigurationService } from '../../../../../platform/configuration/test/common/testConfigurationService.js'; import { IContextKeyService } from '../../../../../platform/contextkey/common/contextkey.js'; import { MockContextKeyService } from '../../../../../platform/keybinding/test/common/mockKeybindingService.js'; @@ -137,6 +138,7 @@ function createPicker( lookupLanguageModel: () => undefined, }); instantiationService.stub(IConfigurationService, new TestConfigurationService()); + instantiationService.stub(IChatInputNotificationService, { getActiveNotification: () => undefined }); instantiationService.stub(IContextKeyService, new MockContextKeyService()); return disposables.add(instantiationService.createInstance(TestSessionTypePicker, session, options)); } diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/agentHostDiscoveredConfigNotification.ts b/src/vs/sessions/contrib/providers/agentHost/browser/agentHostDiscoveredConfigNotification.ts deleted file mode 100644 index 7466e89dfef45..0000000000000 --- a/src/vs/sessions/contrib/providers/agentHost/browser/agentHostDiscoveredConfigNotification.ts +++ /dev/null @@ -1,162 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import { Event } from '../../../../../base/common/event.js'; -import { Disposable } from '../../../../../base/common/lifecycle.js'; -import { localize } from '../../../../../nls.js'; -import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; -import { IDefaultAccountService } from '../../../../../platform/defaultAccount/common/defaultAccount.js'; -import { IStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; -import { AgentHostAllowSignedOutWhenUsableSettingId } from '../../../../../platform/agentHost/common/agentService.js'; -import { IWorkbenchContribution } from '../../../../../workbench/common/contributions.js'; -import { SessionType } from '../../../../../workbench/contrib/chat/common/chatSessionsService.js'; -import { SessionTypeAuthRequirement } from '../../../../services/sessions/common/session.js'; -import { ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; -import { ChatInputNotificationActionKind, ChatInputNotificationSeverity, IChatInputNotificationService } from '../../../../../workbench/contrib/chat/browser/widget/input/chatInputNotificationService.js'; -import { ConditionalAuthState, conditionalAuthState, isAllowSignedOutWhenUsableEnabled, shouldShowDiscoveredConfigNudge } from '../../../../browser/sessionsAuthGate.js'; - -const DISCOVERED_CONFIG_NOTIFICATION_ID = 'agentHost.discoveredConfig.claude'; - -/** Single entry point for starting GitHub Copilot sign-in from a nudge. */ -const SIGN_IN_COMMAND_ID = 'workbench.action.chat.triggerSetup'; - -/** - * Persists the user's dismissal. The discovered config lives on this machine, so - * the preference is scoped to the machine — {@link StorageScope.APPLICATION} to - * span profiles and workspaces, and {@link StorageTarget.MACHINE} so settings - * sync does not carry it to a machine where no such config exists. - */ -const MUTED_STORAGE_KEY = 'agentHost.discoveredConfig.claude.muted'; - -/** - * Surfaces a calm chat-input notification in the Agents window when a signed-out - * user — who has opted into `chat.agentHost.allowSignedOutWhenUsable` — lands - * with the Claude agent running in native mode because it discovered an existing - * configuration on disk. Instead of forcing GitHub sign-in, the Agents window - * lets them in; this banner explains what happened and offers a single "Sign in - * to GitHub" action for anyone who actually meant to use a Copilot subscription. - * - * The banner is scoped to the Claude session type (so it only renders when that - * harness is selected) and clears itself the moment the user signs in or the - * agent stops advertising native mode. Dismissing it with the X persists a - * machine-wide choice not to show it again — the nudge is informational, so a - * user who has read it once has read it for good. Sending a message merely hides - * it for the current window. - */ -export class AgentHostDiscoveredConfigNotificationContribution extends Disposable implements IWorkbenchContribution { - - static readonly ID = 'sessions.contrib.agentHostDiscoveredConfigNotification'; - - private _shown = false; - /** - * Set once the initial default-account resolution has completed. Until then - * {@link IDefaultAccountService.currentDefaultAccount} reads as `null` even for - * a signed-in user, so the nudge stays suppressed to avoid flashing at a - * signed-in user during the startup gap. - */ - private _accountResolved = false; - - constructor( - @IChatInputNotificationService private readonly _chatInputNotificationService: IChatInputNotificationService, - @ISessionsManagementService private readonly _sessionsManagementService: ISessionsManagementService, - @IDefaultAccountService private readonly _defaultAccountService: IDefaultAccountService, - @IConfigurationService private readonly _configurationService: IConfigurationService, - @IStorageService private readonly _storageService: IStorageService, - ) { - super(); - - // Dismissing the banner is the user telling us they've read it, so persist - // that; the storage listener below then re-drives `_update` to tear it - // down. `onDidDismiss` fires only for an explicit dismissal — the - // auto-dismiss on send does not, so sending a message still just hides - // the nudge for this window. - this._register(this._chatInputNotificationService.onDidDismiss(id => { - if (id === DISCOVERED_CONFIG_NOTIFICATION_ID) { - this._storageService.store(MUTED_STORAGE_KEY, true, StorageScope.APPLICATION, StorageTarget.MACHINE); - } - })); - - // Signing in/out flips the nudge; a session-type change is how the agent - // host signals that Claude switched between native and proxy (i.e. whether - // it is usable without GitHub); the opt-in and the mute can both toggle at - // runtime (the mute from another window on this machine). - this._register(Event.any( - this._defaultAccountService.onDidChangeDefaultAccount, - this._sessionsManagementService.onDidChangeSessionTypes, - Event.filter(this._configurationService.onDidChangeConfiguration, e => e.affectsConfiguration(AgentHostAllowSignedOutWhenUsableSettingId), this._store), - this._storageService.onDidChangeValue(StorageScope.APPLICATION, MUTED_STORAGE_KEY, this._store), - )(() => this._update())); - - // Until the account resolves, `currentDefaultAccount === null` reads as - // "signed out" and would flash this signed-out nudge at a signed-in user - // during startup. The account loads silently (no change event fires), so - // await the first resolution, then re-evaluate. - this._defaultAccountService.getDefaultAccount().then(() => { - if (this._store.isDisposed) { - return; - } - this._accountResolved = true; - this._update(); - }); - } - - private _update(): void { - // While the account is unresolved, `currentDefaultAccount` is null for - // everyone; treating that as "signed out" flashes the nudge at a signed-in - // user. Nothing is shown yet, so there is nothing to tear down — just wait. - const authState = conditionalAuthState(this._accountResolved, this._defaultAccountService.currentDefaultAccount !== null); - if (authState === ConditionalAuthState.Unresolved) { - return; - } - - // The Claude agent-host session type, once the host has advertised it. - // Two providers (local / remote agent host) can offer the same id, so - // prefer a usable instance and fall back to any for the display label. - const claudeTypes = this._sessionsManagementService.getAllProviderSessionTypes() - .filter(type => (type.sessionType.chatSessionType ?? type.sessionType.id) === SessionType.AgentHostClaude) - .map(type => type.sessionType); - const claude = claudeTypes.find(type => type.authRequirement === SessionTypeAuthRequirement.None) ?? claudeTypes[0]; - - const show = shouldShowDiscoveredConfigNudge({ - signedIn: authState === ConditionalAuthState.SignedIn, - allowSignedOutWhenUsable: isAllowSignedOutWhenUsableEnabled(this._configurationService), - usableWithoutGitHub: claude?.authRequirement === SessionTypeAuthRequirement.None, - muted: this._storageService.getBoolean(MUTED_STORAGE_KEY, StorageScope.APPLICATION, false), - }); - - if (!show) { - if (this._shown) { - this._chatInputNotificationService.deleteNotification(DISCOVERED_CONFIG_NOTIFICATION_ID); - this._shown = false; - } - return; - } - - // Already up: don't re-push, which would clear a pending user dismissal. - if (this._shown || !claude) { - return; - } - this._shown = true; - - this._chatInputNotificationService.setNotification({ - id: DISCOVERED_CONFIG_NOTIFICATION_ID, - severity: ChatInputNotificationSeverity.Info, - message: localize('agentHost.discoveredConfig.message', "We've discovered your existing {0} configuration.", claude.label), - description: localize('agentHost.discoveredConfig.description', "If you intended to use a Copilot subscription, sign in to GitHub."), - actions: [{ - kind: ChatInputNotificationActionKind.Command, - label: localize('agentHost.discoveredConfig.signIn', "Sign in to GitHub"), - commandId: SIGN_IN_COMMAND_ID, - // Dismissal is permanent now, so a sign-in click — which the user - // may still cancel — must not route through it. The banner retires - // on its own once the account resolves to signed in. - keepOpen: true, - }], - dismissible: true, - autoDismissOnMessage: true, - sessionTypes: [SessionType.AgentHostClaude], - }); - } -} diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/localAgentHost.contribution.ts b/src/vs/sessions/contrib/providers/agentHost/browser/localAgentHost.contribution.ts index 6d7906174b714..65448ea723cc1 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/localAgentHost.contribution.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/localAgentHost.contribution.ts @@ -11,7 +11,7 @@ import { AgentHostContribution } from '../../../../../workbench/contrib/chat/bro import { IAgentHostSessionWorkingDirectoryResolver } from '../../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionWorkingDirectoryResolver.js'; import { AgentHostTerminalContribution } from '../../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentHostTerminalContribution.js'; import { AgentHostAllowSignedOutWhenUsableContribution } from '../../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentHostAllowSignedOutWhenUsableContribution.js'; -import { AgentHostDiscoveredConfigNotificationContribution } from './agentHostDiscoveredConfigNotification.js'; +import { AgentHostSdkSetupNotificationContribution } from '../../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSdkSetupNotification.js'; import { AgentHostSignedOutModelsNotificationContribution } from '../../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSignedOutModelsNotification.js'; import { ISessionsProvidersService } from '../../../../services/sessions/browser/sessionsProvidersService.js'; import { SessionStatus } from '../../../../services/sessions/common/session.js'; @@ -89,6 +89,6 @@ class LocalAgentHostContribution extends Disposable implements IWorkbenchContrib registerWorkbenchContribution2(AgentHostContribution.ID, AgentHostContribution, WorkbenchPhase.AfterRestored); registerWorkbenchContribution2(AgentHostTerminalContribution.ID, AgentHostTerminalContribution, WorkbenchPhase.AfterRestored); registerWorkbenchContribution2(AgentHostAllowSignedOutWhenUsableContribution.ID, AgentHostAllowSignedOutWhenUsableContribution, WorkbenchPhase.AfterRestored); -registerWorkbenchContribution2(AgentHostDiscoveredConfigNotificationContribution.ID, AgentHostDiscoveredConfigNotificationContribution, WorkbenchPhase.AfterRestored); registerWorkbenchContribution2(AgentHostSignedOutModelsNotificationContribution.ID, AgentHostSignedOutModelsNotificationContribution, WorkbenchPhase.AfterRestored); +registerWorkbenchContribution2(AgentHostSdkSetupNotificationContribution.ID, AgentHostSdkSetupNotificationContribution, WorkbenchPhase.AfterRestored); registerWorkbenchContribution2(LocalAgentHostContribution.ID, LocalAgentHostContribution, WorkbenchPhase.AfterRestored); diff --git a/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostDiscoveredConfigNotification.test.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostDiscoveredConfigNotification.test.ts deleted file mode 100644 index 52dede3c4f86d..0000000000000 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostDiscoveredConfigNotification.test.ts +++ /dev/null @@ -1,140 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import assert from 'assert'; -import { timeout } from '../../../../../../base/common/async.js'; -import { Codicon } from '../../../../../../base/common/codicons.js'; -import { Emitter, Event } from '../../../../../../base/common/event.js'; -import { Disposable, DisposableStore } from '../../../../../../base/common/lifecycle.js'; -import { isWeb } from '../../../../../../base/common/platform.js'; -import { mock } from '../../../../../../base/test/common/mock.js'; -import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; -import { AgentHostAllowSignedOutWhenUsableSettingId } from '../../../../../../platform/agentHost/common/agentService.js'; -import { TestConfigurationService } from '../../../../../../platform/configuration/test/common/testConfigurationService.js'; -import { IDefaultAccountService } from '../../../../../../platform/defaultAccount/common/defaultAccount.js'; -import { InMemoryStorageService } from '../../../../../../platform/storage/common/storage.js'; -import { IChatInputNotification, IChatInputNotificationService } from '../../../../../../workbench/contrib/chat/browser/widget/input/chatInputNotificationService.js'; -import { SessionType } from '../../../../../../workbench/contrib/chat/common/chatSessionsService.js'; -import { SessionTypeAuthRequirement } from '../../../../../services/sessions/common/session.js'; -import { IProviderSessionType, ISessionsManagementService } from '../../../../../services/sessions/common/sessionsManagement.js'; -import { AgentHostDiscoveredConfigNotificationContribution } from '../../browser/agentHostDiscoveredConfigNotification.js'; - -class TestChatInputNotificationService extends Disposable implements IChatInputNotificationService { - declare readonly _serviceBrand: undefined; - - readonly onDidChange = Event.None; - private readonly _onDidDismiss = this._register(new Emitter()); - readonly onDidDismiss = this._onDidDismiss.event; - - readonly notifications = new Map(); - - setNotification(notification: IChatInputNotification): void { - this.notifications.set(notification.id, notification); - } - deleteNotification(id: string): void { - this.notifications.delete(id); - } - /** Mirrors the real service: a dismissal is remembered, not forgotten. */ - dismissNotification(id: string): void { - if (this.notifications.has(id)) { - this._onDidDismiss.fire(id); - } - } - getActiveNotification(): IChatInputNotification | undefined { - return [...this.notifications.values()].at(0); - } - handleMessageSent(): void { } - announceRendered(): void { } -} - -/** - * A signed-out user who has opted in, with Claude advertising that it runs on the - * user's own credentials — the one situation the nudge is written for. - */ -function createContribution(store: Pick, storageService = store.add(new InMemoryStorageService())) { - const notificationService = store.add(new TestChatInputNotificationService()); - const claude: IProviderSessionType = { - providerId: 'local-agent-host', - sessionType: { - id: 'claude', - label: 'Claude Code', - icon: Codicon.copilot, - chatSessionType: SessionType.AgentHostClaude, - authRequirement: SessionTypeAuthRequirement.None, - }, - }; - - store.add(new AgentHostDiscoveredConfigNotificationContribution( - notificationService, - new class extends mock() { - override readonly onDidChangeSessionTypes = Event.None; - override getAllProviderSessionTypes(): IProviderSessionType[] { return [claude]; } - }(), - new class extends mock() { - override readonly onDidChangeDefaultAccount = Event.None; - override readonly currentDefaultAccount = null; - override getDefaultAccount() { return Promise.resolve(null); } - }(), - new TestConfigurationService({ [AgentHostAllowSignedOutWhenUsableSettingId]: true }), - storageService, - )); - - return { notificationService, storageService }; -} - -suite('AgentHostDiscoveredConfigNotification', () => { - const store = ensureNoDisposablesAreLeakedInTestSuite(); - - (isWeb ? test.skip : test)('nudges the signed-out user, with dismissal as the only off switch', async () => { - const { notificationService } = createContribution(store); - - // The account resolves asynchronously; the nudge waits for it. - await timeout(0); - - assert.deepStrictEqual([...notificationService.notifications.values()].map(notification => ({ - message: notification.message, - actions: notification.actions.map(action => ({ label: action.label, keepOpen: action.keepOpen })), - dismissible: notification.dismissible, - mute: notification.mute, - sessionTypes: notification.sessionTypes, - })), [{ - message: 'We\'ve discovered your existing Claude Code configuration.', - // `keepOpen` so a sign-in the user then cancels doesn't silence the nudge. - actions: [{ label: 'Sign in to GitHub', keepOpen: true }], - dismissible: true, - mute: undefined, - sessionTypes: [SessionType.AgentHostClaude], - }]); - }); - - (isWeb ? test.skip : test)('dismissing it silences the nudge on this machine for good', async () => { - const storageService = store.add(new InMemoryStorageService()); - const first = createContribution(store, storageService); - await timeout(0); - const notification = first.notificationService.getActiveNotification(); - - first.notificationService.dismissNotification(notification!.id); - - // A fresh contribution stands in for the next window on this machine. - const next = createContribution(store, storageService); - await timeout(0); - - assert.deepStrictEqual({ - afterDismissal: first.notificationService.notifications.size, - nextWindow: next.notificationService.notifications.size, - }, { - afterDismissal: 0, - nextWindow: 0, - }); - }); - - (isWeb ? test : test.skip)('does not nudge on web when signed-out operation is configured', async () => { - const { notificationService } = createContribution(store); - - await timeout(0); - - assert.strictEqual(notificationService.notifications.size, 0); - }); -}); diff --git a/src/vs/sessions/test/browser/sessionsAuthGate.test.ts b/src/vs/sessions/test/browser/sessionsAuthGate.test.ts index c53568bd9c038..42e554457d272 100644 --- a/src/vs/sessions/test/browser/sessionsAuthGate.test.ts +++ b/src/vs/sessions/test/browser/sessionsAuthGate.test.ts @@ -5,7 +5,7 @@ import assert from 'assert'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../base/test/common/utils.js'; -import { ConditionalAuthState, conditionalAuthState, resolveSignedOutWindowGate, shouldShowDiscoveredConfigNudge, shouldShowGitHubWorkspaceGroupSignIn, SignedOutWindowGate } from '../../browser/sessionsAuthGate.js'; +import { ConditionalAuthState, conditionalAuthState, resolveSignedOutWindowGate, shouldShowGitHubWorkspaceGroupSignIn, SignedOutWindowGate } from '../../browser/sessionsAuthGate.js'; import { SessionTypeAuthRequirement } from '../../services/sessions/common/session.js'; suite('Sessions - Auth Gate', () => { @@ -57,48 +57,4 @@ suite('Sessions - Auth Gate', () => { ConditionalAuthState.SignedIn, ]); }); - - test('shows the discovered-config nudge only when signed out, opted in, the type is usable without GitHub, and not muted', () => { - // Independent source of truth: the nudge is the calm inverse of the gate — - // it appears iff the user is signed out AND the opt-in is on AND that type - // is usable without GitHub AND the user has not muted it. Of all 16 input - // combinations only one satisfies every condition. - const cases = [ - { signedIn: true, allowSignedOutWhenUsable: false, usableWithoutGitHub: false, muted: false }, - { signedIn: true, allowSignedOutWhenUsable: false, usableWithoutGitHub: false, muted: true }, - { signedIn: true, allowSignedOutWhenUsable: false, usableWithoutGitHub: true, muted: false }, - { signedIn: true, allowSignedOutWhenUsable: false, usableWithoutGitHub: true, muted: true }, - { signedIn: true, allowSignedOutWhenUsable: true, usableWithoutGitHub: false, muted: false }, - { signedIn: true, allowSignedOutWhenUsable: true, usableWithoutGitHub: false, muted: true }, - { signedIn: true, allowSignedOutWhenUsable: true, usableWithoutGitHub: true, muted: false }, - { signedIn: true, allowSignedOutWhenUsable: true, usableWithoutGitHub: true, muted: true }, - { signedIn: false, allowSignedOutWhenUsable: false, usableWithoutGitHub: false, muted: false }, - { signedIn: false, allowSignedOutWhenUsable: false, usableWithoutGitHub: false, muted: true }, - { signedIn: false, allowSignedOutWhenUsable: false, usableWithoutGitHub: true, muted: false }, - { signedIn: false, allowSignedOutWhenUsable: false, usableWithoutGitHub: true, muted: true }, - { signedIn: false, allowSignedOutWhenUsable: true, usableWithoutGitHub: false, muted: false }, - { signedIn: false, allowSignedOutWhenUsable: true, usableWithoutGitHub: false, muted: true }, - { signedIn: false, allowSignedOutWhenUsable: true, usableWithoutGitHub: true, muted: false }, - { signedIn: false, allowSignedOutWhenUsable: true, usableWithoutGitHub: true, muted: true }, - ]; - - assert.deepStrictEqual(cases.map(shouldShowDiscoveredConfigNudge), [ - false, - false, - false, - false, - false, - false, - false, - false, - false, - false, - false, - false, - false, - false, - true, - false, - ]); - }); }); diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHost.contribution.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHost.contribution.ts index a7816e4d81e08..50c6520f3e078 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHost.contribution.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHost.contribution.ts @@ -23,6 +23,7 @@ import { AgentHostContribution } from './agentHostChatContribution.js'; import { AgentHostCopilotCliSettingsContribution } from './agentHostCopilotCliSettingsContribution.js'; import { AgentHostOpenSessionLinkOpenerContribution } from './openSessionLinkOpener.contribution.js'; import { AgentHostSessionListContribution } from './agentHostSessionListContribution.js'; +import { AgentHostSdkSetupNotificationContribution } from './agentHostSdkSetupNotification.js'; import { AgentHostSignedOutModelsNotificationContribution } from './agentHostSignedOutModelsNotification.js'; import { AgentHostTerminalContribution } from './agentHostTerminalContribution.js'; import { CopilotConfigSlashSubmitHandlerContribution } from './copilotConfigSlashSubmitHandler.js'; @@ -37,5 +38,6 @@ registerWorkbenchContribution2(AgentHostTerminalContribution.ID, AgentHostTermin registerWorkbenchContribution2(AgentHostCopilotCliSettingsContribution.ID, AgentHostCopilotCliSettingsContribution, WorkbenchPhase.AfterRestored); registerWorkbenchContribution2(AgentHostAllowSignedOutWhenUsableContribution.ID, AgentHostAllowSignedOutWhenUsableContribution, WorkbenchPhase.AfterRestored); registerWorkbenchContribution2(AgentHostSignedOutModelsNotificationContribution.ID, AgentHostSignedOutModelsNotificationContribution, WorkbenchPhase.AfterRestored); +registerWorkbenchContribution2(AgentHostSdkSetupNotificationContribution.ID, AgentHostSdkSetupNotificationContribution, WorkbenchPhase.AfterRestored); registerSingleton(IAgentHostByokLmHandler, AgentHostByokLmHandler, InstantiationType.Delayed); diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSdkSetupNotification.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSdkSetupNotification.ts new file mode 100644 index 0000000000000..9f941fb82b01d --- /dev/null +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSdkSetupNotification.ts @@ -0,0 +1,339 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Disposable, DisposableStore } from '../../../../../../base/common/lifecycle.js'; +import { Event } from '../../../../../../base/common/event.js'; +import { localize } from '../../../../../../nls.js'; +import { AgentHostAllowSignedOutWhenUsableSettingId, IAgentHostService } from '../../../../../../platform/agentHost/common/agentService.js'; +import { LOCAL_AGENT_HOST_SCHEME_PREFIX } from '../../../../../../platform/agentHost/common/agentHostConnectionsService.js'; +import type { AgentSdkDownloadStatus, IAgentSdkSetupInfo } from '../../../../../../platform/agentHost/common/agentSdkSetup.js'; +import type { RootState } from '../../../../../../platform/agentHost/common/state/protocol/state.js'; +import { CommandsRegistry } from '../../../../../../platform/commands/common/commands.js'; +import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; +import { IDefaultAccountService } from '../../../../../../platform/defaultAccount/common/defaultAccount.js'; +import { ServicesAccessor } from '../../../../../../platform/instantiation/common/instantiation.js'; +import { IWorkbenchContribution } from '../../../../../common/contributions.js'; +import { IAgentSdkSetupService, type AgentSdkSetupState } from '../../../../../services/agentHost/browser/agentSdkSetupService.js'; +import { ChatEntitlement, IChatEntitlementService } from '../../../../../services/chat/common/chatEntitlementService.js'; +import { hasAnyModelTargetingSessionType } from '../sessionTypeAvailability.js'; +import { ChatInputNotificationActionKind, ChatInputNotificationSeverity, IChatInputNotification, IChatInputNotificationAction, IChatInputNotificationService, isChatInputNotificationApplicableToSessionType } from '../../widget/input/chatInputNotificationService.js'; +import { ILanguageModelsService } from '../../../common/languageModels.js'; + +// #region State + +/** Everything one agent's {@link AgentSdkSetupState} is decided from. */ +export interface IAgentSdkSetupStateInputs { + /** The experimentation flag this whole feature stays behind. */ + readonly allowSignedOutWhenUsable: boolean; + /** Whether the user is signed in to GitHub (Copilot models already work). */ + readonly signedIn: boolean; + /** Whether entitlement has settled; before that "signed out" is not yet a fact. */ + readonly entitlementResolved: boolean; + readonly download: AgentSdkDownloadStatus; + /** Whether a fetch has been asked for and the host has not answered yet. */ + readonly downloadRequested: boolean; + /** Whether this agent has published any model — its own report of "I found an account". */ + readonly hasModels: boolean; +} + +/** + * The whole decision, as one pure function: what the banner renders and what the + * funnel records are two readings of this one state. A signed-in user already + * has Copilot models, so there is nothing to offer and BYOK stays undiscoverable + * for them (a deliberate v1 cut). + */ +export function getAgentSdkSetupState(inputs: IAgentSdkSetupStateInputs): AgentSdkSetupState | undefined { + if (!inputs.allowSignedOutWhenUsable || !inputs.entitlementResolved || inputs.signedIn) { + return undefined; + } + // Ahead of the download status because models are the honest end state: an + // agent that can enumerate a catalog has an account, whatever a status claims. + if (inputs.hasModels) { + return 'resolved'; + } + switch (inputs.download) { + // A fetch in flight has nothing to ask for — the host drives its own + // progress notification while it runs. + case 'downloading': return undefined; + // A request we sent covers the gap before the host answers it, so standing + // consent (or a click) never flashes the offer it has already satisfied. + case 'notDownloaded': return inputs.downloadRequested ? undefined : 'downloadOffered'; + case 'ready': return 'noAccount'; + } +} + +/** + * The state worth reporting to the funnel, or `undefined` when it adds + * nothing to what was last reported for this agent — `_update()` re-runs on every + * model, entitlement and root-state change. Comparing against the last *reported* + * state also counts each step once per user: a download that fails back to the + * offer is the same person still being asked. + */ +export function getAgentSdkSetupStateToReport(previous: AgentSdkSetupState | undefined, state: AgentSdkSetupState | undefined): AgentSdkSetupState | undefined { + // Reaching `resolved` without ever being asked for anything is a user who was + // set up before this feature saw them, not one it converted. + if (state === undefined || state === previous || (state === 'resolved' && previous === undefined)) { + return undefined; + } + return state; +} + +// #endregion + +// #region Banner + +/** + * The "no account" second line: one whole sentence per combination of routes, + * never assembled from localized fragments, because clause order is not stable + * across languages. The GitHub clause is unconditional — every agent behind this + * banner reaches models through our Copilot proxy once signed in, which is + * workbench knowledge rather than something an agent could declare. + */ +function noAccountDescription(setup: IAgentSdkSetupInfo, displayName: string): string { + const provider = setup.signInProviderName; + if (provider && setup.setupDocsUrl) { + return localize('agentHost.sdkSetup.noAccountDescription.all', "Sign in to GitHub to use GitHub Copilot models, sign in to {0} to use your {0} subscription, or read the instructions for other ways to set up {1}.", provider, displayName); + } + if (provider) { + return localize('agentHost.sdkSetup.noAccountDescription.signIn', "Sign in to GitHub to use GitHub Copilot models, or sign in to {0} to use your {0} subscription.", provider); + } + if (setup.setupDocsUrl) { + return localize('agentHost.sdkSetup.noAccountDescription.docs', "Sign in to GitHub to use GitHub Copilot models, or read the instructions for other ways to set up {0}.", displayName); + } + return localize('agentHost.sdkSetup.noAccountDescription', "Sign in to GitHub to use GitHub Copilot models."); +} + +/** + * The session type an agent's sessions run under, derived the same way + * `AgentHostChatContribution` derives it — so agent #3 needs no edit here. + */ +export function agentSdkSetupSessionType(agent: string): string { + return `${LOCAL_AGENT_HOST_SCHEME_PREFIX}${agent}`; +} + +/** + * Each agent's own display name, keyed by provider id. Taken from root state + * rather than the setup channel: the host describes every agent there already, + * and a second wire source for one string would be free to disagree. Templating + * is also what keeps user-facing text out of the host — what crosses the wire is + * a proper noun the workbench cannot invent. + */ +export function getAgentDisplayNames(state: RootState | Error | undefined): ReadonlyMap { + const names = new Map(); + if (!state || state instanceof Error) { + return names; + } + for (const agent of state.agents ?? []) { + if (agent.displayName) { + names.set(agent.provider, agent.displayName); + } + } + return names; +} + +const AGENT_SDK_SETUP_NOTIFICATION_ID_PREFIX = 'agentHost.sdkSetup.'; + +export function agentSdkSetupNotificationId(agent: string): string { + return `${AGENT_SDK_SETUP_NOTIFICATION_ID_PREFIX}${agent}`; +} + +/** + * Whether a setup banner is currently being offered for the given session type. + * + * The pickers ask because the banner lives *inside* a session of the type it is + * scoped to: a harness with no models yet is greyed out by the ordinary + * availability rule, hiding the one thing telling the user how to fix that. + * Matching the setup id specifically matters — an unscoped notification (a quota + * warning, say) applies to every type and would un-grey all of them. + */ +export function hasAgentSdkSetupNotification(chatInputNotificationService: IChatInputNotificationService, sessionType: string): boolean { + return chatInputNotificationService.getActiveNotification(notification => + notification.id.startsWith(AGENT_SDK_SETUP_NOTIFICATION_ID_PREFIX) + && isChatInputNotificationApplicableToSessionType(notification, sessionType) + ) !== undefined; +} + +/** + * Render one agent's banner, or `undefined` when it has nothing to say. + * + * Every string is a template this layer owns, filled with the proper nouns the + * agent declared (`displayName`, `signInProviderName`) and varied by the routes + * it offers — nothing a person reads crosses the wire. The download lines + * never tie the SDK to an account: it is the same SDK behind the Copilot proxy, + * a subscription or a BYO key. + */ +export function createAgentSdkSetupNotification(setup: IAgentSdkSetupInfo, displayName: string, state: AgentSdkSetupState | undefined): IChatInputNotification | undefined { + // Nothing to ask of a user who is already set up. An empty `displayName` means + // the host has not described this agent yet, and "Download the Agent" is worse + // than none; the next root-state change is moments away. + if (!displayName || state === undefined || state === 'resolved') { + return undefined; + } + const base = { + id: agentSdkSetupNotificationId(setup.agent), + severity: ChatInputNotificationSeverity.Info, + dismissible: false, + autoDismissOnMessage: false, + sessionTypes: [agentSdkSetupSessionType(setup.agent)], + } as const; + const action = (label: string, commandId: string): IChatInputNotificationAction => ({ + kind: ChatInputNotificationActionKind.Command, + label, + commandId, + commandArgs: [setup.agent], + keepOpen: true, + }); + if (state === 'downloadOffered') { + return { + ...base, + message: localize('agentHost.sdkSetup.download', "Download the {0} Agent", displayName), + description: localize('agentHost.sdkSetup.downloadDescription', "To use the {0} Agent, we need to download the {0} Agent SDK.", displayName), + actions: [action(localize('agentHost.sdkSetup.downloadAction', "Download"), AGENT_SDK_SETUP_DOWNLOAD_COMMAND_ID)], + }; + } + const actions: IChatInputNotificationAction[] = []; + if (setup.setupDocsUrl) { + actions.push(action(localize('agentHost.sdkSetup.docsAction', "Setup Instructions"), AGENT_SDK_SETUP_OPEN_DOCS_COMMAND_ID)); + } + if (setup.signInProviderName) { + actions.push(action(localize('agentHost.sdkSetup.signInAction', "Sign in to {0}", setup.signInProviderName), AGENT_SDK_SETUP_SIGN_IN_COMMAND_ID)); + } + // Last, because the widget styles the final action as the primary button and + // this is the route that works whatever the user has set up elsewhere. + actions.push(action(localize('agentHost.sdkSetup.gitHubSignInAction', "Sign in to GitHub"), AGENT_SDK_SETUP_GITHUB_SIGN_IN_COMMAND_ID)); + return { + ...base, + message: localize('agentHost.sdkSetup.noAccount', "Choose how you want to use {0}.", displayName), + description: noAccountDescription(setup, displayName), + actions, + }; +} + +// #endregion + +// #region Commands + +export const AGENT_SDK_SETUP_DOWNLOAD_COMMAND_ID = 'workbench.action.chat.agentHost.downloadAgentSdk'; +export const AGENT_SDK_SETUP_OPEN_DOCS_COMMAND_ID = 'workbench.action.chat.agentHost.openAgentSetupDocs'; +export const AGENT_SDK_SETUP_GITHUB_SIGN_IN_COMMAND_ID = 'workbench.action.chat.agentHost.signInToGitHubForAgent'; +export const AGENT_SDK_SETUP_SIGN_IN_COMMAND_ID = 'workbench.action.chat.agentHost.signInToAgent'; + +/** + * The banner's buttons. Commands rather than inline handlers because + * {@link IChatInputNotification} actions address commands by id, and each takes + * the agent id and nothing else — what a route needs beyond that is resolved by + * the service from the agent's own declaration, not from the banner's copy. + */ +function registerAgentSdkSetupCommand(id: string, run: (setupService: IAgentSdkSetupService, agent: string) => void): void { + CommandsRegistry.registerCommand(id, (accessor: ServicesAccessor, agent: unknown) => { + if (typeof agent === 'string') { + run(accessor.get(IAgentSdkSetupService), agent); + } + }); +} + +registerAgentSdkSetupCommand(AGENT_SDK_SETUP_DOWNLOAD_COMMAND_ID, (setupService, agent) => setupService.requestDownload(agent)); +registerAgentSdkSetupCommand(AGENT_SDK_SETUP_OPEN_DOCS_COMMAND_ID, (setupService, agent) => setupService.openSetupDocs(agent)); +registerAgentSdkSetupCommand(AGENT_SDK_SETUP_GITHUB_SIGN_IN_COMMAND_ID, (setupService, agent) => setupService.signInToGitHub(agent)); +registerAgentSdkSetupCommand(AGENT_SDK_SETUP_SIGN_IN_COMMAND_ID, (setupService, agent) => setupService.signIn(agent)); + +// #endregion + +/** + * Offers the SDK download, and explains a missing account once it is on disk, + * for every agent whose setup lives outside the app. + * + * Sibling to `AgentHostSignedOutModelsNotification`, which stays Copilot-scoped + * — these are different asks aimed at different people and share only the + * notification machinery. + */ +export class AgentHostSdkSetupNotificationContribution extends Disposable implements IWorkbenchContribution { + + static readonly ID = 'workbench.contrib.agentHostSdkSetupNotification'; + + /** Pushed notification content by id, so an unchanged answer is not re-pushed (which would clear a dismissal and re-announce). */ + private readonly _shown = new Map(); + + /** Last state reported per agent, so a re-render is not a second event. */ + private readonly _lastReported = new Map(); + + constructor( + @IChatInputNotificationService private readonly _chatInputNotificationService: IChatInputNotificationService, + @IAgentSdkSetupService private readonly _agentSdkSetupService: IAgentSdkSetupService, + @IDefaultAccountService private readonly _defaultAccountService: IDefaultAccountService, + @ILanguageModelsService private readonly _languageModelsService: ILanguageModelsService, + @IConfigurationService private readonly _configurationService: IConfigurationService, + @IChatEntitlementService private readonly _chatEntitlementService: IChatEntitlementService, + @IAgentHostService private readonly _agentHostService: IAgentHostService, + ) { + super(); + this._register(Event.any( + this._agentSdkSetupService.onDidChangeSetups, + this._chatEntitlementService.onDidChangeEntitlement, + this._defaultAccountService.onDidChangeDefaultAccount, + this._languageModelsService.onDidChangeLanguageModels, + Event.filter(this._configurationService.onDidChangeConfiguration, event => event.affectsConfiguration(AgentHostAllowSignedOutWhenUsableSettingId)), + )(() => this._update())); + // The host restarts (and a remote reconnects) behind a fresh root state, so + // re-bind rather than holding one subscription for the window's lifetime. + const rootStateListeners = this._register(new DisposableStore()); + const bindRootState = () => { + rootStateListeners.clear(); + rootStateListeners.add(this._agentHostService.rootState.onDidChange(() => this._update())); + this._update(); + }; + bindRootState(); + this._register(this._agentHostService.onAgentHostStart(bindRootState)); + } + + private _update(): void { + const allowSignedOutWhenUsable = this._configurationService.getValue(AgentHostAllowSignedOutWhenUsableSettingId) === true; + const entitlement = this._chatEntitlementService.entitlement; + const entitlementResolved = entitlement !== ChatEntitlement.Unresolved; + const signedIn = this._defaultAccountService.currentDefaultAccount !== null + || (entitlementResolved && entitlement !== ChatEntitlement.Unknown); + const displayNames = getAgentDisplayNames(this._agentHostService.rootState.value); + const stale = new Set(this._shown.keys()); + for (const setup of this._agentSdkSetupService.setups) { + // An agent can publish its setup status before root state lists it, so a + // missing name here means "not yet", not "never" — and every root-state + // change re-runs this. + const displayName = displayNames.get(setup.agent); + if (!displayName) { + continue; + } + const state = getAgentSdkSetupState({ + allowSignedOutWhenUsable, + signedIn, + entitlementResolved, + download: setup.download, + downloadRequested: this._agentSdkSetupService.isDownloadPending(setup.agent), + hasModels: hasAnyModelTargetingSessionType(this._languageModelsService, agentSdkSetupSessionType(setup.agent)), + }); + // Before the render decision below, because `resolved` — the step the + // funnel exists to count — is exactly the state that renders nothing. + const toReport = getAgentSdkSetupStateToReport(this._lastReported.get(setup.agent), state); + if (toReport) { + this._lastReported.set(setup.agent, toReport); + this._agentSdkSetupService.reportSetupState(setup.agent, toReport); + } + const notification = createAgentSdkSetupNotification(setup, displayName, state); + if (!notification) { + continue; + } + stale.delete(notification.id); + const signature = JSON.stringify(notification); + if (this._shown.get(notification.id) === signature) { + continue; + } + this._shown.set(notification.id, signature); + this._chatInputNotificationService.setNotification(notification); + } + for (const id of stale) { + this._shown.delete(id); + this._chatInputNotificationService.deleteNotification(id); + } + } +} diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/sessionTypeAvailability.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/sessionTypeAvailability.ts index 4b2aa3d18886d..b06795a42a16d 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/sessionTypeAvailability.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/sessionTypeAvailability.ts @@ -24,10 +24,28 @@ export enum SessionTypeAvailability { NoModels, } -export function getSessionTypePickerAvailability(type: string, availability: SessionTypeAvailability, allowSignedOutWhenUsable: boolean): SessionTypeAvailability { - return allowSignedOutWhenUsable && type === SessionType.AgentHostCopilot && availability === SessionTypeAvailability.SignInRequired - ? SessionTypeAvailability.Available - : availability; +/** + * The picker's view of {@link getSessionTypeAvailability}, which keeps a harness + * selectable in the two cases where the raw answer would grey out something the + * user can still act on. + * + * `hasSetupBanner` is the second: a harness whose SDK setup banner is on offer + * has no models *yet*, and the banner saying how to fix that renders inside a + * session of that very type. Not a static allow-list of session types — a + * signed-in user whose Claude harness has no models gets no banner and stays + * greyed out, which is the honest answer for them. + */ +export function getSessionTypePickerAvailability(type: string, availability: SessionTypeAvailability, allowSignedOutWhenUsable: boolean, hasSetupBanner: boolean): SessionTypeAvailability { + if (!allowSignedOutWhenUsable) { + return availability; + } + if (type === SessionType.AgentHostCopilot && availability === SessionTypeAvailability.SignInRequired) { + return SessionTypeAvailability.Available; + } + if (hasSetupBanner && availability === SessionTypeAvailability.NoModels) { + return SessionTypeAvailability.Available; + } + return availability; } /** @@ -71,7 +89,7 @@ export function getSessionTypeAvailability( return SessionTypeAvailability.Available; } const entitlement = chatEntitlementService.entitlement; - const hasTargetedModels = hasModelsTargetingSessionType(languageModelsService, type); + const hasTargetedModels = hasAnyModelTargetingSessionType(languageModelsService, type); const hasVisibleByokModels = allowSignedOutWhenUsable && chatEntitlementService.clientByokEnabled && hasVisibleByokModelsTargetingSessionType(languageModelsService, type); // A visible Agent Host BYOK model can run without a Copilot account. if (entitlement === ChatEntitlement.Unknown && !chatEntitlementService.anonymous && chatSessionsService.requiresCopilotSignInForSessionType(type) && !hasVisibleByokModels) { @@ -100,7 +118,7 @@ export function getSessionTypeAvailability( * type (e.g. a user-configured BYOK model). General-pool models are ignored * since a session type that requires its own models cannot use them. */ -function hasModelsTargetingSessionType(languageModelsService: ILanguageModelsService, type: string): boolean { +export function hasAnyModelTargetingSessionType(languageModelsService: ILanguageModelsService, type: string): boolean { return languageModelsService.getLanguageModelIds().some(id => { const metadata = languageModelsService.lookupLanguageModel(id); return metadata?.targetChatSessionType === type; diff --git a/src/vs/workbench/contrib/chat/browser/widget/input/delegationSessionPickerActionItem.ts b/src/vs/workbench/contrib/chat/browser/widget/input/delegationSessionPickerActionItem.ts index b156bebc3233c..3ebd29d0d0372 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/input/delegationSessionPickerActionItem.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/input/delegationSessionPickerActionItem.ts @@ -27,6 +27,7 @@ import { ACTION_ID_NEW_CHAT } from '../../actions/chatActions.js'; import { AgentSessionProviders, AgentSessionTarget, getAgentCanContinueIn, getAgentSessionProvider, isAgentHostTarget, isFirstPartyAgentSessionProvider } from '../../agentSessions/agentSessions.js'; import { ISessionTypePickerDelegate } from '../../chat.js'; import { IChatInputPickerOptions } from './chatInputPickerActionItem.js'; +import { IChatInputNotificationService } from './chatInputNotificationService.js'; import { ISessionTypeItem, SessionTypePickerActionItem } from './sessionTargetPickerActionItem.js'; import { IGitService } from '../../../../git/common/gitService.js'; @@ -54,9 +55,10 @@ export class DelegationSessionPickerActionItem extends SessionTypePickerActionIt @IStorageService storageService: IStorageService, @IWorkspaceContextService workspaceContextService: IWorkspaceContextService, @IAgentHostEnablementService agentHostEnablementService: IAgentHostEnablementService, + @IChatInputNotificationService chatInputNotificationService: IChatInputNotificationService, @IGitService private readonly gitService: IGitService, ) { - super(action, chatSessionPosition, delegate, pickerOptions, actionWidgetService, keybindingService, contextKeyService, chatSessionsService, commandService, openerService, telemetryService, chatEntitlementService, languageModelsService, configurationService, storageService, workspaceContextService, agentHostEnablementService); + super(action, chatSessionPosition, delegate, pickerOptions, actionWidgetService, keybindingService, contextKeyService, chatSessionsService, commandService, openerService, telemetryService, chatEntitlementService, languageModelsService, configurationService, storageService, workspaceContextService, agentHostEnablementService, chatInputNotificationService); } protected override _run(sessionTypeItem: ISessionTypeItem): void { diff --git a/src/vs/workbench/contrib/chat/browser/widget/input/sessionTargetPickerActionItem.ts b/src/vs/workbench/contrib/chat/browser/widget/input/sessionTargetPickerActionItem.ts index d97be275daa35..71f0e87cb0e1b 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/input/sessionTargetPickerActionItem.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/input/sessionTargetPickerActionItem.ts @@ -32,6 +32,8 @@ import { IChatSessionsService } from '../../../common/chatSessionsService.js'; import { ILanguageModelsService } from '../../../common/languageModels.js'; import { AgentSessionProviders, AgentSessionTarget, getAgentSessionProvider, getAgentSessionProviderDescription, getAgentSessionProviderIcon, getAgentSessionProviderName, isFirstPartyAgentSessionProvider } from '../../agentSessions/agentSessions.js'; import { getSessionTypeAvailability, getSessionTypePickerAvailability, getSessionTypeUnavailableDescription, getSessionTypeUnavailableHover, SessionTypeAvailability } from '../../agentSessions/sessionTypeAvailability.js'; +import { hasAgentSdkSetupNotification } from '../../agentSessions/agentHost/agentHostSdkSetupNotification.js'; +import { IChatInputNotificationService } from './chatInputNotificationService.js'; import { ChatConfiguration, getDefaultNewChatSessionType, isVisibleEditorChatSessionType, recordUserSelectedSessionType } from '../../../common/constants.js'; import { ChatInputPickerActionViewItem, IChatInputPickerOptions } from './chatInputPickerActionItem.js'; import { ISessionTypePickerDelegate } from '../../chat.js'; @@ -91,12 +93,14 @@ export function getConfiguredSessionTypePickerAvailability( chatSessionsService: IChatSessionsService, chatEntitlementService: IChatEntitlementService, languageModelsService: ILanguageModelsService, + chatInputNotificationService: IChatInputNotificationService, ): SessionTypeAvailability { const allowSignedOutWhenUsable = configurationService.getValue(AgentHostAllowSignedOutWhenUsableSettingId) === true; return getSessionTypePickerAvailability( type, getSessionTypeAvailability(chatSessionsService, chatEntitlementService, languageModelsService, type, allowSignedOutWhenUsable), allowSignedOutWhenUsable, + hasAgentSdkSetupNotification(chatInputNotificationService, type), ); } @@ -126,6 +130,7 @@ export class SessionTypePickerActionItem extends ChatInputPickerActionViewItem { @IStorageService protected readonly storageService: IStorageService, @IWorkspaceContextService private readonly workspaceContextService: IWorkspaceContextService, @IAgentHostEnablementService private readonly agentHostEnablementService: IAgentHostEnablementService, + @IChatInputNotificationService protected readonly chatInputNotificationService: IChatInputNotificationService, ) { const actionProvider: IActionWidgetDropdownActionProvider = { @@ -140,6 +145,7 @@ export class SessionTypePickerActionItem extends ChatInputPickerActionViewItem { this.chatSessionsService, this.chatEntitlementService, this.languageModelsService, + this.chatInputNotificationService, ); actions.push(createSessionTypePickerAction( action, diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostSdkSetupNotification.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostSdkSetupNotification.test.ts new file mode 100644 index 0000000000000..ccf3311fbacdb --- /dev/null +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostSdkSetupNotification.test.ts @@ -0,0 +1,244 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { mock } from '../../../../../../base/test/common/mock.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; +import type { IAgentSdkSetupInfo } from '../../../../../../platform/agentHost/common/agentSdkSetup.js'; +import { AGENT_SDK_SETUP_DOWNLOAD_COMMAND_ID, AGENT_SDK_SETUP_GITHUB_SIGN_IN_COMMAND_ID, AGENT_SDK_SETUP_OPEN_DOCS_COMMAND_ID, AGENT_SDK_SETUP_SIGN_IN_COMMAND_ID, agentSdkSetupNotificationId, createAgentSdkSetupNotification, getAgentDisplayNames, getAgentSdkSetupState, getAgentSdkSetupStateToReport, hasAgentSdkSetupNotification, type IAgentSdkSetupStateInputs } from '../../../browser/agentSessions/agentHost/agentHostSdkSetupNotification.js'; +import type { AgentSdkSetupState } from '../../../../../services/agentHost/browser/agentSdkSetupService.js'; +import { ChatInputNotificationActionKind, ChatInputNotificationSeverity, type IChatInputNotification, type IChatInputNotificationAction, type IChatInputNotificationService } from '../../../browser/widget/input/chatInputNotificationService.js'; +import { SessionType } from '../../../common/chatSessionsService.js'; + +/** Signed out, flag on, entitlement settled, SDK missing — the case this feature exists for. */ +const BLOCKED_USER: IAgentSdkSetupStateInputs = { + allowSignedOutWhenUsable: true, + signedIn: false, + entitlementResolved: true, + download: 'notDownloaded', + downloadRequested: false, + hasModels: false, +}; + +function commandIds(actions: readonly IChatInputNotificationAction[]): string[] { + return actions.map(action => action.kind === ChatInputNotificationActionKind.Command ? action.commandId : action.kind); +} + +suite('Agent SDK setup banner', () => { + ensureNoDisposablesAreLeakedInTestSuite(); + + suite('state', () => { + const cases: readonly { readonly name: string; readonly inputs: IAgentSdkSetupStateInputs; readonly expected: AgentSdkSetupState | undefined }[] = [ + { name: 'signed-out user with no SDK is offered the download', inputs: BLOCKED_USER, expected: 'downloadOffered' }, + { name: 'a fetch in flight has nothing to ask for, since the host shows its own progress', inputs: { ...BLOCKED_USER, download: 'downloading' }, expected: undefined }, + // The host answers a download request over IPC, so it keeps saying + // `notDownloaded` for a moment after we ask. Offering the button again in + // that gap would re-ask a user who has already consented. + { name: 'a request the host has not answered yet is not a fresh offer', inputs: { ...BLOCKED_USER, downloadRequested: true }, expected: undefined }, + { name: 'SDK on disk reporting no models means no account', inputs: { ...BLOCKED_USER, download: 'ready' }, expected: 'noAccount' }, + { name: 'models are the honest end state, whatever the status says', inputs: { ...BLOCKED_USER, download: 'ready', hasModels: true }, expected: 'resolved' }, + { name: 'a signed-in user already has Copilot models', inputs: { ...BLOCKED_USER, signedIn: true }, expected: undefined }, + { name: 'nothing shows until entitlement settles, since "signed out" is not yet a fact', inputs: { ...BLOCKED_USER, entitlementResolved: false }, expected: undefined }, + { name: 'the whole feature stays behind its flag', inputs: { ...BLOCKED_USER, allowSignedOutWhenUsable: false }, expected: undefined }, + { name: 'a signed-in user mid-download is still shown nothing', inputs: { ...BLOCKED_USER, signedIn: true, download: 'downloading' }, expected: undefined }, + ]; + + for (const { name, inputs, expected } of cases) { + test(name, () => { + assert.strictEqual(getAgentSdkSetupState(inputs), expected); + }); + } + }); + + suite('presentation', () => { + const claude: IAgentSdkSetupInfo = { agent: 'claude', download: 'notDownloaded', setupDocsUrl: 'https://example.test/claude' }; + + test('the download offer names the SDK, explains it, and carries a single Download button', () => { + const notification = createAgentSdkSetupNotification(claude, 'Claude', 'downloadOffered'); + + assert.ok(notification); + assert.strictEqual(notification.id, agentSdkSetupNotificationId('claude')); + assert.deepStrictEqual(notification.sessionTypes, [SessionType.AgentHostClaude]); + assert.strictEqual(notification.message, 'Download the Claude Agent'); + // An ask that expects a decision explains itself, and does so without + // tying the SDK to an account: the same download serves the Copilot + // proxy, a Claude subscription and a BYO key alike. + assert.strictEqual(notification.description, 'To use the Claude Agent, we need to download the Claude Agent SDK.'); + assert.deepStrictEqual(commandIds(notification.actions), [AGENT_SDK_SETUP_DOWNLOAD_COMMAND_ID]); + assert.deepStrictEqual(notification.actions[0].kind === ChatInputNotificationActionKind.Command ? notification.actions[0].commandArgs : undefined, ['claude']); + }); + + test('every noun comes from the agent, so a second agent needs no entry here', () => { + const codex: IAgentSdkSetupInfo = { agent: 'codex', download: 'notDownloaded', signInProviderName: 'ChatGPT' }; + + assert.deepStrictEqual({ + sessionTypes: createAgentSdkSetupNotification(codex, 'Codex', 'downloadOffered')?.sessionTypes, + download: createAgentSdkSetupNotification(codex, 'Codex', 'downloadOffered')?.message, + noAccount: createAgentSdkSetupNotification(codex, 'Codex', 'noAccount')?.message, + }, { + sessionTypes: [SessionType.AgentHostCodex], + download: 'Download the Codex Agent', + noAccount: 'Choose how you want to use Codex.', + }); + }); + + test('a missing account offers every route the agent declared, GitHub sign-in last', () => { + // Last is the primary button in the widget, and GitHub is the route that + // works whatever the user has (or has not) set up elsewhere. + const codex: IAgentSdkSetupInfo = { agent: 'codex', download: 'ready', setupDocsUrl: 'https://example.test/codex', signInProviderName: 'ChatGPT' }; + const buttons = (setup: IAgentSdkSetupInfo, displayName: string) => + commandIds(createAgentSdkSetupNotification(setup, displayName, 'noAccount')?.actions ?? []); + + assert.deepStrictEqual({ + docsOnly: buttons({ ...claude, download: 'ready' }, 'Claude'), + signInOnly: buttons({ ...codex, setupDocsUrl: undefined }, 'Codex'), + both: buttons(codex, 'Codex'), + neither: buttons({ agent: 'some-future-agent', download: 'ready' }, 'Future'), + }, { + docsOnly: [AGENT_SDK_SETUP_OPEN_DOCS_COMMAND_ID, AGENT_SDK_SETUP_GITHUB_SIGN_IN_COMMAND_ID], + signInOnly: [AGENT_SDK_SETUP_SIGN_IN_COMMAND_ID, AGENT_SDK_SETUP_GITHUB_SIGN_IN_COMMAND_ID], + both: [AGENT_SDK_SETUP_OPEN_DOCS_COMMAND_ID, AGENT_SDK_SETUP_SIGN_IN_COMMAND_ID, AGENT_SDK_SETUP_GITHUB_SIGN_IN_COMMAND_ID], + neither: [AGENT_SDK_SETUP_GITHUB_SIGN_IN_COMMAND_ID], + }); + }); + + test('every button is addressed to the agent, and the sign-in one is labelled by its provider', () => { + const notification = createAgentSdkSetupNotification({ agent: 'codex', download: 'ready', signInProviderName: 'ChatGPT' }, 'Codex', 'noAccount'); + + assert.ok(notification); + // The agent id, not the URL or the provider: each command resolves what it + // needs from the agent's own declaration rather than trusting the banner. + assert.deepStrictEqual(notification.actions.map(action => action.kind === ChatInputNotificationActionKind.Command ? action.commandArgs : undefined), [['codex'], ['codex']]); + assert.deepStrictEqual(notification.actions.map(action => action.label), ['Sign in to ChatGPT', 'Sign in to GitHub']); + }); + + test('the routes named in the copy are the ones the agent declared', () => { + // One whole sentence per combination rather than joined clauses, since a + // translator reorders them freely. GitHub appears in all four: every agent + // behind this banner reaches models through our proxy once signed in. + const noAccount = (setup: Omit) => + createAgentSdkSetupNotification({ agent: 'claude', download: 'ready', ...setup }, 'Claude', 'noAccount')?.description; + + assert.deepStrictEqual({ + gitHubOnly: noAccount({}), + docs: noAccount({ setupDocsUrl: 'https://example.test/claude' }), + signIn: noAccount({ signInProviderName: 'ChatGPT' }), + both: noAccount({ setupDocsUrl: 'https://example.test/claude', signInProviderName: 'ChatGPT' }), + }, { + gitHubOnly: 'Sign in to GitHub to use GitHub Copilot models.', + docs: 'Sign in to GitHub to use GitHub Copilot models, or read the instructions for other ways to set up Claude.', + signIn: 'Sign in to GitHub to use GitHub Copilot models, or sign in to ChatGPT to use your ChatGPT subscription.', + both: 'Sign in to GitHub to use GitHub Copilot models, sign in to ChatGPT to use your ChatGPT subscription, or read the instructions for other ways to set up Claude.', + }); + }); + + test('the banner cannot be dismissed, since it is the only route to a working agent', () => { + const notification = createAgentSdkSetupNotification(claude, 'Claude', 'downloadOffered'); + + assert.ok(notification); + assert.strictEqual(notification.dismissible, false); + assert.strictEqual(notification.autoDismissOnMessage, false); + }); + + test('nothing is rendered once the user is set up, or for an agent the host has not named yet', () => { + assert.strictEqual(createAgentSdkSetupNotification(claude, 'Claude', undefined), undefined); + assert.strictEqual(createAgentSdkSetupNotification({ ...claude, download: 'ready' }, 'Claude', 'resolved'), undefined); + // "Download the Agent" is worse than no banner; the next root-state + // change carries the name. + assert.strictEqual(createAgentSdkSetupNotification({ agent: 'some-future-agent', download: 'notDownloaded' }, '', 'downloadOffered'), undefined); + }); + }); + + suite('display names', () => { + test('reads each agent name the host published, and skips what it did not', () => { + assert.deepStrictEqual([...getAgentDisplayNames({ + agents: [ + { provider: 'claude', displayName: 'Claude', description: '', models: [] }, + { provider: 'nameless', displayName: '', description: '', models: [] }, + ], + })], [['claude', 'Claude']]); + }); + + test('a host that has not reported, or failed, names nobody', () => { + assert.deepStrictEqual([...getAgentDisplayNames(undefined)], []); + assert.deepStrictEqual([...getAgentDisplayNames(new Error('host is down'))], []); + }); + }); + + suite('reachability', () => { + /** A notification service holding the given notifications, none dismissed. */ + function notificationService(notifications: readonly IChatInputNotification[]): IChatInputNotificationService { + return new class extends mock() { + override getActiveNotification(filter?: (notification: IChatInputNotification) => boolean): IChatInputNotification | undefined { + return notifications.find(notification => !filter || filter(notification)); + } + }(); + } + + function bannersFor(...agents: readonly string[]): readonly IChatInputNotification[] { + return agents.flatMap(agent => { + const notification = createAgentSdkSetupNotification({ agent, download: 'notDownloaded' }, agent, 'downloadOffered'); + return notification ? [notification] : []; + }); + } + + test('a banner is found for the session type it is scoped to, and only that one', () => { + const service = notificationService(bannersFor('claude')); + + assert.deepStrictEqual({ + claude: hasAgentSdkSetupNotification(service, SessionType.AgentHostClaude), + codex: hasAgentSdkSetupNotification(service, SessionType.AgentHostCodex), + copilot: hasAgentSdkSetupNotification(service, SessionType.AgentHostCopilot), + }, { claude: true, codex: false, copilot: false }); + }); + + test('an unscoped notification is not mistaken for a setup banner', () => { + // The session-type filter alone passes a notification with no + // `sessionTypes` — a quota warning applies everywhere — so the id + // carries the "this is a setup ask" bit. + const service = notificationService([{ + id: 'chat.quotaExceeded', + severity: ChatInputNotificationSeverity.Warning, + message: 'Out of quota', + description: undefined, + actions: [], + dismissible: true, + autoDismissOnMessage: false, + }]); + + assert.strictEqual(hasAgentSdkSetupNotification(service, SessionType.AgentHostClaude), false); + }); + + test('nothing on offer means nothing to reach', () => { + assert.strictEqual(hasAgentSdkSetupNotification(notificationService([]), SessionType.AgentHostClaude), false); + }); + }); + + suite('funnel', () => { + const cases: readonly { + readonly name: string; + /** The last state *reported* for this agent, not the last one computed. */ + readonly previous: AgentSdkSetupState | undefined; + readonly state: AgentSdkSetupState | undefined; + readonly expected: AgentSdkSetupState | undefined; + }[] = [ + { name: 'first sight of the offer counts', previous: undefined, state: 'downloadOffered', expected: 'downloadOffered' }, + { name: 'an SDK that found no account is where users get stuck', previous: 'downloadOffered', state: 'noAccount', expected: 'noAccount' }, + { name: 'a stuck user who then has models is the conversion', previous: 'noAccount', state: 'resolved', expected: 'resolved' }, + // Counted once per user: re-renders are constant, and a download that + // failed back to the offer is the same person still being asked. + { name: 'a re-render, or a failed download returning to the offer, is not a second offer', previous: 'downloadOffered', state: 'downloadOffered', expected: undefined }, + { name: 'a conversion is not re-counted on every later render', previous: 'resolved', state: 'resolved', expected: undefined }, + { name: 'a fetch in flight, or giving up, moves the user nowhere', previous: 'downloadOffered', state: undefined, expected: undefined }, + { name: 'a user this feature was never for is not a convert', previous: undefined, state: 'resolved', expected: undefined }, + ]; + + for (const { name, previous, state, expected } of cases) { + test(name, () => { + assert.strictEqual(getAgentSdkSetupStateToReport(previous, state), expected); + }); + } + }); +}); diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/sessionTypeAvailability.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/sessionTypeAvailability.test.ts index 48fcbb51ff3a3..5faad77b5a52c 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/sessionTypeAvailability.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/sessionTypeAvailability.test.ts @@ -104,7 +104,7 @@ suite('getSessionTypeAvailability', () => { ensureNoDisposablesAreLeakedInTestSuite(); test('Copilot Agent Host remains setup-selectable when signed-out operation is enabled', () => { - const pickerAvailability = (type: string, allowSignedOutWhenUsable: boolean) => getSessionTypePickerAvailability(type, SessionTypeAvailability.SignInRequired, allowSignedOutWhenUsable); + const pickerAvailability = (type: string, allowSignedOutWhenUsable: boolean) => getSessionTypePickerAvailability(type, SessionTypeAvailability.SignInRequired, allowSignedOutWhenUsable, false); assert.deepStrictEqual({ localCopilot: pickerAvailability(SessionType.AgentHostCopilot, true), localClaude: pickerAvailability(SessionType.AgentHostClaude, true), @@ -118,6 +118,37 @@ suite('getSessionTypeAvailability', () => { }); }); + suite('a harness with a setup banner stays selectable', () => { + // The banner renders inside a session of the type it is scoped to, so + // greying the harness out would hide the only route to it. + const pickerAvailability = (availability: SessionTypeAvailability, hasSetupBanner: boolean, allowSignedOutWhenUsable = true) => + getSessionTypePickerAvailability(SessionType.AgentHostClaude, availability, allowSignedOutWhenUsable, hasSetupBanner); + + test('a signed-out user with no Claude models can still pick the harness the banner belongs to', () => { + assert.strictEqual(pickerAvailability(SessionTypeAvailability.NoModels, true), SessionTypeAvailability.Available); + }); + + test('the same harness with no banner stays greyed out, since there is nothing to send the user to', () => { + // e.g. a signed-in user whose Claude harness has no models: the banner + // is deliberately hidden for them, so "No models available" is honest. + assert.strictEqual(pickerAvailability(SessionTypeAvailability.NoModels, false), SessionTypeAvailability.NoModels); + }); + + test('a banner does not unlock a harness the user must sign in or upgrade for', () => { + assert.deepStrictEqual({ + signIn: pickerAvailability(SessionTypeAvailability.SignInRequired, true), + upgrade: pickerAvailability(SessionTypeAvailability.UpgradeRequired, true), + }, { + signIn: SessionTypeAvailability.SignInRequired, + upgrade: SessionTypeAvailability.UpgradeRequired, + }); + }); + + test('the whole override stays behind the signed-out opt-in', () => { + assert.strictEqual(pickerAvailability(SessionTypeAvailability.NoModels, true, false), SessionTypeAvailability.NoModels); + }); + }); + function availability(config: ITypeConfig, entitlement: ChatEntitlement, modelTargets: readonly (string | undefined)[] = [], anonymous = false): SessionTypeAvailability { return getSessionTypeAvailability( createChatSessionsService(config), diff --git a/src/vs/workbench/contrib/chat/test/browser/widget/input/sessionTargetPickerActionItem.test.ts b/src/vs/workbench/contrib/chat/test/browser/widget/input/sessionTargetPickerActionItem.test.ts index 63bffc6542557..4558d92cc9d27 100644 --- a/src/vs/workbench/contrib/chat/test/browser/widget/input/sessionTargetPickerActionItem.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/widget/input/sessionTargetPickerActionItem.test.ts @@ -13,7 +13,9 @@ import { AgentHostAllowSignedOutWhenUsableSettingId } from '../../../../../../.. import { TestConfigurationService } from '../../../../../../../platform/configuration/test/common/testConfigurationService.js'; import { ChatEntitlement, IChatEntitlementService } from '../../../../../../services/chat/common/chatEntitlementService.js'; import { AgentSessionProviders, getAgentSessionProviderDescription } from '../../../../browser/agentSessions/agentSessions.js'; +import { createAgentSdkSetupNotification } from '../../../../browser/agentSessions/agentHost/agentHostSdkSetupNotification.js'; import { SessionTypeAvailability } from '../../../../browser/agentSessions/sessionTypeAvailability.js'; +import { ChatInputNotificationSeverity, IChatInputNotification, IChatInputNotificationService } from '../../../../browser/widget/input/chatInputNotificationService.js'; import { IChatSessionsService, ResolvedChatSessionsExtensionPoint, SessionType } from '../../../../common/chatSessionsService.js'; import { ILanguageModelsService } from '../../../../common/languageModels.js'; import { createSessionTypePickerAction, getConfiguredSessionTypePickerAvailability, ISessionTypeItem } from '../../../../browser/widget/input/sessionTargetPickerActionItem.js'; @@ -40,15 +42,25 @@ function getMarkdownValue(value: string | IMarkdownString | HTMLElement | undefi return typeof value === 'string' ? value : value instanceof HTMLElement ? value.textContent ?? undefined : value?.value; } -function getCopilotAvailability(allowSignedOutWhenUsable: boolean): SessionTypeAvailability { +interface IAvailabilityInputs { + readonly type: string; + readonly allowSignedOutWhenUsable: boolean; + /** Whether the harness is gated on a Copilot account. */ + readonly requiresCopilotSignIn: boolean; + /** Notifications currently on offer, none dismissed. */ + readonly notifications?: readonly IChatInputNotification[]; +} + +/** Availability for a signed-out user whose harness needs its own models and has none. */ +function getSignedOutAvailability({ type, allowSignedOutWhenUsable, requiresCopilotSignIn, notifications = [] }: IAvailabilityInputs): SessionTypeAvailability { const chatSessionsService = new class extends mock() { - override getChatSessionContribution(type: string): ResolvedChatSessionsExtensionPoint | undefined { - return type === SessionType.AgentHostCopilot + override getChatSessionContribution(candidate: string): ResolvedChatSessionsExtensionPoint | undefined { + return candidate === type ? { type, name: type, displayName: type, description: '', icon: undefined } : undefined; } override requiresCopilotSignInForSessionType(): boolean { - return true; + return requiresCopilotSignIn; } override supportsAutoModelForSessionType(): boolean { return false; @@ -73,16 +85,32 @@ function getCopilotAvailability(allowSignedOutWhenUsable: boolean): SessionTypeA return []; } }(); + const notificationService = new class extends mock() { + override getActiveNotification(filter?: (notification: IChatInputNotification) => boolean): IChatInputNotification | undefined { + return notifications.find(notification => !filter || filter(notification)); + } + }(); return getConfiguredSessionTypePickerAvailability( - SessionType.AgentHostCopilot, + type, new TestConfigurationService({ [AgentHostAllowSignedOutWhenUsableSettingId]: allowSignedOutWhenUsable }), chatSessionsService, entitlementService, languageModelsService, + notificationService, ); } +function getCopilotAvailability(allowSignedOutWhenUsable: boolean): SessionTypeAvailability { + return getSignedOutAvailability({ type: SessionType.AgentHostCopilot, allowSignedOutWhenUsable, requiresCopilotSignIn: true }); +} + +/** The real banner, so the test is bound to the ids and session scoping it actually publishes. */ +function claudeSetupBanner(): readonly IChatInputNotification[] { + const notification = createAgentSdkSetupNotification({ agent: 'claude', download: 'notDownloaded' }, 'Claude', 'downloadOffered'); + return notification ? [notification] : []; +} + suite('SessionTypePickerActionItem', () => { ensureNoDisposablesAreLeakedInTestSuite(); @@ -96,6 +124,54 @@ suite('SessionTypePickerActionItem', () => { }); }); + test('a harness whose SDK setup banner is on offer stays selectable, so the banner can be reached', () => { + // The Claude harness no longer requires a Copilot account, so a signed-out + // user with no Claude models lands on "No models available" — and the banner + // telling them how to fix that only renders inside a Claude session. + const claude = (notifications: readonly IChatInputNotification[]) => getSignedOutAvailability({ + type: SessionType.AgentHostClaude, + allowSignedOutWhenUsable: true, + requiresCopilotSignIn: false, + notifications, + }); + + assert.deepStrictEqual({ + withBanner: claude(claudeSetupBanner()), + withoutBanner: claude([]), + }, { + withBanner: SessionTypeAvailability.Available, + withoutBanner: SessionTypeAvailability.NoModels, + }); + }); + + test('another agent\'s setup banner does not unlock this harness', () => { + assert.strictEqual(getSignedOutAvailability({ + type: SessionType.AgentHostCodex, + allowSignedOutWhenUsable: true, + requiresCopilotSignIn: false, + notifications: claudeSetupBanner(), + }), SessionTypeAvailability.NoModels); + }); + + test('an unscoped notification does not unlock a harness that has nothing to offer', () => { + // `getActiveNotification`'s session-type filter passes notifications with no + // `sessionTypes` at all (a quota warning, say) — those must not read as setup. + assert.strictEqual(getSignedOutAvailability({ + type: SessionType.AgentHostClaude, + allowSignedOutWhenUsable: true, + requiresCopilotSignIn: false, + notifications: [{ + id: 'chat.quotaExceeded', + severity: ChatInputNotificationSeverity.Warning, + message: 'Out of quota', + description: undefined, + actions: [], + dismissible: true, + autoDismissOnMessage: false, + }], + }), SessionTypeAvailability.NoModels); + }); + test('creates an available Codex extension action with hover context', () => { const item = createCodexItem(AgentSessionProviders.Codex); const action = createSessionTypePickerAction( diff --git a/src/vs/workbench/services/agentHost/browser/agentSdkSetupService.ts b/src/vs/workbench/services/agentHost/browser/agentSdkSetupService.ts new file mode 100644 index 0000000000000..42cbd99d2ad85 --- /dev/null +++ b/src/vs/workbench/services/agentHost/browser/agentSdkSetupService.ts @@ -0,0 +1,247 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Emitter, Event } from '../../../../base/common/event.js'; +import { Disposable } from '../../../../base/common/lifecycle.js'; +import { generateUuid } from '../../../../base/common/uuid.js'; +import { AGENT_SDK_SETUP_DOWNLOAD_REQUEST_KEY, IAgentSdkSetupInfo, readAgentSdkSetupInfos, readConsentedSdkAgents, resolveConsentedSdkDownloads, writeConsentedSdkAgents } from '../../../../platform/agentHost/common/agentSdkSetup.js'; +import { IAgentHostService } from '../../../../platform/agentHost/common/agentService.js'; +import { ActionType } from '../../../../platform/agentHost/common/state/sessionActions.js'; +import { ROOT_STATE_URI } from '../../../../platform/agentHost/common/state/sessionState.js'; +import { InstantiationType, registerSingleton } from '../../../../platform/instantiation/common/extensions.js'; +import { createDecorator } from '../../../../platform/instantiation/common/instantiation.js'; +import { ICommandService } from '../../../../platform/commands/common/commands.js'; +import { ILogService } from '../../../../platform/log/common/log.js'; +import { IOpenerService } from '../../../../platform/opener/common/opener.js'; +import { IStorageService, StorageScope, StorageTarget } from '../../../../platform/storage/common/storage.js'; +import { ITelemetryService } from '../../../../platform/telemetry/common/telemetry.js'; +import { ICodexAccountService } from './codexAccountService.js'; + +/** + * The agents whose SDK the user has agreed to fetch, each recorded on its own + * first explicit Download. `APPLICATION` + `USER` so it follows the person, not + * the machine — see {@link resolveConsentedSdkDownloads} for why it is neither + * re-asked per version nor shared between agents. + */ +const AGENT_SDK_DOWNLOAD_CONSENT_KEY = 'agentHost.agentSdkDownloadConsent'; + +/** The Copilot sign-in flow, shared with `AgentHostSignedOutModelsNotification`. */ +const CHAT_SETUP_COMMAND_ID = 'workbench.action.chat.triggerSetup'; + +export const IAgentSdkSetupService = createDecorator('agentSdkSetupService'); + +/** + * Where the user stands with one agent's setup: the download is on offer, the + * SDK is on disk and found no account, or the agent has models. Every other + * case — the feature not applying, a fetch in flight — is `undefined`. + */ +export type AgentSdkSetupState = 'downloadOffered' | 'noAccount' | 'resolved'; + +/** + * One step of the setup funnel: `downloadOffered` → a download (clicked, or + * taken under standing consent) → `noAccount` → a route out of it → + * `resolved`, the step that decides whether this was worth building. The states + * are reported by the banner that computes them, the routes by this service. + */ +type AgentSdkSetupFunnelStep = + | AgentSdkSetupState + | 'downloadClicked' + | 'consentedDownload' + | 'docsClicked' + | 'gitHubSignInClicked' + | 'signInClicked'; + +interface IAgentSdkSetupFunnelEvent { + agent: string; + step: string; +} + +type AgentSdkSetupFunnelClassification = { + agent: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'The agent whose setup this step belongs to, e.g. claude or codex.' }; + step: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Which step of the agent SDK setup funnel was reached (downloadOffered, downloadClicked, consentedDownload, noAccount, docsClicked, gitHubSignInClicked, signInClicked, resolved).' }; + owner: 'TylerLeonhardt'; + comment: 'Tracks how far a signed-out user gets through setting up their own Claude or Codex account.'; +}; + +export interface IAgentSdkSetupService { + readonly _serviceBrand: undefined; + + /** Every agent that has published a setup status, newest state. */ + readonly setups: readonly IAgentSdkSetupInfo[]; + readonly onDidChangeSetups: Event; + + /** + * Ask `agent` to fetch its SDK, and record standing consent to do so again + * for later version bumps. + */ + requestDownload(agent: string): void; + + /** Open the setup instructions `agent` published, if it published any. */ + openSetupDocs(agent: string): void; + + /** Start GitHub sign-in, which reaches every agent's models through our proxy. */ + signInToGitHub(agent: string): void; + + /** Start `agent`'s own sign-in flow, if it declared one. */ + signIn(agent: string): void; + + /** + * Whether `agent` has been asked to fetch its SDK and the host has not + * answered yet — already downloading, as far as this window can tell. + */ + isDownloadPending(agent: string): boolean; + + /** + * Record that the user reached `state`. Public because the banner is + * where these three are computed and this service cannot see them; every other + * step is reported by the method that takes it. + */ + reportSetupState(agent: string, state: AgentSdkSetupState): void; +} + +class AgentSdkSetupService extends Disposable implements IAgentSdkSetupService { + declare readonly _serviceBrand: undefined; + + private readonly _onDidChangeSetups = this._register(new Emitter()); + readonly onDidChangeSetups = this._onDidChangeSetups.event; + + private _setups: readonly IAgentSdkSetupInfo[]; + + /** + * Agents whose SDK we have already re-requested under standing consent, so a + * download that fails (and so reports `notDownloaded` again) is retried on the + * next window rather than immediately, forever. + */ + private readonly _consentedRequests = new Set(); + + /** + * Agents we have asked to fetch and the host has not answered yet. Cleared on + * that answer rather than on success, so a failed download — which republishes + * `notDownloaded` after the `downloading` we cleared on — re-offers the button. + */ + private readonly _pendingRequests = new Set(); + + get setups(): readonly IAgentSdkSetupInfo[] { + return this._setups; + } + + constructor( + @IAgentHostService private readonly _agentHostService: IAgentHostService, + @IStorageService private readonly _storageService: IStorageService, + @ITelemetryService private readonly _telemetryService: ITelemetryService, + @ILogService private readonly _logService: ILogService, + @IOpenerService private readonly _openerService: IOpenerService, + @ICommandService private readonly _commandService: ICommandService, + @ICodexAccountService private readonly _codexAccountService: ICodexAccountService, + ) { + super(); + const initialState = this._agentHostService.rootState.value; + this._setups = readAgentSdkSetupInfos(initialState instanceof Error ? undefined : initialState); + this._applyConsent(); + this._register(this._agentHostService.rootState.onDidChange(state => this._updateSetups(readAgentSdkSetupInfos(state)))); + } + + requestDownload(agent: string): void { + const consented = new Set(this._readConsentedAgents()); + consented.add(agent); + this._storageService.store(AGENT_SDK_DOWNLOAD_CONSENT_KEY, writeConsentedSdkAgents(consented), StorageScope.APPLICATION, StorageTarget.USER); + this._consentedRequests.add(agent); + this._reportStep(agent, 'downloadClicked'); + this._dispatchDownloadRequest(agent); + } + + openSetupDocs(agent: string): void { + const url = this._getSetup(agent)?.setupDocsUrl; + if (!url) { + return; + } + this._reportStep(agent, 'docsClicked'); + // The URL is declared by the agent, so it is validated like any other + // externally-supplied link rather than trusted. + void this._openerService.open(url, { openExternal: true }); + } + + signInToGitHub(agent: string): void { + // A thin wrapper over the ordinary Copilot sign-in, taking the agent id only + // to attribute the click — which is the funnel's most telling drop. + this._reportStep(agent, 'gitHubSignInClicked'); + void this._commandService.executeCommand(CHAT_SETUP_COMMAND_ID); + } + + signIn(agent: string): void { + // Codex is the only agent with an in-app sign-in today, and comparing against + // the service's own `agent` rather than a literal keeps `'codex'` out of the + // workbench. A second such agent turns this comparison into a lookup. + if (agent !== this._codexAccountService.agent) { + return; + } + this._reportStep(agent, 'signInClicked'); + this._codexAccountService.signIn(); + } + + reportSetupState(agent: string, state: AgentSdkSetupState): void { + this._reportStep(agent, state); + } + + isDownloadPending(agent: string): boolean { + return this._pendingRequests.has(agent); + } + + private _reportStep(agent: string, step: AgentSdkSetupFunnelStep): void { + this._telemetryService.publicLog2('agentHost.agentSdkSetup', { agent, step }); + // This feature is diagnosed from a user's attached log far more often than + // from a dashboard; the event says how many, this line says why this person. + this._logService.trace(`[AgentSdkSetup] ${agent}: ${step}`); + } + + private _getSetup(agent: string): IAgentSdkSetupInfo | undefined { + return this._setups.find(setup => setup.agent === agent); + } + + private _dispatchDownloadRequest(agent: string): void { + this._pendingRequests.add(agent); + // A fresh nonce every time so pressing the same button twice is two + // requests; the agent clears the key as it consumes it. + this._agentHostService.dispatch(ROOT_STATE_URI, { + type: ActionType.RootConfigChanged, + config: { [AGENT_SDK_SETUP_DOWNLOAD_REQUEST_KEY]: { agent, request: generateUuid() } }, + }); + // The statuses are unchanged but {@link isDownloadPending} is not, and + // without this the offer stays up until the host answers — the flicker the + // pending set exists to prevent. + this._onDidChangeSetups.fire(this._setups); + } + + private _updateSetups(setups: readonly IAgentSdkSetupInfo[]): void { + this._setups = setups; + for (const setup of setups) { + // Any status but `notDownloaded` is the host answering our request. + if (setup.download !== 'notDownloaded') { + this._pendingRequests.delete(setup.agent); + } + } + this._applyConsent(); + this._onDidChangeSetups.fire(setups); + } + + private _readConsentedAgents(): ReadonlySet { + return readConsentedSdkAgents(this._storageService.get(AGENT_SDK_DOWNLOAD_CONSENT_KEY, StorageScope.APPLICATION)); + } + + /** + * Honour standing consent without asking again. Runs on every status change + * because a host that starts (or a remote that connects) publishes + * `notDownloaded` only once it is up — there is no earlier moment to catch. + */ + private _applyConsent(): void { + for (const agent of resolveConsentedSdkDownloads(this._readConsentedAgents(), this._setups, this._consentedRequests)) { + this._consentedRequests.add(agent); + this._reportStep(agent, 'consentedDownload'); + this._dispatchDownloadRequest(agent); + } + } +} + +registerSingleton(IAgentSdkSetupService, AgentSdkSetupService, InstantiationType.Delayed); diff --git a/src/vs/workbench/services/agentHost/browser/codexAccountService.ts b/src/vs/workbench/services/agentHost/browser/codexAccountService.ts index 390ce39654a52..fbb2f8fd8a201 100644 --- a/src/vs/workbench/services/agentHost/browser/codexAccountService.ts +++ b/src/vs/workbench/services/agentHost/browser/codexAccountService.ts @@ -9,6 +9,7 @@ import { Action, IAction, SubmenuAction, toAction } from '../../../../base/commo import { generateUuid } from '../../../../base/common/uuid.js'; import { localize } from '../../../../nls.js'; import { CODEX_ACCOUNT_SIGN_IN_REQUEST_KEY, CODEX_ACCOUNT_SIGN_OUT_REQUEST_KEY, ICodexAccountInfo, readCodexAccountInfo } from '../../../../platform/agentHost/common/codexAccount.js'; +import { CODEX_AGENT_PROVIDER_ID } from '../../../../platform/agentHost/common/agent.js'; import { AgentHostCodexAgentEnabledSettingId, CodexPreferAgentHostEditorSettingId, IAgentHostService } from '../../../../platform/agentHost/common/agentService.js'; import { ChatAIDisabledSettingId } from '../../../../platform/chat/common/chatSettings.js'; import { ActionType } from '../../../../platform/agentHost/common/state/sessionActions.js'; @@ -25,6 +26,12 @@ export const ICodexAccountService = createDecorator('codex export interface ICodexAccountService { readonly _serviceBrand: undefined; + /** + * The agent whose account this service manages, so callers that dispatch by + * agent id — the SDK setup banner's Sign In button — can check they are + * talking to the right service without carrying a literal `'codex'`. + */ + readonly agent: string; readonly account: ICodexAccountInfo; readonly onDidChangeAccount: Event; signIn(): void; @@ -73,6 +80,8 @@ export function openCodexAuthUrl(openerService: Pick, au class CodexAccountService extends Disposable implements ICodexAccountService { declare readonly _serviceBrand: undefined; + readonly agent = CODEX_AGENT_PROVIDER_ID; + private readonly _onDidChangeAccount = this._register(new Emitter()); readonly onDidChangeAccount = this._onDidChangeAccount.event; diff --git a/src/vs/workbench/services/agentHost/test/browser/codexAccountService.test.ts b/src/vs/workbench/services/agentHost/test/browser/codexAccountService.test.ts index aec3277eaa3c5..5731dd4d83583 100644 --- a/src/vs/workbench/services/agentHost/test/browser/codexAccountService.test.ts +++ b/src/vs/workbench/services/agentHost/test/browser/codexAccountService.test.ts @@ -8,6 +8,7 @@ import { Action, SubmenuAction } from '../../../../../base/common/actions.js'; import { Event } from '../../../../../base/common/event.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; import { AgentHostCodexAgentEnabledSettingId, CodexPreferAgentHostEditorSettingId } from '../../../../../platform/agentHost/common/agentService.js'; +import { CODEX_AGENT_PROVIDER_ID } from '../../../../../platform/agentHost/common/agent.js'; import { ChatAIDisabledSettingId } from '../../../../../platform/chat/common/chatSettings.js'; import { OpenOptions } from '../../../../../platform/opener/common/opener.js'; import { ICodexAccountService, createCodexAccountMenuActions, hasSignedInCodexChatGPTAccount, openCodexAuthUrl, shouldShowCodexAccount } from '../../browser/codexAccountService.js'; @@ -18,6 +19,7 @@ suite('CodexAccountService', () => { function service(status: ICodexAccountService['account']['status'], email?: string): ICodexAccountService & { signInCalls: number; signOutCalls: number } { return { _serviceBrand: undefined, + agent: CODEX_AGENT_PROVIDER_ID, account: { status, email }, onDidChangeAccount: Event.None, signInCalls: 0, From 6e18cea10a5193cbbba08ad5cfe54db291764078 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Thu, 20 Aug 2026 00:00:20 -0700 Subject: [PATCH 2/2] Fix two setup-status races found in review Publish the SDK download status *after* the model catalog on both agents. Publishing `ready` at the top of `_refreshModels` meant the first refresh after a download announced "the SDK is here" while `_models` was still empty -- and `ready` plus zero models is exactly how the window renders "no account found". The invariant was already documented in the setup channel's own `_download()`; the refresh path contradicted it. Re-bind `AgentSdkSetupService` to root state on `onAgentHostStart`. `rootState` is a getter over a protocol client the host replaces on every restart and reconnect, so the single constructor-time subscription went quietly stale -- and because the service is `Delayed`, constructing before the connection bound the no-op state forever. Pending download requests are cleared on re-bind too: a request the previous host never answered never will be, so the Download button comes back rather than staying suppressed. Both fixes carry regression tests that were verified to fail without them. Also corrects the `explicitlyRequested` telemetry doc, which claimed to carry a click-vs-standing-consent split it does not have, and states the banner's ambient-host scope in `agentSdkSetupSessionType`. Co-Authored-By: Claude Opus 5 --- .../node/agentSdkDownloadTelemetry.ts | 7 ++-- .../agentHost/node/claude/claudeAgent.ts | 39 +++++++++--------- .../agentHost/node/codex/codexAgent.ts | 21 ++++++---- .../agentHost/test/node/claudeAgent.test.ts | 41 +++++++++++++++++++ .../test/node/codex/codexModelRefresh.test.ts | 36 ++++++++++++++++ .../agentHostSdkSetupNotification.ts | 3 ++ .../agentHost/browser/agentSdkSetupService.ts | 23 ++++++++--- 7 files changed, 134 insertions(+), 36 deletions(-) diff --git a/src/vs/platform/agentHost/node/agentSdkDownloadTelemetry.ts b/src/vs/platform/agentHost/node/agentSdkDownloadTelemetry.ts index 8c7846b88d3aa..8993588eefe71 100644 --- a/src/vs/platform/agentHost/node/agentSdkDownloadTelemetry.ts +++ b/src/vs/platform/agentHost/node/agentSdkDownloadTelemetry.ts @@ -80,7 +80,7 @@ type AgentSdkDownloadClassification = { packageId: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Which agent SDK was being fetched, e.g. claude or codex.' }; phase: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Whether the download started, completed, or failed.' }; failureReason: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Coarse bucket for a failed download (cancelled, network, filesystem, extract, notConfigured, unsupportedTarget, unknown). Empty unless the phase is failed.' }; - explicitlyRequested: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Whether a user gesture asked for this download, as opposed to a quiet re-fetch under standing consent.' }; + explicitlyRequested: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'Whether the setup flow drove this download and showed its progress — a click, or a quiet re-fetch under standing consent — as opposed to a background fetch nobody was watching.' }; durationMs: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; isMeasurement: true; comment: 'How long the download had been running when it reached this phase.' }; receivedBytes: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; isMeasurement: true; comment: 'Bytes fetched by the time this phase was reached.' }; totalBytes: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; isMeasurement: true; comment: 'Total size the server advertised, or zero when it did not.' }; @@ -91,8 +91,9 @@ type AgentSdkDownloadClassification = { /** * Report one endpoint of a download. Callers pass only terminal and `started` * frames; the throttled `progress` frames are not counted. - * `explicitlyRequested` is the user-gesture-versus-standing-consent split costs - * are measured on. + * `explicitlyRequested` splits setup-driven downloads from background ones, not + * clicks from standing consent — both hold a progress interest. That split is + * the funnel's own `downloadClicked` / `consentedDownload` steps. */ export function reportAgentSdkDownload( telemetryService: ITelemetryService, diff --git a/src/vs/platform/agentHost/node/claude/claudeAgent.ts b/src/vs/platform/agentHost/node/claude/claudeAgent.ts index e2ebc5496b692..6c28556a625a7 100644 --- a/src/vs/platform/agentHost/node/claude/claudeAgent.ts +++ b/src/vs/platform/agentHost/node/claude/claudeAgent.ts @@ -906,9 +906,6 @@ export class ClaudeAgent extends Disposable implements IAgent { const tokenAtStart = this._githubToken; // True only for a dev override, a dev bare import, or an already-cached SDK. const canAttemptNative = await this._sdkService.canLoadWithoutDownload(); - // Free republish: another path may have pulled the SDK down since we last - // looked (the download gesture, or a session the user started). - this._sdkSetupChannel.publishWith(canAttemptNative); if (!canAttemptNative) { // No SDK, so no evidence of an account — say so rather than retaining a stale `true`. this._nativeAccountSetUp = false; @@ -930,22 +927,26 @@ export class ClaudeAgent extends Disposable implements IAgent { // rather than blanking. Sources we didn't attempt resolve fulfilled-empty // and are not counted as failures. this._logService.error('[Claude] All attempted model sources failed (merged refresh); keeping last known-good catalog'); - return; - } - // Unwrap each settled fetch: its models on success, or an empty list on - // rejection (logged) so the other provider's catalog still publishes. - const settledCatalog = (outcome: PromiseSettledResult, label: string): readonly IAgentModelInfo[] => { - if (outcome.status === 'fulfilled') { - return outcome.value; - } - this._logService.error(outcome.reason, `[Claude] Failed to fetch ${label} models (merged refresh); keeping the other provider`); - return []; - }; - const proxyModels = settledCatalog(proxyOutcome, 'proxy'); - const nativeModels = settledCatalog(nativeOutcome, 'native'); - const merged = mergeClaudeModelCatalogs(proxyModels, nativeModels); - this._logService.info(`[Claude] Models refreshed (merged). Count: ${merged.length}, ${merged.map(m => m.name).join(', ')}`); - this._models.set(merged, undefined); + } else { + // Unwrap each settled fetch: its models on success, or an empty list on + // rejection (logged) so the other provider's catalog still publishes. + const settledCatalog = (outcome: PromiseSettledResult, label: string): readonly IAgentModelInfo[] => { + if (outcome.status === 'fulfilled') { + return outcome.value; + } + this._logService.error(outcome.reason, `[Claude] Failed to fetch ${label} models (merged refresh); keeping the other provider`); + return []; + }; + const proxyModels = settledCatalog(proxyOutcome, 'proxy'); + const nativeModels = settledCatalog(nativeOutcome, 'native'); + const merged = mergeClaudeModelCatalogs(proxyModels, nativeModels); + this._logService.info(`[Claude] Models refreshed (merged). Count: ${merged.length}, ${merged.map(m => m.name).join(', ')}`); + this._models.set(merged, undefined); + } + // Last, never first: this is a free republish of "is the SDK on disk" (some + // other path may have fetched it), but announcing `ready` before the catalog + // lands is exactly how the window renders "no account found". + this._sdkSetupChannel.publishWith(canAttemptNative); } /** diff --git a/src/vs/platform/agentHost/node/codex/codexAgent.ts b/src/vs/platform/agentHost/node/codex/codexAgent.ts index 2d6b9466075f4..80266c48a93aa 100644 --- a/src/vs/platform/agentHost/node/codex/codexAgent.ts +++ b/src/vs/platform/agentHost/node/codex/codexAgent.ts @@ -1736,8 +1736,12 @@ export class CodexAgent extends Disposable implements IAgent { } private async _refreshModels(): Promise { - await Promise.all([this._refreshCopilotModels(), this._refreshCodexModels()]); + const [, sdkReady] = await Promise.all([this._refreshCopilotModels(), this._refreshCodexModels()]); this._models.set([...this._copilotModels, ...this._codexModels], undefined); + // Last, never first: also the freshest answer to "is the SDK here" (a + // download that landed elsewhere surfaces here), but announcing `ready` + // before the catalog lands is how the window renders "no account found". + this._sdkSetupChannel.publishWith(sdkReady); } /** @@ -1815,25 +1819,25 @@ export class CodexAgent extends Disposable implements IAgent { } } - private async _refreshCodexModels(): Promise { + private async _refreshCodexModels(): Promise { + // Outside the `try` so a throw still reports what we had established about + // the SDK, rather than a `false` the caller would publish as "not downloaded". + let sdkReady = false; try { // A refresh must never be what pulls the SDK down — the download is an // explicit gesture now — so with no local SDK this reports the honest // empty catalog and the banner offers it. A live connection already // proves the SDK is on disk, so it short-circuits the stat. - const sdkReady = this._connection.kind !== 'idle' || await this._isSdkResolvableWithoutDownload(); - // Also the freshest answer to "is the SDK here" — a download that landed - // elsewhere (a prewarm, another window) surfaces here. - this._sdkSetupChannel.publishWith(sdkReady); + sdkReady = this._connection.kind !== 'idle' || await this._isSdkResolvableWithoutDownload(); if (!sdkReady) { this._codexModels = []; - return; + return sdkReady; } const connection = await this._ensureConnection(); const account = await this._refreshAccount(connection.client, false); if (account.status === 'signedOut' || account.status === 'error') { this._codexModels = []; - return; + return sdkReady; } const configResponse = await connection.client.request<'config/read', ConfigReadResponse>('config/read', { includeLayers: false }); const modelProvider = configResponse.config.model_provider ?? CODEX_OPENAI_MODEL_PROVIDER; @@ -1862,6 +1866,7 @@ export class CodexAgent extends Disposable implements IAgent { // Keep the last known-good catalog; a transient periodic failure must // not make every model disappear. } + return sdkReady; } // #endregion diff --git a/src/vs/platform/agentHost/test/node/claudeAgent.test.ts b/src/vs/platform/agentHost/test/node/claudeAgent.test.ts index 748fe18966afd..aa093bd0ce65a 100644 --- a/src/vs/platform/agentHost/test/node/claudeAgent.test.ts +++ b/src/vs/platform/agentHost/test/node/claudeAgent.test.ts @@ -6133,6 +6133,47 @@ suite('ClaudeAgent — agent SDK setup channel', () => { }); }); + test('a download that lands stays `downloading` until the catalog does, so the banner never flashes "no account"', async () => { + const ctx = createTestContext(disposables, { nativeAccount: NATIVE_ACCOUNT }); + // Let the constructor's own refresh reach enumeration before the gate below + // goes up, so the only blocked enumeration is the download's. + for (let i = 0; i < 100 && ctx.sdk.supportedModelsCallCount === 0; i++) { + await tick(); + } + ctx.sdk.canLoadWithoutDownloadResult = false; + await ctx.agent.refreshModels(); + await settle(); + + ctx.sdk.supportedModelsResult = [ + { value: 'claude-sonnet-4-5-20250929', displayName: 'Claude Sonnet 4.5', description: '', supportedEffortLevels: ['high'] }, + ]; + let releaseEnumeration = () => { }; + ctx.sdk.supportedModelsGate = new Promise(resolve => { releaseEnumeration = resolve; }); + // Resolving the fetch is the moment the SDK lands on disk. + ctx.sdk.ensureAvailableGate = Promise.resolve().then(() => { ctx.sdk.canLoadWithoutDownloadResult = true; }); + const enumerationsBefore = ctx.sdk.supportedModelsCallCount; + + dispatchDownload(ctx); + for (let i = 0; i < 100 && ctx.sdk.supportedModelsCallCount === enumerationsBefore; i++) { + await tick(); + } + const enumerating = { download: readSetup(ctx)?.download, models: ctx.agent.models.get().length }; + + releaseEnumeration(); + for (let i = 0; i < 100 && ctx.agent.models.get().length === 0; i++) { + await tick(); + } + await settle(); + + assert.deepStrictEqual({ enumerating, after: readSetup(ctx)?.download, models: ctx.agent.models.get().length }, { + // `ready` while the catalog is still empty is precisely how the window + // renders "we looked and found no account". + enumerating: { download: 'downloading', models: 0 }, + after: 'ready', + models: 1, + }); + }); + test('the request key is cleared as it is consumed, so an identical later press still lands', async () => { const ctx = createTestContext(disposables); ctx.sdk.canLoadWithoutDownloadResult = false; diff --git a/src/vs/platform/agentHost/test/node/codex/codexModelRefresh.test.ts b/src/vs/platform/agentHost/test/node/codex/codexModelRefresh.test.ts index e0aee4c565d58..b639fbba1726d 100644 --- a/src/vs/platform/agentHost/test/node/codex/codexModelRefresh.test.ts +++ b/src/vs/platform/agentHost/test/node/codex/codexModelRefresh.test.ts @@ -751,6 +751,42 @@ suite('CodexAgent — agent SDK setup channel', () => { }); }); + test('a download that lands stays `downloading` until the catalog does, so the banner never flashes "no account"', async () => { + const sdkDownloader = createNotDownloaded(); + sdkDownloader.loadSdkRootResult = async () => { sdkDownloader.resolvableWithoutDownload = true; return '/tmp/codex-sdk'; }; + const ctx = createAgentContext(disposables, async () => [], {}, sdkDownloader); + let releaseEnumeration = () => { }; + const enumerated = new Promise(resolve => { releaseEnumeration = resolve; }); + const connection = createChatGPTConnection(); + ctx.agent['_ensureConnection'] = async () => ({ + ...connection, + client: { + request: async (method: string) => { + if (method === 'model/list') { + await enumerated; + } + return connection.client.request(method); + }, + }, + } as never); + await settle(); + + dispatchDownload(ctx); + await settle(); + const enumerating = { download: readSetup(ctx)?.download, models: ctx.agent.models.get().length }; + + releaseEnumeration(); + await settle(); + + assert.deepStrictEqual({ enumerating, after: readSetup(ctx)?.download, models: ctx.agent.models.get().length }, { + // `ready` while the catalog is still empty is precisely how the window + // renders "we looked and found no account". + enumerating: { download: 'downloading', models: 0 }, + after: 'ready', + models: 1, + }); + }); + test('the request key is cleared as it is consumed, so an identical later press still lands', async () => { const sdkDownloader = createNotDownloaded(); let downloads = 0; diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSdkSetupNotification.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSdkSetupNotification.ts index 9f941fb82b01d..b7115292dcd62 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSdkSetupNotification.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSdkSetupNotification.ts @@ -108,6 +108,9 @@ function noAccountDescription(setup: IAgentSdkSetupInfo, displayName: string): s /** * The session type an agent's sessions run under, derived the same way * `AgentHostChatContribution` derives it — so agent #3 needs no edit here. + * Scoped to the window's ambient host, which is itself the remote in a remote + * window; the Sessions app's additional `remote--` + * connections are outside this banner, as they are the Copilot one. */ export function agentSdkSetupSessionType(agent: string): string { return `${LOCAL_AGENT_HOST_SCHEME_PREFIX}${agent}`; diff --git a/src/vs/workbench/services/agentHost/browser/agentSdkSetupService.ts b/src/vs/workbench/services/agentHost/browser/agentSdkSetupService.ts index 42cbd99d2ad85..f47ab455febcd 100644 --- a/src/vs/workbench/services/agentHost/browser/agentSdkSetupService.ts +++ b/src/vs/workbench/services/agentHost/browser/agentSdkSetupService.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { Emitter, Event } from '../../../../base/common/event.js'; -import { Disposable } from '../../../../base/common/lifecycle.js'; +import { Disposable, DisposableStore } from '../../../../base/common/lifecycle.js'; import { generateUuid } from '../../../../base/common/uuid.js'; import { AGENT_SDK_SETUP_DOWNLOAD_REQUEST_KEY, IAgentSdkSetupInfo, readAgentSdkSetupInfos, readConsentedSdkAgents, resolveConsentedSdkDownloads, writeConsentedSdkAgents } from '../../../../platform/agentHost/common/agentSdkSetup.js'; import { IAgentHostService } from '../../../../platform/agentHost/common/agentService.js'; @@ -107,7 +107,7 @@ class AgentSdkSetupService extends Disposable implements IAgentSdkSetupService { private readonly _onDidChangeSetups = this._register(new Emitter()); readonly onDidChangeSetups = this._onDidChangeSetups.event; - private _setups: readonly IAgentSdkSetupInfo[]; + private _setups: readonly IAgentSdkSetupInfo[] = []; /** * Agents whose SDK we have already re-requested under standing consent, so a @@ -137,10 +137,21 @@ class AgentSdkSetupService extends Disposable implements IAgentSdkSetupService { @ICodexAccountService private readonly _codexAccountService: ICodexAccountService, ) { super(); - const initialState = this._agentHostService.rootState.value; - this._setups = readAgentSdkSetupInfos(initialState instanceof Error ? undefined : initialState); - this._applyConsent(); - this._register(this._agentHostService.rootState.onDidChange(state => this._updateSetups(readAgentSdkSetupInfos(state)))); + // `rootState` is a getter over a protocol client the host replaces on every + // restart and reconnect, so one subscription taken here would go quietly + // stale — re-bind, as the banner and the Copilot notification both do. + const rootStateListeners = this._register(new DisposableStore()); + const bindRootState = () => { + rootStateListeners.clear(); + rootStateListeners.add(this._agentHostService.rootState.onDidChange(state => this._updateSetups(readAgentSdkSetupInfos(state)))); + // A request the previous host never answered never will be; dropping it + // re-offers the button rather than suppressing the offer for good. + this._pendingRequests.clear(); + const state = this._agentHostService.rootState.value; + this._updateSetups(readAgentSdkSetupInfos(state instanceof Error ? undefined : state)); + }; + bindRootState(); + this._register(this._agentHostService.onAgentHostStart(bindRootState)); } requestDownload(agent: string): void {