Skip to content

Runtime spec conformance: scope-filter matcher parity + TENANT_CLOSED Rule 2 guard (v0.1.25.47)#231

Merged
amavashev merged 8 commits into
mainfrom
fix/runtime-spec-conformance-tenant-closed-and-matcher
Jul 10, 2026
Merged

Runtime spec conformance: scope-filter matcher parity + TENANT_CLOSED Rule 2 guard (v0.1.25.47)#231
amavashev merged 8 commits into
mainfrom
fix/runtime-spec-conformance-tenant-closed-and-matcher

Conversation

@amavashev

@amavashev amavashev commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Two runtime spec-conformance changes, one commit each. Version: 0.1.25.47.

1. Webhook scope-filter matcher parity with the admin plane

What: Ports the two spec-conformance refinements from the admin matcher (runcycles/cycles-server-admin#206, merged) into the runtime dispatch matcher EventEmitterRepository.matchesScope, making the two planes byte-identical:

  1. Blank-scope-as-unscoped — a blank/whitespace-only event scope is treated like null: excluded from every scope-filtered subscription. Previously the bare * filter (empty-prefix startsWith) matched a blank "" scope.
  2. Non-empty child segment for trailing-*tenant:a/* now requires scope.length() > prefix.length(): the degenerate empty-child scope tenant:a/ no longer matches. Spec text (admin OpenAPI scope_filter): "tenant:acme-corp/* matches all scopes under acme-corp" — children only.

Why: without parity, a subscription could receive an event from live runtime dispatch that admin-plane dispatch/replay would skip (or vice versa) for the same (filter, scope) pair.

Semantics table (both planes now identical):

filter scope match?
null / blank anything (incl. null/blank) yes
* any non-blank scope yes
* null / "" / " " no
tenant:a/* tenant:a/workspace:b, tenant:a/workspace:b/agent:c yes
tenant:a/* tenant:a (base), tenant:a/ (empty child), tenant:aX, null, "" no
tenant:a/workspace:b (no *) exactly tenant:a/workspace:b yes (exact only, case-sensitive)
tenant:*/ws:b (mid-string *) literal match only * is literal unless trailing

The admin matcher's full test table is ported into EventEmitterRepositoryTest, pinning both planes to the same (filter, scope, expected) cases, plus one dispatch-level test through emit().

2. TENANT_CLOSED Rule 2 guard on reservation mutations

Spec authority: cycles-governance-admin-v0.1.25.yaml, CASCADE SEMANTICS Rule 2 (terminal-owner mutation guard):

"Every mutating admin-plane operation whose target resource has an owning tenant ... MUST reject with HTTP 409 and error.error_code=TENANT_CLOSED when that owning tenant's status is CLOSED. ... Operations scoped in this guard include (non-exhaustive): ... any reservation create/commit/release/extend ..."

and Mode B invariant (a):

"Rule 2's mutation guard takes effect at or before the moment the CLOSED flip is durable to readers. A concurrent mutation on a child observed AFTER the flip MUST NOT succeed, even if the cascade has not yet touched that child."

Runtime half: spec revision v0.1.25.13 (runcycles/cycles-protocol#125) adds TENANT_CLOSED to the runtime ErrorCode enum and the ERROR SEMANTICS closed-tenant binding for exactly these four operations.

The race window, layer by layer. The tenant-key auth filter (ApiKeyRepository.validate) already reads tenant:<id> fresh per request and 401s tenant keys of SUSPENDED/CLOSED tenants — for tenant-key HTTP traffic the post-flip window was already shut (with 401; the spec revision's "a closed tenant usually surfaces on this plane as 401"). Two real gaps remained, both closed here:

  1. Admin-key mutations — the runtime's admin-on-behalf-of auth (X-Admin-API-Key; allowlist GET list/single + POST release) has no tenant-status check, so an admin release on a closed tenant previously SUCCEEDED, mutating budgets post-flip. Now 409 TENANT_CLOSED.
  2. The auth-check→script race — the filter reads tenant status at auth time; the CLOSED flip can land between that read and the Lua execution. Only an in-script check is atomic with the mutation (Mode B invariant (a) strictly).

How:

  • Guard runs inside reserve/commit/release/extend.lua — same pattern as the existing BUDGET_FROZEN/BUDGET_CLOSED in-script guards, atomic with the budget mutations (no partial success possible) and immune to the 60s tenant-config Caffeine cache. Tenant status read: in-script GET tenant:<id> (the admin plane's own key: JSON with a status field) + cjson.decode — one extra Redis command inside the already-running script per mutation, zero extra network round-trips; no key-layout change.
  • Owning tenant: reserve.lua uses the auth-derived tenant already in ARGV; commit/release/extend read the reservation hash's tenant field (authoritative owner) via their existing HMGETs.
  • No tenant record ⇒ no restriction (runtime-only deployments without a governance plane) — same contract as the admin TerminalOwnerMutationGuard, and the spec revision's "not applicable" carve-out. A present-but-malformed tenant record (undecodable JSON, non-object, missing status) fails closed: 500 INTERNAL_ERROR, no mutation — matching the admin TenantRepository, which propagates parse failures (codex round 2).
  • Precedence (codex round 2): same-key idempotent replays return their original response (Rule 2(b) idempotency); any other mutation on a closed tenant is TENANT_CLOSED even when the reservation is already finalized/expired — the closed-tenant rejection takes precedence over RESERVATION_FINALIZED/RESERVATION_EXPIRED (per the precedence sentence added to chore(deps): bump org.springframework.boot:spring-boot-starter-parent from 3.5.13 to 4.0.5 in /cycles-protocol-service #125's ERROR SEMANTICS). Open-tenant error responses are byte-identical to before.
  • Unaffected: GET/list on closed tenants (spec: "Non-mutating operations (GET endpoints) MUST remain available" — served via admin keys; tenant keys keep the existing auth-layer 401), dry_run + /v1/decide do not 409 — per chore(deps): bump org.springframework.boot:spring-boot-starter-parent from 3.5.13 to 4.0.5 in /cycles-protocol-service #125's amended wording (review round 4), a FRESH non-persisting evaluation on a CLOSED tenant returns 200 decision=DENY with reason_code=TENANT_CLOSED (new typed Enums.ReasonCode value) via a shared gate in the dry_run/decide machinery, so a post-flip dry_run can never stamp a signed ALLOW attestation; malformed records fail those evaluations closed (500, before evidence stamping), SUSPENDED tenants (mutation guard is CLOSED-only; pre-existing auth-layer 401 untouched), idempotent replays of pre-flip mutations (replay-first, mirroring the budget status guards).
  • Error shape: standard ErrorResponse (error/message/request_id/trace_id) via the existing CyclesProtocolExceptionGlobalExceptionHandler path.

Wire compatibility: Enums.ErrorCode gains TENANT_CLOSEDadditive; mirrors the pre-existing governance-plane code (runtime spec revision v0.1.25.13, runcycles/cycles-protocol#125 — cited in CHANGELOG/AUDIT). New 409s appear only for tenants a governance plane has closed; the previously-succeeding call it replaces was an admin release that spec-conformant servers must reject anyway.

Tests

  • TenantClosedGuardIntegrationTest (Testcontainers Redis, real Lua): all four ops at the repository layer (a repository call is exactly "a request already past auth" — the filter→script race) with no-partial-mutation assertions (budget reserved/remaining/spent, reservation state, expires_at); HTTP: admin release on closed tenant → 409 TENANT_CLOSED full-envelope (previously 200), tenant-key mutation → 401 pinned (pre-existing), admin GET+list → 200 (Rule 2 read access); record-absent / ACTIVE pass-through; SUSPENDED (repo-level ops proceed, auth-layer 401 pinned); cross-tenant isolation; replay-across-the-flip. The 409 HTTP calls use a non-contract-validating client until spec v0.1.25.13 merges (the shared validating client checks response enums against cycles-protocol@main); shape asserted explicitly, with a comment to move them to the validating client once the spec lands.
  • Matcher: full ported table (18 cases across 15 test methods) + dispatch-level blank-scope test through emit().
  • Unit: handleScriptError token mapping, tenantClosed factory, GlobalExceptionHandler 409 envelope + no-evidence pin (TENANT_CLOSED deliberately not in EVIDENCE_DENIAL_CODES until spec v0.1.25.13 lands).
  • Full mvn verify -Pintegration-tests green locally (round 5: 1,058 tests across the three modules — 31 model + 496 data + 531 api; benchmarks excluded as in CI); JaCoCo ≥95% line gates pass on all modules. OpenApiContractDiffTest (structural diff vs spec@main) passes with the added enum value.

Open questions

Related PRs

….25.47)

The admin server brought its webhook scope_filter matcher to spec
semantics (cycles-server-admin PR #206). The runtime dispatch matcher
(EventEmitterRepository.matchesScope) already had the trailing-*/exact
split but lacked two refinements, so the two planes could disagree on
the same (filter, scope) pair - an event delivered live by the runtime
could be skipped by admin-plane replay, or vice versa.

Ported the admin matcher 1:1 so both planes are byte-identical:

1. Blank-scope-as-unscoped: a blank/whitespace-only event scope is
   treated like null - excluded from every scope-filtered subscription.
   Previously the bare "*" filter (empty-prefix startsWith) matched a
   blank "" scope.

2. Non-empty child segment for trailing-* filters: "tenant:a/*" now
   requires scope.length() > prefix.length() - the degenerate
   empty-child scope "tenant:a/" no longer matches. Spec text is
   "all scopes UNDER acme-corp" (children only).

Unchanged: null/blank filter matches everything including unscoped
events; no trailing "*" = exact, case-sensitive match; non-trailing
"*" is a literal character.

The matcher is now public static (mirroring the admin's) and the
admin's full matcher test table is ported into
EventEmitterRepositoryTest - null/blank filters, bare "*", trailing-*
child/base/sibling/empty-segment cases, exact match, literal
mid-string "*", case sensitivity, blank-scope edges - pinning both
planes to the same (filter, scope, expected) table. One
dispatch-level test pins the blank-scope refinement end-to-end
through emit().

No wire, Redis, Lua, event, or evidence schema change; delivery
selection shifts only on the two edge cases above.

Version: 0.1.25.47. CHANGELOG.md and AUDIT.md updated.
… (v0.1.25.47)

Adopts the governance spec's CASCADE SEMANTICS Rule 2 (terminal-owner
mutation guard, cycles-governance-admin-v0.1.25.yaml) on the runtime
plane: once the owning tenant's status=CLOSED flip is durable in the
shared Redis, the four reservation mutations - create
(POST /v1/reservations), commit, release, extend - are rejected with
HTTP 409, error=TENANT_CLOSED (standard ErrorResponse envelope). Per
Mode B invariant (a): "a concurrent mutation on a child observed AFTER
the flip MUST NOT succeed, even if the cascade has not yet touched
that child."

What this actually closes, layer by layer: the tenant-key auth filter
(ApiKeyRepository.validate) already reads tenant:<id> fresh per
request and 401s tenant keys of SUSPENDED/CLOSED tenants - for
tenant-key HTTP traffic the post-flip window was already shut (with
401; the runtime spec revision (runcycles/cycles-protocol#125)'s "usually surfaces as 401").
Two real gaps remained, both closed here:

1. Admin-key mutations: the runtime's admin-on-behalf-of auth
   (X-Admin-API-Key; allowlist GET list/single + POST release) has no
   tenant-status check, so an admin release on a closed tenant
   SUCCEEDED, mutating budgets post-flip. Now 409 TENANT_CLOSED.
2. The auth-check->script race: the filter reads tenant status at
   auth time; the CLOSED flip can land between that read and the Lua
   execution. Only an in-script check is atomic with the mutation.

Implementation:

- ErrorCode.TENANT_CLOSED added to Enums (additive; mirrors the
  pre-existing governance code; runtime spec revision v0.1.25.13 adds
  it to the runtime ErrorCode enum - runcycles/cycles-protocol#125).
- Guard lives INSIDE reserve/commit/release/extend.lua, following the
  codebase's precedent for status guards (BUDGET_FROZEN/BUDGET_CLOSED
  in reserve.lua). In-script placement is atomic with the budget
  mutations (Redis executes scripts serially), so a post-flip request
  can never partially succeed; it also bypasses getTenantConfig()'s
  60s Caffeine cache, which would violate invariant (a). Cost: one
  extra in-script GET tenant:<id> + cjson.decode per mutation - no
  extra network round-trip.
- Owning tenant: reserve.lua uses the auth-derived tenant already in
  ARGV[10]; commit/release/extend read the reservation hash's tenant
  field (authoritative owner, written by reserve.lua since inception)
  via their existing HMGETs.
- No tenant record in Redis => guard does NOT fire (runtime-only
  deployments without a governance plane; same contract as the admin
  plane's TerminalOwnerMutationGuard).
- Precedence: after idempotent-replay handling (a replay re-observes a
  pre-flip mutation), before state/expiry/budget checks ("regardless
  of that child's own current status").
- Unaffected: GET/list on closed tenants (spec: post-mortem reads, via
  admin keys; tenant keys keep the existing auth-layer 401), dry_run
  and /v1/decide (non-mutating), SUSPENDED tenants (mutation guard is
  CLOSED-only; pre-existing auth-layer 401 untouched), idempotent
  replays.
- handleScriptError maps the new script token; new
  CyclesProtocolException.tenantClosed factory (message mirrors the
  admin guard's).

Tests:

- TenantClosedGuardIntegrationTest (Testcontainers Redis, real Lua):
  all four ops exercised at the repository layer (a repository call is
  exactly "a request already past auth" - the filter->script race)
  with no-partial-mutation assertions on budget
  reserved/remaining/spent, reservation state, and expires_at. HTTP:
  admin release on closed tenant -> 409 TENANT_CLOSED with full
  ErrorResponse shape (previously 200 - the reachable hole);
  tenant-key mutation -> 401 pinned (pre-existing auth behavior);
  admin GET + list -> 200 (Rule 2 read access). Plus record-absent /
  ACTIVE pass-through, SUSPENDED (repo-level ops proceed; auth-layer
  401 pinned), cross-tenant isolation, replay-across-the-flip. The
  409 HTTP calls use a non-contract-validating client until runtime
  spec v0.1.25.13 merges (cycles-protocol@main's ErrorCode enum does
  not yet list TENANT_CLOSED); shape asserted explicitly.
- Unit: handleScriptError token mapping (with/without tenant field),
  tenantClosed factory, GlobalExceptionHandler 409 envelope with
  error/message/request_id/trace_id (+ pin that no error evidence is
  emitted - TENANT_CLOSED stays out of EVIDENCE_DENIAL_CODES until
  spec v0.1.25.13 lands).

CHANGELOG.md and AUDIT.md updated (shared v0.1.25.47 release with the
matcher-parity fix).
…CLOSED precedence

Codex external review of PR #231 (REVISE-MINOR): two findings applied,
one noted. Verified against real Lua via Testcontainers.

| # | Finding | Disposition |
|---|---------|-------------|
| 1 | Malformed tenant record fell through OPEN to the budget mutation | APPLY |
| 2 | RESERVATION_FINALIZED preceded TENANT_CLOSED for different-key attempts on finalized reservations | APPLY |
| 3 | OpenApiContractDiffTest tracks cycles-protocol@main (sequencing) | NO ACTION - CI contract job already green on the PR |

1. Fail closed on malformed tenant records. All four guards
   (reserve/commit/release/extend.lua) previously skipped the guard when
   a PRESENT tenant:<id> row failed cjson.decode - a corrupt governance
   record behaved like an open tenant. Now: present row that cannot be
   decoded into an object with a string status => INTERNAL_ERROR (500,
   standard ErrorResponse, no mutation), matching the admin plane's
   TenantRepository which propagates parse failures. Absent key remains
   no-guard. handleScriptError gains an explicit INTERNAL_ERROR case so
   the script's diagnostic message survives (previously rewritten to
   "Script error: INTERNAL_ERROR"; also benefits the pre-existing
   corrupted-reservation INTERNAL_ERROR tokens).

2. TENANT_CLOSED takes precedence over RESERVATION_FINALIZED for
   non-replay mutations (governance Rule 2: "regardless of that child's
   own current status"; precedence sentence added to spec PR
   runcycles/cycles-protocol#125 ERROR SEMANTICS). commit.lua and
   release.lua replay branches narrowed to true same-key replays
   (state==terminal AND stored key == request key); a different-key
   attempt now falls through to the closed-tenant guard, then to the
   reservation-state checks. release.lua's post-guard state check
   widened from == "COMMITTED" to ~= "ACTIVE" so an open-tenant
   different-key release on a RELEASED reservation keeps returning
   RESERVATION_FINALIZED (state field preserved) exactly as before.
   extend.lua already had guard-before-state-checks; pinned by test.
   Ordering is now: same-key replay -> closed-tenant guard (incl.
   fail-closed) -> reservation-state errors -> budget checks/mutation.

Tests (all real-Lua via Testcontainers, TenantClosedGuardIntegrationTest):
- Fail-closed: per op x {malformed JSON, bare string, bare number,
  object missing status} => 500 INTERNAL_ERROR with "Malformed tenant
  record" message and no partial mutation (budget reserved/remaining/
  spent, reservation state, expires_at unchanged).
- Precedence: closed tenant + finalized reservation + different key =>
  409 TENANT_CLOSED on commit, release, and extend; same-key replay
  still returns the original response (isIdempotentReplay); open tenant
  + different key still RESERVATION_FINALIZED (no-regression pin).
- Unit: handleScriptError INTERNAL_ERROR message preservation.

AUDIT.md: "accepted edge" deviation resolved (was: different-key on
finalized returned RESERVATION_FINALIZED before the guard); fail-closed
decision recorded. CHANGELOG.md: fail-closed + precedence bullets.

Build: mvn verify -Pintegration-tests green - 1,038 tests
(31 model + 485 data + 522 api), JaCoCo >=95% line gates met.
@amavashev

Copy link
Copy Markdown
Collaborator Author

Codex review round 2 — applied in a93307e

# Finding Disposition
1 Present-but-malformed tenant:<id> record fell through open to the budget mutation APPLIED — all four Lua guards now fail closed: a present record that can't be decoded into an object with a string status returns 500 INTERNAL_ERROR (standard ErrorResponse, message preserved via a new explicit handleScriptError case) with no mutation. Absent key stays no-guard. Matches the admin TenantRepository's parse-failure propagation.
2 RESERVATION_FINALIZED preceded TENANT_CLOSED for different-key attempts on finalized reservations APPLIED — replay branches in commit.lua/release.lua narrowed to true same-key replays; different-key attempts now fall through to the closed-tenant guard first. release.lua's post-guard state check widened to ~= "ACTIVE" so open-tenant responses are byte-identical to before. extend.lua already had guard-before-state-checks (now pinned). Precedence order: same-key replay → closed-tenant guard (incl. fail-closed) → reservation-state errors → budget checks/mutation — per the precedence sentence added to spec PR runcycles/cycles-protocol#125's ERROR SEMANTICS. The former "accepted edge" deviation in AUDIT.md is resolved, not accepted.
3 OpenApiContractDiffTest tracks cycles-protocol@main (sequencing risk) NO ACTION — the "Integration (with contract validation)" CI job on this PR is already green; the tracking policy is deliberate (see the loader's javadoc). Codex also confirmed the matcher port is byte-identical to admin main and the in-script GET matches the repo's standalone-Redis posture.

New real-Lua tests (Testcontainers, TenantClosedGuardIntegrationTest): fail-closed per op × {malformed JSON, bare string, bare number, object-missing-status} with no-partial-mutation assertions; closed tenant + finalized reservation + different key → 409 TENANT_CLOSED on commit/release/extend; same-key replay still returns the original response; open tenant + different key still RESERVATION_FINALIZED (no-regression pin); handleScriptError message-preservation unit test.

Build: mvn verify -Pintegration-tests green — 1,038 tests (31 model + 485 data + 522 api), JaCoCo ≥95% line gates met on all modules.

…losed

Codex round 2 review of PR #231 left one CORRECTNESS finding: the four
Lua guards required only that `status` be a string and rejected only
the exact "CLOSED", so a tenant record like {"status":"CLOZED"} or
lowercase {"status":"closed"} proceeded as an OPEN tenant.

The status check is now a whitelist in all four guards
(reserve/commit/release/extend.lua), evaluated before any mutation:

  status == "CLOSED"              -> 409 TENANT_CLOSED
  status == "ACTIVE"|"SUSPENDED"  -> proceed
  any other string                -> 500 INTERNAL_ERROR
                                     ("Malformed tenant record" diagnostic,
                                      same treatment as the other malformed
                                      shapes)

Rationale for fail-closed on unknown statuses: the governance spec's
TenantStatus enum is a closed set (ACTIVE|SUSPENDED|CLOSED) and its
cascade revision explicitly avoids introducing new status values as a
wire-compat guarantee - so an unknown status string cannot be a
legitimate future value under the current contract; it is corruption,
and a corrupt governance record must never be treated as an open
tenant.

Tests: the malformed-record matrix in TenantClosedGuardIntegrationTest
(real Lua via Testcontainers) gains {"status":"CLOZED"} and the
case-sensitivity pin {"status":"closed"} - both exercised across all
four ops with the existing no-partial-mutation assertions (budget
reserved/remaining/spent, reservation state, expires_at unchanged).

Build: mvn verify -Pintegration-tests green - 1,038 tests
(31 model + 485 data + 522 api), JaCoCo >=95% line gates met.
AUDIT.md and CHANGELOG.md updated.
@amavashev

Copy link
Copy Markdown
Collaborator Author

Codex review round 3 — applied in d3f3225

Finding (CORRECTNESS, applied): unknown tenant status strings failed open — the guards only required status to be a string and rejected only exact "CLOSED", so {"status":"CLOZED"} or lowercase {"status":"closed"} proceeded as an open tenant.

The status check in all four Lua guards is now a whitelist, evaluated before any mutation:

status value Result
"CLOSED" 409 TENANT_CLOSED
"ACTIVE" / "SUSPENDED" proceed
any other string 500 INTERNAL_ERROR ("Malformed tenant record" diagnostic, same as the other malformed shapes)

Rationale for fail-closed on unknown statuses: the governance TenantStatus enum is a closed set (ACTIVE|SUSPENDED|CLOSED) and the cascade revision explicitly introduces no new status values as a wire-compat guarantee — an unknown status string cannot be a legitimate future value under the current contract; it's corruption, and a corrupt governance record must never be treated as an open tenant.

Tests: the malformed-record matrix (real Lua via Testcontainers) gains {"status":"CLOZED"} and the case-sensitivity pin {"status":"closed"}, exercised across all four ops with the existing no-partial-mutation assertions.

Build: mvn verify -Pintegration-tests green — 1,038 tests (31 model + 485 data + 522 api), JaCoCo ≥95% line gates met on all modules. Everything else from round 2 confirmed clean by codex (replay wording, release widening, error shape, compat) — no other changes.

@amavashev

Copy link
Copy Markdown
Collaborator Author

External review (codex) rounds complete: SHIP after round 3. Round 1: fail-open on malformed tenant records + TENANT_CLOSED/RESERVATION_FINALIZED precedence → fixed round 2. Round 2: unknown status strings fail-open → fixed round 3 (closed-set whitelist: CLOSED → 409; ACTIVE/SUSPENDED → proceed; anything else → 500 before mutation). Round 3 confirmed all four Lua guards consistent, replay precedence intact, test matrix pins the edges. 1,038 tests green, JaCoCo ≥95% on all modules. Spec companion: runcycles/cycles-protocol#125 (codex SHIP, CI green) — suggested merge order: #125 first, then this.

…ide (DENY, not signed ALLOW)

External reviewer finding (P3, legitimate): the dry_run path branches to
createDryRunReservation before any Lua runs, bypassing the tenant guard
entirely - so a closed tenant's dry_run could return decision=ALLOW, and
dry_run outcomes are stamped as SIGNED reserve evidence ("the canonical
signed 'would this be allowed?' attestation"). A signed ALLOW
attestation for a request whose live execution MUST fail with 409
TENANT_CLOSED is wrong. /v1/decide shared the same hole.

Resolution (matching the amended spec PR runcycles/cycles-protocol#125):
the non-persisting evaluations do NOT 409. A FRESH evaluation on a
CLOSED tenant returns 200 decision=DENY with reason_code=TENANT_CLOSED
- non-mutating as always, and the DENY is the truthful signed
attestation.

Implementation (one place, both surfaces):

- New shared gate evaluateTenantStatusGate(jedis, tenant) in the
  "Shared non-persisting idempotent-evaluation machinery (dry_run +
  decide)" section, called from evaluateDryRun (fresh path, after the
  idempotency claim, before any budget read) and from
  evaluateDecisionBudget (top, tenant now threaded in). Reads
  tenant:<id> fresh on the caller's connection - never the 60s
  tenant-config cache.
- Same fail-closed whitelist as the Lua mutation guards: absent record
  -> no gate (runtime-only deployments unchanged); CLOSED -> DENY;
  ACTIVE/SUSPENDED -> proceed; undecodable / non-object / missing or
  non-string status / unknown status string -> 500 INTERNAL_ERROR
  ("Malformed tenant record"), thrown BEFORE evidence stamping - the
  server cannot attest against corrupt governance state. Convention
  followed: evidence is emitted only for decisions actually reached, so
  no reserve/decide evidence row is queued and no error-evidence row is
  written (INTERNAL_ERROR is likewise excluded from
  EVIDENCE_DENIAL_CODES).
- Replay precedence unchanged: a cached pre-close dry_run/decide replay
  returns its original payload; only FRESH evaluations DENY.
- Reason-code representation: reason codes are TYPED in this codebase
  (Enums.ReasonCode, mirrored from the spec's DecisionReasonCode) -
  TENANT_CLOSED added there (additive; spec PR #125 adds it to the
  documented DecisionReasonCode values). ReasonCode Jackson round-trip
  baseline updated to 7 values.

Tests:

- Integration (Testcontainers, real evidence identity configured for
  the class so stamping is observable): fresh dry_run on CLOSED tenant
  -> 200 DENY/TENANT_CLOSED, no reservation_id, balances unmutated,
  cycles_evidence stamped and one evidence:pending record queued; same
  for /v1/decide; replay of a pre-close dry_run -> original ALLOW
  payload while a fresh evaluation DENIES; malformed records (all six
  shapes) -> 500 on both surfaces with ZERO evidence rows; absent
  record -> normal ALLOW via HTTP (validating client); ACTIVE and
  SUSPENDED evaluate unchanged.
- Unit (data module, mocked Redis - also restores the >=95% line gate
  over the new gate code): CLOSED -> DENY on both surfaces; all six
  malformed shapes -> 500 on both surfaces; ACTIVE/SUSPENDED/absent ->
  fall through to normal evaluation (BUDGET_NOT_FOUND without budgets,
  proving no short-circuit).

AUDIT.md: the "dry_run and /v1/decide stay unguarded" scope note is
resolved (superseded by this round). CHANGELOG.md: evaluation-behavior
bullet + ReasonCode compatibility note.

Build: mvn verify -Pintegration-tests green - 1,055 tests
(31 model + 496 data + 528 api), JaCoCo >=95% line gates met.
@amavashev

Copy link
Copy Markdown
Collaborator Author

Review round 4 — applied in 77317ff

Reviewer finding (P3, legitimate): dry_run branched to createDryRunReservation before any Lua ran, bypassing the tenant guard — a closed tenant's dry_run could return decision=ALLOW, and dry_run outcomes are stamped as signed reserve evidence ("the canonical signed 'would this be allowed?' attestation"). A signed ALLOW for a request whose live execution MUST fail with 409 TENANT_CLOSED is wrong. /v1/decide shared the hole.

Resolution (matches the amended spec PR runcycles/cycles-protocol#125): non-persisting evaluations do not 409 — a FRESH evaluation on a CLOSED tenant returns 200 decision=DENY, reason_code=TENANT_CLOSED, non-mutating as always; the DENY is the truthful signed attestation.

Implementation point: one shared gate, evaluateTenantStatusGate, in the "Shared non-persisting idempotent-evaluation machinery (dry_run + decide)" section — called from evaluateDryRun (fresh path, after the idempotency claim, before any budget read) and evaluateDecisionBudget (top; tenant now threaded in). Reads tenant:<id> fresh, never the 60s config cache. Same fail-closed whitelist as the Lua guards: absent → no gate; CLOSED → DENY; ACTIVE/SUSPENDED → proceed; undecodable / non-object / missing-or-non-string status / unknown status string → 500 before evidence stamping (no reserve/decide evidence row queued, no error-evidence row — consistent with the existing convention that evidence is emitted only for decisions actually reached; INTERNAL_ERROR is likewise excluded from EVIDENCE_DENIAL_CODES). Cached pre-close replays keep their original payload.

Reason-code representation: typed — Enums.ReasonCode (the spec's DecisionReasonCode mirror) gains TENANT_CLOSED, additive, citing #125's amended DecisionReasonCode vocabulary. Jackson round-trip baseline updated to 7 values.

Tests: integration (evidence identity configured for the class, so stamping is observable): fresh dry_run/decide on CLOSED tenant → DENY/TENANT_CLOSED with cycles_evidence stamped and exactly one evidence:pending record; pre-close replay → original ALLOW while fresh DENIES; all six malformed shapes → 500 on both surfaces with zero evidence rows; absent record → normal ALLOW over HTTP (validating client); ACTIVE/SUSPENDED unchanged. Unit tests in the data module cover every gate branch (also what restores the coverage gate over the new code).

Build: mvn verify -Pintegration-tests green — 1,055 tests (31 model + 496 data + 528 api), JaCoCo ≥95% line gates met. AUDIT.md's "dry_run//v1/decide stay unguarded" scope note is resolved by this round.

…CODES

The round-2 deferral ("TENANT_CLOSED stays out of EVIDENCE_DENIAL_CODES
until runtime spec revision v0.1.25.13 lands") is stale: spec PR
runcycles/cycles-protocol#125 added TENANT_CLOSED to the evidence
ErrorResponseMirror (cycles-evidence-v0.2.yaml, 0.2.1), and both PRs
merge together.

Rationale: EVIDENCE_DENIAL_CODES' criterion is "decision reached and
denied" - it already contains the governance-state denials
BUDGET_FROZEN and BUDGET_CLOSED. A mutation-surface 409 TENANT_CLOSED
is the direct sibling of BUDGET_CLOSED (owner-level instead of
ledger-level), so excluding it was inconsistent; the signed denial
receipt is exactly what a closed-tenant enforcement event should
produce.

Changes:

- GlobalExceptionHandler: Enums.ErrorCode.TENANT_CLOSED added to
  EVIDENCE_DENIAL_CODES (javadoc gains the governance-denial family
  with the sibling rationale and the ErrorResponseMirror citation).
  EVIDENCE_ENDPOINTS unchanged: decide/create/commit/release emit,
  extend does NOT - TENANT_CLOSED on extend carries no error evidence,
  same as every other code there (kept as-is, pinned by test).
- GlobalExceptionHandlerTest: the stale no-evidence pin is replaced -
  TENANT_CLOSED now emits + stamps error evidence like the other
  denial codes. New tests mirror the BUDGET_* ones: create-route
  emission (endpoint/http_status/response payload, evidence stamped),
  commit + release reservation_id hoisting, extend-route no-emission
  pin. The envelope-shape test stays (routeless, so shape-only).
- TenantClosedGuardIntegrationTest: the admin-release wire test (the
  HTTP-reachable 409 surface) now asserts the 409 body is stamped with
  cycles_evidence and EXACTLY ONE error-evidence record lands on
  evidence:pending, carrying the TENANT_CLOSED code in the mirrored
  response and the hoisted reservation_id. The repo-level extend test
  pins zero error-evidence records. (Create/commit closed-tenant 409s
  are not reachable over HTTP - the tenant-key auth filter 401s first,
  as pinned in earlier rounds - so their emission is pinned at the
  handler layer, the same seam every other denial code uses.)
- AUDIT.md: deferred-exclusion note rewritten as resolved with the
  sibling-consistency rationale. CHANGELOG.md: signed-denial-receipts
  bullet.

Build: mvn verify -Pintegration-tests green - 1,058 tests
(31 model + 496 data + 531 api), JaCoCo >=95% line gates met.
@amavashev

Copy link
Copy Markdown
Collaborator Author

Review round 5 — applied in 08016a1

Finding: the round-2 deferral keeping TENANT_CLOSED out of EVIDENCE_DENIAL_CODES "until runtime spec v0.1.25.13 lands" went stale — spec PR runcycles/cycles-protocol#125 added TENANT_CLOSED to the evidence ErrorResponseMirror (cycles-evidence-v0.2.yaml, 0.2.1), and both PRs merge together.

Rationale: EVIDENCE_DENIAL_CODES' criterion is "decision reached and denied" — it already contains the governance-state denials BUDGET_FROZEN/BUDGET_CLOSED. A mutation-surface 409 TENANT_CLOSED is the direct sibling of BUDGET_CLOSED (owner-level instead of ledger-level), so excluding it was inconsistent; the signed denial receipt is exactly what a closed-tenant enforcement event should produce.

Changes:

  • TENANT_CLOSED added to EVIDENCE_DENIAL_CODES. EVIDENCE_ENDPOINTS untouched — decide/create/commit/release emit; extend does not (not an evidence endpoint, same as every other code there; kept as-is and pinned by test).
  • Handler unit tests flipped/extended, mirroring the BUDGET_* ones: create-route emission (endpoint/http_status/response, cycles_evidence stamped), commit + release reservation_id hoisting for this code, extend-route no-emission pin.
  • Wire proof in TenantClosedGuardIntegrationTest: the admin-release 409 (the HTTP-reachable closed-tenant mutation surface) now asserts the stamped cycles_evidence ref and exactly one error record on evidence:pending, carrying TENANT_CLOSED in the mirrored response plus the hoisted reservation_id; the extend test pins zero error-evidence records. Create/commit closed-tenant 409s aren't reachable over HTTP (tenant-key auth 401s first, pinned in earlier rounds), so their emission is pinned at the handler layer — the same seam every other denial code uses.
  • AUDIT.md deferral note rewritten as resolved with the sibling rationale; CHANGELOG signed-denial-receipts bullet.

Build: mvn verify -Pintegration-tests green — 1,058 tests (31 model + 496 data + 531 api), JaCoCo ≥95% line gates met on all modules.

@amavashev

Copy link
Copy Markdown
Collaborator Author

External review (codex) final confirm after rounds 4–5 (dry-run/decide DENY gate + evidence emission): SHIP. Reviewer P3s resolved: shared tenant-status gate on both non-persisting surfaces (fresh evaluations DENY with reason_code=TENANT_CLOSED; cached pre-close replays keep replay precedence per amended spec wording); TENANT_CLOSED added to EVIDENCE_DENIAL_CODES (sibling-consistency with BUDGET_FROZEN/BUDGET_CLOSED; mirror 0.2.1 declares the code) with emission tests incl. reservation_id hoisting and the extend no-emission pin. 1,058 tests green, JaCoCo ≥95% on all modules. Merge order: cycles-protocol#125 first.

…face

Reviewer catch on the round-5 wording: CHANGELOG's "Signed denial
receipts" bullet said "409 TENANT_CLOSED on the evidence endpoints
(decide, create, commit, release) emits an error CyclesEvidence
envelope" - wrongly grouping /v1/decide with the 409 error-evidence
surface. /decide never 409s for a closed tenant: since round 4 it
returns 200 decision=DENY, reason_code=TENANT_CLOSED and emits its
normal DECIDE decision evidence (dry_run create likewise, with
reserve evidence).

Reworded to separate the two surfaces:
- mutation-surface 409s (persisting create, commit, release) emit
  error evidence, reservation_id hoisted on commit/release;
- /v1/decide and dry_run create return 200 DENY and emit their
  decide/reserve decision evidence;
- extend stays a non-evidence endpoint.

Case-insensitive sweep of CHANGELOG.md, AUDIT.md, and the PR body for
other phrasings grouping decide with the 409/error-evidence surface:
two hits fixed (the CHANGELOG bullet above and AUDIT's "emission
follows the endpoint allowlist: decide/create/commit/release emit"
sentence); zero incorrect survivors remain (the only "decide" mentions
left near evidence/409 wording are the corrections themselves and the
malformed-record note, which correctly says no evidence of any kind is
written for failed evaluations).

Docs-only change; no code or test deltas.
@amavashev

Copy link
Copy Markdown
Collaborator Author

Round 6 (docs-only) — applied in cf1d9b7

Reviewer catch: the round-5 CHANGELOG bullet grouped /v1/decide with the 409 error-evidence surface ("409 TENANT_CLOSED on the evidence endpoints (decide, create, commit, release) emits an error CyclesEvidence envelope"). /decide never 409s for a closed tenant — since round 4 it returns 200 decision=DENY, reason_code=TENANT_CLOSED and emits its normal decide decision evidence (dry_run create likewise, with reserve evidence).

Fix: reworded to separate the surfaces — mutation-surface 409s (persisting create, commit, release) emit error evidence with reservation_id hoisted on commit/release; decide + dry_run return 200 DENY and emit their decision/reserve evidence; extend remains a non-evidence endpoint.

Sweep: case-insensitive pass over CHANGELOG.md, AUDIT.md, and the PR body for any phrasing grouping decide with the 409/error-evidence surface — two hits fixed (the CHANGELOG bullet and AUDIT's endpoint-allowlist sentence), zero incorrect survivors remain. No code/test changes; CI runs as usual.

Reviewer catch: cycles-protocol-service/README.md was stale for the new
surfaces. Applied, matching each section's existing style:

- reason_code list (/v1/decide DENY, ~281): added TENANT_CLOSED with a
  note that it appears on fresh (non-replay) dry_run/decide evaluations
  for closed tenants and that cached pre-close replays keep their
  original payload. Same-class staleness fixed in the same list: it
  said "NOT_FOUND" (the enum value is BUDGET_NOT_FOUND) and omitted
  BUDGET_FROZEN/BUDGET_CLOSED - now matches the spec's KNOWN VALUES
  plus TENANT_CLOSED (spec revision v0.1.25.13).
- Error table (~640): new 409 TENANT_CLOSED row - persisting mutation
  surface (create/commit/release/extend), precedence over
  reservation-state errors for non-replay attempts, error-evidence
  emission on the evidence endpoints (extend excluded), absent-record
  carve-out, and the decide/dry_run 200-DENY contrast.
- Extend endpoint "Error conditions" line (~455): 409 TENANT_CLOSED
  added first (precedence over the reservation-state errors listed).
- /v1/decide overview (~258) and the non-dry 409 note (~368): closed
  tenants included in the surfaced-as-DENY / returns-409 phrasing.
- decide-never-409s note (~648): TENANT_CLOSED named explicitly.
- OPERATIONS.md reason-code tag set: new "Tenant state: TENANT_CLOSED"
  line (the code reaches the domain counters via
  e.getErrorCode().name() on the four mutation paths).

Sweep (zero survivors), patterns used across README.md, OPERATIONS.md,
BENCHMARKS.md, MAINTAINERS.md, cycles-protocol-service/README.md,
benchmarks/README.md, docs/:
- enumerations: "reason_code.*values|possible.*codes|error
  conditions:|- (Budget|Reservation|Request|Tenant)
  (denials|state|issues)" - all hits now carry TENANT_CLOSED where it
  belongs;
- prose: "tenant.{0,20}closed" case-insensitive - remaining hits are
  the corrections themselves;
- LIMIT_EXCEEDED regression check: present in the error table (429
  row, added with v0.1.25.46); correctly absent from OPERATIONS'
  domain-counter set (it is a filter-level 429 that never reaches the
  repository counters, same as UNAUTHORIZED/FORBIDDEN which that
  section already documents as filter-level).

Docs-only; no code or test deltas.
@amavashev

Copy link
Copy Markdown
Collaborator Author

Round 7 (docs-only) — applied in f860d78

Finding: cycles-protocol-service/README.md was stale for the new surfaces — the reason_code list omitted TENANT_CLOSED and the error table had no 409 TENANT_CLOSED row.

Applied (matching each section's existing style):

  • reason_code list (/v1/decide DENY): TENANT_CLOSED added, noting it appears on fresh (non-replay) dry_run/decide evaluations for closed tenants (replays keep their original payload). Same-class staleness fixed in the same list: it said NOT_FOUND (the value is BUDGET_NOT_FOUND) and omitted BUDGET_FROZEN/BUDGET_CLOSED — now matches the spec's KNOWN VALUES + TENANT_CLOSED.
  • Error table: new 409 TENANT_CLOSED row — persisting mutation surface (create/commit/release/extend), precedence over reservation-state errors for non-replay attempts, error-evidence emission on the evidence endpoints (extend excluded), absent-record carve-out, decide/dry_run 200-DENY contrast.
  • Extend "Error conditions" line: 409 TENANT_CLOSED added first (precedence).
  • /v1/decide overview + non-dry 409 note + decide-never-409s note: closed-tenant behavior named explicitly.
  • OPERATIONS.md metrics reason-code set: new "Tenant state: TENANT_CLOSED" line (the code reaches the domain counters via e.getErrorCode().name() on the four mutation paths).

Sweep (zero survivors) across root README.md, OPERATIONS.md, BENCHMARKS.md, MAINTAINERS.md, cycles-protocol-service/README.md, benchmarks/README.md, docs/ — patterns: enumeration regex reason_code.*values|possible.*codes|error conditions:|- (Budget|Reservation|Request|Tenant) (denials|state|issues); prose regex tenant.{0,20}closed (case-insensitive); LIMIT_EXCEEDED regression check. All enumeration hits now carry TENANT_CLOSED where it belongs; remaining prose hits are the corrections themselves. LIMIT_EXCEEDED is present in the error table (429 row, v0.1.25.46) and correctly absent from OPERATIONS' domain-counter set (filter-level 429, never reaches repository counters — same documented treatment as UNAUTHORIZED/FORBIDDEN).

Docs-only; no code/test changes. CI runs as usual.

@amavashev
amavashev merged commit eb6db8b into main Jul 10, 2026
8 checks passed
@amavashev
amavashev deleted the fix/runtime-spec-conformance-tenant-closed-and-matcher branch July 10, 2026 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant