feat(mcp): interactive Tool Execution Playground (schema viewer + JSON args editor + run + history)#2644
Open
aashir-athar wants to merge 2 commits into
Open
Conversation
…N args editor + run + history)
Contributor
|
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 (5)
📝 WalkthroughWalkthroughThis PR adds an interactive MCP tool execution playground modal, renders optional per-tool "Try" buttons in the tool list, wires the playground from InstalledServerDetail (only when connected), adds tests for UI and behavior, and supplies translations across locales. ChangesMCP Tool Execution Playground
Sequence DiagramsequenceDiagram
participant User
participant InstalledServerDetail
participant McpToolList
participant McpToolPlayground
participant mcpClientsApi
User->>McpToolList: Click "Try" button on a tool
McpToolList->>InstalledServerDetail: onTryTool(tool)
InstalledServerDetail->>InstalledServerDetail: setState playgroundTool
InstalledServerDetail->>McpToolPlayground: Render modal with tool
Note over User,McpToolPlayground: User enters JSON args
User->>McpToolPlayground: Click Run or press Cmd/Ctrl+Enter
McpToolPlayground->>mcpClientsApi: toolCall(serverId, tool, args)
alt Success
mcpClientsApi-->>McpToolPlayground: result data
McpToolPlayground->>McpToolPlayground: Record to history
McpToolPlayground->>User: Display formatted result
else Error (is_error=true)
mcpClientsApi-->>McpToolPlayground: error response
McpToolPlayground->>User: Display error with alert role
else Exception
mcpClientsApi-->>McpToolPlayground: rejected promise
McpToolPlayground->>User: Display error message
end
User->>McpToolPlayground: Load prior args from history or close modal
McpToolPlayground->>InstalledServerDetail: onClose()
InstalledServerDetail->>InstalledServerDetail: Clear playgroundTool
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/src/components/channels/mcp/McpToolPlayground.test.tsx (1)
9-10: ⚡ Quick winUse shared test render utilities instead of a local harness.
This suite should use the existing
app/src/test/helper render so provider setup stays consistent across tests.As per coding guidelines: Use existing helpers from `app/src/test/` (`test-utils.tsx`, shared mock backend) before adding new harness code.💡 Suggested fix
-import { act, fireEvent, render, screen, waitFor, within } from '`@testing-library/react`'; +import { act, fireEvent, screen, waitFor, within } from '`@testing-library/react`'; +import { render } from '../../../test/test-utils';Also applies to: 38-45
🤖 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/components/channels/mcp/McpToolPlayground.test.tsx` around lines 9 - 10, The test file McpToolPlayground.test.tsx is using a local harness and direct imports from '`@testing-library/react`'; replace that with the shared test helpers from app/src/test/ (e.g., import the custom render and providers from test-utils.tsx and use the shared mock backend setup) so provider/setup is consistent; update imports to use the shared render utility instead of render from '`@testing-library/react`', remove or migrate any local harness/setup code in this file (and any other tests in the same suite that replicate it), and adapt test usages (act, fireEvent, screen, waitFor, within) to work with the shared utilities.app/src/components/channels/mcp/McpToolList.test.tsx (1)
64-66: ⚡ Quick winAvoid style-coupled assertions in this test.
Using
querySelector('p.text-\\[11px\\]')couples the test to Tailwind class names rather than behavior. Prefer behavior-level assertions only.Suggested patch
- // The list_dir item must NOT have a description-styled paragraph — - // find its row and verify it has only the name paragraph. - const listDirItem = screen.getByText('list_dir').closest('li')!; - const descriptionPara = listDirItem.querySelector('p.text-\\[11px\\]'); - expect(descriptionPara).toBeNull(); + // Behavior-level check: only the two defined descriptions are rendered. + expect( + screen.queryAllByText(/Reads a file from disk|Writes data to a file/) + ).toHaveLength(2);As per coding guidelines:
app/**/*.test.{ts,tsx}: Prefer testing behavior over implementation details.🤖 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/components/channels/mcp/McpToolList.test.tsx` around lines 64 - 66, The test in McpToolList.test.tsx uses a style-coupled selector (querySelector('p.text-\\[11px\\]')) — replace this with a behavior-level assertion: locate the list item via listDirItem (as already done) and assert the absence of the description by querying for the description content or role/text within that list item (e.g., using within(listDirItem).queryByText(...) or listDirItem.querySelector by semantic tagless text lookup) and expect that query to be null; remove any dependency on Tailwind class names and assert only on the presence/absence of the description text or semantic element.
🤖 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/channels/mcp/InstalledServerDetail.tsx`:
- Around line 245-250: The McpToolPlayground modal can remain open across
disconnects because it is only keyed by playgroundTool; update rendering to gate
rendering on status === 'connected' (e.g., render McpToolPlayground only when
playgroundTool && status === 'connected') and ensure you clear playgroundTool by
calling setPlaygroundTool(null) wherever you handle explicit
disconnect/uninstall paths (the disconnect/uninstall handlers that change status
or remove the server) so the modal cannot stay open after losing connection;
reference playgroundTool, McpToolPlayground, status, setPlaygroundTool and
server.server_id when making these changes.
In `@app/src/components/channels/mcp/McpToolPlayground.tsx`:
- Around line 143-147: When starting a run (in the block that currently calls
setParseError(null); setIsRunning(true); setResultText(null);
setResultIsError(false); const submittedArgs = argsJson;), also reset the
"copied" UI state so a previous "Copied" label doesn't persist; add a call to
the copy-state setter (e.g. setIsCopied(false) — or if your state is named
setCopied or setResultCopied or setCopyStatus, call that) immediately after
setResultIsError(false) so the copy feedback always reflects the new result.
---
Nitpick comments:
In `@app/src/components/channels/mcp/McpToolList.test.tsx`:
- Around line 64-66: The test in McpToolList.test.tsx uses a style-coupled
selector (querySelector('p.text-\\[11px\\]')) — replace this with a
behavior-level assertion: locate the list item via listDirItem (as already done)
and assert the absence of the description by querying for the description
content or role/text within that list item (e.g., using
within(listDirItem).queryByText(...) or listDirItem.querySelector by semantic
tagless text lookup) and expect that query to be null; remove any dependency on
Tailwind class names and assert only on the presence/absence of the description
text or semantic element.
In `@app/src/components/channels/mcp/McpToolPlayground.test.tsx`:
- Around line 9-10: The test file McpToolPlayground.test.tsx is using a local
harness and direct imports from '`@testing-library/react`'; replace that with the
shared test helpers from app/src/test/ (e.g., import the custom render and
providers from test-utils.tsx and use the shared mock backend setup) so
provider/setup is consistent; update imports to use the shared render utility
instead of render from '`@testing-library/react`', remove or migrate any local
harness/setup code in this file (and any other tests in the same suite that
replicate it), and adapt test usages (act, fireEvent, screen, waitFor, within)
to work with the shared utilities.
🪄 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: f9b8057d-98cf-404f-83b7-bae0583606b4
📒 Files selected for processing (19)
app/src/components/channels/mcp/InstalledServerDetail.tsxapp/src/components/channels/mcp/McpToolList.test.tsxapp/src/components/channels/mcp/McpToolList.tsxapp/src/components/channels/mcp/McpToolPlayground.test.tsxapp/src/components/channels/mcp/McpToolPlayground.tsxapp/src/lib/i18n/chunks/ar-1.tsapp/src/lib/i18n/chunks/bn-1.tsapp/src/lib/i18n/chunks/de-1.tsapp/src/lib/i18n/chunks/en-1.tsapp/src/lib/i18n/chunks/es-1.tsapp/src/lib/i18n/chunks/fr-1.tsapp/src/lib/i18n/chunks/hi-1.tsapp/src/lib/i18n/chunks/id-1.tsapp/src/lib/i18n/chunks/it-1.tsapp/src/lib/i18n/chunks/ko-1.tsapp/src/lib/i18n/chunks/pt-1.tsapp/src/lib/i18n/chunks/ru-1.tsapp/src/lib/i18n/chunks/zh-CN-1.tsapp/src/lib/i18n/en.ts
…ect/uninstall, reset copy-feedback on run, behavior-level test selectors, prettier
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mcpClientsApi.toolCall, and renders the structured result. Closes the long-standing gap between "I can see this tool's schema" and "I can actually invoke it without writing a chat prompt or curl call".McpToolPlayground.tsx(432 lines) — modal dialog with: collapsible JSON-schema viewer; auto-focused JSON args textarea with live-clearing parse errors; Format action (pretty-prints valid JSON, leaves invalid untouched); Run button + Cmd/Ctrl+Enter shortcut from the textarea; result panel with role/aria-live varying by success vs error; copy-to-clipboard with transient "Copied" state; collapsible in-session invocation history capped at 10 entries with one-click "Load" to restore prior args.McpToolList.tsxextended with an optionalonTryTool(tool)callback — when omitted the list renders as before (preserved for any other call site); when provided each tool gets an accessibly-labelled "Try" button next to its name.InstalledServerDetail.tsxwires the playground in: hosts theplaygroundToolstate, passesonTryToolto the list only when the server is connected, renders<McpToolPlayground>conditionally with proper lifecycle binding.mcp.toolList.*(2) andmcp.playground.*(18), mirrored across all 13 locale chunks per the repo'si18n:checkparity rule.McpToolPlayground.test.tsx, 4 added toMcpToolList.test.tsx) plus an existingMcpToolListtest rewritten to match the new row structure.Problem
Today, the only way for a user to test whether an installed MCP server's tool actually does what its description promises is to:
This is an atrocious feedback loop for someone debugging a freshly-installed server, validating env-var changes, or just trying to understand a tool's behaviour. The existing
mcpClientsApi.toolCallRPC has been wired since #2559 — there was simply no UI surface to invoke it directly.The MCP detail view also hits a wall once the server is connected and its tool list is rendered: the list shows a name + description and that's it. No way to peek at the input schema, no way to invoke. Every tool is read-only documentation.
Solution
McpToolPlayground.tsx— the modalA self-contained modal opened by the parent setting
playgroundTool: McpTool | null. Lifecycle bound to the modal's lifetime: focus management, Esc binding, click-outside binding, history state all live inside the component and die with it.Args editor:
<textarea>controlled viaargsJsonstate, initialised to'{}'so the empty state is also valid JSON.onChangelive-clears any stale parse error from the previous Run.aria-label,aria-describedbypointing to the help text,spellCheck={false}(it's code, not prose).JSON.stringify(JSON.parse(raw), null, 2)— pretty-prints valid JSON, returns the original string untouched if invalid (never throws on Format).Run flow:
{}. Bad JSON →setParseError, renderrole="alert", abort.isRunning, clear prior result.await mcpClientsApi.toolCall({ server_id, tool_name, arguments: parsed }).setResultText(JSON.stringify(callResult.result, null, 2)),setResultIsError(callResult.is_error), push to history (cap 10, most-recent-first).role="alert", also pushed to history (so a failed try is recoverable).setIsRunning(false)infinally.Keyboard: Cmd/Ctrl+Enter inside the textarea triggers Run (
preventDefaultso the modifier+Enter doesn't insert a newline). Esc on the document closes (gated through React's effect cleanup so it doesn't survive unmount).Result panel:
role="status" aria-live="polite"— screen readers announce updates as new invocations land.is_error: trueOR an exception was thrown):role="alert" aria-live="assertive".<pre>; the panel ships with a copy-to-clipboard button that flips to "Copied" for ~1.5s on success and silently no-ops on browsers/contexts without clipboard access.History:
History (N)so the count is visible without expanding.McpToolList.tsx— the entry pointOptional
onTryTool?: (tool: McpTool) => voidprop:aria-label="Open execution playground for {name}").One pre-existing test (
does not render description paragraph when description is undefined) used ap + pselector against the previous flat row structure; updated to match the new row structure (<div>wrapper for name + Try, then optional<p>for description) without weakening the assertion.InstalledServerDetail.tsx— the orchestratorThree small additions:
McpToolPlayground+ aplaygroundToolstate slot.onTryTool={status === 'connected' ? setPlaygroundTool : undefined}so the Try button is gated on a live connection (a disconnected server has no tools, but this is belt-and-suspenders against future state drift).<McpToolPlayground serverId={server.server_id} tool={playgroundTool} onClose={() => setPlaygroundTool(null)} />so the modal's lifecycle is bound to a specific tool invocation session.i18n
20 new keys: 2 in
mcp.toolList.*(Try button text + per-tool aria-label with{name}), 18 inmcp.playground.*(title with{name}, close, schema viewer, args label/help, run shortcut hint, Format, run/running/result/result-error/copy/copied/history/history-empty/history-load/invalid-json/unexpected-error). Added toen.tsand to all 13 locale chunks (en-1.ts…zh-CN-1.ts). English values used as untranslated placeholders for non-English locales per the existing pattern.Verified locally:
Submission Checklist
McpToolPlayground.test.tsx, 4 inMcpToolList.test.tsx). Covers: modal a11y attributes (role="dialog",aria-modal,aria-labelledby); description present / absent; close via Esc / button / backdrop click (and not via clicking the dialog card itself); auto-focus on mount; collapsible schema viewer; default{}args; invalid-JSON refusal withrole="alert"; whitespace-only args treated as{}; Format handling for valid and invalid input; success result rendering withrole="status";is_error: truerendering withrole="alert"; thrown RPC exception rendering; Error vs non-Error fallback messages; Run button disabled state during in-flight calls; Cmd+Enter / Ctrl+Enter shortcuts; bare Enter doesn't run; history recording; Load restores args; history cap at 10 with oldest falling off; error invocations recorded; empty-history placeholder; per-tool Try button rendering whenonTryToolis provided and absent when omitted; Try invokes the callback with the right tool object; Try renders even for description-less tools.McpToolPlayground.tsxand every new line inMcpToolList.tsxis exercised by the new tests. Local Vitest: 106/106 passing across the full MCP suite (8 test files); no regression in any pre-existing test.## Related— N/A.mcpClientsApi.toolCallover the existing JSON-RPC; the optional clipboard write usesnavigator.clipboard.writeTextwhich is best-effort and silently degrades.Closes #NNNin## Related— no specific issue; organic UX improvement.Impact
InstalledServerDetailwhich only renders inChannels → ChannelConfigPanel. No iOS / web impact.playgroundTool. History is a bounded in-memory array (cap 10). JSON parse / stringify happens on Run / Format only — never per keystroke. Schema viewer is collapsible (closed by default).McpToolList'sonTryToolis an optional prop. Every existing call site (justInstalledServerDetailtoday) keeps working unchanged when the prop is omitted — verified by the existing test suite passing without modification.role="status"/role="alert"with matchingaria-livepolite/assertive; all interactive buttons havearia-labels; status dots and icons arearia-hidden="true"; Try button aria-label includes the tool name for context.Related
mcp.toolList.tryTool*/mcp.playground.*keys across the 12 non-English locales (they currently fall back to English placeholders; flagged in each locale'suntranslatedcount).AI Authored PR Metadata
Linear Issue
Commit & Branch
feat/mcp-tool-playgroundValidation 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. Full repo currently shows 62 warnings + 0 errors (one error I introduced — duplicate React imports — was fixed before commit by merging into a single import). Zero errors and zero warnings now attributable to the files this PR adds or modifies.pnpm vitest run src/components/channels/mcp/— 106/106 passing including 32 new tests; pre-existingSkills.mcp-coming-soon.test.tsxstill passes (confirms no regression to the intentionally-pinned Skills-page placeholder).pnpm i18n:check— exit 0, every locale at1:1211/1211parity, 0 missing keys, 0 extra keys, 0 drift.Validation Blocked
command:pnpm --filter openhuman-app format:check(chainscargo fmt --check) 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; the two new files were authored with LF.Behavior Changes
Parity Contract
McpToolList's API is backward-compatible (the newonTryToolprop is optional). When omitted the list renders byte-for-byte as before — verified by every pre-existingMcpToolListtest still passing without modification.InstalledServerDetail's existing flows (connect / disconnect / uninstall / config assistant) are untouched.mcpClientsApi.toolCallRPC the agent uses; the core's autonomy gate and per-tool security policy still apply. The clipboard write degrades silently on platforms withoutnavigator.clipboard. Esc handler is bound on document and torn down in the effect cleanup so it doesn't survive unmount.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Bug Fixes