diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index bf1d7be1c8..f600b38d6d 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1902,6 +1902,7 @@ dependencies = [ "codex-feedback", "codex-file-search", "codex-file-watcher", + "codex-git-attribution", "codex-git-utils", "codex-hooks", "codex-login", @@ -2918,6 +2919,16 @@ dependencies = [ "tracing", ] +[[package]] +name = "codex-git-attribution" +version = "0.0.0" +dependencies = [ + "codex-core", + "codex-extension-api", + "codex-features", + "pretty_assertions", +] + [[package]] name = "codex-git-utils" version = "0.0.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 41ba8c69ba..ae7ef5dc3f 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -46,6 +46,7 @@ members = [ "execpolicy", "execpolicy-legacy", "ext/extension-api", + "ext/git-attribution", "external-agent-migration", "external-agent-sessions", "keyring-store", @@ -162,6 +163,7 @@ codex-file-system = { path = "file-system" } codex-exec-server = { path = "exec-server" } codex-execpolicy = { path = "execpolicy" } codex-extension-api = { path = "ext/extension-api" } +codex-git-attribution = { path = "ext/git-attribution" } codex-external-agent-migration = { path = "external-agent-migration" } codex-external-agent-sessions = { path = "external-agent-sessions" } codex-experimental-api-macros = { path = "codex-experimental-api-macros" } diff --git a/codex-rs/app-server/Cargo.toml b/codex-rs/app-server/Cargo.toml index bb26b5bd7a..7a38a75424 100644 --- a/codex-rs/app-server/Cargo.toml +++ b/codex-rs/app-server/Cargo.toml @@ -41,6 +41,7 @@ codex-extension-api = { workspace = true } codex-external-agent-migration = { workspace = true } codex-external-agent-sessions = { workspace = true } codex-features = { workspace = true } +codex-git-attribution = { workspace = true } codex-git-utils = { workspace = true } codex-file-watcher = { workspace = true } codex-hooks = { workspace = true } diff --git a/codex-rs/app-server/src/extensions.rs b/codex-rs/app-server/src/extensions.rs index 8fa5e3f693..9449f8dcad 100644 --- a/codex-rs/app-server/src/extensions.rs +++ b/codex-rs/app-server/src/extensions.rs @@ -5,5 +5,9 @@ use codex_extension_api::ExtensionRegistry; use codex_extension_api::ExtensionRegistryBuilder; pub(crate) fn thread_extensions() -> Arc> { - Arc::new(ExtensionRegistryBuilder::::new().build()) + Arc::new( + ExtensionRegistryBuilder::::new() + .with_extension(codex_git_attribution::extension()) + .build(), + ) } diff --git a/codex-rs/core/src/commit_attribution.rs b/codex-rs/core/src/commit_attribution.rs deleted file mode 100644 index 8df9661431..0000000000 --- a/codex-rs/core/src/commit_attribution.rs +++ /dev/null @@ -1,33 +0,0 @@ -const DEFAULT_ATTRIBUTION_VALUE: &str = "Codex "; - -fn build_commit_message_trailer(config_attribution: Option<&str>) -> Option { - let value = resolve_attribution_value(config_attribution)?; - Some(format!("Co-authored-by: {value}")) -} - -pub(crate) fn commit_message_trailer_instruction( - 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." - )) -} - -fn resolve_attribution_value(config_attribution: Option<&str>) -> Option { - match config_attribution { - Some(value) => { - let trimmed = value.trim(); - if trimmed.is_empty() { - None - } else { - Some(trimmed.to_string()) - } - } - None => Some(DEFAULT_ATTRIBUTION_VALUE.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 deleted file mode 100644 index 8b2ce27e1f..0000000000 --- a/codex-rs/core/src/commit_attribution_tests.rs +++ /dev/null @@ -1,43 +0,0 @@ -use super::build_commit_message_trailer; -use super::commit_message_trailer_instruction; -use super::resolve_attribution_value; - -#[test] -fn blank_attribution_disables_trailer_prompt() { - assert_eq!(build_commit_message_trailer(Some("")), None); - assert_eq!(commit_message_trailer_instruction(Some(" ")), None); -} - -#[test] -fn default_attribution_uses_codex_trailer() { - assert_eq!( - build_commit_message_trailer(/*config_attribution*/ None).as_deref(), - Some("Co-authored-by: Codex ") - ); -} - -#[test] -fn resolve_value_handles_default_custom_and_blank() { - assert_eq!( - resolve_attribution_value(/*config_attribution*/ None), - Some("Codex ".to_string()) - ); - assert_eq!( - resolve_attribution_value(Some("MyAgent ")), - Some("MyAgent ".to_string()) - ); - assert_eq!( - resolve_attribution_value(Some("MyAgent")), - Some("MyAgent".to_string()) - ); - assert_eq!(resolve_attribution_value(Some(" ")), None); -} - -#[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")); -} diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 93acda7854..3ec34694cb 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -27,7 +27,6 @@ mod agent; mod attestation; mod codex_delegate; mod command_canonicalization; -mod commit_attribution; pub mod config; pub mod connectors; pub mod context; diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 6d28b9959c..af36fda487 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -16,7 +16,6 @@ use crate::agent::agent_status_from_event; use crate::agent::status::is_final; use crate::attestation::AttestationProvider; use crate::build_available_skills; -use crate::commit_attribution::commit_message_trailer_instruction; use crate::compact; use crate::config::ManagedFeatures; use crate::config::resolve_tool_suggest_config_from_layer_stack; @@ -2712,13 +2711,6 @@ impl Session { { developer_sections.push(plugin_instructions.render()); } - 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); - } for contributor in self.services.extensions.context_contributors() { for fragment in contributor.contribute( &self.services.session_extension_data, diff --git a/codex-rs/ext/git-attribution/BUILD.bazel b/codex-rs/ext/git-attribution/BUILD.bazel new file mode 100644 index 0000000000..0cb1ab5764 --- /dev/null +++ b/codex-rs/ext/git-attribution/BUILD.bazel @@ -0,0 +1,6 @@ +load("//:defs.bzl", "codex_rust_crate") + +codex_rust_crate( + name = "git-attribution", + crate_name = "codex_git_attribution", +) diff --git a/codex-rs/ext/git-attribution/Cargo.toml b/codex-rs/ext/git-attribution/Cargo.toml new file mode 100644 index 0000000000..edf8c07196 --- /dev/null +++ b/codex-rs/ext/git-attribution/Cargo.toml @@ -0,0 +1,21 @@ +[package] +edition.workspace = true +license.workspace = true +name = "codex-git-attribution" +version.workspace = true + +[lib] +name = "codex_git_attribution" +path = "src/lib.rs" +doctest = false + +[lints] +workspace = true + +[dependencies] +codex-core = { workspace = true } +codex-extension-api = { workspace = true } +codex-features = { workspace = true } + +[dev-dependencies] +pretty_assertions = { workspace = true } diff --git a/codex-rs/ext/git-attribution/src/lib.rs b/codex-rs/ext/git-attribution/src/lib.rs new file mode 100644 index 0000000000..d4b711d555 --- /dev/null +++ b/codex-rs/ext/git-attribution/src/lib.rs @@ -0,0 +1,149 @@ +use std::sync::Arc; + +use codex_core::config::Config; +use codex_extension_api::CodexExtension; +use codex_extension_api::ContextContributor; +use codex_extension_api::ExtensionData; +use codex_extension_api::ExtensionRegistryBuilder; +use codex_extension_api::PromptFragment; +use codex_extension_api::ThreadStartContributor; +use codex_features::Feature; + +const DEFAULT_ATTRIBUTION_VALUE: &str = "Codex "; + +/// Prompt-only extension that contributes the configured git-attribution instruction. +#[derive(Clone, Copy, Debug, Default)] +pub struct GitAttributionExtension; + +impl GitAttributionExtension { + /// Creates an extension instance. + pub fn new() -> Self { + Self + } +} + +impl ContextContributor for GitAttributionExtension { + fn contribute( + &self, + _session_store: &ExtensionData, + thread_store: &ExtensionData, + ) -> Vec { + let Some(config_store) = thread_store.get::() else { + return Vec::new(); + }; + if !config_store.enabled { + return Vec::new(); + } + build_instruction(config_store.prompt.as_deref()) + .map(PromptFragment::developer_policy) + .into_iter() + .collect() + } +} + +#[derive(Clone, Debug, Default)] +struct GitAttributionConfig { + enabled: bool, + prompt: Option, +} + +impl ThreadStartContributor for GitAttributionExtension { + fn contribute( + &self, + config: &Config, + _session_store: &ExtensionData, + thread_store: &ExtensionData, + ) { + thread_store.insert(GitAttributionConfig { + enabled: config.features.enabled(Feature::CodexGitCommit), + prompt: config.commit_attribution.clone(), + }); + } +} + +impl CodexExtension for GitAttributionExtension { + fn install(self: Arc, registry: &mut ExtensionRegistryBuilder) { + registry.thread_start_contributor(self.clone()); + registry.prompt_contributor(self); + } +} + +/// Creates a shared git-attribution extension instance. +pub fn extension() -> Arc { + Arc::new(GitAttributionExtension::new()) +} + +fn build_commit_message_trailer(config_attribution: Option<&str>) -> Option { + let value = resolve_attribution_value(config_attribution)?; + Some(format!("Co-authored-by: {value}")) +} + +fn build_instruction(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." + )) +} + +fn resolve_attribution_value(config_attribution: Option<&str>) -> Option { + match config_attribution { + Some(value) => { + let trimmed = value.trim(); + if trimmed.is_empty() { + None + } else { + Some(trimmed.to_string()) + } + } + None => Some(DEFAULT_ATTRIBUTION_VALUE.to_string()), + } +} + +#[cfg(test)] +mod tests { + use pretty_assertions::assert_eq; + + use super::build_commit_message_trailer; + use super::build_instruction; + use super::resolve_attribution_value; + + #[test] + fn blank_attribution_disables_trailer_prompt() { + assert_eq!(build_commit_message_trailer(Some("")), None); + assert_eq!(build_instruction(Some(" ")), None); + } + + #[test] + fn default_attribution_uses_codex_trailer() { + assert_eq!( + build_commit_message_trailer(/*config_attribution*/ None).as_deref(), + Some("Co-authored-by: Codex ") + ); + } + + #[test] + fn resolve_value_handles_default_custom_and_blank() { + assert_eq!( + resolve_attribution_value(/*config_attribution*/ None), + Some("Codex ".to_string()) + ); + assert_eq!( + resolve_attribution_value(Some("MyAgent ")), + Some("MyAgent ".to_string()) + ); + assert_eq!( + resolve_attribution_value(Some("MyAgent")), + Some("MyAgent".to_string()) + ); + assert_eq!(resolve_attribution_value(Some(" ")), None); + } + + #[test] + fn instruction_mentions_trailer_and_omits_generated_with() { + let instruction = + build_instruction(Some("AgentX ")).expect("instruction expected"); + assert!(instruction.contains("Co-authored-by: AgentX ")); + assert!(instruction.contains("exactly once")); + assert!(!instruction.contains("Generated-with")); + } +}