diff --git a/rust/crates/api/src/error.rs b/rust/crates/api/src/error.rs index d4dc0daa..e8ec73a4 100644 --- a/rust/crates/api/src/error.rs +++ b/rust/crates/api/src/error.rs @@ -52,13 +52,19 @@ pub enum ApiError { body_snippet: String, source: serde_json::Error, }, - /// A provider returned an error response. - /// - /// Boxed: the payload is ~139 bytes, and `ApiError` is the `Err` half of nearly every - /// `Result` in this crate, so inlining it inflates the size of every one of those returns - /// (clippy::result_large_err). Boxing keeps `ApiError` small and means adding a field here - /// cannot silently re-inflate every signature in the crate. - Api(Box), + Api { + status: reqwest::StatusCode, + error_type: Option, + message: Option, + request_id: Option, + body: String, + retryable: bool, + /// Suggested user action based on error type (e.g., "Reduce prompt size" for 413) + suggested_action: Option, + /// Parsed Retry-After header value (seconds) for 429 responses. + /// When present, overrides the exponential backoff delay. + retry_after: Option, + }, RetriesExhausted { attempts: u32, last_error: Box, @@ -75,38 +81,7 @@ pub enum ApiError { }, } -/// Payload of [`ApiError::Api`], held behind a `Box` so the enum stays small. -#[derive(Debug)] -pub struct ApiErrorDetails { - pub status: reqwest::StatusCode, - pub error_type: Option, - pub message: Option, - pub request_id: Option, - pub body: String, - pub retryable: bool, - /// Suggested user action based on error type (e.g., "Reduce prompt size" for 413) - pub suggested_action: Option, - /// Parsed Retry-After header value (seconds) for 429 responses. - /// When present, overrides the exponential backoff delay. - pub retry_after: Option, -} - impl ApiError { - /// Convenience constructor so callers do not have to spell out the `Box` at every site. - #[must_use] - pub fn api(details: ApiErrorDetails) -> Self { - Self::Api(Box::new(details)) - } - - /// Borrow the provider-error payload, if this is one. - #[must_use] - pub fn api_details(&self) -> Option<&ApiErrorDetails> { - match self { - Self::Api(details) => Some(details), - _ => None, - } - } - #[must_use] pub const fn missing_credentials( provider: &'static str, @@ -162,7 +137,7 @@ impl ApiError { /// over the computed backoff delay when it exists. pub fn retry_after(&self) -> Option { match self { - Self::Api(details) => details.retry_after, + Self::Api { retry_after, .. } => *retry_after, Self::RetriesExhausted { last_error, .. } => last_error.retry_after(), _ => None, } @@ -171,7 +146,7 @@ impl ApiError { pub fn is_retryable(&self) -> bool { match self { Self::Http(error) => error.is_connect() || error.is_timeout() || error.is_request(), - Self::Api(details) => details.retryable, + Self::Api { retryable, .. } => *retryable, Self::RetriesExhausted { last_error, .. } => last_error.is_retryable(), Self::MissingCredentials { .. } | Self::ContextWindowExceeded { .. } @@ -189,7 +164,7 @@ impl ApiError { #[must_use] pub fn request_id(&self) -> Option<&str> { match self { - Self::Api(details) => details.request_id.as_deref(), + Self::Api { request_id, .. } => request_id.as_deref(), Self::RetriesExhausted { last_error, .. } => last_error.request_id(), Self::MissingCredentials { .. } | Self::ContextWindowExceeded { .. } @@ -216,12 +191,12 @@ impl ApiError { Self::MissingCredentials { .. } | Self::ExpiredOAuthToken | Self::Auth(_) => { "provider_auth" } - Self::Api(details) if matches!(details.status.as_u16(), 401 | 403) => "provider_auth", + Self::Api { status, .. } if matches!(status.as_u16(), 401 | 403) => "provider_auth", Self::ContextWindowExceeded { .. } => "context_window", - Self::Api(_) if self.is_context_window_failure() => "context_window", - Self::Api(details) if details.status.as_u16() == 429 => "provider_rate_limit", - Self::Api(_) if self.is_generic_fatal_wrapper() => "provider_internal", - Self::Api(_) => "provider_error", + Self::Api { .. } if self.is_context_window_failure() => "context_window", + Self::Api { status, .. } if status.as_u16() == 429 => "provider_rate_limit", + Self::Api { .. } if self.is_generic_fatal_wrapper() => "provider_internal", + Self::Api { .. } => "provider_error", Self::Http(_) | Self::InvalidSseFrame(_) | Self::BackoffOverflow { .. } => { "provider_transport" } @@ -233,12 +208,11 @@ impl ApiError { #[must_use] pub fn is_generic_fatal_wrapper(&self) -> bool { match self { - Self::Api(details) => { - details - .message + Self::Api { message, body, .. } => { + message .as_deref() .is_some_and(looks_like_generic_fatal_wrapper) - || looks_like_generic_fatal_wrapper(&details.body) + || looks_like_generic_fatal_wrapper(body) } Self::RetriesExhausted { last_error, .. } => last_error.is_generic_fatal_wrapper(), Self::MissingCredentials { .. } @@ -255,36 +229,21 @@ impl ApiError { } } - /// True for failures whose own text cannot say what went wrong: transport, decode, and frame - /// errors. - /// - /// These matter because a server that answers an oversized request with a non-SSE HTTP 500 - /// body lands here rather than in [`Self::is_context_window_failure`] — reqwest reports - /// "error decoding response body" and the real cause is lost. Callers that know how large the - /// request was can combine the two signals; callers that do not must treat this as unknown, - /// never as an overflow. - #[must_use] - pub fn is_ambiguous_transport_failure(&self) -> bool { - match self { - Self::Http(_) | Self::Json { .. } | Self::InvalidSseFrame(_) => true, - Self::RetriesExhausted { last_error, .. } => { - last_error.is_ambiguous_transport_failure() - } - _ => false, - } - } - #[must_use] pub fn is_context_window_failure(&self) -> bool { match self { Self::ContextWindowExceeded { .. } => true, - Self::Api(details) => { - matches!(details.status.as_u16(), 400 | 413 | 422) - && (details - .message + Self::Api { + status, + message, + body, + .. + } => { + matches!(status.as_u16(), 400 | 413 | 422) + && (message .as_deref() .is_some_and(looks_like_context_window_error) - || looks_like_context_window_error(&details.body)) + || looks_like_context_window_error(body)) } Self::RetriesExhausted { last_error, .. } => last_error.is_context_window_failure(), Self::MissingCredentials { .. } @@ -368,14 +327,14 @@ impl Display for ApiError { "failed to parse {provider} response for model {model}: {source}; first 200 chars of body: {body_snippet}" ), // #28: enhance 401/403 errors with actionable auth guidance - Self::Api(details) if matches!(details.status.as_u16(), 401 | 403) => { - let (status, error_type, message, request_id, body) = ( - &details.status, - &details.error_type, - &details.message, - &details.request_id, - &details.body, - ); + Self::Api { + status, + error_type, + message, + request_id, + body, + .. + } if matches!(status.as_u16(), 401 | 403) => { if let (Some(error_type), Some(message)) = (error_type, message) { write!(f, "api returned {status} ({error_type})")?; if let Some(request_id) = request_id { @@ -397,14 +356,14 @@ impl Display for ApiError { Run `claw doctor` to verify your credential configuration." ) } - Self::Api(details) => { - let (status, error_type, message, request_id, body) = ( - &details.status, - &details.error_type, - &details.message, - &details.request_id, - &details.body, - ); + Self::Api { + status, + error_type, + message, + request_id, + body, + .. + } => { if let (Some(error_type), Some(message)) = (error_type, message) { write!(f, "api returned {status} ({error_type})")?; if let Some(request_id) = request_id { @@ -510,7 +469,6 @@ fn truncate_body_snippet(body: &str, max_chars: usize) -> String { #[cfg(test)] mod tests { - use super::ApiErrorDetails; use super::{truncate_body_snippet, ApiError}; #[test] @@ -575,7 +533,7 @@ mod tests { #[test] fn detects_generic_fatal_wrapper_and_classifies_it_as_provider_internal() { - let error = ApiError::Api(Box::new(ApiErrorDetails { + let error = ApiError::Api { status: reqwest::StatusCode::INTERNAL_SERVER_ERROR, error_type: Some("api_error".to_string()), message: Some( @@ -587,7 +545,7 @@ mod tests { retryable: true, suggested_action: None, retry_after: None, - })); + }; assert!(error.is_generic_fatal_wrapper()); assert_eq!(error.safe_failure_class(), "provider_internal"); @@ -599,7 +557,7 @@ mod tests { fn retries_exhausted_preserves_nested_request_id_and_failure_class() { let error = ApiError::RetriesExhausted { attempts: 3, - last_error: Box::new(ApiError::Api(Box::new(ApiErrorDetails { + last_error: Box::new(ApiError::Api { status: reqwest::StatusCode::BAD_GATEWAY, error_type: Some("api_error".to_string()), message: Some( @@ -611,7 +569,7 @@ mod tests { retryable: true, suggested_action: None, retry_after: None, - }))), + }), }; assert!(error.is_generic_fatal_wrapper()); @@ -621,7 +579,7 @@ mod tests { #[test] fn classifies_provider_context_window_errors() { - let error = ApiError::Api(Box::new(ApiErrorDetails { + let error = ApiError::Api { status: reqwest::StatusCode::BAD_REQUEST, error_type: Some("invalid_request_error".to_string()), message: Some( @@ -633,7 +591,7 @@ mod tests { retryable: false, suggested_action: None, retry_after: None, - })); + }; assert!(error.is_context_window_failure()); assert_eq!(error.safe_failure_class(), "context_window"); @@ -642,7 +600,7 @@ mod tests { #[test] fn classifies_openai_configured_limit_errors_as_context_window_failures() { - let error = ApiError::Api(Box::new(ApiErrorDetails { + let error = ApiError::Api { status: reqwest::StatusCode::BAD_REQUEST, error_type: Some("invalid_request_error".to_string()), message: Some( @@ -654,7 +612,7 @@ mod tests { retryable: false, suggested_action: None, retry_after: None, - })); + }; assert!(error.is_context_window_failure()); assert_eq!(error.safe_failure_class(), "context_window"); diff --git a/rust/crates/api/src/lib.rs b/rust/crates/api/src/lib.rs index a4774f5a..e03d78ee 100644 --- a/rust/crates/api/src/lib.rs +++ b/rust/crates/api/src/lib.rs @@ -12,7 +12,7 @@ pub use client::{ resolve_startup_auth_source, MessageStream, OAuthTokenSet, ProviderClient, ProviderConfig, ProviderConfigKind, }; -pub use error::{ApiError, ApiErrorDetails}; +pub use error::ApiError; pub use http_client::{ build_http_client, build_http_client_or_default, build_http_client_with, build_http_client_with_opts, ProxyConfig, TimeoutConfig, @@ -29,10 +29,9 @@ pub use providers::openai_compat::{ OpenAiCompatClient, OpenAiCompatConfig, }; pub use providers::{ - context_window_override, detect_provider_kind, estimate_message_request_input_tokens, - max_tokens_for_model, max_tokens_for_model_with_override, model_family_identity_for, - model_family_identity_for_kind, model_token_limit, provider_diagnostics_for_model, - resolve_model_alias, ModelTokenLimit, ProviderDiagnostics, ProviderKind, + detect_provider_kind, max_tokens_for_model, max_tokens_for_model_with_override, + model_family_identity_for, model_family_identity_for_kind, provider_diagnostics_for_model, + resolve_model_alias, ProviderDiagnostics, ProviderKind, }; pub use sse::{parse_frame, SseParser}; pub use types::{ diff --git a/rust/crates/api/src/providers/anthropic.rs b/rust/crates/api/src/providers/anthropic.rs index be6eb3a4..430b3eff 100644 --- a/rust/crates/api/src/providers/anthropic.rs +++ b/rust/crates/api/src/providers/anthropic.rs @@ -12,7 +12,7 @@ use serde::Deserialize; use serde_json::{Map, Value}; use telemetry::{AnalyticsEvent, AnthropicRequestProfile, ClientIdentity, SessionTracer}; -use crate::error::{ApiError, ApiErrorDetails}; +use crate::error::ApiError; use crate::http_client::build_http_client_or_default; use crate::prompt_cache::{PromptCache, PromptCacheRecord, PromptCacheStats}; @@ -892,7 +892,7 @@ async fn expect_success(response: reqwest::Response) -> Result Result bool { const SK_ANT_BEARER_HINT: &str = "sk-ant-* keys go in ANTHROPIC_API_KEY (x-api-key header), not ANTHROPIC_AUTH_TOKEN (Bearer header). Move your key to ANTHROPIC_API_KEY."; fn enrich_bearer_auth_error(error: ApiError, auth: &AuthSource) -> ApiError { - let ApiError::Api(mut details) = error else { + let ApiError::Api { + status, + error_type, + message, + request_id, + body, + retryable, + suggested_action, + retry_after, + .. + } = error + else { return error; }; - if details.status.as_u16() != 401 { - return ApiError::Api(details); + if status.as_u16() != 401 { + return ApiError::Api { + status, + error_type, + message, + request_id, + body, + retryable, + suggested_action, + retry_after, + }; } let Some(bearer_token) = auth.bearer_token() else { - return ApiError::Api(details); + return ApiError::Api { + status, + error_type, + message, + request_id, + body, + retryable, + suggested_action, + retry_after, + }; }; if !bearer_token.starts_with("sk-ant-") { - return ApiError::Api(details); + return ApiError::Api { + status, + error_type, + message, + request_id, + body, + retryable, + suggested_action, + retry_after, + }; } // Only append the hint when the AuthSource is pure BearerToken. If both // api_key and bearer_token are present (`ApiKeyAndBearer`), the x-api-key // header is already being sent alongside the Bearer header and the 401 // is coming from a different cause — adding the hint would be misleading. if auth.api_key().is_some() { - return ApiError::Api(details); + return ApiError::Api { + status, + error_type, + message, + request_id, + body, + retryable, + suggested_action, + retry_after, + }; + } + let enriched_message = match message { + Some(existing) => Some(format!("{existing} — hint: {SK_ANT_BEARER_HINT}")), + None => Some(format!("hint: {SK_ANT_BEARER_HINT}")), + }; + ApiError::Api { + status, + error_type, + message: enriched_message, + request_id, + body, + retryable, + suggested_action, + retry_after, } - details.message = Some(match details.message.take() { - Some(existing) => format!("{existing} — hint: {SK_ANT_BEARER_HINT}"), - None => format!("hint: {SK_ANT_BEARER_HINT}"), - }); - ApiError::Api(details) } fn anthropic_wire_model(model: &str) -> &str { @@ -1027,7 +1083,6 @@ struct AnthropicErrorBody { #[cfg(test)] mod tests { use super::{ALT_REQUEST_ID_HEADER, REQUEST_ID_HEADER}; - use crate::error::ApiErrorDetails; use std::io::{Read, Write}; use std::net::TcpListener; use std::sync::{Mutex, OnceLock}; @@ -1592,7 +1647,7 @@ mod tests { fn enrich_bearer_auth_error_appends_sk_ant_hint_on_401_with_pure_bearer_token() { // given let auth = AuthSource::BearerToken("sk-ant-api03-deadbeef".to_string()); - let error = crate::error::ApiError::Api(Box::new(ApiErrorDetails { + let error = crate::error::ApiError::Api { status: reqwest::StatusCode::UNAUTHORIZED, error_type: Some("authentication_error".to_string()), message: Some("Invalid bearer token".to_string()), @@ -1601,7 +1656,7 @@ mod tests { retryable: false, suggested_action: None, retry_after: None, - })); + }; // when let enriched = super::enrich_bearer_auth_error(error, &auth); @@ -1623,8 +1678,8 @@ mod tests { "request id should still flow through the enriched error: {rendered}" ); match enriched { - crate::error::ApiError::Api(details) => { - assert_eq!(details.status, reqwest::StatusCode::UNAUTHORIZED); + crate::error::ApiError::Api { status, .. } => { + assert_eq!(status, reqwest::StatusCode::UNAUTHORIZED); } other => panic!("expected Api variant, got {other:?}"), } @@ -1634,7 +1689,7 @@ mod tests { fn enrich_bearer_auth_error_leaves_non_401_errors_unchanged() { // given let auth = AuthSource::BearerToken("sk-ant-api03-deadbeef".to_string()); - let error = crate::error::ApiError::Api(Box::new(ApiErrorDetails { + let error = crate::error::ApiError::Api { status: reqwest::StatusCode::INTERNAL_SERVER_ERROR, error_type: Some("api_error".to_string()), message: Some("internal server error".to_string()), @@ -1643,7 +1698,7 @@ mod tests { retryable: true, suggested_action: None, retry_after: None, - })); + }; // when let enriched = super::enrich_bearer_auth_error(error, &auth); @@ -1664,7 +1719,7 @@ mod tests { fn enrich_bearer_auth_error_ignores_401_when_bearer_token_is_not_sk_ant() { // given let auth = AuthSource::BearerToken("oauth-access-token-opaque".to_string()); - let error = crate::error::ApiError::Api(Box::new(ApiErrorDetails { + let error = crate::error::ApiError::Api { status: reqwest::StatusCode::UNAUTHORIZED, error_type: Some("authentication_error".to_string()), message: Some("Invalid bearer token".to_string()), @@ -1673,7 +1728,7 @@ mod tests { retryable: false, suggested_action: None, retry_after: None, - })); + }; // when let enriched = super::enrich_bearer_auth_error(error, &auth); @@ -1693,7 +1748,7 @@ mod tests { api_key: "sk-ant-api03-legitimate".to_string(), bearer_token: "sk-ant-api03-deadbeef".to_string(), }; - let error = crate::error::ApiError::Api(Box::new(ApiErrorDetails { + let error = crate::error::ApiError::Api { status: reqwest::StatusCode::UNAUTHORIZED, error_type: Some("authentication_error".to_string()), message: Some("Invalid bearer token".to_string()), @@ -1702,7 +1757,7 @@ mod tests { retryable: false, suggested_action: None, retry_after: None, - })); + }; // when let enriched = super::enrich_bearer_auth_error(error, &auth); @@ -1719,7 +1774,7 @@ mod tests { fn enrich_bearer_auth_error_ignores_401_when_auth_source_has_no_bearer() { // given let auth = AuthSource::ApiKey("sk-ant-api03-legitimate".to_string()); - let error = crate::error::ApiError::Api(Box::new(ApiErrorDetails { + let error = crate::error::ApiError::Api { status: reqwest::StatusCode::UNAUTHORIZED, error_type: Some("authentication_error".to_string()), message: Some("Invalid x-api-key".to_string()), @@ -1728,7 +1783,7 @@ mod tests { retryable: false, suggested_action: None, retry_after: None, - })); + }; // when let enriched = super::enrich_bearer_auth_error(error, &auth); diff --git a/rust/crates/api/src/providers/mod.rs b/rust/crates/api/src/providers/mod.rs index 9c567a06..993a4e7d 100644 --- a/rust/crates/api/src/providers/mod.rs +++ b/rust/crates/api/src/providers/mod.rs @@ -619,43 +619,16 @@ fn web_passthrough_diagnostic( } } -/// Overrides the context window every sizing guard is computed against. -/// -/// The model *name* is not a reliable statement of the window when the endpoint is a local -/// server: the harness sends its compiled-in default name (a Claude alias) while the backend -/// serves whatever `-c` it was started with. Every guard downstream of [`model_token_limit`] -/// then sizes itself against the name's window instead of the real one, which makes the -/// preflights unable to fire rather than merely inaccurate. This variable is the operator's -/// statement of the truth, and it wins over the table. -const CONTEXT_WINDOW_ENV_VAR: &str = "CLAW_CONTEXT_WINDOW"; - -/// Fraction of the window a single response may be allowed to claim when the window is -/// declared. Output and input share one budget on a local server, so reserving a quarter for -/// the reply leaves three quarters for prompt plus transcript. -const MAX_OUTPUT_SHARE_OF_WINDOW: u32 = 4; - -/// The operator-declared context window, when set and parseable as a positive token count. -#[must_use] -pub fn context_window_override() -> Option { - parse_context_window_override(std::env::var(CONTEXT_WINDOW_ENV_VAR).ok().as_deref()) -} - -#[must_use] -fn parse_context_window_override(value: Option<&str>) -> Option { - value - .and_then(|raw| raw.trim().parse::().ok()) - .filter(|window| *window > 0) -} - #[must_use] pub fn max_tokens_for_model(model: &str) -> u32 { - // A declared window caps the reply too. Asking a 65k server for 32k of output reserves half - // the window for a single message, which the server then silently clamps — leaving the - // client's own accounting wrong by the difference. - model_token_limit(model).map_or_else( - || max_tokens_for_model_by_name(model), - |limit| max_tokens_for_model_by_name(model).min(limit.max_output_tokens), - ) + let canonical = resolve_model_alias(model); + let heuristic = if canonical.contains("opus") { + 32_000 + } else { + 64_000 + }; + + model_token_limit(model).map_or(heuristic, |limit| heuristic.min(limit.max_output_tokens)) } /// Returns the effective max output tokens for a model, preferring a plugin @@ -668,33 +641,6 @@ pub fn max_tokens_for_model_with_override(model: &str, plugin_override: Option Option { - // A declared window replaces the table's, and supplies one for models the table has never - // heard of. Returning `Some` for an unknown model is the point: `None` makes both preflights - // return `Ok` without looking at anything, so an unrecognised local model gets no guard at all. - if let Some(context_window_tokens) = context_window_override() { - return Some(ModelTokenLimit { - max_output_tokens: max_tokens_for_model_by_name(model) - .min(context_window_tokens / MAX_OUTPUT_SHARE_OF_WINDOW), - context_window_tokens, - }); - } - model_token_limit_by_name(model) -} - -#[must_use] -fn max_tokens_for_model_by_name(model: &str) -> u32 { - let canonical = resolve_model_alias(model); - let heuristic = if canonical.contains("opus") { - 32_000 - } else { - 64_000 - }; - model_token_limit_by_name(model) - .map_or(heuristic, |limit| heuristic.min(limit.max_output_tokens)) -} - -#[must_use] -fn model_token_limit_by_name(model: &str) -> Option { let canonical = resolve_model_alias(model); let base_model = canonical.rsplit('/').next().unwrap_or(canonical.as_str()); match base_model { @@ -777,7 +723,7 @@ pub fn preflight_message_request(request: &MessageRequest) -> Result<(), ApiErro Ok(()) } -pub fn estimate_message_request_input_tokens(request: &MessageRequest) -> u32 { +fn estimate_message_request_input_tokens(request: &MessageRequest) -> u32 { let mut estimate = estimate_serialized_tokens(&request.messages); estimate = estimate.saturating_add(estimate_serialized_tokens(&request.system)); estimate = estimate.saturating_add(estimate_serialized_tokens(&request.tools)); diff --git a/rust/crates/api/src/providers/openai_compat.rs b/rust/crates/api/src/providers/openai_compat.rs index 183bd35d..e1cca048 100644 --- a/rust/crates/api/src/providers/openai_compat.rs +++ b/rust/crates/api/src/providers/openai_compat.rs @@ -7,7 +7,7 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH}; use serde::Deserialize; use serde_json::{json, Value}; -use crate::error::{ApiError, ApiErrorDetails}; +use crate::error::ApiError; use crate::http_client::build_http_client_or_default; use crate::types::{ ContentBlockDelta, ContentBlockDeltaEvent, ContentBlockStartEvent, ContentBlockStopEvent, @@ -239,7 +239,7 @@ impl OpenAiCompatClient { .get("code") .and_then(serde_json::Value::as_u64) .map(|c| c as u16); - return Err(ApiError::Api(Box::new(ApiErrorDetails { + return Err(ApiError::Api { status: reqwest::StatusCode::from_u16(code.unwrap_or(400)) .unwrap_or(reqwest::StatusCode::BAD_REQUEST), error_type: err_obj @@ -255,7 +255,7 @@ impl OpenAiCompatClient { .unwrap_or(reqwest::StatusCode::BAD_REQUEST), ), retry_after: None, - }))); + }); } } let payload = serde_json::from_str::(&body).map_err(|error| { @@ -1613,7 +1613,7 @@ fn parse_sse_frame( .map(|c| c as u16); let status = reqwest::StatusCode::from_u16(code.unwrap_or(500)) .unwrap_or(reqwest::StatusCode::INTERNAL_SERVER_ERROR); - return Err(ApiError::Api(Box::new(ApiErrorDetails { + return Err(ApiError::Api { status, error_type: err_obj .get("type") @@ -1625,12 +1625,12 @@ fn parse_sse_frame( retryable: false, suggested_action: suggested_action_for_status(status), retry_after: None, - }))); + }); } } // Detect HTML responses if trimmed.starts_with('<') || trimmed.starts_with("(&payload) .map(Some) @@ -1754,7 +1754,7 @@ async fn expect_success(response: reqwest::Response) -> Result Result { assert_eq!(attempts, 2); - let details = last_error - .api_details() - .expect("last error should be a provider Api error"); - assert_eq!(details.status, reqwest::StatusCode::SERVICE_UNAVAILABLE); - assert!(details.retryable); + assert!(matches!( + *last_error, + ApiError::Api { + status: reqwest::StatusCode::SERVICE_UNAVAILABLE, + retryable: true, + .. + } + )); } other => panic!("expected retries exhausted, got {other:?}"), } diff --git a/rust/crates/claw-rag-service/src/ingest.rs b/rust/crates/claw-rag-service/src/ingest.rs index c7083d14..eae43629 100644 --- a/rust/crates/claw-rag-service/src/ingest.rs +++ b/rust/crates/claw-rag-service/src/ingest.rs @@ -66,7 +66,7 @@ async fn flush_path_batch( #[cfg(feature = "qdrant-index")] let mut qdrant_points: Vec = Vec::with_capacity(batch.len()); - for ((ord, t), vec) in batch.drain(..).zip(vecs) { + for ((ord, t), vec) in batch.drain(..).zip(vecs.into_iter()) { let dim = vec.len(); let cid = insert_chunk(conn, path, ord, &t)?; insert_embedding(conn, cid, dim, &vec)?; diff --git a/rust/crates/runtime/src/compact.rs b/rust/crates/runtime/src/compact.rs index d0273f44..797d3a6b 100644 --- a/rust/crates/runtime/src/compact.rs +++ b/rust/crates/runtime/src/compact.rs @@ -95,21 +95,10 @@ pub fn get_compact_continuation_message( #[must_use] pub fn compact_session(session: &Session, config: CompactionConfig) -> CompactionResult { if !should_compact(session, config) { - // "Nothing to drop" is not the same as "nothing to do". A session can exceed the window - // while holding fewer messages than `preserve_recent_messages` — a single read of a large - // file does it — and message-granular compaction has no move to make there. Trim the - // payloads anyway, so the one lever that still works is not gated behind the one that - // does not. - let mut trimmed_session = session.clone(); - trimmed_session.messages = session - .messages - .iter() - .map(truncate_oversized_tool_results) - .collect(); return CompactionResult { summary: String::new(), formatted_summary: String::new(), - compacted_session: trimmed_session, + compacted_session: session.clone(), removed_message_count: 0, }; } @@ -176,14 +165,7 @@ pub fn compact_session(session: &Session, config: CompactionConfig) -> Compactio k }; let removed = &session.messages[compacted_prefix_len..keep_from]; - // Dropping whole messages is not enough on its own. An agentic turn is five messages - // (user / assistant+tool_use / tool_result / assistant+tool_use / tool_result), so the - // preserved tail IS the pair of tool results that overflowed the window, and the only - // message compaction may drop is the short user prompt. Trim the payloads that must stay. - let preserved = session.messages[keep_from..] - .iter() - .map(truncate_oversized_tool_results) - .collect::>(); + let preserved = session.messages[keep_from..].to_vec(); let summary = merge_compact_summaries(existing_summary.as_deref(), &summarize_messages(removed)); let formatted_summary = format_compact_summary(&summary); @@ -208,78 +190,6 @@ pub fn compact_session(session: &Session, config: CompactionConfig) -> Compactio } } -/// Largest tool-result payload compaction will preserve verbatim, in characters. -/// -/// ~16 KB is ~4 k tokens: big enough that ordinary tool output is never touched, small enough -/// that several preserved results still fit a small window. -const MAX_PRESERVED_TOOL_RESULT_CHARS: usize = 16 * 1024; - -/// Characters kept from the tail of a trimmed payload. The head carries the structure a model -/// needs to recognise what it read; the tail is where a file's most recent edits usually are. -const PRESERVED_TOOL_RESULT_TAIL_CHARS: usize = 2 * 1024; - -/// Returns `message` with any oversized `ToolResult` payload replaced by a head/tail excerpt. -/// -/// Clones only when something is actually trimmed, so the common path stays cheap. -fn truncate_oversized_tool_results(message: &ConversationMessage) -> ConversationMessage { - let needs_trim = message.blocks.iter().any(|block| match block { - ContentBlock::ToolResult { output, .. } => { - output.chars().count() > MAX_PRESERVED_TOOL_RESULT_CHARS - } - _ => false, - }); - if !needs_trim { - return message.clone(); - } - - let blocks = message - .blocks - .iter() - .map(|block| match block { - ContentBlock::ToolResult { - tool_use_id, - tool_name, - output, - is_error, - } => ContentBlock::ToolResult { - tool_use_id: tool_use_id.clone(), - tool_name: tool_name.clone(), - output: excerpt_tool_result(output), - is_error: *is_error, - }, - other => other.clone(), - }) - .collect(); - - ConversationMessage { - role: message.role, - blocks, - usage: message.usage, - } -} - -/// Builds a head + marker + tail excerpt of an oversized tool-result payload. -/// -/// Operates on chars, not bytes, so a multi-byte boundary can never be split. -fn excerpt_tool_result(output: &str) -> String { - let chars: Vec = output.chars().collect(); - let total = chars.len(); - if total <= MAX_PRESERVED_TOOL_RESULT_CHARS { - return output.to_string(); - } - - let tail_len = PRESERVED_TOOL_RESULT_TAIL_CHARS.min(total); - let head_len = MAX_PRESERVED_TOOL_RESULT_CHARS.saturating_sub(tail_len); - let elided = total.saturating_sub(head_len + tail_len); - if elided == 0 { - return output.to_string(); - } - - let head: String = chars[..head_len].iter().collect(); - let tail: String = chars[total - tail_len..].iter().collect(); - format!("{head}\n\n[... {elided} characters elided by compaction; re-read this file if you need the omitted portion ...]\n\n{tail}") -} - fn compacted_summary_prefix_len(session: &Session) -> usize { usize::from( session @@ -663,7 +573,7 @@ fn extract_summary_timeline(summary: &str) -> Vec { #[cfg(test)] mod tests { use super::{ - collect_key_files, compact_session, estimate_session_tokens, format_compact_summary, + collect_key_files, compact_session, format_compact_summary, get_compact_continuation_message, infer_pending_work, should_compact, CompactionConfig, }; use crate::session::{ContentBlock, ConversationMessage, MessageRole, Session}; @@ -853,185 +763,6 @@ mod tests { /// user(ToolResult) pair at the boundary. An orphaned tool-result message /// without the preceding assistant `tool_calls` causes a 400 on the /// OpenAI-compat path (gaebal-gajae repro 2026-04-09). - /// The failure this exists for: an agentic turn is 5 messages (user / assistant+tool_use / - /// tool_result / assistant+tool_use / tool_result), so `preserve_recent_messages: 4` protects - /// exactly the two giant tool results that overflow the window. Message-granular compaction - /// can only drop the tiny user prompt, so the payloads themselves must be trimmed. - #[test] - fn compaction_truncates_oversized_tool_results_it_must_preserve() { - let huge = "z".repeat(80_000); - let mut session = Session::new(); - session - .push_message(ConversationMessage::user_text("read two files")) - .expect("user message"); - session - .push_message(ConversationMessage::assistant(vec![ - ContentBlock::ToolUse { - id: "call-1".to_string(), - name: "read_file".to_string(), - input: "{\"path\":\"a.rs\"}".to_string(), - }, - ])) - .expect("assistant tool use"); - session - .push_message(ConversationMessage::tool_result( - "call-1", - "read_file", - huge.clone(), - false, - )) - .expect("tool result"); - session - .push_message(ConversationMessage::assistant(vec![ - ContentBlock::ToolUse { - id: "call-2".to_string(), - name: "read_file".to_string(), - input: "{\"path\":\"b.rs\"}".to_string(), - }, - ])) - .expect("second assistant tool use"); - session - .push_message(ConversationMessage::tool_result( - "call-2", - "read_file", - huge.clone(), - false, - )) - .expect("second tool result"); - - let before = estimate_session_tokens(&session); - let result = compact_session( - &session, - CompactionConfig { - preserve_recent_messages: 4, - max_estimated_tokens: 0, - }, - ); - let after = estimate_session_tokens(&result.compacted_session); - - assert!( - after < before / 2, - "compaction must shrink an oversized preserved tool result: {before} -> {after}" - ); - - let preserved_output = result - .compacted_session - .messages - .iter() - .flat_map(|message| &message.blocks) - .find_map(|block| match block { - ContentBlock::ToolResult { output, .. } => Some(output.clone()), - _ => None, - }) - .expect("the tool result must still be present, not deleted"); - - assert!( - preserved_output.len() < huge.len(), - "the payload must actually be trimmed" - ); - assert!( - preserved_output.contains("elided"), - "the model must be told the view is partial, got: {}", - &preserved_output[..preserved_output.len().min(200)] - ); - assert!( - preserved_output.starts_with('z'), - "the head of the output must be kept" - ); - assert!( - preserved_output.ends_with('z'), - "the tail of the output must be kept" - ); - } - - /// A session can exceed the window while holding FEWER messages than - /// `preserve_recent_messages` — one read of a large file does it. `should_compact` returns - /// false there (nothing is droppable), so trimming must not be gated behind it. - #[test] - fn oversized_tool_results_are_trimmed_even_when_no_message_can_be_dropped() { - let huge = "q".repeat(80_000); - let mut session = Session::new(); - session - .push_message(ConversationMessage::user_text("read one enormous file")) - .expect("user message"); - session - .push_message(ConversationMessage::assistant(vec![ - ContentBlock::ToolUse { - id: "call-1".to_string(), - name: "read_file".to_string(), - input: "{\"path\":\"huge.rs\"}".to_string(), - }, - ])) - .expect("assistant tool use"); - session - .push_message(ConversationMessage::tool_result( - "call-1", - "read_file", - huge, - false, - )) - .expect("tool result"); - - // 3 messages, preserve 4: there is nothing to remove, yet the session is far too big. - let before = estimate_session_tokens(&session); - let result = compact_session( - &session, - CompactionConfig { - preserve_recent_messages: 4, - max_estimated_tokens: 0, - }, - ); - let after = estimate_session_tokens(&result.compacted_session); - - assert!( - after < before / 2, - "an untrimmable session must still have its payloads trimmed: {before} -> {after}" - ); - } - - /// Trimming must not touch payloads that already fit — otherwise every ordinary tool result - /// would grow an elision marker it does not need. - #[test] - fn compaction_leaves_small_tool_results_untouched() { - let small = "ok".repeat(50); - let mut session = Session::new(); - for index in 0..6 { - session - .push_message(ConversationMessage::user_text(format!("turn {index}"))) - .expect("user message"); - session - .push_message(ConversationMessage::tool_result( - format!("call-{index}"), - "read_file", - small.clone(), - false, - )) - .expect("tool result"); - } - - let result = compact_session( - &session, - CompactionConfig { - preserve_recent_messages: 4, - max_estimated_tokens: 0, - }, - ); - - for block in result - .compacted_session - .messages - .iter() - .flat_map(|message| &message.blocks) - { - if let ContentBlock::ToolResult { output, .. } = block { - assert_eq!( - output, &small, - "a small tool result must pass through as-is" - ); - } - } - } - #[test] fn compaction_does_not_split_tool_use_tool_result_pair() { use crate::session::{ContentBlock, Session}; diff --git a/rust/crates/runtime/src/conversation.rs b/rust/crates/runtime/src/conversation.rs index 851149b9..5a0ebc72 100644 --- a/rust/crates/runtime/src/conversation.rs +++ b/rust/crates/runtime/src/conversation.rs @@ -18,47 +18,6 @@ use crate::usage::{TokenUsage, UsageTracker}; const DEFAULT_AUTO_COMPACTION_INPUT_TOKENS_THRESHOLD: u32 = 100_000; const AUTO_COMPACTION_THRESHOLD_ENV_VAR: &str = "CLAUDE_CODE_AUTO_COMPACT_INPUT_TOKENS"; -const DEFAULT_MAX_STALL_NUDGES: usize = 3; -const MAX_STALL_NUDGES_ENV_VAR: &str = "CLAW_MAX_STALL_NUDGES"; -/// Placeholder recorded in the trace when a turn is resumed rather than started, so the trace -/// does not attribute a second user message to a turn that only had one. -const RESUMED_TURN_MARKER: &str = "[resumed turn]"; -/// Operator-declared context window. Kept as a literal rather than imported because `runtime` -/// sits below `api` in the dependency graph; `api::context_window_override` reads the same name. -const CONTEXT_WINDOW_ENV_VAR: &str = "CLAW_CONTEXT_WINDOW"; -/// Share of a declared context window at which compaction is triggered, leaving the remainder -/// as headroom for the reply plus whatever the next tool result carries. -const AUTO_COMPACTION_WINDOW_PERCENT: u32 = 70; - -/// Injected when the model narrates its next step (or asks for a go-ahead mid-task) -/// instead of emitting the tool call that would perform it. -const STALL_NUDGE_PROMPT: &str = "[auto-continue] You ended the turn without calling a tool, \ - but the work is not finished. Do not describe the next step and do not ask for \ - confirmation — issue the tool call now and keep going until the task is done. If \ - everything asked for is genuinely complete, reply with a one-line summary containing \ - no forward-looking sentence."; - -/// Phrases that mark text as an announcement of work the model has not performed yet. -/// Matched against the final non-empty line of the assistant's reply. -const PENDING_ACTION_MARKERS: [&str; 16] = [ - "let me ", - "let's ", - "lets ", - "i'll ", - "i will ", - "i'm going to", - "i am going to", - "going to run", - "about to ", - "now running", - "now checking", - "now i", - "next i", - "next, i", - "proceeding to", - "moving on to", -]; - /// Fully assembled request payload sent to the upstream model client. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ApiRequest { @@ -131,7 +90,6 @@ impl std::error::Error for ToolError {} #[derive(Debug, Clone, PartialEq, Eq)] pub struct RuntimeError { message: String, - context_window: bool, } impl RuntimeError { @@ -139,28 +97,8 @@ impl RuntimeError { pub fn new(message: impl Into) -> Self { Self { message: message.into(), - context_window: false, } } - - /// A failure the caller may be able to clear by shrinking the session. - /// - /// This is a decision the API layer makes and hands up, not one the caller reconstructs by - /// matching substrings against the rendered message. Recovering from context exhaustion means - /// discarding conversation history, so a transport hiccup misread as an overflow destroys the - /// session for no reason — the classification has to come from whoever saw the real error. - #[must_use] - pub fn context_window(message: impl Into) -> Self { - Self { - message: message.into(), - context_window: true, - } - } - - #[must_use] - pub const fn is_context_window_failure(&self) -> bool { - self.context_window - } } impl Display for RuntimeError { @@ -202,7 +140,6 @@ pub struct ConversationRuntime { hook_abort_signal: HookAbortSignal, hook_progress_reporter: Option>, session_tracer: Option, - max_stall_nudges: usize, } impl ConversationRuntime @@ -257,17 +194,9 @@ where hook_abort_signal: HookAbortSignal::default(), hook_progress_reporter: None, session_tracer: None, - max_stall_nudges: max_stall_nudges_from_env(), } } - /// Override how many auto-continue nudges a stalled turn may receive. `0` disables them. - #[must_use] - pub fn with_max_stall_nudges(mut self, max_stall_nudges: usize) -> Self { - self.max_stall_nudges = max_stall_nudges; - self - } - #[must_use] pub fn with_max_iterations(mut self, max_iterations: usize) -> Self { self.max_iterations = max_iterations; @@ -397,33 +326,14 @@ where } } + #[allow(clippy::too_many_lines)] pub fn run_turn( &mut self, user_input: impl Into, - prompter: Option<&mut dyn PermissionPrompter>, - ) -> Result { - self.drive_turn(Some(user_input.into()), prompter) - } - - /// Continues the turn already recorded in the session, without adding a user message. - /// - /// Recovery paths retry a turn that failed part-way through. Calling [`Self::run_turn`] again - /// with the original input would append that input a second time and invite the model to redo - /// tool calls whose side effects have already landed. Resuming reads the transcript as it - /// stands — including the tool results that did complete — and carries on from there. - pub fn resume_turn( - &mut self, - prompter: Option<&mut dyn PermissionPrompter>, - ) -> Result { - self.drive_turn(None, prompter) - } - - #[allow(clippy::too_many_lines)] - fn drive_turn( - &mut self, - user_input: Option, mut prompter: Option<&mut dyn PermissionPrompter>, ) -> Result { + let user_input = user_input.into(); + // ROADMAP #38: Session-health canary - probe if context was compacted if self.session.compaction.is_some() { if let Err(error) = self.run_session_health_probe() { @@ -435,24 +345,16 @@ where } } - match user_input { - Some(user_input) => { - self.record_turn_started(&user_input); - self.session - .push_user_text(user_input) - .map_err(|error| RuntimeError::new(error.to_string()))?; - } - None => self.record_turn_started(RESUMED_TURN_MARKER), - } + self.record_turn_started(&user_input); + self.session + .push_user_text(user_input) + .map_err(|error| RuntimeError::new(error.to_string()))?; let mut assistant_messages = Vec::new(); let mut tool_results = Vec::new(); let mut prompt_cache_events = Vec::new(); let mut iterations = 0; let mut auto_compaction = None; - let max_stall_nudges = self.max_stall_nudges; - let mut stall_nudges = 0; - let mut tools_ran = false; loop { iterations += 1; @@ -464,23 +366,6 @@ where return Err(error); } - // Compact BEFORE the request goes out, not only after a reply lands. - // - // The usage-based check below is post-hoc: it can only see the context once a call - // has returned and reported its counters. Between that measurement and the next - // request the session can grow enormously — a reasoning model emitting a long - // block, then a tool result carrying a large file. Observed live: a call - // measured at 17,858 prompt tokens generated 10,842 more, a read_file added ~19k on - // top, and the next request hit the provider at 47,755 tokens and was rejected - // outright. No amount of tuning the post-hoc threshold prevents that, because - // nothing measures the session in between. - // - // `estimate_session_tokens` is a local heuristic over the transcript, so this costs - // no API call and works before any usage has ever been recorded. - if let Some(compaction) = self.maybe_auto_compact_before_request() { - auto_compaction = Some(compaction); - } - let request = ApiRequest { system_prompt: self.system_prompt.clone(), messages: self.session.messages.clone(), @@ -496,7 +381,6 @@ where match build_assistant_message(events) { Ok(result) => result, Err(error) => { - let error = self.classify_empty_reply(error); self.record_turn_failed(iterations, &error); return Err(error); } @@ -533,35 +417,8 @@ where } if pending_tool_uses.is_empty() { - // A reply with no tool call usually means the turn is done. On small local - // models it just as often means the model narrated the next step — or asked - // for a go-ahead mid-task — and then emitted EOS, leaving the announced work - // undone. Nudge it back into the loop instead of dropping the user at the - // prompt, bounded so a model that insists it is finished still gets to stop. - let stalled = stall_nudges < max_stall_nudges - && is_stalled_reply( - &assistant_text( - assistant_messages - .last() - .expect("assistant message pushed above"), - ), - tools_ran, - ); - if stalled { - stall_nudges += 1; - // Pushed as a system turn, not a user one. The model needs to see it, but the - // transcript must not claim the user asked for it: this text is persisted, - // re-sent on every later request, and folded into the next compaction summary, - // where a user-labelled nudge becomes indistinguishable from something the - // user actually said. - self.session - .push_message(ConversationMessage::system_text(STALL_NUDGE_PROMPT)) - .map_err(|error| RuntimeError::new(error.to_string()))?; - continue; - } break; } - tools_ran = true; for (tool_use_id, tool_name, input) in pending_tool_uses { let pre_hook_result = self.run_pre_tool_use_hook(&tool_name, &input); @@ -716,62 +573,13 @@ where self.session } - /// Compaction check run immediately before a request is sent, based on a locally estimated - /// transcript size rather than on provider-reported usage. - /// - /// This is the guard that actually keeps a session inside the context window: it sees growth - /// that usage counters cannot, because it runs after tool results and long generations have - /// been appended but before the request that would carry them is built. - /// Decides whether an empty or unreadable reply was an overflow, by size rather than by text. - /// - /// A stream that produces no content says nothing about why. Some backends answer an - /// oversized request that way instead of with a typed error, so the failure is worth - /// recovering from — but only when the session was already large enough for that to be the - /// plausible cause. Below the compaction threshold it is a blip, and discarding the - /// transcript would cost more than the retry could win. - fn classify_empty_reply(&self, error: RuntimeError) -> RuntimeError { - if error.is_context_window_failure() { - return error; - } - if estimate_session_tokens(&self.session) - >= self.auto_compaction_input_tokens_threshold as usize - { - return RuntimeError::context_window(error.to_string()); - } - error - } - - fn maybe_auto_compact_before_request(&mut self) -> Option { - let estimated = estimate_session_tokens(&self.session); - if estimated < self.auto_compaction_input_tokens_threshold as usize { - return None; - } - self.compact_now() - } - fn maybe_auto_compact(&mut self) -> Option { - // Gate on the LIVE context size — the most recent call's input counters — not on a - // cumulative running total, and count cached input as the context it is. - // - // The previous gate (`cumulative_usage().input_tokens`) was wrong twice over: - // 1. It ignored `cache_read_input_tokens`. With prompt caching a warm call reports - // `input_tokens: 2`, so the counter crawled and compaction never fired however - // large the context grew. - // 2. Summing across calls is not a context measurement, and the sum never decreased, - // so once it did trip, every later iteration compacted forever. - // Reading the latest turn fixes both: it rises with the real context and falls back - // below the threshold as soon as a compaction has done its job. - if self.usage_tracker.current_turn_usage().total_input_tokens() + if self.usage_tracker.cumulative_usage().input_tokens < self.auto_compaction_input_tokens_threshold { return None; } - self.compact_now() - } - - /// Compacts the session unconditionally, returning `None` when nothing actually changed. - fn compact_now(&mut self) -> Option { let result = compact_session( &self.session, CompactionConfig { @@ -780,12 +588,7 @@ where }, ); - // `removed_message_count` alone is the wrong test for "did anything happen": compaction - // also trims oversized tool-result payloads in messages it must PRESERVE, and on a - // session too short to have anything droppable that trimming is the entire result. - // Gating on the count would compute the smaller session and then throw it away. - let shrank = result.compacted_session.messages != self.session.messages; - if result.removed_message_count == 0 && !shrank { + if result.removed_message_count == 0 { return None; } @@ -906,88 +709,19 @@ where /// Reads the automatic compaction threshold from the environment. #[must_use] pub fn auto_compaction_threshold_from_env() -> u32 { - resolve_auto_compaction_threshold( + parse_auto_compaction_threshold( std::env::var(AUTO_COMPACTION_THRESHOLD_ENV_VAR) .ok() .as_deref(), - std::env::var(CONTEXT_WINDOW_ENV_VAR).ok().as_deref(), - ) -} - -/// Resolves the threshold from an explicit setting, else from the declared context window. -/// -/// `DEFAULT_AUTO_COMPACTION_INPUT_TOKENS_THRESHOLD` is a 200k-window number. Against a smaller -/// server it sits above the window entirely, so the pre-request guard can never fire and the -/// first sign of trouble is the backend rejecting the request. Deriving from the declared window -/// keeps the guard meaningful without every caller having to compute a second number by hand. -#[must_use] -fn resolve_auto_compaction_threshold(explicit: Option<&str>, context_window: Option<&str>) -> u32 { - if let Some(threshold) = parse_positive_u32(explicit) { - return threshold; - } - parse_positive_u32(context_window).map_or( - DEFAULT_AUTO_COMPACTION_INPUT_TOKENS_THRESHOLD, - |window| { - (u64::from(window) * u64::from(AUTO_COMPACTION_WINDOW_PERCENT) / 100) - .try_into() - .unwrap_or(DEFAULT_AUTO_COMPACTION_INPUT_TOKENS_THRESHOLD) - }, ) } #[must_use] -fn parse_positive_u32(value: Option<&str>) -> Option { +fn parse_auto_compaction_threshold(value: Option<&str>) -> u32 { value .and_then(|raw| raw.trim().parse::().ok()) - .filter(|parsed| *parsed > 0) -} - -/// How many auto-continue nudges a single turn may inject. `0` disables the behaviour. -#[must_use] -pub fn max_stall_nudges_from_env() -> usize { - parse_max_stall_nudges(std::env::var(MAX_STALL_NUDGES_ENV_VAR).ok().as_deref()) -} - -#[must_use] -fn parse_max_stall_nudges(value: Option<&str>) -> usize { - value - .and_then(|raw| raw.trim().parse::().ok()) - .unwrap_or(DEFAULT_MAX_STALL_NUDGES) -} - -/// Concatenated text of an assistant message, ignoring thinking and tool blocks. -#[must_use] -fn assistant_text(message: &ConversationMessage) -> String { - message - .blocks - .iter() - .filter_map(|block| match block { - ContentBlock::Text { text } => Some(text.as_str()), - _ => None, - }) - .collect::>() - .join("\n") -} - -/// True when a tool-less reply is a stall rather than a finished turn. -/// -/// Small local models routinely narrate the next tool call in prose and then emit EOS, which -/// ends the turn with the announced work never performed. `after_tool_use` additionally treats -/// a mid-task question as a stall: once tools have run in this turn, stopping to ask for a -/// go-ahead is the same failure wearing a different hat. -#[must_use] -fn is_stalled_reply(text: &str, after_tool_use: bool) -> bool { - let Some(last_line) = text.lines().map(str::trim).rfind(|line| !line.is_empty()) else { - return false; - }; - let lowered = last_line.to_lowercase(); - - if after_tool_use && last_line.ends_with('?') { - return true; - } - PENDING_ACTION_MARKERS - .iter() - .any(|marker| lowered.contains(marker)) + .filter(|threshold| *threshold > 0) + .unwrap_or(DEFAULT_AUTO_COMPACTION_INPUT_TOKENS_THRESHOLD) } fn build_assistant_message( @@ -1119,13 +853,11 @@ impl ToolExecutor for StaticToolExecutor { #[cfg(test)] mod tests { use super::{ - assistant_text, build_assistant_message, is_stalled_reply, parse_max_stall_nudges, - resolve_auto_compaction_threshold, ApiClient, ApiRequest, AssistantEvent, - AutoCompactionEvent, ConversationRuntime, PromptCacheEvent, RuntimeError, + build_assistant_message, parse_auto_compaction_threshold, ApiClient, ApiRequest, + AssistantEvent, AutoCompactionEvent, ConversationRuntime, PromptCacheEvent, RuntimeError, StaticToolExecutor, ToolExecutor, DEFAULT_AUTO_COMPACTION_INPUT_TOKENS_THRESHOLD, - DEFAULT_MAX_STALL_NUDGES, }; - use crate::compact::{estimate_session_tokens, CompactionConfig}; + use crate::compact::CompactionConfig; use crate::config::{RuntimeFeatureConfig, RuntimeHookConfig}; use crate::permissions::{ PermissionMode, PermissionPolicy, PermissionPromptDecision, PermissionPrompter, @@ -1969,246 +1701,19 @@ mod tests { assert_eq!(runtime.session().messages.len(), 2); } - /// Prompt caching reports the bulk of the context as `cache_read_input_tokens` and leaves - /// `input_tokens` at a token or two, so a gate that reads `input_tokens` alone never fires - /// on a cached session no matter how large the context has grown. - #[test] - fn auto_compaction_counts_cached_context_toward_threshold() { - struct CachedApi; - impl ApiClient for CachedApi { - fn stream( - &mut self, - _request: ApiRequest, - ) -> Result, RuntimeError> { - Ok(vec![ - AssistantEvent::TextDelta("done".to_string()), - AssistantEvent::Usage(TokenUsage { - input_tokens: 2, - output_tokens: 4, - cache_creation_input_tokens: 1_804, - cache_read_input_tokens: 112_639, - }), - AssistantEvent::MessageStop, - ]) - } - } - - let mut session = Session::new(); - session.messages = vec![ - crate::session::ConversationMessage::user_text("one"), - crate::session::ConversationMessage::assistant(vec![ContentBlock::Text { - text: "two".to_string(), - }]), - crate::session::ConversationMessage::user_text("three"), - crate::session::ConversationMessage::assistant(vec![ContentBlock::Text { - text: "four".to_string(), - }]), - ]; - - let mut runtime = ConversationRuntime::new( - session, - CachedApi, - StaticToolExecutor::new(), - PermissionPolicy::new(PermissionMode::DangerFullAccess), - vec!["system".to_string()], - ) - .with_auto_compaction_input_tokens_threshold(100_000); - - let summary = runtime - .run_turn("trigger", None) - .expect("turn should succeed"); - - assert_eq!( - summary.auto_compaction, - Some(AutoCompactionEvent { - removed_message_count: 2, - }) - ); - } - - /// The threshold describes the LIVE context, not a monotonic running total. Once compaction - /// has shrunk the context back down, later small turns must not keep compacting. - #[test] - fn auto_compaction_stops_once_the_context_shrinks_again() { - struct ShrinkingApi { - calls: u32, - } - impl ApiClient for ShrinkingApi { - fn stream( - &mut self, - _request: ApiRequest, - ) -> Result, RuntimeError> { - let call = self.calls; - self.calls += 1; - // The first call is over budget; every call after it is small. - let input_tokens = if call == 0 { 150_000 } else { 900 }; - Ok(vec![ - AssistantEvent::TextDelta("done".to_string()), - AssistantEvent::Usage(TokenUsage { - input_tokens, - output_tokens: 4, - cache_creation_input_tokens: 0, - cache_read_input_tokens: 0, - }), - AssistantEvent::MessageStop, - ]) - } - } - - let mut session = Session::new(); - session.messages = vec![ - crate::session::ConversationMessage::user_text("one"), - crate::session::ConversationMessage::assistant(vec![ContentBlock::Text { - text: "two".to_string(), - }]), - crate::session::ConversationMessage::user_text("three"), - crate::session::ConversationMessage::assistant(vec![ContentBlock::Text { - text: "four".to_string(), - }]), - ]; - - let mut runtime = ConversationRuntime::new( - session, - ShrinkingApi { calls: 0 }, - // Every turn after a compaction runs the session health probe, which calls - // `glob_search`; without it the probe fails the turn before the assertion is reached. - StaticToolExecutor::new().register("glob_search", |_| Ok(String::new())), - PermissionPolicy::new(PermissionMode::DangerFullAccess), - vec!["system".to_string()], - ) - .with_auto_compaction_input_tokens_threshold(100_000); - - let first = runtime.run_turn("trigger", None).expect("first turn"); - assert!( - first.auto_compaction.is_some(), - "the over-budget turn should compact" - ); - - let second = runtime.run_turn("again", None).expect("second turn"); - assert_eq!( - second.auto_compaction, None, - "a small turn after compaction must not compact again" - ); - let third = runtime.run_turn("and again", None).expect("third turn"); - assert_eq!( - third.auto_compaction, None, - "compaction must not latch on once the threshold has been crossed" - ); - } - - /// Usage-based compaction is post-hoc: it only sees the context AFTER a call returns. A - /// large tool result (or a long reasoning block) appended after that measurement can blow - /// the window on the very next request, which the provider rejects outright — so there must - /// also be a check BEFORE the request goes out, based on a locally estimated session size. - #[test] - fn auto_compaction_fires_before_the_first_request_when_the_session_is_already_oversized() { - struct RecordingApi { - largest_request_messages: std::rc::Rc>, - } - impl ApiClient for RecordingApi { - fn stream(&mut self, request: ApiRequest) -> Result, RuntimeError> { - self.largest_request_messages.set( - self.largest_request_messages - .get() - .max(request.messages.len()), - ); - Ok(vec![ - AssistantEvent::TextDelta("done".to_string()), - // Deliberately no Usage event: nothing has been measured yet, which is - // exactly the situation the pre-send check has to cover. - AssistantEvent::MessageStop, - ]) - } - } - - // Six messages that are individually large — the shape of a session that has just - // absorbed a couple of big file reads. - let bulk = "x".repeat(40_000); - let mut session = Session::new(); - for _ in 0..3 { - session - .messages - .push(crate::session::ConversationMessage::user_text(bulk.clone())); - session - .messages - .push(crate::session::ConversationMessage::assistant(vec![ - ContentBlock::Text { text: bulk.clone() }, - ])); - } - let before = estimate_session_tokens(&session); - assert!( - before > 24_000, - "test fixture must start over the threshold, got {before}" - ); - - let largest = std::rc::Rc::new(std::cell::Cell::new(0usize)); - let mut runtime = ConversationRuntime::new( - session, - RecordingApi { - largest_request_messages: std::rc::Rc::clone(&largest), - }, - StaticToolExecutor::new().register("glob_search", |_| Ok(String::new())), - PermissionPolicy::new(PermissionMode::DangerFullAccess), - vec!["system".to_string()], - ) - .with_auto_compaction_input_tokens_threshold(24_000); - - let summary = runtime - .run_turn("trigger", None) - .expect("turn should succeed"); - - assert!( - summary.auto_compaction.is_some(), - "an already-oversized session must compact before the request is sent" - ); - assert!( - estimate_session_tokens(runtime.session()) < before, - "compaction must actually shrink the session" - ); - assert!( - largest.get() < 8, - "the request must be sent with the compacted history, not the original 7 messages" - ); - } - #[test] fn auto_compaction_threshold_defaults_and_parses_values() { assert_eq!( - resolve_auto_compaction_threshold(None, None), + parse_auto_compaction_threshold(None), DEFAULT_AUTO_COMPACTION_INPUT_TOKENS_THRESHOLD ); - assert_eq!(resolve_auto_compaction_threshold(Some("4321"), None), 4321); + assert_eq!(parse_auto_compaction_threshold(Some("4321")), 4321); assert_eq!( - resolve_auto_compaction_threshold(Some("0"), None), + parse_auto_compaction_threshold(Some("0")), DEFAULT_AUTO_COMPACTION_INPUT_TOKENS_THRESHOLD ); assert_eq!( - resolve_auto_compaction_threshold(Some("not-a-number"), None), - DEFAULT_AUTO_COMPACTION_INPUT_TOKENS_THRESHOLD - ); - } - - #[test] - fn auto_compaction_threshold_derives_from_a_declared_context_window() { - // 70% of 65536, so the guard sits inside the window instead of 34k above it. - assert_eq!( - resolve_auto_compaction_threshold(None, Some("65536")), - 45875 - ); - } - - #[test] - fn an_explicit_threshold_still_wins_over_the_declared_window() { - assert_eq!( - resolve_auto_compaction_threshold(Some("48000"), Some("65536")), - 48000 - ); - } - - #[test] - fn an_unparseable_context_window_falls_back_to_the_default_threshold() { - assert_eq!( - resolve_auto_compaction_threshold(None, Some("plenty")), + parse_auto_compaction_threshold(Some("not-a-number")), DEFAULT_AUTO_COMPACTION_INPUT_TOKENS_THRESHOLD ); } @@ -2380,146 +1885,6 @@ mod tests { assert_eq!(error.to_string(), "unknown tool: missing"); } - /// Replays a fixed script of stream outcomes, one per call. - struct ScriptedApi { - steps: Vec, String>>, - calls: usize, - } - - impl ScriptedApi { - fn new(steps: Vec, String>>) -> Self { - Self { steps, calls: 0 } - } - } - - impl ApiClient for ScriptedApi { - fn stream(&mut self, _request: ApiRequest) -> Result, RuntimeError> { - let step = self - .steps - .get(self.calls) - .cloned() - .unwrap_or_else(|| Err("script exhausted".to_string())); - self.calls += 1; - step.map_err(RuntimeError::new) - } - } - - fn tool_call_events(id: &str) -> Vec { - vec![ - AssistantEvent::ToolUse { - id: id.to_string(), - name: "echo".to_string(), - input: "payload".to_string(), - }, - AssistantEvent::MessageStop, - ] - } - - fn scripted_runtime( - steps: Vec, String>>, - ) -> ConversationRuntime { - ConversationRuntime::new( - Session::new(), - ScriptedApi::new(steps), - StaticToolExecutor::new().register("echo", |input| Ok(input.to_string())), - PermissionPolicy::new(PermissionMode::DangerFullAccess), - vec!["system".to_string()], - ) - } - - #[test] - fn a_failed_turn_keeps_the_results_of_tools_that_already_ran() { - // given: a turn whose first iteration runs a tool, and whose second call dies - let mut runtime = scripted_runtime(vec![ - Ok(tool_call_events("tool-1")), - Err("upstream failed".to_string()), - ]); - - // when - runtime - .run_turn("do the thing", None) - .expect_err("the second call fails"); - - // then: the executed tool's result is still in the transcript. The side effect happened; - // a caller that discards this record will let the model run the same tool again. - let tool_results = runtime - .session() - .messages - .iter() - .filter(|message| { - message - .blocks - .iter() - .any(|block| matches!(block, ContentBlock::ToolResult { .. })) - }) - .count(); - assert_eq!(tool_results, 1); - } - - #[test] - fn resume_turn_continues_without_appending_another_user_message() { - // given: a session that already carries a user turn and a completed tool result - let mut runtime = scripted_runtime(vec![ - Ok(tool_call_events("tool-1")), - Err("upstream failed".to_string()), - Ok(vec![ - AssistantEvent::TextDelta("done".to_string()), - AssistantEvent::MessageStop, - ]), - ]); - runtime.run_turn("do the thing", None).expect_err("fails"); - let user_messages_before = runtime - .session() - .messages - .iter() - .filter(|message| message.role == MessageRole::User) - .count(); - - // when - runtime - .resume_turn(None) - .expect("the resumed turn succeeds"); - - // then: the prompt was not repeated - let user_messages_after = runtime - .session() - .messages - .iter() - .filter(|message| message.role == MessageRole::User) - .count(); - assert_eq!(user_messages_before, user_messages_after); - } - - #[test] - fn a_stall_nudge_is_recorded_as_a_system_turn_not_a_user_one() { - // given: a reply that announces work and stops, so the nudge fires once - let mut runtime = nudge_runtime( - vec![ - text_only("Let me check the logs."), - text_only("The logs were clean."), - ], - 1, - ); - runtime - .run_turn("check the logs", None) - .expect("the nudged turn completes"); - - // then: exactly one user message — the real one - let user_messages = runtime - .session() - .messages - .iter() - .filter(|message| message.role == MessageRole::User) - .count(); - assert_eq!(user_messages, 1); - assert!(runtime - .session() - .messages - .iter() - .any(|message| message.role == MessageRole::System - && assistant_text(message).contains("[auto-continue]"))); - } - #[test] fn run_turn_errors_when_max_iterations_is_exceeded() { struct LoopingApi; @@ -2591,136 +1956,4 @@ mod tests { // then assert_eq!(error.to_string(), "upstream failed"); } - - fn text_only(text: &str) -> Vec { - vec![ - AssistantEvent::TextDelta(text.to_string()), - AssistantEvent::MessageStop, - ] - } - - struct CountingApi { - calls: usize, - replies: Vec>, - } - - impl ApiClient for CountingApi { - fn stream(&mut self, _request: ApiRequest) -> Result, RuntimeError> { - let reply = self - .replies - .get(self.calls) - .cloned() - .unwrap_or_else(|| text_only("Now running the next probe:")); - self.calls += 1; - Ok(reply) - } - } - - fn nudge_runtime( - replies: Vec>, - max_stall_nudges: usize, - ) -> ConversationRuntime { - ConversationRuntime::new( - Session::new(), - CountingApi { calls: 0, replies }, - StaticToolExecutor::new().register("probe", |_| Ok("probe output".to_string())), - PermissionPolicy::new(PermissionMode::DangerFullAccess), - vec!["system".to_string()], - ) - .with_max_stall_nudges(max_stall_nudges) - } - - #[test] - fn announced_but_uncalled_tool_is_auto_continued() { - // given: the model narrates the next probe instead of calling it - let mut runtime = nudge_runtime( - vec![ - text_only("Now running the Ford EEC-V tuner probe (ATSP1):"), - vec![ - AssistantEvent::ToolUse { - id: "tool-1".to_string(), - name: "probe".to_string(), - input: "atsp1".to_string(), - }, - AssistantEvent::MessageStop, - ], - text_only("No response on ATSP1, which is expected here."), - ], - 3, - ); - - // when - let summary = runtime.run_turn("run the probes", None).expect("turn runs"); - - // then: the nudge pulled it back into the loop and the tool actually ran - assert_eq!(summary.iterations, 3); - assert_eq!(summary.tool_results.len(), 1); - } - - #[test] - fn nudges_are_bounded_so_a_stuck_model_still_stops() { - // given: every reply is an announcement and nothing is ever called - let mut runtime = nudge_runtime(Vec::new(), 2); - - // when - let summary = runtime.run_turn("run the probes", None).expect("turn runs"); - - // then: initial reply plus exactly two nudged retries - assert_eq!(summary.iterations, 3); - assert!(summary.tool_results.is_empty()); - } - - #[test] - fn a_plain_completion_is_not_nudged() { - // given - let mut runtime = nudge_runtime(vec![text_only("The DTC scan came back clean.")], 3); - - // when - let summary = runtime.run_turn("scan for codes", None).expect("turn runs"); - - // then - assert_eq!(summary.iterations, 1); - } - - #[test] - fn zero_budget_disables_auto_continue() { - // given - let mut runtime = nudge_runtime(vec![text_only("Let me run the DTC scan.")], 0); - - // when - let summary = runtime.run_turn("scan for codes", None).expect("turn runs"); - - // then - assert_eq!(summary.iterations, 1); - } - - #[test] - fn a_question_stalls_only_after_a_tool_has_run() { - assert!(is_stalled_reply("Should I run the DTC scan next?", true)); - assert!(!is_stalled_reply("Which port is the adapter on?", false)); - } - - #[test] - fn stall_detection_reads_the_last_line_not_the_whole_reply() { - // an announcement earlier in the reply does not make a finished turn look stalled - assert!(!is_stalled_reply( - "Let me check the port.\nThe scan finished with no codes stored.", - true - )); - assert!(is_stalled_reply( - "The scan finished.\nNext I'll read the freeze-frame data.", - true - )); - } - - #[test] - fn stall_nudge_budget_parses_from_env_value() { - // asserted against the literal, not the constant: comparing the parser to the same - // constant it falls back to would pass no matter what the default became - assert_eq!(DEFAULT_MAX_STALL_NUDGES, 3); - assert_eq!(parse_max_stall_nudges(None), 3); - assert_eq!(parse_max_stall_nudges(Some("not a number")), 3); - assert_eq!(parse_max_stall_nudges(Some(" 5 ")), 5); - assert_eq!(parse_max_stall_nudges(Some("0")), 0); - } } diff --git a/rust/crates/runtime/src/file_ops.rs b/rust/crates/runtime/src/file_ops.rs index 6e034f29..7a62447a 100644 --- a/rust/crates/runtime/src/file_ops.rs +++ b/rust/crates/runtime/src/file_ops.rs @@ -11,48 +11,8 @@ use serde::{Deserialize, Serialize}; use walkdir::{DirEntry, WalkDir}; /// Maximum file size that can be read (10 MB). -/// -/// This is a *process* guard — it stops us slurping a huge or binary blob into memory. It is -/// not a context guard: 10 MB is on the order of 2.5 M tokens, so it never fires on the files -/// that actually blow a model's context window. `DEFAULT_READ_LINES` / `MAX_READ_CHARS` below -/// are the context guard. const MAX_READ_SIZE: u64 = 10 * 1024 * 1024; -/// Lines returned by `read_file` when the caller does not pass an explicit `limit`. -/// -/// This is a *paging* size, not a refusal. An unbounded read is the easiest way for a model to -/// destroy its own context: one 100 KB source file is ~25 k tokens, which does not fit alongside -/// a system prompt in a 32 k window. When that happens the backend rejects the whole request and -/// the session dies — so reading everything at once is what makes it stop. Returning a page plus -/// `nextOffset` means the read always succeeds and the model can keep going. -const DEFAULT_READ_LINES: usize = 2000; - -/// Hard ceiling on the characters `read_file` returns, applied even when `limit` is explicit. -/// -/// `DEFAULT_READ_LINES` alone does not bound the payload: minified bundles, JSON blobs and -/// generated sources routinely put megabytes on a handful of lines. ~64 KB is ~16 k tokens. -/// -/// That default assumes a large (200 k) context window. Against a small self-hosted backend it -/// is far too generous: two reads at this size put a request over 47 k tokens, which a 32 k -/// window rejects outright — and compaction cannot recover it, because the preserved recent -/// messages ARE the oversized tool results. Override with [`MAX_READ_CHARS_ENV_VAR`]. -const DEFAULT_MAX_READ_CHARS: usize = 64 * 1024; - -/// Environment override for [`DEFAULT_MAX_READ_CHARS`], in characters. -const MAX_READ_CHARS_ENV_VAR: &str = "CLAW_MAX_READ_CHARS"; - -/// Effective character ceiling for one `read_file` page. -/// -/// Read per call rather than cached: a long-lived process should pick up a changed budget, and -/// this runs once per read, not per line. -fn max_read_chars() -> usize { - std::env::var(MAX_READ_CHARS_ENV_VAR) - .ok() - .and_then(|raw| raw.trim().parse::().ok()) - .filter(|chars| *chars > 0) - .unwrap_or(DEFAULT_MAX_READ_CHARS) -} - /// Maximum file size that can be written (10 MB). const MAX_WRITE_SIZE: usize = 10 * 1024 * 1024; @@ -120,17 +80,6 @@ pub struct TextFilePayload { pub start_line: usize, #[serde(rename = "totalLines")] pub total_lines: usize, - /// True when the file has more lines past this page. Signals the caller that it has NOT seen - /// the whole file, so it does not silently reason over a partial view. - #[serde(default)] - pub truncated: bool, - /// Line index to pass back as `offset` to continue reading. Absent once the file is exhausted. - #[serde( - rename = "nextOffset", - default, - skip_serializing_if = "Option::is_none" - )] - pub next_offset: Option, } /// Output envelope for the `read_file` tool. @@ -279,28 +228,10 @@ pub fn read_file( let content = fs::read_to_string(&absolute_path)?; let lines: Vec<&str> = content.lines().collect(); let start_index = offset.unwrap_or(0).min(lines.len()); - // An absent `limit` pages rather than reading to EOF: see DEFAULT_READ_LINES. - let requested_end = start_index - .saturating_add(limit.unwrap_or(DEFAULT_READ_LINES)) - .min(lines.len()); - - // Apply the character ceiling on top of the line window, so a file with very long lines - // cannot blow the budget the line cap was meant to enforce. Always yield at least one line, - // otherwise an oversized single line would return nothing and the caller could not advance. - let mut end_index = start_index; - let mut chars = 0usize; - let char_ceiling = max_read_chars(); - for line in &lines[start_index..requested_end] { - let next = chars + line.chars().count() + 1; - if next > char_ceiling && end_index > start_index { - break; - } - chars = next; - end_index += 1; - } - + let end_index = limit.map_or(lines.len(), |limit| { + start_index.saturating_add(limit).min(lines.len()) + }); let selected = lines[start_index..end_index].join("\n"); - let truncated = end_index < lines.len(); Ok(ReadFileOutput { kind: String::from("text"), @@ -310,8 +241,6 @@ pub fn read_file( num_lines: end_index.saturating_sub(start_index), start_line: start_index.saturating_add(1), total_lines: lines.len(), - truncated, - next_offset: truncated.then_some(end_index), }, }) } @@ -883,18 +812,11 @@ mod tests { use super::{ component_contains_glob, derive_glob_walk_root, edit_file, expand_braces, glob_search, - grep_search, is_symlink_escape, max_read_chars, normalize_for_comparison, read_file, + grep_search, is_symlink_escape, normalize_for_comparison, read_file, read_file_in_workspace, write_file, write_file_in_workspace, GrepSearchInput, - DEFAULT_MAX_READ_CHARS, DEFAULT_READ_LINES, MAX_READ_CHARS_ENV_VAR, MAX_WRITE_SIZE, + MAX_WRITE_SIZE, }; - /// Serializes the tests that mutate `CLAW_MAX_READ_CHARS`; env vars are process-global. - fn env_lock() -> std::sync::MutexGuard<'static, ()> { - static LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); - LOCK.lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - } - fn temp_path(name: &str) -> std::path::PathBuf { let unique = SystemTime::now() .duration_since(UNIX_EPOCH) @@ -915,132 +837,6 @@ mod tests { assert_eq!(read_output.file.content, "two"); } - #[test] - fn unbounded_read_of_a_large_file_pages_instead_of_returning_everything() { - // Regression: an unbounded read used to return the whole file, which on a small-context - // backend overflows the window and the request is rejected outright — the session dies. - let path = temp_path("paged-read.txt"); - let total = DEFAULT_READ_LINES + 500; - let body = (0..total) - .map(|i| format!("line {i}")) - .collect::>() - .join("\n"); - write_file(path.to_string_lossy().as_ref(), &body).expect("write should succeed"); - - let first = read_file(path.to_string_lossy().as_ref(), None, None).expect("first page"); - assert_eq!(first.file.num_lines, DEFAULT_READ_LINES); - assert_eq!(first.file.total_lines, total); - assert!(first.file.truncated, "caller must be told it is partial"); - assert_eq!(first.file.next_offset, Some(DEFAULT_READ_LINES)); - assert!(first.file.content.starts_with("line 0\n")); - - // The advertised next_offset must actually resume where the last page stopped. - let second = read_file( - path.to_string_lossy().as_ref(), - first.file.next_offset, - None, - ) - .expect("second page"); - assert_eq!(second.file.num_lines, 500); - assert!(!second.file.truncated, "the tail is the final page"); - assert_eq!(second.file.next_offset, None); - assert!(second - .file - .content - .starts_with(&format!("line {DEFAULT_READ_LINES}\n"))); - } - - #[test] - fn char_ceiling_bounds_files_whose_lines_are_enormous() { - // A line cap alone does not bound the payload: minified bundles put megabytes on a few - // lines, so the byte ceiling has to apply too — and must still yield forward progress. - let _guard = env_lock(); - std::env::remove_var(MAX_READ_CHARS_ENV_VAR); - let path = temp_path("long-lines.txt"); - let huge = "x".repeat(DEFAULT_MAX_READ_CHARS); - let body = format!("{huge}\n{huge}\n{huge}"); - write_file(path.to_string_lossy().as_ref(), &body).expect("write should succeed"); - - let page = read_file(path.to_string_lossy().as_ref(), None, None).expect("first page"); - assert_eq!(page.file.num_lines, 1, "one oversized line still advances"); - assert!(page.file.truncated); - assert_eq!(page.file.next_offset, Some(1)); - - // An explicit limit must not be able to defeat the ceiling. - let greedy = read_file(path.to_string_lossy().as_ref(), None, Some(3)).expect("greedy"); - assert_eq!(greedy.file.num_lines, 1); - assert!(greedy.file.truncated); - } - - /// The 64 KB default is a 200 k-context number. A small self-hosted backend must be able to - /// shrink it, or two reads overflow a 32 k window in a single turn. - #[test] - fn read_char_ceiling_is_configurable_via_the_environment() { - let _guard = env_lock(); - let path = temp_path("configurable-ceiling.txt"); - // 40 lines of 100 chars = ~4 KB: far under the default ceiling, over a 1 KB one. - let body = (0..40) - .map(|_| "y".repeat(100)) - .collect::>() - .join("\n"); - write_file(path.to_string_lossy().as_ref(), &body).expect("write should succeed"); - - std::env::remove_var(MAX_READ_CHARS_ENV_VAR); - let whole = read_file(path.to_string_lossy().as_ref(), None, None).expect("default read"); - assert_eq!( - whole.file.num_lines, 40, - "default ceiling returns the file whole" - ); - assert!(!whole.file.truncated); - - std::env::set_var(MAX_READ_CHARS_ENV_VAR, "1024"); - let clipped = read_file(path.to_string_lossy().as_ref(), None, None).expect("clipped read"); - std::env::remove_var(MAX_READ_CHARS_ENV_VAR); - assert!( - clipped.file.num_lines < 40, - "a smaller ceiling must actually clip, got {} lines", - clipped.file.num_lines - ); - assert!( - clipped.file.truncated, - "a clipped page must report truncated" - ); - assert_eq!( - clipped.file.next_offset, - Some(clipped.file.num_lines), - "the caller must be told where to resume" - ); - } - - #[test] - fn read_char_ceiling_ignores_junk_and_zero_values() { - let _guard = env_lock(); - for value in ["", "0", "-5", "not-a-number"] { - std::env::set_var(MAX_READ_CHARS_ENV_VAR, value); - assert_eq!( - max_read_chars(), - DEFAULT_MAX_READ_CHARS, - "{value:?} must fall back to the default rather than disabling reads" - ); - } - std::env::set_var(MAX_READ_CHARS_ENV_VAR, "24000"); - assert_eq!(max_read_chars(), 24_000); - std::env::remove_var(MAX_READ_CHARS_ENV_VAR); - assert_eq!(max_read_chars(), DEFAULT_MAX_READ_CHARS); - } - - #[test] - fn small_files_are_returned_whole_and_not_marked_truncated() { - let path = temp_path("small-read.txt"); - write_file(path.to_string_lossy().as_ref(), "one\ntwo\nthree").expect("write"); - - let output = read_file(path.to_string_lossy().as_ref(), None, None).expect("read"); - assert_eq!(output.file.content, "one\ntwo\nthree"); - assert_eq!(output.file.num_lines, 3); - assert!(!output.file.truncated); - assert_eq!(output.file.next_offset, None); - } - #[test] fn edits_file_contents() { let path = temp_path("edit.txt"); diff --git a/rust/crates/runtime/src/prompt.rs b/rust/crates/runtime/src/prompt.rs index ae5bf66b..98683112 100644 --- a/rust/crates/runtime/src/prompt.rs +++ b/rust/crates/runtime/src/prompt.rs @@ -42,25 +42,7 @@ pub const SYSTEM_PROMPT_DYNAMIC_BOUNDARY: &str = "__SYSTEM_PROMPT_DYNAMIC_BOUNDA pub const FRONTIER_MODEL_NAME: &str = "Claude Opus 4.6"; const MAX_INSTRUCTION_FILE_CHARS: usize = 4_000; const MAX_TOTAL_INSTRUCTION_CHARS: usize = 12_000; -/// Character budget for the git diff snapshot embedded in the system prompt. -/// -/// This lives in the *system prompt*, so its cost is paid on every session whose -/// working tree has changed — and a diff changes on essentially every edit. The -/// previous 50_000 budget was four times the entire instruction-file budget and -/// could add ~15_000 tokens to the prompt from a single modified file, which is -/// affordable against a hosted frontier model and ruinous against a local one. -/// -/// Override with `CLAW_MAX_GIT_DIFF_CHARS`. `0` drops the diff snapshot entirely; -/// git status and the recent-commit list are unaffected. -const DEFAULT_MAX_GIT_DIFF_CHARS: usize = 4_000; -const MAX_GIT_DIFF_CHARS_ENV_VAR: &str = "CLAW_MAX_GIT_DIFF_CHARS"; - -fn max_git_diff_chars() -> usize { - std::env::var(MAX_GIT_DIFF_CHARS_ENV_VAR) - .ok() - .and_then(|value| value.trim().parse().ok()) - .unwrap_or(DEFAULT_MAX_GIT_DIFF_CHARS) -} +const MAX_GIT_DIFF_CHARS: usize = 50_000; /// Neutral identity for the model family line in generated prompts. #[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] @@ -516,15 +498,8 @@ fn read_git_diff(cwd: &Path) -> Option { } fn truncate_diff(mut diff: String) -> String { - let budget = max_git_diff_chars(); - // A zero budget means "omit the diff", not "truncate to nothing" -- returning - // an empty string here would still push an empty "Git diff snapshot:" heading - // into the prompt. - if budget == 0 { - return String::new(); - } - if diff.len() > budget { - let mut end = budget; + if diff.len() > MAX_GIT_DIFF_CHARS { + let mut end = MAX_GIT_DIFF_CHARS; while !diff.is_char_boundary(end) { end -= 1; } @@ -783,7 +758,6 @@ fn get_simple_doing_tasks_section() -> String { "Do not add speculative abstractions, compatibility shims, or unrelated cleanup.".to_string(), "Do not create files unless they are required to complete the task.".to_string(), "If an approach fails, diagnose the failure before switching tactics.".to_string(), - "Act in the turn you plan in: when the next step is a tool call, make the call. Never end a turn describing a step you have not taken, and do not stop to ask for a go-ahead on work already requested.".to_string(), "Be careful not to introduce security vulnerabilities such as command injection, XSS, or SQL injection.".to_string(), "Report outcomes faithfully: if verification fails or was not run, say so explicitly.".to_string(), ]); @@ -808,8 +782,7 @@ mod tests { collapse_blank_lines, display_context_path, normalize_instruction_content, render_instruction_content, render_instruction_files, truncate_diff, truncate_instruction_content, ContextFile, ModelFamilyIdentity, ProjectContext, - SystemPromptBuilder, DEFAULT_MAX_GIT_DIFF_CHARS, MAX_GIT_DIFF_CHARS_ENV_VAR, - SYSTEM_PROMPT_DYNAMIC_BOUNDARY, + SystemPromptBuilder, MAX_GIT_DIFF_CHARS, SYSTEM_PROMPT_DYNAMIC_BOUNDARY, }; use crate::config::ConfigLoader; use std::fs; @@ -1471,69 +1444,33 @@ mod tests { assert!(rendered.contains("Project rules")); } - /// Serializes the tests that read or mutate `CLAW_MAX_GIT_DIFF_CHARS`; env vars - /// are process-global and the default-budget tests fail if another test leaves - /// an override set. - fn diff_env_lock() -> std::sync::MutexGuard<'static, ()> { - static LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); - LOCK.lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - } - #[test] fn truncate_diff_preserves_short_content() { - let _guard = diff_env_lock(); - std::env::remove_var(MAX_GIT_DIFF_CHARS_ENV_VAR); let short = "a".repeat(1_000); let result = truncate_diff(short.clone()); assert_eq!(result, short); assert!(!result.contains("[diff truncated")); } - #[test] - fn truncate_diff_honors_env_override() { - let _guard = diff_env_lock(); - std::env::set_var(MAX_GIT_DIFF_CHARS_ENV_VAR, "100"); - let result = truncate_diff("z".repeat(1_000)); - std::env::remove_var(MAX_GIT_DIFF_CHARS_ENV_VAR); - let marker = "\n\n... [diff truncated — too large for system prompt]"; - assert!(result.ends_with(marker)); - assert_eq!(result.len() - marker.len(), 100); - } - - #[test] - fn truncate_diff_zero_budget_omits_the_snapshot_entirely() { - // Zero must yield nothing at all -- a truncated-to-empty body would still - // drag a "Git diff snapshot:" heading and the truncation marker into the - // prompt, which is the opposite of what the operator asked for. - let _guard = diff_env_lock(); - std::env::set_var(MAX_GIT_DIFF_CHARS_ENV_VAR, "0"); - let result = truncate_diff("z".repeat(1_000)); - std::env::remove_var(MAX_GIT_DIFF_CHARS_ENV_VAR); - assert!(result.is_empty()); - } - #[test] fn truncate_diff_caps_oversized_content() { - let _guard = diff_env_lock(); - std::env::remove_var(MAX_GIT_DIFF_CHARS_ENV_VAR); - let large = "x".repeat(DEFAULT_MAX_GIT_DIFF_CHARS + 5_000); + let large = "x".repeat(MAX_GIT_DIFF_CHARS + 5_000); let result = truncate_diff(large); assert!(result.contains("... [diff truncated — too large for system prompt]")); - // The body before the marker must be at most DEFAULT_MAX_GIT_DIFF_CHARS bytes + // The body before the marker must be at most MAX_GIT_DIFF_CHARS bytes let marker = "\n\n... [diff truncated — too large for system prompt]"; let body_len = result.len() - marker.len(); - assert!(body_len <= DEFAULT_MAX_GIT_DIFF_CHARS); + assert!(body_len <= MAX_GIT_DIFF_CHARS); } #[test] fn truncate_diff_respects_utf8_char_boundaries() { - // Build a string where DEFAULT_MAX_GIT_DIFF_CHARS falls in the middle of a + // Build a string where MAX_GIT_DIFF_CHARS falls in the middle of a // multi-byte character (U+1F600 = 4 bytes in UTF-8). - let prefix_len = DEFAULT_MAX_GIT_DIFF_CHARS - 2; + let prefix_len = MAX_GIT_DIFF_CHARS - 2; let mut input = "a".repeat(prefix_len); // Append a 4-byte emoji so bytes [prefix_len..prefix_len+4] are the - // emoji. DEFAULT_MAX_GIT_DIFF_CHARS lands at prefix_len+2, inside the emoji. + // emoji. MAX_GIT_DIFF_CHARS lands at prefix_len+2, inside the emoji. input.push('\u{1F600}'); input.push_str(&"b".repeat(10_000)); @@ -1545,7 +1482,7 @@ mod tests { // inside it would be invalid UTF-8. let marker = "\n\n... [diff truncated — too large for system prompt]"; let body = &result[..result.len() - marker.len()]; - assert!(body.len() <= DEFAULT_MAX_GIT_DIFF_CHARS); + assert!(body.len() <= MAX_GIT_DIFF_CHARS); assert!(body.is_char_boundary(body.len())); } } diff --git a/rust/crates/runtime/src/session.rs b/rust/crates/runtime/src/session.rs index 61cca8c8..2ecfd97d 100644 --- a/rust/crates/runtime/src/session.rs +++ b/rust/crates/runtime/src/session.rs @@ -718,20 +718,6 @@ impl ConversationMessage { } } - /// Text the harness injected on its own initiative, not something the user typed. - /// - /// Reaches the model as an ordinary turn — the wire protocol has nowhere else to put it — but - /// stays labelled in the transcript, so a replay, an export, or a compaction summary does not - /// present the harness's own prompting back as the user's words. - #[must_use] - pub fn system_text(text: impl Into) -> Self { - Self { - role: MessageRole::System, - blocks: vec![ContentBlock::Text { text: text.into() }], - usage: None, - } - } - #[must_use] pub fn assistant(blocks: Vec) -> Self { Self { diff --git a/rust/crates/runtime/src/trident.rs b/rust/crates/runtime/src/trident.rs index 07576822..2346a4ea 100644 --- a/rust/crates/runtime/src/trident.rs +++ b/rust/crates/runtime/src/trident.rs @@ -29,7 +29,7 @@ impl Default for TridentConfig { } /// Statistics from a Trident compaction run. -#[derive(Debug, Clone, PartialEq, Eq, Default)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct TridentStats { pub superseded_count: usize, pub collapsed_chains: usize, @@ -41,6 +41,21 @@ pub struct TridentStats { pub final_message_count: usize, } +impl Default for TridentStats { + fn default() -> Self { + Self { + superseded_count: 0, + collapsed_chains: 0, + messages_collapsed: 0, + clusters_found: 0, + messages_clustered: 0, + tokens_saved_estimate: 0, + original_message_count: 0, + final_message_count: 0, + } + } +} + impl TridentStats { pub fn format_report(&self) -> String { let compression = if self.final_message_count > 0 { @@ -177,7 +192,7 @@ fn stage1_supersede(messages: &[ConversationMessage]) -> (Vec = BTreeSet::new(); - for ops in file_ops.values() { + for (_path, ops) in &file_ops { if ops.len() < 2 { continue; } @@ -190,8 +205,10 @@ fn stage1_supersede(messages: &[ConversationMessage]) -> (Vec = Vec::new(); let mut cluster_buffers: BTreeMap> = BTreeMap::new(); @@ -660,7 +677,7 @@ fn truncate_text(text: &str, max_chars: usize) -> String { mod tests { use super::*; use crate::compact::CompactionConfig; - use crate::session::{ContentBlock, ConversationMessage, Session}; + use crate::session::{ContentBlock, ConversationMessage, MessageRole, Session}; #[test] fn stage1_removes_obsolete_file_reads() { @@ -719,7 +736,7 @@ mod tests { fn stage2_collapses_chatty_messages() { let mut messages = vec![]; for i in 0..6 { - messages.push(ConversationMessage::user_text(format!("ok {i}"))); + messages.push(ConversationMessage::user_text(&format!("ok {i}"))); messages.push(ConversationMessage::assistant(vec![ContentBlock::Text { text: format!("got {i}"), }])); @@ -750,9 +767,9 @@ mod tests { }, ])); messages.push(ConversationMessage::tool_result( - format!("read_{i}"), + &format!("read_{i}"), "read_file", - format!(r#"{{"path":"src/{i}.rs","content":"data {i}"}}"#), + &format!(r#"{{"path":"src/{i}.rs","content":"data {i}"}}"#), false, )); } diff --git a/rust/crates/runtime/src/usage.rs b/rust/crates/runtime/src/usage.rs index ceafa7d0..9241f7c2 100644 --- a/rust/crates/runtime/src/usage.rs +++ b/rust/crates/runtime/src/usage.rs @@ -89,19 +89,6 @@ impl TokenUsage { + self.cache_read_input_tokens } - /// Total tokens the provider had to read to serve this call, i.e. the live size of the - /// context window. - /// - /// `input_tokens` alone is NOT that number. With prompt caching a warm call reports - /// `input_tokens: 2` and puts the real bulk in `cache_read_input_tokens`, so anything - /// budgeting against the context window must sum all three input counters. - #[must_use] - pub fn total_input_tokens(self) -> u32 { - self.input_tokens - .saturating_add(self.cache_creation_input_tokens) - .saturating_add(self.cache_read_input_tokens) - } - #[must_use] pub fn estimate_cost_usd(self) -> UsageCostEstimate { self.estimate_cost_usd_with_pricing(ModelPricing::default_sonnet_tier()) @@ -256,41 +243,6 @@ mod tests { assert_eq!(tracker.cumulative_usage().total_tokens(), 48); } - #[test] - fn total_input_tokens_includes_cached_context() { - // The shape a warm prompt-cached call actually reports: input_tokens is ~nothing and - // the real context size is in the cache counters. - let cached = TokenUsage { - input_tokens: 2, - output_tokens: 1_133, - cache_creation_input_tokens: 1_453, - cache_read_input_tokens: 111_186, - }; - assert_eq!(cached.total_input_tokens(), 112_641); - // Output tokens are not part of the input budget. - assert_ne!(cached.total_input_tokens(), cached.total_tokens()); - - // A backend without prompt caching reports the whole prompt as input_tokens. - let uncached = TokenUsage { - input_tokens: 29_056, - output_tokens: 73, - cache_creation_input_tokens: 0, - cache_read_input_tokens: 0, - }; - assert_eq!(uncached.total_input_tokens(), 29_056); - } - - #[test] - fn total_input_tokens_saturates_instead_of_overflowing() { - let usage = TokenUsage { - input_tokens: u32::MAX, - output_tokens: 0, - cache_creation_input_tokens: u32::MAX, - cache_read_input_tokens: u32::MAX, - }; - assert_eq!(usage.total_input_tokens(), u32::MAX); - } - #[test] fn computes_cost_summary_lines() { let usage = TokenUsage { diff --git a/rust/crates/rusty-claude-cli/src/main.rs b/rust/crates/rusty-claude-cli/src/main.rs index 1de5e233..cda89fa1 100644 --- a/rust/crates/rusty-claude-cli/src/main.rs +++ b/rust/crates/rusty-claude-cli/src/main.rs @@ -74,22 +74,6 @@ use tools::{ const DEFAULT_MODEL: &str = "anthropic/claude-opus-4-7"; -/// Ceiling on model/tool round trips within one turn. -/// -/// `ConversationRuntime` defaults to `usize::MAX`, which for the interactive and `-p` paths means -/// no ceiling at all: a model that keeps re-reading the same file loops until the user kills it. -/// Spawned subagents have always been bounded; this gives the top-level loop the same treatment. -const DEFAULT_MAX_TURN_ITERATIONS: usize = 32; -const MAX_TURN_ITERATIONS_ENV_VAR: &str = "CLAW_MAX_TURN_ITERATIONS"; - -fn max_turn_iterations_from_env() -> usize { - std::env::var(MAX_TURN_ITERATIONS_ENV_VAR) - .ok() - .and_then(|raw| raw.trim().parse::().ok()) - .filter(|iterations| *iterations > 0) - .unwrap_or(DEFAULT_MAX_TURN_ITERATIONS) -} - /// #148: Model provenance for `claw status` JSON/text output. Records where /// the resolved model string came from so claws don't have to re-read argv /// to audit whether their `--model` flag was honored vs falling back to env @@ -1957,7 +1941,9 @@ fn parse_args(args: &[String]) -> Result { // Only reject for known top-level subcommands that don't use compact. let first = rest[0].as_str(); if is_known_top_level_subcommand(first) && first != "prompt" { - return Err("invalid_flag_value: --compact is only supported with prompt mode.\nUsage: claw --compact \"\" or echo \"\" | claw --compact".to_string()); + return Err(format!( + "invalid_flag_value: --compact is only supported with prompt mode.\nUsage: claw --compact \"\" or echo \"\" | claw --compact" + )); } } @@ -3161,11 +3147,10 @@ fn print_model_validation_warning_status( usage, permission_mode, context, - &StatusJsonExtras { - allowed_tools, - format_selection: Some(&format_selection), - ..StatusJsonExtras::default() - }, + None, + None, + allowed_tools, + Some(&format_selection), ); let object = value .as_object_mut() @@ -3241,7 +3226,9 @@ fn parse_system_prompt_args( })?; // #99: validate --date is a plausible date string (no newlines, reasonable length) if value.contains('\n') || value.contains('\r') { - return Err("invalid_flag_value: --date value contains invalid characters.\nUsage: --date ".to_string()); + return Err(format!( + "invalid_flag_value: --date value contains invalid characters.\nUsage: --date " + )); } if value.len() > 20 { return Err(format!( @@ -3478,7 +3465,11 @@ impl DiagnosticCheck { fn json_value(&self) -> Value { // Derive a stable snake_case id from the check name for machine-readable keying (#704). - let id = self.name.to_ascii_lowercase().replace([' ', '-'], "_"); + let id = self + .name + .to_ascii_lowercase() + .replace(' ', "_") + .replace('-', "_"); let mut value = Map::from_iter([ ("id".to_string(), Value::String(id.clone())), ( @@ -6659,8 +6650,10 @@ fn run_resume_command( }, default_permission_mode().as_str(), &context, - // #148: resumed sessions don't have flag provenance - &StatusJsonExtras::default(), + None, // #148: resumed sessions don't have flag provenance + None, + None, + None, )), }) } @@ -6841,15 +6834,16 @@ fn run_resume_command( } SlashCommand::Plugins { action, target } => { // Only list is supported in resume mode (no runtime to reload) - if let Some(action @ ("install" | "uninstall" | "enable" | "disable" | "update")) = - action.as_deref() - { - // #777: use interactive_only: prefix + \n hint so #776's classify/split - // emits error_kind:interactive_only + non-null hint instead of unknown+null. - // Orchestrators can now detect this and switch to a live REPL instead of retrying. - return Err(format!( - "interactive_only: /plugins {action} requires a live session to reload the plugin runtime.\nStart `claw` and run `/plugins {action}` inside the REPL, or use `claw plugins {action}` as a direct CLI command." - ).into()); + match action.as_deref() { + Some(action @ ("install" | "uninstall" | "enable" | "disable" | "update")) => { + // #777: use interactive_only: prefix + \n hint so #776's classify/split + // emits error_kind:interactive_only + non-null hint instead of unknown+null. + // Orchestrators can now detect this and switch to a live REPL instead of retrying. + return Err(format!( + "interactive_only: /plugins {action} requires a live session to reload the plugin runtime.\nStart `claw` and run `/plugins {action}` inside the REPL, or use `claw plugins {action}` as a direct CLI command." + ).into()); + } + _ => {} } let cwd = env::current_dir()?; let payload = plugins_command_payload_for( @@ -7913,15 +7907,6 @@ impl LiveCli { Ok(()) } Err(error) => { - // Salvage the transcript before the failed runtime is torn down. - // - // `prepare_turn_runtime` handed `run_turn` a CLONE of the session, and the turn - // mutated only that clone — appending the user message, each assistant reply, and - // a tool_result for every tool that actually executed. Dropping the clone throws - // all of that away while the side effects of those tools remain on disk, so the - // retry below would re-issue them against a session that shows no sign they ever - // ran. Adopting the clone keeps the record and the reality in step. - *self.runtime.session_mut() = runtime.session().clone(); runtime.shutdown_plugins()?; // Same ownership rule as the success arm: `fail` clears the current // line, so it may only do so when the emitter stopped on a line @@ -7948,17 +7933,37 @@ impl LiveCli { // This eliminates the need for users to manually run /compact when they // hit context limits - the recovery happens automatically. // - // Detection: the API layer classified this when it still had the typed error and - // the outgoing request in hand — see `AnthropicRuntimeClient::runtime_error_from_api`. - // It is deliberately NOT re-derived from the rendered message here: compaction - // discards conversation history, and the substrings that used to gate it - // ("error decoding response body", "Failed to parse input at pos") are emitted by - // ordinary transport failures as well as by overflow, so a dropped connection cost - // the session its transcript. + // Detection: We look for "context_window" or "Context window" in the error + // message, which covers error types like: + // - "context_window_blocked" + // - "Context window blocked" + // - "This model's maximum context length is X tokens..." // ============================================================================ let error_str = error.to_string(); - if error.is_context_window_failure() { + // Detect context window overflow. Some providers (e.g. OpenAI-compat backends) + // return 400 with "no parseable body" instead of a proper context_length_exceeded + // error when the request is too large to even parse — treat that as context overflow too. + // Also detect model-specific context error markers (e.g. llama.cpp returns + // "Context size has been exceeded." / "exceed_context_size_error" / "exceeds the available context size"). + let is_context_window = error_str.contains("context_window") + || error_str.contains("Context window") + || error_str.contains("no parseable body") + || error_str.contains("exceed_context_size") + || error_str.contains("exceeds the available context size") + || error_str + .to_ascii_lowercase() + .contains("context size has been exceeded"); + + // Also treat "assistant stream produced no content" and reqwest decode failures + // as recoverable errors that may benefit from auto-compaction. Some backends (e.g. + // llama.cpp) return a non-SSE HTTP 500 body when context overflows, causing + // reqwest to fail with "error decoding response body" — treat that as context overflow too. + let is_no_content = error_str.contains("assistant stream produced no content") + || error_str.contains("Failed to parse input at pos") + || error_str.contains("error decoding response body"); + + if is_context_window || is_no_content { // If the error tells us the server's actual context window, adapt our // auto-compaction threshold so future auto-compact-trigger checks are accurate. if let Some(window) = extract_context_window_tokens_from_error(&error_str) { @@ -7978,7 +7983,8 @@ impl LiveCli { let max_compact_rounds = 4; let preserve_schedule = [4, 2, 1, 0]; - for (round, &preserve) in preserve_schedule.iter().enumerate() { + for round in 0..max_compact_rounds { + let preserve = preserve_schedule[round]; println!( " Auto-compacting session (round {}/{}, preserving {} recent messages)...", round + 1, @@ -8025,11 +8031,7 @@ impl LiveCli { drop(hook_abort_monitor); let mut rp = CliPermissionPrompter::new(self.permission_mode); - // Resume, do not replay. The salvaged session already carries this turn's - // user message and every tool result that completed before the failure; - // re-running `run_turn(input, ..)` would duplicate the prompt and invite a - // second execution of tools that are not idempotent. - match new_runtime.resume_turn(Some(&mut rp)) { + match new_runtime.run_turn(input, Some(&mut rp)) { Ok(summary) => { self.replace_runtime(new_runtime)?; if !terminal_tail.at_line_start() { @@ -8056,7 +8058,20 @@ impl LiveCli { } Err(retry_error) => { let retry_str = retry_error.to_string(); - if retry_error.is_context_window_failure() + let still_context_window = retry_str.contains("context_window") + || retry_str.contains("Context window") + || retry_str.contains("no parseable body") + || retry_str.contains("exceed_context_size") + || retry_str.contains("exceeds the available context size") + || retry_str + .to_ascii_lowercase() + .contains("context size has been exceeded"); + let still_no_content = retry_str + .contains("assistant stream produced no content") + || retry_str.contains("Failed to parse input at pos") + || retry_str.contains("error decoding response body"); + + if (still_context_window || still_no_content) && round + 1 < max_compact_rounds { // If the retry error reveals the context window, adapt threshold. @@ -8783,8 +8798,8 @@ impl LiveCli { let cwd = env::current_dir()?; // #803: reject flag-shaped tokens in list filter for BOTH text and JSON modes. // Previously the guard was JSON-only (#793); text mode silently returned empty success. - if action == Some("list") { - if let Some(filter) = target { + if action.as_deref() == Some("list") { + if let Some(filter) = target.as_deref() { if filter.starts_with('-') { if matches!(output_format, CliOutputFormat::Json) { // ROADMAP #817: this is a handled local inventory parse error. @@ -9779,49 +9794,31 @@ fn print_status_snapshot( usage, permission_mode.mode.as_str(), &context, - &StatusJsonExtras { - provenance: Some(&provenance), - permission_provenance: Some(&permission_mode), - allowed_tools, - format_selection: Some(&format_selection), - }, + Some(&provenance), + Some(&permission_mode), + allowed_tools, + Some(&format_selection), ))? ), } Ok(()) } -/// Optional provenance and selection inputs for [`status_json_value`]. -/// -/// Grouped into a struct rather than trailing positional parameters: they are all -/// `Option<&_>` and most callers pass `None` for most of them, which makes a -/// positional tail easy to transpose silently. -/// -/// `provenance` (#148) drives the `model_source` field ("flag" | "env" | "config" | -/// "default") and `model_raw` (user input before alias resolution, or null when the -/// source is "default"). Callers without provenance (legacy resume paths) leave it -/// `None`, in which case both fields are omitted. -#[derive(Default)] -struct StatusJsonExtras<'a> { - provenance: Option<&'a ModelProvenance>, - permission_provenance: Option<&'a PermissionModeProvenance>, - allowed_tools: Option<&'a AllowedToolSet>, - format_selection: Option<&'a OutputFormatSelection>, -} - fn status_json_value( model: Option<&str>, usage: StatusUsage, permission_mode: &str, context: &StatusContext, - extras: &StatusJsonExtras<'_>, + // #148: optional provenance for `model` field. Surfaces `model_source` + // ("flag" | "env" | "config" | "default") and `model_raw` (user input + // before alias resolution, or null when source is "default"). Callers + // that don't have provenance (legacy resume paths) pass None, in which + // case both new fields are omitted. + provenance: Option<&ModelProvenance>, + permission_provenance: Option<&PermissionModeProvenance>, + allowed_tools: Option<&AllowedToolSet>, + format_selection: Option<&OutputFormatSelection>, ) -> serde_json::Value { - let StatusJsonExtras { - provenance, - permission_provenance, - allowed_tools, - format_selection, - } = *extras; // #143: top-level `status` marker so claws can distinguish // a clean run from a degraded run (config parse failed but other fields // are still populated). `config_load_error` carries the parse-error string @@ -10303,7 +10300,9 @@ fn sandbox_json_value(status: &runtime::SandboxStatus) -> serde_json::Value { // (#731: "not supported on macOS" is a degraded state, not a hard error; // filesystem_active:true means partial containment is working) // error = enabled but unsupported AND no filesystem sandbox either (nothing active) - let top_status = if !status.enabled || status.active { + let top_status = if !status.enabled { + "ok" + } else if status.active { "ok" } else if status.supported { "warn" @@ -10678,20 +10677,18 @@ fn render_doctor_help_json() -> serde_json::Value { } /// #683-#692: extract structured metadata from help prose -/// Parsed fields of a help topic: usage, purpose, output description, formats, -/// related topics, aliases, local-only, requires-credentials. -type HelpMetadata = ( - Option, - Option, - Option, - Option>, - Option>, - Option>, - bool, - bool, -); - -fn extract_help_metadata(topic: LocalHelpTopic) -> HelpMetadata { +fn extract_help_metadata( + topic: LocalHelpTopic, +) -> ( + Option, // usage + Option, // purpose + Option, // output description + Option>, // formats + Option>, // related + Option>, // aliases + bool, // local_only + bool, // requires_credentials +) { let text = render_help_topic(topic); let mut usage = None; let mut purpose = None; @@ -12735,8 +12732,7 @@ fn build_runtime_with_plugin_state( policy, system_prompt, &feature_config, - ) - .with_max_iterations(max_turn_iterations_from_env()); + ); if emit_output { runtime = runtime.with_hook_progress_reporter(Box::new(CliHookProgressReporter)); } @@ -13150,49 +13146,7 @@ impl ApiClient for AnthropicRuntimeClient { } } -/// How full a request must be, as a percentage of the declared context window, before an -/// unreadable failure is taken to mean the window overflowed. -const AMBIGUOUS_FAILURE_WINDOW_PERCENT: u64 = 85; - -/// True when a request was close enough to the window that an otherwise unreadable failure is -/// most likely an overflow. -/// -/// This is the piece that lets transport errors be classified without matching on their prose. -/// A backend can answer an oversized request with a non-SSE 500 that reqwest reports as -/// "error decoding response body"; the same string also appears when the connection simply -/// dropped. What separates them is not the text, it is whether the request we just sent had any -/// room left. Requires a declared window — with none, every request looks small and this -/// correctly returns false. -fn request_fills_context_window(request: &MessageRequest) -> bool { - let Some(limit) = api::model_token_limit(&request.model) else { - return false; - }; - let estimated = - api::estimate_message_request_input_tokens(request).saturating_add(request.max_tokens); - u64::from(estimated) * 100 - >= u64::from(limit.context_window_tokens) * AMBIGUOUS_FAILURE_WINDOW_PERCENT -} - impl AnthropicRuntimeClient { - /// Converts an API failure into a runtime error, carrying the overflow verdict with it. - /// - /// The verdict is made here, where the error still has its type and the request is still in - /// hand, rather than reconstructed downstream from the rendered message. - fn runtime_error_from_api( - &self, - error: &api::ApiError, - request: &MessageRequest, - ) -> RuntimeError { - let message = format_user_visible_api_error(&self.session_id, error); - if error.is_context_window_failure() - || (error.is_ambiguous_transport_failure() && request_fills_context_window(request)) - { - RuntimeError::context_window(message) - } else { - RuntimeError::new(message) - } - } - /// Consume a single streaming response, optionally applying a stall /// timeout on the first event for post-tool continuations. #[allow(clippy::too_many_lines)] @@ -13205,7 +13159,9 @@ impl AnthropicRuntimeClient { .client .stream_message(message_request) .await - .map_err(|error| self.runtime_error_from_api(&error, message_request))?; + .map_err(|error| { + RuntimeError::new(format_user_visible_api_error(&self.session_id, &error)) + })?; let mut stdout = io::stdout(); let mut sink = io::sink(); let mut tracked = TailTrackingWriter::new(&mut stdout, self.terminal_tail.clone()); @@ -13227,8 +13183,9 @@ impl AnthropicRuntimeClient { loop { let next = if apply_stall_timeout && !received_any_event { match tokio::time::timeout(POST_TOOL_STALL_TIMEOUT, stream.next_event()).await { - Ok(inner) => inner - .map_err(|error| self.runtime_error_from_api(&error, message_request))?, + Ok(inner) => inner.map_err(|error| { + RuntimeError::new(format_user_visible_api_error(&self.session_id, &error)) + })?, Err(_elapsed) => { return Err(RuntimeError::new( "post-tool stall: model did not respond within timeout", @@ -13236,10 +13193,9 @@ impl AnthropicRuntimeClient { } } } else { - stream - .next_event() - .await - .map_err(|error| self.runtime_error_from_api(&error, message_request))? + stream.next_event().await.map_err(|error| { + RuntimeError::new(format_user_visible_api_error(&self.session_id, &error)) + })? }; let Some(event) = next else { @@ -13379,7 +13335,9 @@ impl AnthropicRuntimeClient { ..message_request.clone() }) .await - .map_err(|error| self.runtime_error_from_api(&error, message_request))?; + .map_err(|error| { + RuntimeError::new(format_user_visible_api_error(&self.session_id, &error)) + })?; let mut events = response_to_events(response, out)?; push_prompt_cache_record(&self.client, &mut events); Ok(events) @@ -13503,8 +13461,8 @@ fn format_context_window_blocked_error(session_id: &str, error: &api::ApiError) )); lines.push(format!(" Context window {context_window_tokens} tokens")); } - api::ApiError::Api(details) => { - let detail = details.message.as_deref().unwrap_or(&details.body).trim(); + api::ApiError::Api { message, body, .. } => { + let detail = message.as_deref().unwrap_or(body).trim(); if !detail.is_empty() { lines.push(format!( " Detail {}", @@ -13514,7 +13472,7 @@ fn format_context_window_blocked_error(session_id: &str, error: &api::ApiError) } api::ApiError::RetriesExhausted { last_error, .. } => { let detail = match last_error.as_ref() { - api::ApiError::Api(details) => details.message.as_deref().unwrap_or(&details.body), + api::ApiError::Api { message, body, .. } => message.as_deref().unwrap_or(body), other => return format_context_window_blocked_error(session_id, other), } .trim(); @@ -14532,37 +14490,39 @@ fn convert_messages(messages: &[ConversationMessage]) -> Vec { let content = message .blocks .iter() - .map(|block| match block { - ContentBlock::Text { text } => InputContentBlock::Text { text: text.clone() }, + .filter_map(|block| match block { + ContentBlock::Text { text } => { + Some(InputContentBlock::Text { text: text.clone() }) + } ContentBlock::Thinking { thinking, signature, } => { // 保留 Thinking 块:OpenAI 兼容协议会把它转成 reasoning_content 字段 // 回传给 DeepSeek V4(避免 400 "reasoning_content must be passed back" 错误) - InputContentBlock::Thinking { + Some(InputContentBlock::Thinking { thinking: thinking.clone(), signature: signature.clone(), - } + }) } - ContentBlock::ToolUse { id, name, input } => InputContentBlock::ToolUse { + ContentBlock::ToolUse { id, name, input } => Some(InputContentBlock::ToolUse { id: id.clone(), name: name.clone(), input: serde_json::from_str(input) .unwrap_or_else(|_| serde_json::json!({ "raw": input })), - }, + }), ContentBlock::ToolResult { tool_use_id, output, is_error, .. - } => InputContentBlock::ToolResult { + } => Some(InputContentBlock::ToolResult { tool_use_id: tool_use_id.clone(), content: vec![ToolResultContentBlock::Text { text: output.clone(), }], is_error: *is_error, - }, + }), }) .collect::>(); (!content.is_empty()).then(|| InputMessage { @@ -14815,7 +14775,7 @@ mod tests { SessionLifecycleSummary, SlashCommand, StatusUsage, TerminalTail, TmuxPaneSnapshot, DEFAULT_MODEL, LATEST_SESSION_REFERENCE, STUB_COMMANDS, }; - use api::{ApiError, ApiErrorDetails, MessageResponse, OutputContentBlock, Usage}; + use api::{ApiError, MessageResponse, OutputContentBlock, Usage}; use plugins::{ PluginManager, PluginManagerConfig, PluginTool, PluginToolDefinition, PluginToolPermission, }; @@ -14860,7 +14820,7 @@ mod tests { #[test] fn opaque_provider_wrapper_surfaces_failure_class_session_and_trace() { - let error = ApiError::Api(Box::new(ApiErrorDetails { + let error = ApiError::Api { status: "500".parse().expect("status"), error_type: Some("api_error".to_string()), message: Some( @@ -14872,7 +14832,7 @@ mod tests { retryable: true, suggested_action: None, retry_after: None, -})); +}; let rendered = format_user_visible_api_error("session-issue-22", &error); assert!(rendered.contains("provider_internal")); @@ -14884,7 +14844,7 @@ mod tests { fn retry_exhaustion_uses_retry_failure_class_for_generic_provider_wrapper() { let error = ApiError::RetriesExhausted { attempts: 3, - last_error: Box::new(ApiError::Api(Box::new(ApiErrorDetails { + last_error: Box::new(ApiError::Api { status: "502".parse().expect("status"), error_type: Some("api_error".to_string()), message: Some( @@ -14896,7 +14856,7 @@ mod tests { retryable: true, suggested_action: None, retry_after: None, -}))), +}), }; let rendered = format_user_visible_api_error("session-issue-22", &error); @@ -14949,7 +14909,7 @@ mod tests { #[test] fn provider_context_window_errors_are_reframed_with_same_guidance() { - let error = ApiError::Api(Box::new(ApiErrorDetails { + let error = ApiError::Api { status: "400".parse().expect("status"), error_type: Some("invalid_request_error".to_string()), message: Some( @@ -14961,7 +14921,7 @@ mod tests { retryable: false, suggested_action: None, retry_after: None, -})); +}; let rendered = format_user_visible_api_error("session-issue-32", &error); assert!(rendered.contains("context_window_blocked"), "{rendered}"); @@ -14983,7 +14943,7 @@ mod tests { #[test] fn openai_configured_limit_errors_are_rendered_as_context_window_guidance() { - let error = ApiError::Api(Box::new(ApiErrorDetails { + let error = ApiError::Api { status: "400".parse().expect("status"), error_type: Some("invalid_request_error".to_string()), message: Some( @@ -14995,7 +14955,7 @@ mod tests { retryable: false, suggested_action: None, retry_after: None, - })); + }; let rendered = format_user_visible_api_error("session-issue-32", &error); assert!(rendered.contains("Context window blocked"), "{rendered}"); @@ -15021,7 +14981,7 @@ mod tests { fn retry_wrapped_context_window_errors_keep_recovery_guidance() { let error = ApiError::RetriesExhausted { attempts: 2, - last_error: Box::new(ApiError::Api(Box::new(ApiErrorDetails { + last_error: Box::new(ApiError::Api { status: "413".parse().expect("status"), error_type: Some("invalid_request_error".to_string()), message: Some("Request is too large for this model's context window.".to_string()), @@ -15030,7 +14990,7 @@ mod tests { retryable: false, suggested_action: None, retry_after: None, - }))), + }), }; let rendered = format_user_visible_api_error("session-issue-32", &error); @@ -15077,54 +15037,11 @@ mod tests { ); } - /// Serialises tests that mutate process-global state, and isolates them from the - /// developer's real `~/.claw`. - /// - /// `parse_args` resolves defaults — notably `permissions.defaultMode` — through the - /// user-scope config, which `default_config_home()` reads from `CLAW_CONFIG_HOME`, - /// falling back to `$HOME/.claw`. Without an override, these tests therefore assert - /// against whatever the machine running them happens to have configured: a - /// `defaultMode` of `dontAsk` there resolves to `DangerFullAccess` and turns a dozen - /// `WorkspaceWrite` assertions red. Point the config home at a fresh empty directory - /// so "no user config" is the state under test, and restore the caller's value on drop. - /// - /// Tests that need a *populated* config home can still set `CLAW_CONFIG_HOME` - /// themselves after taking this guard; the guard restores the real value afterwards. - struct EnvGuard { - _lock: MutexGuard<'static, ()>, - config_home: PathBuf, - previous_config_home: Option, - } - - impl Drop for EnvGuard { - fn drop(&mut self) { - match self.previous_config_home.take() { - Some(previous) => std::env::set_var("CLAW_CONFIG_HOME", previous), - None => std::env::remove_var("CLAW_CONFIG_HOME"), - } - // Best effort: a leftover temp dir must never fail a test. - let _ = std::fs::remove_dir_all(&self.config_home); - } - } - - fn env_lock() -> EnvGuard { + fn env_lock() -> MutexGuard<'static, ()> { static LOCK: OnceLock> = OnceLock::new(); - let lock = LOCK - .get_or_init(|| Mutex::new(())) + LOCK.get_or_init(|| Mutex::new(())) .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - - // Deliberately empty: the point is that no user settings.json is discoverable. - let config_home = temp_dir(); - std::fs::create_dir_all(&config_home).expect("isolated config home should be creatable"); - let previous_config_home = std::env::var_os("CLAW_CONFIG_HOME"); - std::env::set_var("CLAW_CONFIG_HOME", &config_home); - - EnvGuard { - _lock: lock, - config_home, - previous_config_home, - } + .unwrap_or_else(std::sync::PoisonError::into_inner) } fn with_current_dir(cwd: &Path, f: impl FnOnce() -> T) -> T { @@ -15851,8 +15768,6 @@ mod tests { #[test] fn removed_login_and_logout_subcommands_error_helpfully() { - // Asserts a `Default`-sourced permission mode, so it must not see a user config. - let _guard = env_lock(); let login = parse_args(&["login".to_string()]).expect_err("login should be removed"); assert!(login.contains("ANTHROPIC_API_KEY")); let logout = parse_args(&["logout".to_string()]).expect_err("logout should be removed"); @@ -16480,7 +16395,10 @@ mod tests { usage, "workspace-write", &context, - &super::StatusJsonExtras::default(), + None, + None, + None, + None, ); assert_eq!( json.get("status").and_then(|v| v.as_str()), @@ -16552,10 +16470,10 @@ mod tests { usage, "workspace-write", &context, - &super::StatusJsonExtras { - allowed_tools: Some(&allowed), - ..super::StatusJsonExtras::default() - }, + None, + None, + Some(&allowed), + None, ); assert_eq!( restricted_json @@ -16585,7 +16503,10 @@ mod tests { usage, "workspace-write", &clean_context, - &super::StatusJsonExtras::default(), + None, + None, + None, + None, ); assert_eq!( clean_json.get("status").and_then(|v| v.as_str()), @@ -17533,7 +17454,7 @@ mod tests { for action in ["remove", "uninstall", "delete"] { assert_eq!( parse_args(&["skills".to_string(), action.to_string()]) - .unwrap_or_else(|_| panic!("skills {action} should parse")), + .expect(&format!("skills {action} should parse")), CliAction::Skills { args: Some(action.to_string()), output_format: CliOutputFormat::Text, @@ -18487,7 +18408,10 @@ mod tests { }, "workspace-write", &context, - &super::StatusJsonExtras::default(), + None, + None, + None, + None, ); assert_eq!( @@ -19761,6 +19685,7 @@ UU conflicted.rs", "openai/gpt-5".to_string(), false, false, + TerminalTail::new(), None, GlobalToolRegistry::builtin(), None, @@ -19820,6 +19745,7 @@ UU conflicted.rs", "qwen-plus".to_string(), false, false, + TerminalTail::new(), None, GlobalToolRegistry::builtin(), None, diff --git a/rust/crates/rusty-claude-cli/src/setup_wizard.rs b/rust/crates/rusty-claude-cli/src/setup_wizard.rs index aa315dd6..c2f7b6ff 100644 --- a/rust/crates/rusty-claude-cli/src/setup_wizard.rs +++ b/rust/crates/rusty-claude-cli/src/setup_wizard.rs @@ -2,6 +2,8 @@ use std::io::{self, IsTerminal, Write}; use runtime::{save_user_provider_settings, ConfigLoader, RuntimeProviderConfig}; +use serde_json; + const PROVIDERS: &[(&str, &str, &str)] = &[ ("1", "Anthropic", "anthropic"), ("2", "xAI / Grok", "xai"), diff --git a/rust/crates/rusty-claude-cli/tests/output_format_contract.rs b/rust/crates/rusty-claude-cli/tests/output_format_contract.rs index f620797e..8984675e 100644 --- a/rust/crates/rusty-claude-cli/tests/output_format_contract.rs +++ b/rust/crates/rusty-claude-cli/tests/output_format_contract.rs @@ -451,16 +451,11 @@ fn direct_resume_safe_slash_commands_route_to_local_json_actions_831() { .output() .expect("git init should launch"); - // `/sandbox`'s status reflects the host's real sandbox capability, so it is the one - // entry with more than one legitimate value: "ok" where namespace isolation works, - // "warn" where it is unsupported and only the filesystem sandbox is active (#731). - // This test is about #831 routing — that these commands produce a local JSON action - // instead of `interactive_only` — not about the kernel the suite happens to run on. - for (command, expected_kind, expected_statuses) in [ - ("/version", "version", &["ok"][..]), - ("/sandbox", "sandbox", &["ok", "warn"][..]), - ("/diff", "diff", &["ok"][..]), - ("/status", "status", &["ok"][..]), + for (command, expected_kind, expected_status) in [ + ("/version", "version", "ok"), + ("/sandbox", "sandbox", "warn"), + ("/diff", "diff", "ok"), + ("/status", "status", "ok"), ] { let output = run_claw(&root, &["--output-format", "json", command], &[]); assert!( @@ -475,9 +470,9 @@ fn direct_resume_safe_slash_commands_route_to_local_json_actions_831() { .unwrap_or_else(|_| panic!("{command} must emit JSON (#831), got: {stdout:?}")); assert_eq!(parsed["kind"], expected_kind, "{command} kind: {parsed}"); - assert!( - expected_statuses.contains(&parsed["status"].as_str().unwrap_or_default()), - "{command} status should be one of {expected_statuses:?}: {parsed}" + assert_eq!( + parsed["status"], expected_status, + "{command} status: {parsed}" ); assert_ne!( parsed["error_kind"], "interactive_only", @@ -1012,13 +1007,13 @@ fn inventory_commands_emit_structured_json_when_requested() { assert!( !plugins .as_object() - .is_some_and(|o| o.contains_key("reload_runtime")), + .map_or(false, |o| o.contains_key("reload_runtime")), "plugins list should not include reload_runtime" ); assert!( !plugins .as_object() - .is_some_and(|o| o.contains_key("target")), + .map_or(false, |o| o.contains_key("target")), "plugins list should not include target" ); // #703: structured summary replaces prose message @@ -1712,13 +1707,13 @@ fn resumed_inventory_commands_emit_structured_json_when_requested() { assert!( !plugins .as_object() - .is_some_and(|o| o.contains_key("reload_runtime")), + .map_or(false, |o| o.contains_key("reload_runtime")), "plugins list should not include reload_runtime" ); assert!( !plugins .as_object() - .is_some_and(|o| o.contains_key("target")), + .map_or(false, |o| o.contains_key("target")), "plugins list should not include target" ); assert!( @@ -2951,7 +2946,7 @@ fn prompt_empty_arg_json_stdout_missing_prompt_823() { "claw prompt empty arg must retain abort action (#823); got: {parsed}" ); assert!( - parsed["hint"].as_str().is_some_and(|h| !h.is_empty()), + parsed["hint"].as_str().map_or(false, |h| !h.is_empty()), "claw prompt empty arg missing_prompt hint must be non-empty (#823)" ); } @@ -2989,9 +2984,9 @@ fn flag_value_errors_have_error_kind_and_hint_756() { "invalid --reasoning-effort must be invalid_flag_value (#756): {parsed}" ); assert!( - parsed["hint"] - .as_str() - .is_some_and(|h| h.contains("low") || h.contains("medium") || h.contains("high")), + parsed["hint"].as_str().map_or(false, |h| h.contains("low") + || h.contains("medium") + || h.contains("high")), "hint must mention valid values (#756): {parsed}" ); @@ -3017,7 +3012,7 @@ fn flag_value_errors_have_error_kind_and_hint_756() { "missing --model value must be missing_flag_value (#756): {parsed2}" ); assert!( - parsed2["hint"].as_str().is_some_and(|h| !h.is_empty()), + parsed2["hint"].as_str().map_or(false, |h| !h.is_empty()), "missing --model hint must be non-empty (#756): {parsed2}" ); } @@ -3261,7 +3256,7 @@ fn short_p_flag_swallows_no_flags_755() { "flag-like token after -p must be rejected as missing_prompt (#755): {parsed2}" ); assert!( - parsed2["hint"].as_str().is_some_and(|h| !h.is_empty()), + parsed2["hint"].as_str().map_or(false, |h| !h.is_empty()), "missing_prompt hint must be non-empty (#755)" ); } @@ -3403,7 +3398,7 @@ fn config_unsupported_section_json_hint_741() { assert!( parsed["supported_sections"] .as_array() - .is_some_and(|a| !a.is_empty()), + .map_or(false, |a| !a.is_empty()), "config {section} JSON must include supported_sections (#741)" ); } @@ -3460,7 +3455,7 @@ fn export_json_has_kind_702() { // On success stdout has kind:export; on failure stderr has type:error. // Either way, both envelopes must be valid JSON. let stdout = String::from_utf8_lossy(&output.stdout); - let _stderr = String::from_utf8_lossy(&output.stderr) + let stderr = String::from_utf8_lossy(&output.stderr) .lines() .filter(|l| l.starts_with('{')) .collect::>() @@ -3552,8 +3547,8 @@ fn config_parse_error_has_typed_error_kind_and_hint_764() { !output.status.success(), "malformed settings.json should cause non-zero exit" ); - let _stdout = String::from_utf8_lossy(&output.stdout); - let _stderr = String::from_utf8_lossy(&output.stderr); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); let json_line = stdout .lines() @@ -3587,8 +3582,8 @@ fn login_logout_removed_subcommands_have_error_kind_and_hint_765() { !output.status.success(), "claw {subcmd} should exit non-zero" ); - let _stdout = String::from_utf8_lossy(&output.stdout); - let _stderr = String::from_utf8_lossy(&output.stderr); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); let json_line = stdout .lines() @@ -3727,8 +3722,8 @@ fn resume_non_slash_trailing_arg_has_typed_error_kind_and_hint_768() { !output.status.success(), "claw --resume latest compact should exit non-zero" ); - let _stdout = String::from_utf8_lossy(&output.stdout); - let _stderr = String::from_utf8_lossy(&output.stderr); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); let json_line = stdout .lines() @@ -3767,8 +3762,8 @@ fn session_with_unknown_subcommand_returns_interactive_only_not_credentials_767( !output.status.success(), "claw session {sub} should exit non-zero" ); - let _stdout = String::from_utf8_lossy(&output.stdout); - let _stderr = String::from_utf8_lossy(&output.stderr); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); let json_line = stdout .lines() @@ -3821,7 +3816,7 @@ fn slash_only_verbs_with_args_return_interactive_only_not_credentials_770() { "claw {} should exit non-zero", args.join(" ") ); - let _stdout = String::from_utf8_lossy(&output.stdout); + let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); let json_line = stdout @@ -3863,8 +3858,8 @@ fn agents_plugins_mcp_unknown_subcommand_have_hint_774() { { let output = run_claw(&root, &["--output-format", "json", "agents", "bogus"], &[]); assert!(!output.status.success(), "agents bogus should fail"); - let _stdout = String::from_utf8_lossy(&output.stdout); - let _stderr = String::from_utf8_lossy(&output.stderr); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); let json_line = stdout .lines() @@ -3887,8 +3882,8 @@ fn agents_plugins_mcp_unknown_subcommand_have_hint_774() { { let output = run_claw(&root, &["--output-format", "json", "plugins", "bogus"], &[]); assert!(!output.status.success(), "plugins bogus should fail"); - let _stdout = String::from_utf8_lossy(&output.stdout); - let _stderr = String::from_utf8_lossy(&output.stderr); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); let json_line = stdout .lines() @@ -3907,7 +3902,7 @@ fn agents_plugins_mcp_unknown_subcommand_have_hint_774() { { let output = run_claw(&root, &["--output-format", "json", "mcp", "bogus"], &[]); assert!(!output.status.success(), "mcp bogus should fail"); - let _stdout = String::from_utf8_lossy(&output.stdout); + let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); let json_str = if stdout.trim().starts_with('{') { @@ -4022,7 +4017,7 @@ fn interactive_only_guard_batch_769_to_771() { "claw {} should exit non-zero", args.join(" ") ); - let _stdout = String::from_utf8_lossy(&output.stdout); + let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); let json_line = stdout @@ -4086,7 +4081,7 @@ fn resume_plugin_mutations_are_typed_interactive_only_777() { !output.status.success(), "/plugins {mutation} in resume mode should exit non-zero" ); - let _stdout = String::from_utf8_lossy(&output.stdout); + let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); let json_line = stdout @@ -4143,7 +4138,7 @@ fn resume_skills_invocation_is_typed_interactive_only_779() { !output.status.success(), "/skills in resume mode should exit non-zero" ); - let _stdout = String::from_utf8_lossy(&output.stdout); + let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); let json_line = stdout @@ -4184,8 +4179,8 @@ fn acp_unsupported_invocation_has_hint_782() { let output = run_claw(&root, &["--output-format", "json", "acp", "start"], &[]); assert!(!output.status.success(), "acp start should fail"); - let _stdout = String::from_utf8_lossy(&output.stdout); - let _stderr = String::from_utf8_lossy(&output.stderr); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); let json_line = stdout .lines() @@ -4222,7 +4217,7 @@ fn init_json_envelope_has_hint_and_already_initialized_783() { // Fresh init — already_initialized should be false, hint should mention CLAUDE.md let output = run_claw(&root, &["--output-format", "json", "init"], &[]); assert!(output.status.success(), "init should succeed"); - let _stdout = String::from_utf8_lossy(&output.stdout); + let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); let raw = if stdout.trim_start().starts_with('{') { @@ -4291,7 +4286,7 @@ fn init_json_envelope_has_hint_and_already_initialized_783() { // Idempotent re-init — already_initialized should be true let output2 = run_claw(&root, &["--output-format", "json", "init"], &[]); assert!(output2.status.success(), "re-init should succeed"); - let _stdout2 = String::from_utf8_lossy(&output2.stdout); + let stdout2 = String::from_utf8_lossy(&output2.stdout); let stderr2 = String::from_utf8_lossy(&output2.stderr); let stdout2 = String::from_utf8_lossy(&output2.stdout); let raw2 = if stdout2.trim_start().starts_with('{') { @@ -4372,7 +4367,7 @@ fn export_arg_errors_have_typed_kind_and_hint_784() { &[], ); assert!(!out1.status.success(), "--output with no value should fail"); - let _stderr1 = String::from_utf8_lossy(&out1.stderr); + let stderr1 = String::from_utf8_lossy(&out1.stderr); let stdout1 = String::from_utf8_lossy(&out1.stdout); let j1: serde_json::Value = stdout1 .lines() @@ -4399,7 +4394,7 @@ fn export_arg_errors_have_typed_kind_and_hint_784() { &[], ); assert!(!out2.status.success(), "extra positional should fail"); - let _stderr2 = String::from_utf8_lossy(&out2.stderr); + let stderr2 = String::from_utf8_lossy(&out2.stderr); let stdout2 = String::from_utf8_lossy(&out2.stdout); let j2: serde_json::Value = stdout2 .lines() @@ -4436,7 +4431,7 @@ fn unknown_subcommand_returns_typed_kind_785() { // "dump" is close enough to "dump-manifests" to trigger the typo suggestion path let output = run_claw(&root, &["--output-format", "json", "dump"], &[]); assert!(!output.status.success(), "unknown subcommand should fail"); - let _stderr = String::from_utf8_lossy(&output.stderr); + let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); let j: serde_json::Value = stdout .lines() @@ -4484,7 +4479,7 @@ fn dump_manifests_missing_dir_has_typed_kind_and_hint_786() { &[], ); assert!(!out1.status.success()); - let _stderr1 = String::from_utf8_lossy(&out1.stderr); + let stderr1 = String::from_utf8_lossy(&out1.stderr); let stdout1 = String::from_utf8_lossy(&out1.stdout); let j1: serde_json::Value = stdout1 .lines() @@ -4516,7 +4511,7 @@ fn dump_manifests_missing_dir_has_typed_kind_and_hint_786() { &[], ); assert!(!out2.status.success()); - let _stderr2 = String::from_utf8_lossy(&out2.stderr); + let stderr2 = String::from_utf8_lossy(&out2.stderr); let stdout2 = String::from_utf8_lossy(&out2.stdout); let j2: serde_json::Value = stdout2 .lines() @@ -4566,7 +4561,7 @@ fn resume_directory_path_returns_typed_kind_and_hint_787() { !output.status.success(), "resume with directory should fail" ); - let _stderr = String::from_utf8_lossy(&output.stderr); + let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); let j: serde_json::Value = stdout .lines() @@ -4616,7 +4611,7 @@ fn skills_show_not_found_emits_single_json_object_788() { assert!(!output.status.success(), "skills show unknown should fail"); // Skills handler emits JSON to stdout; the duplicate was on stderr from the main error path. // After fix: stdout has 1 JSON object, stderr has none (no duplicate). - let _stdout = String::from_utf8_lossy(&output.stdout); + let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); @@ -4778,7 +4773,7 @@ fn system_prompt_unknown_option_returns_typed_kind_790() { &[], ); assert!(!out1.status.success()); - let _stderr1 = String::from_utf8_lossy(&out1.stderr); + let stderr1 = String::from_utf8_lossy(&out1.stderr); let stdout1 = String::from_utf8_lossy(&out1.stdout); let j1: serde_json::Value = stdout1 .lines() @@ -4805,7 +4800,7 @@ fn system_prompt_unknown_option_returns_typed_kind_790() { &[], ); assert!(!out2.status.success()); - let _stderr2 = String::from_utf8_lossy(&out2.stderr); + let stderr2 = String::from_utf8_lossy(&out2.stderr); let stdout2 = String::from_utf8_lossy(&out2.stdout); let j2: serde_json::Value = stdout2 .lines() @@ -4843,7 +4838,7 @@ fn config_extra_args_have_non_null_hint_791() { &[], ); assert!(!out1.status.success()); - let _stderr1 = String::from_utf8_lossy(&out1.stderr); + let stderr1 = String::from_utf8_lossy(&out1.stderr); let stdout1 = String::from_utf8_lossy(&out1.stdout); let j1: serde_json::Value = stdout1 .lines() @@ -4877,7 +4872,7 @@ fn config_extra_args_have_non_null_hint_791() { &[], ); assert!(!out2.status.success()); - let _stderr2 = String::from_utf8_lossy(&out2.stderr); + let stderr2 = String::from_utf8_lossy(&out2.stderr); let stdout2 = String::from_utf8_lossy(&out2.stdout); let j2: serde_json::Value = stdout2 .lines() @@ -5063,7 +5058,7 @@ fn plugins_uninstall_not_found_has_hint_793() { "plugins uninstall not-found must exit non-zero (#793)" ); // Error envelope goes to stderr (propagated via ? to main error handler) - let _stderr = String::from_utf8_lossy(&output.stderr); + let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); let j: serde_json::Value = stdout .lines() @@ -5108,7 +5103,7 @@ fn plugins_install_not_found_path_returns_typed_kind_794() { !output.status.success(), "plugins install not-found-path must exit non-zero (#794)" ); - let _stderr = String::from_utf8_lossy(&output.stderr); + let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); let j: serde_json::Value = stdout .lines() @@ -5372,7 +5367,7 @@ fn agents_create_scaffolds_toml_and_lists_locally_431() { .iter() .any(|agent| { agent["name"] == "my-agent" - && Path::new(agent["path"].as_str().expect("listed agent path")) + && PathBuf::from(agent["path"].as_str().expect("listed agent path")) == fs::canonicalize(&agent_path).expect("canonical listed agent path") })); } @@ -5497,7 +5492,7 @@ fn plugins_extra_args_have_non_null_hint_797() { !output.status.success(), "plugins show with extra arg must exit non-zero (#797)" ); - let _stderr = String::from_utf8_lossy(&output.stderr); + let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); let j: serde_json::Value = stdout .lines() @@ -5563,7 +5558,7 @@ fn plugins_list_trailing_dash_text_error_stays_on_stderr_817() { String::from_utf8_lossy(&output.stdout) ); let stderr = String::from_utf8_lossy(&output.stderr); - let _stdout = String::from_utf8_lossy(&output.stdout); + let stdout = String::from_utf8_lossy(&output.stdout); assert!(stderr.contains("[error-kind: cli_parse]"), "{stderr}"); assert!( stderr.contains("unknown option for `claw plugins list`: --"), @@ -5588,7 +5583,7 @@ fn empty_prompt_has_non_null_hint_798() { !output.status.success(), "empty prompt must exit non-zero (#798)" ); - let _stderr = String::from_utf8_lossy(&output.stderr); + let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); let j: serde_json::Value = stdout .lines() diff --git a/rust/crates/tools/src/lib.rs b/rust/crates/tools/src/lib.rs index e39efd21..2ad7926a 100644 --- a/rust/crates/tools/src/lib.rs +++ b/rust/crates/tools/src/lib.rs @@ -506,17 +506,13 @@ pub fn mvp_tool_specs() -> Vec { }, ToolSpec { name: "read_file", - description: "Read a text file from the workspace. Large files come back one page at \ - a time: if the response has \"truncated\": true you have NOT seen the \ - whole file, and \"nextOffset\" is the line to pass as `offset` to \ - continue. Prefer grep_search to locate what you need over paging \ - through a big file, and never re-read a page you already have.", + description: "Read a text file from the workspace.", input_schema: json!({ "type": "object", "properties": { "path": { "type": "string" }, - "offset": { "type": "integer", "minimum": 0, "description": "First line to read (0-based). Pass the previous response's nextOffset to continue." }, - "limit": { "type": "integer", "minimum": 1, "description": "Maximum lines to return. Capped by a byte ceiling regardless." } + "offset": { "type": "integer", "minimum": 0 }, + "limit": { "type": "integer", "minimum": 1 } }, "required": ["path"], "additionalProperties": false @@ -10405,12 +10401,6 @@ mod tests { #[test] fn repl_executes_python_code() { - // Resolving the python runtime reads PATH, and PATH is process-global: the PowerShell - // tests below blank it while they run. Without this guard that races and this test - // intermittently fails with "python runtime not found" on a machine that has python. - let _guard = env_lock() - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); let result = execute_tool( "REPL", &json!({"language": "python", "code": "print(1 + 1)", "timeout_ms": 500}),