fix: spec compliance for reservation 410 and event charged field#60
Merged
fix: spec compliance for reservation 410 and event charged field#60
Conversation
- Return HTTP 410 RESERVATION_EXPIRED when GET /v1/reservations/{id}
fetches an expired reservation (spec normative requirement)
- Only include `charged` in event response when overage_policy is
ALLOW_IF_AVAILABLE and capping actually occurred (per spec description)
https://claude.ai/code/session_018QQFJnBJBjvMU24LCNMjvT
- GET /v1/reservations/{id}: also check expires_at_ms against server
time for ACTIVE reservations (background expiry service may lag)
- Add test: shouldReturn410ForExpiredReservation
- Add tests: shouldIncludeChargedWhenCapped, shouldOmitChargedWhenNotCapped
https://claude.ai/code/session_018QQFJnBJBjvMU24LCNMjvT
Replace System.currentTimeMillis() with jedis.time() in getReservationById() to match Lua scripts and avoid inconsistent expiry decisions from JVM-Redis clock drift. https://claude.ai/code/session_018QQFJnBJBjvMU24LCNMjvT
The real-time expiry check in getReservationById() now calls jedis.time(), but existing tests used past-dated expires_at values without mocking jedis.time(). Add default mock returning a time before test reservation expiry so ACTIVE reservations don't false-trigger the 410 path. https://claude.ai/code/session_018QQFJnBJBjvMU24LCNMjvT
Remove the 410 RESERVATION_EXPIRED check from getReservationById(). GET is read-only and always returns 200 with the reservation detail including the status field (ACTIVE, COMMITTED, RELEASED, or EXPIRED). The 410 response applies to mutating operations (commit/release/extend) which enforce expiry in their Lua scripts. GET is for debugging and must show the current state regardless of lifecycle stage. https://claude.ai/code/session_018QQFJnBJBjvMU24LCNMjvT
Add Issue 9 (event.lua charged field fix) and document confirmed non-issues validated against the YAML spec (ErrorResponse.details, Balance optional fields, CommitResponse.released, GET reservation 200). https://claude.ai/code/session_018QQFJnBJBjvMU24LCNMjvT
Spec line 52: "Expired reservations MUST return HTTP 410 with error=RESERVATION_EXPIRED." GET endpoint explicitly lists 410 (line 1212). Implementation was returning 200 — now returns 410. - Add EXPIRED status check in getReservationById() - Update integration tests to assert 410 RESERVATION_EXPIRED - Update unit test to assert 410 - Update AUDIT.md with Issue 10 https://claude.ai/code/session_018QQFJnBJBjvMU24LCNMjvT
Spec NORMATIVE: "On replay with the same idempotency_key, the server MUST return the original successful response payload." - event.lua: replay now reconstructs charged (when capping occurred) and balances from stored event hash + current budget state - commit.lua: replay now includes balances and affected_scopes_json by reading affected_scopes from reservation hash - release.lua: replay now includes balances by reading affected_scopes from reservation hash https://claude.ai/code/session_018QQFJnBJBjvMU24LCNMjvT
Spec compliance audit fixes: idempotency replay, event charged field, GET reservation 410. https://claude.ai/code/session_018QQFJnBJBjvMU24LCNMjvT
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
2 participants
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.
Summary
Fixes two spec compliance gaps found during YAML spec audit:
/v1/reservations/{id}now returns HTTP 410 withRESERVATION_EXPIREDwhen the reservation status is EXPIRED. Spec normative: "Expired reservations MUST return HTTP 410 with error=RESERVATION_EXPIRED."chargedfield is now only included in the response whenoverage_policy=ALLOW_IF_AVAILABLEand capping actually occurred (charged < requested). Previously always returned.Test plan
chargedwhen no capping occurschargedwhen ALLOW_IF_AVAILABLE caps the amounthttps://claude.ai/code/session_018QQFJnBJBjvMU24LCNMjvT