From a43ee7ba70b0c50d1526dc2a38d0ef2c03127abb Mon Sep 17 00:00:00 2001 From: "erik.hauan" Date: Thu, 15 Oct 2026 09:32:56 -0700 Subject: [PATCH 1/2] codex-local: fail fast when Ollama is unreachable Preflight GET /api/tags (3s) before delegating to upstream for model-using invocations; bound request/stream retries via seed and -c overrides; log the models-refresh failure once per process. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01RPCzgwU247gMtjybR5v4iR --- codex-rs/Cargo.lock | 2 + codex-rs/local/cli/Cargo.toml | 1 + codex-rs/local/cli/src/lib.rs | 31 ++++++ codex-rs/local/cli/src/main.rs | 20 +++- codex-rs/local/cli/src/overrides.rs | 48 ++++++++- codex-rs/local/cli/src/preflight.rs | 151 ++++++++++++++++++++++++++++ codex-rs/local/cli/src/seed.rs | 2 + codex-rs/local/models/Cargo.toml | 1 + codex-rs/local/models/src/lib.rs | 17 +++- 9 files changed, 262 insertions(+), 11 deletions(-) create mode 100644 codex-rs/local/cli/src/preflight.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 592145cceebd..01d43eee2d45 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3907,6 +3907,7 @@ dependencies = [ "os_info", "owo-colors", "regex-lite", + "reqwest 0.12.28", "serde", "serde_json", "supports-color 3.0.2", @@ -3938,6 +3939,7 @@ dependencies = [ "reqwest 0.12.28", "serde_json", "tokio", + "tracing", "wiremock", ] diff --git a/codex-rs/local/cli/Cargo.toml b/codex-rs/local/cli/Cargo.toml index d8cc7adae5d0..452576c00a9b 100644 --- a/codex-rs/local/cli/Cargo.toml +++ b/codex-rs/local/cli/Cargo.toml @@ -27,6 +27,7 @@ codex-local-models = { path = "../models" } codex-local-resume = { path = "../resume" } dirs = { workspace = true } toml_edit = { workspace = true } +reqwest = { workspace = true } codex-utils-process = { workspace = true } anyhow = { workspace = true } chrono = { workspace = true } diff --git a/codex-rs/local/cli/src/lib.rs b/codex-rs/local/cli/src/lib.rs index 25ef7d8f0314..63f18d5e1f93 100644 --- a/codex-rs/local/cli/src/lib.rs +++ b/codex-rs/local/cli/src/lib.rs @@ -4,6 +4,7 @@ mod homes; mod links; mod overrides; +mod preflight; mod seed; mod sync; mod url; @@ -18,6 +19,8 @@ pub use homes::resolve_homes; pub use links::ensure_home; pub use links::refresh_skills; pub use overrides::apply_overrides; +pub use preflight::check as preflight_check; +pub use preflight::failure_message as preflight_failure_message; pub use seed::seed_config; pub use sync::sync_config; pub use url::resolve_ollama_root; @@ -56,6 +59,8 @@ pub enum Action { Run { argv: Vec, codex_home: PathBuf, + /// Ollama root to probe before delegating; `None` skips the preflight. + preflight_root: Option, }, } @@ -74,13 +79,39 @@ pub fn prepare(args: Vec, env: EnvFn, user_home: Option) -> R sync_config(&homes)?; return Ok(Action::SyncConfig); } + let want_preflight = preflight::needs_preflight(&args, env); let argv = apply_overrides(args, env)?; + let preflight_root = want_preflight.then(|| effective_root(&argv, &homes, &root)); Ok(Action::Run { argv, codex_home: homes.local, + preflight_root, }) } +/// Root that upstream will end up using: the injected base_url override, else the +/// seeded config's base_url, else the env/built-in root. +fn effective_root(argv: &[OsString], homes: &Homes, fallback: &str) -> String { + let key = format!("model_providers.{PROVIDER_ID}.base_url="); + let strip = |u: &str| u.trim_end_matches('/').trim_end_matches("/v1").to_string(); + for w in argv.windows(2) { + if w[0] == "-c" + && let Some(v) = w[1].to_string_lossy().strip_prefix(&key) + { + return strip(v.trim_matches('"')); + } + } + let cfg = std::fs::read_to_string(homes.local.join("config.toml")) + .ok() + .and_then(|t| t.parse::().ok()); + cfg.and_then(|c| { + c["model_providers"][PROVIDER_ID]["base_url"] + .as_str() + .map(strip) + }) + .unwrap_or_else(|| fallback.to_string()) +} + pub(crate) fn env_ollama_root(env: EnvFn) -> Result> { for key in ["CODEX_LOCAL_OLLAMA_URL", "OLLAMA_HOST"] { if let Some(v) = env(key) { diff --git a/codex-rs/local/cli/src/main.rs b/codex-rs/local/cli/src/main.rs index c2d8e29e73fd..9c45fc6dd2cf 100644 --- a/codex-rs/local/cli/src/main.rs +++ b/codex-rs/local/cli/src/main.rs @@ -9,15 +9,29 @@ include!("../../../cli/src/main.rs"); fn main() -> anyhow::Result<()> { codex_build_info::initialize!(); let args: Vec = std::env::args_os().collect(); - let action = codex_local_cli::prepare(args, &codex_local_cli::process_env, codex_local_cli::user_home_dir())?; - let (argv, codex_home) = match action { + let action = codex_local_cli::prepare( + args, + &codex_local_cli::process_env, + codex_local_cli::user_home_dir(), + )?; + let (argv, codex_home, preflight_root) = match action { codex_local_cli::Action::Version(v) => { println!("{v}"); return Ok(()); } codex_local_cli::Action::SyncConfig => return Ok(()), - codex_local_cli::Action::Run { argv, codex_home } => (argv, codex_home), + codex_local_cli::Action::Run { + argv, + codex_home, + preflight_root, + } => (argv, codex_home, preflight_root), }; + if let Some(root) = preflight_root + && let Err(e) = codex_local_cli::preflight_check(&root) + { + eprintln!("{}", codex_local_cli::preflight_failure_message(&root, &e)); + std::process::exit(1); + } // SAFETY: still single-threaded; no runtime or worker threads exist yet. unsafe { std::env::set_var("CODEX_HOME", &codex_home) }; codex_local_models::register(); diff --git a/codex-rs/local/cli/src/overrides.rs b/codex-rs/local/cli/src/overrides.rs index 071527901b1f..eb7bc382f474 100644 --- a/codex-rs/local/cli/src/overrides.rs +++ b/codex-rs/local/cli/src/overrides.rs @@ -8,6 +8,8 @@ use crate::PROVIDER_ID; use crate::env_ollama_root; use crate::resolve_ollama_root; +pub(crate) const MAX_RETRIES: u64 = 2; + /// Strip `--ollama-url` (ours, unknown to upstream) and inject `-c` overrides right /// after argv[0] (root-level, so they reach every subcommand). /// @@ -63,6 +65,14 @@ pub fn apply_overrides(args: Vec, env: EnvFn) -> Result> let v = toml::Value::String(format!("{root}/v1")); push_c(format!("model_providers.{PROVIDER_ID}.base_url={v}")); } + // Bound upstream's retry loops so an outage ends in an error, not an endless + // "Reconnecting..." (config seeding only covers new homes). + push_c(format!( + "model_providers.{PROVIDER_ID}.request_max_retries={MAX_RETRIES}" + )); + push_c(format!( + "model_providers.{PROVIDER_ID}.stream_max_retries={MAX_RETRIES}" + )); out.extend(rest); Ok(out) } @@ -85,11 +95,22 @@ mod tests { .collect() } + const RQ: &str = "model_providers.codex-local.request_max_retries=2"; + const ST: &str = "model_providers.codex-local.stream_max_retries=2"; const URL_KEY: &str = "model_providers.codex-local.base_url"; #[test] fn nothing_set_changes_nothing() { - assert_eq!(run(&["x", "exec", "hi"], &[]), ["x", "exec", "hi"]); + let want = [ + "x", + "-c", + "model_providers.codex-local.request_max_retries=2", + "-c", + "model_providers.codex-local.stream_max_retries=2", + "exec", + "hi", + ]; + assert_eq!(run(&["x", "exec", "hi"], &[]), want); } #[test] @@ -98,7 +119,7 @@ mod tests { let want = |u: &str| format!("{URL_KEY}=\"{u}/v1\""); assert_eq!( run(&["x", "--ollama-url", "http://f:3", "exec"], &env), - ["x", "-c", &want("http://f:3"), "exec"] + ["x", "-c", &want("http://f:3"), "-c", RQ, "-c", ST, "exec"] ); assert_eq!(run(&["x", "exec"], &env)[2], want("http://h1:1")); assert_eq!( @@ -113,11 +134,30 @@ mod tests { assert_eq!(run(&["x", "exec"], &env)[2], "model=\"envm\""); assert_eq!( run(&["x", "-m", "flagm", "exec"], &env), - ["x", "-m", "flagm", "exec"] + [ + "x", + "-c", + "model_providers.codex-local.request_max_retries=2", + "-c", + "model_providers.codex-local.stream_max_retries=2", + "-m", + "flagm", + "exec" + ] ); assert_eq!( run(&["x", "exec", "--", "--ollama-url", "-m"], &[]), - ["x", "exec", "--", "--ollama-url", "-m"] + [ + "x", + "-c", + "model_providers.codex-local.request_max_retries=2", + "-c", + "model_providers.codex-local.stream_max_retries=2", + "exec", + "--", + "--ollama-url", + "-m" + ] ); } } diff --git a/codex-rs/local/cli/src/preflight.rs b/codex-rs/local/cli/src/preflight.rs new file mode 100644 index 000000000000..b1d10f53d61d --- /dev/null +++ b/codex-rs/local/cli/src/preflight.rs @@ -0,0 +1,151 @@ +//! Fail-fast check that Ollama answers before upstream starts its retry loops. + +use std::ffi::OsString; +use std::time::Duration; + +use crate::EnvFn; + +const TIMEOUT: Duration = Duration::from_secs(3); + +/// Subcommands that never talk to the model. +const SKIP: &[&str] = &[ + "sync-config", + "login", + "logout", + "completion", + "debug", + "mcp", + "mcp-server", + "plugin", + "update", + "doctor", + "sandbox", + "execpolicy", + "apply", + "a", + "app-server", + "remote-control", + "app", + "archive", + "unarchive", + "delete", + "migrate-rollouts", + "queue", + "tcp-tunnel", + "agents", + "help", + "features", + "cloud", + "responses-api-proxy", + "stdio-to-uds", +]; + +/// Flags (of the root command) that consume the next argv element. +const VALUE_FLAGS: &[&str] = &[ + "-c", + "--config", + "-m", + "--model", + "-p", + "--profile", + "-C", + "--cd", + "-i", + "--image", + "-s", + "--sandbox", + "-a", + "--ask-for-approval", + "--ollama-url", + "--local-provider", + "--enable", + "--disable", + "--remote", + "--remote-auth-token-env", +]; + +/// Whether this invocation needs a reachable Ollama (exec, review, resume, the TUI, ...). +pub(crate) fn needs_preflight(args: &[OsString], env: EnvFn) -> bool { + if env("CODEX_LOCAL_SKIP_PREFLIGHT").is_some_and(|v| v != "0") { + return false; + } + let mut it = args.iter().skip(1).map(|a| a.to_string_lossy()); + while let Some(a) = it.next() { + match a.as_ref() { + "--" => return true, + "-h" | "--help" | "-V" | "--version" => return false, + f if VALUE_FLAGS.contains(&f) => { + it.next(); + } + f if f.starts_with('-') => {} + sub => return !SKIP.contains(&sub), + } + } + true +} + +/// `GET /api/tags`; the error is a short human string. +pub fn check(root: &str) -> Result<(), String> { + let rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .map_err(|e| e.to_string())?; + rt.block_on(async { + let r = reqwest::Client::new() + .get(format!("{root}/api/tags")) + .timeout(TIMEOUT) + .send() + .await + .map_err(|e| { + if e.is_timeout() { + "timed out after 3s".to_string() + } else if e.is_connect() { + "connection refused or unreachable".to_string() + } else { + e.without_url().to_string() + } + })?; + // Any HTTP answer means something is listening. + drop(r); + Ok(()) + }) +} + +pub fn failure_message(root: &str, err: &str) -> String { + format!( + "codex-local: can't reach Ollama at {root} ({err}). Is it running? \ + Set --ollama-url or CODEX_LOCAL_OLLAMA_URL." + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn a(v: &[&str]) -> Vec { + v.iter().map(OsString::from).collect() + } + + #[test] + fn allowlist_decision() { + let none = |_: &str| None; + assert!(needs_preflight(&a(&["x"]), &none)); + assert!(needs_preflight(&a(&["x", "exec", "hi"]), &none)); + assert!(needs_preflight(&a(&["x", "-m", "login", "exec"]), &none)); + assert!(!needs_preflight(&a(&["x", "login"]), &none)); + assert!(!needs_preflight( + &a(&["x", "-c", "a=b", "mcp", "list"]), + &none + )); + assert!(!needs_preflight(&a(&["x", "--help"]), &none)); + let skip = |k: &str| (k == "CODEX_LOCAL_SKIP_PREFLIGHT").then(|| "1".to_string()); + assert!(!needs_preflight(&a(&["x", "exec"]), &skip)); + } + + #[test] + fn closed_port_fails_fast() { + let t = std::time::Instant::now(); + let e = check("http://127.0.0.1:1").unwrap_err(); + assert!(t.elapsed() < Duration::from_secs(4), "{e}"); + } +} diff --git a/codex-rs/local/cli/src/seed.rs b/codex-rs/local/cli/src/seed.rs index 9cddd921c9df..84dbae718404 100644 --- a/codex-rs/local/cli/src/seed.rs +++ b/codex-rs/local/cli/src/seed.rs @@ -25,6 +25,8 @@ pub(crate) fn seed_toml(root: &str, stock_home: &str) -> Result { ("base_url", s(&format!("{root}/v1"))), ("wire_api", s("responses")), ("requires_openai_auth", Value::Boolean(false)), + ("request_max_retries", Value::Integer(2)), + ("stream_max_retries", Value::Integer(2)), ]); let mut t = Table::new(); t.insert("model".into(), s(DEFAULT_MODEL)); diff --git a/codex-rs/local/models/Cargo.toml b/codex-rs/local/models/Cargo.toml index e9210f3c6c59..0b7e3235cc9e 100644 --- a/codex-rs/local/models/Cargo.toml +++ b/codex-rs/local/models/Cargo.toml @@ -19,6 +19,7 @@ codex-models-manager = { workspace = true } codex-protocol = { workspace = true } reqwest = { workspace = true, features = ["json"] } serde_json = { workspace = true } +tracing = { workspace = true } tokio = { workspace = true, features = ["rt", "macros", "sync"] } [dev-dependencies] diff --git a/codex-rs/local/models/src/lib.rs b/codex-rs/local/models/src/lib.rs index 894274e72361..98c963651b7a 100644 --- a/codex-rs/local/models/src/lib.rs +++ b/codex-rs/local/models/src/lib.rs @@ -43,6 +43,14 @@ pub struct OllamaModelsEndpoint { client: reqwest::Client, } +/// Upstream logs every refresh failure at error level; log ours once per process. +fn warn_once(e: &dyn fmt::Display) { + static LOGGED: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); + if !LOGGED.swap(true, std::sync::atomic::Ordering::Relaxed) { + tracing::warn!("codex-local: Ollama model list unavailable: {e}"); + } +} + impl OllamaModelsEndpoint { /// `root` is the Ollama root (no `/v1`). `freeform_apply_patch` defaults to off. pub fn new(root: impl Into, freeform_apply_patch: bool) -> Self { @@ -81,7 +89,10 @@ impl OllamaModelsEndpoint { .send() .await .and_then(reqwest::Response::error_for_status) - .map_err(|e| invalid(&e))? + .map_err(|e| { + warn_once(&e); + invalid(&e) + })? .json() .await .map_err(|e| invalid(&e))?; @@ -338,9 +349,7 @@ mod tests { }; let manager = factory(&info); let factory = HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault); - let presets = manager - .list_models(RefreshStrategy::Online, factory) - .await; + let presets = manager.list_models(RefreshStrategy::Online, factory).await; let mut ids: Vec<_> = presets.iter().map(|p| p.model.clone()).collect(); ids.sort(); assert_eq!(ids, vec!["a:1", "b:2", "c:3"]); From 5e9e7fa6654b6d3adeba2a1e7d025811a2360e2d Mon Sep 17 00:00:00 2001 From: "erik.hauan" Date: Thu, 15 Oct 2026 10:13:24 -0700 Subject: [PATCH 2/2] codex-local: seed max_threads=6, add CODEX_LOCAL_MAX_AGENTS override Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RPCzgwU247gMtjybR5v4iR --- codex-rs/local/cli/src/overrides.rs | 19 +++++++++++++++++++ codex-rs/local/cli/src/seed.rs | 12 +++++++++++- local/docs/plan-deltas.md | 1 + 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/codex-rs/local/cli/src/overrides.rs b/codex-rs/local/cli/src/overrides.rs index eb7bc382f474..6f6f7b9ffef8 100644 --- a/codex-rs/local/cli/src/overrides.rs +++ b/codex-rs/local/cli/src/overrides.rs @@ -73,6 +73,13 @@ pub fn apply_overrides(args: Vec, env: EnvFn) -> Result> push_c(format!( "model_providers.{PROVIDER_ID}.stream_max_retries={MAX_RETRIES}" )); + // `CODEX_LOCAL_MAX_AGENTS` beats the seeded `[agents] max_threads` in config.toml. + if let Some(raw) = env("CODEX_LOCAL_MAX_AGENTS") { + match raw.trim().parse::() { + Ok(n) if n > 0 => push_c(format!("agents.max_threads={n}")), + _ => eprintln!("codex-local: ignoring invalid CODEX_LOCAL_MAX_AGENTS={raw:?}"), + } + } out.extend(rest); Ok(out) } @@ -113,6 +120,18 @@ mod tests { assert_eq!(run(&["x", "exec", "hi"], &[]), want); } + #[test] + fn max_agents_env_valid_invalid_unset() { + let k = "CODEX_LOCAL_MAX_AGENTS"; + let ma = "agents.max_threads=8"; + assert_eq!(run(&["x", "exec"], &[(k, "8")])[6], ma); + assert_eq!(run(&["x", "exec"], &[(k, "8")]).len(), 8); + for bad in ["0", "-1", "abc", ""] { + assert_eq!(run(&["x", "exec"], &[(k, bad)]).len(), 6); + } + assert_eq!(run(&["x", "exec"], &[]).len(), 6); + } + #[test] fn url_precedence_flag_then_env_chain() { let env = [("CODEX_LOCAL_OLLAMA_URL", "h1:1"), ("OLLAMA_HOST", "h2:2")]; diff --git a/codex-rs/local/cli/src/seed.rs b/codex-rs/local/cli/src/seed.rs index 84dbae718404..98f52075b0f7 100644 --- a/codex-rs/local/cli/src/seed.rs +++ b/codex-rs/local/cli/src/seed.rs @@ -45,7 +45,7 @@ pub(crate) fn seed_toml(root: &str, stock_home: &str) -> Result { ("plugins", Value::Boolean(false)), ]), ); - t.insert("agents".into(), table([("max_threads", Value::Integer(4))])); + t.insert("agents".into(), table([("max_threads", Value::Integer(6))])); t.insert( "shell_environment_policy".into(), table([("set", table([("CODEX_HOME", s(stock_home))]))]), @@ -63,3 +63,13 @@ pub fn seed_config(h: &Homes, root: &str) -> Result { fs::write(&path, body).with_context(|| format!("write {}", path.display()))?; Ok(true) } + +#[cfg(test)] +mod tests { + #[test] + fn seeds_six_agent_threads() { + let body = super::seed_toml("http://127.0.0.1:11434", "/h").unwrap(); + let v: toml::Value = toml::from_str(&body).unwrap(); + assert_eq!(v["agents"]["max_threads"].as_integer(), Some(6)); + } +} diff --git a/local/docs/plan-deltas.md b/local/docs/plan-deltas.md index d5dee726bff2..40f0ab8e06fe 100644 --- a/local/docs/plan-deltas.md +++ b/local/docs/plan-deltas.md @@ -11,4 +11,5 @@ Where the build differs from the approved plan. - **Plugins:** `features.plugins=false` is set to stop the chatgpt.com plugin warm-up. This also disables plugin-bundled skills. - **Session affinity and the mid-history fix:** live in PAIR (Phase 9), not in codex-local. - **Models catalog:** `codex-local-models` builds its manager with `.with_provider_catalog()`, so Ollama's catalog is authoritative and no bundled gpt-* models are merged in; `ensure_loaded` lazily fetches because the initial list is empty. +- **Agent threads:** seeded `[agents] max_threads` is now 6 (existing configs keep 4, never rewritten); override with `CODEX_LOCAL_MAX_AGENTS=` (positive int, beats config.toml, invalid warns and is ignored) or the `agents.max_threads` config key. - **UPSTREAM_SYNC_TOKEN:** the upstream-sync workflow needs this secret (PAT or App token with workflows scope) to push and open the PR whenever the merge touches `.github/workflows/**`; without it that run fails early, otherwise it falls back to GITHUB_TOKEN.