diff --git a/src/vs/platform/agentHost/common/agentHostCustomizationConfig.ts b/src/vs/platform/agentHost/common/agentHostCustomizationConfig.ts index 070f68224565a..3a6fa525bd7ba 100644 --- a/src/vs/platform/agentHost/common/agentHostCustomizationConfig.ts +++ b/src/vs/platform/agentHost/common/agentHostCustomizationConfig.ts @@ -20,13 +20,6 @@ export const enum AgentHostConfigKey { * TODO: revisit magic key in config; refine into a dedicated typed channel. https://github.com/microsoft/vscode/issues/313812 */ DefaultShell = 'defaultShell', - /** - * When true (the default), the Claude provider routes all Anthropic - * `messages` traffic through the local Copilot-CAPI proxy (Copilot-routed - * Claude). When false, the Claude Agent SDK talks to Anthropic directly on - * the user's own credentials (BYO Anthropic — Phase 19). - */ - ClaudeUseCopilotProxy = 'claudeUseCopilotProxy', /** * Experimentation flag for conditional agent-window auth. When true, a * session type that is usable without GitHub (e.g. Claude in native mode with @@ -98,12 +91,6 @@ export const agentHostCustomizationConfigSchema = createSchema({ title: localize('agentHost.config.defaultShell.title', "Default Shell"), description: localize('agentHost.config.defaultShell.description', "Absolute path to the shell executable used by host-managed terminals. Normally pushed by the connected VS Code client from `terminal.integrated.agentHostProfile.` (falling back to `terminal.integrated.defaultProfile.`); when unset, the agent host falls back to the system shell. Only the path is supported; `args` and `env` from the workbench profile are not piped through yet. The workbench only pushes this for the local agent host — remote agent host operators should set this directly in the remote machine's `agent-host-config.json`."), }), - [AgentHostConfigKey.ClaudeUseCopilotProxy]: schemaProperty({ - type: 'boolean', - title: localize('agentHost.config.claudeUseCopilotProxy.title', "Route Claude Through Copilot"), - description: localize('agentHost.config.claudeUseCopilotProxy.description', "When enabled (the default), the Claude agent routes all requests through GitHub Copilot. When disabled, Claude talks to Anthropic directly using your own credentials (API key or Claude subscription)."), - default: true, - }), [AgentHostConfigKey.AllowSignedOutWhenUsable]: schemaProperty({ type: 'boolean', title: localize('agentHost.config.allowSignedOutWhenUsable.title', "Allow Signed-Out Agent Window"), diff --git a/src/vs/platform/agentHost/node/claude/claudeAgent.ts b/src/vs/platform/agentHost/node/claude/claudeAgent.ts index b8fd2c5f52d01..cb27deead6769 100644 --- a/src/vs/platform/agentHost/node/claude/claudeAgent.ts +++ b/src/vs/platform/agentHost/node/claude/claudeAgent.ts @@ -478,8 +478,8 @@ export class ClaudeAgent extends Disposable implements IAgent { // waiting for `authenticate()`. Without this a signed-out window with a local // Claude setup would show an empty picker. `queueMicrotask` runs it off the // ctor stack. The per-session transport is derived on demand at materialize - // (see {@link _defaultTransportMode}), so a `claudeUseCopilotProxy` change - // needs no reactive re-resolve — the next session simply reads it live. + // (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(); }); } @@ -487,18 +487,14 @@ export class ClaudeAgent extends Disposable implements IAgent { * 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 * availability: a started {@link _proxyHandle} means Copilot is serveable now, a - * local Claude setup means native is. The precedence (explicit - * `claudeUseCopilotProxy` override; else sign-in state and local setup) is - * delegated to the pure {@link resolveClaudeTransportMode}. A provider-qualified - * model bypasses this and routes on its own provider. + * local Claude setup means native is. The precedence (sign-in state, then local + * setup) is delegated to the pure {@link resolveClaudeTransportMode}. A + * provider-qualified model bypasses this and routes on its own provider. */ private _defaultTransportMode(): ClaudeTransportMode { - // An absent `claudeUseCopilotProxy` stays `undefined` so the pure function - // can tell an explicit override from "fall through to the sign-in rules". - const explicitProxy = this._configurationService.getRootValue(agentHostCustomizationConfigSchema, AgentHostConfigKey.ClaudeUseCopilotProxy); const allowSignedOutWhenUsable = this._configurationService.getRootValue(agentHostCustomizationConfigSchema, AgentHostConfigKey.AllowSignedOutWhenUsable) === true; const hasExistingSetup = allowSignedOutWhenUsable && detectExistingClaudeSetup(this._environmentService.userHome.fsPath); - return resolveClaudeTransportMode({ explicitProxy, allowSignedOutWhenUsable, hasGitHubToken: this._proxyHandle !== undefined, hasExistingSetup }); + return resolveClaudeTransportMode({ allowSignedOutWhenUsable, hasGitHubToken: this._proxyHandle !== undefined, hasExistingSetup }); } // #region Descriptor + auth @@ -680,6 +676,17 @@ export class ClaudeAgent extends Disposable implements IAgent { * 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 + * 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 + * `CLAUDE_CODE_OAUTH_TOKEN` and an empty `HOME`. Publishing it unconditionally + * would advertise models for an agent that cannot serve a single request, which + * 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. */ private async _refreshModels(): Promise { const tokenAtStart = this._githubToken; diff --git a/src/vs/platform/agentHost/node/claude/claudeModelSelection.ts b/src/vs/platform/agentHost/node/claude/claudeModelSelection.ts index 3389a9164dd61..0dc6b663abc82 100644 --- a/src/vs/platform/agentHost/node/claude/claudeModelSelection.ts +++ b/src/vs/platform/agentHost/node/claude/claudeModelSelection.ts @@ -144,11 +144,18 @@ export function resolveClaudeSessionTransport(inputs: { * so re-stamping it to a transport token would misroute a model-selected * `create_session`. The transport/group token lives only in `_meta`. * - * Proxy models come first to preserve the picker's `models[0]`-is-default - * convention for the common (Copilot) case. Every other field is passed through - * untouched. Either list may be empty — one source failing to fetch contributes - * nothing but must never blank the other — so merging an empty side just yields - * the other side's qualified models. + * Array order is *not* what picks the session default. The picker re-buckets the + * flat list by the `_meta` vendor token and renders group-by-group, so which + * model is pre-selected follows the group ordering — verified end-to-end: with + * both halves populated the Anthropic group sorts first and + * `@provider=anthropic:default` is pre-selected, i.e. the default routes native + * and bills the user's own Anthropic account. Do not reason about the default + * from the order here. (Making that choice explicit rather than emergent needs a + * default/sticky model preference, which does not exist yet.) + * + * Every other field is passed through untouched. Either list may be empty — one + * source failing to fetch contributes nothing but must never blank the other — + * so merging an empty side just yields the other side's qualified models. */ export function mergeClaudeModelCatalogs(proxy: readonly IAgentModelInfo[], native: readonly IAgentModelInfo[]): IAgentModelInfo[] { return [ diff --git a/src/vs/platform/agentHost/node/claude/claudeTransportMode.ts b/src/vs/platform/agentHost/node/claude/claudeTransportMode.ts index 3facce9c8b2da..5e55282c998a5 100644 --- a/src/vs/platform/agentHost/node/claude/claudeTransportMode.ts +++ b/src/vs/platform/agentHost/node/claude/claudeTransportMode.ts @@ -15,15 +15,9 @@ import { vObj, vOptionalProp, vString, type ValidatorType } from '../../../../ba export type ClaudeTransportMode = 'proxy' | 'native'; /** - * The four precedence inputs {@link resolveClaudeTransportMode} decides over. + * The three precedence inputs {@link resolveClaudeTransportMode} decides over. */ export interface IClaudeTransportModeInputs { - /** - * User/workspace-set value of `claudeUseCopilotProxy`, or `undefined` when - * unset — the distinction between an explicit choice and the default is what - * makes an explicit setting a hard override. - */ - readonly explicitProxy: boolean | undefined; /** Whether the experimentation flag enabling signed-out-when-usable is on. */ readonly allowSignedOutWhenUsable: boolean; /** Whether a GitHub Copilot token has been captured (i.e. signed in). */ @@ -33,25 +27,42 @@ export interface IClaudeTransportModeInputs { } /** - * Pure decision (ADR 0001, "D4"): which transport should the Claude provider - * use right now? Precedence, highest first: + * Which transport should the Claude provider fall back to right now? Pure + * decision; precedence, highest first: * - * 1. An explicit `claudeUseCopilotProxy` setting is a HARD override. - * 2. Feature flag off means today's default behavior (always proxy). - * 3. Signed in to GitHub prefers Copilot (proxy). - * 4. Signed out but with the user's own Claude credentials uses native (no GitHub). - * 5. Nothing usable falls back to proxy, which surfaces as requires-GitHub and drives the - * window sign-in gate. + * 1. Feature flag off means today's default behavior (always proxy). + * 2. Signed in to GitHub prefers Copilot (proxy). + * 3. Signed out but with the user's own Claude credentials uses native (no GitHub). + * 4. Nothing usable still falls back to proxy — the safe end, since attempting + * native with no credential would fail inside the SDK rather than at a + * surface that can explain itself. * - * Native mode drops the GitHub Copilot protected resource, so getting this - * decision right is what lets a signed-out user with their own credentials run - * without being forced to sign in. + * This is only the *fallback* for a session whose model names no provider. A + * provider-qualified model routes on its own provider + * (`resolveClaudeSessionTransport`), so getting this decision right is what lets + * a signed-out user with their own credentials start working without being + * 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". Claude advertises the Copilot protected resource as `required: false` + * unconditionally, so `resolveAgentAuthRequirement` separates "usable" from + * "unusable" on the *model count* instead: in case 4 neither half of the merged + * catalog can be enumerated, the published catalog is empty, and the type + * resolves to `Unusable` — surfacing as "no models". The proxy fallback only + * bites at use time, when a model-less/bare session actually 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 + * of the model the user picked; a flag would keep disagreeing with what the + * picker shows (it could not stop a Copilot-routed model from being offered or + * chosen, because neither model enumeration nor the advertised protected + * resources would consult it). Expressing a preference is a *model*-selection + * concern — a default/sticky model — not a transport one. */ export function resolveClaudeTransportMode(inputs: IClaudeTransportModeInputs): ClaudeTransportMode { - const { explicitProxy, allowSignedOutWhenUsable, hasGitHubToken, hasExistingSetup } = inputs; - if (explicitProxy !== undefined) { - return explicitProxy ? 'proxy' : 'native'; - } + const { allowSignedOutWhenUsable, hasGitHubToken, hasExistingSetup } = inputs; if (!allowSignedOutWhenUsable) { return 'proxy'; } diff --git a/src/vs/platform/agentHost/test/node/agentHostGitHubEndpointService.test.ts b/src/vs/platform/agentHost/test/node/agentHostGitHubEndpointService.test.ts index 926ec1057e7f6..adc6a52d2c92e 100644 --- a/src/vs/platform/agentHost/test/node/agentHostGitHubEndpointService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostGitHubEndpointService.test.ts @@ -66,7 +66,7 @@ suite('AgentHostGitHubEndpointService', () => { disposables.add(service.onDidChange(() => fires++)); // An unrelated root-config change must NOT fire. - configService.updateRootConfig({ [AgentHostConfigKey.ClaudeUseCopilotProxy]: false }); + configService.updateRootConfig({ [AgentHostConfigKey.AllowSignedOutWhenUsable]: true }); assert.strictEqual(fires, 0); // Setting the enterprise URI fires once and repoints the endpoints. diff --git a/src/vs/platform/agentHost/test/node/claudeAgent.test.ts b/src/vs/platform/agentHost/test/node/claudeAgent.test.ts index c60ccc641b90e..bd0cc1f3728f3 100644 --- a/src/vs/platform/agentHost/test/node/claudeAgent.test.ts +++ b/src/vs/platform/agentHost/test/node/claudeAgent.test.ts @@ -881,7 +881,7 @@ function createTestContext( [IAgentHostGitHubEndpointService, overrides?.gitHubEndpointService ?? createTestGitHubEndpointService()], ); const instantiationService: IInstantiationService = disposables.add(new InstantiationService(services)); - // Phase 19: seed root config (e.g. `claudeUseCopilotProxy`) BEFORE the agent + // Seed root config (e.g. `allowSignedOutWhenUsable`) BEFORE the agent // resolves its transport mode in the constructor. if (overrides?.rootConfig) { configService.updateRootConfig(overrides.rootConfig); @@ -895,6 +895,23 @@ function tick(): Promise { return new Promise(resolve => setImmediate(resolve)); } +/** + * 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. + */ +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 }); + } +} + /** * A two-turn source transcript (`u1`/`a1`, `u2`/`a2`) used by the Phase 6.5 * fork tests. Forking at `u1` keeps `[u1]` inclusive, anchored on that turn's @@ -1048,33 +1065,16 @@ suite('ClaudeAgent', () => { }); }); - test('native transport: getProtectedResources keeps the Copilot resource but marks it not required', () => { - // Native keeps advertising the Copilot resource with `required: false` - // (rather than dropping it) so the host can silently probe for a GitHub - // token when the user is already signed in, while the window gate still - // treats the type as usable without GitHub. See `getProtectedResources`. - const { agent } = createTestContext(disposables, { rootConfig: { claudeUseCopilotProxy: false } }); - assert.deepStrictEqual( - agent.getProtectedResources().map(r => ({ resource: r.resource, required: r.required })), - [ - { resource: 'https://api.github.com', required: false }, - { resource: 'https://api.github.com/repos', required: false }, - ], - ); - }); - 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), `authenticate` re-resolves (rule 3: 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. - const userHome = URI.file(await fs.mkdtemp(`${os.tmpdir()}/claude-probe-home-`)); - 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 { + // 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, @@ -1111,9 +1111,7 @@ suite('ClaudeAgent', () => { proxyStarts: 1, }, }); - } finally { - await fs.rm(userHome.fsPath, { recursive: true, force: true }); - } + }); }); test('coalesces concurrent refreshModels calls onto one CAPI models request', async () => { @@ -1171,14 +1169,12 @@ suite('ClaudeAgent', () => { assert.deepStrictEqual(agent.models.get(), []); }); - test('native transport: models populate from supportedModels() with no proxy start and no CAPI models() call', async () => { + 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. - const userHome = URI.file(await fs.mkdtemp(`${os.tmpdir()}/claude-native-models-`)); - 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, proxy, api, sdk } = createTestContext(disposables, { rootConfig: { claudeUseCopilotProxy: false }, userHome }); + // 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 = [ @@ -1201,17 +1197,16 @@ suite('ClaudeAgent', () => { supportedModelsCalls: 1, capiModelsCalls: 0, }); - } finally { - await fs.rm(userHome.fsPath, { recursive: true, force: true }); - } + }); }); - test('native without a credential publishes an empty catalog instead of the SDK static list', async () => { + test('signed out without a credential 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. - const { agent, sdk } = createTestContext(disposables, { rootConfig: { claudeUseCopilotProxy: false } }); + // gate. `/mock-home` has no `.claude` credential, so the native half is + // never attempted; signed out, neither is the proxy half. + const { agent, sdk } = createTestContext(disposables); sdk.supportedModelsResult = [ { value: 'claude-sonnet-4-5-20250929', displayName: 'Claude Sonnet 4.5', description: '' }, ]; @@ -1228,11 +1223,8 @@ suite('ClaudeAgent', () => { }); test('native model enumeration closes the throwaway query (no leaked subprocess)', async () => { - const userHome = URI.file(await fs.mkdtemp(`${os.tmpdir()}/claude-native-close-`)); - 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 { sdk } = createTestContext(disposables, { rootConfig: { claudeUseCopilotProxy: false }, userHome }); + await withNativeSetup(async userHome => { + const { sdk } = createTestContext(disposables, { userHome }); sdk.supportedModelsResult = [ { value: 'claude-sonnet-4-5-20250929', displayName: 'Claude Sonnet 4.5', description: '' }, ]; @@ -1248,78 +1240,46 @@ suite('ClaudeAgent', () => { queries: 1, closed: 1, }); - } finally { - await fs.rm(userHome.fsPath, { recursive: true, force: true }); - } + }); }); test('native-default authenticate still starts the proxy so Copilot-routed models can run', async () => { // With the merged catalog always on, a native default no longer short- // circuits sign-in: `authenticate` falls through to acquire a proxy handle // so a session that later picks a Copilot-routed model has a started proxy - // to run against — even though the explicit `claudeUseCopilotProxy: false` - // keeps the model-less default (`_defaultTransportMode`) native. - const { agent, proxy } = createTestContext(disposables, { rootConfig: { claudeUseCopilotProxy: false } }); - const accepted = await agent.authenticate('https://api.github.com', 'tok'); - await tick(); - assert.deepStrictEqual({ accepted, proxyStarts: proxy.startCalls.length }, { accepted: true, proxyStarts: 1 }); - }); - - test('unusable native (explicit proxy off, no setup) does not demand GitHub sign-in', async () => { - // The "unusable" case: explicit `claudeUseCopilotProxy=false` is a hard - // override to native even with no usable credentials. It must degrade to - // "no models" (NoModels), NOT a GitHub sign-in prompt — so the Copilot - // resource is advertised `required: false` and `createSession` resolves - // (native needs no proxy) instead of throwing `AHP_AUTH_REQUIRED` the way - // proxy mode does before authentication (cf. the AHP_AUTH_REQUIRED test). - const { agent } = createTestContext(disposables, { rootConfig: { claudeUseCopilotProxy: false } }); - const created = await agent.createSession({ workingDirectories: [URI.file('/workspace')] }); - assert.deepStrictEqual({ - copilotRequired: agent.getProtectedResources().find(r => r.resource === 'https://api.github.com')?.required, - createdWithoutAuthPrompt: created.provisional === true, - }, { - copilotRequired: false, - createdWithoutAuthPrompt: true, + // 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 }); }); }); - test('a host-default transport flip no longer proactively demands auth (sign-in defers to first send)', () => { + test('a host-default transport flip no longer proactively demands auth (sign-in defers to first send)', async () => { // A host-default flip only changes the fallback transport for model-less // sessions; the merged catalog still publishes both providers and // `getProtectedResources()` keeps Copilot optional, so a flip must NOT fire // `auth/required`. Sign-in for a Copilot-routed model defers to the first - // send, where `_ensureAuthenticated` throws `AHP_AUTH_REQUIRED`. - const { agent, configService } = createTestContext(disposables, { rootConfig: { claudeUseCopilotProxy: false } }); - const events: Omit[] = []; - disposables.add(agent.onDidRequireAuth(e => events.push(e))); - - configService.updateRootConfig({ claudeUseCopilotProxy: true }); - - assert.deepStrictEqual(events, []); - }); - - test('transport flip does not emit auth/required when a proxy handle already exists', async () => { - const { agent, proxy, configService } = createTestContext(disposables); - await agent.authenticate('https://api.github.com', 'tok'); - await tick(); - assert.strictEqual(proxy.startCalls.length, 1); - - const events: Omit[] = []; - disposables.add(agent.onDidRequireAuth(e => events.push(e))); - configService.updateRootConfig({ claudeUseCopilotProxy: false }); // → native - configService.updateRootConfig({ claudeUseCopilotProxy: true }); // → proxy; handle persists - - assert.deepStrictEqual(events, []); - }); - - test('transport flip proxy→native does not emit auth/required', () => { - const { agent, configService } = createTestContext(disposables); - const events: Omit[] = []; - disposables.add(agent.onDidRequireAuth(e => events.push(e))); + // 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, + }); + const events: Omit[] = []; + disposables.add(agent.onDidRequireAuth(e => events.push(e))); - configService.updateRootConfig({ claudeUseCopilotProxy: false }); + await agent.authenticate('https://api.github.com', 'tok'); + await tick(); - assert.deepStrictEqual(events, []); + assert.deepStrictEqual(events, []); + }); }); test('construction in proxy mode does not emit auth/required', async () => { @@ -1334,12 +1294,12 @@ suite('ClaudeAgent', () => { test('re-authenticating an unchanged token starts the proxy when a prior start left no handle', async () => { // authenticate() always attempts the proxy on sign-in (so the merged - // catalog's Copilot models are runnable), even under an explicit native - // default. A proxy-start failure is soft: it leaves BOTH the token and the - // handle unset. Re-authenticating with the SAME token must therefore retry - // start() — the uncommitted token reads as new, not as an "unchanged" - // short-circuit (which is additionally guarded by `&& this._proxyHandle`). - const { agent, proxy } = createTestContext(disposables, { rootConfig: { claudeUseCopilotProxy: false } }); + // catalog's Copilot models are runnable). A proxy-start failure is soft: it + // leaves BOTH the token and the handle unset. Re-authenticating with the + // SAME token must therefore retry start() — the uncommitted token reads as + // new, not as an "unchanged" short-circuit (which is additionally guarded by + // `&& this._proxyHandle`). + const { agent, proxy } = createTestContext(disposables); let failNext = true; proxy.start = async (token: string) => { proxy.startCalls.push({ token }); @@ -1350,8 +1310,8 @@ suite('ClaudeAgent', () => { return { baseUrl: 'http://127.0.0.1:0', nonce: `nonce-for-${token}`, dispose: () => { proxy.disposeCount++; } }; }; - // First authenticate: the native default still attempts the proxy; start - // fails softly, leaving token 'T' uncommitted and no handle. + // First authenticate: start fails softly, leaving token 'T' uncommitted and + // no handle. await agent.authenticate('https://api.github.com', 'T'); // Re-auth with the SAME token: uncommitted token ⇒ must retry start() (now succeeds). await agent.authenticate('https://api.github.com', 'T'); @@ -5482,22 +5442,6 @@ suite('ClaudeAgent', () => { suite('ClaudeAgent — per-session provider', () => { const disposables = ensureNoDisposablesAreLeakedInTestSuite(); - /** - * 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. Mirrors the native-transport tests. - */ - async function withNativeSetup(body: (userHome: URI) => Promise): Promise { - const userHome = URI.file(await fs.mkdtemp(`${os.tmpdir()}/claude-per-session-`)); - 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 }); - } - } - /** * The per-session proxy bearer (`ANTHROPIC_AUTH_TOKEN`) is injected into * `Options.settings.env` only for the Copilot proxy transport; the native @@ -5705,8 +5649,8 @@ suite('ClaudeAgent — per-session provider', () => { ); ctx.sdk.queryAdvance = undefined; - // Sign into Copilot: absent an explicit `claudeUseCopilotProxy`, this - // flips the host default native→proxy and acquires a proxy handle. + // 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(); @@ -5887,11 +5831,9 @@ suite('ClaudeAgent — per-session provider', () => { // the merged catalog's Copilot models can run, but a `start()` failure is // always soft — GitHub sign-in itself succeeded and a Copilot-routed model // simply re-drives sign-in on its first send. So a transient failure must - // resolve sign-in as success, not reject. (Shown here under an explicit - // native default, where nothing downstream even needs the handle.) - const { agent, proxy } = createTestContext(disposables, { - rootConfig: { claudeUseCopilotProxy: false }, - }); + // resolve sign-in as success, not reject. (Shown here on a first sign-in, + // where there is no prior handle to tear down.) + const { agent, proxy } = createTestContext(disposables); proxy.startError = new Error('proxy boom'); const ok = await agent.authenticate('https://api.github.com', 'tok'); diff --git a/src/vs/platform/agentHost/test/node/claudeTransportMode.test.ts b/src/vs/platform/agentHost/test/node/claudeTransportMode.test.ts index 99e396aae65e3..cea3ffc6f69dd 100644 --- a/src/vs/platform/agentHost/test/node/claudeTransportMode.test.ts +++ b/src/vs/platform/agentHost/test/node/claudeTransportMode.test.ts @@ -15,49 +15,27 @@ suite('claudeTransportMode', () => { ensureNoDisposablesAreLeakedInTestSuite(); test('resolveClaudeTransportMode precedence over the full input matrix', () => { - const explicitValues: readonly (boolean | undefined)[] = [undefined, true, false]; const bools: readonly boolean[] = [false, true]; const actual: Record = {}; - for (const explicitProxy of explicitValues) { - for (const allowSignedOutWhenUsable of bools) { - for (const hasGitHubToken of bools) { - for (const hasExistingSetup of bools) { - const key = `explicit=${explicitProxy},flag=${allowSignedOutWhenUsable},token=${hasGitHubToken},setup=${hasExistingSetup}`; - actual[key] = resolveClaudeTransportMode({ explicitProxy, allowSignedOutWhenUsable, hasGitHubToken, hasExistingSetup }); - } + for (const allowSignedOutWhenUsable of bools) { + for (const hasGitHubToken of bools) { + for (const hasExistingSetup of bools) { + const key = `flag=${allowSignedOutWhenUsable},token=${hasGitHubToken},setup=${hasExistingSetup}`; + actual[key] = resolveClaudeTransportMode({ allowSignedOutWhenUsable, hasGitHubToken, hasExistingSetup }); } } } assert.deepStrictEqual(actual, { - // Explicit unset: the flag/sign-in/setup rules decide. - 'explicit=undefined,flag=false,token=false,setup=false': 'proxy', // flag off ⇒ today's default - 'explicit=undefined,flag=false,token=false,setup=true': 'proxy', // flag off ignores setup - 'explicit=undefined,flag=false,token=true,setup=false': 'proxy', - 'explicit=undefined,flag=false,token=true,setup=true': 'proxy', - 'explicit=undefined,flag=true,token=false,setup=false': 'proxy', // nothing usable ⇒ requires-GitHub - 'explicit=undefined,flag=true,token=false,setup=true': 'native', // signed out + own creds ⇒ native - 'explicit=undefined,flag=true,token=true,setup=false': 'proxy', // signed in ⇒ prefer Copilot - 'explicit=undefined,flag=true,token=true,setup=true': 'proxy', // signed in wins over setup - // Explicit proxy=true: hard override, always proxy. - 'explicit=true,flag=false,token=false,setup=false': 'proxy', - 'explicit=true,flag=false,token=false,setup=true': 'proxy', - 'explicit=true,flag=false,token=true,setup=false': 'proxy', - 'explicit=true,flag=false,token=true,setup=true': 'proxy', - 'explicit=true,flag=true,token=false,setup=false': 'proxy', - 'explicit=true,flag=true,token=false,setup=true': 'proxy', - 'explicit=true,flag=true,token=true,setup=false': 'proxy', - 'explicit=true,flag=true,token=true,setup=true': 'proxy', - // Explicit proxy=false: hard override, always native. - 'explicit=false,flag=false,token=false,setup=false': 'native', - 'explicit=false,flag=false,token=false,setup=true': 'native', - 'explicit=false,flag=false,token=true,setup=false': 'native', - 'explicit=false,flag=false,token=true,setup=true': 'native', - 'explicit=false,flag=true,token=false,setup=false': 'native', - 'explicit=false,flag=true,token=false,setup=true': 'native', - 'explicit=false,flag=true,token=true,setup=false': 'native', - 'explicit=false,flag=true,token=true,setup=true': 'native', + 'flag=false,token=false,setup=false': 'proxy', // flag off ⇒ today's default + 'flag=false,token=false,setup=true': 'proxy', // flag off ignores setup + 'flag=false,token=true,setup=false': 'proxy', + 'flag=false,token=true,setup=true': 'proxy', + 'flag=true,token=false,setup=false': 'proxy', // nothing usable ⇒ safe end (fails at use, not here) + 'flag=true,token=false,setup=true': 'native', // signed out + own creds ⇒ native + 'flag=true,token=true,setup=false': 'proxy', // signed in ⇒ prefer Copilot + 'flag=true,token=true,setup=true': 'proxy', // signed in wins over setup }); }); diff --git a/src/vs/sessions/browser/sessionsAuthGate.ts b/src/vs/sessions/browser/sessionsAuthGate.ts index af5a56423d3ed..6f28b1f9e1fd0 100644 --- a/src/vs/sessions/browser/sessionsAuthGate.ts +++ b/src/vs/sessions/browser/sessionsAuthGate.ts @@ -10,6 +10,30 @@ import type { IConfigurationService } from '../../platform/configuration/common/ import { SessionTypeAuthRequirement } from '../services/sessions/common/session.js'; import type { ISessionsManagementService } from '../services/sessions/common/sessionsManagement.js'; +/** + * Predicates behind the Agents window's conditional authentication — when the + * window may open for a user who is signed out of GitHub. + * + * Two gates, at different altitudes, are easy to confuse: + * + * - The **window gate** is the last-resort, window-level block that forces + * sign-in before *any* of the sessions UI is shown (backed by + * `SessionsWelcomeVisibleContext`). Historically unconditional; it now lifts as + * soon as some session type can work without GitHub. Note the *editor* window + * is untouched by all of this — its chat-setup modal already offers a "Don't + * sign in" escape hatch, and it is that missing escape hatch in the Agents + * window (a non-dismissible modal) that this machinery restores conditionally. + * - The **per-type gate** is the on-demand sign-in surfaced when the user selects + * a specific session type that needs GitHub. It already existed + * (`getSessionTypeAvailability()` → `SignInRequired`) and still carries most of + * the work: once the window is open, each type answers for itself. + * + * "Requires GitHub auth" is a property of a session type *at a moment in time*, + * not a fixed trait — Claude and Codex both move as their own credentials come + * and go. It is resolved by each provider into + * {@link SessionTypeAuthRequirement} and read here provider-agnostically. + */ + /** * Whether the `chat.agentHost.allowSignedOutWhenUsable` experimentation opt-in * is enabled. When off (the default), the conditional-auth feature is dark and diff --git a/src/vs/sessions/browser/sessionsSetUpService.ts b/src/vs/sessions/browser/sessionsSetUpService.ts index 9c76e845efd1c..08d9263e2cd93 100644 --- a/src/vs/sessions/browser/sessionsSetUpService.ts +++ b/src/vs/sessions/browser/sessionsSetUpService.ts @@ -271,10 +271,17 @@ class SessionsSetUpWidget extends Disposable { } /** - * Whether the Agents window must fall back to forcing GitHub sign-in. Every - * caller is on a signed-out path, so this is simply the inverse of "can work - * without GitHub" — always true while the opt-in is off, which is today's - * mandatory-sign-in behavior. + * The **window gate**: whether the Agents window must fall back to forcing + * GitHub sign-in before showing any of the sessions UI. Every caller is on a + * signed-out path, so this is simply the inverse of "can work without GitHub" + * — always true while the opt-in is off, which is today's mandatory-sign-in + * behavior. + * + * Deliberately a *last resort*, not the primary gate. The moment any session + * type is usable without GitHub the window opens, and per-type on-demand + * sign-in carries the rest — so this never blocks a user who has their own + * credentials. See `sessionsAuthGate.ts` for the window-gate vs per-type-gate + * distinction. */ private _mustForceGitHubSignIn(): boolean { return !this._usableWithoutGitHub.get(); diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index 5b9018aa525c8..29f4b3bc1bf33 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -263,12 +263,29 @@ export const CopilotCLISessionType: ISessionType = { /** * Resolve what an agent needs before it can serve a request, from what it - * advertises. An agent that still requires the GitHub Copilot protected - * resource needs sign-in; one that has dropped the requirement is running on - * its own credentials — but only counts as usable if it actually has models, - * since an agent with an empty catalog cannot produce a request no matter who - * is signed in. Absent resources mean the host has not resolved the agent yet, - * so assume GitHub until it does. + * advertises — rather than from a static per-type flag, which cannot track + * credentials that come and go. The advertised protected-resource set already + * crosses the agent-host IPC boundary and already updates reactively, so it is + * the signal rather than a parallel field providers would have to keep in sync. + * + * An agent that still requires the GitHub Copilot protected resource needs + * sign-in; one that has dropped the requirement is running on its own + * credentials. Note both Claude and Codex encode "not required" by *keeping* the + * Copilot resource and marking it `required: false` rather than omitting it — + * that lets the host silently forward a token to an already-signed-in user + * without forcing sign-in on anyone else. This treats the two identically. + * + * The model count is the second, load-bearing half. `required: false` alone + * would read as "usable without GitHub" even for an agent that cannot serve + * anything, because an agent may advertise a *static* model catalog that answers + * regardless of credentials (the Claude SDK's `supportedModels()` does exactly + * this). Providers are therefore expected to publish an empty catalog when they + * genuinely cannot run, and an empty catalog is what distinguishes + * {@link SessionTypeAuthRequirement.Unusable} from + * {@link SessionTypeAuthRequirement.None} here. + * + * Absent resources mean the host has not resolved the agent yet, so assume + * GitHub until it does. */ export function resolveAgentAuthRequirement(agent: AgentInfo): SessionTypeAuthRequirement { if (!agent.protectedResources || protectedResourcesRequireGitHubCopilotSignIn(agent.protectedResources)) { diff --git a/src/vs/sessions/contrib/providers/agentHost/test/browser/sessionTypeAuthRequirement.test.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/sessionTypeAuthRequirement.test.ts index 52a92daf2e6e7..40a208afe6239 100644 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/sessionTypeAuthRequirement.test.ts +++ b/src/vs/sessions/contrib/providers/agentHost/test/browser/sessionTypeAuthRequirement.test.ts @@ -30,11 +30,11 @@ suite('Agent Host - session type auth requirement', () => { test('an agent is only usable without GitHub when it drops the requirement AND has models', () => { // Independent source of truth. The `unusable` row is the one that matters: - // Claude pinned to native by an explicit `claudeUseCopilotProxy: false` - // with no credentials still advertises the Copilot resource as - // `required: false`, so the requirement alone would wrongly read as - // "usable without GitHub". Its empty model catalog is what distinguishes - // it. See the amendment in docs/adr/0001-conditional-agent-window-auth.md. + // Claude always advertises the Copilot resource as `required: false` + // (per-session routing means no host-global mode can make it strictly + // required), so the requirement alone would wrongly read as "usable without + // GitHub" even when neither half of the merged catalog could be enumerated. + // Its empty model catalog is what distinguishes it. const cases = [ { name: 'unresolved (no resources yet)', agent: agent(undefined, 4) }, { name: 'proxy: Copilot required', agent: agent([copilotRequired], 4) }, diff --git a/src/vs/sessions/services/sessions/common/session.ts b/src/vs/sessions/services/sessions/common/session.ts index 5d08dc776b67b..5b656d9e5d994 100644 --- a/src/vs/sessions/services/sessions/common/session.ts +++ b/src/vs/sessions/services/sessions/common/session.ts @@ -41,10 +41,13 @@ export interface ISessionType { } /** - * What a session type needs before it can serve a request. The three values are - * mutually exclusive: {@link Unusable} is deliberately distinct from - * {@link GitHub}, because a type that cannot run at all must not be presented as - * a reason to demand GitHub sign-in (see `src/vs/sessions/CONTEXT.md`). + * What a session type needs before it can serve a request. + * + * Deliberately three states rather than a boolean. A boolean collapses + * {@link Unusable} into {@link GitHub}, which turns "this agent cannot run" into + * a sign-in prompt that would not fix anything — the user signs in, and the type + * is still broken. Providers resolve the value from what their agent advertises, + * so it moves as credentials come and go rather than being a fixed trait. */ export const enum SessionTypeAuthRequirement { /** Runs on the user's own credentials — usable while signed out of GitHub. */ @@ -53,9 +56,8 @@ export const enum SessionTypeAuthRequirement { GitHub = 'github', /** * Cannot run at all right now, and signing in to GitHub would not help — e.g. - * Claude pinned to native mode by an explicit `claudeUseCopilotProxy: false` - * with no local Claude credentials. Surfaces as "no models", not a sign-in - * prompt. + * Claude advertising the Copilot resource as optional but publishing an empty + * model catalog. Surfaces as "no models", not a sign-in prompt. */ Unusable = 'unusable', }