fix(cli): remove dead --list-extensions handler from #4456#4800
Merged
Conversation
PR #4456 added a duplicate --list-extensions handler at line 965 and a preconnect guard, but PR #4673 (merged earlier) already implemented the same fix at line 470 using the existing handleListExtensions() function. The code from #4456 is unreachable because the early handler exits via process.exit(0) before execution reaches line 965. Removes: - Dead inline handler block in gemini.tsx (~40 lines) - Unnecessary preconnect guard for list-extensions - 3 test cases that tested the unreachable code path (~242 lines)
Contributor
📋 Review SummaryThis PR cleanly removes dead code from the 🔍 General Feedback
🎯 Specific Feedback🔵 Low
✅ Highlights
|
Collaborator
|
Thanks for the PR! Template looks good ✓ — all required sections present and filled. On direction: straightforward dead-code removal. PR #4673 merged first and added the early On approach: the scope is tight and correct. Three things removed: the unreachable handler, its three test cases, and the now-unnecessary Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ — 所有必填部分齐全。 方向：清晰的死代码删除。PR #4673 先合入，在第 470 行添加了早期 方案：范围紧凑且正确。删除三样东西：不可达的处理器、三个测试用例、以及已无必要的 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Contributor
There was a problem hiding this comment.
Pull request overview
Removes unreachable --list-extensions handling code in the CLI entrypoint (gemini.tsx) that was made dead by the earlier, correct early-exit handler (wired to handleListExtensions()), and deletes tests that exclusively exercised the dead path.
Changes:
- Removed a redundant
getListExtensions()-based preconnect guard and always runspreconnectApi()on the normal startup path. - Deleted the unreachable inline
--list-extensionshandler block that previously attempted to initialize config + print extensions. - Removed three test cases that only covered the deleted, unreachable handler.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/cli/src/gemini.tsx | Deletes unreachable --list-extensions handler and simplifies preconnect logic. |
| packages/cli/src/gemini.test.tsx | Removes tests for the dead handler path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Collaborator
Author
|
@qwen-code /triage |
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. |
Collaborator
|
@/tmp/stage-2.md |
Collaborator
|
@/tmp/stage-3.md |
qwen-code-ci-bot
approved these changes
Jun 5, 2026
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Collaborator
Author
Code ReviewIndependent proposal (before reading diff): I'd delete the unreachable handler at line 965 (duplicate Comparison with PR: Matches exactly. The diff is clean deletion across 2 files (+1/-284). I traced the control flow: TestingUnit tests: cd packages/cli && npx vitest run src/gemini.test.tsx✅ 24 tests passed (13.36s) Tmux real-scenario test ( Before (installed build)After (this PR via dev build)Both output "No extensions installed." and exit cleanly with code 0. Behavior is identical — dead code removal has no user-visible impact, as expected. 中文说明代码审查独立方案（读 diff 前）： 删除第 965 行不可达的处理器（重复的 与 PR 对比： 完全匹配。Diff 是跨 2 个文件的干净删除（+1/-284）。追踪控制流：第 470 行 测试单元测试： 24 通过（13.36s） Tmux 真实场景测试： Before/After 均输出 "No extensions installed." 并以 code 0 干净退出。行为一致——死代码删除无用户可见影响，符合预期。 — Qwen Code · qwen3.7-max |
Collaborator
Author
|
Reflecting on the whole picture: This is the kind of PR I like to see — it makes the codebase smaller and less confusing without changing any behavior. The motivation is airtight: two PRs (#4456 and #4673) raced to fix the same issue (#4450), #4673 won the merge race, and #4456's handler became unreachable dead code. The 284 lines being removed were literally impossible to execute. The implementation is provably correct. You can trace it: line 470 checks The tmux test confirms what we already knew from reading the code: LGTM. Ships cleanly. ✅ 中文说明回顾整体： 这是我喜欢看到的 PR——让代码库更小更清晰，而不改变任何行为。动机无可争议：两个 PR（#4456 和 #4673）竞相修复同一问题（#4450），#4673 先合入，#4456 的处理器成为不可达的死代码。被删除的 284 行代码字面上不可能执行。 实现可证明正确。可以追踪：第 470 行检查 tmux 测试证实了从代码阅读中已知的结论： LGTM，干净交付。✅ — Qwen Code · qwen3.7-max |
wenshao
reviewed
Jun 5, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
[Suggestion] Config.getListExtensions() and its plumbing become dead code after this PR.
Once the last production caller (the removed preconnect guard and dead handler) is gone, the following chain has zero production callers:
ConfigParameters.listExtensions(packages/core/src/config/config.ts:711)private readonly listExtensionsfield (config.ts:1076)- Constructor assignment
this.listExtensions = params.listExtensions ?? false(config.ts:1269) getListExtensions()method (config.ts:3337)- CLI-side forwarding
listExtensions: argv.listExtensions || false(packages/cli/src/config/config.ts:1836)
Note: argv.listExtensions itself is NOT dead — it's still used directly at gemini.tsx:470. Only the Config class's storage and getter are orphaned.
Consider removing this chain in this PR or a follow-up. Six test mocks in gemini.test.tsx (lines 191, 317, 627, 760, 871, 957) also set getListExtensions: () => false and can be cleaned up together.
(Posted as a body-level comment because the affected file packages/core/src/config/config.ts is not in this PR's diff.)
— qwen3.7-max via Qwen Code /review
wenshao
approved these changes
Jun 5, 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
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.
What this PR does
Removes the duplicate
--list-extensionshandler and related dead code introduced by #4456, which was superseded by #4673.Why it's needed
PR #4673 (
fix(cli): honor list extensions flag) merged first and implemented the same fix at line 470 using the existinghandleListExtensions()function. The handler from #4456 at line 965 is unreachable because the early handler callsprocess.exit(0)before execution reaches it. This leaves ~284 lines of dead code on main.Specifically removed:
gemini.tsx:842— unnecessaryif (!config.getListExtensions())preconnect guard (early handler exits before preconnect runs)gemini.tsx:965-1002— duplicate inline--list-extensionshandler usingconfig.getExtensions()(unreachable)gemini.test.tsx— 3 test cases (~242 lines) that tested the unreachable code pathReviewer Test Plan
How to verify
qwen --list-extensionsstill works (handled by the fix(cli): honor list extensions flag #4673 handler at line 470).handles --list-extensions before sandbox and app config startupstill passes.argv.listExtensionsat line 470 →handleListExtensions()→process.exit(0)— never reaches line 965.Evidence (Before & After)
N/A — pure dead code removal, no user-visible behavior change.
vitest run packages/cli/src/gemini.test.tsx— 24 tests passed (including #4673's handler test).Tested on
Risk & Scope
Linked Issues
Fixes dead-code debt from #4456 (superseded by #4673, both fixing #4450).
中文说明
做了什么
删除 #4456 引入的重复
--list-extensions处理逻辑及相关死代码，该逻辑已被 #4673 取代。为什么需要
PR #4673 先合入 main，在第 470 行使用现有的
handleListExtensions()函数实现了相同的修复。由于该处理逻辑调用process.exit(0)后程序直接退出，#4456 在第 965 行添加的处理逻辑永远不会被执行，留下了约 284 行死代码。具体删除：
gemini.tsx:842— 多余的 preconnect 跳过逻辑（早期 handler 在 preconnect 之前就退出了）gemini.tsx:965-1002— 重复的 inline--list-extensionshandler（不可达）gemini.test.tsx— 3 个测试不可达代码路径的测试用例（约 242 行）如何验证
qwen --list-extensions仍然正常工作（由 fix(cli): honor list extensions flag #4673 的 handler 处理）。handles --list-extensions before sandbox and app config startup通过。argv.listExtensions→handleListExtensions()→process.exit(0)，永远到不了第 965 行。风险
无 — 纯死代码删除，无行为变化。