Skip to content

Commit bb0beef

Browse files
canvrno-oaicanvrno-oai
committed
tui: keep plugin-backed app mentions visible in skill picker
1 parent 4ce939c commit bb0beef

File tree

6 files changed

+252
-59
lines changed

6 files changed

+252
-59
lines changed

‎codex-rs/tui/src/bottom_pane/chat_composer.rs‎

Lines changed: 21 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -3575,24 +3575,6 @@ impl ChatComposer {
35753575

35763576
fn mention_items(&self) -> Vec<MentionItem> {
35773577
let mut mentions = Vec::new();
3578-
let plugin_namespaces: HashSet<String> =
3579-
self.plugins.as_ref().map_or_else(HashSet::new, |plugins| {
3580-
plugins
3581-
.iter()
3582-
.filter_map(|plugin| {
3583-
let (plugin_name, _) = plugin
3584-
.config_name
3585-
.split_once('@')
3586-
.unwrap_or((plugin.config_name.as_str(), ""));
3587-
let plugin_name = plugin_name.trim();
3588-
if plugin_name.is_empty() {
3589-
None
3590-
} else {
3591-
Some(plugin_name.to_ascii_lowercase())
3592-
}
3593-
})
3594-
.collect()
3595-
});
35963578
let plugin_display_names: HashSet<String> =
35973579
self.plugins.as_ref().map_or_else(HashSet::new, |plugins| {
35983580
plugins
@@ -3603,13 +3585,6 @@ impl ChatComposer {
36033585

36043586
if let Some(skills) = self.skills.as_ref() {
36053587
for skill in skills {
3606-
let is_plugin_namespaced_skill =
3607-
skill.name.split_once(':').is_some_and(|(namespace, _)| {
3608-
plugin_namespaces.contains(&namespace.to_ascii_lowercase())
3609-
});
3610-
if is_plugin_namespaced_skill {
3611-
continue;
3612-
}
36133588
let display_name = skill_display_name(skill).to_string();
36143589
let description = skill_description(skill);
36153590
let skill_name = skill.name.clone();
@@ -5419,7 +5394,7 @@ mod tests {
54195394
}
54205395

54215396
#[test]
5422-
fn mention_items_hide_plugin_owned_skill_and_app_duplicates() {
5397+
fn mention_items_keep_plugin_owned_skills_but_hide_duplicate_apps() {
54235398
let (tx, _rx) = unbounded_channel::<AppEvent>();
54245399
let sender = AppEventSender::new(tx);
54255400
let mut composer = ChatComposer::new(
@@ -5481,13 +5456,27 @@ mod tests {
54815456
}],
54825457
}));
54835458

5484-
let mentions = composer.mention_items();
5485-
assert_eq!(mentions.len(), 1);
5486-
assert_eq!(mentions[0].display_name, "Google Calendar".to_string());
5487-
assert_eq!(mentions[0].category_tag, Some("[Plugin]".to_string()));
5459+
let mut mention_summaries: Vec<_> = composer
5460+
.mention_items()
5461+
.into_iter()
5462+
.map(|mention| (mention.display_name, mention.category_tag, mention.path))
5463+
.collect();
5464+
mention_summaries.sort();
5465+
54885466
assert_eq!(
5489-
mentions[0].path,
5490-
Some("plugin://google-calendar@debug".to_string())
5467+
mention_summaries,
5468+
vec![
5469+
(
5470+
"Google Calendar".to_string(),
5471+
Some("[Plugin]".to_string()),
5472+
Some("plugin://google-calendar@debug".to_string()),
5473+
),
5474+
(
5475+
"Google Calendar".to_string(),
5476+
Some("[Skill]".to_string()),
5477+
Some("/tmp/repo/google-calendar/SKILL.md".to_string()),
5478+
),
5479+
]
54915480
);
54925481
}
54935482

‎codex-rs/tui/src/bottom_pane/file_search_popup.rs‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ impl FileSearchPopup {
7070
}
7171

7272
self.display_query = query.to_string();
73-
self.matches = matches;
73+
self.matches = matches.into_iter().take(MAX_POPUP_ROWS).collect();
7474
self.waiting = false;
7575
let len = self.matches.len();
7676
self.state.clamp_selection(len);
@@ -152,3 +152,33 @@ impl WidgetRef for &FileSearchPopup {
152152
);
153153
}
154154
}
155+
156+
#[cfg(test)]
157+
mod tests {
158+
use super::*;
159+
use codex_file_search::MatchType;
160+
use pretty_assertions::assert_eq;
161+
162+
fn file_match(index: usize) -> FileMatch {
163+
FileMatch {
164+
score: index as u32,
165+
path: PathBuf::from(format!("src/file_{index:02}.rs")),
166+
match_type: MatchType::File,
167+
root: PathBuf::from("/tmp/repo"),
168+
indices: None,
169+
}
170+
}
171+
172+
#[test]
173+
fn set_matches_keeps_only_the_first_page_of_results() {
174+
let mut popup = FileSearchPopup::new();
175+
popup.set_query("file");
176+
popup.set_matches("file", (0..(MAX_POPUP_ROWS + 2)).map(file_match).collect());
177+
178+
assert_eq!(
179+
popup.matches,
180+
(0..MAX_POPUP_ROWS).map(file_match).collect::<Vec<_>>()
181+
);
182+
assert_eq!(popup.calculate_required_height(), MAX_POPUP_ROWS as u16);
183+
}
184+
}

‎codex-rs/tui/src/bottom_pane/skill_popup.rs‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ impl SkillPopup {
180180
})
181181
});
182182

183+
out.truncate(MAX_POPUP_ROWS);
183184
out
184185
}
185186
}
@@ -229,3 +230,43 @@ fn skill_popup_hint_line() -> Line<'static> {
229230
" to close".into(),
230231
])
231232
}
233+
234+
#[cfg(test)]
235+
mod tests {
236+
use super::*;
237+
use pretty_assertions::assert_eq;
238+
239+
fn mention_item(index: usize) -> MentionItem {
240+
MentionItem {
241+
display_name: format!("Mention {index:02}"),
242+
description: Some(format!("Description {index:02}")),
243+
insert_text: format!("$mention-{index:02}"),
244+
search_terms: vec![format!("mention-{index:02}")],
245+
path: Some(format!("skill://mention-{index:02}")),
246+
category_tag: Some("[Skill]".to_string()),
247+
sort_rank: 1,
248+
}
249+
}
250+
251+
#[test]
252+
fn filtered_mentions_are_capped_to_max_popup_rows() {
253+
let popup = SkillPopup::new((0..(MAX_POPUP_ROWS + 2)).map(mention_item).collect());
254+
255+
let filtered_names: Vec<String> = popup
256+
.filtered_items()
257+
.into_iter()
258+
.map(|idx| popup.mentions[idx].display_name.clone())
259+
.collect();
260+
261+
assert_eq!(
262+
filtered_names,
263+
(0..MAX_POPUP_ROWS)
264+
.map(|idx| format!("Mention {idx:02}"))
265+
.collect::<Vec<_>>()
266+
);
267+
assert_eq!(
268+
popup.calculate_required_height(72),
269+
(MAX_POPUP_ROWS as u16) + 2
270+
);
271+
}
272+
}

‎codex-rs/tui_app_server/src/bottom_pane/chat_composer.rs‎

Lines changed: 87 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3590,24 +3590,6 @@ impl ChatComposer {
35903590

35913591
fn mention_items(&self) -> Vec<MentionItem> {
35923592
let mut mentions = Vec::new();
3593-
let plugin_namespaces: HashSet<String> =
3594-
self.plugins.as_ref().map_or_else(HashSet::new, |plugins| {
3595-
plugins
3596-
.iter()
3597-
.filter_map(|plugin| {
3598-
let (plugin_name, _) = plugin
3599-
.config_name
3600-
.split_once('@')
3601-
.unwrap_or((plugin.config_name.as_str(), ""));
3602-
let plugin_name = plugin_name.trim();
3603-
if plugin_name.is_empty() {
3604-
None
3605-
} else {
3606-
Some(plugin_name.to_ascii_lowercase())
3607-
}
3608-
})
3609-
.collect()
3610-
});
36113593
let plugin_display_names: HashSet<String> =
36123594
self.plugins.as_ref().map_or_else(HashSet::new, |plugins| {
36133595
plugins
@@ -3618,13 +3600,6 @@ impl ChatComposer {
36183600

36193601
if let Some(skills) = self.skills.as_ref() {
36203602
for skill in skills {
3621-
let is_plugin_namespaced_skill =
3622-
skill.name.split_once(':').is_some_and(|(namespace, _)| {
3623-
plugin_namespaces.contains(&namespace.to_ascii_lowercase())
3624-
});
3625-
if is_plugin_namespaced_skill {
3626-
continue;
3627-
}
36283603
let display_name = skill_display_name(skill).to_string();
36293604
let description = skill_description(skill);
36303605
let skill_name = skill.name.clone();
@@ -5433,6 +5408,93 @@ mod tests {
54335408
assert_eq!(mention.path, Some("plugin://sample@test".to_string()));
54345409
}
54355410

5411+
#[test]
5412+
fn mention_items_keep_plugin_owned_skills_but_hide_duplicate_apps() {
5413+
let (tx, _rx) = unbounded_channel::<AppEvent>();
5414+
let sender = AppEventSender::new(tx);
5415+
let mut composer = ChatComposer::new(
5416+
true,
5417+
sender,
5418+
false,
5419+
"Ask Codex to do anything".to_string(),
5420+
false,
5421+
);
5422+
composer.set_connectors_enabled(true);
5423+
composer.set_text_content("$goog".to_string(), Vec::new(), Vec::new());
5424+
composer.set_skill_mentions(Some(vec![SkillMetadata {
5425+
name: "google-calendar:availability".to_string(),
5426+
description: "Find availability and plan event changes".to_string(),
5427+
short_description: None,
5428+
interface: Some(codex_core::skills::model::SkillInterface {
5429+
display_name: Some("Google Calendar".to_string()),
5430+
short_description: None,
5431+
icon_small: None,
5432+
icon_large: None,
5433+
brand_color: None,
5434+
default_prompt: None,
5435+
}),
5436+
dependencies: None,
5437+
policy: None,
5438+
permission_profile: None,
5439+
managed_network_override: None,
5440+
path_to_skills_md: PathBuf::from("/tmp/repo/google-calendar/SKILL.md"),
5441+
scope: codex_protocol::protocol::SkillScope::Repo,
5442+
}]));
5443+
composer.set_plugin_mentions(Some(vec![PluginCapabilitySummary {
5444+
config_name: "google-calendar@debug".to_string(),
5445+
display_name: "Google Calendar".to_string(),
5446+
description: Some(
5447+
"Connect Google Calendar for scheduling, availability, and event management."
5448+
.to_string(),
5449+
),
5450+
has_skills: true,
5451+
mcp_server_names: vec!["google-calendar".to_string()],
5452+
app_connector_ids: vec![codex_core::plugins::AppConnectorId(
5453+
"google_calendar".to_string(),
5454+
)],
5455+
}]));
5456+
composer.set_connector_mentions(Some(ConnectorsSnapshot {
5457+
connectors: vec![AppInfo {
5458+
id: "google_calendar".to_string(),
5459+
name: "Google Calendar".to_string(),
5460+
description: Some("Look up events and availability".to_string()),
5461+
logo_url: None,
5462+
logo_url_dark: None,
5463+
distribution_channel: None,
5464+
branding: None,
5465+
app_metadata: None,
5466+
labels: None,
5467+
install_url: Some("https://example.test/google-calendar".to_string()),
5468+
is_accessible: true,
5469+
is_enabled: true,
5470+
plugin_display_names: vec!["Google Calendar".to_string()],
5471+
}],
5472+
}));
5473+
5474+
let mut mention_summaries: Vec<_> = composer
5475+
.mention_items()
5476+
.into_iter()
5477+
.map(|mention| (mention.display_name, mention.category_tag, mention.path))
5478+
.collect();
5479+
mention_summaries.sort();
5480+
5481+
assert_eq!(
5482+
mention_summaries,
5483+
vec![
5484+
(
5485+
"Google Calendar".to_string(),
5486+
Some("[Plugin]".to_string()),
5487+
Some("plugin://google-calendar@debug".to_string()),
5488+
),
5489+
(
5490+
"Google Calendar".to_string(),
5491+
Some("[Skill]".to_string()),
5492+
Some("/tmp/repo/google-calendar/SKILL.md".to_string()),
5493+
),
5494+
]
5495+
);
5496+
}
5497+
54365498
#[test]
54375499
fn plugin_mention_popup_snapshot() {
54385500
snapshot_composer_state("plugin_mention_popup", false, |composer| {

‎codex-rs/tui_app_server/src/bottom_pane/file_search_popup.rs‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ impl FileSearchPopup {
7070
}
7171

7272
self.display_query = query.to_string();
73-
self.matches = matches;
73+
self.matches = matches.into_iter().take(MAX_POPUP_ROWS).collect();
7474
self.waiting = false;
7575
let len = self.matches.len();
7676
self.state.clamp_selection(len);
@@ -152,3 +152,33 @@ impl WidgetRef for &FileSearchPopup {
152152
);
153153
}
154154
}
155+
156+
#[cfg(test)]
157+
mod tests {
158+
use super::*;
159+
use codex_file_search::MatchType;
160+
use pretty_assertions::assert_eq;
161+
162+
fn file_match(index: usize) -> FileMatch {
163+
FileMatch {
164+
score: index as u32,
165+
path: PathBuf::from(format!("src/file_{index:02}.rs")),
166+
match_type: MatchType::File,
167+
root: PathBuf::from("/tmp/repo"),
168+
indices: None,
169+
}
170+
}
171+
172+
#[test]
173+
fn set_matches_keeps_only_the_first_page_of_results() {
174+
let mut popup = FileSearchPopup::new();
175+
popup.set_query("file");
176+
popup.set_matches("file", (0..(MAX_POPUP_ROWS + 2)).map(file_match).collect());
177+
178+
assert_eq!(
179+
popup.matches,
180+
(0..MAX_POPUP_ROWS).map(file_match).collect::<Vec<_>>()
181+
);
182+
assert_eq!(popup.calculate_required_height(), MAX_POPUP_ROWS as u16);
183+
}
184+
}

0 commit comments

Comments
 (0)