Sweetmantech/myc 3643 api mcp mcp server#31
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a new MCP API route at Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Route as app/mcp/route.ts
participant Handler as Paid MCP Handler
participant Tools as Registered Tools
rect rgb(200,220,255)
Note over Client,Route: Initial request (no cached handler)
Client->>Route: GET/POST /mcp
Route->>Route: check cached handler
Route->>Handler: createPaidMcpHandler(serverInfo, context)
Handler->>Tools: register get_random_number
Handler->>Tools: register add
Handler->>Tools: register hello-remote
Handler-->>Route: handler ready (cached)
Route-->>Client: proxy response
end
rect rgb(220,255,220)
Note over Client,Route: Subsequent requests (cached handler)
Client->>Route: GET/POST /mcp
Route->>Route: retrieve cached handler
Route->>Handler: forward request
Handler->>Tools: invoke requested tool
Tools-->>Handler: tool result
Handler-->>Route: response
Route-->>Client: response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
app/mcp/route.ts (2)
34-48: Remove unnecessary optional chaining.The optional chaining operators (
?.) on lines 45-46 are unnecessary if the Zod schema validation works correctly. TheinputSchemaguarantees thatargs.aandargs.bare integers, so they cannot be undefined.Using optional chaining here could mask validation failures (e.g., if
argsis somehow undefined,args?.a + args?.bsilently producesNaNinstead of failing explicitly).Apply this diff to simplify the code:
async args => { - const result = args?.a + args?.b; - return { content: [{ type: "text", text: result?.toString() }] }; + const result = args.a + args.b; + return { content: [{ type: "text", text: result.toString() }] }; },
85-99: Consider adding error handling.The GET and POST handlers correctly delegate to the shared handler instance. However, consider adding error handling to catch and log any initialization or request processing failures.
Example implementation:
export async function GET(req: Request) { try { const handler = await getHandler(); return handler(req); } catch (error) { console.error('MCP GET handler error:', error); return new Response('Internal Server Error', { status: 500 }); } } export async function POST(req: Request) { try { const handler = await getHandler(); return handler(req); } catch (error) { console.error('MCP POST handler error:', error); return new Response('Internal Server Error', { status: 500 }); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
app/mcp/route.ts(1 hunks)package.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/mcp/route.ts (1)
lib/const.ts (1)
SMART_ACCOUNT_ADDRESS(4-4)
⏰ 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: Vercel Agent Review
🔇 Additional comments (2)
app/mcp/route.ts (2)
1-6: LGTM!The imports and singleton handler pattern are appropriate for a Next.js API route. The lazy initialization approach ensures the handler is only created when needed.
49-61: LGTM!The
hello-remotetool implementation is clean and correct. The type assertionas constappropriately narrows the type for the content object.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| }, | ||
| }, | ||
| async (args: { min: number; max: number }) => { | ||
| const randomNumber = Math.floor(Math.random() * (args.max - args.min + 1)) + args.min; |
There was a problem hiding this comment.
| const randomNumber = Math.floor(Math.random() * (args.max - args.min + 1)) + args.min; | |
| const min = Math.min(args.min, args.max); | |
| const max = Math.max(args.min, args.max); | |
| const randomNumber = Math.floor(Math.random() * (max - min + 1)) + min; |
The random number generation function doesn't validate that min is less than or equal to max. When min > max, the formula produces incorrect results instead of either swapping the values or returning an error.
View Details
Analysis
Random number generation function fails to handle min > max
What fails: The registerGetRandomNumberTool() function in lib/mcp/registerGetRandomNumberTool.ts generates incorrect random numbers when min > max, violating the documented contract: "Generates a random number between min and max (inclusive)."
How to reproduce:
// Simulate the original buggy implementation
const min = 5, max = 1;
const randomNumber = Math.floor(Math.random() * (max - min + 1)) + min;
// Math.random() * (1 - 5 + 1) = Math.random() * (-3)
// Results in values between 2-4, never reaching full rangeResult: When calling with min=5, max=1, the function returns values in range [2, 4] instead of generating a number between the min and max values.
Expected: The function should either:
- Swap the parameters internally to handle reversed inputs (implemented fix)
- Return an error for invalid input
Fix implemented: Added normalization to handle any parameter order:
const min = Math.min(args.min, args.max);
const max = Math.max(args.min, args.max);
const randomNumber = Math.floor(Math.random() * (max - min + 1)) + min;This ensures the function generates random numbers in the correct range regardless of parameter order, maintaining the documented contract.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
app/mcp/route.ts (1)
24-27: Consider sourcingserverInfoname/version from configuration instead of hardcoding.Using
"recoup-mcp"and"0.0.1"works, but these values are likely to change over time. Reading them from a central config or environment variables (with sensible defaults) would keep MCP metadata in sync with your actual service name and version without code changes.lib/mcp/registerGetRandomNumberTool.ts (1)
21-26: Validate thatminis not greater thanmaxbefore generating the random number.If a caller passes
min > max,(args.max - args.min + 1)becomes negative and the returned value will be outside the intended range.Add a simple guard before computing the random value:
- async (args: { min: number; max: number }) => { - const randomNumber = Math.floor(Math.random() * (args.max - args.min + 1)) + args.min; + async (args: { min: number; max: number }) => { + if (args.min > args.max) { + throw new Error("min must be less than or equal to max"); + } + const randomNumber = Math.floor(Math.random() * (args.max - args.min + 1)) + args.min; return { content: [{ type: "text", text: randomNumber.toString() }], }; },
🧹 Nitpick comments (1)
lib/mcp/registerAddTool.ts (1)
14-18: Reduce repeated@ts-expect-errorcasts for the input schema.The add tool (and other MCP tools) repeat the same
@ts-expect-errorcast for Zod number schemas. That works, but it scatters a fragile type suppression across multiple files and could hide future type drift between your Zod version andx402-mcp’s expectations.Consider centralizing this into a small helper or alias so the suppression lives in one place, for example:
// e.g. in a shared mcp/zod-compat module // @ts-expect-error - Zod version mismatch with x402-mcp types export const intNumberSchema = z.number().int() as z.ZodType<number>;and then:
inputSchema: { a: intNumberSchema, b: intNumberSchema, }This keeps runtime behavior unchanged while making the type workaround easier to audit and eventually remove.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/mcp/route.ts(1 hunks)lib/mcp/registerAddTool.ts(1 hunks)lib/mcp/registerGetRandomNumberTool.ts(1 hunks)lib/mcp/registerHelloRemoteTool.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/mcp/route.ts (4)
lib/mcp/registerGetRandomNumberTool.ts (1)
registerGetRandomNumberTool(10-28)lib/mcp/registerAddTool.ts (1)
registerAddTool(10-26)lib/mcp/registerHelloRemoteTool.ts (1)
registerHelloRemoteTool(10-24)lib/const.ts (1)
SMART_ACCOUNT_ADDRESS(4-4)
⏰ 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: Vercel Agent Review
🔇 Additional comments (2)
lib/mcp/registerHelloRemoteTool.ts (1)
10-23: LGTM – greeting tool registration is clear and consistent.The schema, handler signature, and returned content structure align with the other MCP tools and look good as-is.
app/mcp/route.ts (1)
15-37: Lazy singleton initialization of the MCP handler looks good.The
getHandlerpattern cleanly ensures tools are registered once and the samecreatePaidMcpHandlerinstance is reused by both GET and POST, which keeps setup overhead low and avoids duplicated registration logic.
1 participant
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.