feat(download): Open file/folder actions for completed downloads (task 08) - #101
Merged
Conversation
…nloads (task 08) Expose two Tauri IPC commands (`download_open_file`, `download_open_folder`) that launch a completed download with the OS default app or reveal it in the host file manager. A new `FileOpener` domain port, implemented by `SystemFileOpener`, dispatches to `xdg-open` on Linux, `open`/`open -R` on macOS and `explorer`/`explorer /select,<path>` on Windows. Application handlers validate the download is in `Completed` state and surface `DomainError::NotFound` when the file is missing — the frontend mutation maps that to a localized "File not found" toast. UI adds the actions in the downloads row dropdown (Completed rows only) and as buttons in the detail panel's File info section.
📝 WalkthroughWalkthroughAdds cross-platform "open file" and "open folder" functionality: new FileOpener domain port, SystemFileOpener adapter, command handlers, Tauri IPC commands, wiring into CommandBus, frontend buttons/menu items, i18n strings, and tests. Changes
Sequence DiagramsequenceDiagram
participant FE as Frontend (React)
participant IPC as Tauri IPC
participant CB as CommandBus / Handler
participant Repo as Download Repository
participant Port as FileOpener Port
participant OS as Operating System
FE->>IPC: download_open_file(id)
IPC->>CB: handle_open_download_file(cmd)
CB->>Repo: find_by_id(id)
Repo-->>CB: download or NotFound
CB->>CB: validate download.state == Completed
CB->>Port: open_file(destination_path) (spawn_blocking)
Port->>OS: spawn native opener (xdg-open/open/start)
OS-->>Port: result / exit code
Port-->>CB: Ok or DomainError
CB-->>IPC: Ok or AppError (mapped to String)
IPC-->>FE: Ok or error string
FE->>FE: show toast if error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 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 docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR implements "Open file" and "Open folder" actions for completed downloads across the full stack: a new Confidence Score: 5/5Safe to merge — previous P1 issues are resolved and all remaining findings are P2 or informational. The two blocking issues from the previous review (spawn_blocking missing, Windows path quoting) are both fixed in this revision. No new P0 or P1 bugs were found. The openFileMissing key reuse for folder not-found errors and the test scaffold duplication are pre-existing P2 concerns already captured in prior review threads. No files require special attention for merge readiness.
|
| Filename | Overview |
|---|---|
| src-tauri/src/adapters/driven/filesystem/file_opener.rs | New SystemFileOpener adapter with per-OS launchers; blocking calls correctly wrapped via spawn_blocking in handlers; Windows /select, two-arg approach addresses previous path-with-spaces concern |
| src-tauri/src/application/commands/open_download_file.rs | Handler validates Completed state, wraps blocking open_file in spawn_blocking, maps NotFound; ~200-line test scaffold duplicated from open_download_folder.rs (noted in previous review) |
| src-tauri/src/application/commands/open_download_folder.rs | Symmetric to open_download_file handler; uses reveal_file with graceful fallback when file is gone; same duplicated test scaffold issue |
| src-tauri/src/application/command_bus.rs | Adds optional file_opener field via builder pattern matching checksum_computer; no breaking changes to existing fixtures |
| src-tauri/src/adapters/driving/tauri_ipc.rs | Two new Tauri commands download_open_file and download_open_folder wired correctly to handlers; Result<(), String> signatures consistent with other IPC commands |
| src/views/DownloadsView/DownloadsTable.tsx | Adds openFile/openFolder to RowActions context; Completed-only menu items with mutations; openFileMissing key reused for folder not-found case (flagged in prior review) |
| src/views/DownloadDetailsPanel/FileInfoSection.tsx | Adds Open file and Open folder buttons conditionally on Completed state; same openFileMissing/openFolderError inconsistency as DownloadsTable (noted in prior review) |
| src/i18n/locales/en.json | Adds openFile/openFolder action labels and toast keys; missing openFolderMissing key (covered by prior review comment) |
| src/i18n/locales/fr.json | French translations added symmetrically with en.json |
Sequence Diagram
sequenceDiagram
participant UI as Frontend (React)
participant IPC as Tauri IPC
participant Bus as CommandBus
participant Repo as DownloadRepository
participant BTP as spawn_blocking
participant OS as SystemFileOpener
UI->>IPC: download_open_file(id) / download_open_folder(id)
IPC->>Bus: handle_open_download_file / handle_open_download_folder
Bus->>Repo: find_by_id(id)
Repo-->>Bus: Download (or NotFound)
Bus->>Bus: validate state == Completed
Bus->>BTP: spawn_blocking(opener.open_file / reveal_file)
BTP->>OS: open_file(path) / reveal_file(path)
OS->>OS: check path.exists()
OS-->>BTP: Ok(()) or DomainError::NotFound
BTP-->>Bus: Result
Bus-->>IPC: AppError mapped
IPC-->>UI: Result<(), String>
UI->>UI: toast on error (openFileMissing / openFileError)
Reviews (2): Last reviewed commit: "fix(download): address PR #101 review co..." | Re-trigger Greptile
Comment on lines
+151
to
+162
| .map_err(|e| DomainError::StorageError(format!("failed to launch {program}: {e}")))?; | ||
| // Launchers on Windows (cmd/start/explorer) can return non-zero even on | ||
| // successful hand-off, so we trust the spawn-did-not-error signal only. | ||
| Ok(()) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn open_file_errors_when_path_missing() { |
There was a problem hiding this comment.
Command::status() call inside an async Tauri handler
run_launcher calls std::process::Command::status(), which is a synchronous blocking call. It is invoked from handle_open_download_file/handle_open_download_folder, which are awaited inside async Tauri IPC handlers. The module doc acknowledges the issue but the fix is not applied at the call site — blocking a Tokio worker thread starves other tasks on that thread while the child process runs. Wrapping the call in tokio::task::spawn_blocking would keep the async runtime healthy on slow launchers or network-mounted destinations.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src-tauri/src/adapters/driven/filesystem/file_opener.rs
Line: 151-162
Comment:
**Blocking `Command::status()` call inside an async Tauri handler**
`run_launcher` calls `std::process::Command::status()`, which is a synchronous blocking call. It is invoked from `handle_open_download_file`/`handle_open_download_folder`, which are awaited inside async Tauri IPC handlers. The module doc acknowledges the issue but the fix is not applied at the call site — blocking a Tokio worker thread starves other tasks on that thread while the child process runs. Wrapping the call in `tokio::task::spawn_blocking` would keep the async runtime healthy on slow launchers or network-mounted destinations.
How can I resolve this? If you propose a fix, please make it concise.
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/views/DownloadsView/__tests__/DownloadsTable.test.tsx (1)
124-135:⚠️ Potential issue | 🟠 MajorReset locale in shared setup to prevent order-dependent test failures.
A previous test sets
i18nextLngtofr, but this setup does not restore a default locale. Later tests asserting English labels can fail depending on execution order.Proposed fix
beforeEach(() => { + window.localStorage.setItem('i18nextLng', 'en'); useUiStore.setState({ selectedDownloadId: null, selectedDownloadIds: [], detailsPanelOpen: false, filterBarExpanded: false, }); useDownloadStore.setState({ progressMap: {} }); invokeMock.mockClear(); invokeMock.mockResolvedValue(undefined); toastErrorMock.mockClear(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/DownloadsView/__tests__/DownloadsTable.test.tsx` around lines 124 - 135, In the shared beforeEach setup, reset the i18next locale so tests aren't order-dependent: update the beforeEach block (the one calling useUiStore.setState and useDownloadStore.setState) to also clear or set the i18next storage key (e.g., localStorage.removeItem('i18nextLng') or localStorage.setItem('i18nextLng','en')) so tests that assert English labels always run with a known locale.
🧹 Nitpick comments (1)
src/views/DownloadsView/DownloadsTable.tsx (1)
371-382: Prefer structured error codes over string matching for failure classification.Using
"not found"substring checks ties UI behavior to backend message text. A small wording change can silently break the missing-file toast path. Consider returning a typed error code from IPC (e.g.,NOT_FOUND) and branching on that instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/DownloadsView/DownloadsTable.tsx` around lines 371 - 382, The current error handlers in openFileMut and openFolderMut rely on fragile string matching of err.message to detect missing-file cases; update both useTauriMutation errorMessage callbacks to branch on a structured error code (e.g., check err.code === 'NOT_FOUND' or err?.details?.code) instead of searching for "not found", and coordinate with the IPC handlers ('download_open_file' and 'download_open_folder') to return that typed error code on missing files so the toast selection (downloads.table.toast.openFileMissing vs openFileError/openFolderError) is resilient to message text changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src-tauri/src/adapters/driven/filesystem/file_opener.rs`:
- Around line 68-82: The NotFound guard and Linux fallback mishandle
single-component relative paths because path.parent() can be an empty path;
update the parent handling so empty parents are treated as the current directory
("."), i.e. compute parent from path.parent() but normalize any empty Path to
Path::new(".") before calling is_dir()/using it, then use that normalized parent
when deciding the NotFound error and when computing target for run_launcher
(refer to the parent, path, target and run_launcher usages and ensure the
unwrap_or/unwrap_or_else branches return Path::new(".") instead of an empty
path).
In `@src-tauri/src/application/commands/open_download_file.rs`:
- Around line 34-36: The call to
opener.open_file(Path::new(download.destination_path())) currently maps all
errors to AppError::Domain which loses file-not-found semantics; update the
error mapping in open_download_file.rs so that when opener.open_file returns an
IO error with ErrorKind::NotFound you return AppError::NotFound (preserving the
download.destination_path() context if needed), and for all other errors
continue to map to AppError::Domain; target the expression around
opener.open_file(...) and replace the single map_err(AppError::Domain) with
logic that matches the underlying error kind and returns AppError::NotFound for
NotFound, otherwise AppError::Domain.
---
Outside diff comments:
In `@src/views/DownloadsView/__tests__/DownloadsTable.test.tsx`:
- Around line 124-135: In the shared beforeEach setup, reset the i18next locale
so tests aren't order-dependent: update the beforeEach block (the one calling
useUiStore.setState and useDownloadStore.setState) to also clear or set the
i18next storage key (e.g., localStorage.removeItem('i18nextLng') or
localStorage.setItem('i18nextLng','en')) so tests that assert English labels
always run with a known locale.
---
Nitpick comments:
In `@src/views/DownloadsView/DownloadsTable.tsx`:
- Around line 371-382: The current error handlers in openFileMut and
openFolderMut rely on fragile string matching of err.message to detect
missing-file cases; update both useTauriMutation errorMessage callbacks to
branch on a structured error code (e.g., check err.code === 'NOT_FOUND' or
err?.details?.code) instead of searching for "not found", and coordinate with
the IPC handlers ('download_open_file' and 'download_open_folder') to return
that typed error code on missing files so the toast selection
(downloads.table.toast.openFileMissing vs openFileError/openFolderError) is
resilient to message text changes.
🪄 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: 81272ca2-2392-4af0-937e-6cdf1b823988
📒 Files selected for processing (18)
CHANGELOG.mdsrc-tauri/src/adapters/driven/filesystem/file_opener.rssrc-tauri/src/adapters/driven/filesystem/mod.rssrc-tauri/src/adapters/driving/tauri_ipc.rssrc-tauri/src/application/command_bus.rssrc-tauri/src/application/commands/mod.rssrc-tauri/src/application/commands/open_download_file.rssrc-tauri/src/application/commands/open_download_folder.rssrc-tauri/src/domain/ports/driven/file_opener.rssrc-tauri/src/domain/ports/driven/mod.rssrc-tauri/src/domain/ports/driven/tests.rssrc-tauri/src/lib.rssrc/i18n/locales/en.jsonsrc/i18n/locales/fr.jsonsrc/views/DownloadDetailsPanel/FileInfoSection.tsxsrc/views/DownloadDetailsPanel/__tests__/FileInfoSection.test.tsxsrc/views/DownloadsView/DownloadsTable.tsxsrc/views/DownloadsView/__tests__/DownloadsTable.test.tsx
There was a problem hiding this comment.
2 issues found across 18 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src-tauri/src/adapters/driven/filesystem/file_opener.rs">
<violation number="1" location="src-tauri/src/adapters/driven/filesystem/file_opener.rs:56">
P1: Using `cmd /C start` with an unescaped path lets `cmd` interpret metacharacters in filenames, which can execute unintended commands on Windows.</violation>
<violation number="2" location="src-tauri/src/adapters/driven/filesystem/file_opener.rs:113">
P2: Build the Windows `explorer /select` arguments without embedding the raw path into a formatted switch string; paths with spaces can fail to select the intended file.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| std::ffi::OsString::from("/C"), | ||
| std::ffi::OsString::from("start"), | ||
| std::ffi::OsString::from(""), | ||
| path.as_os_str().to_os_string(), |
There was a problem hiding this comment.
P1: Using cmd /C start with an unescaped path lets cmd interpret metacharacters in filenames, which can execute unintended commands on Windows.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src-tauri/src/adapters/driven/filesystem/file_opener.rs, line 56:
<comment>Using `cmd /C start` with an unescaped path lets `cmd` interpret metacharacters in filenames, which can execute unintended commands on Windows.</comment>
<file context>
@@ -0,0 +1,186 @@
+ std::ffi::OsString::from("/C"),
+ std::ffi::OsString::from("start"),
+ std::ffi::OsString::from(""),
+ path.as_os_str().to_os_string(),
+ ],
+ );
</file context>
- Wrap Command::status() calls in tokio::task::spawn_blocking so the async Tauri IPC handlers stop starving the tokio runtime on slow launchers (CodeRabbit + greptile). - Preserve DomainError::NotFound as AppError::NotFound in both command handlers so the UI can still surface the localized "file not found" toast instead of the generic failure path. - Normalize empty Path::parent() to "." in reveal_file so single- component relative paths like "file.bin" no longer spuriously error with NotFound and no longer hand an empty arg to xdg-open. - On Windows, pass "/select," and the target path as two separate OsString arguments so explorer can reveal files whose path contains spaces (e.g. "C:\\My Downloads\\file.mp4"). - Reset i18nextLng to "en" in DownloadsTable test setup so specs that assert English labels don't depend on test execution order.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src-tauri/src/application/commands/open_download_file.rs (1)
70-251: Consider extracting shared test mocks to reduce duplication.The mock implementations (
Repo,Engine,Bus,FS,Http,Loader,Cfg,Creds,Clip,Arch,RecordingOpener,build_bus,make_completed) are largely duplicated between this file andopen_download_folder.rs. Consolidating these into a shared test support module (e.g.,application::test_support) would reduce maintenance burden and ensure consistency.Also applies to: 252-307
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/application/commands/open_download_file.rs` around lines 70 - 251, Extract the duplicated test mocks and helpers into a shared test support module (e.g., application::test_support): move the structs and impls Repo, Engine, Bus, FS, Http, Loader, Cfg, Creds, Clip, Arch, RecordingOpener and the helper functions build_bus and make_completed into that module, make them public (pub) so tests can reuse them, then replace the local definitions in open_download_file.rs and open_download_folder.rs with use application::test_support::{Repo, Engine, Bus, FS, Http, Loader, Cfg, Creds, Clip, Arch, RecordingOpener, build_bus, make_completed}; ensure any referenced types (DownloadRepository, DownloadEngine, EventBus, FileStorage, HttpClient, PluginLoader, ConfigStore, CredentialStore, ClipboardObserver, ArchiveExtractor) are still in scope via existing imports.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src-tauri/src/application/commands/open_download_file.rs`:
- Around line 70-251: Extract the duplicated test mocks and helpers into a
shared test support module (e.g., application::test_support): move the structs
and impls Repo, Engine, Bus, FS, Http, Loader, Cfg, Creds, Clip, Arch,
RecordingOpener and the helper functions build_bus and make_completed into that
module, make them public (pub) so tests can reuse them, then replace the local
definitions in open_download_file.rs and open_download_folder.rs with use
application::test_support::{Repo, Engine, Bus, FS, Http, Loader, Cfg, Creds,
Clip, Arch, RecordingOpener, build_bus, make_completed}; ensure any referenced
types (DownloadRepository, DownloadEngine, EventBus, FileStorage, HttpClient,
PluginLoader, ConfigStore, CredentialStore, ClipboardObserver, ArchiveExtractor)
are still in scope via existing imports.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 122f32f2-780f-4a50-85f2-4307ca793904
📒 Files selected for processing (5)
src-tauri/src/adapters/driven/filesystem/file_opener.rssrc-tauri/src/application/command_bus.rssrc-tauri/src/application/commands/open_download_file.rssrc-tauri/src/application/commands/open_download_folder.rssrc/views/DownloadsView/__tests__/DownloadsTable.test.tsx
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
• Feature: Add Open file and Open folder actions for completed downloads in UI and Details panel
• Port: Introduce
FileOpenerdomain port for OS-level file operations (open_file, reveal_file)• Adapter: Implement
SystemFileOpenerwith platform-specific launchers (xdg-open/open/explorer)• Application: Add command handlers
open_download_fileandopen_download_folderwith state validation• IPC: Register Tauri commands delegating to application layer with error mapping
• Frontend: Wire UI buttons and mutations with i18n support and graceful error handling
• Tests: 10 Rust tests (domain ports, adapters, handlers), 9 TypeScript tests (components, mutations)
Changes
FileOpener(open_file, reveal_file) with mock implementation for testingSystemFileOpenerdispatches per-OS (Linux xdg-open, macOS open -R, Windows explorer /select)download_open_file(id)anddownload_open_folder(id)with Result<(), String> signatureType
feat
Summary by cubic
Add “Open file” and “Open folder” actions for completed downloads so users can launch the file or reveal it in their OS file manager from both the table and the details panel. Implements PRD‑v2 P0.8 (task 08).
New Features
download_open_file(id)anddownload_open_folder(id)Tauri commands.FileOpenerport withSystemFileOpenerusingxdg-open(Linux),open/open -R(macOS), andexplorer//select(Windows); wired viaCommandBus::with_file_opener.Completeddownloads; missing files surface asNotFoundand map to a localized “File not found” toast (en/fr).useTauriMutationand error toasts.Bug Fixes
tokio::task::spawn_blockingto avoid stalling the async runtime./select,and the path as separate args toexplorerto handle spaces.reveal_file, normalize emptyPath::parent()to “.” to support relative filenames and avoid empty args toxdg-open.Written for commit b6bd963. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Tests