fix: keep workspace navigation visible in terminal theme (#4408)

* fix: keep workspace navigation visible in terminal theme

refs #4300

* fix: clear workspace highlight on directional pane focus

refs #4300

---------

Co-authored-by: JJ Liebig <jonathan.liebig@gmail.com>

1 files changed, 466 insertions, 1 deletions

diff --git a/src/client/shell/tests/workspace_navigation.rs b/src/client/shell/tests/workspace_navigation.rs
index 7ea02066..c117a319 100644
--- a/src/client/shell/tests/workspace_navigation.rs
+++ b/src/client/shell/tests/workspace_navigation.rs
@@ -72,6 +72,81 @@ fn workspace_rect(state: &ClientShellState, endpoint: &ClientEndpointId, workspa
 }
 
 #[test]
+fn local_navigation_highlight_stays_visible_with_terminal_theme() {
+    use ratatui::style::Color;
+
+    for compact in [false, true] {
+        for selection_bg in [Color::Reset, Color::Rgb(70, 63, 93)] {
+            let mut config = ClientShellConfig::from_config(&Config::default());
+            config.palette = Palette::terminal();
+            config.palette.selection_bg = selection_bg;
+            let expected_bg = if selection_bg == Color::Reset {
+                config.palette.active_row_bg
+            } else {
+                selection_bg
+            };
+            let mut state = ClientShellState::new(config);
+            state.set_snapshot(Box::new(workspaces(3)));
+            state.set_pane_surface(surface());
+            state.sidebar_collapsed = compact;
+            state.compose(100, 28).unwrap();
+            enter_navigation(&mut state);
+
+            for workspace_id in ["ws_1", "ws_2"] {
+                assert_selected(&state, &ClientEndpointId::Local, workspace_id);
+                let buffer = state.compose(100, 28).unwrap().to_ratatui_buffer().unwrap();
+                let selected = workspace_rect(&state, &ClientEndpointId::Local, workspace_id);
+                for y in selected.y..selected.bottom() {
+                    for x in selected.x..selected.right() {
+                        assert_eq!(
+                            buffer[(x, y)].bg,
+                            expected_bg,
+                            "compact={compact}, {workspace_id}, ({x}, {y})"
+                        );
+                    }
+                }
+                let untouched = workspace_rect(&state, &ClientEndpointId::Local, "ws_3");
+                assert_ne!(buffer[(untouched.x, untouched.y)].bg, expected_bg);
+                if workspace_id != "ws_1" {
+                    let focused = workspace_rect(&state, &ClientEndpointId::Local, "ws_1");
+                    assert_eq!(
+                        buffer[(focused.x, focused.y)].bg,
+                        if selection_bg == Color::Reset {
+                            state.config.palette.sidebar_bg
+                        } else {
+                            state.config.palette.active_row_bg
+                        }
+                    );
+                    assert_ne!(buffer[(focused.x, focused.y)].bg, expected_bg);
+                }
+                preview_key(&mut state, b"\x1b[B");
+            }
+            assert_eq!(
+                state
+                    .snapshot
+                    .as_ref()
+                    .unwrap()
+                    .focused_workspace_id
+                    .as_deref(),
+                Some("ws_1")
+            );
+            preview_key(&mut state, b"\x1b");
+            let buffer = state.compose(100, 28).unwrap().to_ratatui_buffer().unwrap();
+            let focused = workspace_rect(&state, &ClientEndpointId::Local, "ws_1");
+            assert_eq!(
+                buffer[(focused.x, focused.y)].bg,
+                state.config.palette.active_row_bg
+            );
+            let cancelled = workspace_rect(&state, &ClientEndpointId::Local, "ws_3");
+            assert_eq!(
+                buffer[(cancelled.x, cancelled.y)].bg,
+                state.config.palette.sidebar_bg
+            );
+        }
+    }
+}
+
+#[test]
 fn navigation_highlights_only_the_preview_and_activates_on_enter() {
     for (compact, cols) in [(true, 100), (false, 100), (false, 44)] {
         for terminal_theme in [false, true] {
@@ -113,7 +188,13 @@ fn navigation_highlights_only_the_preview_and_activates_on_enter() {
                 assert_ne!(buffer[(other.x + 2, other.y)].bg, color);
                 assert_eq!(
                     buffer[(focused.x + 2, focused.y)].bg,
-                    if cols == 44 {
+                    if terminal_theme {
+                        if cols == 44 {
+                            palette.panel_bg
+                        } else {
+                            palette.sidebar_bg
+                        }
+                    } else if cols == 44 {
                         palette.surface_dim
                     } else {
                         palette.active_row_bg
@@ -503,3 +584,387 @@ fn aggregate_navigation_reveals_overflow_and_preserves_order() {
         assert_selected(&state, &ClientEndpointId::Local, "ws_1");
     }
 }
+
+fn local_navigation_state(compact: bool) -> ClientShellState {
+    let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default()));
+    state.config.palette = Palette::terminal();
+    state.sidebar_collapsed = compact;
+    state.set_snapshot(Box::new(workspaces(3)));
+    state.set_pane_surface(surface());
+    state.compose(100, 28).unwrap();
+    state
+}
+
+fn request_local_navigation(state: &mut ClientShellState, down: usize) -> String {
+    enter_navigation(state);
+    for _ in 0..down {
+        preview_key(state, b"\x1b[B");
+    }
+    let outcome = state.handle_input_bytes(b"\r");
+    let [ClientShellAction::Endpoint { request, .. }] = outcome.actions.as_slice() else {
+        panic!("expected a local workspace focus request");
+    };
+    assert!(matches!(
+        request.method,
+        crate::api::schema::Method::WorkspaceFocus(_)
+    ));
+    request.id.clone()
+}
+
+fn assert_local_highlight(state: &mut ClientShellState, selected_id: &str) {
+    let buffer = state.compose(100, 28).unwrap().to_ratatui_buffer().unwrap();
+    for workspace_id in ["ws_1", "ws_2", "ws_3"] {
+        let rect = workspace_rect(state, &ClientEndpointId::Local, workspace_id);
+        assert_eq!(
+            (rect.x..rect.right())
+                .any(|x| buffer[(x, rect.y)].bg == state.config.palette.active_row_bg),
+            workspace_id == selected_id,
+            "expected only {selected_id} highlighted, checking {workspace_id}"
+        );
+    }
+}
+
+fn set_local_focus(state: &mut ClientShellState, workspace_id: &str, revision: u64) {
+    let mut snapshot = workspaces(3);
+    snapshot.revision = revision;
+    snapshot.focused_workspace_id = Some(workspace_id.into());
+    for workspace in &mut snapshot.workspaces {
+        workspace.focused = workspace.workspace_id == workspace_id;
+    }
+    state.set_snapshot(Box::new(snapshot));
+    let mut frame = surface();
+    frame.projection_revision = revision;
+    state.set_pane_surface(frame);
+}
+
+#[test]
+fn accepted_local_navigation_keeps_highlight_until_authoritative_focus() {
+    for compact in [false, true] {
+        for response_first in [false, true] {
+            let mut state = local_navigation_state(compact);
+            let request_id = request_local_navigation(&mut state, 2);
+            assert_eq!(state.mode, ClientShellMode::Terminal);
+            assert!(state.navigate_workspace_id.is_none());
+            assert_eq!(
+                state
+                    .snapshot
+                    .as_ref()
+                    .unwrap()
+                    .focused_workspace_id
+                    .as_deref(),
+                Some("ws_1")
+            );
+            assert_eq!(state.focused_pane_id().as_deref(), Some("pane_1"));
+            assert_local_highlight(&mut state, "ws_3");
+            state.invalidate_pane_surface();
+            assert_local_highlight(&mut state, "ws_3");
+            state.set_pane_surface(surface());
+            if response_first {
+                state.handle_endpoint_result(
+                    "boot-1",
+                    &request_id,
+                    Ok(crate::api::schema::ResponseResult::Ok {}),
+                );
+                assert_local_highlight(&mut state, "ws_3");
+            }
+            set_local_focus(&mut state, "ws_1", 2);
+            assert_local_highlight(&mut state, "ws_3");
+            set_local_focus(&mut state, "ws_3", 3);
+            assert_local_highlight(&mut state, "ws_3");
+            if !response_first {
+                state.handle_endpoint_result(
+                    "boot-1",
+                    &request_id,
+                    Ok(crate::api::schema::ResponseResult::Ok {}),
+                );
+            }
+            set_local_focus(&mut state, "ws_2", 4);
+            assert_local_highlight(&mut state, "ws_2");
+        }
+    }
+}
+
+#[test]
+fn failed_local_navigation_releases_only_its_own_highlight() {
+    for failure in ["rejected", "endpoint_timeout", "cancelled"] {
+        let mut state = local_navigation_state(false);
+        let request_id = request_local_navigation(&mut state, 2);
+        assert_local_highlight(&mut state, "ws_3");
+        if failure == "cancelled" {
+            assert!(state.cancel_endpoint_request(&request_id));
+        } else {
+            state.handle_endpoint_result(
+                "boot-1",
+                &request_id,
+                Err(ClientShellEndpointError {
+                    code: Some(failure.into()),
+                    message: "focus failed".into(),
+                }),
+            );
+        }
+        assert_local_highlight(&mut state, "ws_1");
+        state.handle_endpoint_result(
+            "boot-1",
+            &request_id,
+            Ok(crate::api::schema::ResponseResult::Ok {}),
+        );
+        assert_local_highlight(&mut state, "ws_1");
+    }
+    for old_down in [1, 2] {
+        let mut state = local_navigation_state(false);
+        let old_request = request_local_navigation(&mut state, old_down);
+        let latest_request = request_local_navigation(&mut state, 2);
+        state.cancel_endpoint_request(&old_request);
+        assert_local_highlight(&mut state, "ws_3");
+        state.cancel_endpoint_request(&latest_request);
+        assert_local_highlight(&mut state, "ws_1");
+    }
+}
+
+#[test]
+fn pending_navigation_highlight_does_not_survive_identity_changes() {
+    for change in [
+        "disconnect",
+        "retire",
+        "boot",
+        "generation",
+        "deleted",
+        "endpoint",
+    ] {
+        let mut state = local_navigation_state(false);
+        let request_id = request_local_navigation(&mut state, 2);
+        state.handle_endpoint_result(
+            "boot-1",
+            &request_id,
+            Ok(crate::api::schema::ResponseResult::Ok {}),
+        );
+        assert_local_highlight(&mut state, "ws_3");
+        let mut snapshot = workspaces(3);
+        match change {
+            "disconnect" => {
+                state.mark_endpoint_disconnected(&ClientEndpointId::Local);
+                state.set_endpoint_status(&ClientEndpointId::Local, ClientEndpointStatus::Online);
+                state.set_snapshot(Box::new(snapshot));
+            }
+            "retire" => {
+                state.retire_endpoint(&ClientEndpointId::Local);
+                state.set_endpoint_status(&ClientEndpointId::Local, ClientEndpointStatus::Online);
+                state.set_snapshot(Box::new(snapshot));
+            }
+            "boot" => {
+                snapshot.boot_id = "replacement-boot".into();
+                state.set_snapshot(Box::new(snapshot));
+            }
+            "generation" => state.set_endpoint_snapshot_for_generation(
+                &ClientEndpointId::Local,
+                1,
+                Box::new(snapshot),
+            ),
+            "deleted" => {
+                snapshot.workspaces.pop();
+                state.set_snapshot(Box::new(snapshot));
+                state.set_snapshot(Box::new(workspaces(3)));
+            }
+            "endpoint" => {
+                let profile = remote_profile();
+                let remote = ClientEndpointId::Ssh(profile.id.clone());
+                state.set_endpoint_catalog(&[profile]);
+                state.set_endpoint_status(&remote, ClientEndpointStatus::Online);
+                state.set_endpoint_snapshot(&remote, Box::new(snapshot));
+                assert!(state.activate_endpoint_projection(&remote));
+                assert!(state.pending_workspace_highlight.is_none());
+                assert!(state.activate_endpoint_projection(&ClientEndpointId::Local));
+                state.set_endpoint_catalog(&[]);
+            }
+            _ => unreachable!(),
+        }
+        assert!(state.pending_workspace_highlight.is_none(), "{change}");
+        let mut frame = surface();
+        frame.boot_id = state.snapshot.as_ref().unwrap().boot_id.clone();
+        state.set_pane_surface(frame);
+        assert_local_highlight(&mut state, "ws_1");
+    }
+}
+
+#[test]
+fn navigation_highlight_requires_enqueued_focus_and_yields_to_new_intent() {
+    let mut state = local_navigation_state(false);
+    state.set_endpoint_methods(Some(Vec::new()));
+    enter_navigation(&mut state);
+    preview_key(&mut state, b"\x1b[B");
+    let refused = state.handle_input_bytes(b"\r");
+    assert!(refused.actions.is_empty());
+    assert_eq!(state.mode, ClientShellMode::Terminal);
+    assert!(state.pending_workspace_highlight.is_none());
+    // The unsupported-action notice spans the sidebar on this narrow frame.
+    assert!(state.visible_endpoint_notice.take().is_some());
+    assert_local_highlight(&mut state, "ws_1");
+
+    state.set_endpoint_methods(None);
+    request_local_navigation(&mut state, 2);
+    enter_navigation(&mut state);
+    preview_key(&mut state, b"\x1b");
+    assert_local_highlight(&mut state, "ws_1");
+
+    request_local_navigation(&mut state, 2);
+    let mut unrelated = ClientShellInput::default();
+    state.push_endpoint_method(
+        crate::api::schema::Method::ServerReloadConfig(crate::api::schema::EmptyParams::default()),
+        &mut unrelated,
+    );
+    let [ClientShellAction::Endpoint { request, .. }] = unrelated.actions.as_slice() else {
+        panic!("expected unrelated request");
+    };
+    state.cancel_endpoint_request(&request.id);
+    assert_local_highlight(&mut state, "ws_3");
+    let mut focus = ClientShellInput::default();
+    state.focus_or_activate(
+        ClientEndpointId::Local,
+        ClientEndpointFocusTarget::Workspace("ws_2".into()),
+        &mut focus,
+    );
+    assert!(state.pending_workspace_highlight.is_none());
+}
+
+#[test]
+fn directional_pane_focus_releases_an_accepted_workspace_highlight() {
+    use crate::api::schema::{Method, PaneDirection, ResponseResult};
+
+    for (key, direction) in [
+        (b'h', PaneDirection::Left),
+        (b'j', PaneDirection::Down),
+        (b'k', PaneDirection::Up),
+        (b'l', PaneDirection::Right),
+    ] {
+        for rejected in [false, true] {
+            let mut state = local_navigation_state(false);
+            let pending_request = request_local_navigation(&mut state, 2);
+            assert_local_highlight(&mut state, "ws_3");
+            preview_key(&mut state, &[0x02]);
+            let outcome = state.handle_input_bytes(&[key]);
+            let [ClientShellAction::Endpoint { request, .. }] = outcome.actions.as_slice() else {
+                panic!("expected a directional pane focus request");
+            };
+            let Method::PaneFocusDirection(params) = &request.method else {
+                panic!("expected PaneFocusDirection");
+            };
+            assert_eq!(params.direction, direction);
+            assert_eq!(params.pane_id.as_deref(), Some("pane_1"));
+            assert!(state.pending_workspace_highlight.is_none());
+            assert_local_highlight(&mut state, "ws_1");
+            let result = if rejected {
+                Err(ClientShellEndpointError {
+                    code: Some("rejected".into()),
+                    message: "focus rejected".into(),
+                })
+            } else {
+                Ok(ResponseResult::Ok {})
+            };
+            state.handle_endpoint_result("boot-1", &pending_request, result);
+            assert_local_highlight(&mut state, "ws_1");
+        }
+    }
+}
+
+#[test]
+fn direct_agent_focus_repaints_when_releasing_a_workspace_highlight() {
+    let mut config = Config::default();
+    config.keys.focus_agent = crate::config::BindingConfig::one("ctrl+alt+1");
+    let mut projected = workspaces(3);
+    projected.agents.push(agent("agent", AgentStatus::Idle, 1));
+
+    for pending in [false, true] {
+        let mut state = local_navigation_state(false);
+        state.config.keybinds = ClientShellConfig::from_config(&config).keybinds;
+        state.set_snapshot(Box::new(projected.clone()));
+        state.compose(100, 28).unwrap();
+        if pending {
+            request_local_navigation(&mut state, 2);
+            assert_local_highlight(&mut state, "ws_3");
+        }
+
+        // Direct bindings do not inherit the repaint from leaving prefix mode.
+        let outcome =
+            state.handle_raw_events(vec![RawInputEvent::Key(crate::input::TerminalKey::new(
+                KeyCode::Char('1'),
+                KeyModifiers::CONTROL | KeyModifiers::ALT,
+            ))]);
+        assert!(
+            matches!(outcome.actions.as_slice(), [ClientShellAction::Endpoint { request, .. }]
+            if matches!(&request.method, crate::api::schema::Method::PaneFocus(params)
+                if params.pane_id == "pane_1"))
+        );
+        assert!(state.pending_workspace_highlight.is_none());
+        assert_eq!(outcome.repaint, pending);
+        assert_local_highlight(&mut state, "ws_1");
+    }
+}
+
+#[test]
+fn cancelled_close_does_not_restore_an_older_navigation_highlight() {
+    let mut state = local_navigation_state(false);
+    request_local_navigation(&mut state, 2);
+    state.open_confirm_close_overlay("ws_1".into());
+    preview_key(&mut state, b"\x1b");
+    assert_eq!(state.mode, ClientShellMode::Navigate);
+    preview_key(&mut state, b"\x1b");
+    assert_eq!(state.mode, ClientShellMode::Terminal);
+    assert_local_highlight(&mut state, "ws_1");
+}
+
+#[test]
+fn coalesced_navigation_focus_does_not_leave_a_permanent_highlight() {
+    let mut state = local_navigation_state(false);
+    let before_request = std::time::Instant::now();
+    let request_id = request_local_navigation(&mut state, 2);
+    state.handle_endpoint_result(
+        "boot-1",
+        &request_id,
+        Ok(crate::api::schema::ResponseResult::Ok {}),
+    );
+    // Another client can focus the original workspace before the server projects
+    // either change, so a successful request need not produce a new snapshot.
+    assert!(!state.tick_workspace_highlight(before_request));
+    assert_local_highlight(&mut state, "ws_3");
+    let now = std::time::Instant::now();
+    assert!(state.tick_workspace_highlight(now + std::time::Duration::from_secs(2)));
+    assert_local_highlight(&mut state, "ws_1");
+    assert!(!state.tick_workspace_highlight(now + std::time::Duration::from_secs(3)));
+}
+
+#[test]
+fn navigation_highlight_ends_for_noop_focus_and_focused_creation() {
+    let mut state = local_navigation_state(false);
+    request_local_navigation(&mut state, 0);
+    assert!(state.pending_workspace_highlight.is_none());
+    set_local_focus(&mut state, "ws_2", 2);
+    assert_local_highlight(&mut state, "ws_2");
+
+    for focus in [false, true] {
+        for method in [
+            crate::api::schema::Method::WorkspaceCreate(
+                crate::api::schema::WorkspaceCreateParams {
+                    source_workspace_id: None,
+                    cwd: None,
+                    focus,
+                    label: None,
+                    env: Default::default(),
+                },
+            ),
+            crate::api::schema::Method::TabCreate(crate::api::schema::TabCreateParams {
+                workspace_id: Some("ws_1".into()),
+                cwd: None,
+                focus,
+                label: None,
+                env: Default::default(),
+            }),
+        ] {
+            let mut state = local_navigation_state(false);
+            request_local_navigation(&mut state, 2);
+            let mut outcome = ClientShellInput::default();
+            state.push_endpoint_method(method, &mut outcome);
+            assert_eq!(state.pending_workspace_highlight.is_none(), focus);
+            assert_local_highlight(&mut state, if focus { "ws_1" } else { "ws_3" });
+        }
+    }
+}