diff --git a/CLAUDE.md b/CLAUDE.md index 8931149b..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.** @@ -125,8 +129,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** 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 (`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..3a484f59 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,18 @@ 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}` }, + }); + 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 0f7ab991..d6466d17 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,17 @@ 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}` }, + }); + await res.arrayBuffer(); // release the socket between polls + return res.status === 200; }); }); @@ -142,9 +141,12 @@ 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. 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) {