mirror of
https://github.com/openai/codex.git
synced 2026-05-26 22:15:13 +00:00
extension: move git attribution into an extension (#21738)
## Why Git commit attribution is prompt policy, not session orchestration. After #21737 adds the extension-registry seam, this moves that prompt-only behavior out of `codex-core` so `Session` can consume extension-contributed prompt fragments instead of owning a one-off policy path itself. Before this PR, `Session` injected the trailer instruction directly from `codex-core` ([session assembly](a57a747eb6/codex-rs/core/src/session/mod.rs (L2733-L2739)), [helper module](a57a747eb6/codex-rs/core/src/commit_attribution.rs (L1-L33))). This branch moves that same responsibility into [`codex-git-attribution`](b5029a6736/codex-rs/ext/git-attribution/src/lib.rs (L14-L100)). ## What changed - Added the `codex-git-attribution` extension crate. - Snapshot `CodexGitCommit` plus `commit_attribution` at thread start, then contribute the developer-policy fragment through the extension registry. - Register the extension in app-server thread extensions. - Remove the old `codex-core` helper module and direct `Session` injection path. This keeps the existing behavior intact: the prompt is only contributed when `CodexGitCommit` is enabled, blank attribution still disables the trailer, and the default remains `Codex <noreply@openai.com>`. ## Stack - Stacked on #21737.
This commit is contained in:
11
codex-rs/Cargo.lock
generated
11
codex-rs/Cargo.lock
generated
@@ -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"
|
||||
|
||||
@@ -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" }
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -5,5 +5,9 @@ use codex_extension_api::ExtensionRegistry;
|
||||
use codex_extension_api::ExtensionRegistryBuilder;
|
||||
|
||||
pub(crate) fn thread_extensions() -> Arc<ExtensionRegistry<Config>> {
|
||||
Arc::new(ExtensionRegistryBuilder::<Config>::new().build())
|
||||
Arc::new(
|
||||
ExtensionRegistryBuilder::<Config>::new()
|
||||
.with_extension(codex_git_attribution::extension())
|
||||
.build(),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -1,33 +0,0 @@
|
||||
const DEFAULT_ATTRIBUTION_VALUE: &str = "Codex <noreply@openai.com>";
|
||||
|
||||
fn build_commit_message_trailer(config_attribution: Option<&str>) -> Option<String> {
|
||||
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<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."
|
||||
))
|
||||
}
|
||||
|
||||
fn resolve_attribution_value(config_attribution: Option<&str>) -> Option<String> {
|
||||
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;
|
||||
@@ -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 <noreply@openai.com>")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_value_handles_default_custom_and_blank() {
|
||||
assert_eq!(
|
||||
resolve_attribution_value(/*config_attribution*/ None),
|
||||
Some("Codex <noreply@openai.com>".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
resolve_attribution_value(Some("MyAgent <me@example.com>")),
|
||||
Some("MyAgent <me@example.com>".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 <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"));
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
@@ -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,
|
||||
|
||||
6
codex-rs/ext/git-attribution/BUILD.bazel
Normal file
6
codex-rs/ext/git-attribution/BUILD.bazel
Normal file
@@ -0,0 +1,6 @@
|
||||
load("//:defs.bzl", "codex_rust_crate")
|
||||
|
||||
codex_rust_crate(
|
||||
name = "git-attribution",
|
||||
crate_name = "codex_git_attribution",
|
||||
)
|
||||
21
codex-rs/ext/git-attribution/Cargo.toml
Normal file
21
codex-rs/ext/git-attribution/Cargo.toml
Normal file
@@ -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 }
|
||||
149
codex-rs/ext/git-attribution/src/lib.rs
Normal file
149
codex-rs/ext/git-attribution/src/lib.rs
Normal file
@@ -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 <noreply@openai.com>";
|
||||
|
||||
/// 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<PromptFragment> {
|
||||
let Some(config_store) = thread_store.get::<GitAttributionConfig>() 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<String>,
|
||||
}
|
||||
|
||||
impl ThreadStartContributor<Config> 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<Config> for GitAttributionExtension {
|
||||
fn install(self: Arc<Self>, registry: &mut ExtensionRegistryBuilder<Config>) {
|
||||
registry.thread_start_contributor(self.clone());
|
||||
registry.prompt_contributor(self);
|
||||
}
|
||||
}
|
||||
|
||||
/// Creates a shared git-attribution extension instance.
|
||||
pub fn extension() -> Arc<GitAttributionExtension> {
|
||||
Arc::new(GitAttributionExtension::new())
|
||||
}
|
||||
|
||||
fn build_commit_message_trailer(config_attribution: Option<&str>) -> Option<String> {
|
||||
let value = resolve_attribution_value(config_attribution)?;
|
||||
Some(format!("Co-authored-by: {value}"))
|
||||
}
|
||||
|
||||
fn build_instruction(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."
|
||||
))
|
||||
}
|
||||
|
||||
fn resolve_attribution_value(config_attribution: Option<&str>) -> Option<String> {
|
||||
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 <noreply@openai.com>")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_value_handles_default_custom_and_blank() {
|
||||
assert_eq!(
|
||||
resolve_attribution_value(/*config_attribution*/ None),
|
||||
Some("Codex <noreply@openai.com>".to_string())
|
||||
);
|
||||
assert_eq!(
|
||||
resolve_attribution_value(Some("MyAgent <me@example.com>")),
|
||||
Some("MyAgent <me@example.com>".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 <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"));
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user