Inventory Reservation Service

GET .../ledger?limit=500 rejects its own documented maximum

Any dashboard, audit tool, or support script that paginates the append-only ledger using the documented maximum page size of 500 gets a `400 Bad Request` instead of a page of results -- for the exact value the API's own error message claims is valid. It is a small thing, but it is the kind of small thing that erodes trust in an API's documentation: if the boundary value in the error message is wrong, what else is?

Restocking with quantity: 0 returns a raw database error

A warehouse-integration script or admin tool that occasionally computes a restock delta of exactly zero (nothing changed since the last sync) -- a perfectly normal, meaningless no-op -- gets back a `500 Internal Server Error` with a raw Postgres constraint-violation message instead of a clean rejection. Anyone integrating against this API has to specifically special-case "don't send zero" rather than trusting the API to handle it gracefully, and every occurrence pollutes error-rate monitoring with a 5xx that isn't actually a server problem.

Reservation API responses swap created_at and expires_at

Any client showing a countdown ("your hold expires in 4:58") or checking whether a reservation is about to lapse computes `expires_at - created_at` (or just compares `expires_at` to "now") and gets a negative number or an already-past timestamp for a reservation that was just created and is nowhere close to expiring. A checkout page built on this API would show "this item's hold expired 4 minutes ago" the instant a customer adds it to their cart.

Access logs always report status 0, never the real response code

Log-based monitoring and alerting (e.g. "page if the 5xx rate exceeds X%") is completely blind: every request, whether it succeeded, was rejected, or crashed, is logged with the same meaningless status. A real outage -- every request suddenly 500ing -- would produce logs that look identical to a healthy service. On-call has no way to tell from the logs alone that anything is wrong.

Add Reservation.IsActive() and SKU.IsInStock()

Callers who want to know "is this reservation still holding stock?" have to compare `r.Status == inventory.ReservationActive` themselves; callers who want "does this SKU have anything left to sell?" have to compare `sku.Available > 0` themselves. Both are one-line checks, but they are duplicated at every call site instead of being named, self-documenting methods on the domain types themselves -- every future caller (HTTP handlers, the load-test tool, a future admin CLI) will otherwise reimplement them slightly differently each time.

Add Service.ListActiveReservations(ctx, skuID)

`Service` can fetch a single reservation by id (`GetReservation`), but there is no way to ask "what is currently holding stock against this SKU?" without already knowing every individual reservation id. Support staff investigating "why is available lower than I expect" have no query to run short of hitting Postgres directly.

Reconcile reports false drift for any SKU that's ever had a release

This project's reconciliation tooling exists specifically so that "does the materialized inventory match the ledger?" is something you can trust, not something you have to re-derive by hand during an incident. If `Reconcile` cries wolf on perfectly healthy SKUs -- reporting drift where there is none -- the team will do exactly what everyone does with noisy alerts: start ignoring them. The one time `Reconcile` reports a real drift, on-call may not take it seriously, because "oh, that report is always a little off."

Restocking an existing SKU resets its version counter to 1

`sku_inventory.version` exists so the optimistic-locking strategy (and any future tooling that reasons about "has this row changed since I last looked?") can detect concurrent writers. If a routine restock silently resets it back to 1, any external system caching "the version I last saw was N" now sees a version lower than one it already observed -- undermining the entire assumption that version only ever climbs. This kind of bug tends to stay invisible for a long time (nothing crashes) and then causes deeply confusing behavior in whatever eventually relies on version being monotonic.

The demo SKUs are never seeded on a fresh database

The README's entire quickstart -- `docker compose up --build`, then run the load test against the `flash-sale` SKU -- depends on this seeding step actually creating that SKU. With this bug, a brand new deployment starts up successfully (no errors, no crash) but the demo SKUs simply do not exist: the load-test tool's default target immediately 404s, and anyone following the README's own instructions hits a wall on their very first command with no indication of why.

Optimistic claims add to available instead of subtracting

This is a sign error in the exact statement responsible for one of this project's two core locking strategies. Left alone, every optimistic reservation would either corrupt the SKU's numbers or (thanks to a database safety net) fail outright with a confusing error -- either way, any deployment configured to use `LOCK_STRATEGY=optimistic` is completely broken for its one job: correctly tracking how much stock is left.

Committing a reservation returns stock to available instead of marking it sold

This is a revenue-accounting bug: a customer completes checkout, `Commit` reports success, and yet the units they just paid for silently become available for someone else to buy -- while `sold` (the figure that presumably drives revenue/units-sold reporting) never moves at all. Depending on which of two concurrent buyers' request wins the race for that "freed" stock, this can mean double-selling a physical unit that was already promised to someone else, and it permanently understates sales in any dashboard built on `sold`.

Add GET /v1/skus (list all SKUs)

The API can fetch one SKU at a time by id (`GET /v1/skus/{id}`), but there is no way to discover what SKUs exist at all short of querying Postgres directly. An admin dashboard or a support engineer investigating "what's even in the catalog right now?" has nothing to call.

Re-committing an already-committed reservation now fails

This project's README explicitly documents idempotent commit as a guarantee: "committing an already-committed reservation must return the same result without incrementing sold again, because a retried payment webhook calling this twice is a completely normal occurrence." With this bug, the second call from a retried webhook now gets an error instead of the expected success -- depending on how the caller handles that error, this can mean re-attempting a charge that already went through, or reporting a successful payment as failed to the customer.

POST /v1/reservations/{id}/commit always returns 404

This is the checkout endpoint -- the one that turns a hold into a sale. With this bug, it fails 100% of the time, for every reservation, no matter how valid the id. Every payment success would be followed by a failed commit call, always reported as "reservation not found" -- a misleading error that would send whoever's debugging it looking in completely the wrong place instead of the real, one-line routing bug.

optimistic_lock_retries_total counts every claim as at least one retry

This metric is one half of the README's own headline deliverable: the benchmark comparing the two locking strategies reports a "retry rate" computed from this counter. If every successful, zero-contention claim is counted as at least one retry, the reported retry rate is inflated across the board -- an operator deciding whether to switch `LOCK_STRATEGY` based on "how much contention is the optimistic strategy actually seeing in production" would be working from a number that's wrong even under perfectly healthy, uncontended traffic.

Add a per-reservation maximum quantity cap

Nothing stops a single `Reserve` call from claiming a SKU's entire remaining stock in one request. For a genuine flash sale (the exact scenario this project is built around), that means one request -- a bug in a client, a scraper, or someone gaming the sale -- can claim all 100 units before anyone else gets a chance, defeating the point of a fair sale entirely.

The optimistic strategy retries the wrong errors

This defeats the entire point of the optimistic locking strategy under the exact conditions it's supposed to handle: real contention. A genuine flash sale means many buyers racing to update the same SKU row -- version conflicts are the expected, normal, recoverable case the retry loop exists for. With this bug, a real conflict now fails immediately instead of retrying, so under real load `LOCK_STRATEGY=optimistic` would reject far more legitimate buyers than it should.

Add GET /v1/skus/{id}/reservations (list by SKU, filterable by status)

There is no way to discover a SKU's reservations short of already knowing each individual reservation id. Support staff investigating "what's holding this SKU's stock right now" (or "what happened to this customer's order") have no endpoint to call.

Add Service.ExtendReservation (push out a hold's expiry)

A reservation's TTL is fixed at creation time. A slow checkout flow (a customer stuck on a payment form, a flaky payment gateway) has no way to ask for more time before the reaper reclaims its hold -- the only option today is to fail the checkout and make the customer start over.

Add a duplicate order_reference guard

Nothing stops the same `order_reference` from getting two independent, active reservations against the same SKU. A double-submitted "Buy Now" click -- a slow network causing a browser or client to fire the same request twice, a retried request from a flaky mobile connection -- today silently creates two separate holds for what should be one order, claiming twice the stock a single order actually needs.

Add inventory.Repair (fix drift the reconciliation tool finds)

`Reconcile` can detect drift between the materialized `sku_inventory` row and what the ledger says is true, but there is no way to fix it short of hand-writing SQL during an incident -- exactly the kind of manual, error-prone, hard-to-audit intervention this project's append-only ledger design is supposed to make unnecessary.

The reaper can claim the same due reservation twice under concurrency

This is the exact "many replicas must not double-release the same due work" guarantee the README calls out as this project's reason for running two app replicas by default. Without it, two reaper replicas polling at the same moment can both claim and release the SAME expired reservation -- crediting its quantity back to `available` twice. In a real flash sale that means phantom stock appearing out of nowhere, which can then be sold to a second buyer for a unit that was never actually free.

Concurrent commit/release calls on the same reservation can both apply

This is the mechanism the README explicitly credits for making commit idempotent under real concurrency: "the FOR UPDATE row lock on the reservation serializes concurrent commit calls for the same id." Without it, two callers racing to resolve the same reservation -- a retried payment webhook and a manual support action landing at the same instant, or two webhook deliveries from a payment provider's own at-least-once retry policy -- can both read status: active before either has written anything, and both apply their own transition. For two concurrent commits, that means sold incrementing twice for a single reservation: a real double-sell of the same unit, silently, with no error to anyone.