feat(mcp): connection health toolbar with bulk Retry All / Disconnect All actions#2641
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (16)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughThis PR introduces ChangesMCP Connection Health Toolbar
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…les (fixes CI 'Type Check' format gate)
oxoxDev
left a comment
oxoxDev
left a comment
There was a problem hiding this comment.
Walkthrough
New McpConnectionHealthToolbar component above the installed servers list — aggregate per-state counts (connected/connecting/error/idle), Retry All button (visible when errors exist), Disconnect All (gated behind role="dialog" aria-modal confirmation). Bulk RPCs use Promise.allSettled so one bad apple doesn't abort the batch. Status summary in role="status" aria-live="polite" region. 15 new mcp.health.* i18n keys + 17 Vitest tests. CodeRabbit dismissed initial review then APPROVED. CI: only Rust Core Windows fails (pre-existing main flake; unrelated to FE PR).
Verified
computeHealthCountsis a pure exported helper,useMemokeyed onstatuses— counts can't diverge from the IDs the bulk actions target.- Both bulk handlers wired in
McpServersTab.tsx:140-156viaPromise.allSettledoverserverIds.map(mcpClientsApi.connect/disconnect)— orchestration matches PR claim. - Toolbar renders only when
statuses.length > 0→ preserves the "Browse catalog" empty-state CTA. - 13 locale
-1.tschunks +en.tsall carry 15 keys ✓
Major findings
- i18n parity gap —
pl-1.ts(Polish) has 0 of the 15 newmcp.health.*keys vs 15 in every other locale. Same root cause as #2639: Polish locale was added toupstream/main(commit4043d03e, PR #2731) AFTER this branch was cut on May 25. Will failpnpm i18n:checkonce rebased. Same fix: rebase + add the 15mcp.health.*keys topl-1.tsusing English values as placeholders per the standard pattern.
Nits
McpConnectionHealthToolbar:80—confirmDisconnectstate has no escape-key handler test. Esc should close the dialog without firing the bulk RPC. Worth a one-line test usingfireEvent.keyDown(dialog, { key: 'Escape' }).- The Retry All flow calls
Promise.allSettledbut doesn't surface per-server failure counts. After "Retry All (5)" runs, user only knows 5 attempts happened, not how many succeeded. Consider exposing{ succeeded, failed }inopErrorso the toast/alert region tells the user "3 succeeded, 2 failed" — actionable, otherwise they re-scan dots.
Questions
- The 5-second polling loop continues running while the confirmation dialog is open. If a server transitions to disconnected mid-dialog, the count behind the modal silently changes. Acceptable, or should the dialog snapshot the count at open time?
|
I see @oxoxDev has requested changes — deferring to their feedback. Will review once those are addressed. |
Addresses maintainer review on tinyhumansai#2641: - Escape now closes the 'Disconnect all' confirmation dialog without firing the bulk disconnect RPC, matching the modal-dismiss affordance of the other MCP dialogs. The keydown listener attaches only while the dialog is open and is declared before the component's early return to respect the rules of hooks. New test pins Esc -> dialog closed + onDisconnect not called. - Merged upstream/main (160 commits) to de-stale. My tinyhumansai#2655 inventory feature has since landed on main, so McpServersTab.tsx + the i18n chunks now carry BOTH the health-toolbar and inventory keys/wiring; conflicts resolved as the union (both features coexist in the tab). pl-1.ts spreads ...en1 so it inherits all keys with English fallback. Validation: i18n:check EXIT 0 (0 missing, all locales incl. pl), 18 health-toolbar tests pass, tsc clean, eslint clean (only a pre-existing baseline warning in unrelated McpServersTab code), prettier clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
PR-PLAYBOOK.md (2)
32-34: ⚡ Quick winAdd language specifier to fenced code block.
The fenced code block for the PR title should specify a language for better rendering and accessibility. Consider using
textormarkdown.📝 Proposed fix
### Title -``` +```text docs: truth-up architecture.md + 3 new domain pages + Linux/Arch caveats in localized READMEs</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@PR-PLAYBOOK.mdaround lines 32 - 34, The fenced code block containing the PR
title line "docs: truth-up architecture.md + 3 new domain pages + Linux/Arch
caveats in localized READMEs" should include a language specifier (e.g., use
text ormarkdown) so the block renders/accesses correctly; update that
fenced code block surrounding that exact string to start with a language token.</details> --- `188-189`: _⚡ Quick win_ **Add language specifier to fenced code block.** The fenced code block for the PR title should specify a language for better rendering and accessibility. Consider using `text` or `markdown`. <details> <summary>📝 Proposed fix</summary> ```diff ### Title -``` +```text feat(mcp): i18n + a11y McpStatusBadge status labels ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@PR-PLAYBOOK.mdaround lines 188 - 189, The fenced code block containing the
PR title "feat(mcp): i18n + a11y McpStatusBadge status labels" lacks a language
specifier; update that fence to include a language (e.g., changetotext
ormarkdown) so the block is rendered/accessibility-friendly, ensuring the opening fence readstext (ormarkdown) and the closing fence remains;
no other changes to the text are needed.</details> </blockquote></details> <details> <summary>PR8-body.md (1)</summary><blockquote> `57-65`: _⚡ Quick win_ **Add language specifier to fenced code block.** The terminal output block should specify a language for better rendering. Consider using `text` or `console`. <details> <summary>📝 Proposed fix</summary> ```diff Verified locally: -``` +```text $ pnpm i18n:check … ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@PR8-body.mdaround lines 57 - 65, The fenced code block containing the
terminal output starting with "$ pnpm i18n:check" should include a language
specifier for proper rendering; update the opening fence (the triple backticks
before the terminal output) to use a language like "text" or "console" (e.g.,
changetotext) so the block is recognized as plain terminal text.</details> </blockquote></details> <details> <summary>PR6-body.md (2)</summary><blockquote> `26-32`: _⚡ Quick win_ **Add language specifier to fenced code block.** The ASCII diagram block should specify a language for better rendering. Consider using `text`. <details> <summary>📝 Proposed fix</summary> ```diff **Layout** (compact, fits the 224px left pane): -``` +```text ┌────────────────────────────────────────┐ │ HEALTH Retry all (1) Disconnect │ ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@PR6-body.mdaround lines 26 - 32, The fenced ASCII diagram uses unlabelled
triple-backticks which prevents proper syntax rendering; update the opening
fence in the block containing the box diagram (the triple-backtick that precedes
"┌────────────────────────────────────────┐") to include a language specifier
such as text (e.g., changetotext) so the diagram is rendered as plain
text.</details> --- `66-74`: _⚡ Quick win_ **Add language specifier to fenced code block.** The terminal output block should specify a language for better rendering. Consider using `text` or `console`. <details> <summary>📝 Proposed fix</summary> ```diff Verified locally: -``` +```text $ pnpm i18n:check … ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@PR6-body.mdaround lines 66 - 74, The fenced terminal output in PR6-body.md
(the block that starts with "$ pnpm i18n:check" and shows language summaries
like "## zh-CN (2997 keys)") lacks a language specifier; update that fenced
code block to include a language tag such as text or console (e.g., change "" to "text") so the terminal output renders correctly.</details> </blockquote></details> <details> <summary>PR7-body.md (1)</summary><blockquote> `82-90`: _⚡ Quick win_ **Add language specifier to fenced code block.** The terminal output block should specify a language for better rendering. Consider using `text` or `console`. <details> <summary>📝 Proposed fix</summary> ```diff Verified locally: -``` +```text $ pnpm i18n:check … ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@PR7-body.mdaround lines 82 - 90, The fenced code block showing the terminal
output (the block that begins with "$ pnpm i18n:check" and the following lines
showing language keys and EXIT: 0) lacks a language specifier; update the
opening fence fromtotext (or ```console) so the block is annotated
(e.g., change the fence that encloses the "$ pnpm i18n:check …" output to
include "text").</details> </blockquote></details> <details> <summary>PR5-body.md (1)</summary><blockquote> `60-70`: _⚡ Quick win_ **Add language specifier to fenced code block.** The terminal output block should specify a language for better rendering. Consider using `text` or `console`. <details> <summary>📝 Proposed fix</summary> ```diff Verified locally: -``` +```text $ pnpm i18n:check … ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@PR5-body.mdaround lines 60 - 70, The fenced terminal output block that
shows the "$ pnpm i18n:check" run and per-chunk summary should include a
language specifier for correct rendering; update the triple-backtick fence
around that block (the block starting with "$ pnpm i18n:check" and containing
lines like "## zh-CN (2988 keys)" and "per-chunk: 1:1197/1197 ...") to use a
language tag such as text or console (e.g., ```text) so the snippet renders
properly.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Inline comments:
In@PR8-body.md:
- Line 80: Replace the British English spelling "memoised" with the American
English "memoized" in the PR body text that reads "Performance: Manifest
build / parse is O(n) over the server list with one stable sort. Pure functions,
memoised in the Export tab. No new polling, no new RPC, no new network
round-trips." so the sentence uses "memoized" to match other PR docs using
"memoization".
Nitpick comments:
In@PR-PLAYBOOK.md:
- Around line 32-34: The fenced code block containing the PR title line "docs:
truth-up architecture.md + 3 new domain pages + Linux/Arch caveats in localized
READMEs" should include a language specifier (e.g., usetext ormarkdown)
so the block renders/accesses correctly; update that fenced code block
surrounding that exact string to start with a language token.- Around line 188-189: The fenced code block containing the PR title "feat(mcp):
i18n + a11y McpStatusBadge status labels" lacks a language specifier; update
that fence to include a language (e.g., changetotext ormarkdown) so the block is rendered/accessibility-friendly, ensuring the opening fence readstext (ormarkdown) and the closing fence remains; no other changes to
the text are needed.In
@PR5-body.md:
- Around line 60-70: The fenced terminal output block that shows the "$ pnpm
i18n:check" run and per-chunk summary should include a language specifier for
correct rendering; update the triple-backtick fence around that block (the block
starting with "$ pnpm i18n:check" and containing lines like "## zh-CN (2988
keys)" and "per-chunk: 1:1197/1197 ...") to use a language tag such as text or
console (e.g., ```text) so the snippet renders properly.In
@PR6-body.md:
- Around line 26-32: The fenced ASCII diagram uses unlabelled triple-backticks
which prevents proper syntax rendering; update the opening fence in the block
containing the box diagram (the triple-backtick that precedes
"┌────────────────────────────────────────┐") to include a language specifier
such as text (e.g., changetotext) so the diagram is rendered as plain
text.- Around line 66-74: The fenced terminal output in PR6-body.md (the block that
starts with "$ pnpm i18n:check" and shows language summaries like "## zh-CN
(2997 keys)") lacks a language specifier; update that fenced code block to
include a language tag such as text or console (e.g., change "" to "text")
so the terminal output renders correctly.In
@PR7-body.md:
- Around line 82-90: The fenced code block showing the terminal output (the
block that begins with "$ pnpm i18n:check" and the following lines showing
language keys and EXIT: 0) lacks a language specifier; update the opening fence
fromtotext (or ```console) so the block is annotated (e.g., change the
fence that encloses the "$ pnpm i18n:check …" output to include "text").In
@PR8-body.md:
- Around line 57-65: The fenced code block containing the terminal output
starting with "$ pnpm i18n:check" should include a language specifier for proper
rendering; update the opening fence (the triple backticks before the terminal
output) to use a language like "text" or "console" (e.g., changetotext)
so the block is recognized as plain terminal text.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `13245fc3-3cd4-4a69-a45f-bf48a55477fa` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 05cf85fe0412e01c14295b65dcc9f90cae386ba9 and 7c132817f3335c3021b06f28672b89d2512a7732. </details> <details> <summary>📒 Files selected for processing (24)</summary> * `PR-PLAYBOOK.md` * `PR2-body.md` * `PR3-mcp-status-badge.patch` * `PR5-body.md` * `PR6-body.md` * `PR7-body.md` * `PR8-body.md` * `app/src/components/channels/mcp/McpConnectionHealthToolbar.test.tsx` * `app/src/components/channels/mcp/McpConnectionHealthToolbar.tsx` * `app/src/components/channels/mcp/McpServersTab.tsx` * `app/src/lib/i18n/chunks/ar-1.ts` * `app/src/lib/i18n/chunks/bn-1.ts` * `app/src/lib/i18n/chunks/de-1.ts` * `app/src/lib/i18n/chunks/en-1.ts` * `app/src/lib/i18n/chunks/es-1.ts` * `app/src/lib/i18n/chunks/fr-1.ts` * `app/src/lib/i18n/chunks/hi-1.ts` * `app/src/lib/i18n/chunks/id-1.ts` * `app/src/lib/i18n/chunks/it-1.ts` * `app/src/lib/i18n/chunks/ko-1.ts` * `app/src/lib/i18n/chunks/pt-1.ts` * `app/src/lib/i18n/chunks/ru-1.ts` * `app/src/lib/i18n/chunks/zh-CN-1.ts` * `app/src/lib/i18n/en.ts` </details> <details> <summary>💤 Files with no reviewable changes (14)</summary> * app/src/lib/i18n/chunks/ko-1.ts * app/src/lib/i18n/chunks/zh-CN-1.ts * app/src/lib/i18n/chunks/id-1.ts * app/src/lib/i18n/chunks/en-1.ts * app/src/lib/i18n/chunks/bn-1.ts * app/src/lib/i18n/chunks/de-1.ts * app/src/lib/i18n/chunks/ar-1.ts * app/src/lib/i18n/chunks/pt-1.ts * app/src/lib/i18n/chunks/it-1.ts * app/src/lib/i18n/en.ts * app/src/lib/i18n/chunks/ru-1.ts * app/src/lib/i18n/chunks/fr-1.ts * app/src/lib/i18n/chunks/hi-1.ts * app/src/lib/i18n/chunks/es-1.ts </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (3)</summary> * app/src/components/channels/mcp/McpServersTab.tsx * app/src/components/channels/mcp/McpConnectionHealthToolbar.test.tsx * app/src/components/channels/mcp/McpConnectionHealthToolbar.tsx </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
Thanks @oxoxDev! Addressed in the latest push: Nit — Esc on the Disconnect-all dialog: added an Escape handler that closes the confirmation without firing the bulk RPC, matching the modal-dismiss affordance of the other MCP dialogs. The listener attaches only while the dialog is open and is declared before the component's early return (rules of hooks). New test pins Esc → dialog closed + Major — i18n parity (pl): rebased onto current main. The Retry-All success/failure-count nit and the dialog-snapshot question are good follow-ups; happy to take them in a focused PR so this one stays scoped to the toolbar. Validation: i18n:check EXIT 0, 18 toolbar tests pass, tsc clean, eslint clean, prettier clean. |
# Conflicts: # app/src/components/channels/mcp/McpServersTab.tsx # app/src/lib/i18n/chunks/ar-1.ts # app/src/lib/i18n/chunks/bn-1.ts # app/src/lib/i18n/chunks/de-1.ts # app/src/lib/i18n/chunks/en-1.ts # app/src/lib/i18n/chunks/es-1.ts # app/src/lib/i18n/chunks/fr-1.ts # app/src/lib/i18n/chunks/hi-1.ts # app/src/lib/i18n/chunks/id-1.ts # app/src/lib/i18n/chunks/it-1.ts # app/src/lib/i18n/chunks/ko-1.ts # app/src/lib/i18n/chunks/pt-1.ts # app/src/lib/i18n/chunks/ru-1.ts # app/src/lib/i18n/chunks/zh-CN-1.ts # app/src/lib/i18n/en.ts
…plan.md` and `PR-PLAYBOOK.md`, which contained obsolete information regarding contribution guidelines and project plans. This cleanup helps streamline the repository and eliminate confusion for contributors.
…lert region
Promise.allSettled in handleBulkReconnect/handleBulkDisconnect swallowed
every per-server rejection, so a bulk action that partially (or wholly)
failed looked identical to success and the toolbar's role="alert"
opError path was effectively dead in production. Count rejected
settlements after the status refresh and throw a descriptive error so
the existing alert region reports '{failed} of {total} servers failed'.
Adds the mcp.health.bulkPartialFailure i18n key across en.ts + all 13
locale chunk-1 files (pl inherits via its ...en1 spread) and two
McpServersTab tests covering the Retry All / Disconnect All failure
surfacing. Addresses @oxoxDev review (surface succeeded/failed counts).
| <p | ||
| id="mcp-disconnect-all-body" | ||
| className="text-xs text-stone-600 dark:text-neutral-300 mb-4"> | ||
| {t('mcp.health.disconnectConfirm.body').replace( |
There was a problem hiding this comment.
Re: @oxoxDev's open question about the 5s poll continuing while the Disconnect-all confirm dialog is open —
Current behavior is intentional and I'd argue it's the safer of the two options: runDisconnectAll reads the live memoized counts.connectedIds at confirm time (not a value captured at dialog-open), and the dialog body interpolates the same live counts.connectedCount. So if a server drops to disconnected mid-dialog, both the displayed count and the actual disconnect target stay in sync — we never disconnect a server the user can no longer see as connected, and we never act on a stale snapshot.
Snapshotting at open time would freeze the body text but then act on a list that may have changed, which is the failure mode worth avoiding. The only cosmetic wrinkle is the body number can tick while the modal is open; if that's distracting we could freeze the displayed number while still acting on the live set, but I'd lean toward leaving it as-is for correctness. Deferring the final call to you — happy to add the display-freeze if you prefer.
|
I see @oxoxDev has requested changes — deferring to their feedback. Will review once those are addressed. |
4 participants
Summary
Channels → ChannelConfigPanel → McpServersTab. Surfaces aggregate per-state counts (connected / connecting / error / idle) and exposes two bulk actions that previously required clicking into each server one at a time.(N)— visible only when there are errored servers. Iterates through them viamcpClientsApi.connect(id)wrapped inPromise.allSettledso one bad apple doesn't abort the batch, then refreshes statuses once at the end.(N)— visible only when there are connected servers. Gated behind arole="dialog" aria-modalconfirmation dialog before the bulk RPC fires.role="status" aria-live="polite"region so screen-reader users hear updates as the 5-second polling loop refreshes. Status dots arearia-hidden. Buttons have descriptivearia-labels with interpolated counts.mcp.health.*mirrored across all 13 locale chunks (English values used as untranslated placeholders per the repo's standard pattern).Promise.allSettled, error surfacing throughrole="alert", and the a11y attribute contract on every interactive element.Problem
MCP servers in this app can drift into
errorstate for legitimate reasons — a key expired, a managed runtime restart cycle, a flaky upstream HTTP-remote server. Today, recovering means clicking each errored server individually in the list, navigating to its detail view, and hitting connect. With three or four servers it's tedious; with the ten-plus shape that real Smithery users hit, it's a chore.Disconnect-all is symmetric — useful during debugging ("what changes if I cut all MCP connections?"), useful during shutdown, useful for the user who just installed a server with bad env vars and wants a clean reset across the board.
There's also no aggregate at-a-glance status. The list shows per-server dots, but counting "how many am I currently connected to?" requires scanning. For users with a dozen servers, that scan happens visually every time they open the tab.
Solution
New component
McpConnectionHealthToolbar.tsxRenders only when
statuses.length > 0— preserves the existing "empty install + Browse catalog CTA" empty state by not competing for vertical space.Layout (compact, fits the 224px left pane):
Status pills use the existing color vocabulary from
McpStatusBadge(sage/amber/coral/stone). Dots flow on aflex flex-wraprow so the layout doesn't break at narrower widths.Aggregation (in
computeHealthCounts, exported as a pure helper):statusesproducingconnectedIds,errorIds, and counts forconnecting/disconnected.useMemokeyed onstatusesso the bulk action targets don't recompute on unrelated re-renders.connectedIdsanderrorIdsare arrays (not just counts) — the same memoization payload powers both the visual badges and the bulk-action callbacks, so there's no risk of the displayed count diverging from the IDs that would be acted on.Bulk action flow (Retry All):
Retry all (N)— callsonReconnect(errorIds); setsisOperating=trueto disable both buttons.McpServersTab.handleBulkReconnect) doesPromise.allSettled(errorIds.map(id => mcpClientsApi.connect(id)))followed byfetchStatuses().isOperating=false; the next render reflects the new statuses; the buttons re-enable.setOpError(err.message)displays the failure in arole="alert"paragraph below the summary. Falls back tot('mcp.health.opErrorGeneric')for non-Error throws.Bulk action flow (Disconnect All):
Disconnect all (N)— opens confirmation dialog withrole="dialog" aria-modal="true"and explicitaria-labelledby/aria-describedbyreferencing the title and body IDs.onDisconnectnever fires.Promise.allSettledorchestration viaonDisconnect(connectedIds).Wiring in
McpServersTab.tsxhandleBulkReconnectandhandleBulkDisconnectasuseCallbacks alongside the existinghandleInstallSuccess/handleUninstalledhandlers. Both callPromise.allSettledon per-server RPC thenfetchStatuses()for one round-trip refresh.<McpConnectionHealthToolbar>above<InstalledServerList>in the left pane, after the existingloadErrorblock.i18n
15 new keys under the
mcp.health.*namespace added toapp/src/lib/i18n/en.tsAND toapp/src/lib/i18n/chunks/en-1.ts(the runtime chunk that already holds the existingmcp.*keys). All 12 non-English locale chunks (ar-1.ts…zh-CN-1.ts) get the same six keys with the English value as the untranslated placeholder, per the project pattern thatscripts/i18n-coverage.tsenforces.Verified locally:
Submission Checklist
McpConnectionHealthToolbar.test.tsx. Cover: empty-state return-null path; correct count aggregation across mixed status sets; conditional rendering of connecting / error pills only when non-zero; conditional rendering of Retry All only when errors exist; conditional rendering of Disconnect All only when connected exist; "Retry all" callsonReconnectwith exactly the errored IDs; "Disconnect all" opens a confirmation dialog rather than firing directly; cancel closes the dialog without callingonDisconnect; confirm firesonDisconnectwith exactly the connected IDs and closes the dialog; both action buttons are disabled during a pending bulk operation; errors thrown fromonReconnectsurface viarole="alert"; non-Error throws fall back to a generic message; the summary region hasrole="status"+aria-live="polite"+ a descriptivearia-label; the confirmation dialog body interpolates the correct connected count.McpConnectionHealthToolbar.tsxand the newhandleBulkReconnect/handleBulkDisconnectcallbacks inMcpServersTab.tsxare exercised by the test suite. Local Vitest: 91/91 passing across the full MCP suite (8 test files); no regression in any pre-existing test.## Related— N/A.mcpClientsApi.connect/mcpClientsApi.disconnectround-tripped through the existing JSON-RPC.Closes #NNNin## Related— no specific issue; organic UX improvement on the MCP surface.Impact
McpServersTabis desktop-only viaChannelConfigPanel. No iOS / web impact.Promise.allSettled— strictly equivalent to a user manually triggering N connects/disconnects, just dispatched together. No new polling loop.role="dialog" aria-modal). The dialog body explicitly states secrets and configurations are preserved, addressing the "did I just lose my install state?" anxiety.nullwhenstatuses.length === 0, preserving the existing zero-installed flow byte-for-byte. All pre-existing MCP tests still pass unchanged.untranslatedtotal).aria-labels with interpolated counts on action buttons;role="dialog" aria-modal aria-labelledby aria-describedbyon the confirmation modal; decorativearia-hiddenstatus dots;role="alert"for operation errors.Related
mcp.health.*keys across the 12 non-English locales (currently fall back to English placeholders; flagged in each locale'suntranslatedcount).AI Authored PR Metadata
Linear Issue
Commit & Branch
feat/mcp-connection-health-toolbarValidation Run
All four key gates passed locally:
pnpm --filter openhuman-app compile— clean (tsc --noEmit, no output = success).pnpm --filter openhuman-app lint— clean for new files. The full repo currently produces 62 warnings (0 errors), of which zero are in the files this PR adds or modifies; the single warning atMcpServersTab.tsx:70:18is the pre-existingsetLoading(false)inside the initial-loaduseEffect, untouched by this PR.pnpm vitest run src/components/channels/mcp/— 91/91 passing including the 17 new tests and all pre-existing MCP component tests (no regression).pnpm i18n:check— exit 0, every locale at1:1206/1206parity, 0 missing keys, 0 extra keys, 0 drift.Validation Blocked
command:pnpm --filter openhuman-app format:check(chainscargo fmt --checkvia the workspace script) and the husky pre-push hook (which runspnpm format→cargo fmt).error:'cargo' is not recognized as an internal or external command, operable program or batch file.— no Rust toolchain installed on the dev machine.impact:Usedgit push --no-verifyper CLAUDE.md's allowance for unrelated pre-existing breakage. This PR touches zero Rust files, socargo fmt --checkwould have been a no-op for the changed files. Prettier on the changed TS files runs in CI on a clean Linux checkout with LF line endings; verified locally that the new files were written with LF.Behavior Changes
Parity Contract
statuses.length === 0, the toolbar returnsnull— the existing layout renders exactly as before. All pre-existing MCP tests pass unchanged. Per-server connect/disconnect via the existing detail view is untouched.Promise.allSettledensures partial failures within a bulk action don't abort the rest; the next poll tick reconciles any drift; thrown errors at the orchestration boundary surface throughrole="alert"rather than being swallowed.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Internationalization