From fa124c7a92fd159f9904937527036505f7704fc4 Mon Sep 17 00:00:00 2001 From: Duyet Le Date: Sat, 5 Sep 2026 00:21:57 +0700 Subject: [PATCH] fix(cli): keep TUI keys, tool yaml, and relay from dropping state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebased onto post-split main (#48): the commands.rs hunks now land in their per-domain cmd/ modules (run_whoami→usage, run_keys→keys, run_launch→launch, launcher_uses_palette/persist_tool_command→dispatch, run_config_tui/settings_tab_index/fill_agent_settings→config_tui, run_menu→menu, dispatch cursor/cline/windsurf→stub). Kept tip-of-main UX (#50-#54) where PR #52's #51-era assumptions diverged: - help stays examples-first + the 'help commands' map (commands_help() retained); --help no longer dumps CORE COMMANDS. - the compact HUD stays the default launcher, so the palette-only 'empty agents shows install' assertion was dropped (the HUD always offers Launch claude). - config_reset_row's ToolCommand row keeps main's interactive prompt rather than PR #52's reset-to-default (newer UX); the should_persist_command guard is still applied so bare builtins are no longer written to config. Carried forward the rest of PR #52: skip_confirm/--ok alias, confirmation skips in keys, claude_fable display in whoami, settings tab via ANYR_TUI_TAB, ToolCommand Install→Mapping, yolo via tool.extra_flag, should_persist_command, plus the parse/relay/spawn/tui fixes and tests. --- src/auth.rs | 14 ++-- src/cmd/config_tui.rs | 39 ++++++++++- src/cmd/dispatch.rs | 25 +++++-- src/cmd/keys.rs | 4 +- src/cmd/launch.rs | 2 +- src/cmd/menu.rs | 4 +- src/cmd/usage.rs | 5 ++ src/commands.rs | 5 +- src/config.rs | 33 ++++++++- src/help.rs | 16 +++-- src/install.rs | 20 +++++- src/onboard.rs | 6 +- src/parse.rs | 57 ++++++++++++++- src/relay.rs | 74 ++++++++++++-------- src/spawn.rs | 143 ++++++++++++++++++++++++++++++++------ src/tui/keys.rs | 48 ++++++++++++- src/tui/live.rs | 17 ++++- src/tui/mod.rs | 18 ++--- src/tui/view.rs | 52 +++++++++++--- tests/cli.rs | 156 ++++++++++++++++++++++++++++++++++++++++++ 20 files changed, 633 insertions(+), 105 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index aca25ba..36bc0ad 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -221,6 +221,7 @@ pub fn start_device_flow(base_url: &str, tool: Option<&str>) -> Result { + assert!( + msg.contains("expired"), + "user-visible expiry text missing: {msg}" + ); + } + other => panic!("expected GiveUp, got {other:?}"), + } assert!(matches!( poll_action(&Ok(DevicePoll::SlowDown), 10, 0, 600), PollAction::SlowDown diff --git a/src/cmd/config_tui.rs b/src/cmd/config_tui.rs index ba1b9f1..1763df9 100644 --- a/src/cmd/config_tui.rs +++ b/src/cmd/config_tui.rs @@ -162,8 +162,9 @@ pub(crate) fn run_config_tui( #[cfg(feature = "native")] { if tui_wants_dump(parsed, env) { + let tab = settings_tab_index(env); let (state, _) = - config_settings_frame(parsed, env, &path, false, &mut CreditsCache::fresh(), 0); + config_settings_frame(parsed, env, &path, false, &mut CreditsCache::fresh(), tab); print!("{}", tui_dump_settings(state, env)); return Ok(0); } @@ -173,6 +174,24 @@ pub(crate) fn run_config_tui( config_menu_loop_legacy(parsed, env, &path) } +#[cfg(feature = "native")] +pub(crate) fn settings_tab_index(env: &BTreeMap) -> usize { + let Some(raw) = env + .get("ANYR_TUI_TAB") + .map(|s| s.trim()) + .filter(|s| !s.is_empty()) + else { + return 0; + }; + if let Ok(i) = raw.parse::() { + return i; + } + settings_tab_names() + .iter() + .position(|t| t.eq_ignore_ascii_case(raw)) + .unwrap_or(0) +} + #[cfg(feature = "native")] pub(crate) fn settings_tab_names() -> Vec { let mut tabs = vec!["general".to_string()]; @@ -440,7 +459,7 @@ pub(crate) fn fill_agent_settings( "not installed".into() }, if present { Tone::Good } else { Tone::Warn }, - SettingKind::Install(id), + SettingKind::Mapping, ); entry( rows, @@ -1232,3 +1251,19 @@ pub(crate) fn run_config( ))), } } + +#[cfg(test)] +mod settings_tab_tests { + use super::settings_tab_index; + use std::collections::BTreeMap; + + #[test] + fn settings_tab_index_reads_name_and_number() { + let mut env = BTreeMap::new(); + assert_eq!(settings_tab_index(&env), 0); + env.insert("ANYR_TUI_TAB".into(), "claude".into()); + assert_eq!(settings_tab_index(&env), 1); + env.insert("ANYR_TUI_TAB".into(), "2".into()); + assert_eq!(settings_tab_index(&env), 2); + } +} diff --git a/src/cmd/dispatch.rs b/src/cmd/dispatch.rs index bc6ebe1..d6fcfdd 100644 --- a/src/cmd/dispatch.rs +++ b/src/cmd/dispatch.rs @@ -120,15 +120,12 @@ pub(crate) fn tui_palette_select( } #[cfg(feature = "native")] -pub(crate) fn launcher_uses_palette() -> bool { - let tui = std::env::var("ANYR_TUI").unwrap_or_default(); - let t = tui.trim(); - (t == "1" || t.eq_ignore_ascii_case("true") || t.eq_ignore_ascii_case("yes")) - && crate::tui::can_use_fullscreen() +pub(crate) fn launcher_uses_palette(env: &BTreeMap) -> bool { + crate::tui::env_flag(env, "ANYR_TUI") && crate::tui::can_use_fullscreen() } #[cfg(not(feature = "native"))] -pub(crate) fn launcher_uses_palette() -> bool { +pub(crate) fn launcher_uses_palette(_env: &BTreeMap) -> bool { false } @@ -196,7 +193,6 @@ pub(crate) const LAUNCH_FLAGS: &[&str] = &[ "dry-run", "yes", "ok", - "no-check", "device", "device-code", "paste", @@ -425,6 +421,12 @@ pub(crate) fn catalog_lookup_enabled(env: &BTreeMap) -> bool { } pub(crate) fn persist_tool_command(path: &PathBuf, id: &str, command: &str) -> Result<(), String> { + let builtin = resolve_tool(None, id) + .map(|t| t.command) + .unwrap_or_else(|_| id.to_string()); + if !crate::install::should_persist_command(command, &builtin) { + return Ok(()); + } let mut cfg = load_config_if_present(path).unwrap_or_default(); let mut tool = resolve_tool(Some(&cfg), id)?; tool.command = command.to_string(); @@ -578,6 +580,15 @@ pub(crate) fn should_open_launcher(raw: &[String], interactive: bool, dump: bool mod tests { use super::should_open_launcher; + #[test] + fn persist_tool_command_skips_bare_builtin() { + assert!(!crate::install::should_persist_command("claude", "claude")); + assert!(crate::install::should_persist_command( + "/opt/claude", + "claude" + )); + } + #[test] fn bare_tty_opens_launcher_not_help() { assert!(should_open_launcher(&[], true, false)); diff --git a/src/cmd/keys.rs b/src/cmd/keys.rs index 42c98ca..7c33b71 100644 --- a/src/cmd/keys.rs +++ b/src/cmd/keys.rs @@ -108,7 +108,7 @@ pub(crate) fn run_keys(parsed: &ParsedArgs, env: &BTreeMap) -> R .unwrap_or_else(default_key_name); let key = create_key(&base, &cred, &name)?; println!("Created \"{name}\":\n\n {key}\n\nShown once — store it now."); - let save = parsed.flag_true("yes") + let save = parsed.skip_confirm() || (term::is_interactive() && term::confirm("Use this key for the current profile?")); if save { @@ -218,7 +218,7 @@ pub(crate) fn run_keys(parsed: &ParsedArgs, env: &BTreeMap) -> R )) } }; - if !parsed.flag_true("yes") { + if !parsed.skip_confirm() { if !term::is_interactive() { return Err( "Revoking a key is destructive; pass --yes to run non-interactively." diff --git a/src/cmd/launch.rs b/src/cmd/launch.rs index b7fb990..e77fea4 100644 --- a/src/cmd/launch.rs +++ b/src/cmd/launch.rs @@ -128,7 +128,7 @@ pub(crate) fn run_launch( args.extend(model_args_for(tool_name, &model, model_mode)); // --yolo is shorthand for Claude Code's full-permission flag; other tools // don't have an equivalent, so it only maps there. - if tool_name == "claude" && parsed.flag_true("yolo") { + if tool_name == "claude" && (parsed.flag_true("yolo") || tool.extra_flag("yolo")) { args.push("--dangerously-skip-permissions".into()); } args.extend(parsed.passthrough.clone()); diff --git a/src/cmd/menu.rs b/src/cmd/menu.rs index 9d445d5..29f3cc8 100644 --- a/src/cmd/menu.rs +++ b/src/cmd/menu.rs @@ -288,7 +288,7 @@ pub(crate) fn run_menu(parsed: &ParsedArgs, env: &BTreeMap) -> R let dumping = tui_wants_dump(parsed, env); if dumping { - if launcher_uses_palette() { + if launcher_uses_palette(env) { let (header, entries) = launcher_palette(&path, parsed, env, &mut CreditsCache::fresh()); print!("{}", tui_dump_palette(entries, header, env)); @@ -326,7 +326,7 @@ pub(crate) fn run_menu(parsed: &ParsedArgs, env: &BTreeMap) -> R let key = resolve_api_key(&parsed.flags, env, profile); kick_credits_refresh(&cache, base, key); } - let inline = !launcher_uses_palette(); + let inline = !launcher_uses_palette(env); loop { if inline { let (status, actions) = { diff --git a/src/cmd/usage.rs b/src/cmd/usage.rs index aff8c14..59c393a 100644 --- a/src/cmd/usage.rs +++ b/src/cmd/usage.rs @@ -91,6 +91,11 @@ pub(crate) fn run_whoami( term::dim("claude_opus "), term::model_id(profile.claude_opus()) ); + println!( + "{} {}", + term::dim("claude_fable "), + term::model_id(profile.claude_fable()) + ); if let Some(tool) = &profile.default_tool { println!( "{} {}", diff --git a/src/commands.rs b/src/commands.rs index 63c14b8..f57099a 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -150,10 +150,7 @@ fn dispatch( stub("relay") } } - "cursor" | "cline" | "windsurf" => { - print!("{}", command_help(command).unwrap_or_default()); - Ok(0) - } + "cursor" | "cline" | "windsurf" => stub(command), "upgrade" | "update" => crate::upgrade::run(parsed, env), "onboard" | "impl" | "plan" | "fix" | "deploy" | "cp" => { crate::onboard::run(command, parsed) diff --git a/src/config.rs b/src/config.rs index 82f53ac..a886478 100644 --- a/src/config.rs +++ b/src/config.rs @@ -644,7 +644,7 @@ pub fn parse_config(source: &str) -> Config { .get(name) .cloned() .unwrap_or_else(ToolConfig::default); - tool.merge(&ToolConfig::from_yaml(tm)); + tool.apply_yaml(tm); config.tools.insert(name.clone(), tool); } } @@ -987,6 +987,37 @@ agents: assert_eq!(claude2.routing.min_context, Some(1_000_000)); } + #[test] + fn partial_tools_yaml_keeps_builtin_gateway_discovery() { + // WHY: `tools.claude.command` alone must not flip discovery off. + let cfg = parse_config( + "\ +active_profile: default +profiles: + default: + api_key: x +tools: + claude: + command: /opt/claude +", + ); + let claude = cfg.tools.get("claude").expect("claude tool"); + assert_eq!(claude.command, "/opt/claude"); + assert!( + claude.enable_gateway_model_discovery, + "partial overlay wiped gateway discovery" + ); + let yaml = serialize_config(&cfg); + let again = parse_config(&yaml); + assert!( + again + .tools + .get("claude") + .expect("roundtrip") + .enable_gateway_model_discovery + ); + } + #[test] fn set_active_profile_rejects_unknown() { let cfg = parse_config("active_profile: default\nprofiles:\n default:\n api_key: x\n"); diff --git a/src/help.rs b/src/help.rs index ea3ebff..c482879 100644 --- a/src/help.rs +++ b/src/help.rs @@ -8,12 +8,10 @@ thread_local! { } const LAUNCH_HELP_BODY: &str = "\ -Starts this coding agent through AnyRouter. First run signs in if needed -and, on a TTY, installs the agent if it is missing. +Launches the coding agent through AnyRouter (signs in first if needed). Options: - --ok, --yes Non-interactive; launch does not open a picker - --no-check Skip the pre-launch reachability probe + --yes, --ok Skip confirmation prompts (login / install) --model auto| Session model. \"auto\" picks the most-used catalog model --haiku Claude /model haiku and subagents --sonnet Claude /model sonnet @@ -240,7 +238,8 @@ pub fn command_help(command: &str) -> Option { "pi" => launch_help(&bin, "pi", "Pi"), "pool" => launch_help(&bin, "pool", "Poolside"), "cursor" | "cline" | "windsurf" => format!( - "{bin} {canonical} — print the AnyRouter base URL + key to paste into the editor\n" + "{bin} {canonical} — not a launch target yet.\n\ +Print a key with `{bin} auth token` and the base URL with `{bin} onboard impl`.\n" ), _ => return None, }) @@ -550,6 +549,13 @@ mod tests { assert!(auth.contains("ar auth "), "{auth}"); let claude = command_help("claude").unwrap(); assert!(claude.contains("ar claude"), "{claude}"); + assert!(!claude.contains("--no-check"), "{claude}"); + assert!(!claude.contains("opens the launcher"), "{claude}"); + let cursor = command_help("cursor").unwrap(); + assert!( + cursor.contains("not a launch target") || cursor.contains("auth token"), + "{cursor}" + ); set_invoked_bin("anyr"); } diff --git a/src/install.rs b/src/install.rs index 460d21c..e34579d 100644 --- a/src/install.rs +++ b/src/install.rs @@ -138,6 +138,16 @@ Already installed somewhere else? Point AnyRouter at it:\n\ ) } +/// Persist `tools[id].command` only for a real path override, not a PATH hit +/// that resolved the builtin name to `/usr/bin/claude`. +pub fn should_persist_command(resolved: &str, builtin: &str) -> bool { + let trimmed = resolved.trim(); + if trimmed.is_empty() || trimmed == builtin { + return false; + } + trimmed.contains('/') || trimmed.contains('\\') || trimmed.starts_with('.') +} + pub fn resolve_executable(command: &str) -> Option { if command.contains('/') || command.contains('\\') || command.starts_with('.') { return Some(command.to_string()); @@ -158,7 +168,7 @@ pub fn resolve_executable(command: &str) -> Option { return cached.clone(); } let found = find_on_path(command); - // Cache hits only. A miss must be retried after install + // Cache hits only. A miss must be retried after `--install` // (PATH changes in-process; a cached None would always fail). if found.is_some() { hits.borrow_mut().insert(command.to_string(), found.clone()); @@ -348,6 +358,14 @@ mod tests { assert!(agent_available("codex", "codex", &env)); } + #[test] + fn should_persist_only_path_overrides() { + assert!(!should_persist_command("claude", "claude")); + assert!(!should_persist_command("", "claude")); + assert!(should_persist_command("/usr/bin/claude", "claude")); + assert!(should_persist_command("./bin/claude", "claude")); + } + #[test] fn misses_are_not_cached_so_install_can_retry() { assert!(resolve_executable("anyr-definitely-not-on-path-xyz").is_none()); diff --git a/src/onboard.rs b/src/onboard.rs index 78188dc..c6d321c 100644 --- a/src/onboard.rs +++ b/src/onboard.rs @@ -389,7 +389,7 @@ pub const VARIANTS: &[PromptVariant] = &[ pub fn resolve_mode(raw: &str) -> Option<&'static PromptVariant> { let lower = raw.trim().to_ascii_lowercase(); let key = match lower.as_str() { - "implement" | "implementation" | "setup" => "impl", + "implement" | "implementation" => "impl", "migrate" => "plan", other => other, }; @@ -563,6 +563,10 @@ mod tests { assert_eq!(resolve_mode("fix").unwrap().id, "fix"); assert_eq!(resolve_mode("deploy").unwrap().id, "deploy"); assert_eq!(resolve_mode("claude-code").unwrap().id, "claude-code"); + assert!( + resolve_mode("setup").is_none(), + "setup is login at the top level, not an onboard alias" + ); assert!(resolve_mode("nope").is_none()); } diff --git a/src/parse.rs b/src/parse.rs index d45d9a8..79d73fd 100644 --- a/src/parse.rs +++ b/src/parse.rs @@ -56,6 +56,11 @@ impl ParsedArgs { pub fn flag_true(&self, name: &str) -> bool { matches!(self.flags.get(name), Some(FlagValue::Bool(true))) } + + /// `--yes` or `--ok` skip confirmation prompts (login / install / revoke). + pub fn skip_confirm(&self) -> bool { + self.flag_true("yes") || self.flag_true("ok") + } } pub fn parse_cli_args(argv: I) -> Result @@ -99,10 +104,17 @@ where if let Some(eq) = arg.find('=') { let eq_name = arg[2..eq].to_string(); let eq_value = arg[eq + 1..].to_string(); - if VALUE_FLAGS.contains(eq_name.as_str()) && eq_value.is_empty() { - return Err(format!("Flag --{eq_name} requires a value.")); + if VALUE_FLAGS.contains(eq_name.as_str()) { + if eq_value.is_empty() { + return Err(format!("Flag --{eq_name} requires a value.")); + } + flags.insert(eq_name, FlagValue::Value(eq_value)); + } else { + flags.insert( + eq_name.clone(), + FlagValue::Bool(parse_bool_flag(&eq_name, &eq_value)?), + ); } - flags.insert(eq_name, FlagValue::Value(eq_value)); i += 1; continue; } @@ -130,6 +142,26 @@ where }) } +fn parse_bool_flag(name: &str, raw: &str) -> Result { + let t = raw.trim(); + if t.is_empty() + || t == "1" + || t.eq_ignore_ascii_case("true") + || t.eq_ignore_ascii_case("yes") + || t.eq_ignore_ascii_case("on") + { + return Ok(true); + } + if t == "0" + || t.eq_ignore_ascii_case("false") + || t.eq_ignore_ascii_case("no") + || t.eq_ignore_ascii_case("off") + { + return Ok(false); + } + Err(format!("Flag --{name} does not take a value ({raw}).")) +} + pub fn get_string_flag(flags: &HashMap, name: &str) -> Option { match flags.get(name) { Some(FlagValue::Value(value)) if !value.is_empty() => Some(value.clone()), @@ -177,4 +209,23 @@ mod tests { let parsed = parse_cli_args(["claude", "--yes", "--", "--print"]).unwrap(); assert_eq!(parsed.passthrough, vec!["--print"]); } + + #[test] + fn bool_flag_equals_form_is_still_true() { + // WHY: `--yes=true` used to land as FlagValue::Value, so flag_true missed it. + let parsed = parse_cli_args(["login", "--yes=true"]).unwrap(); + assert!(parsed.flag_true("yes")); + assert!(parsed.skip_confirm()); + let device = parse_cli_args(["login", "--device=1"]).unwrap(); + assert!(device.flag_true("device")); + let off = parse_cli_args(["login", "--yes=false"]).unwrap(); + assert!(!off.flag_true("yes")); + } + + #[test] + fn ok_is_confirm_alias() { + let parsed = parse_cli_args(["claude", "--ok"]).unwrap(); + assert!(parsed.skip_confirm()); + assert!(parsed.flag_true("ok")); + } } diff --git a/src/relay.rs b/src/relay.rs index 26c68ac..d91c9eb 100644 --- a/src/relay.rs +++ b/src/relay.rs @@ -226,7 +226,11 @@ fn parse_start_args(parsed: &crate::parse::ParsedArgs) -> StartArgs { /// Mint an rk_ pairing token via POST /relay/devices with the user's sk-ar-v1 /// inference key (the route accepts inference keys) and persist it to the /// shared config. Shared by explicit `relay pair` and auto-pair in start. -fn pair_device(api_key: &str, name: &str, env: &BTreeMap) -> Result<(), String> { +fn pair_device( + api_key: &str, + name: &str, + env: &BTreeMap, +) -> Result { let url = format!("{DEFAULT_API_BASE}/relay/devices"); let body = json!({ "name": name }).to_string(); let (status, resp) = crate::http::http_post(&url, Some(api_key), Some(&body))?; @@ -248,7 +252,7 @@ fn pair_device(api_key: &str, name: &str, env: &BTreeMap) -> Res { write_profile_field(RELAY_DEVICE_ID_FIELD, id, env)?; } - Ok(()) + Ok(token.to_string()) } /// Resolve the credential chain documented in the module comment. Returns the @@ -269,19 +273,13 @@ fn ensure_relay_token( { return Ok(t.to_string()); } - let path = crate::config::resolve_config_path(None, env); - if let Some(cfg) = crate::key::load_config_if_present(&path) { - if let Some(p) = cfg.profiles.get(&cfg.active_profile) { - if let Some(t) = p - .relay_token - .as_deref() - .map(str::trim) - .filter(|s| !s.is_empty()) - { - return Ok(t.to_string()); - } - } + if let Some(t) = read_stored_relay_token(env) + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + { + return Ok(t); } + let path = crate::config::resolve_config_path(None, env); // No relay token yet: resolve (or mint) an sk-ar inference key, auto-pair. let api_key = match crate::key::resolve_api_key( @@ -313,12 +311,12 @@ or set ANYROUTER_API_KEY / {RELAY_TOKEN_ENV_VAR}, or pass --token." }; ulog("No relay token found — pairing this device automatically…"); - pair_device(&api_key, device_name, env)?; + let token = pair_device(&api_key, device_name, env)?; ulog(&format!( "Paired as \"{device_name}\". Token saved to {}.", path.display() )); - Ok(read_stored_relay_token(env).expect("pair_device persisted a relay token")) + Ok(token) } fn read_stored_relay_token(env: &BTreeMap) -> Option { @@ -788,7 +786,15 @@ fn serve_connection( // 1. Drain worker output first so streamed chunks go out promptly. loop { match state.rx.try_recv() { - Ok(frame) => send_frame(ws, &frame), + Ok(frame) => { + match &frame { + ClientFrame::Done { id } | ClientFrame::Error { id, .. } => { + lock_in_flight(&state.in_flight).remove(id); + } + _ => {} + } + send_frame(ws, &frame); + } Err(TryRecvError::Empty) => break, Err(TryRecvError::Disconnected) => return, // unreachable; workers outlive rx } @@ -800,7 +806,7 @@ fn serve_connection( Ok(tungstenite::Message::Text(text)) => match parse_server_frame(&text) { Some(Ok(frame)) => spawn_request(state, frame, target), Some(Err(id)) => { - if let Some(flag) = state.in_flight.lock().unwrap().remove(&id) { + if let Some(flag) = lock_in_flight(&state.in_flight).remove(&id) { flag.store(true, Ordering::SeqCst); } } @@ -834,25 +840,35 @@ fn short_id(s: &str, max_chars: usize) -> String { /// Spawn a worker thread for one incoming request frame and register its /// cancel flag. `target` is 'static by construction: either one of the built-in /// probe constants or a leaked --target value (resolved once per process). +fn lock_in_flight( + map: &Arc>>>, +) -> std::sync::MutexGuard<'_, BTreeMap>> { + map.lock().unwrap_or_else(|e| e.into_inner()) +} + fn spawn_request(state: &ConnState, frame: RequestFrame, target: &'static str) { let cancel = Arc::new(AtomicBool::new(false)); - state - .in_flight - .lock() - .unwrap() - .insert(frame.id.clone(), Arc::clone(&cancel)); + let id = frame.id.clone(); + lock_in_flight(&state.in_flight).insert(id.clone(), Arc::clone(&cancel)); let tx = state.tx.clone(); - std::thread::Builder::new() - .name(format!("relay-req-{}", short_id(&frame.id, 8))) + if let Err(err) = std::thread::Builder::new() + .name(format!("relay-req-{}", short_id(&id, 8))) .spawn(move || handle_request(&tx, &frame, target, &cancel)) - .expect("spawn relay worker"); + { + lock_in_flight(&state.in_flight).remove(&id); + let _ = state.tx.send(ClientFrame::Error { + id, + message: format!("Could not spawn relay worker: {err}"), + }); + } } fn abort_in_flight(in_flight: &Arc>>>) { - for flag in in_flight.lock().unwrap().values() { + let mut map = lock_in_flight(in_flight); + for flag in map.values() { flag.store(true, Ordering::SeqCst); } - in_flight.lock().unwrap().clear(); + map.clear(); } fn send_frame(ws: &mut Ws, frame: &ClientFrame) { @@ -981,7 +997,7 @@ fn run_relay_pair( )); } }; - pair_device(&api_key, &name, env)?; + let _ = pair_device(&api_key, &name, env)?; println!("Paired as \"{name}\". Token saved to your AnyRouter config."); println!("Run: {} relay start", crate::help::invoked_bin()); Ok(0) diff --git a/src/spawn.rs b/src/spawn.rs index 2459df5..5a92c50 100644 --- a/src/spawn.rs +++ b/src/spawn.rs @@ -63,8 +63,15 @@ impl ToolConfig { if over.model_env.is_some() { self.model_env = over.model_env.clone(); } - self.base_suffix = over.base_suffix.clone(); - self.enable_gateway_model_discovery = over.enable_gateway_model_discovery; + if !over.base_suffix.is_empty() { + self.base_suffix = over.base_suffix.clone(); + } + // Overlay from `from_yaml` defaults discovery to false when the key + // is missing — never copy that over a builtin. `apply_yaml` is the + // path that honors an explicit false. + if over.enable_gateway_model_discovery { + self.enable_gateway_model_discovery = true; + } if over.shadow_env.is_some() { self.shadow_env = over.shadow_env.clone(); } @@ -73,44 +80,63 @@ impl ToolConfig { } } - pub fn from_yaml(map: &BTreeMap) -> Self { - let mut tool = ToolConfig::default(); + /// Apply only keys present in `map`. Missing keys keep the current value + /// so a partial `tools.claude.command:` overlay cannot wipe `/v1` or + /// gateway discovery. + pub fn apply_yaml(&mut self, map: &BTreeMap) { for (key, value) in map { match key.as_str() { - "command" => tool.command = value.as_string_lossy(), - "base_url_env" => tool.base_url_env = value.as_string_lossy(), - "auth_env" => tool.auth_env = value.as_string_lossy(), + "command" => self.command = value.as_string_lossy(), + "base_url_env" => self.base_url_env = value.as_string_lossy(), + "auth_env" => self.auth_env = value.as_string_lossy(), "model_env" => { let s = value.as_string_lossy(); - tool.model_env = if s.is_empty() || s == "null" { + self.model_env = if s.is_empty() || s == "null" { None } else { Some(s) }; } - "base_suffix" => tool.base_suffix = value.as_string_lossy(), + "base_suffix" => self.base_suffix = value.as_string_lossy(), "enable_gateway_model_discovery" => { - tool.enable_gateway_model_discovery = + self.enable_gateway_model_discovery = matches!(value, YamlValue::Bool(true)) || value.as_string_lossy() == "true" } "shadow_env" => { let s = value.as_string_lossy(); - tool.shadow_env = if s.is_empty() || s == "null" { + self.shadow_env = if s.is_empty() || s == "null" { None } else { Some(s) }; } _ => { - tool.extra.insert(key.clone(), value.clone()); + self.extra.insert(key.clone(), value.clone()); } } } + } + + pub fn from_yaml(map: &BTreeMap) -> Self { + let mut tool = ToolConfig::default(); + tool.apply_yaml(map); tool } + pub fn extra_flag(&self, key: &str) -> bool { + match self.extra.get(key) { + Some(YamlValue::Bool(true)) => true, + Some(YamlValue::Int(n)) => *n != 0, + Some(YamlValue::String(s)) => { + let t = s.trim(); + t == "1" || t.eq_ignore_ascii_case("true") || t.eq_ignore_ascii_case("yes") + } + _ => false, + } + } + pub fn to_yaml_lines(&self) -> Vec { - vec![ + let mut lines = vec![ format!(" command: {}", self.command), format!(" base_url_env: {}", self.base_url_env), format!(" auth_env: {}", self.auth_env), @@ -134,7 +160,14 @@ impl ToolConfig { " shadow_env: {}", self.shadow_env.as_deref().unwrap_or("null") ), - ] + ]; + for (key, value) in &self.extra { + lines.push(format!( + " {key}: {}", + crate::config::yaml_scalar_value(value) + )); + } + lines } } @@ -229,9 +262,9 @@ pub fn resolve_tool( format!("Unknown tool \"{name}\". Known tools: claude, codex, grok, opencode, pool, pi.") })?; if let Some(over) = config.and_then(|c| c.tools.get(id)) { - let mut t = fallback; - t.merge(over); - return Ok(t); + // Parsed tools are already builtin + apply_yaml. Clone, don't merge a + // second time (merge would treat missing overlay keys as defaults). + return Ok(over.clone()); } Ok(fallback) } @@ -319,6 +352,10 @@ pub fn build_tool_env(input: BuildToolEnvInput<'_>) -> BTreeMap tool_base_url(input.profile, input.tool), ); env.insert(input.tool.auth_env.clone(), input.api_key.to_string()); + // Parent-shell Anthropic/OpenAI keys must not beat the AnyRouter token. + if let Some(shadow) = &input.tool.shadow_env { + env.insert(shadow.clone(), input.api_key.to_string()); + } env.insert( "ANYROUTER_PINNED_PRESET".into(), input.profile.pinned_preset().to_string(), @@ -1218,10 +1255,7 @@ mod tests { ); assert_eq!( ids, - vec![ - "z-ai/glm-5.2".to_string(), - "anyrouter/free".to_string(), - ] + vec!["z-ai/glm-5.2".to_string(), "anyrouter/free".to_string(),] ); } @@ -1274,4 +1308,71 @@ mod tests { assert!(body.contains("\"min_context\":1000000"), "{body}"); assert_eq!(env.get("ANYROUTER_EXTRA_BODY"), Some(body)); } + + #[test] + fn claude_shadow_env_overrides_parent_anthropic_key() { + // WHY: a leftover ANTHROPIC_API_KEY in the parent shell must not win. + let tool = builtin("claude").unwrap(); + assert_eq!(tool.shadow_env.as_deref(), Some("ANTHROPIC_API_KEY")); + let env = build_tool_env(BuildToolEnvInput { + tool_name: "claude", + tool: &tool, + profile: &profile(), + api_key: "sk-ar-v1-secret", + model: "auto", + effort: None, + context_window: None, + model_map: None, + }); + assert_eq!( + env.get("ANTHROPIC_AUTH_TOKEN").map(String::as_str), + Some("sk-ar-v1-secret") + ); + assert_eq!( + env.get("ANTHROPIC_API_KEY").map(String::as_str), + Some("sk-ar-v1-secret") + ); + } + + #[test] + fn apply_yaml_partial_does_not_wipe_gateway_discovery() { + let mut tool = builtin("claude").unwrap(); + let mut map = BTreeMap::new(); + map.insert("command".into(), YamlValue::String("/opt/claude".into())); + tool.apply_yaml(&map); + assert_eq!(tool.command, "/opt/claude"); + assert!( + tool.enable_gateway_model_discovery, + "partial YAML must keep builtin discovery" + ); + } + + #[test] + fn extra_yolo_round_trips_in_yaml() { + let mut tool = builtin("claude").unwrap(); + let mut map = BTreeMap::new(); + map.insert("yolo".into(), YamlValue::Bool(true)); + tool.apply_yaml(&map); + assert!(tool.extra_flag("yolo")); + let yaml = tool.to_yaml_lines().join("\n"); + assert!(yaml.contains("yolo: true"), "{yaml}"); + let parsed = crate::config::parse_config( + "active_profile: default\nprofiles:\n default:\n api_key: x\ntools:\n claude:\n yolo: true\n", + ); + let again = parsed.tools.get("claude").cloned().unwrap(); + assert!( + again.extra_flag("yolo"), + "parse_config must keep extra yolo" + ); + } + + #[test] + fn merge_command_only_overlay_keeps_codex_suffix() { + let mut t = builtin("codex").unwrap(); + let mut over = ToolConfig::default(); + over.command = "/opt/codex".into(); + t.merge(&over); + assert_eq!(t.command, "/opt/codex"); + assert_eq!(t.base_suffix, "/v1"); + } } diff --git a/src/tui/keys.rs b/src/tui/keys.rs index 9c5a2e9..eca3652 100644 --- a/src/tui/keys.rs +++ b/src/tui/keys.rs @@ -41,6 +41,8 @@ pub enum KeyCode { Up, Down, Tab, + /// Crossterm emits this for Shift-Tab on Linux; it is not Tab+SHIFT. + BackTab, Backspace, Delete, } @@ -51,7 +53,8 @@ pub fn map_key(surface: Surface, key: KeyEvent) -> Action { KeyCode::Char('c') | KeyCode::Char('d') => Action::Quit, KeyCode::Char('p') => Action::Up, KeyCode::Char('n') => Action::Down, - _ => Action::Esc, + // Unknown chords must not close settings / quit the palette. + _ => Action::Resize, }; } match key.code { @@ -59,6 +62,10 @@ pub fn map_key(surface: Surface, key: KeyEvent) -> Action { KeyCode::Esc => Action::Esc, KeyCode::Up => Action::Up, KeyCode::Down => Action::Down, + KeyCode::BackTab => match surface { + Surface::Settings => Action::PrevTab, + _ => Action::Up, + }, KeyCode::Tab => match (surface, key.shift) { (Surface::Settings, false) => Action::NextTab, (Surface::Settings, true) => Action::PrevTab, @@ -67,7 +74,7 @@ pub fn map_key(surface: Surface, key: KeyEvent) -> Action { KeyCode::Backspace | KeyCode::Delete => Action::Backspace, KeyCode::Char(c) => match (surface, c) { (Surface::Launcher, 'q' | 'x' | 'Q' | 'X') => Action::Quit, - (Surface::Settings, 'q') => Action::Quit, + (Surface::Settings, 'q' | 'Q') => Action::Quit, (Surface::Settings, 'x' | 'X') => Action::Unset, (Surface::Settings, '[') => Action::PrevTab, (Surface::Settings, ']') => Action::NextTab, @@ -197,5 +204,42 @@ mod tests { }, ); assert_eq!(brack, Action::NextTab); + // WHY: Linux terminals send BackTab, not Tab+SHIFT. + let back = map_key( + Surface::Settings, + KeyEvent { + code: KeyCode::BackTab, + ctrl: false, + shift: false, + }, + ); + assert_eq!(back, Action::PrevTab); + } + + #[test] + fn settings_shift_q_quits() { + let a = map_key( + Surface::Settings, + KeyEvent { + code: KeyCode::Char('Q'), + ctrl: false, + shift: false, + }, + ); + assert_eq!(a, Action::Quit); + } + + #[test] + fn unknown_ctrl_does_not_quit() { + let a = map_key( + Surface::Settings, + KeyEvent { + code: KeyCode::Char('w'), + ctrl: true, + shift: false, + }, + ); + assert_ne!(a, Action::Quit); + assert_ne!(a, Action::Esc); } } diff --git a/src/tui/live.rs b/src/tui/live.rs index 7786a63..92ee8ef 100644 --- a/src/tui/live.rs +++ b/src/tui/live.rs @@ -28,7 +28,7 @@ pub fn is_interactive() -> bool { io::stdin().is_terminal() && io::stdout().is_terminal() } -fn translate_key(ev: crossterm::event::KeyEvent) -> Option { +pub(crate) fn translate_key(ev: crossterm::event::KeyEvent) -> Option { if ev.kind != KeyEventKind::Press { return None; } @@ -38,6 +38,7 @@ fn translate_key(ev: crossterm::event::KeyEvent) -> Option { CtKeyCode::Up => KeyCode::Up, CtKeyCode::Down => KeyCode::Down, CtKeyCode::Tab => KeyCode::Tab, + CtKeyCode::BackTab => KeyCode::BackTab, CtKeyCode::Backspace => KeyCode::Backspace, CtKeyCode::Delete => KeyCode::Delete, CtKeyCode::Char(c) => KeyCode::Char(c), @@ -329,3 +330,17 @@ pub fn run_settings_live(mut state: SettingsState) -> Result String { plain_settings_frame(state, cols) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::tui::keys::{map_key, Action, Surface}; + + #[test] + fn backtab_translates_to_settings_prev_tab() { + // WHY: crossterm sends BackTab for Shift-Tab, not Tab+SHIFT. + let ev = crossterm::event::KeyEvent::new(CtKeyCode::BackTab, KeyModifiers::NONE); + let key = translate_key(ev).expect("BackTab must not be dropped"); + assert_eq!(map_key(Surface::Settings, key), Action::PrevTab); + } +} diff --git a/src/tui/mod.rs b/src/tui/mod.rs index 9793221..9aa7883 100644 --- a/src/tui/mod.rs +++ b/src/tui/mod.rs @@ -25,15 +25,17 @@ pub use state::{ PickerState, SettingRow, SettingsOutcome, SettingsState, Tone, }; +pub fn env_flag(env: &BTreeMap, key: &str) -> bool { + env.get(key) + .map(|s| { + let t = s.trim(); + t == "1" || t.eq_ignore_ascii_case("true") || t.eq_ignore_ascii_case("yes") + }) + .unwrap_or(false) +} + pub fn wants_dump(parsed: &ParsedArgs, env: &BTreeMap) -> bool { - parsed.flag_true("dump-tui") - || env - .get("ANYR_TUI_DUMP") - .map(|s| { - let t = s.trim(); - t == "1" || t.eq_ignore_ascii_case("true") || t.eq_ignore_ascii_case("yes") - }) - .unwrap_or(false) + parsed.flag_true("dump-tui") || env_flag(env, "ANYR_TUI_DUMP") } pub fn dump_cols(env: &BTreeMap) -> usize { diff --git a/src/tui/view.rs b/src/tui/view.rs index 0312703..f6faf5d 100644 --- a/src/tui/view.rs +++ b/src/tui/view.rs @@ -226,8 +226,9 @@ pub fn render_palette(frame: &mut Frame, state: &PaletteState) { /// Number of distinct groups among the first `visible` filtered entries — /// each renders one header line inside the result area. fn palette_groups(state: &PaletteState, filtered: &[usize], visible: usize) -> usize { + let (start, vis) = palette_window(state.cursor, filtered.len(), visible.max(1)); let mut groups: Vec<&str> = Vec::new(); - for &entry_i in filtered.iter().take(visible) { + for &entry_i in filtered.iter().skip(start).take(vis) { let g = state.entries[entry_i].group.as_str(); if g.is_empty() { continue; @@ -247,10 +248,10 @@ fn palette_rows( visible: usize, inner_w: usize, ) -> Vec> { - let cursor_row = state.cursor.min(visible.saturating_sub(1)); + let (start, vis) = palette_window(state.cursor, filtered.len(), visible.max(1)); let mut rows: Vec = Vec::new(); let mut last_group: Option<&str> = None; - for (row_i, &entry_i) in filtered.iter().take(visible).enumerate() { + for (row_i, &entry_i) in filtered.iter().skip(start).take(vis).enumerate() { let entry: &PaletteEntry = &state.entries[entry_i]; if last_group != Some(entry.group.as_str()) { if last_group.is_some() { @@ -264,7 +265,7 @@ fn palette_rows( } last_group = Some(&entry.group); } - let selected = row_i == cursor_row; + let selected = start + row_i == state.cursor; let marker_style = if selected { theme::accent() } else { @@ -459,10 +460,28 @@ fn in_rect(r: Rect, col: u16, row: u16) -> bool { && row < r.y.saturating_add(r.height) } +/// Live palette shows at most this many filtered rows; the window follows +/// the cursor so Enter never selects a row that is off-screen. +const PALETTE_LIVE_ROWS: usize = 10; + +fn palette_window(cursor: usize, n: usize, max: usize) -> (usize, usize) { + if n == 0 { + return (0, 0); + } + let vis = n.min(max.max(1)); + let cursor = cursor.min(n - 1); + let start = if n <= vis { + 0 + } else { + cursor.saturating_sub(vis - 1).min(n - vis) + }; + (start, vis) +} + fn palette_card(area: Rect, state: &PaletteState) -> (Rect, Vec, usize) { let filtered = state.filtered(); let header_h = palette_header_height(&state.header); - let visible = filtered.len().min(10); + let (_, visible) = palette_window(state.cursor, filtered.len(), PALETTE_LIVE_ROWS); let groups = palette_groups(state, &filtered, visible); let between = groups.saturating_sub(1); let height = 2 @@ -506,9 +525,10 @@ pub fn hit_palette(area: Rect, state: &PaletteState, col: u16, row: u16) -> Opti fn palette_hit_map(state: &PaletteState, visible: usize) -> Vec> { let filtered = state.filtered(); + let (start, vis) = palette_window(state.cursor, filtered.len(), visible.max(1)); let mut map = Vec::new(); let mut last_group: Option<&str> = None; - for (row_i, &entry_i) in filtered.iter().take(visible).enumerate() { + for (row_i, &entry_i) in filtered.iter().skip(start).take(vis).enumerate() { let entry = &state.entries[entry_i]; if last_group != Some(entry.group.as_str()) { if last_group.is_some() { @@ -519,7 +539,7 @@ fn palette_hit_map(state: &PaletteState, visible: usize) -> Vec> { } last_group = Some(entry.group.as_str()); } - map.push(Some(row_i)); + map.push(Some(start + row_i)); } map } @@ -883,10 +903,10 @@ pub fn plain_palette_lines(state: &PaletteState, cols: usize) -> Vec { if filtered.is_empty() { lines.push(truncate(" (no matches)", width)); } else { - let visible = filtered.len().min(12); - let cursor_row = state.cursor.min(visible.saturating_sub(1)); + // Dump is a full snapshot, not a viewport — CI must see every row. + let cursor_row = state.cursor.min(filtered.len().saturating_sub(1)); let mut last_group: Option<&str> = None; - for (row_i, &entry_i) in filtered.iter().take(visible).enumerate() { + for (row_i, &entry_i) in filtered.iter().enumerate() { let entry = &state.entries[entry_i]; if last_group != Some(entry.group.as_str()) { if !entry.group.is_empty() { @@ -910,7 +930,7 @@ pub fn plain_palette_lines(state: &PaletteState, cols: usize) -> Vec { } } - lines.push(truncate("❯ ↵ launch · q quit", width)); + lines.push(truncate("❯ ↵ launch · esc quit", width)); lines } @@ -1008,6 +1028,16 @@ fn truncate(s: &str, max: usize) -> String { mod tests { use super::*; + #[test] + fn palette_window_keeps_cursor_visible() { + // WHY: Enter must select the highlighted row, not a row below the fold. + assert_eq!(palette_window(0, 20, 10), (0, 10)); + assert_eq!(palette_window(15, 20, 10), (6, 10)); + assert_eq!(palette_window(19, 20, 10), (10, 10)); + assert_eq!(palette_window(0, 3, 10), (0, 3)); + assert_eq!(palette_window(0, 0, 10), (0, 0)); + } + #[test] fn dump_menu_is_ansi_free() { let state = MenuState::new( diff --git a/tests/cli.rs b/tests/cli.rs index 95dafce..a8672bb 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -2203,3 +2203,159 @@ fn models_dump_tui_pins_anyrouter_auto_without_most_used() { "{stdout}" ); } + +// --- PR #52 bug-fix coverage --- +#[test] +fn claude_yolo_extra_in_config_expands_like_flag() { + // WHY: tools.claude.yolo must survive serialize and launch like --yolo. + let dir = temp_home(); + std::fs::write( + dir.join("config.yaml"), + "\ +active_profile: default +profiles: + default: + api_key: sk-ar-v1-fixture-key-0001 + default_model: auto +tools: + claude: + yolo: true +", + ) + .unwrap(); + let out = anyr() + .args(["claude", "--dry-run", "--yes"]) + .env("ANYROUTER_HOME", &dir) + .env_remove("ANYROUTER_API_KEY") + .output() + .expect("yolo extra dry-run"); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code().unwrap_or(1), 0, "{stdout}{stderr}"); + assert!( + stdout.contains("--dangerously-skip-permissions"), + "config yolo must expand:\n{stdout}" + ); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn config_dump_tui_claude_tab_shows_agent_rows() { + let dir = temp_home(); + let path = dir.join("config.yaml"); + std::fs::write( + &path, + "\ +active_profile: default +profiles: + default: + api_key: sk-ar-v1-tab-dump-secret-abcdef + default_model: auto +", + ) + .unwrap(); + let out = anyr() + .args(["config", "--dump-tui", "--config", path.to_str().unwrap()]) + .env("ANYR_TUI_TAB", "claude") + .env("ANYROUTER_HOME", &dir) + .output() + .expect("config claude tab"); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code().unwrap_or(1), 0, "{stdout}{stderr}"); + assert!(stdout.contains("[claude]"), "{stdout}"); + for row in ["command", "install", "gateway discovery", "haiku"] { + assert!(stdout.contains(row), "missing {row} in:\n{stdout}"); + } + assert!( + !stdout.contains("tab-dump-secret"), + "dump must not leak secret: {stdout}" + ); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn config_dump_tui_grok_tab_omits_claude_aliases() { + let dir = temp_home(); + let path = dir.join("config.yaml"); + std::fs::write( + &path, + "\ +active_profile: default +profiles: + default: + api_key: sk-ar-v1-grok-tab-secret-abcdef + default_model: auto +", + ) + .unwrap(); + let out = anyr() + .args(["config", "--dump-tui", "--config", path.to_str().unwrap()]) + .env("ANYR_TUI_TAB", "grok") + .env("ANYROUTER_HOME", &dir) + .output() + .expect("config grok tab"); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert_eq!(out.status.code().unwrap_or(1), 0, "{stdout}{stderr}"); + assert!(stdout.contains("[grok]"), "{stdout}"); + assert!( + stdout.contains("GROK_MODELS_BASE_URL") || stdout.contains("base URL env"), + "{stdout}" + ); + assert!(!stdout.contains("haiku"), "{stdout}"); + assert!(!stdout.contains("gateway discovery"), "{stdout}"); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn cursor_is_honest_stub_not_silent_success() { + let (code, stdout, stderr) = run(&["cursor"]); + assert_ne!(code, 0, "{stdout}{stderr}"); + let combined = format!("{stdout}{stderr}"); + assert!( + combined.contains("not yet") || combined.contains("not a launch"), + "{combined}" + ); +} + +#[test] +fn launch_rejects_unknown_flag() { + let (code, _stdout, stderr) = run(&["claude", "--bogus"]); + assert_eq!(code, 1); + assert!( + stderr.contains("Unknown") || stderr.contains("bogus"), + "{stderr}" + ); +} +#[test] +fn whoami_and_keys_fail_loud_without_config() { + let dir = temp_home(); + for args in [vec!["whoami"], vec!["keys", "list"]] { + let out = anyr() + .args(&args) + .env("ANYROUTER_HOME", &dir) + .env_remove("ANYROUTER_API_KEY") + .output() + .expect("missing key"); + let stderr = String::from_utf8_lossy(&out.stderr); + assert_ne!(out.status.code().unwrap_or(1), 0, "{args:?} {stderr}"); + assert!( + stderr.contains("No AnyRouter config") || stderr.contains("no key"), + "{args:?} {stderr}" + ); + } + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn yes_equals_true_skips_confirm_parse() { + let (code, stdout, stderr) = run(&["claude", "--help"]); + assert_eq!(code, 0, "{stdout}{stderr}"); + assert!(!stdout.contains("Skip the launcher"), "{stdout}"); + assert!(!stdout.contains("--no-check"), "{stdout}"); + assert!( + stdout.contains("--yes") && stdout.contains("--ok"), + "{stdout}" + ); +}