feat: add enclave mcp package#1390
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Contributor
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a new Changes
Sequence DiagramsequenceDiagram
autonumber
actor Client
participant MCP as MCP Server
participant Fetcher as Doc Fetcher
participant Parser as HTML Parser
participant Transport as StdioTransport
Client->>MCP: read_doc(slug)
activate MCP
MCP->>Fetcher: fetchDocPage(url)
activate Fetcher
Fetcher->>Parser: parse & sanitize HTML
activate Parser
Parser-->>Fetcher: cleaned text/markdown
deactivate Parser
Fetcher-->>MCP: page content
deactivate Fetcher
MCP-->>Client: title + content
deactivate MCP
Client->>MCP: search_docs(query)
activate MCP
MCP->>Fetcher: fetchDocPage (parallel for all pages)
activate Fetcher
Fetcher-->>MCP: page contents or per-page errors
deactivate Fetcher
Note over MCP: case-insensitive search, snippets returned
MCP-->>Client: matching snippets
deactivate MCP
MCP->>Transport: connect()
Transport-->>MCP: stdin/stdout messages
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/bump-versions.ts (1)
385-391:⚠️ Potential issue | 🟡 MinorKeep the dry-run output aligned with the actual bump list.
packages/enclave-mcpis now updated inbumpNpmPackages(), but the--dry-runsummary above still omits it. That makes the release preview incomplete.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/bump-versions.ts` around lines 385 - 391, The dry-run summary is missing 'packages/enclave-mcp' even though bumpNpmPackages() now updates it; update the packagesToBump array used for the dry-run output to include 'packages/enclave-mcp' so the preview matches the real bump list, i.e., locate the packagesToBump constant and add the same entry used by bumpNpmPackages() (ensure both lists are derived from the same source or consolidate into a single source of truth to avoid future drift).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/enclave-mcp/package.json`:
- Around line 1-35: Add an "engines" field to package.json specifying Node
>=18.20.0 to ensure required ESM features (JSON import attributes, global fetch,
top-level await) are available; update the package's README to document the Node
requirement and the CLI runtime expectation. Locate the package.json top-level
object (around "name", "version", "type", "bin") and add "engines": { "node":
">=18.20.0" }, and add a short note in the README mentioning the Node version
requirement for running the enclave-mcp CLI.
In `@packages/enclave-mcp/src/index.ts`:
- Around line 117-145: The search_docs tool currently swallows fetchDocPage
errors and accepts empty queries; update the server.registerTool handler for
'search_docs' to validate and reject empty/whitespace-only queries up front, and
collect page-load errors instead of ignoring them: when iterating DOC_PAGES and
calling fetchDocPage, push failed page identifiers (e.g., page.url or
page.title) into a separate failures array and continue, then after the
Promise.all return a response that includes both results and an explicit
failures summary if any fetches failed (or an error status for widespread
failures); ensure the final no-hit case only returns “No results found” when
there were zero results and zero failures, and surface fetch failures clearly in
the returned content.
- Around line 22-69: The code currently hard-codes DOC_PAGES and scrapes the
rendered DOM in fetchDocPage (using BASE_URL and parse), which is brittle and
lacks a network timeout; replace the static DOC_PAGES with a generated docs
manifest/sitemap (or an API endpoint that returns canonical doc slugs/paths) and
drive your corpus from that manifest instead of the hand-maintained array,
update fetchDocPage to prefer fetching a stable export (raw markdown or a
manifest-provided HTML source) rather than scraping the rendered site, and add a
fetch timeout/AbortController to fetchDocPage so requests cannot hang
indefinitely; keep parse-based DOM fallback only as a last-resort when no
manifest/source is available.
- Line 63: The callback passed to root.querySelectorAll(...).forEach currently
uses an expression-bodied arrow that implicitly returns el.remove(), triggering
the useIterableCallbackReturn rule; change the arrow to a block-bodied callback
so it does not implicitly return (e.g., replace (el) => el.remove() with (el) =>
{ el.remove(); }) ensuring you reference the same root.querySelectorAll and
forEach invocation and the el.remove() call.
---
Outside diff comments:
In `@scripts/bump-versions.ts`:
- Around line 385-391: The dry-run summary is missing 'packages/enclave-mcp'
even though bumpNpmPackages() now updates it; update the packagesToBump array
used for the dry-run output to include 'packages/enclave-mcp' so the preview
matches the real bump list, i.e., locate the packagesToBump constant and add the
same entry used by bumpNpmPackages() (ensure both lists are derived from the
same source or consolidate into a single source of truth to avoid future drift).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ab63c372-7f22-42d5-9992-5a1ce8d7e426
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
package.jsonpackages/enclave-mcp/README.mdpackages/enclave-mcp/package.jsonpackages/enclave-mcp/src/index.tspackages/enclave-mcp/tsconfig.jsonpackages/enclave-mcp/tsup.config.tspnpm-workspace.yamlscripts/build-circuits.tsscripts/bump-versions.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/enclave-mcp/README.md (1)
9-15: Document the MCP resources alongside the tools.The server also exposes
docs://indexand per-pagedocs://<slug>resources, but this README only describes tool calls. Adding a short resources section would make the full client-facing surface discoverable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/enclave-mcp/README.md` around lines 9 - 15, Add a short "Resources" section to the README documenting the MCP's docs:// resources so consumers can discover the server-facing URIs in addition to the tool functions; explicitly mention docs://index for the index and docs://<slug> for per-page access and show that these correspond to the existing tool names (list_docs, read_doc, search_docs) so readers understand the relationship between the tools and the raw resource URIs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/enclave-mcp/src/index.ts`:
- Around line 144-151: The async handler that looks up DOC_PAGES by slug calls
fetchDocPage without handling failures; wrap the fetchDocPage call in a
try/catch inside the async ({ slug }) => handler so that network/parse errors
return a structured MCP error instead of throwing: on catch return { content: [{
type: 'text', text: `Failed to fetch page "${slug}": ${err.message ||
String(err)}` }], isError: true } (preserving the existing invalid-slug
behavior), referencing DOC_PAGES, fetchDocPage and the async ({ slug }) =>
handler to locate where to add the try/catch.
- Around line 90-107: The fetchDocPage function currently fetches every page on
each call; add an in-memory cache (e.g., a Map keyed by fullUrl) at module scope
and check it at the start of fetchDocPage to return cached content when present;
when missing, store the in-flight Promise or resulting content in the cache (to
avoid duplicate concurrent requests), perform the existing fetch/parse/cleanup
logic, then cache and return the cleaned text (use fullUrl as the key and
reference fetchDocPage, fullUrl, and the cached Promise/content in your
implementation).
---
Nitpick comments:
In `@packages/enclave-mcp/README.md`:
- Around line 9-15: Add a short "Resources" section to the README documenting
the MCP's docs:// resources so consumers can discover the server-facing URIs in
addition to the tool functions; explicitly mention docs://index for the index
and docs://<slug> for per-page access and show that these correspond to the
existing tool names (list_docs, read_doc, search_docs) so readers understand the
relationship between the tools and the raw resource URIs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1164db94-bd8b-402f-ad67-0f1e102bf2e0
📒 Files selected for processing (3)
packages/enclave-mcp/README.mdpackages/enclave-mcp/package.jsonpackages/enclave-mcp/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/enclave-mcp/package.json
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.
Closes #1058
Summary by CodeRabbit
New Features
Documentation
Chores