From 224a5e7228a604eec7f7a2ef0eec1001745eca2e Mon Sep 17 00:00:00 2001 From: Albert Mavashev Date: Fri, 22 May 2026 07:58:36 -0400 Subject: [PATCH 1/2] feat(listReservations): add expires_*/finalized_* time-window filters (v0.1.25.21) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #162. Implements cycles-protocol-v0.yaml revision 2026-05-22 (runcycles/cycles-protocol#98). Follow-up to v0.1.25.20's from/to window filter — addresses the operational use case that revision intentionally left out: cleanup sweepers that need to locate reservations expiring or already finalized within a window. Four new optional query parameters on GET /v1/reservations: * expires_from / expires_to — inclusive bounds on expires_at_ms. * finalized_from / finalized_to — inclusive bounds on finalized_at_ms. All ISO 8601 date-time. Each pair binds to its target field regardless of sort_by. The three windows (from/to + expires_* + finalized_*) compose with AND semantics. ReservationSummary gains an optional finalized_at_ms field so clients filtering on finalized_* can see the timestamp without a follow-up getReservation call. Optional (@JsonInclude NON_NULL) for strict-schema back-compat. ACTIVE/EXPIRED exclusion under finalized_*: per the spec, rows where finalized_at_ms is absent MUST be excluded when either bound is supplied. The predicate naturally fails on field-absent rows; making the exclusion normative ensures conformant servers agree. Implementation: * Controller: 4 new query params, each validated independently (malformed → 400 with distinct per-param message, from > to → 400 before any repository call, blank strings treated as unset). Mirrors the v0.1.25.20 parseIsoToEpochMs pattern. * Repository: two new predicate helpers (expiresAtInWindow, finalizedAtInWindow). finalizedAtInWindow resolves the timestamp from committed_at OR released_at, matching buildReservationSummary's projection logic. Applied in both legacy SCAN-cursor and sorted paths. * FilterHasher: gains four trailing Long args (10 → 14) with INDEPENDENT gated emission per pair. Preserves byte-exact back-compat for BOTH v0.1.25.18 cursors (no window canonical, golden 2f397ea0e8fb53b7) AND v0.1.25.20 cursors with from/to set (canonical |fr=|to= without expires/finalized, golden ad7204d521cfd133 — newly locked down here). * ReservationSummary model gets the new field; toSummary projection updated. * Java signature change: listReservations 14 → 18 args. No wire change for clients that omit the new params; the new response-body field is optional with NON_NULL serialization. Coverage (557 tests, +19 from v0.1.25.20's 538): * FilterHasherTest +3: expires/finalized distinctness, finalized vs from/to distinctness, v0.1.25.20 8-byte golden lockdown. * RedisReservationQueryTest +6 under ExpiresAndFinalizedWindowFilter nested class: legacy expires_from excludes-below, legacy expires_to excludes-above, finalized excludes ACTIVE rows (no committed_at/released_at), finalized resolves from released_at when committed_at absent, all-three AND composition, cursor mismatch on expires window change. * ReservationControllerTest +10 under ListReservations nested class: 4 malformed-*, 2 reversed-window, expires propagation with verify() lock, finalized propagation with verify() lock, all-three combined with distinct epoch-ms per pair to catch slot mix-ups, blank-as-unset for new windows. JaCoCo 95% bundle gate met. Docs: * AUDIT.md: top date line + new long-form section walking through the rationale, three-pair design, schema addition, cursor back-compat, validation choices, and out-of-scope (time_field-pivoted alternative). * CHANGELOG.md: [0.1.25.21] entry under Keep-a-Changelog format. * README.md (cycles-protocol-service): four new param rows. * pom.xml revision bumped 0.1.25.20 → 0.1.25.21. --- AUDIT.md | 45 ++- CHANGELOG.md | 37 +++ cycles-protocol-service/README.md | 4 + .../api/controller/ReservationController.java | 30 +- .../controller/ReservationControllerTest.java | 192 ++++++++++-- .../RedisReservationRepository.java | 85 +++++- .../data/repository/support/FilterHasher.java | 34 ++- .../repository/RedisReservationQueryTest.java | 278 ++++++++++++++++-- .../repository/support/FilterHasherTest.java | 85 ++++-- .../protocol/model/ReservationSummary.java | 5 + cycles-protocol-service/pom.xml | 2 +- 11 files changed, 720 insertions(+), 77 deletions(-) diff --git a/AUDIT.md b/AUDIT.md index 3a88477..a837a70 100644 --- a/AUDIT.md +++ b/AUDIT.md @@ -1,6 +1,7 @@ # Cycles Protocol v0.1.25 — Server Implementation Audit -**Date:** 2026-05-21 (v0.1.25.20 — `from` / `to` ISO-8601 time-window filter on `GET /v1/reservations` per cycles-protocol revision 2026-05-21; closes runcycles/cycles-server#159. Two new query params on `listReservations`, both `string`/`format: date-time`, both inclusive bounds on `created_at_ms`, both bind to `created_at_ms` regardless of `sort_by`. Implemented in both the legacy SCAN-cursor and sorted paths. `FilterHasher.hash(...)` now folds `fromMs`/`toMs` into the canonical hash so sorted-path cursors invalidate on window change (the legacy Redis-SCAN cursor is not window-validated, matching how it already treats every other filter). Validation: malformed values → 400, `from > to` → 400 before any repository call, blank strings treated as unset, missing/unparseable `created_at` rows defensively excluded when either bound supplied. Pure additive wire change — all v0.1.25.x clients that don't send the params continue to work byte-for-byte. 538 tests pass (375 data + 163 api).), +**Date:** 2026-05-22 (v0.1.25.21 — `expires_from`/`expires_to` and `finalized_from`/`finalized_to` ISO-8601 time-window filters on `GET /v1/reservations` per `cycles-protocol-v0.yaml` revision 2026-05-22 (runcycles/cycles-protocol#98); closes runcycles/cycles-server#162. Four new query params mirroring the v0.1.25.20 `from`/`to` shape: `expires_*` binds to `expires_at_ms` (required field, every row), `finalized_*` binds to `finalized_at_ms` (populated only on COMMITTED/RELEASED; ACTIVE and EXPIRED normatively excluded). Three windows compose with AND. `finalized_at_ms` added as an optional field on `ReservationSummary` so clients filtering with `finalized_*` can see the timestamp without a follow-up `getReservation` — strict-schema-compatible because the field is `@JsonInclude(NON_NULL)`. `FilterHasher` extends with four more `Long` args (10 → 14) using independent gated emission per pair — preserves byte-exact back-compat for v0.1.25.18 cursors (golden `2f397ea0e8fb53b7`) AND v0.1.25.20 cursors with from/to set (golden `ad7204d521cfd133`). `RedisReservationRepository.listReservations` signature 14 → 18 args. Two new predicate helpers (`expiresAtInWindow`, `finalizedAtInWindow`) applied in both legacy SCAN-cursor and sorted paths. Validation: each new pair `from > to` → 400; malformed values → 400 with distinct per-param message; blank strings treated as unset. 557 tests pass (384 data + 173 api), +19 vs v0.1.25.20.), +2026-05-21 (v0.1.25.20 — `from` / `to` ISO-8601 time-window filter on `GET /v1/reservations` per cycles-protocol revision 2026-05-21; closes runcycles/cycles-server#159. Two new query params on `listReservations`, both `string`/`format: date-time`, both inclusive bounds on `created_at_ms`, both bind to `created_at_ms` regardless of `sort_by`. Implemented in both the legacy SCAN-cursor and sorted paths. `FilterHasher.hash(...)` now folds `fromMs`/`toMs` into the canonical hash so sorted-path cursors invalidate on window change (the legacy Redis-SCAN cursor is not window-validated, matching how it already treats every other filter). Validation: malformed values → 400, `from > to` → 400 before any repository call, blank strings treated as unset, missing/unparseable `created_at` rows defensively excluded when either bound supplied. Pure additive wire change — all v0.1.25.x clients that don't send the params continue to work byte-for-byte. 538 tests pass (375 data + 163 api).), 2026-05-21 (v0.1.25.19 — supply-chain CVE patch; re-pin `tomcat.version=10.1.55` in `cycles-protocol-service/pom.xml` to close 7 new CVEs flagged by Trivy against `tomcat-embed-core 10.1.54` (CRITICAL: CVE-2026-43512, CVE-2026-43515, CVE-2026-41293; HIGH: CVE-2026-43513, CVE-2026-42498, CVE-2026-41284; LOW: CVE-2026-43514 — all fixed in 10.1.55 / 11.0.22). Mirrors the v0.1.25.16 pattern; the override was dropped in v0.1.25.18 when SB 3.5.14's BOM caught up to 10.1.54, now re-added one patch higher because Trivy DB updates between 2026-05-11 (last green main run) and 2026-05-21 surfaced a new wave on the same artifact. Removable once Spring Boot ships with 10.1.55+ as its managed version. `commons-lang3.version=3.18.0` retained (CVE-2025-48924 still unfixed in SB 3.5.14's managed 3.17.0). No production code or test changes; all 537 protocol-service tests pass.), 2026-04-26 (v0.1.25.18 — dependency hygiene matching `cycles-server-events` v0.1.25.12: bump `spring-boot-starter-parent` 3.5.13 → 3.5.14 (patch with upstream security hardening — constant-time comparison for remote DevTools secret, `RandomValuePropertySource` SecureRandom, hostname verification applied consistently for Cassandra/RabbitMQ SSL, plus symlink-handling fixes); **drop `10.1.54` override** since Spring Boot 3.5.14's BOM now manages 10.1.54 directly (verified against `spring-boot-dependencies-3.5.14.pom`); commons-lang3 3.18.0 override retained — Spring Boot 3.5.14's BOM still manages 3.17.0. **Jedis 7.4.1 → 6.2.0** to align all three services on the same Redis client major (events at 6.2.0 since v0.1.25.12, admin at 6.2.0 in v0.1.25.41); all call sites use stable APIs (`Jedis`, `JedisPool`, `Pipeline`, `Response`, `ScanParams`, `ScanResult`, `JedisNoScriptException`) — no 7.x-only API usage. No code changes; all 152 tests pass.), 2026-04-19 (v0.1.25.17 — supply-chain CVE fix follow-up; pin `commons-lang3.version=3.18.0` to close CVE-2025-48924 (Trivy HIGH) on the `commons-lang3-3.17.0` jar that ships in the fat-jar image via `swagger-core-jakarta` (OpenAPI UI). Spring Boot 3.5.13's BOM manages commons-lang3 at 3.17.0 — override is removable once Spring Boot ships a managed version of 3.18.0+. All 152 tests pass), @@ -26,6 +27,48 @@ --- +### 2026-05-22 — v0.1.25.21: `expires_*` / `finalized_*` time-window filters on listReservations + +Closes [#162](https://github.com/runcycles/cycles-server/issues/162). Spec landed in `cycles-protocol-v0.yaml` revision 2026-05-22 (PR runcycles/cycles-protocol#98); this is the matching runtime impl. + +**Why the spec change.** The v0.1.25.20 `from`/`to` window covers "what happened in the last 24h" cleanly but binds to `created_at_ms`, which is unhelpful for the operational use case the original issue thread also called out: "cleanup routines on expired or abandoned reservations." A reservation created at T-7d that just expired this morning is invisible to a 24h `created_at` window — exactly what a sweeper is looking for. The new windows give sweepers a direct path: query the expiry timestamp, query the finalization timestamp. + +**Three independent pairs.** Each window pair binds to its target field regardless of `sort_by`, matching the v0.1.25.20 sort-key-independence rule. The three pairs compose with AND semantics; a row must satisfy every supplied predicate. This keeps the contract predictable across the matrix of (window, sort_key) combinations — no per-key filter semantics to memorize. + +**Finalized-row exclusion.** The spec makes the ACTIVE/EXPIRED exclusion normative: when either `finalized_*` bound is supplied, rows missing both `committed_at` and `released_at` MUST be excluded from results. The predicate naturally fails on field-absent rows; making the exclusion normative ensures conformant servers agree. Callers wanting EXPIRED-row windows should use `expires_*` instead (which works on every row since `expires_at_ms` is required). + +**Schema addition: `finalized_at_ms` on `ReservationSummary`.** Pre-revision the field was declared only on `ReservationDetail` (with `additionalProperties: false` on the summary blocking servers from sending it). The filter would have been useful only via a follow-up `getReservation` per row. Adding the field to the summary makes the filter genuinely useful; strict-schema clients remain compatible because the field is optional (`@JsonInclude(NON_NULL)`). Old clients that don't know about the field get pre-revision responses byte-for-byte (the field is absent on ACTIVE rows, which is the common case for unfiltered list calls). + +**Two execution paths.** `RedisReservationRepository.listReservations` retains its v0.1.25.12 dual-path shape. Two new helpers — `expiresAtInWindow(fields, fromMs, toMs)` and `finalizedAtInWindow(fields, fromMs, toMs)` — are applied as per-row predicates in both the legacy SCAN-cursor and sorted paths, immediately after the existing scope/status/tenant predicates and the v0.1.25.20 `createdAtInWindow`. Predicate bodies follow the same defensive shape: missing or unparseable hash field → row excluded when EITHER bound is supplied. + +**`finalizedAtInWindow` field resolution.** Mirrors `buildReservationSummary`'s projection logic: read `committed_at` first (populates the timestamp for COMMITTED rows), fall through to `released_at` for RELEASED rows. Both absent → row excluded. The legacy `committed_at`/`released_at` Redis fields are the source of truth; the wire field `finalized_at_ms` is a projection. Both filter and serializer agree on the source. + +**Cursor invalidation extends to all six bounds.** `FilterHasher.hash(...)` gains four trailing `Long` arguments (10 → 14). Each window pair emits its canonical block with **independent gating** — the v0.1.25.20 `from`/`to` block only emits when `fromMs || toMs != null`, the new `expires_*` block only when `expiresFromMs || expiresToMs != null`, and the `finalized_*` block likewise. This preserves byte-exact back-compat for **both** prior cursor generations: + +- v0.1.25.18 cursor (no windows) → canonical `t=acme|...|ts=` → golden hash `2f397ea0e8fb53b7` (locked down in v0.1.25.20) +- v0.1.25.20 cursor (from/to only) → canonical `t=acme|...|ts=|fr=100|to=200` → golden hash `ad7204d521cfd133` (newly locked down in v0.1.25.21 to prevent a future refactor from accidentally unioning the gating) + +**Validation choices** (mirror the v0.1.25.20 contract): + +- ISO-8601 parsed via `Instant.parse(...)`; malformed → 400 with distinct `Invalid {param_name}` message identifying which parameter failed. +- `expires_from > expires_to` and `finalized_from > finalized_to` → 400 *before* the repository call. +- Blank-string values for any of the six bounds treated as unset. +- Equal bounds (point window) accepted on each pair. + +**Internal Java signature changes, no wire impact beyond the schema addition.** `listReservations` 14 → 18 args; `listReservationsSorted` mirrors; `FilterHasher.hash` 10 → 14 args. All Java callers updated. The single wire-format change is the optional `finalized_at_ms` field on `ReservationSummary` — covered by the optional-property guarantee. + +**Coverage.** + +- `FilterHasherTest` (+3): expires_* values differ from base/from-to, finalized_* values differ from from-to/expires_*, v0.1.25.20 8-byte golden lockdown. +- `RedisReservationQueryTest` (+6) under `ExpiresAndFinalizedWindowFilter` nested class: legacy-path `expires_from` excludes-below, legacy-path `expires_to` excludes-above, legacy-path `finalized_from` excludes-ACTIVE-rows, `finalized_at` resolves from `released_at` when `committed_at` absent, all-three AND composition (created + expires + finalized), cursor mismatch on expires window change rejected with 400. +- `ReservationControllerTest` (+10) under `ListReservations` nested class: malformed-expires_from, malformed-expires_to, malformed-finalized_from, malformed-finalized_to, reversed-expires-window, reversed-finalized-window, expires propagation with `verify(...)` lock, finalized propagation with `verify(...)` lock, all-three combined with distinct epoch-ms per pair to catch slot mix-ups, blank-as-unset for new windows. + +557 protocol-service tests pass (384 data + 173 api), +19 vs v0.1.25.20. JaCoCo 95% bundle gate met. + +**Out of scope (intentionally).** The `time_field`-pivoted alternative parameter shape (one `from`/`to` plus a `time_field=created_at|expires_at|finalized_at` selector) was considered and rejected — it would have been an awkward retroactive change to the v0.1.25.20 shape and split the family-wide `from`/`to` convention. Three parallel pairs is the cleaner expansion. + +--- + ### 2026-05-21 — v0.1.25.20: `from` / `to` time-window filter on listReservations Closes [#159](https://github.com/runcycles/cycles-server/issues/159). Spec landed in `cycles-protocol-v0.yaml` revision 2026-05-21 (PR runcycles/cycles-protocol#97); this is the matching runtime impl. diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a1067c..3c4098b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,43 @@ changes to request/response bodies or Lua-script semantics would require a minor bump. "Internal signature changes" (e.g. Java method parameters) are called out but are not breaking to API clients. +## [0.1.25.21] — 2026-05-22 + +`expires_from`/`expires_to` and `finalized_from`/`finalized_to` ISO-8601 time-window filters on `GET /v1/reservations`, implementing `cycles-protocol-v0.yaml` revision 2026-05-22 ([runcycles/cycles-protocol#98](https://github.com/runcycles/cycles-protocol/pull/98)). Closes [#162](https://github.com/runcycles/cycles-server/issues/162). + +### Added + +- **Four new query parameters on `listReservations`** mirroring the v0.1.25.20 `from`/`to` shape. All ISO 8601 `format: date-time`, all optional, all inclusive bounds: + - `expires_from` / `expires_to` — bound on `expires_at_ms` (required field; applies to every row regardless of status). + - `finalized_from` / `finalized_to` — bound on `finalized_at_ms` (populated only on COMMITTED/RELEASED; ACTIVE and EXPIRED rows are normatively excluded since the field is absent). +- The three window filters (`from`/`to` + `expires_*` + `finalized_*`) compose with AND semantics — a row must satisfy every supplied predicate to be returned. +- **`finalized_at_ms` on `ReservationSummary`.** Pre-revision the field was only on `ReservationDetail`, which meant clients filtering with `finalized_*` couldn't see the timestamp they were filtering on without a per-row `getReservation` call. The summary now carries the field with the same population semantics. Strict-schema clients remain compatible because the field is optional (`@JsonInclude(NON_NULL)`). + +### Validation + +- Each pair rejects `expires_from > expires_to` and `finalized_from > finalized_to` with HTTP 400 before any repository call. +- Malformed ISO-8601 → 400 with distinct `Invalid {param_name}` message identifying which parameter failed. +- Blank-string values for any of the six bounds treated as unset per the normative carve-out in the 2026-05-22 spec revision. + +### Internal + +- `RedisReservationRepository.listReservations(...)` signature gains trailing `Long expiresFromMs, Long expiresToMs, Long finalizedFromMs, Long finalizedToMs` (14 → 18 args). Private `listReservationsSorted(...)` mirrors. Two new predicate helpers: `expiresAtInWindow(fields, fromMs, toMs)` and `finalizedAtInWindow(fields, fromMs, toMs)`, applied in both legacy SCAN-cursor and sorted paths after the existing scope/status/tenant predicates. +- `finalizedAtInWindow` resolves the timestamp from `committed_at` OR `released_at` (whichever is set), matching `buildReservationSummary`'s projection logic. Both fields absent → row excluded per the normative ACTIVE/EXPIRED exclusion. +- `FilterHasher.hash(...)` gains four trailing `Long` arguments (10 → 14 args) with independent gated emission. Each window pair emits its `|ef=|et=` / `|ff=|ft=` block only when at least one of its bounds is non-null — preserves byte-exact back-compat for **both** v0.1.25.18 cursors (no window canonical) and v0.1.25.20 cursors (`|fr=|to=` canonical, no expires/finalized). Locked down by `FilterHasherTest.preservesV01_25_20HashWhenOnlyFromTo` (golden `ad7204d521cfd133`). +- `ReservationSummary.finalizedAtMs` projection added to `toSummary(...)` builder. + +### Coverage + +- 557 protocol-service tests pass (384 data + 173 api), up from 538 in v0.1.25.20 (+19 new): + - `FilterHasherTest`: +3 new (expires/finalized distinctness, finalized vs from/to distinctness, v0.1.25.20 8-byte golden lockdown). + - `RedisReservationQueryTest`: +6 new under `ExpiresAndFinalizedWindowFilter` nested class. + - `ReservationControllerTest`: +10 new under `ListReservations` nested class (4 malformed-*, 2 reversed-window, expires propagation, finalized propagation, all-three combined, blank-as-unset for new windows). +- JaCoCo 95% bundle gate met. + +### Behavior change + +None for existing callers. All four new params are optional; the gated FilterHasher emission preserves byte-exact cursor back-compat for both v0.1.25.18 and v0.1.25.20 sorted-path cursors. The single new response-body field on `ReservationSummary` is optional with `@JsonInclude(NON_NULL)`, so v0.1.25.20-shape responses go out byte-for-byte when no terminal-state rows are returned. + ## [0.1.25.20] — 2026-05-21 `from` / `to` ISO-8601 time-window filter on `GET /v1/reservations`, diff --git a/cycles-protocol-service/README.md b/cycles-protocol-service/README.md index 478daf9..78858fc 100644 --- a/cycles-protocol-service/README.md +++ b/cycles-protocol-service/README.md @@ -461,6 +461,10 @@ List reservations for the effective tenant. Optional recovery/debug endpoint. Re | `sort_dir` | `asc` or `desc`. Defaults to `desc` when `sort_by` is provided. Ignored unless `sort_by` is set. | | `from` | ISO 8601 date-time (v0.1.25.20+). Inclusive lower bound on `created_at_ms`. Always binds to `created_at_ms` regardless of `sort_by`. May be supplied alone (no upper bound) or with `to`. Blank string treated as unset. | | `to` | ISO 8601 date-time (v0.1.25.20+). Inclusive upper bound on `created_at_ms`. Same binding and blank-as-unset rules as `from`. `from > to` returns `400 INVALID_REQUEST`. | +| `expires_from` | ISO 8601 date-time (v0.1.25.21+). Inclusive lower bound on `expires_at_ms`. Applies to every row regardless of status. May be supplied alone or with `expires_to`. Blank string treated as unset. | +| `expires_to` | ISO 8601 date-time (v0.1.25.21+). Inclusive upper bound on `expires_at_ms`. Same blank-as-unset rule. `expires_from > expires_to` returns `400 INVALID_REQUEST`. | +| `finalized_from` | ISO 8601 date-time (v0.1.25.21+). Inclusive lower bound on `finalized_at_ms`. Populated only on COMMITTED and RELEASED rows — ACTIVE and EXPIRED rows are normatively excluded when this is set. May be supplied alone or with `finalized_to`. Blank string treated as unset. | +| `finalized_to` | ISO 8601 date-time (v0.1.25.21+). Inclusive upper bound on `finalized_at_ms`. Same blank-as-unset rule and ACTIVE/EXPIRED exclusion. `finalized_from > finalized_to` returns `400 INVALID_REQUEST`. | When `sort_by` or `sort_dir` is provided, the cursor encodes the `(sort_by, sort_dir, filters)` tuple — reusing a sorted cursor after diff --git a/cycles-protocol-service/cycles-protocol-service-api/src/main/java/io/runcycles/protocol/api/controller/ReservationController.java b/cycles-protocol-service/cycles-protocol-service-api/src/main/java/io/runcycles/protocol/api/controller/ReservationController.java index f7e933f..6fecc65 100644 --- a/cycles-protocol-service/cycles-protocol-service-api/src/main/java/io/runcycles/protocol/api/controller/ReservationController.java +++ b/cycles-protocol-service/cycles-protocol-service-api/src/main/java/io/runcycles/protocol/api/controller/ReservationController.java @@ -252,7 +252,11 @@ public ResponseEntity list( @RequestParam(value = "sort_by", required = false) String sortBy, @RequestParam(value = "sort_dir", required = false) String sortDir, @RequestParam(required = false) String from, - @RequestParam(value = "to", required = false) String toParam) { + @RequestParam(value = "to", required = false) String toParam, + @RequestParam(value = "expires_from", required = false) String expiresFrom, + @RequestParam(value = "expires_to", required = false) String expiresTo, + @RequestParam(value = "finalized_from", required = false) String finalizedFrom, + @RequestParam(value = "finalized_to", required = false) String finalizedTo) { // Validate status against ReservationStatus enum if provided if (status != null) { try { @@ -296,6 +300,23 @@ public ResponseEntity list( throw new CyclesProtocolException(Enums.ErrorCode.INVALID_REQUEST, "from must be less than or equal to to (received from=" + from + ", to=" + toParam + ")", 400); } + // cycles-protocol revision 2026-05-22: expires_from/expires_to bind to + // expires_at_ms; finalized_from/finalized_to bind to finalized_at_ms. + // Each pair validates independently with its own from > to check; + // empty strings are treated as unset per the normative blank-string + // carve-out in the shared TIME-RANGE FILTERS validation block. + Long expiresFromMs = parseIsoToEpochMs(expiresFrom, "expires_from"); + Long expiresToMs = parseIsoToEpochMs(expiresTo, "expires_to"); + if (expiresFromMs != null && expiresToMs != null && expiresFromMs > expiresToMs) { + throw new CyclesProtocolException(Enums.ErrorCode.INVALID_REQUEST, + "expires_from must be less than or equal to expires_to (received expires_from=" + expiresFrom + ", expires_to=" + expiresTo + ")", 400); + } + Long finalizedFromMs = parseIsoToEpochMs(finalizedFrom, "finalized_from"); + Long finalizedToMs = parseIsoToEpochMs(finalizedTo, "finalized_to"); + if (finalizedFromMs != null && finalizedToMs != null && finalizedFromMs > finalizedToMs) { + throw new CyclesProtocolException(Enums.ErrorCode.INVALID_REQUEST, + "finalized_from must be less than or equal to finalized_to (received finalized_from=" + finalizedFrom + ", finalized_to=" + finalizedTo + ")", 400); + } // v0.1.25.8 (cycles-protocol revision 2026-04-13): tenant param // semantics differ by auth type. ApiKeyAuth: optional, falls // back to authenticated tenant, validation-only when present. @@ -316,11 +337,12 @@ public ResponseEntity list( effectiveTenant = tenant != null ? tenant : extractAuthTenantId(); authorizeTenant(effectiveTenant); } - LOG.info("GET /v1/reservations - tenant: {}, admin: {}, sort_by: {}, sort_dir: {}, from: {}, to: {}", - effectiveTenant, isAdminAuth(), sortBy, sortDir, fromMs, toMs); + LOG.info("GET /v1/reservations - tenant: {}, admin: {}, sort_by: {}, sort_dir: {}, from: {}, to: {}, expires_from: {}, expires_to: {}, finalized_from: {}, finalized_to: {}", + effectiveTenant, isAdminAuth(), sortBy, sortDir, fromMs, toMs, + expiresFromMs, expiresToMs, finalizedFromMs, finalizedToMs); return ResponseEntity.ok(repository.listReservations(effectiveTenant, idempotencyKey, status, workspace, app, workflow, agent, toolset, limit, cursor, sortBy, sortDir, - fromMs, toMs)); + fromMs, toMs, expiresFromMs, expiresToMs, finalizedFromMs, finalizedToMs)); } private Long parseIsoToEpochMs(String raw, String paramName) { diff --git a/cycles-protocol-service/cycles-protocol-service-api/src/test/java/io/runcycles/protocol/api/controller/ReservationControllerTest.java b/cycles-protocol-service/cycles-protocol-service-api/src/test/java/io/runcycles/protocol/api/controller/ReservationControllerTest.java index 82478ff..92eec20 100644 --- a/cycles-protocol-service/cycles-protocol-service-api/src/test/java/io/runcycles/protocol/api/controller/ReservationControllerTest.java +++ b/cycles-protocol-service/cycles-protocol-service-api/src/test/java/io/runcycles/protocol/api/controller/ReservationControllerTest.java @@ -499,7 +499,7 @@ void shouldListReservations() throws Exception { .reservations(Collections.emptyList()) .hasMore(false) .build(); - when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any())) + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations")) @@ -518,7 +518,7 @@ void shouldRejectInvalidStatusFilter() throws Exception { void shouldListWithActiveStatusFilter() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); - when(repository.listReservations(eq(TENANT), any(), eq("ACTIVE"), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any())) + when(repository.listReservations(eq(TENANT), any(), eq("ACTIVE"), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations").param("status", "ACTIVE")) @@ -530,7 +530,7 @@ void shouldListWithActiveStatusFilter() throws Exception { void shouldListWithCommittedStatusFilter() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); - when(repository.listReservations(eq(TENANT), any(), eq("COMMITTED"), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any())) + when(repository.listReservations(eq(TENANT), any(), eq("COMMITTED"), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations").param("status", "COMMITTED")) @@ -541,7 +541,7 @@ void shouldListWithCommittedStatusFilter() throws Exception { void shouldListWithExplicitTenantMatchingAuth() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); - when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any())) + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations").param("tenant", TENANT)) @@ -559,7 +559,7 @@ void shouldRejectListWithTenantMismatch() throws Exception { void shouldDefaultTenantFromAuth() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); - when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any())) + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any())) .thenReturn(resp); // No tenant param — should use auth tenant @@ -595,7 +595,7 @@ void shouldRejectInvalidSortDir() throws Exception { void shouldPropagateSortParams() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); - when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), eq("status"), eq("asc"), any(), any())) + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), eq("status"), eq("asc"), any(), any(), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations") .param("sort_by", "status") @@ -608,7 +608,7 @@ void shouldPropagateSortParams() throws Exception { void shouldAcceptAllSpecSortByValues() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); - when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any())) + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any())) .thenReturn(resp); for (String value : new String[] {"reservation_id", "tenant", "scope_path", "status", "reserved", "created_at_ms", "expires_at_ms"}) { @@ -653,7 +653,7 @@ void shouldRejectReversedWindow() throws Exception { // No repository invocation expected — validation runs before list call. verify(repository, org.mockito.Mockito.never()).listReservations( any(), any(), any(), any(), any(), any(), any(), any(), - anyInt(), any(), any(), any(), any(), any()); + anyInt(), any(), any(), any(), any(), any(), any(), any(), any(), any()); } // Derive the expected epoch-ms from the same parser the controller uses so a @@ -670,7 +670,7 @@ void shouldPropagateFromOnly() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), - eq(50), any(), any(), any(), eq(SAMPLE_FROM_TO_EPOCH_MS), eq((Long) null))) + eq(50), any(), any(), any(), eq(SAMPLE_FROM_TO_EPOCH_MS), eq((Long) null), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations").param("from", SAMPLE_FROM_TO_INSTANT)) .andExpect(status().isOk()); @@ -678,7 +678,7 @@ void shouldPropagateFromOnly() throws Exception { // a wrong eq(...) literal caused Mockito to silently return null while the // test still asserted HTTP 200. verify(repository).listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), - eq(50), any(), any(), any(), eq(SAMPLE_FROM_TO_EPOCH_MS), eq((Long) null)); + eq(50), any(), any(), any(), eq(SAMPLE_FROM_TO_EPOCH_MS), eq((Long) null), any(), any(), any(), any()); } @Test @@ -687,12 +687,12 @@ void shouldPropagateToOnly() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), - eq(50), any(), any(), any(), eq((Long) null), eq(SAMPLE_FROM_TO_EPOCH_MS))) + eq(50), any(), any(), any(), eq((Long) null), eq(SAMPLE_FROM_TO_EPOCH_MS), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations").param("to", SAMPLE_FROM_TO_INSTANT)) .andExpect(status().isOk()); verify(repository).listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), - eq(50), any(), any(), any(), eq((Long) null), eq(SAMPLE_FROM_TO_EPOCH_MS)); + eq(50), any(), any(), any(), eq((Long) null), eq(SAMPLE_FROM_TO_EPOCH_MS), any(), any(), any(), any()); } @Test @@ -701,14 +701,14 @@ void shouldAcceptEqualBounds() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), - eq(50), any(), any(), any(), eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS))) + eq(50), any(), any(), any(), eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations") .param("from", SAMPLE_FROM_TO_INSTANT) .param("to", SAMPLE_FROM_TO_INSTANT)) .andExpect(status().isOk()); verify(repository).listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), - eq(50), any(), any(), any(), eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS)); + eq(50), any(), any(), any(), eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS), any(), any(), any(), any()); } @Test @@ -717,7 +717,7 @@ void shouldCombineWithDifferentSortKey() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), - eq(50), any(), eq("expires_at_ms"), any(), any(), any())) + eq(50), any(), eq("expires_at_ms"), any(), any(), any(), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations") .param("sort_by", "expires_at_ms") @@ -732,13 +732,171 @@ void shouldTreatBlankAsUnset() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), - eq(50), any(), any(), any(), eq((Long) null), eq((Long) null))) + eq(50), any(), any(), any(), eq((Long) null), eq((Long) null), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations") .param("from", "") .param("to", "")) .andExpect(status().isOk()); } + + // cycles-protocol revision 2026-05-22: expires_from / expires_to bind + // to expires_at_ms; finalized_from / finalized_to bind to + // finalized_at_ms. Each pair validates independently with its own + // from > to check; malformed values surface a distinct 400 message + // identifying which param failed. + + @Test + @DisplayName("expires_from=bogus → 400 INVALID_REQUEST") + void shouldRejectMalformedExpiresFrom() throws Exception { + mockMvc.perform(get("/v1/reservations").param("expires_from", "not-a-date")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("INVALID_REQUEST")) + .andExpect(jsonPath("$.message").value( + org.hamcrest.Matchers.containsString("Invalid expires_from"))); + } + + @Test + @DisplayName("expires_to=bogus → 400 INVALID_REQUEST") + void shouldRejectMalformedExpiresTo() throws Exception { + mockMvc.perform(get("/v1/reservations").param("expires_to", "2026-13-99T99:99:99Z")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("INVALID_REQUEST")) + .andExpect(jsonPath("$.message").value( + org.hamcrest.Matchers.containsString("Invalid expires_to"))); + } + + @Test + @DisplayName("finalized_from=bogus → 400 INVALID_REQUEST") + void shouldRejectMalformedFinalizedFrom() throws Exception { + mockMvc.perform(get("/v1/reservations").param("finalized_from", "not-a-date")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("INVALID_REQUEST")) + .andExpect(jsonPath("$.message").value( + org.hamcrest.Matchers.containsString("Invalid finalized_from"))); + } + + @Test + @DisplayName("finalized_to=bogus → 400 INVALID_REQUEST") + void shouldRejectMalformedFinalizedTo() throws Exception { + mockMvc.perform(get("/v1/reservations").param("finalized_to", "not-a-date")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("INVALID_REQUEST")) + .andExpect(jsonPath("$.message").value( + org.hamcrest.Matchers.containsString("Invalid finalized_to"))); + } + + @Test + @DisplayName("expires_from > expires_to → 400 before any repository call") + void shouldRejectReversedExpiresWindow() throws Exception { + mockMvc.perform(get("/v1/reservations") + .param("expires_from", "2026-05-22T12:00:00Z") + .param("expires_to", "2026-05-22T11:00:00Z")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("INVALID_REQUEST")) + .andExpect(jsonPath("$.message").value( + org.hamcrest.Matchers.containsString("expires_from must be less than or equal to expires_to"))); + } + + @Test + @DisplayName("finalized_from > finalized_to → 400 before any repository call") + void shouldRejectReversedFinalizedWindow() throws Exception { + mockMvc.perform(get("/v1/reservations") + .param("finalized_from", "2026-05-22T12:00:00Z") + .param("finalized_to", "2026-05-22T11:00:00Z")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("INVALID_REQUEST")) + .andExpect(jsonPath("$.message").value( + org.hamcrest.Matchers.containsString("finalized_from must be less than or equal to finalized_to"))); + } + + @Test + @DisplayName("expires_from/expires_to → epoch-ms propagates to repository (independent of from/to)") + void shouldPropagateExpiresWindow() throws Exception { + ReservationListResponse resp = ReservationListResponse.builder() + .reservations(Collections.emptyList()).hasMore(false).build(); + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), + eq(50), any(), any(), any(), eq((Long) null), eq((Long) null), + eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS), eq((Long) null), eq((Long) null))) + .thenReturn(resp); + mockMvc.perform(get("/v1/reservations") + .param("expires_from", SAMPLE_FROM_TO_INSTANT) + .param("expires_to", SAMPLE_FROM_TO_INSTANT)) + .andExpect(status().isOk()); + // Lock in that the stub fired — the eq(...) matchers all matched the + // controller's parsed values. Without this verify(), a future refactor + // that misroutes expires_from into the from slot would silently pass. + verify(repository).listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), + eq(50), any(), any(), any(), eq((Long) null), eq((Long) null), + eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS), eq((Long) null), eq((Long) null)); + } + + @Test + @DisplayName("finalized_from/finalized_to → epoch-ms propagates to repository") + void shouldPropagateFinalizedWindow() throws Exception { + ReservationListResponse resp = ReservationListResponse.builder() + .reservations(Collections.emptyList()).hasMore(false).build(); + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), + eq(50), any(), any(), any(), eq((Long) null), eq((Long) null), + eq((Long) null), eq((Long) null), eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS))) + .thenReturn(resp); + mockMvc.perform(get("/v1/reservations") + .param("finalized_from", SAMPLE_FROM_TO_INSTANT) + .param("finalized_to", SAMPLE_FROM_TO_INSTANT)) + .andExpect(status().isOk()); + verify(repository).listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), + eq(50), any(), any(), any(), eq((Long) null), eq((Long) null), + eq((Long) null), eq((Long) null), eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS)); + } + + @Test + @DisplayName("all three windows combined → all 6 epoch-ms values propagate independently") + void shouldPropagateAllThreeWindows() throws Exception { + ReservationListResponse resp = ReservationListResponse.builder() + .reservations(Collections.emptyList()).hasMore(false).build(); + // 2026-05-21T12:00:00Z (SAMPLE_FROM_TO) is the from/to value. + // Use different instants for expires/finalized so a slot mix-up would fail. + final String EXPIRES_INSTANT = "2026-05-23T00:00:00Z"; + final long EXPIRES_MS = java.time.Instant.parse(EXPIRES_INSTANT).toEpochMilli(); + final String FINALIZED_INSTANT = "2026-05-24T00:00:00Z"; + final long FINALIZED_MS = java.time.Instant.parse(FINALIZED_INSTANT).toEpochMilli(); + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), + eq(50), any(), any(), any(), + eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS), + eq(EXPIRES_MS), eq(EXPIRES_MS), + eq(FINALIZED_MS), eq(FINALIZED_MS))) + .thenReturn(resp); + mockMvc.perform(get("/v1/reservations") + .param("from", SAMPLE_FROM_TO_INSTANT) + .param("to", SAMPLE_FROM_TO_INSTANT) + .param("expires_from", EXPIRES_INSTANT) + .param("expires_to", EXPIRES_INSTANT) + .param("finalized_from", FINALIZED_INSTANT) + .param("finalized_to", FINALIZED_INSTANT)) + .andExpect(status().isOk()); + verify(repository).listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), + eq(50), any(), any(), any(), + eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS), + eq(EXPIRES_MS), eq(EXPIRES_MS), + eq(FINALIZED_MS), eq(FINALIZED_MS)); + } + + @Test + @DisplayName("expires_from/expires_to/finalized_from/finalized_to blank strings treated as unset") + void shouldTreatBlankAsUnsetForNewWindows() throws Exception { + ReservationListResponse resp = ReservationListResponse.builder() + .reservations(Collections.emptyList()).hasMore(false).build(); + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), + eq(50), any(), any(), any(), eq((Long) null), eq((Long) null), + eq((Long) null), eq((Long) null), eq((Long) null), eq((Long) null))) + .thenReturn(resp); + mockMvc.perform(get("/v1/reservations") + .param("expires_from", "") + .param("expires_to", "") + .param("finalized_from", "") + .param("finalized_to", "")) + .andExpect(status().isOk()); + } } // v0.1.25.8 (cycles-protocol revision 2026-04-13): admin-on-behalf-of @@ -757,7 +915,7 @@ void setAdminAuth() { void adminListWithTenantFilter() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); - when(repository.listReservations(eq("any-tenant"), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any())) + when(repository.listReservations(eq("any-tenant"), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations").param("tenant", "any-tenant")) .andExpect(status().isOk()); diff --git a/cycles-protocol-service/cycles-protocol-service-data/src/main/java/io/runcycles/protocol/data/repository/RedisReservationRepository.java b/cycles-protocol-service/cycles-protocol-service-data/src/main/java/io/runcycles/protocol/data/repository/RedisReservationRepository.java index 6c82a47..61b01a8 100644 --- a/cycles-protocol-service/cycles-protocol-service-data/src/main/java/io/runcycles/protocol/data/repository/RedisReservationRepository.java +++ b/cycles-protocol-service/cycles-protocol-service-data/src/main/java/io/runcycles/protocol/data/repository/RedisReservationRepository.java @@ -554,7 +554,9 @@ public ReservationListResponse listReservations(String tenant, String idempotenc String workflow, String agent, String toolset, int limit, String startCursor, String sortBy, String sortDir, - Long fromMs, Long toMs) { + Long fromMs, Long toMs, + Long expiresFromMs, Long expiresToMs, + Long finalizedFromMs, Long finalizedToMs) { boolean sortRequested = sortBy != null || sortDir != null; Optional parsedCursor = SortedListCursor.decode(startCursor); @@ -564,7 +566,7 @@ public ReservationListResponse listReservations(String tenant, String idempotenc if (sortRequested || parsedCursor.isPresent()) { return listReservationsSorted(tenant, idempotencyKey, status, workspace, app, workflow, agent, toolset, limit, parsedCursor.orElse(null), sortBy, sortDir, - fromMs, toMs); + fromMs, toMs, expiresFromMs, expiresToMs, finalizedFromMs, finalizedToMs); } try (Jedis jedis = jedisPool.getResource()) { @@ -605,6 +607,8 @@ public ReservationListResponse listReservations(String tenant, String idempotenc if (agentSegment != null && !scopeHasSegment(scopePath, agentSegment)) continue; if (toolsetSegment != null && !scopeHasSegment(scopePath, toolsetSegment)) continue; if (!createdAtInWindow(fields, fromMs, toMs)) continue; + if (!expiresAtInWindow(fields, expiresFromMs, expiresToMs)) continue; + if (!finalizedAtInWindow(fields, finalizedFromMs, finalizedToMs)) continue; result.add(toSummary(buildReservationSummary(fields))); @@ -644,7 +648,9 @@ private ReservationListResponse listReservationsSorted( String tenant, String idempotencyKey, String status, String workspace, String app, String workflow, String agent, String toolset, int limit, SortedListCursor resumeCursor, String sortBy, String sortDir, - Long fromMs, Long toMs) { + Long fromMs, Long toMs, + Long expiresFromMs, Long expiresToMs, + Long finalizedFromMs, Long finalizedToMs) { // Normalize for cursor storage + comparator use. Null sort_dir with a non-null // sort_by defaults to DESC per spec; null sort_by with a non-null sort_dir defaults @@ -655,7 +661,8 @@ private ReservationListResponse listReservationsSorted( : (resumeCursor != null ? resumeCursor.getSortDir() : "desc"); String filterHash = FilterHasher.hash(tenant, idempotencyKey, status, - workspace, app, workflow, agent, toolset, fromMs, toMs); + workspace, app, workflow, agent, toolset, fromMs, toMs, + expiresFromMs, expiresToMs, finalizedFromMs, finalizedToMs); // Spec: cursor is valid only for the same (sort_by, sort_dir, filters) tuple. if (resumeCursor != null) { @@ -707,6 +714,8 @@ private ReservationListResponse listReservationsSorted( if (agentSegment != null && !scopeHasSegment(scopePath, agentSegment)) continue; if (toolsetSegment != null && !scopeHasSegment(scopePath, toolsetSegment)) continue; if (!createdAtInWindow(fields, fromMs, toMs)) continue; + if (!expiresAtInWindow(fields, expiresFromMs, expiresToMs)) continue; + if (!finalizedAtInWindow(fields, finalizedFromMs, finalizedToMs)) continue; matching.add(toSummary(buildReservationSummary(fields))); } catch (Exception e) { @@ -785,6 +794,69 @@ private static boolean createdAtInWindow(Map fields, Long fromMs return true; } + /** + * Inclusive time-window predicate for listReservations expires_from / expires_to + * filters (cycles-protocol-v0.yaml revision 2026-05-22). Reads the per-reservation + * {@code expires_at} hash field (epoch-ms decimal string). Same defensive shape as + * {@link #createdAtInWindow}: missing or unparseable {@code expires_at} is treated + * as out-of-window when EITHER bound is supplied. Returns true when both bounds + * are null (filter inactive). + * + *

{@code expires_at_ms} is REQUIRED on every reservation per the spec, so the + * field-absent path here is purely defensive against malformed Redis writes — in + * normal operation every hydrated reservation has it. + */ + private static boolean expiresAtInWindow(Map fields, Long fromMs, Long toMs) { + if (fromMs == null && toMs == null) return true; + String expiresAtStr = fields.get("expires_at"); + if (expiresAtStr == null) return false; + long expiresAt; + try { + expiresAt = Long.parseLong(expiresAtStr); + } catch (NumberFormatException e) { + return false; + } + if (fromMs != null && expiresAt < fromMs) return false; + if (toMs != null && expiresAt > toMs) return false; + return true; + } + + /** + * Inclusive time-window predicate for listReservations finalized_from / + * finalized_to filters (cycles-protocol-v0.yaml revision 2026-05-22). + * + *

Per the spec, {@code finalized_at_ms} is populated ONLY on COMMITTED and + * RELEASED rows; absent on ACTIVE and EXPIRED. Mirrors the same projection logic + * as {@link #buildReservationSummary}: {@code committed_at} populates the + * timestamp for COMMITTED rows, {@code released_at} for RELEASED rows. Rows + * missing both hash fields (ACTIVE, EXPIRED, malformed) MUST be excluded from + * results when either bound is supplied — the spec makes this exclusion + * normative so conformant servers agree on the contract. + * + *

Returns true when both bounds are null (filter inactive); this case + * preserves the row regardless of whether {@code finalized_at_ms} is + * present, matching the v0.1.25.20 unfiltered behavior byte-for-byte. + */ + private static boolean finalizedAtInWindow(Map fields, Long fromMs, Long toMs) { + if (fromMs == null && toMs == null) return true; + String committedAt = fields.get("committed_at"); + String releasedAt = fields.get("released_at"); + // Resolve the finalized timestamp the same way buildReservationSummary does. + // committed_at wins if both somehow exist (shouldn't, given the lifecycle is + // commit XOR release); released_at falls through. Both absent → row excluded. + String finalizedAtStr = committedAt != null ? committedAt : releasedAt; + if (finalizedAtStr == null) return false; + long finalizedAt; + try { + finalizedAt = Long.parseLong(finalizedAtStr); + } catch (NumberFormatException e) { + return false; + } + if (fromMs != null && finalizedAt < fromMs) return false; + if (toMs != null && finalizedAt > toMs) return false; + return true; + } + private static int findSliceStart(List sorted, String sortBy, String sortDir, SortedListCursor cursor) { // Walk the sorted list looking for the first row strictly greater than the cursor's @@ -1215,6 +1287,11 @@ private ReservationSummary toSummary(ReservationDetail detail) { .reserved(detail.getReserved()) .createdAtMs(detail.getCreatedAtMs()) .expiresAtMs(detail.getExpiresAtMs()) + // Spec: cycles-protocol-v0.yaml revision 2026-05-22. Optional; + // null on ACTIVE and EXPIRED rows. NON_NULL JsonInclude on the + // class skips the field from the wire when absent, preserving + // byte-for-byte response shape for pre-revision callers. + .finalizedAtMs(detail.getFinalizedAtMs()) .scopePath(detail.getScopePath()) .affectedScopes(detail.getAffectedScopes()) .build(); diff --git a/cycles-protocol-service/cycles-protocol-service-data/src/main/java/io/runcycles/protocol/data/repository/support/FilterHasher.java b/cycles-protocol-service/cycles-protocol-service-data/src/main/java/io/runcycles/protocol/data/repository/support/FilterHasher.java index ab690b6..db3e5ea 100644 --- a/cycles-protocol-service/cycles-protocol-service-data/src/main/java/io/runcycles/protocol/data/repository/support/FilterHasher.java +++ b/cycles-protocol-service/cycles-protocol-service-data/src/main/java/io/runcycles/protocol/data/repository/support/FilterHasher.java @@ -22,7 +22,9 @@ private FilterHasher() {} public static String hash(String tenant, String idempotencyKey, String status, String workspace, String app, String workflow, String agent, String toolset, - Long fromMs, Long toMs) { + Long fromMs, Long toMs, + Long expiresFromMs, Long expiresToMs, + Long finalizedFromMs, Long finalizedToMs) { StringBuilder canonical = new StringBuilder(256); canonical.append("t=").append(nullSafe(tenant)).append('|'); canonical.append("i=").append(nullSafe(idempotencyKey)).append('|'); @@ -32,18 +34,34 @@ public static String hash(String tenant, String idempotencyKey, String status, canonical.append("wf=").append(nullSafe(workflow)).append('|'); canonical.append("ag=").append(nullSafe(agent)).append('|'); canonical.append("ts=").append(nullSafe(toolset)); - // Back-compat: only emit the from/to fields when at least one bound is set. - // A canonical form that always carried `|fr=|to=` would change the hash for - // every pre-window cursor (including any v0.1.25.18 sorted-path cursor - // mid-pagination across the deployment), breaking the stated wire back-compat - // for clients that never send the new params. Gated emission preserves the - // v0.1.25.12 8-field hash byte-exactly for the no-window case while still - // uniquely identifying any combination of supplied bounds. + // Back-compat: each window pair only emits its canonical block when at + // least one of its bounds is set. A canonical form that always carried + // every window's |...=|...= chunks would change the hash for every + // pre-window cursor (a v0.1.25.18 sorted-path cursor mid-pagination + // across the deployment, a v0.1.25.20 cursor with from/to set but + // no expires_*/finalized_*, etc.), breaking the wire back-compat + // guarantee. Independent gating preserves byte-exact hashes for + // every prior generation of cursor that didn't carry the supplied + // bounds, while still uniquely identifying any new combination. + // + // v0.1.25.20 added the `fr=`/`to=` pair (created_at_ms window). + // v0.1.25.21 adds `ef=`/`et=` (expires_at_ms) and `ff=`/`ft=` + // (finalized_at_ms) per cycles-protocol-v0.yaml revision 2026-05-22. if (fromMs != null || toMs != null) { canonical.append('|'); canonical.append("fr=").append(nullSafeLong(fromMs)).append('|'); canonical.append("to=").append(nullSafeLong(toMs)); } + if (expiresFromMs != null || expiresToMs != null) { + canonical.append('|'); + canonical.append("ef=").append(nullSafeLong(expiresFromMs)).append('|'); + canonical.append("et=").append(nullSafeLong(expiresToMs)); + } + if (finalizedFromMs != null || finalizedToMs != null) { + canonical.append('|'); + canonical.append("ff=").append(nullSafeLong(finalizedFromMs)).append('|'); + canonical.append("ft=").append(nullSafeLong(finalizedToMs)); + } try { MessageDigest md = MessageDigest.getInstance("SHA-256"); byte[] digest = md.digest(canonical.toString().getBytes(StandardCharsets.UTF_8)); diff --git a/cycles-protocol-service/cycles-protocol-service-data/src/test/java/io/runcycles/protocol/data/repository/RedisReservationQueryTest.java b/cycles-protocol-service/cycles-protocol-service-data/src/test/java/io/runcycles/protocol/data/repository/RedisReservationQueryTest.java index 98f7eee..0751cad 100644 --- a/cycles-protocol-service/cycles-protocol-service-data/src/test/java/io/runcycles/protocol/data/repository/RedisReservationQueryTest.java +++ b/cycles-protocol-service/cycles-protocol-service-data/src/test/java/io/runcycles/protocol/data/repository/RedisReservationQueryTest.java @@ -407,7 +407,7 @@ void shouldReturnEmptyListWhenNoReservations() { when(jedis.scan(eq("0"), any(ScanParams.class))).thenReturn(scanResult); ReservationListResponse response = repository.listReservations( - "acme", null, null, null, null, null, null, null, 100, null, null, null, null, null); + "acme", null, null, null, null, null, null, null, 100, null, null, null, null, null, null, null, null, null); assertThat(response.getReservations()).isEmpty(); assertThat(response.getHasMore()).isFalse(); @@ -433,7 +433,7 @@ void shouldFilterByStatus() { // Filter for ACTIVE but reservation is COMMITTED ReservationListResponse response = repository.listReservations( - "acme", null, "ACTIVE", null, null, null, null, null, 100, null, null, null, null, null); + "acme", null, "ACTIVE", null, null, null, null, null, 100, null, null, null, null, null, null, null, null, null); assertThat(response.getReservations()).isEmpty(); } @@ -463,7 +463,7 @@ void shouldFilterByTenantExcludingOtherTenants() { when(pipeline.hgetAll("reservation:res_r2")).thenReturn(resp2); ReservationListResponse response = repository.listReservations( - "acme", null, null, null, null, null, null, null, 100, null, null, null, null, null); + "acme", null, null, null, null, null, null, null, 100, null, null, null, null, null, null, null, null, null); assertThat(response.getReservations()).hasSize(1); assertThat(response.getReservations().get(0).getReservationId()).isEqualTo("r1"); @@ -495,7 +495,7 @@ void shouldFilterByWorkspaceSubjectField() { when(pipeline.hgetAll("reservation:res_r2")).thenReturn(resp2); ReservationListResponse response = repository.listReservations( - "acme", null, null, "dev", null, null, null, null, 100, null, null, null, null, null); + "acme", null, null, "dev", null, null, null, null, 100, null, null, null, null, null, null, null, null, null); assertThat(response.getReservations()).hasSize(1); assertThat(response.getReservations().get(0).getReservationId()).isEqualTo("r1"); @@ -527,7 +527,7 @@ void shouldFilterByAppSubjectField() { when(pipeline.hgetAll("reservation:res_r2")).thenReturn(resp2); ReservationListResponse response = repository.listReservations( - "acme", null, null, null, "myapp", null, null, null, 100, null, null, null, null, null); + "acme", null, null, null, "myapp", null, null, null, 100, null, null, null, null, null, null, null, null, null); assertThat(response.getReservations()).hasSize(1); assertThat(response.getReservations().get(0).getReservationId()).isEqualTo("r1"); @@ -556,7 +556,7 @@ void shouldRespectLimitAndReturnHasMore() { when(pipeline.hgetAll("reservation:res_r2")).thenReturn(resp1); ReservationListResponse response = repository.listReservations( - "acme", null, null, null, null, null, null, null, 1, null, null, null, null, null); + "acme", null, null, null, null, null, null, null, 1, null, null, null, null, null, null, null, null, null); assertThat(response.getReservations()).hasSize(1); assertThat(response.getHasMore()).isTrue(); @@ -584,7 +584,7 @@ void shouldReturnMatchingStatusFilter() { // Filter for COMMITTED and reservation IS COMMITTED ReservationListResponse response = repository.listReservations( - "acme", null, "COMMITTED", null, null, null, null, null, 100, null, null, null, null, null); + "acme", null, "COMMITTED", null, null, null, null, null, 100, null, null, null, null, null, null, null, null, null); assertThat(response.getReservations()).hasSize(1); assertThat(response.getReservations().get(0).getReservationId()).isEqualTo("r1"); @@ -623,7 +623,7 @@ void shouldFilterByIdempotencyKey() { when(pipeline.hgetAll("reservation:res_r2")).thenReturn(resp2); ReservationListResponse response = repository.listReservations( - "acme", "idem-abc", null, null, null, null, null, null, 100, null, null, null, null, null); + "acme", "idem-abc", null, null, null, null, null, null, 100, null, null, null, null, null, null, null, null, null); assertThat(response.getReservations()).hasSize(1); assertThat(response.getReservations().get(0).getReservationId()).isEqualTo("r1"); @@ -650,7 +650,7 @@ void shouldReturnEmptyWhenIdempotencyKeyDoesNotMatch() { when(pipeline.hgetAll("reservation:res_r1")).thenReturn(resp); ReservationListResponse response = repository.listReservations( - "acme", "idem-nonexistent", null, null, null, null, null, null, 100, null, null, null, null, null); + "acme", "idem-nonexistent", null, null, null, null, null, null, 100, null, null, null, null, null, null, null, null, null); assertThat(response.getReservations()).isEmpty(); } @@ -689,7 +689,7 @@ void shouldSkipMalformedReservationInList() { when(pipeline.hgetAll("reservation:res_r1")).thenReturn(resp2); ReservationListResponse response = repository.listReservations( - "acme", null, null, null, null, null, null, null, 100, null, null, null, null, null); + "acme", null, null, null, null, null, null, null, 100, null, null, null, null, null, null, null, null, null); // Broken reservation skipped, valid one returned assertThat(response.getReservations()).hasSize(1); @@ -738,7 +738,7 @@ void sortsByCreatedAtAsc() { ReservationListResponse response = repository.listReservations( "acme", null, null, null, null, null, null, null, 100, null, - "created_at_ms", "asc", null, null); + "created_at_ms", "asc", null, null, null, null, null, null); assertThat(response.getReservations()).extracting(ReservationSummary::getReservationId) .containsExactly("r2", "r3", "r1"); @@ -781,7 +781,7 @@ void paginatesAcrossPages() { ReservationListResponse page1 = repository.listReservations( "acme", null, null, null, null, null, null, null, 2, null, - "created_at_ms", "asc", null, null); + "created_at_ms", "asc", null, null, null, null, null, null); assertThat(page1.getReservations()).extracting(ReservationSummary::getReservationId) .containsExactly("r1", "r2"); @@ -790,7 +790,7 @@ void paginatesAcrossPages() { ReservationListResponse page2 = repository.listReservations( "acme", null, null, null, null, null, null, null, 2, page1.getNextCursor(), - "created_at_ms", "asc", null, null); + "created_at_ms", "asc", null, null, null, null, null, null); assertThat(page2.getReservations()).extracting(ReservationSummary::getReservationId) .containsExactly("r3", "r4"); @@ -823,7 +823,7 @@ void cursorMismatchRejected() { ReservationListResponse page1 = repository.listReservations( "acme", null, null, null, null, null, null, null, 1, null, - "created_at_ms", "asc", null, null); + "created_at_ms", "asc", null, null, null, null, null, null); String cursor = page1.getNextCursor(); assertThat(cursor).isNotNull(); @@ -831,7 +831,7 @@ void cursorMismatchRejected() { // Re-use cursor under a different sort_by — MUST 400 per spec. assertThatThrownBy(() -> repository.listReservations( "acme", null, null, null, null, null, null, null, 1, cursor, - "status", "asc", null, null)) + "status", "asc", null, null, null, null, null, null)) .isInstanceOf(io.runcycles.protocol.data.exception.CyclesProtocolException.class) .hasMessageContaining("cursor is not valid"); } @@ -879,7 +879,7 @@ void sortedHydrationStopsAtCap() { ReservationListResponse response = repository.listReservations( "acme", null, null, null, null, null, null, null, 5, null, - "created_at_ms", "asc", null, null); + "created_at_ms", "asc", null, null, null, null, null, null); assertThat(response.getReservations()).hasSize(5); assertThat(response.getReservations()) @@ -905,7 +905,7 @@ void legacyCursorPreserved() { // "42" is a legacy SCAN cursor. With no sort params, repo must honour it and // call jedis.scan with that exact cursor value — not route to sorted path. ReservationListResponse response = repository.listReservations( - "acme", null, null, null, null, null, null, null, 100, "42", null, null, null, null); + "acme", null, null, null, null, null, null, null, 100, "42", null, null, null, null, null, null, null, null); assertThat(response.getReservations()).isEmpty(); verify(jedis).scan(eq("42"), any(ScanParams.class)); @@ -948,7 +948,7 @@ void legacyPathFromExcludesBelow() { ReservationListResponse response = repository.listReservations( "acme", null, null, null, null, null, null, null, 100, null, null, null, - 3000L, null); + 3000L, null, null, null, null, null); assertThat(response.getReservations()).extracting(ReservationSummary::getReservationId) .containsExactly("r2"); @@ -981,7 +981,7 @@ void legacyPathToExcludesAbove() { ReservationListResponse response = repository.listReservations( "acme", null, null, null, null, null, null, null, 100, null, null, null, - null, 7000L); + null, 7000L, null, null, null, null); assertThat(response.getReservations()).extracting(ReservationSummary::getReservationId) .containsExactly("r1"); @@ -1027,7 +1027,7 @@ void legacyPathInclusiveBounds() { ReservationListResponse response = repository.listReservations( "acme", null, null, null, null, null, null, null, 100, null, null, null, - 3000L, 7000L); + 3000L, 7000L, null, null, null, null); assertThat(response.getReservations()).extracting(ReservationSummary::getReservationId) .containsExactlyInAnyOrder("r1", "r2", "r3"); @@ -1067,7 +1067,7 @@ void sortedPathHonoursWindow() { ReservationListResponse response = repository.listReservations( "acme", null, null, null, null, null, null, null, 100, null, - "created_at_ms", "asc", 200L, 1000L); + "created_at_ms", "asc", 200L, 1000L, null, null, null, null); assertThat(response.getReservations()).extracting(ReservationSummary::getReservationId) .containsExactly("r2", "r3"); @@ -1099,7 +1099,7 @@ void cursorMismatchOnWindowChange() { ReservationListResponse page1 = repository.listReservations( "acme", null, null, null, null, null, null, null, 1, null, - "created_at_ms", "asc", 200L, 1000L); + "created_at_ms", "asc", 200L, 1000L, null, null, null, null); String cursor = page1.getNextCursor(); assertThat(cursor).isNotNull(); @@ -1107,7 +1107,7 @@ void cursorMismatchOnWindowChange() { // from/to so the cursor tuple invalidates on window change). assertThatThrownBy(() -> repository.listReservations( "acme", null, null, null, null, null, null, null, 1, cursor, - "created_at_ms", "asc", 999L, 1000L)) + "created_at_ms", "asc", 999L, 1000L, null, null, null, null)) .isInstanceOf(io.runcycles.protocol.data.exception.CyclesProtocolException.class) .hasMessageContaining("cursor is not valid"); } @@ -1142,7 +1142,7 @@ void missingCreatedAtExcludedWithBound() { ReservationListResponse response = repository.listReservations( "acme", null, null, null, null, null, null, null, 100, null, null, null, - 1000L, null); + 1000L, null, null, null, null, null); assertThat(response.getReservations()).extracting(ReservationSummary::getReservationId) .containsExactly("r1"); @@ -1175,10 +1175,238 @@ void unparseableCreatedAtExcludedWithBound() { ReservationListResponse response = repository.listReservations( "acme", null, null, null, null, null, null, null, 100, null, null, null, - null, 10000L); + null, 10000L, null, null, null, null); assertThat(response.getReservations()).extracting(ReservationSummary::getReservationId) .containsExactly("r1"); } } + + // cycles-protocol revision 2026-05-22 — expires_from / expires_to filter + // binds to expires_at_ms (required on every row); finalized_from / + // finalized_to filter binds to finalized_at_ms (resolved from + // committed_at or released_at; ACTIVE/EXPIRED rows have neither and + // are normatively excluded). + @Nested + @DisplayName("listReservations — expires_* / finalized_* time windows") + class ExpiresAndFinalizedWindowFilter { + + @SuppressWarnings("unchecked") + @Test + @DisplayName("legacy path: expires_from excludes rows expiring earlier") + void expiresFromExcludesEarlierExpiry() { + when(jedisPool.getResource()).thenReturn(jedis); + doNothing().when(jedis).close(); + + // Row r1 expires at 1000 (early), r2 expires at 5000. expires_from=3000. + Map early = reservationFields("r1", "ACTIVE"); + early.put("expires_at", "1000"); + Map late = reservationFields("r2", "ACTIVE"); + late.put("expires_at", "5000"); + Response> resp1 = mock(Response.class); + Response> resp2 = mock(Response.class); + when(resp1.get()).thenReturn(early); + when(resp2.get()).thenReturn(late); + + ScanResult scanResult = mock(ScanResult.class); + when(scanResult.getResult()).thenReturn(List.of("reservation:res_r1", "reservation:res_r2")); + when(scanResult.getCursor()).thenReturn("0"); + when(jedis.scan(eq("0"), any(ScanParams.class))).thenReturn(scanResult); + when(jedis.pipelined()).thenReturn(pipeline); + when(pipeline.hgetAll("reservation:res_r1")).thenReturn(resp1); + when(pipeline.hgetAll("reservation:res_r2")).thenReturn(resp2); + + ReservationListResponse response = repository.listReservations( + "acme", null, null, null, null, null, null, null, 100, null, null, null, + null, null, 3000L, null, null, null); + + assertThat(response.getReservations()).extracting(ReservationSummary::getReservationId) + .containsExactly("r2"); + } + + @SuppressWarnings("unchecked") + @Test + @DisplayName("legacy path: expires_to excludes rows expiring later") + void expiresToExcludesLaterExpiry() { + when(jedisPool.getResource()).thenReturn(jedis); + doNothing().when(jedis).close(); + + Map inside = reservationFields("r1", "ACTIVE"); + inside.put("expires_at", "5000"); + Map later = reservationFields("r2", "ACTIVE"); + later.put("expires_at", "9000"); + Response> resp1 = mock(Response.class); + Response> resp2 = mock(Response.class); + when(resp1.get()).thenReturn(inside); + when(resp2.get()).thenReturn(later); + + ScanResult scanResult = mock(ScanResult.class); + when(scanResult.getResult()).thenReturn(List.of("reservation:res_r1", "reservation:res_r2")); + when(scanResult.getCursor()).thenReturn("0"); + when(jedis.scan(eq("0"), any(ScanParams.class))).thenReturn(scanResult); + when(jedis.pipelined()).thenReturn(pipeline); + when(pipeline.hgetAll("reservation:res_r1")).thenReturn(resp1); + when(pipeline.hgetAll("reservation:res_r2")).thenReturn(resp2); + + ReservationListResponse response = repository.listReservations( + "acme", null, null, null, null, null, null, null, 100, null, null, null, + null, null, null, 7000L, null, null); + + assertThat(response.getReservations()).extracting(ReservationSummary::getReservationId) + .containsExactly("r1"); + } + + @SuppressWarnings("unchecked") + @Test + @DisplayName("legacy path: finalized_from excludes ACTIVE rows (no committed_at/released_at)") + void finalizedFromExcludesActiveRows() { + when(jedisPool.getResource()).thenReturn(jedis); + doNothing().when(jedis).close(); + + // r1 is ACTIVE — has neither committed_at nor released_at, so the + // predicate must drop it when finalized_from is supplied. r2 is + // COMMITTED at ts=5000 → kept. + Map active = reservationFields("r1", "ACTIVE"); + Map committed = reservationFields("r2", "COMMITTED"); + committed.put("committed_at", "5000"); + Response> resp1 = mock(Response.class); + Response> resp2 = mock(Response.class); + when(resp1.get()).thenReturn(active); + when(resp2.get()).thenReturn(committed); + + ScanResult scanResult = mock(ScanResult.class); + when(scanResult.getResult()).thenReturn(List.of("reservation:res_r1", "reservation:res_r2")); + when(scanResult.getCursor()).thenReturn("0"); + when(jedis.scan(eq("0"), any(ScanParams.class))).thenReturn(scanResult); + when(jedis.pipelined()).thenReturn(pipeline); + when(pipeline.hgetAll("reservation:res_r1")).thenReturn(resp1); + when(pipeline.hgetAll("reservation:res_r2")).thenReturn(resp2); + + ReservationListResponse response = repository.listReservations( + "acme", null, null, null, null, null, null, null, 100, null, null, null, + null, null, null, null, 1000L, null); + + assertThat(response.getReservations()).extracting(ReservationSummary::getReservationId) + .containsExactly("r2"); + } + + @SuppressWarnings("unchecked") + @Test + @DisplayName("legacy path: finalized_at resolves from released_at when committed_at absent") + void finalizedAtResolvesFromReleasedAt() { + when(jedisPool.getResource()).thenReturn(jedis); + doNothing().when(jedis).close(); + + // r1 is RELEASED at ts=5000 — finalized_at should resolve from + // released_at; expect inside the window. + Map released = reservationFields("r1", "RELEASED"); + released.put("released_at", "5000"); + // r2 is RELEASED at ts=9000 — outside window. + Map releasedLate = reservationFields("r2", "RELEASED"); + releasedLate.put("released_at", "9000"); + Response> resp1 = mock(Response.class); + Response> resp2 = mock(Response.class); + when(resp1.get()).thenReturn(released); + when(resp2.get()).thenReturn(releasedLate); + + ScanResult scanResult = mock(ScanResult.class); + when(scanResult.getResult()).thenReturn(List.of("reservation:res_r1", "reservation:res_r2")); + when(scanResult.getCursor()).thenReturn("0"); + when(jedis.scan(eq("0"), any(ScanParams.class))).thenReturn(scanResult); + when(jedis.pipelined()).thenReturn(pipeline); + when(pipeline.hgetAll("reservation:res_r1")).thenReturn(resp1); + when(pipeline.hgetAll("reservation:res_r2")).thenReturn(resp2); + + ReservationListResponse response = repository.listReservations( + "acme", null, null, null, null, null, null, null, 100, null, null, null, + null, null, null, null, 3000L, 7000L); + + assertThat(response.getReservations()).extracting(ReservationSummary::getReservationId) + .containsExactly("r1"); + } + + @SuppressWarnings("unchecked") + @Test + @DisplayName("all three windows compose with AND semantics") + void threeWindowsAndCompose() { + when(jedisPool.getResource()).thenReturn(jedis); + doNothing().when(jedis).close(); + + // Three rows. r1 satisfies created+expires+finalized. r2 fails created + // (outside from/to). r3 fails finalized (ACTIVE → no finalized_at). + Map matches = reservationFields("r1", "COMMITTED"); + matches.put("created_at", "5000"); + matches.put("expires_at", "5000"); + matches.put("committed_at", "5000"); + Map wrongCreated = reservationFields("r2", "COMMITTED"); + wrongCreated.put("created_at", "100"); // before from=3000 + wrongCreated.put("expires_at", "5000"); + wrongCreated.put("committed_at", "5000"); + Map activeRow = reservationFields("r3", "ACTIVE"); + activeRow.put("created_at", "5000"); + activeRow.put("expires_at", "5000"); + + Response> resp1 = mock(Response.class); + Response> resp2 = mock(Response.class); + Response> resp3 = mock(Response.class); + when(resp1.get()).thenReturn(matches); + when(resp2.get()).thenReturn(wrongCreated); + when(resp3.get()).thenReturn(activeRow); + + ScanResult scanResult = mock(ScanResult.class); + when(scanResult.getResult()).thenReturn(List.of( + "reservation:res_r1", "reservation:res_r2", "reservation:res_r3")); + when(scanResult.getCursor()).thenReturn("0"); + when(jedis.scan(eq("0"), any(ScanParams.class))).thenReturn(scanResult); + when(jedis.pipelined()).thenReturn(pipeline); + when(pipeline.hgetAll("reservation:res_r1")).thenReturn(resp1); + when(pipeline.hgetAll("reservation:res_r2")).thenReturn(resp2); + when(pipeline.hgetAll("reservation:res_r3")).thenReturn(resp3); + + ReservationListResponse response = repository.listReservations( + "acme", null, null, null, null, null, null, null, 100, null, null, null, + 3000L, 7000L, 3000L, 7000L, 3000L, 7000L); + + assertThat(response.getReservations()).extracting(ReservationSummary::getReservationId) + .containsExactly("r1"); + } + + @SuppressWarnings("unchecked") + @Test + @DisplayName("cursor reused with different expires_from → 400 (FilterHasher folds expires_*)") + void cursorMismatchOnExpiresWindowChange() { + when(jedisPool.getResource()).thenReturn(jedis); + doNothing().when(jedis).close(); + + Map f1 = reservationFields("r1", "ACTIVE"); + f1.put("expires_at", "5000"); + Map f2 = reservationFields("r2", "ACTIVE"); + f2.put("expires_at", "7000"); + Response> resp1 = mock(Response.class); + Response> resp2 = mock(Response.class); + when(resp1.get()).thenReturn(f1); + when(resp2.get()).thenReturn(f2); + + ScanResult scanResult = mock(ScanResult.class); + when(scanResult.getResult()).thenReturn(List.of("reservation:res_r1", "reservation:res_r2")); + when(scanResult.getCursor()).thenReturn("0"); + when(jedis.scan(eq("0"), any(ScanParams.class))).thenReturn(scanResult); + when(jedis.pipelined()).thenReturn(pipeline); + when(pipeline.hgetAll("reservation:res_r1")).thenReturn(resp1); + when(pipeline.hgetAll("reservation:res_r2")).thenReturn(resp2); + + ReservationListResponse page1 = repository.listReservations( + "acme", null, null, null, null, null, null, null, 1, null, + "expires_at_ms", "asc", null, null, 2000L, 10000L, null, null); + String cursor = page1.getNextCursor(); + assertThat(cursor).isNotNull(); + + // Reuse cursor under a different expires_from → MUST 400. + assertThatThrownBy(() -> repository.listReservations( + "acme", null, null, null, null, null, null, null, 1, cursor, + "expires_at_ms", "asc", null, null, 9999L, 10000L, null, null)) + .isInstanceOf(io.runcycles.protocol.data.exception.CyclesProtocolException.class) + .hasMessageContaining("cursor is not valid"); + } + } } diff --git a/cycles-protocol-service/cycles-protocol-service-data/src/test/java/io/runcycles/protocol/data/repository/support/FilterHasherTest.java b/cycles-protocol-service/cycles-protocol-service-data/src/test/java/io/runcycles/protocol/data/repository/support/FilterHasherTest.java index 38fe5c8..adb6c58 100644 --- a/cycles-protocol-service/cycles-protocol-service-data/src/test/java/io/runcycles/protocol/data/repository/support/FilterHasherTest.java +++ b/cycles-protocol-service/cycles-protocol-service-data/src/test/java/io/runcycles/protocol/data/repository/support/FilterHasherTest.java @@ -11,18 +11,18 @@ class FilterHasherTest { @Test @DisplayName("same inputs produce same hash") void deterministic() { - String h1 = FilterHasher.hash("acme", null, "ACTIVE", "prod", null, null, null, null, null, null); - String h2 = FilterHasher.hash("acme", null, "ACTIVE", "prod", null, null, null, null, null, null); + String h1 = FilterHasher.hash("acme", null, "ACTIVE", "prod", null, null, null, null, null, null, null, null, null, null); + String h2 = FilterHasher.hash("acme", null, "ACTIVE", "prod", null, null, null, null, null, null, null, null, null, null); assertThat(h1).isEqualTo(h2); } @Test @DisplayName("different filter values produce different hashes") void distinguishes() { - String h1 = FilterHasher.hash("acme", null, "ACTIVE", "prod", null, null, null, null, null, null); - String h2 = FilterHasher.hash("acme", null, "COMMITTED", "prod", null, null, null, null, null, null); - String h3 = FilterHasher.hash("acme", null, "ACTIVE", "dev", null, null, null, null, null, null); - String h4 = FilterHasher.hash("other", null, "ACTIVE", "prod", null, null, null, null, null, null); + String h1 = FilterHasher.hash("acme", null, "ACTIVE", "prod", null, null, null, null, null, null, null, null, null, null); + String h2 = FilterHasher.hash("acme", null, "COMMITTED", "prod", null, null, null, null, null, null, null, null, null, null); + String h3 = FilterHasher.hash("acme", null, "ACTIVE", "dev", null, null, null, null, null, null, null, null, null, null); + String h4 = FilterHasher.hash("other", null, "ACTIVE", "prod", null, null, null, null, null, null, null, null, null, null); assertThat(h1).isNotEqualTo(h2); assertThat(h1).isNotEqualTo(h3); assertThat(h1).isNotEqualTo(h4); @@ -32,15 +32,15 @@ void distinguishes() { @DisplayName("null and empty string treated equivalently (trailing empties collapse)") void nullEmptyEquivalence() { // FilterHasher treats null as "" — both represent "no filter applied". - String hNull = FilterHasher.hash("acme", null, null, null, null, null, null, null, null, null); - String hEmpty = FilterHasher.hash("acme", "", "", "", "", "", "", "", null, null); + String hNull = FilterHasher.hash("acme", null, null, null, null, null, null, null, null, null, null, null, null, null); + String hEmpty = FilterHasher.hash("acme", "", "", "", "", "", "", "", null, null, null, null, null, null); assertThat(hNull).isEqualTo(hEmpty); } @Test @DisplayName("hash is 16 hex chars") void shape() { - String h = FilterHasher.hash("acme", null, null, null, null, null, null, null, null, null); + String h = FilterHasher.hash("acme", null, null, null, null, null, null, null, null, null, null, null, null, null); assertThat(h).hasSize(16); assertThat(h).matches("[0-9a-f]{16}"); } @@ -49,10 +49,10 @@ void shape() { @Test @DisplayName("different from/to values produce different hashes") void timeWindowDistinguishes() { - String base = FilterHasher.hash("acme", null, null, null, null, null, null, null, null, null); - String withFrom = FilterHasher.hash("acme", null, null, null, null, null, null, null, 1700000000000L, null); - String withTo = FilterHasher.hash("acme", null, null, null, null, null, null, null, null, 1700060000000L); - String withBoth = FilterHasher.hash("acme", null, null, null, null, null, null, null, 1700000000000L, 1700060000000L); + String base = FilterHasher.hash("acme", null, null, null, null, null, null, null, null, null, null, null, null, null); + String withFrom = FilterHasher.hash("acme", null, null, null, null, null, null, null, 1700000000000L, null, null, null, null, null); + String withTo = FilterHasher.hash("acme", null, null, null, null, null, null, null, null, 1700060000000L, null, null, null, null); + String withBoth = FilterHasher.hash("acme", null, null, null, null, null, null, null, 1700000000000L, 1700060000000L, null, null, null, null); assertThat(base).isNotEqualTo(withFrom); assertThat(base).isNotEqualTo(withTo); assertThat(withFrom).isNotEqualTo(withTo); @@ -64,8 +64,8 @@ void timeWindowDistinguishes() { void timeWindowPositional() { // Defensive: ensure a future refactor that flips fromMs/toMs in the canonical form // is caught. The two values are positionally distinct in the canonical string. - String forward = FilterHasher.hash("acme", null, null, null, null, null, null, null, 100L, 200L); - String swapped = FilterHasher.hash("acme", null, null, null, null, null, null, null, 200L, 100L); + String forward = FilterHasher.hash("acme", null, null, null, null, null, null, null, 100L, 200L, null, null, null, null); + String swapped = FilterHasher.hash("acme", null, null, null, null, null, null, null, 200L, 100L, null, null, null, null); assertThat(forward).isNotEqualTo(swapped); } @@ -74,13 +74,64 @@ void timeWindowPositional() { void preservesPreWindowHashWhenBothBoundsNull() { // Locks down the wire back-compat contract: a sorted-path cursor issued by // v0.1.25.12–v0.1.25.18 (which had no from/to fields in the canonical form) - // must continue to validate against v0.1.25.20 when the client never sends + // must continue to validate against v0.1.25.20+ when the client never sends // the new params. The golden value is the truncated (8-byte / 16-hex) SHA-256 // of "t=acme|i=|st=|ws=|ap=|wf=|ag=|ts=" — the pre-window canonical form. // Verified independently via: // python -c "import hashlib; print(hashlib.sha256(b't=acme|i=|st=|ws=|ap=|wf=|ag=|ts=').hexdigest()[:16])" // → 2f397ea0e8fb53b7 - String hash = FilterHasher.hash("acme", null, null, null, null, null, null, null, null, null); + String hash = FilterHasher.hash("acme", null, null, null, null, null, null, null, null, null, null, null, null, null); assertThat(hash).isEqualTo("2f397ea0e8fb53b7"); } + + // cycles-protocol revision 2026-05-22 — expires_* / finalized_* window-filter + // inclusion in hash. Independent gating: each pair only emits its canonical + // block when at least one of its bounds is set. + @Test + @DisplayName("expires_* values produce different hashes from base and from from/to") + void expiresWindowDistinguishes() { + String base = FilterHasher.hash("acme", null, null, null, null, null, null, null, null, null, null, null, null, null); + String withExpiresFrom = FilterHasher.hash("acme", null, null, null, null, null, null, null, null, null, 1700000000000L, null, null, null); + String withExpiresTo = FilterHasher.hash("acme", null, null, null, null, null, null, null, null, null, null, 1700060000000L, null, null); + String withFromAndExpires = FilterHasher.hash("acme", null, null, null, null, null, null, null, 1700000000000L, 1700060000000L, 1700000000000L, 1700060000000L, null, null); + assertThat(base).isNotEqualTo(withExpiresFrom); + assertThat(base).isNotEqualTo(withExpiresTo); + assertThat(withExpiresFrom).isNotEqualTo(withExpiresTo); + assertThat(base).isNotEqualTo(withFromAndExpires); + } + + @Test + @DisplayName("finalized_* values produce different hashes from base and from from/to and from expires_*") + void finalizedWindowDistinguishes() { + String base = FilterHasher.hash("acme", null, null, null, null, null, null, null, null, null, null, null, null, null); + String withFinalizedFrom = FilterHasher.hash("acme", null, null, null, null, null, null, null, null, null, null, null, 1700000000000L, null); + String withFinalizedTo = FilterHasher.hash("acme", null, null, null, null, null, null, null, null, null, null, null, null, 1700060000000L); + // Distinct from a from/to with same numeric values — gates emit different canonical fragments. + String withFromTo = FilterHasher.hash("acme", null, null, null, null, null, null, null, 1700000000000L, 1700060000000L, null, null, null, null); + // Distinct from an expires_* with same numeric values — different prefix in canonical form. + String withExpires = FilterHasher.hash("acme", null, null, null, null, null, null, null, null, null, 1700000000000L, 1700060000000L, null, null); + assertThat(base).isNotEqualTo(withFinalizedFrom); + assertThat(base).isNotEqualTo(withFinalizedTo); + assertThat(withFinalizedFrom).isNotEqualTo(withFinalizedTo); + assertThat(withFinalizedFrom).isNotEqualTo(withFromTo); + assertThat(withFinalizedFrom).isNotEqualTo(withExpires); + } + + @Test + @DisplayName("v0.1.25.20 back-compat: from/to set but no expires_*/finalized_* matches v0.1.25.20 canonical form") + void preservesV01_25_20HashWhenOnlyFromTo() { + // Independent gating means a sorted-path cursor issued by v0.1.25.20 + // (which had `|fr=|to=` in the canonical form but no expires_*/finalized_* + // fields) must continue to validate against v0.1.25.21+ when the client + // never sends the new pairs. The golden value is the truncated (8-byte / + // 16-hex) SHA-256 of + // "t=acme|i=|st=|ws=|ap=|wf=|ag=|ts=|fr=100|to=200" + // Verified independently via: + // python -c "import hashlib; print(hashlib.sha256(b't=acme|i=|st=|ws=|ap=|wf=|ag=|ts=|fr=100|to=200').hexdigest()[:16])" + String hash = FilterHasher.hash("acme", null, null, null, null, null, null, null, 100L, 200L, null, null, null, null); + // → ad7204d521cfd133. Locking this byte-exactly ensures a future refactor + // that accidentally drops the gating (and adds |ef=|et=|ff=|ft= even when + // all four are null) breaks this assertion. + assertThat(hash).isEqualTo("ad7204d521cfd133"); + } } diff --git a/cycles-protocol-service/cycles-protocol-service-model/src/main/java/io/runcycles/protocol/model/ReservationSummary.java b/cycles-protocol-service/cycles-protocol-service-model/src/main/java/io/runcycles/protocol/model/ReservationSummary.java index 3124956..4bd6528 100644 --- a/cycles-protocol-service/cycles-protocol-service-model/src/main/java/io/runcycles/protocol/model/ReservationSummary.java +++ b/cycles-protocol-service/cycles-protocol-service-model/src/main/java/io/runcycles/protocol/model/ReservationSummary.java @@ -19,6 +19,11 @@ public class ReservationSummary { @NotNull @Valid @JsonProperty("reserved") private Amount reserved; @NotNull @Min(0) @JsonProperty("created_at_ms") private Long createdAtMs; @NotNull @Min(0) @JsonProperty("expires_at_ms") private Long expiresAtMs; + /** Wall-clock time at which the reservation reached a terminal state. + * Populated on COMMITTED and RELEASED rows only; absent on ACTIVE and + * EXPIRED. Mirrors the same field on ReservationDetail. + * Spec: cycles-protocol-v0.yaml revision 2026-05-22. */ + @Min(0) @JsonProperty("finalized_at_ms") private Long finalizedAtMs; @NotNull @JsonProperty("scope_path") private String scopePath; @NotNull @JsonProperty("affected_scopes") private List affectedScopes; } diff --git a/cycles-protocol-service/pom.xml b/cycles-protocol-service/pom.xml index aeede7c..ebd1e72 100644 --- a/cycles-protocol-service/pom.xml +++ b/cycles-protocol-service/pom.xml @@ -18,7 +18,7 @@ cycles-protocol-service-api - 0.1.25.20 + 0.1.25.21 21 21 21 From 2608e7d5b8b63e08dbcadd9bf9a91d702fda9d08 Mon Sep 17 00:00:00 2001 From: Albert Mavashev Date: Fri, 22 May 2026 08:19:30 -0400 Subject: [PATCH 2/2] fix(listReservations): unify finalized_at resolver between predicate and projection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Real Low finding from reviewer on #163. `finalizedAtInWindow` (the new filter predicate) used a committed-wins rule when both `committed_at` and `released_at` were set on a row: `committedAt != null ? committedAt : releasedAt`. `buildReservationSummary` (the existing projection that emits the wire `finalized_at_ms`) used a last-write-wins assignment order where `released_at` overwrites `committed_at`: if (committedAtStr != null) finalizedAtMs = parseLong(committedAtStr); if (releasedAtStr != null) finalizedAtMs = parseLong(releasedAtStr); In the normal lifecycle a row has at most one of these fields (commit XOR release per the Lua scripts), so the disagreement only surfaces on malformed Redis writes. But the spec contract is that the filter operates against the returned `finalized_at_ms` — under the bug, a row with both fields would be filtered using one timestamp and returned with another. Centralized via `resolveFinalizedAtStr(fields)`: String releasedAt = fields.get("released_at"); if (releasedAt != null) return releasedAt; return fields.get("committed_at"); Both call sites now agree on released-wins, matching the pre-fix projection behavior (so no wire change) and the spec contract. Returns the raw String form so each caller can choose its own parse-failure policy: - Predicate (finalizedAtInWindow): catches NumberFormatException, excludes the row defensively when the filter is active. - Projection (buildReservationSummary): lets it propagate as a data-corruption signal (unchanged from pre-fix behavior). Regression test: finalizedResolverAgreesWhenBothFieldsSet — constructs a malformed row with committed_at=3000 and released_at=5000, queries with finalized window [4000, 6000]: - Pre-fix: committed-wins → 3000 not in window → row excluded → response empty (filter says "no match"). But the projection would have emitted 5000 if the row HAD been returned. Disagree. - Post-fix: released-wins → 5000 in window → row included. Projection also emits 5000 on the returned summary. Both paths agree. Test verifies BOTH the inclusion AND the response's finalized_at_ms == 5000. 558 protocol-service tests pass (385 data + 173 api), +1 vs the initial #163 commit. JaCoCo 95% bundle gate met. No version bump — staying at 0.1.25.21. Pure correctness fix on a same-PR finding. --- .../RedisReservationRepository.java | 48 ++++++++++++++----- .../repository/RedisReservationQueryTest.java | 46 ++++++++++++++++++ 2 files changed, 81 insertions(+), 13 deletions(-) diff --git a/cycles-protocol-service/cycles-protocol-service-data/src/main/java/io/runcycles/protocol/data/repository/RedisReservationRepository.java b/cycles-protocol-service/cycles-protocol-service-data/src/main/java/io/runcycles/protocol/data/repository/RedisReservationRepository.java index 61b01a8..11a4a8a 100644 --- a/cycles-protocol-service/cycles-protocol-service-data/src/main/java/io/runcycles/protocol/data/repository/RedisReservationRepository.java +++ b/cycles-protocol-service/cycles-protocol-service-data/src/main/java/io/runcycles/protocol/data/repository/RedisReservationRepository.java @@ -839,12 +839,7 @@ private static boolean expiresAtInWindow(Map fields, Long fromMs */ private static boolean finalizedAtInWindow(Map fields, Long fromMs, Long toMs) { if (fromMs == null && toMs == null) return true; - String committedAt = fields.get("committed_at"); - String releasedAt = fields.get("released_at"); - // Resolve the finalized timestamp the same way buildReservationSummary does. - // committed_at wins if both somehow exist (shouldn't, given the lifecycle is - // commit XOR release); released_at falls through. Both absent → row excluded. - String finalizedAtStr = committedAt != null ? committedAt : releasedAt; + String finalizedAtStr = resolveFinalizedAtStr(fields); if (finalizedAtStr == null) return false; long finalizedAt; try { @@ -857,6 +852,34 @@ private static boolean finalizedAtInWindow(Map fields, Long from return true; } + /** + * Centralized resolver for the projected {@code finalized_at_ms} timestamp, + * shared between the {@link #finalizedAtInWindow} predicate and the + * {@link #buildReservationSummary} projection. Both call sites MUST agree + * on which Redis hash field is the source of truth, otherwise a malformed + * row with both {@code committed_at} and {@code released_at} populated + * could be filtered using one timestamp and returned with another — + * violating the spec's contract that the filter operates against the + * returned {@code finalized_at_ms}. + * + *

Released-wins: {@code released_at} dominates when both are set, matching + * the projection's last-write-wins assignment order in + * {@link #buildReservationSummary}. In the normal lifecycle the two fields + * are mutually exclusive (commit XOR release), so this rule is a defensive + * tie-breaker for malformed Redis writes only. + * + *

Returns {@code null} when both fields are absent (ACTIVE / EXPIRED rows + * in the normal lifecycle). Returns the raw string form so callers can + * decide on parse-failure handling (the predicate swallows + * NumberFormatException to exclude the row from filter results; the + * projection lets it propagate as a data-corruption signal). + */ + private static String resolveFinalizedAtStr(Map fields) { + String releasedAt = fields.get("released_at"); + if (releasedAt != null) return releasedAt; + return fields.get("committed_at"); + } + private static int findSliceStart(List sorted, String sortBy, String sortDir, SortedListCursor cursor) { // Walk the sorted list looking for the first row strictly greater than the cursor's @@ -1326,13 +1349,12 @@ private ReservationDetail buildReservationSummary(Map fields) th if (chargedAmountStr != null) { committed = new Amount(unit, Long.parseLong(chargedAmountStr)); } - String committedAtStr = fields.get("committed_at"); - if (committedAtStr != null) { - finalizedAtMs = Long.parseLong(committedAtStr); - } - String releasedAtStr = fields.get("released_at"); - if (releasedAtStr != null) { - finalizedAtMs = Long.parseLong(releasedAtStr); + // Shared resolver with finalizedAtInWindow — the predicate and the + // projection MUST agree on which hash field wins when both are set. + // See resolveFinalizedAtStr javadoc for the released-wins rationale. + String finalizedAtStr = resolveFinalizedAtStr(fields); + if (finalizedAtStr != null) { + finalizedAtMs = Long.parseLong(finalizedAtStr); } // Parse metadata if present diff --git a/cycles-protocol-service/cycles-protocol-service-data/src/test/java/io/runcycles/protocol/data/repository/RedisReservationQueryTest.java b/cycles-protocol-service/cycles-protocol-service-data/src/test/java/io/runcycles/protocol/data/repository/RedisReservationQueryTest.java index 0751cad..2343d26 100644 --- a/cycles-protocol-service/cycles-protocol-service-data/src/test/java/io/runcycles/protocol/data/repository/RedisReservationQueryTest.java +++ b/cycles-protocol-service/cycles-protocol-service-data/src/test/java/io/runcycles/protocol/data/repository/RedisReservationQueryTest.java @@ -1371,6 +1371,52 @@ void threeWindowsAndCompose() { .containsExactly("r1"); } + @SuppressWarnings("unchecked") + @Test + @DisplayName("malformed row with both committed_at and released_at — predicate and projection agree (released-wins)") + void finalizedResolverAgreesWhenBothFieldsSet() { + // Defensive regression: in the normal lifecycle commit XOR release, + // so a row has at most one of committed_at / released_at. A + // malformed Redis write could produce both. Before centralizing + // resolveFinalizedAtStr, the predicate would filter using + // committed_at while the projection emitted released_at — the + // row could be filtered using one timestamp and returned with + // another, violating the spec contract that the filter operates + // against the returned finalized_at_ms. + // + // This test pins the post-fix behavior: both paths use released_at + // (last-write-wins, matching the original projection logic). + when(jedisPool.getResource()).thenReturn(jedis); + doNothing().when(jedis).close(); + + Map malformed = reservationFields("r1", "RELEASED"); + malformed.put("committed_at", "3000"); + malformed.put("released_at", "5000"); + Response> resp1 = mock(Response.class); + when(resp1.get()).thenReturn(malformed); + + ScanResult scanResult = mock(ScanResult.class); + when(scanResult.getResult()).thenReturn(List.of("reservation:res_r1")); + when(scanResult.getCursor()).thenReturn("0"); + when(jedis.scan(eq("0"), any(ScanParams.class))).thenReturn(scanResult); + when(jedis.pipelined()).thenReturn(pipeline); + when(pipeline.hgetAll("reservation:res_r1")).thenReturn(resp1); + + // Window [4000, 6000] — would match released_at=5000 but NOT committed_at=3000. + // Pre-fix the predicate would have excluded this row (committed-wins); + // post-fix it must include it (released-wins, matching projection). + ReservationListResponse response = repository.listReservations( + "acme", null, null, null, null, null, null, null, 100, null, null, null, + null, null, null, null, 4000L, 6000L); + assertThat(response.getReservations()).extracting(ReservationSummary::getReservationId) + .containsExactly("r1"); + + // And the projection emits released_at on the returned summary — + // confirms predicate and projection agree on which timestamp. + assertThat(response.getReservations().get(0).getFinalizedAtMs()) + .isEqualTo(5000L); + } + @SuppressWarnings("unchecked") @Test @DisplayName("cursor reused with different expires_from → 400 (FilterHasher folds expires_*)")