mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[3 of 4] tui: route feature and memory toggles through app server (#22915)
## Why Experimental feature toggles and memory settings can update several related config values in one interaction. Keeping those writes local in a remote TUI session is especially dangerous because the UI can diverge from the app-server config while also leaving behind partially stale supporting keys. This is **[3 of 4]** in a stacked series that moves TUI-owned config mutations onto app-server APIs. ## What changed - Routed feature flag persistence through app-server batch writes, including the supporting reviewer and permission updates used by guardian approval. - Routed Windows sandbox mode persistence and legacy Windows feature cleanup through app-server writes. - Routed memory settings through app-server batch writes and updated the TUI tests to exercise the embedded app-server path. ## Config keys affected - `features.<feature_key>` - `profiles.<profile>.features.<feature_key>` - `approval_policy` - `sandbox_mode` - `approvals_reviewer` - `windows.sandbox` - `features.experimental_windows_sandbox` - `features.elevated_windows_sandbox` - `features.enable_experimental_windows_sandbox` - Profile-scoped Windows legacy feature variants under `profiles.<profile>.features.*` - `memories.use_memories` - `memories.generate_memories` - Profile-scoped memory variants under `profiles.<profile>.memories.*` ## Suggested manual validation - Connect the TUI to a remote app server, toggle guardian approval on and off, and confirm the remote config updates `features.guardian_approval`, reviewer state, approval policy, and sandbox mode coherently. - Toggle a default-false experimental feature at the root level, disable it again, and confirm the key clears instead of lingering as an unnecessary explicit `false`. - Change memory settings and confirm the remote config updates both memory keys while the running TUI reflects the new state. - On Windows, switch sandbox mode through the TUI and confirm `windows.sandbox` is updated while the legacy Windows feature keys are cleared. ## Stack 1. [#22913](https://github.com/openai/codex/pull/22913) `[1 of 4]` primary settings writes 2. [#22914](https://github.com/openai/codex/pull/22914) `[2 of 4]` app and skill enablement 3. [#22915](https://github.com/openai/codex/pull/22915) `[3 of 4]` feature and memory toggles 4. [#22916](https://github.com/openai/codex/pull/22916) `[4 of 4]` startup and onboarding bookkeeping
This commit is contained in:
committed by
GitHub
Unverified
parent
16d85e2708
commit
e8378c7f0c
@@ -50,7 +50,6 @@ use crate::legacy_core::config::Config;
|
||||
use crate::legacy_core::config::ConfigBuilder;
|
||||
use crate::legacy_core::config::ConfigOverrides;
|
||||
use crate::legacy_core::config::PermissionProfileSnapshot;
|
||||
use crate::legacy_core::config::edit::ConfigEdit;
|
||||
use crate::legacy_core::config::edit::ConfigEditsBuilder;
|
||||
#[cfg(target_os = "windows")]
|
||||
use crate::legacy_core::windows_sandbox::WindowsSandboxLevelExt;
|
||||
@@ -91,6 +90,7 @@ use codex_app_server_protocol::ClientRequest;
|
||||
use codex_app_server_protocol::CodexErrorInfo as AppServerCodexErrorInfo;
|
||||
use codex_app_server_protocol::ConfigBatchWriteParams;
|
||||
use codex_app_server_protocol::ConfigLayerSource;
|
||||
use codex_app_server_protocol::ConfigReadResponse;
|
||||
use codex_app_server_protocol::ConfigValueWriteParams;
|
||||
use codex_app_server_protocol::ConfigWriteResponse;
|
||||
use codex_app_server_protocol::FeedbackUploadParams;
|
||||
@@ -113,6 +113,7 @@ use codex_app_server_protocol::PluginReadResponse;
|
||||
use codex_app_server_protocol::PluginUninstallParams;
|
||||
use codex_app_server_protocol::PluginUninstallResponse;
|
||||
use codex_app_server_protocol::RateLimitSnapshot;
|
||||
use codex_app_server_protocol::SandboxMode as AppServerSandboxMode;
|
||||
use codex_app_server_protocol::SendAddCreditsNudgeEmailParams;
|
||||
use codex_app_server_protocol::ServerNotification;
|
||||
use codex_app_server_protocol::ServerRequest;
|
||||
@@ -127,12 +128,17 @@ use codex_app_server_protocol::ThreadStartSource;
|
||||
use codex_app_server_protocol::Turn;
|
||||
use codex_app_server_protocol::TurnError as AppServerTurnError;
|
||||
use codex_app_server_protocol::TurnStatus;
|
||||
use codex_app_server_protocol::WriteStatus;
|
||||
use codex_config::ConfigLayerStackOrdering;
|
||||
use codex_config::LoaderOverrides;
|
||||
use codex_config::types::ApprovalsReviewer;
|
||||
use codex_config::types::MemoriesToml;
|
||||
use codex_config::types::ModelAvailabilityNuxConfig;
|
||||
#[cfg(target_os = "windows")]
|
||||
use codex_config::types::WindowsToml;
|
||||
use codex_exec_server::EnvironmentManager;
|
||||
use codex_features::Feature;
|
||||
use codex_features::FeaturesToml;
|
||||
use codex_model_provider::create_model_provider;
|
||||
use codex_model_provider_info::ModelProviderInfo;
|
||||
use codex_models_manager::model_presets::HIDE_GPT_5_1_CODEX_MAX_MIGRATION_PROMPT_CONFIG;
|
||||
|
||||
@@ -198,6 +198,28 @@ impl App {
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) async fn read_effective_config_after_overridden_write(
|
||||
&mut self,
|
||||
app_server: &mut AppServerSession,
|
||||
setting: &str,
|
||||
) -> Option<ConfigReadResponse> {
|
||||
let cwd = self.chat_widget.config_ref().cwd.display().to_string();
|
||||
match crate::config_update::read_effective_config(app_server.request_handle(), cwd).await {
|
||||
Ok(response) => Some(response),
|
||||
Err(err) => {
|
||||
tracing::warn!(
|
||||
error = %err,
|
||||
setting,
|
||||
"failed to refresh effective config after an overridden write"
|
||||
);
|
||||
self.chat_widget.add_error_message(format!(
|
||||
"{setting} were saved, but Codex could not refresh the effective config: {err}"
|
||||
));
|
||||
None
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) async fn rebuild_config_for_resume_or_fallback(
|
||||
&mut self,
|
||||
current_cwd: &Path,
|
||||
@@ -311,7 +333,11 @@ impl App {
|
||||
Some(permission_profile)
|
||||
}
|
||||
|
||||
pub(super) async fn update_feature_flags(&mut self, updates: Vec<(Feature, bool)>) {
|
||||
pub(super) async fn update_feature_flags(
|
||||
&mut self,
|
||||
app_server: &mut AppServerSession,
|
||||
updates: Vec<(Feature, bool)>,
|
||||
) {
|
||||
if updates.is_empty() {
|
||||
return;
|
||||
}
|
||||
@@ -319,13 +345,6 @@ impl App {
|
||||
let auto_review_preset = auto_review_mode();
|
||||
let mut next_config = self.config.clone();
|
||||
let active_profile = self.active_profile.clone();
|
||||
let scoped_segments = |key: &str| {
|
||||
if let Some(profile) = active_profile.as_deref() {
|
||||
vec!["profiles".to_string(), profile.to_string(), key.to_string()]
|
||||
} else {
|
||||
vec![key.to_string()]
|
||||
}
|
||||
};
|
||||
let windows_sandbox_changed = updates.iter().any(|(feature, _)| {
|
||||
matches!(
|
||||
feature,
|
||||
@@ -358,8 +377,7 @@ impl App {
|
||||
(root_blocks_disable, profile_configured)
|
||||
};
|
||||
let mut permissions_history_label: Option<&'static str> = None;
|
||||
let mut builder = ConfigEditsBuilder::for_config(&self.config)
|
||||
.with_profile(self.active_profile.as_deref());
|
||||
let mut config_edits = Vec::new();
|
||||
|
||||
for (feature, enabled) in updates {
|
||||
let feature_key = feature.key();
|
||||
@@ -394,18 +412,24 @@ impl App {
|
||||
// experiment's matching `/permissions` mode until the user
|
||||
// changes it explicitly.
|
||||
feature_config.approvals_reviewer = auto_review_preset.approvals_reviewer;
|
||||
feature_edits.push(ConfigEdit::SetPath {
|
||||
segments: scoped_segments("approvals_reviewer"),
|
||||
value: auto_review_preset.approvals_reviewer.to_string().into(),
|
||||
});
|
||||
feature_edits.push(crate::config_update::replace_config_value(
|
||||
crate::config_update::profile_scoped_key_path(
|
||||
active_profile.as_deref(),
|
||||
"approvals_reviewer",
|
||||
),
|
||||
serde_json::json!(auto_review_preset.approvals_reviewer.to_string()),
|
||||
));
|
||||
if previous_approvals_reviewer != auto_review_preset.approvals_reviewer {
|
||||
permissions_history_label = Some("Auto-review");
|
||||
}
|
||||
} else if !effective_enabled {
|
||||
if profile_approvals_reviewer_configured || self.active_profile.is_none() {
|
||||
feature_edits.push(ConfigEdit::ClearPath {
|
||||
segments: scoped_segments("approvals_reviewer"),
|
||||
});
|
||||
feature_edits.push(crate::config_update::clear_config_value(
|
||||
crate::config_update::profile_scoped_key_path(
|
||||
active_profile.as_deref(),
|
||||
"approvals_reviewer",
|
||||
),
|
||||
));
|
||||
}
|
||||
feature_config.approvals_reviewer = ApprovalsReviewer::User;
|
||||
if previous_approvals_reviewer != ApprovalsReviewer::User {
|
||||
@@ -438,14 +462,20 @@ impl App {
|
||||
continue;
|
||||
};
|
||||
feature_edits.extend([
|
||||
ConfigEdit::SetPath {
|
||||
segments: scoped_segments("approval_policy"),
|
||||
value: "on-request".into(),
|
||||
},
|
||||
ConfigEdit::SetPath {
|
||||
segments: scoped_segments("sandbox_mode"),
|
||||
value: "workspace-write".into(),
|
||||
},
|
||||
crate::config_update::replace_config_value(
|
||||
crate::config_update::profile_scoped_key_path(
|
||||
active_profile.as_deref(),
|
||||
"approval_policy",
|
||||
),
|
||||
serde_json::json!("on-request"),
|
||||
),
|
||||
crate::config_update::replace_config_value(
|
||||
crate::config_update::profile_scoped_key_path(
|
||||
active_profile.as_deref(),
|
||||
"sandbox_mode",
|
||||
),
|
||||
serde_json::json!("workspace-write"),
|
||||
),
|
||||
]);
|
||||
approval_policy_override = Some(auto_review_preset.approval_policy);
|
||||
permission_profile_override = Some(permission_profile);
|
||||
@@ -454,18 +484,60 @@ impl App {
|
||||
}
|
||||
next_config = feature_config;
|
||||
feature_updates_to_apply.push((feature, effective_enabled));
|
||||
builder = builder
|
||||
.with_edits(feature_edits)
|
||||
.set_feature_enabled(feature_key, effective_enabled);
|
||||
config_edits.extend(feature_edits);
|
||||
config_edits.push(crate::config_update::build_feature_enabled_edit(
|
||||
active_profile.as_deref(),
|
||||
feature_key,
|
||||
effective_enabled,
|
||||
));
|
||||
}
|
||||
|
||||
// Persist first so the live session does not diverge from disk if the
|
||||
// config edit fails. Runtime/UI state is patched below only after the
|
||||
// durable config update succeeds.
|
||||
if let Err(err) = builder.apply().await {
|
||||
tracing::error!(error = %err, "failed to persist feature flags");
|
||||
self.chat_widget
|
||||
.add_error_message(format!("Failed to update experimental features: {err}"));
|
||||
let write_response = match crate::config_update::write_config_batch(
|
||||
app_server.request_handle(),
|
||||
config_edits,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(response) => response,
|
||||
Err(err) => {
|
||||
tracing::error!(error = %err, "failed to persist feature flags");
|
||||
self.chat_widget
|
||||
.add_error_message(format!("Failed to update experimental features: {err}"));
|
||||
return;
|
||||
}
|
||||
};
|
||||
if write_response.status == WriteStatus::OkOverridden {
|
||||
let message = overridden_write_message(&write_response);
|
||||
tracing::warn!(
|
||||
message,
|
||||
"feature flag config write was overridden by effective config"
|
||||
);
|
||||
self.chat_widget.add_error_message(format!(
|
||||
"Experimental feature changes were saved but not applied: {message}"
|
||||
));
|
||||
if let Some(effective_config) = self
|
||||
.read_effective_config_after_overridden_write(
|
||||
app_server,
|
||||
"Experimental feature changes",
|
||||
)
|
||||
.await
|
||||
{
|
||||
self.sync_feature_state_from_effective_config(
|
||||
&effective_config,
|
||||
&feature_updates_to_apply,
|
||||
);
|
||||
self.sync_auto_review_runtime_state_from_effective_config(
|
||||
&effective_config,
|
||||
&feature_updates_to_apply,
|
||||
)
|
||||
.await;
|
||||
if windows_sandbox_changed {
|
||||
self.propagate_windows_sandbox_turn_context();
|
||||
}
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -550,26 +622,7 @@ impl App {
|
||||
}
|
||||
|
||||
if windows_sandbox_changed {
|
||||
#[cfg(target_os = "windows")]
|
||||
{
|
||||
let windows_sandbox_level = WindowsSandboxLevel::from_config(&self.config);
|
||||
self.app_event_tx
|
||||
.send(AppEvent::CodexOp(AppCommand::override_turn_context(
|
||||
/*cwd*/ None,
|
||||
/*approval_policy*/ None,
|
||||
/*approvals_reviewer*/ None,
|
||||
/*permission_profile*/ None,
|
||||
/*active_permission_profile*/ None,
|
||||
#[cfg(target_os = "windows")]
|
||||
Some(windows_sandbox_level),
|
||||
/*model*/ None,
|
||||
/*effort*/ None,
|
||||
/*summary*/ None,
|
||||
/*service_tier*/ None,
|
||||
/*collaboration_mode*/ None,
|
||||
/*personality*/ None,
|
||||
)));
|
||||
}
|
||||
self.propagate_windows_sandbox_turn_context();
|
||||
}
|
||||
|
||||
if let Some(label) = permissions_history_label {
|
||||
@@ -582,42 +635,43 @@ impl App {
|
||||
|
||||
pub(super) async fn update_memory_settings(
|
||||
&mut self,
|
||||
app_server: &mut AppServerSession,
|
||||
use_memories: bool,
|
||||
generate_memories: bool,
|
||||
) -> bool {
|
||||
let active_profile = self.active_profile.clone();
|
||||
let scoped_memory_segments = |key: &str| {
|
||||
if let Some(profile) = active_profile.as_deref() {
|
||||
vec![
|
||||
"profiles".to_string(),
|
||||
profile.to_string(),
|
||||
"memories".to_string(),
|
||||
key.to_string(),
|
||||
]
|
||||
} else {
|
||||
vec!["memories".to_string(), key.to_string()]
|
||||
let edits =
|
||||
crate::config_update::build_memory_settings_edits(use_memories, generate_memories);
|
||||
|
||||
let write_response = match crate::config_update::write_config_batch(
|
||||
app_server.request_handle(),
|
||||
edits,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(response) => response,
|
||||
Err(err) => {
|
||||
tracing::error!(error = %err, "failed to persist memory settings");
|
||||
self.chat_widget
|
||||
.add_error_message(format!("Failed to save memory settings: {err}"));
|
||||
return false;
|
||||
}
|
||||
};
|
||||
let edits = [
|
||||
ConfigEdit::SetPath {
|
||||
segments: scoped_memory_segments("use_memories"),
|
||||
value: use_memories.into(),
|
||||
},
|
||||
ConfigEdit::SetPath {
|
||||
segments: scoped_memory_segments("generate_memories"),
|
||||
value: generate_memories.into(),
|
||||
},
|
||||
];
|
||||
|
||||
if let Err(err) = ConfigEditsBuilder::for_config(&self.config)
|
||||
.with_edits(edits)
|
||||
.apply()
|
||||
.await
|
||||
{
|
||||
tracing::error!(error = %err, "failed to persist memory settings");
|
||||
self.chat_widget
|
||||
.add_error_message(format!("Failed to save memory settings: {err}"));
|
||||
return false;
|
||||
if write_response.status == WriteStatus::OkOverridden {
|
||||
let message = overridden_write_message(&write_response);
|
||||
tracing::warn!(
|
||||
message,
|
||||
"memory settings config write was overridden by effective config"
|
||||
);
|
||||
self.chat_widget.add_error_message(format!(
|
||||
"Memory setting changes were saved but not applied: {message}"
|
||||
));
|
||||
let Some(effective_config) = self
|
||||
.read_effective_config_after_overridden_write(app_server, "Memory setting changes")
|
||||
.await
|
||||
else {
|
||||
return false;
|
||||
};
|
||||
return self.sync_memory_state_from_effective_config(&effective_config);
|
||||
}
|
||||
|
||||
self.config.memories.use_memories = use_memories;
|
||||
@@ -635,12 +689,13 @@ impl App {
|
||||
) {
|
||||
let previous_generate_memories = self.config.memories.generate_memories;
|
||||
if !self
|
||||
.update_memory_settings(use_memories, generate_memories)
|
||||
.update_memory_settings(app_server, use_memories, generate_memories)
|
||||
.await
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
let generate_memories = self.config.memories.generate_memories;
|
||||
if previous_generate_memories == generate_memories {
|
||||
return;
|
||||
}
|
||||
@@ -754,6 +809,311 @@ impl App {
|
||||
Personality::Pragmatic => "Pragmatic",
|
||||
}
|
||||
}
|
||||
|
||||
fn sync_feature_state_from_effective_config(
|
||||
&mut self,
|
||||
effective_config: &ConfigReadResponse,
|
||||
feature_updates: &[(Feature, bool)],
|
||||
) {
|
||||
let active_profile = self.active_profile.clone();
|
||||
let active_profile = active_profile.as_deref();
|
||||
for (feature, _) in feature_updates {
|
||||
let enabled =
|
||||
feature_enabled_from_effective_config(effective_config, active_profile, *feature);
|
||||
if let Err(err) = self.config.features.set_enabled(*feature, enabled) {
|
||||
tracing::warn!(
|
||||
error = %err,
|
||||
feature = feature.key(),
|
||||
"failed to sync effective feature state after an overridden write"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
self.chat_widget.set_feature_enabled(*feature, enabled);
|
||||
}
|
||||
|
||||
if feature_updates
|
||||
.iter()
|
||||
.any(|(feature, _)| *feature == Feature::GuardianApproval)
|
||||
&& !self.config.features.enabled(Feature::GuardianApproval)
|
||||
{
|
||||
self.set_approvals_reviewer_in_app_and_widget(ApprovalsReviewer::User);
|
||||
return;
|
||||
}
|
||||
|
||||
if let Some(reviewer) =
|
||||
approvals_reviewer_from_effective_config(effective_config, active_profile)
|
||||
{
|
||||
self.set_approvals_reviewer_in_app_and_widget(reviewer);
|
||||
}
|
||||
if let Some(policy) =
|
||||
approval_policy_from_effective_config(effective_config, active_profile)
|
||||
{
|
||||
if let Err(err) = self
|
||||
.config
|
||||
.permissions
|
||||
.approval_policy
|
||||
.set(policy.to_core())
|
||||
{
|
||||
tracing::warn!(
|
||||
error = %err,
|
||||
"failed to sync effective approval policy after an overridden write"
|
||||
);
|
||||
self.chat_widget.add_error_message(format!(
|
||||
"Failed to refresh overridden Auto-review settings: {err}"
|
||||
));
|
||||
} else {
|
||||
self.chat_widget.set_approval_policy(policy);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn sync_auto_review_runtime_state_from_effective_config(
|
||||
&mut self,
|
||||
effective_config: &ConfigReadResponse,
|
||||
feature_updates: &[(Feature, bool)],
|
||||
) {
|
||||
if !feature_updates
|
||||
.iter()
|
||||
.any(|(feature, _)| *feature == Feature::GuardianApproval)
|
||||
|| !self.config.features.enabled(Feature::GuardianApproval)
|
||||
|| sandbox_mode_from_effective_config(effective_config, self.active_profile.as_deref())
|
||||
!= Some(AppServerSandboxMode::WorkspaceWrite)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
let auto_review_preset = auto_review_mode();
|
||||
let mut config = self.config.clone();
|
||||
let Some(permission_profile) = self.try_set_builtin_active_permission_profile_on_config(
|
||||
&mut config,
|
||||
auto_review_preset.active_permission_profile.clone(),
|
||||
"Failed to refresh overridden Auto-review settings",
|
||||
"failed to sync overridden Auto-review permission profile",
|
||||
) else {
|
||||
return;
|
||||
};
|
||||
self.config = config;
|
||||
if let Err(err) = self
|
||||
.chat_widget
|
||||
.set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active(
|
||||
permission_profile.clone(),
|
||||
auto_review_preset.active_permission_profile.clone(),
|
||||
))
|
||||
{
|
||||
tracing::warn!(
|
||||
error = %err,
|
||||
"failed to sync overridden Auto-review permission profile on chat config"
|
||||
);
|
||||
self.chat_widget.add_error_message(format!(
|
||||
"Failed to refresh overridden Auto-review settings: {err}"
|
||||
));
|
||||
return;
|
||||
}
|
||||
|
||||
self.runtime_permission_profile_override = Some(permission_profile);
|
||||
self.sync_active_thread_permission_settings_to_cached_session()
|
||||
.await;
|
||||
|
||||
let approval_policy = AskForApproval::from(self.config.permissions.approval_policy.value());
|
||||
let op = AppCommand::override_turn_context(
|
||||
/*cwd*/ None,
|
||||
Some(approval_policy),
|
||||
Some(self.config.approvals_reviewer),
|
||||
/*permission_profile*/ None,
|
||||
Some(auto_review_preset.active_permission_profile),
|
||||
/*windows_sandbox_level*/ None,
|
||||
/*model*/ None,
|
||||
/*effort*/ None,
|
||||
/*summary*/ None,
|
||||
/*service_tier*/ None,
|
||||
/*collaboration_mode*/ None,
|
||||
/*personality*/ None,
|
||||
);
|
||||
let replay_state_op =
|
||||
ThreadEventStore::op_can_change_pending_replay_state(&op).then(|| op.clone());
|
||||
let submitted = self.chat_widget.submit_op(op);
|
||||
if submitted && let Some(op) = replay_state_op.as_ref() {
|
||||
self.note_active_thread_outbound_op(op).await;
|
||||
self.refresh_pending_thread_approvals().await;
|
||||
}
|
||||
}
|
||||
|
||||
fn sync_memory_state_from_effective_config(
|
||||
&mut self,
|
||||
effective_config: &ConfigReadResponse,
|
||||
) -> bool {
|
||||
let Some(memories) = memories_from_effective_config(effective_config) else {
|
||||
tracing::warn!(
|
||||
"config/read omitted memories after an overridden memory settings write"
|
||||
);
|
||||
return false;
|
||||
};
|
||||
let use_memories = memories
|
||||
.use_memories
|
||||
.unwrap_or(self.config.memories.use_memories);
|
||||
let generate_memories = memories
|
||||
.generate_memories
|
||||
.unwrap_or(self.config.memories.generate_memories);
|
||||
self.config.memories.use_memories = use_memories;
|
||||
self.config.memories.generate_memories = generate_memories;
|
||||
self.chat_widget
|
||||
.set_memory_settings(use_memories, generate_memories);
|
||||
true
|
||||
}
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
pub(super) async fn sync_windows_sandbox_after_overridden_write(
|
||||
&mut self,
|
||||
app_server: &mut AppServerSession,
|
||||
write_response: &ConfigWriteResponse,
|
||||
) {
|
||||
let message = overridden_write_message(write_response);
|
||||
tracing::warn!(
|
||||
message,
|
||||
"Windows sandbox config write was overridden by effective config"
|
||||
);
|
||||
self.chat_widget.add_error_message(format!(
|
||||
"Windows sandbox changes were saved but not applied: {message}"
|
||||
));
|
||||
let Some(effective_config) = self
|
||||
.read_effective_config_after_overridden_write(app_server, "Windows sandbox changes")
|
||||
.await
|
||||
else {
|
||||
return;
|
||||
};
|
||||
let Some(mode) = windows_sandbox_mode_from_effective_config(
|
||||
&effective_config,
|
||||
self.active_profile.as_deref(),
|
||||
) else {
|
||||
return;
|
||||
};
|
||||
self.config.permissions.windows_sandbox_mode = Some(mode);
|
||||
self.chat_widget.set_windows_sandbox_mode(Some(mode));
|
||||
self.propagate_windows_sandbox_turn_context();
|
||||
}
|
||||
|
||||
fn propagate_windows_sandbox_turn_context(&self) {
|
||||
#[cfg(target_os = "windows")]
|
||||
{
|
||||
let windows_sandbox_level = WindowsSandboxLevel::from_config(&self.config);
|
||||
self.app_event_tx
|
||||
.send(AppEvent::CodexOp(AppCommand::override_turn_context(
|
||||
/*cwd*/ None,
|
||||
/*approval_policy*/ None,
|
||||
/*approvals_reviewer*/ None,
|
||||
/*permission_profile*/ None,
|
||||
/*active_permission_profile*/ None,
|
||||
Some(windows_sandbox_level),
|
||||
/*model*/ None,
|
||||
/*effort*/ None,
|
||||
/*summary*/ None,
|
||||
/*service_tier*/ None,
|
||||
/*collaboration_mode*/ None,
|
||||
/*personality*/ None,
|
||||
)));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn overridden_write_message(write_response: &ConfigWriteResponse) -> &str {
|
||||
write_response
|
||||
.overridden_metadata
|
||||
.as_ref()
|
||||
.map(|metadata| metadata.message.as_str())
|
||||
.unwrap_or("the effective config is overridden by a higher-priority layer")
|
||||
}
|
||||
|
||||
fn feature_enabled_from_effective_config(
|
||||
effective_config: &ConfigReadResponse,
|
||||
active_profile: Option<&str>,
|
||||
feature: Feature,
|
||||
) -> bool {
|
||||
let profile_features = active_profile
|
||||
.and_then(|profile| effective_config.config.profiles.get(profile))
|
||||
.and_then(|profile| profile.additional.get("features"))
|
||||
.and_then(features_toml_from_json);
|
||||
let root_features = effective_config
|
||||
.config
|
||||
.additional
|
||||
.get("features")
|
||||
.and_then(features_toml_from_json);
|
||||
profile_features
|
||||
.as_ref()
|
||||
.and_then(|features| features.entries().get(feature.key()).copied())
|
||||
.or_else(|| {
|
||||
root_features
|
||||
.as_ref()
|
||||
.and_then(|features| features.entries().get(feature.key()).copied())
|
||||
})
|
||||
.unwrap_or_else(|| feature.default_enabled())
|
||||
}
|
||||
|
||||
fn approvals_reviewer_from_effective_config(
|
||||
effective_config: &ConfigReadResponse,
|
||||
active_profile: Option<&str>,
|
||||
) -> Option<ApprovalsReviewer> {
|
||||
active_profile
|
||||
.and_then(|profile| effective_config.config.profiles.get(profile))
|
||||
.and_then(|profile| profile.approvals_reviewer)
|
||||
.or(effective_config.config.approvals_reviewer)
|
||||
.map(codex_app_server_protocol::ApprovalsReviewer::to_core)
|
||||
}
|
||||
|
||||
fn approval_policy_from_effective_config(
|
||||
effective_config: &ConfigReadResponse,
|
||||
active_profile: Option<&str>,
|
||||
) -> Option<AskForApproval> {
|
||||
active_profile
|
||||
.and_then(|profile| effective_config.config.profiles.get(profile))
|
||||
.and_then(|profile| profile.approval_policy)
|
||||
.or(effective_config.config.approval_policy)
|
||||
}
|
||||
|
||||
fn sandbox_mode_from_effective_config(
|
||||
effective_config: &ConfigReadResponse,
|
||||
active_profile: Option<&str>,
|
||||
) -> Option<AppServerSandboxMode> {
|
||||
active_profile
|
||||
.and_then(|profile| effective_config.config.profiles.get(profile))
|
||||
.and_then(|profile| profile.additional.get("sandbox_mode"))
|
||||
.and_then(|mode| serde_json::from_value(mode.clone()).ok())
|
||||
.or(effective_config.config.sandbox_mode)
|
||||
}
|
||||
|
||||
fn memories_from_effective_config(effective_config: &ConfigReadResponse) -> Option<MemoriesToml> {
|
||||
effective_config
|
||||
.config
|
||||
.additional
|
||||
.get("memories")
|
||||
.and_then(|memories| serde_json::from_value(memories.clone()).ok())
|
||||
}
|
||||
|
||||
fn features_toml_from_json(value: &serde_json::Value) -> Option<FeaturesToml> {
|
||||
serde_json::from_value(value.clone()).ok()
|
||||
}
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
fn windows_sandbox_mode_from_effective_config(
|
||||
effective_config: &ConfigReadResponse,
|
||||
active_profile: Option<&str>,
|
||||
) -> Option<codex_config::types::WindowsSandboxModeToml> {
|
||||
let profile_windows = active_profile
|
||||
.and_then(|profile| effective_config.config.profiles.get(profile))
|
||||
.and_then(|profile| profile.additional.get("windows"))
|
||||
.and_then(windows_toml_from_json);
|
||||
let root_windows = effective_config
|
||||
.config
|
||||
.additional
|
||||
.get("windows")
|
||||
.and_then(windows_toml_from_json);
|
||||
profile_windows
|
||||
.and_then(|windows| windows.sandbox)
|
||||
.or_else(|| root_windows.and_then(|windows| windows.sandbox))
|
||||
}
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
fn windows_toml_from_json(value: &serde_json::Value) -> Option<WindowsToml> {
|
||||
serde_json::from_value(value.clone()).ok()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -902,6 +1262,46 @@ terminal_resize_reflow_max_rows = 9000
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn overridden_disabled_guardian_does_not_apply_auto_review_companions() -> Result<()> {
|
||||
let mut app = make_test_app().await;
|
||||
let original_policy = app.config.permissions.approval_policy.value();
|
||||
let effective_config: ConfigReadResponse = serde_json::from_value(serde_json::json!({
|
||||
"config": {
|
||||
"approval_policy": AskForApproval::OnRequest,
|
||||
"approvals_reviewer": codex_app_server_protocol::ApprovalsReviewer::AutoReview,
|
||||
"sandbox_mode": AppServerSandboxMode::WorkspaceWrite,
|
||||
"features": {
|
||||
"guardian_approval": false,
|
||||
},
|
||||
},
|
||||
"origins": {},
|
||||
}))?;
|
||||
|
||||
app.sync_feature_state_from_effective_config(
|
||||
&effective_config,
|
||||
&[(Feature::GuardianApproval, /*enabled*/ true)],
|
||||
);
|
||||
|
||||
assert!(!app.config.features.enabled(Feature::GuardianApproval));
|
||||
assert!(
|
||||
!app.chat_widget
|
||||
.config_ref()
|
||||
.features
|
||||
.enabled(Feature::GuardianApproval)
|
||||
);
|
||||
assert_eq!(app.config.approvals_reviewer, ApprovalsReviewer::User);
|
||||
assert_eq!(
|
||||
app.chat_widget.config_ref().approvals_reviewer,
|
||||
ApprovalsReviewer::User
|
||||
);
|
||||
assert_eq!(
|
||||
app.config.permissions.approval_policy.value(),
|
||||
original_policy
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn rebuild_config_for_resume_or_fallback_uses_current_config_on_same_cwd_error()
|
||||
-> Result<()> {
|
||||
|
||||
@@ -1160,16 +1160,21 @@ impl App {
|
||||
}
|
||||
let profile = self.active_profile.as_deref();
|
||||
let elevated_enabled = matches!(mode, WindowsSandboxEnableMode::Elevated);
|
||||
let builder = ConfigEditsBuilder::for_config(&self.config)
|
||||
.with_profile(profile)
|
||||
.set_windows_sandbox_mode(if elevated_enabled {
|
||||
"elevated"
|
||||
} else {
|
||||
"unelevated"
|
||||
})
|
||||
.clear_legacy_windows_sandbox_keys();
|
||||
match builder.apply().await {
|
||||
Ok(()) => {
|
||||
let edits = crate::config_update::build_windows_sandbox_mode_edits(
|
||||
profile,
|
||||
elevated_enabled,
|
||||
);
|
||||
match crate::config_update::write_config_batch(
|
||||
app_server.request_handle(),
|
||||
edits,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(response) if response.status == WriteStatus::OkOverridden => {
|
||||
self.sync_windows_sandbox_after_overridden_write(app_server, &response)
|
||||
.await;
|
||||
}
|
||||
Ok(_) => {
|
||||
if elevated_enabled {
|
||||
self.config.set_windows_sandbox_enabled(/*value*/ false);
|
||||
self.config
|
||||
@@ -1305,7 +1310,7 @@ impl App {
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(()) => {
|
||||
Ok(_) => {
|
||||
let effort_label = effort
|
||||
.map(|selected_effort| selected_effort.to_string())
|
||||
.unwrap_or_else(|| "default".to_string());
|
||||
@@ -1386,7 +1391,7 @@ impl App {
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(()) => {
|
||||
Ok(_) => {
|
||||
let label = Self::personality_label(personality);
|
||||
let mut message = format!("Personality set to {label}");
|
||||
if let Some(profile) = profile {
|
||||
@@ -1426,7 +1431,7 @@ impl App {
|
||||
match crate::config_update::write_config_batch(app_server.request_handle(), edits)
|
||||
.await
|
||||
{
|
||||
Ok(()) => {
|
||||
Ok(_) => {
|
||||
let mut message = if let Some(service_tier) = service_tier {
|
||||
format!("Service tier set to {service_tier}")
|
||||
} else {
|
||||
@@ -1620,7 +1625,7 @@ impl App {
|
||||
}
|
||||
}
|
||||
AppEvent::UpdateFeatureFlags { updates } => {
|
||||
self.update_feature_flags(updates).await;
|
||||
self.update_feature_flags(app_server, updates).await;
|
||||
}
|
||||
AppEvent::UpdateMemorySettings {
|
||||
use_memories,
|
||||
|
||||
@@ -1719,8 +1719,9 @@ async fn update_feature_flags_enabling_guardian_selects_auto_review() -> Result<
|
||||
let codex_home = tempdir()?;
|
||||
app.config.codex_home = codex_home.path().to_path_buf().abs();
|
||||
let auto_review = auto_review_mode();
|
||||
let mut app_server = start_config_write_test_app_server(&app).await?;
|
||||
|
||||
app.update_feature_flags(vec![(Feature::GuardianApproval, true)])
|
||||
app.update_feature_flags(&mut app_server, vec![(Feature::GuardianApproval, true)])
|
||||
.await;
|
||||
|
||||
assert!(app.config.features.enabled(Feature::GuardianApproval));
|
||||
@@ -1809,6 +1810,7 @@ async fn update_feature_flags_enabling_guardian_selects_auto_review() -> Result<
|
||||
assert!(config.contains("approvals_reviewer = \"guardian_subagent\""));
|
||||
assert!(config.contains("approval_policy = \"on-request\""));
|
||||
assert!(config.contains("sandbox_mode = \"workspace-write\""));
|
||||
app_server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -1847,8 +1849,9 @@ async fn update_feature_flags_disabling_guardian_clears_review_policy_and_restor
|
||||
.set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::legacy(
|
||||
PermissionProfile::workspace_write(),
|
||||
))?;
|
||||
let mut app_server = start_config_write_test_app_server(&app).await?;
|
||||
|
||||
app.update_feature_flags(vec![(Feature::GuardianApproval, false)])
|
||||
app.update_feature_flags(&mut app_server, vec![(Feature::GuardianApproval, false)])
|
||||
.await;
|
||||
|
||||
assert!(!app.config.features.enabled(Feature::GuardianApproval));
|
||||
@@ -1902,6 +1905,7 @@ async fn update_feature_flags_disabling_guardian_clears_review_policy_and_restor
|
||||
assert!(!config.contains("approvals_reviewer ="));
|
||||
assert!(config.contains("approval_policy = \"on-request\""));
|
||||
assert!(config.contains("sandbox_mode = \"workspace-write\""));
|
||||
app_server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -1923,8 +1927,9 @@ async fn update_feature_flags_enabling_guardian_overrides_explicit_manual_review
|
||||
app.config.approvals_reviewer = ApprovalsReviewer::User;
|
||||
app.chat_widget
|
||||
.set_approvals_reviewer(ApprovalsReviewer::User);
|
||||
let mut app_server = start_config_write_test_app_server(&app).await?;
|
||||
|
||||
app.update_feature_flags(vec![(Feature::GuardianApproval, true)])
|
||||
app.update_feature_flags(&mut app_server, vec![(Feature::GuardianApproval, true)])
|
||||
.await;
|
||||
|
||||
assert!(app.config.features.enabled(Feature::GuardianApproval));
|
||||
@@ -1970,6 +1975,7 @@ async fn update_feature_flags_enabling_guardian_overrides_explicit_manual_review
|
||||
assert!(config.contains("guardian_approval = true"));
|
||||
assert!(config.contains("approval_policy = \"on-request\""));
|
||||
assert!(config.contains("sandbox_mode = \"workspace-write\""));
|
||||
app_server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -1995,8 +2001,9 @@ async fn update_feature_flags_disabling_guardian_clears_manual_review_policy_wit
|
||||
app.config.approvals_reviewer = ApprovalsReviewer::User;
|
||||
app.chat_widget
|
||||
.set_approvals_reviewer(ApprovalsReviewer::User);
|
||||
let mut app_server = start_config_write_test_app_server(&app).await?;
|
||||
|
||||
app.update_feature_flags(vec![(Feature::GuardianApproval, false)])
|
||||
app.update_feature_flags(&mut app_server, vec![(Feature::GuardianApproval, false)])
|
||||
.await;
|
||||
|
||||
assert!(!app.config.features.enabled(Feature::GuardianApproval));
|
||||
@@ -2030,6 +2037,7 @@ async fn update_feature_flags_disabling_guardian_clears_manual_review_policy_wit
|
||||
let config = std::fs::read_to_string(codex_home.path().join("config.toml"))?;
|
||||
assert!(!config.contains("guardian_approval = true"));
|
||||
assert!(!config.contains("approvals_reviewer ="));
|
||||
app_server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -2052,8 +2060,9 @@ async fn update_feature_flags_enabling_guardian_in_profile_sets_profile_auto_rev
|
||||
app.config.approvals_reviewer = ApprovalsReviewer::User;
|
||||
app.chat_widget
|
||||
.set_approvals_reviewer(ApprovalsReviewer::User);
|
||||
let mut app_server = start_config_write_test_app_server(&app).await?;
|
||||
|
||||
app.update_feature_flags(vec![(Feature::GuardianApproval, true)])
|
||||
app.update_feature_flags(&mut app_server, vec![(Feature::GuardianApproval, true)])
|
||||
.await;
|
||||
|
||||
assert!(app.config.features.enabled(Feature::GuardianApproval));
|
||||
@@ -2102,6 +2111,7 @@ async fn update_feature_flags_enabling_guardian_in_profile_sets_profile_auto_rev
|
||||
profile_config.get("approvals_reviewer"),
|
||||
Some(&TomlValue::String("guardian_subagent".to_string()))
|
||||
);
|
||||
app_server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -2137,8 +2147,9 @@ guardian_approval = true
|
||||
app.config.approvals_reviewer = ApprovalsReviewer::AutoReview;
|
||||
app.chat_widget
|
||||
.set_approvals_reviewer(ApprovalsReviewer::AutoReview);
|
||||
let mut app_server = start_config_write_test_app_server(&app).await?;
|
||||
|
||||
app.update_feature_flags(vec![(Feature::GuardianApproval, false)])
|
||||
app.update_feature_flags(&mut app_server, vec![(Feature::GuardianApproval, false)])
|
||||
.await;
|
||||
|
||||
assert!(!app.config.features.enabled(Feature::GuardianApproval));
|
||||
@@ -2191,6 +2202,7 @@ guardian_approval = true
|
||||
.and_then(|table| table.get("approvals_reviewer")),
|
||||
Some(&TomlValue::String("user".to_string()))
|
||||
);
|
||||
app_server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -2217,8 +2229,9 @@ async fn update_feature_flags_disabling_guardian_in_profile_keeps_inherited_non_
|
||||
app.config.approvals_reviewer = ApprovalsReviewer::AutoReview;
|
||||
app.chat_widget
|
||||
.set_approvals_reviewer(ApprovalsReviewer::AutoReview);
|
||||
let mut app_server = start_config_write_test_app_server(&app).await?;
|
||||
|
||||
app.update_feature_flags(vec![(Feature::GuardianApproval, false)])
|
||||
app.update_feature_flags(&mut app_server, vec![(Feature::GuardianApproval, false)])
|
||||
.await;
|
||||
|
||||
assert!(app.config.features.enabled(Feature::GuardianApproval));
|
||||
@@ -2257,6 +2270,7 @@ async fn update_feature_flags_disabling_guardian_in_profile_keeps_inherited_non_
|
||||
.and_then(|table| table.get("approvals_reviewer")),
|
||||
Some(&TomlValue::String("guardian_subagent".to_string()))
|
||||
);
|
||||
app_server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -5758,3 +5772,6 @@ async fn side_backtrack_rejection_reports_unavailable_message_snapshot() {
|
||||
rendered
|
||||
);
|
||||
}
|
||||
async fn start_config_write_test_app_server(app: &App) -> Result<AppServerSession> {
|
||||
Box::pin(crate::start_embedded_app_server_for_picker(&app.config)).await
|
||||
}
|
||||
|
||||
@@ -8,11 +8,14 @@ use codex_app_server_client::AppServerRequestHandle;
|
||||
use codex_app_server_protocol::ClientRequest;
|
||||
use codex_app_server_protocol::ConfigBatchWriteParams;
|
||||
use codex_app_server_protocol::ConfigEdit;
|
||||
use codex_app_server_protocol::ConfigReadParams;
|
||||
use codex_app_server_protocol::ConfigReadResponse;
|
||||
use codex_app_server_protocol::ConfigWriteResponse;
|
||||
use codex_app_server_protocol::MergeStrategy;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::SkillsConfigWriteParams;
|
||||
use codex_app_server_protocol::SkillsConfigWriteResponse;
|
||||
use codex_features::FEATURES;
|
||||
use codex_protocol::config_types::SERVICE_TIER_DEFAULT_REQUEST_VALUE;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use color_eyre::eyre::Result;
|
||||
@@ -94,12 +97,64 @@ pub(crate) fn build_service_tier_selection_edits(
|
||||
vec![service_tier_edit]
|
||||
}
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
pub(crate) fn build_windows_sandbox_mode_edits(
|
||||
profile: Option<&str>,
|
||||
elevated_enabled: bool,
|
||||
) -> Vec<ConfigEdit> {
|
||||
let feature_key_path =
|
||||
|feature: &str| profile_scoped_key_path(profile, &format!("features.{feature}"));
|
||||
vec![
|
||||
replace_config_value(
|
||||
profile_scoped_key_path(profile, "windows.sandbox"),
|
||||
serde_json::json!(if elevated_enabled {
|
||||
"elevated"
|
||||
} else {
|
||||
"unelevated"
|
||||
}),
|
||||
),
|
||||
clear_config_value(feature_key_path("experimental_windows_sandbox")),
|
||||
clear_config_value(feature_key_path("elevated_windows_sandbox")),
|
||||
clear_config_value(feature_key_path("enable_experimental_windows_sandbox")),
|
||||
]
|
||||
}
|
||||
|
||||
pub(crate) fn build_feature_enabled_edit(
|
||||
profile: Option<&str>,
|
||||
feature_key: &str,
|
||||
enabled: bool,
|
||||
) -> ConfigEdit {
|
||||
let key_path = profile_scoped_key_path(profile, &format!("features.{feature_key}"));
|
||||
let is_default_false_feature = FEATURES
|
||||
.iter()
|
||||
.find(|spec| spec.key == feature_key)
|
||||
.is_some_and(|spec| !spec.default_enabled);
|
||||
if enabled || profile.is_some() || !is_default_false_feature {
|
||||
replace_config_value(key_path, serde_json::json!(enabled))
|
||||
} else {
|
||||
clear_config_value(key_path)
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn build_memory_settings_edits(
|
||||
use_memories: bool,
|
||||
generate_memories: bool,
|
||||
) -> Vec<ConfigEdit> {
|
||||
vec![
|
||||
replace_config_value("memories.use_memories", serde_json::json!(use_memories)),
|
||||
replace_config_value(
|
||||
"memories.generate_memories",
|
||||
serde_json::json!(generate_memories),
|
||||
),
|
||||
]
|
||||
}
|
||||
|
||||
pub(crate) async fn write_config_batch(
|
||||
request_handle: AppServerRequestHandle,
|
||||
edits: Vec<ConfigEdit>,
|
||||
) -> Result<()> {
|
||||
) -> Result<ConfigWriteResponse> {
|
||||
let request_id = RequestId::String(format!("tui-config-write-{}", Uuid::new_v4()));
|
||||
let _: ConfigWriteResponse = request_handle
|
||||
request_handle
|
||||
.request_typed(ClientRequest::ConfigBatchWrite {
|
||||
request_id,
|
||||
params: ConfigBatchWriteParams {
|
||||
@@ -110,8 +165,24 @@ pub(crate) async fn write_config_batch(
|
||||
},
|
||||
})
|
||||
.await
|
||||
.wrap_err("config/batchWrite failed in TUI")?;
|
||||
Ok(())
|
||||
.wrap_err("config/batchWrite failed in TUI")
|
||||
}
|
||||
|
||||
pub(crate) async fn read_effective_config(
|
||||
request_handle: AppServerRequestHandle,
|
||||
cwd: String,
|
||||
) -> Result<ConfigReadResponse> {
|
||||
let request_id = RequestId::String(format!("tui-config-read-{}", Uuid::new_v4()));
|
||||
request_handle
|
||||
.request_typed(ClientRequest::ConfigRead {
|
||||
request_id,
|
||||
params: ConfigReadParams {
|
||||
include_layers: false,
|
||||
cwd: Some(cwd),
|
||||
},
|
||||
})
|
||||
.await
|
||||
.wrap_err("config/read failed in TUI")
|
||||
}
|
||||
|
||||
pub(crate) async fn write_skill_enabled(
|
||||
|
||||
Reference in New Issue
Block a user