Add custom WebMCP tools - #405
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…p-tools # Conflicts: # server/lib/oapi/oapi.go
masnwilliams
left a comment
masnwilliams
left a comment
There was a problem hiding this comment.
requesting changes on the custom-tool lifecycle before merge. the happy path works in a headful image, but there is a reproducible recovery failure and two structural issues that should be addressed here.
-
failed page registrations are marked current (
server/runtime/custom-webmcp.ts:761-774).installPageDefinitionsreturns per-tool errors, but reconciliation commitsdocumentKeyandsignatureregardless; the next pass exits asregistrationCurrent(:735-737). i reproduced this by makingdocument.modelContext.registerToolfail once, then restoring it: add returned 201, but the tool remained undiscoverable after the failure cleared. removing and re-adding it did register. only commit successful installs; retry/report failed registrations, and add a same-document recovery test. -
the new
custom-webmcp.tsis 1,048 lines and owns validation/matching, registry transactions, CDP page/OOPIF tracking, page-runtime source generation, and binding RPC. please decompose this around explicit owners (definitions, browser reconciliation, page runtime/transport) rather than making one registry coordinate all five lifecycles. -
custom metadata is independently held by the Node registry, a published state file, and a Go cache (
browser_repl.go:100-153). on file read/decode failure the API silently serves cached metadata, even if a definition was replaced or removed. please make the discovery snapshot authoritative or define and test an intentional stale-read policy, and collapse the redundant generation/revision bookkeeping where possible.
non-blocking follow-up: webmcp.invokeTool in the REPL now calls full listTools() before even native-tool invocations (browser-repl.ts:339-345). that adds target refresh/settling and a new failure dependency to the existing path; avoid the preflight for native tools when possible.
focused Go and TypeScript tests pass, and HTTP/REPL custom invocation, URL reconciliation, and native discovery/invocation passed in the built headful image. those successes do not cover the one-time install failure above.
|
bugbot run |
masnwilliams
left a comment
masnwilliams
left a comment
There was a problem hiding this comment.
re-reviewed at 26f56c9. the three change requests from my previous review are addressed: failed page registrations now retry on the same document with an e2e regression test; definition ownership, browser reconciliation, and page runtime/transport are split into focused modules; and discovery reads one authoritative atomic snapshot rather than maintaining a second Go metadata cache. the native REPL invocation path also avoids full discovery for known native refs. focused Go/TypeScript tests and typecheck pass locally; CI and BugBot are green on this commit. approving.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5e28c7b. Configure here.
2 participants
Summary
POST /webmcp/custom-toolsand the Browser REPL, with per-tool IDs, conflict detection, listing, removal, and atomic namespace replacementGET /webmcp/toolswith nested MCP metadata andsource.customprovenance; invoke them with the existingtool_refthrough/webmcp/invokeorwebmcp.invokeToolQA and validation
API compatibility
Discovery consumers must read
tool.name,tool.description,tool.inputSchema, andtool.annotations; legacy flat fields are not returned. Custom sources includetarget_idfor the registration tab. CDP-backed custom outputs over 240 KiB return an explicit error. Coordinate discovery consumer updates before release.