Daily branch 2025 10 01 #43
Conversation
- Add missing serviceKey parameter in getMemoryById() - Make deleteMemoryForBackend() and deleteUserMemory() idempotent - Handle non-existent memories gracefully during deletion - Prevents duplicate delete request errors
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds client-side file download handling and UI updates, registers a GetTerminalFilesHandler component and message branch, refactors sandbox uploader to fetch blobs via sandbox.downloadUrl with stricter error checks and token metadata, surfaces upload-status notifications in chat/messages, and updates system prompt with /home/user/upload guidance. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as FilePartRenderer
participant Net as fetch(url)
participant Browser as Browser
User->>UI: Click "Download"
UI->>Net: GET file URL
alt 200 OK
Net-->>UI: Blob
UI->>UI: Create blob URL
UI->>Browser: Trigger download (anchor.click)
UI->>UI: Revoke blob URL
else Error
Net-->>UI: Non-OK / throw
UI->>Browser: window.open(url, "_blank")
end
sequenceDiagram
autonumber
participant Msg as MessagePartHandler
participant Comp as GetTerminalFilesHandler
participant UI as ToolBlock
Note over Msg: part.type === "tool-get_terminal_files"
Msg->>Comp: Render({ part, status })
alt input-streaming
Comp->>UI: Show "Preparing…" (shimmer)
else input-available
Comp->>UI: Show "Sharing…" with file list
else output-available
Comp->>UI: Show "Shared" with file list/count
else output-error
Comp->>UI: Show "Failed to share"
end
sequenceDiagram
autonumber
participant SFU as sandbox-file-uploader
participant SB as sandbox
participant Net as fetch
participant Up as Upload API
SFU->>SB: downloadUrl(path)
SB-->>SFU: URL
SFU->>Net: GET URL
alt 200 OK
Net-->>SFU: Blob
SFU->>Up: POST Blob
alt Upload OK
Up-->>SFU: { id, tokens, ... }
SFU->>SFU: Save metadata
else Upload Error
Up-->>SFU: Non-OK
SFU->>SFU: Throw upload error
end
else Download Error
Net-->>SFU: Non-OK
SFU->>SFU: Throw download error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
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)
app/components/tools/GetTerminalFilesHandler.tsx (1)
22-24: Add explicit return type annotation.Per coding guidelines, functions should have explicit type annotations for better type safety.
- const getFileNames = (paths: string[]) => { + const getFileNames = (paths: string[]): string => { return paths.map((path) => path.split("/").pop() || path).join(", "); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/components/FilePartRenderer.tsx(6 hunks)app/components/MessagePartHandler.tsx(2 hunks)app/components/tools/GetTerminalFilesHandler.tsx(1 hunks)lib/ai/tools/get-terminal-files.ts(1 hunks)lib/ai/tools/run-terminal-cmd.ts(1 hunks)lib/ai/tools/utils/sandbox-file-uploader.ts(1 hunks)lib/system-prompt.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id helper type from ./_generated/dataModel to type document IDs (e.g., Id<'users'>) instead of string
When defining Record types, specify key and value types matching validators (e.g., Record<Id<'users'>, string>)
Be strict with types for document IDs; prefer Id<'table'> over string in function args and variables
Useas constfor string literals in discriminated unions
Declare arrays with explicit generic type: const arr: Array = [...]
Declare records with explicit generic types: const record: Record<KeyType, ValueType> = {...}
Files:
lib/ai/tools/get-terminal-files.tslib/ai/tools/utils/sandbox-file-uploader.tsapp/components/MessagePartHandler.tsxapp/components/tools/GetTerminalFilesHandler.tsxlib/ai/tools/run-terminal-cmd.tslib/system-prompt.tsapp/components/FilePartRenderer.tsx
🧠 Learnings (1)
📚 Learning: 2025-08-29T13:33:09.937Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.937Z
Learning: Applies to convex/**/*.ts : Use ctx.storage.getUrl(fileId) to obtain signed URLs; it returns null if the file does not exist
Applied to files:
lib/ai/tools/utils/sandbox-file-uploader.ts
🧬 Code graph analysis (2)
app/components/MessagePartHandler.tsx (1)
app/components/tools/GetTerminalFilesHandler.tsx (1)
GetTerminalFilesHandler(11-74)
app/components/tools/GetTerminalFilesHandler.tsx (1)
types/chat.ts (1)
ChatStatus(60-60)
🪛 Biome (2.1.2)
app/components/tools/GetTerminalFilesHandler.tsx
[error] 49-49: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 50-50: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (15)
lib/ai/tools/run-terminal-cmd.ts (1)
30-30: LGTM! Clear guidance update.The updated text better reflects the download-centric workflow introduced in this PR and provides clearer guidance to the AI on when and how to use the terminal files tool.
lib/system-prompt.ts (1)
140-140: LGTM! Helpful operational context.This addition provides clear guidance to the AI about where user attachments are located and what to do if files are missing. The instruction aligns well with the broader attachment/download workflow introduced in this PR.
lib/ai/tools/get-terminal-files.ts (1)
10-17: LGTM! Enhanced tool description.The expanded description provides much clearer guidance with concrete examples of file paths and usage scenarios. This will help the AI understand when and how to use the terminal files tool effectively.
lib/ai/tools/utils/sandbox-file-uploader.ts (3)
45-56: LGTM! Improved file retrieval with proper error handling.The new approach using
sandbox.downloadUrlfollowed byfetchprovides better separation of concerns and includes appropriate error handling with descriptive error messages. The 30-second expiration is a reasonable security measure for temporary download URLs.
69-73: LGTM! Explicit upload error handling.Good addition of explicit error handling for upload failures with descriptive error messages including status codes.
13-13: Ensureapi.fileActions.saveFilereturns the newtokensfield and that every consumer ofUploadedFileInfohandles it correctly.app/components/FilePartRenderer.tsx (6)
2-2: LGTM! Appropriate import addition.Adding
useCallbackis correct for the newhandleDownloadfunction to ensure stable function identity.
4-4: LGTM! Icon change aligns with download action.Replacing
EyewithDownloadaccurately reflects the new download behavior instead of the previous open-in-view action.
18-36: Good implementation with appropriate fallback.The client-side download flow is well-implemented:
- Fetches the file and creates a blob
- Uses programmatic download with proper cleanup
- Falls back to opening in new tab on error
- Logs errors for debugging
The approach works across modern browsers and handles edge cases appropriately.
68-81: LGTM! UI changes align with download semantics.The changes correctly reflect the download action:
- Download icon replaces the previous Eye icon
onClickhandler invokeshandleDownloadinstead of opening URLaria-labelupdated to "Download {fileName}" for accessibility
99-99: LGTM! Correct memoization dependency.Adding
handleDownloadto theFilePreviewCardmemoization dependencies ensures the component updates when the handler changes.
201-201: LGTM! Proper dependency management.Adding
FilePreviewCardto the dependency array ensures correct memoization behavior when the memoized component changes.app/components/MessagePartHandler.tsx (2)
8-8: LGTM!The import follows the existing pattern and properly brings in the new GetTerminalFilesHandler component.
79-80: LGTM!The new case handler follows the established pattern for tool rendering and correctly wires GetTerminalFilesHandler into the message part switch.
app/components/tools/GetTerminalFilesHandler.tsx (1)
26-73: Verify exhaustive handling ofstate. Confirm that the four cases (“input-streaming”, “input-available”, “output-available”, “output-error”) cover all possible values ofpart.state, or explicitly handle or document the intended fallback for any other states.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/api/chat/route.ts(1 hunks)app/components/Messages.tsx(4 hunks)app/components/chat.tsx(3 hunks)convex/memories.ts(4 hunks)lib/ai/tools/update-memory.ts(1 hunks)lib/db/actions.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
convex/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/**/*.ts: Always use the new Convex function syntax (query/mutation/action objects with args/returns/handler) when defining Convex functions
When a function returns null, include returns: v.null() and return null explicitly
Use internalQuery/internalMutation/internalAction for private functions callable only by other Convex functions; do not expose sensitive logic via public query/mutation/action
Use query/mutation/action only for public API functions
Do not try to register functions via the api or internal objects
Always include argument and return validators for all Convex functions (query/internalQuery/mutation/internalMutation/action/internalAction)
In JS implementations, functions without an explicit return value implicitly return null
Use ctx.runQuery from queries/mutations/actions to call a query
Use ctx.runMutation from mutations/actions to call a mutation
Use ctx.runAction from actions to call an action
Only call an action from another action when crossing runtimes (e.g., V8 to Node); otherwise extract shared helper code
Minimize calls from actions to queries/mutations to avoid race conditions from splitting transactions
Pass FunctionReference values (from api/internal) to ctx.runQuery/ctx.runMutation/ctx.runAction; do not pass function implementations
When calling a function in the same file via ctx.run*, add an explicit return type annotation at the call site to avoid TS circularity
Use the generated api object for public functions and internal object for internal functions from convex/_generated/api.ts
Respect file-based routing for function references: e.g., convex/example.ts export f -> api.example.f; nested paths map to dot-separated namespaces
For paginated queries use paginationOptsValidator in args and .paginate(args.paginationOpts) on a query
v.bigint() is deprecated; use v.int64() for signed 64-bit integers
Use v.record(keys, values) for record-like data; v.map() and v.set() are not supported
For full-text search, use withSearchIndex("ind...
Files:
convex/memories.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id helper type from ./_generated/dataModel to type document IDs (e.g., Id<'users'>) instead of string
When defining Record types, specify key and value types matching validators (e.g., Record<Id<'users'>, string>)
Be strict with types for document IDs; prefer Id<'table'> over string in function args and variables
Useas constfor string literals in discriminated unions
Declare arrays with explicit generic type: const arr: Array = [...]
Declare records with explicit generic types: const record: Record<KeyType, ValueType> = {...}
Files:
convex/memories.tsapp/components/chat.tsxapp/api/chat/route.tslib/db/actions.tslib/ai/tools/update-memory.tsapp/components/Messages.tsx
🧬 Code graph analysis (3)
app/api/chat/route.ts (1)
lib/utils/sandbox-file-utils.ts (1)
uploadSandboxFiles(54-70)
lib/ai/tools/update-memory.ts (1)
lib/db/actions.ts (1)
getMemoryById(284-297)
app/components/Messages.tsx (1)
app/components/ShimmerText.tsx (1)
ShimmerText(12-39)
🔇 Additional comments (6)
lib/db/actions.ts (1)
287-287: LGTM! Consistent service key authentication.This change aligns
getMemoryByIdwith other backend calls in the file that already includeserviceKeyfor proper authentication.convex/memories.ts (2)
196-236: LGTM! Proper idempotent deletion.Making the deletion operation idempotent is a solid improvement that aligns with REST semantics. The ownership verification at line 223 is still enforced when the memory exists, maintaining security.
307-348: LGTM! Consistent idempotent behavior.The changes mirror the improvements in
deleteMemoryForBackend, ensuring both frontend and backend deletion paths are idempotent while preserving ownership checks.lib/ai/tools/update-memory.ts (1)
139-167: LGTM! Graceful handling of missing memory.The try/catch around
getMemoryById(lines 149-156) properly handles the case where memory doesn't exist, allowing the delete operation to proceed idempotently. The fallback message at line 165 ensures a meaningful response even when the memory is missing.app/components/Messages.tsx (1)
404-411: LGTM! Clean upload status UI implementation.The upload status banner is appropriately positioned and uses ShimmerText for a polished loading effect. The conditional rendering ensures it only appears when actively uploading.
app/components/chat.tsx (1)
170-178: Alldata-upload-statusemissions match{ message: string; isUploading: boolean }, so the payload assertion is safe.
…nagement - Create new DataControlsTab component with database icon - Add delete all chats functionality (moved from Account tab) - Add delete terminal sandbox feature for pro/ultra users - Create /api/delete-sandboxes endpoint with subscription validation - Update SettingsDialog to include new Data controls tab - Clean up AccountTab by removing delete chats section
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/components/chat.tsx (2)
84-103: Clear uploadStatus when chat transitions occur.The effect handles chat ID transitions but doesn't clear
uploadStatuswhen switching between chats via route changes. Lines 98-101 set a new chat ID without clearing the upload banner, which could leave stale upload status visible.Apply this diff to clear
uploadStatusduring route-based chat transitions:// If a chat id is present in the route, treat as existing chat if (routeChatId) { setChatId(routeChatId); setIsExistingChat(true); + setUploadStatus(null); return; }
231-243: Ensure reset runs on all chat initializations
Reset is only invoked viainitializeNewChat(GlobalState.tsx:323–326), which your UI calls only on “New Chat” clicks (ChatHeader, SidebarHeader, ChatItem) and not when switching to an existing chat viainitializeChatoractivateChat. Invoke the reset function in those flows as well to clearuploadStatusand other local state.
🧹 Nitpick comments (2)
app/api/delete-sandboxes/route.ts (1)
11-16: Remove redundant authorization check.The
getUserIDAndProhelper (lib/auth/get-user-id.ts) throws aChatSDKError("unauthorized:auth")if the session or user ID is missing. If it returns successfully,userIdwill always be truthy. This manual check is unreachable dead code.Apply this diff to remove the redundant check:
const { userId, subscription } = await getUserIDAndPro(req); - if (!userId) { - return new Response(JSON.stringify({ error: "Unauthorized" }), { - status: 401, - headers: { "Content-Type": "application/json" }, - }); - } - // Only allow subscribed users to delete sandboxesapp/components/DataControlsTab.tsx (1)
29-41: Add user feedback for deletion errors.When chat deletion fails, the user receives no feedback before being redirected to
/. Consider showing an error toast and only redirecting on success.Apply this diff to provide proper error feedback:
const handleDeleteAllChats = async () => { if (isDeletingChats) return; setIsDeletingChats(true); try { await deleteAllChats(); + setShowDeleteChats(false); + window.location.href = "/"; } catch (error) { console.error("Failed to delete all chats:", error); + toast.error("Failed to delete chats"); } finally { - setShowDeleteChats(false); - window.location.href = "/"; setIsDeletingChats(false); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/api/chat/route.ts(1 hunks)app/api/delete-sandboxes/route.ts(1 hunks)app/components/AccountTab.tsx(0 hunks)app/components/DataControlsTab.tsx(1 hunks)app/components/SettingsDialog.tsx(3 hunks)app/components/chat.tsx(5 hunks)app/components/tools/GetTerminalFilesHandler.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- app/components/AccountTab.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- app/components/tools/GetTerminalFilesHandler.tsx
- app/api/chat/route.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id helper type from ./_generated/dataModel to type document IDs (e.g., Id<'users'>) instead of string
When defining Record types, specify key and value types matching validators (e.g., Record<Id<'users'>, string>)
Be strict with types for document IDs; prefer Id<'table'> over string in function args and variables
Useas constfor string literals in discriminated unions
Declare arrays with explicit generic type: const arr: Array = [...]
Declare records with explicit generic types: const record: Record<KeyType, ValueType> = {...}
Files:
app/components/DataControlsTab.tsxapp/components/SettingsDialog.tsxapp/components/chat.tsxapp/api/delete-sandboxes/route.ts
🧬 Code graph analysis (2)
app/components/DataControlsTab.tsx (2)
app/contexts/GlobalState.tsx (1)
useGlobalState(439-445)convex/chats.ts (1)
deleteAllChats(456-524)
app/api/delete-sandboxes/route.ts (1)
lib/auth/get-user-id.ts (1)
getUserIDAndPro(41-88)
🔇 Additional comments (16)
app/components/chat.tsx (4)
43-46: LGTM! Clean type definition for upload status.The discriminated union pattern with
nullfor idle state is clear and type-safe.
170-176: LGTM! Upload status handler logic is correct.The handler correctly updates
uploadStatusfrom the data stream and clears it whenisUploadingis false.
215-215: LGTM! Upload status cleared on error.Correctly addresses the past review comment by clearing
uploadStatusin the error handler.
468-468: LGTM! Upload status correctly propagated to Messages.The
uploadStatusprop is correctly passed to the Messages component.app/components/SettingsDialog.tsx (3)
4-4: LGTM!The new imports for
Databaseicon andDataControlsTabcomponent are correctly added and align with their usage in the tab list and content rendering.Also applies to: 11-11
28-28: LGTM!The new "Data controls" tab entry follows the existing pattern and is correctly structured with matching id, label, and icon.
130-131: LGTM!The conditional rendering of
DataControlsTabfollows the existing pattern used for other tabs.app/api/delete-sandboxes/route.ts (4)
1-5: LGTM!The imports are correct and
maxDuration = 60is appropriate for an operation that may need to kill multiple sandboxes.
18-24: LGTM!The subscription check correctly restricts sandbox deletion to paid users (pro/ultra) and returns a clear 403 response for free users.
51-60: LGTM!The error handling correctly logs the error for debugging and returns a generic 500 response to the client without leaking internal details.
26-45: Verify pagination: ensure all sandboxes are deleted. The code callsawait paginator.nextItems()only once, so only the first page of sandboxes will be killed. Loopawait paginator.nextItems()until it returns no items to delete all pages.app/components/DataControlsTab.tsx (5)
1-27: LGTM!The imports and component setup are correct. State management follows React patterns with clear separation between chat deletion and sandbox deletion states.
43-64: LGTM!The sandbox deletion handler has proper error handling with user feedback via toasts and correctly manages loading state. The error message extraction from the response is well implemented.
66-83: LGTM!The delete chats UI section is clear, accessible, and uses appropriate destructive styling.
85-105: LGTM!The delete sandbox UI is correctly gated for paid users only and provides clear descriptive text explaining the action's impact.
107-163: LGTM!Both confirmation dialogs follow consistent patterns with clear warnings, proper loading states, and accessible button labels.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation