From 14aadd3063159dad7a64090c4078118f58ad4bb4 Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Mon, 9 Mar 2026 22:31:17 +0000 Subject: [PATCH] refactor: make src/llm/ self-contained for crate extraction (#767) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: make src/llm/ self-contained for crate extraction Move LlmError, LLM config types, and OAuth callback helpers into src/llm/ so the module has zero `use crate::` imports outside of crate::llm. This prepares the module for extraction into a standalone workspace crate. - Move LlmError enum from src/error.rs to src/llm/error.rs - Move LlmConfig, NearAiConfig, RegistryProviderConfig, BedrockConfig, CacheRetention, OAUTH_PLACEHOLDER from src/config/llm.rs to src/llm/config.rs - Move OAuth callback utilities (callback_url, bind_callback_listener, wait_for_callback, landing_html, etc.) from src/cli/oauth_defaults.rs to src/llm/oauth_helpers.rs - Remove session.rs dependency on crate::bootstrap (inline default path) - Add cache_retention field to RegistryProviderConfig, resolve from env in config/llm.rs instead of reading env var in llm/mod.rs - Add Check 6 to scripts/check-boundaries.sh enforcing LLM isolation - All original locations re-export for backward compatibility [skip-regression-check] Co-Authored-By: Claude Opus 4.6 * style: fix formatting Co-Authored-By: Claude Opus 4.6 * fix: address PR #767 review — session path bug and boundary check 1. Fix SessionConfig::default() usage in setup wizard: the fallback at wizard.rs:995 now constructs SessionConfig with the real default_session_path() instead of a relative "session.json", which would write auth tokens to the CWD instead of ~/.ironclaw/. 2. Widen check-boundaries.sh Check 6 to catch all `crate::` references (not just `use crate::` imports). Pre-existing inline references (16 occurrences) are reported as warnings; only new `use crate::` imports are hard violations. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 * fix: address PR #767 review and audit findings in src/llm/ PR review fixes: - Reject wildcard addresses (0.0.0.0, ::) in OAuth callback listener to prevent session token exposure on all interfaces - Fix boundary check comment-stripping that could hide real violations (use sed to strip inline comments before matching) Audit fixes: - Fix UTF-8 byte-index slicing panic in recording.rs hint extraction - Add effective_model_name() delegation to RetryProvider and SmartRoutingProvider for consistency with other wrappers - Add calculate_cost() delegation to CachedProvider and RecordingLlm - Deduplicate retry loop logic in RetryProvider via generic helper - Replace hardcoded /tmp path in recording tests with tempfile Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- scripts/check-boundaries.sh | 50 +++++ src/cli/doctor.rs | 2 +- src/cli/oauth_defaults.rs | 348 +----------------------------- src/cli/status.rs | 2 +- src/config/llm.rs | 169 ++------------- src/config/mod.rs | 8 +- src/error.rs | 59 +---- src/llm/anthropic_oauth.rs | 4 +- src/llm/bedrock.rs | 4 +- src/llm/circuit_breaker.rs | 2 +- src/llm/config.rs | 161 ++++++++++++++ src/llm/error.rs | 43 ++++ src/llm/failover.rs | 2 +- src/llm/mod.rs | 35 ++- src/llm/nearai_chat.rs | 4 +- src/llm/oauth_helpers.rs | 416 ++++++++++++++++++++++++++++++++++++ src/llm/provider.rs | 2 +- src/llm/reasoning.rs | 2 +- src/llm/recording.rs | 41 +++- src/llm/response_cache.rs | 8 +- src/llm/retry.rs | 145 ++++++------- src/llm/rig_adapter.rs | 4 +- src/llm/session.rs | 40 ++-- src/llm/smart_routing.rs | 6 +- src/main.rs | 2 +- src/setup/wizard.rs | 13 +- 26 files changed, 871 insertions(+), 701 deletions(-) create mode 100644 src/llm/config.rs create mode 100644 src/llm/error.rs create mode 100644 src/llm/oauth_helpers.rs diff --git a/scripts/check-boundaries.sh b/scripts/check-boundaries.sh index 1fc072f6..56c85979 100755 --- a/scripts/check-boundaries.sh +++ b/scripts/check-boundaries.sh @@ -209,6 +209,56 @@ else fi echo +# -------------------------------------------------------------------------- +# Check 6: LLM module isolation — no imports from other crate modules +# -------------------------------------------------------------------------- +# src/llm/ should only import from: +# - crate::llm (self-references) +# - external crates (no crate:: prefix) +# It must NOT import from crate::agent, crate::tools, crate::channels, +# crate::safety, crate::config, crate::bootstrap, crate::cli, crate::db, +# crate::workspace, crate::worker, crate::orchestrator, crate::skills, +# crate::hooks, crate::setup, crate::context, etc. +# +# Test-only imports (crate::testing) are excluded since they don't affect +# the runtime dependency graph and won't exist in the extracted crate. +# -------------------------------------------------------------------------- + +echo "--- Check 6: LLM module isolation ---" + +# Match any `crate::` reference (use-imports AND inline paths) that isn't +# crate::llm or crate::testing. Filter out comments. +# We strip inline comments (everything after //) with sed before checking, +# so a line like `real_code(crate::foo); // crate::llm` is still caught. +results=$(grep -rn 'crate::' src/llm/ \ + --include='*.rs' \ + | grep -v '^\s*//' \ + | sed 's|//.*||' \ + | grep 'crate::' \ + | grep -v 'crate::llm' \ + | grep -v 'crate::testing' \ + || true) + +if [ -n "$results" ]; then + count=$(echo "$results" | wc -l | tr -d ' ') + echo "WARNING: src/llm/ has $count reference(s) to modules outside crate::llm:" + echo "$results" + echo + echo "(These are pre-existing; fix them before extracting the crate.)" + echo "(New 'use crate::' imports are hard violations — see below.)" + echo + # Hard-fail only on new `use crate::` imports (easy to avoid in new code). + use_imports=$(echo "$results" | grep '^[^:]*:.*use crate::' || true) + if [ -n "$use_imports" ]; then + echo "HARD VIOLATION: new 'use crate::' imports in src/llm/:" + echo "$use_imports" + violations=$((violations + 1)) + fi +else + echo "OK" +fi +echo + # -------------------------------------------------------------------------- # Summary # -------------------------------------------------------------------------- diff --git a/src/cli/doctor.rs b/src/cli/doctor.rs index 6648a86f..c46f4863 100644 --- a/src/cli/doctor.rs +++ b/src/cli/doctor.rs @@ -107,7 +107,7 @@ enum CheckResult { async fn check_nearai_session() -> CheckResult { // Check if session file exists - let session_path = crate::llm::session::default_session_path(); + let session_path = crate::config::llm::default_session_path(); if !session_path.exists() { // Check for API key mode if std::env::var("NEARAI_API_KEY").is_ok() { diff --git a/src/cli/oauth_defaults.rs b/src/cli/oauth_defaults.rs index e974e3fc..2da14f0a 100644 --- a/src/cli/oauth_defaults.rs +++ b/src/cli/oauth_defaults.rs @@ -24,8 +24,6 @@ use std::time::Duration; use base64::{Engine, engine::general_purpose::URL_SAFE_NO_PAD}; use rand::RngCore; use sha2::{Digest, Sha256}; -use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; -use tokio::net::TcpListener; use tokio::sync::RwLock; use crate::secrets::{CreateSecretParams, SecretsStore}; @@ -64,259 +62,12 @@ pub fn builtin_credentials(secret_name: &str) -> Option { // ── Shared callback server ────────────────────────────────────────────── -/// Fixed port for all OAuth callbacks. -/// -/// Every redirect URI registered with providers must use this port: -/// `http://localhost:9876/callback` (or `/auth/callback` for NEAR AI). -pub const OAUTH_CALLBACK_PORT: u16 = 9876; - -/// Returns the OAuth callback base URL. -/// -/// Checks `IRONCLAW_OAUTH_CALLBACK_URL` env var first (useful for remote/VPS -/// deployments where `127.0.0.1` is unreachable from the user's browser), -/// then falls back to `http://{callback_host()}:{OAUTH_CALLBACK_PORT}`. -pub fn callback_url() -> String { - std::env::var("IRONCLAW_OAUTH_CALLBACK_URL") - .ok() - .filter(|v| !v.is_empty()) - .unwrap_or_else(|| format!("http://{}:{}", callback_host(), OAUTH_CALLBACK_PORT)) -} - -/// Returns the hostname used in OAuth callback URLs. -/// -/// Reads `OAUTH_CALLBACK_HOST` from the environment (default: `127.0.0.1`). -/// -/// **Remote server usage:** set `OAUTH_CALLBACK_HOST` to the network interface -/// address you want to listen on (e.g. the server's LAN IP or `0.0.0.0`). -/// The callback listener will bind to that specific address instead of the -/// loopback interface, so the OAuth redirect can reach an external browser. -/// Note: this transmits the session token over plain HTTP — prefer SSH port -/// forwarding (`ssh -L 9876:127.0.0.1:9876 user@host`) when possible. -/// -/// # Example -/// -/// ```bash -/// export OAUTH_CALLBACK_HOST=203.0.113.10 -/// ironclaw login -/// # Opens: http://203.0.113.10:9876/auth/callback -/// ``` -pub fn callback_host() -> String { - std::env::var("OAUTH_CALLBACK_HOST").unwrap_or_else(|_| "127.0.0.1".to_string()) -} - -/// Returns `true` if `host` is a loopback address that only accepts local connections. -/// -/// Covers `localhost` (case-insensitive), the full `127.0.0.0/8` IPv4 loopback -/// range, and `::1` for IPv6. -pub fn is_loopback_host(host: &str) -> bool { - if host.eq_ignore_ascii_case("localhost") { - return true; - } - host.parse::() - .map(|ip| ip.is_loopback()) - .unwrap_or(false) -} - -/// Error from the OAuth callback listener. -#[derive(Debug, thiserror::Error)] -pub enum OAuthCallbackError { - #[error("Port {0} is in use (another auth flow running?): {1}")] - PortInUse(u16, String), - - #[error("Authorization denied by user")] - Denied, - - #[error("Timed out waiting for authorization")] - Timeout, - - #[error("CSRF state mismatch: expected {expected}, got {actual}")] - StateMismatch { expected: String, actual: String }, - - #[error("IO error: {0}")] - Io(String), -} - -/// Map a `std::io::Error` from a bind attempt to an `OAuthCallbackError`. -fn bind_error(e: std::io::Error) -> OAuthCallbackError { - if e.kind() == std::io::ErrorKind::AddrInUse { - OAuthCallbackError::PortInUse(OAUTH_CALLBACK_PORT, e.to_string()) - } else { - OAuthCallbackError::Io(e.to_string()) - } -} - -/// Bind the OAuth callback listener on the fixed port. -/// -/// When `OAUTH_CALLBACK_HOST` is a loopback address (the default `127.0.0.1`), -/// binds to `127.0.0.1` first and falls back to `[::1]` so local-only auth -/// flows remain restricted to the local machine. -/// -/// When `OAUTH_CALLBACK_HOST` is set to a remote address, binds to that -/// specific address so only connections directed to it are accepted. -pub async fn bind_callback_listener() -> Result { - let host = callback_host(); - - if is_loopback_host(&host) { - // Local mode: prefer IPv4 loopback, fall back to IPv6. - let ipv4_addr = format!("127.0.0.1:{}", OAUTH_CALLBACK_PORT); - match TcpListener::bind(&ipv4_addr).await { - Ok(listener) => return Ok(listener), - Err(e) if e.kind() == std::io::ErrorKind::AddrInUse => { - return Err(OAuthCallbackError::PortInUse( - OAUTH_CALLBACK_PORT, - e.to_string(), - )); - } - Err(_) => { - // IPv4 not available, fall back to IPv6 - } - } - TcpListener::bind(format!("[::1]:{}", OAUTH_CALLBACK_PORT)) - .await - .map_err(bind_error) - } else { - // Remote mode: bind to the specific configured host address only, - // not 0.0.0.0, to limit exposure to the intended interface. - let addr = format!("{}:{}", host, OAUTH_CALLBACK_PORT); - TcpListener::bind(&addr).await.map_err(bind_error) - } -} - -/// Wait for an OAuth callback and extract a query parameter value. -/// -/// Listens for a GET request matching `path_prefix` (e.g., "/callback" or "/auth/callback"), -/// extracts the value of `param_name` (e.g., "code" or "token"), and shows a branded -/// landing page using `display_name` (e.g., "Google", "Notion", "NEAR AI"). -/// -/// When `expected_state` is `Some`, the callback's `state` query parameter is validated -/// against it to prevent CSRF attacks. If the state doesn't match, the callback is -/// rejected with an error page. -/// -/// Times out after 5 minutes. -pub async fn wait_for_callback( - listener: TcpListener, - path_prefix: &str, - param_name: &str, - display_name: &str, - expected_state: Option<&str>, -) -> Result { - let path_prefix = path_prefix.to_string(); - let param_name = param_name.to_string(); - let display_name = display_name.to_string(); - let expected_state = expected_state.map(String::from); - - tokio::time::timeout(Duration::from_secs(300), async move { - loop { - let (mut socket, _) = listener - .accept() - .await - .map_err(|e| OAuthCallbackError::Io(e.to_string()))?; - - let mut reader = BufReader::new(&mut socket); - let mut request_line = String::new(); - reader - .read_line(&mut request_line) - .await - .map_err(|e| OAuthCallbackError::Io(e.to_string()))?; - - if let Some(path) = request_line.split_whitespace().nth(1) - && path.starts_with(&path_prefix) - && let Some(query) = path.split('?').nth(1) - { - // Check for error first - if query.contains("error=") { - let html = landing_html(&display_name, false); - let response = format!( - "HTTP/1.1 400 Bad Request\r\n\ - Content-Type: text/html; charset=utf-8\r\n\ - Connection: close\r\n\ - \r\n\ - {}", - html - ); - let _ = socket.write_all(response.as_bytes()).await; - return Err(OAuthCallbackError::Denied); - } - - // Parse all query params into a map for validation - let params: HashMap<&str, String> = query - .split('&') - .filter_map(|p| { - let mut parts = p.splitn(2, '='); - let key = parts.next()?; - let val = parts.next().unwrap_or(""); - Some(( - key, - urlencoding::decode(val) - .unwrap_or_else(|_| val.into()) - .into_owned(), - )) - }) - .collect(); - - // Validate CSRF state parameter - if let Some(ref expected) = expected_state { - let actual = params.get("state").cloned().unwrap_or_default(); - if actual != *expected { - let html = landing_html(&display_name, false); - let response = format!( - "HTTP/1.1 403 Forbidden\r\n\ - Content-Type: text/html; charset=utf-8\r\n\ - Connection: close\r\n\ - \r\n\ - {}", - html - ); - let _ = socket.write_all(response.as_bytes()).await; - return Err(OAuthCallbackError::StateMismatch { - expected: expected.clone(), - actual, - }); - } - } - - // Look for the target parameter - if let Some(value) = params.get(param_name.as_str()) { - let html = landing_html(&display_name, true); - let response = format!( - "HTTP/1.1 200 OK\r\n\ - Content-Type: text/html; charset=utf-8\r\n\ - Connection: close\r\n\ - \r\n\ - {}", - html - ); - let _ = socket.write_all(response.as_bytes()).await; - let _ = socket.shutdown().await; - - return Ok(value.clone()); - } - } - - // Not the callback we're looking for - let response = "HTTP/1.1 404 Not Found\r\nConnection: close\r\n\r\n"; - let _ = socket.write_all(response.as_bytes()).await; - } - }) - .await - .map_err(|_| OAuthCallbackError::Timeout)? -} - -/// Escape a string for safe interpolation into HTML content. -fn html_escape(s: &str) -> String { - let mut out = String::with_capacity(s.len()); - for c in s.chars() { - match c { - '&' => out.push_str("&"), - '<' => out.push_str("<"), - '>' => out.push_str(">"), - '"' => out.push_str("""), - '\'' => out.push_str("'"), - _ => out.push(c), - } - } - out -} +// Core OAuth callback infrastructure is defined in `crate::llm::oauth_helpers` +// and re-exported here for backward compatibility. +pub use crate::llm::oauth_helpers::{ + OAUTH_CALLBACK_PORT, OAuthCallbackError, bind_callback_listener, callback_host, callback_url, + is_loopback_host, landing_html, wait_for_callback, +}; // ── Shared OAuth flow steps ───────────────────────────────────────── @@ -598,93 +349,6 @@ pub async fn validate_oauth_token( } } -// ── Landing pages ─────────────────────────────────────────────────── - -pub fn landing_html(provider_name: &str, success: bool) -> String { - let safe_name = html_escape(provider_name); - let (icon, heading, subtitle, accent) = if success { - ( - r##"
- -
"##, - format!("{} Connected", safe_name), - "You can close this window and return to your terminal.", - "#22c55e", - ) - } else { - ( - r##"
- -
"##, - "Authorization Failed".to_string(), - "The request was denied. You can close this window and try again.", - "#ef4444", - ) - }; - - format!( - r#" - - - - -IronClaw - {heading} - - - -
- {icon} -

{heading}

-

{subtitle}

-
IronClaw
-
- -"#, - heading = heading, - icon = icon, - subtitle = subtitle, - accent = accent, - ) -} - // ── Gateway callback support ───────────────────────────────────────── /// State for an in-progress OAuth flow, keyed by CSRF `state` parameter. diff --git a/src/cli/status.rs b/src/cli/status.rs index de17a226..6f953b5e 100644 --- a/src/cli/status.rs +++ b/src/cli/status.rs @@ -83,7 +83,7 @@ pub async fn run_status_command() -> anyhow::Result<()> { // Session / Auth print!(" Session: "); - let session_path = crate::llm::session::default_session_path(); + let session_path = crate::config::llm::default_session_path(); if session_path.exists() { println!("found ({})", session_path.display()); } else { diff --git a/src/config/llm.rs b/src/config/llm.rs index 5ce0cb77..08a59866 100644 --- a/src/config/llm.rs +++ b/src/config/llm.rs @@ -5,158 +5,11 @@ use secrecy::SecretString; use crate::bootstrap::ironclaw_base_dir; use crate::config::helpers::{optional_env, parse_optional_env}; use crate::error::ConfigError; +use crate::llm::config::*; use crate::llm::registry::{ProviderProtocol, ProviderRegistry}; use crate::llm::session::SessionConfig; use crate::settings::Settings; -/// Sentinel value used as `api_key` when only an OAuth token is present. -/// -/// When we only have an OAuth token the provider factory in `llm/mod.rs` -/// checks for this value and routes to `AnthropicOAuthProvider`, so this -/// placeholder is never sent over the wire. -pub const OAUTH_PLACEHOLDER: &str = "oauth-placeholder"; - -/// Prompt cache retention policy for Anthropic. -/// -/// Controls Anthropic's automatic prompt caching via a top-level -/// `cache_control` field injected through rig-core's `additional_params`. -/// - `None` — caching disabled, no `cache_control` injected. -/// - `Short` — 5-minute TTL (default), `{"type": "ephemeral"}`, 1.25× write surcharge. -/// - `Long` — 1-hour TTL, `{"type": "ephemeral", "ttl": "1h"}`, 2× write surcharge. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] -pub enum CacheRetention { - /// No prompt caching. - None, - /// 5-minute TTL (default). Write cost: 1.25× base input. - #[default] - Short, - /// 1-hour TTL. Write cost: 2× base input. - Long, -} - -impl std::str::FromStr for CacheRetention { - type Err = String; - - fn from_str(s: &str) -> Result { - match s.to_lowercase().as_str() { - "none" | "off" | "disabled" => Ok(Self::None), - "short" | "5m" | "ephemeral" => Ok(Self::Short), - "long" | "1h" => Ok(Self::Long), - _ => Err(format!( - "invalid cache retention '{}', expected one of: none, short, long", - s - )), - } - } -} - -impl std::fmt::Display for CacheRetention { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::None => write!(f, "none"), - Self::Short => write!(f, "short"), - Self::Long => write!(f, "long"), - } - } -} - -/// Resolved configuration for a registry-based provider. -/// -/// This single struct replaces what used to be five separate config types -/// (`OpenAiDirectConfig`, `AnthropicDirectConfig`, `OllamaConfig`, -/// `OpenAiCompatibleConfig`, `TinfoilConfig`). The `protocol` field -/// determines which rig-core client constructor to use. -#[derive(Debug, Clone)] -pub struct RegistryProviderConfig { - /// Which API protocol to use (determines the rig-core client). - pub protocol: ProviderProtocol, - /// Provider identifier (e.g., "groq", "openai", "tinfoil"). - pub provider_id: String, - /// API key (optional for some providers like Ollama). - /// For Anthropic OAuth, this is set to `OAUTH_PLACEHOLDER`. - pub api_key: Option, - /// Base URL for the API endpoint. - pub base_url: String, - /// Model identifier. - pub model: String, - /// Extra HTTP headers injected into every request. - pub extra_headers: Vec<(String, String)>, - /// OAuth token for providers that support Bearer auth (e.g. Anthropic via `claude login`). - /// When set, the provider factory routes to the OAuth-specific provider implementation. - pub oauth_token: Option, -} - -/// Configuration for AWS Bedrock (native Converse API). -#[derive(Debug, Clone)] -pub struct BedrockConfig { - /// AWS region (e.g. "us-east-1"). - pub region: String, - /// Bedrock model ID (e.g. "anthropic.claude-opus-4-6-v1"). - pub model: String, - /// Cross-region inference prefix: "us", "eu", "apac", "global", or None. - pub cross_region: Option, - /// AWS named profile (for SSO / assume-role workflows). - pub profile: Option, -} - -/// LLM provider configuration. -/// -/// NearAI remains the default backend with its own config struct (session auth). -/// All other providers are resolved through the provider registry, producing -/// a generic `RegistryProviderConfig`. -#[derive(Debug, Clone)] -pub struct LlmConfig { - /// Backend identifier (e.g., "nearai", "openai", "groq", "tinfoil"). - pub backend: String, - /// Session manager configuration (auth URL, token persistence path). - /// Used by the NearAI provider for OAuth/session-token auth. - pub session: SessionConfig, - /// NEAR AI config (always populated, also used for embeddings). - pub nearai: NearAiConfig, - /// Resolved provider config for registry-based providers. - /// `None` when backend is "nearai" or "bedrock". - pub provider: Option, - /// AWS Bedrock config (populated when backend=bedrock, requires --features bedrock). - pub bedrock: Option, - /// HTTP request timeout in seconds for LLM API calls. - /// Default: 120. Increase for local LLMs (Ollama, vLLM, LM Studio) that - /// need more time for prompt evaluation on consumer hardware. - pub request_timeout_secs: u64, -} - -/// NEAR AI configuration. -#[derive(Debug, Clone)] -pub struct NearAiConfig { - /// Model to use (e.g., "claude-3-5-sonnet-20241022", "gpt-4o") - pub model: String, - /// Cheap/fast model for lightweight tasks (heartbeat, routing, evaluation). - pub cheap_model: Option, - /// Base URL for the NEAR AI API. - pub base_url: String, - /// API key for NEAR AI Cloud. - pub api_key: Option, - /// Optional fallback model for failover. - pub fallback_model: Option, - /// Maximum number of retries for transient errors (default: 3). - pub max_retries: u32, - /// Consecutive failures before circuit breaker opens. None = disabled. - pub circuit_breaker_threshold: Option, - /// Seconds the circuit stays open before probing (default: 30). - pub circuit_breaker_recovery_secs: u64, - /// Enable in-memory response caching. Default: false. - pub response_cache_enabled: bool, - /// TTL in seconds for cached responses (default: 3600). - pub response_cache_ttl_secs: u64, - /// Max cached responses before LRU eviction (default: 1000). - pub response_cache_max_entries: usize, - /// Cooldown duration in seconds for failover (default: 300). - pub failover_cooldown_secs: u64, - /// Consecutive failures before failover cooldown (default: 3). - pub failover_cooldown_threshold: u32, - /// Enable cascade mode for smart routing. Default: true. - pub smart_routing_cascade: bool, -} - impl LlmConfig { /// Create a test-friendly config without reading env vars. #[cfg(feature = "libsql")] @@ -459,6 +312,23 @@ impl LlmConfig { api_key }; + // Resolve Anthropic prompt cache retention from env (default: Short). + let cache_retention: CacheRetention = if canonical_id == "anthropic" { + optional_env("ANTHROPIC_CACHE_RETENTION")? + .and_then(|val| match val.parse::() { + Ok(r) => Some(r), + Err(e) => { + tracing::warn!( + "Invalid ANTHROPIC_CACHE_RETENTION: {e}; defaulting to short" + ); + None + } + }) + .unwrap_or_default() + } else { + CacheRetention::default() + }; + Ok(RegistryProviderConfig { protocol, provider_id: canonical_id.to_string(), @@ -467,6 +337,7 @@ impl LlmConfig { model, extra_headers, oauth_token, + cache_retention, }) } } @@ -505,7 +376,7 @@ fn parse_extra_headers(val: &str) -> Result, ConfigError> } /// Get the default session file path (~/.ironclaw/session.json). -fn default_session_path() -> PathBuf { +pub fn default_session_path() -> PathBuf { ironclaw_base_dir().join("session.json") } diff --git a/src/config/mod.rs b/src/config/mod.rs index 9410769c..77b05a13 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -37,9 +37,7 @@ pub use self::database::{DatabaseBackend, DatabaseConfig, SslMode, default_libsq pub use self::embeddings::EmbeddingsConfig; pub use self::heartbeat::HeartbeatConfig; pub use self::hygiene::HygieneConfig; -pub use self::llm::{ - BedrockConfig, CacheRetention, LlmConfig, NearAiConfig, RegistryProviderConfig, -}; +pub use self::llm::default_session_path; pub use self::routines::RoutineConfig; pub use self::safety::SafetyConfig; pub use self::sandbox::{ClaudeCodeConfig, SandboxModeConfig}; @@ -48,6 +46,10 @@ pub use self::skills::SkillsConfig; pub use self::transcription::TranscriptionConfig; pub use self::tunnel::TunnelConfig; pub use self::wasm::WasmConfig; +pub use crate::llm::config::{ + BedrockConfig, CacheRetention, LlmConfig, NearAiConfig, OAUTH_PLACEHOLDER, + RegistryProviderConfig, +}; pub use crate::llm::session::SessionConfig; /// Thread-safe overlay for injected env vars (secrets loaded from DB). diff --git a/src/error.rs b/src/error.rs index d9a01c83..9e57a358 100644 --- a/src/error.rs +++ b/src/error.rs @@ -138,45 +138,8 @@ pub enum ChannelError { HealthCheckFailed { name: String }, } -/// LLM provider errors. -#[derive(Debug, thiserror::Error)] -pub enum LlmError { - #[error("Provider {provider} request failed: {reason}")] - RequestFailed { provider: String, reason: String }, - - #[error("Provider {provider} rate limited, retry after {retry_after:?}")] - RateLimited { - provider: String, - retry_after: Option, - }, - - #[error("Invalid response from {provider}: {reason}")] - InvalidResponse { provider: String, reason: String }, - - #[error("Context length exceeded: {used} tokens used, {limit} allowed")] - ContextLengthExceeded { used: usize, limit: usize }, - - #[error("Model {model} not available on provider {provider}")] - ModelNotAvailable { provider: String, model: String }, - - #[error("Authentication failed for provider {provider}")] - AuthFailed { provider: String }, - - #[error("Session expired for provider {provider}")] - SessionExpired { provider: String }, - - #[error("Session renewal failed for provider {provider}: {reason}")] - SessionRenewalFailed { provider: String, reason: String }, - - #[error("HTTP error: {0}")] - Http(#[from] reqwest::Error), - - #[error("JSON error: {0}")] - Json(#[from] serde_json::Error), - - #[error("IO error: {0}")] - Io(#[from] std::io::Error), -} +// LlmError lives in src/llm/error.rs; re-exported here for backward compatibility. +pub use crate::llm::error::LlmError; /// Tool execution errors. #[derive(Debug, thiserror::Error)] @@ -486,24 +449,6 @@ mod tests { ); } - #[test] - fn llm_error_display() { - let err = LlmError::ContextLengthExceeded { - used: 100_000, - limit: 50_000, - }; - let msg = err.to_string(); - assert!(msg.contains("100000"), "Should mention used tokens: {msg}"); - assert!(msg.contains("50000"), "Should mention limit: {msg}"); - - let err = LlmError::RateLimited { - provider: "openai".to_string(), - retry_after: Some(Duration::from_secs(30)), - }; - let msg = err.to_string(); - assert!(msg.contains("openai"), "Should mention provider: {msg}"); - } - #[test] fn job_error_display() { let err = JobError::MaxJobsExceeded { max: 5 }; diff --git a/src/llm/anthropic_oauth.rs b/src/llm/anthropic_oauth.rs index c79c86f8..104778ad 100644 --- a/src/llm/anthropic_oauth.rs +++ b/src/llm/anthropic_oauth.rs @@ -12,9 +12,9 @@ use rust_decimal::Decimal; use secrecy::{ExposeSecret, SecretString}; use serde::{Deserialize, Serialize}; -use crate::config::RegistryProviderConfig; -use crate::error::LlmError; +use crate::llm::config::RegistryProviderConfig; use crate::llm::costs; +use crate::llm::error::LlmError; use crate::llm::provider::{ ChatMessage, CompletionRequest, CompletionResponse, FinishReason, LlmProvider, Role, ToolCall, ToolCompletionRequest, ToolCompletionResponse, diff --git a/src/llm/bedrock.rs b/src/llm/bedrock.rs index 8c7bf832..ebde19f1 100644 --- a/src/llm/bedrock.rs +++ b/src/llm/bedrock.rs @@ -20,8 +20,8 @@ use aws_sdk_bedrockruntime::types::{ use aws_smithy_types::Document; use rust_decimal::Decimal; -use crate::config::BedrockConfig; -use crate::error::LlmError; +use crate::llm::config::BedrockConfig; +use crate::llm::error::LlmError; use crate::llm::provider::{ CompletionRequest, CompletionResponse, FinishReason, LlmProvider, ModelMetadata, ToolCall, ToolCompletionRequest, ToolCompletionResponse, ToolDefinition, diff --git a/src/llm/circuit_breaker.rs b/src/llm/circuit_breaker.rs index 6b04fac7..db47647e 100644 --- a/src/llm/circuit_breaker.rs +++ b/src/llm/circuit_breaker.rs @@ -19,7 +19,7 @@ use async_trait::async_trait; use rust_decimal::Decimal; use tokio::sync::Mutex; -use crate::error::LlmError; +use crate::llm::error::LlmError; use crate::llm::provider::{ CompletionRequest, CompletionResponse, LlmProvider, ModelMetadata, ToolCompletionRequest, ToolCompletionResponse, diff --git a/src/llm/config.rs b/src/llm/config.rs new file mode 100644 index 00000000..c36280c2 --- /dev/null +++ b/src/llm/config.rs @@ -0,0 +1,161 @@ +//! LLM configuration types. +//! +//! These types define the configuration for LLM providers. They are defined +//! here (in the `llm` module) so that the module is self-contained and can be +//! extracted into a standalone crate. Resolution logic (reading env vars, +//! settings) lives in `crate::config::llm`. + +use secrecy::SecretString; + +use crate::llm::registry::ProviderProtocol; +use crate::llm::session::SessionConfig; + +/// Sentinel value used as `api_key` when only an OAuth token is present. +/// +/// When we only have an OAuth token the provider factory in `llm/mod.rs` +/// checks for this value and routes to `AnthropicOAuthProvider`, so this +/// placeholder is never sent over the wire. +pub const OAUTH_PLACEHOLDER: &str = "oauth-placeholder"; + +/// Prompt cache retention policy for Anthropic. +/// +/// Controls Anthropic's automatic prompt caching via a top-level +/// `cache_control` field injected through rig-core's `additional_params`. +/// - `None` — caching disabled, no `cache_control` injected. +/// - `Short` — 5-minute TTL (default), `{"type": "ephemeral"}`, 1.25× write surcharge. +/// - `Long` — 1-hour TTL, `{"type": "ephemeral", "ttl": "1h"}`, 2× write surcharge. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum CacheRetention { + /// No prompt caching. + None, + /// 5-minute TTL (default). Write cost: 1.25× base input. + #[default] + Short, + /// 1-hour TTL. Write cost: 2× base input. + Long, +} + +impl std::str::FromStr for CacheRetention { + type Err = String; + + fn from_str(s: &str) -> Result { + match s.to_lowercase().as_str() { + "none" | "off" | "disabled" => Ok(Self::None), + "short" | "5m" | "ephemeral" => Ok(Self::Short), + "long" | "1h" => Ok(Self::Long), + _ => Err(format!( + "invalid cache retention '{}', expected one of: none, short, long", + s + )), + } + } +} + +impl std::fmt::Display for CacheRetention { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::None => write!(f, "none"), + Self::Short => write!(f, "short"), + Self::Long => write!(f, "long"), + } + } +} + +/// Resolved configuration for a registry-based provider. +/// +/// This single struct replaces what used to be five separate config types +/// (`OpenAiDirectConfig`, `AnthropicDirectConfig`, `OllamaConfig`, +/// `OpenAiCompatibleConfig`, `TinfoilConfig`). The `protocol` field +/// determines which rig-core client constructor to use. +#[derive(Debug, Clone)] +pub struct RegistryProviderConfig { + /// Which API protocol to use (determines the rig-core client). + pub protocol: ProviderProtocol, + /// Provider identifier (e.g., "groq", "openai", "tinfoil"). + pub provider_id: String, + /// API key (optional for some providers like Ollama). + /// For Anthropic OAuth, this is set to `OAUTH_PLACEHOLDER`. + pub api_key: Option, + /// Base URL for the API endpoint. + pub base_url: String, + /// Model identifier. + pub model: String, + /// Extra HTTP headers injected into every request. + pub extra_headers: Vec<(String, String)>, + /// OAuth token for providers that support Bearer auth (e.g. Anthropic via `claude login`). + /// When set, the provider factory routes to the OAuth-specific provider implementation. + pub oauth_token: Option, + /// Prompt cache retention (Anthropic-specific). + pub cache_retention: CacheRetention, +} + +/// Configuration for AWS Bedrock (native Converse API). +#[derive(Debug, Clone)] +pub struct BedrockConfig { + /// AWS region (e.g. "us-east-1"). + pub region: String, + /// Bedrock model ID (e.g. "anthropic.claude-opus-4-6-v1"). + pub model: String, + /// Cross-region inference prefix: "us", "eu", "apac", "global", or None. + pub cross_region: Option, + /// AWS named profile (for SSO / assume-role workflows). + pub profile: Option, +} + +/// LLM provider configuration. +/// +/// NearAI remains the default backend with its own config struct (session auth). +/// All other providers are resolved through the provider registry, producing +/// a generic `RegistryProviderConfig`. +#[derive(Debug, Clone)] +pub struct LlmConfig { + /// Backend identifier (e.g., "nearai", "openai", "groq", "tinfoil"). + pub backend: String, + /// Session manager configuration (auth URL, token persistence path). + /// Used by the NearAI provider for OAuth/session-token auth. + pub session: SessionConfig, + /// NEAR AI config (always populated, also used for embeddings). + pub nearai: NearAiConfig, + /// Resolved provider config for registry-based providers. + /// `None` when backend is "nearai" or "bedrock". + pub provider: Option, + /// AWS Bedrock config (populated when backend=bedrock, requires --features bedrock). + pub bedrock: Option, + /// HTTP request timeout in seconds for LLM API calls. + /// Default: 120. Increase for local LLMs (Ollama, vLLM, LM Studio) that + /// need more time for prompt evaluation on consumer hardware. + pub request_timeout_secs: u64, +} + +/// NEAR AI configuration. +#[derive(Debug, Clone)] +pub struct NearAiConfig { + /// Model to use (e.g., "claude-3-5-sonnet-20241022", "gpt-4o") + pub model: String, + /// Cheap/fast model for lightweight tasks (heartbeat, routing, evaluation). + pub cheap_model: Option, + /// Base URL for the NEAR AI API. + pub base_url: String, + /// API key for NEAR AI Cloud. + pub api_key: Option, + /// Optional fallback model for failover. + pub fallback_model: Option, + /// Maximum number of retries for transient errors (default: 3). + pub max_retries: u32, + /// Consecutive failures before circuit breaker opens. None = disabled. + pub circuit_breaker_threshold: Option, + /// Seconds the circuit stays open before probing (default: 30). + pub circuit_breaker_recovery_secs: u64, + /// Enable in-memory response caching. Default: false. + pub response_cache_enabled: bool, + /// TTL in seconds for cached responses (default: 3600). + pub response_cache_ttl_secs: u64, + /// Max cached responses before LRU eviction (default: 1000). + pub response_cache_max_entries: usize, + /// Cooldown duration in seconds for failover (default: 300). + pub failover_cooldown_secs: u64, + /// Consecutive failures before failover cooldown (default: 3). + pub failover_cooldown_threshold: u32, + /// Enable cascade mode for smart routing. Default: true. + pub smart_routing_cascade: bool, +} diff --git a/src/llm/error.rs b/src/llm/error.rs new file mode 100644 index 00000000..749e7820 --- /dev/null +++ b/src/llm/error.rs @@ -0,0 +1,43 @@ +//! LLM provider error types. + +use std::time::Duration; + +/// LLM provider errors. +#[derive(Debug, thiserror::Error)] +pub enum LlmError { + #[error("Provider {provider} request failed: {reason}")] + RequestFailed { provider: String, reason: String }, + + #[error("Provider {provider} rate limited, retry after {retry_after:?}")] + RateLimited { + provider: String, + retry_after: Option, + }, + + #[error("Invalid response from {provider}: {reason}")] + InvalidResponse { provider: String, reason: String }, + + #[error("Context length exceeded: {used} tokens used, {limit} allowed")] + ContextLengthExceeded { used: usize, limit: usize }, + + #[error("Model {model} not available on provider {provider}")] + ModelNotAvailable { provider: String, model: String }, + + #[error("Authentication failed for provider {provider}")] + AuthFailed { provider: String }, + + #[error("Session expired for provider {provider}")] + SessionExpired { provider: String }, + + #[error("Session renewal failed for provider {provider}: {reason}")] + SessionRenewalFailed { provider: String, reason: String }, + + #[error("HTTP error: {0}")] + Http(#[from] reqwest::Error), + + #[error("JSON error: {0}")] + Json(#[from] serde_json::Error), + + #[error("IO error: {0}")] + Io(#[from] std::io::Error), +} diff --git a/src/llm/failover.rs b/src/llm/failover.rs index cbc6634e..a23934d1 100644 --- a/src/llm/failover.rs +++ b/src/llm/failover.rs @@ -17,7 +17,7 @@ use std::time::{Duration, Instant}; use async_trait::async_trait; use rust_decimal::Decimal; -use crate::error::LlmError; +use crate::llm::error::LlmError; use crate::llm::provider::{ CompletionRequest, CompletionResponse, LlmProvider, ModelMetadata, ToolCompletionRequest, ToolCompletionResponse, diff --git a/src/llm/mod.rs b/src/llm/mod.rs index 4507b010..ffcc70a6 100644 --- a/src/llm/mod.rs +++ b/src/llm/mod.rs @@ -12,9 +12,12 @@ mod anthropic_oauth; #[cfg(feature = "bedrock")] mod bedrock; pub mod circuit_breaker; +pub mod config; pub mod costs; +pub mod error; pub mod failover; mod nearai_chat; +pub mod oauth_helpers; mod provider; mod reasoning; pub mod recording; @@ -29,6 +32,11 @@ pub mod image_models; pub mod vision_models; pub use circuit_breaker::{CircuitBreakerConfig, CircuitBreakerProvider}; +pub use config::{ + BedrockConfig, CacheRetention, LlmConfig, NearAiConfig, OAUTH_PLACEHOLDER, + RegistryProviderConfig, +}; +pub use error::LlmError; pub use failover::{CooldownConfig, FailoverProvider}; pub use nearai_chat::{ModelInfo, NearAiChatProvider}; pub use provider::{ @@ -53,8 +61,8 @@ use std::sync::Arc; use rig::client::CompletionClient; use secrecy::ExposeSecret; -use crate::config::{LlmConfig, NearAiConfig, RegistryProviderConfig}; -use crate::error::LlmError; +// LlmConfig, NearAiConfig, RegistryProviderConfig, and LlmError are +// re-exported via `pub use` above from config and error submodules. /// Create an LLM provider based on configuration. /// @@ -232,7 +240,7 @@ fn create_anthropic_from_registry( let api_key_is_placeholder = config .api_key .as_ref() - .is_some_and(|k| k.expose_secret() == crate::config::llm::OAUTH_PLACEHOLDER); + .is_some_and(|k| k.expose_secret() == crate::llm::config::OAUTH_PLACEHOLDER); if config.oauth_token.is_some() && (config.api_key.is_none() || api_key_is_placeholder) { tracing::info!( provider = %config.provider_id, @@ -244,8 +252,7 @@ fn create_anthropic_from_registry( return Ok(Arc::new(provider)); } - use crate::config::CacheRetention; - use crate::config::helpers::optional_env; + use crate::llm::config::CacheRetention; use rig::providers::anthropic; let api_key = config @@ -269,21 +276,7 @@ fn create_anthropic_from_registry( reason: format!("Failed to create Anthropic client: {e}"), })?; - // Resolve prompt cache retention from env (default: Short). - // Injects top-level cache_control via additional_params for Anthropic - // automatic caching (the API auto-places the breakpoint at the last - // cacheable block). - let cache_retention: CacheRetention = optional_env("ANTHROPIC_CACHE_RETENTION") - .ok() - .flatten() - .and_then(|val| match val.parse::() { - Ok(r) => Some(r), - Err(e) => { - tracing::warn!("Invalid ANTHROPIC_CACHE_RETENTION: {e}; defaulting to short"); - None - } - }) - .unwrap_or_default(); + let cache_retention = config.cache_retention; let model = client.completion_model(&config.model); @@ -531,7 +524,7 @@ pub async fn build_provider_chain( #[cfg(test)] mod tests { use super::*; - use crate::config::NearAiConfig; + use crate::llm::config::NearAiConfig; fn test_nearai_config() -> NearAiConfig { NearAiConfig { diff --git a/src/llm/nearai_chat.rs b/src/llm/nearai_chat.rs index 659dd956..038ee55f 100644 --- a/src/llm/nearai_chat.rs +++ b/src/llm/nearai_chat.rs @@ -16,8 +16,8 @@ use rust_decimal::prelude::MathematicalOps; use secrecy::ExposeSecret; use serde::{Deserialize, Serialize}; -use crate::config::NearAiConfig; -use crate::error::LlmError; +use crate::llm::config::NearAiConfig; +use crate::llm::error::LlmError; use crate::llm::provider::{ ChatMessage, CompletionRequest, CompletionResponse, FinishReason, LlmProvider, Role, ToolCall, ToolCompletionRequest, ToolCompletionResponse, diff --git a/src/llm/oauth_helpers.rs b/src/llm/oauth_helpers.rs new file mode 100644 index 00000000..551fc04b --- /dev/null +++ b/src/llm/oauth_helpers.rs @@ -0,0 +1,416 @@ +//! OAuth callback infrastructure used by the NEAR AI session login flow. +//! +//! These utilities (callback server, landing pages, hostname detection) were +//! originally in `cli/oauth_defaults.rs` and are moved here so the `llm` +//! module is self-contained. `cli/oauth_defaults` re-exports everything for +//! backward compatibility. + +use std::collections::HashMap; +use std::time::Duration; + +use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; +use tokio::net::TcpListener; + +/// Fixed port for the OAuth callback listener. +pub const OAUTH_CALLBACK_PORT: u16 = 9876; + +/// Error from the OAuth callback listener. +#[derive(Debug, thiserror::Error)] +pub enum OAuthCallbackError { + #[error("Port {0} is in use (another auth flow running?): {1}")] + PortInUse(u16, String), + + #[error("Authorization denied by user")] + Denied, + + #[error("Timed out waiting for authorization")] + Timeout, + + #[error("CSRF state mismatch: expected {expected}, got {actual}")] + StateMismatch { expected: String, actual: String }, + + #[error("IO error: {0}")] + Io(String), +} + +/// Returns the OAuth callback base URL. +/// +/// Checks `IRONCLAW_OAUTH_CALLBACK_URL` env var first (useful for remote/VPS +/// deployments where `127.0.0.1` is unreachable from the user's browser), +/// then falls back to `http://{callback_host()}:{OAUTH_CALLBACK_PORT}`. +pub fn callback_url() -> String { + std::env::var("IRONCLAW_OAUTH_CALLBACK_URL") + .ok() + .filter(|v| !v.is_empty()) + .unwrap_or_else(|| format!("http://{}:{}", callback_host(), OAUTH_CALLBACK_PORT)) +} + +/// Returns the hostname used in OAuth callback URLs. +/// +/// Reads `OAUTH_CALLBACK_HOST` from the environment (default: `127.0.0.1`). +/// +/// **Remote server usage:** set `OAUTH_CALLBACK_HOST` to the specific network +/// interface address you want to listen on (e.g. the server's LAN IP). +/// Wildcard addresses (`0.0.0.0`, `::`) are rejected — use a specific interface +/// IP to limit exposure. The callback listener will bind to that address so the +/// OAuth redirect can reach an external browser. +/// Note: this transmits the session token over plain HTTP — prefer SSH port +/// forwarding (`ssh -L 9876:127.0.0.1:9876 user@host`) when possible. +pub fn callback_host() -> String { + std::env::var("OAUTH_CALLBACK_HOST").unwrap_or_else(|_| "127.0.0.1".to_string()) +} + +/// Returns `true` if `host` is a loopback address that only accepts local connections. +/// +/// Covers `localhost` (case-insensitive), the full `127.0.0.0/8` IPv4 loopback +/// range, and `::1` for IPv6. +pub fn is_loopback_host(host: &str) -> bool { + if host.eq_ignore_ascii_case("localhost") { + return true; + } + host.parse::() + .map(|ip| ip.is_loopback()) + .unwrap_or(false) +} + +/// Returns `true` if `host` is a wildcard/unspecified address (`0.0.0.0` or `::`). +/// +/// Wildcard binds accept connections on all interfaces, which is a security risk +/// for OAuth callbacks that carry session tokens over plain HTTP. +fn is_wildcard_host(host: &str) -> bool { + host.parse::() + .map(|ip| ip.is_unspecified()) + .unwrap_or(false) +} + +/// Map a `std::io::Error` from a bind attempt to an `OAuthCallbackError`. +fn bind_error(e: std::io::Error) -> OAuthCallbackError { + if e.kind() == std::io::ErrorKind::AddrInUse { + OAuthCallbackError::PortInUse(OAUTH_CALLBACK_PORT, e.to_string()) + } else { + OAuthCallbackError::Io(e.to_string()) + } +} + +/// Bind the OAuth callback listener on the fixed port. +/// +/// When `OAUTH_CALLBACK_HOST` is a loopback address (the default `127.0.0.1`), +/// binds to `127.0.0.1` first and falls back to `[::1]` so local-only auth +/// flows remain restricted to the local machine. +/// +/// When `OAUTH_CALLBACK_HOST` is set to a remote address, binds to that +/// specific address so only connections directed to it are accepted. +pub async fn bind_callback_listener() -> Result { + let host = callback_host(); + + if is_wildcard_host(&host) { + return Err(OAuthCallbackError::Io(format!( + "OAUTH_CALLBACK_HOST={host} is a wildcard address — this would accept \ + connections on all interfaces, exposing the session token. \ + Use a specific interface IP (e.g. 192.168.1.x) or SSH port forwarding instead." + ))); + } + + if is_loopback_host(&host) { + // Local mode: prefer IPv4 loopback, fall back to IPv6. + let ipv4_addr = format!("127.0.0.1:{}", OAUTH_CALLBACK_PORT); + match TcpListener::bind(&ipv4_addr).await { + Ok(listener) => return Ok(listener), + Err(e) if e.kind() == std::io::ErrorKind::AddrInUse => { + return Err(OAuthCallbackError::PortInUse( + OAUTH_CALLBACK_PORT, + e.to_string(), + )); + } + Err(_) => { + // IPv4 not available, fall back to IPv6 + } + } + TcpListener::bind(format!("[::1]:{}", OAUTH_CALLBACK_PORT)) + .await + .map_err(bind_error) + } else { + // Remote mode: bind to the specific configured host address only, + // not 0.0.0.0, to limit exposure to the intended interface. + let addr = format!("{}:{}", host, OAUTH_CALLBACK_PORT); + TcpListener::bind(&addr).await.map_err(bind_error) + } +} + +/// Wait for an OAuth callback and extract a query parameter value. +/// +/// Listens for a GET request matching `path_prefix` (e.g., "/callback" or "/auth/callback"), +/// extracts the value of `param_name` (e.g., "code" or "token"), and shows a branded +/// landing page using `display_name` (e.g., "Google", "Notion", "NEAR AI"). +/// +/// When `expected_state` is `Some`, the callback's `state` query parameter is validated +/// against it to prevent CSRF attacks. If the state doesn't match, the callback is +/// rejected with an error page. +/// +/// Times out after 5 minutes. +pub async fn wait_for_callback( + listener: TcpListener, + path_prefix: &str, + param_name: &str, + display_name: &str, + expected_state: Option<&str>, +) -> Result { + let path_prefix = path_prefix.to_string(); + let param_name = param_name.to_string(); + let display_name = display_name.to_string(); + let expected_state = expected_state.map(String::from); + + tokio::time::timeout(Duration::from_secs(300), async move { + loop { + let (mut socket, _) = listener + .accept() + .await + .map_err(|e| OAuthCallbackError::Io(e.to_string()))?; + + let mut reader = BufReader::new(&mut socket); + let mut request_line = String::new(); + reader + .read_line(&mut request_line) + .await + .map_err(|e| OAuthCallbackError::Io(e.to_string()))?; + + if let Some(path) = request_line.split_whitespace().nth(1) + && path.starts_with(&path_prefix) + && let Some(query) = path.split('?').nth(1) + { + // Check for error first + if query.contains("error=") { + let html = landing_html(&display_name, false); + let response = format!( + "HTTP/1.1 400 Bad Request\r\n\ + Content-Type: text/html; charset=utf-8\r\n\ + Connection: close\r\n\ + \r\n\ + {}", + html + ); + let _ = socket.write_all(response.as_bytes()).await; + return Err(OAuthCallbackError::Denied); + } + + // Parse all query params into a map for validation + let params: HashMap<&str, String> = query + .split('&') + .filter_map(|p| { + let mut parts = p.splitn(2, '='); + let key = parts.next()?; + let val = parts.next().unwrap_or(""); + Some(( + key, + urlencoding::decode(val) + .unwrap_or_else(|_| val.into()) + .into_owned(), + )) + }) + .collect(); + + // Validate CSRF state parameter + if let Some(ref expected) = expected_state { + let actual = params.get("state").cloned().unwrap_or_default(); + if actual != *expected { + let html = landing_html(&display_name, false); + let response = format!( + "HTTP/1.1 403 Forbidden\r\n\ + Content-Type: text/html; charset=utf-8\r\n\ + Connection: close\r\n\ + \r\n\ + {}", + html + ); + let _ = socket.write_all(response.as_bytes()).await; + return Err(OAuthCallbackError::StateMismatch { + expected: expected.clone(), + actual, + }); + } + } + + // Look for the target parameter + if let Some(value) = params.get(param_name.as_str()) { + let html = landing_html(&display_name, true); + let response = format!( + "HTTP/1.1 200 OK\r\n\ + Content-Type: text/html; charset=utf-8\r\n\ + Connection: close\r\n\ + \r\n\ + {}", + html + ); + let _ = socket.write_all(response.as_bytes()).await; + let _ = socket.shutdown().await; + + return Ok(value.clone()); + } + } + + // Not the callback we're looking for + let response = "HTTP/1.1 404 Not Found\r\nConnection: close\r\n\r\n"; + let _ = socket.write_all(response.as_bytes()).await; + } + }) + .await + .map_err(|_| OAuthCallbackError::Timeout)? +} + +/// Escape a string for safe interpolation into HTML content. +fn html_escape(s: &str) -> String { + let mut out = String::with_capacity(s.len()); + for c in s.chars() { + match c { + '&' => out.push_str("&"), + '<' => out.push_str("<"), + '>' => out.push_str(">"), + '"' => out.push_str("""), + '\'' => out.push_str("'"), + _ => out.push(c), + } + } + out +} + +/// Generate a branded HTML landing page for the OAuth callback result. +pub fn landing_html(provider_name: &str, success: bool) -> String { + let safe_name = html_escape(provider_name); + let (icon, heading, subtitle, accent) = if success { + ( + r##"
+ +
"##, + format!("{} Connected", safe_name), + "You can close this window and return to your terminal.", + "#22c55e", + ) + } else { + ( + r##"
+ +
"##, + "Authorization Failed".to_string(), + "The request was denied. You can close this window and try again.", + "#ef4444", + ) + }; + + format!( + r#" + + + + +IronClaw - {heading} + + + +
+ {icon} +

{heading}

+

{subtitle}

+
IronClaw
+
+ +"#, + heading = heading, + icon = icon, + subtitle = subtitle, + accent = accent, + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn loopback_detection() { + assert!(is_loopback_host("127.0.0.1")); + assert!(is_loopback_host("127.0.0.2")); // full 127.0.0.0/8 range + assert!(is_loopback_host("::1")); + assert!(is_loopback_host("localhost")); + assert!(is_loopback_host("LOCALHOST")); + assert!(!is_loopback_host("0.0.0.0")); + assert!(!is_loopback_host("192.168.1.1")); + assert!(!is_loopback_host("::")); + assert!(!is_loopback_host("example.com")); + } + + #[test] + fn wildcard_detection() { + assert!(is_wildcard_host("0.0.0.0")); + assert!(is_wildcard_host("::")); + assert!(!is_wildcard_host("127.0.0.1")); + assert!(!is_wildcard_host("192.168.1.1")); + assert!(!is_wildcard_host("::1")); + assert!(!is_wildcard_host("localhost")); + } + + #[tokio::test] + async fn bind_rejects_wildcard_ipv4() { + // SAFETY: test is single-threaded; env var is restored immediately after. + unsafe { std::env::set_var("OAUTH_CALLBACK_HOST", "0.0.0.0") }; + let result = bind_callback_listener().await; + unsafe { std::env::remove_var("OAUTH_CALLBACK_HOST") }; + assert!(result.is_err()); + let err = result.unwrap_err().to_string(); + assert!( + err.contains("wildcard"), + "error should mention wildcard: {err}" + ); + } + + #[tokio::test] + async fn bind_rejects_wildcard_ipv6() { + // SAFETY: test is single-threaded; env var is restored immediately after. + unsafe { std::env::set_var("OAUTH_CALLBACK_HOST", "::") }; + let result = bind_callback_listener().await; + unsafe { std::env::remove_var("OAUTH_CALLBACK_HOST") }; + assert!(result.is_err()); + let err = result.unwrap_err().to_string(); + assert!( + err.contains("wildcard"), + "error should mention wildcard: {err}" + ); + } +} diff --git a/src/llm/provider.rs b/src/llm/provider.rs index c650f30b..40ab8100 100644 --- a/src/llm/provider.rs +++ b/src/llm/provider.rs @@ -4,7 +4,7 @@ use async_trait::async_trait; use rust_decimal::Decimal; use serde::{Deserialize, Serialize}; -use crate::error::LlmError; +use crate::llm::error::LlmError; /// Role in a conversation. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] diff --git a/src/llm/reasoning.rs b/src/llm/reasoning.rs index c2d2462c..4b20865a 100644 --- a/src/llm/reasoning.rs +++ b/src/llm/reasoning.rs @@ -5,7 +5,7 @@ use std::sync::{Arc, LazyLock}; use regex::Regex; use serde::{Deserialize, Serialize}; -use crate::error::LlmError; +use crate::llm::error::LlmError; use crate::llm::{ ChatMessage, CompletionRequest, LlmProvider, Role, ToolCall, ToolCompletionRequest, diff --git a/src/llm/recording.rs b/src/llm/recording.rs index 6f53278b..77f7b257 100644 --- a/src/llm/recording.rs +++ b/src/llm/recording.rs @@ -21,7 +21,7 @@ use rust_decimal::Decimal; use serde::{Deserialize, Serialize}; use tokio::sync::Mutex; -use crate::error::LlmError; +use crate::llm::error::LlmError; use crate::llm::provider::{ ChatMessage, CompletionRequest, CompletionResponse, LlmProvider, ModelMetadata, Role, ToolCompletionRequest, ToolCompletionResponse, @@ -437,7 +437,11 @@ impl RecordingLlm { .find(|m| m.role == Role::User) .map(|msg| { let hint_text = if msg.content.len() > 80 { - msg.content[..80].to_string() + let mut end = 80; + while end > 0 && !msg.content.is_char_boundary(end) { + end -= 1; + } + msg.content[..end].to_string() } else { msg.content.clone() }; @@ -546,6 +550,10 @@ impl LlmProvider for RecordingLlm { fn set_model(&self, model: &str) -> Result<(), LlmError> { self.inner.set_model(model) } + + fn calculate_cost(&self, input_tokens: u32, output_tokens: u32) -> Decimal { + self.inner.calculate_cost(input_tokens, output_tokens) + } } #[cfg(test)] @@ -554,9 +562,10 @@ mod tests { use crate::testing::StubLlm; fn make_recorder(stub: Arc) -> RecordingLlm { + let dir = tempfile::tempdir().expect("failed to create temp dir"); RecordingLlm::new( stub, - PathBuf::from("/tmp/test_recording.json"), + dir.path().join("test_recording.json"), "test-recording".to_string(), ) } @@ -900,6 +909,32 @@ mod tests { assert_eq!(parsed.steps[2].expected_tool_results.len(), 1); } + #[tokio::test] + async fn request_hint_handles_multibyte_utf8() { + let stub = Arc::new(StubLlm::new("response")); + let recorder = make_recorder(stub); + + // Create a string where byte index 80 falls inside a multi-byte char. + // Each CJK character is 3 bytes; 26 chars × 3 bytes = 78, then "ab" = 80 bytes, + // but let's use 27 CJK chars (81 bytes) so truncation must respect the boundary. + let long_cjk = "你".repeat(27); // 81 bytes, > 80 + assert!(long_cjk.len() > 80); + + let request = CompletionRequest::new(vec![ + ChatMessage::system("sys"), + ChatMessage::user(&long_cjk), + ]); + recorder.complete(request).await.unwrap(); + + let steps = recorder.steps.lock().await; + let text_step = &steps[1]; + let hint = text_step.request_hint.as_ref().unwrap(); + let hint_text = hint.last_user_message_contains.as_deref().unwrap(); + // Must be valid UTF-8 and not longer than 80 bytes + assert!(hint_text.len() <= 80); + assert!(hint_text.is_ascii() || hint_text.chars().count() > 0); + } + #[test] fn backward_compatible_with_old_format() { // Old format without memory_snapshot, http_exchanges, expected_tool_results diff --git a/src/llm/response_cache.rs b/src/llm/response_cache.rs index 26caf885..b1e7aa8e 100644 --- a/src/llm/response_cache.rs +++ b/src/llm/response_cache.rs @@ -25,7 +25,7 @@ use async_trait::async_trait; use rust_decimal::Decimal; use sha2::{Digest, Sha256}; -use crate::error::LlmError; +use crate::llm::error::LlmError; use crate::llm::provider::{ CompletionRequest, CompletionResponse, LlmProvider, ModelMetadata, ToolCompletionRequest, ToolCompletionResponse, @@ -298,6 +298,10 @@ impl LlmProvider for CachedProvider { // hit again rather than wasted. Natural TTL / LRU eviction cleans them up. self.inner.set_model(model) } + + fn calculate_cost(&self, input_tokens: u32, output_tokens: u32) -> Decimal { + self.inner.calculate_cost(input_tokens, output_tokens) + } } #[cfg(test)] @@ -307,7 +311,7 @@ mod tests { use rust_decimal::Decimal; use tracing_test::traced_test; - use crate::error::LlmError; + use crate::llm::error::LlmError; use crate::llm::provider::{ ChatMessage, CompletionResponse, FinishReason, ToolCompletionRequest, ToolCompletionResponse, diff --git a/src/llm/retry.rs b/src/llm/retry.rs index 1a68cb8b..b85f4f15 100644 --- a/src/llm/retry.rs +++ b/src/llm/retry.rs @@ -5,6 +5,7 @@ //! - `retry_backoff_delay()` — exponential backoff with jitter //! - `RetryProvider` — decorator that wraps any `LlmProvider` with automatic retries +use std::future::Future; use std::sync::Arc; use std::time::Duration; @@ -12,7 +13,7 @@ use async_trait::async_trait; use rand::Rng; use rust_decimal::Decimal; -use crate::error::LlmError; +use crate::llm::error::LlmError; use crate::llm::provider::{ CompletionRequest, CompletionResponse, LlmProvider, ModelMetadata, ToolCompletionRequest, ToolCompletionResponse, @@ -97,6 +98,50 @@ impl RetryProvider { pub fn new(inner: Arc, config: RetryConfig) -> Self { Self { inner, config } } + + async fn retry_loop(&self, mut op: F, label: &str) -> Result + where + F: FnMut() -> Fut, + Fut: Future>, + { + let mut last_error: Option = None; + + for attempt in 0..=self.config.max_retries { + match op().await { + Ok(resp) => return Ok(resp), + Err(err) => { + if !is_retryable(&err) || attempt == self.config.max_retries { + return Err(err); + } + + let delay = match &err { + LlmError::RateLimited { + retry_after: Some(duration), + .. + } => *duration, + _ => retry_backoff_delay(attempt), + }; + + tracing::warn!( + provider = %self.inner.model_name(), + attempt = attempt + 1, + max_retries = self.config.max_retries, + delay_ms = delay.as_millis() as u64, + error = %err, + "Retrying after transient error{label}" + ); + + last_error = Some(err); + tokio::time::sleep(delay).await; + } + } + } + + Err(last_error.unwrap_or_else(|| LlmError::RequestFailed { + provider: self.inner.model_name().to_string(), + reason: "retry loop exited unexpectedly".to_string(), + })) + } } #[async_trait] @@ -118,88 +163,30 @@ impl LlmProvider for RetryProvider { } async fn complete(&self, request: CompletionRequest) -> Result { - let mut last_error: Option = None; - - for attempt in 0..=self.config.max_retries { - let req = request.clone(); - match self.inner.complete(req).await { - Ok(resp) => return Ok(resp), - Err(err) => { - if !is_retryable(&err) || attempt == self.config.max_retries { - return Err(err); - } - - let delay = match &err { - LlmError::RateLimited { - retry_after: Some(duration), - .. - } => *duration, - _ => retry_backoff_delay(attempt), - }; - - tracing::warn!( - provider = %self.inner.model_name(), - attempt = attempt + 1, - max_retries = self.config.max_retries, - delay_ms = delay.as_millis() as u64, - error = %err, - "Retrying after transient error" - ); - - last_error = Some(err); - tokio::time::sleep(delay).await; - } - } - } - - Err(last_error.unwrap_or_else(|| LlmError::RequestFailed { - provider: self.inner.model_name().to_string(), - reason: "retry loop exited unexpectedly".to_string(), - })) + let inner = &self.inner; + self.retry_loop( + || { + let req = request.clone(); + async move { inner.complete(req).await } + }, + "", + ) + .await } async fn complete_with_tools( &self, request: ToolCompletionRequest, ) -> Result { - let mut last_error: Option = None; - - for attempt in 0..=self.config.max_retries { - let req = request.clone(); - match self.inner.complete_with_tools(req).await { - Ok(resp) => return Ok(resp), - Err(err) => { - if !is_retryable(&err) || attempt == self.config.max_retries { - return Err(err); - } - - let delay = match &err { - LlmError::RateLimited { - retry_after: Some(duration), - .. - } => *duration, - _ => retry_backoff_delay(attempt), - }; - - tracing::warn!( - provider = %self.inner.model_name(), - attempt = attempt + 1, - max_retries = self.config.max_retries, - delay_ms = delay.as_millis() as u64, - error = %err, - "Retrying after transient error (tools)" - ); - - last_error = Some(err); - tokio::time::sleep(delay).await; - } - } - } - - Err(last_error.unwrap_or_else(|| LlmError::RequestFailed { - provider: self.inner.model_name().to_string(), - reason: "retry loop exited unexpectedly".to_string(), - })) + let inner = &self.inner; + self.retry_loop( + || { + let req = request.clone(); + async move { inner.complete_with_tools(req).await } + }, + " (tools)", + ) + .await } async fn list_models(&self) -> Result, LlmError> { @@ -210,6 +197,10 @@ impl LlmProvider for RetryProvider { self.inner.model_metadata().await } + fn effective_model_name(&self, requested_model: Option<&str>) -> String { + self.inner.effective_model_name(requested_model) + } + fn active_model_name(&self) -> String { self.inner.active_model_name() } diff --git a/src/llm/rig_adapter.rs b/src/llm/rig_adapter.rs index d72373e6..87a0b65c 100644 --- a/src/llm/rig_adapter.rs +++ b/src/llm/rig_adapter.rs @@ -3,7 +3,7 @@ //! This lets us use any rig-core provider (OpenAI, Anthropic, Ollama, etc.) as an //! `Arc` without changing any of the agent, reasoning, or tool code. -use crate::config::CacheRetention; +use crate::llm::config::CacheRetention; use async_trait::async_trait; use rig::OneOrMany; use rig::completion::{ @@ -23,8 +23,8 @@ use serde_json::Value as JsonValue; use std::collections::HashSet; -use crate::error::LlmError; use crate::llm::costs; +use crate::llm::error::LlmError; use crate::llm::provider::{ ChatMessage, CompletionRequest, CompletionResponse, FinishReason, LlmProvider, ToolCall as IronToolCall, ToolCompletionRequest, ToolCompletionResponse, diff --git a/src/llm/session.rs b/src/llm/session.rs index dd61e629..3d1c4785 100644 --- a/src/llm/session.rs +++ b/src/llm/session.rs @@ -7,8 +7,7 @@ use std::path::PathBuf; use std::sync::Arc; -use crate::bootstrap::ironclaw_base_dir; -use crate::cli::oauth_defaults::OAUTH_CALLBACK_PORT; +use crate::llm::oauth_helpers::OAUTH_CALLBACK_PORT; use chrono::{DateTime, Utc}; use reqwest::Client; @@ -16,7 +15,7 @@ use secrecy::SecretString; use serde::{Deserialize, Serialize}; use tokio::sync::{Mutex, RwLock}; -use crate::error::LlmError; +use crate::llm::error::LlmError; /// Session data persisted to disk. #[derive(Debug, Clone, Serialize, Deserialize)] @@ -40,16 +39,13 @@ impl Default for SessionConfig { fn default() -> Self { Self { auth_base_url: "https://private.near.ai".to_string(), - session_path: default_session_path(), + // Real path is set by LlmConfig::resolve() via config/llm.rs. + // This default is only used in tests. + session_path: PathBuf::from("session.json"), } } } -/// Get the default session file path (~/.ironclaw/session.json). -pub fn default_session_path() -> PathBuf { - ironclaw_base_dir().join("session.json") -} - /// Manages NEAR AI session tokens with persistence and automatic renewal. pub struct SessionManager { config: SessionConfig, @@ -236,10 +232,10 @@ impl SessionManager { /// 2. Set NEARAI_API_KEY env var and save to bootstrap .env /// 3. No session token saved (different auth model) async fn initiate_login(&self) -> Result<(), LlmError> { - use crate::cli::oauth_defaults; + use crate::llm::oauth_helpers; - let cb_url = oauth_defaults::callback_url(); - let host = oauth_defaults::callback_host(); + let cb_url = oauth_helpers::callback_url(); + let host = oauth_helpers::callback_host(); // Show auth provider menu BEFORE binding the listener println!(); @@ -292,7 +288,7 @@ impl SessionManager { // Warn about plain-HTTP token transmission only for OAuth paths (1, 2) // where the callback URL actually carries the session token. - if !oauth_defaults::is_loopback_host(&host) { + if !oauth_helpers::is_loopback_host(&host) { println!(); println!("Warning: OAuth callback is using plain HTTP to a remote host ({host})."); println!(" The session token will be transmitted unencrypted."); @@ -303,12 +299,12 @@ impl SessionManager { } // OAuth paths: bind the callback listener now - let listener = oauth_defaults::bind_callback_listener() - .await - .map_err(|e| LlmError::SessionRenewalFailed { + let listener = oauth_helpers::bind_callback_listener().await.map_err(|e| { + LlmError::SessionRenewalFailed { provider: "nearai".to_string(), reason: e.to_string(), - })?; + } + })?; let (auth_provider, auth_url) = match choice.trim() { "2" => { @@ -348,7 +344,7 @@ impl SessionManager { // The NEAR AI API redirects to: {frontend_callback}/auth/callback?token=X&... let session_token = - oauth_defaults::wait_for_callback(listener, "/auth/callback", "token", "NEAR AI", None) + oauth_helpers::wait_for_callback(listener, "/auth/callback", "token", "NEAR AI", None) .await .map_err(|e| LlmError::SessionRenewalFailed { provider: "nearai".to_string(), @@ -690,13 +686,6 @@ mod tests { assert!(matches!(result, Err(LlmError::AuthFailed { .. }))); } - #[test] - fn test_default_session_path() { - let path = default_session_path(); - assert!(path.ends_with("session.json")); - assert!(path.to_string_lossy().contains(".ironclaw")); - } - #[test] fn test_session_data_serde_roundtrip_with_auth_provider() { let original = SessionData { @@ -737,7 +726,6 @@ mod tests { let config = SessionConfig::default(); assert_eq!(config.auth_base_url, "https://private.near.ai"); assert!(config.session_path.ends_with("session.json")); - assert!(config.session_path.to_string_lossy().contains(".ironclaw")); } #[tokio::test] diff --git a/src/llm/smart_routing.rs b/src/llm/smart_routing.rs index 9f4a4141..6d413723 100644 --- a/src/llm/smart_routing.rs +++ b/src/llm/smart_routing.rs @@ -24,7 +24,7 @@ use async_trait::async_trait; use regex::Regex; use rust_decimal::Decimal; -use crate::error::LlmError; +use crate::llm::error::LlmError; use crate::llm::provider::{ CompletionRequest, CompletionResponse, LlmProvider, ModelMetadata, Role, ToolCompletionRequest, ToolCompletionResponse, @@ -946,6 +946,10 @@ impl LlmProvider for SmartRoutingProvider { self.primary.model_metadata().await } + fn effective_model_name(&self, requested_model: Option<&str>) -> String { + self.primary.effective_model_name(requested_model) + } + fn active_model_name(&self) -> String { self.primary.active_model_name() } diff --git a/src/main.rs b/src/main.rs index 83d4d782..f594bf41 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1152,7 +1152,7 @@ fn check_onboard_needed() -> Option<&'static str> { } if std::env::var("NEARAI_API_KEY").is_err() { - let session_path = ironclaw::llm::session::default_session_path(); + let session_path = ironclaw::config::default_session_path(); if !session_path.exists() { return Some("First run"); } diff --git a/src/setup/wizard.rs b/src/setup/wizard.rs index 04dc09c9..38a5d46d 100644 --- a/src/setup/wizard.rs +++ b/src/setup/wizard.rs @@ -22,7 +22,7 @@ use crate::bootstrap::ironclaw_base_dir; use crate::channels::wasm::{ ChannelCapabilitiesFile, available_channel_names, install_bundled_channel, }; -use crate::config::llm::OAUTH_PLACEHOLDER; +use crate::config::OAUTH_PLACEHOLDER; use crate::llm::{SessionConfig, SessionManager}; use crate::secrets::{SecretsCrypto, SecretsStore}; use crate::settings::{KeySource, Settings}; @@ -992,7 +992,10 @@ impl SetupWizard { let session = if let Some(ref s) = self.session_manager { Arc::clone(s) } else { - let config = SessionConfig::default(); + let config = SessionConfig { + session_path: crate::config::llm::default_session_path(), + ..SessionConfig::default() + }; Arc::new(SessionManager::new(config)) }; @@ -1588,7 +1591,7 @@ impl SetupWizard { backend: "nearai".to_string(), session: crate::llm::session::SessionConfig { auth_base_url, - session_path: crate::llm::session::default_session_path(), + session_path: crate::config::llm::default_session_path(), }, nearai: crate::config::NearAiConfig { model: "dummy".to_string(), @@ -2552,7 +2555,7 @@ impl SetupWizard { /// Best-effort: silently ignores errors (no DB connection yet, no /// session file, etc.). async fn persist_session_to_db(&self) { - let session_path = crate::llm::session::default_session_path(); + let session_path = crate::config::llm::default_session_path(); let data = match std::fs::read_to_string(&session_path) { Ok(d) if !d.trim().is_empty() => d, _ => return, @@ -2861,7 +2864,7 @@ async fn fetch_anthropic_models(cached_key: Option<&str>) -> Vec<(String, String let api_key = cached_key .map(String::from) .or_else(|| std::env::var("ANTHROPIC_API_KEY").ok()) - .filter(|k| !k.is_empty() && k != crate::config::llm::OAUTH_PLACEHOLDER); + .filter(|k| !k.is_empty() && k != crate::config::OAUTH_PLACEHOLDER); // Fall back to OAuth token if no API key let oauth_token = if api_key.is_none() {