From a0474ae770ed185f6531a3c69ab64c60bed2c670 Mon Sep 17 00:00:00 2001 From: Can Celik Date: Fri, 11 Sep 2026 22:58:01 +0300 Subject: [PATCH 01/16] fix: preserve whole-word selection while dragging (#3969) refs #1836 --- .../website/src/content/docs/quick-start.mdx | 2 +- src/client/shell.rs | 2 + src/client/shell/actions.rs | 99 +--- src/client/shell/input.rs | 4 +- src/client/shell/mouse.rs | 86 ++-- src/client/shell/state.rs | 90 ++-- src/client/shell/tests/mouse_selection.rs | 440 ++++++++++++++---- src/client/shell/word_selection.rs | 216 +++++++++ 8 files changed, 680 insertions(+), 259 deletions(-) create mode 100644 src/client/shell/word_selection.rs diff --git a/docs/next/website/src/content/docs/quick-start.mdx b/docs/next/website/src/content/docs/quick-start.mdx index 5ac2d56d36..8b34de9146 100644 --- a/docs/next/website/src/content/docs/quick-start.mdx +++ b/docs/next/website/src/content/docs/quick-start.mdx @@ -17,7 +17,7 @@ When a session has no workspaces, Herdr opens one automatically. A workspace is ## Use the mouse -Herdr is mouse-native, so start by clicking panes, tabs, workspaces, and agents to focus them. Drag split borders to resize. Right-click for context menus, including splitting panes and creating tabs. Drag-select text to copy it to your clipboard; double-click a token to copy it directly. Copying does not require Ctrl+C. +Herdr is mouse-native, so start by clicking panes, tabs, workspaces, and agents to focus them. Drag split borders to resize. Right-click for context menus, including splitting panes and creating tabs. Drag-select text to copy it to your clipboard. Double-click a token to select it, or hold the second press and drag to extend the selection by whole words. Both gestures copy when you release the mouse. Copying does not require Ctrl+C. Ctrl-click opens pane links when your terminal sends the modified click to Herdr. This works for OSC 8 hyperlinks and visible `http://` or `https://` URLs. On macOS, use Ctrl-click for Herdr-handled pane links while mouse capture is enabled; Cmd-click is only available through the terminal-native bypass path, such as Shift-Cmd-click or `ui.mouse_capture = false`. diff --git a/src/client/shell.rs b/src/client/shell.rs index 98b77d25e8..4c2fc6cdb4 100644 --- a/src/client/shell.rs +++ b/src/client/shell.rs @@ -31,7 +31,9 @@ mod scroll; mod settings; mod state; mod surface_patch; +mod word_selection; mod worktrees; +use word_selection::ClientWordSelection; pub(in crate::client::shell) use render::sidebar; pub(crate) use state::*; diff --git a/src/client/shell/actions.rs b/src/client/shell/actions.rs index 119988f611..7fb2b6ced0 100644 --- a/src/client/shell/actions.rs +++ b/src/client/shell/actions.rs @@ -294,54 +294,6 @@ impl ClientShellState { ); } - pub(super) fn request_word_selection( - &mut self, - hit: &PaneHit, - viewport_row: u16, - col: u16, - outcome: &mut ClientShellInput, - ) { - let absolute_row = crate::selection::absolute_row_for_viewport(viewport_row, hit.scroll); - let content_revision = self - .pane_surface - .as_ref() - .and_then(|surface| { - surface - .panes - .iter() - .find(|pane| pane.pane_id == hit.pane_id) - }) - .map(|pane| pane.content_revision); - self.word_selection_generation = self.word_selection_generation.saturating_add(1); - let generation = self.word_selection_generation; - self.pending_word_selection = Some(generation); - if !self.push_endpoint_method_with_kind( - crate::api::schema::Method::PaneSelectionRead( - crate::api::schema::PaneSelectionReadParams { - pane_id: hit.pane_id.clone(), - anchor: crate::api::schema::PaneTextPoint { - row: absolute_row, - col: 0, - }, - cursor: crate::api::schema::PaneTextPoint { - row: absolute_row, - col: hit.inner_rect.width.saturating_sub(1), - }, - content_revision, - }, - ), - PendingEndpointKind::WordSelection { - pane_id: hit.pane_id.clone(), - absolute_row, - col, - generation, - }, - outcome, - ) { - self.pending_word_selection = None; - } - } - pub(super) fn push_endpoint_method( &mut self, method: crate::api::schema::Method, @@ -666,58 +618,9 @@ impl ClientShellState { PendingEndpointKind::WordSelection { pane_id, absolute_row, - col, generation, } => { - if self.pending_word_selection != Some(generation) - || self.snapshot.as_deref().is_none_or(|snapshot| { - !snapshot.panes.iter().any(|pane| pane.pane_id == pane_id) - }) - { - return (false, Vec::new()); - } - self.pending_word_selection = None; - let row_text = match result { - Ok(crate::api::schema::ResponseResult::PaneSelection { - pane_id: returned_pane_id, - text, - }) if returned_pane_id == pane_id => text, - Ok(crate::api::schema::ResponseResult::PaneSelection { .. }) => { - return (false, Vec::new()) - } - Ok(_) => { - self.endpoint_error = Some( - "endpoint returned an unexpected word-selection result".to_owned(), - ); - return (true, Vec::new()); - } - Err(_) => return (true, Vec::new()), - }; - let Some((start_col, end_col)) = - crate::app::actions::word_bounds_at_column(&row_text, col) - else { - self.selection = None; - return (true, Vec::new()); - }; - let mut selection = crate::selection::Selection::absolute_range( - pane_id, - (absolute_row, start_col), - (absolute_row, end_col), - ); - if !selection.finish() { - return (false, Vec::new()); - } - self.selection = Some(selection); - self.selection_autoscroll = None; - self.selection_autoscroll_deadline = None; - if !self.config.copy_on_select { - return (true, Vec::new()); - } - self.selection_highlight_clear_deadline = - Some(std::time::Instant::now() + std::time::Duration::from_millis(500)); - let mut outcome = ClientShellInput::default(); - self.request_selection_copy(&mut outcome, false); - return (true, outcome.actions); + return self.complete_word_selection_row(pane_id, absolute_row, generation, result); } PendingEndpointKind::PaneLinkActivate { pane_id, diff --git a/src/client/shell/input.rs b/src/client/shell/input.rs index 41a9f64ced..8a3684a3db 100644 --- a/src/client/shell/input.rs +++ b/src/client/shell/input.rs @@ -134,7 +134,7 @@ impl ClientShellState { outcome.repaint = true; return true; } - self.pending_word_selection = None; + self.word_selection_gesture = None; if self.copy_or_terminal_mode() != ClientShellMode::Copy && self.selection.take().is_some() { self.stop_selection_autoscroll(); @@ -526,7 +526,7 @@ impl ClientShellState { if matches!(key.code, KeyCode::Modifier(_)) { return None; } - self.pending_word_selection = None; + self.word_selection_gesture = None; if self.mode != ClientShellMode::Copy && self.copy_or_terminal_mode() != ClientShellMode::Copy && !self.config.copy_on_select diff --git a/src/client/shell/mouse.rs b/src/client/shell/mouse.rs index af324174d1..912e81edba 100644 --- a/src/client/shell/mouse.rs +++ b/src/client/shell/mouse.rs @@ -162,14 +162,44 @@ impl ClientShellState { ) } + fn active_selection_pane(&self) -> Option { + let pane_id = if let Some(gesture) = self.word_selection_gesture.as_ref() { + if gesture.released { + return None; + } + &gesture.pane_id + } else { + &self + .selection + .as_ref() + .filter(|selection| selection.is_in_progress())? + .pane_id + }; + self.hits + .panes + .iter() + .find(|hit| &hit.pane_id == pane_id) + .cloned() + } + fn update_selection_cursor_with_metrics( &mut self, hit: &PaneHit, column: u16, row: u16, metrics: Option, + outcome: &mut ClientShellInput, ) { - if let Some(selection) = self.selection.as_mut() { + if self.word_selection_gesture.is_some() { + let viewport_row = row + .saturating_sub(hit.inner_rect.y) + .min(hit.inner_rect.height.saturating_sub(1)); + let col = column + .saturating_sub(hit.inner_rect.x) + .min(hit.inner_rect.width.saturating_sub(1)); + let absolute_row = crate::selection::absolute_row_for_viewport(viewport_row, metrics); + self.drag_word_selection((absolute_row, col), outcome); + } else if let Some(selection) = self.selection.as_mut() { selection.drag(column, row, hit.inner_rect, metrics); } } @@ -190,8 +220,11 @@ impl ClientShellState { let (anchor_row, anchor_col) = selection.anchor_screen_pos(hit.inner_rect, metrics); anchor_row != row || anchor_col != column }); - let is_dragging = was_dragging || moved_from_anchor; - self.update_selection_cursor_with_metrics(hit, column, row, metrics); + self.update_selection_cursor_with_metrics(hit, column, row, metrics, outcome); + let is_dragging = self + .word_selection_gesture + .as_ref() + .map_or(was_dragging || moved_from_anchor, |gesture| gesture.dragged); if is_dragging { if let Some(selection) = self.selection.as_mut() { if selection.is_just_click() { @@ -244,7 +277,7 @@ impl ClientShellState { offset_from_bottom, ..metrics }; - self.update_selection_cursor_with_metrics(hit, column, row, Some(projected)); + self.update_selection_cursor_with_metrics(hit, column, row, Some(projected), outcome); self.push_pane_scroll_offset(hit.pane_id.clone(), offset_from_bottom, outcome); } self.selection_autoscroll = Some(ClientSelectionAutoscroll { @@ -268,20 +301,10 @@ impl ClientShellState { if !matches!( mouse.kind, MouseEventKind::ScrollUp | MouseEventKind::ScrollDown - ) || !self - .selection - .as_ref() - .is_some_and(crate::selection::Selection::is_in_progress) - { + ) { return false; } - let Some(hit) = self.selection.as_ref().and_then(|selection| { - self.hits - .panes - .iter() - .find(|hit| hit.pane_id == selection.pane_id) - .cloned() - }) else { + let Some(hit) = self.active_selection_pane() else { return false; }; let Some(metrics) = self.selection_scroll_metrics(&hit) else { @@ -307,6 +330,7 @@ impl ClientShellState { mouse.column, mouse.row, Some(projected), + outcome, ); self.push_pane_scroll_offset(hit.pane_id, offset_from_bottom, outcome); outcome.repaint = true; @@ -344,9 +368,15 @@ impl ClientShellState { self.selection_autoscroll_deadline = None; return outcome; }; - if !self.selection.as_ref().is_some_and(|selection| { - selection.pane_id == autoscroll.pane_id && selection.is_dragging() - }) { + let dragging = self.word_selection_gesture.as_ref().map_or_else( + || { + self.selection.as_ref().is_some_and(|selection| { + selection.pane_id == autoscroll.pane_id && selection.is_dragging() + }) + }, + |gesture| gesture.pane_id == autoscroll.pane_id && gesture.dragged && !gesture.released, + ); + if !dragging { self.stop_selection_autoscroll(); return outcome; } @@ -388,6 +418,7 @@ impl ClientShellState { autoscroll.last_mouse_column, autoscroll.last_mouse_row, Some(metrics), + &mut outcome, ); self.push_pane_scroll_offset(autoscroll.pane_id.clone(), next_offset, &mut outcome); self.selection_autoscroll = Some(autoscroll); @@ -1659,13 +1690,7 @@ impl ClientShellState { } if mouse.kind == MouseEventKind::Drag(MouseButton::Left) { - let selection_hit = self.selection.as_ref().and_then(|selection| { - self.hits - .panes - .iter() - .find(|hit| hit.pane_id == selection.pane_id) - .cloned() - }); + let selection_hit = self.active_selection_pane(); if let Some(hit) = selection_hit { self.update_selection_drag(&hit, mouse.column, mouse.row, outcome); // Consume every motion, but do not rebuild a frame for every intermediate position. @@ -1674,6 +1699,13 @@ impl ClientShellState { return; } } + if mouse.kind == MouseEventKind::Up(MouseButton::Left) + && self.word_selection_gesture.is_some() + { + self.finish_word_selection(outcome); + outcome.repaint = true; + return; + } if mouse.kind == MouseEventKind::Up(MouseButton::Left) && self.selection.is_some() { self.stop_selection_autoscroll(); let copied = self @@ -1854,7 +1886,7 @@ impl ClientShellState { } self.stop_selection_autoscroll(); self.selection_highlight_clear_deadline = None; - self.pending_word_selection = None; + self.word_selection_gesture = None; let previous_pane_click = self.last_pane_click.take(); self.workspace_press = None; self.tab_press = None; diff --git a/src/client/shell/state.rs b/src/client/shell/state.rs index 9507797d70..e2e406a289 100644 --- a/src/client/shell/state.rs +++ b/src/client/shell/state.rs @@ -696,7 +696,6 @@ pub(super) enum PendingEndpointKind { WordSelection { pane_id: String, absolute_row: u32, - col: u16, generation: u64, }, PaneLinkActivate { @@ -941,7 +940,7 @@ pub(crate) struct ClientShellState { pub(super) selection_autoscroll: Option, pub(super) selection_autoscroll_deadline: Option, pub(super) selection_highlight_clear_deadline: Option, - pub(super) pending_word_selection: Option, + pub(super) word_selection_gesture: Option, pub(super) word_selection_generation: u64, pub(super) copy_mode: Option, pub(super) copy_session_generation: u64, @@ -1099,7 +1098,7 @@ impl ClientShellState { selection_autoscroll: None, selection_autoscroll_deadline: None, selection_highlight_clear_deadline: None, - pending_word_selection: None, + word_selection_gesture: None, word_selection_generation: 0, copy_mode: None, copy_session_generation: 0, @@ -1288,7 +1287,7 @@ impl ClientShellState { self.selection_autoscroll = None; self.selection_autoscroll_deadline = None; self.selection_highlight_clear_deadline = None; - self.pending_word_selection = None; + self.word_selection_gesture = None; self.copy_mode = None; if self.mode == ClientShellMode::Copy { self.mode = ClientShellMode::Terminal; @@ -1431,18 +1430,32 @@ impl ClientShellState { { self.reveal_focused_tab = true; } - if self.selection.as_ref().is_some_and(|selection| { - snapshot.focused_pane_id.as_deref() != Some(selection.pane_id.as_str()) - || !snapshot - .panes - .iter() - .any(|pane| pane.pane_id == selection.pane_id) - }) { + let selection_focus_lost = if let Some(gesture) = self.word_selection_gesture.as_mut() { + let focused_pane = snapshot.focused_pane_id.as_deref(); + // Remember confirmed focus across intermediate snapshots with no + // focused pane, without rejecting the gesture's in-flight focus request. + gesture.focus_confirmed |= focused_pane == Some(gesture.pane_id.as_str()); + !snapshot + .panes + .iter() + .any(|pane| pane.pane_id == gesture.pane_id) + || (gesture.focus_confirmed + && focused_pane.is_some_and(|pane_id| pane_id != gesture.pane_id)) + } else { + self.selection.as_ref().is_some_and(|selection| { + snapshot.focused_pane_id.as_deref() != Some(selection.pane_id.as_str()) + || !snapshot + .panes + .iter() + .any(|pane| pane.pane_id == selection.pane_id) + }) + }; + if selection_focus_lost { self.selection = None; self.selection_autoscroll = None; self.selection_autoscroll_deadline = None; self.selection_highlight_clear_deadline = None; - self.pending_word_selection = None; + self.word_selection_gesture = None; self.last_pane_click = None; } if let Some(copy_pane_id) = self @@ -1667,7 +1680,7 @@ impl ClientShellState { self.selection_autoscroll = None; self.selection_autoscroll_deadline = None; self.selection_highlight_clear_deadline = None; - self.pending_word_selection = None; + self.word_selection_gesture = None; self.copy_mode = None; self.reset_copy_pipeline(); self.chrome_drag = None; @@ -1685,38 +1698,51 @@ impl ClientShellState { self.popup_pending = false; self.popup_pending_deadline = None; } - let selection_content_changed = self.selection.as_ref().is_some_and(|selection| { + let selection_pane = match &self.word_selection_gesture { + Some(gesture) => Some(&gesture.pane_id), + None => self.selection.as_ref().map(|selection| &selection.pane_id), + }; + let selection_content_changed = selection_pane.is_some_and(|pane_id| { let Some(previous_surface) = self.pane_surface.as_ref() else { return false; }; let previous = previous_surface .panes .iter() - .find(|pane| pane.pane_id == selection.pane_id); - let next = surface - .panes - .iter() - .find(|pane| pane.pane_id == selection.pane_id); + .find(|pane| &pane.pane_id == pane_id); + let next = surface.panes.iter().find(|pane| &pane.pane_id == pane_id); let (Some(previous), Some(next)) = (previous, next) else { return false; }; - previous.inner_rect.width != next.inner_rect.width + if previous.inner_rect.width != next.inner_rect.width || previous.inner_rect.height != next.inner_rect.height || previous.alternate_screen_active != next.alternate_screen_active - // Manual mouse selections track a live buffer range, not a content revision. - || (self.config.copy_on_select - && previous.content_revision != next.content_revision - && (!previous.content_revision.is_multiple_of(2) - || !next.content_revision.is_multiple_of(2) - || !selection_cells_unchanged( - selection, - previous_surface, - previous, - &surface, - next, - ))) + { + return true; + } + if previous.content_revision == next.content_revision { + return false; + } + match (&self.word_selection_gesture, &self.selection) { + // Word gestures cache boundaries outside the selected cells too. + (Some(_), _) => true, + (None, Some(selection)) => { + self.config.copy_on_select + && (!previous.content_revision.is_multiple_of(2) + || !next.content_revision.is_multiple_of(2) + || !selection_cells_unchanged( + selection, + previous_surface, + previous, + &surface, + next, + )) + } + (None, None) => false, + } }); if selection_content_changed { + self.word_selection_gesture = None; self.selection = None; self.stop_selection_autoscroll(); self.selection_highlight_clear_deadline = None; diff --git a/src/client/shell/tests/mouse_selection.rs b/src/client/shell/tests/mouse_selection.rs index ea78705afc..0a54203e44 100644 --- a/src/client/shell/tests/mouse_selection.rs +++ b/src/client/shell/tests/mouse_selection.rs @@ -300,128 +300,370 @@ fn disabled_mouse_chrome_keeps_tab_wheel_but_removes_split_drag_hits() { } #[test] -fn client_double_click_selects_and_copies_endpoint_row_word() { - // The row response may arrive on either side of the second mouse release. - for (copy_on_select, release_before_response) in - [(false, false), (false, true), (true, false), (true, true)] - { - let mut config = Config::default(); - config.ui.copy_on_select = copy_on_select; - let mut state = ClientShellState::new(ClientShellConfig::from_config(&config)); - state.set_snapshot(Box::new(snapshot())); - state.set_pane_surface(surface()); - state.compose(106, 20).expect("composed frame"); - let pane = state.hits.panes[0].clone(); - let click = || { - RawInputEvent::Mouse(crossterm::event::MouseEvent { - kind: MouseEventKind::Down(MouseButton::Left), - column: pane.inner_rect.x + 1, - row: pane.inner_rect.y, - modifiers: KeyModifiers::empty(), - }) - }; - let release = || { - RawInputEvent::Mouse(crossterm::event::MouseEvent { - kind: MouseEventKind::Up(MouseButton::Left), - column: pane.inner_rect.x + 1, - row: pane.inner_rect.y, - modifiers: KeyModifiers::empty(), - }) - }; - - state.handle_raw_events(vec![click()]); - state.handle_raw_events(vec![release()]); - assert!(state.selection.is_none(), "plain clicks must not select"); - let second = state.handle_raw_events(vec![click()]); - let ClientShellAction::Endpoint { request, .. } = second - .actions - .iter() - .find(|action| { - matches!( - action, - ClientShellAction::Endpoint { request, .. } - if matches!(request.method, crate::api::schema::Method::PaneSelectionRead(_)) - ) - }) - .expect("word-row read") - else { - unreachable!() - }; - let word_request_id = request.id.clone(); - assert!(matches!( - &request.method, - crate::api::schema::Method::PaneSelectionRead(params) - if params.anchor == crate::api::schema::PaneTextPoint { row: 0, col: 0 } - && params.cursor == crate::api::schema::PaneTextPoint { row: 0, col: 3 } - )); - +fn client_double_click_selects_word_and_copies_only_after_release() { + for (copy_on_select, release_before_response) in [(false, true), (true, false)] { + let mut state = word_drag_state(copy_on_select); + let initial = start_word_drag(&mut state); + let release = MouseEventKind::Up(MouseButton::Left); if release_before_response { - let released = state.handle_raw_events(vec![release()]); - assert!(released.actions.is_empty()); + assert!(word_drag_mouse(&mut state, release, 0, 8) + .actions + .is_empty()); } - let (repaint, actions) = state.handle_endpoint_result( - "boot-1", - &word_request_id, - Ok(crate::api::schema::ResponseResult::PaneSelection { - pane_id: "pane_1".into(), - text: "LIVE".into(), - }), - ); - assert!(repaint); - assert!(state - .selection - .as_ref() - .is_some_and(crate::selection::Selection::is_finalized)); - let deadline = state.selection_highlight_clear_deadline; + let mut actions = word_row_reply(&mut state, &initial, "alpha bravo charlie"); if !release_before_response { - let released = state.handle_raw_events(vec![release()]); - assert!(released.actions.is_empty(), "release must not copy twice"); + assert!(actions.is_empty(), "holding the second press must not copy"); + assert!(state.selection.as_ref().unwrap().is_in_progress()); + state.tick_copy_feedback(std::time::Instant::now() + std::time::Duration::from_secs(1)); + assert!(state.selection.as_ref().unwrap().is_visible()); + assert!(state.copy_feedback.is_none()); + actions = word_drag_mouse(&mut state, release, 0, 8).actions; } - assert!( - state.selection.as_ref().is_some_and(crate::selection::Selection::is_finalized), - "mouse release must retain the finalized word selection (copy_on_select={copy_on_select})" - ); + assert!(state.selection.as_ref().unwrap().is_finalized()); assert_eq!( state.selection.as_ref().unwrap().ordered_cells(), - ((0, 0), (0, 3)) + ((0, 6), (0, 10)) ); - assert_eq!(state.selection_highlight_clear_deadline, deadline); - if !copy_on_select { + assert!( + word_drag_mouse(&mut state, release, 0, 8) + .actions + .is_empty(), + "copy only once" + ); + if copy_on_select { + assert!( + matches!(&actions[..], [ClientShellAction::Endpoint { request, .. }] + if matches!(&request.method, crate::api::schema::Method::PaneSelectionRead(params) + if params.anchor.col == 6 && params.cursor.col == 10)) + ); + let copied = word_row_reply(&mut state, &word_read_id(&actions), "bravo"); + assert!( + matches!(&copied[..], [ClientShellAction::ClipboardWrite(bytes)] if bytes == b"bravo") + ); + assert!(state.tick_copy_feedback(state.selection_highlight_clear_deadline.unwrap())); + assert!(state.selection.is_none()); + } else { assert!(actions.is_empty(), "manual selection must not auto-copy"); - assert!(state.selection_highlight_clear_deadline.is_none()); state.tick_copy_feedback(std::time::Instant::now() + std::time::Duration::from_secs(1)); assert!( state.selection.is_some(), "manual selection must not expire" ); - continue; } - let [ClientShellAction::Endpoint { request, .. }] = &actions[..] else { - panic!("auto-copy should read the selected word"); - }; - let copy_request_id = request.id.clone(); - assert!(matches!( - &request.method, - crate::api::schema::Method::PaneSelectionRead(params) - if params.anchor.col == 0 && params.cursor.col == 3 - )); - let (_, actions) = state.handle_endpoint_result( + } +} + +fn word_drag_state(copy_on_select: bool) -> ClientShellState { + let mut config = Config::default(); + config.ui.copy_on_select = copy_on_select; + let mut state = ClientShellState::new(ClientShellConfig::from_config(&config)); + state.set_snapshot(Box::new(snapshot())); + let mut pane_surface = surface(); + let buffer = Buffer::with_lines([ + "alpha bravo charlie", + "delta echo foxtrot ", + "golf hotel india ", + ]); + pane_surface.frame = FrameData::from_ratatui_buffer_with_hyperlinks(&buffer, None, &[]); + pane_surface.panes[0].rect.width = 19; + pane_surface.panes[0].rect.height = 3; + pane_surface.panes[0].inner_rect = pane_surface.panes[0].rect; + state.set_pane_surface(pane_surface); + state.compose(106, 20).expect("composed frame"); + state +} + +fn word_drag_mouse( + state: &mut ClientShellState, + kind: MouseEventKind, + row: u16, + col: u16, +) -> ClientShellInput { + let pane = state.hits.panes[0].clone(); + state.handle_raw_events(vec![RawInputEvent::Mouse(crossterm::event::MouseEvent { + kind, + column: pane.inner_rect.x + col, + row: pane.inner_rect.y + row, + modifiers: KeyModifiers::empty(), + })]) +} + +fn word_read_id(actions: &[ClientShellAction]) -> String { + actions + .iter() + .find_map(|action| match action { + ClientShellAction::Endpoint { request, .. } + if matches!( + request.method, + crate::api::schema::Method::PaneSelectionRead(_) + ) => + { + Some(request.id.clone()) + } + _ => None, + }) + .expect("selection read") +} + +fn word_row_reply(state: &mut ClientShellState, id: &str, text: &str) -> Vec { + state + .handle_endpoint_result( "boot-1", - ©_request_id, + id, Ok(crate::api::schema::ResponseResult::PaneSelection { pane_id: "pane_1".into(), - text: "LIVE".into(), + text: text.into(), }), + ) + .1 +} + +fn start_word_drag(state: &mut ClientShellState) -> String { + word_drag_mouse(state, MouseEventKind::Down(MouseButton::Left), 0, 8); + word_drag_mouse(state, MouseEventKind::Up(MouseButton::Left), 0, 8); + assert!(state.selection.is_none(), "plain clicks must not select"); + let second = word_drag_mouse(state, MouseEventKind::Down(MouseButton::Left), 0, 8); + assert!(second.actions.iter().any(|action| matches!(action, ClientShellAction::Endpoint { request, .. } + if matches!(&request.method, crate::api::schema::Method::PaneSelectionRead(params) + if params.anchor.col == 0 && params.cursor.col == state.hits.panes[0].inner_rect.width - 1)))); + word_read_id(&second.actions) +} + +#[test] +fn double_click_drag_selects_whole_words_in_both_directions() { + let mut state = word_drag_state(false); + let initial = start_word_drag(&mut state); + word_row_reply(&mut state, &initial, "alpha bravo charlie"); + for (col, expected) in [ + (14, ((0, 6), (0, 18))), + (2, ((0, 0), (0, 10))), + (8, ((0, 6), (0, 10))), + (11, ((0, 6), (0, 11))), + (16, ((0, 6), (0, 18))), + ] { + let motion = word_drag_mouse(&mut state, MouseEventKind::Drag(MouseButton::Left), 0, col); + assert!( + motion.actions.is_empty(), + "reuse the row while dragging within it" + ); + assert_eq!(state.selection.as_ref().unwrap().ordered_cells(), expected); + } + assert!( + word_drag_mouse(&mut state, MouseEventKind::Up(MouseButton::Left), 0, 16) + .actions + .is_empty() + ); + assert!(state.selection.as_ref().unwrap().is_finalized()); +} + +#[test] +fn double_click_drag_waits_for_latest_row_before_copying() { + for release_before_anchor in [false, true] { + let mut state = word_drag_state(true); + let initial = start_word_drag(&mut state); + if !release_before_anchor { + assert!(word_row_reply(&mut state, &initial, "alpha bravo charlie").is_empty()); + } + let first_motion = + word_drag_mouse(&mut state, MouseEventKind::Drag(MouseButton::Left), 1, 8); + for col in [1, 3, 7] { + assert!( + word_drag_mouse(&mut state, MouseEventKind::Drag(MouseButton::Left), 2, col) + .actions + .is_empty() + ); + } + assert!( + word_drag_mouse(&mut state, MouseEventKind::Up(MouseButton::Left), 2, 7) + .actions + .is_empty() + ); + let final_read = if release_before_anchor { + word_row_reply(&mut state, &initial, "alpha bravo charlie") + } else { + word_row_reply( + &mut state, + &word_read_id(&first_motion.actions), + "delta echo foxtrot", + ) + }; + assert!( + matches!(&final_read[..], [ClientShellAction::Endpoint { request, .. }] + if matches!(&request.method, crate::api::schema::Method::PaneSelectionRead(params) + if params.anchor.row == 2 && params.cursor.row == 2)) + ); + let copy = word_row_reply(&mut state, &word_read_id(&final_read), "golf hotel india"); + assert!( + matches!(©[..], [ClientShellAction::Endpoint { request, .. }] + if matches!(&request.method, crate::api::schema::Method::PaneSelectionRead(params) + if params.anchor == crate::api::schema::PaneTextPoint { row: 0, col: 6 } + && params.cursor == crate::api::schema::PaneTextPoint { row: 2, col: 9 })) + ); + let copied = word_row_reply( + &mut state, + &word_read_id(©), + "bravo charlie\ndelta echo foxtrot\ngolf hotel", + ); + assert!( + matches!(&copied[..], [ClientShellAction::ClipboardWrite(bytes)] + if bytes == b"bravo charlie\ndelta echo foxtrot\ngolf hotel") + ); + } +} + +#[test] +fn double_click_drag_ignores_row_reply_after_typing_or_new_click() { + for typing in [false, true] { + let mut state = word_drag_state(false); + let initial = start_word_drag(&mut state); + word_row_reply(&mut state, &initial, "alpha bravo charlie"); + let drag = word_drag_mouse(&mut state, MouseEventKind::Drag(MouseButton::Left), 1, 8); + let row_id = word_read_id(&drag.actions); + if typing { + state.handle_input_bytes(b"x"); + } else { + word_drag_mouse(&mut state, MouseEventKind::Down(MouseButton::Left), 0, 0); + word_drag_mouse(&mut state, MouseEventKind::Up(MouseButton::Left), 0, 0); + } + assert!(word_row_reply(&mut state, &row_id, "delta echo foxtrot").is_empty()); + assert!(state.selection.is_none()); + } +} + +#[test] +fn double_click_drag_survives_focus_lag_after_anchor_reply() { + let mut state = word_drag_state(true); + let initial = start_word_drag(&mut state); + word_row_reply(&mut state, &initial, "alpha bravo charlie"); + let mut lagging = snapshot(); + lagging.focused_pane_id = None; + lagging.panes[0].focused = false; + state.set_snapshot(Box::new(lagging)); + assert!(state.selection.is_some()); + word_drag_mouse(&mut state, MouseEventKind::Drag(MouseButton::Left), 0, 14); + assert_eq!( + state.selection.as_ref().unwrap().ordered_cells(), + ((0, 6), (0, 18)) + ); + let released = word_drag_mouse(&mut state, MouseEventKind::Up(MouseButton::Left), 0, 14); + assert_eq!(released.actions.len(), 1); +} + +#[test] +fn double_click_drag_invalidates_cached_boundaries_outside_selected_cells() { + for copy_on_select in [false, true] { + let mut state = word_drag_state(copy_on_select); + let initial = start_word_drag(&mut state); + word_row_reply(&mut state, &initial, "alpha bravo charlie"); + let mut changed = state.pane_surface.as_ref().unwrap().clone(); + changed.surface_revision += 1; + changed.panes[0].content_revision += 2; + changed.frame.cells[14].symbol = " ".into(); + state.set_pane_surface(changed); + assert!( + state.selection.is_none(), + "unchanged selected cells do not validate cached boundaries outside the selection" + ); + assert!( + word_drag_mouse(&mut state, MouseEventKind::Drag(MouseButton::Left), 0, 14) + .actions + .is_empty() + ); + assert!( + word_drag_mouse(&mut state, MouseEventKind::Up(MouseButton::Left), 0, 14) + .actions + .is_empty() ); - assert!(matches!( - &actions[..], - [ClientShellAction::ClipboardWrite(bytes)] if bytes == b"LIVE" - )); - assert!(state.tick_copy_feedback(deadline.expect("auto-copy highlight deadline"))); assert!(state.selection.is_none()); } } +#[test] +fn double_click_release_ignores_reply_after_focus_or_content_changes() { + for focus_changed in [false, true] { + let mut state = word_drag_state(true); + let initial = start_word_drag(&mut state); + word_drag_mouse(&mut state, MouseEventKind::Up(MouseButton::Left), 0, 8); + if focus_changed { + let mut lagging = snapshot(); + lagging.focused_pane_id = None; + lagging.panes[0].focused = false; + state.set_snapshot(Box::new(lagging)); + let mut unfocused = snapshot(); + unfocused.focused_pane_id = Some("pane_2".into()); + unfocused.panes[0].focused = false; + let mut other = unfocused.panes[0].clone(); + other.pane_id = "pane_2".into(); + other.focused = true; + unfocused.panes.push(other); + state.set_snapshot(Box::new(unfocused)); + } else { + let mut changed = state.pane_surface.as_ref().unwrap().clone(); + changed.surface_revision += 1; + changed.panes[0].content_revision += 2; + state.set_pane_surface(changed); + } + assert!( + word_row_reply(&mut state, &initial, "alpha bravo charlie").is_empty(), + "a stale released gesture must not copy" + ); + assert!(state.selection.is_none()); + } +} + +#[test] +fn double_click_drag_resize_cancels_pending_word_lookup() { + for anchor_ready in [false, true] { + let mut state = word_drag_state(true); + let initial = start_word_drag(&mut state); + let pending = if anchor_ready { + word_row_reply(&mut state, &initial, "alpha bravo charlie"); + let motion = word_drag_mouse(&mut state, MouseEventKind::Drag(MouseButton::Left), 1, 8); + word_read_id(&motion.actions) + } else { + initial + }; + word_drag_mouse(&mut state, MouseEventKind::Up(MouseButton::Left), 1, 8); + let mut resized = state.pane_surface.as_ref().unwrap().clone(); + resized.surface_revision += 1; + resized.panes[0].rect.width += 5; + resized.panes[0].inner_rect.width += 5; + state.set_pane_surface(resized); + assert!(word_row_reply(&mut state, &pending, "alpha bravo charlie extra").is_empty()); + assert!( + state.selection.is_none(), + "a late reply must not restore a resized selection" + ); + assert!(state.selection_autoscroll.is_none()); + } +} + +#[test] +fn double_click_drag_autoscroll_keeps_absolute_word_anchor() { + let mut state = word_drag_state(false); + state.hits.panes[0].scroll = Some(crate::pane::ScrollMetrics { + max_offset_from_bottom: 10, + offset_from_bottom: 5, + viewport_rows: 3, + }); + let initial = start_word_drag(&mut state); + word_row_reply(&mut state, &initial, "alpha bravo charlie"); + word_drag_mouse(&mut state, MouseEventKind::Drag(MouseButton::Left), 0, 14); + let tick = state.tick_selection_autoscroll(state.selection_autoscroll_deadline.unwrap()); + word_row_reply( + &mut state, + &word_read_id(&tick.actions), + "delta echo foxtrot", + ); + assert_eq!( + state.selection.as_ref().unwrap().ordered_cells(), + ((4, 11), (5, 10)) + ); + word_drag_mouse(&mut state, MouseEventKind::Up(MouseButton::Left), 0, 14); + assert!(state.selection.as_ref().unwrap().is_finalized()); + assert!(state.selection_autoscroll.is_none()); +} + #[test] fn pane_content_updates_preserve_active_selection_only_when_selected_cells_stay_stable() { let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default())); diff --git a/src/client/shell/word_selection.rs b/src/client/shell/word_selection.rs new file mode 100644 index 0000000000..42a03fc26c --- /dev/null +++ b/src/client/shell/word_selection.rs @@ -0,0 +1,216 @@ +use super::*; + +/// Held second press. Keep only one row read in flight and use the latest +/// pointer position when it returns, so remote latency cannot queue up motion. +#[derive(Debug)] +pub(super) struct ClientWordSelection { + pub(super) pane_id: String, + pub(super) focus_confirmed: bool, + anchor: (u32, u16), + anchor_bounds: Option<(u16, u16)>, + cursor: (u32, u16), + end_col: u16, + content_revision: Option, + cached_row: Option<(u32, String)>, + pending_row: Option, + pub(super) dragged: bool, + pub(super) released: bool, +} + +impl ClientShellState { + pub(super) fn request_word_selection( + &mut self, + hit: &PaneHit, + viewport_row: u16, + col: u16, + outcome: &mut ClientShellInput, + ) { + let row = crate::selection::absolute_row_for_viewport(viewport_row, hit.scroll); + self.word_selection_generation = self.word_selection_generation.saturating_add(1); + self.word_selection_gesture = Some(ClientWordSelection { + pane_id: hit.pane_id.clone(), + focus_confirmed: self + .snapshot + .as_deref() + .and_then(|snapshot| snapshot.focused_pane_id.as_deref()) + == Some(hit.pane_id.as_str()), + anchor: (row, col), + anchor_bounds: None, + cursor: (row, col), + end_col: hit.inner_rect.width.saturating_sub(1), + content_revision: self.pane_surface.as_ref().and_then(|surface| { + surface + .panes + .iter() + .find(|pane| pane.pane_id == hit.pane_id) + .map(|pane| pane.content_revision) + }), + cached_row: None, + pending_row: None, + dragged: false, + released: false, + }); + self.request_word_selection_row(row, outcome); + } + + fn cancel_word_selection(&mut self) { + self.word_selection_gesture = None; + self.selection = None; + self.stop_selection_autoscroll(); + } + + fn request_word_selection_row(&mut self, row: u32, outcome: &mut ClientShellInput) { + let Some(gesture) = self.word_selection_gesture.as_mut() else { + return; + }; + if gesture.pending_row.is_some() { + return; + } + gesture.pending_row = Some(row); + let pane_id = gesture.pane_id.clone(); + let params = crate::api::schema::PaneSelectionReadParams { + pane_id: pane_id.clone(), + anchor: crate::api::schema::PaneTextPoint { row, col: 0 }, + cursor: crate::api::schema::PaneTextPoint { + row, + col: gesture.end_col, + }, + content_revision: gesture.content_revision, + }; + if !self.push_endpoint_method_with_kind( + crate::api::schema::Method::PaneSelectionRead(params), + PendingEndpointKind::WordSelection { + pane_id, + absolute_row: row, + generation: self.word_selection_generation, + }, + outcome, + ) { + self.cancel_word_selection(); + } + } + + pub(super) fn drag_word_selection( + &mut self, + cursor: (u32, u16), + outcome: &mut ClientShellInput, + ) { + let Some(gesture) = self.word_selection_gesture.as_mut() else { + return; + }; + if gesture.released || gesture.cursor == cursor { + return; + } + gesture.cursor = cursor; + gesture.dragged = true; + self.update_word_selection(outcome); + } + + pub(super) fn finish_word_selection(&mut self, outcome: &mut ClientShellInput) { + self.stop_selection_autoscroll(); + if let Some(gesture) = self.word_selection_gesture.as_mut() { + gesture.released = true; + } + // A pending row reply will finish the selection if its bounds are not ready yet. + self.update_word_selection(outcome); + } + + fn update_word_selection(&mut self, outcome: &mut ClientShellInput) { + let Some(gesture) = self.word_selection_gesture.as_ref() else { + return; + }; + let Some((anchor_start, anchor_end)) = gesture.anchor_bounds else { + return; + }; + let Some((_, text)) = gesture + .cached_row + .as_ref() + .filter(|(row, _)| *row == gesture.cursor.0) + else { + self.request_word_selection_row(gesture.cursor.0, outcome); + return; + }; + let (start_col, end_col) = + crate::app::actions::word_bounds_at_column(text, gesture.cursor.1) + .unwrap_or((gesture.cursor.1, gesture.cursor.1)); + let start = (gesture.anchor.0, anchor_start).min((gesture.cursor.0, start_col)); + let end = (gesture.anchor.0, anchor_end).max((gesture.cursor.0, end_col)); + self.selection = Some(crate::selection::Selection::absolute_range( + gesture.pane_id.clone(), + start, + end, + )); + if gesture.released { + let dragged = gesture.dragged; + if let Some(selection) = self.selection.as_mut() { + selection.finish(); + } + self.word_selection_gesture = None; + if self.config.copy_on_select { + self.request_selection_copy(outcome, false); + if dragged { + self.selection = None; + } else { + self.selection_highlight_clear_deadline = + Some(std::time::Instant::now() + std::time::Duration::from_millis(500)); + } + } + } + outcome.repaint = true; + } + + pub(super) fn complete_word_selection_row( + &mut self, + pane_id: String, + absolute_row: u32, + generation: u64, + result: Result, + ) -> (bool, Vec) { + if self.word_selection_generation != generation + || self.word_selection_gesture.as_ref().is_none_or(|gesture| { + gesture.pane_id != pane_id || gesture.pending_row != Some(absolute_row) + }) + { + return (false, Vec::new()); + } + if self + .snapshot + .as_deref() + .is_none_or(|snapshot| !snapshot.panes.iter().any(|pane| pane.pane_id == pane_id)) + { + self.cancel_word_selection(); + return (true, Vec::new()); + } + let text = match result { + Ok(crate::api::schema::ResponseResult::PaneSelection { + pane_id: returned_pane_id, + text, + }) if returned_pane_id == pane_id => text, + other => { + if matches!(other, Ok(value) if !matches!(value, crate::api::schema::ResponseResult::PaneSelection { .. })) + { + self.endpoint_error = + Some("endpoint returned an unexpected word-selection result".to_owned()); + } + self.cancel_word_selection(); + return (true, Vec::new()); + } + }; + let Some(gesture) = self.word_selection_gesture.as_mut() else { + return (false, Vec::new()); + }; + gesture.pending_row = None; + if gesture.anchor_bounds.is_none() { + gesture.anchor_bounds = + crate::app::actions::word_bounds_at_column(&text, gesture.anchor.1); + if gesture.anchor_bounds.is_none() { + self.cancel_word_selection(); + return (true, Vec::new()); + } + } + gesture.cached_row = Some((absolute_row, text)); + let mut outcome = ClientShellInput::default(); + self.update_word_selection(&mut outcome); + (outcome.repaint, outcome.actions) + } +} From 38d002de59331b27aee2f5838639ea27a04878f8 Mon Sep 17 00:00:00 2001 From: Ogulcan Celik Date: Fri, 11 Sep 2026 23:10:17 +0300 Subject: [PATCH 02/16] docs: enable nesting in throwaway repro setup --- .agents/skills/herdr-throwaway-repro/SKILL.md | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/.agents/skills/herdr-throwaway-repro/SKILL.md b/.agents/skills/herdr-throwaway-repro/SKILL.md index 15a9be2fc8..0f1cd838db 100644 --- a/.agents/skills/herdr-throwaway-repro/SKILL.md +++ b/.agents/skills/herdr-throwaway-repro/SKILL.md @@ -50,6 +50,33 @@ Use `/var/tmp` or a dedicated reproduction directory as the new pane's cwd. Save Choose a short unique name such as `repro--`. +Before launching, explicitly allow nesting in the configuration the disposable +client will actually load. `HERDR_ENV=1` is inherited from the outer pane, so the +launch is otherwise rejected unless `[experimental].allow_nested` is enabled. +An isolated `XDG_CONFIG_HOME` does not inherit this setting from the user's global +config, even when nesting is enabled there. + +Create a test-only config under the reproduction directory using the file-writing +tool. For a default-config reproduction, its contents can be: + +```toml +[experimental] +allow_nested = true +``` + +If reproducing with the user's configuration, copy that configuration into the +test directory and enable `allow_nested` in its existing `[experimental]` table +(or add the table if absent). Do not create duplicate tables or keys. Never edit +the user's global configuration to permit a reproduction, and do not unset +`HERDR_ENV` to bypass the nesting check. + +Pass the absolute test config path as `HERDR_CONFIG_PATH` when launching below. +Use the same override for config validation and any commands that must load the +test config. This override selects a config file; it does not isolate the saved +machine catalog or other global state. Use test-only `XDG_CONFIG_HOME` and +`XDG_STATE_HOME` as well when the reproduction changes saved machines, and retain +those overrides on every command addressing that test environment. + Run the named session inside the new outer pane. Clear inherited session selection, socket overrides, and caller IDs so the nested runtime cannot accidentally address the parent session: ```bash @@ -60,11 +87,17 @@ env \ -u HERDR_WORKSPACE_ID \ -u HERDR_TAB_ID \ -u HERDR_PANE_ID \ + HERDR_CONFIG_PATH= \ herdr --session ``` Add reproduction-specific environment variables to this launch command when needed. Environment variables that configure the server must be present before the named server starts. +Validate the test config with `herdr config check` using the same config override +before launch. After launch, read the outer pane to catch startup errors such as +`nested herdr is disabled by default`; do not assume the launch succeeded merely +because `pane run` returned successfully. + Do not continue until the named session's API is ready. Confirm readiness by addressing that session from the parent and listing its panes. ## Address only the disposable session From fb5525fc2234a246dfdfc9e0d7ed73c824cbd8a6 Mon Sep 17 00:00:00 2001 From: Mark Jaquith Date: Fri, 11 Sep 2026 16:36:18 -0400 Subject: [PATCH 03/16] fix: opencode v2 lifecycle reporting (#3757) * fix: support opencode v2 lifecycle reporting * fix: repair Japanese docs * fix: ignore payload-less OpenCode events * fix: harden opencode v2 integration install and reporting Follow-ups on top of the V2 lifecycle support: - create `cli.json` when OpenCode has no V1 TUI preferences (`tui.json` or `kv.json`) to migrate, instead of only registering into an existing file - settle stalled socket attempts with a plain connect timer, and resend the latest lifecycle state after a failed delivery so the pane cannot get stuck - resolve the OpenCode state directory from `XDG_STATE_HOME` - remove the managed `herdr-opencode` directory on uninstall - document failed executions reporting `blocked`, keep the integration test environment independent of an inherited `XDG_STATE_HOME`, and simplify `reconcileBlockers` to reassign its map --------- Co-authored-by: Jonathan Liebig --- .../website/src/content/docs/integrations.mdx | 6 +- .../src/content/docs/ja/integrations.mdx | 6 +- .../src/content/docs/zh-cn/integrations.mdx | 6 +- src/integration/actions.rs | 11 +- .../assets/opencode/herdr-agent-state.js | 11 +- .../assets/opencode/herdr-agent-state.test.ts | 10 + .../assets/opencode/herdr-tui-session.js | 243 +++++++++++++++++- .../assets/opencode/herdr-tui-session.test.ts | 224 +++++++++++++++- src/integration/assets/opencode/tui.js | 5 + src/integration/env.rs | 39 +++ src/integration/mod.rs | 5 +- src/integration/opencode_config.rs | 154 +++++++++-- src/integration/registry.rs | 12 + src/integration/targets.rs | 23 +- src/integration/tests.rs | 95 +++++++ src/integration/types.rs | 1 + 16 files changed, 804 insertions(+), 47 deletions(-) create mode 100644 src/integration/assets/opencode/tui.js diff --git a/docs/next/website/src/content/docs/integrations.mdx b/docs/next/website/src/content/docs/integrations.mdx index 1f64f2b91e..4cbd0a631b 100644 --- a/docs/next/website/src/content/docs/integrations.mdx +++ b/docs/next/website/src/content/docs/integrations.mdx @@ -215,7 +215,11 @@ Install the OpenCode plugin: herdr integration install opencode ``` -Herdr writes the plugin to `~/.config/opencode/plugins/herdr-agent-state.js`. The OpenCode config directory must already exist. Uninstall removes only that plugin file. +The integration supports OpenCode V1 `1.18.29` or later and OpenCode V2 (tested with beta `19242`). The OpenCode config directory must already exist. Herdr installs the server entrypoint at `~/.config/opencode/plugins/herdr-agent-state.js`, the shared TUI plugin at `herdr-tui-session.js`, and a V2 TUI entrypoint at `herdr-opencode/tui.js` in that config directory. + +Install registers the V1 TUI plugin in `tui.jsonc` and the V2 TUI plugin in `cli.json`, preserving other preferences and plugins. If OpenCode still has V1 TUI preferences to import (`tui.json` or `kv.json`), Herdr defers registration so the first-start migration can run; start `opencode2` once and reinstall the integration afterward. Otherwise Herdr creates `cli.json` with the plugin registration. Restart the OpenCode TUI after installation. Uninstall removes the managed plugin files and their configuration entries. + +V2 lifecycle reporting runs in the pane-local TUI, which associates events with its selected root session even when multiple panes share one OpenCode server. Completion and interruption clear the working state; pending permission requests, pending forms, and failed executions keep the pane blocked. V2 Mini and headless clients do not run the TUI plugin and therefore do not provide this lifecycle reporting. The plugin reports lifecycle state and session identity while OpenCode runs inside a Herdr pane. After OpenCode emits a session-bearing event, Herdr can use the reported session id to resume the pane with `opencode --session `. Native screen manifest detection remains available when the plugin is not installed. diff --git a/docs/next/website/src/content/docs/ja/integrations.mdx b/docs/next/website/src/content/docs/ja/integrations.mdx index 44d1b3e750..124010e177 100644 --- a/docs/next/website/src/content/docs/ja/integrations.mdx +++ b/docs/next/website/src/content/docs/ja/integrations.mdx @@ -217,7 +217,11 @@ OpenCode プラグインをインストールします: herdr integration install opencode ``` -Herdr はプラグインを `~/.config/opencode/plugins/herdr-agent-state.js` に書き込みます。OpenCode の設定ディレクトリはあらかじめ存在している必要があります。アンインストールはそのプラグインファイルだけを削除します。 +この連携は OpenCode V1 `1.18.29` 以降と OpenCode V2(beta `19242` で検証済み)に対応します。OpenCode の設定ディレクトリはあらかじめ存在している必要があります。Herdr はサーバー側のエントリポイントを `~/.config/opencode/plugins/herdr-agent-state.js` に、共通 TUI プラグインを同じ設定ディレクトリの `herdr-tui-session.js` に、V2 TUI エントリポイントを `herdr-opencode/tui.js` にインストールします。 + +インストール時、V1 TUI プラグインを `tui.jsonc` に、V2 TUI プラグインを `cli.json` に登録し、他の設定やプラグインは保持します。OpenCode に移行すべき V1 TUI 設定(`tui.json` または `kv.json`)が残っている場合、Herdr は初回起動時の移行を妨げないよう登録を見送ります。その場合は一度 `opencode2` を起動してから連携を再インストールしてください。それ以外の場合は Herdr が `cli.json` を作成してプラグインを登録します。インストール後は OpenCode TUI を再起動してください。アンインストールは管理対象のプラグインファイルと設定項目を削除します。 + +V2 のライフサイクル報告はペイン内の TUI で実行され、複数のペインが同じ OpenCode サーバーを共有する場合も、選択されたルートセッションにイベントを対応付けます。完了時または中断時に working 状態を解除し、未処理の権限要求やフォーム、実行失敗は blocked 状態を維持します。V2 Mini とヘッドレスクライアントは TUI プラグインを実行しないため、このライフサイクル報告は利用できません。 このプラグインは、OpenCode が Herdr のペイン内で動いている間、ライフサイクル状態とセッション識別を報告します。OpenCode がセッション情報を含むイベントを発行した後、Herdr は報告されたセッション id を使って `opencode --session ` でペインを resume できます。プラグインがインストールされていないときは、スクリーンマニフェスト検出が引き続き利用できます。 diff --git a/docs/next/website/src/content/docs/zh-cn/integrations.mdx b/docs/next/website/src/content/docs/zh-cn/integrations.mdx index ea0fc3d3b6..e5535808a1 100644 --- a/docs/next/website/src/content/docs/zh-cn/integrations.mdx +++ b/docs/next/website/src/content/docs/zh-cn/integrations.mdx @@ -217,7 +217,11 @@ Herdr 的 Droid 钩子使用 `~/.factory`。Factory 配置目录必须已经存 herdr integration install opencode ``` -Herdr 把插件写入 `~/.config/opencode/plugins/herdr-agent-state.js`。OpenCode 配置目录必须已经存在。卸载只删除那个插件文件。 +该集成支持 OpenCode V1 `1.18.29` 及更高版本,以及 OpenCode V2(已验证 beta `19242`)。OpenCode 配置目录必须已经存在。Herdr 将服务端入口安装到 `~/.config/opencode/plugins/herdr-agent-state.js`,并在该配置目录安装共享 TUI 插件 `herdr-tui-session.js` 和 V2 TUI 入口 `herdr-opencode/tui.js`。 + +安装会在 `tui.jsonc` 中注册 V1 TUI 插件,并在 `cli.json` 中注册 V2 TUI 插件,同时保留其他设置和插件。如果 OpenCode 仍有 V1 TUI 偏好需要迁移(`tui.json` 或 `kv.json`),Herdr 会推迟注册,以便首次启动时执行迁移;此时请先启动一次 `opencode2`,然后重新安装集成。否则 Herdr 会创建 `cli.json` 并注册插件。安装后请重启 OpenCode TUI。卸载会删除受管理的插件文件及其配置条目。 + +V2 生命周期上报在窗格本地的 TUI 中运行。即使多个窗格共享同一个 OpenCode 服务端,事件也只归属于该 TUI 选中的根会话。完成和中断会清除工作状态;待处理的权限请求、表单以及执行失败会使窗格保持阻塞。V2 Mini 和无界面客户端不运行 TUI 插件,因此不提供此生命周期上报。 该插件在 OpenCode 运行于 Herdr 窗格内时上报生命周期状态和会话身份。在 OpenCode 发出携带会话信息的事件后,Herdr 可以用上报的会话 id 通过 `opencode --session ` 恢复该窗格。插件未安装时,屏幕清单检测仍然可用。 diff --git a/src/integration/actions.rs b/src/integration/actions.rs index 24d1f9e37c..e8b3ab6375 100644 --- a/src/integration/actions.rs +++ b/src/integration/actions.rs @@ -144,7 +144,7 @@ fn install_target_inner(target: crate::api::schema::IntegrationTarget) -> io::Re } crate::api::schema::IntegrationTarget::Opencode => { let installed = install_opencode()?; - vec![ + let mut messages = vec![ format!( "installed opencode integration plugin to {}", installed.plugin_path.display() @@ -157,7 +157,14 @@ fn install_target_inner(target: crate::api::schema::IntegrationTarget) -> io::Re "ensured opencode tui plugin config at {}", installed.tui_config_path.display() ), - ] + ]; + if installed.cli_config_path.is_none() { + messages.push( + "to enable OpenCode V2, start opencode2 once, then reinstall this integration" + .to_string(), + ); + } + messages } crate::api::schema::IntegrationTarget::Kilo => { let installed = install_kilo()?; diff --git a/src/integration/assets/opencode/herdr-agent-state.js b/src/integration/assets/opencode/herdr-agent-state.js index 45032335c1..73a7053400 100644 --- a/src/integration/assets/opencode/herdr-agent-state.js +++ b/src/integration/assets/opencode/herdr-agent-state.js @@ -2,7 +2,7 @@ // managed by herdr; reinstalling or updating the integration overwrites this file. // add custom hooks/plugins beside this file instead of editing it. // HERDR_INTEGRATION_ID=opencode -// HERDR_INTEGRATION_VERSION=11 +// HERDR_INTEGRATION_VERSION=12 import net from "node:net"; @@ -199,3 +199,12 @@ export const HerdrAgentStatePlugin = async () => { }, }; }; + +// V1 (1.18.29+) calls server(). V2 calls setup() instead. Its shared server +// cannot attribute sessions using its process environment: the pane-local TUI +// owns both selection and lifecycle reporting there, including remote servers. +export default { + id: "herdr.opencode", + server: HerdrAgentStatePlugin, + setup() {}, +}; diff --git a/src/integration/assets/opencode/herdr-agent-state.test.ts b/src/integration/assets/opencode/herdr-agent-state.test.ts index a3eea84843..38c1f05c89 100644 --- a/src/integration/assets/opencode/herdr-agent-state.test.ts +++ b/src/integration/assets/opencode/herdr-agent-state.test.ts @@ -227,6 +227,16 @@ function requestMethod(request: unknown): unknown { return isRecord(request) ? request.method : undefined; } +test("dual server entrypoint keeps V1 hooks and never reports from the V2 shared server", async () => { + const module = await import(`./herdr-agent-state.js?test=${++importCounter}`); + expect(module.default.server).toBe(module.HerdrAgentStatePlugin); + expect(await module.default.setup({})).toBeUndefined(); + expect(requests).toHaveLength(0); + const hooks = await module.default.server(); + await hooks["chat.message"]({ sessionID: "v1-root" }); + expect(requests.map(requestState)).toEqual(["working"]); +}); + function requestState(request: unknown): unknown { return requestParam(request, "state"); } diff --git a/src/integration/assets/opencode/herdr-tui-session.js b/src/integration/assets/opencode/herdr-tui-session.js index f4f08d57a8..4e5631acb4 100644 --- a/src/integration/assets/opencode/herdr-tui-session.js +++ b/src/integration/assets/opencode/herdr-tui-session.js @@ -1,7 +1,7 @@ // installed by herdr // managed by herdr; reinstalling or updating the integration overwrites this file. // HERDR_INTEGRATION_ID=opencode-tui -// HERDR_INTEGRATION_VERSION=11 +// HERDR_INTEGRATION_VERSION=12 import net from "node:net"; @@ -10,11 +10,11 @@ const AGENT = "opencode"; const ROUTE_POLL_INTERVAL_MS = 100; const SELECTION_RETRY_DELAYS_MS = [100, 400, 1_000]; -function requestOnce(sessionID) { +function requestOnce(sessionID, state, seq, isCurrent = () => true) { const paneId = process.env.HERDR_PANE_ID; const socketPath = process.env.HERDR_SOCKET_PATH; if (!paneId || !socketPath) { - return Promise.resolve(); + return Promise.resolve(true); } const socketEndpoint = @@ -23,35 +23,50 @@ function requestOnce(sessionID) { id: `${SOURCE}:tui:${Date.now()}:${Math.floor(Math.random() * 1_000_000) .toString() .padStart(6, "0")}`, - method: "pane.report_agent_session", + method: state === undefined ? "pane.report_agent_session" : "pane.report_agent", params: { pane_id: paneId, source: SOURCE, agent: AGENT, agent_session_id: sessionID, - session_start_source: "select", + ...(state === undefined ? { session_start_source: "select" } : { state, seq }), }, }; return new Promise((resolve) => { + let settled = false; + let timer; + const settle = (delivered) => { + if (settled) return; + settled = true; + clearTimeout(timer); + client.destroy(); + resolve(delivered); + }; const client = net.createConnection(socketEndpoint, () => { + if (!isCurrent()) { + settle(false); + return; + } client.write(`${JSON.stringify(request)}\n`); }); - const finish = () => { - client.destroy(); - resolve(); - }; - client.setTimeout(500, finish); - client.on("data", finish); - client.on("error", finish); - client.on("end", finish); - client.on("close", resolve); + // A plain timer, not socket.setTimeout, so a connection that never finishes + // connecting still settles and cannot block later reports behind the queue. + timer = setTimeout(() => settle(false), 500); + timer.unref?.(); + client.on("data", () => settle(true)); + client.on("error", () => settle(false)); + client.on("end", () => settle(false)); + client.on("close", () => settle(false)); }); } export default { id: "herdr.opencode.session-selection", + // Keep this plain object dependency-free: V1 and V2 expose different SDK + // packages, but both loaders accept their own lifecycle entry on this object. + setup, tui: async (api) => { if ( process.env.HERDR_ENV !== "1" || @@ -111,3 +126,203 @@ export default { api.lifecycle.onDispose(() => clearInterval(routePoll)); }, }; + +function setup(api) { + if (process.env.HERDR_ENV !== "1" || !process.env.HERDR_SOCKET_PATH || !process.env.HERDR_PANE_ID) return; + + let disposed = false; + let selected; + let generation = 0; + let sequence = Date.now() * 1000; + let chain = Promise.resolve(); + let retryIndex = 0; + let nextSelectionAt = 0; + let state = "idle"; + let retryTimer; + const sessions = new Map(); + let blockers = new Map(); + // Event callbacks may precede cache updates. Retain each delta until the + // cache reflects it, so late hydration cannot undo a reply or lose an ask. + const blockerChanges = new Map(); + + function root(id) { + const seen = new Set(); + while (typeof id === "string" && !seen.has(id)) { + seen.add(id); + const session = api.data.session.get(id) ?? sessions.get(id); + if (!session) return; + if (!session.parentID) return id; + id = session.parentID; + } + } + + function current() { + const route = api.ui.router.current(); + return route.type === "session" ? root(route.sessionID) : undefined; + } + + // Selection and lifecycle use one queue. Recheck attribution at dispatch, + // not just when receiving the event, and reject A -> B -> A stale work too. + function enqueue(value) { + const sessionID = selected; + const revision = generation; + const isCurrent = () => !disposed && revision === generation && !!sessionID && current() === sessionID; + chain = chain.then(async () => { + if (!isCurrent()) return; + const delivered = await requestOnce(sessionID, value, value === undefined ? undefined : ++sequence, isCurrent); + if (!delivered) scheduleStateRetry(); + }).catch(() => {}); + } + + // A dropped report must not strand the pane on a stale state once the + // selection retry schedule has run out: resend the latest state until the + // socket accepts it or the selection is no longer current. + function scheduleStateRetry() { + if (disposed || retryTimer) return; + retryTimer = setTimeout(() => { + retryTimer = undefined; + publish(); + }, 500); + retryTimer.unref?.(); + } + + function publish() { + enqueue(blockers.size ? "blocked" : state); + } + + function changeBlocker(id, kind, requestID, present) { + if (typeof requestID !== "string") return; + const key = `${kind}:${requestID}`; + blockerChanges.set(key, { id, kind, present }); + if (present) blockers.set(key, id); + else blockers.delete(key); + } + + function reconcileBlockers() { + const next = new Map(); + const hydrated = new Set(); + const members = new Set([selected, ...api.data.session.family(selected), ...blockers.values()]); + for (const member of members) { + if (root(member) !== selected) continue; + for (const kind of ["permission", "form"]) { + const items = api.data.session[kind].list(member); + if (items === undefined) { + for (const [key, owner] of blockers) { + if (owner === member && key.startsWith(`${kind}:`)) next.set(key, owner); + } + continue; + } + hydrated.add(`${kind}:${member}`); + for (const item of items) next.set(`${kind}:${item.id}`, member); + } + } + for (const [key, change] of blockerChanges) { + if (hydrated.has(`${change.kind}:${change.id}`) && next.has(key) === change.present) { + blockerChanges.delete(key); + } else if (change.present) { + next.set(key, change.id); + } else { + next.delete(key); + } + } + const changed = (blockers.size > 0) !== (next.size > 0); + blockers = next; + return changed; + } + + function syncSelection() { + if (disposed) return; + const id = current(); + if (id !== selected) { + selected = id; + generation += 1; + retryIndex = 0; + nextSelectionAt = 0; + blockers.clear(); + blockerChanges.clear(); + if (id) { + state = api.data.session.status(id) === "running" ? "working" : "idle"; + } + } + if (!id) return; + const blockersChanged = reconcileBlockers(); + if (Date.now() < nextSelectionAt) { + if (blockersChanged) publish(); + return; + } + enqueue(undefined); + publish(); + const delay = SELECTION_RETRY_DELAYS_MS[retryIndex++]; + nextSelectionAt = delay === undefined ? Number.POSITIVE_INFINITY : Date.now() + delay; + } + + function receive({ details: event }) { + if (disposed) return; + const data = event.data; + if (data == null) return; + if (event.type === "session.created") { + sessions.set(data.sessionID, { id: data.sessionID, parentID: data.parentID }); + } + if (event.type === "session.deleted") { + const affected = data.sessionID === selected || [...blockers.values()].includes(data.sessionID); + sessions.delete(data.sessionID); + // Deletion is delivered after the cache can remove the session. Use + // stored ownership rather than looking up the deleted child's ancestry. + for (const [key, owner] of blockers) if (owner === data.sessionID) blockers.delete(key); + for (const [key, change] of blockerChanges) { + if (change.id === data.sessionID) blockerChanges.delete(key); + } + syncSelection(); + if (selected && affected) publish(); + return; + } + syncSelection(); + const id = event.type === "form.created" ? data.form.sessionID : data.sessionID; + if (!selected || root(id) !== selected) return; + switch (event.type) { + case "permission.asked": + changeBlocker(id, "permission", data.id, true); + break; + case "permission.replied": + changeBlocker(id, "permission", data.requestID, false); + break; + case "form.created": + changeBlocker(id, "form", data.form.id, true); + break; + case "form.replied": + case "form.cancelled": + changeBlocker(id, "form", data.id, false); + break; + case "session.execution.started": + if (id !== selected) return; + state = "working"; + break; + case "session.execution.succeeded": + case "session.execution.interrupted": + if (id !== selected) return; + state = "idle"; + break; + case "session.execution.failed": + if (id !== selected) return; + state = "blocked"; + break; + default: + return; + } + publish(); + } + + const unsubscribe = api.data.listen(receive); + syncSelection(); + const poll = setInterval(syncSelection, ROUTE_POLL_INTERVAL_MS); + return () => { + disposed = true; + generation += 1; + clearTimeout(retryTimer); + clearInterval(poll); + unsubscribe(); + sessions.clear(); + blockers.clear(); + blockerChanges.clear(); + }; +} diff --git a/src/integration/assets/opencode/herdr-tui-session.test.ts b/src/integration/assets/opencode/herdr-tui-session.test.ts index 1bcb1f81e3..dd03152737 100644 --- a/src/integration/assets/opencode/herdr-tui-session.test.ts +++ b/src/integration/assets/opencode/herdr-tui-session.test.ts @@ -3,15 +3,25 @@ import { afterEach, beforeEach, expect, mock, test } from "bun:test"; const requests: unknown[] = []; const activeDisposers: Array<() => void> = []; const requestWaiters: Array<() => void> = []; +const stateWaiters: Array<() => void> = []; let importCounter = 0; +let holdConnections = false; +let failConnections = false; +const connections: Array<() => void> = []; mock.module("node:net", () => ({ default: { createConnection(_path: string, onConnect: () => void) { const handlers = new Map void>(); const client = { + destroyed: false, write(input: string) { - requests.push(JSON.parse(input.trim())); + if (client.destroyed) return; + const request = JSON.parse(input.trim()); + requests.push(request); + if (isRecord(request) && isRecord(request.params) && request.params.state !== undefined) { + stateWaiters.shift()?.(); + } requestWaiters.shift()?.(); queueMicrotask(() => client.emit("data")); }, @@ -19,12 +29,16 @@ mock.module("node:net", () => ({ on(event: string, handler: () => void) { handlers.set(event, handler); }, - destroy() {}, + destroy() { + client.destroyed = true; + }, emit(event: string) { handlers.get(event)?.(); }, }; - queueMicrotask(onConnect); + if (holdConnections) connections.push(onConnect); + else if (failConnections) queueMicrotask(() => client.emit("error")); + else queueMicrotask(onConnect); return client; }, }, @@ -33,6 +47,10 @@ mock.module("node:net", () => ({ beforeEach(() => { requests.length = 0; requestWaiters.length = 0; + stateWaiters.length = 0; + holdConnections = false; + failConnections = false; + connections.length = 0; process.env.HERDR_ENV = "1"; process.env.HERDR_SOCKET_PATH = "test.sock"; process.env.HERDR_PANE_ID = "test:p1"; @@ -93,6 +111,10 @@ function waitForNextRequest(): Promise { return new Promise((resolve) => requestWaiters.push(resolve)); } +function waitForStateReport(): Promise { + return new Promise((resolve) => stateWaiters.push(resolve)); +} + test("reports a root session when only the local route changes", async () => { const plugin = await loadPlugin(); const tui = fakeApi(); @@ -179,3 +201,199 @@ function requestParam(request: unknown, name: string): unknown { function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null; } + +function v2Api() { + const sessions = new Map([ + ["a", { id: "a" }], + ["b", { id: "b" }], + ["child", { id: "child", parentID: "a" }], + ]); + let route = { type: "session", sessionID: "a" }; + const listeners = new Set<(event: unknown) => void>(); + const permissions = new Map | undefined>(); + const forms = new Map | undefined>(); + return { + api: { + ui: { router: { current: () => route } }, + data: { + session: { + get: (id: string) => sessions.get(id), + family: () => [...sessions.keys()], + status: () => "idle", + permission: { list: (id: string) => permissions.get(id) }, + form: { list: (id: string) => forms.get(id) }, + }, + listen: (handler: (event: unknown) => void) => { + listeners.add(handler); + return () => listeners.delete(handler); + }, + }, + }, + select(sessionID: string) { route = { type: "session", sessionID }; }, + home() { route = { type: "home", sessionID: "" }; }, + emit(type: string, data?: object) { + for (const listener of listeners) listener({ details: { type, data } }); + }, + listeners, + sessions, + permissions, + forms, + }; +} + +const flushReports = () => new Promise((resolve) => setTimeout(resolve, 10)); +const states = () => requests.filter((r) => requestParam(r, "state") !== undefined) + .map((r) => requestParam(r, "state")); + +test("V2 ignores events without data", async () => { + const plugin = await loadPlugin(); + const tui = v2Api(); + const dispose = await plugin.setup(tui.api); + activeDisposers.push(dispose); + await flushReports(); + requests.length = 0; + expect(() => tui.emit("legacy.event")).not.toThrow(); + tui.emit("session.execution.started", { sessionID: "a" }); + await flushReports(); + expect(states()).toEqual(["working"]); +}); + +test("V2 completes and interrupts without legacy idle events", async () => { + for (const terminal of ["succeeded", "interrupted", "failed"]) { + const plugin = await loadPlugin(); + const tui = v2Api(); + const dispose = await plugin.setup(tui.api); + activeDisposers.push(dispose); + await flushReports(); + requests.length = 0; + tui.emit("session.execution.started", { sessionID: "a" }); + tui.emit(`session.execution.${terminal}`, { sessionID: "a" }); + await flushReports(); + expect(states()).toEqual(["working", terminal === "failed" ? "blocked" : "idle"]); + dispose(); + } +}); + +test("V2 aggregates root and child blockers and ignores other roots and child completion", async () => { + const plugin = await loadPlugin(); + const tui = v2Api(); + const dispose = await plugin.setup(tui.api); + activeDisposers.push(dispose); + await flushReports(); + requests.length = 0; + tui.emit("session.execution.started", { sessionID: "a" }); + tui.emit("permission.asked", { sessionID: "a", id: "permission-a" }); + tui.emit("form.created", { form: { sessionID: "child", id: "form-child" } }); + tui.emit("permission.replied", { sessionID: "a", requestID: "permission-a" }); + tui.emit("session.execution.succeeded", { sessionID: "child" }); + tui.emit("session.execution.started", { sessionID: "b" }); + tui.emit("permission.asked", { sessionID: "b", id: "other" }); + await flushReports(); + expect(states().at(-1)).toBe("blocked"); + expect(requests.every((r) => requestParam(r, "agent_session_id") === "a")).toBe(true); + tui.emit("form.cancelled", { sessionID: "child", id: "form-child" }); + tui.emit("session.execution.succeeded", { sessionID: "a" }); + await flushReports(); + expect(states().slice(-2)).toEqual(["working", "idle"]); +}); + +test("V2 discards queued reports after selection changes and stops on disposal", async () => { + const plugin = await loadPlugin(); + const tui = v2Api(); + const dispose = await plugin.setup(tui.api); + activeDisposers.push(dispose); + await flushReports(); + requests.length = 0; + tui.emit("session.execution.started", { sessionID: "a" }); + tui.select("b"); + tui.emit("session.execution.started", { sessionID: "b" }); + await flushReports(); + expect(requests.every((r) => requestParam(r, "agent_session_id") === "b")).toBe(true); + requests.length = 0; + tui.emit("session.execution.succeeded", { sessionID: "b" }); + tui.home(); + await flushReports(); + expect(requests).toHaveLength(0); + dispose(); + expect(tui.listeners.size).toBe(0); + tui.select("a"); + await new Promise((resolve) => setTimeout(resolve, 250)); + expect(requests).toHaveLength(0); +}); + +test("V2 reconciles late blocker hydration without reviving an already-replied request", async () => { + const plugin = await loadPlugin(); + const tui = v2Api(); + const dispose = await plugin.setup(tui.api); + activeDisposers.push(dispose); + await flushReports(); + tui.permissions.set("child", [{ id: "late" }]); + await waitForStateReport(); + expect(states().at(-1)).toBe("blocked"); + tui.emit("permission.replied", { sessionID: "child", requestID: "late" }); + await waitForStateReport(); + expect(states().at(-1)).toBe("idle"); + tui.permissions.set("child", []); + tui.forms.set("child", [{ id: "second" }]); + await waitForStateReport(); + expect(states().at(-1)).toBe("blocked"); + tui.sessions.delete("child"); + tui.emit("session.deleted", { sessionID: "child" }); + await flushReports(); + expect(states().at(-1)).toBe("idle"); +}); + +test("V2 never writes a delayed connection after disposal or a session switch", async () => { + for (const action of ["dispose", "switch"]) { + const plugin = await loadPlugin(); + const tui = v2Api(); + holdConnections = true; + requests.length = 0; + const dispose = await plugin.setup(tui.api); + activeDisposers.push(dispose); + await flushReports(); + expect(connections.length).toBeGreaterThan(0); + if (action === "dispose") dispose(); + else tui.select("b"); + holdConnections = false; + for (const connect of connections.splice(0)) connect(); + await flushReports(); + expect(requests).toHaveLength(0); + dispose(); + } +}); + +test("V2 settles a connection that never completes", async () => { + const plugin = await loadPlugin(); + const tui = v2Api(); + holdConnections = true; + const dispose = await plugin.setup(tui.api); + activeDisposers.push(dispose); + const started = Date.now(); + while (connections.length <= 1 && Date.now() - started < 2_000) { + await new Promise((resolve) => setTimeout(resolve, 25)); + } + expect(connections.length).toBeGreaterThan(1); + dispose(); +}); + +test("V2 resends the latest state after a failed delivery", async () => { + const plugin = await loadPlugin(); + const tui = v2Api(); + const dispose = await plugin.setup(tui.api); + activeDisposers.push(dispose); + await flushReports(); + // Exhaust the selection retry schedule so only the event report remains. + await new Promise((resolve) => setTimeout(resolve, 1_600)); + requests.length = 0; + tui.emit("session.execution.started", { sessionID: "a" }); + await flushReports(); + failConnections = true; + tui.emit("session.execution.succeeded", { sessionID: "a" }); + const resend = waitForStateReport(); + await new Promise((resolve) => setTimeout(resolve, 700)); + failConnections = false; + await resend; + expect(states().at(-1)).toBe("idle"); + dispose(); +}); diff --git a/src/integration/assets/opencode/tui.js b/src/integration/assets/opencode/tui.js new file mode 100644 index 0000000000..cc1ffe6aa1 --- /dev/null +++ b/src/integration/assets/opencode/tui.js @@ -0,0 +1,5 @@ +// installed by herdr +// HERDR_INTEGRATION_ID=opencode-tui-v2 +// HERDR_INTEGRATION_VERSION=12 +// V2 resolves the directory's tui entrypoint; V1 uses the original file. +export { default } from "../herdr-tui-session.js"; diff --git a/src/integration/env.rs b/src/integration/env.rs index c4ee1c34bf..48320c72cf 100644 --- a/src/integration/env.rs +++ b/src/integration/env.rs @@ -127,6 +127,14 @@ pub(crate) fn opencode_dir() -> io::Result { Ok(home_dir()?.join(".config/opencode")) } +pub(crate) fn opencode_state_dir() -> io::Result { + if let Some(value) = std::env::var_os("XDG_STATE_HOME").filter(|value| !value.is_empty()) { + return expand_tilde_path(PathBuf::from(value)).map(|path| path.join("opencode")); + } + + Ok(home_dir()?.join(".local/state/opencode")) +} + pub(crate) fn kilo_dir() -> io::Result { Ok(home_dir()?.join(".config/kilo")) } @@ -248,3 +256,34 @@ pub(crate) fn integration_env_lock() -> IntegrationEnvLock { appdata: std::env::var_os("APPDATA"), } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn opencode_state_dir_defaults_to_local_state() { + let _lock = integration_env_lock(); + let original = std::env::var_os("XDG_STATE_HOME"); + std::env::remove_var("XDG_STATE_HOME"); + let expected = home_dir().unwrap().join(".local/state/opencode"); + assert_eq!(opencode_state_dir().unwrap(), expected); + match original { + Some(value) => std::env::set_var("XDG_STATE_HOME", value), + None => std::env::remove_var("XDG_STATE_HOME"), + } + } + + #[test] + fn opencode_state_dir_honors_xdg_state_home() { + let _lock = integration_env_lock(); + let original = std::env::var_os("XDG_STATE_HOME"); + let xdg = std::env::temp_dir().join("herdr-xdg-state"); + std::env::set_var("XDG_STATE_HOME", &xdg); + assert_eq!(opencode_state_dir().unwrap(), xdg.join("opencode")); + match original { + Some(value) => std::env::set_var("XDG_STATE_HOME", value), + None => std::env::remove_var("XDG_STATE_HOME"), + } + } +} diff --git a/src/integration/mod.rs b/src/integration/mod.rs index 3bb8485c11..d956c7bcd7 100644 --- a/src/integration/mod.rs +++ b/src/integration/mod.rs @@ -170,7 +170,10 @@ const OPENCODE_PLUGIN_ASSET: &str = include_str!("assets/opencode/herdr-agent-st const OPENCODE_TUI_PLUGIN_INSTALL_NAME: &str = "herdr-tui-session.js"; const OPENCODE_TUI_PLUGIN_SPEC: &str = "./herdr-tui-session.js"; const OPENCODE_TUI_PLUGIN_ASSET: &str = include_str!("assets/opencode/herdr-tui-session.js"); -const OPENCODE_INTEGRATION_VERSION: u32 = 11; +const OPENCODE_V2_TUI_PLUGIN_DIR: &str = "herdr-opencode"; +const OPENCODE_V2_TUI_PLUGIN_SPEC: &str = "./herdr-opencode"; +const OPENCODE_V2_TUI_PLUGIN_ASSET: &str = include_str!("assets/opencode/tui.js"); +const OPENCODE_INTEGRATION_VERSION: u32 = 12; const KILO_PLUGIN_INSTALL_NAME: &str = "herdr-agent-state.js"; const KILO_PLUGIN_ASSET: &str = include_str!("assets/kilo/herdr-agent-state.js"); const KILO_INTEGRATION_VERSION: u32 = 4; diff --git a/src/integration/opencode_config.rs b/src/integration/opencode_config.rs index 57431f9376..2ab91e844c 100644 --- a/src/integration/opencode_config.rs +++ b/src/integration/opencode_config.rs @@ -13,25 +13,53 @@ pub(crate) fn tui_config_path(config_dir: &Path) -> PathBuf { } pub(crate) fn validate_tui_plugin_config(config_dir: &Path) -> io::Result<()> { - let config_path = tui_config_path(config_dir); + validate_plugin_config(&tui_config_path(config_dir), "plugin")?; + validate_plugin_config(&config_dir.join("cli.json"), "plugins") +} + +fn validate_plugin_config(config_path: &Path, key: &str) -> io::Result<()> { if !config_path.is_file() { return Ok(()); } - let content = fs::read_to_string(&config_path)?; - let root = parse_root(&content, &config_path)?; - let object = root_object(&root, &config_path)?; + let content = fs::read_to_string(config_path)?; + let root = parse_root(&content, config_path)?; + let object = root_object(&root, config_path)?; if object - .get("plugin") + .get(key) .is_some_and(|property| property.array_value().is_none()) { - return Err(invalid_plugin_list(&config_path)); + return Err(invalid_plugin_list(config_path)); } Ok(()) } pub(crate) fn add_tui_plugin(config_dir: &Path, plugin_spec: &str) -> io::Result { - let config_path = tui_config_path(config_dir); + add_plugin(tui_config_path(config_dir), "plugin", plugin_spec) +} + +pub(crate) fn add_cli_plugin( + config_dir: &Path, + state_dir: &Path, + plugin_spec: &str, +) -> io::Result> { + let path = config_dir.join("cli.json"); + // OpenCode imports V1 TUI preferences (`tui.json`, `kv.json`) into cli.json on + // its first V2 start, but only while cli.json is absent. Defer registration + // while those sources still exist so we do not skip the migration; otherwise + // create cli.json ourselves, since OpenCode will never do it for a fresh V2 + // install with nothing to migrate. + if !path.is_file() && cli_migration_pending(config_dir, state_dir) { + return Ok(None); + } + add_plugin(path, "plugins", plugin_spec).map(Some) +} + +fn cli_migration_pending(config_dir: &Path, state_dir: &Path) -> bool { + config_dir.join("tui.json").is_file() || state_dir.join("kv.json").is_file() +} + +fn add_plugin(config_path: PathBuf, key: &str, plugin_spec: &str) -> io::Result { let content = if config_path.is_file() { fs::read_to_string(&config_path)? } else { @@ -40,7 +68,7 @@ pub(crate) fn add_tui_plugin(config_dir: &Path, plugin_spec: &str) -> io::Result let root = parse_root(&content, &config_path)?; let object = root_object(&root, &config_path)?; - match object.get("plugin") { + match object.get(key) { Some(property) => { let plugins = property .array_value() @@ -56,7 +84,7 @@ pub(crate) fn add_tui_plugin(config_dir: &Path, plugin_spec: &str) -> io::Result } None => { object.append( - "plugin", + key, CstInputValue::Array(vec![CstInputValue::String(plugin_spec.to_string())]), ); } @@ -67,20 +95,27 @@ pub(crate) fn add_tui_plugin(config_dir: &Path, plugin_spec: &str) -> io::Result } pub(crate) fn remove_tui_plugin(config_dir: &Path, plugin_spec: &str) -> io::Result { - let config_path = tui_config_path(config_dir); + remove_plugin(&tui_config_path(config_dir), "plugin", plugin_spec) +} + +pub(crate) fn remove_cli_plugin(config_dir: &Path, plugin_spec: &str) -> io::Result { + remove_plugin(&config_dir.join("cli.json"), "plugins", plugin_spec) +} + +fn remove_plugin(config_path: &Path, key: &str, plugin_spec: &str) -> io::Result { if !config_path.is_file() { return Ok(false); } - let content = fs::read_to_string(&config_path)?; - let root = parse_root(&content, &config_path)?; - let object = root_object(&root, &config_path)?; - let Some(property) = object.get("plugin") else { + let content = fs::read_to_string(config_path)?; + let root = parse_root(&content, config_path)?; + let object = root_object(&root, config_path)?; + let Some(property) = object.get(key) else { return Ok(false); }; let plugins = property .array_value() - .ok_or_else(|| invalid_plugin_list(&config_path))?; + .ok_or_else(|| invalid_plugin_list(config_path))?; let mut removed = false; for entry in plugins.elements() { if entry @@ -98,23 +133,30 @@ pub(crate) fn remove_tui_plugin(config_dir: &Path, plugin_spec: &str) -> io::Res property.remove(); } - fs::write(&config_path, root.to_string())?; + fs::write(config_path, root.to_string())?; Ok(true) } pub(crate) fn tui_plugin_is_configured(config_dir: &Path, plugin_spec: &str) -> bool { - let config_path = tui_config_path(config_dir); - let Ok(content) = fs::read_to_string(&config_path) else { + plugin_is_configured(&tui_config_path(config_dir), "plugin", plugin_spec) +} + +pub(crate) fn cli_plugin_is_configured(config_dir: &Path, plugin_spec: &str) -> bool { + plugin_is_configured(&config_dir.join("cli.json"), "plugins", plugin_spec) +} + +fn plugin_is_configured(config_path: &Path, key: &str, plugin_spec: &str) -> bool { + let Ok(content) = fs::read_to_string(config_path) else { return false; }; - let Ok(root) = parse_root(&content, &config_path) else { + let Ok(root) = parse_root(&content, config_path) else { return false; }; - let Ok(object) = root_object(&root, &config_path) else { + let Ok(object) = root_object(&root, config_path) else { return false; }; object - .get("plugin") + .get(key) .and_then(|property| property.array_value()) .is_some_and(|plugins| { plugins.elements().iter().any(|entry| { @@ -154,6 +196,7 @@ fn jsonc_parse_options() -> ParseOptions { fn plugin_entry_matches(entry: &Value, plugin_spec: &str) -> bool { entry.as_str() == Some(plugin_spec) + || entry.get("package").and_then(Value::as_str) == Some(plugin_spec) || entry .as_array() .and_then(|parts| parts.first()) @@ -295,4 +338,73 @@ mod tests { fs::remove_dir_all(dir).unwrap(); } + + #[test] + fn cli_registration_preserves_options_and_other_preferences() { + let dir = unique_dir(); + let state = unique_dir(); + let path = dir.join("cli.json"); + fs::write(&path, r#"{"theme":{"name":"catppuccin"},"plugins":[{"package":"./herdr-opencode","options":{"custom":true}},"example"]}"#).unwrap(); + add_cli_plugin(&dir, &state, "./herdr-opencode").unwrap(); + assert!(cli_plugin_is_configured(&dir, "./herdr-opencode")); + assert_eq!(parse_config(&path)["plugins"].as_array().unwrap().len(), 2); + assert_eq!(parse_config(&path)["plugins"][0]["options"]["custom"], true); + assert!(remove_cli_plugin(&dir, "./herdr-opencode").unwrap()); + assert_eq!(parse_config(&path)["plugins"], json!(["example"])); + assert_eq!(parse_config(&path)["theme"]["name"], "catppuccin"); + add_cli_plugin(&dir, &state, "./herdr-opencode").unwrap(); + add_cli_plugin(&dir, &state, "./herdr-opencode").unwrap(); + assert_eq!( + parse_config(&path)["plugins"], + json!(["example", "./herdr-opencode"]) + ); + fs::remove_dir_all(dir).unwrap(); + fs::remove_dir_all(state).unwrap(); + } + + #[test] + fn cli_registration_creates_missing_config_when_no_migration_pending() { + let dir = unique_dir(); + let state = unique_dir(); + let path = add_cli_plugin(&dir, &state, "./herdr-opencode") + .unwrap() + .expect("cli.json should be created when OpenCode has nothing to migrate"); + assert_eq!(path, dir.join("cli.json")); + assert_eq!( + parse_config(&path), + json!({ "plugins": ["./herdr-opencode"] }) + ); + assert!(cli_plugin_is_configured(&dir, "./herdr-opencode")); + fs::remove_dir_all(dir).unwrap(); + fs::remove_dir_all(state).unwrap(); + } + + #[test] + fn cli_registration_defers_while_migration_pending() { + let dir = unique_dir(); + let state = unique_dir(); + fs::write(dir.join("tui.json"), "{}").unwrap(); + assert!(add_cli_plugin(&dir, &state, "./herdr-opencode") + .unwrap() + .is_none()); + assert!(!dir.join("cli.json").exists()); + + fs::remove_file(dir.join("tui.json")).unwrap(); + fs::write(state.join("kv.json"), "{}").unwrap(); + assert!(add_cli_plugin(&dir, &state, "./herdr-opencode") + .unwrap() + .is_none()); + assert!(!dir.join("cli.json").exists()); + + fs::remove_dir_all(dir).unwrap(); + fs::remove_dir_all(state).unwrap(); + } + + #[test] + fn invalid_cli_plugin_list_fails_preflight() { + let dir = unique_dir(); + fs::write(dir.join("cli.json"), r#"{"plugins":{}}"#).unwrap(); + assert!(validate_tui_plugin_config(&dir).is_err()); + fs::remove_dir_all(dir).unwrap(); + } } diff --git a/src/integration/registry.rs b/src/integration/registry.rs index 4625197059..f3bb8c0487 100644 --- a/src/integration/registry.rs +++ b/src/integration/registry.rs @@ -427,6 +427,18 @@ fn opencode_tui_integration_is_valid(plugin_path: &Path, expected_version: u32) config_dir, super::OPENCODE_TUI_PLUGIN_SPEC, ) + && (!config_dir.join("cli.json").exists() + || (super::opencode_config::cli_plugin_is_configured( + config_dir, + super::OPENCODE_V2_TUI_PLUGIN_SPEC, + ) && fs::read_to_string( + config_dir + .join(super::OPENCODE_V2_TUI_PLUGIN_DIR) + .join("tui.js"), + ) + .ok() + .and_then(|content| parse_integration_version(&content)) + .is_some_and(|version| version >= expected_version))) } pub(crate) fn integration_status_at( diff --git a/src/integration/targets.rs b/src/integration/targets.rs index 0690fc906e..d204c55293 100644 --- a/src/integration/targets.rs +++ b/src/integration/targets.rs @@ -22,13 +22,14 @@ use super::config_edit::{ use super::env::{ antigravity_cli_dir, claude_dir, codex_dir, copilot_dir, cursor_dir, devin_dir, droid_dir, grok_dir, hermes_dir, hermes_plugin_dir, kilo_dir, kimi_dir, mastracode_dir, omp_extension_dir, - opencode_dir, pi_extension_dir, qodercli_dir, qwen_dir, + opencode_dir, opencode_state_dir, pi_extension_dir, qodercli_dir, qwen_dir, }; use super::file_ops::{ make_executable, remove_dir_all_if_exists, remove_file_if_exists, remove_legacy_bash_hook_file, }; use super::opencode_config::{ - add_tui_plugin, remove_tui_plugin, tui_config_path, validate_tui_plugin_config, + add_cli_plugin, add_tui_plugin, remove_cli_plugin, remove_tui_plugin, tui_config_path, + validate_tui_plugin_config, }; use super::types::{ AntigravityCliInstallPaths, AntigravityCliUninstallResult, ClaudeInstallPaths, @@ -468,11 +469,20 @@ pub(crate) fn install_opencode() -> io::Result { let tui_plugin_path = dir.join(OPENCODE_TUI_PLUGIN_INSTALL_NAME); fs::write(&tui_plugin_path, OPENCODE_TUI_PLUGIN_ASSET)?; let tui_config_path = add_tui_plugin(&dir, OPENCODE_TUI_PLUGIN_SPEC)?; + let v2_dir = dir.join(super::OPENCODE_V2_TUI_PLUGIN_DIR); + fs::create_dir_all(&v2_dir)?; + fs::write(v2_dir.join("tui.js"), super::OPENCODE_V2_TUI_PLUGIN_ASSET)?; + let cli_config_path = add_cli_plugin( + &dir, + &opencode_state_dir()?, + super::OPENCODE_V2_TUI_PLUGIN_SPEC, + )?; Ok(OpenCodeInstallPaths { plugin_path, tui_plugin_path, tui_config_path, + cli_config_path, }) } @@ -821,6 +831,15 @@ pub(crate) fn uninstall_opencode() -> io::Result { let plugin_path = dir.join("plugins").join(OPENCODE_PLUGIN_INSTALL_NAME); let tui_plugin_path = dir.join(OPENCODE_TUI_PLUGIN_INSTALL_NAME); let mut errors = Vec::new(); + remove_cli_plugin(&dir, super::OPENCODE_V2_TUI_PLUGIN_SPEC).unwrap_or_else(|err| { + errors.push(err.to_string()); + false + }); + let v2_dir = dir.join(super::OPENCODE_V2_TUI_PLUGIN_DIR); + remove_dir_all_if_exists(&v2_dir).unwrap_or_else(|err| { + errors.push(format!("failed to remove {}: {err}", v2_dir.display())); + false + }); let updated_tui_config = remove_tui_plugin(&dir, OPENCODE_TUI_PLUGIN_SPEC).unwrap_or_else(|err| { errors.push(err.to_string()); diff --git a/src/integration/tests.rs b/src/integration/tests.rs index b22100742b..dc597d1d53 100644 --- a/src/integration/tests.rs +++ b/src/integration/tests.rs @@ -134,6 +134,7 @@ fn clear_integration_path_env() { std::env::remove_var(COPILOT_HOME_ENV_VAR); std::env::remove_var(KIMI_CODE_HOME_ENV_VAR); std::env::remove_var("XDG_CONFIG_HOME"); + std::env::remove_var("XDG_STATE_HOME"); #[cfg(windows)] std::env::remove_var("APPDATA"); std::env::remove_var(QODERCLI_CONFIG_DIR_ENV_VAR); @@ -2301,11 +2302,105 @@ fn install_opencode_writes_server_and_tui_plugins() { let tui_config: Value = serde_json::from_str(&fs::read_to_string(&installed.tui_config_path).unwrap()).unwrap(); assert_eq!(tui_config["plugin"], json!([OPENCODE_TUI_PLUGIN_SPEC])); + let cli_config_path = installed + .cli_config_path + .expect("cli.json should be created when OpenCode has nothing to migrate"); + assert_eq!(cli_config_path, opencode_dir.join("cli.json")); + let cli_config: Value = + serde_json::from_str(&fs::read_to_string(&cli_config_path).unwrap()).unwrap(); + assert_eq!(cli_config["plugins"], json!([OPENCODE_V2_TUI_PLUGIN_SPEC])); std::env::remove_var("HOME"); let _ = fs::remove_dir_all(base); } +#[test] +fn opencode_install_defers_v2_registration_while_migration_pending() { + let _lock = integration_env_lock(); + let base = unique_base(); + let home = base.join("home"); + let opencode_dir = home.join(".config/opencode"); + fs::create_dir_all(&opencode_dir).unwrap(); + fs::write(opencode_dir.join("tui.json"), "{}").unwrap(); + std::env::set_var("HOME", &home); + + let installed = install_opencode().unwrap(); + + assert!(installed.cli_config_path.is_none()); + assert!(!opencode_dir.join("cli.json").exists()); + assert!(opencode_dir + .join(OPENCODE_V2_TUI_PLUGIN_DIR) + .join("tui.js") + .is_file()); + + std::env::remove_var("HOME"); + let _ = fs::remove_dir_all(base); +} + +#[test] +fn opencode_v2_install_status_and_uninstall_preserve_cli_preferences() { + let _lock = integration_env_lock(); + let base = unique_base(); + let home = base.join("home"); + let dir = home.join(".config/opencode"); + fs::create_dir_all(&dir).unwrap(); + std::env::set_var("HOME", &home); + let cli = dir.join("cli.json"); + fs::write( + &cli, + r#"{"theme":{"name":"catppuccin"},"plugins":["other"]}"#, + ) + .unwrap(); + let installed = install_opencode().unwrap(); + assert_eq!(installed.cli_config_path, Some(cli.clone())); + let status = || { + integration_status_at( + crate::api::schema::IntegrationTarget::Opencode, + installed.plugin_path.clone(), + OPENCODE_INTEGRATION_VERSION, + ) + .state + }; + assert_eq!(status(), IntegrationStatusKind::Current); + let entry = dir.join(OPENCODE_V2_TUI_PLUGIN_DIR).join("tui.js"); + assert_eq!( + fs::read_to_string(&entry).unwrap(), + OPENCODE_V2_TUI_PLUGIN_ASSET + ); + fs::remove_file(&entry).unwrap(); + assert_eq!(status(), IntegrationStatusKind::Outdated); + install_opencode().unwrap(); + super::opencode_config::remove_cli_plugin(&dir, OPENCODE_V2_TUI_PLUGIN_SPEC).unwrap(); + assert_eq!(status(), IntegrationStatusKind::Outdated); + install_opencode().unwrap(); + uninstall_opencode().unwrap(); + assert!(!entry.exists()); + assert_eq!( + serde_json::from_str::(&fs::read_to_string(cli).unwrap()).unwrap(), + json!({"theme":{"name":"catppuccin"},"plugins":["other"]}) + ); + std::env::remove_var("HOME"); + let _ = fs::remove_dir_all(base); +} + +#[test] +fn opencode_invalid_cli_config_does_not_overwrite_existing_plugins() { + let _lock = integration_env_lock(); + let base = unique_base(); + let home = base.join("home"); + let dir = home.join(".config/opencode"); + fs::create_dir_all(dir.join("plugins")).unwrap(); + std::env::set_var("HOME", &home); + let plugin = dir.join("plugins").join(OPENCODE_PLUGIN_INSTALL_NAME); + fs::write(&plugin, "previous integration").unwrap(); + fs::write(dir.join("cli.json"), r#"{"plugins":{}}"#).unwrap(); + assert!(install_opencode().is_err()); + assert_eq!(fs::read_to_string(plugin).unwrap(), "previous integration"); + assert!(!dir.join("tui.jsonc").exists()); + std::env::remove_var("HOME"); + let _ = fs::remove_dir_all(base); +} + #[test] fn opencode_status_requires_the_tui_plugin_and_config_entry() { let _lock = integration_env_lock(); diff --git a/src/integration/types.rs b/src/integration/types.rs index 2eaf7bf5e4..8169ab9823 100644 --- a/src/integration/types.rs +++ b/src/integration/types.rs @@ -44,6 +44,7 @@ pub(crate) struct OpenCodeInstallPaths { pub plugin_path: PathBuf, pub tui_plugin_path: PathBuf, pub tui_config_path: PathBuf, + pub cli_config_path: Option, } #[derive(Debug)] From 9ad65d9031e8cb16a7b553c0e6f74809e9811e92 Mon Sep 17 00:00:00 2001 From: akbash Date: Sat, 12 Sep 2026 00:38:57 +0300 Subject: [PATCH 04/16] fix: suppress Windows dead-key fallback characters in remote panes (#3972) refs #3948 Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com> --- src/client/input/windows_vti.rs | 25 +++++++++++++++ src/input/encode.rs | 36 ++++++++++++++++++++-- src/input/model.rs | 54 +++++++++++++++++++++++++++++---- src/protocol/wire.rs | 49 +++++++++++++++++++++++++++++- 4 files changed, 154 insertions(+), 10 deletions(-) diff --git a/src/client/input/windows_vti.rs b/src/client/input/windows_vti.rs index d89a02c745..679d5b3198 100644 --- a/src/client/input/windows_vti.rs +++ b/src/client/input/windows_vti.rs @@ -2545,6 +2545,31 @@ mod tests { } } + #[test] + fn vti_altgr_dead_key_preserves_native_record_without_command_modifiers() { + // AltGr+4 press captured in #3948, Spanish ISO layout. + let record = WindowsKeyRecord { + key_down: true, + repeat_count: 1, + virtual_key_code: 52, + virtual_scan_code: 5, + unicode: 0, + control_key_state: 9, + }; + let events = translate_with_provenance(win32_input_mode_encoded_record(record)); + assert_eq!( + events, + vec![crate::protocol::ClientInputEvent::Key { + code: crate::protocol::ClientKeyCode::Char('4'), + modifiers: 0, + kind: crate::protocol::ClientKeyKind::Press, + repeat_count: 1, + generated_text: None, + source: crate::protocol::ClientKeySource::WindowsConsole { record }, + }] + ); + } + #[test] fn vti_us_international_dead_key_only_emits_composed_text() { fn encode_for_kitty(events: Vec, flags: u16) -> Vec { diff --git a/src/input/encode.rs b/src/input/encode.rs index 1dd56351cd..27e59dce52 100644 --- a/src/input/encode.rs +++ b/src/input/encode.rs @@ -16,10 +16,10 @@ pub fn encode_key(key: KeyEvent, protocol: KeyboardProtocol) -> Vec { } pub fn encode_terminal_key(key: TerminalKey, protocol: KeyboardProtocol) -> Vec { - // A zero Unicode value on this Windows character event means the host layout is - // still composing a dead key. Kitty panes must not receive its physical fallback. + // The host layout has not committed text for this Windows dead key. Neither + // legacy nor Kitty panes should receive its physical character fallback. // Legacy Windows panes take the native ConPTY fallback before reaching this encoder. - if matches!(protocol, KeyboardProtocol::Kitty { .. }) && key.is_windows_shift_dead_key() { + if key.is_windows_dead_key() { return Vec::new(); } @@ -571,6 +571,36 @@ mod tests { assert_eq!(actual.shifted_codepoint, shifted_codepoint); } + #[test] + fn kitty_all_keys_does_not_encode_windows_altgr_dead_key_phases() { + use crossterm::event::KeyEventKind; + + let key = TerminalKey::new(KeyCode::Char('4'), KeyModifiers::empty()).with_windows_record( + crate::input::WindowsKeyRecord { + key_down: true, + repeat_count: 1, + virtual_key_code: 52, + virtual_scan_code: 5, + unicode: 0, + control_key_state: 9, + }, + ); + for kind in [ + KeyEventKind::Press, + KeyEventKind::Repeat, + KeyEventKind::Release, + ] { + assert!( + encode_terminal_key( + key.clone().with_kind(kind), + KeyboardProtocol::Kitty { flags: 31 }, + ) + .is_empty(), + "{kind:?}" + ); + } + } + #[test] fn legacy_enter() { let key = KeyEvent::new(KeyCode::Enter, KeyModifiers::empty()); diff --git a/src/input/model.rs b/src/input/model.rs index 50cd7e42ad..3828174ffc 100644 --- a/src/input/model.rs +++ b/src/input/model.rs @@ -78,7 +78,7 @@ pub struct TerminalKey { pub shifted_codepoint: Option, pub generated_text: Option, physical_identity_hint: bool, - windows_shift_dead_key: bool, + windows_dead_key: bool, source: KeySource, } @@ -92,7 +92,7 @@ impl TerminalKey { shifted_codepoint: None, generated_text: None, physical_identity_hint: false, - windows_shift_dead_key: false, + windows_dead_key: false, source: KeySource::Synthesized, } } @@ -160,8 +160,10 @@ impl TerminalKey { mut self, record: Option, ) -> Self { - self.windows_shift_dead_key = matches!(self.code, KeyCode::Char(_)) - && self.modifiers == KeyModifiers::SHIFT + // AltGr is normalized to text-only modifiers by the Windows input mapper. + // Command chords can also have zero Unicode, so retain their fallback keys. + self.windows_dead_key = matches!(self.code, KeyCode::Char(_)) + && self.modifiers.difference(KeyModifiers::SHIFT).is_empty() && record.is_some_and(|record| record.unicode == 0); self } @@ -189,8 +191,8 @@ impl TerminalKey { None } - pub(crate) fn is_windows_shift_dead_key(&self) -> bool { - self.windows_shift_dead_key + pub(crate) fn is_windows_dead_key(&self) -> bool { + self.windows_dead_key } pub(crate) fn identity(&self) -> KeyIdentity { @@ -422,6 +424,46 @@ mod tests { assert_eq!(key.repeat_count, 1); } + #[test] + fn windows_composition_hint_requires_uncommitted_text_not_a_command() { + let record = WindowsKeyRecord { + key_down: true, + repeat_count: 1, + virtual_key_code: 52, + virtual_scan_code: 5, + unicode: 0, + control_key_state: 9, + }; + for modifiers in [ + KeyModifiers::CONTROL, + KeyModifiers::ALT, + KeyModifiers::CONTROL | KeyModifiers::ALT, + KeyModifiers::SUPER, + ] { + let key = TerminalKey::new(KeyCode::Char('4'), modifiers) + .with_windows_composition_hint(Some(record)); + assert!( + !key.is_windows_dead_key(), + "command modifiers: {modifiers:?}" + ); + } + for (code, source) in [ + (KeyCode::Left, Some(record)), + (KeyCode::Char('4'), None), + ( + KeyCode::Char('~'), + Some(WindowsKeyRecord { + unicode: 126, + ..record + }), + ), + ] { + let key = + TerminalKey::new(code, KeyModifiers::empty()).with_windows_composition_hint(source); + assert!(!key.is_windows_dead_key(), "{code:?}, {source:?}"); + } + } + #[test] fn release_clears_generated_text_and_grouped_repeat_count() { let release = TerminalKey::new(KeyCode::Char('a'), KeyModifiers::empty()) diff --git a/src/protocol/wire.rs b/src/protocol/wire.rs index 96881b146f..052c73a3fd 100644 --- a/src/protocol/wire.rs +++ b/src/protocol/wire.rs @@ -2066,7 +2066,7 @@ mod tests { else { panic!("pane dead key should remain a key"); }; - assert!(key.is_windows_shift_dead_key()); + assert!(key.is_windows_dead_key()); assert_eq!(key.windows_record(), None); assert!(crate::input::encode_terminal_key( key, @@ -2075,6 +2075,53 @@ mod tests { .is_empty()); } + #[tokio::test] + async fn client_shell_remote_altgr_dead_key_emits_only_composed_text() { + // Spanish ISO AltGr+4, then Space, captured in #3948: + // https://github.com/herdrdev/herdr/issues/3948#issuecomment-5633222390 + let records = [ + ('4', ClientKeyKind::Press, 52, 5, 0, 9), + ('4', ClientKeyKind::Release, 52, 5, 0, 9), + ('~', ClientKeyKind::Press, 32, 57, 126, 0), + (' ', ClientKeyKind::Release, 32, 57, 32, 0), + ]; + let (runtime, _rx) = crate::terminal::TerminalRuntime::test_with_channel(80, 24); + let mut output = Vec::new(); + for (ch, kind, virtual_key_code, virtual_scan_code, unicode, control_key_state) in records { + let event = ClientInputEvent::Key { + code: ClientKeyCode::Char(ch), + modifiers: 0, + kind, + repeat_count: 1, + generated_text: None, + source: ClientKeySource::WindowsConsole { + record: crate::input::WindowsKeyRecord { + key_down: kind == ClientKeyKind::Press, + repeat_count: 1, + virtual_key_code, + virtual_scan_code, + unicode, + control_key_state, + }, + }, + }; + let crate::raw_input::RawInputEvent::Key(key) = event.to_raw_input_event() else { + panic!("captured input should remain a key"); + }; + let pane_event = ClientPaneInputEvent::from_terminal_key(key).expect("pane key"); + let crate::raw_input::RawInputEvent::Key(key) = + pane_event.to_raw_input_event_with_windows_source(false) + else { + panic!("remote input should remain a key"); + }; + let bytes = runtime.encode_terminal_key(key); + let expected: &[u8] = if ch == '~' { b"~" } else { b"" }; + assert_eq!(bytes, expected, "captured {ch:?} {kind:?}"); + output.extend(bytes); + } + assert_eq!(output, b"~", "dead key must not insert its base character"); + } + #[test] fn client_shell_key_roundtrip_preserves_physical_generated_text_encoding() { let key = crate::input::TerminalKey::new( From cc36a2943e85134768631d6d289e331c0edb940d Mon Sep 17 00:00:00 2001 From: Can Celik Date: Sat, 12 Sep 2026 13:53:33 +0300 Subject: [PATCH 05/16] fix: preserve complete alternate-screen history reads (#3979) --- src/terminal/history_read.rs | 302 +++++++++++++++++++++++++++++++---- 1 file changed, 270 insertions(+), 32 deletions(-) diff --git a/src/terminal/history_read.rs b/src/terminal/history_read.rs index decce3aaa0..3938116f43 100644 --- a/src/terminal/history_read.rs +++ b/src/terminal/history_read.rs @@ -1,3 +1,5 @@ +use std::collections::HashMap; + use crate::ghostty::{CellWide, ScreenTextRow}; use crate::pane::TerminalReadSnapshot; @@ -54,30 +56,27 @@ pub(crate) fn merge_scrolled_up( if previous_text == next_text { return UpwardMerge::Unchanged; } - let Some(shift) = best_upward_shift(&previous_text, &next_text) else { - return UpwardMerge::Unaligned; - }; - let Some(boundary) = (0..previous_text.len().saturating_sub(shift)).find_map(|index| { - let next_index = index + shift; - (!previous_text[index].is_empty() && previous_text[index] == next_text[next_index]) - .then_some(next_index) - }) else { + let Some((shift, anchor)) = upward_alignment(&previous_text, &next_text) else { return UpwardMerge::Unaligned; }; - let added: Vec<_> = next.rows[..boundary] + let history_text = row_identities(history); + // A retained fragment may lack a complete scrollbar track/thumb pattern; + // keep exact text matching alongside normalized identities for that case. + let anchor_text = row_text(&previous.rows[anchor]); + let Some(history_anchor) = history_text .iter() - .enumerate() - .filter(|(index, _)| { - next_text[*index].is_empty() || previous_text.get(*index) != Some(&next_text[*index]) - }) - .map(|(_, row)| row.clone()) - .collect(); - if added.is_empty() { + .take(previous.rows.len()) + .position(|text| text == &previous_text[anchor] || text.as_str() == anchor_text.trim_end()) + else { return UpwardMerge::Unaligned; + }; + let boundary = anchor + shift; + // Refresh the overlap as well as new rows: a pinned header may have obscured + // transcript text at the top of the previous viewport. + history.splice(0..history_anchor, next.rows[..boundary].iter().cloned()); + UpwardMerge::Advanced { + rows: boundary.saturating_sub(history_anchor), } - let rows = added.len(); - history.splice(0..0, added); - UpwardMerge::Advanced { rows } } pub(crate) fn snapshot_text( @@ -96,12 +95,21 @@ pub(crate) fn snapshot_text( TerminalReadSnapshot { text, truncated } } -fn best_upward_shift(previous: &[String], next: &[String]) -> Option { - let mut best = None; +fn upward_alignment(previous: &[String], next: &[String]) -> Option<(usize, usize)> { + let mut previous_counts = HashMap::new(); + let mut next_counts = HashMap::new(); + for text in previous { + *previous_counts.entry(text.as_str()).or_insert(0usize) += 1; + } + for text in next { + *next_counts.entry(text.as_str()).or_insert(0usize) += 1; + } + let mut alignment = None; for shift in 1..previous.len() { let overlap = previous.len() - shift; let mut comparable = 0usize; let mut matches = 0usize; + let mut first_anchor = None; for index in 0..overlap { let before = &previous[index]; let after = &next[index + shift]; @@ -111,26 +119,59 @@ fn best_upward_shift(previous: &[String], next: &[String]) -> Option { comparable += 1; if before == after { matches += 1; + if previous_counts.get(before.as_str()) == Some(&1) + && next_counts.get(after.as_str()) == Some(&1) + { + first_anchor.get_or_insert(index); + } } } - if comparable == 0 - || matches.saturating_mul(100) < comparable.saturating_mul(MIN_ALIGNMENT_RATIO_PERCENT) - { + // Repeated rows alone cannot distinguish a small scroll from a larger one. + let Some(anchor) = first_anchor else { + continue; + }; + if matches.saturating_mul(100) < comparable.saturating_mul(MIN_ALIGNMENT_RATIO_PERCENT) { continue; } - if best.is_none_or(|(_, best_matches, best_comparable)| { - matches > best_matches || (matches == best_matches && comparable > best_comparable) - }) { - best = Some((shift, matches, comparable)); + // A row unique in each viewport can still occur elsewhere in the transcript. + // Competing plausible shifts are ambiguous, not votes for the best score. + if alignment.is_some() { + return None; } + alignment = Some((shift, anchor)); } - best.map(|(shift, _, _)| shift) + alignment } fn row_identities(rows: &[ScreenTextRow]) -> Vec { - rows.iter() + let mut identities: Vec<_> = rows + .iter() .map(|row| row_text(row).trim_end().to_string()) - .collect() + .collect(); + let mut start = 0; + while start < rows.len() { + let mut end = start; + let mut track = false; + let mut thumb = false; + while end < rows.len() && rows[end].cells.len() == rows[start].cells.len() { + match rows[end].cells.last().map(|cell| cell.graphemes.as_slice()) { + Some([0x2502]) => track = true, + Some([0x2503 | 0x2588]) => thumb = true, + _ => break, + } + end += 1; + } + // Require a vertical track and thumb, not an isolated border character. + // Normalize comparison keys only; retained terminal cells stay untouched. + if end - start >= 3 && track && thumb { + for identity in &mut identities[start..end] { + identity.pop(); + identity.truncate(identity.trim_end().len()); + } + } + start = end.max(start + 1); + } + identities } fn wrapped_text(rows: &[ScreenTextRow]) -> String { @@ -257,7 +298,164 @@ mod tests { ); assert_eq!( row_identities(&history), - ["line 2", "line 3", "sticky", "line 4", "line 5", "line 6", "line 7"] + ["sticky", "line 2", "line 3", "line 4", "line 5", "line 6", "line 7"] + ); + } + + #[test] + fn later_overlap_recovers_text_obscured_by_a_pinned_header() { + let initial = snapshot(&["line 4", "line 5", "line 6", "line 7", "line 8"]); + let scrolled = snapshot(&["pinned", "line 3", "line 4", "line 5", "line 6"]); + let older = snapshot(&["pinned", "line 1", "line 2", "line 3", "line 4"]); + let top = snapshot(&["title", "line 0", "line 1", "line 2", "line 3"]); + let mut history = initial.rows.clone(); + for (previous, next) in [(&initial, &scrolled), (&scrolled, &older), (&older, &top)] { + assert!(matches!( + merge_scrolled_up(&mut history, previous, next), + UpwardMerge::Advanced { .. } + )); + } + assert_eq!( + row_identities(&history), + [ + "title", "line 0", "line 1", "line 2", "line 3", "line 4", "line 5", "line 6", + "line 7", "line 8" + ] + ); + } + + #[test] + fn scrolling_preserves_repeated_continuation_rows() { + let rows: Vec<_> = (1..=18) + .flat_map(|number| { + [ + row(&format!("line {number:03} begins")), + row("same continuation"), + row("same ending"), + ] + }) + .collect(); + for shift in [3, 15] { + for offset in 0..3 { + let previous = ScreenSnapshot { + cols: 20, + rows: rows[offset + shift..offset + shift + 36].to_vec(), + }; + let next = ScreenSnapshot { + cols: 20, + rows: rows[offset..offset + 36].to_vec(), + }; + let mut history = previous.rows.clone(); + + assert_eq!( + merge_scrolled_up(&mut history, &previous, &next), + UpwardMerge::Advanced { rows: shift }, + "shift={shift}, offset={offset}" + ); + assert_eq!(&history[..shift], &next.rows[..shift]); + assert_eq!(&history[shift..], &previous.rows); + } + } + } + + fn transcript(first: usize, scrollbar: bool) -> ScreenSnapshot { + let mut rows: Vec<_> = (first..first + 10) + .enumerate() + .map(|(index, number)| { + let edge = if !scrollbar { + ' ' + } else if (3..6).contains(&index) { + '┃' + } else { + '│' + }; + row(&format!("{:<19}{edge}", format!("line {number}"))) + }) + .collect(); + rows.extend([row("prompt"), row("status")]); + ScreenSnapshot { cols: 20, rows } + } + + #[test] + fn scrolling_with_an_appearing_scrollbar_recovers_history_and_preserves_cells() { + let initial = transcript(10, false); + let scrolled = transcript(7, true); + let mut history = initial.rows.clone(); + + assert_eq!( + merge_scrolled_up(&mut history, &initial, &scrolled), + UpwardMerge::Advanced { rows: 3 } + ); + assert_eq!(&history[..3], &scrolled.rows[..3]); + assert_eq!(&history[3..], &initial.rows); + assert!(initial.similar_text(&transcript(10, true))); + assert!(!initial.similar_text(&scrolled)); + + let mut older = transcript(4, true); + for line in &mut older.rows[..10] { + let edge = line.cells.last_mut().unwrap(); + edge.graphemes = if edge.graphemes == ['│' as u32] { + vec!['█' as u32] + } else { + vec!['│' as u32] + }; + } + assert_eq!( + merge_scrolled_up(&mut history, &scrolled, &older), + UpwardMerge::Advanced { rows: 3 } + ); + assert_eq!(&history[..3], &older.rows[..3]); + } + + #[test] + fn chained_scroll_matches_a_retained_anchor_after_the_scrollbar_thumb_moves() { + let initial = transcript(10, true); + let mut scrolled = transcript(7, true); + let mut older = transcript(4, true); + for line in &mut scrolled.rows[..3] { + *line = row(&format!("{:<19}│", "repeated")); + } + for line in &mut older.rows[3..6] { + *line = row(&format!("{:<19}┃", "repeated")); + } + let mut history = initial.rows.clone(); + assert_eq!( + merge_scrolled_up(&mut history, &initial, &scrolled), + UpwardMerge::Advanced { rows: 3 } + ); + assert_eq!( + merge_scrolled_up(&mut history, &scrolled, &older), + UpwardMerge::Advanced { rows: 3 } + ); + assert_eq!(&history[..6], &older.rows[..6]); + assert_eq!(&history[6..], &initial.rows); + } + + #[test] + fn scrollbar_normalization_does_not_hide_real_edge_text_or_align_unrelated_output() { + let initial = transcript(10, false); + let mut changed = transcript(10, true); + for line in &mut changed.rows[..10] { + line.cells[18].graphemes = vec!['x' as u32]; + } + assert!(!initial.similar_text(&changed)); + let mut history = initial.rows.clone(); + assert_eq!( + merge_scrolled_up(&mut history, &initial, &changed), + UpwardMerge::Unaligned + ); + assert_eq!(history, initial.rows); + + let mut real_edge = initial.clone(); + for line in &mut real_edge.rows[..10] { + line.cells[19].graphemes = vec!['x' as u32]; + } + assert!(!real_edge.similar_text(&transcript(10, true))); + + let boxed = snapshot(&["one │", "two │", "three │", "four │"]); + assert_eq!( + row_identities(&boxed.rows), + ["one │", "two │", "three │", "four │"] ); } @@ -281,6 +479,46 @@ mod tests { assert_eq!(history, previous.rows); } + #[test] + fn history_anchor_matches_text_despite_different_wrap_metadata() { + let previous = snapshot(&["line 3", "line 4", "line 5", "status"]); + let next = snapshot(&["line 1", "line 2", "line 3", "line 4"]); + let mut history = previous.rows.clone(); + history[0].wrap_continuation = true; + assert_eq!( + merge_scrolled_up(&mut history, &previous, &next), + UpwardMerge::Advanced { rows: 2 } + ); + assert_eq!( + row_identities(&history), + ["line 1", "line 2", "line 3", "line 4", "line 5", "status"] + ); + } + + #[test] + fn competing_unique_anchors_do_not_prove_scroll_distance() { + let previous = snapshot(&["U1", "U2", "U3", "c", "d"]); + let next = snapshot(&["a", "b", "U2", "U3", "U1"]); + let mut history = previous.rows.clone(); + assert_eq!( + merge_scrolled_up(&mut history, &previous, &next), + UpwardMerge::Unaligned + ); + assert_eq!(history, previous.rows); + } + + #[test] + fn repeated_rows_alone_do_not_prove_scroll_distance() { + let previous = snapshot(&["a", "b", "a", "b", "a", "b"]); + let next = snapshot(&["b", "a", "b", "a", "b", "a"]); + let mut history = previous.rows.clone(); + assert_eq!( + merge_scrolled_up(&mut history, &previous, &next), + UpwardMerge::Unaligned + ); + assert_eq!(history, previous.rows); + } + #[test] fn snapshot_text_limits_rendered_rows_before_unwrapping() { let mut first = row("hello "); From a5d5f6f654a32b27b2bd4dd8edd82f6795359269 Mon Sep 17 00:00:00 2001 From: JJ Liebig Date: Sat, 12 Sep 2026 15:52:18 +0400 Subject: [PATCH 06/16] fix: ignore nested OMP session reports (#3994) refs #2593 --- .../assets/herdr-agent-state.test.ts | 28 +++++++++++++++++++ .../assets/omp/herdr-agent-state.ts | 8 ++++-- src/integration/mod.rs | 2 +- src/pane.rs | 13 +++++++++ 4 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/integration/assets/herdr-agent-state.test.ts b/src/integration/assets/herdr-agent-state.test.ts index 48cc8708ee..19b36d9430 100644 --- a/src/integration/assets/herdr-agent-state.test.ts +++ b/src/integration/assets/herdr-agent-state.test.ts @@ -11,6 +11,7 @@ const originalEnvironment = { HERDR_OMP_IDLE_DEBOUNCE_MS: process.env.HERDR_OMP_IDLE_DEBOUNCE_MS, HERDR_PANE_ID: process.env.HERDR_PANE_ID, HERDR_SOCKET_PATH: process.env.HERDR_SOCKET_PATH, + OMPCODE: process.env.OMPCODE, }; let server: Server | undefined; @@ -229,6 +230,33 @@ for (const integration of integrations) { }); } +test("OMP ignores nested sessions launched inside another OMP shell", async () => { + const requests = await startRecordingServer("omp-nested"); + process.env.OMPCODE = "1"; + const { handlers, pi } = createExtensionHarness(); + + const { default: install } = await importFresh("./omp/herdr-agent-state.ts"); + install(pi); + + // OMP sets OMPCODE on every shell it spawns. A nested `omp` inherits it and + // must not claim the pane's session for its short-lived conversation. + expect(handlers.size).toBe(0); + await handlers.get("session_start")?.( + { reason: "startup" }, + { + hasUI: true, + isIdle: () => true, + sessionManager: { + getSessionFile: () => "/tmp/omp-nested.jsonl", + getSessionId: () => "omp-nested", + }, + }, + ); + await Bun.sleep(25); + + expect(requests).toEqual([]); +}); + test("OMP accepts POSIX and Windows session paths", async () => { const { isAbsoluteSessionPath } = await importFresh("./omp/herdr-agent-state.ts"); diff --git a/src/integration/assets/omp/herdr-agent-state.ts b/src/integration/assets/omp/herdr-agent-state.ts index 0e49378755..fce62106f6 100644 --- a/src/integration/assets/omp/herdr-agent-state.ts +++ b/src/integration/assets/omp/herdr-agent-state.ts @@ -2,7 +2,7 @@ // managed by herdr; reinstalling or updating the integration overwrites this file. // add custom hooks/plugins beside this file instead of editing it. // HERDR_INTEGRATION_ID=omp -// HERDR_INTEGRATION_VERSION=9 +// HERDR_INTEGRATION_VERSION=10 // @ts-nocheck import net from "node:net"; @@ -14,9 +14,13 @@ const socketEndpoint = process.platform === "win32" && socketPath ? `\\\\.\\pipe\\${socketPath}` : socketPath; const paneId = process.env.HERDR_PANE_ID; const source = "herdr:omp"; +// OMP marks every shell it spawns with OMPCODE=1. A nested `omp` launched from +// a parent session's shell inherits it, so that process is not the pane's root +// agent and must not report its short-lived session over the parent's. +const nestedOmpSession = process.env.OMPCODE === "1"; function enabled() { - return HERDR_ENV === "1" && !!socketPath && !!paneId; + return HERDR_ENV === "1" && !!socketPath && !!paneId && !nestedOmpSession; } let requestQueue = Promise.resolve(); diff --git a/src/integration/mod.rs b/src/integration/mod.rs index d956c7bcd7..49808ca273 100644 --- a/src/integration/mod.rs +++ b/src/integration/mod.rs @@ -27,7 +27,7 @@ const PI_EXTENSION_ASSET: &str = include_str!("assets/pi/herdr-agent-state.ts"); const PI_INTEGRATION_VERSION: u32 = 9; const OMP_EXTENSION_INSTALL_NAME: &str = "herdr-omp-agent-state.ts"; const OMP_EXTENSION_ASSET: &str = include_str!("assets/omp/herdr-agent-state.ts"); -const OMP_INTEGRATION_VERSION: u32 = 9; +const OMP_INTEGRATION_VERSION: u32 = 10; const CLAUDE_HOOK_INSTALL_NAME: &str = if cfg!(windows) { "herdr-agent-state.ps1" } else { diff --git a/src/pane.rs b/src/pane.rs index 0ba411d0c0..c56cfbb3ed 100644 --- a/src/pane.rs +++ b/src/pane.rs @@ -147,6 +147,9 @@ impl PaneLaunchEnv { fn apply_pane_launch_env(cmd: &mut CommandBuilder, launch_env: &PaneLaunchEnv) { cmd.env_remove("CODEX_THREAD_ID"); + // OMP sets OMPCODE for shells it spawns. A pane launched from inside OMP + // must not inherit it or its root agent would look like a nested session. + cmd.env_remove("OMPCODE"); for (key, value) in &launch_env.extra { cmd.env(key, value); } @@ -3557,6 +3560,16 @@ mod tests { assert!(cmd.get_env("CODEX_THREAD_ID").is_none()); } + #[test] + fn pane_launch_env_removes_outer_ompcode_marker() { + let mut cmd = CommandBuilder::new("shell"); + cmd.env("OMPCODE", "1"); + + apply_pane_launch_env(&mut cmd, &PaneLaunchEnv::default()); + + assert!(cmd.get_env("OMPCODE").is_none()); + } + #[test] fn pane_terminal_identity_removes_outer_windows_terminal_session() { let mut cmd = CommandBuilder::new("shell"); From 6cf4bbb031b5e6b16e39ea204a5f7e5dc90f7474 Mon Sep 17 00:00:00 2001 From: JJ Liebig Date: Sat, 12 Sep 2026 19:24:34 +0400 Subject: [PATCH 07/16] fix: finish windows ssh setup after bridge exit (#3719) refs #3651 --- src/remote/attach.rs | 80 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 75 insertions(+), 5 deletions(-) diff --git a/src/remote/attach.rs b/src/remote/attach.rs index 4f8d2b7c74..e6abc8f4a9 100644 --- a/src/remote/attach.rs +++ b/src/remote/attach.rs @@ -350,8 +350,10 @@ fn windows_powershell_streaming_application_command(path: &str, args: &[&str]) - .map(|arg| crate::platform::quote_windows_command_line_arg(arg)) .collect::>() .join(" "); + // Start-Process -Wait waits for descendants, including a cold-started server. + // Retain the handle so Windows PowerShell 5.1 keeps the application's exit code. windows_powershell_script_command(&format!( - "$process = Start-Process -FilePath {} -ArgumentList {} -NoNewWindow -Wait -PassThru -ErrorAction Stop; exit $process.ExitCode", + "$process = Start-Process -FilePath {} -ArgumentList {} -NoNewWindow -PassThru -ErrorAction Stop; $null = $process.Handle; $process.WaitForExit(); exit $process.ExitCode", crate::platform::quote_powershell_arg(path), crate::platform::quote_powershell_arg(&command_line), )) @@ -3615,6 +3617,74 @@ mod tests { } } + #[cfg(windows)] + #[test] + fn windows_bridge_returns_application_exit_while_descendant_is_running() { + let pid_file = std::env::temp_dir().join(format!( + "herdr bridge descendant {}-{}.pid", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("current time") + .as_nanos() + )); + let script = format!( + "$child = Start-Process powershell.exe -ArgumentList '-NoProfile -NonInteractive -Command Start-Sleep -Seconds 30' -NoNewWindow -PassThru; Set-Content -LiteralPath {} -Value $child.Id; exit 23", + crate::platform::quote_powershell_arg(&pid_file.to_string_lossy()) + ); + let encoded = base64::engine::general_purpose::STANDARD.encode( + script + .encode_utf16() + .flat_map(u16::to_le_bytes) + .collect::>(), + ); + let command = windows_powershell_streaming_application_command( + "powershell.exe", + &["-NoProfile", "-NonInteractive", "-EncodedCommand", &encoded], + ); + let mut launcher = Command::new("powershell.exe"); + launcher + .args(["-NoProfile", "-NonInteractive", "-EncodedCommand"]) + .arg(command.split_whitespace().last().unwrap()) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + crate::platform::configure_background_command(&mut launcher); + let mut launcher = launcher.spawn().expect("launch Windows bridge command"); + let deadline = Instant::now() + Duration::from_secs(10); + let status = loop { + let status = launcher.try_wait().expect("poll bridge launcher"); + if status.is_some() || Instant::now() >= deadline { + break status; + } + thread::sleep(Duration::from_millis(20)); + }; + if status.is_none() { + let mut cleanup = Command::new("taskkill.exe"); + cleanup.args(["/PID", &launcher.id().to_string(), "/T", "/F"]); + crate::platform::configure_background_command(&mut cleanup); + let _ = cleanup.output(); + let _ = launcher.kill(); + } + let _ = launcher.wait(); + // Clean up the launcher before a missing PID can fail the test. + let descendant = fs::read_to_string(&pid_file); + let _ = fs::remove_file(pid_file); + let descendant = descendant.expect("descendant PID"); + let descendant = descendant.trim().parse::().expect("numeric PID"); + let mut cleanup = Command::new("powershell.exe"); + cleanup + .args(["-NoProfile", "-NonInteractive", "-Command"]) + .arg(format!("Stop-Process -Id {descendant} -ErrorAction Stop")); + crate::platform::configure_background_command(&mut cleanup); + let descendant_was_running = cleanup.status().expect("stop test descendant").success(); + assert!( + descendant_was_running, + "descendant must outlive the application" + ); + assert_eq!(status.and_then(|status| status.code()), Some(23)); + } + #[test] fn windows_remote_commands_use_one_encoded_powershell_grammar() { fn decode(command: &str) -> String { @@ -3661,22 +3731,22 @@ mod tests { ( "direct bridge", executable.bridge_command("agents"), - "$process = Start-Process -FilePath herdr.exe -ArgumentList '--session agents remote-client-bridge' -NoNewWindow -Wait -PassThru -ErrorAction Stop; exit $process.ExitCode", + "$process = Start-Process -FilePath herdr.exe -ArgumentList '--session agents remote-client-bridge' -NoNewWindow -PassThru -ErrorAction Stop; $null = $process.Handle; $process.WaitForExit(); exit $process.ExitCode", ), ( "API bridge with explicit default session", remote_api_bridge_command(&RemoteHerdr::for_platform(RemotePlatform { os: "windows", arch: "x86_64" }), "default", false), - "$process = Start-Process -FilePath herdr.exe -ArgumentList '--session default remote-api-bridge' -NoNewWindow -Wait -PassThru -ErrorAction Stop; exit $process.ExitCode", + "$process = Start-Process -FilePath herdr.exe -ArgumentList '--session default remote-api-bridge' -NoNewWindow -PassThru -ErrorAction Stop; $null = $process.Handle; $process.WaitForExit(); exit $process.ExitCode", ), ( "API bridge capability probe", remote_api_bridge_command(&RemoteHerdr::for_platform(RemotePlatform { os: "windows", arch: "x86_64" }), "agents", true), - "$process = Start-Process -FilePath herdr.exe -ArgumentList '--session agents remote-api-bridge --check' -NoNewWindow -Wait -PassThru -ErrorAction Stop; exit $process.ExitCode", + "$process = Start-Process -FilePath herdr.exe -ArgumentList '--session agents remote-api-bridge --check' -NoNewWindow -PassThru -ErrorAction Stop; $null = $process.Handle; $process.WaitForExit(); exit $process.ExitCode", ), ( "saved bridge with closed stdin", executable.saved_bridge_command("agents"), - "$process = Start-Process -FilePath herdr.exe -ArgumentList '--session agents remote-client-bridge' -NoNewWindow -Wait -PassThru -ErrorAction Stop; exit $process.ExitCode", + "$process = Start-Process -FilePath herdr.exe -ArgumentList '--session agents remote-client-bridge' -NoNewWindow -PassThru -ErrorAction Stop; $null = $process.Handle; $process.WaitForExit(); exit $process.ExitCode", ), ]; From c17dc3fda7e124bab07bd04bc47c82994c9e91e3 Mon Sep 17 00:00:00 2001 From: JJ Liebig Date: Sat, 12 Sep 2026 20:00:01 +0400 Subject: [PATCH 08/16] fix: keep windows endpoint writes progressing (#3721) refs #3651 --- src/client/endpoint/writer.rs | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/src/client/endpoint/writer.rs b/src/client/endpoint/writer.rs index 973ff2348e..c4e9492312 100644 --- a/src/client/endpoint/writer.rs +++ b/src/client/endpoint/writer.rs @@ -163,11 +163,23 @@ fn write_frame( stopped: &AtomicBool, ) -> io::Result<()> { let deadline = Instant::now() + WRITE_TIMEOUT; + #[cfg(windows)] + let mut deadline = deadline; while !frame.is_empty() && !stopped.load(Ordering::Acquire) { - match writer.write(frame) { + // Match interprocess's 512-byte pipe buffer hint: larger nonblocking Windows + // writes can make no progress when the peer polls instead of blocking on read. + #[cfg(windows)] + let chunk = &frame[..frame.len().min(512)]; + #[cfg(not(windows))] + let chunk = frame; + match writer.write(chunk) { Ok(0) => {} Ok(written) => { frame = &frame[written..]; + #[cfg(windows)] + { + deadline = Instant::now() + WRITE_TIMEOUT; + } continue; } Err(error) if error.kind() == io::ErrorKind::Interrupted => continue, @@ -276,7 +288,24 @@ mod tests { #[test] fn native_endpoint_flush_drains_large_frames_before_detach() { - let (stream, mut peer, path) = streams(); + // The SSH bridge polls for available bytes instead of posting a blocking read. + struct PollingPeer(LocalStream); + impl io::Read for PollingPeer { + fn read(&mut self, buffer: &mut [u8]) -> io::Result { + loop { + match crate::ipc::poll_local_stream_read_count(&mut self.0, buffer)? { + crate::ipc::LocalStreamReadCount::Data(count) => return Ok(count), + crate::ipc::LocalStreamReadCount::Closed => return Ok(0), + crate::ipc::LocalStreamReadCount::Pending => { + std::thread::sleep(IO_POLL_INTERVAL); + } + } + } + } + } + let (stream, peer, path) = streams(); + peer.set_nonblocking(true).unwrap(); + let mut peer = PollingPeer(peer); let mut transport = NativeEndpointTransport::with_lifetime(stream, ()).unwrap(); let (done, received) = mpsc::channel(); let reader = std::thread::spawn(move || { From d184b41fa36923c132629af725ff98bb02aa1b61 Mon Sep 17 00:00:00 2001 From: akbash Date: Sat, 12 Sep 2026 19:19:33 +0300 Subject: [PATCH 09/16] fix: batch queued endpoint input frames (#4004) refs #3833 Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com> --- src/client/endpoint/writer.rs | 213 +++++++++++++++++++++++++++++----- 1 file changed, 185 insertions(+), 28 deletions(-) diff --git a/src/client/endpoint/writer.rs b/src/client/endpoint/writer.rs index c4e9492312..56c472cd3b 100644 --- a/src/client/endpoint/writer.rs +++ b/src/client/endpoint/writer.rs @@ -9,20 +9,30 @@ use super::EndpointTransport; use crate::ipc::LocalStream; use crate::protocol::ClientMessage; -const MAX_QUEUED_MESSAGES: usize = 256; +const MAX_QUEUED_BATCHES: usize = 256; +const MAX_BATCH_BYTES: usize = 64 * 1024; const MAX_QUEUED_BYTES: usize = 2 * crate::protocol::MAX_GRAPHICS_FRAME_SIZE; const WRITE_TIMEOUT: Duration = Duration::from_secs(5); const IO_POLL_INTERVAL: Duration = Duration::from_millis(2); +#[derive(Default)] +struct FrameBatch { + frames: Vec>, + bytes: usize, +} + enum WriterCommand { - Frame(Vec), + Frames(Arc>), Flush(mpsc::Sender<()>), } -/// The UI only enqueues complete frames. A worker owns partial writes, cancellation, and the -/// bridge lifetime, so neither socket backpressure nor bridge teardown can block other endpoints. +/// The UI batches complete frames until the worker claims them, so a short burst of tiny input +/// frames does not exhaust command slots. Frames retain their individual write boundaries. +/// A worker owns partial writes, cancellation, and the bridge lifetime; socket backpressure and +/// bridge teardown never block other endpoints. pub(crate) struct NativeEndpointTransport { sender: mpsc::SyncSender, + pending_batch: Option>>, queued_bytes: Arc, stopped: Arc, error: Arc>>, @@ -34,7 +44,7 @@ impl NativeEndpointTransport { lifetime: impl Send + 'static, ) -> io::Result { stream.set_nonblocking(true)?; - let (sender, receiver) = mpsc::sync_channel::(MAX_QUEUED_MESSAGES); + let (sender, receiver) = mpsc::sync_channel::(MAX_QUEUED_BATCHES); let queued_bytes = Arc::new(AtomicUsize::new(0)); let stopped = Arc::new(AtomicBool::new(false)); let error = Arc::new(Mutex::new(None)); @@ -49,15 +59,14 @@ impl NativeEndpointTransport { if worker_stop.load(Ordering::Acquire) { break; } - let frame = match command { - WriterCommand::Frame(frame) => frame, + let batch = match command { + WriterCommand::Frames(batch) => batch, WriterCommand::Flush(done) => { let _ = done.send(()); continue; } }; - let result = write_frame(&mut stream, &frame, &worker_stop); - worker_bytes.fetch_sub(frame.len(), Ordering::AcqRel); + let result = write_batch(&mut stream, &batch, &worker_stop, &worker_bytes); if let Err(error) = result { if let Ok(mut slot) = worker_error.lock() { *slot = Some(error); @@ -69,12 +78,43 @@ impl NativeEndpointTransport { })?; Ok(Self { sender, + pending_batch: None, queued_bytes, stopped, error, }) } + fn enqueue_frame(&mut self, frame: Vec) -> io::Result<()> { + if let Some(batch) = &self.pending_batch { + let mut batch = batch + .lock() + .map_err(|_| io::Error::other("endpoint batch lock poisoned"))?; + // An empty batch has already been claimed by the worker. Never append to it. + if !batch.frames.is_empty() + && frame.len() <= MAX_BATCH_BYTES.saturating_sub(batch.bytes) + { + batch.bytes += frame.len(); + batch.frames.push(frame); + return Ok(()); + } + } + let batch = Arc::new(Mutex::new(FrameBatch { + bytes: frame.len(), + frames: vec![frame], + })); + self.sender + .try_send(WriterCommand::Frames(batch.clone())) + .map_err(|error| match error { + mpsc::TrySendError::Full(_) => queue_full(), + mpsc::TrySendError::Disconnected(_) => { + io::Error::new(io::ErrorKind::BrokenPipe, "endpoint writer stopped") + } + })?; + self.pending_batch = Some(batch); + Ok(()) + } + pub(crate) fn stop_handle(&self) -> Arc { self.stopped.clone() } @@ -103,17 +143,9 @@ impl EndpointTransport for NativeEndpointTransport { { return Err(queue_full()); } - self.sender - .try_send(WriterCommand::Frame(frame)) - .map_err(|error| { - self.queued_bytes.fetch_sub(len, Ordering::AcqRel); - match error { - mpsc::TrySendError::Full(_) => queue_full(), - mpsc::TrySendError::Disconnected(_) => { - io::Error::new(io::ErrorKind::BrokenPipe, "endpoint writer stopped") - } - } - }) + self.enqueue_frame(frame).inspect_err(|_| { + self.queued_bytes.fetch_sub(len, Ordering::AcqRel); + }) } fn disconnect(&mut self) { @@ -121,6 +153,8 @@ impl EndpointTransport for NativeEndpointTransport { } fn flush(&mut self, deadline: Instant) -> io::Result<()> { + // Later frames must stay after the flush command, even if its wait times out. + self.pending_batch = None; let (done, completion) = mpsc::channel(); self.sender .try_send(WriterCommand::Flush(done)) @@ -157,6 +191,29 @@ fn queue_full() -> io::Error { ) } +fn write_batch( + writer: &mut impl io::Write, + batch: &Mutex, + stopped: &AtomicBool, + queued_bytes: &AtomicUsize, +) -> io::Result<()> { + // Claim the frames before doing any I/O. The producer never waits for socket progress. + let batch = std::mem::take( + &mut *batch + .lock() + .map_err(|_| io::Error::other("endpoint batch lock poisoned"))?, + ); + for frame in batch.frames { + if stopped.load(Ordering::Acquire) { + break; + } + let result = write_frame(writer, &frame, stopped); + queued_bytes.fetch_sub(frame.len(), Ordering::AcqRel); + result?; + } + Ok(()) +} + fn write_frame( writer: &mut impl io::Write, mut frame: &[u8], @@ -404,20 +461,120 @@ mod tests { worker.join().unwrap(); } + fn queued_transport( + capacity: usize, + ) -> (NativeEndpointTransport, mpsc::Receiver) { + let (sender, receiver) = mpsc::sync_channel(capacity); + ( + NativeEndpointTransport { + sender, + pending_batch: None, + queued_bytes: Arc::new(AtomicUsize::new(0)), + stopped: Arc::new(AtomicBool::new(false)), + error: Arc::new(Mutex::new(None)), + }, + receiver, + ) + } + #[test] - fn a_full_queue_is_a_connection_failure_not_silent_input_loss() { - let (sender, _receiver) = mpsc::sync_channel(1); - let mut transport = NativeEndpointTransport { - sender, - queued_bytes: Arc::new(AtomicUsize::new(0)), - stopped: Arc::new(AtomicBool::new(false)), - error: Arc::new(Mutex::new(None)), - }; + fn stdin_burst_is_queued_in_order_without_worker_progress() { + let (mut transport, receiver) = queued_transport(MAX_QUEUED_BATCHES); + let input = (0..128) + .map(|index| format!("{index:04}: ordered input burst\n")) + .collect::(); + let mut framer = crate::raw_input::RawInputByteFramer::for_host_input(); + let mut expected = Vec::new(); + for data in framer.push(input.as_bytes()) { + let message = ClientMessage::Input { data }; + crate::protocol::write_message(&mut expected, &message).unwrap(); + transport + .send(&message) + .expect("a small stdin burst must fit"); + } + let queued_bytes = transport.queued_bytes.clone(); + assert_eq!(queued_bytes.load(Ordering::Acquire), expected.len()); + let mut received = Vec::new(); + for command in receiver.try_iter() { + let WriterCommand::Frames(batch) = command else { + panic!("unexpected flush"); + }; + assert!(batch.lock().unwrap().bytes <= MAX_BATCH_BYTES); + write_batch(&mut received, &batch, &transport.stopped, &queued_bytes).unwrap(); + } + assert_eq!(received, expected); + assert_eq!(queued_bytes.load(Ordering::Acquire), 0); + + // The producer still holds the last claimed batch; new input must get a new command. transport.send(&ClientMessage::Detach).unwrap(); + let WriterCommand::Frames(batch) = receiver.try_recv().unwrap() else { + panic!("expected a new batch after the worker claimed the previous one"); + }; + write_batch(&mut received, &batch, &transport.stopped, &queued_bytes).unwrap(); + crate::protocol::write_message(&mut expected, &ClientMessage::Detach).unwrap(); + assert_eq!(received, expected); + assert_eq!(queued_bytes.load(Ordering::Acquire), 0); + } + + #[test] + fn later_frames_cannot_join_a_batch_before_a_flush() { + let (mut transport, receiver) = queued_transport(3); + let first = ClientMessage::ClientShellFocus { focused: true }; + let last = ClientMessage::Detach; + transport.send(&first).unwrap(); + assert_eq!( + transport.flush(Instant::now()).unwrap_err().kind(), + io::ErrorKind::TimedOut + ); + transport.send(&last).unwrap(); + + let mut received = Vec::new(); + let WriterCommand::Frames(batch) = receiver.try_recv().unwrap() else { + panic!("expected first batch"); + }; + write_batch( + &mut received, + &batch, + &transport.stopped, + &transport.queued_bytes, + ) + .unwrap(); + let mut expected = Vec::new(); + crate::protocol::write_message(&mut expected, &first).unwrap(); + assert_eq!(received, expected); + assert!(matches!( + receiver.try_recv().unwrap(), + WriterCommand::Flush(_) + )); + let WriterCommand::Frames(batch) = receiver.try_recv().unwrap() else { + panic!("expected a separate batch after flush"); + }; + write_batch( + &mut received, + &batch, + &transport.stopped, + &transport.queued_bytes, + ) + .unwrap(); + crate::protocol::write_message(&mut expected, &last).unwrap(); + assert_eq!(received, expected); + assert_eq!(transport.queued_bytes.load(Ordering::Acquire), 0); + } + + #[test] + fn a_full_queue_is_a_connection_failure_not_silent_input_loss() { + let (mut transport, _receiver) = queued_transport(1); + transport + .send(&ClientMessage::Input { + data: vec![b'x'; MAX_BATCH_BYTES], + }) + .unwrap(); + let queued = transport.queued_bytes.load(Ordering::Acquire); assert_eq!( transport.send(&ClientMessage::Detach).unwrap_err().kind(), io::ErrorKind::ConnectionAborted ); + assert_eq!(transport.queued_bytes.load(Ordering::Acquire), queued); transport .queued_bytes .store(MAX_QUEUED_BYTES, Ordering::Release); From 96f232446743d603c0184f62ee8817ecfa27191b Mon Sep 17 00:00:00 2001 From: akbash Date: Sat, 12 Sep 2026 22:55:19 +0300 Subject: [PATCH 10/16] fix: allow retrying partial worktree removal (#3315) refs #3314 Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com> --- .../tests/agents_worktrees_notifications.rs | 170 +++++++++++------- src/client/shell/worktrees.rs | 4 +- 2 files changed, 109 insertions(+), 65 deletions(-) diff --git a/src/client/shell/tests/agents_worktrees_notifications.rs b/src/client/shell/tests/agents_worktrees_notifications.rs index 8b4cc8bd37..fb47ab8730 100644 --- a/src/client/shell/tests/agents_worktrees_notifications.rs +++ b/src/client/shell/tests/agents_worktrees_notifications.rs @@ -1073,70 +1073,112 @@ fn worktree_open_filters_and_clicks_a_stable_public_entry() { } #[test] -fn worktree_remove_escalates_dirty_failure_to_force_confirmation() { - let mut snapshot = snapshot(); - snapshot.workspaces[0].worktree = Some(ClientShellWorktree { - key: "repo-key".into(), - label: "repo".into(), - is_linked_worktree: true, - }); - let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default())); - state.set_snapshot(Box::new(snapshot)); - state.set_pane_surface(surface()); - let mut prepare = ClientShellInput::default(); - state.record_binding( - crate::input::KeybindMatch::Action(crate::input::KeybindAction::RemoveWorktree), - &mut prepare, - ); - let [ClientShellAction::Endpoint { request, .. }] = &prepare.actions[..] else { - panic!("remove worktree should prepare through worktree.list"); - }; - let request_id = request.id.clone(); - state.handle_endpoint_result( - "boot-1", - &request_id, - Ok(worktree_list_result(Some("ws_1"))), - ); - let remove = state.handle_input_bytes(b"\r"); - let [ClientShellAction::Endpoint { request, .. }] = &remove.actions[..] else { - panic!("worktree remove should use endpoint API"); - }; - assert!(matches!( - &request.method, - crate::api::schema::Method::WorktreeRemove(params) - if params.workspace_id == "ws_1" && !params.force - )); - let request_id = request.id.clone(); - state.handle_endpoint_result( - "boot-1", - &request_id, - Err(ClientShellEndpointError { - code: Some("dirty_worktree_requires_force".into()), - message: "dirty worktree".into(), - }), - ); - let frame = state.compose(106, 30).expect("force remove modal"); - let text = frame - .cells - .chunks(frame.width as usize) - .map(|row| { - row.iter() - .map(|cell| cell.symbol.as_str()) - .collect::() - }) - .collect::>() - .join("\n"); - assert!(text.contains("delete anyway")); - assert!(text.contains("permanently deleted")); - let force = state.handle_input_bytes(b"\r"); - let [ClientShellAction::Endpoint { request, .. }] = &force.actions[..] else { - panic!("forced worktree remove should use endpoint API"); - }; - assert!(matches!( - &request.method, - crate::api::schema::Method::WorktreeRemove(params) - if params.workspace_id == "ws_1" && params.force - )); +fn worktree_remove_escalates_recoverable_failure_to_force_confirmation() { + for (code, message, expect_force) in [ + ("dirty_worktree_requires_force", "dirty worktree", true), + ( + "worktree_remove_failed", + "fatal: '/repo-feature' is not a working tree", + true, + ), + ("worktree_remove_failed", "Permission denied", false), + ("server_unavailable", "is not a working tree", false), + ] { + let mut snapshot = snapshot(); + snapshot.workspaces[0].worktree = Some(ClientShellWorktree { + key: "repo-key".into(), + label: "repo".into(), + is_linked_worktree: true, + }); + let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default())); + state.set_snapshot(Box::new(snapshot)); + state.set_pane_surface(surface()); + let mut prepare = ClientShellInput::default(); + state.record_binding( + crate::input::KeybindMatch::Action(crate::input::KeybindAction::RemoveWorktree), + &mut prepare, + ); + let [ClientShellAction::Endpoint { request, .. }] = &prepare.actions[..] else { + panic!("remove worktree should prepare through worktree.list"); + }; + let request_id = request.id.clone(); + state.handle_endpoint_result( + "boot-1", + &request_id, + Ok(worktree_list_result(Some("ws_1"))), + ); + let remove = state.handle_input_bytes(b"\r"); + let [ClientShellAction::Endpoint { request, .. }] = &remove.actions[..] else { + panic!("worktree remove should use endpoint API"); + }; + assert!(matches!( + &request.method, + crate::api::schema::Method::WorktreeRemove(params) + if params.workspace_id == "ws_1" && !params.force + )); + let request_id = request.id.clone(); + let (_, actions) = state.handle_endpoint_result( + "boot-1", + &request_id, + Err(ClientShellEndpointError { + code: Some(code.into()), + message: message.into(), + }), + ); + assert!(actions.is_empty(), "failure must not retry automatically"); + let Some(ClientShellOverlay::WorktreeRemove(remove)) = &state.overlay else { + panic!("failed remove should keep its confirmation"); + }; + assert!(!remove.removing); + assert_eq!(remove.force_confirmation, expect_force); + if !expect_force { + assert_eq!(remove.error.as_deref(), Some(message)); + continue; + } + assert!(remove.error.is_none()); + let frame = state.compose(106, 30).expect("force remove modal"); + let text = frame + .cells + .chunks(frame.width as usize) + .map(|row| { + row.iter() + .map(|cell| cell.symbol.as_str()) + .collect::() + }) + .collect::>() + .join("\n"); + assert!(text.contains("delete anyway")); + assert!(text.contains("permanently deleted")); + let force = state.handle_input_bytes(b"\r"); + let [ClientShellAction::Endpoint { request, .. }] = &force.actions[..] else { + panic!("forced worktree remove should use endpoint API"); + }; + assert!(matches!( + &request.method, + crate::api::schema::Method::WorktreeRemove(params) + if params.workspace_id == "ws_1" && params.force + )); + let request_id = request.id.clone(); + let (_, actions) = state.handle_endpoint_result( + "boot-1", + &request_id, + Err(ClientShellEndpointError { + code: Some("worktree_remove_failed".into()), + message: "fatal: '/repo-feature' is not a working tree".into(), + }), + ); + assert!(actions.is_empty()); + let Some(ClientShellOverlay::WorktreeRemove(remove)) = &state.overlay else { + panic!("forced failure should keep its confirmation"); + }; + assert!(!remove.removing); + assert_eq!( + remove.error.as_deref(), + Some("fatal: '/repo-feature' is not a working tree") + ); + state.handle_input_bytes(b"\x1b"); + assert!(state.overlay.is_none()); + } } #[test] diff --git a/src/client/shell/worktrees.rs b/src/client/shell/worktrees.rs index bf92355700..c50b86bed2 100644 --- a/src/client/shell/worktrees.rs +++ b/src/client/shell/worktrees.rs @@ -518,7 +518,9 @@ impl ClientShellState { true } (PendingEndpointKind::WorktreeRemove { forced: false }, Err(error)) - if error.code.as_deref() == Some("dirty_worktree_requires_force") => + if error.code.as_deref() == Some("dirty_worktree_requires_force") + || (error.code.as_deref() == Some("worktree_remove_failed") + && crate::worktree::is_not_working_tree_remove_error(&error.message)) => { if let Some(ClientShellOverlay::WorktreeRemove(remove)) = self.overlay.as_mut() { remove.removing = false; From e3a46f4f89142d929bf35c31aac85e5f374d8dcf Mon Sep 17 00:00:00 2001 From: Can Celik Date: Sun, 13 Sep 2026 01:36:59 +0300 Subject: [PATCH 11/16] fix: recognize codex composer sparkles (#4015) refs #3988 --- distribution/agent-detection/codex.toml | 6 ++-- src/detect/manifest/tests.rs | 40 +++++++++++++++++++++++++ src/detect/manifests/codex.toml | 6 ++-- 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/distribution/agent-detection/codex.toml b/distribution/agent-detection/codex.toml index 884492c41a..7840ca6fdf 100644 --- a/distribution/agent-detection/codex.toml +++ b/distribution/agent-detection/codex.toml @@ -1,7 +1,7 @@ id = "codex" -version = "2026.09.05.1" +version = "2026.09.12.1" min_engine_version = 3 -updated_at = "2026-09-05T00:00:00Z" +updated_at = "2026-09-12T00:00:00Z" [[rules]] id = "osc_title_blocked" @@ -69,6 +69,8 @@ id = "weak_blocker" state = "blocked" priority = 600 region = "whole_recent_without_current_prompt_marker" +# Sparkles can replace the space after ›. A later response marker makes that prompt stale. +not = [{ regex = ['(?m)^›[⠁⠂⠄⠈⠐⠠⡀⢀][^\n]*(?:\n(?:[^•■✗✓\n][^\n]*)?)*\z'] }] any = [ { contains = ["[y/n]"] }, { contains = ["yes (y)"] }, diff --git a/src/detect/manifest/tests.rs b/src/detect/manifest/tests.rs index e1073f3c6a..c55229522d 100644 --- a/src/detect/manifest/tests.rs +++ b/src/detect/manifest/tests.rs @@ -1194,6 +1194,46 @@ fn codex_weak_blocker_ignores_wrapped_current_prompt_text() { ); } +#[test] +fn codex_sparkle_prompt_preserves_live_states() { + for marker in ["› ", "›⠁", "›⠂", "›⠄", "›⠈", "›⠐", "›⠠", "›⡀", "›⢀"] + { + let screen = format!("Do you want to proceed? [y/n]\n{marker}unsent draft\n"); + let result = osc_explain(Agent::Codex, &screen, "project | Ready", ""); + assert_eq!(result.state, AgentState::Idle, "{marker}"); + + let working = format!( + "Do you want to proceed? [y/n]\n• Working (4s • esc to interrupt)\n{marker}draft\n" + ); + let result = osc_explain(Agent::Codex, &working, "project", ""); + assert_eq!(result.state, AgentState::Working, "{marker}"); + + let approval = format!("{screen}Press enter to confirm or esc to cancel\n"); + let result = osc_explain(Agent::Codex, &approval, "project", ""); + assert_eq!(result.state, AgentState::Blocked, "{marker}"); + assert!(result.visible_blocker); + + for response_marker in ['•', '■', '✗', '✓'] { + let response = format!("{screen}{response_marker} Do you want to proceed? [y/n]\n"); + let result = osc_explain(Agent::Codex, &response, "project", ""); + assert_eq!( + result.state, + AgentState::Blocked, + "{marker} {response_marker}" + ); + } + } +} + +#[test] +fn codex_weak_blocker_does_not_ignore_arbitrary_prompt_suffixes() { + for line in ["›text", "›⠋draft", "›⠀draft", " ›⠁draft", "quoted ›⠁draft"] { + let screen = format!("Do you want to proceed? [y/n]\n{line}\n"); + let result = osc_explain(Agent::Codex, &screen, "project", ""); + assert_eq!(result.state, AgentState::Blocked, "{line}"); + } +} + #[test] fn codex_transcript_viewer_outranks_working_fallback() { let screen = "• Working (4s • esc to interrupt)\n\ diff --git a/src/detect/manifests/codex.toml b/src/detect/manifests/codex.toml index 884492c41a..7840ca6fdf 100644 --- a/src/detect/manifests/codex.toml +++ b/src/detect/manifests/codex.toml @@ -1,7 +1,7 @@ id = "codex" -version = "2026.09.05.1" +version = "2026.09.12.1" min_engine_version = 3 -updated_at = "2026-09-05T00:00:00Z" +updated_at = "2026-09-12T00:00:00Z" [[rules]] id = "osc_title_blocked" @@ -69,6 +69,8 @@ id = "weak_blocker" state = "blocked" priority = 600 region = "whole_recent_without_current_prompt_marker" +# Sparkles can replace the space after ›. A later response marker makes that prompt stale. +not = [{ regex = ['(?m)^›[⠁⠂⠄⠈⠐⠠⡀⢀][^\n]*(?:\n(?:[^•■✗✓\n][^\n]*)?)*\z'] }] any = [ { contains = ["[y/n]"] }, { contains = ["yes (y)"] }, From 5eafc612674c4ac8f7c938769159a3cfd22e52d4 Mon Sep 17 00:00:00 2001 From: akbash Date: Sun, 13 Sep 2026 02:20:23 +0300 Subject: [PATCH 12/16] fix: honor space row gaps across machines (#3740) * fix: honor space row gaps across machines refs #3738 * test: cover scrolling with machine space gaps refs #3738 --------- Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com> --- src/client/shell/endpoint_sidebar.rs | 25 +++++- src/client/shell/tests/endpoints.rs | 109 +++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 4 deletions(-) diff --git a/src/client/shell/endpoint_sidebar.rs b/src/client/shell/endpoint_sidebar.rs index 193d75d1fc..bce484a352 100644 --- a/src/client/shell/endpoint_sidebar.rs +++ b/src/client/shell/endpoint_sidebar.rs @@ -314,7 +314,20 @@ pub(super) fn render_expanded( } }) .collect::>(); - let gaps = vec![0; rows.len()]; + let gaps = rows + .iter() + .enumerate() + .map(|(index, row)| match (row, rows.get(index + 1)) { + ( + Row::Workspace { endpoint, .. }, + Some(Row::Workspace { + endpoint: next_endpoint, + entry, + }), + ) if endpoint == next_endpoint => u16::from(!entry.indented) * config.spaces.row_gap, + _ => 0, + }) + .collect::>(); if std::mem::take(state.reveal_navigation_workspace) { let selected_row = rows.iter().position(|row| match row { Row::Workspace { endpoint, entry } => { @@ -355,7 +368,7 @@ pub(super) fn render_expanded( let show_scrollbar = metrics.max_offset_from_bottom > 0 && body.width > 1; let content_width = body.width.saturating_sub(u16::from(show_scrollbar)); let mut y = body.y; - for row in rows.iter().skip(*state.workspace_scroll) { + for (row_index, row) in rows.iter().enumerate().skip(*state.workspace_scroll) { match row { Row::Endpoint(index) => { if y >= body.bottom() { @@ -383,7 +396,9 @@ pub(super) fn render_expanded( ), endpoint_id: endpoint.endpoint_id.clone(), }); - y = y.saturating_add(1); + y = y + .saturating_add(1) + .saturating_add(gaps.get(row_index).copied().unwrap_or(0)); } Row::Workspace { endpoint, entry } => { let endpoint = &state.endpoints[*endpoint]; @@ -460,7 +475,9 @@ pub(super) fn render_expanded( indented: entry.indented, group_toggle, }); - y = y.saturating_add(height); + y = y + .saturating_add(height) + .saturating_add(gaps.get(row_index).copied().unwrap_or(0)); } } } diff --git a/src/client/shell/tests/endpoints.rs b/src/client/shell/tests/endpoints.rs index a3a8097958..2e5e87acbb 100644 --- a/src/client/shell/tests/endpoints.rs +++ b/src/client/shell/tests/endpoints.rs @@ -437,6 +437,115 @@ fn saved_machine_preserves_endpoint_scoped_worktree_collapses() { .any(|hit| { hit.endpoint_id == remote_id && hit.workspace_id == "remote_ws_2" })); } +#[test] +fn expanded_machine_sidebar_applies_space_row_gap_within_each_machine() { + let (mut state, remote_id) = state_with_remote(); + state.config.spaces.row_gap = 1; + + let add_second_workspace = |snapshot: &mut ClientShellSnapshot| { + let mut workspace = snapshot.workspaces[0].clone(); + workspace.workspace_id = "ws_2".into(); + workspace.active_tab_id = "tab_2".into(); + workspace.number = 2; + workspace.label = "second-workspace".into(); + workspace.focused = false; + snapshot.workspaces.push(workspace); + }; + let mut local = snapshot(); + add_second_workspace(&mut local); + state.set_snapshot(Box::new(local)); + let mut remote = snapshot(); + remote.boot_id = "remote-boot".into(); + remote.workspaces[0].worktree = Some(ClientShellWorktree { + key: "repo".into(), + label: "repo".into(), + is_linked_worktree: false, + }); + add_second_workspace(&mut remote); + remote.workspaces[1].worktree = Some(ClientShellWorktree { + key: "repo".into(), + label: "repo".into(), + is_linked_worktree: true, + }); + let mut third = remote.workspaces[1].clone(); + third.workspace_id = "ws_3".into(); + third.number = 3; + third.label = "third-workspace".into(); + third.worktree = None; + remote.workspaces.push(third); + state.set_endpoint_snapshot(&remote_id, Box::new(remote)); + + state.compose(100, 40).expect("combined endpoint frame"); + let local_workspaces = state + .hits + .workspaces + .iter() + .filter(|hit| hit.endpoint_id.is_local()) + .collect::>(); + assert_eq!(local_workspaces.len(), 2); + assert_eq!( + local_workspaces[1].rect.y, + local_workspaces[0].rect.bottom() + 1 + ); + + let local_machine = state + .hits + .machines + .iter() + .find(|hit| hit.endpoint_id.is_local()) + .expect("local machine"); + let remote_machine = state + .hits + .machines + .iter() + .find(|hit| hit.endpoint_id == remote_id) + .expect("remote machine"); + assert_eq!(local_workspaces[0].rect.y, local_machine.rect.bottom()); + assert_eq!(remote_machine.rect.y, local_workspaces[1].rect.bottom()); + + let remote_workspaces = state + .hits + .workspaces + .iter() + .filter(|hit| hit.endpoint_id == remote_id) + .collect::>(); + assert_eq!(remote_workspaces.len(), 3); + assert_eq!(remote_workspaces[0].rect.y, remote_machine.rect.bottom()); + assert_eq!( + remote_workspaces[1].rect.y, + remote_workspaces[0].rect.bottom() + ); + assert_eq!( + remote_workspaces[2].rect.y, + remote_workspaces[1].rect.bottom() + 1 + ); + + state.workspace_scroll = usize::MAX; + state.compose(100, 18).expect("scrolled endpoint frame"); + let metrics = state + .hits + .workspace_scroll_metrics + .expect("workspace scroll metrics"); + assert!(metrics.max_offset_from_bottom > 0); + assert_eq!(metrics.offset_from_bottom, 0); + assert_eq!(state.workspace_scroll, metrics.max_offset_from_bottom); + let visible_remote = state + .hits + .workspaces + .iter() + .filter(|hit| hit.endpoint_id == remote_id) + .collect::>(); + assert_eq!(visible_remote.len(), 3); + let gap_y = visible_remote[1].rect.bottom(); + assert_eq!(visible_remote[2].rect.y, gap_y + 1); + assert!(visible_remote[2].rect.bottom() <= state.hits.workspace_body.bottom()); + assert!(state + .hits + .workspaces + .iter() + .all(|hit| gap_y < hit.rect.top() || gap_y >= hit.rect.bottom())); +} + #[test] fn active_workspace_is_the_only_highlight_when_machine_is_expanded() { let (mut state, endpoint_id) = state_with_remote(); From 98a8c6ce52a054b582a6210c79d76e10a99ccccc Mon Sep 17 00:00:00 2001 From: Can Celik Date: Sun, 13 Sep 2026 02:40:13 +0300 Subject: [PATCH 13/16] fix: restore alternate-screen pane width (#4016) refs #3329 --- src/pane/terminal.rs | 17 ++++++++++ src/server/headless/render.rs | 55 ++++++++++++++++++++++++++++++++ src/server/headless/tests/mod.rs | 28 ++++++++++++++++ 3 files changed, 100 insertions(+) diff --git a/src/pane/terminal.rs b/src/pane/terminal.rs index b201702b77..dfd4f5369d 100644 --- a/src/pane/terminal.rs +++ b/src/pane/terminal.rs @@ -5606,6 +5606,23 @@ mod tests { } } + #[test] + fn enabling_in_band_size_reports_after_alt_screen_resize_reports_current_size() { + let (tx, _rx) = mpsc::channel(4); + let terminal = crate::ghostty::Terminal::new(91, 24, 0).unwrap(); + let pane = GhosttyPaneTerminal::new(terminal, tx.clone()).unwrap(); + let pane_id = PaneId::from_raw(1); + pane.process_pty_bytes(pane_id, 0, b"\x1b[?1049h", &tx); + assert!(pane.resize(24, 92, 9, 18).is_empty()); + + let result = pane.process_pty_bytes(pane_id, 0, b"\x1b[?2048h", &tx); + + assert_eq!( + result.terminal_responses, + vec![Bytes::from_static(b"\x1b[48;24;92;432;828t")] + ); + } + #[test] fn resize_returns_in_band_size_report_response() { let (tx, _rx) = mpsc::channel(4); diff --git a/src/server/headless/render.rs b/src/server/headless/render.rs index 5e36be0119..fc55417f18 100644 --- a/src/server/headless/render.rs +++ b/src/server/headless/render.rs @@ -405,6 +405,61 @@ impl HeadlessServer { return; } + // Resize from the controlling client's geometry before drawing any observer. + // Retained updates fall back here when a pane changes alternate screens. + for (client_id, (cols, rows), cell_size, _, _) in &render_targets { + let Some(client) = self.clients.get(client_id) else { + continue; + }; + if !client.is_active_shell_client() { + continue; + } + let Some(tab_id) = self.shell_tab_id_for_client(*client_id) else { + continue; + }; + if self.tab_geometry_controllers.get(&tab_id) != Some(client_id) { + continue; + } + let changed = client + .render_state + .last_pane_surface() + .is_none_or(|surface| { + surface.panes.iter().any(|pane| { + let Some((workspace_index, pane_id)) = + self.app.parse_pane_id(&pane.pane_id) + else { + return false; + }; + self.app + .state + .runtime_for_pane_in_workspace( + &self.app.terminal_runtimes, + workspace_index, + pane_id, + ) + .is_some_and(|runtime| { + runtime.alternate_screen_active() != pane.alternate_screen_active + }) + }) + }); + if changed { + if let Some(target) = self.shell_target_for_client(*client_id) { + crate::ui::resize_tab_surface( + &self.app.state, + &self.app.terminal_runtimes, + target.workspace_index, + target.tab_index, + Rect::new(0, 0, *cols, *rows), + if cell_size.is_known() { + *cell_size + } else { + crate::kitty_graphics::HostCellSize::default() + }, + ); + } + } + } + let mut broken_clients: Vec = Vec::new(); for (client_id, (cols, rows), cell_size, _is_foreground, mode) in render_targets { let area = Rect::new(0, 0, cols, rows); diff --git a/src/server/headless/tests/mod.rs b/src/server/headless/tests/mod.rs index d666ae2248..2c13dc8b03 100644 --- a/src/server/headless/tests/mod.rs +++ b/src/server/headless/tests/mod.rs @@ -1056,6 +1056,25 @@ async fn retained_snapshot_survives_a_writer_waiting_for_the_terminal_core() { shutdown_test_runtimes(&mut server); } +#[tokio::test] +async fn first_shell_surface_resizes_a_pane_that_entered_alternate_screen() { + let mut server = test_headless_server(); + let pane_id = install_shared_view_test_runtime(&mut server); + let (_control, render) = connect_test_shell(&mut server, 7, 80, 23); + let initial_size = server.app.state.workspaces[0].test_runtimes[&pane_id].current_size(); + + write_shared_test_pane(&mut server, pane_id, b"\x1b[?1049hALT"); + server.render_and_stream(); + + let surface = recv_pane_surface(&render, "first alternate-screen surface"); + assert!(surface.panes[0].alternate_screen_active); + assert_eq!( + server.app.state.workspaces[0].test_runtimes[&pane_id].current_size(), + (initial_size.0, initial_size.1 + 1) + ); + shutdown_test_runtimes(&mut server); +} + #[tokio::test] async fn different_size_shells_receive_geometry_specific_patches_from_one_dirty_collection() { let mut server = test_headless_server(); @@ -1067,6 +1086,7 @@ async fn different_size_shells_receive_geometry_specific_patches_from_one_dirty_ server.render_and_stream(); let large_initial = recv_pane_surface(&large_render, "large initial surface"); let small_initial = recv_pane_surface(&small_render, "small initial surface"); + let initial_size = server.app.state.workspaces[0].test_runtimes[&pane_id].current_size(); assert_eq!( (large_initial.frame.width, large_initial.frame.height), (80, 23) @@ -1134,6 +1154,10 @@ async fn different_size_shells_receive_geometry_specific_patches_from_one_dirty_ let small_alt = recv_pane_surface(&small_render, "small alternate-screen surface"); assert!(large_alt.panes[0].alternate_screen_active); assert!(small_alt.panes[0].alternate_screen_active); + assert_eq!( + server.app.state.workspaces[0].test_runtimes[&pane_id].current_size(), + (initial_size.0, initial_size.1 + 1) + ); assert_eq!( large_alt.panes[0].inner_rect.width, large_initial.panes[0].inner_rect.width + 1 @@ -1150,6 +1174,10 @@ async fn different_size_shells_receive_geometry_specific_patches_from_one_dirty_ let small_main = recv_pane_surface(&small_render, "small restored main-screen surface"); assert!(!large_main.panes[0].alternate_screen_active); assert!(!small_main.panes[0].alternate_screen_active); + assert_eq!( + server.app.state.workspaces[0].test_runtimes[&pane_id].current_size(), + initial_size + ); assert_eq!( large_main.panes[0].inner_rect, large_initial.panes[0].inner_rect From aa6b531a423067d8a71c2a57b41185bff17036be Mon Sep 17 00:00:00 2001 From: akbash Date: Sun, 13 Sep 2026 03:04:27 +0300 Subject: [PATCH 14/16] fix: update grok session after new (#2683) * fix: update grok session after new refs #2681 * fix: avoid powershell args shadowing refs #2681 --------- Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com> --- .../website/src/content/docs/integrations.mdx | 2 +- .../src/content/docs/ja/integrations.mdx | 2 +- .../src/content/docs/session-state.mdx | 2 +- .../src/content/docs/zh-cn/integrations.mdx | 2 +- .../assets/grok/herdr-agent-state.ps1 | 20 +++++- .../assets/grok/herdr-agent-state.sh | 20 +++--- src/integration/mod.rs | 2 +- src/integration/tests.rs | 35 +++++++++- src/terminal/state.rs | 65 ++++++++++++++++++- tests/cli/hooks.rs | 22 +++++++ 10 files changed, 155 insertions(+), 17 deletions(-) diff --git a/docs/next/website/src/content/docs/integrations.mdx b/docs/next/website/src/content/docs/integrations.mdx index 4cbd0a631b..e8a5a7f3bc 100644 --- a/docs/next/website/src/content/docs/integrations.mdx +++ b/docs/next/website/src/content/docs/integrations.mdx @@ -91,7 +91,7 @@ Use `HERDR_BIN_PATH` and the CLI wrappers for portable integrations. Code that n Some integrations report native agent session references. Herdr uses official session references to resume Claude Code, Codex, Devin CLI, Droid, Kimi Code CLI, Qoder CLI, Qwen Code, Cursor Agent CLI, Grok CLI, GitHub Copilot CLI, Pi, OMP, Hermes Agent, OpenCode, Kilo Code CLI, MastraCode, and Antigravity CLI panes after a Herdr server restart unless `[session] resume_agents_on_restore = false` disables it. -Native session restore requires current Herdr integrations: Pi integration version `2`, OMP version `3`, Claude Code version `6`, Codex version `5`, GitHub Copilot CLI version `2`, Devin CLI version `2`, Droid version `2`, Kimi Code CLI version `3`, Qoder CLI version `2`, Qwen Code version `1`, Cursor Agent CLI version `1`, Grok CLI version `1`, OpenCode version `5`, Kilo Code CLI version `1`, Hermes Agent version `5`, MastraCode version `1`, or Antigravity CLI version `1`. Check installed versions with `herdr integration status`. +Native session restore requires current Herdr integrations: Pi integration version `2`, OMP version `3`, Claude Code version `6`, Codex version `5`, GitHub Copilot CLI version `2`, Devin CLI version `2`, Droid version `2`, Kimi Code CLI version `3`, Qoder CLI version `2`, Qwen Code version `1`, Cursor Agent CLI version `1`, Grok CLI version `2`, OpenCode version `5`, Kilo Code CLI version `1`, Hermes Agent version `5`, MastraCode version `1`, or Antigravity CLI version `1`. Check installed versions with `herdr integration status`. ## Pi diff --git a/docs/next/website/src/content/docs/ja/integrations.mdx b/docs/next/website/src/content/docs/ja/integrations.mdx index 124010e177..0ff999335c 100644 --- a/docs/next/website/src/content/docs/ja/integrations.mdx +++ b/docs/next/website/src/content/docs/ja/integrations.mdx @@ -93,7 +93,7 @@ Herdr の外では何もしないように、`HERDR_ENV=1` で必要な変数が 一部のインテグレーションは、エージェントのネイティブセッション参照を報告します。Herdr は公式のセッション参照を使って、`[session] resume_agents_on_restore = false` で無効化されていない限り、Herdr サーバーの再起動後に Claude Code、Codex、Devin CLI、Droid、Kimi Code CLI、Qoder CLI、Qwen Code、Cursor Agent CLI、Grok CLI、GitHub Copilot CLI、Pi、OMP、Hermes Agent、OpenCode、Kilo Code CLI、MastraCode、Antigravity CLI のペインを resume します。 -エージェントネイティブのセッション復元には最新の Herdr インテグレーションが必要です: Pi インテグレーションはバージョン `2`、OMP は `3`、Claude Code は `6`、Codex は `5`、GitHub Copilot CLI は `2`、Devin CLI は `2`、Droid は `2`、Kimi Code CLI は `3`、Qoder CLI は `2`、Qwen Code は `1`、Cursor Agent CLI は `1`、Grok CLI は `1`、OpenCode は `5`、Kilo Code CLI は `1`、Hermes Agent は `5`、MastraCode は `1`、Antigravity CLI は `1` です。インストール済みバージョンは `herdr integration status` で確認してください。 +エージェントネイティブのセッション復元には最新の Herdr インテグレーションが必要です: Pi インテグレーションはバージョン `2`、OMP は `3`、Claude Code は `6`、Codex は `5`、GitHub Copilot CLI は `2`、Devin CLI は `2`、Droid は `2`、Kimi Code CLI は `3`、Qoder CLI は `2`、Qwen Code は `1`、Cursor Agent CLI は `1`、Grok CLI は `2`、OpenCode は `5`、Kilo Code CLI は `1`、Hermes Agent は `5`、MastraCode は `1`、Antigravity CLI は `1` です。インストール済みバージョンは `herdr integration status` で確認してください。 ## Pi diff --git a/docs/next/website/src/content/docs/session-state.mdx b/docs/next/website/src/content/docs/session-state.mdx index ed147193c0..e47e57a833 100644 --- a/docs/next/website/src/content/docs/session-state.mdx +++ b/docs/next/website/src/content/docs/session-state.mdx @@ -70,7 +70,7 @@ Native session restore requires these Herdr integration versions or newer: | Claude Code | `6` | `claude --resume ` | | Codex | `5` | `codex resume ` | | Cursor Agent CLI | `1` | `cursor-agent --resume ` | -| Grok CLI | `1` | `grok --resume ` | +| Grok CLI | `2` | `grok --resume ` | | GitHub Copilot CLI | `2` | `copilot --resume=` | | Devin CLI | `2` | `devin --resume ` | | Droid | `2` | `droid --resume ` | diff --git a/docs/next/website/src/content/docs/zh-cn/integrations.mdx b/docs/next/website/src/content/docs/zh-cn/integrations.mdx index e5535808a1..aaf84e5446 100644 --- a/docs/next/website/src/content/docs/zh-cn/integrations.mdx +++ b/docs/next/website/src/content/docs/zh-cn/integrations.mdx @@ -93,7 +93,7 @@ Herdr 以两种不同方式使用集成: 一些集成会上报智能体的原生会话引用。除非被 `[session] resume_agents_on_restore = false` 禁用,Herdr 会在服务器重启后使用官方会话引用恢复 Claude Code、Codex、Devin CLI、Droid、Kimi Code CLI、Qoder CLI、Qwen Code、Cursor Agent CLI、Grok CLI、GitHub Copilot CLI、Pi、OMP、Hermes Agent、OpenCode、Kilo Code CLI、MastraCode 和 Antigravity CLI 的窗格。 -原生会话恢复需要最新的 Herdr 集成: Pi 集成版本 `2`、OMP 版本 `3`、Claude Code 版本 `6`、Codex 版本 `5`、GitHub Copilot CLI 版本 `2`、Devin CLI 版本 `2`、Droid 版本 `2`、Kimi Code CLI 版本 `3`、Qoder CLI 版本 `2`、Qwen Code 版本 `1`、Cursor Agent CLI 版本 `1`、Grok CLI 版本 `1`、OpenCode 版本 `5`、Kilo Code CLI 版本 `1`、Hermes Agent 版本 `5`、MastraCode 版本 `1`、Antigravity CLI 版本 `1`。用 `herdr integration status` 查看已安装版本。 +原生会话恢复需要最新的 Herdr 集成: Pi 集成版本 `2`、OMP 版本 `3`、Claude Code 版本 `6`、Codex 版本 `5`、GitHub Copilot CLI 版本 `2`、Devin CLI 版本 `2`、Droid 版本 `2`、Kimi Code CLI 版本 `3`、Qoder CLI 版本 `2`、Qwen Code 版本 `1`、Cursor Agent CLI 版本 `1`、Grok CLI 版本 `2`、OpenCode 版本 `5`、Kilo Code CLI 版本 `1`、Hermes Agent 版本 `5`、MastraCode 版本 `1`、Antigravity CLI 版本 `1`。用 `herdr integration status` 查看已安装版本。 ## Pi diff --git a/src/integration/assets/grok/herdr-agent-state.ps1 b/src/integration/assets/grok/herdr-agent-state.ps1 index 69c4f51512..1b183a3b90 100644 --- a/src/integration/assets/grok/herdr-agent-state.ps1 +++ b/src/integration/assets/grok/herdr-agent-state.ps1 @@ -2,7 +2,7 @@ # managed by herdr; reinstalling or updating the integration overwrites this file. # add custom hooks beside this file instead of editing it. # HERDR_INTEGRATION_ID=grok -# HERDR_INTEGRATION_VERSION=1 +# HERDR_INTEGRATION_VERSION=2 param([string]$Action = "") @@ -26,6 +26,12 @@ $event = if ($null -ne $payload -and $payload.hook_event_name -is [string]) { } if ($null -ne $event -and $event -notin @("session_start", "SessionStart", "sessionStart")) { exit 0 } +$sessionStartSource = if ($null -ne $payload -and $payload.source -is [string]) { + $payload.source +} else { + $null +} + $sessionId = $env:GROK_SESSION_ID if ([string]::IsNullOrWhiteSpace($sessionId) -and $null -ne $payload) { if ($payload.session_id -is [string]) { $sessionId = $payload.session_id } @@ -35,7 +41,17 @@ if ([string]::IsNullOrWhiteSpace($sessionId)) { exit 0 } $seq = [DateTime]::UtcNow.Ticks $herdr = if ([string]::IsNullOrWhiteSpace($env:HERDR_BIN_PATH)) { "herdr" } else { $env:HERDR_BIN_PATH } +$herdrArgs = @( + "pane", "report-agent-session", $env:HERDR_PANE_ID, + "--source", "herdr:grok", + "--agent", "grok", + "--seq", "$seq", + "--agent-session-id", "$sessionId" +) +if (-not [string]::IsNullOrWhiteSpace($sessionStartSource)) { + $herdrArgs += @("--session-start-source", "$sessionStartSource") +} try { - & $herdr pane report-agent-session $env:HERDR_PANE_ID --source herdr:grok --agent grok --seq $seq --agent-session-id $sessionId 2>$null | Out-Null + & $herdr @herdrArgs 2>$null | Out-Null } catch { } diff --git a/src/integration/assets/grok/herdr-agent-state.sh b/src/integration/assets/grok/herdr-agent-state.sh index bdf0f6715b..3eaa3431fd 100644 --- a/src/integration/assets/grok/herdr-agent-state.sh +++ b/src/integration/assets/grok/herdr-agent-state.sh @@ -3,7 +3,7 @@ # managed by herdr; reinstalling or updating the integration overwrites this file. # add custom hooks beside this file instead of editing it. # HERDR_INTEGRATION_ID=grok -# HERDR_INTEGRATION_VERSION=1 +# HERDR_INTEGRATION_VERSION=2 set -eu @@ -62,6 +62,7 @@ def first_text(*keys): hook_event_name = first_text("hook_event_name", "hookEventName") if hook_event_name not in (None, "session_start", "SessionStart", "sessionStart"): raise SystemExit(0) +session_start_source = first_text("source") # Grok injects GROK_SESSION_ID into every hook process; prefer it and fall # back to the event payload's session id fields. @@ -72,16 +73,19 @@ if not agent_session_id: request_id = f"{source}:{int(time.time() * 1000)}:{random.randrange(1_000_000):06d}" report_seq = time.time_ns() +params = { + "pane_id": pane_id, + "source": source, + "agent": "grok", + "seq": report_seq, + "agent_session_id": agent_session_id, +} +if session_start_source: + params["session_start_source"] = session_start_source request = { "id": request_id, "method": "pane.report_agent_session", - "params": { - "pane_id": pane_id, - "source": source, - "agent": "grok", - "seq": report_seq, - "agent_session_id": agent_session_id, - }, + "params": params, } try: diff --git a/src/integration/mod.rs b/src/integration/mod.rs index 49808ca273..55dcb2be6c 100644 --- a/src/integration/mod.rs +++ b/src/integration/mod.rs @@ -296,7 +296,7 @@ const GROK_HOOK_ASSET: &str = if cfg!(windows) { } else { include_str!("assets/grok/herdr-agent-state.sh") }; -const GROK_INTEGRATION_VERSION: u32 = 1; +const GROK_INTEGRATION_VERSION: u32 = 2; pub(crate) const INSTALL_WARNING_PREFIX: &str = "warning:"; diff --git a/src/integration/tests.rs b/src/integration/tests.rs index dc597d1d53..e8aad72b1c 100644 --- a/src/integration/tests.rs +++ b/src/integration/tests.rs @@ -2897,6 +2897,7 @@ fn bundled_integration_asset_versions_match_expected_versions() { MASTRACODE_HOOK_ASSET, MASTRACODE_INTEGRATION_VERSION, ), + ("grok", GROK_HOOK_ASSET, GROK_INTEGRATION_VERSION), ] { assert_eq!( parse_integration_version(asset), @@ -4056,7 +4057,39 @@ fn install_antigravity_cli_errors_when_config_dir_missing() { } #[test] -fn grok_v1_integration_status_is_current() { +fn grok_v1_integration_status_is_outdated() { + let _lock = integration_env_lock(); + let base = unique_base(); + let grok_dir = base.join(".grok"); + let hooks_dir = grok_dir.join("hooks"); + fs::create_dir_all(&hooks_dir).unwrap(); + std::env::set_var(GROK_CONFIG_DIR_ENV_VAR, &grok_dir); + let hook_path = hooks_dir.join(GROK_HOOK_INSTALL_NAME); + fs::write( + &hook_path, + "#!/bin/sh\n# HERDR_INTEGRATION_ID=grok\n# HERDR_INTEGRATION_VERSION=1\n", + ) + .unwrap(); + fs::write( + hooks_dir.join(GROK_HOOK_CONFIG_INSTALL_NAME), + serde_json::to_string(&grok_hook_config(&hook_path)).unwrap(), + ) + .unwrap(); + + let grok = installed_integration_statuses() + .into_iter() + .find(|status| status.target == crate::api::schema::IntegrationTarget::Grok) + .expect("grok integration status"); + assert_eq!(grok.installed_version, Some(1)); + assert_eq!(grok.expected_version, GROK_INTEGRATION_VERSION); + assert_eq!(grok.state, IntegrationStatusKind::Outdated); + + clear_integration_path_env(); + let _ = fs::remove_dir_all(base); +} + +#[test] +fn grok_v2_integration_status_is_current() { let _lock = integration_env_lock(); let base = unique_base(); let grok_dir = base.join(".grok"); diff --git a/src/terminal/state.rs b/src/terminal/state.rs index dc28af1065..a7c8457481 100644 --- a/src/terminal/state.rs +++ b/src/terminal/state.rs @@ -1331,6 +1331,7 @@ impl TerminalState { | ("herdr:hermes", "hermes", Some("startup" | "new" | "resume")) | ("herdr:opencode", "opencode", Some("select")) | ("herdr:pi", "pi", Some("new" | "resume" | "fork")) + | ("herdr:grok", "grok", Some("new")) | ( "herdr:omp", "omp", @@ -1635,7 +1636,8 @@ impl TerminalState { session_ref: &crate::agent_resume::AgentSessionRef, session_start_source: Option<&str>, ) -> bool { - Self::session_start_source_is_recognized(session_start_source) + (source, agent_label) != ("herdr:grok", "grok") + && Self::session_start_source_is_recognized(session_start_source) && self.foreground_agent_confirms_session_owner(source, agent_label, session_ref) } @@ -4652,6 +4654,38 @@ mod tests { ); } + #[test] + fn grok_new_session_ref_replaces_existing_session_ref() { + let mut terminal = test_terminal(); + terminal + .set_agent_session_ref( + "herdr:grok".into(), + "grok".into(), + crate::agent_resume::AgentSessionRef::id("grok-old"), + Some(20), + ) + .expect("initial session should be accepted"); + + let mutation = terminal + .set_agent_session_ref_for_session_start( + "herdr:grok".into(), + "grok".into(), + crate::agent_resume::AgentSessionRef::id("grok-new"), + Some(21), + Some("new".into()), + ) + .expect("new should replace the grok session"); + + assert!(mutation.session_ref_changed); + assert_eq!( + terminal + .persisted_agent_session + .as_ref() + .map(|session| session.session_ref.value.as_str()), + Some("grok-new") + ); + } + #[test] fn opencode_server_new_does_not_replace_existing_session_ref() { let mut terminal = test_terminal(); @@ -5105,6 +5139,35 @@ mod tests { ); } + #[test] + fn grok_new_session_does_not_replace_a_different_owner() { + let mut terminal = test_terminal(); + terminal.set_persisted_agent_session(crate::agent_resume::PersistedAgentSession { + source: "herdr:claude".into(), + agent: "claude".into(), + session_ref: crate::agent_resume::AgentSessionRef::id("claude-session").unwrap(), + }); + terminal.set_detected_state(Some(Agent::Grok), AgentState::Idle); + + let mutation = terminal.set_agent_session_ref_for_session_start( + "herdr:grok".into(), + "grok".into(), + crate::agent_resume::AgentSessionRef::id("grok-session"), + Some(21), + Some("new".into()), + ); + + assert!(mutation.is_none()); + assert_eq!( + terminal.persisted_agent_session.as_ref().map(|session| ( + session.source.as_str(), + session.agent.as_str(), + session.session_ref.value.as_str() + )), + Some(("herdr:claude", "claude", "claude-session")) + ); + } + #[test] fn foreground_agent_session_replaces_stale_different_owner_session_ref() { for session_start_source in ["resume", "startup"] { diff --git a/tests/cli/hooks.rs b/tests/cli/hooks.rs index de2bb271b9..31978a0318 100644 --- a/tests/cli/hooks.rs +++ b/tests/cli/hooks.rs @@ -37,6 +37,15 @@ fn run_devin_hook( ) } +fn run_grok_hook(hook_input: &str, envs: &[(&str, &str)]) -> Option { + run_shell_hook_with_env( + "src/integration/assets/grok/herdr-agent-state.sh", + &["session"], + hook_input, + envs, + ) +} + fn run_shell_hook(asset_path: &str, args: &[&str], hook_input: &str) -> Option { run_shell_hook_with_env(asset_path, args, hook_input, &[]) } @@ -237,6 +246,19 @@ fn copilot_hook_reports_session_id_from_stdin() { assert!(camel["params"].get("state").is_none()); } +#[test] +fn grok_hook_reports_new_session_source() { + let request = run_grok_hook( + r#"{"hook_event_name":"session_start","source":"new","session_id":"new-session"}"#, + &[("GROK_SESSION_ID", "new-session")], + ) + .expect("grok session start should report session identity"); + + assert_eq!(request["method"], "pane.report_agent_session"); + assert_eq!(request["params"]["agent_session_id"], "new-session"); + assert_eq!(request["params"]["session_start_source"], "new"); +} + #[test] fn copilot_hook_does_not_report_lifecycle_state() { for payload in [ From 3b9f58b70ed50f80e8aaa99ead2e4ccfa2c5c4ba Mon Sep 17 00:00:00 2001 From: JJ Liebig Date: Sun, 13 Sep 2026 04:17:50 +0400 Subject: [PATCH 15/16] fix: deflake windows endpoint flush drain test (#4021) The polling peer drains at production's 2ms Windows read cadence, so a 1 MiB frame took ~5.5s locally and exceeded the test's 10s flush deadline under CI load. Use a 256 KiB frame, still well above the 64 KiB batch limit, and a 30s deadline so the assertion no longer depends on CI scheduling. --- src/client/endpoint/writer.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/client/endpoint/writer.rs b/src/client/endpoint/writer.rs index 56c472cd3b..f7d67454a5 100644 --- a/src/client/endpoint/writer.rs +++ b/src/client/endpoint/writer.rs @@ -373,16 +373,18 @@ mod tests { done.send((first, second)).unwrap(); }); let input = ClientMessage::Input { - data: vec![b'x'; 1024 * 1024], + // Comfortably above MAX_BATCH_BYTES, but small enough that the polling peer's + // Windows 2ms read cadence drains it well within the flush deadline under CI load. + data: vec![b'x'; 256 * 1024], }; transport.send(&input).unwrap(); transport.send(&ClientMessage::Detach).unwrap(); // Large-frame correctness must not depend on the registry's short exit grace period. transport - .flush(Instant::now() + Duration::from_secs(10)) + .flush(Instant::now() + Duration::from_secs(30)) .unwrap(); drop(transport); - let (first, second) = received.recv_timeout(Duration::from_secs(10)).unwrap(); + let (first, second) = received.recv_timeout(Duration::from_secs(30)).unwrap(); assert_eq!(first, input); assert_eq!(second, ClientMessage::Detach); reader.join().unwrap(); From bafbc0949dd996cf7fd0848c8965e254348cc11e Mon Sep 17 00:00:00 2001 From: Rhett CfZhuang Date: Sun, 13 Sep 2026 08:43:38 +0800 Subject: [PATCH 16/16] fix(linux): avoid blocking proc reads for wsl agents (#2179) Co-authored-by: JJ Liebig --- src/platform/linux.rs | 164 +++++++++++++++++++++++++++++++++++------- 1 file changed, 140 insertions(+), 24 deletions(-) diff --git a/src/platform/linux.rs b/src/platform/linux.rs index ac0645c1ce..29534dbee9 100644 --- a/src/platform/linux.rs +++ b/src/platform/linux.rs @@ -34,6 +34,7 @@ enum ProcessDetectionMode { struct ProcGroupMember { pid: u32, comm: String, + state: char, } pub fn raise_server_nofile_limit() {} @@ -47,6 +48,11 @@ pub(crate) fn should_query_host_terminal_palette() -> bool { } fn running_inside_wsl() -> bool { + static RUNNING_INSIDE_WSL: OnceLock = OnceLock::new(); + *RUNNING_INSIDE_WSL.get_or_init(detect_running_inside_wsl) +} + +fn detect_running_inside_wsl() -> bool { proc_file_indicates_wsl("/proc/sys/kernel/osrelease") || proc_file_indicates_wsl("/proc/version") || WSL_MARKER_ENV_VARS @@ -140,23 +146,41 @@ pub(crate) fn available_pane_shell(child_pid: u32) -> Option { } pub fn foreground_job(child_pid: u32) -> Option { - if let Some(tpgid) = foreground_process_group_id(child_pid) { - return foreground_job_for_group(child_pid, tpgid); - } - - if process_detection_mode() != ProcessDetectionMode::ChildGroups { - return None; - } - - foreground_job_for_group(child_pid, child_groups_foreground_process_group(child_pid)?) + let process_group_id = foreground_process_group_id(child_pid).or_else(|| { + (process_detection_mode() == ProcessDetectionMode::ChildGroups) + .then(|| child_groups_foreground_process_group(child_pid)) + .flatten() + })?; + foreground_job_for_group(child_pid, process_group_id) } fn foreground_job_for_group(child_pid: u32, process_group_id: u32) -> Option { let members = foreground_process_group_members(child_pid, process_group_id)?; + foreground_job_from_members( + process_group_id, + members, + running_inside_wsl(), + process_argv, + ) +} + +fn foreground_job_from_members( + process_group_id: u32, + members: Vec, + running_inside_wsl: bool, + mut read_argv: impl FnMut(u32) -> Option>, +) -> Option { let processes = members .into_iter() .map(|member| { - let argv = process_argv(member.pid); + // Reading procfs cmdline enters access_remote_vm. On WSL, that read can + // block indefinitely while a multithreaded process is exiting. A state + // check alone has a race, so WSL uses the cheap comm-based identity when + // it already identifies a supported agent without inspecting cmdline. + let argv = + process_allows_remote_memory_read(member.state, &member.comm, running_inside_wsl) + .then(|| read_argv(member.pid)) + .flatten(); ForegroundProcess { pid: member.pid, name: member.comm, @@ -181,8 +205,8 @@ fn foreground_job_for_group(child_pid: u32, process_group_id: u32) -> Option Option { - let shell_group_id = process_pgrp_and_comm(child_pid) - .map(|(pgrp, _)| pgrp) + let shell_group_id = process_pgrp_comm_and_state(child_pid) + .map(|(pgrp, _, _)| pgrp) .filter(|pgrp| *pgrp > 0)? as u32; child_groups_foreground_process_group_with( @@ -190,7 +214,7 @@ fn child_groups_foreground_process_group(child_pid: u32) -> Option { shell_group_id, process_task_ids, process_task_children, - |pid| process_pgrp_and_comm(pid).map(|(pgrp, _)| pgrp), + |pid| process_pgrp_comm_and_state(pid).map(|(pgrp, _, _)| pgrp), ) } @@ -311,17 +335,19 @@ fn numeric_file_name(entry: &std::fs::DirEntry) -> Option { } fn live_process_group_member(process_group_id: u32, pid: u32) -> Option { - let (pgrp, comm) = process_pgrp_and_comm(pid)?; - (pgrp > 0 && pgrp as u32 == process_group_id).then_some(ProcGroupMember { pid, comm }) + let (pgrp, comm, state) = process_pgrp_comm_and_state(pid)?; + (pgrp > 0 && pgrp as u32 == process_group_id).then_some(ProcGroupMember { pid, comm, state }) } pub fn foreground_group_leader_job(process_group_id: u32) -> Option { - let (pgrp, name) = process_pgrp_and_comm(process_group_id)?; + let (pgrp, name, state) = process_pgrp_comm_and_state(process_group_id)?; if pgrp as u32 != process_group_id { return None; } - let argv = process_argv(process_group_id); + let argv = process_allows_remote_memory_read(state, &name, running_inside_wsl()) + .then(|| process_argv(process_group_id)) + .flatten(); Some(ForegroundJob { process_group_id, processes: vec![ForegroundProcess { @@ -350,18 +376,28 @@ pub fn foreground_process_group_id_for_tty_fd(fd: RawFd) -> Option { (pgid > 0).then_some(pgid as u32) } -fn process_pgrp_and_comm(pid: u32) -> Option<(i32, String)> { +fn process_pgrp_comm_and_state(pid: u32) -> Option<(i32, String, char)> { let stat = std::fs::read_to_string(format!("/proc/{pid}/stat")).ok()?; - process_pgrp_and_comm_from_stat(&stat) + process_pgrp_comm_and_state_from_stat(&stat) } -fn process_pgrp_and_comm_from_stat(stat: &str) -> Option<(i32, String)> { +fn process_pgrp_comm_and_state_from_stat(stat: &str) -> Option<(i32, String, char)> { let close = stat.rfind(')')?; let comm = stat.get(1 + stat.find('(')?..close)?.to_string(); let rest = stat.get(close + 2..)?; let fields: Vec<&str> = rest.split_whitespace().collect(); + let state = fields.first()?.chars().next()?; let pgrp: i32 = fields.get(2)?.parse().ok()?; - Some((pgrp, comm)) + Some((pgrp, comm, state)) +} + +fn process_state_allows_remote_memory_read(state: char) -> bool { + !matches!(state, 'D' | 'Z' | 'X' | 'x') +} + +fn process_allows_remote_memory_read(state: char, comm: &str, running_inside_wsl: bool) -> bool { + process_state_allows_remote_memory_read(state) + && (!running_inside_wsl || crate::detect::identify_agent(comm).is_none()) } fn process_argv(pid: u32) -> Option> { @@ -391,6 +427,10 @@ pub fn process_agent_hint(pid: u32) -> Option { if pid == 0 { return None; } + let (_, comm, state) = process_pgrp_comm_and_state(pid)?; + if !process_allows_remote_memory_read(state, &comm, running_inside_wsl()) { + return None; + } let environ = std::fs::read(format!("/proc/{pid}/environ")).ok()?; super::parse_agent_env_hint(&environ) } @@ -989,6 +1029,7 @@ mod tests { (*pgrp == process_group_id).then(|| ProcGroupMember { pid, comm: (*comm).to_string(), + state: 'S', }) }, ) @@ -1023,6 +1064,7 @@ mod tests { (pid == process_group_id).then(|| ProcGroupMember { pid, comm: "leader".to_string(), + state: 'S', }) }, ) @@ -1032,7 +1074,8 @@ mod tests { members, vec![ProcGroupMember { pid: 200, - comm: "leader".to_string() + comm: "leader".to_string(), + state: 'S', }] ); } @@ -1058,6 +1101,7 @@ mod tests { .then(|| ProcGroupMember { pid, comm: format!("member-{pid}"), + state: 'S', }) .filter(|_| process_group_id == 200) }, @@ -1076,11 +1120,83 @@ mod tests { #[test] fn proc_stat_parsing_keeps_group_leader_inputs_live() { assert_eq!( - process_pgrp_and_comm_from_stat("123 (name with ) paren) S 1 456 789 0 456"), - Some((456, "name with ) paren".to_string())) + process_pgrp_comm_and_state_from_stat("123 (name with ) paren) S 1 456 789 0 456"), + Some((456, "name with ) paren".to_string(), 'S')) ); } + #[test] + fn foreground_job_does_not_read_remote_memory_for_uninterruptible_members() { + let argv_reads = RefCell::new(Vec::new()); + let job = foreground_job_from_members( + 200, + vec![ + ProcGroupMember { + pid: 200, + comm: "codex".to_string(), + state: 'D', + }, + ProcGroupMember { + pid: 201, + comm: "helper".to_string(), + state: 'S', + }, + ], + true, + |pid| { + argv_reads.borrow_mut().push(pid); + Some(vec![format!("process-{pid}")]) + }, + ) + .unwrap(); + + assert_eq!(argv_reads.into_inner(), vec![201]); + assert_eq!(job.processes[0].name, "codex"); + assert_eq!(job.processes[0].argv, None); + assert_eq!(job.processes[1].argv, Some(vec!["process-201".to_string()])); + } + + #[test] + fn foreground_job_on_wsl_skips_known_agents_but_reads_wrappers() { + let argv_reads = RefCell::new(Vec::new()); + let job = foreground_job_from_members( + 200, + vec![ + ProcGroupMember { + pid: 200, + comm: "codex".to_string(), + state: 'S', + }, + ProcGroupMember { + pid: 201, + comm: "node".to_string(), + state: 'S', + }, + ], + true, + |pid| { + argv_reads.borrow_mut().push(pid); + Some(vec![format!("process-{pid}")]) + }, + ) + .unwrap(); + + assert_eq!(argv_reads.into_inner(), vec![201]); + assert_eq!(job.processes[0].name, "codex"); + assert_eq!(job.processes[0].argv, None); + assert_eq!(job.processes[1].argv, Some(vec!["process-201".to_string()])); + } + + #[test] + fn remote_memory_reads_reject_dead_and_uninterruptible_states() { + for state in ['D', 'Z', 'X', 'x'] { + assert!(!process_state_allows_remote_memory_read(state)); + } + for state in ['R', 'S', 'I', 'T', 't'] { + assert!(process_state_allows_remote_memory_read(state)); + } + } + #[test] fn clipboard_commands_prefer_wayland_when_available() { let _guard = env_lock().lock().unwrap();