diff --git a/codex-rs/terminal-browser/src/actions.rs b/codex-rs/terminal-browser/src/actions.rs index 7f2544c964..dc6c21309f 100644 --- a/codex-rs/terminal-browser/src/actions.rs +++ b/codex-rs/terminal-browser/src/actions.rs @@ -5,9 +5,6 @@ use serde_json::json; use crate::cdp::CdpClient; use crate::input::BrowserKeyInput; -use crate::input::BrowserMouseButton; -use crate::input::BrowserMouseInput; -use crate::input::BrowserMouseKind; use crate::scripts; const MAX_SCREENSHOT_BYTES: usize = 4 * 1024 * 1024; @@ -16,13 +13,6 @@ const MAX_SNAPSHOT_URL_CHARS: usize = 2_048; const MAX_SNAPSHOT_TITLE_CHARS: usize = 512; const MAX_SNAPSHOT_TEXT_CHARS: usize = 6_000; -#[derive(Default)] -pub(crate) struct HumanMouseDispatchState { - viewport: Option<(u16, u16, f64, f64)>, - last_position: Option<(f64, f64)>, - buttons: u8, -} - pub(crate) struct PageMetadata { pub(crate) url: Option, pub(crate) title: Option, @@ -54,12 +44,17 @@ pub(crate) async fn press(client: &CdpClient, key: &str) -> Result Result pub(crate) async fn dispatch_human_key(client: &CdpClient, input: &BrowserKeyInput) -> Result<()> { let text = input.text.as_deref().unwrap_or_default(); + let event_type = if text.is_empty() { + "rawKeyDown" + } else { + "keyDown" + }; let modifiers = input.modifiers.cdp_mask(); client .call( "Input.dispatchKeyEvent", json!({ - "type": "rawKeyDown", + "type": event_type, "key": input.key, "code": input.code, "text": text, @@ -138,114 +138,6 @@ pub(crate) async fn dispatch_human_key(client: &CdpClient, input: &BrowserKeyInp Ok(()) } -pub(crate) async fn release_human_mouse_buttons( - client: &CdpClient, - state: &mut HumanMouseDispatchState, -) -> Result<()> { - let Some((x, y)) = state.last_position else { - state.buttons = 0; - return Ok(()); - }; - for button in [ - BrowserMouseButton::Left, - BrowserMouseButton::Middle, - BrowserMouseButton::Right, - ] { - let button_mask = button.cdp_buttons_mask(); - if state.buttons & button_mask == 0 { - continue; - } - state.buttons &= !button_mask; - client - .call( - "Input.dispatchMouseEvent", - json!({ - "type": "mouseReleased", - "x": x, - "y": y, - "button": button.as_cdp(), - "buttons": state.buttons, - "modifiers": 0, - "clickCount": 0, - "deltaX": 0.0, - "deltaY": 0.0, - }), - ) - .await?; - } - Ok(()) -} - -pub(crate) async fn insert_human_text(client: &CdpClient, text: &str) -> Result<()> { - client - .call("Input.insertText", json!({ "text": text })) - .await?; - Ok(()) -} - -pub(crate) async fn dispatch_human_mouse( - client: &CdpClient, - input: BrowserMouseInput, - state: &mut HumanMouseDispatchState, -) -> Result<()> { - anyhow::ensure!( - input.viewport_cols > 0 && input.viewport_rows > 0, - "browser viewport must be non-zero" - ); - let (width, height) = if let Some((cols, rows, width, height)) = state.viewport - && cols == input.viewport_cols - && rows == input.viewport_rows - { - (width, height) - } else { - let metrics = client.call("Page.getLayoutMetrics", json!({})).await?; - let width = metrics - .pointer("/cssLayoutViewport/clientWidth") - .or_else(|| metrics.pointer("/layoutViewport/clientWidth")) - .and_then(Value::as_f64) - .context("browser layout metrics omitted viewport width")?; - let height = metrics - .pointer("/cssLayoutViewport/clientHeight") - .or_else(|| metrics.pointer("/layoutViewport/clientHeight")) - .and_then(Value::as_f64) - .context("browser layout metrics omitted viewport height")?; - state.viewport = Some((input.viewport_cols, input.viewport_rows, width, height)); - (width, height) - }; - let x = f64::from(input.column) * width / f64::from(input.viewport_cols); - let y = f64::from(input.row) * height / f64::from(input.viewport_rows); - let modifiers = input.modifiers.cdp_mask(); - let (event_type, delta_x, delta_y) = match input.kind { - BrowserMouseKind::Move => ("mouseMoved", 0.0, 0.0), - BrowserMouseKind::Down => ("mousePressed", 0.0, 0.0), - BrowserMouseKind::Up => ("mouseReleased", 0.0, 0.0), - BrowserMouseKind::Wheel { delta_x, delta_y } => ("mouseWheel", delta_x, delta_y), - }; - match input.kind { - BrowserMouseKind::Down => state.buttons |= input.button.cdp_buttons_mask(), - BrowserMouseKind::Up => state.buttons &= !input.button.cdp_buttons_mask(), - BrowserMouseKind::Move | BrowserMouseKind::Wheel { .. } => {} - } - client - .call( - "Input.dispatchMouseEvent", - json!({ - "type": event_type, - "x": x, - "y": y, - "button": input.button.as_cdp(), - "buttons": state.buttons, - "modifiers": modifiers, - "clickCount": if matches!(input.kind, BrowserMouseKind::Down | BrowserMouseKind::Up) { 1 } else { 0 }, - "deltaX": delta_x, - "deltaY": delta_y, - }), - ) - .await?; - state.last_position = Some((x, y)); - Ok(()) -} - pub(crate) fn bounded_snapshot_json(mut snapshot: Value) -> Result { truncate_string_field(&mut snapshot, "url", MAX_SNAPSHOT_URL_CHARS); truncate_string_field(&mut snapshot, "title", MAX_SNAPSHOT_TITLE_CHARS); diff --git a/codex-rs/terminal-browser/src/actions_tests.rs b/codex-rs/terminal-browser/src/actions_tests.rs index 3c1cd17bf4..91e2ec204d 100644 --- a/codex-rs/terminal-browser/src/actions_tests.rs +++ b/codex-rs/terminal-browser/src/actions_tests.rs @@ -9,17 +9,14 @@ use tokio_tungstenite::accept_async; use tokio_tungstenite::tungstenite::Message; use super::BrowserToolOutput; -use super::HumanMouseDispatchState; -use super::dispatch_human_mouse; -use super::release_human_mouse_buttons; +use super::dispatch_human_key; +use super::press; use crate::accessibility::click; use crate::accessibility::snapshot; use crate::cdp::CdpClient; use crate::handles::BrowserHandles; use crate::input::BrowserInputModifiers; -use crate::input::BrowserMouseButton; -use crate::input::BrowserMouseInput; -use crate::input::BrowserMouseKind; +use crate::input::BrowserKeyInput; use crate::navigation::LoadState; use crate::navigation::NavigateRequest; use crate::navigation::NavigationAction; @@ -187,93 +184,45 @@ async fn native_click_uses_the_accessibility_handle_box_center() { } #[tokio::test] -async fn human_mouse_preserves_drag_buttons_and_cleans_up_held_buttons() { +async fn keyboard_input_uses_key_down_when_chromium_needs_text() { let (url, server) = test_server(|mut socket| { tokio::spawn(async move { initialize(&mut socket).await; - let metrics = request(&mut socket).await; - assert_eq!(metrics["method"], "Page.getLayoutMetrics"); - respond( - &mut socket, - &metrics, - json!({ "cssLayoutViewport": { "clientWidth": 800, "clientHeight": 600 } }), - ) - .await; - for (expected_type, expected_buttons) in - [("mousePressed", 1), ("mouseMoved", 1), ("mouseReleased", 0)] - { - let input = request(&mut socket).await; - assert_eq!(input["method"], "Input.dispatchMouseEvent"); - assert_eq!(input["params"]["type"], expected_type); - assert_eq!(input["params"]["buttons"], expected_buttons); - respond(&mut socket, &input, json!({})).await; - } - for expected_buttons in [1, 3] { - let input = request(&mut socket).await; - assert_eq!(input["params"]["type"], "mousePressed"); - assert_eq!(input["params"]["buttons"], expected_buttons); - respond(&mut socket, &input, json!({})).await; - } - for (expected_button, expected_buttons) in [("left", 2), ("right", 0)] { - let input = request(&mut socket).await; - assert_eq!(input["params"]["type"], "mouseReleased"); - assert_eq!(input["params"]["button"], expected_button); - assert_eq!(input["params"]["buttons"], expected_buttons); - assert_eq!(input["params"]["x"], 200.0); - assert_eq!(input["params"]["y"], 150.0); - assert_eq!(input["params"]["clickCount"], 0); - respond(&mut socket, &input, json!({})).await; + for (expected_key, expected_text) in [("Enter", "\r"), ("?", "?")] { + let key_down = request(&mut socket).await; + assert_eq!(key_down["method"], "Input.dispatchKeyEvent"); + assert_eq!(key_down["params"]["type"], "keyDown"); + assert_eq!(key_down["params"]["key"], expected_key); + assert_eq!(key_down["params"]["text"], expected_text); + respond(&mut socket, &key_down, json!({})).await; + + let key_up = request(&mut socket).await; + assert_eq!(key_up["method"], "Input.dispatchKeyEvent"); + assert_eq!(key_up["params"]["type"], "keyUp"); + assert_eq!(key_up["params"]["key"], expected_key); + respond(&mut socket, &key_up, json!({})).await; } }) }) .await; let client = CdpClient::connect(&url).await.expect("connect client"); - let mut state = HumanMouseDispatchState::default(); - for kind in [ - BrowserMouseKind::Down, - BrowserMouseKind::Move, - BrowserMouseKind::Up, - ] { - dispatch_human_mouse( - &client, - BrowserMouseInput { - kind, - button: BrowserMouseButton::Left, - column: 10, - row: 5, - viewport_cols: 80, - viewport_rows: 24, - modifiers: BrowserInputModifiers::default(), - }, - &mut state, - ) - .await - .expect("dispatch mouse input"); - } - for button in [BrowserMouseButton::Left, BrowserMouseButton::Right] { - dispatch_human_mouse( - &client, - BrowserMouseInput { - kind: BrowserMouseKind::Down, - button, - column: 20, - row: 6, - viewport_cols: 80, - viewport_rows: 24, - modifiers: BrowserInputModifiers::default(), - }, - &mut state, - ) - .await - .expect("dispatch mouse press"); - } - release_human_mouse_buttons(&client, &mut state) - .await - .expect("release held mouse buttons"); - release_human_mouse_buttons(&client, &mut state) - .await - .expect("repeated cleanup is empty"); + press(&client, "Enter").await.expect("press Enter"); + dispatch_human_key( + &client, + &BrowserKeyInput { + key: "?".to_string(), + code: "Slash".to_string(), + text: Some("?".to_string()), + modifiers: BrowserInputModifiers { + shift: true, + ..Default::default() + }, + }, + ) + .await + .expect("dispatch human key"); + server.await.expect("server task"); } diff --git a/codex-rs/terminal-browser/src/human_control.rs b/codex-rs/terminal-browser/src/human_control.rs index c6d45652dd..f8e3e98dc9 100644 --- a/codex-rs/terminal-browser/src/human_control.rs +++ b/codex-rs/terminal-browser/src/human_control.rs @@ -3,18 +3,20 @@ use std::sync::Arc; use std::sync::Weak; use std::sync::atomic::Ordering; +use anyhow::Context; use anyhow::Result; use anyhow::anyhow; use tokio::sync::mpsc; use tokio::sync::oneshot; use crate::actions; -use crate::actions::HumanMouseDispatchState; use crate::input::BrowserKeyInput; use crate::input::BrowserMouseInput; use crate::input::BrowserMouseKind; use crate::session::Inner; use crate::session::TerminalBrowser; +use crate::terminal_input; +use crate::terminal_input::HumanTerminalMouseState; const HUMAN_INPUT_CAPACITY: usize = 128; const HUMAN_CONTROL_INPUT_CAPACITY: usize = 1; @@ -67,6 +69,20 @@ impl HumanInput { let _ = completion_tx.send(result); } } + + fn complete_stale(self) { + match self { + Self::ReleaseMouseButtons { + completion_tx, + after_release: HumanControlAfterRelease::Continue, + } => { + let _ = completion_tx.send(Ok(())); + } + input => { + input.complete_release(Err("browser control transition was canceled".to_string())) + } + } + } } struct QueuedHumanInput { @@ -385,10 +401,10 @@ impl Drop for HumanControlTransition<'_> { #[expect( clippy::await_holding_invalid_type, - reason = "human input must remain serialized with model actions until its CDP call completes" + reason = "human input must remain serialized with model actions until Carbonyl receives it" )] async fn run_human_input_worker(inner: Weak, mut receivers: HumanInputReceivers) { - let mut mouse_state = HumanMouseDispatchState::default(); + let mut mouse_state = HumanTerminalMouseState::default(); let mut mouse_generation = None; let mut deferred = VecDeque::new(); while let Some(queued) = next_human_input(&mut receivers, &mut deferred).await { @@ -399,14 +415,14 @@ async fn run_human_input_worker(inner: Weak, mut receivers: HumanInputRec if !inner.human_control.load(Ordering::SeqCst) || inner.human_control_generation.load(Ordering::SeqCst) != generation { - input.complete_release(Err("browser control transition was canceled".to_string())); + input.complete_stale(); continue; } if matches!(&input, HumanInput::ReleaseMouseButtons { .. }) { drain_human_inputs_for_generation(&mut receivers, generation, &mut deferred); } if mouse_generation != Some(generation) { - mouse_state = HumanMouseDispatchState::default(); + mouse_state = HumanTerminalMouseState::default(); mouse_generation = Some(generation); } let (result, completion_tx, crash_on_error) = { @@ -414,7 +430,7 @@ async fn run_human_input_worker(inner: Weak, mut receivers: HumanInputRec if !inner.human_control.load(Ordering::SeqCst) || inner.human_control_generation.load(Ordering::SeqCst) != generation { - input.complete_release(Err("browser control transition was canceled".to_string())); + input.complete_stale(); continue; } let cdp = { @@ -425,23 +441,37 @@ async fn run_human_input_worker(inner: Weak, mut receivers: HumanInputRec }; session.cdp.clone() }; + let writer = inner + .process + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .as_ref() + .map(|process| process.writer_sender()) + .context("terminal browser process is not open"); match input { - HumanInput::Key(input) => { - (actions::dispatch_human_key(&cdp, &input).await, None, true) - } - HumanInput::Text(text) => { - (actions::insert_human_text(&cdp, &text).await, None, true) - } - HumanInput::Mouse(input) => ( - actions::dispatch_human_mouse(&cdp, input, &mut mouse_state).await, + HumanInput::Key(input) => match terminal_input::key_bytes(&input) { + Some(bytes) => (send_terminal_input(writer, bytes).await, None, true), + None => (actions::dispatch_human_key(&cdp, &input).await, None, true), + }, + HumanInput::Text(text) => ( + send_terminal_input(writer, text.into_bytes()).await, None, true, ), + HumanInput::Mouse(input) => { + let bytes = terminal_input::mouse_bytes(input, &mut mouse_state); + ( + send_optional_terminal_input(writer, bytes).await, + None, + true, + ) + } HumanInput::ReleaseMouseButtons { completion_tx, after_release, } => { - let result = actions::release_human_mouse_buttons(&cdp, &mut mouse_state).await; + let bytes = terminal_input::release_mouse_bytes(&mut mouse_state); + let result = send_optional_terminal_input(writer, bytes).await; let crash_on_error = result.is_err(); let result = result.and_then(|()| { finish_human_control_after_release(&inner, generation, after_release) @@ -467,6 +497,23 @@ async fn run_human_input_worker(inner: Weak, mut receivers: HumanInputRec } } +async fn send_terminal_input(writer: Result>>, bytes: Vec) -> Result<()> { + writer? + .send(bytes) + .await + .map_err(|_| anyhow!("terminal browser input channel closed")) +} + +async fn send_optional_terminal_input( + writer: Result>>, + bytes: Option>, +) -> Result<()> { + match bytes { + Some(bytes) => send_terminal_input(writer, bytes).await, + None => Ok(()), + } +} + async fn next_human_input( receivers: &mut HumanInputReceivers, deferred: &mut VecDeque, diff --git a/codex-rs/terminal-browser/src/human_control_tests.rs b/codex-rs/terminal-browser/src/human_control_tests.rs index cbe6e86cfe..5da5fcef1c 100644 --- a/codex-rs/terminal-browser/src/human_control_tests.rs +++ b/codex-rs/terminal-browser/src/human_control_tests.rs @@ -92,6 +92,28 @@ async fn release_discards_its_backlog_and_preserves_a_new_generation() { assert!(deferred.is_empty()); } +#[tokio::test] +async fn stale_cleanup_release_is_idempotent_while_stale_end_stays_rejected() { + let (cleanup_tx, cleanup_rx) = oneshot::channel(); + HumanInput::ReleaseMouseButtons { + completion_tx: cleanup_tx, + after_release: HumanControlAfterRelease::Continue, + } + .complete_stale(); + let (end_tx, end_rx) = oneshot::channel(); + HumanInput::ReleaseMouseButtons { + completion_tx: end_tx, + after_release: HumanControlAfterRelease::End, + } + .complete_stale(); + + assert_eq!(cleanup_rx.await.expect("cleanup acknowledgement"), Ok(())); + assert_eq!( + end_rx.await.expect("end acknowledgement"), + Err("browser control transition was canceled".to_string()) + ); +} + #[test] fn ending_release_deactivates_before_acknowledgement() { let browser = TerminalBrowser::discover(); diff --git a/codex-rs/terminal-browser/src/input.rs b/codex-rs/terminal-browser/src/input.rs index 8ab94bfffc..615396bbcf 100644 --- a/codex-rs/terminal-browser/src/input.rs +++ b/codex-rs/terminal-browser/src/input.rs @@ -43,26 +43,6 @@ pub enum BrowserMouseButton { Right, } -impl BrowserMouseButton { - pub(crate) fn as_cdp(self) -> &'static str { - match self { - Self::None => "none", - Self::Left => "left", - Self::Middle => "middle", - Self::Right => "right", - } - } - - pub(crate) fn cdp_buttons_mask(self) -> u8 { - match self { - Self::None => 0, - Self::Left => 1, - Self::Right => 2, - Self::Middle => 4, - } - } -} - /// Kind of mouse event forwarded during exclusive human control. #[derive(Clone, Copy, Debug, PartialEq)] pub enum BrowserMouseKind { @@ -76,7 +56,7 @@ pub enum BrowserMouseKind { Wheel { delta_x: f64, delta_y: f64 }, } -/// Terminal-relative mouse event that Carbonyl maps into its current CSS viewport. +/// Terminal-relative mouse event forwarded to Carbonyl's native input parser. #[derive(Clone, Copy, Debug, PartialEq)] pub struct BrowserMouseInput { /// Mouse operation to perform. diff --git a/codex-rs/terminal-browser/src/lib.rs b/codex-rs/terminal-browser/src/lib.rs index 0155f62490..cbe7327327 100644 --- a/codex-rs/terminal-browser/src/lib.rs +++ b/codex-rs/terminal-browser/src/lib.rs @@ -19,6 +19,7 @@ mod sandbox; mod screen; mod scripts; mod session; +mod terminal_input; mod tool_dispatch; mod url_policy; diff --git a/codex-rs/terminal-browser/src/scripts.rs b/codex-rs/terminal-browser/src/scripts.rs index 5c260695a7..c984aa277f 100644 --- a/codex-rs/terminal-browser/src/scripts.rs +++ b/codex-rs/terminal-browser/src/scripts.rs @@ -20,3 +20,11 @@ pub(crate) fn key_code(key: &str) -> &str { _ => key, } } + +pub(crate) fn key_text(key: &str) -> &str { + match key { + "Enter" => "\r", + _ if key.chars().count() == 1 => key, + _ => "", + } +} diff --git a/codex-rs/terminal-browser/src/terminal_input.rs b/codex-rs/terminal-browser/src/terminal_input.rs new file mode 100644 index 0000000000..b357b9b55f --- /dev/null +++ b/codex-rs/terminal-browser/src/terminal_input.rs @@ -0,0 +1,128 @@ +use crate::input::BrowserInputModifiers; +use crate::input::BrowserKeyInput; +use crate::input::BrowserMouseButton; +use crate::input::BrowserMouseInput; +use crate::input::BrowserMouseKind; + +/// Tracks the pointer state Carbonyl expects across terminal mouse reports. +#[derive(Default)] +pub(crate) struct HumanTerminalMouseState { + last_position: Option<(u16, u16)>, + pressed_button: BrowserMouseButton, +} + +/// Encodes a browser key as terminal input when Carbonyl understands that key directly. +/// +/// Modified printable shortcuts fall back to CDP so combinations such as Ctrl+C do not become +/// Carbonyl process-control bytes. +pub(crate) fn key_bytes(input: &BrowserKeyInput) -> Option> { + if let Some(text) = input.text.as_deref() + && !text.is_empty() + { + return Some(text.as_bytes().to_vec()); + } + match input.key.as_str() { + "Enter" => Some(vec![b'\r']), + "Tab" if !input.modifiers.shift => Some(vec![b'\t']), + "Backspace" => Some(vec![0x7f]), + "Escape" => Some(vec![0x1b]), + "ArrowUp" => Some(arrow_bytes(/*key*/ b'A', input.modifiers)), + "ArrowDown" => Some(arrow_bytes(/*key*/ b'B', input.modifiers)), + "ArrowRight" => Some(arrow_bytes(/*key*/ b'C', input.modifiers)), + "ArrowLeft" => Some(arrow_bytes(/*key*/ b'D', input.modifiers)), + "Tab" | "Delete" | "Home" | "End" | "PageUp" | "PageDown" => None, + _ => None, + } +} + +pub(crate) fn mouse_bytes( + input: BrowserMouseInput, + state: &mut HumanTerminalMouseState, +) -> Option> { + state.last_position = Some((input.column, input.row)); + let modifiers = mouse_modifier_bits(input.modifiers); + let (button_code, suffix) = match input.kind { + BrowserMouseKind::Down => { + state.pressed_button = input.button; + (mouse_button_code(input.button)? + modifiers, b'M') + } + BrowserMouseKind::Up => { + state.pressed_button = BrowserMouseButton::None; + (mouse_button_code(input.button)? + modifiers, b'm') + } + BrowserMouseKind::Move => { + let button = if state.pressed_button == BrowserMouseButton::None { + input.button + } else { + state.pressed_button + }; + let button_code = mouse_button_code(button).unwrap_or(/*default*/ 3); + (button_code + 32 + modifiers, b'M') + } + BrowserMouseKind::Wheel { delta_y, .. } if delta_y < 0.0 => (64 + modifiers, b'M'), + BrowserMouseKind::Wheel { delta_y, .. } if delta_y > 0.0 => (65 + modifiers, b'M'), + BrowserMouseKind::Wheel { .. } => return None, + }; + Some(sgr_mouse_bytes( + button_code, + input.column, + input.row, + suffix, + )) +} + +pub(crate) fn release_mouse_bytes(state: &mut HumanTerminalMouseState) -> Option> { + let button = std::mem::take(&mut state.pressed_button); + let (column, row) = state.last_position?; + Some(sgr_mouse_bytes( + mouse_button_code(button)?, + column, + row, + /*suffix*/ b'm', + )) +} + +fn arrow_bytes(key: u8, modifiers: BrowserInputModifiers) -> Vec { + let modifier = keyboard_modifier_code(modifiers); + if modifier == 1 { + vec![0x1b, b'[', key] + } else { + format!("\x1b[1;{modifier}{key}", key = char::from(key)).into_bytes() + } +} + +fn keyboard_modifier_code(modifiers: BrowserInputModifiers) -> u8 { + 1 + u8::from(modifiers.shift) + + (u8::from(modifiers.alt) << 1) + + (u8::from(modifiers.control) << 2) + + (u8::from(modifiers.meta) << 3) +} + +fn mouse_modifier_bits(modifiers: BrowserInputModifiers) -> u8 { + (u8::from(modifiers.shift) << 2) + | (u8::from(modifiers.alt || modifiers.meta) << 3) + | (u8::from(modifiers.control) << 4) +} + +fn mouse_button_code(button: BrowserMouseButton) -> Option { + match button { + BrowserMouseButton::None => None, + BrowserMouseButton::Left => Some(0), + BrowserMouseButton::Middle => Some(1), + BrowserMouseButton::Right => Some(2), + } +} + +fn sgr_mouse_bytes(button: u8, column: u16, row: u16, suffix: u8) -> Vec { + format!( + "\x1b[<{button};{};{}{}", + column.saturating_add(/*rhs*/ 1), + row.saturating_add(/*rhs*/ 1), + char::from(suffix) + ) + .into_bytes() +} + +#[cfg(test)] +#[path = "terminal_input_tests.rs"] +mod tests; diff --git a/codex-rs/terminal-browser/src/terminal_input_tests.rs b/codex-rs/terminal-browser/src/terminal_input_tests.rs new file mode 100644 index 0000000000..e4915d761f --- /dev/null +++ b/codex-rs/terminal-browser/src/terminal_input_tests.rs @@ -0,0 +1,106 @@ +use pretty_assertions::assert_eq; + +use super::*; + +#[test] +fn plain_text_and_navigation_keys_use_carbonyl_terminal_input() { + let text = BrowserKeyInput { + key: "?".to_string(), + code: "Slash".to_string(), + text: Some("?".to_string()), + modifiers: BrowserInputModifiers { + shift: true, + ..Default::default() + }, + }; + let enter = BrowserKeyInput { + key: "Enter".to_string(), + code: "Enter".to_string(), + text: None, + modifiers: BrowserInputModifiers::default(), + }; + let command_left = BrowserKeyInput { + key: "ArrowLeft".to_string(), + code: "ArrowLeft".to_string(), + text: None, + modifiers: BrowserInputModifiers { + meta: true, + ..Default::default() + }, + }; + + assert_eq!(key_bytes(&text), Some(b"?".to_vec())); + assert_eq!(key_bytes(&enter), Some(b"\r".to_vec())); + assert_eq!(key_bytes(&command_left), Some(b"\x1b[1;9D".to_vec())); +} + +#[test] +fn modified_printable_shortcuts_fall_back_to_cdp() { + let control_c = BrowserKeyInput { + key: "c".to_string(), + code: "KeyC".to_string(), + text: None, + modifiers: BrowserInputModifiers { + control: true, + ..Default::default() + }, + }; + + assert_eq!(key_bytes(&control_c), None); +} + +#[test] +fn mouse_reports_preserve_carbonyl_coordinates_and_button_state() { + let mut state = HumanTerminalMouseState::default(); + let press = BrowserMouseInput { + kind: BrowserMouseKind::Down, + button: BrowserMouseButton::Left, + column: 1, + row: 0, + viewport_cols: 40, + viewport_rows: 20, + modifiers: BrowserInputModifiers::default(), + }; + let drag = BrowserMouseInput { + kind: BrowserMouseKind::Move, + column: 2, + row: 1, + ..press + }; + + assert_eq!( + mouse_bytes(press, &mut state), + Some(b"\x1b[<0;2;1M".to_vec()) + ); + assert_eq!( + mouse_bytes(drag, &mut state), + Some(b"\x1b[<32;3;2M".to_vec()) + ); + assert_eq!( + release_mouse_bytes(&mut state), + Some(b"\x1b[<0;3;2m".to_vec()) + ); + assert_eq!(release_mouse_bytes(&mut state), None); +} + +#[test] +fn wheel_reports_use_carbonyl_scroll_buttons() { + let mut state = HumanTerminalMouseState::default(); + let scroll_up = BrowserMouseInput { + kind: BrowserMouseKind::Wheel { + delta_x: 0.0, + delta_y: -100.0, + }, + button: BrowserMouseButton::None, + column: 4, + row: 3, + viewport_cols: 40, + viewport_rows: 20, + modifiers: BrowserInputModifiers::default(), + }; + + assert_eq!( + mouse_bytes(scroll_up, &mut state), + Some(b"\x1b[<64;5;4M".to_vec()) + ); +} diff --git a/codex-rs/terminal-browser/src/url_policy.rs b/codex-rs/terminal-browser/src/url_policy.rs index fd90b7335d..38fad90355 100644 --- a/codex-rs/terminal-browser/src/url_policy.rs +++ b/codex-rs/terminal-browser/src/url_policy.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use tokio::task::JoinHandle; use url::Url; +use crate::actions; use crate::cdp::CdpClient; use crate::cdp::CdpEvent; use crate::session::Inner; @@ -24,17 +25,15 @@ pub(crate) fn spawn_navigation_policy_task(inner: Arc, cdp: CdpClient) -> loop { match events.recv().await { Ok(CdpEvent::Message(message)) => { - let is_main_frame = message.get("method").and_then(serde_json::Value::as_str) - == Some("Page.frameNavigated") - && message.pointer("/params/frame/parentId").is_none(); - let Some(url) = is_main_frame - .then(|| message.pointer("/params/frame/url")) - .flatten() - .and_then(serde_json::Value::as_str) - else { + if is_page_load_event(&message) { + refresh_observed_page_metadata(&inner, &cdp).await; + continue; + } + let Some(url) = observed_main_frame_url(&message) else { continue; }; if is_allowed_observed_url(url) { + inner.update_view(|view| view.url = Some(url.to_string())); continue; } tracing::warn!("blocked disallowed terminal-browser main-frame navigation"); @@ -54,3 +53,31 @@ pub(crate) fn spawn_navigation_policy_task(inner: Arc, cdp: CdpClient) -> } }) } + +fn observed_main_frame_url(message: &serde_json::Value) -> Option<&str> { + (message.get("method").and_then(serde_json::Value::as_str) == Some("Page.frameNavigated") + && message.pointer("/params/frame/parentId").is_none()) + .then(|| message.pointer("/params/frame/url")) + .flatten() + .and_then(serde_json::Value::as_str) +} + +fn is_page_load_event(message: &serde_json::Value) -> bool { + message.get("method").and_then(serde_json::Value::as_str) == Some("Page.loadEventFired") +} + +async fn refresh_observed_page_metadata(inner: &Inner, cdp: &CdpClient) { + let Ok(metadata) = actions::page_metadata(cdp).await else { + return; + }; + if metadata.url.as_deref().is_none_or(is_allowed_observed_url) { + inner.update_view(|view| { + view.url = metadata.url; + view.title = metadata.title; + }); + } +} + +#[cfg(test)] +#[path = "url_policy_tests.rs"] +mod tests; diff --git a/codex-rs/terminal-browser/src/url_policy_tests.rs b/codex-rs/terminal-browser/src/url_policy_tests.rs new file mode 100644 index 0000000000..598a053c6d --- /dev/null +++ b/codex-rs/terminal-browser/src/url_policy_tests.rs @@ -0,0 +1,40 @@ +use pretty_assertions::assert_eq; +use serde_json::json; + +use super::*; + +#[test] +fn observes_only_main_frame_navigation_urls() { + let main_frame = json!({ + "method": "Page.frameNavigated", + "params": { "frame": { "id": "main", "url": "https://example.test/next" } } + }); + let child_frame = json!({ + "method": "Page.frameNavigated", + "params": { + "frame": { + "id": "child", + "parentId": "main", + "url": "https://example.test/frame" + } + } + }); + + assert_eq!( + observed_main_frame_url(&main_frame), + Some("https://example.test/next") + ); + assert_eq!(observed_main_frame_url(&child_frame), None); +} + +#[test] +fn recognizes_page_load_events_for_metadata_refresh() { + assert!(is_page_load_event(&json!({ + "method": "Page.loadEventFired", + "params": { "timestamp": 1 } + }))); + assert!(!is_page_load_event(&json!({ + "method": "Page.domContentEventFired", + "params": { "timestamp": 1 } + }))); +} diff --git a/codex-rs/tui/src/app/terminal_browser.rs b/codex-rs/tui/src/app/terminal_browser.rs index 886cf75e71..78e70cf7e9 100644 --- a/codex-rs/tui/src/app/terminal_browser.rs +++ b/codex-rs/tui/src/app/terminal_browser.rs @@ -792,6 +792,10 @@ impl App { ); return; } + if !browser.is_human_control_active() { + self.owned_screen_frame + .select_right_rail_content(OwnedScreenRightRailContent::Browser); + } let app_event_tx = self.app_event_tx.unscoped(); tokio::spawn(async move { let result = browser.toggle_human_control(target.token).await; diff --git a/codex-rs/tui/src/app/terminal_browser_tests.rs b/codex-rs/tui/src/app/terminal_browser_tests.rs index 26dad17c90..f0e26eea4d 100644 --- a/codex-rs/tui/src/app/terminal_browser_tests.rs +++ b/codex-rs/tui/src/app/terminal_browser_tests.rs @@ -165,6 +165,36 @@ async fn visible_browser_updates_select_the_shared_right_rail_and_hidden_updates assert!(!browser.view().visible); } +#[tokio::test] +async fn terminal_browser_control_command_focuses_the_browser_rail() { + let mut app = make_test_app().await; + app.chat_widget.owned_screen = App::owned_screen_for_behavior( + AltScreenBehavior::Owned, + &app.chat_widget, + app.keymap.pager.clone(), + ); + app.chat_widget + .set_feature_enabled(Feature::TerminalBrowser, /*enabled*/ true); + let thread_id = ThreadId::new(); + app.chat_widget + .by_slot_mut(PaneSlot::Parent) + .expect("parent pane") + .attach_thread(thread_id, /*receiver*/ None); + let browser = Arc::new(TerminalBrowser::discover()); + browser.set_visibility(/*visible*/ true); + app.terminal_browser = Some(browser); + app.terminal_browser_owner_thread_id = Some(thread_id); + app.owned_screen_frame + .set_right_rail_content(OwnedScreenRightRailContent::Browser); + + app.toggle_terminal_browser_control().await; + + assert_eq!( + app.owned_screen_frame.focus(), + crate::app::owned_screen_frame::OwnedScreenFrameFocus::Summary + ); +} + #[cfg(any(target_os = "macos", target_os = "linux"))] #[tokio::test] async fn managed_network_without_runtime_blocks_panel_but_not_doctor() {