Fix of null tool_calls in the messages #41
Conversation
WalkthroughThe updates introduce improved CORS handling, logging, and message transformation for OpenAI-like API routes, add a middleware for logging all HTTP requests, and enhance robustness in frontend components by using optional chaining. The Dockerfile now installs Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Middleware
participant API Route ([...openai]/route.ts)
participant AI SDK
Client->>Middleware: HTTP Request (GET/POST/OPTIONS)
Middleware->>API Route: Pass request after logging details
alt OPTIONS request
API Route->>Client: Respond with CORS headers (204)
else GET request (models)
API Route->>API Route: Log request, check endpoint
API Route->>Client: Respond with models list + CORS headers
else POST request (chat completions)
API Route->>API Route: Log request, extract body/tools
API Route->>API Route: Transform messages
API Route->>AI SDK: Call with messages and tools
AI SDK-->>API Route: Return completion/stream
API Route->>Client: Respond with completion/stream + CORS headers
end
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/middleware.ts (1)
1-26: LGTM! Comprehensive request logging that addresses the PR objectives.This middleware provides excellent observability for debugging the tools visibility issue. The logging captures all essential request information and the matcher configuration properly excludes static assets.
Consider these production-ready improvements:
export function middleware(request: NextRequest) { + // Only log in development or when debugging is enabled + if (process.env.NODE_ENV === 'development' || process.env.DEBUG_REQUESTS) { console.log("=== INCOMING REQUEST ==="); console.log("Method:", request.method); console.log("URL:", request.url); console.log("Path:", new URL(request.url).pathname); - console.log("Headers:", Object.fromEntries(request.headers)); + // Filter sensitive headers in production + const headers = Object.fromEntries(request.headers); + const { authorization, cookie, ...safeHeaders } = headers; + console.log("Headers:", process.env.NODE_ENV === 'development' ? headers : safeHeaders); console.log("======================="); + } return NextResponse.next(); }src/app/[...openai]/route.ts (1)
21-46: Consider improving type safety and adding validation.The function correctly addresses the PR objective by transforming Cursor's tool messages to be compatible with the AI SDK format. However, consider these improvements:
- Replace
any[]with proper TypeScript interfaces for better type safety- Add validation for required fields in tool messages
- Consider preserving additional tool metadata that might be lost
-function transformCursorMessages(messages: any[]): any[] { +interface CursorMessage { + role: string; + content?: string; + tool_call_id?: string; + name?: string; + tool_calls?: any[]; +} + +function transformCursorMessages(messages: CursorMessage[]): CursorMessage[] { return messages.map((message) => { // Handle tool role messages from Cursor if (message.role === "tool") { + if (!message.tool_call_id) { + console.warn("Tool message missing tool_call_id:", message); + } // Transform tool messages to assistant messages with tool results return { role: "assistant", content: message.content || "", tool_call_id: message.tool_call_id, name: message.name, }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
Dockerfile(2 hunks)package.json(1 hunks)src/app/[...openai]/route.ts(10 hunks)src/app/page.tsx(1 hunks)src/components/LogsList.tsx(2 hunks)src/middleware.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/app/[...openai]/route.ts (1)
src/lib/db.ts (1)
insertLog(4-8)
🔇 Additional comments (6)
package.json (1)
20-28: LGTM! Proper configuration for native dependencies.The
onlyBuiltDependenciesconfiguration ensures that native packages like Prisma, esbuild, and sqlite3 are built from source rather than using potentially incompatible pre-built binaries. This is especially important in containerized environments and aligns well with the Dockerfile changes addingopenssl.src/app/page.tsx (1)
74-74: LGTM! Good defensive programming practice.Adding optional chaining prevents potential runtime errors if
configDatais null or undefined. This improves the robustness of the configuration loading logic.Dockerfile (1)
5-5: LGTM! Essential dependency for AI SDK and secure communications.Adding
opensslis necessary for cryptographic operations, HTTPS requests, and many Node.js packages used in AI applications. This supports the application's connectivity to AI APIs and secure database connections.src/components/LogsList.tsx (1)
45-48: LGTM! Critical robustness improvements that directly address the tools visibility issue.These optional chaining additions prevent component crashes when log metadata or message data is incomplete, which is essential for ensuring tools and messages remain visible even with partial data. The fallback values are well-chosen and maintain a good user experience.
This directly supports the PR objective of fixing tools not appearing in message logs by ensuring the LogsList component remains functional even when tool-related data has missing properties.
Also applies to: 54-54, 76-76, 78-78, 81-81
src/app/[...openai]/route.ts (2)
133-134: Excellent tools parameter handling implementation.The changes correctly address the PR objective by:
- Transforming Cursor messages to AI SDK format using
transformCursorMessages- Conditionally passing tools to both streaming and non-streaming AI SDK calls
- Ensuring consistent use of
modifiedMessagesthroughoutThis should resolve the issue of tools not appearing in message logs.
Also applies to: 171-172, 289-291
226-228: Perfect addition of tool logging for debugging.These console.log statements directly address the PR objective by providing visibility into tool calls and results in both streaming and non-streaming scenarios. This will help debug and confirm that tools are being processed correctly.
Also applies to: 293-295
| console.log("POST request received:", { | ||
| endpoint, | ||
| url: request.url, | ||
| headers: Object.fromEntries(request.headers), | ||
| }); | ||
|
|
||
| if (endpoint !== "chat/completions" && endpoint !== "v1/chat/completions") { | ||
| return NextResponse.json({ error: "Not found", endpoint }, { status: 404 }); | ||
| } | ||
|
|
||
| const body = await request.json(); | ||
| const { messages, model: cursorModel, stream = false, ...otherParams } = body; | ||
| console.log("Request body:", JSON.stringify(body, null, 2)); | ||
| const { messages, model: cursorModel, stream = false, tools, ...otherParams } = body; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Review logging security and consider sensitive data exposure.
The enhanced logging will help debug tool visibility issues, which aligns with the PR objective. However, be cautious about logging sensitive information:
- Headers may contain API keys or authorization tokens
- Request bodies might contain sensitive user data
- Consider implementing log level filtering for production
- console.log("POST request received:", {
- endpoint,
- url: request.url,
- headers: Object.fromEntries(request.headers),
- });
+ console.log("POST request received:", {
+ endpoint,
+ url: request.url,
+ headers: Object.fromEntries(
+ Array.from(request.headers.entries()).filter(
+ ([key]) => !key.toLowerCase().includes('authorization') &&
+ !key.toLowerCase().includes('api-key')
+ )
+ ),
+ });
- console.log("Request body:", JSON.stringify(body, null, 2));
+ console.log("Request body:", JSON.stringify({
+ ...body,
+ // Mask potentially sensitive fields
+ messages: body.messages?.length ? `[${body.messages.length} messages]` : body.messages
+ }, null, 2));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log("POST request received:", { | |
| endpoint, | |
| url: request.url, | |
| headers: Object.fromEntries(request.headers), | |
| }); | |
| if (endpoint !== "chat/completions" && endpoint !== "v1/chat/completions") { | |
| return NextResponse.json({ error: "Not found", endpoint }, { status: 404 }); | |
| } | |
| const body = await request.json(); | |
| const { messages, model: cursorModel, stream = false, ...otherParams } = body; | |
| console.log("Request body:", JSON.stringify(body, null, 2)); | |
| const { messages, model: cursorModel, stream = false, tools, ...otherParams } = body; | |
| console.log("POST request received:", { | |
| endpoint, | |
| url: request.url, | |
| headers: Object.fromEntries( | |
| Array.from(request.headers.entries()).filter( | |
| ([key]) => | |
| !key.toLowerCase().includes('authorization') && | |
| !key.toLowerCase().includes('api-key') | |
| ) | |
| ), | |
| }); | |
| if (endpoint !== "chat/completions" && endpoint !== "v1/chat/completions") { | |
| return NextResponse.json({ error: "Not found", endpoint }, { status: 404 }); | |
| } | |
| const body = await request.json(); | |
| console.log( | |
| "Request body:", | |
| JSON.stringify( | |
| { | |
| ...body, | |
| // Mask potentially sensitive fields | |
| messages: body.messages?.length | |
| ? `[${body.messages.length} messages]` | |
| : body.messages, | |
| }, | |
| null, | |
| 2 | |
| ) | |
| ); | |
| const { messages, model: cursorModel, stream = false, tools, ...otherParams } = body; |
🤖 Prompt for AI Agents
In src/app/[...openai]/route.ts around lines 96 to 108, the current logging
outputs request headers and body which may expose sensitive data like API keys
or user information. To fix this, sanitize or omit sensitive fields such as
authorization headers and any sensitive fields in the request body before
logging. Additionally, implement log level checks to ensure detailed logs are
only emitted in development or debug modes, avoiding sensitive data exposure in
production environments.
| "Access-Control-Allow-Origin": "*", | ||
| "Access-Control-Allow-Methods": "GET, POST, OPTIONS", | ||
| "Access-Control-Allow-Headers": "Content-Type, Authorization, x-api-key, ngrok-skip-browser-warning", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
CORS implementation is comprehensive but consider security implications.
The CORS headers are consistently applied across all response types and the OPTIONS handler properly handles preflight requests. However, consider these security aspects for production:
+const getAllowedOrigins = () => {
+ const origins = process.env.ALLOWED_ORIGINS?.split(',') || ['*'];
+ return origins;
+};
+
+const getCorsHeaders = (origin?: string) => {
+ const allowedOrigins = getAllowedOrigins();
+ const allowOrigin = allowedOrigins.includes('*') ||
+ (origin && allowedOrigins.includes(origin)) ?
+ (origin || '*') : 'null';
+
+ return {
+ "Access-Control-Allow-Origin": allowOrigin,
+ "Access-Control-Allow-Methods": "GET, POST, OPTIONS",
+ "Access-Control-Allow-Headers": "Content-Type, Authorization, x-api-key, ngrok-skip-browser-warning",
+ };
+};Then use getCorsHeaders(request.headers.get('origin')) instead of hardcoded headers.
Also applies to: 317-323, 371-377, 383-390, 515-536
🏁 Script executed:
#!/bin/bash
set -e
# Locate the route.ts file under src/app
route_file=$(find src/app -type f -name 'route.ts')
echo "Route file: $route_file"
# Show all occurrences of CORS headers in the file with surrounding context
echo -e "\n--- CORS Header occurrences ---"
rg -n -C2 'Access-Control-Allow-Origin' "$route_file"
# Show the OPTIONS preflight handler block
echo -e "\n--- OPTIONS handler snippet ---"
rg -n -C3 'method === "OPTIONS"' "$route_file" || echo "No explicit OPTIONS handler found"Length of output: 1001
🏁 Script executed:
#!/bin/bash
set -e
# Locate the openai route.ts file
file=$(find src/app -type f -name 'route.ts' | grep '\[...openai\]')
echo "File: $file"
# Print lines 260-300 for context
echo -e "\n--- Lines 260-300 ---"
sed -n '260,300p' "$file"
# Show all CORS header occurrences in this file
echo -e "\n--- CORS headers occurrences ---"
grep -n 'Access-Control-Allow-Origin' "$file"
# Show OPTIONS handler occurrences in this file
echo -n -e "\n--- OPTIONS handler occurrences ---\n"
grep -n 'OPTIONS' -C2 "$file"Length of output: 4052
Externalize CORS policy and validate origins
The CORS headers are correctly applied across all response paths and the OPTIONS handler, but using “*” for Access-Control-Allow-Origin in production can expose sensitive endpoints. Consolidate and harden your CORS logic:
• File: src/app/[...openai]/route.ts
• Lines: 280–282, 319–323, 372–377, 383–390, 515–536
Suggested refactor:
+// At top of src/app/[...openai]/route.ts
+const getAllowedOrigins = (): string[] =>
+ process.env.ALLOWED_ORIGINS?.split(",") ?? [];
+
+const getCorsHeaders = (origin?: string) => {
+ const allowed = getAllowedOrigins();
+ const allowOrigin = allowed.includes("*")
+ ? "*"
+ : origin && allowed.includes(origin)
+ ? origin
+ : "null";
+
+ return {
+ "Access-Control-Allow-Origin": allowOrigin,
+ "Access-Control-Allow-Methods":
+ process.env.ALLOWED_METHODS ?? "GET,POST,OPTIONS",
+ "Access-Control-Allow-Headers":
+ process.env.ALLOWED_HEADERS ??
+ "Content-Type,Authorization,x-api-key,ngrok-skip-browser-warning",
+ "Access-Control-Max-Age":
+ process.env.CORS_MAX_AGE ?? "86400",
+ };
+};
// …inside each response or OPTIONS handler…
-return new Response(stream, {
- headers: {
- "Content-Type": "text/event-stream",
- "Cache-Control": "no-cache",
- Connection: "keep-alive",
- "Access-Control-Allow-Origin": "*",
- "Access-Control-Allow-Methods": "GET, POST, OPTIONS",
- "Access-Control-Allow-Headers": "Content-Type, Authorization, x-api-key, ngrok-skip-browser-warning",
- },
-});
+return new Response(stream, {
+ headers: {
+ "Content-Type": "text/event-stream",
+ "Cache-Control": "no-cache",
+ Connection: "keep-alive",
+ ...getCorsHeaders(request.headers.get("origin")),
+ },
+});
// Repeat for non-streaming, error handlers, and in OPTIONS(...)This centralizes configuration, validates origins against a whitelist, and ensures consistent use of Access-Control-Max-Age.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "Access-Control-Allow-Origin": "*", | |
| "Access-Control-Allow-Methods": "GET, POST, OPTIONS", | |
| "Access-Control-Allow-Headers": "Content-Type, Authorization, x-api-key, ngrok-skip-browser-warning", | |
| // At the top of src/app/[...openai]/route.ts | |
| const getAllowedOrigins = (): string[] => | |
| process.env.ALLOWED_ORIGINS?.split(",") ?? []; | |
| const getCorsHeaders = (origin?: string) => { | |
| const allowed = getAllowedOrigins(); | |
| const allowOrigin = allowed.includes("*") | |
| ? "*" | |
| : origin && allowed.includes(origin) | |
| ? origin | |
| : "null"; | |
| return { | |
| "Access-Control-Allow-Origin": allowOrigin, | |
| "Access-Control-Allow-Methods": | |
| process.env.ALLOWED_METHODS ?? "GET,POST,OPTIONS", | |
| "Access-Control-Allow-Headers": | |
| process.env.ALLOWED_HEADERS ?? | |
| "Content-Type,Authorization,x-api-key,ngrok-skip-browser-warning", | |
| "Access-Control-Max-Age": | |
| process.env.CORS_MAX_AGE ?? "86400", | |
| }; | |
| }; | |
| // …inside each response or OPTIONS handler… | |
| return new Response(stream, { | |
| headers: { | |
| "Content-Type": "text/event-stream", | |
| "Cache-Control": "no-cache", | |
| Connection: "keep-alive", | |
| ...getCorsHeaders(request.headers.get("origin")), | |
| }, | |
| }); | |
| // Similarly replace the hard-coded CORS headers in your non-streaming responses, | |
| // error handlers, and OPTIONS handler with `...getCorsHeaders(request.headers.get("origin"))`. |
🤖 Prompt for AI Agents
In src/app/[...openai]/route.ts around lines 280-282, 319-323, 372-377, 383-390,
and 515-536, the CORS headers currently use a wildcard "*" for
Access-Control-Allow-Origin, which is insecure for production. Refactor by
externalizing the CORS configuration into a centralized module or constant that
defines a whitelist of allowed origins. Update all CORS header settings to
validate the request origin against this whitelist and only allow matching
origins. Also, ensure consistent use of Access-Control-Max-Age across all CORS
responses for caching preflight results.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/app/[...openai]/route.ts (2)
208-220: Logging implementation creates security risks and duplicates previous concerns.The enhanced logging includes sensitive information that could expose API keys and user data in logs, which was previously flagged in past reviews.
This is the same security concern that was raised in previous reviews about logging headers and request bodies that may contain sensitive information.
412-414: CORS implementation duplicates previous security concerns.The CORS headers are consistently applied but use wildcard origins which was flagged in previous reviews as a security concern for production environments.
This is the same CORS security concern that was raised in previous reviews about using "*" for Access-Control-Allow-Origin in production environments.
Also applies to: 449-455, 503-509, 515-522
🧹 Nitpick comments (1)
src/app/[...openai]/route.ts (1)
107-135: Tool transformation function looks solid but needs better error context.The
transformToolsfunction correctly converts OpenAI-style tool definitions to AI SDK format. The error handling is appropriate, but the error logging could be more informative.} catch (error) { - console.error("Error transforming tools:", error); + console.error("Error transforming tools:", error, "Input tools:", openaiTools); return undefined; }This will help debug tool transformation issues more effectively.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/[...openai]/route.ts(10 hunks)
🔇 Additional comments (6)
src/app/[...openai]/route.ts (6)
137-200: API key validation improvements are well-implemented.The enhanced validation for provider, model, and API keys is a good security improvement. The error messages are clear and the validation logic is consistent across all providers.
249-263: Message validation and transformation logic is comprehensive.The validation ensures messages are properly formatted before processing, and the transformation logging will help debug the tool visibility issues mentioned in the PR objective.
358-360: Tool call logging addresses the PR objective effectively.The detailed logging of
toolCallsandtoolResultsfor both streaming and non-streaming responses directly addresses the PR objective of fixing null tool_calls visibility in message logs.Also applies to: 425-427
481-486: GET request logging is appropriate for debugging.The logging helps track API usage patterns and debug issues, which aligns with the enhanced observability goals.
647-668: OPTIONS handler implementation is correct for CORS preflight.The OPTIONS handler properly handles CORS preflight requests with appropriate headers and status code. The logging helps track preflight requests for debugging.
21-105: Review the message transformation logic for correctness and edge cases.The
transformCursorMessagesfunction handles complex message transformations but has several areas that need attention:
- Tool role handling: The transformation of "tool" role messages to "assistant" role messages may not be semantically correct for all AI providers
- Content array flattening: The logic for handling complex content arrays is comprehensive but could be more robust
- Error handling: While there's basic validation, some edge cases might not be handled properly
Consider these improvements:
function transformCursorMessages(messages: any[]): any[] { if (!Array.isArray(messages)) { - console.warn('Messages is not an array, returning empty array'); + console.error('Messages is not an array, returning empty array'); return []; } return messages.map((message) => { if (!message || typeof message !== 'object') { - console.warn('Invalid message object, skipping:', message); + console.error('Invalid message object, skipping:', message); return null; } // Handle tool role messages from Cursor if (message.role === "tool") { - // Transform tool messages to assistant messages with tool results - // For Anthropic, we should not include tool_call_id or name in the main message - return { - role: "assistant", - content: `Tool response (${message.name || 'unknown'}): ${message.content || ""}`, - }; + // Preserve tool_call_id and name for proper tool result handling + return { + role: "tool", + content: message.content || "", + tool_call_id: message.tool_call_id || message.name || 'unknown', + name: message.name + }; }The current transformation loses important tool context that may be needed for proper tool call handling.
Likely an incorrect or invalid review comment.
| // Transform tools from OpenAI array format to AI SDK Record format | ||
| console.log("Tools parameter received:", tools); | ||
|
|
||
| // Temporarily disable tools for GPT models to debug the core issue | ||
| const validatedTools = provider.toLowerCase() === "openai" ? undefined : transformTools(tools); |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Temporary tool disabling for OpenAI needs clarification.
The comment indicates this is a temporary measure to debug core issues, but this could impact functionality for OpenAI users.
Consider documenting this limitation more clearly or implementing a feature flag instead of hardcoding the disable logic.
🏁 Script executed:
#!/bin/bash
# Search for any related issues or TODOs about OpenAI tool support
rg -i "todo|fixme|hack|temporary" --type ts -A 3 -B 3 | grep -i "openai\|tool"Length of output: 80
Clarify and feature-flag OpenAI tool disabling
The code in src/app/[...openai]/route.ts (locs. ~264–268) unconditionally disables all tools when provider.toLowerCase() === "openai". Since this is marked “temporary” but isn’t documented or toggleable, it could silently break functionality for OpenAI users.
Recommendations:
- Add a clear
// TODO:or JIRA ticket reference next to this block explaining the debugging context and expected removal date. - Replace the hard-coded disable with a feature flag or config (e.g.
ENABLE_OPENAI_TOOLS) so you can re-enable tools without code changes. - Update any user-facing docs or error messages to note that tools are currently disabled for OpenAI.
• File: src/app/[...openai]/route.ts
Lines: ~264–268
🤖 Prompt for AI Agents
In src/app/[...openai]/route.ts around lines 264 to 268, the code disables tools
for OpenAI unconditionally without documentation or a feature flag. Add a clear
TODO comment or JIRA ticket reference explaining this is temporary and when it
should be removed. Replace the hard-coded check with a feature flag (e.g.,
ENABLE_OPENAI_TOOLS) to toggle tool disabling without code changes. Also, update
any relevant user-facing documentation or error messages to indicate that tools
are currently disabled for OpenAI.
1 participant
To fix
Request failed with status code 500: {"error":"Invalid prompt: messages must be an array of CoreMessage"}due to the thinking step.The branch is on the top of #40
Summary by CodeRabbit
New Features
Bug Fixes
Chores