test(roadtrip): cover the surface the domain shipped without

The domain arrived with 557 lines, a service test, and nothing else. No controller test, no e2e, and no MCP test — so the guard chain, the trip scoping on every route, the cross-trip track check and the day_edit parity were all unasserted, and the whole domain sat in the 80 catch-all where it could shed twenty points against the headroom of the domains beside it. The e2e is the one that earns its place. `@RequireAddon` is metadata and does nothing on its own, and AddonGuard is not global, so the ordering the controller's own comment calls load-bearing is exactly the kind of thing a later change removes without noticing — it had already happened once. Deleting the guard now turns three cases red, which I checked by deleting it. The MCP tools had never executed at all: the shared harness seeds the addon disabled, so `when: roadtripAddonOn` kept all five from attaching. They now run against the same checks the REST routes make, including the demo account and a member without day_edit, because the repo requires the two surfaces to move together. Two client modules that had no test get one: the overlay builder, where two decisions live that are not cosmetic — which offered road is drawn as the one you are on, and that a label hangs where its own route is unlike every other, rather than at a midpoint that can fall on a stretch two routes share — and the service marker, where the thing worth pinning is that an ordinary place gets nothing back, and that the anchor arithmetic includes the border. The domain carries its own coverage floor now instead of hiding in the catch-all. Pinned under the measured figure on purpose; the exact ratchet gets regenerated with the repo's own script after a full run. Along the way, roadtrip_vias and roadtrip_day_tracks join the reset list every other domain's tables are on. Without that a via written by one case was still there for the next one, which is how the first version of the MCP suite passed an assertion it should have failed.
2026-10-18 21:16:47 +00:00 · 2026-09-07 16:27:59 +02:00
parent 41e256bf0f
commit 65be8db122
7 changed files with 1019 additions and 0 deletions
@@ -0,0 +1,273 @@
+/**
+ * Road trip module e2e — the real guard chain against a temp SQLite db.
+ *
+ * The unit test next door pins the handler bodies. What only a booted container
+ * can show is the thing the controller's own comment calls load-bearing: that
+ * `@RequireAddon` does nothing on its own, so an instance with the addon
+ * switched off has to answer 404 here, and has to do it before asking who is
+ * calling. Delete AddonGuard from the chain and every case in the first block
+ * below turns red.
+ *
+ * The addon ships disabled, so "off" is the state an existing install upgrades
+ * into. That is what makes it worth a test rather than a comment.
+ */
+import { describe, it, expect, beforeAll, afterAll, beforeEach, vi, type MockInstance } from 'vitest';
+import request from 'supertest';
+import cookieParser from 'cookie-parser';
+import type { Server } from 'http';
+import { Test } from '@nestjs/testing';
+import { DatabaseModule } from '../../src/nest/database/database.module';
+import { RealtimeModule } from '../../src/nest/realtime/realtime.module';
+import { seedUser, sessionCookie } from './harness';
+
+const { db } = vi.hoisted(() => {
+  // eslint-disable-next-line @typescript-eslint/no-require-imports
+  const Database = require('better-sqlite3');
+  const tmp = new Database(':memory:');
+  tmp.exec('PRAGMA journal_mode = WAL');
+  tmp.exec('PRAGMA foreign_keys = ON');
+  tmp.exec(`CREATE TABLE users (id INTEGER PRIMARY KEY AUTOINCREMENT, username TEXT NOT NULL,
+    email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0,
+    avatar TEXT);`);
+  tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER, title TEXT, end_date TEXT);');
+  tmp.exec('CREATE TABLE trip_members (trip_id INTEGER NOT NULL, user_id INTEGER NOT NULL);');
+  tmp.exec(`CREATE TABLE days (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL,
+    day_number INTEGER, date TEXT, title TEXT, notes TEXT);`);
+  tmp.exec(`CREATE TABLE places (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL, name TEXT,
+    lat REAL, lng REAL, route_geometry TEXT, stop_type TEXT);`);
+  tmp.exec(`CREATE TABLE roadtrip_vias (id INTEGER PRIMARY KEY AUTOINCREMENT,
+    day_id INTEGER NOT NULL REFERENCES days(id) ON DELETE CASCADE,
+    after_order_index INTEGER NOT NULL, sequence INTEGER NOT NULL DEFAULT 0,
+    lat REAL NOT NULL, lng REAL NOT NULL, created_at TEXT DEFAULT CURRENT_TIMESTAMP);`);
+  tmp.exec(`CREATE TABLE roadtrip_day_tracks (
+    day_id INTEGER PRIMARY KEY REFERENCES days(id) ON DELETE CASCADE,
+    place_id INTEGER NOT NULL REFERENCES places(id) ON DELETE CASCADE, stray_km REAL);`);
+  // AddonsService reads this; StorageRegistryService reads app_settings at init.
+  tmp.exec(`CREATE TABLE addons (id TEXT PRIMARY KEY, enabled INTEGER NOT NULL DEFAULT 0,
+    name TEXT, description TEXT, category TEXT, sort_order INTEGER DEFAULT 0);`);
+  tmp.exec('CREATE TABLE app_settings (key TEXT PRIMARY KEY, value TEXT);');
+  return { db: tmp };
+});
+
+vi.mock('../../src/db/database', () => ({
+  db,
+  canAccessTrip: (tripId: number | string, userId: number) =>
+    db.prepare(`
+      SELECT t.id, t.user_id FROM trips t
+      LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ?
+      WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL)
+    `).get(userId, tripId, userId),
+  isOwner: () => false,
+  getPlaceWithTags: () => null,
+  closeDb: () => {},
+  reinitialize: () => {},
+}));
+vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() }));
+
+import { PermissionsService } from '../../src/nest/permissions/permissions.service';
+import { RoadtripModule } from '../../src/nest/roadtrip/roadtrip.module';
+import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter';
+import { ZodValidationPipe } from '../../src/nest/common/zod-validation.pipe';
+
+const ADDON_ID = 'roadtrip';
+
+function setAddon(enabled: boolean): void {
+  db.prepare('INSERT INTO addons (id, enabled) VALUES (?, ?) ON CONFLICT(id) DO UPDATE SET enabled = excluded.enabled')
+    .run(ADDON_ID, enabled ? 1 : 0);
+}
+
+describe('Roadtrip e2e (real guard chain + temp SQLite)', () => {
+  let server: Server;
+  let app: Awaited<ReturnType<typeof build>>;
+  let checkPermission: MockInstance;
+
+  async function build() {
+    const moduleRef = await Test.createTestingModule({
+      imports: [DatabaseModule, RealtimeModule, RoadtripModule],
+    }).compile();
+    const nest = moduleRef.createNestApplication();
+    nest.use(cookieParser());
+    nest.useGlobalPipes(new ZodValidationPipe());
+    nest.useGlobalFilters(new TrekExceptionFilter());
+    await nest.init();
+    return nest;
+  }
+
+  beforeAll(async () => {
+    seedUser(db as never, { id: 1 });
+    seedUser(db as never, { id: 2, email: 'other@example.test' });
+    db.prepare('INSERT INTO trips (id, user_id, title) VALUES (5, 1, ?)').run('Norway');
+    db.prepare('INSERT INTO trips (id, user_id, title) VALUES (6, 2, ?)').run('Somebody else');
+    db.prepare('INSERT INTO days (id, trip_id, day_number) VALUES (3, 5, 1)').run();
+    db.prepare('INSERT INTO days (id, trip_id, day_number) VALUES (4, 6, 1)').run();
+    // A track (a place carrying a route geometry) on each trip.
+    db.prepare("INSERT INTO places (id, trip_id, name, route_geometry) VALUES (10, 5, 'Scenic', '[[1,2]]')").run();
+    db.prepare("INSERT INTO places (id, trip_id, name, route_geometry) VALUES (11, 6, 'Theirs', '[[1,2]]')").run();
+    // An ordinary place, which is not a track.
+    db.prepare("INSERT INTO places (id, trip_id, name) VALUES (12, 5, 'Just a stop')").run();
+    app = await build();
+    checkPermission = vi.spyOn(app.get(PermissionsService), 'checkPermission');
+    server = app.getHttpServer();
+  });
+
+  beforeEach(() => {
+    db.prepare('DELETE FROM roadtrip_vias').run();
+    db.prepare('DELETE FROM roadtrip_day_tracks').run();
+    checkPermission.mockReturnValue(true);
+    setAddon(true);
+  });
+
+  afterAll(async () => { await app?.close(); });
+
+  const cookie = () => sessionCookie(1);
+
+  describe('the addon gate', () => {
+    // Six routes, all of them. `@RequireAddon` is metadata and inert without the
+    // guard, and the guard is not global — so this block is the only thing that
+    // fails if somebody drops AddonGuard from the chain again.
+    const routes: [string, string, unknown?][] = [
+      ['get', '/api/trips/5/roadtrip/vias'],
+      ['get', '/api/trips/5/roadtrip/days/3/vias'],
+      ['post', '/api/trips/5/roadtrip/days/3/vias', { after_order_index: 0, lat: 53, lng: 10 }],
+      ['post', '/api/trips/5/roadtrip/days/3/vias/batch', { vias: [{ after_order_index: 0, lat: 53, lng: 10 }] }],
+      ['put', '/api/trips/5/roadtrip/days/3/vias', { vias: [] }],
+      ['delete', '/api/trips/5/roadtrip/days/3/vias/1'],
+    ];
+
+    it('ROADTRIP-E2E-001: answers 404 on every route while the addon is off', async () => {
+      setAddon(false);
+      for (const [method, url, body] of routes) {
+        const req = (request(server) as never as Record<string, (u: string) => request.Test>)[method](url)
+          .set('Cookie', cookie());
+        const res = await (body ? req.send(body as object) : req);
+        expect(res.status, `${method.toUpperCase()} ${url}`).toBe(404);
+      }
+    });
+
+    it('ROADTRIP-E2E-002: a disabled addon owes an anonymous caller a 404, not a 401', async () => {
+      // The gate leads the chain for this reason: a 401 tells a stranger the
+      // route exists. Both answers refuse; only one of them says nothing.
+      setAddon(false);
+      const res = await request(server).get('/api/trips/5/roadtrip/vias');
+      expect(res.status).toBe(404);
+    });
+
+    it('ROADTRIP-E2E-003: with the addon on, an anonymous caller gets 401', async () => {
+      const res = await request(server).get('/api/trips/5/roadtrip/vias');
+      expect(res.status).toBe(401);
+    });
+  });
+
+  describe('trip scoping', () => {
+    it('ROADTRIP-E2E-004: a trip the caller cannot reach is 404', async () => {
+      const res = await request(server).get('/api/trips/6/roadtrip/vias').set('Cookie', cookie());
+      expect(res.status).toBe(404);
+    });
+
+    it('ROADTRIP-E2E-005: a day from another trip is not reachable through one the caller has', async () => {
+      // Day 4 exists and is real; it just belongs to trip 6. The guard proves
+      // the trip, `requireDay` proves the day is part of it.
+      const res = await request(server).get('/api/trips/5/roadtrip/days/4/vias').set('Cookie', cookie());
+      expect(res.status).toBe(404);
+      expect(res.body).toEqual({ error: 'Day not found' });
+    });
+
+    it('ROADTRIP-E2E-006: a track from another trip cannot label this day', async () => {
+      const res = await request(server)
+        .post('/api/trips/5/roadtrip/days/3/vias/batch')
+        .set('Cookie', cookie())
+        .send({ vias: [{ after_order_index: 0, lat: 53, lng: 10 }], track: { place_id: 11 } });
+      expect(res.status).toBe(404);
+      expect(res.body).toEqual({ error: 'Track not found' });
+    });
+
+    it('ROADTRIP-E2E-007: a place without a route geometry is not a track', async () => {
+      const res = await request(server)
+        .post('/api/trips/5/roadtrip/days/3/vias/batch')
+        .set('Cookie', cookie())
+        .send({ vias: [{ after_order_index: 0, lat: 53, lng: 10 }], track: { place_id: 12 } });
+      expect(res.status).toBe(404);
+      expect(res.body).toEqual({ error: 'Track not found' });
+    });
+  });
+
+  describe('permission', () => {
+    it('ROADTRIP-E2E-008: reading needs no day_edit, writing does', async () => {
+      checkPermission.mockReturnValue(false);
+      expect((await request(server).get('/api/trips/5/roadtrip/vias').set('Cookie', cookie())).status).toBe(200);
+      const write = await request(server)
+        .post('/api/trips/5/roadtrip/days/3/vias')
+        .set('Cookie', cookie())
+        .send({ after_order_index: 0, lat: 53, lng: 10 });
+      expect(write.status).toBe(403);
+    });
+  });
+
+  describe('the wire contract', () => {
+    it('ROADTRIP-E2E-009: a via round-trips through create, list, move and delete', async () => {
+      const created = await request(server)
+        .post('/api/trips/5/roadtrip/days/3/vias')
+        .set('Cookie', cookie())
+        .send({ after_order_index: 0, lat: 53, lng: 10 });
+      expect(created.status).toBe(201);
+      const id = created.body.via.id;
+      expect(created.body.via).toMatchObject({ day_id: 3, after_order_index: 0, sequence: 0, lat: 53, lng: 10 });
+
+      const listed = await request(server).get('/api/trips/5/roadtrip/days/3/vias').set('Cookie', cookie());
+      expect(listed.body.vias.map((v: { id: number }) => v.id)).toEqual([id]);
+
+      const moved = await request(server)
+        .put(`/api/trips/5/roadtrip/days/3/vias/${id}`)
+        .set('Cookie', cookie())
+        .send({ lat: 54, lng: 11 });
+      // 200, not the 201 Nest defaults a body-carrying write to.
+      expect(moved.status).toBe(200);
+      expect(moved.body.via).toMatchObject({ lat: 54, lng: 11 });
+
+      expect((await request(server).delete(`/api/trips/5/roadtrip/days/3/vias/${id}`).set('Cookie', cookie())).status)
+        .toBe(200);
+      const after = await request(server).get('/api/trips/5/roadtrip/days/3/vias').set('Cookie', cookie());
+      expect(after.body.vias).toEqual([]);
+    });
+
+    it('ROADTRIP-E2E-010: the batch route answers 200 and records the track it was fitted to', async () => {
+      const res = await request(server)
+        .post('/api/trips/5/roadtrip/days/3/vias/batch')
+        .set('Cookie', cookie())
+        .send({
+          vias: [{ after_order_index: 0, lat: 53, lng: 10 }, { after_order_index: 0, lat: 53.5, lng: 10.5 }],
+          track: { place_id: 10, stray_km: 1.5 },
+        });
+      expect(res.status).toBe(200);
+      expect(res.body.vias).toHaveLength(2);
+
+      const all = await request(server).get('/api/trips/5/roadtrip/vias').set('Cookie', cookie());
+      expect(all.body.tracks).toEqual([{ day_id: 3, place_id: 10, stray_km: 1.5 }]);
+    });
+
+    it('ROADTRIP-E2E-011: the re-anchor route is declared before the id route', async () => {
+      // `PUT days/:dayId/vias` and `PUT days/:dayId/vias/:id` differ by one
+      // segment. Declared the other way round, the batch would be swallowed by
+      // the id route and every re-anchoring would 404 on a via named "vias".
+      const created = await request(server)
+        .post('/api/trips/5/roadtrip/days/3/vias')
+        .set('Cookie', cookie())
+        .send({ after_order_index: 2, lat: 53, lng: 10 });
+      const res = await request(server)
+        .put('/api/trips/5/roadtrip/days/3/vias')
+        .set('Cookie', cookie())
+        .send({ vias: [{ id: created.body.via.id, after_order_index: 1 }] });
+      expect(res.status).toBe(200);
+      expect(res.body.vias[0]).toMatchObject({ after_order_index: 1 });
+    });
+
+    it('ROADTRIP-E2E-012: a body the contract refuses is a 400, not a stored row', async () => {
+      const res = await request(server)
+        .post('/api/trips/5/roadtrip/days/3/vias')
+        .set('Cookie', cookie())
+        .send({ after_order_index: 0, lat: 999, lng: 10 });
+      expect(res.status).toBe(400);
+      expect(db.prepare('SELECT COUNT(*) c FROM roadtrip_vias').get()).toEqual({ c: 0 });
+    });
+  });
+});
@@ -34,6 +34,11 @@ const RESET_TABLES = [
  'collab_messages',
  'collab_polls',
  'collab_notes',
+  // Road trip (#1797). Both hang off days, so they go before it like the rest
+  // of the day content; a new domain's tables belong here or its rows leak from
+  // one case into the next.
+  'roadtrip_day_tracks',
+  'roadtrip_vias',
  // Day content
  'day_notes',
  'todo_category_assignees',
@@ -0,0 +1,329 @@
+/**
+ * The five road-trip MCP tools.
+ *
+ * They had no test at all, and the shared harness seeds the addon disabled, so
+ * `when: roadtripAddonOn` kept them from ever attaching: the handler bodies had
+ * never run. What matters here is the same thing the REST e2e pins, because the
+ * repo requires the two surfaces to move together: the addon gate, the
+ * day-belongs-to-this-trip check, the `day_edit` permission, and the cross-trip
+ * track check on the batch tool.
+ */
+import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest';
+
+const { testDb, dbMock } = vi.hoisted(() => {
+  // eslint-disable-next-line @typescript-eslint/no-require-imports
+  const Database = require('better-sqlite3');
+  const db = new Database(':memory:');
+  db.exec('PRAGMA journal_mode = WAL');
+  db.exec('PRAGMA foreign_keys = ON');
+  db.exec('PRAGMA busy_timeout = 5000');
+  const mock = {
+    db,
+    closeDb: () => {},
+    reinitialize: () => {},
+    getPlaceWithTags: () => null,
+    canAccessTrip: (tripId: unknown, userId: number) =>
+      db.prepare(`SELECT t.id, t.user_id FROM trips t LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ? WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL)`).get(userId, tripId, userId),
+    isOwner: (tripId: unknown, userId: number) =>
+      !!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId),
+  };
+  return { testDb: db, dbMock: mock };
+});
+
+vi.mock('../../../src/db/database', () => dbMock);
+vi.mock('../../../src/config', () => ({
+  JWT_SECRET: 'test-jwt-secret-for-trek-testing-only',
+  ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2',
+  updateJwtSecret: () => {},
+}));
+vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn() }));
+
+import { createTables } from '../../../src/db/schema';
+import { runMigrations } from '../../../src/db/migrations';
+import { resetTestDb, setAddonEnabled } from '../../helpers/test-db';
+import { createUser, createTrip, createDay, createPlace } from '../../helpers/factories';
+import { createMcpHarness, parseToolResult, type McpHarness } from '../../helpers/mcp-harness';
+import { ADDON_IDS } from '../../../src/addons';
+import { addTripMember } from '../../helpers/factories';
+import { PermissionsService } from '../../../src/nest/permissions/permissions.service';
+import { DatabaseService } from '../../../src/nest/database/database.service';
+import { DEMO_EMAIL_PRIMARY } from '../../../src/nest/common/demo';
+
+// The permissions cache is module-scoped, so a write through any instance is
+// what the tool's own check reads back.
+const savePermissions = new PermissionsService(new DatabaseService(testDb)).savePermissions.bind(
+  new PermissionsService(new DatabaseService(testDb)),
+);
+
+beforeAll(() => {
+  createTables(testDb);
+  runMigrations(testDb);
+});
+
+beforeEach(() => {
+  resetTestDb(testDb);
+  // resetTestDb leaves the addons table alone, so each run restates it. The
+  // addon ships disabled, which is why the tools were invisible before.
+  setAddonEnabled(testDb, ADDON_IDS.ROADTRIP, true);
+});
+
+afterAll(() => { testDb.close(); });
+
+async function withHarness(userId: number, fn: (h: McpHarness) => Promise<void>) {
+  const h = await createMcpHarness({ userId, withResources: false, scopes: null });
+  try { await fn(h); } finally { await h.cleanup(); }
+}
+
+/** A trip with one day, plus a place carrying a route geometry to use as a track. */
+function scenario() {
+  const { user } = createUser(testDb);
+  const trip = createTrip(testDb, user.id, { title: 'Norway' });
+  const day = createDay(testDb, trip.id, { day_number: 1 });
+  const track = createPlace(testDb, trip.id, { name: 'Scenic' });
+  testDb.prepare("UPDATE places SET route_geometry = '[[1,2],[3,4]]' WHERE id = ?").run(track.id);
+  return { user, trip, day, track };
+}
+
+describe('road-trip MCP tools', () => {
+  it('MCP-ROADTRIP-001: the five tools are gone while the addon is off', async () => {
+    const { user } = createUser(testDb);
+    setAddonEnabled(testDb, ADDON_IDS.ROADTRIP, false);
+
+    await withHarness(user.id, async (h) => {
+      const names = (await h.client.listTools()).tools.map(t => t.name);
+      for (const tool of ['list_route_vias', 'add_route_via', 'add_route_vias', 'reanchor_route_vias', 'remove_route_via']) {
+        expect(names, tool).not.toContain(tool);
+      }
+    });
+  });
+
+  it('MCP-ROADTRIP-002: and there while it is on', async () => {
+    const { user } = createUser(testDb);
+    await withHarness(user.id, async (h) => {
+      const names = (await h.client.listTools()).tools.map(t => t.name);
+      for (const tool of ['list_route_vias', 'add_route_via', 'add_route_vias', 'reanchor_route_vias', 'remove_route_via']) {
+        expect(names, tool).toContain(tool);
+      }
+    });
+  });
+
+  it('MCP-ROADTRIP-003: adding a via, then reading it back for the day and for the trip', async () => {
+    const { user, trip, day } = scenario();
+
+    await withHarness(user.id, async (h) => {
+      const added = await h.client.callTool({
+        name: 'add_route_via',
+        arguments: { tripId: trip.id, dayId: day.id, after_order_index: 0, lat: 53, lng: 10 },
+      });
+      expect(added.isError).toBeFalsy();
+      expect((parseToolResult(added) as { via: { day_id: number } }).via).toMatchObject({ day_id: day.id, lat: 53 });
+
+      const forDay = parseToolResult(await h.client.callTool({
+        name: 'list_route_vias',
+        arguments: { tripId: trip.id, dayId: day.id },
+      })) as { vias: unknown[] };
+      expect(forDay.vias).toHaveLength(1);
+
+      // Without a day, the answer covers the whole trip and carries the tracks
+      // alongside, which is the shape the client reads on every load.
+      const forTrip = parseToolResult(await h.client.callTool({
+        name: 'list_route_vias',
+        arguments: { tripId: trip.id },
+      })) as { vias: unknown[]; tracks: unknown[] };
+      expect(forTrip.vias).toHaveLength(1);
+      expect(forTrip.tracks).toEqual([]);
+    });
+  });
+
+  it('MCP-ROADTRIP-004: a day from another trip is refused on every tool', async () => {
+    // The same check the REST routes make, which is what the repo means by the
+    // two surfaces moving together.
+    const { user, trip } = scenario();
+    const other = createTrip(testDb, user.id, { title: 'Elsewhere' });
+    const otherDay = createDay(testDb, other.id, { day_number: 1 });
+
+    await withHarness(user.id, async (h) => {
+      const calls: [string, Record<string, unknown>][] = [
+        ['list_route_vias', { tripId: trip.id, dayId: otherDay.id }],
+        ['add_route_via', { tripId: trip.id, dayId: otherDay.id, after_order_index: 0, lat: 53, lng: 10 }],
+        ['add_route_vias', { tripId: trip.id, dayId: otherDay.id, vias: [{ after_order_index: 0, lat: 53, lng: 10 }] }],
+        ['reanchor_route_vias', { tripId: trip.id, dayId: otherDay.id, vias: [] }],
+        ['remove_route_via', { tripId: trip.id, dayId: otherDay.id, viaId: 1 }],
+      ];
+      for (const [name, args] of calls) {
+        const res = await h.client.callTool({ name, arguments: args });
+        expect(res.isError, name).toBeTruthy();
+      }
+      expect(testDb.prepare('SELECT COUNT(*) c FROM roadtrip_vias').get()).toEqual({ c: 0 });
+    });
+  });
+
+  it('MCP-ROADTRIP-005: a trip the caller cannot reach is refused', async () => {
+    const { user } = createUser(testDb);
+    const stranger = createUser(testDb, { email: 'other@example.test' });
+    const theirs = createTrip(testDb, stranger.user.id, { title: 'Theirs' });
+    const theirDay = createDay(testDb, theirs.id, { day_number: 1 });
+
+    await withHarness(user.id, async (h) => {
+      const res = await h.client.callTool({
+        name: 'add_route_via',
+        arguments: { tripId: theirs.id, dayId: theirDay.id, after_order_index: 0, lat: 53, lng: 10 },
+      });
+      expect(res.isError).toBeTruthy();
+    });
+  });
+
+  it('MCP-ROADTRIP-006: a chain lands in one call and records its track', async () => {
+    const { user, trip, day, track } = scenario();
+
+    await withHarness(user.id, async (h) => {
+      const res = await h.client.callTool({
+        name: 'add_route_vias',
+        arguments: {
+          tripId: trip.id,
+          dayId: day.id,
+          vias: [{ after_order_index: 0, lat: 53, lng: 10 }, { after_order_index: 0, lat: 53.5, lng: 10.5 }],
+          track: { place_id: track.id, stray_km: 1.5 },
+        },
+      });
+      expect(res.isError).toBeFalsy();
+      expect((parseToolResult(res) as { vias: unknown[] }).vias).toHaveLength(2);
+
+      const all = parseToolResult(await h.client.callTool({
+        name: 'list_route_vias',
+        arguments: { tripId: trip.id },
+      })) as { tracks: { day_id: number; place_id: number; stray_km: number }[] };
+      expect(all.tracks).toEqual([{ day_id: day.id, place_id: track.id, stray_km: 1.5 }]);
+    });
+  });
+
+  it('MCP-ROADTRIP-007: a track from another trip cannot label this day', async () => {
+    const { user, trip, day } = scenario();
+    const stranger = createUser(testDb, { email: 'other2@example.test' });
+    const theirs = createTrip(testDb, stranger.user.id, { title: 'Theirs' });
+    const theirTrack = createPlace(testDb, theirs.id, { name: 'Not yours' });
+    testDb.prepare("UPDATE places SET route_geometry = '[[1,2]]' WHERE id = ?").run(theirTrack.id);
+
+    await withHarness(user.id, async (h) => {
+      const res = await h.client.callTool({
+        name: 'add_route_vias',
+        arguments: {
+          tripId: trip.id,
+          dayId: day.id,
+          vias: [{ after_order_index: 0, lat: 53, lng: 10 }],
+          track: { place_id: theirTrack.id },
+        },
+      });
+      expect(res.isError).toBeTruthy();
+      expect(testDb.prepare('SELECT COUNT(*) c FROM roadtrip_vias').get()).toEqual({ c: 0 });
+    });
+  });
+
+  it('MCP-ROADTRIP-008: re-anchoring moves what it names and removes what it lists', async () => {
+    const { user, trip, day } = scenario();
+
+    await withHarness(user.id, async (h) => {
+      const first = parseToolResult(await h.client.callTool({
+        name: 'add_route_via',
+        arguments: { tripId: trip.id, dayId: day.id, after_order_index: 0, lat: 53, lng: 10 },
+      })) as { via: { id: number } };
+      const second = parseToolResult(await h.client.callTool({
+        name: 'add_route_via',
+        arguments: { tripId: trip.id, dayId: day.id, after_order_index: 1, lat: 54, lng: 11 },
+      })) as { via: { id: number } };
+
+      const res = parseToolResult(await h.client.callTool({
+        name: 'reanchor_route_vias',
+        arguments: {
+          tripId: trip.id,
+          dayId: day.id,
+          vias: [{ id: second.via.id, after_order_index: 0 }],
+          remove: [first.via.id],
+        },
+      })) as { vias: { id: number; after_order_index: number }[] };
+
+      expect(res.vias).toHaveLength(1);
+      expect(res.vias[0]).toMatchObject({ id: second.via.id, after_order_index: 0 });
+    });
+  });
+
+  it('MCP-ROADTRIP-010: a member without day_edit may read but not write', async () => {
+    // The permission the controller demands is the permission the tool demands:
+    // an assistant must never be able to do through a tool what the person it
+    // is acting for cannot do through the UI.
+    const { user, trip, day } = scenario();
+    const member = createUser(testDb, { email: 'member@example.test' });
+    addTripMember(testDb, trip.id, member.user.id);
+    savePermissions({ day_edit: 'trip_owner' });
+
+    try {
+      await withHarness(member.user.id, async (h) => {
+        const read = await h.client.callTool({
+          name: 'list_route_vias',
+          arguments: { tripId: trip.id, dayId: day.id },
+        });
+        expect(read.isError).toBeFalsy();
+
+        for (const name of ['add_route_via', 'add_route_vias', 'reanchor_route_vias', 'remove_route_via']) {
+          const args: Record<string, unknown> = { tripId: trip.id, dayId: day.id };
+          if (name === 'add_route_via') Object.assign(args, { after_order_index: 0, lat: 53, lng: 10 });
+          if (name === 'add_route_vias') args.vias = [{ after_order_index: 0, lat: 53, lng: 10 }];
+          if (name === 'reanchor_route_vias') args.vias = [];
+          if (name === 'remove_route_via') args.viaId = 1;
+          const res = await h.client.callTool({ name, arguments: args });
+          expect(res.isError, name).toBeTruthy();
+        }
+      });
+      expect(testDb.prepare('SELECT COUNT(*) c FROM roadtrip_vias').get()).toEqual({ c: 0 });
+    } finally {
+      savePermissions({ day_edit: 'trip_member' });
+    }
+    // The owner is unaffected.
+    await withHarness(user.id, async (h) => {
+      const res = await h.client.callTool({
+        name: 'add_route_via',
+        arguments: { tripId: trip.id, dayId: day.id, after_order_index: 0, lat: 53, lng: 10 },
+      });
+      expect(res.isError).toBeFalsy();
+    });
+  });
+
+  it('MCP-ROADTRIP-011: the demo account may look but never write', async () => {
+    const { trip, day } = scenario();
+    const demo = createUser(testDb, { email: DEMO_EMAIL_PRIMARY });
+    addTripMember(testDb, trip.id, demo.user.id);
+
+    process.env.DEMO_MODE = 'true';
+    try {
+      await withHarness(demo.user.id, async (h) => {
+        const read = await h.client.callTool({
+          name: 'list_route_vias',
+          arguments: { tripId: trip.id, dayId: day.id },
+        });
+        expect(read.isError).toBeFalsy();
+
+        const write = await h.client.callTool({
+          name: 'add_route_via',
+          arguments: { tripId: trip.id, dayId: day.id, after_order_index: 0, lat: 53, lng: 10 },
+        });
+        expect(write.isError).toBeTruthy();
+        expect((write.content as { text: string }[])[0].text).toContain('demo');
+      });
+    } finally {
+      delete process.env.DEMO_MODE;
+    }
+    expect(testDb.prepare('SELECT COUNT(*) c FROM roadtrip_vias').get()).toEqual({ c: 0 });
+  });
+
+  it('MCP-ROADTRIP-009: removing one that is not on the day is refused, not silently ignored', async () => {
+    const { user, trip, day } = scenario();
+
+    await withHarness(user.id, async (h) => {
+      const res = await h.client.callTool({
+        name: 'remove_route_via',
+        arguments: { tripId: trip.id, dayId: day.id, viaId: 9999 },
+      });
+      expect(res.isError).toBeTruthy();
+    });
+  });
+});
@@ -0,0 +1,183 @@
+/**
+ * ROADTRIP-CTL-001..016 — the seven via routes.
+ *
+ * What is worth pinning here is not the happy path, which the service tests
+ * already cover, but the checks a later change could delete without anything
+ * noticing: the guard chain, the day-belongs-to-this-trip test on every route,
+ * the track-belongs-to-this-trip test on the batch route, and the exact 404
+ * bodies the client reads.
+ *
+ * The guard chain is asserted through the decorator metadata rather than by
+ * booting Nest: the controller's own comment calls the ORDER load-bearing, and
+ * a class-level assertion is the only thing that fails when somebody removes
+ * AddonGuard again. `@RequireAddon` is metadata and does nothing on its own, so
+ * dropping the guard from the list is a silent regression that opens all seven
+ * routes on an instance that switched the addon off.
+ */
+import { describe, it, expect, vi } from 'vitest';
+import { HttpException } from '@nestjs/common';
+import { GUARDS_METADATA } from '@nestjs/common/constants';
+import { RoadtripController } from '../../../src/nest/roadtrip/roadtrip.controller';
+import type { RoadtripService } from '../../../src/nest/roadtrip/roadtrip.service';
+import { AddonGuard } from '../../../src/nest/addons/addon.guard';
+import { JwtAuthGuard } from '../../../src/nest/auth/jwt-auth.guard';
+import { TripAccessGuard } from '../../../src/nest/permissions/trip-access.guard';
+
+const VIA = { id: 5, day_id: 4, after_order_index: 0, sequence: 0, lat: 53, lng: 10 };
+
+function svc(o: Partial<RoadtripService> = {}): RoadtripService {
+  return {
+    dayExists: vi.fn().mockReturnValue(true),
+    trackExists: vi.fn().mockReturnValue(true),
+    listForDay: vi.fn().mockReturnValue([VIA]),
+    listForTrip: vi.fn().mockReturnValue([VIA]),
+    tracksForTrip: vi.fn().mockReturnValue([]),
+    create: vi.fn().mockReturnValue(VIA),
+    createMany: vi.fn().mockReturnValue([VIA]),
+    reanchor: vi.fn().mockReturnValue([VIA]),
+    move: vi.fn().mockReturnValue(VIA),
+    remove: vi.fn().mockReturnValue(true),
+    ...o,
+  } as unknown as RoadtripService;
+}
+
+function thrown(fn: () => unknown): { status: number; body: unknown } {
+  try { fn(); } catch (err) {
+    expect(err).toBeInstanceOf(HttpException);
+    const e = err as HttpException;
+    return { status: e.getStatus(), body: e.getResponse() };
+  }
+  throw new Error('expected throw');
+}
+
+describe('RoadtripController — the guard chain', () => {
+  it('ROADTRIP-CTL-001: the addon gate leads, and the auth and trip guards follow', () => {
+    const guards = Reflect.getMetadata(GUARDS_METADATA, RoadtripController) as unknown[];
+
+    // AddonGuard first, and present at all: @RequireAddon is metadata and inert
+    // without it, and the guard is not registered globally either. Without this
+    // line all seven routes answered on an instance that had the addon off,
+    // which is the bug this branch fixed once already.
+    expect(guards).toEqual([AddonGuard, JwtAuthGuard, TripAccessGuard]);
+    // It leads because a disabled addon owes an anonymous caller a 404 rather
+    // than a 401 that tells them the route exists.
+    expect(guards[0]).toBe(AddonGuard);
+  });
+});
+
+describe('RoadtripController — reads', () => {
+  it('ROADTRIP-CTL-002: the trip read hands back vias and tracks in one answer', () => {
+    const s = svc();
+    expect(new RoadtripController(s).listAll('7')).toEqual({ vias: [VIA], tracks: [] });
+    expect(s.listForTrip).toHaveBeenCalledWith('7');
+    expect(s.tracksForTrip).toHaveBeenCalledWith('7');
+  });
+
+  it('ROADTRIP-CTL-003: the day read checks the day belongs to this trip', () => {
+    const s = svc();
+    expect(new RoadtripController(s).list('7', '4')).toEqual({ vias: [VIA] });
+    expect(s.dayExists).toHaveBeenCalledWith('4', '7');
+  });
+
+  it('ROADTRIP-CTL-004: a day from another trip is not readable through this one', () => {
+    // The guard proves the caller may reach the trip; this proves the day is
+    // part of it. Without the check a valid day id from somebody else's trip is
+    // reachable by way of a trip the caller does have access to.
+    const s = svc({ dayExists: vi.fn().mockReturnValue(false) });
+    expect(thrown(() => new RoadtripController(s).list('7', '999')))
+      .toEqual({ status: 404, body: { error: 'Day not found' } });
+  });
+});
+
+describe('RoadtripController — writes', () => {
+  const body = { after_order_index: 0, lat: 53, lng: 10 };
+
+  it('ROADTRIP-CTL-005: creating one checks the day first', () => {
+    const s = svc();
+    expect(new RoadtripController(s).create('7', '4', body)).toEqual({ via: VIA });
+    expect(s.dayExists).toHaveBeenCalledWith('4', '7');
+    expect(s.create).toHaveBeenCalledWith('4', body);
+  });
+
+  it('ROADTRIP-CTL-006: every write refuses a day from another trip', () => {
+    const s = svc({ dayExists: vi.fn().mockReturnValue(false) });
+    const c = new RoadtripController(s);
+    const notFound = { status: 404, body: { error: 'Day not found' } };
+
+    expect(thrown(() => c.create('7', '999', body))).toEqual(notFound);
+    expect(thrown(() => c.createMany('7', '999', { vias: [body] }))).toEqual(notFound);
+    expect(thrown(() => c.reanchor('7', '999', { vias: [] }))).toEqual(notFound);
+    expect(thrown(() => c.update('7', '999', '5', { lat: 1, lng: 2 }))).toEqual(notFound);
+    expect(thrown(() => c.remove('7', '999', '5'))).toEqual(notFound);
+
+    // None of them reached the service.
+    expect(s.create).not.toHaveBeenCalled();
+    expect(s.createMany).not.toHaveBeenCalled();
+    expect(s.reanchor).not.toHaveBeenCalled();
+    expect(s.move).not.toHaveBeenCalled();
+    expect(s.remove).not.toHaveBeenCalled();
+  });
+
+  it('ROADTRIP-CTL-007: a batch lays the whole chain in one call', () => {
+    const s = svc();
+    const batch = { vias: [body, body], replace_legs: [0] };
+    expect(new RoadtripController(s).createMany('7', '4', batch)).toEqual({ vias: [VIA] });
+    expect(s.createMany).toHaveBeenCalledWith('4', batch);
+  });
+
+  it('ROADTRIP-CTL-008: a track from another trip cannot become this day label', () => {
+    // Permission is not enough on its own: a place id from somebody else's trip
+    // would otherwise become this day's label, and a place that is not a track
+    // would become a label that can never be drawn.
+    const s = svc({ trackExists: vi.fn().mockReturnValue(false) });
+    expect(thrown(() => new RoadtripController(s).createMany('7', '4', { vias: [body], track: { place_id: 3 } })))
+      .toEqual({ status: 404, body: { error: 'Track not found' } });
+    expect(s.createMany).not.toHaveBeenCalled();
+  });
+
+  it('ROADTRIP-CTL-009: the track check runs against the trip, not the day', () => {
+    const s = svc();
+    new RoadtripController(s).createMany('7', '4', { vias: [body], track: { place_id: 3 } });
+    expect(s.trackExists).toHaveBeenCalledWith(3, '7');
+  });
+
+  it('ROADTRIP-CTL-010: clearing a day track is not a track to look up', () => {
+    // `null` says the day follows nothing any more, so there is no place id to
+    // check the existence of. Treating it as one would refuse the only way to
+    // detach a track.
+    const s = svc({ trackExists: vi.fn().mockReturnValue(false) });
+    expect(new RoadtripController(s).createMany('7', '4', { vias: [body], track: null })).toEqual({ vias: [VIA] });
+    expect(s.trackExists).not.toHaveBeenCalled();
+  });
+
+  it('ROADTRIP-CTL-011: re-anchoring goes through as one batch', () => {
+    const s = svc();
+    const plan = { vias: [{ id: 5, after_order_index: 1 }], remove: [6] };
+    expect(new RoadtripController(s).reanchor('7', '4', plan)).toEqual({ vias: [VIA] });
+    expect(s.reanchor).toHaveBeenCalledWith('4', plan);
+  });
+
+  it('ROADTRIP-CTL-012: moving a via reports where it went', () => {
+    const s = svc();
+    expect(new RoadtripController(s).update('7', '4', '5', { lat: 54, lng: 11 })).toEqual({ via: VIA });
+    expect(s.move).toHaveBeenCalledWith('5', '4', 54, 11);
+  });
+
+  it('ROADTRIP-CTL-013: a via that is not on this day is a 404, not a silent no-op', () => {
+    const s = svc({ move: vi.fn().mockReturnValue(null) });
+    expect(thrown(() => new RoadtripController(s).update('7', '4', '999', { lat: 1, lng: 2 })))
+      .toEqual({ status: 404, body: { error: 'Via not found' } });
+  });
+
+  it('ROADTRIP-CTL-014: removing one reports success', () => {
+    const s = svc();
+    expect(new RoadtripController(s).remove('7', '4', '5')).toEqual({ success: true });
+    expect(s.remove).toHaveBeenCalledWith('5', '4');
+  });
+
+  it('ROADTRIP-CTL-015: removing a via that is not there is a 404', () => {
+    const s = svc({ remove: vi.fn().mockReturnValue(false) });
+    expect(thrown(() => new RoadtripController(s).remove('7', '4', '999')))
+      .toEqual({ status: 404, body: { error: 'Via not found' } });
+  });
+});
@@ -118,6 +118,14 @@ export default defineConfig({
        'src/nest/realtime/**/*.ts': { statements: 99, branches: 100, functions: 99, lines: 99 },
        'src/nest/reservation-import/**/*.ts': { statements: 61, branches: 55, functions: 41, lines: 61 },
        'src/nest/reservations/**/*.ts': { statements: 92, branches: 83, functions: 96, lines: 96 },
+        // New domain in this change. Measured over its own suites at
+        // 94.0/81.1/100/99.1 and pinned under that: the subset run does not
+        // load the controller the way the full run does, so the real figure is
+        // not this one. Regenerate with scripts/coverage-thresholds.mjs after a
+        // full run. What matters now is that it carries its own floor instead of
+        // hiding in the 80 catch-all, where 557 new lines could shed twenty
+        // points against the headroom of the domains beside them.
+        'src/nest/roadtrip/**/*.ts': { statements: 88, branches: 80, functions: 95, lines: 95 },
        'src/nest/settings/**/*.ts': { statements: 87, branches: 71, functions: 99, lines: 88 },
        'src/nest/share/**/*.ts': { statements: 97, branches: 87, functions: 99, lines: 99 },
        'src/nest/storage/**/*.ts': { statements: 94, branches: 84, functions: 97, lines: 94 },