mirror of
https://github.com/openai/codex.git
synced 2026-05-14 16:22:51 +00:00
feat: add config-change extension contributor (#22488)
## Why Extensions can observe thread and turn lifecycle events today, but there was no single host-owned hook for changes to the effective thread configuration. That makes features that need to react to model, permission, or tool-suggest updates either depend on individual mutation paths or risk going stale after runtime config refreshes. This adds a typed config-change contributor so extension-owned state can stay synchronized with the effective thread config while the host remains responsible for deciding when config changed. ## What Changed - Added `ConfigContributor<C>` to `codex_extension_api`, with before/after immutable snapshots of the effective config plus session/thread extension stores. - Added registry builder/accessor support through `config_contributor` and `config_contributors`. - Emits config-change callbacks after committed updates from session settings, per-turn setting updates, and `refresh_runtime_config`. - Builds effective config snapshots only when config contributors are registered, and suppresses no-op callbacks when the before/after snapshots are equal. - Added a core session regression test that verifies contributors observe both model changes and user-layer runtime config changes, including access to session and thread extension stores. ## Validation Added `config_change_contributor_observes_effective_config_changes` in `codex-rs/core/src/session/tests.rs` to cover the new contributor path.
This commit is contained in:
@@ -1317,7 +1317,16 @@ impl Session {
|
||||
&self,
|
||||
updates: SessionSettingsUpdate,
|
||||
) -> ConstraintResult<()> {
|
||||
let (previous_cwd, permission_profile_changed, next_cwd, codex_home, session_source) = {
|
||||
let notify_config_contributors = !self.services.extensions.config_contributors().is_empty();
|
||||
let (
|
||||
previous_config,
|
||||
new_config,
|
||||
previous_cwd,
|
||||
permission_profile_changed,
|
||||
next_cwd,
|
||||
codex_home,
|
||||
session_source,
|
||||
) = {
|
||||
let mut state = self.state.lock().await;
|
||||
let updated = match state.session_configuration.apply(&updates) {
|
||||
Ok(updated) => updated,
|
||||
@@ -1327,6 +1336,10 @@ impl Session {
|
||||
}
|
||||
};
|
||||
|
||||
let previous_config = notify_config_contributors
|
||||
.then(|| Self::build_effective_session_config(&state.session_configuration));
|
||||
let new_config =
|
||||
notify_config_contributors.then(|| Self::build_effective_session_config(&updated));
|
||||
let previous_cwd = state.session_configuration.cwd.clone();
|
||||
let previous_permission_profile = state.session_configuration.permission_profile();
|
||||
let updated_permission_profile = updated.permission_profile();
|
||||
@@ -1337,6 +1350,8 @@ impl Session {
|
||||
let session_source = updated.session_source.clone();
|
||||
state.session_configuration = updated;
|
||||
(
|
||||
previous_config,
|
||||
new_config,
|
||||
previous_cwd,
|
||||
permission_profile_changed,
|
||||
next_cwd,
|
||||
@@ -1345,6 +1360,7 @@ impl Session {
|
||||
)
|
||||
};
|
||||
|
||||
self.emit_config_changed_contributors(previous_config.as_ref(), new_config.as_ref());
|
||||
self.maybe_refresh_shell_snapshot_for_cwd(
|
||||
&previous_cwd,
|
||||
&next_cwd,
|
||||
@@ -1397,8 +1413,11 @@ impl Session {
|
||||
// Refresh only the user layer from the incoming snapshot. Preserve thread-local
|
||||
// layers such as request/session overrides that were present when this session
|
||||
// was created.
|
||||
let config = {
|
||||
let notify_config_contributors = !self.services.extensions.config_contributors().is_empty();
|
||||
let (previous_config, new_config, config) = {
|
||||
let mut state = self.state.lock().await;
|
||||
let previous_config = notify_config_contributors
|
||||
.then(|| Self::build_effective_session_config(&state.session_configuration));
|
||||
let mut config = (*state.session_configuration.original_config_do_not_use).clone();
|
||||
config.config_layer_stack = config
|
||||
.config_layer_stack
|
||||
@@ -1407,8 +1426,11 @@ impl Session {
|
||||
resolve_tool_suggest_config_from_layer_stack(&config.config_layer_stack);
|
||||
let config = Arc::new(config);
|
||||
state.session_configuration.original_config_do_not_use = Arc::clone(&config);
|
||||
config
|
||||
let new_config = notify_config_contributors
|
||||
.then(|| Self::build_effective_session_config(&state.session_configuration));
|
||||
(previous_config, new_config, config)
|
||||
};
|
||||
self.emit_config_changed_contributors(previous_config.as_ref(), new_config.as_ref());
|
||||
self.services.skills_manager.clear_cache();
|
||||
self.services.plugins_manager.clear_cache();
|
||||
let hooks = build_hooks_for_config(
|
||||
@@ -1429,6 +1451,28 @@ impl Session {
|
||||
}
|
||||
}
|
||||
|
||||
fn emit_config_changed_contributors(
|
||||
&self,
|
||||
previous_config: Option<&Config>,
|
||||
new_config: Option<&Config>,
|
||||
) {
|
||||
let (Some(previous_config), Some(new_config)) = (previous_config, new_config) else {
|
||||
return;
|
||||
};
|
||||
if previous_config == new_config {
|
||||
return;
|
||||
}
|
||||
for contributor in self.services.extensions.config_contributors() {
|
||||
contributor.on_config_changed(
|
||||
&self.services.session_extension_data,
|
||||
&self.services.thread_extension_data,
|
||||
self.conversation_id,
|
||||
previous_config,
|
||||
new_config,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn reload_user_config_layer(&self) {
|
||||
// Refresh layer-backed runtime state for an existing session, including enabled plugin,
|
||||
// skill, and hook state. Derived config fields such as feature gates and legacy notify
|
||||
|
||||
@@ -1915,6 +1915,138 @@ async fn record_token_usage_info_notifies_extension_contributors() {
|
||||
assert_eq!(expected, actual);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn config_change_contributor_observes_effective_config_changes() {
|
||||
struct SessionConfigMarker;
|
||||
struct ThreadConfigMarker;
|
||||
|
||||
#[derive(Debug, PartialEq)]
|
||||
struct RecordedConfigChange {
|
||||
thread_id: ThreadId,
|
||||
previous_model: Option<String>,
|
||||
new_model: Option<String>,
|
||||
previous_disabled_tools: Vec<ToolSuggestDisabledTool>,
|
||||
new_disabled_tools: Vec<ToolSuggestDisabledTool>,
|
||||
saw_session_store: bool,
|
||||
saw_thread_store: bool,
|
||||
}
|
||||
|
||||
struct ConfigRecorder {
|
||||
records: Arc<std::sync::Mutex<Vec<RecordedConfigChange>>>,
|
||||
}
|
||||
|
||||
impl codex_extension_api::ConfigContributor<crate::config::Config> for ConfigRecorder {
|
||||
fn on_config_changed(
|
||||
&self,
|
||||
session_store: &codex_extension_api::ExtensionData,
|
||||
thread_store: &codex_extension_api::ExtensionData,
|
||||
thread_id: ThreadId,
|
||||
previous_config: &crate::config::Config,
|
||||
new_config: &crate::config::Config,
|
||||
) {
|
||||
self.records
|
||||
.lock()
|
||||
.expect("config change records lock")
|
||||
.push(RecordedConfigChange {
|
||||
thread_id,
|
||||
previous_model: previous_config.model.clone(),
|
||||
new_model: new_config.model.clone(),
|
||||
previous_disabled_tools: previous_config.tool_suggest.disabled_tools.clone(),
|
||||
new_disabled_tools: new_config.tool_suggest.disabled_tools.clone(),
|
||||
saw_session_store: session_store.get::<SessionConfigMarker>().is_some(),
|
||||
saw_thread_store: thread_store.get::<ThreadConfigMarker>().is_some(),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
let (mut session, _turn_context) = make_session_and_context().await;
|
||||
let records = Arc::new(std::sync::Mutex::new(Vec::new()));
|
||||
let mut builder = codex_extension_api::ExtensionRegistryBuilder::<crate::config::Config>::new();
|
||||
builder.config_contributor(Arc::new(ConfigRecorder {
|
||||
records: Arc::clone(&records),
|
||||
}));
|
||||
session.services.extensions = Arc::new(builder.build());
|
||||
session
|
||||
.services
|
||||
.session_extension_data
|
||||
.insert(SessionConfigMarker);
|
||||
session
|
||||
.services
|
||||
.thread_extension_data
|
||||
.insert(ThreadConfigMarker);
|
||||
|
||||
let original_model = session.collaboration_mode().await.model().to_string();
|
||||
let original_disabled_tools = session
|
||||
.get_config()
|
||||
.await
|
||||
.tool_suggest
|
||||
.disabled_tools
|
||||
.clone();
|
||||
let next_model = if original_model == "gpt-5.4" {
|
||||
"gpt-5.2"
|
||||
} else {
|
||||
"gpt-5.4"
|
||||
};
|
||||
let collaboration_mode = session.collaboration_mode().await.with_updates(
|
||||
Some(next_model.to_string()),
|
||||
/*effort*/ None,
|
||||
/*developer_instructions*/ None,
|
||||
);
|
||||
session
|
||||
.update_settings(SessionSettingsUpdate {
|
||||
collaboration_mode: Some(collaboration_mode),
|
||||
..Default::default()
|
||||
})
|
||||
.await
|
||||
.expect("update settings");
|
||||
|
||||
let codex_home = session.codex_home().await;
|
||||
std::fs::create_dir_all(&codex_home).expect("create codex home");
|
||||
std::fs::write(
|
||||
codex_home.join(CONFIG_TOML_FILE),
|
||||
r#"[tool_suggest]
|
||||
disabled_tools = [
|
||||
{ type = "connector", id = " calendar " },
|
||||
{ type = "plugin", id = "slack@openai-curated" },
|
||||
]
|
||||
"#,
|
||||
)
|
||||
.expect("write user config");
|
||||
let next_config = load_latest_config_for_session(&session).await;
|
||||
session.refresh_runtime_config(next_config).await;
|
||||
|
||||
let expected_disabled_tools = vec![
|
||||
ToolSuggestDisabledTool::connector("calendar"),
|
||||
ToolSuggestDisabledTool::plugin("slack@openai-curated"),
|
||||
];
|
||||
let expected = vec![
|
||||
RecordedConfigChange {
|
||||
thread_id: session.conversation_id,
|
||||
previous_model: Some(original_model),
|
||||
new_model: Some(next_model.to_string()),
|
||||
previous_disabled_tools: original_disabled_tools.clone(),
|
||||
new_disabled_tools: original_disabled_tools.clone(),
|
||||
saw_session_store: true,
|
||||
saw_thread_store: true,
|
||||
},
|
||||
RecordedConfigChange {
|
||||
thread_id: session.conversation_id,
|
||||
previous_model: Some(next_model.to_string()),
|
||||
new_model: Some(next_model.to_string()),
|
||||
previous_disabled_tools: original_disabled_tools,
|
||||
new_disabled_tools: expected_disabled_tools,
|
||||
saw_session_store: true,
|
||||
saw_thread_store: true,
|
||||
},
|
||||
];
|
||||
let actual = records
|
||||
.lock()
|
||||
.expect("config change records lock")
|
||||
.drain(..)
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(expected, actual);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn record_initial_history_reconstructs_forked_transcript() {
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
|
||||
@@ -438,6 +438,18 @@ impl Session {
|
||||
per_turn_config
|
||||
}
|
||||
|
||||
pub(crate) fn build_effective_session_config(
|
||||
session_configuration: &SessionConfiguration,
|
||||
) -> Config {
|
||||
let mut config =
|
||||
Self::build_per_turn_config(session_configuration, session_configuration.cwd.clone());
|
||||
config.model = Some(session_configuration.collaboration_mode.model().to_string());
|
||||
config.permissions.approval_policy = session_configuration.approval_policy.clone();
|
||||
config.permissions.active_permission_profile =
|
||||
session_configuration.active_permission_profile.clone();
|
||||
config
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub(crate) fn make_turn_context(
|
||||
thread_id: ThreadId,
|
||||
@@ -588,6 +600,7 @@ impl Session {
|
||||
sub_id: String,
|
||||
updates: SessionSettingsUpdate,
|
||||
) -> CodexResult<Arc<TurnContext>> {
|
||||
let notify_config_contributors = !self.services.extensions.config_contributors().is_empty();
|
||||
let update_result: CodexResult<_> = {
|
||||
let mut state = self.state.lock().await;
|
||||
match state.session_configuration.clone().apply(&updates) {
|
||||
@@ -612,6 +625,11 @@ impl Session {
|
||||
previous_permission_profile != next_permission_profile;
|
||||
let codex_home = next.codex_home.clone();
|
||||
let session_source = next.session_source.clone();
|
||||
let previous_config = notify_config_contributors.then(|| {
|
||||
Self::build_effective_session_config(&state.session_configuration)
|
||||
});
|
||||
let new_config = notify_config_contributors
|
||||
.then(|| Self::build_effective_session_config(&next));
|
||||
state.session_configuration = next.clone();
|
||||
Ok((
|
||||
next,
|
||||
@@ -620,6 +638,8 @@ impl Session {
|
||||
previous_cwd,
|
||||
codex_home,
|
||||
session_source,
|
||||
previous_config,
|
||||
new_config,
|
||||
))
|
||||
}
|
||||
Err(err) => Err(CodexErr::InvalidRequest(err.to_string())),
|
||||
@@ -633,6 +653,8 @@ impl Session {
|
||||
previous_cwd,
|
||||
codex_home,
|
||||
session_source,
|
||||
previous_config,
|
||||
new_config,
|
||||
) = match update_result {
|
||||
Ok(update) => update,
|
||||
Err(err) => {
|
||||
@@ -649,6 +671,7 @@ impl Session {
|
||||
}
|
||||
};
|
||||
|
||||
self.emit_config_changed_contributors(previous_config.as_ref(), new_config.as_ref());
|
||||
self.maybe_refresh_shell_snapshot_for_cwd(
|
||||
&previous_cwd,
|
||||
&session_configuration.cwd,
|
||||
|
||||
@@ -67,6 +67,23 @@ pub trait TurnLifecycleContributor: Send + Sync {
|
||||
fn on_turn_abort(&self, _input: TurnAbortInput<'_>) {}
|
||||
}
|
||||
|
||||
/// Contributor for host-owned configuration changes.
|
||||
///
|
||||
/// Implementations should treat the supplied values as immutable before/after
|
||||
/// snapshots of the effective thread configuration.
|
||||
pub trait ConfigContributor<C>: Send + Sync {
|
||||
/// Called after the host commits a changed thread configuration.
|
||||
fn on_config_changed(
|
||||
&self,
|
||||
_session_store: &ExtensionData,
|
||||
_thread_store: &ExtensionData,
|
||||
_thread_id: ThreadId,
|
||||
_previous_config: &C,
|
||||
_new_config: &C,
|
||||
) {
|
||||
}
|
||||
}
|
||||
|
||||
/// Contributor for token usage checkpoints reported by the model provider.
|
||||
///
|
||||
/// Implementations should keep this callback cheap. The host calls it after
|
||||
|
||||
@@ -15,6 +15,7 @@ pub use codex_tools::ToolSpec;
|
||||
pub use codex_tools::parse_tool_input_schema;
|
||||
pub use contributors::ApprovalReviewContributor;
|
||||
pub use contributors::ApprovalReviewFuture;
|
||||
pub use contributors::ConfigContributor;
|
||||
pub use contributors::ContextContributor;
|
||||
pub use contributors::ExtensionToolExecutor;
|
||||
pub use contributors::ExtensionToolFuture;
|
||||
|
||||
@@ -2,6 +2,7 @@ use std::sync::Arc;
|
||||
|
||||
use crate::ApprovalReviewContributor;
|
||||
use crate::ApprovalReviewFuture;
|
||||
use crate::ConfigContributor;
|
||||
use crate::ContextContributor;
|
||||
use crate::ExtensionData;
|
||||
use crate::ThreadLifecycleContributor;
|
||||
@@ -14,6 +15,7 @@ use crate::TurnLifecycleContributor;
|
||||
pub struct ExtensionRegistryBuilder<C> {
|
||||
thread_lifecycle_contributors: Vec<Arc<dyn ThreadLifecycleContributor<C>>>,
|
||||
turn_lifecycle_contributors: Vec<Arc<dyn TurnLifecycleContributor>>,
|
||||
config_contributors: Vec<Arc<dyn ConfigContributor<C>>>,
|
||||
token_usage_contributors: Vec<Arc<dyn TokenUsageContributor>>,
|
||||
context_contributors: Vec<Arc<dyn ContextContributor>>,
|
||||
tool_contributors: Vec<Arc<dyn ToolContributor>>,
|
||||
@@ -26,6 +28,7 @@ impl<C> Default for ExtensionRegistryBuilder<C> {
|
||||
Self {
|
||||
thread_lifecycle_contributors: Vec::new(),
|
||||
turn_lifecycle_contributors: Vec::new(),
|
||||
config_contributors: Vec::new(),
|
||||
token_usage_contributors: Vec::new(),
|
||||
approval_review_contributors: Vec::new(),
|
||||
context_contributors: Vec::new(),
|
||||
@@ -59,6 +62,11 @@ impl<C> ExtensionRegistryBuilder<C> {
|
||||
self.turn_lifecycle_contributors.push(contributor);
|
||||
}
|
||||
|
||||
/// Registers one config contributor.
|
||||
pub fn config_contributor(&mut self, contributor: Arc<dyn ConfigContributor<C>>) {
|
||||
self.config_contributors.push(contributor);
|
||||
}
|
||||
|
||||
/// Registers one token-usage contributor.
|
||||
pub fn token_usage_contributor(&mut self, contributor: Arc<dyn TokenUsageContributor>) {
|
||||
self.token_usage_contributors.push(contributor);
|
||||
@@ -84,6 +92,7 @@ impl<C> ExtensionRegistryBuilder<C> {
|
||||
ExtensionRegistry {
|
||||
thread_lifecycle_contributors: self.thread_lifecycle_contributors,
|
||||
turn_lifecycle_contributors: self.turn_lifecycle_contributors,
|
||||
config_contributors: self.config_contributors,
|
||||
token_usage_contributors: self.token_usage_contributors,
|
||||
approval_review_contributors: self.approval_review_contributors,
|
||||
context_contributors: self.context_contributors,
|
||||
@@ -97,6 +106,7 @@ impl<C> ExtensionRegistryBuilder<C> {
|
||||
pub struct ExtensionRegistry<C> {
|
||||
thread_lifecycle_contributors: Vec<Arc<dyn ThreadLifecycleContributor<C>>>,
|
||||
turn_lifecycle_contributors: Vec<Arc<dyn TurnLifecycleContributor>>,
|
||||
config_contributors: Vec<Arc<dyn ConfigContributor<C>>>,
|
||||
token_usage_contributors: Vec<Arc<dyn TokenUsageContributor>>,
|
||||
context_contributors: Vec<Arc<dyn ContextContributor>>,
|
||||
tool_contributors: Vec<Arc<dyn ToolContributor>>,
|
||||
@@ -115,6 +125,11 @@ impl<C> ExtensionRegistry<C> {
|
||||
&self.turn_lifecycle_contributors
|
||||
}
|
||||
|
||||
/// Returns the registered config contributors.
|
||||
pub fn config_contributors(&self) -> &[Arc<dyn ConfigContributor<C>>] {
|
||||
&self.config_contributors
|
||||
}
|
||||
|
||||
/// Returns the registered token-usage contributors.
|
||||
pub fn token_usage_contributors(&self) -> &[Arc<dyn TokenUsageContributor>] {
|
||||
&self.token_usage_contributors
|
||||
|
||||
Reference in New Issue
Block a user