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..8993588eefe71 --- /dev/null +++ b/src/vs/platform/agentHost/node/agentSdkDownloadTelemetry.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 { 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 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.' }; + 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` 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, + 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..6c28556a625a7 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,24 @@ 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(); + 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,29 +920,33 @@ 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 // 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; + } 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); } - // 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); } /** @@ -922,6 +957,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 +972,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 +2094,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 +2115,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 +2131,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..80266c48a93aa 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(), ]; } @@ -1702,42 +1736,35 @@ 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); - } - - 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], - ); + // 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); } /** - * 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 { @@ -1792,17 +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 { - 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. + 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; @@ -1831,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 @@ -5607,10 +5643,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 +5664,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 +5680,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..aa093bd0ce65a 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,212 @@ 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('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; + 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 +6457,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 +6541,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 +6670,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 +6786,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 +8390,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 +8399,7 @@ suite('ClaudeAgent — Phase 11 customizations', () => { [IClaudeProxyService, proxy], [ISessionDataService, sessionData], [IClaudeAgentSdkService, sdk], + [IAgentSdkDownloader, sdkDownloader], [IAgentPluginManager, pluginManager], [IAgentHostGitService, createNoopGitService()], [IAgentHostCheckpointService, NULL_CHECKPOINT_SERVICE], @@ -8176,7 +8445,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..b639fbba1726d 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,178 @@ 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('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; + 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..b7115292dcd62 --- /dev/null +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSdkSetupNotification.ts @@ -0,0 +1,342 @@ +/*--------------------------------------------------------------------------------------------- + * 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. + * 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}`; +} + +/** + * 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..f47ab455febcd --- /dev/null +++ b/src/vs/workbench/services/agentHost/browser/agentSdkSetupService.ts @@ -0,0 +1,258 @@ +/*--------------------------------------------------------------------------------------------- + * 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, 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'; +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(); + // `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 { + 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,