From f949fe7337873fc68009cc6e9f521a11b8a3fe53 Mon Sep 17 00:00:00 2001 From: Albert Mavashev Date: Fri, 19 Jun 2026 11:19:30 -0400 Subject: [PATCH 1/2] feat(reservation): surface committed + opt-in metadata on listReservations (v0.1.25.36) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements runcycles/cycles-server#201 (follow-up to #197) against cycles-protocol v0.1.25.8 (runcycles/cycles-protocol#115). The list path already hydrated a full ReservationDetail per row, but toSummary dropped committed / metadata / committed_metadata before serializing — the data was read then thrown away. Hybrid projection: - committed (the COMMIT charge, a small scalar) now projects UNCONDITIONALLY on list rows, on the same footing as finalized_at_ms; NON_NULL strips it on non-COMMITTED rows. - metadata (reserve-time) and committed_metadata (commit-time) are arbitrary-size, possibly-PII maps, so they stay OFF list rows by default and are projected only when the caller opts in via a new comma-separated `include` query parameter (?include=metadata,committed_metadata). Unrecognized / empty tokens are ignored without error. The three fields move from ReservationDetail up to the shared ReservationSummary base (ReservationDetail becomes a marker subclass; the single-GET path still serializes all three). A new ReservationInclude enum parses the param (locale-root, case-insensitive). `include` is projection-only and is deliberately NOT folded into FilterHasher, so changing it mid-pagination never invalidates a cursor. listReservations gains an include-aware overload; the legacy 18-arg signature delegates with an empty set, which normalizes null and takes a private EnumSet snapshot. Tests: ReservationIncludeTest (parse edge cases), RedisReservationQueryTest +5 (committed-always + include combinations + legacy-overload default), ReservationControllerTest +3 (param parsing/threading). Full mvn verify green against the v0.1.25.8 spec, JaCoCo 95% gate met. AUDIT.md updated. Spec-first: gated on cycles-protocol#115 reaching main before the contract test resolves the new fields against the published spec. --- AUDIT.md | 8 ++ .../api/controller/ReservationController.java | 15 ++- .../controller/ReservationControllerTest.java | 103 +++++++++++---- .../RedisReservationRepository.java | 75 +++++++++-- .../repository/RedisReservationQueryTest.java | 119 ++++++++++++++++++ .../protocol/model/ReservationDetail.java | 20 +-- .../protocol/model/ReservationInclude.java | 57 +++++++++ .../protocol/model/ReservationSummary.java | 17 +++ .../model/ReservationIncludeTest.java | 71 +++++++++++ cycles-protocol-service/pom.xml | 2 +- 10 files changed, 442 insertions(+), 45 deletions(-) create mode 100644 cycles-protocol-service/cycles-protocol-service-model/src/main/java/io/runcycles/protocol/model/ReservationInclude.java create mode 100644 cycles-protocol-service/cycles-protocol-service-model/src/test/java/io/runcycles/protocol/model/ReservationIncludeTest.java diff --git a/AUDIT.md b/AUDIT.md index aaa18386..ed6ba641 100644 --- a/AUDIT.md +++ b/AUDIT.md @@ -5,6 +5,14 @@ --- +### 2026-06-19 — v0.1.25.36: surface `committed` + opt-in metadata on `listReservations` + +Follow-up to v0.1.25.34/#197: the same fields surfaced on the single-row `getReservation` were dropped from the `GET /v1/reservations` list rows (runcycles/cycles-server#201). The list path already hydrated a full `ReservationDetail` per row (`buildReservationSummary`), but `toSummary` down-converted to `ReservationSummary` and discarded `committed`, `metadata`, and `committed_metadata` — so the data was read then thrown away. + +Hybrid projection per cycles-protocol v0.1.25.8 (runcycles/cycles-protocol#115). `committed` (the COMMIT charge, a small scalar) now projects UNCONDITIONALLY on list rows — same footing as `finalized_at_ms`, NON_NULL strips it off non-COMMITTED rows. The arbitrary-size, possibly-PII metadata maps (`metadata`, `committed_metadata`) stay OFF list rows by default and are projected only when the caller opts in via a new comma-separated `include` query parameter (`?include=metadata,committed_metadata`); unrecognized/empty tokens are ignored, never 400. The three fields move from `ReservationDetail` up to the shared `ReservationSummary` base (`ReservationDetail` becomes a marker subclass — single-GET still serializes all three unconditionally); a new `ReservationInclude` enum parses the param. `include` is projection-only and deliberately NOT folded into `FilterHasher`, so changing it mid-pagination never invalidates a cursor (contrast the window filters). `listReservations` gains an include-aware overload; the legacy 18-arg signature delegates with an empty set (no metadata), preserving the unconditional-`committed` change but default-lean lists. + +`ReservationIncludeTest` (parse: null/blank/whitespace/case/unknown/duplicate/trailing-comma), `RedisReservationQueryTest` +5 (committed-always + each include combination + legacy-overload default), `ReservationControllerTest` +3 (param parsed → empty/both/ignored-unknown sets threaded). Full `mvn verify` green against the v0.1.25.8 spec, JaCoCo 95% gate met. Spec-first: gated on cycles-protocol#115 reaching `main` before the contract test resolves the new fields against the published spec. + ### 2026-06-19 — v0.1.25.35: loud error when a retired-keys config is unusable Closes a silent gap in the v0.1.25.33 rotation-history publication. The active-key `nbf` clamp only fires when a retired entry has a usable window; if `cycles.evidence.signing.retired-keys` is configured (non-blank) but produces ZERO PUBLISHABLE entries, there is nothing to clamp against, so the active key publishes UNBOUNDED at the configured `nbf` (default 0 = since epoch). That silently reverts a rotated server to the never-rotated posture: pre-rotation evidence won't resolve, and the current key could resolve a backdated envelope as authentic. 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 29075aa7..1891e861 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 @@ -12,6 +12,7 @@ import java.time.Instant; import java.time.format.DateTimeParseException; import java.util.Map; +import java.util.Set; import io.swagger.v3.oas.annotations.*; import io.swagger.v3.oas.annotations.tags.Tag; import jakarta.servlet.http.HttpServletRequest; @@ -283,7 +284,8 @@ public ResponseEntity list( @RequestParam(value = "expires_from", required = false) String expiresFrom, @RequestParam(value = "expires_to", required = false) String expiresTo, @RequestParam(value = "finalized_from", required = false) String finalizedFrom, - @RequestParam(value = "finalized_to", required = false) String finalizedTo) { + @RequestParam(value = "finalized_to", required = false) String finalizedTo, + @RequestParam(value = "include", required = false) String include) { // Validate status against ReservationStatus enum if provided if (status != null) { try { @@ -364,12 +366,17 @@ public ResponseEntity list( effectiveTenant = tenant != null ? tenant : extractAuthTenantId(); authorizeTenant(effectiveTenant); } - LOG.info("GET /v1/reservations - tenant: {}, admin: {}, sort_by: {}, sort_dir: {}, from: {}, to: {}, expires_from: {}, expires_to: {}, finalized_from: {}, finalized_to: {}", + // cycles-protocol revision 2026-06-19 (cycles-server#201): include is a + // comma-separated field-projection list. Unrecognized / empty tokens are + // ignored (no 400). Projection-only — never bound to the cursor. + Set includeFields = ReservationInclude.parseCsv(include); + LOG.info("GET /v1/reservations - tenant: {}, admin: {}, sort_by: {}, sort_dir: {}, from: {}, to: {}, expires_from: {}, expires_to: {}, finalized_from: {}, finalized_to: {}, include: {}", effectiveTenant, isAdminAuth(), sortBy, sortDir, fromMs, toMs, - expiresFromMs, expiresToMs, finalizedFromMs, finalizedToMs); + expiresFromMs, expiresToMs, finalizedFromMs, finalizedToMs, includeFields); return ResponseEntity.ok(repository.listReservations(effectiveTenant, idempotencyKey, status, workspace, app, workflow, agent, toolset, limit, cursor, sortBy, sortDir, - fromMs, toMs, expiresFromMs, expiresToMs, finalizedFromMs, finalizedToMs)); + fromMs, toMs, expiresFromMs, expiresToMs, finalizedFromMs, finalizedToMs, + includeFields)); } private Long parseIsoToEpochMs(String raw, String paramName) { diff --git a/cycles-protocol-service/cycles-protocol-service-api/src/test/java/io/runcycles/protocol/api/controller/ReservationControllerTest.java b/cycles-protocol-service/cycles-protocol-service-api/src/test/java/io/runcycles/protocol/api/controller/ReservationControllerTest.java index 2dbe2646..7e280a42 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 @@ -25,6 +25,7 @@ import org.springframework.test.web.servlet.MockMvc; import java.util.Collections; +import java.util.EnumSet; import java.util.List; import java.util.UUID; @@ -654,7 +655,7 @@ void shouldListReservations() throws Exception { .reservations(Collections.emptyList()) .hasMore(false) .build(); - when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any())) + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations")) @@ -662,6 +663,60 @@ void shouldListReservations() throws Exception { .andExpect(jsonPath("$.has_more").value(false)); } + // cycles-protocol revision 2026-06-19 (cycles-server#201): include is a + // comma-separated field-projection list, parsed at the controller and + // threaded to the repository. Unrecognized / empty tokens are ignored. + @Test + @DisplayName("no include param → empty projection set propagates") + void shouldDefaultIncludeToEmptySet() throws Exception { + ReservationListResponse resp = ReservationListResponse.builder() + .reservations(Collections.emptyList()).hasMore(false).build(); + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), + eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any(), + eq(EnumSet.noneOf(ReservationInclude.class)))).thenReturn(resp); + + mockMvc.perform(get("/v1/reservations")).andExpect(status().isOk()); + + verify(repository).listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), + eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any(), + eq(EnumSet.noneOf(ReservationInclude.class))); + } + + @Test + @DisplayName("include=metadata,committed_metadata → both tokens propagate") + void shouldPropagateIncludeTokens() throws Exception { + ReservationListResponse resp = ReservationListResponse.builder() + .reservations(Collections.emptyList()).hasMore(false).build(); + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), + eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any(), + eq(EnumSet.of(ReservationInclude.METADATA, ReservationInclude.COMMITTED_METADATA)))) + .thenReturn(resp); + + mockMvc.perform(get("/v1/reservations").param("include", "metadata,committed_metadata")) + .andExpect(status().isOk()); + + verify(repository).listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), + eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any(), + eq(EnumSet.of(ReservationInclude.METADATA, ReservationInclude.COMMITTED_METADATA))); + } + + @Test + @DisplayName("include=bogus (unrecognized) → 200 and empty projection set, no 400") + void shouldIgnoreUnrecognizedIncludeToken() throws Exception { + ReservationListResponse resp = ReservationListResponse.builder() + .reservations(Collections.emptyList()).hasMore(false).build(); + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), + eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any(), + eq(EnumSet.noneOf(ReservationInclude.class)))).thenReturn(resp); + + mockMvc.perform(get("/v1/reservations").param("include", "bogus,also_bogus")) + .andExpect(status().isOk()); + + verify(repository).listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), + eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any(), + eq(EnumSet.noneOf(ReservationInclude.class))); + } + @Test void shouldRejectInvalidStatusFilter() throws Exception { mockMvc.perform(get("/v1/reservations").param("status", "BOGUS")) @@ -673,7 +728,7 @@ void shouldRejectInvalidStatusFilter() throws Exception { void shouldListWithActiveStatusFilter() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); - when(repository.listReservations(eq(TENANT), any(), eq("ACTIVE"), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any())) + when(repository.listReservations(eq(TENANT), any(), eq("ACTIVE"), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations").param("status", "ACTIVE")) @@ -685,7 +740,7 @@ void shouldListWithActiveStatusFilter() throws Exception { void shouldListWithCommittedStatusFilter() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); - when(repository.listReservations(eq(TENANT), any(), eq("COMMITTED"), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any())) + when(repository.listReservations(eq(TENANT), any(), eq("COMMITTED"), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations").param("status", "COMMITTED")) @@ -696,7 +751,7 @@ void shouldListWithCommittedStatusFilter() throws Exception { void shouldListWithExplicitTenantMatchingAuth() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); - when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any())) + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations").param("tenant", TENANT)) @@ -714,7 +769,7 @@ void shouldRejectListWithTenantMismatch() throws Exception { void shouldDefaultTenantFromAuth() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); - when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any())) + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any(), any())) .thenReturn(resp); // No tenant param — should use auth tenant @@ -750,7 +805,7 @@ void shouldRejectInvalidSortDir() throws Exception { void shouldPropagateSortParams() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); - when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), eq("status"), eq("asc"), any(), any(), any(), any(), any(), any())) + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), eq("status"), eq("asc"), any(), any(), any(), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations") .param("sort_by", "status") @@ -763,7 +818,7 @@ void shouldPropagateSortParams() throws Exception { void shouldAcceptAllSpecSortByValues() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); - when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any())) + when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any(), any())) .thenReturn(resp); for (String value : new String[] {"reservation_id", "tenant", "scope_path", "status", "reserved", "created_at_ms", "expires_at_ms"}) { @@ -808,7 +863,7 @@ void shouldRejectReversedWindow() throws Exception { // No repository invocation expected — validation runs before list call. verify(repository, org.mockito.Mockito.never()).listReservations( any(), any(), any(), any(), any(), any(), any(), any(), - anyInt(), any(), any(), any(), any(), any(), any(), any(), any(), any()); + anyInt(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any()); } // Derive the expected epoch-ms from the same parser the controller uses so a @@ -825,7 +880,7 @@ void shouldPropagateFromOnly() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), - eq(50), any(), any(), any(), eq(SAMPLE_FROM_TO_EPOCH_MS), eq((Long) null), any(), any(), any(), any())) + eq(50), any(), any(), any(), eq(SAMPLE_FROM_TO_EPOCH_MS), eq((Long) null), any(), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations").param("from", SAMPLE_FROM_TO_INSTANT)) .andExpect(status().isOk()); @@ -833,7 +888,7 @@ void shouldPropagateFromOnly() throws Exception { // a wrong eq(...) literal caused Mockito to silently return null while the // test still asserted HTTP 200. verify(repository).listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), - eq(50), any(), any(), any(), eq(SAMPLE_FROM_TO_EPOCH_MS), eq((Long) null), any(), any(), any(), any()); + eq(50), any(), any(), any(), eq(SAMPLE_FROM_TO_EPOCH_MS), eq((Long) null), any(), any(), any(), any(), any()); } @Test @@ -842,12 +897,12 @@ void shouldPropagateToOnly() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), - eq(50), any(), any(), any(), eq((Long) null), eq(SAMPLE_FROM_TO_EPOCH_MS), any(), any(), any(), any())) + eq(50), any(), any(), any(), eq((Long) null), eq(SAMPLE_FROM_TO_EPOCH_MS), any(), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations").param("to", SAMPLE_FROM_TO_INSTANT)) .andExpect(status().isOk()); verify(repository).listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), - eq(50), any(), any(), any(), eq((Long) null), eq(SAMPLE_FROM_TO_EPOCH_MS), any(), any(), any(), any()); + eq(50), any(), any(), any(), eq((Long) null), eq(SAMPLE_FROM_TO_EPOCH_MS), any(), any(), any(), any(), any()); } @Test @@ -856,14 +911,14 @@ void shouldAcceptEqualBounds() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), - eq(50), any(), any(), any(), eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS), any(), any(), any(), any())) + eq(50), any(), any(), any(), eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS), any(), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations") .param("from", SAMPLE_FROM_TO_INSTANT) .param("to", SAMPLE_FROM_TO_INSTANT)) .andExpect(status().isOk()); verify(repository).listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), - eq(50), any(), any(), any(), eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS), any(), any(), any(), any()); + eq(50), any(), any(), any(), eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS), any(), any(), any(), any(), any()); } @Test @@ -872,7 +927,7 @@ void shouldCombineWithDifferentSortKey() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); when(repository.listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), - eq(50), any(), eq("expires_at_ms"), any(), any(), any(), any(), any(), any(), any())) + eq(50), any(), eq("expires_at_ms"), any(), any(), any(), any(), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations") .param("sort_by", "expires_at_ms") @@ -887,7 +942,7 @@ 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), any(), any(), any(), any())) + eq(50), any(), any(), any(), eq((Long) null), eq((Long) null), any(), any(), any(), any(), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations") .param("from", "") @@ -972,7 +1027,7 @@ void shouldPropagateExpiresWindow() 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(), eq((Long) null), eq((Long) null), - eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS), eq((Long) null), eq((Long) null))) + eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS), eq((Long) null), eq((Long) null), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations") .param("expires_from", SAMPLE_FROM_TO_INSTANT) @@ -983,7 +1038,7 @@ void shouldPropagateExpiresWindow() throws Exception { // that misroutes expires_from into the from slot would silently pass. verify(repository).listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), eq((Long) null), eq((Long) null), - eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS), eq((Long) null), eq((Long) null)); + eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS), eq((Long) null), eq((Long) null), any()); } @Test @@ -993,7 +1048,7 @@ void shouldPropagateFinalizedWindow() 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(), eq((Long) null), eq((Long) null), - eq((Long) null), eq((Long) null), eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS))) + eq((Long) null), eq((Long) null), eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations") .param("finalized_from", SAMPLE_FROM_TO_INSTANT) @@ -1001,7 +1056,7 @@ void shouldPropagateFinalizedWindow() throws Exception { .andExpect(status().isOk()); verify(repository).listReservations(eq(TENANT), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), eq((Long) null), eq((Long) null), - eq((Long) null), eq((Long) null), eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS)); + eq((Long) null), eq((Long) null), eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS), any()); } @Test @@ -1019,7 +1074,7 @@ void shouldPropagateAllThreeWindows() throws Exception { eq(50), any(), any(), any(), eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS), eq(EXPIRES_MS), eq(EXPIRES_MS), - eq(FINALIZED_MS), eq(FINALIZED_MS))) + eq(FINALIZED_MS), eq(FINALIZED_MS), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations") .param("from", SAMPLE_FROM_TO_INSTANT) @@ -1033,7 +1088,7 @@ void shouldPropagateAllThreeWindows() throws Exception { eq(50), any(), any(), any(), eq(SAMPLE_FROM_TO_EPOCH_MS), eq(SAMPLE_FROM_TO_EPOCH_MS), eq(EXPIRES_MS), eq(EXPIRES_MS), - eq(FINALIZED_MS), eq(FINALIZED_MS)); + eq(FINALIZED_MS), eq(FINALIZED_MS), any()); } @Test @@ -1043,7 +1098,7 @@ void shouldTreatBlankAsUnsetForNewWindows() 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(), eq((Long) null), eq((Long) null), - eq((Long) null), eq((Long) null), eq((Long) null), eq((Long) null))) + eq((Long) null), eq((Long) null), eq((Long) null), eq((Long) null), any())) .thenReturn(resp); mockMvc.perform(get("/v1/reservations") .param("expires_from", "") @@ -1070,7 +1125,7 @@ void setAdminAuth() { void adminListWithTenantFilter() throws Exception { ReservationListResponse resp = ReservationListResponse.builder() .reservations(Collections.emptyList()).hasMore(false).build(); - when(repository.listReservations(eq("any-tenant"), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any())) + when(repository.listReservations(eq("any-tenant"), any(), any(), any(), any(), any(), any(), any(), eq(50), any(), any(), any(), any(), any(), any(), any(), any(), any(), 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 8e9dd423..84979e03 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 @@ -1078,6 +1078,36 @@ public ReservationListResponse listReservations(String tenant, String idempotenc Long fromMs, Long toMs, Long expiresFromMs, Long expiresToMs, Long finalizedFromMs, Long finalizedToMs) { + return listReservations(tenant, idempotencyKey, status, workspace, app, workflow, + agent, toolset, limit, startCursor, sortBy, sortDir, fromMs, toMs, + expiresFromMs, expiresToMs, finalizedFromMs, finalizedToMs, + EnumSet.noneOf(ReservationInclude.class)); + } + + /** + * include-aware overload (cycles-protocol revision 2026-06-19, + * cycles-server#201). {@code include} is PROJECTION-ONLY: it selects which + * optional metadata maps are serialized onto each summary row and is + * deliberately NOT part of the filter/cursor binding (see + * {@link FilterHasher} — it is not passed there), so changing {@code include} + * across pages never invalidates a cursor. {@code committed} is always + * projected regardless of {@code include}. + */ + public ReservationListResponse listReservations(String tenant, String idempotencyKey, + String status, String workspace, String app, + String workflow, String agent, String toolset, + int limit, String startCursor, + String sortBy, String sortDir, + Long fromMs, Long toMs, + Long expiresFromMs, Long expiresToMs, + Long finalizedFromMs, Long finalizedToMs, + Set include) { + // Normalize once at entry: tolerate a null arg (direct callers) and take a + // private snapshot so a caller mutating their set mid-scan can't affect + // projection. EnumSet.copyOf requires a non-empty source. + Set includeFields = (include == null || include.isEmpty()) + ? EnumSet.noneOf(ReservationInclude.class) + : EnumSet.copyOf(include); boolean sortRequested = sortBy != null || sortDir != null; Optional parsedCursor = SortedListCursor.decode(startCursor); @@ -1087,7 +1117,7 @@ public ReservationListResponse listReservations(String tenant, String idempotenc if (sortRequested || parsedCursor.isPresent()) { return listReservationsSorted(tenant, idempotencyKey, status, workspace, app, workflow, agent, toolset, limit, parsedCursor.orElse(null), sortBy, sortDir, - fromMs, toMs, expiresFromMs, expiresToMs, finalizedFromMs, finalizedToMs); + fromMs, toMs, expiresFromMs, expiresToMs, finalizedFromMs, finalizedToMs, includeFields); } try (Jedis jedis = jedisPool.getResource()) { @@ -1131,7 +1161,7 @@ public ReservationListResponse listReservations(String tenant, String idempotenc if (!expiresAtInWindow(fields, expiresFromMs, expiresToMs)) continue; if (!finalizedAtInWindow(fields, finalizedFromMs, finalizedToMs)) continue; - result.add(toSummary(buildReservationSummary(fields))); + result.add(toSummary(buildReservationSummary(fields), includeFields)); if (result.size() >= limit) { String nextCursor = scan.getCursor(); @@ -1171,7 +1201,8 @@ private ReservationListResponse listReservationsSorted( int limit, SortedListCursor resumeCursor, String sortBy, String sortDir, Long fromMs, Long toMs, Long expiresFromMs, Long expiresToMs, - Long finalizedFromMs, Long finalizedToMs) { + Long finalizedFromMs, Long finalizedToMs, + Set include) { // 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 @@ -1181,6 +1212,10 @@ private ReservationListResponse listReservationsSorted( String effectiveSortDir = sortDir != null ? sortDir.toLowerCase() : (resumeCursor != null ? resumeCursor.getSortDir() : "desc"); + // include is deliberately NOT folded into the filter hash: it is a + // projection-only parameter (which fields serialize), not a filter + // (which rows / what order). Changing include across pages MUST NOT + // invalidate a cursor. Spec: cycles-protocol-v0 revision 2026-06-19. String filterHash = FilterHasher.hash(tenant, idempotencyKey, status, workspace, app, workflow, agent, toolset, fromMs, toMs, expiresFromMs, expiresToMs, finalizedFromMs, finalizedToMs); @@ -1238,7 +1273,7 @@ private ReservationListResponse listReservationsSorted( if (!expiresAtInWindow(fields, expiresFromMs, expiresToMs)) continue; if (!finalizedAtInWindow(fields, finalizedFromMs, finalizedToMs)) continue; - matching.add(toSummary(buildReservationSummary(fields))); + matching.add(toSummary(buildReservationSummary(fields), include)); } catch (Exception e) { LOG.warn("Failed to parse reservation: {}", key, e); } @@ -1851,8 +1886,8 @@ private boolean scopeHasSegment(String scopePath, String segment) { return startOk && endOk; } - private ReservationSummary toSummary(ReservationDetail detail) { - return ReservationSummary.builder() + private ReservationSummary toSummary(ReservationDetail detail, Set include) { + ReservationSummary.ReservationSummaryBuilder builder = ReservationSummary.builder() .reservationId(detail.getReservationId()) .status(detail.getStatus()) .idempotencyKey(detail.getIdempotencyKey()) @@ -1868,7 +1903,23 @@ private ReservationSummary toSummary(ReservationDetail detail) { .finalizedAtMs(detail.getFinalizedAtMs()) .scopePath(detail.getScopePath()) .affectedScopes(detail.getAffectedScopes()) - .build(); + // committed (the COMMIT charge) is projected UNCONDITIONALLY on list + // rows, on the same footing as finalized_at_ms — a small scalar, and + // NON_NULL strips it on non-COMMITTED rows. Spec: cycles-protocol-v0 + // revision 2026-06-19 (cycles-server#201). + .committed(detail.getCommitted()); + // metadata / committed_metadata are arbitrary-size, possibly-PII maps, so + // they are OMITTED FROM LIST ROWS BY DEFAULT and projected only when the + // caller opts in via ?include=. The single-row getReservation path always + // carries them (it serializes the ReservationDetail directly). Spec rev + // 2026-06-19. + if (include.contains(ReservationInclude.METADATA)) { + builder.metadata(detail.getMetadata()); + } + if (include.contains(ReservationInclude.COMMITTED_METADATA)) { + builder.committedMetadata(detail.getCommittedMetadata()); + } + return builder.build(); } private ReservationDetail buildReservationSummary(Map fields) throws Exception { @@ -1924,7 +1975,15 @@ private ReservationDetail buildReservationSummary(Map fields) th committedMetadata = objectMapper.readValue(committedMetadataJson, Map.class); } - ReservationDetail detail = new ReservationDetail(committed, finalizedAtMs, metadata, committedMetadata); + // As of cycles-protocol revision 2026-06-19 (cycles-server#201) committed + // / metadata / committed_metadata live on the shared ReservationSummary + // base, so set them via the inherited setters rather than a detail-only + // constructor. + ReservationDetail detail = new ReservationDetail(); + detail.setCommitted(committed); + detail.setFinalizedAtMs(finalizedAtMs); + detail.setMetadata(metadata); + detail.setCommittedMetadata(committedMetadata); detail.setReservationId(fields.get("reservation_id")); detail.setStatus(Enums.ReservationStatus.valueOf(stateStr)); detail.setIdempotencyKey(fields.get("idempotency_key")); 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 2343d26b..5b82d0e6 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 @@ -591,6 +591,125 @@ void shouldReturnMatchingStatusFilter() { } } + // ---- listReservations field projection: committed + opt-in metadata ---- + // Spec: cycles-protocol-v0.yaml revision 2026-06-19 (cycles-server#201). + + @Nested + @DisplayName("listReservations field projection (committed + include)") + class ListReservationsIncludeProjection { + + /** A COMMITTED reservation carrying a charge plus reserve-time and + * commit-time metadata, so the projection rules are observable. */ + private Map committedFields(String id) { + Map fields = reservationFields(id, "COMMITTED"); + fields.put("charged_amount", "4200"); + fields.put("finalized_at", "1700050000000"); + fields.put("metadata_json", "{\"session_id\":\"s-1\"}"); + fields.put("committed_metadata_json", "{\"request_id\":\"req-9\"}"); + return fields; + } + + @SuppressWarnings("unchecked") + private ReservationListResponse listSingleCommitted(Set include) { + when(jedisPool.getResource()).thenReturn(jedis); + doNothing().when(jedis).close(); + + Pipeline pipeline = mock(Pipeline.class); + Response> resp = mock(Response.class); + when(resp.get()).thenReturn(committedFields("r1")); + + ScanResult scanResult = mock(ScanResult.class); + when(scanResult.getResult()).thenReturn(List.of("reservation:res_r1")); + when(scanResult.getCursor()).thenReturn("0"); + when(jedis.scan(eq("0"), any(ScanParams.class))).thenReturn(scanResult); + when(jedis.pipelined()).thenReturn(pipeline); + when(pipeline.hgetAll("reservation:res_r1")).thenReturn(resp); + + return repository.listReservations( + "acme", null, null, null, null, null, null, null, 100, null, null, null, + null, null, null, null, null, null, include); + } + + @Test + @DisplayName("committed is projected unconditionally; metadata maps omitted by default") + void committedAlwaysMetadataOptIn() { + ReservationListResponse response = + listSingleCommitted(EnumSet.noneOf(ReservationInclude.class)); + + assertThat(response.getReservations()).hasSize(1); + ReservationSummary row = response.getReservations().get(0); + // committed is surfaced on the list row with no include opt-in. + assertThat(row.getCommitted()).isNotNull(); + assertThat(row.getCommitted().getAmount()).isEqualTo(4200L); + // metadata / committed_metadata are omitted unless explicitly requested. + assertThat(row.getMetadata()).isNull(); + assertThat(row.getCommittedMetadata()).isNull(); + } + + @Test + @DisplayName("include=committed_metadata surfaces only committed_metadata") + void includeCommittedMetadataOnly() { + ReservationListResponse response = + listSingleCommitted(EnumSet.of(ReservationInclude.COMMITTED_METADATA)); + + ReservationSummary row = response.getReservations().get(0); + assertThat(row.getCommitted().getAmount()).isEqualTo(4200L); + assertThat(row.getCommittedMetadata()).containsEntry("request_id", "req-9"); + assertThat(row.getMetadata()).isNull(); + } + + @Test + @DisplayName("include=metadata surfaces only reserve-time metadata") + void includeMetadataOnly() { + ReservationListResponse response = + listSingleCommitted(EnumSet.of(ReservationInclude.METADATA)); + + ReservationSummary row = response.getReservations().get(0); + assertThat(row.getMetadata()).containsEntry("session_id", "s-1"); + assertThat(row.getCommittedMetadata()).isNull(); + } + + @Test + @DisplayName("include=metadata,committed_metadata surfaces both maps") + void includeBothMaps() { + ReservationListResponse response = listSingleCommitted( + EnumSet.of(ReservationInclude.METADATA, ReservationInclude.COMMITTED_METADATA)); + + ReservationSummary row = response.getReservations().get(0); + assertThat(row.getMetadata()).containsEntry("session_id", "s-1"); + assertThat(row.getCommittedMetadata()).containsEntry("request_id", "req-9"); + } + + @Test + @DisplayName("18-arg overload defaults to no metadata projection (committed still present)") + void legacyOverloadOmitsMetadata() { + when(jedisPool.getResource()).thenReturn(jedis); + doNothing().when(jedis).close(); + + Pipeline pipeline = mock(Pipeline.class); + @SuppressWarnings("unchecked") + Response> resp = mock(Response.class); + when(resp.get()).thenReturn(committedFields("r1")); + + @SuppressWarnings("unchecked") + ScanResult scanResult = mock(ScanResult.class); + when(scanResult.getResult()).thenReturn(List.of("reservation:res_r1")); + when(scanResult.getCursor()).thenReturn("0"); + when(jedis.scan(eq("0"), any(ScanParams.class))).thenReturn(scanResult); + when(jedis.pipelined()).thenReturn(pipeline); + when(pipeline.hgetAll("reservation:res_r1")).thenReturn(resp); + + ReservationListResponse response = repository.listReservations( + "acme", null, null, null, null, null, null, null, 100, null, null, null, + null, null, null, null, null, null); + + ReservationSummary row = response.getReservations().get(0); + assertThat(row.getCommitted().getAmount()).isEqualTo(4200L); + assertThat(row.getMetadata()).isNull(); + assertThat(row.getCommittedMetadata()).isNull(); + } + } + // ---- listReservations with idempotency_key filter ---- @Nested diff --git a/cycles-protocol-service/cycles-protocol-service-model/src/main/java/io/runcycles/protocol/model/ReservationDetail.java b/cycles-protocol-service/cycles-protocol-service-model/src/main/java/io/runcycles/protocol/model/ReservationDetail.java index 0efe2b3d..a03ea2f6 100644 --- a/cycles-protocol-service/cycles-protocol-service-model/src/main/java/io/runcycles/protocol/model/ReservationDetail.java +++ b/cycles-protocol-service/cycles-protocol-service-model/src/main/java/io/runcycles/protocol/model/ReservationDetail.java @@ -1,17 +1,21 @@ package io.runcycles.protocol.model; import com.fasterxml.jackson.annotation.*; -import jakarta.validation.Valid; import lombok.*; -import java.util.Map; -/** Cycles Protocol v0.1.25 */ -@Data @EqualsAndHashCode(callSuper = true) @NoArgsConstructor @AllArgsConstructor +/** + * Cycles Protocol v0.1.25 — single-reservation detail + * (GET /v1/reservations/{reservation_id}). + * + *

As of cycles-protocol-v0.yaml revision 2026-06-19 (cycles-server#201) the + * {@code committed}, {@code metadata}, and {@code committed_metadata} fields + * live on {@link ReservationSummary}, so the list and single-row projections + * share one shape. ReservationDetail remains a distinct type for the + * single-GET response, where those fields are always populated (when present + * on the record); the list path opts into the metadata maps via the + * {@code include} query parameter. */ +@EqualsAndHashCode(callSuper = true) @NoArgsConstructor @JsonInclude(JsonInclude.Include.NON_NULL) @JsonIgnoreProperties(ignoreUnknown = false) public class ReservationDetail extends ReservationSummary { - @Valid @JsonProperty("committed") private Amount committed; - @JsonProperty("finalized_at_ms") private Long finalizedAtMs; - @JsonProperty("metadata") private Map metadata; - @JsonProperty("committed_metadata") private Map committedMetadata; } diff --git a/cycles-protocol-service/cycles-protocol-service-model/src/main/java/io/runcycles/protocol/model/ReservationInclude.java b/cycles-protocol-service/cycles-protocol-service-model/src/main/java/io/runcycles/protocol/model/ReservationInclude.java new file mode 100644 index 00000000..8d80bd3b --- /dev/null +++ b/cycles-protocol-service/cycles-protocol-service-model/src/main/java/io/runcycles/protocol/model/ReservationInclude.java @@ -0,0 +1,57 @@ +package io.runcycles.protocol.model; + +import java.util.EnumSet; +import java.util.Locale; +import java.util.Set; + +/** + * Field-projection tokens for the {@code listReservations} {@code include} + * query parameter (cycles-protocol-v0.yaml revision 2026-06-19). + * + *

Projection-only: {@code include} selects which optional heavy fields are + * serialized onto each {@link ReservationSummary} list row. It never affects + * which rows match, their ordering, pagination, or cursor / filter-hash + * binding. Unrecognized and empty tokens are ignored without error + * (forward/backward compatible). {@code committed} is NOT gated here — it is + * always projected on list rows. + */ +public enum ReservationInclude { + METADATA("metadata"), + COMMITTED_METADATA("committed_metadata"); + + private final String wire; + + ReservationInclude(String wire) { + this.wire = wire; + } + + public String wire() { + return wire; + } + + /** + * Parse a comma-separated {@code include} value into the set of recognized + * tokens. Surrounding whitespace is trimmed; empty tokens (e.g. a trailing + * comma, or {@code include=}) and unrecognized tokens are ignored. Null or + * blank input yields an empty set. + */ + public static Set parseCsv(String raw) { + EnumSet out = EnumSet.noneOf(ReservationInclude.class); + if (raw == null || raw.isBlank()) { + return out; + } + for (String token : raw.split(",")) { + String t = token.trim().toLowerCase(Locale.ROOT); + if (t.isEmpty()) { + continue; + } + for (ReservationInclude inc : values()) { + if (inc.wire.equals(t)) { + out.add(inc); + break; + } + } + } + return out; + } +} diff --git a/cycles-protocol-service/cycles-protocol-service-model/src/main/java/io/runcycles/protocol/model/ReservationSummary.java b/cycles-protocol-service/cycles-protocol-service-model/src/main/java/io/runcycles/protocol/model/ReservationSummary.java index 4bd6528c..428c2ba5 100644 --- a/cycles-protocol-service/cycles-protocol-service-model/src/main/java/io/runcycles/protocol/model/ReservationSummary.java +++ b/cycles-protocol-service/cycles-protocol-service-model/src/main/java/io/runcycles/protocol/model/ReservationSummary.java @@ -5,6 +5,7 @@ import jakarta.validation.constraints.*; import lombok.*; import java.util.List; +import java.util.Map; /** Cycles Protocol v0.1.25 */ @Data @Builder @NoArgsConstructor @AllArgsConstructor @@ -26,4 +27,20 @@ public class ReservationSummary { @Min(0) @JsonProperty("finalized_at_ms") private Long finalizedAtMs; @NotNull @JsonProperty("scope_path") private String scopePath; @NotNull @JsonProperty("affected_scopes") private List affectedScopes; + /** The amount charged at COMMIT. Present on COMMITTED rows only; NON_NULL + * strips it on ACTIVE / EXPIRED / RELEASED rows. Projected UNCONDITIONALLY + * on listReservations rows (no include opt-in needed), on the same footing + * as finalized_at_ms. Spec: cycles-protocol-v0.yaml revision 2026-06-19 + * (cycles-server#201). */ + @Valid @JsonProperty("committed") private Amount committed; + /** RESERVE-time metadata. Always present (when populated) on the single-row + * ReservationDetail; on listReservations it is OMITTED BY DEFAULT and + * projected only when the caller opts in via include=metadata — the map is + * arbitrary-size and may carry application PII, so it is not sprayed across + * every list row. Spec: cycles-protocol-v0.yaml revision 2026-06-19. */ + @JsonProperty("metadata") private Map metadata; + /** COMMIT-time metadata. Same include-gated list semantics as metadata + * (include=committed_metadata). Spec: cycles-protocol-v0.yaml revision + * 2026-06-19 (cycles-server#197 read-path, #201 list projection). */ + @JsonProperty("committed_metadata") private Map committedMetadata; } diff --git a/cycles-protocol-service/cycles-protocol-service-model/src/test/java/io/runcycles/protocol/model/ReservationIncludeTest.java b/cycles-protocol-service/cycles-protocol-service-model/src/test/java/io/runcycles/protocol/model/ReservationIncludeTest.java new file mode 100644 index 00000000..e52fe49b --- /dev/null +++ b/cycles-protocol-service/cycles-protocol-service-model/src/test/java/io/runcycles/protocol/model/ReservationIncludeTest.java @@ -0,0 +1,71 @@ +package io.runcycles.protocol.model; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.util.EnumSet; +import java.util.Set; + +import static org.assertj.core.api.Assertions.assertThat; + +@DisplayName("ReservationInclude.parseCsv") +class ReservationIncludeTest { + + @Test + @DisplayName("null and blank yield an empty set") + void nullAndBlank() { + assertThat(ReservationInclude.parseCsv(null)).isEmpty(); + assertThat(ReservationInclude.parseCsv("")).isEmpty(); + assertThat(ReservationInclude.parseCsv(" ")).isEmpty(); + } + + @Test + @DisplayName("recognized single tokens parse") + void singleTokens() { + assertThat(ReservationInclude.parseCsv("metadata")) + .containsExactly(ReservationInclude.METADATA); + assertThat(ReservationInclude.parseCsv("committed_metadata")) + .containsExactly(ReservationInclude.COMMITTED_METADATA); + } + + @Test + @DisplayName("comma-separated list parses both, order-independent") + void bothTokens() { + Set parsed = + ReservationInclude.parseCsv("committed_metadata,metadata"); + assertThat(parsed).isEqualTo( + EnumSet.of(ReservationInclude.METADATA, ReservationInclude.COMMITTED_METADATA)); + } + + @Test + @DisplayName("surrounding whitespace is trimmed and matching is case-insensitive") + void whitespaceAndCase() { + assertThat(ReservationInclude.parseCsv(" METADATA , Committed_Metadata ")) + .containsExactlyInAnyOrder( + ReservationInclude.METADATA, ReservationInclude.COMMITTED_METADATA); + } + + @Test + @DisplayName("unrecognized and empty tokens are ignored without error") + void unknownAndEmptyIgnored() { + // trailing comma + bogus token + a valid one + assertThat(ReservationInclude.parseCsv("metadata,,bogus,")) + .containsExactly(ReservationInclude.METADATA); + assertThat(ReservationInclude.parseCsv("nope")).isEmpty(); + assertThat(ReservationInclude.parseCsv(",")).isEmpty(); + } + + @Test + @DisplayName("duplicate tokens collapse") + void duplicatesCollapse() { + assertThat(ReservationInclude.parseCsv("metadata,metadata")) + .containsExactly(ReservationInclude.METADATA); + } + + @Test + @DisplayName("wire() exposes the token spelling") + void wireSpelling() { + assertThat(ReservationInclude.METADATA.wire()).isEqualTo("metadata"); + assertThat(ReservationInclude.COMMITTED_METADATA.wire()).isEqualTo("committed_metadata"); + } +} diff --git a/cycles-protocol-service/pom.xml b/cycles-protocol-service/pom.xml index eddea0af..5494e4d9 100644 --- a/cycles-protocol-service/pom.xml +++ b/cycles-protocol-service/pom.xml @@ -18,7 +18,7 @@ cycles-protocol-service-api - 0.1.25.35 + 0.1.25.36 21 21 21 From 97ca49c75b5de0e4ba4af46b77e77bccc26554de Mon Sep 17 00:00:00 2001 From: Albert Mavashev Date: Fri, 19 Jun 2026 11:29:55 -0400 Subject: [PATCH 2/2] test(reservation): prove include does not invalidate the pagination cursor Adds RedisReservationQueryTest#includeDoesNotBindCursor, the sorted-pagination case the review flagged as missing: page 1 with no include captures the cursor, page 2 reuses that cursor while flipping include=committed_metadata. Asserts the cursor stays valid (no 400, contrast cursorMismatchRejected where sort_by binds) and that the newly-requested field projects onto page 2. Locks in the projection-only / not-cursor-bound guarantee against a future refactor that might fold include into FilterHasher. --- .../repository/RedisReservationQueryTest.java | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) 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 5b82d0e6..db7e7119 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 @@ -917,6 +917,76 @@ void paginatesAcrossPages() { assertThat(page2.getNextCursor()).isNull(); } + // cycles-protocol revision 2026-06-19 (cycles-server#201): include is + // PROJECTION-ONLY and MUST NOT bind the cursor. Changing it between pages + // must neither 400 (contrast cursorMismatchRejected, where sort_by does + // bind) nor disturb paging — it only changes which fields serialize. + @SuppressWarnings("unchecked") + @Test + @DisplayName("include can change between pages without invalidating the cursor") + void includeDoesNotBindCursor() { + when(jedisPool.getResource()).thenReturn(jedis); + doNothing().when(jedis).close(); + + // COMMITTED rows carrying commit metadata, distinct created_at for a + // deterministic sort. + Map f1 = committedRow("r1", 100); + Map f2 = committedRow("r2", 200); + Map f3 = committedRow("r3", 300); + Map f4 = committedRow("r4", 400); + Response> resp1 = mock(Response.class); + Response> resp2 = mock(Response.class); + Response> resp3 = mock(Response.class); + Response> resp4 = mock(Response.class); + when(resp1.get()).thenReturn(f1); + when(resp2.get()).thenReturn(f2); + when(resp3.get()).thenReturn(f3); + when(resp4.get()).thenReturn(f4); + + 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); + + // Page 1: no include → committed_metadata omitted; capture the cursor. + ReservationListResponse page1 = repository.listReservations( + "acme", null, null, null, null, null, null, null, 2, null, + "created_at_ms", "asc", null, null, null, null, null, null, + EnumSet.noneOf(ReservationInclude.class)); + assertThat(page1.getReservations()).extracting(ReservationSummary::getReservationId) + .containsExactly("r1", "r2"); + assertThat(page1.getReservations().get(0).getCommittedMetadata()).isNull(); + String cursor = page1.getNextCursor(); + assertThat(cursor).isNotNull(); + + // Page 2: SAME cursor, but now opt into committed_metadata. The cursor + // must remain valid (no 400) and paging continues at r3/r4, with the + // newly-requested field projected onto this page. + ReservationListResponse page2 = repository.listReservations( + "acme", null, null, null, null, null, null, null, 2, cursor, + "created_at_ms", "asc", null, null, null, null, null, null, + EnumSet.of(ReservationInclude.COMMITTED_METADATA)); + assertThat(page2.getReservations()).extracting(ReservationSummary::getReservationId) + .containsExactly("r3", "r4"); + assertThat(page2.getReservations().get(0).getCommittedMetadata()) + .containsEntry("request_id", "req-r3"); + } + + private Map committedRow(String id, long createdAt) { + Map f = reservationFields(id, "COMMITTED"); + f.put("created_at", String.valueOf(createdAt)); + f.put("charged_amount", "4200"); + f.put("committed_metadata_json", "{\"request_id\":\"req-" + id + "\"}"); + return f; + } + @SuppressWarnings("unchecked") @Test @DisplayName("cursor reused with different sort_by → 400 INVALID_REQUEST")