mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
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.
This commit is contained in:
Maurice
@@ -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.');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user