feat(cli): add MCP health pill to footer#3741
Merged
Merged
Conversation
Surfaces MCP servers stuck in DISCONNECTED next to the Background tasks pill, so a failed-to-connect or dropped MCP isn't invisible to the user until they happen to run /mcp. v1 is a visual indicator only — Down-arrow focus chain into the pill (Enter to open /mcp) is deferred to a follow-up to keep this PR small and validate the right scope first. - New `useMCPHealth` hook subscribes to mcp-client's listener API and exposes raw counts (total / disconnected / connecting / connected) rather than a pre-formatted label, so future surfaces (boot screen, tooltips) can derive their own presentation. - `MCPHealthPill` component renders ` · N MCPs offline` (warning color) when `disconnectedCount > 0`, hidden otherwise. Connecting is intentionally suppressed to avoid flicker during boot/reconnect — the state that matters is the one that doesn't recover on its own. - Footer renders MCPHealthPill after BackgroundTasksPill, sharing the same left-bottom region. This is a deliberate parallel-pill pattern, not a `kind` extension of the Background tasks dialog: MCP connections have no terminal status (disconnected ↔ connecting ↔ connected, with a 30s health-check auto-reconnect loop), so they don't fit the task contract that the combined dialog is built around.
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
commented
Apr 29, 2026
wenshao
left a comment
Collaborator
Author
There was a problem hiding this comment.
No issues found. LGTM! ✅ — gpt-5.5 via Qwen Code /review
doudouOUC
approved these changes
May 2, 2026
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Summary
Adds MCPHealthPill to the footer — surfaces MCP servers stuck in DISCONNECTED next to the existing Background tasks pill. Design decision to keep it as a parallel pill (not extending Background Tasks framework) is well-justified: MCPs have a non-terminal lifecycle cycle (disconnected ↔ connecting ↔ connected) that doesn't fit the terminal-status contract of background tasks.
Warnings
W1. Hook subscription logic lacks test coverage
useMCPHealth.ts has non-trivial subscription logic (listener register/remove, mount resync, incremental Map merge) but no tests. Current tests only cover the pure function getPillLabel. This is consistent with useBackgroundTaskView (which also lacks hook-level tests), but the MCP hook's incremental Map merge pattern is more nuanced. Suggest adding at minimum:
it('registers listener on mount and removes on unmount', () => {
const { unmount } = renderHook(() => useMCPHealth());
expect(addMCPStatusChangeListener).toHaveBeenCalledTimes(1);
unmount();
expect(removeMCPStatusChangeListener).toHaveBeenCalledTimes(1);
});
it('resyncs state on mount', () => {
vi.mocked(getAllMCPServerStatuses).mockReturnValue(
new Map([['server-a', MCPServerStatus.DISCONNECTED]]),
);
const { result } = renderHook(() => useMCPHealth());
expect(result.current.disconnectedCount).toBe(1);
});W2. Component rendering behavior untested
MCPHealthPill returns null when healthy and renders warning-colored text when disconnected — this behavioral contract has no test protection. Easy to silently break in future refactors. Suggest adding an Ink rendering test to verify both paths.
Suggestions
S1. Add comment for server-removal assumption
The listener only does next.set(name, status) — if the core API ever adds server removal (removeMCPServerStatus), the hook would retain stale entries. Worth a brief comment near line 44:
// NOTE: core API currently has no server-removal path, so set() is
// sufficient. If removeMCPServerStatus is added, this needs next.delete(name).S2. useSyncExternalStore consistency (low priority)
Footer.tsx and useIdeTrustListener.ts use useSyncExternalStore for external subscriptions, while this PR uses useState + useEffect. However, the directly comparable useBackgroundTaskView also uses useState + useEffect, so this is consistent with the peer hook. No action needed — only worth revisiting if React concurrent mode becomes relevant.
Test checklist
-
getPillLabel— 6 cases, all passing - Hook subscription lifecycle (register / unmount → remove)
- Hook mount resync (state changed before listener attached)
- Component render path (null vs warning-colored pill)
- Manual smoke: bad MCP config → pill appears → fix config → pill clears
Overall: clean implementation, sound architecture, no bugs or security issues. The test gap (W1/W2) is the main improvement area but doesn't block merge given the scope and existing patterns. Approving.
🤖 Generated with Qwen Code
3 tasks
mabry1985
added a commit
to protoLabsAI/protoCLI
that referenced
this pull request
May 3, 2026
Cherry-picked from QwenLM/qwen-code: 9e8f826 Adds a small status pill to the footer showing MCP server health (connected / connecting / disconnected counts). Subscribes to the mcp-client module-level listener API and re-renders on transitions. Adaptations: - Skipped upstream's Footer left-column restructure (statusLineLines + leftBottomContent + BackgroundTasksPill) — those depend on the un-ported background-agents subsystem. Instead, render <MCPHealthPill /> alongside our existing leftContent in the same Box, preserving our simpler footer layout. - Skipped the BackgroundTasksPill import for the same reason. The hook (`useMCPHealth`) and component (`MCPHealthPill`) added verbatim from the cherry-pick — both new files, no fork-side divergence. Co-authored-by: Automaker <automaker@localhost> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
TaimoorSiddiquiOfficial
pushed a commit
to TaimoorSiddiquiOfficial/HopCode
that referenced
this pull request
May 3, 2026
DragonnZhang
pushed a commit
that referenced
this pull request
May 8, 2026
Surfaces MCP servers stuck in DISCONNECTED next to the Background tasks pill, so a failed-to-connect or dropped MCP isn't invisible to the user until they happen to run /mcp. v1 is a visual indicator only — Down-arrow focus chain into the pill (Enter to open /mcp) is deferred to a follow-up to keep this PR small and validate the right scope first. - New `useMCPHealth` hook subscribes to mcp-client's listener API and exposes raw counts (total / disconnected / connecting / connected) rather than a pre-formatted label, so future surfaces (boot screen, tooltips) can derive their own presentation. - `MCPHealthPill` component renders ` · N MCPs offline` (warning color) when `disconnectedCount > 0`, hidden otherwise. Connecting is intentionally suppressed to avoid flicker during boot/reconnect — the state that matters is the one that doesn't recover on its own. - Footer renders MCPHealthPill after BackgroundTasksPill, sharing the same left-bottom region. This is a deliberate parallel-pill pattern, not a `kind` extension of the Background tasks dialog: MCP connections have no terminal status (disconnected ↔ connecting ↔ connected, with a 30s health-check auto-reconnect loop), so they don't fit the task contract that the combined dialog is built around. Co-authored-by: wenshao <wenshao@U-K7F6PQY3-2157.local>
xaelistic
pushed a commit
to xaelistic/qwen-code
that referenced
this pull request
Jun 7, 2026
xaelistic
pushed a commit
to xaelistic/qwen-code
that referenced
this pull request
Jun 7, 2026
Surfaces MCP servers stuck in DISCONNECTED next to the Background tasks pill, so a failed-to-connect or dropped MCP isn't invisible to the user until they happen to run /mcp. v1 is a visual indicator only — Down-arrow focus chain into the pill (Enter to open /mcp) is deferred to a follow-up to keep this PR small and validate the right scope first. - New `useMCPHealth` hook subscribes to mcp-client's listener API and exposes raw counts (total / disconnected / connecting / connected) rather than a pre-formatted label, so future surfaces (boot screen, tooltips) can derive their own presentation. - `MCPHealthPill` component renders ` · N MCPs offline` (warning color) when `disconnectedCount > 0`, hidden otherwise. Connecting is intentionally suppressed to avoid flicker during boot/reconnect — the state that matters is the one that doesn't recover on its own. - Footer renders MCPHealthPill after BackgroundTasksPill, sharing the same left-bottom region. This is a deliberate parallel-pill pattern, not a `kind` extension of the Background tasks dialog: MCP connections have no terminal status (disconnected ↔ connecting ↔ connected, with a 30s health-check auto-reconnect loop), so they don't fit the task contract that the combined dialog is built around. Co-authored-by: wenshao <wenshao@U-K7F6PQY3-2157.local>
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
Surfaces MCP servers stuck in
DISCONNECTEDnext to the existing Background tasks pill, so a failed-to-connect or dropped MCP server isn't invisible to the user until they happen to run/mcp.v1 is a visual indicator only — Down-arrow focus chain into the pill (Enter to open
/mcp) is deferred to a follow-up to keep this PR small and validate the right scope first.Size: +182 / 0 across 4 files (3 new + 1 modified).
Before / After
Before: MCP server in
DISCONNECTEDstate — UI gives no signal until the user runs/mcpor notices a tool from that server failing.After: Footer left-bottom region shows
· 1 MCP offline(warning color) when any configured server is inDISCONNECTED. The pill auto-clears once the 30s health-check auto-reconnect succeeds.Why a parallel pill, not a
kindextension of Background tasks dialogInitial framing was to add
kind: 'mcp'to the combined Background tasksDialogEntryunion (#3720). On investigation that's a category error:running → { completed | failed | cancelled }, plus anotifiedfield that guarantees one terminal notification). MCPs have no terminal status —disconnected ↔ connecting ↔ connectedis a connection-state cycle with a 30s health-check auto-reconnect loop.ListBody/DetailBody/terminalStatusPresentationare all built around the terminal-status contract; bending them to host an indefinite-state entry would dilute the framework's invariants./mcpalready opensMCPManagementDialogwith per-server status / tools / OAuth — that's the right surface for MCP detail. The Footer pill is purely an attention indicator that points there.So: parallel pill in the same Footer region as
BackgroundTasksPill, sharing the layout pattern but not the registry contract.What changed
packages/cli/src/ui/hooks/useMCPHealth.ts(new) — subscribes tomcp-client's module-level listener API (addMCPStatusChangeListener/removeMCPStatusChangeListener/getAllMCPServerStatuses) and exposes raw counts (totalCount/disconnectedCount/connectingCount/connectedCount). Returns counts not a formatted label, so future surfaces (boot screen, tooltips) can derive their own presentation.packages/cli/src/ui/components/mcp/MCPHealthPill.tsx(new) — renders· N MCP(s) offlinein warning color whendisconnectedCount > 0, hidden otherwise.getPillLabelis exported for testability.packages/cli/src/ui/components/Footer.tsx— renders<MCPHealthPill />immediately after<BackgroundTasksPill />in the left-bottom region.MCPHealthPill.test.tsx— 6 cases covering empty / all-connected / all-connecting / 1 offline (singular) / N offline (plural) / mixed states.Design notes
Why suppress
connectingfrom the pill in v1? Boot and reconnect cycles transit throughCONNECTINGfor a few seconds; surfacing it would make the pill flicker during normal operation. The state that doesn't recover on its own — and therefore deserves user attention — isDISCONNECTED. If a server gets stuck inCONNECTING, the existing 30s health check will eventually mark itDISCONNECTED.Why a separate hook + component instead of extending the Background tasks framework? See above — different lifecycle contract, different existing detail surface. The pill framework (Footer left-bottom inline pills) absorbs a second consumer cleanly without forcing the registry framework to absorb a non-task type.
Test plan
vitest runpackages/cli/src/ui/components/mcp/MCPHealthPill.test.tsx — 6 / 6 passtsc --noEmitclean (cli package)1 MCP offlinein warning color in the footer; fix the config / restart → confirm pill clears once server reconnects.Follow-ups (not in this PR)
/mcp. Coordinating focus across multiple parallel pills (BackgroundTasksPill + MCPHealthPill) needs a small focus-chain abstraction; deferred until there's pressure.connectingafter a stuck-threshold (e.g., >5s inCONNECTING) so users see when an MCP is hanging mid-handshake.