Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 48 additions & 2 deletions codex-rs/core/src/session/turn.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::path::PathBuf;
use std::sync::Arc;
use std::sync::atomic::Ordering;

Expand Down Expand Up @@ -67,8 +68,10 @@ use codex_analytics::InvocationType;
use codex_analytics::TurnResolvedConfigFact;
use codex_analytics::build_track_events_context;
use codex_async_utils::OrCancelExt;
use codex_exec_server::ExecutorFileSystem;
use codex_features::Feature;
use codex_git_utils::get_git_repo_root;
use codex_git_utils::get_git_repo_root_with_fs;
use codex_hooks::HookEvent;
use codex_hooks::HookEventAfterAgent;
use codex_hooks::HookPayload;
Expand Down Expand Up @@ -100,6 +103,7 @@ use codex_protocol::protocol::WarningEvent;
use codex_protocol::user_input::UserInput;
use codex_tools::ToolName;
use codex_tools::filter_request_plugin_install_discoverable_tools_for_client;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_stream_parser::AssistantTextChunk;
use codex_utils_stream_parser::AssistantTextStreamParser;
use codex_utils_stream_parser::ProposedPlanSegment;
Expand Down Expand Up @@ -367,8 +371,11 @@ pub(crate) async fn run_turn(
// Although from the perspective of codex.rs, TurnDiffTracker has the lifecycle of a Task which contains
// many turns, from the perspective of the user, it is a single turn.
#[allow(deprecated)]
let display_root = get_git_repo_root(turn_context.cwd.as_path())
.unwrap_or_else(|| turn_context.cwd.clone().into_path_buf());
let display_root = turn_diff_display_root(
&turn_context.cwd,
turn_context.environments.primary_filesystem(),
)
.await;
let turn_diff_tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::with_display_root(
display_root,
)));
Expand Down Expand Up @@ -2262,6 +2269,19 @@ async fn try_run_sampling_request(
outcome
}

async fn turn_diff_display_root(
cwd: &AbsolutePathBuf,
fs: Option<Arc<dyn ExecutorFileSystem>>,
) -> PathBuf {
match fs {
Some(fs) => get_git_repo_root_with_fs(fs.as_ref(), cwd)
.await
.map(AbsolutePathBuf::into_path_buf),
None => get_git_repo_root(cwd.as_path()),
}
.unwrap_or_else(|| cwd.clone().into_path_buf())
}

pub(crate) fn get_last_assistant_message_from_turn(responses: &[ResponseItem]) -> Option<String> {
for item in responses.iter().rev() {
if let Some(message) = last_assistant_message_from_item(item, /*plan_mode*/ false) {
Expand All @@ -2270,3 +2290,29 @@ pub(crate) fn get_last_assistant_message_from_turn(responses: &[ResponseItem]) -
}
None
}

#[cfg(test)]
mod tests {
use super::*;
use codex_exec_server::LOCAL_FS;
use pretty_assertions::assert_eq;

#[tokio::test]
async fn turn_diff_display_root_uses_selected_filesystem_or_local_fallback() {
let repo = tempfile::tempdir().expect("repo tempdir");
std::fs::create_dir(repo.path().join(".git")).expect("create git dir");
let nested = repo.path().join("nested");
std::fs::create_dir(&nested).expect("create nested dir");
let nested = AbsolutePathBuf::try_from(nested).expect("absolute nested path");
let repo_root = repo.path().to_path_buf();

assert_eq!(
turn_diff_display_root(&nested, Some(Arc::clone(&LOCAL_FS))).await,
repo_root
);
assert_eq!(
turn_diff_display_root(&nested, /*fs*/ None).await,
repo_root
);
}
}
95 changes: 61 additions & 34 deletions codex-rs/core/tests/suite/apply_patch_cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use codex_protocol::user_input::UserInput;
use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0;
use codex_utils_absolute_path::AbsolutePathBuf;
use core_test_support::assert_regex_match;
use core_test_support::get_remote_test_env;
use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_completed;
use core_test_support::responses::ev_function_call;
Expand Down Expand Up @@ -67,6 +68,15 @@ async fn apply_patch_harness_with(
Box::pin(TestCodexHarness::with_remote_aware_builder(builder)).await
}

async fn local_apply_patch_harness_with(
configure: impl FnOnce(TestCodexBuilder) -> TestCodexBuilder,
) -> Result<TestCodexHarness> {
let builder = configure(test_codex()).with_config(|config| {
config.include_apply_patch_tool = true;
});
Box::pin(TestCodexHarness::with_builder(builder)).await
}

async fn submit_without_wait(harness: &TestCodexHarness, prompt: &str) -> Result<()> {
submit_without_wait_with_turn_permissions(
harness,
Expand Down Expand Up @@ -1358,40 +1368,9 @@ async fn apply_patch_shell_command_heredoc_with_cd_emits_turn_diff() -> Result<(
Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn apply_patch_turn_diff_paths_stay_repo_relative_when_session_cwd_is_nested() -> Result<()> {
skip_if_no_network!(Ok(()));

let harness = apply_patch_harness_with(|builder| {
builder
.with_model("gpt-5.4")
.with_config(|config| {
config.cwd = config.cwd.join("subdir");
})
.with_workspace_setup(|cwd, fs| async move {
fs.create_directory(
&cwd,
CreateDirectoryOptions { recursive: true },
/*sandbox*/ None,
)
.await?;
let repo_root = cwd.parent().expect("nested cwd should have parent");
fs.write_file(
&repo_root.join(".git"),
b"gitdir: /tmp/fake-worktree\n".to_vec(),
/*sandbox*/ None,
)
.await?;
fs.write_file(
&repo_root.join("repo.txt"),
b"before\n".to_vec(),
/*sandbox*/ None,
)
.await?;
Ok(())
})
})
.await?;
async fn assert_apply_patch_turn_diff_paths_stay_repo_relative_when_session_cwd_is_nested(
harness: TestCodexHarness,
) -> Result<()> {
let test = harness.test();
let codex = test.codex.clone();
let repo_root = harness
Expand Down Expand Up @@ -1437,6 +1416,54 @@ async fn apply_patch_turn_diff_paths_stay_repo_relative_when_session_cwd_is_nest
Ok(())
}

fn nested_repo_relative_diff_builder(builder: TestCodexBuilder) -> TestCodexBuilder {
builder
.with_model("gpt-5.4")
.with_config(|config| {
config.cwd = config.cwd.join("subdir");
})
.with_workspace_setup(|cwd, fs| async move {
fs.create_directory(
&cwd,
CreateDirectoryOptions { recursive: true },
/*sandbox*/ None,
)
.await?;
let repo_root = cwd.parent().expect("nested cwd should have parent");
fs.write_file(
&repo_root.join(".git"),
b"gitdir: /tmp/fake-worktree\n".to_vec(),
/*sandbox*/ None,
)
.await?;
fs.write_file(
&repo_root.join("repo.txt"),
b"before\n".to_vec(),
/*sandbox*/ None,
)
.await?;
Ok(())
})
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn apply_patch_turn_diff_paths_stay_repo_relative_when_session_cwd_is_nested() -> Result<()> {
let harness = local_apply_patch_harness_with(nested_repo_relative_diff_builder).await?;
assert_apply_patch_turn_diff_paths_stay_repo_relative_when_session_cwd_is_nested(harness).await
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn apply_patch_turn_diff_paths_stay_repo_relative_for_remote_nested_session_cwd() -> Result<()>
{
skip_if_no_network!(Ok(()));
let Some(_remote_env) = get_remote_test_env() else {
return Ok(());
};

let harness = apply_patch_harness_with(nested_repo_relative_diff_builder).await?;
assert_apply_patch_turn_diff_paths_stay_repo_relative_when_session_cwd_is_nested(harness).await
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn apply_patch_shell_command_failure_propagates_error_and_skips_diff() -> Result<()> {
skip_if_no_network!(Ok(()));
Expand Down
15 changes: 15 additions & 0 deletions codex-rs/git-utils/src/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,21 @@ pub fn get_git_repo_root(base_dir: &Path) -> Option<PathBuf> {
find_ancestor_git_entry(base).map(|(repo_root, _)| repo_root)
}

/// Return the git repository root for `base_dir` using the provided executor
/// filesystem. This is the remote-environment equivalent of [`get_git_repo_root`].
pub async fn get_git_repo_root_with_fs(
fs: &dyn ExecutorFileSystem,
base_dir: &AbsolutePathBuf,
) -> Option<AbsolutePathBuf> {
let base = match fs.get_metadata(base_dir, /*sandbox*/ None).await {
Ok(metadata) if metadata.is_directory => base_dir.clone(),
_ => base_dir.parent()?,
};
find_ancestor_git_entry_with_fs(fs, &base)
.await
.map(|(repo_root, _)| repo_root)
}

/// Timeout for git commands to prevent freezing on large repositories
const GIT_COMMAND_TIMEOUT: TokioDuration = TokioDuration::from_secs(5);

Expand Down
1 change: 1 addition & 0 deletions codex-rs/git-utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ pub use info::default_branch_name;
pub use info::get_git_remote_urls;
pub use info::get_git_remote_urls_assume_git_repo;
pub use info::get_git_repo_root;
pub use info::get_git_repo_root_with_fs;
pub use info::get_has_changes;
pub use info::get_head_commit_hash;
pub use info::git_diff_to_remote;
Expand Down
Loading