fix(packing): accept weight, bag and quantity on create across REST, MCP and plugin RPC (#2154)

server/tests/e2e/packing.e2e.test.ts

@@ -207,6 +207,47 @@ describe('Packing e2e (real auth guard + real SQL over temp SQLite)', () => {
     expect(db.prepare('SELECT name FROM packing_items WHERE id = ?').get(res.body.item.id)).toEqual({ name: 'Socks' });
   });
 
+  it('201 on create persists weight_grams, bag_id and quantity (#2154)', async () => {
+    const bag = await request(server).post(`/api/trips/${tripId}/packing/bags`).set('Cookie', sessionCookie(1)).send({ name: 'Carry-On' });
+    const res = await request(server)
+      .post(`/api/trips/${tripId}/packing`)
+      .set('Cookie', sessionCookie(1))
+      .send({ name: 'Tent', weight_grams: 250, bag_id: bag.body.bag.id, quantity: 3 });
+    expect(res.status).toBe(201);
+    expect(res.body.item).toMatchObject({ name: 'Tent', weight_grams: 250, bag_id: bag.body.bag.id, quantity: 3 });
+    expect(db.prepare('SELECT weight_grams, bag_id, quantity FROM packing_items WHERE id = ?').get(res.body.item.id))
+      .toEqual({ weight_grams: 250, bag_id: bag.body.bag.id, quantity: 3 });
+  });
+
+  it('400 "Bag not found" on create for a bag off the trip; camelCase keys still strip (#2154)', async () => {
+    // A REAL bag on a different trip — existence alone must not be enough.
+    const otherTripId = Number(db.prepare('INSERT INTO trips (user_id, title) VALUES (1, ?)').run('Other').lastInsertRowid);
+    const foreignBag = Number(db.prepare('INSERT INTO packing_bags (trip_id, name) VALUES (?, ?)').run(otherTripId, 'Foreign').lastInsertRowid);
+
+    const cross = await request(server).post(`/api/trips/${tripId}/packing`).set('Cookie', sessionCookie(1)).send({ name: 'Tent', bag_id: foreignBag });
+    expect(cross.status).toBe(400);
+    expect(cross.body).toEqual({ error: 'Bag not found' });
+    const dead = await request(server).post(`/api/trips/${tripId}/packing`).set('Cookie', sessionCookie(1)).send({ name: 'Tent', bag_id: 99999 });
+    expect(dead.status).toBe(400);
+    expect(dead.body).toEqual({ error: 'Bag not found' });
+    expect((db.prepare('SELECT COUNT(*) AS n FROM packing_items WHERE trip_id = ?').get(tripId) as { n: number }).n).toBe(0);
+
+    // camelCase was never part of the contract: the keys strip as they always did.
+    const camel = await request(server).post(`/api/trips/${tripId}/packing`).set('Cookie', sessionCookie(1)).send({ name: 'Probe', weightGrams: 250, bagId: foreignBag });
+    expect(camel.status).toBe(201);
+    expect(camel.body.item).toMatchObject({ weight_grams: null, bag_id: null, quantity: 1 });
+  });
+
+  it('400 "Bag not found" on update for a bag off the trip, leaving the row alone (#2154)', async () => {
+    const otherTripId = Number(db.prepare('INSERT INTO trips (user_id, title) VALUES (1, ?)').run('Other').lastInsertRowid);
+    const foreignBag = Number(db.prepare('INSERT INTO packing_bags (trip_id, name) VALUES (?, ?)').run(otherTripId, 'Foreign').lastInsertRowid);
+    const id = insertItem(tripId, 'Tent');
+    const res = await request(server).put(`/api/trips/${tripId}/packing/${id}`).set('Cookie', sessionCookie(1)).send({ bag_id: foreignBag });
+    expect(res.status).toBe(400);
+    expect(res.body).toEqual({ error: 'Bag not found' });
+    expect((db.prepare('SELECT bag_id FROM packing_items WHERE id = ?').get(id) as { bag_id: number | null }).bag_id).toBeNull();
+  });
+
   it('403 on create without permission, writing nothing', async () => {
     checkPermission.mockReturnValue(false);
     const res = await request(server).post(`/api/trips/${tripId}/packing`).set('Cookie', sessionCookie(1)).send({ name: 'X' });
@@ -344,6 +385,13 @@ describe('Packing e2e (real auth guard + real SQL over temp SQLite)', () => {
     expect(missing.body).toEqual({ error: 'Bag not found' });
   });
 
+  it('201 on bag create persists weight_limit_grams (#2154)', async () => {
+    const res = await request(server).post(`/api/trips/${tripId}/packing/bags`).set('Cookie', sessionCookie(1)).send({ name: 'Backpack', weight_limit_grams: 8000 });
+    expect(res.status).toBe(201);
+    expect(res.body.bag).toMatchObject({ name: 'Backpack', weight_limit_grams: 8000 });
+    expect((db.prepare('SELECT weight_limit_grams FROM packing_bags WHERE id = ?').get(res.body.bag.id) as { weight_limit_grams: number | null }).weight_limit_grams).toBe(8000);
+  });
+
   it('bag members: sets roster members only, dropping off-trip user ids', async () => {
     db.prepare('INSERT INTO trip_members (trip_id, user_id) VALUES (?, ?)').run(tripId, 2);
     const created = await request(server).post(`/api/trips/${tripId}/packing/bags`).set('Cookie', sessionCookie(1)).send({ name: 'Main' });

server/tests/unit/mcp/tools-packing-advanced.test.ts

@@ -155,6 +155,21 @@ describe('Tool: create_packing_bag', () => {
     });
   });
 
+  it('creates a bag with a weight limit (#2154)', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    await withHarness(user.id, async (h) => {
+      const result = await h.client.callTool({
+        name: 'create_packing_bag',
+        arguments: { tripId: trip.id, name: 'Checked bag', weight_limit_grams: 23000 },
+      });
+      const data = parseToolResult(result) as any;
+      expect(data.bag.weight_limit_grams).toBe(23000);
+      const row = testDb.prepare('SELECT weight_limit_grams FROM packing_bags WHERE id = ?').get(data.bag.id) as { weight_limit_grams: number | null };
+      expect(row.weight_limit_grams).toBe(23000);
+    });
+  });
+
   it('returns access denied for non-member', async () => {
     const { user } = createUser(testDb);
     const { user: other } = createUser(testDb);

server/tests/unit/mcp/tools-packing.test.ts

@@ -240,6 +240,47 @@ describe('Tool: create_packing_item sharing', () => {
   });
 });
 
+// ---------------------------------------------------------------------------
+// create_packing_item: bag, quantity, weight in one write (#2154)
+//
+// The tool used to inherit the create schema's gap: an assistant could not
+// give a new item a weight or put it in a bag without a second write.
+// ---------------------------------------------------------------------------
+
+describe('Tool: create_packing_item bag, quantity, weight (#2154)', () => {
+  it('creates the item with its bag, quantity and weight in one write', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const bagId = Number(testDb.prepare('INSERT INTO packing_bags (trip_id, name) VALUES (?, ?)').run(trip.id, 'Carry-On').lastInsertRowid);
+    await withHarness(user.id, async (h) => {
+      const result = await h.client.callTool({
+        name: 'create_packing_item',
+        arguments: { tripId: trip.id, name: 'Tent', bag_id: bagId, quantity: 3, weight_grams: 250 },
+      });
+      const row = itemRow(createdItemId(result));
+      expect(row.bag_id).toBe(bagId);
+      expect(row.quantity).toBe(3);
+      expect(row.weight_grams).toBe(250);
+    });
+  });
+
+  it('refuses a bag from another trip, inserting nothing', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const otherTrip = createTrip(testDb, user.id);
+    const foreignBag = Number(testDb.prepare('INSERT INTO packing_bags (trip_id, name) VALUES (?, ?)').run(otherTrip.id, 'Foreign').lastInsertRowid);
+    await withHarness(user.id, async (h) => {
+      const result = await h.client.callTool({
+        name: 'create_packing_item',
+        arguments: { tripId: trip.id, name: 'Tent', bag_id: foreignBag },
+      });
+      expect(result.isError).toBe(true);
+      expect(errorText(result)).toContain('Bag not found');
+    });
+    expect((testDb.prepare('SELECT COUNT(*) AS n FROM packing_items WHERE trip_id = ?').get(trip.id) as { n: number }).n).toBe(0);
+  });
+});
+
 // ---------------------------------------------------------------------------
 // update_packing_item
 // ---------------------------------------------------------------------------
@@ -430,6 +471,23 @@ describe('Tool: update_packing_item bag, quantity, weight, privacy', () => {
       expect(itemRow(item.id).bag_id).toBeNull();
     });
   });
+
+  it('refuses a bag from another trip (#2154)', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const otherTrip = createTrip(testDb, user.id);
+    const item = createPackingItem(testDb, trip.id);
+    const foreignBag = makeBag(otherTrip.id, 'Foreign');
+    await withHarness(user.id, async (h) => {
+      const result = await h.client.callTool({
+        name: 'update_packing_item',
+        arguments: { tripId: trip.id, itemId: item.id, bag_id: foreignBag },
+      });
+      expect(result.isError).toBe(true);
+      expect(errorText(result)).toContain('Bag not found');
+      expect(itemRow(item.id).bag_id).toBeNull();
+    });
+  });
 });
 
 // ---------------------------------------------------------------------------

server/tests/unit/nest/packing.controller.test.ts

@@ -95,6 +95,26 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r
       new PackingController(svc).create(user, '5', { name: 'Power bank', visibility: 'shared', recipient_ids: [2] }, 'sock');
       expect(broadcastToViewers).toHaveBeenCalledWith('5', 'packing:created', { item }, [1, 2], 'sock');
     });
+
+    it('forwards weight_grams, bag_id and quantity to the service (#2154)', () => {
+      // The create route used to hand only the six legacy fields through, so
+      // the values a caller sent vanished into a 201.
+      const createItem = vi.fn().mockReturnValue({ id: 9, name: 'Tent', is_private: 0 });
+      const svc = makeService({ createItem, broadcast: vi.fn() } as Partial<PackingService>);
+      new PackingController(svc).create(user, '5', { name: 'Tent', weight_grams: 250, bag_id: 3, quantity: 3 }, 'sock');
+      expect(createItem).toHaveBeenCalledWith('5', expect.objectContaining({ weight_grams: 250, bag_id: 3, quantity: 3 }), user.id);
+    });
+
+    it('400 "Bag not found" for a bag off the trip (#2154), broadcasting nothing', () => {
+      // Body validation error, so 400 — the 404 'Bag not found' stays with the
+      // /bags/:bagId path routes.
+      const broadcast = vi.fn();
+      const svc = makeService({ createItem: vi.fn().mockReturnValue({ invalidBag: true }), broadcast } as Partial<PackingService>);
+      expect(thrown(() => new PackingController(svc).create(user, '5', { name: 'Tent', bag_id: 999 }))).toEqual({
+        status: 400, body: { error: 'Bag not found' },
+      });
+      expect(broadcast).not.toHaveBeenCalled();
+    });
   });
 
   it('GET / lists items for the trip, scoped to the viewer (#858)', () => {
@@ -184,6 +204,15 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r
       expect(broadcast).toHaveBeenCalledWith('5', 'packing:updated', { item: updated }, 'sock');
     });
 
+    it('400 "Bag not found" for a bag off the trip (#2154), broadcasting nothing', () => {
+      const broadcast = vi.fn();
+      const svc = makeService({ updateItem: vi.fn().mockReturnValue({ invalidBag: true }), broadcast } as Partial<PackingService>);
+      expect(thrown(() => new PackingController(svc).update(user, '5', '9', { bag_id: 999 }))).toEqual({
+        status: 400, body: { error: 'Bag not found' },
+      });
+      expect(broadcast).not.toHaveBeenCalled();
+    });
+
     it('forwards the X-Base-Updated-At token and 409s on a conflict (#1135)', () => {
       const updateItem = vi.fn().mockReturnValue({ conflict: true, server: { id: 9, name: 'Theirs' } });
       const broadcast = vi.fn();
@@ -314,6 +343,13 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r
       expect(broadcast).toHaveBeenCalledWith('5', 'packing:bag-created', { bag: { id: 3, name: 'Carry-on' } }, 'sock');
     });
 
+    it('forwards weight_limit_grams on bag create (#2154)', () => {
+      const createBag = vi.fn().mockReturnValue({ id: 3, name: 'Backpack', weight_limit_grams: 8000 });
+      const svc = makeService({ createBag, broadcast: vi.fn() } as Partial<PackingService>);
+      new PackingController(svc).createBag(user, '5', { name: 'Backpack', weight_limit_grams: 8000 });
+      expect(createBag).toHaveBeenCalledWith('5', { name: 'Backpack', color: undefined, weight_limit_grams: 8000 });
+    });
+
     it('404 on bag update when missing', () => {
       const svc = makeService({ updateBag: vi.fn().mockReturnValue(null) } as Partial<PackingService>);
       expect(thrown(() => new PackingController(svc).updateBag(user, '5', '3', { name: 'X' }))).toEqual({

server/tests/unit/nest/packing.service.test.ts

@@ -62,7 +62,7 @@ import { resetTestDb } from '../../helpers/test-db';
 import { createUser, createAdmin, createTrip, addTripMember } from '../../helpers/factories';
 import { DatabaseService } from '../../../src/nest/database/database.service';
 import type { PermissionsService } from '../../../src/nest/permissions/permissions.service';
-import { PackingService } from '../../../src/nest/packing/packing.service';
+import { PackingService, isInvalidBagRef } from '../../../src/nest/packing/packing.service';
 // Was packing.bridge, deleted with the other three that had no consumer outside the
 // container. The assertions stayed; they point at the service now.
 const bridgeListItems = (tripId: string | number, viewerId?: number) => svc.listItems(tripId, viewerId);
@@ -685,6 +685,60 @@ describe('legacy-quirk fixes', () => {
   });
 });
 
+// ── Create-path fields + bag trip scope (#2154) ───────────────────────────────
+
+describe('create-path fields + bag trip scope (#2154)', () => {
+  it('PACK-SVC-054: createItem persists weight_grams, bag_id and quantity in one write', () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const bag = svc.createBag(trip.id, { name: 'Carry-On' }) as any;
+
+    const item = svc.createItem(trip.id, { name: 'Tent', weight_grams: 250, bag_id: bag.id, quantity: 3 }, user.id) as any;
+    expect(item).toMatchObject({ weight_grams: 250, bag_id: bag.id, quantity: 3 });
+  });
+
+  it('PACK-SVC-055: createItem refuses a bag off the trip with the invalidBag sentinel, inserting nothing', () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const otherTrip = createTrip(testDb, user.id);
+    const foreignBag = svc.createBag(otherTrip.id, { name: 'Not yours' }) as any;
+
+    // Existence alone is not enough — the bag must belong to THIS trip.
+    expect(svc.createItem(trip.id, { name: 'Tent', bag_id: foreignBag.id }, user.id)).toEqual({ invalidBag: true });
+    // A dead id refuses the same way (it used to be an SQLite FK error).
+    expect(isInvalidBagRef(svc.createItem(trip.id, { name: 'Tent', bag_id: 99999 }, user.id))).toBe(true);
+    expect((testDb.prepare('SELECT COUNT(*) AS n FROM packing_items WHERE trip_id = ?').get(trip.id) as any).n).toBe(0);
+  });
+
+  it('PACK-SVC-056: updateItem refuses a cross-trip bag_id and leaves the row alone; null still clears', () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const otherTrip = createTrip(testDb, user.id);
+    const own = svc.createBag(trip.id, { name: 'Mine' }) as any;
+    const foreign = svc.createBag(otherTrip.id, { name: 'Not mine' }) as any;
+    const item = svc.createItem(trip.id, { name: 'Tent', bag_id: own.id }, user.id) as any;
+
+    const refused = svc.updateItem(trip.id, item.id, { bag_id: foreign.id }, ['bag_id'], undefined, user.id);
+    expect(isInvalidBagRef(refused)).toBe(true);
+    expect((testDb.prepare('SELECT bag_id FROM packing_items WHERE id = ?').get(item.id) as any).bag_id).toBe(own.id);
+
+    // Clearing the bag with an explicit null is untouched by the check.
+    const cleared = svc.updateItem(trip.id, item.id, { bag_id: null }, ['bag_id'], undefined, user.id) as any;
+    expect(cleared.bag_id).toBeNull();
+  });
+
+  it('PACK-SVC-057: createBag persists weight_limit_grams; omitted stays null', () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+
+    const limited = svc.createBag(trip.id, { name: 'Checked', weight_limit_grams: 23000 }) as any;
+    expect(limited.weight_limit_grams).toBe(23000);
+
+    const bare = svc.createBag(trip.id, { name: 'Day pack' }) as any;
+    expect(bare.weight_limit_grams).toBeNull();
+  });
+});
+
 // ── Wrapper helpers (carried over from the old delegation suite) ──────────────
 
 describe('canEdit', () => {

server/tests/unit/packing/packing.rpc.test.ts

@@ -157,6 +157,39 @@ describe('PackingRpc through the router', () => {
     expect(res.error.message).toBe('no packing item 404 on trip 1');
   });
 
+  it('PACKING-RPC-018 create hands the widened fields through the schema (#2154)', async () => {
+    const f = build();
+    await f.host('db:write:packing').dispatch(req('packing.create', { tripId: 1, input: { name: 'Tent', weight_grams: 250, bag_id: 19, quantity: 3 } }), 42);
+    expect(f.data.createItem).toHaveBeenCalledWith('1', { name: 'Tent', weight_grams: 250, bag_id: 19, quantity: 3 }, 42);
+  });
+
+  it('PACKING-RPC-019 an off-trip bag is BAD_PARAMS on create and update alike, broadcasting nothing (#2154)', async () => {
+    const f = build();
+    const host = f.host('db:write:packing');
+
+    f.data.createItem.mockReturnValueOnce({ invalidBag: true } as never);
+    const created = (await host.dispatch(req('packing.create', { tripId: 1, input: { name: 'Tent', bag_id: 404 } }), 42)) as RpcError;
+    expect(created.error.code).toBe('BAD_PARAMS');
+    expect(created.error.message).toBe('no packing bag 404 on trip 1');
+
+    f.data.updateItem.mockReturnValueOnce({ invalidBag: true } as never);
+    const updated = (await host.dispatch(req('packing.update', { tripId: 1, itemId: 70, input: { bag_id: 404 } }), 42)) as RpcError;
+    expect(updated.error.code).toBe('BAD_PARAMS');
+    expect(updated.error.message).toBe('no packing bag 404 on trip 1');
+
+    // The sentinel must never leave the process as an item.
+    expect(f.realtime.broadcast).not.toHaveBeenCalled();
+  });
+
+  it('PACKING-RPC-020 createBag forwards a numeric weight limit and drops a non-number (#2154)', async () => {
+    const f = build();
+    const host = f.host('db:write:packing');
+    await host.dispatch(req('packing.createBag', { tripId: 1, input: { name: 'Bag', weight_limit_grams: 23000 } }), 42);
+    expect(f.data.createBag).toHaveBeenCalledWith('1', { name: 'Bag', color: undefined, weight_limit_grams: 23000 });
+    await host.dispatch(req('packing.createBag', { tripId: 1, input: { name: 'Bag', weight_limit_grams: 'heavy' } }), 42);
+    expect(f.data.createBag).toHaveBeenLastCalledWith('1', { name: 'Bag', color: undefined, weight_limit_grams: undefined });
+  });
+
   it('PACKING-RPC-008 the class is listed in its module providers', () => {
     expectRegisteredProvider(PackingModule, PackingRpc);
   });