Conversations page#37
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| conversations: t.prismaField({ | ||
| type: ['Conversation'], | ||
| args: { | ||
| limit: t.arg.int({ defaultValue: 20 }), | ||
| offset: t.arg.int({ defaultValue: 0 }), | ||
| }, | ||
| resolve: async (query, _root, args, ctx: Context) => { | ||
| const prisma = ctx.prisma as any; | ||
|
|
||
| // Input validation to prevent abuse | ||
| const limit = Math.min(Math.max(args.limit || 1, 1), 100); // Clamp between 1 and 100 | ||
| const offset = Math.max(args.offset || 0, 0); // Ensure non-negative | ||
|
|
||
| return prisma.conversation.findMany({ | ||
| ...query, | ||
| orderBy: { | ||
| updatedAt: 'desc', | ||
| }, | ||
| take: limit, | ||
| skip: offset, | ||
| }); |
There was a problem hiding this comment.
The new conversations resolver returns prisma.conversation.findMany with no where clause. Every authenticated request will therefore receive the complete conversations table, regardless of the user or workspace making the call, unlike the conversation field which delegates to conversationService for scoping. In a multi-tenant app this leaks other users’ conversations and bypasses any permission filtering done in the service layer. The resolver should include the current user/workspace from ctx or invoke the service so only allowed conversations are returned.
Useful? React with 👍 / 👎.
| // Propagate workspace header if present in cookies | ||
| const res = NextResponse.next(); | ||
| const workspaceId = req.cookies.get('workspaceId')?.value; | ||
| if (workspaceId) { | ||
| res.headers.set('x-workspace-id', workspaceId); | ||
| } | ||
|
|
||
| return res; |
There was a problem hiding this comment.
The middleware attempts to propagate a workspaceId cookie via res.headers.set('x-workspace-id', ...), but this mutates only the outgoing response. Downstream routes and server components read headers from the request object (headers() in makeServerGql), so they will never see x-workspace-id. As a result GraphQL calls cannot determine the active workspace and will default to whatever the API chooses, potentially mixing data across tenants. Use NextResponse.next({ request: { headers } }) to add the header to the request before continuing.
Useful? React with 👍 / 👎.
|
@codex review please |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export async function POST(request: NextRequest) { | ||
| try { | ||
| const apiPort = process.env.API_PORT || '4001'; | ||
| const apiUrl = `http://localhost:${apiPort}/chat`; | ||
|
|
There was a problem hiding this comment.
POST requests to /api/chat always forward to http://localhost:${process.env.API_PORT || 4001}/chat, but the API server now selects an available port via portfinder and writes the actual port to .api-port when 4001 is occupied (apps/api/server.ts lines 382-404). In that scenario this route keeps calling 4001, so chat streaming fails with connection errors despite the discovery file being present. Please read the port from the discovery file or reuse the same resolver as other GraphQL calls so chat requests reach the running API instance.
Useful? React with 👍 / 👎.
| try { | ||
| const apiPort = process.env.API_PORT || '4001'; | ||
| const apiUrl = `http://localhost:${apiPort}/chat/${conversationId}`; | ||
|
|
There was a problem hiding this comment.
The server-side getConversationMessages helper still hardcodes the API host to localhost:${process.env.API_PORT || '4001'}. When the API server falls back to another port (portfinder sets this and writes it to .api-port), this call continues to hit 4001 and returns empty/error, causing conversation pages to render without messages even though the API is available on a different port. Align this lookup with the new port discovery mechanism used elsewhere so the fetch targets the actual running API.
Useful? React with 👍 / 👎.
|
@codex review please |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const messages = | ||
| await container.messageService.getConversationMessages( | ||
| conversationId, | ||
| threadId, | ||
| { limit, offset } |
There was a problem hiding this comment.
The new /chat/:conversationId handler limits the result to limit/offset (default 50/0) and forwards those values to getConversationMessages (getConversationMessages orders by createdAt: 'asc'). Because the UI doesn’t paginate, any conversation with more than 50 messages will only ever receive the first 50 oldest messages and the latest conversation history is inaccessible when viewing an existing conversation or resuming a chat. Consider either removing the cap for this view or fetching the most recent page instead of the oldest.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
https://github.com/carterax/carterax.pm/blob/992be5666a27f91f0ce76eac90c5b381d654a3db/apps/api/src/modules/conversation/mutation.ts#L170-L173
The analyzeConversation resolver always sets includeTicketsInSpec to true before queuing the outbox event because it uses args.includeTicketsInSpec || true. Any client explicitly passing includeTicketsInSpec: false will still generate specs with tickets, defeating the API flag. The fallback should respect a false value (e.g., use nullish coalescing) so callers can opt out.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| async rewrites() { | ||
| return [ | ||
| { | ||
| source: '/api/graphql', | ||
| destination: apiUrl, |
There was a problem hiding this comment.
The Next.js rewrite hardcodes /api/graphql to a local apiUrl derived from .api-port/default localhost:4001, and that port file is only written in development. In production builds this still points to http://localhost:4001 and ignores API_URL, so browser GraphQL calls will be proxied to the wrong host whenever the API runs on a different port or domain. The rewrite should honor the production API endpoint rather than a dev-only port file.
Useful? React with 👍 / 👎.
|
Summary
Testing
|
2 participants
No description provided.