Review an inventory reservation saga

Review exercise · TypeScript

Review brief

Nightmare60 min

An order service introduces an inventory reservation workflow consumed by multiple workers. Commands and outbox deliveries are at least once, and expiry may race confirmation or cancellation. Review the state and delivery boundaries that protect stock and events.

Behavioral contract

A reserve command has a stable messageId. Its inbox record, guarded stock decrement, pending reservation, and ReservationCreated outbox row commit in one database transaction.
A reservation decrement succeeds only when available stock is still at least the requested positive quantity; concurrent reservations must never make stock negative.
Pending may transition once to Confirmed before expiresAt, or once to Expired at or after expiresAt. Competing confirmation and expiry cannot both apply their effects.
Cancellation may transition Pending or Confirmed to Canceled and restores stock exactly once; duplicate or concurrent cancellation commands are no-ops after the first applied transition.
Every applied state transition writes its corresponding stable-ID outbox event in the same transaction; a non-applied duplicate writes nothing.
An outbox event keeps its event ID as the broker idempotency key across every publish retry. Broker acknowledgement marks it delivered.
Retry delays use bounded exponential backoff with jitter so independent workers do not retry in lockstep.
Workers and storage operations in scope may fail between any awaited operations; consumers tolerate duplicate events carrying the same stable event ID.

Review instructions

1Trace concurrency, crashes, duplicate commands, and a publish retry across all five changed files.
2Combine symptoms that share one missing atomic boundary, but keep independent transition and retry defects separate.
3Prioritize no more than eight findings and anchor each to the narrowest useful line.

Learning objectives

• Reason about inbox state and outbox atomicity
• Validate guarded transitions under concurrency
• Separate root causes from duplicate-delivery symptoms

src/inventory-saga-types.tsTypeScript

@@ -0,0 +1,72 @@

1+export type ReservationState = "pending" | "confirmed" | "expired" | "canceled";

3+export interface Reservation {

4+ id: string;

5+ orderId: string;

6+ sku: string;

7+ quantity: number;

8+ state: ReservationState;

9+ expiresAt: Date;

10+}

11+

12+export interface ReserveInventoryCommand {

13+ messageId: string;

14+ orderId: string;

15+ sku: string;

16+ quantity: number;

17+ expiresAt: Date;

18+}

19+

20+export interface OutboxEvent {

21+ id: string;

22+ type: "ReservationCreated" | "ReservationConfirmed" | "ReservationExpired" | "ReservationCanceled";

23+ reservationId: string;

24+ occurredAt: Date;

25+}

26+

27+export interface InventoryTransaction {

28+ decrementStock(sku: string, quantity: number): Promise<void>;

29+ decrementStockIfAvailable(sku: string, quantity: number): Promise<boolean>;

30+ incrementStock(sku: string, quantity: number): Promise<void>;

31+ insertReservation(reservation: Reservation): Promise<void>;

32+ setReservationState(id: string, state: ReservationState): Promise<void>;

33+ transitionReservation(

34+ id: string,

35+ from: readonly ReservationState[],

36+ to: ReservationState,

37+ ): Promise<boolean>;

38+ insertOutbox(event: OutboxEvent): Promise<void>;

39+ markInboxProcessed(messageId: string): Promise<void>;

40+}

41+

42+export interface SagaDependencies {

43+ database: {

44+ transaction<T>(work: (transaction: InventoryTransaction) => Promise<T>): Promise<T>;

45+ };

46+ inventory: { available(sku: string): Promise<number> };

47+ reservations: {

48+ get(id: string): Promise<Reservation | undefined>;

49+ listPendingExpiredAt(now: Date): Promise<readonly Reservation[]>;

50+ };

51+ inbox: {

52+ has(messageId: string): Promise<boolean>;

53+ markProcessed(messageId: string): Promise<void>;

54+ };

55+ outbox: { insert(event: OutboxEvent): Promise<void> };

56+ newId(): string;

57+ now(): Date;

58+}

59+

60+export interface EventBroker {

61+ publish(input: {

62+ event: OutboxEvent;

63+ idempotencyKey: string;

64+ }): Promise<void>;

65+}

66+

67+export interface PublishDependencies {

68+ broker: EventBroker;

69+ markDelivered(eventId: string): Promise<void>;

70+ sleep(milliseconds: number): Promise<void>;

71+ newId(): string;

72+}

Reference solution

Findings are ordered by priority.

Findings

1. Availability is checked outside the transaction that decrements stock

Location: src/reserve-inventory.ts:17-18 and src/reserve-inventory.ts:35-38

Two workers can both read the same available quantity, both pass the check, and then each call the unconditional decrementStock. Their transactions can drive stock negative and create reservations for more units than exist.

Perform a guarded decrement inside the transaction—using the supplied decrementStockIfAvailable or an equivalent conditional update—and create the reservation only when that update succeeds.

2. The inbox record commits before any reservation effect

Location: src/reserve-inventory.ts:13-18

The separate has/mark sequence is neither atomic nor tied to the business transaction. A crash after markProcessed, an insufficient-stock exception, or a later database failure leaves the message recorded as complete; redelivery then returns without creating the reservation. Concurrent duplicates can also both pass has before either mark is visible.

Insert the unique inbox record in the same transaction as the guarded stock change, reservation, and outbox row. Treat a duplicate-key result as the no-op outcome.

3. Reservation state and its outbound event commit independently

Location: src/reserve-inventory.ts:35-40 and src/reservation-transitions.ts:33-38

The reservation transaction commits before the separate outbox store call. The same split is repeated for confirmation, expiry, and cancellation. A crash or outbox failure after state commits leaves durable business state with no event, while a retry may see the new state and decline to recreate the missing event.

Insert each stable-ID outbox row through the transaction's insertOutbox method alongside the state change, including the initial inventory and inbox effects.

4. Confirmation and expiry use stale reads and unguarded transitions

Location: src/reservation-transitions.ts:24-35 and src/reservation-transitions.ts:42-49

Confirmation reads Pending before its transaction, while expiry lists Pending reservations before its own transaction. If they race, both can call setReservationState; expiry can restore stock and emit Expired even though confirmation also reports success, with the last write deciding the stored state.

Use one conditional transition inside each transaction (Pending to the intended target with the time predicate), apply stock and outbox effects only when that transition wins, and report the actual applied result.

5. Every cancellation restores inventory, including duplicates

Location: src/reservation-transitions.ts:62-71

Cancellation checks only that a reservation exists. A repeated delivery, two concurrent cancellations, or cancellation after expiry increments stock again and emits another event before setting Canceled. Stock can therefore exceed its real quantity.

Guard a transaction from Pending or Confirmed to Canceled and increment stock and write the event only for the single successful transition.

6. Each publish retry invents a new idempotency key

Location: src/publish-outbox-event.ts:10-16

newId() runs inside the retry loop. If the broker accepted an event but its acknowledgement was lost, the next attempt presents a different identity and downstream deduplication cannot recognize the repeated event.

Use the stable event.id as the idempotency key on every attempt, including a retry after markDelivered fails.

7. Fixed retry delays synchronize failing workers

Location: src/publish-outbox-event.ts:18-20

Every failed publisher waits exactly one second. A shared broker outage causes many workers to retry together, creating periodic load spikes that can delay recovery.

Calculate a bounded exponential delay from the attempt number and add independent jitter before sleeping.

8. Tests exercise only sequential success and an immediate retry

Location: src/inventory-saga.test.ts:12-70

The reservation test uses one worker and dependencies that cannot fail, while the publishing test makes the second call succeed immediately and asserts only call counts. There is no coverage for duplicate message IDs, a crash at transaction boundaries, simultaneous reservations, competing expiry and confirmation, repeated cancellation, or stable publish identity.

Add deterministic tests with barriers and failure injection for duplicates, crashes, and competing transitions, plus an assertion that every retry carries the same event ID.

Reasonable non-findings

Rejecting non-positive quantities before storage access is correct.
Refusing confirmation at exactly expiresAt matches the exclusive pre-expiry confirmation rule; the defect is that the predicate is not guarded with the write.
Marking an outbox row delivered only after broker acknowledgement is the correct ordering.
Four publish attempts are a reasonable bounded policy because the contract does not require a particular maximum.
The outbox event itself already receives one stable ID. The publishing code fails by replacing that identity only in the idempotency-key field.