From d8716efb8eed135a2525fc57efddd4d7ac226fa3 Mon Sep 17 00:00:00 2001 From: TheArchitectit Date: Wed, 17 Jun 2026 12:32:00 -0500 Subject: [PATCH] fix(team): sub-agents honor resolved model + fall through on model-not-found Team agents failed with 'Model not found' 404s because of two precedence bugs in the sub-agent provider chain: 1. providerFallbacks.primary silently overrode the agent's resolved model. new_with_fallback_config took fallback_config.primary() as THE primary, ignoring the model the caller (Agent tool / subagentModel) actually picked. So every spawned agent used the configured (dead) primary instead of its own model. Fix: the caller's model is the primary; providerFallbacks (primary + fallbacks) are recovery entries appended after it, deduped. 2. A 404 'model not found' is not retryable, so the chain died on the first (dead) model instead of advancing to the next configured fallback. Fix: add fallback_chain_eligible() that also treats 404/400 with a 'not found' / 'model ... unavailable' body as chain-eligible, so a dead primary advances to kimi/qwen/etc. Added status_code()/response_body() accessors to ApiError for the detection. 3. resolve_agent_model defaulted to hard-coded claude-opus-4-6 even when the session was connected to a custom endpoint (e.g. custom/openclaw). Fix: fall back to the config's top-level `model` before DEFAULT_AGENT_MODEL, so sub-agents inherit the session's actual provider by default. Verified with fallback_chain_tests (404 model-not-found eligible; 401 auth not eligible; 429 still eligible). 3 tests pass. Co-Authored-By: Claude Fable 5 --- rust/crates/api/src/error.rs | 43 ++++++++++ rust/crates/tools/src/lib.rs | 155 ++++++++++++++++++++++++++++++++--- 2 files changed, 188 insertions(+), 10 deletions(-) diff --git a/rust/crates/api/src/error.rs b/rust/crates/api/src/error.rs index e8ec73a4..20e06547 100644 --- a/rust/crates/api/src/error.rs +++ b/rust/crates/api/src/error.rs @@ -180,6 +180,49 @@ impl ApiError { } } + /// HTTP status code for an `Api` error, if any. Used by callers (e.g. the + /// sub-agent provider chain) to decide whether to advance to the next + /// configured model — a 404 "model not found" on the primary should fall + /// through to the next fallback rather than killing the whole chain. + #[must_use] + pub fn status_code(&self) -> Option { + match self { + Self::Api { status, .. } => Some(*status), + Self::RetriesExhausted { last_error, .. } => last_error.status_code(), + Self::MissingCredentials { .. } + | Self::ContextWindowExceeded { .. } + | Self::ExpiredOAuthToken + | Self::Auth(_) + | Self::InvalidApiKeyEnv(_) + | Self::Http(_) + | Self::Io(_) + | Self::Json { .. } + | Self::InvalidSseFrame(_) + | Self::BackoffOverflow { .. } + | Self::RequestBodySizeExceeded { .. } => None, + } + } + + /// Response body (best-effort) for an `Api` error, if any. + #[must_use] + pub fn response_body(&self) -> Option<&str> { + match self { + Self::Api { body, .. } => Some(body.as_str()), + Self::RetriesExhausted { last_error, .. } => last_error.response_body(), + Self::MissingCredentials { .. } + | Self::ContextWindowExceeded { .. } + | Self::ExpiredOAuthToken + | Self::Auth(_) + | Self::InvalidApiKeyEnv(_) + | Self::Http(_) + | Self::Io(_) + | Self::Json { .. } + | Self::InvalidSseFrame(_) + | Self::BackoffOverflow { .. } + | Self::RequestBodySizeExceeded { .. } => None, + } + } + #[must_use] pub fn safe_failure_class(&self) -> &'static str { match self { diff --git a/rust/crates/tools/src/lib.rs b/rust/crates/tools/src/lib.rs index f65919df..2918d0ab 100644 --- a/rust/crates/tools/src/lib.rs +++ b/rust/crates/tools/src/lib.rs @@ -5630,12 +5630,26 @@ fn resolve_agent_model(model: Option<&str>) -> String { if let Some(m) = model.map(str::trim).filter(|m| !m.is_empty()) { return m.to_string(); } + // subagentModel (if set) pins the sub-agent model. if let Some(fast) = load_subagent_model_from_config() { return fast; } + // Otherwise default to the session's configured `model` so sub-agents use + // the same provider the user is actually connected with, rather than the + // hard-coded DEFAULT_AGENT_MODEL (which may not exist on a custom endpoint). + if let Some(session_model) = load_main_model_from_config() { + return session_model; + } DEFAULT_AGENT_MODEL.to_string() } +/// Read the top-level `model` from merged config as a sub-agent default. +fn load_main_model_from_config() -> Option { + let cwd = std::env::current_dir().ok()?; + let config = ConfigLoader::default_for(&cwd).load().ok()?; + config.model().map(str::to_string) +} + /// Read the `subagentModel` setting from merged config so the Agent tool /// honors it even when the caller didn't pass an explicit model. fn load_subagent_model_from_config() -> Option { @@ -6601,18 +6615,33 @@ impl ProviderRuntimeClient { allowed_tools: BTreeSet, fallback_config: &ProviderFallbackConfig, ) -> Result { - let primary_model = fallback_config.primary().map_or(model, str::to_string); - let primary = build_provider_entry(&primary_model)?; + // The caller's resolved `model` is the primary. `providerFallbacks` + // (primary + fallbacks) are recovery entries tried in order after the + // primary fails with a retryable error. Previously the config's + // `providerFallbacks.primary` silently overrode the resolved model, + // which made sub-agents ignore `subagentModel` / the Agent tool's + // explicit model — e.g. spawning agents that always used a dead + // configured primary instead of the model the caller picked. + let primary = build_provider_entry(&model)?; let mut chain = vec![primary]; - for fallback_model in fallback_config.fallbacks() { - match build_provider_entry(fallback_model) { + let mut seen: BTreeSet = std::iter::once(model.clone()).collect(); + let mut push_fallback = |chain: &mut Vec, m: &str| { + if seen.contains(m) { + return; + } + seen.insert(m.to_string()); + match build_provider_entry(m) { Ok(entry) => chain.push(entry), Err(error) => { - eprintln!( - "warning: skipping unavailable fallback provider {fallback_model}: {error}" - ); + eprintln!("warning: skipping unavailable fallback provider {m}: {error}") } } + }; + if let Some(config_primary) = fallback_config.primary() { + push_fallback(&mut chain, config_primary); + } + for fallback_model in fallback_config.fallbacks() { + push_fallback(&mut chain, fallback_model); } Ok(Self { runtime: tokio::runtime::Runtime::new().map_err(|error| error.to_string())?, @@ -6640,6 +6669,111 @@ fn load_provider_fallback_config() -> ProviderFallbackConfig { }) } +/// Whether an API error should advance the provider chain to the next model. +/// In addition to the standard retryable statuses (429/500/502/503/504), a +/// 404 whose body indicates the model itself is unknown ("model not found") +/// is chain-eligible: a dead primary model shouldn't abort the whole team — +/// fall through to the next configured fallback. +fn fallback_chain_eligible(error: &ApiError) -> bool { + if error.is_retryable() { + return true; + } + is_model_not_found(error) +} + +/// True when the error body indicates the requested model doesn't exist on +/// the provider (HTTP 404 + a "not found" / "model" message). +fn is_model_not_found(error: &ApiError) -> bool { + let Some(status) = error.status_code() else { + return false; + }; + if status != reqwest::StatusCode::NOT_FOUND && status != reqwest::StatusCode::BAD_REQUEST { + return false; + } + let body = error + .response_body() + .unwrap_or_default() + .to_ascii_lowercase(); + let message = error.to_string().to_ascii_lowercase(); + body.contains("not found") + || body.contains("does not exist") + || message.contains("not found") + || (body.contains("model") && body.contains("unavailable")) +} + +/// Short human-readable reason for a chain fallthrough, for the log line. +fn fallback_reason(error: &ApiError) -> &'static str { + if error.is_retryable() { + "retryable error" + } else if is_model_not_found(error) { + "model not found" + } else { + "error" + } +} + +#[cfg(test)] +mod fallback_chain_tests { + use super::{fallback_chain_eligible, is_model_not_found}; + use api::ApiError; + + fn model_not_found_404() -> ApiError { + ApiError::Api { + status: reqwest::StatusCode::NOT_FOUND, + error_type: Some("invalid_request_error".to_string()), + message: Some("Model 'openai/glm-5.1-fast' not found.".to_string()), + request_id: None, + body: "{\"detail\":\"Model 'openai/glm-5.1-fast' not found. Use GET /v1/models to see available models.\"}".to_string(), + retryable: false, + suggested_action: None, + retry_after: None, + } + } + + fn auth_error() -> ApiError { + ApiError::Api { + status: reqwest::StatusCode::UNAUTHORIZED, + error_type: None, + message: None, + request_id: None, + body: "unauthorized".to_string(), + retryable: false, + suggested_action: None, + retry_after: None, + } + } + + fn rate_limited() -> ApiError { + ApiError::Api { + status: reqwest::StatusCode::TOO_MANY_REQUESTS, + error_type: None, + message: None, + request_id: None, + body: "slow down".to_string(), + retryable: true, + suggested_action: None, + retry_after: None, + } + } + + #[test] + fn model_not_found_404_is_chain_eligible() { + assert!(is_model_not_found(&model_not_found_404())); + assert!(fallback_chain_eligible(&model_not_found_404())); + } + + #[test] + fn auth_error_is_not_chain_eligible() { + assert!(!is_model_not_found(&auth_error())); + assert!(!fallback_chain_eligible(&auth_error())); + } + + #[test] + fn retryable_error_remains_chain_eligible() { + assert!(fallback_chain_eligible(&rate_limited())); + } +} + impl ApiClient for ProviderRuntimeClient { fn stream(&mut self, request: ApiRequest) -> Result, RuntimeError> { let tools = tool_specs_for_allowed_tools(Some(&self.allowed_tools)) @@ -6673,10 +6807,11 @@ impl ApiClient for ProviderRuntimeClient { let attempt = runtime.block_on(stream_with_provider(&entry.client, &message_request)); match attempt { Ok(events) => return Ok(events), - Err(error) if error.is_retryable() && index + 1 < chain.len() => { + Err(error) if fallback_chain_eligible(&error) && index + 1 < chain.len() => { eprintln!( - "provider {} failed with retryable error, falling back: {error}", - entry.model + "provider {} failed ({}, falling back to next in chain): {error}", + entry.model, + fallback_reason(&error) ); last_error = Some(error); }