From c91fa70675b4c4afce8073c5e8b35740009f8299 Mon Sep 17 00:00:00 2001 From: Elyas Mehtabuddin Date: Wed, 30 Sep 2026 10:32:39 -0700 Subject: [PATCH 1/3] feat(menubar): change a route's algorithm and models from the menu bar Signed-off-by: Elyas Mehtabuddin --- Cargo.lock | 19 + crates/switchyard-menubar/Cargo.toml | 6 +- crates/switchyard-menubar/README.md | 128 +- crates/switchyard-menubar/src/main.rs | 26 +- crates/switchyard-menubar/src/models.rs | 702 +++++++++++ crates/switchyard-menubar/src/picker.rs | 1096 +++++++++++++++++ crates/switchyard-menubar/src/server.rs | 278 +++++ .../switchyard-menubar/src/server_config.rs | 880 +++++++++++++ crates/switchyard-menubar/src/tray.rs | 87 +- scripts/macos/uninstall.sh | 3 +- 10 files changed, 3172 insertions(+), 53 deletions(-) create mode 100644 crates/switchyard-menubar/src/models.rs create mode 100644 crates/switchyard-menubar/src/picker.rs create mode 100644 crates/switchyard-menubar/src/server.rs create mode 100644 crates/switchyard-menubar/src/server_config.rs diff --git a/Cargo.lock b/Cargo.lock index f4a4ebcab..1b0fec06c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3170,10 +3170,13 @@ dependencies = [ "objc2", "objc2-app-kit", "objc2-foundation", + "security-framework", "serde", "serde_json", + "switchyard-runner", "tempfile", "toml 0.9.12+spec-1.1.0", + "toml_edit 0.25.13+spec-1.1.0", "tray-icon", ] @@ -3643,6 +3646,19 @@ dependencies = [ "winnow 0.5.40", ] +[[package]] +name = "toml_edit" +version = "0.25.13+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6975367e4d2ef766d86af01ffad14b622fecc8d4357a998fbc4deb6e9bacaf9b" +dependencies = [ + "indexmap", + "toml_datetime 1.1.1+spec-1.1.0", + "toml_parser", + "toml_writer", + "winnow 1.0.4", +] + [[package]] name = "toml_parser" version = "1.1.2+spec-1.1.0" @@ -4240,6 +4256,9 @@ name = "winnow" version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "23b97319f7b8343df12cc98938e5c3eb436064524c8d2b4e30a1d3a36eecdf81" +dependencies = [ + "memchr", +] [[package]] name = "wiremock" diff --git a/crates/switchyard-menubar/Cargo.toml b/crates/switchyard-menubar/Cargo.toml index d12551f0d..e4ede3faa 100644 --- a/crates/switchyard-menubar/Cargo.toml +++ b/crates/switchyard-menubar/Cargo.toml @@ -17,13 +17,17 @@ publish = false chrono = { version = "0.4", default-features = false, features = ["std", "clock"] } serde.workspace = true serde_json.workspace = true +tempfile = "3" toml = "0.9" +toml_edit = "0.25" [target.'cfg(target_os = "macos")'.dependencies] objc2 = "0.6" objc2-app-kit = "0.3" objc2-foundation = "0.3" +security-framework = "3" tray-icon = "0.25" [dev-dependencies] -tempfile = "3" +# The tests check each edited config with the server's own config parser. +switchyard-runner.workspace = true diff --git a/crates/switchyard-menubar/README.md b/crates/switchyard-menubar/README.md index e20ef1880..0a24110df 100644 --- a/crates/switchyard-menubar/README.md +++ b/crates/switchyard-menubar/README.md @@ -2,12 +2,14 @@ A macOS menu bar companion for a Switchyard server running in the background. -The server does the routing. This process only reads what the server already -wrote, so quitting it never affects traffic. +The server does the routing. This process reads what the server wrote and +edits the server config only when you ask it to, so quitting it never affects +traffic. Click the glyph to see whether the server is answering, today's and this week's requests and tokens, estimated savings, and which models served the -week. It can also restart the server and open either config file. +week. It can also restart the server, open either config file, and change a +route's algorithm and models. ## Install @@ -33,6 +35,8 @@ exist, so edits survive a reinstall. Logs are in `~/.switchyard/logs/`. ```toml server_url = "http://127.0.0.1:4123" routing_log = "~/.switchyard/routing.jsonl" +config_file = "~/.switchyard/composite.toml" +launchd_label = "com.nvidia.switchyard.server" baseline_model = "gpt-5.6-sol" refresh_seconds = 30 @@ -42,6 +46,124 @@ cached_input_per_mtok = 0.125 output_per_mtok = 10.0 ``` +## Change a route's algorithm and models + +Click the glyph, then **Change routing…**. The window reads the server config +named by `config_file`. + +1. Pick a route. The window shows its algorithm and the model for each role. +2. Pick an algorithm: `passthrough`, `random`, `llm_classifier`, `composite`, + `stage_router`, `advisor`, `plan_execute`, or `auto`. The window says what + each one does and which roles it needs. For example, `composite` needs a + judge, a capable model, and an efficient model. +3. For each role, pick an LLM client from the config, then a model. The model + box lists the models from the client's `GET /models` endpoint. Type to + filter the list, or type a model ID that is not listed. +4. Click **Apply**. + +Apply checks the new config with +`switchyard-server --config --dry-run`, using the `switchyard-server` +installed next to the menu bar app. If the check fails, the window shows the +server's error and the file does not change. If it passes, the app saves the +file, keeps the old one as `.switchyard-backup.`, restarts +the server with `launchctl kickstart -k gui//`, and says +whether the server answers `/health`. If `config_file` is a symlink, the app +writes the file that the link points to, puts the backup next to that file, +and leaves the link in place. + +The app changes as little of the file as it can: + +- Comments, formatting, and every table the change does not touch stay as + they were. +- A role keeps its target when that target already names the chosen model. + If another target names that model on that client, the route uses it. + Otherwise the app changes the route's own target in place, so settings such + as `extra_body` and `omit_body_fields` stay. +- The app copies a target instead of changing it when another route uses it, + or when an earlier role in the same Apply already took it. For example, if + you move the efficient model up to Capable and pick a new efficient model, + Capable takes the old efficient target, and Efficient gets a copy of that + target with the new model. +- A changed or copied target also keeps its model-specific settings. For + example, a target with `omit_body_fields = ["reasoning_effort"]` keeps that + setting when it switches from a Claude model to a GPT model. Check these + settings after you move a role to another model family. +- Switching algorithms keeps the route's `id`, `context_window`, + `tool_calling`, `reasoning`, and `vision`, and keeps `subagents` when the new + algorithm accepts it (`passthrough`, `stage_router`, and `composite`). It + removes the old type's other settings and writes the settings the new type + requires with the values from the routing docs, such as + `confidence_threshold = 0.5`. Edit the file to tune them. +- Targets that no route uses any more stay in the file. + +The window cannot show a custom-mode `llm_classifier` route or a route of +another type. Applying to such a route replaces its settings with the +algorithm you pick. + +The check runs with the menu bar app's environment. If a client reads its +key from `api_key_env`, the menu bar's LaunchAgent needs that variable too. +Otherwise the check fails and the app saves nothing. + +If a chosen model has no price in `menubar.toml`, the window says so. +Savings stay hidden until you add the price and restart the menu bar app. + +### Model lists + +The app saves every model list it fetches in `model-lists.json`, in the same +directory as the settings file it was started with. With the default settings +file, that is `~/.switchyard/model-lists.json`. The file holds the model IDs +and the time each list was fetched, keyed by the list's URL. It never holds a +key. + +When a role needs a list, the window uses the list it already has, or else +the one in `model-lists.json`. Only when neither exists does the app send one +`GET /models` request. After a list is in `model-lists.json`, the app does not +fetch it again on its own, even after a restart. If a fetch fails and the +window has no list to show, the app tries again the next time you open the +window. Clients whose models share a URL share one list and one request. + +If the app cannot write `model-lists.json`, the window still uses the fetched +list and shows "Could not save the list" with the error. Because the list is +not in the file, the app fetches it again after a restart. + +Click **Refresh models** to fetch the lists that the window's roles use again, +one request per URL. The result area then shows each list's model count or +error. If a fetch fails, the window keeps the list it has and shows the error +under it. + +When a role's model box is empty or holds a listed model ID, the note under +it shows the list's model count and age, such as "10 models, listed 2 minutes +ago. Type to filter." While you type, the note says how many models match, +such as "2 of 10 models match." When nothing matches, it says "No listed +model matches. Apply uses the ID as typed." + +### Keys for model lists + +The app never writes a key to a file. For a client with `api_key_env`, it +reads that variable from its own environment. A client with `forward_auth` +stores no key, and a LaunchAgent does not get the variables from your shell +profile. So when a list needs a key and none is available, the role's note +says so, and the window shows a key field under the roles. Paste the key and +click **Save key**. The app saves the key in your login Keychain as +"Switchyard model list", with the client's `base_url` as the account, and +fetches the list with it. Clients with the same `base_url` share the key. + +If macOS cannot save the key, or cannot hand over a saved one, the window +shows the Keychain's error. You can always type a model ID that is not +listed. + +The app lists models with the system `curl`. It writes the key to curl's +stdin, never to its command line, and runs curl with `-q`, so curl ignores +`~/.curlrc`. Without `-q`, a `verbose` line in that file would print the key +into the error text that the window shows. + +The app uses the Keychain rather than your login shell's environment. +Reading that environment means starting your shell from the app, which runs +your whole shell profile and fails if the profile waits for input. The +Keychain needs no shell, and macOS asks before another app reads the key. +After you reinstall the menu bar app, macOS may ask once whether the new +build may read it. + ## How savings are computed The server writes one JSONL record per call, with the model that answered and diff --git a/crates/switchyard-menubar/src/main.rs b/crates/switchyard-menubar/src/main.rs index 0e9e7e0f5..ac41f1dae 100644 --- a/crates/switchyard-menubar/src/main.rs +++ b/crates/switchyard-menubar/src/main.rs @@ -3,9 +3,11 @@ //! Menu bar companion for a locally running Switchyard server. //! -//! The server owns routing; this process only reads what the server already -//! wrote. It shows today's and this week's traffic, and what that traffic -//! would have cost had every call gone to the capable model instead. +//! The server owns routing; this process reads what the server wrote. It +//! shows today's and this week's traffic, and what that traffic would have +//! cost had every call gone to the capable model instead. Its "Change +//! routing…" window edits a route in the server config and restarts the +//! server. mod app; mod config; @@ -13,10 +15,21 @@ mod health; mod pricing; mod rollup; mod summary; -// The status item and the glyph it draws are the only platform-specific code. +// Only the macOS picker window calls these. They build on every target so +// that their tests also run in CI, which does not run on macOS. +#[cfg_attr(not(target_os = "macos"), allow(dead_code))] +mod models; +#[cfg_attr(not(target_os = "macos"), allow(dead_code))] +mod server; +#[cfg_attr(not(target_os = "macos"), allow(dead_code))] +mod server_config; +// The status item, the glyph it draws, and the picker window are the only +// platform-specific code. #[cfg(target_os = "macos")] mod icon; #[cfg(target_os = "macos")] +mod picker; +#[cfg(target_os = "macos")] mod tray; use std::path::PathBuf; @@ -65,7 +78,8 @@ fn main() -> ExitCode { return ExitCode::SUCCESS; } - let config = match Config::load(&settings.unwrap_or_else(Config::default_path)) { + let settings = settings.unwrap_or_else(Config::default_path); + let config = match Config::load(&settings) { Ok(config) => config, Err(error) => { eprintln!("switchyard-menubar: {error}"); @@ -85,7 +99,7 @@ fn main() -> ExitCode { } #[cfg(target_os = "macos")] - if let Err(error) = tray::run(config) { + if let Err(error) = tray::run(config, &settings) { eprintln!("switchyard-menubar: {error}"); return ExitCode::FAILURE; } diff --git a/crates/switchyard-menubar/src/models.rs b/crates/switchyard-menubar/src/models.rs new file mode 100644 index 000000000..e966c784a --- /dev/null +++ b/crates/switchyard-menubar/src/models.rs @@ -0,0 +1,702 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Lists the models an LLM client offers, from its `GET /models` endpoint, +//! and keeps each fetched list in a cache file. +//! +//! The request goes through the system `curl`, the same way the menu runs +//! `launchctl`, so the app needs no HTTP or TLS stack of its own. The key is +//! written to curl's stdin, never to its command line, where other processes +//! could read it. +//! +//! The cache file holds model IDs and fetch times, never a key. The app uses +//! a list from the cache file until the user refreshes it, so it fetches a +//! list on its own only when the file does not have it. + +use std::collections::{BTreeMap, HashMap}; +use std::io::Write; +use std::path::Path; +use std::process::{Command, Stdio}; +use std::sync::{Mutex, PoisonError}; +use std::time::{SystemTime, UNIX_EPOCH}; + +use serde::{Deserialize, Serialize}; + +use crate::server_config::Client; + +/// The name of the cache file. It sits next to the settings file that the +/// app was started with. +pub const CACHE_FILE: &str = "model-lists.json"; + +/// The Keychain service that holds keys saved from the route picker. Each +/// item's account is the client's `base_url`, so clients that share a +/// gateway share its key. +#[cfg(target_os = "macos")] +const KEYCHAIN_SERVICE: &str = "Switchyard model list"; + +/// How long one listing may take before curl gives up. +const TIMEOUT_SECONDS: &str = "20"; + +/// Held while a thread rewrites the cache file, so that two threads that +/// finish at the same time do not drop each other's lists. +static CACHE_WRITE: Mutex<()> = Mutex::new(()); + +/// One fetched model list, as the cache file stores it. +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +pub struct ModelList { + /// The model IDs, sorted. + pub models: Vec, + /// When the list was fetched, in seconds since 1970-01-01 UTC. + pub fetched_at: u64, +} + +/// The model list at one URL, as [`load`] found it. +#[derive(Clone, Debug, PartialEq)] +pub struct Loaded { + /// The URL that serves the list. + pub url: String, + /// The newest list: fetched just now, or read from the cache file. + pub list: Option, + /// Why the list could not be fetched or saved in the cache file, when + /// that happened. + pub error: Option, +} + +/// Why a client's models could not be listed. +#[derive(Clone, Debug, PartialEq)] +pub enum ListError { + /// The client needs a key, and none is set or saved. + NoKey, + /// The Keychain did not return the saved key. + Keychain(String), + /// The server answered 401 or 403. + Rejected(u16), + Failed(String), + /// The list was fetched, but the cache file could not be written. + NotCached(String), +} + +impl ListError { + /// Says what went wrong, in one sentence. + pub fn reason(&self) -> String { + match self { + Self::NoKey => "This client needs a key to list models, and none is saved.".to_string(), + Self::Keychain(error) => { + format!("Could not read the saved key from the Keychain: {error}.") + } + Self::Rejected(status) => format!("The server rejected the key (HTTP {status})."), + Self::Failed(error) => format!("Could not list models: {error}."), + Self::NotCached(error) => format!("Could not save the list: {error}."), + } + } + + /// Says what the user can do while the window has no list to show. The + /// window always accepts a typed model ID, so every answer offers that. + pub fn advice(&self) -> &'static str { + if self.needs_key() { + "Paste the key below and click Save key, or type a model ID." + } else { + "Click Refresh models to try again, or type a model ID." + } + } + + /// Returns whether saving a key could fix this error. + pub fn needs_key(&self) -> bool { + matches!(self, Self::NoKey | Self::Keychain(_) | Self::Rejected(_)) + } +} + +/// Returns the model list at each URL that `clients` use, in the order the +/// URLs first appear. Clients whose models share a URL share one list and +/// one request. +/// +/// Without `refresh`, a list in the cache file comes back without a +/// request, and only a URL with no cached list is fetched. With `refresh`, +/// every URL is fetched. A fetched list replaces the one in the cache file +/// unless the file holds a newer list for that URL. When the file cannot be +/// written, the fetched list still comes back, with +/// [`ListError::NotCached`], and the file keeps its old list. When a fetch +/// fails, the cached list comes back with the error. +/// +/// `typed_key` is a key the user just entered. When it is set, clients that +/// send a key use it instead of their environment variable or the Keychain. +pub fn load( + cache: &Path, + clients: &[Client], + refresh: bool, + typed_key: Option<&str>, +) -> Vec { + let mut cached = read_cache(cache); + let mut saved_keys = HashMap::new(); + let mut fetched = BTreeMap::new(); + let mut loaded = Vec::new(); + for (url, client) in by_url(clients) { + let old = cached.remove(&url); + if old.is_some() && !refresh { + loaded.push(Loaded { + url, + list: old, + error: None, + }); + continue; + } + let result = + key(client, typed_key, &mut saved_keys).and_then(|key| list(client, key.as_deref())); + loaded.push(match result { + Ok(models) => { + let new = ModelList { + models, + fetched_at: now(), + }; + fetched.insert(url.clone(), new.clone()); + Loaded { + url, + list: Some(new), + error: None, + } + } + Err(error) => Loaded { + url, + list: old, + error: Some(error), + }, + }); + } + if !fetched.is_empty() + && let Err(error) = write_cache(cache, &fetched) + { + for entry in loaded + .iter_mut() + .filter(|entry| fetched.contains_key(&entry.url)) + { + entry.error = Some(ListError::NotCached(error.clone())); + } + } + loaded +} + +/// Returns the URL that serves the client's model list. It is also the +/// list's key in the cache file and in [`Loaded::url`]. +pub fn list_url(client: &Client) -> String { + models_url(&client.format, &client.base_url) +} + +/// Lists the client's model IDs, sorted. When the client needs a key and +/// `key` is `None`, returns [`ListError::NoKey`] without sending a request. +fn list(client: &Client, key: Option<&str>) -> Result, ListError> { + if sends_key(client) && key.is_none() { + return Err(ListError::NoKey); + } + fetch(&list_url(client), &client.format, key) +} + +/// Returns the models that contain every word of `query`, ignoring case. +pub fn matching<'a>(models: &'a [String], query: &str) -> Vec<&'a str> { + let query = query.to_lowercase(); + let words: Vec<&str> = query.split_whitespace().collect(); + models + .iter() + .map(String::as_str) + .filter(|model| { + let model = model.to_lowercase(); + words.iter().all(|word| model.contains(word)) + }) + .collect() +} + +/// Returns the models endpoint for a client. It builds the URL the way the +/// server builds its request URLs: OpenAI clients drop a trailing +/// `/chat/completions` or `/responses`, and Anthropic clients use +/// `/v1/models`. +fn models_url(format: &str, base_url: &str) -> String { + let (base, query) = match base_url.split_once('?') { + Some((base, query)) => (base, format!("?{query}")), + None => (base_url, String::new()), + }; + let base = base.trim_end_matches('/'); + let path = if format == "anthropic_messages" { + let root = base.strip_suffix("/messages").unwrap_or(base); + if root.ends_with("/v1") { + format!("{root}/models") + } else { + format!("{root}/v1/models") + } + } else { + let root = base + .strip_suffix("/chat/completions") + .or_else(|| base.strip_suffix("/responses")) + .unwrap_or(base); + format!("{root}/models") + }; + format!("{path}{query}") +} + +/// Pairs each models URL with the client whose settings fetch it: the first +/// client with that URL that sends a key, or else the first client with it. +fn by_url(clients: &[Client]) -> Vec<(String, &Client)> { + let mut urls: Vec<(String, &Client)> = Vec::new(); + for client in clients { + let url = list_url(client); + match urls.iter_mut().find(|(seen, _)| *seen == url) { + Some(entry) if !sends_key(entry.1) && sends_key(client) => entry.1 = client, + Some(_) => {} + None => urls.push((url, client)), + } + } + urls +} + +/// Returns whether the server sends this client a key. If it does, listing +/// the client's models needs a key too. +fn sends_key(client: &Client) -> bool { + client.forward_auth || client.api_key_env.is_some() +} + +/// Returns the key to list the client's models with, or `None` when the +/// client needs no key or none is available. +/// +/// The typed key comes first, then the client's `api_key_env` variable when +/// this process has it, then the Keychain item for the client's `base_url`. +/// `saved` remembers each Keychain answer, so macOS asks at most once per +/// `base_url` when it needs the user's permission to hand over a key. +fn key( + client: &Client, + typed_key: Option<&str>, + saved: &mut HashMap, String>>, +) -> Result, ListError> { + if !sends_key(client) { + return Ok(None); + } + if let Some(key) = typed_key.map(str::to_string).or_else(|| env_key(client)) { + return Ok(Some(key)); + } + saved + .entry(client.base_url.clone()) + .or_insert_with(|| saved_key(&client.base_url)) + .clone() + .map_err(ListError::Keychain) +} + +fn env_key(client: &Client) -> Option { + client + .api_key_env + .as_deref() + .and_then(|variable| std::env::var(variable).ok()) + .filter(|key| !key.trim().is_empty()) +} + +/// Returns the key saved in the Keychain for `base_url`, or `None` when no +/// key is saved. +#[cfg(target_os = "macos")] +fn saved_key(base_url: &str) -> Result, String> { + /// The Keychain's `errSecItemNotFound` status. + const NOT_FOUND: i32 = -25300; + match security_framework::passwords::get_generic_password(KEYCHAIN_SERVICE, base_url) { + Ok(bytes) => String::from_utf8(bytes) + .map(|key| Some(key).filter(|key| !key.trim().is_empty())) + .map_err(|_| "the saved key is not text".to_string()), + Err(error) if error.code() == NOT_FOUND => Ok(None), + Err(error) => Err(error.to_string()), + } +} + +#[cfg(not(target_os = "macos"))] +fn saved_key(_base_url: &str) -> Result, String> { + Ok(None) +} + +/// Saves a key in the login Keychain for every client with this `base_url`. +#[cfg(target_os = "macos")] +pub fn save_key(base_url: &str, key: &str) -> Result<(), String> { + security_framework::passwords::set_generic_password( + KEYCHAIN_SERVICE, + base_url, + key.trim().as_bytes(), + ) + .map_err(|error| error.to_string()) +} + +/// Reads the cache file. A missing or unreadable file reads as empty, so the +/// app fetches its lists again and writes a new file. +fn read_cache(path: &Path) -> BTreeMap { + std::fs::read_to_string(path) + .ok() + .and_then(|text| serde_json::from_str(&text).ok()) + .unwrap_or_default() +} + +/// Adds `lists` to the cache file. A list replaces the one at the same URL +/// unless that one is newer: two loads can overlap, and the load that +/// fetched a list first can write it last. The text goes to a temporary file +/// that is renamed over the cache file, so a reader never sees half a file. +fn write_cache(path: &Path, lists: &BTreeMap) -> Result<(), String> { + let _writing = CACHE_WRITE.lock().unwrap_or_else(PoisonError::into_inner); + let mut all = read_cache(path); + for (url, list) in lists { + if all + .get(url) + .is_none_or(|cached| cached.fetched_at <= list.fetched_at) + { + all.insert(url.clone(), list.clone()); + } + } + let text = serde_json::to_string_pretty(&all) + .map_err(|error| format!("write {}: {error}", path.display()))?; + let dir = path + .parent() + .filter(|dir| !dir.as_os_str().is_empty()) + .unwrap_or(Path::new(".")); + let mut file = tempfile::NamedTempFile::new_in(dir) + .map_err(|error| format!("create a file in {}: {error}", dir.display()))?; + file.write_all(text.as_bytes()) + .and_then(|()| file.as_file().sync_all()) + .map_err(|error| format!("write {}: {error}", path.display()))?; + file.persist(path) + .map_err(|error| format!("replace {}: {}", path.display(), error.error))?; + Ok(()) +} + +/// Returns the current time in seconds since 1970-01-01 UTC. +pub fn now() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_or(0, |since| since.as_secs()) +} + +fn fetch(url: &str, format: &str, key: Option<&str>) -> Result, ListError> { + let failed = |error: String| ListError::Failed(error); + let mut child = Command::new("curl") + // curl honors `-q` only as its first argument. `-q` makes curl ignore + // ~/.curlrc, where a `verbose` line would print the key into the + // error text that the window shows. + .args([ + "-q", + "--silent", + "--show-error", + "--max-time", + TIMEOUT_SECONDS, + ]) + // Read request headers from stdin, one per line. + .args(["--header", "@-"]) + .args(["--write-out", "\n%{http_code}", url]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .map_err(|error| failed(format!("run curl: {error}")))?; + if let Some(mut stdin) = child.stdin.take() { + let headers = match (key, format) { + (Some(key), "anthropic_messages") => { + format!("x-api-key: {key}\nanthropic-version: 2023-06-01\n") + } + (Some(key), _) => format!("Authorization: Bearer {key}\n"), + (None, _) => String::new(), + }; + stdin + .write_all(headers.as_bytes()) + .map_err(|error| failed(format!("send headers to curl: {error}")))?; + } + let output = child + .wait_with_output() + .map_err(|error| failed(format!("run curl: {error}")))?; + if !output.status.success() { + return Err(failed( + String::from_utf8_lossy(&output.stderr).trim().to_string(), + )); + } + + let stdout = String::from_utf8_lossy(&output.stdout); + let (body, status) = stdout.rsplit_once('\n').unwrap_or(("", &stdout)); + match status.trim().parse::() { + Ok(200) => parse_ids(body).map_err(failed), + Ok(status @ (401 | 403)) => Err(ListError::Rejected(status)), + Ok(status) => Err(failed(format!("{url} answered HTTP {status}"))), + Err(_) => Err(failed(format!("{url} sent no HTTP status"))), + } +} + +/// Parses the model IDs. OpenAI and Anthropic both answer with +/// `{"data": [{"id": ...}, ...]}`. +fn parse_ids(body: &str) -> Result, String> { + #[derive(Deserialize)] + struct Listing { + data: Vec, + } + #[derive(Deserialize)] + struct Entry { + id: String, + } + + let listing: Listing = serde_json::from_str(body) + .map_err(|error| format!("the model list is not the expected JSON: {error}"))?; + let mut ids: Vec = listing.data.into_iter().map(|entry| entry.id).collect(); + ids.sort(); + ids.dedup(); + Ok(ids) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::io::{BufRead, BufReader}; + use std::net::TcpListener; + use std::sync::Arc; + + /// A local HTTP server that answers every request with the current + /// `response` and records the head of each request. Its thread waits in + /// `accept` until the test process exits. + struct Stub { + /// The `base_url` of a client that lists models here. + url: String, + response: Arc>, + requests: Arc>>>, + } + + impl Stub { + fn start(response: String) -> Self { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind"); + let url = format!("http://{}/v1", listener.local_addr().expect("addr")); + let response = Arc::new(Mutex::new(response)); + let requests = Arc::new(Mutex::new(Vec::new())); + let (answer, record) = (Arc::clone(&response), Arc::clone(&requests)); + std::thread::spawn(move || { + for mut stream in listener.incoming().map_while(Result::ok) { + let mut head = Vec::new(); + let mut reader = BufReader::new(stream.try_clone().expect("clone stream")); + let mut line = String::new(); + while reader.read_line(&mut line).is_ok_and(|read| read > 0) && line != "\r\n" { + head.push(line.trim_end().to_string()); + line.clear(); + } + record.lock().expect("lock").push(head); + let response = answer.lock().expect("lock").clone(); + let _ = stream.write_all(response.as_bytes()); + } + }); + Self { + url, + response, + requests, + } + } + + fn answer(&self, response: String) { + *self.response.lock().expect("lock") = response; + } + + fn requests(&self) -> Vec> { + self.requests.lock().expect("lock").clone() + } + } + + fn listing(models: &[&str]) -> String { + let data: Vec<_> = models + .iter() + .map(|id| serde_json::json!({ "id": id })) + .collect(); + format!( + "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n{}", + serde_json::json!({ "data": data }) + ) + } + + fn client(name: &str, format: &str, base_url: &str) -> Client { + Client { + name: name.to_string(), + format: format.to_string(), + base_url: base_url.to_string(), + api_key_env: None, + forward_auth: true, + } + } + + fn models(loaded: &[Loaded]) -> Vec<&str> { + loaded + .iter() + .flat_map(|entry| entry.list.iter().flat_map(|list| &list.models)) + .map(String::as_str) + .collect() + } + + #[test] + fn fetches_a_missing_list_once_and_then_uses_the_cached_list() { + let stub = Stub::start(listing(&["model-b", "model-a"])); + let dir = tempfile::tempdir().expect("tempdir"); + let cache = dir.path().join(CACHE_FILE); + // Both clients list their models at the same URL. + let clients = [ + client("gateway", "openai_responses", &stub.url), + client("gateway_chat", "openai_chat", &stub.url), + ]; + + let first = load(&cache, &clients, false, Some("test-key")); + // No key this time: a cached list needs neither a request nor a key. + let second = load(&cache, &clients, false, None); + + let requests = stub.requests(); + assert_eq!(requests.len(), 1, "{requests:?}"); + assert_eq!(requests[0][0], "GET /v1/models HTTP/1.1"); + assert!( + requests[0].contains(&"Authorization: Bearer test-key".to_string()), + "{requests:?}" + ); + assert_eq!(first.len(), 1); + assert_eq!(first[0].url, format!("{}/models", stub.url)); + assert_eq!(first[0].error, None); + assert_eq!(models(&first), ["model-a", "model-b"]); + assert_eq!(second, first); + let text = std::fs::read_to_string(&cache).expect("read cache"); + assert!(!text.contains("test-key"), "the cache holds no key: {text}"); + } + + #[test] + fn refresh_fetches_again_and_replaces_the_cached_list() { + let stub = Stub::start(listing(&["old-model"])); + let dir = tempfile::tempdir().expect("tempdir"); + let cache = dir.path().join(CACHE_FILE); + let clients = [client("gateway", "openai_chat", &stub.url)]; + load(&cache, &clients, false, Some("test-key")); + stub.answer(listing(&["new-model"])); + + let refreshed = load(&cache, &clients, true, Some("test-key")); + let later = load(&cache, &clients, false, None); + + assert_eq!(stub.requests().len(), 2); + assert_eq!(models(&refreshed), ["new-model"]); + assert_eq!(refreshed[0].error, None); + assert_eq!(later, refreshed, "the cache file holds the new list"); + } + + #[test] + fn a_failed_refresh_keeps_the_cached_list() { + let stub = Stub::start(listing(&["old-model"])); + let dir = tempfile::tempdir().expect("tempdir"); + let cache = dir.path().join(CACHE_FILE); + let clients = [client("gateway", "openai_chat", &stub.url)]; + let first = load(&cache, &clients, false, Some("test-key")); + stub.answer( + "HTTP/1.1 500 Internal Server Error\r\nConnection: close\r\n\r\n{}".to_string(), + ); + + let refreshed = load(&cache, &clients, true, Some("test-key")); + let later = load(&cache, &clients, false, None); + + assert_eq!(stub.requests().len(), 2); + assert_eq!(refreshed[0].list, first[0].list); + assert_eq!( + refreshed[0].error, + Some(ListError::Failed(format!( + "{}/models answered HTTP 500", + stub.url + ))) + ); + assert_eq!(later, first, "the cache file still holds the old list"); + } + + #[test] + fn fetches_a_shared_url_with_the_client_that_sends_a_key() { + let stub = Stub::start(listing(&["model-a"])); + let dir = tempfile::tempdir().expect("tempdir"); + // The first client at the URL sends no key, so listing with it would + // leave out the key the second client needs. + let keyless = Client { + forward_auth: false, + ..client("open", "openai_chat", &stub.url) + }; + let clients = [keyless, client("gateway", "openai_responses", &stub.url)]; + + let loaded = load( + &dir.path().join(CACHE_FILE), + &clients, + false, + Some("test-key"), + ); + + let requests = stub.requests(); + assert_eq!(requests.len(), 1, "{requests:?}"); + assert!( + requests[0].contains(&"Authorization: Bearer test-key".to_string()), + "{requests:?}" + ); + assert_eq!(models(&loaded), ["model-a"]); + } + + #[test] + fn returns_a_fetched_list_that_the_cache_file_cannot_hold() { + let stub = Stub::start(listing(&["model-a"])); + let dir = tempfile::tempdir().expect("tempdir"); + // The cache file's directory does not exist, so the file cannot be + // written. + let cache = dir.path().join("missing").join(CACHE_FILE); + let clients = [client("gateway", "openai_chat", &stub.url)]; + + let loaded = load(&cache, &clients, false, Some("test-key")); + + assert_eq!(models(&loaded), ["model-a"]); + assert!( + matches!(loaded[0].error, Some(ListError::NotCached(_))), + "{loaded:?}" + ); + assert!(!cache.exists()); + } + + #[test] + fn reports_a_rejected_key_and_a_missing_key() { + let stub = + Stub::start("HTTP/1.1 401 Unauthorized\r\nConnection: close\r\n\r\n{}".to_string()); + let gateway = client("gateway", "openai_chat", &stub.url); + + assert_eq!(list(&gateway, Some("wrong")), Err(ListError::Rejected(401))); + assert_eq!(list(&gateway, None), Err(ListError::NoKey)); + assert_eq!(stub.requests().len(), 1, "a missing key sends no request"); + } + + #[test] + fn builds_the_models_url_like_the_server_builds_request_urls() { + for (format, base_url, expected) in [ + ( + "openai_chat", + "https://api.example/v1/", + "https://api.example/v1/models", + ), + ( + "openai_responses", + "https://api.example/v1/responses", + "https://api.example/v1/models", + ), + ( + "anthropic_messages", + "https://api.example", + "https://api.example/v1/models", + ), + ( + "anthropic_messages", + "https://api.example/v1/messages", + "https://api.example/v1/models", + ), + ( + "openai_chat", + "https://api.example/v1?api-version=1", + "https://api.example/v1/models?api-version=1", + ), + ] { + assert_eq!( + models_url(format, base_url), + expected, + "{format} {base_url}" + ); + } + } + + #[test] + fn matches_every_word_ignoring_case() { + let models = [ + "claude-opus-5-5".to_string(), + "claude-sonnet-5".to_string(), + "gpt-5.6-sol".to_string(), + ]; + + assert_eq!(matching(&models, "CLAUDE opus"), ["claude-opus-5-5"]); + assert_eq!(matching(&models, "").len(), 3); + } +} diff --git a/crates/switchyard-menubar/src/picker.rs b/crates/switchyard-menubar/src/picker.rs new file mode 100644 index 000000000..a860a1747 --- /dev/null +++ b/crates/switchyard-menubar/src/picker.rs @@ -0,0 +1,1096 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! The "Change routing…" window. The user picks a route, its algorithm, and +//! a model for each role, and Apply checks, saves, and restarts the server. +//! +//! The window only collects choices. `server_config` edits the config, +//! `models` lists and caches models, and `server` checks, saves, and +//! restarts, so all of that runs without a GUI. Model lists, Keychain access, +//! and Apply run on worker threads, which send their results through a +//! channel that the menu bar's event loop drains by calling [`Picker::poll`]. + +use std::cell::RefCell; +use std::collections::HashMap; +use std::path::PathBuf; +use std::sync::mpsc::{Receiver, Sender, channel}; + +use objc2::rc::Retained; +use objc2::runtime::ProtocolObject; +use objc2::{DefinedClass, MainThreadOnly, define_class, msg_send, sel}; +use objc2_app_kit::{ + NSApplication, NSBackingStoreType, NSButton, NSColor, NSComboBox, NSComboBoxDelegate, + NSControl, NSControlTextEditingDelegate, NSFont, NSPopUpButton, NSSecureTextField, NSTextField, + NSTextFieldDelegate, NSView, NSWindow, NSWindowStyleMask, +}; +use objc2_foundation::{ + MainThreadMarker, NSNotification, NSObject, NSObjectProtocol, NSPoint, NSRect, NSSize, + NSString, ns_string, +}; + +use crate::config::Config; +use crate::models::{self, ListError, Loaded, ModelList}; +use crate::server; +use crate::server_config::{ALGORITHMS, Choice, Client, Role, Route, ServerConfig, Tier}; + +// Control tags. Role rows use `CLIENT_TAG + row` for the client popup and +// `MODEL_TAG + row` for the model box. A pick from the model list arrives as +// `SELECTED_TAG + row`. +const ROUTE_TAG: isize = 1; +const ALGORITHM_TAG: isize = 2; +const SAVE_KEY_TAG: isize = 3; +const APPLY_TAG: isize = 4; +const REFRESH_TAG: isize = 5; +const CLIENT_TAG: isize = 100; +const MODEL_TAG: isize = 200; +const SELECTED_TAG: isize = 300; + +const WIDTH: f64 = 780.0; +const MARGIN: f64 = 20.0; +const FIELD_X: f64 = 110.0; +const FIELD_WIDTH: f64 = WIDTH - FIELD_X - MARGIN; +const CLIENT_WIDTH: f64 = 290.0; +const MODEL_X: f64 = FIELD_X + CLIENT_WIDTH + 10.0; +const MODEL_WIDTH: f64 = WIDTH - MODEL_X - MARGIN; +/// Height of one role row: the popup and combo box, then a note of up to +/// three lines under them. +const ROLE_HEIGHT: f64 = 80.0; +const NOTE_HEIGHT: f64 = 44.0; +/// Height of the key area: two lines of text above the key field. +const KEY_HEIGHT: f64 = 64.0; +/// Height of the algorithm's description: up to two lines, then its roles. +const SUMMARY_HEIGHT: f64 = 60.0; +/// Height of the result area, which fits a wrapped `--dry-run` error. +const STATUS_HEIGHT: f64 = 124.0; + +define_class!( + /// Receives every control's action and text change, and queues the + /// control's tag for [`Picker::poll`]. The picker handles the tags later, + /// outside AppKit's callback, so an event that AppKit sends while the + /// picker updates its controls only adds a tag and does not reenter it. + // SAFETY: NSObject has no subclassing requirements, and `ControlTarget` + // does not implement `Drop`. + #[unsafe(super(NSObject))] + #[thread_kind = MainThreadOnly] + #[name = "SwitchyardPickerControlTarget"] + #[ivars = RefCell>] + struct ControlTarget; + + impl ControlTarget { + // SAFETY: AppKit calls an action method with the control that sent it. + #[unsafe(method(controlChanged:))] + fn control_changed(&self, sender: &NSControl) { + self.ivars().borrow_mut().push(sender.tag()); + } + } + + unsafe impl NSObjectProtocol for ControlTarget {} + + unsafe impl NSControlTextEditingDelegate for ControlTarget { + // SAFETY: the signature matches the protocol method. + #[unsafe(method(controlTextDidChange:))] + fn control_text_did_change(&self, notification: &NSNotification) { + let control = notification + .object() + .and_then(|object| object.downcast::().ok()); + if let Some(control) = control { + self.ivars().borrow_mut().push(control.tag()); + } + } + } + + unsafe impl NSTextFieldDelegate for ControlTarget {} + + unsafe impl NSComboBoxDelegate for ControlTarget { + // SAFETY: the signature matches the protocol method. + #[unsafe(method(comboBoxSelectionDidChange:))] + fn combo_box_selection_did_change(&self, notification: &NSNotification) { + let control = notification + .object() + .and_then(|object| object.downcast::().ok()); + if let Some(control) = control { + self.ivars() + .borrow_mut() + .push(control.tag() - MODEL_TAG + SELECTED_TAG); + } + } + } +); + +impl ControlTarget { + fn new(mtm: MainThreadMarker) -> Retained { + let this = Self::alloc(mtm).set_ivars(RefCell::new(Vec::new())); + // SAFETY: NSObject's `init` has this signature. + unsafe { msg_send![super(this), init] } + } +} + +/// A result that a worker thread sends back to the window. +enum Done { + /// Model lists that load number `load` found. `status`, when set, is the + /// outcome to show in the result area, because the user asked for this + /// load. + Listed { + load: u64, + lists: Vec, + status: Option, + }, + Applied(Result), +} + +/// What the window knows about the model list at one URL. +#[derive(Default)] +struct ListState { + /// The newest list: fetched, or read from the cache file. + list: Option, + /// Why the last load failed, when it did. + error: Option, + /// `started` is the newest load started for this list, and `shown` is the + /// newest load whose result the window shows. Loads are numbered in the + /// order they start. + started: u64, + shown: u64, +} + +impl ListState { + /// Returns whether a worker thread is loading this list now. + fn loading(&self) -> bool { + self.started > self.shown + } + + /// Records the result of load number `load`. A worker that started + /// earlier can finish later, so the window ignores a result from a load + /// older than the one it shows. Otherwise the error comes from this + /// result, and the newer of the two lists stays, because a failed load + /// returns the list from the cache file, which can be older than the one + /// the window has. The result of the newest load ends the loading state. + fn finish(&mut self, load: u64, list: Option, error: Option) { + if load < self.shown { + return; + } + self.shown = load; + let fetched_at = |list: &Option| list.as_ref().map(|list| list.fetched_at); + if fetched_at(&list) >= fetched_at(&self.list) { + self.list = list; + } + self.error = error; + } +} + +/// The controls of one role row. +struct RoleRow { + client: Retained, + model: Retained, + note: Retained, +} + +/// The controls that the picker reads or updates after it lays them out. +struct Controls { + route: Retained, + algorithm: Retained, + rows: Vec, + key_label: Retained, + key: Retained, + save_key: Retained, + status: Retained, + apply: Retained, +} + +/// The picker window and what the user has chosen in it. +pub struct Picker { + mtm: MainThreadMarker, + window: Retained, + target: Retained, + settings: Config, + /// The file that keeps fetched model lists. + cache: PathBuf, + config: Option, + routes: Vec, + clients: Vec, + route: usize, + algorithm: usize, + roles: Vec, + /// One choice per role, in role order. + choices: Vec, + /// The latest choice for each tier on this route. When the next algorithm + /// has no role for a tier, its choice stays here, so switching back + /// restores it. `select_route` clears the map. + by_tier: HashMap, + /// Model lists by URL, kept while the app runs. Clients that list their + /// models at the same URL share one entry. + lists: HashMap, + /// How many list loads the window has started. It numbers the loads. + loads: u64, + status: String, + /// Whether an Apply is running. + busy: bool, + sender: Sender, + receiver: Receiver, + controls: Option, +} + +impl Picker { + pub fn new(mtm: MainThreadMarker, settings: &Config, cache: PathBuf) -> Self { + // SAFETY: the window is not released when closed, because `Picker` + // owns it and shows it again from the menu. + let window = unsafe { + let window = NSWindow::initWithContentRect_styleMask_backing_defer( + NSWindow::alloc(mtm), + rect(0.0, 0.0, WIDTH, 400.0), + NSWindowStyleMask::Titled + | NSWindowStyleMask::Closable + | NSWindowStyleMask::Miniaturizable, + NSBackingStoreType::Buffered, + false, + ); + window.setReleasedWhenClosed(false); + window + }; + window.setTitle(ns_string!("Change routing")); + let (sender, receiver) = channel(); + Self { + mtm, + window, + target: ControlTarget::new(mtm), + settings: settings.clone(), + cache, + config: None, + routes: Vec::new(), + clients: Vec::new(), + route: 0, + algorithm: 0, + roles: Vec::new(), + choices: Vec::new(), + by_tier: HashMap::new(), + lists: HashMap::new(), + loads: 0, + status: String::new(), + busy: false, + sender, + receiver, + controls: None, + } + } + + /// Rereads the server config and brings the window to the front. + pub fn show(&mut self) { + if !self.busy { + // Opening the window again retries a list that failed and has + // nothing to show. The window fetches a list that it already has + // only when the user clicks Refresh models or Save key. + self.lists + .retain(|_, state| state.loading() || state.list.is_some()); + let route = self.routes.get(self.route).map(|route| route.key.clone()); + self.load(route.as_deref()); + } + let first = self.controls.is_none(); + self.layout(); + if first { + self.window.center(); + } + // An accessory app must activate itself before its window can take + // keyboard focus. `activate` needs macOS 14, so use the older call. + #[allow(deprecated)] + NSApplication::sharedApplication(self.mtm).activateIgnoringOtherApps(true); + self.window.makeKeyAndOrderFront(None); + } + + /// Handles queued control events and finished background work. + pub fn poll(&mut self) { + let tags = std::mem::take(&mut *self.target.ivars().borrow_mut()); + for tag in tags { + self.on_control(tag); + } + while let Ok(done) = self.receiver.try_recv() { + self.on_done(done); + } + } + + fn load(&mut self, route: Option<&str>) { + let path = &self.settings.config_file; + let loaded = std::fs::read_to_string(path) + .map_err(|error| format!("read {}: {error}", path.display())) + .and_then(|text| ServerConfig::parse(&text)); + match loaded { + Ok(config) => { + self.routes = config.routes(); + self.clients = config.clients(); + self.route = route + .and_then(|key| self.routes.iter().position(|route| route.key == key)) + .unwrap_or(0); + self.config = Some(config); + self.select_route(); + if self.routes.is_empty() { + self.status = format!( + "{} has no routes to change.", + self.settings.config_file.display() + ); + } + } + Err(error) => { + self.config = None; + self.routes.clear(); + self.clients.clear(); + self.roles.clear(); + self.choices.clear(); + self.status = error; + } + } + } + + /// Shows the selected route's current algorithm and models. + fn select_route(&mut self) { + self.by_tier.clear(); + let (Some(config), Some(route)) = (&self.config, self.routes.get(self.route)) else { + self.roles.clear(); + self.choices.clear(); + return; + }; + match config.algorithm(&route.key) { + Some(current) => { + self.algorithm = ALGORITHMS + .iter() + .position(|algorithm| algorithm.kind == current.kind) + .unwrap_or(0); + self.roles = config.roles(&route.key, current); + self.choices = config.choices(&route.key); + self.status.clear(); + } + None => { + self.algorithm = 0; + self.roles = config.roles(&route.key, &ALGORITHMS[0]); + self.choices = vec![Choice::default(); self.roles.len()]; + self.status = format!( + "{} is a {} route, which this window cannot show. Applying replaces its \ + settings with the algorithm you choose.", + route.id, route.kind + ); + } + } + self.fill_clients(); + self.list_models(); + } + + /// Switches the algorithm. Each new role starts from the latest choice + /// for its tier on this route, so the capable model stays the capable one. + /// Choices for tiers the new algorithm lacks stay in `by_tier`. + fn select_algorithm(&mut self, index: usize) { + self.read_models(); + for (role, choice) in self.roles.iter().zip(&self.choices) { + if let Some(tier) = role.tier { + self.by_tier.insert(tier, choice.clone()); + } + } + let (Some(config), Some(route), Some(algorithm)) = ( + &self.config, + self.routes.get(self.route), + ALGORITHMS.get(index), + ) else { + return; + }; + let roles = config.roles(&route.key, algorithm); + self.choices = roles + .iter() + .map(|role| { + role.tier + .and_then(|tier| self.by_tier.get(&tier).cloned()) + .unwrap_or_default() + }) + .collect(); + self.roles = roles; + self.algorithm = index; + self.fill_clients(); + self.list_models(); + self.layout(); + } + + /// Gives every role without a client the first role's client. + fn fill_clients(&mut self) { + let fallback = self + .choices + .iter() + .map(|choice| &choice.client) + .find(|client| !client.is_empty()) + .or_else(|| self.clients.first().map(|client| &client.name)) + .cloned() + .unwrap_or_default(); + for choice in &mut self.choices { + if choice.client.is_empty() { + choice.client = fallback.clone(); + } + } + } + + /// Loads the lists of the rows' clients that the window does not have: + /// from the cache file, or fetched when the cache file has none. + fn list_models(&mut self) { + let clients = self.row_clients(|state| state.is_none()); + if !clients.is_empty() { + self.load_lists(clients, false, None); + } + } + + /// Fetches the lists that the rows use again, one request per URL. + fn refresh_models(&mut self) { + let clients = self.row_clients(|state| !state.is_some_and(ListState::loading)); + if clients.is_empty() { + return; + } + self.set_status("Refreshing the model lists…".to_string()); + self.load_lists(clients, true, None); + } + + /// Returns the clients that the rows use and whose list state passes + /// `wanted`, each client once. + fn row_clients(&self, wanted: impl Fn(Option<&ListState>) -> bool) -> Vec { + let mut clients: Vec = Vec::new(); + for choice in &self.choices { + if let Some(client) = self.client(&choice.client) + && wanted(self.lists.get(&models::list_url(client))) + && !clients.contains(client) + { + clients.push(client.clone()); + } + } + clients + } + + /// Loads the lists that `clients` use on a worker thread. With + /// `refresh`, the worker fetches them even when the cache file has + /// them, and the outcome replaces the result area's text, because the + /// user asked for it. `key` is a key the user just typed, with its + /// `base_url`; the worker saves it in the Keychain before it fetches. + fn load_lists(&mut self, clients: Vec, refresh: bool, key: Option<(String, String)>) { + self.loads += 1; + let load = self.loads; + for client in &clients { + self.lists + .entry(models::list_url(client)) + .or_default() + .started = load; + } + self.update_rows(); + let cache = self.cache.clone(); + let sender = self.sender.clone(); + std::thread::spawn(move || { + let mut status = Vec::new(); + if let Some((base_url, key)) = &key { + status.push(match models::save_key(base_url, key) { + Ok(()) => format!("Saved the key for {base_url} in your login Keychain."), + Err(error) => format!( + "Could not save the key in your login Keychain: {error}. The window \ + used the key for this list only." + ), + }); + } + let typed_key = key.as_ref().map(|(_, key)| key.as_str()); + let lists = models::load(&cache, &clients, refresh, typed_key); + let status = refresh.then(|| { + status.extend(lists.iter().map(outcome)); + status.join("\n") + }); + let _ = sender.send(Done::Listed { + load, + lists, + status, + }); + }); + } + + fn client(&self, name: &str) -> Option<&Client> { + self.clients.iter().find(|client| client.name == name) + } + + fn on_control(&mut self, tag: isize) { + let Some(controls) = &self.controls else { + return; + }; + match tag { + ROUTE_TAG => { + if let Some(index) = selected(&controls.route) + && index != self.route + { + self.route = index; + self.select_route(); + self.layout(); + } + } + ALGORITHM_TAG => { + if let Some(index) = selected(&controls.algorithm) + && index != self.algorithm + { + self.select_algorithm(index); + } + } + SAVE_KEY_TAG => self.save_key(), + REFRESH_TAG => self.refresh_models(), + APPLY_TAG => self.apply(), + _ if (CLIENT_TAG..MODEL_TAG).contains(&tag) => { + let row = tag.abs_diff(CLIENT_TAG); + let client = controls + .rows + .get(row) + .and_then(|ui| selected(&ui.client)) + .and_then(|index| self.clients.get(index)) + .map(|client| client.name.clone()); + if let (Some(client), Some(choice)) = (client, self.choices.get_mut(row)) { + choice.client = client; + self.list_models(); + self.update_row(row); + } + } + _ if (MODEL_TAG..SELECTED_TAG).contains(&tag) => { + let row = tag.abs_diff(MODEL_TAG); + if let (Some(ui), Some(choice)) = + (controls.rows.get(row), self.choices.get_mut(row)) + { + choice.model = ui.model.stringValue().to_string(); + } + self.update_row(row); + } + _ if tag >= SELECTED_TAG => { + // The box shows the picked model only after this event, so + // read the picked item rather than the box's text. + let row = tag.abs_diff(SELECTED_TAG); + let picked = controls + .rows + .get(row) + .and_then(|ui| ui.model.objectValueOfSelectedItem()) + .and_then(|value| value.downcast::().ok()); + if let (Some(picked), Some(choice)) = (picked, self.choices.get_mut(row)) { + choice.model = picked.to_string(); + self.update_row(row); + } + } + _ => {} + } + } + + fn on_done(&mut self, done: Done) { + match done { + Done::Listed { + load, + lists, + status, + } => { + for Loaded { url, list, error } in lists { + self.lists.entry(url).or_default().finish(load, list, error); + } + self.update_rows(); + if let Some(status) = status { + self.set_status(status); + } + } + Done::Applied(result) => { + self.busy = false; + match result { + Ok(message) => { + let unpriced = self.unpriced_models(); + let route = self.routes.get(self.route).map(|route| route.key.clone()); + self.load(route.as_deref()); + self.layout(); + self.set_status(format!("{message}{unpriced}")); + } + Err(error) => self.set_status(format!("Not saved. {error}")), + } + } + } + } + + /// Returns a note for each chosen model that has no price in menubar.toml. + fn unpriced_models(&self) -> String { + let mut notes = String::new(); + let mut seen = Vec::new(); + for choice in &self.choices { + let model = choice.model.trim(); + if !self.settings.prices.contains_key(model) && !seen.contains(&model) { + seen.push(model); + notes.push_str(&format!( + "\nmenubar.toml has no price for {model}, so savings stay hidden until you \ + add one." + )); + } + } + notes + } + + /// Saves the typed key for the key field's `base_url`, then fetches the + /// lists of every client with that `base_url` using the typed key. + fn save_key(&mut self) { + let (Some(controls), Some(base_url)) = (&self.controls, self.key_url().map(str::to_string)) + else { + return; + }; + let key = controls.key.stringValue().to_string().trim().to_string(); + controls.key.setStringValue(ns_string!("")); + if key.is_empty() { + self.set_status("Paste a key into the field first.".to_string()); + return; + } + let clients: Vec = self + .clients + .iter() + .filter(|client| client.base_url == base_url) + .cloned() + .collect(); + self.set_status(format!( + "Saving the key for {base_url} and listing its models…" + )); + self.load_lists(clients, true, Some((base_url, key))); + } + + fn apply(&mut self) { + if self.busy { + return; + } + self.read_models(); + let (Some(route), Some(algorithm)) = + (self.routes.get(self.route), ALGORITHMS.get(self.algorithm)) + else { + return; + }; + let route = route.key.clone(); + let choices = self.choices.clone(); + let settings = self.settings.clone(); + let sender = self.sender.clone(); + std::thread::spawn(move || { + let result = server::apply(&settings, &route, algorithm, &choices); + let _ = sender.send(Done::Applied(result)); + }); + self.busy = true; + self.set_status("Checking the config with switchyard-server --dry-run…".to_string()); + } + + /// Copies the typed model IDs into the choices. + fn read_models(&mut self) { + let Some(controls) = &self.controls else { + return; + }; + for (row, choice) in controls.rows.iter().zip(&mut self.choices) { + choice.model = row.model.stringValue().to_string(); + } + } + + fn set_status(&mut self, status: String) { + self.status = status; + if let Some(controls) = &self.controls { + controls + .status + .setStringValue(&NSString::from_str(&self.status)); + controls + .apply + .setEnabled(!self.busy && self.config.is_some()); + } + } + + fn update_rows(&self) { + for row in 0..self.choices.len() { + self.update_row(row); + } + let Some(controls) = &self.controls else { + return; + }; + let key_url = self.key_url(); + let hidden = key_url.is_none(); + controls.key_label.setHidden(hidden); + controls.key.setHidden(hidden); + controls.save_key.setHidden(hidden); + if let Some(url) = key_url { + controls + .key_label + .setStringValue(&NSString::from_str(&format!( + "To list the models at {url}, paste its key here and click Save key. The \ + app keeps the key in your login Keychain, not in a file." + ))); + } + } + + /// Returns the `base_url` that the key field saves a key for: the + /// `base_url` of the first row whose list failed because it needs a + /// working key. + fn key_url(&self) -> Option<&str> { + self.choices.iter().find_map(|choice| { + let client = self.client(&choice.client)?; + let error = self.lists.get(&models::list_url(client))?.error.as_ref()?; + error.needs_key().then_some(client.base_url.as_str()) + }) + } + + /// Fills a row's model list and writes the note under it. + fn update_row(&self, row: usize) { + let (Some(controls), Some(choice)) = (&self.controls, self.choices.get(row)) else { + return; + }; + let Some(ui) = controls.rows.get(row) else { + return; + }; + ui.model.removeAllItems(); + let client = self.client(&choice.client); + let state = client.and_then(|client| self.lists.get(&models::list_url(client))); + let loading = state.is_some_and(ListState::loading); + let note = match (client, state) { + (None, _) if choice.client.is_empty() => { + "The config has no llm clients. Type a model ID.".to_string() + } + (None, _) => format!( + "The config has no llm client named \"{}\". Type a model ID.", + choice.client + ), + ( + Some(_), + Some(ListState { + list: Some(list), + error, + .. + }), + ) => { + let mut note = fill(ui, list, &choice.model); + if loading { + note.push_str(" Refreshing…"); + } else if let Some(error) = error { + note.push('\n'); + note.push_str(&error.reason()); + } + note + } + ( + Some(_), + Some(ListState { + list: None, + error: Some(error), + .. + }), + ) if !loading => format!("No model list yet. {} {}", error.reason(), error.advice()), + (Some(_), _) => "Loading models…".to_string(), + }; + let note = NSString::from_str(¬e); + ui.note.setStringValue(¬e); + // The note fits three lines, and its tooltip shows the whole text. + ui.note.setToolTip(Some(¬e)); + } + + /// Rebuilds the window's controls for the current route and algorithm. + fn layout(&mut self) { + let mtm = self.mtm; + let height = 2.0 * MARGIN + + 34.0 + + 32.0 + + SUMMARY_HEIGHT + + ROLE_HEIGHT * self.roles.len() as f64 + + KEY_HEIGHT + + STATUS_HEIGHT + + 32.0; + let view = NSView::initWithFrame(NSView::alloc(mtm), rect(0.0, 0.0, WIDTH, height)); + // Controls are placed from the top; AppKit's origin is bottom left. + let mut top = MARGIN; + let mut next = |row_height: f64| { + let y = height - top - row_height; + top += row_height; + y + }; + + let y = next(34.0); + view.addSubview(&self.label("Route", rect(MARGIN, y + 12.0, 80.0, 18.0))); + let routes: Vec = self.routes.iter().map(|route| route.id.clone()).collect(); + let route = self.popup( + &routes, + self.route, + rect(FIELD_X, y + 8.0, FIELD_WIDTH, 26.0), + ); + route.setTag(ROUTE_TAG); + view.addSubview(&route); + + let y = next(32.0); + view.addSubview(&self.label("Algorithm", rect(MARGIN, y + 10.0, 80.0, 18.0))); + let kinds: Vec = ALGORITHMS.iter().map(|a| a.kind.to_string()).collect(); + let algorithm = self.popup( + &kinds, + self.algorithm, + rect(FIELD_X, y + 6.0, FIELD_WIDTH, 26.0), + ); + algorithm.setTag(ALGORITHM_TAG); + view.addSubview(&algorithm); + + let y = next(SUMMARY_HEIGHT); + let roles: Vec<&str> = self.roles.iter().map(|role| role.label.as_str()).collect(); + let summary = format!( + "{}\nRoles: {}.", + ALGORITHMS.get(self.algorithm).map_or("", |a| a.summary), + roles.join(", ") + ); + let summary = self.wrapping( + &summary, + rect(FIELD_X, y + 4.0, FIELD_WIDTH, SUMMARY_HEIGHT - 8.0), + ); + view.addSubview(&summary); + + let clients: Vec = self + .clients + .iter() + .map(|client| format!("{} ({})", client.name, client.format)) + .collect(); + let mut rows = Vec::with_capacity(self.roles.len()); + for (row, (role, choice)) in self.roles.iter().zip(&self.choices).enumerate() { + let y = next(ROLE_HEIGHT); + let tag = isize::try_from(row).unwrap_or(0); + // The popup and the combo box sit above the note. + let controls_y = y + NOTE_HEIGHT + 8.0; + view.addSubview(&self.label(&role.label, rect(MARGIN, controls_y + 4.0, 86.0, 18.0))); + let selected = self + .clients + .iter() + .position(|client| client.name == choice.client) + .unwrap_or(0); + let client = self.popup( + &clients, + selected, + rect(FIELD_X, controls_y, CLIENT_WIDTH, 26.0), + ); + client.setTag(CLIENT_TAG + tag); + view.addSubview(&client); + + let model = NSComboBox::initWithFrame( + NSComboBox::alloc(mtm), + rect(MODEL_X, controls_y, MODEL_WIDTH, 26.0), + ); + model.setStringValue(&NSString::from_str(&choice.model)); + model.setPlaceholderString(Some(ns_string!("Type to search, or enter a model ID"))); + model.setNumberOfVisibleItems(14); + model.setCompletes(false); + model.setTag(MODEL_TAG + tag); + // SAFETY: the target lives as long as the window. + unsafe { model.setDelegate(Some(ProtocolObject::from_ref(&*self.target))) }; + view.addSubview(&model); + + let note = self.wrapping( + "", + rect(MODEL_X + 2.0, y + 4.0, MODEL_WIDTH - 2.0, NOTE_HEIGHT), + ); + note.setFont(Some(&NSFont::systemFontOfSize( + NSFont::smallSystemFontSize(), + ))); + note.setTextColor(Some(&NSColor::secondaryLabelColor())); + view.addSubview(¬e); + rows.push(RoleRow { + client, + model, + note, + }); + } + + let y = next(KEY_HEIGHT); + let key_label = self.wrapping("", rect(FIELD_X, y + 30.0, FIELD_WIDTH, 32.0)); + view.addSubview(&key_label); + let key = NSSecureTextField::initWithFrame( + NSSecureTextField::alloc(mtm), + rect(FIELD_X, y + 4.0, FIELD_WIDTH - 110.0, 22.0), + ); + key.setPlaceholderString(Some(ns_string!("Paste the key"))); + view.addSubview(&key); + let save_key = self.button( + "Save key", + SAVE_KEY_TAG, + rect(WIDTH - MARGIN - 100.0, y, 100.0, 30.0), + ); + view.addSubview(&save_key); + + let y = next(STATUS_HEIGHT); + let status = self.wrapping( + &self.status, + rect(MARGIN, y + 4.0, WIDTH - 2.0 * MARGIN, STATUS_HEIGHT - 8.0), + ); + status.setSelectable(true); + view.addSubview(&status); + + let refresh = self.button( + "Refresh models", + REFRESH_TAG, + rect(MARGIN, MARGIN - 4.0, 150.0, 32.0), + ); + refresh.setEnabled(!self.choices.is_empty()); + view.addSubview(&refresh); + let apply = self.button( + "Apply", + APPLY_TAG, + rect(WIDTH - MARGIN - 100.0, MARGIN - 4.0, 100.0, 32.0), + ); + apply.setEnabled(!self.busy && self.config.is_some()); + view.addSubview(&apply); + + // Keep the window's top edge in place as its height changes. + let frame = self.window.frame(); + let top_left = NSPoint::new(frame.origin.x, frame.origin.y + frame.size.height); + self.window.setContentSize(NSSize::new(WIDTH, height)); + self.window.setContentView(Some(&view)); + self.window.setFrameTopLeftPoint(top_left); + + self.controls = Some(Controls { + route, + algorithm, + rows, + key_label, + key, + save_key, + status, + apply, + }); + self.update_rows(); + } + + fn label(&self, text: &str, frame: NSRect) -> Retained { + let label = NSTextField::labelWithString(&NSString::from_str(text), self.mtm); + label.setFrame(frame); + label + } + + fn wrapping(&self, text: &str, frame: NSRect) -> Retained { + let label = NSTextField::wrappingLabelWithString(&NSString::from_str(text), self.mtm); + label.setFrame(frame); + label + } + + fn popup(&self, items: &[String], selected: usize, frame: NSRect) -> Retained { + let popup = + NSPopUpButton::initWithFrame_pullsDown(NSPopUpButton::alloc(self.mtm), frame, false); + for item in items { + popup.addItemWithTitle(&NSString::from_str(item)); + } + popup.selectItemAtIndex(isize::try_from(selected).unwrap_or(0)); + // SAFETY: `ControlTarget` implements `controlChanged:` and lives as + // long as the window. + unsafe { + popup.setTarget(Some(&self.target)); + popup.setAction(Some(sel!(controlChanged:))); + } + popup + } + + fn button(&self, title: &str, tag: isize, frame: NSRect) -> Retained { + // SAFETY: `ControlTarget` implements `controlChanged:` and lives as + // long as the window. + let button = unsafe { + NSButton::buttonWithTitle_target_action( + &NSString::from_str(title), + Some(&self.target), + Some(sel!(controlChanged:)), + self.mtm, + ) + }; + button.setFrame(frame); + button.setTag(tag); + button + } +} + +/// The selected item of a popup, if any. +fn selected(popup: &NSPopUpButton) -> Option { + usize::try_from(popup.indexOfSelectedItem()).ok() +} + +fn rect(x: f64, y: f64, width: f64, height: f64) -> NSRect { + NSRect::new(NSPoint::new(x, y), NSSize::new(width, height)) +} + +/// Puts the listed models that match `query` in the row's list, and says how +/// many there are. +fn fill(ui: &RoleRow, list: &ModelList, query: &str) -> String { + let query = query.trim(); + let listed = &list.models; + // A complete ID shows the whole list, so another model is one click away. + let shown: Vec<&str> = if listed.iter().any(|model| model == query) { + listed.iter().map(String::as_str).collect() + } else { + models::matching(listed, query) + }; + for model in &shown { + // SAFETY: the combo box stores NSString values. + unsafe { ui.model.addItemWithObjectValue(&NSString::from_str(model)) }; + } + if query.is_empty() || shown.len() == listed.len() { + format!( + "{} models, listed {}. Type to filter.", + listed.len(), + age(list.fetched_at) + ) + } else if shown.is_empty() { + "No listed model matches. Apply uses the ID as typed.".to_string() + } else { + format!("{} of {} models match.", shown.len(), listed.len()) + } +} + +/// Describes one loaded list for the result area. +fn outcome(loaded: &Loaded) -> String { + let mut line = format!("{}:", loaded.url); + if let Some(list) = &loaded.list { + line.push_str(&format!( + " {} models, listed {}.", + list.models.len(), + age(list.fetched_at) + )); + } + if let Some(error) = &loaded.error { + line.push(' '); + line.push_str(&error.reason()); + } + line +} + +/// Says how long ago `fetched_at` was, such as "3 hours ago". +fn age(fetched_at: u64) -> String { + let minutes = models::now().saturating_sub(fetched_at) / 60; + let (count, unit) = match minutes { + 0 => return "just now".to_string(), + 1..60 => (minutes, "minute"), + 60..1440 => (minutes / 60, "hour"), + _ => (minutes / 1440, "day"), + }; + let plural = if count == 1 { "" } else { "s" }; + format!("{count} {unit}{plural} ago") +} + +#[cfg(test)] +mod tests { + use super::*; + + fn list(fetched_at: u64) -> Option { + Some(ModelList { + models: vec!["model".to_string()], + fetched_at, + }) + } + + #[test] + fn a_failed_load_keeps_the_newer_list_and_shows_the_error() { + // Load 1 fetched a list that the cache file does not have, so load 2 + // fails with no list at all. + let mut state = ListState { + list: list(100), + started: 2, + shown: 1, + ..ListState::default() + }; + let failed = ListError::Failed("connection refused".to_string()); + + state.finish(2, None, Some(failed.clone())); + + assert!(!state.loading()); + assert_eq!(state.list, list(100)); + assert_eq!(state.error, Some(failed)); + } + + #[test] + fn ignores_a_load_that_finishes_after_a_newer_one() { + let mut state = ListState { + started: 2, + ..ListState::default() + }; + + state.finish(2, list(200), None); + state.finish(1, None, Some(ListError::Rejected(401))); + + assert!(!state.loading()); + assert_eq!(state.list, list(200)); + assert_eq!(state.error, None); + } +} diff --git a/crates/switchyard-menubar/src/server.rs b/crates/switchyard-menubar/src/server.rs new file mode 100644 index 000000000..387fa7ecc --- /dev/null +++ b/crates/switchyard-menubar/src/server.rs @@ -0,0 +1,278 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Controls the installed server: check a config with its `--dry-run`, save +//! it, and restart the LaunchAgent that runs it. + +use std::io::Write; +use std::path::{Path, PathBuf}; +use std::process::Command; +use std::time::{Duration, Instant}; + +use chrono::Local; + +use crate::config::Config; +use crate::health::{ServerStatus, probe}; +use crate::server_config::{Algorithm, Choice, ServerConfig}; + +/// How long to wait for the restarted server to answer `/health`. +const RESTART_WAIT: Duration = Duration::from_secs(10); + +/// Switches a route to new models, checks the result with the server's +/// `--dry-run`, saves it, and restarts the server. +/// +/// Returns what happened on success. An error means the config file was not +/// changed. +pub fn apply( + settings: &Config, + route: &str, + algorithm: &Algorithm, + choices: &[Choice], +) -> Result { + let path = &settings.config_file; + // Read the file again so an edit made while the window was open is kept. + let text = std::fs::read_to_string(path) + .map_err(|error| format!("read {}: {error}", path.display()))?; + let edited = ServerConfig::parse(&text)?.edit(route, algorithm, choices)?; + if edited == text { + return Ok("Nothing to save: the route already uses these models.".to_string()); + } + + let binary = server_binary()?; + let saved = save_checked(&binary, path, &edited)?; + let restarted = match restart(&settings.launchd_label) { + Err(error) => format!("The restart failed: {error}"), + Ok(()) if wait_for_health(&settings.server_url) => format!( + "Restarted {}, and the server answers {}/health.", + settings.launchd_label, settings.server_url + ), + Ok(()) => format!( + "Restarted {}, but the server has not answered {}/health yet. Check its log.", + settings.launchd_label, settings.server_url + ), + }; + Ok(format!("{saved}\n{restarted}")) +} + +/// Returns the `switchyard-server` installed next to this app, which is the +/// binary that the server's LaunchAgent runs. +fn server_binary() -> Result { + std::env::current_exe() + .map(|exe| exe.with_file_name("switchyard-server")) + .map_err(|error| format!("find switchyard-server next to this app: {error}")) +} + +/// Checks `text` with `switchyard-server --dry-run`, then replaces `path` +/// with it, keeping the old file as a timestamped backup. +/// +/// The real file is `path`, or the file it points to when `path` is a +/// symlink. The new text goes to a temporary file next to the real file. The +/// temporary file is synced to disk and then renamed over the real file, so a +/// reader never sees a half-written config. When the check fails, the +/// temporary file is removed, `path` does not change, and the server's error +/// is returned. +fn save_checked(binary: &Path, path: &Path, text: &str) -> Result { + // Resolve a symlink and replace the file it points to, so the link stays. + let file = + std::fs::canonicalize(path).map_err(|error| format!("find {}: {error}", path.display()))?; + let dir = file.parent().unwrap_or(Path::new("/")); + let mut candidate = tempfile::Builder::new() + .prefix(".switchyard-picker-") + .suffix(".toml") + .tempfile_in(dir) + .map_err(|error| format!("create a file in {}: {error}", dir.display()))?; + candidate + .write_all(text.as_bytes()) + .and_then(|()| candidate.as_file().sync_all()) + .map_err(|error| format!("write {}: {error}", candidate.path().display()))?; + if let Ok(metadata) = std::fs::metadata(&file) { + candidate + .as_file() + .set_permissions(metadata.permissions()) + .map_err(|error| format!("copy the permissions of {}: {error}", path.display()))?; + } + + // Name the real file in errors, not the temporary one. + let summary = check(binary, candidate.path()).map_err(|error| { + error.replace( + &candidate.path().display().to_string(), + &path.display().to_string(), + ) + })?; + + let backup = PathBuf::from(format!( + "{}.switchyard-backup.{}", + file.display(), + Local::now().format("%Y%m%d%H%M%S") + )); + std::fs::copy(&file, &backup) + .map_err(|error| format!("back up {}: {error}", path.display()))?; + candidate + .persist(&file) + .map_err(|error| format!("replace {}: {}", path.display(), error.error))?; + Ok(format!( + "switchyard-server --dry-run: {summary}\nSaved {}, and kept the old file as {}.", + path.display(), + backup.display() + )) +} + +/// Runs `switchyard-server --config --dry-run` and returns its +/// summary, or its error output when the config is invalid. +fn check(binary: &Path, config: &Path) -> Result { + let output = Command::new(binary) + .arg("--config") + .arg(config) + .arg("--dry-run") + .output() + .map_err(|error| format!("run {}: {error}", binary.display()))?; + let stdout = String::from_utf8_lossy(&output.stdout).trim().to_string(); + let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + if output.status.success() { + Ok(stdout) + } else if stderr.is_empty() { + Err(stdout) + } else { + Err(stderr) + } +} + +/// Restarts the server's LaunchAgent. +pub fn restart(launchd_label: &str) -> Result<(), String> { + let uid = command("id", &["-u"])?; + let target = format!("gui/{}/{launchd_label}", uid.trim()); + command("launchctl", &["kickstart", "-k", &target]).map(|_| ()) +} + +fn wait_for_health(server_url: &str) -> bool { + let deadline = Instant::now() + RESTART_WAIT; + loop { + if probe(server_url) == ServerStatus::Running { + return true; + } + if Instant::now() >= deadline { + return false; + } + std::thread::sleep(Duration::from_millis(250)); + } +} + +/// Runs a command, returning its stdout or a message naming what failed. +pub fn command(program: &str, args: &[&str]) -> Result { + let output = Command::new(program) + .args(args) + .output() + .map_err(|error| format!("run {program}: {error}"))?; + if !output.status.success() { + return Err(format!( + "{program} {}: {}", + args.join(" "), + String::from_utf8_lossy(&output.stderr).trim() + )); + } + Ok(String::from_utf8_lossy(&output.stdout).into_owned()) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::os::unix::fs::PermissionsExt; + + /// Writes a stand-in for `switchyard-server` that runs `body`. + fn fake_server(dir: &Path, body: &str) -> PathBuf { + let path = dir.join("switchyard-server"); + std::fs::write(&path, format!("#!/bin/sh\n{body}\n")).expect("write script"); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)) + .expect("make script executable"); + path + } + + fn backups(dir: &Path) -> Vec { + std::fs::read_dir(dir) + .expect("read dir") + .map(|entry| entry.expect("entry").path()) + .filter(|path| path.display().to_string().contains(".switchyard-backup.")) + .collect() + } + + #[test] + fn a_config_that_fails_the_check_is_not_saved() { + let dir = tempfile::tempdir().expect("tempdir"); + let config = dir.path().join("composite.toml"); + std::fs::write(&config, "old").expect("write config"); + // The real server names the file it was given; the error must name + // the user's file instead of the temporary copy. + let binary = fake_server( + dir.path(), + r#"echo "invalid server config $2: route gateway cannot forward both" >&2; exit 1"#, + ); + + let error = save_checked(&binary, &config, "new").expect_err("check fails"); + + assert_eq!( + error, + format!( + "invalid server config {}: route gateway cannot forward both", + config.display() + ) + ); + assert_eq!(std::fs::read_to_string(&config).expect("read"), "old"); + assert!(backups(dir.path()).is_empty()); + assert_eq!( + std::fs::read_dir(dir.path()).expect("read dir").count(), + 2, + "the temporary file is removed" + ); + } + + #[test] + fn a_config_that_passes_the_check_replaces_the_file_and_keeps_a_backup() { + let dir = tempfile::tempdir().expect("tempdir"); + let config = dir.path().join("composite.toml"); + std::fs::write(&config, "old").expect("write config"); + std::fs::set_permissions(&config, std::fs::Permissions::from_mode(0o640)) + .expect("set permissions"); + let binary = fake_server( + dir.path(), + r#"grep -q new "$2" && echo "server OK: gateway""#, + ); + + let message = save_checked(&binary, &config, "new").expect("saved"); + + assert!(message.starts_with("switchyard-server --dry-run: server OK: gateway\n")); + assert_eq!(std::fs::read_to_string(&config).expect("read"), "new"); + let mode = std::fs::metadata(&config) + .expect("metadata") + .permissions() + .mode(); + assert_eq!(mode & 0o777, 0o640, "the file keeps its permissions"); + let backups = backups(dir.path()); + assert_eq!(backups.len(), 1); + assert_eq!(std::fs::read_to_string(&backups[0]).expect("read"), "old"); + } + + #[test] + fn saving_through_a_symlink_replaces_the_linked_file() { + let dir = tempfile::tempdir().expect("tempdir"); + let real = dir.path().join("dotfiles.toml"); + let link = dir.path().join("composite.toml"); + std::fs::write(&real, "old").expect("write config"); + std::os::unix::fs::symlink(&real, &link).expect("symlink"); + let binary = fake_server(dir.path(), r#"echo "server OK: gateway""#); + + save_checked(&binary, &link, "new").expect("saved"); + + let link_type = std::fs::symlink_metadata(&link) + .expect("metadata") + .file_type(); + assert!(link_type.is_symlink(), "the link is kept"); + assert_eq!(std::fs::read_to_string(&real).expect("read"), "new"); + } + + #[test] + fn reports_which_command_failed() { + let error = command("switchyard-does-not-exist", &[]).expect_err("missing program"); + + assert!(error.contains("switchyard-does-not-exist")); + } +} diff --git a/crates/switchyard-menubar/src/server_config.rs b/crates/switchyard-menubar/src/server_config.rs new file mode 100644 index 000000000..67e6f2a94 --- /dev/null +++ b/crates/switchyard-menubar/src/server_config.rs @@ -0,0 +1,880 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Reads the server config and rewrites one route's algorithm and models. +//! +//! Edits go through `toml_edit`, so comments, formatting, and every table the +//! edit does not touch stay as the user wrote them. The server's own +//! `--dry-run` decides whether the result is valid; this module only builds it. + +use std::collections::HashMap; + +use toml_edit::{DocumentMut, Item, Table, TableLike, Value}; + +/// The part a model plays in a route. A choice made for one algorithm carries +/// over to the role with the same tier when the user switches algorithms. +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +pub enum Tier { + Judge, + Capable, + Efficient, +} + +/// A route `type` the picker can write. +#[derive(Debug)] +pub struct Algorithm { + pub kind: &'static str, + pub summary: &'static str, + /// Each role's label, tier, and the key path under the route that names + /// the role's target. The list is empty for `random`, whose roles are the + /// entries of its `targets` list. + roles: &'static [(&'static str, Tier, &'static [&'static str])], + /// Required settings written when a route switches to this type. They + /// match the values the routing docs use in their examples. + settings: &'static [(&'static [&'static str], Setting)], + /// Whether the type accepts a nested `subagents` policy. + subagents: bool, +} + +#[derive(Debug)] +enum Setting { + Text(&'static str), + Number(f64), +} + +impl Setting { + fn value(&self) -> Value { + match self { + Self::Text(text) => Value::from(*text), + Self::Number(number) => Value::from(*number), + } + } +} + +const RANDOM: &str = "random"; + +/// The route types from the routing docs, in the order the picker lists them. +/// `noop` and the experimental `prefill_router` are left out: one calls no +/// model, and the other needs a checkpoint file rather than models. +pub const ALGORITHMS: &[Algorithm] = &[ + Algorithm { + kind: "passthrough", + summary: "Sends every request to one model.", + roles: &[("Model", Tier::Capable, &["target"])], + settings: &[], + subagents: true, + }, + Algorithm { + kind: RANDOM, + summary: "Splits requests at random between the models, for A/B tests and baselines.", + roles: &[], + settings: &[], + subagents: false, + }, + Algorithm { + kind: "llm_classifier", + summary: "A judge model reads each request and picks the capable or the efficient model.", + roles: &[ + ("Judge", Tier::Judge, &["classifier_target"]), + ("Capable", Tier::Capable, &["strong_target"]), + ("Efficient", Tier::Efficient, &["weak_target"]), + ], + settings: &[ + (&["mode"], Setting::Text("capability")), + (&["base_threshold"], Setting::Number(0.5)), + ], + subagents: false, + }, + Algorithm { + kind: "composite", + summary: "A judge picks the default model on each user turn, and tool results move \ + requests between the capable and the efficient model.", + roles: &[ + ("Judge", Tier::Judge, &["classifier", "target"]), + ("Capable", Tier::Capable, &["stage", "capable_target"]), + ("Efficient", Tier::Efficient, &["stage", "efficient_target"]), + ], + settings: &[ + (&["classifier", "base_threshold"], Setting::Number(0.5)), + ( + &["classifier", "classify_trigger"], + Setting::Text("user_turn"), + ), + (&["stage", "confidence_threshold"], Setting::Number(0.5)), + ], + subagents: true, + }, + Algorithm { + kind: "stage_router", + summary: "Tool results and agent progress move each request between the capable and \ + the efficient model.", + roles: &[ + ("Capable", Tier::Capable, &["capable_target"]), + ("Efficient", Tier::Efficient, &["efficient_target"]), + ], + settings: &[ + (&["picker"], Setting::Text("efficient_first")), + (&["confidence_threshold"], Setting::Number(0.5)), + ], + subagents: true, + }, + Algorithm { + kind: "advisor", + summary: "The executor answers every turn, and a stronger advisor reviews its final \ + answers.", + roles: &[ + ("Executor", Tier::Efficient, &["executor_target"]), + ("Advisor", Tier::Capable, &["advisor_target"]), + ], + settings: &[], + subagents: false, + }, + Algorithm { + kind: "plan_execute", + summary: "The capable model inspects and plans. The efficient model takes over after \ + the first file edit.", + roles: &[ + ("Capable", Tier::Capable, &["capable_target"]), + ("Efficient", Tier::Efficient, &["efficient_target"]), + ], + settings: &[], + subagents: false, + }, + Algorithm { + kind: "auto", + summary: "Switchyard's recommended preset: a stage router that starts on the efficient \ + model.", + roles: &[ + ("Capable", Tier::Capable, &["capable_target"]), + ("Efficient", Tier::Efficient, &["efficient_target"]), + ], + settings: &[], + subagents: false, + }, +]; + +/// Route keys every type accepts. Switching types keeps these and removes +/// the rest, so settings of the old type cannot fail the new type's checks. +const COMMON_ROUTE_KEYS: [&str; 6] = [ + "id", + "type", + "context_window", + "tool_calling", + "reasoning", + "vision", +]; + +/// One entry under `[routes]`. +#[derive(Clone, Debug, PartialEq)] +pub struct Route { + /// The route's table name in this file. Callers never send it. + pub key: String, + /// The public model ID that callers send. + pub id: String, + pub kind: String, +} + +/// One entry under `[llm_clients]`. +#[derive(Clone, Debug, PartialEq)] +pub struct Client { + pub name: String, + pub format: String, + pub base_url: String, + pub api_key_env: Option, + pub forward_auth: bool, +} + +/// A model on an LLM client, as a target names it. +#[derive(Clone, Debug, Default, Eq, PartialEq)] +pub struct Choice { + pub client: String, + pub model: String, +} + +/// A model the algorithm needs, such as the judge. +#[derive(Clone, Debug, PartialEq)] +pub struct Role { + pub label: String, + pub tier: Option, + slot: Slot, +} + +/// Where a route names a role's target. +#[derive(Clone, Debug, PartialEq)] +enum Slot { + /// A key path, such as `stage.capable_target`. + Key(&'static [&'static str]), + /// An entry of a random route's `targets` list. + Listed(usize), +} + +/// A parsed server config that keeps the original text's layout. +#[derive(Clone, Debug)] +pub struct ServerConfig { + doc: DocumentMut, +} + +impl ServerConfig { + pub fn parse(text: &str) -> Result { + text.parse::() + .map(|doc| Self { doc }) + .map_err(|error| format!("parse the server config: {error}")) + } + + pub fn routes(&self) -> Vec { + entries(self.table("routes")) + .map(|(key, route)| Route { + key: key.to_string(), + id: text(route, "id").unwrap_or(key).to_string(), + kind: text(route, "type").unwrap_or_default().to_string(), + }) + .collect() + } + + pub fn clients(&self) -> Vec { + entries(self.table("llm_clients")) + .map(|(name, client)| Client { + name: name.to_string(), + format: text(client, "format").unwrap_or_default().to_string(), + base_url: text(client, "base_url").unwrap_or_default().to_string(), + api_key_env: text(client, "api_key_env").map(str::to_string), + forward_auth: client + .get("forward_auth") + .and_then(Item::as_bool) + .unwrap_or(false), + }) + .collect() + } + + /// The algorithm a route uses now, or `None` when the picker cannot show + /// it, such as a custom-mode classifier whose groups are free-form. + pub fn algorithm(&self, route: &str) -> Option<&'static Algorithm> { + let route = self.route(route)?; + if text(route, "mode") == Some("custom") { + return None; + } + let kind = text(route, "type")?; + ALGORITHMS.iter().find(|algorithm| algorithm.kind == kind) + } + + /// The roles `algorithm` needs on `route`. A random route keeps its + /// number of targets, and has at least two. + pub fn roles(&self, route: &str, algorithm: &Algorithm) -> Vec { + if algorithm.kind != RANDOM { + return algorithm + .roles + .iter() + .map(|(label, tier, path)| Role { + label: (*label).to_string(), + tier: Some(*tier), + slot: Slot::Key(path), + }) + .collect(); + } + let listed = self + .route(route) + .filter(|route| text(*route, "type") == Some(RANDOM)) + .and_then(|route| { + route + .get("targets")? + .as_array() + .map(|targets| targets.len()) + }) + .unwrap_or(0); + (0..listed.max(2)) + .map(|index| Role { + label: format!("Model {}", index + 1), + tier: [Tier::Capable, Tier::Efficient].get(index).copied(), + slot: Slot::Listed(index), + }) + .collect() + } + + /// The model each role of the route's current algorithm uses. A role + /// whose target is missing gets an empty choice. + pub fn choices(&self, route: &str) -> Vec { + let Some(algorithm) = self.algorithm(route) else { + return Vec::new(); + }; + self.roles(route, algorithm) + .iter() + .map(|role| { + self.target_at(route, &role.slot) + .and_then(|name| self.target(name)) + .unwrap_or_default() + }) + .collect() + } + + /// Returns the config text with `route` switched to `algorithm`, using + /// one choice per role. + /// + /// For each role, in order of preference: keep the route's target when it + /// already names the model; use another target that names exactly this + /// model on this client; change the route's target in place when no other + /// route or role uses it, including a role earlier in this edit; otherwise + /// add a new target that copies it. A changed or copied target keeps + /// settings such as `extra_body` and `omit_body_fields`, including + /// settings meant for the old model. Targets no route uses any more are + /// left in the file. + pub fn edit( + &self, + route: &str, + algorithm: &Algorithm, + choices: &[Choice], + ) -> Result { + if self.route(route).is_none() { + return Err(format!("The config has no route {route}.")); + } + let roles = self.roles(route, algorithm); + if roles.len() != choices.len() { + return Err(format!( + "{} needs {} models, not {}.", + algorithm.kind, + roles.len(), + choices.len() + )); + } + let clients = self.clients(); + for (role, choice) in roles.iter().zip(choices) { + if choice.model.trim().is_empty() { + return Err(format!("Choose a model for {}.", role.label)); + } + if !clients.iter().any(|client| client.name == choice.client) { + return Err(format!("The config has no llm client {}.", choice.client)); + } + } + + let current = self.algorithm(route); + let same_type = current.is_some_and(|current| current.kind == algorithm.kind); + // When the type changes, each new role starts from the old role with + // the same tier, so a composite's capable target stays the capable one. + let by_tier: Vec<(Option, &str)> = current + .map(|current| { + self.roles(route, current) + .iter() + .filter_map(|role| Some((role.tier, self.target_at(route, &role.slot)?))) + .collect() + }) + .unwrap_or_default(); + let mut references = self.target_references(); + + let mut doc = self.doc.clone(); + let mut names = Vec::with_capacity(roles.len()); + for (role, choice) in roles.iter().zip(choices) { + let existing = if same_type { + self.target_at(route, &role.slot) + } else { + role.tier.and_then(|tier| { + by_tier + .iter() + .find(|(old, _)| *old == Some(tier)) + .map(|(_, name)| *name) + }) + }; + let choice = Choice { + client: choice.client.clone(), + model: choice.model.trim().to_string(), + }; + names.push(choose_target( + &mut doc, + &mut references, + route, + role, + &choice, + existing, + )?); + } + + let table = doc + .get_mut("routes") + .and_then(Item::as_table_like_mut) + .and_then(|routes| routes.get_mut(route)) + .and_then(Item::as_table_like_mut) + .ok_or_else(|| format!("The config has no route {route}."))?; + if !same_type { + reset_route(table, algorithm); + } + for (role, name) in roles.iter().zip(&names) { + set_slot(table, &role.slot, name); + } + if !same_type { + for (path, setting) in algorithm.settings { + set_at(table, path, setting.value()); + } + } + Ok(doc.to_string()) + } + + fn table(&self, name: &str) -> Option<&dyn TableLike> { + self.doc.get(name)?.as_table_like() + } + + fn route(&self, route: &str) -> Option<&dyn TableLike> { + self.table("routes")?.get(route)?.as_table_like() + } + + fn target(&self, name: &str) -> Option { + let target = self.table("targets")?.get(name)?.as_table_like()?; + Some(Choice { + client: text(target, "llm_client")?.to_string(), + model: text(target, "id")?.to_string(), + }) + } + + fn target_at(&self, route: &str, slot: &Slot) -> Option<&str> { + let route = self.route(route)?; + match slot { + Slot::Key(path) => { + let (last, parents) = path.split_last()?; + let mut table = route; + for key in parents { + table = table.get(key)?.as_table_like()?; + } + text(table, last) + } + Slot::Listed(index) => route.get("targets")?.as_array()?.get(*index)?.as_str(), + } + } + + /// Counts how many times each target name appears in any route, nested + /// policies included. A name that appears once belongs to one role. + fn target_references(&self) -> HashMap<&str, usize> { + let mut counts: HashMap<&str, usize> = entries(self.table("targets")) + .map(|(name, _)| (name, 0)) + .collect(); + if let Some(routes) = self.doc.get("routes") { + visit_strings(routes, &mut |text| { + if let Some(count) = counts.get_mut(text) { + *count += 1; + } + }); + } + counts + } +} + +/// Picks or writes the target that serves `choice` for one role, and returns +/// its name. Reusing a target adds one to its reference count, so a later +/// role in the same edit copies that target instead of changing it in place. +fn choose_target( + doc: &mut DocumentMut, + references: &mut HashMap<&str, usize>, + route: &str, + role: &Role, + choice: &Choice, + existing: Option<&str>, +) -> Result { + let targets = doc + .get_mut("targets") + .and_then(Item::as_table_like_mut) + .ok_or("The config has no [targets] table.")?; + + if let Some(name) = existing + && targets + .get(name) + .is_some_and(|target| names_choice(target, choice)) + { + return Ok(name.to_string()); + } + if let Some((name, _)) = targets + .iter() + .find(|(_, target)| names_choice(target, choice)) + { + if let Some(count) = references.get_mut(name) { + *count += 1; + } + return Ok(name.to_string()); + } + if let Some(name) = existing + && references.get(name) == Some(&1) + && let Some(target) = targets.get_mut(name).and_then(Item::as_table_like_mut) + { + set_value(target, "llm_client", Value::from(choice.client.as_str())); + set_value(target, "id", Value::from(choice.model.as_str())); + return Ok(name.to_string()); + } + + let mut table = Table::new(); + if let Some(source) = existing + .and_then(|name| targets.get(name)) + .and_then(Item::as_table_like) + { + for (key, item) in source.iter() { + table.insert(key, item.clone()); + } + } + set_value( + &mut table, + "llm_client", + Value::from(choice.client.as_str()), + ); + set_value(&mut table, "id", Value::from(choice.model.as_str())); + let base = format!("{route}_{}", role.label.to_lowercase().replace(' ', "_")); + let mut name = base.clone(); + let mut suffix = 1; + while targets.contains_key(&name) { + suffix += 1; + name = format!("{base}_{suffix}"); + } + targets.insert(&name, Item::Table(table)); + Ok(name) +} + +/// Whether a target names exactly this model on this client. +fn names_choice(target: &Item, choice: &Choice) -> bool { + target.as_table_like().is_some_and(|target| { + text(target, "llm_client") == Some(choice.client.as_str()) + && text(target, "id") == Some(choice.model.as_str()) + }) +} + +/// Removes the old type's settings and sets the new `type`. +fn reset_route(route: &mut dyn TableLike, algorithm: &Algorithm) { + let kept = + |key: &str| COMMON_ROUTE_KEYS.contains(&key) || (algorithm.subagents && key == "subagents"); + let stale: Vec = route + .iter() + .map(|(key, _)| key) + .filter(|key| !kept(key)) + .map(str::to_string) + .collect(); + for key in stale { + route.remove(&key); + } + set_value(route, "type", Value::from(algorithm.kind)); +} + +fn set_slot(route: &mut dyn TableLike, slot: &Slot, name: &str) { + match slot { + Slot::Key(path) => set_at(route, path, Value::from(name)), + Slot::Listed(index) => { + if !route.get("targets").is_some_and(Item::is_array) { + route.insert("targets", toml_edit::value(toml_edit::Array::new())); + } + if let Some(targets) = route.get_mut("targets").and_then(Item::as_array_mut) { + if *index < targets.len() { + targets.replace(*index, name); + } else { + targets.push(name); + } + } + } + } +} + +/// Sets a value at a key path, creating missing tables on the way. +fn set_at(table: &mut dyn TableLike, path: &[&str], value: Value) { + match path { + [] => {} + [key] => set_value(table, key, value), + [key, rest @ ..] => { + if !table.get(key).is_some_and(Item::is_table_like) { + table.insert(key, Item::Table(Table::new())); + } + if let Some(child) = table.get_mut(key).and_then(Item::as_table_like_mut) { + set_at(child, rest, value); + } + } + } +} + +/// Replaces a value, keeping the spacing and trailing comment around it. +fn set_value(table: &mut dyn TableLike, key: &str, mut value: Value) { + if let Some(old) = table.get_mut(key).and_then(Item::as_value_mut) { + *value.decor_mut() = old.decor().clone(); + *old = value; + } else { + table.insert(key, Item::Value(value)); + } +} + +fn entries(table: Option<&dyn TableLike>) -> impl Iterator { + table + .into_iter() + .flat_map(|table| table.iter()) + .filter_map(|(key, item)| Some((key, item.as_table_like()?))) +} + +fn text<'a>(table: &'a dyn TableLike, key: &str) -> Option<&'a str> { + table.get(key)?.as_str() +} + +fn visit_strings(item: &Item, visit: &mut dyn FnMut(&str)) { + match item { + Item::Value(value) => visit_value(value, visit), + Item::Table(table) => table + .iter() + .for_each(|(_, item)| visit_strings(item, visit)), + Item::ArrayOfTables(tables) => tables + .iter() + .flat_map(Table::iter) + .for_each(|(_, item)| visit_strings(item, visit)), + Item::None => {} + } +} + +fn visit_value(value: &Value, visit: &mut dyn FnMut(&str)) { + match value { + Value::String(text) => visit(text.value()), + Value::Array(array) => array.iter().for_each(|value| visit_value(value, visit)), + Value::InlineTable(table) => table + .iter() + .for_each(|(_, value)| visit_value(value, visit)), + _ => {} + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A composite route with a GPT judge on a Responses client and Claude on + /// a Chat Completions client, both on one gateway. + const GATEWAY: &str = r#"schema_version = 1 + +[llm_clients.gateway] +format = "openai_responses" +base_url = "https://gateway.example.com/v1" +forward_auth = true + +# Claude caches prompts on the chat endpoint. +[llm_clients.gateway_chat] +format = "openai_chat" +base_url = "https://gateway.example.com/v1" +forward_auth = true + +# The gateway rejects any thinking setting for these models. +[targets.gateway_capable] +id = "claude-opus-5-5" +llm_client = "gateway_chat" # chat, for caching +omit_body_fields = ["reasoning_effort"] + +[targets.gateway_efficient] +id = "claude-sonnet-5" +llm_client = "gateway_chat" +omit_body_fields = ["reasoning_effort"] + +[targets.gateway_judge] +id = "gpt-5.6-terra" +llm_client = "gateway" + +[routes.gateway] +id = "switchyard-gateway" +type = "composite" +context_window = 200000 + +[routes.gateway.classifier] +target = "gateway_judge" +base_threshold = 0.5 +classify_trigger = "user_turn" +message_hash_fallback = true + +[routes.gateway.stage] +capable_target = "gateway_capable" +efficient_target = "gateway_efficient" +confidence_threshold = 0.5 +"#; + + fn choice(client: &str, model: &str) -> Choice { + Choice { + client: client.to_string(), + model: model.to_string(), + } + } + + fn algorithm(kind: &str) -> &'static Algorithm { + ALGORITHMS + .iter() + .find(|algorithm| algorithm.kind == kind) + .expect("known algorithm") + } + + /// Edits the config and checks the result with the server's parser. + fn edit(text: &str, route: &str, kind: &str, choices: &[Choice]) -> String { + let edited = ServerConfig::parse(text) + .expect("parse") + .edit(route, algorithm(kind), choices) + .expect("edit"); + if let Err(error) = switchyard_runner::Runner::from_toml(&edited) { + panic!("the server rejects the edited config: {error}\n{edited}"); + } + edited + } + + #[test] + fn every_algorithm_writes_a_config_the_server_accepts() { + let gateway = ServerConfig::parse(GATEWAY).expect("parse"); + let models = [ + choice("gateway", "gpt-5.6-terra"), + choice("gateway", "gpt-5.6-sol"), + choice("gateway_chat", "claude-sonnet-5"), + ]; + for algorithm in ALGORITHMS { + let roles = gateway.roles("gateway", algorithm).len(); + + let edited = edit(GATEWAY, "gateway", algorithm.kind, &models[3 - roles..]); + + let config = ServerConfig::parse(&edited).expect("parse"); + assert_eq!( + config.algorithm("gateway").map(|a| a.kind), + Some(algorithm.kind) + ); + assert_eq!(config.choices("gateway"), models[3 - roles..]); + } + } + + #[test] + fn a_target_one_role_reuses_is_not_changed_for_another_role() { + // Capable moves to the efficient model, and Efficient to a new one. + let chosen = [ + choice("gateway", "gpt-5.6-terra"), + choice("gateway_chat", "claude-sonnet-5"), + choice("gateway", "gpt-5.6-luna"), + ]; + + let edited = edit(GATEWAY, "gateway", "composite", &chosen); + + assert_eq!( + ServerConfig::parse(&edited) + .expect("parse") + .choices("gateway"), + chosen + ); + } + + #[test] + fn switching_models_and_back_restores_the_file() { + let gpt = edit( + GATEWAY, + "gateway", + "composite", + &[ + choice("gateway", "gpt-5.6-terra"), + choice("gateway", "gpt-5.6-sol"), + choice("gateway", "gpt-5.6-luna"), + ], + ); + + let config = ServerConfig::parse(&gpt).expect("parse"); + assert_eq!( + config.choices("gateway"), + [ + choice("gateway", "gpt-5.6-terra"), + choice("gateway", "gpt-5.6-sol"), + choice("gateway", "gpt-5.6-luna"), + ] + ); + assert!( + gpt.contains( + "[targets.gateway_capable]\nid = \"gpt-5.6-sol\"\nllm_client = \"gateway\" # chat, for caching\nomit_body_fields = [\"reasoning_effort\"]" + ), + "the target keeps its other settings and comments:\n{gpt}" + ); + + let claude = edit( + &gpt, + "gateway", + "composite", + &[ + choice("gateway", "gpt-5.6-terra"), + choice("gateway_chat", "claude-opus-5-5"), + choice("gateway_chat", "claude-sonnet-5"), + ], + ); + assert_eq!(claude, GATEWAY); + } + + #[test] + fn changing_the_algorithm_replaces_only_the_route_settings() { + let edited = edit( + GATEWAY, + "gateway", + "stage_router", + &[ + choice("gateway_chat", "claude-opus-5-5"), + choice("gateway_chat", "claude-sonnet-5"), + ], + ); + + let config = ServerConfig::parse(&edited).expect("parse"); + assert_eq!( + config.algorithm("gateway").map(|a| a.kind), + Some("stage_router") + ); + let route = config.route("gateway").expect("route"); + assert_eq!(text(route, "id"), Some("switchyard-gateway")); + assert_eq!( + route.get("context_window").and_then(Item::as_integer), + Some(200000) + ); + assert_eq!(text(route, "capable_target"), Some("gateway_capable")); + assert_eq!(text(route, "efficient_target"), Some("gateway_efficient")); + assert_eq!(text(route, "picker"), Some("efficient_first")); + assert!(route.get("classifier").is_none() && route.get("stage").is_none()); + assert!( + edited.contains("[targets.gateway_judge]"), + "unused targets stay in the file" + ); + } + + #[test] + fn a_target_another_route_uses_is_copied_not_changed() { + let shared = format!( + "{GATEWAY}\n[routes.direct]\nid = \"direct\"\ntype = \"passthrough\"\ntarget = \"gateway_capable\"\n" + ); + + let edited = edit( + &shared, + "gateway", + "composite", + &[ + choice("gateway", "gpt-5.6-terra"), + choice("gateway", "gpt-5.6-sol"), + choice("gateway_chat", "claude-sonnet-5"), + ], + ); + + let config = ServerConfig::parse(&edited).expect("parse"); + assert_eq!( + config.choices("direct"), + [choice("gateway_chat", "claude-opus-5-5")] + ); + assert_eq!( + config.target_at("gateway", &Slot::Key(&["stage", "capable_target"])), + Some("gateway_capable_2") + ); + assert!( + edited.contains( + "[targets.gateway_capable_2]\nid = \"gpt-5.6-sol\"\nllm_client = \"gateway\" # chat, for caching\nomit_body_fields = [\"reasoning_effort\"]" + ), + "the copy keeps the shared target's settings:\n{edited}" + ); + } + + #[test] + fn rejects_a_missing_model_or_an_unknown_client() { + let config = ServerConfig::parse(GATEWAY).expect("parse"); + for (capable, expected) in [ + (choice("gateway", " "), "Choose a model for Capable."), + ( + choice("typo", "gpt-5.6-sol"), + "The config has no llm client typo.", + ), + ] { + let error = config + .edit( + "gateway", + algorithm("composite"), + &[ + choice("gateway", "gpt-5.6-terra"), + capable, + choice("gateway", "gpt-5.6-luna"), + ], + ) + .expect_err("invalid choice"); + + assert_eq!(error, expected); + } + } +} diff --git a/crates/switchyard-menubar/src/tray.rs b/crates/switchyard-menubar/src/tray.rs index 6f3181c8e..2568bf955 100644 --- a/crates/switchyard-menubar/src/tray.rs +++ b/crates/switchyard-menubar/src/tray.rs @@ -3,22 +3,26 @@ //! The macOS status item: event loop, menu, and the menu's actions. //! -//! Everything platform-specific lives here, so the rest of the crate builds -//! and is tested on any target. +//! Everything platform-specific lives here and in the picker window, so the +//! rest of the crate builds and is tested on any target. -use std::process::Command; +use std::path::Path; use std::time::{Duration, Instant}; use objc2_app_kit::{NSApplication, NSApplicationActivationPolicy, NSEventMask}; use objc2_foundation::{MainThreadMarker, NSDate, NSDefaultRunLoopMode}; -use tray_icon::menu::{IsMenuItem, Menu, MenuEvent, MenuItem, PredefinedMenuItem}; +use tray_icon::menu::{IsMenuItem, Menu, MenuEvent, MenuItem, PredefinedMenuItem, Submenu}; use tray_icon::{Icon, TrayIconBuilder}; use crate::app::refresh; use crate::config::Config; use crate::icon; +use crate::models::CACHE_FILE; +use crate::picker::Picker; +use crate::server::{command, restart}; use crate::summary::Row; +const CHANGE_ROUTING: &str = "change-routing"; const RESTART: &str = "restart-server"; const OPEN_CONFIG: &str = "open-config"; const OPEN_SETTINGS: &str = "open-settings"; @@ -27,12 +31,17 @@ const QUIT: &str = "quit"; /// How long the loop blocks waiting for a UI event before checking the clock. const EVENT_POLL: f64 = 0.1; -/// Runs the status item until the user quits. -pub fn run(config: Config) -> Result<(), String> { +/// Runs the status item until the user quits. `settings` is the settings +/// file that `config` came from; the model list cache sits next to it. +pub fn run(config: Config, settings: &Path) -> Result<(), String> { let mtm = MainThreadMarker::new().ok_or("the menu bar must run on the main thread")?; let ns_app = NSApplication::sharedApplication(mtm); // Accessory keeps the process out of the Dock and the app switcher. ns_app.setActivationPolicy(NSApplicationActivationPolicy::Accessory); + // An accessory app shows no menu bar, but text fields still find copy + // and paste shortcuts through the app's main menu. + let edit_menu = edit_menu()?; + edit_menu.init_for_nsapp(); let glyph = Icon::from_rgba(icon::glyph(), icon::SIZE, icon::SIZE) .map_err(|error| format!("build icon: {error}"))?; @@ -46,16 +55,25 @@ pub fn run(config: Config) -> Result<(), String> { ns_app.finishLaunching(); + let mut picker: Option = None; let interval = Duration::from_secs(config.refresh_seconds.max(5)); let mut next = Instant::now() + interval; loop { pump_events(&ns_app); + if let Some(picker) = picker.as_mut() { + picker.poll(); + } let mut redraw = false; while let Ok(event) = MenuEvent::receiver().try_recv() { match event.id.as_ref() { QUIT => return Ok(()), - RESTART => report(restart_server(&config)), + CHANGE_ROUTING => picker + .get_or_insert_with(|| { + Picker::new(mtm, &config, settings.with_file_name(CACHE_FILE)) + }) + .show(), + RESTART => report(restart(&config.launchd_label)), OPEN_CONFIG => report(open(&config.config_file)), OPEN_SETTINGS => report(open(&Config::default_path())), // Informational rows are disabled, so nothing else fires. @@ -71,34 +89,11 @@ pub fn run(config: Config) -> Result<(), String> { } } -/// Restarts the server's LaunchAgent. -fn restart_server(config: &Config) -> Result<(), String> { - let uid = command("id", &["-u"])?; - let target = format!("gui/{}/{}", uid.trim(), config.launchd_label); - command("launchctl", &["kickstart", "-k", &target]).map(|_| ()) -} - /// Opens a config file in the user's editor. fn open(path: &std::path::Path) -> Result<(), String> { command("open", &["-t", &path.display().to_string()]).map(|_| ()) } -/// Runs a command, returning its stdout or a message naming what failed. -fn command(program: &str, args: &[&str]) -> Result { - let output = Command::new(program) - .args(args) - .output() - .map_err(|error| format!("run {program}: {error}"))?; - if !output.status.success() { - return Err(format!( - "{program} {}: {}", - args.join(" "), - String::from_utf8_lossy(&output.stderr).trim() - )); - } - Ok(String::from_utf8_lossy(&output.stdout).into_owned()) -} - fn report(result: Result<(), String>) { if let Err(error) = result { eprintln!("switchyard-menubar: {error}"); @@ -122,6 +117,7 @@ fn menu(rows: &[Row]) -> Result { } append(&PredefinedMenuItem::separator())?; for (id, text) in [ + (CHANGE_ROUTING, "Change routing…"), (RESTART, "Restart server"), (OPEN_CONFIG, "Open server config…"), (OPEN_SETTINGS, "Open menu bar settings…"), @@ -132,6 +128,25 @@ fn menu(rows: &[Row]) -> Result { Ok(menu) } +/// The app's hidden main menu, which gives text fields their Edit shortcuts. +fn edit_menu() -> Result { + let edit = Submenu::with_items( + "Edit", + true, + &[ + &PredefinedMenuItem::undo(None), + &PredefinedMenuItem::redo(None), + &PredefinedMenuItem::separator(), + &PredefinedMenuItem::cut(None), + &PredefinedMenuItem::copy(None), + &PredefinedMenuItem::paste(None), + &PredefinedMenuItem::select_all(None), + ], + ) + .map_err(|error| format!("build the Edit menu: {error}"))?; + Menu::with_items(&[&edit]).map_err(|error| format!("build the main menu: {error}")) +} + /// Drains pending AppKit events, blocking briefly when there are none. fn pump_events(ns_app: &NSApplication) { let mut expiration = Some(NSDate::dateWithTimeIntervalSinceNow(EVENT_POLL)); @@ -148,15 +163,3 @@ fn pump_events(ns_app: &NSApplication) { expiration = Some(NSDate::distantPast()); } } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn reports_which_command_failed() { - let error = command("switchyard-does-not-exist", &[]).expect_err("missing program"); - - assert!(error.contains("switchyard-does-not-exist")); - } -} diff --git a/scripts/macos/uninstall.sh b/scripts/macos/uninstall.sh index 44dc9114f..5b677a0c5 100755 --- a/scripts/macos/uninstall.sh +++ b/scripts/macos/uninstall.sh @@ -64,6 +64,7 @@ done step "Done" say "Left in place, delete them if you want:" -say " $SY_HOME (binaries, config, routing log, logs)" +say " $SY_HOME (binaries, config, routing log, model lists, logs)" say " $CODEX_CONFIG.switchyard-current.* (configs preserved during restore)" say " $CODEX_PROFILE_CONFIG.switchyard-backup.* (profile backups)" +say " Keychain items named \"Switchyard model list\" (keys saved by Change routing…)" From 26e6198caeaf2a30880058fff1515a6af17ef33a Mon Sep 17 00:00:00 2001 From: Elyas Mehtabuddin Date: Thu, 1 Oct 2026 13:07:12 -0700 Subject: [PATCH 2/3] fix(menubar): show Apply's outcome first, scroll long role lists, and check keys before saving them Signed-off-by: Elyas Mehtabuddin --- crates/switchyard-menubar/README.md | 125 ++-- crates/switchyard-menubar/src/models.rs | 312 +++++++--- crates/switchyard-menubar/src/picker.rs | 368 +++++++++--- crates/switchyard-menubar/src/server.rs | 282 ++++++--- .../switchyard-menubar/src/server_config.rs | 561 ++++++++++++++---- crates/switchyard-menubar/src/tray.rs | 18 +- scripts/macos/uninstall.sh | 2 +- 7 files changed, 1260 insertions(+), 408 deletions(-) diff --git a/crates/switchyard-menubar/README.md b/crates/switchyard-menubar/README.md index 0a24110df..9bb033374 100644 --- a/crates/switchyard-menubar/README.md +++ b/crates/switchyard-menubar/README.md @@ -59,25 +59,41 @@ named by `config_file`. 3. For each role, pick an LLM client from the config, then a model. The model box lists the models from the client's `GET /models` endpoint. Type to filter the list, or type a model ID that is not listed. -4. Click **Apply**. +4. Click **Apply**, or press Return. Escape or Cmd-W closes the window. + +When a route has more roles than the screen can show, such as a `random` +route with many models, the roles scroll, and the buttons stay on the screen. Apply checks the new config with `switchyard-server --config --dry-run`, using the `switchyard-server` installed next to the menu bar app. If the check fails, the window shows the server's error and the file does not change. If it passes, the app saves the -file, keeps the old one as `.switchyard-backup.`, restarts -the server with `launchctl kickstart -k gui//`, and says -whether the server answers `/health`. If `config_file` is a symlink, the app -writes the file that the link points to, puts the backup next to that file, -and leaves the link in place. +file, keeps the old one as `.switchyard-backup.`, and +restarts the server with `launchctl kickstart -k gui//`. +A second backup in the same second gets `-2` added to its name, so a backup +never replaces another one. If another program changes the file while the +check runs, Apply saves nothing and asks you to click Apply again. If +`config_file` is a symlink, the app writes the file that the link points to, +puts the backup next to that file, and leaves the link in place. + +The result area lists what happened, most important first: the saved file +and its backup, the restart and whether the server answers `/health` within +10 seconds, notes about the change, and how many routes passed the check. If +the server does not answer, the result says how to put the backup back. The app changes as little of the file as it can: - Comments, formatting, and every table the change does not touch stay as they were. - A role keeps its target when that target already names the chosen model. - If another target names that model on that client, the route uses it. - Otherwise the app changes the route's own target in place, so settings such +- If another target names that model on that client, the route uses it when + the route would get the same `system_prompt`, `reasoning_effort`, + `extra_body`, and `omit_body_fields` from it. The route also uses it when + `reasoning_effort`, `extra_body`, or `omit_body_fields` differ, because the + server rejects two targets that name one model on one client with + different values for those. When the role then shares the target with + another route, the result says so. +- Otherwise the app changes the route's own target in place, so settings such as `extra_body` and `omit_body_fields` stay. - The app copies a target instead of changing it when another route uses it, or when an earlier role in the same Apply already took it. For example, if @@ -86,26 +102,48 @@ The app changes as little of the file as it can: target with the new model. - A changed or copied target also keeps its model-specific settings. For example, a target with `omit_body_fields = ["reasoning_effort"]` keeps that - setting when it switches from a Claude model to a GPT model. Check these - settings after you move a role to another model family. + setting when it switches from a Claude model to a GPT model. `--dry-run` + does not catch a setting that the new model rejects. So when a target moves + to another model family, such as from `gpt-…` to `claude-…`, or to a client + with another `format`, the result lists the `omit_body_fields`, + `reasoning_effort`, and `extra_body` that the target kept, or says that it + has none. The app does not change them; edit the file if the new model + needs other values. - Switching algorithms keeps the route's `id`, `context_window`, `tool_calling`, `reasoning`, and `vision`, and keeps `subagents` when the new algorithm accepts it (`passthrough`, `stage_router`, and `composite`). It removes the old type's other settings and writes the settings the new type requires with the values from the routing docs, such as - `confidence_threshold = 0.5`. Edit the file to tune them. + `confidence_threshold = 0.5`. The result lists the settings it removed, and + the backup still has them. Edit the file to tune them. - Targets that no route uses any more stay in the file. -The window cannot show a custom-mode `llm_classifier` route or a route of -another type. Applying to such a route replaces its settings with the -algorithm you pick. +The window cannot show the settings of a custom-mode `llm_classifier` route +or of a route whose `type` is not in the Algorithm list. Applying to such a +route replaces its settings with the algorithm and models you pick. + +The check runs with the menu bar app's environment, not your shell's. If a +client reads its key from `api_key_env`, the menu bar app's LaunchAgent needs +that variable too. Otherwise the check fails and the app saves nothing. The +window says so under each role that uses such a client, and again in the +result when the check fails. To add the variable, put it under +`EnvironmentVariables` in +`~/Library/LaunchAgents/com.nvidia.switchyard.menubar.plist`, then load the +agent again: + +```sh +launchctl bootout gui/$UID/com.nvidia.switchyard.menubar +launchctl bootstrap gui/$UID ~/Library/LaunchAgents/com.nvidia.switchyard.menubar.plist +``` -The check runs with the menu bar app's environment. If a client reads its -key from `api_key_env`, the menu bar's LaunchAgent needs that variable too. -Otherwise the check fails and the app saves nothing. +That puts the key in the plist file as plain text, and `make install-macos` +writes the plist again without it. A client with `forward_auth = true` needs +no key in the config, because the server sends each caller's own key. -If a chosen model has no price in `menubar.toml`, the window says so. -Savings stay hidden until you add the price and restart the menu bar app. +Apply does not add prices, because the app has no price source besides +`menubar.toml`. If a chosen model has no price there, the result names the +model. Savings stay hidden until you add the price and restart the menu bar +app. ### Model lists @@ -126,31 +164,40 @@ If the app cannot write `model-lists.json`, the window still uses the fetched list and shows "Could not save the list" with the error. Because the list is not in the file, the app fetches it again after a restart. -Click **Refresh models** to fetch the lists that the window's roles use again, -one request per URL. The result area then shows each list's model count or -error. If a fetch fails, the window keeps the list it has and shows the error -under it. +Click **Refresh models** to fetch every list that the window's roles use, +even a cached one. The app sends one request per URL, and fetches the URLs at +the same time, so a slow URL does not hold back the others. Each role's note +changes as its list arrives, and the result area then shows each list's +model count or error. If a fetch fails, the window keeps the list it has and +shows the error under it. When a role's model box is empty or holds a listed model ID, the note under -it shows the list's model count and age, such as "10 models, listed 2 minutes -ago. Type to filter." While you type, the note says how many models match, -such as "2 of 10 models match." When nothing matches, it says "No listed -model matches. Apply uses the ID as typed." +it shows the list's model count and age, such as "10 models, fetched 2 +minutes ago. Type to filter." While you type, the note says how many models +match, such as "2 of 10 models match." When nothing matches, it says "No +listed model matches. Apply uses the ID as typed." ### Keys for model lists -The app never writes a key to a file. For a client with `api_key_env`, it -reads that variable from its own environment. A client with `forward_auth` -stores no key, and a LaunchAgent does not get the variables from your shell -profile. So when a list needs a key and none is available, the role's note -says so, and the window shows a key field under the roles. Paste the key and -click **Save key**. The app saves the key in your login Keychain as -"Switchyard model list", with the client's `base_url` as the account, and -fetches the list with it. Clients with the same `base_url` share the key. - -If macOS cannot save the key, or cannot hand over a saved one, the window -shows the Keychain's error. You can always type a model ID that is not -listed. +The app never writes a key to a file. To list a client's models, it uses the +first key it finds: the key you just typed, then the variable named by the +client's `api_key_env` in the menu bar app's own environment, then the login +Keychain item for the client's `base_url`. A LaunchAgent does not load your +shell profile, so a key you export in `~/.zshrc` is not in the app's +environment. A `forward_auth` client has no key in the config, because the +server sends each caller's own key upstream. + +When the app finds no key, the role's note says so and a key field appears +under the roles. Paste the API key for the `base_url` that the field names, +and click **Save key**. The app first lists the models with the key. If the +models endpoint rejects it, the app does not save it. Otherwise the app saves +the key in your login Keychain as "Switchyard model list", with the client's +`base_url` as the account. Clients with the same `base_url` share the key. +The app uses a saved key only to list models at that `base_url`. It refuses +a key with a line break, because a line break would add a request header. + +If macOS cannot save the key, or cannot read a saved one, the window shows +the Keychain's error. You can always type a model ID that is not listed. The app lists models with the system `curl`. It writes the key to curl's stdin, never to its command line, and runs curl with `-q`, so curl ignores diff --git a/crates/switchyard-menubar/src/models.rs b/crates/switchyard-menubar/src/models.rs index e966c784a..a19e4af3c 100644 --- a/crates/switchyard-menubar/src/models.rs +++ b/crates/switchyard-menubar/src/models.rs @@ -67,10 +67,14 @@ pub struct Loaded { pub enum ListError { /// The client needs a key, and none is set or saved. NoKey, + /// The client reads its key from this `api_key_env` variable, which this + /// process does not have, and no key is saved. + NoEnv(String), /// The Keychain did not return the saved key. Keychain(String), - /// The server answered 401 or 403. + /// The models endpoint answered 401 or 403. Rejected(u16), + /// The request failed, or the answer was not a model list. Failed(String), /// The list was fetched, but the cache file could not be written. NotCached(String), @@ -80,36 +84,74 @@ impl ListError { /// Says what went wrong, in one sentence. pub fn reason(&self) -> String { match self { - Self::NoKey => "This client needs a key to list models, and none is saved.".to_string(), + Self::NoKey => { + "This LLM client needs an API key to list models, and the app does not have one." + .to_string() + } + Self::NoEnv(variable) => missing_env_note(variable), Self::Keychain(error) => { - format!("Could not read the saved key from the Keychain: {error}.") + format!("Could not read the saved key from your login Keychain: {error}.") + } + Self::Rejected(status) => { + format!("The models endpoint rejected the key (HTTP {status}).") } - Self::Rejected(status) => format!("The server rejected the key (HTTP {status})."), Self::Failed(error) => format!("Could not list models: {error}."), Self::NotCached(error) => format!("Could not save the list: {error}."), } } /// Says what the user can do while the window has no list to show. The - /// window always accepts a typed model ID, so every answer offers that. + /// window accepts a typed model ID, so most answers offer that. A missing + /// variable also stops Apply, so that answer offers only the key field. pub fn advice(&self) -> &'static str { - if self.needs_key() { - "Paste the key below and click Save key, or type a model ID." - } else { - "Click Refresh models to try again, or type a model ID." + match self { + Self::NoEnv(_) => "Or paste the key below to list models.", + _ if self.needs_key() => { + "Paste a valid API key below and click Save key, or type a model ID." + } + _ => "Click Refresh models to try again, or type a model ID.", } } /// Returns whether saving a key could fix this error. pub fn needs_key(&self) -> bool { - matches!(self, Self::NoKey | Self::Keychain(_) | Self::Rejected(_)) + matches!( + self, + Self::NoKey | Self::NoEnv(_) | Self::Keychain(_) | Self::Rejected(_) + ) } } +/// Says that this process lacks a client's `api_key_env` variable. Apply's +/// `--dry-run` check runs in this process's environment, so the check cannot +/// read the key either. +pub fn missing_env_note(variable: &str) -> String { + format!( + "The app's environment has no {variable}, which Apply needs: add it to the menu bar \ + app's LaunchAgent." + ) +} + +/// Returns the client's `api_key_env` variable when this process does not +/// have it set. +pub fn missing_env(client: &Client) -> Option<&str> { + client + .api_key_env + .as_deref() + .filter(|_| env_key(client).is_none()) +} + +/// The Keychain's answer for each `base_url`. The threads of one [`load`] +/// share it. +type SavedKeys = Mutex, String>>>; + /// Returns the model list at each URL that `clients` use, in the order the /// URLs first appear. Clients whose models share a URL share one list and /// one request. /// +/// Each URL loads on its own thread, so a slow URL does not hold back the +/// others. `loaded` gets each URL's result as soon as it is ready. +/// /// Without `refresh`, a list in the cache file comes back without a /// request, and only a URL with no cached list is fetched. With `refresh`, /// every URL is fetched. A fetched list replaces the one in the cache file @@ -125,54 +167,73 @@ pub fn load( clients: &[Client], refresh: bool, typed_key: Option<&str>, + loaded: &(dyn Fn(&Loaded) + Sync), ) -> Vec { - let mut cached = read_cache(cache); - let mut saved_keys = HashMap::new(); - let mut fetched = BTreeMap::new(); - let mut loaded = Vec::new(); - for (url, client) in by_url(clients) { - let old = cached.remove(&url); - if old.is_some() && !refresh { - loaded.push(Loaded { + let cached = read_cache(cache); + let saved_keys = SavedKeys::default(); + std::thread::scope(|scope| { + let workers: Vec<_> = by_url(clients) + .into_iter() + .map(|(url, client)| { + let old = cached.get(&url).cloned(); + let saved_keys = &saved_keys; + scope.spawn(move || { + let entry = load_url(cache, url, client, old, refresh, typed_key, saved_keys); + loaded(&entry); + entry + }) + }) + .collect(); + workers + .into_iter() + .map(|worker| { + worker + .join() + .unwrap_or_else(|panic| std::panic::resume_unwind(panic)) + }) + .collect() + }) +} + +/// Loads the list at one URL: from the cache file, or fetched with the +/// client's key and then added to the cache file. +fn load_url( + cache: &Path, + url: String, + client: &Client, + old: Option, + refresh: bool, + typed_key: Option<&str>, + saved_keys: &SavedKeys, +) -> Loaded { + if old.is_some() && !refresh { + return Loaded { + url, + list: old, + error: None, + }; + } + match key(client, typed_key, saved_keys).and_then(|key| list(client, key.as_deref())) { + Ok(models) => { + let new = ModelList { + models, + fetched_at: now(), + }; + let error = write_cache(cache, &url, &new) + .err() + .map(ListError::NotCached); + Loaded { url, - list: old, - error: None, - }); - continue; - } - let result = - key(client, typed_key, &mut saved_keys).and_then(|key| list(client, key.as_deref())); - loaded.push(match result { - Ok(models) => { - let new = ModelList { - models, - fetched_at: now(), - }; - fetched.insert(url.clone(), new.clone()); - Loaded { - url, - list: Some(new), - error: None, - } + list: Some(new), + error, } - Err(error) => Loaded { - url, - list: old, - error: Some(error), - }, - }); - } - if !fetched.is_empty() - && let Err(error) = write_cache(cache, &fetched) - { - for entry in loaded - .iter_mut() - .filter(|entry| fetched.contains_key(&entry.url)) - { - entry.error = Some(ListError::NotCached(error.clone())); } + Err(error) => Loaded { + url, + list: old, + error: Some(error), + }, } - loaded } /// Returns the URL that serves the client's model list. It is also the @@ -182,12 +243,14 @@ pub fn list_url(client: &Client) -> String { } /// Lists the client's model IDs, sorted. When the client needs a key and -/// `key` is `None`, returns [`ListError::NoKey`] without sending a request. +/// `key` is `None`, returns [`ListError::NoEnv`] or [`ListError::NoKey`] +/// without sending a request. fn list(client: &Client, key: Option<&str>) -> Result, ListError> { - if sends_key(client) && key.is_none() { - return Err(ListError::NoKey); + match (key, &client.api_key_env) { + (None, Some(variable)) => Err(ListError::NoEnv(variable.clone())), + (None, None) if client.forward_auth => Err(ListError::NoKey), + _ => fetch(&list_url(client), &client.format, key), } - fetch(&list_url(client), &client.format, key) } /// Returns the models that contain every word of `query`, ignoring case. @@ -205,9 +268,9 @@ pub fn matching<'a>(models: &'a [String], query: &str) -> Vec<&'a str> { } /// Returns the models endpoint for a client. It builds the URL the way the -/// server builds its request URLs: OpenAI clients drop a trailing -/// `/chat/completions` or `/responses`, and Anthropic clients use -/// `/v1/models`. +/// server builds its request URLs. OpenAI clients drop a trailing +/// `/chat/completions` or `/responses`. Anthropic clients drop a trailing +/// `/v1/messages` to `/v1`, and add `/v1` to any other path. fn models_url(format: &str, base_url: &str) -> String { let (base, query) = match base_url.split_once('?') { Some((base, query)) => (base, format!("?{query}")), @@ -215,7 +278,10 @@ fn models_url(format: &str, base_url: &str) -> String { }; let base = base.trim_end_matches('/'); let path = if format == "anthropic_messages" { - let root = base.strip_suffix("/messages").unwrap_or(base); + let root = base + .strip_suffix("/messages") + .filter(|root| root.ends_with("/v1")) + .unwrap_or(base); if root.ends_with("/v1") { format!("{root}/models") } else { @@ -257,12 +323,13 @@ fn sends_key(client: &Client) -> bool { /// /// The typed key comes first, then the client's `api_key_env` variable when /// this process has it, then the Keychain item for the client's `base_url`. -/// `saved` remembers each Keychain answer, so macOS asks at most once per -/// `base_url` when it needs the user's permission to hand over a key. +/// `saved` stores each Keychain answer, and a thread holds its lock while +/// it reads the Keychain. So macOS asks at most once per `base_url` when it +/// needs the user's permission to hand over a key. fn key( client: &Client, typed_key: Option<&str>, - saved: &mut HashMap, String>>, + saved: &SavedKeys, ) -> Result, ListError> { if !sends_key(client) { return Ok(None); @@ -271,6 +338,8 @@ fn key( return Ok(Some(key)); } saved + .lock() + .unwrap_or_else(PoisonError::into_inner) .entry(client.base_url.clone()) .or_insert_with(|| saved_key(&client.base_url)) .clone() @@ -296,7 +365,7 @@ fn saved_key(base_url: &str) -> Result, String> { .map(|key| Some(key).filter(|key| !key.trim().is_empty())) .map_err(|_| "the saved key is not text".to_string()), Err(error) if error.code() == NOT_FOUND => Ok(None), - Err(error) => Err(error.to_string()), + Err(error) => Err(keychain_error(&error)), } } @@ -313,7 +382,20 @@ pub fn save_key(base_url: &str, key: &str) -> Result<(), String> { base_url, key.trim().as_bytes(), ) - .map_err(|error| error.to_string()) + .map_err(|error| keychain_error(&error)) +} + +/// Returns the Keychain's message without its final period, so the window +/// can end its own sentence after it. +#[cfg(target_os = "macos")] +fn keychain_error(error: &security_framework::base::Error) -> String { + error.to_string().trim_end_matches('.').to_string() +} + +/// Returns whether a key has a line break, which would end the header that +/// carries the key and start another one. +pub fn has_line_break(key: &str) -> bool { + key.contains(['\r', '\n']) } /// Reads the cache file. A missing or unreadable file reads as empty, so the @@ -325,21 +407,21 @@ fn read_cache(path: &Path) -> BTreeMap { .unwrap_or_default() } -/// Adds `lists` to the cache file. A list replaces the one at the same URL -/// unless that one is newer: two loads can overlap, and the load that -/// fetched a list first can write it last. The text goes to a temporary file -/// that is renamed over the cache file, so a reader never sees half a file. -fn write_cache(path: &Path, lists: &BTreeMap) -> Result<(), String> { +/// Adds the list at `url` to the cache file. The list replaces the one at +/// the same URL unless that one is newer: two loads can overlap, and the +/// load that fetched a list first can write it last. The text goes to a +/// temporary file that is renamed over the cache file, so a reader never +/// sees half a file. +fn write_cache(path: &Path, url: &str, list: &ModelList) -> Result<(), String> { let _writing = CACHE_WRITE.lock().unwrap_or_else(PoisonError::into_inner); let mut all = read_cache(path); - for (url, list) in lists { - if all - .get(url) - .is_none_or(|cached| cached.fetched_at <= list.fetched_at) - { - all.insert(url.clone(), list.clone()); - } + if all + .get(url) + .is_some_and(|cached| cached.fetched_at > list.fetched_at) + { + return Ok(()); } + all.insert(url.to_string(), list.clone()); let text = serde_json::to_string_pretty(&all) .map_err(|error| format!("write {}: {error}", path.display()))?; let dir = path @@ -365,6 +447,11 @@ pub fn now() -> u64 { fn fetch(url: &str, format: &str, key: Option<&str>) -> Result, ListError> { let failed = |error: String| ListError::Failed(error); + if key.is_some_and(has_line_break) { + return Err(failed( + "the key has a line break, so the app did not send it".to_string(), + )); + } let mut child = Command::new("curl") // curl honors `-q` only as its first argument. `-q` makes curl ignore // ~/.curlrc, where a `verbose` line would print the key into the @@ -378,7 +465,9 @@ fn fetch(url: &str, format: &str, key: Option<&str>) -> Result, List ]) // Read request headers from stdin, one per line. .args(["--header", "@-"]) - .args(["--write-out", "\n%{http_code}", url]) + // `--url` keeps a URL that starts with `-` from being read as an + // option. + .args(["--write-out", "\n%{http_code}", "--url", url]) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) @@ -529,9 +618,9 @@ mod tests { client("gateway_chat", "openai_chat", &stub.url), ]; - let first = load(&cache, &clients, false, Some("test-key")); + let first = load(&cache, &clients, false, Some("test-key"), &|_| {}); // No key this time: a cached list needs neither a request nor a key. - let second = load(&cache, &clients, false, None); + let second = load(&cache, &clients, false, None, &|_| {}); let requests = stub.requests(); assert_eq!(requests.len(), 1, "{requests:?}"); @@ -555,11 +644,11 @@ mod tests { let dir = tempfile::tempdir().expect("tempdir"); let cache = dir.path().join(CACHE_FILE); let clients = [client("gateway", "openai_chat", &stub.url)]; - load(&cache, &clients, false, Some("test-key")); + load(&cache, &clients, false, Some("test-key"), &|_| {}); stub.answer(listing(&["new-model"])); - let refreshed = load(&cache, &clients, true, Some("test-key")); - let later = load(&cache, &clients, false, None); + let refreshed = load(&cache, &clients, true, Some("test-key"), &|_| {}); + let later = load(&cache, &clients, false, None, &|_| {}); assert_eq!(stub.requests().len(), 2); assert_eq!(models(&refreshed), ["new-model"]); @@ -573,13 +662,13 @@ mod tests { let dir = tempfile::tempdir().expect("tempdir"); let cache = dir.path().join(CACHE_FILE); let clients = [client("gateway", "openai_chat", &stub.url)]; - let first = load(&cache, &clients, false, Some("test-key")); + let first = load(&cache, &clients, false, Some("test-key"), &|_| {}); stub.answer( "HTTP/1.1 500 Internal Server Error\r\nConnection: close\r\n\r\n{}".to_string(), ); - let refreshed = load(&cache, &clients, true, Some("test-key")); - let later = load(&cache, &clients, false, None); + let refreshed = load(&cache, &clients, true, Some("test-key"), &|_| {}); + let later = load(&cache, &clients, false, None, &|_| {}); assert_eq!(stub.requests().len(), 2); assert_eq!(refreshed[0].list, first[0].list); @@ -610,6 +699,7 @@ mod tests { &clients, false, Some("test-key"), + &|_| {}, ); let requests = stub.requests(); @@ -630,7 +720,7 @@ mod tests { let cache = dir.path().join("missing").join(CACHE_FILE); let clients = [client("gateway", "openai_chat", &stub.url)]; - let loaded = load(&cache, &clients, false, Some("test-key")); + let loaded = load(&cache, &clients, false, Some("test-key"), &|_| {}); assert_eq!(models(&loaded), ["model-a"]); assert!( @@ -640,19 +730,55 @@ mod tests { assert!(!cache.exists()); } + #[test] + fn the_cache_file_keeps_the_newer_of_two_lists() { + let dir = tempfile::tempdir().expect("tempdir"); + let cache = dir.path().join(CACHE_FILE); + let url = "https://api.example/v1/models"; + let list = |model: &str, fetched_at| ModelList { + models: vec![model.to_string()], + fetched_at, + }; + + // The load that fetched first writes last. + write_cache(&cache, url, &list("new", 200)).expect("write"); + write_cache(&cache, url, &list("old", 100)).expect("write"); + + assert_eq!(read_cache(&cache).get(url), Some(&list("new", 200))); + } + #[test] fn reports_a_rejected_key_and_a_missing_key() { let stub = Stub::start("HTTP/1.1 401 Unauthorized\r\nConnection: close\r\n\r\n{}".to_string()); let gateway = client("gateway", "openai_chat", &stub.url); + let from_env = Client { + api_key_env: Some("SWITCHYARD_MENUBAR_TEST_UNSET_KEY".to_string()), + forward_auth: false, + ..gateway.clone() + }; assert_eq!(list(&gateway, Some("wrong")), Err(ListError::Rejected(401))); assert_eq!(list(&gateway, None), Err(ListError::NoKey)); - assert_eq!(stub.requests().len(), 1, "a missing key sends no request"); + assert_eq!( + list(&from_env, None), + Err(ListError::NoEnv( + "SWITCHYARD_MENUBAR_TEST_UNSET_KEY".to_string() + )) + ); + assert!(matches!( + list(&gateway, Some("key\r\nX-Injected: yes")), + Err(ListError::Failed(_)) + )); + assert_eq!( + stub.requests().len(), + 1, + "a missing key or a key with a line break sends no request" + ); } #[test] - fn builds_the_models_url_like_the_server_builds_request_urls() { + fn builds_the_models_url_with_the_servers_url_rules() { for (format, base_url, expected) in [ ( "openai_chat", @@ -674,6 +800,12 @@ mod tests { "https://api.example/v1/messages", "https://api.example/v1/models", ), + // The server sends messages to /foo/messages/v1/messages here. + ( + "anthropic_messages", + "https://api.example/foo/messages", + "https://api.example/foo/messages/v1/models", + ), ( "openai_chat", "https://api.example/v1?api-version=1", diff --git a/crates/switchyard-menubar/src/picker.rs b/crates/switchyard-menubar/src/picker.rs index a860a1747..f6b27f0c4 100644 --- a/crates/switchyard-menubar/src/picker.rs +++ b/crates/switchyard-menubar/src/picker.rs @@ -19,9 +19,10 @@ use objc2::rc::Retained; use objc2::runtime::ProtocolObject; use objc2::{DefinedClass, MainThreadOnly, define_class, msg_send, sel}; use objc2_app_kit::{ - NSApplication, NSBackingStoreType, NSButton, NSColor, NSComboBox, NSComboBoxDelegate, - NSControl, NSControlTextEditingDelegate, NSFont, NSPopUpButton, NSSecureTextField, NSTextField, - NSTextFieldDelegate, NSView, NSWindow, NSWindowStyleMask, + NSAccessibility, NSApplication, NSBackingStoreType, NSButton, NSColor, NSComboBox, + NSComboBoxDelegate, NSControl, NSControlTextEditingDelegate, NSFont, NSPopUpButton, + NSResponder, NSScreen, NSScrollView, NSSecureTextField, NSTextField, NSTextFieldDelegate, + NSTextView, NSView, NSWindow, NSWindowStyleMask, }; use objc2_foundation::{ MainThreadMarker, NSNotification, NSObject, NSObjectProtocol, NSPoint, NSRect, NSSize, @@ -41,6 +42,7 @@ const ALGORITHM_TAG: isize = 2; const SAVE_KEY_TAG: isize = 3; const APPLY_TAG: isize = 4; const REFRESH_TAG: isize = 5; +const CLOSE_TAG: isize = 6; const CLIENT_TAG: isize = 100; const MODEL_TAG: isize = 200; const SELECTED_TAG: isize = 300; @@ -56,11 +58,11 @@ const MODEL_WIDTH: f64 = WIDTH - MODEL_X - MARGIN; /// three lines under them. const ROLE_HEIGHT: f64 = 80.0; const NOTE_HEIGHT: f64 = 44.0; -/// Height of the key area: two lines of text above the key field. -const KEY_HEIGHT: f64 = 64.0; +/// Height of the key area: three lines of text above the key field. +const KEY_HEIGHT: f64 = 80.0; /// Height of the algorithm's description: up to two lines, then its roles. const SUMMARY_HEIGHT: f64 = 60.0; -/// Height of the result area, which fits a wrapped `--dry-run` error. +/// Height of the result area. A longer result scrolls. const STATUS_HEIGHT: f64 = 124.0; define_class!( @@ -125,16 +127,47 @@ impl ControlTarget { } } +define_class!( + /// A model box that scrolls itself into view when it gets keyboard + /// focus, so Tab can reach a role row that the scroll view hides. + // SAFETY: NSComboBox has no subclassing requirements, and `ModelBox` + // does not implement `Drop`. + #[unsafe(super(NSComboBox, NSTextField, NSControl, NSView, NSResponder, NSObject))] + #[thread_kind = MainThreadOnly] + #[name = "SwitchyardPickerModelBox"] + struct ModelBox; + + impl ModelBox { + // SAFETY: the signature matches NSResponder's method. + #[unsafe(method(becomeFirstResponder))] + fn become_first_responder(&self) -> bool { + // SAFETY: NSResponder's `becomeFirstResponder` takes no + // arguments and returns a BOOL. + let became: bool = unsafe { msg_send![super(self), becomeFirstResponder] }; + if became { + self.scrollRectToVisible(self.bounds()); + } + became + } + } +); + +impl ModelBox { + fn new(mtm: MainThreadMarker, frame: NSRect) -> Retained { + // SAFETY: NSComboBox's `initWithFrame:` has this signature. + unsafe { msg_send![Self::alloc(mtm), initWithFrame: frame] } + } +} + /// A result that a worker thread sends back to the window. enum Done { - /// Model lists that load number `load` found. `status`, when set, is the - /// outcome to show in the result area, because the user asked for this - /// load. + /// The model list at one URL, from load number `load`. Listed { load: u64, - lists: Vec, - status: Option, + loaded: Loaded, }, + /// The outcome of a load that the user asked for, for the result area. + Status(String), Applied(Result), } @@ -180,7 +213,7 @@ impl ListState { /// The controls of one role row. struct RoleRow { client: Retained, - model: Retained, + model: Retained, note: Retained, } @@ -192,7 +225,7 @@ struct Controls { key_label: Retained, key: Retained, save_key: Retained, - status: Retained, + status: Retained, apply: Retained, } @@ -309,7 +342,12 @@ impl Picker { fn load(&mut self, route: Option<&str>) { let path = &self.settings.config_file; let loaded = std::fs::read_to_string(path) - .map_err(|error| format!("read {}: {error}", path.display())) + .map_err(|error| { + format!( + "Could not read {}: {error}. Check config_file in the menu bar settings.", + path.display() + ) + }) .and_then(|text| ServerConfig::parse(&text)); match loaded { Ok(config) => { @@ -361,9 +399,10 @@ impl Picker { self.roles = config.roles(&route.key, &ALGORITHMS[0]); self.choices = vec![Choice::default(); self.roles.len()]; self.status = format!( - "{} is a {} route, which this window cannot show. Applying replaces its \ - settings with the algorithm you choose.", - route.id, route.kind + "This window cannot show the current settings of route {}: its type is not \ + in the Algorithm list, or it is a custom-mode llm_classifier. Apply replaces \ + those settings with the algorithm and models you pick.", + route.id ); } } @@ -455,11 +494,13 @@ impl Picker { clients } - /// Loads the lists that `clients` use on a worker thread. With - /// `refresh`, the worker fetches them even when the cache file has + /// Loads the lists that `clients` use on a worker thread, which loads + /// each URL on its own thread and reports each list as it arrives. With + /// `refresh`, the worker fetches the lists even when the cache file has /// them, and the outcome replaces the result area's text, because the /// user asked for it. `key` is a key the user just typed, with its - /// `base_url`; the worker saves it in the Keychain before it fetches. + /// `base_url`. The worker lists the models with it first, and saves it in + /// the Keychain only when the models endpoint did not reject it. fn load_lists(&mut self, clients: Vec, refresh: bool, key: Option<(String, String)>) { self.loads += 1; let load = self.loads; @@ -473,27 +514,22 @@ impl Picker { let cache = self.cache.clone(); let sender = self.sender.clone(); std::thread::spawn(move || { + let typed_key = key.as_ref().map(|(_, key)| key.as_str()); + let lists = models::load(&cache, &clients, refresh, typed_key, &|loaded| { + let _ = sender.send(Done::Listed { + load, + loaded: loaded.clone(), + }); + }); + if !refresh { + return; + } let mut status = Vec::new(); if let Some((base_url, key)) = &key { - status.push(match models::save_key(base_url, key) { - Ok(()) => format!("Saved the key for {base_url} in your login Keychain."), - Err(error) => format!( - "Could not save the key in your login Keychain: {error}. The window \ - used the key for this list only." - ), - }); + status.push(keep_key(base_url, key, &lists)); } - let typed_key = key.as_ref().map(|(_, key)| key.as_str()); - let lists = models::load(&cache, &clients, refresh, typed_key); - let status = refresh.then(|| { - status.extend(lists.iter().map(outcome)); - status.join("\n") - }); - let _ = sender.send(Done::Listed { - load, - lists, - status, - }); + status.extend(lists.iter().map(outcome)); + let _ = sender.send(Done::Status(status.join("\n"))); }); } @@ -525,6 +561,7 @@ impl Picker { SAVE_KEY_TAG => self.save_key(), REFRESH_TAG => self.refresh_models(), APPLY_TAG => self.apply(), + CLOSE_TAG => self.window.close(), _ if (CLIENT_TAG..MODEL_TAG).contains(&tag) => { let row = tag.abs_diff(CLIENT_TAG); let client = controls @@ -536,7 +573,9 @@ impl Picker { if let (Some(client), Some(choice)) = (client, self.choices.get_mut(row)) { choice.client = client; self.list_models(); - self.update_row(row); + // A new client can show or hide the key field and + // enable or disable Apply. + self.update_rows(); } } _ if (MODEL_TAG..SELECTED_TAG).contains(&tag) => { @@ -570,17 +609,12 @@ impl Picker { match done { Done::Listed { load, - lists, - status, + loaded: Loaded { url, list, error }, } => { - for Loaded { url, list, error } in lists { - self.lists.entry(url).or_default().finish(load, list, error); - } + self.lists.entry(url).or_default().finish(load, list, error); self.update_rows(); - if let Some(status) = status { - self.set_status(status); - } } + Done::Status(status) => self.set_status(status), Done::Applied(result) => { self.busy = false; match result { @@ -606,16 +640,17 @@ impl Picker { if !self.settings.prices.contains_key(model) && !seen.contains(&model) { seen.push(model); notes.push_str(&format!( - "\nmenubar.toml has no price for {model}, so savings stay hidden until you \ - add one." + "\nmenubar.toml has no price for {model}. Savings stay hidden until you add \ + one and restart the menu bar app." )); } } notes } - /// Saves the typed key for the key field's `base_url`, then fetches the - /// lists of every client with that `base_url` using the typed key. + /// Lists the models of every client with the key field's `base_url` + /// using the typed key, and then saves the key unless the models + /// endpoint rejected it. fn save_key(&mut self) { let (Some(controls), Some(base_url)) = (&self.controls, self.key_url().map(str::to_string)) else { @@ -627,6 +662,14 @@ impl Picker { self.set_status("Paste a key into the field first.".to_string()); return; } + if models::has_line_break(&key) { + self.set_status( + "Could not use the key, because it has a line break. Paste the key again \ + without line breaks." + .to_string(), + ); + return; + } let clients: Vec = self .clients .iter() @@ -634,7 +677,7 @@ impl Picker { .cloned() .collect(); self.set_status(format!( - "Saving the key for {base_url} and listing its models…" + "Checking the key by listing the models at {base_url}…" )); self.load_lists(clients, true, Some((base_url, key))); } @@ -674,15 +717,22 @@ impl Picker { fn set_status(&mut self, status: String) { self.status = status; if let Some(controls) = &self.controls { - controls - .status - .setStringValue(&NSString::from_str(&self.status)); - controls - .apply - .setEnabled(!self.busy && self.config.is_some()); + controls.status.setString(&NSString::from_str(&self.status)); + controls.apply.setEnabled(self.can_apply()); } } + /// Returns whether Apply can run: no Apply is running, the config has + /// loaded, and every role names an LLM client from the config. + fn can_apply(&self) -> bool { + !self.busy + && self.config.is_some() + && self + .choices + .iter() + .all(|choice| self.client(&choice.client).is_some()) + } + fn update_rows(&self) { for row in 0..self.choices.len() { self.update_row(row); @@ -690,6 +740,7 @@ impl Picker { let Some(controls) = &self.controls else { return; }; + controls.apply.setEnabled(self.can_apply()); let key_url = self.key_url(); let hidden = key_url.is_none(); controls.key_label.setHidden(hidden); @@ -699,9 +750,13 @@ impl Picker { controls .key_label .setStringValue(&NSString::from_str(&format!( - "To list the models at {url}, paste its key here and click Save key. The \ - app keeps the key in your login Keychain, not in a file." + "To list the models at {url}, paste the API key for that address and click \ + Save key. The app uses the key only to list models there and keeps it in \ + your login Keychain, not in a file." ))); + controls + .key + .setAccessibilityLabel(Some(&NSString::from_str(&format!("API key for {url}")))); } } @@ -729,15 +784,17 @@ impl Picker { let state = client.and_then(|client| self.lists.get(&models::list_url(client))); let loading = state.is_some_and(ListState::loading); let note = match (client, state) { - (None, _) if choice.client.is_empty() => { - "The config has no llm clients. Type a model ID.".to_string() - } + (None, _) if self.clients.is_empty() => "The config has no [llm_clients] entries. \ + Add an LLM client to the server config, \ + then open this window again." + .to_string(), (None, _) => format!( - "The config has no llm client named \"{}\". Type a model ID.", + "The config has no LLM client named \"{}\". Pick an LLM client from the list on \ + the left.", choice.client ), ( - Some(_), + Some(client), Some(ListState { list: Some(list), error, @@ -751,6 +808,12 @@ impl Picker { note.push('\n'); note.push_str(&error.reason()); } + // A cached list loads without a key, so the note must name + // the missing variable that Apply needs. + if let Some(variable) = models::missing_env(client) { + note.push('\n'); + note.push_str(&models::missing_env_note(variable)); + } note } ( @@ -770,16 +833,23 @@ impl Picker { } /// Rebuilds the window's controls for the current route and algorithm. + /// The role rows sit in a scroll view. It is as tall as the rows when the + /// screen has room, and shorter when it does not, so the buttons under + /// the rows stay on the screen. fn layout(&mut self) { let mtm = self.mtm; - let height = 2.0 * MARGIN - + 34.0 - + 32.0 - + SUMMARY_HEIGHT - + ROLE_HEIGHT * self.roles.len() as f64 - + KEY_HEIGHT - + STATUS_HEIGHT - + 32.0; + let fixed = 2.0 * MARGIN + 34.0 + 32.0 + SUMMARY_HEIGHT + KEY_HEIGHT + STATUS_HEIGHT + 32.0; + let rows_height = ROLE_HEIGHT * self.roles.len() as f64; + let screen = self.window.screen().or_else(|| NSScreen::mainScreen(mtm)); + let room = screen.as_ref().map_or(f64::INFINITY, |screen| { + self.window + .contentRectForFrameRect(screen.visibleFrame()) + .size + .height + - fixed + }); + let rows_shown = rows_height.min(room.max(ROLE_HEIGHT)); + let height = fixed + rows_shown; let view = NSView::initWithFrame(NSView::alloc(mtm), rect(0.0, 0.0, WIDTH, height)); // Controls are placed from the top; AppKit's origin is bottom left. let mut top = MARGIN; @@ -794,10 +864,11 @@ impl Picker { let routes: Vec = self.routes.iter().map(|route| route.id.clone()).collect(); let route = self.popup( &routes, - self.route, + Some(self.route), rect(FIELD_X, y + 8.0, FIELD_WIDTH, 26.0), ); route.setTag(ROUTE_TAG); + route.setAccessibilityLabel(Some(ns_string!("Route"))); view.addSubview(&route); let y = next(32.0); @@ -805,10 +876,11 @@ impl Picker { let kinds: Vec = ALGORITHMS.iter().map(|a| a.kind.to_string()).collect(); let algorithm = self.popup( &kinds, - self.algorithm, + Some(self.algorithm), rect(FIELD_X, y + 6.0, FIELD_WIDTH, 26.0), ); algorithm.setTag(ALGORITHM_TAG); + algorithm.setAccessibilityLabel(Some(ns_string!("Algorithm"))); view.addSubview(&algorithm); let y = next(SUMMARY_HEIGHT); @@ -829,38 +901,46 @@ impl Picker { .iter() .map(|client| format!("{} ({})", client.name, client.format)) .collect(); + let rows_y = next(rows_shown); + let rows_view = + NSView::initWithFrame(NSView::alloc(mtm), rect(0.0, 0.0, WIDTH, rows_height)); let mut rows = Vec::with_capacity(self.roles.len()); for (row, (role, choice)) in self.roles.iter().zip(&self.choices).enumerate() { - let y = next(ROLE_HEIGHT); + let y = rows_height - ROLE_HEIGHT * (row + 1) as f64; let tag = isize::try_from(row).unwrap_or(0); // The popup and the combo box sit above the note. let controls_y = y + NOTE_HEIGHT + 8.0; - view.addSubview(&self.label(&role.label, rect(MARGIN, controls_y + 4.0, 86.0, 18.0))); + rows_view + .addSubview(&self.label(&role.label, rect(MARGIN, controls_y + 4.0, 86.0, 18.0))); + // When the config has no client with the choice's name, the + // popup selects nothing, and the note names the missing client. let selected = self .clients .iter() - .position(|client| client.name == choice.client) - .unwrap_or(0); + .position(|client| client.name == choice.client); let client = self.popup( &clients, selected, rect(FIELD_X, controls_y, CLIENT_WIDTH, 26.0), ); client.setTag(CLIENT_TAG + tag); - view.addSubview(&client); + client.setAccessibilityLabel(Some(&NSString::from_str(&format!( + "{} LLM client", + role.label + )))); + rows_view.addSubview(&client); - let model = NSComboBox::initWithFrame( - NSComboBox::alloc(mtm), - rect(MODEL_X, controls_y, MODEL_WIDTH, 26.0), - ); + let model = ModelBox::new(mtm, rect(MODEL_X, controls_y, MODEL_WIDTH, 26.0)); model.setStringValue(&NSString::from_str(&choice.model)); - model.setPlaceholderString(Some(ns_string!("Type to search, or enter a model ID"))); + model.setPlaceholderString(Some(ns_string!("Type to filter, or type any model ID"))); model.setNumberOfVisibleItems(14); model.setCompletes(false); model.setTag(MODEL_TAG + tag); + model + .setAccessibilityLabel(Some(&NSString::from_str(&format!("{} model", role.label)))); // SAFETY: the target lives as long as the window. unsafe { model.setDelegate(Some(ProtocolObject::from_ref(&*self.target))) }; - view.addSubview(&model); + rows_view.addSubview(&model); let note = self.wrapping( "", @@ -870,22 +950,34 @@ impl Picker { NSFont::smallSystemFontSize(), ))); note.setTextColor(Some(&NSColor::secondaryLabelColor())); - view.addSubview(¬e); + rows_view.addSubview(¬e); rows.push(RoleRow { client, model, note, }); } + let scroll = NSScrollView::initWithFrame( + NSScrollView::alloc(mtm), + rect(0.0, rows_y, WIDTH, rows_shown), + ); + scroll.setHasVerticalScroller(true); + scroll.setAutohidesScrollers(true); + scroll.setDrawsBackground(false); + scroll.setDocumentView(Some(&rows_view)); + view.addSubview(&scroll); + // The rows view's origin is its bottom left, so the scroll view + // starts at the last role. Show the first role instead. + rows_view.scrollPoint(NSPoint::new(0.0, rows_height - rows_shown)); let y = next(KEY_HEIGHT); - let key_label = self.wrapping("", rect(FIELD_X, y + 30.0, FIELD_WIDTH, 32.0)); + let key_label = self.wrapping("", rect(FIELD_X, y + 30.0, FIELD_WIDTH, 48.0)); view.addSubview(&key_label); let key = NSSecureTextField::initWithFrame( NSSecureTextField::alloc(mtm), rect(FIELD_X, y + 4.0, FIELD_WIDTH - 110.0, 22.0), ); - key.setPlaceholderString(Some(ns_string!("Paste the key"))); + key.setPlaceholderString(Some(ns_string!("Paste the API key"))); view.addSubview(&key); let save_key = self.button( "Save key", @@ -895,12 +987,32 @@ impl Picker { view.addSubview(&save_key); let y = next(STATUS_HEIGHT); - let status = self.wrapping( - &self.status, + // The result can be longer than the area, such as a long --dry-run + // error, so the area scrolls. + let status_scroll = NSScrollView::initWithFrame( + NSScrollView::alloc(mtm), rect(MARGIN, y + 4.0, WIDTH - 2.0 * MARGIN, STATUS_HEIGHT - 8.0), ); - status.setSelectable(true); - view.addSubview(&status); + status_scroll.setHasVerticalScroller(true); + status_scroll.setAutohidesScrollers(true); + status_scroll.setDrawsBackground(false); + let size = status_scroll.contentSize(); + let status = NSTextView::initWithFrame( + NSTextView::alloc(mtm), + rect(0.0, 0.0, size.width, size.height), + ); + // The text view grows down as the text gets longer, and wraps at the + // area's width. + status.setMinSize(size); + status.setMaxSize(NSSize::new(size.width, f64::MAX)); + status.setVerticallyResizable(true); + status.setHorizontallyResizable(false); + status.setEditable(false); + status.setDrawsBackground(false); + status.setFont(Some(&NSFont::systemFontOfSize(NSFont::systemFontSize()))); + status.setString(&NSString::from_str(&self.status)); + status_scroll.setDocumentView(Some(&status)); + view.addSubview(&status_scroll); let refresh = self.button( "Refresh models", @@ -909,12 +1021,21 @@ impl Picker { ); refresh.setEnabled(!self.choices.is_empty()); view.addSubview(&refresh); + let close = self.button( + "Close", + CLOSE_TAG, + rect(WIDTH - MARGIN - 210.0, MARGIN - 4.0, 100.0, 32.0), + ); + // Escape closes the window, as in a macOS dialog. + close.setKeyEquivalent(&NSString::from_str("\u{1b}")); + view.addSubview(&close); let apply = self.button( "Apply", APPLY_TAG, rect(WIDTH - MARGIN - 100.0, MARGIN - 4.0, 100.0, 32.0), ); - apply.setEnabled(!self.busy && self.config.is_some()); + // Return runs Apply, the window's default button. + apply.setKeyEquivalent(ns_string!("\r")); view.addSubview(&apply); // Keep the window's top edge in place as its height changes. @@ -922,7 +1043,27 @@ impl Picker { let top_left = NSPoint::new(frame.origin.x, frame.origin.y + frame.size.height); self.window.setContentSize(NSSize::new(WIDTH, height)); self.window.setContentView(Some(&view)); + // Tab moves between the controls in the order they appear, including + // the controls inside the scroll view. + self.window.recalculateKeyViewLoop(); self.window.setFrameTopLeftPoint(top_left); + // A taller window can reach past the bottom of the screen. Move it + // back onto the screen, so that Apply stays in reach. + if let Some(screen) = &screen { + let visible = screen.visibleFrame(); + let mut frame = self.window.frame(); + frame.origin.x = frame + .origin + .x + .min(visible.origin.x + visible.size.width - frame.size.width) + .max(visible.origin.x); + frame.origin.y = frame + .origin + .y + .max(visible.origin.y) + .min(visible.origin.y + visible.size.height - frame.size.height); + self.window.setFrameOrigin(frame.origin); + } self.controls = Some(Controls { route, @@ -949,13 +1090,24 @@ impl Picker { label } - fn popup(&self, items: &[String], selected: usize, frame: NSRect) -> Retained { + /// Returns a popup that lists `items` with `selected` chosen, or with no + /// item chosen when `selected` is `None`. + fn popup( + &self, + items: &[String], + selected: Option, + frame: NSRect, + ) -> Retained { let popup = NSPopUpButton::initWithFrame_pullsDown(NSPopUpButton::alloc(self.mtm), frame, false); for item in items { popup.addItemWithTitle(&NSString::from_str(item)); } - popup.selectItemAtIndex(isize::try_from(selected).unwrap_or(0)); + popup.selectItemAtIndex( + selected + .and_then(|index| isize::try_from(index).ok()) + .unwrap_or(-1), + ); // SAFETY: `ControlTarget` implements `controlChanged:` and lives as // long as the window. unsafe { @@ -1008,7 +1160,7 @@ fn fill(ui: &RoleRow, list: &ModelList, query: &str) -> String { } if query.is_empty() || shown.len() == listed.len() { format!( - "{} models, listed {}. Type to filter.", + "{} models, fetched {}. Type to filter.", listed.len(), age(list.fetched_at) ) @@ -1019,12 +1171,32 @@ fn fill(ui: &RoleRow, list: &ModelList, query: &str) -> String { } } +/// Saves a typed key in the Keychain unless the models endpoint rejected +/// it while the app listed `lists` with it, and says what happened. +fn keep_key(base_url: &str, key: &str, lists: &[Loaded]) -> String { + if lists + .iter() + .any(|loaded| matches!(loaded.error, Some(ListError::Rejected(_)))) + { + return format!( + "Did not save the key for {base_url}, because the models endpoint rejected it." + ); + } + match models::save_key(base_url, key) { + Ok(()) => format!("Saved the key for {base_url} in your login Keychain."), + Err(error) => format!( + "Could not save the key in your login Keychain: {error}. The app used the key this \ + time only and did not keep it. To try again, paste the key and click Save key." + ), + } +} + /// Describes one loaded list for the result area. fn outcome(loaded: &Loaded) -> String { let mut line = format!("{}:", loaded.url); if let Some(list) = &loaded.list { line.push_str(&format!( - " {} models, listed {}.", + " {} models, fetched {}.", list.models.len(), age(list.fetched_at) )); diff --git a/crates/switchyard-menubar/src/server.rs b/crates/switchyard-menubar/src/server.rs index 387fa7ecc..87db0dca1 100644 --- a/crates/switchyard-menubar/src/server.rs +++ b/crates/switchyard-menubar/src/server.rs @@ -4,7 +4,8 @@ //! Controls the installed server: check a config with its `--dry-run`, save //! it, and restart the LaunchAgent that runs it. -use std::io::Write; +use std::fs::OpenOptions; +use std::io::{ErrorKind, Write}; use std::path::{Path, PathBuf}; use std::process::Command; use std::time::{Duration, Instant}; @@ -13,16 +14,21 @@ use chrono::Local; use crate::config::Config; use crate::health::{ServerStatus, probe}; +use crate::models; use crate::server_config::{Algorithm, Choice, ServerConfig}; /// How long to wait for the restarted server to answer `/health`. const RESTART_WAIT: Duration = Duration::from_secs(10); +/// How many backups one file can get within one second. +const BACKUPS_PER_SECOND: u32 = 100; + /// Switches a route to new models, checks the result with the server's /// `--dry-run`, saves it, and restarts the server. /// -/// Returns what happened on success. An error means the config file was not -/// changed. +/// Returns what happened on success, one line each: the saved file and its +/// backup, the restart, notes about the edit, and the check. An error means +/// the config file was not changed. pub fn apply( settings: &Config, route: &str, @@ -31,27 +37,68 @@ pub fn apply( ) -> Result { let path = &settings.config_file; // Read the file again so an edit made while the window was open is kept. - let text = std::fs::read_to_string(path) - .map_err(|error| format!("read {}: {error}", path.display()))?; - let edited = ServerConfig::parse(&text)?.edit(route, algorithm, choices)?; - if edited == text { - return Ok("Nothing to save: the route already uses these models.".to_string()); + let text = std::fs::read_to_string(path).map_err(|error| { + format!( + "Could not read {}: {error}. Check config_file in the menu bar settings.", + path.display() + ) + })?; + let config = ServerConfig::parse(&text)?; + let edited = config.edit(route, algorithm, choices)?; + if edited.text == text { + return Ok( + "Nothing to save. The route already uses this algorithm and these models.".to_string(), + ); } let binary = server_binary()?; - let saved = save_checked(&binary, path, &edited)?; - let restarted = match restart(&settings.launchd_label) { - Err(error) => format!("The restart failed: {error}"), - Ok(()) if wait_for_health(&settings.server_url) => format!( - "Restarted {}, and the server answers {}/health.", - settings.launchd_label, settings.server_url + let backup = save_checked(&binary, path, &text, &edited.text).map_err(explain)?; + let label = &settings.launchd_label; + let health = format!("{}/health", settings.server_url); + let mut lines = vec![format!( + "Saved {}. The old file is at {}.", + path.display(), + backup.display() + )]; + lines.push(match restart(label) { + Err(error) => format!( + "Could not restart {label}: {error}. The server uses the new config the next time \ + it starts. Click Restart server in the menu to try again." ), + Ok(()) if wait_for_health(&settings.server_url) => { + format!("Restarted {label}, and the server answers {health}.") + } Ok(()) => format!( - "Restarted {}, but the server has not answered {}/health yet. Check its log.", - settings.launchd_label, settings.server_url + "Restarted {label}, but the server did not answer {health} within {} seconds. \ + Check the server log, by default in ~/.switchyard/logs/. To undo, copy {} over {} \ + and click Restart server.", + RESTART_WAIT.as_secs(), + backup.display(), + path.display() ), - }; - Ok(format!("{saved}\n{restarted}")) + }); + lines.extend(edited.notes); + let routes = config.routes().len(); + let plural = if routes == 1 { "" } else { "s" }; + lines.push(format!( + "The switchyard-server --dry-run check passed for {routes} route{plural}." + )); + Ok(lines.join("\n")) +} + +/// Adds advice to a check error that comes from a missing `api_key_env` +/// variable. The check runs with this app's environment, which is not the +/// user's shell environment. +fn explain(error: String) -> String { + let variable = error + .split_once("could not read api_key_env ") + .and_then(|(_, rest)| rest.split([':', ' ', '\n']).next()) + .filter(|variable| !variable.is_empty()) + .map(str::to_string); + match variable { + Some(variable) => format!("{error}\n{}", models::missing_env_note(&variable)), + None => error, + } } /// Returns the `switchyard-server` installed next to this app, which is the @@ -59,82 +106,139 @@ pub fn apply( fn server_binary() -> Result { std::env::current_exe() .map(|exe| exe.with_file_name("switchyard-server")) - .map_err(|error| format!("find switchyard-server next to this app: {error}")) + .map_err(|error| { + format!( + "Could not find switchyard-server next to the menu bar app: {error}. Run make \ + install-macos from the Switchyard repository to install it." + ) + }) } /// Checks `text` with `switchyard-server --dry-run`, then replaces `path` -/// with it, keeping the old file as a timestamped backup. +/// with it, keeping `original`, the text that `path` held, as a timestamped +/// backup. Returns the backup's path. /// /// The real file is `path`, or the file it points to when `path` is a /// symlink. The new text goes to a temporary file next to the real file. The /// temporary file is synced to disk and then renamed over the real file, so a -/// reader never sees a half-written config. When the check fails, the -/// temporary file is removed, `path` does not change, and the server's error -/// is returned. -fn save_checked(binary: &Path, path: &Path, text: &str) -> Result { +/// reader never sees a half-written config. When the check fails, or when +/// the real file no longer holds `original`, the temporary file is removed, +/// `path` does not change, and the error is returned. +fn save_checked(binary: &Path, path: &Path, original: &str, text: &str) -> Result { // Resolve a symlink and replace the file it points to, so the link stays. - let file = - std::fs::canonicalize(path).map_err(|error| format!("find {}: {error}", path.display()))?; + let file = std::fs::canonicalize(path) + .map_err(|error| format!("Could not find {}: {error}", path.display()))?; let dir = file.parent().unwrap_or(Path::new("/")); let mut candidate = tempfile::Builder::new() .prefix(".switchyard-picker-") .suffix(".toml") .tempfile_in(dir) - .map_err(|error| format!("create a file in {}: {error}", dir.display()))?; + .map_err(|error| format!("Could not create a file in {}: {error}", dir.display()))?; candidate .write_all(text.as_bytes()) .and_then(|()| candidate.as_file().sync_all()) - .map_err(|error| format!("write {}: {error}", candidate.path().display()))?; + .map_err(|error| format!("Could not write {}: {error}", candidate.path().display()))?; if let Ok(metadata) = std::fs::metadata(&file) { candidate .as_file() .set_permissions(metadata.permissions()) - .map_err(|error| format!("copy the permissions of {}: {error}", path.display()))?; + .map_err(|error| { + format!( + "Could not copy the permissions of {}: {error}", + path.display() + ) + })?; } // Name the real file in errors, not the temporary one. - let summary = check(binary, candidate.path()).map_err(|error| { + check(binary, candidate.path()).map_err(|error| { error.replace( &candidate.path().display().to_string(), &path.display().to_string(), ) })?; - let backup = PathBuf::from(format!( - "{}.switchyard-backup.{}", - file.display(), - Local::now().format("%Y%m%d%H%M%S") - )); - std::fs::copy(&file, &backup) - .map_err(|error| format!("back up {}: {error}", path.display()))?; + // The check takes a moment. Keep an edit that another program saved + // in the meantime. + if std::fs::read_to_string(&file).ok().as_deref() != Some(original) { + return Err(format!( + "Could not replace {}, because it changed on disk while Apply was checking it. \ + Click Apply again to apply your choices to the new file.", + path.display() + )); + } + let backup = back_up(&file, original) + .map_err(|error| format!("Could not back up {}: {error}", path.display()))?; candidate .persist(&file) - .map_err(|error| format!("replace {}: {}", path.display(), error.error))?; - Ok(format!( - "switchyard-server --dry-run: {summary}\nSaved {}, and kept the old file as {}.", - path.display(), - backup.display() + .map_err(|error| format!("Could not replace {}: {}", path.display(), error.error))?; + Ok(backup) +} + +/// Writes `text` to a new backup of `file` and returns its path. The backup +/// is `.switchyard-backup.`, the name the installer uses, +/// with `-2`, `-3`, and so on added when that name is taken, so a backup +/// never replaces another one. The backup gets the file's permissions before +/// it gets the text. +fn back_up(file: &Path, text: &str) -> Result { + let permissions = std::fs::metadata(file)?.permissions(); + let stamp = Local::now().format("%Y%m%d%H%M%S"); + for number in 1..=BACKUPS_PER_SECOND { + let suffix = if number == 1 { + String::new() + } else { + format!("-{number}") + }; + let backup = PathBuf::from(format!( + "{}.switchyard-backup.{stamp}{suffix}", + file.display() + )); + let mut out = match OpenOptions::new() + .write(true) + .create_new(true) + .open(&backup) + { + Ok(out) => out, + Err(error) if error.kind() == ErrorKind::AlreadyExists => continue, + Err(error) => return Err(error), + }; + let written = out + .set_permissions(permissions.clone()) + .and_then(|()| out.write_all(text.as_bytes())) + .and_then(|()| out.sync_all()); + if let Err(error) = written { + let _ = std::fs::remove_file(&backup); + return Err(error); + } + return Ok(backup); + } + Err(std::io::Error::new( + ErrorKind::AlreadyExists, + format!("{BACKUPS_PER_SECOND} backups from this second already exist"), )) } -/// Runs `switchyard-server --config --dry-run` and returns its -/// summary, or its error output when the config is invalid. -fn check(binary: &Path, config: &Path) -> Result { +/// Runs `switchyard-server --config --dry-run`, and returns its +/// error output when the config is invalid. +fn check(binary: &Path, config: &Path) -> Result<(), String> { let output = Command::new(binary) .arg("--config") .arg(config) .arg("--dry-run") .output() - .map_err(|error| format!("run {}: {error}", binary.display()))?; - let stdout = String::from_utf8_lossy(&output.stdout).trim().to_string(); - let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + .map_err(|error| { + format!( + "Could not run {}: {error}. Apply needs switchyard-server next to the menu bar \ + app. Run make install-macos from the Switchyard repository to install it.", + binary.display() + ) + })?; if output.status.success() { - Ok(stdout) - } else if stderr.is_empty() { - Err(stdout) - } else { - Err(stderr) + return Ok(()); } + let stdout = String::from_utf8_lossy(&output.stdout).trim().to_string(); + let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + Err(if stderr.is_empty() { stdout } else { stderr }) } /// Restarts the server's LaunchAgent. @@ -162,10 +266,10 @@ pub fn command(program: &str, args: &[&str]) -> Result { let output = Command::new(program) .args(args) .output() - .map_err(|error| format!("run {program}: {error}"))?; + .map_err(|error| format!("Could not run {program}: {error}"))?; if !output.status.success() { return Err(format!( - "{program} {}: {}", + "{program} {} failed: {}", args.join(" "), String::from_utf8_lossy(&output.stderr).trim() )); @@ -207,7 +311,7 @@ mod tests { r#"echo "invalid server config $2: route gateway cannot forward both" >&2; exit 1"#, ); - let error = save_checked(&binary, &config, "new").expect_err("check fails"); + let error = save_checked(&binary, &config, "old", "new").expect_err("check fails"); assert_eq!( error, @@ -237,18 +341,62 @@ mod tests { r#"grep -q new "$2" && echo "server OK: gateway""#, ); - let message = save_checked(&binary, &config, "new").expect("saved"); + let backup = save_checked(&binary, &config, "old", "new").expect("saved"); - assert!(message.starts_with("switchyard-server --dry-run: server OK: gateway\n")); assert_eq!(std::fs::read_to_string(&config).expect("read"), "new"); - let mode = std::fs::metadata(&config) - .expect("metadata") - .permissions() - .mode(); - assert_eq!(mode & 0o777, 0o640, "the file keeps its permissions"); - let backups = backups(dir.path()); - assert_eq!(backups.len(), 1); - assert_eq!(std::fs::read_to_string(&backups[0]).expect("read"), "old"); + let mode = |path: &Path| { + std::fs::metadata(path) + .expect("metadata") + .permissions() + .mode() + & 0o777 + }; + assert_eq!(mode(&config), 0o640, "the file keeps its permissions"); + assert_eq!(backups(dir.path()).len(), 1); + assert_eq!(std::fs::read_to_string(&backup).expect("read"), "old"); + assert_eq!(mode(&backup), 0o640, "the backup gets the same permissions"); + } + + #[test] + fn a_second_save_keeps_the_first_backup() { + let dir = tempfile::tempdir().expect("tempdir"); + let config = dir.path().join("composite.toml"); + std::fs::write(&config, "old").expect("write config"); + let binary = fake_server(dir.path(), "exit 0"); + + // Both saves usually fall in the same second, so their backups get + // the same timestamp. + save_checked(&binary, &config, "old", "new").expect("saved"); + save_checked(&binary, &config, "new", "newer").expect("saved"); + + let mut kept: Vec = backups(dir.path()) + .iter() + .map(|backup| std::fs::read_to_string(backup).expect("read")) + .collect(); + kept.sort(); + assert_eq!(kept, ["new", "old"]); + assert_eq!(std::fs::read_to_string(&config).expect("read"), "newer"); + } + + #[test] + fn a_file_that_changes_during_the_check_is_not_replaced() { + let dir = tempfile::tempdir().expect("tempdir"); + let config = dir.path().join("composite.toml"); + std::fs::write(&config, "old").expect("write config"); + // Another program saves the file while the check runs. + let binary = fake_server( + dir.path(), + &format!("echo edited-elsewhere > '{}'", config.display()), + ); + + let error = save_checked(&binary, &config, "old", "new").expect_err("file changed"); + + assert!(error.contains(&config.display().to_string()), "{error}"); + assert_eq!( + std::fs::read_to_string(&config).expect("read"), + "edited-elsewhere\n" + ); + assert!(backups(dir.path()).is_empty()); } #[test] @@ -260,7 +408,7 @@ mod tests { std::os::unix::fs::symlink(&real, &link).expect("symlink"); let binary = fake_server(dir.path(), r#"echo "server OK: gateway""#); - save_checked(&binary, &link, "new").expect("saved"); + save_checked(&binary, &link, "old", "new").expect("saved"); let link_type = std::fs::symlink_metadata(&link) .expect("metadata") diff --git a/crates/switchyard-menubar/src/server_config.rs b/crates/switchyard-menubar/src/server_config.rs index 67e6f2a94..50896ff74 100644 --- a/crates/switchyard-menubar/src/server_config.rs +++ b/crates/switchyard-menubar/src/server_config.rs @@ -164,6 +164,19 @@ const COMMON_ROUTE_KEYS: [&str; 6] = [ "vision", ]; +/// Target settings that change the request body. The server keeps one +/// target per model on a client, so it rejects two targets that name the +/// same model on the same client with different values for these. +const REQUEST_SETTINGS: [&str; 3] = ["omit_body_fields", "reasoning_effort", "extra_body"]; + +/// Target settings that a route takes on when it uses the target. +const TARGET_SETTINGS: [&str; 4] = [ + "system_prompt", + "reasoning_effort", + "extra_body", + "omit_body_fields", +]; + /// One entry under `[routes]`. #[derive(Clone, Debug, PartialEq)] pub struct Route { @@ -208,6 +221,26 @@ enum Slot { Listed(usize), } +impl Slot { + /// Returns the slot's key path under the route, such as + /// `stage.capable_target`. + fn path(&self) -> String { + match self { + Self::Key(path) => path.join("."), + Self::Listed(_) => "targets".to_string(), + } + } +} + +/// The result of [`ServerConfig::edit`]. +#[derive(Debug)] +pub struct Edited { + /// The new config text. + pub text: String, + /// What the user should know about the edit, one sentence each. + pub notes: Vec, +} + /// A parsed server config that keeps the original text's layout. #[derive(Clone, Debug)] pub struct ServerConfig { @@ -218,7 +251,12 @@ impl ServerConfig { pub fn parse(text: &str) -> Result { text.parse::() .map(|doc| Self { doc }) - .map_err(|error| format!("parse the server config: {error}")) + .map_err(|error| { + format!( + "Could not parse the server config. Fix this TOML error and try again:\n\ + {error}" + ) + }) } pub fn routes(&self) -> Vec { @@ -246,8 +284,9 @@ impl ServerConfig { .collect() } - /// The algorithm a route uses now, or `None` when the picker cannot show - /// it, such as a custom-mode classifier whose groups are free-form. + /// Returns the algorithm a route uses now, or `None` when the picker + /// cannot show it, such as a custom-mode classifier whose groups are + /// free-form. pub fn algorithm(&self, route: &str) -> Option<&'static Algorithm> { let route = self.route(route)?; if text(route, "mode") == Some("custom") { @@ -257,8 +296,8 @@ impl ServerConfig { ALGORITHMS.iter().find(|algorithm| algorithm.kind == kind) } - /// The roles `algorithm` needs on `route`. A random route keeps its - /// number of targets, and has at least two. + /// Returns the roles that `algorithm` needs on `route`. A random route + /// keeps its number of targets, and has at least two. pub fn roles(&self, route: &str, algorithm: &Algorithm) -> Vec { if algorithm.kind != RANDOM { return algorithm @@ -290,8 +329,8 @@ impl ServerConfig { .collect() } - /// The model each role of the route's current algorithm uses. A role - /// whose target is missing gets an empty choice. + /// Returns the model that each role of the route's current algorithm + /// uses. A role whose target is missing gets an empty choice. pub fn choices(&self, route: &str) -> Vec { let Some(algorithm) = self.algorithm(route) else { return Vec::new(); @@ -307,24 +346,25 @@ impl ServerConfig { } /// Returns the config text with `route` switched to `algorithm`, using - /// one choice per role. + /// one choice per role, and notes about the edit. /// /// For each role, in order of preference: keep the route's target when it /// already names the model; use another target that names exactly this - /// model on this client; change the route's target in place when no other - /// route or role uses it, including a role earlier in this edit; otherwise - /// add a new target that copies it. A changed or copied target keeps - /// settings such as `extra_body` and `omit_body_fields`, including - /// settings meant for the old model. Targets no route uses any more are - /// left in the file. + /// model on this client when the route would get the same target + /// settings from it, or when the server would reject a second target for + /// that model; change the route's target in place when no other route or + /// role uses it, including a role earlier in this edit; otherwise add a + /// new target that copies it. A changed or copied target keeps settings + /// such as `extra_body` and `omit_body_fields`, including settings meant + /// for the old model. Targets no route uses any more are left in the file. pub fn edit( &self, route: &str, algorithm: &Algorithm, choices: &[Choice], - ) -> Result { + ) -> Result { if self.route(route).is_none() { - return Err(format!("The config has no route {route}.")); + return Err(format!("The config has no [routes.{route}] table.")); } let roles = self.roles(route, algorithm); if roles.len() != choices.len() { @@ -338,29 +378,35 @@ impl ServerConfig { let clients = self.clients(); for (role, choice) in roles.iter().zip(choices) { if choice.model.trim().is_empty() { - return Err(format!("Choose a model for {}.", role.label)); + return Err(format!("Pick a model for {}.", role.label)); + } + if choice.client.is_empty() { + return Err(format!("Pick an LLM client for {}.", role.label)); } if !clients.iter().any(|client| client.name == choice.client) { - return Err(format!("The config has no llm client {}.", choice.client)); + return Err(format!( + "The config has no LLM client named \"{}\".", + choice.client + )); } } let current = self.algorithm(route); let same_type = current.is_some_and(|current| current.kind == algorithm.kind); + let old_roles = current + .map(|current| self.roles(route, current)) + .unwrap_or_default(); // When the type changes, each new role starts from the old role with // the same tier, so a composite's capable target stays the capable one. - let by_tier: Vec<(Option, &str)> = current - .map(|current| { - self.roles(route, current) - .iter() - .filter_map(|role| Some((role.tier, self.target_at(route, &role.slot)?))) - .collect() - }) - .unwrap_or_default(); - let mut references = self.target_references(); + let by_tier: Vec<(Option, &str)> = old_roles + .iter() + .filter_map(|role| Some((role.tier, self.target_at(route, &role.slot)?))) + .collect(); + let mut users = self.target_users(); let mut doc = self.doc.clone(); let mut names = Vec::with_capacity(roles.len()); + let mut notes = Vec::new(); for (role, choice) in roles.iter().zip(choices) { let existing = if same_type { self.target_at(route, &role.slot) @@ -376,14 +422,30 @@ impl ServerConfig { client: choice.client.clone(), model: choice.model.trim().to_string(), }; - names.push(choose_target( - &mut doc, - &mut references, - route, - role, - &choice, - existing, - )?); + let (name, moved_from) = + choose_target(&mut doc, &mut users, route, role, &choice, existing)?; + if let Some(old) = moved_from + && let Some(note) = kept_settings_note(&doc, &clients, &name, &old, &choice) + { + notes.push(note); + } + let mut others: Vec<&str> = users + .get(name.as_str()) + .into_iter() + .flatten() + .copied() + .filter(|other| *other != route) + .collect(); + others.dedup(); + if Some(name.as_str()) != existing && !others.is_empty() { + let plural = if others.len() == 1 { "" } else { "s" }; + notes.push(format!( + "{} now shares [targets.{name}] with route{plural} {}.", + role.label, + others.join(", ") + )); + } + names.push(name); } let table = doc @@ -391,9 +453,18 @@ impl ServerConfig { .and_then(Item::as_table_like_mut) .and_then(|routes| routes.get_mut(route)) .and_then(Item::as_table_like_mut) - .ok_or_else(|| format!("The config has no route {route}."))?; + .ok_or_else(|| format!("The config has no [routes.{route}] table."))?; if !same_type { - reset_route(table, algorithm); + let old_slots: Vec = old_roles.iter().map(|role| role.slot.path()).collect(); + let removed = reset_route(table, algorithm, &old_slots); + if !removed.is_empty() { + notes.push(format!( + "Switching to {} removed these settings from [routes.{route}]: {}. The \ + backup still has them.", + algorithm.kind, + removed.join(", ") + )); + } } for (role, name) in roles.iter().zip(&names) { set_slot(table, &role.slot, name); @@ -403,7 +474,10 @@ impl ServerConfig { set_at(table, path, setting.value()); } } - Ok(doc.to_string()) + Ok(Edited { + text: doc.to_string(), + notes, + }) } fn table(&self, name: &str) -> Option<&dyn TableLike> { @@ -437,62 +511,73 @@ impl ServerConfig { } } - /// Counts how many times each target name appears in any route, nested - /// policies included. A name that appears once belongs to one role. - fn target_references(&self) -> HashMap<&str, usize> { - let mut counts: HashMap<&str, usize> = entries(self.table("targets")) - .map(|(name, _)| (name, 0)) + /// Returns, for each target name, the routes that name it: one entry per + /// mention in any route, nested policies included. A target with one + /// entry belongs to one role. + fn target_users(&self) -> HashMap<&str, Vec<&str>> { + let mut users: HashMap<&str, Vec<&str>> = entries(self.table("targets")) + .map(|(name, _)| (name, Vec::new())) .collect(); - if let Some(routes) = self.doc.get("routes") { - visit_strings(routes, &mut |text| { - if let Some(count) = counts.get_mut(text) { - *count += 1; + for (route, item) in self + .table("routes") + .into_iter() + .flat_map(|routes| routes.iter()) + { + visit_strings(item, &mut |text| { + if let Some(routes) = users.get_mut(text) { + routes.push(route); } }); } - counts + users } } -/// Picks or writes the target that serves `choice` for one role, and returns -/// its name. Reusing a target adds one to its reference count, so a later -/// role in the same edit copies that target instead of changing it in place. -fn choose_target( +/// Picks or writes the target that serves `choice` for one role. Returns its +/// name, and the model that the target named before when the edit changed +/// the target in place or copied it. Reusing a target adds the route to the +/// target's users, so a later role in the same edit copies that target +/// instead of changing it in place. +fn choose_target<'a>( doc: &mut DocumentMut, - references: &mut HashMap<&str, usize>, - route: &str, + users: &mut HashMap<&'a str, Vec<&'a str>>, + route: &'a str, role: &Role, choice: &Choice, existing: Option<&str>, -) -> Result { +) -> Result<(String, Option), String> { let targets = doc .get_mut("targets") .and_then(Item::as_table_like_mut) .ok_or("The config has no [targets] table.")?; + let current = existing + .and_then(|name| targets.get(name)) + .and_then(Item::as_table_like); if let Some(name) = existing - && targets - .get(name) - .is_some_and(|target| names_choice(target, choice)) + && current.is_some_and(|target| names_choice(target, choice)) { - return Ok(name.to_string()); + return Ok((name.to_string(), None)); } - if let Some((name, _)) = targets - .iter() - .find(|(_, target)| names_choice(target, choice)) - { - if let Some(count) = references.get_mut(name) { - *count += 1; + let old = current.and_then(|target| { + Some(Choice { + client: text(target, "llm_client")?.to_string(), + model: text(target, "id")?.to_string(), + }) + }); + if let Some(name) = reusable(&*targets, current, choice).map(str::to_string) { + if let Some(routes) = users.get_mut(name.as_str()) { + routes.push(route); } - return Ok(name.to_string()); + return Ok((name, None)); } if let Some(name) = existing - && references.get(name) == Some(&1) + && users.get(name).is_some_and(|routes| routes.len() == 1) && let Some(target) = targets.get_mut(name).and_then(Item::as_table_like_mut) { - set_value(target, "llm_client", Value::from(choice.client.as_str())); set_value(target, "id", Value::from(choice.model.as_str())); - return Ok(name.to_string()); + set_value(target, "llm_client", Value::from(choice.client.as_str())); + return Ok((name.to_string(), old)); } let mut table = Table::new(); @@ -501,15 +586,17 @@ fn choose_target( .and_then(Item::as_table_like) { for (key, item) in source.iter() { - table.insert(key, item.clone()); + let mut item = item.clone(); + unplace(&mut item); + table.insert(key, item); } } + set_value(&mut table, "id", Value::from(choice.model.as_str())); set_value( &mut table, "llm_client", Value::from(choice.client.as_str()), ); - set_value(&mut table, "id", Value::from(choice.model.as_str())); let base = format!("{route}_{}", role.label.to_lowercase().replace(' ', "_")); let mut name = base.clone(); let mut suffix = 1; @@ -518,19 +605,136 @@ fn choose_target( name = format!("{base}_{suffix}"); } targets.insert(&name, Item::Table(table)); - Ok(name) + Ok((name, old)) } -/// Whether a target names exactly this model on this client. -fn names_choice(target: &Item, choice: &Choice) -> bool { - target.as_table_like().is_some_and(|target| { - text(target, "llm_client") == Some(choice.client.as_str()) - && text(target, "id") == Some(choice.model.as_str()) +/// Returns a note about the request settings that a target kept when the +/// edit moved it from `old` to `new`, a model of another family or on a +/// client with another request format. A setting meant for the old model can +/// make the provider reject the new model's requests, and `--dry-run` does +/// not catch that. So the note lists the kept settings, or says that the +/// target has none, and the user decides. +fn kept_settings_note( + doc: &DocumentMut, + clients: &[Client], + name: &str, + old: &Choice, + new: &Choice, +) -> Option { + let format = |name: &str| { + clients + .iter() + .find(|client| client.name == name) + .map(|client| client.format.as_str()) + }; + if family(&old.model) == family(&new.model) && format(&old.client) == format(&new.client) { + return None; + } + let target = doc + .get("targets")? + .as_table_like()? + .get(name)? + .as_table_like()?; + let kept: Vec = REQUEST_SETTINGS + .iter() + .filter_map(|key| Some(format!("{key} = {}", setting(target, key)?))) + .collect(); + Some(if kept.is_empty() { + format!( + "[targets.{name}] now names {} and has no omit_body_fields, reasoning_effort, or \ + extra_body. Check whether the new model needs one of them.", + new.model + ) + } else { + format!( + "[targets.{name}] now names {} and kept {} from {}. Check that the new model \ + accepts these settings.", + new.model, + kept.join(", "), + old.model + ) }) } -/// Removes the old type's settings and sets the new `type`. -fn reset_route(route: &mut dyn TableLike, algorithm: &Algorithm) { +/// Returns a model's family: the first word of the last part of its ID, +/// such as `gpt` for `openai/gpt-5.6-sol`. +fn family(model: &str) -> String { + let name = model.rsplit('/').next().unwrap_or(model); + name.split(['-', '.', '_', ':']) + .next() + .unwrap_or(name) + .to_lowercase() +} + +/// Returns another target that names `choice` and that a role whose target +/// is `current` may use. The role takes on that target's settings. So it +/// uses a target with the same settings as `current`, or else a target whose +/// request settings differ from `current`, because the server would reject a +/// second target for this model with other request settings. +fn reusable<'t>( + targets: &'t dyn TableLike, + current: Option<&dyn TableLike>, + choice: &Choice, +) -> Option<&'t str> { + let same = |target: &dyn TableLike, keys: &[&str]| { + keys.iter() + .all(|key| setting(target, key) == current.and_then(|current| setting(current, key))) + }; + let candidates: Vec<(&str, &dyn TableLike)> = targets + .iter() + .filter_map(|(name, target)| Some((name, target.as_table_like()?))) + .filter(|(_, target)| names_choice(*target, choice)) + .collect(); + candidates + .iter() + .find(|(_, target)| same(*target, &TARGET_SETTINGS)) + .or_else(|| { + candidates + .iter() + .find(|(_, target)| !same(*target, &REQUEST_SETTINGS)) + }) + .map(|(name, _)| *name) +} + +/// Returns a target's setting as plain data, so the same value written in +/// another layout compares equal. +fn setting(target: &dyn TableLike, key: &str) -> Option { + let mut table = Table::new(); + table.insert(key, target.get(key)?.clone()); + DocumentMut::from(table) + .to_string() + .parse::() + .ok()? + .remove(key) +} + +/// Clears the file position of every table in `item`. A copied sub-table +/// then follows the header of the table it is copied into, not the header +/// of the table it came from. +fn unplace(item: &mut Item) { + if let Some(table) = item.as_table_mut() { + table.set_position(None); + for (_, child) in table.iter_mut() { + unplace(child); + } + } +} + +/// Returns whether a target names exactly this model on this client. +fn names_choice(target: &dyn TableLike, choice: &Choice) -> bool { + text(target, "llm_client") == Some(choice.client.as_str()) + && text(target, "id") == Some(choice.model.as_str()) +} + +/// Removes the old type's settings and sets the new `type`. Returns the +/// removed settings as key paths, such as `classifier.base_threshold`. The +/// list leaves out `old_slots`, the old roles' targets, which the new roles +/// replace. +fn reset_route( + route: &mut dyn TableLike, + algorithm: &Algorithm, + old_slots: &[String], +) -> Vec { let kept = |key: &str| COMMON_ROUTE_KEYS.contains(&key) || (algorithm.subagents && key == "subagents"); let stale: Vec = route @@ -539,10 +743,27 @@ fn reset_route(route: &mut dyn TableLike, algorithm: &Algorithm) { .filter(|key| !kept(key)) .map(str::to_string) .collect(); + let mut removed = Vec::new(); for key in stale { - route.remove(&key); + if let Some(item) = route.remove(&key) { + key_paths(&key, &item, &mut removed); + } } + removed.retain(|path| !old_slots.contains(path)); set_value(route, "type", Value::from(algorithm.kind)); + removed +} + +/// Adds the key path of every value in `item` to `paths`. +fn key_paths(path: &str, item: &Item, paths: &mut Vec) { + match item.as_table_like() { + Some(table) => { + for (key, child) in table.iter() { + key_paths(&format!("{path}.{key}"), child, paths); + } + } + None => paths.push(path.to_string()), + } } fn set_slot(route: &mut dyn TableLike, slot: &Slot, name: &str) { @@ -691,17 +912,24 @@ confidence_threshold = 0.5 } /// Edits the config and checks the result with the server's parser. - fn edit(text: &str, route: &str, kind: &str, choices: &[Choice]) -> String { + fn edit(text: &str, route: &str, kind: &str, choices: &[Choice]) -> Edited { let edited = ServerConfig::parse(text) .expect("parse") .edit(route, algorithm(kind), choices) .expect("edit"); - if let Err(error) = switchyard_runner::Runner::from_toml(&edited) { - panic!("the server rejects the edited config: {error}\n{edited}"); + if let Err(error) = switchyard_runner::Runner::from_toml(&edited.text) { + panic!( + "the server rejects the edited config: {error}\n{}", + edited.text + ); } edited } + fn capable_target(config: &ServerConfig) -> Option<&str> { + config.target_at("gateway", &Slot::Key(&["stage", "capable_target"])) + } + #[test] fn every_algorithm_writes_a_config_the_server_accepts() { let gateway = ServerConfig::parse(GATEWAY).expect("parse"); @@ -715,7 +943,7 @@ confidence_threshold = 0.5 let edited = edit(GATEWAY, "gateway", algorithm.kind, &models[3 - roles..]); - let config = ServerConfig::parse(&edited).expect("parse"); + let config = ServerConfig::parse(&edited.text).expect("parse"); assert_eq!( config.algorithm("gateway").map(|a| a.kind), Some(algorithm.kind) @@ -736,7 +964,7 @@ confidence_threshold = 0.5 let edited = edit(GATEWAY, "gateway", "composite", &chosen); assert_eq!( - ServerConfig::parse(&edited) + ServerConfig::parse(&edited.text) .expect("parse") .choices("gateway"), chosen @@ -754,7 +982,8 @@ confidence_threshold = 0.5 choice("gateway", "gpt-5.6-sol"), choice("gateway", "gpt-5.6-luna"), ], - ); + ) + .text; let config = ServerConfig::parse(&gpt).expect("parse"); assert_eq!( @@ -782,7 +1011,7 @@ confidence_threshold = 0.5 choice("gateway_chat", "claude-sonnet-5"), ], ); - assert_eq!(claude, GATEWAY); + assert_eq!(claude.text, GATEWAY); } #[test] @@ -797,7 +1026,7 @@ confidence_threshold = 0.5 ], ); - let config = ServerConfig::parse(&edited).expect("parse"); + let config = ServerConfig::parse(&edited.text).expect("parse"); assert_eq!( config.algorithm("gateway").map(|a| a.kind), Some("stage_router") @@ -813,16 +1042,26 @@ confidence_threshold = 0.5 assert_eq!(text(route, "picker"), Some("efficient_first")); assert!(route.get("classifier").is_none() && route.get("stage").is_none()); assert!( - edited.contains("[targets.gateway_judge]"), + edited.text.contains("[targets.gateway_judge]"), "unused targets stay in the file" ); + let removed = edited.notes.join("\n"); + assert!( + removed.contains("classifier.message_hash_fallback") + && !removed.contains("stage.capable_target"), + "the result lists the removed settings, not the replaced targets: {removed}" + ); } #[test] fn a_target_another_route_uses_is_copied_not_changed() { - let shared = format!( - "{GATEWAY}\n[routes.direct]\nid = \"direct\"\ntype = \"passthrough\"\ntarget = \"gateway_capable\"\n" - ); + // The shared target has a sub-table, which the copy must keep under + // its own header. + let shared = GATEWAY.replace( + "omit_body_fields = [\"reasoning_effort\"]\n\n[targets.gateway_efficient]", + "omit_body_fields = [\"reasoning_effort\"]\n\n[targets.gateway_capable.extra_body]\n\ + top_k = 5\n\n[targets.gateway_efficient]", + ) + "\n[routes.direct]\nid = \"direct\"\ntype = \"passthrough\"\ntarget = \"gateway_capable\"\n"; let edited = edit( &shared, @@ -835,32 +1074,142 @@ confidence_threshold = 0.5 ], ); - let config = ServerConfig::parse(&edited).expect("parse"); + let config = ServerConfig::parse(&edited.text).expect("parse"); assert_eq!( config.choices("direct"), [choice("gateway_chat", "claude-opus-5-5")] ); assert_eq!( - config.target_at("gateway", &Slot::Key(&["stage", "capable_target"])), - Some("gateway_capable_2") + config.choices("gateway")[1], + choice("gateway", "gpt-5.6-sol") ); + let copy = capable_target(&config).expect("capable target"); + assert_ne!(copy, "gateway_capable"); + let targets = config.table("targets").expect("targets"); + let (source, copied) = ( + targets.get("gateway_capable").and_then(Item::as_table_like), + targets.get(copy).and_then(Item::as_table_like), + ); + for key in TARGET_SETTINGS { + assert_eq!( + source.and_then(|target| setting(target, key)), + copied.and_then(|target| setting(target, key)), + "the copy keeps {key}" + ); + } + let header = |name: &str| edited.text.find(&format!("[{name}]")).expect(name); assert!( - edited.contains( - "[targets.gateway_capable_2]\nid = \"gpt-5.6-sol\"\nllm_client = \"gateway\" # chat, for caching\nomit_body_fields = [\"reasoning_effort\"]" - ), - "the copy keeps the shared target's settings:\n{edited}" + header(&format!("targets.{copy}")) < header(&format!("targets.{copy}.extra_body")), + "the copied sub-table follows the copy's header:\n{}", + edited.text + ); + } + + #[test] + fn uses_another_routes_target_only_when_the_route_keeps_its_settings_or_must_share() { + // `prompted` names the model that Capable moves to, and has a + // system prompt that the gateway route does not have. + for (omit, shared) in [ + // Same request settings: the server accepts a second target for + // the model, so Capable keeps its own target. + ("omit_body_fields = [\"reasoning_effort\"]\n", false), + // Other request settings: the server would reject a second + // target, so Capable shares `prompted`, and the result says so. + ("", true), + ] { + let text = format!( + "{GATEWAY}\n[targets.prompted]\nid = \"gpt-5.6-sol\"\nllm_client = \"gateway\"\n\ + system_prompt = \"You answer for route other.\"\n{omit}\n[routes.other]\n\ + id = \"other\"\ntype = \"passthrough\"\ntarget = \"prompted\"\n" + ); + + let edited = edit( + &text, + "gateway", + "composite", + &[ + choice("gateway", "gpt-5.6-terra"), + choice("gateway", "gpt-5.6-sol"), + choice("gateway_chat", "claude-sonnet-5"), + ], + ); + + let config = ServerConfig::parse(&edited.text).expect("parse"); + assert_eq!( + config.choices("gateway")[1], + choice("gateway", "gpt-5.6-sol") + ); + assert_eq!( + capable_target(&config) == Some("prompted"), + shared, + "{omit}" + ); + assert_eq!( + edited + .notes + .iter() + .any(|note| note.contains("prompted") && note.contains("other")), + shared, + "{:?}", + edited.notes + ); + } + } + + #[test] + fn a_move_to_another_model_family_reports_the_settings_the_target_kept() { + let gpt = edit( + GATEWAY, + "gateway", + "composite", + &[ + choice("gateway", "gpt-5.6-terra"), + choice("gateway", "gpt-5.6-sol"), + choice("gateway", "gpt-5.6-luna"), + ], + ) + .text; + + // The judge target has no request settings, and the capable target + // keeps omit_body_fields from its GPT model. Efficient stays. + let edited = edit( + &gpt, + "gateway", + "composite", + &[ + choice("gateway_chat", "claude-haiku-4-5"), + choice("gateway_chat", "claude-opus-5-5"), + choice("gateway", "gpt-5.6-luna"), + ], ); + + let note = |target: &str| { + edited + .notes + .iter() + .find(|note| note.contains(&format!("[targets.{target}]"))) + .cloned() + }; + let capable = note("gateway_capable").expect("a note for the capable target"); + assert!( + capable.contains("claude-opus-5-5") + && capable.contains(r#"omit_body_fields = ["reasoning_effort"]"#), + "{capable}" + ); + let judge = note("gateway_judge").expect("a note for the judge target"); + assert!( + judge.contains("claude-haiku-4-5") && !judge.contains(" = "), + "{judge}" + ); + assert_eq!(note("gateway_efficient"), None); } #[test] fn rejects_a_missing_model_or_an_unknown_client() { let config = ServerConfig::parse(GATEWAY).expect("parse"); - for (capable, expected) in [ - (choice("gateway", " "), "Choose a model for Capable."), - ( - choice("typo", "gpt-5.6-sol"), - "The config has no llm client typo.", - ), + for (capable, named) in [ + (choice("gateway", " "), "Capable"), + (choice("typo", "gpt-5.6-sol"), "typo"), ] { let error = config .edit( @@ -874,7 +1223,7 @@ confidence_threshold = 0.5 ) .expect_err("invalid choice"); - assert_eq!(error, expected); + assert!(error.contains(named), "{error}"); } } } diff --git a/crates/switchyard-menubar/src/tray.rs b/crates/switchyard-menubar/src/tray.rs index 2568bf955..5de887944 100644 --- a/crates/switchyard-menubar/src/tray.rs +++ b/crates/switchyard-menubar/src/tray.rs @@ -38,10 +38,11 @@ pub fn run(config: Config, settings: &Path) -> Result<(), String> { let ns_app = NSApplication::sharedApplication(mtm); // Accessory keeps the process out of the Dock and the app switcher. ns_app.setActivationPolicy(NSApplicationActivationPolicy::Accessory); - // An accessory app shows no menu bar, but text fields still find copy - // and paste shortcuts through the app's main menu. - let edit_menu = edit_menu()?; - edit_menu.init_for_nsapp(); + // An accessory app shows no menu bar, but AppKit still finds keyboard + // shortcuts, such as Cmd-C in a text field and Cmd-W in the picker + // window, through the app's main menu. + let main_menu = main_menu()?; + main_menu.init_for_nsapp(); let glyph = Icon::from_rgba(icon::glyph(), icon::SIZE, icon::SIZE) .map_err(|error| format!("build icon: {error}"))?; @@ -128,8 +129,9 @@ fn menu(rows: &[Row]) -> Result { Ok(menu) } -/// The app's hidden main menu, which gives text fields their Edit shortcuts. -fn edit_menu() -> Result { +/// Builds the app's hidden main menu, which gives text fields their Edit +/// shortcuts and windows their Cmd-W shortcut. +fn main_menu() -> Result { let edit = Submenu::with_items( "Edit", true, @@ -144,7 +146,9 @@ fn edit_menu() -> Result { ], ) .map_err(|error| format!("build the Edit menu: {error}"))?; - Menu::with_items(&[&edit]).map_err(|error| format!("build the main menu: {error}")) + let window = Submenu::with_items("Window", true, &[&PredefinedMenuItem::close_window(None)]) + .map_err(|error| format!("build the Window menu: {error}"))?; + Menu::with_items(&[&edit, &window]).map_err(|error| format!("build the main menu: {error}")) } /// Drains pending AppKit events, blocking briefly when there are none. diff --git a/scripts/macos/uninstall.sh b/scripts/macos/uninstall.sh index 5b677a0c5..eeb01c97c 100755 --- a/scripts/macos/uninstall.sh +++ b/scripts/macos/uninstall.sh @@ -67,4 +67,4 @@ say "Left in place, delete them if you want:" say " $SY_HOME (binaries, config, routing log, model lists, logs)" say " $CODEX_CONFIG.switchyard-current.* (configs preserved during restore)" say " $CODEX_PROFILE_CONFIG.switchyard-backup.* (profile backups)" -say " Keychain items named \"Switchyard model list\" (keys saved by Change routing…)" +say " Keychain items named \"Switchyard model list\" (keys saved from Change routing…; delete them in Keychain Access)" From 45c8992d019a6c3da131f56bb858687f6aa26277 Mon Sep 17 00:00:00 2001 From: Elyas Mehtabuddin Date: Fri, 2 Oct 2026 11:09:04 -0700 Subject: [PATCH 3/3] fix(menubar): check the server config for outside edits right before replacing it Signed-off-by: Elyas Mehtabuddin --- crates/switchyard-menubar/README.md | 9 +++++---- crates/switchyard-menubar/src/server.rs | 15 +++++++++------ 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/crates/switchyard-menubar/README.md b/crates/switchyard-menubar/README.md index 9bb033374..8e9234170 100644 --- a/crates/switchyard-menubar/README.md +++ b/crates/switchyard-menubar/README.md @@ -71,10 +71,11 @@ server's error and the file does not change. If it passes, the app saves the file, keeps the old one as `.switchyard-backup.`, and restarts the server with `launchctl kickstart -k gui//`. A second backup in the same second gets `-2` added to its name, so a backup -never replaces another one. If another program changes the file while the -check runs, Apply saves nothing and asks you to click Apply again. If -`config_file` is a symlink, the app writes the file that the link points to, -puts the backup next to that file, and leaves the link in place. +never replaces another one. If another program changes the file while Apply +checks it and writes the backup, Apply saves nothing, removes that backup, and +asks you to click Apply again. If `config_file` is a symlink, the app writes +the file that the link points to, puts the backup next to that file, and +leaves the link in place. The result area lists what happened, most important first: the saved file and its backup, the restart and whether the server answers `/health` within diff --git a/crates/switchyard-menubar/src/server.rs b/crates/switchyard-menubar/src/server.rs index 87db0dca1..0b9beb3a5 100644 --- a/crates/switchyard-menubar/src/server.rs +++ b/crates/switchyard-menubar/src/server.rs @@ -122,8 +122,9 @@ fn server_binary() -> Result { /// symlink. The new text goes to a temporary file next to the real file. The /// temporary file is synced to disk and then renamed over the real file, so a /// reader never sees a half-written config. When the check fails, or when -/// the real file no longer holds `original`, the temporary file is removed, -/// `path` does not change, and the error is returned. +/// the real file no longer holds `original` after the backup is written, the +/// temporary file and the new backup are removed, `path` does not change, +/// and the error is returned. fn save_checked(binary: &Path, path: &Path, original: &str, text: &str) -> Result { // Resolve a symlink and replace the file it points to, so the link stays. let file = std::fs::canonicalize(path) @@ -158,17 +159,19 @@ fn save_checked(binary: &Path, path: &Path, original: &str, text: &str) -> Resul ) })?; - // The check takes a moment. Keep an edit that another program saved - // in the meantime. + let backup = back_up(&file, original) + .map_err(|error| format!("Could not back up {}: {error}", path.display()))?; + // The check and the backup take a moment. Keep an edit that another + // program saved in the meantime. Compare right before the rename, because + // the rename replaces the file whatever it holds. if std::fs::read_to_string(&file).ok().as_deref() != Some(original) { + let _ = std::fs::remove_file(&backup); return Err(format!( "Could not replace {}, because it changed on disk while Apply was checking it. \ Click Apply again to apply your choices to the new file.", path.display() )); } - let backup = back_up(&file, original) - .map_err(|error| format!("Could not back up {}: {error}", path.display()))?; candidate .persist(&file) .map_err(|error| format!("Could not replace {}: {}", path.display(), error.error))?;