Add WebMCP page#182
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (4)
📝 WalkthroughWalkthroughThis PR adds a reusable MCP contract, refactors the MCP CLI to use it, and implements a WebMCP browser page (with entry script, bootstrap logic, styling, tests, and docs/build wiring) that registers MCP tools at runtime via document.modelContext. ChangesMCP Contract Foundation & CLI Refactoring
WebMCP Browser Interface
Sequence DiagramsequenceDiagram
participant Browser as Browser
participant Entry as webmcp-entry.mjs
participant Page as bootstrapWebMcpPage
participant Contract as anywaydata-mcp-contract
participant Model as document.modelContext
Browser->>Entry: load /webmcp.html
Entry->>Entry: initializeWebMCPPolyfill()
Entry->>Page: import & invoke bootstrapWebMcpPage
Page->>Page: findModelContext(document)
Page->>Contract: listAnyWayDataMcpTools()
Contract-->>Page: tool list
Page->>Page: render tool cards
alt modelContext available
Page->>Model: registerTool for each
Model->>Page: invoke registered handler
Page->>Contract: executeAnyWayDataMcpTool(name, args)
Contract-->>Page: tool result
Page->>Page: setStatus success
else modelContext unavailable
Page->>Page: setStatus warning
end
Page-->>Browser: return destroy()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 103ea030b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Greptile SummaryThis PR introduces a dedicated
Confidence Score: 5/5Safe to merge; all changed paths are additive and the one behavioural change to the options schema is intentional and covered by updated tests. The refactoring correctly extracts shared logic into the new contract module, the Node MCP server path is functionally equivalent to before as verified by the updated mcp.test.js suite, and the new WebMCP page handles all lifecycle and error states including the previously-flagged status-stuck-loading gap. The two maintenance concerns flagged are about future extensibility, not current behaviour. packages/core/js/mcp/anywaydata-mcp-contract.js — the split between the tool registry array and the hardcoded execution switch is worth watching as new tools are added. Important Files Changed
Reviews (9): Last reviewed commit: "Remove external Dockerfile frontend depe..." | Re-trigger Greptile |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a dedicated browser-hosted WebMCP page (webmcp.html) and refactors the existing Node MCP server to share a single MCP tool/resource contract with the browser page, improving consistency and discoverability across both surfaces.
Changes:
- Added
apps/web/webmcp.htmlplus a small JS bootstrap to register AnyWayData MCP tools via WebMCP whenmodelContext.registerToolis available. - Introduced a shared MCP contract module in
@anywaydata/coreand updated the Node MCP server to use it (instead of duplicating tool/resource definitions). - Updated navigation + docs to link to the new WebMCP page, and added Jest tests for both the contract and the page bootstrap.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents the new webmcp.html page for local + docker usage. |
| pnpm-lock.yaml | Locks new dependencies introduced for WebMCP support and shared contract usage. |
| packages/core/src/tests/mcp/anywaydata-mcp-contract.test.js | Adds unit tests to validate the shared MCP contract surface. |
| packages/core/package.json | Exposes the new ./mcp/* export path for consumers. |
| packages/core/js/mcp/anywaydata-mcp-contract.js | New shared MCP tool/resource contract used by both browser WebMCP and Node MCP server. |
| package.json | Adds workspace + WebMCP dependencies and Jest path mapping for the new @anywaydata/core/mcp/* import path. |
| docs-src/docs/070-interfaces-and-deployment/020-web-ui.md | Adds WebMCP link + local/docker URLs. |
| apps/web/webmcp.html | New WebMCP page with human-readable guidance and placeholders for tool/resource lists. |
| apps/web/vite.config.mjs | Adds webmcp.html as a build entry and adds explicit aliases for @anywaydata/core/* subpaths. |
| apps/web/styles.css | Adds WebMCP-specific layout and card styling. |
| apps/web/src/webmcp-page.mjs | Implements page bootstrap: renders tools/resources, reads install guide, registers tools into WebMCP. |
| apps/web/src/webmcp-entry.mjs | Loads the WebMCP global/polyfill and bootstraps the page module. |
| apps/web/src/tests/jest/webmcp-page.test.js | Adds Jest coverage for rendering + tool registration + WebMCP response shape. |
| apps/web/index.html | Adds WebMCP nav + landing page link. |
| apps/web/generator.html | Adds WebMCP nav link. |
| apps/web/combinatorial.html | Adds WebMCP nav link. |
| apps/web/app.html | Adds WebMCP nav link. |
| apps/mcp/src/index.js | Replaces duplicated MCP tool/resource implementation with shared contract calls. |
| apps/anywaydata/README.md | Documents the new webmcp.html entry point in the app README. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@apps/web/src/webmcp-page.mjs`:
- Around line 201-205: The code currently calls registerTools(...) and if it
throws the page stays in a loading state and themeToggle is not cleaned up; wrap
the await registerTools({ modelContext, tools }) call in a try/catch (or
try/catch/finally), on error call setStatus(statusElement, `Failed to register
AnyWayData tools: ${error.message || error}`, { isLoading: false }) and clean up
the themeToggle (remove the element or call its teardown method) inside the
catch, and ensure setStatus is called with isLoading: false in the finally so
the page never remains stuck in the "Registering…" state; reference setStatus,
registerTools, statusElement, and themeToggle to locate the fixes.
In `@packages/core/js/mcp/anywaydata-mcp-contract.js`:
- Around line 376-380: The example config object codex_local_repo contains a
machine-specific cwd value ('D:/github/grid-table-editor'); remove this
hardcoded Windows path and replace it with a portable alternative (either omit
the cwd field so the default working directory is used, use a relative path, or
replace with a clearly marked placeholder like '<path-to-repo>' or
process.cwd()). Update the codex_local_repo entry (command, args, cwd) to avoid
any absolute, OS-specific path and ensure the sample is platform-agnostic and
copy-paste safe.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1577e75b-462e-415a-a2a9-d4e8b93a3cd8
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
README.mdapps/anywaydata/README.mdapps/mcp/src/index.jsapps/web/app.htmlapps/web/combinatorial.htmlapps/web/generator.htmlapps/web/index.htmlapps/web/src/tests/browser/app/abstractions/components/test-data-panel.component.jsapps/web/src/tests/jest/webmcp-page.test.jsapps/web/src/webmcp-entry.mjsapps/web/src/webmcp-page.mjsapps/web/styles.cssapps/web/vite.config.mjsapps/web/webmcp.htmldocs-src/docs/070-interfaces-and-deployment/020-web-ui.mdpackage.jsonpackages/core/js/mcp/anywaydata-mcp-contract.jspackages/core/package.jsonpackages/core/src/tests/mcp/anywaydata-mcp-contract.test.js
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/web/src/webmcp-page.mjs (1)
187-199:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
themeTogglecleanup in the error handler.The catch block (lines 193-199) sets the error status and rethrows, but does not destroy the
themeTogglecomponent created at lines 167-170. This leaves event listeners and DOM elements orphaned when registration fails.🔧 Add cleanup in the catch block
} catch (error) { const message = error instanceof Error ? error.message : String(error); setStatus(statusElement, `Unable to register AnyWayData WebMCP tools: ${message}`, { severity: 'error', }); + themeToggle?.destroy?.(); throw error; }🤖 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 `@apps/web/src/webmcp-page.mjs` around lines 187 - 199, The catch block around registerTools does not clean up the themeToggle created earlier (themeToggle variable), leaving listeners/DOM orphaned; update the catch to check if themeToggle is defined and call its cleanup method (e.g., themeToggle.destroy() or themeToggle.remove()/unmount() depending on the component API), or if no such method exists remove its DOM node and unregister its event listeners before calling setStatus and rethrowing; ensure you reference the themeToggle identifier and perform the cleanup prior to setting the error status and throwing the error.
🤖 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.
Duplicate comments:
In `@apps/web/src/webmcp-page.mjs`:
- Around line 187-199: The catch block around registerTools does not clean up
the themeToggle created earlier (themeToggle variable), leaving listeners/DOM
orphaned; update the catch to check if themeToggle is defined and call its
cleanup method (e.g., themeToggle.destroy() or themeToggle.remove()/unmount()
depending on the component API), or if no such method exists remove its DOM node
and unregister its event listeners before calling setStatus and rethrowing;
ensure you reference the themeToggle identifier and perform the cleanup prior to
setting the error status and throwing the error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a3d7869b-3f2d-489d-b384-3287a23d5054
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
apps/mcp/src/mcp.test.jsapps/web/src/tests/jest/webmcp-page.test.jsapps/web/src/webmcp-page.mjspackage.jsonpackages/core/js/mcp/anywaydata-mcp-contract.jspackages/core/src/tests/mcp/anywaydata-mcp-contract.test.js
💤 Files with no reviewable changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/tests/mcp/anywaydata-mcp-contract.test.js
- packages/core/js/mcp/anywaydata-mcp-contract.js
Owner
Author
|
still seems too experimental so moving it to test environment only. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/tests/jest/webmcp-page.test.js (1)
32-40: ⚡ Quick winExercise the real
document.modelContextdiscovery path in this test.The test title says “through document.modelContext”, but Line 38 injects
modelContextdirectly, sofindModelContext(documentObj)is not covered.Suggested minimal update
test('renders cards and registers tools through document.modelContext', async () => { const modelContext = createModelContextDouble(); + dom.window.document.modelContext = modelContext; const createThemeToggleComponentFn = jest.fn(() => ({ destroy: jest.fn() })); const page = await bootstrapWebMcpPage({ documentObj: dom.window.document, - modelContext, createThemeToggleComponentFn, });🤖 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 `@apps/web/src/tests/jest/webmcp-page.test.js` around lines 32 - 40, The test currently passes modelContext directly to bootstrapWebMcpPage which bypasses the findModelContext(documentObj) path; update the test to instead attach the model context to the DOM (e.g., set documentObj.modelContext = createModelContextDouble()) and remove the modelContext argument so bootstrapWebMcpPage exercises the real document.modelContext discovery path (ensure any helper mocks like createThemeToggleComponentFn remain passed as before and the test still awaits bootstrapWebMcpPage).
🤖 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.
Nitpick comments:
In `@apps/web/src/tests/jest/webmcp-page.test.js`:
- Around line 32-40: The test currently passes modelContext directly to
bootstrapWebMcpPage which bypasses the findModelContext(documentObj) path;
update the test to instead attach the model context to the DOM (e.g., set
documentObj.modelContext = createModelContextDouble()) and remove the
modelContext argument so bootstrapWebMcpPage exercises the real
document.modelContext discovery path (ensure any helper mocks like
createThemeToggleComponentFn remain passed as before and the test still awaits
bootstrapWebMcpPage).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 368a246e-105f-4155-a0eb-89daa2a9a1f6
📒 Files selected for processing (10)
README.mdapps/mcp/src/mcp.test.jsapps/web/src/tests/jest/webmcp-page.test.jsapps/web/src/webmcp-entry.mjsapps/web/src/webmcp-page.mjsapps/web/styles.cssapps/web/webmcp.htmldocs-src/docs/070-interfaces-and-deployment/020-web-ui.mddocs-src/docs/070-interfaces-and-deployment/040-mcp.mdpackages/core/js/mcp/anywaydata-mcp-contract.js
💤 Files with no reviewable changes (1)
- docs-src/docs/070-interfaces-and-deployment/020-web-ui.md
✅ Files skipped from review due to trivial changes (3)
- docs-src/docs/070-interfaces-and-deployment/040-mcp.md
- README.md
- apps/web/styles.css
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/src/webmcp-entry.mjs
- apps/mcp/src/mcp.test.js
- packages/core/js/mcp/anywaydata-mcp-contract.js
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
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
webmcp.htmlpage for browser-hosted WebMCP accessCloses #172.
Verification
pnpm run verify:uipnpm run verify:localSummary by CodeRabbit
New Features
Documentation
Style
Tests