mirror of
https://github.com/openai/codex.git
synced 2026-11-10 21:00:47 +00:00
fix git attribution
This commit is contained in:
@@ -37,9 +37,11 @@ codex-core = { workspace = true }
|
||||
codex-core-plugins = { workspace = true }
|
||||
codex-device-key = { workspace = true }
|
||||
codex-exec-server = { workspace = true }
|
||||
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-hooks = { workspace = true }
|
||||
codex-otel = { workspace = true }
|
||||
|
||||
@@ -0,0 +1,13 @@
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_core::ExtensionContext;
|
||||
use codex_extension_api::ExtensionRegistry;
|
||||
use codex_extension_api::ExtensionRegistryBuilder;
|
||||
|
||||
pub(crate) fn thread_extensions() -> Arc<ExtensionRegistry<ExtensionContext>> {
|
||||
Arc::new(
|
||||
ExtensionRegistryBuilder::<ExtensionContext>::new()
|
||||
.with_extension(codex_git_attribution::extension())
|
||||
.build(),
|
||||
)
|
||||
}
|
||||
@@ -82,6 +82,7 @@ mod config_manager_service;
|
||||
mod connection_rpc_gate;
|
||||
mod dynamic_tools;
|
||||
mod error_code;
|
||||
mod extensions;
|
||||
mod filters;
|
||||
mod fs_watch;
|
||||
mod fuzzy_file_search;
|
||||
|
||||
@@ -120,6 +120,7 @@ mod tests {
|
||||
use std::sync::atomic::AtomicUsize;
|
||||
use std::sync::atomic::Ordering;
|
||||
use tempfile::TempDir;
|
||||
use crate::extensions::thread_extensions;
|
||||
|
||||
#[tokio::test]
|
||||
async fn strict_refresh_reports_thread_planning_failures() -> anyhow::Result<()> {
|
||||
@@ -183,6 +184,7 @@ mod tests {
|
||||
auth_manager,
|
||||
SessionSource::Exec,
|
||||
Arc::new(EnvironmentManager::default_for_tests()),
|
||||
thread_extensions(),
|
||||
/*analytics_events_client*/ None,
|
||||
thread_store,
|
||||
Some(state_db.clone()),
|
||||
|
||||
@@ -7,6 +7,7 @@ use std::sync::atomic::AtomicBool;
|
||||
use crate::config_manager::ConfigManager;
|
||||
use crate::connection_rpc_gate::ConnectionRpcGate;
|
||||
use crate::error_code::invalid_request;
|
||||
use crate::extensions::thread_extensions;
|
||||
use crate::fs_watch::FsWatchManager;
|
||||
use crate::outgoing_message::ConnectionId;
|
||||
use crate::outgoing_message::ConnectionRequestId;
|
||||
@@ -298,6 +299,7 @@ impl MessageProcessor {
|
||||
auth_manager.clone(),
|
||||
session_source,
|
||||
environment_manager,
|
||||
thread_extensions(),
|
||||
Some(analytics_events_client.clone()),
|
||||
Arc::clone(&thread_store),
|
||||
state_db.clone(),
|
||||
|
||||
@@ -364,6 +364,7 @@ impl Session {
|
||||
skills_manager: Arc<SkillsManager>,
|
||||
plugins_manager: Arc<PluginsManager>,
|
||||
mcp_manager: Arc<McpManager>,
|
||||
extensions: Arc<codex_extension_api::ExtensionRegistry<crate::ExtensionContext>>,
|
||||
skills_watcher: Arc<SkillsWatcher>,
|
||||
agent_control: AgentControl,
|
||||
environment_manager: Arc<EnvironmentManager>,
|
||||
@@ -845,6 +846,9 @@ impl Session {
|
||||
skills_manager,
|
||||
plugins_manager: Arc::clone(&plugins_manager),
|
||||
mcp_manager: Arc::clone(&mcp_manager),
|
||||
extensions,
|
||||
session_extension_data: codex_extension_api::ExtensionData::new(),
|
||||
thread_extension_data: codex_extension_api::ExtensionData::new(),
|
||||
skills_watcher,
|
||||
agent_control,
|
||||
network_proxy,
|
||||
|
||||
@@ -3596,6 +3596,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() {
|
||||
skills_manager,
|
||||
plugins_manager,
|
||||
mcp_manager,
|
||||
Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()),
|
||||
Arc::new(SkillsWatcher::noop()),
|
||||
AgentControl::default(),
|
||||
Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()),
|
||||
@@ -3743,6 +3744,9 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
skills_manager,
|
||||
plugins_manager,
|
||||
mcp_manager,
|
||||
extensions: Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()),
|
||||
session_extension_data: codex_extension_api::ExtensionData::new(),
|
||||
thread_extension_data: codex_extension_api::ExtensionData::new(),
|
||||
skills_watcher,
|
||||
agent_control,
|
||||
network_proxy: None,
|
||||
@@ -3922,6 +3926,7 @@ async fn make_session_with_config_and_rx(
|
||||
skills_manager,
|
||||
plugins_manager,
|
||||
mcp_manager,
|
||||
Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()),
|
||||
Arc::new(SkillsWatcher::noop()),
|
||||
AgentControl::default(),
|
||||
Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()),
|
||||
@@ -4024,6 +4029,7 @@ async fn make_session_with_history_source_and_agent_control_and_rx(
|
||||
skills_manager,
|
||||
plugins_manager,
|
||||
mcp_manager,
|
||||
Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()),
|
||||
Arc::new(SkillsWatcher::noop()),
|
||||
agent_control,
|
||||
Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()),
|
||||
@@ -5448,6 +5454,9 @@ where
|
||||
skills_manager,
|
||||
plugins_manager,
|
||||
mcp_manager,
|
||||
extensions: Arc::new(codex_extension_api::ExtensionRegistryBuilder::new().build()),
|
||||
session_extension_data: codex_extension_api::ExtensionData::new(),
|
||||
thread_extension_data: codex_extension_api::ExtensionData::new(),
|
||||
skills_watcher,
|
||||
agent_control,
|
||||
network_proxy: None,
|
||||
@@ -5994,6 +6003,52 @@ async fn make_multi_agent_v2_usage_hint_test_session(
|
||||
(session, turn_context)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn build_initial_context_includes_git_attribution_from_extensions() {
|
||||
let (mut session, mut turn_context) = make_session_and_context().await;
|
||||
session.services.extensions = Arc::new(
|
||||
codex_extension_api::ExtensionRegistryBuilder::<crate::ExtensionContext>::new()
|
||||
.with_extension(codex_git_attribution::extension())
|
||||
.build(),
|
||||
);
|
||||
turn_context
|
||||
.features
|
||||
.enable(Feature::CodexGitCommit)
|
||||
.expect("test features should allow git attribution");
|
||||
|
||||
let initial_context = session.build_initial_context(&turn_context).await;
|
||||
let developer_messages = developer_message_texts(&initial_context);
|
||||
|
||||
assert!(
|
||||
developer_messages
|
||||
.iter()
|
||||
.flatten()
|
||||
.any(|text| text.contains("Co-authored-by: Codex <noreply@openai.com>")),
|
||||
"expected git attribution developer text, got {developer_messages:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn build_initial_context_omits_git_attribution_when_feature_is_disabled() {
|
||||
let (mut session, turn_context) = make_session_and_context().await;
|
||||
session.services.extensions = Arc::new(
|
||||
codex_extension_api::ExtensionRegistryBuilder::<crate::ExtensionContext>::new()
|
||||
.with_extension(codex_git_attribution::extension())
|
||||
.build(),
|
||||
);
|
||||
|
||||
let initial_context = session.build_initial_context(&turn_context).await;
|
||||
let developer_messages = developer_message_texts(&initial_context);
|
||||
|
||||
assert!(
|
||||
!developer_messages
|
||||
.iter()
|
||||
.flatten()
|
||||
.any(|text| text.contains("Co-authored-by:")),
|
||||
"did not expect git attribution developer text, got {developer_messages:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn build_initial_context_adds_multi_agent_v2_root_usage_hint_as_developer_message() {
|
||||
let (session, turn_context) =
|
||||
|
||||
@@ -743,6 +743,7 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() {
|
||||
skills_manager,
|
||||
plugins_manager,
|
||||
mcp_manager,
|
||||
extensions: crate::empty_extension_registry(),
|
||||
skills_watcher,
|
||||
conversation_history: InitialHistory::New,
|
||||
session_source: SessionSource::SubAgent(SubAgentSource::Other(
|
||||
|
||||
@@ -6,6 +6,7 @@ use crate::agent::AgentControl;
|
||||
use crate::client::ModelClient;
|
||||
use crate::config::StartedNetworkProxy;
|
||||
use crate::exec_policy::ExecPolicyManager;
|
||||
use crate::extensions::ExtensionContext;
|
||||
use crate::guardian::GuardianRejection;
|
||||
use crate::guardian::GuardianRejectionCircuitBreaker;
|
||||
use crate::mcp::McpManager;
|
||||
@@ -18,6 +19,8 @@ use arc_swap::ArcSwap;
|
||||
use codex_analytics::AnalyticsEventsClient;
|
||||
use codex_core_plugins::PluginsManager;
|
||||
use codex_exec_server::EnvironmentManager;
|
||||
use codex_extension_api::ExtensionData;
|
||||
use codex_extension_api::ExtensionRegistry;
|
||||
use codex_hooks::Hooks;
|
||||
use codex_login::AuthManager;
|
||||
use codex_mcp::McpConnectionManager;
|
||||
@@ -59,6 +62,9 @@ pub(crate) struct SessionServices {
|
||||
pub(crate) skills_manager: Arc<SkillsManager>,
|
||||
pub(crate) plugins_manager: Arc<PluginsManager>,
|
||||
pub(crate) mcp_manager: Arc<McpManager>,
|
||||
pub(crate) extensions: Arc<ExtensionRegistry<ExtensionContext>>,
|
||||
pub(crate) session_extension_data: ExtensionData,
|
||||
pub(crate) thread_extension_data: ExtensionData,
|
||||
pub(crate) skills_watcher: Arc<SkillsWatcher>,
|
||||
pub(crate) agent_control: AgentControl,
|
||||
pub(crate) network_proxy: Option<StartedNetworkProxy>,
|
||||
|
||||
@@ -2,6 +2,7 @@ use super::*;
|
||||
use crate::ThreadManager;
|
||||
use crate::config::AgentRoleConfig;
|
||||
use crate::config::DEFAULT_AGENT_MAX_DEPTH;
|
||||
use crate::empty_extension_registry;
|
||||
use crate::function_tool::FunctionCallError;
|
||||
use crate::init_state_db;
|
||||
use crate::session::tests::make_session_and_context;
|
||||
@@ -3163,6 +3164,7 @@ async fn tool_handlers_cascade_close_and_resume_and_keep_explicitly_closed_subtr
|
||||
AuthManager::from_auth_for_testing(CodexAuth::from_api_key("dummy")),
|
||||
SessionSource::Exec,
|
||||
Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()),
|
||||
empty_extension_registry(),
|
||||
/*analytics_events_client*/ None,
|
||||
thread_store_from_config(&config, state_db.clone()),
|
||||
state_db.clone(),
|
||||
|
||||
@@ -2,13 +2,14 @@ use std::sync::Arc;
|
||||
|
||||
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::Stores;
|
||||
|
||||
const DEFAULT_ATTRIBUTION_VALUE: &str = "Codex <noreply@openai.com>";
|
||||
|
||||
pub trait GitAttributionContext {
|
||||
fn git_attribution_enabled(&self) -> bool;
|
||||
fn commit_attribution(&self) -> Option<&str>;
|
||||
}
|
||||
|
||||
@@ -24,6 +25,9 @@ impl GitAttributionExtension {
|
||||
|
||||
/// Returns the model-visible trailer instruction, if attribution is enabled.
|
||||
pub fn instruction<C: GitAttributionContext>(&self, context: &C) -> Option<String> {
|
||||
if !context.git_attribution_enabled() {
|
||||
return None;
|
||||
}
|
||||
let trailer = build_commit_message_trailer(context.commit_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."
|
||||
@@ -32,9 +36,14 @@ impl GitAttributionExtension {
|
||||
}
|
||||
|
||||
impl<C: GitAttributionContext> ContextContributor<C> for GitAttributionExtension {
|
||||
fn contribute(&self, context: &C, _stores: &Stores<'_>) -> Vec<PromptFragment> {
|
||||
fn contribute(
|
||||
&self,
|
||||
context: &C,
|
||||
_session_store: &ExtensionData,
|
||||
_thread_store: &ExtensionData,
|
||||
) -> Vec<PromptFragment> {
|
||||
self.instruction(context)
|
||||
.map(PromptFragment::developer_capability)
|
||||
.map(PromptFragment::developer_policy)
|
||||
.into_iter()
|
||||
.collect()
|
||||
}
|
||||
@@ -51,7 +60,6 @@ pub fn extension() -> Arc<GitAttributionExtension> {
|
||||
Arc::new(GitAttributionExtension::new())
|
||||
}
|
||||
|
||||
// This is just a copy/paste
|
||||
fn build_commit_message_trailer(config_attribution: Option<&str>) -> Option<String> {
|
||||
let value = resolve_attribution_value(config_attribution)?;
|
||||
Some(format!("Co-authored-by: {value}"))
|
||||
@@ -70,3 +78,6 @@ fn resolve_attribution_value(config_attribution: Option<&str>) -> Option<String>
|
||||
None => Some(DEFAULT_ATTRIBUTION_VALUE.to_string()),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests;
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
use super::GitAttributionContext;
|
||||
use super::GitAttributionExtension;
|
||||
use super::build_commit_message_trailer;
|
||||
use super::resolve_attribution_value;
|
||||
|
||||
struct TestContext {
|
||||
git_attribution_enabled: bool,
|
||||
commit_attribution: Option<&'static str>,
|
||||
}
|
||||
|
||||
impl GitAttributionContext for TestContext {
|
||||
fn git_attribution_enabled(&self) -> bool {
|
||||
self.git_attribution_enabled
|
||||
}
|
||||
|
||||
fn commit_attribution(&self) -> Option<&str> {
|
||||
self.commit_attribution
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn blank_attribution_disables_trailer_prompt() {
|
||||
assert_eq!(build_commit_message_trailer(Some("")), None);
|
||||
assert_eq!(
|
||||
GitAttributionExtension::new().instruction(&TestContext {
|
||||
git_attribution_enabled: true,
|
||||
commit_attribution: Some(" "),
|
||||
}),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn disabled_context_omits_instruction() {
|
||||
assert_eq!(
|
||||
GitAttributionExtension::new().instruction(&TestContext {
|
||||
git_attribution_enabled: false,
|
||||
commit_attribution: None,
|
||||
}),
|
||||
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 = GitAttributionExtension::new()
|
||||
.instruction(&TestContext {
|
||||
git_attribution_enabled: true,
|
||||
commit_attribution: 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