diff --git a/codex-rs/core/src/skills/injection.rs b/codex-rs/core/src/skills/injection.rs index 77b3ceea10a..19ccdb2078e 100644 --- a/codex-rs/core/src/skills/injection.rs +++ b/codex-rs/core/src/skills/injection.rs @@ -78,14 +78,17 @@ fn emit_skill_injected_metric(otel: Option<&OtelManager>, skill: &SkillMetadata, ); } -/// Collect explicitly mentioned skills from `$name` text mentions. +/// Collect explicitly mentioned skills from structured and text mentions. /// -/// Text inputs are scanned once to extract `$skill-name` tokens, then we iterate `skills` -/// in their existing order to preserve prior ordering semantics. Explicit links are -/// resolved by path and plain names are only used when the match is unambiguous. +/// Structured `UserInput::Skill` selections are resolved first by path against +/// enabled skills. Text inputs are then scanned to extract `$skill-name` tokens, and we +/// iterate `skills` in their existing order to preserve prior ordering semantics. +/// Explicit links are resolved by path and plain names are only used when the match +/// is unambiguous. /// -/// Complexity: `O(S + T + N_t * S)` time, `O(S)` space, where: -/// `S` = number of skills, `T` = total text length, `N_t` = number of text inputs. +/// Complexity: `O(T + (N_s + N_t) * S)` time, `O(S + M)` space, where: +/// `S` = number of skills, `T` = total text length, `N_s` = number of structured skill inputs, +/// `N_t` = number of text inputs, `M` = max mentions parsed from a single text input. pub(crate) fn collect_explicit_skill_mentions( inputs: &[UserInput], skills: &[SkillMetadata], @@ -102,12 +105,33 @@ pub(crate) fn collect_explicit_skill_mentions( let mut selected: Vec = Vec::new(); let mut seen_names: HashSet = HashSet::new(); let mut seen_paths: HashSet = HashSet::new(); + let mut blocked_plain_names: HashSet = HashSet::new(); + + for input in inputs { + if let UserInput::Skill { name, path } = input { + blocked_plain_names.insert(name.clone()); + if selection_context.disabled_paths.contains(path) || seen_paths.contains(path) { + continue; + } + + if let Some(skill) = selection_context + .skills + .iter() + .find(|skill| skill.path.as_path() == path.as_path()) + { + seen_paths.insert(skill.path.clone()); + seen_names.insert(skill.name.clone()); + selected.push(skill.clone()); + } + } + } for input in inputs { if let UserInput::Text { text, .. } = input { let mentioned_names = extract_tool_mentions(text); select_skills_from_mentions( &selection_context, + &blocked_plain_names, &mentioned_names, &mut seen_names, &mut seen_paths, @@ -254,6 +278,7 @@ pub(crate) fn extract_tool_mentions(text: &str) -> ToolMentions<'_> { /// Select mentioned skills while preserving the order of `skills`. fn select_skills_from_mentions( selection_context: &SkillSelectionContext<'_>, + blocked_plain_names: &HashSet, mentions: &ToolMentions<'_>, seen_names: &mut HashSet, seen_paths: &mut HashSet, @@ -296,6 +321,9 @@ fn select_skills_from_mentions( continue; } + if blocked_plain_names.contains(skill.name.as_str()) { + continue; + } if !mentions.plain_names.contains(skill.name.as_str()) { continue; } @@ -586,10 +614,10 @@ mod tests { } #[test] - fn collect_explicit_skill_mentions_ignores_structured_inputs() { + fn collect_explicit_skill_mentions_prioritizes_structured_inputs() { let alpha = make_skill("alpha-skill", "/tmp/alpha"); let beta = make_skill("beta-skill", "/tmp/beta"); - let skills = vec![alpha.clone(), beta]; + let skills = vec![alpha.clone(), beta.clone()]; let inputs = vec![ UserInput::Text { text: "please run $alpha-skill".to_string(), @@ -604,7 +632,50 @@ mod tests { let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts); - assert_eq!(selected, vec![alpha]); + assert_eq!(selected, vec![beta, alpha]); + } + + #[test] + fn collect_explicit_skill_mentions_skips_invalid_structured_and_blocks_plain_fallback() { + let alpha = make_skill("alpha-skill", "/tmp/alpha"); + let skills = vec![alpha]; + let inputs = vec![ + UserInput::Text { + text: "please run $alpha-skill".to_string(), + text_elements: Vec::new(), + }, + UserInput::Skill { + name: "alpha-skill".to_string(), + path: PathBuf::from("/tmp/missing"), + }, + ]; + let connector_counts = HashMap::new(); + + let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts); + + assert_eq!(selected, Vec::new()); + } + + #[test] + fn collect_explicit_skill_mentions_skips_disabled_structured_and_blocks_plain_fallback() { + let alpha = make_skill("alpha-skill", "/tmp/alpha"); + let skills = vec![alpha]; + let inputs = vec![ + UserInput::Text { + text: "please run $alpha-skill".to_string(), + text_elements: Vec::new(), + }, + UserInput::Skill { + name: "alpha-skill".to_string(), + path: PathBuf::from("/tmp/alpha"), + }, + ]; + let disabled = HashSet::from([PathBuf::from("/tmp/alpha")]); + let connector_counts = HashMap::new(); + + let selected = collect_mentions(&inputs, &skills, &disabled, &connector_counts); + + assert_eq!(selected, Vec::new()); } #[test] diff --git a/codex-rs/core/tests/suite/live_reload.rs b/codex-rs/core/tests/suite/live_reload.rs index e46c77128f0..0ebcb332efb 100644 --- a/codex-rs/core/tests/suite/live_reload.rs +++ b/codex-rs/core/tests/suite/live_reload.rs @@ -101,7 +101,7 @@ async fn live_skills_reload_refreshes_skill_cache_after_skill_change() -> Result }); let test = builder.build(&server).await?; - let skill_path = std::fs::canonicalize(test.codex_home_path().join("skills/demo/SKILL.md"))?; + let skill_path = dunce::canonicalize(test.codex_home_path().join("skills/demo/SKILL.md"))?; submit_skill_turn(&test, skill_path.clone(), "please use $demo").await?; let first_request = responses diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 4afbfea01ab..5ce5ebafb4d 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -157,6 +157,7 @@ use crate::app_event::AppEvent; use crate::app_event::ConnectorsSnapshot; use crate::app_event_sender::AppEventSender; use crate::bottom_pane::LocalImageAttachment; +use crate::bottom_pane::MentionBinding; use crate::bottom_pane::textarea::TextArea; use crate::bottom_pane::textarea::TextAreaState; use crate::clipboard_paste::normalize_pasted_path; @@ -290,7 +291,8 @@ pub(crate) struct ChatComposer { skills: Option>, connectors_snapshot: Option, dismissed_mention_popup_token: Option, - mention_paths: HashMap, + mention_bindings: HashMap, + recent_submission_mention_bindings: Vec, /// When enabled, `Enter` submits immediately and `Tab` requests queuing behavior. steer_enabled: bool, collaboration_modes_enabled: bool, @@ -309,6 +311,12 @@ struct FooterFlash { expires_at: Instant, } +#[derive(Clone, Debug)] +struct ComposerMentionBinding { + mention: String, + path: String, +} + /// Popup state – at most one can be visible at any time. enum ActivePopup { None, @@ -382,7 +390,8 @@ impl ChatComposer { skills: None, connectors_snapshot: None, dismissed_mention_popup_token: None, - mention_paths: HashMap::new(), + mention_bindings: HashMap::new(), + recent_submission_mention_bindings: Vec::new(), steer_enabled: false, collaboration_modes_enabled: false, config, @@ -414,8 +423,21 @@ impl ChatComposer { self.connectors_snapshot = connectors_snapshot; } - pub(crate) fn take_mention_paths(&mut self) -> HashMap { - std::mem::take(&mut self.mention_paths) + pub(crate) fn take_mention_bindings(&mut self) -> Vec { + let elements = self.current_mention_elements(); + let mut ordered = Vec::new(); + for (id, mention) in elements { + if let Some(binding) = self.mention_bindings.remove(&id) + && binding.mention == mention + { + ordered.push(MentionBinding { + mention: binding.mention, + path: binding.path, + }); + } + } + self.mention_bindings.clear(); + ordered } /// Enables or disables "Steer" behavior for submission keys. @@ -518,7 +540,12 @@ impl ChatComposer { }; // Persistent ↑/↓ history is text-only (backwards-compatible and avoids persisting // attachments), but local in-session ↑/↓ history can rehydrate elements and image paths. - self.set_text_content(entry.text, entry.text_elements, entry.local_image_paths); + self.set_text_content_with_mention_bindings( + entry.text, + entry.text_elements, + entry.local_image_paths, + entry.mention_bindings, + ); true } @@ -732,41 +759,41 @@ impl ChatComposer { /// This is the "fresh draft" path: it clears pending paste payloads and /// mention link targets. Callers restoring a previously submitted draft /// that must keep `$name -> path` resolution should use - /// [`Self::set_text_content_with_mention_paths`] instead. + /// [`Self::set_text_content_with_mention_bindings`] instead. pub(crate) fn set_text_content( &mut self, text: String, text_elements: Vec, local_image_paths: Vec, ) { - self.set_text_content_with_mention_paths( + self.set_text_content_with_mention_bindings( text, text_elements, local_image_paths, - HashMap::new(), + Vec::new(), ); } /// Replace the entire composer content while restoring mention link targets. /// /// Mention popup insertion stores both visible text (for example `$file`) - /// and hidden `mention_paths` used to resolve the canonical target during + /// and hidden mention bindings used to resolve the canonical target during /// submission. Use this method when restoring an interrupted or blocked /// draft; if callers restore only text and images, mentions can appear /// intact to users while resolving to the wrong target or dropping on /// retry. - pub(crate) fn set_text_content_with_mention_paths( + pub(crate) fn set_text_content_with_mention_bindings( &mut self, text: String, text_elements: Vec, local_image_paths: Vec, - mention_paths: HashMap, + mention_bindings: Vec, ) { // Clear any existing content, placeholders, and attachments first. self.textarea.set_text_clearing_elements(""); self.pending_pastes.clear(); self.attached_images.clear(); - self.mention_paths = mention_paths; + self.mention_bindings.clear(); self.textarea.set_text_with_elements(&text, &text_elements); @@ -782,6 +809,8 @@ impl ChatComposer { } } + self.bind_mentions_from_snapshot(mention_bindings); + self.textarea.set_cursor(0); self.sync_popups(); } @@ -808,12 +837,14 @@ impl ChatComposer { .iter() .map(|img| img.path.clone()) .collect(); + let mention_bindings = self.snapshot_mention_bindings(); self.set_text_content(String::new(), Vec::new(), Vec::new()); self.history.reset_navigation(); self.history.record_local_submission(HistoryEntry { text: previous.clone(), text_elements, local_image_paths, + mention_bindings, }); Some(previous) } @@ -845,6 +876,14 @@ impl ChatComposer { .collect() } + pub(crate) fn mention_bindings(&self) -> Vec { + self.snapshot_mention_bindings() + } + + pub(crate) fn take_recent_submission_mention_bindings(&mut self) -> Vec { + std::mem::take(&mut self.recent_submission_mention_bindings) + } + fn prune_attached_images_for_submission(&mut self, text: &str, text_elements: &[TextElement]) { if self.attached_images.is_empty() { return; @@ -1476,10 +1515,7 @@ impl ChatComposer { if close_popup { if let Some((insert_text, path)) = selected_mention { - if let Some(path) = path.as_deref() { - self.record_mention_path(&insert_text, path); - } - self.insert_selected_mention(&insert_text); + self.insert_selected_mention(&insert_text, path.as_deref()); } self.active_popup = ActivePopup::None; } @@ -1786,7 +1822,7 @@ impl ChatComposer { self.textarea.set_cursor(new_cursor); } - fn insert_selected_mention(&mut self, insert_text: &str) { + fn insert_selected_mention(&mut self, insert_text: &str, path: Option<&str>) { let cursor_offset = self.textarea.cursor(); let text = self.textarea.text(); let safe_cursor = Self::clamp_to_char_boundary(text, cursor_offset); @@ -1807,28 +1843,30 @@ impl ChatComposer { .unwrap_or(after_cursor.len()); let end_idx = safe_cursor + end_rel_idx; - let inserted = insert_text.to_string(); + // Remove the active token and insert the selected mention as an atomic element. + self.textarea.replace_range(start_idx..end_idx, ""); + self.textarea.set_cursor(start_idx); + let id = self.textarea.insert_element(insert_text); - let mut new_text = - String::with_capacity(text.len() - (end_idx - start_idx) + inserted.len() + 1); - new_text.push_str(&text[..start_idx]); - new_text.push_str(&inserted); - new_text.push(' '); - new_text.push_str(&text[end_idx..]); + if let (Some(path), Some(mention)) = + (path, Self::mention_name_from_insert_text(insert_text)) + { + self.mention_bindings.insert( + id, + ComposerMentionBinding { + mention, + path: path.to_string(), + }, + ); + } - // Mention insertion rebuilds plain text, so drop existing elements. - self.textarea.set_text_clearing_elements(&new_text); - let new_cursor = start_idx.saturating_add(inserted.len()).saturating_add(1); + self.textarea.insert_str(" "); + let new_cursor = start_idx + .saturating_add(insert_text.len()) + .saturating_add(1); self.textarea.set_cursor(new_cursor); } - fn record_mention_path(&mut self, insert_text: &str, path: &str) { - let Some(name) = Self::mention_name_from_insert_text(insert_text) else { - return; - }; - self.mention_paths.insert(name, path.to_string()); - } - fn mention_name_from_insert_text(insert_text: &str) -> Option { let name = insert_text.strip_prefix('$')?; if name.is_empty() { @@ -1845,6 +1883,67 @@ impl ChatComposer { } } + fn current_mention_elements(&self) -> Vec<(u64, String)> { + self.textarea + .text_element_snapshots() + .into_iter() + .filter_map(|snapshot| { + Self::mention_name_from_insert_text(snapshot.text.as_str()) + .map(|mention| (snapshot.id, mention)) + }) + .collect() + } + + fn snapshot_mention_bindings(&self) -> Vec { + let mut ordered = Vec::new(); + for (id, mention) in self.current_mention_elements() { + if let Some(binding) = self.mention_bindings.get(&id) + && binding.mention == mention + { + ordered.push(MentionBinding { + mention: binding.mention.clone(), + path: binding.path.clone(), + }); + } + } + ordered + } + + fn bind_mentions_from_snapshot(&mut self, mention_bindings: Vec) { + self.mention_bindings.clear(); + if mention_bindings.is_empty() { + return; + } + + let text = self.textarea.text().to_string(); + let mut scan_from = 0usize; + for binding in mention_bindings { + let token = format!("${}", binding.mention); + let Some(range) = + find_next_mention_token_range(text.as_str(), token.as_str(), scan_from) + else { + continue; + }; + + let id = if let Some(id) = self.textarea.add_element_range(range.clone()) { + Some(id) + } else { + self.textarea.element_id_for_exact_range(range.clone()) + }; + + if let Some(id) = id { + self.mention_bindings.insert( + id, + ComposerMentionBinding { + mention: binding.mention, + path: binding.path, + }, + ); + scan_from = range.end; + } + } + } + /// Prepare text for submission/queuing. Returns None if submission should be suppressed. /// On success, clears pending paste payloads because placeholders have been expanded. /// @@ -1856,6 +1955,7 @@ impl ChatComposer { let mut text = self.textarea.text().to_string(); let original_input = text.clone(); let original_text_elements = self.textarea.text_elements(); + let original_mention_bindings = self.snapshot_mention_bindings(); let original_local_image_paths = self .attached_images .iter() @@ -1864,6 +1964,7 @@ impl ChatComposer { let original_pending_pastes = self.pending_pastes.clone(); let mut text_elements = original_text_elements.clone(); let input_starts_with_space = original_input.starts_with(' '); + self.recent_submission_mention_bindings.clear(); self.textarea.set_text_clearing_elements(""); if !self.pending_pastes.is_empty() { @@ -1909,10 +2010,11 @@ impl ChatComposer { self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new( history_cell::new_info_event(message, None), ))); - self.set_text_content( + self.set_text_content_with_mention_bindings( original_input.clone(), original_text_elements, original_local_image_paths, + original_mention_bindings, ); self.pending_pastes.clone_from(&original_pending_pastes); self.textarea.set_cursor(original_input.len()); @@ -1929,10 +2031,11 @@ impl ChatComposer { self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new( history_cell::new_error_event(err.user_message()), ))); - self.set_text_content( + self.set_text_content_with_mention_bindings( original_input.clone(), original_text_elements, original_local_image_paths, + original_mention_bindings, ); self.pending_pastes.clone_from(&original_pending_pastes); self.textarea.set_cursor(original_input.len()); @@ -1950,6 +2053,7 @@ impl ChatComposer { if text.is_empty() && self.attached_images.is_empty() { return None; } + self.recent_submission_mention_bindings = original_mention_bindings.clone(); if record_history && (!text.is_empty() || !self.attached_images.is_empty()) { let local_image_paths = self .attached_images @@ -1960,6 +2064,7 @@ impl ChatComposer { text: text.clone(), text_elements: text_elements.clone(), local_image_paths, + mention_bindings: original_mention_bindings, }); } self.pending_pastes.clear(); @@ -2022,6 +2127,7 @@ impl ChatComposer { let original_input = self.textarea.text().to_string(); let original_text_elements = self.textarea.text_elements(); + let original_mention_bindings = self.snapshot_mention_bindings(); let original_local_image_paths = self .attached_images .iter() @@ -2053,10 +2159,11 @@ impl ChatComposer { } } else { // Restore text if submission was suppressed. - self.set_text_content( + self.set_text_content_with_mention_bindings( original_input, original_text_elements, original_local_image_paths, + original_mention_bindings, ); self.pending_pastes = original_pending_pastes; (InputResult::None, true) @@ -2245,10 +2352,11 @@ impl ChatComposer { _ => unreachable!(), }; if let Some(entry) = replace_entry { - self.set_text_content( + self.set_text_content_with_mention_bindings( entry.text, entry.text_elements, entry.local_image_paths, + entry.mention_bindings, ); return (InputResult::None, true); } @@ -2930,6 +3038,8 @@ impl ChatComposer { insert_text: format!("${skill_name}"), search_terms, path: Some(skill.path.to_string_lossy().into_owned()), + category_tag: (skill.scope == codex_core::protocol::SkillScope::Repo) + .then(|| "[Repo]".to_string()), }); } } @@ -2952,23 +3062,26 @@ impl ChatComposer { insert_text: format!("${slug}"), search_terms, path: Some(format!("app://{connector_id}")), + category_tag: Some("[App]".to_string()), }); } } + let mut counts: HashMap = HashMap::new(); + for mention in &mentions { + *counts.entry(mention.insert_text.clone()).or_insert(0) += 1; + } + for mention in &mut mentions { + if counts.get(&mention.insert_text).copied().unwrap_or(0) <= 1 { + mention.category_tag = None; + } + } + mentions } fn connector_brief_description(connector: &AppInfo) -> String { - let status_label = if connector.is_accessible { - "Connected" - } else { - "Can be installed" - }; - match Self::connector_description(connector) { - Some(description) => format!("{status_label} - {description}"), - None => status_label.to_string(), - } + Self::connector_description(connector).unwrap_or_default() } fn connector_description(connector: &AppInfo) -> Option { @@ -3049,6 +3162,42 @@ fn is_mention_name_char(byte: u8) -> bool { matches!(byte, b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' | b'_' | b'-') } +fn find_next_mention_token_range(text: &str, token: &str, from: usize) -> Option> { + if token.is_empty() || from >= text.len() { + return None; + } + let bytes = text.as_bytes(); + let token_bytes = token.as_bytes(); + let mut index = from; + + while index < bytes.len() { + if bytes[index] != b'$' { + index += 1; + continue; + } + + let end = index.saturating_add(token_bytes.len()); + if end > bytes.len() { + return None; + } + if &bytes[index..end] != token_bytes { + index += 1; + continue; + } + + if bytes + .get(end) + .is_none_or(|byte| !is_mention_name_char(*byte)) + { + return Some(index..end); + } + + index = end; + } + + None +} + impl Renderable for ChatComposer { fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> { if !self.input_enabled { @@ -5513,6 +5662,40 @@ mod tests { assert_eq!(vec![path], imgs); } + #[test] + fn submit_captures_recent_mention_bindings_before_clearing_textarea() { + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + composer.set_steer_enabled(true); + + let mention_bindings = vec![MentionBinding { + mention: "figma".to_string(), + path: "/tmp/user/figma/SKILL.md".to_string(), + }]; + composer.set_text_content_with_mention_bindings( + "$figma please".to_string(), + Vec::new(), + Vec::new(), + mention_bindings.clone(), + ); + + let (result, _) = + composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + assert!(matches!(result, InputResult::Submitted { .. })); + assert_eq!( + composer.take_recent_submission_mention_bindings(), + mention_bindings + ); + assert!(composer.take_mention_bindings().is_empty()); + } + #[test] fn history_navigation_restores_image_attachments() { let (tx, _rx) = unbounded_channel::(); diff --git a/codex-rs/tui/src/bottom_pane/chat_composer_history.rs b/codex-rs/tui/src/bottom_pane/chat_composer_history.rs index da9f46ae4b8..6d9322cd194 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer_history.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer_history.rs @@ -3,6 +3,8 @@ use std::path::PathBuf; use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; +use crate::bottom_pane::MentionBinding; +use crate::mention_codec::decode_history_mentions; use codex_core::protocol::Op; use codex_protocol::user_input::TextElement; @@ -11,6 +13,7 @@ pub(crate) struct HistoryEntry { pub(crate) text: String, pub(crate) text_elements: Vec, pub(crate) local_image_paths: Vec, + pub(crate) mention_bindings: Vec, } impl HistoryEntry { @@ -19,14 +22,24 @@ impl HistoryEntry { text: String::new(), text_elements: Vec::new(), local_image_paths: Vec::new(), + mention_bindings: Vec::new(), } } pub(crate) fn from_text(text: String) -> Self { + let decoded = decode_history_mentions(&text); Self { - text, + text: decoded.text, text_elements: Vec::new(), local_image_paths: Vec::new(), + mention_bindings: decoded + .mentions + .into_iter() + .map(|mention| MentionBinding { + mention: mention.mention, + path: mention.path, + }) + .collect(), } } } diff --git a/codex-rs/tui/src/bottom_pane/command_popup.rs b/codex-rs/tui/src/bottom_pane/command_popup.rs index 928fb541b3b..48cf255ee04 100644 --- a/codex-rs/tui/src/bottom_pane/command_popup.rs +++ b/codex-rs/tui/src/bottom_pane/command_popup.rs @@ -220,6 +220,7 @@ impl CommandPopup { match_indices: indices.map(|v| v.into_iter().map(|i| i + 1).collect()), display_shortcut: None, description: Some(description), + category_tag: None, wrap_indent: None, is_disabled: false, disabled_reason: None, diff --git a/codex-rs/tui/src/bottom_pane/file_search_popup.rs b/codex-rs/tui/src/bottom_pane/file_search_popup.rs index 3e6ef814a22..9eb8121b693 100644 --- a/codex-rs/tui/src/bottom_pane/file_search_popup.rs +++ b/codex-rs/tui/src/bottom_pane/file_search_popup.rs @@ -125,6 +125,7 @@ impl WidgetRef for &FileSearchPopup { .map(|v| v.iter().map(|&i| i as usize).collect()), display_shortcut: None, description: None, + category_tag: None, wrap_indent: None, is_disabled: false, disabled_reason: None, diff --git a/codex-rs/tui/src/bottom_pane/list_selection_view.rs b/codex-rs/tui/src/bottom_pane/list_selection_view.rs index 9bc046f0100..3cf74e3a1a2 100644 --- a/codex-rs/tui/src/bottom_pane/list_selection_view.rs +++ b/codex-rs/tui/src/bottom_pane/list_selection_view.rs @@ -258,6 +258,7 @@ impl ListSelectionView { display_shortcut: item.display_shortcut, match_indices: None, description, + category_tag: None, wrap_indent, is_disabled, disabled_reason: item.disabled_reason.clone(), diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index b9c77fc72d1..960e67fed61 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -13,7 +13,6 @@ //! //! Some UI is time-based rather than input-based, such as the transient "press again to quit" //! hint. The pane schedules redraws so those hints can expire even when the UI is otherwise idle. -use std::collections::HashMap; use std::path::PathBuf; use crate::app_event::ConnectorsSnapshot; @@ -55,6 +54,14 @@ pub(crate) struct LocalImageAttachment { pub(crate) placeholder: String, pub(crate) path: PathBuf, } + +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct MentionBinding { + /// Mention token text without the leading `$`. + pub(crate) mention: String, + /// Canonical mention target (for example `app://...` or absolute SKILL.md path). + pub(crate) path: String, +} mod chat_composer; mod chat_composer_history; mod command_popup; @@ -228,14 +235,19 @@ impl BottomPane { self.request_redraw(); } - pub fn take_mention_paths(&mut self) -> HashMap { - self.composer.take_mention_paths() + pub fn take_mention_bindings(&mut self) -> Vec { + self.composer.take_mention_bindings() } - /// Clear pending attachments and mention paths e.g. when a slash command doesn't submit text. + pub fn take_recent_submission_mention_bindings(&mut self) -> Vec { + self.composer.take_recent_submission_mention_bindings() + } + + /// Clear pending attachments and mention bindings e.g. when a slash command doesn't submit text. pub(crate) fn drain_pending_submission_state(&mut self) { let _ = self.take_recent_submission_images_with_placeholders(); - let _ = self.take_mention_paths(); + let _ = self.take_recent_submission_mention_bindings(); + let _ = self.take_mention_bindings(); } pub fn set_steer_enabled(&mut self, enabled: bool) { @@ -419,7 +431,7 @@ impl BottomPane { /// /// This is intended for fresh input where mention linkage does not need to /// survive; it routes to `ChatComposer::set_text_content`, which resets - /// `mention_paths`. + /// mention bindings. pub(crate) fn set_composer_text( &mut self, text: String, @@ -437,18 +449,18 @@ impl BottomPane { /// Use this when rehydrating a draft after a local validation/gating /// failure (for example unsupported image submit) so previously selected /// mention targets remain stable across retry. - pub(crate) fn set_composer_text_with_mention_paths( + pub(crate) fn set_composer_text_with_mention_bindings( &mut self, text: String, text_elements: Vec, local_image_paths: Vec, - mention_paths: HashMap, + mention_bindings: Vec, ) { - self.composer.set_text_content_with_mention_paths( + self.composer.set_text_content_with_mention_bindings( text, text_elements, local_image_paths, - mention_paths, + mention_bindings, ); self.request_redraw(); } @@ -481,6 +493,10 @@ impl BottomPane { self.composer.local_images() } + pub(crate) fn composer_mention_bindings(&self) -> Vec { + self.composer.mention_bindings() + } + #[cfg(test)] pub(crate) fn composer_local_image_paths(&self) -> Vec { self.composer.local_image_paths() diff --git a/codex-rs/tui/src/bottom_pane/selection_popup_common.rs b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs index 3f827dc41f8..6377bedb954 100644 --- a/codex-rs/tui/src/bottom_pane/selection_popup_common.rs +++ b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs @@ -30,6 +30,7 @@ pub(crate) struct GenericDisplayRow { pub display_shortcut: Option, pub match_indices: Option>, // indices to bold (char positions) pub description: Option, // optional grey text after the name + pub category_tag: Option, // optional right-side category label pub disabled_reason: Option, // optional disabled message pub is_disabled: bool, pub wrap_indent: Option, // optional indent for wrapped lines @@ -337,6 +338,10 @@ fn build_full_line(row: &GenericDisplayRow, desc_col: usize) -> Line<'static> { } full_spans.push(desc.clone().dim()); } + if let Some(tag) = row.category_tag.as_deref().filter(|tag| !tag.is_empty()) { + full_spans.push(" ".into()); + full_spans.push(tag.to_string().dim()); + } Line::from(full_spans) } diff --git a/codex-rs/tui/src/bottom_pane/skill_popup.rs b/codex-rs/tui/src/bottom_pane/skill_popup.rs index 390b44295f2..6ce8d4e373d 100644 --- a/codex-rs/tui/src/bottom_pane/skill_popup.rs +++ b/codex-rs/tui/src/bottom_pane/skill_popup.rs @@ -24,8 +24,11 @@ pub(crate) struct MentionItem { pub(crate) insert_text: String, pub(crate) search_terms: Vec, pub(crate) path: Option, + pub(crate) category_tag: Option, } +const MENTION_NAME_TRUNCATE_LEN: usize = 24; + pub(crate) struct SkillPopup { query: String, mentions: Vec, @@ -94,13 +97,14 @@ impl SkillPopup { .into_iter() .map(|(idx, indices, _score)| { let mention = &self.mentions[idx]; - let name = truncate_text(&mention.display_name, 21); + let name = truncate_text(&mention.display_name, MENTION_NAME_TRUNCATE_LEN); let description = mention.description.clone().unwrap_or_default(); GenericDisplayRow { name, match_indices: indices, display_shortcut: None, description: Some(description).filter(|desc| !desc.is_empty()), + category_tag: mention.category_tag.clone(), is_disabled: false, disabled_reason: None, wrap_indent: None, diff --git a/codex-rs/tui/src/bottom_pane/textarea.rs b/codex-rs/tui/src/bottom_pane/textarea.rs index a90d5b87c68..2caa45602f5 100644 --- a/codex-rs/tui/src/bottom_pane/textarea.rs +++ b/codex-rs/tui/src/bottom_pane/textarea.rs @@ -25,9 +25,17 @@ fn is_word_separator(ch: char) -> bool { #[derive(Debug, Clone)] struct TextElement { + id: u64, range: Range, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct TextElementSnapshot { + pub(crate) id: u64, + pub(crate) range: Range, + pub(crate) text: String, +} + #[derive(Debug)] pub(crate) struct TextArea { text: String, @@ -35,6 +43,7 @@ pub(crate) struct TextArea { wrap_cache: RefCell>, preferred_col: Option, elements: Vec, + next_element_id: u64, kill_buffer: String, } @@ -58,6 +67,7 @@ impl TextArea { wrap_cache: RefCell::new(None), preferred_col: None, elements: Vec::new(), + next_element_id: 1, kill_buffer: String::new(), } } @@ -87,7 +97,11 @@ impl TextArea { if start >= end { continue; } - self.elements.push(TextElement { range: start..end }); + let id = self.next_element_id(); + self.elements.push(TextElement { + id, + range: start..end, + }); } self.elements.sort_by_key(|e| e.range.start); } @@ -766,6 +780,28 @@ impl TextArea { .collect() } + pub(crate) fn text_element_snapshots(&self) -> Vec { + self.elements + .iter() + .filter_map(|element| { + self.text + .get(element.range.clone()) + .map(|text| TextElementSnapshot { + id: element.id, + range: element.range.clone(), + text: text.to_string(), + }) + }) + .collect() + } + + pub(crate) fn element_id_for_exact_range(&self, range: Range) -> Option { + self.elements + .iter() + .find(|element| element.range == range) + .map(|element| element.id) + } + /// Renames a single text element in-place, keeping it atomic. /// /// Use this when the element payload is an identifier (e.g. a placeholder) that must be @@ -835,41 +871,47 @@ impl TextArea { true } - pub fn insert_element(&mut self, text: &str) { + pub fn insert_element(&mut self, text: &str) -> u64 { let start = self.clamp_pos_for_insertion(self.cursor_pos); self.insert_str_at(start, text); let end = start + text.len(); - self.add_element(start..end); + let id = self.add_element(start..end); // Place cursor at end of inserted element self.set_cursor(end); + id } /// Mark an existing text range as an atomic element without changing the text. /// /// This is used to convert already-typed tokens (like `/plan`) into elements /// so they render and edit atomically. Overlapping or duplicate ranges are ignored. - pub fn add_element_range(&mut self, range: Range) { + pub fn add_element_range(&mut self, range: Range) -> Option { let start = self.clamp_pos_to_char_boundary(range.start.min(self.text.len())); let end = self.clamp_pos_to_char_boundary(range.end.min(self.text.len())); if start >= end { - return; + return None; } if self .elements .iter() .any(|e| e.range.start == start && e.range.end == end) { - return; + return None; } if self .elements .iter() .any(|e| start < e.range.end && end > e.range.start) { - return; + return None; } - self.elements.push(TextElement { range: start..end }); + let id = self.next_element_id(); + self.elements.push(TextElement { + id, + range: start..end, + }); self.elements.sort_by_key(|e| e.range.start); + Some(id) } pub fn remove_element_range(&mut self, range: Range) -> bool { @@ -884,10 +926,18 @@ impl TextArea { len_before != self.elements.len() } - fn add_element(&mut self, range: Range) { - let elem = TextElement { range }; + fn add_element(&mut self, range: Range) -> u64 { + let id = self.next_element_id(); + let elem = TextElement { id, range }; self.elements.push(elem); self.elements.sort_by_key(|e| e.range.start); + id + } + + fn next_element_id(&mut self) -> u64 { + let id = self.next_element_id; + self.next_element_id = self.next_element_id.saturating_add(1); + id } fn find_element_containing(&self, pos: usize) -> Option { diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 43755c1f41d..19e7ff791c1 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -164,6 +164,7 @@ use crate::bottom_pane::ExperimentalFeaturesView; use crate::bottom_pane::FeedbackAudience; use crate::bottom_pane::InputResult; use crate::bottom_pane::LocalImageAttachment; +use crate::bottom_pane::MentionBinding; use crate::bottom_pane::QUIT_SHORTCUT_TIMEOUT; use crate::bottom_pane::SelectionAction; use crate::bottom_pane::SelectionItem; @@ -210,6 +211,8 @@ mod skills; use self::skills::collect_tool_mentions; use self::skills::find_app_mentions; use self::skills::find_skill_mentions_with_tool_mentions; +use crate::mention_codec::LinkedMention; +use crate::mention_codec::encode_history_mentions; use crate::streaming::chunking::AdaptiveChunkingPolicy; use crate::streaming::commit_tick::CommitTickScope; use crate::streaming::commit_tick::run_commit_tick; @@ -639,7 +642,7 @@ pub(crate) struct UserMessage { text: String, local_images: Vec, text_elements: Vec, - mention_paths: HashMap, + mention_bindings: Vec, } impl From for UserMessage { @@ -649,7 +652,7 @@ impl From for UserMessage { local_images: Vec::new(), // Plain text conversion has no UI element ranges. text_elements: Vec::new(), - mention_paths: HashMap::new(), + mention_bindings: Vec::new(), } } } @@ -661,7 +664,7 @@ impl From<&str> for UserMessage { local_images: Vec::new(), // Plain text conversion has no UI element ranges. text_elements: Vec::new(), - mention_paths: HashMap::new(), + mention_bindings: Vec::new(), } } } @@ -687,7 +690,7 @@ pub(crate) fn create_initial_user_message( text, local_images, text_elements, - mention_paths: HashMap::new(), + mention_bindings: Vec::new(), }) } } @@ -701,14 +704,14 @@ fn remap_placeholders_for_message(message: UserMessage, next_label: &mut usize) text, text_elements, local_images, - mention_paths, + mention_bindings, } = message; if local_images.is_empty() { return UserMessage { text, text_elements, local_images, - mention_paths, + mention_bindings, }; } @@ -763,7 +766,7 @@ fn remap_placeholders_for_message(message: UserMessage, next_label: &mut usize) text: rebuilt, local_images: remapped_images, text_elements: rebuilt_elements, - mention_paths, + mention_bindings, } } @@ -1666,7 +1669,7 @@ impl ChatWidget { text: self.bottom_pane.composer_text(), text_elements: self.bottom_pane.composer_text_elements(), local_images: self.bottom_pane.composer_local_images(), - mention_paths: HashMap::new(), + mention_bindings: self.bottom_pane.composer_mention_bindings(), }; let mut to_merge: Vec = self.queued_user_messages.drain(..).collect(); @@ -1678,7 +1681,7 @@ impl ChatWidget { text: String::new(), text_elements: Vec::new(), local_images: Vec::new(), - mention_paths: HashMap::new(), + mention_bindings: Vec::new(), }; let mut combined_offset = 0usize; let mut next_image_label = 1usize; @@ -1700,7 +1703,7 @@ impl ChatWidget { elem })); combined.local_images.extend(message.local_images); - combined.mention_paths.extend(message.mention_paths); + combined.mention_bindings.extend(message.mention_bindings); } Some(combined) @@ -1711,11 +1714,15 @@ impl ChatWidget { text, local_images, text_elements, - mention_paths: _, + mention_bindings, } = user_message; let local_image_paths = local_images.into_iter().map(|img| img.path).collect(); - self.bottom_pane - .set_composer_text(text, text_elements, local_image_paths); + self.bottom_pane.set_composer_text_with_mention_bindings( + text, + text_elements, + local_image_paths, + mention_bindings, + ); } fn on_plan_update(&mut self, update: UpdatePlanArgs) { @@ -3022,7 +3029,9 @@ impl ChatWidget { .bottom_pane .take_recent_submission_images_with_placeholders(), text_elements, - mention_paths: self.bottom_pane.take_mention_paths(), + mention_bindings: self + .bottom_pane + .take_recent_submission_mention_bindings(), }; if self.is_session_configured() && !self.is_plan_streaming_in_tui() { // Submitted is only emitted when steer is enabled. @@ -3045,7 +3054,9 @@ impl ChatWidget { .bottom_pane .take_recent_submission_images_with_placeholders(), text_elements, - mention_paths: self.bottom_pane.take_mention_paths(), + mention_bindings: self + .bottom_pane + .take_recent_submission_mention_bindings(), }; self.queue_user_message(user_message); } @@ -3414,7 +3425,7 @@ impl ChatWidget { .bottom_pane .take_recent_submission_images_with_placeholders(), text_elements: prepared_elements, - mention_paths: self.bottom_pane.take_mention_paths(), + mention_bindings: self.bottom_pane.take_recent_submission_mention_bindings(), }; if self.is_session_configured() { self.reasoning_buffer.clear(); @@ -3551,13 +3562,18 @@ impl ChatWidget { text, local_images, text_elements, - mention_paths, + mention_bindings, } = user_message; if text.is_empty() && local_images.is_empty() { return; } if !local_images.is_empty() && !self.current_model_supports_images() { - self.restore_blocked_image_submission(text, text_elements, local_images, mention_paths); + self.restore_blocked_image_submission( + text, + text_elements, + local_images, + mention_bindings, + ); return; } @@ -3594,16 +3610,43 @@ impl ChatWidget { }); } - let mentions = collect_tool_mentions(&text, &mention_paths); + let mentions = collect_tool_mentions(&text, &HashMap::new()); + let bound_names: HashSet = mention_bindings + .iter() + .map(|binding| binding.mention.clone()) + .collect(); let mut skill_names_lower: HashSet = HashSet::new(); + let mut selected_skill_paths: HashSet = HashSet::new(); if let Some(skills) = self.bottom_pane.skills() { skill_names_lower = skills .iter() .map(|skill| skill.name.to_ascii_lowercase()) .collect(); + + for binding in &mention_bindings { + let path = binding + .path + .strip_prefix("skill://") + .unwrap_or(binding.path.as_str()); + let path = Path::new(path); + 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.clone(), + }); + } + } + 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.clone()) + { + continue; + } items.push(UserInput::Skill { name: skill.name.clone(), path: skill.path.clone(), @@ -3611,9 +3654,33 @@ impl ChatWidget { } } + let mut selected_app_ids: HashSet = HashSet::new(); if let Some(apps) = self.connectors_for_mentions() { + for binding in &mention_bindings { + let Some(app_id) = binding + .path + .strip_prefix("app://") + .filter(|id| !id.is_empty()) + else { + continue; + }; + if !selected_app_ids.insert(app_id.to_string()) { + continue; + } + if let Some(app) = apps.iter().find(|app| app.id == app_id) { + items.push(UserInput::Mention { + name: app.name.clone(), + path: binding.path.clone(), + }); + } + } + let app_mentions = find_app_mentions(&mentions, apps, &skill_names_lower); for app in app_mentions { + let slug = codex_core::connectors::connector_mention_slug(&app); + if bound_names.contains(&slug) || !selected_app_ids.insert(app.id.clone()) { + continue; + } let app_id = app.id.as_str(); items.push(UserInput::Mention { name: app.name.clone(), @@ -3654,8 +3721,16 @@ impl ChatWidget { // Persist the text to cross-session message history. if !text.is_empty() { + let encoded_mentions = mention_bindings + .iter() + .map(|binding| LinkedMention { + mention: binding.mention.clone(), + path: binding.path.clone(), + }) + .collect::>(); + let history_text = encode_history_mentions(&text, &encoded_mentions); self.codex_op_tx - .send(Op::AddToHistory { text: text.clone() }) + .send(Op::AddToHistory { text: history_text }) .unwrap_or_else(|e| { tracing::error!("failed to send AddHistory op: {e}"); }); @@ -3678,7 +3753,7 @@ impl ChatWidget { /// /// The blocked-image path intentionally keeps the draft in the composer so /// users can remove attachments and retry. We must restore - /// `mention_paths` alongside visible text; restoring only `$name` tokens + /// mention bindings alongside visible text; restoring only `$name` tokens /// makes the draft look correct while degrading mention resolution to /// name-only heuristics on retry. fn restore_blocked_image_submission( @@ -3686,15 +3761,15 @@ impl ChatWidget { text: String, text_elements: Vec, local_images: Vec, - mention_paths: HashMap, + mention_bindings: Vec, ) { // Preserve the user's composed payload so they can retry after changing models. let local_image_paths = local_images.iter().map(|img| img.path.clone()).collect(); - self.bottom_pane.set_composer_text_with_mention_paths( + self.bottom_pane.set_composer_text_with_mention_bindings( text, text_elements, local_image_paths, - mention_paths, + mention_bindings, ); self.add_to_history(history_cell::new_warning_event( self.image_inputs_not_supported_message(), @@ -6436,7 +6511,7 @@ impl ChatWidget { text, local_images: Vec::new(), text_elements: Vec::new(), - mention_paths: HashMap::new(), + mention_bindings: Vec::new(), }; if should_queue { self.queue_user_message(user_message); diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 7610d207577..f338157ee6b 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -10,6 +10,7 @@ use crate::app_event::ExitMode; use crate::app_event_sender::AppEventSender; use crate::bottom_pane::FeedbackAudience; use crate::bottom_pane::LocalImageAttachment; +use crate::bottom_pane::MentionBinding; use crate::history_cell::UserHistoryCell; use crate::test_backend::VT100Backend; use crate::tui::FrameRequester; @@ -61,6 +62,7 @@ use codex_core::protocol::UndoCompletedEvent; use codex_core::protocol::UndoStartedEvent; use codex_core::protocol::ViewImageToolCallEvent; use codex_core::protocol::WarningEvent; +use codex_core::skills::model::SkillMetadata; use codex_otel::OtelManager; use codex_otel::RuntimeMetricsSummary; use codex_protocol::ThreadId; @@ -77,6 +79,7 @@ use codex_protocol::plan_tool::PlanItemArg; use codex_protocol::plan_tool::StepStatus; use codex_protocol::plan_tool::UpdatePlanArgs; use codex_protocol::protocol::CodexErrorInfo; +use codex_protocol::protocol::SkillScope; use codex_protocol::user_input::TextElement; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; @@ -391,7 +394,82 @@ async fn submission_preserves_text_elements_and_local_images() { } #[tokio::test] -async fn blocked_image_restore_preserves_mention_paths() { +async fn submission_prefers_selected_duplicate_skill_path() { + let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(None).await; + + let conversation_id = ThreadId::new(); + let rollout_file = NamedTempFile::new().unwrap(); + let configured = codex_core::protocol::SessionConfiguredEvent { + session_id: conversation_id, + forked_from_id: None, + thread_name: None, + model: "test-model".to_string(), + model_provider_id: "test-provider".to_string(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::ReadOnly, + cwd: PathBuf::from("/home/user/project"), + reasoning_effort: Some(ReasoningEffortConfig::default()), + history_log_id: 0, + history_entry_count: 0, + initial_messages: None, + rollout_path: Some(rollout_file.path().to_path_buf()), + }; + chat.handle_codex_event(Event { + id: "initial".into(), + msg: EventMsg::SessionConfigured(configured), + }); + drain_insert_history(&mut rx); + + let repo_skill_path = PathBuf::from("/tmp/repo/figma/SKILL.md"); + let user_skill_path = PathBuf::from("/tmp/user/figma/SKILL.md"); + chat.set_skills(Some(vec![ + SkillMetadata { + name: "figma".to_string(), + description: "Repo skill".to_string(), + short_description: None, + interface: None, + dependencies: None, + path: repo_skill_path, + scope: SkillScope::Repo, + }, + SkillMetadata { + name: "figma".to_string(), + description: "User skill".to_string(), + short_description: None, + interface: None, + dependencies: None, + path: user_skill_path.clone(), + scope: SkillScope::User, + }, + ])); + + chat.bottom_pane.set_composer_text_with_mention_bindings( + "please use $figma now".to_string(), + Vec::new(), + Vec::new(), + vec![MentionBinding { + mention: "figma".to_string(), + path: user_skill_path.to_string_lossy().into_owned(), + }], + ); + chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + + let items = match next_submit_op(&mut op_rx) { + Op::UserTurn { items, .. } => items, + other => panic!("expected Op::UserTurn, got {other:?}"), + }; + let selected_skill_paths = items + .iter() + .filter_map(|item| match item { + UserInput::Skill { path, .. } => Some(path.clone()), + _ => None, + }) + .collect::>(); + assert_eq!(selected_skill_paths, vec![user_skill_path]); +} + +#[tokio::test] +async fn blocked_image_restore_preserves_mention_bindings() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; let placeholder = "[Image #1]"; @@ -404,23 +482,33 @@ async fn blocked_image_restore_preserves_mention_paths() { placeholder: placeholder.to_string(), path: PathBuf::from("/tmp/blocked.png"), }]; - let mention_paths = - HashMap::from([("file".to_string(), "/tmp/skills/file/SKILL.md".to_string())]); + let mention_bindings = vec![MentionBinding { + mention: "file".to_string(), + path: "/tmp/skills/file/SKILL.md".to_string(), + }]; chat.restore_blocked_image_submission( text.clone(), - text_elements.clone(), + text_elements, local_images.clone(), - mention_paths.clone(), + mention_bindings.clone(), ); + let mention_start = text.find("$file").expect("mention token exists"); + let expected_elements = vec![ + TextElement::new((0..placeholder.len()).into(), Some(placeholder.to_string())), + TextElement::new( + (mention_start..mention_start + "$file".len()).into(), + Some("$file".to_string()), + ), + ]; assert_eq!(chat.bottom_pane.composer_text(), text); - assert_eq!(chat.bottom_pane.composer_text_elements(), text_elements); + assert_eq!(chat.bottom_pane.composer_text_elements(), expected_elements); assert_eq!( chat.bottom_pane.composer_local_image_paths(), vec![local_images[0].path.clone()], ); - assert_eq!(chat.bottom_pane.take_mention_paths(), mention_paths); + assert_eq!(chat.bottom_pane.take_mention_bindings(), mention_bindings); let cells = drain_insert_history(&mut rx); let warning = cells @@ -468,7 +556,7 @@ async fn interrupted_turn_restores_queued_messages_with_images_and_elements() { path: first_images[0].clone(), }], text_elements: first_elements, - mention_paths: HashMap::new(), + mention_bindings: Vec::new(), }); chat.queued_user_messages.push_back(UserMessage { text: second_text, @@ -477,7 +565,7 @@ async fn interrupted_turn_restores_queued_messages_with_images_and_elements() { path: second_images[0].clone(), }], text_elements: second_elements, - mention_paths: HashMap::new(), + mention_bindings: Vec::new(), }); chat.refresh_queued_user_messages(); @@ -545,7 +633,7 @@ async fn interrupted_turn_restore_keeps_active_mode_for_resubmission() { text: "Implement the plan.".to_string(), local_images: Vec::new(), text_elements: Vec::new(), - mention_paths: HashMap::new(), + mention_bindings: Vec::new(), }); chat.refresh_queued_user_messages(); @@ -605,7 +693,7 @@ async fn remap_placeholders_uses_attachment_labels() { text, text_elements: elements, local_images: attachments, - mention_paths: HashMap::new(), + mention_bindings: Vec::new(), }; let mut next_label = 3usize; let remapped = remap_placeholders_for_message(message, &mut next_label); @@ -666,7 +754,7 @@ async fn remap_placeholders_uses_byte_ranges_when_placeholder_missing() { text, text_elements: elements, local_images: attachments, - mention_paths: HashMap::new(), + mention_bindings: Vec::new(), }; let mut next_label = 3usize; let remapped = remap_placeholders_for_message(message, &mut next_label); diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index daabb60273a..9cb7bee210c 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -82,6 +82,7 @@ pub mod live_wrap; mod markdown; mod markdown_render; mod markdown_stream; +mod mention_codec; mod model_migration; mod notifications; pub mod onboarding; diff --git a/codex-rs/tui/src/mention_codec.rs b/codex-rs/tui/src/mention_codec.rs new file mode 100644 index 00000000000..5a7670e1403 --- /dev/null +++ b/codex-rs/tui/src/mention_codec.rs @@ -0,0 +1,240 @@ +use std::collections::HashMap; +use std::collections::VecDeque; + +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct LinkedMention { + pub(crate) mention: String, + pub(crate) path: String, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct DecodedHistoryText { + pub(crate) text: String, + pub(crate) mentions: Vec, +} + +pub(crate) fn encode_history_mentions(text: &str, mentions: &[LinkedMention]) -> String { + if mentions.is_empty() || text.is_empty() { + return text.to_string(); + } + + let mut mentions_by_name: HashMap<&str, VecDeque<&str>> = HashMap::new(); + for mention in mentions { + mentions_by_name + .entry(mention.mention.as_str()) + .or_default() + .push_back(mention.path.as_str()); + } + + let bytes = text.as_bytes(); + let mut out = String::with_capacity(text.len()); + let mut index = 0usize; + + while index < bytes.len() { + if bytes[index] == b'$' { + let name_start = index + 1; + if let Some(first) = bytes.get(name_start) + && is_mention_name_char(*first) + { + let mut name_end = name_start + 1; + while let Some(next) = bytes.get(name_end) + && is_mention_name_char(*next) + { + name_end += 1; + } + + let name = &text[name_start..name_end]; + if let Some(path) = mentions_by_name.get_mut(name).and_then(VecDeque::pop_front) { + out.push('['); + out.push('$'); + out.push_str(name); + out.push_str("]("); + out.push_str(path); + out.push(')'); + index = name_end; + continue; + } + } + } + + let Some(ch) = text[index..].chars().next() else { + break; + }; + out.push(ch); + index += ch.len_utf8(); + } + + out +} + +pub(crate) fn decode_history_mentions(text: &str) -> DecodedHistoryText { + let bytes = text.as_bytes(); + let mut out = String::with_capacity(text.len()); + let mut mentions = Vec::new(); + let mut index = 0usize; + + while index < bytes.len() { + if bytes[index] == b'[' + && let Some((name, path, end_index)) = parse_linked_tool_mention(text, bytes, index) + && !is_common_env_var(name) + && is_tool_path(path) + { + out.push('$'); + out.push_str(name); + mentions.push(LinkedMention { + mention: name.to_string(), + path: path.to_string(), + }); + index = end_index; + continue; + } + + let Some(ch) = text[index..].chars().next() else { + break; + }; + out.push(ch); + index += ch.len_utf8(); + } + + DecodedHistoryText { + text: out, + mentions, + } +} + +fn parse_linked_tool_mention<'a>( + text: &'a str, + text_bytes: &[u8], + start: usize, +) -> Option<(&'a str, &'a str, usize)> { + let dollar_index = start + 1; + if text_bytes.get(dollar_index) != Some(&b'$') { + return None; + } + + let name_start = dollar_index + 1; + let first_name_byte = text_bytes.get(name_start)?; + if !is_mention_name_char(*first_name_byte) { + return None; + } + + let mut name_end = name_start + 1; + while let Some(next_byte) = text_bytes.get(name_end) + && is_mention_name_char(*next_byte) + { + name_end += 1; + } + + if text_bytes.get(name_end) != Some(&b']') { + return None; + } + + let mut path_start = name_end + 1; + while let Some(next_byte) = text_bytes.get(path_start) + && next_byte.is_ascii_whitespace() + { + path_start += 1; + } + if text_bytes.get(path_start) != Some(&b'(') { + return None; + } + + let mut path_end = path_start + 1; + while let Some(next_byte) = text_bytes.get(path_end) + && *next_byte != b')' + { + path_end += 1; + } + if text_bytes.get(path_end) != Some(&b')') { + return None; + } + + let path = text[path_start + 1..path_end].trim(); + if path.is_empty() { + return None; + } + + let name = &text[name_start..name_end]; + Some((name, path, path_end + 1)) +} + +fn is_mention_name_char(byte: u8) -> bool { + matches!(byte, b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' | b'_' | b'-') +} + +fn is_common_env_var(name: &str) -> bool { + let upper = name.to_ascii_uppercase(); + matches!( + upper.as_str(), + "PATH" + | "HOME" + | "USER" + | "SHELL" + | "PWD" + | "TMPDIR" + | "TEMP" + | "TMP" + | "LANG" + | "TERM" + | "XDG_CONFIG_HOME" + ) +} + +fn is_tool_path(path: &str) -> bool { + path.starts_with("app://") + || path.starts_with("mcp://") + || path.starts_with("skill://") + || path + .rsplit(['/', '\\']) + .next() + .is_some_and(|name| name.eq_ignore_ascii_case("SKILL.md")) +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + #[test] + fn decode_history_mentions_restores_visible_tokens() { + let decoded = decode_history_mentions( + "Use [$figma](app://figma-1) and [$figma](/tmp/figma/SKILL.md).", + ); + assert_eq!(decoded.text, "Use $figma and $figma."); + assert_eq!( + decoded.mentions, + vec![ + LinkedMention { + mention: "figma".to_string(), + path: "app://figma-1".to_string(), + }, + LinkedMention { + mention: "figma".to_string(), + path: "/tmp/figma/SKILL.md".to_string(), + }, + ] + ); + } + + #[test] + fn encode_history_mentions_links_bound_mentions_in_order() { + let text = "$figma then $figma then $other"; + let encoded = encode_history_mentions( + text, + &[ + LinkedMention { + mention: "figma".to_string(), + path: "app://figma-app".to_string(), + }, + LinkedMention { + mention: "figma".to_string(), + path: "/tmp/figma/SKILL.md".to_string(), + }, + ], + ); + assert_eq!( + encoded, + "[$figma](app://figma-app) then [$figma](/tmp/figma/SKILL.md) then $other" + ); + } +}