From 706f830dc6c7234ace6a84e5d2ebffbf28dbbd5e Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 14 Apr 2026 12:49:49 -0700 Subject: [PATCH] Fix remote skill popup loading (#17702) ## Summary Fix the TUI `$` skill popup so personal skills appear reliably when Codex is connected to a remote app-server. ## What changed - load skills on TUI startup with an explicit forced refresh - refresh skills using the actual current cwd instead of an empty `cwds` list - resync an already-open `$` popup when skill mentions are updated - add a regression test for refreshing an open mention popup ## Root cause The TUI was sometimes sending `list_skills` with `cwds: []` after `SessionConfigured`. For the launchd app-server flow, the server resolved that empty cwd list to its own process cwd, which was `/`. The response therefore came back tagged with `cwd: "/"`, and the TUI later filtered skills by exact cwd match against the actual project cwd such as `/Users/starr/code/dream`. That dropped all personal skills from the mention list, so `$` only showed plugins/apps. ## Verification Built successfully with remote cache disabled: ```bash cd /Users/starr/code/codex-worktrees/starr-skill-popup-20260413130509 bazel --output_base=/tmp/codex-bazel-verify-starr-skill-popup build //codex-rs/cli:codex --noremote_accept_cached --noremote_upload_local_results --disk_cache= ``` Also verified interactively in a PTY against the live app-server at `ws://127.0.0.1:4511`: - launched the built TUI - typed `$` - confirmed personal skills appeared in the popup, including entries such as `Applied Devbox`, `CI Debug`, `Channel Summarization`, `Codex PR Review`, and `Daily Digest` ## Files changed - `codex-rs/tui/src/app.rs` - `codex-rs/tui/src/chatwidget.rs` - `codex-rs/tui/src/bottom_pane/chat_composer.rs` Co-authored-by: Codex --- codex-rs/tui/src/app.rs | 11 ++++++ codex-rs/tui/src/bottom_pane/chat_composer.rs | 37 +++++++++++++++++++ codex-rs/tui/src/chatwidget.rs | 23 ++++++------ 3 files changed, 59 insertions(+), 12 deletions(-) diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index ba8de0bac..fd842ca63 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -3868,6 +3868,17 @@ impl App { app.enqueue_primary_thread_session(started.session, started.turns) .await?; } + match app_server + .skills_list(codex_app_server_protocol::SkillsListParams { + cwds: vec![app.config.cwd.to_path_buf()], + force_reload: true, + per_cwd_extra_user_roots: None, + }) + .await + { + Ok(response) => app.handle_skills_list_response(response), + Err(err) => tracing::warn!("failed to load skills on startup: {err:#}"), + } // On startup, if Agent mode (workspace-write) or ReadOnly is active, warn about world-writable dirs on Windows. #[cfg(target_os = "windows")] diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index cb1e642a5..b1c3129ff 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -522,6 +522,7 @@ impl ChatComposer { pub fn set_skill_mentions(&mut self, skills: Option>) { self.skills = skills; + self.sync_popups(); } pub fn set_plugin_mentions(&mut self, plugins: Option>) { @@ -5053,6 +5054,42 @@ mod tests { assert_eq!(mention.path, Some("plugin://sample@test".to_string())); } + #[test] + fn set_skill_mentions_refreshes_open_mention_popup() { + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + /*has_input_focus*/ true, + sender, + /*enhanced_keys_supported*/ false, + "Ask Codex to do anything".to_string(), + /*disable_paste_burst*/ false, + ); + composer.set_text_content("$".to_string(), Vec::new(), Vec::new()); + assert!(matches!(composer.active_popup, ActivePopup::None)); + + let skill_path = test_path_buf("/tmp/skill/SKILL.md").abs(); + composer.set_skill_mentions(Some(vec![SkillMetadata { + name: "codex".to_string(), + description: "Primary personal Codex repo skill.".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + path_to_skills_md: skill_path.clone(), + scope: codex_protocol::protocol::SkillScope::User, + }])); + + let ActivePopup::Skill(popup) = &composer.active_popup else { + panic!("expected mention popup to open after skills update"); + }; + let mention = popup + .selected_mention() + .expect("expected skill mention to be selected"); + assert_eq!(mention.insert_text, "$codex".to_string()); + assert_eq!(mention.path, Some(skill_path.display().to_string())); + } + #[test] fn mention_items_show_plugin_owned_skill_and_app_duplicates() { let skill_path = test_path_buf("/tmp/repo/google-calendar/SKILL.md").abs(); diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 25f9472c8..ee3391b90 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -2039,10 +2039,7 @@ impl ChatWidget { self.replay_initial_messages(messages); } self.saw_copy_source_this_turn = false; - self.submit_op(AppCommand::list_skills( - Vec::new(), - /*force_reload*/ true, - )); + self.refresh_skills_for_current_cwd(/*force_reload*/ true); if self.connectors_enabled() { self.prefetch_connectors(); } @@ -6156,10 +6153,7 @@ impl ChatWidget { } } ServerNotification::SkillsChanged(_) => { - self.submit_op(AppCommand::list_skills( - Vec::new(), - /*force_reload*/ true, - )); + self.refresh_skills_for_current_cwd(/*force_reload*/ true); } ServerNotification::ModelRerouted(_) => {} ServerNotification::DeprecationNotice(notification) => { @@ -6731,10 +6725,7 @@ impl ChatWidget { EventMsg::McpListToolsResponse(ev) => self.on_list_mcp_tools(ev), EventMsg::ListSkillsResponse(ev) => self.on_list_skills(ev), EventMsg::SkillsUpdateAvailable => { - self.submit_op(AppCommand::list_skills( - Vec::new(), - /*force_reload*/ true, - )); + self.refresh_skills_for_current_cwd(/*force_reload*/ true); } EventMsg::ShutdownComplete => self.on_shutdown_complete(), EventMsg::TurnDiff(TurnDiffEvent { unified_diff }) => self.on_turn_diff(unified_diff), @@ -10229,6 +10220,14 @@ impl ChatWidget { pub(crate) fn clear_esc_backtrack_hint(&mut self) { self.bottom_pane.clear_esc_backtrack_hint(); } + + fn refresh_skills_for_current_cwd(&mut self, force_reload: bool) { + self.submit_op(AppCommand::list_skills( + vec![self.config.cwd.to_path_buf()], + force_reload, + )); + } + /// Forward a command directly to codex. pub(crate) fn submit_op(&mut self, op: T) -> bool where