Merge test to main #140
Merged
Merge test to main #140
+178
−75
Conversation
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
Logs Privy JWT and API key validation attempts to help debug authentication failures in tool calls. Co-Authored-By: Claude Opus 4.5 <[email protected]>
This reverts commit ee1700a.
- Add CORS headers to createUIMessageStreamResponse call in handleChatStream - Add x-api-key to Access-Control-Allow-Headers in getCorsHeaders - Update handleChatStream test to expect CORS headers Fixes cross-origin requests to /api/chat endpoint being blocked. Co-authored-by: Claude Opus 4.5 <[email protected]>
* refactor: Use HTTP transport for MCP to properly pass authInfo Replace InMemoryTransport with StreamableHTTPClientTransport to make HTTP requests to /api/mcp endpoint with forwarded auth token. This ensures MCP tool handlers receive proper authInfo through the established withMcpAuth middleware flow. - Add authToken to ChatRequestBody type - Extract auth token from request headers in validateChatRequest - Use HTTP transport to /api/mcp with Authorization header - Update tests to include authToken in request bodies Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix: Make authToken optional for internal flows Internal flows like email processing don't have an auth token from HTTP headers. Make authToken optional and skip MCP tools when not present. Co-Authored-By: Claude Opus 4.5 <[email protected]> * refactor: Extract getBaseUrl to lib/networking Move getBaseUrl utility to its own file following SRP. Co-Authored-By: Claude Opus 4.5 <[email protected]> * refactor: Extract getMcpTools to lib/mcp Move MCP tools fetching logic to its own file following SRP. Co-Authored-By: Claude Opus 4.5 <[email protected]> * test: Add unit tests for getBaseUrl and getMcpTools Co-Authored-By: Claude Opus 4.5 <[email protected]> * refactor: Use AI SDK built-in SSE transport for MCP Replace StreamableHTTPClientTransport with AI SDK's built-in transport config which provides a simpler API: transport: { type: "sse", url: "...", headers: { Authorization: "..." } } This should fix the "Streamable HTTP error" issue. Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix: Correct MCP endpoint URL to /mcp The MCP route is at app/mcp/route.ts, not app/api/mcp/route.ts. Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix: Use HTTP transport instead of SSE for MCP The mcp-handler server only supports HTTP transport, not SSE. Co-Authored-By: Claude Opus 4.5 <[email protected]> --------- Co-authored-by: Claude Opus 4.5 <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThis PR refactors the MCP tool retrieval mechanism from in-process to HTTP-based, adds optional authentication token support throughout the chat request pipeline, and enhances streaming responses with CORS headers. The changes include new utility functions for base URL resolution and updated CORS header handling. Changes
Sequence DiagramsequenceDiagram
participant Client
participant ChatHandler as Chat Request Handler
participant Validator as Validator
participant MCP as MCP Service (HTTP)
participant Sheets as Google Sheets Service
participant Aggregator as Tool Aggregator
Client->>ChatHandler: POST /chat with request body
ChatHandler->>Validator: validateChatRequest(body)
Validator->>Validator: Extract authToken from headers
Validator-->>ChatHandler: Return ChatRequestBody + authToken
ChatHandler->>Aggregator: setupToolsForRequest(body + authToken)
alt authToken present
Aggregator->>MCP: getMcpTools(authToken)
MCP-->>Aggregator: ToolSet from HTTP endpoint
else authToken absent
Aggregator->>Aggregator: Skip MCP tools (empty set)
end
Aggregator->>Sheets: getGoogleSheetsTools()
Sheets-->>Aggregator: Google Sheets ToolSet
Aggregator->>Aggregator: Merge tools (Sheets takes precedence)
Aggregator-->>ChatHandler: Combined ToolSet
ChatHandler->>ChatHandler: Create streaming response
ChatHandler->>ChatHandler: Add CORS headers via getCorsHeaders()
ChatHandler-->>Client: UIMessageStream with CORS headers
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ Finishing touches
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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Merge test branch changes to main
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.