From d88711f18976d0b3634aa98845328700b1352584 Mon Sep 17 00:00:00 2001 From: Gabriel Cohen Date: Fri, 13 Mar 2026 14:26:22 -0700 Subject: [PATCH] Use git hooks for commit attribution Co-authored-by: Codex --- codex-rs/core/config.schema.json | 1 + codex-rs/core/src/codex.rs | 8 -- codex-rs/core/src/commit_attribution.rs | 106 ++++++++++++++++-- codex-rs/core/src/commit_attribution_tests.rs | 102 ++++++++++++++--- codex-rs/core/src/config/config_tests.rs | 12 ++ codex-rs/core/src/config/mod.rs | 24 ++++ codex-rs/core/src/features.rs | 2 +- codex-rs/core/src/tools/handlers/shell.rs | 8 ++ .../core/src/unified_exec/process_manager.rs | 17 ++- 9 files changed, 245 insertions(+), 35 deletions(-) diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 5a0353b5df..b8a5cde04e 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -1778,6 +1778,7 @@ "description": "Preferred backend for storing CLI auth credentials. file (default): Use a file in the Codex home directory. keyring: Use an OS-specific keyring service. auto: Use the keyring if available, otherwise use a file." }, "commit_attribution": { + "default": null, "description": "Optional commit attribution text for commit message co-author trailers.\n\nSet to an empty string to disable automatic commit attribution.", "type": "string" }, diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 0e5d6bc2aa..3739672dbf 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -17,7 +17,6 @@ use crate::analytics_client::AppInvocation; use crate::analytics_client::InvocationType; use crate::analytics_client::build_track_events_context; use crate::apps::render_apps_section; -use crate::commit_attribution::commit_message_trailer_instruction; use crate::compact; use crate::compact::InitialContextInjection; use crate::compact::run_inline_auto_compact_task; @@ -3433,13 +3432,6 @@ impl Session { if turn_context.apps_enabled() { developer_sections.push(render_apps_section()); } - if turn_context.features.enabled(Feature::CodexGitCommit) - && let Some(commit_message_instruction) = commit_message_trailer_instruction( - turn_context.config.commit_attribution.as_deref(), - ) - { - developer_sections.push(commit_message_instruction); - } if let Some(user_instructions) = turn_context.user_instructions.as_deref() { contextual_user_sections.push( UserInstructions { diff --git a/codex-rs/core/src/commit_attribution.rs b/codex-rs/core/src/commit_attribution.rs index 8df9661431..d396a1b357 100644 --- a/codex-rs/core/src/commit_attribution.rs +++ b/codex-rs/core/src/commit_attribution.rs @@ -1,17 +1,54 @@ -const DEFAULT_ATTRIBUTION_VALUE: &str = "Codex "; +use std::collections::HashMap; +use std::fs; +#[cfg(unix)] +use std::os::unix::fs::PermissionsExt; +use std::path::Path; +use std::path::PathBuf; -fn build_commit_message_trailer(config_attribution: Option<&str>) -> Option { - let value = resolve_attribution_value(config_attribution)?; - Some(format!("Co-authored-by: {value}")) +use crate::config::Config; + +const DEFAULT_ATTRIBUTION_VALUE: &str = "Codex "; +const PREPARE_COMMIT_MSG_HOOK_NAME: &str = "prepare-commit-msg"; + +pub(crate) fn configure_git_hooks_env_for_config( + env: &mut HashMap, + config: &Config, +) { + configure_git_hooks_env( + env, + config.codex_home.as_path(), + config.commit_attribution.as_deref(), + ); } -pub(crate) fn commit_message_trailer_instruction( +pub(crate) fn configure_git_hooks_env( + env: &mut HashMap, + codex_home: &Path, config_attribution: Option<&str>, -) -> Option { - let trailer = build_commit_message_trailer(config_attribution)?; - Some(format!( - "When you write or edit a git commit message, ensure the message ends with this trailer exactly once:\n{trailer}\n\nRules:\n- Keep existing trailers and append this trailer at the end if missing.\n- Do not duplicate this trailer if it already exists.\n- Keep one blank line between the commit body and trailer block." - )) +) { + let Some(value) = resolve_attribution_value(config_attribution) else { + return; + }; + + 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()); +} + +pub(crate) fn injected_git_config_env(env: &HashMap) -> Vec<(String, String)> { + let mut pairs = env + .iter() + .filter(|(key, _)| { + *key == "GIT_CONFIG_COUNT" + || key.starts_with("GIT_CONFIG_KEY_") + || key.starts_with("GIT_CONFIG_VALUE_") + }) + .map(|(key, value)| (key.clone(), value.clone())) + .collect::>(); + pairs.sort_unstable_by(|(left, _), (right, _)| left.cmp(right)); + pairs } fn resolve_attribution_value(config_attribution: Option<&str>) -> Option { @@ -28,6 +65,55 @@ fn resolve_attribution_value(config_attribution: Option<&str>) -> Option } } +fn ensure_codex_hook_scripts(codex_home: &Path, value: &str) -> std::io::Result { + let hooks_dir = codex_home.join("hooks").join("commit-attribution"); + fs::create_dir_all(&hooks_dir)?; + + let script = build_hook_script(value); + let hook_path = hooks_dir.join(PREPARE_COMMIT_MSG_HOOK_NAME); + let should_write = match fs::read_to_string(&hook_path) { + Ok(existing) => existing != script, + Err(_) => true, + }; + + if should_write { + fs::write(&hook_path, script.as_bytes())?; + } + + #[cfg(unix)] + { + let mut perms = fs::metadata(&hook_path)?.permissions(); + perms.set_mode(0o755); + fs::set_permissions(&hook_path, perms)?; + } + + Ok(hooks_dir) +} + +fn build_hook_script(value: &str) -> String { + let escaped_value = value.replace('\'', "'\"'\"'"); + format!( + "#!/usr/bin/env bash\nset -euo pipefail\n\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\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/{PREPARE_COMMIT_MSG_HOOK_NAME}\"\n if [[ -x \"$existing_hook\" && \"$existing_hook\" != \"$0\" ]]; then\n \"$existing_hook\" \"$@\"\n fi\nfi\n" + ) +} + +fn set_git_runtime_config(env: &mut HashMap, key: &str, value: &str) { + let mut index = env + .get("GIT_CONFIG_COUNT") + .and_then(|count| count.parse::().ok()) + .unwrap_or(0); + + while env.contains_key(&format!("GIT_CONFIG_KEY_{index}")) + || env.contains_key(&format!("GIT_CONFIG_VALUE_{index}")) + { + index += 1; + } + + env.insert(format!("GIT_CONFIG_KEY_{index}"), key.to_string()); + env.insert(format!("GIT_CONFIG_VALUE_{index}"), value.to_string()); + env.insert("GIT_CONFIG_COUNT".to_string(), (index + 1).to_string()); +} + #[cfg(test)] #[path = "commit_attribution_tests.rs"] mod tests; diff --git a/codex-rs/core/src/commit_attribution_tests.rs b/codex-rs/core/src/commit_attribution_tests.rs index be7661a604..8413d8cb98 100644 --- a/codex-rs/core/src/commit_attribution_tests.rs +++ b/codex-rs/core/src/commit_attribution_tests.rs @@ -1,19 +1,43 @@ -use super::build_commit_message_trailer; -use super::commit_message_trailer_instruction; +use super::configure_git_hooks_env; +use super::configure_git_hooks_env_for_config; +use super::injected_git_config_env; use super::resolve_attribution_value; +use crate::config::test_config; +use pretty_assertions::assert_eq; +use std::collections::HashMap; +use std::fs; +use tempfile::tempdir; #[test] -fn blank_attribution_disables_trailer_prompt() { - assert_eq!(build_commit_message_trailer(Some("")), None); - assert_eq!(commit_message_trailer_instruction(Some(" ")), None); +fn blank_attribution_disables_hook_env_injection() { + let tmp = tempdir().expect("create temp dir"); + let mut env = HashMap::new(); + + configure_git_hooks_env(&mut env, tmp.path(), Some("")); + + assert!(env.is_empty()); } #[test] fn default_attribution_uses_codex_trailer() { + let tmp = tempdir().expect("create temp dir"); + let mut env = HashMap::new(); + + configure_git_hooks_env(&mut env, tmp.path(), None); + + let hook_path = tmp + .path() + .join("hooks") + .join("commit-attribution") + .join("prepare-commit-msg"); + let script = fs::read_to_string(hook_path).expect("read generated hook"); + + assert_eq!(env.get("GIT_CONFIG_COUNT"), Some(&"1".to_string())); assert_eq!( - build_commit_message_trailer(None).as_deref(), - Some("Co-authored-by: Codex ") + env.get("GIT_CONFIG_KEY_0"), + Some(&"core.hooksPath".to_string()) ); + assert!(script.contains("Co-authored-by=Codex ")); } #[test] @@ -34,10 +58,62 @@ fn resolve_value_handles_default_custom_and_blank() { } #[test] -fn instruction_mentions_trailer_and_omits_generated_with() { - let instruction = commit_message_trailer_instruction(Some("AgentX ")) - .expect("instruction expected"); - assert!(instruction.contains("Co-authored-by: AgentX ")); - assert!(instruction.contains("exactly once")); - assert!(!instruction.contains("Generated-with")); +fn custom_attribution_writes_custom_hook_script() { + let tmp = tempdir().expect("create temp dir"); + let mut env = HashMap::new(); + + configure_git_hooks_env(&mut env, tmp.path(), Some("AgentX ")); + + let hook_path = tmp + .path() + .join("hooks") + .join("commit-attribution") + .join("prepare-commit-msg"); + let script = fs::read_to_string(hook_path).expect("read generated hook"); + + assert!(script.contains("Co-authored-by=AgentX ")); + assert!(script.contains("existing_hook")); +} + +#[test] +fn helper_configures_env_from_config() { + let tmp = tempdir().expect("create temp dir"); + let mut config = test_config(); + config.codex_home = tmp.path().to_path_buf(); + config.commit_attribution = Some("AgentX ".to_string()); + let mut env = HashMap::new(); + + configure_git_hooks_env_for_config(&mut env, &config); + + assert_eq!(env.get("GIT_CONFIG_COUNT"), Some(&"1".to_string())); + assert_eq!( + env.get("GIT_CONFIG_KEY_0"), + Some(&"core.hooksPath".to_string()) + ); + assert!( + env.get("GIT_CONFIG_VALUE_0") + .expect("missing hooks path") + .contains("commit-attribution") + ); +} + +#[test] +fn injected_git_config_env_returns_sorted_runtime_overrides() { + let env = HashMap::from([ + ("IGNORED".to_string(), "value".to_string()), + ("GIT_CONFIG_VALUE_1".to_string(), "beta".to_string()), + ("GIT_CONFIG_KEY_0".to_string(), "core.hooksPath".to_string()), + ("GIT_CONFIG_COUNT".to_string(), "2".to_string()), + ("GIT_CONFIG_VALUE_0".to_string(), "/tmp/hooks".to_string()), + ]); + + assert_eq!( + injected_git_config_env(&env), + vec![ + ("GIT_CONFIG_COUNT".to_string(), "2".to_string()), + ("GIT_CONFIG_KEY_0".to_string(), "core.hooksPath".to_string()), + ("GIT_CONFIG_VALUE_0".to_string(), "/tmp/hooks".to_string()), + ("GIT_CONFIG_VALUE_1".to_string(), "beta".to_string()), + ] + ); } diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 00e5a17564..d2b60381e1 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -158,6 +158,18 @@ consolidation_model = "gpt-5" ); } +#[test] +fn config_toml_ignores_malformed_commit_attribution() { + let cfg = toml::from_str::( + r#" +commit_attribution = true +"#, + ) + .expect("TOML deserialization should succeed"); + + assert_eq!(cfg.commit_attribution, None); +} + #[test] fn parses_bundled_skills_config() { let cfg: ConfigToml = toml::from_str( diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index e4e90fca46..89e399c52a 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -84,6 +84,7 @@ use schemars::JsonSchema; use serde::Deserialize; use serde::Deserializer; use serde::Serialize; +use serde::de::IgnoredAny; use similar::DiffableStr; use std::collections::BTreeMap; use std::collections::HashMap; @@ -1103,6 +1104,7 @@ pub struct ConfigToml { /// Optional commit attribution text for commit message co-author trailers. /// /// Set to an empty string to disable automatic commit attribution. + #[serde(default, deserialize_with = "deserialize_optional_commit_attribution")] pub commit_attribution: Option, /// When set, restricts ChatGPT login to a specific workspace identifier. @@ -1411,6 +1413,28 @@ enum WebSearchToolConfigInput { Config(WebSearchToolConfig), } +#[derive(Deserialize)] +#[serde(untagged)] +enum CommitAttributionInput { + String(String), + Ignored(IgnoredAny), +} + +fn deserialize_optional_commit_attribution<'de, D>( + deserializer: D, +) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + let value = Option::::deserialize(deserializer)?; + + Ok(match value { + None => None, + Some(CommitAttributionInput::String(value)) => Some(value), + Some(CommitAttributionInput::Ignored(_)) => None, + }) +} + fn deserialize_optional_web_search_tool_config<'de, D>( deserializer: D, ) -> Result, D::Error> diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index d0e4e290b0..7681d9d986 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -124,7 +124,7 @@ pub enum Feature { RemoteModels, /// Experimental shell snapshotting. ShellSnapshot, - /// Enable git commit attribution guidance via model instructions. + /// Enable git commit attribution via runtime git hook injection. CodexGitCommit, /// Enable runtime metrics snapshots via a manual reader. RuntimeMetrics, diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index d8a564b17c..a9dd04a48e 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -5,6 +5,8 @@ use codex_protocol::models::ShellToolCallParams; 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; @@ -324,6 +326,9 @@ 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 dependency_env = session.dependency_env().await; if !dependency_env.is_empty() { exec_params.env.extend(dependency_env.clone()); @@ -335,6 +340,9 @@ 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); diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 9eb269c07d..2eb18c2224 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -13,8 +13,11 @@ use tokio::time::Duration; 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; use crate::protocol::ExecCommandSource; use crate::sandboxing::ExecRequest; use crate::tools::context::ExecCommandToolOutput; @@ -570,10 +573,18 @@ impl UnifiedExecProcessManager { cwd: PathBuf, context: &UnifiedExecContext, ) -> Result<(UnifiedExecProcess, Option), UnifiedExecError> { - let env = apply_unified_exec_env(create_env( + let mut env = create_env( &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 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 mut orchestrator = ToolOrchestrator::new(); let mut runtime = UnifiedExecRuntime::new(self, context.turn.tools_config.unified_exec_backend); @@ -598,7 +609,7 @@ impl UnifiedExecProcessManager { command: request.command.clone(), cwd, env, - explicit_env_overrides: context.turn.shell_environment_policy.r#set.clone(), + explicit_env_overrides, network: request.network.clone(), tty: request.tty, sandbox_permissions: request.sandbox_permissions,