mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Add cwd-relative turn diff paths (#39625)
## What changed - Add the opt-in `cwd_relative_turn_diffs` feature. - When enabled, render turn diff paths relative to each selected environment's working directory instead of the detected Git root. - Preserve repository-root-relative paths when the feature is disabled. ## Testing - Cover enabled and disabled behavior for nested working directories across supported originators. GitOrigin-RevId: 8b4908706a82b01f33ccd035b0945f9a29fce26f
This commit is contained in:
@@ -548,6 +548,9 @@
|
||||
"current_time_reminder": {
|
||||
"$ref": "#/definitions/FeatureToml_for_CurrentTimeReminderConfigToml"
|
||||
},
|
||||
"cwd_relative_turn_diffs": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"default_mode_request_user_input": {
|
||||
"type": "boolean"
|
||||
},
|
||||
@@ -5418,6 +5421,9 @@
|
||||
"current_time_reminder": {
|
||||
"$ref": "#/definitions/FeatureToml_for_CurrentTimeReminderConfigToml"
|
||||
},
|
||||
"cwd_relative_turn_diffs": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"default_mode_request_user_input": {
|
||||
"type": "boolean"
|
||||
},
|
||||
|
||||
@@ -223,7 +223,27 @@ pub(crate) async fn run_turn(
|
||||
// Keep the exact model-visible state used by this turn and its inline compactions.
|
||||
let (world_state, display_roots) = tokio::join!(
|
||||
sess.record_context_updates_and_set_reference_context_item(first_step_context.as_ref()),
|
||||
turn_diff_display_roots(first_step_context.as_ref()),
|
||||
async {
|
||||
if first_step_context
|
||||
.turn
|
||||
.config
|
||||
.features
|
||||
.enabled(Feature::CwdRelativeTurnDiffs)
|
||||
{
|
||||
first_step_context
|
||||
.environments
|
||||
.turn_environments()
|
||||
.map(|environment| {
|
||||
(
|
||||
environment.selection().environment_id,
|
||||
environment.cwd().clone(),
|
||||
)
|
||||
})
|
||||
.collect()
|
||||
} else {
|
||||
turn_diff_display_roots(first_step_context.as_ref()).await
|
||||
}
|
||||
},
|
||||
);
|
||||
let mut world_state = world_state?;
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use anyhow::Result;
|
||||
use base64::Engine;
|
||||
use base64::engine::general_purpose::STANDARD as BASE64_STANDARD;
|
||||
use codex_core::StartThreadOptions;
|
||||
use codex_core::TurnInputRequest;
|
||||
use core_test_support::responses::ev_apply_patch_custom_tool_call;
|
||||
use core_test_support::responses::ev_apply_patch_shell_command_call_via_heredoc;
|
||||
@@ -67,6 +68,7 @@ use core_test_support::test_target_os;
|
||||
use core_test_support::wait_for_event;
|
||||
use core_test_support::wait_for_event_with_timeout;
|
||||
use serde_json::json;
|
||||
use test_case::test_case;
|
||||
use wiremock::Mock;
|
||||
use wiremock::Respond;
|
||||
use wiremock::ResponseTemplate;
|
||||
@@ -1640,6 +1642,103 @@ async fn apply_patch_turn_diff_paths_stay_repo_relative_when_session_cwd_is_nest
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case("codex_work_web", true, "work.txt"; "web work uses cwd when enabled")]
|
||||
#[test_case("codex_work_mobile", true, "work.txt"; "mobile work uses cwd when enabled")]
|
||||
#[test_case("codex_work_web", false, "subdir/work.txt"; "disabled feature keeps repository root")]
|
||||
#[test_case("codex_work_desktop", true, "work.txt"; "desktop uses cwd when enabled")]
|
||||
#[test_case("codex_work_cca", true, "work.txt"; "legacy cca uses cwd when enabled")]
|
||||
#[test_case("codex_cli_rs", true, "work.txt"; "coding originator uses cwd when enabled")]
|
||||
#[test_case("codex_cli_rs", false, "subdir/work.txt"; "coding originator keeps repository root when disabled")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn apply_patch_turn_diff_skips_git_root_when_feature_is_enabled(
|
||||
originator: &str,
|
||||
cwd_relative_turn_diffs: bool,
|
||||
expected_path: &str,
|
||||
) -> Result<()> {
|
||||
skip_if_wine_exec!(Ok(()), "asserts POSIX repository paths");
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let harness = apply_patch_harness_with(|builder| {
|
||||
builder
|
||||
.with_model("gpt-5.4")
|
||||
.with_config(move |config| {
|
||||
config.cwd = config.cwd.join("subdir");
|
||||
if cwd_relative_turn_diffs {
|
||||
config
|
||||
.features
|
||||
.enable(Feature::CwdRelativeTurnDiffs)
|
||||
.expect("enable turn diff root feature");
|
||||
}
|
||||
})
|
||||
.with_workspace_setup(|cwd, fs| async move {
|
||||
let cwd_uri = PathUri::from_host_native_path(&cwd)?;
|
||||
fs.create_directory(
|
||||
&cwd_uri,
|
||||
CreateDirectoryOptions { recursive: true },
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await?;
|
||||
let repo_root = cwd.parent().expect("nested cwd should have parent");
|
||||
let git_uri = PathUri::from_host_native_path(repo_root.join(".git"))?;
|
||||
let work_file_uri = PathUri::from_host_native_path(cwd.join("work.txt"))?;
|
||||
fs.write_file(
|
||||
&git_uri,
|
||||
b"gitdir: /tmp/fake-worktree\n".to_vec(),
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await?;
|
||||
fs.write_file(&work_file_uri, b"before\n".to_vec(), /*sandbox*/ None)
|
||||
.await?;
|
||||
Ok(())
|
||||
})
|
||||
})
|
||||
.await?;
|
||||
let test = harness.test();
|
||||
let codex = test
|
||||
.thread_manager
|
||||
.start_thread(StartThreadOptions {
|
||||
metrics_service_name: Some(originator.to_string()),
|
||||
environments: Some(test.codex.environment_selections().await),
|
||||
..StartThreadOptions::new(test.config.clone())
|
||||
})
|
||||
.await?
|
||||
.thread;
|
||||
let patch = "*** Begin Patch\n*** Update File: work.txt\n@@\n-before\n+after\n*** End Patch";
|
||||
mount_apply_patch(&harness, "apply-work-diff-root", patch, "updated work file").await;
|
||||
|
||||
codex
|
||||
.start_or_steer_turn(
|
||||
TurnInputRequest::user_input(vec![UserInput::Text {
|
||||
text: "update the work file".into(),
|
||||
text_elements: Vec::new(),
|
||||
}])
|
||||
.with_thread_settings(ThreadSettingsOverrides {
|
||||
approval_policy: Some(AskForApproval::Never),
|
||||
sandbox_policy: Some(SandboxPolicy::DangerFullAccess),
|
||||
..Default::default()
|
||||
}),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let mut last_diff = None;
|
||||
wait_for_event(&codex, |event| match event {
|
||||
EventMsg::TurnDiff(event) => {
|
||||
last_diff = Some(event.unified_diff.clone());
|
||||
false
|
||||
}
|
||||
EventMsg::TurnComplete(_) => true,
|
||||
_ => false,
|
||||
})
|
||||
.await;
|
||||
|
||||
let diff = last_diff.expect("expected TurnDiff event after update");
|
||||
assert!(
|
||||
diff.contains(&format!("diff --git a/{expected_path} b/{expected_path}")),
|
||||
"unexpected turn diff path for {originator}: {diff:?}",
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn apply_patch_shell_command_failure_propagates_error_and_skips_diff() -> Result<()> {
|
||||
// TODO(anp): Remove after apply_patch shell fixtures use target-native commands.
|
||||
|
||||
@@ -146,6 +146,8 @@ pub enum Feature {
|
||||
ShellSnapshot,
|
||||
/// Allow turns to start while selected executors are still starting.
|
||||
DeferredExecutor,
|
||||
/// Use the current working directory for turn diff display paths.
|
||||
CwdRelativeTurnDiffs,
|
||||
/// Enable runtime metrics snapshots via a manual reader.
|
||||
RuntimeMetrics,
|
||||
/// Enable startup memory extraction and file-backed memory consolidation.
|
||||
@@ -874,6 +876,12 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
stage: Stage::UnderDevelopment,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::CwdRelativeTurnDiffs,
|
||||
key: "cwd_relative_turn_diffs",
|
||||
stage: Stage::UnderDevelopment,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::JsRepl,
|
||||
key: "js_repl",
|
||||
|
||||
@@ -59,6 +59,26 @@ fn executor_capability_discovery_is_an_opt_in_map_feature() {
|
||||
assert!(features.enabled(Feature::ExecutorCapabilityDiscovery));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cwd_relative_turn_diffs_is_an_opt_in_map_feature() {
|
||||
let mut features = Features::with_defaults();
|
||||
assert!(!features.enabled(Feature::CwdRelativeTurnDiffs));
|
||||
|
||||
features.apply_map(&BTreeMap::from([(
|
||||
"cwd_relative_turn_diffs".to_string(),
|
||||
true,
|
||||
)]));
|
||||
|
||||
assert!(features.enabled(Feature::CwdRelativeTurnDiffs));
|
||||
|
||||
features.apply_map(&BTreeMap::from([(
|
||||
"cwd_relative_turn_diffs".to_string(),
|
||||
false,
|
||||
)]));
|
||||
|
||||
assert!(!features.enabled(Feature::CwdRelativeTurnDiffs));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn default_enabled_features_are_stable() {
|
||||
for spec in crate::FEATURES {
|
||||
|
||||
Reference in New Issue
Block a user