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:
parent
3438fe8f2d
commit
d8716efb8e
|
|
@ -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 {
|
||||||
|
|
|
||||||
|
|
@ -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);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue