Skip to content

Commit 1a58096

Browse files
authored
fix: Prevent slash command popup from activating on invalid inputs (#7704)
## Slash Command popup issue #7659 When recalling history, the composer(`codex_tui::bottom_pane::chat_composer`) restores the previous prompt text (which may start with `/`) and then calls `sync_command_popup`. The logic in `sync_command_popup` treats any first line that starts with `/` and has the caret inside the initial `/name` token as an active slash command name: ```rust let is_editing_slash_command_name = if first_line.starts_with('/') && caret_on_first_line { let token_end = first_line .char_indices() .find(|(_, c)| c.is_whitespace()) .map(|(i, _)| i) .unwrap_or(first_line.len()); cursor <= token_end } else { false }; ``` This detection does not distinguish between an actual interactive slash command being typed and a normal historical prompt that happens to begin with `/`. As a result, after history recall, the restored prompt like `/ test` is interpreted as an "editing command name" context and the slash-command popup is (re)activated. Once `active_popup` is `ActivePopup::Command`, subsequent `Up` key presses are handled by `handle_key_event_with_slash_popup` instead of `handle_key_event_without_popup`, so they no longer trigger `history.navigate_up(...)` and the session prompt history cannot be scrolled.
1 parent cb9a189 commit 1a58096

File tree

1 file changed

+125
-11
lines changed

1 file changed

+125
-11
lines changed

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

Lines changed: 125 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1579,6 +1579,56 @@ impl ChatComposer {
15791579
}
15801580
}
15811581

1582+
/// If the cursor is currently within a slash command on the first line,
1583+
/// extract the command name and the rest of the line after it.
1584+
/// Returns None if the cursor is outside a slash command.
1585+
fn slash_command_under_cursor(first_line: &str, cursor: usize) -> Option<(&str, &str)> {
1586+
if !first_line.starts_with('/') {
1587+
return None;
1588+
}
1589+
1590+
let name_start = 1usize;
1591+
let name_end = first_line[name_start..]
1592+
.find(char::is_whitespace)
1593+
.map(|idx| name_start + idx)
1594+
.unwrap_or_else(|| first_line.len());
1595+
1596+
if cursor > name_end {
1597+
return None;
1598+
}
1599+
1600+
let name = &first_line[name_start..name_end];
1601+
let rest_start = first_line[name_end..]
1602+
.find(|c: char| !c.is_whitespace())
1603+
.map(|idx| name_end + idx)
1604+
.unwrap_or(name_end);
1605+
let rest = &first_line[rest_start..];
1606+
1607+
Some((name, rest))
1608+
}
1609+
1610+
/// Heuristic for whether the typed slash command looks like a valid
1611+
/// prefix for any known command (built-in or custom prompt).
1612+
/// Empty names only count when there is no extra content after the '/'.
1613+
fn looks_like_slash_prefix(&self, name: &str, rest_after_name: &str) -> bool {
1614+
if name.is_empty() {
1615+
return rest_after_name.is_empty();
1616+
}
1617+
1618+
let builtin_match = built_in_slash_commands()
1619+
.into_iter()
1620+
.any(|(cmd_name, _)| cmd_name.starts_with(name));
1621+
1622+
if builtin_match {
1623+
return true;
1624+
}
1625+
1626+
let prompt_prefix = format!("{PROMPTS_CMD_PREFIX}:");
1627+
self.custom_prompts
1628+
.iter()
1629+
.any(|p| format!("{prompt_prefix}{}", p.name).starts_with(name))
1630+
}
1631+
15821632
/// Synchronize `self.command_popup` with the current text in the
15831633
/// textarea. This must be called after every modification that can change
15841634
/// the text so the popup is shown/updated/hidden as appropriate.
@@ -1596,17 +1646,10 @@ impl ChatComposer {
15961646
let cursor = self.textarea.cursor();
15971647
let caret_on_first_line = cursor <= first_line_end;
15981648

1599-
let is_editing_slash_command_name = if first_line.starts_with('/') && caret_on_first_line {
1600-
// Compute the end of the initial '/name' token (name may be empty yet).
1601-
let token_end = first_line
1602-
.char_indices()
1603-
.find(|(_, c)| c.is_whitespace())
1604-
.map(|(i, _)| i)
1605-
.unwrap_or(first_line.len());
1606-
cursor <= token_end
1607-
} else {
1608-
false
1609-
};
1649+
let is_editing_slash_command_name = caret_on_first_line
1650+
&& Self::slash_command_under_cursor(first_line, cursor)
1651+
.is_some_and(|(name, rest)| self.looks_like_slash_prefix(name, rest));
1652+
16101653
// If the cursor is currently positioned within an `@token`, prefer the
16111654
// file-search popup over the slash popup so users can insert a file path
16121655
// as an argument to the command (e.g., "/review @docs/...").
@@ -3873,4 +3916,75 @@ mod tests {
38733916
assert_eq!(composer.textarea.text(), "z".repeat(count));
38743917
assert!(composer.pending_pastes.is_empty());
38753918
}
3919+
3920+
#[test]
3921+
fn slash_popup_not_activated_for_slash_space_text_history_like_input() {
3922+
use crossterm::event::KeyCode;
3923+
use crossterm::event::KeyEvent;
3924+
use crossterm::event::KeyModifiers;
3925+
use tokio::sync::mpsc::unbounded_channel;
3926+
3927+
let (tx, _rx) = unbounded_channel::<AppEvent>();
3928+
let sender = AppEventSender::new(tx);
3929+
let mut composer = ChatComposer::new(
3930+
true,
3931+
sender,
3932+
false,
3933+
"Ask Codex to do anything".to_string(),
3934+
false,
3935+
);
3936+
3937+
// Simulate history-like content: "/ test"
3938+
composer.set_text_content("/ test".to_string());
3939+
3940+
// After set_text_content -> sync_popups is called; popup should NOT be Command.
3941+
assert!(
3942+
matches!(composer.active_popup, ActivePopup::None),
3943+
"expected no slash popup for '/ test'"
3944+
);
3945+
3946+
// Up should be handled by history navigation path, not slash popup handler.
3947+
let (result, _redraw) =
3948+
composer.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE));
3949+
assert_eq!(result, InputResult::None);
3950+
}
3951+
3952+
#[test]
3953+
fn slash_popup_activated_for_bare_slash_and_valid_prefixes() {
3954+
// use crossterm::event::{KeyCode, KeyEvent, KeyModifiers};
3955+
use tokio::sync::mpsc::unbounded_channel;
3956+
3957+
let (tx, _rx) = unbounded_channel::<AppEvent>();
3958+
let sender = AppEventSender::new(tx);
3959+
let mut composer = ChatComposer::new(
3960+
true,
3961+
sender,
3962+
false,
3963+
"Ask Codex to do anything".to_string(),
3964+
false,
3965+
);
3966+
3967+
// Case 1: bare "/"
3968+
composer.set_text_content("/".to_string());
3969+
assert!(
3970+
matches!(composer.active_popup, ActivePopup::Command(_)),
3971+
"bare '/' should activate slash popup"
3972+
);
3973+
3974+
// Case 2: valid prefix "/re" (matches /review, /resume, etc.)
3975+
composer.set_text_content("/re".to_string());
3976+
assert!(
3977+
matches!(composer.active_popup, ActivePopup::Command(_)),
3978+
"'/re' should activate slash popup via prefix match"
3979+
);
3980+
3981+
// Case 3: invalid prefix "/zzz" – still allowed to open popup if it
3982+
// matches no built-in command, our current logic will *not* open popup.
3983+
// Verify that explicitly.
3984+
composer.set_text_content("/zzz".to_string());
3985+
assert!(
3986+
matches!(composer.active_popup, ActivePopup::None),
3987+
"'/zzz' should not activate slash popup because it is not a prefix of any built-in command"
3988+
);
3989+
}
38763990
}

0 commit comments

Comments
 (0)