Repository navigation
WebMCP: publish full skill-lib catalogue and invoke live MCP tools - #87
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: b23b73b73a
ℹ️ 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".
| setText(toolsStatus, `Calling ${tool.name}…`, 'hmmm'); | ||
| try { | ||
| const result = await invokeMcpTool(boundEndpoint, tool.name, args, { token: tokenInput?.value || '' }); | ||
| if (requestRevision !== revision) return; |
There was a problem hiding this comment.
If the repository, skill, outcome, endpoint, or credential changes while a tools/call request is in flight, the associated listeners increment revision, so this early return discards the executor's eventual success or failure. An effectful call may already have changed the remote repository, leaving the user without its only receipt and potentially prompting a duplicate retry; retain the response with the invocation's original context rather than silently dropping it.
Useful? React with 👍 / 👎.
| if (body.length > MAX_ARGUMENT_CHARS) throw new Error('MCP arguments exceed the request limit'); | ||
|
|
||
| let response; | ||
| try { |
There was a problem hiding this comment.
When an endpoint accepts the connection but never completes the response or stalls while sending its body, this fetch has no timeout unless an external caller supplies a signal, and the page binding supplies none. The surrounding busy state therefore never clears, leaving discovery and invocation controls disabled even after the endpoint is edited; create a bounded abort signal for browser requests.
Useful? React with 👍 / 👎.
| } | ||
| const size = Number(response.headers.get('content-length') || 0); |
There was a problem hiding this comment.
For a chunked response or any response without a trustworthy Content-Length, response.text() buffers the entire payload before the character limit is checked. A misconfigured or compromised endpoint can therefore consume unbounded browser memory despite MAX_RESPONSE_CHARS; stream the body and abort as soon as the limit is exceeded.
Useful? React with 👍 / 👎.
| export async function discoverMcpTools(endpoint, options = {}) { | ||
| const result = await mcpRequest(endpoint, 'tools/list', {}, options); | ||
| if (!Array.isArray(result.tools)) throw new Error('MCP tools/list returned no tool catalogue'); | ||
| const names = new Set(); |
There was a problem hiding this comment.
When tools/list returns a nextCursor, this reads only the initial page and silently ignores every later tool. An executor located on a later page is consequently absent from the selector and leaves skill enactment disabled even though the endpoint advertises it; the official MCP SDK pagination documentation specifies that clients follow nextCursor until it is omitted.
Useful? React with 👍 / 👎.
| }); | ||
| } catch { | ||
| throw new Error('MCP request failed: check endpoint availability, HTTPS, CORS, and network access'); | ||
| } |
There was a problem hiding this comment.
For a 2026-era request, a conforming server can return HTTP 400 with a matching JSON-RPC error body, such as a header-mismatch or missing-capability error; the official TypeScript SDK documentation treats these as in-band protocol errors. Rejecting on response.ok before reading that body replaces the actionable MCP error with only MCP HTTP 400, so the page cannot display the endpoint's actual failure as promised.
Useful? React with 👍 / 👎.
| setResult(`hmmm: Invalid tool arguments: ${error.message}`); | ||
| return; | ||
| } | ||
| await execute(tool, args); |
There was a problem hiding this comment.
When the discovered dropdown contains tiw_execute_skill, the generic Call button reaches this path with arbitrary hand-entered arguments, bypassing the dedicated enactment checks for a repository head, selected skill, outcome, and canonical envelope. It also skips the forced confirmation when the server advertises readOnlyHint: true, so the same execution tool that the guarded button treats as effectful can be invoked without those human-in-loop protections; exclude this tool from the generic path or route it through the guarded enactment flow.
Useful? React with 👍 / 👎.
FROM → TO
From a curated WebMCP subset and five registry-only MCP tools → to the entire verified skill-lib catalogue (all 45 currently registered skills) plus explicit browser-side Streamable HTTP tool discovery and invocation.
Changes
Enact selected skill via MCPonly when the connected endpoint advertisestiw_execute_skill; require repository head, selected skill, human outcome and explicit approval. A server reply is not reported as a verified completed job.Contract
DONE WHEN: every skill in the source registry can be inspected in the human page and read-only tool registry; a user can invoke an advertised MCP tool with actual response and explicit failure; no operational execution is implied without the advertised
tiw_execute_skilltool.PROVEN BY:
npm run test:mcp, WebMCP tests, site build gates, builder-history gate, and explicit loopback tool invocation.FAILS IF: any skills are hidden, write privileges are granted by selection, credentials persist, a fabricated runner result is shown, or an unavailable executor is represented as working.
hmmm: The deployed public endpoint provides no executor. Connecting and verifying an independently authorized skill runner remains a separate operational dependency; no public-site changes should install VM-shell authority in the registry server.