Open
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a DELETE /api/chats endpoint with request-body validation and centralized access checks; implements a delete handler that performs cascade deletion via a Supabase helper which removes related records before deleting the room. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as "API\nDELETE /api/chats"
participant Handler as "deleteChatHandler"
participant Validator as "validateDeleteChatBody / validateChatAccess"
participant DB as "deleteRoomWithRelations (Supabase)"
Client->>API: DELETE /api/chats { id }
API->>Handler: forward request
Handler->>Validator: validate body & access
Validator->>Validator: parse JSON, Zod validate, auth checks
alt validation fails
Validator-->>Handler: NextResponse (4xx)
Handler-->>Client: 4xx response with CORS headers
else validation succeeds
Validator-->>Handler: { id }
Handler->>DB: deleteRoomWithRelations(id)
DB->>DB: delete memory_emails → delete memories → delete room_reports → delete segment_rooms → delete rooms
alt DB delete fails
DB-->>Handler: false
Handler-->>Client: 500 { status: "error", error: "Failed to delete chat" }
else DB delete succeeds
DB-->>Handler: true
Handler-->>Client: 200 { status: "success", id, message: "Chat deleted successfully" }
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 (1)
lib/chats/deleteChatHandler.ts (1)
39-44: Consider sanitizing error messages in production.Exposing
error.messagedirectly in the response could leak internal implementation details or sensitive information. While useful for debugging, you may want to log the actual error server-side and return a generic message to clients.♻️ Optional: Sanitize error response
} catch (error) { - const message = error instanceof Error ? error.message : "Server error"; + if (error instanceof Error) { + console.error("[ERROR] deleteChatHandler:", error.message); + } return NextResponse.json( - { status: "error", error: message }, + { status: "error", error: "Server error" }, { status: 500, headers: getCorsHeaders() }, ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/chats/deleteChatHandler.ts` around lines 39 - 44, The catch block in deleteChatHandler currently returns error.message to the client; instead, log the full error server-side (e.g., using your logger or console.error) and return a sanitized generic response (e.g., "Internal server error") via NextResponse.json so internal details are not leaked; update the catch handling around NextResponse.json and getCorsHeaders to log the original Error and send only the generic message to clients.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/chats/validateDeleteChatBody.ts`:
- Around line 69-71: The call to buildGetChatsParams can return { params: null,
error } when access is denied, so update validateDeleteChatBody to handle that
case: after calling buildGetChatsParams({ account_id: accountId }) check if
result.params is null and propagate/return the error (or throw) instead of
dereferencing params.account_ids; include the returned error message when
failing so callers get the access-denied reason and avoid a potential runtime
crash in validateDeleteChatBody.
- Around line 73-81: The access check in validateDeleteChatBody.ts incorrectly
skips authorization when room.account_id is falsy; remove the `&&
room.account_id` guard so the code always validates authorization when
params.account_ids is present (noting buildGetChatsParams always sets
params.account_ids), and treat a null/undefined room.account_id as not
allowed—i.e., if !params.account_ids.includes(room.account_id) or
room.account_id == null then return the same 403 NextResponse.json({ status:
"error", error: "Access denied to this chat" }, ...); also update or remove the
misleading "admin access" comment.
In `@lib/supabase/rooms/deleteRoomWithRelations.ts`:
- Around line 10-69: The current deleteRoomWithRelations function performs
multiple sequential deletions (tables: memory_emails, memories, room_reports,
segment_rooms, rooms) and risks partial failure; replace this with a single
transactional operation by creating a PostgreSQL RPC (or ALTER TABLE foreign
keys with ON DELETE CASCADE) that wraps all related deletes in one transaction,
then update deleteRoomWithRelations to call that RPC (or rely on cascade and
simply delete from rooms by id); reference function deleteRoomWithRelations and
the related tables (memories, memory_emails, room_reports, segment_rooms, rooms)
when implementing the DB-side transaction and update the JS call to invoke the
RPC instead of issuing multiple supabase.delete() calls.
---
Nitpick comments:
In `@lib/chats/deleteChatHandler.ts`:
- Around line 39-44: The catch block in deleteChatHandler currently returns
error.message to the client; instead, log the full error server-side (e.g.,
using your logger or console.error) and return a sanitized generic response
(e.g., "Internal server error") via NextResponse.json so internal details are
not leaked; update the catch handling around NextResponse.json and
getCorsHeaders to log the original Error and send only the generic message to
clients.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a70a7686-c73b-4d6e-9b0e-b28c40fe4893
⛔ Files ignored due to path filters (2)
lib/chats/__tests__/deleteChatHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/chats/__tests__/validateDeleteChatBody.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (4)
app/api/chats/route.tslib/chats/deleteChatHandler.tslib/chats/validateDeleteChatBody.tslib/supabase/rooms/deleteRoomWithRelations.ts
Comment on lines
+10
to
+69
| export async function deleteRoomWithRelations(roomId: string): Promise<boolean> { | ||
| const { data: memories, error: selectMemoriesError } = await supabase | ||
| .from("memories") | ||
| .select("id") | ||
| .eq("room_id", roomId); | ||
|
|
||
| if (selectMemoriesError) { | ||
| console.error("[ERROR] deleteRoomWithRelations select memories:", selectMemoriesError); | ||
| return false; | ||
| } | ||
|
|
||
| const memoryIds = (memories || []).map(memory => memory.id); | ||
|
|
||
| if (memoryIds.length > 0) { | ||
| const { error: deleteMemoryEmailsError } = await supabase | ||
| .from("memory_emails") | ||
| .delete() | ||
| .in("memory", memoryIds); | ||
|
|
||
| if (deleteMemoryEmailsError) { | ||
| console.error("[ERROR] deleteRoomWithRelations memory_emails:", deleteMemoryEmailsError); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| const { error: deleteMemoriesError } = await supabase | ||
| .from("memories") | ||
| .delete() | ||
| .eq("room_id", roomId); | ||
| if (deleteMemoriesError) { | ||
| console.error("[ERROR] deleteRoomWithRelations memories:", deleteMemoriesError); | ||
| return false; | ||
| } | ||
|
|
||
| const { error: deleteRoomReportsError } = await supabase | ||
| .from("room_reports") | ||
| .delete() | ||
| .eq("room_id", roomId); | ||
| if (deleteRoomReportsError) { | ||
| console.error("[ERROR] deleteRoomWithRelations room_reports:", deleteRoomReportsError); | ||
| return false; | ||
| } | ||
|
|
||
| const { error: deleteSegmentRoomsError } = await supabase | ||
| .from("segment_rooms") | ||
| .delete() | ||
| .eq("room_id", roomId); | ||
| if (deleteSegmentRoomsError) { | ||
| console.error("[ERROR] deleteRoomWithRelations segment_rooms:", deleteSegmentRoomsError); | ||
| return false; | ||
| } | ||
|
|
||
| const { error: deleteRoomError } = await supabase.from("rooms").delete().eq("id", roomId); | ||
|
|
||
| if (deleteRoomError) { | ||
| console.error("[ERROR] deleteRoomWithRelations room:", deleteRoomError); | ||
| return false; | ||
| } | ||
|
|
||
| return true; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there's an existing pattern for transactional deletes in the codebase
rg -n "rpc\(" --type=ts lib/supabase/Repository: recoupable/api
Length of output: 40
🏁 Script executed:
# Read the full file to see imports and JSDoc comments
cat -n lib/supabase/rooms/deleteRoomWithRelations.tsRepository: recoupable/api
Length of output: 2642
🏁 Script executed:
# Search for other delete operations in the supabase lib
find lib/supabase -name "*delete*.ts" -o -name "*remove*.ts" | head -20Repository: recoupable/api
Length of output: 473
🏁 Script executed:
# Look for transaction or atomicity patterns in the codebase
rg -n "transaction|atomic|rollback|RPC" --type=ts lib/ | head -30Repository: recoupable/api
Length of output: 2400
🏁 Script executed:
# Search for other delete patterns to understand existing conventions
rg -B2 -A10 "\.delete\(\)" --type=ts lib/supabase/ | head -50Repository: recoupable/api
Length of output: 3710
Consider enforcing transactional guarantees for cascading deletes at the database level.
Sequential delete operations leave the system vulnerable to partial failures: if room_reports deletion succeeds but segment_rooms fails, data remains inconsistent with no rollback. While the function appropriately returns false on error to prevent further operations, this pattern is used across multiple delete functions in the codebase.
For critical cascading deletions like this, consider implementing one of these approaches:
- Database-level cascading: Use foreign key constraints with
ON DELETE CASCADEto atomically delete related records - Supabase RPC: Create a PostgreSQL function that wraps all deletes in a single transaction
- Explicit transaction management: If the Supabase JS client supports it, wrap the entire operation
This is a systemic concern affecting other delete functions (deleteSegments, deleteApiKey, etc.) and warrants a codebase-wide solution rather than per-function workarounds.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/supabase/rooms/deleteRoomWithRelations.ts` around lines 10 - 69, The
current deleteRoomWithRelations function performs multiple sequential deletions
(tables: memory_emails, memories, room_reports, segment_rooms, rooms) and risks
partial failure; replace this with a single transactional operation by creating
a PostgreSQL RPC (or ALTER TABLE foreign keys with ON DELETE CASCADE) that wraps
all related deletes in one transaction, then update deleteRoomWithRelations to
call that RPC (or rely on cascade and simply delete from rooms by id); reference
function deleteRoomWithRelations and the related tables (memories,
memory_emails, room_reports, segment_rooms, rooms) when implementing the DB-side
transaction and update the JS call to invoke the RPC instead of issuing multiple
supabase.delete() calls.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
lib/supabase/rooms/deleteRoomWithRelations.ts (1)
10-64:⚠️ Potential issue | 🔴 CriticalFail-open deletion flow can return success with partial data still present.
From Line 16 through Line 55, intermediate delete/select errors are only logged and execution continues; the function returns
trueunless Line 57 fails. This can acknowledge chat deletion even when related rows remain. Please fail fast on any step, and ideally make this atomic (RPC/DB transaction) to prevent partial deletes.Suggested minimal fix (fail-fast) in this file
export async function deleteRoomWithRelations(roomId: string): Promise<boolean> { const { data: memories, error: selectMemoriesError } = await supabase .from("memories") .select("id") .eq("room_id", roomId); if (selectMemoriesError) { console.error("[WARN] deleteRoomWithRelations select memories:", selectMemoriesError); + return false; } const memoryIds = (memories || []).map(memory => memory.id); if (memoryIds.length > 0) { const { error: deleteMemoryEmailsError } = await supabase .from("memory_emails") .delete() .in("memory", memoryIds); if (deleteMemoryEmailsError) { console.error("[WARN] deleteRoomWithRelations memory_emails:", deleteMemoryEmailsError); + return false; } } const { error: deleteMemoriesError } = await supabase .from("memories") .delete() .eq("room_id", roomId); if (deleteMemoriesError) { console.error("[WARN] deleteRoomWithRelations memories:", deleteMemoriesError); + return false; } const { error: deleteRoomReportsError } = await supabase .from("room_reports") .delete() .eq("room_id", roomId); if (deleteRoomReportsError) { console.error("[WARN] deleteRoomWithRelations room_reports:", deleteRoomReportsError); + return false; } const { error: deleteSegmentRoomsError } = await supabase .from("segment_rooms") .delete() .eq("room_id", roomId); if (deleteSegmentRoomsError) { console.error("[WARN] deleteRoomWithRelations segment_rooms:", deleteSegmentRoomsError); + return false; } const { error: deleteRoomError } = await supabase.from("rooms").delete().eq("id", roomId); if (deleteRoomError) { console.error("[ERROR] deleteRoomWithRelations room:", deleteRoomError); return false; } return true; }#!/bin/bash set -euo pipefail # Verify whether DB-level cascade or transactional RPC already exists for room deletion. # Expected: # - If results contain an RPC/function handling room cascade delete, prefer that path. # - If FK constraints include ON DELETE CASCADE for relevant relations, sequential deletes can be simplified. fd -i '\.sql$' | xargs rg -n -i \ 'create\s+(or\s+replace\s+)?function\s+.*delete.*room|on\s+delete\s+cascade|foreign\s+key.*(memory_emails|memories|room_reports|segment_rooms|rooms)'As per coding guidelines, "For domain functions, ensure: ... Proper error handling ... Avoid side effects ... DRY ... KISS" and "For Supabase operations, ensure: ... Handle errors gracefully".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/supabase/rooms/deleteRoomWithRelations.ts` around lines 10 - 64, The deleteRoomWithRelations function currently logs intermediate Supabase errors and continues, allowing partial deletes; update it to fail-fast: after each Supabase call (the select on "memories" and each delete on "memory_emails", "memories", "room_reports", "segment_rooms", and final "rooms") check for an error and immediately return false (or throw) instead of only logging; ideally replace the sequential deletes with a single DB-side atomic operation (use an RPC/SQL function or a transaction) to delete room and relations atomically and call that RPC from deleteRoomWithRelations to ensure no partial state.lib/chats/validateChatAccess.ts (1)
42-50:⚠️ Potential issue | 🔴 CriticalDon't skip the ownership check when
room.account_idis null.Line 43 turns authorization off for rooms with a
NULLaccount_id, so any authenticated caller can pass this validator for those chats.buildGetChatsParams({ account_id })still yields an allowed-account list on this path, so a missing room account should be denied rather than treated like admin access.🔒 Suggested fix
- // If params.account_ids is undefined, it means admin access (all records) - if (params.account_ids && room.account_id) { - if (!params.account_ids.includes(room.account_id)) { - return NextResponse.json( - { status: "error", error: "Access denied to this chat" }, - { status: 403, headers: getCorsHeaders() }, - ); - } + if (!room.account_id || !params.account_ids.includes(room.account_id)) { + return NextResponse.json( + { status: "error", error: "Access denied to this chat" }, + { status: 403, headers: getCorsHeaders() }, + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/chats/validateChatAccess.ts` around lines 42 - 50, The ownership check currently skips rooms with a null room.account_id; update validateChatAccess logic so that when params.account_ids is present you deny access if room.account_id is missing or not in the allowed list. Concretely, in the block using params.account_ids and room.account_id, change the guard to check params.account_ids alone and then return the 403 NextResponse.json (using the same message and getCorsHeaders()) if either room.account_id is falsy or params.account_ids does not include room.account_id.
🧹 Nitpick comments (1)
lib/supabase/rooms/deleteRoomWithRelations.ts (1)
10-65: Consider splitting this into small helpers to keep it under 50 lines and reduce repetition.The function currently exceeds the 50-line guideline and repeats near-identical delete/error blocks. Extracting a tiny internal helper for
deleteWhere(table, column, value)would keep behavior the same and improve readability.As per coding guidelines, "For domain functions, ensure: ... Keep functions under 50 lines ... DRY: Consolidate similar logic into shared utilities ... KISS: Prefer simple, readable implementations over clever optimizations".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/supabase/rooms/deleteRoomWithRelations.ts` around lines 10 - 65, The deleteRoomWithRelations function is overlong and repeats identical delete/error handling blocks; extract a small internal helper (e.g., deleteWhere(tableName: string, column: string, value: string | string[])) and replace repeated blocks in deleteRoomWithRelations with calls to that helper for memory_emails, memories, room_reports, segment_rooms and rooms, preserving existing logging behavior and return false on final room delete error; ensure you still select memory ids via the existing query (memories -> memoryIds) and pass memoryIds (array) to deleteWhere when deleting memory_emails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/supabase/rooms/deleteRoomWithRelations.ts`:
- Line 1: The import at the top of deleteRoomWithRelations.ts uses a relative
path ("import supabase from \"../serverClient\""); change it to the canonical
project path "import supabase from '@/lib/supabase/serverClient'". Update the
import statement for the supabase client (supabase) to use that exact module
specifier and ensure the file follows the project's Supabase function pattern
(JSDoc, error handling, and return typing) around the related exported
function(s) in this module.
---
Duplicate comments:
In `@lib/chats/validateChatAccess.ts`:
- Around line 42-50: The ownership check currently skips rooms with a null
room.account_id; update validateChatAccess logic so that when params.account_ids
is present you deny access if room.account_id is missing or not in the allowed
list. Concretely, in the block using params.account_ids and room.account_id,
change the guard to check params.account_ids alone and then return the 403
NextResponse.json (using the same message and getCorsHeaders()) if either
room.account_id is falsy or params.account_ids does not include room.account_id.
In `@lib/supabase/rooms/deleteRoomWithRelations.ts`:
- Around line 10-64: The deleteRoomWithRelations function currently logs
intermediate Supabase errors and continues, allowing partial deletes; update it
to fail-fast: after each Supabase call (the select on "memories" and each delete
on "memory_emails", "memories", "room_reports", "segment_rooms", and final
"rooms") check for an error and immediately return false (or throw) instead of
only logging; ideally replace the sequential deletes with a single DB-side
atomic operation (use an RPC/SQL function or a transaction) to delete room and
relations atomically and call that RPC from deleteRoomWithRelations to ensure no
partial state.
---
Nitpick comments:
In `@lib/supabase/rooms/deleteRoomWithRelations.ts`:
- Around line 10-65: The deleteRoomWithRelations function is overlong and
repeats identical delete/error handling blocks; extract a small internal helper
(e.g., deleteWhere(tableName: string, column: string, value: string | string[]))
and replace repeated blocks in deleteRoomWithRelations with calls to that helper
for memory_emails, memories, room_reports, segment_rooms and rooms, preserving
existing logging behavior and return false on final room delete error; ensure
you still select memory ids via the existing query (memories -> memoryIds) and
pass memoryIds (array) to deleteWhere when deleting memory_emails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3c07373a-8d09-4e07-9ab4-49f621dfbe76
⛔ Files ignored due to path filters (3)
lib/chats/__tests__/validateChatAccess.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/chats/__tests__/validateDeleteChatBody.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/chats/__tests__/validateUpdateChatBody.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (4)
lib/chats/validateChatAccess.tslib/chats/validateDeleteChatBody.tslib/chats/validateUpdateChatBody.tslib/supabase/rooms/deleteRoomWithRelations.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/supabase/rooms/deleteRoomWithRelations.ts`:
- Around line 1-9: The JSDoc for deleteRoomWithRelations is inaccurate: it
claims the function returns false when any step fails, but the implementation
only returns false if the final rooms delete fails and treats intermediate
errors as warnings; update the JSDoc return description to reflect the actual
"best-effort" behavior (i.e., the function returns true if the room delete
succeeds even if prior related-deletes logged warnings), and note that
intermediate failures are logged and do not short-circuit; alternatively, if you
prefer fail-fast behavior, change each intermediate error handling in
deleteRoomWithRelations (memory_emails, memories, room_reports, segment_rooms)
to return false immediately on error instead of just logging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: da38a57e-36b5-46f2-8dfb-d8d1a7d4fbe7
⛔ Files ignored due to path filters (1)
lib/chats/__tests__/validateUpdateChatBody.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (5)
lib/chats/deleteChatHandler.tslib/chats/validateChatAccess.tslib/chats/validateDeleteChatBody.tslib/chats/validateUpdateChatBody.tslib/supabase/rooms/deleteRoomWithRelations.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- lib/chats/validateUpdateChatBody.ts
- lib/chats/validateChatAccess.ts
- lib/chats/validateDeleteChatBody.ts
Comment on lines
+1
to
+9
| import supabase from "@/lib/supabase/serverClient"; | ||
|
|
||
| /** | ||
| * Deletes a room and related room data. | ||
| * This removes memory_emails, memories, room_reports, segment_rooms, then the room. | ||
| * | ||
| * @param roomId - The room ID to delete | ||
| * @returns True when deletion succeeds, false when any step fails | ||
| */ |
There was a problem hiding this comment.
JSDoc return description is inconsistent with actual behavior.
The JSDoc states the function returns "false when any step fails," but the implementation only returns false when the final rooms deletion fails (line 59-61). Intermediate failures (memory_emails, memories, room_reports, segment_rooms) log warnings but continue execution and still return true if the room itself is deleted.
Either update the JSDoc to accurately reflect the current "best-effort" behavior, or update the logic to fail fast on any error.
📝 Proposed JSDoc fix (if keeping current behavior)
/**
* Deletes a room and related room data.
* This removes memory_emails, memories, room_reports, segment_rooms, then the room.
*
* `@param` roomId - The room ID to delete
- * `@returns` True when deletion succeeds, false when any step fails
+ * `@returns` True when room deletion succeeds (related data deletion is best-effort), false when room deletion fails
*/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/supabase/rooms/deleteRoomWithRelations.ts` around lines 1 - 9, The JSDoc
for deleteRoomWithRelations is inaccurate: it claims the function returns false
when any step fails, but the implementation only returns false if the final
rooms delete fails and treats intermediate errors as warnings; update the JSDoc
return description to reflect the actual "best-effort" behavior (i.e., the
function returns true if the room delete succeeds even if prior related-deletes
logged warnings), and note that intermediate failures are logged and do not
short-circuit; alternatively, if you prefer fail-fast behavior, change each
intermediate error handling in deleteRoomWithRelations (memory_emails, memories,
room_reports, segment_rooms) to return false immediately on error instead of
just logging.
Collaborator
Author
|
Deployed smoke test results (run only against this PR preview): 1) Bearer flow + cascade behavior
This validates delete behavior and gives an end-to-end cascade signal via the memory-consuming compact path: the room was usable for chat+compact before deletion and is no longer resolvable after deletion. 2) x-api-key compatibility
No failures on the PR deployment for the tested paths. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
DELETE /api/chatsroute handler in API servicememory_emails,memories,room_reports,segment_rooms,rooms)Validation
pnpm exec eslint app/api/chats/route.ts lib/chats/deleteChatHandler.ts lib/chats/validateDeleteChatBody.ts lib/chats/__tests__/deleteChatHandler.test.ts lib/chats/__tests__/validateDeleteChatBody.test.ts lib/supabase/rooms/deleteRoomWithRelations.tspnpm test lib/chats/__tests__/validateDeleteChatBody.test.ts lib/chats/__tests__/deleteChatHandler.test.tsSummary by CodeRabbit
New Features
Improvements
Bug Fixes