feat(tui): Show skillset picker on startup when skillset_per_session is enabled (#351)

## Summary
🤖 Generated with [Nori](https://www.npmjs.com/package/nori-ai)

- Remove the `.worktrees/` directory guard from the
`skillset_per_session` startup checks so the skillset picker always
appears when the setting is enabled, regardless of directory context
- Add `on_dismiss` callback to `SelectionViewParams` so dismissing the
picker without selection triggers deferred agent spawn (behaving as if
the feature is disabled)
- Add `SkillsetPickerDismissed` AppEvent variant to handle picker
dismissal gracefully

## Test Plan
- [x] 3 new tests: `on_dismiss_callback_fires_on_ctrl_c`,
`on_dismiss_callback_not_fired_on_accept`,
`test_skillset_picker_dismiss_sends_event`
- [x] All 918 existing tests pass with no regressions
- [x] Clippy passes clean
- [ ] Manual: Enable `skillset_per_session` in config, launch nori
outside a `.worktrees/` directory, verify picker appears
- [ ] Manual: Dismiss the picker (Escape), verify the agent starts
normally
- [ ] Manual: Select a skillset from the picker, verify the agent starts
with the skillset active

Share Nori with your team: https://www.npmjs.com/package/nori-ai

Co-authored-by: Nori <contact@tilework.tech>

Author: theahura

codex-rs/tui/docs.md

@@ -136,6 +136,8 @@ During background system info collection on unix, `check_worktree_cleanup()` run
 
 The `desc_col` is computed once per render pass from the widest visible name plus 2 columns of padding. The stacked fallback prevents descriptions from being squeezed into 1-2 characters of horizontal space on narrow terminals. Because both `render_rows()` and `measure_rows_height()` call the same `wrap_row()` function, layout and height calculation are always consistent.
 
+`SelectionViewParams` supports an optional `on_dismiss: Option<SelectionAction>` callback that fires when the picker is dismissed without selection (Escape or Ctrl-C). The callback is invoked in `ListSelectionView::on_ctrl_c()` before marking the view as complete. It does not fire when the user makes a selection via `accept()`. This is used by the skillset picker to send `SkillsetPickerDismissed` when the deferred agent spawn needs a fallback trigger.
+
 **Undo Snapshot Picker (`/undo`):**
 
 The `/undo` slash command sends `Op::UndoList` (not `Op::Undo`) to the ACP backend. When the backend responds with `UndoListResult`, the TUI opens a `ListSelectionView` modal (the same pattern used by the approvals popup, etc.) displaying all available snapshots. Each item shows `[short_id] truncated_label` where the label is truncated to 60 characters. Selecting a snapshot dispatches `Op::UndoTo { index }` to restore to that point. If no snapshots are available, an info message is displayed instead of the modal.
@@ -199,9 +201,9 @@ The `/switch-skillset` command integrates with the external `nori-skillsets` CLI
 
 The worktree context is detected by `handle_switch_skillset_command()`: if the cwd's parent directory is named `.worktrees`, the cwd is passed as `install_dir`. When `skillset_per_session` is enabled, the cwd is used as `install_dir` even when not in a worktree. This enables per-worktree or per-session skillset installation.
 
-When `skillset_per_session` is enabled in `NoriConfig`, the skillset picker is automatically triggered at startup in `App::run()`, regardless of whether the session is in a worktree.
+When `skillset_per_session` is enabled in `NoriConfig`, the skillset picker is automatically triggered at startup in `App::run()`, regardless of whether the session is in a worktree. The agent spawn is deferred (`ChatWidgetInit::deferred_spawn = true`) so that `nori-skillsets switch` can write `.claude/CLAUDE.md` to disk before the agent reads it. During the deferred period, a dummy channel is created in `constructors.rs` so the widget has a valid `op_tx`. The real agent spawns after the user picks a skillset (`SkillsetSwitchResult` triggers `spawn_deferred_agent()`). If the user dismisses the picker without selecting a skillset (Escape/Ctrl-C), the picker's `on_dismiss` callback sends `AppEvent::SkillsetPickerDismissed`, which also triggers `spawn_deferred_agent()` -- the agent starts without a skillset, behaving as if the feature were disabled. The `server_for_deferred_spawn` field on `App` holds the `ConversationManager` until one of these paths consumes it via `.take()`.
 
-Events: `AppEvent::SkillsetListResult` (carries `install_dir: Option<PathBuf>`), `AppEvent::InstallSkillset`, `AppEvent::SwitchSkillset`, `AppEvent::SkillsetInstallResult`, `AppEvent::SkillsetSwitchResult`, `AppEvent::OpenSkillsetPerSessionWorktreeChoice`
+Events: `AppEvent::SkillsetListResult` (carries `install_dir: Option<PathBuf>`), `AppEvent::InstallSkillset`, `AppEvent::SwitchSkillset`, `AppEvent::SkillsetInstallResult`, `AppEvent::SkillsetSwitchResult`, `AppEvent::SkillsetPickerDismissed`, `AppEvent::OpenSkillsetPerSessionWorktreeChoice`
 
 The "Per Session Skillsets" toggle in `/config` is built in `nori/config_picker.rs`. Toggling it on emits `AppEvent::OpenSkillsetPerSessionWorktreeChoice`, which opens a worktree choice modal (`skillset_worktree_choice_params()`) letting the user choose between "With Auto Worktrees" and "Without Auto Worktrees". The choice determines whether `auto_worktree` is also enabled. Toggling it off emits `AppEvent::SetConfigSkillsetPerSession`, handled in `app/config_persistence.rs` via `persist_skillset_per_session_setting()` to write `skillset_per_session` under `[tui]` in `config.toml`. The "Auto Worktree" toggle is always independently toggleable regardless of the `skillset_per_session` state.
 

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

@@ -822,6 +822,19 @@ impl App {
                     );
                 }
             }
+            AppEvent::SkillsetPickerDismissed => {
+                // The skillset picker was dismissed without selection. If the
+                // agent spawn was deferred, spawn it now without a skillset
+                // (behaves as if skillset_per_session is disabled).
+                #[cfg(feature = "nori-config")]
+                if let Some(server) = self.server_for_deferred_spawn.take() {
+                    self.chat_widget.spawn_deferred_agent(
+                        self.config.clone(),
+                        self.app_event_tx.clone(),
+                        server,
+                    );
+                }
+            }
             AppEvent::ExecuteScript { prompt, args } => {
                 let tx = self.app_event_tx.clone();
                 let nori_config = codex_acp::config::NoriConfig::load().unwrap_or_default();

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

@@ -302,14 +302,14 @@ impl App {
 
         let enhanced_keys_supported = tui.enhanced_keys_supported();
 
-        // When skillset_per_session is enabled and we're in a worktree, defer
-        // spawning the agent until after the user picks a skillset and the switch
-        // writes `.claude/CLAUDE.md` to disk.
+        // When skillset_per_session is enabled, defer spawning the agent until
+        // after the user picks a skillset and the switch writes
+        // `.claude/CLAUDE.md` to disk. If the user dismisses the picker, the
+        // agent spawns without a skillset.
         #[cfg(feature = "nori-config")]
         let needs_deferred_spawn = {
             let nori_cfg = codex_acp::config::NoriConfig::load().unwrap_or_default();
             nori_cfg.skillset_per_session
-                && crate::system_info::extract_worktree_name(&config.cwd).is_some()
         };
         #[cfg(not(feature = "nori-config"))]
         let needs_deferred_spawn = false;
@@ -428,15 +428,15 @@ impl App {
             );
         }
 
-        // If skillset_per_session is enabled and we're in a worktree, show the
-        // skillset picker. The agent spawn was deferred so that
-        // `nori-skillsets switch` can write `.claude/CLAUDE.md` before the agent
-        // reads it. Once the user picks a skillset and the switch completes,
-        // `event_handling.rs` triggers `spawn_deferred_agent()`.
+        // If skillset_per_session is enabled, show the skillset picker. The
+        // agent spawn was deferred so that `nori-skillsets switch` can write
+        // `.claude/CLAUDE.md` before the agent reads it. Once the user picks a
+        // skillset and the switch completes, `event_handling.rs` triggers
+        // `spawn_deferred_agent()`. If the user dismisses the picker, the
+        // `SkillsetPickerDismissed` event triggers the deferred spawn without a
+        // skillset.
         #[cfg(feature = "nori-config")]
-        if nori_config.skillset_per_session
-            && crate::system_info::extract_worktree_name(&app.config.cwd).is_some()
-        {
+        if nori_config.skillset_per_session {
             app.chat_widget.handle_switch_skillset_command();
         }
 

codex-rs/tui/src/app_event.rs

@@ -374,6 +374,11 @@ pub(crate) enum AppEvent {
         message: String,
     },
 
+    /// The skillset picker was dismissed without selection. When agent spawn was
+    /// deferred for skillset_per_session, this triggers spawning the agent
+    /// without a skillset (behaves as if the feature is disabled).
+    SkillsetPickerDismissed,
+
     /// Execute a custom prompt script asynchronously.
     ExecuteScript {
         /// The custom prompt to execute.

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

@@ -973,8 +973,10 @@ mod tests {
 
     #[test]
     fn footer_with_worktree_name_disabled() {
-        let mut segment_config = FooterSegmentConfig::default();
-        segment_config.worktree_name = false;
+        let segment_config = FooterSegmentConfig {
+            worktree_name: false,
+            ..Default::default()
+        };
 
         snapshot_footer(
             "footer_with_worktree_name_disabled",
@@ -1021,8 +1023,10 @@ mod tests {
 
     #[test]
     fn footer_with_git_branch_disabled() {
-        let mut segment_config = FooterSegmentConfig::default();
-        segment_config.git_branch = false;
+        let segment_config = FooterSegmentConfig {
+            git_branch: false,
+            ..Default::default()
+        };
 
         snapshot_footer(
             "footer_with_git_branch_disabled",
@@ -1038,10 +1042,12 @@ mod tests {
 
     #[test]
     fn footer_with_multiple_segments_disabled() {
-        let mut segment_config = FooterSegmentConfig::default();
-        segment_config.git_branch = false;
-        segment_config.git_stats = false;
-        segment_config.token_usage = false;
+        let segment_config = FooterSegmentConfig {
+            git_branch: false,
+            git_stats: false,
+            token_usage: false,
+            ..Default::default()
+        };
 
         snapshot_footer(
             "footer_with_multiple_segments_disabled",
@@ -1063,17 +1069,18 @@ mod tests {
 
     #[test]
     fn footer_with_all_segments_disabled() {
-        let mut segment_config = FooterSegmentConfig::default();
-        segment_config.prompt_summary = false;
-        segment_config.vim_mode = false;
-        segment_config.git_branch = false;
-        segment_config.worktree_name = false;
-        segment_config.git_stats = false;
-        segment_config.context = false;
-        segment_config.approval_mode = false;
-        segment_config.nori_profile = false;
-        segment_config.nori_version = false;
-        segment_config.token_usage = false;
+        let segment_config = FooterSegmentConfig {
+            prompt_summary: false,
+            vim_mode: false,
+            git_branch: false,
+            worktree_name: false,
+            git_stats: false,
+            context: false,
+            approval_mode: false,
+            nori_profile: false,
+            nori_version: false,
+            token_usage: false,
+        };
 
         snapshot_footer(
             "footer_with_all_segments_disabled",
@@ -1098,9 +1105,11 @@ mod tests {
 
     #[test]
     fn footer_vertical_with_segments_disabled() {
-        let mut segment_config = FooterSegmentConfig::default();
-        segment_config.nori_profile = false;
-        segment_config.nori_version = false;
+        let segment_config = FooterSegmentConfig {
+            nori_profile: false,
+            nori_version: false,
+            ..Default::default()
+        };
 
         snapshot_footer(
             "footer_vertical_with_segments_disabled",

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

@@ -53,6 +53,9 @@ pub(crate) struct SelectionViewParams {
     pub search_placeholder: Option<String>,
     pub header: Box<dyn Renderable>,
     pub initial_selected_idx: Option<usize>,
+    /// Optional callback fired when the picker is dismissed without selection
+    /// (e.g. via Escape or Ctrl-C).
+    pub on_dismiss: Option<SelectionAction>,
 }
 
 impl Default for SelectionViewParams {
@@ -66,6 +69,7 @@ impl Default for SelectionViewParams {
             search_placeholder: None,
             header: Box::new(()),
             initial_selected_idx: None,
+            on_dismiss: None,
         }
     }
 }
@@ -83,6 +87,7 @@ pub(crate) struct ListSelectionView {
     last_selected_actual_idx: Option<usize>,
     header: Box<dyn Renderable>,
     initial_selected_idx: Option<usize>,
+    on_dismiss: Option<SelectionAction>,
 }
 
 impl ListSelectionView {
@@ -114,6 +119,7 @@ impl ListSelectionView {
             last_selected_actual_idx: None,
             header,
             initial_selected_idx: params.initial_selected_idx,
+            on_dismiss: params.on_dismiss,
         };
         s.apply_filter();
         s
@@ -331,6 +337,9 @@ impl BottomPaneView for ListSelectionView {
     }
 
     fn on_ctrl_c(&mut self) -> CancellationEvent {
+        if let Some(cb) = self.on_dismiss.take() {
+            cb(&self.app_event_tx);
+        }
         self.complete = true;
         CancellationEvent::Handled
     }
@@ -607,6 +616,71 @@ mod tests {
         );
     }
 
+    #[test]
+    fn on_dismiss_callback_fires_on_ctrl_c() {
+        let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
+        let tx = AppEventSender::new(tx_raw);
+        let dismissed = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false));
+        let dismissed_clone = dismissed.clone();
+        let items = vec![SelectionItem {
+            name: "Option A".to_string(),
+            dismiss_on_select: true,
+            ..Default::default()
+        }];
+        let mut view = ListSelectionView::new(
+            SelectionViewParams {
+                title: Some("Test".to_string()),
+                items,
+                on_dismiss: Some(Box::new(move |_tx| {
+                    dismissed_clone.store(true, std::sync::atomic::Ordering::SeqCst);
+                })),
+                ..Default::default()
+            },
+            tx,
+        );
+
+        // Dismiss via Ctrl-C
+        view.on_ctrl_c();
+
+        assert!(
+            dismissed.load(std::sync::atomic::Ordering::SeqCst),
+            "on_dismiss callback should fire when picker is dismissed via Ctrl-C"
+        );
+        assert!(view.is_complete(), "view should be complete after dismiss");
+    }
+
+    #[test]
+    fn on_dismiss_callback_not_fired_on_accept() {
+        let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
+        let tx = AppEventSender::new(tx_raw);
+        let dismissed = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false));
+        let dismissed_clone = dismissed.clone();
+        let items = vec![SelectionItem {
+            name: "Option A".to_string(),
+            dismiss_on_select: true,
+            ..Default::default()
+        }];
+        let mut view = ListSelectionView::new(
+            SelectionViewParams {
+                title: Some("Test".to_string()),
+                items,
+                on_dismiss: Some(Box::new(move |_tx| {
+                    dismissed_clone.store(true, std::sync::atomic::Ordering::SeqCst);
+                })),
+                ..Default::default()
+            },
+            tx,
+        );
+
+        // Accept via Enter (should NOT fire on_dismiss)
+        view.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
+
+        assert!(
+            !dismissed.load(std::sync::atomic::Ordering::SeqCst),
+            "on_dismiss callback should NOT fire when an item is selected"
+        );
+    }
+
     #[test]
     fn narrow_width_keeps_all_rows_visible() {
         let (tx_raw, _rx) = unbounded_channel::<AppEvent>();

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

@@ -27,6 +27,8 @@ mod footer;
 mod history_search_popup;
 mod list_selection_view;
 mod prompt_args;
+#[cfg(test)]
+pub(crate) use list_selection_view::ListSelectionView;
 pub(crate) use list_selection_view::SelectionViewParams;
 mod paste_burst;
 pub mod popup_consts;

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

@@ -303,8 +303,14 @@ impl ChatWidget {
     ) {
         match (names, error) {
             (Some(names), None) if !names.is_empty() => {
-                let params =
-                    crate::nori::skillset_picker::skillset_picker_params(names, install_dir);
+                let on_dismiss: SelectionAction = Box::new(|tx| {
+                    tx.send(AppEvent::SkillsetPickerDismissed);
+                });
+                let params = crate::nori::skillset_picker::skillset_picker_params(
+                    names,
+                    install_dir,
+                    Some(on_dismiss),
+                );
                 self.bottom_pane.show_selection_view(params);
             }
             (_, Some(error)) => {

codex-rs/tui/src/chatwidget/tests/mod.rs

@@ -28,7 +28,6 @@ use codex_core::protocol::FileChange;
 use codex_core::protocol::Op;
 use codex_core::protocol::PatchApplyBeginEvent;
 use codex_core::protocol::PatchApplyEndEvent;
-use codex_core::protocol::RateLimitWindow;
 use codex_core::protocol::StreamErrorEvent;
 use codex_core::protocol::TaskCompleteEvent;
 use codex_core::protocol::TaskStartedEvent;
@@ -71,18 +70,6 @@ fn test_config() -> Config {
     .expect("config")
 }
 
-fn snapshot(percent: f64) -> RateLimitSnapshot {
-    RateLimitSnapshot {
-        primary: Some(RateLimitWindow {
-            used_percent: percent,
-            window_minutes: Some(60),
-            resets_at: None,
-        }),
-        secondary: None,
-        credits: None,
-    }
-}
-
 fn drain_insert_history(
     rx: &mut tokio::sync::mpsc::UnboundedReceiver<AppEvent>,
 ) -> Vec<Vec<ratatui::text::Line<'static>>> {