fix(realtime): road trip vias and day boundaries send the same events everywhere

server/src/nest/roadtrip/roadtrip.controller.ts

@@ -1,6 +1,8 @@
 import { ADDON_IDS } from '../../addons';
+import type { User } from '../../types';
 import { AddonGuard } from '../addons/addon.guard';
 import { RequireAddon } from '../addons/require-addon.decorator';
+import { CurrentUser } from '../auth-core/current-user.decorator';
 import { JwtAuthGuard } from '../auth-core/jwt-auth.guard';
 import { RequirePermission, TripAccessGuard } from '../permissions/trip-access.guard';
 import {
@@ -37,6 +39,10 @@ import type { RoadtripDayTrack, RoadtripVia } from '@trek/shared';
  * 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.
+ *
+ * The events are the service's (RoadtripService.addVia and its siblings), the same for
+ * these routes and the road trip tools; a route hands it a writer that leaves out the
+ * socket that sent the request.
  */
 @Controller('api/trips/:tripId/roadtrip')
 // AddonGuard FIRST: @RequireAddon is metadata and does nothing on its own, and the guard
@@ -63,14 +69,14 @@ export class RoadtripController {
   @RequirePermission('day_edit')
   @Post('days/:dayId/vias')
   async create(
+    @CurrentUser() user: User,
     @Param('tripId') tripId: string,
     @Param('dayId') dayId: string,
     @Body() body: RoadtripViaCreateDto,
     @Headers('x-socket-id') socketId?: string,
   ): Promise<{ via: RoadtripVia }> {
     await this.requireDay(dayId, tripId);
-    const via = await this.roadtrip.create(dayId, body);
-    await this.announce(tripId, dayId, socketId);
+    const via = await this.roadtrip.addVia(dayId, body, this.roadtrip.restWriter(tripId, user, socketId));
     return { via };
   }
 
@@ -86,6 +92,7 @@ export class RoadtripController {
   @Post('days/:dayId/vias/batch')
   @HttpCode(200)
   async createMany(
+    @CurrentUser() user: User,
     @Param('tripId') tripId: string,
     @Param('dayId') dayId: string,
     @Body() body: RoadtripViaBatchDto,
@@ -98,20 +105,7 @@ export class RoadtripController {
     if (body.track && !(await this.roadtrip.trackExists(body.track.place_id, tripId))) {
       throw new HttpException({ error: 'Track not found' }, 404);
     }
-    const vias = await this.roadtrip.createMany(dayId, body);
-    await 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.
-    const tracks = await this.roadtrip.tracksForTrip(tripId);
-    this.roadtrip.broadcast(
-      tripId,
-      'roadtripTrack:changed',
-      {
-        dayId,
-        track: tracks.find((t) => String(t.day_id) === String(dayId)) ?? null,
-      },
-      socketId,
-    );
+    const vias = await this.roadtrip.addVias(tripId, dayId, body, this.roadtrip.restWriter(tripId, user, socketId));
     return { vias };
   }
 
@@ -126,14 +120,14 @@ export class RoadtripController {
   @Put('days/:dayId/vias')
   @HttpCode(200)
   async reanchor(
+    @CurrentUser() user: User,
     @Param('tripId') tripId: string,
     @Param('dayId') dayId: string,
     @Body() body: RoadtripViaReanchorDto,
     @Headers('x-socket-id') socketId?: string,
   ): Promise<{ vias: RoadtripVia[] }> {
     await this.requireDay(dayId, tripId);
-    const vias = await this.roadtrip.reanchor(dayId, body);
-    await this.announce(tripId, dayId, socketId);
+    const vias = await this.roadtrip.reanchorVias(dayId, body, this.roadtrip.restWriter(tripId, user, socketId));
     return { vias };
   }
 
@@ -141,6 +135,7 @@ export class RoadtripController {
   @Put('days/:dayId/vias/:id')
   @HttpCode(200)
   async update(
+    @CurrentUser() user: User,
     @Param('tripId') tripId: string,
     @Param('dayId') dayId: string,
     @Param('id') id: string,
@@ -148,48 +143,28 @@ export class RoadtripController {
     @Headers('x-socket-id') socketId?: string,
   ): Promise<{ via: RoadtripVia }> {
     await this.requireDay(dayId, tripId);
-    const via = await this.roadtrip.move(id, dayId, body.lat, body.lng, body.after_order_index);
+    const writer = this.roadtrip.restWriter(tripId, user, socketId);
+    const via = await this.roadtrip.moveVia(dayId, id, body, writer);
     if (!via) throw new HttpException({ error: 'Via not found' }, 404);
-    await this.announce(tripId, dayId, socketId);
     return { via };
   }
 
   @RequirePermission('day_edit')
   @Delete('days/:dayId/vias/:id')
   async remove(
+    @CurrentUser() user: User,
     @Param('tripId') tripId: string,
     @Param('dayId') dayId: string,
     @Param('id') id: string,
     @Headers('x-socket-id') socketId?: string,
   ): Promise<{ success: true }> {
     await this.requireDay(dayId, tripId);
-    if (!(await this.roadtrip.remove(id, dayId))) {
+    if (!(await this.roadtrip.removeVia(dayId, id, this.roadtrip.restWriter(tripId, user, socketId)))) {
       throw new HttpException({ error: 'Via not found' }, 404);
     }
-    await 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 async announce(tripId: string, dayId: string, socketId: string | undefined): Promise<void> {
-    this.roadtrip.broadcast(
-      tripId,
-      'roadtripVia:changed',
-      {
-        dayId,
-        vias: await 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

server/src/nest/roadtrip/roadtrip.mcp.ts

@@ -41,6 +41,8 @@ const BOOKEND_VIA_NOTE =
  * the controller demands, so an assistant can never do through a tool what a person
  * cannot do through the UI. Reads go through the same day-belongs-to-trip check, so a
  * valid day id from another trip is not reachable by way of one the caller can see.
+ * The writes and their events are the service's use cases the routes call too; a tool
+ * call has no socket of its own, so the tab that asked hears the change like everyone else.
  */
 @McpController()
 export class RoadtripMcp {
@@ -99,8 +101,8 @@ export class RoadtripMcp {
     if (!(await this.tripsRepo.findAccessible(tripId, ctx.userId))) return noAccess();
     if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied();
     if (!(await this.roadtrip.dayExists(dayId, tripId))) return noAccess();
-    const via = await this.roadtrip.create(dayId, { after_order_index, lat, lng });
-    await this.announce(tripId, dayId);
+    const writer = await this.guards.tripWriter(tripId, ctx.userId);
+    const via = await this.roadtrip.addVia(dayId, { after_order_index, lat, lng }, writer);
     return ok({ via });
   }
 
@@ -173,18 +175,8 @@ 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 && !(await this.roadtrip.trackExists(track.place_id, tripId))) return noAccess();
-    const made = await this.roadtrip.createMany(dayId, { vias, replace_legs, track });
-    await this.announce(tripId, dayId);
-    const tracks = await this.roadtrip.tracksForTrip(String(tripId));
-    this.roadtrip.broadcast(
-      String(tripId),
-      'roadtripTrack:changed',
-      {
-        dayId,
-        track: tracks.find((t) => String(t.day_id) === String(dayId)) ?? null,
-      },
-      undefined,
-    );
+    const writer = await this.guards.tripWriter(tripId, ctx.userId);
+    const made = await this.roadtrip.addVias(tripId, dayId, { vias, replace_legs, track }, writer);
     return ok({ vias: made });
   }
 
@@ -231,8 +223,8 @@ export class RoadtripMcp {
     if (!(await this.tripsRepo.findAccessible(tripId, ctx.userId))) return noAccess();
     if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied();
     if (!(await this.roadtrip.dayExists(dayId, tripId))) return noAccess();
-    const next = await this.roadtrip.reanchor(dayId, { vias, remove });
-    await this.announce(tripId, dayId);
+    const writer = await this.guards.tripWriter(tripId, ctx.userId);
+    const next = await this.roadtrip.reanchorVias(dayId, { vias, remove }, writer);
     return ok({ vias: next });
   }
 
@@ -252,8 +244,8 @@ export class RoadtripMcp {
     if (!(await this.tripsRepo.findAccessible(tripId, ctx.userId))) return noAccess();
     if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied();
     if (!(await this.roadtrip.dayExists(dayId, tripId))) return noAccess();
-    if (!(await this.roadtrip.remove(viaId, dayId))) return noAccess();
-    await this.announce(tripId, dayId);
+    const writer = await this.guards.tripWriter(tripId, ctx.userId);
+    if (!(await this.roadtrip.removeVia(dayId, viaId, writer))) return noAccess();
     return ok({ success: true });
   }
 
@@ -270,29 +262,10 @@ export class RoadtripMcp {
     if (!(await this.tripsRepo.findAccessible(input.tripId, ctx.userId))) return noAccess();
     if (!(await this.guards.hasTripPermission('day_edit', input.tripId, ctx.userId))) return permissionDenied();
     if (!(await this.roadtrip.dayExists(input.dayId, input.tripId))) return noAccess();
-    const via = await this.roadtrip.move(input.viaId, input.dayId, input.lat, input.lng, input.after_order_index);
+    const writer = await this.guards.tripWriter(input.tripId, ctx.userId);
+    const { lat, lng, after_order_index } = input;
+    const via = await this.roadtrip.moveVia(input.dayId, input.viaId, { lat, lng, after_order_index }, writer);
     if (!via) return noAccess();
-    await this.announce(input.tripId, input.dayId);
     return ok({ via });
   }
-
-  /**
-   * 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 async announce(tripId: number, dayId: number): Promise<void> {
-    this.roadtrip.broadcast(
-      String(tripId),
-      'roadtripVia:changed',
-      {
-        dayId,
-        vias: await this.roadtrip.listForDay(String(dayId)),
-      },
-      undefined,
-    );
-  }
 }

server/src/nest/roadtrip/roadtrip.service.ts

@@ -6,13 +6,31 @@ import type { DaysRepository } from '../../db/repositories/Days.repository';
 import type { PlacesRepository } from '../../db/repositories/Places.repository';
 import type { RoadtripDayTracksRepository } from '../../db/repositories/RoadtripDayTracks.repository';
 import type { RoadtripViasRepository } from '../../db/repositories/RoadtripVias.repository';
+import type { User } from '../../types';
 import { DomainError } from '../common/domain-error';
 import { toRowId } from '../common/row-id';
+import { restTripWriter, type TripWriter } from '../common/trip-writer';
 import { UnitOfWork } from '../database/unit-of-work';
 import { RealtimeService } from '../realtime/realtime.service';
 import { InjectRepository } from '@mikro-orm/nestjs';
-import { Injectable } from '@nestjs/common';
-import type { RoadtripDayTrack, RoadtripVia, TrekWsPayload, TrekWsTripEventName } from '@trek/shared';
+import { Injectable, Logger } from '@nestjs/common';
+import type { RoadtripDayTrack, RoadtripVia } from '@trek/shared';
+
+/** A via as a write places it: after which stop of the day, and where. */
+type ViaInput = { after_order_index: number; lat: number; lng: number };
+
+/** A chain of vias laid on one day, the legs it clears first and the track it was fitted to. */
+type ViaChainInput = {
+  vias: ViaInput[];
+  replace_legs?: number[];
+  track?: { place_id: number; stray_km?: number | null } | null;
+};
+
+/** The vias a re-anchoring moves to other legs, and those whose leg is gone. */
+type ViaReanchorInput = { vias: { id: number; after_order_index: number }[]; remove?: number[] };
+
+/** Where a dragged via lands, and the leg it now follows when the drag crossed a stop. */
+type ViaMove = { lat: number; lng: number; after_order_index?: number };
 
 /**
  * Via points: the places a day's drive is made to pass through without stopping.
@@ -30,6 +48,8 @@ import type { RoadtripDayTrack, RoadtripVia, TrekWsPayload, TrekWsTripEventName
  */
 @Injectable()
 export class RoadtripService {
+  private readonly logger = new Logger(RoadtripService.name);
+
   constructor(
     private readonly realtime: RealtimeService,
     private readonly uow: UnitOfWork,
@@ -39,24 +59,98 @@ export class RoadtripService {
     @InjectRepository(RoadtripDayTracks) private readonly tracksRepo: RoadtripDayTracksRepository,
   ) {}
 
+  /** The REST caller of a via write: its events leave out the socket that sent the request. */
+  restWriter(tripId: string, user: User, socketId: string | undefined): TripWriter {
+    return restTripWriter(this.realtime, tripId, user, socketId);
+  }
+
   /**
-   * Tells the trip's other clients what the drive looks like now.
+   * The via writes below, one use case each for the routes and the tools alike. The caller
+   * has checked that the day belongs to the trip and that the writer may edit its days; each
+   * writes, then tells the trip what the day's drive runs through now, after the commit.
+   */
+  async addVia(dayId: string | number, input: ViaInput, writer: TripWriter): Promise<RoadtripVia> {
+    const via = await this.create(dayId, input);
+    await this.announceDay(dayId, writer);
+    return via;
+  }
+
+  /**
+   * A chain laid on one day, then the track the day follows now: a batch is how a day starts
+   * following a recorded track, so the rail draws a badge for it and the map a line. The
+   * track event goes out even when the batch named none, as it always has.
+   */
+  async addVias(
+    tripId: string | number,
+    dayId: string | number,
+    input: ViaChainInput,
+    writer: TripWriter,
+  ): Promise<RoadtripVia[]> {
+    const vias = await this.createMany(dayId, input);
+    await this.announceDay(dayId, writer);
+    try {
+      const tracks = await this.tracksForTrip(tripId);
+      writer.events.emit('roadtripTrack:changed', {
+        dayId: String(dayId),
+        track: tracks.find((t) => String(t.day_id) === String(dayId)) ?? null,
+      });
+    } catch {
+      this.logger.warn(`Committed vias for day ${String(dayId)}: track notification failed`);
+    }
+    return vias;
+  }
+
+  async reanchorVias(dayId: string | number, input: ViaReanchorInput, writer: TripWriter): Promise<RoadtripVia[]> {
+    const vias = await this.reanchor(dayId, input);
+    await this.announceDay(dayId, writer);
+    return vias;
+  }
+
+  /** Null, and nothing said, when the via is not on this day. */
+  async moveVia(
+    dayId: string | number,
+    id: string | number,
+    to: ViaMove,
+    writer: TripWriter,
+  ): Promise<RoadtripVia | null> {
+    const via = await this.move(id, dayId, to.lat, to.lng, to.after_order_index);
+    if (via) await this.announceDay(dayId, writer);
+    return via;
+  }
+
+  /** False, and nothing said, when the via is not on this day. */
+  async removeVia(dayId: string | number, id: string | number, writer: TripWriter): Promise<boolean> {
+    const removed = await this.remove(id, dayId);
+    if (removed) await this.announceDay(dayId, writer);
+    return removed;
+  }
+
+  /**
+   * Tells the trip what this day's drive is routed through 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
+   * 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.
+   * The whole day's list goes out, read back rather than assembled from the write: a via
+   * carries no identity anybody reads, the client holds them per day, a reanchor rewrites
+   * the set and a batch may clear legs before filling them. REST leaves out the socket that
+   * made the change, so the person dragging does not get their point handed back mid-drag;
+   * a tool call has no socket, so the tab that asked hears it like everyone else.
+   *
+   * The day id goes out as text, the way the routes have always sent it, on every surface.
+   * Other senders of this event (assignments, accommodations, hike plans) still send a
+   * number, as their own routes always did; the client reads it through `Number()` either way.
+   *
+   * The write is committed by the time this runs, so a failed read or send is logged and
+   * swallowed: the caller gets the row it wrote, not a 500 for a change that landed.
    */
-  broadcast<E extends TrekWsTripEventName>(
-    tripId: string,
-    event: E,
-    payload: TrekWsPayload<E>,
-    socketId: string | undefined,
-  ): void {
-    this.realtime.broadcast(tripId, event, payload, socketId);
+  private async announceDay(dayId: string | number, writer: TripWriter): Promise<void> {
+    try {
+      writer.events.emit('roadtripVia:changed', { dayId: String(dayId), vias: await this.listForDay(dayId) });
+    } catch {
+      this.logger.warn(`Committed vias for day ${String(dayId)}: realtime notification failed`);
+    }
   }
 
   /**
@@ -110,6 +204,7 @@ export class RoadtripService {
     return await this.viasRepo.listForTrip(tripIdNum);
   }
 
+  /** Silent: writes and tells nobody. Callers that change a trip go through `addVia`. */
   async create(
     dayId: string | number,
     input: { after_order_index: number; lat: number; lng: number; sequence?: number },
@@ -140,6 +235,8 @@ export class RoadtripService {
    *
    * `replace_legs` clears by leg rather than by day, so vias the traveller placed by hand
    * on other legs survive a track being laid on this one.
+   *
+   * Silent: writes and tells nobody. Callers that change a trip go through `addVias`.
    */
   async createMany(
     dayId: string | number,
@@ -202,7 +299,11 @@ export class RoadtripService {
     return await this.placesRepo.isTrackInTrip(placeId, tripIdNum);
   }
 
-  /** Moving a via is the whole edit; where it sits in the chain does not change. */
+  /**
+   * Moving a via is the whole edit; where it sits in the chain does not change.
+   *
+   * Silent: writes and tells nobody. Callers that change a trip go through `moveVia`.
+   */
   async move(
     id: string | number,
     dayId: string | number,
@@ -243,6 +344,8 @@ export class RoadtripService {
    * Silent about ids it does not find. The caller computed this list from a snapshot of
    * the day, and a via somebody else deleted in the meantime is not a failure of the
    * re-anchoring; failing the batch over it would leave the rest of the day mis-pinned.
+   *
+   * Silent: writes and tells nobody. Callers that change a trip go through `reanchorVias`.
    */
   async reanchor(
     dayId: string | number,
@@ -300,6 +403,7 @@ export class RoadtripService {
     });
   }
 
+  /** Silent: writes and tells nobody. Callers that change a trip go through `removeVia`. */
   async remove(id: string | number, dayId: string | number): Promise<boolean> {
     const idNum = toRowId(id);
     const dayIdNum = toRowId(dayId);