From 716f45cfbcb6d1cdfafc5a69fd9d8fefb879e50c Mon Sep 17 00:00:00 2001 From: Hunter Bown Date: Mon, 11 May 2026 14:31:41 -0500 Subject: [PATCH] fix(ui): Ctrl+O guard accepts extra modifier bits + AGENTS two-binary note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Ctrl+O thinking-pager arm guarded on `key.modifiers == KeyModifiers::CONTROL` (exact match), so any additional modifier bit set by the terminal — Shift while a native-selection mouse bypass was active, Caps Lock indicator on some keyboard layouts — silently fell through to the $EDITOR arm at ui.rs:2833 and did nothing visible when the composer was empty. The user saw the "thinking collapsed; press Ctrl+O for full text" affordance, pressed it, and the handler appeared to ignore them. Relaxed to `contains(KeyModifiers::CONTROL)` to match the established pattern at Ctrl+P (ui.rs:2068) and Ctrl+B (ui.rs:2077). With the existing `app.input.is_empty()` guard preserved, the $EDITOR arm still owns the non-empty-composer case, so the two handlers continue to partition Ctrl+O cleanly. Also documents the two-binary install gotcha in AGENTS.md: the CLI dispatcher (`crates/cli` → `deepseek`) and the TUI runtime (`crates/tui` → `deepseek-tui`) ship as separate executables, and `cargo install --path crates/cli` alone leaves the TUI stale — which is how both this fix and the active_cell fix from dc2433a8b initially appeared to be no-ops during local maintainer testing. The release pipeline packages both binaries, so end users were never affected by that side; this is purely a maintainer-local footgun and is now spelled out for future agents. Extends the existing v0.8.29 CHANGELOG entry to credit both halves of the Ctrl+O fix. --- AGENTS.md | 6 ++++++ CHANGELOG.md | 28 +++++++++++++++++----------- crates/tui/src/tui/ui.rs | 2 +- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6d541cd7..3e882972 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,6 +12,12 @@ This file provides context for AI assistants working on this project. - Run (canonical): `deepseek` — use the **`deepseek` binary**, not `deepseek-tui`. The dispatcher delegates to the TUI for interactive use and is the supported entry point for every flow (`deepseek`, `deepseek -p "..."`, `deepseek doctor`, `deepseek mcp …`, etc.). - Run from source: `cargo run --bin deepseek` (or `cargo run -p deepseek-tui-cli`). - Local dev shorthand: after `cargo build --release`, run `./target/release/deepseek`. +- **Two binaries, two installs.** `deepseek` (the CLI dispatcher, `crates/cli`) and `deepseek-tui` (the TUI runtime, `crates/tui`) ship as **separate executables**. The dispatcher resolves and spawns `deepseek-tui` as a sibling on PATH for interactive use, so installing only the CLI leaves the TUI stale and your fix won't appear to run. Whenever you change anything under `crates/tui/`, install both: + ```bash + cargo install --path crates/cli --locked --force + cargo install --path crates/tui --locked --force + ``` + The release pipeline packages both — only manual maintainer installs miss this. If a fix you just made "isn't taking effect," check `stat -f '%Sm' ~/.cargo/bin/deepseek-tui` before reaching for `tracing::debug!`. ### Build Dependencies - **Rust** 1.88+ (the workspace declares `rust-version = "1.88"` because we diff --git a/CHANGELOG.md b/CHANGELOG.md index 9482793e..13691e02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -87,17 +87,23 @@ coverage additions. after tool calls when users keep the onboarding default model alias. - **`Ctrl+O` expands thinking blocks still in flight.** - `open_thinking_pager` only searched `app.history`, but after - `ThinkingComplete` the finalized thinking entry sits in - `app.active_cell` with `streaming = false` until the active cell - flushes at end-of-turn. During that window the transcript still - rendered the "thinking collapsed; press Ctrl+O for full text" - affordance from `render_thinking`, so the handler surfaced - "No thinking blocks to expand" while the affordance pointed at - it. Routed through the existing `cell_at_virtual_index` / - `virtual_cell_count` resolver that `open_tool_details_pager` - already uses; selection-based and most-recent lookups both reach - in-flight entries now. Regression-guarded by + Two compounding bugs were making the "thinking collapsed; press + Ctrl+O for full text" affordance a lie. (1) `open_thinking_pager` + only searched `app.history`, but after `ThinkingComplete` the + finalized thinking entry sits in `app.active_cell` with + `streaming = false` until the active cell flushes at end-of-turn; + during that window the handler surfaced "No thinking blocks to + expand" while the affordance pointed at the live entry. Routed + through the existing `cell_at_virtual_index` / `virtual_cell_count` + resolver that `open_tool_details_pager` already uses, so + selection-based and most-recent lookups both reach in-flight + entries. (2) The keybinding guard required `key.modifiers == + KeyModifiers::CONTROL` (exact match), so any extra modifier bit + set by the terminal — Shift while a native-selection bypass was + active, Caps Lock indicator on some keyboard layouts — silently + fell through to the `$EDITOR` arm and did nothing visible on an + empty composer. Relaxed to `contains(KeyModifiers::CONTROL)` to + match the existing Ctrl+P / Ctrl+B pattern. Regression-guarded by `open_thinking_pager_finds_thinking_in_active_cell`. - **Skill completions no longer flood the top-level slash menu** (#1437, PR #1442 from **@reidliu41**). Installed skills now diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index 34fa5779..c1e72dac 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -2244,7 +2244,7 @@ async fn run_event_loop( continue; } KeyCode::Char('o') - if key.modifiers == KeyModifiers::CONTROL + if key.modifiers.contains(KeyModifiers::CONTROL) && app.input.is_empty() && open_thinking_pager(app) => {