Ask the agent SDK about setup instead of guessing from the filesystem - #331739
Draft
TylerLeonhardt wants to merge 2 commits into
Draft
Ask the agent SDK about setup instead of guessing from the filesystem#331739TylerLeonhardt wants to merge 2 commits into
TylerLeonhardt wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Replaces filesystem-based agent setup detection with SDK-reported state and explicit, consented downloads.
Changes:
- Adds the SDK setup wire channel, consent handling, progress, and telemetry.
- Uses Claude/Codex SDK account and model discovery instead of filesystem checks.
- Adds localized setup banners, picker reachability, and coverage.
Show a summary per file
| File | Description |
|---|---|
src/vs/workbench/services/agentHost/test/browser/codexAccountService.test.ts |
Updates Codex service fixture. |
src/vs/workbench/services/agentHost/browser/codexAccountService.ts |
Exposes the managed agent ID. |
src/vs/workbench/services/agentHost/browser/agentSdkSetupService.ts |
Manages setup state, consent, requests, and telemetry. |
src/vs/workbench/contrib/chat/test/browser/widget/input/sessionTargetPickerActionItem.test.ts |
Tests setup-banner picker availability. |
src/vs/workbench/contrib/chat/test/browser/agentSessions/sessionTypeAvailability.test.ts |
Tests setup-based availability overrides. |
src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostSdkSetupNotification.test.ts |
Covers setup state, presentation, and telemetry funnel. |
src/vs/workbench/contrib/chat/browser/widget/input/sessionTargetPickerActionItem.ts |
Keeps setup-capable harnesses selectable. |
src/vs/workbench/contrib/chat/browser/widget/input/delegationSessionPickerActionItem.ts |
Forwards notification service dependency. |
src/vs/workbench/contrib/chat/browser/agentSessions/sessionTypeAvailability.ts |
Adds setup-aware availability and model helper. |
src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSdkSetupNotification.ts |
Implements the localized setup banner. |
src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHost.contribution.ts |
Registers the setup contribution. |
src/vs/sessions/test/browser/sessionsAuthGate.test.ts |
Removes obsolete nudge coverage. |
src/vs/sessions/contrib/providers/agentHost/test/browser/agentHostDiscoveredConfigNotification.test.ts |
Removes discovered-config tests. |
src/vs/sessions/contrib/providers/agentHost/browser/localAgentHost.contribution.ts |
Registers the new banner in Sessions. |
src/vs/sessions/contrib/providers/agentHost/browser/agentHostDiscoveredConfigNotification.ts |
Removes the filesystem-based nudge. |
src/vs/sessions/contrib/chat/test/browser/sessionTypePicker.test.ts |
Updates picker dependency setup. |
src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts |
Applies setup-aware picker availability. |
src/vs/sessions/contrib/chat/browser/mobile/mobileSessionTypePicker.ts |
Forwards the notification dependency on mobile. |
src/vs/sessions/browser/sessionsAuthGate.ts |
Removes discovered-config policy. |
src/vs/platform/agentHost/test/node/testAgentSdkDownloader.ts |
Adds a deterministic downloader test double. |
src/vs/platform/agentHost/test/node/codex/codexSessionTitleSpans.test.ts |
Uses the downloader test double. |
src/vs/platform/agentHost/test/node/codex/codexSessionConfigKeys.test.ts |
Uses the downloader test double. |
src/vs/platform/agentHost/test/node/codex/codexModelRefresh.test.ts |
Tests SDK-driven Codex setup and downloads. |
src/vs/platform/agentHost/test/node/codex/codexLocalAuth.test.ts |
Removes filesystem-auth tests. |
src/vs/platform/agentHost/test/node/codex/codexAgent.test.ts |
Tests deferred Codex discovery. |
src/vs/platform/agentHost/test/node/claudeTransportMode.test.ts |
Tests SDK account interpretation. |
src/vs/platform/agentHost/test/node/claudeSubagentResolver.test.ts |
Updates the SDK service contract. |
src/vs/platform/agentHost/test/node/claudeAgent.integrationTest.ts |
Updates the integration SDK stub. |
src/vs/platform/agentHost/test/node/agentSdkDownloadTelemetry.test.ts |
Tests failure classification. |
src/vs/platform/agentHost/test/node/agentSdkDownloader.test.ts |
Tests download telemetry and retries. |
src/vs/platform/agentHost/test/common/agentSdkSetup.test.ts |
Tests setup parsing and consent resolution. |
src/vs/platform/agentHost/node/codex/codexLocalAuth.ts |
Removes Codex filesystem sniffing. |
src/vs/platform/agentHost/node/codex/codexAgent.ts |
Publishes SDK setup and SDK-derived models. |
src/vs/platform/agentHost/node/claude/claudeTransportMode.ts |
Derives Claude setup from SDK account information. |
src/vs/platform/agentHost/node/claude/claudeAgentSdkService.ts |
Makes SDK download explicit. |
src/vs/platform/agentHost/node/claude/claudeAgent.ts |
Uses SDK account probing and deferred discovery. |
src/vs/platform/agentHost/node/agentSdkSetupChannel.ts |
Handles per-agent setup publication and requests. |
src/vs/platform/agentHost/node/agentSdkDownloadTelemetry.ts |
Adds download telemetry reporting. |
src/vs/platform/agentHost/node/agentSdkDownloader.ts |
Instruments downloads and explicit retries. |
src/vs/platform/agentHost/common/agentSdkSetup.ts |
Defines setup status, request, and consent contracts. |
Review details
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 41/41 changed files
- Comments generated: 4
- Review effort level: Balanced
| const initialState = this._agentHostService.rootState.value; | ||
| this._setups = readAgentSdkSetupInfos(initialState instanceof Error ? undefined : initialState); | ||
| this._applyConsent(); | ||
| this._register(this._agentHostService.rootState.onDidChange(state => this._updateSetups(readAgentSdkSetupInfos(state)))); |
| if (this._downloadInFlight) { | ||
| return; | ||
| } | ||
| const progressInterest = this._downloader.acquireDownloadProgressInterest(this._agent.sdkPackage); |
Comment on lines
+116
to
+119
| // 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(); |
Comment on lines
+112
to
+114
| export function agentSdkSetupSessionType(agent: string): string { | ||
| return `${LOCAL_AGENT_HOST_SCHEME_PREFIX}${agent}`; | ||
| } |
TylerLeonhardt
marked this pull request as ready for review
August 20, 2026 06:40
TylerLeonhardt
marked this pull request as draft
August 20, 2026 06:41
A developer who pays for Claude directly hit a GitHub Copilot sign-in wall, because session-type availability was inferred by sniffing for config files on disk. That guess was wrong in both directions: it gated users who had a working account, and it advertised agents to users who had none. Replace the inference with what the agent's own SDK reports, and make the SDK download an explicit choice rather than something that happens on startup. - Agents publish an SDK setup status (`notDownloaded` / `downloading` / `ready`) over the root config channel, plus the capabilities they offer for getting an account. The workbench derives "no account" from `ready` + zero models, so there is a single wire source per fact. - Agents declare capabilities only; every user-facing string is localized in the workbench via `vs/nls`. - The download is offered by a banner and performed on request. Consent is recorded per agent, so a later version bump re-downloads silently for that agent while a different agent still asks. - Background fetches stay invisible: only the explicit gesture registers download progress interest. - `AgentSdkSetupChannel` holds the nonce handling, in-flight latch and publish ordering once, so Claude and Codex differ only in their capability literals. Removes the filesystem-sniffing paths this replaces: `codexLocalAuth` and the "we discovered your existing configuration" notification, which asked users to sign in again after they had already declined the sign-in modal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Publish the SDK download status *after* the model catalog on both agents. Publishing `ready` at the top of `_refreshModels` meant the first refresh after a download announced "the SDK is here" while `_models` was still empty -- and `ready` plus zero models is exactly how the window renders "no account found". The invariant was already documented in the setup channel's own `_download()`; the refresh path contradicted it. Re-bind `AgentSdkSetupService` to root state on `onAgentHostStart`. `rootState` is a getter over a protocol client the host replaces on every restart and reconnect, so the single constructor-time subscription went quietly stale -- and because the service is `Delayed`, constructing before the connection bound the no-op state forever. Pending download requests are cleared on re-bind too: a request the previous host never answered never will be, so the Download button comes back rather than staying suppressed. Both fixes carry regression tests that were verified to fail without them. Also corrects the `explicitlyRequested` telemetry doc, which claimed to carry a click-vs-standing-consent split it does not have, and states the banner's ambient-host scope in `agentSdkSetupSessionType`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
TylerLeonhardt
force-pushed
the
agents/agent-sdk-download-setup-detection
branch
from
August 20, 2026 07:07
df76c7d to
6e18cea
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
A developer who pays for Claude directly hits a GitHub Copilot sign-in wall. Session-type availability was inferred by sniffing the filesystem for agent config files, and that guess is wrong in both directions: it gates users who have a perfectly good account, and it advertises agents to users who have none.
Separately, the agent SDK was downloaded on startup as a side effect of chat discovery — no one ever asked.
The change
Stop guessing. Ask the agent's own SDK, and make the download an explicit choice.
Wire contract. Each agent publishes an SDK setup status —
notDownloaded/downloading/ready— on a per-agent root-config key, alongside the capabilities it offers for getting an account (setupDocsUrl,signInProviderName). Account state is deliberately not on the wire: the workbench derives "no account" fromready+ zero models, which already flows over AHP. Two wire sources for one truth would be free to disagree.No user-facing strings from the host. Agents declare capabilities; the banner owns every sentence and localizes it via
vs/nls. What crosses the wire is proper nouns the workbench cannot invent (ChatGPT, the agent's own display name).Explicit download. A banner offers it; the workbench requests it by nonce. Consent is recorded per agent in application storage — pressing Download for Codex is not permission to fetch Claude's SDK. A later version bump re-downloads silently for a consented agent rather than nagging, since the cache is keyed by version and Insiders invalidates it daily.
Background fetches stay invisible. Only a setup-driven fetch registers download progress interest, so a fetch the user never asked for never surfaces a progress UI, and the host keeps reporting
notDownloadedfor it. Standing consent counts as setup-driven: a version bump re-downloads without a second question, but the user still sees why their agent is briefly unavailable.AgentSdkSetupChannelholds the nonce handling, in-flight latch and publish ordering once; Claude and Codex differ only in their capability literals.Removed
The filesystem-sniffing paths this replaces:
codexLocalAuth, and the "we've discovered your existing Claude configuration" notification — which asked users to sign in again right after they had declined the sign-in modal.Testing
product.json: download offered → clicked → progress → models enumerate → banner resolves, for both agents; plus the per-agent consent case (consented agent re-downloads silently on a version bump while the other still asks).Known gaps
A failed download is not yet surfaced to the user as an error — it republishes
notDownloaded, so the banner re-offers the button. Deferred deliberately.The banner is scoped to the window's ambient agent host — which is the remote server in a remote window, so those are covered. The Sessions app's additional connections (
remote-<authority>-<agent>) show no banner. That is pre-existing structure shared with the Copilot sign-in notification, which is scoped identically; sessions against those hosts still fetch the SDK on demand via the ordinary load path. Moving both banners to per-connection setup state is the follow-up.🤖 Generated with Claude Code