fix: prevent false done status when developer output is not actionable
This commit is contained in:
parent
1f146a3a95
commit
9ce288a3a2
1 changed files with 227 additions and 0 deletions
|
|
@ -149,6 +149,64 @@ pub fn parse_verdict(report: &str) -> Verdict {
|
||||||
Verdict::FixNeeded
|
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<Mutex<Connection>>) -> Result<usize, String> {
|
fn recover_interrupted_tickets(db: &Arc<Mutex<Connection>>) -> Result<usize, String> {
|
||||||
let inflight = {
|
let inflight = {
|
||||||
let conn = db.lock().map_err(|e| format!("DB lock failed: {}", e))?;
|
let conn = db.lock().map_err(|e| format!("DB lock failed: {}", e))?;
|
||||||
|
|
@ -695,6 +753,26 @@ async fn process_ticket(
|
||||||
return Ok(true);
|
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))?;
|
let conn = db.lock().map_err(|e| format!("DB lock: {}", e))?;
|
||||||
ProcessedTicket::set_developer_report(&conn, &ticket.id, &developer_report)
|
ProcessedTicket::set_developer_report(&conn, &ticket.id, &developer_report)
|
||||||
|
|
@ -751,6 +829,8 @@ pub fn start(db: Arc<Mutex<Connection>>, app_handle: AppHandle, process_registry
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
use crate::db;
|
use crate::db;
|
||||||
|
use std::path::Path;
|
||||||
|
use std::process::Command;
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_build_analyst_prompt_contains_ticket_info() {
|
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");
|
ProcessedTicket::get_by_id(&guard, &ticket.id).expect("ticket lookup should succeed");
|
||||||
assert_eq!(updated.status, "Pending");
|
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());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue