From 146de98dfd66f3ab5cdfbcdde35d64a213972ed5 Mon Sep 17 00:00:00 2001 From: celia-oai Date: Tue, 9 Jun 2026 18:14:36 -0700 Subject: [PATCH] fix --- codex-rs/login/src/auth/storage.rs | 4 +- codex-rs/login/src/auth/storage_tests.rs | 10 ++-- codex-rs/rmcp-client/src/oauth.rs | 16 +++++-- codex-rs/secrets/src/lib.rs | 15 +++--- codex-rs/tui/src/app/config_persistence.rs | 53 ++-------------------- 5 files changed, 30 insertions(+), 68 deletions(-) diff --git a/codex-rs/login/src/auth/storage.rs b/codex-rs/login/src/auth/storage.rs index 96427a6159..ca313a0a34 100644 --- a/codex-rs/login/src/auth/storage.rs +++ b/codex-rs/login/src/auth/storage.rs @@ -30,6 +30,7 @@ use codex_protocol::account::PlanType as AccountPlanType; use codex_secrets::LocalSecretsNamespace; use codex_secrets::SecretName; use codex_secrets::SecretScope; +use codex_secrets::SecretsBackendKind; use codex_secrets::SecretsManager; use once_cell::sync::Lazy; @@ -278,8 +279,9 @@ impl SecretsKeyringAuthStorage { fn new(codex_home: PathBuf, keyring_store: Arc) -> Self { let direct_storage = DirectKeyringAuthStorage::new(codex_home.clone(), Arc::clone(&keyring_store)); - let secrets_manager = SecretsManager::new_local_with_keyring_store( + let secrets_manager = SecretsManager::new_with_keyring_store_and_namespace( codex_home.clone(), + SecretsBackendKind::Local, keyring_store, LocalSecretsNamespace::CliAuth, ); diff --git a/codex-rs/login/src/auth/storage_tests.rs b/codex-rs/login/src/auth/storage_tests.rs index 74fea21132..8175186d96 100644 --- a/codex-rs/login/src/auth/storage_tests.rs +++ b/codex-rs/login/src/auth/storage_tests.rs @@ -4,6 +4,7 @@ use anyhow::Context; use base64::Engine; use codex_secrets::LocalSecretsNamespace; use codex_secrets::SecretScope; +use codex_secrets::SecretsBackendKind; use codex_secrets::SecretsManager; use codex_secrets::compute_keyring_account; use pretty_assertions::assert_eq; @@ -199,8 +200,9 @@ fn seed_secrets_backend_and_fallback_auth_file_for_delete( codex_home: &Path, auth: &AuthDotJson, ) -> anyhow::Result { - let manager = SecretsManager::new_local_with_keyring_store( + let manager = SecretsManager::new_with_keyring_store_and_namespace( codex_home.to_path_buf(), + SecretsBackendKind::Local, Arc::new(mock_keyring.clone()), LocalSecretsNamespace::CliAuth, ); @@ -219,8 +221,9 @@ fn seed_secrets_backend_with_auth( codex_home: &Path, auth: &AuthDotJson, ) -> anyhow::Result<()> { - let manager = SecretsManager::new_local_with_keyring_store( + let manager = SecretsManager::new_with_keyring_store_and_namespace( codex_home.to_path_buf(), + SecretsBackendKind::Local, Arc::new(mock_keyring.clone()), LocalSecretsNamespace::CliAuth, ); @@ -237,8 +240,9 @@ fn assert_keyring_saved_auth_and_removed_fallback( codex_home: &Path, expected: &AuthDotJson, ) -> anyhow::Result<()> { - let manager = SecretsManager::new_local_with_keyring_store( + let manager = SecretsManager::new_with_keyring_store_and_namespace( codex_home.to_path_buf(), + SecretsBackendKind::Local, Arc::new(mock_keyring.clone()), LocalSecretsNamespace::CliAuth, ); diff --git a/codex-rs/rmcp-client/src/oauth.rs b/codex-rs/rmcp-client/src/oauth.rs index 60dfac9480..d0005d20a6 100644 --- a/codex-rs/rmcp-client/src/oauth.rs +++ b/codex-rs/rmcp-client/src/oauth.rs @@ -24,6 +24,7 @@ use codex_config::types::OAuthCredentialsStoreMode; use codex_secrets::LocalSecretsNamespace; use codex_secrets::SecretName; use codex_secrets::SecretScope; +use codex_secrets::SecretsBackendKind; use codex_secrets::SecretsManager; use oauth2::AccessToken; use oauth2::RefreshToken; @@ -220,8 +221,9 @@ fn load_oauth_tokens_from_secrets_keyring( url: &str, ) -> Result> { let codex_home = find_codex_home()?; - let manager = SecretsManager::new_local_with_keyring_store( + let manager = SecretsManager::new_with_keyring_store_and_namespace( codex_home.to_path_buf(), + SecretsBackendKind::Local, Arc::new(keyring_store.clone()), LocalSecretsNamespace::McpOAuth, ); @@ -313,8 +315,9 @@ fn save_oauth_tokens_to_secrets_keyring( ) -> Result<()> { let serialized = serde_json::to_string(tokens).context("failed to serialize OAuth tokens")?; let codex_home = find_codex_home()?; - let manager = SecretsManager::new_local_with_keyring_store( + let manager = SecretsManager::new_with_keyring_store_and_namespace( codex_home.to_path_buf(), + SecretsBackendKind::Local, Arc::new(keyring_store.clone()), LocalSecretsNamespace::McpOAuth, ); @@ -428,8 +431,9 @@ fn delete_oauth_tokens_from_secrets_keyring( url: &str, ) -> Result { let codex_home = find_codex_home()?; - let manager = SecretsManager::new_local_with_keyring_store( + let manager = SecretsManager::new_with_keyring_store_and_namespace( codex_home.to_path_buf(), + SecretsBackendKind::Local, Arc::new(keyring_store.clone()), LocalSecretsNamespace::McpOAuth, ); @@ -991,8 +995,9 @@ mod tests { &tokens, )?; - let manager = SecretsManager::new_local_with_keyring_store( + let manager = SecretsManager::new_with_keyring_store_and_namespace( env.path().to_path_buf(), + SecretsBackendKind::Local, Arc::new(store.clone()), LocalSecretsNamespace::McpOAuth, ); @@ -1102,8 +1107,9 @@ mod tests { &tokens.url, )?; - let manager = SecretsManager::new_local_with_keyring_store( + let manager = SecretsManager::new_with_keyring_store_and_namespace( env.path().to_path_buf(), + SecretsBackendKind::Local, Arc::new(store.clone()), LocalSecretsNamespace::McpOAuth, ); diff --git a/codex-rs/secrets/src/lib.rs b/codex-rs/secrets/src/lib.rs index 5ab952d024..c3ddc8a24d 100644 --- a/codex-rs/secrets/src/lib.rs +++ b/codex-rs/secrets/src/lib.rs @@ -110,11 +110,6 @@ impl SecretsManager { Self { backend } } - pub fn new_local(codex_home: PathBuf, namespace: LocalSecretsNamespace) -> Self { - let keyring_store: Arc = Arc::new(DefaultKeyringStore); - Self::new_local_with_keyring_store(codex_home, keyring_store, namespace) - } - pub fn new_with_keyring_store( codex_home: PathBuf, backend_kind: SecretsBackendKind, @@ -128,18 +123,20 @@ impl SecretsManager { Self { backend } } - pub fn new_local_with_keyring_store( + pub fn new_with_keyring_store_and_namespace( codex_home: PathBuf, + backend_kind: SecretsBackendKind, keyring_store: Arc, namespace: LocalSecretsNamespace, ) -> Self { - Self { - backend: Arc::new(LocalSecretsBackend::new_with_namespace( + let backend: Arc = match backend_kind { + SecretsBackendKind::Local => Arc::new(LocalSecretsBackend::new_with_namespace( codex_home, keyring_store, namespace, )), - } + }; + Self { backend } } pub fn set(&self, scope: &SecretScope, name: &SecretName, value: &str) -> Result<()> { diff --git a/codex-rs/tui/src/app/config_persistence.rs b/codex-rs/tui/src/app/config_persistence.rs index 107740f973..7d1f6047e0 100644 --- a/codex-rs/tui/src/app/config_persistence.rs +++ b/codex-rs/tui/src/app/config_persistence.rs @@ -497,6 +497,9 @@ impl App { 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, @@ -504,21 +507,6 @@ impl App { ) .await { - let guardian_approval_was_updated = feature_updates_to_apply - .iter() - .any(|(feature, _)| *feature == Feature::GuardianApproval); - let guardian_disable_applied = feature_updates_to_apply - .iter() - .any(|(feature, enabled)| *feature == Feature::GuardianApproval && !*enabled) - && !feature_enabled_from_effective_config( - &effective_config, - Feature::GuardianApproval, - ); - if !guardian_disable_applied { - self.chat_widget.add_error_message(format!( - "Experimental feature changes were saved but not applied: {message}" - )); - } self.sync_feature_state_from_effective_config( &effective_config, &feature_updates_to_apply, @@ -531,41 +519,6 @@ impl App { if windows_sandbox_changed { self.propagate_windows_sandbox_turn_context(); } - if guardian_approval_was_updated - && !self.config.features.enabled(Feature::GuardianApproval) - { - self.sync_active_thread_permission_settings_to_cached_session() - .await; - let op = AppCommand::override_turn_context( - /*cwd*/ None, - /*approval_policy*/ None, - Some(self.config.approvals_reviewer), - /*permission_profile*/ None, - /*active_permission_profile*/ None, - /*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; - } - } - if guardian_disable_applied && let Some(label) = permissions_history_label { - self.chat_widget - .add_info_message(format!("Permissions updated to {label}"), None); - } - } else { - self.chat_widget.add_error_message(format!( - "Experimental feature changes were saved but not applied: {message}" - )); } return; }