From 67c8598b87d290c3339add5126aa5118356b7ff2 Mon Sep 17 00:00:00 2001 From: lucaronin Date: Fri, 24 Apr 2026 12:22:01 +0200 Subject: [PATCH] fix: keep clone flows off the UI thread --- docs/ABSTRACTIONS.md | 4 +- docs/ARCHITECTURE.md | 4 +- src-tauri/src/commands/git_clone.rs | 18 ++++ src-tauri/src/commands/mod.rs | 1 + .../src/commands/vault/lifecycle_cmds.rs | 6 +- src-tauri/src/git/clone.rs | 94 +++++++++++++++++-- src-tauri/src/lib.rs | 2 +- src/components/CloneVaultModal.test.tsx | 27 +++++- src/components/CloneVaultModal.tsx | 2 +- src/mock-tauri/mock-handlers.ts | 5 + 10 files changed, 144 insertions(+), 19 deletions(-) create mode 100644 src-tauri/src/commands/git_clone.rs diff --git a/docs/ABSTRACTIONS.md b/docs/ABSTRACTIONS.md index 1df20e14..9f2f5664 100644 --- a/docs/ABSTRACTIONS.md +++ b/docs/ABSTRACTIONS.md @@ -627,9 +627,9 @@ Tolaria tracks managed vault-level AI guidance separately from normal note conte Tolaria delegates remote auth to the user's system git setup: - `CloneVaultModal` captures a remote URL and local destination -- `clone_repo` shells out to system git for clone operations +- `clone_git_repo` and `create_getting_started_vault` both run system git clone work in blocking Tokio tasks so clone UIs stay responsive - `git_add_remote` uses the same system git path and refuses remotes whose history is unrelated or ahead of the local vault -- Existing `git_pull` / `git_push` commands keep surfacing raw git errors +- Existing `git_pull` / `git_push` commands keep surfacing raw git errors, and clone commands fail fast when git wants interactive terminal input - No provider-specific token or username is stored in app settings ## Settings diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index cc15a53a..288bb1dc 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -460,9 +460,9 @@ Tolaria no longer implements provider-specific OAuth or remote-repository APIs. **Flow:** 1. User opens `CloneVaultModal` from onboarding or the vault menu 2. User pastes any git URL and chooses a local destination -3. `clone_repo()` shells out to `git clone` +3. The `clone_git_repo()` Tauri command runs `git clone` inside a blocking Tokio task so the Tauri window stays responsive during slow or failing clones 4. `git_push()` / `git_pull()` continue to use the same system git path -5. If auth fails, the raw git stderr is surfaced in the UI +5. Clone commands disable interactive terminal / askpass prompts and surface the git failure back to the UI instead of freezing the app waiting for input **Auth model:** - SSH keys, Git Credential Manager, macOS Keychain helpers, `gh auth`, and other git helpers all work without app-specific setup diff --git a/src-tauri/src/commands/git_clone.rs b/src-tauri/src/commands/git_clone.rs new file mode 100644 index 00000000..2e34d753 --- /dev/null +++ b/src-tauri/src/commands/git_clone.rs @@ -0,0 +1,18 @@ +use super::expand_tilde; + +#[cfg(desktop)] +#[tauri::command] +pub async fn clone_git_repo(url: String, local_path: String) -> Result { + let url = url.trim().to_string(); + let local_path = expand_tilde(&local_path).into_owned(); + + tokio::task::spawn_blocking(move || super::git::clone_repo(url, local_path)) + .await + .map_err(|e| format!("Task panicked: {e}"))? +} + +#[cfg(mobile)] +#[tauri::command] +pub async fn clone_git_repo(_url: String, _local_path: String) -> Result { + Err("Git clone is not available on mobile".into()) +} diff --git a/src-tauri/src/commands/mod.rs b/src-tauri/src/commands/mod.rs index 2ec88bad..8adc3630 100644 --- a/src-tauri/src/commands/mod.rs +++ b/src-tauri/src/commands/mod.rs @@ -2,6 +2,7 @@ mod ai; mod delete; mod folders; mod git; +pub mod git_clone; mod git_connect; mod system; mod vault; diff --git a/src-tauri/src/commands/vault/lifecycle_cmds.rs b/src-tauri/src/commands/vault/lifecycle_cmds.rs index 33036e3a..e1105456 100644 --- a/src-tauri/src/commands/vault/lifecycle_cmds.rs +++ b/src-tauri/src/commands/vault/lifecycle_cmds.rs @@ -57,9 +57,11 @@ fn canonical_vault_path_string(vault_dir: &Path) -> String { } #[tauri::command] -pub fn create_getting_started_vault(target_path: Option) -> Result { +pub async fn create_getting_started_vault(target_path: Option) -> Result { let path = resolve_getting_started_target(target_path.as_deref())?; - vault::create_getting_started_vault(&path) + tokio::task::spawn_blocking(move || vault::create_getting_started_vault(&path)) + .await + .map_err(|e| format!("Task panicked: {e}"))? } fn resolve_getting_started_target(target_path: Option<&str>) -> Result { diff --git a/src-tauri/src/git/clone.rs b/src-tauri/src/git/clone.rs index 9158af55..9ebc444c 100644 --- a/src-tauri/src/git/clone.rs +++ b/src-tauri/src/git/clone.rs @@ -1,12 +1,18 @@ use std::path::Path; -use std::process::Command; +use std::process::{Command, Output, Stdio}; + +struct CloneRequest<'a> { + url: &'a str, + dest: &'a Path, +} /// Clone a git repository to a local path using the system git configuration. pub fn clone_repo(url: &str, local_path: &str) -> Result { let dest = Path::new(local_path); + let request = CloneRequest { url, dest }; prepare_clone_destination(dest)?; - if let Err(err) = run_clone(url, dest) { + if let Err(err) = run_clone(&request) { cleanup_failed_clone(dest); return Err(err); } @@ -64,12 +70,14 @@ fn directory_has_entries(dest: &Path) -> Result { .map(|mut entries| entries.next().is_some()) } -fn run_clone(url: &str, dest: &Path) -> Result<(), String> { - let destination = dest - .to_str() - .ok_or_else(|| format!("Destination '{}' is not valid UTF-8", dest.display()))?; - let output = Command::new("git") - .args(["clone", "--progress", url, destination]) +fn run_clone(request: &CloneRequest<'_>) -> Result<(), String> { + let destination = request.dest.to_str().ok_or_else(|| { + format!( + "Destination '{}' is not valid UTF-8", + request.dest.display() + ) + })?; + let output = build_clone_command(request, destination) .output() .map_err(|e| format!("Failed to run git clone: {}", e))?; @@ -77,8 +85,36 @@ fn run_clone(url: &str, dest: &Path) -> Result<(), String> { return Ok(()); } + Err(format!( + "git clone failed: {}", + clone_failure_message(&output) + )) +} + +fn build_clone_command(request: &CloneRequest<'_>, destination: &str) -> Command { + let mut command = Command::new("git"); + command + .args(["clone", "--progress", request.url, destination]) + .env("GIT_TERMINAL_PROMPT", "0") + .env("SSH_ASKPASS_REQUIRE", "never") + .stdin(Stdio::null()); + command +} + +fn clone_failure_message(output: &Output) -> String { let stderr = String::from_utf8_lossy(&output.stderr); - Err(format!("git clone failed: {}", stderr.trim())) + let stderr = stderr.trim(); + if !stderr.is_empty() { + return stderr.to_string(); + } + + let stdout = String::from_utf8_lossy(&output.stdout); + let stdout = stdout.trim(); + if !stdout.is_empty() { + return stdout.to_string(); + } + + format!("git clone exited with status {}", output.status) } fn cleanup_failed_clone(dest: &Path) { @@ -91,6 +127,7 @@ fn cleanup_failed_clone(dest: &Path) { mod tests { use super::*; use std::fs; + use std::os::unix::process::ExitStatusExt; use std::path::Path; use std::process::Command as StdCommand; @@ -160,4 +197,43 @@ mod tests { ); assert!(result.unwrap_err().contains("git clone failed")); } + + #[test] + fn test_clone_failure_message_falls_back_to_stdout() { + let output = Output { + status: std::process::ExitStatus::from_raw(128), + stdout: b"fatal: stdout only".to_vec(), + stderr: Vec::new(), + }; + + assert_eq!(clone_failure_message(&output), "fatal: stdout only"); + } + + #[test] + fn test_build_clone_command_disables_interactive_prompts() { + let dest = Path::new("/tmp/repo"); + let request = CloneRequest { + url: "https://example.com/repo.git", + dest, + }; + let command = build_clone_command(&request, "/tmp/repo"); + let envs = command + .get_envs() + .map(|(key, value)| { + ( + key.to_string_lossy().to_string(), + value.map(|entry| entry.to_string_lossy().to_string()), + ) + }) + .collect::>(); + + assert_eq!( + envs.get("GIT_TERMINAL_PROMPT"), + Some(&Some("0".to_string())) + ); + assert_eq!( + envs.get("SSH_ASKPASS_REQUIRE"), + Some(&Some("never".to_string())) + ); + } } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index e72c22df..705e0755 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -259,7 +259,7 @@ macro_rules! app_invoke_handler { commands::download_and_install_app_update, commands::load_vault_list, commands::save_vault_list, - commands::clone_repo, + commands::git_clone::clone_git_repo, commands::search_vault, commands::create_empty_vault, commands::create_getting_started_vault, diff --git a/src/components/CloneVaultModal.test.tsx b/src/components/CloneVaultModal.test.tsx index 6b21e25b..a6ce8823 100644 --- a/src/components/CloneVaultModal.test.tsx +++ b/src/components/CloneVaultModal.test.tsx @@ -46,7 +46,7 @@ describe('CloneVaultModal', () => { expect(screen.getByTestId('clone-vault-path')).toHaveValue('~/Vaults/my-vault') }) - it('calls clone_repo and reports the cloned vault on submit', async () => { + it('calls clone_git_repo and reports the cloned vault on submit', async () => { render() fireEvent.change(screen.getByTestId('clone-repo-url'), { @@ -55,7 +55,7 @@ describe('CloneVaultModal', () => { fireEvent.click(screen.getByTestId('clone-vault-submit')) await waitFor(() => { - expect(mockInvokeFn).toHaveBeenCalledWith('clone_repo', { + expect(mockInvokeFn).toHaveBeenCalledWith('clone_git_repo', { url: 'git@github.com:user/my-vault.git', localPath: '~/Vaults/my-vault', }) @@ -82,4 +82,27 @@ describe('CloneVaultModal', () => { expect(screen.getByTestId('clone-vault-error')).toHaveTextContent('Clone failed: Error: Permission denied') }) }) + + it('keeps progress visible while the clone request is pending', async () => { + let resolveClone: ((value: string) => void) | null = null + mockInvokeFn.mockReturnValueOnce(new Promise((resolve) => { + resolveClone = resolve + })) + + render() + + fireEvent.change(screen.getByTestId('clone-repo-url'), { + target: { value: 'git@github.com:user/my-vault.git' }, + }) + fireEvent.click(screen.getByTestId('clone-vault-submit')) + + expect(screen.getByTestId('clone-vault-submit')).toHaveTextContent('Cloning...') + expect(screen.getByTestId('clone-vault-submit')).toBeDisabled() + + resolveClone?.('Cloned successfully') + + await waitFor(() => { + expect(onVaultCloned).toHaveBeenCalledWith('~/Vaults/my-vault', 'my-vault') + }) + }) }) diff --git a/src/components/CloneVaultModal.tsx b/src/components/CloneVaultModal.tsx index 574de02a..9cb28ecd 100644 --- a/src/components/CloneVaultModal.tsx +++ b/src/components/CloneVaultModal.tsx @@ -108,7 +108,7 @@ function useCloneVaultForm(onClose: () => void, onVaultCloned: (path: string, la setCloneError(null) try { - await tauriCall('clone_repo', { url: trimmedUrl, localPath: trimmedPath }) + await tauriCall('clone_git_repo', { url: trimmedUrl, localPath: trimmedPath }) onVaultCloned(trimmedPath, labelFromPath(trimmedPath)) handleClose() } catch (error) { diff --git a/src/mock-tauri/mock-handlers.ts b/src/mock-tauri/mock-handlers.ts index 09ab9e87..ed830e9a 100644 --- a/src/mock-tauri/mock-handlers.ts +++ b/src/mock-tauri/mock-handlers.ts @@ -428,6 +428,11 @@ export const mockHandlers: Record any> = { setMockRemoteState(localPath, true) return `Cloned to ${localPath}` }, + clone_git_repo: (args: { url: string; localPath?: string; local_path?: string }) => { + const localPath = args.localPath ?? args.local_path ?? '' + setMockRemoteState(localPath, true) + return `Cloned to ${localPath}` + }, purge_trash: () => [], delete_note: (args: { path: string }) => args.path, batch_delete_notes: (args: { paths: string[] }) => args.paths,