- Notifications
You must be signed in to change notification settings - Fork474
add search, fetch tools to MCP server for better ChatGPT compatibility#917
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:dev
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
@kosmoz is attempting to deploy a commit to theStack Team onVercel. A member of the Team first needs toauthorize it. |
coderabbitaibot commentedSep 25, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughImplements a new exported function Changes
Sequence Diagram(s)sequenceDiagram autonumber actor Client participant Route as Internal Route participant Tools as MCP Tools participant Loader as getDocsById participant FS as FileSystem participant Parser as OpenAPI Parser Client->>Route: call tool "fetch" with { id } Route->>Tools: dispatch fetch Tools->>Loader: getDocsById({ id }) rect rgba(220,235,255,0.35) note over Loader,FS: Multi-path resolution (primary + fallbacks) Loader->>FS: read content/{path} alt Not found / error Loader->>FS: read content/docs/{path} alt Not found / error Loader->>FS: read content/api/{path} end end end alt API page with EnhancedAPIPage Loader->>Parser: extractOpenApiDetails(content, page) alt Parse success Parser-->>Loader: OpenAPI summary Loader-->>Tools: formatted OpenAPI result else Parse fails Loader-->>Tools: formatted fallback API text end else Non-API page Loader-->>Tools: title + description + content end Tools-->>Route: CallToolResult Route-->>Client: response (success or isError)sequenceDiagram autonumber actor Client participant Route participant Tools as MCP Tools participant Index as Page Index Client->>Route: call tool "search" with query Route->>Tools: dispatch search Tools->>Index: match by title/description/content Index-->>Tools: results[] Tools-->>Route: structured results[] Route-->>Client: results[]Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧬 Code graph analysis (1)docs/src/app/api/internal/[transport]/route.ts (1)
🪛 Biome (2.1.2)docs/src/app/api/internal/[transport]/route.ts[error] 227-227: Unexpected empty object pattern. (lint/correctness/noEmptyPattern) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
🔇 Additional comments (5)
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 |
recursemlbot left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Review by RecurseML
🔍 Review performed on6bf092a..b1dd548
| Severity | Location | Issue | Delete |
|---|---|---|---|
| docs/src/app/api/internal/[transport]/route.ts:296 | Single-word API field 'url' technically conforms to both camelCase and snake_case conventions |
| ) | ||
| .map((page)=>({ | ||
| id:page.url, | ||
| title:page.data.title, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
According to the naming convention rule 'naming.mdc', REST API response body fields should use snake_case. In the search tool implementation, the field 'url' is being returned as part of a REST API response. Since this is a direct field in an API response that could be consumed externally, it should follow the snake_case convention.
However, in this case 'url' is a single word without any compound words, so the snake_case and camelCase representations are identical. No change is needed for this specific property, but it's important to maintain snake_case convention for API response fields.
🔍 This comment matches yournaming.mdc rule.
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
Uh oh!
There was an error while loading.Please reload this page.
| // Check if this is an API page and handle OpenAPI spec extraction | ||
| constisApiPage=page.url.startsWith("/api/"); | ||
| // Try primary path first, then fallback to docs/ prefix or api/ prefix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Consider refactoring the duplicate logic for handling API pages in both the primary and alternative file path attempts to reduce code duplication.
Some new issue(s) might be present. Please use the following link(s) to view them: Additionally, the following low severity issue(s) were found: https://zeropath.com/app/issues/2bd9a525-5fa1-49df-9722-9996bf21882f Reply to this PR with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/src/app/api/internal/[transport]/route.ts (1)
248-279:Fix Biome error: Unexpected empty object pattern in handler arg (second occurrence).
Same as above.- async ({}) => {+ async () => {
🧹 Nitpick comments (9)
docs/src/app/api/internal/[transport]/route.ts (9)
7-7:Type import is appropriate. Consider typing helper returns explicitly.
Import looks good. Recommend annotating extractOpenApiDetails to return Promise for consistency with getDocsById.Apply this diff to the function signature at Lines 14-17:
-async function extractOpenApiDetails(+async function extractOpenApiDetails( content: string, page: { data: { title: string, description?: string } },-) {+): Promise<CallToolResult> {
14-96:Harden OpenAPI helper and path handling (optional).
- Consider resolving specFile relative to a known root to avoid brittle relative paths.
- Regex parsing of JSX props is brittle; OK as a stopgap, but document or add TODO to replace with a proper parser.
124-128:Avoid potential unhandled promise rejections from PostHog capture.
Prefix with void (or .catch(() => {})) since posthog-node v4 methods are async.- nodeClient?.capture({+ void nodeClient?.capture({ event: "get_docs_by_id", properties: { id }, distinctId: "mcp-handler", });
130-133:Mark not-found as error and include the requested id.
Improves client handling and observability.- if (!page) {- return { content: [{ type: "text", text: "Page not found." }] };- }+ if (!page) {+ return {+ content: [{ type: "text", text: `Page not found: ${id}` }],+ isError: true,+ };+ }
168-171:Deduplicate fallback paths to avoid content/docs/docs/...
If page.file.path already contains a docs/api prefix, this prevents broken double-prefixed attempts.- const altPaths = [- `content/docs/${page.file.path}`,- `content/api/${page.file.path}`,- ];+ const altPaths = [+ `content/docs/${page.file.path}`,+ `content/api/${page.file.path}`,+ ].filter((p) => p !== filePath);
349-376:Capabilities: add search/fetch looks good; consider aligning existing tool names.
Your newly added capability entries for search/fetch match the tool names. For consistency and discoverability, consider aligning earlier capability keys (listAvailableDocs, getDocById, getStackAuthSetupInstructions) to the actual registered tool names (list_available_docs, get_docs_by_id, get_stack_auth_setup_instructions).If you decide to align, update the keys above accordingly (outside this hunk):
// In capabilities.tools:list_available_docs:{/* ... */},get_docs_by_id:{/* ... */},get_stack_auth_setup_instructions:{/* ... */},Note: Verify @vercel/mcp-adapter and client expectations; some clients may use capabilities to resolve tools by key.
123-219:Minor content formatting consistency.
In several places, description may be undefined; consider using nullish coalescing to avoid “undefined” appearing in output.Example change:
- text: `Title: ${page.data.title}\nDescription: ${page.data.description}\nContent:\n${content}`,+ text: `Title: ${page.data.title}\nDescription: ${page.data.description ?? ''}\nContent:\n${content}`,
9-11:PostHog SDK best practices (non-blocking analytics).
Current usage is fine; optionally add a process-exit hook to flush/shutdown in long-lived processes, or use captureImmediate in serverless. Not blocking for this PR.
281-309:Use structuredContent for JSON results
MCP SDK doesn’t define a “json” content type. WhenCallToolResultsupportsstructuredContent, return your{ results }via astructuredContentfield instead of embedding JSON in a text item; otherwise fall back to the existingcontent: [{ type: 'text', text: JSON.stringify({ results }) }].
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/src/app/api/internal/[transport]/route.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer ES6 Map over Record when representing key–value collections
Files:
docs/src/app/api/internal/[transport]/route.ts
🪛 Biome (2.1.2)
docs/src/app/api/internal/[transport]/route.ts
[error] 227-227: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Security Check
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
CLAassistant commentedSep 25, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/src/app/api/internal/[transport]/route.ts (3)
73-81:Mark OpenAPI spec read failure as an error.Return includes error text but lacks isError:true, making it harder for clients to detect failures.
Apply this diff:
return { content: [ { type: "text" as const, text: errorText, }, ],+ isError: true, };
148-165:Avoid 'undefined' in responses: normalize description field.Some responses interpolate page.data.description without a fallback; template strings will print “undefined”.
Apply this diff:
- text: `Title: ${page.data.title}\nDescription: ${page.data.description}\nContent:\n${content}`,+ text: `Title: ${page.data.title}\nDescription: ${page.data.description ?? ""}\nContent:\n${content}`,- text: `Title: ${page.data.title}\nDescription: ${page.data.description}\nContent:\n${content}`,+ text: `Title: ${page.data.title}\nDescription: ${page.data.description ?? ""}\nContent:\n${content}`,- text: `Title: ${page.data.title}\nDescription: ${page.data.description}\nContent:\n${content}`,+ text: `Title: ${page.data.title}\nDescription: ${page.data.description ?? ""}\nContent:\n${content}`,- text: `Title: ${page.data.title}\nDescription: ${page.data.description}\nContent:\n${content}`,+ text: `Title: ${page.data.title}\nDescription: ${page.data.description ?? ""}\nContent:\n${content}`,- text: `Title: ${page.data.title}\nDescription: ${page.data.description}\nError: Could not read file at any of the attempted paths: ${filePath}, ${altPaths.join(", ")}`,+ text: `Title: ${page.data.title}\nDescription: ${page.data.description ?? ""}\nError: Could not read file at any of the attempted paths: ${filePath}, ${altPaths.join(", ")}`,Also applies to: 183-201, 209-217
112-113:Use strict inequality.Minor style/consistency improvement.
Apply this diff:
- return !(v.slugs[0] == "API-Reference");+ return v.slugs[0] !== "API-Reference";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/src/app/api/internal/[transport]/route.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer ES6 Map over Record when representing key–value collections
Files:
docs/src/app/api/internal/[transport]/route.ts
🪛 Biome (2.1.2)
docs/src/app/api/internal/[transport]/route.ts
[error] 227-227: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Security Check
🔇 Additional comments (4)
docs/src/app/api/internal/[transport]/route.ts (4)
7-7:Type import for CallToolResult is correct.Good: aligns return types for tool handlers.
315-319:Fetch tool delegation looks good.Delegating to getDocsById keeps behavior consistent and reuses logic.
227-227:Fix Biome error: replace empty object pattern parameter.This triggers lint/correctness/noEmptyPattern and will fail CI.
Apply this diff:
- async ({}) => {+ async () => {
288-300:Prevent runtime TypeError in search (optional chaining misuse, missing fields).description and content may be undefined; current chaining still calls .includes on undefined.
Apply this diff for robust, case‑insensitive search:
- async ({ query }) => {- const q = query.toLowerCase();- const results = allPages- .filter(- (page) =>- page.data.title.toLowerCase().includes(q) ||- page.data.description?.toLowerCase().includes(q) ||- page.data.content.toLowerCase().includes(q),- )- .map((page) => ({- id: page.url,- title: page.data.title,- url: page.url,- }));+ async ({ query }) => {+ const q = query.toLowerCase();+ const results = allPages+ .filter((page) => {+ const title = (page.data.title ?? "").toLowerCase();+ const description = (page.data.description ?? "").toLowerCase();+ const content = (page.data.content ?? "").toLowerCase();+ return title.includes(q) || description.includes(q) || content.includes(q);+ })+ .map((page) => ({+ id: page.url,+ title: page.data.title ?? "",+ url: page.url,+ }));Optional (nice-to-have):
- Limit results (e.g.,
.slice(0, 50)) to keep responses small.- Capture telemetry:
nodeClient?.capture({ event: "search", properties: { q }, distinctId: "mcp-handler" });
Thanks so much! Can you review this@madster456 ? |
vercelbot commentedOct 10, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
The latest updates on your projects. Learn more aboutVercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit theCursor dashboard to activate Pro and start your 14-day free trial.
| (page)=> | ||
| page.data.title.toLowerCase().includes(q)|| | ||
| page.data.description?.toLowerCase().includes(q)|| | ||
| page.data.content.toLowerCase().includes(q), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Bug: Search Tool Fails Due to Missing Content
Thesearch tool tries to filter pages usingpage.data.content, butpage.data objects don't include the page's content. Content is loaded separately from files, so attempting to accesspage.data.content will result in a runtime error when.toLowerCase() is called onundefined.
| async({ query})=>{ | ||
| constq=query.toLowerCase(); | ||
| constresults=allPages | ||
| .filter( | ||
| (page)=> | ||
| page.data.title.toLowerCase().includes(q)|| | ||
| page.data.description?.toLowerCase().includes(q)|| | ||
| page.data.content.toLowerCase().includes(q), | ||
| ) | ||
| .map((page)=>({ | ||
| id:page.url, | ||
| title:page.data.title, | ||
| url:page.url, | ||
| })); | ||
| return{ | ||
| content:[ | ||
| { | ||
| type:"text", | ||
| text:JSON.stringify({ results}), | ||
| }, | ||
| ], | ||
| }; | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The newsearch tool is missing PostHog analytics tracking, unlike all other tools in the handler.
View Details
📝 Patch Details
diff --git a/docs/src/app/api/internal/[transport]/route.ts b/docs/src/app/api/internal/[transport]/route.tsindex 07742661..14b10f3a 100644--- a/docs/src/app/api/internal/[transport]/route.ts+++ b/docs/src/app/api/internal/[transport]/route.ts@@ -285,6 +285,11 @@ const handler = createMcpHandler( "Search for Stack Auth documentation pages.\n\nUse this tool to find documentation pages that contain a specific keyword or phrase.", { query: z.string() }, async ({ query }) => {+ nodeClient?.capture({+ event: "search",+ properties: { query },+ distinctId: "mcp-handler",+ }); const q = query.toLowerCase(); const results = allPages .filter(
Analysis
Missing PostHog analytics tracking in search tool
What fails: Thesearch tool indocs/src/app/api/internal/[transport]/route.ts lacks PostHog analytics tracking vianodeClient?.capture(), while all other MCP tools (list_available_docs,get_docs_by_id,get_stack_auth_setup_instructions) include this tracking.
How to reproduce:
- Compare tool implementations in
docs/src/app/api/internal/[transport]/route.ts - Search tool (lines 287-309) missing
nodeClient?.capture()call - Other tools have tracking:
list_available_docs(line 124),getDocsByIdfunction (line 228),get_stack_auth_setup_instructions(line 249)
Result: Inconsistent analytics collection - search tool usage not captured in PostHog metrics
Expected: All MCP tools should have consistent PostHog event tracking for usage analytics per established codebase pattern
Uh oh!
There was an error while loading.Please reload this page.
This PR adds
searchandfetchtools to the MCP server, which should make it compatible with ChatGPT Deep Research (https://platform.openai.com/docs/mcp).The
searchtool makes a naive substring search for all page contents and returns the matching pages.The
fetchtool is identical to the existingget_docs_by_idtool. The tool callback function of theget_docs_by_idtool was extracted in order to make it reusable.High-level PR Summary
This PR adds two new tools (
searchandfetch) to the MCP (Model Context Protocol) server to improve compatibility with ChatGPT Deep Research. Thesearchtool performs a basic substring search across page contents and returns matching pages, while thefetchtool serves as an alias for the existingget_docs_by_idtool. The implementation extracts the callback function fromget_docs_by_idto a separate reusable function that can be used by both tools. The PR also includes the necessary OpenAPI specification updates to document these new tools.⏱️ Estimated Review Time: 15-30 minutes
💡 Review Order Suggestion
docs/src/app/api/internal/[transport]/route.tsImportant
Add
searchandfetchtools to MCP server for enhanced ChatGPT compatibility, withfetchreusingget_docs_by_idfunctionality.searchtool to perform substring search on documentation pages.fetchtool, identical toget_docs_by_id, for retrieving documentation by ID.getDocsByIdfunction for reuse byfetchtool.searchandfetchtools into MCP server handler inroute.ts.This description was created by
forb1dd548. You cancustomize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Improvements