fix(cli): remove residual blank lines after MCP init completes#3509
Merged
Conversation
ConfigInitDisplay rendered <Box marginTop={1}> plus a content line, so
the live area grew by 2 rows during startup. When initialization
finished and the component unmounted, Ink shrank the live area but the
rows it had already committed to the terminal scrollback cannot be
reclaimed, leaving a visible gap above the input.
Move the MCP init status into the Footer's left-bottom status slot
(always mounted, fixed height) so the live area height stays constant
across the init → ready transition. The status participates in the
existing priority chain: ctrlC / ctrlD / escape / vim / shell /
autoAccept / configInit / hint.
Collaborator
Author
E2E Test CasesConfig Init Display
Show 2 Lines below after initializingAt most 3 lines including statusline text 
Footer with two lines: 
Display without statuslineFirst, show 'initializing...': 
Then, changed to '? for shortcuts': 
|
Audit follow-up. Previously the configInit branch preceded the
suppressHint branch in the footer's left-bottom priority chain. With
a custom status line configured, <Text>{null}</Text> collapses to
zero rows in Ink, so the footer's bottom row went from 1 row during
init to 0 rows after — a 1-row height oscillation that reintroduces
the same scrollback-residue symptom the original fix eliminated in
the default case.
Swap the order so suppressHint short-circuits to null first: the
init message now shares the hint's suppression rule, keeping the
footer's height constant in every configuration.
Also:
- Gate the hook's return on isConfigInitialized directly instead of
letting the effect clear state, avoiding a one-frame flash where
the stale "Initializing..." message leaks through on the first
render after init completes.
- Cover the new behavior with three Footer tests, including a
regression test for the custom-status-line case.
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
Author
测试报告自动化测试
Footer 测试用例第 3 条是关键回归测试：防止未来有人把 盲审覆盖的场景跑了 8 轮无方向审计，核心不变量「live 区高度跨
关键验证点：
需要人工验证（我无法在 CI/沙盒里做的）
如需补充其他 case 的覆盖，请在 review 中指出。 |
Reverting a UX trade-off introduced in the previous commit. That
change suppressed the init message whenever a custom status line was
active, arguing that <Text>{null}</Text> collapses to zero rows in
Ink and any non-zero init row would re-create a one-row shrink on
completion.
Zero shrink was the wrong goal. Hiding init progress from users who
have configured a status line is a real usability loss — the status
line does not surface MCP connection state, so those users now see
no feedback during startup. A one-time, one-line shrink on init
completion is a far smaller regression than the original two-row
scrollback residue this PR was created to fix, and strictly better
than the silent alternative.
Keep the init message in the left-bottom slot and let it sit above
suppressHint in the priority chain. Update the regression test so
that it pins the new behavior (init is visible with or without a
status line) and prevents the suppression from being reintroduced.
BZ-D
commented
Apr 22, 2026
| ) : showAutoAcceptIndicator !== undefined && | ||
| showAutoAcceptIndicator !== ApprovalMode.DEFAULT ? ( | ||
| <AutoAcceptIndicator approvalMode={showAutoAcceptIndicator} /> | ||
| ) : configInitMessage ? ( |
Collaborator
Author
There was a problem hiding this comment.
Moving the init status into Footer regresses screen-reader mode. Composer only renders <Footer /> when !isScreenReaderEnabled, while the old ConfigInitDisplay was rendered regardless, so users running with a screen reader now lose all MCP init progress feedback during startup. We should keep an accessible path for this message outside the footer gate, or special-case screen-reader mode here.
Collaborator
Author
There was a problem hiding this comment.
Good catch — the old ConfigInitDisplay was rendered in Composer outside the !isScreenReaderEnabled && <Footer /> gate, and moving it into the footer silently dropped the init feedback for screen-reader users.
Fixed in 29838ad: Composer now renders configInitMessage as a plain <Text> node when isScreenReaderEnabled is true, reusing the same useConfigInitMessage hook. This keeps the Footer-based rendering (and its constant live-area height) for everyone else — screen-reader users never experienced the residual-blank-line issue that motivated the move in the first place, so an independent text node is harmless for them.
Collaborator
Author
ReviewOverviewFixes #3095 — extra blank line below the status bar at startup. Root-cause analysis is accurate: Approach: move the MCP init status into Code quality
Risks & suggestions🟡 1. One-row residual still exists when a custom statusline is active The code comment at 🟡 2. Vim / Shell / AutoAccept modes mask init status
🟢 3. Test coverage Three new assertions (initializing / initialized / custom statusline) cover the main branches well. Adding 🟢 4. Minor Inline VerdictLGTM, mergeable. Root cause is diagnosed correctly and the fix goes after the only physically correct axis (keep live-area height constant). Hook extraction and the gating detail are handled well, and test coverage is adequate. The main caveat is the residual single row in the custom-statusline case, which is already documented in code comments and test comments as an accepted trade-off — a one-line mention in the PR description would make it easier for reviewers to evaluate. |
Footer is gated behind !isScreenReaderEnabled, so moving the init message inside Footer silenced it for screen-reader users. Render the same message as a plain Text node in Composer when the screen reader is active — screen-reader users don't suffer from the live-area residual row issue that motivated the original move, so an independent node is safe for them.
qqqys
reviewed
Apr 22, 2026
qqqys
left a comment
Collaborator
There was a problem hiding this comment.
Review
Solid, well-motivated fix. The root-cause framing in the description — that scrollback is append-only so any startup-mounted block leaves residue — is exactly right, and moving into the always-mounted Footer slot is the structurally correct solution. A few notes below.
Works well
- Hook extraction is clean. Gating on
isConfigInitializedbefore returning state (rather than clearing from the effect) avoids a transient frame of the stale "Connecting to MCP servers…" string — nice subtlety, good that it's commented. - Lazy
useState(() => t('Initializing...'))avoids re-translating each render. - Accessibility preserved: screen-reader users still get the message inline in
Composerwhere their reader announces it naturally. createMockUIStatenow defaultsisConfigInitialized: trueso existing golden snapshots don't flip — good defensive catch.
Issues / questions
1. The screen-reader path still has the original bug.
{isScreenReaderEnabled && configInitMessage && (
<Text>{configInitMessage}</Text>
)}This is still a conditionally-mounted block — when configInitMessage flips to null it unmounts and leaves a residual row. Smaller (1 row, no marginTop={1}) but not zero. The description claims "the only robust fix is constant live-area height", and this branch quietly violates that. Not a blocker for screen-reader users (visual layout matters less), but worth calling out, or keeping a 1-row placeholder post-init for consistency.
2. Priority-chain ordering vs. autoAccept.
configInit sits after showAutoAcceptIndicator, so a user launched in YOLO / auto-accept-edits will never see the init spinner. The old component rendered regardless of footer state. Is intentional suppression desired here? A one-line rationale comment next to the ordering would help.
3. Doubled subscription.
useConfigInitMessage is now called in both Composer and Footer, producing two independent appEvents.on('mcp-client-update', …) listeners. Perf-wise negligible (events are rare, ~1s window), just semantically redundant. Not worth restructuring for.
4. Test coverage gaps.
- No test for the new
isScreenReaderEnabled && configInitMessagebranch inComposer. - No direct unit test for the
useConfigInitMessagehook — in particular theConnecting to MCP servers… (n/total)path, which is now the only place that string lives.
5. Manual-test checkboxes still unchecked.
The two manual verifications in the test plan are exactly what this PR is about — Ink scrollback behavior is invisible to unit tests. Please run them before merge.
Verdict
Approve-pending on (1) — either acknowledge or fix — and ideally a small hook-level test for the MCP-progress string. Other points are optional.
…ess under YOLO - ScreenReaderAppLayout already mounts <Footer /> directly, so the separate <Text> branch in Composer was producing a duplicated 'Connecting to MCP servers...' line in screen-reader mode. Remove it. - Move configInitMessage ahead of AutoAcceptIndicator in the footer's priority chain so users launched with YOLO / auto-accept-edits still see the ~1s startup progress; the approval-mode indicator takes over as soon as init finishes. - Add unit tests for useConfigInitMessage covering the idle, progress, reset, and unsubscribe paths.
Collaborator
Author
|
Thanks for the thorough review @qqqys — addressed in c4064a6. 1 & 3 (screen-reader path + doubled subscription). You were right that the extra branch in
2 (priority-chain ordering). Agreed — suppressing init progress under YOLO / auto-accept-edits was not intentional. Moved 4 (hook tests). Added 5 (manual checkboxes). Manually verified Re (1) specifically: I agree the screen-reader branch as it stood was not constant-height. Dropping it rather than patching it also matches the "Footer is always mounted at fixed height" framing in the description. |
wenshao
approved these changes
Apr 23, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅ — gpt-5.4 via Qwen Code /review
xaelistic
pushed a commit
to xaelistic/qwen-code
that referenced
this pull request
Jun 7, 2026
…M#3509) * fix(cli): remove residual blank lines after MCP init completes (QwenLM#3095) ConfigInitDisplay rendered <Box marginTop={1}> plus a content line, so the live area grew by 2 rows during startup. When initialization finished and the component unmounted, Ink shrank the live area but the rows it had already committed to the terminal scrollback cannot be reclaimed, leaving a visible gap above the input. Move the MCP init status into the Footer's left-bottom status slot (always mounted, fixed height) so the live area height stays constant across the init → ready transition. The status participates in the existing priority chain: ctrlC / ctrlD / escape / vim / shell / autoAccept / configInit / hint. * fix(cli): suppress MCP init message when custom status line is active Audit follow-up. Previously the configInit branch preceded the suppressHint branch in the footer's left-bottom priority chain. With a custom status line configured, <Text>{null}</Text> collapses to zero rows in Ink, so the footer's bottom row went from 1 row during init to 0 rows after — a 1-row height oscillation that reintroduces the same scrollback-residue symptom the original fix eliminated in the default case. Swap the order so suppressHint short-circuits to null first: the init message now shares the hint's suppression rule, keeping the footer's height constant in every configuration. Also: - Gate the hook's return on isConfigInitialized directly instead of letting the effect clear state, avoiding a one-frame flash where the stale "Initializing..." message leaks through on the first render after init completes. - Cover the new behavior with three Footer tests, including a regression test for the custom-status-line case. * fix(cli): show MCP init progress even under a custom status line Reverting a UX trade-off introduced in the previous commit. That change suppressed the init message whenever a custom status line was active, arguing that <Text>{null}</Text> collapses to zero rows in Ink and any non-zero init row would re-create a one-row shrink on completion. Zero shrink was the wrong goal. Hiding init progress from users who have configured a status line is a real usability loss — the status line does not surface MCP connection state, so those users now see no feedback during startup. A one-time, one-line shrink on init completion is a far smaller regression than the original two-row scrollback residue this PR was created to fix, and strictly better than the silent alternative. Keep the init message in the left-bottom slot and let it sit above suppressHint in the priority chain. Update the regression test so that it pins the new behavior (init is visible with or without a status line) and prevents the suppression from being reintroduced. * fix(cli): keep MCP init progress visible in screen-reader mode Footer is gated behind !isScreenReaderEnabled, so moving the init message inside Footer silenced it for screen-reader users. Render the same message as a plain Text node in Composer when the screen reader is active — screen-reader users don't suffer from the live-area residual row issue that motivated the original move, so an independent node is safe for them. * refactor(cli): drop duplicated screen-reader init path and show progress under YOLO - ScreenReaderAppLayout already mounts <Footer /> directly, so the separate <Text> branch in Composer was producing a duplicated 'Connecting to MCP servers...' line in screen-reader mode. Remove it. - Move configInitMessage ahead of AutoAcceptIndicator in the footer's priority chain so users launched with YOLO / auto-accept-edits still see the ~1s startup progress; the approval-mode indicator takes over as soon as init finishes. - Add unit tests for useConfigInitMessage covering the idle, progress, reset, and unsubscribe paths.
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
3 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
Fixes #3095. During startup
ConfigInitDisplayrendered a<Box marginTop={1}>plus a content line, so Ink's live area grew by 2 rows. When initialization finished (~1s) and the component unmounted, Ink shrank the live area but the rows it had already committed to the terminal scrollback cannot be reclaimed — leaving a visible blank gap above the input.Root cause (first principles): any component that appears in the live area at startup and later unmounts will leave residual rows in scrollback, because terminal rows that have scrolled past the live region are physically fixed. The only robust fix is to keep the live area height constant across the init → ready transition.
Approach: move the MCP init status into the
Footer's left-bottom status slot —Footeris always mounted at fixed height, so the init message just occupies the same row as the idle hint (? for shortcuts). The status participates in the existing priority chain:ctrlC / ctrlD / escape / vim / shell / autoAccept / configInit / hint. Zero layout shift.Changes
useConfigInitMessage(isConfigInitialized)— subscribes tomcp-client-updateand returns a string ornull(extracted from the deleted component).Composer.tsx: drop<ConfigInitDisplay />and its import.Footer.tsx: insertconfigInitMessagebranch intoleftBottomContent, rendered with<GeminiSpinner />+ text.ConfigInitDisplay.tsx.Footer.test.tsx: mockUIStatenow setsisConfigInitialized: true(otherwise the init branch fires and changes the snapshot).Test plan
vitest run packages/cli/src/ui/components/Footer— 8/8 passvitest run packages/cli/src/ui/components/Composer— 14/14 passtsc --noEmitclean for all touched filesInitializing...briefly in footer, then normal hint