Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,12 @@ jobs:
ETCD_TEST_URL: http://127.0.0.1:2379
steps:
- uses: actions/checkout@v6
with:
# crates/aisix-core/tests/compat_debt.rs resolves "has a release
# shipped since this compat marker's anchor?" from `git tag`. The
# default shallow checkout carries no tags, and that gate FAILS
# rather than skips when it finds none — do not drop this.
fetch-tags: true
Comment thread
coderabbitai[bot] marked this conversation as resolved.
- uses: dtolnay/rust-toolchain@stable
with:
components: llvm-tools-preview
Expand Down
16 changes: 15 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,24 @@ The five kinds are the cross-plane taxonomy (cp-admin.yaml `kind`); this repo's
- 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:<field>` 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).
- **Never make a field of a projected resource required at the TYPE level.** Requiredness belongs in the strict schema (`require_property` in `models/schema.rs`), never in the struct: the loader validates leniently and then deserializes, and a row it cannot deserialize is **skipped entirely** (`aisix-etcd/src/loader.rs`). Skipping is survivable for a resource the request path treats as optional, but an `api_key` row that fails to load stops authenticating **every** kind of traffic, not just the feature whose field changed — a far worse outcome than the field defaulting. So a new non-`Option` field, or one that loses `#[serde(default)]`, silently turns every already-projected row into a dead one. Give it a serde default whose meaning is fail-closed, and add it to `required` in the strict schema so the write path still refuses to guess. The control plane must also re-emit the affected collection once (`ReprojectMcpAclOnce` is the pattern) — the stored shape changed, but nothing else re-projects a row whose *content* did not. (Lesson from #993: `allow` was required at the type level in #992, which made every key still projected as `mcp_access: {"mode": "inherit"}` unloadable.)
- **Never change a projected field's shape or value domain in place — the previous release must keep loading the row.** The supported upgrade order is control plane first, then data planes, with an arbitrarily long window; a released DP is immutable, so whatever the new CP projects must still parse one release back. Unknown *fields* are tolerated by design (serde_ignored → partial-compat report), but a malformed *known* field — a reshape, a lost default, a new enum value — fails the row and the loader skips it whole (the blast radius of the rule above). A reshape therefore ships under a NEW field name (the old one is never reused), or as a **same-name dual-generation document** when the old and new keys don't collide: the CP emits one document valid for both generations, and this side carries a consumed-and-ignored tombstone for the old selector — `McpAccess::legacy_mode` is the template: `#[serde(default, rename = "...", skip_serializing)]` + `#[schemars(skip)]` so the strict write path still rejects it, with a comment naming the retirement condition. A new enum value in an existing field cannot be made safe DP-side at all (lenient parsing keeps enums closed — it row-kills every older DP), so the paired CP PR must gate it behind `dpCompatGate` until the fleet minimum reads it. Whenever new semantics are invisible to the old release, verify the old default direction there: fail-closed or no-op is required; if it is fail-open, the CP must project an old-shape tombstone at the most restrictive value.
- **Never change a projected field's shape or value domain in place — the previous release must keep loading the row.** The supported upgrade order is control plane first, then data planes, with an arbitrarily long window; a released DP is immutable, so whatever the new CP projects must still parse one release back. Unknown *fields* are tolerated by design (serde_ignored → partial-compat report), but a malformed *known* field — a reshape, a lost default, a new enum value — fails the row and the loader skips it whole (the blast radius of the rule above). A reshape therefore ships under a NEW field name (the old one is never reused), or as a **same-name dual-generation document** when the old and new keys don't collide: the CP emits one document valid for both generations, and this side carries a consumed-and-ignored tombstone for the old selector — `McpAccess::legacy_mode` is the template: `#[serde(default, rename = "...", skip_serializing)]` + `#[schemars(skip)]` so the strict write path still rejects it, with a `COMPAT-SINCE:` marker naming the retirement condition (see the next section). A new enum value in an existing field cannot be made safe DP-side at all (lenient parsing keeps enums closed — it row-kills every older DP), so the paired CP PR must gate it behind `dpCompatGate` until the fleet minimum reads it. Whenever new semantics are invisible to the old release, verify the old default direction there: fail-closed or no-op is required; if it is fail-open, the CP must project an old-shape tombstone at the most restrictive value.
- **`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).

## Time-Boxed Compat Code Carries a `COMPAT-SINCE:` Marker

**A shim meant to live for exactly one release gets a machine-readable deadline, not a sentence in a comment. Prose deadlines never come due — both of the ones this repo was carrying were found by grep, not by process.**

Put the marker on the compat code, in whatever comment syntax the file uses:

```text
COMPAT-SINCE: 0.10.0 #1009 — what this tolerates and why it can go
```

Anchor on a release that has **already shipped**, never on a guess at the next version — "due at 0.11.0" fails silently when the next release turns out to be `1.0.0`. The gate rejects an anchor that is not in `git tag`, so a predicted number cannot be written in the first place.

`crates/aisix-core/tests/compat_debt.rs` is the gate. It runs in the required `rust unit + coverage` job on every PR, and fails once a stable tag with a higher `MAJOR.MINOR` than the anchor exists; release candidates never count as shipped, and a patch on the anchor's own line does not come due. When it fires, either remove the code and its marker and close the tracking issue, or re-anchor deliberately to the newest release and say why in the issue. Full rules, including how a `release/X.Y` maintenance line is scoped, live in that file's module docs.

## AISIX Product Terminology

Use the following terms in public prose, generated API descriptions, release
Expand Down
15 changes: 11 additions & 4 deletions crates/aisix-core/src/models/mcp_policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,18 +103,25 @@ pub struct McpAccess {
pub deny: Vec<String>,

/// Compatibility tombstone for the pre-0.10.0 `mode` selector. The
/// control plane projects `"mode": "deny"` alongside the layered
/// control plane projected `"mode": "deny"` alongside the layered
/// shape so a 0.9.x data plane — where `mode` is required and `deny`
/// means "no MCP tool access" — still loads the whole api_key row,
/// means "no MCP tool access" — still loaded the whole api_key row,
/// fail-closed, instead of skipping it (a skipped row stops the key
/// authenticating for EVERY kind of traffic). This generation
/// consumes and ignores the value; the field exists only so the
/// loader does not report the tombstone as partial compat on every
/// row. Any JSON shape is accepted so a malformed tombstone can
/// never kill the row. Hidden from the schemas — the strict write
/// path closes unknown fields, so resource authors cannot set it —
/// and never re-serialized. Retire together with the CP emission
/// once 0.9.x is out of the supported upgrade window.
/// and never re-serialized.
///
/// COMPAT-SINCE: 0.10.0 #1009 — the control plane stopped emitting this
/// tombstone at the 0.10.0 compat floor, but documents written before the
/// run-once reprojection still carry `mode`, so this generation reads and
/// drops it rather than reporting partial compat on every row.
///
/// Retiring it removes the field, the `#[schemars(skip)]` exclusion, and
/// the loader test that pins the tolerance.
#[serde(default, rename = "mode", skip_serializing)]
#[schemars(skip)]
pub legacy_mode: Option<serde_json::Value>,
Expand Down
Loading