test: add integration test suite for budget consumption edge cases

[Bogunrot]

1. Linked Issue

Closes #6

2. Problem Statement (The Bug)

The acceptance criteria demand tests proving that concurrent agent requests do not bypass budget checks — but the budget consumption path could not pass that test, because it was genuinely racy:

• BudgetService.assertWithinBudget was a read-then-check pre-flight that did not mutate state, and consume incremented spent afterwards. Two concurrent agent requests both read spent = 0 against a 1000 limit, both passed the check, and both incremented — ending at 1200 > 1000.
• BudgetReservationService (the distributed-lock reservation mechanism) existed but was never wired into any module or pipeline — it was dead code. Even its own check was non-persisting, so a lock alone could not stop the race.

This cannot be fixed with a local patch to the test: writing a test that asserts "concurrent requests do not bypass checks" against the current code would fail, because the bypass is real. The check had to become authoritative and persisted before any funds move.

3. Solution Comparison and Decision (Why reservation, and not the alternatives)

Option	Why rejected
A. Keep the non-mutating pre-flight, add the lock around it	A lock around a read-only check changes nothing: the second request still reads the same spent, still passes, and both consume → overspend. Treats the symptom.
B. Make consume the authoritative atomic check (UPDATE ... WHERE spent + amount <= limit)	The limit would only be enforced after funds moved on-chain — a payment could succeed on Stellar and then be rejected by the budget. Fails after the damage is done.
C. Leave BudgetReservationService unwired and test it in isolation	The issue explicitly requires verifying the real path; testing a helper that nothing calls proves nothing and leaves dead code in the tree.

Decision — reservation-based consumption (Option D): wire BudgetReservationService into the pipeline and make reserve persist the reservation atomically (spent is incremented) under the distributed lock, at creation time, before any funds can move. consume becomes a settle step (no double-count) and release returns headroom on cancellation or failed execution. This is the only option where the budget is checked-and-booked as a single atomic step in the request path.

4. The Change (Code modifications)

• budget-reservation.service.ts — reserve now checks the limit and persists the reservation (incrementSpent) inside the lock; over-limit throws BudgetExceededException (422 BUDGET_EXCEEDED), missing budget / lock failure throw ConflictException (409 CONFLICT).
• budget.service.ts — reserve (authoritative), consume (settle only — the spend was already booked), release (clamped, never negative). Racy assertWithinBudget removed; nothing references it anymore.
• budget.repository.ts — added decrementSpent (atomic spent: { decrement }).
• budget.module.ts — BudgetReservationService + RedisLock are now provided (previously dead code).
• transaction.service.ts — create reserves before persisting (releasing on persist failure); execute settles on success and releases when funds never moved; cancel (also the approval-rejection path) releases.
• event-names.ts — budget.released.

Core reservation:

return await this.redisLock.withLock(lockKey, async () => {
  const budget = await this.budgetRepository.findById(organizationId, budgetId);
  if (!budget) throw new ConflictException('Budget not found');

  const projected = new Decimal(budget.spent).plus(amount);
  if (projected.greaterThan(budget.limitAmount)) {
    throw new BudgetExceededException('Transaction would exceed the budget limit', {
      budgetId, limit: budget.limitAmount.toFixed(7),
      spent: budget.spent.toFixed(7), attempted: amount,
    });
  }
  // Persist the reservation atomically — the check and the increment are
  // serialized by the lock, so concurrent requests cannot overspend.
  return this.budgetRepository.incrementSpent(budgetId, new Decimal(amount));
});

Behavioral comparison across entry points:

Entry point	Before	After
TransactionService.create (budgeted)	read-only pre-flight; concurrent requests both pass	authoritative reserve books the amount atomically; the second concurrent request gets BUDGET_EXCEEDED (422)
TransactionService.execute success	consume incremented spent (after on-chain)	consume settles the already-booked reservation — no double count
TransactionService.execute failure	spent stayed incremented forever	release returns the headroom (only when funds never moved)
TransactionService.cancel / approval rejection	spent stayed incremented forever	release returns the headroom
BudgetReservationService	defined but never wired in	provided in BudgetModule, used by the pipeline
Over-limit failure	CONFLICT 409 from reservation spec	BUDGET_EXCEEDED 422 with limit/spent/attempted details

5. Compatibility Note (On INTERFACE_VERSION)

No interface version exists in this project (the API version is the stable api/v1 prefix) and none is modified. HTTP surface is unchanged — same routes, same request/response shapes. The one intentional behavioral change is that a budgeted transaction that breaches the limit now fails earlier (at creation) with the same BUDGET_EXCEEDED error the pre-flight used to throw, plus a new budget.released event. The removed assertWithinBudget had no callers outside the transaction pipeline (verified by grep) — it is replaced, not broken.

6. Incidental Fixes (Two things the issue called out that also got fixed)

• Dead code eliminated: BudgetReservationService was defined, spec'd, and never wired into a module or the pipeline. It is now provided in BudgetModule and used by every budgeted transaction.
• Stranded budget spend: previously, a transaction that failed on-chain or was cancelled permanently ate its reservation (spent was only ever incremented). The release lifecycle now returns that headroom.

7. Testing (Proving it works)

The core behavior is verified by integration tests in src/modules/budgets/tests/ that exercise the real BudgetService → BudgetReservationService → BudgetRepository chain (mocked Prisma/Redis stand in for the external stores):

• budget-consumption.integration.spec.ts
  • does not let concurrent agent requests overspend the budget (lock serialises) — 5 concurrent 300-unit spends on a 1000 budget: exactly 3 succeed, 2 fail with BudgetExceededException, spent never exceeds 900.
  • the DB-level atomic precondition also prevents overspend if the lock is bypassed — same scenario with serialization removed: the conditional increment still caps spend at the limit (3/5 succeed, rest fail with a typed ConflictException).
  • allows a spend landing exactly on the limit at 7-dp precision / rejects a spend one 7-dp unit past the limit with exact details — Decimal arithmetic, no float drift.
  • emits a warning exactly at the 80% utilisation boundary, and not below it and attributes daily-limit spend correctly at the exact day boundary — time-frame boundaries.
  • returns reserved headroom after a release, never drives spent below zero, settles without double-counting.
• error-envelope.integration.spec.ts — the real AllExceptionsFilter + real ZodValidationPipe:
  • BudgetExceededException → 422 { success:false, error:{ code:'BUDGET_EXCEEDED', … }, requestId }
  • ConflictException → 409 CONFLICT; Zod rejection of an 8-dp amount → 422 VALIDATION_ERROR with limitAmount details
  • Prisma P2002 → 409 CONFLICT, P2025 → 404 NOT_FOUND, P1000 → 400 BAD_REQUEST (typed envelope, raw error never leaks)

Results: before this PR the concurrency scenario would overspend; after, Test Files 12 passed, Tests 128 passed, plus npm run typecheck, npm run lint and npm run build all clean. The pre-existing suite (117 tests on main) had no failures; the 11 new tests are additive.

8. Additional Notes (Scope)

Single commit on feat/budget-consumption-edge-cases. Scope is the budget consumption path + the transaction pipeline's budget hooks; wallets, policies, risk, stellar, orgs and the developer API-key module are untouched.

[drips-wave]

@Bogunrot Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

[mergekeeper]

MergeKeeper review

Scope: in scope for linked issue #6.
Verdict: clean

Successfully added integration tests and the required reservation-based concurrency and budget consumption logic fulfilling issue #6.

Reviewed commit: 2992018f09c59c1c7a2085dfe97065827f1bd40d.
CI and merge eligibility are checked separately.