diff --git a/client/src/api/wsEventPolicy.ts b/client/src/api/wsEventPolicy.ts index 9a9b3762b..46c22033a 100644 --- a/client/src/api/wsEventPolicy.ts +++ b/client/src/api/wsEventPolicy.ts @@ -30,6 +30,11 @@ export const HANDLED_OUTSIDE_TRIP_STORE = [ 'collab:message:created', 'collab:message:reacted', 'collab:message:deleted', + // Road trip shaping — components/Roadtrip/useRoadtripVias. The points a drive is + // routed through live in that hook and nowhere else; a store slice for them would be a + // second copy of the same list to keep in step. + 'roadtripVia:changed', + 'roadtripTrack:changed', // In-app notifications — hooks/useInAppNotificationListener 'notification:new', 'notification:updated', diff --git a/server/src/nest/roadtrip/roadtrip.controller.ts b/server/src/nest/roadtrip/roadtrip.controller.ts index 6bfd54994..7efcb7042 100644 --- a/server/src/nest/roadtrip/roadtrip.controller.ts +++ b/server/src/nest/roadtrip/roadtrip.controller.ts @@ -1,4 +1,4 @@ -import { Body, Controller, Delete, Get, HttpCode, HttpException, Param, Post, Put, UseGuards } from '@nestjs/common'; +import { Body, Controller, Delete, Get, Headers, HttpCode, HttpException, Param, Post, Put, UseGuards } from '@nestjs/common'; import type { RoadtripDayTrack, RoadtripVia } from '@trek/shared'; import { RoadtripService } from './roadtrip.service'; import { RoadtripViaBatchDto, RoadtripViaCreateDto, RoadtripViaReanchorDto, RoadtripViaUpdateDto } from './roadtrip.dto'; @@ -14,11 +14,12 @@ import { ADDON_IDS } from '../../addons'; * Gated on the road trip addon: an instance with it switched off gets 404s here, the same * as every other addon surface, and nothing about the plain day plan changes. * - * No WebSocket broadcast, unlike the assignment routes. A via is a detail of how one - * person's route is drawn rather than a change to the itinerary everybody reads, and - * adding an event family for it would mean extending the typed trip-event catalogue for - * something a collaborator sees on their next load anyway. Worth revisiting if vias ever - * become shared editing rather than route shaping. + * Every write here broadcasts, like the assignment routes. This used to be silent, on + * the reasoning that a via is how one person draws their route rather than a change to + * the itinerary everybody reads. That does not survive contact with two people planning + * one road trip: they look at the same line on the same map, and a reshaped drive moves + * every arrival time after it. Waiting for the other side's next reload is not "eventual + * consistency", it is two people editing a route neither can see the other change. */ @Controller('api/trips/:tripId/roadtrip') // AddonGuard FIRST: @RequireAddon is metadata and does nothing on its own, and the guard @@ -48,9 +49,12 @@ export class RoadtripController { @Param('tripId') tripId: string, @Param('dayId') dayId: string, @Body() body: RoadtripViaCreateDto, + @Headers('x-socket-id') socketId?: string, ): { via: RoadtripVia } { this.requireDay(dayId, tripId); - return { via: this.roadtrip.create(dayId, body) }; + const via = this.roadtrip.create(dayId, body); + this.announce(tripId, dayId, socketId); + return { via }; } /** @@ -68,6 +72,7 @@ export class RoadtripController { @Param('tripId') tripId: string, @Param('dayId') dayId: string, @Body() body: RoadtripViaBatchDto, + @Headers('x-socket-id') socketId?: string, ): { vias: RoadtripVia[] } { this.requireDay(dayId, tripId); // Permission is not enough on its own: a place id from somebody else's trip would @@ -76,7 +81,15 @@ export class RoadtripController { if (body.track && !this.roadtrip.trackExists(body.track.place_id, tripId)) { throw new HttpException({ error: 'Track not found' }, 404); } - return { vias: this.roadtrip.createMany(dayId, body) }; + const vias = this.roadtrip.createMany(dayId, body); + this.announce(tripId, dayId, socketId); + // A batch is how a day starts following a recorded track, so the track it now follows + // is news in its own right: the rail draws a badge for it and the map a line. + this.roadtrip.broadcast(tripId, 'roadtripTrack:changed', { + dayId, + track: this.roadtrip.tracksForTrip(tripId).find(t => String(t.day_id) === String(dayId)) ?? null, + }, socketId); + return { vias }; } /** @@ -93,9 +106,12 @@ export class RoadtripController { @Param('tripId') tripId: string, @Param('dayId') dayId: string, @Body() body: RoadtripViaReanchorDto, + @Headers('x-socket-id') socketId?: string, ): { vias: RoadtripVia[] } { this.requireDay(dayId, tripId); - return { vias: this.roadtrip.reanchor(dayId, body) }; + const vias = this.roadtrip.reanchor(dayId, body); + this.announce(tripId, dayId, socketId); + return { vias }; } @RequirePermission('day_edit') @@ -106,10 +122,12 @@ export class RoadtripController { @Param('dayId') dayId: string, @Param('id') id: string, @Body() body: RoadtripViaUpdateDto, + @Headers('x-socket-id') socketId?: string, ): { via: RoadtripVia } { this.requireDay(dayId, tripId); const via = this.roadtrip.move(id, dayId, body.lat, body.lng, body.after_order_index); if (!via) throw new HttpException({ error: 'Via not found' }, 404); + this.announce(tripId, dayId, socketId); return { via }; } @@ -119,14 +137,31 @@ export class RoadtripController { @Param('tripId') tripId: string, @Param('dayId') dayId: string, @Param('id') id: string, + @Headers('x-socket-id') socketId?: string, ): { success: true } { this.requireDay(dayId, tripId); if (!this.roadtrip.remove(id, dayId)) { throw new HttpException({ error: 'Via not found' }, 404); } + this.announce(tripId, dayId, socketId); return { success: true }; } + /** + * Says what this day's drive is routed through now. + * + * Read back rather than assembled from what the write returned: a reanchor rewrites the + * whole set, a batch may clear legs before filling them, and one shape for all five + * routes means a client applies them all the same way. The originating socket is + * excluded, so the person dragging does not get their own point handed back mid-drag. + */ + private announce(tripId: string, dayId: string, socketId: string | undefined): void { + this.roadtrip.broadcast(tripId, 'roadtripVia:changed', { + dayId, + vias: this.roadtrip.listForDay(dayId), + }, socketId); + } + /** * The guard proves the caller may reach the trip; this proves the day is part of it. * Without it a valid day id from someone else's trip would be editable through a trip diff --git a/server/src/nest/roadtrip/roadtrip.mcp.ts b/server/src/nest/roadtrip/roadtrip.mcp.ts index 2778d27d3..c7b08e084 100644 --- a/server/src/nest/roadtrip/roadtrip.mcp.ts +++ b/server/src/nest/roadtrip/roadtrip.mcp.ts @@ -77,7 +77,9 @@ export class RoadtripMcp { if (!this.db.canAccessTrip(tripId, ctx.userId)) return noAccess(); if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); if (!this.roadtrip.dayExists(dayId, tripId)) return noAccess(); - return ok({ via: this.roadtrip.create(dayId, { after_order_index, lat, lng }) }); + const via = this.roadtrip.create(dayId, { after_order_index, lat, lng }); + this.announce(tripId, dayId); + return ok({ via }); } @Tool({ @@ -121,7 +123,13 @@ export class RoadtripMcp { // a place id from another trip, or a place that is not a track at all, become this // day's label. if (track && !this.roadtrip.trackExists(track.place_id, tripId)) return noAccess(); - return ok({ vias: this.roadtrip.createMany(dayId, { vias, replace_legs, track }) }); + const made = this.roadtrip.createMany(dayId, { vias, replace_legs, track }); + this.announce(tripId, dayId); + this.roadtrip.broadcast(String(tripId), 'roadtripTrack:changed', { + dayId, + track: this.roadtrip.tracksForTrip(String(tripId)).find(t => String(t.day_id) === String(dayId)) ?? null, + }, undefined); + return ok({ vias: made }); } @Tool({ @@ -149,7 +157,9 @@ export class RoadtripMcp { if (!this.db.canAccessTrip(tripId, ctx.userId)) return noAccess(); if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); if (!this.roadtrip.dayExists(dayId, tripId)) return noAccess(); - return ok({ vias: this.roadtrip.reanchor(dayId, { vias, remove }) }); + const next = this.roadtrip.reanchor(dayId, { vias, remove }); + this.announce(tripId, dayId); + return ok({ vias: next }); } @Tool({ @@ -170,6 +180,22 @@ export class RoadtripMcp { if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); if (!this.roadtrip.dayExists(dayId, tripId)) return noAccess(); if (!this.roadtrip.remove(viaId, dayId)) return noAccess(); + this.announce(tripId, dayId); return ok({ success: true }); } + + /** + * Says what this day's drive is routed through now. + * + * The same announcement the REST routes make, because it is the same change: a via laid + * by an assistant moves the line on everybody's map exactly as one dragged by hand. + * No originating socket to exclude here — a tool call has no socket of its own, so the + * client that asked for it hears about it like everyone else. + */ + private announce(tripId: number, dayId: number): void { + this.roadtrip.broadcast(String(tripId), 'roadtripVia:changed', { + dayId, + vias: this.roadtrip.listForDay(String(dayId)), + }, undefined); + } } diff --git a/server/src/nest/roadtrip/roadtrip.module.ts b/server/src/nest/roadtrip/roadtrip.module.ts index 65723a76c..5cdfeecbd 100644 --- a/server/src/nest/roadtrip/roadtrip.module.ts +++ b/server/src/nest/roadtrip/roadtrip.module.ts @@ -5,13 +5,14 @@ import { RoadtripMcp } from './roadtrip.mcp'; import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; import { PermissionsModule } from '../permissions/permissions.module'; import { AuthModule } from '../auth/auth.module'; +import { RealtimeModule } from '../realtime/realtime.module'; import { AddonsModule } from '../addons/addons.module'; /** Road trip domain (#1797): the points a drive is routed through. Registered in AppModule. */ @Module({ // McpShared brings the tool guards, Auth the demo check, Permissions the trip guard, // Addons the enabled-check the MCP tools gate on (the controller has @RequireAddon). - imports: [McpSharedModule, PermissionsModule, AuthModule, AddonsModule], + imports: [McpSharedModule, PermissionsModule, AuthModule, AddonsModule, RealtimeModule], controllers: [RoadtripController], providers: [RoadtripService, RoadtripMcp], exports: [RoadtripService], diff --git a/server/src/nest/roadtrip/roadtrip.service.ts b/server/src/nest/roadtrip/roadtrip.service.ts index cd5ae43f1..44da412b5 100644 --- a/server/src/nest/roadtrip/roadtrip.service.ts +++ b/server/src/nest/roadtrip/roadtrip.service.ts @@ -1,6 +1,7 @@ import { Injectable } from '@nestjs/common'; -import type { RoadtripDayTrack, RoadtripVia } from '@trek/shared'; +import type { RoadtripDayTrack, RoadtripVia, TrekWsPayload, TrekWsTripEventName } from '@trek/shared'; import { DatabaseService } from '../database/database.service'; +import { RealtimeService } from '../realtime/realtime.service'; /** * Via points: the places a day's drive is made to pass through without stopping. @@ -12,7 +13,25 @@ import { DatabaseService } from '../database/database.service'; */ @Injectable() export class RoadtripService { - constructor(private readonly db: DatabaseService) {} + constructor( + private readonly db: DatabaseService, + private readonly realtime: RealtimeService, + ) {} + + /** + * Tells the trip's other clients what the drive looks like now. + * + * Two people planning a road trip look at the same line on the same map, and a reshaped + * drive moves every arrival time after it — so this is not "how one person draws their + * route", which is what it was taken for when these routes were written silent. + * + * The whole day's list goes out rather than the one point that changed: a via carries no + * identity anybody reads, the client holds them per day, and a drag is a burst of writes + * whose only interesting state is the one that lands last. + */ + broadcast(tripId: string, event: E, payload: TrekWsPayload, socketId: string | undefined): void { + this.realtime.broadcast(tripId, event, payload, socketId); + } /** The day exists and belongs to this trip. 404 material, checked before every write. */ dayExists(dayId: string | number, tripId: string | number): boolean { diff --git a/server/tests/unit/nest/roadtrip.controller.test.ts b/server/tests/unit/nest/roadtrip.controller.test.ts index 22b3cf385..b360b9266 100644 --- a/server/tests/unit/nest/roadtrip.controller.test.ts +++ b/server/tests/unit/nest/roadtrip.controller.test.ts @@ -37,6 +37,8 @@ function svc(o: Partial = {}): RoadtripService { reanchor: vi.fn().mockReturnValue([VIA]), move: vi.fn().mockReturnValue(VIA), remove: vi.fn().mockReturnValue(true), + // Every write announces the day's new shape to the trip's other clients. + broadcast: vi.fn(), ...o, } as unknown as RoadtripService; } @@ -188,4 +190,52 @@ describe('RoadtripController — writes', () => { expect(thrown(() => new RoadtripController(s).remove('7', '4', '999'))) .toEqual({ status: 404, body: { error: 'Via not found' } }); }); + + /** + * Two people planning one road trip look at the same line on the same map, and a + * reshaped drive moves every arrival time after it. These routes used to write in + * silence, which left the other side reading a route nobody could see change. + */ + describe('telling the trip', () => { + it('SRV-ROADTRIP-020: every write announces the new shape of the day', () => { + const cases: [string, (c: RoadtripController) => unknown][] = [ + ['create', c => c.create('7', '4', { after_order_index: 0, lat: 1, lng: 2 }, 'sock')], + ['createMany', c => c.createMany('7', '4', { vias: [{ after_order_index: 0, lat: 1, lng: 2 }] }, 'sock')], + ['reanchor', c => c.reanchor('7', '4', { vias: [] }, 'sock')], + ['update', c => c.update('7', '4', '9', { lat: 1, lng: 2 }, 'sock')], + ['remove', c => c.remove('7', '4', '9', 'sock')], + ]; + for (const [name, run] of cases) { + const s = svc(); + run(new RoadtripController(s)); + expect(s.broadcast, name).toHaveBeenCalledWith( + '7', + 'roadtripVia:changed', + { dayId: '4', vias: [VIA] }, + 'sock', + ); + } + }); + + it('SRV-ROADTRIP-021: the day list is read back, not assembled from the write', () => { + // A reanchor rewrites the whole set and a batch may clear legs before filling them, + // so what the write returned is not what the day now holds. + const s = svc({ reanchor: vi.fn().mockReturnValue([]) } as Partial); + new RoadtripController(s).reanchor('7', '4', { vias: [] }, 'sock'); + expect(s.broadcast).toHaveBeenCalledWith('7', 'roadtripVia:changed', { dayId: '4', vias: [VIA] }, 'sock'); + }); + + it('SRV-ROADTRIP-022: laying a track down says which track the day now follows', () => { + const track = { day_id: 4, place_id: 3, name: 'B96' }; + const s = svc({ tracksForTrip: vi.fn().mockReturnValue([track]) } as Partial); + new RoadtripController(s).createMany('7', '4', { vias: [], track: { place_id: 3 } }, 'sock'); + expect(s.broadcast).toHaveBeenCalledWith('7', 'roadtripTrack:changed', { dayId: '4', track }, 'sock'); + }); + + it('SRV-ROADTRIP-023: a refused write announces nothing', () => { + const s = svc({ remove: vi.fn().mockReturnValue(false) } as Partial); + expect(() => new RoadtripController(s).remove('7', '4', '9', 'sock')).toThrow(); + expect(s.broadcast).not.toHaveBeenCalled(); + }); + }); }); diff --git a/server/tests/unit/nest/roadtrip.service.test.ts b/server/tests/unit/nest/roadtrip.service.test.ts index e047f7c93..833cd644a 100644 --- a/server/tests/unit/nest/roadtrip.service.test.ts +++ b/server/tests/unit/nest/roadtrip.service.test.ts @@ -43,7 +43,9 @@ function makeService() { run: (sql: string, ...params: unknown[]) => raw.prepare(sql).run(...params as never[]), transaction: (fn: (conn: unknown) => T) => raw.transaction(() => fn(raw))(), }; - return new RoadtripService(db as never); + // The realtime side is exercised through the controller and the MCP layer; here the + // service is under test for what it writes, so a broadcast that goes nowhere is right. + return new RoadtripService(db as never, { broadcast: () => {} } as never); } describe('RoadtripService', () => { diff --git a/shared/src/realtime/events.schema.spec.ts b/shared/src/realtime/events.schema.spec.ts index 826c5373b..378ef549b 100644 --- a/shared/src/realtime/events.schema.spec.ts +++ b/shared/src/realtime/events.schema.spec.ts @@ -28,6 +28,8 @@ const FIXTURES: Record> = { 'day:updated': { day: { id: 2, date: '2026-06-11' } }, 'day:deleted': { dayId: 2 }, 'day:reordered': { orderedIds: [2, 1] }, + 'roadtripVia:changed': { dayId: 2, vias: [{ id: 9, lat: 53.5, lng: 9.9 }] }, + 'roadtripTrack:changed': { dayId: 2, track: null }, 'dayNote:created': { dayId: 2, note: { id: 5 } }, 'dayNote:updated': { dayId: 2, note: { id: 5 } }, 'dayNote:deleted': { noteId: 5, dayId: 2 }, @@ -138,11 +140,12 @@ const DRIFT_VARIANTS: Partial[]> }; describe('@trek/shared realtime event registry', () => { - it('WSEVT-REG-001: pins the authoritative inventory counts (69 trip + 32 user = 101)', () => { + it('WSEVT-REG-001: pins the authoritative inventory counts (71 trip + 32 user = 103)', () => { // 67th to 69th trip event: the three collab:link:* a shared link emits. - expect(TREK_WS_TRIP_EVENT_NAMES).toHaveLength(69); + // 70th and 71st: the road trip's vias and tracks, which used to be written silently. + expect(TREK_WS_TRIP_EVENT_NAMES).toHaveLength(71); expect(TREK_WS_USER_EVENT_NAMES).toHaveLength(32); - expect(TREK_WS_EVENT_NAMES).toHaveLength(101); + expect(TREK_WS_EVENT_NAMES).toHaveLength(103); }); it('WSEVT-REG-002: every name is domain:action shaped and outside the reserved plugin: namespace', () => { diff --git a/shared/src/realtime/events.schema.ts b/shared/src/realtime/events.schema.ts index 378e17733..fae358992 100644 --- a/shared/src/realtime/events.schema.ts +++ b/shared/src/realtime/events.schema.ts @@ -92,6 +92,25 @@ export const TREK_WS_EVENTS = { payload: z.union([z.object({ orderedIds: idList }), z.object({ day: entity })]), }, + // ── Road trip ──────────────────────────────────────────────────────────── + // The points a drive is routed through, and the tracks a day follows. Broadcast in one + // event carrying the day's whole list rather than one event per point: a via has no + // identity anyone reads, the client keeps them per day anyway, and a drag is a rapid + // burst of writes whose only interesting state is the one that lands last. + // + // These used to be silent, on the reasoning that a via is how one person draws a route + // rather than a change to the itinerary everybody reads. That does not hold: two people + // planning a road trip see the same line on the same map, and a reshaped drive changes + // every arrival time after it. + 'roadtripVia:changed': { + scope: 'trip', + payload: z.object({ dayId: id, vias: z.array(entity) }), + }, + 'roadtripTrack:changed': { + scope: 'trip', + payload: z.object({ dayId: id, track: entity.nullable() }), + }, + // ── Day notes ──────────────────────────────────────────────────────────── 'dayNote:created': { scope: 'trip', payload: z.object({ dayId: id, note: entity }) }, 'dayNote:updated': { scope: 'trip', payload: z.object({ dayId: id, note: entity }) },