From 7b666736df5f6671c8508f8890ca15374ac2fd11 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 8 Aug 2026 20:03:20 +0200 Subject: [PATCH] test(server): cover the MCP surfaces that moved into the gated tree The coverage gate reads src/nest/** only, so the tools that lived in src/mcp/ were never measured. Moving them exposed what was never covered: - airports.mcp.ts sat at 0% branches - search_airports and get_airport had no test at all, in either world - the two weather catch blocks, which turn a provider failure into an isError result instead of a transport-level error - the transport guards: trip access, day ids belonging to another trip, the not-a-transport-type check on update, and delete_transport in full - the price path, where a transport with a price links a budget item and broadcasts budget:created None of this is new behaviour; it is behaviour that shipped untested because it sat outside the measured tree. One case documents a subtlety worth keeping: the weather fallback message only fires for a rejection with no .message at all. `new Error('')` still carries an empty one and passes straight through, so the case rejects with a non-Error. --- .../unit/mcp/tools-tags-maps-weather.test.ts | 113 +++++++++ .../tests/unit/mcp/tools-transports.test.ts | 228 ++++++++++++++++++ 2 files changed, 341 insertions(+) diff --git a/server/tests/unit/mcp/tools-tags-maps-weather.test.ts b/server/tests/unit/mcp/tools-tags-maps-weather.test.ts index df214c250..3f93a64d1 100644 --- a/server/tests/unit/mcp/tools-tags-maps-weather.test.ts +++ b/server/tests/unit/mcp/tools-tags-maps-weather.test.ts @@ -46,6 +46,7 @@ import { resetTestDb } from '../../helpers/test-db'; import { createUser } from '../../helpers/factories'; import { createMcpHarness, parseToolResult, type McpHarness } from '../../helpers/mcp-harness'; import { MapsService } from '../../../src/nest/maps/maps.service'; +import { getWeather, getDetailedWeather } from '../../../src/nest/weather/weather.impl'; // The geo tools live on the DI-discovered maps.mcp.ts since the maps fold; the // test registry builds a real MapsService over the mocked db proxy, so stub the @@ -394,3 +395,115 @@ describe('Tool: get_detailed_weather', () => { }); }); }); + +// --------------------------------------------------------------------------- +// Weather failure path +// +// Both weather tools swallow a provider failure and answer with isError plus +// the upstream message, rather than letting it surface as a transport-level +// error. That branch never had a case: the tools lived in src/mcp/, which the +// coverage gate does not measure, and the move into src/nest/ exposed it. +// --------------------------------------------------------------------------- + +describe('Weather tools: provider failure', () => { + it('get_weather answers isError with the upstream message', async () => { + const { user } = createUser(testDb); + vi.mocked(getWeather).mockRejectedValueOnce(new Error('Open-Meteo unreachable')); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'get_weather', + arguments: { lat: 48.8566, lng: 2.3522, date: '2025-07-01' }, + }); + expect((result as { isError?: boolean }).isError).toBe(true); + expect((result as { content: { text: string }[] }).content[0].text).toBe('Open-Meteo unreachable'); + }); + }); + + it('get_weather falls back to a generic message when the rejection is not an Error', async () => { + const { user } = createUser(testDb); + // A non-Error rejection has no .message, which is the only way the ?? fallback + // fires — `new Error('')` still carries an (empty) message and passes through. + vi.mocked(getWeather).mockRejectedValueOnce('boom' as unknown as Error); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'get_weather', + arguments: { lat: 48.8566, lng: 2.3522, date: '2025-07-01' }, + }); + expect((result as { isError?: boolean }).isError).toBe(true); + expect((result as { content: { text: string }[] }).content[0].text).toBe('Weather service not available.'); + }); + }); + + it('get_detailed_weather answers isError with the upstream message', async () => { + const { user } = createUser(testDb); + vi.mocked(getDetailedWeather).mockRejectedValueOnce(new Error('rate limited')); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'get_detailed_weather', + arguments: { lat: 48.8566, lng: 2.3522, date: '2025-07-01' }, + }); + expect((result as { isError?: boolean }).isError).toBe(true); + expect((result as { content: { text: string }[] }).content[0].text).toBe('rate limited'); + }); + }); +}); + +// --------------------------------------------------------------------------- +// Airports +// +// The airport lookups had no coverage at all before they moved out of the +// legacy registrar — the same blind spot as the weather catch above. +// --------------------------------------------------------------------------- + +describe('Tool: search_airports', () => { + it('returns matches for a city name, capped by limit', async () => { + const { user } = createUser(testDb); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ name: 'search_airports', arguments: { query: 'zurich', limit: 5 } }); + const data = parseToolResult(result) as { airports: { iata: string }[] }; + expect(Array.isArray(data.airports)).toBe(true); + expect(data.airports.length).toBeGreaterThan(0); + expect(data.airports.length).toBeLessThanOrEqual(5); + expect(data.airports.some(a => a.iata === 'ZRH')).toBe(true); + }); + }); + + it('applies the default limit of 10 when none is given', async () => { + const { user } = createUser(testDb); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ name: 'search_airports', arguments: { query: 'a' } }); + const data = parseToolResult(result) as { airports: unknown[] }; + expect(data.airports.length).toBeLessThanOrEqual(10); + }); + }); + + it('returns an empty list for a query that matches nothing', async () => { + const { user } = createUser(testDb); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ name: 'search_airports', arguments: { query: 'zzzzzznotanairport' } }); + const data = parseToolResult(result) as { airports: unknown[] }; + expect(data.airports).toEqual([]); + }); + }); +}); + +describe('Tool: get_airport', () => { + it('returns the airport for a known IATA code, lowercase input included', async () => { + const { user } = createUser(testDb); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ name: 'get_airport', arguments: { iata: 'zrh' } }); + const data = parseToolResult(result) as { airport: { iata: string; tz: string } }; + expect(data.airport.iata).toBe('ZRH'); + expect(typeof data.airport.tz).toBe('string'); + }); + }); + + it('answers isError for an unknown code rather than a null airport', async () => { + const { user } = createUser(testDb); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ name: 'get_airport', arguments: { iata: 'QQQ' } }); + expect((result as { isError?: boolean }).isError).toBe(true); + expect((result as { content: { text: string }[] }).content[0].text).toBe('Airport not found.'); + }); + }); +}); diff --git a/server/tests/unit/mcp/tools-transports.test.ts b/server/tests/unit/mcp/tools-transports.test.ts index 08632144a..87c998d64 100644 --- a/server/tests/unit/mcp/tools-transports.test.ts +++ b/server/tests/unit/mcp/tools-transports.test.ts @@ -199,3 +199,231 @@ describe('Tool: update_transport', () => { }); }); }); + +// --------------------------------------------------------------------------- +// Guards, the budget link and delete +// +// These paths were never covered: the tools lived in src/mcp/, which the +// coverage gate does not measure. Moving them into reservations.mcp.ts put them +// inside src/nest/** and made the gap visible. +// --------------------------------------------------------------------------- + +describe('Transport tools: access and validation', () => { + it('create_transport refuses a trip the caller cannot see', async () => { + const { user } = createUser(testDb); + const { user: stranger } = createUser(testDb, { username: 'stranger' }); + const trip = createTrip(testDb, stranger.id); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'create_transport', + arguments: { tripId: trip.id, type: 'flight', title: 'ZRH → CDG' }, + }); + expect((result as { isError?: boolean }).isError).toBe(true); + expect((result as { content: { text: string }[] }).content[0].text).toContain('Trip not found'); + }); + }); + + it('create_transport rejects a start_day_id from another trip', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const otherTrip = createTrip(testDb, user.id); + const foreignDay = Number(testDb.prepare("INSERT INTO days (trip_id, date, day_number) VALUES (?, '2026-07-01', 1)").run(otherTrip.id).lastInsertRowid); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'create_transport', + arguments: { tripId: trip.id, type: 'train', title: 'ICE', start_day_id: foreignDay }, + }); + expect((result as { isError?: boolean }).isError).toBe(true); + expect((result as { content: { text: string }[] }).content[0].text).toBe('start_day_id does not belong to this trip.'); + }); + }); + + it('create_transport rejects an end_day_id from another trip', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const otherTrip = createTrip(testDb, user.id); + const foreignDay = Number(testDb.prepare("INSERT INTO days (trip_id, date, day_number) VALUES (?, '2026-07-02', 1)").run(otherTrip.id).lastInsertRowid); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'create_transport', + arguments: { tripId: trip.id, type: 'train', title: 'ICE', end_day_id: foreignDay }, + }); + expect((result as { isError?: boolean }).isError).toBe(true); + expect((result as { content: { text: string }[] }).content[0].text).toBe('end_day_id does not belong to this trip.'); + }); + }); +}); + +describe('Transport tools: the price link', () => { + it('create_transport with a price creates the budget item and broadcasts it', 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_transport', + arguments: { tripId: trip.id, type: 'flight', title: 'ZRH → CDG', price: 240, budget_category: 'Flights' }, + }); + const data = parseToolResult(result) as { reservation: { id: number } }; + + const item = testDb.prepare('SELECT name, category, total_price FROM budget_items WHERE reservation_id = ?').get(data.reservation.id) as { name: string; category: string; total_price: number }; + expect(item).toMatchObject({ name: 'ZRH → CDG', category: 'Flights', total_price: 240 }); + // The price also rides along in the reservation metadata, as the REST path does. + expect(broadcastMock.mock.calls.some(c => c[1] === 'budget:created')).toBe(true); + }); + }); + + it('create_transport falls back to the transport type as the budget category', 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_transport', + arguments: { tripId: trip.id, type: 'train', title: 'ICE 599', price: 89 }, + }); + const data = parseToolResult(result) as { reservation: { id: number } }; + const item = testDb.prepare('SELECT category FROM budget_items WHERE reservation_id = ?').get(data.reservation.id) as { category: string }; + expect(item.category).toBe('train'); + }); + }); + + it('create_transport with price 0 records no budget item', 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_transport', + arguments: { tripId: trip.id, type: 'car', title: 'Rental', price: 0 }, + }); + const data = parseToolResult(result) as { reservation: { id: number } }; + expect(testDb.prepare('SELECT id FROM budget_items WHERE reservation_id = ?').get(data.reservation.id)).toBeUndefined(); + }); + }); +}); + +describe('Tool: update_transport (guards)', () => { + it('404s an unknown reservation id', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'update_transport', + arguments: { tripId: trip.id, reservationId: 999999, title: 'nope' }, + }); + expect((result as { isError?: boolean }).isError).toBe(true); + expect((result as { content: { text: string }[] }).content[0].text).toBe('Transport not found.'); + }); + }); + + it('refuses a reservation that is not a transport type', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + await withHarness(user.id, async (h) => { + const created = await h.client.callTool({ + name: 'create_reservation', + arguments: { tripId: trip.id, type: 'restaurant', title: 'Dinner' }, + }); + const { reservation } = parseToolResult(created) as { reservation: { id: number } }; + + const result = await h.client.callTool({ + name: 'update_transport', + arguments: { tripId: trip.id, reservationId: reservation.id, title: 'still dinner' }, + }); + expect((result as { isError?: boolean }).isError).toBe(true); + expect((result as { content: { text: string }[] }).content[0].text).toBe('Reservation is not a transport type. Use update_reservation instead.'); + }); + }); + + it('rejects a start_day_id from another trip', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const otherTrip = createTrip(testDb, user.id); + const foreignDay = Number(testDb.prepare("INSERT INTO days (trip_id, date, day_number) VALUES (?, '2026-08-01', 1)").run(otherTrip.id).lastInsertRowid); + await withHarness(user.id, async (h) => { + const created = await h.client.callTool({ + name: 'create_transport', + arguments: { tripId: trip.id, type: 'flight', title: 'ZRH → CDG', endpoints: flightEndpoints }, + }); + const { reservation } = parseToolResult(created) as { reservation: { id: number } }; + + const result = await h.client.callTool({ + name: 'update_transport', + arguments: { tripId: trip.id, reservationId: reservation.id, start_day_id: foreignDay }, + }); + expect((result as { isError?: boolean }).isError).toBe(true); + expect((result as { content: { text: string }[] }).content[0].text).toBe('start_day_id does not belong to this trip.'); + }); + }); + + it('errors on an unresolvable airport code when replacing endpoints', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + await withHarness(user.id, async (h) => { + const created = await h.client.callTool({ + name: 'create_transport', + arguments: { tripId: trip.id, type: 'flight', title: 'ZRH → CDG', endpoints: flightEndpoints }, + }); + const { reservation } = parseToolResult(created) as { reservation: { id: number } }; + + const result = await h.client.callTool({ + name: 'update_transport', + arguments: { + tripId: trip.id, + reservationId: reservation.id, + endpoints: [{ role: 'from', sequence: 0, name: 'Nowhere', code: 'QQQ' }], + }, + }); + expect((result as { isError?: boolean }).isError).toBe(true); + expect((result as { content: { text: string }[] }).content[0].text).toContain('Could not resolve airport code'); + }); + }); +}); + +describe('Tool: delete_transport', () => { + it('deletes the booking and broadcasts it', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + await withHarness(user.id, async (h) => { + const created = await h.client.callTool({ + name: 'create_transport', + arguments: { tripId: trip.id, type: 'flight', title: 'ZRH → CDG', endpoints: flightEndpoints }, + }); + const { reservation } = parseToolResult(created) as { reservation: { id: number } }; + broadcastMock.mockClear(); + + const result = await h.client.callTool({ + name: 'delete_transport', + arguments: { tripId: trip.id, reservationId: reservation.id }, + }); + expect(parseToolResult(result)).toEqual({ success: true }); + expect(testDb.prepare('SELECT id FROM reservations WHERE id = ?').get(reservation.id)).toBeUndefined(); + expect(broadcastMock.mock.calls.some(c => c[1] === 'reservation:deleted')).toBe(true); + }); + }); + + it('404s an unknown reservation id', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'delete_transport', + arguments: { tripId: trip.id, reservationId: 999999 }, + }); + expect((result as { isError?: boolean }).isError).toBe(true); + expect((result as { content: { text: string }[] }).content[0].text).toBe('Transport not found.'); + }); + }); + + it('refuses a trip the caller cannot see', async () => { + const { user } = createUser(testDb); + const { user: stranger } = createUser(testDb, { username: 'stranger2' }); + const trip = createTrip(testDb, stranger.id); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'delete_transport', + arguments: { tripId: trip.id, reservationId: 1 }, + }); + expect((result as { isError?: boolean }).isError).toBe(true); + expect((result as { content: { text: string }[] }).content[0].text).toContain('Trip not found'); + }); + }); +});