From 7acb4f22f8078991b6a35f36636604c41791e146 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Thu, 13 Aug 2026 10:38:48 +0800 Subject: [PATCH 1/4] chore(review): land the open review-thread fixes from the model-kind batch - tests/e2e: both new suites adopt the standard readiness gate (caller key seeded last, authenticate GET /v1/models, no catch-all) instead of probing the behavior under test; stale probe/slot comments corrected. - loader.rs: un-fuse the doc comments of merge_partial_compat_fields and warn_partial_compat_deduped (the WARN-dedup paragraph sat on the wrong function). - CLAUDE.md: correct the AGENTS.md path (crates/aisix-proxy/), and state the DP shape mapping (embedding = block on the direct shape) plus the wildcard resolved-row identity rule explicitly. - Cargo.toml comments: attach the right GHSA ids to the 0.101 connector removal; narrow the jsonwebtoken aws_lc_rs note to the version-unification claim. --- CLAUDE.md | 4 ++- Cargo.toml | 2 +- crates/aisix-etcd/src/loader.rs | 12 ++++----- crates/aisix-proxy/Cargo.toml | 5 ++-- .../cases/semantic-member-gates-e2e.test.ts | 23 ++++++++-------- .../src/cases/wildcard-identity-e2e.test.ts | 26 +++++++++---------- 6 files changed, 37 insertions(+), 35 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 8931149b..a72b7b03 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -125,8 +125,10 @@ This repo reads its config from etcd, but users never write etcd directly — th **A Model is one table but five kinds (`direct` / `routing` / `ensemble` / `semantic` / `embedding`, plus wildcard display-name aliases), and every request carries TWO model identities: the caller-addressed entry (may be a virtual parent) and the dispatched target. For direct models they coincide, so a mechanism built and tested against direct models silently never decides the composite case — the most-repeated silent-bug class here (#962, #1087, #1237, #1267, #786).** +The five kinds are the cross-plane taxonomy (cp-admin.yaml `kind`); this repo's `model_one_of` implements four dispatch shapes, with `embedding` carried as the `embedding` block on the direct shape (`models/model.rs`). For a wildcard-served request three names are in play — the caller-minted alias, the wildcard row's `display_name`, and the concrete upstream model — and "caller-addressed entry" means the **resolved row**: gates, buckets, and telemetry key on the row's `display_name`, never on the caller-minted string (#959). + - When you touch a model-keyed mechanism (a limit, a guard, an ACL, a config knob, usage/metric attribution, cache keying), answer in the doc comment: does it key on the **requested** entry, the **dispatched** target, or **both**, and what is the behavior for each of the six shapes. -- The per-target invariant (`aisix-proxy/AGENTS.md`: "a per-model gate binds each target") is written around `resolve_attempt_models` — the routing-group trunk. **Ensemble panel/judge (`ProxyModelCaller::call`, the streaming judge) and semantic targets (`semantic::resolve`) bypass that trunk**, so a gate wired only into the trunk is silently absent there (the 2026-08 audit found member IP allowlist, health consumption, and retries all missing on the semantic path for exactly this reason — #958). A new per-target gate must be wired into the sub-dispatch paths too, or explicitly deferred with a filed issue. Prefer routing every dispatch through one shared chokepoint so the family can't drift. +- The per-target invariant (`crates/aisix-proxy/AGENTS.md`: "a per-model gate binds each target") is written around `resolve_attempt_models` — the routing-group trunk. **Ensemble panel/judge (`ProxyModelCaller::call`, the streaming judge) and semantic targets (`semantic::resolve`) bypass that trunk**, so a gate wired only into the trunk is silently absent there (the 2026-08 audit found member IP allowlist, health consumption, and retries all missing on the semantic path for exactly this reason — #958). A new per-target gate must be wired into the sub-dispatch paths too, or explicitly deferred with a filed issue. Prefer routing every dispatch through one shared chokepoint so the family can't drift. - **Strict writes, lenient loads.** `model_one_of` has two variants: the **strict** schema (declarative resources file, the published `schemas/resources/model.schema.json`, every strict validator consumer) forbids a knob a kind never resolves — accepted-but-unread config is the #962 class; the **lenient** loader keeps the base XOR so stored rows written by an older build still load, with `Model::strip_kind_inapplicable` dropping the dead knob and reporting it as `inapplicable:` through the partial-compat channel. The two lists MUST mirror each other exactly (strict-forbidden ⇔ lenient-stripped) — a field forbidden-but-not-stripped half-honors; stripped-but-not-forbidden vanishes on load while the write path accepts it. A knob is enforced exactly as written or rejected, never half-honored (#963). - **`ensemble` is an experimental surface.** Its known parity gaps — member `allowed_cidrs`/guardrail/cooldown/health consumption, Prometheus token+spend attribution, response caching, parent-level generic knobs — are deliberate TODOs under a single future design pass. Do NOT piecemeal-fix one gap ahead of that pass, and do NOT re-audit them as fresh findings. (The one exception is a marshal-family or shared-chokepoint change where covering ensemble is a one-line parallel edit, e.g. projecting an entry-level field the DP already enforces.) - Adding a NEW kind = sweeping every existing model-keyed mechanism against it (grep the kind predicates in `models/model.rs`; every hit re-answers the questions above). diff --git a/Cargo.toml b/Cargo.toml index 7313b7f5..c879ffac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -166,7 +166,7 @@ flate2 = "1" # `default-https-client` (hyper 1 + rustls 0.23/aws-lc) instead of the # legacy `rustls` feature: on aws-sdk-bedrockruntime the latter drags the # retired hyper 0.14 + rustls 0.21 connector (rustls-webpki 0.101, EOL, -# GHSA-82j2-j2ch-gfr8 et al.) into the build even though every Bedrock +# GHSA-xgp8-3hg3-c2mh / GHSA-965h-392x-2mh5) into the build even though every Bedrock # client is built on `upstream_tls::aws_http_client()` and never uses it. aws-config = { version = "1", default-features = false, features = ["behavior-version-latest", "default-https-client", "rt-tokio"] } aws-sdk-bedrockruntime = { version = "1", default-features = false, features = ["default-https-client", "rt-tokio"] } diff --git a/crates/aisix-etcd/src/loader.rs b/crates/aisix-etcd/src/loader.rs index 7c6a4777..76d5bbf5 100644 --- a/crates/aisix-etcd/src/loader.rs +++ b/crates/aisix-etcd/src/loader.rs @@ -572,12 +572,6 @@ fn normalize_ignored_path(path: &str) -> String { .join(".") } -/// WARN once per (kind, field-set) for the process lifetime. Resyncs -/// rebuild the whole snapshot on a cadence; without dedup every cycle -/// would re-log every YELLOW row. The set is capped: past the cap new -/// combinations keep logging (never silently dropped) but are no longer -/// remembered, so a pathological fleet re-logs on each resync instead -/// of growing memory without bound. /// Add `fields` to the partial-compat row already recorded for `key` /// this build, or start one if none exists. Exactly one row per etcd key /// so the supervisor's key-addressed retained report never drops a half @@ -599,6 +593,12 @@ fn merge_partial_compat_fields(stats: &mut BuildStats, key: &str, kind: &str, fi } } +/// WARN once per (kind, field-set) for the process lifetime. Resyncs +/// rebuild the whole snapshot on a cadence; without dedup every cycle +/// would re-log every YELLOW row. The set is capped: past the cap new +/// combinations keep logging (never silently dropped) but are no longer +/// remembered, so a pathological fleet re-logs on each resync instead +/// of growing memory without bound. fn warn_partial_compat_deduped(key: &str, kind: &str, fields: &[String]) { use std::collections::HashSet; use std::sync::{Mutex, OnceLock}; diff --git a/crates/aisix-proxy/Cargo.toml b/crates/aisix-proxy/Cargo.toml index de1e4c63..346cdc25 100644 --- a/crates/aisix-proxy/Cargo.toml +++ b/crates/aisix-proxy/Cargo.toml @@ -37,8 +37,9 @@ axum.workspace = true reqwest.workspace = true # Inbound OIDC/JWT verification on the proxy auth path (jwt.rs). Same # major as the vertex provider's service-account signer. `aws_lc_rs` -# reuses the aws-lc backend already installed as the process-wide rustls -# provider, so no second crypto stack enters the tree. +# resolves to the same aws-lc-rs version already installed as the +# process-wide rustls provider — version-unified, not a parallel +# crypto backend. jsonwebtoken = { version = "10", features = ["aws_lc_rs"] } tower.workspace = true tower-http.workspace = true diff --git a/tests/e2e/src/cases/semantic-member-gates-e2e.test.ts b/tests/e2e/src/cases/semantic-member-gates-e2e.test.ts index 7fffd1af..06b526dc 100644 --- a/tests/e2e/src/cases/semantic-member-gates-e2e.test.ts +++ b/tests/e2e/src/cases/semantic-member-gates-e2e.test.ts @@ -206,7 +206,6 @@ describe("semantic router member gates e2e", () => { app = await spawnApp(); seed = new SeedClient(etcd, app.etcdPrefix); - await seed.createApiKey({ key_hash: CALLER_KEY_HASH, allowed_models: ["*"] }); const embed = await startEmbeddingMock(); const slowEmbed = await startEmbeddingMock({ delayMs: 3000 }); @@ -271,8 +270,8 @@ describe("semantic router member gates e2e", () => { ); // Background-unhealthy displacement: this member's upstream always // 500s, request-path cooldown is DISABLED (so only the background - // prober's Unhealthy verdict can displace it), and the 1s probe - // interval marks it within a few seconds. + // prober's Unhealthy verdict can displace it), and the 5s probe + // interval marks it within the test's 30s poll budget. await directModel("smg-unhealthy", await chatUpstream("unused-500", { status: 500 }), { cooldown: { enabled: false }, background_model_check: { @@ -320,15 +319,17 @@ describe("semantic router member gates e2e", () => { }, }); - // Readiness: an unmatched prompt on the IP router falls through to - // the open default → 200 once everything propagated. + // The caller key is seeded LAST: once it authenticates, revision + // order implies every resource above is in the snapshot + // (tests/e2e/AGENTS.md). The gate exercises none of the member-gate + // behavior under test, so a defect there fails its own case by name + // instead of surfacing as a propagation timeout here. + await seed.createApiKey({ key_hash: CALLER_KEY_HASH, allowed_models: ["*"] }); await waitConfigPropagation(async () => { - try { - const r = await chat("smg-router-ip", "hello there"); - return r.status === 200 && r.content === "served-open"; - } catch { - return false; - } + const res = await fetch(`${app!.proxyUrl}/v1/models`, { + headers: { authorization: `Bearer ${CALLER_PLAINTEXT}` }, + }); + return res.status === 200; }); }); diff --git a/tests/e2e/src/cases/wildcard-identity-e2e.test.ts b/tests/e2e/src/cases/wildcard-identity-e2e.test.ts index 0f7ab991..e60853de 100644 --- a/tests/e2e/src/cases/wildcard-identity-e2e.test.ts +++ b/tests/e2e/src/cases/wildcard-identity-e2e.test.ts @@ -69,7 +69,6 @@ describe("wildcard alias identity e2e", () => { app = await spawnApp(); seed = new SeedClient(etcd, app.etcdPrefix); - await seed.createApiKey({ key_hash: CALLER_KEY_HASH, allowed_models: ["*"] }); const upstream = await startOpenAiUpstream({ nonStreamBody: chatBody("served-wid") }); upstreams.push(upstream); @@ -116,17 +115,16 @@ describe("wildcard alias identity e2e", () => { provider_key_id: pk2.id, }); - // Readiness via a wildcard-served alias: any suffix must resolve. - // listModels hides wildcard patterns, so probe with a chat call — - // 404 until the row propagates. The probe consumes the shared rpm - // slot, so tests below re-align on a fresh window first. + // The caller key is seeded LAST: once it authenticates, revision + // order implies both wildcard rows above are in the snapshot + // (tests/e2e/AGENTS.md). The gate neither resolves a wildcard alias + // nor consumes the shared rpm bucket under test. + await seed.createApiKey({ key_hash: CALLER_KEY_HASH, allowed_models: ["*"] }); await waitConfigPropagation(async () => { - try { - const r = await chat("wid/readiness-probe"); - return r.status === 200 || r.status === 429; - } catch { - return false; - } + const res = await fetch(`${app!.proxyUrl}/v1/models`, { + headers: { authorization: `Bearer ${CALLER_PLAINTEXT}` }, + }); + return res.status === 200; }); }); @@ -142,9 +140,9 @@ describe("wildcard alias identity e2e", () => { } await awaitWindowHeadroom(5); - // The readiness probe already consumed a slot of the SHARED bucket - // (itself evidence of the fix), so align by burning `wid/alpha` - // until a fresh window admits it — that 200 is alias #1's slot. + // Align on a window that admits `wid/alpha` — that 200 is alias + // #1's slot in the SHARED bucket (rpm=1, fixed windows keyed on + // unix time, so the first attempt may land in a spent window). const deadline = Date.now() + 90_000; let aligned = false; while (Date.now() < deadline) { From ba46e502ea3aaeed029f49f959aa5600778267c7 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Thu, 13 Aug 2026 10:56:50 +0800 Subject: [PATCH 2/4] chore(review): scope the wildcard row-keying rule to the gate/metric family; release gate sockets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CLAUDE.md: the row-keying statement now names inline buckets, metric labels and health keys only — usage-event requested_model and model_name policy conditions intentionally keep the caller-supplied string. - e2e gates: consume the /v1/models response body between polls. --- CLAUDE.md | 2 +- tests/e2e/src/cases/semantic-member-gates-e2e.test.ts | 1 + tests/e2e/src/cases/wildcard-identity-e2e.test.ts | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index a72b7b03..cadaecd6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -125,7 +125,7 @@ This repo reads its config from etcd, but users never write etcd directly — th **A Model is one table but five kinds (`direct` / `routing` / `ensemble` / `semantic` / `embedding`, plus wildcard display-name aliases), and every request carries TWO model identities: the caller-addressed entry (may be a virtual parent) and the dispatched target. For direct models they coincide, so a mechanism built and tested against direct models silently never decides the composite case — the most-repeated silent-bug class here (#962, #1087, #1237, #1267, #786).** -The five kinds are the cross-plane taxonomy (cp-admin.yaml `kind`); this repo's `model_one_of` implements four dispatch shapes, with `embedding` carried as the `embedding` block on the direct shape (`models/model.rs`). For a wildcard-served request three names are in play — the caller-minted alias, the wildcard row's `display_name`, and the concrete upstream model — and "caller-addressed entry" means the **resolved row**: gates, buckets, and telemetry key on the row's `display_name`, never on the caller-minted string (#959). +The five kinds are the cross-plane taxonomy (cp-admin.yaml `kind`); this repo's `model_one_of` implements four dispatch shapes, with `embedding` carried as the `embedding` block on the direct shape (`models/model.rs`). For a wildcard-served request three names are in play — the caller-minted alias, the wildcard row's `display_name`, and the concrete upstream model — and "caller-addressed entry" means the **resolved row** for the gate/metric family: inline rate-limit buckets, Prometheus metric labels, and health keys use the row's `display_name`, not the caller-minted string (#959). Usage-event attribution (`requested_model`) and `model_name` policy conditions intentionally keep the caller-supplied name. - When you touch a model-keyed mechanism (a limit, a guard, an ACL, a config knob, usage/metric attribution, cache keying), answer in the doc comment: does it key on the **requested** entry, the **dispatched** target, or **both**, and what is the behavior for each of the six shapes. - The per-target invariant (`crates/aisix-proxy/AGENTS.md`: "a per-model gate binds each target") is written around `resolve_attempt_models` — the routing-group trunk. **Ensemble panel/judge (`ProxyModelCaller::call`, the streaming judge) and semantic targets (`semantic::resolve`) bypass that trunk**, so a gate wired only into the trunk is silently absent there (the 2026-08 audit found member IP allowlist, health consumption, and retries all missing on the semantic path for exactly this reason — #958). A new per-target gate must be wired into the sub-dispatch paths too, or explicitly deferred with a filed issue. Prefer routing every dispatch through one shared chokepoint so the family can't drift. diff --git a/tests/e2e/src/cases/semantic-member-gates-e2e.test.ts b/tests/e2e/src/cases/semantic-member-gates-e2e.test.ts index 06b526dc..3a484f59 100644 --- a/tests/e2e/src/cases/semantic-member-gates-e2e.test.ts +++ b/tests/e2e/src/cases/semantic-member-gates-e2e.test.ts @@ -329,6 +329,7 @@ describe("semantic router member gates e2e", () => { const res = await fetch(`${app!.proxyUrl}/v1/models`, { headers: { authorization: `Bearer ${CALLER_PLAINTEXT}` }, }); + await res.arrayBuffer(); // release the socket between polls return res.status === 200; }); }); diff --git a/tests/e2e/src/cases/wildcard-identity-e2e.test.ts b/tests/e2e/src/cases/wildcard-identity-e2e.test.ts index e60853de..b3ed7102 100644 --- a/tests/e2e/src/cases/wildcard-identity-e2e.test.ts +++ b/tests/e2e/src/cases/wildcard-identity-e2e.test.ts @@ -124,6 +124,7 @@ describe("wildcard alias identity e2e", () => { const res = await fetch(`${app!.proxyUrl}/v1/models`, { headers: { authorization: `Bearer ${CALLER_PLAINTEXT}` }, }); + await res.arrayBuffer(); // release the socket between polls return res.status === 200; }); }); From 86ec3d0ee44a33f396c087e2a499a7e723c5eff9 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Thu, 13 Aug 2026 11:09:02 +0800 Subject: [PATCH 3/4] =?UTF-8?q?rules:=20PR=20batching=20=E2=80=94=20one=20?= =?UTF-8?q?PR=20per=20session=20by=20default=20(per-PR=20CodeRabbit=20bill?= =?UTF-8?q?ing/throttling)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CLAUDE.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index cadaecd6..a38166a8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -88,6 +88,10 @@ After every `gh pr create` or force-push, spawn a fresh `general-purpose` Agent Output HIGH/MEDIUM/LOW per finding with **concrete suggested code**, not vague "consider". **Merge gate:** every HIGH and MEDIUM is either fixed in code or explicitly justified in the PR (e.g. "feature gap, filed as #N, agreed not to block"); silent merge is not enough. For findings that surface gateway/product-behavior gaps, file separate issues and link them. Self-review misses the author's blind spots — an independent agent catches them. +## PR Batching — One PR per Session by Default + +This repo is developed end-to-end by agents — no human reviewer needs small review units — and CodeRabbit bills and rate-limits **per PR**. Fanning one effort into many small PRs burns review quota and stalls the session on throttled bot reviews. Keep ONE open PR per session and push follow-up and related work to it as additional commits (rule and doc riders included) instead of opening another. Split only when a fix must merge independently ahead of the batch, or when the user asks for separate delivery. + ## Handler Families Stay in Lockstep — Fix the Whole Class **The client-facing endpoint handlers come in families that share dispatch, auth, routing, telemetry, and guardrail logic — `/v1/chat/completions`, `/v1/messages` (+`count_tokens`), `/v1/responses`, plus embeddings/rerank/audio/images and the jobs surface (files/batches/fine-tuning). A bug or feature landed on one almost always applies to the others, and a gap on the unfixed siblings is SILENT: nothing errors, the behavior just quietly degrades.** From a8f1518d0b63bea6c307ec0173b9bdcc86161e62 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Thu, 13 Aug 2026 11:29:55 +0800 Subject: [PATCH 4/4] test(e2e): the alignment-loop comment no longer claims a pre-consumed window --- tests/e2e/src/cases/wildcard-identity-e2e.test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/e2e/src/cases/wildcard-identity-e2e.test.ts b/tests/e2e/src/cases/wildcard-identity-e2e.test.ts index b3ed7102..d6466d17 100644 --- a/tests/e2e/src/cases/wildcard-identity-e2e.test.ts +++ b/tests/e2e/src/cases/wildcard-identity-e2e.test.ts @@ -142,8 +142,11 @@ describe("wildcard alias identity e2e", () => { await awaitWindowHeadroom(5); // Align on a window that admits `wid/alpha` — that 200 is alias - // #1's slot in the SHARED bucket (rpm=1, fixed windows keyed on - // unix time, so the first attempt may land in a spent window). + // #1's slot in the SHARED bucket. Nothing has consumed the bucket + // yet (the readiness gate sends no chat traffic), so the first + // attempt normally succeeds; the loop stays as cheap insurance + // should an earlier consumer ever be added (rpm=1, fixed windows + // keyed on unix time). const deadline = Date.now() + 90_000; let aligned = false; while (Date.now() < deadline) {