diff --git a/codex-rs/features/src/lib.rs b/codex-rs/features/src/lib.rs index 391e33e77..1b0bd1548 100644 --- a/codex-rs/features/src/lib.rs +++ b/codex-rs/features/src/lib.rs @@ -1068,11 +1068,7 @@ pub const FEATURES: &[FeatureSpec] = &[ FeatureSpec { id: Feature::MentionsV2, key: "mentions_v2", - stage: Stage::Experimental { - name: "Mentions v2", - menu_description: "Use a unified @ mention popup for files, folders, apps, plugins, and skills.", - announcement: "", - }, + stage: Stage::UnderDevelopment, default_enabled: false, }, FeatureSpec { diff --git a/codex-rs/features/src/tests.rs b/codex-rs/features/src/tests.rs index e1f48745f..b8cd2fd5c 100644 --- a/codex-rs/features/src/tests.rs +++ b/codex-rs/features/src/tests.rs @@ -304,6 +304,13 @@ fn auth_elicitation_is_under_development() { ); } +#[test] +fn mentions_v2_is_under_development_and_disabled_by_default() { + assert_eq!(Feature::MentionsV2.stage(), Stage::UnderDevelopment); + assert_eq!(Feature::MentionsV2.default_enabled(), false); + assert_eq!(feature_for_key("mentions_v2"), Some(Feature::MentionsV2)); +} + #[test] fn remote_control_is_removed_and_disabled_by_default() { assert_eq!(Feature::RemoteControl.stage(), Stage::Removed); diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index e62cab9e8..21208de94 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -256,7 +256,6 @@ use std::path::PathBuf; use std::time::Duration; use std::time::Instant; -#[cfg(test)] use ratatui::style::Color; /// If the pasted content exceeds this number of characters, replace it with a @@ -590,6 +589,7 @@ impl ChatComposer { pub fn set_mentions_v2_enabled(&mut self, enabled: bool) { self.mentions_v2_enabled = enabled; + self.history.set_at_mention_restore_enabled(enabled); self.sync_popups(); } @@ -609,11 +609,13 @@ impl ChatComposer { 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 { + for (id, sigil, mention) in elements { if let Some(binding) = self.draft.mention_bindings.remove(&id) + && binding.sigil == sigil && binding.mention == mention { ordered.push(MentionBinding { + sigil: binding.sigil, mention: binding.mention, path: binding.path, }); @@ -1195,7 +1197,7 @@ 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 + /// that must keep sigiled mention target resolution should use /// [`Self::set_text_content_with_mention_bindings`] instead. pub(crate) fn set_text_content( &mut self, @@ -1649,7 +1651,7 @@ impl ChatComposer { result } - /// Return true if either the slash-command popup or the file-search popup is active. + /// Return true if any popup or history search is active. pub(crate) fn popup_active(&self) -> bool { self.history_search.is_some() || self.popups.active() } @@ -1802,7 +1804,6 @@ impl ChatComposer { KeyEvent { code: KeyCode::Esc, .. } => { - // Hide popup without modifying text, remember token to avoid immediate reopen. if let Some(tok) = Self::current_at_token(&self.draft.textarea) { self.popups.dismissed_file_token = Some(tok); } @@ -1827,24 +1828,17 @@ impl ChatComposer { }; let sel_path = sel.to_string_lossy().to_string(); - // If selected path looks like an image (png/jpeg), attach as image instead of inserting text. - let is_image = Self::is_image_path(&sel_path); - if is_image { - // Determine dimensions; if that fails fall back to normal path insertion. + if Self::is_image_path(&sel_path) { let path_buf = PathBuf::from(&sel_path); match image::image_dimensions(&path_buf) { Ok((width, height)) => { tracing::debug!("selected image dimensions={}x{}", width, height); - // Remove the current @token (mirror logic from insert_selected_path without inserting text) - // using the flat text and byte-offset cursor API. let cursor_offset = self.draft.textarea.cursor(); let text = self.draft.textarea.text(); - // Clamp to a valid char boundary to avoid panics when slicing. let safe_cursor = Self::clamp_to_char_boundary(text, cursor_offset); let before_cursor = &text[..safe_cursor]; let after_cursor = &text[safe_cursor..]; - // Determine token boundaries in the full text. let start_idx = before_cursor .char_indices() .rfind(|(_, c)| c.is_whitespace()) @@ -1861,17 +1855,14 @@ impl ChatComposer { self.draft.textarea.set_cursor(start_idx); self.attach_image(path_buf); - // Add a trailing space to keep typing fluid. self.draft.textarea.insert_str(" "); } Err(err) => { tracing::trace!("image dimensions lookup failed: {err}"); - // Fallback to plain path insertion if metadata read fails. self.insert_selected_path(&sel_path); } } } else { - // Non-image: inserting file path. self.insert_selected_path(&sel_path); } self.popups.active = ActivePopup::None; @@ -1881,6 +1872,7 @@ impl ChatComposer { } } + /// Handle key events when the legacy skill mention popup is visible. fn handle_key_event_with_skill_popup(&mut self, key_event: KeyEvent) -> (InputResult, bool) { if self.handle_shortcut_overlay_key(&key_event) { return (InputResult::None, true); @@ -1962,6 +1954,7 @@ impl ChatComposer { return (InputResult::None, true); } self.footer.mode = reset_mode_after_activity(self.footer.mode); + let can_switch_search_mode = self.current_editable_at_token().is_some(); let ActivePopup::MentionV2(popup) = &mut self.popups.active else { unreachable!(); @@ -1969,6 +1962,7 @@ impl ChatComposer { let mut selected: Option = None; let mut close_popup = false; + let mut submit_without_popup = false; let result = match key_event { KeyEvent { @@ -1999,16 +1993,24 @@ impl ChatComposer { modifiers: KeyModifiers::NONE, .. } => { - popup.previous_search_mode(); - (InputResult::None, true) + if can_switch_search_mode { + popup.previous_search_mode(); + (InputResult::None, true) + } else { + self.handle_input_basic(key_event) + } } KeyEvent { code: KeyCode::Right, modifiers: KeyModifiers::NONE, .. } => { - popup.next_search_mode(); - (InputResult::None, true) + if can_switch_search_mode { + popup.next_search_mode(); + (InputResult::None, true) + } else { + self.handle_input_basic(key_event) + } } KeyEvent { code: KeyCode::Esc, .. @@ -2021,14 +2023,19 @@ impl ChatComposer { } KeyEvent { code: KeyCode::Tab, .. + } => { + selected = popup.selected(); + close_popup = true; + (InputResult::None, true) } - | KeyEvent { + KeyEvent { code: KeyCode::Enter, modifiers: KeyModifiers::NONE, .. } => { selected = popup.selected(); close_popup = true; + submit_without_popup = selected.is_none(); (InputResult::None, true) } input => self.handle_input_basic(input), @@ -2046,6 +2053,9 @@ impl ChatComposer { } } self.popups.active = ActivePopup::None; + if submit_without_popup { + return self.handle_key_event_without_popup(key_event); + } } result @@ -2241,14 +2251,14 @@ impl ChatComposer { /// second `@` in `@scope/pkg@latest`), keep treating the surrounding /// whitespace-delimited token as the active token rather than starting a /// new token at that nested prefix. - /// - If the token under the cursor starts with `prefix`, that token is - /// returned without the leading prefix. When `allow_empty` is true, a - /// lone prefix character yields `Some(String::new())` to surface hints. - fn current_prefixed_token( + /// - If the token under the cursor starts with `prefix`, its byte range and + /// text without the leading prefix are returned. When `allow_empty` is + /// true, a lone prefix character yields `Some(String::new())` to surface hints. + fn current_prefixed_token_range( textarea: &TextArea, prefix: char, allow_empty: bool, - ) -> Option { + ) -> Option<(Range, String)> { let cursor_offset = textarea.cursor(); let text = textarea.text(); @@ -2321,15 +2331,17 @@ impl ChatComposer { let left_match = token_left.filter(|t| t.starts_with(prefix)); let right_match = token_right.filter(|t| t.starts_with(prefix)); - let left_prefixed = left_match.map(|t| t[prefix.len_utf8()..].to_string()); - let right_prefixed = right_match.map(|t| t[prefix.len_utf8()..].to_string()); + let left_prefixed = + left_match.map(|t| (start_left..end_left, t[prefix.len_utf8()..].to_string())); + let right_prefixed = + right_match.map(|t| (start_right..end_right, t[prefix.len_utf8()..].to_string())); if at_whitespace { if right_prefixed.is_some() { return right_prefixed; } if token_left.is_some_and(|t| t == prefix_str) { - return allow_empty.then(String::new); + return allow_empty.then(|| (start_left..end_left, String::new())); } return left_prefixed; } @@ -2347,6 +2359,14 @@ impl ChatComposer { left_prefixed.or(right_prefixed) } + fn current_prefixed_token( + textarea: &TextArea, + prefix: char, + allow_empty: bool, + ) -> Option { + Self::current_prefixed_token_range(textarea, prefix, allow_empty).map(|(_, token)| token) + } + /// Extract the `@token` that the cursor is currently positioned on, if any. /// /// The returned string **does not** include the leading `@`. @@ -2354,11 +2374,48 @@ impl ChatComposer { Self::current_prefixed_token(textarea, '@', /*allow_empty*/ false) } + fn current_editable_at_token_with_options(&self, allow_empty: bool) -> Option { + let (range, token) = + Self::current_prefixed_token_range(&self.draft.textarea, '@', allow_empty)?; + if self + .draft + .textarea + .element_id_for_exact_range(range.clone()) + .is_some() + { + return None; + } + + let name_len = token + .as_bytes() + .iter() + .take_while(|byte| is_mention_name_char(**byte)) + .count(); + let mention_end = range.start + '@'.len_utf8() + name_len; + if name_len > 0 + && mention_end < range.end + && ends_plaintext_at_mention(self.draft.textarea.text().as_bytes(), mention_end) + && self + .draft + .textarea + .element_id_for_exact_range(range.start..mention_end) + .is_some() + { + return None; + } + + Some(token) + } + + fn current_editable_at_token(&self) -> Option { + self.current_editable_at_token_with_options(/*allow_empty*/ false) + } + fn current_mentions_v2_token(&self) -> Option { if !self.mentions_v2_enabled { return None; } - Self::current_prefixed_token(&self.draft.textarea, '@', /*allow_empty*/ true) + self.current_editable_at_token_with_options(/*allow_empty*/ true) } fn current_mention_token(&self) -> Option { @@ -2441,12 +2498,13 @@ impl ChatComposer { self.draft.textarea.set_cursor(start_idx); let id = self.draft.textarea.insert_element(insert_text); - if let (Some(path), Some(mention)) = - (path, Self::mention_name_from_insert_text(insert_text)) + if let (Some(path), Some((sigil, mention))) = + (path, Self::mention_token_from_insert_text(insert_text)) { self.draft.mention_bindings.insert( id, ComposerMentionBinding { + sigil, mention, path: path.to_string(), }, @@ -2460,8 +2518,12 @@ impl ChatComposer { self.draft.textarea.set_cursor(new_cursor); } - fn mention_name_from_insert_text(insert_text: &str) -> Option { - let name = insert_text.strip_prefix('$')?; + fn mention_token_from_insert_text(insert_text: &str) -> Option<(char, String)> { + let sigil = insert_text.chars().next()?; + if !matches!(sigil, '$' | '@') { + return None; + } + let name = &insert_text[sigil.len_utf8()..]; if name.is_empty() { return None; } @@ -2470,31 +2532,33 @@ impl ChatComposer { .iter() .all(|byte| is_mention_name_char(*byte)) { - Some(name.to_string()) + Some((sigil, name.to_string())) } else { None } } - fn current_mention_elements(&self) -> Vec<(u64, String)> { + fn current_mention_elements(&self) -> Vec<(u64, char, String)> { self.draft .textarea .text_element_snapshots() .into_iter() .filter_map(|snapshot| { - Self::mention_name_from_insert_text(snapshot.text.as_str()) - .map(|mention| (snapshot.id, mention)) + Self::mention_token_from_insert_text(snapshot.text.as_str()) + .map(|(sigil, mention)| (snapshot.id, sigil, mention)) }) .collect() } fn snapshot_mention_bindings(&self) -> Vec { let mut ordered = Vec::new(); - for (id, mention) in self.current_mention_elements() { + for (id, sigil, mention) in self.current_mention_elements() { if let Some(binding) = self.draft.mention_bindings.get(&id) + && binding.sigil == sigil && binding.mention == mention { ordered.push(MentionBinding { + sigil: binding.sigil, mention: binding.mention.clone(), path: binding.path.clone(), }); @@ -2512,7 +2576,7 @@ impl ChatComposer { let text = self.draft.textarea.text().to_string(); let mut scan_from = 0usize; for binding in mention_bindings { - let token = format!("${}", binding.mention); + let token = format!("{}{}", binding.sigil, binding.mention); let Some(range) = find_next_mention_token_range(text.as_str(), token.as_str(), scan_from) else { @@ -2531,6 +2595,7 @@ impl ChatComposer { self.draft.mention_bindings.insert( id, ComposerMentionBinding { + sigil: binding.sigil, mention: binding.mention, path: binding.path, }, @@ -2540,6 +2605,21 @@ impl ChatComposer { } } + fn plugin_at_mention_highlights(&self) -> Vec<(Range, Style)> { + self.draft + .textarea + .text_element_snapshots() + .into_iter() + .filter_map(|snapshot| { + let binding = self.draft.mention_bindings.get(&snapshot.id)?; + if !binding.path.starts_with("plugin://") || !snapshot.text.starts_with('@') { + return None; + } + Some((snapshot.range, Style::default().fg(Color::Magenta))) + }) + .collect() + } + /// Prepare text for submission/queuing. Returns None if submission should be suppressed. /// On success, clears pending paste payloads because placeholders have been expanded. /// @@ -3411,7 +3491,7 @@ impl ChatComposer { let file_token = if self.mentions_v2_enabled { None } else { - Self::current_at_token(&self.draft.textarea) + self.current_editable_at_token() }; let browsing_history = self .history @@ -3537,10 +3617,8 @@ impl ChatComposer { } } - /// Synchronize `self.file_search_popup` with the current text in the textarea. - /// Note this is only called when the active popup is NOT Command. + /// Synchronize the legacy file-search popup with the current `@` token. fn sync_file_search_popup(&mut self, query: String) { - // If user dismissed popup for this exact query, don't reopen until text changes. if self.popups.dismissed_file_token.as_ref() == Some(&query) { return; } @@ -3901,16 +3979,46 @@ 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 ends_plaintext_at_mention(bytes: &[u8], index: usize) -> bool { + bytes.get(index).is_none_or(|byte| { + byte.is_ascii_whitespace() + || *byte == b'.' + && bytes.get(index + 1).is_none_or(|next| { + next.is_ascii_whitespace() + || !next.is_ascii_alphanumeric() && *next != b'_' && *next != b'-' + }) + || !matches!(*byte, b'.' | b'/' | b'\\') + && !byte.is_ascii_alphanumeric() + && *byte != b'_' + && *byte != b'-' + }) +} + +fn starts_plaintext_at_mention(text: &str, index: usize) -> bool { + if index == 0 { + return true; + } + + text.get(..index) + .and_then(|prefix| prefix.chars().next_back()) + .is_some_and(|ch| ch.is_whitespace() || !is_mention_name_char_char(ch)) +} + +fn is_mention_name_char_char(ch: char) -> bool { + ch.is_ascii_alphanumeric() || matches!(ch, '_' | '-') +} + 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 sigil = *token_bytes.first()?; let mut index = from; while index < bytes.len() { - if bytes[index] != b'$' { + if bytes[index] != sigil { index += 1; continue; } @@ -3924,10 +4032,24 @@ fn find_next_mention_token_range(text: &str, token: &str, from: usize) -> Option continue; } - if bytes - .get(end) - .is_none_or(|byte| !is_mention_name_char(*byte)) - { + // Fix for restored `@` mentions: rebinding must not attach to embedded substrings such + // as email addresses, while preserving the existing `$` mention matching behavior. + let starts_plaintext_mention = if sigil == b'@' { + starts_plaintext_at_mention(text, index) + } else { + true + }; + // Fix for restored `@` mentions: mirror history encoding's trailing boundary so path-like + // text such as `@sample/pkg` is not rebound as the plain `@sample` mention. + let ends_plaintext_mention = if sigil == b'@' { + ends_plaintext_at_mention(bytes, end) + } else { + bytes + .get(end) + .is_none_or(|byte| !is_mention_name_char(*byte)) + }; + + if starts_plaintext_mention && ends_plaintext_mention { return Some(index..end); } @@ -4286,8 +4408,15 @@ impl ChatComposer { .textarea .render_ref_masked(textarea_rect, buf, &mut state, mask_char); } else { - let highlight_ranges = self.history_search_highlight_ranges(); - if highlight_ranges.is_empty() { + let mut highlights = self.plugin_at_mention_highlights(); + let search_highlight_style = + Style::default().add_modifier(Modifier::REVERSED | Modifier::BOLD); + highlights.extend( + self.history_search_highlight_ranges() + .into_iter() + .map(|range| (range, search_highlight_style)), + ); + if highlights.is_empty() { StatefulWidgetRef::render_ref( &(&self.draft.textarea), textarea_rect, @@ -4295,12 +4424,6 @@ impl ChatComposer { &mut state, ); } else { - let highlight_style = - Style::default().add_modifier(Modifier::REVERSED | Modifier::BOLD); - let highlights = highlight_ranges - .into_iter() - .map(|range| (range, highlight_style)) - .collect::>(); self.draft.textarea.render_ref_styled_with_highlights( textarea_rect, buf, @@ -4736,6 +4859,142 @@ mod tests { ); } + fn plugin_mention_foreground_color(composer: &ChatComposer) -> Option { + let area = Rect::new(0, 0, 40, 5); + let mut buf = Buffer::empty(area); + composer.render(area, &mut buf); + + let textarea_row = 1; + let row_text = (0..area.width) + .map(|x| { + buf[(x, textarea_row)] + .symbol() + .chars() + .next() + .unwrap_or(' ') + }) + .collect::(); + let mention_x = row_text + .find("@sample") + .expect("expected plugin mention in composer row"); + buf[(mention_x as u16, textarea_row)].style().fg + } + + #[test] + fn plugin_at_mentions_use_plugin_accent_style() { + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + /*has_input_focus*/ true, + sender, + /*enhanced_keys_supported*/ true, + "Ask Codex to do anything".to_string(), + /*disable_paste_burst*/ false, + ); + composer.set_text_content_with_mention_bindings( + "@sample plugin".to_string(), + Vec::new(), + Vec::new(), + vec![MentionBinding { + sigil: '@', + mention: "sample".to_string(), + path: "plugin://sample@test".to_string(), + }], + ); + + assert_eq!( + plugin_mention_foreground_color(&composer), + Some(Color::Magenta) + ); + } + + #[test] + fn plugin_at_mentions_render_with_plugin_accent_snapshot() { + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + /*has_input_focus*/ true, + sender, + /*enhanced_keys_supported*/ true, + "Ask Codex to do anything".to_string(), + /*disable_paste_burst*/ false, + ); + composer.set_text_content_with_mention_bindings( + "@sample plugin".to_string(), + Vec::new(), + Vec::new(), + vec![MentionBinding { + sigil: '@', + mention: "sample".to_string(), + path: "plugin://sample@test".to_string(), + }], + ); + + let area = Rect::new(0, 0, 40, 5); + let mut buf = Buffer::empty(area); + composer.render(area, &mut buf); + + let textarea_row = 1; + let mut text = String::new(); + let mut magenta = String::new(); + for x in 0..area.width { + let cell = &buf[(x, textarea_row)]; + text.push(cell.symbol().chars().next().unwrap_or(' ')); + magenta.push(if cell.style().fg == Some(Color::Magenta) { + '^' + } else { + ' ' + }); + } + while text.ends_with(' ') { + text.pop(); + } + while magenta.ends_with(' ') { + magenta.pop(); + } + + insta::assert_snapshot!( + "plugin_at_mentions_render_with_plugin_accent", + format!("text: {text}\nmagenta: {magenta}") + ); + } + + #[test] + fn recalled_plugin_at_mentions_keep_plugin_accent_style() { + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + /*has_input_focus*/ true, + sender, + /*enhanced_keys_supported*/ true, + "Ask Codex to do anything".to_string(), + /*disable_paste_burst*/ false, + ); + composer.set_text_content_with_mention_bindings( + "@sample plugin".to_string(), + Vec::new(), + Vec::new(), + vec![MentionBinding { + sigil: '@', + mention: "sample".to_string(), + path: "plugin://sample@test".to_string(), + }], + ); + let (result, _) = + composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + assert!(matches!(result, InputResult::Submitted { .. })); + + composer.set_text_content(String::new(), Vec::new(), Vec::new()); + let (_, needs_redraw) = + composer.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE)); + assert!(needs_redraw); + + assert_eq!( + plugin_mention_foreground_color(&composer), + Some(Color::Magenta) + ); + } + #[test] fn status_line_hyperlink_marks_pr_number_cells() { let (tx, _rx) = unbounded_channel::(); @@ -6446,6 +6705,291 @@ mod tests { } } + #[test] + fn set_text_content_rebinds_at_sigiled_mentions() { + 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, + ); + + let mention_bindings = vec![MentionBinding { + sigil: '@', + 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(), + ); + + assert_eq!(composer.mention_bindings(), mention_bindings); + } + + #[test] + fn set_text_content_rebinds_matching_sigil_only() { + 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, + ); + + let mention_bindings = vec![MentionBinding { + sigil: '$', + mention: "figma".to_string(), + path: "app://figma".to_string(), + }]; + composer.set_text_content_with_mention_bindings( + "@figma then $figma".to_string(), + Vec::new(), + Vec::new(), + mention_bindings.clone(), + ); + + let bound_tokens = composer + .current_mention_elements() + .into_iter() + .map(|(_, sigil, mention)| (sigil, mention)) + .collect::>(); + assert_eq!(bound_tokens, vec![('$', "figma".to_string())]); + assert_eq!(composer.mention_bindings(), mention_bindings); + } + + #[test] + fn set_text_content_rebinds_both_sigil_forms() { + 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, + ); + + let mention_bindings = vec![ + MentionBinding { + sigil: '@', + mention: "figma".to_string(), + path: "plugin://figma@test".to_string(), + }, + MentionBinding { + sigil: '$', + mention: "figma".to_string(), + path: "app://figma".to_string(), + }, + ]; + composer.set_text_content_with_mention_bindings( + "@figma then $figma".to_string(), + Vec::new(), + Vec::new(), + mention_bindings.clone(), + ); + + let bound_tokens = composer + .current_mention_elements() + .into_iter() + .map(|(_, sigil, mention)| (sigil, mention)) + .collect::>(); + assert_eq!( + bound_tokens, + vec![('@', "figma".to_string()), ('$', "figma".to_string())] + ); + assert_eq!(composer.mention_bindings(), mention_bindings); + } + + #[test] + fn set_text_content_rebinds_at_mentions_after_email_substrings() { + 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, + ); + + let text = "foo@sample.com then @sample".to_string(); + let mention_start = text.rfind("@sample").expect("expected bound mention token"); + let mention_range = mention_start..mention_start + "@sample".len(); + let mention_bindings = vec![MentionBinding { + sigil: '@', + mention: "sample".to_string(), + path: "plugin://sample@test".to_string(), + }]; + composer.set_text_content_with_mention_bindings( + text, + Vec::new(), + Vec::new(), + mention_bindings.clone(), + ); + + // Fix coverage: only the plaintext `@sample` should be atomic; the email substring stays editable. + assert_eq!( + composer + .draft + .textarea + .text_element_snapshots() + .into_iter() + .map(|snapshot| snapshot.range) + .collect::>(), + vec![mention_range] + ); + assert_eq!(composer.mention_bindings(), mention_bindings); + } + + #[test] + fn set_text_content_rebinds_at_mentions_after_punctuation() { + 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, + ); + + let text = "Please ask (@sample)".to_string(); + let mention_start = text.find("@sample").expect("expected bound mention token"); + let mention_range = mention_start..mention_start + "@sample".len(); + let mention_bindings = vec![MentionBinding { + sigil: '@', + mention: "sample".to_string(), + path: "plugin://sample@test".to_string(), + }]; + composer.set_text_content_with_mention_bindings( + text, + Vec::new(), + Vec::new(), + mention_bindings.clone(), + ); + + assert_eq!( + composer + .draft + .textarea + .text_element_snapshots() + .into_iter() + .map(|snapshot| snapshot.range) + .collect::>(), + vec![mention_range] + ); + assert_eq!(composer.mention_bindings(), mention_bindings); + } + + #[test] + fn bound_at_mentions_do_not_block_arrow_navigation() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + 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_with_mention_bindings( + "go @figma now".to_string(), + Vec::new(), + Vec::new(), + vec![MentionBinding { + sigil: '@', + mention: "figma".to_string(), + path: "plugin://figma@debug".to_string(), + }], + ); + composer.draft.textarea.set_cursor("go".len()); + + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Right, KeyModifiers::NONE)); + assert_eq!(composer.draft.textarea.cursor(), "go".len() + 1); + assert!(matches!(composer.popups.active, ActivePopup::None)); + + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Right, KeyModifiers::NONE)); + assert_eq!(composer.draft.textarea.cursor(), "go @figma".len()); + assert!(matches!(composer.popups.active, ActivePopup::None)); + + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Left, KeyModifiers::NONE)); + assert_eq!(composer.draft.textarea.cursor(), "go ".len()); + assert!(matches!(composer.popups.active, ActivePopup::None)); + + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Left, KeyModifiers::NONE)); + assert_eq!(composer.draft.textarea.cursor(), "go".len()); + assert!(matches!(composer.popups.active, ActivePopup::None)); + } + + #[test] + fn restored_bound_at_mentions_do_not_open_mention_popup() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + for (text, move_cursor_to_end) in [ + ("@sample".to_string(), false), + ("Please ask @sample.".to_string(), true), + ] { + 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_plugin_mentions(Some(vec![PluginCapabilitySummary { + config_name: "sample@test".to_string(), + display_name: "sample".to_string(), + description: None, + has_skills: true, + mcp_server_names: vec!["sample".to_string()], + app_connector_ids: Vec::new(), + }])); + + composer.set_text_content_with_mention_bindings( + text.clone(), + Vec::new(), + Vec::new(), + vec![MentionBinding { + sigil: '@', + mention: "sample".to_string(), + path: "plugin://sample@test".to_string(), + }], + ); + if move_cursor_to_end { + composer.move_cursor_to_end(); + } + + assert!(matches!(composer.popups.active, ActivePopup::None)); + + let (result, consumed) = + composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + assert!(consumed); + match result { + InputResult::Submitted { + text: submitted, .. + } => assert_eq!(submitted, text), + _ => panic!("expected restored bound mention to submit"), + } + } + } + #[test] fn enter_submits_when_file_popup_has_no_selection() { use crossterm::event::KeyCode; @@ -6478,6 +7022,39 @@ mod tests { } } + #[test] + fn enter_submits_when_unified_mention_popup_has_no_selection() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + 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_mentions_v2_enabled(/*enabled*/ true); + + let input = "npx -y @kaeawc/auto-mobile@latest"; + composer.draft.textarea.insert_str(input); + composer.draft.textarea.set_cursor(input.len()); + composer.sync_popups(); + + assert!(matches!(composer.popups.active, ActivePopup::MentionV2(_))); + + let (result, consumed) = + composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + assert!(consumed); + match result { + InputResult::Submitted { text, .. } => assert_eq!(text, input), + _ => panic!("expected Submitted"), + } + } + /// Behavior: if the ASCII path has a pending first char (flicker suppression) and a non-ASCII /// char arrives next, the pending ASCII char should still be preserved and the overall input /// should submit normally (i.e. we should not misclassify this as a paste burst). @@ -8783,6 +9360,7 @@ mod tests { ); let mention_bindings = vec![MentionBinding { + sigil: '$', mention: "figma".to_string(), path: "/tmp/user/figma/SKILL.md".to_string(), }]; diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/draft_state.rs b/codex-rs/tui/src/bottom_pane/chat_composer/draft_state.rs index f739f10ec..b219097ee 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer/draft_state.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer/draft_state.rs @@ -40,6 +40,7 @@ impl DraftState { #[derive(Clone, Debug)] pub(super) struct ComposerMentionBinding { + pub(super) sigil: char, pub(super) mention: String, pub(super) path: String, } 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 6a490e81e..23b553fa0 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer_history.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer_history.rs @@ -18,7 +18,7 @@ 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 crate::mention_codec::decode_history_mentions_with_at_mentions; use codex_protocol::ThreadId; use codex_protocol::user_input::TextElement; @@ -47,7 +47,11 @@ impl HistoryEntry { /// recorded with the full `HistoryEntry` value built by the composer; using `new` for a local /// image or paste submission would make recall lose placeholder ownership. pub(crate) fn new(text: String) -> Self { - let decoded = decode_history_mentions(&text); + Self::new_with_at_mentions(text, /*at_mentions_enabled*/ true) + } + + pub(crate) fn new_with_at_mentions(text: String, at_mentions_enabled: bool) -> Self { + let decoded = decode_history_mentions_with_at_mentions(&text, at_mentions_enabled); Self { text: decoded.text, text_elements: Vec::new(), @@ -57,6 +61,7 @@ impl HistoryEntry { .mentions .into_iter() .map(|mention| MentionBinding { + sigil: mention.sigil, mention: mention.mention, path: mention.path, }) @@ -132,6 +137,8 @@ pub(crate) struct ChatComposerHistory { /// Active incremental history search, if Ctrl+R search mode is open. search: Option, + /// Whether persistent history restore should rehydrate `@` tool mentions. + at_mention_restore_enabled: bool, } #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -226,9 +233,21 @@ impl ChatComposerHistory { history_cursor: None, last_history_text: None, search: None, + at_mention_restore_enabled: false, } } + pub fn set_at_mention_restore_enabled(&mut self, enabled: bool) { + if self.at_mention_restore_enabled == enabled { + return; + } + self.at_mention_restore_enabled = enabled; + self.fetched_history.clear(); + self.history_cursor = None; + self.last_history_text = None; + self.search = None; + } + /// Updates persistent history metadata when a new session is configured. /// /// This clears fetched entries, local entries, navigation cursors, and active search state @@ -393,7 +412,9 @@ impl ChatComposerHistory { return HistoryEntryResponse::Ignored; } - let entry = entry.map(HistoryEntry::new); + let entry = entry.map(|entry| { + HistoryEntry::new_with_at_mentions(entry, self.at_mention_restore_enabled) + }); if let Some(entry) = entry.clone() { self.fetched_history.insert(offset, entry); } @@ -840,6 +861,75 @@ mod tests { ); } + #[test] + fn persistent_restore_gates_at_mentions() { + let (tx, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx); + let mut history = ChatComposerHistory::new(); + history.set_metadata(test_thread_id(), /*log_id*/ 42, /*entry_count*/ 1); + + assert!(history.navigate_up(&tx).is_none()); + let disabled = history.on_entry_response( + /*log_id*/ 42, + /*offset*/ 0, + Some("[@sample](plugin://sample@test) and [$figma](app://figma)".to_string()), + &tx, + ); + assert_eq!( + disabled, + HistoryEntryResponse::Found(HistoryEntry { + text: "$sample and $figma".to_string(), + text_elements: Vec::new(), + local_image_paths: Vec::new(), + remote_image_urls: Vec::new(), + mention_bindings: vec![ + MentionBinding { + sigil: '$', + mention: "sample".to_string(), + path: "plugin://sample@test".to_string(), + }, + MentionBinding { + sigil: '$', + mention: "figma".to_string(), + path: "app://figma".to_string(), + }, + ], + pending_pastes: Vec::new(), + }) + ); + + history.set_at_mention_restore_enabled(/*enabled*/ true); + assert!(history.navigate_up(&tx).is_none()); + let enabled = history.on_entry_response( + /*log_id*/ 42, + /*offset*/ 0, + Some("[@sample](plugin://sample@test) and [$figma](app://figma)".to_string()), + &tx, + ); + assert_eq!( + enabled, + HistoryEntryResponse::Found(HistoryEntry { + text: "@sample and $figma".to_string(), + text_elements: Vec::new(), + local_image_paths: Vec::new(), + remote_image_urls: Vec::new(), + mention_bindings: vec![ + MentionBinding { + sigil: '@', + mention: "sample".to_string(), + path: "plugin://sample@test".to_string(), + }, + MentionBinding { + sigil: '$', + mention: "figma".to_string(), + path: "app://figma".to_string(), + }, + ], + pending_pastes: Vec::new(), + }) + ); + } + #[test] fn navigation_with_async_fetch() { let (tx, mut rx) = unbounded_channel::(); 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 b1dd7f914..a555070a8 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 @@ -50,6 +50,7 @@ fn plugin_candidate(plugin: &PluginCapabilitySummary) -> Candidate { .config_name .split_once('@') .unwrap_or((plugin.config_name.as_str(), "")); + let mention_name = plugin_mention_name(plugin_name, plugin.display_name.as_str()); let mut search_terms = vec![plugin_name.to_string(), plugin.config_name.clone()]; if plugin.display_name != plugin_name { search_terms.push(plugin.display_name.clone()); @@ -64,12 +65,88 @@ fn plugin_candidate(plugin: &PluginCapabilitySummary) -> Candidate { search_terms, mention_type: MentionType::Plugin, selection: Selection::Tool { - insert_text: format!("${plugin_name}"), + insert_text: format!("@{mention_name}"), path: Some(format!("plugin://{}", plugin.config_name)), }, } } +fn plugin_mention_name(plugin_name: &str, display_name: &str) -> String { + let plugin_segments = split_plugin_name_segments(plugin_name); + let display_segments = split_display_name_segments(display_name); + + if plugin_segments.len() == display_segments.len() + && plugin_segments.iter().zip(&display_segments).all( + |((plugin_segment, _), display_segment)| { + plugin_segment.eq_ignore_ascii_case(display_segment.as_str()) + }, + ) + { + let mut result = String::new(); + for ((_, separator), display_segment) in plugin_segments.into_iter().zip(display_segments) { + result.push_str(display_segment.as_str()); + if let Some(separator) = separator { + result.push(separator); + } + } + return result; + } + + title_case_plugin_name(plugin_name) +} + +fn split_plugin_name_segments(plugin_name: &str) -> Vec<(String, Option)> { + let mut segments = Vec::new(); + let mut current = String::new(); + + for ch in plugin_name.chars() { + if matches!(ch, '-' | '_') { + if !current.is_empty() { + segments.push((std::mem::take(&mut current), Some(ch))); + } + } else { + current.push(ch); + } + } + + if !current.is_empty() { + segments.push((current, None)); + } + + segments +} + +fn split_display_name_segments(display_name: &str) -> Vec { + display_name + .split(|ch: char| !ch.is_ascii_alphanumeric()) + .filter(|segment| !segment.is_empty()) + .map(ToString::to_string) + .collect() +} + +fn title_case_plugin_name(plugin_name: &str) -> String { + let mut result = String::with_capacity(plugin_name.len()); + let mut capitalize_next = true; + + for ch in plugin_name.chars() { + if matches!(ch, '-' | '_') { + capitalize_next = true; + result.push(ch); + continue; + } + + if capitalize_next && ch.is_ascii_alphabetic() { + result.push(ch.to_ascii_uppercase()); + capitalize_next = false; + } else { + result.push(ch); + capitalize_next = false; + } + } + + result +} + fn plugin_description(plugin: &PluginCapabilitySummary) -> Option { let capability_labels = plugin_capability_labels(plugin); plugin.description.clone().or_else(|| { @@ -109,3 +186,30 @@ fn optional_skill_description(skill: &SkillMetadata) -> Option { let description = skill_description(skill).trim(); (!description.is_empty()).then(|| description.to_string()) } + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + #[test] + fn plugin_mention_name_uses_display_segments_when_they_match_plugin_name() { + assert_eq!( + plugin_mention_name("mcp-search", "MCP Search"), + "MCP-Search" + ); + assert_eq!( + plugin_mention_name("google_calendar", "Google Calendar"), + "Google_Calendar" + ); + } + + #[test] + fn plugin_mention_name_falls_back_to_title_cased_plugin_name() { + assert_eq!(plugin_mention_name("sample", "Sample Plugin"), "Sample"); + assert_eq!( + plugin_mention_name("browser-use", "Browser Use"), + "Browser-Use" + ); + } +} diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 1fcc7635c..7ef385570 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -83,7 +83,9 @@ pub(crate) struct LocalImageAttachment { #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) struct MentionBinding { - /// Mention token text without the leading `$`. + /// Visible mention sigil (`$` or `@`). + pub(crate) sigil: char, + /// Mention token text without the leading sigil (`$` or `@`). pub(crate) mention: String, /// Canonical mention target (for example `app://...` or absolute SKILL.md path). pub(crate) path: String, diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__plugin_at_mentions_render_with_plugin_accent.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__plugin_at_mentions_render_with_plugin_accent.snap new file mode 100644 index 000000000..0f6c47dca --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__plugin_at_mentions_render_with_plugin_accent.snap @@ -0,0 +1,6 @@ +--- +source: tui/src/bottom_pane/chat_composer.rs +expression: "format!(\"text: {text}\\nmagenta: {magenta}\")" +--- +text: › @sample plugin +magenta: ^^^^^^^ diff --git a/codex-rs/tui/src/chatwidget/input_submission.rs b/codex-rs/tui/src/chatwidget/input_submission.rs index 4700ffc0c..578a35728 100644 --- a/codex-rs/tui/src/chatwidget/input_submission.rs +++ b/codex-rs/tui/src/chatwidget/input_submission.rs @@ -363,6 +363,7 @@ impl ChatWidget { let encoded_mentions = mention_bindings .iter() .map(|binding| LinkedMention { + sigil: binding.sigil, mention: binding.mention.clone(), path: binding.path.clone(), }) diff --git a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs index 23487af43..27ae4daff 100644 --- a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs +++ b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs @@ -570,6 +570,7 @@ async fn submission_prefers_selected_duplicate_skill_path() { Vec::new(), Vec::new(), vec![MentionBinding { + sigil: '$', mention: "figma".to_string(), path: user_skill_path.to_string_lossy().into_owned(), }], @@ -605,6 +606,7 @@ async fn blocked_image_restore_preserves_mention_bindings() { path: PathBuf::from("/tmp/blocked.png"), }]; let mention_bindings = vec![MentionBinding { + sigil: '$', mention: "file".to_string(), path: "/tmp/skills/file/SKILL.md".to_string(), }]; @@ -1236,6 +1238,7 @@ async fn submit_user_message_ignores_inaccessible_app_mentions_from_bindings() { remote_image_urls: Vec::new(), text_elements: Vec::new(), mention_bindings: vec![MentionBinding { + sigil: '$', mention: "arabica-uae".to_string(), path: "app://arabica_uae".to_string(), }], diff --git a/codex-rs/tui/src/chatwidget/tests/plan_mode.rs b/codex-rs/tui/src/chatwidget/tests/plan_mode.rs index 5fc2fa2ce..74cd52703 100644 --- a/codex-rs/tui/src/chatwidget/tests/plan_mode.rs +++ b/codex-rs/tui/src/chatwidget/tests/plan_mode.rs @@ -1244,6 +1244,7 @@ async fn submit_user_message_emits_structured_plugin_mentions_from_bindings() { remote_image_urls: Vec::new(), text_elements: Vec::new(), mention_bindings: vec![MentionBinding { + sigil: '$', mention: "sample".to_string(), path: "plugin://sample@test".to_string(), }], diff --git a/codex-rs/tui/src/chatwidget/tests/review_mode.rs b/codex-rs/tui/src/chatwidget/tests/review_mode.rs index 48bc341f6..628174072 100644 --- a/codex-rs/tui/src/chatwidget/tests/review_mode.rs +++ b/codex-rs/tui/src/chatwidget/tests/review_mode.rs @@ -875,6 +875,7 @@ async fn manual_interrupt_restores_pending_steer_mention_bindings_to_composer() chat.on_agent_message_delta("Final answer line\n".to_string()); let mention_bindings = vec![MentionBinding { + sigil: '$', mention: "figma".to_string(), path: "/tmp/skills/figma/SKILL.md".to_string(), }]; diff --git a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs index 0346b9706..ff1feb084 100644 --- a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs +++ b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs @@ -656,6 +656,7 @@ async fn goal_slash_command_uses_plain_text_for_mentions() { Vec::new(), Vec::new(), vec![MentionBinding { + sigil: '$', mention: "figma".to_string(), path: "app://figma".to_string(), }], @@ -917,6 +918,7 @@ fn merged_history_record_preserves_raw_text_and_rebased_elements() { remote_image_urls: Vec::new(), text_elements: vec![TextElement::new((4..10).into(), Some("$figma".to_string()))], mention_bindings: vec![MentionBinding { + sigil: '$', mention: "figma".to_string(), path: "app://figma".to_string(), }], @@ -1034,6 +1036,7 @@ async fn interrupted_merged_message_history_encodes_mentions_once() { Vec::new(), Vec::new(), vec![MentionBinding { + sigil: '$', mention: "figma".to_string(), path: "app://figma".to_string(), }], diff --git a/codex-rs/tui/src/mention_codec.rs b/codex-rs/tui/src/mention_codec.rs index f6cd38bd4..520a6712e 100644 --- a/codex-rs/tui/src/mention_codec.rs +++ b/codex-rs/tui/src/mention_codec.rs @@ -6,6 +6,7 @@ use codex_utils_plugins::mention_syntax::TOOL_MENTION_SIGIL; #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) struct LinkedMention { + pub(crate) sigil: char, pub(crate) mention: String, pub(crate) path: String, } @@ -22,10 +23,16 @@ pub(crate) fn encode_history_mentions(text: &str, mentions: &[LinkedMention]) -> return text.to_string(); } - let mut mentions_by_name: HashMap<&str, VecDeque<&str>> = HashMap::new(); + let mut mentions_by_token: HashMap<(char, &str), VecDeque<&str>> = HashMap::new(); for mention in mentions { - mentions_by_name - .entry(mention.mention.as_str()) + if !matches!( + mention.sigil, + TOOL_MENTION_SIGIL | PLUGIN_TEXT_MENTION_SIGIL + ) { + continue; + } + mentions_by_token + .entry((mention.sigil, mention.mention.as_str())) .or_default() .push_back(mention.path.as_str()); } @@ -35,28 +42,38 @@ pub(crate) fn encode_history_mentions(text: &str, mentions: &[LinkedMention]) -> let mut index = 0usize; while index < bytes.len() { - if bytes[index] == TOOL_MENTION_SIGIL as u8 { - 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) + if matches!( + bytes[index], + byte if byte == TOOL_MENTION_SIGIL as u8 || byte == PLUGIN_TEXT_MENTION_SIGIL as u8 + ) { + let sigil = bytes[index] as char; + if sigil == TOOL_MENTION_SIGIL || starts_plaintext_mention(text, index) { + let name_start = index + 1; + if let Some(first) = bytes.get(name_start) + && is_mention_name_char(*first) { - name_end += 1; - } + 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(TOOL_MENTION_SIGIL); - out.push_str(name); - out.push_str("]("); - out.push_str(path); - out.push(')'); - index = name_end; - continue; + let name = &text[name_start..name_end]; + if (sigil == TOOL_MENTION_SIGIL || ends_plaintext_mention(bytes, name_end)) + && let Some(path) = mentions_by_token + .get_mut(&(sigil, name)) + .and_then(VecDeque::pop_front) + { + out.push('['); + out.push(sigil); + out.push_str(name); + out.push_str("]("); + out.push_str(path); + out.push(')'); + index = name_end; + continue; + } } } } @@ -71,7 +88,10 @@ pub(crate) fn encode_history_mentions(text: &str, mentions: &[LinkedMention]) -> out } -pub(crate) fn decode_history_mentions(text: &str) -> DecodedHistoryText { +pub(crate) fn decode_history_mentions_with_at_mentions( + text: &str, + at_mentions_enabled: bool, +) -> DecodedHistoryText { let bytes = text.as_bytes(); let mut out = String::with_capacity(text.len()); let mut mentions = Vec::new(); @@ -79,11 +99,13 @@ pub(crate) fn decode_history_mentions(text: &str) -> DecodedHistoryText { while index < bytes.len() { if bytes[index] == b'[' - && let Some((name, path, end_index)) = parse_history_linked_mention(text, bytes, index) + && let Some((sigil, name, path, end_index)) = + parse_history_linked_mention(text, bytes, index, at_mentions_enabled) { - out.push(TOOL_MENTION_SIGIL); + out.push(sigil); out.push_str(name); mentions.push(LinkedMention { + sigil, mention: name.to_string(), path: path.to_string(), }); @@ -108,22 +130,32 @@ fn parse_history_linked_mention<'a>( text: &'a str, text_bytes: &[u8], start: usize, -) -> Option<(&'a str, &'a str, usize)> { - // TUI writes `$name`, but may read plugin `[@name](plugin://...)` links from other clients. - if let Some(mention @ (name, path, _)) = + at_mentions_enabled: bool, +) -> Option<(char, &'a str, &'a str, usize)> { + // TUI historically wrote `$name`, but selected unified `@` mentions should preserve `@` on + // history round-trip for any canonical tool path. + if let Some((name, path, end_index)) = parse_linked_tool_mention(text, text_bytes, start, TOOL_MENTION_SIGIL) && !is_common_env_var(name) && is_tool_path(path) { - return Some(mention); + return Some((TOOL_MENTION_SIGIL, name, path, end_index)); } - if let Some(mention @ (name, path, _)) = + if at_mentions_enabled { + if let Some((name, path, end_index)) = + parse_linked_tool_mention(text, text_bytes, start, PLUGIN_TEXT_MENTION_SIGIL) + && !is_common_env_var(name) + && is_tool_path(path) + { + return Some((PLUGIN_TEXT_MENTION_SIGIL, name, path, end_index)); + } + } else if let Some((name, path, end_index)) = parse_linked_tool_mention(text, text_bytes, start, PLUGIN_TEXT_MENTION_SIGIL) && !is_common_env_var(name) && path.starts_with("plugin://") { - return Some(mention); + return Some((TOOL_MENTION_SIGIL, name, path, end_index)); } None @@ -190,6 +222,35 @@ 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 starts_plaintext_mention(text: &str, index: usize) -> bool { + if index == 0 { + return true; + } + + text.get(..index) + .and_then(|prefix| prefix.chars().next_back()) + .is_some_and(|ch| ch.is_whitespace() || !is_mention_name_char_char(ch)) +} + +fn ends_plaintext_mention(text_bytes: &[u8], index: usize) -> bool { + text_bytes.get(index).is_none_or(|byte| { + byte.is_ascii_whitespace() + || *byte == b'.' + && text_bytes.get(index + 1).is_none_or(|next| { + next.is_ascii_whitespace() + || !next.is_ascii_alphanumeric() && *next != b'_' && *next != b'-' + }) + || !matches!(*byte, b'.' | b'/' | b'\\') + && !byte.is_ascii_alphanumeric() + && *byte != b'_' + && *byte != b'-' + }) +} + +fn is_mention_name_char_char(ch: char) -> bool { + ch.is_ascii_alphanumeric() || matches!(ch, '_' | '-') +} + fn is_common_env_var(name: &str) -> bool { let upper = name.to_ascii_uppercase(); matches!( @@ -226,22 +287,26 @@ mod tests { #[test] fn decode_history_mentions_restores_visible_tokens() { - let decoded = decode_history_mentions( + let decoded = decode_history_mentions_with_at_mentions( "Use [$figma](app://figma-1), [$sample](plugin://sample@test), and [$figma](/tmp/figma/SKILL.md).", + /*at_mentions_enabled*/ true, ); assert_eq!(decoded.text, "Use $figma, $sample, and $figma."); assert_eq!( decoded.mentions, vec![ LinkedMention { + sigil: '$', mention: "figma".to_string(), path: "app://figma-1".to_string(), }, LinkedMention { + sigil: '$', mention: "sample".to_string(), path: "plugin://sample@test".to_string(), }, LinkedMention { + sigil: '$', mention: "figma".to_string(), path: "/tmp/figma/SKILL.md".to_string(), }, @@ -251,18 +316,21 @@ mod tests { #[test] fn decode_history_mentions_restores_plugin_links_with_at_sigil() { - let decoded = decode_history_mentions( + let decoded = decode_history_mentions_with_at_mentions( "Use [@sample](plugin://sample@test) and [$figma](app://figma-1).", + /*at_mentions_enabled*/ true, ); - assert_eq!(decoded.text, "Use $sample and $figma."); + assert_eq!(decoded.text, "Use @sample and $figma."); assert_eq!( decoded.mentions, vec![ LinkedMention { + sigil: '@', mention: "sample".to_string(), path: "plugin://sample@test".to_string(), }, LinkedMention { + sigil: '$', mention: "figma".to_string(), path: "app://figma-1".to_string(), }, @@ -271,13 +339,58 @@ mod tests { } #[test] - fn decode_history_mentions_ignores_at_sigil_for_non_plugin_paths() { - let decoded = decode_history_mentions("Use [@figma](app://figma-1)."); + fn decode_history_mentions_without_at_mentions_uses_legacy_plugin_fallback() { + let decoded = decode_history_mentions_with_at_mentions( + "Use [@sample](plugin://sample@test) and [$figma](app://figma-1).", + /*at_mentions_enabled*/ false, + ); + assert_eq!(decoded.text, "Use $sample and $figma."); + assert_eq!( + decoded.mentions, + vec![ + LinkedMention { + sigil: '$', + mention: "sample".to_string(), + path: "plugin://sample@test".to_string(), + }, + LinkedMention { + sigil: '$', + mention: "figma".to_string(), + path: "app://figma-1".to_string(), + }, + ] + ); + } + + #[test] + fn decode_history_mentions_without_at_mentions_ignores_at_non_plugin_paths() { + let decoded = decode_history_mentions_with_at_mentions( + "Use [@figma](app://figma-1).", + /*at_mentions_enabled*/ false, + ); assert_eq!(decoded.text, "Use [@figma](app://figma-1)."); assert_eq!(decoded.mentions, Vec::::new()); } + #[test] + fn decode_history_mentions_restores_at_sigil_for_tool_paths() { + let decoded = decode_history_mentions_with_at_mentions( + "Use [@figma](app://figma-1).", + /*at_mentions_enabled*/ true, + ); + + assert_eq!(decoded.text, "Use @figma."); + assert_eq!( + decoded.mentions, + vec![LinkedMention { + sigil: '@', + mention: "figma".to_string(), + path: "app://figma-1".to_string(), + }] + ); + } + #[test] fn encode_history_mentions_links_bound_mentions_in_order() { let text = "$figma then $sample then $figma then $other"; @@ -285,14 +398,17 @@ mod tests { text, &[ LinkedMention { + sigil: '$', mention: "figma".to_string(), path: "app://figma-app".to_string(), }, LinkedMention { + sigil: '$', mention: "sample".to_string(), path: "plugin://sample@test".to_string(), }, LinkedMention { + sigil: '$', mention: "figma".to_string(), path: "/tmp/figma/SKILL.md".to_string(), }, @@ -303,4 +419,161 @@ mod tests { "[$figma](app://figma-app) then [$sample](plugin://sample@test) then [$figma](/tmp/figma/SKILL.md) then $other" ); } + + #[test] + fn encode_history_mentions_links_dollar_mentions_after_punctuation() { + let encoded = encode_history_mentions( + "($figma)", + &[LinkedMention { + sigil: '$', + mention: "figma".to_string(), + path: "app://figma".to_string(), + }], + ); + assert_eq!(encoded, "([$figma](app://figma))"); + } + + #[test] + fn encode_history_mentions_links_dollar_mentions_with_path_like_suffixes() { + let mention = LinkedMention { + sigil: '$', + mention: "figma".to_string(), + path: "app://figma".to_string(), + }; + + assert_eq!( + encode_history_mentions("$figma/docs", std::slice::from_ref(&mention)), + "[$figma](app://figma)/docs" + ); + assert_eq!( + encode_history_mentions("$figma.suffix", std::slice::from_ref(&mention)), + "[$figma](app://figma).suffix" + ); + assert_eq!( + encode_history_mentions("$figma\\docs", &[mention]), + "[$figma](app://figma)\\docs" + ); + } + + #[test] + fn encode_history_mentions_preserves_at_sigils() { + let text = "@figma then @sample then $other"; + let encoded = encode_history_mentions( + text, + &[ + LinkedMention { + sigil: '@', + mention: "figma".to_string(), + path: "/tmp/figma/SKILL.md".to_string(), + }, + LinkedMention { + sigil: '@', + mention: "sample".to_string(), + path: "plugin://sample@test".to_string(), + }, + ], + ); + assert_eq!( + encoded, + "[@figma](/tmp/figma/SKILL.md) then [@sample](plugin://sample@test) then $other" + ); + } + + #[test] + fn encode_history_mentions_links_both_sigils_for_same_name() { + let text = "@figma then $figma"; + let encoded = encode_history_mentions( + text, + &[ + LinkedMention { + sigil: '@', + mention: "figma".to_string(), + path: "plugin://figma@test".to_string(), + }, + LinkedMention { + sigil: '$', + mention: "figma".to_string(), + path: "app://figma".to_string(), + }, + ], + ); + assert_eq!( + encoded, + "[@figma](plugin://figma@test) then [$figma](app://figma)" + ); + } + + #[test] + fn encode_history_mentions_does_not_let_at_token_steal_later_tool_binding() { + let text = "@figma then $figma"; + let encoded = encode_history_mentions( + text, + &[LinkedMention { + sigil: '$', + mention: "figma".to_string(), + path: "app://figma-app".to_string(), + }], + ); + assert_eq!(encoded, "@figma then [$figma](app://figma-app)"); + } + + #[test] + fn encode_history_mentions_links_at_mentions_after_unicode_whitespace() { + // Fix coverage: full-width space should remain a valid plaintext boundary for `@` links. + let text = "foo　@sample"; + let encoded = encode_history_mentions( + text, + &[LinkedMention { + sigil: '@', + mention: "sample".to_string(), + path: "plugin://sample@test".to_string(), + }], + ); + assert_eq!(encoded, "foo　[@sample](plugin://sample@test)"); + } + + #[test] + fn encode_history_mentions_links_sentence_ending_at_mentions() { + let text = "Please ask @figma."; + let encoded = encode_history_mentions( + text, + &[LinkedMention { + sigil: '@', + mention: "figma".to_string(), + path: "/tmp/figma/SKILL.md".to_string(), + }], + ); + assert_eq!(encoded, "Please ask [@figma](/tmp/figma/SKILL.md)."); + } + + #[test] + fn encode_history_mentions_links_parenthesized_at_mentions() { + let text = "Please ask (@figma)"; + let encoded = encode_history_mentions( + text, + &[LinkedMention { + sigil: '@', + mention: "figma".to_string(), + path: "plugin://figma@test".to_string(), + }], + ); + assert_eq!(encoded, "Please ask ([@figma](plugin://figma@test))"); + } + + #[test] + fn encode_history_mentions_skips_embedded_at_substrings() { + let text = "foo@sample.com npx @sample/pkg then @sample"; + let encoded = encode_history_mentions( + text, + &[LinkedMention { + sigil: '@', + mention: "sample".to_string(), + path: "plugin://sample@test".to_string(), + }], + ); + assert_eq!( + encoded, + "foo@sample.com npx @sample/pkg then [@sample](plugin://sample@test)" + ); + } }