Skip to content

Commit d8716ef

Browse files
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>
1 parent 3438fe8 commit d8716ef

2 files changed

Lines changed: 188 additions & 10 deletions

File tree

rust/crates/api/src/error.rs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,49 @@ impl ApiError {
180180
}
181181
}
182182

183+
/// HTTP status code for an `Api` error, if any. Used by callers (e.g. the
184+
/// sub-agent provider chain) to decide whether to advance to the next
185+
/// configured model — a 404 "model not found" on the primary should fall
186+
/// through to the next fallback rather than killing the whole chain.
187+
#[must_use]
188+
pub fn status_code(&self) -> Option<reqwest::StatusCode> {
189+
match self {
190+
Self::Api { status, .. } => Some(*status),
191+
Self::RetriesExhausted { last_error, .. } => last_error.status_code(),
192+
Self::MissingCredentials { .. }
193+
| Self::ContextWindowExceeded { .. }
194+
| Self::ExpiredOAuthToken
195+
| Self::Auth(_)
196+
| Self::InvalidApiKeyEnv(_)
197+
| Self::Http(_)
198+
| Self::Io(_)
199+
| Self::Json { .. }
200+
| Self::InvalidSseFrame(_)
201+
| Self::BackoffOverflow { .. }
202+
| Self::RequestBodySizeExceeded { .. } => None,
203+
}
204+
}
205+
206+
/// Response body (best-effort) for an `Api` error, if any.
207+
#[must_use]
208+
pub fn response_body(&self) -> Option<&str> {
209+
match self {
210+
Self::Api { body, .. } => Some(body.as_str()),
211+
Self::RetriesExhausted { last_error, .. } => last_error.response_body(),
212+
Self::MissingCredentials { .. }
213+
| Self::ContextWindowExceeded { .. }
214+
| Self::ExpiredOAuthToken
215+
| Self::Auth(_)
216+
| Self::InvalidApiKeyEnv(_)
217+
| Self::Http(_)
218+
| Self::Io(_)
219+
| Self::Json { .. }
220+
| Self::InvalidSseFrame(_)
221+
| Self::BackoffOverflow { .. }
222+
| Self::RequestBodySizeExceeded { .. } => None,
223+
}
224+
}
225+
183226
#[must_use]
184227
pub fn safe_failure_class(&self) -> &'static str {
185228
match self {

rust/crates/tools/src/lib.rs

Lines changed: 145 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5630,12 +5630,26 @@ fn resolve_agent_model(model: Option<&str>) -> String {
56305630
if let Some(m) = model.map(str::trim).filter(|m| !m.is_empty()) {
56315631
return m.to_string();
56325632
}
5633+
// subagentModel (if set) pins the sub-agent model.
56335634
if let Some(fast) = load_subagent_model_from_config() {
56345635
return fast;
56355636
}
5637+
// Otherwise default to the session's configured `model` so sub-agents use
5638+
// the same provider the user is actually connected with, rather than the
5639+
// hard-coded DEFAULT_AGENT_MODEL (which may not exist on a custom endpoint).
5640+
if let Some(session_model) = load_main_model_from_config() {
5641+
return session_model;
5642+
}
56365643
DEFAULT_AGENT_MODEL.to_string()
56375644
}
56385645

5646+
/// Read the top-level `model` from merged config as a sub-agent default.
5647+
fn load_main_model_from_config() -> Option<String> {
5648+
let cwd = std::env::current_dir().ok()?;
5649+
let config = ConfigLoader::default_for(&cwd).load().ok()?;
5650+
config.model().map(str::to_string)
5651+
}
5652+
56395653
/// Read the `subagentModel` setting from merged config so the Agent tool
56405654
/// honors it even when the caller didn't pass an explicit model.
56415655
fn load_subagent_model_from_config() -> Option<String> {
@@ -6601,18 +6615,33 @@ impl ProviderRuntimeClient {
66016615
allowed_tools: BTreeSet<String>,
66026616
fallback_config: &ProviderFallbackConfig,
66036617
) -> Result<Self, String> {
6604-
let primary_model = fallback_config.primary().map_or(model, str::to_string);
6605-
let primary = build_provider_entry(&primary_model)?;
6618+
// The caller's resolved `model` is the primary. `providerFallbacks`
6619+
// (primary + fallbacks) are recovery entries tried in order after the
6620+
// primary fails with a retryable error. Previously the config's
6621+
// `providerFallbacks.primary` silently overrode the resolved model,
6622+
// which made sub-agents ignore `subagentModel` / the Agent tool's
6623+
// explicit model — e.g. spawning agents that always used a dead
6624+
// configured primary instead of the model the caller picked.
6625+
let primary = build_provider_entry(&model)?;
66066626
let mut chain = vec![primary];
6607-
for fallback_model in fallback_config.fallbacks() {
6608-
match build_provider_entry(fallback_model) {
6627+
let mut seen: BTreeSet<String> = std::iter::once(model.clone()).collect();
6628+
let mut push_fallback = |chain: &mut Vec<ProviderEntry>, m: &str| {
6629+
if seen.contains(m) {
6630+
return;
6631+
}
6632+
seen.insert(m.to_string());
6633+
match build_provider_entry(m) {
66096634
Ok(entry) => chain.push(entry),
66106635
Err(error) => {
6611-
eprintln!(
6612-
"warning: skipping unavailable fallback provider {fallback_model}: {error}"
6613-
);
6636+
eprintln!("warning: skipping unavailable fallback provider {m}: {error}")
66146637
}
66156638
}
6639+
};
6640+
if let Some(config_primary) = fallback_config.primary() {
6641+
push_fallback(&mut chain, config_primary);
6642+
}
6643+
for fallback_model in fallback_config.fallbacks() {
6644+
push_fallback(&mut chain, fallback_model);
66166645
}
66176646
Ok(Self {
66186647
runtime: tokio::runtime::Runtime::new().map_err(|error| error.to_string())?,
@@ -6640,6 +6669,111 @@ fn load_provider_fallback_config() -> ProviderFallbackConfig {
66406669
})
66416670
}
66426671

6672+
/// Whether an API error should advance the provider chain to the next model.
6673+
/// In addition to the standard retryable statuses (429/500/502/503/504), a
6674+
/// 404 whose body indicates the model itself is unknown ("model not found")
6675+
/// is chain-eligible: a dead primary model shouldn't abort the whole team —
6676+
/// fall through to the next configured fallback.
6677+
fn fallback_chain_eligible(error: &ApiError) -> bool {
6678+
if error.is_retryable() {
6679+
return true;
6680+
}
6681+
is_model_not_found(error)
6682+
}
6683+
6684+
/// True when the error body indicates the requested model doesn't exist on
6685+
/// the provider (HTTP 404 + a "not found" / "model" message).
6686+
fn is_model_not_found(error: &ApiError) -> bool {
6687+
let Some(status) = error.status_code() else {
6688+
return false;
6689+
};
6690+
if status != reqwest::StatusCode::NOT_FOUND && status != reqwest::StatusCode::BAD_REQUEST {
6691+
return false;
6692+
}
6693+
let body = error
6694+
.response_body()
6695+
.unwrap_or_default()
6696+
.to_ascii_lowercase();
6697+
let message = error.to_string().to_ascii_lowercase();
6698+
body.contains("not found")
6699+
|| body.contains("does not exist")
6700+
|| message.contains("not found")
6701+
|| (body.contains("model") && body.contains("unavailable"))
6702+
}
6703+
6704+
/// Short human-readable reason for a chain fallthrough, for the log line.
6705+
fn fallback_reason(error: &ApiError) -> &'static str {
6706+
if error.is_retryable() {
6707+
"retryable error"
6708+
} else if is_model_not_found(error) {
6709+
"model not found"
6710+
} else {
6711+
"error"
6712+
}
6713+
}
6714+
6715+
#[cfg(test)]
6716+
mod fallback_chain_tests {
6717+
use super::{fallback_chain_eligible, is_model_not_found};
6718+
use api::ApiError;
6719+
6720+
fn model_not_found_404() -> ApiError {
6721+
ApiError::Api {
6722+
status: reqwest::StatusCode::NOT_FOUND,
6723+
error_type: Some("invalid_request_error".to_string()),
6724+
message: Some("Model 'openai/glm-5.1-fast' not found.".to_string()),
6725+
request_id: None,
6726+
body: "{\"detail\":\"Model 'openai/glm-5.1-fast' not found. Use GET /v1/models to see available models.\"}".to_string(),
6727+
retryable: false,
6728+
suggested_action: None,
6729+
retry_after: None,
6730+
}
6731+
}
6732+
6733+
fn auth_error() -> ApiError {
6734+
ApiError::Api {
6735+
status: reqwest::StatusCode::UNAUTHORIZED,
6736+
error_type: None,
6737+
message: None,
6738+
request_id: None,
6739+
body: "unauthorized".to_string(),
6740+
retryable: false,
6741+
suggested_action: None,
6742+
retry_after: None,
6743+
}
6744+
}
6745+
6746+
fn rate_limited() -> ApiError {
6747+
ApiError::Api {
6748+
status: reqwest::StatusCode::TOO_MANY_REQUESTS,
6749+
error_type: None,
6750+
message: None,
6751+
request_id: None,
6752+
body: "slow down".to_string(),
6753+
retryable: true,
6754+
suggested_action: None,
6755+
retry_after: None,
6756+
}
6757+
}
6758+
6759+
#[test]
6760+
fn model_not_found_404_is_chain_eligible() {
6761+
assert!(is_model_not_found(&model_not_found_404()));
6762+
assert!(fallback_chain_eligible(&model_not_found_404()));
6763+
}
6764+
6765+
#[test]
6766+
fn auth_error_is_not_chain_eligible() {
6767+
assert!(!is_model_not_found(&auth_error()));
6768+
assert!(!fallback_chain_eligible(&auth_error()));
6769+
}
6770+
6771+
#[test]
6772+
fn retryable_error_remains_chain_eligible() {
6773+
assert!(fallback_chain_eligible(&rate_limited()));
6774+
}
6775+
}
6776+
66436777
impl ApiClient for ProviderRuntimeClient {
66446778
fn stream(&mut self, request: ApiRequest) -> Result<Vec<AssistantEvent>, RuntimeError> {
66456779
let tools = tool_specs_for_allowed_tools(Some(&self.allowed_tools))
@@ -6673,10 +6807,11 @@ impl ApiClient for ProviderRuntimeClient {
66736807
let attempt = runtime.block_on(stream_with_provider(&entry.client, &message_request));
66746808
match attempt {
66756809
Ok(events) => return Ok(events),
6676-
Err(error) if error.is_retryable() && index + 1 < chain.len() => {
6810+
Err(error) if fallback_chain_eligible(&error) && index + 1 < chain.len() => {
66776811
eprintln!(
6678-
"provider {} failed with retryable error, falling back: {error}",
6679-
entry.model
6812+
"provider {} failed ({}, falling back to next in chain): {error}",
6813+
entry.model,
6814+
fallback_reason(&error)
66806815
);
66816816
last_error = Some(error);
66826817
}

0 commit comments

Comments
 (0)