docs(webmcp): the ui_ prefix means browser-fulfilled, not "drives the UI" - #3640
Conversation
… UI" `ui_ask_user` (#3635) collects input rather than driving a screen, which made three definitional docstrings inaccurate — each defined the namespace by what the tools happened to do rather than by what the prefix asserts. The prefix asserts exactly one thing: the BROWSER resolves the call. That is what the server's skip gate and pause predicate key on, so a browser-fulfilled tool named anything else would either get executed server-side or leave the stream waiting for a result nobody sends. Worth stating, because the narrowness is the point and both wrong readings are inviting: - "it drives the UI" — no longer true of the whole set, - "it's a WebMCP tool" — never true. The catalog is WebMCP-SHAPED and is deliberately not exposed to browser-native agents (`document.modelContext` / `navigator.modelContext`). The shared shape buys familiarity and a clean annotations contract, nothing else. Comments only — no executable line changes. Left the incidental "WebMCP UI tool" mentions elsewhere alone: they label the layer in passing rather than define it, and rewriting ~20 of them would be churn. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_bc653279-f601-442e-84eb-ca3c100c1512) |
📄 Knowledge reviewDosu skipped reviewing this PR because your organization has used its |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
WalkthroughUpdated documentation for the v1 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
mcpjam-inspector/client/src/lib/webmcp/ui-tools-catalog.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: ESLint configuration in --config is invalid:
mcpjam-inspector/client/src/lib/webmcp/ui-tools-registry.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: ESLint configuration in --config is invalid:
mcpjam-inspector/shared/client-fulfilled-tools.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: ESLint configuration in --config is invalid:
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@mcpjam-inspector/shared/client-fulfilled-tools.ts`:
- Around line 18-22: Update the explanatory comment near
isClientFulfilledToolName to scope its claim specifically to ui_* tool names, or
explicitly mention both the ui_ prefix and isAppToolAlias(name) predicates; keep
the documented server skip-gate and pause-predicate behavior accurate without
changing the predicate implementation.
🪄 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 Plus
Run ID: a58c918d-a48b-4cdf-9858-0f8af492444d
📒 Files selected for processing (3)
mcpjam-inspector/client/src/lib/webmcp/ui-tools-catalog.tsmcpjam-inspector/client/src/lib/webmcp/ui-tools-registry.tsmcpjam-inspector/shared/client-fulfilled-tools.ts
| * (`document.modelContext` / `navigator.modelContext`). The prefix is load | ||
| * bearing precisely because it is this narrow: it is the token the server's | ||
| * skip gate and pause predicate key on, so a browser-fulfilled tool named | ||
| * anything else would either be executed server-side or leave the stream | ||
| * waiting on a result nobody will send. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Scope the predicate claim to ui_* tools.
isClientFulfilledToolName also accepts isAppToolAlias(name) at Line 53. Therefore, the server does not recognize browser-fulfilled calls only through the ui_ prefix. As written, “a browser-fulfilled tool named anything else” is too broad and can mislead maintainers about the accepted client-fulfilled names.
Limit this explanation to ui_* tools, or document both naming predicates.
🤖 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 `@mcpjam-inspector/shared/client-fulfilled-tools.ts` around lines 18 - 22,
Update the explanatory comment near isClientFulfilledToolName to scope its claim
specifically to ui_* tool names, or explicitly mention both the ui_ prefix and
isAppToolAlias(name) predicates; keep the documented server skip-gate and
pause-predicate behavior accurate without changing the predicate implementation.
Internal previewPreview URL: https://mcp-inspector-pr-3640.up.railway.app |
Follow-up to #3635. Comments only — no executable line changes.
ui_ask_usercollects input rather than driving a screen, which left three definitional docstrings inaccurate. Each defined the namespace by what the tools happened to do, rather than by what the prefix actually asserts.The prefix asserts exactly one thing: the browser resolves the call. That's what the server's skip gate (
isSkippableClientFulfilledToolCall) and pause predicate (hasUnresolvedClientFulfilledToolCalls) key on — a browser-fulfilled tool named anything else would either be executed server-side or leave the stream waiting on a result nobody sends.Worth writing down because the narrowness is the point, and both wrong readings are inviting:
document.modelContext/navigator.modelContext). The shared shape buys familiarity and a clean annotations contract, nothing more.Updated:
shared/client-fulfilled-tools.ts,client/src/lib/webmcp/ui-tools-registry.ts,client/src/lib/webmcp/ui-tools-catalog.ts.Deliberately not updated: the ~20 incidental "WebMCP UI tool" mentions elsewhere. They label the layer in passing rather than define it, and rewriting them all would be churn for no clarity gain.
Client typecheck clean;
client/src/lib/webmcp+client/src/lib/__tests__green (1,215 tests).🤖 Generated with Claude Code
Note
Low Risk
Comments only; no logic, APIs, or server/client execution paths change.
Overview
Documentation-only — no runtime behavior changes. Three module-level comment blocks are rewritten so the
ui_*namespace is defined by what the prefix asserts (the browser fulfills the tool call and suppliesaddToolOutput), not by what most tools happen to do.The updated text spells out that
ui_ask_useris the counterexample (collects input via an inline card, does not drive the inspector), and that the catalog is WebMCP-shaped for annotations/familiarity but is not registered ondocument.modelContext/navigator.modelContext.shared/client-fulfilled-tools.tsties that narrow meaning to the server skip gate and stream pause predicate so readers do not mis-name browser-fulfilled tools.Touched files:
client-fulfilled-tools.ts,ui-tools-registry.ts,ui-tools-catalog.ts.Reviewed by Cursor Bugbot for commit 7a9734d. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Clarifies that the
ui_*prefix means the browser resolves the tool call, not that it drives the UI or is a WebMCP tool. Updates docstrings inshared/client-fulfilled-tools.ts,client/src/lib/webmcp/ui-tools-registry.ts, andclient/src/lib/webmcp/ui-tools-catalog.ts; no executable changes.Written for commit 7a9734d. Summary will update on new commits.