feat(mcp): correct a journey entry, not just write one

location_name was settable when creating an entry and never afterwards,
which is the sharpest create-versus-update asymmetry in the surface.
Coordinates, weather, tags and the pros and cons verdict were persisted by
the insert but had no way in, so an entry written by an assistant never
placed on the journey map.
This commit is contained in:
Maurice
2026-08-28 13:07:39 +02:00
committed by Maurice
parent a5ac6b15e6
commit 922c60c167
2 changed files with 372 additions and 8 deletions
+84 -8
View File
@@ -45,6 +45,28 @@ function jsonContent(uri: string, data: unknown) {
};
}
/*
* The three entry fields with a fixed set of values. `journey_entries.type` and
* `.visibility` are plain TEXT columns and the REST route forwards whatever the
* client sent, but the row type (JourneyEntry in src/types) only ever models
* these, so the tools name them rather than take a free string: a model that
* invents "friends-only" would write a value nothing reads.
*/
const ENTRY_TYPE = z.enum(['entry', 'checkin', 'skeleton']);
const ENTRY_VISIBILITY = z.enum(['private', 'shared', 'public']);
type EntryType = z.infer<typeof ENTRY_TYPE>;
type EntryVisibility = z.infer<typeof ENTRY_VISIBILITY>;
/**
* The verdict on a place. Either side defaults to empty so a caller can send
* only the half it has, which is what the entry editor does when a place was
* all good or all bad. The service stores nothing when both come back empty.
*/
const PROS_CONS = z.object({
pros: z.array(z.string()).default([]),
cons: z.array(z.string()).default([]),
});
/**
* Journey MCP surface — ported 1:1 from the legacy registrar
* src/mcp/tools/journey.ts (23 tools): identical names, descriptions, zod input
@@ -55,6 +77,11 @@ function jsonContent(uri: string, data: unknown) {
* marker could not express, and folding it into a predicate would have put it
* out of reach of the boot-time scope gate. The registration-time addon
* early-return becomes the `when:` gate.
*
* Since the port the surface has grown past the legacy 23: the two entry tools
* take the rest of the columns their REST routes always accepted (a place, its
* coordinates, weather, tags, the verdict), and get_journey_stats answers what
* GET /api/journeys/:id/stats answers.
*/
@McpController()
export class JourneyMcp {
@@ -93,6 +120,28 @@ export class JourneyMcp {
return ok({ journey });
}
@Tool({
name: 'get_journey_stats',
description: 'What a journey adds up to: distance travelled in metres, calendar days spanned, countries in visit order, the furthest point reached, and entry, photo and place counts. Prefer this over get_journey whenever the question is about totals, since the stats get_journey carries are three counts and nothing else.',
inputSchema: {
journeyId: z.number().int().positive(),
include_route: z.boolean().optional().describe('Also return the route itself, up to 400 stops with coordinates. Off by default: the totals and the country list do not need it.'),
},
annotations: TOOL_ANNOTATIONS_READONLY,
when: journeyAddonOn,
access: { group: 'journey', mode: 'read' },
})
getJourneyStats({ journeyId, include_route }: { journeyId: number; include_route?: boolean }, ctx: McpContext) {
const stats = this.journey.journeyStats(journeyId, ctx.userId);
if (!stats) return notFound('Journey not found or access denied.');
// Same payload as GET /api/journeys/:id/stats. The route is left out unless
// it was asked for: 400 coordinate pairs answer a different question than
// the totals do, and Studio is the caller that actually draws them.
if (include_route) return ok({ stats });
const { points: _route, ...totals } = stats;
return ok({ stats: totals });
}
@Tool({
name: 'list_journey_entries',
description: 'List all entries in a journey.',
@@ -237,7 +286,7 @@ export class JourneyMcp {
@Tool({
name: 'create_journey_entry',
description: 'Create a new entry in a journey.',
description: 'Create a new entry in a journey. Give location_lat/location_lng whenever the place is known, otherwise the entry is text-only and never appears on the journey map or in its distance.',
inputSchema: {
journeyId: z.number().int().positive(),
entry_date: z.string().regex(/^\d{4}-\d{2}-\d{2}$/).describe('Entry date (YYYY-MM-DD)'),
@@ -245,7 +294,14 @@ export class JourneyMcp {
story: z.string().optional(),
entry_time: z.string().optional().describe('Time of day (e.g. "14:30")'),
location_name: z.string().optional(),
location_lat: z.number().min(-90).max(90).optional().describe('Latitude; needed, with location_lng, to place the entry on the journey map'),
location_lng: z.number().min(-180).max(180).optional(),
mood: z.string().optional(),
weather: z.string().max(100).optional().describe('Weather as the traveller recorded it (e.g. "sunny", "24C and windy")'),
tags: z.array(z.string()).optional(),
pros_cons: PROS_CONS.optional().describe('The verdict on the place: what was worth it and what was not'),
visibility: ENTRY_VISIBILITY.optional().describe('Defaults to private; "shared" and "public" expose the entry through the journey share link'),
type: ENTRY_TYPE.optional().describe('Defaults to "entry"; "skeleton" is the stub TREK derives from a trip place and hides behind the hide-skeletons preference'),
sort_order: z.number().int().min(0).optional(),
},
annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT,
@@ -253,7 +309,12 @@ export class JourneyMcp {
access: { group: 'journey', mode: 'write' },
})
createJourneyEntry(
{ journeyId, ...data }: { journeyId: number; entry_date: string; title?: string; story?: string; entry_time?: string; location_name?: string; mood?: string; sort_order?: number },
{ journeyId, ...data }: {
journeyId: number; entry_date: string; title?: string; story?: string; entry_time?: string;
location_name?: string; location_lat?: number; location_lng?: number; mood?: string; weather?: string;
tags?: string[]; pros_cons?: { pros: string[]; cons: string[] }; visibility?: EntryVisibility;
type?: EntryType; sort_order?: number;
},
ctx: McpContext,
) {
if (this.auth.isDemoUser(ctx.userId)) return demoDenied();
@@ -266,21 +327,36 @@ export class JourneyMcp {
@Tool({
name: 'update_journey_entry',
description: 'Update an existing journey entry.',
description: 'Update an existing journey entry: its text, date, place, coordinates, weather, tags, verdict or visibility. Fields left out keep their value; pass null to clear one. To move an entry within its day use reorder_journey_entries rather than setting sort_order here.',
inputSchema: {
entryId: z.number().int().positive(),
title: z.string().max(300).optional(),
story: z.string().optional(),
title: z.string().max(300).nullable().optional(),
story: z.string().nullable().optional(),
entry_date: z.string().regex(/^\d{4}-\d{2}-\d{2}$/).optional(),
entry_time: z.string().optional(),
mood: z.string().optional(),
entry_time: z.string().nullable().optional(),
location_name: z.string().nullable().optional(),
location_lat: z.number().min(-90).max(90).nullable().optional().describe('Latitude, or null to take the entry off the journey map'),
location_lng: z.number().min(-180).max(180).nullable().optional(),
mood: z.string().nullable().optional(),
weather: z.string().max(100).nullable().optional(),
tags: z.array(z.string()).nullable().optional(),
pros_cons: PROS_CONS.nullable().optional().describe('Replaces the whole verdict; null clears it'),
visibility: ENTRY_VISIBILITY.optional(),
type: ENTRY_TYPE.optional().describe('Promote a trip-derived "skeleton" to a real "entry" once it has been written up'),
sort_order: z.number().int().min(0).optional(),
},
annotations: TOOL_ANNOTATIONS_WRITE,
when: journeyAddonOn,
access: { group: 'journey', mode: 'write' },
})
updateJourneyEntry(
{ entryId, ...data }: { entryId: number; title?: string; story?: string; entry_date?: string; entry_time?: string; mood?: string },
{ entryId, ...data }: {
entryId: number; title?: string | null; story?: string | null; entry_date?: string;
entry_time?: string | null; location_name?: string | null; location_lat?: number | null;
location_lng?: number | null; mood?: string | null; weather?: string | null;
tags?: string[] | null; pros_cons?: { pros: string[]; cons: string[] } | null;
visibility?: EntryVisibility; type?: EntryType; sort_order?: number;
},
ctx: McpContext,
) {
if (this.auth.isDemoUser(ctx.userId)) return demoDenied();
+288
View File
@@ -34,6 +34,23 @@ vi.mock('../../../src/config', () => ({
const { broadcastMock } = vi.hoisted(() => ({ broadcastMock: vi.fn() }));
vi.mock('../../../src/websocket', () => ({ broadcast: broadcastMock, broadcastToUser: broadcastMock }));
/*
* get_journey_stats resolves a country per stop. The real lookup loads and
* indexes 4MB of gzipped admin-0 boundaries on first call, which is Atlas'
* work and has its own tests (journey-stats.service.test.ts stubs it for the
* same reason). Only this one export is replaced: AtlasService is constructed
* by the harness and imports the rest of the module for real.
*/
vi.mock('../../../src/nest/atlas/atlas-geo', async (importOriginal) => ({
...(await importOriginal<Record<string, unknown>>()),
// Rough boxes, enough to tell two countries apart in a fixture.
getCountryFromCoords: (lat: number, lng: number) => {
if (lat > 47 && lat < 55 && lng > 5 && lng < 15) return 'DE';
if (lat > 42 && lat < 51 && lng > -5 && lng < 8) return 'FR';
return null;
},
}));
import { createTables } from '../../../src/db/schema';
import { runMigrations } from '../../../src/db/migrations';
import { resetTestDb } from '../../helpers/test-db';
@@ -413,6 +430,277 @@ describe('journey write tools', () => {
});
});
// ---------------------------------------------------------------------------
// The entry columns the REST route has always accepted. The INSERT writes a
// place, its coordinates, weather, tags, the verdict, the visibility and the
// type; the UPDATE allow-list carries all of them plus sort_order. The tools
// took a location_name at create and nothing at all after that, so an entry
// written through MCP never landed on the journey map and a wrong place name
// could not be corrected. Asserted against the row, not the tool's echo.
// ---------------------------------------------------------------------------
function entryRow(id: number) {
return testDb.prepare('SELECT * FROM journey_entries WHERE id = ?').get(id) as any;
}
async function seedEntry(h: McpHarness, journeyId: number, args: Record<string, unknown> = {}) {
return (parseToolResult(await h.client.callTool({
name: 'create_journey_entry',
arguments: { journeyId, entry_date: '2026-07-01', ...args },
})) as any).entry;
}
describe('journey entry fields', () => {
it('create_journey_entry persists the place, coordinates, weather, tags, verdict, visibility and type', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const journey = await seedJourney(h);
const entry = await seedEntry(h, journey.id, {
title: 'Reykjavik',
location_name: 'Hallgrimskirkja',
location_lat: 64.1418,
location_lng: -21.9266,
weather: 'overcast, 9C',
tags: ['church', 'view'],
pros_cons: { pros: ['the tower'], cons: ['the queue'] },
visibility: 'public',
type: 'checkin',
});
const row = entryRow(entry.id);
expect(row.location_name).toBe('Hallgrimskirkja');
expect(row.location_lat).toBeCloseTo(64.1418, 4);
expect(row.location_lng).toBeCloseTo(-21.9266, 4);
expect(row.weather).toBe('overcast, 9C');
expect(JSON.parse(row.tags)).toEqual(['church', 'view']);
expect(JSON.parse(row.pros_cons)).toEqual({ pros: ['the tower'], cons: ['the queue'] });
expect(row.visibility).toBe('public');
expect(row.type).toBe('checkin');
});
});
it('create_journey_entry accepts a one-sided verdict and fills the other side in', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const journey = await seedJourney(h);
const entry = await seedEntry(h, journey.id, { pros_cons: { pros: ['cheap'] } });
expect(JSON.parse(entryRow(entry.id).pros_cons)).toEqual({ pros: ['cheap'], cons: [] });
});
});
it('create_journey_entry defaults to a private entry when neither is given', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const journey = await seedJourney(h);
const row = entryRow((await seedEntry(h, journey.id)).id);
expect(row.visibility).toBe('private');
expect(row.type).toBe('entry');
});
});
it('create_journey_entry refuses coordinates off the globe and an unknown visibility', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const journey = await seedJourney(h);
for (const bad of [{ location_lat: 91 }, { location_lng: -181 }, { visibility: 'friends' }]) {
expect((await h.client.callTool({
name: 'create_journey_entry',
arguments: { journeyId: journey.id, entry_date: '2026-07-01', ...bad },
})).isError).toBe(true);
}
});
});
it('update_journey_entry corrects the place and its coordinates', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const journey = await seedJourney(h);
const entry = await seedEntry(h, journey.id, {
location_name: 'Hallgrimskirja', location_lat: 64.0, location_lng: -21.0,
});
await h.client.callTool({
name: 'update_journey_entry',
arguments: {
entryId: entry.id,
location_name: 'Hallgrimskirkja',
location_lat: 64.1418,
location_lng: -21.9266,
},
});
const row = entryRow(entry.id);
expect(row.location_name).toBe('Hallgrimskirkja');
expect(row.location_lat).toBeCloseTo(64.1418, 4);
expect(row.location_lng).toBeCloseTo(-21.9266, 4);
});
});
it('update_journey_entry sets weather, tags, the verdict, visibility and sort_order', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const journey = await seedJourney(h);
const entry = await seedEntry(h, journey.id);
const data = parseToolResult(await h.client.callTool({
name: 'update_journey_entry',
arguments: {
entryId: entry.id,
weather: 'rain all day',
tags: ['museum'],
pros_cons: { pros: ['free on Sundays'], cons: ['packed'] },
visibility: 'shared',
sort_order: 3,
},
})) as any;
const row = entryRow(entry.id);
expect(row.weather).toBe('rain all day');
expect(JSON.parse(row.tags)).toEqual(['museum']);
expect(JSON.parse(row.pros_cons)).toEqual({ pros: ['free on Sundays'], cons: ['packed'] });
expect(row.visibility).toBe('shared');
expect(row.sort_order).toBe(3);
// The enrichment parses both back out, so the caller sees the objects.
expect(data.entry.tags).toEqual(['museum']);
expect(data.entry.pros_cons).toEqual({ pros: ['free on Sundays'], cons: ['packed'] });
});
});
it('update_journey_entry promotes a trip-derived skeleton to a real entry', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const journey = await seedJourney(h);
const entry = await seedEntry(h, journey.id, { type: 'skeleton', title: 'Blue Lagoon' });
expect(entryRow(entry.id).type).toBe('skeleton');
await h.client.callTool({ name: 'update_journey_entry', arguments: { entryId: entry.id, type: 'entry' } });
expect(entryRow(entry.id).type).toBe('entry');
});
});
it('update_journey_entry clears a field on null and leaves the omitted ones alone', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const journey = await seedJourney(h);
const entry = await seedEntry(h, journey.id, {
title: 'Vik',
entry_time: '14:30',
location_name: 'Reynisfjara',
location_lat: 63.4,
location_lng: -19.04,
weather: 'gale',
tags: ['beach'],
pros_cons: { pros: ['the stacks'], cons: ['sneaker waves'] },
});
await h.client.callTool({
name: 'update_journey_entry',
arguments: {
entryId: entry.id,
entry_time: null,
location_lat: null,
location_lng: null,
weather: null,
tags: null,
pros_cons: null,
},
});
const row = entryRow(entry.id);
expect(row.entry_time).toBeNull();
expect(row.location_lat).toBeNull();
expect(row.location_lng).toBeNull();
expect(row.weather).toBeNull();
expect(row.tags).toBeNull();
expect(row.pros_cons).toBeNull();
// Untouched: the service only writes the keys it was handed.
expect(row.title).toBe('Vik');
expect(row.location_name).toBe('Reynisfjara');
});
});
it('update_journey_entry refuses an unknown visibility and an out-of-range longitude', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const journey = await seedJourney(h);
const entry = await seedEntry(h, journey.id);
for (const bad of [{ visibility: 'friends' }, { location_lng: 181 }, { type: 'note' }]) {
expect((await h.client.callTool({
name: 'update_journey_entry', arguments: { entryId: entry.id, ...bad },
})).isError).toBe(true);
}
// And none of the refusals touched the row.
const row = entryRow(entry.id);
expect(row.visibility).toBe('private');
expect(row.location_lng).toBeNull();
});
});
});
describe('Tool: get_journey_stats', () => {
async function twoStopJourney(h: McpHarness) {
const journey = await seedJourney(h);
await seedEntry(h, journey.id, { title: 'Paris', entry_date: '2026-07-01', location_lat: 48.85, location_lng: 2.35 });
await seedEntry(h, journey.id, { title: 'Berlin', entry_date: '2026-07-04', location_lat: 52.52, location_lng: 13.4 });
return journey;
}
it('answers with the distance, days and countries get_journey does not carry', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const journey = await twoStopJourney(h);
const data = parseToolResult(await h.client.callTool({
name: 'get_journey_stats', arguments: { journeyId: journey.id },
})) as any;
// Paris to Berlin is about 880km great-circle, and the figure is metres.
expect(data.stats.distance).toBeGreaterThan(800_000);
expect(data.stats.distance).toBeLessThan(950_000);
expect(data.stats.days).toBe(4);
expect(data.stats.steps).toBe(2);
expect(data.stats.countries.map((c: any) => c.code)).toEqual(['FR', 'DE']);
expect(data.stats.start).toBe('2026-07-01');
expect(data.stats.end).toBe('2026-07-04');
// get_journey still answers with the three counts and nothing more.
const full = parseToolResult(await h.client.callTool({
name: 'get_journey', arguments: { journeyId: journey.id },
})) as any;
expect(Object.keys(full.journey.stats).sort()).toEqual(['entries', 'photos', 'places']);
});
});
it('leaves the route out unless include_route asks for it', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const journey = await twoStopJourney(h);
const lean = parseToolResult(await h.client.callTool({
name: 'get_journey_stats', arguments: { journeyId: journey.id },
})) as any;
expect(lean.stats.points).toBeUndefined();
const full = parseToolResult(await h.client.callTool({
name: 'get_journey_stats', arguments: { journeyId: journey.id, include_route: true },
})) as any;
expect(full.stats.points).toHaveLength(2);
expect(full.stats.points.map((p: any) => p.label)).toEqual(['Paris', 'Berlin']);
expect(full.stats.points[0].country).toBe('FR');
});
});
it('refuses a journey the caller cannot see', async () => {
const { user } = createUser(testDb);
const { journeyId } = foreignJourney();
await withHarness(user.id, async (h) => {
expect((await h.client.callTool({
name: 'get_journey_stats', arguments: { journeyId },
})).isError).toBe(true);
});
});
});
describe('journey share-link tools', () => {
it('creates, reads and revokes the link', async () => {
const { user } = createUser(testDb);