fix: /review to respect session cwd (#8738)
Fixes /review base-branch prompt resolution to use the session/turn cwd (respecting runtime cwd overrides) so merge-base/diff guidance is computed from the intended repo; adds a regression test for cwd overrides; tested with cargo test -p codex-core --test all review_uses_overridden_cwd_for_base_branch_merge_base.
This commit is contained in:
parent
649badd102
commit
0b53aed2d0
2 changed files with 100 additions and 1 deletions
|
|
@ -2068,7 +2068,7 @@ mod handlers {
|
|||
review_request: ReviewRequest,
|
||||
) {
|
||||
let turn_context = sess.new_default_turn_with_sub_id(sub_id.clone()).await;
|
||||
match resolve_review_request(review_request, config.cwd.as_path()) {
|
||||
match resolve_review_request(review_request, turn_context.cwd.as_path()) {
|
||||
Ok(resolved) => {
|
||||
spawn_review_thread(
|
||||
Arc::clone(sess),
|
||||
|
|
|
|||
|
|
@ -709,6 +709,105 @@ async fn review_history_surfaces_in_parent_session() {
|
|||
server.verify().await;
|
||||
}
|
||||
|
||||
/// `/review` should use the session's current cwd (including runtime overrides)
|
||||
/// when resolving base-branch review prompts (merge-base computation).
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn review_uses_overridden_cwd_for_base_branch_merge_base() {
|
||||
skip_if_no_network!();
|
||||
|
||||
let sse_raw = r#"[{"type":"response.completed", "response": {"id": "__ID__"}}]"#;
|
||||
let server = start_responses_server_with_sse(sse_raw, 1).await;
|
||||
|
||||
let initial_cwd = TempDir::new().unwrap();
|
||||
|
||||
let repo_dir = TempDir::new().unwrap();
|
||||
let repo_path = repo_dir.path();
|
||||
|
||||
fn run_git(repo_path: &std::path::Path, args: &[&str]) {
|
||||
let output = std::process::Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(repo_path)
|
||||
.args(args)
|
||||
.output()
|
||||
.expect("spawn git");
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"git {:?} failed: stdout={:?} stderr={:?}",
|
||||
args,
|
||||
String::from_utf8_lossy(&output.stdout),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
}
|
||||
|
||||
run_git(repo_path, &["init", "-b", "main"]);
|
||||
run_git(repo_path, &["config", "user.email", "test@example.com"]);
|
||||
run_git(repo_path, &["config", "user.name", "Test User"]);
|
||||
std::fs::write(repo_path.join("file.txt"), "hello\n").unwrap();
|
||||
run_git(repo_path, &["add", "."]);
|
||||
run_git(repo_path, &["commit", "-m", "initial"]);
|
||||
|
||||
let head_sha = std::process::Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(repo_path)
|
||||
.args(["rev-parse", "HEAD"])
|
||||
.output()
|
||||
.expect("rev-parse HEAD");
|
||||
assert!(head_sha.status.success());
|
||||
let head_sha = String::from_utf8(head_sha.stdout)
|
||||
.expect("utf8 sha")
|
||||
.trim()
|
||||
.to_string();
|
||||
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
let codex = new_conversation_for_server(&server, &codex_home, |config| {
|
||||
config.cwd = initial_cwd.path().to_path_buf();
|
||||
})
|
||||
.await;
|
||||
|
||||
codex
|
||||
.submit(Op::OverrideTurnContext {
|
||||
cwd: Some(repo_path.to_path_buf()),
|
||||
approval_policy: None,
|
||||
sandbox_policy: None,
|
||||
model: None,
|
||||
effort: None,
|
||||
summary: None,
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
codex
|
||||
.submit(Op::Review {
|
||||
review_request: ReviewRequest {
|
||||
target: ReviewTarget::BaseBranch {
|
||||
branch: "main".to_string(),
|
||||
},
|
||||
user_facing_hint: None,
|
||||
},
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let _entered = wait_for_event(&codex, |ev| matches!(ev, EventMsg::EnteredReviewMode(_))).await;
|
||||
let _complete = wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await;
|
||||
|
||||
let requests = get_responses_requests(&server).await;
|
||||
assert_eq!(requests.len(), 1);
|
||||
let body = requests[0].body_json::<serde_json::Value>().unwrap();
|
||||
let input = body["input"].as_array().expect("input array");
|
||||
|
||||
let saw_merge_base_sha = input
|
||||
.iter()
|
||||
.filter_map(|msg| msg["content"][0]["text"].as_str())
|
||||
.any(|text| text.contains(&head_sha));
|
||||
assert!(
|
||||
saw_merge_base_sha,
|
||||
"expected review prompt to include merge-base sha {head_sha}"
|
||||
);
|
||||
|
||||
server.verify().await;
|
||||
}
|
||||
|
||||
/// Start a mock Responses API server and mount the given SSE stream body.
|
||||
async fn start_responses_server_with_sse(sse_raw: &str, expected_requests: usize) -> MockServer {
|
||||
let server = MockServer::start().await;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue