From b84867b4c221fff3bca79cd6d533f6bb48b033d0 Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 8 Oct 2026 15:56:18 +0200 Subject: [PATCH] fix(mcp): update bookings through updateWithCost like REST and the plugin RPC update_reservation and update_transport accept a type but still called the bare ReservationsService.update. Only updateWithCost re-files a linked expense that still carries the category derived from the old type, so changing a booking from flight to train over MCP left its expense filed under flights while the same change over REST or RPC moved it. Both tools now call updateWithCost without a price entry, and send the cost events and the accommodation change after the commit, as REST does. --- .../src/nest/reservations/reservations.mcp.ts | 15 ++++++--- .../tests/unit/mcp/tools-reservations.test.ts | 20 ++++++++++++ .../tests/unit/mcp/tools-transports.test.ts | 32 +++++++++++++++++++ 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/server/src/nest/reservations/reservations.mcp.ts b/server/src/nest/reservations/reservations.mcp.ts index ed8968da6..e71120a87 100644 --- a/server/src/nest/reservations/reservations.mcp.ts +++ b/server/src/nest/reservations/reservations.mcp.ts @@ -447,11 +447,15 @@ export class ReservationsMcp { if (assignment_id != null && !(await this.assignments.getAssignmentForTrip(assignment_id, tripId))) return errorResult('assignment_id does not belong to this trip.'); - const { reservation } = await this.reservations.update(reservationId, tripId, { + // The booking and the linked expenses a type change re-files are one write, + // the same service path REST and the plugin RPC take. + const { reservation, accommodationChanged, costEvents } = await this.reservations.updateWithCost(String(reservationId), String(tripId), { title, type, reservation_time, reservation_end_time, url, location, confirmation_number, notes, status, place_id: place_id !== undefined ? place_id ?? undefined : undefined, assignment_id: assignment_id !== undefined ? assignment_id ?? undefined : undefined, - }, existing); + }, existing, undefined); + if (accommodationChanged) this.guards.safeBroadcast(tripId, 'accommodation:updated', {}); + for (const { event, payload } of costEvents) this.guards.safeBroadcast(tripId, event, payload); this.guards.safeBroadcast(tripId, 'reservation:updated', { reservation }); return ok({ reservation }); } @@ -860,7 +864,8 @@ export class ReservationsMcp { if (applied.endpointsChanged) resolvedEndpoints = applied.endpoints; } - const { reservation } = await this.reservations.update(reservationId, tripId, { + // One write with the linked expenses a type change re-files, as on update_reservation. + const { reservation, accommodationChanged, costEvents } = await this.reservations.updateWithCost(String(reservationId), String(tripId), { title, type, reservation_time: departureTime, @@ -874,7 +879,9 @@ export class ReservationsMcp { metadata: nextMetadata, endpoints: resolvedEndpoints, needs_review, - }, existing); + }, existing, undefined); + if (accommodationChanged) this.guards.safeBroadcast(tripId, 'accommodation:updated', {}); + for (const { event, payload } of costEvents) this.guards.safeBroadcast(tripId, event, payload); this.guards.safeBroadcast(tripId, 'reservation:updated', { reservation }); return ok({ reservation }); } diff --git a/server/tests/unit/mcp/tools-reservations.test.ts b/server/tests/unit/mcp/tools-reservations.test.ts index 25614aaa0..b8bf45cf1 100644 --- a/server/tests/unit/mcp/tools-reservations.test.ts +++ b/server/tests/unit/mcp/tools-reservations.test.ts @@ -212,6 +212,26 @@ describe('Tool: update_reservation', () => { }); }); + it('re-files a linked expense on a type change in the same write, as REST and the plugin RPC do', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const reservation = createReservation(testDb, trip.id, { title: 'Dinner', type: 'restaurant' }); + const auto = createBudgetItem(testDb, trip.id, { name: 'Dinner', category: 'food' }); + const picked = createBudgetItem(testDb, trip.id, { name: 'Taxi', category: 'transport' }); + testDb.prepare('UPDATE budget_items SET reservation_id = ? WHERE id IN (?, ?)').run(reservation.id, auto.id, picked.id); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ name: 'update_reservation', arguments: { tripId: trip.id, reservationId: reservation.id, type: 'activity' } }); + expect((parseToolResult(result) as { reservation: { type: string } }).reservation.type).toBe('activity'); + }); + // restaurant -> activity moves the auto-derived category; the hand-picked one stays. + expect(testDb.prepare('SELECT id, category FROM budget_items WHERE trip_id = ? ORDER BY id').all(trip.id)) + .toEqual([{ id: auto.id, category: 'activities' }, { id: picked.id, category: 'transport' }]); + const updated = broadcastMock.mock.calls.filter(c => c[1] === 'budget:updated').map(c => (c[2] as { item: { id: number } }).item.id); + expect(updated).toEqual([auto.id]); + const events = broadcastMock.mock.calls.map(c => c[1]); + expect(events.indexOf('budget:updated')).toBeLessThan(events.indexOf('reservation:updated')); + }); + it('returns error for reservation not found', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); diff --git a/server/tests/unit/mcp/tools-transports.test.ts b/server/tests/unit/mcp/tools-transports.test.ts index 2e789a514..6f770d1a7 100644 --- a/server/tests/unit/mcp/tools-transports.test.ts +++ b/server/tests/unit/mcp/tools-transports.test.ts @@ -189,6 +189,38 @@ describe('Tool: update_transport', () => { expect(data.reservation.endpoints).toHaveLength(2); }); }); + + it('re-files a linked expense on a type change in the same write, as REST and the plugin RPC do', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + await withHarness(user.id, async (h) => { + const created = parseToolResult(await h.client.callTool({ + name: 'create_transport', + arguments: { tripId: trip.id, type: 'flight', title: 'F', endpoints: flightEndpoints }, + })) as { reservation: { id: number } }; + const auto = Number(testDb.prepare("INSERT INTO budget_items (trip_id, name, category, total_price, reservation_id) VALUES (?, 'Fare', 'flights', 100, ?)") + .run(trip.id, created.reservation.id).lastInsertRowid); + const picked = Number(testDb.prepare("INSERT INTO budget_items (trip_id, name, category, total_price, reservation_id) VALUES (?, 'Lounge', 'food', 20, ?)") + .run(trip.id, created.reservation.id).lastInsertRowid); + broadcastMock.mockClear(); + + const result = await h.client.callTool({ + name: 'update_transport', + arguments: { tripId: trip.id, reservationId: created.reservation.id, type: 'train' }, + }); + expect((parseToolResult(result) as { reservation: { type: string } }).reservation.type).toBe('train'); + + // flight -> train moves the auto-derived category; the hand-picked one stays. + const category = (id: number) => (testDb.prepare('SELECT category FROM budget_items WHERE id = ?').get(id) as { category: string }).category; + expect(category(auto)).toBe('transport'); + expect(category(picked)).toBe('food'); + const updated = broadcastMock.mock.calls.filter(c => c[1] === 'budget:updated').map(c => (c[2] as { item: { id: number } }).item.id); + expect(updated).toEqual([auto]); + // The expense goes out after the commit, before the booking. + const events = broadcastMock.mock.calls.map(c => c[1]); + expect(events.indexOf('budget:updated')).toBeLessThan(events.indexOf('reservation:updated')); + }); + }); }); // ---------------------------------------------------------------------------