diff --git a/app/androidApp/build.gradle.kts b/app/androidApp/build.gradle.kts index 1b53f41..d41fbc4 100644 --- a/app/androidApp/build.gradle.kts +++ b/app/androidApp/build.gradle.kts @@ -144,4 +144,5 @@ dependencies { implementation(libs.androidx.core.ktx) implementation(libs.androidx.lifecycle.runtime.compose) implementation(libs.zxing.embedded) + implementation(libs.markdown.renderer) } diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/Markdown.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/Markdown.kt new file mode 100644 index 0000000..91dbf0a --- /dev/null +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/Markdown.kt @@ -0,0 +1,44 @@ +package com.example.aiapp + +import androidx.compose.material3.MaterialTheme +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.text.font.FontFamily +import com.mikepenz.markdown.m3.Markdown +import com.mikepenz.markdown.m3.markdownColor +import com.mikepenz.markdown.m3.markdownTypography + +/** + * An assistant's reply, rendered as the markdown it is written in. + * + * The parsing is the library's. Markdown is somebody else's specification, and a hand-written + * subset of one disagrees with it at the edges -- which is where the bug reports come from, one + * case at a time. This file's whole job is the mapping onto the app's palette. + * + * Colours come from the theme rather than from the renderer's defaults, so code, links and rules + * are the same Catppuccin values the rest of the app uses. Nothing here picks a colour of its own. + */ +@Composable +fun MarkdownText(text: String, modifier: Modifier = Modifier) { + Markdown( + content = text, + colors = + markdownColor( + text = MaterialTheme.colorScheme.onSurface, + codeText = codeColor, + inlineCodeText = codeColor, + linkText = linkColor, + dividerColor = MaterialTheme.colorScheme.outlineVariant, + codeBackground = MaterialTheme.colorScheme.surfaceVariant, + inlineCodeBackground = MaterialTheme.colorScheme.surfaceVariant, + ), + // Body text at the size everything else in the transcript uses, and code in a monospace + // face: a code block set in the body font stops looking like code at all. + typography = + markdownTypography( + text = MaterialTheme.typography.bodyLarge, + code = MaterialTheme.typography.bodyMedium.copy(fontFamily = FontFamily.Monospace), + ), + modifier = modifier, + ) +} diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt index 4095c5a..b01a892 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt @@ -372,10 +372,6 @@ fun SessionScreen( } } - // A send that never came back cannot stay outstanding forever; the - // turn ending is the latest moment it could still have been in flight. - LaunchedEffect(running) { if (!running) queued = emptyList() } - LaunchedEffect(summary.setupName, summary.provider) { offeredModels = try { @@ -394,13 +390,14 @@ fun SessionScreen( } } - fun act(action: () -> Unit) { + fun act(onFailure: () -> Unit = {}, action: () -> Unit) { scope.launch { try { withContext(Dispatchers.IO) { action() } actionError = null } catch (e: ApiException) { actionError = e.message + onFailure() } } } @@ -412,7 +409,17 @@ fun SessionScreen( input = "" pendingAttachments = emptyList() if (running && text.isNotEmpty()) queued = queued + text - act { sendMessage(settings, summary.id, text, attachments) } + // A held message leaves this list exactly two ways: the session + // reads it, which comes back as a UserMessage (see `apply`), or the + // send itself failed and there is nothing to wait for. Clearing the + // whole list when a turn ended was neither -- the server holds a + // queue of its own and takes one message per turn, so ending a turn + // is precisely when the *rest* are still waiting. It wiped them off + // the screen while they were on their way, which reads as messages + // two and three having been dropped. + act(onFailure = { queued = queued - text }) { + sendMessage(settings, summary.id, text, attachments) + } } // The system photo picker; the image uploads as soon as it's chosen, @@ -547,8 +554,7 @@ fun SessionScreen( items(items.asReversed()) { item -> when (item) { is TranscriptItem.UserMsg -> UserBubble(item.text) - is TranscriptItem.AssistantMsg -> - Text(item.text, style = MaterialTheme.typography.bodyLarge) + is TranscriptItem.AssistantMsg -> MarkdownText(item.text) is TranscriptItem.ToolRun -> ToolCard( tool = item, diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/Theme.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/Theme.kt index e59ebaf..18e4129 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/Theme.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/Theme.kt @@ -115,6 +115,21 @@ val awaitingColor: Color val warningColor: Color @Composable get() = Mocha.Yellow +/** + * Code: a fenced block, an inline span, a tool's input. + * + * Green because on this palette it is what a literal is coloured as, and because code sits on + * Surface 0 where the ordinary text colour would say nothing about it being code. + */ +val codeColor: Color + @Composable get() = Mocha.Green + +/** + * A link. Blue is what a link is on every Catppuccin surface, and the one colour to leave alone. + */ +val linkColor: Color + @Composable get() = Mocha.Blue + /** Past a limit. The scheme's error colour, for the reason [failedColor] gives. */ val overLimitColor: Color @Composable get() = MaterialTheme.colorScheme.error diff --git a/app/gradle/libs.versions.toml b/app/gradle/libs.versions.toml index 63acf55..25d886f 100644 --- a/app/gradle/libs.versions.toml +++ b/app/gradle/libs.versions.toml @@ -13,6 +13,12 @@ androidx-activityCompose = "1.13.0" # .toUri), and a transitive it merely inherited could change under it. androidx-core-ktx = "1.19.0" zxing-embedded = "4.3.0" +# Markdown rendering for assistant replies. The widely-used Compose +# Multiplatform renderer; markdown is somebody else's specification and a +# hand-written subset disagrees with it at the edges, one bug report at a +# time. Latest stable, checked 2026-08-29 against Maven Central -- 0.27.0 +# exists only as release candidates. +markdown-renderer = "0.26.0" # Declared rather than inherited for the same reason as core-ktx: SessionScreen # now calls repeatOnLifecycle/LocalLifecycleOwner directly, to hold the event # stream open only while the screen is on screen. Latest stable, checked @@ -38,6 +44,9 @@ androidx-lifecycle-runtime-compose = { module = "androidx.lifecycle:lifecycle-ru # Services / ML Kit model download involved. zxing-embedded = { module = "com.journeyapps:zxing-android-embedded", version.ref = "zxing-embedded" } desugar-jdk-libs = { module = "com.android.tools:desugar_jdk_libs", version.ref = "desugar-jdk-libs" } +# The -m3 flavour: it takes its colours and type from the ambient Material 3 +# theme, so the app's Catppuccin scheme is what it draws with. +markdown-renderer = { module = "com.mikepenz:multiplatform-markdown-renderer-m3", version.ref = "markdown-renderer" } # Declared directly rather than through the plugin's `compose.*` accessors, # which are deprecated as of CMP 1.11. compose-runtime = { module = "org.jetbrains.compose.runtime:runtime", version.ref = "compose-multiplatform" } diff --git a/server/src/main.rs b/server/src/main.rs index 2a5b24e..831dc05 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -188,19 +188,18 @@ async fn main() -> Result<()> { .await .context("failed to load TLS cert/key")?; - let monitor = Arc::new(usage::UsageMonitor::new(vec![Box::new( - usage::ClaudeUsage { - credentials_path: std::env::home_dir() - .unwrap_or_else(|| PathBuf::from("/")) - .join(".claude/.credentials.json"), - }, - )])); + // No providers listed here any more: which machines can be asked, and + // about what, comes from the setups at the moment the screen is opened + // -- so a machine added from the phone reports its limits without a + // restart, and the backend's own account stops standing in for every + // machine's. + let monitor = Arc::new(usage::UsageMonitor::new()); // The bearer-token middleware wraps the entire router -- routes and // fallback alike -- here and only here, so a new route can't forget // auth. Zero unauthenticated endpoints. let app = routes::router(Arc::clone(&manager)) - .merge(routes::usage_router(monitor)) + .merge(routes::usage_router(monitor, Arc::clone(&manager))) .merge(routes::models_router(Arc::clone(&models))) .layer(axum::middleware::from_fn_with_state( Arc::clone(&manager), diff --git a/server/src/routes.rs b/server/src/routes.rs index ae8490d..48df13b 100644 --- a/server/src/routes.rs +++ b/server/src/routes.rs @@ -621,19 +621,37 @@ async fn interrupt( Ok(StatusCode::NO_CONTENT) } +/// The usage screen needs two things that live in different places: the +/// cache, and the current list of machines to ask. Carried together rather +/// than the monitor holding the manager, which would point the dependency +/// upward -- `usage` sits below the session layer and should not reach +/// into it. +#[derive(Clone)] +pub struct UsageState { + monitor: Arc, + manager: Arc, +} + /// Separate router because its state is the usage monitor, not the /// session manager; merged (and auth-wrapped) with the rest in `main`. -pub fn usage_router(monitor: Arc) -> Router { +pub fn usage_router( + monitor: Arc, + manager: Arc, +) -> Router { Router::new() .route("/usage", get(usage)) - .with_state(monitor) + .with_state(UsageState { monitor, manager }) } async fn usage( - State(monitor): State>, + State(state): State, ) -> Result>, ApiError> { + // Read here rather than inside the fetch, so the list of machines is + // the one that existed when the request arrived and cannot change + // under a fetch that takes an ssh round trip per machine. + let setups = state.manager.setups(); // The fetch is blocking by design (see `usage`); off the workers. - let snapshots = tokio::task::spawn_blocking(move || monitor.snapshots()) + let snapshots = tokio::task::spawn_blocking(move || state.monitor.snapshots(&setups)) .await .context("usage fetch panicked")?; Ok(axum::Json(snapshots)) diff --git a/server/src/session/transport.rs b/server/src/session/transport.rs index 6950016..b075c75 100644 --- a/server/src/session/transport.rs +++ b/server/src/session/transport.rs @@ -103,8 +103,12 @@ impl Transport { Self::Here => None, Self::Ssh { ssh, .. } => Some(ssh), }; - let mut command = - crate::ssh::command(host, &launch.program, &launch.args, launch.cwd.as_deref()); + let mut command = tokio::process::Command::from(crate::ssh::command( + host, + &launch.program, + &launch.args, + launch.cwd.as_deref(), + )); match streams { Streams::Piped => { command @@ -141,6 +145,35 @@ impl Transport { }) } + /// Runs `launch` to completion and returns its stdout, blocking. + /// + /// The synchronous twin of `capture`, for callers that are already on a + /// blocking task and would otherwise need a runtime to ask a machine a + /// question. Both build the invocation the same way -- see + /// `crate::ssh::command` -- so there is still only one description of + /// what running something on another machine means. + pub fn capture_blocking(&self, launch: &Launch) -> Result { + let host = match self { + Self::Here => None, + Self::Ssh { ssh, .. } => Some(ssh), + }; + let output = + crate::ssh::command(host, &launch.program, &launch.args, launch.cwd.as_deref()) + .output() + .with_context(|| { + format!("couldn't run \"{}\" {}", launch.program, self.describe()) + })?; + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); + anyhow::bail!(if stderr.is_empty() { + format!("couldn't reach it ({})", output.status) + } else { + stderr + }); + } + Ok(String::from_utf8_lossy(&output.stdout).into_owned()) + } + /// How to say where this runs, for a log line a person reads. pub fn describe(&self) -> String { match self { diff --git a/server/src/ssh.rs b/server/src/ssh.rs index a4c8819..6a1bc50 100644 --- a/server/src/ssh.rs +++ b/server/src/ssh.rs @@ -11,8 +11,7 @@ //! only one place to configure connections (PLAN.md, rule 23). use std::path::Path; - -use tokio::process::Command; +use std::process::Command; use crate::config::SshConfig; @@ -34,6 +33,13 @@ const SSH_OPTIONS: [&str; 3] = [ /// decision and differs by more than the transport does -- a probe wants /// pipes it will drain, a session wants files that outlive this server -- /// so `Transport::spawn` applies it rather than this. +/// +/// A plain [`std::process::Command`], which `tokio` converts from, because +/// not every caller is async: the usage fetch is blocking by nature (it +/// makes a blocking HTTP call) and reads a file from the same machine on +/// the way, and it should not have to build an ssh invocation of its own +/// to do that. One place knows what a correct invocation is; how it is run +/// is the caller's business. pub fn command( remote: Option<&SshConfig>, program: &str, @@ -147,9 +153,8 @@ mod tests { /// The rendered argv, for asserting on what would actually run. fn argv(command: &Command) -> Vec { - let std = command.as_std(); - std::iter::once(std.get_program()) - .chain(std.get_args()) + std::iter::once(command.get_program()) + .chain(command.get_args()) .map(|arg| arg.to_string_lossy().into_owned()) .collect() } @@ -175,10 +180,7 @@ mod tests { Some(Path::new("/tmp/x")), ); assert_eq!(argv(&command), ["claude", "-p", "--verbose"]); - assert_eq!( - command.as_std().get_current_dir(), - Some(Path::new("/tmp/x")) - ); + assert_eq!(command.get_current_dir(), Some(Path::new("/tmp/x"))); } #[test] diff --git a/server/src/usage.rs b/server/src/usage.rs index 4c93a02..b626b37 100644 --- a/server/src/usage.rs +++ b/server/src/usage.rs @@ -15,14 +15,35 @@ //! //! One [`UsageProvider`] per paid service, so a second service later is a //! new impl behind the same snapshot shape, not a parallel screen. +//! +//! **Asked of the machine that spends the tokens, not of this one.** A +//! session runs wherever its setup says, so the account being billed is +//! that machine's, and reading this machine's credentials reports on an +//! account that may have run nothing. In the layout this project is aiming +//! at that is not a rounding error: `ai-server` belongs on the host, the +//! host has no `claude` CLI, and the CLI machine is a remote -- so the one +//! set of numbers the screen could show would be the numbers of an account +//! with no sessions. Credentials are therefore read through the session +//! `Transport`, one snapshot per setup that offers Claude. +//! +//! The token is read *to* the backend and the HTTP call is made from here, +//! rather than running the request on the far machine: it needs no tooling +//! there beyond a shell, and it keeps the one place that knows the wire +//! format in one place. The cost is that a remote machine's token is in +//! this process's memory for the length of a fetch, which is the same +//! trust the backend already has over that machine (it can start processes +//! on it). -use std::path::PathBuf; +use std::collections::HashMap; use std::sync::Mutex; use std::time::{Duration, Instant}; use serde::Serialize; use serde_json::Value; +use crate::config::{DriverKind, SetupConfig}; +use crate::session::transport::{Launch, Transport}; + const USAGE_URL: &str = "https://api.anthropic.com/api/oauth/usage"; const MIN_POLL_INTERVAL: Duration = Duration::from_secs(180); /// Matched to the CLI version the wire formats were pinned against. @@ -42,15 +63,43 @@ pub struct UsageWindow { pub active: bool, } +/// What came back when a machine was asked about its limits. +/// +/// Four answers rather than a flag and a message, because the screen has to +/// treat them differently and a reader has to. "Nobody is logged in here" +/// is a machine working exactly as configured -- somebody chose not to put +/// an account on it -- while "I could not reach it" is a fault worth +/// chasing, and "the endpoint refused me" is a third thing that says +/// nothing about the machine at all. Collapsing them into one `error` +/// string made the first look like the last, so a perfectly healthy setup +/// read as broken. +#[derive(Debug, Clone, Serialize, PartialEq)] +#[serde(tag = "state", rename_all = "camelCase")] +pub enum UsageState { + /// Numbers were fetched; `windows` has them. + Ok, + /// The machine answered and has no Claude credentials. A choice, not a + /// fault: nothing to report and nothing to fix. + NotLoggedIn, + /// The machine could not be asked at all. + Unreachable { detail: String }, + /// The machine is logged in, but the usage endpoint did not answer. + Failed { detail: String }, +} + #[derive(Debug, Clone, Serialize)] #[serde(rename_all = "camelCase")] pub struct UsageSnapshot { pub provider: String, - pub available: bool, + /// Which machine these are the numbers for. The point of the whole + /// module: they belong to an account on a particular box. + pub setup: String, + /// That machine's current label, resolved when the snapshot is built, + /// so renaming a setup renames it here too. + pub setup_name: String, + #[serde(flatten)] + pub state: UsageState, pub windows: Vec, - /// Why `available` is false, written for the screen. - #[serde(skip_serializing_if = "Option::is_none")] - pub error: Option, /// Epoch seconds the numbers were fetched (they can be up to the poll /// interval old). pub fetched_at: f64, @@ -62,30 +111,55 @@ pub trait UsageProvider: Send + Sync { fn fetch(&self) -> UsageSnapshot; } -/// Reads the numbers behind Claude Code's `/usage` using its own stored -/// credentials -- nothing to configure, and it reports on exactly the -/// account the local CLI runs as. +/// Reads the numbers behind Claude Code's `/usage` from one machine, using +/// the credentials that machine stores -- nothing to configure, and it +/// reports on exactly the account whose CLI runs the sessions there. pub struct ClaudeUsage { - pub credentials_path: PathBuf, + pub setup: String, + pub setup_name: String, + /// How to reach that machine. `Here` for the backend's own. + pub transport: Transport, } +/// Where Claude Code keeps its credentials, as a shell word rather than a +/// path: `$HOME` is expanded by the shell on the machine being asked, +/// which is the only place that knows what it is. +const CREDENTIALS: &str = "$HOME/.claude/.credentials.json"; + impl ClaudeUsage { - fn unavailable(&self, error: String) -> UsageSnapshot { + fn snapshot(&self, state: UsageState, windows: Vec) -> UsageSnapshot { UsageSnapshot { provider: self.name().to_string(), - available: false, - windows: Vec::new(), - error: Some(error), + setup: self.setup.clone(), + setup_name: self.setup_name.clone(), + state, + windows, fetched_at: crate::session::now(), } } - fn access_token(&self) -> Result { - let text = std::fs::read_to_string(&self.credentials_path).map_err(|err| { - format!( - "couldn't read {} ({err}) -- is Claude Code logged in on this machine?", - self.credentials_path.display() - ) + /// The machine's stored OAuth token, or which of the two ways of not + /// having one this is. + /// + /// Read through `sh -c` so `$HOME` resolves on the far machine; a path + /// built here would be this machine's home directory, which over ssh + /// is somebody else's. + fn access_token(&self) -> Result { + let launch = Launch::new( + "sh", + vec!["-c".to_string(), format!("cat {CREDENTIALS}")], + None, + ); + let text = self.transport.capture_blocking(&launch).map_err(|err| { + // `cat` failing because the file is absent is a machine with + // nobody logged in, which is a choice; anything else is a + // machine that could not be asked. + let detail = format!("{err:#}"); + if detail.contains("No such file") || detail.contains("not found") { + UsageState::NotLoggedIn + } else { + UsageState::Unreachable { detail } + } })?; serde_json::from_str::(&text) .ok() @@ -96,10 +170,9 @@ impl ClaudeUsage { .as_str() .map(String::from) }) - .ok_or_else(|| { - "credential file has no claudeAiOauth.accessToken -- log in with `claude` once" - .to_string() - }) + // A file that exists but carries no token is the same situation + // as no file: nobody has logged in here yet. + .ok_or(UsageState::NotLoggedIn) } } @@ -111,7 +184,7 @@ impl UsageProvider for ClaudeUsage { fn fetch(&self) -> UsageSnapshot { let token = match self.access_token() { Ok(token) => token, - Err(error) => return self.unavailable(error), + Err(state) => return self.snapshot(state, Vec::new()), }; let text = match ureq::get(USAGE_URL) .header("Authorization", &format!("Bearer {token}")) @@ -123,20 +196,26 @@ impl UsageProvider for ClaudeUsage { Ok(text) => text, Err(err) => { // The error string can embed the URL but never the token. - return self.unavailable(format!("usage endpoint unreachable: {err}")); + return self.snapshot( + UsageState::Failed { + detail: format!("usage endpoint unreachable: {err}"), + }, + Vec::new(), + ); } }; let body: Value = match serde_json::from_str(&text) { Ok(body) => body, - Err(err) => return self.unavailable(format!("usage endpoint sent non-JSON: {err}")), + Err(err) => { + return self.snapshot( + UsageState::Failed { + detail: format!("usage endpoint sent non-JSON: {err}"), + }, + Vec::new(), + ); + } }; - UsageSnapshot { - provider: self.name().to_string(), - available: true, - windows: parse_windows(&body), - error: None, - fetched_at: crate::session::now(), - } + self.snapshot(UsageState::Ok, parse_windows(&body)) } } @@ -184,42 +263,93 @@ fn parse_windows(body: &Value) -> Vec { .collect() } -/// The cache in front of whatever providers exist: at most one real fetch -/// per provider per [`MIN_POLL_INTERVAL`], no matter how often the phone -/// asks. +/// Which paid services a machine can be asked about. +/// +/// Derived from what the setup says it can run, so a machine with no +/// Claude provider is not asked about Claude limits -- it has none, and a +/// row saying so would be a fact about nothing. A second service later +/// adds a branch here and an impl beside [`ClaudeUsage`], not a screen. +fn providers_for(setup: &SetupConfig) -> Vec> { + let mut found: Vec> = Vec::new(); + if setup + .providers + .iter() + .any(|provider| provider.kind == DriverKind::ClaudeCli) + { + found.push(Box::new(ClaudeUsage { + setup: setup.id.clone(), + setup_name: setup.name.clone(), + transport: Transport::for_setup(setup), + })); + } + found +} + +/// The cache in front of whatever machines exist: at most one real fetch +/// per machine per service per [`MIN_POLL_INTERVAL`], no matter how often +/// the phone asks. +/// +/// One machine's numbers for one service, and when they were fetched. +/// +/// Keyed by the machine and the service rather than by position: the set +/// is no longer fixed at startup -- setups are added, renamed and removed +/// from the phone -- and a positional cache would hand one machine's +/// numbers to another the moment the list shifted. +type Cached = HashMap<(String, &'static str), (Instant, UsageSnapshot)>; + +#[derive(Default)] pub struct UsageMonitor { - providers: Vec>, - cache: Mutex>, + cache: Mutex, } impl UsageMonitor { - pub fn new(providers: Vec>) -> Self { - Self { - providers, - cache: Mutex::new(Vec::new()), - } + pub fn new() -> Self { + Self::default() } - /// Blocking -- call via `spawn_blocking`. - pub fn snapshots(&self) -> Vec { - let mut cache = self.cache.lock().unwrap(); - self.providers - .iter() - .enumerate() - .map(|(i, provider)| { - if let Some((fetched, snapshot)) = cache.get(i) + /// One snapshot per machine that offers a paid service, in the order + /// the machines are configured. + /// + /// Blocking -- call via `spawn_blocking`. Takes the setups rather than + /// holding the manager, so this module stays below the session layer + /// rather than reaching up into it. + pub fn snapshots(&self, setups: &[SetupConfig]) -> Vec { + let mut fresh = Vec::new(); + for setup in setups { + for provider in providers_for(setup) { + let key = (setup.id.clone(), provider.name()); + if let Some((fetched, snapshot)) = self.cache.lock().unwrap().get(&key) && fetched.elapsed() < MIN_POLL_INTERVAL { - return snapshot.clone(); + // Cached numbers, but the machine's *name* is read + // fresh: a rename should show immediately rather than + // waiting out the poll interval it has nothing to do + // with. + let mut snapshot = snapshot.clone(); + snapshot.setup_name = setup.name.clone(); + fresh.push(snapshot); + continue; } + // Fetched without the lock held: this makes a network call + // per machine, and holding the cache across them would + // serialise every phone asking for the screen behind the + // slowest ssh connection. let snapshot = provider.fetch(); - match cache.get_mut(i) { - Some(slot) => *slot = (Instant::now(), snapshot.clone()), - None => cache.push((Instant::now(), snapshot.clone())), - } - snapshot - }) - .collect() + self.cache + .lock() + .unwrap() + .insert(key, (Instant::now(), snapshot.clone())); + fresh.push(snapshot); + } + } + // Machines that have gone away should not keep their numbers alive. + let live: std::collections::HashSet<&str> = + setups.iter().map(|setup| setup.id.as_str()).collect(); + self.cache + .lock() + .unwrap() + .retain(|(setup, _), _| live.contains(setup.as_str())); + fresh } } @@ -252,14 +382,61 @@ mod tests { assert_eq!(windows[3].resets_at, None); } + /// A setup naming a machine that cannot be dialled, so nothing here + /// touches the network beyond ssh failing to resolve it. + fn unreachable_setup() -> SetupConfig { + SetupConfig { + id: "far".to_string(), + name: "somewhere else".to_string(), + ssh: Some(crate::config::SshConfig { + address: "no-such-host.invalid".to_string(), + port: None, + identity_file: None, + options: vec!["ConnectTimeout=1".to_string()], + }), + providers: vec![crate::config::ProviderConfig { + name: "claude-cli".to_string(), + kind: DriverKind::ClaudeCli, + command: None, + models: vec![], + }], + } + } + #[test] - fn missing_credentials_degrade_to_unavailable() { + fn a_machine_that_cannot_be_asked_says_so_rather_than_looking_logged_out() { let provider = ClaudeUsage { - credentials_path: PathBuf::from("/nonexistent/creds.json"), + setup: "far".to_string(), + setup_name: "somewhere else".to_string(), + transport: Transport::for_setup(&unreachable_setup()), }; let snapshot = provider.fetch(); - assert!(!snapshot.available); - assert!(snapshot.error.expect("reason").contains("logged in")); + // The distinction the old single `error` string could not make: + // this machine was never reached, which is not the same as a + // machine that answered and has nobody logged in. + assert!( + matches!(snapshot.state, UsageState::Unreachable { .. }), + "{:?}", + snapshot.state + ); + assert_eq!(snapshot.setup, "far"); + assert_eq!(snapshot.setup_name, "somewhere else"); + assert!(snapshot.windows.is_empty()); + } + + #[test] + fn only_machines_that_can_run_claude_are_asked_about_it() { + let mut echo_only = unreachable_setup(); + echo_only.providers = vec![crate::config::ProviderConfig { + name: "echo".to_string(), + kind: DriverKind::Echo, + command: None, + models: vec![], + }]; + // A machine with no Claude on it has no Claude limits, and a row + // reporting on it would be a fact about nothing. + assert!(providers_for(&echo_only).is_empty()); + assert_eq!(providers_for(&unreachable_setup()).len(), 1); } #[test]