Dev#7
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.
Reviewed commit: 751ff86624
ℹ️ 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".
| "/chat-knowledge/records/:id", | ||
| validate({ params: z.object({ id: idSchema }) }), | ||
| asyncHandler(async (req: Request) => { | ||
| const record = await getKnowledgeRecordById(String(req.params.id)); |
There was a problem hiding this comment.
This handler fetches a knowledge record by raw ID only, so in any deployment that uses multiple ownerId scopes, a caller who learns another tenant's record ID can read that record across owner boundaries. Other chat-knowledge endpoints derive owner scope via getRequestOwnerId, so this endpoint should enforce the same owner filter before returning data.
Useful? React with 👍 / 👎.
| chatId: idSchema, | ||
| branchId: branchIdSchema, | ||
| collectionId: idSchema.optional(), | ||
| includeArchived: z.coerce.boolean().optional(), |
There was a problem hiding this comment.
Using z.coerce.boolean() on query params makes any non-empty string truthy, so a request like ?includeArchived=false is parsed as true and archived records are incorrectly included. This breaks filtering semantics for normal HTTP clients that send booleans as strings; parse "true"/"false" explicitly instead of generic coercion.
Useful? React with 👍 / 👎.
| target: [ | ||
| knowledgeRecordAccessState.chatId, | ||
| knowledgeRecordAccessState.branchId, | ||
| knowledgeRecordAccessState.recordId, |
There was a problem hiding this comment.
This upsert conflict target includes branchId, but chat-scope writes pass branchId = null; in SQLite, NULL values do not collide in unique constraints, so these "upserts" insert new rows instead of updating existing access state. Over time this creates duplicate access-state rows for the same record and makes reads with limit(1) potentially return stale state.
Useful? React with 👍 / 👎.
1 participant
No description provided.