mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-21 06:26:26 +00:00
feat(mcp): trip length, reminders, cover search, and the member permission
A dateless trip made by a tool silently got seven days, and reminder_days was stuck on the default of three with no way to turn it off. Cover images had no search at all. add_trip_member and remove_trip_member hardcoded an owner check while REST consults the configurable permission matrix and the plugin RPC does too: three surfaces, two behaviours. They now use the same permission call update_trip already made, so an instance that lowered member_manage behaves consistently. The guest tools stay owner-only, because their REST routes are owner-only by design and document it.
This commit is contained in:
Maurice
@@ -1,8 +1,8 @@
|
||||
import {
|
||||
McpController, Tool, Resource, ResourceTemplate, Prompt, type McpContext,
|
||||
TOOL_ANNOTATIONS_READONLY, TOOL_ANNOTATIONS_WRITE,
|
||||
TOOL_ANNOTATIONS_READONLY, TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_OPEN_WORLD_READONLY,
|
||||
TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT,
|
||||
demoDenied, ok,
|
||||
demoDenied, errorResult, ok,
|
||||
} from '../../nest-mcp';
|
||||
import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service';
|
||||
import { z } from 'zod';
|
||||
@@ -13,7 +13,7 @@ import { TripReadModelService } from '../trip-read-model/trip-read-model.service
|
||||
import { ADDON_IDS } from '../../addons';
|
||||
import { MAX_MCP_TRIP_DAYS, noAccess, permissionDenied } from '../../mcp/tools/_shared';
|
||||
import { canRead, canReadTrips, canDeleteTrips } from '../../mcp/scopes';
|
||||
import { TripsService, NotFoundError, ValidationError } from './trips.service';
|
||||
import { TripsService, MAX_TRIP_DAYS, NotFoundError, ValidationError } from './trips.service';
|
||||
import { TodoService } from '../todo/todo.service';
|
||||
import { CollabService } from '../collab/collab.service';
|
||||
import { AddonsService } from '../addons/addons.service';
|
||||
@@ -87,13 +87,18 @@ export class TripsMcp {
|
||||
start_date: z.string().regex(/^\d{4}-\d{2}-\d{2}$/).optional().describe('Start date (YYYY-MM-DD)'),
|
||||
end_date: z.string().regex(/^\d{4}-\d{2}-\d{2}$/).optional().describe('End date (YYYY-MM-DD)'),
|
||||
currency: z.string().length(3).optional().describe('Currency code (e.g. EUR, USD)'),
|
||||
day_count: z.number().int().min(1).max(MAX_TRIP_DAYS).optional().describe(
|
||||
'How many days a trip without dates gets (default 7). Ignored when start_date and end_date are both set, because the range decides the count.'),
|
||||
reminder_days: z.number().int().min(0).max(30).optional().describe(
|
||||
'How many days before departure the pre-trip reminder fires (default 3). 0 turns the reminder off.'),
|
||||
},
|
||||
annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT,
|
||||
access: { group: 'trips', mode: 'write' },
|
||||
})
|
||||
async createTrip(
|
||||
{ title, description, start_date, end_date, currency }: {
|
||||
{ title, description, start_date, end_date, currency, day_count, reminder_days }: {
|
||||
title: string; description?: string; start_date?: string; end_date?: string; currency?: string;
|
||||
day_count?: number; reminder_days?: number;
|
||||
},
|
||||
ctx: McpContext,
|
||||
) {
|
||||
@@ -111,7 +116,7 @@ export class TripsMcp {
|
||||
if (start_date && end_date && new Date(end_date) < new Date(start_date)) {
|
||||
return { content: [{ type: 'text' as const, text: 'End date must be after start date.' }], isError: true };
|
||||
}
|
||||
const { trip } = this.trips.create(ctx.userId, { title, description, start_date, end_date, currency }, MAX_MCP_TRIP_DAYS);
|
||||
const { trip } = this.trips.create(ctx.userId, { title, description, start_date, end_date, currency, day_count, reminder_days }, MAX_MCP_TRIP_DAYS);
|
||||
return ok({ trip });
|
||||
}
|
||||
|
||||
@@ -121,12 +126,18 @@ export class TripsMcp {
|
||||
inputSchema: {
|
||||
tripId: z.number().int().positive(),
|
||||
title: z.string().min(1).max(200).optional(),
|
||||
description: z.string().max(2000).optional(),
|
||||
description: z.string().max(2000).nullable().optional().describe('Trip description; null removes it'),
|
||||
start_date: z.string().regex(/^\d{4}-\d{2}-\d{2}$/).optional(),
|
||||
end_date: z.string().regex(/^\d{4}-\d{2}-\d{2}$/).optional(),
|
||||
clear_dates: z.boolean().optional().describe(
|
||||
'Drop both dates and turn the trip back into a dateless one. Cannot be combined with start_date or end_date; pair it with day_count to say how many days the dateless trip keeps'),
|
||||
currency: z.string().length(3).optional(),
|
||||
is_archived: z.boolean().optional().describe('Archive (true) or unarchive (false) the trip'),
|
||||
cover_image: z.string().optional().describe('Cover image path, e.g. /uploads/covers/abc.jpg'),
|
||||
cover_image: z.string().nullable().optional().describe('Cover image path, e.g. /uploads/covers/abc.jpg; null removes the cover'),
|
||||
day_count: z.number().int().min(1).max(MAX_TRIP_DAYS).optional().describe(
|
||||
'Resize a trip without dates to this many days. Days that still hold content are never trimmed. Ignored while the trip has a date range, because the range decides the count'),
|
||||
reminder_days: z.number().int().min(0).max(30).optional().describe(
|
||||
'How many days before departure the pre-trip reminder fires. 0 turns the reminder off'),
|
||||
date_shift_mode: z.enum(['keep_bookings', 'shift_all']).optional().describe(
|
||||
'When changing dates: keep_bookings (default) keeps dated reservations/accommodations on their dates while day plans move; shift_all moves the whole itinerary, bookings included'),
|
||||
},
|
||||
@@ -134,15 +145,18 @@ export class TripsMcp {
|
||||
access: { group: 'trips', mode: 'write' },
|
||||
})
|
||||
async updateTrip(
|
||||
{ tripId, title, description, start_date, end_date, currency, is_archived, cover_image, date_shift_mode }: {
|
||||
tripId: number; title?: string; description?: string; start_date?: string; end_date?: string;
|
||||
currency?: string; is_archived?: boolean; cover_image?: string; date_shift_mode?: 'keep_bookings' | 'shift_all';
|
||||
{ tripId, title, description, start_date, end_date, clear_dates, currency, is_archived, cover_image, day_count, reminder_days, date_shift_mode }: {
|
||||
tripId: number; title?: string; description?: string | null; start_date?: string; end_date?: string;
|
||||
clear_dates?: boolean; currency?: string; is_archived?: boolean; cover_image?: string | null;
|
||||
day_count?: number; reminder_days?: number; date_shift_mode?: 'keep_bookings' | 'shift_all';
|
||||
},
|
||||
ctx: McpContext,
|
||||
) {
|
||||
if (this.auth.isDemoUser(ctx.userId)) return demoDenied();
|
||||
if (!this.trips.canAccessTrip(tripId, ctx.userId)) return noAccess();
|
||||
if (!this.guards.hasTripPermission('trip_edit', tripId, ctx.userId)) return permissionDenied();
|
||||
if (clear_dates && (start_date || end_date))
|
||||
return errorResult('clear_dates cannot be combined with start_date or end_date.');
|
||||
if (start_date) {
|
||||
const d = new Date(start_date + 'T00:00:00Z');
|
||||
if (Number.isNaN(d.getTime()) || d.toISOString().slice(0, 10) !== start_date)
|
||||
@@ -153,13 +167,40 @@ export class TripsMcp {
|
||||
if (Number.isNaN(d.getTime()) || d.toISOString().slice(0, 10) !== end_date)
|
||||
return { content: [{ type: 'text' as const, text: 'end_date is not a valid calendar date.' }], isError: true };
|
||||
}
|
||||
// updateTrip reads every field with a `!== undefined` presence sentinel, so a
|
||||
// dated trip only goes back to dateless once an explicit null reaches it. A flag
|
||||
// rather than a nullable start_date/end_date: clients that serialise an omitted
|
||||
// optional as null would otherwise wipe the dates of every trip they touched.
|
||||
const dates: { start_date?: string | null; end_date?: string | null } = clear_dates
|
||||
? { start_date: null, end_date: null }
|
||||
: { start_date, end_date };
|
||||
// update() re-anchors the budget before the trip row moves off the old
|
||||
// currency (#1543) and then runs the legacy updateTrip core.
|
||||
const { updatedTrip } = await this.trips.update(tripId, ctx.userId, { title, description, start_date, end_date, currency, is_archived, cover_image, date_shift_mode }, 'user');
|
||||
const { updatedTrip } = await this.trips.update(tripId, ctx.userId, { title, description, ...dates, currency, is_archived, cover_image, day_count, reminder_days, date_shift_mode }, 'user');
|
||||
this.guards.safeBroadcast(tripId, 'trip:updated', { trip: updatedTrip });
|
||||
return ok({ trip: updatedTrip });
|
||||
}
|
||||
|
||||
@Tool({
|
||||
name: 'search_cover_images',
|
||||
description: 'Search Unsplash for candidate cover photos and return their URLs, thumbnails and photographer credits. Use this when the user wants a cover image but has no URL of their own, then pass the chosen photo\'s url back as update_trip\'s cover_image. Nothing is saved to a trip here.',
|
||||
inputSchema: {
|
||||
query: z.string().min(1).max(200).describe('What the cover should show, e.g. "Lisbon rooftops at sunset"'),
|
||||
},
|
||||
annotations: TOOL_ANNOTATIONS_OPEN_WORLD_READONLY,
|
||||
access: (ctx) => canReadTrips(ctx.scopes),
|
||||
})
|
||||
async searchCoverImages({ query }: { query: string }, ctx: McpContext) {
|
||||
try {
|
||||
const result = await this.trips.searchCoverImages(query, ctx.userId);
|
||||
if ('error' in result) return errorResult(result.error);
|
||||
return ok({ photos: result.photos });
|
||||
} catch (err) {
|
||||
console.error('Unsplash cover image error:', err);
|
||||
return errorResult('Error searching for cover images');
|
||||
}
|
||||
}
|
||||
|
||||
@Tool({
|
||||
name: 'delete_trip',
|
||||
description: 'Delete a trip. Only the trip owner can delete it.',
|
||||
@@ -288,7 +329,7 @@ export class TripsMcp {
|
||||
|
||||
@Tool({
|
||||
name: 'add_trip_member',
|
||||
description: 'Add a user to a trip by their username or email address. Only the trip owner can do this.',
|
||||
description: 'Add a user to a trip by their username or email address. Needs the member_manage permission, which by default only the trip owner holds. Use create_trip_guest instead for a companion who has no TREK account.',
|
||||
inputSchema: {
|
||||
tripId: z.number().int().positive(),
|
||||
identifier: z.string().min(1).describe('Username or email of the user to add'),
|
||||
@@ -300,8 +341,11 @@ export class TripsMcp {
|
||||
if (this.auth.isDemoUser(ctx.userId)) return demoDenied();
|
||||
if (!this.trips.canAccessTrip(tripId, ctx.userId)) return noAccess();
|
||||
const ownerRow = this.trips.getOwner(tripId);
|
||||
if (!ownerRow || ownerRow.user_id !== ctx.userId)
|
||||
return { content: [{ type: 'text' as const, text: 'Only the trip owner can add members.' }], isError: true };
|
||||
if (!ownerRow) return noAccess();
|
||||
// member_manage rather than a hardcoded owner test: the action is admin-lowerable
|
||||
// in the permission matrix, and POST /api/trips/:id/members has always honoured
|
||||
// that setting (and the admin bypass inside checkPermission) where this did not.
|
||||
if (!this.guards.hasTripPermission('member_manage', tripId, ctx.userId)) return permissionDenied();
|
||||
try {
|
||||
const result = this.members.addMember(tripId, identifier, ownerRow.user_id, ctx.userId);
|
||||
this.guards.safeBroadcast(tripId, 'member:added', { member: result.member });
|
||||
@@ -314,7 +358,7 @@ export class TripsMcp {
|
||||
|
||||
@Tool({
|
||||
name: 'remove_trip_member',
|
||||
description: 'Remove a member from a trip. Only the trip owner can do this.',
|
||||
description: 'Remove somebody else from a trip. Needs the member_manage permission, which by default only the trip owner holds. When the user means themselves, prefer leave_trip: it says so plainly and needs no permission.',
|
||||
inputSchema: {
|
||||
tripId: z.number().int().positive(),
|
||||
memberId: z.number().int().positive().describe('User ID of the member to remove'),
|
||||
@@ -325,14 +369,38 @@ export class TripsMcp {
|
||||
async removeTripMember({ tripId, memberId }: { tripId: number; memberId: number }, ctx: McpContext) {
|
||||
if (this.auth.isDemoUser(ctx.userId)) return demoDenied();
|
||||
if (!this.trips.canAccessTrip(tripId, ctx.userId)) return noAccess();
|
||||
const ownerRow = this.trips.getOwner(tripId);
|
||||
if (!ownerRow || ownerRow.user_id !== ctx.userId)
|
||||
return { content: [{ type: 'text' as const, text: 'Only the trip owner can remove members.' }], isError: true };
|
||||
// Giving up your own access is not member management, so it carries no permission
|
||||
// requirement: the same self-removal bypass DELETE /api/trips/:id/members/:userId has.
|
||||
if (memberId !== ctx.userId && !this.guards.hasTripPermission('member_manage', tripId, ctx.userId))
|
||||
return permissionDenied();
|
||||
this.members.removeMember(tripId, memberId);
|
||||
this.guards.safeBroadcast(tripId, 'member:removed', { userId: memberId });
|
||||
return ok({ success: true });
|
||||
}
|
||||
|
||||
@Tool({
|
||||
name: 'leave_trip',
|
||||
description: 'Leave a trip you were invited to, giving up your own access to it. Prefer this over remove_trip_member whenever the user means themselves. The owner cannot leave their own trip.',
|
||||
inputSchema: {
|
||||
tripId: z.number().int().positive(),
|
||||
},
|
||||
annotations: TOOL_ANNOTATIONS_DELETE,
|
||||
access: { group: 'trips', mode: 'write' },
|
||||
})
|
||||
async leaveTrip({ tripId }: { tripId: number }, ctx: McpContext) {
|
||||
if (this.auth.isDemoUser(ctx.userId)) return demoDenied();
|
||||
if (!this.trips.canAccessTrip(tripId, ctx.userId)) return noAccess();
|
||||
const ownerRow = this.trips.getOwner(tripId);
|
||||
if (!ownerRow) return noAccess();
|
||||
// removeMember only deletes a trip_members row, and the owner has none, so an
|
||||
// owner calling this would get a success that changed nothing. Say so instead.
|
||||
if (ownerRow.user_id === ctx.userId)
|
||||
return errorResult('You own this trip, so you cannot leave it. Hand it to another member first, or delete it.');
|
||||
this.members.removeMember(tripId, ctx.userId);
|
||||
this.guards.safeBroadcast(tripId, 'member:removed', { userId: ctx.userId });
|
||||
return ok({ success: true });
|
||||
}
|
||||
|
||||
// --- GUESTS (#1362) ---
|
||||
//
|
||||
// add_trip_member resolves an existing account by username or email, so a
|
||||
@@ -345,7 +413,7 @@ export class TripsMcp {
|
||||
|
||||
@Tool({
|
||||
name: 'create_trip_guest',
|
||||
description: 'Add a travelling companion who has no TREK account to a trip. Use this when the person cannot be found by username or email — a guest can be assigned to budget splits, packing items, to-dos and day participants like any member, but never signs in and is never emailed. Only the trip owner can do this.',
|
||||
description: 'Add a travelling companion who has no TREK account to a trip. Use this when the person cannot be found by username or email. A guest can be assigned to budget splits, packing items, to-dos and day participants like any member, but never signs in and is never emailed. Only the trip owner can do this.',
|
||||
inputSchema: {
|
||||
tripId: z.number().int().positive(),
|
||||
name: z.string().min(1).max(50).describe('Display name of the guest, e.g. "Anna"'),
|
||||
@@ -399,7 +467,7 @@ export class TripsMcp {
|
||||
|
||||
@Tool({
|
||||
name: 'delete_trip_guest',
|
||||
description: 'Remove a guest from a trip. This deletes the guest outright — they exist only for this trip — and re-splits any expenses they were part of. Only the trip owner can do this.',
|
||||
description: 'Remove a guest from a trip. This deletes the guest outright (they exist only for this trip) and re-splits any expenses they were part of. Only the trip owner can do this.',
|
||||
inputSchema: {
|
||||
tripId: z.number().int().positive(),
|
||||
guestId: z.number().int().positive().describe('User ID of the guest, from list_trip_members'),
|
||||
|
||||
@@ -38,7 +38,8 @@ vi.mock('../../../src/websocket', () => ({ broadcast: broadcastMock }));
|
||||
import { createTables } from '../../../src/db/schema';
|
||||
import { runMigrations } from '../../../src/db/migrations';
|
||||
import { resetTestDb } from '../../helpers/test-db';
|
||||
import { createUser, createTrip, addTripMember } from '../../helpers/factories';
|
||||
import { invalidatePermissionsCache } from '../../../src/nest/permissions/permissions-cache';
|
||||
import { createUser, createAdmin, createTrip, addTripMember } from '../../helpers/factories';
|
||||
import { createMcpHarness, parseToolResult, type McpHarness } from '../../helpers/mcp-harness';
|
||||
|
||||
beforeAll(() => {
|
||||
@@ -50,6 +51,9 @@ beforeEach(() => {
|
||||
resetTestDb(testDb);
|
||||
broadcastMock.mockClear();
|
||||
delete process.env.DEMO_MODE;
|
||||
// resetTestDb truncates app_settings, but the permission cache is module-scoped
|
||||
// and would keep serving whatever the previous case configured.
|
||||
invalidatePermissionsCache();
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
@@ -61,6 +65,12 @@ async function withHarness(userId: number, fn: (h: McpHarness) => Promise<void>)
|
||||
try { await fn(h); } finally { await h.cleanup(); }
|
||||
}
|
||||
|
||||
/** Lower a configurable action the way the admin permission panel does. */
|
||||
function setPermission(action: string, level: string) {
|
||||
testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)').run(`perm_${action}`, level);
|
||||
invalidatePermissionsCache();
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// list_trip_members
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -161,6 +171,57 @@ describe('Tool: add_trip_member', () => {
|
||||
expect(result.isError).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
it('lets a collaborator add when member_manage sits at trip_member', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: collaborator } = createUser(testDb);
|
||||
const { user: outsider } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, collaborator.id);
|
||||
setPermission('member_manage', 'trip_member');
|
||||
await withHarness(collaborator.id, async (h) => {
|
||||
const result = await h.client.callTool({
|
||||
name: 'add_trip_member',
|
||||
arguments: { tripId: trip.id, identifier: outsider.username },
|
||||
});
|
||||
expect(result.isError).toBeFalsy();
|
||||
const row = testDb.prepare('SELECT invited_by FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, outsider.id) as any;
|
||||
expect(row).toBeTruthy();
|
||||
expect(row.invited_by).toBe(collaborator.id);
|
||||
});
|
||||
});
|
||||
|
||||
it('lets a site admin on the trip add a member without owning it', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: admin } = createAdmin(testDb);
|
||||
const { user: outsider } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, admin.id);
|
||||
await withHarness(admin.id, async (h) => {
|
||||
const result = await h.client.callTool({
|
||||
name: 'add_trip_member',
|
||||
arguments: { tripId: trip.id, identifier: outsider.username },
|
||||
});
|
||||
expect(result.isError).toBeFalsy();
|
||||
expect(testDb.prepare('SELECT user_id FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, outsider.id)).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
it('still refuses a collaborator while member_manage sits at its trip_owner default', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: collaborator } = createUser(testDb);
|
||||
const { user: outsider } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, collaborator.id);
|
||||
await withHarness(collaborator.id, async (h) => {
|
||||
const result = await h.client.callTool({
|
||||
name: 'add_trip_member',
|
||||
arguments: { tripId: trip.id, identifier: outsider.username },
|
||||
});
|
||||
expect(result.isError).toBe(true);
|
||||
expect(testDb.prepare('SELECT user_id FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, outsider.id)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -209,6 +270,150 @@ describe('Tool: remove_trip_member', () => {
|
||||
expect(result.isError).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
it('leaves another member in place when a collaborator has no member_manage', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: member } = createUser(testDb);
|
||||
const { user: other } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, member.id);
|
||||
addTripMember(testDb, trip.id, other.id);
|
||||
await withHarness(member.id, async (h) => {
|
||||
const result = await h.client.callTool({
|
||||
name: 'remove_trip_member',
|
||||
arguments: { tripId: trip.id, memberId: other.id },
|
||||
});
|
||||
expect(result.isError).toBe(true);
|
||||
expect(testDb.prepare('SELECT user_id FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, other.id)).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
it('lets a collaborator remove another member when member_manage sits at trip_member', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: member } = createUser(testDb);
|
||||
const { user: other } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, member.id);
|
||||
addTripMember(testDb, trip.id, other.id);
|
||||
setPermission('member_manage', 'trip_member');
|
||||
await withHarness(member.id, async (h) => {
|
||||
const result = await h.client.callTool({
|
||||
name: 'remove_trip_member',
|
||||
arguments: { tripId: trip.id, memberId: other.id },
|
||||
});
|
||||
expect(result.isError).toBeFalsy();
|
||||
expect(testDb.prepare('SELECT user_id FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, other.id)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
it('lets a site admin on the trip remove a member without owning it', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: admin } = createAdmin(testDb);
|
||||
const { user: other } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, admin.id);
|
||||
addTripMember(testDb, trip.id, other.id);
|
||||
await withHarness(admin.id, async (h) => {
|
||||
const result = await h.client.callTool({
|
||||
name: 'remove_trip_member',
|
||||
arguments: { tripId: trip.id, memberId: other.id },
|
||||
});
|
||||
expect(result.isError).toBeFalsy();
|
||||
expect(testDb.prepare('SELECT user_id FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, other.id)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
it('lets a member remove themselves without member_manage', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: member } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, member.id);
|
||||
await withHarness(member.id, async (h) => {
|
||||
const result = await h.client.callTool({
|
||||
name: 'remove_trip_member',
|
||||
arguments: { tripId: trip.id, memberId: member.id },
|
||||
});
|
||||
expect((parseToolResult(result) as any).success).toBe(true);
|
||||
expect(testDb.prepare('SELECT user_id FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, member.id)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// leave_trip
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('Tool: leave_trip', () => {
|
||||
it('drops the caller from the roster', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: member } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, member.id);
|
||||
await withHarness(member.id, async (h) => {
|
||||
const result = await h.client.callTool({ name: 'leave_trip', arguments: { tripId: trip.id } });
|
||||
expect((parseToolResult(result) as any).success).toBe(true);
|
||||
expect(testDb.prepare('SELECT user_id FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, member.id)).toBeUndefined();
|
||||
expect(testDb.prepare('SELECT id FROM trips WHERE id = ?').get(trip.id)).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
it('broadcasts member:removed for the leaving user', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: member } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, member.id);
|
||||
await withHarness(member.id, async (h) => {
|
||||
await h.client.callTool({ name: 'leave_trip', arguments: { tripId: trip.id } });
|
||||
expect(broadcastMock).toHaveBeenCalledWith(trip.id, 'member:removed', expect.objectContaining({ userId: member.id }));
|
||||
});
|
||||
});
|
||||
|
||||
it('leaves the other members alone', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: member } = createUser(testDb);
|
||||
const { user: other } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, member.id);
|
||||
addTripMember(testDb, trip.id, other.id);
|
||||
await withHarness(member.id, async (h) => {
|
||||
await h.client.callTool({ name: 'leave_trip', arguments: { tripId: trip.id } });
|
||||
expect(testDb.prepare('SELECT user_id FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, other.id)).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
it('refuses the owner and keeps the trip theirs', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
await withHarness(owner.id, async (h) => {
|
||||
const result = await h.client.callTool({ name: 'leave_trip', arguments: { tripId: trip.id } });
|
||||
expect(result.isError).toBe(true);
|
||||
const row = testDb.prepare('SELECT user_id FROM trips WHERE id = ?').get(trip.id) as any;
|
||||
expect(row.user_id).toBe(owner.id);
|
||||
});
|
||||
});
|
||||
|
||||
it('returns access denied for a non-member', async () => {
|
||||
const { user: outsider } = createUser(testDb);
|
||||
const { user: owner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
await withHarness(outsider.id, async (h) => {
|
||||
const result = await h.client.callTool({ name: 'leave_trip', arguments: { tripId: trip.id } });
|
||||
expect(result.isError).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
it('blocks demo user', async () => {
|
||||
process.env.DEMO_MODE = 'true';
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user } = createUser(testDb, { email: 'demo@nomad.app' });
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, user.id);
|
||||
await withHarness(user.id, async (h) => {
|
||||
const result = await h.client.callTool({ name: 'leave_trip', arguments: { tripId: trip.id } });
|
||||
expect(result.isError).toBe(true);
|
||||
expect(testDb.prepare('SELECT user_id FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, user.id)).toBeTruthy();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -411,7 +616,7 @@ describe('Tool: create_trip_guest', () => {
|
||||
expect(row.is_guest).toBe(1);
|
||||
expect(row.password_hash).toBe('');
|
||||
expect(row.display_name).toBe('Anna');
|
||||
// A synthetic address on an undeliverable domain — a guest is never emailed.
|
||||
// A synthetic address on an undeliverable domain: a guest is never emailed.
|
||||
expect(row.email).toMatch(/@guests\.invalid$/);
|
||||
});
|
||||
});
|
||||
@@ -654,3 +859,38 @@ describe('Tool: delete_trip_guest', () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// The guest routes carry TripOwnerGuard, which deliberately does NOT consult the
|
||||
// permission matrix: handing a trip over and creating or deleting guests are the
|
||||
// things a collaborator must never do, however generously the trip is configured.
|
||||
// Lowering member_manage moves add/remove_trip_member and must move nothing here.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('Guest tools stay owner-only', () => {
|
||||
it('refuses a collaborator all three guest tools even with member_manage at trip_member', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: collaborator } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, collaborator.id);
|
||||
let guestId = 0;
|
||||
await withHarness(owner.id, async (h) => { guestId = await makeGuest(h, trip.id, 'Anna'); });
|
||||
|
||||
setPermission('member_manage', 'trip_member');
|
||||
await withHarness(collaborator.id, async (h) => {
|
||||
expect((await h.client.callTool({
|
||||
name: 'create_trip_guest', arguments: { tripId: trip.id, name: 'Bea' },
|
||||
})).isError).toBe(true);
|
||||
expect((await h.client.callTool({
|
||||
name: 'rename_trip_guest', arguments: { tripId: trip.id, guestId, name: 'Renamed' },
|
||||
})).isError).toBe(true);
|
||||
expect((await h.client.callTool({
|
||||
name: 'delete_trip_guest', arguments: { tripId: trip.id, guestId },
|
||||
})).isError).toBe(true);
|
||||
});
|
||||
|
||||
const row = testDb.prepare('SELECT display_name FROM users WHERE id = ?').get(guestId) as any;
|
||||
expect(row.display_name).toBe('Anna');
|
||||
expect(testDb.prepare('SELECT COUNT(*) AS n FROM users WHERE is_guest = 1').get()).toEqual({ n: 1 });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -7,7 +7,7 @@
|
||||
* notice riding the attach ctx, and the scope gating (declarative
|
||||
* trips:write markers + the canReadTrips/canDeleteTrips predicates).
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest';
|
||||
import { describe, it, expect, vi, beforeAll, beforeEach, afterEach, afterAll } from 'vitest';
|
||||
|
||||
const { testDb, dbMock } = vi.hoisted(() => {
|
||||
const Database = require('better-sqlite3');
|
||||
@@ -55,6 +55,11 @@ beforeEach(() => {
|
||||
delete process.env.DEMO_MODE;
|
||||
});
|
||||
|
||||
// Only search_cover_images stubs fetch, and it stubs a different response per case.
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
testDb.close();
|
||||
});
|
||||
@@ -134,6 +139,57 @@ describe('Tool: create_trip', () => {
|
||||
expect(result.isError).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
it('gives a dateless trip the requested day_count instead of the default 7', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
await withHarness(user.id, async (h) => {
|
||||
const result = await h.client.callTool({ name: 'create_trip', arguments: { title: 'Open Ended', day_count: 12 } });
|
||||
const data = parseToolResult(result) as any;
|
||||
const days = testDb.prepare('SELECT COUNT(*) as c FROM days WHERE trip_id = ?').get(data.trip.id) as { c: number };
|
||||
expect(days.c).toBe(12);
|
||||
const row = testDb.prepare('SELECT start_date, end_date FROM trips WHERE id = ?').get(data.trip.id) as any;
|
||||
expect(row.start_date).toBeNull();
|
||||
expect(row.end_date).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
it('refuses a day_count outside 1..365', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
await withHarness(user.id, async (h) => {
|
||||
expect((await h.client.callTool({ name: 'create_trip', arguments: { title: 'Zero', day_count: 0 } })).isError).toBe(true);
|
||||
expect((await h.client.callTool({ name: 'create_trip', arguments: { title: 'Huge', day_count: 400 } })).isError).toBe(true);
|
||||
expect(testDb.prepare('SELECT COUNT(*) as c FROM trips').get()).toEqual({ c: 0 });
|
||||
});
|
||||
});
|
||||
|
||||
it('stores the requested reminder_days', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
await withHarness(user.id, async (h) => {
|
||||
const result = await h.client.callTool({ name: 'create_trip', arguments: { title: 'Reminded', reminder_days: 14 } });
|
||||
const data = parseToolResult(result) as any;
|
||||
const row = testDb.prepare('SELECT reminder_days FROM trips WHERE id = ?').get(data.trip.id) as any;
|
||||
expect(row.reminder_days).toBe(14);
|
||||
});
|
||||
});
|
||||
|
||||
it('turns the reminder off with reminder_days 0 rather than falling back to 3', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
await withHarness(user.id, async (h) => {
|
||||
const result = await h.client.callTool({ name: 'create_trip', arguments: { title: 'Quiet', reminder_days: 0 } });
|
||||
const data = parseToolResult(result) as any;
|
||||
const row = testDb.prepare('SELECT reminder_days FROM trips WHERE id = ?').get(data.trip.id) as any;
|
||||
expect(row.reminder_days).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
it('refuses a reminder_days outside 0..30', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
await withHarness(user.id, async (h) => {
|
||||
expect((await h.client.callTool({ name: 'create_trip', arguments: { title: 'Early', reminder_days: 31 } })).isError).toBe(true);
|
||||
expect((await h.client.callTool({ name: 'create_trip', arguments: { title: 'Negative', reminder_days: -1 } })).isError).toBe(true);
|
||||
expect(testDb.prepare('SELECT COUNT(*) as c FROM trips').get()).toEqual({ c: 0 });
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -272,6 +328,173 @@ describe('Tool: update_trip', () => {
|
||||
).all(ownPlanId, user.id) as { date: string }[];
|
||||
expect(shifted.map(r => r.date)).toEqual(['2026-09-09', '2026-09-10']);
|
||||
});
|
||||
|
||||
it('clear_dates turns a dated trip back into a dateless one and un-dates its days', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id, { start_date: '2026-07-01', end_date: '2026-07-05' });
|
||||
await withHarness(user.id, async (h) => {
|
||||
const result = await h.client.callTool({ name: 'update_trip', arguments: { tripId: trip.id, clear_dates: true } });
|
||||
expect(result.isError).toBeFalsy();
|
||||
const row = testDb.prepare('SELECT start_date, end_date FROM trips WHERE id = ?').get(trip.id) as any;
|
||||
expect(row.start_date).toBeNull();
|
||||
expect(row.end_date).toBeNull();
|
||||
const dated = testDb.prepare('SELECT COUNT(*) as c FROM days WHERE trip_id = ? AND date IS NOT NULL').get(trip.id) as { c: number };
|
||||
expect(dated.c).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
it('clear_dates with a day_count resizes the now dateless trip', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id, { start_date: '2026-07-01', end_date: '2026-07-05' });
|
||||
await withHarness(user.id, async (h) => {
|
||||
await h.client.callTool({ name: 'update_trip', arguments: { tripId: trip.id, clear_dates: true, day_count: 3 } });
|
||||
const days = testDb.prepare('SELECT COUNT(*) as c FROM days WHERE trip_id = ?').get(trip.id) as { c: number };
|
||||
expect(days.c).toBe(3);
|
||||
});
|
||||
});
|
||||
|
||||
it('refuses clear_dates alongside a start_date and leaves the trip untouched', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id, { start_date: '2026-07-01', end_date: '2026-07-05' });
|
||||
await withHarness(user.id, async (h) => {
|
||||
const result = await h.client.callTool({
|
||||
name: 'update_trip',
|
||||
arguments: { tripId: trip.id, clear_dates: true, start_date: '2026-08-01' },
|
||||
});
|
||||
expect(result.isError).toBe(true);
|
||||
const row = testDb.prepare('SELECT start_date FROM trips WHERE id = ?').get(trip.id) as any;
|
||||
expect(row.start_date).toBe('2026-07-01');
|
||||
});
|
||||
});
|
||||
|
||||
it('resizes a dateless trip with day_count', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
testDb.prepare('INSERT INTO days (trip_id, day_number, date) VALUES (?, 1, NULL), (?, 2, NULL)').run(trip.id, trip.id);
|
||||
await withHarness(user.id, async (h) => {
|
||||
await h.client.callTool({ name: 'update_trip', arguments: { tripId: trip.id, day_count: 6 } });
|
||||
const days = testDb.prepare('SELECT COUNT(*) as c FROM days WHERE trip_id = ?').get(trip.id) as { c: number };
|
||||
expect(days.c).toBe(6);
|
||||
});
|
||||
});
|
||||
|
||||
it('refuses a day_count outside 1..365', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id, { title: 'Untouched' });
|
||||
await withHarness(user.id, async (h) => {
|
||||
expect((await h.client.callTool({ name: 'update_trip', arguments: { tripId: trip.id, day_count: 0 } })).isError).toBe(true);
|
||||
expect((await h.client.callTool({ name: 'update_trip', arguments: { tripId: trip.id, day_count: 400 } })).isError).toBe(true);
|
||||
expect(testDb.prepare('SELECT COUNT(*) as c FROM days WHERE trip_id = ?').get(trip.id)).toEqual({ c: 0 });
|
||||
});
|
||||
});
|
||||
|
||||
it('changes reminder_days and can turn the reminder off', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
await withHarness(user.id, async (h) => {
|
||||
await h.client.callTool({ name: 'update_trip', arguments: { tripId: trip.id, reminder_days: 10 } });
|
||||
expect((testDb.prepare('SELECT reminder_days FROM trips WHERE id = ?').get(trip.id) as any).reminder_days).toBe(10);
|
||||
await h.client.callTool({ name: 'update_trip', arguments: { tripId: trip.id, reminder_days: 0 } });
|
||||
expect((testDb.prepare('SELECT reminder_days FROM trips WHERE id = ?').get(trip.id) as any).reminder_days).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
it('refuses a reminder_days outside 0..30 and keeps the stored value', 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_trip', arguments: { tripId: trip.id, reminder_days: 31 } });
|
||||
expect(result.isError).toBe(true);
|
||||
expect((testDb.prepare('SELECT reminder_days FROM trips WHERE id = ?').get(trip.id) as any).reminder_days).toBe(3);
|
||||
});
|
||||
});
|
||||
|
||||
it('clears the description with an explicit null', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id, { description: 'A great trip' });
|
||||
await withHarness(user.id, async (h) => {
|
||||
await h.client.callTool({ name: 'update_trip', arguments: { tripId: trip.id, description: null } });
|
||||
const row = testDb.prepare('SELECT description FROM trips WHERE id = ?').get(trip.id) as any;
|
||||
expect(row.description).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
it('clears the cover image with an explicit null', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
testDb.prepare('UPDATE trips SET cover_image = ? WHERE id = ?').run('/uploads/covers/old.jpg', trip.id);
|
||||
await withHarness(user.id, async (h) => {
|
||||
await h.client.callTool({ name: 'update_trip', arguments: { tripId: trip.id, cover_image: null } });
|
||||
const row = testDb.prepare('SELECT cover_image FROM trips WHERE id = ?').get(trip.id) as any;
|
||||
expect(row.cover_image).toBeNull();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// search_cover_images
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('Tool: search_cover_images', () => {
|
||||
function stubUnsplash(body: unknown, init: { ok?: boolean; status?: number } = {}) {
|
||||
vi.stubGlobal('fetch', vi.fn(async () => ({
|
||||
ok: init.ok ?? true,
|
||||
status: init.status ?? 200,
|
||||
json: async () => body,
|
||||
})));
|
||||
}
|
||||
|
||||
it('returns the photo candidates for a query', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
stubUnsplash({
|
||||
results: [{
|
||||
id: 'p1',
|
||||
urls: { regular: 'https://images.unsplash.com/photo-1.jpg', small: 'https://images.unsplash.com/thumb-1.jpg' },
|
||||
alt_description: 'Rooftops at sunset',
|
||||
user: { name: 'Ada L.' },
|
||||
links: { html: 'https://unsplash.com/photos/p1' },
|
||||
}],
|
||||
});
|
||||
await withHarness(user.id, async (h) => {
|
||||
const result = await h.client.callTool({ name: 'search_cover_images', arguments: { query: 'Lisbon rooftops' } });
|
||||
const data = parseToolResult(result) as any;
|
||||
expect(data.photos).toHaveLength(1);
|
||||
expect(data.photos[0].url).toBe('https://images.unsplash.com/photo-1.jpg');
|
||||
expect(data.photos[0].photographer).toBe('Ada L.');
|
||||
});
|
||||
});
|
||||
|
||||
it('reports an upstream failure as a tool error', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
stubUnsplash({ errors: ['Rate Limit Exceeded'] }, { ok: false, status: 429 });
|
||||
await withHarness(user.id, async (h) => {
|
||||
const result = await h.client.callTool({ name: 'search_cover_images', arguments: { query: 'Lisbon' } });
|
||||
expect(result.isError).toBe(true);
|
||||
expect((result.content as { text: string }[])[0].text).toBe('Rate Limit Exceeded');
|
||||
});
|
||||
});
|
||||
|
||||
it('refuses an empty query without reaching Unsplash', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const fetchMock = vi.fn();
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
await withHarness(user.id, async (h) => {
|
||||
const result = await h.client.callTool({ name: 'search_cover_images', arguments: { query: '' } });
|
||||
expect(result.isError).toBe(true);
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
it('saves nothing on the trip', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
stubUnsplash({ results: [] });
|
||||
await withHarness(user.id, async (h) => {
|
||||
await h.client.callTool({ name: 'search_cover_images', arguments: { query: 'Lisbon' } });
|
||||
const row = testDb.prepare('SELECT cover_image FROM trips WHERE id = ?').get(trip.id) as any;
|
||||
expect(row.cover_image).toBeNull();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -674,8 +897,8 @@ describe('scope gating', () => {
|
||||
}
|
||||
}
|
||||
|
||||
const WRITE_TOOLS = ['create_trip', 'update_trip', 'add_trip_member', 'remove_trip_member', 'copy_trip'];
|
||||
const READ_TOOLS = ['list_trip_members', 'export_trip_ics'];
|
||||
const WRITE_TOOLS = ['create_trip', 'update_trip', 'add_trip_member', 'remove_trip_member', 'leave_trip', 'copy_trip'];
|
||||
const READ_TOOLS = ['list_trip_members', 'export_trip_ics', 'search_cover_images'];
|
||||
const NAV_TOOLS = ['list_trips', 'get_trip_summary'];
|
||||
const SHARE_TOOLS = ['get_share_link', 'create_share_link', 'delete_share_link'];
|
||||
|
||||
|
||||
Reference in New Issue
Block a user