feat(settings): add MCP server configuration panel#2355
Conversation
Adds a Settings panel under Developer Options that lets users configure external MCP clients (Claude Desktop, Cursor, Codex, Zed) to connect to the openhuman-core mcp stdio server — removing the need to hand-edit vendor JSON files. - New Tauri commands: mcp_resolve_binary_path (returns binary path + OS), mcp_open_client_config (opens per-OS config file in system editor) - React panel with tool list (10 tools), per-client JSON snippet generator, copy-to-clipboard, and Tauri-gated "Open Config File" button - OS-aware config file paths for all four supported clients - 8 Vitest tests + 9 Rust unit tests; all checks pass Closes tinyhumansai#2030
|
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 (1)
📝 WalkthroughWalkthroughAdds a Tauri backend to locate/open the bundled openhuman-core binary and per-client config files, a new McpServerPanel React settings UI (Claude/Cursor/Codex/Zed) with JSON snippets, tests, settings routing and menu entry, and localization strings. ChangesMCP Server Configuration UI
Sequence DiagramsequenceDiagram
participant User
participant Panel as McpServerPanel
participant Tauri as tauri_invoke
participant Resolver as resolve_binary_path
participant Config as config_path_for_client
participant OS as Filesystem
User->>Panel: open Settings Mcp Server
Panel->>Tauri: mcp_resolve_binary_path
Tauri->>Resolver: resolve_binary_path
Resolver->>OS: lookup and stat binary
Resolver-->>Tauri: return McpBinaryInfo
Panel->>Panel: build JSON snippet
User->>Panel: click Open Config File
Panel->>Tauri: mcp_open_client_config
Tauri->>Config: config_path_for_client
Config->>OS: ensure dirs and file
Config->>OS: spawn platform opener
OS-->>Tauri: success or error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
app/src/lib/i18n/en.ts (1)
1-2282: 💤 Low valueConsider splitting the translation file into domain modules.
The file has grown to 2282 lines, significantly exceeding the ~500-line guideline. While translation files are often kept monolithic, the natural section boundaries (Settings, Chat, Onboarding, Memory, etc.) suggest an opportunity to improve maintainability by splitting into smaller modules:
app/src/lib/i18n/ en/ settings.ts chat.ts onboarding.ts memory.ts common.ts index.ts // re-exports all modules as a single mapThis would preserve backward compatibility while making each domain easier to navigate and maintain. Most modern i18n libraries (e.g.,
react-i18next) support namespace-based loading.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/lib/i18n/en.ts` around lines 1 - 2282, The en.ts file (const en: TranslationMap and export default en) is too large; split it into domain modules (e.g., settings.ts, chat.ts, onboarding.ts, memory.ts, common.ts) each exporting a partial TranslationMap, then create an index.ts that imports and merges them into the single const en: TranslationMap = { ...common, ...settings, ...chat, ...onboarding, ...memory } and re-export export default en so existing imports (which expect the default en map and the TranslationMap type) keep working; ensure each module uses the same TranslationMap type and move corresponding keys from the big file into their domain modules.
🤖 Prompt for all review comments with AI agents
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 `@app/src/components/settings/panels/McpServerPanel.test.tsx`:
- Around line 23-29: The test's mock path doesn't match the component's import
so the stubbed useSettingsNavigation isn't applied; update the vi.mock call in
McpServerPanel.test.tsx to mock the exact module path used by the component
(useSettingsNavigation) — i.e., change the mock path from
'../../hooks/useSettingsNavigation' to the same relative path McpServerPanel
imports (../hooks/useSettingsNavigation) so the useSettingsNavigation export
(navigateToSettings, navigateBack, breadcrumbs) is correctly mocked for the
McpServerPanel tests.
In `@app/src/components/settings/panels/McpServerPanel.tsx`:
- Around line 99-109: Replace the direct console.debug calls in the
McpServerPanel promise chain with the project's namespaced debug logger (follow
the existing pattern used elsewhere, e.g. create/import a debug instance for the
"settings:McpServerPanel" namespace) — update the three occurrences around
invoke<McpBinaryInfo>('mcp_resolve_binary_path') (the resolved-success handler
that calls setBinaryInfo and the catch handler that calls setBinaryError) to use
that debug logger and include the same contextual strings and error details;
ensure the import/creation of the debug logger follows the module's established
pattern and only logs dev-level details.
- Around line 24-35: MCP_TOOLS currently contains hardcoded English strings;
replace these with i18n-backed strings by importing the app's translation
hook/function (e.g., useTranslation or t) into McpServerPanel and either store
translation keys in MCP_TOOLS (e.g., { nameKey: 'mcp.core.list_tools', descKey:
'mcp.core.list_tools_desc' }) or call t(...) when rendering name/description so
the displayed text is localized; also update the tablist label (the label string
rendered in the component near the same file) to use the translation function
and audit the other occurrence referenced (around the other line) to ensure it
also uses the same translation keys.
- Line 5: Replace the local import of isTauri from
'../../../utils/tauriCommands/common' with the repo-standard isTauri() exported
by the webview account service (use the isTauri() from webviewAccountService) so
Tauri detection is consistent across the frontend; locate the import and any
Tauri-guarded calls in McpServerPanel (e.g., places calling isTauri() or
invoking Tauri APIs) and either call the shared isTauri() or wrap the
invoke(...) calls in try/catch to safely guard Tauri API usage.
- Around line 115-119: The code currently sets os = binaryInfo?.os ?? 'macos',
causing Windows/Linux users to see macOS config paths when binaryInfo is
missing; change this to derive os from the runtime when binaryInfo is undefined
(e.g., map process.platform to the expected os token) so
configFilePathFor(activeClient, os) receives the actual platform; update the
logic around binaryInfo/binaryPath and references to
buildSnippet/configFilePathFor so they use the detected runtime OS instead of
the hardcoded 'macos'.
In `@app/src/lib/i18n/en.ts`:
- Around line 1951-1952: The string for key
'settings.mcpServer.binaryPathNotFound' is too developer‑centric; replace it
with a user-friendly message that explains the problem in plain language and
optionally suggests the developer command as secondary guidance (e.g., first say
the binary couldn't be found or OpenHuman server isn't installed/run, then
append a parenthetical developer hint like "developers: run `cargo build --bin
openhuman-core`"). Update the value for settings.mcpServer.binaryPathNotFound so
it communicates the issue to end users while retaining the original build
command as an optional developer note.
---
Nitpick comments:
In `@app/src/lib/i18n/en.ts`:
- Around line 1-2282: The en.ts file (const en: TranslationMap and export
default en) is too large; split it into domain modules (e.g., settings.ts,
chat.ts, onboarding.ts, memory.ts, common.ts) each exporting a partial
TranslationMap, then create an index.ts that imports and merges them into the
single const en: TranslationMap = { ...common, ...settings, ...chat,
...onboarding, ...memory } and re-export export default en so existing imports
(which expect the default en map and the TranslationMap type) keep working;
ensure each module uses the same TranslationMap type and move corresponding keys
from the big file into their domain modules.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dca6084f-bce6-468d-8bc3-6cd6e308dfc2
📒 Files selected for processing (8)
app/src-tauri/src/lib.rsapp/src-tauri/src/mcp_commands.rsapp/src/components/settings/hooks/useSettingsNavigation.tsapp/src/components/settings/panels/DeveloperOptionsPanel.tsxapp/src/components/settings/panels/McpServerPanel.test.tsxapp/src/components/settings/panels/McpServerPanel.tsxapp/src/lib/i18n/en.tsapp/src/pages/Settings.tsx
Fixes drift between en.ts and en-5.ts chunk, and adds English-fallback translations for all 11 locale chunk files so the i18n coverage check passes.
- Fix mock path in test: ../../hooks → ../hooks/useSettingsNavigation - Replace console.debug with namespaced debug logger (debug package) - Fix OS fallback on binary error: use navigator.userAgent instead of hardcoding 'macos', so Windows/Linux users see correct config paths - Add i18n key for tablist aria-label (settings.mcpServer.clientSelectorAriaLabel) - Improve binary-not-found message to be user-friendly - Add aria-label and updated message to en-5.ts chunk + all locale chunks
graycyrus
left a comment
There was a problem hiding this comment.
Review — PR #2355: feat(settings): add MCP server configuration panel
Nice work on this one. The panel is well-structured, the Rust commands are cleanly separated with proper error handling, the i18n coverage is thorough, and the test suite covers the right surface area. All CodeRabbit findings have been addressed in the latest commits.
Changes summary
| Area | Files | What changed |
|---|---|---|
| Rust (Tauri shell) | mcp_commands.rs, lib.rs |
Two new commands: binary path resolution (dev/release) + config file opener with OS dispatch |
| Frontend | McpServerPanel.tsx, Settings.tsx, DeveloperOptionsPanel.tsx, useSettingsNavigation.ts |
New settings panel with client tabs, snippet generator, copy/open actions, Tauri gate |
| Tests | McpServerPanel.test.tsx, mcp_commands.rs (9 unit tests) |
8 Vitest + 9 Rust tests covering clients, OS combos, clipboard, error fallback |
| i18n | 12 locale chunk files + en.ts |
18 new translation keys across all locales |
Issue alignment
Closes #2030 — all acceptance criteria met. Panel renders with 10 tools (original 6 + 4 added since the issue was filed), per-client snippets generate correctly, copy and open-config work, binary resolution covers dev and release modes.
Findings
1 minor finding (details in inline comment). No critical or major issues.
…@graycyrus on mcp_commands.rs:267) When mcp_open_client_config creates the config file for the first time, write `{}` instead of an empty string so MCP clients (Zed, Claude Desktop, etc.) can parse the file as valid JSON before the user pastes the snippet.
M3gA-Mind
left a comment
There was a problem hiding this comment.
pr-manager summary: All actionable review comments have been addressed and pushed.
Fixed in this run:
app/src-tauri/src/mcp_commands.rs— seeded newly-created config file with{}instead of empty bytes, so MCP clients don't fail to parse it before the user pastes the snippet (addresses @graycyrus).
Already addressed in prior commits (verified against current code):
- Mock path mismatch in
McpServerPanel.test.tsx— already corrected to../hooks/useSettingsNavigation. isTauriimport source — PR author correctly dismissed;utils/tauriCommands/commonis the canonical definition; CodeRabbit agreed.- Hardcoded MCP_TOOLS strings — aria-label was i18n-backed; tool names/descriptions are MCP protocol identifiers not UI copy; CodeRabbit agreed.
console.debug→ namespaced logger — already usingdebug('mcp-server-panel').- macOS OS fallback — already using
navigator.userAgentdetection instead of hardcoded'macos'. binaryPathNotFoundmessage — already user-friendly with developer hint as secondary.
Deferred (out of scope for this PR):
Standards: app/src/lib/i18n/en.ts has grown to 2282 lines, exceeding the ~500-line guideline (flagged by @coderabbitai). This is a pre-existing structural issue that predates this PR — en.ts was already >2200 lines before the 18 new MCP keys added here. Recommended follow-up: open a dedicated refactor PR to split en.ts into domain modules (settings.ts, chat.ts, onboarding.ts, etc.) with an index.ts merging them. Should not block this PR.
graycyrus
left a comment
There was a problem hiding this comment.
Continuation review — previous REQUEST_CHANGES addressed.
Commit 735fceb fixes the empty-file config seeding (b"" → b"{}") flagged in review 1. All prior CodeRabbit findings were already resolved in earlier commits.
| File | Change |
|---|---|
mcp_commands.rs:267 |
Config file seed changed from empty bytes to valid JSON {} |
0 critical, 0 major, 0 minor. All graycyrus threads resolved. Clean PR — moving to to-be-approved/.
|
@M3gA-Mind pls test this once properly locally and update your observations here |
Summary
mcp_resolve_binary_path(returns binary path + OS) andmcp_open_client_config(opens the client's config file in the system editor)Problem
openhuman-core mcpstdio server ships 10 memory/tree tools but has zero UI surface — users must locate the binary, find the per-client config file path, and hand-write the JSONSolution
app/src-tauri/src/mcp_commands.rs: two new Tauri shell commands with OS-aware path resolution, auto-create config dirs/files if absent, and platform-specificopen/explorer/xdg-opendispatchMcpServerPanel.tsx: reads binary path on mount viainvoke, generates the correct snippet shape per client (Zed usescontext_servers, others usemcpServers), gracefully degrades when binary is not foundtarget/debug/), env override (OPENHUMAN_CORE_BINARY_PATH), and release mode (sibling of host exe)Submission Checklist
pnpm test:coveragepasses; new React component and Rust pure functions are fully covered; Tauri command wrappers and release-mode binary path are not testable without a packaged build (noted in PR as a known caveat)11.1.4in the UICloses #2030Impact
openhuman-coreis not bundled in the packaged app — shows a build instruction fallback messageRelated
11.1.4(MCP stdio server)openhuman-corebinary is included in the packaged.appbundle (sidecar was removed in fix(core,cef): run core in-process and stop orphaning CEF helpers on Cmd+Q #1061; if the binary is not bundled the panel will show the fallback message in production)AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
feat/mcp-settings-panelValidation Run
pnpm --filter openhuman-app format:checkpnpm typecheckpnpm debug unit McpServerPanel.test.tsx— 8/8 passedcargo fmt --check+cargo check --manifest-path app/src-tauri/Cargo.toml— cleanValidation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
isTauri()guard on "Open Config File" button; binary-not-found degrades to placeholder with build instructionsDuplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Tests
Localization