fix(resources): control de concurrencia optimista en PUT /resources/:id/inventory (#294)

.gitignore

@@ -31,3 +31,6 @@ apps/api/uploads/
 
 # Entorno local personal (scripts, seeds, docker extra) — no subir al repo
 .local/
+
+# Local git worktrees for parallel agent work — never commit
+.worktrees/

apps/api/drizzle/0055_resource_inventory_version.sql

@@ -0,0 +1,11 @@
+-- #294: PUT /resources/:id/inventory replaced the declared inventory with an
+-- unconditional delete+reinsert (no concurrency control), so a line merged
+-- concurrently by POST /resources/:id/inventory-entries or the donation intake
+-- worker (both via Resource.receiveInventory) between the owner's form load
+-- and their save was silently discarded (lost update). This counter backs an
+-- optimistic-concurrency check: the owner's PUT must send back the version it
+-- read (`expectedVersion`), bumped on every inventory change; a mismatch means
+-- someone else changed the inventory in the meantime and the write is
+-- rejected with 409 instead of overwriting it.
+ALTER TABLE "resources"
+  ADD COLUMN IF NOT EXISTS "inventory_version" integer NOT NULL DEFAULT 0;

apps/api/openapi.json

@@ -1124,6 +1124,7 @@
     },
     "/resources/{resourceId}/inventory": {
       "get": {
+        "description": "Returns the declared lines plus the optimistic-concurrency `version` (#294) — send it back as `expectedVersion` on the PUT below.",
         "operationId": "ResourcesController_getMyInventoryAction",
         "parameters": [
           {
@@ -1143,10 +1144,7 @@
             "content": {
               "application/json": {
                 "schema": {
-                  "type": "array",
-                  "items": {
-                    "$ref": "#/components/schemas/SupplyLineResponseDto"
-                  }
+                  "$ref": "#/components/schemas/InventoryViewDto"
                 }
               }
             }
@@ -1172,6 +1170,7 @@
         ]
       },
       "put": {
+        "description": "Optimistic concurrency (#294): `expectedVersion` must match the current `version` (read from GET) or the request fails with 409 — someone else changed the inventory (inventory-entries, a donation) since it was loaded.",
         "operationId": "ResourcesController_updateMyInventoryAction",
         "parameters": [
           {
@@ -1210,6 +1209,9 @@
           },
           "404": {
             "description": "Resource not found"
+          },
+          "409": {
+            "description": "expectedVersion is stale — the inventory changed since it was loaded (#294)"
           }
         },
         "security": [
@@ -10212,6 +10214,26 @@
           "category"
         ]
       },
+      "InventoryViewDto": {
+        "type": "object",
+        "properties": {
+          "items": {
+            "type": "array",
+            "items": {
+              "$ref": "#/components/schemas/SupplyLineResponseDto"
+            }
+          },
+          "version": {
+            "type": "number",
+            "example": 3,
+            "description": "Optimistic-concurrency version of the declared inventory. Pass back as `expectedVersion` on PUT; a mismatch means it changed since this read (#294)."
+          }
+        },
+        "required": [
+          "items",
+          "version"
+        ]
+      },
       "UpdateInventoryDto": {
         "type": "object",
         "properties": {
@@ -10221,10 +10243,16 @@
             "items": {
               "$ref": "#/components/schemas/SupplyLineDto"
             }
+          },
+          "expectedVersion": {
+            "type": "number",
+            "example": 3,
+            "description": "The inventory `version` read from GET /resources/:id/inventory. Rejected with 409 if it no longer matches the current version (#294)."
           }
         },
         "required": [
-          "items"
+          "items",
+          "expectedVersion"
         ]
       },
       "VerifyResourceDto": {

apps/api/src/contexts/resources/application/get-my-inventory.spec.ts

@@ -59,34 +59,52 @@ describe('GetMyInventory', () => {
     const bus = new FakeEventBus();
     const id = await makeResource(repo, bus);
 
-    const lines = await new GetMyInventory(repo, noMembership).execute({
+    const { items } = await new GetMyInventory(repo, noMembership).execute({
       resourceId: id,
       requesterUserId: OWNER_ID,
     });
 
-    expect(lines).toHaveLength(1);
-    expect(lines[0]).toMatchObject({ name: 'Agua', quantity: 10, unit: 'l' });
+    expect(items).toHaveLength(1);
+    expect(items[0]).toMatchObject({ name: 'Agua', quantity: 10, unit: 'l' });
+  });
+
+  // #294: the caller must be able to send this back as `expectedVersion` on
+  // PUT /resources/:id/inventory for the optimistic-concurrency check.
+  it('reports the current inventoryVersion, starting at 0 for a freshly registered resource', async () => {
+    const repo = new InMemoryResourceRepository();
+    const bus = new FakeEventBus();
+    const id = await makeResource(repo, bus);
+
+    const { version } = await new GetMyInventory(repo, noMembership).execute({
+      resourceId: id,
+      requesterUserId: OWNER_ID,
+    });
+
+    expect(version).toBe(0);
   });
 
   it('coordinator (not owner) can read', async () => {
     const repo = new InMemoryResourceRepository();
     const bus = new FakeEventBus();
     const id = await makeResource(repo, bus);
 
-    const lines = await new GetMyInventory(repo, coordOnlyMembership).execute({
+    const { items } = await new GetMyInventory(
+      repo,
+      coordOnlyMembership,
+    ).execute({
       resourceId: id,
       requesterUserId: COORD_ID,
     });
 
-    expect(lines).toHaveLength(1);
+    expect(items).toHaveLength(1);
   });
 
   it('point manager (entity-scoped grant, not owner/coordinator) can read (#316)', async () => {
     const repo = new InMemoryResourceRepository();
     const bus = new FakeEventBus();
     const id = await makeResource(repo, bus);
 
-    const lines = await new GetMyInventory(repo, noMembership).execute({
+    const { items } = await new GetMyInventory(repo, noMembership).execute({
       resourceId: id,
       requesterUserId: MANAGER_ID,
       grants: [
@@ -98,7 +116,7 @@ describe('GetMyInventory', () => {
       ],
     });
 
-    expect(lines).toHaveLength(1);
+    expect(items).toHaveLength(1);
   });
 
   it('third party → UnauthorizedInventoryChangeError', async () => {

apps/api/src/contexts/resources/application/get-my-inventory.ts

@@ -12,13 +12,24 @@ export interface GetMyInventoryQuery {
   grants?: PrincipalGrant[];
 }
 
+export interface GetMyInventoryResult {
+  items: SupplyLineSnapshot[];
+  /**
+   * Optimistic-concurrency version of the declared inventory (#294). The
+   * caller must send it back as `expectedVersion` on
+   * `PUT /resources/:id/inventory`; a mismatch there means the inventory
+   * changed since this read.
+   */
+  version: number;
+}
+
 export class GetMyInventory {
   constructor(
     private readonly repo: ResourceRepository,
     private readonly membershipReader: ResourceMembershipReader,
   ) {}
 
-  async execute(q: GetMyInventoryQuery): Promise<SupplyLineSnapshot[]> {
+  async execute(q: GetMyInventoryQuery): Promise<GetMyInventoryResult> {
     const resource = await loadResourceForManagement({
       repo: this.repo,
       membershipReader: this.membershipReader,
@@ -28,6 +39,9 @@ export class GetMyInventory {
       makeForbidden: () => new UnauthorizedInventoryChangeError(),
     });
 
-    return resource.items.map((i) => i.toSnapshot());
+    return {
+      items: resource.items.map((i) => i.toSnapshot()),
+      version: resource.inventoryVersion,
+    };
   }
 }

apps/api/src/contexts/resources/application/inventory-version-conflict.error.ts

@@ -0,0 +1,16 @@
+/**
+ * Optimistic-concurrency conflict on `PUT /resources/:id/inventory` (#294):
+ * the caller's `expectedVersion` no longer matches the resource's current
+ * `inventoryVersion` — someone else (an inventory-entry, the donation intake
+ * worker, or another PUT) changed the declared inventory since the caller
+ * loaded it. Mapped to 409 so the client reloads and retries instead of
+ * silently overwriting the concurrent change.
+ */
+export class InventoryVersionConflictError extends Error {
+  constructor() {
+    super(
+      'The inventory has changed since it was loaded; reload it and try again',
+    );
+    this.name = 'InventoryVersionConflictError';
+  }
+}

apps/api/src/contexts/resources/application/update-my-inventory.spec.ts

@@ -7,10 +7,14 @@ import { ResourceType } from '../domain/resource-enums';
 import { ResourceEmergencyStatusReader } from '../domain/ports/emergency-status-reader';
 import { ResourceMembershipReader } from '../domain/ports/membership-reader';
 import { Category } from '../../supplies/domain/category';
-import { SupplyLineProps } from '../../supplies/domain/supply-line';
+import {
+  SupplyLine,
+  SupplyLineProps,
+  SupplyLineValidationError,
+} from '../../supplies/domain/supply-line';
 import { ResourceNotFoundError } from './resource-not-found.error';
 import { UnauthorizedInventoryChangeError } from './unauthorized-inventory-change.error';
-import { SupplyLineValidationError } from '../../supplies/domain/supply-line';
+import { InventoryVersionConflictError } from './inventory-version-conflict.error';
 
 const EM = '11111111-1111-4111-8111-111111111111';
 const OWNER_ID = 'owner-user-0000-0000-000000000000';
@@ -66,6 +70,7 @@ describe('UpdateMyInventory', () => {
       resourceId: id,
       requesterUserId: OWNER_ID,
       lines: [line('Arroz', 3)],
+      expectedVersion: 0,
     });
 
     const found = await repo.findById(ResourceId.fromString(id));
@@ -81,6 +86,7 @@ describe('UpdateMyInventory', () => {
       resourceId: id,
       requesterUserId: COORD_ID,
       lines: [line('Mantas', 5)],
+      expectedVersion: 0,
     });
 
     const found = await repo.findById(ResourceId.fromString(id));
@@ -96,6 +102,7 @@ describe('UpdateMyInventory', () => {
       resourceId: id,
       requesterUserId: MANAGER_ID,
       lines: [line('Kits', 7)],
+      expectedVersion: 0,
       grants: [
         {
           roleId: 'point_manager',
@@ -126,6 +133,7 @@ describe('UpdateMyInventory', () => {
       resourceId: id,
       requesterUserId: OWNER_ID,
       lines: [line('Arroz', 3)],
+      expectedVersion: 0,
     });
 
     expect(membershipQueried).toBe(false);
@@ -140,12 +148,78 @@ describe('UpdateMyInventory', () => {
       resourceId: id,
       requesterUserId: OWNER_ID,
       lines: [],
+      expectedVersion: 0,
     });
 
     const found = await repo.findById(ResourceId.fromString(id));
     expect(found?.items).toHaveLength(0);
   });
 
+  it('a successful replace advances inventoryVersion, so a stale caller is rejected next time (#294)', async () => {
+    const repo = new InMemoryResourceRepository();
+    const bus = new FakeEventBus();
+    const id = await makeResource(repo, bus, [line('Agua', 10)]);
+
+    await new UpdateMyInventory(repo, noMembership).execute({
+      resourceId: id,
+      requesterUserId: OWNER_ID,
+      lines: [line('Arroz', 3)],
+      expectedVersion: 0,
+    });
+
+    const found = await repo.findById(ResourceId.fromString(id));
+    expect(found?.inventoryVersion).toBe(1);
+
+    // Retrying with the now-stale version (0) must fail, not overwrite again.
+    await expect(
+      new UpdateMyInventory(repo, noMembership).execute({
+        resourceId: id,
+        requesterUserId: OWNER_ID,
+        lines: [line('Mantas', 1)],
+        expectedVersion: 0,
+      }),
+    ).rejects.toBeInstanceOf(InventoryVersionConflictError);
+  });
+
+  it('#294: a concurrent merge (receiveInventory) between the form load and the save is NOT silently discarded', async () => {
+    // Reproduces the reported lost-update scenario: the owner opens the
+    // inventory edit form (reads version 0, seeded with line A); while the
+    // form is open, an operator/worker merges in line B via
+    // POST /resources/:id/inventory-entries (Resource.receiveInventory),
+    // bumping the version to 1. The owner then saves the form unchanged
+    // (still carrying expectedVersion 0) — the PUT must reject the stale
+    // write with a conflict instead of overwriting B away.
+    const repo = new InMemoryResourceRepository();
+    const bus = new FakeEventBus();
+    const id = await makeResource(repo, bus, [line('Agua', 10)]);
+    const loadedVersion = 0;
+
+    // Concurrent merge lands first (operator records a manual entry).
+    const resource = await repo.findById(ResourceId.fromString(id));
+    resource!.receiveInventory([SupplyLine.create(line('Mantas', 5))]);
+    await repo.save(resource!);
+
+    const beforeOverwrite = await repo.findById(ResourceId.fromString(id));
+    expect(beforeOverwrite?.items.map((i) => i.name).sort()).toEqual([
+      'Agua',
+      'Mantas',
+    ]);
+
+    // Owner's stale form save (still thinks the version is 0) must be rejected.
+    await expect(
+      new UpdateMyInventory(repo, noMembership).execute({
+        resourceId: id,
+        requesterUserId: OWNER_ID,
+        lines: [line('Agua', 10)],
+        expectedVersion: loadedVersion,
+      }),
+    ).rejects.toBeInstanceOf(InventoryVersionConflictError);
+
+    // Mantas must still be there — the concurrent merge was NOT lost.
+    const after = await repo.findById(ResourceId.fromString(id));
+    expect(after?.items.map((i) => i.name).sort()).toEqual(['Agua', 'Mantas']);
+  });
+
   it('third party (not owner, not coordinator) → UnauthorizedInventoryChangeError', async () => {
     const repo = new InMemoryResourceRepository();
     const bus = new FakeEventBus();
@@ -156,6 +230,7 @@ describe('UpdateMyInventory', () => {
         resourceId: id,
         requesterUserId: THIRD_ID,
         lines: [line('Agua', 1)],
+        expectedVersion: 0,
       }),
     ).rejects.toBeInstanceOf(UnauthorizedInventoryChangeError);
   });
@@ -168,6 +243,7 @@ describe('UpdateMyInventory', () => {
         resourceId: '99999999-9999-4999-8999-999999999999',
         requesterUserId: OWNER_ID,
         lines: [],
+        expectedVersion: 0,
       }),
     ).rejects.toBeInstanceOf(ResourceNotFoundError);
   });
@@ -182,6 +258,7 @@ describe('UpdateMyInventory', () => {
         resourceId: id,
         requesterUserId: OWNER_ID,
         lines: [{ ...line('Agua', 0) }],
+        expectedVersion: 0,
       }),
     ).rejects.toBeInstanceOf(SupplyLineValidationError);
   });