Conversation
…o Soft & Elevated design" This reverts commit 57874af.
…rch IO, and implement async IndexedDB thumbnail cache
…n focus lock, and redesign sync modal
- Add click handler to `.active-action--progress` to trigger reopening of progress modal - Implement `reopenProgressModal` to reset dismissal state, close dropdown, and restore modal - Pre-populate progress modal immediately with current action state to avoid flickering - Style progress row with pointer cursor and glassmorphic hover effects - Replace progress modal large icon spinner with elegant 16px CSS spinner (`.preloader-small-invert`) - Update success state to replace spinner with Font Awesome checkmark - Update developer onboarding guidelines in `AGENTS.md`
…out spacing alignments - Upgraded Tauri and plugins dependencies to v2.0 in Cargo.toml. - Standardized window layouts, CLI commands, and scopes in tauri.conf.json. - Implemented secure capabilities/permissions manifest mapping all 71 custom Rust commands and core/plugin resources. - Upgraded backend main window signatures, window effects, and path utilities. - Upgraded frontend main logic, IPC cores, OS normalization, and keyboard arrow navigation. - Fixed layout double-spacing double offsets on explorer, dual-pane, and miller columns. - Standardized drag start and OS drop event handling under v2 (tauri://drag-over, tauri://drag-drop, and tauri://drag-leave using Channel API and document.elementFromPoint hit-tester).
…d design refinements
|
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:
📝 WalkthroughSummary by CodeRabbit
WalkthroughMigrates to Tauri v2, adds an FTP discovery/operations subsystem, refactors backend progress/size logic and macOS window helpers, and implements major frontend rewrites (context menu, events, IndexedDB thumbnail cache, progress modal, settings/modals), theme tokens, CI tweaks, and extensive docs/specs/reviews. ChangesBackend: Tauri v2 Migration & FTP Module
Frontend: UI/UX Overhaul & Event Handling
Documentation: Developer Guides, Planning & Review
CI & Tools
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src-tauri/src/utils.rs (1)
809-845: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftUse the repo-standard parallel walker here.
This reintroduces a manual recursive
fs::read_dirwalk on a hot path. On large trees, that bypasses the project'sjwalk/rayonconvention and will be noticeably slower than the parallel traversal already used elsewhere in this file.As per coding guidelines,
src-tauri/src/utils.rs: "Use jwalk and walkdir crates for highly parallel directory traversals; leverage rayon for multi-threaded operation mapping in Rust backend".🤖 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 `@src-tauri/src/utils.rs` around lines 809 - 845, The manual recursive fs::read_dir loop in dir_info_with_state bypasses the repo-standard parallel walker and must be replaced with a jwalk/rayon-based parallel traversal: use jwalk::WalkDir (or WalkDirGeneric/WalkDirParallel) to walk the given path, map entries in parallel with rayon to compute each file's size and count, aggregate totals into SimpleDirInfo (size, count_elements), honor state.should_stop() inside the traversal to early-return, and keep the existing accumulation callback accumulate_and_emit (when update_id is Some) for each file; ensure you still handle file vs dir correctly and return exact same SimpleDirInfo shape.
🟡 Minor comments (6)
AGENTS.md-12-12 (1)
12-12:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRefresh the feature list: SSHFS appears stale after FTP migration.
The guide still presents SSHFS as an active integration, while the supplied PR context indicates a move to native FTP. Please update the feature inventory to avoid onboarding drift.
🤖 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 `@AGENTS.md` at line 12, Update the "Rich features" feature list entry by removing the stale "SSHFS mount integration" phrase and replacing it with the current native FTP integration phrasing (e.g., swap "SSHFS mount integration" for "native FTP integration" or "FTP mount support"); locate the text under the "Rich features" bullet in AGENTS.md (the line containing "SSHFS mount integration") and edit that single bullet so the features list reflects FTP instead of SSHFS.ui/contextmenu.js-321-327 (1)
321-327:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHide the submenu when closing the parent menu.
hide()only animates the main menu. If a submenu is open, it can remain visible until the next hover/open cycle. CallhideSubMenu()as part of the close path too.Suggested fix
hide() { + this.hideSubMenu(); this.menu.classList.add("context-menu--closing"); this._closeTimeout = setTimeout(() => { this.menu.style.display = "none"; this.menu.classList.remove("context-menu--closing"); this._closeTimeout = null;🤖 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 `@ui/contextmenu.js` around lines 321 - 327, The hide() method currently only animates and hides the main menu; update hide() (in ui/contextmenu.js) to also close any open submenu by calling this.hideSubMenu() as part of the close path (invoke it before starting the close animation and before setting this._closeTimeout) so submenus are hidden immediately and do not remain visible after the parent is closed; ensure you still clear/reset this._closeTimeout and remove the "context-menu--closing" class as before.ui/events.js-139-145 (1)
139-145:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEscape
pathbefore using it inquerySelector.A valid filename can contain
",], or other selector metacharacters, so this lookup can throw and stop rename handling for that event. UseCSS.escape(path)before interpolating it into the attribute selector.Suggested fix
- const existsInUi = document.querySelector(`[itempath="${path}"]`); + const existsInUi = document.querySelector( + `[itempath="${CSS.escape(path)}"]`, + );🤖 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 `@ui/events.js` around lines 139 - 145, The attribute selector uses raw path which can contain characters that break querySelector; before interpolating, call CSS.escape(path) and use the escaped value in document.querySelector to locate the element (replace the current document.querySelector(`[itempath="${path}"]`) usage). Update the lookup that sets existsInUi and any other places that build an attribute selector from path (e.g., where handleDynamicRemove/handleDynamicCreate are invoked based on that lookup) to use the escaped string.src-tauri/src/bin/test_prompt.rs-1-18 (1)
1-18:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake the probe fail loudly.
This helper exits with success even when bind, broadcast setup, or send fails, which makes network-consent verification unreliable in scripts.
Suggested fix
-fn main() { +fn main() -> std::io::Result<()> { println!("Testing UDP broadcast to 255.255.255.255..."); - if let Ok(socket) = std::net::UdpSocket::bind("0.0.0.0:0") { - let _ = socket.set_broadcast(true); - match socket.send_to(b"trigger", "255.255.255.255:9") { - Ok(bytes) => println!("Sent {} bytes to broadcast", bytes), - Err(e) => println!("Error sending to broadcast: {}", e), - } - } + let socket = std::net::UdpSocket::bind("0.0.0.0:0")?; + socket.set_broadcast(true)?; + let bytes = socket.send_to(b"trigger", "255.255.255.255:9")?; + println!("Sent {} bytes to broadcast", bytes); println!("Testing UDP multicast to 224.0.0.251..."); - if let Ok(socket) = std::net::UdpSocket::bind("0.0.0.0:0") { - match socket.send_to(b"trigger", "224.0.0.251:5353") { - Ok(bytes) => println!("Sent {} bytes to multicast", bytes), - Err(e) => println!("Error sending to multicast: {}", e), - } - } + let socket = std::net::UdpSocket::bind("0.0.0.0:0")?; + let bytes = socket.send_to(b"trigger", "224.0.0.251:5353")?; + println!("Sent {} bytes to multicast", bytes); + Ok(()) }🤖 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 `@src-tauri/src/bin/test_prompt.rs` around lines 1 - 18, The test helper currently swallows errors and always exits success; modify main to fail loudly by checking results of std::net::UdpSocket::bind, socket.set_broadcast and socket.send_to and exiting with a non-zero status on any error (e.g., propagate errors or call std::process::exit(1)). Specifically, in the main function, replace the silent if let Ok(...) blocks around UdpSocket::bind, calls to set_broadcast, and send_to with error-aware logic: log/print the error using the existing println! messages and then immediately exit non-zero (or return a Err from main) when any of UdpSocket::bind, socket.set_broadcast, or socket.send_to fails so scripts can detect failure.ui/index.html-59-64 (1)
59-64:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDon't hardcode the macOS shortcut hint.
⌘Fis incorrect on Linux/Windows and can drift from the actual configurable shortcut. Render this from the active platform/shortcut mapping so the hint stays accurate.🤖 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 `@ui/index.html` around lines 59 - 64, Replace the hardcoded "⌘F" label inside the element with class file-searchbar-shortcut by setting its text at runtime from the app's shortcut mapping for the "find" action (e.g., call your shortcut registry like shortcutManager.getShortcut('find') or a helper getShortcutFor('find')), and fall back to platform-aware defaults (Ctrl+F on Windows/Linux, ⌘F on macOS using navigator.platform or platform detection). Update the HTML to leave the kbd empty or with a data-key attribute and add a small init script that queries document.querySelector('.file-searchbar-shortcut') and writes the resolved label so the displayed hint always reflects the active/configurable shortcut.ui/utils.js-14-27 (1)
14-27:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReset the cached DB promise after open failures.
If
indexedDB.open()fails once,dbPromisestays rejected and every later cache read/write in this session fails immediately. Clear the cached promise on failure before rethrowing so the cache can recover on the next attempt.Suggested fix
function getDB() { if (dbPromise) return dbPromise; dbPromise = new Promise((resolve, reject) => { const request = indexedDB.open(DB_NAME, DB_VERSION); @@ - request.onerror = (e) => reject(e.target.error); + request.onerror = (e) => { + dbPromise = null; + reject(e.target.error); + }; }); - return dbPromise; + return dbPromise.catch((error) => { + dbPromise = null; + throw 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 `@ui/utils.js` around lines 14 - 27, The cached dbPromise created in getDB stays rejected after indexedDB.open() fails, causing all subsequent calls to fail; update request.onerror (and optionally request.onblocked/onupgradeneeded error paths) to clear dbPromise (set dbPromise = null) before rejecting the promise so the cache can be retried on the next getDB call; locate getDB, dbPromise, indexedDB.open and the request.onerror handler to make this change.
🧹 Nitpick comments (2)
docs/planning/popup-glassmorphism-overhaul/implementation-tracker.md (1)
22-33: ⚡ Quick winT11/T13/T14/T15/T21 evidence is too broad to verify completion.
These rows mark tasks as completed but only cite
ui/style.csswithout line ranges, unlike the rest of the table. Add exact line spans (or section anchors) so completion claims stay auditable.🤖 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 `@docs/planning/popup-glassmorphism-overhaul/implementation-tracker.md` around lines 22 - 33, The table rows for T11, T13, T14, T15 and T21 claim "Completed" but only reference `ui/style.css` without specific line ranges or anchors; update each of those entries (T11, T13, T14, T15, T21) to include the exact line spans or section anchors in `ui/style.css` that show the restyled rules (e.g., `ui/style.css:START-END` or a named comment anchor) so reviewers can verify changes; locate the CSS selectors `.item-properties-popup`, `.find-duplicates-popup`, `.yt-download-popup`, `.llm-prompt-input-popup`, and `.search-bar-container` in the stylesheet and record their corresponding line ranges or anchor IDs in the table.docs/review/context-menu-visual-overhaul/findings.md (1)
269-272: ⚡ Quick winCR-012 severity is likely under-classified for an XSS-class finding.
If
innerHTMLinterpolation can include untrusted names, classifying it as “Minor” may deprioritize a security-relevant fix. Recalibrate severity (or explicitly bound trust assumptions) to avoid false confidence.🤖 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 `@docs/review/context-menu-visual-overhaul/findings.md` around lines 269 - 272, Update the CR-012 finding to escalate its severity from "Minor" to at least "High" (or "Medium-High") and add explicit remediation/assumption text: indicate that any use of innerHTML with untrusted label/app names is unsafe, recommend replacing innerHTML with textContent or using a proper sanitizer/escaper for those specific insertions, and state the required trust boundary for label/app name sources to avoid false confidence; reference the CR-012 identifier and the innerHTML occurrences so reviewers can locate the unsafe assignments and implement the change.
🤖 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 `@AGENTS.md`:
- Line 63: Replace the outdated platform note "**Core**: **Tauri v1**" with the
correct target runtime by changing it to "**Core**: **Tauri v2**" (or a phrasing
that explicitly states the repository is migrating to/using Tauri v2), and
ensure any adjacent explanatory sentence reflects the v2 migration described in
this PR so documentation aligns with the architecture changes.
In `@docs/design/specs/collapsible-sidebar-sections.md`:
- Around line 178-207: The toggleCollapseSection function updates only visual
state and persisted state, but never updates the ARIA state; modify
toggleCollapseSection to set the corresponding button/section element's
aria-expanded attribute to "true" when expanding and "false" when collapsing
(use sectionEl or the toggle control found inside it), and also update
aria-expanded when restoring state on load (the same logic referenced at the
restore code path around the other restore function). Ensure you set
aria-expanded before/after the CSS transition consistently so screen readers
always reflect the current expanded/collapsed state.
In `@docs/design/specs/viewmode-selector.md`:
- Around line 241-257: The switchView function currently updates only class
"active" and aria-pressed but does not maintain roving tabindex, so keyboard Tab
focus won't follow the active segmented control; update switchView (used when
IsDualPaneEnabled is false and when ViewMode changes) to set tabindex="0" on the
button whose dataset.view matches ViewMode and set tabindex="-1" on all other
".view-mode-btn" elements to implement roving-tabindex behavior, ensuring the
active control is tabbable and others are skipped by Tab.
- Around line 194-197: The .view-mode-group.disabled rule only
visually/pointer-disables the group; add a semantic disabled state by updating
the spec to require the group element expose aria-disabled="true" (and prefer
using .view-mode-group[aria-disabled="true"] in examples), remove it from the
tab order (tabindex="-1") when disabled, and ensure any interactive children
(e.g., buttons or inputs inside .view-mode-group) are either marked disabled or
have aria-hidden/aria-disabled set so assistive tech sees them as unavailable;
update the described CSS selector and the prose/examples that reference
.view-mode-group.disabled (including the other occurrences called out) to
reflect these semantic changes.
In `@docs/superpowers/plans/2026-05-20-image-background-removal.md`:
- Around line 18-21: Update the plan checklist to include explicit backend
acceptance criteria for path validation: require the remove_background command
and its handler (generate_handler! registration) to validate from_path and
output_path server-side for existence, allowed directories (whitelist/sandbox),
write permissions, and enforce .png output, and add tests/verifications in Steps
2–4 to confirm compilation and runtime rejection of malformed or tampered IPC
inputs; reference the remove_background command, generate_handler!, from_path,
and output_path in the criteria so implementers know where to add checks.
In `@docs/superpowers/specs/2026-05-16-popup-overhaul-design.md`:
- Around line 35-40: The new modal spec introduces `.soft-popup`,
`.soft-header`, and `.soft-body` that conflict with the required `.props-card`
layout; update the spec so PopupManager continues to render the mandated
`.props-card` structure (including `.props-card__hero`, `.props-card__list`,
`.props-card__footer` using the 88px 1fr grid) and describe `.soft-*` as purely
visual/utility classes layered on top of `.props-card` (e.g.,
`.props-card.soft-popup`, `.props-card__hero .soft-header`, `.props-card__list
.soft-body`), ensuring the modal primitives reference `PopupManager` and
preserve the `.props-card` DOM contract.
In `@docs/superpowers/specs/2026-05-20-image-background-removal-design.md`:
- Around line 30-33: Remove the api_key parameter from the command interface (do
not accept api_key alongside from_path and output_path); keep only
file-path/options like from_path and output_path in the payload, and change the
implementation so the backend Rust code resolves the API key from secure
storage/settings (e.g., a config manager/secure vault) before making the call,
ensuring no renderer/UI layer or IPC passes secrets such as api_key.
In `@src-tauri/src/remote/ftp.rs`:
- Around line 684-701: The loop currently calls download_ftp_dir_recursive and
download_ftp_file which each re-fetch config and call get_ftp_client, causing a
fresh login per child; change the implementations and all call sites so a single
&mut FtpStream is opened once (e.g. create client via get_ftp_client at the
top-level caller) and then passed down into download_ftp_dir_recursive and
download_ftp_file as a &mut FtpStream reference; update their signatures to
accept &mut FtpStream, remove internal reconnect logic and any duplicated config
fetching, and ensure the recursion uses the same mutable stream for all child
operations (handle lifetimes and borrow checks accordingly).
- Around line 729-743: The code calls to_str().unwrap() on Path values in
upload_ftp_dir_recursive and upload_ftp_file (e.g., passing
path.to_str().unwrap()) which can panic for non-UTF-8 filenames; change the APIs
and call sites to accept &Path or PathBuf instead of &str (update
upload_ftp_dir_recursive and upload_ftp_file signatures to take &Path),
propagate Path values (path or path.to_path_buf()) through the recursive calls,
and only convert to UTF-8 for logging or building remote names using
to_string_lossy() or explicit OsStr handling (e.g., use entry_name =
path.file_name().map(|s| s.to_string_lossy().to_string()) and avoid unwraps) so
no .to_str().unwrap() remains in the recursive upload flow.
- Around line 235-236: The code currently masks keychain lookup failures by
using get_keychain_password(&config.name).unwrap_or_else(|_| "".to_string()),
which hides the original error; change this to propagate the keychain error
instead (e.g., use the ? operator or map_err to convert into the function's
error type) so that a failed get_keychain_password(&config.name) returns the
real error rather than an empty password; update the surrounding function's
signature/return handling if necessary to return Result and reference the
password binding and get_keychain_password call when making the change.
- Around line 278-284: The code currently treats any entry whose metadata_char
is 'l' as a directory by setting is_dir = 1; change this so symlinks are not
automatically classified as directories: set is_symlink = metadata_char == 'l'
and set is_dir = metadata_char == 'd' (remove the || is_symlink logic), and if
you need to follow a symlink, resolve it explicitly later (e.g., attempt FTP CWD
or inspect the LIST target string) rather than marking it as a directory in the
parsing step (refer to metadata_char, is_symlink, is_dir and the
parts[0].chars().next()? usage).
- Around line 169-176: The code currently writes the password to the macOS
Keychain via set_keychain_password but unconditionally replaces v_clone.password
with "__keychain__", which discards the secret even when the Keychain write
fails; update the logic in the save routine (the block using v.clone(), v_clone,
set_keychain_password and "__keychain__") so that you only set v_clone.password
= "__keychain__" after set_keychain_password succeeds, and on Err either
propagate/return the error from save_connections or keep the original plain
password in the saved struct (choose propagate to fail save_connections or keep
original to preserve usability); ensure you reference set_keychain_password,
v_clone and the "__keychain__" marker when making the change.
In `@src-tauri/src/remote/ftp/ftp_discovery.rs`:
- Around line 87-103: When get_local_ipv4() returns None the current fallback
only scans 192.168.1.0/24; update the fallback in ftp_discovery.rs to iterate
common private ranges instead: add scanning for 192.168.0.0/24 (and keep .1/24),
the 10.0.0.0/8 space (practical scan strategy: at least 10.x.0.1..254 for common
x values or configurable subnet stepping), and the 172.16.0.0/12 block (e.g.
172.16..172.31 .0/24 ranges), pushing the same ports (21 and 2121) into the
targets Vec; modify the fallback branch that currently builds targets so it
generates addresses for these additional private ranges while keeping existing
behavior for loop structure and target pushing.
In `@src-tauri/src/utils.rs`:
- Around line 800-802: The capped traversal currently only updates
state.total_size and flips limit_reached when update_id is Some because
accumulate_and_emit(id) is only called inside that branch; ensure the size
accounting and limit check run regardless of update_id by always adding the
current entry size to state.total_size and setting state.limit_reached when
state.total_size >= SIZE_CALC_LIMIT_BYTES, while keeping
accumulate_and_emit(size, count, id, state) calls only for emitting when
update_id.is_some(); apply the same change to the other occurrences referenced
in dir_info_incremental_capped so traversal stops at SIZE_CALC_LIMIT_BYTES even
when update_id is None.
In `@ui/contextmenu.js`:
- Around line 390-393: The "Eject Disk" label is renamed to "Unmount" for
FTP/SSHFS mounts but the disable logic still hides the action unless
itemisremovable === "1"; update the disable check that controls the menu item's
enabled state (the code that reads getAttribute("itemisremovable") and uses
labelText === "Eject Disk") to also treat remote mounts as actionable by
inspecting the same path string used earlier
(this.selectedItem.getAttribute("itempath") and testing startsWith("ftp://"),
includes("sshfs") or startsWith("/tmp/codriver-sshfs-mount")); ensure you apply
this change to both places where the relabeling occurs so the Unmount action
remains visible for remote mounts even when itemisremovable !== "1".
- Around line 439-447: The code injects external app names via innerHTML (inside
the Applications.forEach block where subItemButton is built when item.label ===
"Open with"), which risks DOM injection; instead create the child elements
(e.g., a span with class "context-item-group" and a nested span with class
"context-label") using document.createElement, set the label span's textContent
= app[0], and append those elements to subItemButton (leave
subItemButton.className = "context-item" in place) so no untrusted string is
passed to innerHTML.
In `@ui/index.html`:
- Around line 580-733: The theme editor is missing the new token
sidebar_top_blur_overlay_color so custom themes can't round-trip; add a new
props-card row (matching existing rows like the Sidebar / site_bar_color and
Header Nav / nav_bar_color) that exposes a color input, hex text input and
opacity slider for sidebar_top_blur_overlay_color (use classes analogous to
theme-sitebar-color, theme-sitebar-color-text, theme-sitebar-color-opacity but
for the overlay, e.g. theme-sidebar-top-blur-overlay-color,
theme-sidebar-top-blur-overlay-color-text,
theme-sidebar-top-blur-overlay-color-opacity) and ensure the save/load code that
reads/writes theme values includes this new token name so the editor round-trips
the bundled theme schema.
- Around line 509-555: The inputs in the modal are missing accessible labels for
assistive tech—add unique id attributes to each input (e.g.,
id="full-dualpane-search-input", "full-dualpane-search-file-content-input",
"full-search-max-items-input", "full-search-search-depth-input") and connect
them to their visible <dt> labels either by turning the <dt> text into <label
for="..."> or by adding aria-labelledby on the input pointing to the
corresponding label element id; ensure ids are unique across the page and apply
the same pattern for the other modal sections referenced (lines 580-733,
798-845, 885-956) so every input/select has a programmatic label.
In `@ui/models.js`:
- Around line 24-28: The HTML currently interpolates this.id into an inline
onclick handler (onclick="reopenProgressModal('...')") which is unsafe; instead
remove the inline onclick and output a data-action-id attribute (e.g.,
data-action-id="${this.id}") on the progress element (the branch guarded by
this.isProgress that produces the active-action--progress div) and implement a
delegated click handler elsewhere (module initialization) that calls
reopenProgressModal(event.currentTarget.dataset.actionId) for elements matching
the .active-action--progress selector; this avoids JS string-escaping issues and
prevents injection while keeping the same behavior.
In `@ui/utils.js`:
- Around line 791-830: The template injects user-controlled currentFile into
modal.innerHTML which can lead to HTML injection; instead stop interpolating
currentFile into the innerHTML string and create/set the node text and title via
DOM APIs: locate where modal.innerHTML is set and replace the element with class
"progress-modal-current-file" so that you create a dd (or query the appended
modal and querySelector('.progress-modal-current-file')) and set its textContent
= currentFile and its title = currentFile (remove ${currentFile} from the
template), leaving other template parts unchanged; ensure any other uses of
currentFile in this block use textContent/title rather than string
interpolation.
---
Outside diff comments:
In `@src-tauri/src/utils.rs`:
- Around line 809-845: The manual recursive fs::read_dir loop in
dir_info_with_state bypasses the repo-standard parallel walker and must be
replaced with a jwalk/rayon-based parallel traversal: use jwalk::WalkDir (or
WalkDirGeneric/WalkDirParallel) to walk the given path, map entries in parallel
with rayon to compute each file's size and count, aggregate totals into
SimpleDirInfo (size, count_elements), honor state.should_stop() inside the
traversal to early-return, and keep the existing accumulation callback
accumulate_and_emit (when update_id is Some) for each file; ensure you still
handle file vs dir correctly and return exact same SimpleDirInfo shape.
---
Minor comments:
In `@AGENTS.md`:
- Line 12: Update the "Rich features" feature list entry by removing the stale
"SSHFS mount integration" phrase and replacing it with the current native FTP
integration phrasing (e.g., swap "SSHFS mount integration" for "native FTP
integration" or "FTP mount support"); locate the text under the "Rich features"
bullet in AGENTS.md (the line containing "SSHFS mount integration") and edit
that single bullet so the features list reflects FTP instead of SSHFS.
In `@src-tauri/src/bin/test_prompt.rs`:
- Around line 1-18: The test helper currently swallows errors and always exits
success; modify main to fail loudly by checking results of
std::net::UdpSocket::bind, socket.set_broadcast and socket.send_to and exiting
with a non-zero status on any error (e.g., propagate errors or call
std::process::exit(1)). Specifically, in the main function, replace the silent
if let Ok(...) blocks around UdpSocket::bind, calls to set_broadcast, and
send_to with error-aware logic: log/print the error using the existing println!
messages and then immediately exit non-zero (or return a Err from main) when any
of UdpSocket::bind, socket.set_broadcast, or socket.send_to fails so scripts can
detect failure.
In `@ui/contextmenu.js`:
- Around line 321-327: The hide() method currently only animates and hides the
main menu; update hide() (in ui/contextmenu.js) to also close any open submenu
by calling this.hideSubMenu() as part of the close path (invoke it before
starting the close animation and before setting this._closeTimeout) so submenus
are hidden immediately and do not remain visible after the parent is closed;
ensure you still clear/reset this._closeTimeout and remove the
"context-menu--closing" class as before.
In `@ui/events.js`:
- Around line 139-145: The attribute selector uses raw path which can contain
characters that break querySelector; before interpolating, call CSS.escape(path)
and use the escaped value in document.querySelector to locate the element
(replace the current document.querySelector(`[itempath="${path}"]`) usage).
Update the lookup that sets existsInUi and any other places that build an
attribute selector from path (e.g., where
handleDynamicRemove/handleDynamicCreate are invoked based on that lookup) to use
the escaped string.
In `@ui/index.html`:
- Around line 59-64: Replace the hardcoded "⌘F" label inside the element with
class file-searchbar-shortcut by setting its text at runtime from the app's
shortcut mapping for the "find" action (e.g., call your shortcut registry like
shortcutManager.getShortcut('find') or a helper getShortcutFor('find')), and
fall back to platform-aware defaults (Ctrl+F on Windows/Linux, ⌘F on macOS using
navigator.platform or platform detection). Update the HTML to leave the kbd
empty or with a data-key attribute and add a small init script that queries
document.querySelector('.file-searchbar-shortcut') and writes the resolved label
so the displayed hint always reflects the active/configurable shortcut.
In `@ui/utils.js`:
- Around line 14-27: The cached dbPromise created in getDB stays rejected after
indexedDB.open() fails, causing all subsequent calls to fail; update
request.onerror (and optionally request.onblocked/onupgradeneeded error paths)
to clear dbPromise (set dbPromise = null) before rejecting the promise so the
cache can be retried on the next getDB call; locate getDB, dbPromise,
indexedDB.open and the request.onerror handler to make this change.
---
Nitpick comments:
In `@docs/planning/popup-glassmorphism-overhaul/implementation-tracker.md`:
- Around line 22-33: The table rows for T11, T13, T14, T15 and T21 claim
"Completed" but only reference `ui/style.css` without specific line ranges or
anchors; update each of those entries (T11, T13, T14, T15, T21) to include the
exact line spans or section anchors in `ui/style.css` that show the restyled
rules (e.g., `ui/style.css:START-END` or a named comment anchor) so reviewers
can verify changes; locate the CSS selectors `.item-properties-popup`,
`.find-duplicates-popup`, `.yt-download-popup`, `.llm-prompt-input-popup`, and
`.search-bar-container` in the stylesheet and record their corresponding line
ranges or anchor IDs in the table.
In `@docs/review/context-menu-visual-overhaul/findings.md`:
- Around line 269-272: Update the CR-012 finding to escalate its severity from
"Minor" to at least "High" (or "Medium-High") and add explicit
remediation/assumption text: indicate that any use of innerHTML with untrusted
label/app names is unsafe, recommend replacing innerHTML with textContent or
using a proper sanitizer/escaper for those specific insertions, and state the
required trust boundary for label/app name sources to avoid false confidence;
reference the CR-012 identifier and the innerHTML occurrences so reviewers can
locate the unsafe assignments and implement the change.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 82c52d83-dd6c-413a-8fc1-e764bf7099d4
⛔ Files ignored due to path filters (4)
src-tauri/Cargo.lockis excluded by!**/*.locksrc-tauri/gen/schemas/acl-manifests.jsonis excluded by!**/gen/**src-tauri/gen/schemas/capabilities.jsonis excluded by!**/gen/**src-tauri/gen/schemas/macOS-schema.jsonis excluded by!**/gen/**
📒 Files selected for processing (43)
AGENTS.mddocs/design/specs/collapsible-sidebar-sections.mddocs/design/specs/viewmode-selector.mddocs/planning/disk-sidebar-fixes/decisions.mddocs/planning/disk-sidebar-fixes/handoffs.mddocs/planning/disk-sidebar-fixes/implementation-tracker.mddocs/planning/disk-sidebar-fixes/plan.mddocs/planning/popup-glassmorphism-overhaul/decisions.mddocs/planning/popup-glassmorphism-overhaul/handoffs.mddocs/planning/popup-glassmorphism-overhaul/implementation-tracker.mddocs/planning/popup-glassmorphism-overhaul/plan.mddocs/review/collapsible-sidebar/findings.mddocs/review/collapsible-sidebar/summary.mddocs/review/context-menu-visual-overhaul/findings.mddocs/review/context-menu-visual-overhaul/summary.mddocs/review/popup-overhaul/findings.mddocs/review/popup-overhaul/summary.mddocs/superpowers/plans/2026-05-20-image-background-removal.mddocs/superpowers/specs/2026-05-16-popup-overhaul-design.mddocs/superpowers/specs/2026-05-20-image-background-removal-design.mdsrc-tauri/Cargo.tomlsrc-tauri/Info.plistsrc-tauri/capabilities/migrated.jsonsrc-tauri/permissions/app-commands.tomlsrc-tauri/src/bin/test_prompt.rssrc-tauri/src/main.rssrc-tauri/src/remote/ftp.rssrc-tauri/src/remote/ftp/ftp_discovery.rssrc-tauri/src/remote/mod.rssrc-tauri/src/utils.rssrc-tauri/src/window_tauri_ext.rssrc-tauri/tauri.conf.jsonthemes/Dark.jsonthemes/Default.jsonthemes/Hacker.jsonthemes/Light.jsonui/contextmenu.jsui/events.jsui/index.htmlui/main_logic.jsui/models.jsui/style.cssui/utils.js
…ad directory after file copy
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src-tauri/src/remote/ftp.rs (1)
624-655:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftUploads still reconnect once per file/directory.
upload_ftp_filestill looks up the config and opens a newFtpStreamon every call, soupload_ftp_dir_recursiveandcopy_ftp_to_ftpstill do O(n) logins across a tree. On large transfers this will be slow and can hit server connection limits; the upload path should mirror the new download-side&mut FtpStreamflow.Refactor sketch
-pub fn upload_ftp_file( - connection_name: &str, - local_src: &Path, - remote_dest: &str, - progress_callback: Option<&FtpProgressCallback>, -) -> Result<(), String> { - let config = { - let conns = FTP_CONNECTIONS.lock().unwrap(); - conns.get(connection_name).cloned() - } - .ok_or_else(|| format!("Connection {} not found", connection_name))?; - +pub fn upload_ftp_file( + client: &mut FtpStream, + local_src: &Path, + remote_dest: &str, + progress_callback: Option<&FtpProgressCallback>, +) -> Result<(), String> { let mut local_file = File::open(local_src).map_err(|e| format!("Failed to open local source file: {}", e))?; - - let mut client = get_ftp_client(&config)?; let raw_dest = to_raw_ftp_path(remote_dest); @@ -pub fn upload_ftp_dir_recursive( - connection_name: &str, - local_dir: &Path, - remote_dest_dir: &str, - progress_callback: Option<&FtpProgressCallback>, -) -> Result<(), String> { - let config = { - let conns = FTP_CONNECTIONS.lock().unwrap(); - conns.get(connection_name).cloned() - } - .ok_or_else(|| format!("Connection {} not found", connection_name))?; - - let mut client = get_ftp_client(&config)?; +pub fn upload_ftp_dir_recursive( + client: &mut FtpStream, + local_dir: &Path, + remote_dest_dir: &str, + progress_callback: Option<&FtpProgressCallback>, +) -> Result<(), String> { @@ - upload_ftp_dir_recursive( - connection_name, + upload_ftp_dir_recursive( + client, &path, &remote_item_path, progress_callback, )?; } else { upload_ftp_file( - connection_name, + client, &path, &remote_item_path, progress_callback, )?; }Also applies to: 697-740, 743-775
🤖 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 `@src-tauri/src/remote/ftp.rs` around lines 624 - 655, upload_ftp_file currently opens a new FtpStream per call causing O(n) logins; change it to accept and use an existing mutable client instead of looking up config/get_ftp_client inside the function. Specifically, update upload_ftp_file to take a &mut FtpStream (or whatever concrete client type returned by get_ftp_client) rather than connection_name, remove the FTP_CONNECTIONS lookup and get_ftp_client call, keep local file opening and the CallbackReader logic, and call client.put_file on that &mut FtpStream; then update callers upload_ftp_dir_recursive and copy_ftp_to_ftp to acquire one FtpStream via get_ftp_client and pass &mut client through the recursive/file loop so the same connection is reused for all uploads. Ensure progress_callback handling remains unchanged and adjust signatures where necessary.
🤖 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 `@src-tauri/src/utils.rs`:
- Around line 695-699: The code increments state.total_size and then compares
against the hardcoded SIZE_CALC_LIMIT_BYTES; instead use the configured per-call
cap held in SizeCalcState (e.g., state.limit_bytes) when deciding to set
state.limit_reached in dir_info_incremental_capped (and the similar branch at
lines ~734-738). Replace the direct comparison to SIZE_CALC_LIMIT_BYTES with a
check that uses state.limit_bytes (or falls back to SIZE_CALC_LIMIT_BYTES only
if limit_bytes is None), and set limit_reached when state.total_size >= the
resolved cap.
In `@ui/events.js`:
- Around line 171-184: The isRemote helper currently only checks
"/tmp/codriver-sshfs-mount" and misses macOS paths under
"/private/tmp/codriver-sshfs-mount", causing stale panes; update the isRemote
predicate in events.js to also treat paths that start with
"/private/tmp/codriver-sshfs-mount" as remote (so LeftDualPanePath,
RightDualPanePath and CurrentDir are correctly detected), leaving the rest of
the logic (refreshBothViews and SelectedItemPaneSide handling) unchanged.
---
Outside diff comments:
In `@src-tauri/src/remote/ftp.rs`:
- Around line 624-655: upload_ftp_file currently opens a new FtpStream per call
causing O(n) logins; change it to accept and use an existing mutable client
instead of looking up config/get_ftp_client inside the function. Specifically,
update upload_ftp_file to take a &mut FtpStream (or whatever concrete client
type returned by get_ftp_client) rather than connection_name, remove the
FTP_CONNECTIONS lookup and get_ftp_client call, keep local file opening and the
CallbackReader logic, and call client.put_file on that &mut FtpStream; then
update callers upload_ftp_dir_recursive and copy_ftp_to_ftp to acquire one
FtpStream via get_ftp_client and pass &mut client through the recursive/file
loop so the same connection is reused for all uploads. Ensure progress_callback
handling remains unchanged and adjust signatures where necessary.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 854ef12f-eb14-4afd-ae2f-c64b335e05cc
📒 Files selected for processing (5)
src-tauri/src/main.rssrc-tauri/src/remote/ftp.rssrc-tauri/src/utils.rsui/events.jsui/utils.js
Comment on lines
+695
to
+699
| state.total_size += size; | ||
| state.total_count += 1; | ||
| if state.limit_bytes.is_some() && state.total_size >= SIZE_CALC_LIMIT_BYTES { | ||
| state.limit_reached = true; | ||
| } |
There was a problem hiding this comment.
Use the configured size cap instead of the global 10 GB constant.
SizeCalcState::new(Some(limit)) and dir_info_incremental_capped(...) now expose a caller-defined cap, but these branches still stop only at SIZE_CALC_LIMIT_BYTES. Any caller passing a different threshold gets incorrect traversal behavior.
Suggested fix
- if state.limit_bytes.is_some() && state.total_size >= SIZE_CALC_LIMIT_BYTES {
- state.limit_reached = true;
- }
+ if let Some(limit_bytes) = state.limit_bytes {
+ if state.total_size >= limit_bytes {
+ state.limit_reached = true;
+ }
+ }
@@
- if state.limit_bytes.is_some() && state.total_size >= SIZE_CALC_LIMIT_BYTES {
- state.limit_reached = true;
- }
+ if let Some(limit_bytes) = state.limit_bytes {
+ if state.total_size >= limit_bytes {
+ state.limit_reached = true;
+ }
+ }Also applies to: 734-738
🤖 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 `@src-tauri/src/utils.rs` around lines 695 - 699, The code increments
state.total_size and then compares against the hardcoded SIZE_CALC_LIMIT_BYTES;
instead use the configured per-call cap held in SizeCalcState (e.g.,
state.limit_bytes) when deciding to set state.limit_reached in
dir_info_incremental_capped (and the similar branch at lines ~734-738). Replace
the direct comparison to SIZE_CALC_LIMIT_BYTES with a check that uses
state.limit_bytes (or falls back to SIZE_CALC_LIMIT_BYTES only if limit_bytes is
None), and set limit_reached when state.total_size >= the resolved cap.
Comment on lines
+171
to
+184
| // Auto-reload remote directories (FTP/SSHFS) if copy/move finishes and either pane/current folder is remote | ||
| setTimeout(async () => { | ||
| const isRemote = (p) => p && (p.startsWith("ftp://") || p.includes("sshfs") || p.startsWith("/tmp/codriver-sshfs-mount")); | ||
| if (typeof IsDualPaneEnabled !== "undefined" && IsDualPaneEnabled) { | ||
| const leftIsRemote = typeof LeftDualPanePath !== "undefined" && isRemote(LeftDualPanePath); | ||
| const rightIsRemote = typeof RightDualPanePath !== "undefined" && isRemote(RightDualPanePath); | ||
| if (leftIsRemote || rightIsRemote) { | ||
| if (typeof refreshBothViews === "function") { | ||
| await refreshBothViews(typeof SelectedItemPaneSide !== "undefined" ? SelectedItemPaneSide : ""); | ||
| } | ||
| } | ||
| } else { | ||
| const currentIsRemote = typeof CurrentDir !== "undefined" && isRemote(CurrentDir); | ||
| if (currentIsRemote) { |
There was a problem hiding this comment.
Handle the /private/tmp SSHFS mount prefix here too.
This helper only treats /tmp/codriver-sshfs-mount as remote, so macOS panes that still hold /private/tmp/codriver-sshfs-mount/... will skip the post-copy refresh and stay stale.
Suggested fix
- const isRemote = (p) => p && (p.startsWith("ftp://") || p.includes("sshfs") || p.startsWith("/tmp/codriver-sshfs-mount"));
+ const isRemote = (p) => p && (
+ p.startsWith("ftp://") ||
+ p.includes("sshfs") ||
+ p.startsWith("/tmp/codriver-sshfs-mount") ||
+ p.startsWith("/private/tmp/codriver-sshfs-mount")
+ );🤖 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 `@ui/events.js` around lines 171 - 184, The isRemote helper currently only
checks "/tmp/codriver-sshfs-mount" and misses macOS paths under
"/private/tmp/codriver-sshfs-mount", causing stale panes; update the isRemote
predicate in events.js to also treat paths that start with
"/private/tmp/codriver-sshfs-mount" as remote (so LeftDualPanePath,
RightDualPanePath and CurrentDir are correctly detected), leaving the rest of
the logic (refreshBothViews and SelectedItemPaneSide handling) unchanged.
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.
📌 Overview
This pull request brings massive enhancements to CoDriver (v0.7.2) across core capabilities, AI integration, remote protocol mounts, visual personalization, and desktop performance optimization. We have overhauled the legacy SSHFS mounts to a highly robust native FTP integration, integrated vision-to-generation AI workflows, replaced slow full-directory reloads with dynamic watch-events, and standardized the aesthetic language.
🛠️ Feature Walkthrough
📡 Native FTP Network Drives Integration
suppaftpcrate running under Passive mode."CoDriver-FTP"), saving only__keychain__placeholders in config JSONs.🧠 Semantic AI Integration (Gemini & OpenAI)
16:9,9:16,1:1).⚡ Lightning-Fast Filesystem Watcher reloading
notifycrate.handleDynamicCreate) on file insertion, sweeps targeted items (handleDynamicRemove) out of the DOM on deletion, and re-binds renames (old-to-new) and attributes in-place.🎨 Visual Refinements & Aesthetics Overhaul
Cmd,Ctrl,Alt,Shift) to capture custom shortcuts safely in event capturing mode..props-cardpattern displaying via native flexbox rules.concurrency = 8), and output a complete deletion overview details layout.📂 Key Files Modified & Added
suppaftp)0.7.2)