make codex better at git (#10145)

adds basic git context to the session prefix so the model can anchor git
actions and be a bit more version-aware. structured it in a
multiroot-friendly shape even though we only have one root today
This commit is contained in:
pash-openai
2026-02-02 16:57:29 -08:00
committed by GitHub
parent e24058b7a8
commit 019d89ff86
12 changed files with 394 additions and 23 deletions

View File

@@ -1,10 +1,13 @@
use std::path::PathBuf;
use std::sync::Arc;
use std::sync::OnceLock;
use std::sync::RwLock;
use crate::api_bridge::CoreAuthProvider;
use crate::api_bridge::auth_provider_from_auth;
use crate::api_bridge::map_api_error;
use crate::auth::UnauthorizedRecovery;
use crate::turn_metadata::build_turn_metadata_header;
use codex_api::AggregateStreamExt;
use codex_api::ChatClient as ApiChatClient;
use codex_api::CompactClient as ApiCompactClient;
@@ -72,6 +75,13 @@ use crate::transport_manager::TransportManager;
pub const WEB_SEARCH_ELIGIBLE_HEADER: &str = "x-oai-web-search-eligible";
pub const X_CODEX_TURN_STATE_HEADER: &str = "x-codex-turn-state";
pub const X_CODEX_TURN_METADATA_HEADER: &str = "x-codex-turn-metadata";
#[derive(Debug, Default)]
struct TurnMetadataCache {
cwd: Option<PathBuf>,
header: Option<HeaderValue>,
}
#[derive(Debug)]
struct ModelClientState {
@@ -85,6 +95,7 @@ struct ModelClientState {
summary: ReasoningSummaryConfig,
session_source: SessionSource,
transport_manager: TransportManager,
turn_metadata_cache: Arc<RwLock<TurnMetadataCache>>,
}
#[derive(Debug, Clone)]
@@ -136,11 +147,13 @@ impl ModelClient {
summary,
session_source,
transport_manager,
turn_metadata_cache: Arc::new(RwLock::new(TurnMetadataCache::default())),
}),
}
}
pub fn new_session(&self) -> ModelClientSession {
pub fn new_session(&self, turn_metadata_cwd: Option<PathBuf>) -> ModelClientSession {
self.prewarm_turn_metadata_header(turn_metadata_cwd);
ModelClientSession {
state: Arc::clone(&self.state),
connection: None,
@@ -149,6 +162,38 @@ impl ModelClient {
turn_state: Arc::new(OnceLock::new()),
}
}
/// Refresh turn metadata in the background and update a cached header that request
/// builders can read without blocking.
fn prewarm_turn_metadata_header(&self, turn_metadata_cwd: Option<PathBuf>) {
let turn_metadata_cwd =
turn_metadata_cwd.map(|cwd| std::fs::canonicalize(&cwd).unwrap_or(cwd));
if let Ok(mut cache) = self.state.turn_metadata_cache.write()
&& cache.cwd != turn_metadata_cwd
{
cache.cwd = turn_metadata_cwd.clone();
cache.header = None;
}
let Some(cwd) = turn_metadata_cwd else {
return;
};
let turn_metadata_cache = Arc::clone(&self.state.turn_metadata_cache);
if let Ok(handle) = tokio::runtime::Handle::try_current() {
let _task = handle.spawn(async move {
let header = build_turn_metadata_header(cwd.as_path())
.await
.and_then(|value| HeaderValue::from_str(value.as_str()).ok());
if let Ok(mut cache) = turn_metadata_cache.write()
&& cache.cwd.as_ref() == Some(&cwd)
{
cache.header = header;
}
});
}
}
}
impl ModelClient {
@@ -257,6 +302,14 @@ impl ModelClient {
}
impl ModelClientSession {
fn turn_metadata_header(&self) -> Option<HeaderValue> {
self.state
.turn_metadata_cache
.try_read()
.ok()
.and_then(|cache| cache.header.clone())
}
/// Streams a single model turn using either the Responses or Chat
/// Completions wire API, depending on the configured provider.
///
@@ -332,6 +385,7 @@ impl ModelClientSession {
prompt: &Prompt,
compression: Compression,
) -> ApiResponsesOptions {
let turn_metadata_header = self.turn_metadata_header();
let model_info = &self.state.model_info;
let default_reasoning_effort = model_info.default_reasoning_level;
@@ -380,7 +434,11 @@ impl ModelClientSession {
store_override: None,
conversation_id: Some(conversation_id),
session_source: Some(self.state.session_source.clone()),
extra_headers: build_responses_headers(&self.state.config, Some(&self.turn_state)),
extra_headers: build_responses_headers(
&self.state.config,
Some(&self.turn_state),
turn_metadata_header.as_ref(),
),
compression,
turn_state: Some(Arc::clone(&self.turn_state)),
}
@@ -713,6 +771,7 @@ fn experimental_feature_headers(config: &Config) -> ApiHeaderMap {
fn build_responses_headers(
config: &Config,
turn_state: Option<&Arc<OnceLock<String>>>,
turn_metadata_header: Option<&HeaderValue>,
) -> ApiHeaderMap {
let mut headers = experimental_feature_headers(config);
headers.insert(
@@ -731,6 +790,9 @@ fn build_responses_headers(
{
headers.insert(X_CODEX_TURN_STATE_HEADER, header_value);
}
if let Some(header_value) = turn_metadata_header {
headers.insert(X_CODEX_TURN_METADATA_HEADER, header_value.clone());
}
headers
}

View File

@@ -119,6 +119,7 @@ use crate::error::Result as CodexResult;
use crate::exec::StreamOutput;
use crate::exec_policy::ExecPolicyUpdateError;
use crate::feedback_tags;
use crate::git_info::get_git_repo_root;
use crate::instructions::UserInstructions;
use crate::mcp::CODEX_APPS_MCP_SERVER_NAME;
use crate::mcp::auth::compute_auth_statuses;
@@ -531,7 +532,6 @@ pub(crate) struct TurnContext {
pub(crate) truncation_policy: TruncationPolicy,
pub(crate) dynamic_tools: Vec<DynamicToolSpec>,
}
impl TurnContext {
pub(crate) fn resolve_path(&self, path: Option<String>) -> PathBuf {
path.as_ref()
@@ -3406,7 +3406,9 @@ pub(crate) async fn run_turn(
// many turns, from the perspective of the user, it is a single turn.
let turn_diff_tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
let mut client_session = turn_context.client.new_session();
let mut client_session = turn_context
.client
.new_session(Some(turn_context.cwd.clone()));
loop {
// Note that pending_input would be something like a message the user
@@ -4469,8 +4471,6 @@ pub(super) fn get_last_assistant_message_from_turn(responses: &[ResponseItem]) -
#[cfg(test)]
pub(crate) use tests::make_session_and_context;
use crate::git_info::get_git_repo_root;
#[cfg(test)]
pub(crate) use tests::make_session_and_context_with_rx;

View File

@@ -335,7 +335,9 @@ async fn drain_to_completed(
turn_context: &TurnContext,
prompt: &Prompt,
) -> CodexResult<()> {
let mut client_session = turn_context.client.new_session();
let mut client_session = turn_context
.client
.new_session(Some(turn_context.cwd.clone()));
let mut stream = client_session.stream(prompt).await?;
loop {
let maybe_event = stream.next().await;

View File

@@ -28,6 +28,7 @@ impl EnvironmentContext {
cwd,
// should compare all fields except shell
shell: _,
..
} = other;
self.cwd == *cwd

View File

@@ -1,3 +1,4 @@
use std::collections::BTreeMap;
use std::collections::HashSet;
use std::path::Path;
use std::path::PathBuf;
@@ -109,6 +110,73 @@ pub async fn collect_git_info(cwd: &Path) -> Option<GitInfo> {
Some(git_info)
}
/// Collect fetch remotes in a multi-root-friendly format: {"origin": "https://..."}.
pub async fn get_git_remote_urls(cwd: &Path) -> Option<BTreeMap<String, String>> {
let is_git_repo = run_git_command_with_timeout(&["rev-parse", "--git-dir"], cwd)
.await?
.status
.success();
if !is_git_repo {
return None;
}
get_git_remote_urls_assume_git_repo(cwd).await
}
/// Collect fetch remotes without checking whether `cwd` is in a git repo.
pub async fn get_git_remote_urls_assume_git_repo(cwd: &Path) -> Option<BTreeMap<String, String>> {
let output = run_git_command_with_timeout(&["remote", "-v"], cwd).await?;
if !output.status.success() {
return None;
}
let stdout = String::from_utf8(output.stdout).ok()?;
parse_git_remote_urls(stdout.as_str())
}
/// Return the current HEAD commit hash without checking whether `cwd` is in a git repo.
pub async fn get_head_commit_hash(cwd: &Path) -> Option<String> {
let output = run_git_command_with_timeout(&["rev-parse", "HEAD"], cwd).await?;
if !output.status.success() {
return None;
}
let stdout = String::from_utf8(output.stdout).ok()?;
let hash = stdout.trim();
if hash.is_empty() {
None
} else {
Some(hash.to_string())
}
}
fn parse_git_remote_urls(stdout: &str) -> Option<BTreeMap<String, String>> {
let mut remotes = BTreeMap::new();
for line in stdout.lines() {
let Some(fetch_line) = line.strip_suffix(" (fetch)") else {
continue;
};
let Some((name, url_part)) = fetch_line
.split_once('\t')
.or_else(|| fetch_line.split_once(' '))
else {
continue;
};
let url = url_part.trim_start();
if !url.is_empty() {
remotes.insert(name.to_string(), url.to_string());
}
}
if remotes.is_empty() {
None
} else {
Some(remotes)
}
}
/// A minimal commit summary entry used for pickers (subject + timestamp + sha).
#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct CommitLogEntry {
@@ -185,11 +253,9 @@ pub async fn git_diff_to_remote(cwd: &Path) -> Option<GitDiffToRemote> {
/// Run a git command with a timeout to prevent blocking on large repositories
async fn run_git_command_with_timeout(args: &[&str], cwd: &Path) -> Option<std::process::Output> {
let result = timeout(
GIT_COMMAND_TIMEOUT,
Command::new("git").args(args).current_dir(cwd).output(),
)
.await;
let mut command = Command::new("git");
command.args(args).current_dir(cwd).kill_on_drop(true);
let result = timeout(GIT_COMMAND_TIMEOUT, command.output()).await;
match result {
Ok(Ok(output)) => Some(output),

View File

@@ -101,6 +101,7 @@ pub mod state_db;
pub mod terminal;
mod tools;
pub mod turn_diff_tracker;
mod turn_metadata;
pub use rollout::ARCHIVED_SESSIONS_SUBDIR;
pub use rollout::INTERACTIVE_SESSION_SOURCES;
pub use rollout::RolloutRecorder;
@@ -131,6 +132,7 @@ pub mod util;
pub use apply_patch::CODEX_APPLY_PATCH_ARG1;
pub use client::WEB_SEARCH_ELIGIBLE_HEADER;
pub use client::X_CODEX_TURN_METADATA_HEADER;
pub use command_safety::is_dangerous_command;
pub use command_safety::is_safe_command;
pub use exec_policy::ExecPolicyError;

View File

@@ -0,0 +1,43 @@
use std::collections::BTreeMap;
use std::path::Path;
use serde::Serialize;
use crate::git_info::get_git_remote_urls_assume_git_repo;
use crate::git_info::get_git_repo_root;
use crate::git_info::get_head_commit_hash;
#[derive(Serialize)]
struct TurnMetadataWorkspace {
#[serde(skip_serializing_if = "Option::is_none")]
associated_remote_urls: Option<BTreeMap<String, String>>,
#[serde(skip_serializing_if = "Option::is_none")]
latest_git_commit_hash: Option<String>,
}
#[derive(Serialize)]
struct TurnMetadata {
workspaces: BTreeMap<String, TurnMetadataWorkspace>,
}
pub(crate) async fn build_turn_metadata_header(cwd: &Path) -> Option<String> {
let repo_root = get_git_repo_root(cwd)?;
let (latest_git_commit_hash, associated_remote_urls) = tokio::join!(
get_head_commit_hash(cwd),
get_git_remote_urls_assume_git_repo(cwd)
);
if latest_git_commit_hash.is_none() && associated_remote_urls.is_none() {
return None;
}
let mut workspaces = BTreeMap::new();
workspaces.insert(
repo_root.to_string_lossy().into_owned(),
TurnMetadataWorkspace {
associated_remote_urls,
latest_git_commit_hash,
},
);
serde_json::to_string(&TurnMetadata { workspaces }).ok()
}

View File

@@ -102,7 +102,7 @@ async fn run_request(input: Vec<ResponseItem>) -> Value {
SessionSource::Exec,
TransportManager::new(),
)
.new_session();
.new_session(None);
let mut prompt = Prompt::default();
prompt.input = input;

View File

@@ -103,7 +103,7 @@ async fn run_stream_with_bytes(sse_body: &[u8]) -> Vec<ResponseEvent> {
SessionSource::Exec,
TransportManager::new(),
)
.new_session();
.new_session(None);
let mut prompt = Prompt::default();
prompt.input = vec![ResponseItem::Message {

View File

@@ -1,3 +1,4 @@
use std::process::Command;
use std::sync::Arc;
use codex_app_server_protocol::AuthMode;
@@ -23,6 +24,7 @@ use core_test_support::load_default_config_for_test;
use core_test_support::responses;
use core_test_support::test_codex::test_codex;
use futures::StreamExt;
use pretty_assertions::assert_eq;
use tempfile::TempDir;
use wiremock::matchers::header;
@@ -98,7 +100,7 @@ async fn responses_stream_includes_subagent_header_on_review() {
session_source,
TransportManager::new(),
)
.new_session();
.new_session(None);
let mut prompt = Prompt::default();
prompt.input = vec![ResponseItem::Message {
@@ -197,7 +199,7 @@ async fn responses_stream_includes_subagent_header_on_other() {
session_source,
TransportManager::new(),
)
.new_session();
.new_session(None);
let mut prompt = Prompt::default();
prompt.input = vec![ResponseItem::Message {
@@ -354,7 +356,7 @@ async fn responses_respects_model_info_overrides_from_config() {
session_source,
TransportManager::new(),
)
.new_session();
.new_session(None);
let mut prompt = Prompt::default();
prompt.input = vec![ResponseItem::Message {
@@ -393,3 +395,196 @@ async fn responses_respects_model_info_overrides_from_config() {
Some("detailed")
);
}
#[tokio::test]
async fn responses_stream_includes_turn_metadata_header_for_git_workspace_e2e() {
core_test_support::skip_if_no_network!();
let server = responses::start_mock_server().await;
let response_body = responses::sse(vec![
responses::ev_response_created("resp-1"),
responses::ev_completed("resp-1"),
]);
let provider = ModelProviderInfo {
name: "mock".into(),
base_url: Some(format!("{}/v1", server.uri())),
env_key: None,
env_key_instructions: None,
experimental_bearer_token: None,
wire_api: WireApi::Responses,
query_params: None,
http_headers: None,
env_http_headers: None,
request_max_retries: Some(0),
stream_max_retries: Some(0),
stream_idle_timeout_ms: Some(5_000),
requires_openai_auth: false,
supports_websockets: false,
};
let codex_home = TempDir::new().expect("failed to create TempDir");
let mut config = load_default_config_for_test(&codex_home).await;
config.model_provider_id = provider.name.clone();
config.model_provider = provider.clone();
let effort = config.model_reasoning_effort;
let summary = config.model_reasoning_summary;
let model = ModelsManager::get_model_offline(config.model.as_deref());
config.model = Some(model.clone());
let config = Arc::new(config);
let conversation_id = ThreadId::new();
let auth_mode = AuthMode::Chatgpt;
let session_source =
SessionSource::SubAgent(SubAgentSource::Other("turn-metadata-e2e".to_string()));
let model_info = ModelsManager::construct_model_info_offline(model.as_str(), &config);
let otel_manager = OtelManager::new(
conversation_id,
model.as_str(),
model_info.slug.as_str(),
None,
Some("test@test.com".to_string()),
Some(auth_mode),
false,
"test".to_string(),
session_source.clone(),
);
let client = ModelClient::new(
Arc::clone(&config),
None,
model_info,
otel_manager,
provider,
effort,
summary,
conversation_id,
session_source,
TransportManager::new(),
);
let workspace = TempDir::new().expect("workspace tempdir");
let cwd = workspace.path();
let mut prompt = Prompt::default();
prompt.input = vec![ResponseItem::Message {
id: None,
role: "user".into(),
content: vec![ContentItem::InputText {
text: "hello".into(),
}],
end_turn: None,
}];
let first_request = responses::mount_sse_once(&server, response_body.clone()).await;
let mut first_session = client.new_session(Some(cwd.to_path_buf()));
let mut first_stream = first_session
.stream(&prompt)
.await
.expect("stream first turn");
while let Some(event) = first_stream.next().await {
if matches!(event, Ok(ResponseEvent::Completed { .. })) {
break;
}
}
assert_eq!(
first_request
.single_request()
.header("x-codex-turn-metadata"),
None
);
let git_config_global = cwd.join("empty-git-config");
std::fs::write(&git_config_global, "").expect("write empty git config");
let run_git = |args: &[&str]| {
let output = Command::new("git")
.env("GIT_CONFIG_GLOBAL", &git_config_global)
.env("GIT_CONFIG_NOSYSTEM", "1")
.args(args)
.current_dir(cwd)
.output()
.expect("git command should run");
assert!(
output.status.success(),
"git {:?} failed: stdout={} stderr={}",
args,
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
output
};
run_git(&["init"]);
run_git(&["config", "user.name", "Test User"]);
run_git(&["config", "user.email", "test@example.com"]);
std::fs::write(cwd.join("README.md"), "hello").expect("write README");
run_git(&["add", "."]);
run_git(&["commit", "-m", "initial commit"]);
run_git(&[
"remote",
"add",
"origin",
"https://github.com/openai/codex.git",
]);
let expected_head = String::from_utf8(run_git(&["rev-parse", "HEAD"]).stdout)
.expect("git rev-parse output should be valid UTF-8")
.trim()
.to_string();
let expected_origin = String::from_utf8(run_git(&["remote", "get-url", "origin"]).stdout)
.expect("git remote get-url output should be valid UTF-8")
.trim()
.to_string();
let repo_root = std::fs::canonicalize(cwd)
.unwrap_or_else(|_| cwd.to_path_buf())
.to_string_lossy()
.into_owned();
let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(5);
loop {
let request_recorder = responses::mount_sse_once(&server, response_body.clone()).await;
let mut session = client.new_session(Some(cwd.to_path_buf()));
tokio::time::sleep(std::time::Duration::from_millis(50)).await;
let mut stream = session.stream(&prompt).await.expect("stream post-git turn");
while let Some(event) = stream.next().await {
if matches!(event, Ok(ResponseEvent::Completed { .. })) {
break;
}
}
let maybe_header = request_recorder
.single_request()
.header("x-codex-turn-metadata");
if let Some(header_value) = maybe_header {
let parsed: serde_json::Value = serde_json::from_str(&header_value)
.expect("x-codex-turn-metadata should be valid JSON");
let workspace = parsed
.get("workspaces")
.and_then(serde_json::Value::as_object)
.and_then(|workspaces| workspaces.get(&repo_root))
.expect("metadata should include cwd repo root workspace entry");
assert_eq!(
workspace
.get("latest_git_commit_hash")
.and_then(serde_json::Value::as_str),
Some(expected_head.as_str())
);
assert_eq!(
workspace
.get("associated_remote_urls")
.and_then(serde_json::Value::as_object)
.and_then(|remotes| remotes.get("origin"))
.and_then(serde_json::Value::as_str),
Some(expected_origin.as_str())
);
return;
}
if tokio::time::Instant::now() >= deadline {
break;
}
tokio::time::sleep(std::time::Duration::from_millis(25)).await;
}
panic!("x-codex-turn-metadata was never observed within 5s after git setup");
}

View File

@@ -1190,7 +1190,7 @@ async fn azure_responses_request_includes_store_and_reasoning_ids() {
SessionSource::Exec,
TransportManager::new(),
)
.new_session();
.new_session(None);
let mut prompt = Prompt::default();
prompt.input.push(ResponseItem::Reasoning {

View File

@@ -53,7 +53,7 @@ async fn responses_websocket_streams_request() {
.await;
let harness = websocket_harness(&server).await;
let mut session = harness.client.new_session();
let mut session = harness.client.new_session(None);
let prompt = prompt_with_input(vec![message_item("hello")]);
stream_until_complete(&mut session, &prompt).await;
@@ -83,7 +83,7 @@ async fn responses_websocket_emits_websocket_telemetry_events() {
let harness = websocket_harness(&server).await;
harness.otel_manager.reset_runtime_metrics();
let mut session = harness.client.new_session();
let mut session = harness.client.new_session(None);
let prompt = prompt_with_input(vec![message_item("hello")]);
stream_until_complete(&mut session, &prompt).await;
@@ -113,7 +113,7 @@ async fn responses_websocket_emits_reasoning_included_event() {
.await;
let harness = websocket_harness(&server).await;
let mut session = harness.client.new_session();
let mut session = harness.client.new_session(None);
let prompt = prompt_with_input(vec![message_item("hello")]);
let mut stream = session
@@ -147,7 +147,7 @@ async fn responses_websocket_appends_on_prefix() {
.await;
let harness = websocket_harness(&server).await;
let mut session = harness.client.new_session();
let mut session = harness.client.new_session(None);
let prompt_one = prompt_with_input(vec![message_item("hello")]);
let prompt_two = prompt_with_input(vec![message_item("hello"), message_item("second")]);
@@ -183,7 +183,7 @@ async fn responses_websocket_creates_on_non_prefix() {
.await;
let harness = websocket_harness(&server).await;
let mut session = harness.client.new_session();
let mut session = harness.client.new_session(None);
let prompt_one = prompt_with_input(vec![message_item("hello")]);
let prompt_two = prompt_with_input(vec![message_item("different")]);