Repository navigation
feat: 隐藏wegent button;文件无法上传问题修复;支持文件拖拽 - #1458
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds native and browser file drag-and-drop to the Tauri desktop chat composer via a new Rust ChangesDesktop file drop, dynamic floating menus, and attachment API
AppsPage UI updates
Sequence Diagram(s)sequenceDiagram
participant User
participant Form
participant ProjectChatComposer
participant TauriRuntime
participant readDroppedFiles as read_dropped_files<br/>(Rust)
rect rgba(100, 149, 237, 0.5)
Note over User, Form: Browser drag-and-drop path
User->>Form: dragover event
Form->>ProjectChatComposer: handleDragOver → set dropEffect='copy'
User->>Form: drop event
Form->>ProjectChatComposer: handleDrop → extract DataTransfer files
ProjectChatComposer->>ProjectChatComposer: onFileSelect(files)
end
rect rgba(144, 238, 144, 0.5)
Note over User, readDroppedFiles: Tauri native drop path
User->>TauriRuntime: native file drop onto window
TauriRuntime->>ProjectChatComposer: onDragDropEvent(payload.paths)
ProjectChatComposer->>readDroppedFiles: invoke('read_dropped_files', {paths})
readDroppedFiles-->>ProjectChatComposer: DroppedFilePayload[] {name, bytes}
ProjectChatComposer->>ProjectChatComposer: convert bytes → File[]
ProjectChatComposer->>ProjectChatComposer: onFileSelect(files)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
wework/src-tauri/src/lib.rs (1)
172-202: ⚡ Quick winConsider adding a file size limit to prevent memory exhaustion.
The command reads entire files into memory without checking their size. The frontend defines
MAX_FILE_SIZE = 100 * 1024 * 1024(100 MB) inattachments.ts, but this backend command will happily load arbitrarily large files. A malicious or accidental drop of a very large file could exhaust memory.🛡️ Suggested size check before reading
let name = path .file_name() .and_then(|value| value.to_str()) .map(String::from) .ok_or_else(|| "Dropped file name is invalid".to_string())?; + + const MAX_FILE_SIZE: u64 = 100 * 1024 * 1024; // 100 MB + let metadata = std::fs::metadata(&path) + .map_err(|error| format!("Failed to read file metadata for {name}: {error}"))?; + if metadata.len() > MAX_FILE_SIZE { + return Err(format!("File {name} exceeds maximum size of 100 MB")); + } + let bytes = std::fs::read(&path) .map_err(|error| format!("Failed to read dropped file {name}: {error}"))?;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src-tauri/src/lib.rs` around lines 172 - 202, The read_dropped_files function reads entire files into memory without validating their size, which could cause memory exhaustion with large files. Before calling std::fs::read(&path), add a file size check by using std::fs::metadata to get the file's size and compare it against a maximum file size limit (matching the frontend's 100 MB limit defined in attachments.ts). If the file exceeds the limit, return an appropriate error message instead of proceeding with the read operation.wework/src/components/chat/composer/ProjectChatComposer.tsx (1)
104-137: 💤 Low valueStale
disabledoronFileSelectmay be captured in the event handler.The
onDragDropEventcallback capturesdisabledandonFileSelectfrom the closure, but these values are read inside an async event handler that persists across renders. IfdisabledoronFileSelectchanges, the listener isn't re-registered until the effect re-runs (and the old listener is cleaned up).This is mostly fine since the effect deps include both values, but there's a brief window where the old callback could fire with stale values before cleanup completes. Consider using refs for these values if this causes issues.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/components/chat/composer/ProjectChatComposer.tsx` around lines 104 - 137, The onDragDropEvent callback captures disabled and onFileSelect from the closure, which can be stale if these props change. Create refs using useRef for both disabled and onFileSelect values, then update these refs in a separate useEffect that depends on these values. Inside the listenForNativeFileDrops function within the onDragDropEvent callback, access disabled and onFileSelect through their refs (using ref.current) instead of capturing them from the closure. This ensures the event handler always uses the current values rather than stale ones.wework/src/api/attachments.ts (1)
48-55: 💤 Low valueTauri upload path only reports 0% and 100% progress.
The Tauri fetch path doesn't support granular upload progress tracking, unlike the XHR path which reports actual progress. This is a known limitation of
@tauri-apps/plugin-http. Users may experience a jump from 0% to 100% with no intermediate feedback for large files.Consider documenting this limitation or showing an indeterminate progress indicator in the UI when running under Tauri.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wework/src/api/attachments.ts` around lines 48 - 55, The Tauri fetch path in the shouldUseTauriFetch() block currently only reports progress at 0% and 100%, which is a limitation of the `@tauri-apps/plugin-http` library that doesn't support granular upload progress tracking. Add a code comment above the shouldUseTauriFetch() conditional block to document this limitation and explain that the onProgress callback will only receive two events (0% and 100%) for Tauri-based uploads. Optionally, consider using a special progress value or flag to signal to the caller that the progress tracking is indeterminate when using Tauri, so the UI can adapt accordingly (such as showing a loading spinner instead of a progress bar).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wework/src-tauri/tauri.conf.json`:
- Line 21: The configuration value for dragDropEnabled in tauri.conf.json is set
to false, but the test in macos-window-chrome.test.ts expects it to be true.
Given the test name 'enables native file drop events in the desktop webview'
indicates drag-drop should be enabled, update the dragDropEnabled configuration
from false to true to align with the test expectations and the intended
behavior.
In `@wework/src/components/common/BranchSelector.tsx`:
- Around line 165-169: The t() function calls in the BranchSelector component
(appearing at three locations with setError calls and other message assignments)
include inline fallback literals as the second parameter, which hides missing
translations from being detected. Remove the second string parameter (the
Chinese fallback text like '分支加载失败') from each t() call so they only pass the
translation key, and ensure those translation keys are properly added to the
locale files for all supported languages to provide the translations instead of
relying on inline fallbacks.
In `@wework/src/test/macos-window-chrome.test.ts`:
- Around line 34-40: The test assertion in the 'enables native file drop events
in the desktop webview' test is checking that mainWindow.dragDropEnabled is
true, but according to the drag-and-drop design, the webview drag-drop must be
disabled to allow Tauri native drop events to be handled consistently. Change
the expect statement for mainWindow.dragDropEnabled from toBe(true) to
toBe(false) to match the intended behavior where native file drop events are
enabled by disabling the webview's built-in drag-drop functionality.
---
Nitpick comments:
In `@wework/src-tauri/src/lib.rs`:
- Around line 172-202: The read_dropped_files function reads entire files into
memory without validating their size, which could cause memory exhaustion with
large files. Before calling std::fs::read(&path), add a file size check by using
std::fs::metadata to get the file's size and compare it against a maximum file
size limit (matching the frontend's 100 MB limit defined in attachments.ts). If
the file exceeds the limit, return an appropriate error message instead of
proceeding with the read operation.
In `@wework/src/api/attachments.ts`:
- Around line 48-55: The Tauri fetch path in the shouldUseTauriFetch() block
currently only reports progress at 0% and 100%, which is a limitation of the
`@tauri-apps/plugin-http` library that doesn't support granular upload progress
tracking. Add a code comment above the shouldUseTauriFetch() conditional block
to document this limitation and explain that the onProgress callback will only
receive two events (0% and 100%) for Tauri-based uploads. Optionally, consider
using a special progress value or flag to signal to the caller that the progress
tracking is indeterminate when using Tauri, so the UI can adapt accordingly
(such as showing a loading spinner instead of a progress bar).
In `@wework/src/components/chat/composer/ProjectChatComposer.tsx`:
- Around line 104-137: The onDragDropEvent callback captures disabled and
onFileSelect from the closure, which can be stale if these props change. Create
refs using useRef for both disabled and onFileSelect values, then update these
refs in a separate useEffect that depends on these values. Inside the
listenForNativeFileDrops function within the onDragDropEvent callback, access
disabled and onFileSelect through their refs (using ref.current) instead of
capturing them from the closure. This ensures the event handler always uses the
current values rather than stale ones.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7750c951-b141-46f2-a992-659733c8adad
📒 Files selected for processing (14)
wework/scripts/dev-mac-app.shwework/src-tauri/src/lib.rswework/src-tauri/tauri.conf.jsonwework/src/App.apps.test.tsxwework/src/api/attachments.test.tswework/src/api/attachments.tswework/src/api/http.tswework/src/components/chat/ChatInput.test.tsxwework/src/components/chat/composer/ProjectChatComposer.tsxwework/src/components/chat/composer/WorktreeBranchSelector.tsxwework/src/components/common/BranchSelector.tsxwework/src/lib/floating-menu.tswework/src/pages/AppsPage.tsxwework/src/test/macos-window-chrome.test.ts
Summary by CodeRabbit
New Features
Improvements
Tests
/appslayout, attachment API behavior, chat input dropped files, and branch menu sizing/behavior.