mirror of
https://github.com/openai/codex.git
synced 2026-05-21 03:33:41 +00:00
fix(tui): warn on unsupported iTerm2 pet versions (#23371)
## Why Older iTerm2 builds can be detected as supporting the image transport that terminal pets use, but in practice they fail to render the pet flow correctly. Instead of silently attempting image rendering, Codex should tell the user that their iTerm2 version is too old and that upgrading is the fix. ## What Changed - gate iTerm2 pet auto-detection on version `3.6.0` or newer - show a dedicated upgrade message for older or unknown iTerm2 versions instead of the generic unsupported-terminal warning - keep the existing generic unsupported-terminal path for non-iTerm terminals - add regression coverage for iTerm2 version parsing and the old-iTerm warning path ## How to Test 1. Start Codex in iTerm2 3.6 or newer. 2. Run `/pets`. 3. Confirm the pets picker opens instead of showing a warning. 4. Start Codex in an older iTerm2 build, or exercise the equivalent test path. 5. Run `/pets`. 6. Confirm Codex warns that pets require iTerm2 3.6 or newer and tells the user to upgrade. 7. Also verify that a non-iTerm unsupported terminal still shows the generic unsupported-terminal message. Targeted tests: - `cargo test -p codex-terminal-detection` - `cargo test -p codex-tui pets::` - `cargo test -p codex-tui slash_pets_on_unsupported_terminal` - `cargo test -p codex-tui slash_pets_on_old_iterm2`
This commit is contained in:
@@ -15,6 +15,18 @@ fn force_tmux_pet_image_unsupported(chat: &mut ChatWidget) {
|
||||
));
|
||||
}
|
||||
|
||||
fn force_terminal_pet_image_unsupported(chat: &mut ChatWidget) {
|
||||
chat.set_pet_image_support_for_tests(crate::pets::PetImageSupport::Unsupported(
|
||||
crate::pets::PetImageUnsupportedReason::Terminal,
|
||||
));
|
||||
}
|
||||
|
||||
fn force_old_iterm2_pet_image_unsupported(chat: &mut ChatWidget) {
|
||||
chat.set_pet_image_support_for_tests(crate::pets::PetImageSupport::Unsupported(
|
||||
crate::pets::PetImageUnsupportedReason::Iterm2TooOld,
|
||||
));
|
||||
}
|
||||
|
||||
fn fast_tier_command() -> ServiceTierCommand {
|
||||
ServiceTierCommand {
|
||||
id: ServiceTier::Fast.request_value().to_string(),
|
||||
@@ -1934,6 +1946,44 @@ async fn slash_pets_with_arg_on_unsupported_terminal_warns_without_selection() {
|
||||
assert_matches!(op_rx.try_recv(), Err(TryRecvError::Empty));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn slash_pets_on_unsupported_terminal_shows_terminal_warning() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
force_terminal_pet_image_unsupported(&mut chat);
|
||||
|
||||
chat.dispatch_command(SlashCommand::Pets);
|
||||
|
||||
assert!(!chat.bottom_pane.has_active_view());
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
let rendered = cells
|
||||
.iter()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
assert!(rendered.contains("Pets aren’t available in this terminal."));
|
||||
assert!(rendered.contains("Kitty graphics or Sixel support"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn slash_pets_on_old_iterm2_shows_upgrade_warning() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
force_old_iterm2_pet_image_unsupported(&mut chat);
|
||||
|
||||
chat.dispatch_command(SlashCommand::Pets);
|
||||
|
||||
assert!(!chat.bottom_pane.has_active_view());
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
let rendered = cells
|
||||
.iter()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
assert!(rendered.contains("Pets require iTerm2 3.6 or newer."));
|
||||
assert!(rendered.contains("Upgrade iTerm2 to use terminal pets."));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn slash_fork_requests_current_fork() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
|
||||
@@ -21,6 +21,7 @@ const ESC: &str = "\x1b";
|
||||
const ST: &str = "\x1b\\";
|
||||
const KITTY_CHUNK_SIZE: usize = 4096;
|
||||
const SIXEL_CACHE_VERSION: &str = "v2";
|
||||
const ITERM2_KITTY_MIN_VERSION: (u64, u64, u64) = (3, 6, 0);
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum ImageProtocol {
|
||||
@@ -55,6 +56,7 @@ impl PetImageSupport {
|
||||
pub(crate) enum PetImageUnsupportedReason {
|
||||
Tmux,
|
||||
Zellij,
|
||||
Iterm2TooOld,
|
||||
Terminal,
|
||||
}
|
||||
|
||||
@@ -67,6 +69,9 @@ impl PetImageUnsupportedReason {
|
||||
Self::Zellij => {
|
||||
"Pets are disabled in Zellij. Terminal images don’t stay reliably pane-local in Zellij. Run Codex outside Zellij to use pets."
|
||||
}
|
||||
Self::Iterm2TooOld => {
|
||||
"Pets require iTerm2 3.6 or newer. Upgrade iTerm2 to use terminal pets."
|
||||
}
|
||||
Self::Terminal => {
|
||||
"Pets aren’t available in this terminal. Terminal pets need image support, and this terminal environment doesn’t expose a supported image protocol. Try a terminal with Kitty graphics or Sixel support, or run Codex outside tmux."
|
||||
}
|
||||
@@ -142,6 +147,10 @@ fn pet_image_support_for_terminal(info: &TerminalInfo) -> PetImageSupport {
|
||||
return PetImageSupport::Supported(ImageProtocol::KittyLocalFile);
|
||||
}
|
||||
|
||||
if is_iterm2_terminal(info) {
|
||||
return PetImageSupport::Unsupported(PetImageUnsupportedReason::Iterm2TooOld);
|
||||
}
|
||||
|
||||
if supports_kitty_graphics(info) {
|
||||
return PetImageSupport::Supported(ImageProtocol::Kitty);
|
||||
}
|
||||
@@ -154,6 +163,14 @@ fn pet_image_support_for_terminal(info: &TerminalInfo) -> PetImageSupport {
|
||||
}
|
||||
|
||||
fn supports_iterm2_kitty_graphics(info: &TerminalInfo) -> bool {
|
||||
is_iterm2_terminal(info)
|
||||
&& version_is_at_least(
|
||||
info.version.as_deref(),
|
||||
/*minimum*/ ITERM2_KITTY_MIN_VERSION,
|
||||
)
|
||||
}
|
||||
|
||||
fn is_iterm2_terminal(info: &TerminalInfo) -> bool {
|
||||
matches!(info.name, TerminalName::Iterm2)
|
||||
|| terminal_field_contains(info.term_program.as_deref(), "iterm")
|
||||
}
|
||||
@@ -181,6 +198,24 @@ fn terminal_field_contains(value: Option<&str>, needle: &str) -> bool {
|
||||
value.is_some_and(|value| value.to_ascii_lowercase().contains(needle))
|
||||
}
|
||||
|
||||
fn version_is_at_least(version: Option<&str>, minimum: (u64, u64, u64)) -> bool {
|
||||
parse_dotted_version(version).is_some_and(|version| version >= minimum)
|
||||
}
|
||||
|
||||
fn parse_dotted_version(version: Option<&str>) -> Option<(u64, u64, u64)> {
|
||||
let version = version?;
|
||||
let mut parts = version.split('.');
|
||||
let major = parts.next()?.parse().ok()?;
|
||||
let minor = parts.next().unwrap_or("0").parse().ok()?;
|
||||
let patch = parts.next().unwrap_or("0").parse().ok()?;
|
||||
|
||||
if parts.next().is_some() {
|
||||
return None;
|
||||
}
|
||||
|
||||
Some((major, minor, patch))
|
||||
}
|
||||
|
||||
pub fn kitty_delete_image(image_id: u32) -> String {
|
||||
wrap_for_tmux_if_needed(&format!("{ESC}_Ga=d,d=I,i={image_id},q=2;{ST}"))
|
||||
}
|
||||
@@ -400,16 +435,18 @@ mod tests {
|
||||
#[test]
|
||||
fn pet_image_support_detects_iterm2_kitty_file_graphics() {
|
||||
for info in [
|
||||
terminal_info_for_test(
|
||||
terminal_info_with_version_for_test(
|
||||
TerminalName::Iterm2,
|
||||
/*multiplexer*/ None,
|
||||
Some("iTerm.app"),
|
||||
Some("3.6.10"),
|
||||
/*term*/ None,
|
||||
),
|
||||
terminal_info_for_test(
|
||||
terminal_info_with_version_for_test(
|
||||
TerminalName::Unknown,
|
||||
/*multiplexer*/ None,
|
||||
Some("iTerm.app"),
|
||||
Some("3.6.10"),
|
||||
Some("xterm-256color"),
|
||||
),
|
||||
] {
|
||||
@@ -420,6 +457,49 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pet_image_support_rejects_old_iterm2_versions() {
|
||||
for info in [
|
||||
terminal_info_with_version_for_test(
|
||||
TerminalName::Iterm2,
|
||||
/*multiplexer*/ None,
|
||||
Some("iTerm.app"),
|
||||
Some("3.5.14"),
|
||||
/*term*/ None,
|
||||
),
|
||||
terminal_info_with_version_for_test(
|
||||
TerminalName::Unknown,
|
||||
/*multiplexer*/ None,
|
||||
Some("iTerm.app"),
|
||||
/*version*/ None,
|
||||
Some("xterm-256color"),
|
||||
),
|
||||
terminal_info_with_version_for_test(
|
||||
TerminalName::Iterm2,
|
||||
/*multiplexer*/ None,
|
||||
Some("iTerm.app"),
|
||||
Some("3.5"),
|
||||
/*term*/ None,
|
||||
),
|
||||
] {
|
||||
assert_eq!(
|
||||
pet_image_support_for_terminal(&info),
|
||||
PetImageSupport::Unsupported(PetImageUnsupportedReason::Iterm2TooOld)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pet_image_support_old_iterm2_message_mentions_upgrade() {
|
||||
let message = PetImageSupport::Unsupported(PetImageUnsupportedReason::Iterm2TooOld)
|
||||
.unsupported_message();
|
||||
|
||||
assert_eq!(
|
||||
message,
|
||||
Some("Pets require iTerm2 3.6 or newer. Upgrade iTerm2 to use terminal pets.")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pet_image_support_detects_kitty_graphics_terminals() {
|
||||
for info in [
|
||||
@@ -538,16 +618,42 @@ mod tests {
|
||||
multiplexer: Option<Multiplexer>,
|
||||
term_program: Option<&str>,
|
||||
term: Option<&str>,
|
||||
) -> TerminalInfo {
|
||||
terminal_info_with_version_for_test(
|
||||
name,
|
||||
multiplexer,
|
||||
term_program,
|
||||
/*version*/ None,
|
||||
term,
|
||||
)
|
||||
}
|
||||
|
||||
fn terminal_info_with_version_for_test(
|
||||
name: TerminalName,
|
||||
multiplexer: Option<Multiplexer>,
|
||||
term_program: Option<&str>,
|
||||
version: Option<&str>,
|
||||
term: Option<&str>,
|
||||
) -> TerminalInfo {
|
||||
TerminalInfo {
|
||||
name,
|
||||
term_program: term_program.map(str::to_string),
|
||||
version: /*version*/ None,
|
||||
version: version.map(str::to_string),
|
||||
term: term.map(str::to_string),
|
||||
multiplexer,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_dotted_version_requires_simple_numeric_components() {
|
||||
assert_eq!(parse_dotted_version(Some("3.6.10")), Some((3, 6, 10)));
|
||||
assert_eq!(parse_dotted_version(Some("3.6")), Some((3, 6, 0)));
|
||||
assert_eq!(parse_dotted_version(Some("3")), Some((3, 0, 0)));
|
||||
assert_eq!(parse_dotted_version(Some("3.6.10.1")), None);
|
||||
assert_eq!(parse_dotted_version(Some("3.6beta")), None);
|
||||
assert_eq!(parse_dotted_version(/*version*/ None), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sixel_frame_encodes_without_external_crate() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
|
||||
Reference in New Issue
Block a user