Skip to content

feat: v0.3.0 inventory foundation — locations removal, BigBang migration, inventory schema contract#207

Merged
GitAddRemote merged 38 commits into
mainfrom
feature/v0.3.0-inventory-mvp
May 23, 2026
Merged

feat: v0.3.0 inventory foundation — locations removal, BigBang migration, inventory schema contract#207
GitAddRemote merged 38 commits into
mainfrom
feature/v0.3.0-inventory-mvp

Conversation

@GitAddRemote
Copy link
Copy Markdown
Owner

Summary

Issues closed by this PR

Closes #186, #187, #201

Test plan

  • pnpm typecheck passes (verified locally)
  • pnpm test -- --testPathPattern="user-inventory|org-inventory" passes — 27 tests, 2 suites (verified locally)
  • pnpm migration:run on a clean database succeeds with zero errors
  • pnpm migration:revert executes down() without errors on a freshly migrated database
  • SELECT count(*) FROM information_schema.tables WHERE table_schema = 'public' after migration run matches expected table count
  • pnpm db:health-check passes after migration run
  • INSERT INTO org_inventory_item (quantity, unit_of_measure) VALUES (0.000125, 'uscu') succeeds
  • INSERT INTO org_inventory_item (..., quality) VALUES (..., -1) is rejected by CHECK constraint

…ntion

All database table names standardised to singular form across entity
decorators and the station-normalized-schema.md reference doc.
No migrations affected — changes will be reflected in BigBang baseline.
…ngle schema foundation (ISSUE-187)

- Replace 27 incremental migrations with a single 1748000000000-BigBangBaselineMigration
- Drop deprecated locations table (ISSUE-186) and refresh_token table (moved to Redis)
- Add inventory schema: unit_of_measure enum, DECIMAL(12,6) quantity, quality smallint, location_type/location_uex_id replacing FK
- Add org_shared_inventory_summary as a regular VIEW grouped by (org_id, uex_item_id, unit_of_measure)
- Add all 26 station_* normalized location tables plus station_etl_warning and station_etl_run_state
- Include seed data: system user, Admin/Inventory Manager/Member roles, uex_sync_config with 7 endpoints
- Update data-source.ts to import only BigBangBaselineMigration
…UE-201)

- Add unitOfMeasure, quality, locationType, locationUexId fields to org and user inventory entities
- Remove locationId FK and ManyToOne location relation from both entities
- Update create/update/search DTOs: unitOfMeasure enum, quality (0–32767), minQuality/maxQuality filters
- Remove locationName and locationId from response DTOs
- Default unitOfMeasure to 'unit' in service insert paths
- Remove location joins from repository queries
- Update service spec assertions to include unitOfMeasure default
Copilot AI review requested due to automatic review settings May 21, 2026 16:25
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR lays the groundwork for inventory v0.3.0 by removing the deprecated locations module, consolidating the backend schema into a single baseline (“BigBang”) migration using singular table names, and updating inventory APIs/entities/DTOs to the new schema contract (unit of measure, quality, and non-FK location identity).

Changes:

  • Removed the legacy locations backend module and related frontend location picker/cache code paths.
  • Replaced the incremental migration chain with a single 1748000000000-BigBangBaselineMigration baseline migration and updated the data source to reference it.
  • Updated inventory domain models and APIs to drop location_id FK and add unit_of_measure, quality, location_type, and location_uex_id, plus new search/sort options.

Reviewed changes

Copilot reviewed 81 out of 83 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
frontend/src/services/locationCache.ts Removed storable locations caching layer.
frontend/src/services/location.service.ts Removed frontend locations API client.
frontend/src/services/inventory.service.ts Removed location-related fields/filters/sort options from inventory client types and query builders.
frontend/src/pages/Inventory.editor-mode.test.tsx Updated inventory editor-mode tests to reflect removal of location editing.
frontend/src/hooks/useMemoizedLocations.ts Removed memoized location filtering hook.
frontend/src/components/location/SystemLocationSelector.tsx Removed system+location selector UI component.
frontend/src/components/inventory/InventoryNewRow.tsx Removed “new row” location input/validation props and UI.
frontend/src/components/inventory/InventoryInlineRow.tsx Removed inline location editing; renders placeholder for location display.
frontend/src/components/inventory/InventoryFiltersPanel.tsx Removed location filter/group/sort controls from the filters panel.
backend/src/modules/user-inventory/user-inventory.service.ts Removed location joins/filters; added unit/quality filters and quality sort; updated DTO mapping.
backend/src/modules/user-inventory/user-inventory.service.spec.ts Updated unit tests to remove location expectations and reflect new fields/sort behavior.
backend/src/modules/user-inventory/user-inventory.module.ts Removed Location entity from module TypeORM feature list.
backend/src/modules/user-inventory/user-inventory.controller.ts Removed locationId query param; added min/max quality and unitOfMeasure query params; added quality sort option.
backend/src/modules/user-inventory/inventory-sharing.service.ts Removed location relation usage when sharing inventory.
backend/src/modules/user-inventory/entities/user-inventory-item.entity.ts Renamed table to singular; removed location_id relation; added unit_of_measure/quality/location_type/location_uex_id.
backend/src/modules/user-inventory/dto/user-inventory-item.dto.ts Removed locationId/locationName; added unitOfMeasure/quality/locationType/locationUexId and quality/unit filters/sort; updated quantity bounds.
backend/src/modules/uex/entities/uex-star-system.entity.ts Renamed UEX table mapping to singular form.
backend/src/modules/uex/entities/uex-space-station.entity.ts Renamed UEX table mapping to singular form.
backend/src/modules/uex/entities/uex-planet.entity.ts Renamed UEX table mapping to singular form.
backend/src/modules/uex/entities/uex-outpost.entity.ts Renamed UEX table mapping to singular form.
backend/src/modules/uex/entities/uex-moon.entity.ts Renamed UEX table mapping to singular form.
backend/src/modules/uex/entities/uex-item.entity.ts Renamed UEX table mapping to singular form.
backend/src/modules/uex/entities/uex-company.entity.ts Renamed UEX table mapping to singular form.
backend/src/modules/uex/entities/uex-commodity.entity.ts Renamed UEX table mapping to singular form.
backend/src/modules/uex/entities/uex-city.entity.ts Renamed UEX table mapping to singular form.
backend/src/modules/uex/entities/uex-category.entity.ts Renamed UEX table mapping to singular form.
backend/src/modules/uex-sync/uex-sync.module.ts Removed LocationsSyncService wiring.
backend/src/modules/uex-sync/uex-sync.controller.ts Removed “locations” endpoint option from admin sync trigger; simplified endpoint typing.
backend/src/modules/uex-sync/uex-sync.controller.spec.ts Updated controller tests to reflect removed “locations” endpoint.
backend/src/modules/uex-sync/schedulers/uex-sync.scheduler.ts Removed scheduled locations sync cron job and LocationsSyncService dependency.
backend/src/modules/uex-sync/dto/sync-health.dto.ts Removed “locations” from allowed endpoints values for sync trigger DTO.
backend/src/modules/org-inventory/org-inventory.service.ts Removed location references; added unitOfMeasure defaulting and quality/unit filters; updated conflict message and DTO mapping.
backend/src/modules/org-inventory/org-inventory.service.spec.ts Updated org-inventory service tests to remove location-based expectations and add unitOfMeasure default.
backend/src/modules/org-inventory/org-inventory.repository.ts Removed location relation joins and location-based filters/sorts; added quality/unit filters and quality sort.
backend/src/modules/org-inventory/org-inventory.module.ts Removed Location entity from module TypeORM feature list.
backend/src/modules/org-inventory/org-inventory.controller.ts Removed locationId query param; added min/max quality and unitOfMeasure query params; added quality sort option.
backend/src/modules/org-inventory/org-inventory.controller.spec.ts Updated controller tests to reflect removed location query param validation.
backend/src/modules/org-inventory/entities/org-inventory-item.entity.ts Renamed table to singular; removed location_id relation/index; added unit_of_measure/quality/location_type/location_uex_id.
backend/src/modules/org-inventory/dto/org-inventory-item.dto.ts Removed locationId/locationName; added unitOfMeasure/quality/locationType/locationUexId, quality/unit filters/sort, and updated quantity bounds/swagger docs.
backend/src/modules/oauth-clients/oauth-client.entity.ts Renamed table mapping to singular form.
backend/src/modules/games/game.entity.ts Renamed table mapping from games to game.
backend/src/modules/auth/refresh-token.entity.ts Renamed table mapping to singular form (even though baseline migration omits refresh_token table).
backend/src/modules/auth/password-reset.entity.ts Renamed table mapping to singular form.
backend/src/migrations/1748000000000-BigBangBaselineMigration.ts Added baseline migration creating full schema from scratch with singular naming and updated inventory schema.
backend/src/data-source.ts Removed old migrations and Location entity; referenced only the BigBang baseline migration.
backend/src/app.module.ts Removed LocationsModule from Nest application module imports.
backend/src/modules/locations/locations.service.ts Deleted deprecated locations service.
backend/src/modules/locations/locations.service.spec.ts Deleted locations service tests.
backend/src/modules/locations/locations.module.ts Deleted locations Nest module.
backend/src/modules/locations/locations.controller.ts Deleted locations controller and /api/locations endpoints.
backend/src/modules/locations/location-population.service.ts Deleted UEX→locations population service.
backend/src/modules/locations/location-population.service.spec.ts Deleted population service tests.
backend/src/modules/locations/entities/location.entity.ts Deleted deprecated Location entity.
backend/src/modules/locations/dto/location.dto.ts Deleted locations DTOs.
backend/src/migrations/* (multiple files) Deleted superseded incremental migrations (replaced by the baseline migration).
Comments suppressed due to low confidence (1)

backend/src/modules/user-inventory/user-inventory.service.ts:162

  • create() merges into an existing row based on (user_id, game_id, uex_item_id, shared_org_id) only. Now that inventory rows include unit_of_measure and the new (location_type, location_uex_id) location contract, this merge can incorrectly combine records from different units and/or different locations, losing location info and mixing quantities. The lookup should include unit_of_measure (defaulting to 'unit' when omitted) and location identity (e.g., COALESCE comparison on location_type + location_uex_id).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread backend/src/migrations/1748000000000-BigBangBaselineMigration.ts
Comment thread backend/src/migrations/1748000000000-BigBangBaselineMigration.ts
Comment thread backend/src/migrations/1748000000000-BigBangBaselineMigration.ts
Comment thread backend/src/modules/org-inventory/org-inventory.service.ts
Comment thread backend/src/modules/org-inventory/org-inventory.repository.ts Outdated
- BigBang migration: add firstName/lastName/phoneNumber/bio to user table
- BigBang migration: add game_id column to organization table with FK added post-game table creation
- BigBang migration: fix user_organization_role.userId type from INTEGER to BIGINT to match user.id BIGSERIAL
- org-inventory service: parseFloat quantity from DECIMAL to avoid string serialization
- org-inventory repository: expand findExistingItem conflict key to include unitOfMeasure and location identity
- frontend InventoryPortlet: replace stale locationName/locationId with locationType/locationUexId
- frontend InventoryItem type: add unitOfMeasure, quality, locationType, locationUexId fields
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 82 out of 84 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (2)

backend/src/modules/user-inventory/user-inventory.service.ts:162

  • In create(), the merge/duplicate-detection query no longer includes unit_of_measure or the new (location_type, location_uex_id) identity. This will merge quantities for the same uexItemId across different units/locations into a single row, losing correctness. Include unit_of_measure (defaulting to 'unit') and nullable location identity (COALESCE/IS NULL semantics) in the WHERE clause, and ensure the same fields are used when deciding whether to merge.
    backend/src/modules/user-inventory/inventory-sharing.service.ts:108
  • When splitting an item to share part of its quantity, the newly created UserInventoryItem record does not copy unitOfMeasure, quality, locationType, or locationUexId from the source item. This will silently change units/metadata on the shared record (e.g., SCU becomes default 'unit', location cleared). Populate the new record with the source item's unit/quality/location fields as well.

Comment thread backend/src/modules/user-inventory/dto/user-inventory-item.dto.ts
Comment thread backend/src/modules/org-inventory/dto/org-inventory-item.dto.ts
Comment thread frontend/src/components/inventory/InventoryPortlet.tsx
Comment thread backend/src/modules/user-inventory/user-inventory.controller.ts
Comment thread backend/src/modules/org-inventory/org-inventory.controller.ts
- DTOs: replace @IsEnum(array) with @isin() for unitOfMeasure validation
- DTOs: add @max(32767) to quality/minQuality/maxQuality fields to match SMALLINT
- DTOs: add @maxlength(30) to locationType fields to match VARCHAR(30)
- Controllers: validate unitOfMeasure query param against allowed set, throw 400 on invalid input
- InventoryPortlet: guard location display against null locationUexId; require both locationType and locationUexId to render label
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 82 out of 84 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

backend/src/modules/user-inventory/user-inventory.service.ts:160

  • In create(), the merge lookup only keys on (user_id, game_id, uex_item_id, shared_org_id). With the new schema, this will incorrectly merge inventory rows that differ by unitOfMeasure and/or (locationType, locationUexId), causing data loss (e.g., merging SCU cargo into unit-count items, or different locations into one row). Update the existing query to include unit_of_measure (defaulting to 'unit' when not provided) and the nullable location identity fields (using COALESCE/IS NULL semantics) so only truly identical rows are merged.
    backend/src/modules/user-inventory/inventory-sharing.service.ts:108
  • When splitting an item to share with an org, the newly created UserInventoryItem does not copy over unitOfMeasure, quality, locationType, or locationUexId from the source row. This will silently change units/lose location + quality context for the shared record (and can create incorrect aggregates). Include these fields when building newItem so the shared row preserves the original schema contract.

Comment thread backend/src/modules/user-inventory/dto/user-inventory-item.dto.ts
Comment thread backend/src/modules/org-inventory/dto/org-inventory-item.dto.ts
Comment thread frontend/src/components/inventory/InventoryInlineRow.tsx Outdated
- Backend DTOs: reduce @max from 999999999.999999 to 999999.999999 to match
  DECIMAL(12,6) max storable value and prevent DB overflow on valid API requests
- Frontend: lower MIN_INVENTORY_QUANTITY from 0.01 to 0.000001 to allow μSCU
  precision entries; lower EDITOR_MODE_QUANTITY_MAX to 999999.999999 to match DB ceiling
- Frontend: update all inputProps min/step from 0.01 to 0.000001 across
  InventoryInlineRow, Inventory.tsx new-item and action forms
- Frontend: update stepper buttons to use 0.000001 floor and 6 decimal places
- Frontend: update valueRange defaults to 999999.999999 in Inventory.tsx and
  InventoryFiltersPanel reset
- Frontend tests: update quantity error text and max-clamp assertions to new values
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 82 out of 84 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

backend/src/modules/user-inventory/user-inventory.service.ts:162

  • create() merge lookup still only matches on (user_id, game_id, uex_item_id, shared_org_id). With the new schema, distinct rows can differ by unit_of_measure, location_type/location_uex_id, and potentially quality. As written, a create for the same item but different unit/location/quality will be incorrectly merged into the existing row (silently corrupting inventory state). Update the merge WHERE clause (and/or add a unique index) so the merge key matches the new schema contract (at least unit_of_measure + location identity; consider including quality as well).
    backend/src/modules/user-inventory/inventory-sharing.service.ts:108
  • When splitting an item to share with an org, the newly created shared row does not copy the new schema fields (unitOfMeasure, quality, locationType, locationUexId) from the original item. This will default unit_of_measure to 'unit' and drop location/quality on the shared record, producing incorrect data. Include these fields when creating newItem (or otherwise ensure the shared row preserves the original item’s unit/location/quality).

Comment thread backend/src/modules/uex-sync/uex-sync.controller.ts
Comment thread backend/src/migrations/1748000000000-BigBangBaselineMigration.ts
star_systems, planets, and space_stations had no live sync service in this PR
(locations sync was removed in #186). Seeding them into uex_sync_state and
uex_sync_config would leave them permanently stale in health monitoring. These
entries will be seeded by the ETL migrations for #191, #192, and #193 when
those sync steps are implemented.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 82 out of 84 changed files in this pull request and generated 3 comments.

Comment thread backend/src/modules/user-inventory/user-inventory.service.ts
Comment thread backend/src/modules/user-inventory/inventory-sharing.service.ts
Comment thread frontend/src/services/inventory.service.ts
…nd search

- user-inventory service: include unit_of_measure and COALESCE location
  identity (location_type, location_uex_id) in merge-detection query so
  items with different units or locations are not incorrectly merged
- inventory-sharing service: copy unitOfMeasure, quality, locationType,
  locationUexId from original item to newly split shared record
- frontend inventory.service: add unitOfMeasure, minQuality, maxQuality to
  InventorySearchParams and buildInventoryQuery; add same plus quality sort
  option to buildOrgInventoryQuery params
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 82 out of 84 changed files in this pull request and generated 9 comments.

Comments suppressed due to low confidence (3)

backend/src/modules/user-inventory/dto/user-inventory-item.dto.ts:64

  • locationUexId has no lower-bound validation. Because UserInventoryService.create() uses COALESCE(location_uex_id, -1) for merge matching, a client-provided -1 would collide with NULL and could merge distinct rows incorrectly. Add a minimum constraint (and/or explicitly forbid -1) and apply the same rule to the update DTO.
    backend/src/modules/org-inventory/dto/org-inventory-item.dto.ts:99
  • Same as user inventory: locationUexId lacks a minimum bound. Negative values (especially -1) don’t make sense for UEX IDs and can break uniqueness/merge semantics (since other code uses -1 as a NULL sentinel). Add @Min(1) (or at least forbid -1) and mirror it on the update DTO.
    backend/src/modules/user-inventory/user-inventory.service.ts:166
  • Merge matching uses COALESCE(location_uex_id, -1) with -1 as a NULL sentinel. Because DTO validation currently allows negative integers, a client-provided locationUexId = -1 would collide with NULL and can merge unrelated rows. Add input validation/normalization (and/or switch to NULL-safe equality without sentinel values).

Comment thread backend/src/modules/user-inventory/dto/user-inventory-item.dto.ts
Comment thread backend/src/modules/org-inventory/dto/org-inventory-item.dto.ts
Comment thread backend/src/modules/user-inventory/user-inventory.service.ts
Comment thread backend/src/migrations/1748000000000-BigBangBaselineMigration.ts
Comment thread backend/src/migrations/1748000000000-BigBangBaselineMigration.ts
Comment thread backend/src/modules/org-inventory/org-inventory.service.ts Outdated
Comment thread frontend/src/services/inventory.service.ts
Comment thread backend/src/modules/uex/entities/uex-item.entity.ts
Comment thread backend/src/modules/uex/entities/uex-commodity.entity.ts
…table names, and missing validation

- Replace COALESCE(-1/empty) sentinels in user-inventory merge query with IS NOT DISTINCT FROM to prevent false merges when locationUexId=-1 or locationType=''
- Add @isnotempty() to locationType DTOs (user and org) to prevent empty-string ambiguity with NULL in merge detection
- Add @min(1) to locationUexId in Create and Update DTOs for both user and org inventory to forbid the -1 sentinel value at the API boundary
- Wrap org-inventory create in a pessimistic-locked transaction (FOR UPDATE) to close the check-then-insert race condition
- Add DB-level partial unique indexes (uq_user_inv_composite, uq_org_inv_composite) on both inventory tables to enforce composite identity at the database layer
- Rename raw SQL ON CONFLICT references from uex_items → uex_item and uex_commodities → uex_commodity to match singular table names from baseline migration
- Add unitOfMeasure, minQuality, maxQuality, and quality sort to getOrgInventory params type in frontend inventory service to align with backend API and query builder
- Update org-inventory service spec to inject DataSource mock and rewrite create tests around transactional query builder path
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

… input, env cleanup

- Wire GET /api/orgs/:orgId/inventory/summary to query org_shared_inventory_summary
  view and return per-(uex_item_id, unit_of_measure) aggregates instead of simple
  counts; update OrgInventorySummaryDto accordingly (#201)
- Org inventory create() now merges quantity via SQL NUMERIC arithmetic when an
  identical row exists, fixing split failures in org mode (same pattern as
  user-inventory)
- InventoryNewRow quantity field: inputMode 'decimal' + pattern allows fractional
  input on mobile keyboards, aligning with DECIMAL(12,6) quantity support (#201)
- Remove UEX_LOCATIONS_SYNC_ENABLED from env.validation.ts, .env.production.example,
  and .env.staging.example — locations sync was removed (#186)
Add allowDuplicate?: boolean to CreateUserInventoryItemDto and
CreateOrgInventoryItemDto. When true, both create() service methods skip
the merge-on-matching-identity path and always insert a new row.

Pass allowDuplicate: true from handleSplit() in Inventory.tsx so that
splitting an existing stack always yields two independent rows instead of
merging the new quantity back into the source. Also forward all identity
fields (unitOfMeasure, quality, locationType, locationUexId) from the
source item to ensure the split row has the same identity fingerprint
without triggering the merge guard.

Add unit tests covering the allowDuplicate bypass in both service specs.
Copilot AI review requested due to automatic review settings May 22, 2026 20:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Drop the allowDuplicate flag approach which bypassed the service merge
check but still collided against the DB-level partial unique indexes
(uq_user_inv_composite, uq_org_inv_composite) on active, non-deleted rows.

Add POST /api/inventory/:id/split and POST /api/orgs/:orgId/inventory/:id/split
endpoints that execute a single atomic transaction:
  1. Pessimistic-lock and fetch the source row
  2. Validate splitQuantity < source quantity
  3. Soft-delete the source (deleted=TRUE, active=FALSE) — clears it from
     both unique indexes (both are WHERE deleted=FALSE AND active=TRUE)
  4. Insert two new rows (remaining and split quantities) with the same
     identity; no collision because source is now excluded from the index

Frontend handleSplit() now calls splitItem / splitOrgItem and drops the
two-step update+create pattern entirely.
…ctive rows

uq_user_inv_composite and uq_org_inv_composite enforce uniqueness over the
identity columns (user/org, game, item, unit_of_measure, location, sharedOrg)
for active non-deleted rows. Since quantity is not part of the index, two rows
produced by a split operation — which share the same identity — would collide
on the second insert even after the source row is soft-deleted.

Deduplication on create is already handled by the pessimistic-lock +
merge-on-match check in both service create() methods. The indexes added
nothing beyond what the service enforces and actively prevented split.
Copilot AI review requested due to automatic review settings May 22, 2026 21:42
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

…-save collision check

The unique composite indexes were dropped to allow split, but that
removed the only protection against two concurrent creates producing
duplicate active stacks (no row to pessimistic-lock when neither exists
yet) and against update() silently creating identity collisions.

Three-part fix:
1. Restore the identity indexes as non-unique (idx_user_inv_identity,
   idx_org_inv_identity) — same columns as before, still accelerate the
   merge lookup and the new collision query, but no longer block split's
   second insert.

2. Replace pessimistic_write lock in create() with pg_advisory_xact_lock
   keyed on the identity string. The advisory lock serializes concurrent
   "no existing row" requests at the transaction level without requiring
   a row to lock, so two simultaneous creates for the same identity cannot
   both observe "nothing found" and both insert.

3. Add an explicit pre-save identity collision query in both update()
   paths. If any other active non-deleted row already has the post-update
   identity, a ConflictException is raised before save, restoring the
   protection that the former 23505 backstop provided.
update() performed a read-time collision check outside any transaction,
so two concurrent updates (or a concurrent create() and update()) could
both observe no collision and commit duplicate active stacks.

Wrap the identity check and save in the same transaction as create(),
and acquire pg_advisory_xact_lock on the post-update identity key before
the check. The lock key format matches create() so create and update
mutually exclude each other for the same identity. The 23505 catch block
is kept as a last-resort guard inside the transaction.

Update service specs to exercise the collision and happy-path through the
transaction mock (transactionRepository.save / transactionQueryBuilder.getOne)
rather than directly on the outer repository mock.
Copilot AI review requested due to automatic review settings May 22, 2026 22:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

…e lock

Both user-inventory and org-inventory update() methods previously loaded
the source row outside the transaction, leaving a TOCTOU window where a
concurrent split() or delete() could modify the row between the load and
the advisory-lock+save inside the transaction.

- Fetch the row inside the transaction with setLock('pessimistic_write')
  so the row is pinned for the entire update lifecycle
- Acquire the advisory lock on the post-update identity after the row
  is loaded, before the collision check and save
- Remove the pre-transaction findByIdNotDeleted/findInventoryItem calls
  in update(); NotFoundException is now raised from inside the transaction
- Update spec helpers (buildUpdateTxManager) to supply two sequential
  getOne results: first for the row fetch, second for the collision check
@GitAddRemote GitAddRemote merged commit 70150d8 into main May 23, 2026
12 checks passed
@GitAddRemote GitAddRemote deleted the feature/v0.3.0-inventory-mvp branch May 23, 2026 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove deprecated locations module and all associated code

2 participants