refactor(cli): remove unused ApiKeyInput component - #10139
Conversation
|
Thanks for the PR! Template looks good ✓ Problem: verified, not theoretical — re-checked against current Direction: aligned — this is dead-surface cleanup tracked in #10000, and the provider-first auth flow that replaced these per-plan screens is in place. Deleting an unmounted component changes no behavior. Size: not applicable — Approach: the scope feels right — this is already the minimal change (pure deletion of exactly the dead declarations; no drive-by edits, no locale deletions). Nothing to cut. Risk: no elevated risk signals — no high-risk path match. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已核实，非理论性问题——本次 re-run 已对照当前 方向：对齐——这是 #10000 跟踪的死代码表面清理，取代这些按 plan 划分界面的 provider 统一认证流程已经就位。删除未挂载的组件不改变任何行为。 规模：不适用—— 方案：范围合理——这已是最小改动（纯删除恰好无人引用的声明；无顺手改动、无语言条目删除）。没有可砍的部分。 风险：无升级风险信号——未命中高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewNo findings. My independent take on "remove an unreferenced component" is exactly what this diff does: prove zero consumers corpus-wide, delete the whole file, touch nothing else. I re-verified the load-bearing claims against current
One file, −97/+0, no drive-by edits. Nothing to flag. TestingEvidence carried here: the PR's own CI results on the reviewed commit, fetched via the API (unattended run — I do not build or execute PR code). CI is now fully settled on Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Notes on the signal, outside the table: everything that ran is green. The macOS/Windows Real-scenario (tmux) testing: N/A — pure deletion of an unmounted component, nothing user-visible changes; and this is an unattended run in any case. 中文说明代码审查：无问题。面对"删除无人引用的组件"，我独立的方案与 diff 完全一致：全库证明零消费者后整文件删除、不动其他任何内容。本次 re-run 我对照当前
单文件 −97/+0，无顺手改动，无可指摘之处。 测试证据：本节引用的是被审提交自身 CI 的最终结果（经 API 获取；无人值守运行不构建、不执行 PR 代码）。 表格之外的说明：所有实际运行的检查均为绿色。macOS/Windows 的 真实场景（tmux）测试：N/A——纯删除未挂载组件，无任何用户可见变化；且本次为无人值守运行。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 3/5 — clean review at every stage and CI fully green, but this is a cross-repository Stepping back: my independent plan for "remove an unreferenced component" was exactly what landed here — prove zero consumers corpus-wide, delete the whole file, touch nothing else — and the diff matches it line for line. The problem is real and verified, not asserted: re-checked against current The only reason this doesn't end in an approval is the guardrail: fork PRs with a 中文说明置信度：3/5 —— 各阶段审查均干净且 CI 已全绿，但这是一个跨仓库（fork）的 退一步看整体：我面对"删除无人引用的组件"的独立方案与最终落地的完全一致——全库证明零消费者后整文件删除、不动其他内容，diff 逐行吻合。问题真实存在且经过验证，而非仅凭声明：今天已对照当前 唯一没有给出批准的原因是护栏：fork 的 — Qwen Code · qwen3.8-max Reviewed at |
|
⏸️ Still deferred to @wenshao — per your re-trigger, the full pass ran again: every load-bearing claim re-verified against today's 中文说明⏸️ 继续转交 @wenshao —— 应你的重新触发，完整流程已再跑一遍：所有关键声明已对照今天的 |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Review: PR #10139 — refactor(cli): remove unused ApiKeyInput component
Files: 1 file, +0/−97 lines
Author: @qqqys
Scope: Deletes dead packages/cli/src/ui/components/ApiKeyInput.tsx (component, ApiKeyInputPlan interface, 3 URL constants)
Verified
References (no consumers remain):
packages/cli/— zero matches forApiKeyInput(any identifier), zero imports from the file path, zero references to the URL constants (CODING_PLAN_API_KEY_URL,CODING_PLAN_INTL_API_KEY_URL,TOKEN_PLAN_API_KEY_URL), zero references toApiKeyInputPlan.packages/desktop/has its ownApiKeyInputcomponent underpackages/desktop/apps/electron/src/renderer/components/apisetup/— a completely separate component in a different package, not affected.integration-tests/,.github/,docs-site/— zero matches for the deleted identifiers.
Build & lint: Author reports npm run build && npm run bundle && npm run typecheck and npm run lint:ci green on Linux. Worktree fetch blocked on this Windows machine (known getaddrinfo issue), so Windows-side re-run is not possible here.
Minor observation
The PR description says ink-link keeps five live users. After removing this file, ink-link has zero direct consumers in packages/cli/src/. It appears in package-lock.json as a transitive dependency but is not a direct dependency of any package's package.json and is not installed in node_modules. This is not a blocker — the PR's scope is component removal, not dependency cleanup — but the claim about five live users appears to be stale.
Verdict
No blockers. The deletion is clean, well-documented, and correctly verified. The component, its interface, and its URL constants have zero consumers in the codebase. The ink-link note is a minor documentation inaccuracy (the dependency is transitive, not a direct dependency with five consumers), not a correctness issue.
Part of the dead-surface cleanup tracked in #10000.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent 6c": none — all checks above ran to completion within the tool budget..
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent 6c"：none — all checks above ran to completion within the tool budget.。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 51 passed · 0 failed · 51 total Flakiness gate: not applicable — no runnable changed test files (0 out-of-scope file(s) noted in the log) 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：51 通过 · 0 失败 · 51 总计 抖动门：不适用 — no runnable changed test files (0 out-of-scope file(s) noted in the log) Verification reportPR #10139 Deep Verification —
|
| cell | oracle | base (HEAD^1) | head (PR) |
|---|---|---|---|
\bApiKeyInput\b over test-plan dirs |
hits outside the file itself | 1 hit = the declaration only | 0 hits |
\bApiKeyInputPlan\b + 3 URL constants |
hits outside the file itself | 5 hits = declarations only | 0 hits |
| repo-wide safety net (constants/interface) | any hit anywhere | declarations only | 0 hits |
| distinct-identifier control | navigateToCustomApiKeyInput present |
n/a | 2 hits (untouched) |
Scripted: 14/14 assertions pass (01-census.mjs). See 02-census-reference-graph.png.
Build-artifact A/B cells
| cell | environment | oracle | base | head |
|---|---|---|---|---|
| tsc output | npm run build in each tree |
ApiKeyInput.{js,d.ts,js.map} emitted |
present (3 files) | absent |
| esbuild bundle | npm run bundle in each tree |
identifier census in chunks+cli.js | 0 hits (never reachable) | 0 hits |
| bundle content | normalized multiset compare | 469 files, byte totals | 469/469 identical, Δ=0 bytes | — |
The deletion is visible to the build system (tsc cell flips) but was never load-bearing in the shipped artifact: esbuild only bundles modules reachable from packages/cli/src/cli.ts, and nothing ever imported ApiKeyInput.tsx, so the base bundle already lacked it. That is the strongest form of a dead-code A/B — X/X cells unchanged. See 01-bundle-ab-base-vs-head.png.
Raw bundle byte totals differ by −27,894 bytes; the entire delta is accounted by three named confounds of building in a scratch worktree (git short-SHA string, content-hash chunk-name cascade, and ../../-prefixed module-path comments/CJS keys from symlink realpath resolution — 2,697 modules, each differing by an exact multiple of 6 bytes). After normalizing exactly those, byte totals are equal and every file matches.
Findings
None. Every PR claim checked out; see the walkthrough below.
Reviewer Test Plan walkthrough
rg -n '\bApiKeyInput\b' packages integrations integration-tests scripts .github docs-site→ zero hits at head ✓ (executed withgit grep --extended-regexp;rgis not installed in this container — identical semantics over tracked files; the builtdist/was additionally word-boundary-grepped: zero hits).- Same for
\bApiKeyInputPlan\b|\bCODING_PLAN_API_KEY_URL\b|…overpackages integration-tests→ zero hits ✓. npm run build && npm run bundle && npm run typecheck→ build+bundle pre-existed at head;npm run typecheckre-run at head: exit 0 across all workspaces + integration-tests ✓. The test plan's note "typecheck does not run in CI" is accurate:ci.ymlruns onlytypecheck:integration(line 635), not the full workspace typecheck.npm run lint:ci→ equivalent command (eslint . --ext .ts,.tsx --max-warnings 0+eslint integration-tests --max-warnings 0)exit 0 ✓, with a liveness probe (a planted unused variable inpackages/cli/src/lint-probe.tswas caught with@typescript-eslint/no-unused-vars, exit 1, then removed). Note:eslint .scanstmp/, which is not in the config ignores — verification scratch worktrees/harnesses must be excluded (--ignore-pattern 'tmp/'), otherwise the gate reports thousands of false errors.
Secondary claims verified
- ink-link users: 6 importers at base (including
ApiKeyInput.tsx) → exactly 5 at head (AuthDialog.tsx,ProviderSetupSteps.tsx,QwenOAuthProgress.tsx,ArenaStartDialog.tsx,StatusMessages.tsx) — matches the PR's "keeps five live users". - i18n keys:
'API key cannot be empty.'is still consumed byuseProviderSetupFlow.ts:284;'Enter to submit, Esc to go back'byProviderSetupSteps.tsx:41— both live, so keeping every locale entry is correct (no orphaned keys created by this PR). - URLs survive as literals: the two console URLs remain in
docs/users/configuration/model-providers.md; the token-plan URL remains inpackages/core/src/providers/presets/alibaba-token-plan.tsandpackages/vscode-ide-companion/src/services/subscriptionPlanDefinitions.ts. - Commit reachability: local
git rev-list HEAD^1..HEAD^2= 1 commit, matching the metadata snapshot's 1-commit array — no shallow gap.
Test-suite evidence and environment attribution
| run | env | result |
|---|---|---|
head, full packages/cli suite |
container env (QWEN_HOME set), loaded machine |
888/889 files, 24754 passed, 12 failed, 92 skipped |
| the 12 failures | all in settings.test.ts (loadSettings/loadEnvironment) |
assume QWEN_HOME unset; environment.ts branches on process.env['QWEN_HOME']; this container exports it (the verify agent's own home) |
head, settings.test.ts alone, env -u QWEN_HOME |
clean | 169/169 pass |
base, same file, env -u QWEN_HOME (A/A) |
clean | 169/169 pass |
base, same file, QWEN_HOME set (A/A) |
container env | the identical 12 failures, name for name |
head, full suite, env -u QWEN_HOME, loaded |
clean env | 886/889 files; 4 failures = 3×15s timeouts + 1 chained assertion in the same file |
| head, the 3 timeout files re-run idle | clean env | 38 passed / 1 skipped, 0 failed (one test naturally takes ~6.5s on this machine — under concurrent builds+lint it crossed the 15s timeout) |
Conclusion: every observed failure reproduces on base or disappears under the environment the tests require; none is attributable to a 97-line deletion of an unmounted component.
Not covered
- No TUI before/after captures — the component was never mounted; there is no screen to capture (the PR says the same).
- Sibling dead-code sweep — whether the same auth refactor left other orphaned files is out of scope for this PR (tracked under [find-simplifications] candidate ledger #10000 per the description); only this file's consumer graph was verified.
- Base-side full
npm run typecheck/ full vitest suite — the base side was verified at the level the change can act on (census, build, bundle, and the settings-file A/A); a full base suite run would only re-measure the environment. - Integration tests — not run; the deletion cannot affect CLI behavior (proven by the bundle identity), matching the PR's own scope note.
dist/web-shell/(335 files in headdist/) is produced by a separate serve-fast-path pipeline step the base tree never ran; excluded from the bundle comparison by scope, not by result.- Per-commit attribution — single-commit PR; the merge-ref checkout carries all three relevant commits.
- The vitest A/A runs on base were executed in the terminal with observed counts; the head-side runs have full captured logs.
rgis not present in the container (git grep used as the equivalent oracle).
Methodology
Environment: the CI verify container (node:22-bookworm), working tree at refs/pull/10139/merge (depth 2), npm ci + npm run build pre-run at HEAD. Base arm: git worktree add tmp/base-tree HEAD^1 wired to the already-installed root node_modules via symlinks (root + per-package, including packages/channels/*), plus a locally generated packages/web-templates build; the PR touches no package.json/lockfile, and the only internal symlink realpath worth naming — node_modules/@qwen-code/qwen-code-core → head tree packages/core — is byte-identical across the diff because the PR does not touch packages/core. Base bundle: npm run bundle in the worktree; base tsc cell: npm run build in packages/cli (composite tsc --build). Head gates: npm run typecheck, lint:ci equivalent with liveness probe, and npx vitest run in packages/cli. Harnesses 01-census.mjs, 02-bundle-ab.mjs, 03-gates.mjs (plus diagnostics diag-diff-chunks.mjs, diag-module-sizes.mjs) live in this directory with raw logs (logs-*.txt). Evidence images: 01-bundle-ab-base-vs-head.png, 02-census-reference-graph.png, 03-gates-and-attribution.png.
Flakiness gate log
verdict: n/a
summary: no runnable changed test files (0 out-of-scope file(s) noted in the log)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
Thanks for the verification pass, @doudouOUC — the consumer analysis matches mine exactly. One correction on the
The removed I suspect the discrepancy comes from the environment you noted: with the worktree fetch blocked by the Nothing to change in the diff or the description, so I've left both as they are. CI is now fully green (0 failures), which also clears the 中文说明感谢核验 —— 消费者分析与我的结论一致。但关于
被删除的 差异很可能来自您提到的环境问题：worktree fetch 被 因此 diff 与描述均无需改动。CI 现已全绿（0 失败），自动评审那条 |
4 participants
What this PR does
Removes an API-key input component that nothing in the codebase references, together with the plan-type interface and the three plan-URL constants declared in the same file. The component once rendered the API-key entry screen for the subscription-plan authentication flows, but the application no longer imports or renders it anywhere; its only remaining references were its own declarations. The authentication experience that replaced it is the unified provider setup flow that now serves every provider, including the subscription plans. Nothing else changes — in particular, the two translation strings the component used are still used by live authentication screens, so every locale entry is kept.
Why it's needed
Dead surface is a cost: every contributor who reads it has to work out what wires it up, and the answer here is "nothing". The component was built for the plan-based API-key screens; the provider-first authentication refactor replaced those per-flow screens with a single generic provider setup experience and this file was left behind. No production caller, no test, no snapshot, no dynamic import path, and no documentation row references it — the three exported URL constants likewise appear only at their declarations, while the same URLs live on as literals in the provider presets and user docs where they are actually consumed. Removing it shrinks the auth UI surface a reviewer of that area must keep in their head.
Reviewer Test Plan
How to verify
This is a pure deletion of code nothing references. Verify the absence of consumers rather than behavior:
rg -n '\bApiKeyInput\b' packages integrations integration-tests scripts .github docs-site— expect zero hits after this PR (before it the only hit was the component's own declaration). The similarly namednavigateToCustomApiKeyInputtest helper is a distinct identifier and is untouched.rg -n '\bApiKeyInputPlan\b|\bCODING_PLAN_API_KEY_URL\b|\bCODING_PLAN_INTL_API_KEY_URL\b|\bTOKEN_PLAN_API_KEY_URL\b' packages integration-tests— expect zero hits after this PR (before it each appeared only at its declaration in the removed file).npm run build && npm run bundle && npm run typecheck— all green locally with this change (a type-only break from a removed export would surface here; typecheck does not run in CI).npm run lint:ci— green locally with this change.No user-visible behavior changes, so no before/after TUI evidence applies.
Evidence (Before & After)
N/A — non-UI change (dead-code deletion; nothing mounted or called this component).
Tested on
Environment (optional)
Local
npm run build && npm run bundle && npm run typecheckandnpm run lint:ciin a clean worktree offupstream/main.Risk & Scope
.qwenfiles; if a consumer were ever found, reverting the single commit restores everything.ink-linkkeeps five live users).Linked Issues
Part of the dead-surface cleanup tracked in #10000.
Machine details (find-simplifications)
api-key-input— class 5, added-then-removed scaffolding: a whole dead component file left behind when its feature was deliberately replaced.packages/cli/src/ui/components/ApiKeyInput.tsx— 97 lines deleted, 0 added (git diff --numstat), the component, its plan-type interface, and three plan-URL constants..github,.husky,.vscode,patches, root build/lint manifests, tracked.qwenfiles) resolvesApiKeyInputto exactly one reference — its own declaration.ApiKeyInputPlanappears only in-file as the component's own prop type; each of the three URL constants appears exactly once (its declaration). Zero module-path imports of the file. Zero production consumers. Hidden-consumer checklist rows run:string-keys(no registry or literal reference; the kebab-caseapi-key-inputliterals in the replacement wizard's design PRD are new-flow ViewLevel names, not references to this component),build-graph,generated,vi-mock,mirrors,dyn-import— all clean;cli-flags/assetsnot applicable. No test, snapshot, or integration-test reference (second pass without test exclusions); no locale entries to remove (both i18n keys the component used are still consumed by the live provider setup flow); no docs rows; no filename-allowlist entry (PascalCase.tsxneeds none).packages/cli/src— landable; the package exports only its entry and./export, neither re-exports this file), full-corpus grep, own-file check, hidden-consumer checklist, test-only check, unwire history (deliberate: the provider-first auth refactor997796f532(refactor(cli): provider-first auth registry with unified install pipeline #3864, 2026-05-08) replaced the per-flow screens with the unified provider setup flow and left this file behind), design-doc ownership (the custom-API-key wizard PRD documents the replacement flow, not this component).upstream/main(f9f5f2fcb6): same single reference, no new consumer, branch base fetched successfully before cutting.npm run build && npm run bundle && npm run typecheckgreen;npm run lint:cigreen; targeted vitest skipped by design (the surface had no test — no applicable targeted unit test; corpus re-grep on the landing checkout returned zero surviving references). Self-audit: two clean passes.ink-linkkeeps five live users.中文说明
本 PR 做了什么
删除一个在代码库中已无任何引用的 API Key 输入组件，以及同一文件中声明的套餐类型接口和三个套餐 URL 常量。该组件曾用于渲染订阅套餐认证流程的 API Key 输入界面，但应用中已没有任何地方导入或渲染它；它仅存的引用就是它自己的声明。取代它的认证体验是如今服务于所有提供商（包括订阅套餐）的统一 provider 安装流程。除此之外没有任何其他改动——特别地，该组件使用的两条翻译文案仍被在用的认证界面使用，因此所有语言文件条目均予以保留。
为什么需要
死代码是一种成本：每个读到它的人都得弄清是什么在用它，而这里的答案是"没有任何东西"。该组件是为基于套餐的 API Key 界面而编写的；provider 优先的认证重构用单一的通用 provider 安装体验取代了那些按流程划分的界面，此文件被遗留下来。没有生产调用者、没有测试、没有快照、没有动态导入路径、也没有文档行引用它——三个导出的 URL 常量同样只出现在各自的声明处，而相同的 URL 以字面量形式继续存在于真正消费它们的 provider 预设与用户文档中。删除它可以缩小该领域的审阅者需要装在脑子里的认证 UI 表面。
评审者测试计划
如何验证
这是一次对无任何引用代码的纯删除。请验证消费者的缺失，而不是行为：
rg -n '\bApiKeyInput\b' packages integrations integration-tests scripts .github docs-site—— 本 PR 之后应无任何命中（之前唯一的命中是该组件自身的声明）。名称相似的navigateToCustomApiKeyInput测试辅助函数是另一个标识符，未被触碰。rg -n '\bApiKeyInputPlan\b|\bCODING_PLAN_API_KEY_URL\b|\bCODING_PLAN_INTL_API_KEY_URL\b|\bTOKEN_PLAN_API_KEY_URL\b' packages integration-tests—— 本 PR 之后应无任何命中（之前每个符号都只出现在被删文件的声明处）。npm run build && npm run bundle && npm run typecheck—— 本地在包含本改动的情况下全部通过（删除导出可能引发的纯类型问题会在这里暴露；typecheck 不在 CI 中运行）。npm run lint:ci—— 本地在包含本改动的情况下通过。由于没有用户可见的行为变化，无需前后对比的 TUI 证据。
证据（改动前后）
N/A —— 非 UI 改动（死代码删除；该组件从未被挂载或调用）。
测试环境
环境（可选）
在基于
upstream/main的干净 worktree 中本地运行npm run build && npm run bundle && npm run typecheck与npm run lint:ci。风险与范围
.qwen文件）验证；万一未来发现消费者，回滚这一个提交即可完整恢复。ink-link仍有 5 处在用）。关联 Issue
属于 #10000 跟踪的死代码清理工作。
机器细节见上方英文 "Machine details (find-simplifications)" 折叠块：候选编号
api-key-input（第 5 类：功能被有意取代后遗留的整文件死组件），共删除 97 行、新增 0 行；全语料库搜索证明ApiKeyInput的唯一引用即其自身声明，ApiKeyInputPlan仅作组件自身的属性类型使用，三个 URL 常量各仅出现于声明处，文件路径零导入；调查协议各步骤（含落地前对刚拉取的upstream/main基准f9f5f2fcb6的重新核验）均已执行；构建、打包、类型检查与lint:ci全部通过；因该表面本无测试，按流程跳过定向 vitest，以落地检出上的语料库复扫（零残留引用）兜底；无快照、无语言键删除（文案仍被在用代码消费）、无文档行、无允许列表条目等连带项，ink-link仍有 5 处在用。