Preserve pending Effects toggles across save refreshes (#53496)

## Why

Saving an Effects preference refreshes the picker. When users toggle repeatedly before saves finish, that refresh can discard newer choices and cause subsequent toggles to use stale values.

## What changed

- Update Effects checkboxes immediately and track pending saves per row, preserving newer choices when an earlier save takes effect.
- Preserve pending Composer stars choices while Animations disables the row, and clear pending state on a regular panel refresh.
- Use the shared list toggle handling for Effects, including Space support, while giving configured key bindings precedence over Space toggling.

## Testing

Add regression tests for repeated toggles across save refreshes, pending Composer stars choices while disabled, clearing pending state, and a Space cancel binding taking precedence over toggling. Update the Effects picker snapshot.

GitOrigin-RevId: 847dcc2dfd40448c7adc56c8c40879db28b1fc39

codex-rs/tui/src/app/config_panel.rs

@@ -102,8 +102,13 @@ impl App {
         } else {
             None
         };
-        self.chat_widget
-            .refresh_config_panel(notice.map(str::to_string));
+        let notice = notice.map(str::to_string);
+        if effective {
+            self.chat_widget
+                .refresh_config_panel_after_effective_save(notice, preference);
+        } else {
+            self.chat_widget.refresh_config_panel(notice);
+        }
         tui.frame_requester().schedule_frame();
     }
 }

codex-rs/tui/src/app/config_panel_tests.rs

@@ -11,6 +11,26 @@ use crate::config_panel::ConfigEditorHandoff;
 use crate::local_settings::LocalSettings;
 use codex_protocol::config_types::TrustLevel;
 use pretty_assertions::assert_eq;
+use tokio::sync::mpsc::UnboundedReceiver;
+
+fn open_effects_editor(app: &mut App) {
+    let handoff = ConfigEditorHandoff::new(
+        ConfigEditor::Effects,
+        crate::key_hint::plain(KeyCode::Enter),
+    );
+    handoff.mark_activation_inactive();
+    app.chat_widget.open_config_editor(
+        handoff,
+        crate::config_panel::ResumeCurrentAvailability::Available,
+    );
+}
+
+fn next_config_preference(rx: &mut UnboundedReceiver<AppEvent>) -> Result<ConfigPreference> {
+    let AppEvent::SaveConfigPreference(preference) = rx.try_recv()? else {
+        panic!("expected config preference save");
+    };
+    Ok(preference)
+}
 
 #[tokio::test]
 async fn config_toggle_save_uses_host_config_and_applies_live_while_offline() -> Result<()> {
@@ -124,32 +144,120 @@ async fn config_toggle_save_uses_host_config_and_applies_live_while_offline() ->
 }
 
 #[tokio::test]
-async fn effects_editor_refreshes_after_animation_save() -> Result<()> {
+async fn effects_editor_refresh_preserves_pending_toggles() -> Result<()> {
     let (mut app, mut rx, _ops) = crate::app::tests::make_test_app_with_channels().await;
     let mut tui = crate::tui::test_support::make_test_tui()?;
     app.local_settings.tui.animations = false;
     app.chat_widget.local_settings.tui.animations = false;
     app.chat_widget.config_panel_tui.animations = false;
-    let handoff = ConfigEditorHandoff::new(
-        ConfigEditor::Effects,
-        crate::key_hint::plain(KeyCode::Enter),
-    );
-    handoff.mark_activation_inactive();
-    app.chat_widget.open_config_editor(
-        handoff,
-        crate::config_panel::ResumeCurrentAvailability::Available,
-    );
+    open_effects_editor(&mut app);
 
     app.chat_widget.handle_key_event(KeyCode::Enter.into());
-    let AppEvent::SaveConfigPreference(preference) = rx.try_recv()? else {
-        panic!("expected animation save");
-    };
-    app.save_config_preference(&mut tui, preference).await;
+    app.chat_widget.handle_key_event(KeyCode::Enter.into());
+    let first_preference = next_config_preference(&mut rx)?;
+    app.save_config_preference(&mut tui, first_preference).await;
 
     insta::assert_snapshot!(
         "effects_picker_after_animation_save",
         render_bottom_popup(&app.chat_widget, /*width*/ 80)
     );
+
+    app.chat_widget.handle_key_event(KeyCode::Enter.into());
+    let second_preference = next_config_preference(&mut rx)?;
+    app.save_config_preference(&mut tui, second_preference)
+        .await;
+    let third_preference = next_config_preference(&mut rx)?;
+    assert_eq!(
+        third_preference,
+        ConfigPreference::Toggle {
+            setting: ConfigToggle::Animations,
+            enabled: true,
+        }
+    );
+    app.save_config_preference(&mut tui, third_preference).await;
+    assert!(app.chat_widget.config_panel_tui.animations);
+    Ok(())
+}
+
+#[tokio::test]
+async fn effects_editor_preserves_pending_stars_while_disabled() -> Result<()> {
+    let (mut app, mut rx, _ops) = crate::app::tests::make_test_app_with_channels().await;
+    let mut tui = crate::tui::test_support::make_test_tui()?;
+    app.local_settings.tui.animations = true;
+    app.local_settings.tui.effects.starfield = true;
+    app.chat_widget.local_settings.tui.animations = true;
+    app.chat_widget.local_settings.tui.effects.starfield = true;
+    app.chat_widget.config_panel_tui.animations = true;
+    app.chat_widget.config_panel_tui.effects.starfield = true;
+    open_effects_editor(&mut app);
+
+    for shortcut in ['1', '2', '2'] {
+        app.chat_widget
+            .handle_key_event(KeyCode::Char(shortcut).into());
+    }
+    let animations_off = next_config_preference(&mut rx)?;
+    assert_eq!(
+        animations_off,
+        ConfigPreference::Toggle {
+            setting: ConfigToggle::Animations,
+            enabled: false,
+        }
+    );
+    app.save_config_preference(&mut tui, animations_off).await;
+
+    app.chat_widget.handle_key_event(KeyCode::Char('1').into());
+    for _ in 0..3 {
+        let preference = next_config_preference(&mut rx)?;
+        app.save_config_preference(&mut tui, preference).await;
+    }
+
+    assert_eq!(
+        (
+            app.chat_widget.config_panel_tui.animations,
+            app.chat_widget.config_panel_tui.effects.starfield,
+        ),
+        (true, true),
+    );
+    app.chat_widget.handle_key_event(KeyCode::Char('2').into());
+    assert_eq!(
+        next_config_preference(&mut rx)?,
+        ConfigPreference::Toggle {
+            setting: ConfigToggle::ComposerStars,
+            enabled: false,
+        }
+    );
+    Ok(())
+}
+
+#[tokio::test]
+async fn effects_editor_reset_clears_pending_toggle_state() -> Result<()> {
+    let (mut app, mut rx, _ops) = crate::app::tests::make_test_app_with_channels().await;
+    let mut tui = crate::tui::test_support::make_test_tui()?;
+    app.local_settings.tui.animations = true;
+    app.local_settings.tui.effects.starfield = true;
+    app.chat_widget.local_settings.tui.animations = true;
+    app.chat_widget.local_settings.tui.effects.starfield = true;
+    app.chat_widget.config_panel_tui.animations = true;
+    app.chat_widget.config_panel_tui.effects.starfield = true;
+    open_effects_editor(&mut app);
+
+    app.chat_widget.handle_key_event(KeyCode::Char('1').into());
+    app.chat_widget.handle_key_event(KeyCode::Char('2').into());
+    let _animations_off = next_config_preference(&mut rx)?;
+    let stars_off = next_config_preference(&mut rx)?;
+
+    app.chat_widget.refresh_config_panel(/*notice*/ None);
+    app.save_config_preference(&mut tui, stars_off).await;
+
+    assert!(!app.chat_widget.config_panel_tui.effects.starfield);
+    app.chat_widget.handle_key_event(KeyCode::Char('2').into());
+    assert_eq!(
+        next_config_preference(&mut rx)?,
+        ConfigPreference::Toggle {
+            setting: ConfigToggle::ComposerStars,
+            enabled: true,
+        }
+    );
     Ok(())
 }
 

codex-rs/tui/src/app/snapshots/codex_tui__app__config_panel__tests__effects_picker_after_animation_save.snap

@@ -6,8 +6,8 @@ expression: "render_bottom_popup(&app.chat_widget, 80)"
   Configure visual effects
 
 
-› 1. [x] Animations      Animate the welcome screen, working indicator, and
-                         transitions
-  2. [x] Composer stars  Show stars briefly for new Astra tasks
+› 1. [ ] Animations                 Animate the welcome screen, working
+                                    indicator, and transitions
+     [-] Composer stars (disabled)  Turn on Animations to show Astra task stars
 
   enter toggle · esc back

codex-rs/tui/src/bottom_pane/bottom_pane_view.rs

@@ -107,6 +107,14 @@ pub(crate) trait BottomPaneView: Renderable {
         None
     }
 
+    /// Current toggle values for list items that preserve local choices across refreshes.
+    fn selection_toggle_states(&self) -> Vec<Option<super::SelectionToggleState>> {
+        Vec::new()
+    }
+
+    /// Mark one previously emitted toggle action as complete.
+    fn complete_selection_toggle(&mut self, _index: usize) {}
+
     /// Current query for searchable list views that preserve filtering across refreshes.
     fn search_query(&self) -> Option<&str> {
         None

codex-rs/tui/src/bottom_pane/list_selection_view.rs

@@ -142,6 +142,13 @@ pub(crate) struct SelectionToggle {
     pub action: Box<SelectionToggleAction>,
 }
 
+/// A displayed toggle value with the number of its save actions still in flight.
+#[derive(Clone, Copy)]
+pub(crate) struct SelectionToggleState {
+    pub is_on: bool,
+    pub pending_actions: usize,
+}
+
 /// Callback invoked whenever the highlighted item changes (arrow keys, search
 /// filter, number-key jump).  Receives the *actual* index into the unfiltered
 /// `items` list and the event sender.  Used by the theme picker for live preview.
@@ -165,6 +172,10 @@ pub(crate) struct SelectionItem {
     pub child_label: Option<char>,
     pub name_prefix_spans: Vec<Span<'static>>,
     pub toggle: Option<SelectionToggle>,
+    /// Render `[x]` and treat the primary accept key as a toggle action.
+    pub checkbox_toggle: bool,
+    /// Toggle actions sent from this row that have not finished saving yet.
+    pub pending_toggle_actions: usize,
     pub toggle_placeholder: Option<&'static str>,
     pub display_shortcut: Option<ShortcutHint>,
     /// Required row metadata retained when the secondary description hides.
@@ -378,23 +389,29 @@ pub(crate) struct ListSelectionView {
 }
 
 const SELECTION_TOGGLE_ON_PREFIX: &str = "[*] ";
+const SELECTION_CHECKBOX_ON_PREFIX: &str = "[x] ";
 const SELECTION_TOGGLE_OFF_PREFIX: &str = "[ ] ";
 pub(crate) const SELECTION_TOGGLE_UNAVAILABLE_PREFIX: &str = "[-] ";
 pub(crate) const SELECTION_TOGGLE_BLOCKED_PREFIX: &str = "[!] ";
 
-fn selection_toggle_prefix(toggle: &SelectionToggle) -> &'static str {
+fn selection_toggle_prefix(item: &SelectionItem, toggle: &SelectionToggle) -> &'static str {
     if toggle.is_on {
-        SELECTION_TOGGLE_ON_PREFIX
+        if item.checkbox_toggle {
+            SELECTION_CHECKBOX_ON_PREFIX
+        } else {
+            SELECTION_TOGGLE_ON_PREFIX
+        }
     } else {
         SELECTION_TOGGLE_OFF_PREFIX
     }
 }
 
 fn selection_item_toggle_prefix(item: &SelectionItem) -> Option<&'static str> {
-    item.toggle
-        .as_ref()
-        .map(selection_toggle_prefix)
-        .or(item.toggle_placeholder)
+    item.toggle_placeholder.or_else(|| {
+        item.toggle
+            .as_ref()
+            .map(|toggle| selection_toggle_prefix(item, toggle))
+    })
 }
 
 impl ListSelectionView {
@@ -430,6 +447,7 @@ impl ListSelectionView {
         };
 
         toggle.is_on = !toggle.is_on;
+        item.pending_toggle_actions = item.pending_toggle_actions.saturating_add(/*rhs*/ 1);
         (toggle.action)(toggle.is_on, &app_event_tx);
     }
 }
@@ -941,6 +959,15 @@ impl ListSelectionView {
 
     fn accept(&mut self, kind: SelectionActionKind) {
         let selected_actual_idx = self.selected_actual_idx();
+        if matches!(kind, SelectionActionKind::Primary)
+            && selected_actual_idx
+                .and_then(|idx| self.active_items().get(idx))
+                .is_some_and(|item| item.checkbox_toggle)
+        {
+            self.toggle_selected();
+            self.last_selected_actual_idx = selected_actual_idx;
+            return;
+        }
         if let Some(item) = selected_actual_idx.and_then(|idx| self.active_items().get(idx)) {
             if item.is_disabled || item.disabled_reason.is_some() {
                 return;
@@ -1317,6 +1344,7 @@ impl BottomPaneView for ListSelectionView {
                 modifiers: KeyModifiers::NONE,
                 ..
             } if self.selected_item_has_toggle()
+                && self.keymap.action_for(key_event).is_none()
                 && (!search_field_visible
                     || matches!(self.search_mode, SearchMode::Immediate)
                         && self.search_query.is_empty()) =>
@@ -1438,6 +1466,28 @@ impl BottomPaneView for ListSelectionView {
         self.selected_actual_idx()
     }
 
+    fn selection_toggle_states(&self) -> Vec<Option<SelectionToggleState>> {
+        self.active_items()
+            .iter()
+            .map(|item| {
+                item.toggle
+                    .as_ref()
+                    .filter(|_| item.pending_toggle_actions > 0)
+                    .map(|toggle| SelectionToggleState {
+                        is_on: toggle.is_on,
+                        pending_actions: item.pending_toggle_actions,
+                    })
+            })
+            .collect()
+    }
+
+    fn complete_selection_toggle(&mut self, index: usize) {
+        if let Some(item) = self.active_items_mut().get_mut(index) {
+            item.pending_toggle_actions =
+                item.pending_toggle_actions.saturating_sub(/*rhs*/ 1);
+        }
+    }
+
     fn search_query(&self) -> Option<&str> {
         self.is_searchable.then_some(self.search_query.as_str())
     }
@@ -2530,6 +2580,43 @@ mod tests {
         );
     }
 
+    #[test]
+    fn configured_cancel_binding_takes_precedence_over_space_toggle() {
+        let (tx_raw, mut rx) = unbounded_channel::<AppEvent>();
+        let tx = AppEventSender::new(tx_raw);
+        let mut keymap = crate::keymap::RuntimeKeymap::defaults().list;
+        keymap.cancel = vec![crate::key_hint::plain(KeyCode::Char(' '))];
+        let mut view = ListSelectionView::new(
+            SelectionViewParams {
+                items: vec![SelectionItem {
+                    name: "Animations".to_string(),
+                    toggle: Some(SelectionToggle {
+                        is_on: true,
+                        action: Box::new(|_enabled, tx: &_| {
+                            tx.send(AppEvent::OpenApprovalsPopup);
+                        }),
+                    }),
+                    checkbox_toggle: true,
+                    ..Default::default()
+                }],
+                ..Default::default()
+            },
+            tx,
+            keymap,
+        );
+
+        view.handle_key_event(KeyEvent::new(KeyCode::Char(' '), KeyModifiers::NONE));
+
+        assert_eq!(view.completion, Some(ViewCompletion::Cancelled));
+        assert!(
+            view.active_items()[0]
+                .toggle
+                .as_ref()
+                .is_some_and(|toggle| toggle.is_on)
+        );
+        assert!(rx.try_recv().is_err());
+    }
+
     #[test]
     fn single_line_row_display_truncates_instead_of_wrapping() {
         let (tx_raw, _rx) = unbounded_channel::<AppEvent>();

codex-rs/tui/src/bottom_pane/mod.rs

@@ -177,6 +177,7 @@ pub(crate) use list_selection_view::SearchMode;
 pub(crate) use list_selection_view::SelectionDescriptionLayout;
 pub(crate) use list_selection_view::SelectionRowDisplay;
 pub(crate) use list_selection_view::SelectionToggle;
+pub(crate) use list_selection_view::SelectionToggleState;
 pub(crate) use list_selection_view::SelectionViewParams;
 pub(crate) use list_selection_view::SideContentWidth;
 pub(crate) use list_selection_view::popup_content_width;
@@ -1626,6 +1627,30 @@ impl BottomPane {
             .and_then(|view| view.selected_index())
     }
 
+    pub(crate) fn selection_toggle_states_for_active_view(
+        &self,
+        view_id: &'static str,
+    ) -> Option<Vec<Option<SelectionToggleState>>> {
+        self.view_stack
+            .last()
+            .filter(|view| view.view_id() == Some(view_id))
+            .map(|view| view.selection_toggle_states())
+    }
+
+    pub(crate) fn complete_selection_toggle_for_active_view(
+        &mut self,
+        view_id: &'static str,
+        index: usize,
+    ) {
+        if let Some(view) = self
+            .view_stack
+            .last_mut()
+            .filter(|view| view.view_id() == Some(view_id))
+        {
+            view.complete_selection_toggle(index);
+        }
+    }
+
     pub(crate) fn selected_index_for_present_view(&self, view_id: &'static str) -> Option<usize> {
         self.view_stack
             .iter()

codex-rs/tui/src/chatwidget/config_panel.rs

@@ -119,6 +119,50 @@ impl ChatWidget {
     }
 
     pub(crate) fn refresh_config_panel(&mut self, notice: Option<String>) -> bool {
+        self.refresh_config_panel_with_effects_state(notice, Vec::new())
+    }
+
+    /// Refresh after an effective save without discarding newer Effects choices still queued.
+    pub(crate) fn refresh_config_panel_after_effective_save(
+        &mut self,
+        notice: Option<String>,
+        preference: ConfigPreference,
+    ) -> bool {
+        if let ConfigPreference::Toggle { setting, .. } = preference {
+            match setting {
+                ConfigToggle::Animations => {
+                    self.bottom_pane.complete_selection_toggle_for_active_view(
+                        crate::config_panel::EFFECTS_VIEW_ID,
+                        /*index*/ 0,
+                    );
+                }
+                ConfigToggle::ComposerStars => {
+                    self.bottom_pane.complete_selection_toggle_for_active_view(
+                        crate::config_panel::EFFECTS_VIEW_ID,
+                        /*index*/ 1,
+                    );
+                }
+                ConfigToggle::AutomaticRecaps
+                | ConfigToggle::CopyOnSelectClearSelection
+                | ConfigToggle::Notifications
+                | ConfigToggle::PasteDetection
+                | ConfigToggle::StartInVimMode
+                | ConfigToggle::StatusLineColors
+                | ConfigToggle::Tips => {}
+            }
+        }
+        let toggle_states = self
+            .bottom_pane
+            .selection_toggle_states_for_active_view(crate::config_panel::EFFECTS_VIEW_ID)
+            .unwrap_or_default();
+        self.refresh_config_panel_with_effects_state(notice, toggle_states)
+    }
+
+    fn refresh_config_panel_with_effects_state(
+        &mut self,
+        notice: Option<String>,
+        effects_toggle_states: Vec<Option<crate::bottom_pane::SelectionToggleState>>,
+    ) -> bool {
         let panel_updated = self.bottom_pane.update_config_panel(&ConfigPanelUpdate {
             tui: self.config_panel_tui.clone(),
             keymap: self.bottom_pane.list_keymap(),
@@ -132,7 +176,11 @@ impl ChatWidget {
             return panel_updated;
         };
         let keymap = self.bottom_pane.list_keymap();
-        let mut params = crate::config_panel::effects(&self.config_panel_tui, &keymap);
+        let mut params = crate::config_panel::effects_with_toggle_states(
+            &self.config_panel_tui,
+            &keymap,
+            &effects_toggle_states,
+        );
         params.initial_selected_idx = Some(selected_idx);
         params.subtitle = notice.or(params.subtitle);
         let effects_updated = self

codex-rs/tui/src/config_panel/mod.rs

@@ -9,6 +9,7 @@ mod toggle;
 pub(crate) use render::EFFECTS_VIEW_ID;
 pub(crate) use render::copy_on_select;
 pub(crate) use render::effects;
+pub(crate) use render::effects_with_toggle_states;
 pub(crate) use render::notification_condition;
 pub(crate) use render::resume_directory;
 pub(crate) use render::right_click_paste;

codex-rs/tui/src/config_panel/render.rs

@@ -19,6 +19,8 @@ use crate::bottom_pane::SelectionDescriptionLayout;
 use crate::bottom_pane::SelectionItem;
 use crate::bottom_pane::SelectionRowDisplay;
 use crate::bottom_pane::SelectionTab;
+use crate::bottom_pane::SelectionToggle;
+use crate::bottom_pane::SelectionToggleState;
 use crate::bottom_pane::SelectionViewParams;
 use crate::bottom_pane::footer_hint_items_line;
 use crate::bottom_pane::popup_consts::picker_hint_line_for_keymap;
@@ -34,13 +36,19 @@ use ratatui::layout::Rect;
 use ratatui::style::Stylize;
 use ratatui::text::Line;
 use ratatui::widgets::Paragraph;
-use std::sync::atomic::AtomicBool;
-use std::sync::atomic::Ordering;
 use unicode_width::UnicodeWidthStr;
 
 pub(crate) const EFFECTS_VIEW_ID: &str = "config-effects";
 
 pub(crate) fn effects(tui: &codex_config::types::Tui, keymap: &ListKeymap) -> SelectionViewParams {
+    effects_with_toggle_states(tui, keymap, &[])
+}
+
+pub(crate) fn effects_with_toggle_states(
+    tui: &codex_config::types::Tui,
+    keymap: &ListKeymap,
+    toggle_states: &[Option<SelectionToggleState>],
+) -> SelectionViewParams {
     let footer_hint = [(ListAction::Accept, "toggle"), (ListAction::Cancel, "back")]
         .into_iter()
         .filter_map(|(action, label)| {
@@ -61,6 +69,12 @@ pub(crate) fn effects(tui: &codex_config::types::Tui, keymap: &ListKeymap) -> Se
             ConfigToggle::ComposerStars,
         ),
     ];
+    let animations_enabled = toggle_states
+        .first()
+        .copied()
+        .flatten()
+        .map(|state| state.is_on)
+        .unwrap_or(tui.animations);
     SelectionViewParams {
         view_id: Some(EFFECTS_VIEW_ID),
         title: Some("Effects".to_string()),
@@ -69,28 +83,32 @@ pub(crate) fn effects(tui: &codex_config::types::Tui, keymap: &ListKeymap) -> Se
         allow_input_when_disconnected: true,
         items: settings
             .into_iter()
-            .map(|(label, description, setting)| {
-                let enabled = setting.enabled(tui);
-                let next_enabled = AtomicBool::new(!enabled);
-                let available = setting != ConfigToggle::ComposerStars || tui.animations;
+            .enumerate()
+            .map(|(index, (label, description, setting))| {
+                let toggle_state = toggle_states.get(index).copied().flatten();
+                let enabled = toggle_state
+                    .map(|state| state.is_on)
+                    .unwrap_or_else(|| setting.enabled(tui));
+                let available = setting != ConfigToggle::ComposerStars || animations_enabled;
                 SelectionItem {
                     name: label.to_string(),
-                    toggle_placeholder: Some(if !available {
-                        SELECTION_TOGGLE_UNAVAILABLE_PREFIX
-                    } else if enabled {
-                        "[x] "
-                    } else {
-                        "[ ] "
+                    toggle: Some(SelectionToggle {
+                        is_on: enabled,
+                        action: Box::new(move |enabled, tx| {
+                            tx.send(AppEvent::SaveConfigPreference(ConfigPreference::Toggle {
+                                setting,
+                                enabled,
+                            }));
+                        }),
                     }),
+                    pending_toggle_actions: toggle_state
+                        .map(|state| state.pending_actions)
+                        .unwrap_or_default(),
+                    checkbox_toggle: available,
+                    toggle_placeholder: (!available).then_some(SELECTION_TOGGLE_UNAVAILABLE_PREFIX),
                     description: available.then(|| description.to_string()),
                     disabled_reason: (!available)
                         .then(|| "Turn on Animations to show Astra task stars".to_string()),
-                    actions: vec![Box::new(move |tx| {
-                        tx.send(AppEvent::SaveConfigPreference(ConfigPreference::Toggle {
-                            setting,
-                            enabled: next_enabled.fetch_xor(/*val*/ true, Ordering::Relaxed),
-                        }));
-                    })],
                     dismiss_on_select: false,
                     ..Default::default()
                 }