Merge pull request #104 from runcycles/feat/reservations-sort-by-sort-dir-v0.1.25.12

feat(reservations): sort_by + sort_dir on GET /v1/reservations (v0.1.25.12)

Author: amavashev

AUDIT.md

@@ -1,6 +1,7 @@
 # Cycles Protocol v0.1.25 — Server Implementation Audit
 
-**Date:** 2026-04-14 (automated performance regression detection — nightly trend + release gate, no version bump),
+**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),
+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),
 2026-04-14 (v0.1.25.10 — custom Micrometer counters for reserve/commit/release/extend/expired/events + overdraft, plus Redis-disconnect resilience test; dormant emitExpiredEvent key-prefix bug fixed as a side effect),
@@ -17,6 +18,78 @@
 
 ---
 
+### 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.
+
+**Spec shape:**
+
+- `sort_by` enum (7 values): `reservation_id`, `tenant`, `scope_path`, `status`, `reserved`, `created_at_ms`, `expires_at_ms`.
+- `sort_dir` enum: `asc`, `desc`. Defaults to `desc` when `sort_by` is provided. When both are omitted, legacy behaviour (Redis-SCAN arbitrary order) is preserved exactly — zero-risk to existing clients.
+- Invalid enum values → HTTP 400 `INVALID_REQUEST` with the bad token echoed in the message.
+- Cursors MUST bind to `(sort_by, sort_dir, filters)`; mismatched reuse → HTTP 400.
+
+**Implementation shape — dual-path:**
+
+Existing code uses Redis `SCAN` to page `reservation:res_*` keys. SCAN returns keys in arbitrary, cursor-coupled order, which is fundamentally incompatible with server-side sorting. The alternative — per-tenant ZSET indices per sort key — would have required new Lua scripts, dual-write paths on every reservation state transition, and a backfill migration. Disproportionate to runtime-plane scale (per-tenant N typically ≤ 10³; O(N) in-memory sort per sorted page is cheaper than index maintenance).
+
+So the controller branches:
+
+- **Legacy path** (no `sort_by` AND no `sort_dir` AND no sorted cursor) — unchanged `SCAN`/pipelined `HGETALL`/opaque-cursor loop. Byte-for-byte identical to v0.1.25.11.
+- **Sorted path** (either sort param OR a decoded sorted cursor present) — full `SCAN` pass with filter predicates applied in-stream, deterministic in-memory `Comparator` sort, opaque slice cursor.
+
+**New types (`cycles-protocol-service-data/.../repository/support/`):**
+
+- `ReservationComparators.of(sortBy, sortDir)` — per-key extractors + `.thenComparing(reservation_id ASC)` tiebreaker so pagination boundaries are unambiguous under ties. Null-safe via `Comparator.nullsLast`. Also exposes `extractSortValue(ReservationSummary, sortBy)` for cursor `lsv` encoding.
+- `FilterHasher.hash(t, i, st, ws, ap, wf, ag, ts)` — SHA-256 of canonical `k=v|k=v|...` over eight filter fields, first 16 hex chars (8-byte truncation). Not a security boundary — sole job is cheap detection of cross-tuple cursor reuse. Trades length for brevity in the base64url cursor payload.
+- `SortedListCursor` record `{v:int, sb, sd, fh, lsv, lrid}` — v=1, base64url-no-pad Jackson JSON. `decode(String)` returns `Optional.empty()` on null/blank/all-digit input, which routes old numeric cursors straight to the legacy SCAN path — backward-compat kept at the cursor-parsing boundary rather than the controller boundary.
+
+**Repository change:**
+
+`RedisReservationRepository.listReservations` signature extended with trailing `String sortBy, String sortDir` (10 → 12 args). The 10-arg overload was removed intentionally — keeping both caused Mockito stubs defined over the 10-arg form to fail to match 12-arg call-sites from the updated controller, which silently made unit tests pass with `null` responses. Single explicit signature surfaces the contract at the type level.
+
+New private helpers on the repository:
+
+- `listReservationsSorted(...)` — full SCAN pass (no early termination), pipelined `HGETALL`, status/scope/filter predicates applied as rows stream through, in-memory sort via the comparator, opaque slice cursor emitted when `idx + limit < total`.
+- `findSliceStart(rows, cursor)` — binary-search-like boundary finder over the sorted list; returns the first index strictly greater than `(lsv, lrid)` per the active comparator.
+- `compareAtBoundary(...)` — honours sort direction so boundary comparison matches emitted order (desc cursor page-forward goes to smaller `lsv`, not larger).
+
+**Cross-tuple cursor rejection:**
+
+On resume, the decoded cursor's `(sb, sd, fh)` is compared to the current request's derived tuple. Mismatch throws `CyclesProtocolException(INVALID_REQUEST, ..., 400)`. Prevents the class of bug where a client paginates by `created_at_ms asc`, changes to `reserved desc` on the UI, re-submits the old cursor, and silently gets a corrupt mid-stream slice.
+
+**Controller validation (at `ReservationController.list`):**
+
+Matches the existing `status` validation pattern at line 246 — uppercase the incoming parameter, parse against the enum, throw `CyclesProtocolException(INVALID_REQUEST, "sort_by must be one of ...", 400)` on failure. No Spring `@Valid` involved; intentional — error-body shape and message wording must stay under cycles-protocol error-envelope control, not Jackson's default.
+
+**Tests (≥95 % coverage preserved):**
+
+- `ReservationControllerTest` — 4 new: invalid `sort_by` → 400, invalid `sort_dir` → 400, both params propagated to repo (argument captor), all 7 spec enum values accepted. Pre-existing Mockito stubs updated from 10-arg to 12-arg (`, any(), any()`) to match the new repository signature.
+- `ReservationComparatorsTest` (new) — per-field asc/desc for all 7 sort keys, `reservation_id` tiebreaker under ties, null-subject-tenant safety under both directions, `extractSortValue` correctness.
+- `SortedListCursorTest` (new) — round-trip encode/decode, malformed base64url → empty Optional (legacy fallback), digit-only input → empty Optional, JSON with missing fields → empty Optional.
+- `FilterHasherTest` (new) — determinism, null-vs-empty-string equivalence (trailing empties collapse so clients can omit the trailing filters without forcing a new tuple), 16-hex-char output shape.
+- `RedisReservationQueryTest.SortedListReservationsTest` (new `@Nested`) — Testcontainers Redis seeded with 25 reservations across statuses and timestamps; paginates `sort_by=created_at_ms&sort_dir=asc&limit=10` across 3 pages with `has_more` transitions checked; cursor reused under different `sort_by` throws; legacy numeric cursor still works without sort params; scope_path lexicographic ordering verified.
+- Pre-existing `listReservations` call-sites (~12 across the Testcontainers file) updated from 10-arg to 12-arg.
+
+**Release bookkeeping:**
+
+- `cycles-protocol-service/pom.xml` `<revision>` → `0.1.25.12`.
+- Both prod docker-compose image pins bumped: `docker-compose.prod.yml`, `docker-compose.full-stack.prod.yml`.
+- Six-doc markdown matrix updated: AUDIT.md (this entry), CHANGELOG.md, OPERATIONS.md, BENCHMARKS.md, README.md × 2.
+- Benchmarks unaffected (sort path is off for existing workloads because clients haven't added the params yet); release can use the `[benchmark-skip]` marker in the GH release notes body. Documented in CHANGELOG.
+
+**Verification:**
+
+- `mvn -B test --file cycles-protocol-service/pom.xml -Dtest='!*IntegrationTest'` — 352 data-module tests + 344 API-module tests green.
+- Testcontainers Redis integration tests exercise the sort-path cursor round-trip.
+
+**Out of scope (deferred):**
+
+- Per-tenant ZSET indices by sort key — follow-up if any tenant exceeds ~10 k reservations and sorted-list latency crosses a service-level objective.
+- Admin spec `/v1/auth/introspect` (cycles-governance-admin-v0.1.25.yaml revisions `101416f` / `6aca3f9`) — not a runtime-plane concern.
+
+---
+
 ### 2026-04-14 — Automated performance regression detection (no version bump)
 
 Closes the last remaining gap in the v0.1.25.11 scorecard: **performance regression detection was 6/10** because BENCHMARKS.md tracked trends but nothing failed automatically on a regression. A 2× p99 slowdown could merge, tag, and ship through the release workflow with only human review to catch it. This change adds an automated gate.

BENCHMARKS.md

@@ -21,6 +21,14 @@ running them would only measure environmental noise. Skipped releases:
   increments on success/failure paths; no Redis/Lua changes). The
   `ReservationExpiryService` prefix fix is off the request hot path
   (runs in the background sweep).
+- **v0.1.25.11** — test-only release (concurrent-retry / counter-
+  accuracy regression tests).
+- **v0.1.25.12** — `sort_by` + `sort_dir` on `GET /v1/reservations`
+  list endpoint. The sorted path is opt-in (clients must pass the
+  new params to activate it); legacy list behaviour is byte-for-byte
+  unchanged and all existing benchmarks exercise the legacy path.
+  Benchmarks for the sorted path are worth adding once real tenant
+  populations exercise the O(N) full-SCAN; see OPERATIONS.md.
 
 Last benchmarked release: **v0.1.25.7**.
 

CHANGELOG.md

@@ -14,6 +14,45 @@ 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.12] — 2026-04-16
+
+### Added
+
+- `GET /v1/reservations` now accepts `sort_by` and `sort_dir` query
+  parameters (cycles-protocol spec revision 2026-04-16). Valid
+  `sort_by` values: `reservation_id`, `tenant`, `scope_path`,
+  `status`, `reserved`, `created_at_ms`, `expires_at_ms`. Valid
+  `sort_dir` values: `asc`, `desc` (default `desc` when `sort_by`
+  is provided). Invalid enum values return HTTP 400
+  `INVALID_REQUEST`.
+- Server-side ordering with deterministic `reservation_id ASC`
+  tiebreaker so pagination is unambiguous under ties.
+- Opaque sorted cursor (base64url-no-pad JSON, `{v,sb,sd,fh,lsv,lrid}`)
+  that binds to the `(sort_by, sort_dir, filters)` tuple via an
+  8-byte SHA-256 filter hash. Reusing a cursor under a different
+  tuple returns HTTP 400.
+
+### Wire format
+
+Backward compatible. Omitting `sort_by`/`sort_dir` preserves the
+existing Redis-SCAN cursor semantics; legacy all-digit cursors
+continue to work unchanged.
+
+### Internal
+
+- New `support.SortedListCursor`, `support.FilterHasher`,
+  `support.ReservationComparators` utilities.
+- `RedisReservationRepository.listReservations` signature extended
+  with trailing `sortBy`, `sortDir` parameters (10 → 12 args).
+  Direct callers inside the service have been updated; external
+  callers (if any) must add two trailing `null`s.
+
+### Notes for upgraders
+
+No action required for clients that don't use the new parameters.
+Ops teams monitoring sorted-list query latency should watch for
+the documented O(N) full-SCAN behaviour — see OPERATIONS.md.
+
 ## [0.1.25.11] — 2026-04-14
 
 ### Added

OPERATIONS.md

@@ -366,6 +366,28 @@ don't fit.
 | `admin.api-key` | (empty) | Set to a fixed-length secret to enable the admin-on-behalf-of endpoint (v0.1.25.8+). Leave empty to disable. |
 | `management.endpoints.web.exposure.include` | `health,info,prometheus` | Add more actuator endpoints if you need them, but `prometheus` is the one ops cares about. |
 
+## Reservation list sorting (v0.1.25.12+)
+
+`GET /v1/reservations` accepts `sort_by` (one of `reservation_id`,
+`tenant`, `scope_path`, `status`, `reserved`, `created_at_ms`,
+`expires_at_ms`) and `sort_dir` (`asc` or `desc`, default `desc`).
+
+Implementation: full-SCAN + in-memory sort per sorted page. This is
+**O(N)** in reservations matching the filter — fine at the current
+runtime-plane target of ≤ 10³ reservations per tenant. Watch the
+Spring Boot `http_server_requests_seconds{uri="/v1/reservations"}`
+p99: if it climbs above 500 ms under real load, a tenant has grown
+past the in-memory threshold and per-tenant ZSET indexing becomes
+worthwhile. Track the top `tenant` tag on that metric to spot who.
+
+Legacy clients (no sort params) stay on the existing Redis-SCAN
+cursor path and are unaffected by this concern.
+
+Cursors encode the `(sort_by, sort_dir, filters)` tuple — reusing
+a cursor with a different sort or filter set returns HTTP 400
+`INVALID_REQUEST`. This is intentional; front-end code that mutates
+the sort/filter on a page change must reset to the first page.
+
 ## Getting help
 
 - Bug reports / feature requests:

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.11`).
+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`).
 
 ## 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.11
+ghcr.io/runcycles/cycles-server:<version>    # e.g. 0.1.25.12
 ```
 
 ## Testing

cycles-protocol-service/README.md

@@ -456,6 +456,14 @@ List reservations for the effective tenant. Optional recovery/debug endpoint. Re
 | `workspace` / `app` / `workflow` / `agent` / `toolset` | Subject field filters |
 | `limit` | Max results per page (default `50`, max `200`) |
 | `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. |
+
+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.
 
 **Response** `200 OK`
 ```json

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

@@ -241,7 +241,9 @@ public ResponseEntity<ReservationListResponse> list(
             @RequestParam(required = false) String agent,
             @RequestParam(required = false) String toolset,
             @RequestParam(defaultValue = "50") @Min(1) @Max(200) int limit,
-            @RequestParam(required = false) String cursor) {
+            @RequestParam(required = false) String cursor,
+            @RequestParam(value = "sort_by", required = false) String sortBy,
+            @RequestParam(value = "sort_dir", required = false) String sortDir) {
         // Validate status against ReservationStatus enum if provided
         if (status != null) {
             try {
@@ -251,6 +253,27 @@ public ResponseEntity<ReservationListResponse> list(
                     "Invalid status filter: " + status + ". Must be one of: ACTIVE, COMMITTED, RELEASED, EXPIRED", 400);
             }
         }
+        // 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.
+        if (sortBy != null) {
+            try {
+                Enums.ReservationSortBy.valueOf(sortBy.toUpperCase());
+            } 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());
+            } catch (IllegalArgumentException e) {
+                throw new CyclesProtocolException(Enums.ErrorCode.INVALID_REQUEST,
+                    "Invalid sort_dir: " + sortDir + ". Must be one of: asc, desc", 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.
@@ -271,8 +294,9 @@ public ResponseEntity<ReservationListResponse> list(
             effectiveTenant = tenant != null ? tenant : extractAuthTenantId();
             authorizeTenant(effectiveTenant);
         }
-        LOG.info("GET /v1/reservations - tenant: {}, admin: {}", effectiveTenant, isAdminAuth());
+        LOG.info("GET /v1/reservations - tenant: {}, admin: {}, sort_by: {}, sort_dir: {}",
+                effectiveTenant, isAdminAuth(), sortBy, sortDir);
         return ResponseEntity.ok(repository.listReservations(effectiveTenant, idempotencyKey,
-                status, workspace, app, workflow, agent, toolset, limit, cursor));
+                status, workspace, app, workflow, agent, toolset, limit, cursor, sortBy, sortDir));
     }
 }