From 9ce288a3a2536307871052049570c76b0544eade Mon Sep 17 00:00:00 2001 From: thibaud-lclr Date: Mon, 20 Apr 2026 12:13:44 +0200 Subject: [PATCH] fix: prevent false done status when developer output is not actionable --- src-tauri/src/services/orchestrator.rs | 227 +++++++++++++++++++++++++ 1 file changed, 227 insertions(+) diff --git a/src-tauri/src/services/orchestrator.rs b/src-tauri/src/services/orchestrator.rs index fe57439..40f20be 100644 --- a/src-tauri/src/services/orchestrator.rs +++ b/src-tauri/src/services/orchestrator.rs @@ -149,6 +149,64 @@ pub fn parse_verdict(report: &str) -> Verdict { Verdict::FixNeeded } +fn developer_report_indicates_permission_block(report: &str) -> bool { + let lowered = report.to_lowercase(); + [ + "refus d'autorisation", + "permission d'edition", + "permission d’edition", + "permission denied", + "read-only file system", + "operation not permitted", + "cannot touch", + ] + .iter() + .any(|needle| lowered.contains(needle)) +} + +fn evaluate_developer_completion( + developer_report: &str, + commit_count: usize, + has_diff: bool, +) -> Result<(), String> { + if developer_report_indicates_permission_block(developer_report) { + return Err("Developer agent reported a worktree write permission issue.".to_string()); + } + + if commit_count == 0 { + return Err( + "Developer run completed without any commit in the worktree branch.".to_string(), + ); + } + + if !has_diff { + return Err("Developer run completed without any diff against the base branch.".to_string()); + } + + Ok(()) +} + +fn validate_developer_completion( + project: &Project, + branch_name: &str, + developer_report: &str, +) -> Result<(), String> { + if developer_report_indicates_permission_block(developer_report) { + return evaluate_developer_completion(developer_report, 0, false); + } + + let commits = worktree_manager::list_commits( + &project.path, + &project.base_branch, + branch_name, + ) + .map_err(|e| format!("Failed to verify developer commits: {}", e))?; + let diff = worktree_manager::get_diff(&project.path, &project.base_branch, branch_name) + .map_err(|e| format!("Failed to verify developer diff: {}", e))?; + + evaluate_developer_completion(developer_report, commits.len(), !diff.trim().is_empty()) +} + fn recover_interrupted_tickets(db: &Arc>) -> Result { let inflight = { let conn = db.lock().map_err(|e| format!("DB lock failed: {}", e))?; @@ -695,6 +753,26 @@ async fn process_ticket( return Ok(true); } + if let Err(validation_error) = + validate_developer_completion(&project, &branch_name, &developer_report) + { + { + let conn = db.lock().map_err(|e| format!("DB lock: {}", e))?; + ProcessedTicket::set_developer_report(&conn, &ticket.id, &developer_report) + .map_err(|e| format!("set_developer_report: {}", e))?; + } + + record_ticket_error( + db, + app_handle, + &project.id, + &ticket.id, + ticket.artifact_id, + &validation_error, + ); + return Ok(true); + } + { let conn = db.lock().map_err(|e| format!("DB lock: {}", e))?; ProcessedTicket::set_developer_report(&conn, &ticket.id, &developer_report) @@ -751,6 +829,8 @@ pub fn start(db: Arc>, app_handle: AppHandle, process_registry mod tests { use super::*; use crate::db; + use std::path::Path; + use std::process::Command; #[test] fn test_build_analyst_prompt_contains_ticket_info() { @@ -878,4 +958,151 @@ mod tests { ProcessedTicket::get_by_id(&guard, &ticket.id).expect("ticket lookup should succeed"); assert_eq!(updated.status, "Pending"); } + + #[test] + fn test_evaluate_developer_completion_rejects_permission_block_report() { + let result = evaluate_developer_completion( + "Je suis bloque par un refus d'autorisation pour l'edition des fichiers.", + 1, + true, + ); + assert!(result.is_err()); + assert!( + result + .unwrap_err() + .contains("permission") + ); + } + + #[test] + fn test_evaluate_developer_completion_rejects_no_commit() { + let result = evaluate_developer_completion("Correction appliquee.", 0, true); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("commit")); + } + + #[test] + fn test_evaluate_developer_completion_rejects_empty_diff() { + let result = evaluate_developer_completion("Correction appliquee.", 1, false); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("diff")); + } + + #[test] + fn test_evaluate_developer_completion_accepts_valid_fix() { + let result = evaluate_developer_completion("Correction appliquee.", 1, true); + assert!(result.is_ok()); + } + + fn setup_test_repo() -> tempfile::TempDir { + let dir = tempfile::tempdir().expect("temp dir"); + let path = dir.path().to_str().expect("utf8 path"); + + let init = Command::new("git") + .args(["init"]) + .current_dir(path) + .output() + .expect("git init"); + assert!(init.status.success(), "git init should succeed"); + + let set_email = Command::new("git") + .args(["config", "user.email", "orchai-test@example.com"]) + .current_dir(path) + .output() + .expect("git config email"); + assert!(set_email.status.success()); + + let set_name = Command::new("git") + .args(["config", "user.name", "Orchai Test"]) + .current_dir(path) + .output() + .expect("git config name"); + assert!(set_name.status.success()); + + std::fs::write(dir.path().join("README.md"), "# Orchai test repo") + .expect("write readme"); + + let add = Command::new("git") + .args(["add", "."]) + .current_dir(path) + .output() + .expect("git add"); + assert!(add.status.success()); + + let commit = Command::new("git") + .args(["commit", "-m", "init"]) + .current_dir(path) + .output() + .expect("git commit"); + assert!(commit.status.success()); + + dir + } + + fn current_branch(path: &str) -> String { + let output = Command::new("git") + .args(["rev-parse", "--abbrev-ref", "HEAD"]) + .current_dir(path) + .output() + .expect("current branch"); + assert!(output.status.success()); + String::from_utf8_lossy(&output.stdout).trim().to_string() + } + + fn build_project(project_path: &str, base_branch: &str) -> Project { + Project { + id: "p-test".into(), + name: "Test project".into(), + path: project_path.to_string(), + cloned_from: None, + base_branch: base_branch.to_string(), + created_at: "2026-01-01T00:00:00Z".into(), + } + } + + #[test] + fn test_validate_developer_completion_rejects_branch_without_commit() { + let repo = setup_test_repo(); + let repo_path = repo.path().to_str().expect("utf8 path"); + let base_branch = current_branch(repo_path); + let (_wt_path, branch_name) = + worktree_manager::create_worktree(repo_path, &base_branch, 100) + .expect("worktree"); + + let project = build_project(repo_path, &base_branch); + let result = validate_developer_completion(&project, &branch_name, "Correction appliquee."); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("commit")); + } + + #[test] + fn test_validate_developer_completion_accepts_branch_with_commit_and_diff() { + let repo = setup_test_repo(); + let repo_path = repo.path().to_str().expect("utf8 path"); + let base_branch = current_branch(repo_path); + let (wt_path, branch_name) = + worktree_manager::create_worktree(repo_path, &base_branch, 101) + .expect("worktree"); + + let fix_file = Path::new(&wt_path).join("fix.txt"); + std::fs::write(&fix_file, "critical fix").expect("write fix"); + + let add = Command::new("git") + .args(["add", "."]) + .current_dir(&wt_path) + .output() + .expect("git add"); + assert!(add.status.success()); + + let commit = Command::new("git") + .args(["commit", "-m", "fix: add critical fix"]) + .current_dir(&wt_path) + .output() + .expect("git commit"); + assert!(commit.status.success()); + + let project = build_project(repo_path, &base_branch); + let result = validate_developer_completion(&project, &branch_name, "Correction appliquee."); + assert!(result.is_ok()); + } }