feat: add GitPushResult with error classification for push failures

Replaces raw string return from git_push with a structured GitPushResult
that classifies errors as rejected/auth_error/network_error/error, each
with an actionable user-facing message.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Test
2026-03-07 00:29:54 +01:00
parent a6d60695a2
commit c14927df8f
3 changed files with 173 additions and 9 deletions

View File

@@ -15,7 +15,7 @@ pub use conflict::{
};
pub use history::{get_file_diff, get_file_diff_at_commit, get_file_history};
pub use pulse::{get_last_commit_info, get_vault_pulse, LastCommitInfo, PulseCommit, PulseFile};
pub use remote::{git_pull, git_push, has_remote, GitPullResult};
pub use remote::{git_pull, git_push, has_remote, GitPullResult, GitPushResult};
pub use status::{get_modified_files, ModifiedFile};
use serde::Serialize;

View File

@@ -110,8 +110,74 @@ fn parse_updated_files(stdout: &str) -> Vec<String> {
.collect()
}
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq)]
pub struct GitPushResult {
pub status: String, // "ok" | "rejected" | "auth_error" | "network_error" | "error"
pub message: String,
}
/// Classify a git push stderr message into a user-friendly status and message.
pub fn classify_push_error(stderr: &str) -> GitPushResult {
let lower = stderr.to_lowercase();
if lower.contains("non-fast-forward")
|| lower.contains("[rejected]")
|| lower.contains("fetch first")
|| lower.contains("failed to push some refs")
&& (lower.contains("updates were rejected") || lower.contains("non-fast-forward"))
{
return GitPushResult {
status: "rejected".to_string(),
message: "Push rejected: remote has new commits. Pull first, then push.".to_string(),
};
}
if lower.contains("authentication failed")
|| lower.contains("could not read username")
|| lower.contains("permission denied")
|| lower.contains("403")
|| lower.contains("invalid credentials")
{
return GitPushResult {
status: "auth_error".to_string(),
message: "Push failed: authentication error. Check your credentials.".to_string(),
};
}
if lower.contains("could not resolve host")
|| lower.contains("unable to access")
|| lower.contains("connection refused")
|| lower.contains("network is unreachable")
|| lower.contains("timed out")
{
return GitPushResult {
status: "network_error".to_string(),
message: "Push failed: network error. Check your connection and try again.".to_string(),
};
}
// Fallback: extract the hint line if present, otherwise use the full stderr
let hint_line = stderr
.lines()
.find(|l| l.trim_start().starts_with("hint:"))
.map(|l| l.trim_start().strip_prefix("hint:").unwrap_or(l).trim())
.unwrap_or("")
.to_string();
let detail = if hint_line.is_empty() {
stderr.trim().to_string()
} else {
hint_line
};
GitPushResult {
status: "error".to_string(),
message: format!("Push failed: {detail}"),
}
}
/// Push to remote.
pub fn git_push(vault_path: &str) -> Result<String, String> {
pub fn git_push(vault_path: &str) -> Result<GitPushResult, String> {
let vault = Path::new(vault_path);
let output = Command::new("git")
@@ -122,13 +188,13 @@ pub fn git_push(vault_path: &str) -> Result<String, String> {
if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr);
return Err(format!("git push failed: {}", stderr));
return Ok(classify_push_error(&stderr));
}
// git push often writes to stderr even on success
let stdout = String::from_utf8_lossy(&output.stdout);
let stderr = String::from_utf8_lossy(&output.stderr);
Ok(format!("{}{}", stdout, stderr))
Ok(GitPushResult {
status: "ok".to_string(),
message: "Pushed to remote".to_string(),
})
}
#[cfg(test)]
@@ -227,6 +293,104 @@ mod tests {
assert!(files.is_empty());
}
#[test]
fn test_classify_push_error_non_fast_forward() {
let stderr = r#"To github.com:user/repo.git
! [rejected] main -> main (non-fast-forward)
error: failed to push some refs to 'github.com:user/repo.git'
hint: Updates were rejected because the remote contains work that you do not
hint: have locally."#;
let result = classify_push_error(stderr);
assert_eq!(result.status, "rejected");
assert!(result.message.contains("Pull first"));
}
#[test]
fn test_classify_push_error_fetch_first() {
let stderr = "error: failed to push some refs\nhint: Updates were rejected because the tip of your current branch is behind\nhint: its remote counterpart. Integrate the remote changes (e.g.\nhint: 'git pull ...') before pushing again.\nhint: See the 'Note about fast-forwards' in 'git push --help' for details.\n ! [rejected] main -> main (fetch first)\n";
let result = classify_push_error(stderr);
assert_eq!(result.status, "rejected");
}
#[test]
fn test_classify_push_error_auth_failure() {
let stderr = "remote: Permission denied to user/repo.git\nfatal: unable to access 'https://github.com/user/repo.git/': The requested URL returned error: 403";
let result = classify_push_error(stderr);
assert_eq!(result.status, "auth_error");
assert!(result.message.contains("authentication"));
}
#[test]
fn test_classify_push_error_network() {
let stderr = "fatal: unable to access 'https://github.com/user/repo.git/': Could not resolve host: github.com";
let result = classify_push_error(stderr);
assert_eq!(result.status, "network_error");
assert!(result.message.contains("network"));
}
#[test]
fn test_classify_push_error_unknown() {
let stderr = "error: something unexpected happened\nhint: Try again later";
let result = classify_push_error(stderr);
assert_eq!(result.status, "error");
assert!(result.message.contains("Try again later"));
}
#[test]
fn test_classify_push_error_unknown_no_hint() {
let stderr = "error: something totally weird";
let result = classify_push_error(stderr);
assert_eq!(result.status, "error");
assert!(result.message.contains("something totally weird"));
}
#[test]
fn test_git_push_result_serialization() {
let result = GitPushResult {
status: "rejected".to_string(),
message: "Push rejected".to_string(),
};
let json = serde_json::to_string(&result).unwrap();
assert!(json.contains("\"rejected\""));
let parsed: GitPushResult = serde_json::from_str(&json).unwrap();
assert_eq!(parsed.status, "rejected");
}
#[test]
fn test_git_push_success_returns_ok() {
let (_bare, clone_a, _clone_b) = setup_remote_pair();
let vp_a = clone_a.path().to_str().unwrap();
fs::write(clone_a.path().join("note.md"), "# Note\n").unwrap();
git_commit(vp_a, "initial").unwrap();
let result = git_push(vp_a).unwrap();
assert_eq!(result.status, "ok");
}
#[test]
fn test_git_push_rejected_returns_rejected() {
let (_bare, clone_a, clone_b) = setup_remote_pair();
let vp_a = clone_a.path().to_str().unwrap();
let vp_b = clone_b.path().to_str().unwrap();
// Both clones commit and push — second push should be rejected
fs::write(clone_a.path().join("note.md"), "# A\n").unwrap();
git_commit(vp_a, "from A").unwrap();
git_push(vp_a).unwrap();
git_pull(vp_b).unwrap();
fs::write(clone_b.path().join("note.md"), "# B\n").unwrap();
git_commit(vp_b, "from B").unwrap();
git_push(vp_b).unwrap();
// Now A has a new commit but hasn't pulled B's changes
fs::write(clone_a.path().join("other.md"), "# Other\n").unwrap();
git_commit(vp_a, "from A again").unwrap();
let result = git_push(vp_a).unwrap();
assert_eq!(result.status, "rejected");
assert!(result.message.contains("Pull first"));
}
#[test]
fn test_git_pull_result_serialization() {
let result = GitPullResult {