From 7516dfe48a7ea9243dadeae98d420d77d9081ccd Mon Sep 17 00:00:00 2001 From: mehmet turac Date: Sun, 24 May 2026 21:58:42 +0300 Subject: [PATCH] fix: treat name-only git remotes as local-only --- src-tauri/src/commands/git.rs | 2 +- src-tauri/src/git/remote.rs | 83 +++++++++++++++++++++++++++++++++-- src/hooks/useAutoSync.test.ts | 34 ++++++++++++++ src/hooks/useAutoSync.ts | 12 ++++- src/types.ts | 2 +- 5 files changed, 127 insertions(+), 6 deletions(-) diff --git a/src-tauri/src/commands/git.rs b/src-tauri/src/commands/git.rs index b6b36e40..d1028dd3 100644 --- a/src-tauri/src/commands/git.rs +++ b/src-tauri/src/commands/git.rs @@ -475,7 +475,7 @@ mod tests { assert_eq!(pull.status, "no_remote"); let push = git_push(vault.clone()).await.unwrap(); - assert_eq!(push.status, "error"); + assert_eq!(push.status, "no_remote"); let status = git_remote_status(vault.clone()).await.unwrap(); assert!(!status.has_remote); diff --git a/src-tauri/src/git/remote.rs b/src-tauri/src/git/remote.rs index 89234787..507067b6 100644 --- a/src-tauri/src/git/remote.rs +++ b/src-tauri/src/git/remote.rs @@ -18,10 +18,22 @@ pub struct GitPullResult { pub fn has_remote(vault_path: &str) -> Result { let vault = Path::new(vault_path); let output = git_command() - .args(["remote"]) + .args(["config", "--get-regexp", r"^remote\..*\.url$"]) .current_dir(vault) .output() - .map_err(|e| format!("Failed to run git remote: {}", e))?; + .map_err(|e| format!("Failed to inspect git remotes: {}", e))?; + + if !output.status.success() { + if output.status.code() == Some(1) { + return Ok(false); + } + let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + return Err(if stderr.is_empty() { + "Failed to inspect git remotes".to_string() + } else { + stderr + }); + } Ok(!String::from_utf8_lossy(&output.stdout).trim().is_empty()) } @@ -181,7 +193,7 @@ fn current_branch(vault: &Path) -> Result { #[derive(Debug, Serialize, Deserialize, Clone, PartialEq)] pub struct GitPushResult { - pub status: String, // "ok" | "rejected" | "auth_error" | "network_error" | "error" + pub status: String, // "ok" | "rejected" | "auth_error" | "network_error" | "no_remote" | "error" pub message: String, } @@ -210,6 +222,10 @@ pub fn classify_push_error(stderr: &str) -> GitPushResult { ); } + if is_no_remote_push_error(&lower) { + return push_error("no_remote", "No remote configured"); + } + push_error( "error", format!("Push failed: {}", push_error_detail(stderr)), @@ -259,6 +275,18 @@ fn is_network_push_error(lower: &str) -> bool { ) } +fn is_no_remote_push_error(lower: &str) -> bool { + contains_any( + lower, + &[ + "no configured push destination", + "does not appear to be a git repository", + "no such remote", + "no upstream branch", + ], + ) +} + fn push_error_detail(stderr: &str) -> String { let hint_line = stderr .lines() @@ -283,6 +311,13 @@ fn push_error_detail(stderr: &str) -> String { pub fn git_push(vault_path: &str) -> Result { let vault = Path::new(vault_path); + if !has_remote(vault_path)? { + return Ok(GitPushResult { + status: "no_remote".to_string(), + message: "No remote configured".to_string(), + }); + } + let output = git_command() .args(["push"]) .current_dir(vault) @@ -331,6 +366,27 @@ mod tests { assert!(has_remote(vp).unwrap()); } + #[test] + fn test_has_remote_ignores_name_only_remote_without_url() { + let dir = setup_git_repo(); + let vault = dir.path(); + let vp = vault.to_str().unwrap(); + + git_command() + .args(["config", "remote.origin.prune", "true"]) + .current_dir(vault) + .output() + .unwrap(); + + let remote_names = git_command() + .args(["remote"]) + .current_dir(vault) + .output() + .unwrap(); + assert!(String::from_utf8_lossy(&remote_names.stdout).contains("origin")); + assert!(!has_remote(vp).unwrap()); + } + #[test] fn test_git_pull_no_remote_returns_no_remote() { let dir = setup_git_repo(); @@ -430,6 +486,14 @@ hint: have locally."#; assert!(result.message.contains("network")); } + #[test] + fn test_classify_push_error_no_remote() { + let stderr = "fatal: No configured push destination."; + let result = classify_push_error(stderr); + assert_eq!(result.status, "no_remote"); + assert!(result.message.contains("No remote")); + } + #[test] fn test_classify_push_error_unknown() { let stderr = "error: something unexpected happened\nhint: Try again later"; @@ -469,6 +533,19 @@ hint: have locally."#; assert_eq!(result.status, "ok"); } + #[test] + fn test_git_push_no_remote_returns_no_remote() { + let dir = setup_git_repo(); + let vault = dir.path(); + let vp = vault.to_str().unwrap(); + + fs::write(vault.join("note.md"), "# Note\n").unwrap(); + git_commit(vp, "initial").unwrap(); + + let result = git_push(vp).unwrap(); + assert_eq!(result.status, "no_remote"); + } + #[test] fn test_git_push_rejected_returns_rejected() { let (_bare, clone_a, clone_b) = setup_remote_pair(); diff --git a/src/hooks/useAutoSync.test.ts b/src/hooks/useAutoSync.test.ts index fea1f079..2e280b33 100644 --- a/src/hooks/useAutoSync.test.ts +++ b/src/hooks/useAutoSync.test.ts @@ -549,6 +549,40 @@ describe('useAutoSync', () => { }) }) + it('pullAndPush skips push for local-only vaults with no remote', async () => { + const { result } = renderSync() + await waitFor(() => { + expect(result.current.syncStatus).toBe('idle') + }) + + mockInvokeFn.mockClear() + mockInvokeFn.mockImplementation((cmd: string) => { + if (cmd === 'get_conflict_files') return Promise.resolve([]) + if (cmd === 'get_last_commit_info') return Promise.resolve(MOCK_COMMIT_INFO) + if (cmd === 'git_remote_status') return Promise.resolve({ branch: 'main', ahead: 0, behind: 0, hasRemote: false }) + if (cmd === 'git_pull') { + return Promise.resolve({ + status: 'no_remote', + message: 'No remote configured', + updatedFiles: [], + conflictFiles: [], + }) + } + if (cmd === 'git_push') return Promise.resolve({ status: 'ok', message: 'Pushed successfully' }) + return Promise.resolve(upToDate()) + }) + + await act(async () => { + result.current.pullAndPush() + }) + + await waitFor(() => { + expect(mockInvokeFn).toHaveBeenCalledWith('git_pull', { vaultPath: '/Users/luca/Laputa' }) + expect(mockInvokeFn).not.toHaveBeenCalledWith('git_push', { vaultPath: '/Users/luca/Laputa' }) + expect(result.current.syncStatus).toBe('idle') + }) + }) + it('surfaces pull conflicts and pull errors during recovery pushes', async () => { let mode: 'conflict' | 'error' = 'conflict' diff --git a/src/hooks/useAutoSync.ts b/src/hooks/useAutoSync.ts index 0eca2b6e..fb75b38b 100644 --- a/src/hooks/useAutoSync.ts +++ b/src/hooks/useAutoSync.ts @@ -521,7 +521,17 @@ export function useAutoSync({ }) } - const pushOutcomes = await Promise.all(pullVaultPaths.map(async (path) => ({ + const pushVaultPaths = pullOutcomes + .filter((outcome) => outcome.result.status !== 'no_remote') + .map((outcome) => outcome.vaultPath) + + if (pushVaultPaths.length === 0) { + clearConflictState(setSyncStatus, setConflictFiles, setConflictVaultPath) + void refreshRemoteStatus(pullVaultPaths) + return + } + + const pushOutcomes = await Promise.all(pushVaultPaths.map(async (path) => ({ result: await tauriCall('git_push', { vaultPath: path }), vaultPath: path, }))) diff --git a/src/types.ts b/src/types.ts index 1b6f6882..9cd02d16 100644 --- a/src/types.ts +++ b/src/types.ts @@ -141,7 +141,7 @@ export interface GitPullResult { } export interface GitPushResult { - status: 'ok' | 'rejected' | 'auth_error' | 'network_error' | 'error' + status: 'ok' | 'rejected' | 'auth_error' | 'network_error' | 'no_remote' | 'error' message: string }