mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(mcp): accept every transport type the planner offers
create_transport and update_transport took flight, train, car and cruise. The picker in TransportModal has ten modes and ReservationsPanel renders all of them, so a bus, a ferry or a taxi could be planned in the UI and not through an assistant - which is where importing a travel history from another service runs aground. Both lists now come from one constant in the picker's order rather than four literals repeated across two schemas, which is how they drifted. legs[] stays on flight and train: the form writes metadata.legs for those two and for nothing else.
This commit is contained in:
Maurice
@@ -15,9 +15,18 @@ import type { EndpointInput } from './reservations.service';
|
||||
import { AssignmentsService } from '../assignments/assignments.service';
|
||||
import { transportLegsInputSchema, type TransportLegInput } from '@trek/shared';
|
||||
|
||||
const TRANSPORT_TYPES = ['flight', 'train', 'car', 'cruise'] as const;
|
||||
/** Only these two carry per-segment detail; a car or cruise has no legs. */
|
||||
// The type picker in the planner (client/src/components/Planner/TransportModal.tsx)
|
||||
// offers ten transport modes and ReservationsPanel renders all of them; the tools
|
||||
// below used to accept four, so a bus or a ferry could be planned in the UI and not
|
||||
// through an assistant. Both lists are the picker's, in the picker's order.
|
||||
const TRANSPORT_TYPES = ['flight', 'train', 'bus', 'car', 'taxi', 'bicycle', 'cruise', 'ferry', 'transit', 'transport_other'] as const;
|
||||
/** Only these two carry per-segment detail: the transport form writes metadata.legs for a flight or a train and for nothing else. */
|
||||
const LEG_TRANSPORT_TYPES = ['flight', 'train'] as const;
|
||||
/** Everything the picker offers that is not a transport — create_reservation's half. */
|
||||
const BOOKING_TYPES = ['hotel', 'restaurant', 'event', 'tour', 'activity', 'parking', 'other'] as const;
|
||||
|
||||
type TransportType = typeof TRANSPORT_TYPES[number];
|
||||
type BookingType = typeof BOOKING_TYPES[number];
|
||||
|
||||
const endpointObjectSchema = z.object({
|
||||
role: z.enum(['from', 'to', 'stop']).describe('Endpoint role: "from" (origin), "to" (destination), or "stop" (intermediate)'),
|
||||
@@ -300,11 +309,11 @@ export class ReservationsMcp {
|
||||
|
||||
@Tool({
|
||||
name: 'create_reservation',
|
||||
description: 'Recommend a reservation for a trip. Created as pending — the user must confirm it. For flights, trains, cars, and cruises, use create_transport instead. Linking: hotel → use place_id + start_day_id + end_day_id (all three required to create the accommodation link); restaurant/event/tour/activity/parking/other → use assignment_id. Set price to record the cost; it will appear on the booking and in the Budget tab.',
|
||||
description: 'Recommend a reservation for a trip. Created as pending — the user must confirm it. For anything travelled ON — flight, train, bus, car, taxi, bicycle, cruise, ferry, transit, transport_other — use create_transport instead. Linking: hotel → use place_id + start_day_id + end_day_id (all three required to create the accommodation link); restaurant/event/tour/activity/parking/other → use assignment_id. Set price to record the cost; it will appear on the booking and in the Budget tab.',
|
||||
inputSchema: {
|
||||
tripId: z.number().int().positive(),
|
||||
title: z.string().min(1).max(200),
|
||||
type: z.enum(['hotel', 'restaurant', 'event', 'tour', 'activity', 'parking', 'other']).describe('Reservation type: "hotel", "restaurant", "event", "tour", "activity", "parking", or "other"'),
|
||||
type: z.enum(BOOKING_TYPES).describe('Reservation type: "hotel", "restaurant", "event", "tour", "activity", "parking", or "other"'),
|
||||
reservation_time: z.string().optional().describe('ISO 8601 datetime or time string'),
|
||||
location: z.string().max(500).optional(),
|
||||
confirmation_number: z.string().max(100).optional(),
|
||||
@@ -324,7 +333,7 @@ export class ReservationsMcp {
|
||||
})
|
||||
async createReservation(
|
||||
{ tripId, title, type, reservation_time, location, confirmation_number, notes, day_id, place_id, start_day_id, end_day_id, check_in, check_out, assignment_id, price, budget_category }: {
|
||||
tripId: number; title: string; type: 'hotel' | 'restaurant' | 'event' | 'tour' | 'activity' | 'parking' | 'other';
|
||||
tripId: number; title: string; type: BookingType;
|
||||
reservation_time?: string; location?: string; confirmation_number?: string; notes?: string;
|
||||
day_id?: number; place_id?: number; start_day_id?: number; end_day_id?: number;
|
||||
check_in?: string; check_out?: string; assignment_id?: number; price?: number; budget_category?: string;
|
||||
@@ -379,12 +388,12 @@ export class ReservationsMcp {
|
||||
|
||||
@Tool({
|
||||
name: 'update_reservation',
|
||||
description: 'Update an existing reservation in a trip. Use status "confirmed" to confirm a pending recommendation, or "pending" to revert it. For flights, trains, cars, and cruises, use update_transport instead. Linking: hotel → use place_id to link to an accommodation place; restaurant/event/tour/activity/parking/other → use assignment_id to link to a day assignment.',
|
||||
description: 'Update an existing reservation in a trip. Use status "confirmed" to confirm a pending recommendation, or "pending" to revert it. For anything travelled ON — flight, train, bus, car, taxi, bicycle, cruise, ferry, transit, transport_other — use update_transport instead. Linking: hotel → use place_id to link to an accommodation place; restaurant/event/tour/activity/parking/other → use assignment_id to link to a day assignment.',
|
||||
inputSchema: {
|
||||
tripId: z.number().int().positive(),
|
||||
reservationId: z.number().int().positive(),
|
||||
title: z.string().min(1).max(200).optional(),
|
||||
type: z.enum(['hotel', 'restaurant', 'event', 'tour', 'activity', 'parking', 'other']).optional().describe('Reservation type: "hotel", "restaurant", "event", "tour", "activity", "parking", or "other"'),
|
||||
type: z.enum(BOOKING_TYPES).optional().describe('Reservation type: "hotel", "restaurant", "event", "tour", "activity", "parking", or "other"'),
|
||||
reservation_time: z.string().optional().describe('ISO 8601 datetime or time string'),
|
||||
location: z.string().max(500).optional(),
|
||||
confirmation_number: z.string().max(100).optional(),
|
||||
@@ -399,7 +408,7 @@ export class ReservationsMcp {
|
||||
async updateReservation(
|
||||
{ tripId, reservationId, title, type, reservation_time, location, confirmation_number, notes, status, place_id, assignment_id }: {
|
||||
tripId: number; reservationId: number; title?: string;
|
||||
type?: 'hotel' | 'restaurant' | 'event' | 'tour' | 'activity' | 'parking' | 'other';
|
||||
type?: BookingType;
|
||||
reservation_time?: string; location?: string; confirmation_number?: string; notes?: string;
|
||||
status?: 'pending' | 'confirmed' | 'cancelled'; place_id?: number | null; assignment_id?: number | null;
|
||||
},
|
||||
@@ -571,10 +580,10 @@ export class ReservationsMcp {
|
||||
|
||||
@Tool({
|
||||
name: 'create_transport',
|
||||
description: 'Create a transport booking (flight, train, car, or cruise) for a trip. Use endpoints[] to record origin/destination and intermediate stops — for flights, set code to the IATA airport code (use search_airports first). For a booking WITH STOPOVERS also pass legs[] (one entry per segment, one fewer than endpoints[]), otherwise every segment inherits the stop time as both its arrival and its departure. The top-level confirmation_number is the booking reference; when a single segment was booked under its own reference, put that one on the leg instead. Created as pending — confirm with update_transport. Set price to record the cost; it will appear on the booking and in the Budget tab.',
|
||||
description: 'Create a transport booking for a trip — flight, train, bus, car, taxi, bicycle, cruise, ferry, transit or transport_other. Use endpoints[] to record origin/destination and intermediate stops — for flights, set code to the IATA airport code (use search_airports first). For a booking WITH STOPOVERS also pass legs[] (one entry per segment, one fewer than endpoints[]), otherwise every segment inherits the stop time as both its arrival and its departure. The top-level confirmation_number is the booking reference; when a single segment was booked under its own reference, put that one on the leg instead. Created as pending — confirm with update_transport. Set price to record the cost; it will appear on the booking and in the Budget tab.',
|
||||
inputSchema: {
|
||||
tripId: z.number().int().positive(),
|
||||
type: z.enum(['flight', 'train', 'car', 'cruise']),
|
||||
type: z.enum(TRANSPORT_TYPES),
|
||||
title: z.string().min(1).max(200),
|
||||
status: z.enum(['pending', 'confirmed', 'cancelled']).optional().default('pending'),
|
||||
start_day_id: z.number().int().positive().optional().describe('Departure day'),
|
||||
@@ -595,7 +604,7 @@ export class ReservationsMcp {
|
||||
})
|
||||
async createTransport(
|
||||
{ tripId, type, title, status, start_day_id, end_day_id, reservation_time, reservation_end_time, confirmation_number, notes, metadata, endpoints, legs, needs_review, price, budget_category }: {
|
||||
tripId: number; type: 'flight' | 'train' | 'car' | 'cruise'; title: string;
|
||||
tripId: number; type: TransportType; title: string;
|
||||
status?: 'pending' | 'confirmed' | 'cancelled'; start_day_id?: number; end_day_id?: number;
|
||||
reservation_time?: string; reservation_end_time?: string; confirmation_number?: string; notes?: string;
|
||||
metadata?: Record<string, string>; endpoints?: TransportEndpoint[]; legs?: TransportLegInput[]; needs_review?: boolean;
|
||||
@@ -683,7 +692,7 @@ export class ReservationsMcp {
|
||||
inputSchema: {
|
||||
tripId: z.number().int().positive(),
|
||||
reservationId: z.number().int().positive(),
|
||||
type: z.enum(['flight', 'train', 'car', 'cruise']).optional(),
|
||||
type: z.enum(TRANSPORT_TYPES).optional(),
|
||||
title: z.string().min(1).max(200).optional(),
|
||||
status: z.enum(['pending', 'confirmed', 'cancelled']).optional(),
|
||||
start_day_id: z.number().int().positive().optional().describe('Departure day'),
|
||||
@@ -702,7 +711,7 @@ export class ReservationsMcp {
|
||||
})
|
||||
async updateTransport(
|
||||
{ tripId, reservationId, type, title, status, start_day_id, end_day_id, reservation_time, reservation_end_time, confirmation_number, notes, metadata, endpoints, legs, needs_review }: {
|
||||
tripId: number; reservationId: number; type?: 'flight' | 'train' | 'car' | 'cruise'; title?: string;
|
||||
tripId: number; reservationId: number; type?: TransportType; title?: string;
|
||||
status?: 'pending' | 'confirmed' | 'cancelled'; start_day_id?: number; end_day_id?: number;
|
||||
reservation_time?: string; reservation_end_time?: string; confirmation_number?: string; notes?: string;
|
||||
metadata?: Record<string, string>; endpoints?: TransportEndpoint[]; legs?: TransportLegInput[]; needs_review?: boolean;
|
||||
|
||||
@@ -1040,3 +1040,160 @@ describe('Tool: update_transport (multi-leg)', () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// The full type list
|
||||
//
|
||||
// The planner's transport picker offers ten modes; these tools accepted four, so
|
||||
// a bus or a ferry could be planned in the UI and not through an assistant. The
|
||||
// cases below pin the whole picker list, and pin that widening it did not also
|
||||
// hand legs[] to a mode whose form never writes them.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/** client/src/components/Planner/TransportModal.tsx, in the picker's order. */
|
||||
const PICKER_TRANSPORT_TYPES = [
|
||||
'flight', 'train', 'bus', 'car', 'taxi', 'bicycle', 'cruise', 'ferry', 'transit', 'transport_other',
|
||||
] as const;
|
||||
|
||||
/** The six the tools used to reject outright. */
|
||||
const NEWLY_ACCEPTED = ['bus', 'taxi', 'bicycle', 'ferry', 'transit', 'transport_other'] as const;
|
||||
|
||||
describe('Transport tools: the full type list', () => {
|
||||
it.each(PICKER_TRANSPORT_TYPES)('create_transport accepts %s', async (type) => {
|
||||
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, title: `A ${type} booking` },
|
||||
});
|
||||
expect((result as { isError?: boolean }).isError).toBeFalsy();
|
||||
const data = parseToolResult(result) as any;
|
||||
expect(data.reservation.type).toBe(type);
|
||||
const row = testDb.prepare('SELECT type FROM reservations WHERE id = ?').get(data.reservation.id) as any;
|
||||
expect(row.type).toBe(type);
|
||||
});
|
||||
});
|
||||
|
||||
it('stores a bus booking with its stops, times and booking reference', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const depDay = createDay(testDb, trip.id, { day_number: 1 });
|
||||
const arrDay = createDay(testDb, trip.id, { day_number: 2 });
|
||||
await withHarness(user.id, async (h) => {
|
||||
const result = await h.client.callTool({
|
||||
name: 'create_transport',
|
||||
arguments: {
|
||||
tripId: trip.id, type: 'bus', title: 'Zurich → Milan',
|
||||
start_day_id: depDay.id, end_day_id: arrDay.id,
|
||||
reservation_time: '22:30', reservation_end_time: '06:15',
|
||||
confirmation_number: 'FLIX-8891',
|
||||
endpoints: [
|
||||
{ role: 'from', sequence: 0, name: 'Zurich Sihlquai', lat: 47.3846, lng: 8.5324 },
|
||||
{ role: 'to', sequence: 1, name: 'Milano Lampugnano', lat: 45.4936, lng: 9.1177 },
|
||||
],
|
||||
},
|
||||
});
|
||||
const data = parseToolResult(result) as any;
|
||||
expect(data.reservation.type).toBe('bus');
|
||||
expect(data.reservation.confirmation_number).toBe('FLIX-8891');
|
||||
expect(data.reservation.endpoints).toHaveLength(2);
|
||||
const row = testDb.prepare('SELECT day_id, end_day_id FROM reservations WHERE id = ?').get(data.reservation.id) as any;
|
||||
expect(row.day_id).toBe(depDay.id);
|
||||
expect(row.end_day_id).toBe(arrDay.id);
|
||||
});
|
||||
});
|
||||
|
||||
it('reports a missing coordinate on a non-flight endpoint rather than failing the insert', 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: 'ferry', title: 'Piraeus → Santorini',
|
||||
endpoints: [{ role: 'from', sequence: 0, name: 'Piraeus' }],
|
||||
},
|
||||
});
|
||||
expect((result as { isError?: boolean }).isError).toBe(true);
|
||||
expect(errorText(result)).toContain('missing coordinates');
|
||||
});
|
||||
});
|
||||
|
||||
it.each(NEWLY_ACCEPTED)('update_transport accepts %s as the new type', async (type) => {
|
||||
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: 'car', title: 'Rental' },
|
||||
});
|
||||
const { reservation } = parseToolResult(created) as { reservation: { id: number } };
|
||||
|
||||
const result = await h.client.callTool({
|
||||
name: 'update_transport',
|
||||
arguments: { tripId: trip.id, reservationId: reservation.id, type },
|
||||
});
|
||||
expect((result as { isError?: boolean }).isError).toBeFalsy();
|
||||
const row = testDb.prepare('SELECT type FROM reservations WHERE id = ?').get(reservation.id) as any;
|
||||
expect(row.type).toBe(type);
|
||||
});
|
||||
});
|
||||
|
||||
it('update_transport reaches a booking created as a bus', 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: 'bus', title: 'Night bus' },
|
||||
});
|
||||
const { reservation } = parseToolResult(created) as { reservation: { id: number } };
|
||||
|
||||
const result = await h.client.callTool({
|
||||
name: 'update_transport',
|
||||
arguments: { tripId: trip.id, reservationId: reservation.id, status: 'confirmed' },
|
||||
});
|
||||
expect((result as { isError?: boolean }).isError).toBeFalsy();
|
||||
const row = testDb.prepare('SELECT status FROM reservations WHERE id = ?').get(reservation.id) as any;
|
||||
expect(row.status).toBe('confirmed');
|
||||
});
|
||||
});
|
||||
|
||||
it('still refuses a type the picker does not offer', 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: 'teleport', title: 'Nope' },
|
||||
});
|
||||
expect((result as { isError?: boolean }).isError).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
it('still refuses legs on a mode whose form never writes them', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const day = createDay(testDb, trip.id, { day_number: 1 });
|
||||
await withHarness(user.id, async (h) => {
|
||||
const result = await h.client.callTool({
|
||||
name: 'create_transport',
|
||||
arguments: {
|
||||
tripId: trip.id, type: 'bus', title: 'Zurich → Milan via Lugano',
|
||||
endpoints: [
|
||||
{ role: 'from', sequence: 0, name: 'Zurich', lat: 47.38, lng: 8.53 },
|
||||
{ role: 'stop', sequence: 1, name: 'Lugano', lat: 46.01, lng: 8.96 },
|
||||
{ role: 'to', sequence: 2, name: 'Milan', lat: 45.49, lng: 9.11 },
|
||||
],
|
||||
legs: [
|
||||
{ dep_day_id: day.id, dep_time: '22:30', arr_day_id: day.id, arr_time: '00:40' },
|
||||
{ dep_day_id: day.id, dep_time: '00:50', arr_day_id: day.id, arr_time: '06:15' },
|
||||
],
|
||||
},
|
||||
});
|
||||
expect((result as { isError?: boolean }).isError).toBe(true);
|
||||
expect(errorText(result)).toContain('only supported for flight and train');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user