From c270ef81ef504fa401fbe0af565c6dd39fc1ed14 Mon Sep 17 00:00:00 2001 From: Hunter Bown Date: Thu, 7 May 2026 03:48:09 -0500 Subject: [PATCH] fix(tui): harden terminal resume and runtime context Summary: - Keep default auto alternate-screen mode inside the TUI so transcript scrolling stays app-owned unless users explicitly opt out. - Queue terminal resume events when the engine channel is full, avoiding stranded paused terminal state after interactive tool cancellation or bursts. - Scope crash-checkpoint recovery to the resolved launch workspace instead of the shell cwd. - Add runtime deepseek_version to the prompt environment block so agents can distinguish installed runtime identity from a stale checkout. Test plan: - cargo test -p deepseek-tui --locked on a simulated merge with current main - cargo fmt --all -- --check - git diff --check - Existing PR CI was green for lint, version drift, Linux/macOS/Windows tests, npm wrapper smoke, and GitGuardian. --- config.example.toml | 2 +- crates/tui/src/core/engine/tool_execution.rs | 73 +++++++++++++++----- crates/tui/src/main.rs | 70 +++++++++++++------ crates/tui/src/prompts.rs | 9 ++- crates/tui/src/prompts/base.md | 4 ++ crates/tui/src/session_manager.rs | 26 ++++++- docs/CONFIGURATION.md | 2 +- 7 files changed, 141 insertions(+), 45 deletions(-) diff --git a/config.example.toml b/config.example.toml index 5fe9492f..f227cb02 100644 --- a/config.example.toml +++ b/config.example.toml @@ -244,7 +244,7 @@ max_subagents = 10 # optional (1-20) # TUI # ───────────────────────────────────────────────────────────────────────────────── [tui] -alternate_screen = "auto" # auto | always | never +alternate_screen = "auto" # auto/always use the TUI screen; never uses terminal scrollback mouse_capture = true # true copies only transcript user/assistant text; false uses raw terminal selection/copy terminal_probe_timeout_ms = 500 # optional startup terminal-mode timeout (100-5000ms) osc8_links = true # emit OSC 8 escapes around URLs (Cmd+click in iTerm2/Ghostty/Kitty/WezTerm/Terminal.app 13+); set false for terminals that misrender diff --git a/crates/tui/src/core/engine/tool_execution.rs b/crates/tui/src/core/engine/tool_execution.rs index 9497f3cc..e1a41343 100644 --- a/crates/tui/src/core/engine/tool_execution.rs +++ b/crates/tui/src/core/engine/tool_execution.rs @@ -25,11 +25,11 @@ use super::*; /// over, mouse wheel scrolled the host terminal instead of the transcript, /// and the TUI rendered as if into a regular cooked-mode buffer. /// -/// `Drop` runs synchronously and can't await, so we use `try_send` on a -/// **clone of the event channel** to push `ResumeEvents` non-blockingly. The -/// engine event channel is the same one we sent `PauseEvents` on, so by the -/// time we drop there is by construction at least one consumed slot, which -/// keeps `try_send` reliable in practice. +/// `Drop` runs synchronously and can't await, so we first use `try_send` on a +/// **clone of the event channel** to push `ResumeEvents` non-blockingly. If the +/// channel is full we enqueue the resume on the active Tokio runtime instead of +/// dropping it; otherwise a burst of engine events can strand the UI in the +/// paused terminal state. pub(super) struct InteractiveTerminalGuard { tx: Option>, } @@ -52,18 +52,40 @@ impl InteractiveTerminalGuard { impl Drop for InteractiveTerminalGuard { fn drop(&mut self) { if let Some(tx) = self.tx.take() { - // Synchronous, non-blocking. If the channel is full we still want - // the resume to land — log so a cancellation that loses the - // resume is visible in traces, but don't panic. The TUI also - // re-sends a resume on its own teardown path as a backstop. - if let Err(err) = tx.try_send(Event::ResumeEvents) { - tracing::warn!( - target: "engine.tool_execution", - ?err, - "InteractiveTerminalGuard: try_send(ResumeEvents) failed; \ - terminal may stay in paused state until the next \ - pause/resume cycle" - ); + match tx.try_send(Event::ResumeEvents) { + Ok(()) => {} + Err(tokio::sync::mpsc::error::TrySendError::Full(event)) => { + match tokio::runtime::Handle::try_current() { + Ok(handle) => { + handle.spawn(async move { + if let Err(err) = tx.send(event).await { + tracing::warn!( + target: "engine.tool_execution", + ?err, + "InteractiveTerminalGuard: async send(ResumeEvents) failed; \ + terminal may stay in paused state until the next \ + pause/resume cycle" + ); + } + }); + } + Err(err) => { + tracing::warn!( + target: "engine.tool_execution", + ?err, + "InteractiveTerminalGuard: event channel full and no Tokio runtime \ + available to queue ResumeEvents; terminal may stay paused until \ + the next pause/resume cycle" + ); + } + } + } + Err(tokio::sync::mpsc::error::TrySendError::Closed(_)) => { + tracing::debug!( + target: "engine.tool_execution", + "InteractiveTerminalGuard: event channel closed before ResumeEvents" + ); + } } } } @@ -258,7 +280,7 @@ impl Engine { mod tests { use super::*; use serde_json::json; - use std::sync::Mutex; + use std::{sync::Mutex, time::Duration}; /// Tests in this module mutate `DEEPSEEK_TOOL_AUDIT_LOG` which is /// process-global; serialise through this guard so the parallel @@ -269,6 +291,21 @@ mod tests { AUDIT_TEST_GUARD.lock().unwrap_or_else(|e| e.into_inner()) } + #[tokio::test] + async fn terminal_guard_queues_resume_when_event_channel_is_full() { + let (tx, mut rx) = mpsc::channel(1); + tx.try_send(Event::status("filler")).expect("fill channel"); + + drop(InteractiveTerminalGuard { tx: Some(tx) }); + + assert!(matches!(rx.recv().await, Some(Event::Status { .. }))); + let resumed = tokio::time::timeout(Duration::from_secs(1), rx.recv()) + .await + .expect("queued resume event") + .expect("event channel still open"); + assert!(matches!(resumed, Event::ResumeEvents)); + } + #[test] fn emit_tool_audit_writes_jsonl_line_when_env_var_set() { let _g = audit_test_guard(); diff --git a/crates/tui/src/main.rs b/crates/tui/src/main.rs index 286f6410..566e120b 100644 --- a/crates/tui/src/main.rs +++ b/crates/tui/src/main.rs @@ -776,7 +776,8 @@ async fn main() -> Result<()> { Some(id) } else if !cli.fresh { // Check for crash-recovery checkpoint (unless --fresh was passed). - try_recover_checkpoint() + let workspace = resolve_workspace(&cli); + try_recover_checkpoint(&workspace) } else { None }; @@ -3534,7 +3535,7 @@ fn should_use_alt_screen(cli: &Cli, config: &Config) -> bool { match mode.as_str() { "always" => true, "never" => false, - _ => !is_zellij(), + _ => true, } } @@ -3581,16 +3582,12 @@ fn default_mouse_capture_enabled(terminal_emulator: Option<&str>) -> bool { true } -fn is_zellij() -> bool { - std::env::var_os("ZELLIJ").is_some() -} - /// Check for a crash-recovery checkpoint and return the session ID if /// recovery is possible *and* the checkpoint belongs to the current /// workspace. /// /// The checkpoint must exist and its file mtime must be within 24 hours. -/// **The checkpoint's workspace must also match `std::env::current_dir()` +/// **The checkpoint's workspace must also match the resolved launch workspace /// after canonicalisation.** If the workspace doesn't match, the /// checkpoint is persisted as a regular session (so the user can find it /// via `deepseek sessions` / `deepseek resume `) and cleared, and the @@ -3601,7 +3598,7 @@ fn is_zellij() -> bool { /// On a successful match the checkpoint is persisted as a regular session, /// cleared, and a notice is printed to stderr. Returns `None` if there is /// nothing to recover or the workspace doesn't match. -fn try_recover_checkpoint() -> Option { +fn try_recover_checkpoint(launch_workspace: &Path) -> Option { let manager = session_manager::SessionManager::default_location().ok()?; let session = manager.load_checkpoint().ok().flatten()?; @@ -3621,21 +3618,13 @@ fn try_recover_checkpoint() -> Option { return None; } - // Refuse to silently restore a session from another workspace. We compare - // canonicalised paths so that `~/foo` vs `/Users/x/foo` and symlink - // variants resolve consistently. If either side fails to canonicalise - // (e.g. the saved workspace was deleted), fall back to a strict equality - // check on the raw paths. + // Refuse to silently restore a session from another workspace. Compare + // against the resolved launch workspace, not the shell cwd, so callers + // using `--workspace` cannot accidentally recover a checkpoint from the + // directory their shell happened to be in. let session_workspace = session.metadata.workspace.clone(); - let current_workspace = std::env::current_dir().ok()?; - let workspace_matches = { - let lhs = std::fs::canonicalize(&session_workspace).ok(); - let rhs = std::fs::canonicalize(¤t_workspace).ok(); - match (lhs, rhs) { - (Some(a), Some(b)) => a == b, - _ => session_workspace == current_workspace, - } - }; + let workspace_matches = + session_manager::workspace_scope_matches(&session_workspace, launch_workspace); if !workspace_matches { // Persist the checkpoint so the user can find it via `deepseek @@ -3649,10 +3638,11 @@ fn try_recover_checkpoint() -> Option { "Note: an interrupted session ({}…) from another workspace ({}) is \ available. Run `deepseek resume {}` from there to recover it, or \ use `deepseek sessions` to list all saved sessions. Starting fresh \ - here.", + in {}.", &session_id_for_notice.chars().take(8).collect::(), session_workspace.display(), session_id_for_notice, + launch_workspace.display(), ); return None; } @@ -4368,6 +4358,40 @@ mod terminal_mode_tests { Cli::try_parse_from(args).expect("CLI args should parse") } + #[test] + fn alternate_screen_defaults_on_in_auto_mode() { + let cli = parse_cli(&["deepseek"]); + let config = Config::default(); + + assert!(should_use_alt_screen(&cli, &config)); + } + + #[test] + fn no_alt_screen_flag_disables_alternate_screen() { + let cli = parse_cli(&["deepseek", "--no-alt-screen"]); + let config = Config::default(); + + assert!(!should_use_alt_screen(&cli, &config)); + } + + #[test] + fn config_can_disable_alternate_screen() { + let cli = parse_cli(&["deepseek"]); + let config = Config { + tui: Some(crate::config::TuiConfig { + alternate_screen: Some("never".to_string()), + mouse_capture: None, + terminal_probe_timeout_ms: None, + status_items: None, + osc8_links: None, + notification_condition: None, + }), + ..Config::default() + }; + + assert!(!should_use_alt_screen(&cli, &config)); + } + #[test] #[cfg(not(windows))] fn mouse_capture_defaults_on_when_alternate_screen_is_active() { diff --git a/crates/tui/src/prompts.rs b/crates/tui/src/prompts.rs index b482f89f..71284567 100644 --- a/crates/tui/src/prompts.rs +++ b/crates/tui/src/prompts.rs @@ -39,7 +39,7 @@ pub const HANDOFF_RELATIVE_PATH: &str = ".deepseek/handoff.md"; const INSTRUCTIONS_FILE_MAX_BYTES: usize = 100 * 1024; /// Render a `## Environment` block listing the resolved locale tag, -/// host platform, login shell, and current working directory. +/// runtime version, host platform, login shell, and current working directory. /// /// The block is appended to the workspace-static portion of the /// system prompt (after mode prompt + project context, before @@ -48,6 +48,7 @@ const INSTRUCTIONS_FILE_MAX_BYTES: usize = 100 * 1024; /// guess from the user's first message. `locale_tag` is resolved by /// the caller from `Settings` so this function stays I/O-free. fn render_environment_block(workspace: &Path, locale_tag: &str) -> String { + let deepseek_version = env!("CARGO_PKG_VERSION"); let platform = std::env::consts::OS; let shell = std::env::var("SHELL").unwrap_or_else(|_| "unknown".to_string()); let pwd = workspace.display(); @@ -56,6 +57,7 @@ fn render_environment_block(workspace: &Path, locale_tag: &str) -> String { "## Environment\n\ \n\ - lang: {locale_tag}\n\ + - deepseek_version: {deepseek_version}\n\ - platform: {platform}\n\ - shell: {shell}\n\ - pwd: {pwd}" @@ -523,6 +525,10 @@ mod tests { let block = render_environment_block(tmp.path(), "zh-Hans"); assert!(block.starts_with("## Environment")); assert!(block.contains("- lang: zh-Hans")); + assert!(block.contains(&format!( + "- deepseek_version: {}", + env!("CARGO_PKG_VERSION") + ))); assert!(block.contains(&format!("- pwd: {}", tmp.path().display()))); assert!(block.contains("- platform:")); assert!(block.contains("- shell:")); @@ -548,6 +554,7 @@ mod tests { }; assert!(prompt.contains("## Environment")); assert!(prompt.contains("- lang: ja")); + assert!(prompt.contains("- deepseek_version:")); } #[test] diff --git a/crates/tui/src/prompts/base.md b/crates/tui/src/prompts/base.md index 97cec022..c77bc897 100644 --- a/crates/tui/src/prompts/base.md +++ b/crates/tui/src/prompts/base.md @@ -6,6 +6,10 @@ Use the language indicated by the `lang` field in the `## Environment` section a Code, file paths, identifiers, tool names, environment variables, command-line flags, URLs, and log lines stay in their original form — translating `read_file` to `读取文件` would break tool calls. Only natural-language prose mirrors the user. +## Runtime Identity + +If the user asks what DeepSeek TUI version you are running, use the `deepseek_version` field in the `## Environment` section as the runtime version. Workspace files such as `Cargo.toml` describe the checkout you are inspecting; they may be stale, dirty, or intentionally different from the installed runtime. If those disagree, report both instead of replacing the runtime version with the workspace version. + ## Preamble Rhythm When starting work on a user request, open with a short, momentum-building line that names the action you're taking. Keep it reserved — state what you're doing, not how you feel about it. diff --git a/crates/tui/src/session_manager.rs b/crates/tui/src/session_manager.rs index d130fe8b..4c82ab1d 100644 --- a/crates/tui/src/session_manager.rs +++ b/crates/tui/src/session_manager.rs @@ -460,7 +460,7 @@ impl SessionManager { } } -fn workspace_scope_matches(saved_workspace: &Path, current_workspace: &Path) -> bool { +pub(crate) fn workspace_scope_matches(saved_workspace: &Path, current_workspace: &Path) -> bool { if paths_equivalent(saved_workspace, current_workspace) { return true; } @@ -1122,6 +1122,30 @@ mod tests { ); } + #[test] + fn workspace_scope_matches_subdirectories_in_same_git_checkout() { + let tmp = tempdir().expect("tempdir"); + let repo = tmp.path().join("repo"); + let nested = repo.join("crates").join("tui"); + fs::create_dir_all(&nested).expect("mkdir nested"); + fs::write(repo.join(".git"), "gitdir: .git/worktrees/repo").expect("write git marker"); + + assert!(workspace_scope_matches(&repo, &nested)); + } + + #[test] + fn workspace_scope_rejects_sibling_git_checkouts() { + let tmp = tempdir().expect("tempdir"); + let first = tmp.path().join("repo-a"); + let second = tmp.path().join("repo-b"); + fs::create_dir_all(&first).expect("mkdir first"); + fs::create_dir_all(&second).expect("mkdir second"); + fs::write(first.join(".git"), "gitdir: .git/worktrees/a").expect("write first marker"); + fs::write(second.join(".git"), "gitdir: .git/worktrees/b").expect("write second marker"); + + assert!(!workspace_scope_matches(&first, &second)); + } + #[test] fn test_offline_queue_round_trip_and_clear() { let tmp = tempdir().expect("tempdir"); diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index c6780eeb..65650ac6 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -420,7 +420,7 @@ If you are upgrading from older releases: - `[notifications].include_summary` (bool, optional): defaults to `false`. When `true`, the notification body includes the elapsed duration and the turn's cost in the configured display currency. -- `tui.alternate_screen` (string, optional): `auto`, `always`, or `never`. `auto` disables the alternate screen in Zellij; `--no-alt-screen` forces inline mode. Set `never` or run with `--no-alt-screen` when you want real terminal scrollback. +- `tui.alternate_screen` (string, optional): `auto`, `always`, or `never`. `auto` and `always` use the TUI-owned alternate screen so transcript scrolling stays inside the app; `--no-alt-screen` forces inline mode. Set `never` or run with `--no-alt-screen` only when you intentionally want real terminal scrollback. - `tui.mouse_capture` (bool, optional, default `true` on non-Windows terminals when the alternate screen is active; `false` on Windows and inside JetBrains JediTerm — PyCharm/IDEA/CLion/etc. — where mouse-event escapes leak into the input stream as garbled text, see #878 / #898): enable internal mouse scrolling, transcript selection, and right-click context actions. TUI-owned drag selection copies only user/assistant transcript text. Set this to `false` or run with `--no-mouse-capture` for raw terminal selection; set it to `true` or run with `--mouse-capture` to opt in anywhere it's defaulted off. - `tui.terminal_probe_timeout_ms` (int, optional, default `500`): startup terminal-mode probe timeout in milliseconds. Values are clamped to `100..=5000`; timeout emits a warning and aborts startup instead of hanging indefinitely. - `tui.osc8_links` (bool, optional, default `true`): emit OSC 8 escape sequences around URLs in transcript output so terminals that support them (iTerm2, Terminal.app 13+, Ghostty, Kitty, WezTerm, Alacritty, recent gnome-terminal/konsole) render them as Cmd+click hyperlinks. Terminals without OSC 8 support render the plain URL and ignore the escape. Set `false` for terminals that misrender the sequence; selection/clipboard output always strips the escapes.