Preserve snapshot git runtime config

Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
Gabriel Cohen
2026-03-19 15:01:57 -07:00
parent 3483340191
commit 71ed2fdb39
8 changed files with 204 additions and 46 deletions

View File

@@ -22,30 +22,28 @@ const COMMIT_HOOK_NAMES: &[&str] = &[
pub(crate) fn configure_git_hooks_env_for_config(
env: &mut HashMap<String, String>,
config: &Config,
) {
) -> Vec<(String, String)> {
configure_git_hooks_env(
env,
config.codex_home.as_path(),
config.commit_attribution.as_deref(),
);
)
}
pub(crate) fn configure_git_hooks_env(
env: &mut HashMap<String, String>,
codex_home: &Path,
config_attribution: Option<&str>,
) {
let Some(value) = resolve_attribution_value(config_attribution) else {
return;
) -> Vec<(String, String)> {
let Some((key, value)) = git_hooks_runtime_config(codex_home, config_attribution) else {
return Vec::new();
};
let Ok(hooks_path) = ensure_codex_hook_scripts(codex_home, &value) else {
return;
};
set_git_runtime_config(env, "core.hooksPath", hooks_path.to_string_lossy().as_ref());
set_git_runtime_config(env, &key, &value);
vec![(key, value)]
}
#[cfg(test)]
pub(crate) fn injected_git_config_env(env: &HashMap<String, String>) -> Vec<(String, String)> {
let mut pairs = env
.iter()
@@ -74,6 +72,18 @@ fn resolve_attribution_value(config_attribution: Option<&str>) -> Option<String>
}
}
fn git_hooks_runtime_config(
codex_home: &Path,
config_attribution: Option<&str>,
) -> Option<(String, String)> {
let value = resolve_attribution_value(config_attribution)?;
let hooks_path = ensure_codex_hook_scripts(codex_home, &value).ok()?;
Some((
"core.hooksPath".to_string(),
hooks_path.to_string_lossy().into_owned(),
))
}
fn ensure_codex_hook_scripts(codex_home: &Path, value: &str) -> std::io::Result<PathBuf> {
let hooks_dir = codex_home.join("hooks").join("commit-attribution");
fs::create_dir_all(&hooks_dir)?;
@@ -105,14 +115,48 @@ fn build_hook_script(hook_name: &str, value: &str) -> String {
let escaped_value = value.replace('\'', "'\"'\"'");
let prepare_commit_msg_body = if hook_name == PREPARE_COMMIT_MSG_HOOK_NAME {
format!(
"\nmsg_file=\"${{1:-}}\"\nif [[ -n \"$msg_file\" && -f \"$msg_file\" ]]; then\n git interpret-trailers \\\n --in-place \\\n --if-exists doNothing \\\n --if-missing add \\\n --trailer 'Co-authored-by={escaped_value}' \\\n \"$msg_file\" || true\nfi\n"
r#"
msg_file="${{1:-}}"
if [[ -n "$msg_file" && -f "$msg_file" ]]; then
git interpret-trailers \
--in-place \
--if-exists doNothing \
--if-missing add \
--trailer 'Co-authored-by={escaped_value}' \
"$msg_file" || true
fi
"#
)
} else {
String::new()
};
format!(
"#!/usr/bin/env bash\nset -euo pipefail\n\nunset GIT_CONFIG_COUNT\nwhile IFS='=' read -r name _; do\n case \"$name\" in\n GIT_CONFIG_KEY_*|GIT_CONFIG_VALUE_*) unset \"$name\" ;;\n esac\ndone < <(env)\n\nexisting_hooks_path=\"$(git config --path core.hooksPath 2>/dev/null || true)\"\nif [[ -z \"$existing_hooks_path\" ]]; then\n git_dir=\"$(git rev-parse --git-common-dir 2>/dev/null || git rev-parse --git-dir 2>/dev/null || true)\"\n if [[ -n \"$git_dir\" ]]; then\n existing_hooks_path=\"$git_dir/hooks\"\n fi\nfi\n\nif [[ -n \"$existing_hooks_path\" ]]; then\n existing_hook=\"$existing_hooks_path/{hook_name}\"\n if [[ -x \"$existing_hook\" && \"$existing_hook\" != \"$0\" ]]; then\n \"$existing_hook\" \"$@\"\n fi\nfi\n{prepare_commit_msg_body}"
r#"#!/usr/bin/env bash
set -euo pipefail
unset GIT_CONFIG_COUNT
while IFS='=' read -r name _; do
case "$name" in
GIT_CONFIG_KEY_*|GIT_CONFIG_VALUE_*) unset "$name" ;;
esac
done < <(env)
existing_hooks_path="$(git config --path core.hooksPath 2>/dev/null || true)"
if [[ -z "$existing_hooks_path" ]]; then
git_dir="$(git rev-parse --git-common-dir 2>/dev/null || git rev-parse --git-dir 2>/dev/null || true)"
if [[ -n "$git_dir" ]]; then
existing_hooks_path="$git_dir/hooks"
fi
fi
if [[ -n "$existing_hooks_path" ]]; then
existing_hook="$existing_hooks_path/{hook_name}"
if [[ -x "$existing_hook" && "$existing_hook" != "$0" ]]; then
"$existing_hook" "$@"
fi
fi
{prepare_commit_msg_body}"#
)
}

View File

@@ -130,6 +130,7 @@ pub(crate) async fn execute_user_shell_command(
session_shell.as_ref(),
turn_context.cwd.as_path(),
&turn_context.shell_environment_policy.r#set,
&[],
);
let call_id = Uuid::new_v4().to_string();

View File

@@ -6,7 +6,6 @@ use std::sync::Arc;
use crate::codex::TurnContext;
use crate::commit_attribution::configure_git_hooks_env_for_config;
use crate::commit_attribution::injected_git_config_env;
use crate::exec::ExecParams;
use crate::exec_env::create_env;
use crate::exec_policy::ExecApprovalRequest;
@@ -326,9 +325,11 @@ impl ShellHandler {
} = args;
let mut exec_params = exec_params;
if session.features().enabled(Feature::CodexGitCommit) {
configure_git_hooks_env_for_config(&mut exec_params.env, turn.config.as_ref());
}
let runtime_git_config_overrides = if session.features().enabled(Feature::CodexGitCommit) {
configure_git_hooks_env_for_config(&mut exec_params.env, turn.config.as_ref())
} else {
Vec::new()
};
let dependency_env = session.dependency_env().await;
if !dependency_env.is_empty() {
exec_params.env.extend(dependency_env.clone());
@@ -340,10 +341,6 @@ impl ShellHandler {
explicit_env_overrides.insert(key.clone(), value.clone());
}
}
for (key, value) in injected_git_config_env(&exec_params.env) {
explicit_env_overrides.insert(key, value);
}
let exec_permission_approvals_enabled =
session.features().enabled(Feature::ExecPermissionApprovals);
let requested_additional_permissions = additional_permissions.clone();
@@ -443,6 +440,7 @@ impl ShellHandler {
timeout_ms: exec_params.expiration.timeout_ms(),
env: exec_params.env.clone(),
explicit_env_overrides,
runtime_git_config_overrides,
network: exec_params.network.clone(),
sandbox_permissions: effective_additional_permissions.sandbox_permissions,
additional_permissions: normalized_additional_permissions,

View File

@@ -70,6 +70,7 @@ pub(crate) fn maybe_wrap_shell_lc_with_snapshot(
session_shell: &Shell,
cwd: &Path,
explicit_env_overrides: &HashMap<String, String>,
runtime_git_config_overrides: &[(String, String)],
) -> Vec<String> {
if cfg!(windows) {
return command.to_vec();
@@ -113,13 +114,14 @@ pub(crate) fn maybe_wrap_shell_lc_with_snapshot(
.map(|arg| format!(" '{}'", shell_single_quote(arg)))
.collect::<String>();
let (override_captures, override_exports) = build_override_exports(explicit_env_overrides);
let rewritten_script = if override_exports.is_empty() {
let runtime_git_config_appends = build_runtime_git_config_appends(runtime_git_config_overrides);
let rewritten_script = if override_exports.is_empty() && runtime_git_config_appends.is_empty() {
format!(
"if . '{snapshot_path}' >/dev/null 2>&1; then :; fi\n\nexec '{original_shell}' -c '{original_script}'{trailing_args}"
)
} else {
format!(
"{override_captures}\n\nif . '{snapshot_path}' >/dev/null 2>&1; then :; fi\n\n{override_exports}\n\nexec '{original_shell}' -c '{original_script}'{trailing_args}"
"{override_captures}\n\nif . '{snapshot_path}' >/dev/null 2>&1; then :; fi\n\n{override_exports}{runtime_git_config_appends}\n\nexec '{original_shell}' -c '{original_script}'{trailing_args}"
)
};
@@ -161,6 +163,29 @@ fn build_override_exports(explicit_env_overrides: &HashMap<String, String>) -> (
(captures, restores)
}
fn build_runtime_git_config_appends(runtime_git_config_overrides: &[(String, String)]) -> String {
if runtime_git_config_overrides.is_empty() {
return String::new();
}
let mut script = String::from(
"\ncase \"${GIT_CONFIG_COUNT:-}\" in\n ''|*[!0-9]*) __CODEX_GIT_CONFIG_INDEX=0 ;;\n *) __CODEX_GIT_CONFIG_INDEX=\"$GIT_CONFIG_COUNT\" ;;\nesac\n",
);
for (key, value) in runtime_git_config_overrides {
let key = shell_single_quote(key);
let value = shell_single_quote(value);
script.push_str(&format!(
"eval \"export GIT_CONFIG_KEY_${{__CODEX_GIT_CONFIG_INDEX}}='{key}'\"\n\
eval \"export GIT_CONFIG_VALUE_${{__CODEX_GIT_CONFIG_INDEX}}='{value}'\"\n\
__CODEX_GIT_CONFIG_INDEX=$((__CODEX_GIT_CONFIG_INDEX + 1))\n"
));
}
script.push_str("export GIT_CONFIG_COUNT=\"$__CODEX_GIT_CONFIG_INDEX\"\n");
script
}
fn is_valid_shell_variable_name(name: &str) -> bool {
let mut chars = name.chars();
let Some(first) = chars.next() else {

View File

@@ -42,8 +42,13 @@ fn maybe_wrap_shell_lc_with_snapshot_bootstraps_in_user_shell() {
"echo hello".to_string(),
];
let rewritten =
maybe_wrap_shell_lc_with_snapshot(&command, &session_shell, dir.path(), &HashMap::new());
let rewritten = maybe_wrap_shell_lc_with_snapshot(
&command,
&session_shell,
dir.path(),
&HashMap::new(),
&[],
);
assert_eq!(rewritten[0], "/bin/zsh");
assert_eq!(rewritten[1], "-c");
@@ -68,8 +73,13 @@ fn maybe_wrap_shell_lc_with_snapshot_escapes_single_quotes() {
"echo 'hello'".to_string(),
];
let rewritten =
maybe_wrap_shell_lc_with_snapshot(&command, &session_shell, dir.path(), &HashMap::new());
let rewritten = maybe_wrap_shell_lc_with_snapshot(
&command,
&session_shell,
dir.path(),
&HashMap::new(),
&[],
);
assert!(rewritten[2].contains(r#"exec '/bin/bash' -c 'echo '"'"'hello'"'"''"#));
}
@@ -91,8 +101,13 @@ fn maybe_wrap_shell_lc_with_snapshot_uses_bash_bootstrap_shell() {
"echo hello".to_string(),
];
let rewritten =
maybe_wrap_shell_lc_with_snapshot(&command, &session_shell, dir.path(), &HashMap::new());
let rewritten = maybe_wrap_shell_lc_with_snapshot(
&command,
&session_shell,
dir.path(),
&HashMap::new(),
&[],
);
assert_eq!(rewritten[0], "/bin/bash");
assert_eq!(rewritten[1], "-c");
@@ -117,8 +132,13 @@ fn maybe_wrap_shell_lc_with_snapshot_uses_sh_bootstrap_shell() {
"echo hello".to_string(),
];
let rewritten =
maybe_wrap_shell_lc_with_snapshot(&command, &session_shell, dir.path(), &HashMap::new());
let rewritten = maybe_wrap_shell_lc_with_snapshot(
&command,
&session_shell,
dir.path(),
&HashMap::new(),
&[],
);
assert_eq!(rewritten[0], "/bin/sh");
assert_eq!(rewritten[1], "-c");
@@ -145,8 +165,13 @@ fn maybe_wrap_shell_lc_with_snapshot_preserves_trailing_args() {
"arg1".to_string(),
];
let rewritten =
maybe_wrap_shell_lc_with_snapshot(&command, &session_shell, dir.path(), &HashMap::new());
let rewritten = maybe_wrap_shell_lc_with_snapshot(
&command,
&session_shell,
dir.path(),
&HashMap::new(),
&[],
);
assert!(
rewritten[2]
@@ -171,8 +196,13 @@ fn maybe_wrap_shell_lc_with_snapshot_skips_when_cwd_mismatch() {
"echo hello".to_string(),
];
let rewritten =
maybe_wrap_shell_lc_with_snapshot(&command, &session_shell, &command_cwd, &HashMap::new());
let rewritten = maybe_wrap_shell_lc_with_snapshot(
&command,
&session_shell,
&command_cwd,
&HashMap::new(),
&[],
);
assert_eq!(rewritten, command);
}
@@ -195,8 +225,13 @@ fn maybe_wrap_shell_lc_with_snapshot_accepts_dot_alias_cwd() {
];
let command_cwd = dir.path().join(".");
let rewritten =
maybe_wrap_shell_lc_with_snapshot(&command, &session_shell, &command_cwd, &HashMap::new());
let rewritten = maybe_wrap_shell_lc_with_snapshot(
&command,
&session_shell,
&command_cwd,
&HashMap::new(),
&[],
);
assert_eq!(rewritten[0], "/bin/zsh");
assert_eq!(rewritten[1], "-c");
@@ -231,6 +266,7 @@ fn maybe_wrap_shell_lc_with_snapshot_restores_explicit_override_precedence() {
&session_shell,
dir.path(),
&explicit_env_overrides,
&[],
);
let output = Command::new(&rewritten[0])
.args(&rewritten[1..])
@@ -265,8 +301,13 @@ fn maybe_wrap_shell_lc_with_snapshot_keeps_snapshot_path_without_override() {
"-lc".to_string(),
"printf '%s' \"$PATH\"".to_string(),
];
let rewritten =
maybe_wrap_shell_lc_with_snapshot(&command, &session_shell, dir.path(), &HashMap::new());
let rewritten = maybe_wrap_shell_lc_with_snapshot(
&command,
&session_shell,
dir.path(),
&HashMap::new(),
&[],
);
let output = Command::new(&rewritten[0])
.args(&rewritten[1..])
.output()
@@ -302,6 +343,7 @@ fn maybe_wrap_shell_lc_with_snapshot_applies_explicit_path_override() {
&session_shell,
dir.path(),
&explicit_env_overrides,
&[],
);
let output = Command::new(&rewritten[0])
.args(&rewritten[1..])
@@ -342,6 +384,7 @@ fn maybe_wrap_shell_lc_with_snapshot_does_not_embed_override_values_in_argv() {
&session_shell,
dir.path(),
&explicit_env_overrides,
&[],
);
assert!(!rewritten[2].contains("super-secret-value"));
@@ -386,6 +429,7 @@ fn maybe_wrap_shell_lc_with_snapshot_preserves_unset_override_variables() {
&session_shell,
dir.path(),
&explicit_env_overrides,
&[],
);
let output = Command::new(&rewritten[0])
@@ -396,3 +440,45 @@ fn maybe_wrap_shell_lc_with_snapshot_preserves_unset_override_variables() {
assert!(output.status.success(), "command failed: {output:?}");
assert_eq!(String::from_utf8_lossy(&output.stdout), "unset");
}
#[test]
fn maybe_wrap_shell_lc_with_snapshot_appends_runtime_git_config_after_snapshot() {
let dir = tempdir().expect("create temp dir");
let snapshot_path = dir.path().join("snapshot.sh");
std::fs::write(
&snapshot_path,
"# Snapshot file\nexport GIT_CONFIG_COUNT='1'\nexport GIT_CONFIG_KEY_0='user.name'\nexport GIT_CONFIG_VALUE_0='Pavel'\n",
)
.expect("write snapshot");
let session_shell = shell_with_snapshot(
ShellType::Bash,
"/bin/bash",
snapshot_path,
dir.path().to_path_buf(),
);
let command = vec![
"/bin/bash".to_string(),
"-lc".to_string(),
"printf '%s|%s|%s|%s|%s' \"$GIT_CONFIG_COUNT\" \"$GIT_CONFIG_KEY_0\" \"$GIT_CONFIG_VALUE_0\" \"$GIT_CONFIG_KEY_1\" \"$GIT_CONFIG_VALUE_1\"".to_string(),
];
let runtime_git_config_overrides =
vec![("core.hooksPath".to_string(), "/codex/hooks".to_string())];
let rewritten = maybe_wrap_shell_lc_with_snapshot(
&command,
&session_shell,
dir.path(),
&HashMap::new(),
&runtime_git_config_overrides,
);
let output = Command::new(&rewritten[0])
.args(&rewritten[1..])
.output()
.expect("run rewritten command");
assert!(output.status.success(), "command failed: {output:?}");
assert_eq!(
String::from_utf8_lossy(&output.stdout),
"2|user.name|Pavel|core.hooksPath|/codex/hooks"
);
}

View File

@@ -48,6 +48,7 @@ pub struct ShellRequest {
pub timeout_ms: Option<u64>,
pub env: HashMap<String, String>,
pub explicit_env_overrides: HashMap<String, String>,
pub runtime_git_config_overrides: Vec<(String, String)>,
pub network: Option<NetworkProxy>,
pub sandbox_permissions: SandboxPermissions,
pub additional_permissions: Option<PermissionProfile>,
@@ -225,6 +226,7 @@ impl ToolRuntime<ShellRequest, ExecToolCallOutput> for ShellRuntime {
session_shell.as_ref(),
&req.cwd,
&req.explicit_env_overrides,
&req.runtime_git_config_overrides,
);
let command = if matches!(session_shell.shell_type, ShellType::PowerShell)
&& ctx.session.features().enabled(Feature::PowershellUtf8)

View File

@@ -50,6 +50,7 @@ pub struct UnifiedExecRequest {
pub cwd: PathBuf,
pub env: HashMap<String, String>,
pub explicit_env_overrides: HashMap<String, String>,
pub runtime_git_config_overrides: Vec<(String, String)>,
pub network: Option<NetworkProxy>,
pub tty: bool,
pub sandbox_permissions: SandboxPermissions,
@@ -195,6 +196,7 @@ impl<'a> ToolRuntime<UnifiedExecRequest, UnifiedExecProcess> for UnifiedExecRunt
session_shell.as_ref(),
&req.cwd,
&req.explicit_env_overrides,
&req.runtime_git_config_overrides,
);
let command = if matches!(session_shell.shell_type, ShellType::PowerShell)
&& ctx.session.features().enabled(Feature::PowershellUtf8)

View File

@@ -14,7 +14,6 @@ use tokio::time::Instant;
use tokio_util::sync::CancellationToken;
use crate::commit_attribution::configure_git_hooks_env_for_config;
use crate::commit_attribution::injected_git_config_env;
use crate::exec_env::create_env;
use crate::exec_policy::ExecApprovalRequest;
use crate::features::Feature;
@@ -577,14 +576,14 @@ impl UnifiedExecProcessManager {
&context.turn.shell_environment_policy,
Some(context.session.conversation_id),
);
if context.turn.features.enabled(Feature::CodexGitCommit) {
configure_git_hooks_env_for_config(&mut env, context.turn.config.as_ref());
}
let runtime_git_config_overrides = if context.turn.features.enabled(Feature::CodexGitCommit)
{
configure_git_hooks_env_for_config(&mut env, context.turn.config.as_ref())
} else {
Vec::new()
};
let env = apply_unified_exec_env(env);
let mut explicit_env_overrides = context.turn.shell_environment_policy.r#set.clone();
for (key, value) in injected_git_config_env(&env) {
explicit_env_overrides.insert(key, value);
}
let explicit_env_overrides = context.turn.shell_environment_policy.r#set.clone();
let mut orchestrator = ToolOrchestrator::new();
let mut runtime =
UnifiedExecRuntime::new(self, context.turn.tools_config.unified_exec_backend);
@@ -610,6 +609,7 @@ impl UnifiedExecProcessManager {
cwd,
env,
explicit_env_overrides,
runtime_git_config_overrides,
network: request.network.clone(),
tty: request.tty,
sandbox_permissions: request.sandbox_permissions,