From 7bd38f4624b5fc3d9c773886f10751d0c19dccd8 Mon Sep 17 00:00:00 2001 From: Aashir Athar Date: Mon, 25 May 2026 19:24:07 +0500 Subject: [PATCH 1/4] feat(mcp): connection health toolbar with bulk Retry All / Disconnect All actions --- .../mcp/McpConnectionHealthToolbar.test.tsx | 301 ++++++++++++++++++ .../mcp/McpConnectionHealthToolbar.tsx | 239 ++++++++++++++ .../components/channels/mcp/McpServersTab.tsx | 33 +- app/src/lib/i18n/chunks/ar-1.ts | 16 + app/src/lib/i18n/chunks/bn-1.ts | 16 + app/src/lib/i18n/chunks/de-1.ts | 16 + app/src/lib/i18n/chunks/en-1.ts | 16 + app/src/lib/i18n/chunks/es-1.ts | 16 + app/src/lib/i18n/chunks/fr-1.ts | 16 + app/src/lib/i18n/chunks/hi-1.ts | 16 + app/src/lib/i18n/chunks/id-1.ts | 16 + app/src/lib/i18n/chunks/it-1.ts | 16 + app/src/lib/i18n/chunks/ko-1.ts | 16 + app/src/lib/i18n/chunks/pt-1.ts | 16 + app/src/lib/i18n/chunks/ru-1.ts | 16 + app/src/lib/i18n/chunks/zh-CN-1.ts | 16 + app/src/lib/i18n/en.ts | 16 + 17 files changed, 796 insertions(+), 1 deletion(-) create mode 100644 app/src/components/channels/mcp/McpConnectionHealthToolbar.test.tsx create mode 100644 app/src/components/channels/mcp/McpConnectionHealthToolbar.tsx diff --git a/app/src/components/channels/mcp/McpConnectionHealthToolbar.test.tsx b/app/src/components/channels/mcp/McpConnectionHealthToolbar.test.tsx new file mode 100644 index 0000000000..755a777a96 --- /dev/null +++ b/app/src/components/channels/mcp/McpConnectionHealthToolbar.test.tsx @@ -0,0 +1,301 @@ +/** + * Tests for McpConnectionHealthToolbar — aggregate status counts + + * Retry All / Disconnect All bulk-action surface with confirmation + * dialog. + */ +import { act, fireEvent, render, screen } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; + +import McpConnectionHealthToolbar from './McpConnectionHealthToolbar'; +import type { ConnStatus, ServerStatus } from './types'; + +const statusFor = (server_id: string, status: ServerStatus): ConnStatus => ({ + server_id, + qualified_name: `acme/${server_id}`, + display_name: server_id, + status, + tool_count: status === 'connected' ? 3 : 0, +}); + +describe('McpConnectionHealthToolbar', () => { + it('renders nothing when statuses array is empty', () => { + const { container } = render( + {}} + onDisconnect={async () => {}} + /> + ); + expect(container.firstChild).toBeNull(); + }); + + it('always shows connected + disconnected counts (even when zero)', () => { + render( + {}} + onDisconnect={async () => {}} + /> + ); + expect(screen.getByText('0 connected')).toBeInTheDocument(); + expect(screen.getByText('1 idle')).toBeInTheDocument(); + }); + + it('only shows connecting count when there are connecting servers', () => { + const { rerender } = render( + {}} + onDisconnect={async () => {}} + /> + ); + expect(screen.queryByText(/connecting/)).not.toBeInTheDocument(); + rerender( + {}} + onDisconnect={async () => {}} + /> + ); + expect(screen.getByText('1 connecting')).toBeInTheDocument(); + }); + + it('only shows error count when there are errored servers', () => { + const { rerender } = render( + {}} + onDisconnect={async () => {}} + /> + ); + expect(screen.queryByText(/error/)).not.toBeInTheDocument(); + rerender( + {}} + onDisconnect={async () => {}} + /> + ); + expect(screen.getByText('1 error')).toBeInTheDocument(); + }); + + it('aggregates counts correctly across a mixed status set', () => { + render( + {}} + onDisconnect={async () => {}} + /> + ); + expect(screen.getByText('2 connected')).toBeInTheDocument(); + expect(screen.getByText('1 connecting')).toBeInTheDocument(); + expect(screen.getByText('1 error')).toBeInTheDocument(); + expect(screen.getByText('2 idle')).toBeInTheDocument(); + }); + + it('hides "Retry all" button when there are no errors', () => { + render( + {}} + onDisconnect={async () => {}} + /> + ); + expect(screen.queryByRole('button', { name: /Retry all/i })).not.toBeInTheDocument(); + }); + + it('hides "Disconnect all" button when nothing is connected', () => { + render( + {}} + onDisconnect={async () => {}} + /> + ); + expect(screen.queryByRole('button', { name: /Disconnect all/i })).not.toBeInTheDocument(); + }); + + it('shows "Retry all (N)" button with the correct error count when errors exist', () => { + render( + {}} + onDisconnect={async () => {}} + /> + ); + expect( + screen.getByRole('button', { name: 'Retry all 2 errored MCP servers' }) + ).toBeInTheDocument(); + expect(screen.getByText('Retry all (2)')).toBeInTheDocument(); + }); + + it('calls onReconnect with the errored server IDs when "Retry all" is clicked', async () => { + const onReconnect = vi.fn().mockResolvedValue(undefined); + render( + {}} + /> + ); + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: /Retry all/i })); + }); + expect(onReconnect).toHaveBeenCalledTimes(1); + expect(onReconnect).toHaveBeenCalledWith(['srv-1', 'srv-3']); + }); + + it('does NOT call onDisconnect directly — opens confirm dialog first', () => { + const onDisconnect = vi.fn(); + render( + {}} + onDisconnect={onDisconnect} + /> + ); + fireEvent.click(screen.getByRole('button', { name: /Disconnect all/i })); + expect(onDisconnect).not.toHaveBeenCalled(); + // Confirm dialog appears with accessible structure + const dialog = screen.getByRole('dialog'); + expect(dialog).toHaveAttribute('aria-modal', 'true'); + expect(screen.getByText('Disconnect all MCP servers?')).toBeInTheDocument(); + }); + + it('cancel in the dialog closes it without calling onDisconnect', () => { + const onDisconnect = vi.fn(); + render( + {}} + onDisconnect={onDisconnect} + /> + ); + fireEvent.click(screen.getByRole('button', { name: /Disconnect all \d+ connected MCP servers/i })); + fireEvent.click(screen.getByRole('button', { name: 'Cancel' })); + expect(onDisconnect).not.toHaveBeenCalled(); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + }); + + it('confirm in the dialog fires onDisconnect with connected IDs and closes the dialog', async () => { + const onDisconnect = vi.fn().mockResolvedValue(undefined); + render( + {}} + onDisconnect={onDisconnect} + /> + ); + fireEvent.click(screen.getByRole('button', { name: /Disconnect all \d+ connected MCP servers/i })); + // Confirm button inside dialog + const dialogConfirm = screen.getAllByRole('button', { name: 'Disconnect all' })[0]; + await act(async () => { + fireEvent.click(dialogConfirm); + }); + expect(onDisconnect).toHaveBeenCalledTimes(1); + expect(onDisconnect).toHaveBeenCalledWith(['srv-1', 'srv-3']); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + }); + + it('disables both action buttons while a bulk operation is pending', async () => { + let resolveOp: (() => void) | undefined; + const onReconnect = vi.fn( + () => + new Promise(res => { + resolveOp = res; + }) + ); + render( + {}} + /> + ); + fireEvent.click(screen.getByRole('button', { name: /Retry all/i })); + // While the promise is pending, both buttons should be disabled. + expect(screen.getByRole('button', { name: /Retry all/i })).toBeDisabled(); + expect(screen.getByRole('button', { name: /Disconnect all \d+ connected MCP servers/i })).toBeDisabled(); + // Resolve and re-render — buttons re-enable. + await act(async () => { + resolveOp?.(); + }); + expect(screen.getByRole('button', { name: /Retry all/i })).not.toBeDisabled(); + }); + + it('surfaces a thrown error from onReconnect via role="alert"', async () => { + const onReconnect = vi.fn().mockRejectedValue(new Error('upstream RPC died')); + render( + {}} + /> + ); + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: /Retry all/i })); + }); + const alert = screen.getByRole('alert'); + expect(alert).toHaveTextContent('upstream RPC died'); + }); + + it('falls back to a generic error message when the thrown value is not an Error instance', async () => { + const onReconnect = vi.fn().mockRejectedValue('not-an-error-object'); + render( + {}} + /> + ); + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: /Retry all/i })); + }); + expect(screen.getByRole('alert')).toHaveTextContent('Bulk operation failed. See logs.'); + }); + + it('the summary region is a polite live region with an accessible label', () => { + render( + {}} + onDisconnect={async () => {}} + /> + ); + const status = screen.getByRole('status', { name: 'MCP connection health summary' }); + expect(status).toHaveAttribute('aria-live', 'polite'); + }); + + it('confirm dialog body interpolates the connected count', () => { + render( + {}} + onDisconnect={async () => {}} + /> + ); + fireEvent.click(screen.getByRole('button', { name: /Disconnect all \d+ connected MCP servers/i })); + expect( + screen.getByText(/This will disconnect 3 currently-connected MCP servers/) + ).toBeInTheDocument(); + }); +}); diff --git a/app/src/components/channels/mcp/McpConnectionHealthToolbar.tsx b/app/src/components/channels/mcp/McpConnectionHealthToolbar.tsx new file mode 100644 index 0000000000..b8291f2238 --- /dev/null +++ b/app/src/components/channels/mcp/McpConnectionHealthToolbar.tsx @@ -0,0 +1,239 @@ +/** + * Aggregate health summary + bulk actions for the MCP server list. + * + * Lives in the left pane of `McpServersTab` above the list. Reads the + * polled `statuses` array (no extra fetches) and surfaces: + * + * - Live counts per state (connected / connecting / error / disconnected), + * announced through a `role="status" aria-live="polite"` region so + * screen readers hear updates as the polling loop refreshes. + * - `Retry all` button — visible only when there are servers in error + * state; iterates through them and calls `onReconnect` once. + * - `Disconnect all` button — visible only when there are connected + * servers; opens a confirm dialog (`role="dialog" aria-modal`) before + * firing `onDisconnect`. + * + * Parent (`McpServersTab`) owns the actual `mcpClientsApi` calls and the + * subsequent `fetchStatuses()` refresh; this component only orchestrates + * the user intent. + */ +import { useMemo, useState } from 'react'; + +import { useT } from '../../../lib/i18n/I18nContext'; +import type { ConnStatus } from './types'; + +interface McpConnectionHealthToolbarProps { + statuses: ConnStatus[]; + /** Reconnect every server in error state. Caller resolves after refresh. */ + onReconnect: (serverIds: string[]) => Promise; + /** Disconnect every currently-connected server. Caller resolves after refresh. */ + onDisconnect: (serverIds: string[]) => Promise; +} + +interface HealthCounts { + connectedIds: string[]; + errorIds: string[]; + connectedCount: number; + connectingCount: number; + errorCount: number; + disconnectedCount: number; +} + +const computeHealthCounts = (statuses: ConnStatus[]): HealthCounts => { + const connectedIds: string[] = []; + const errorIds: string[] = []; + let connectingCount = 0; + let disconnectedCount = 0; + for (const s of statuses) { + switch (s.status) { + case 'connected': + connectedIds.push(s.server_id); + break; + case 'error': + errorIds.push(s.server_id); + break; + case 'connecting': + connectingCount += 1; + break; + case 'disconnected': + disconnectedCount += 1; + break; + } + } + return { + connectedIds, + errorIds, + connectedCount: connectedIds.length, + connectingCount, + errorCount: errorIds.length, + disconnectedCount, + }; +}; + +const McpConnectionHealthToolbar = ({ + statuses, + onReconnect, + onDisconnect, +}: McpConnectionHealthToolbarProps) => { + const { t } = useT(); + const [isOperating, setIsOperating] = useState(false); + const [confirmDisconnect, setConfirmDisconnect] = useState(false); + const [opError, setOpError] = useState(null); + + const counts = useMemo(() => computeHealthCounts(statuses), [statuses]); + + // Nothing to summarise — match the parent's existing "hide chrome when + // there's nothing installed" pattern. + if (statuses.length === 0) return null; + + const runRetryAll = async () => { + if (counts.errorIds.length === 0 || isOperating) return; + setIsOperating(true); + setOpError(null); + try { + await onReconnect(counts.errorIds); + } catch (err) { + setOpError(err instanceof Error ? err.message : t('mcp.health.opErrorGeneric')); + } finally { + setIsOperating(false); + } + }; + + const runDisconnectAll = async () => { + if (counts.connectedIds.length === 0 || isOperating) return; + setConfirmDisconnect(false); + setIsOperating(true); + setOpError(null); + try { + await onDisconnect(counts.connectedIds); + } catch (err) { + setOpError(err instanceof Error ? err.message : t('mcp.health.opErrorGeneric')); + } finally { + setIsOperating(false); + } + }; + + return ( +
+
+ + {t('mcp.health.title')} + +
+ {counts.errorCount > 0 && ( + + )} + {counts.connectedCount > 0 && ( + + )} +
+
+
+ + + {counts.connectingCount > 0 && ( + + + )} + {counts.errorCount > 0 && ( + + + )} + + +
+ + {opError && ( +

+ {opError} +

+ )} + + {confirmDisconnect && ( +
+
+

+ {t('mcp.health.disconnectConfirm.title')} +

+

+ {t('mcp.health.disconnectConfirm.body').replace( + '{count}', + String(counts.connectedCount) + )} +

+
+ + +
+
+
+ )} +
+ ); +}; + +export default McpConnectionHealthToolbar; diff --git a/app/src/components/channels/mcp/McpServersTab.tsx b/app/src/components/channels/mcp/McpServersTab.tsx index 13e9c2868c..ba06c91757 100644 --- a/app/src/components/channels/mcp/McpServersTab.tsx +++ b/app/src/components/channels/mcp/McpServersTab.tsx @@ -13,6 +13,7 @@ import InstallDialog from './InstallDialog'; import InstalledServerDetail from './InstalledServerDetail'; import InstalledServerList from './InstalledServerList'; import McpCatalogBrowser from './McpCatalogBrowser'; +import McpConnectionHealthToolbar from './McpConnectionHealthToolbar'; import type { ConnStatus, InstalledServer } from './types'; const log = debug('mcp-clients:tab'); @@ -131,6 +132,31 @@ const McpServersTab = () => { [loadInstalled, fetchStatuses] ); + // Bulk Retry — iterate through errored servers, swallow per-server + // failures via `Promise.allSettled` so one bad apple doesn't abort the + // batch, then refresh statuses once at the end. The toolbar shows its + // own disabled state during the await; the next poll tick reconciles + // any drift. + const handleBulkReconnect = useCallback( + async (serverIds: string[]) => { + log('bulk reconnect ids=%o', serverIds); + await Promise.allSettled(serverIds.map(id => mcpClientsApi.connect(id))); + await fetchStatuses(); + }, + [fetchStatuses] + ); + + // Bulk Disconnect — same shape as bulk reconnect. The toolbar gates this + // behind a confirmation dialog before we get here. + const handleBulkDisconnect = useCallback( + async (serverIds: string[]) => { + log('bulk disconnect ids=%o', serverIds); + await Promise.allSettled(serverIds.map(id => mcpClientsApi.disconnect(id))); + await fetchStatuses(); + }, + [fetchStatuses] + ); + const selectedServerId = rightPane.mode === 'detail' ? rightPane.serverId : null; const selectedServer = servers.find(s => s.server_id === selectedServerId) ?? null; const selectedConnStatus = statuses.find(s => s.server_id === selectedServerId); @@ -154,13 +180,18 @@ const McpServersTab = () => { {t('mcp.alphaBannerText')}
- {/* Left pane: installed list */} + {/* Left pane: health toolbar + installed list */}
{loadError && (
{loadError}
)} + Date: Tue, 26 May 2026 05:06:33 +0500 Subject: [PATCH 2/4] style(mcp/health-toolbar): prettier --write on the two new toolbar files (fixes CI 'Type Check' format gate) --- .../mcp/McpConnectionHealthToolbar.test.tsx | 16 ++++++++++++---- .../channels/mcp/McpConnectionHealthToolbar.tsx | 4 +--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/app/src/components/channels/mcp/McpConnectionHealthToolbar.test.tsx b/app/src/components/channels/mcp/McpConnectionHealthToolbar.test.tsx index 755a777a96..6087f83376 100644 --- a/app/src/components/channels/mcp/McpConnectionHealthToolbar.test.tsx +++ b/app/src/components/channels/mcp/McpConnectionHealthToolbar.test.tsx @@ -182,7 +182,9 @@ describe('McpConnectionHealthToolbar', () => { onDisconnect={onDisconnect} /> ); - fireEvent.click(screen.getByRole('button', { name: /Disconnect all \d+ connected MCP servers/i })); + fireEvent.click( + screen.getByRole('button', { name: /Disconnect all \d+ connected MCP servers/i }) + ); fireEvent.click(screen.getByRole('button', { name: 'Cancel' })); expect(onDisconnect).not.toHaveBeenCalled(); expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); @@ -201,7 +203,9 @@ describe('McpConnectionHealthToolbar', () => { onDisconnect={onDisconnect} /> ); - fireEvent.click(screen.getByRole('button', { name: /Disconnect all \d+ connected MCP servers/i })); + fireEvent.click( + screen.getByRole('button', { name: /Disconnect all \d+ connected MCP servers/i }) + ); // Confirm button inside dialog const dialogConfirm = screen.getAllByRole('button', { name: 'Disconnect all' })[0]; await act(async () => { @@ -230,7 +234,9 @@ describe('McpConnectionHealthToolbar', () => { fireEvent.click(screen.getByRole('button', { name: /Retry all/i })); // While the promise is pending, both buttons should be disabled. expect(screen.getByRole('button', { name: /Retry all/i })).toBeDisabled(); - expect(screen.getByRole('button', { name: /Disconnect all \d+ connected MCP servers/i })).toBeDisabled(); + expect( + screen.getByRole('button', { name: /Disconnect all \d+ connected MCP servers/i }) + ).toBeDisabled(); // Resolve and re-render — buttons re-enable. await act(async () => { resolveOp?.(); @@ -293,7 +299,9 @@ describe('McpConnectionHealthToolbar', () => { onDisconnect={async () => {}} /> ); - fireEvent.click(screen.getByRole('button', { name: /Disconnect all \d+ connected MCP servers/i })); + fireEvent.click( + screen.getByRole('button', { name: /Disconnect all \d+ connected MCP servers/i }) + ); expect( screen.getByText(/This will disconnect 3 currently-connected MCP servers/) ).toBeInTheDocument(); diff --git a/app/src/components/channels/mcp/McpConnectionHealthToolbar.tsx b/app/src/components/channels/mcp/McpConnectionHealthToolbar.tsx index b8291f2238..304a46b69f 100644 --- a/app/src/components/channels/mcp/McpConnectionHealthToolbar.tsx +++ b/app/src/components/channels/mcp/McpConnectionHealthToolbar.tsx @@ -170,9 +170,7 @@ const McpConnectionHealthToolbar = ({ {counts.errorCount > 0 && ( )} From 017380885028dd6cc041a98724dec067aecb8623 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Thu, 28 May 2026 20:28:23 -0700 Subject: [PATCH 3/4] Remove outdated documentation files: deleted `openhuman-contribution-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. --- PR-PLAYBOOK.md | 355 --------------------------------- PR2-body.md | 83 -------- PR3-mcp-status-badge.patch | 107 ---------- PR5-body.md | 138 ------------- PR6-body.md | 143 ------------- PR7-body.md | 160 --------------- PR8-body.md | 138 ------------- openhuman-contribution-plan.md | 283 -------------------------- 8 files changed, 1407 deletions(-) delete mode 100644 PR-PLAYBOOK.md delete mode 100644 PR2-body.md delete mode 100644 PR3-mcp-status-badge.patch delete mode 100644 PR5-body.md delete mode 100644 PR6-body.md delete mode 100644 PR7-body.md delete mode 100644 PR8-body.md delete mode 100644 openhuman-contribution-plan.md diff --git a/PR-PLAYBOOK.md b/PR-PLAYBOOK.md deleted file mode 100644 index 15c5644c18..0000000000 --- a/PR-PLAYBOOK.md +++ /dev/null @@ -1,355 +0,0 @@ -# OpenHuman PR Playbook - -Reference for all 4 PRs against [`tinyhumansai/openhuman`](https://github.com/tinyhumansai/openhuman) from your fork [`aashir-athar/openhuman`](https://github.com/aashir-athar/openhuman). - -**Working directory:** `D:/openhuman/` (single consolidated worktree). -**Remotes:** `origin → aashir-athar/openhuman.git`, `upstream → tinyhumansai/openhuman` (push disabled). - ---- - -## Universal rules (apply to every push) - -1. **Always use explicit file lists in `git add`** — never `-A` or `.`. Three files in `D:/openhuman/` must stay out of every commit: - - `PR3-mcp-status-badge.patch` - - `openhuman-contribution-plan.md` - - `PR-PLAYBOOK.md` (this file) -2. **If the pre-push hook fails on `cargo fmt` not found**, add `--no-verify` to the push. CLAUDE.md authorizes `--no-verify` for unrelated pre-existing breakage. CI on Linux runs the real cargo fmt check. -3. **For PR creation**: if `gh` auth fails (TLS timeout etc.), use the URL GitHub prints after `git push`: - `https://github.com/aashir-athar/openhuman/pull/new/` - That opens a pre-filled form pointing at `tinyhumansai/openhuman:main` — paste title + description there, add labels in sidebar. - ---- - -## PR 1 — Docs truth-up - -**Branch:** `docs/truth-up-and-architecture-pages` -**Worktree state:** 6 modified + 3 new files in `D:/openhuman/`. -**Status:** Ready to commit + push. -**Label on PR:** `docs` - -### Title - -``` -docs: truth-up architecture.md + 3 new domain pages + Linux/Arch caveats in localized READMEs -``` - -### Description - -```markdown -## Summary - -- Purge 9 stale QuickJS / `rquickjs` skill-runtime references throughout `gitbooks/developing/architecture.md` — the runtime was removed and `src/openhuman/skills/` is metadata-only per CLAUDE.md, but multiple diagrams, tables, and prose sections still described it as active. -- Fix two related factual bugs in the same file: "Yarn workspace" → "pnpm workspace" (root `package.json` declares `pnpm@10.10.0`); rewrite the stale top-level `skills/` row to point at `src/openhuman/skills/` and describe its actual post-QuickJS-removal state. -- Add three contributor-audience architecture pages under `gitbooks/developing/architecture/` (`memory-tree.md`, `mcp-registry.md`, `security.md`) for currently-undocumented active domains. Linked from `gitbooks/SUMMARY.md`. -- Backfill the Linux Wayland warning + Arch AUR pointer block (present in English `README.md` since #2463) into `README.zh-CN.md`, `README.ja-JP.md`, `README.ko.md`, `README.de.md`. English content under `` markers — native-speaker translation follow-up invited. - -## Problem - -The architecture book diverged from the code on three load-bearing points: - -1. **Stale runtime model.** CLAUDE.md is explicit: *"Skills runtime removed: the QuickJS / `rquickjs` runtime that previously executed skill packages is gone."* Yet `architecture.md` still described a QuickJS runtime engine, per-skill 64 MB sandbox limits, "QuickJS Skill Instance executes tool" in the MCP flow, and the same in the end-to-end data flow. -2. **Stale build-tool reference** ("Yarn workspace") inconsistent with `pnpm@10.10.0` in `package.json`. -3. **Stale path reference** to a top-level `skills/` directory that no longer exists. - -Three active, substantial domains (`memory_tree`, `mcp_registry`, `security`) had no gitbook architecture page despite recent activity (#2556, #2559) and despite each having a rich internal-audience `README.md` / `mod.rs` rustdoc invisible to the gitbook. - -The Linux Wayland / AUR block was only added to the English README; localized README readers running install on Arch + Wayland hit an unexplained crash with no caveat in their language. - -## Solution - -**`gitbooks/developing/architecture.md`** — purged QuickJS references from the high-level diagram, performance table, MCP flow diagram, security architecture diagram + bullet, and end-to-end data flow. Where a replacement was needed (e.g. "Sandboxed QuickJS per skill (64 MB)"), used what the code actually does: tool execution gated by `SecurityPolicy` + a host-selected sandbox backend from `src/openhuman/security/`. Three intentional "QuickJS was removed" historical references retained — clearly marked as such — to make the migration explicit. - -**Three new architecture pages** — followed the existing `agent-harness.md` / `frontend.md` / `tauri-shell.md` convention: YAML frontmatter, H1 with source path in backticks, ASCII diagram, layout table sourced from the existing `README.md` / `mod.rs`, "Calls into" / "Called by" / "Tests" / "Related" sections. Describe only what the code currently does — no aspirational claims. Tables prettier-aligned to match existing style. - -**Localized READMEs** — inserted the Linux/Arch block right after the install bash code-block (matching the structural position in `README.md`), wrapped in `` / `` markers so native speakers know exactly what needs translation in a follow-up PR. - -## Submission Checklist - -- [x] Tests added or updated — **N/A: docs-only change, no behavioural code touched.** -- [x] **Diff coverage ≥ 80%** — **N/A: no changed lines appear in any lcov report.** `diff-cover` returns 100% by definition. -- [x] Coverage matrix updated — **N/A: behaviour-only change** (no behaviour change; pure documentation truth-up). -- [x] All affected feature IDs listed under `## Related` — **N/A: no feature behaviour changed.** -- [x] No new external network dependencies introduced — **N/A: docs only.** -- [x] Manual smoke checklist updated if release-cut surfaces touched — **N/A: no release-cut surface touched.** -- [x] Linked issue closed via `Closes #NNN` in `## Related` — **N/A: no linked issue.** - -## Impact - -- Runtime/platform impact: **none.** Docs only. -- Performance/security/migration/compatibility: **none.** -- New contributors reading `architecture.md` now get an accurate mental model of the post-QuickJS skill-metadata + native-tool-runtime split, and three of the most active Rust domains finally have contributor-audience overviews. -- Localized README readers see the same install caveat their English counterparts do. - -## Related - -- Closes: -- Follow-up PR(s)/TODOs: - - Native-speaker translation of the four `` blocks in the localized READMEs. - ---- - -## AI Authored PR Metadata - -### Linear Issue -- Key: N/A -- URL: N/A - -### Commit & Branch -- Branch: `docs/truth-up-and-architecture-pages` -- Commit SHA: (filled by GitHub after push) - -### Validation Run -- [x] `pnpm --filter openhuman-app format:check` — fails on ~1044 pre-existing files unrelated to this PR (Windows CRLF vs `endOfLine: "lf"`); zero of my files are in the failure list; the 3 new architecture pages explicitly pass prettier -- [x] `pnpm typecheck` — N/A (docs only, no TypeScript touched) -- [x] Focused tests: N/A (docs only) -- [x] Rust fmt/check (if changed): N/A (no Rust touched) -- [x] Tauri fmt/check (if changed): N/A (no Tauri touched) - -### Validation Blocked -- `command:` N/A -- `error:` N/A -- `impact:` N/A - -### Behavior Changes -- Intended behavior change: None — documentation only -- User-visible effect: Localized README readers now see the same Linux/Arch install caveats as English readers; contributors get accurate runtime architecture and three new domain-level architecture pages - -### Parity Contract -- Legacy behavior preserved: Yes — no code changed; only documentation corrected to match current code -- Guard/fallback/dispatch parity checks: N/A - -### Duplicate / Superseded PR Handling -- Duplicate PR(s): None known -- Canonical PR: This one -- Resolution: N/A - ---- - -> Local pre-push hook bypassed with `--no-verify` because `cargo fmt` failed on a machine without a Rust toolchain installed. This PR is docs-only (zero Rust changes), so CI's `cargo fmt --check` on the Linux runner is the authoritative gate. -``` - -### Commands (PowerShell) - -```powershell -cd D:\openhuman - -# Stage (explicit file list — never -A or .) -git add README.de.md README.ja-JP.md README.ko.md README.zh-CN.md ` - gitbooks/SUMMARY.md ` - gitbooks/developing/architecture.md ` - gitbooks/developing/architecture/memory-tree.md ` - gitbooks/developing/architecture/mcp-registry.md ` - gitbooks/developing/architecture/security.md - -# Commit (single-line subject; full body lives in the PR description) -git commit -m "docs: truth-up architecture.md (purge QuickJS refs) + add 3 domain pages + backfill Linux/Arch caveats to localized READMEs" - -# Push (use --no-verify if pre-push hook fails on missing cargo) -git push --no-verify -u origin docs/truth-up-and-architecture-pages - -# Then open this URL in browser, paste the title + description above, add "docs" label: -# https://github.com/aashir-athar/openhuman/pull/new/docs/truth-up-and-architecture-pages -``` - ---- - -## PR 2 — Backend stub closure (NO COMMANDS — all stale) - -**Branch:** *(none — `feat/close-backend-stubs` was deleted after verification)* -**Status:** All 3 audit deliverables verified STALE against current code; cannot ship as specified. - -### Verification verdict - -| Deliverable | Verdict | -|---|---| -| **2.1** Wire FTS5 insert in `insert_sql_record.rs:137` | STALE — file is intentionally a Phase 5 stub with 8 existing tests; doing it honestly requires designing schema + migration + DI plumbing (multi-day feature, not 1-day stub wiring) | -| **2.2** Add `webview_notifications/rpc.rs` | STALE — domain is intentionally Tauri-IPC only per its own `schemas.rs` comment: *"v1 has no user-facing controllers: the on/off toggle lives in the Tauri shell"* | -| **2.3** Add unit tests for `src/core/dispatch.rs` | STALE — file already has 12 inline tests covering every case the audit listed (valid routing, unknown method, empty method, tier-2 domain dispatch, legacy alias resolution, etc.) | - -### To revisit - -PR 2 needs a fresh audit pass to identify alternative real backend gaps (in the same spirit as PR 1.2 which was replaced when the audit was found stale). When ready, start a fresh session and ask me to "find real 1–3 day backend gaps" with no prescribed deliverables. - ---- - -## PR 3 — McpStatusBadge i18n + a11y (reduced scope) - -**Branch:** `feat/mcp-servers-ui-panel` (empty branch in git refs, off `upstream/main`) -**Worktree state:** Changes saved as `D:/openhuman/PR3-mcp-status-badge.patch` (107-line diff). -**Status:** Apply patch after PR 1 ships, then commit + push. -**Label on PR:** none required (PR 3 is a feature/a11y change, default labels are fine) - -### Why only McpStatusBadge - -6 of 8 original PR 3 deliverables were verified STALE. The full MCP UI surface already exists at `app/src/components/channels/mcp/` (8 components, all with tests, RPC client `mcpClientsApi`, ships via `pages/Channels.tsx` → `ChannelConfigPanel` → `McpServersTab`). The `Skills.tsx` `` is intentionally pinned by `Skills.mcp-coming-soon.test.tsx`. Only the McpStatusBadge i18n + a11y gap was real. - -### Title - -``` -feat(mcp): i18n + a11y McpStatusBadge status labels -``` - -### Description - -```markdown -## Summary - -- Route the 4 hardcoded status labels in `McpStatusBadge.tsx` ('Connected' / 'Connecting' / 'Disconnected' / 'Error') through `useT()` using the existing `channels.status.*` keys. The badge's own docstring says it *"Mirrors ChannelStatusBadge"*; the labels are identical English; reusing the shared key set avoids redundant translation work. -- Add `role="status"` and `aria-live="polite"` so screen readers announce state changes — matches the alpha-banner pattern already used in `McpServersTab`. -- Add a 7-test `McpStatusBadge.test.tsx` covering each `ServerStatus` variant, the a11y attributes, the disconnected fallback for unknown statuses, and className passthrough. - -## Problem - -`McpStatusBadge.tsx` slipped past the #2577 React i18n sweep because it's an isolated leaf component without a co-located test. Lines 7–25 hardcoded English labels (`label: 'Connected'`, etc.) directly in a `STATUS_STYLES` object, violating the CLAUDE.md rule that *every user-visible string in `app/src/**` must go through `useT()`*. Non-EN users saw English labels on every MCP server connection state. - -The same `` had no `role` or `aria-live`, so screen readers wouldn't announce state changes — important for a long-running connection that can transition between `connecting` → `connected` → `error` while the user is on a different part of the page. - -## Solution - -- Reuse the existing `channels.status.{connected,connecting,disconnected,error}` i18n keys rather than introducing `mcp.status.*` duplicates. The labels are character-identical to the channel status set; the badge's docstring already calls it a "mirror" of `ChannelStatusBadge`. If the MCP vocabulary ever diverges (e.g. adds "spawning" / "handshaking"), the keys can be split then. -- Add `role="status"` + `aria-live="polite"` (matches `McpServersTab`'s alpha banner). -- Co-locate `McpStatusBadge.test.tsx` covering: every status variant renders the right label, a11y attributes present, fallback to "Disconnected" for unknown status values, className passthrough preserves built-in classes. - -## Submission Checklist - -- [x] Tests added or updated — 7 tests added in new `McpStatusBadge.test.tsx`. -- [x] **Diff coverage ≥ 80%** — All changed lines in `McpStatusBadge.tsx` are exercised by the new test file (`it.each` over all 4 status variants + a11y + fallback + className). -- [x] Coverage matrix updated — **N/A: behaviour-only change** (no new feature; same component, just i18n + a11y). -- [x] All affected feature IDs listed under `## Related` — **N/A: leaf-component fix.** -- [x] No new external network dependencies introduced — N/A. -- [x] Manual smoke checklist updated if release-cut surfaces touched — **N/A: no release-cut surface touched.** -- [x] Linked issue closed via `Closes #NNN` in `## Related` — **N/A: no linked issue.** - -## Impact - -- Runtime/platform impact: **none** (drop-in component change). -- Performance/security/migration/compatibility: none. -- Localized users will now see translated status labels in their locale; screen reader users will hear connection state changes announced. - -## Related - -- Closes: -- Follow-up PR(s)/TODOs: none. - ---- - -## AI Authored PR Metadata - -### Linear Issue -- Key: N/A -- URL: N/A - -### Commit & Branch -- Branch: `feat/mcp-servers-ui-panel` -- Commit SHA: (filled by GitHub after push) - -### Validation Run -- [x] `pnpm --filter openhuman-app format:check` — `McpStatusBadge.test.tsx` clean; `McpStatusBadge.tsx` inherits CRLF from Windows checkout (CI on Linux passes) -- [x] `pnpm typecheck` — clean -- [x] Focused tests: `pnpm vitest run src/components/channels/mcp/` — 82/82 pass (includes the new file + the pinned `Skills.mcp-coming-soon.test.tsx` confirming no regression) -- [x] Rust fmt/check (if changed): N/A (no Rust touched) -- [x] Tauri fmt/check (if changed): N/A (no Tauri touched) - -### Validation Blocked -- `command:` N/A -- `error:` N/A -- `impact:` N/A - -### Behavior Changes -- Intended behavior change: Localized status labels + screen-reader announcement of state changes -- User-visible effect: Non-EN users see translated labels; screen readers announce connection state transitions - -### Parity Contract -- Legacy behavior preserved: Yes — labels render identically in English; the fallback for unknown statuses still resolves to "Disconnected" style + label -- Guard/fallback/dispatch parity checks: Test covers unknown status fallback to "Disconnected" label + style - -### Duplicate / Superseded PR Handling -- Duplicate PR(s): None known -- Canonical PR: This one -- Resolution: N/A -``` - -### Commands (PowerShell, run after PR 1 is pushed) - -```powershell -cd D:\openhuman - -# Switch to the empty PR 3 branch (already off upstream/main) -git switch feat/mcp-servers-ui-panel - -# Apply the saved patch -git apply PR3-mcp-status-badge.patch - -# Verify the expected diff (1 modified + 1 untracked .test.tsx) -git status - -# Stage explicitly -git add app/src/components/channels/mcp/McpStatusBadge.tsx ` - app/src/components/channels/mcp/McpStatusBadge.test.tsx - -# Commit -git commit -m "feat(mcp): i18n + a11y McpStatusBadge status labels" - -# Push (use --no-verify if pre-push hook fails on missing cargo) -git push --no-verify -u origin feat/mcp-servers-ui-panel - -# Open this URL and paste the title + description above: -# https://github.com/aashir-athar/openhuman/pull/new/feat/mcp-servers-ui-panel - -# After PR 3 is opened, the patch is no longer needed: -Remove-Item PR3-mcp-status-badge.patch -``` - ---- - -## PR 4 — LSP tool backend (deferred) - -**Branch:** `feat/lsp-tool-backend` (empty branch in git refs, off `upstream/main`) -**Status:** Verified REAL (audit accurate); deferred to a fresh session per the master prompt's *"ONE PR per session"* rule and *"if rabbit hole, surface limitation"* clause. - -### Scope (when you resume) - -Per master prompt PR 4 spec: -- New `src/openhuman/lsp/` domain (`client.rs`, `pool.rs`, `discovery.rs`, `types.rs`, `schemas.rs`, `rpc.rs`) -- LSP JSON-RPC over stdio (Content-Length framing, request/response correlation) -- Cross-platform server discovery for at minimum `rust-analyzer` -- Wire into existing `tools/impl/system/lsp.rs` (which is the stable capability-gated stub) -- Tests with mock LSP server -- Keep behind `OPENHUMAN_LSP_ENABLED=1` env gate -- Update `src/openhuman/about_app/` - -### Reusable infrastructure already in the codebase - -- `src/openhuman/mcp_client/stdio.rs` — tokio Command + ChildStdin/ChildStdout pattern (LSP uses the same wire framing, different methods) -- Controller pattern (`all_controller_schemas`, `all_registered_controllers`, `handle_*`) from any existing domain -- Wire into `src/core/all.rs` like every other domain - -### Rabbit holes to surface, not invent solutions for - -- Cross-platform server discovery (Windows / Linux / macOS / asdf / mise / path-relative) -- Auto-install vs error-if-missing -- Multi-root workspace detection - -### How to resume - -```powershell -cd D:\openhuman -git fetch upstream -git switch feat/lsp-tool-backend -git rebase upstream/main # bring branch up to date if upstream has moved -``` - -Then start a fresh Claude session with the master prompt's `CURRENT TASK` line set to `PR 4`. - ---- - -## Summary - -| PR | Branch | Status | Action | -|---|---|---|---| -| 1 | `docs/truth-up-and-architecture-pages` | Ready in worktree | Run PR 1 commands above | -| 2 | *(none)* | All audit deliverables STALE | Skip; revisit with fresh audit later if desired | -| 3 | `feat/mcp-servers-ui-panel` | Patch saved | Run PR 3 commands after PR 1 ships | -| 4 | `feat/lsp-tool-backend` | Deferred | Resume in fresh session | - -**Branches preserved in git refs:** `docs/truth-up-and-architecture-pages`, `feat/mcp-servers-ui-panel`, `feat/lsp-tool-backend`, `main`. The deleted `feat/close-backend-stubs` was PR 2's empty branch — no work to preserve. diff --git a/PR2-body.md b/PR2-body.md deleted file mode 100644 index bc463e940b..0000000000 --- a/PR2-body.md +++ /dev/null @@ -1,83 +0,0 @@ -## Summary - -- `src/openhuman/cwd_jail/windows.rs` hard-coded `SECURITY_CAPABILITIES.Capabilities` to `null` and only logged a warning when `jail.allow_net == true`. The child AppContainer process got **no network access** regardless of the flag. -- The module's own docstring already promised the documented behaviour (*"we honor `jail.allow_net` by adding `internetClient` and `privateNetworkClientServer` capabilities"*) — this PR makes the implementation match. -- Wires `DeriveCapabilitySidsFromName` for two well-known manifest capabilities, builds the `Vec`, attaches it to `SECURITY_CAPABILITIES` before `CreateProcessW`. OS-allocated SIDs are owned by a `CapabilityDerivation` wrapper that `LocalFree`s each SID + array on Drop per MSDN. -- Adds 4 Windows-only unit tests covering the capability-name set, the FFI happy path against `internetClient`, and incidental coverage of `sanitize_profile_name`. - -## Problem - -The `cwd_jail` module is the unified facade for OS-specific tool-execution jails (Landlock on Linux, Seatbelt on macOS, **AppContainer on Windows**). The Windows backend's docstring says network capabilities are honoured when `jail.allow_net == true`, but the actual code at L137–148 only emitted a `log::warn!` and then constructed `SECURITY_CAPABILITIES` with `Capabilities: null_mut(), CapabilityCount: 0`. Result: every Windows-jailed tool ran with no network, even when explicitly opted in. The existing `_unused()` sentinel function at the bottom hinted the original author *knew* `SID_AND_ATTRIBUTES` would be needed later — this PR is that "later". - -## Solution - -**Capability derivation.** Added `DeriveCapabilitySidsFromName` to the existing `Win32::Security::Isolation` import (feature already enabled in `Cargo.toml`). A new `derive_capability(name)` helper calls the Win32 API and returns a `CapabilityDerivation` wrapper that owns both the per-capability SID array and the per-SID `LocalAlloc` backings, releasing them in Drop in the correct order per MSDN. - -**Spawn integration.** Replaced the warn-and-skip block: when `jail.allow_net == true`, derive SIDs for each of `NET_CAPABILITY_NAMES`, build a `Vec` with `SE_GROUP_ENABLED`, and point `SECURITY_CAPABILITIES.Capabilities` at it. Per-capability failures log a warning and the others still go through; total failure with `allow_net=true` logs an error so the privilege regression is loud. - -**Coarse-switch scope.** `NET_CAPABILITY_NAMES = ["internetClient", "privateNetworkClientServer"]` — outbound public internet + LAN access incl. inbound `bind()`. **Intentionally excludes** `internetClientServer` (server-side public internet) because `allow_net` is a coarse switch; callers needing a richer surface should add a real policy struct. - -**Lifetime safety.** Both `cap_attrs` and `_cap_derivations` are declared before `caps` so they outlive `CreateProcessW`. After `CreateProcessW` returns synchronously the OS has captured the `SECURITY_CAPABILITIES` into the child PEB; we can then drop our copies without affecting the child. - -**Cleanup.** Removed the now-unnecessary `_unused()` sentinel — `SID_AND_ATTRIBUTES` is now genuinely used. - -## Submission Checklist - -- [x] Tests added or updated — 4 new tests in `#[cfg(test)] mod tests` (Windows-targeted; the file is `#![cfg(target_os = "windows")]`). -- [x] **Diff coverage ≥ 80%** — `derive_capability` and the new constant are directly exercised by `derive_capability_resolves_well_known_internet_client` and `net_capability_names_covers_basic_internet_and_lan`. The integration branch inside `spawn_in_container` is reachable only via real AppContainer spawn, which depends on the separate Child-wrapper TODO; flagged out-of-scope below. -- [x] Coverage matrix updated — **N/A: internal correctness fix, no user-facing feature row.** -- [x] All affected feature IDs listed under `## Related` — **N/A: no feature IDs.** -- [x] No new external network dependencies introduced — no new crates; the runtime *user-process* gets network when `allow_net=true` is set, which is the documented behaviour. -- [x] Manual smoke checklist updated if release-cut surfaces touched — **N/A: no release-cut surface.** -- [x] Linked issue closed via `Closes #NNN` in `## Related` — no linked issue found; happy to link one if there's a tracking issue. - -## Impact - -- **Platform**: Windows-only behaviour change (file is `#![cfg(target_os = "windows")]`). -- **Backward compatibility**: `jail.allow_net == false` (default) is unchanged — `cap_attrs` stays empty, `SECURITY_CAPABILITIES` is null/0 exactly as before. -- **Forward-facing**: Capabilities are now correctly attached to `CreateProcessW`. The AppContainer spawn still returns `Unsupported` at the end due to the separate `std::process::Child` wrapper TODO (the *parent* problem is that `Child` has no stable `FromRawHandle` constructor). When that TODO is resolved, Windows jails with `allow_net=true` will immediately benefit from this work — no second change needed. -- **Security**: Strict positive — `allow_net` now actually means what it says; loud `log::error!` if all capabilities fail to derive (the privilege regression that was previously silent). - -## Related - -- Closes: -- Follow-up PR(s)/TODOs: - - Custom `OpenhumanChild` wrapper so AppContainer spawn can actually return a usable handle on Windows-stable (the TODO at the end of `spawn_in_container`). With this PR landed, that follow-up only has to solve handle wrapping — capabilities are already correct. - - Optional: an `is_capability_supported(name)` probe so a future audit can verify the AppContainer is actually receiving network rights via `ProcessSecurityCapabilities` introspection. - ---- - -## AI Authored PR Metadata - -### Linear Issue -- Key: N/A -- URL: N/A - -### Commit & Branch -- Branch: `fix/windows-cwd-jail-correctness` -- Commit SHA: f7c9e5f3 - -### Validation Run -- [ ] `pnpm --filter openhuman-app format:check` — **VALIDATION BLOCKED**: no Rust toolchain on the contributor's dev machine; the pre-push hook's `cargo fmt --check` cannot run locally. Used `git push --no-verify` per CLAUDE.md's allowance for unrelated pre-existing breakage; CI on the Windows + Ubuntu runners is the authoritative gate. -- [ ] `pnpm typecheck` — **N/A**: Rust-only change. -- [ ] Focused tests — **VALIDATION BLOCKED** (same reason); 4 new `#[cfg(test)]` tests are in the file ready to run under `cargo test -p openhuman --lib` on a Windows host. -- [ ] Rust fmt/check — **VALIDATION BLOCKED** (same reason). File was hand-formatted to match existing 4-space-indent / line-width conventions in this module; happy to revise if `cargo fmt --check` flags anything. -- [x] Tauri fmt/check (if changed) — N/A (no Tauri touched). - -### Validation Blocked -- `command:` `pnpm rust:format` (and by extension the pre-push hook), `cargo check`, `cargo test`. -- `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:` Used `git push --no-verify`. Cannot self-verify compilation, formatting, or test pass locally. Code was manually reviewed against MSDN for FFI correctness and against the existing module patterns. CI is the gate. - -### Behavior Changes -- Intended behavior change: `jail.allow_net = true` now actually grants network capabilities to AppContainer-jailed children on Windows. -- User-visible effect: None today, because the surrounding `spawn_in_container` still returns `Unsupported` due to a separate `std::process::Child` wrapper TODO. When that follow-up lands, this fix is what makes the resulting jailed children actually able to reach the network when permitted. - -### Parity Contract -- Legacy behavior preserved: `allow_net = false` (default) path is byte-identical to before — `cap_attrs` is empty, `SECURITY_CAPABILITIES.Capabilities` stays null, `CapabilityCount` stays 0. -- Guard/fallback/dispatch parity checks: New error path is loud (`log::error!`) when `allow_net = true` but all capabilities fail to derive; this is a strictly louder failure mode than the previous silent no-net. - -### Duplicate / Superseded PR Handling -- Duplicate PR(s): None known. -- Canonical PR: This one. -- Resolution: N/A. diff --git a/PR3-mcp-status-badge.patch b/PR3-mcp-status-badge.patch deleted file mode 100644 index 3841556aba..0000000000 --- a/PR3-mcp-status-badge.patch +++ /dev/null @@ -1,107 +0,0 @@ -diff --git a/app/src/components/channels/mcp/McpStatusBadge.test.tsx b/app/src/components/channels/mcp/McpStatusBadge.test.tsx -new file mode 100644 -index 00000000..2c3a840a ---- /dev/null -+++ b/app/src/components/channels/mcp/McpStatusBadge.test.tsx -@@ -0,0 +1,44 @@ -+/** -+ * Tests for McpStatusBadge — renders the i18n'd label and a11y role -+ * for each ServerStatus, and forwards custom className. -+ */ -+import { render, screen } from '@testing-library/react'; -+import { describe, expect, it } from 'vitest'; -+ -+import McpStatusBadge from './McpStatusBadge'; -+import type { ServerStatus } from './types'; -+ -+describe('McpStatusBadge', () => { -+ it.each<[ServerStatus, string]>([ -+ ['connected', 'Connected'], -+ ['connecting', 'Connecting'], -+ ['disconnected', 'Disconnected'], -+ ['error', 'Error'], -+ ])('renders i18n label for status=%s', (status, expectedLabel) => { -+ render(); -+ expect(screen.getByRole('status')).toHaveTextContent(expectedLabel); -+ }); -+ -+ it('exposes role="status" and aria-live="polite" for assistive tech', () => { -+ render(); -+ const badge = screen.getByRole('status'); -+ expect(badge).toHaveAttribute('aria-live', 'polite'); -+ }); -+ -+ it('falls back to the disconnected style for an unknown status value', () => { -+ // ServerStatus is a closed union, but the runtime fallback exists for -+ // forward-compat with possible future Rust-side variants — exercise it. -+ render(); -+ expect(screen.getByRole('status')).toHaveTextContent('Disconnected'); -+ }); -+ -+ it('appends the optional className without dropping the built-in classes', () => { -+ render(); -+ const badge = screen.getByRole('status'); -+ expect(badge.className).toContain('my-custom-class'); -+ expect(badge.className).toContain('ml-2'); -+ // Built-in look-and-feel preserved. -+ expect(badge.className).toContain('rounded-full'); -+ expect(badge.className).toContain('bg-sage-500/10'); -+ }); -+}); -diff --git a/app/src/components/channels/mcp/McpStatusBadge.tsx b/app/src/components/channels/mcp/McpStatusBadge.tsx -index 586fb8fc..009c54c9 100644 ---- a/app/src/components/channels/mcp/McpStatusBadge.tsx -+++ b/app/src/components/channels/mcp/McpStatusBadge.tsx -@@ -1,25 +1,28 @@ - /** - * Status badge for MCP server connection states. -- * Mirrors ChannelStatusBadge but uses ServerStatus values. -+ * Mirrors ChannelStatusBadge but uses ServerStatus values; reuses the -+ * shared `channels.status.*` i18n keys since the label vocabulary is -+ * identical (Connected / Connecting / Disconnected / Error). - */ -+import { useT } from '../../../lib/i18n/I18nContext'; - import type { ServerStatus } from './types'; - --const STATUS_STYLES: Record = { -+const STATUS_META: Record = { - connected: { -- label: 'Connected', -+ i18nKey: 'channels.status.connected', - className: 'bg-sage-500/10 text-sage-700 border-sage-500/30 dark:text-sage-300', - }, - connecting: { -- label: 'Connecting', -+ i18nKey: 'channels.status.connecting', - className: 'bg-amber-500/10 text-amber-700 border-amber-500/30 dark:text-amber-300', - }, - disconnected: { -- label: 'Disconnected', -+ i18nKey: 'channels.status.disconnected', - className: - 'bg-stone-100 dark:bg-neutral-800 text-stone-500 dark:text-neutral-400 border-stone-200 dark:border-neutral-700', - }, - error: { -- label: 'Error', -+ i18nKey: 'channels.status.error', - className: 'bg-coral-500/10 text-coral-700 border-coral-500/30 dark:text-coral-300', - }, - }; -@@ -30,11 +33,14 @@ interface McpStatusBadgeProps { - } - - const McpStatusBadge = ({ status, className = '' }: McpStatusBadgeProps) => { -- const style = STATUS_STYLES[status] ?? STATUS_STYLES.disconnected; -+ const { t } = useT(); -+ const meta = STATUS_META[status] ?? STATUS_META.disconnected; - return ( - -- {style.label} -+ role="status" -+ aria-live="polite" -+ className={`shrink-0 px-2 py-1 text-[11px] border rounded-full ${meta.className} ${className}`}> -+ {t(meta.i18nKey)} - - ); - }; diff --git a/PR5-body.md b/PR5-body.md deleted file mode 100644 index 2bf43d2b9c..0000000000 --- a/PR5-body.md +++ /dev/null @@ -1,138 +0,0 @@ -## Summary - -- New **search/filter feature** for the installed MCP servers list in `Channels → ChannelConfigPanel → McpServersTab`. As users install more MCP servers, scanning the left-pane list becomes a real chore — this PR adds a controlled search input, live filtering, an accessible result counter, and arrow-key navigation across the list. -- New component `McpServerSearch.tsx`: controlled `` wrapped in a `role="search"` landmark, with a clear button that appears when the value is non-empty. Intentionally **no global keyboard shortcut** to avoid colliding with the app-wide `CommandProvider` in `App.tsx`. -- Extended `InstalledServerList.tsx`: optional `filter` prop case-insensitively matches against `display_name`, `qualified_name`, and `description`. New "X of Y servers" indicator announced via `role="status" aria-live="polite"`. New "No servers match \"{query}\"" empty state. ArrowUp/ArrowDown move focus across the visible server buttons (clamped at the edges; only the filtered set is traversed). -- 6 new i18n keys under the `mcp.installed.search.*` namespace, mirrored across all 13 locale chunks (English values used as the untranslated placeholder per the repo's standard pattern; counted in each locale's "untranslated" total exactly like the existing keys). -- 18 new Vitest tests (7 for `McpServerSearch`, 11 for the new behaviour in `InstalledServerList`). - -## Problem - -The MCP feature surface ships today via `pages/Channels.tsx` → `ChannelConfigPanel.tsx` → `McpServersTab.tsx`. The left pane is a flat scrolling list of installed servers (`InstalledServerList`). With three or four installs it's fine. The moment a user installs the eighth or fifteenth — which is the actual usage shape with the Smithery catalog — finding a specific server requires either scrolling or page-search via the browser/CEF shortcut. - -There's also no keyboard-only path for moving through the list. Each server is a `