feat(listReservations): add from/to created-time range filters#97
Merged
Conversation
Closes runcycles/cycles-server#159. Adds two optional query parameters to 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 + time-window-by-created_at_ms is well-defined. Naming matches the family-wide convention already in use on listAuditLogs, listEvents, listWebhookDeliveries, listTenantEvents, and listTenantWebhookDeliveries (all `from`/`to` + format: date-time). Bespoke names (`created_after`/`created_before`) or Unix-epoch wire types would split the convention for clients and codegen. Validation: * `from > to` MUST return 400 INVALID_REQUEST. * Either alone is valid; absent means "no bound on that side." * Malformed date-time values MUST return 400. Additive-parameter guarantee: servers that don't recognize the parameters MUST ignore without error and return the unfiltered set. Backward compatible — purely additive, no request or response schema changes, both ApiKeyAuth and AdminKeyAuth callers see them. Spec changes: - cycles-protocol-v0.yaml: two new query params on listReservations plus a new TIME-RANGE FILTERS (NORMATIVE, ADDITIVE) prose block in the operation description. - changelogs/cycles-protocol-v0.md: new v0.1.25 — 2026-05-21 entry. - CONFORMANCE.md: listReservations SHOULD bullet updated to mention the new time-window query path. info.version stays at 0.1.25 per the existing revision convention (multiple dated v0.1.25 entries already coexist in the changelog). Verification: - npx spectral lint cycles-protocol-v0.yaml --fail-severity=error → 0 errors. 20 pre-existing warnings on top-level schema descriptions, unchanged from main. - python scripts/validate_changelogs.py → all 5 specs OK. Opening as DRAFT pending issue author confirmation on the `from`/`to` naming and five open questions raised in runcycles/cycles-server#159 (comment 4508958838): inclusivity, from>to=400, open intervals, sort_by binding, rationale nit on the expires_at use case.
Companion to a510f29 — the merge-check CI step (`make merge-check` / `python scripts/merge_specs.py` + git-diff guard on `merged/`) fails when a source spec is modified without rerunning the merge script. This commit picks up the `from`/`to` query params on listReservations and the TIME-RANGE FILTERS prose block in the merged protocol artifact. No semantic changes beyond what's already in cycles-protocol-v0.yaml — pure mechanical regeneration via scripts/merge_specs.py. Verified: - `python scripts/merge_specs.py` writes both merged artifacts; only cycles-openapi-protocol-merged.yaml changed (admin merge already excludes the runtime base). - `git diff merged/` shows the same TIME-RANGE FILTERS block and two new query params, just propagated through the merge. - `npx spectral lint merged/cycles-openapi-protocol-merged.yaml --fail-severity=error` → 0 errors (pre-existing warnings only).
4 tasks
amavashev
added a commit
to runcycles/cycles-server
that referenced
this pull request
May 21, 2026
Closes #159. Implements cycles-protocol-v0.yaml revision 2026-05-21 (runcycles/cycles-protocol#97). Two new optional query parameters on GET /v1/reservations: * from: ISO 8601 date-time. Inclusive lower bound on created_at_ms. * to: ISO 8601 date-time. Inclusive upper bound on created_at_ms. Either may be supplied alone (open interval) or together (closed window). The filter always binds to created_at_ms regardless of sort_by, so sort-by-expires_at_ms + window-by-created_at_ms is well-defined. Implementation: * Controller: parses ISO-8601 via Instant.parse; malformed → 400 INVALID_REQUEST; from > to → 400 before any repository call; blank-string values treated as unset. * Repository: window predicate applied in both legacy SCAN-cursor and sorted paths. Predicate body shared via createdAtInWindow helper. Missing/unparseable created_at rows defensively excluded when either bound is supplied. * FilterHasher: fromMs/toMs folded into canonical hash so cursors invalidate on window change (same contract as v0.1.25.12 sort_by/sort_dir/filter-mismatch). * Java signature change: listReservations gains trailing Long fromMs, Long toMs (12 → 14 args). No wire change for clients that omit the new params. Coverage: * FilterHasherTest: 2 new cases (from/to distinct hash, positional). * RedisReservationQueryTest: 7 new cases under TimeWindowFilter nested class. * ReservationControllerTest: 7 new cases under ListReservations nested class (malformed-from/to, reversed-window, from-only, to-only, equal-bounds, sort-key independence, blank-as-unset). * 537 tests pass (374 data + 163 api). Docs: * AUDIT.md: new dated entry walks through naming, sort-binding, cursor invalidation, validation choices, defensive read-side, coverage, and out-of-scope notes (expires_at follow-up). * CHANGELOG.md: [0.1.25.19] entry under Keep-a-Changelog format. * pom.xml revision bumped 0.1.25.18 → 0.1.25.19.
This was referenced May 21, 2026
Merged
amavashev
added a commit
to runcycles/cycles-client-typescript
that referenced
this pull request
May 21, 2026
…0.3.2) (#103) Client-side companion to cycles-protocol-v0.yaml revision 2026-05-21 (runcycles/cycles-protocol#97) and runcycles/cycles-server#160. Closes the TypeScript-client side of issue #159. The existing `listReservations(params?: Record<string, string>)` signature already forwards arbitrary keys to the URL query string, so the new `from` and `to` ISO-8601 date-time params work over the wire today without a code change. This commit adds a regression test that pins the contract: URL-encoded form "from=2026-05-21T00%3A00%3A00Z&to=2026-05-22T00%3A00%3A00Z" must land in fetch's call. Future tightening of the Record signature cannot silently drop the new params. Unlike Python, TypeScript has no reserved-keyword issue — callers can write `client.listReservations({ from: "...", to: "..." })` directly. No protocol or wire-format change; servers older than v0.1.25.20 silently ignore the new params per the additive-parameter guarantee in cycles-protocol-v0.yaml. 316 tests pass at 98.4% statement coverage / 99.62% line coverage (gate ≥95%). Bumped to 0.3.2, updated AUDIT.md and CHANGELOG.md.
amavashev
added a commit
to runcycles/cycles-client-rust
that referenced
this pull request
May 21, 2026
Client-side companion to cycles-protocol-v0.yaml revision 2026-05-21 (runcycles/cycles-protocol#97) and runcycles/cycles-server#160. Closes the Rust-client side of issue #159. `ListReservationsParams` is strongly-typed (unlike the loose **kwargs / Record<string, string> shapes in the Python and TS clients), so this PR adds two new `Option<String>` fields with `#[serde(rename = ...)]` to project to the spec-mandated query-string names: /// Inclusive lower bound on `created_at_ms`. ISO 8601 date-time. #[serde(rename = "from", skip_serializing_if = "Option::is_none")] pub from: Option<String>, /// Inclusive upper bound on `created_at_ms`. ISO 8601 date-time. #[serde(rename = "to", skip_serializing_if = "Option::is_none")] pub to: Option<String>, Pure additive struct change. Callers using `ListReservationsParams::default()` or `..Default::default()` continue to compile and behave identically; new fields default to `None` and are skipped during serialization. Adds `list_reservations_forwards_from_to_window` in tests/client_test.rs that uses wiremock's `query_param` matcher to assert the new fields land on the wire under the spec-mandated names ("from" / "to", not the Rust struct field names — though they happen to match here). Pre-existing drift on `ListReservationsParams` (missing `workspace` / `workflow` / `toolset` subject filters, missing `sort_by` / `sort_dir` from the v0.1.25.12 spec revision, missing `idempotency_key`) is NOT addressed in this PR — kept scope tight. Worth a follow-up to bring the struct to full v0.1.25 spec parity, but separate from the #159 chain. No protocol or wire-format change; servers older than v0.1.25.20 silently ignore the new params per the additive-parameter guarantee in cycles-protocol-v0.yaml. 134 tests pass, clippy + doc-tests clean. Bumped to 0.2.5, updated AUDIT.md and CHANGELOG.md.
4 tasks
amavashev
added a commit
that referenced
this pull request
May 22, 2026
…oses runcycles/cycles-server#162) (#98) * feat(listReservations): add expires_* / finalized_* range filters Closes runcycles/cycles-server#162. Follow-up to #97 (the 2026-05-21 from/to revision) addressing the use case that revision intentionally left out: cleanup sweepers that need to locate reservations expiring or already finalized within a window. Adds four optional query parameters to GET /v1/reservations, mirroring the shape of the from/to window filter: * expires_from / expires_to — bound on expires_at_ms. * finalized_from / finalized_to — bound on finalized_at_ms. All ISO 8601 date-time strings. Either side of each pair may be supplied alone (open interval) or paired (closed window). The three window filters (from/to + expires_* + finalized_*) compose with AND semantics; a row must satisfy every supplied predicate to be returned. Each pair always binds to its target field regardless of sort_by, matching the 2026-05-21 sort-key-independence rule for from/to. finalized_at_ms is OPTIONAL on ReservationSummary / ReservationDetail. Rows where the field is absent (typically ACTIVE reservations not yet finalized) MUST be excluded from results when either finalized_from or finalized_to is supplied — the predicate naturally fails on field-absent rows; making the exclusion normative ensures all conformant servers agree. Validation mirrors the 2026-05-21 contract: from > to within each pair → 400 INVALID_REQUEST, malformed date-time → 400, either alone is valid, additive-parameter guarantee preserved. Sorted-path cursor invalidation extends to all six window-bound values via FilterHasher (server-side impl concern). Legacy SCAN cursors do not carry filter state and are not window-validated (matching how the legacy path already treats every other filter). TIME-RANGE FILTERS prose block in the listReservations operation description rewritten to cover all three field bindings and the AND-composition rule. Spec changes: - cycles-protocol-v0.yaml: 4 new query params on listReservations plus the expanded TIME-RANGE FILTERS (NORMATIVE, ADDITIVE) prose block. - changelogs/cycles-protocol-v0.md: new v0.1.25 — 2026-05-22 entry. - CONFORMANCE.md: listReservations SHOULD bullet updated. - merged/cycles-openapi-protocol-merged.yaml: mechanically regenerated by scripts/merge_specs.py. info.version stays at 0.1.25 per the existing revision convention (multiple dated v0.1.25 entries coexist; the validator only requires the topmost heading to match info.version). Verification: - npx spectral lint cycles-protocol-v0.yaml --fail-severity=error → 0 errors. 20 pre-existing warnings on top-level schema descriptions, unchanged from main. - python scripts/validate_changelogs.py → all 5 specs OK. - python scripts/merge_specs.py → merged artifact regenerated cleanly (no merge-check drift). * fix(listReservations): address reviewer P2/P3 findings on #98 Three real findings from the review pass: P2 — ReservationSummary had no `finalized_at_ms` field. The prose in the operation description and the changelog claimed it was OPTIONAL on the summary, but the schema has `additionalProperties: false` and didn't declare the field at all. listReservations returns ReservationSummary, so conformant servers could not have included the timestamp in list results and the filter would have been useful only via a follow-up getReservation call per row. Added `finalized_at_ms` as an OPTIONAL int64 property on ReservationSummary with the same shape as on ReservationDetail plus a normative description of population semantics. Old clients with strict schemas remain compatible because the field is OPTIONAL (absent in pre-revision responses, valid under the new schema when present). P2 — finalized_at_ms EXPIRED-row semantics were ambiguous. The prose said the field was populated when a reservation reached a terminal state "commit, release, expiry" — but issue #162 (and the current cycles-server runtime impl in RedisReservationRepository.buildReservationSummary) only writes finalized_at_ms from committed_at or released_at, never from an expired-sweep timestamp. EXPIRED rows have the field absent. Narrowed the contract: finalized_at_ms is populated ONLY on COMMITTED and RELEASED rows. ACTIVE and EXPIRED rows have it absent and are naturally excluded from `finalized_*` filter results. Added a normative pointer telling callers who want a window over EXPIRED rows to use `expires_*` (which works on every row since `expires_at_ms` is required). Updated three places in lockstep so the contract is consistent: - The TIME-RANGE FILTERS prose block on the listReservations operation - The `finalized_from` / `finalized_to` parameter descriptions - The `finalized_at_ms` field descriptions on both ReservationSummary and ReservationDetail P3 — changelog wording "No request or response schema changes" was wrong, even before this fix-up (the PR adds four query parameters), and is now further wrong because the ReservationSummary fix above adds an OPTIONAL response-body property. Reworded to: "Four new query parameters on listReservations; one new OPTIONAL property (finalized_at_ms) on ReservationSummary mirroring the same field already on ReservationDetail. No request-body schema changes." Spec changes: - cycles-protocol-v0.yaml: ReservationSummary.finalized_at_ms added; ReservationDetail.finalized_at_ms gains a description; TIME-RANGE FILTERS prose + finalized_from / finalized_to descriptions tightened. - changelogs/cycles-protocol-v0.md: finalized_* bullet and the backward-compat bullet rewritten to reflect the new contract and the ReservationSummary addition. - merged/cycles-openapi-protocol-merged.yaml: mechanically regenerated by scripts/merge_specs.py. Verification: - npx spectral lint cycles-protocol-v0.yaml --fail-severity=error → 0 errors. 20 pre-existing warnings on top-level schema descriptions, unchanged from main. - python scripts/validate_changelogs.py → all 5 specs OK. - python scripts/merge_specs.py → merged artifact regenerated; git diff --exit-code --quiet merged/ now clean post-commit. No version bump — staying at info.version 0.1.25 with the revision-2026-05-22 entry. * docs(listReservations): make blank-string-as-unset normative for window bounds Real P2 from the review pass: issue cycles-server#162 specified that blank-string window-bound values are treated as unset, but the spec PR didn't carry that contract through normatively. With `format: date-time` declared on each bound, strict implementers could reasonably 400 on `?expires_from=&finalized_to=` as malformed — diverging from the cycles-server reference implementation that's shipped this behavior since the original from/to revision (v0.1.25.20). Same gap existed in the v0.1.25.20 spec: the original from/to contract didn't explicitly call out blanks either; the server just handled it via `parseIsoToEpochMs` returning null on `raw.isBlank()`. This PR adds the normative rule once, in the shared TIME-RANGE FILTERS validation block, covering all six bounds — both the new `expires_*` / `finalized_*` pairs from this revision and the v0.1.25.20 `from` / `to` pair. So the rule becomes retroactively explicit for the original revision too, which closes a latent divergence hazard. Rationale included in the spec prose: clients commonly emit unconditional query strings whose values come from possibly-unset variables (e.g. `?from=${maybeUnset}`), and an unset variable rendering as `""` is the common failure mode. Rejecting these as malformed surfaces a cryptic 400 that adds nothing over treating them as unset. The additive-parameter guarantee is designed to avoid exactly this class of paper-cut divergence. Spec changes: - cycles-protocol-v0.yaml: new normative bullet in the TIME-RANGE FILTERS (NORMATIVE, ADDITIVE) validation sub-list. Servers MUST treat blank-string bounds as unset and MUST NOT 400 on them despite the format: date-time declaration. - changelogs/cycles-protocol-v0.md: matching bullet under the Validation section. - merged/cycles-openapi-protocol-merged.yaml: mechanically regenerated. The per-param descriptions are intentionally not duplicated — the shared block is normative, and a six-fold prose copy would just be drift bait. Verification: - npx spectral lint cycles-protocol-v0.yaml --fail-severity=error → 0 errors. 20 pre-existing warnings unchanged. - python scripts/validate_changelogs.py → all 5 specs OK. - python scripts/merge_specs.py → clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft — opening for review while the issue author confirms naming and the five open questions raised in cycles-server#159 (comment).
Summary
Adds two optional query parameters to
listReservations(GET /v1/reservations) so clients can request a created-time slice (e.g., last 24 hours) without the page-size-escalation workaround.from: ISO 8601 date-time. Inclusive lower bound oncreated_at_ms.to: ISO 8601 date-time. Inclusive upper bound oncreated_at_ms.Either may be supplied alone (open interval) or together (closed window). The filter always binds to
created_at_msregardless ofsort_by, so sort-by-expires_at_ms+ window-by-created_at_msis well-defined.Closes runcycles/cycles-server#159.
Naming and wire-type rationale
Matches the family-wide
from/to+format: date-timeconvention already in use on every other list endpoint in the suite:listAuditLogscycles-governance-admin-v0.1.25.yaml:5833-5842listWebhookDeliveries:6508-6517listEvents(admin):6639-6648listTenantEvents:7600-7609listTenantWebhookDeliveries:7690-7699Bespoke names (
created_after/created_before) or Unix-epoch wire types would split the convention for clients and codegen. Server converts on ingest; entity-sidecreated_at_ms(int64 epoch ms) is unchanged.Validation
from > toMUST return HTTP 400INVALID_REQUEST.Additive-parameter guarantee
Servers that don't recognize the new parameters MUST ignore them without error and return the unfiltered set. Older clients that never send them get the pre-revision wire behavior byte-for-byte. Matches the
trace_id/request_idadditive treatment onlistEvents.Changed files
cycles-protocol-v0.yaml— two new query params onlistReservationsplus aTIME-RANGE FILTERS (NORMATIVE, ADDITIVE)prose block in the operation description.changelogs/cycles-protocol-v0.md— new## v0.1.25 — 2026-05-21entry at the top, following the existing same-version-multiple-revisions convention.CONFORMANCE.md—listReservationsSHOULD bullet updated to mention the time-window query path.info.versionstays at0.1.25per the existing revision convention (multiple datedv0.1.25entries already coexist in the changelog: 2026-04-18 trace_id, 2026-04-16 server-side sort, etc.). Happy to bump to0.1.26if reviewers prefer.Open questions (for issue author confirmation before un-drafting)
These are answered in the spec as written, but I want explicit confirmation from @Prikis on the comment thread:
from/tovs.created_after/created_before— going with the family convention.from > to: server returns 400INVALID_REQUEST— confirmed in description.fromalone valid (no upper bound),toalone valid (no lower bound), absent = no bound — confirmed in description.created_at_msregardless ofsort_by— confirmed in description.Plus a rationale nit: the issue's "cleanup expired/abandoned reservations" use case actually wants
expires_at/finalized_atfilters, notcreated_at. Out of scope here — happy to file a follow-up if there's appetite.Verification
npx spectral lint cycles-protocol-v0.yaml --fail-severity=error→ 0 errors. 20 pre-existing warnings on top-level schema descriptions, unchanged frommain(verified withgit stashround-trip).python scripts/validate_changelogs.py→ all 5 specs OK.Test plan
from/tonaming and the five open questions