diff --git a/docs/CLI.md b/docs/CLI.md index 6e0e15ea59e..53e66defd8d 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -81,6 +81,7 @@ Exit codes (stable intent): `0` ok, `1` below threshold, usage errors for bad ar ```powershell codexbar serve --port 8080 +codexbar serve --request-timeout 20 # answer 504 after 20 s; 0 (default) waits # Non-loopback binds need a dashboard token and --allow-plain-http (cleartext bearer). # Prefer: $env:CODEXBAR_DASHBOARD_TOKEN = '...' ``` @@ -91,6 +92,7 @@ Typical endpoints: `/health`, `/usage`, `/cost` (and dashboard snapshot routes w ```powershell codexbar config providers +codexbar config providers --json # [{provider, displayName, enabled, defaultEnabled}] codexbar config enable -p cursor codexbar config disable -p cursor printf '%s' $env:OPENROUTER_API_KEY | codexbar config set-api-key -p openrouter --stdin diff --git a/rust/src/cli/config.rs b/rust/src/cli/config.rs index b49be7ae2cd..9b3349ba5b5 100755 --- a/rust/src/cli/config.rs +++ b/rust/src/cli/config.rs @@ -30,7 +30,17 @@ pub enum ConfigCommand { show_secrets: bool, }, /// List providers and enabled state - Providers, + Providers { + /// Output format: text or json + #[arg(short, long, default_value = "text", value_parser = ["text", "json"])] + format: String, + /// Shorthand for --format json + #[arg(long)] + json: bool, + /// Pretty-print JSON output + #[arg(long)] + pretty: bool, + }, /// Enable a provider Enable { /// Provider CLI name or alias @@ -67,7 +77,11 @@ pub async fn run(args: ConfigArgs) -> anyhow::Result<()> { format, show_secrets, } => dump_config(&format, show_secrets).await, - ConfigCommand::Providers => list_providers().await, + ConfigCommand::Providers { + format, + json, + pretty, + } => list_providers(json || format == "json", pretty).await, ConfigCommand::Enable { provider } => set_provider_enabled(&provider, true).await, ConfigCommand::Disable { provider } => set_provider_enabled(&provider, false).await, ConfigCommand::SetApiKey { @@ -323,30 +337,63 @@ fn redact_secrets_value(value: serde_json::Value) -> serde_json::Value { } /// List provider enabled state. -async fn list_providers() -> anyhow::Result<()> { - let settings = Settings::load(); - for id in ProviderId::all() { - let state = if settings.is_provider_enabled(*id) { - "enabled" - } else { - "disabled" - }; - let default_marker = if instantiate_provider(*id).metadata().default_enabled { - " default" - } else { - "" - }; - println!( - "{}: {}{} ({})", - id.cli_name(), - state, - default_marker, - id.display_name() - ); +async fn list_providers(as_json: bool, pretty: bool) -> anyhow::Result<()> { + let statuses = provider_statuses(&Settings::load()); + if as_json { + println!("{}", provider_statuses_json(&statuses, pretty)?); + } else { + for status in &statuses { + println!("{}", status.text_line()); + } } Ok(()) } +/// One provider of the inventory, in the shape of CodexBar's own +/// `config providers --json` (upstream `ConfigProviderStatusResult`). +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize)] +#[serde(rename_all = "camelCase")] +struct ProviderStatus { + default_enabled: bool, + display_name: String, + enabled: bool, + provider: String, +} + +impl ProviderStatus { + /// The text form: `codex: enabled default (Codex)`. + fn text_line(&self) -> String { + let state = if self.enabled { "enabled" } else { "disabled" }; + let default_marker = if self.default_enabled { " default" } else { "" }; + format!( + "{}: {}{} ({})", + self.provider, state, default_marker, self.display_name + ) + } +} + +/// Every provider the CLI knows with its switched-on state; the text and the +/// JSON listing print these same rows. +fn provider_statuses(settings: &Settings) -> Vec { + ProviderId::all() + .iter() + .map(|id| ProviderStatus { + default_enabled: instantiate_provider(*id).metadata().default_enabled, + display_name: id.display_name().to_string(), + enabled: settings.is_provider_enabled(*id), + provider: id.cli_name().to_string(), + }) + .collect() +} + +fn provider_statuses_json(statuses: &[ProviderStatus], pretty: bool) -> anyhow::Result { + Ok(if pretty { + serde_json::to_string_pretty(statuses)? + } else { + serde_json::to_string(statuses)? + }) +} + /// Enable or disable a provider by CLI name. async fn set_provider_enabled(provider: &str, enabled: bool) -> anyhow::Result<()> { let id = parse_provider(provider)?; @@ -478,12 +525,103 @@ async fn show_paths() -> anyhow::Result<()> { #[cfg(test)] mod tests { - use super::{ConfigFileError, read_json_config, sanitize_settings_for_dump}; + use super::{ + ConfigArgs, ConfigCommand, ConfigFileError, ProviderStatus, provider_statuses, + provider_statuses_json, read_json_config, sanitize_settings_for_dump, + }; + use crate::core::ProviderId; #[cfg(windows)] use crate::secure_file; use crate::settings::ManualCookies; + use crate::settings::Settings; + use clap::Parser; use serde_json::json; + /// The first entries of CodexBar 0.63.0's (macOS) `config providers + /// --json`, as VibeTV recorded and parses them. + const MAC_PROVIDER_ROWS: &str = r#"[ + { + "defaultEnabled": true, + "displayName": "Codex", + "enabled": true, + "provider": "codex" + }, + { + "defaultEnabled": false, + "displayName": "Claude", + "enabled": true, + "provider": "claude" + } +]"#; + + #[test] + fn provider_inventory_json_has_the_mac_cli_shape() { + let rows = vec![ + ProviderStatus { + default_enabled: true, + display_name: "Codex".into(), + enabled: true, + provider: "codex".into(), + }, + ProviderStatus { + default_enabled: false, + display_name: "Claude".into(), + enabled: true, + provider: "claude".into(), + }, + ]; + // Pretty output is the recording byte for byte: names, order, types. + assert_eq!( + provider_statuses_json(&rows, true).unwrap(), + MAC_PROVIDER_ROWS + ); + let compact = provider_statuses_json(&rows, false).unwrap(); + assert!(!compact.contains('\n')); + let recorded: serde_json::Value = serde_json::from_str(MAC_PROVIDER_ROWS).unwrap(); + assert_eq!( + serde_json::from_str::(&compact).unwrap(), + recorded + ); + } + + #[test] + fn provider_inventory_lists_every_provider_with_its_state() { + let mut settings = Settings::default(); + settings.enabled_providers.clear(); + settings.enable_provider(ProviderId::Claude); + let rows = provider_statuses(&settings); + assert_eq!(rows.len(), ProviderId::all().len()); + let row = |name: &str| rows.iter().find(|row| row.provider == name).unwrap(); + assert!(row("claude").enabled); + assert_eq!(row("claude").display_name, "Claude"); + assert!(!row("codex").enabled); + // The text listing is unchanged and comes from the same rows. + assert!(row("claude").text_line().starts_with("claude: enabled")); + assert!(row("claude").text_line().ends_with("(Claude)")); + assert!(row("codex").text_line().starts_with("codex: disabled")); + } + + #[test] + fn providers_command_accepts_the_mac_cli_json_options() { + let parse = |args: &[&str]| { + let mut argv = vec!["config", "providers"]; + argv.extend_from_slice(args); + match ConfigArgs::try_parse_from(argv).map(|parsed| parsed.command) { + Ok(ConfigCommand::Providers { + format, + json, + pretty, + }) => Some((json || format == "json", pretty)), + _ => None, + } + }; + assert_eq!(parse(&[]), Some((false, false))); + assert_eq!(parse(&["--json"]), Some((true, false))); + assert_eq!(parse(&["--format", "json", "--pretty"]), Some((true, true))); + assert_eq!(parse(&["--format", "text"]), Some((false, false))); + assert_eq!(parse(&["--format", "yaml"]), None); + } + #[cfg(windows)] #[test] fn validates_dpapi_protected_manual_cookies_without_plaintext() { diff --git a/rust/src/cli/serve/mod.rs b/rust/src/cli/serve/mod.rs index 511d631ee22..c54d313ab94 100644 --- a/rust/src/cli/serve/mod.rs +++ b/rust/src/cli/serve/mod.rs @@ -87,6 +87,19 @@ pub struct ServeArgs { /// request (upstream 0.50.1 #2960). #[arg(long, value_parser = ["redacted", "full"])] pub identity: Option, + + /// Total per-request deadline in seconds; 0 disables (default: 0) + /// + /// CodexBar's own `serve` option. A request still unanswered after this + /// long gets `504 Gateway Timeout`; its work runs on to the end. Unlike + /// CodexBar, whose default is 30, the default here stays "no deadline", + /// which is how this server behaved before the option existed. + #[arg( + long = "request-timeout", + value_name = "SECONDS", + default_value_t = 0.0 + )] + pub request_timeout: f64, } /// Normalized serve bind configuration after startup validation. @@ -102,6 +115,9 @@ struct ServeConfig { /// Dashboard snapshot identity mode. `None` means follow the app's /// `hide_personal_info` setting per request (upstream 0.50.1 #2960). identity: Option, + /// Deadline for answering one request (`--request-timeout`); `None` + /// waits for the answer however long it takes. + request_timeout: Option, /// Dashboard state (coordinator + producer). Always `Some` from `run`; /// `None` only in pure-transport tests, where dashboard routes answer 503. dashboard: Option, @@ -198,10 +214,29 @@ fn validate_serve_args(args: &ServeArgs) -> anyhow::Result { token_digest: token.as_ref().map(|t| sha256_digest(t.as_bytes())), head_read_budget: HEAD_READ_TIMEOUT, identity, + request_timeout: request_timeout(args.request_timeout)?, dashboard: None, }) } +/// Upstream's longest request deadline: one day. +const MAX_REQUEST_TIMEOUT_SECS: f64 = 86_400.0; + +/// `--request-timeout` as CodexBar reads it: seconds, fractions allowed, +/// capped at a day; zero means no deadline; negative or non-finite is an +/// error. +fn request_timeout(seconds: f64) -> anyhow::Result> { + if !seconds.is_finite() || seconds < 0.0 { + anyhow::bail!("--request-timeout must be zero or greater."); + } + if seconds == 0.0 { + return Ok(None); + } + Ok(Some(Duration::from_secs_f64( + seconds.min(MAX_REQUEST_TIMEOUT_SECS), + ))) +} + fn resolve_dashboard_token(cli_token: Option<&str>) -> anyhow::Result> { // Prefer env (already merged by clap env=) but still reject empty/whitespace. if let Some(raw) = cli_token { @@ -326,7 +361,17 @@ async fn handle_client(mut stream: TcpStream, config: &ServeConfig) -> anyhow::R }; let request = String::from_utf8_lossy(&head); let response = match parse_request(&request) { - Ok(request) => route_request(&request, config).await, + Ok(request) => match config.request_timeout { + None => route_request(&request, config).await, + Some(deadline) => { + let config = config.clone(); + answer_within( + deadline, + async move { route_request(&request, &config).await }, + ) + .await + } + }, Err(status) => json_response(status, serde_json::json!({ "error": "bad request" })), }; stream.write_all(response.as_bytes()).await?; @@ -334,6 +379,22 @@ async fn handle_client(mut stream: TcpStream, config: &ServeConfig) -> anyhow::R Ok(()) } +/// Answer of `work`, or `504 Gateway Timeout` once `deadline` has passed. +/// +/// The work runs as its own task and is not stopped at the deadline: a slow +/// dashboard snapshot still finishes and lands in the coordinator's cache for +/// the next request, as upstream's `--request-timeout` describes. +async fn answer_within(deadline: Duration, work: F) -> String +where + F: std::future::Future + Send + 'static, +{ + match tokio::time::timeout(deadline, tokio::spawn(work)).await { + Ok(Ok(response)) => response, + Ok(Err(_)) => json_response(500, serde_json::json!({ "error": "internal error" })), + Err(_) => json_response(504, serde_json::json!({ "error": "request timed out" })), + } +} + /// Read one complete request head under one overall deadline. /// /// Upstream 0.48.0 #2684 (`CLILocalHTTPServer.readRequest`): the deadline is a @@ -695,6 +756,7 @@ fn http_response( 405 => "Method Not Allowed", 500 => "Internal Server Error", 503 => "Service Unavailable", + 504 => "Gateway Timeout", _ => "Internal Server Error", }; let extra = extra_headers diff --git a/rust/src/cli/serve/tests.rs b/rust/src/cli/serve/tests.rs index b7c3e384ad5..6ac77cd6646 100644 --- a/rust/src/cli/serve/tests.rs +++ b/rust/src/cli/serve/tests.rs @@ -63,6 +63,7 @@ fn validate_serve_args_accepts_loopback_without_token() { dashboard_token: None, allow_plain_http: false, identity: Some("redacted".into()), + request_timeout: 0.0, }) .unwrap(); assert_eq!(config.host, "127.0.0.1"); @@ -78,6 +79,7 @@ fn validate_serve_args_rejects_lan_without_token() { dashboard_token: None, allow_plain_http: true, identity: Some("redacted".into()), + request_timeout: 0.0, }) .unwrap_err() .to_string(); @@ -93,6 +95,7 @@ fn validate_serve_args_rejects_lan_without_allow_plain_http() { dashboard_token: Some("tok".into()), allow_plain_http: false, identity: Some("redacted".into()), + request_timeout: 0.0, }) .unwrap_err() .to_string(); @@ -155,6 +158,7 @@ fn head_test_config(budget: Duration, token: Option<&str>) -> ServeConfig { token_digest: token.map(|t| sha256_digest(t.as_bytes())), head_read_budget: budget, identity: Some(DashboardIdentity::Redacted), + request_timeout: None, dashboard: None, } } @@ -869,3 +873,109 @@ async fn request_roundtrip_dashboard(request: &[u8], config: ServeConfig) -> Str server_task.await.unwrap().unwrap(); String::from_utf8_lossy(&response).into_owned() } + +#[derive(clap::Parser)] +struct ServeCli { + #[command(flatten)] + args: ServeArgs, +} + +fn parse_serve(args: &[&str]) -> Result { + use clap::Parser; + let mut argv = vec!["serve"]; + argv.extend_from_slice(args); + ServeCli::try_parse_from(argv).map(|cli| cli.args) +} + +#[test] +fn request_timeout_option_is_accepted_as_the_mac_cli_writes_it() { + // What VibeTV passes on the Mac: no deadline. + let args = parse_serve(&["--request-timeout", "0"]).unwrap(); + assert!( + validate_serve_args(&args) + .unwrap() + .request_timeout + .is_none() + ); + + let args = parse_serve(&["--request-timeout", "2.5"]).unwrap(); + assert_eq!( + validate_serve_args(&args).unwrap().request_timeout, + Some(Duration::from_millis(2_500)) + ); + + // Without the option the server waits as it always did. + let args = parse_serve(&[]).unwrap(); + assert!( + validate_serve_args(&args) + .unwrap() + .request_timeout + .is_none() + ); +} + +#[test] +fn request_timeout_is_capped_at_a_day_and_rejects_nonsense() { + assert_eq!( + request_timeout(1e9).unwrap(), + Some(Duration::from_secs(86_400)) + ); + for bad in [-1.0, f64::NAN, f64::INFINITY] { + let err = request_timeout(bad).unwrap_err().to_string(); + assert!(err.contains("--request-timeout"), "{bad}: {err}"); + } + assert!(parse_serve(&["--request-timeout", "soon"]).is_err()); +} + +#[tokio::test] +async fn request_past_its_deadline_gets_504_and_its_work_still_finishes() { + let (finished_tx, finished_rx) = tokio::sync::oneshot::channel(); + let response = answer_within(Duration::from_millis(20), async move { + tokio::time::sleep(Duration::from_millis(150)).await; + // The receiver may be gone in other tests; here it is awaited below. + let _sent = finished_tx.send(()); + json_response(200, serde_json::json!({ "status": "ok" })) + }) + .await; + assert!( + response.starts_with("HTTP/1.1 504 Gateway Timeout\r\n"), + "{response}" + ); + assert!(response.ends_with(r#"{"error":"request timed out"}"#)); + // The slow work was not cancelled by the deadline. + tokio::time::timeout(Duration::from_secs(2), finished_rx) + .await + .expect("work must run on past the deadline") + .unwrap(); +} + +#[tokio::test] +async fn request_inside_its_deadline_is_answered_unchanged() { + let response = answer_within(Duration::from_secs(5), async { + json_response(200, serde_json::json!({ "status": "ok" })) + }) + .await; + assert!(response.starts_with("HTTP/1.1 200 OK\r\n"), "{response}"); +} + +#[tokio::test] +async fn snapshot_slower_than_the_deadline_answers_504_then_serves_the_finished_build() { + let state = dashboard::DashboardState::stub( + stub_build(DashboardIdMode::Redacted, false, Duration::from_millis(300)), + 3600, + Some(DashboardIdMode::Redacted), + ); + let mut config = dashboard_test_config(None, Some(state)); + config.request_timeout = Some(Duration::from_millis(40)); + let request: &[u8] = b"GET /dashboard/v1/snapshot HTTP/1.1\r\nHost: 127.0.0.1\r\n\r\n"; + + let late = request_roundtrip_dashboard(request, config.clone()).await; + assert!(late.starts_with("HTTP/1.1 504 Gateway Timeout"), "{late}"); + + // The build ran on past the deadline and was kept: the next request is + // answered from it at once, inside the same short deadline. + tokio::time::sleep(Duration::from_millis(600)).await; + let healed = request_roundtrip_dashboard(request, config).await; + assert!(healed.starts_with("HTTP/1.1 200"), "{healed}"); + assert!(healed.contains("\"schemaVersion\": 1")); +}