refactor(server): accommodation and packing item writes run as one use case per write, so REST and MCP send the same events

This commit is contained in:
Maurice
2026-10-10 16:09:20 +02:00
parent aba2ed1ee3
commit f9e756391c
20 changed files with 1093 additions and 229 deletions
+2 -2
View File
@@ -82,10 +82,10 @@ export function trekDemoToolGate(isDemoUser: (userId: number) => Promise<boolean
/**
* A service refusal (`DomainError`) thrown out of a tool answers the call with
* `errorResult(publicMessage)`: the same text REST sends as `{ error }`, and the
* `errorResult(publicMessage)` (or the refusal's own `mcpMessage`): the same text REST sends as `{ error }`, and the
* same result the tools built by hand from a `{ error, status }` return. Any
* other error propagates as before. Given to McpModule.forRoot (AppModule) and
* to the MCP test registry.
*/
export const trekMcpErrorMapper: McpErrorMapper = (err) =>
err instanceof DomainError ? errorResult(err.publicMessage) : undefined;
err instanceof DomainError ? errorResult(err.mcpMessage ?? err.publicMessage) : undefined;
@@ -1,34 +1,11 @@
import {
Body,
Controller,
Delete,
Query,
Get,
Headers,
HttpException,
Param,
Post,
Put,
UseGuards,
} from '@nestjs/common';
import { Body, Controller, Delete, Query, Get, Headers, Param, Post, Put, UseGuards } from '@nestjs/common';
import type { User } from '../../types';
import { AccommodationsService, type MirrorSender } from './accommodations.service';
import { AccommodationsService, type StayInput } from './accommodations.service';
import { AccommodationCreateDto, AccommodationUpdateDto } from './accommodations.dto';
import { JwtAuthGuard } from '../auth-core/jwt-auth.guard';
import { CurrentUser } from '../auth-core/current-user.decorator';
import { RequirePermission, TripAccessGuard } from '../permissions/trip-access.guard';
type AccommodationBody = {
place_id?: number;
start_day_id?: number;
end_day_id?: number;
check_in?: string | null;
check_in_end?: string | null;
check_out?: string | null;
confirmation?: string | null;
notes?: string | null;
};
/**
* /api/trips/:tripId/accommodations — trip-scoped lodging blocks.
*
@@ -40,9 +17,12 @@ type AccommodationBody = {
* reservation/budget deletions) with the forwarded X-Socket-Id.
*
* A booking also writes the day stop that puts it on the route. What that did to
* the day plan goes to every socket, the sender's included (see mirrorSender),
* and the answers carry the stop alongside the existing fields as well, for a
* session whose socket is down at that moment.
* the day plan goes to every socket, the sender's included, and the answers
* carry the stop alongside the existing fields as well, for a session whose
* socket is down at that moment.
*
* The handlers are adapters: the whole write, its checks and its events live in
* AccommodationsService's stay use cases, which the MCP tools call as well.
*/
@Controller('api/trips/:tripId/accommodations')
// TripAccessGuard resolves :tripId and 404s a trip the user cannot reach; mutations
@@ -52,21 +32,6 @@ type AccommodationBody = {
export class AccommodationsController {
constructor(private readonly accommodations: AccommodationsService) {}
/**
* The mirror's door out, with no socket to skip.
*
* The booking events above it are echo-suppressed because the client already drew
* what it sent. It never sent the day plan's side of the write: the stop is in the
* answer, but the order the night was seated into and the vias re-pinned around it
* are not, and a session that applies the stop from the answer alone keeps the
* stops behind it on their old numbers until a reload. Over the socket the stop
* and the order arrive together and in order, and the answer's copy of the stop
* is a duplicate the store already drops.
*/
private mirrorSender(tripId: string): MirrorSender {
return (event, payload) => this.accommodations.broadcast(tripId, event, payload, undefined);
}
@Get()
async list(@CurrentUser() user: User, @Param('tripId') tripId: string) {
return { accommodations: await this.accommodations.list(tripId) };
@@ -80,19 +45,7 @@ export class AccommodationsController {
@Body() rawBody: AccommodationCreateDto,
@Headers('x-socket-id') socketId?: string,
) {
const body = rawBody as AccommodationBody;
const { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes } = body;
if (!place_id || !start_day_id || !end_day_id) {
throw new HttpException({ error: 'place_id, start_day_id, and end_day_id are required' }, 400);
}
const errors = await this.accommodations.validateRefs(tripId, place_id, start_day_id, end_day_id);
if (errors.length > 0) {
throw new HttpException({ error: errors[0].message }, 404);
}
const { accommodation, mirror } = await this.accommodations.create(tripId, { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes } as never);
this.accommodations.broadcast(tripId, 'accommodation:created', { accommodation }, socketId);
this.accommodations.broadcast(tripId, 'reservation:created', {}, socketId);
await this.accommodations.announceMirror(tripId, mirror, this.mirrorSender(tripId), socketId);
const { accommodation, mirror } = await this.accommodations.createStay(tripId, rawBody as StayInput, this.accommodations.restWriter(tripId, user, socketId));
// The stop rides in the answer as well, for the session that booked the night
// with its socket down: over the socket it would already have it.
return { accommodation, assignment: mirror.created };
@@ -107,19 +60,7 @@ export class AccommodationsController {
@Body() rawBody: AccommodationUpdateDto,
@Headers('x-socket-id') socketId?: string,
) {
const body = rawBody as AccommodationBody;
const existing = await this.accommodations.get(id, tripId);
if (!existing) {
throw new HttpException({ error: 'Accommodation not found' }, 404);
}
const { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes } = body;
const errors = await this.accommodations.validateRefs(tripId, place_id, start_day_id, end_day_id);
if (errors.length > 0) {
throw new HttpException({ error: errors[0].message }, 404);
}
const { accommodation, mirror } = await this.accommodations.update(id, existing as never, { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes } as never);
this.accommodations.broadcast(tripId, 'accommodation:updated', { accommodation }, socketId);
await this.accommodations.announceMirror(tripId, mirror, this.mirrorSender(tripId), socketId);
const { accommodation, mirror } = await this.accommodations.updateStay(tripId, id, rawBody as StayInput, this.accommodations.restWriter(tripId, user, socketId));
return { accommodation, assignment: mirror.created, movedAssignment: mirror.moved, removedAssignments: mirror.removed };
}
@@ -132,21 +73,10 @@ export class AccommodationsController {
@Headers('x-socket-id') socketId?: string,
@Query('keepStop') keepStop?: string,
) {
if (!(await this.accommodations.get(id, tripId))) {
throw new HttpException({ error: 'Accommodation not found' }, 404);
}
// Turning a night back into a pause in road trip mode: the booking goes, the
// stop stays and becomes the traveller's. Everywhere else a cancelled booking
// takes the stop it brought with it.
const { linkedReservationIds, deletedBudgetItemIds, mirror } = await this.accommodations.remove(id, { keepStop: keepStop === 'true' });
await this.accommodations.announceMirror(tripId, mirror, this.mirrorSender(tripId), socketId);
for (const reservationId of linkedReservationIds) {
this.accommodations.broadcast(tripId, 'reservation:deleted', { reservationId }, socketId);
}
for (const itemId of deletedBudgetItemIds) {
this.accommodations.broadcast(tripId, 'budget:deleted', { itemId }, socketId);
}
this.accommodations.broadcast(tripId, 'accommodation:deleted', { accommodationId: Number(id) }, socketId);
const { mirror } = await this.accommodations.deleteStay(tripId, id, this.accommodations.restWriter(tripId, user, socketId), { keepStop: keepStop === 'true' });
return { success: true, removedAssignments: mirror.removed, updatedAssignments: mirror.updated };
}
}
@@ -9,7 +9,7 @@ import { z } from 'zod';
import { PlacesService } from '../places/places.service';
import { noAccess, permissionDenied } from '../../mcp/tools/_shared';
import { UnitOfWork } from '../database/unit-of-work';
import { AccommodationsService, type MirrorSender } from './accommodations.service';
import { AccommodationsService } from './accommodations.service';
function parseId(value: string | string[]): number | null {
const n = Number(Array.isArray(value) ? value[0] : value);
@@ -43,12 +43,6 @@ export class AccommodationsMcp {
private readonly uow: UnitOfWork,
) {}
/** The tools' own broadcast, handed to announceMirror so the day stop a booking
* writes is announced exactly as the REST route announces it. */
private mirrorSender(tripId: number): MirrorSender {
return (event, payload) => this.guards.safeBroadcast(tripId, event, payload as Record<string, unknown>);
}
@Tool({
name: 'create_accommodation',
description: 'Add an accommodation (hotel, Airbnb, etc.) to a trip, linked to a place and a date range. This also puts the place on its check-in day, so the stay shows up as a stop on the route.',
@@ -73,13 +67,8 @@ export class AccommodationsMcp {
},
ctx: McpContext,
) {
if (!(await this.accommodations.verifyTripAccess(tripId, ctx.userId))) return noAccess();
if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied();
const errors = await this.accommodations.validateAccommodationRefs(tripId, place_id, start_day_id, end_day_id);
if (errors.length > 0) return { content: [{ type: 'text' as const, text: errors.map(e => e.message).join(', ') }], isError: true };
const { accommodation, mirror } = await this.accommodations.createAccommodation(tripId, { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes });
this.guards.safeBroadcast(tripId, 'accommodation:created', { accommodation });
await this.accommodations.announceMirror(tripId, mirror, this.mirrorSender(tripId));
const writer = await this.guards.tripWriter(tripId, ctx.userId);
const { accommodation, mirror } = await this.accommodations.createStay(tripId, { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes }, writer);
return ok({ accommodation, assignment: mirror.created });
}
@@ -141,8 +130,9 @@ export class AccommodationsMcp {
// place as lodging, and place:created is the only announcement it gets here.
const place = (await result.mirror).stamped ?? result.place;
this.guards.safeBroadcast(tripId, 'place:created', { place });
this.guards.safeBroadcast(tripId, 'accommodation:created', { accommodation: result.accommodation });
await this.accommodations.announceMirror(tripId, { ...result.mirror, stamped: null }, this.mirrorSender(tripId));
// The same events a booking made on its own sends, the partner reservation's
// refetch ping among them; the stamp already went out as place:created.
await this.accommodations.announceStayCreated(tripId, { accommodation: result.accommodation, mirror: { ...result.mirror, stamped: null } }, await this.guards.tripWriter(tripId, ctx.userId));
return ok({ place, accommodation: result.accommodation, assignment: (await result.mirror).created });
} catch {
return { content: [{ type: 'text' as const, text: 'Failed to create place and accommodation.' }], isError: true };
@@ -174,13 +164,8 @@ export class AccommodationsMcp {
},
ctx: McpContext,
) {
if (!(await this.accommodations.verifyTripAccess(tripId, ctx.userId))) return noAccess();
if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied();
const existing = await this.accommodations.getAccommodation(accommodationId, tripId);
if (!existing) return { content: [{ type: 'text' as const, text: 'Accommodation not found.' }], isError: true };
const { accommodation, mirror } = await this.accommodations.updateAccommodation(accommodationId, existing, { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes });
this.guards.safeBroadcast(tripId, 'accommodation:updated', { accommodation });
await this.accommodations.announceMirror(tripId, mirror, this.mirrorSender(tripId));
const writer = await this.guards.tripWriter(tripId, ctx.userId);
const { accommodation, mirror } = await this.accommodations.updateStay(tripId, accommodationId, { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes }, writer);
// movedAssignment rather than a delete/create pair: an edit carries the booking's
// own stop across instead of rebuilding it, so the caller sees the same row.
return ok({ accommodation, assignment: mirror.created, movedAssignment: mirror.moved, removedAssignments: mirror.removed });
@@ -197,14 +182,10 @@ export class AccommodationsMcp {
access: { group: 'trips', mode: 'write' },
})
async deleteAccommodation({ tripId, accommodationId }: { tripId: number; accommodationId: number }, ctx: McpContext) {
if (!(await this.accommodations.verifyTripAccess(tripId, ctx.userId))) return noAccess();
if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied();
if (!(await this.accommodations.getAccommodation(accommodationId, tripId))) return { content: [{ type: 'text' as const, text: 'Accommodation not found.' }], isError: true };
const writer = await this.guards.tripWriter(tripId, ctx.userId);
// linkedReservationId stays the first one so the tool's answer keeps its shape;
// linkedReservationIds carries the rest for a block that had more than one booking.
const { linkedReservationId, linkedReservationIds, mirror } = await this.accommodations.deleteAccommodation(accommodationId);
await this.accommodations.announceMirror(tripId, mirror, this.mirrorSender(tripId));
this.guards.safeBroadcast(tripId, 'accommodation:deleted', { id: accommodationId, linkedReservationId, linkedReservationIds });
const { linkedReservationId, linkedReservationIds, mirror } = await this.accommodations.deleteStay(tripId, accommodationId, writer);
return ok({ success: true, linkedReservationId, linkedReservationIds, removedAssignments: mirror.removed });
}
@@ -26,6 +26,8 @@ import type { User } from '../../types';
import { RESERVATION_METADATA } from '../../db/json-columns';
import { decodeJson, encodeJson } from '../../utils/json-column';
import { TripAccessService } from '../trip-membership/trip-access.service';
import { DomainError } from '../common/domain-error';
import { requireTripWrite, restTripWriter, type TripWriter } from '../common/trip-writer';
type Trip = TripAccess;
@@ -73,6 +75,20 @@ export interface DayAccommodation {
notes: string | null;
}
/** The fields a stay write takes from its caller, before the required ones are checked. */
export interface StayInput {
place_id?: number;
start_day_id?: number;
end_day_id?: number;
check_in?: string | null;
check_in_end?: string | null;
check_out?: string | null;
confirmation?: string | null;
notes?: string | null;
}
const stayNotFound = () => new DomainError(404, 'Accommodation not found', { mcpMessage: 'Accommodation not found.' });
export interface CreateAccommodationData {
place_id: number;
start_day_id: number;
@@ -169,6 +185,92 @@ export class AccommodationsService {
return this.deleteAccommodation(id, opts);
}
// -------------------------------------------------------------------------
// Stay writes, one per use case
//
// Each holds the whole write for every surface: the trip gate, the reference
// checks, the transaction and every event the write sends. The REST route and
// the MCP tool are adapters over these, so a client sees the same events
// whichever of the two made the change. REST's events are the canonical ones;
// MCP used to send fewer (no reservation:created on a booking, no
// reservation:deleted/budget:deleted on its cascade).
// -------------------------------------------------------------------------
/** Every referenced place and day must exist on this trip (404). MCP lists every miss. */
private async requireStayRefs(tripId: string | number, placeId?: number, startDayId?: number, endDayId?: number): Promise<void> {
const errors = await this.validateAccommodationRefs(tripId, placeId, startDayId, endDayId);
if (errors.length > 0) {
throw new DomainError(404, errors[0].message, { mcpMessage: errors.map((e) => e.message).join(', ') });
}
}
/** The REST caller of a stay write: skips the sender's socket, as the route always did. */
restWriter(tripId: string, user: User, socketId: string | undefined): TripWriter {
return restTripWriter(this.realtime, tripId, user, socketId);
}
private gate(tripId: string | number, writer: TripWriter) {
// Coerced, as verifyTripAccess always did for this domain.
return requireTripWrite({ access: this.trips, permissions: this.permissions }, 'day_edit', Number(tripId), writer);
}
/** Book a night: the stay, its partner hotel reservation and the day stop it implies. */
async createStay(tripId: string | number, input: StayInput, writer: TripWriter) {
await this.gate(tripId, writer);
const { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes } = input;
if (!place_id || !start_day_id || !end_day_id) {
throw new DomainError(400, 'place_id, start_day_id, and end_day_id are required');
}
await this.requireStayRefs(tripId, place_id, start_day_id, end_day_id);
const result = await this.createAccommodation(tripId, { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes } as CreateAccommodationData);
await this.announceStayCreated(tripId, result, writer);
return result;
}
/**
* The events of a booked night: the stay, the partner reservation it brought (a
* refetch ping, the bookings view has no row to apply), then the day stop.
* Public for a surface that writes the stay inside a larger transaction.
*/
async announceStayCreated(tripId: string | number, result: { accommodation: unknown; mirror: AccommodationMirror }, writer: TripWriter) {
writer.events.emit('accommodation:created', { accommodation: result.accommodation } as TrekWsPayload<'accommodation:created'>);
writer.events.emit('reservation:created', {});
await this.announceMirror(tripId, result.mirror, (event, payload) => writer.events.emitAll(event, payload), writer.socketId);
}
/** Change a stay; moving it to another day or place carries its own stop along. */
async updateStay(tripId: string | number, id: string | number, fields: StayInput, writer: TripWriter) {
await this.gate(tripId, writer);
const existing = await this.getAccommodation(id, tripId);
if (!existing) throw stayNotFound();
const { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes } = fields;
await this.requireStayRefs(tripId, place_id, start_day_id, end_day_id);
const result = await this.updateAccommodation(id, existing as DayAccommodation, { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes } as Parameters<AccommodationsService['updateAccommodation']>[2]);
writer.events.emit('accommodation:updated', { accommodation: result.accommodation } as TrekWsPayload<'accommodation:updated'>);
await this.announceMirror(tripId, result.mirror, (event, payload) => writer.events.emitAll(event, payload), writer.socketId);
return result;
}
/** Cancel a stay, with the reservations and expenses that hang off it. */
async deleteStay(tripId: string | number, id: string | number, writer: TripWriter, opts: { keepStop?: boolean } = {}) {
await this.gate(tripId, writer);
if (!(await this.getAccommodation(id, tripId))) throw stayNotFound();
const result = await this.deleteAccommodation(id, opts);
await this.announceMirror(tripId, result.mirror, (event, payload) => writer.events.emitAll(event, payload), writer.socketId);
for (const reservationId of result.linkedReservationIds) writer.events.emit('reservation:deleted', { reservationId });
for (const itemId of result.deletedBudgetItemIds) writer.events.emit('budget:deleted', { itemId });
const accommodationId = Number(id);
// REST's payload is the canonical one. The MCP tool always named the stay `id`
// and listed the bookings it took along, so its event keeps those keys too.
writer.events.emit(
'accommodation:deleted',
writer.surface === 'mcp'
? ({ accommodationId, id: accommodationId, linkedReservationId: result.linkedReservationId, linkedReservationIds: result.linkedReservationIds } as TrekWsPayload<'accommodation:deleted'>)
: { accommodationId },
);
return result;
}
// -------------------------------------------------------------------------
// For a surface that writes the stay row itself
//
+16 -4
View File
@@ -18,32 +18,44 @@ import { HttpException } from '@nestjs/common';
* call with `errorResult(publicMessage)`.
*
* `code` names the refusal for logs and tests; it never reaches the body, so
* adding one changes no response.
* adding one changes no response. `mcpMessage` is for the few refusals whose
* MCP wording has always differed from the REST body (the trip gate's
* "Trip not found or access denied."); it names that drift in one place
* instead of in every tool.
*/
export class DomainError extends HttpException {
readonly code: string;
readonly publicMessage: string;
readonly details?: Readonly<Record<string, unknown>>;
readonly mcpMessage?: string;
constructor(
status: number,
publicMessage: string,
options: { code?: string; details?: Record<string, unknown> } = {},
options: { code?: string; details?: Record<string, unknown>; mcpMessage?: string } = {},
) {
super({ ...options.details, error: publicMessage }, status);
super(bodyOf(publicMessage, options.details), status);
this.name = 'DomainError';
this.message = publicMessage;
this.publicMessage = publicMessage;
this.code = options.code ?? defaultCode(status);
if (options.details) this.details = { ...options.details };
if (options.mcpMessage !== undefined) this.mcpMessage = options.mcpMessage;
}
/** The JSON body REST answers with: `{ error, ...details }`. */
toBody(): Record<string, unknown> {
return { ...this.details, error: this.publicMessage };
return bodyOf(this.publicMessage, this.details);
}
}
/** `error` first, as every hand-written body had it, and never overridden by a detail. */
function bodyOf(error: string, details: Readonly<Record<string, unknown>> | undefined): Record<string, unknown> {
const body: Record<string, unknown> = { error };
for (const [key, value] of Object.entries(details ?? {})) if (key !== 'error') body[key] = value;
return body;
}
const CODES: Record<number, string> = {
400: 'bad_request',
401: 'unauthorized',
+129
View File
@@ -0,0 +1,129 @@
import { DomainError } from './domain-error';
import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared';
/**
* How a trip write's realtime events leave on the surface that made it.
*
* A use case decides WHICH events a write sends and with WHAT payload, once for
* every surface. The sink decides how they travel: REST skips the socket that
* sent the request, MCP tags `_source: 'mcp'` and never lets a failed broadcast
* fail the tool.
*/
export interface TripEventSink {
/** An event the originating client already applied. `onlyUserIds` limits delivery to those users (a restricted item). */
emit<E extends TrekWsTripEventName>(
event: E,
payload: TrekWsPayload<E>,
onlyUserIds?: readonly number[] | null,
): void;
/** An event every socket needs, the sender's included (the day-plan side of a booking). */
emitAll<E extends TrekWsTripEventName>(event: E, payload: TrekWsPayload<E>): void;
}
/**
* Who makes a trip write, from where. `surface` exists for the few payloads and
* routings that have always differed between the two (each use is commented);
* everything else a use case does is the same for both.
*/
export interface TripWriter {
userId: number;
/** The global role, for the permission check (`admin` passes every action). */
role: string;
/** REST's X-Socket-Id, handed on to follow-up writes that broadcast themselves. */
socketId?: string;
surface: 'rest' | 'mcp';
events: TripEventSink;
}
/** The trip broadcast as RealtimeService implements it, without importing that domain. */
export type TripBroadcast = (
tripId: number | string,
event: string,
payload: Record<string, unknown>,
excludeSid?: number | string,
onlyUserId?: number,
) => void;
/** REST's sink: skips the sender's socket, the call shapes the controllers always used. */
export function restTripEvents(
broadcast: TripBroadcast,
tripId: number | string,
socketId: string | undefined,
): TripEventSink {
return {
emit(event, payload, onlyUserIds) {
if (onlyUserIds == null) {
broadcast(tripId, event, payload as Record<string, unknown>, socketId);
return;
}
for (const uid of new Set(onlyUserIds)) {
if (uid != null) broadcast(tripId, event, payload as Record<string, unknown>, socketId, uid);
}
},
emitAll(event, payload) {
broadcast(tripId, event, payload as Record<string, unknown>, undefined);
},
};
}
/**
* A REST caller as a trip use case takes it. `realtime` is RealtimeService; its
* overloads are typed per event, so it is taken here by its runtime shape and
* called with exactly the arguments each emit passes (test doubles record arity).
*/
export function restTripWriter(
realtime: { broadcast: unknown },
tripId: number | string,
user: { id: number; role: string },
socketId: string | undefined,
): TripWriter {
const broadcast = (realtime.broadcast as TripBroadcast).bind(realtime);
return {
userId: user.id,
role: user.role,
socketId,
surface: 'rest',
events: restTripEvents(broadcast, tripId, socketId),
};
}
/** The trip a user cannot reach. REST's guard answers it first; MCP keeps its own wording. */
export const tripNotFound = () =>
new DomainError(404, 'Trip not found', { mcpMessage: 'Trip not found or access denied.' });
/** The permission a user lacks on a trip they can see. */
export const tripPermissionDenied = () =>
new DomainError(403, 'No permission', {
mcpMessage: 'You do not have permission to perform this action on this trip.',
});
/** What the trip gate needs, structurally, so the shared kernel imports no domain. */
interface TripGate {
access: { findAccessible(tripId: number | string, userId: number): Promise<{ user_id: number } | undefined | null> };
permissions: {
checkPermission(
action: string,
role: string,
tripUserId: number | null,
userId: number,
isMember: boolean,
): Promise<boolean>;
};
}
/**
* The trip-scoped write gate every surface shares: the trip must be visible to
* the writer (404) and the writer must hold `action` on it (403). REST's
* TripAccessGuard runs the same two checks before the handler, so on that
* surface this never refuses; it is what MCP used to repeat in every tool.
* The id goes to the lookup as given: a domain that coerced it before still does.
*/
export async function requireTripWrite(gate: TripGate, action: string, tripId: number | string, writer: TripWriter) {
const trip = await gate.access.findAccessible(tripId, writer.userId);
if (!trip) throw tripNotFound();
const shared = trip.user_id !== writer.userId;
if (!(await gate.permissions.checkPermission(action, writer.role, trip.user_id, writer.userId, shared))) {
throw tripPermissionDenied();
}
return trip;
}
@@ -6,6 +6,7 @@ import { Users } from '../../db/entities/Users.entity';
import type { UsersRepository } from '../../db/repositories/Users.repository';
import { PermissionsService } from '../permissions/permissions.service';
import { RealtimeService } from '../realtime/realtime.service';
import type { TripEventSink, TripWriter } from '../common/trip-writer';
/**
* The impure MCP tool guards that used to live as plain functions in
@@ -84,6 +85,28 @@ export class McpToolGuardsService {
return this.permissions.checkPermission(action, role ?? 'user', tripOwnerId, userId, tripOwnerId !== userId);
}
/**
* The tool caller as a trip use case takes it: its role for the permission
* check, and safeBroadcast as the way the write's events leave, so they keep
* the MCP marker and a failed broadcast never fails the tool.
*/
async tripWriter(tripId: number, userId: number): Promise<TripWriter> {
const role = (await this.users.getRole(userId)) ?? 'user';
return { userId, role, surface: 'mcp', events: this.tripEvents(tripId) };
}
/** safeBroadcast in the shape of a TripEventSink. MCP skips no socket, so both doors are the same. */
tripEvents(tripId: number): TripEventSink {
return {
// The three-argument call when nothing scopes it, as every tool made it.
emit: (event, payload, onlyUserIds) =>
onlyUserIds === undefined
? this.safeBroadcast(tripId, event, payload as Record<string, unknown>)
: this.safeBroadcast(tripId, event, payload as Record<string, unknown>, onlyUserIds && [...onlyUserIds]),
emitAll: (event, payload) => this.safeBroadcast(tripId, event, payload as Record<string, unknown>),
};
}
/** True when the user has the global admin role (mirrors REST `user.role === 'admin'` gates). */
async isAdminUser(userId: number): Promise<boolean> {
return (await this.users.getRole(userId)) === 'admin';
@@ -0,0 +1,147 @@
import type { User } from '../../types';
import { isUpdateConflict } from '../common/conflictResult';
import { DomainError } from '../common/domain-error';
import { requireTripWrite, type TripWriter } from '../common/trip-writer';
import { PermissionsService } from '../permissions/permissions.service';
import { PackingService, isInvalidBagRef } from './packing.service';
import { Injectable } from '@nestjs/common';
import type { TrekWsPayload } from '@trek/shared';
/** The item fields a create takes, as PackingService.createItem reads them. */
export type PackingItemCreateInput = Parameters<PackingService['createItem']>[1];
/** The item fields an update takes, as PackingService.updateItem reads them. */
export type PackingItemUpdateInput = Parameters<PackingService['updateItem']>[2];
/** A packing item with the privacy fields that decide who an event reaches (#858). */
type PrivacyItem = { is_private?: number; owner_id?: number | null; recipients?: { user_id: number }[] };
const itemNotFound = () => new DomainError(404, 'Item not found', { mcpMessage: 'Packing item not found.' });
// The payload named a bag that is not on this trip (#2154): 400, as on create.
const bagNotFound = () => new DomainError(400, 'Bag not found', { mcpMessage: 'Bag not found.' });
/** Writes that can move a bag's weight, the only ones worth a bag-totals ping (#2191). */
const WEIGHT_KEYS = ['weight_grams', 'quantity', 'bag_id'];
/**
* The packing item writes, one method per use case, for every surface.
*
* Each holds the trip gate, the reference and conflict checks, the write and
* every event it sends; PackingController and PackingMcp are adapters over it.
* Before this each surface ran its own copy of the gate and the broadcasts, and
* they had drifted: the MCP update pinged bag totals on every write to a common
* item, which the REST route had stopped doing on purpose.
*
* One difference stays, on purpose, because changing it would change who sees
* what: an update to a restricted item reaches its owner only on REST
* (PackingService.broadcastUpdate), and its owner plus recipients on MCP. Which
* of the two is right is a product decision; until it is made, each surface
* keeps the routing it had.
*/
@Injectable()
export class PackingWritesService {
constructor(
private readonly packing: PackingService,
private readonly permissions: PermissionsService,
) {}
/**
* The REST caller of a packing write. Its events go through PackingService's own
* broadcast and broadcastToViewers, skipping the sender's socket, exactly the
* calls emitToViewers made for the routes.
*/
restWriter(tripId: string, user: User, socketId: string | undefined): TripWriter {
return {
userId: user.id,
role: user.role,
socketId,
surface: 'rest',
events: {
emit: (event, payload, onlyUserIds) =>
onlyUserIds == null
? this.packing.broadcast(tripId, event, payload, socketId)
: this.packing.broadcastToViewers(tripId, event, payload, [...onlyUserIds], socketId),
emitAll: (event, payload) => this.packing.broadcast(tripId, event, payload, undefined),
},
};
}
private gate(tripId: string | number, writer: TripWriter) {
return requireTripWrite(
{
access: { findAccessible: (id, userId) => this.packing.verifyTripAccess(id, userId) },
permissions: this.permissions,
},
'packing_edit',
tripId,
writer,
);
}
async createItem(tripId: string | number, input: PackingItemCreateInput, writer: TripWriter) {
await this.gate(tripId, writer);
const item = await this.packing.createItem(tripId, input, writer.userId);
if (isInvalidBagRef(item)) throw bagNotFound();
// A restricted item reaches its owner and recipients only; a common one the room.
writer.events.emit('packing:created', { item } as TrekWsPayload<'packing:created'>, this.packing.viewersOf(item));
this.packing.broadcastBagTotals(String(tripId));
return item;
}
/**
* Change an item. `bodyKeys` names the fields the caller sent (an explicit null
* clears one); `ifMatch` is REST's X-Base-Updated-At for a stale offline write.
*/
async updateItem(
tripId: string | number,
itemId: number,
input: PackingItemUpdateInput,
bodyKeys: string[],
writer: TripWriter,
ifMatch?: string,
) {
await this.gate(tripId, writer);
// Read before the write: getting it wrong leaks a freshly privatized item.
const wasPrivate = !!(await this.packing.getItemPrivacy(tripId, itemId))?.is_private;
const updated = await this.packing.updateItem(tripId, itemId, input, bodyKeys, ifMatch, writer.userId);
if (!updated) throw itemNotFound();
// A stale offline overwrite: the client resolves it against the server row (#1135).
if (isUpdateConflict(updated)) throw new DomainError(409, 'conflict', { details: { server: updated.server } });
if (isInvalidBagRef(updated)) throw bagNotFound();
const item = updated as PrivacyItem;
if (writer.surface === 'rest')
this.packing.broadcastUpdate(String(tripId), itemId, item, wasPrivate, writer.socketId);
else this.announceUpdateToViewers(itemId, item, wasPrivate, writer);
// Checking an item off is the most frequent packing write there is, and every
// ping costs every connected client a listBags round trip.
if (WEIGHT_KEYS.some((k) => bodyKeys.includes(k))) this.packing.broadcastBagTotals(String(tripId));
return updated;
}
/**
* The four privacy transitions of an update (#858) as the MCP tools always sent
* them: a restricted item goes to its owner and recipients.
*/
private announceUpdateToViewers(itemId: number, item: PrivacyItem, wasPrivate: boolean, writer: TripWriter) {
const viewers = this.packing.viewersOf(item);
const payload = { item } as TrekWsPayload<'packing:updated'>;
if (item.is_private) {
// Newly restricted: off the room's screens first, then back to who may see it.
if (!wasPrivate) writer.events.emit('packing:deleted', { itemId });
writer.events.emit(wasPrivate ? 'packing:updated' : 'packing:created', payload, viewers);
return;
}
// Newly common: the members who never had the row need it created, not updated.
if (wasPrivate) writer.events.emit('packing:created', payload);
writer.events.emit('packing:updated', payload);
}
async deleteItem(tripId: string | number, itemId: number, writer: TripWriter) {
await this.gate(tripId, writer);
const deleted = await this.packing.deleteItem(tripId, itemId, writer.userId);
if (!deleted) throw itemNotFound();
// Scoped to the people who could see it (owner + recipients, #858, #1976).
writer.events.emit('packing:deleted', { itemId }, this.packing.viewersOf(deleted));
this.packing.broadcastBagTotals(String(tripId));
return deleted;
}
}
+12 -43
View File
@@ -13,8 +13,8 @@ import {
} from '@nestjs/common';
import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared';
import type { User } from '../../types';
import { PackingService, isInvalidBagRef } from './packing.service';
import { isUpdateConflict } from '../common/conflictResult';
import { PackingService } from './packing.service';
import { PackingWritesService } from './packing-writes.service';
import { JwtAuthGuard } from '../auth-core/jwt-auth.guard';
import { CurrentUser } from '../auth-core/current-user.decorator';
import { RequirePermission, TripAccessGuard } from '../permissions/trip-access.guard';
@@ -59,7 +59,10 @@ type PackingItemRow = { is_private?: number; owner_id?: number | null; recipient
// passes, so the HTTP and MCP paths cannot demand different rights.
@UseGuards(JwtAuthGuard, TripAccessGuard)
export class PackingController {
constructor(private readonly packing: PackingService) {}
constructor(
private readonly packing: PackingService,
private readonly writes: PackingWritesService,
) {}
/** Loads the trip or throws the legacy 404; returns it for the permission check. */
@@ -99,14 +102,9 @@ export class PackingController {
@Headers('x-socket-id') socketId?: string,
) {
// checked arrives as boolean or legacy 0/1 — the service coerces by truthiness.
const item = await this.packing.createItem(tripId, { name: body.name, category: body.category, checked: body.checked === undefined ? undefined : !!body.checked, weight_grams: body.weight_grams, bag_id: body.bag_id, quantity: body.quantity, is_private: body.is_private, visibility: body.visibility, recipient_ids: body.recipient_ids }, user.id);
// A bag referenced in the body must exist on this trip (#2154). The payload
// is at fault, so 400 — the 404 'Bag not found' stays with the path routes.
if (isInvalidBagRef(item)) {
throw new HttpException({ error: 'Bag not found' }, 400);
}
this.packing.emitToViewers(tripId, 'packing:created', { item }, item, socketId);
this.packing.broadcastBagTotals(tripId);
// A bag the body names must exist on this trip (#2154); the use case answers
// that 400, the 404 'Bag not found' stays with the path routes.
const item = await this.writes.createItem(tripId, { name: body.name, category: body.category, checked: body.checked === undefined ? undefined : !!body.checked, weight_grams: body.weight_grams, bag_id: body.bag_id, quantity: body.quantity, is_private: body.is_private, visibility: body.visibility, recipient_ids: body.recipient_ids }, this.writes.restWriter(tripId, user, socketId));
return { item };
}
@@ -142,32 +140,12 @@ export class PackingController {
if (itemId === null) {
throw new HttpException({ error: 'Item not found' }, 404);
}
// Privacy state before the change, so a public↔private toggle (#858) can route
// the broadcast correctly instead of leaking a freshly-privatized item.
const before = await this.packing.getItemPrivacy(tripId, itemId);
const { name, checked, category, weight_grams, bag_id, quantity, packed_quantity, is_private } = body;
// bodyKeys carries which keys the request actually provided (the presence-
// sentinel protocol); the parsed body only ever holds known schema keys.
// checked arrives as boolean or legacy 0/1 — normalize to the 0/1 the SQL binds.
const updated = await this.packing.updateItem(tripId, itemId, { name, checked: checked === undefined ? undefined : checked ? 1 : 0, category, weight_grams, bag_id, quantity, packed_quantity, is_private }, Object.keys(body), ifMatch, user.id);
if (!updated) {
throw new HttpException({ error: 'Item not found' }, 404);
}
// Stale offline overwrite — surface the conflict for client-side resolution (#1135).
if (isUpdateConflict(updated)) {
throw new HttpException({ error: 'conflict', server: updated.server }, 409);
}
// A bag referenced in the body must exist on this trip (#2154) — see create.
if (isInvalidBagRef(updated)) {
throw new HttpException({ error: 'Bag not found' }, 400);
}
this.packing.broadcastUpdate(tripId, itemId, updated as PackingItemRow, !!before?.is_private, socketId);
// Only when the write could actually move a weight. Checking an item off is
// the most frequent packing write there is, and every ping costs every
// connected client a listBags round trip.
if (['weight_grams', 'quantity', 'bag_id'].some(k => Object.keys(body).includes(k))) {
this.packing.broadcastBagTotals(tripId);
}
// The use case answers 404, the 409 conflict (#1135) and the 400 bag (#2154).
const updated = await this.writes.updateItem(tripId, itemId, { name, checked: checked === undefined ? undefined : checked ? 1 : 0, category, weight_grams, bag_id, quantity, packed_quantity, is_private }, Object.keys(body), this.writes.restWriter(tripId, user, socketId), ifMatch);
return { item: updated };
}
@@ -184,16 +162,7 @@ export class PackingController {
if (itemId === null) {
throw new HttpException({ error: 'Item not found' }, 404);
}
const deleted = await this.packing.deleteItem(tripId, itemId, user.id);
if (!deleted) {
throw new HttpException({ error: 'Item not found' }, 404);
}
// Scope the delete to the people who could see it (owner + recipients, #858).
// `deleted` is already a concretely-typed `PackingItemRow` (Plan 3e Task 3's
// `PackingItemsRepository`, via `PackingService.deleteItem`'s return type) —
// no cast needed, unlike `updated` above (that one's inferred as `unknown`).
this.packing.emitToViewers(tripId, 'packing:deleted', { itemId }, deleted, socketId);
this.packing.broadcastBagTotals(tripId);
await this.writes.deleteItem(tripId, itemId, this.writes.restWriter(tripId, user, socketId));
return { success: true };
}
+10 -57
View File
@@ -8,7 +8,8 @@ import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service';
import { z } from 'zod';
import { ADDON_IDS } from '../../addons';
import { noAccess, permissionDenied, adminRequired } from '../../mcp/tools/_shared';
import { PackingService, isInvalidBagRef } from './packing.service';
import { PackingService } from './packing.service';
import { PackingWritesService } from './packing-writes.service';
import {
packingCreateBagRequestSchema,
packingCreateItemRequestSchema,
@@ -50,6 +51,7 @@ export class PackingMcp {
private readonly packing: PackingService,
readonly addons: AddonsService,
private readonly guards: McpToolGuardsService,
private readonly writes: PackingWritesService,
) {}
// --- PACKING ---
@@ -77,9 +79,7 @@ export class PackingMcp {
{ tripId, name, category, bag_id, quantity, weight_grams, checked, is_private, visibility, recipient_ids }: { tripId: number; name: string; category?: string; bag_id?: number | null; quantity?: number; weight_grams?: number | null; checked?: boolean | number; is_private?: boolean; visibility?: PackingVisibility; recipient_ids?: number[] },
ctx: McpContext,
) {
if (!(await this.packing.verifyTripAccess(tripId, ctx.userId))) return noAccess();
if (!(await this.guards.hasTripPermission('packing_edit', tripId, ctx.userId))) return permissionDenied();
const item = await this.packing.createItem(tripId, {
const item = await this.writes.createItem(tripId, {
name,
category: category || 'General',
bag_id,
@@ -90,13 +90,7 @@ export class PackingMcp {
is_private,
visibility,
recipient_ids,
}, ctx.userId);
// A referenced bag must exist on this trip (#2154), as on the REST route.
if (isInvalidBagRef(item)) return errorResult('Bag not found.');
// A restricted item (#858) reaches its owner and recipients only; a Common
// one answers null here and goes to the whole room.
this.guards.safeBroadcast(tripId, 'packing:created', { item }, this.packing.viewersOf(item));
this.packing.broadcastBagTotals(String(tripId));
}, await this.guards.tripWriter(tripId, ctx.userId));
return ok({ item });
}
@@ -113,13 +107,9 @@ export class PackingMcp {
access: { group: 'packing', mode: 'write' },
})
async togglePackingItem({ tripId, itemId, checked }: { tripId: number; itemId: number; checked: boolean }, ctx: McpContext) {
if (!(await this.packing.verifyTripAccess(tripId, ctx.userId))) return noAccess();
if (!(await this.guards.hasTripPermission('packing_edit', tripId, ctx.userId))) return permissionDenied();
const item = await this.packing.updateItem(tripId, itemId, { checked: checked ? 1 : 0 }, ['checked'], undefined, ctx.userId);
if (!item) return errorResult('Packing item not found.');
// Scoped to the people who may see it, exactly as the REST route does
// (#858, #1976). A Common item answers null here and goes to the room.
this.guards.safeBroadcast(tripId, 'packing:updated', { item }, this.packing.viewersOf(item));
// The update use case: a toggle never changes the privacy, so this is its
// plain update to whoever may see the item (#858, #1976).
const item = await this.writes.updateItem(tripId, itemId, { checked: checked ? 1 : 0 }, ['checked'], await this.guards.tripWriter(tripId, ctx.userId));
return ok({ item });
}
@@ -135,14 +125,7 @@ export class PackingMcp {
access: { group: 'packing', mode: 'write' },
})
async deletePackingItem({ tripId, itemId }: { tripId: number; itemId: number }, ctx: McpContext) {
if (!(await this.packing.verifyTripAccess(tripId, ctx.userId))) return noAccess();
if (!(await this.guards.hasTripPermission('packing_edit', tripId, ctx.userId))) return permissionDenied();
const deleted = await this.packing.deleteItem(tripId, itemId, ctx.userId);
if (!deleted) return errorResult('Packing item not found.');
// deleteItem hands back the row it removed, so the delete can be scoped to
// the same people the item was ever visible to (#1976).
this.guards.safeBroadcast(tripId, 'packing:deleted', { itemId }, this.packing.viewersOf(deleted));
this.packing.broadcastBagTotals(String(tripId));
await this.writes.deleteItem(tripId, itemId, await this.guards.tripWriter(tripId, ctx.userId));
return ok({ success: true });
}
@@ -170,44 +153,14 @@ export class PackingMcp {
{ tripId, itemId, name, category, bag_id, quantity, packed_quantity, weight_grams, is_private }: { tripId: number; itemId: number; name?: string; category?: string; bag_id?: number | null; quantity?: number; packed_quantity?: number | null; weight_grams?: number | null; is_private?: boolean },
ctx: McpContext,
) {
if (!(await this.packing.verifyTripAccess(tripId, ctx.userId))) return noAccess();
if (!(await this.guards.hasTripPermission('packing_edit', tripId, ctx.userId))) return permissionDenied();
const fields = { name, category, bag_id, quantity, packed_quantity, weight_grams, is_private };
// The service reads presence from bodyKeys, so a field has to be named there
// for an explicit null to clear it rather than read as "leave it alone".
const bodyKeys = Object.keys(fields).filter(k => fields[k as keyof typeof fields] !== undefined);
// Privacy state before the write, so a public↔private flip routes the
// broadcast the way the REST route does instead of leaking a freshly
// privatized item (or leaving a stale copy on everyone else's screen).
const wasPrivate = !!(await this.packing.getItemPrivacy(tripId, itemId))?.is_private;
const item = await this.packing.updateItem(tripId, itemId, fields, bodyKeys, undefined, ctx.userId);
if (!item) return errorResult('Packing item not found.');
// A referenced bag must exist on this trip (#2154), as on the REST route.
if (isInvalidBagRef(item)) return errorResult('Bag not found.');
this.broadcastItemUpdate(tripId, itemId, item, wasPrivate);
const item = await this.writes.updateItem(tripId, itemId, fields, bodyKeys, await this.guards.tripWriter(tripId, ctx.userId));
return ok({ item });
}
/**
* The four privacy transitions of an item update (#858), as
* PackingService.broadcastUpdate does them for REST, but over safeBroadcast so
* the events keep the MCP marker and the tool survives a broadcast failure.
*/
private broadcastItemUpdate(tripId: number, itemId: number, item: { is_private?: number; owner_id?: number | null; recipients?: { user_id: number }[] }, wasPrivate: boolean) {
const viewers = this.packing.viewersOf(item);
if (item.is_private) {
// Newly restricted: take it off the room's screens first, then hand it
// back to the people who may still see it.
if (!wasPrivate) this.guards.safeBroadcast(tripId, 'packing:deleted', { itemId });
this.guards.safeBroadcast(tripId, wasPrivate ? 'packing:updated' : 'packing:created', { item }, viewers);
return;
}
// Newly common: the members who never had the row need it created, not updated.
if (wasPrivate) this.guards.safeBroadcast(tripId, 'packing:created', { item });
this.packing.broadcastBagTotals(String(tripId));
this.guards.safeBroadcast(tripId, 'packing:updated', { item });
}
@Tool({
name: 'set_packing_item_sharing',
description: 'Move an existing packing item between the three sharing tiers: the common list the whole trip pools into, the owner\'s own list, or shared with named trip members. Only the item\'s owner may change this. Everything else about an item is update_packing_item.',
+2 -1
View File
@@ -4,6 +4,7 @@ import { PackingController } from './packing.controller';
import { AdminPackingTemplatesController } from './admin-packing-templates.controller';
import { PackingMcp } from './packing.mcp';
import { PackingService } from './packing.service';
import { PackingWritesService } from './packing-writes.service';
import { PackingRpc } from './packing.rpc';
import { PluginGuardsModule } from '../../nest-rpc/plugin-guards.module';
import { RealtimeModule } from '../realtime/realtime.module';
@@ -34,7 +35,7 @@ import { TripMembers } from '../../db/entities/TripMembers.entity';
MikroOrmModule.forFeature([PackingItems, PackingItemContributors, PackingBags, PackingCategoryAssignees, PackingTemplates, PackingTemplateCategories, PackingTemplateItems, Trips, TripMembers]),
McpSharedModule, NotificationsModule, PermissionsModule, RealtimeModule, PluginGuardsModule, AddonsModule, AuditModule],
controllers: [PackingController, AdminPackingTemplatesController],
providers: [PackingService, PackingMcp, PackingRpc],
providers: [PackingService, PackingWritesService, PackingMcp, PackingRpc],
exports: [PackingService],
})
export class PackingModule {}