diff --git a/aimux-core/src/decision_model.rs b/aimux-core/src/decision_model.rs index 49a900fe6..0aeabddff 100644 --- a/aimux-core/src/decision_model.rs +++ b/aimux-core/src/decision_model.rs @@ -550,20 +550,6 @@ pub trait DecisionModel: Send + Sync { fn provider(&self) -> &str; fn model_id(&self) -> &str; fn capabilities(&self) -> DecisionCapabilities; - /// The provider snapshot written into decision recordings (#226): identity - /// plus connection metadata (base URL, credential *source*, capabilities, - /// redacted options). Never carries a credential value. - fn config_snapshot(&self) -> crate::recording::ProviderRecord { - let mut snapshot = - crate::recording::ProviderRecord::from_model(self.provider(), self.model_id()); - snapshot.profile = Some(serde_json::json!({"decision_capabilities": self.capabilities()})); - snapshot - } - /// Retry settings for [`decide`] (#226). A per-call `max_retries` - /// overrides only the retry count. - fn retry_config(&self) -> retry::RetryConfig { - retry::RetryConfig::default() - } /// Perform one provider attempt. /// # Errors /// Returns provider, transport or invalid response errors. @@ -575,8 +561,18 @@ pub trait DecisionModel: Send + Sync { /// Returns invalid request, unsupported capability, provider, invalid response, /// retry exhaustion, timeout or caller abort errors. pub async fn decide( + model: &dyn DecisionModel, + options: DecisionCallOptions, +) -> Result { + decide_with_retry_delay(model, options, None).await +} + +/// [`decide`] with an optional crate-internal override of the first retry +/// delay (tests only; `None` keeps the core default). +async fn decide_with_retry_delay( model: &dyn DecisionModel, mut options: DecisionCallOptions, + initial_retry_delay: Option, ) -> Result { options.validate()?; let capabilities = model.capabilities(); @@ -648,11 +644,14 @@ pub async fn decide( } let timeout = timeout::OperationTimeout::new(options.timeout.unwrap_or_default())?; let abort_signal = options.abort_signal.clone(); - let retries = retry::prepare_retries_with_config( - options.max_retries, - model.retry_config(), - abort_signal.clone(), - ); + let retries = retry::prepare_retries(options.max_retries, abort_signal.clone()); + #[cfg(test)] + let retries = match initial_retry_delay { + Some(delay) => retries.with_initial_delay(delay), + None => retries, + }; + #[cfg(not(test))] + let _ = initial_retry_delay; let context = crate::recording::recorder().map(|recorder| { let ctx = crate::recording::RecordingContext::new(crate::recording::new_call_id(), recorder); @@ -664,8 +663,10 @@ pub async fn decide( model.model_id(), &capabilities, ); - ctx.recorder - .record_provider(&ctx.call_id, &model.config_snapshot().redacted()); + ctx.recorder.record_provider( + &ctx.call_id, + &crate::recording::ProviderRecord::from_model(model.provider(), model.model_id()), + ); ctx }); let result = timeout::run( @@ -721,3 +722,112 @@ pub async fn decide( } result } + +#[cfg(test)] +mod retry_tests { + //! Decision retries use only the core call-level `max_retries` (A6 item 2). + //! The first retry delay is set to zero through the crate-internal + //! [`retry::PreparedRetries::with_initial_delay`], so no test waits. + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::time::Duration; + + use super::*; + use crate::error::{ApiCallError, RetryErrorReason}; + + struct FailingModel { + calls: AtomicUsize, + status: u16, + retryable: bool, + } + + #[async_trait] + impl DecisionModel for FailingModel { + fn provider(&self) -> &str { + "test" + } + fn model_id(&self) -> &str { + "test" + } + fn capabilities(&self) -> DecisionCapabilities { + DecisionCapabilities { + supports_images: false, + max_images: None, + supports_typed_choices: false, + rounding: DecisionRounding::default(), + probability_source: DecisionProbabilitySource::ModelEstimate, + supports_boolean: true, + supports_choice: true, + supports_score: true, + returns_distributions: false, + max_questions: None, + min_choices: None, + max_choices: None, + max_score_levels: None, + } + } + async fn do_decide(&self, _: &DecisionCallOptions) -> Result { + self.calls.fetch_add(1, Ordering::SeqCst); + Err(AiMuxError::ApiCall(Box::new(ApiCallError { + status_code: Some(self.status), + is_retryable: self.retryable, + ..ApiCallError::new("failure", "https://example.test", serde_json::json!({})) + }))) + } + } + + fn options(max_retries: Option) -> DecisionCallOptions { + let mut options: DecisionCallOptions = serde_json::from_value(serde_json::json!({ + "state": "text", + "questions": [{"id": "q", "type": "boolean", "instructions": "test"}] + })) + .unwrap(); + options.max_retries = max_retries; + options + } + + async fn run(status: u16, retryable: bool, max_retries: Option) -> (usize, AiMuxError) { + let model = FailingModel { + calls: AtomicUsize::new(0), + status, + retryable, + }; + let error = decide_with_retry_delay(&model, options(max_retries), Some(Duration::ZERO)) + .await + .unwrap_err(); + (model.calls.load(Ordering::SeqCst), error) + } + + #[tokio::test] + async fn call_level_max_retries_sets_exact_attempts_until_exhausted() { + for status in [429, 503, 529] { + for (max_retries, attempts) in [(Some(1), 2), (Some(3), 4), (None, 3)] { + let (calls, error) = run(status, true, max_retries).await; + assert_eq!( + calls, attempts, + "status {status}, max_retries {max_retries:?}" + ); + let AiMuxError::Retry(retry) = &error else { + panic!("expected retry exhaustion, got {error:?}"); + }; + assert_eq!(retry.reason, RetryErrorReason::MaxRetriesExceeded); + assert_eq!(retry.errors.len(), attempts); + let AiMuxError::ApiCall(last) = retry.last_error() else { + panic!("expected API error"); + }; + assert_eq!(last.status_code, Some(status)); + } + } + } + + #[tokio::test] + async fn zero_max_retries_and_non_retryable_errors_make_one_attempt() { + let (calls, error) = run(503, true, Some(0)).await; + assert_eq!(calls, 1); + assert!(matches!(&error, AiMuxError::ApiCall(e) if e.status_code == Some(503))); + for status in [401, 422, 499] { + let (calls, error) = run(status, false, Some(3)).await; + assert_eq!(calls, 1, "status {status}"); + assert!(matches!(&error, AiMuxError::ApiCall(e) if e.status_code == Some(status))); + } + } +} diff --git a/aimux-core/src/recording.rs b/aimux-core/src/recording.rs index b1c08b9f4..dd506b006 100644 --- a/aimux-core/src/recording.rs +++ b/aimux-core/src/recording.rs @@ -210,22 +210,6 @@ pub struct ProviderRecord { pub provider: String, /// `model.model_id()`,如 "gpt-4o"。 pub model_id: String, - /// Decision connection snapshot (#226, RFC-0038 录制): the provider base - /// URL. Informational only: replay takes the endpoint from the caller's - /// registry. Absent for every other operation. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub base_url: Option, - /// Where the credential came from, never its value: `"env:VAR"`, - /// `"explicit"` or `"none"` (#226). - #[serde(default, skip_serializing_if = "Option::is_none")] - pub api_key_source: Option, - /// Decision capabilities and default protocol (#226). - #[serde(default, skip_serializing_if = "Option::is_none")] - pub profile: Option, - /// Connection options (headers, retries); sensitive headers are redacted - /// when recorded (#226). - #[serde(default, skip_serializing_if = "Option::is_none")] - pub provider_options: Option, } impl ProviderRecord { @@ -236,10 +220,6 @@ impl ProviderRecord { provider_id: provider_id.to_string(), provider: provider.to_string(), model_id: model_id.to_string(), - base_url: None, - api_key_source: None, - profile: None, - provider_options: None, } } @@ -251,14 +231,6 @@ impl ProviderRecord { let provider_id = provider.split('.').next().unwrap_or_default(); Self::new(provider_id, provider, model_id) } - - /// The same record with sensitive keys in `provider_options` redacted: - /// what a recorder may write. - #[must_use] - pub fn redacted(mut self) -> Self { - self.provider_options = self.provider_options.map(redact_json); - self - } } /// ③ HTTP 侧:单次 attempt 的 wire 交换。 @@ -664,13 +636,6 @@ pub fn is_sensitive_key(name: &str) -> bool { || squashed.contains("apikey") } -/// Redact sensitive keys of a provider-options snapshot (#226): what -/// [`ProviderRecord::redacted`] applies, for providers building one. -#[must_use] -pub fn redact_provider_options(value: serde_json::Value) -> serde_json::Value { - redact_json(value) -} - /// 递归脱敏(JSON 中含敏感键的项值替换为 `[REDACTED]`)。 /// 覆盖 `CallOptions.headers`/`provider_options` 任意层级。 pub(crate) fn redact_json(value: serde_json::Value) -> serde_json::Value { @@ -2001,6 +1966,29 @@ mod tests { ) } + /// Recordings written before R.4 carried #226's `baseUrl` / `profile` / + /// `providerOptions` in the provider record. `ProviderRecord` is not + /// `deny_unknown_fields`, so they still load (and replay) as identity. + #[test] + fn provider_record_ignores_removed_snapshot_fields() { + let old = serde_json::json!({ + "providerId": "openai", + "provider": "openai", + "modelId": "gpt-6-luna", + "baseUrl": "https://api.openai.com/v1", + "apiKeySource": "env:OPENAI_API_KEY", + "profile": {"decision_capabilities": {}}, + "providerOptions": {"headers": {"x-api-key": "[REDACTED]"}} + }); + let record: ProviderRecord = serde_json::from_value(old).unwrap(); + assert_eq!(record.provider_id, "openai"); + assert_eq!(record.model_id, "gpt-6-luna"); + assert_eq!( + serde_json::to_value(&record).unwrap(), + serde_json::json!({"providerId": "openai", "provider": "openai", "modelId": "gpt-6-luna"}) + ); + } + #[test] fn gate_is_off_by_default() { assert!(recorder().is_none()); diff --git a/aimux-core/src/retry.rs b/aimux-core/src/retry.rs index 5ed5a7893..ddfe14837 100644 --- a/aimux-core/src/retry.rs +++ b/aimux-core/src/retry.rs @@ -15,31 +15,6 @@ const DEFAULT_INITIAL_DELAY_MS: u64 = 2_000; /// Default multiplier applied to the delay after each retry. const DEFAULT_BACKOFF_FACTOR: u32 = 2; -/// Retry settings a decision model may carry (#226, -/// [`DecisionModel::retry_config`](crate::DecisionModel::retry_config)). -/// -/// A per-call `max_retries` overrides only [`Self::max_retries`]; the -/// configured delay and backoff factor remain in effect. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct RetryConfig { - /// Maximum number of retries after the initial attempt. - pub max_retries: u32, - /// Initial delay between attempts. - pub initial_delay: Duration, - /// Multiplier applied to the delay after each retry. - pub backoff_factor: u32, -} - -impl Default for RetryConfig { - fn default() -> Self { - Self { - max_retries: DEFAULT_MAX_RETRIES, - initial_delay: Duration::from_millis(DEFAULT_INITIAL_DELAY_MS), - backoff_factor: DEFAULT_BACKOFF_FACTOR, - } - } -} - /// A prepared operation retry function and its resolved retry count. /// /// This is the Rust representation of the AI SDK's @@ -70,25 +45,16 @@ pub(crate) fn prepare_retries( } } -/// Bind a model's retry settings, applying a per-call retry-count override. -#[must_use] -pub(crate) fn prepare_retries_with_config( - max_retries: Option, - mut config: RetryConfig, - abort: Option, -) -> PreparedRetries { - if let Some(max_retries) = max_retries { - config.max_retries = max_retries; - } - PreparedRetries { - max_retries: config.max_retries, - initial_delay_ms: u64::try_from(config.initial_delay.as_millis()).unwrap_or(u64::MAX), - backoff_factor: u64::from(config.backoff_factor), - abort_signal: abort, +impl PreparedRetries { + /// Replace the delay before the first retry. Crate-internal: only this + /// crate's tests use it (zero delay); callers set nothing but `max_retries`. + #[cfg(test)] + #[must_use] + pub(crate) fn with_initial_delay(mut self, delay: Duration) -> Self { + self.initial_delay_ms = u64::try_from(delay.as_millis()).unwrap_or(u64::MAX); + self } -} -impl PreparedRetries { /// Retry a complete operation using API-call retryability and response hints. /// /// # Errors diff --git a/aimux-providers/src/decision_support.rs b/aimux-providers/src/decision_support.rs index 72e6dce0e..1353e317f 100644 --- a/aimux-providers/src/decision_support.rs +++ b/aimux-providers/src/decision_support.rs @@ -37,51 +37,6 @@ impl DecisionEndpoint { Ok(base.trim_end_matches('/').to_string()) } - fn headers(&self) -> &crate::shared::ProviderHeaders { - match self { - Self::OpenAI(config) => &config.headers, - Self::Compatible(config) => &config.headers, - } - } - - /// #226's decision connection snapshot: identity, base URL, credential - /// *source* and the connection headers/retries. The credential value is - /// never read; sensitive headers are redacted again when recorded. - pub(crate) fn snapshot( - &self, - provider: &str, - model_id: &str, - retry_config: aimux_core::retry::RetryConfig, - profile: Value, - ) -> aimux_core::recording::ProviderRecord { - use crate::shared::Credential; - let headers = self.headers(); - let mut record = aimux_core::recording::ProviderRecord::from_model(provider, model_id); - record.base_url = self.base_url().ok(); - record.api_key_source = Some(match &headers.credential { - Credential::None => "none".into(), - Credential::Explicit(_) => "explicit".into(), - Credential::Env { var, .. } => format!("env:{var}"), - }); - let mut wire_headers = serde_json::Map::new(); - for (name, value) in &headers.fixed { - wire_headers.insert(name.clone(), Value::String(value.clone())); - } - for (name, value) in headers.user.iter().flatten() { - if let Some(value) = value { - wire_headers.insert(name.clone(), Value::String(value.clone())); - } - } - record.provider_options = Some(aimux_core::recording::redact_provider_options( - serde_json::json!({ - "headers": wire_headers, - "max_retries": retry_config.max_retries, - }), - )); - record.profile = Some(profile); - record - } - async fn request_headers( &self, call_headers: Option<&HashMap>, diff --git a/aimux-providers/src/jev.rs b/aimux-providers/src/jev.rs index c498881d6..cf6f8384e 100644 --- a/aimux-providers/src/jev.rs +++ b/aimux-providers/src/jev.rs @@ -23,8 +23,6 @@ pub struct JevConfig { pub endpoint: String, pub headers: HashMap, pub probability_source: DecisionProbabilitySource, - /// Retry settings [`decide`] uses. - pub retry_config: aimux_core::retry::RetryConfig, } impl JevConfig { @@ -35,7 +33,6 @@ impl JevConfig { endpoint: "https://api.typesafe.ai/v1/systemone".into(), headers: HashMap::new(), probability_source: DecisionProbabilitySource::Native, - retry_config: aimux_core::retry::RetryConfig::default(), } } @@ -95,27 +92,6 @@ impl DecisionModel for JevDecisionModel { fn model_id(&self) -> &str { &self.model_id } - fn config_snapshot(&self) -> aimux_core::recording::ProviderRecord { - let mut snapshot = - aimux_core::recording::ProviderRecord::from_model(self.provider(), self.model_id()); - snapshot.base_url = Some(self.config.endpoint.clone()); - snapshot.api_key_source = Some( - if self.config.api_key.is_empty() { - "none" - } else { - "explicit" - } - .into(), - ); - snapshot.profile = Some(serde_json::json!({"decision_capabilities": self.capabilities()})); - snapshot.provider_options = Some(aimux_core::recording::redact_provider_options( - serde_json::json!({"headers": self.config.headers}), - )); - snapshot - } - fn retry_config(&self) -> aimux_core::retry::RetryConfig { - self.config.retry_config - } fn capabilities(&self) -> DecisionCapabilities { DecisionCapabilities { supports_images: false, diff --git a/aimux-providers/src/openai/decision.rs b/aimux-providers/src/openai/decision.rs index 0963d7d2a..4d926354f 100644 --- a/aimux-providers/src/openai/decision.rs +++ b/aimux-providers/src/openai/decision.rs @@ -14,7 +14,6 @@ use crate::decision_support::DecisionEndpoint; pub struct OpenAIDecisionModel { endpoint: DecisionEndpoint, model_id: String, - retry_config: aimux_core::retry::RetryConfig, } impl OpenAIDecisionModel { @@ -32,16 +31,8 @@ impl OpenAIDecisionModel { Ok(Self { endpoint: DecisionEndpoint::OpenAI(config), model_id: model_id.into(), - retry_config: aimux_core::retry::RetryConfig::default(), }) } - - /// Replace the retry settings [`decide`] uses for this model (#226). - #[must_use] - pub fn with_retry_config(mut self, retry_config: aimux_core::retry::RetryConfig) -> Self { - self.retry_config = retry_config; - self - } } fn text_description(description: &DecisionDescription) -> Result<&str, AiMuxError> { @@ -407,17 +398,6 @@ impl DecisionModel for OpenAIDecisionModel { fn model_id(&self) -> &str { &self.model_id } - fn retry_config(&self) -> aimux_core::retry::RetryConfig { - self.retry_config - } - fn config_snapshot(&self) -> aimux_core::recording::ProviderRecord { - self.endpoint.snapshot( - self.provider(), - self.model_id(), - self.retry_config, - json!({"decision_protocol":"openai_decisions", "decision_capabilities":self.capabilities()}), - ) - } fn capabilities(&self) -> DecisionCapabilities { DecisionCapabilities { supports_images: true, diff --git a/aimux-providers/src/openai/mod.rs b/aimux-providers/src/openai/mod.rs index af77a47d9..5694eef67 100644 --- a/aimux-providers/src/openai/mod.rs +++ b/aimux-providers/src/openai/mod.rs @@ -290,6 +290,22 @@ pub struct OpenAIProvider { } impl OpenAIProvider { + /// The same provider without a credential: no key is resolved and no + /// `Authorization` header is sent (`auth = none`). + pub(crate) fn without_credential(mut self) -> Self { + self.headers.credential = Credential::None; + self + } + + /// The same provider reading its default env key from `env` (tests). + #[cfg(test)] + pub(crate) fn with_env_source(mut self, env: crate::shared::EnvSource) -> Self { + if let Credential::Env { source, .. } = &mut self.headers.credential { + *source = env; + } + self + } + fn model_config(&self, method: &str) -> OpenAIModelConfig { let mut config = OpenAIModelConfig::fixed( format!("{}.{method}", self.name), diff --git a/aimux-providers/src/preset.rs b/aimux-providers/src/preset.rs index b3e9cb837..aa00f7b36 100644 --- a/aimux-providers/src/preset.rs +++ b/aimux-providers/src/preset.rs @@ -377,6 +377,7 @@ fn assemble( (AuthMode::ApiKey, None) => Credential::Env { var: descriptor.env_var.to_string(), description: descriptor.display.to_string(), + source: crate::shared::EnvSource::Process, }, (AuthMode::None, None) => Credential::None, }; diff --git a/aimux-providers/src/provider_handle.rs b/aimux-providers/src/provider_handle.rs index ef55cf797..97acba310 100644 --- a/aimux-providers/src/provider_handle.rs +++ b/aimux-providers/src/provider_handle.rs @@ -1,32 +1,30 @@ //! #226's by-name provider handle for native OpenAI and the official local -//! decision runtimes, kept with #226's resolution rules (explicit argument, -//! then the runtime's `*_BASE_URL` / `*_API_KEY` environment variables, then -//! the default local endpoint) on top of the package factories. +//! decision runtimes on top of the package factories. A key is only the +//! explicit argument or a package's own default env (`OPENAI_API_KEY`, read +//! at request time); there is no generic `_API_KEY` lookup (A6 item 4). +//! A local runtime's base URL is the argument, then `*_BASE_URL`, then the +//! default local endpoint. //! //! Other names resolve through [`create_provider`](crate::create_provider). use std::collections::HashMap; use std::sync::Arc; -use aimux_core::decision_model::{ - DecisionCallOptions, DecisionCapabilities, DecisionModel, DecisionResult, -}; +use aimux_core::decision_model::DecisionModel; use aimux_core::provider::ProviderDiscovery; -use aimux_core::retry::RetryConfig; use aimux_core::{ AiMuxError, EmbeddingModel, EvaluationModel, Files, ImageModel, LanguageModel, Provider, RerankingModel, SearchModel, Skills, SpeechModel, TranscriptionModel, VideoModel, }; use aimux_provider_utils::HeaderMapOpt; -use async_trait::async_trait; use serde::{Deserialize, Serialize}; -use serde_json::Value; use crate::openai::OpenAIProviderSettings; use crate::openai_compatible::{OpenAICompatibleProviderSettings, create_openai_compatible}; /// Per-handle options of [`provider_handle`] (#226). #[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct ProviderOptions { /// Override the default or environment base URL. pub base_url: Option, @@ -36,11 +34,6 @@ pub struct ProviderOptions { pub organization: Option, /// OpenAI project ID (`OpenAI-Project` header). pub project: Option, - /// Decision retry-count default; a per-call `max_retries` still wins. - pub max_retries: Option, - /// Generation request-body overrides. Native decisions refuse a handle - /// that carries them (#226). - pub body_overrides: Option, } /// Official local decision runtimes: name, default base URL, base URL env. @@ -66,19 +59,46 @@ fn non_empty_env(name: &str) -> Option { /// (`ollama`, `llamacpp`, `localai`, `vllm`, `sglang`, `laya`) or any other /// built-in package / registry preset. /// -/// - `openai`: `api_key = None` reads `OPENAI_API_KEY` now. -/// - local runtimes: `api_key = None` reads `{NAME}_API_KEY` if set, otherwise -/// sends the placeholder key `not-needed`; the base URL is `options.base_url`, else `{NAME}_BASE_URL`, -/// else the runtime's default local endpoint. +/// - `openai`: `api_key = None` reads the package default `OPENAI_API_KEY` at +/// request time (#200:538); `Some("")` is used as given and never falls +/// back (RFC-0040:67). With `options.base_url` set and no key, no env is +/// read and no credential is sent. +/// - local runtimes: `api_key = None` sends the placeholder key `not-needed`; +/// no `{NAME}_API_KEY` is read. The base URL is `options.base_url`, else +/// `{NAME}_BASE_URL`, else the runtime's default local endpoint. /// /// # Errors /// -/// `LoadApiKey` for a missing OpenAI key, `NoSuchProvider` for an unknown -/// name, and the factory's settings errors. +/// `NoSuchProvider` for an unknown name, and the factory's settings errors. +/// A missing OpenAI key fails the request with `LoadApiKey`. pub fn provider_handle( name: &str, api_key: Option, options: Option, +) -> Result, AiMuxError> { + provider_handle_with( + name, + api_key, + options, + #[cfg(test)] + Handles::default(), + ) +} + +/// Test-only injection for [`provider_handle_with`]: the env source of a +/// package-default key and the transport. Not compiled into the library. +#[cfg(test)] +#[derive(Default)] +struct Handles { + env: crate::shared::EnvSource, + fetch: Option, +} + +fn provider_handle_with( + name: &str, + api_key: Option, + options: Option, + #[cfg(test)] handles: Handles, ) -> Result, AiMuxError> { let options = options.unwrap_or_default(); let mut headers: HeaderMapOpt = options @@ -88,18 +108,25 @@ pub fn provider_handle( .into_iter() .map(|(name, value)| (name, Some(value))) .collect(); - let mut api_key_source = None; let inner: Arc = if name == "openai" { - let api_key = - aimux_provider_utils::load_api_key(api_key.as_deref(), "OPENAI_API_KEY", "OpenAI")?; - Arc::new(crate::create_openai(OpenAIProviderSettings { + let auth_none = api_key.is_none() && options.base_url.is_some(); + let provider = crate::create_openai(OpenAIProviderSettings { base_url: options.base_url.clone(), - api_key: Some(api_key), + api_key, organization: options.organization.clone(), project: options.project.clone(), headers: Some(headers), + #[cfg(test)] + fetch: handles.fetch, ..Default::default() - })?) + })?; + #[cfg(test)] + let provider = provider.with_env_source(handles.env); + Arc::new(if auth_none { + provider.without_credential() + } else { + provider + }) } else { if let Some(organization) = &options.organization { headers.insert("OpenAI-Organization".into(), Some(organization.clone())); @@ -115,21 +142,9 @@ pub fn provider_handle( .clone() .or_else(|| non_empty_env(url_env)) .unwrap_or_else(|| (*default_url).into()); - // #226: explicit key, else `{NAME}_API_KEY`, else the placeholder - // `not-needed` (sent as `Authorization: Bearer not-needed`), - // recorded with source `none`. - let key_env = format!("{}_API_KEY", name.to_uppercase()); - let api_key = match (api_key, non_empty_env(&key_env)) { - (Some(key), _) => key, - (None, Some(key)) => { - api_key_source = Some(format!("env:{key_env}")); - key - } - (None, None) => { - api_key_source = Some("none".into()); - PLACEHOLDER_API_KEY.into() - } - }; + // Explicit key, else the placeholder `not-needed` (sent as + // `Authorization: Bearer not-needed`). No `{NAME}_API_KEY` is read. + let api_key = api_key.unwrap_or_else(|| PLACEHOLDER_API_KEY.into()); Arc::new(create_openai_compatible( OpenAICompatibleProviderSettings { name: name.into(), @@ -151,67 +166,25 @@ pub fn provider_handle( )? } }; - Ok(Box::new(ProviderHandle { - inner, - max_retries: options.max_retries, - has_body_overrides: options.body_overrides.is_some(), - api_key_source, - })) + Ok(Box::new(ProviderHandle { inner })) } -/// A provider with #226's handle-level decision settings. +/// The resolved provider behind a by-name handle. struct ProviderHandle { inner: Arc, - max_retries: Option, - has_body_overrides: bool, - /// Credential source when the handle resolved the key itself (the - /// placeholder or an environment variable); `None` keeps the model's own. - api_key_source: Option, -} - -impl ProviderHandle { - fn generation_overrides_unsupported(&self) -> Result<(), AiMuxError> { - if self.has_body_overrides { - return Err(AiMuxError::UnsupportedFunctionality( - "body_overrides are not supported by the package model factories".into(), - )); - } - Ok(()) - } } impl Provider for ProviderHandle { fn decision_model(&self, id: &str) -> Result, AiMuxError> { - if self.has_body_overrides { - return Err(AiMuxError::UnsupportedFunctionality( - "native decisions require an official adapter without generation body_overrides" - .into(), - )); - } - let model = self.inner.decision_model(id)?; - if self.max_retries.is_none() && self.api_key_source.is_none() { - return Ok(model); - } - let retry_config = RetryConfig { - max_retries: self.max_retries.unwrap_or(model.retry_config().max_retries), - ..model.retry_config() - }; - Ok(Box::new(HandleDecisionModel { - inner: model, - retry_config, - api_key_source: self.api_key_source.clone(), - })) + self.inner.decision_model(id) } fn language_model(&self, id: &str) -> Result, AiMuxError> { - self.generation_overrides_unsupported()?; self.inner.language_model(id) } fn embedding_model(&self, id: &str) -> Result, AiMuxError> { - self.generation_overrides_unsupported()?; self.inner.embedding_model(id) } fn image_model(&self, id: &str) -> Result, AiMuxError> { - self.generation_overrides_unsupported()?; self.inner.image_model(id) } fn transcription_model( @@ -246,38 +219,295 @@ impl Provider for ProviderHandle { } } -/// A decision model with handle-level retry settings and credential source. -struct HandleDecisionModel { - inner: Box, - retry_config: RetryConfig, - api_key_source: Option, -} +#[cfg(test)] +mod env_tests { + //! A6 item 4 / D23. The env a key could come from is an injected plain + //! map ([`EnvSource::Vars`]) that logs every read, never the process + //! environment and never `std::env::set_var`. + use std::sync::{Arc, Mutex}; + + use aimux_core::decision_model::{DecisionCallOptions, decide}; + use aimux_core::recording::{self, RingRecorder}; + use aimux_core::replay::MockDecisionReplayModel; + use aimux_provider_utils::{ + Fetch, FetchError, FetchFunction, FetchRequest, FetchResponse, ReqwestFetch, + }; + use async_trait::async_trait; + use serde_json::Value; + use wiremock::matchers::{method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; -#[async_trait] -impl DecisionModel for HandleDecisionModel { - fn provider(&self) -> &str { - self.inner.provider() + use super::*; + use crate::shared::{EnvSource, TestEnv}; + + fn env(vars: &[(&str, &str)]) -> Arc> { + Arc::new(Mutex::new(TestEnv { + vars: vars + .iter() + .map(|(k, v)| ((*k).to_string(), (*v).to_string())) + .collect(), + reads: Vec::new(), + })) } - fn model_id(&self) -> &str { - self.inner.model_id() + + fn reads(env: &Arc>) -> Vec { + env.lock().unwrap().reads.clone() } - fn capabilities(&self) -> DecisionCapabilities { - self.inner.capabilities() + + /// Sends every request to `target`'s origin, keeping path and headers, so + /// the OpenAI handle keeps its default base URL. + struct Redirect(String); + + #[async_trait] + impl Fetch for Redirect { + async fn fetch(&self, mut request: FetchRequest) -> Result { + let target = url::Url::parse(&self.0).unwrap(); + request.url.set_scheme(target.scheme()).unwrap(); + request.url.set_host(target.host_str()).unwrap(); + request.url.set_port(target.port()).unwrap(); + ReqwestFetch.fetch(request).await + } } - fn retry_config(&self) -> RetryConfig { - self.retry_config + + fn fixture(name: &str) -> Value { + let text = match name { + "openai" => include_str!("../tests/fixtures/openai_decisions.json"), + _ => include_str!("../tests/fixtures/runtime_decisions.json"), + }; + serde_json::from_str(text).unwrap() } - fn config_snapshot(&self) -> aimux_core::recording::ProviderRecord { - let mut snapshot = self.inner.config_snapshot(); - if let Some(source) = &self.api_key_source { - snapshot.api_key_source = Some(source.clone()); - } - if let Some(serde_json::Value::Object(options)) = &mut snapshot.provider_options { - options.insert("max_retries".into(), self.retry_config.max_retries.into()); + + fn options(name: &str) -> DecisionCallOptions { + let mut options: DecisionCallOptions = + serde_json::from_value(fixture(name)["options"].clone()).unwrap(); + options.max_retries = Some(0); + options + } + + async fn mock(route: &str, name: &str) -> MockServer { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path(route)) + .respond_with(ResponseTemplate::new(200).set_body_json(&fixture(name)["response"])) + .mount(&server) + .await; + server + } + + async fn authorization(server: &MockServer) -> Vec> { + server + .received_requests() + .await + .unwrap() + .iter() + .map(|request| { + request + .headers + .get("authorization") + .map(|v| v.to_str().unwrap().to_string()) + }) + .collect() + } + + fn openai( + api_key: Option<&str>, + options: Option, + env: &Arc>, + server: &MockServer, + ) -> Box { + let fetch: FetchFunction = Arc::new(Redirect(server.uri())); + provider_handle_with( + "openai", + api_key.map(Into::into), + options, + Handles { + env: EnvSource::Vars(env.clone()), + fetch: Some(fetch), + }, + ) + .unwrap() + } + + /// (1) The generic `_API_KEY` is gone: a local runtime without a key + /// never looks one up, so `OLLAMA_API_KEY` / `VLLM_API_KEY` never reach + /// the wire. + #[tokio::test] + async fn generic_provider_api_key_env_is_not_read() { + for name in ["ollama", "vllm"] { + let server = mock("/v1/systemone", "runtime").await; + let env = env(&[("OLLAMA_API_KEY", "x-ollama"), ("VLLM_API_KEY", "x-vllm")]); + let handle = provider_handle_with( + name, + None, + Some(ProviderOptions { + base_url: Some(format!("{}/v1", server.uri())), + ..Default::default() + }), + Handles { + env: EnvSource::Vars(env.clone()), + fetch: None, + }, + ) + .unwrap(); + decide( + handle.decision_model("served").unwrap().as_ref(), + options("runtime"), + ) + .await + .unwrap(); + let sent = authorization(&server).await; + assert_eq!(sent.len(), 1); + let sent = sent[0].clone().unwrap_or_default(); + assert!( + !sent.contains("x-ollama") && !sent.contains("x-vllm"), + "{sent}" + ); + assert!(reads(&env).is_empty(), "{name}: {:?}", reads(&env)); } - snapshot } - async fn do_decide(&self, options: &DecisionCallOptions) -> Result { - self.inner.do_decide(options).await + + /// (2) The OpenAI package default env is read at request time: the key is + /// put into the env only after the handle and the model exist. + #[tokio::test] + async fn openai_default_env_is_read_at_request_time() { + let server = mock("/v1/decisions", "openai").await; + let env = env(&[]); + let handle = openai(None, None, &env, &server); + let model = handle.decision_model("gpt-6-luna").unwrap(); + assert!(reads(&env).is_empty(), "read at construction"); + env.lock() + .unwrap() + .vars + .insert("OPENAI_API_KEY".into(), "sk-set-after-construction".into()); + decide(model.as_ref(), options("openai")).await.unwrap(); + assert_eq!( + authorization(&server).await, + vec![Some("Bearer sk-set-after-construction".into())] + ); + assert_eq!(reads(&env), vec!["OPENAI_API_KEY".to_string()]); + } + + /// Unset default env fails the request (not the construction) with + /// `LoadApiKey`, and nothing is sent. + #[tokio::test] + async fn openai_missing_default_env_fails_the_request() { + let server = mock("/v1/decisions", "openai").await; + let env = env(&[]); + let handle = openai(None, None, &env, &server); + let model = handle.decision_model("gpt-6-luna").unwrap(); + let error = decide(model.as_ref(), options("openai")).await.unwrap_err(); + assert!(matches!(error, AiMuxError::LoadApiKey { .. }), "{error:?}"); + assert!(authorization(&server).await.is_empty()); + } + + /// (3) `Some("")` is used as given and never falls back to the env. + #[tokio::test] + async fn explicit_empty_key_does_not_fall_back_to_env() { + let server = mock("/v1/decisions", "openai").await; + let env = env(&[("OPENAI_API_KEY", "sk-from-env")]); + let handle = openai(Some(""), None, &env, &server); + decide( + handle.decision_model("gpt-6-luna").unwrap().as_ref(), + options("openai"), + ) + .await + .unwrap(); + let sent = authorization(&server).await; + assert_eq!(sent.len(), 1); + assert!( + !sent[0].clone().unwrap_or_default().contains("sk-from-env"), + "{sent:?}" + ); + assert!(reads(&env).is_empty(), "{:?}", reads(&env)); + } + + /// (4a) A changed base URL without a key reads no env and sends no + /// credential. + #[tokio::test] + async fn changed_base_url_does_not_read_env() { + let server = mock("/v1/decisions", "openai").await; + let env = env(&[("OPENAI_API_KEY", "sk-from-env")]); + let handle = openai( + None, + Some(ProviderOptions { + base_url: Some(format!("{}/v1", server.uri())), + ..Default::default() + }), + &env, + &server, + ); + decide( + handle.decision_model("gpt-6-luna").unwrap().as_ref(), + options("openai"), + ) + .await + .unwrap(); + assert_eq!(authorization(&server).await, vec![None]); + assert!(reads(&env).is_empty(), "{:?}", reads(&env)); + } + + /// (4b) `auth = none` (a keyless local runtime) reads no env. + #[tokio::test] + async fn auth_none_does_not_read_env() { + let server = mock("/v1/systemone", "runtime").await; + let env = env(&[("OPENAI_API_KEY", "sk-from-env"), ("LAYA_API_KEY", "x")]); + let handle = provider_handle_with( + "laya", + None, + Some(ProviderOptions { + base_url: Some(format!("{}/v1", server.uri())), + ..Default::default() + }), + Handles { + env: EnvSource::Vars(env.clone()), + fetch: None, + }, + ) + .unwrap(); + decide( + handle.decision_model("served").unwrap().as_ref(), + options("runtime"), + ) + .await + .unwrap(); + assert!(reads(&env).is_empty(), "{:?}", reads(&env)); + } + + /// (4c) Offline replay of a recorded decision reads no env and makes no + /// request, even though the live handle reads its key from the env. + #[tokio::test] + async fn offline_replay_does_not_read_env() { + let server = mock("/v1/decisions", "openai").await; + let env = env(&[("OPENAI_API_KEY", "sk-from-env")]); + let handle = openai(None, None, &env, &server); + // The recorder is process-global and the other tests here run in + // parallel, so this test keeps only the recording of its own call, + // told apart by a unique `state`. + let mut request = options("openai"); + request.state = serde_json::json!({ "test": "offline_replay_does_not_read_env" }); + let recorder = Arc::new(RingRecorder::new()); + recording::init_recording(Some(recorder.clone())); + let live = decide( + handle.decision_model("gpt-6-luna").unwrap().as_ref(), + request.clone(), + ) + .await; + recording::init_recording(None); + live.unwrap(); + let records: Vec<_> = recorder + .completed() + .into_iter() + .filter(|record| { + serde_json::to_string(&record.input) + .unwrap() + .contains("offline_replay_does_not_read_env") + }) + .collect(); + assert_eq!(records.len(), 1); + env.lock().unwrap().reads.clear(); + let replay = MockDecisionReplayModel::new("openai", "gpt-6-luna", records).unwrap(); + decide(&replay, request).await.unwrap(); + assert!(reads(&env).is_empty(), "{:?}", reads(&env)); + assert_eq!(server.received_requests().await.unwrap().len(), 1); } } diff --git a/aimux-providers/src/runtime_decision.rs b/aimux-providers/src/runtime_decision.rs index f6f454897..36888b35a 100644 --- a/aimux-providers/src/runtime_decision.rs +++ b/aimux-providers/src/runtime_decision.rs @@ -29,7 +29,6 @@ pub(crate) struct RuntimeDecisionModel { /// what [`DecisionModel::provider`] reports. pub(crate) vendor: String, pub(crate) model_id: String, - pub(crate) retry_config: aimux_core::retry::RetryConfig, } impl RuntimeDecisionModel { @@ -52,7 +51,6 @@ impl RuntimeDecisionModel { endpoint, vendor: vendor.into(), model_id: model_id.into(), - retry_config: aimux_core::retry::RetryConfig::default(), }) } @@ -634,17 +632,6 @@ impl DecisionModel for RuntimeDecisionModel { fn model_id(&self) -> &str { &self.model_id } - fn retry_config(&self) -> aimux_core::retry::RetryConfig { - self.retry_config - } - fn config_snapshot(&self) -> aimux_core::recording::ProviderRecord { - self.endpoint.snapshot( - self.provider(), - self.model_id(), - self.retry_config, - json!({"decision_default_protocol":"systemone", "decision_capabilities":self.capabilities()}), - ) - } fn capabilities(&self) -> DecisionCapabilities { let provider = self.provider(); DecisionCapabilities { diff --git a/aimux-providers/src/shared/mod.rs b/aimux-providers/src/shared/mod.rs index 3de9b6dac..d7b6c33c5 100644 --- a/aimux-providers/src/shared/mod.rs +++ b/aimux-providers/src/shared/mod.rs @@ -37,6 +37,47 @@ pub(crate) fn is_valid_hostname_part(part: &str) -> bool { && bytes[bytes.len() - 1] != b'-' } +/// Where a [`Credential::Env`] value is read from, on every request. Outside +/// this crate's tests it is always the process environment. +#[derive(Clone, Debug, Default)] +pub(crate) enum EnvSource { + /// The process environment. + #[default] + Process, + /// A plain map injected by this crate's tests, which also logs every + /// variable read so a test can assert that nothing was read. + #[cfg(test)] + Vars(std::sync::Arc>), +} + +/// The injected environment of [`EnvSource::Vars`]: plain data. +#[cfg(test)] +#[derive(Debug, Default)] +pub(crate) struct TestEnv { + pub vars: std::collections::HashMap, + pub reads: Vec, +} + +impl EnvSource { + fn api_key(&self, var: &str, description: &str) -> Result { + match self { + Self::Process => load_api_key(None, var, description), + #[cfg(test)] + Self::Vars(env) => { + let mut env = env.lock().expect("test env lock"); + env.reads.push(var.to_string()); + env.vars + .get(var) + .cloned() + .ok_or_else(|| AiMuxError::LoadApiKey { + env_var: var.to_string(), + description: description.to_string(), + }) + } + } + } +} + /// Where the credential of a provider comes from. #[derive(Clone, Debug)] pub(crate) enum Credential { @@ -49,7 +90,11 @@ pub(crate) enum Credential { Explicit(Resolvable), /// Read the environment variable on every request; unset fails that /// request with `AiMuxError::LoadApiKey`. - Env { var: String, description: String }, + Env { + var: String, + description: String, + source: EnvSource, + }, } impl Credential { @@ -64,6 +109,7 @@ impl Credential { None => Self::Env { var: var.to_string(), description: description.to_string(), + source: EnvSource::Process, }, } } @@ -75,7 +121,11 @@ impl Credential { match self { Self::None => Ok(None), Self::Explicit(key) => Ok(Some(key.resolve().await?)), - Self::Env { var, description } => Ok(Some(load_api_key(None, var, description)?)), + Self::Env { + var, + description, + source, + } => Ok(Some(source.api_key(var, description)?)), } } } diff --git a/aimux-providers/tests/decision_handle_test.rs b/aimux-providers/tests/decision_handle_test.rs index bbf6d5a71..3181213ab 100644 --- a/aimux-providers/tests/decision_handle_test.rs +++ b/aimux-providers/tests/decision_handle_test.rs @@ -35,36 +35,33 @@ fn decision_providers_resolve_by_name() { ); } +/// A6 item 2: retry is call-level only, so the handle has no `max_retries`. #[test] -fn handle_options_reach_decisions() { - let handle = provider_handle( - "laya", - None, - Some(ProviderOptions { - max_retries: Some(0), - ..Default::default() - }), - ) - .unwrap(); - let model = handle.decision_model("served-alias").unwrap(); - assert_eq!(model.retry_config().max_retries, 0); - let overridden = provider_handle( - "vllm", - None, - Some(ProviderOptions { - body_overrides: Some(json!({"temperature": 0})), - ..Default::default() - }), - ) - .unwrap(); - assert!(matches!( - overridden.decision_model("served-alias"), - Err(aimux_core::AiMuxError::UnsupportedFunctionality(_)) - )); +fn handle_has_no_max_retries_option() { + let error = serde_json::from_value::(json!({"max_retries": 0})) + .unwrap_err() + .to_string(); + assert!(error.contains("unknown field `max_retries`"), "{error}"); } +/// `body_overrides` no longer exists (A6 item 3): it is an unknown field like +/// any other, not a specially rejected one. #[test] -fn config_snapshot_matches_model_identity() { +fn body_overrides_is_an_unknown_field() { + let error = serde_json::from_value::(json!({ + "body_overrides": {"temperature": 0} + })) + .unwrap_err() + .to_string(); + assert!(error.contains("unknown field `body_overrides`"), "{error}"); + let other = serde_json::from_value::(json!({"not_a_field": 1})) + .unwrap_err() + .to_string(); + assert!(other.contains("unknown field `not_a_field`"), "{other}"); +} + +#[test] +fn recorded_identity_matches_model_identity() { for (name, key) in [ ("openai", Some("sk-test")), ("vllm", Some("sk-test")), @@ -74,15 +71,16 @@ fn config_snapshot_matches_model_identity() { .unwrap() .decision_model("served-alias") .unwrap(); - let snap = model.config_snapshot(); + // What `decide` records: provider name + model name only (A6 item 1). + let snap = + aimux_core::recording::ProviderRecord::from_model(model.provider(), model.model_id()); assert_eq!(snap.provider, model.provider()); assert_eq!(snap.provider_id, name); assert_eq!(snap.model_id, model.model_id()); - assert!(snap.base_url.is_some()); - assert_eq!( - snap.api_key_source.as_deref(), - Some(if key.is_some() { "explicit" } else { "none" }) - ); + let json = serde_json::to_value(&snap).unwrap(); + assert!(json.get("apiKeySource").is_none(), "{json}"); + assert!(json.get("baseUrl").is_none(), "{json}"); + let _ = key; } } @@ -143,15 +141,11 @@ async fn recorded_decision_file_contains_no_raw_credentials() { let record: Value = serde_json::from_str(written.lines().next().unwrap()).unwrap(); let provider = &record["provider"]; assert_eq!(provider["providerId"], "openai"); - assert_eq!(provider["apiKeySource"], "explicit"); - assert_eq!( - provider["providerOptions"]["headers"]["x-api-key"], - "[REDACTED]" - ); - assert_eq!( - provider["providerOptions"]["headers"]["X-Client"], - "decision-tests" - ); + assert_eq!(provider["modelId"], "gpt-6-luna"); + // Identity only (A6 item 1): no credential source, base URL or headers. + assert!(provider.get("apiKeySource").is_none(), "{provider}"); + assert!(provider.get("baseUrl").is_none(), "{provider}"); + assert!(provider.get("providerOptions").is_none(), "{provider}"); println!( "checked {} ({} bytes): no raw Authorization / x-api-key values", path.display(), @@ -160,7 +154,7 @@ async fn recorded_decision_file_contains_no_raw_credentials() { } /// #226 (D15): a keyless local runtime sends the placeholder credential -/// `Authorization: Bearer not-needed`; the snapshot records source `none`. +/// `Authorization: Bearer not-needed`. #[tokio::test] #[serial] async fn keyless_local_runtime_sends_placeholder_bearer() { @@ -180,10 +174,6 @@ async fn keyless_local_runtime_sends_placeholder_bearer() { .unwrap() .decision_model("served-alias") .unwrap(); - assert_eq!( - model.config_snapshot().api_key_source.as_deref(), - Some("none") - ); let mut options: DecisionCallOptions = serde_json::from_value( serde_json::from_str::(include_str!("fixtures/runtime_decisions.json")).unwrap() @@ -208,7 +198,7 @@ async fn keyless_local_runtime_sends_placeholder_bearer() { } /// Explicit `Authorization` and `x-api-key` request headers reach the wire but -/// are recorded under their names with the value `[REDACTED]`. +/// their values never appear in the recording. #[tokio::test] #[serial] async fn recorded_explicit_auth_headers_are_redacted() { @@ -255,7 +245,10 @@ async fn recorded_explicit_auth_headers_are_redacted() { assert!(!written.contains(secret), "{secret} leaked: {written}"); } let record: Value = serde_json::from_str(written.lines().next().unwrap()).unwrap(); - let headers = &record["provider"]["providerOptions"]["headers"]; - assert_eq!(headers["Authorization"], "[REDACTED]"); - assert_eq!(headers["x-api-key"], "[REDACTED]"); + // The provider record is identity only (A6 item 1): the connection + // headers are no longer copied into it at all. + assert!( + record["provider"].get("providerOptions").is_none(), + "{record}" + ); } diff --git a/aimux-providers/tests/decision_recording_test.rs b/aimux-providers/tests/decision_recording_test.rs index 37dab216b..b21e0fbba 100644 --- a/aimux-providers/tests/decision_recording_test.rs +++ b/aimux-providers/tests/decision_recording_test.rs @@ -82,22 +82,17 @@ async fn ring_records_one_complete_call_and_replays_exact_inputs_with_redaction( assert_eq!(records[0].input.operation, RecordingOperation::Decision); assert!(records[0].input.prompt.is_empty()); assert!(records[0].input.decision_capabilities.is_some()); - // Identity plus #226's connection snapshot (credential source only). + // Identity only (A6 item 1): no credential source or base URL. let provider = serde_json::to_value(&records[0].provider).unwrap(); for (key, value) in [ ("providerId", "jev"), ("provider", "jev"), ("modelId", "jev-latest"), - ("apiKeySource", "explicit"), ] { assert_eq!(provider[key], json!(value), "{key}"); } - assert!( - provider["baseUrl"] - .as_str() - .unwrap() - .ends_with("/v1/systemone") - ); + assert!(provider.get("apiKeySource").is_none(), "{provider}"); + assert!(provider.get("baseUrl").is_none(), "{provider}"); assert_eq!(records[0].exchanges.len(), 1); assert_eq!(records[0].exchanges[0].attempt, 1); assert_eq!(records[0].exchanges[0].exchange_index, 1); diff --git a/aimux-providers/tests/jev_test.rs b/aimux-providers/tests/jev_test.rs index 652b861a5..d57a2d21c 100644 --- a/aimux-providers/tests/jev_test.rs +++ b/aimux-providers/tests/jev_test.rs @@ -1,6 +1,5 @@ //! Offline contract tests using official examples and recorded live responses. //! No live credentials or external requests are needed to replay them. -use std::time::Duration; use aimux_core::decision_model::*; use aimux_core::{AiMuxError, Provider}; @@ -153,9 +152,7 @@ fn options() -> DecisionCallOptions { } fn model(server: &MockServer) -> Box { - let mut config = - JevConfig::new("test-key").with_endpoint(format!("{}/v1/systemone", server.uri())); - config.retry_config.initial_delay = Duration::ZERO; + let config = JevConfig::new("test-key").with_endpoint(format!("{}/v1/systemone", server.uri())); JevProvider::new(config) .decision_model("jev-latest") .unwrap() @@ -355,15 +352,20 @@ async fn invalid_requests_never_reach_http() { assert!(server.received_requests().await.unwrap().is_empty()); } +/// Jev maps statuses to the standard retry rules. Retry counting itself is +/// covered without delay in aimux-core (`decision_model::retry_tests`); here +/// classification uses `max_retries = 0` (no backoff wait) and a +/// `max_retries = 1` run per retryable status (429, 529, 503) proves it is +/// retried and then exhausted (one real default backoff wait each). #[tokio::test] async fn official_provider_failures_use_standard_retry_rules_and_preserve_codes() { - for (status, code, attempts) in [ - (401, "authentication_error", 1), - (422, "validation_error", 1), - (429, "rate_limit", 3), - (529, "overloaded", 3), - (503, "unavailable", 3), - (499, "cancelled", 1), + for (status, code, retryable) in [ + (401, "authentication_error", false), + (422, "validation_error", false), + (429, "rate_limit", true), + (529, "overloaded", true), + (503, "unavailable", true), + (499, "cancelled", false), ] { let server = MockServer::start().await; mount( @@ -372,19 +374,52 @@ async fn official_provider_failures_use_standard_retry_rules_and_preserve_codes( json!({"error":{"message":"test failure", "code":code}}), ) .await; - let error = decide(model(&server).as_ref(), options()) - .await - .unwrap_err(); - let error = match error { - AiMuxError::Retry(error) => error.last_error().clone(), - error => error, - }; - let AiMuxError::ApiCall(error) = error else { + let mut request = options(); + request.max_retries = Some(if retryable { 0 } else { 2 }); + let AiMuxError::ApiCall(error) = + decide(model(&server).as_ref(), request).await.unwrap_err() + else { panic!("expected API error") }; assert_eq!(error.status_code, Some(status)); assert_eq!(error.provider_code.as_deref(), Some(code)); - assert_eq!(server.received_requests().await.unwrap().len(), attempts); + assert_eq!(error.is_retryable, retryable, "status {status}"); + assert_eq!(server.received_requests().await.unwrap().len(), 1); + } + + for (status, code) in [ + (429, "rate_limit"), + (529, "overloaded"), + (503, "unavailable"), + ] { + let server = MockServer::start().await; + mount( + &server, + status, + json!({"error":{"message":"test failure", "code":code}}), + ) + .await; + let mut request = options(); + request.max_retries = Some(1); + let error = decide(model(&server).as_ref(), request).await.unwrap_err(); + let AiMuxError::Retry(retry) = &error else { + panic!("status {status}: expected retry exhaustion, got {error:?}") + }; + assert_eq!( + retry.reason, + aimux_core::RetryErrorReason::MaxRetriesExceeded, + "status {status}" + ); + let AiMuxError::ApiCall(last) = retry.last_error() else { + panic!("status {status}: expected API error") + }; + assert_eq!(last.status_code, Some(status)); + assert_eq!(last.provider_code.as_deref(), Some(code)); + assert_eq!( + server.received_requests().await.unwrap().len(), + 2, + "status {status}" + ); } } diff --git a/aimux-providers/tests/openai_decision_test.rs b/aimux-providers/tests/openai_decision_test.rs index 9d601a69f..4ee42830b 100644 --- a/aimux-providers/tests/openai_decision_test.rs +++ b/aimux-providers/tests/openai_decision_test.rs @@ -4,7 +4,6 @@ use std::sync::{ Arc, atomic::{AtomicUsize, Ordering}, }; -use std::time::Duration; use aimux_core::decision_model::*; use aimux_core::recording::{self, RingRecorder}; @@ -33,7 +32,6 @@ fn config(server: &MockServer) -> ProviderOptions { "X-Client".into(), "decision-tests".into(), )])), - ..Default::default() } } fn model(server: &MockServer) -> Box { @@ -290,11 +288,7 @@ async fn retry_and_standard_provider_errors_use_the_shared_transport() { }) .unwrap() .decision("gpt-6-luna") - .unwrap() - .with_retry_config(aimux_core::retry::RetryConfig { - initial_delay: Duration::ZERO, - ..Default::default() - }), + .unwrap(), ); let mut request = options(); request.max_retries = Some(1); diff --git a/aimux-providers/tests/runtime_decision_test.rs b/aimux-providers/tests/runtime_decision_test.rs index 928802317..275045968 100644 --- a/aimux-providers/tests/runtime_decision_test.rs +++ b/aimux-providers/tests/runtime_decision_test.rs @@ -604,22 +604,20 @@ fn local_factories_and_wrapper_factories_preserve_credentials_for_replay() { .unwrap(); let model = handle.decision_model("served-alias").unwrap(); assert_eq!(model.provider(), provider); - let record = model.config_snapshot(); - assert_eq!(record.api_key_source.as_deref(), Some("none")); + let record = + aimux_core::recording::ProviderRecord::from_model(model.provider(), model.model_id()); let registry = registry_with(&record.provider_id, handle); let rebuilt = aimux_providers::rebuild_decision_provider(&record, ®istry).unwrap(); assert_eq!(rebuilt.provider(), provider); } let provider = provider_handle("vllm", Some("explicit-key".into()), None).unwrap(); let model = provider.decision_model("served-alias").unwrap(); - assert_eq!( - model.config_snapshot().api_key_source.as_deref(), - Some("explicit") - ); // The explicit key is not in the recording: without a caller registration // the model cannot be rebuilt. let empty = aimux_core::create_provider_registry(Default::default(), Default::default()); - assert!(aimux_providers::rebuild_decision_provider(&model.config_snapshot(), &empty).is_err()); + let record = + aimux_core::recording::ProviderRecord::from_model(model.provider(), model.model_id()); + assert!(aimux_providers::rebuild_decision_provider(&record, &empty).is_err()); } #[tokio::test] @@ -697,3 +695,54 @@ async fn runtime_errors_preserve_native_message_code_and_status() { } } } + +/// A6 item 2: decisions retry only on the core call-level `maxRetries` +/// (default 2); there is no model- or handle-level retry config. Every +/// attempt fails retryably, so `max_retries = N` makes exactly `N + 1` +/// requests (N retries). +#[tokio::test] +#[serial] +async fn call_level_max_retries_sets_the_exact_retry_count() { + for (max_retries, expected_requests) in [(Some(0), 1), (Some(1), 2), (Some(3), 4), (None, 3)] { + let server = MockServer::start().await; + let calls = Arc::new(AtomicUsize::new(0)); + let responder = calls.clone(); + Mock::given(method("POST")) + .and(path("/v1/systemone")) + .respond_with(move |_: &Request| { + responder.fetch_add(1, Ordering::SeqCst); + ResponseTemplate::new(503) + .insert_header("retry-after", "0") + .set_body_json(json!({"error":{"message":"busy"}})) + }) + .mount(&server) + .await; + let mut request = options(); + request.max_retries = max_retries; + let error = decide(model("vllm", &server).as_ref(), request) + .await + .unwrap_err(); + assert_eq!( + calls.load(Ordering::SeqCst), + expected_requests, + "max_retries = {max_retries:?}" + ); + if expected_requests == 1 { + assert!( + matches!(&error, AiMuxError::ApiCall(e) if e.status_code == Some(503)), + "{error:?}" + ); + } else { + let AiMuxError::Retry(retry) = &error else { + panic!("expected retry exhaustion, got {error:?}"); + }; + assert_eq!( + retry.reason, + aimux_core::RetryErrorReason::MaxRetriesExceeded + ); + assert!( + matches!(retry.last_error(), AiMuxError::ApiCall(e) if e.status_code == Some(503)) + ); + } + } +} diff --git a/bindings/node/__test__/error.test.ts b/bindings/node/__test__/error.test.ts index a8ab022e6..c3e34fd52 100644 --- a/bindings/node/__test__/error.test.ts +++ b/bindings/node/__test__/error.test.ts @@ -7,7 +7,9 @@ import test from 'ava' import { createServer, type Server } from 'node:http' import { openai, + anthropic, provider, + createProvider, mockReplay, streamText, AimuxError, @@ -281,3 +283,97 @@ test.serial('a missing provider setting is a LoadSettingError at call time', asy if (saved !== undefined) process.env.AZURE_RESOURCE_NAME = saved } }) + +// A6 item 3: `bodyOverrides` no longer exists, so it fails like any other +// unknown config key (not with a dedicated "no longer supported" message). +for (const key of ['bodyOverrides', 'body_overrides', 'notAField']) { + test(`provider() rejects config key ${key} as an unknown field`, async (t) => { + const config = { [key]: '{"temperature":0}' } as any + for (const make of [ + () => provider('groq', 'sk-test-fake-key', 'llama-3.3-70b', config), + () => createProvider('groq', 'sk-test-fake-key', config), + () => openai('sk-test-fake-key', 'gpt-4o', config), + ]) { + // A refused config rejects the returned promise; it never throws + // synchronously. + const p = make() + t.true(p instanceof Promise) + const err = (await t.throwsAsync(p)) as AimuxError + t.true(err instanceof InvalidArgumentError) + t.regex(err.message, new RegExp(`unknown field \`${key}\``)) + t.notRegex(err.message, /no longer supported/) + } + }) +} + +test('provider() still accepts the known config keys', async (t) => { + const model = await provider('groq', 'sk-test-fake-key', 'llama-3.3-70b', { + baseUrl: 'http://127.0.0.1:1', + headers: '{"X-Test":"1"}', + }) + t.truthy(model) +}) + +test('provider config maxRetries points to the call options, not a generic unknown field', async (t) => { + const config = { maxRetries: 0 } as any + for (const make of [ + () => provider('groq', 'sk-test-fake-key', 'llama-3.3-70b', config), + () => createProvider('groq', 'sk-test-fake-key', config), + () => openai('sk-test-fake-key', 'gpt-4o', config), + ]) { + const p = make() + t.true(p instanceof Promise) + const err = (await t.throwsAsync(p)) as AimuxError + t.true(err instanceof InvalidArgumentError) + t.regex(err.message, /call-level option: pass it in the call options \(`maxRetries`\)/) + t.notRegex(err.message, /unknown field/) + } +}) + +test('a config of the wrong type rejects the promise, never throws synchronously', async (t) => { + for (const [make, pattern] of [ + // provider / createProvider take a ProviderConfig only: a string is wrong. + [() => provider('groq', 'sk-test-fake-key', 'llama-3.3-70b', 'oops' as any), /a ProviderConfig object, got a string/], + [() => createProvider('groq', 'sk-test-fake-key', 'oops' as any), /a ProviderConfig object, got a string/], + // `string | ProviderConfig` constructors: a string is a base URL, so use + // a number and an array. + [() => openai('sk-test-fake-key', 'gpt-4o', 42 as any), /base URL string or a ProviderConfig object, got a number/], + [() => openai('sk-test-fake-key', 'gpt-4o', [] as any), /base URL string or a ProviderConfig object, got an array/], + [() => anthropic('sk-test-fake-key', 'claude', 42 as any), /got a number/], + ] as const) { + const p = make() + t.true(p instanceof Promise) + const err = (await t.throwsAsync(p)) as AimuxError + t.true(err instanceof InvalidArgumentError, String(err)) + t.regex(err.message, pattern) + } +}) + +test('provider config keys set to undefined are treated as absent', async (t) => { + const model = await provider('groq', 'sk-test-fake-key', 'llama-3.3-70b', { + baseUrl: undefined, + headers: undefined, + }) + t.truthy(model) +}) + +test('a boolean config rejects the promise', async (t) => { + for (const make of [ + () => provider('groq', 'sk-test-fake-key', 'llama-3.3-70b', true as any), + () => createProvider('groq', 'sk-test-fake-key', true as any), + () => openai('sk-test-fake-key', 'gpt-4o', true as any), + ]) { + const p = make() + t.true(p instanceof Promise) + const err = (await t.throwsAsync(p)) as AimuxError + t.true(err instanceof InvalidArgumentError, String(err)) + t.regex(err.message, /got a boolean/) + } +}) + +// TS types allow `| null` for the optional config: null means absent. +test('a null config resolves as if absent', async (t) => { + t.truthy(await provider('groq', 'sk-test-fake-key', 'llama-3.3-70b', null as any)) + t.truthy(await createProvider('groq', 'sk-test-fake-key', null as any)) + t.truthy(await openai('sk-test-fake-key', 'gpt-4o', null as any)) +}) diff --git a/bindings/node/index.d.ts b/bindings/node/index.d.ts index a2b91c5a6..a7a280e5c 100644 --- a/bindings/node/index.d.ts +++ b/bindings/node/index.d.ts @@ -500,18 +500,6 @@ export interface ProviderConfig { * declares are accepted. */ params?: Record - /** - * Rejected with `InvalidArgumentError`: retry is a per-call setting - * (`maxRetries` in the call options). The field exists only so passing it - * is reported instead of ignored. - */ - maxRetries?: number - /** - * Rejected with `InvalidArgumentError`: request-body overrides were - * removed. The field exists only so passing it is reported instead of - * ignored. - */ - bodyOverrides?: string } /** 刷盘全局 recorder(阻塞至 JSONL 落盘;ring 模式为 no-op)。 */ diff --git a/bindings/node/src/lib.rs b/bindings/node/src/lib.rs index c30b1d26d..15cb849ce 100644 --- a/bindings/node/src/lib.rs +++ b/bindings/node/src/lib.rs @@ -626,7 +626,9 @@ impl AsyncGenerator for StreamTextGenerator { /// Passed as the 3rd argument to `openai()` / `anthropic()` / `deepseek()`. /// For backward compatibility a bare string is still accepted (treated as /// `baseUrl`). See RFC-0017. -#[napi(object)] +#[napi(object, object_from_js = false)] +#[derive(Default, serde::Deserialize)] +#[serde(deny_unknown_fields, rename_all = "camelCase")] pub struct ProviderConfig { /// Base URL for API calls (e.g. a relay/proxy endpoint). pub base_url: Option, @@ -642,37 +644,66 @@ pub struct ProviderConfig { /// name (`provider()` / `createProvider()`). Only parameters the preset /// declares are accepted. pub params: Option>, - /// Rejected with `InvalidArgumentError`: retry is a per-call setting - /// (`maxRetries` in the call options). The field exists only so passing it - /// is reported instead of ignored. - pub max_retries: Option, - /// Rejected with `InvalidArgumentError`: request-body overrides were - /// removed. The field exists only so passing it is reported instead of - /// ignored. - pub body_overrides: Option, } -impl ProviderConfig { - /// Fail on the two removed keys instead of dropping them silently. - fn reject_removed_keys(&self) -> MResult<()> { - let removed = if self.max_retries.is_some() { - "`max_retries` is a call-level option: pass it in the call options (`maxRetries`), \ - not in the provider configuration" - } else if self.body_overrides.is_some() { - "`body_overrides` is no longer supported: request-body overrides were removed from \ - the provider configuration and the call options" - } else { - return Ok(()); - }; - Err(AiMuxBindingError::from(&AiMuxError::InvalidArgument( - removed.into(), - ))) +/// Reject keys that belong somewhere else before the generic unknown-field +/// check, so they get a pointed message. +fn reject_removed_keys(raw: &serde_json::Map) -> Option { + raw.contains_key("maxRetries").then(|| { + "`max_retries` is a call-level option: pass it in the call options (`maxRetries`), \ + not in the provider configuration" + .to_string() + }) +} + +/// The constructors take their config argument as raw JSON (napi skips +/// `undefined` properties, like absent keys), so converting it never refuses +/// the call synchronously: every refusal happens here, inside the async body, +/// and rejects the returned promise with `InvalidArgumentError`. A JS string +/// is a base URL only where `allow_url` (the `string | ProviderConfig` +/// constructors). Otherwise the value must be an object, checked by +/// [`reject_removed_keys`], then deserialized with serde +/// `deny_unknown_fields`: the struct above is the single list of accepted keys. +fn parse_config(value: serde_json::Value, allow_url: bool) -> MResult { + let invalid = |message: String| AiMuxBindingError::from(AiMuxError::InvalidArgument(message)); + match value { + serde_json::Value::String(url) if allow_url => Ok(NativeArg::Url(url)), + serde_json::Value::Object(raw) => { + if let Some(message) = reject_removed_keys(&raw) { + return Err(invalid(message)); + } + serde_json::from_value::(serde_json::Value::Object(raw)) + .map(NativeArg::Config) + .map_err(|error| invalid(error.to_string())) + } + other => Err(invalid(format!( + "config must be {}, got {}", + if allow_url { + "a base URL string or a ProviderConfig object" + } else { + "a ProviderConfig object" + }, + match other { + serde_json::Value::Null => "null", + serde_json::Value::Bool(_) => "a boolean", + serde_json::Value::Number(_) => "a number", + serde_json::Value::String(_) => "a string", + serde_json::Value::Array(_) => "an array", + serde_json::Value::Object(_) => "an object", + } + ))), } } +/// A parsed config argument. +enum NativeArg { + Url(String), + Config(ProviderConfig), +} + /// What the native constructors take from their 3rd argument (a base URL /// string, or a [`ProviderConfig`]). Every native constructor goes through -/// here, so `maxRetries` / `bodyOverrides` are rejected for all of them and +/// here, so `maxRetries` and unknown keys are rejected for all of them and /// none of them drops a key silently. struct NativeConfig { base_url: Option, @@ -682,7 +713,11 @@ struct NativeConfig { project: Option, } -fn native_config(config: Option>) -> MResult { +fn native_config(config: Option) -> MResult { + native_from(config.map(|value| parse_config(value, true)).transpose()?) +} + +fn native_from(config: Option) -> MResult { let mut native = NativeConfig { base_url: None, headers: None, @@ -691,9 +726,8 @@ fn native_config(config: Option>) -> MResult {} - Some(Either::A(url)) => native.base_url = Some(url), - Some(Either::B(cfg)) => { - cfg.reject_removed_keys()?; + Some(NativeArg::Url(url)) => native.base_url = Some(url), + Some(NativeArg::Config(cfg)) => { native.base_url = cfg.base_url; if let Some(json_str) = &cfg.headers { let h: std::collections::HashMap = @@ -988,7 +1022,9 @@ pub fn list_sessions() -> AimuxResult { pub async fn openai( api_key: String, model_id: String, - config: Option>, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option< + serde_json::Value, + >, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1019,7 +1055,9 @@ pub async fn openai( pub async fn anthropic( api_key: String, model_id: String, - config: Option>, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option< + serde_json::Value, + >, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1052,11 +1090,13 @@ pub async fn anthropic( pub async fn deepseek( api_key: String, model_id: String, - config: Option>, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option< + serde_json::Value, + >, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { - let settings = named_settings(Some(api_key), config)?; + let settings = named_settings(Some(api_key), config, true)?; let model = create_named_provider("deepseek", settings) .and_then(|p| p.language_model(&model_id)) .map_err(|e| AiMuxBindingError::from(&e))?; @@ -1079,7 +1119,9 @@ pub async fn deepseek( pub async fn google( api_key: String, model_id: String, - config: Option>, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option< + serde_json::Value, + >, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1112,7 +1154,9 @@ pub async fn google( pub async fn cohere( api_key: String, model_id: String, - config: Option>, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option< + serde_json::Value, + >, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1145,7 +1189,9 @@ pub async fn cohere( pub async fn mistral( api_key: String, model_id: String, - config: Option>, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option< + serde_json::Value, + >, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1178,7 +1224,9 @@ pub async fn mistral( pub async fn xai( api_key: String, model_id: String, - config: Option>, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option< + serde_json::Value, + >, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1213,7 +1261,9 @@ pub async fn bedrock( secret_access_key: String, region: String, model_id: String, - config: Option>, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option< + serde_json::Value, + >, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1257,7 +1307,9 @@ pub async fn vertex( project: String, location: String, model_id: String, - config: Option>, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option< + serde_json::Value, + >, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1294,7 +1346,9 @@ pub async fn anthropic_aws( api_key: String, region: String, model_id: String, - config: Option>, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option< + serde_json::Value, + >, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1337,7 +1391,9 @@ pub async fn azure( resource_name: String, deployment: String, api_version: Option, - config: Option>, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option< + serde_json::Value, + >, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1383,11 +1439,11 @@ pub async fn provider( name: String, api_key: Option, model_id: String, - config: Option, + #[napi(ts_arg_type = "ProviderConfig | undefined | null")] config: Option, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { - let settings = named_settings(api_key, config.map(Either::B))?; + let settings = named_settings(api_key, config, false)?; let model = create_named_provider(&name, settings) .and_then(|p| p.language_model(&model_id)) .map_err(|e| AiMuxBindingError::from(&e))?; @@ -1495,11 +1551,11 @@ impl ProviderHandle { pub async fn create_provider( name: String, api_key: Option, - config: Option, + #[napi(ts_arg_type = "ProviderConfig | undefined | null")] config: Option, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { - let settings = named_settings(api_key, config.map(Either::B))?; + let settings = named_settings(api_key, config, false)?; let inner = create_named_provider(&name, settings).map_err(|e| AiMuxBindingError::from(&e))?; Ok(ProviderHandle { inner }) @@ -1531,13 +1587,17 @@ pub async fn get_model_specs(source_url: Option) -> AimuxResult /// caller's own. fn named_settings( api_key: Option, - config: Option>, + config: Option, + allow_url: bool, ) -> MResult { + let config = config + .map(|value| parse_config(value, allow_url)) + .transpose()?; let params = match &config { - Some(Either::B(cfg)) => cfg.params.clone().unwrap_or_default(), + Some(NativeArg::Config(cfg)) => cfg.params.clone().unwrap_or_default(), _ => Default::default(), }; - let native = native_config(config)?; + let native = native_from(config)?; let mut headers = aimux_provider_utils::HeaderMapOpt::new(); for (name, value) in [ ("OpenAI-Organization", native.organization), diff --git a/bindings/node/src/types/ProviderRecord.ts b/bindings/node/src/types/ProviderRecord.ts index 91cea4f62..a7a5071dc 100644 --- a/bindings/node/src/types/ProviderRecord.ts +++ b/bindings/node/src/types/ProviderRecord.ts @@ -1,5 +1,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -import type { JsonValue } from "./serde_json/JsonValue"; /** * ② 配置侧:provider 身份(请求回放重建用)。 @@ -19,24 +18,4 @@ provider: string, /** * `model.model_id()`,如 "gpt-4o"。 */ -modelId: string, -/** - * Decision connection snapshot (#226, RFC-0038 录制): the provider base - * URL. Informational only: replay takes the endpoint from the caller's - * registry. Absent for every other operation. - */ -baseUrl?: string | null, -/** - * Where the credential came from, never its value: `"env:VAR"`, - * `"explicit"` or `"none"` (#226). - */ -apiKeySource?: string | null, -/** - * Decision capabilities and default protocol (#226). - */ -profile?: JsonValue | null, -/** - * Connection options (headers, retries); sensitive headers are redacted - * when recorded (#226). - */ -providerOptions?: JsonValue | null, }; +modelId: string, }; diff --git a/bindings/python/python/aimux/__init__.py b/bindings/python/python/aimux/__init__.py index 7e9657b49..c4af5956f 100644 --- a/bindings/python/python/aimux/__init__.py +++ b/bindings/python/python/aimux/__init__.py @@ -221,7 +221,7 @@ def provider( config: Full ProviderOptions dict — base_url / headers / organization / project / params (a preset's template parameters, e.g. ``{"account_id": "..."}``). ``max_retries`` (a call-level option) and - ``body_overrides`` are rejected with ``InvalidArgumentError``. + any other key are rejected with ``InvalidArgumentError``. """ config_json = json.dumps(config) if config is not None else None return _native_provider(name, api_key, model_id, base_url, config_json) @@ -245,7 +245,7 @@ def create_provider( config: Full ProviderOptions dict — base_url / headers / organization / project / params (a preset's template parameters, e.g. ``{"account_id": "..."}``). ``max_retries`` (a call-level option) and - ``body_overrides`` are rejected with ``InvalidArgumentError``. + any other key are rejected with ``InvalidArgumentError``. """ config_json = json.dumps(config) if config is not None else None return _native_create_provider(name, api_key, base_url, config_json) diff --git a/bindings/python/src/lib.rs b/bindings/python/src/lib.rs index cbe901273..8e91429ac 100644 --- a/bindings/python/src/lib.rs +++ b/bindings/python/src/lib.rs @@ -776,8 +776,8 @@ fn named_settings( /// Create a language model from the built-in registry by provider name /// (RFC-0017 phase 4). `api_key=None` reads the provider's env var. /// `config_json` is a JSON object (`base_url` / `headers` / `organization` / -/// `project` / `params`; any other key, `max_retries` and `body_overrides` -/// included, is rejected as an invalid argument); the `base_url` parameter +/// `project` / `params`; any other key, `max_retries` included, is rejected +/// as an invalid argument); the `base_url` parameter /// wins over the JSON field. #[pyfunction] #[pyo3(signature = (name, api_key, model_id, base_url=None, config_json=None))] diff --git a/bindings/python/tests/test_aimux.py b/bindings/python/tests/test_aimux.py index ccb1bb8ce..37853b2eb 100644 --- a/bindings/python/tests/test_aimux.py +++ b/bindings/python/tests/test_aimux.py @@ -57,28 +57,40 @@ def test_provider_accepts_full_config(): assert hasattr(model, "generate_text") -@pytest.mark.parametrize( - "key,value", - [("max_retries", 0), ("body_overrides", {"temperature": 0.1})], -) -def test_provider_rejects_removed_config_keys(key, value): - """max_retries is call-level and body_overrides is gone: neither is silently ignored.""" +def test_provider_rejects_max_retries_config_key(): + """max_retries is call-level: it is not silently ignored.""" from aimux import provider - with pytest.raises(Exception, match=key): - provider("groq", "sk-test-fake-key", "llama-3.3-70b", config={key: value}) + with pytest.raises(Exception, match="max_retries"): + provider("groq", "sk-test-fake-key", "llama-3.3-70b", config={"max_retries": 0}) -@pytest.mark.parametrize( - "key,value", - [("max_retries", 0), ("body_overrides", {"temperature": 0.1})], -) -def test_create_provider_rejects_removed_config_keys(key, value): - """create_provider() goes through the same ProviderOptions check.""" +def test_create_provider_rejects_max_retries_config_key(): + """create_provider() goes through the same config check.""" from aimux import InvalidArgumentError, create_provider - with pytest.raises(InvalidArgumentError, match=key): - create_provider("groq", "sk-test-fake-key", config={key: value}) + with pytest.raises(InvalidArgumentError, match="max_retries"): + create_provider("groq", "sk-test-fake-key", config={"max_retries": 0}) + + +@pytest.mark.parametrize("key", ["body_overrides", "not_a_field"]) +def test_provider_unknown_config_key(key): + """body_overrides no longer exists (A6): it fails like any unknown field.""" + from aimux import InvalidArgumentError, provider + + with pytest.raises(InvalidArgumentError, match=f"unknown field `{key}`") as info: + provider("groq", "sk-test-fake-key", "llama-3.3-70b", config={key: {"temperature": 0.1}}) + assert "no longer supported" not in str(info.value) + + +@pytest.mark.parametrize("key", ["body_overrides", "not_a_field"]) +def test_create_provider_unknown_config_key(key): + """create_provider() reports body_overrides through the same unknown-field path.""" + from aimux import InvalidArgumentError, create_provider + + with pytest.raises(InvalidArgumentError, match=f"unknown field `{key}`") as info: + create_provider("groq", "sk-test-fake-key", config={key: {"temperature": 0.1}}) + assert "no longer supported" not in str(info.value) def test_provider_base_url_param_wins_over_config(): diff --git a/docs/amendments/A6-remove-226-per-rfc.md b/docs/amendments/A6-remove-226-per-rfc.md new file mode 100644 index 000000000..6ab986011 --- /dev/null +++ b/docs/amendments/A6-remove-226-per-rfc.md @@ -0,0 +1,47 @@ +# A6:按 RFC 删除 #226 这一组(关闭 D3/D4/D5/D6/D14/D16/D17) + +> 决定人:Trey,2026-10-11。代码行号均为 fork `exp/restart@8455db9a`。 +> 文档:#200 = `docs/aisdk-architecture-alignment.md`(8455db9a 仓内);RFC-0040 = `upstream/docs/rfc-0040-provider-auth@3757cfda:rfc/0040-provider-auth.md`;RFC-0041 = `upstream/docs/rfc-0041-replay-governance@84fbcf42:rfc/0041-replay-governance.md`。**注意:0040/0041 不在 8455db9a 仓内 `rfc/`,只在上游分支上**(只读取,未改上游)。 + +## 决定 +按维护者 RFC 删除 #226 恢复的四处语义,同时删掉 D16 临时白名单。边界脚本不带任何白名单也要退出码 0。 + +## 删除项、RFC 依据、代码位置 + +### 1. 录制 `config_snapshot`(录制只记 provider 名和 model 名) +- 依据:#200:199(禁止通过 `config_snapshot()` 读配置推导凭证/重建);#200:1436(删除清单 `LanguageModel::config_snapshot()`);#200:1470(`ProviderRecord`、`record_provider`、`api_key_source`);RFC-0041:35、:41(不保存 settings、key 来源)、:338;RFC-0040:197(B02)。 +- 代码:`aimux-core/src/decision_model.rs:556`(trait 默认 `fn config_snapshot`)、:668(`record_provider(..., &model.config_snapshot().redacted())`);覆盖实现 `aimux-providers/src/jev.rs:98`、`openai/decision.rs:413`、`provider_handle.rs:270-273`、`runtime_decision.rs:640`;`aimux-core/src/recording.rs:221,240`(`ProviderRecord.api_key_source`)。 +- 测试:`aimux-providers/tests/decision_handle_test.rs:67,77,184`;`runtime_decision_test.rs:607,616,622`。 + +### 2. 模型级 `retry_config()`(只用 core `maxRetries`) +- 依据:#200:29(retry 混进模型协议 `retry_config()`);#200:292(`max_retries` 属用户操作选项);#200:1434(删除各模态 `retry_config()`);#200:1025(不得重新引入 provider 默认 `max_retries`)。 +- 代码:`aimux-core/src/decision_model.rs:564`(定义)、:653(调用);`aimux-core/src/retry.rs:19`(文档引用);`aimux-providers/src/decision_support.rs:54,78`;`jev.rs:27,38,116-117`;`openai/decision.rs:17,35,41-42,410-411,417`;`provider_handle.rs:195-201`。 + +### 3. `body_overrides` 拒绝逻辑(字段不存在,传入按未知字段报错) +- 依据:#200:159(D15 删除通用 `body_overrides`);#200:340、:1458、:1577;RFC-0040:178(迁移说明列"移除的 bodyOverrides")。 +- 代码:`aimux-providers/src/provider_handle.rs:43`(字段)、:157、:166、:174-176、:185-187(两处拒绝);`tests/decision_handle_test.rs:55`。 +- 绑定(要**改成未知字段错误并改测试**,不能只删):`bindings/node/src/lib.rs:652`、:661-662("no longer supported" 专门报错);`bindings/python/src/lib.rs:779`(文档);`bindings/python/python/aimux/__init__.py:224,248`;`bindings/python/tests/test_aimux.py:62-74`(断言 `InvalidArgumentError`)。 + +### 4. provider_handle 隐式读 `_API_KEY` +- 依据:RFC-0040:32-33(aimux 不扫描任意进程 env;不隐式授予 env 读取)、:71(仅 descriptor 标记的 credential 字段接受 `{ "env": "NAME" }`)、:92(显式→声明 env);#200:534(仅 descriptor 标记字段支持 env 引用)、:540(env 来源不进录制)。 +- 代码:`aimux-providers/src/provider_handle.rs:3`(模块文档)、:59-63(`non_empty_env`)、:69-70(文档)、:91、:118-131(`format!("{}_API_KEY")` 回退与 `api_key_source`)、:158、:169、:192、:202、:253;openai 分支 :94 `load_api_key(..., "OPENAI_API_KEY", ...)`(见歧义 1)。 +- 注:`_API_KEY` 全仓命中多(providers/bindings/docs/tools/fixtures/ffi),大多是合法的显式声明,**只删隐式读取路径**。 + +### 5. D16 白名单 +- `scripts/check_provider_boundaries.sh:54-60`(`d16_allow`)、:104-107(调用)、:172(`PLACEHOLDER_API_KEY` 例外,见歧义 2);规则本身 :95-102、:126、:170 保留。 + +## 影响面(逐项核) +- 录制/回放:`ProviderRecord` 结构变化 → `aimux-core/src/decision_replay.rs`、`aimux-providers/src/replay.rs`、`tools/aimux-replay`、`fixtures/` 下录制文件需重生成并看 diff。 +- 绑定:`api_key_source`/`decision_capabilities` 出现在 flutter/go/java/kotlin/python/swift 的 decision 文件(`bindings/flutter/lib/decision.dart`、`bindings/go/decision.go`、`bindings/java/.../DecisionModel.java`、`AimuxFFI.java`、`bindings/kotlin/.../DecisionModel.kt`、`Model.kt`、`bindings/python/python/aimux/__init__.py`、`bindings/python/tests/test_decision.py`、`bindings/swift/.../DecisionModel.swift`);node/python 的 body_overrides 见第 3 项。 +- 测试:上列 Rust 测试;绑定测试对比基线(Python、Flutter、4 个 Node target 原本就红)。 + +## RFC 歧义(未定,交 Trey) +1. **openai 原生包默认读 `OPENAI_API_KEY`**:#200:538「原生 provider 未显式提供 key 时,仍按包规则在请求时读取默认 env」,与第 4 项"只显式传或 descriptor 声明"字面冲突。`provider_handle.rs:94` 是删还是保留(视作包规则/descriptor 默认 env)? +2. **D15 占位 key 未在本次列表**:RFC-0040:155(B2)「本地服务无假 key…不要保留 PLACEHOLDER_API_KEY」。删白名单后脚本 :170-172 会拦 `provider_handle.rs:57` 的 `PLACEHOLDER_API_KEY`,"无白名单退出码 0"实际要求 D15 也选"不发 header"。 +3. **录制保留 provider 名**:#200:1470 删除清单包括整个 `ProviderRecord`/`record_provider`;RFC-0041:267 目标形如 `target: {modelId, ...}`。Trey 的"记 provider 名 + model 名"是过渡做法,最终形态按 RFC-0041 G3(:338)再改。 +4. **本地 runtime 的 `*_BASE_URL` env**(`provider_handle.rs:114-117`):RFC-0040:155、:191 保留 URL env;本次不删,但它也是隐式 env 读取,需确认不算第 4 项。 +5. RFC-0038:25「重试配置」措辞(原 D6)是否修文:本次只记录随 D5 关闭,未改 RFC。 + +## 更新 2026-10-11:歧义 1、2 已决 +1. 原生 provider 的默认 env 保留(D23,#200:538,RFC-0040:67)。R.3 只删通用隐式读取。 +2. D15 改为不发 header,删除 `PLACEHOLDER_API_KEY`(D24,RFC-0040:155、:192)。R.5 可以完成。