From c7d367b0331633b23f1a70e6da18abee266227b3 Mon Sep 17 00:00:00 2001 From: Albert Mavashev Date: Thu, 21 May 2026 10:44:31 -0400 Subject: [PATCH 1/5] feat(listReservations): add from/to time-window filter (v0.1.25.19) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #159. Implements cycles-protocol-v0.yaml revision 2026-05-21 (runcycles/cycles-protocol#97). Two new optional query parameters on GET /v1/reservations: * from: ISO 8601 date-time. Inclusive lower bound on created_at_ms. * to: ISO 8601 date-time. Inclusive upper bound on created_at_ms. Either may be supplied alone (open interval) or together (closed window). The filter always binds to created_at_ms regardless of sort_by, so sort-by-expires_at_ms + window-by-created_at_ms is well-defined. Implementation: * Controller: parses ISO-8601 via Instant.parse; malformed → 400 INVALID_REQUEST; from > to → 400 before any repository call; blank-string values treated as unset. * Repository: window predicate applied in both legacy SCAN-cursor and sorted paths. Predicate body shared via createdAtInWindow helper. Missing/unparseable created_at rows defensively excluded when either bound is supplied. * FilterHasher: fromMs/toMs folded into canonical hash so cursors invalidate on window change (same contract as v0.1.25.12 sort_by/sort_dir/filter-mismatch). * Java signature change: listReservations gains trailing Long fromMs, Long toMs (12 → 14 args). No wire change for clients that omit the new params. Coverage: * FilterHasherTest: 2 new cases (from/to distinct hash, positional). * RedisReservationQueryTest: 7 new cases under TimeWindowFilter nested class. * ReservationControllerTest: 7 new cases under ListReservations nested class (malformed-from/to, reversed-window, from-only, to-only, equal-bounds, sort-key independence, blank-as-unset). * 537 tests pass (374 data + 163 api). Docs: * AUDIT.md: new dated entry walks through naming, sort-binding, cursor invalidation, validation choices, defensive read-side, coverage, and out-of-scope notes (expires_at follow-up). * CHANGELOG.md: [0.1.25.19] entry under Keep-a-Changelog format. * pom.xml revision bumped 0.1.25.18 → 0.1.25.19. --- AUDIT.md | 40 ++- CHANGELOG.md | 57 ++++ .../api/controller/ReservationController.java | 34 +- .../controller/ReservationControllerTest.java | 127 +++++++- .../RedisReservationRepository.java | 39 ++- .../data/repository/support/FilterHasher.java | 11 +- .../repository/RedisReservationQueryTest.java | 304 +++++++++++++++++- .../repository/support/FilterHasherTest.java | 42 ++- cycles-protocol-service/pom.xml | 2 +- 9 files changed, 610 insertions(+), 46 deletions(-) diff --git a/AUDIT.md b/AUDIT.md index 8056652..ccd1114 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 cursors invalidate on window change. 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. 537 tests pass (374 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. + +**Cursor invalidation.** `FilterHasher.hash(...)` now takes two additional `Long` arguments (`fromMs`, `toMs`) and folds them into the canonical string after the existing eight string fields. A cursor 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. `null` values render as empty in the canonical form, so an `(unset, unset)` cursor still matches a follow-up call that omits both params (back-compat for clients that paginate without ever sending the window). + +**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` — 2 new cases: distinct hash on from/to differences, positional distinctness (`from=100, to=200` ≠ `from=200, to=100`). +- `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 537 protocol-service tests pass (374 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..d46adbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,63 @@ 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. +- **Cursor invalidation on window change.** `FilterHasher.hash(...)` + now folds `fromMs` and `toMs` into the canonical hash, so a cursor + issued under one `(from, to)` returns HTTP 400 INVALID_REQUEST if + re-used under a different window — same contract as `sort_by` / + `sort_dir` / subject-filter mismatches (v0.1.25.12). + +### 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 + +- 537 tests across the protocol-service modules pass (374 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/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..c54a4bd 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,116 @@ 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()); + } + + @Test + @DisplayName("from only → epoch-ms propagates to repository") + void shouldPropagateFromOnly() throws Exception { + ReservationListResponse resp = ReservationListResponse.builder() + .reservations(Collections.emptyList()).hasMore(false).build(); + // 2026-05-21T12:00:00Z = epoch ms 1779710400000 + long expectedFromMs = 1779710400000L; + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), + eq(50), any(), any(), any(), eq(expectedFromMs), eq((Long) null))) + .thenReturn(resp); + mockMvc.perform(get("/v1/reservations").param("from", "2026-05-21T12:00:00Z")) + .andExpect(status().isOk()); + } + + @Test + @DisplayName("to only → epoch-ms propagates to repository") + void shouldPropagateToOnly() throws Exception { + ReservationListResponse resp = ReservationListResponse.builder() + .reservations(Collections.emptyList()).hasMore(false).build(); + long expectedToMs = 1779710400000L; + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), + eq(50), any(), any(), any(), eq((Long) null), eq(expectedToMs))) + .thenReturn(resp); + mockMvc.perform(get("/v1/reservations").param("to", "2026-05-21T12:00:00Z")) + .andExpect(status().isOk()); + } + + @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(); + long expectedMs = 1779710400000L; + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), + eq(50), any(), any(), any(), eq(expectedMs), eq(expectedMs))) + .thenReturn(resp); + mockMvc.perform(get("/v1/reservations") + .param("from", "2026-05-21T12:00:00Z") + .param("to", "2026-05-21T12:00:00Z")) + .andExpect(status().isOk()); + } + + @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 +744,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..9aa8196 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('|'); @@ -30,7 +31,9 @@ public static String hash(String tenant, String idempotencyKey, String status, canonical.append("ap=").append(nullSafe(app)).append('|'); canonical.append("wf=").append(nullSafe(workflow)).append('|'); canonical.append("ag=").append(nullSafe(agent)).append('|'); - canonical.append("ts=").append(nullSafe(toolset)); + canonical.append("ts=").append(nullSafe(toolset)).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 +50,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..d91c42f 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,40 @@ 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); + } } 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 From 4b122f7d714f1d7116eed872f988d5bae20cc7b2 Mon Sep 17 00:00:00 2001 From: Albert Mavashev Date: Thu, 21 May 2026 11:41:24 -0400 Subject: [PATCH 2/5] fix(listReservations): address PR #160 code-review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two real bugs found during the post-push code-review pass against PR #160: 1. FilterHasher.hash unconditionally appended `|fr=...|to=...` to the canonical string, even when both bounds were null. This changed the SHA-256 hash for every pre-window cursor, including any v0.1.25.18 sorted-path cursor mid-pagination across the deployment boundary — the server would reject those cursors with 400 INVALID_REQUEST despite the client never sending the new params. That falsified the PR's stated "byte-for-byte" back-compat claim for the sorted path. Fix: gate the fr=/to= emission behind `fromMs != null || toMs != null`. When both bounds are null the canonical form reverts byte-exactly to the v0.1.25.12 8-field shape, so any pre-window cursor continues to resolve. When either bound is supplied, the new fields render in the canonical string as before, so cursors still invalidate on window change. Adds an explicit regression test `preservesPreWindowHashWhenBothBoundsNull` that locks the truncated 8-byte SHA-256 to its v0.1.25.18 golden value (2f397ea0e8fb53b7). 2. Three new ReservationControllerTest cases (shouldPropagateFromOnly / shouldPropagateToOnly / shouldAcceptEqualBounds) hard-coded 1779710400000L as the epoch-ms for "2026-05-21T12:00:00Z", but the correct value is 1779364800000L (the literal was off by 4 days — it's actually 2026-05-25T12:00:00Z). Mockito's eq(...) never matched the controller's computed value, the stub silently returned null, ResponseEntity.ok(null) still produced HTTP 200, and the tests passed vacuously without asserting anything about epoch-ms propagation. Fix: drop the literal entirely. Derive the expected ms from the same parser the controller uses (java.time.Instant.parse(...).toEpochMilli()). Each test also gains a verify(repository).listReservations(... eq(...) ...) assertion so a future Mockito miss raises a wanted-but-not-invoked failure instead of slipping through as a vacuous 200. No production behavior change beyond restoring the back-compat invariant for v0.1.25.18 sorted-path cursors. All 538 protocol-service tests pass (375 data + 163 api). JaCoCo 95% bundle gate met. Also folds in the .19 → .20 revision bump from the rebase onto main post-#161 (Tomcat CVE hygiene), so the PR's cumulative version is now v0.1.25.20. --- .../controller/ReservationControllerTest.java | 35 +++++++++++++------ .../data/repository/support/FilterHasher.java | 16 +++++++-- .../repository/support/FilterHasherTest.java | 15 ++++++++ 3 files changed, 52 insertions(+), 14 deletions(-) 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 c54a4bd..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 @@ -656,18 +656,29 @@ void shouldRejectReversedWindow() throws Exception { 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(); - // 2026-05-21T12:00:00Z = epoch ms 1779710400000 - long expectedFromMs = 1779710400000L; when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), - eq(50), any(), any(), any(), eq(expectedFromMs), eq((Long) null))) + eq(50), any(), any(), any(), eq(SAMPLE_FROM_TO_EPOCH_MS), eq((Long) null))) .thenReturn(resp); - mockMvc.perform(get("/v1/reservations").param("from", "2026-05-21T12:00:00Z")) + 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 @@ -675,12 +686,13 @@ void shouldPropagateFromOnly() throws Exception { void shouldPropagateToOnly() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); - long expectedToMs = 1779710400000L; when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), - eq(50), any(), any(), any(), eq((Long) null), eq(expectedToMs))) + eq(50), any(), any(), any(), eq((Long) null), eq(SAMPLE_FROM_TO_EPOCH_MS))) .thenReturn(resp); - mockMvc.perform(get("/v1/reservations").param("to", "2026-05-21T12:00:00Z")) + 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 @@ -688,14 +700,15 @@ void shouldPropagateToOnly() throws Exception { void shouldAcceptEqualBounds() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); - long expectedMs = 1779710400000L; when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), - eq(50), any(), any(), any(), eq(expectedMs), eq(expectedMs))) + 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", "2026-05-21T12:00:00Z") - .param("to", "2026-05-21T12:00:00Z")) + .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 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 9aa8196..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 @@ -31,9 +31,19 @@ public static String hash(String tenant, String idempotencyKey, String status, canonical.append("ap=").append(nullSafe(app)).append('|'); canonical.append("wf=").append(nullSafe(workflow)).append('|'); canonical.append("ag=").append(nullSafe(agent)).append('|'); - canonical.append("ts=").append(nullSafe(toolset)).append('|'); - canonical.append("fr=").append(nullSafeLong(fromMs)).append('|'); - canonical.append("to=").append(nullSafeLong(toMs)); + 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)); 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 d91c42f..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 @@ -68,4 +68,19 @@ void timeWindowPositional() { 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"); + } } From 6082ba86e63b6466587fdb4f2861fd9ae4083e5e Mon Sep 17 00:00:00 2001 From: Albert Mavashev Date: Thu, 21 May 2026 11:53:55 -0400 Subject: [PATCH 3/5] docs(listReservations): address review P3s on user-facing docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two P3 follow-up findings from the human review pass: 1. cycles-protocol-service/README.md GET /v1/reservations parameter table omitted the new from / to query params. Added rows for both, plus a clarifying note that legacy SCAN cursors do not carry filter state (clients paginating with from / to but no sort_by must keep the window stable across pages — same convention the legacy path already enforces for status / workspace / app / etc). 2. CHANGELOG.md and AUDIT.md previously claimed "a cursor issued under one (from, to) returns 400 if re-used under a different window" without qualification, but the FilterHasher-driven check only applies to the sorted-path cursor (the SortedListCursor.fh field embedded in the opaque cursor returned when sort_by / sort_dir are supplied). Legacy SCAN cursors have no embedded filter state and are not window-validated. Qualified both docs to "sorted-path cursor invalidation" with an explicit legacy-path caveat so the contract reads accurately on first pass. Also reconciled a stale AUDIT.md line that said "null values render as empty in the canonical form" — the fix-up in 4b122f7 changed the canonical form to omit the |fr=|to= appendix entirely when both bounds are null, so the v0.1.25.18 8-field shape is preserved byte-exactly. Reworded to reflect the gated-emission behavior and cite the FilterHasherTest.preservesPreWindowHashWhenBothBoundsNull regression test as the lockdown. Bumped the in-doc test counts from 537 → 538 (374 data + 163 api → 375 data + 163 api) to reflect the new regression test added in the fix-up commit. The v0.1.25.19 Tomcat-hygiene entry's count is unchanged (correct at the time of that PR). No code or test changes — pure documentation. No version bump (continues v0.1.25.20). --- AUDIT.md | 6 +++--- CHANGELOG.md | 20 ++++++++++++++------ cycles-protocol-service/README.md | 14 ++++++++++---- 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/AUDIT.md b/AUDIT.md index ccd1114..855eaad 100644 --- a/AUDIT.md +++ b/AUDIT.md @@ -1,6 +1,6 @@ # 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 cursors invalidate on window change. 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. 537 tests pass (374 data + 163 api).), +**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 cursors invalidate on window change. 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), @@ -38,7 +38,7 @@ Closes [#159](https://github.com/runcycles/cycles-server/issues/159). Spec lande **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. -**Cursor invalidation.** `FilterHasher.hash(...)` now takes two additional `Long` arguments (`fromMs`, `toMs`) and folds them into the canonical string after the existing eight string fields. A cursor 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. `null` values render as empty in the canonical form, so an `(unset, unset)` cursor still matches a follow-up call that omits both params (back-compat for clients that paginate without ever sending the window). +**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.** @@ -57,7 +57,7 @@ Closes [#159](https://github.com/runcycles/cycles-server/issues/159). Spec lande - `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 537 protocol-service tests pass (374 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`. +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. diff --git a/CHANGELOG.md b/CHANGELOG.md index d46adbf..3a1067c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,11 +29,19 @@ implementing `cycles-protocol-v0.yaml` revision 2026-05-21 and closing `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. -- **Cursor invalidation on window change.** `FilterHasher.hash(...)` - now folds `fromMs` and `toMs` into the canonical hash, so a cursor - issued under one `(from, to)` returns HTTP 400 INVALID_REQUEST if - re-used under a different window — same contract as `sort_by` / - `sort_dir` / subject-filter mismatches (v0.1.25.12). +- **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 @@ -60,7 +68,7 @@ implementing `cycles-protocol-v0.yaml` revision 2026-05-21 and closing ### Coverage -- 537 tests across the protocol-service modules pass (374 data + 163 +- 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 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 From 631e91781b4cecc0d7c2f13d014250cf8268a9c1 Mon Sep 17 00:00:00 2001 From: Albert Mavashev Date: Thu, 21 May 2026 11:56:58 -0400 Subject: [PATCH 4/5] docs(AUDIT): qualify cursor-invalidation claim in top summary line The previous doc fixup (6082ba8) qualified the detailed "Sorted-path cursor invalidation" section but left the parallel claim in the top-of-file Date summary at AUDIT.md:3 unqualified. The summary still read "cursors invalidate on window change" without distinguishing sorted-path from legacy SCAN. Reviewer correctly flagged this as a trailing inconsistency. Updated to "sorted-path cursors invalidate on window change (the legacy Redis-SCAN cursor is not window-validated, matching how it already treats every other filter)." Wording now matches the detailed section and the README. No code or test changes. Pure summary-line correction. Still v0.1.25.20. --- AUDIT.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AUDIT.md b/AUDIT.md index 855eaad..094bf85 100644 --- a/AUDIT.md +++ b/AUDIT.md @@ -1,6 +1,6 @@ # 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 cursors invalidate on window change. 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-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), From c92618f8bb1d8dcfb0ecb725ea7b311e52e5acd7 Mon Sep 17 00:00:00 2001 From: Albert Mavashev Date: Thu, 21 May 2026 12:00:49 -0400 Subject: [PATCH 5/5] docs(AUDIT): fix FilterHasher coverage count --- AUDIT.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AUDIT.md b/AUDIT.md index 094bf85..3a88477 100644 --- a/AUDIT.md +++ b/AUDIT.md @@ -53,7 +53,7 @@ Closes [#159](https://github.com/runcycles/cycles-server/issues/159). Spec lande **Coverage.** -- `FilterHasherTest` — 2 new cases: distinct hash on from/to differences, positional distinctness (`from=100, to=200` ≠ `from=200, to=100`). +- `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.