mirror of
https://github.com/openai/codex.git
synced 2026-03-14 10:05:37 +03:00
Compare commits
3 Commits
dev/steve/
...
codex/comm
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
3483340191 | ||
|
|
dbb891a65f | ||
|
|
d88711f189 |
@@ -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"
|
||||
},
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -1,17 +1,63 @@
|
||||
const DEFAULT_ATTRIBUTION_VALUE: &str = "Codex <noreply@openai.com>";
|
||||
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<String> {
|
||||
let value = resolve_attribution_value(config_attribution)?;
|
||||
Some(format!("Co-authored-by: {value}"))
|
||||
use crate::config::Config;
|
||||
|
||||
const DEFAULT_ATTRIBUTION_VALUE: &str = "Codex <noreply@openai.com>";
|
||||
const PREPARE_COMMIT_MSG_HOOK_NAME: &str = "prepare-commit-msg";
|
||||
const COMMIT_HOOK_NAMES: &[&str] = &[
|
||||
"applypatch-msg",
|
||||
"commit-msg",
|
||||
"post-commit",
|
||||
"pre-applypatch",
|
||||
"pre-commit",
|
||||
"pre-merge-commit",
|
||||
PREPARE_COMMIT_MSG_HOOK_NAME,
|
||||
];
|
||||
|
||||
pub(crate) fn configure_git_hooks_env_for_config(
|
||||
env: &mut HashMap<String, String>,
|
||||
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<String, String>,
|
||||
codex_home: &Path,
|
||||
config_attribution: Option<&str>,
|
||||
) -> Option<String> {
|
||||
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<String, String>) -> 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::<Vec<_>>();
|
||||
pairs.sort_unstable_by(|(left, _), (right, _)| left.cmp(right));
|
||||
pairs
|
||||
}
|
||||
|
||||
fn resolve_attribution_value(config_attribution: Option<&str>) -> Option<String> {
|
||||
@@ -28,6 +74,65 @@ fn resolve_attribution_value(config_attribution: Option<&str>) -> Option<String>
|
||||
}
|
||||
}
|
||||
|
||||
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)?;
|
||||
|
||||
for hook_name in COMMIT_HOOK_NAMES {
|
||||
let script = build_hook_script(hook_name, value);
|
||||
let hook_path = hooks_dir.join(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(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"
|
||||
)
|
||||
} 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}"
|
||||
)
|
||||
}
|
||||
|
||||
fn set_git_runtime_config(env: &mut HashMap<String, String>, key: &str, value: &str) {
|
||||
let mut index = env
|
||||
.get("GIT_CONFIG_COUNT")
|
||||
.and_then(|count| count.parse::<usize>().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;
|
||||
|
||||
@@ -1,19 +1,68 @@
|
||||
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 <noreply@openai.com>")
|
||||
env.get("GIT_CONFIG_KEY_0"),
|
||||
Some(&"core.hooksPath".to_string())
|
||||
);
|
||||
assert!(script.contains("Co-authored-by=Codex <noreply@openai.com>"));
|
||||
assert!(script.contains("existing_hook=\"$existing_hooks_path/prepare-commit-msg\""));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn generated_hooks_preserve_other_commit_hooks() {
|
||||
let tmp = tempdir().expect("create temp dir");
|
||||
let mut env = HashMap::new();
|
||||
|
||||
configure_git_hooks_env(&mut env, tmp.path(), None);
|
||||
|
||||
let hooks_dir = tmp.path().join("hooks").join("commit-attribution");
|
||||
for hook_name in [
|
||||
"applypatch-msg",
|
||||
"commit-msg",
|
||||
"post-commit",
|
||||
"pre-applypatch",
|
||||
"pre-commit",
|
||||
"pre-merge-commit",
|
||||
"prepare-commit-msg",
|
||||
] {
|
||||
let script = fs::read_to_string(hooks_dir.join(hook_name)).expect("read generated hook");
|
||||
assert!(script.contains(&format!(
|
||||
"existing_hook=\"$existing_hooks_path/{hook_name}\""
|
||||
)));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -34,10 +83,71 @@ 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 <agent@example.com>"))
|
||||
.expect("instruction expected");
|
||||
assert!(instruction.contains("Co-authored-by: AgentX <agent@example.com>"));
|
||||
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 <agent@example.com>"));
|
||||
|
||||
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 <agent@example.com>"));
|
||||
assert!(script.contains("existing_hook=\"$existing_hooks_path/prepare-commit-msg\""));
|
||||
assert!(script.contains("\"$existing_hook\" \"$@\""));
|
||||
let pre_commit = fs::read_to_string(
|
||||
tmp.path()
|
||||
.join("hooks")
|
||||
.join("commit-attribution")
|
||||
.join("pre-commit"),
|
||||
)
|
||||
.expect("read generated pre-commit hook");
|
||||
assert!(!pre_commit.contains("Co-authored-by=AgentX <agent@example.com>"));
|
||||
}
|
||||
|
||||
#[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 <agent@example.com>".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()),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
@@ -158,6 +158,18 @@ consolidation_model = "gpt-5"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn config_toml_ignores_malformed_commit_attribution() {
|
||||
let cfg = toml::from_str::<ConfigToml>(
|
||||
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(
|
||||
|
||||
@@ -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<String>,
|
||||
|
||||
/// When set, restricts ChatGPT login to a specific workspace identifier.
|
||||
@@ -1411,6 +1413,29 @@ enum WebSearchToolConfigInput {
|
||||
Config(WebSearchToolConfig),
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
#[serde(untagged)]
|
||||
enum CommitAttributionInput {
|
||||
String(String),
|
||||
// Ignore malformed values so a bad local override does not prevent Codex from starting.
|
||||
Ignored(IgnoredAny),
|
||||
}
|
||||
|
||||
fn deserialize_optional_commit_attribution<'de, D>(
|
||||
deserializer: D,
|
||||
) -> Result<Option<String>, D::Error>
|
||||
where
|
||||
D: Deserializer<'de>,
|
||||
{
|
||||
let value = Option::<CommitAttributionInput>::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<Option<WebSearchToolConfig>, D::Error>
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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<DeferredNetworkApproval>), 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,
|
||||
|
||||
340
codex-rs/core/tests/suite/commit_attribution.rs
Normal file
340
codex-rs/core/tests/suite/commit_attribution.rs
Normal file
@@ -0,0 +1,340 @@
|
||||
use std::path::Path;
|
||||
use std::process::Command;
|
||||
use std::time::Duration;
|
||||
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use anyhow::ensure;
|
||||
use core_test_support::assert_regex_match;
|
||||
use core_test_support::responses::ResponseMock;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_function_call;
|
||||
use core_test_support::responses::ev_response_created;
|
||||
use core_test_support::responses::mount_sse_sequence;
|
||||
use core_test_support::responses::sse;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::skip_if_windows;
|
||||
use core_test_support::test_codex_exec::TestCodexExecBuilder;
|
||||
use core_test_support::test_codex_exec::test_codex_exec;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
|
||||
const CALL_ID: &str = "commit-attribution-shell-command";
|
||||
const COMMIT_MESSAGE: &str = "commit attribution test";
|
||||
|
||||
fn git_test_env() -> [(&'static str, &'static str); 2] {
|
||||
[
|
||||
("GIT_CONFIG_GLOBAL", "/dev/null"),
|
||||
("GIT_CONFIG_NOSYSTEM", "1"),
|
||||
]
|
||||
}
|
||||
|
||||
fn run_git(repo: &Path, args: &[&str]) -> Result<String> {
|
||||
let output = Command::new("git")
|
||||
.envs(git_test_env())
|
||||
.args(args)
|
||||
.current_dir(repo)
|
||||
.output()
|
||||
.with_context(|| format!("run git {}", args.join(" ")))?;
|
||||
ensure!(
|
||||
output.status.success(),
|
||||
"git {} failed: {}",
|
||||
args.join(" "),
|
||||
String::from_utf8_lossy(&output.stderr).trim()
|
||||
);
|
||||
Ok(String::from_utf8_lossy(&output.stdout).into_owned())
|
||||
}
|
||||
|
||||
fn init_repo(repo: &Path) -> Result<()> {
|
||||
run_git(repo, &["init"])?;
|
||||
run_git(repo, &["config", "user.name", "Commit Attribution Test"])?;
|
||||
run_git(
|
||||
repo,
|
||||
&["config", "user.email", "commit-attribution@example.com"],
|
||||
)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn write_config(home: &Path, commit_attribution_line: Option<&str>) -> Result<()> {
|
||||
let mut config = String::new();
|
||||
if let Some(line) = commit_attribution_line {
|
||||
config.push_str(line);
|
||||
config.push('\n');
|
||||
config.push('\n');
|
||||
}
|
||||
config.push_str("[features]\ncodex_git_commit = true\n");
|
||||
std::fs::write(home.join("config.toml"), config).context("write config.toml")
|
||||
}
|
||||
|
||||
async fn mount_commit_turn(server: &wiremock::MockServer, command: &str) -> Result<ResponseMock> {
|
||||
let arguments = serde_json::to_string(&json!({
|
||||
"command": command,
|
||||
"login": false,
|
||||
"timeout_ms": 10_000,
|
||||
}))?;
|
||||
Ok(mount_sse_sequence(
|
||||
server,
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_function_call(CALL_ID, "shell_command", &arguments),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_assistant_message("msg-1", "done"),
|
||||
ev_completed("resp-2"),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await)
|
||||
}
|
||||
|
||||
fn run_exec(
|
||||
builder: &TestCodexExecBuilder,
|
||||
server: &wiremock::MockServer,
|
||||
) -> Result<std::process::Output> {
|
||||
let mut cmd = builder.cmd_with_server(server);
|
||||
cmd.timeout(Duration::from_secs(30));
|
||||
cmd.envs(git_test_env())
|
||||
.arg("--dangerously-bypass-approvals-and-sandbox")
|
||||
.arg("--skip-git-repo-check")
|
||||
.arg("make the requested commit");
|
||||
cmd.output().context("run codex-exec")
|
||||
}
|
||||
|
||||
fn latest_commit_message(repo: &Path) -> Result<String> {
|
||||
run_git(repo, &["log", "-1", "--format=%B"])
|
||||
}
|
||||
|
||||
fn assert_shell_command_succeeded(mock: &ResponseMock) {
|
||||
let Some(output) = mock.function_call_output_text(CALL_ID) else {
|
||||
panic!("shell_command output should be recorded");
|
||||
};
|
||||
assert_regex_match(
|
||||
r"(?s)^Exit code: 0\nWall time: [0-9]+(?:\.[0-9]+)? seconds\nOutput:\n.*$",
|
||||
&output.replace("\r\n", "\n"),
|
||||
);
|
||||
}
|
||||
|
||||
fn codex_hooks_dir(builder: &TestCodexExecBuilder) -> std::path::PathBuf {
|
||||
builder.home_path().join("hooks").join("commit-attribution")
|
||||
}
|
||||
|
||||
fn assert_trailer_once(message: &str, expected: &str) {
|
||||
assert!(
|
||||
message.trim_end().ends_with(expected),
|
||||
"expected commit message to end with {expected:?}, got: {message:?}"
|
||||
);
|
||||
assert_eq!(message.matches(expected).count(), 1);
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn exec_cli_commit_attribution_defaults_when_unset() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let builder = test_codex_exec();
|
||||
init_repo(builder.cwd_path())?;
|
||||
write_config(builder.home_path(), None)?;
|
||||
let mock = mount_commit_turn(
|
||||
&server,
|
||||
&format!("git commit --allow-empty -m '{COMMIT_MESSAGE}'"),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let output = run_exec(&builder, &server)?;
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"codex-exec failed: {}",
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
assert_shell_command_succeeded(&mock);
|
||||
|
||||
let message = latest_commit_message(builder.cwd_path())?;
|
||||
assert_trailer_once(&message, "Co-authored-by: Codex <noreply@openai.com>");
|
||||
assert!(
|
||||
codex_hooks_dir(&builder)
|
||||
.join("prepare-commit-msg")
|
||||
.exists()
|
||||
);
|
||||
assert!(codex_hooks_dir(&builder).join("commit-msg").exists());
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn exec_cli_commit_attribution_uses_configured_value() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let builder = test_codex_exec();
|
||||
init_repo(builder.cwd_path())?;
|
||||
write_config(
|
||||
builder.home_path(),
|
||||
Some(r#"commit_attribution = "AgentX <agent@example.com>""#),
|
||||
)?;
|
||||
let mock = mount_commit_turn(
|
||||
&server,
|
||||
&format!("git commit --allow-empty -m '{COMMIT_MESSAGE}'"),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let output = run_exec(&builder, &server)?;
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"codex-exec failed: {}",
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
assert_shell_command_succeeded(&mock);
|
||||
|
||||
let message = latest_commit_message(builder.cwd_path())?;
|
||||
assert_trailer_once(&message, "Co-authored-by: AgentX <agent@example.com>");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn exec_cli_commit_attribution_can_be_disabled() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let builder = test_codex_exec();
|
||||
init_repo(builder.cwd_path())?;
|
||||
write_config(builder.home_path(), Some(r#"commit_attribution = """#))?;
|
||||
let mock = mount_commit_turn(
|
||||
&server,
|
||||
&format!("git commit --allow-empty -m '{COMMIT_MESSAGE}'"),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let output = run_exec(&builder, &server)?;
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"codex-exec failed: {}",
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
assert_shell_command_succeeded(&mock);
|
||||
|
||||
let message = latest_commit_message(builder.cwd_path())?;
|
||||
assert!(!message.contains("Co-authored-by:"));
|
||||
assert!(!codex_hooks_dir(&builder).exists());
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn exec_cli_commit_attribution_ignores_malformed_config() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let builder = test_codex_exec();
|
||||
init_repo(builder.cwd_path())?;
|
||||
write_config(builder.home_path(), Some("commit_attribution = true"))?;
|
||||
let mock = mount_commit_turn(
|
||||
&server,
|
||||
&format!("git commit --allow-empty -m '{COMMIT_MESSAGE}'"),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let output = run_exec(&builder, &server)?;
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"codex-exec failed: {}",
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
assert_shell_command_succeeded(&mock);
|
||||
|
||||
let message = latest_commit_message(builder.cwd_path())?;
|
||||
assert_trailer_once(&message, "Co-authored-by: Codex <noreply@openai.com>");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn exec_cli_commit_attribution_preserves_user_commit_hooks() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_windows!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let builder = test_codex_exec();
|
||||
init_repo(builder.cwd_path())?;
|
||||
write_config(builder.home_path(), None)?;
|
||||
|
||||
let user_hooks = builder.cwd_path().join("user-hooks");
|
||||
std::fs::create_dir_all(&user_hooks).context("create user hooks dir")?;
|
||||
std::fs::write(
|
||||
user_hooks.join("pre-commit"),
|
||||
"#!/usr/bin/env bash\nset -euo pipefail\nroot=\"$(git rev-parse --show-toplevel)\"\nprintf pre-commit > \"$root/pre-commit.marker\"\n",
|
||||
)?;
|
||||
std::fs::write(
|
||||
user_hooks.join("post-commit"),
|
||||
"#!/usr/bin/env bash\nset -euo pipefail\nroot=\"$(git rev-parse --show-toplevel)\"\nprintf post-commit > \"$root/post-commit.marker\"\n",
|
||||
)?;
|
||||
std::fs::write(
|
||||
user_hooks.join("prepare-commit-msg"),
|
||||
"#!/usr/bin/env bash\nset -euo pipefail\nroot=\"$(git rev-parse --show-toplevel)\"\nprintf prepare-commit-msg > \"$root/prepare-commit-msg.marker\"\nprintf '\\nuser-prepare-hook\\n' >> \"$1\"\n",
|
||||
)?;
|
||||
std::fs::write(
|
||||
user_hooks.join("commit-msg"),
|
||||
"#!/usr/bin/env bash\nset -euo pipefail\nroot=\"$(git rev-parse --show-toplevel)\"\nprintf commit-msg > \"$root/commit-msg.marker\"\n",
|
||||
)?;
|
||||
for hook_name in [
|
||||
"pre-commit",
|
||||
"post-commit",
|
||||
"prepare-commit-msg",
|
||||
"commit-msg",
|
||||
] {
|
||||
let mut perms = std::fs::metadata(user_hooks.join(hook_name))?.permissions();
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
perms.set_mode(0o755);
|
||||
}
|
||||
std::fs::set_permissions(user_hooks.join(hook_name), perms)?;
|
||||
}
|
||||
run_git(
|
||||
builder.cwd_path(),
|
||||
&[
|
||||
"config",
|
||||
"core.hooksPath",
|
||||
user_hooks.to_string_lossy().as_ref(),
|
||||
],
|
||||
)?;
|
||||
|
||||
let mock = mount_commit_turn(
|
||||
&server,
|
||||
&format!("git commit --allow-empty -m '{COMMIT_MESSAGE}'"),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let output = run_exec(&builder, &server)?;
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"codex-exec failed: {}",
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
assert_shell_command_succeeded(&mock);
|
||||
|
||||
let message = latest_commit_message(builder.cwd_path())?;
|
||||
assert!(message.contains("user-prepare-hook"));
|
||||
assert_trailer_once(&message, "Co-authored-by: Codex <noreply@openai.com>");
|
||||
for marker in [
|
||||
"pre-commit.marker",
|
||||
"post-commit.marker",
|
||||
"prepare-commit-msg.marker",
|
||||
"commit-msg.marker",
|
||||
] {
|
||||
assert!(
|
||||
builder.cwd_path().join(marker).exists(),
|
||||
"expected {marker} to be created by the user's hook"
|
||||
);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -68,6 +68,8 @@ mod client_websockets;
|
||||
mod code_mode;
|
||||
mod codex_delegate;
|
||||
mod collaboration_instructions;
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
mod commit_attribution;
|
||||
mod compact;
|
||||
mod compact_remote;
|
||||
mod compact_resume_fork;
|
||||
|
||||
Reference in New Issue
Block a user