diff --git a/AUDIT.md b/AUDIT.md index 8056652..3a88477 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.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.), +**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).), +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), 2026-04-19 (v0.1.25.16 — supply-chain CVE fix; bump `spring-boot-starter-parent` 3.5.11 → 3.5.13 and pin `tomcat.version=10.1.54` to close 5 HIGH/CRITICAL CVEs flagged by the new PR-time Trivy scan — CVE-2026-22732 CRITICAL on `spring-security-web` (fixed 6.5.9, pulled in transitively by 3.5.13), CVE-2026-29129 HIGH + CVE-2026-29145 CRITICAL on `tomcat-embed-core` (fixed 10.1.53, transitive), CVE-2026-34483 HIGH + CVE-2026-34487 HIGH on `tomcat-embed-core` (fixed 10.1.54, explicit property override since Spring Boot 3.5.14 with 10.1.54+ as managed version hasn't shipped yet); no code changes, all 152 tests pass), @@ -25,6 +26,43 @@ --- +### 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. + +**Why the spec change.** The original "fetch last 24h of reservations" workflow required clients to sort by `created_at_ms` and walk pages until the trailing row fell out of the window, doubling page size on each retry. For high-volume agent clusters this scans far more rows than the caller needs. With server-side `from` / `to`, the scan is boundaried before hydration and cursor pagination over that window stays cursor-stable. + +**Naming and wire-type.** `from` / `to` with `format: date-time` matches the family-wide convention already in use on `listAuditLogs`, `listEvents`, `listWebhookDeliveries`, `listTenantEvents`, `listTenantWebhookDeliveries` in the governance-admin spec. Bespoke `created_after`/`created_before` names or Unix-epoch wire types would have split the convention for clients and codegen. + +**Sort-binding semantics.** The filter always binds to `created_at_ms`, never to the column referenced by `sort_by`. A client doing `sort_by=expires_at_ms&from=…&to=…` gets reservations *created* in the window, ordered by *expiry*. This keeps the contract predictable across sort keys — no per-key filter semantics to memorize. + +**Two execution paths.** `RedisReservationRepository.listReservations` retains its v0.1.25.12 dual-path shape (legacy SCAN-cursor when no sort params are present, full sorted-path otherwise). Both paths apply the window filter as a per-row predicate immediately after the existing scope/status/tenant predicates and before hydration. Predicate body is shared via a `createdAtInWindow(fields, fromMs, toMs)` static helper. + +**Sorted-path cursor invalidation.** `FilterHasher.hash(...)` now takes two additional `Long` arguments (`fromMs`, `toMs`). When at least one is non-null, the canonical string appends `|fr=|to=` after the existing eight string fields; when both are null, the appendix is **omitted entirely** so the canonical form reverts byte-exactly to the v0.1.25.12 8-field shape. This means a **sorted-path cursor** (the opaque cursor stored in `SortedListCursor.filterHash`, returned when `sort_by` / `sort_dir` is supplied) issued under one window returns HTTP 400 `INVALID_REQUEST` if re-used under a different one — same contract as the v0.1.25.12 `sort_by`/`sort_dir`/filter-mismatch path — while a sorted-path cursor issued by a v0.1.25.18 server (pre-window era) still resolves on v0.1.25.20 when the client never sends `from`/`to`, because the gated-emission canonical form matches byte-exactly. The legacy Redis-SCAN cursor (returned when no sort params are supplied) does not carry filter state and is **not** window-validated; this matches how the legacy path already treats `status` / `workspace` / `app` / other filters. Locked down by `FilterHasherTest.preservesPreWindowHashWhenBothBoundsNull` (golden `2f397ea0e8fb53b7`) and `RedisReservationQueryTest.cursorMismatchOnWindowChange`. + +**Validation choices.** + +- ISO-8601 parsed via `Instant.parse(...)`. Malformed values surface `DateTimeParseException` → 400 `INVALID_REQUEST` with `Invalid from: ` / `Invalid to: ` message — same shape as the existing `sort_by` / `sort_dir` rejections. +- `from > to` → 400 *before* the repository call. Detecting reversed windows after the scan would waste server work for an obviously-broken client; the controller-level guard is the right boundary. +- Blank-string values (`?from=&to=`) are treated as unset. This matches the additive-parameter intent — an omitted bound and an empty bound both mean "no bound on that side." Avoids the cryptic-400 hazard from a client sending an unconditional `?from=${maybeUnset}`. +- Equal bounds (`from == to`) are accepted as a degenerate closed point-window. Mathematically consistent with the inclusive-both-ends contract; clients chasing a single millisecond can do so without a special-case 400. + +**Defensive read-side.** The window predicate also drops rows whose `created_at` hash field is missing or unparseable, but only when at least one bound is supplied. Without this, a stale/malformed Redis row would leak past a time filter that's supposed to exclude it. With both bounds null (filter inactive), missing/unparseable rows still surface through the rest of the pipeline as they always did — the unfiltered path is unchanged. + +**Internal Java signature change, no wire impact.** `listReservations(...)` gains trailing `Long fromMs, Long toMs` (12 → 14 args). Private `listReservationsSorted(...)` mirrors. `FilterHasher.hash(...)` gains the same two trailing args (8 → 10). All Java callers updated. No wire format change — clients that omit `from`/`to` get exactly the v0.1.25.18 response byte-for-byte. + +**Coverage.** + +- `FilterHasherTest` — 3 new cases: distinct hash on from/to differences, positional distinctness (`from=100, to=200` ≠ `from=200, to=100`), and the pre-window 8-field hash back-compat golden case. +- `RedisReservationQueryTest` — 7 new cases under a `TimeWindowFilter` nested class: legacy-path `from` excludes below, legacy-path `to` excludes above, inclusive bounds (rows at `created_at = from` and `created_at = to` both kept, row at `to+1` dropped), sorted-path window with sort-by-`created_at_ms` ordering preserved, cursor mismatch on window change rejected with 400, missing `created_at` field excluded under window, unparseable `created_at` excluded under window. +- `ReservationControllerTest` — 7 new cases under the `ListReservations` nested class: malformed `from` → 400, malformed `to` → 400, `from > to` → 400 with `verify(repository, never())` to lock the pre-repository check, `from` only propagates, `to` only propagates, equal bounds accepted, combination with `sort_by=expires_at_ms` propagates correctly, blank strings treated as unset. + +All 538 protocol-service tests pass (375 data + 163 api). The 95% coverage gate per CLAUDE.md is satisfied — the only new untested branch is the equality fallthrough on `null` for both bounds, which is covered by every pre-existing `listReservations` test that doesn't pass `from`/`to`. + +**Out of scope.** The issue's rationale mentions cleanup of expired/abandoned reservations as a use case — that actually wants `expires_at` or `finalized_at` window filters, not `created_at`. Flagged on the issue thread and intentionally left as a follow-up to keep this change small and reviewable. + +--- + ### 2026-04-18 — v0.1.25.15: audit-log retention TTL (runtime-side fix) Closes a gap surfaced by the post-v0.1.25.14 alignment audit: runtime's `AuditRepository.log()` was writing `audit:log:{id}` string keys with no `EX`, so runtime-written audit rows persisted indefinitely until Redis memory-eviction kicked in. This broke the 400-day retention story the admin plane tells operators — admin's `AuditRepository` already applies tiered TTL (400d authenticated / 30d unauthenticated) via a conditional Lua `SET … EX ttl`, but runtime-written rows were silently non-compliant with that contract. The audit dashboard reads from the shared index, so stale admin ZSETs would also accumulate pointers to long-expired runtime rows without a compensating sweep. diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b2b9cd..3a1067c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,71 @@ 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.20] — 2026-05-21 + +`from` / `to` ISO-8601 time-window filter on `GET /v1/reservations`, +implementing `cycles-protocol-v0.yaml` revision 2026-05-21 and closing +[runcycles/cycles-server#159](https://github.com/runcycles/cycles-server/issues/159). + +### Added + +- **`from` and `to` query parameters on `listReservations`** — both + `string` `format: date-time` (ISO 8601), both optional, both + inclusive bounds on the reservation's `created_at_ms`. The filter + is fixed to `created_at_ms` regardless of `sort_by`, so + `sort_by=expires_at_ms&from=…&to=…` returns reservations *created* + in the window, ordered by *expiry*. Implemented in both the legacy + SCAN-cursor path and the sorted path. +- **Sorted-path cursor invalidation on window change.** + `FilterHasher.hash(...)` now folds `fromMs` and `toMs` into the + canonical hash that's embedded in the **sorted-path cursor** + (the opaque cursor returned when `sort_by` or `sort_dir` is + supplied, or when resuming a sorted cursor from a prior page), + so reusing such a cursor under a different `(from, to)` returns + HTTP 400 INVALID_REQUEST — same contract as the v0.1.25.12 + `sort_by` / `sort_dir` / subject-filter mismatch path. The + legacy Redis-SCAN cursor (returned when no sort params are + supplied) is unchanged and does not carry filter state; clients + paginating with `from` / `to` but no `sort_by` must keep their + window stable across pages, matching how the legacy path + already treats every other filter. + +### Validation + +- Malformed `from` or `to` (anything that `Instant.parse` rejects) → + HTTP 400 INVALID_REQUEST with message `Invalid from: …` or + `Invalid to: …`. +- `from > to` → HTTP 400 INVALID_REQUEST before any repository call, + with message `from must be less than or equal to to`. Equal bounds + (closed point window) are valid. +- Blank-string values for either parameter are treated as unset (no + 400). Matches the additive-parameter intent: an omitted bound and + an empty-string bound both mean "no bound on that side." +- Defensive: rows whose `created_at` hash field is missing or + unparseable are excluded when either bound is supplied. Malformed + storage rows cannot leak past a time filter. + +### Internal + +- `RedisReservationRepository.listReservations(...)` signature gains + trailing `Long fromMs, Long toMs` (14 args total). Same shape on the + private `listReservationsSorted(...)` helper. Internal Java signature + change only — wire format is purely additive, all v0.1.25.x clients + that don't send `from`/`to` continue to work byte-for-byte. + +### Coverage + +- 538 tests across the protocol-service modules pass (375 data + 163 + api). New tests: + - `FilterHasherTest`: from/to inclusion, positional distinctness. + - `RedisReservationQueryTest`: 7 new cases covering legacy-path + from/to, sorted-path from/to, inclusive-bound semantics, + cursor-mismatch-on-window-change, and missing/unparseable + `created_at` defensive exclusion. + - `ReservationControllerTest`: 7 new cases covering malformed-from, + malformed-to, reversed-window, from-only, to-only, equal-bounds, + combination with `sort_by=expires_at_ms`, blank-string handling. + ## [0.1.25.19] — 2026-05-21 Supply-chain CVE patch. No code, API, or Lua-script changes — pom-only. diff --git a/cycles-protocol-service/README.md b/cycles-protocol-service/README.md index 575413a..478daf9 100644 --- a/cycles-protocol-service/README.md +++ b/cycles-protocol-service/README.md @@ -459,12 +459,18 @@ List reservations for the effective tenant. Optional recovery/debug endpoint. Re | `cursor` | Opaque pagination cursor from previous response | | `sort_by` | One of `reservation_id`, `tenant`, `scope_path`, `status`, `reserved`, `created_at_ms`, `expires_at_ms` (v0.1.25.12+). Omit for legacy unordered behaviour. | | `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`. | When `sort_by` or `sort_dir` is provided, the cursor encodes the -`(sort_by, sort_dir, filters)` tuple — reusing a cursor after -changing the sort key, direction, or any filter returns `400 -INVALID_REQUEST`. When both are omitted, the legacy Redis-SCAN -cursor path is used and existing clients are unaffected. +`(sort_by, sort_dir, filters)` tuple — reusing a sorted cursor after +changing the sort key, direction, or any filter (including `from` / +`to`) returns `400 INVALID_REQUEST`. When both are omitted, the +legacy Redis-SCAN cursor path is used and existing clients are +unaffected; the legacy cursor does **not** carry filter state, so +callers paginating with `from` / `to` but no `sort_by` must keep +their window stable across pages (same convention as `status` / +`workspace` / other filters on the legacy path). **Response** `200 OK` ```json 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 6ef1b3b..f7e933f 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 @@ -9,6 +9,8 @@ import io.runcycles.protocol.model.audit.AuditLogEntry; import io.runcycles.protocol.model.event.*; +import java.time.Instant; +import java.time.format.DateTimeParseException; import java.util.Map; import io.swagger.v3.oas.annotations.*; import io.swagger.v3.oas.annotations.tags.Tag; @@ -248,7 +250,9 @@ public ResponseEntity list( @RequestParam(defaultValue = "50") @Min(1) @Max(200) int limit, @RequestParam(required = false) String cursor, @RequestParam(value = "sort_by", required = false) String sortBy, - @RequestParam(value = "sort_dir", required = false) String sortDir) { + @RequestParam(value = "sort_dir", required = false) String sortDir, + @RequestParam(required = false) String from, + @RequestParam(value = "to", required = false) String toParam) { // Validate status against ReservationStatus enum if provided if (status != null) { try { @@ -281,6 +285,17 @@ public ResponseEntity list( "Invalid sort_dir: " + sortDir + ". Must be one of: asc, desc", 400); } } + // cycles-protocol revision 2026-05-21: from/to ISO-8601 date-time + // window filter on listReservations. Both are optional and additive; + // each is parsed independently. The filter binds to created_at_ms + // regardless of sort_by per spec. Malformed values → 400; from > to + // → 400 before any repository work. + Long fromMs = parseIsoToEpochMs(from, "from"); + Long toMs = parseIsoToEpochMs(toParam, "to"); + if (fromMs != null && toMs != null && fromMs > toMs) { + throw new CyclesProtocolException(Enums.ErrorCode.INVALID_REQUEST, + "from must be less than or equal to to (received from=" + from + ", to=" + toParam + ")", 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. @@ -301,9 +316,20 @@ public ResponseEntity list( effectiveTenant = tenant != null ? tenant : extractAuthTenantId(); authorizeTenant(effectiveTenant); } - LOG.info("GET /v1/reservations - tenant: {}, admin: {}, sort_by: {}, sort_dir: {}", - effectiveTenant, isAdminAuth(), sortBy, sortDir); + LOG.info("GET /v1/reservations - tenant: {}, admin: {}, sort_by: {}, sort_dir: {}, from: {}, to: {}", + effectiveTenant, isAdminAuth(), sortBy, sortDir, fromMs, toMs); return ResponseEntity.ok(repository.listReservations(effectiveTenant, idempotencyKey, - status, workspace, app, workflow, agent, toolset, limit, cursor, sortBy, sortDir)); + status, workspace, app, workflow, agent, toolset, limit, cursor, sortBy, sortDir, + fromMs, toMs)); + } + + private Long parseIsoToEpochMs(String raw, String paramName) { + if (raw == null || raw.isBlank()) return null; + try { + return Instant.parse(raw).toEpochMilli(); + } catch (DateTimeParseException e) { + throw new CyclesProtocolException(Enums.ErrorCode.INVALID_REQUEST, + "Invalid " + paramName + ": " + raw + " is not a valid ISO 8601 date-time", 400); + } } } 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 58cf3ed..82478ff 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 @@ -29,6 +29,7 @@ import java.util.UUID; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.when; import static org.mockito.Mockito.verify; @@ -498,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())) + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations")) @@ -517,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())) + when(repository.listReservations(eq(TENANT), any(), eq("ACTIVE"), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations").param("status", "ACTIVE")) @@ -529,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())) + when(repository.listReservations(eq(TENANT), any(), eq("COMMITTED"), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations").param("status", "COMMITTED")) @@ -540,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())) + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations").param("tenant", TENANT)) @@ -558,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())) + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any())) .thenReturn(resp); // No tenant param — should use auth tenant @@ -594,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"))) + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), eq("status"), eq("asc"), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations") .param("sort_by", "status") @@ -607,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())) + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), 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"}) { @@ -615,6 +616,129 @@ void shouldAcceptAllSpecSortByValues() throws Exception { .andExpect(status().isOk()); } } + + // cycles-protocol revision 2026-05-21: from/to ISO-8601 window filter on + // listReservations. Controller parses ISO-8601 and validates from<=to; + // malformed values and reversed bounds MUST return 400 INVALID_REQUEST. + @Test + @DisplayName("from=bogus → 400 INVALID_REQUEST") + void shouldRejectMalformedFrom() throws Exception { + mockMvc.perform(get("/v1/reservations").param("from", "not-a-date")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("INVALID_REQUEST")) + .andExpect(jsonPath("$.message").value( + org.hamcrest.Matchers.containsString("Invalid from"))); + } + + @Test + @DisplayName("to=bogus → 400 INVALID_REQUEST") + void shouldRejectMalformedTo() throws Exception { + mockMvc.perform(get("/v1/reservations").param("to", "2026-13-45T99:99:99Z")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("INVALID_REQUEST")) + .andExpect(jsonPath("$.message").value( + org.hamcrest.Matchers.containsString("Invalid to"))); + } + + @Test + @DisplayName("from > to → 400 INVALID_REQUEST before repository is called") + void shouldRejectReversedWindow() throws Exception { + mockMvc.perform(get("/v1/reservations") + .param("from", "2026-05-21T12:00:00Z") + .param("to", "2026-05-21T11:00:00Z")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("INVALID_REQUEST")) + .andExpect(jsonPath("$.message").value( + org.hamcrest.Matchers.containsString("from must be less than or equal to to"))); + // 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()); + } + + // Derive the expected epoch-ms from the same parser the controller uses so a + // hand-typed literal can't drift again (prior commit hard-coded the wrong value + // for 2026-05-21T12:00:00Z, causing eq(...) to miss; Mockito then returned null; + // ResponseEntity.ok(null) still emits HTTP 200, so the tests passed vacuously). + private static final String SAMPLE_FROM_TO_INSTANT = "2026-05-21T12:00:00Z"; + private static final long SAMPLE_FROM_TO_EPOCH_MS = + java.time.Instant.parse(SAMPLE_FROM_TO_INSTANT).toEpochMilli(); + + @Test + @DisplayName("from only → epoch-ms propagates to repository") + 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))) + .thenReturn(resp); + mockMvc.perform(get("/v1/reservations").param("from", SAMPLE_FROM_TO_INSTANT)) + .andExpect(status().isOk()); + // Lock in that the stub fired — guards against the prior failure mode where + // 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)); + } + + @Test + @DisplayName("to only → epoch-ms propagates to repository") + 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))) + .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)); + } + + @Test + @DisplayName("from = to (equal bounds, closed point window) → 200, both propagate") + 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))) + .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)); + } + + @Test + @DisplayName("from/to combine with sort_by=expires_at_ms — filter binds to created_at, sort is independent") + 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())) + .thenReturn(resp); + mockMvc.perform(get("/v1/reservations") + .param("sort_by", "expires_at_ms") + .param("from", "2026-05-21T00:00:00Z") + .param("to", "2026-05-22T00:00:00Z")) + .andExpect(status().isOk()); + } + + @Test + @DisplayName("from/to blank strings treated as unset (no 400, no filter)") + 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))) + .thenReturn(resp); + mockMvc.perform(get("/v1/reservations") + .param("from", "") + .param("to", "")) + .andExpect(status().isOk()); + } } // v0.1.25.8 (cycles-protocol revision 2026-04-13): admin-on-behalf-of @@ -633,7 +757,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())) + when(repository.listReservations(eq("any-tenant"), any(), any(), any(), any(), any(), any(), any(), eq(50), 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 16a7508..6c82a47 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 @@ -553,7 +553,8 @@ public ReservationListResponse listReservations(String tenant, String idempotenc String status, String workspace, String app, String workflow, String agent, String toolset, int limit, String startCursor, - String sortBy, String sortDir) { + String sortBy, String sortDir, + Long fromMs, Long toMs) { boolean sortRequested = sortBy != null || sortDir != null; Optional parsedCursor = SortedListCursor.decode(startCursor); @@ -562,7 +563,8 @@ public ReservationListResponse listReservations(String tenant, String idempotenc // expecting the cursor to carry the sort state — we honour that). if (sortRequested || parsedCursor.isPresent()) { return listReservationsSorted(tenant, idempotencyKey, status, workspace, app, - workflow, agent, toolset, limit, parsedCursor.orElse(null), sortBy, sortDir); + workflow, agent, toolset, limit, parsedCursor.orElse(null), sortBy, sortDir, + fromMs, toMs); } try (Jedis jedis = jedisPool.getResource()) { @@ -602,6 +604,7 @@ public ReservationListResponse listReservations(String tenant, String idempotenc if (workflowSegment != null && !scopeHasSegment(scopePath, workflowSegment)) continue; if (agentSegment != null && !scopeHasSegment(scopePath, agentSegment)) continue; if (toolsetSegment != null && !scopeHasSegment(scopePath, toolsetSegment)) continue; + if (!createdAtInWindow(fields, fromMs, toMs)) continue; result.add(toSummary(buildReservationSummary(fields))); @@ -640,7 +643,8 @@ public ReservationListResponse listReservations(String tenant, String idempotenc 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) { + int limit, SortedListCursor resumeCursor, String sortBy, String sortDir, + Long fromMs, Long toMs) { // 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 @@ -651,7 +655,7 @@ private ReservationListResponse listReservationsSorted( : (resumeCursor != null ? resumeCursor.getSortDir() : "desc"); String filterHash = FilterHasher.hash(tenant, idempotencyKey, status, - workspace, app, workflow, agent, toolset); + workspace, app, workflow, agent, toolset, fromMs, toMs); // Spec: cursor is valid only for the same (sort_by, sort_dir, filters) tuple. if (resumeCursor != null) { @@ -702,6 +706,7 @@ private ReservationListResponse listReservationsSorted( if (workflowSegment != null && !scopeHasSegment(scopePath, workflowSegment)) continue; if (agentSegment != null && !scopeHasSegment(scopePath, agentSegment)) continue; if (toolsetSegment != null && !scopeHasSegment(scopePath, toolsetSegment)) continue; + if (!createdAtInWindow(fields, fromMs, toMs)) continue; matching.add(toSummary(buildReservationSummary(fields))); } catch (Exception e) { @@ -754,6 +759,32 @@ private ReservationListResponse listReservationsSorted( } } + /** + * Inclusive time-window predicate for listReservations from/to filters + * (cycles-protocol-v0.yaml revision 2026-05-21). Reads the per-reservation + * {@code created_at} hash field (stored as epoch-ms decimal string) and + * returns true iff the row is inside the requested window. A row with + * missing or unparseable {@code created_at} is treated as out-of-window + * when EITHER bound is supplied — leaking malformed-write rows past a + * time filter would silently break the contract. + * + *

Returns true when both bounds are null (filter inactive). + */ + private static boolean createdAtInWindow(Map fields, Long fromMs, Long toMs) { + if (fromMs == null && toMs == null) return true; + String createdAtStr = fields.get("created_at"); + if (createdAtStr == null) return false; + long createdAt; + try { + createdAt = Long.parseLong(createdAtStr); + } catch (NumberFormatException e) { + return false; + } + if (fromMs != null && createdAt < fromMs) return false; + if (toMs != null && createdAt > 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 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 5aa242d..ab690b6 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 @@ -21,7 +21,8 @@ private FilterHasher() {} public static String hash(String tenant, String idempotencyKey, String status, String workspace, String app, String workflow, - String agent, String toolset) { + String agent, String toolset, + Long fromMs, Long toMs) { StringBuilder canonical = new StringBuilder(256); canonical.append("t=").append(nullSafe(tenant)).append('|'); canonical.append("i=").append(nullSafe(idempotencyKey)).append('|'); @@ -31,6 +32,18 @@ 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. + if (fromMs != null || toMs != null) { + canonical.append('|'); + canonical.append("fr=").append(nullSafeLong(fromMs)).append('|'); + canonical.append("to=").append(nullSafeLong(toMs)); + } try { MessageDigest md = MessageDigest.getInstance("SHA-256"); byte[] digest = md.digest(canonical.toString().getBytes(StandardCharsets.UTF_8)); @@ -47,4 +60,8 @@ public static String hash(String tenant, String idempotencyKey, String status, private static String nullSafe(String s) { return s == null ? "" : s; } + + private static String nullSafeLong(Long v) { + return v == null ? "" : Long.toString(v); + } } 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 4e4592b..98f7eee 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); + "acme", null, null, null, null, null, null, null, 100, 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); + "acme", null, "ACTIVE", null, null, null, null, null, 100, 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); + "acme", null, null, null, null, null, null, null, 100, 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); + "acme", null, null, "dev", null, null, null, null, 100, 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); + "acme", null, null, null, "myapp", null, null, null, 100, 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); + "acme", null, null, null, null, null, null, null, 1, 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); + "acme", null, "COMMITTED", null, null, null, null, null, 100, 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); + "acme", "idem-abc", null, null, null, null, null, null, 100, 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); + "acme", "idem-nonexistent", null, null, null, null, null, null, 100, 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); + "acme", null, null, null, null, null, null, null, 100, 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"); + "created_at_ms", "asc", 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"); + "created_at_ms", "asc", 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"); + "created_at_ms", "asc", 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"); + "created_at_ms", "asc", 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")) + "status", "asc", 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"); + "created_at_ms", "asc", null, null); assertThat(response.getReservations()).hasSize(5); assertThat(response.getReservations()) @@ -905,10 +905,280 @@ 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); + "acme", null, null, null, null, null, null, null, 100, "42", null, null, null, null); assertThat(response.getReservations()).isEmpty(); verify(jedis).scan(eq("42"), any(ScanParams.class)); } } + + // cycles-protocol revision 2026-05-21 — from/to inclusive window filter on + // listReservations. Verifies the predicate is applied in both the legacy + // SCAN-cursor path and the sorted path. The filter is fixed to created_at_ms + // regardless of sort_by, and missing/unparseable created_at rows are + // excluded when EITHER bound is supplied (defensive against malformed writes). + @Nested + @DisplayName("listReservations — from/to time window") + class TimeWindowFilter { + + @SuppressWarnings("unchecked") + @Test + @DisplayName("legacy path: from drops rows with created_at < from") + void legacyPathFromExcludesBelow() { + when(jedisPool.getResource()).thenReturn(jedis); + doNothing().when(jedis).close(); + + Map before = reservationFields("r1", "ACTIVE"); + before.put("created_at", "1000"); + Map inside = reservationFields("r2", "ACTIVE"); + inside.put("created_at", "5000"); + + Response> resp1 = mock(Response.class); + Response> resp2 = mock(Response.class); + when(resp1.get()).thenReturn(before); + when(resp2.get()).thenReturn(inside); + + 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, + 3000L, null); + + assertThat(response.getReservations()).extracting(ReservationSummary::getReservationId) + .containsExactly("r2"); + } + + @SuppressWarnings("unchecked") + @Test + @DisplayName("legacy path: to drops rows with created_at > to") + void legacyPathToExcludesAbove() { + when(jedisPool.getResource()).thenReturn(jedis); + doNothing().when(jedis).close(); + + Map inside = reservationFields("r1", "ACTIVE"); + inside.put("created_at", "5000"); + Map after = reservationFields("r2", "ACTIVE"); + after.put("created_at", "9000"); + + Response> resp1 = mock(Response.class); + Response> resp2 = mock(Response.class); + when(resp1.get()).thenReturn(inside); + when(resp2.get()).thenReturn(after); + + 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, 7000L); + + assertThat(response.getReservations()).extracting(ReservationSummary::getReservationId) + .containsExactly("r1"); + } + + @SuppressWarnings("unchecked") + @Test + @DisplayName("legacy path: closed window keeps rows on both bounds") + void legacyPathInclusiveBounds() { + when(jedisPool.getResource()).thenReturn(jedis); + doNothing().when(jedis).close(); + + // Row at exactly from-boundary and at exactly to-boundary must both be included. + Map onFrom = reservationFields("r1", "ACTIVE"); + onFrom.put("created_at", "3000"); + Map middle = reservationFields("r2", "ACTIVE"); + middle.put("created_at", "5000"); + Map onTo = reservationFields("r3", "ACTIVE"); + onTo.put("created_at", "7000"); + Map outside = reservationFields("r4", "ACTIVE"); + outside.put("created_at", "7001"); + + Response> resp1 = mock(Response.class); + Response> resp2 = mock(Response.class); + Response> resp3 = mock(Response.class); + Response> resp4 = mock(Response.class); + when(resp1.get()).thenReturn(onFrom); + when(resp2.get()).thenReturn(middle); + when(resp3.get()).thenReturn(onTo); + when(resp4.get()).thenReturn(outside); + + ScanResult scanResult = mock(ScanResult.class); + when(scanResult.getResult()).thenReturn(List.of( + "reservation:res_r1", "reservation:res_r2", + "reservation:res_r3", "reservation:res_r4")); + 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); + when(pipeline.hgetAll("reservation:res_r4")).thenReturn(resp4); + + ReservationListResponse response = repository.listReservations( + "acme", null, null, null, null, null, null, null, 100, null, null, null, + 3000L, 7000L); + + assertThat(response.getReservations()).extracting(ReservationSummary::getReservationId) + .containsExactlyInAnyOrder("r1", "r2", "r3"); + } + + @SuppressWarnings("unchecked") + @Test + @DisplayName("sorted path: from/to applied before in-memory sort, ordering preserved") + void sortedPathHonoursWindow() { + when(jedisPool.getResource()).thenReturn(jedis); + doNothing().when(jedis).close(); + + // r1 outside window, r2/r3 inside. Sort asc by created_at_ms should yield [r2, r3]. + Map outside = reservationFields("r1", "ACTIVE"); + outside.put("created_at", "100"); + Map in1 = reservationFields("r2", "ACTIVE"); + in1.put("created_at", "500"); + Map in2 = reservationFields("r3", "ACTIVE"); + in2.put("created_at", "700"); + + Response> resp1 = mock(Response.class); + Response> resp2 = mock(Response.class); + Response> resp3 = mock(Response.class); + when(resp1.get()).thenReturn(outside); + when(resp2.get()).thenReturn(in1); + when(resp3.get()).thenReturn(in2); + + 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, + "created_at_ms", "asc", 200L, 1000L); + + assertThat(response.getReservations()).extracting(ReservationSummary::getReservationId) + .containsExactly("r2", "r3"); + } + + @SuppressWarnings("unchecked") + @Test + @DisplayName("cursor reused with different from/to → 400 (filter hash differs)") + void cursorMismatchOnWindowChange() { + when(jedisPool.getResource()).thenReturn(jedis); + doNothing().when(jedis).close(); + + Map f1 = reservationFields("r1", "ACTIVE"); + f1.put("created_at", "500"); + Map f2 = reservationFields("r2", "ACTIVE"); + f2.put("created_at", "700"); + 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, + "created_at_ms", "asc", 200L, 1000L); + String cursor = page1.getNextCursor(); + assertThat(cursor).isNotNull(); + + // Re-use cursor with a different from → MUST 400 (FilterHasher includes + // 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)) + .isInstanceOf(io.runcycles.protocol.data.exception.CyclesProtocolException.class) + .hasMessageContaining("cursor is not valid"); + } + + @SuppressWarnings("unchecked") + @Test + @DisplayName("legacy path: missing created_at field excluded when bound supplied") + void missingCreatedAtExcludedWithBound() { + when(jedisPool.getResource()).thenReturn(jedis); + doNothing().when(jedis).close(); + + // Defensive: a malformed-write row missing created_at MUST NOT leak past + // the time filter. createdAtInWindow returns false for missing field + // when EITHER bound is supplied. + Map ok = reservationFields("r1", "ACTIVE"); + ok.put("created_at", "5000"); + Map broken = reservationFields("r2", "ACTIVE"); + broken.remove("created_at"); + + Response> resp1 = mock(Response.class); + Response> resp2 = mock(Response.class); + when(resp1.get()).thenReturn(ok); + when(resp2.get()).thenReturn(broken); + + 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, + 1000L, null); + + assertThat(response.getReservations()).extracting(ReservationSummary::getReservationId) + .containsExactly("r1"); + } + + @SuppressWarnings("unchecked") + @Test + @DisplayName("legacy path: unparseable created_at excluded when bound supplied") + void unparseableCreatedAtExcludedWithBound() { + when(jedisPool.getResource()).thenReturn(jedis); + doNothing().when(jedis).close(); + + Map ok = reservationFields("r1", "ACTIVE"); + ok.put("created_at", "5000"); + Map garbage = reservationFields("r2", "ACTIVE"); + garbage.put("created_at", "not-a-number"); + + Response> resp1 = mock(Response.class); + Response> resp2 = mock(Response.class); + when(resp1.get()).thenReturn(ok); + when(resp2.get()).thenReturn(garbage); + + 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, 10000L); + + assertThat(response.getReservations()).extracting(ReservationSummary::getReservationId) + .containsExactly("r1"); + } + } } 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 94282ce..38fe5c8 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); - String h2 = FilterHasher.hash("acme", null, "ACTIVE", "prod", null, null, null, null); + 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); 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); - String h2 = FilterHasher.hash("acme", null, "COMMITTED", "prod", null, null, null, null); - String h3 = FilterHasher.hash("acme", null, "ACTIVE", "dev", null, null, null, null); - String h4 = FilterHasher.hash("other", null, "ACTIVE", "prod", null, null, null, null); + 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); assertThat(h1).isNotEqualTo(h2); assertThat(h1).isNotEqualTo(h3); assertThat(h1).isNotEqualTo(h4); @@ -32,16 +32,55 @@ 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); - String hEmpty = FilterHasher.hash("acme", "", "", "", "", "", "", ""); + String hNull = FilterHasher.hash("acme", null, null, null, null, null, null, null, null, null); + String hEmpty = FilterHasher.hash("acme", "", "", "", "", "", "", "", 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); + String h = FilterHasher.hash("acme", null, null, null, null, null, null, null, null, null); assertThat(h).hasSize(16); assertThat(h).matches("[0-9a-f]{16}"); } + + // cycles-protocol revision 2026-05-21 — from/to window filter inclusion in hash. + @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); + assertThat(base).isNotEqualTo(withFrom); + assertThat(base).isNotEqualTo(withTo); + assertThat(withFrom).isNotEqualTo(withTo); + assertThat(withFrom).isNotEqualTo(withBoth); + } + + @Test + @DisplayName("from/to are positional — swapping yields a different hash") + 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); + assertThat(forward).isNotEqualTo(swapped); + } + + @Test + @DisplayName("v0.1.25.18 back-compat: null from/to produces the pre-window 8-field hash byte-exactly") + 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 + // 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); + assertThat(hash).isEqualTo("2f397ea0e8fb53b7"); + } } diff --git a/cycles-protocol-service/pom.xml b/cycles-protocol-service/pom.xml index 895d272..aeede7c 100644 --- a/cycles-protocol-service/pom.xml +++ b/cycles-protocol-service/pom.xml @@ -18,7 +18,7 @@ cycles-protocol-service-api - 0.1.25.19 + 0.1.25.20 21 21 21