From b6de5b524cdc07400b0ac049ae70ac8e90285e67 Mon Sep 17 00:00:00 2001 From: felixxia-oai Date: Mon, 20 Jul 2026 15:32:18 +0000 Subject: [PATCH] Use app-server skill metadata directly in the TUI (#34368) ## Why The TUI already receives skill metadata from the app-server protocol, but converted it into the core-skills model before using it. ## What changed - Use `codex_app_server_protocol::SkillMetadata` throughout skill toggles, mentions, and input submission. - Remove the protocol-to-core conversion and the TUI's `codex-core-skills` dependency. GitOrigin-RevId: 43d43d74bfb67fc21ef11a0ca0592aa44b89a122 --- codex-rs/Cargo.lock | 1 - codex-rs/tui/Cargo.toml | 1 - codex-rs/tui/src/bottom_pane/chat_composer.rs | 28 +++--- .../bottom_pane/mentions_v2/search_catalog.rs | 4 +- codex-rs/tui/src/bottom_pane/mod.rs | 7 +- codex-rs/tui/src/chatwidget.rs | 5 +- .../tui/src/chatwidget/input_submission.rs | 12 ++- codex-rs/tui/src/chatwidget/skills.rs | 90 ++++--------------- codex-rs/tui/src/chatwidget/tests.rs | 2 +- .../chatwidget/tests/composer_submission.rs | 10 +-- codex-rs/tui/src/skills_helpers.rs | 2 +- 11 files changed, 46 insertions(+), 116 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index c1ad4d27fa..02de9b4c05 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -4114,7 +4114,6 @@ dependencies = [ "codex-config", "codex-connectors", "codex-core-plugins", - "codex-core-skills", "codex-exec-server", "codex-features", "codex-feedback", diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 91f4b991fd..7fd75990eb 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -35,7 +35,6 @@ codex-cloud-config = { workspace = true } codex-config = { workspace = true } codex-connectors = { workspace = true } codex-core-plugins = { workspace = true } -codex-core-skills = { workspace = true } codex-exec-server = { workspace = true } codex-features = { workspace = true } codex-feedback = { workspace = true } diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index d260507ebd..2404ad13ca 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -292,10 +292,10 @@ use crate::history_cell; use crate::skills_helpers::skill_display_name; use crate::tui::FrameRequester; use crate::ui_consts::LIVE_PREFIX_COLS; -use codex_connectors::AppInfo; #[cfg(test)] -use codex_core_skills::model::SkillInterface; -use codex_core_skills::model::SkillMetadata; +use codex_app_server_protocol::SkillInterface; +use codex_app_server_protocol::SkillMetadata; +use codex_connectors::AppInfo; use codex_file_search::FileMatch; #[cfg(test)] use codex_plugin::AppConnectorId; @@ -4027,7 +4027,7 @@ impl ChatComposer { description, insert_text: format!("${skill_name}"), search_terms, - path: Some(skill.path_to_skills_md.to_string_lossy().into_owned()), + path: Some(skill.path.to_string_lossy().into_owned()), category_tag: Some("[Skill]".to_string()), sort_rank: 1, }); @@ -6641,10 +6641,9 @@ mod tests { short_description: None, interface: None, dependencies: None, - policy: None, - path_to_skills_md: test_path_buf(&format!("/tmp/{name}/SKILL.md")).abs(), + path: test_path_buf(&format!("/tmp/{name}/SKILL.md")).abs(), scope: crate::test_support::skill_scope_user(), - plugin_id: None, + enabled: true, } } @@ -7292,10 +7291,9 @@ mod tests { short_description: None, interface: None, dependencies: None, - policy: None, - path_to_skills_md: skill_path.clone(), + path: skill_path.clone(), scope: crate::test_support::skill_scope_user(), - plugin_id: None, + enabled: true, }])); let ActivePopup::Skill(popup) = &composer.popups.active else { @@ -7335,10 +7333,9 @@ mod tests { default_prompt: None, }), dependencies: None, - policy: None, - path_to_skills_md: skill_path.clone(), + path: skill_path.clone(), scope: crate::test_support::skill_scope_repo(), - plugin_id: None, + enabled: true, }])); composer.set_plugin_mentions(Some(vec![PluginCapabilitySummary { config_name: "google-calendar@debug".to_string(), @@ -7451,10 +7448,9 @@ mod tests { default_prompt: None, }), dependencies: None, - policy: None, - path_to_skills_md: test_path_buf("/tmp/repo/google-calendar/SKILL.md").abs(), + path: test_path_buf("/tmp/repo/google-calendar/SKILL.md").abs(), scope: crate::test_support::skill_scope_repo(), - plugin_id: None, + enabled: true, }])); composer.set_plugin_mentions(Some(vec![PluginCapabilitySummary { config_name: "google-calendar@debug".to_string(), diff --git a/codex-rs/tui/src/bottom_pane/mentions_v2/search_catalog.rs b/codex-rs/tui/src/bottom_pane/mentions_v2/search_catalog.rs index a555070a86..6ae6c04d3a 100644 --- a/codex-rs/tui/src/bottom_pane/mentions_v2/search_catalog.rs +++ b/codex-rs/tui/src/bottom_pane/mentions_v2/search_catalog.rs @@ -1,4 +1,4 @@ -use codex_core_skills::model::SkillMetadata; +use codex_app_server_protocol::SkillMetadata; use codex_plugin::PluginCapabilitySummary; use crate::skills_helpers::skill_description; @@ -40,7 +40,7 @@ fn skill_candidate(skill: &SkillMetadata) -> Candidate { mention_type: MentionType::Skill, selection: Selection::Tool { insert_text: format!("${skill_name}"), - path: Some(skill.path_to_skills_md.to_string_lossy().into_owned()), + path: Some(skill.path.to_string_lossy().into_owned()), }, } } diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index b8d6c9c357..72cdc89619 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -36,8 +36,8 @@ use crate::terminal_palette::effective_stdout_color_level; use crate::tui::FrameRequester; pub(crate) use bottom_pane_view::BottomPaneView; pub(crate) use bottom_pane_view::ViewCompletion; +use codex_app_server_protocol::SkillMetadata; use codex_app_server_protocol::ToolRequestUserInputParams; -use codex_core_skills::model::SkillMetadata; use codex_features::Features; use codex_file_search::FileMatch; use codex_plugin::PluginCapabilitySummary; @@ -2650,10 +2650,9 @@ mod tests { short_description: None, interface: None, dependencies: None, - policy: None, - path_to_skills_md: test_path_buf("/tmp/test-skill/SKILL.md").abs(), + path: test_path_buf("/tmp/test-skill/SKILL.md").abs(), scope: crate::test_support::skill_scope_user(), - plugin_id: None, + enabled: true, }]), }); diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 4de4aa1ff0..a3d539e54c 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -102,7 +102,7 @@ use codex_app_server_protocol::RequestId as AppServerRequestId; use codex_app_server_protocol::ReviewTarget; use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ServerRequest; -use codex_app_server_protocol::SkillMetadata as ProtocolSkillMetadata; +use codex_app_server_protocol::SkillMetadata; use codex_app_server_protocol::SkillsListResponse; use codex_app_server_protocol::ThreadGoal as AppThreadGoal; use codex_app_server_protocol::ThreadGoalStatus as AppThreadGoalStatus; @@ -123,7 +123,6 @@ use codex_config::types::ApprovalsReviewer; use codex_config::types::Notifications; use codex_config::types::WindowsSandboxModeToml; use codex_connectors::AppInfo; -use codex_core_skills::model::SkillMetadata; use codex_features::FEATURES; use codex_features::Feature; #[cfg(test)] @@ -590,7 +589,7 @@ pub(crate) struct ChatWidget { collab_agent_metadata: HashMap, pending_collab_spawn_requests: HashMap, suppressed_exec_calls: HashSet, - skills_all: Vec, + skills_all: Vec, skills_initial_state: Option>, last_unified_wait: Option, unified_exec_wait_streak: Option, diff --git a/codex-rs/tui/src/chatwidget/input_submission.rs b/codex-rs/tui/src/chatwidget/input_submission.rs index ec55f34f81..830cd3eea6 100644 --- a/codex-rs/tui/src/chatwidget/input_submission.rs +++ b/codex-rs/tui/src/chatwidget/input_submission.rs @@ -203,14 +203,12 @@ impl ChatWidget { .strip_prefix("skill://") .unwrap_or(binding.path.as_str()); let path = Path::new(path); - if let Some(skill) = skills - .iter() - .find(|skill| skill.path_to_skills_md.as_path() == path) - && selected_skill_paths.insert(skill.path_to_skills_md.clone()) + if let Some(skill) = skills.iter().find(|skill| skill.path.as_path() == path) + && selected_skill_paths.insert(skill.path.clone()) { items.push(UserInput::Skill { name: skill.name.clone(), - path: skill.path_to_skills_md.to_path_buf(), + path: skill.path.to_path_buf(), }); } } @@ -218,13 +216,13 @@ impl ChatWidget { let skill_mentions = find_skill_mentions_with_tool_mentions(&mentions, skills); for skill in skill_mentions { if bound_names.contains(skill.name.as_str()) - || !selected_skill_paths.insert(skill.path_to_skills_md.clone()) + || !selected_skill_paths.insert(skill.path.clone()) { continue; } items.push(UserInput::Skill { name: skill.name.clone(), - path: skill.path_to_skills_md.to_path_buf(), + path: skill.path.to_path_buf(), }); } } diff --git a/codex-rs/tui/src/chatwidget/skills.rs b/codex-rs/tui/src/chatwidget/skills.rs index c408042d5e..3d5f9cc1b6 100644 --- a/codex-rs/tui/src/chatwidget/skills.rs +++ b/codex-rs/tui/src/chatwidget/skills.rs @@ -10,14 +10,10 @@ use crate::bottom_pane::SkillsToggleView; use crate::bottom_pane::popup_consts::standard_popup_hint_line; use crate::skills_helpers::skill_description; use crate::skills_helpers::skill_display_name; -use codex_app_server_protocol::SkillMetadata as ProtocolSkillMetadata; +use codex_app_server_protocol::SkillMetadata; use codex_app_server_protocol::SkillsListEntry; use codex_app_server_protocol::SkillsListResponse; use codex_connectors::AppInfo; -use codex_core_skills::model::SkillDependencies; -use codex_core_skills::model::SkillInterface; -use codex_core_skills::model::SkillMetadata; -use codex_core_skills::model::SkillToolDependency; use codex_features::Feature; use codex_protocol::parse_command::ParsedCommand; use codex_utils_absolute_path::AbsolutePathBuf; @@ -85,19 +81,16 @@ impl ChatWidget { let items: Vec = self .skills_all .iter() - .filter_map(|skill| { - let core_skill = protocol_skill_to_core(skill)?; - let display_name = skill_display_name(&core_skill); - let description = skill_description(&core_skill).to_string(); - let name = core_skill.name.clone(); - let path = core_skill.path_to_skills_md; - Some(SkillsToggleItem { + .map(|skill| { + let display_name = skill_display_name(skill); + let description = skill_description(skill).to_string(); + SkillsToggleItem { name: display_name, - skill_name: name, + skill_name: skill.name.clone(), description, enabled: skill.enabled, - path, - }) + path: skill.path.clone(), + } }) .collect(); @@ -187,10 +180,7 @@ impl ChatWidget { } } -fn skills_for_cwd( - cwd: &AbsolutePathBuf, - skills_entries: &[SkillsListEntry], -) -> Vec { +fn skills_for_cwd(cwd: &AbsolutePathBuf, skills_entries: &[SkillsListEntry]) -> Vec { skills_entries .iter() .find(|entry| entry.cwd.as_path() == cwd.as_path()) @@ -198,62 +188,14 @@ fn skills_for_cwd( .unwrap_or_default() } -fn enabled_skills_for_mentions(skills: &[ProtocolSkillMetadata]) -> Vec { +fn enabled_skills_for_mentions(skills: &[SkillMetadata]) -> Vec { skills .iter() .filter(|skill| skill.enabled) - .filter_map(protocol_skill_to_core) + .cloned() .collect() } -fn protocol_skill_to_core(skill: &ProtocolSkillMetadata) -> Option { - let scope = serde_json::to_value(skill.scope) - .and_then(serde_json::from_value) - .inspect_err(|err| { - tracing::warn!( - skill_name = %skill.name, - %err, - "Failed to map app-server skill scope" - ); - }) - .ok()?; - - Some(SkillMetadata { - name: skill.name.clone(), - description: skill.description.clone(), - short_description: skill.short_description.clone(), - interface: skill.interface.clone().map(|interface| SkillInterface { - display_name: interface.display_name, - short_description: interface.short_description, - icon_small: interface.icon_small, - icon_large: interface.icon_large, - brand_color: interface.brand_color, - default_prompt: interface.default_prompt, - }), - dependencies: skill - .dependencies - .clone() - .map(|dependencies| SkillDependencies { - tools: dependencies - .tools - .into_iter() - .map(|tool| SkillToolDependency { - r#type: tool.r#type, - value: tool.value, - description: tool.description, - transport: tool.transport, - command: tool.command, - url: tool.url, - }) - .collect(), - }), - policy: None, - path_to_skills_md: skill.path.clone(), - scope, - plugin_id: None, - }) -} - pub(crate) fn collect_tool_mentions( text: &str, mention_paths: &HashMap, @@ -283,23 +225,23 @@ pub(crate) fn find_skill_mentions_with_tool_mentions( let mut matches: Vec = Vec::new(); for skill in skills { - if seen_paths.contains(&skill.path_to_skills_md) { + if seen_paths.contains(&skill.path) { continue; } - let path_str = skill.path_to_skills_md.to_string_lossy(); + let path_str = skill.path.to_string_lossy(); if mention_skill_paths.contains(path_str.as_ref()) { - seen_paths.insert(skill.path_to_skills_md.clone()); + seen_paths.insert(skill.path.clone()); seen_names.insert(skill.name.clone()); matches.push(skill.clone()); } } for skill in skills { - if seen_paths.contains(&skill.path_to_skills_md) { + if seen_paths.contains(&skill.path) { continue; } if mentions.names.contains(&skill.name) && seen_names.insert(skill.name.clone()) { - seen_paths.insert(skill.path_to_skills_md.clone()); + seen_paths.insert(skill.path.clone()); matches.push(skill.clone()); } } diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 24423d3737..b8cd9e2b3f 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -99,6 +99,7 @@ pub(super) use codex_app_server_protocol::RateLimitWindow; pub(super) use codex_app_server_protocol::ReasoningSummaryTextDeltaNotification; pub(super) use codex_app_server_protocol::ReviewTarget; pub(super) use codex_app_server_protocol::ServerNotification; +pub(super) use codex_app_server_protocol::SkillMetadata; pub(super) use codex_app_server_protocol::SkillSummary; pub(super) use codex_app_server_protocol::ThreadClosedNotification; pub(super) use codex_app_server_protocol::ThreadItem as AppServerThreadItem; @@ -121,7 +122,6 @@ pub(super) use codex_config::types::ApprovalsReviewer; pub(super) use codex_config::types::Notifications; pub(super) use codex_config::types::WindowsSandboxModeToml; pub(super) use codex_core_plugins::OPENAI_CURATED_MARKETPLACE_NAME; -pub(super) use codex_core_skills::model::SkillMetadata; pub(super) use codex_features::FEATURES; pub(super) use codex_features::Feature; pub(super) use codex_git_utils::CommitLogEntry; diff --git a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs index 2b3a78eb25..c334a7ab35 100644 --- a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs +++ b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs @@ -690,10 +690,9 @@ async fn submission_prefers_selected_duplicate_skill_path() { short_description: None, interface: None, dependencies: None, - policy: None, - path_to_skills_md: repo_skill_path, + path: repo_skill_path, scope: crate::test_support::skill_scope_repo(), - plugin_id: None, + enabled: true, }, SkillMetadata { name: "figma".to_string(), @@ -701,10 +700,9 @@ async fn submission_prefers_selected_duplicate_skill_path() { short_description: None, interface: None, dependencies: None, - policy: None, - path_to_skills_md: user_skill_path.clone(), + path: user_skill_path.clone(), scope: crate::test_support::skill_scope_user(), - plugin_id: None, + enabled: true, }, ])); diff --git a/codex-rs/tui/src/skills_helpers.rs b/codex-rs/tui/src/skills_helpers.rs index 88d879c6a5..6843791951 100644 --- a/codex-rs/tui/src/skills_helpers.rs +++ b/codex-rs/tui/src/skills_helpers.rs @@ -1,4 +1,4 @@ -use codex_core_skills::model::SkillMetadata; +use codex_app_server_protocol::SkillMetadata; use codex_utils_fuzzy_match::fuzzy_match; pub(crate) fn skill_display_name(skill: &SkillMetadata) -> String {