Add addEventListener/removeEventListener with DOM-model on* semantics#571
Closed
Add addEventListener/removeEventListener with DOM-model on* semantics#571
Conversation
This PR adds comprehensive tool support for MCP Apps, enabling apps to register their own tools and handle tool calls from the host. - Add `registerTool()` method for registering tools with input/output schemas - Add `oncalltool` setter for handling tool call requests from host - Add `onlisttools` setter for handling tool list requests from host - Add `sendToolListChanged()` for notifying host of tool updates - Registered tools support enable/disable/update/remove operations - Add `sendCallTool()` method for calling tools on the app - Add `sendListTools()` method for listing available app tools - Fix: Use correct ListToolsResultSchema (was ListToolsRequestSchema) - Add comprehensive tests for tool registration lifecycle - Add tests for input/output schema validation - Add tests for bidirectional tool call communication - Add tests for tool list change notifications - All 27 tests passing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Implement automatic `oncalltool` and `onlisttools` handlers that are initialized when apps register tools. This removes the need for manual handler setup and ensures tools work seamlessly out of the box. - Add automatic `oncalltool` handler that routes calls to registered tools - Add automatic `onlisttools` handler that returns full Tool objects with JSON schemas - Convert Zod schemas to MCP-compliant JSON Schema using `zod-to-json-schema` - Add 27 comprehensive tests covering automatic handlers and tool lifecycle - Test coverage includes error handling, schema validation, and multi-app isolation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Always return inputSchema as object (never undefined) - Keep filter for enabled tools only in list - Update test to match behavior (only enabled tools in list) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Avoid double-verb naming pattern for consistency with existing API.
- Add McpUiScreenshotRequest/Result and McpUiClickRequest/Result types - Add onscreenshot and onclick handlers to App class - Add screenshot() and click() methods to AppBridge class - Generate updated Zod schemas
…t-view - Uncomment and fix the navigate-to tool for animated navigation - Add get-current-view tool to query camera position and bounding box - Add flyToBoundingBox function for smooth camera animation - Add setLabel function for displaying location labels
- get-document-info: Get title, current page, total pages, zoom level - go-to-page: Navigate to a specific page - get-page-text: Extract text from a page - search-text: Search for text across the document - set-zoom: Adjust zoom level
…dertoy, wiki-explorer, and threejs budget-allocator: - get-allocations: Get current budget allocations - set-allocation: Set allocation for a category - set-total-budget: Adjust total budget - set-company-stage: Change stage for benchmarks - get-benchmark-comparison: Compare against benchmarks shadertoy: - set-shader-source: Update shader source code - get-shader-info: Get shader source and compilation status - Sends errors via updateModelContext wiki-explorer: - search-article: Search for Wikipedia articles - get-current-article: Get current article info - highlight-node: Highlight a graph node - get-visible-nodes: List visible nodes threejs: - set-scene-source: Update the Three.js scene source code - get-scene-info: Get current scene state and any errors - Sends syntax errors to model via updateModelContext
Wiki Explorer: - Add expand-node tool - the critical missing tool for graph exploration - Claude can now programmatically expand nodes to discover linked articles Server descriptions updated to mention widget tools: - map-server: navigate-to, get-current-view - pdf-server: go-to-page, get-page-text, search-text, set-zoom, get-document-info - budget-allocator: get-allocations, set-allocation, set-total-budget, etc. - shadertoy: set-shader-source, get-shader-info - wiki-explorer: expand-node, search-article, highlight-node, etc. All descriptions now mention 'Use list_widget_tools to discover available actions.'
…etails The server tool descriptions now just mention that widgets are interactive and can be controlled, without teaching the model about list_widget_tools (which is the client's responsibility to teach). Before: 'The widget exposes tools: X, Y, Z. Use list_widget_tools to discover...' After: 'The widget is interactive and exposes tools for X and Y.'
These will be proposed separately from the app tool registration changes.
- Resolve import conflicts in src/app.ts (keep both ReadResource* and Tool* imports) - Fix threejs-server wrapper: preserve registerWidgetTools + adopt main's fullscreen sizing - Fix pdf-server description: adopt disableInteract ternary, keep app-tool mention - Remove redundant bridge.connect() in app-bridge.test.ts (parent beforeEach already connects) - Remove orphaned codePreview declaration in shadertoy (usage removed in branch refactor) - Fix pre-commit hook: exclude deleted files from re-staging (--diff-filter=ACMR) - Add .claude/ to .prettierignore (local dev artifacts)
When registerTool is called before connect() on an App created without
explicit tools capability, setRequestHandler's capability assertion
would throw, breaking app initialization at module load.
Auto-register { tools: { listChanged: true } } on first registerTool
call (pre-connect only), mirroring McpServer.registerTool behavior.
Fixes pdf-annotations e2e failures where the PDF canvas never rendered
because registerTool threw at module scope.
Converts the 5 placeholder app-registered tools into 12 tools that map directly to the server's interact commands (navigate, search, find, search_navigate, zoom, add_annotations, update_annotations, remove_annotations, highlight_text, fill_form, get_text, get_screenshot) plus the existing get-document-info. Implementation stays DRY by dispatching through the existing processCommands() handler — each tool callback constructs a PdfCommand and runs it via a small runCommand() wrapper. For get_text and get_screenshot, the page-data collection is extracted from handleGetPages into a shared collectPageData() helper so results can be returned directly instead of round-tripping through the server. Tool names and zod schemas mirror the interact command parameter shapes from server.ts so the model sees the same surface whether it goes through interact or app-tools. The server-side interact tool is unchanged and remains available for hosts without app-tool support.
…ion handling The on* setters in App and AppBridge previously wrapped setNotificationHandler and setRequestHandler directly, which replace rather than append. Two pieces of code listening to the same event (e.g. useHostStyleVariables and useHostFonts both setting onhostcontextchanged) would silently stomp each other. This introduces a ProtocolWithEvents intermediate class that: - Adds addEventListener(name, handler) / removeEventListener(name, handler). The first listener for an event lazily registers a dispatcher that fans out to all listeners. Subclasses supply an event-name -> schema map. - Overrides setRequestHandler / setNotificationHandler to throw when a handler for the same method is already registered, so accidental overwrites surface as errors instead of silent bugs. - Exposes setDefaultRequestHandler for overridable constructor defaults (AppBridge's requestdisplaymode, App.registerTool's auto-handlers). The notification on* setters now delegate to addEventListener (append semantics), and useHostStyles switches to addEventListener with proper effect cleanup.
…oexistence The on* setters (ontoolinput, oninitialized, onsizechange, etc.) previously delegated to addEventListener, which meant each assignment appended a listener rather than replacing it. This violated the universal JavaScript convention that `obj.onfoo = callback` has replace semantics (like DOM's `el.onclick`), and silently broke patterns like save/restore (read previous handler, wrap, then restore). This commit rewrites ProtocolWithEvents to follow the DOM event model: - on* setters have replace semantics (like el.onclick) - on* getters return the current handler (or undefined) - addEventListener/removeEventListener coexist independently - Dispatch order: onEventDispatch → on* handler → addEventListener listeners - Listener array is snapshot-copied during dispatch for reentrancy safety For request-handler setters (oncalltool, onteardown, onmessage, etc.): - Added getters returning the stored user callback - Use replaceRequestHandler to bypass double-set protection - Accept undefined to clear Direct setRequestHandler/setNotificationHandler calls still throw on double-registration to catch accidental overwrites in advanced usage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
PreviewPreview deployments for this PR have been cleaned up. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| it("App notification setters replace (DOM onclick model)", async () => { | ||
| const a: unknown[] = []; | ||
| const b: unknown[] = []; | ||
| app.ontoolinput = (p) => a.push(p); |
|
|
||
| it("App notification setter can be cleared with undefined", async () => { | ||
| const a: unknown[] = []; | ||
| app.ontoolinput = (p) => a.push(p); |
| const tool2 = app.registerTool("tool2", {}, async (_args: any) => ({ | ||
| content: [], | ||
| })); | ||
| const tool3 = app.registerTool("tool3", {}, async (_args: any) => ({ |
| const appCapabilities = { tools: { listChanged: true } }; | ||
| app = new App(testAppInfo, appCapabilities, { autoResize: false }); | ||
|
|
||
| const tool1 = app.registerTool( |
@modelcontextprotocol/ext-apps
@modelcontextprotocol/server-basic-preact
@modelcontextprotocol/server-basic-react
@modelcontextprotocol/server-basic-solid
@modelcontextprotocol/server-basic-svelte
@modelcontextprotocol/server-basic-vanillajs
@modelcontextprotocol/server-basic-vue
@modelcontextprotocol/server-budget-allocator
@modelcontextprotocol/server-cohort-heatmap
@modelcontextprotocol/server-customer-segmentation
@modelcontextprotocol/server-debug
@modelcontextprotocol/server-map
@modelcontextprotocol/server-pdf
@modelcontextprotocol/server-scenario-modeler
@modelcontextprotocol/server-shadertoy
@modelcontextprotocol/server-sheet-music
@modelcontextprotocol/server-system-monitor
@modelcontextprotocol/server-threejs
@modelcontextprotocol/server-transcript
@modelcontextprotocol/server-video-resource
@modelcontextprotocol/server-wiki-explorer
commit: |
setDefaultRequestHandler was an untracked handler registration that could silently overwrite user-set handlers if called after on* setters. replaceRequestHandler serves the same purpose (allows re-registration) without the silent-overwrite risk. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ma emission
- Output schema validation no longer returns parseResult.data (the
validated structuredContent alone) instead of the full CallToolResult.
- tools/list now includes the tool title field.
- outputSchema is only emitted when the tool defines one (previously
always emitted a placeholder { type: "object", properties: {} }).
- Remove unused Request/Result imports.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Superseded by a clean rebased PR — this branch included unrelated tool-registration commits. |
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
2 participants
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.
Adds DOM-model event semantics to
on*setters and introducesaddEventListener/removeEventListenerfor multi-listener support. Theon*setters now have replace semantics (likeel.onclick) with getters, and coexist withaddEventListenerlisteners — both fire on dispatch.useHostStylesbroken: fonts handler clobbers theme/CSS variables)onhostcontextchangedand SDK hooks are mutually exclusive)getHostContext()robust (context merge now happens unconditionally inonEventDispatch, not in a replaceable setter wrapper)How it works
New
ProtocolWithEventsbase class betweenProtocolandApp/AppBridge. Each notification event gets a slot with two independent channels:on*setteraddEventListenerapp.ontoolinput = undefinedremoveEventListener(event, fn)app.ontoolinputreturns handlerDispatch order:
onEventDispatch()(side-effects) →on*handler →addEventListenerlisteners.Other changes
on*properties on bothAppandAppBridgereplaceRequestHandlerfor request-handleron*setters (replace semantics, no double-set throw)registerToolbug fixes: output validation return type,titleintools/list, conditionaloutputSchemaTest plan
on*replace semantics, coexistence withaddEventListener, getters, clearingsetRequestHandler/setNotificationHandler🤖 Generated with Claude Code