fix(#8): enrich 404 "Budget not found" with unit hint, bump 0.2.3#9
Merged
Conversation
The server indexes budgets by the composite key (scope, unit) in reserve.lua, so a scope with an ACTIVE budget in one unit (e.g. USD_MICROCENTS) surfaces as HTTP 404 NOT_FOUND "Budget not found for provided scope: tenant:rider" when the client sends a reservation in a different unit (e.g. TOKENS). The raw message reads like a scope-lookup miss, which led users to believe the scope didn't exist (issue #8). create_reservation, create_reservation_with_metadata, decide, and create_event now post-process errors through enrich_budget_not_found, which detects the server's exact 404 marker and rewrites the message to include the unit that was sent plus a one-line explanation of the (scope, unit) indexing invariant. Non-matching 404s and other error kinds pass through unchanged. Amount, WithCyclesConfig::new, the with_cycles_usage example, and README Quick Start document the invariant so new users don't hit it. Adds 5 unit tests (enrichment helper) and 4 wiremock integration tests (reserve, decide, event enrichment + non-matching 404 pass- through). Coverage 95.55% (>=95%). Issue #8.
Strengthens the 0.2.3 audit entry with direct citations to
cycles-protocol-v0.yaml:
- line 667 ("Ledger state for a single (scope, unit) balance")
establishes that the (scope, unit) composite is spec-normative,
not just server implementation detail.
- line 56 documents the UNIT_MISMATCH rule for commit/event but
leaves reserve unit-mismatch semantics under-specified.
- lines 1187-1200 show POST /v1/reservations responses: 200, 400,
401, 403, 409, 500 - no 404 documented, flagging the server's
404 as out-of-spec (to be filed against cycles-server).
Also clarifies that the client preserves all Error::Api fields
other than message (status, code, request_id, retry_after,
details) so error classification, retry logic, and correlation
behave identically.
Refreshes the Test Coverage table from a clean tarpaulin run
(95.55%, 472/494 lines) and corrects the test count breakdown
(143 tests: 131 running + 12 live ignored; previous audit's
"141 total" did not sum correctly).
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
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 #8 — a misleading
404 NOT_FOUND "Budget not found for provided scope: tenant:rider"that was actually a unit mismatch, not a scope miss.Root cause. The protocol spec defines
Balanceas "Ledger state for a single (scope, unit) balance" (cycles-protocol-v0.yamlline 667), so a single scope may hold multiple budgets keyed by unit. The reference server'sreserve.luaimplements this by keying budgets as"budget:" .. scope .. ":" .. estimate_unit. When a reservation targets a scope that has an ACTIVE budget in one unit (e.g.USD_MICROCENTS) but is sent in a different unit (e.g.TOKENS), the script returnsBUDGET_NOT_FOUNDand the server maps it toHTTP 404. The raw message reads like a scope-lookup miss, which led users to believe the scope didn't exist.Fix.
create_reservation,create_reservation_with_metadata,decide, andcreate_eventpost-process errors through a newenrich_budget_not_found(err, unit)helper that detects the exact server marker and rewritesError::Api.messageto include the unit that was sent plus a one-line explanation of the(scope, unit)indexing invariant. All otherError::Apifields (status,code,request_id,retry_after,details) are preserved unchanged, so error classification, retry logic,request_idcorrelation, and downstream pattern matching behave identically. Non-matching 404s pass through untouched.Example enriched error for the issue's repro:
What's in this PR
src/client.rs—enrich_budget_not_foundhelper + 5 unit tests covering happy path, wire-format unit names (USD_MICROCENTSnotUsdMicrocents), non-matching messages, non-404 status, and non-Apierror kinds. Wired into the 4 request methods that carry anAmount.tests/client_test.rs— 4 new wiremock integration tests coveringreserve/decide/eventenrichment end-to-end (includingrequest_idpreservation) and a pass-through test that proves an unrelated 404 is not rewritten.Amount(src/models/common.rs),WithCyclesConfig::new(src/lifecycle.rs),examples/with_cycles_usage.rs, and README Quick Start all note the(scope, unit)invariant so new users don't hit this.Cargo.toml0.2.2→0.2.3;Cargo.lockupdated to match.CHANGELOG.md— 0.2.3 entry withFixed+Docssections, links to issue [Bug] Rust client returns 404 for a budget that exists at the given scope #8.AUDIT.md—Issues Found & Resolved (0.2.3)entry with direct spec citations (lines 667, 56, 1187–1200). Test Coverage table refreshed from a cleancargo tarpaulinrun (previous table had stale numbers) — new totals 472/494 = 95.55%, above the 95% project threshold.Spec compliance
Reviewed against
cycles-protocol-v0.yamlv0.1.24. Two independent passes (manual + Explore subagent) found zero client-side issues:ErrorResponse.messagehas no normative content constraint — client-side rewriting is wire-transparent.status: 404, code: Some(ErrorCode::NotFound)is consistent with the spec's definition ofNOT_FOUND.(scope, unit)invariant my docs claim is spec-normative (line 667), not just server trivia.The audit did surface two server-side spec gaps that are out of scope for this PR (follow-up issue incoming against
cycles-server):POST /v1/reservationsdocuments responses200, 400, 401, 403, 409, 500(lines 1187–1200) — no 404 is documented, yet the server returns one here.UNIT_MISMATCH→ HTTP 400 for commit and event, but the analogous rule for reserve is under-specified; the server uses 404NOT_FOUNDinstead of 400UNIT_MISMATCH.This client change handles the out-of-spec server response defensively; the root cure belongs on the server.
Test plan
cargo build— cleancargo test— 131 running tests pass (43 lib unit + 30 wiremock client + 18 models + 10 error + 10 config + 5 lifecycle + 4 guard + 2 response + 1 retry + 8 doc); 12 live-server tests ignored as usualcargo test --lib enrich_budget_not_found— 5/5 new unit tests passcargo clippy --all-targets -- -W clippy::all— no new warnings from this change (pre-existing{:?}format-string hints untouched)cargo fmt --check— cleancargo tarpaulin --out Stdout --ignore-tests -- --skip live— 95.55% overall,client.rs137/143 = 95.80%cycles-serverwith a mismatched-unit reservation (deferred to reviewer; the wiremock tests exercise the exact server error shape)Follow-ups
cycles-serverissue for the two spec gaps identified above.reserve.luadistinguish "no budget at this scope" from "budget exists at this scope in a different unit" and return400 UNIT_MISMATCH(which already exists inErrorCodeand is used for commit/event) with the actual stored unit(s).Closes #8.