F-002: refactor(app): make hook and clipboard methods non-blocking#11
F-002: refactor(app): make hook and clipboard methods non-blocking#11Sephyi wants to merge 1 commit intodevelopmentfrom
Conversation
Replace synchronous std::process::Command and arboard calls with non-blocking alternatives so the tokio runtime isn't blocked: - handle_hook, hook_dir, hook_path, hook_install, hook_uninstall, and hook_status are now async; hook_dir uses tokio::process::Command with an explicit std::process::Output annotation (CLAUDE.md gotcha). - copy_to_clipboard wraps arboard::Clipboard::new() and set_text() in tokio::task::spawn_blocking; its signature changes from &str to String to satisfy the 'static bound. Closes audit entry F-002 from #3.
There was a problem hiding this comment.
Pull request overview
Refactors the app’s hook and clipboard paths to avoid blocking the async runtime, addressing audit finding F-002 (sync process/clipboard calls on the tokio runtime).
Changes:
- Make hook command handling async and run
git rev-parse --git-dirviatokio::process::Command. - Offload clipboard writes (
arboard) totokio::task::spawn_blockingand make clipboard helper async. - Update command dispatch / clipboard flow to
awaitthe new async helpers.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Ok(()) | ||
| }) | ||
| .await | ||
| .map_err(|e| Error::Config(format!("Clipboard task panicked: {e}")))? |
There was a problem hiding this comment.
The JoinError from spawn_blocking can represent either a panic or task cancellation (e.g., runtime shutdown). This error message always says "panicked", which can be misleading. Consider branching on e.is_panic() / e.is_cancelled() and wording the message accordingly (or avoid claiming panic unless is_panic() is true).
| .map_err(|e| Error::Config(format!("Clipboard task panicked: {e}")))? | |
| .map_err(|e| { | |
| Error::Config(if e.is_panic() { | |
| format!("Clipboard task panicked: {e}") | |
| } else if e.is_cancelled() { | |
| format!("Clipboard task was cancelled: {e}") | |
| } else { | |
| format!("Clipboard task failed to join: {e}") | |
| }) | |
| })? |
| // Annotated type per CLAUDE.md gotcha: tokio::process::Command output | ||
| // needs explicit std::process::Output typing when used with `?`. |
There was a problem hiding this comment.
This comment references the CLAUDE.md gotcha but says the std::process::Output annotation is needed when using ?. The documented gotcha is specifically about .ok()? (type inference failure); with .await? and subsequent output.status usage, the annotation isn't required. Consider either removing the comment or rewording it to match the actual gotcha to avoid confusing future readers.
| // Annotated type per CLAUDE.md gotcha: tokio::process::Command output | |
| // needs explicit std::process::Output typing when used with `?`. | |
| // Keep the explicit type annotation for clarity; it is not required by `.await?` here. |
Summary
refactor(app): make hook and clipboard methods non-blocking.
Audit context
Closes audit entry F-002 from #3.
Verification
cargo fmt --checkcargo clippy --all-targets --all-features -- -D warningscargo test --all-targets