docs(auth): add custom API key wizard PRD#3583
Merged
Merged
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Contributor
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
reviewed
Apr 26, 2026
| │ "modelProviders": { │ | ||
| │ "openai": [ │ | ||
| │ { │ | ||
| │ "id": "qwen/qwen3-coder", │ |
Collaborator
There was a problem hiding this comment.
[Suggestion] The OpenAI review-screen JSON example follows a two-model input example (qwen/qwen3-coder,openai/gpt-4.1) but shows only one modelProviders.openai entry. Later sections require one entry per normalized model ID, so this can mislead the implementation or tests about whether the preview should display all normalized models or only the selected/default model.
Suggested change
| │ "id": "qwen/qwen3-coder", │ | |
| │ "openai": [ │ | |
| │ { │ | |
| │ "id": "qwen/qwen3-coder", │ | |
| │ "name": "qwen/qwen3-coder", │ | |
| │ "baseUrl": "https://openrouter.ai/api/v1", │ | |
| │ "envKey": "QWEN_CUSTOM_API_KEY_OPENAI_HTTPS_OPENROUTER_AI_API_V1"│ | |
| │ }, │ | |
| │ { │ | |
| │ "id": "openai/gpt-4.1", │ | |
| │ "name": "openai/gpt-4.1", │ | |
| │ "baseUrl": "https://openrouter.ai/api/v1", │ | |
| │ "envKey": "QWEN_CUSTOM_API_KEY_OPENAI_HTTPS_OPENROUTER_AI_API_V1"│ | |
| │ } │ | |
| │ ] │ |
— gpt-5.5 via Qwen Code /review
wenshao
requested changes
May 3, 2026
| .toUpperCase() | ||
| .replace(/[^A-Z0-9]+/g, '_') | ||
| .replace(/_+/g, '_') | ||
| .replace(/^_+|_+$/g, ''); |
Collaborator
There was a problem hiding this comment.
[Critical] generateCustomApiKeyEnvKey 的规范化函数将所有非字母数字字符替换为 _ 并合并连续下划线，这会导致不同的 URL 产生相同的 envKey，造成静默配置覆盖。
例如 https://my-api.com/v1 和 https://my.api.com/v1 都会规范化为 HTTPS_MY_API_COM_V1。合并规则（envKey !== generatedEnvKey 的条目被静默替换）放大了该问题：配置第二个端点会覆盖第一个端点的 API key 和模型条目，用户不会收到任何警告。
Suggested change
| .replace(/^_+|_+$/g, ''); | |
| const normalize = (value: string) => | |
| value | |
| .trim() | |
| .toUpperCase() | |
| // 保留 URL 中的关键分隔符（. - /），避免不同 URL 产生相同 envKey | |
| .replace(/[^A-Z0-9.\-/]+/g, '_') | |
| .replace(/_+/g, '_') | |
| .replace(/^_+|_+$/g, ''); |
或使用哈希方案：对完整 protocol + baseUrl 做 SHA-256 前 16 字符作为后缀。
— deepseek-v4-pro via Qwen Code /review
| ] | ||
| security.auth.selectedType = selectedProtocol | ||
| model.name = firstModelId | ||
| reloadModelProvidersConfig() |
Collaborator
There was a problem hiding this comment.
[Suggestion] Step 6 在认证之前就将设置写入 settings.json。认证失败后，设置已持久化到磁盘，但文档未定义回滚或恢复路径。
建议：先写入临时位置，认证成功后再提交；或至少定义认证失败时的回滚策略，避免留下损坏的配置。
— deepseek-v4-pro via Qwen Code /review
| reloadModelProvidersConfig() | ||
| refreshAuth(selectedProtocol) | ||
| ``` | ||
|
|
Collaborator
There was a problem hiding this comment.
[Suggestion] model.name = firstModelId 无条件覆盖当前活跃模型，没有确认或警告。用户可能只是想通过向导添加一个备用端点，而非切换模型。特别是当用户已通过 Coding Plan 配置了模型时，此操作会静默切换到自定义模型。
建议：在审查界面添加提示「这将把活跃模型切换为 X，是否继续？」，或仅在用户之前未配置模型时才设置 model.name。
— deepseek-v4-pro via Qwen Code /review
| │ ├─ Review generated JSON | ||
| │ └─ Save + authenticate | ||
| │ | ||
| └─ Qwen OAuth |
Collaborator
There was a problem hiding this comment.
[Suggestion] Non-goal 3 声明「不添加 advanced generationConfig…」，但 main 分支的已实现代码（AuthDialog.tsx）已经包含 custom-advanced-config 视图（Step 5/6，含「Enable thinking」和「Enable modality」）。PRD 的状态机和视图级别列表中均缺少此步骤。
建议：在 PRD 中补充 custom-advanced-config 步骤的文档，或明确说明此步骤将在后续版本中移除，使规范与实现保持一致。
— deepseek-v4-pro via Qwen Code /review
| baseUrl | ||
| -> trim | ||
| -> uppercase | ||
| -> replace every non A-Z / 0-9 character with _ |
Collaborator
There was a problem hiding this comment.
[Suggestion] 规范化规则文本描述与伪代码不一致。文本（第 531-538 行）规定 protocol 只做 trim → uppercase → replace，不做合并下划线和去首尾下划线；但伪代码（第 544-550 行）对 protocol 和 baseUrl 统一应用了完整的 normalize()（含 collapse 和 trim edges）。当前三个协议（openai/anthropic/gemini）没有特殊字符，结果碰巧一致。未来添加含连字符的协议（如 vertex-ai）时会暴露歧义。
建议：统一文本和伪代码，明确 protocol 是否应包含 collapse + trim 步骤。
— deepseek-v4-pro via Qwen Code /review
| Model IDs: qwen/qwen3-coder,openai/gpt-4.1 | ||
| ``` | ||
|
|
||
| The wizard should produce: |
Collaborator
There was a problem hiding this comment.
[Suggestion] Process env sync 节使用同步风格伪代码，但 Node.js 文件 I/O 本质上是异步的。应明确要求 await 写入完成后再执行 process.env[generatedEnvKey] = apiKey 和 refreshAuth(selectedProtocol)，否则可能导致认证使用到未刷盘的凭据。
Suggested change
| The wizard should produce: | |
| After writing settings.json.env[generatedEnvKey], immediately sync: | |
| await writeSettings(...) | |
| process.env[generatedEnvKey] = apiKey |
— deepseek-v4-pro via Qwen Code /review
| - Entries under other `modelProviders` protocol keys are preserved. | ||
|
|
||
| ### Authentication | ||
|
|
Collaborator
There was a problem hiding this comment.
[Suggestion] 测试计划缺少以下关键场景：
- 输入验证错误路径（空 baseUrl、非 http(s) 前缀、空 API key、空 model IDs）
- Esc 逐级后退导航链（review → model-id → api-key → base-url → protocol → type-select）
- envKey 生成的边界情况（尾部斜杠、特殊字符、端口号）
- settings.json 写入失败时的恢复行为
- 现有流程（Coding Plan、ModelStudio Standard）的回归测试
建议：在「Tests」章节补充上述场景。
— deepseek-v4-pro via Qwen Code /review
| - Base URL is compatible with the selected protocol | ||
| - API key is valid for this endpoint | ||
| - Model ID exists for this provider | ||
| ``` |
Collaborator
There was a problem hiding this comment.
[Suggestion] 认证失败后的恢复 UX 未明确指定：
- 用户回落到哪个界面？（审查界面？API key 输入？还是回到第一步？）
- 已输入的值是否保留以便编辑重试？
- 重试提交是否会正确覆盖已持久化的（可能错误的）设置？
建议：明确指定失败后的目标界面、状态保留策略和重试行为。
— deepseek-v4-pro via Qwen Code /review
wenshao
reviewed
May 8, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Meta concern (most important)
The implementation has already shipped to main. Verified against current code:
packages/cli/src/auth/providers/custom/customProvider.ts—customProvider+generateCustomEnvKeyexist with the PRD'sQWEN_CUSTOM_API_KEY_…prefix.packages/cli/src/auth/install/applyProviderInstallPlan.ts:110anduseAuth.ts:166,420,567,853— wire upsecurity.auth.selectedType.useAuth.ts:902-905— explicit comment that auth ordering was intentionally reversed vs. the PRD (see inline drift comment).
The PR has been static since 2026-05-03 with CHANGES_REQUESTED. Since the feature has shipped, this is now a retro-spec and disagrees with reality in several specific places. Two sensible paths:
- Reconcile the PRD with shipped behavior before merge so it serves as architecture documentation. Add a
> Status: implemented in <commit-sha>line at the top so the doc's role is unambiguous. - Close without merge and rely on code/tests as source of truth.
Merging as-is creates the worst outcome: an authoritative-looking design doc that contradicts shipped code.
Drift highlights (left as inline comments)
- §Updated
/authtree (lines 124-152): wizard is now top-levelCustom ProviderwithuiGroup: 'custom', not nested underAPI Key → Custom API Key. - §Non-goals #3 (line 92): contradicts shipped
customProvider.ts:43 showAdvancedConfig: true(thinking / multimodal / contextWindow / maxTokens steps actually exist). - §Step 6 Save and Authenticate (line 447): shipped does the opposite ordering (auth first, persist
selectedTypeonly on success) — seeuseAuth.ts:902-905.
Standing prior-review issues
The 9 inline comments from the previous /review pass remain unresolved (env-key collision at L548, auto-overwrite of model.name at L463, failed-auth recovery at L480, normalization rule mismatch at L531, etc.). If reconciling the PRD, just describe what shipped does today rather than re-litigating each.
Smaller PRD-internal note
- Lines 273-278: PRD's
id === nameinvariant comes fromcustomProvider.modelNamePrefix: ''flowing throughspecToModelConfig(providerConfig.ts:201-208), not a special code path. Worth saying so a future reader doesn't assume it's enforced elsewhere. - §Implementation Notes (line 739): shipped id is
custom-openai-compatible, but the provider supports OpenAI/Anthropic/Gemini protocols. PRD doesn't specify the id; if the doc is reconciled, recommend renaming tocustomor noting the historical naming.
Comment on lines
+124
to
+152
| ### Updated `/auth` tree | ||
|
|
||
| ```text | ||
| /auth | ||
| └─ Select Authentication Method | ||
| ├─ Alibaba Cloud Coding Plan | ||
| │ └─ Select Region | ||
| │ └─ Enter API Key | ||
| │ └─ Save + authenticate | ||
| │ | ||
| ├─ API Key | ||
| │ └─ Select API Key Type | ||
| │ ├─ Alibaba Cloud ModelStudio Standard API Key | ||
| │ │ ├─ Select Region | ||
| │ │ ├─ Enter API Key | ||
| │ │ ├─ Enter Model IDs | ||
| │ │ └─ Save + authenticate | ||
| │ │ | ||
| │ └─ Custom API Key | ||
| │ ├─ Select Protocol | ||
| │ ├─ Enter Base URL | ||
| │ ├─ Enter API Key | ||
| │ ├─ Enter Model IDs | ||
| │ ├─ Review generated JSON | ||
| │ └─ Save + authenticate | ||
| │ | ||
| └─ Qwen OAuth | ||
| ``` | ||
|
|
Collaborator
There was a problem hiding this comment.
[Drift] PRD ↔ shipped placement / label.
The PRD nests custom setup under /auth → API Key → Custom API Key, but shipped code places it at the top level of /auth:
customProvider.ts:28-29:id: 'custom-openai-compatible',label: 'Custom Provider',uiGroup: 'custom'allProviders.ts: registered as a top-level provider, not nested under anyapi-key-type-selectflow.
If reconciling the PRD, please redraw this tree to show Custom Provider as a sibling of Coding Plan / OpenRouter / Third-party providers, not as a child of API Key. Otherwise readers will infer a UI hierarchy that no longer exists.
Comment on lines
+89
to
+97
| ## Non-goals | ||
|
|
||
| 1. Do not require users to manually enter `envKey`. | ||
| 2. Do not introduce provider name as a separate concept. | ||
| 3. Do not add advanced `generationConfig`, `capabilities`, or per-model overrides to the wizard. | ||
| 4. Do not remove the documentation link entirely; it should remain available for advanced configuration. | ||
| 5. Do not change the existing Coding Plan or ModelStudio Standard API key flows. | ||
| 6. Do not attempt to auto-detect protocol from `baseUrl` in the first version; users select the protocol explicitly. | ||
|
|
Collaborator
There was a problem hiding this comment.
[Drift] Non-goal #3 contradicts shipped behavior.
The shipped customProvider.ts:43 sets showAdvancedConfig: true, which (via shouldShowStep('advancedConfig') in providerConfig.ts:392-413) makes the wizard ask for enableThinking, multimodal toggles, contextWindowSize, and maxTokens. Those are exactly the generationConfig overrides this non-goal disclaims.
(Prior review at line 150 flagged the missing custom-advanced-config view in the state machine; this is the same drift surfacing in the goals section.)
Reconcile by either:
- removing non-goal 如何自定义密钥文件 .env可能与其他文件冲突 #3 and adding the advanced-config step to the wizard description and state machine, or
- explicitly noting that the advanced-config step was added post-PRD.
Comment on lines
+447
to
+466
| ### Step 6: Save and Authenticate | ||
|
|
||
| On Enter from the review screen: | ||
|
|
||
| ```text | ||
| save: | ||
| env[generatedEnvKey] = apiKey | ||
| modelProviders[selectedProtocol] = [ | ||
| ...new custom configs using generatedEnvKey, | ||
| ...existing configs whose envKey !== generatedEnvKey | ||
| ] | ||
| security.auth.selectedType = selectedProtocol | ||
| model.name = firstModelId | ||
| reloadModelProvidersConfig() | ||
| refreshAuth(selectedProtocol) | ||
| ``` | ||
|
|
||
| Success message: | ||
|
|
||
| ```text |
Collaborator
There was a problem hiding this comment.
[Drift] Save-then-auth ordering is the opposite of what shipped.
The pseudo-code here writes settings.json (including security.auth.selectedType) and then calls refreshAuth(). Shipped code does the reverse and there's an explicit comment explaining why:
// useAuth.ts:902-905
// We previously used a useEffect to trigger authentication automatically when
// settings.security.auth.selectedType changed. This caused problems: if authentication failed,
// the UI could get stuck, since settings.json would update before success. Now, we
// update selectedType in settings only when authentication fully succeeds.This is the same concern the previous /review raised at line 460 — but resolved in the opposite direction from the PRD. If reconciling, please:
- Update Step 6 to attempt auth first using the in-memory key +
process.envsync. - Persist
security.auth.selectedTypeonly on auth success. - Define the rollback behavior explicitly (does the env entry stay? does
model.namestay?) to address the standing failed-auth recovery question at line 480.
wenshao
approved these changes
May 13, 2026
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
4 participants
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.
Summary
/auth -> API Key -> Custom API KeyRelated Issue
Closes #3582
Test Plan