mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
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 <noreply@openai.com>
This commit is contained in:
committed by
GitHub
Unverified
parent
c24124b37d
commit
706f830dc6
@@ -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")]
|
||||
|
||||
@@ -522,6 +522,7 @@ impl ChatComposer {
|
||||
|
||||
pub fn set_skill_mentions(&mut self, skills: Option<Vec<SkillMetadata>>) {
|
||||
self.skills = skills;
|
||||
self.sync_popups();
|
||||
}
|
||||
|
||||
pub fn set_plugin_mentions(&mut self, plugins: Option<Vec<PluginCapabilitySummary>>) {
|
||||
@@ -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::<AppEvent>();
|
||||
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();
|
||||
|
||||
@@ -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<T>(&mut self, op: T) -> bool
|
||||
where
|
||||
|
||||
Reference in New Issue
Block a user