mirror of
https://github.com/openai/codex.git
synced 2026-11-09 00:39:32 +00:00
Persist disabled plugin IDs in thread settings (#44332)
## What changed - Add `disabled_plugin_ids` to thread startup options, settings overrides, snapshots, and persisted turn contexts. Omitted overrides preserve the current list; an empty list clears it. - Restore the selection on resume and capture it before truncating history for a fork. Prefer the owning thread's latest settings snapshot, falling back only to the latest turn context so stale history cannot resurrect an older selection. - Freeze the selection at turn admission and retain compatibility with histories that lack the new fields. ## Testing Add coverage for owned settings, explicit clears, latest-turn fallback, and legacy snapshots. Extend settings tests to cover plugin selection during runtime restoration and compaction, and verify initial turn persistence without extra settings checkpoints in both history modes. GitOrigin-RevId: 75ffbff5763cd93cc5ac69d73b08b237f59bc009
This commit is contained in:
1 parent
e722303e38
commit
b5544d5732
29 files changed
+374
-12
No files matched your search
@@ -436,6 +436,7 @@ pub(crate) struct SessionSpawnArgs {
|
||||
pub(crate) code_mode_session_provider: Arc<dyn codex_code_mode::CodeModeSessionProvider>,
|
||||
pub(crate) extensions: Arc<codex_extension_api::ExtensionRegistry<crate::config::Config>>,
|
||||
pub(crate) conversation_history: InitialHistory,
|
||||
pub(crate) disabled_plugin_ids: Option<Vec<String>>,
|
||||
pub(crate) requested_history_mode: Option<ThreadHistoryMode>,
|
||||
pub(crate) fork_persistence: ForkPersistence,
|
||||
pub(crate) session_source: SessionSource,
|
||||
@@ -540,6 +541,7 @@ impl Session {
|
||||
code_mode_session_provider,
|
||||
extensions,
|
||||
conversation_history,
|
||||
disabled_plugin_ids,
|
||||
requested_history_mode,
|
||||
fork_persistence,
|
||||
session_source,
|
||||
@@ -727,6 +729,22 @@ impl Session {
|
||||
} else {
|
||||
dynamic_tools
|
||||
};
|
||||
let disabled_plugin_ids = disabled_plugin_ids.unwrap_or_else(|| {
|
||||
let settings_owner = match &conversation_history {
|
||||
InitialHistory::Resumed(resumed) => Some(resumed.conversation_id),
|
||||
InitialHistory::Forked(_) => forked_from_thread_id,
|
||||
InitialHistory::New | InitialHistory::Cleared => None,
|
||||
};
|
||||
settings_owner
|
||||
.and_then(|thread_id| {
|
||||
codex_history::latest_disabled_plugin_ids(
|
||||
conversation_history.get_rollout_items(),
|
||||
thread_id,
|
||||
)
|
||||
})
|
||||
.map(<[String]>::to_vec)
|
||||
.unwrap_or_default()
|
||||
});
|
||||
// TODO (aibrahim): Consolidate config.model and config.model_reasoning_effort into config.collaboration_mode
|
||||
// to avoid extracting these fields separately and constructing CollaborationMode here.
|
||||
let collaboration_mode = CollaborationMode {
|
||||
@@ -771,6 +789,7 @@ impl Session {
|
||||
runtime_workspace_roots: config.workspace_roots.clone(),
|
||||
codex_home: config.codex_home.clone(),
|
||||
thread_name: None,
|
||||
disabled_plugin_ids,
|
||||
original_config_do_not_use: Arc::clone(&config),
|
||||
metrics_service_name,
|
||||
app_server_client_name: None,
|
||||
|
||||
@@ -147,6 +147,7 @@ pub(super) async fn spawn_review_thread(
|
||||
config: per_turn_config,
|
||||
auth_manager: auth_manager_for_context,
|
||||
initial_settings: Arc::clone(&step_settings),
|
||||
disabled_plugin_ids: parent_turn_context.disabled_plugin_ids.clone(),
|
||||
current_settings: ArcSwap::from(step_settings),
|
||||
session_telemetry: session_telemetry_for_context,
|
||||
provider: provider_for_context,
|
||||
|
||||
@@ -266,6 +266,7 @@ async fn record_initial_history_resumed_bare_turn_context_does_not_hydrate_previ
|
||||
let previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
root_turn_id: None,
|
||||
disabled_plugin_ids: None,
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.clone(),
|
||||
workspace_roots: None,
|
||||
@@ -316,6 +317,7 @@ async fn record_initial_history_resumed_hydrates_previous_turn_settings_from_lif
|
||||
let mut previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
root_turn_id: None,
|
||||
disabled_plugin_ids: None,
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.clone(),
|
||||
workspace_roots: None,
|
||||
@@ -1598,6 +1600,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis
|
||||
let previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
root_turn_id: Some("root-turn".to_string()),
|
||||
disabled_plugin_ids: None,
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.clone(),
|
||||
workspace_roots: None,
|
||||
@@ -1695,6 +1698,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis
|
||||
serde_json::to_value(Some(TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
root_turn_id: Some("root-turn".to_string()),
|
||||
disabled_plugin_ids: None,
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.clone(),
|
||||
workspace_roots: None,
|
||||
@@ -1730,6 +1734,7 @@ async fn record_initial_history_resumed_aborted_turn_without_id_clears_active_tu
|
||||
let previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
root_turn_id: None,
|
||||
disabled_plugin_ids: None,
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.clone(),
|
||||
workspace_roots: None,
|
||||
@@ -1868,6 +1873,7 @@ async fn record_initial_history_resumed_unmatched_abort_preserves_active_turn_fo
|
||||
let current_context_item = TurnContextItem {
|
||||
turn_id: Some(current_turn_id.clone()),
|
||||
root_turn_id: None,
|
||||
disabled_plugin_ids: None,
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.clone(),
|
||||
workspace_roots: None,
|
||||
@@ -1998,6 +2004,7 @@ async fn record_initial_history_resumed_trailing_incomplete_turn_compaction_clea
|
||||
let previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
root_turn_id: None,
|
||||
disabled_plugin_ids: None,
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.clone(),
|
||||
workspace_roots: None,
|
||||
@@ -2176,6 +2183,7 @@ async fn record_initial_history_resumed_replaced_incomplete_compacted_turn_clear
|
||||
let previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
root_turn_id: None,
|
||||
disabled_plugin_ids: None,
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.clone(),
|
||||
workspace_roots: None,
|
||||
|
||||
@@ -116,6 +116,8 @@ pub(crate) struct SessionConfiguration {
|
||||
pub(super) codex_home: AbsolutePathBuf,
|
||||
/// Optional user-facing name for the thread, updated during the session.
|
||||
pub(super) thread_name: Option<String>,
|
||||
/// Thread-owned plugin selection inherited by future turns.
|
||||
pub(super) disabled_plugin_ids: Vec<String>,
|
||||
|
||||
// TODO(pakrym): Remove config from here
|
||||
pub(super) original_config_do_not_use: Arc<Config>,
|
||||
@@ -302,6 +304,7 @@ impl SessionConfiguration {
|
||||
reasoning_summary: self.step_settings.reasoning_summary,
|
||||
personality: self.step_settings.personality,
|
||||
collaboration_mode: self.step_settings.collaboration_mode.clone(),
|
||||
disabled_plugin_ids: self.disabled_plugin_ids.clone(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -330,6 +333,7 @@ impl SessionConfiguration {
|
||||
service_tier: Some(self.step_settings.service_tier.clone()),
|
||||
collaboration_mode: Some(self.step_settings.collaboration_mode.clone()),
|
||||
personality: self.step_settings.personality,
|
||||
disabled_plugin_ids: Some(self.disabled_plugin_ids.clone()),
|
||||
..Default::default()
|
||||
}
|
||||
}
|
||||
@@ -370,6 +374,9 @@ impl SessionConfiguration {
|
||||
current_environments: &[TurnEnvironmentSelection],
|
||||
) -> ConstraintResult<Self> {
|
||||
let mut next_configuration = self.clone();
|
||||
if let Some(disabled_plugin_ids) = &updates.disabled_plugin_ids {
|
||||
next_configuration.disabled_plugin_ids = disabled_plugin_ids.clone();
|
||||
}
|
||||
let current_file_system_sandbox_policy =
|
||||
self.file_system_sandbox_policy(current_environments);
|
||||
let file_system_policy_has_rebindable_project_root_write =
|
||||
@@ -573,6 +580,7 @@ pub(crate) struct SessionSettingsUpdate {
|
||||
pub(crate) service_tier_for_turn: Option<String>,
|
||||
pub(crate) app_server_client_name: Option<String>,
|
||||
pub(crate) app_server_client_version: Option<String>,
|
||||
pub(crate) disabled_plugin_ids: Option<Vec<String>>,
|
||||
}
|
||||
|
||||
pub(crate) struct AppServerClientMetadata {
|
||||
|
||||
@@ -3807,6 +3807,7 @@ async fn record_initial_history_forked_hydrates_previous_turn_settings() {
|
||||
let previous_context_item = TurnContextItem {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
root_turn_id: None,
|
||||
disabled_plugin_ids: None,
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.clone(),
|
||||
workspace_roots: None,
|
||||
@@ -4527,6 +4528,7 @@ async fn set_rate_limits_retains_previous_credits() {
|
||||
runtime_workspace_roots: config.workspace_roots.clone(),
|
||||
codex_home: config.codex_home.clone(),
|
||||
thread_name: None,
|
||||
disabled_plugin_ids: Vec::new(),
|
||||
original_config_do_not_use: Arc::clone(&config),
|
||||
metrics_service_name: None,
|
||||
app_server_client_name: None,
|
||||
@@ -4647,6 +4649,7 @@ async fn set_rate_limits_updates_plan_type_when_present() {
|
||||
runtime_workspace_roots: config.workspace_roots.clone(),
|
||||
codex_home: config.codex_home.clone(),
|
||||
thread_name: None,
|
||||
disabled_plugin_ids: Vec::new(),
|
||||
original_config_do_not_use: Arc::clone(&config),
|
||||
metrics_service_name: None,
|
||||
app_server_client_name: None,
|
||||
@@ -5293,6 +5296,7 @@ pub(crate) async fn make_session_configuration_for_tests() -> SessionConfigurati
|
||||
runtime_workspace_roots: config.workspace_roots.clone(),
|
||||
codex_home: config.codex_home.clone(),
|
||||
thread_name: None,
|
||||
disabled_plugin_ids: Vec::new(),
|
||||
original_config_do_not_use: Arc::clone(&config),
|
||||
metrics_service_name: None,
|
||||
app_server_client_name: None,
|
||||
@@ -6376,6 +6380,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() {
|
||||
runtime_workspace_roots: config.workspace_roots.clone(),
|
||||
codex_home: config.codex_home.clone(),
|
||||
thread_name: None,
|
||||
disabled_plugin_ids: Vec::new(),
|
||||
original_config_do_not_use: Arc::clone(&config),
|
||||
metrics_service_name: None,
|
||||
app_server_client_name: None,
|
||||
@@ -6535,6 +6540,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
runtime_workspace_roots: config.workspace_roots.clone(),
|
||||
codex_home: config.codex_home.clone(),
|
||||
thread_name: None,
|
||||
disabled_plugin_ids: Vec::new(),
|
||||
original_config_do_not_use: Arc::clone(&config),
|
||||
metrics_service_name: None,
|
||||
app_server_client_name: None,
|
||||
@@ -6835,6 +6841,7 @@ async fn make_session_with_config_and_rx(
|
||||
runtime_workspace_roots: config.workspace_roots.clone(),
|
||||
codex_home: config.codex_home.clone(),
|
||||
thread_name: None,
|
||||
disabled_plugin_ids: Vec::new(),
|
||||
original_config_do_not_use: Arc::clone(&config),
|
||||
metrics_service_name: None,
|
||||
app_server_client_name: None,
|
||||
@@ -6963,6 +6970,7 @@ async fn make_session_with_history_source_and_agent_control_and_rx(
|
||||
runtime_workspace_roots: config.workspace_roots.clone(),
|
||||
codex_home: config.codex_home.clone(),
|
||||
thread_name: None,
|
||||
disabled_plugin_ids: Vec::new(),
|
||||
original_config_do_not_use: Arc::clone(&config),
|
||||
metrics_service_name: None,
|
||||
app_server_client_name: None,
|
||||
@@ -8701,6 +8709,7 @@ where
|
||||
runtime_workspace_roots: config.workspace_roots.clone(),
|
||||
codex_home: config.codex_home.clone(),
|
||||
thread_name: None,
|
||||
disabled_plugin_ids: Vec::new(),
|
||||
original_config_do_not_use: Arc::clone(&config),
|
||||
metrics_service_name: None,
|
||||
app_server_client_name: None,
|
||||
|
||||
@@ -1272,6 +1272,7 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() {
|
||||
code_mode_session_provider: Arc::new(codex_code_mode::DisabledCodeModeSessionProvider),
|
||||
extensions: codex_extension_api::empty_extension_registry(),
|
||||
conversation_history: InitialHistory::New,
|
||||
disabled_plugin_ids: None,
|
||||
requested_history_mode: None,
|
||||
fork_persistence: ForkPersistence::Copied,
|
||||
session_source: SessionSource::SubAgent(SubAgentSource::Other(
|
||||
|
||||
@@ -71,6 +71,7 @@ pub(super) fn prepare_update(overrides: ThreadSettingsOverrides) -> SessionSetti
|
||||
service_tier,
|
||||
collaboration_mode,
|
||||
personality,
|
||||
disabled_plugin_ids,
|
||||
} = overrides;
|
||||
SessionSettingsUpdate {
|
||||
step_settings: StepSettingsUpdate {
|
||||
@@ -90,6 +91,7 @@ pub(super) fn prepare_update(overrides: ThreadSettingsOverrides) -> SessionSetti
|
||||
permission_profile,
|
||||
active_permission_profile,
|
||||
windows_sandbox_level,
|
||||
disabled_plugin_ids,
|
||||
..Default::default()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -295,6 +295,8 @@ pub struct TurnContext {
|
||||
/// Frozen settings used to construct this context. Legacy turn consumers
|
||||
/// keep this view even when later steps use different settings.
|
||||
pub(crate) initial_settings: Arc<ResolvedStepSettings>,
|
||||
/// Thread-owned plugin selection captured when this turn was admitted.
|
||||
pub(crate) disabled_plugin_ids: Vec<String>,
|
||||
/// Snapshot for the next step; request consumers use their captured StepContext.
|
||||
pub(super) current_settings: ArcSwap<ResolvedStepSettings>,
|
||||
/// Turn-wide telemetry; model-attributed step work should use `StepContext::session_telemetry`.
|
||||
@@ -594,6 +596,7 @@ impl TurnContext {
|
||||
use_model_token_budget_defaults: self.use_model_token_budget_defaults,
|
||||
auth_manager: self.auth_manager.clone(),
|
||||
initial_settings: Arc::clone(&step_settings),
|
||||
disabled_plugin_ids: self.disabled_plugin_ids.clone(),
|
||||
current_settings: ArcSwap::from(step_settings),
|
||||
session_telemetry,
|
||||
provider: self.provider.clone(),
|
||||
@@ -655,6 +658,7 @@ impl TurnContext {
|
||||
TurnContextItem {
|
||||
turn_id: Some(self.sub_id.clone()),
|
||||
root_turn_id: self.turn_metadata_state.root_turn_id(),
|
||||
disabled_plugin_ids: Some(self.disabled_plugin_ids.clone()),
|
||||
cwd,
|
||||
workspace_roots: (!workspace_roots.is_empty()).then_some(workspace_roots),
|
||||
current_date: self.current_date.clone(),
|
||||
@@ -866,6 +870,7 @@ impl Session {
|
||||
use_model_token_budget_defaults,
|
||||
auth_manager,
|
||||
initial_settings: Arc::clone(&step_settings),
|
||||
disabled_plugin_ids: session_configuration.disabled_plugin_ids.clone(),
|
||||
current_settings: ArcSwap::from(step_settings),
|
||||
session_telemetry: session_telemetry_for_context,
|
||||
provider,
|
||||
|
||||
Reference in new issue
Block a user