diff --git a/crates/tui/src/core/engine.rs b/crates/tui/src/core/engine.rs index 9fa110a9..ff053451 100644 --- a/crates/tui/src/core/engine.rs +++ b/crates/tui/src/core/engine.rs @@ -673,6 +673,16 @@ impl Engine { } } } + + // #420: graceful MCP shutdown — send SIGTERM and give stdio servers + // a brief window to exit before drop fires SIGKILL via kill_on_drop. + // Best-effort: pool may not exist (no MCP configured) and the lock + // can fail under contention; either way the kill_on_drop fallback + // still reaps the children. + if let Some(pool) = self.mcp_pool.as_ref() { + let mut guard = pool.lock().await; + guard.shutdown_all().await; + } } async fn emit_session_updated(&self) { diff --git a/crates/tui/src/mcp.rs b/crates/tui/src/mcp.rs index e830db47..b9a595e4 100644 --- a/crates/tui/src/mcp.rs +++ b/crates/tui/src/mcp.rs @@ -260,14 +260,51 @@ pub enum ConnectionState { pub trait McpTransport: Send + Sync { async fn send(&mut self, msg: serde_json::Value) -> Result<()>; async fn recv(&mut self) -> Result; + + /// Graceful shutdown — stdio transports send SIGTERM to the child and + /// give it a brief window to exit before tokio's `kill_on_drop` fires + /// SIGKILL as the backstop. Default is a no-op for non-stdio transports + /// that have no child process. Whalescale#420. + async fn shutdown(&mut self) {} } pub struct StdioTransport { - _child: Child, + child: Child, stdin: ChildStdin, reader: tokio::io::BufReader, } +/// How long `StdioTransport::shutdown` waits for the child to exit on SIGTERM +/// before `kill_on_drop` fires SIGKILL. Tuned short so a hung MCP server +/// can't stall TUI exit; well-behaved servers almost always exit within +/// a few hundred ms. +const STDIO_SHUTDOWN_GRACE: Duration = Duration::from_millis(2_000); + +/// Best-effort SIGTERM. On Unix uses `libc::kill`; on Windows there's no +/// equivalent so we let `kill_on_drop` (TerminateProcess) handle it via the +/// subsequent Drop. Returns whether a signal was actually sent. +fn send_sigterm(child: &Child) -> bool { + #[cfg(unix)] + { + if let Some(pid) = child.id() { + // SAFETY: pid was just obtained from `child.id()`. `libc::kill` + // with `SIGTERM` is async-signal-safe and never observes invalid + // memory. Worst case (pid wrap / process already gone) returns + // ESRCH, which we deliberately ignore. + unsafe { + let _ = libc::kill(pid as i32, libc::SIGTERM); + } + return true; + } + false + } + #[cfg(not(unix))] + { + let _ = child; + false + } +} + #[async_trait::async_trait] impl McpTransport for StdioTransport { async fn send(&mut self, msg: serde_json::Value) -> Result<()> { @@ -296,6 +333,25 @@ impl McpTransport for StdioTransport { } } } + + /// Send SIGTERM and wait up to `STDIO_SHUTDOWN_GRACE` for graceful exit + /// before letting Drop / `kill_on_drop` fire SIGKILL as the backstop. + async fn shutdown(&mut self) { + send_sigterm(&self.child); + // Give the child a window to exit cleanly. Discard the result — + // either it exits (success) or the timeout fires (Drop will SIGKILL). + let _ = tokio::time::timeout(STDIO_SHUTDOWN_GRACE, self.child.wait()).await; + } +} + +/// Drop fallback (#420): if `shutdown` was never called explicitly, still +/// fire SIGTERM before tokio's `kill_on_drop` sends SIGKILL. The two +/// signals arrive back-to-back so well-behaved servers at least see the +/// SIGTERM first; misbehaving ones get SIGKILL'd anyway. +impl Drop for StdioTransport { + fn drop(&mut self) { + send_sigterm(&self.child); + } } pub struct SseTransport { @@ -552,7 +608,7 @@ impl McpConnection { let stdout = child.stdout.take().context("Failed to get MCP stdout")?; Box::new(StdioTransport { - _child: child, + child, stdin, reader: tokio::io::BufReader::new(stdout), }) @@ -1413,6 +1469,25 @@ impl McpPool { self.connections.clear(); } + /// Graceful shutdown of every connection in the pool: send SIGTERM to + /// each stdio child and give them a short grace period before drop + /// fires SIGKILL. Whalescale#420. + /// + /// Call from the TUI exit path *before* dropping the pool to give + /// MCP servers a chance to flush state. The fallback Drop on + /// `StdioTransport` still sends SIGTERM if this never runs, so even + /// abnormal exits avoid leaking PIDs without a signal. + #[allow(dead_code)] // Wired in by callers that want graceful shutdown + pub async fn shutdown_all(&mut self) { + let names: Vec = self.connections.keys().cloned().collect(); + for name in names { + if let Some(conn) = self.connections.get_mut(&name) { + conn.transport.shutdown().await; + } + } + self.connections.clear(); + } + /// Get the underlying configuration #[allow(dead_code)] // Public API for MCP consumers pub fn config(&self) -> &McpConfig { @@ -1980,4 +2055,51 @@ mod tests { "non-secret preserved: {redacted}" ); } + + /// #420: `StdioTransport::shutdown` reaps the child process by sending + /// SIGTERM and giving it a brief grace period before drop fires SIGKILL. + /// The test spawns `cat` (which exits immediately on stdin EOF / SIGTERM) + /// and verifies the transport tears down cleanly. Unix-only because + /// SIGTERM doesn't exist on Windows; on Windows the test would just + /// duplicate the kill_on_drop path. + #[cfg(unix)] + #[tokio::test] + async fn stdio_transport_shutdown_terminates_child() { + use tokio::process::Command as TokioCommand; + let mut cmd = TokioCommand::new("cat"); + cmd.stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()) + .kill_on_drop(true); + let mut child = cmd.spawn().expect("spawn cat"); + let pid = child.id().expect("child pid"); + let stdin = child.stdin.take().expect("child stdin"); + let stdout = child.stdout.take().expect("child stdout"); + let mut transport = StdioTransport { + child, + stdin, + reader: tokio::io::BufReader::new(stdout), + }; + + // shutdown() should send SIGTERM and complete within the grace window. + let start = std::time::Instant::now(); + transport.shutdown().await; + let elapsed = start.elapsed(); + assert!( + elapsed < STDIO_SHUTDOWN_GRACE + Duration::from_millis(500), + "shutdown blocked beyond grace window: {elapsed:?}" + ); + + // The child should be reaped — kill(pid, 0) returning ESRCH means + // the pid is gone. If it's still alive, kill(0) returns 0, which + // means our shutdown didn't terminate it. + // SAFETY: pid was just collected from a tokio Child we spawned. + // libc::kill with signal 0 only checks pid existence and is + // async-signal-safe. + let still_alive = unsafe { libc::kill(pid as i32, 0) } == 0; + assert!( + !still_alive, + "child {pid} survived StdioTransport::shutdown — SIGTERM not delivered" + ); + } } diff --git a/crates/tui/src/sandbox/seatbelt.rs b/crates/tui/src/sandbox/seatbelt.rs index 6af5e0b2..ee3b762c 100644 --- a/crates/tui/src/sandbox/seatbelt.rs +++ b/crates/tui/src/sandbox/seatbelt.rs @@ -174,9 +174,43 @@ fn generate_policy(policy: &SandboxPolicy, cwd: &Path) -> String { full_policy.push('\n'); full_policy.push_str(r#"(allow file-read* (subpath "/private/var/db"))"#); + // Cargo home (#558): cargo build/test/publish reach into ~/.cargo/registry + // and ~/.cargo/git for crate metadata, downloaded tarballs, and unpacked + // sources. Sandboxed workspace-write was previously rejecting these, + // making `cargo publish` unrunnable from inside the TUI's shell tool. + // Read access is always allowed; write access is granted whenever the + // policy allows any write at all (the registry caches need to be + // mutable for `cargo build` to populate them on a cache miss). Skipped + // entirely when neither `CARGO_HOME` nor `HOME` is set — without one of + // those we have no path to plumb into the policy params. + if resolve_cargo_home().is_some() { + full_policy.push_str("\n\n; Cargo home (~/.cargo) — registry/index/git caches\n"); + full_policy.push_str(r#"(allow file-read* (subpath (param "CARGO_HOME")))"#); + if !matches!(policy, SandboxPolicy::ReadOnly) { + full_policy.push('\n'); + full_policy.push_str(r#"(allow file-write* (subpath (param "CARGO_HOME_REGISTRY")))"#); + full_policy.push('\n'); + full_policy.push_str(r#"(allow file-write* (subpath (param "CARGO_HOME_GIT")))"#); + } + } + full_policy } +/// Resolve the user's cargo home — `CARGO_HOME` if set, else `$HOME/.cargo`. +/// Returns `None` only on hosts where neither env var is set (essentially +/// never on a real macOS user account; can happen in CI containers without +/// `HOME` exported). +fn resolve_cargo_home() -> Option { + if let Ok(explicit) = std::env::var("CARGO_HOME") + && !explicit.trim().is_empty() + { + return Some(PathBuf::from(explicit)); + } + let home = std::env::var("HOME").ok()?; + Some(PathBuf::from(home).join(".cargo")) +} + /// Generate the write access portion of the Seatbelt policy. fn generate_write_policy(policy: &SandboxPolicy, cwd: &Path) -> String { // Full disk write access @@ -264,6 +298,21 @@ fn generate_params(policy: &SandboxPolicy, cwd: &Path) -> Vec<(String, PathBuf)> } } + // Cargo home (#558): paired with the policy lines emitted by + // `generate_policy` when `resolve_cargo_home()` succeeds. Both helpers + // use the same fallback chain so the policy text and the -DKEY=VALUE + // params stay in sync — emit one without the other and sandbox-exec + // refuses to load the profile. + if let Some(home) = resolve_cargo_home() { + let canonical_home = home.canonicalize().unwrap_or_else(|_| home.clone()); + params.push(( + "CARGO_HOME_REGISTRY".to_string(), + canonical_home.join("registry"), + )); + params.push(("CARGO_HOME_GIT".to_string(), canonical_home.join("git"))); + params.push(("CARGO_HOME".to_string(), canonical_home)); + } + params } @@ -316,6 +365,12 @@ pub fn detect_denial(exit_code: i32, stderr: &str) -> bool { mod tests { use super::*; + /// Serializes tests that mutate process-global env vars (HOME, CARGO_HOME) + /// so they don't race with each other or with sibling tests in this + /// crate that read those vars. Mirrors the pattern in main.rs::tests + /// (commit d06eaed0). + static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + #[test] fn test_is_available() { // This test just checks the function doesn't panic @@ -368,6 +423,101 @@ mod tests { assert!(params.iter().any(|(k, _)| k == "DARWIN_USER_CACHE_DIR")); } + /// #558: cargo publish reaches into ~/.cargo/registry; the seatbelt has + /// to allow read+write inside it. Both the policy text and the param + /// table must be in sync — emitting one without the other makes + /// sandbox-exec refuse to load the profile. + #[test] + fn test_cargo_home_paths_emitted_in_policy_and_params_when_home_set() { + let _guard = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner()); + + // SAFETY: HOME / CARGO_HOME are process-global. ENV_LOCK serializes + // all tests in this module that mutate them, and we always restore + // the prior value before returning. + let saved_home = std::env::var_os("HOME"); + let saved_cargo = std::env::var_os("CARGO_HOME"); + unsafe { + std::env::set_var("HOME", "/tmp/seatbelt-cargo-test"); + std::env::remove_var("CARGO_HOME"); + } + + let policy = SandboxPolicy::default(); + let cwd = Path::new("/tmp/test"); + + let policy_text = generate_policy(&policy, cwd); + assert!(policy_text.contains(r#"(allow file-read* (subpath (param "CARGO_HOME")))"#)); + assert!(policy_text.contains("CARGO_HOME_REGISTRY")); + assert!(policy_text.contains("CARGO_HOME_GIT")); + + let params = generate_params(&policy, cwd); + assert!(params.iter().any(|(k, _)| k == "CARGO_HOME")); + assert!(params.iter().any(|(k, _)| k == "CARGO_HOME_REGISTRY")); + assert!(params.iter().any(|(k, _)| k == "CARGO_HOME_GIT")); + + // Read-only policy should still emit CARGO_HOME read rule but skip writes. + let read_only_text = generate_policy(&SandboxPolicy::ReadOnly, cwd); + assert!( + read_only_text.contains(r#"(allow file-read* (subpath (param "CARGO_HOME")))"#), + "read-only mode should still allow reading the cargo registry: {read_only_text}" + ); + assert!( + !read_only_text + .contains(r#"(allow file-write* (subpath (param "CARGO_HOME_REGISTRY")))"#), + "read-only mode must NOT grant write access to the cargo registry" + ); + + // Restore. + // SAFETY: restoring the prior value the test stashed at entry. + unsafe { + match saved_home { + Some(v) => std::env::set_var("HOME", v), + None => std::env::remove_var("HOME"), + } + match saved_cargo { + Some(v) => std::env::set_var("CARGO_HOME", v), + None => std::env::remove_var("CARGO_HOME"), + } + } + } + + /// #558: if neither `CARGO_HOME` nor `HOME` is set, the cargo lines and + /// their params must both be omitted — emitting one without the other + /// would crash sandbox-exec on profile load. + #[test] + fn test_cargo_home_skipped_when_no_env() { + let _guard = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner()); + + let saved_home = std::env::var_os("HOME"); + let saved_cargo = std::env::var_os("CARGO_HOME"); + // SAFETY: HOME/CARGO_HOME are process-global; ENV_LOCK serializes + // mutations here and we restore the prior values before returning. + unsafe { + std::env::remove_var("HOME"); + std::env::remove_var("CARGO_HOME"); + } + + let policy = SandboxPolicy::default(); + let cwd = Path::new("/tmp/test"); + let policy_text = generate_policy(&policy, cwd); + let params = generate_params(&policy, cwd); + + assert!(!policy_text.contains("CARGO_HOME")); + assert!(!params.iter().any(|(k, _)| k.starts_with("CARGO_HOME"))); + + // Restore. + // SAFETY: restoring the prior values the test stashed at entry. + unsafe { + match saved_home { + Some(v) => std::env::set_var("HOME", v), + None => std::env::remove_var("HOME"), + } + match saved_cargo { + Some(v) => std::env::set_var("CARGO_HOME", v), + None => std::env::remove_var("CARGO_HOME"), + } + } + } + #[test] fn test_create_seatbelt_args() { let policy = SandboxPolicy::default(); diff --git a/crates/tui/src/tools/shell.rs b/crates/tui/src/tools/shell.rs index 7518c6d8..0377a036 100644 --- a/crates/tui/src/tools/shell.rs +++ b/crates/tui/src/tools/shell.rs @@ -139,6 +139,47 @@ fn kill_child_process_group(child: &mut Child) -> std::io::Result<()> { } } +/// Configure parent-death signaling so shell-spawned children are reaped when +/// the TUI dies abnormally (#421). On Linux this installs +/// `PR_SET_PDEATHSIG(SIGTERM)` via `pre_exec` — the kernel then sends SIGTERM +/// to the child the moment the parent process exits, even on SIGKILL of the +/// TUI. The cancellation path already SIGKILLs the whole process group, so +/// this only fires when the parent dies without running its drop / cleanup +/// code (panic during shutdown, OOM, hardware crash, etc.). +/// +/// On macOS / Windows there's no kernel equivalent. The existing graceful +/// path (`kill_child_process_group` from the cancellation token) still +/// handles normal shutdown; abnormal exit can leak children — tracked as a +/// follow-up watchdog item per the original issue's acceptance criteria. +#[cfg(target_os = "linux")] +fn install_parent_death_signal(cmd: &mut Command) { + use std::os::unix::process::CommandExt; + // SAFETY: `pre_exec` runs in the child between fork and exec. The closure + // only calls `libc::prctl` with stack-allocated constant arguments and + // does not touch heap memory or the parent's locks. Both requirements + // (async-signal-safe + no allocation in the post-fork window) are met. + unsafe { + cmd.pre_exec(|| { + let result = libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGTERM, 0, 0, 0); + if result == -1 { + // Surface the errno but do not abort the spawn — the child + // will simply lose the parent-death cleanup safety net. + Err(std::io::Error::last_os_error()) + } else { + Ok(()) + } + }); + } +} + +#[cfg(not(target_os = "linux"))] +fn install_parent_death_signal(_cmd: &mut Command) { + // No kernel-level equivalent on macOS / Windows. The cooperative + // cancellation + process_group SIGKILL path covers normal shutdown; + // abnormal exit (panic without unwind, SIGKILL of the TUI) can still + // leak children on those platforms — tracked as a follow-up. +} + #[derive(Clone, Copy, Debug)] struct ShellExitStatus { code: Option, @@ -669,6 +710,7 @@ impl ShellManager { { cmd.process_group(0); } + install_parent_death_signal(&mut cmd); if stdin_data.is_some() { cmd.stdin(Stdio::piped()); @@ -811,6 +853,7 @@ impl ShellManager { { cmd.process_group(0); } + install_parent_death_signal(&mut cmd); for (key, value) in &exec_env.env { cmd.env(key, value); diff --git a/crates/tui/src/tools/subagent/mod.rs b/crates/tui/src/tools/subagent/mod.rs index c6d3121b..659f71c0 100644 --- a/crates/tui/src/tools/subagent/mod.rs +++ b/crates/tui/src/tools/subagent/mod.rs @@ -56,31 +56,59 @@ const SUBAGENT_RESTART_REASON: &str = "Interrupted by process restart"; const VALID_SUBAGENT_TYPES: &str = "general, explore, plan, review, implementer, verifier, custom, \ worker, explorer, awaiter, default, implement, builder, verify, validator, tester"; +/// Whale species names rotated through `whale_nickname_for_index` to label +/// sub-agents in the UI. English and Simplified-Chinese names are interleaved +/// so any newly spawned agent has a roughly even chance of either — the goal +/// is friendly variety, not a strict locale match. pub const WHALE_NICKNAMES: &[&str] = &[ "Blue", + "蓝鲸", "Humpback", + "座头鲸", "Sperm", + "抹香鲸", "Fin", + "长须鲸", "Sei", + "塞鲸", "Bryde's", + "布氏鲸", "Minke", + "小须鲸", "Antarctic Minke", + "南极小须鲸", "Gray", + "灰鲸", "Bowhead", + "弓头鲸", "North Atlantic Right", + "北大西洋露脊鲸", "North Pacific Right", + "北太平洋露脊鲸", "Southern Right", + "南露脊鲸", "Beluga", + "白鲸", "Narwhal", + "独角鲸", "Orca", + "虎鲸", "Pilot", + "领航鲸", "False Killer", + "伪虎鲸", "Pygmy Killer", + "小虎鲸", "Melon-headed", + "瓜头鲸", "Beaked", + "喙鲸", "Cuvier's Beaked", + "柯氏喙鲸", "Baird's Beaked", + "贝氏喙鲸", "Blainville's Beaked", + "柏氏喙鲸", ]; /// Removal version for deprecated tool aliases. diff --git a/crates/tui/src/tui/app.rs b/crates/tui/src/tui/app.rs index ea857059..18130bca 100644 --- a/crates/tui/src/tui/app.rs +++ b/crates/tui/src/tui/app.rs @@ -46,6 +46,10 @@ use crate::utils::is_chinese_system_locale; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum OnboardingState { Welcome, + /// Pick the UI locale before any other config decisions (#566). + /// Defaults to auto-detection from `LC_ALL` / `LANG`; explicit picks + /// land in `~/.deepseek/settings.toml` via `Settings::set("locale", …)`. + Language, ApiKey, TrustDirectory, Tips, @@ -1262,6 +1266,31 @@ impl App { self.needs_redraw = true; } + /// Apply a locale tag selected from the onboarding language picker (#566). + /// Persists the value to `~/.deepseek/settings.toml` and immediately + /// re-resolves `ui_locale` so the rest of onboarding renders in the new + /// language. `App` doesn't keep `Settings` resident — it loads on entry + /// and rewrites on exit, mirroring the pattern used by the `/config` + /// surface. + pub fn set_locale_from_onboarding(&mut self, tag: &str) -> anyhow::Result<()> { + let mut settings = Settings::load().unwrap_or_else(|_| Settings::default()); + settings.set("locale", tag)?; + settings.save()?; + self.ui_locale = crate::localization::resolve_locale(&settings.locale); + self.needs_redraw = true; + Ok(()) + } + + /// Locale tag currently persisted in `~/.deepseek/settings.toml` (or + /// `"auto"` when no settings file exists). Used by the onboarding + /// language picker to highlight the current selection without `App` + /// having to keep `Settings` resident. + pub fn current_locale_tag(&self) -> String { + Settings::load() + .map(|s| s.locale) + .unwrap_or_else(|_| "auto".to_string()) + } + pub fn set_mode(&mut self, mode: AppMode) -> bool { let previous_mode = self.mode; if previous_mode == mode { diff --git a/crates/tui/src/tui/onboarding/language.rs b/crates/tui/src/tui/onboarding/language.rs new file mode 100644 index 00000000..5e4409de --- /dev/null +++ b/crates/tui/src/tui/onboarding/language.rs @@ -0,0 +1,98 @@ +//! Language picker for first-run onboarding (#566). +//! +//! Surfaces every locale the TUI ships translations for, plus an `auto` +//! option that defers to `LC_ALL` / `LANG`. Selection persists via +//! `Settings::save` immediately so the rest of onboarding (and every +//! subsequent session) reads the chosen tag. + +use ratatui::style::{Modifier, Style}; +use ratatui::text::{Line, Span}; + +use crate::palette; +use crate::tui::app::App; + +/// Locale options shown in the picker. Order matches the keyboard hotkeys +/// (1-5). Each entry is `(hotkey, settings_tag, native_name, english_label)`. +/// `settings_tag` is what `Settings::set("locale", …)` accepts and what +/// `localization::Locale` resolves on next read. +pub const LANGUAGE_OPTIONS: &[(char, &str, &str, &str)] = &[ + ('1', "auto", "Auto-detect", "(LC_ALL / LANG)"), + ('2', "en", "English", ""), + ('3', "ja", "日本語", "(Japanese)"), + ('4', "zh-Hans", "简体中文", "(Simplified Chinese)"), + ('5', "pt-BR", "Português (Brasil)", "(Brazilian Portuguese)"), +]; + +pub fn lines(app: &App) -> Vec> { + let current_owned = app.current_locale_tag(); + let current = current_owned.as_str(); + + let mut out: Vec> = vec![ + Line::from(Span::styled( + "Choose your language", + Style::default() + .fg(palette::DEEPSEEK_SKY) + .add_modifier(Modifier::BOLD), + )), + Line::from(""), + Line::from(Span::styled( + "Pick the UI language. You can change it any time with `/settings set locale `.", + Style::default().fg(palette::TEXT_MUTED), + )), + Line::from(""), + ]; + + for (hotkey, tag, native, english) in LANGUAGE_OPTIONS { + let is_current = current == *tag; + let bullet = if is_current { "●" } else { "○" }; + let bullet_color = if is_current { + palette::DEEPSEEK_BLUE + } else { + palette::TEXT_MUTED + }; + let mut spans: Vec> = vec![ + Span::styled(format!(" {bullet} "), Style::default().fg(bullet_color)), + Span::styled( + format!("[{hotkey}] "), + Style::default() + .fg(palette::TEXT_PRIMARY) + .add_modifier(Modifier::BOLD), + ), + Span::styled( + native.to_string(), + Style::default().fg(palette::TEXT_PRIMARY), + ), + ]; + if !english.is_empty() { + spans.push(Span::styled( + format!(" {english}"), + Style::default().fg(palette::TEXT_MUTED), + )); + } + out.push(Line::from(spans)); + } + + out.push(Line::from("")); + out.push(Line::from(vec![ + Span::styled("Press ", Style::default().fg(palette::TEXT_MUTED)), + Span::styled( + "1-5", + Style::default() + .fg(palette::TEXT_PRIMARY) + .add_modifier(Modifier::BOLD), + ), + Span::styled(" to choose, or ", Style::default().fg(palette::TEXT_MUTED)), + Span::styled( + "Enter", + Style::default() + .fg(palette::TEXT_PRIMARY) + .add_modifier(Modifier::BOLD), + ), + Span::styled( + " to keep the current setting", + Style::default().fg(palette::TEXT_MUTED), + ), + ])); + + out +} diff --git a/crates/tui/src/tui/onboarding/mod.rs b/crates/tui/src/tui/onboarding/mod.rs index 6f805759..47fa694b 100644 --- a/crates/tui/src/tui/onboarding/mod.rs +++ b/crates/tui/src/tui/onboarding/mod.rs @@ -1,6 +1,7 @@ //! Onboarding flow rendering and helpers. pub mod api_key; +pub mod language; pub mod trust_directory; pub mod welcome; @@ -32,6 +33,7 @@ pub fn render(f: &mut Frame, area: Rect, app: &App) { let lines = match app.onboarding { OnboardingState::Welcome => welcome::lines(), + OnboardingState::Language => language::lines(app), OnboardingState::ApiKey => api_key::lines(app), OnboardingState::TrustDirectory => trust_directory::lines(app), OnboardingState::Tips => tips_lines(), @@ -66,7 +68,8 @@ pub fn render(f: &mut Frame, area: Rect, app: &App) { fn onboarding_step(app: &App) -> (usize, usize) { let needs_trust = !app.trust_mode && needs_trust(&app.workspace); - let mut total = 2; // Welcome + Tips + // Welcome + Language + Tips are always shown. + let mut total = 3; if app.onboarding_needs_api_key { total += 1; } @@ -76,13 +79,11 @@ fn onboarding_step(app: &App) -> (usize, usize) { let step = match app.onboarding { OnboardingState::Welcome => 1, - OnboardingState::ApiKey => 2, + OnboardingState::Language => 2, + OnboardingState::ApiKey => 3, OnboardingState::TrustDirectory => { - if app.onboarding_needs_api_key { - 3 - } else { - 2 - } + // Welcome (1) + Language (2) + optional ApiKey + if app.onboarding_needs_api_key { 4 } else { 3 } } OnboardingState::Tips => total, OnboardingState::None => total, diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index 6f51976d..b9976723 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -1559,7 +1559,10 @@ async fn run_event_loop( // Handle onboarding flow if app.onboarding != OnboardingState::None { - let advance_onboarding = |app: &mut App| { + // After Welcome (and the new Language step) we route to either + // the API-key step, the trust prompt, or the tips screen + // depending on what the user still needs to set up. + let advance_after_language = |app: &mut App| { app.status_message = None; if app.onboarding_needs_api_key { app.onboarding = OnboardingState::ApiKey; @@ -1569,6 +1572,10 @@ async fn run_event_loop( app.onboarding = OnboardingState::Tips; } }; + let advance_onboarding = |app: &mut App| { + app.status_message = None; + app.onboarding = OnboardingState::Language; + }; match key.code { KeyCode::Char('c') if key.modifiers.contains(KeyModifiers::CONTROL) => { @@ -1581,10 +1588,42 @@ async fn run_event_loop( app.api_key_cursor = 0; app.status_message = None; } + KeyCode::Esc if app.onboarding == OnboardingState::Language => { + app.onboarding = OnboardingState::Welcome; + app.status_message = None; + } + // Language picker hotkeys: 1-5 select + persist (#566). + KeyCode::Char(c) + if app.onboarding == OnboardingState::Language + && c.is_ascii_digit() + && let Some((_, tag, _, _)) = + onboarding::language::LANGUAGE_OPTIONS + .iter() + .find(|(hotkey, _, _, _)| *hotkey == c) => + { + match app.set_locale_from_onboarding(tag) { + Ok(()) => { + app.push_status_toast( + format!("Language set to {tag}"), + StatusToastLevel::Info, + Some(2_500), + ); + advance_after_language(app); + } + Err(err) => { + app.status_message = Some(format!("Failed to save locale: {err}")); + } + } + } KeyCode::Enter => match app.onboarding { OnboardingState::Welcome => { advance_onboarding(app); } + OnboardingState::Language => { + // Enter without a digit pick keeps the existing + // setting (which defaults to "auto"). + advance_after_language(app); + } OnboardingState::ApiKey => { let key = app.api_key_input.trim().to_string(); if let ApiKeyValidation::Reject(message) =