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 <noreply@anthropic.com>
This commit is contained in:
TheArchitectit 2026-06-17 12:32:00 -05:00
parent 3438fe8f2d
commit d8716efb8e
2 changed files with 188 additions and 10 deletions

View File

@ -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<reqwest::StatusCode> {
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] #[must_use]
pub fn safe_failure_class(&self) -> &'static str { pub fn safe_failure_class(&self) -> &'static str {
match self { match self {

View File

@ -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()) { if let Some(m) = model.map(str::trim).filter(|m| !m.is_empty()) {
return m.to_string(); return m.to_string();
} }
// subagentModel (if set) pins the sub-agent model.
if let Some(fast) = load_subagent_model_from_config() { if let Some(fast) = load_subagent_model_from_config() {
return fast; 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() 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<String> {
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 /// Read the `subagentModel` setting from merged config so the Agent tool
/// honors it even when the caller didn't pass an explicit model. /// honors it even when the caller didn't pass an explicit model.
fn load_subagent_model_from_config() -> Option<String> { fn load_subagent_model_from_config() -> Option<String> {
@ -6601,18 +6615,33 @@ impl ProviderRuntimeClient {
allowed_tools: BTreeSet<String>, allowed_tools: BTreeSet<String>,
fallback_config: &ProviderFallbackConfig, fallback_config: &ProviderFallbackConfig,
) -> Result<Self, String> { ) -> Result<Self, String> {
let primary_model = fallback_config.primary().map_or(model, str::to_string); // The caller's resolved `model` is the primary. `providerFallbacks`
let primary = build_provider_entry(&primary_model)?; // (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]; let mut chain = vec![primary];
for fallback_model in fallback_config.fallbacks() { let mut seen: BTreeSet<String> = std::iter::once(model.clone()).collect();
match build_provider_entry(fallback_model) { let mut push_fallback = |chain: &mut Vec<ProviderEntry>, m: &str| {
if seen.contains(m) {
return;
}
seen.insert(m.to_string());
match build_provider_entry(m) {
Ok(entry) => chain.push(entry), Ok(entry) => chain.push(entry),
Err(error) => { Err(error) => {
eprintln!( eprintln!("warning: skipping unavailable fallback provider {m}: {error}")
"warning: skipping unavailable fallback provider {fallback_model}: {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 { Ok(Self {
runtime: tokio::runtime::Runtime::new().map_err(|error| error.to_string())?, 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 { impl ApiClient for ProviderRuntimeClient {
fn stream(&mut self, request: ApiRequest) -> Result<Vec<AssistantEvent>, RuntimeError> { fn stream(&mut self, request: ApiRequest) -> Result<Vec<AssistantEvent>, RuntimeError> {
let tools = tool_specs_for_allowed_tools(Some(&self.allowed_tools)) 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)); let attempt = runtime.block_on(stream_with_provider(&entry.client, &message_request));
match attempt { match attempt {
Ok(events) => return Ok(events), 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!( eprintln!(
"provider {} failed with retryable error, falling back: {error}", "provider {} failed ({}, falling back to next in chain): {error}",
entry.model entry.model,
fallback_reason(&error)
); );
last_error = Some(error); last_error = Some(error);
} }