diff --git a/CHANGELOG.md b/CHANGELOG.md index 0968b891..e2111a88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - **A leaked `serve.token` is rotated on Windows, not just Unix.** After SBS-953, a world-readable token was replaced on Unix, but Windows still tightened the DACL and reused the same secret. The ACL is now inspected before tightening; if anyone other than the current user, SYSTEM, or Administrators can read the file, the token is replaced. Closes SBS-1043. ### Fixed +- **`serve --refresh-interval` now controls response caching.** Successful usage and cost responses are cached separately by provider selection for the requested TTL. A zero interval disables caching, and provider failures are retried on the next request. Fixes #273. - **Codex latest-session cost follows transcript time.** Copying or touching an older rollout no longer makes it replace a newer session in local cost summaries. Fixes #271. - **A corrupt `window_geometry.json` no longer wipes other windows' saved positions.** SBS-1024 locked the persist so two surfaces could not drop each other's keys, but a file that would not read or parse still loaded as empty defaults, and the next save replaced the whole file with only the window that just moved. Persist now refuses that write — the same fail-closed rule API keys already use — instead of rewriting siblings to an empty store. Closes SBS-1041. - **Loading settings no longer rewrites another install's start-at-login command.** Every `Settings::load` repaired `HKCU\...\Run\Ceiling` whenever the value was not the quoted path of this process. A portable CLI or a second tree therefore replaced the installed desktop's startup entry, and any extra arguments were stripped. Repair now runs only when this process owns that entry — the same intended exe, or a stale `codexbar-cli.exe` / `codexbar-desktop.exe` sibling in the same directory — and leaves custom arguments and other trees alone. Closes SBS-1053. diff --git a/docs/CLI.md b/docs/CLI.md index 336a957c..50e98a7b 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -131,7 +131,7 @@ codexbar serve [OPTIONS] Useful options: - `--port ` - local HTTP port (default `8080`). -- `--refresh-interval ` - response cache TTL (default `60`). +- `--refresh-interval ` - successful `/usage` and `/cost` response cache TTL (default `60`). Provider selections are cached separately. Set to `0` to disable caching. - `--allow-unauthenticated` - skip the per-user bearer token. Any local process can then read usage. Existing scripts that do not send `Authorization` need this flag. - `--include-identity` - include account email, organization, login method, and raw provider errors. Those fields are omitted by default. diff --git a/rust/src/cli/serve.rs b/rust/src/cli/serve.rs index 04355257..c94865c6 100644 --- a/rust/src/cli/serve.rs +++ b/rust/src/cli/serve.rs @@ -3,7 +3,8 @@ use std::io::{self, Write}; use std::path::{Path, PathBuf}; use std::sync::Arc; -use std::time::Duration; +use std::sync::Mutex; +use std::time::{Duration, Instant}; use clap::Args; use tokio::io::{AsyncReadExt, AsyncWriteExt}; @@ -54,6 +55,42 @@ struct ServeState { include_identity: bool, settings: Settings, accounts: ConfiguredAccounts, + cache: ResponseCache, +} + +#[derive(Clone, Debug, Eq, Hash, PartialEq)] +enum CacheKey { + Usage(Option), + Cost(Option), +} + +struct ResponseCache { + ttl: Duration, + entries: Mutex>, +} + +impl ResponseCache { + fn new(ttl: Duration) -> Self { + Self { + ttl, + entries: Mutex::new(Default::default()), + } + } + + fn get_at(&self, key: &CacheKey, now: Instant) -> Option { + if self.ttl.is_zero() { + return None; + } + let entries = self.entries.lock().unwrap(); + let (created_at, response) = entries.get(key)?; + (now.saturating_duration_since(*created_at) < self.ttl).then(|| response.clone()) + } + + fn insert_at(&self, key: CacheKey, response: String, now: Instant) { + if !self.ttl.is_zero() { + self.entries.lock().unwrap().insert(key, (now, response)); + } + } } pub async fn run(args: ServeArgs) -> anyhow::Result<()> { @@ -77,6 +114,7 @@ pub async fn run(args: ServeArgs) -> anyhow::Result<()> { include_identity: args.include_identity, settings, accounts, + cache: ResponseCache::new(Duration::from_secs(args.refresh_interval)), }); let limiter = Arc::new(Semaphore::new(MAX_CONCURRENT_CONNECTIONS)); // Printed once, immediately before the accept loop. Announcing it right @@ -187,20 +225,36 @@ async fn route_request(request: &ServeRequest, state: &ServeState) -> String { serde_json::json!({ "status": "ok", "version": env!("CARGO_PKG_VERSION") }), ), "/usage" => { - usage_response( + let key = CacheKey::Usage(request.query.get("provider").cloned()); + if let Some(response) = state.cache.get_at(&key, Instant::now()) { + return response; + } + let (response, cacheable) = usage_response_with_cacheability( request.query.get("provider").map(String::as_str), state.include_identity, &state.settings, &state.accounts, ) - .await + .await; + if cacheable { + state.cache.insert_at(key, response.clone(), Instant::now()); + } + response } "/cost" => { - cost_response( + let key = CacheKey::Cost(request.query.get("provider").cloned()); + if let Some(response) = state.cache.get_at(&key, Instant::now()) { + return response; + } + let response = cost_response( request.query.get("provider").map(String::as_str), &state.settings, ) - .await + .await; + if response.starts_with("HTTP/1.1 200") { + state.cache.insert_at(key, response.clone(), Instant::now()); + } + response } _ => json_response(404, serde_json::json!({ "error": "not found" })), } @@ -212,15 +266,29 @@ async fn usage_response( settings: &Settings, accounts: &ConfiguredAccounts, ) -> String { + usage_response_with_cacheability(provider, include_identity, settings, accounts) + .await + .0 +} + +async fn usage_response_with_cacheability( + provider: Option<&str>, + include_identity: bool, + settings: &Settings, + accounts: &ConfiguredAccounts, +) -> (String, bool) { let selection = match ProviderSelection::from_arg(provider) { Ok(selection) => selection, Err(error) => { - return json_response(400, serde_json::json!({ "error": error.to_string() })); + return ( + json_response(400, serde_json::json!({ "error": error.to_string() })), + false, + ); } }; let providers = match selection.resolved_ids(settings) { Ok(providers) => providers, - Err(error) => return no_enabled_providers_response(error.to_string()), + Err(error) => return (no_enabled_providers_response(error.to_string()), false), }; let ctx = FetchContext { source_mode: SourceMode::Auto, @@ -236,6 +304,7 @@ async fn usage_response( }; let mut results = Vec::new(); + let mut cacheable = true; for provider_id in providers { let provider = instantiate_provider(provider_id); match provider @@ -248,13 +317,19 @@ async fn usage_response( "usage": public_usage(result.usage, include_identity), "cost": result.cost, })), - Err(error) => results.push(serde_json::json!({ - "provider": provider_id.cli_name(), - "error": public_error(error.to_string(), include_identity), - })), + Err(error) => { + cacheable = false; + results.push(serde_json::json!({ + "provider": provider_id.cli_name(), + "error": public_error(error.to_string(), include_identity), + })); + } } } - json_response(200, serde_json::Value::Array(results)) + ( + json_response(200, serde_json::Value::Array(results)), + cacheable, + ) } async fn cost_response(provider: Option<&str>, settings: &Settings) -> String { @@ -870,9 +945,61 @@ mod tests { include_identity: false, settings: Settings::default(), accounts: ConfiguredAccounts::default(), + cache: ResponseCache::new(Duration::from_secs(60)), } } + #[test] + fn response_cache_hits_before_ttl_and_expires_at_ttl() { + let cache = ResponseCache::new(Duration::from_secs(10)); + let key = CacheKey::Usage(Some("codex".into())); + let start = Instant::now(); + cache.insert_at(key.clone(), "response".into(), start); + + assert_eq!( + cache.get_at(&key, start + Duration::from_secs(9)), + Some("response".into()) + ); + assert_eq!(cache.get_at(&key, start + Duration::from_secs(10)), None); + } + + #[test] + fn zero_ttl_disables_response_cache() { + let cache = ResponseCache::new(Duration::ZERO); + let key = CacheKey::Cost(None); + let now = Instant::now(); + cache.insert_at(key.clone(), "response".into(), now); + + assert_eq!(cache.get_at(&key, now), None); + assert!(cache.entries.lock().unwrap().is_empty()); + } + + #[test] + fn usage_cost_and_provider_selections_have_independent_cache_entries() { + let cache = ResponseCache::new(Duration::from_secs(60)); + let now = Instant::now(); + cache.insert_at(CacheKey::Usage(None), "all usage".into(), now); + cache.insert_at( + CacheKey::Usage(Some("codex".into())), + "codex usage".into(), + now, + ); + cache.insert_at(CacheKey::Cost(None), "all cost".into(), now); + + assert_eq!( + cache.get_at(&CacheKey::Usage(None), now), + Some("all usage".into()) + ); + assert_eq!( + cache.get_at(&CacheKey::Usage(Some("codex".into())), now), + Some("codex usage".into()) + ); + assert_eq!( + cache.get_at(&CacheKey::Cost(None), now), + Some("all cost".into()) + ); + } + fn settings_with_no_enabled_providers() -> Settings { let mut settings = Settings::default(); settings.enabled_providers.clear();