Skip to content

agentHost: remove the claudeUseCopilotProxy setting - #329650

Merged
Tyler James Leonhardt (TylerLeonhardt) merged 1 commit into
mainfrom
tyler/remove-claude-use-copilot-proxy
Aug 7, 2026
Merged

agentHost: remove the claudeUseCopilotProxy setting#329650
Tyler James Leonhardt (TylerLeonhardt) merged 1 commit into
mainfrom
tyler/remove-claude-use-copilot-proxy

Conversation

@TylerLeonhardt

@TylerLeonhardt Tyler James Leonhardt (TylerLeonhardt) commented Aug 7, 2026

Copy link
Copy Markdown
Member

Since #329331 the Claude model picker shows Copilot-proxy and native-Anthropic models together, each carrying a provider-qualified id (@provider=copilot:… / @provider=anthropic:…), and a session routes on the provider of the model it was started with (resolveClaudeSessionTransport). The host-global claudeUseCopilotProxy override is no longer needed.

Why it had to go — it was incoherent, not merely redundant

Model enumeration (_refreshModels) gates the proxy half of the catalog on holding a GitHub token and the native half on detectExistingClaudeSetupneither consulted the setting — and getProtectedResources() advertises the Copilot resource required: false unconditionally.

So claudeUseCopilotProxy: false did not stop Copilot-routed Claude from being offered or used by a signed-in user, which is precisely what its title ("Route Claude Through Copilot") promised it would do. A setting that contradicts its own description is worse than no setting.

What it still did, post-#329331, was decide the fallback for a session whose model names no provider (model-less, or a bare/legacy id) — and there it was only ever load-bearing in one state: opt-in on and signed out and a local Anthropic credential present, where an explicit true forced proxy and produced a dead-end AHP_AUTH_REQUIRED for a user who could have just worked. That is the state the signed-out feature exists to serve, so the override's only remaining effect was to break it.

Change

resolveClaudeTransportMode drops its explicitProxy input and reduces from five rules to four (three inputs):

opt-in off  => proxy
signed in   => proxy
local setup => native
otherwise   => proxy

Removal is a clean deletion, not a deprecation: the key was never forwarded from a VS Code setting (AgentHostRootConfigForwarder never managed it) and was only ever hand-written into agent-host-config.json.

Two intentional behavior changes

  1. Native-without-a-credential is no longer a reachable host default. Signed out with no local setup now falls back to proxy — the safe end, since attempting native with no credential fails inside the SDK rather than at a surface that can explain itself. Note the fallback is not what the window gate reads: Claude advertises the Copilot resource required: false unconditionally, so with neither half of the catalog enumerable the published catalog is empty and the type resolves to Unusable ("no models"). The proxy fallback only bites at use time, when a model-less/bare session materializes with no proxy handle and _ensureAuthenticated raises AHP_AUTH_REQUIRED.
  2. Unusable survives, on a different route — Copilot advertised as optional while publishing an empty model catalog (signed out, no local credential, so neither half of the merged catalog enumerates). The empty-catalog behaviour is unchanged; only the example that produces it moved.

Drive-by fix: mergeClaudeModelCatalogs documented the wrong default

Its JSDoc claimed proxy-first ordering "preserves the picker's models[0]-is-default convention for the common (Copilot) case." The e2e disproved this. The picker re-buckets the flat list by the _meta vendor token and renders group-by-group, so array order does not select the default — the Anthropic group sorts first and @provider=anthropic:default is pre-selected, which routes native. Comment corrected, with an explicit warning not to infer the default from the merge order.

Validation

npm run typecheck-client clean. 241 targeted unit tests passing (ClaudeAgent, Claude transport mode, AgentHostGitHubEndpointService, Agent Host - session type auth requirement).

End-to-end in the Agents window on a machine signed in to GitHub and holding local Anthropic credentials, so both halves of the catalog enumerate:

Check Result
Merged catalog with the setting gone Picker shows two vendor groups — Anthropic (Fable, Haiku, Opus 1M, Sonnet, Sonnet 5 1M) and Copilot (Claude Haiku 4.5 … Opus 5)
Native route @provider=anthropic:default → reply OK; log shows claude-opus-5[1m], no ClaudeProxyService POST /v1/messages → direct to Anthropic
Proxy route @provider=copilot:claude-opus-4.6 → reply OK; log shows [ClaudeProxyService] POST /v1/messages + a Copilot credits entry
Stale key upgrade Seeded claudeUseCopilotProxy: false (27 keys) into a profile → starts with no error; first config write emits 26 keys, that key the only difference. It self-cleans; no migration needed

Known gap (follow-up, not a regression)

No lever remains to express a transport preference, and the picker's default is not obviously the safe one — the pre-selected model routes native and bills the user's own Anthropic account (a one-word reply cost $0.38 on claude-opus-5[1m]).

The residual gap is not "everyone gets Copilot"; it is that whichever vendor group sorts first silently decides who pays. The fix is a default/sticky model preference (adjacent to #323049), not a transport flag — the transport is downstream of the model, and a flag would once again disagree with what the picker shows. That reasoning is recorded on resolveClaudeTransportMode.

Note on the docs

The rationale here previously lived in two markdown files under src/vs/sessions/ (an auth glossary and an ADR). They were never committed upstream, and they had already drifted from the code. This PR keeps the durable half and puts it on the code it constrains instead:

  • SessionTypeAuthRequirement — why it is three states and not a boolean.
  • resolveAgentAuthRequirement — why auth is derived from advertised protected resources rather than a static flag, why required: false beats omitting the resource, and why the model count is the load-bearing second half.
  • _refreshModels — why gating the native catalog is load-bearing, not an optimization: supportedModels() is a static list that answers with no credentials at all, so an empty catalog is the honest signal.
  • sessionsAuthGate.ts — window gate vs per-type gate.
  • _mustForceGitHubSignIn — the window gate as a deliberate last resort.
  • resolveClaudeTransportMode — why no host-global transport preference exists.

🤖 Generated with Claude Code

Copilot AI balanced review requested due to automatic review settings August 7, 2026 17:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Removes the obsolete host-wide Claude proxy setting now that transport is selected per session from the model provider.

Changes:

  • Removes claudeUseCopilotProxy from configuration and transport resolution.
  • Updates Claude transport and authentication tests.
  • Adds and updates authentication decision documentation.
Show a summary per file
File Description
src/vs/platform/agentHost/common/agentHostCustomizationConfig.ts Removes the configuration key and schema.
src/vs/platform/agentHost/node/claude/claudeAgent.ts Removes the explicit proxy override lookup.
src/vs/platform/agentHost/node/claude/claudeTransportMode.ts Reduces fallback resolution to three inputs.
src/vs/platform/agentHost/test/node/claudeAgent.test.ts Updates Claude routing tests and fixtures.
src/vs/platform/agentHost/test/node/claudeTransportMode.test.ts Updates the transport decision matrix.
src/vs/platform/agentHost/test/node/agentHostGitHubEndpointService.test.ts Replaces the removed test configuration key.
src/vs/sessions/services/sessions/common/session.ts Updates Unusable documentation.
src/vs/sessions/contrib/providers/agentHost/test/browser/sessionTypeAuthRequirement.test.ts Updates merged-catalog test context.
src/vs/sessions/CONTEXT.md Adds an authentication glossary.
src/vs/sessions/docs/adr/0001-conditional-agent-window-auth.md Adds the authentication decision record and amendments.

Review details

  • Files reviewed: 10/10 changed files
  • Comments generated: 8
  • Review effort level: Balanced

Comment thread src/vs/platform/agentHost/node/claude/claudeTransportMode.ts Outdated
Comment thread src/vs/sessions/docs/adr/0001-conditional-agent-window-auth.md Outdated
Comment thread src/vs/sessions/docs/adr/0001-conditional-agent-window-auth.md Outdated
Comment thread src/vs/sessions/CONTEXT.md Outdated
Comment thread src/vs/sessions/CONTEXT.md Outdated
Comment thread src/vs/sessions/CONTEXT.md Outdated
Comment thread src/vs/sessions/CONTEXT.md Outdated
Comment thread src/vs/sessions/CONTEXT.md Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review details

Suppressed comments (9)

src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts:285

  • This state is now recoverable by GitHub sign-in: signed-out Claude with no local setup advertises an optional Copilot resource and zero models, but authenticate() starts the proxy and refreshes the Copilot catalog. Classifying that state as Unusable suppresses the per-type sign-in affordance (getSessionTypeAvailability sees required: false and reports “No models”), notably when another native provider has already opened the window. Represent the empty, proxy-recoverable state as GitHub or carry an explicit recoverability signal instead.
 * 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.

src/vs/platform/agentHost/common/agentHostCustomizationConfig.ts:23

  • The setting remains documented throughout node/claude/phase18-plan.md, phase19-plan.md, and roadmap.md, including instructions to set it and assertions that it controls the current transport. Removing the schema while leaving those source documents unchanged makes the clean deletion incomplete and sends maintainers toward a nonexistent key; remove the obsolete plans or clearly update them for per-session provider routing.
	DefaultShell = 'defaultShell',
	/**

src/vs/sessions/browser/sessionsAuthGate.ts:35

  • This file-level block is much longer than the repository's 1–2 sentence JSDoc convention and duplicates details already documented on the individual predicates below. Keep only the distinction between the window and per-type gates so the implementation remains scannable.
/**
 * 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:

src/vs/sessions/services/sessions/common/session.ts:47

  • This enum JSDoc exceeds the repository's 1–2 sentence limit and embeds provider implementation details that are already drifting from the enum contract. Keep the type-level documentation to the contract and document provider-specific derivation at its implementation.
/**
 * 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

src/vs/platform/agentHost/node/claude/claudeTransportMode.ts:33

  • This function JSDoc grows into a full decision record, exceeding the repository's 1–2 sentence JSDoc convention and making a four-branch pure function harder to scan. Keep the API contract here and retain longer architectural rationale in a dedicated design document.
/**
 * Which transport should the Claude provider fall back to right now? Pure
 * decision; precedence, highest first:
 *
 *  1. Feature flag off means today's default behavior (always proxy).

src/vs/platform/agentHost/node/claude/claudeModelSelection.ts:158

  • The newly added picker-default narrative substantially extends an already long JSDoc and records an end-to-end observation that can drift with picker sorting. Keep only the invariant relevant to this merge function: input order does not determine the rendered default.
 * 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

src/vs/platform/agentHost/node/claude/claudeAgent.ts:689

  • This added paragraph narrates downstream UI behavior and exceeds the repository's 1–2 sentence JSDoc convention. The load-bearing constraint can be stated directly without embedding the full auth-state rationale here.
	 * 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

src/vs/platform/agentHost/test/node/claudeAgent.test.ts:903

  • This helper JSDoc narrates setup and cleanup mechanics that are already obvious from the implementation, exceeding the repository's 1–2 sentence convention. A short purpose statement is sufficient.
/**
 * 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.

src/vs/sessions/browser/sessionsSetUpService.ts:277

  • This method JSDoc expands into a multi-paragraph design rationale, contrary to the repository's 1–2 sentence JSDoc convention. The method name and called observable already convey the last-resort behavior; keep the contract concise.
	/**
	 * 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
  • Files reviewed: 12/12 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Since #329331 the Claude model picker shows Copilot-proxy and native-Anthropic
models together, each carrying a provider-qualified id, and a session routes on
the provider of the model it was started with (resolveClaudeSessionTransport).
That makes the host-global override redundant — and, worse, incoherent.

Model enumeration (_refreshModels) gates the proxy half on holding a GitHub
token and the native half on detectExistingClaudeSetup; neither consulted the
setting. getProtectedResources() advertises the Copilot resource required:false
unconditionally. So `claudeUseCopilotProxy: false` never stopped Copilot-routed
Claude from being offered or used by a signed-in user, which is exactly what its
title ("Route Claude Through Copilot") promised. Its only remaining effect was
on the model-less fallback, where an explicit `true` broke the one state the
signed-out feature exists to serve: opt-in on, signed out, local Anthropic
credential present — forced to proxy and dead-ended on AHP_AUTH_REQUIRED.

resolveClaudeTransportMode drops its explicitProxy input and reduces to three:
opt-in off => proxy; signed in => proxy; else local setup => native; else proxy.

Clean deletion, not a deprecation: the key was never forwarded from a VS Code
setting (AgentHostRootConfigForwarder never managed it) and was only ever
hand-written into agent-host-config.json. A profile carrying a stale value
starts with no error and drops the key on the first config write.

Also corrects mergeClaudeModelCatalogs' claim that proxy-first ordering makes
Copilot the session default. It does not: the picker re-buckets by the _meta
vendor token, so the Anthropic group sorts first and the pre-selected model
routes native. Verified end-to-end.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@vs-code-engineering

Copy link
Copy Markdown
Contributor

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

Sandeep Somavarapu (@sandy081)

Matched files:

  • src/vs/sessions/services/sessions/common/session.ts

Ladislau Szomoru (@lszomoru)

Matched files:

  • src/vs/sessions/services/sessions/common/session.ts

@TylerLeonhardt
Tyler James Leonhardt (TylerLeonhardt) merged commit ca6ea1b into main Aug 7, 2026
29 checks passed
@TylerLeonhardt
Tyler James Leonhardt (TylerLeonhardt) deleted the tyler/remove-claude-use-copilot-proxy branch August 7, 2026 18:31
@vs-code-engineering vs-code-engineering Bot added this to the 1.133.0 milestone Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants