fix: allow reducing an oversubscribed QUANTITY_TRACKED booking

[iuryeng]

Fixes #2725

Problem

Once a QUANTITY_TRACKED asset's pool is over-reserved by other bookings, computeBookingAvailableQuantity returns a negative available (total - inCustody - reserved, unclamped). The adjust-quantity guard compared the submitted quantity against that unconditionally:

if (quantity > availability.available) { /* 400 */ }

With available = -7, every positive submission satisfies quantity > -7, including a reduction all the way down to 1. There was no number a user could enter to recover — confirmed against the exact repro in the issue.

Root cause

apps/webapp/app/routes/api+/bookings.$bookingId.adjust-asset-quantity.ts:181 measured every submission against availability, regardless of direction. A reduction of this booking's own reservation can never oversubscribe the pool — every other booking's reservation, the custody count, and the total stock are untouched by this booking's own delta — so only an increase needs to be checked.

Fix

• Only measure an increase (relative to the current booked quantity) against remaining availability. A reduction always proceeds.
• The "current" quantity is re-read inside the transaction, under the already-held asset lock (tx.bookingAsset.findUniqueOrThrow), rather than trusting the pre-transaction snapshot — closes a narrow TOCTOU window and matches the existing pattern in modules/asset/service.server.ts:2628.
• The rejection message for a still-blocked increase is clamped to never show a negative "available" count, and explains when the pool is already over-committed by other bookings.

Testing

Added 4 regression tests to the existing mocked-route test (test/routes-tests/api+/bookings.$bookingId.adjust-asset-quantity.test.ts, same style as the pre-existing ownership-guard suite — no real DB needed):

• Reduces a booking's quantity even when the pool is oversubscribed (the reported bug) — and asserts computeBookingAvailableQuantity is not even called for a reduction.
• Still rejects an increase beyond remaining availability, with the clamped/explanatory message.
• Allows an increase that fits within remaining availability.
• Treats a resubmitted, unchanged quantity as a pass-through.

All 11 tests (7 existing + 4 new) pass. eslint, prettier, and typecheck are clean. This PR also went through the repo's Claude-powered pre-commit security reviewer three times across iterations — final verdict: Approve with notes, Low risk. Two low-severity notes worth being upfront about:

1. The over-commitment count in the error message is same-org-only info, exposed to a caller who already passed the route's permission gate — flagged as informational, not a blocker.
2. computeBookingAvailableQuantity still reads on the ambient db client rather than tx — pre-existing, not introduced by this diff; the asset-level lock still serializes the common case. Noted for a possible follow-up, out of scope here.

The reviewer also asked (twice) to confirm the reduction branch can't be reached with quantity <= 0 now that it skips the availability check — confirmed: the Zod schema (AdjustBookingAssetQuantitySchema, above this hunk) already enforces z.coerce.number().int().positive(...), so 0/negative values are rejected before this code runs. No change needed there.

Scope note: a related, narrower frontend gap (not fixed here)

While tracing the consuming dialog (components/booking/adjust-booking-asset-quantity-dialog.tsx), I noticed its maxQuantity prop is documented as "available + currently booked" but its only caller (asset-row-actions-dropdown.tsx:231) passes asset.quantity (total workspace stock) instead. For the numbers in this issue that doesn't block anything (a reduction target is well under total stock), which is why the reported symptom is fully explained — and fully fixed — by the backend change alone. But it means a target reduction larger than total stock (e.g. total=2, reduce a 17-unit overbooking down to 5) would still be incorrectly blocked client-side before ever reaching this now-fixed backend guard. Flagging it as a separate, narrower issue rather than folding it into this PR — happy to open a follow-up if useful.

What I didn't do

No full local E2E (real Postgres/Supabase + browser) — validated via the route-level test that exercises the real action() function with only the DB layer mocked, matching this repo's own established convention for this exact route (the pre-existing 7 ownership-guard tests use the identical technique). Happy to do a fuller manual pass if useful before merge.

Summary by CodeRabbit

• Bug Fixes
  • Adjusting a tracked asset quantity now correctly handles over-committed availability.
  • Reducing quantities remains allowed even when availability is negative.
  • Increasing quantities is validated against current availability, with clearer over-commitment details when rejected.
  • Activity records now reflect the latest quantity accurately.
• Tests
  • Added coverage for increases, reductions, unchanged quantities, and over-committed inventory scenarios.

[coderabbitai]

Walkthrough

The booking asset adjustment route now re-reads the locked quantity, checks availability only for increases, permits reductions in over-committed pools, and records the locked baseline. Tests cover transaction behavior, accepted changes, rejected increases, and error messaging.

Changes

Booking quantity adjustment

Layer / File(s)	Summary
Locked quantity and directional availability guard apps/webapp/app/routes/api+/bookings.$bookingId.adjust-asset-quantity.ts	The route re-reads the current booking-asset quantity after locking, applies availability validation only to increases, enhances over-commit errors, and uses the re-read quantity for activity logging.
Directional guard regression coverage apps/webapp/test/routes-tests/api+/bookings.$bookingId.adjust-asset-quantity.test.ts	Transaction mocks and tests cover reductions, unchanged quantities, valid increases, rejected increases, availability calls, persistence, and clamped over-commit messaging.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: donkoko

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant AdjustRoute as adjust-asset-quantity route
  participant BookingAsset as bookingAsset transaction
  participant Availability as computeBookingAvailableQuantity

  Client->>AdjustRoute: submit requested quantity
  AdjustRoute->>BookingAsset: lock and re-read current quantity
  alt requested quantity increases
    AdjustRoute->>Availability: compute available quantity
    Availability-->>AdjustRoute: return availability
  end
  AdjustRoute->>BookingAsset: persist accepted quantity
  AdjustRoute-->>Client: return adjustment result or validation error

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main fix: allowing reductions for oversubscribed QUANTITY_TRACKED bookings.
Linked Issues check	✅ Passed	The route change and regression tests implement the directional availability guard requested in #2725.
Out of Scope Changes check	✅ Passed	The changes stay focused on the booking quantity adjustment fix and its tests, with no clear unrelated additions.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

apps/webapp/test/routes-tests/api+/bookings.$bookingId.adjust-asset-quantity.test.ts (1)

348-352: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Prefer outcome assertions over helper-call assertions.

These assertions couple the route tests to computeBookingAvailableQuantity internals. The response status, error content, and persisted quantity already cover the user-visible directional behavior.

As per coding guidelines, “Write behavior-driven tests focusing on observable outcomes rather than implementation details” and “Avoid testing internal private methods or state.”

Also applies to: 375-375, 399-399, 422-424

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@apps/webapp/test/routes-tests/api`+/bookings.$bookingId.adjust-asset-quantity.test.ts
around lines 348 - 352, Replace the computeBookingAvailableQuantity call-count
assertions in the booking adjustment tests with observable outcome assertions.
For each affected reduction/increase scenario around the route tests, assert the
response status and error content where applicable, and verify the persisted
booking quantity; remove assertions that inspect
consumptionMocks.computeBookingAvailableQuantity invocation details.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@apps/webapp/test/routes-tests/api`+/bookings.$bookingId.adjust-asset-quantity.test.ts:
- Around line 348-352: Replace the computeBookingAvailableQuantity call-count
assertions in the booking adjustment tests with observable outcome assertions.
For each affected reduction/increase scenario around the route tests, assert the
response status and error content where applicable, and verify the persisted
booking quantity; remove assertions that inspect
consumptionMocks.computeBookingAvailableQuantity invocation details.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 41af5522-d1ce-4001-8a13-3a3dd6780354

📥 Commits

Reviewing files that changed from the base of the PR and between 04d6aeb and 70afa62.

📒 Files selected for processing (2)

• apps/webapp/app/routes/api+/bookings.$bookingId.adjust-asset-quantity.ts
• apps/webapp/test/routes-tests/api+/bookings.$bookingId.adjust-asset-quantity.test.ts