feat(listReservations): add from/to time-window filter (v0.1.25.19, closes #159)

[amavashev]

Closes #159. Implements cycles-protocol-v0.yaml revision 2026-05-21 (runcycles/cycles-protocol#97, merged 2026-05-21).

Summary

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&from=…&to=… returns reservations created in the window, ordered by expiry. Pure additive wire change — clients that don't send the new params get the v0.1.25.18 response byte-for-byte.

Why

The old "fetch last 24h of reservations" workflow required clients to sort by created_at_ms and walk pages, doubling page size on each retry until the trailing row fell outside the window. For high-volume agent clusters that's O(N) for an O(window) operation. With server-side from/to, the scan is boundaried before hydration and cursor pagination over that window stays cursor-stable.

Implementation

• Controller (ReservationController.list) — parses ISO-8601 via Instant.parse. Malformed values → 400 INVALID_REQUEST (Invalid from: … / Invalid to: …). from > to → 400 before the repository call, with verify(repository, never()) locking that boundary. Blank strings treated as unset.
• Repository (RedisReservationRepository.listReservations, listReservationsSorted) — window predicate applied as a per-row filter in both the legacy SCAN-cursor path and the sorted path, after existing scope/status/tenant predicates and before hydration. Shared createdAtInWindow(fields, fromMs, toMs) helper. Missing/unparseable created_at rows defensively excluded when either bound is supplied.
• FilterHasher — hash(...) now takes trailing Long fromMs, Long toMs. They're folded into the canonical string after the eight existing string fields, so a cursor issued under one (from, to) returns 400 if re-used under a different window — 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). All Java callers updated. No wire format change.

Validation rules

Input	Result
from=not-a-date	400 INVALID_REQUEST, message Invalid from: …
to=not-a-date	400 INVALID_REQUEST, message Invalid to: …
from > to	400 INVALID_REQUEST, message from must be less than or equal to to. No repository call.
from alone	Open-upper-bound window
to alone	Open-lower-bound window
from == to	Closed point-window, accepted
from=&to= (blank)	Treated as unset; no 400
Row with missing/unparseable created_at, window active	Excluded
Row with missing/unparseable created_at, window inactive	Unchanged behavior (filter no-ops)

Coverage

537 tests pass (374 data + 163 api) under mvn test. 16 new tests:

• FilterHasherTest: 2 new (from/to distinct hash, positional swap).
• RedisReservationQueryTest: 7 new under TimeWindowFilter nested class — legacy path from/to/inclusive-bounds, sorted path window, cursor-mismatch-on-window-change, missing created_at defensive, unparseable created_at defensive.
• ReservationControllerTest: 7 new under ListReservations nested class — malformed-from, malformed-to, reversed-window, from-only, to-only, equal-bounds, sort-key independence, blank-as-unset.

Tests were run with -Dcontract.spec.url=file:///path/to/local/cycles-protocol-v0.yaml due to a local SSL cert quirk; CI hits the default raw.githubusercontent.com URL which now resolves to the merged spec carrying from/to.

Out of scope

The issue 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; leaving it as a follow-up to keep this change reviewable.

Test plan

• CI green (lint, build, full test suite against the merged spec)
• Manual smoke against a running server: curl '…/v1/reservations?from=…&to=…', then curl '…&from=…&to=…&sort_by=expires_at_ms'
• Reviewer sign-off on the AUDIT.md narrative + CHANGELOG entry
• After merge: cycles-client-{python,typescript,rust} regen pulls in the additive params (no breaking change)

[amavashev]

Code review

Found 2 issues:

1. FilterHasher canonical-form change breaks v0.1.25.18 sorted-path cursor continuity. The new code unconditionally appends |fr=…|to=… to the canonical string, even when both bounds are null. SHA-256 of t=acme|i=|st=|ws=|ap=|wf=|ag=|ts= (old) ≠ SHA-256 of t=acme|i=|st=|ws=|ap=|wf=|ag=|ts=|fr=|to= (new). Any SortedListCursor issued by v0.1.25.12–v0.1.25.18 carries the old 8-field hash; when re-presented to v0.1.25.19 — even with no from/to params — listReservationsSorted rejects it with 400 INVALID_REQUEST "cursor is not valid". This falsifies the AUDIT.md claim that v0.1.25.x clients omitting the new params continue byte-for-byte. Fix: only append |fr=…|to=… when at least one of fromMs / toMs is non-null, preserving the original 8-field canonical form for the no-window case.

cycles-server/cycles-protocol-service/cycles-protocol-service-data/src/main/java/io/runcycles/protocol/data/repository/support/FilterHasher.java

Lines 32 to 37 in b3d3d26

	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));
	try {

2. Three new controller tests assert nothing — wrong literal epoch-ms. 1779710400000L is 2026-05-25T12:00:00Z, not 2026-05-21T12:00:00Z (correct value is 1779364800000L, off by 4 days). In shouldPropagateFromOnly, shouldPropagateToOnly, and shouldAcceptEqualBounds, the Mockito eq(1779710400000L) matcher never fires against the controller's actual computed ms; the stub returns null; the controller returns ResponseEntity.ok(null) which is still HTTP 200; the tests pass vacuously without verifying epoch-ms propagation at all. Fix: replace 1779710400000L with 1779364800000L (or compute it from Instant.parse(...).toEpochMilli() in the test setup to eliminate the hard-coded literal entirely).

cycles-server/cycles-protocol-service/cycles-protocol-service-api/src/test/java/io/runcycles/protocol/api/controller/ReservationControllerTest.java

Lines 660 to 700 in b3d3d26

	@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());
	}

Also considered but filtered out (real but lower-severity): legacy-path cursor doesn't invalidate on filter change — pre-existing across all filters (status/workspace/etc), the PR follows the same pattern; stale revision 2026-04-16 Javadoc on listReservations — doc nit; from > to check fires before admin-tenant-required check — pedantic ordering of two unrelated 400s.

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}