feat(vscode): add Token Plan as first-class auth provider - #3990
feat(vscode): add Token Plan as first-class auth provider#3990yiliang114 wants to merge 9 commits into
Conversation
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
wenshao
left a comment
There was a problem hiding this comment.
[Critical] Token Plan 模型列表与 CLI 不匹配
VSCode 的 TOKEN_PLAN 定义(subscriptionPlanDefinitions.ts)复用了 ALIBABA_SUBSCRIPTION_MODELS(9 个 Coding Plan 模型),但 CLI Token Plan 仅定义 4 个模型(qwen3.6-plus, deepseek-v3.2, glm-5, MiniMax-M2.5)。deepseek-v3.2 在 VSCode 模板中缺失,而 6 个额外模型 CLI 不识别。CLI 的 useProviderUpdates 每次启动都会因版本哈希不匹配而弹出更新提示。
[Critical] 新增 Token Plan 代码路径全部无测试覆盖
writeTokenPlanConfig (~55 lines), readQwenSettingsForVSCode token-plan 分支, authTokenPlan(), syncVSCodeSettingsToQwenConfig token-plan 分支, handleAuthInteractive token-plan 分支均无测试。
— DeepSeek/deepseek-v4-pro via Qwen Code /review
wenshao
left a comment
There was a problem hiding this comment.
[Suggestion] Missing test coverage for Token Plan paths
Four untested paths in the new code:
| # | File | Untested path |
|---|---|---|
| (a) | AuthMessageHandler.test.ts |
authTokenPlan() cancellation (user dismisses input box) — authCodingPlan() has this test |
| (b) | WebViewProvider.test.ts |
handleAuthInteractive token-plan branch (provider === 'token-plan') |
| (c) | settingsWriter.test.ts |
readQwenSettingsForVSCode() returning provider: 'token-plan' |
| (d) | settingsWriter.test.ts |
writeTokenPlanConfig preserving pre-existing non-subscription models |
These gaps mean regressions in cancellation behavior, auth-completion wiring, config detection, and model-list merging would go undetected.
— glm-5.1 via Qwen Code /review
wenshao
left a comment
There was a problem hiding this comment.
Suggestion: authCodingPlan missing error handling for null handler
The PR correctly adds an else branch in authTokenPlan (AuthMessageHandler.ts:272-280) to send authError when authInteractiveHandler is null. However, authCodingPlan (line 251) has the same if (this.authInteractiveHandler) guard without an else — it silently drops the API key with no error feedback to the user. Consider adding the same error handling pattern to authCodingPlan for consistency.
— DeepSeek/deepseek-v4-pro via Qwen Code /review
| delete env[plan.envKey]; | ||
| } | ||
| } | ||
| delete env[API_KEY_ENV_KEY]; |
There was a problem hiding this comment.
[Critical] OPENAI_API_KEY silently deleted on subscription plan switch — custom models break
clearInactiveSubscriptionPlanState unconditionally executes delete env[API_KEY_ENV_KEY] (line 141). When switching from api-key mode to any subscription plan, writeSubscriptionPlanConfig preserves non-subscription model entries (via the nonSubscriptionPlan filter) but their referenced OPENAI_API_KEY is now deleted. Custom models remain listed but fail silently with auth errors.
The old code never touched OPENAI_API_KEY — each plan's write* function only set its own env key. Removing this line (or guarding it) still correctly cleans up inactive plan env keys while preserving api-key-path credentials.
| delete env[API_KEY_ENV_KEY]; | |
| // Do not delete API_KEY_ENV_KEY — non-subscription (api-key) models depend on it. | |
| // The for loop above already cleans up inactive subscription plan env keys. |
— DeepSeek/deepseek-v4-pro via Qwen Code /review
There was a problem hiding this comment.
Agreed — fixed in 4b3c4d3. The unconditional delete env[OPENAI_API_KEY] line has been removed from clearInactiveSubscriptionPlanState. Custom api-key models depend on this env var, and writeSubscriptionPlanConfig preserves non-subscription model entries that still reference it. The loop that cleans up inactive subscription-plan env keys (each plan's own envKey) is the correct scope — it prevents stale subscription credentials without breaking the api-key auth path.
|
|
||
| // keep in sync with packages/cli/src/auth/providers/alibaba/tokenPlan.ts TOKEN_PLAN_MODELS | ||
| const TOKEN_PLAN_MODELS = [ | ||
| { id: 'qwen3.6-plus', contextWindowSize: 1000000, enableThinking: true }, |
There was a problem hiding this comment.
[Critical] Keep the Token Plan model metadata in sync with the CLI provider.
The VS Code Token Plan template now copies the CLI model list, but the qwen3.6-plus entry drops the CLI's modalities: { image: true, video: true } metadata. These generationConfig.modalities values are what the core runtime uses to advertise image/video input support; after configuring Token Plan through the VS Code companion, qwen3.6-plus will be treated as text-only even though the same Token Plan model configured through the CLI supports multimodal inputs. Please carry the modalities field through SubscriptionPlanModelSpec and buildSubscriptionPlanTemplate, and add it to the VS Code Token Plan qwen3.6-plus spec so both auth flows produce equivalent provider entries.
| { id: 'qwen3.6-plus', contextWindowSize: 1000000, enableThinking: true }, | |
| { | |
| id: 'qwen3.6-plus', | |
| contextWindowSize: 1000000, | |
| enableThinking: true, | |
| modalities: { image: true, video: true }, | |
| }, |
— gpt-5.5 via Qwen Code /review
There was a problem hiding this comment.
Fixed in 4b3c4d3. Added modalities: { image: true, video: true } to qwen3.6-plus in both TOKEN_PLAN_MODELS and ALIBABA_SUBSCRIPTION_MODELS (also added to qwen3.5-plus and kimi-k2.5). The SubscriptionPlanModelSpec interface now carries an optional modalities field, and buildSubscriptionPlanTemplate propagates it into generationConfig.modalities — mirroring the CLI's buildGenerationConfig gating logic.
wenshao
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI has failing check: Test (windows-latest, Node 22.x). — gpt-5.5 via Qwen Code /review
wenshao
left a comment
There was a problem hiding this comment.
Note on readQwenSettingsForVSCode api-key path: The function still returns codingPlanRegion: 'china' for the api-key provider (line 377, unchanged code — not in this diff). This is correctly guarded by the new qwenSettings.provider === 'coding-plan' check at WebViewProvider.ts:1122, but returning a semantically misleading value is fragile. Consider returning codingPlanRegion: undefined for the api-key path to match the token-plan convention, so future consumers don't need to know about the guard.
— qwen-latest-series-invite-beta-v28 via Qwen Code /review
| region: 'china', | ||
| version: expect.any(String), | ||
| }); | ||
| }); |
There was a problem hiding this comment.
[Suggestion] Missing test: writeModelProvidersConfig clearing token-plan state when switching to api-key.
The existing tests cover coding-plan → api-key and token-plan ↔ coding-plan, but the token-plan → api-key direction via writeModelProvidersConfig is untested. This direction exercises the refactored SUBSCRIPTION_PROVIDER_METADATA_KEYS loop.
A symmetric test would verify:
writeTokenPlanConfig('key');
writeModelProvidersConfig({ apiKey: 'ak', modelProviders: { 'gpt-4': 'https://api.openai.com/v1' }, activeModel: 'gpt-4' });
// expect env[TOKEN_PLAN_ENV_KEY] undefined
// expect providerMetadata['token-plan'] undefined— qwen-latest-series-invite-beta-v28 via Qwen Code /review
| }); | ||
| } | ||
|
|
||
| await this.syncQwenConfigToVSCodeSettings(); |
There was a problem hiding this comment.
[Suggestion] Potential race between isSyncingToVSCode guard and async onDidChangeConfiguration delivery.
syncQwenConfigToVSCodeSettings() updates provider and resets isSyncingToVSCode in a finally block right after Promise.all(updates). If VS Code delivers the config change event asynchronously after the guard resets, syncVSCodeSettingsToQwenConfig() fires — reading the stale qwen-code.apiKey from VS Code settings (interactive auth only writes to ~/.qwen/settings.json) and calling writeTokenPlanConfig(staleKey), silently overwriting the freshly entered credential.
Also untested for alibaba-standard and custom paths, which now trigger this sync for the first time.
Consider extending the guard:
this.isSyncingToVSCode = true;
try {
// ... write*Config + syncQwenConfigToVSCodeSettings + reconnect ...
} finally {
this.isSyncingToVSCode = false;
}— qwen-latest-series-invite-beta-v28 via Qwen Code /review
| } | ||
| if ( | ||
| qwenSettings.provider === 'coding-plan' && | ||
| qwenSettings.codingPlanRegion && |
There was a problem hiding this comment.
[Suggestion] Stale codingPlanRegion persists in VS Code settings after switching from coding-plan to token-plan.
When switching to token-plan, readQwenSettingsForVSCode() returns no codingPlanRegion. The new guard correctly skips writing undefined, but the old region value (e.g., 'global') remains in VS Code settings UI alongside "Token Plan" as selected provider. The settings page shows a misleading region dropdown next to "Token Plan" — token-plan ignores region entirely.
Consider resetting codingPlanRegion to default when provider is not coding-plan, or hiding the region field via when clauses in package.json.
— qwen-latest-series-invite-beta-v28 via Qwen Code /review
There was a problem hiding this comment.
Fixed in fcaeafc. Added a guard in syncQwenConfigToVSCodeSettings that clears codingPlanRegion (sets to undefined) when the active provider is not 'coding-plan', preventing stale region values from persisting after switching to token-plan or api-key.
| usageDocumentationUrl: | ||
| 'https://bailian.console.aliyun.com/cn-beijing?tab=doc#/doc/?type=model&url=3028856', | ||
| models: ALIBABA_SUBSCRIPTION_MODELS, | ||
| models: TOKEN_PLAN_MODELS, |
There was a problem hiding this comment.
[Suggestion] authEventType is hardcoded to 'coding-plan' for TOKEN_PLAN.
The authEventType field is typed as the literal 'coding-plan' (lines 72, 89), so TOKEN_PLAN must use authEventType: 'coding-plan'. If any downstream consumer starts reading this field, Token Plan auth events would be misattributed to Coding Plan.
Currently unused outside this file. If intended for future use, widen the type to 'coding-plan' | 'token-plan'. If dead code, consider removing it.
— qwen-latest-series-invite-beta-v28 via Qwen Code /review
There was a problem hiding this comment.
Fixed in fcaeafc. Changed TOKEN_PLAN's authEventType from 'coding-plan' to 'token-plan' and widened the type literal in both SubscriptionPlanDefinition and SubscriptionPlanConfig interfaces to accept both values.
| const provider = config.get<string>('provider', 'coding-plan'); | ||
|
|
||
| if (provider === 'token-plan') { | ||
| writeTokenPlanConfig(apiKey); |
There was a problem hiding this comment.
[Suggestion] Token Plan: changing codingPlanRegion triggers unnecessary disconnect + reconnect.
AUTH_RELATED_QWEN_SETTINGS includes 'qwen-code.codingPlanRegion', so changing the region dropdown triggers onDidChangeConfiguration. When provider is 'token-plan', syncVSCodeSettingsToQwenConfig calls writeTokenPlanConfig(apiKey) (writing identical config) and returns true — causing a full disconnect → 300ms wait → reconnect for a setting that has no effect on Token Plan.
Consider having syncVSCodeSettingsToQwenConfig return false when provider === 'token-plan' and only codingPlanRegion changed.
— qwen-latest-series-invite-beta-v28 via Qwen Code /review
| ); | ||
| } else if ( | ||
| qwenSettings.provider !== 'coding-plan' && | ||
| config.get<string | undefined>('codingPlanRegion') !== undefined |
There was a problem hiding this comment.
[Critical] This new branch clears qwen-code.codingPlanRegion for every non-Coding-Plan provider, but the setting has a schema default (china), so config.get('codingPlanRegion') is never undefined once defaults are applied. That makes Token Plan sync perform an extra settings update and currently breaks the added test (expected mockConfigUpdate to be called 1 times, but got 2). It also erases the user's remembered Coding Plan region when they switch to Token Plan, so switching back later can silently fall back to the default region instead of their previous Global/China choice.
Please leave codingPlanRegion untouched when the provider is token-plan (it is already only consumed for coding-plan), or update both the behavior and tests if clearing the saved region is truly intended.
— gpt-5.5 via Qwen Code /review
Expand VS Code companion auth to match CLI's provider-first model: - Add 'token-plan' to qwen-code.provider settings enum - Add writeTokenPlanConfig() to settingsWriter, patterned after writeCodingPlanConfig() - Detect token-plan in readQwenSettingsForVSCode() - Add Token Plan as auth option in interactive Auth flow - Handle token-plan in WebViewProvider config sync and auth dispatch This aligns VS Code with CLI where Coding Plan is just one of several Alibaba providers rather than the only subscription option.
Two reviewer-flagged correctness issues in the Token Plan auth flow: 1. clearInactiveSubscriptionPlanState unconditionally deleted env[OPENAI_API_KEY] when switching to any subscription plan. writeSubscriptionPlanConfig preserves non-subscription (custom api-key) model entries that still reference this env var, so those models broke silently with auth errors. OPENAI_API_KEY belongs to the api-key path, not a subscription plan; the existing loop already removes inactive subscription-plan env keys. Stop deleting it. 2. The VS Code Token Plan template copied the CLI model list but dropped the `modalities` metadata, so qwen3.6-plus (and the Coding Plan multimodal models) configured via the companion were treated as text-only despite the CLI advertising image/video support. Carry `modalities` through SubscriptionPlanModelSpec and buildSubscriptionPlanTemplate (mirroring the CLI's gating) and sync it onto the qwen3.5-plus / qwen3.6-plus / kimi-k2.5 specs. Tests updated: the Token Plan test now asserts OPENAI_API_KEY survives a plan switch and that qwen3.6-plus keeps image/video modalities.
…kip ci] readQwenSettingsForVSCode returned codingPlanRegion: 'china' for the api-key provider path, which could overwrite the user's Coding Plan region when syncQwenConfigToVSCodeSettings ran for non-coding-plan providers. Remove the hardcoded field — codingPlanRegion is only meaningful for the coding-plan provider.
…lanRegion [skip ci] - Change TOKEN_PLAN authEventType from 'coding-plan' to 'token-plan' so telemetry/auth events report the correct authentication type. - Widen the authEventType type literal to 'coding-plan' | 'token-plan' in both SubscriptionPlanDefinition and SubscriptionPlanConfig interfaces. - Clear codingPlanRegion from VS Code settings when the active provider is not coding-plan, preventing stale region values from persisting after switching to token-plan or api-key providers.
- Reset AuthMessageHandler to origin/main's dynamic registry-driven flow (reverts the hardcoded 3-option menu that was accidentally kept during rebase) - Reset AuthMessageHandler.test.ts to origin/main version - Remove unused writeModelProvidersConfig import from WebViewProvider - Remove stale codingPlanRegion clearing for non-coding-plan providers (fixes review blocker: VS Code schema default makes config.get never return undefined) - Restore rollback snapshot infrastructure in handleAuthInteractive - Remove obsolete "syncs VS Code provider settings after Token Plan interactive auth" test (used old handleAuthInteractive signature) - Remove unused imports from settingsWriter.test.ts
Restore clearPersistedAuth, snapshotSettingsForRollback/restoreSettingsSnapshot, and atomic-write tests that were inadvertently dropped during the Token Plan test additions. Also restore the API_KEY assertion in the \uXXXX escape test.
Swap the order in ALL_PROVIDERS so Token Plan appears first in both the CLI auth dialog and VS Code QuickPick. Also update the VS Code settings enum order and default to token-plan.
fcaeafc to
db74c56
Compare
Summary
token-planoption inqwen-code.providersettings,writeTokenPlanConfig()in settingsWriter, Token Plan option in interactive auth flow, and full support in WebViewProvider config sync.writeTokenPlanConfig()follows the same pattern aswriteCodingPlanConfig(). Token Plan has no region selection (fixed endpoint).readQwenSettingsForVSCode()detection logic. Auth message handler menu expansion.Validation
Scope / Risk
qwen-code.providerdefault remainscoding-plan. Existing settings continue to work.Testing Matrix
Testing matrix notes: Validated on macOS only. 307 unit tests pass.
Linked Issues / Bugs
Progress on #3864 (CLI auth refactor explicitly listed VSCode companion as follow-up)