fix: allow codex developer agents to write git worktree metadata

This commit is contained in:
thibaud-lclr 2026-04-21 08:57:42 +02:00
parent 9de295dfe8
commit f0850b3fd3

View file

@ -1,4 +1,4 @@
use crate::models::agent::{Agent, AgentRole}; use crate::models::agent::{Agent, AgentRole, AgentTool};
use crate::models::graylog::GraylogCredentials; use crate::models::graylog::GraylogCredentials;
use crate::models::module::{ use crate::models::module::{
ProjectModule, MODULE_GRAYLOG_AUTO_RESOLVE, MODULE_TULEAP_AUTO_RESOLVE, ProjectModule, MODULE_GRAYLOG_AUTO_RESOLVE, MODULE_TULEAP_AUTO_RESOLVE,
@ -11,6 +11,7 @@ use crate::services::process_registry::ProcessRegistry;
use crate::services::{cli_process, notifier, worktree_manager}; use crate::services::{cli_process, notifier, worktree_manager};
use rusqlite::Connection; use rusqlite::Connection;
use std::mem; use std::mem;
use std::path::{Path, PathBuf};
use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::{Arc, Mutex}; use std::sync::{Arc, Mutex};
use tauri::{AppHandle, Emitter}; use tauri::{AppHandle, Emitter};
@ -153,6 +154,64 @@ pub fn parse_verdict(report: &str) -> Verdict {
Verdict::FixNeeded Verdict::FixNeeded
} }
fn resolve_path_from_working_dir(working_dir: &Path, path: &str) -> Option<PathBuf> {
let trimmed = path.trim();
if trimmed.is_empty() {
return None;
}
let candidate = PathBuf::from(trimmed);
if candidate.is_absolute() {
Some(candidate)
} else {
Some(working_dir.join(candidate))
}
}
fn codex_additional_writable_dirs(working_dir: &str) -> Vec<String> {
let output = std::process::Command::new("git")
.args(["rev-parse", "--git-dir", "--git-common-dir"])
.current_dir(working_dir)
.output();
let output = match output {
Ok(value) if value.status.success() => value,
_ => return Vec::new(),
};
let working_dir_path = Path::new(working_dir);
let normalized_working_dir =
std::fs::canonicalize(working_dir_path).unwrap_or_else(|_| working_dir_path.to_path_buf());
let mut dirs = Vec::new();
for line in String::from_utf8_lossy(&output.stdout).lines() {
let Some(path) = resolve_path_from_working_dir(working_dir_path, line) else {
continue;
};
let normalized = std::fs::canonicalize(&path).unwrap_or(path);
if normalized.starts_with(&normalized_working_dir) {
continue;
}
let normalized = normalized.to_string_lossy().to_string();
if !dirs.contains(&normalized) {
dirs.push(normalized);
}
}
dirs
}
fn build_agent_cli_args(agent: &Agent, working_dir: &str) -> Vec<String> {
let mut args = agent.tool.to_non_interactive_args();
if agent.tool == AgentTool::Codex {
for dir in codex_additional_writable_dirs(working_dir) {
args.push("--add-dir".to_string());
args.push(dir);
}
}
args
}
fn developer_report_indicates_permission_block(report: &str) -> bool { fn developer_report_indicates_permission_block(report: &str) -> bool {
let lowered = report.to_lowercase(); let lowered = report.to_lowercase();
[ [
@ -184,7 +243,9 @@ fn evaluate_developer_completion(
} }
if !has_diff { if !has_diff {
return Err("Developer run completed without any diff against the base branch.".to_string()); return Err(
"Developer run completed without any diff against the base branch.".to_string(),
);
} }
Ok(()) Ok(())
@ -199,19 +260,18 @@ fn validate_developer_completion(
return evaluate_developer_completion(developer_report, 0, false); return evaluate_developer_completion(developer_report, 0, false);
} }
let commits = worktree_manager::list_commits( let commits = worktree_manager::list_commits(&project.path, &project.base_branch, branch_name)
&project.path, .map_err(|e| format!("Failed to verify developer commits: {}", e))?;
&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) let diff = worktree_manager::get_diff(&project.path, &project.base_branch, branch_name)
.map_err(|e| format!("Failed to verify developer diff: {}", e))?; .map_err(|e| format!("Failed to verify developer diff: {}", e))?;
evaluate_developer_completion(developer_report, commits.len(), !diff.trim().is_empty()) evaluate_developer_completion(developer_report, commits.len(), !diff.trim().is_empty())
} }
fn should_flush_progress_buffer(buffer: &str, elapsed_since_last_emit: std::time::Duration) -> bool { fn should_flush_progress_buffer(
buffer: &str,
elapsed_since_last_emit: std::time::Duration,
) -> bool {
if buffer.is_empty() { if buffer.is_empty() {
return false; return false;
} }
@ -424,24 +484,18 @@ async fn process_ticket(
None => None, None => None,
}; };
let enabled = ProjectModule::is_enabled( let enabled =
&conn, ProjectModule::is_enabled(&conn, &project.id, MODULE_TULEAP_AUTO_RESOLVE)
&project.id, .map_err(|e| format!("module lookup failed: {}", e))?;
MODULE_TULEAP_AUTO_RESOLVE,
)
.map_err(|e| format!("module lookup failed: {}", e))?;
if enabled { if enabled {
selected = Some((ticket, project, tracker)); selected = Some((ticket, project, tracker));
break; break;
} }
} }
"graylog" => { "graylog" => {
let enabled = ProjectModule::is_enabled( let enabled =
&conn, ProjectModule::is_enabled(&conn, &project.id, MODULE_GRAYLOG_AUTO_RESOLVE)
&project.id, .map_err(|e| format!("module lookup failed: {}", e))?;
MODULE_GRAYLOG_AUTO_RESOLVE,
)
.map_err(|e| format!("module lookup failed: {}", e))?;
if enabled { if enabled {
selected = Some((ticket, project, None)); selected = Some((ticket, project, None));
break; break;
@ -640,7 +694,7 @@ async fn process_ticket(
build_analyst_prompt(&ticket, &project), build_analyst_prompt(&ticket, &project),
&analyst_agent.custom_prompt, &analyst_agent.custom_prompt,
); );
let analyst_args = analyst_agent.tool.to_non_interactive_args(); let analyst_args = build_agent_cli_args(&analyst_agent, &project.path);
let analyst_result = run_cli_command( let analyst_result = run_cli_command(
analyst_agent.tool.to_command(), analyst_agent.tool.to_command(),
&analyst_args, &analyst_args,
@ -753,7 +807,7 @@ async fn process_ticket(
build_developer_prompt(&ticket, &project, &analyst_report, &wt_path), build_developer_prompt(&ticket, &project, &analyst_report, &wt_path),
&developer_agent.custom_prompt, &developer_agent.custom_prompt,
); );
let developer_args = developer_agent.tool.to_non_interactive_args(); let developer_args = build_agent_cli_args(&developer_agent, &wt_path);
let developer_result = run_cli_command( let developer_result = run_cli_command(
developer_agent.tool.to_command(), developer_agent.tool.to_command(),
&developer_args, &developer_args,
@ -998,20 +1052,16 @@ mod tests {
#[test] #[test]
fn test_should_flush_progress_buffer_when_elapsed_interval_exceeded() { fn test_should_flush_progress_buffer_when_elapsed_interval_exceeded() {
let should_flush = should_flush_progress_buffer( let should_flush =
"some progress", should_flush_progress_buffer("some progress", std::time::Duration::from_millis(300));
std::time::Duration::from_millis(300),
);
assert!(should_flush); assert!(should_flush);
} }
#[test] #[test]
fn test_should_flush_progress_buffer_when_buffer_reaches_size_limit() { fn test_should_flush_progress_buffer_when_buffer_reaches_size_limit() {
let payload = "x".repeat(2048); let payload = "x".repeat(2048);
let should_flush = should_flush_progress_buffer( let should_flush =
&payload, should_flush_progress_buffer(&payload, std::time::Duration::from_millis(10));
std::time::Duration::from_millis(10),
);
assert!(should_flush); assert!(should_flush);
} }
@ -1029,11 +1079,7 @@ mod tests {
true, true,
); );
assert!(result.is_err()); assert!(result.is_err());
assert!( assert!(result.unwrap_err().contains("permission"));
result
.unwrap_err()
.contains("permission")
);
} }
#[test] #[test]
@ -1081,8 +1127,7 @@ mod tests {
.expect("git config name"); .expect("git config name");
assert!(set_name.status.success()); assert!(set_name.status.success());
std::fs::write(dir.path().join("README.md"), "# Orchai test repo") std::fs::write(dir.path().join("README.md"), "# Orchai test repo").expect("write readme");
.expect("write readme");
let add = Command::new("git") let add = Command::new("git")
.args(["add", "."]) .args(["add", "."])
@ -1122,14 +1167,86 @@ mod tests {
} }
} }
fn collect_add_dirs(args: &[String]) -> Vec<String> {
let mut dirs = Vec::new();
let mut index = 0usize;
while index + 1 < args.len() {
if args[index] == "--add-dir" {
dirs.push(args[index + 1].clone());
index += 2;
continue;
}
index += 1;
}
dirs
}
fn build_test_agent(tool: crate::models::agent::AgentTool) -> Agent {
Agent {
id: "a-test".into(),
name: "Agent test".into(),
role: AgentRole::Developer,
tool,
custom_prompt: String::new(),
is_default: false,
created_at: "2026-01-01T00:00:00Z".into(),
updated_at: "2026-01-01T00:00:00Z".into(),
}
}
#[test]
fn test_build_agent_cli_args_adds_git_metadata_dirs_for_codex_worktree() {
let repo = setup_test_repo();
let repo_path = repo.path().to_str().expect("utf8 path");
let base_branch = current_branch(repo_path);
let (worktree_path, _branch_name) =
worktree_manager::create_worktree(repo_path, &base_branch, 202)
.expect("worktree creation should succeed");
let agent = build_test_agent(crate::models::agent::AgentTool::Codex);
let args = build_agent_cli_args(&agent, &worktree_path);
let add_dirs = collect_add_dirs(&args);
let rev_parse = Command::new("git")
.args(["rev-parse", "--git-dir", "--git-common-dir"])
.current_dir(&worktree_path)
.output()
.expect("git rev-parse should succeed");
assert!(rev_parse.status.success(), "git rev-parse should succeed");
let expected_dirs: Vec<String> = String::from_utf8_lossy(&rev_parse.stdout)
.lines()
.map(|line| {
let path = Path::new(line);
let absolute = if path.is_absolute() {
path.to_path_buf()
} else {
Path::new(&worktree_path).join(path)
};
std::fs::canonicalize(&absolute)
.unwrap_or(absolute)
.to_string_lossy()
.to_string()
})
.collect();
for expected in expected_dirs {
assert!(
add_dirs.contains(&expected),
"Expected --add-dir to contain '{}', got {:?}",
expected,
add_dirs
);
}
}
#[test] #[test]
fn test_validate_developer_completion_rejects_branch_without_commit() { fn test_validate_developer_completion_rejects_branch_without_commit() {
let repo = setup_test_repo(); let repo = setup_test_repo();
let repo_path = repo.path().to_str().expect("utf8 path"); let repo_path = repo.path().to_str().expect("utf8 path");
let base_branch = current_branch(repo_path); let base_branch = current_branch(repo_path);
let (_wt_path, branch_name) = let (_wt_path, branch_name) =
worktree_manager::create_worktree(repo_path, &base_branch, 100) worktree_manager::create_worktree(repo_path, &base_branch, 100).expect("worktree");
.expect("worktree");
let project = build_project(repo_path, &base_branch); let project = build_project(repo_path, &base_branch);
let result = validate_developer_completion(&project, &branch_name, "Correction appliquee."); let result = validate_developer_completion(&project, &branch_name, "Correction appliquee.");
@ -1143,8 +1260,7 @@ mod tests {
let repo_path = repo.path().to_str().expect("utf8 path"); let repo_path = repo.path().to_str().expect("utf8 path");
let base_branch = current_branch(repo_path); let base_branch = current_branch(repo_path);
let (wt_path, branch_name) = let (wt_path, branch_name) =
worktree_manager::create_worktree(repo_path, &base_branch, 101) worktree_manager::create_worktree(repo_path, &base_branch, 101).expect("worktree");
.expect("worktree");
let fix_file = Path::new(&wt_path).join("fix.txt"); let fix_file = Path::new(&wt_path).join("fix.txt");
std::fs::write(&fix_file, "critical fix").expect("write fix"); std::fs::write(&fix_file, "critical fix").expect("write fix");