Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 47 additions & 3 deletions codex-rs/core/src/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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();
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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());
Comment thread
jif-oai marked this conversation as resolved.
self.services.skills_manager.clear_cache();
self.services.plugins_manager.clear_cache();
let hooks = build_hooks_for_config(
Expand All @@ -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
Expand Down
132 changes: 132 additions & 0 deletions codex-rs/core/src/session/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1912,6 +1912,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;
Expand Down
23 changes: 23 additions & 0 deletions codex-rs/core/src/session/turn_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,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,
Expand Down Expand Up @@ -584,6 +596,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) {
Expand All @@ -608,6 +621,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,
Expand All @@ -616,6 +634,8 @@ impl Session {
previous_cwd,
codex_home,
session_source,
previous_config,
new_config,
))
}
Err(err) => Err(CodexErr::InvalidRequest(err.to_string())),
Expand All @@ -629,6 +649,8 @@ impl Session {
previous_cwd,
codex_home,
session_source,
previous_config,
new_config,
) = match update_result {
Ok(update) => update,
Err(err) => {
Expand All @@ -645,6 +667,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,
Expand Down
17 changes: 17 additions & 0 deletions codex-rs/ext/extension-api/src/contributors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,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
Expand Down
1 change: 1 addition & 0 deletions codex-rs/ext/extension-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading
Loading