From ea41d7324d12f95f84fa2093284d17c00fdf5138 Mon Sep 17 00:00:00 2001 From: Trey Kim Date: Sun, 11 Oct 2026 12:57:13 +0800 Subject: [PATCH 01/12] R.1: remove body_overrides; report it as an unknown field (A6 item 3) Drop ProviderOptions.body_overrides and both #226 rejections in provider_handle.rs; ProviderOptions now denies unknown fields. Node ProviderConfig loses bodyOverrides and reports any unknown key as InvalidArgumentError 'unknown field `x`'. Python docs updated; Python already rejects unknown keys via serde. Tests in Rust/Python/Node. Adds docs/amendments/A6-remove-226-per-rfc.md verbatim. --- aimux-providers/src/provider_handle.rs | 27 +------- aimux-providers/tests/decision_handle_test.rs | 29 ++++---- bindings/node/__test__/error.test.ts | 27 ++++++++ bindings/node/index.d.ts | 6 -- bindings/node/src/lib.rs | 67 +++++++++++++++---- bindings/python/python/aimux/__init__.py | 4 +- bindings/python/src/lib.rs | 4 +- bindings/python/tests/test_aimux.py | 44 +++++++----- docs/amendments/A6-remove-226-per-rfc.md | 47 +++++++++++++ 9 files changed, 178 insertions(+), 77 deletions(-) create mode 100644 docs/amendments/A6-remove-226-per-rfc.md diff --git a/aimux-providers/src/provider_handle.rs b/aimux-providers/src/provider_handle.rs index ef55cf797..9b199ab05 100644 --- a/aimux-providers/src/provider_handle.rs +++ b/aimux-providers/src/provider_handle.rs @@ -20,13 +20,13 @@ use aimux_core::{ 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, @@ -38,9 +38,6 @@ pub struct ProviderOptions { 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. @@ -154,7 +151,6 @@ 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, })) } @@ -163,31 +159,13 @@ pub fn provider_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); @@ -203,15 +181,12 @@ impl Provider for ProviderHandle { })) } 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( diff --git a/aimux-providers/tests/decision_handle_test.rs b/aimux-providers/tests/decision_handle_test.rs index bbf6d5a71..3be4dd446 100644 --- a/aimux-providers/tests/decision_handle_test.rs +++ b/aimux-providers/tests/decision_handle_test.rs @@ -48,19 +48,22 @@ fn handle_options_reach_decisions() { .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(_)) - )); +} + +/// `body_overrides` no longer exists (A6 item 3): it is an unknown field like +/// any other, not a specially rejected one. +#[test] +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] diff --git a/bindings/node/__test__/error.test.ts b/bindings/node/__test__/error.test.ts index a8ab022e6..a9870d147 100644 --- a/bindings/node/__test__/error.test.ts +++ b/bindings/node/__test__/error.test.ts @@ -8,6 +8,7 @@ import { createServer, type Server } from 'node:http' import { openai, provider, + createProvider, mockReplay, streamText, AimuxError, @@ -281,3 +282,29 @@ 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), + ]) { + const err = (await t.throwsAsync(make)) 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) +}) diff --git a/bindings/node/index.d.ts b/bindings/node/index.d.ts index a2b91c5a6..26a74e047 100644 --- a/bindings/node/index.d.ts +++ b/bindings/node/index.d.ts @@ -506,12 +506,6 @@ export interface ProviderConfig { * 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..65445dfca 100644 --- a/bindings/node/src/lib.rs +++ b/bindings/node/src/lib.rs @@ -626,7 +626,7 @@ 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)] pub struct ProviderConfig { /// Base URL for API calls (e.g. a relay/proxy endpoint). pub base_url: Option, @@ -646,33 +646,76 @@ pub struct ProviderConfig { /// (`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, + /// Keys of the JS object that are not fields above, reported as unknown + /// fields instead of being dropped. + #[napi(skip)] + pub unknown_fields: Vec, } +/// The JS names of the [`ProviderConfig`] fields. +const PROVIDER_CONFIG_FIELDS: &[&str] = &[ + "baseUrl", + "headers", + "organization", + "project", + "params", + "maxRetries", +]; + +impl FromNapiValue for ProviderConfig { + unsafe fn from_napi_value(env: sys::napi_env, napi_val: sys::napi_value) -> Result { + let obj = unsafe { Object::from_napi_value(env, napi_val)? }; + let names = obj.get_property_names()?; + let mut unknown_fields = Vec::new(); + for i in 0..names.get_array_length()? { + let name: String = names.get_element(i)?; + if !PROVIDER_CONFIG_FIELDS.contains(&name.as_str()) { + unknown_fields.push(name); + } + } + Ok(Self { + base_url: obj.get("baseUrl")?, + headers: obj.get("headers")?, + organization: obj.get("organization")?, + project: obj.get("project")?, + params: obj.get("params")?, + max_retries: obj.get("maxRetries")?, + unknown_fields, + }) + } +} + +impl ValidateNapiValue for ProviderConfig {} + impl ProviderConfig { - /// Fail on the two removed keys instead of dropping them silently. + /// Fail on an unknown key or on `max_retries` instead of dropping them + /// silently. fn reject_removed_keys(&self) -> MResult<()> { - let removed = if self.max_retries.is_some() { + let message = if let Some(name) = self.unknown_fields.first() { + format!( + "unknown field `{name}`, expected one of {}", + PROVIDER_CONFIG_FIELDS + .iter() + .map(|f| format!("`{f}`")) + .collect::>() + .join(", ") + ) + } else 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" + .to_string() } else { return Ok(()); }; Err(AiMuxBindingError::from(&AiMuxError::InvalidArgument( - removed.into(), + message, ))) } } /// 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, 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 可以完成。 From 6f0aa3672a89ae1591ff33e54f70221e434ff2ec Mon Sep 17 00:00:00 2001 From: Trey Kim Date: Sun, 11 Oct 2026 13:02:14 +0800 Subject: [PATCH 02/12] node: ProviderConfig via serde deny_unknown_fields (single source of keys) Replace the hand-written key allowlist: the JS object is read as a raw JSON map (undefined values skipped), maxRetries is rejected first with the call-level message, then the map deserializes into ProviderConfig with #[serde(deny_unknown_fields, rename_all = "camelCase")]. The max_retries struct field is gone (index.d.ts loses maxRetries?). Tests: maxRetries gets the call-level message, undefined keys are absent. --- bindings/node/__test__/error.test.ts | 22 +++++++ bindings/node/index.d.ts | 6 -- bindings/node/src/lib.rs | 97 +++++++++++++--------------- 3 files changed, 68 insertions(+), 57 deletions(-) diff --git a/bindings/node/__test__/error.test.ts b/bindings/node/__test__/error.test.ts index a9870d147..9652e63fc 100644 --- a/bindings/node/__test__/error.test.ts +++ b/bindings/node/__test__/error.test.ts @@ -308,3 +308,25 @@ test('provider() still accepts the known config keys', async (t) => { }) 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 err = (await t.throwsAsync(make)) 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('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) +}) diff --git a/bindings/node/index.d.ts b/bindings/node/index.d.ts index 26a74e047..a7a280e5c 100644 --- a/bindings/node/index.d.ts +++ b/bindings/node/index.d.ts @@ -500,12 +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 } /** 刷盘全局 recorder(阻塞至 JSONL 落盘;ring 模式为 no-op)。 */ diff --git a/bindings/node/src/lib.rs b/bindings/node/src/lib.rs index 65445dfca..c1e8a7b8f 100644 --- a/bindings/node/src/lib.rs +++ b/bindings/node/src/lib.rs @@ -627,6 +627,8 @@ impl AsyncGenerator for StreamTextGenerator { /// For backward compatibility a bare string is still accepted (treated as /// `baseUrl`). See RFC-0017. #[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,74 +644,67 @@ 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, - /// Keys of the JS object that are not fields above, reported as unknown - /// fields instead of being dropped. + /// Why the JS object was refused (a call-level key, an unknown field, a + /// mistyped value), reported as `InvalidArgumentError` by + /// [`native_config`] instead of being dropped. #[napi(skip)] - pub unknown_fields: Vec, + #[serde(skip)] + pub invalid: Option, } -/// The JS names of the [`ProviderConfig`] fields. -const PROVIDER_CONFIG_FIELDS: &[&str] = &[ - "baseUrl", - "headers", - "organization", - "project", - "params", - "maxRetries", -]; +/// 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 JS object is read as a raw JSON map (`undefined` values skipped, like +/// absent keys), checked by [`reject_removed_keys`], then deserialized with +/// serde `deny_unknown_fields`: the struct above is the single list of +/// accepted keys. impl FromNapiValue for ProviderConfig { unsafe fn from_napi_value(env: sys::napi_env, napi_val: sys::napi_value) -> Result { let obj = unsafe { Object::from_napi_value(env, napi_val)? }; let names = obj.get_property_names()?; - let mut unknown_fields = Vec::new(); + let mut raw = serde_json::Map::new(); for i in 0..names.get_array_length()? { let name: String = names.get_element(i)?; - if !PROVIDER_CONFIG_FIELDS.contains(&name.as_str()) { - unknown_fields.push(name); + let value: Unknown = obj.get_named_property(&name)?; + if value.get_type()? == ValueType::Undefined { + continue; } + let value = unsafe { serde_json::Value::from_napi_value(env, value.raw())? }; + raw.insert(name, value); } - Ok(Self { - base_url: obj.get("baseUrl")?, - headers: obj.get("headers")?, - organization: obj.get("organization")?, - project: obj.get("project")?, - params: obj.get("params")?, - max_retries: obj.get("maxRetries")?, - unknown_fields, - }) + let invalid = |message: String| Self { + invalid: Some(message), + ..Self::default() + }; + if let Some(message) = reject_removed_keys(&raw) { + return Ok(invalid(message)); + } + Ok( + serde_json::from_value::(serde_json::Value::Object(raw)) + .unwrap_or_else(|error| invalid(error.to_string())), + ) } } impl ValidateNapiValue for ProviderConfig {} impl ProviderConfig { - /// Fail on an unknown key or on `max_retries` instead of dropping them - /// silently. - fn reject_removed_keys(&self) -> MResult<()> { - let message = if let Some(name) = self.unknown_fields.first() { - format!( - "unknown field `{name}`, expected one of {}", - PROVIDER_CONFIG_FIELDS - .iter() - .map(|f| format!("`{f}`")) - .collect::>() - .join(", ") - ) - } else 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" - .to_string() - } else { - return Ok(()); - }; - Err(AiMuxBindingError::from(&AiMuxError::InvalidArgument( - message, - ))) + /// The refusal recorded while reading the JS object, if any. + fn check(&self) -> MResult<()> { + match &self.invalid { + Some(message) => Err(AiMuxBindingError::from(&AiMuxError::InvalidArgument( + message.clone(), + ))), + None => Ok(()), + } } } @@ -736,7 +731,7 @@ fn native_config(config: Option>) -> MResult {} Some(Either::A(url)) => native.base_url = Some(url), Some(Either::B(cfg)) => { - cfg.reject_removed_keys()?; + cfg.check()?; native.base_url = cfg.base_url; if let Some(json_str) = &cfg.headers { let h: std::collections::HashMap = From 5ac114daed1853fc7411c92a382debc212e24e2c Mon Sep 17 00:00:00 2001 From: Trey Kim Date: Sun, 11 Oct 2026 13:10:34 +0800 Subject: [PATCH 03/12] R.2: remove model-level retry_config; decisions retry on core maxRetries only (A6 item 2) Drop RetryConfig and prepare_retries_with_config from aimux-core, DecisionModel::retry_config, the retry fields/overrides in jev, openai/decision (with_retry_config), runtime_decision and decision_support (snapshot no longer records max_retries), and the handle-level ProviderOptions.max_retries default in provider_handle. decide() uses retry::prepare_retries(options.max_retries, ...). New mock test: max_retries = 0/1/3/None makes exactly 1/2/4/3 requests. --- aimux-core/src/decision_model.rs | 11 +---- aimux-core/src/retry.rs | 43 ------------------- aimux-providers/src/decision_support.rs | 8 +--- aimux-providers/src/jev.rs | 6 --- aimux-providers/src/openai/decision.rs | 13 ------ aimux-providers/src/provider_handle.rs | 21 +-------- aimux-providers/src/runtime_decision.rs | 6 --- aimux-providers/tests/decision_handle_test.rs | 18 +++----- aimux-providers/tests/jev_test.rs | 5 +-- aimux-providers/tests/openai_decision_test.rs | 8 +--- .../tests/runtime_decision_test.rs | 39 +++++++++++++++++ 11 files changed, 52 insertions(+), 126 deletions(-) diff --git a/aimux-core/src/decision_model.rs b/aimux-core/src/decision_model.rs index 49a900fe6..2bfddbae8 100644 --- a/aimux-core/src/decision_model.rs +++ b/aimux-core/src/decision_model.rs @@ -559,11 +559,6 @@ pub trait DecisionModel: Send + Sync { 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. @@ -648,11 +643,7 @@ 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()); let context = crate::recording::recorder().map(|recorder| { let ctx = crate::recording::RecordingContext::new(crate::recording::new_call_id(), recorder); diff --git a/aimux-core/src/retry.rs b/aimux-core/src/retry.rs index 5ed5a7893..497ba03a7 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,24 +45,6 @@ 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 { /// Retry a complete operation using API-call retryability and response hints. /// diff --git a/aimux-providers/src/decision_support.rs b/aimux-providers/src/decision_support.rs index 72e6dce0e..984bf1c92 100644 --- a/aimux-providers/src/decision_support.rs +++ b/aimux-providers/src/decision_support.rs @@ -45,13 +45,12 @@ impl DecisionEndpoint { } /// #226's decision connection snapshot: identity, base URL, credential - /// *source* and the connection headers/retries. The credential value is + /// *source* and the connection headers. 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; @@ -73,10 +72,7 @@ impl DecisionEndpoint { } } record.provider_options = Some(aimux_core::recording::redact_provider_options( - serde_json::json!({ - "headers": wire_headers, - "max_retries": retry_config.max_retries, - }), + serde_json::json!({ "headers": wire_headers }), )); record.profile = Some(profile); record diff --git a/aimux-providers/src/jev.rs b/aimux-providers/src/jev.rs index c498881d6..2017b5b0d 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(), } } @@ -113,9 +110,6 @@ impl DecisionModel for JevDecisionModel { )); 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..18c35d27d 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,14 +398,10 @@ 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()}), ) } diff --git a/aimux-providers/src/provider_handle.rs b/aimux-providers/src/provider_handle.rs index 9b199ab05..ab14dfe4a 100644 --- a/aimux-providers/src/provider_handle.rs +++ b/aimux-providers/src/provider_handle.rs @@ -12,7 +12,6 @@ use aimux_core::decision_model::{ DecisionCallOptions, DecisionCapabilities, DecisionModel, DecisionResult, }; 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, @@ -36,8 +35,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, } /// Official local decision runtimes: name, default base URL, base URL env. @@ -150,7 +147,6 @@ pub fn provider_handle( }; Ok(Box::new(ProviderHandle { inner, - max_retries: options.max_retries, api_key_source, })) } @@ -158,7 +154,6 @@ pub fn provider_handle( /// A provider with #226's handle-level decision settings. struct ProviderHandle { inner: Arc, - max_retries: Option, /// 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, @@ -167,16 +162,11 @@ struct ProviderHandle { impl Provider for ProviderHandle { fn decision_model(&self, id: &str) -> Result, AiMuxError> { let model = self.inner.decision_model(id)?; - if self.max_retries.is_none() && self.api_key_source.is_none() { + if 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(), })) } @@ -221,10 +211,9 @@ impl Provider for ProviderHandle { } } -/// A decision model with handle-level retry settings and credential source. +/// A decision model with the handle's credential source. struct HandleDecisionModel { inner: Box, - retry_config: RetryConfig, api_key_source: Option, } @@ -239,17 +228,11 @@ impl DecisionModel for HandleDecisionModel { fn capabilities(&self) -> DecisionCapabilities { self.inner.capabilities() } - fn retry_config(&self) -> RetryConfig { - self.retry_config - } 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()); - } snapshot } async fn do_decide(&self, options: &DecisionCallOptions) -> Result { diff --git a/aimux-providers/src/runtime_decision.rs b/aimux-providers/src/runtime_decision.rs index f6f454897..c14eadb37 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,14 +632,10 @@ 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()}), ) } diff --git a/aimux-providers/tests/decision_handle_test.rs b/aimux-providers/tests/decision_handle_test.rs index 3be4dd446..6e76b62cb 100644 --- a/aimux-providers/tests/decision_handle_test.rs +++ b/aimux-providers/tests/decision_handle_test.rs @@ -35,19 +35,13 @@ 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); +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 diff --git a/aimux-providers/tests/jev_test.rs b/aimux-providers/tests/jev_test.rs index 652b861a5..e072f7568 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() 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..9410533c4 100644 --- a/aimux-providers/tests/runtime_decision_test.rs +++ b/aimux-providers/tests/runtime_decision_test.rs @@ -697,3 +697,42 @@ 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(_)), "{error:?}"); + } else { + assert!(matches!(error, AiMuxError::Retry(_)), "{error:?}"); + } + } +} From 8cfdf46928c9de1918943dbc85cf16fb9c5f18b9 Mon Sep 17 00:00:00 2001 From: Trey Kim Date: Sun, 11 Oct 2026 13:16:35 +0800 Subject: [PATCH 04/12] R.2 tests: zero-delay retry tests in aimux-core; one real retry run in jev_test Retry counting moves into aimux-core decision_model::retry_tests, using a crate-internal #[cfg(test)] PreparedRetries::with_initial_delay(ZERO); asserts attempts = max_retries + 1 and RetryErrorReason::MaxRetriesExceeded with the last 503/429/529 error. jev_test keeps status classification at max_retries = 0 plus one max_retries = 1 run on 503 asserting 2 requests and retry exhaustion. No public API, feature or binding change. --- aimux-core/src/decision_model.rs | 126 ++++++++++++++++++ aimux-core/src/retry.rs | 9 ++ aimux-providers/tests/jev_test.rs | 59 +++++--- .../tests/runtime_decision_test.rs | 16 ++- 4 files changed, 192 insertions(+), 18 deletions(-) diff --git a/aimux-core/src/decision_model.rs b/aimux-core/src/decision_model.rs index 2bfddbae8..1f2bae854 100644 --- a/aimux-core/src/decision_model.rs +++ b/aimux-core/src/decision_model.rs @@ -570,8 +570,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(); @@ -644,6 +654,13 @@ 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(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); @@ -712,3 +729,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/retry.rs b/aimux-core/src/retry.rs index 497ba03a7..ddfe14837 100644 --- a/aimux-core/src/retry.rs +++ b/aimux-core/src/retry.rs @@ -46,6 +46,15 @@ pub(crate) fn prepare_retries( } 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 + } + /// Retry a complete operation using API-call retryability and response hints. /// /// # Errors diff --git a/aimux-providers/tests/jev_test.rs b/aimux-providers/tests/jev_test.rs index e072f7568..5f70ad48e 100644 --- a/aimux-providers/tests/jev_test.rs +++ b/aimux-providers/tests/jev_test.rs @@ -352,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 single +/// `max_retries = 1` run proves a 503 is retried and then exhausted (one +/// real default backoff wait). #[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( @@ -369,20 +374,42 @@ 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); } + + let server = MockServer::start().await; + mount( + &server, + 503, + json!({"error":{"message":"test failure", "code":"unavailable"}}), + ) + .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!("expected retry exhaustion, got {error:?}") + }; + assert_eq!( + retry.reason, + aimux_core::RetryErrorReason::MaxRetriesExceeded + ); + let AiMuxError::ApiCall(last) = retry.last_error() else { + panic!("expected API error") + }; + assert_eq!(last.status_code, Some(503)); + assert_eq!(last.provider_code.as_deref(), Some("unavailable")); + assert_eq!(server.received_requests().await.unwrap().len(), 2); } #[test] diff --git a/aimux-providers/tests/runtime_decision_test.rs b/aimux-providers/tests/runtime_decision_test.rs index 9410533c4..b8d21aa60 100644 --- a/aimux-providers/tests/runtime_decision_test.rs +++ b/aimux-providers/tests/runtime_decision_test.rs @@ -730,9 +730,21 @@ async fn call_level_max_retries_sets_the_exact_retry_count() { "max_retries = {max_retries:?}" ); if expected_requests == 1 { - assert!(matches!(error, AiMuxError::ApiCall(_)), "{error:?}"); + assert!( + matches!(&error, AiMuxError::ApiCall(e) if e.status_code == Some(503)), + "{error:?}" + ); } else { - assert!(matches!(error, AiMuxError::Retry(_)), "{error:?}"); + 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)) + ); } } } From ff8fae776516b0b8f1cfc8ce57314d5681c7874c Mon Sep 17 00:00:00 2001 From: Trey Kim Date: Sun, 11 Oct 2026 13:58:35 +0800 Subject: [PATCH 05/12] R.3: no implicit _API_KEY; keep native default env at request time (A6 item 4, D23) provider_handle no longer reads the generic format!("{}_API_KEY") for local runtimes (#226). openai with api_key None keeps the package default OPENAI_API_KEY, now read at request time (#200:538); Some("") is used as given (RFC-0040:67); a changed base URL without a key sends no credential and reads no env. Tests inject env as a plain-data map (crate-internal EnvSource::Vars), never std::env::set_var: generic key not read, env set after construction is used, Some("") no fallback, replay/auth=none/changed base URL read nothing. api_key_source field stays for R.4; placeholder key for R.5. --- aimux-providers/src/openai/mod.rs | 16 ++ aimux-providers/src/preset.rs | 1 + aimux-providers/src/provider_handle.rs | 368 +++++++++++++++++++++++-- aimux-providers/src/shared/mod.rs | 54 +++- 4 files changed, 408 insertions(+), 31 deletions(-) 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 ab14dfe4a..d6a8da0a7 100644 --- a/aimux-providers/src/provider_handle.rs +++ b/aimux-providers/src/provider_handle.rs @@ -1,7 +1,9 @@ //! #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). @@ -60,19 +62,39 @@ 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, Handles::default()) +} + +/// What [`provider_handle`] uses from its surroundings: the env source of a +/// package-default key and the transport. Only this crate's tests set them. +#[derive(Default)] +struct Handles { + env: crate::shared::EnvSource, + fetch: Option, +} + +fn provider_handle_with( + name: &str, + api_key: Option, + options: Option, + handles: Handles, ) -> Result, AiMuxError> { let options = options.unwrap_or_default(); let mut headers: HeaderMapOpt = options @@ -84,16 +106,25 @@ pub fn provider_handle( .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), + fetch: handles.fetch, ..Default::default() - })?) + })?; + #[cfg(test)] + let provider = provider.with_env_source(handles.env); + #[cfg(not(test))] + let _ = 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())); @@ -109,21 +140,13 @@ 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`), recorded with source + // `none`. No `{NAME}_API_KEY` is read. + let api_key = api_key.unwrap_or_else(|| { + api_key_source = Some("none".into()); + PLACEHOLDER_API_KEY.into() + }); Arc::new(create_openai_compatible( OpenAICompatibleProviderSettings { name: name.into(), @@ -239,3 +262,290 @@ impl DecisionModel for HandleDecisionModel { self.inner.do_decide(options).await } } + +#[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::decide; + use aimux_core::recording::{self, RingRecorder}; + use aimux_core::replay::MockDecisionReplayModel; + use aimux_provider_utils::{ + Fetch, FetchError, FetchFunction, FetchRequest, FetchResponse, ReqwestFetch, + }; + use serde_json::Value; + use serial_test::serial; + use wiremock::matchers::{method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + 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 reads(env: &Arc>) -> Vec { + env.lock().unwrap().reads.clone() + } + + /// 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 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 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] + #[serial] + 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)); + } + } + + /// (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] + #[serial] + 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] + #[serial] + 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] + #[serial] + 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] + #[serial] + 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] + #[serial] + 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] + #[serial] + 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); + let recorder = Arc::new(RingRecorder::new()); + recording::init_recording(Some(recorder.clone())); + let live = decide( + handle.decision_model("gpt-6-luna").unwrap().as_ref(), + options("openai"), + ) + .await; + recording::init_recording(None); + live.unwrap(); + let records = recorder.completed(); + assert_eq!(records.len(), 1); + env.lock().unwrap().reads.clear(); + let replay = MockDecisionReplayModel::new("openai", "gpt-6-luna", records).unwrap(); + decide(&replay, options("openai")).await.unwrap(); + assert!(reads(&env).is_empty(), "{:?}", reads(&env)); + assert_eq!(server.received_requests().await.unwrap().len(), 1); + } +} 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)?)), } } } From 2c57b5f2d1365daa0260f78ff61528c370353631 Mon Sep 17 00:00:00 2001 From: Trey Kim Date: Sun, 11 Oct 2026 14:12:20 +0800 Subject: [PATCH 06/12] R.4: remove config_snapshot and api_key_source; decision recordings keep identity only (A6 item 1, D3) decide() records ProviderRecord::from_model(provider, model_id). Removed DecisionModel::config_snapshot and its jev/openai/runtime/handle overrides, DecisionEndpoint::snapshot, ProviderHandle's api_key_source and HandleDecisionModel, and ProviderRecord.api_key_source (ts-rs regenerated). Recording tests now assert no apiKeySource / baseUrl / providerOptions in the provider record; config_snapshot() callers rewritten to the recorded identity. --- aimux-core/src/decision_model.rs | 15 ++--- aimux-core/src/recording.rs | 5 -- aimux-providers/src/decision_support.rs | 41 ------------ aimux-providers/src/jev.rs | 18 ----- aimux-providers/src/openai/decision.rs | 7 -- aimux-providers/src/provider_handle.rs | 65 +++---------------- aimux-providers/src/runtime_decision.rs | 7 -- aimux-providers/tests/decision_handle_test.rs | 46 ++++++------- .../tests/decision_recording_test.rs | 11 +--- .../tests/runtime_decision_test.rs | 12 ++-- bindings/node/src/types/ProviderRecord.ts | 5 -- 11 files changed, 41 insertions(+), 191 deletions(-) diff --git a/aimux-core/src/decision_model.rs b/aimux-core/src/decision_model.rs index 1f2bae854..0aeabddff 100644 --- a/aimux-core/src/decision_model.rs +++ b/aimux-core/src/decision_model.rs @@ -550,15 +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 - } /// Perform one provider attempt. /// # Errors /// Returns provider, transport or invalid response errors. @@ -672,8 +663,10 @@ async fn decide_with_retry_delay( 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( diff --git a/aimux-core/src/recording.rs b/aimux-core/src/recording.rs index b1c08b9f4..2f5bcbb60 100644 --- a/aimux-core/src/recording.rs +++ b/aimux-core/src/recording.rs @@ -215,10 +215,6 @@ pub struct ProviderRecord { /// 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, @@ -237,7 +233,6 @@ impl ProviderRecord { provider: provider.to_string(), model_id: model_id.to_string(), base_url: None, - api_key_source: None, profile: None, provider_options: None, } diff --git a/aimux-providers/src/decision_support.rs b/aimux-providers/src/decision_support.rs index 984bf1c92..1353e317f 100644 --- a/aimux-providers/src/decision_support.rs +++ b/aimux-providers/src/decision_support.rs @@ -37,47 +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. The credential value is - /// never read; sensitive headers are redacted again when recorded. - pub(crate) fn snapshot( - &self, - provider: &str, - model_id: &str, - 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 }), - )); - 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 2017b5b0d..cf6f8384e 100644 --- a/aimux-providers/src/jev.rs +++ b/aimux-providers/src/jev.rs @@ -92,24 +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 capabilities(&self) -> DecisionCapabilities { DecisionCapabilities { supports_images: false, diff --git a/aimux-providers/src/openai/decision.rs b/aimux-providers/src/openai/decision.rs index 18c35d27d..4d926354f 100644 --- a/aimux-providers/src/openai/decision.rs +++ b/aimux-providers/src/openai/decision.rs @@ -398,13 +398,6 @@ impl DecisionModel for OpenAIDecisionModel { fn model_id(&self) -> &str { &self.model_id } - fn config_snapshot(&self) -> aimux_core::recording::ProviderRecord { - self.endpoint.snapshot( - self.provider(), - self.model_id(), - json!({"decision_protocol":"openai_decisions", "decision_capabilities":self.capabilities()}), - ) - } fn capabilities(&self) -> DecisionCapabilities { DecisionCapabilities { supports_images: true, diff --git a/aimux-providers/src/provider_handle.rs b/aimux-providers/src/provider_handle.rs index d6a8da0a7..671fa60a1 100644 --- a/aimux-providers/src/provider_handle.rs +++ b/aimux-providers/src/provider_handle.rs @@ -10,16 +10,13 @@ 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::{ 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 crate::openai::OpenAIProviderSettings; @@ -104,7 +101,6 @@ fn provider_handle_with( .into_iter() .map(|(name, value)| (name, Some(value))) .collect(); - let mut api_key_source = None; let inner: Arc = if name == "openai" { let auth_none = api_key.is_none() && options.base_url.is_some(); let provider = crate::create_openai(OpenAIProviderSettings { @@ -141,12 +137,8 @@ fn provider_handle_with( .or_else(|| non_empty_env(url_env)) .unwrap_or_else(|| (*default_url).into()); // Explicit key, else the placeholder `not-needed` (sent as - // `Authorization: Bearer not-needed`), recorded with source - // `none`. No `{NAME}_API_KEY` is read. - let api_key = api_key.unwrap_or_else(|| { - api_key_source = Some("none".into()); - PLACEHOLDER_API_KEY.into() - }); + // `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(), @@ -168,30 +160,17 @@ fn provider_handle_with( )? } }; - Ok(Box::new(ProviderHandle { - inner, - 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, - /// 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 Provider for ProviderHandle { fn decision_model(&self, id: &str) -> Result, AiMuxError> { - let model = self.inner.decision_model(id)?; - if self.api_key_source.is_none() { - return Ok(model); - } - Ok(Box::new(HandleDecisionModel { - inner: model, - api_key_source: self.api_key_source.clone(), - })) + self.inner.decision_model(id) } fn language_model(&self, id: &str) -> Result, AiMuxError> { self.inner.language_model(id) @@ -234,35 +213,6 @@ impl Provider for ProviderHandle { } } -/// A decision model with the handle's credential source. -struct HandleDecisionModel { - inner: Box, - api_key_source: Option, -} - -#[async_trait] -impl DecisionModel for HandleDecisionModel { - fn provider(&self) -> &str { - self.inner.provider() - } - fn model_id(&self) -> &str { - self.inner.model_id() - } - fn capabilities(&self) -> DecisionCapabilities { - self.inner.capabilities() - } - 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()); - } - snapshot - } - async fn do_decide(&self, options: &DecisionCallOptions) -> Result { - self.inner.do_decide(options).await - } -} - #[cfg(test)] mod env_tests { //! A6 item 4 / D23. The env a key could come from is an injected plain @@ -270,12 +220,13 @@ mod env_tests { //! environment and never `std::env::set_var`. use std::sync::{Arc, Mutex}; - use aimux_core::decision_model::decide; + 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 serial_test::serial; use wiremock::matchers::{method, path}; diff --git a/aimux-providers/src/runtime_decision.rs b/aimux-providers/src/runtime_decision.rs index c14eadb37..36888b35a 100644 --- a/aimux-providers/src/runtime_decision.rs +++ b/aimux-providers/src/runtime_decision.rs @@ -632,13 +632,6 @@ impl DecisionModel for RuntimeDecisionModel { fn model_id(&self) -> &str { &self.model_id } - fn config_snapshot(&self) -> aimux_core::recording::ProviderRecord { - self.endpoint.snapshot( - self.provider(), - self.model_id(), - json!({"decision_default_protocol":"systemone", "decision_capabilities":self.capabilities()}), - ) - } fn capabilities(&self) -> DecisionCapabilities { let provider = self.provider(); DecisionCapabilities { diff --git a/aimux-providers/tests/decision_handle_test.rs b/aimux-providers/tests/decision_handle_test.rs index 6e76b62cb..3181213ab 100644 --- a/aimux-providers/tests/decision_handle_test.rs +++ b/aimux-providers/tests/decision_handle_test.rs @@ -61,7 +61,7 @@ fn body_overrides_is_an_unknown_field() { } #[test] -fn config_snapshot_matches_model_identity() { +fn recorded_identity_matches_model_identity() { for (name, key) in [ ("openai", Some("sk-test")), ("vllm", Some("sk-test")), @@ -71,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; } } @@ -140,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(), @@ -157,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() { @@ -177,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() @@ -205,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() { @@ -252,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/runtime_decision_test.rs b/aimux-providers/tests/runtime_decision_test.rs index b8d21aa60..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] diff --git a/bindings/node/src/types/ProviderRecord.ts b/bindings/node/src/types/ProviderRecord.ts index 91cea4f62..b749e7eb0 100644 --- a/bindings/node/src/types/ProviderRecord.ts +++ b/bindings/node/src/types/ProviderRecord.ts @@ -26,11 +26,6 @@ modelId: string, * 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). */ From f5e72b032a66cd0bea589d172202eee13d97b807 Mon Sep 17 00:00:00 2001 From: Trey Kim Date: Sun, 11 Oct 2026 14:19:16 +0800 Subject: [PATCH 07/12] R.4: drop ProviderRecord base_url/profile/provider_options (identity only) Nothing writes them since config_snapshot is gone. Also removes the #226-only ProviderRecord::redacted and redact_provider_options. ProviderRecord is not deny_unknown_fields; a new test shows an old record carrying baseUrl / apiKeySource / profile / providerOptions still deserializes (and re-serializes as identity only). ts-rs regenerated. --- aimux-core/src/recording.rs | 53 ++++++++++------------- bindings/node/src/types/ProviderRecord.ts | 18 +------- 2 files changed, 24 insertions(+), 47 deletions(-) diff --git a/aimux-core/src/recording.rs b/aimux-core/src/recording.rs index 2f5bcbb60..dd506b006 100644 --- a/aimux-core/src/recording.rs +++ b/aimux-core/src/recording.rs @@ -210,18 +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, - /// 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 { @@ -232,9 +220,6 @@ impl ProviderRecord { provider_id: provider_id.to_string(), provider: provider.to_string(), model_id: model_id.to_string(), - base_url: None, - profile: None, - provider_options: None, } } @@ -246,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 交换。 @@ -659,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 { @@ -1996,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/bindings/node/src/types/ProviderRecord.ts b/bindings/node/src/types/ProviderRecord.ts index b749e7eb0..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,19 +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, -/** - * 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, }; From e8bfc0aa0ab56e56665b288019ecbc31c14f12e8 Mon Sep 17 00:00:00 2001 From: Trey Kim Date: Sun, 11 Oct 2026 15:01:30 +0800 Subject: [PATCH 08/12] node: refuse bad ProviderConfig in from_napi_value, no error state in the struct Drop #[napi(skip)] invalid: Option; from_napi_value now returns the InvalidArgumentError directly. The constructors' 3rd arg becomes ConfigArg (dispatch on JS type) because napi's Either swallowed the member error. --- bindings/node/__test__/error.test.ts | 6 +- bindings/node/src/lib.rs | 112 ++++++++++++++++----------- 2 files changed, 72 insertions(+), 46 deletions(-) diff --git a/bindings/node/__test__/error.test.ts b/bindings/node/__test__/error.test.ts index 9652e63fc..1c4b66574 100644 --- a/bindings/node/__test__/error.test.ts +++ b/bindings/node/__test__/error.test.ts @@ -293,7 +293,9 @@ for (const key of ['bodyOverrides', 'body_overrides', 'notAField']) { () => createProvider('groq', 'sk-test-fake-key', config), () => openai('sk-test-fake-key', 'gpt-4o', config), ]) { - const err = (await t.throwsAsync(make)) as AimuxError + // Native constructors refuse while converting the argument (a sync + // throw); the async wrappers reject. Either way it is the typed error. + const err = (await t.throwsAsync(async () => make())) as AimuxError t.true(err instanceof InvalidArgumentError) t.regex(err.message, new RegExp(`unknown field \`${key}\``)) t.notRegex(err.message, /no longer supported/) @@ -316,7 +318,7 @@ test('provider config maxRetries points to the call options, not a generic unkno () => createProvider('groq', 'sk-test-fake-key', config), () => openai('sk-test-fake-key', 'gpt-4o', config), ]) { - const err = (await t.throwsAsync(make)) as AimuxError + const err = (await t.throwsAsync(async () => make())) 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/) diff --git a/bindings/node/src/lib.rs b/bindings/node/src/lib.rs index c1e8a7b8f..96ed75840 100644 --- a/bindings/node/src/lib.rs +++ b/bindings/node/src/lib.rs @@ -644,12 +644,6 @@ pub struct ProviderConfig { /// name (`provider()` / `createProvider()`). Only parameters the preset /// declares are accepted. pub params: Option>, - /// Why the JS object was refused (a call-level key, an unknown field, a - /// mistyped value), reported as `InvalidArgumentError` by - /// [`native_config`] instead of being dropped. - #[napi(skip)] - #[serde(skip)] - pub invalid: Option, } /// Reject keys that belong somewhere else before the generic unknown-field @@ -680,30 +674,64 @@ impl FromNapiValue for ProviderConfig { let value = unsafe { serde_json::Value::from_napi_value(env, value.raw())? }; raw.insert(name, value); } - let invalid = |message: String| Self { - invalid: Some(message), - ..Self::default() + // A refused object (call-level key, unknown field, mistyped value) + // throws `InvalidArgumentError` right here; nothing is kept on the + // config itself. + let invalid = |message: String| -> Result { + let env = Env::from_raw(env); + Err(crate::error::create_throwable( + &env, + &AiMuxBindingError::from(AiMuxError::InvalidArgument(message)), + )?) }; if let Some(message) = reject_removed_keys(&raw) { - return Ok(invalid(message)); + return invalid(message); + } + match serde_json::from_value::(serde_json::Value::Object(raw)) { + Ok(config) => Ok(config), + Err(error) => invalid(error.to_string()), } - Ok( - serde_json::from_value::(serde_json::Value::Object(raw)) - .unwrap_or_else(|error| invalid(error.to_string())), - ) } } impl ValidateNapiValue for ProviderConfig {} -impl ProviderConfig { - /// The refusal recorded while reading the JS object, if any. - fn check(&self) -> MResult<()> { - match &self.invalid { - Some(message) => Err(AiMuxBindingError::from(&AiMuxError::InvalidArgument( - message.clone(), - ))), - None => Ok(()), +/// The native constructors' 3rd argument: a base URL string or a +/// [`ProviderConfig`]. Unlike napi's `Either`, which swallows the member +/// error and reports "Value is none of these types", this dispatches on the +/// JS type so a refused config object surfaces its own +/// `InvalidArgumentError`. +pub enum ConfigArg { + Url(String), + Config(ProviderConfig), +} + +impl TypeName for ConfigArg { + fn type_name() -> &'static str { + "string | ProviderConfig" + } + + fn value_type() -> ValueType { + ValueType::Unknown + } +} + +impl ValidateNapiValue for ConfigArg {} + +impl FromNapiValue for ConfigArg { + unsafe fn from_napi_value(env: sys::napi_env, napi_val: sys::napi_value) -> Result { + let value = unsafe { Unknown::from_napi_value(env, napi_val)? }; + match value.get_type()? { + ValueType::String => Ok(Self::Url(unsafe { + String::from_napi_value(env, napi_val)? + })), + ValueType::Object => Ok(Self::Config(unsafe { + ProviderConfig::from_napi_value(env, napi_val)? + })), + _ => Err(Error::new( + Status::InvalidArg, + "config must be a base URL string or a ProviderConfig object".to_owned(), + )), } } } @@ -720,7 +748,7 @@ struct NativeConfig { project: Option, } -fn native_config(config: Option>) -> MResult { +fn native_config(config: Option) -> MResult { let mut native = NativeConfig { base_url: None, headers: None, @@ -729,9 +757,8 @@ fn native_config(config: Option>) -> MResult {} - Some(Either::A(url)) => native.base_url = Some(url), - Some(Either::B(cfg)) => { - cfg.check()?; + Some(ConfigArg::Url(url)) => native.base_url = Some(url), + Some(ConfigArg::Config(cfg)) => { native.base_url = cfg.base_url; if let Some(json_str) = &cfg.headers { let h: std::collections::HashMap = @@ -1026,7 +1053,7 @@ 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, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1057,7 +1084,7 @@ 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, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1090,7 +1117,7 @@ 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, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1117,7 +1144,7 @@ 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, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1150,7 +1177,7 @@ 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, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1183,7 +1210,7 @@ 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, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1216,7 +1243,7 @@ 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, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1251,7 +1278,7 @@ 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, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1295,7 +1322,7 @@ pub async fn vertex( project: String, location: String, model_id: String, - config: Option>, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1332,7 +1359,7 @@ 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, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1375,7 +1402,7 @@ pub async fn azure( resource_name: String, deployment: String, api_version: Option, - config: Option>, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1425,7 +1452,7 @@ pub async fn provider( ) -> 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.map(ConfigArg::Config))?; let model = create_named_provider(&name, settings) .and_then(|p| p.language_model(&model_id)) .map_err(|e| AiMuxBindingError::from(&e))?; @@ -1537,7 +1564,7 @@ pub async fn create_provider( ) -> 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.map(ConfigArg::Config))?; let inner = create_named_provider(&name, settings).map_err(|e| AiMuxBindingError::from(&e))?; Ok(ProviderHandle { inner }) @@ -1567,12 +1594,9 @@ pub async fn get_model_specs(source_url: Option) -> AimuxResult /// The settings of a by-name provider from a Node `ProviderConfig` (3rd /// factory arg). Organization and project become their headers, below the /// caller's own. -fn named_settings( - api_key: Option, - config: Option>, -) -> MResult { +fn named_settings(api_key: Option, config: Option) -> MResult { let params = match &config { - Some(Either::B(cfg)) => cfg.params.clone().unwrap_or_default(), + Some(ConfigArg::Config(cfg)) => cfg.params.clone().unwrap_or_default(), _ => Default::default(), }; let native = native_config(config)?; From a59fdbc8b5bb7af7cbf481ab726f590c4b896bf2 Mon Sep 17 00:00:00 2001 From: Trey Kim Date: Sun, 11 Oct 2026 15:03:38 +0800 Subject: [PATCH 09/12] jev test: end-to-end retry exhaustion for 429 and 529 too (max_retries=1) --- aimux-providers/tests/jev_test.rs | 63 ++++++++++++++++++------------- 1 file changed, 37 insertions(+), 26 deletions(-) diff --git a/aimux-providers/tests/jev_test.rs b/aimux-providers/tests/jev_test.rs index 5f70ad48e..d57a2d21c 100644 --- a/aimux-providers/tests/jev_test.rs +++ b/aimux-providers/tests/jev_test.rs @@ -354,9 +354,9 @@ async fn invalid_requests_never_reach_http() { /// 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 single -/// `max_retries = 1` run proves a 503 is retried and then exhausted (one -/// real default backoff wait). +/// 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, retryable) in [ @@ -387,29 +387,40 @@ async fn official_provider_failures_use_standard_retry_rules_and_preserve_codes( assert_eq!(server.received_requests().await.unwrap().len(), 1); } - let server = MockServer::start().await; - mount( - &server, - 503, - json!({"error":{"message":"test failure", "code":"unavailable"}}), - ) - .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!("expected retry exhaustion, got {error:?}") - }; - assert_eq!( - retry.reason, - aimux_core::RetryErrorReason::MaxRetriesExceeded - ); - let AiMuxError::ApiCall(last) = retry.last_error() else { - panic!("expected API error") - }; - assert_eq!(last.status_code, Some(503)); - assert_eq!(last.provider_code.as_deref(), Some("unavailable")); - assert_eq!(server.received_requests().await.unwrap().len(), 2); + 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}" + ); + } } #[test] From 82dc7dc08c4d88e0f6a1d8b789dd63f8c474609f Mon Sep 17 00:00:00 2001 From: Trey Kim Date: Sun, 11 Oct 2026 15:09:24 +0800 Subject: [PATCH 10/12] provider_handle: test-only Handles under #[cfg(test)], drop #[serial] from env_tests Prod compiles no Handles / fetch / env plumbing. The env is injected, so the tests need no serialization; the replay test keeps only its own recording (unique state) since the recorder is process-global. --- aimux-providers/src/provider_handle.rs | 45 ++++++++++++++++---------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/aimux-providers/src/provider_handle.rs b/aimux-providers/src/provider_handle.rs index d6a8da0a7..e08579bf2 100644 --- a/aimux-providers/src/provider_handle.rs +++ b/aimux-providers/src/provider_handle.rs @@ -79,11 +79,18 @@ pub fn provider_handle( api_key: Option, options: Option, ) -> Result, AiMuxError> { - provider_handle_with(name, api_key, options, Handles::default()) + provider_handle_with( + name, + api_key, + options, + #[cfg(test)] + Handles::default(), + ) } -/// What [`provider_handle`] uses from its surroundings: the env source of a -/// package-default key and the transport. Only this crate's tests set them. +/// 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, @@ -94,7 +101,7 @@ fn provider_handle_with( name: &str, api_key: Option, options: Option, - handles: Handles, + #[cfg(test)] handles: Handles, ) -> Result, AiMuxError> { let options = options.unwrap_or_default(); let mut headers: HeaderMapOpt = options @@ -113,13 +120,12 @@ fn provider_handle_with( 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); - #[cfg(not(test))] - let _ = handles.env; Arc::new(if auth_none { provider.without_credential() } else { @@ -277,7 +283,6 @@ mod env_tests { Fetch, FetchError, FetchFunction, FetchRequest, FetchResponse, ReqwestFetch, }; use serde_json::Value; - use serial_test::serial; use wiremock::matchers::{method, path}; use wiremock::{Mock, MockServer, ResponseTemplate}; @@ -376,7 +381,6 @@ mod env_tests { /// never looks one up, so `OLLAMA_API_KEY` / `VLLM_API_KEY` never reach /// the wire. #[tokio::test] - #[serial] async fn generic_provider_api_key_env_is_not_read() { for name in ["ollama", "vllm"] { let server = mock("/v1/systemone", "runtime").await; @@ -414,7 +418,6 @@ mod env_tests { /// (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] - #[serial] async fn openai_default_env_is_read_at_request_time() { let server = mock("/v1/decisions", "openai").await; let env = env(&[]); @@ -436,7 +439,6 @@ mod env_tests { /// Unset default env fails the request (not the construction) with /// `LoadApiKey`, and nothing is sent. #[tokio::test] - #[serial] async fn openai_missing_default_env_fails_the_request() { let server = mock("/v1/decisions", "openai").await; let env = env(&[]); @@ -449,7 +451,6 @@ mod env_tests { /// (3) `Some("")` is used as given and never falls back to the env. #[tokio::test] - #[serial] 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")]); @@ -472,7 +473,6 @@ mod env_tests { /// (4a) A changed base URL without a key reads no env and sends no /// credential. #[tokio::test] - #[serial] 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")]); @@ -497,7 +497,6 @@ mod env_tests { /// (4b) `auth = none` (a keyless local runtime) reads no env. #[tokio::test] - #[serial] 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")]); @@ -526,25 +525,37 @@ mod env_tests { /// (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] - #[serial] 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(), - options("openai"), + request.clone(), ) .await; recording::init_recording(None); live.unwrap(); - let records = recorder.completed(); + 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, options("openai")).await.unwrap(); + decide(&replay, request).await.unwrap(); assert!(reads(&env).is_empty(), "{:?}", reads(&env)); assert_eq!(server.received_requests().await.unwrap().len(), 1); } From 6afdee6bdf4fe1d04c8e50cffcf4b43c8cb6aded Mon Sep 17 00:00:00 2001 From: Trey Kim Date: Sun, 11 Oct 2026 15:28:40 +0800 Subject: [PATCH 11/12] node: config errors reject the promise; take config as raw JSON Constructors accept the config argument as serde_json::Value (d.ts unchanged), so conversion never throws synchronously; maxRetries, unknown keys and wrong types are refused inside the async body as InvalidArgumentError. ConfigArg and the custom ProviderConfig FromNapiValue are gone. --- bindings/node/__test__/error.test.ts | 32 ++++- bindings/node/src/lib.rs | 184 +++++++++++++-------------- 2 files changed, 119 insertions(+), 97 deletions(-) diff --git a/bindings/node/__test__/error.test.ts b/bindings/node/__test__/error.test.ts index 1c4b66574..c2da5b8e6 100644 --- a/bindings/node/__test__/error.test.ts +++ b/bindings/node/__test__/error.test.ts @@ -7,6 +7,7 @@ import test from 'ava' import { createServer, type Server } from 'node:http' import { openai, + anthropic, provider, createProvider, mockReplay, @@ -293,9 +294,11 @@ for (const key of ['bodyOverrides', 'body_overrides', 'notAField']) { () => createProvider('groq', 'sk-test-fake-key', config), () => openai('sk-test-fake-key', 'gpt-4o', config), ]) { - // Native constructors refuse while converting the argument (a sync - // throw); the async wrappers reject. Either way it is the typed error. - const err = (await t.throwsAsync(async () => make())) as AimuxError + // 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/) @@ -318,13 +321,34 @@ test('provider config maxRetries points to the call options, not a generic unkno () => createProvider('groq', 'sk-test-fake-key', config), () => openai('sk-test-fake-key', 'gpt-4o', config), ]) { - const err = (await t.throwsAsync(async () => make())) as AimuxError + 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, diff --git a/bindings/node/src/lib.rs b/bindings/node/src/lib.rs index 96ed75840..15cb849ce 100644 --- a/bindings/node/src/lib.rs +++ b/bindings/node/src/lib.rs @@ -656,86 +656,51 @@ fn reject_removed_keys(raw: &serde_json::Map) -> Opti }) } -/// The JS object is read as a raw JSON map (`undefined` values skipped, like -/// absent keys), checked by [`reject_removed_keys`], then deserialized with -/// serde `deny_unknown_fields`: the struct above is the single list of -/// accepted keys. -impl FromNapiValue for ProviderConfig { - unsafe fn from_napi_value(env: sys::napi_env, napi_val: sys::napi_value) -> Result { - let obj = unsafe { Object::from_napi_value(env, napi_val)? }; - let names = obj.get_property_names()?; - let mut raw = serde_json::Map::new(); - for i in 0..names.get_array_length()? { - let name: String = names.get_element(i)?; - let value: Unknown = obj.get_named_property(&name)?; - if value.get_type()? == ValueType::Undefined { - continue; +/// 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)); } - let value = unsafe { serde_json::Value::from_napi_value(env, value.raw())? }; - raw.insert(name, value); - } - // A refused object (call-level key, unknown field, mistyped value) - // throws `InvalidArgumentError` right here; nothing is kept on the - // config itself. - let invalid = |message: String| -> Result { - let env = Env::from_raw(env); - Err(crate::error::create_throwable( - &env, - &AiMuxBindingError::from(AiMuxError::InvalidArgument(message)), - )?) - }; - if let Some(message) = reject_removed_keys(&raw) { - return invalid(message); - } - match serde_json::from_value::(serde_json::Value::Object(raw)) { - Ok(config) => Ok(config), - Err(error) => invalid(error.to_string()), + 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", + } + ))), } } -impl ValidateNapiValue for ProviderConfig {} - -/// The native constructors' 3rd argument: a base URL string or a -/// [`ProviderConfig`]. Unlike napi's `Either`, which swallows the member -/// error and reports "Value is none of these types", this dispatches on the -/// JS type so a refused config object surfaces its own -/// `InvalidArgumentError`. -pub enum ConfigArg { +/// A parsed config argument. +enum NativeArg { Url(String), Config(ProviderConfig), } -impl TypeName for ConfigArg { - fn type_name() -> &'static str { - "string | ProviderConfig" - } - - fn value_type() -> ValueType { - ValueType::Unknown - } -} - -impl ValidateNapiValue for ConfigArg {} - -impl FromNapiValue for ConfigArg { - unsafe fn from_napi_value(env: sys::napi_env, napi_val: sys::napi_value) -> Result { - let value = unsafe { Unknown::from_napi_value(env, napi_val)? }; - match value.get_type()? { - ValueType::String => Ok(Self::Url(unsafe { - String::from_napi_value(env, napi_val)? - })), - ValueType::Object => Ok(Self::Config(unsafe { - ProviderConfig::from_napi_value(env, napi_val)? - })), - _ => Err(Error::new( - Status::InvalidArg, - "config must be a base URL string or a ProviderConfig object".to_owned(), - )), - } - } -} - /// What the native constructors take from their 3rd argument (a base URL /// string, or a [`ProviderConfig`]). Every native constructor goes through /// here, so `maxRetries` and unknown keys are rejected for all of them and @@ -748,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, @@ -757,8 +726,8 @@ fn native_config(config: Option) -> MResult { }; match config { None => {} - Some(ConfigArg::Url(url)) => native.base_url = Some(url), - Some(ConfigArg::Config(cfg)) => { + 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 = @@ -1053,7 +1022,9 @@ pub fn list_sessions() -> AimuxResult { pub async fn openai( api_key: String, model_id: String, - #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option< + serde_json::Value, + >, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1084,7 +1055,9 @@ pub async fn openai( pub async fn anthropic( api_key: String, model_id: String, - #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option< + serde_json::Value, + >, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1117,11 +1090,13 @@ pub async fn anthropic( pub async fn deepseek( api_key: String, model_id: String, - #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] 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))?; @@ -1144,7 +1119,9 @@ pub async fn deepseek( pub async fn google( api_key: String, model_id: String, - #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option< + serde_json::Value, + >, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1177,7 +1154,9 @@ pub async fn google( pub async fn cohere( api_key: String, model_id: String, - #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option< + serde_json::Value, + >, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1210,7 +1189,9 @@ pub async fn cohere( pub async fn mistral( api_key: String, model_id: String, - #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option< + serde_json::Value, + >, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1243,7 +1224,9 @@ pub async fn mistral( pub async fn xai( api_key: String, model_id: String, - #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option< + serde_json::Value, + >, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1278,7 +1261,9 @@ pub async fn bedrock( secret_access_key: String, region: String, model_id: String, - #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option< + serde_json::Value, + >, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1322,7 +1307,9 @@ pub async fn vertex( project: String, location: String, model_id: String, - #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option< + serde_json::Value, + >, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1359,7 +1346,9 @@ pub async fn anthropic_aws( api_key: String, region: String, model_id: String, - #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option< + serde_json::Value, + >, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1402,7 +1391,9 @@ pub async fn azure( resource_name: String, deployment: String, api_version: Option, - #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option, + #[napi(ts_arg_type = "string | ProviderConfig | undefined | null")] config: Option< + serde_json::Value, + >, ) -> AimuxResult { AimuxResult({ let __r: crate::error::MResult = async { @@ -1448,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(ConfigArg::Config))?; + 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))?; @@ -1560,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(ConfigArg::Config))?; + let settings = named_settings(api_key, config, false)?; let inner = create_named_provider(&name, settings).map_err(|e| AiMuxBindingError::from(&e))?; Ok(ProviderHandle { inner }) @@ -1594,12 +1585,19 @@ pub async fn get_model_specs(source_url: Option) -> AimuxResult /// The settings of a by-name provider from a Node `ProviderConfig` (3rd /// factory arg). Organization and project become their headers, below the /// caller's own. -fn named_settings(api_key: Option, config: Option) -> MResult { +fn named_settings( + api_key: Option, + config: Option, + allow_url: bool, +) -> MResult { + let config = config + .map(|value| parse_config(value, allow_url)) + .transpose()?; let params = match &config { - Some(ConfigArg::Config(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), From 961ac53491f6227825bf95260f23b2bbcfd7b0c4 Mon Sep 17 00:00:00 2001 From: Trey Kim Date: Sun, 11 Oct 2026 16:34:14 +0800 Subject: [PATCH 12/12] node: tests for boolean config (rejects) and null config (resolves as absent) --- bindings/node/__test__/error.test.ts | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/bindings/node/__test__/error.test.ts b/bindings/node/__test__/error.test.ts index c2da5b8e6..c3e34fd52 100644 --- a/bindings/node/__test__/error.test.ts +++ b/bindings/node/__test__/error.test.ts @@ -356,3 +356,24 @@ test('provider config keys set to undefined are treated as absent', async (t) => }) 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)) +})