fix(cli): stabilize extension list spacing - #5445
Conversation
There was a problem hiding this comment.
Pull request overview
This PR stabilizes the visual alignment of the extensions list in the CLI TUI by moving status-column spacing from leading whitespace inside Text nodes into Ink layout spacing, avoiding renderer-dependent whitespace behavior that caused snapshot flakiness on CI.
Changes:
- Replaced leading text spaces before
(active)/(disabled)withBox-levelmarginLeftfor consistent spacing. - Adjusted the status column box width to account for parentheses while preserving aligned output.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks for the PR! Template looks good ✓ On direction: This is a straightforward CI snapshot stability fix — the extension list status column used literal leading spaces inside On approach: The change is minimal and focused — one file, swapping Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 方向:这是一个很直接的 CI snapshot 稳定性修复——扩展列表状态列之前在 方案:改动很小、很聚焦——一个文件,把 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
| </Box> | ||
| <Box width={maxStatusWidth + 4} flexShrink={0}> | ||
| <Text color={activeColor}> ({activeString})</Text> | ||
| <Box marginLeft={2} width={maxStatusWidth + 2} flexShrink={0}> |
There was a problem hiding this comment.
[Suggestion] The total horizontal footprint of the status column (marginLeft(2) + width(maxStatusWidth + 2) = maxStatusWidth + 4) is implicitly coupled to the + 4 in fixedWidth on line 170. Before this PR, the + 4 lived in one place (width={maxStatusWidth + 4}), making the correspondence easy to spot. Now it's split across two props on a different line from fixedWidth, so a future edit to the margin or width padding could silently break the description-width calculation.
Consider extracting a shared named constant:
| <Box marginLeft={2} width={maxStatusWidth + 2} flexShrink={0}> | |
| <Box marginLeft={statusGap} width={maxStatusWidth + statusGap} flexShrink={0}> |
…and on line 170:
const statusGap = 2;
const fixedWidth = 2 + maxNameWidth + statusGap + maxStatusWidth + statusGap + 15;This keeps the two sites in sync and makes the intent self-documenting.
— qwen3.7-max via Qwen Code /review
✅ Local verification — recommend mergeI built and tested this PR locally as a merge reference. Verdict: verified, safe to merge. The change preserves the exact visible output (proven byte-for-byte locally) while removing the renderer-dependent leading-whitespace that broke the scheduled CI snapshot. Environment: dedicated git worktree at PR head What the PR changes (1 file)In - <Box width={maxStatusWidth + 4} flexShrink={0}>
- <Text color={activeColor}> ({activeString})</Text> // 2 leading spaces inside the Text node
+ <Box marginLeft={2} width={maxStatusWidth + 2} flexShrink={0}>
+ <Text color={activeColor}>({activeString})</Text> // gap is now geometric marginLeftThe total reserved width is unchanged: old Verification1. Snapshot test (fixed) — passes 2. Render A/B (fixed vs pre-fix) — byte-identical, and the CI bug is not locally reproducible
3. Reviewer Test Plan gates — all pass
Why it fixes the CI flake (and the one honest caveat)
Caveat: I could not reproduce the original CI snapshot mismatch on macOS (it is environment-specific), so this is verified by construction + output-preservation, not by reproducing the red CI run. The existing snapshot remains the regression guard and is unchanged, which itself confirms the visible output is preserved. ConclusionMinimal, layout-only change; visible output preserved byte-for-byte; all gates green; removes a real renderer-fragility. LGTM — recommend merge. 中文版✅ 本地验证 —— 建议合并我在本地构建并测试了该 PR,作为合并参考。结论:验证通过,可安全合并。 该改动完整保留了可见输出(本地已逐字节证明),同时移除了导致定时 CI snapshot 失败的、依赖渲染器的文本前导空格。 环境: 在 PR 改了什么(1 个文件)在 - <Box width={maxStatusWidth + 4} flexShrink={0}>
- <Text color={activeColor}> ({activeString})</Text> // 2 个前导空格在 Text 节点里
+ <Box marginLeft={2} width={maxStatusWidth + 2} flexShrink={0}>
+ <Text color={activeColor}>({activeString})</Text> // 间距改为几何 marginLeft总预留宽度不变:旧 验证1. Snapshot 测试(修复版)—— 通过 2. 渲染 A/B(修复版 vs pre-fix)—— 字节级一致,且 CI bug 本地不可复现
3. Reviewer Test Plan 各项 gate —— 全部通过
为什么它能修复 CI flake(以及一点诚实的保留)
保留: 我无法在 macOS 上复现最初的 CI snapshot 不一致(它是环境特定的),所以这是通过 构造正确性 + 输出保留 来验证的,而非通过复现那次红色 CI run。现有 snapshot 仍是回归守护、且未改动,这本身就佐证了可见输出被保留。 结论最小化的纯布局改动;可见输出逐字节保留;所有 gate 全绿;移除了一处真实的渲染器脆弱性。LGTM —— 建议合并。 |
Code ReviewThe diff is clean and minimal. Three substantive changes:
No correctness bugs, no security issues, no AGENTS.md violations. The change is exactly what the goal requires — nothing more. Real-Scenario TestingTested with installed Before (installed qwen v0.18.3)After (this PR)Same empty-state rendering — no extensions installed to show the status column. The PR's fix only affects rows where extensions exist (status labels CI Test Results (from
|
| Check | Status |
|---|---|
| Lint | ✅ pass |
| Test (macOS) | ✅ pass (15m43s) |
| Test (Ubuntu) | ✅ pass (16m27s) |
| CodeQL | ✅ pass |
| Test (Windows) | ⏳ pending |
The snapshot tests that previously failed on CI now pass on both macOS and Linux runners — which is exactly the problem this PR was designed to solve.
中文说明
代码审查
diff 干净、最小化。三处实质性改动:
-
marginLeft={2}+width={maxStatusWidth + 2}替换旧的width={maxStatusWidth + 4}+ 内联" "空格。总视觉间距保持不变(2 布局 + 内容 = 旧的 4 宽度含 2 内联空格)。这是正确的修复——Ink 布局引擎在各渲染器中一致处理间距,而文本节点前导空格依赖渲染器行为。 -
Prettier 格式化——外层
<Box>和描述<Text>的多行 JSX 换行,无语义变化。 -
fixedWidth计算器(2 + maxNameWidth + 2 + maxStatusWidth + 4 + 15)仍然用+ 4作为状态列间距,正确匹配新的marginLeft={2} + width={maxStatusWidth + 2}= 4。无需更新。
无正确性 bug,无安全问题,无 AGENTS.md 违规。改动恰好覆盖目标——不多不少。
真实场景测试
在 tmux(200×50 视口)中使用已安装的 qwen v0.18.3 测试。注意:此 CI 环境没有安装扩展,因此无法直接渲染带状态标签的扩展列表。"未安装扩展"的空状态渲染一致。真正的证据来自 CI 单元测试。
CI 测试结果(来自 gh pr checks):Lint ✅、Test (macOS) ✅、Test (Ubuntu) ✅、CodeQL ✅、Test (Windows) ⏳ 待完成。
之前在 CI 上失败的 snapshot 测试现在在 macOS 和 Linux runner 上都通过了——这正是此 PR 要解决的问题。
— Qwen Code · qwen3.7-max
|
This is a clean, well-scoped fix. The root cause (text-node whitespace being renderer-dependent) is correctly identified, and the solution (Ink layout CI confirms: snapshot tests that were failing on macOS and Linux now pass on both. The fix preserves the same visual spacing, so no user-visible change — just CI stability. Approving. ✅ 中文说明这是一个干净、范围合理的修复。根因(文本节点空白依赖渲染器)定位正确,方案(用 Ink 布局 CI 确认:之前在 macOS 和 Linux 上失败的 snapshot 测试现在两个平台都通过了。修复保持相同的视觉间距,用户无感知变化——只是 CI 稳定性提升。 批准 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
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. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running. — qwen3.7-max via Qwen Code /review
What this PR does
This PR keeps the extension list's visible alignment the same while moving the status-column spacing from literal leading text spaces into Ink layout spacing.
Why it's needed
The scheduled Release workflow failed in Quality Checks because the extension list snapshot rendered one fewer leading space before status labels on CI than the committed snapshot expected. Using layout spacing avoids relying on renderer-specific preservation of leading whitespace inside text nodes.
Reviewer Test Plan
How to verify
Confirm that installed-extension rows still render with the same aligned status column, and that the extension list snapshot test remains stable. The related failed run was https://github.com/QwenLM/qwen-code/actions/runs/27854449183.
Evidence (Before & After)
Before: the scheduled Release run on 2026-06-20 failed in Workspace Tests with snapshot mismatches around the spaces before
(active)and(disabled)labels. After:npx vitest run src/ui/components/extensions/steps/ExtensionListStep.test.tsxpassed with 5 tests,npx prettier --check packages/cli/src/ui/components/extensions/steps/ExtensionListStep.tsxpassed,npm run lintpassed,npm run typecheckpassed, andnpm run buildpassed. The build still reports pre-existing vscode companion curly warnings and Browserslist freshness notices, but exits successfully.Tested on
Environment (optional)
Node.js v22.22.0, npm 10.9.4.
Risk & Scope
Linked Issues
Fixes #5425
Fixes #5371
Related Actions runs: https://github.com/QwenLM/qwen-code/actions/runs/27854449183 and https://github.com/QwenLM/qwen-code/actions/runs/27797790330
中文说明
What this PR does
这个 PR 保持扩展列表的可见对齐不变,但把状态列前面的间距从文本节点里的前导空格改成 Ink 布局间距。
Why it's needed
定时 Release workflow 在 Quality Checks 里失败,因为 CI 上扩展列表 snapshot 在状态标签前少渲染了一个空格,和已提交 snapshot 不一致。使用布局间距可以避免依赖文本节点前导空格在不同渲染环境里的保留行为。
Reviewer Test Plan
How to verify
确认已安装扩展列表行仍然以同样的状态列对齐方式渲染,并确认扩展列表 snapshot 测试保持稳定。相关失败 run 是 https://github.com/QwenLM/qwen-code/actions/runs/27854449183。
Evidence (Before & After)
Before: 2026-06-20 的定时 Release run 在 Workspace Tests 中失败,失败点是
(active)和(disabled)标签前空格数量的 snapshot mismatch。After:npx vitest run src/ui/components/extensions/steps/ExtensionListStep.test.tsx通过 5 个测试,npx prettier --check packages/cli/src/ui/components/extensions/steps/ExtensionListStep.tsx通过,npm run lint通过,npm run typecheck通过,npm run build通过。build 仍会报告既有的 vscode companion curly warning 和 Browserslist 数据提示,但命令成功退出。Tested on
Environment (optional)
Node.js v22.22.0, npm 10.9.4.
Risk & Scope
Linked Issues
Fixes #5425
Fixes #5371
相关 Actions runs: https://github.com/QwenLM/qwen-code/actions/runs/27854449183 和 https://github.com/QwenLM/qwen-code/actions/runs/27797790330