Skip to content

Commit 0b53aed

Browse files
authored
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.
1 parent 649badd commit 0b53aed

File tree

2 files changed

+100
-1
lines changed

2 files changed

+100
-1
lines changed

codex-rs/core/src/codex.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2068,7 +2068,7 @@ mod handlers {
20682068
review_request: ReviewRequest,
20692069
) {
20702070
let turn_context = sess.new_default_turn_with_sub_id(sub_id.clone()).await;
2071-
match resolve_review_request(review_request, config.cwd.as_path()) {
2071+
match resolve_review_request(review_request, turn_context.cwd.as_path()) {
20722072
Ok(resolved) => {
20732073
spawn_review_thread(
20742074
Arc::clone(sess),

codex-rs/core/tests/suite/review.rs

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -709,6 +709,105 @@ async fn review_history_surfaces_in_parent_session() {
709709
server.verify().await;
710710
}
711711

712+
/// `/review` should use the session's current cwd (including runtime overrides)
713+
/// when resolving base-branch review prompts (merge-base computation).
714+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
715+
async fn review_uses_overridden_cwd_for_base_branch_merge_base() {
716+
skip_if_no_network!();
717+
718+
let sse_raw = r#"[{"type":"response.completed", "response": {"id": "__ID__"}}]"#;
719+
let server = start_responses_server_with_sse(sse_raw, 1).await;
720+
721+
let initial_cwd = TempDir::new().unwrap();
722+
723+
let repo_dir = TempDir::new().unwrap();
724+
let repo_path = repo_dir.path();
725+
726+
fn run_git(repo_path: &std::path::Path, args: &[&str]) {
727+
let output = std::process::Command::new("git")
728+
.arg("-C")
729+
.arg(repo_path)
730+
.args(args)
731+
.output()
732+
.expect("spawn git");
733+
assert!(
734+
output.status.success(),
735+
"git {:?} failed: stdout={:?} stderr={:?}",
736+
args,
737+
String::from_utf8_lossy(&output.stdout),
738+
String::from_utf8_lossy(&output.stderr)
739+
);
740+
}
741+
742+
run_git(repo_path, &["init", "-b", "main"]);
743+
run_git(repo_path, &["config", "user.email", "[email protected]"]);
744+
run_git(repo_path, &["config", "user.name", "Test User"]);
745+
std::fs::write(repo_path.join("file.txt"), "hello\n").unwrap();
746+
run_git(repo_path, &["add", "."]);
747+
run_git(repo_path, &["commit", "-m", "initial"]);
748+
749+
let head_sha = std::process::Command::new("git")
750+
.arg("-C")
751+
.arg(repo_path)
752+
.args(["rev-parse", "HEAD"])
753+
.output()
754+
.expect("rev-parse HEAD");
755+
assert!(head_sha.status.success());
756+
let head_sha = String::from_utf8(head_sha.stdout)
757+
.expect("utf8 sha")
758+
.trim()
759+
.to_string();
760+
761+
let codex_home = TempDir::new().unwrap();
762+
let codex = new_conversation_for_server(&server, &codex_home, |config| {
763+
config.cwd = initial_cwd.path().to_path_buf();
764+
})
765+
.await;
766+
767+
codex
768+
.submit(Op::OverrideTurnContext {
769+
cwd: Some(repo_path.to_path_buf()),
770+
approval_policy: None,
771+
sandbox_policy: None,
772+
model: None,
773+
effort: None,
774+
summary: None,
775+
})
776+
.await
777+
.unwrap();
778+
779+
codex
780+
.submit(Op::Review {
781+
review_request: ReviewRequest {
782+
target: ReviewTarget::BaseBranch {
783+
branch: "main".to_string(),
784+
},
785+
user_facing_hint: None,
786+
},
787+
})
788+
.await
789+
.unwrap();
790+
791+
let _entered = wait_for_event(&codex, |ev| matches!(ev, EventMsg::EnteredReviewMode(_))).await;
792+
let _complete = wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await;
793+
794+
let requests = get_responses_requests(&server).await;
795+
assert_eq!(requests.len(), 1);
796+
let body = requests[0].body_json::<serde_json::Value>().unwrap();
797+
let input = body["input"].as_array().expect("input array");
798+
799+
let saw_merge_base_sha = input
800+
.iter()
801+
.filter_map(|msg| msg["content"][0]["text"].as_str())
802+
.any(|text| text.contains(&head_sha));
803+
assert!(
804+
saw_merge_base_sha,
805+
"expected review prompt to include merge-base sha {head_sha}"
806+
);
807+
808+
server.verify().await;
809+
}
810+
712811
/// Start a mock Responses API server and mount the given SSE stream body.
713812
async fn start_responses_server_with_sse(sse_raw: &str, expected_requests: usize) -> MockServer {
714813
let server = MockServer::start().await;

0 commit comments

Comments
 (0)