Merge pull request #106 from runcycles/fix/reservations-sort-hydration-cap-v0.1.25.13

fix(reservations): hydration cap + enum wire annotations (v0.1.25.13)

Author: amavashev

AUDIT.md

@@ -1,6 +1,7 @@
 # Cycles Protocol v0.1.25 — Server Implementation Audit
 
-**Date:** 2026-04-16 (v0.1.25.12 — `sort_by` + `sort_dir` on `GET /v1/reservations` per cycles-protocol spec revision 2026-04-16; 7-value sort enum, opaque cursor binds `(sort_by, sort_dir, filters)` tuple, legacy SCAN-cursor path preserved when both params omitted),
+**Date:** 2026-04-16 (v0.1.25.13 — hydration cap + enum wire annotations on the sorted `GET /v1/reservations` path; `SORTED_HYDRATE_CAP=2000` guard on the in-memory sort hydration with WARN-on-cap, matches admin plane's v0.1.25.24 pattern; `@JsonValue`/`@JsonCreator fromWire` on `ReservationSortBy` + `SortDirection` to mirror admin's `SortSpec`/`SortDirection` contract),
+2026-04-16 (v0.1.25.12 — `sort_by` + `sort_dir` on `GET /v1/reservations` per cycles-protocol spec revision 2026-04-16; 7-value sort enum, opaque cursor binds `(sort_by, sort_dir, filters)` tuple, legacy SCAN-cursor path preserved when both params omitted),
 2026-04-14 (automated performance regression detection — nightly trend + release gate, no version bump),
 2026-04-14 (nightly soak test — long-duration stability coverage, no version bump),
 2026-04-14 (v0.1.25.11 — concurrent retry-storm test for idempotency cache expiry + concurrent accuracy test for custom counters; closes two gaps flagged in the v0.1.25.10 review),
@@ -18,6 +19,29 @@
 
 ---
 
+### 2026-04-16 — v0.1.25.13: hydration cap + enum wire annotations on the sorted list path
+
+Closes two follow-up gaps surfaced by the three-step review of v0.1.25.12 against the admin-plane implementation of the same feature in `cycles-server-admin` v0.1.25.24:
+
+**P1 — `listReservationsSorted` hydrates an unbounded population before the in-memory sort.** The sorted path in v0.1.25.12 does a full `SCAN` of `reservation:res_*`, hydrates every matching row into a `List<ReservationSummary>`, then sorts + slices. For a tenant with 2M reservations under a single workspace, that's 2M `HGETALL` round-trips and a 2M-entry heap object before the cursor walk even starts. The admin plane ran into exactly the same shape on `ApiKeyRepository.listAllTenantsSorted` and `BudgetRepository.listAllTenantsSorted` and closed it with a `SORTED_HYDRATE_CAP = 2000` constant: when the per-key hydration loop observes `matching.size() >= cap` it sets `capped = true`, breaks out of the SCAN loop, logs a WARN naming the tenant + sort tuple, and the sort/slice/cursor code then operates on the capped slice as normal. Page still fills, `has_more` + `next_cursor` still populate from the capped slice, so the UI isn't blocked — the cap is a heap-safety bound, not a correctness bound.
+
+This release ports the same constant + `scanLoop:` labeled-break + post-loop WARN to `RedisReservationRepository.listReservations` on the sorted path only. The legacy (no-sort-params, no-decoded-sorted-cursor) path is intentionally uncapped because it streams page-by-page via the SCAN cursor and never builds an unbounded in-memory list.
+
+**Why 2000:** Same rationale as admin — covers 99%+ of production tenants, keeps the heap bound predictable at roughly 2000 × ~2 KB ReservationSummary = ~4 MB per concurrent sorted-list call. Operators who outgrow the cap should either (a) narrow filters (`status`, `idempotency_key`, scope segments `workspace`/`app`/`workflow`/`agent`/`toolset`), or (b) revisit the deferred per-key ZSET index ADR at `docs/deferred-optimizations/sorted-list-zset-indices.md`.
+
+**P2 — `ReservationSortBy` and `SortDirection` lacked Jackson wire annotations.** In v0.1.25.12 the enums were plain Java enums; the controller did manual `String.toUpperCase()` → `Enum.valueOf()` conversion with a try/catch → 400. That works, but it diverges from the admin plane where `SortSpec` and `SortDirection` carry `@JsonValue getWire()` + `@JsonCreator fromWire(String)` so Jackson handles the lowercase-on-wire contract natively and controllers delegate to `fromWire(...)` with the same try/catch → 400 shape. This release brings the two runtime-plane enums into line: lowercase `getWire()`, `null → null` on `fromWire(null)`, `IllegalArgumentException` on unknown tokens (which the controller converts to 400 with the documented allow-list payload). The wire contract is unchanged — lowercase tokens in and out — but direct JSON-over-wire uses of these enums (future list-response DTOs that echo the sort tuple back, for instance) now serialize/deserialize correctly without custom converters.
+
+**Test strategy:**
+
+- `RedisReservationQueryTest#sortedHydrationStopsAtCap` — mocks a SCAN page returning `cap + 10` keys, stubs pipeline `hgetAll` for each, invokes the 5-row sorted page, asserts exactly 5 rows in the documented ascending-`created_at_ms` order and `has_more=true`, `next_cursor != null`. Exercises the capped-slice cursor path. (The full hydration loop consults only up to `cap` rows; remaining stubs are unused as designed.)
+- `EnumsTest` (new, `cycles-protocol-service-model`) — round-trip tests for both enums: `getWire()` emits lowercase, `fromWire` accepts canonical lowercase + uppercase + mixed case, `fromWire(null)` returns null, `fromWire("bogus")` throws `IllegalArgumentException`, `for each value: fromWire(getWire(v)) == v`.
+
+No spec change; wire format is identical. No signature changes. No caller-visible behaviour change for tenants under the cap.
+
+**Backward compatibility:** Full. Behaviour-visible for tenants whose sorted-list query previously returned >2000 matching rows (silently; those were already on the O(N) full-SCAN path documented in v0.1.25.12). Post-cap, rows beyond row 2000 in the capped slice are unreachable without narrowing filters — same contract the admin plane established in v0.1.25.24 for `ApiKey` + `Budget` cross-tenant sorted paths.
+
+---
+
 ### 2026-04-16 — v0.1.25.12: `sort_by` + `sort_dir` on GET /v1/reservations
 
 Closes the runtime-protocol gap opened by **cycles-protocol spec revision 2026-04-16** (commits `064e95f` + `a2a8f13`): list-reservations needed server-side ordering with client-selectable sort key + direction, and the cursor needed to encode the sort state so page breaks remain consistent.

CHANGELOG.md

@@ -14,6 +14,42 @@ 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.13] — 2026-04-16
+
+### Fixed
+
+- `GET /v1/reservations` sorted path no longer hydrates an unbounded
+  reservation population before the in-memory sort. A
+  `SORTED_HYDRATE_CAP = 2000` guard mirrors the admin-plane pattern
+  shipped in `cycles-server-admin` v0.1.25.24: once the cap is hit
+  the SCAN loop breaks, a WARN is logged so operators can see the
+  window was truncated, and the cursor page still fills from the
+  capped slice. Callers that need to see past the cap should narrow
+  filters (`status`, `idempotency_key`, `workspace`/`app`/
+  `workflow`/`agent`/`toolset`) — same workaround doc pattern as
+  admin.
+
+### Internal
+
+- `Enums.ReservationSortBy` and `Enums.SortDirection` now carry
+  `@JsonValue getWire()` + `@JsonCreator fromWire(String)` Jackson
+  annotations matching the admin plane's `SortSpec` /
+  `SortDirection` pattern. Wire form stays lowercase, parsing stays
+  case-insensitive with `null → null`. Controller-level validation
+  is unchanged: unknown tokens still surface as HTTP 400
+  `INVALID_REQUEST` with the documented allow-list payload.
+- `RedisReservationRepository.SORTED_HYDRATE_CAP` is package-private
+  (test-visible) to allow cap-hit tests to assert the bound
+  deterministically.
+
+### Notes for upgraders
+
+Behavior-visible for callers that previously relied on the sorted
+path silently returning all rows even for a single tenant with
+thousands of reservations: the page shape is identical, but rows
+beyond row 2000 in the capped slice are now unreachable without
+narrowing filters. Same trade-off doc as the admin plane cap.
+
 ## [0.1.25.12] — 2026-04-16
 
 ### Added

README.md

@@ -117,7 +117,7 @@ mvn clean install
 ./build-all.sh
 ```
 
-The fat JAR is produced at `cycles-protocol-service-api/target/cycles-protocol-service-api-<version>.jar` (where `<version>` is the `revision` property in `cycles-protocol-service/pom.xml` — e.g. `0.1.25.12`).
+The fat JAR is produced at `cycles-protocol-service-api/target/cycles-protocol-service-api-<version>.jar` (where `<version>` is the `revision` property in `cycles-protocol-service/pom.xml` — e.g. `0.1.25.13`).
 
 ## Docker Deployment
 
@@ -136,7 +136,7 @@ Pre-built images are published to GitHub Container Registry on each release:
 
 ```
 ghcr.io/runcycles/cycles-server:latest
-ghcr.io/runcycles/cycles-server:<version>    # e.g. 0.1.25.12
+ghcr.io/runcycles/cycles-server:<version>    # e.g. 0.1.25.13
 ```
 
 ## Testing

cycles-protocol-service/cycles-protocol-service-api/src/main/java/io/runcycles/protocol/api/controller/ReservationController.java

@@ -256,19 +256,21 @@ public ResponseEntity<ReservationListResponse> list(
         // v0.1.25.12 (cycles-protocol revision 2026-04-16): validate
         // sort_by / sort_dir at the controller boundary so clients get
         // a clean 400 INVALID_REQUEST on typos before the repo runs.
-        // Enum values on the wire are lowercase (sort_by=created_at_ms);
-        // uppercase them to match the Java enum constant.
+        // v0.1.25.13: delegate parsing to Enums.*.fromWire (adds @JsonValue /
+        // @JsonCreator round-trip), which is case-insensitive and throws
+        // IllegalArgumentException on unknown values — caught and mapped to
+        // the same 400 payload as before.
         if (sortBy != null) {
             try {
-                Enums.ReservationSortBy.valueOf(sortBy.toUpperCase());
+                Enums.ReservationSortBy.fromWire(sortBy);
             } catch (IllegalArgumentException e) {
                 throw new CyclesProtocolException(Enums.ErrorCode.INVALID_REQUEST,
                     "Invalid sort_by: " + sortBy + ". Must be one of: reservation_id, tenant, scope_path, status, reserved, created_at_ms, expires_at_ms", 400);
             }
         }
         if (sortDir != null) {
             try {
-                Enums.SortDirection.valueOf(sortDir.toUpperCase());
+                Enums.SortDirection.fromWire(sortDir);
             } catch (IllegalArgumentException e) {
                 throw new CyclesProtocolException(Enums.ErrorCode.INVALID_REQUEST,
                     "Invalid sort_dir: " + sortDir + ". Must be one of: asc, desc", 400);

cycles-protocol-service/cycles-protocol-service-data/src/main/java/io/runcycles/protocol/data/repository/RedisReservationRepository.java

@@ -47,6 +47,21 @@ public class RedisReservationRepository {
         .map(Enums.UnitEnum::name)
         .collect(Collectors.joining(","));
 
+    /**
+     * v0.1.25.13 hydration cap for the sorted listReservations path. The sorted
+     * path must full-SCAN every `reservation:res_*` key across all tenants
+     * before the in-memory sort, which is unbounded in the naive case. At
+     * runtime scale (10k reservations per active tenant) this can exhaust
+     * heap before the cursor even runs. Cap at 2000 rows per request; when
+     * the cap is hit we break out of hydration, sort the capped slice, and
+     * serve the cursor from that slice. Operators that need to see beyond
+     * the cap should narrow filters (status, scope segments, tenant) — the
+     * deferred-optimization ADR (docs/deferred-optimizations/
+     * sorted-list-zset-indices.md) tracks the longer-term per-tenant ZSET
+     * index work that will retire this cap.
+     */
+    static final int SORTED_HYDRATE_CAP = 2000;
+
     private static final ThreadLocal<MessageDigest> SHA256_DIGEST = ThreadLocal.withInitial(() -> {
         try {
             return MessageDigest.getInstance("SHA-256");
@@ -660,6 +675,8 @@ private ReservationListResponse listReservationsSorted(
             String toolsetSegment = toolset != null ? "toolset:" + toolset.toLowerCase() : null;
 
             String scanCursor = "0";
+            boolean capped = false;
+            scanLoop:
             do {
                 ScanResult<String> scan = jedis.scan(scanCursor, params);
                 List<String> keys = scan.getResult();
@@ -672,6 +689,7 @@ private ReservationListResponse listReservationsSorted(
                     pipeline.sync();
 
                     for (String key : keys) {
+                        if (matching.size() >= SORTED_HYDRATE_CAP) { capped = true; break scanLoop; }
                         try {
                             Map<String, String> fields = responses.get(key).get();
                             if (fields.isEmpty()) continue;
@@ -694,6 +712,11 @@ private ReservationListResponse listReservationsSorted(
                 scanCursor = scan.getCursor();
             } while (!"0".equals(scanCursor));
 
+            if (capped) {
+                LOG.warn("listReservationsSorted hydration capped at {} rows for tenant={} sort_by={} sort_dir={}; narrow filters to see beyond the cap",
+                    SORTED_HYDRATE_CAP, tenant, effectiveSortBy, effectiveSortDir);
+            }
+
             // Full in-memory sort. Acceptable at runtime-plane scale; metric below lets us
             // spot tenants that outgrow this path and need per-key ZSET indices.
             matching.sort(ReservationComparators.of(effectiveSortBy, effectiveSortDir));

cycles-protocol-service/cycles-protocol-service-data/src/test/java/io/runcycles/protocol/data/repository/RedisReservationQueryTest.java

@@ -836,6 +836,60 @@ void cursorMismatchRejected() {
                 .hasMessageContaining("cursor is not valid");
         }
 
+        @SuppressWarnings("unchecked")
+        @Test
+        @DisplayName("hydration stops at SORTED_HYDRATE_CAP; page still fills from capped slice")
+        void sortedHydrationStopsAtCap() {
+            when(jedisPool.getResource()).thenReturn(jedis);
+            doNothing().when(jedis).close();
+
+            int cap = RedisReservationRepository.SORTED_HYDRATE_CAP;
+            int total = cap + 10;
+
+            // Return ALL keys from a single SCAN page. The hydration guard sits
+            // inside the per-key loop, so the break exits after exactly `cap`
+            // rows are added — remaining keys are never consulted even though
+            // they're still in the scan result.
+            List<String> keys = new ArrayList<>(total);
+            for (int i = 0; i < total; i++) {
+                keys.add(String.format("reservation:res_r%05d", i));
+            }
+
+            ScanResult<String> scanResult = mock(ScanResult.class);
+            when(scanResult.getResult()).thenReturn(keys);
+            // getCursor() is never read once the hydration cap breaks out of the
+            // labeled scanLoop; stub leniently so strict-mode doesn't flag it.
+            lenient().when(scanResult.getCursor()).thenReturn("0");
+            when(jedis.scan(eq("0"), any(ScanParams.class))).thenReturn(scanResult);
+            when(jedis.pipelined()).thenReturn(pipeline);
+
+            // Default pipeline.hgetAll stub from Base returns empty map; override
+            // for the first `cap` keys so they pass the tenant filter and land
+            // in the matching list. Keys beyond index `cap` are stubbed too, so
+            // if the guard is ever removed the test fails loud (page fills past
+            // the cap and the pipeline verifier trips).
+            for (int i = 0; i < total; i++) {
+                String id = String.format("r%05d", i);
+                Map<String, String> f = reservationFields(id, "ACTIVE");
+                f.put("created_at", String.valueOf(1_700_000_000_000L + i));
+                Response<Map<String, String>> resp = mock(Response.class);
+                lenient().when(resp.get()).thenReturn(f);
+                lenient().when(pipeline.hgetAll("reservation:res_" + id)).thenReturn(resp);
+            }
+
+            ReservationListResponse response = repository.listReservations(
+                "acme", null, null, null, null, null, null, null, 5, null,
+                "created_at_ms", "asc");
+
+            assertThat(response.getReservations()).hasSize(5);
+            assertThat(response.getReservations())
+                .extracting(ReservationSummary::getReservationId)
+                .containsExactly("r00000", "r00001", "r00002", "r00003", "r00004");
+            // has_more must be true — the capped slice still has rows beyond the page.
+            assertThat(response.getHasMore()).isTrue();
+            assertThat(response.getNextCursor()).isNotNull();
+        }
+
         @SuppressWarnings("unchecked")
         @Test
         @DisplayName("legacy numeric cursor with no sort params routes to legacy SCAN path")

cycles-protocol-service/cycles-protocol-service-model/src/main/java/io/runcycles/protocol/model/Enums.java

@@ -1,5 +1,8 @@
 package io.runcycles.protocol.model;
 
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonValue;
+
 /**
  * Cycles Protocol v0.1.25
  * All enums in one file for convenience
@@ -76,6 +79,13 @@ public enum ErrorCode {
      * Sort keys accepted by GET /v1/reservations.
      * Spec: cycles-protocol-v0.yaml revision 2026-04-16.
      * Default when sort_by is provided but unset on the wire: CREATED_AT_MS.
+     *
+     * Wire form is lowercase (reservation_id, created_at_ms, …). @JsonValue
+     * emits the wire form on serialization; @JsonCreator fromWire parses
+     * case-insensitively and returns null on null input so callers that
+     * want 400-on-unknown can convert IllegalArgumentException into a
+     * CyclesProtocolException themselves, matching the admin SortDirection
+     * pattern.
      */
     public enum ReservationSortBy {
         RESERVATION_ID,
@@ -84,11 +94,37 @@ public enum ReservationSortBy {
         STATUS,
         RESERVED,
         CREATED_AT_MS,
-        EXPIRES_AT_MS
+        EXPIRES_AT_MS;
+
+        @JsonValue
+        public String getWire() {
+            return name().toLowerCase();
+        }
+
+        @JsonCreator
+        public static ReservationSortBy fromWire(String value) {
+            if (value == null) return null;
+            return ReservationSortBy.valueOf(value.toUpperCase());
+        }
     }
 
-    /** Sort direction for list endpoints. Default DESC. */
+    /**
+     * Sort direction for list endpoints. Default DESC. Wire form is
+     * lowercase ("asc" / "desc"). See ReservationSortBy for the Jackson
+     * round-trip contract — same pattern.
+     */
     public enum SortDirection {
-        ASC, DESC
+        ASC, DESC;
+
+        @JsonValue
+        public String getWire() {
+            return name().toLowerCase();
+        }
+
+        @JsonCreator
+        public static SortDirection fromWire(String value) {
+            if (value == null) return null;
+            return SortDirection.valueOf(value.toUpperCase());
+        }
     }
 }