Uh oh!
There was an error while loading. Please reload this page.
fix: #302 Phase A clean cut — drop Provider enumeration for catalog vendors (closes AISIX-Cloud#417) - #375
Merged
Merged
Conversation
…ider enumeration for catalog vendors Closesapi7/AISIX-Cloud#417 and lands the dispatch half of api7/AISIX-Cloud#302 Phase A. ## The bug class this kills Pre-#302 the DP enumerated every catalog vendor as a closed `Provider` enum variant. cp-api admitted any models.dev provider (xai, openrouter, future long-tail) but `Model` rows with an un-enumerated `provider` string failed `validate_model` at snapshot load — silently dropped to `stats.rejections`. The customer's chat got 404 model_not_found instead of a routable request. Adding `Provider::Xai` (or any other vendor) to fix one instance just repaints the bug; the next long-tail repeats it. ## Phase A clean cut in this PR **Schema (`crates/aisix-core/src/models/schema.rs`)**: - `model_schema()` `provider` field: closed-enum → `{type:"string", minLength:1}`. Any catalog vendor admits. **Model entity (`crates/aisix-core/src/models/model.rs`)**: - `Model.provider`: `Option<Provider>` → `Option<String>`. Free-form vendor identity, informational only — routing reads `ProviderKey`. - `Provider` enum trimmed from 17 variants to 6 first-class (`Openai`, `Anthropic`, `Google`, `Deepseek`, `Cohere`, `Jina`) — the only ones that have specialized dispatch code paths (Anthropic native `/v1/messages`, Cohere/Jina native `/v1/rerank`, Deepseek `reasoning_content` lift, etc.). `default_base_url` removed (the DP does not enumerate per-vendor URLs anymore). - 11 long-tail variants deleted: Groq / Mistral / Togetherai / FireworksAi / Perplexity / Moonshotai / Alibaba / Zhipuai / Baseten / Huggingface / Cerebras. **Hub (`crates/aisix-gateway/src/hub.rs`)**: - Drop `bridges: DashMap<Provider, ...>` per-Provider registry + `register(Provider, ...)` + `get(Provider)` + `providers()` / `len()` / `is_empty()`. - Hub now has only two tiers: `specialized_bridges` (open string vendor) + `family_bridges` (closed 5-value `Adapter`). `dispatch_two_tier` is the only dispatch entry point. **Dispatch (`crates/aisix-proxy/src/dispatch.rs`)**: - `resolve_bridge(hub, pk, provider: Provider)` → `resolve_bridge(hub, pk)`. Legacy fallback dropped. - `require_provider(model) -> Provider` → `-> &str` (vendor id for logs/metrics; not used for routing). - `resolve_base_url(provider, pk) -> String` → `resolve_base_url(pk) -> Result<String, ProxyError>`. Empty `api_base` errors loud — cp-api must populate api_base for every catalog vendor. **Proxy handlers**: - `audio.rs` / `images.rs` / `completions.rs` / `messages.rs` / `responses.rs` / `embeddings.rs` / `chat.rs` / `background.rs` / `rerank.rs` / `passthrough.rs`: all dispatch via `ProviderKey` through `Hub::dispatch_two_tier`; the per-Provider preflight `hub.get(provider).is_some()` checks become `resolve_bridge(hub, &pk).is_some()`. - `images.rs` / `responses.rs` / `messages.rs` endpoint guards compare `model.provider.as_deref() != Some("openai" | "anthropic")` (string compare) instead of `Provider::Xxx` enum match. - `rerank.rs` Cohere/Jina dispatch already keyed on `model.provider` string (#213 Phase 2 pattern); fixed Arc move. **`build_hub()` (`crates/aisix-server/src/main.rs`)**: - Delete 11 long-tail per-Provider registrations. - Family bridges: `Adapter::Openai` + `Adapter::Anthropic` + `Adapter::Vertex` + `Adapter::AzureOpenai` + `Adapter::Bedrock` (all 5). - Specialized vendor bridges: `openai` / `anthropic` (canonical labels), `google` (Gemini openai-compat), `deepseek` (reasoning lift), `cohere` (chat-compat namespace). **OpenAiBridge (`crates/aisix-provider-openai/src/bridge.rs`)**: - No changes — already handles `api_base` correctly. The 11 long-tail `default_base` arms are now unreachable dead code via the legacy `with_name` instances that were registered; they remain in the file but no live PK can hit them (cp-api populates `api_base` for every catalog vendor). **Schema regen**: `schemas/resources/model.schema.json` re-emitted; the closed-enum block on `provider` is gone, replaced with a free-form `{type:"string", minLength:1}`. ## Net effect Any new long-tail vendor cp-api admits (xai, openrouter, or one we haven't heard of yet) routes through the `Adapter::Openai` family bridge with no DP code change. Adding a vendor takes a single `adapter_map.yaml` line in cp-api, not a DP enum variant + register + schema entry round-trip. ## Test plan - [x] `cargo test --workspace` — all green (1085+ tests, 0 failed) - [x] `cargo fmt --all -- --check` — clean - [x] `cargo clippy --workspace --all-targets -- -D warnings` — clean (one pre-existing `too_many_arguments` suppressed at the function boundary, not introduced by this PR) - [x] Schema regen committed - [ ] E2E xai chat round-trip (deferred — needs rebuilt DP image; tracked in api7/AISIX-Cloud#430) ## Net diff 23 files changed, 531 insertions(+), 834 deletions(-). Net deletion.
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (25)
Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. Comment |
There was a problem hiding this comment.
Pull request overview
This PR removes the data-plane’s need to enumerate every catalog vendor as a closed Provider enum by switching dispatch to a two-tier lookup keyed off ProviderKey (specialized vendor → adapter family). It also opens Model.provider from a closed enum to a free-form string so newly admitted vendors (e.g. xai, openrouter) won’t be schema-rejected at snapshot load.
Changes:
- Opened
Model.providerschema from enum → non-empty string (and regenerated the published JSON schema). - Refactored Hub/dispatch to remove the legacy
Provider-keyed registry and route viaHub::dispatch_two_tierusingProviderKey.provider+ProviderKey.adapter. - Updated proxy handlers/tests to use
ProviderKey-based resolution and string-based provider guards.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| schemas/resources/model.schema.json | Regenerated published schema to make provider a free-form string (and removed the Provider definition block). |
| crates/aisix-server/src/main.rs | Updates hub construction to register adapter-family bridges and specialized vendor overrides (no per-vendor Provider enum registry). |
| crates/aisix-proxy/src/responses.rs | Switches provider checks/base URL resolution to string/ProviderKey-based routing. |
| crates/aisix-proxy/src/rerank.rs | Adjusts provider label derivation and ProviderKey test fixtures for the new shapes. |
| crates/aisix-proxy/src/passthrough.rs | Updates provider matching to use Option<String>/as_deref() instead of Provider. |
| crates/aisix-proxy/src/models.rs | Updates /v1/models “owned_by” derivation to use Option<String> provider. |
| crates/aisix-proxy/src/messages.rs | Reworks Anthropic vs cross-provider dispatch branching and ProviderKey-based base URL/bridge resolution. |
| crates/aisix-proxy/src/lib.rs | Updates many proxy integration tests to use specialized vendor registration + ProviderKey adapter/provider fields. |
| crates/aisix-proxy/src/images.rs | Updates OpenAI-only guard and bridge resolution to the ProviderKey-based dispatch path. |
| crates/aisix-proxy/src/embeddings.rs | Updates bridge resolution to ProviderKey-based dispatch and adjusts tests accordingly. |
| crates/aisix-proxy/src/dispatch.rs | Removes legacy Provider fallback; resolve_bridge now only uses dispatch_two_tier; base URL resolution now errors if api_base missing. |
| crates/aisix-proxy/src/completions.rs | Updates bridge resolution to ProviderKey-based dispatch and adjusts tests accordingly. |
| crates/aisix-proxy/src/chat.rs | Updates preflight and dispatch to ProviderKey-based bridge resolution and string-based provider labels. |
| crates/aisix-proxy/src/background.rs | Updates background model-check dispatch to resolve bridges via ProviderKey-based lookup. |
| crates/aisix-proxy/src/audio.rs | Updates base URL resolution to ProviderKey-based lookup and adjusts tests accordingly. |
| crates/aisix-gateway/src/hub.rs | Removes Provider-keyed registry and exposes specialized + family bridge tiers plus dispatch_two_tier. |
| crates/aisix-gateway/src/bridge.rs | Updates tests to assert Model.provider is now a string. |
| crates/aisix-etcd/src/supervisor.rs | Updates schema-rejection tests to use a real schema violation now that provider is open string. |
| crates/aisix-etcd/src/loader.rs | Same as supervisor: updates rejection-path tests post-schema change. |
| crates/aisix-core/src/models/schema.rs | Opens Model.provider in the runtime JSON schema and adds tests for arbitrary provider strings. |
| crates/aisix-core/src/models/model.rs | Changes Model.provider to Option<String> and trims Provider enum to only first-class/specialized vendors. |
| crates/aisix-admin/src/playground_handler.rs | Updates tests to register specialized bridges and populate ProviderKey adapter/provider fields. |
| crates/aisix-admin/src/lib.rs | Updates admin tests: “unknown provider” is no longer a schema error; uses empty display_name as the rejection sentinel. |
Comments suppressed due to low confidence (2)
crates/aisix-proxy/src/audio.rs:304
provideris now a&strfromrequire_provider, soformat!("{provider:?}")will include quotes (e.g. ""openai"") and will leak into logs/metrics labels. Useprovider.to_ascii_lowercase()(orprovider.to_lowercase()) instead of Debug formatting for the label.
This issue also appears on line 423 of the same file.
let base = crate::dispatch::resolve_base_url(&pk_entry.value)?;
// build_v1_url owns the /v1 prefix; callers pass the suffix
// (e.g. `/audio/transcriptions`) so this code is agnostic to
// whether the customer's api_base ends in /v1 or not.
let url = crate::dispatch::build_v1_url(&base, upstream_path);
crates/aisix-proxy/src/audio.rs:427
- Same issue as multipart path:
provideris&str, soformat!("{provider:?}")adds quotes and corrupts the provider label used for access logs/metrics. Preferprovider.to_ascii_lowercase()for the label.
let base = crate::dispatch::resolve_base_url(&pk_entry.value)?;
let provider_label = format!("{provider:?}").to_lowercase();
// Rewrite model field.
if let Some(m) = body.get_mut("model") {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+37
to
+42
| /// Returns `None` when the ProviderKey carries no `adapter` (a | ||
| /// pre-Phase-A row that escaped the schema migration) AND no | ||
| /// specialized bridge is registered for its vendor string. Caller | ||
| /// surfaces this as 503 "no dispatch path". | ||
| pub(crate) fn resolve_bridge(hub: &Hub, provider_key: &ProviderKey) -> Option<Arc<dyn Bridge>> { | ||
| hub.dispatch_two_tier(provider_key) |
Comment on lines
115
to
120
| @@ -116,7 +116,7 @@ async fn dispatch( | |||
| let provider = crate::dispatch::require_provider(model)?; | |||
| let pk_entry = crate::dispatch::resolve_provider_key(&snapshot, model)?; | |||
| let bridge = crate::dispatch::resolve_bridge(&state.hub, &pk_entry.value, provider) | |||
| let bridge = crate::dispatch::resolve_bridge(&state.hub, &pk_entry.value) | |||
| .ok_or(ProxyError::ProviderUnavailable)?; | |||
Comment on lines
135
to
140
| @@ -136,7 +136,7 @@ async fn dispatch( | |||
| let provider = crate::dispatch::require_provider(model)?; | |||
| let pk_entry = crate::dispatch::resolve_provider_key(&snapshot, model)?; | |||
| let bridge = crate::dispatch::resolve_bridge(&state.hub, &pk_entry.value, provider) | |||
| let bridge = crate::dispatch::resolve_bridge(&state.hub, &pk_entry.value) | |||
| .ok_or(ProxyError::ProviderUnavailable)?; | |||
Comment on lines
538
to
543
| @@ -534,7 +539,7 @@ async fn dispatch( | |||
| let provider = crate::dispatch::require_provider(model).map_err(with_model)?; | |||
| let pk_entry = | |||
| crate::dispatch::resolve_provider_key(&snapshot, model).map_err(with_model)?; | |||
| let bridge = crate::dispatch::resolve_bridge(&state.hub, &pk_entry.value, provider) | |||
| let bridge = crate::dispatch::resolve_bridge(&state.hub, &pk_entry.value) | |||
| .ok_or_else(|| with_model(ProxyError::ProviderUnavailable))?; | |||
Comment on lines
+54
to
58
| "description": "Upstream vendor identity, free-form string (e.g. `\"openai\"`, `\"xai\"`, `\"openrouter\"`, any models.dev catalog id). Carried through to telemetry / logs but **not consumed by dispatch** — routing reads `ProviderKey.adapter` + `ProviderKey.provider` instead, so a new long-tail vendor admitted by cp-api works without a DP code change. None for routing models.\n\nCloses the schema-validation half of api7/AISIX-Cloud#417 and the dispatch half of api7/AISIX-Cloud#302 Phase A.", | ||
| "type": [ | ||
| "string", | ||
| "null" | ||
| ] |
…rds + compat shim + metric labels Round-1 audit on #375 flagged three HIGH: ## HIGH-1 (CRITICAL): family bridges silently routed to api.openai.com / api.anthropic.com The previous PR (#365) had a defensive guard in `OpenAiBridge::resolve_base` that refused to fall back to `OPENAI_DEFAULT_BASE` when the bridge was serving a non-openai vendor with empty `api_base`. That guard was dropped in the Phase A rewrite. After the schema enum was opened, an xai PK with empty `api_base` would route through the family bridge → fall back to `https://api.openai.com/v1` → leak the xai secret as a Bearer token to OpenAI. Same hole on the Anthropic side. **Fix**: restore the guard in both bridges. `resolve_base` now returns `Result<String, BridgeError>`; an empty `api_base` + non-openai (or non-anthropic) `ProviderKey.provider` returns `BridgeError::Config` instead of falling back. Vendor string is normalized (trim + ascii_lowercase) before comparing so `"OpenAI"` / `"openai "` cannot bypass. `crates/aisix-provider-openai/src/bridge.rs::resolve_base` + 5 production call sites use `?`. 14 test call sites use `.unwrap()`. Equivalent change in `crates/aisix-provider-anthropic/src/bridge.rs::resolve_base`. New tests: - `family_bridge_refuses_non_openai_vendor_with_empty_api_base` (covers openrouter / xai / case variants / whitespace) - `family_bridge_allows_openai_vendor_with_empty_api_base` - `family_bridge_allows_legacy_empty_provider_with_empty_api_base` - `family_bridge_allows_non_openai_vendor_with_populated_api_base` ## HIGH-2: `format!("{provider:?}")` on `&str` emits quoted strings in metric labels `require_provider` returns `&str` post-refactor, but five sites still built provider labels with `format!("{provider:?}").to_lowercase()` — `Debug` on `&str` quotes the value, so Prometheus labels became `"\"openai\""` instead of `"openai"`, silently breaking dashboards. **Fix**: `completions.rs:126`, `audio.rs:305,424`, `embeddings.rs:175,186`, `chat.rs:570,690` switched to `provider.to_ascii_lowercase()` (the same call `images.rs:141` and `messages.rs:636` were already using post-refactor). ## HIGH-3: pre-Phase-A PK rows with empty `provider` + `adapter: None` returned 503 A clean cut without a migration step would 503 every chat request through an existing on-disk PK row that hadn't been re-saved through cp-api's Phase B marshaler. **Fix**: `crates/aisix-proxy/src/dispatch.rs::resolve_bridge` now takes a third arg `model_provider: Option<&str>`. After the two-tier dispatch path misses, if the PK carries both empty `provider` and `adapter: None` (pre-Phase-A on-disk shape), fall back to `hub.get_specialized(model_provider)`. cp-api now writes both fields on every PK; once the operator's pre-cutover rows have been re-saved, the fallback path becomes unreachable. All 8 production call sites of `resolve_bridge` updated. New tests: - `legacy_pk_with_empty_fields_falls_back_to_model_provider` - `compat_shim_does_not_fire_for_post_phase_a_pk` (regression-guard: a future PR that drops `Adapter::Openai` family must FAIL the family test, not get rescued by the shim) ## MEDIUM-1: `provider` schema was unbounded free-form string (log injection / cardinality risk) cp-api admits arbitrary strings → flows into `state.metrics.record_request` labels and `tracing::warn!` lines. A crafted `provider: "line1\nline2:fake"` could inject a log entry; a crafted long string could blow Prometheus label cardinality. **Fix**: `crates/aisix-core/src/models/schema.rs:120` adds `"maxLength": 64, "pattern": "^[a-z0-9][a-z0-9_-]*$"`. Every models.dev catalog id satisfies this pattern. ## MEDIUM-2: stale chat.rs comment referencing the removed legacy fallback `chat.rs:892-895` claimed `resolve_bridge` "falls back to the legacy Provider-keyed registry" — false post-refactor. Misleading on cutover risk. **Fix**: rewritten to describe the actual two-tier + compat-shim flow. ## MEDIUM-3: dead `From<Provider> for Adapter` impl The conversion was only referenced by its own test post-refactor. Latent maintenance hazard. **Fix**: deleted both the impl and `adapter_from_provider_covers_every_variant`. `ProviderKey.adapter` is the authoritative Adapter identity; `Model.provider → Adapter` mapping has no caller. ## LOW-2: Anthropic family test could pass with wrong bridge type The test `build_hub_registers_anthropic_family_bridge` only checked `bridge.name() == "anthropic"` — would still pass if a specialized `"some-anthropic-compat" → AnthropicBridge` registration shadowed the family tier. **Fix**: pre-flight assertion that `hub.get_specialized("some-anthropic-compat")` is `None`, so the dispatch must come from the family tier specifically. ## Test plan - [x] `cargo test --workspace --no-fail-fast` — 1090+ tests, 0 failed - [x] `cargo fmt --all -- --check` — clean - [x] `cargo clippy --workspace --all-targets -- -D warnings` — clean ## What is NOT addressed in this commit - **LOW-1** (no end-to-end xai test in this PR): deferred to api7/AISIX-Cloud#430 — needs rebuilt aisix-e2e-api + DP image. Tracked in the e2e companion branch `test/issue-417-xai-e2e`. ## Net delta 13 files, 312 insertions, 115 deletions.
…regression-guard + deprecation telemetry Round-2 audit found: ## HIGH (new): schema regex rejected `wafer.ai` The MEDIUM-1 fix in commit 3fc4de4 added `pattern: "^[a-z0-9][a-z0-9_-]*$"` to guard against log-injection / cardinality explosion. The audit's live check against `https://models.dev/api.json` found one real catalog id (`wafer.ai`) that contains a dot — the new pattern rejected it, re-creating the exact #417 bug class for that vendor. **Fix**: broaden pattern to `^[a-z0-9][a-z0-9._-]*$` (include `.`). Added positive tests for `wafer.ai`, `fireworks-ai`, `togetherai`, and a negative-tests block for log-injection / case / leading-punct / NUL-byte cases the original concern motivated. ## MEDIUM-1: regression-guard test didn't pin the contract `compat_shim_does_not_fire_for_post_phase_a_pk` used `adapter:None` — `dispatch_two_tier`'s `pk.adapter?` short-circuits to None regardless of `Adapter::Openai` family registration, so the test passed vacuously. A future PR that drops the family registration would not have failed this test. **Fix**: rewritten as `compat_shim_does_not_rescue_missing_family_for_post_phase_a_pk` using `adapter:Some(Openai)` + `provider:"vendor-without-specialized"` + no family registered. Two-tier path goes: specialized miss → family miss → returns None. Compat shim must NOT fire because `provider` is non-empty. If a future PR drops the family registration, this test fires loud. ## MEDIUM-3: compat shim was silent `resolve_bridge` fell through to `hub.get_specialized(model_provider)` for pre-Phase-A PKs without any signal that the legacy path fired. The "one-cycle" deprecation promise was unenforceable. **Fix**: added `tracing::warn!(target: "aisix_proxy::dispatch", pk_display_name, model_provider, ...)` inside the shim. Operators / SREs grep logs for the target to detect un-migrated PK rows still in production. ## Test plan - [x] `cargo test --workspace` — 1090+ tests, 0 failed (added `model_accepts_arbitrary_provider_string` extension + `model_rejects_provider_strings_outside_pattern` + `compat_shim_does_not_rescue_missing_family_for_post_phase_a_pk`) - [x] `cargo fmt --all -- --check` — clean - [x] `cargo clippy --workspace --all-targets -- -D warnings` — clean - [x] Schema regen via `cargo run -p aisix-core --bin dump-schema` - [x] E2E `dp-catalog-non-featured-routing-live.spec.ts` — 1 passed, 14.1s, against `aisix:phase-a-clean-cut` DP image + `aisix-e2e-api` rebuilt from current AISIX-Cloud branch
Round-3 audit noted that `maxLength: 64` on the provider schema had no test coverage — a regression that dropped the cap would silently allow ~10KB vendor strings into Prometheus label cardinality. Adds a one-line negative test asserting strings > 64 chars are rejected. Round-3 audit summary: all round-1 and round-2 HIGH/MEDIUM findings correctly closed. One LOW deferred — compat-shim `tracing::warn!` fires per-request inside the legacy branch, which could be noisy on heavily-loaded un-migrated PKs. Filed as a follow-up; not a merge blocker (operators want the migration-debt signal).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
crates/aisix-gateway/src/hub.rs:96
dispatch_two_tierdoes an exact, case-sensitive lookup onpk.provider(ProviderKey.provider) with no trimming/normalization. SinceProviderKey.provideris currently just a free-form string in the schema, a value like"DeepSeek"or" deepseek "would silently miss the specialized bridge and may change behavior (e.g. skipping DeepSeek-specific handling) or even fail dispatch ifadapteris unset. Consider normalizing vendor ids at insertion/lookup (trim + lowercase) or tighteningProviderKey.providervalidation to enforce the canonical form.
pub fn dispatch_two_tier(&self, pk: &ProviderKey) -> Option<Arc<dyn Bridge>> {
if let Some(b) = self.specialized_bridges.get(&pk.provider) {
return Some(b.clone());
}
let adapter = pk.adapter?;
Comment on lines
+55
to
+57
| "type": [ | ||
| "string", | ||
| "null" |
Comment on lines
174
to
178
| /// The upstream base URL: `provider_key.api_base` override if set, | ||
| /// otherwise the `Provider`'s built-in default. Tolerates an operator | ||
| /// pasting the full upstream URL into `api_base` by stripping any | ||
| /// trailing endpoint suffix — see [`API_BASE_ENDPOINT_SUFFIXES`] for | ||
| /// the full list and [`build_v1_url`] for the matching `/v1` synthesis. |
Comment on lines
+547
to
+551
| /// Every first-class `Provider` variant must have a non-empty | ||
| /// `as_str` wire id and a working `Adapter::from` arm. A | ||
| /// regression that added a new variant but forgot to update | ||
| /// either would compile fine but silently break dispatch | ||
| /// downstream. |
Uh oh!
There was an error while loading. Please reload this page.
moonming
commented
May 22, 2026
MemberAuthor
Round-4 update: pure clean cut (option A)Per user direction, this update completes the deletion the soft-deprecated path had left behind:
Kept (compat shim): Stats: −464 net LOC ( Cross-PR test dependency: AISIX-Cloud#464
Filed api7/AISIX-Cloud#464 for the test update on the AISIX-Cloud side; not modifying the test files myself per source-blind e2e rule. The 4 cell-failures in the matrix suite are the expected fallout and will resolve once #464 lands. AuditRound-4 audit will run against this push. |
This was referenced May 22, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closesapi7/AISIX-Cloud#417 and lands the dispatch half of api7/AISIX-Cloud#302 Phase A. Supersedes #365 (band-aid Provider::Xai approach, closed).
The bug class this kills
Pre-#302 the DP enumerated every catalog vendor as a closed
Providerenum variant. cp-api admitted any models.dev provider (xai, openrouter, future long-tail) butModelrows with an un-enumeratedproviderstring failedvalidate_modelat snapshot load — silently dropped tostats.rejections. Customer chat → 404 model_not_found. AddingProvider::Xaijust repaints the bug; the next long-tail repeats it.What this PR does
Schema (
crates/aisix-core/src/models/schema.rs)model_schema()providerfield: closed-enum →{type:"string", minLength:1, maxLength:64, pattern:"^[a-z0-9][a-z0-9._-]*$"}. Pattern accepts the dot character because at least one real models.dev id (wafer.ai) contains it; bounded length + character set guard against log-injection / Prometheus cardinality explosion.Model entity (
crates/aisix-core/src/models/model.rs)Model.provider:Option<Provider>→Option<String>. Vendor identity is open string; routing readsProviderKey.Providerenum trimmed 17 → 6 first-class variants. 11 long-tail variants deleted (Groq / Mistral / Togetherai / FireworksAi / Perplexity / Moonshotai / Alibaba / Zhipuai / Baseten / Huggingface / Cerebras).default_base_urlmethod removed — DP no longer enumerates per-vendor URLs.Hub (
crates/aisix-gateway/src/hub.rs)Providerregistry. Hub now has onlyspecialized_bridges(open string vendor) +family_bridges(closed 5-valueAdapter).dispatch_two_tieris the only dispatch entry.Dispatch (
crates/aisix-proxy/src/dispatch.rs)resolve_bridge(hub, pk, model_provider): legacy fallback gone. Includes a one-cycle compat shim for pre-Phase-A PK rows (emptyprovider+adapter: None) falling back tohub.get_specialized(Model.provider). Emitstracing::warn!(target: \"aisix_proxy::dispatch\", ...)so operators can detect un-migrated rows.require_provider:Provider→&str.resolve_base_url(pk) -> Result: errors loud when api_base is empty; cp-api must populate.Bridge safety guards (
crates/aisix-provider-openai/src/bridge.rs,crates/aisix-provider-anthropic/src/bridge.rs)OpenAiBridge::resolve_baseandAnthropicBridge::resolve_basenow returnResult<String, BridgeError>and refuse to fall back toOPENAI_DEFAULT_BASE/ANTHROPIC_DEFAULT_BASEwhen the family bridge serves a non-openai / non-anthropic vendor with emptyapi_base. Vendor string normalized (trim()+to_ascii_lowercase()) before comparing. Closes the credential-leak primitive surfaced in the round-1 audit.Proxy handlers
ProviderKeythroughHub::dispatch_two_tier.Provider::Xxx.provider_label:format!(\"{provider:?}\").to_lowercase()→provider.to_ascii_lowercase()(5 sites;Debugon&strwas producing quoted strings).build_hub()(crates/aisix-server/src/main.rs)Adaptervariants).Schema regen:
schemas/resources/model.schema.jsonre-emitted.Net effect
Any new long-tail vendor cp-api admits (xai, openrouter, wafer.ai, or one we haven't heard of yet) routes through the
Adapter::Openaifamily bridge with no DP code change.Test plan
cargo test --workspace— 1095+ tests, 0 failedcargo fmt --all -- --check— cleancargo clippy --workspace --all-targets -- -D warnings— cleanaisix:phase-a-clean-cut) + rebuilt cp-api (aisix-e2e-apifrom current main with /v1/messages returns OpenAI-shape error envelope; Anthropic SDKs expect {type:'error', error:{type, message}} #336 admission gate).dp-catalog-non-featured-routing-live.spec.ts: 1 passed, 8.7s. Full chain — tenant signup → environment → gateway cert → startDP → POST xai PK (201) → POST Model (201) → POST ApiKey (201) → poll DP/v1/models→ POST/v1/chat/completions(200) → OpenAI envelope shape + upstream model echo verified. Spec lives in api7/AISIX-Cloud#429.Audit response
Two independent audit passes (CLAUDE.md §8).
Round 1:
format!(\"{provider:?}\")emitted quoted metric labels): addressed — 5 call sites converted to.to_ascii_lowercase().From<Provider> for Adapter): addressed — impl + test deleted.Round 2:
wafer.ai, real models.dev catalog id): addressed — pattern broadened to allow., positive tests for wafer.ai / fireworks-ai / togetherai added.adapter:None): addressed — rewritten withadapter:Some(Openai)so a future PR that dropsAdapter::Openaifamily fires the test.tracing::warn!emitted whenever the shim fires.Follow-up issues filed during this PR
Config→ 400 for customer-fixable cases)