From f4533fc69bbeec68122a213ccb868bc306d8118b Mon Sep 17 00:00:00 2001 From: Maurice Date: Tue, 20 Oct 2026 12:53:40 +0200 Subject: [PATCH] feat(tours): save_tour_change saves a confirmed change to a city walk whole or not at all An assistant can now save the change prepare_tour_change prepared, once the traveller said yes to it exactly as shown. Until now a proposal could only be looked at; the traveller had to redo it by hand in the planner. - The save names the proposal: the same changes, the revision it was prepared on, the proposal_id the preparation answered and confirmed true. Anything else, a change altered on the way or no yes, is refused and nothing is saved. - Place editing on the trip and the tour are checked again before anything is written. A tour changed since the proposal was prepared is a conflict on the tour's one revision (no second one), telling the assistant to prepare the change anew instead of writing over newer edits. The changes are laid on the saved tour once more inside the write, so a station visited or skipped meanwhile, or a place of the trip gone, refuses the save with every reason. - The change goes through the write the planner's save uses, in one transaction: line, stations, keys, measures and visits together or not at all, and the trip is told only after the commit. Stations kept keep their keys and visits. Where the router gives no way on foot the walk keeps its stations only and waits for the planner, as a created one does. - Every save is kept as a step of the tour (tour_change_steps): the proposal, the revision it was prepared on and the one it raised the tour to, the changes and the tour as it was before them, so the step can be followed and taken back later. Nothing of the conversation is kept. - A new walk has no revision to name and stays with create_city_walk; both tool descriptions and the wiki say which is which. --- server/scripts/repository-owners.json | 1 + .../src/db/entities/TourChangeSteps.entity.ts | 40 ++ server/src/db/entities/Tours.entity.ts | 3 + server/src/db/entities/index.ts | 2 + server/src/db/json-columns.ts | 49 +++ server/src/db/kysely/TourChangeSteps.table.ts | 15 + server/src/db/kysely/db.ts | 2 + ...cords_the_changes_saved_from_a_proposal.ts | 34 ++ .../TourChangeSteps.repository.ts | 38 ++ server/src/nest/tours/city-walk.mcp.ts | 2 +- server/src/nest/tours/tour-change-effects.ts | 2 +- .../nest/tours/tour-change-preview.service.ts | 26 +- server/src/nest/tours/tour-change-proposal.ts | 41 ++ .../nest/tours/tour-change-save.service.ts | 171 ++++++++ server/src/nest/tours/tour-change.mcp.ts | 62 ++- server/src/nest/tours/tour-revision.ts | 14 +- server/src/nest/tours/tours.module.ts | 8 +- server/src/nest/tours/tours.service.ts | 100 +++-- server/tests/unit/db/entities-index.test.ts | 7 +- .../tests/unit/db/entity-class-fields.test.ts | 4 +- server/tests/unit/db/legacy-baseline.test.ts | 1 + .../TourChangeSteps.repository.test.ts | 96 ++++ .../unit/nest/tour-change-save.mcp.test.ts | 409 ++++++++++++++++++ server/tests/unit/utils/json-column.test.ts | 23 + shared/src/tours/tour-change.schema.spec.ts | 37 +- shared/src/tours/tour-change.schema.ts | 30 +- wiki/MCP-Addon-Tools.md | 5 +- 27 files changed, 1171 insertions(+), 51 deletions(-) create mode 100644 server/src/db/entities/TourChangeSteps.entity.ts create mode 100644 server/src/db/kysely/TourChangeSteps.table.ts create mode 100644 server/src/db/migrations/Migration20200101045700_a_tour_records_the_changes_saved_from_a_proposal.ts create mode 100644 server/src/db/repositories/TourChangeSteps.repository.ts create mode 100644 server/src/nest/tours/tour-change-proposal.ts create mode 100644 server/src/nest/tours/tour-change-save.service.ts create mode 100644 server/tests/unit/db/repositories/TourChangeSteps.repository.test.ts create mode 100644 server/tests/unit/nest/tour-change-save.mcp.test.ts diff --git a/server/scripts/repository-owners.json b/server/scripts/repository-owners.json index 88edca426..2849a19b5 100644 --- a/server/scripts/repository-owners.json +++ b/server/scripts/repository-owners.json @@ -108,6 +108,7 @@ "Tags": "tags", "TodoCategoryAssignees": "todo", "TodoItems": "todo", + "TourChangeSteps": "tours", "TourObstacleReviews": "tours", "TourStationVisits": "tours", "TourSources": "tours", diff --git a/server/src/db/entities/TourChangeSteps.entity.ts b/server/src/db/entities/TourChangeSteps.entity.ts new file mode 100644 index 000000000..64cdf9a48 --- /dev/null +++ b/server/src/db/entities/TourChangeSteps.entity.ts @@ -0,0 +1,40 @@ +import { EntityRepositoryType, type Opt, type Ref, defineEntity, p } from '@mikro-orm/core'; +import { TourChangeStepsRepository } from '../repositories/TourChangeSteps.repository'; +import { DbTimestampType } from '../types'; +import { Tours } from './Tours.entity'; + +export class TourChangeSteps { + [EntityRepositoryType]?: TourChangeStepsRepository; + id!: number & Opt; + place!: Ref; + place_id!: number; + proposal_id!: string; + base_revision!: number; + revision!: number; + changes!: string; + before_state!: string; + saved_by?: number | null; + saved_at!: string & Opt; +} + +export const TourChangeStepsSchema = defineEntity({ + class: TourChangeSteps, + repository: () => TourChangeStepsRepository, + uniques: [{ properties: ['place', 'revision'] }], + checks: [ + { name: 'tour_change_steps_base_revision_check', expression: 'base_revision >= 1' }, + { name: 'tour_change_steps_revision_check', expression: 'revision = base_revision + 1' }, + ], + properties: { + id: p.integer().primary(), + place: () => p.manyToOne(Tours).ref().name('place_id').deleteRule('cascade').hidden(), + place_id: p.integer().persist(false), + proposal_id: p.text(), + base_revision: p.integer(), + revision: p.integer(), + changes: p.text(), + before_state: p.text(), + saved_by: p.integer().nullable(), + saved_at: p.type(DbTimestampType).defaultRaw(`CURRENT_TIMESTAMP`), + }, +}); diff --git a/server/src/db/entities/Tours.entity.ts b/server/src/db/entities/Tours.entity.ts index 44811119d..abc216209 100644 --- a/server/src/db/entities/Tours.entity.ts +++ b/server/src/db/entities/Tours.entity.ts @@ -1,6 +1,7 @@ import { Collection, EntityRepositoryType, type Opt, PrimaryKeyProp, type Ref, defineEntity, p } from '@mikro-orm/core'; import { ToursRepository } from '../repositories/Tours.repository'; import { Places } from './Places.entity'; +import { TourChangeSteps } from './TourChangeSteps.entity'; import { TourObstacleReviews } from './TourObstacleReviews.entity'; import { TourSources } from './TourSources.entity'; import { TourTypes } from './TourTypes.entity'; @@ -45,6 +46,7 @@ export class Tours { ski_discipline?: TToursSkiDiscipline | null; ski_technique?: TToursSkiTechnique | null; tourTypeRef!: Ref; + tour_change_steps_collection = new Collection(this); tour_obstacle_reviews_collection = new Collection(this); tour_sources: Ref | null = null; tour_waypoints_collection = new Collection(this); @@ -175,6 +177,7 @@ export const ToursSchema = defineEntity({ ski_discipline: p.enum(() => ToursSkiDiscipline).nullable(), ski_technique: p.enum(() => ToursSkiTechnique).nullable(), tourTypeRef: () => p.manyToOne(TourTypes).ref().joinColumn('tour_type').hidden().index('idx_tours_tour_type'), + tour_change_steps_collection: () => p.oneToMany(TourChangeSteps).mappedBy('place').hidden(), tour_obstacle_reviews_collection: () => p.oneToMany(TourObstacleReviews).mappedBy('place').hidden(), tour_sources: () => p.oneToOne(TourSources).ref().mappedBy('place').hidden(), tour_waypoints_collection: () => p.oneToMany(TourWaypoints).mappedBy('place').hidden(), diff --git a/server/src/db/entities/index.ts b/server/src/db/entities/index.ts index 1c54d06bd..f1472b892 100644 --- a/server/src/db/entities/index.ts +++ b/server/src/db/entities/index.ts @@ -107,6 +107,7 @@ import { ShareTokensSchema } from './ShareTokens.entity'; import { TagsSchema } from './Tags.entity'; import { TodoCategoryAssigneesSchema } from './TodoCategoryAssignees.entity'; import { TodoItemsSchema } from './TodoItems.entity'; +import { TourChangeStepsSchema } from './TourChangeSteps.entity'; import { TourObstacleReviewsSchema } from './TourObstacleReviews.entity'; import { TourSourcesSchema } from './TourSources.entity'; import { TourStationVisitsSchema } from './TourStationVisits.entity'; @@ -252,6 +253,7 @@ export const ALL_ENTITIES: readonly EntitySchema[] = [ TagsSchema, TodoCategoryAssigneesSchema, TodoItemsSchema, + TourChangeStepsSchema, TourObstacleReviewsSchema, TourSourcesSchema, TourStationVisitsSchema, diff --git a/server/src/db/json-columns.ts b/server/src/db/json-columns.ts index 84f222552..890b8984e 100644 --- a/server/src/db/json-columns.ts +++ b/server/src/db/json-columns.ts @@ -8,16 +8,21 @@ import { paddleLockSchema, paddleRegionBoundarySchema, paddleRegionGraphStatsSchema, + tourChangeSchema, tourFixedLegSchema, + tourWaypointSchema, trackMarksSchema, type Ferrata, + type GeometryPoint, type HikingPreferences, type ObstacleSnapshot, type PaddleGeometry, type PaddleLock, type PaddleRegionBoundary, type PaddleRegionGraphStats, + type TourChange, type TourFixedLeg, + type TourWaypoint, type TrackMarks, } from '@trek/shared'; @@ -218,3 +223,47 @@ export const TOUR_OBSTACLE_SNAPSHOT: JsonColumn = { schema: obstacleSnapshotSchema.nullable(), fallback: () => null, }; + +/** + * MCP-T03: the changes a saved step confirmed, as they were prepared. A broken + * row reads as none, so a step never claims changes it cannot show. + */ +export const TOUR_CHANGE_STEP_CHANGES: JsonColumn = { + column: 'tour_change_steps.changes', + schema: z.array(tourChangeSchema), + fallback: () => [], +}; + +/** A point of a saved line: latitude, longitude and the height where it has one. */ +const savedLinePoint = z.union([z.tuple([z.number(), z.number()]), z.tuple([z.number(), z.number(), z.number()])]); + +/** A tour as it was before a saved step (MCP-T03), as its JSON column holds it. */ +export interface TourChangeStepBefore { + route_geometry: GeometryPoint[] | null; + waypoints: TourWaypoint[]; + distance: number | null; + duration: number | null; + elevation_gain: number | null; + elevation_loss: number | null; +} + +/** + * MCP-T03: a tour as it was before a saved step changed it: its line (null + * while it had none), its points with their keys, and the measures the save + * replaced, null where they were not known. A broken row reads as no earlier + * state, so nothing is ever restored from a guess. + */ +export const TOUR_CHANGE_STEP_BEFORE: JsonColumn = { + column: 'tour_change_steps.before_state', + schema: z + .object({ + route_geometry: z.array(savedLinePoint).nullable(), + waypoints: z.array(tourWaypointSchema), + distance: z.number().nullable(), + duration: z.number().nullable(), + elevation_gain: z.number().nullable(), + elevation_loss: z.number().nullable(), + }) + .nullable(), + fallback: () => null, +}; diff --git a/server/src/db/kysely/TourChangeSteps.table.ts b/server/src/db/kysely/TourChangeSteps.table.ts new file mode 100644 index 000000000..c9db7254d --- /dev/null +++ b/server/src/db/kysely/TourChangeSteps.table.ts @@ -0,0 +1,15 @@ +// Generated by scripts/generate-db-types.ts from the migrated schema. Do not edit by hand: +// change the migration, then run `npm run gen:db-types`. +import type { InsertOptional } from './columns'; + +export interface TourChangeStepsTable { + id: InsertOptional; + place_id: number; + proposal_id: string; + base_revision: number; + revision: number; + changes: string; + before_state: string; + saved_by: InsertOptional; + saved_at: InsertOptional; +} diff --git a/server/src/db/kysely/db.ts b/server/src/db/kysely/db.ts index 525206197..fe15b81ba 100644 --- a/server/src/db/kysely/db.ts +++ b/server/src/db/kysely/db.ts @@ -113,6 +113,7 @@ import type { ShareTokensTable } from './ShareTokens.table'; import type { TagsTable } from './Tags.table'; import type { TodoCategoryAssigneesTable } from './TodoCategoryAssignees.table'; import type { TodoItemsTable } from './TodoItems.table'; +import type { TourChangeStepsTable } from './TourChangeSteps.table'; import type { TourObstacleReviewsTable } from './TourObstacleReviews.table'; import type { TourSourcesTable } from './TourSources.table'; import type { TourStationVisitsTable } from './TourStationVisits.table'; @@ -264,6 +265,7 @@ export interface DB { tags: TagsTable; todo_category_assignees: TodoCategoryAssigneesTable; todo_items: TodoItemsTable; + tour_change_steps: TourChangeStepsTable; tour_obstacle_reviews: TourObstacleReviewsTable; tour_sources: TourSourcesTable; tour_station_visits: TourStationVisitsTable; diff --git a/server/src/db/migrations/Migration20200101045700_a_tour_records_the_changes_saved_from_a_proposal.ts b/server/src/db/migrations/Migration20200101045700_a_tour_records_the_changes_saved_from_a_proposal.ts new file mode 100644 index 000000000..f45bf650a --- /dev/null +++ b/server/src/db/migrations/Migration20200101045700_a_tour_records_the_changes_saved_from_a_proposal.ts @@ -0,0 +1,34 @@ +import { Migration } from '@mikro-orm/migrations'; + +/** + * MCP-T03: a change to a saved tour the traveller confirmed through an + * assistant is kept as one step of the tour, so it can be followed and taken + * back later (MCP-T04). A step names the proposal it saved (`proposal_id`), + * the revision the tour was at when it was prepared (`base_revision`) and the + * one the save raised it to (`revision`, always the next), the changes as + * confirmed (JSON) and the tour as it was before them (`before_state`, JSON: + * its line, its points and the measures the save replaced). Nothing of the + * conversation is kept, only the plan. `saved_by` names who confirmed it, + * kept as a plain id: a step outlives its author, who then reads as unknown. + * One step per revision of a tour, and the steps go with their tour. + */ +export class Migration20200101045700_a_tour_records_the_changes_saved_from_a_proposal extends Migration { + override name = 'Migration20200101045700_a_tour_records_the_changes_saved_from_a_proposal'; + + override up(): void { + this.addSql(` + CREATE TABLE IF NOT EXISTS tour_change_steps ( + id INTEGER PRIMARY KEY, + place_id INTEGER NOT NULL REFERENCES tours(place_id) ON DELETE CASCADE, + proposal_id TEXT NOT NULL, + base_revision INTEGER NOT NULL CHECK (base_revision >= 1), + revision INTEGER NOT NULL CHECK (revision = base_revision + 1), + changes TEXT NOT NULL, + before_state TEXT NOT NULL, + saved_by INTEGER, + saved_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + UNIQUE (place_id, revision) + ) + `); + } +} diff --git a/server/src/db/repositories/TourChangeSteps.repository.ts b/server/src/db/repositories/TourChangeSteps.repository.ts new file mode 100644 index 000000000..e40b9ca1c --- /dev/null +++ b/server/src/db/repositories/TourChangeSteps.repository.ts @@ -0,0 +1,38 @@ +import type { TourChangeSteps } from '../entities/TourChangeSteps.entity'; +import type { DB } from '../kysely/db'; +import { TrekRepository } from './_shared/trek-repository'; + +type StepsDB = Pick; + +/** What a save of a confirmed change records; `changes` and `before_state` are their JSON text. */ +export interface TourChangeStepWrite { + place_id: number; + proposal_id: string; + base_revision: number; + revision: number; + changes: string; + before_state: string; + saved_by: number | null; +} + +/** + * `tour_change_steps` (MCP-T03): the changes to a tour saved from a proposal + * the traveller confirmed, one step per revision they raised the tour to, + * each with the tour as it was before, so a step can be followed and taken + * back (MCP-T04). A step goes with its tour. + */ +export class TourChangeStepsRepository extends TrekRepository { + private db() { + return this.kysely(); + } + + /** Records one saved step and answers its id; the caller holds the transaction of the save. */ + async record(step: TourChangeStepWrite): Promise { + const inserted = await this.db() + .insertInto('tour_change_steps') + .values(step) + .returning('id') + .executeTakeFirstOrThrow(); + return inserted.id; + } +} diff --git a/server/src/nest/tours/city-walk.mcp.ts b/server/src/nest/tours/city-walk.mcp.ts index a045cb231..8366ee766 100644 --- a/server/src/nest/tours/city-walk.mcp.ts +++ b/server/src/nest/tours/city-walk.mcp.ts @@ -60,7 +60,7 @@ export class CityWalkMcp { @Tool({ name: 'create_city_walk', description: - 'Create a city walk in a trip from stations the traveller confirmed (for instance stations_for_create of propose_city_walk), the way the planner saves a new walk: a new tour of type walk with its stations in their order, each with its name, stay_minutes (0 for a point passed through, null when not known), importance (optional, must, or null for normal), protected (a station any later proposal has to keep where it is), opening_hours as OpenStreetMap writes them and the place of the trip it stands for (linked_place_id; a place that is not of this trip leaves the station unlinked, a tour is refused). The way between the stations is planned on foot by the router the proposal uses, and the walk is saved with it, its distance and its walking time (foot_way planned true). Where the router gives no way (foot_way planned false, with why and position as in the estimate of the proposal), the walk is saved with its stations only (route_pending true, distance and walking time null), and TREK plans the way on foot when someone opens it in the planner and saves it; no straight line is stored as a route. Put it on a day with assign_place_to_day and give it a start time with update_assignment_time. Needs place editing permission on the trip. Create it only after the traveller said yes.', + 'Create a city walk in a trip from stations the traveller confirmed (for instance stations_for_create of propose_city_walk), the way the planner saves a new walk: a new tour of type walk with its stations in their order, each with its name, stay_minutes (0 for a point passed through, null when not known), importance (optional, must, or null for normal), protected (a station any later proposal has to keep where it is), opening_hours as OpenStreetMap writes them and the place of the trip it stands for (linked_place_id; a place that is not of this trip leaves the station unlinked, a tour is refused). The way between the stations is planned on foot by the router the proposal uses, and the walk is saved with it, its distance and its walking time (foot_way planned true). Where the router gives no way (foot_way planned false, with why and position as in the estimate of the proposal), the walk is saved with its stations only (route_pending true, distance and walking time null), and TREK plans the way on foot when someone opens it in the planner and saves it; no straight line is stored as a route. Put it on a day with assign_place_to_day and give it a start time with update_assignment_time. Needs place editing permission on the trip. Create it only after the traveller said yes. A change to a walk already saved is prepared with prepare_tour_change and saved with save_tour_change instead.', inputSchema: { tripId: idSchema.describe('The trip to create the walk in'), name: create.name, diff --git a/server/src/nest/tours/tour-change-effects.ts b/server/src/nest/tours/tour-change-effects.ts index 79ce0101d..b32073199 100644 --- a/server/src/nest/tours/tour-change-effects.ts +++ b/server/src/nest/tours/tour-change-effects.ts @@ -160,7 +160,7 @@ function warningsOf(gone: readonly DraftPoint[]) { } /** The points as a save takes them: a kept point with its key, a new one without, the server gives it one. */ -function asWaypoint(draft: DraftPoint): TourWaypoint { +export function asWaypoint(draft: DraftPoint): TourWaypoint { const { ref: _ref, origin: _origin, change: _change, after: _after, replaces: _replaces, ...point } = draft; return point; } diff --git a/server/src/nest/tours/tour-change-preview.service.ts b/server/src/nest/tours/tour-change-preview.service.ts index f4bde1869..699c1b120 100644 --- a/server/src/nest/tours/tour-change-preview.service.ts +++ b/server/src/nest/tours/tour-change-preview.service.ts @@ -6,6 +6,7 @@ import { DomainError } from '../common/domain-error'; import { CityWalkFootWayService } from './city-walk-foot-way.service'; import { applyChanges, refusalText, type StationDone } from './tour-change-apply'; import { describeChange } from './tour-change-effects'; +import { proposalIdOf, staleProposal } from './tour-change-proposal'; import { ToursService } from './tours.service'; import { InjectRepository } from '@mikro-orm/nestjs'; import { Injectable } from '@nestjs/common'; @@ -39,7 +40,8 @@ function doneByKey(visits: ReadonlyArray) * plans before and after. It writes nothing: no tour, station, visit or day * plan changes, and turning the proposal down leaves the plan as it is. The * proposal names the revision it was prepared on, so a save of it can be - * refused once the tour has moved on. + * refused once the tour has moved on, and carries the id a save of it has to + * name (MCP-T03). */ @Injectable() export class TourChangePreviewService { @@ -50,8 +52,19 @@ export class TourChangePreviewService { @InjectRepository(TourWaypoints) private readonly waypointsRepo: TourWaypointsRepository, ) {} - async prepare(tripId: number, placeId: number, userId: number, { changes }: TourChangeRequest) { + /** + * The changes laid onto the tour as it is saved now, with the visits of all + * its days and the places the new stations name, or the refusal naming + * every change the rules of the tour do not allow. A save lays them again + * on the tour it is about to write, so it is held to the same rules, and + * names the revision they were prepared on: a tour that moved past it is a + * conflict before any rule is asked. + */ + async lay(tripId: number, placeId: number, changes: readonly TourChange[], preparedOn?: number) { const { tour, waypoints } = await this.tours.getTour(String(tripId), String(placeId)); + if (preparedOn !== undefined && tour.revision !== preparedOn) { + throw staleProposal(preparedOn, tour.revision ?? null); + } const [visits, linkable] = await Promise.all([ this.visitsRepo.listForPlace(tripId, placeId), this.waypointsRepo.linkablePlaceIds(tripId, linkedIds(changes)), @@ -72,13 +85,20 @@ export class TourChangePreviewService { details: { refused: outcome.refused }, }); } + return { tour, waypoints, outcome }; + } + + async prepare(tripId: number, placeId: number, userId: number, { changes }: TourChangeRequest) { + const { tour, waypoints, outcome } = await this.lay(tripId, placeId, changes); const [wayBefore, wayAfter] = await Promise.all([ this.footWays.plan(userId, tripId, waypoints), this.footWays.plan(userId, tripId, outcome.points), ]); + const revision = tour.revision ?? null; return { prepared_only: true, - base: { place_id: tour.place_id, name: tour.name, tour_type: tour.tour_type, revision: tour.revision ?? null }, + proposal_id: proposalIdOf({ tripId, placeId, revision, changes }), + base: { place_id: tour.place_id, name: tour.name, tour_type: tour.tour_type, revision }, changes, ...describeChange({ saved: waypoints, diff --git a/server/src/nest/tours/tour-change-proposal.ts b/server/src/nest/tours/tour-change-proposal.ts new file mode 100644 index 000000000..4e39f771d --- /dev/null +++ b/server/src/nest/tours/tour-change-proposal.ts @@ -0,0 +1,41 @@ +import type { DomainError } from '../common/domain-error'; +import { tourRevisionConflict } from './tour-revision'; +import type { TourChange } from '@trek/shared'; + +import { createHash } from 'node:crypto'; + +/** What a proposal is: these changes, to this tour of this trip, at this revision. */ +export interface ProposalBasis { + tripId: number; + placeId: number; + /** Null only for a tour read before revisions were counted, which no save can name. */ + revision: number | null; + changes: readonly TourChange[]; +} + +/** + * MCP-T03: the id of a prepared proposal, the same for the same tour, + * revision and changes and different for any other. A preparation answers + * it, and a save has to name it, so what is saved is the very proposal the + * traveller was shown and nothing an assistant changed on the way. It binds a + * save to its review, it is no secret: the revision guards against a tour + * that moved on. + */ +export function proposalIdOf({ tripId, placeId, revision, changes }: ProposalBasis): string { + return createHash('sha256') + .update(JSON.stringify([tripId, placeId, revision, changes])) + .digest('hex') + .slice(0, 32); +} + +/** + * The conflict of a save whose proposal was prepared on a revision the tour + * has moved past: nothing is saved, and the assistant is told to read the + * tour again and prepare the change anew rather than retry the old one. + */ +export function staleProposal(prepared: number, current: number | null): DomainError { + return tourRevisionConflict( + current, + `The tour was changed after this proposal was prepared (revision ${prepared} then, ${current ?? 'unknown'} now). Nothing was saved: read it again with get_tour_plan and prepare the change anew.`, + ); +} diff --git a/server/src/nest/tours/tour-change-save.service.ts b/server/src/nest/tours/tour-change-save.service.ts new file mode 100644 index 000000000..3d81b4b3f --- /dev/null +++ b/server/src/nest/tours/tour-change-save.service.ts @@ -0,0 +1,171 @@ +import { TourChangeSteps } from '../../db/entities/TourChangeSteps.entity'; +import { TOUR_CHANGE_STEP_BEFORE, TOUR_CHANGE_STEP_CHANGES } from '../../db/json-columns'; +import type { TourChangeStepsRepository } from '../../db/repositories/TourChangeSteps.repository'; +import { encodeJson } from '../../utils/json-column'; +import { DomainError } from '../common/domain-error'; +import { UnitOfWork } from '../database/unit-of-work'; +import type { FootWay } from './city-walk-briefing.helpers'; +import { CityWalkFootWayService } from './city-walk-foot-way.service'; +import type { DraftPoint } from './tour-change-apply'; +import { asWaypoint } from './tour-change-effects'; +import { TourChangePreviewService } from './tour-change-preview.service'; +import { proposalIdOf } from './tour-change-proposal'; +import { ToursService, type TourCreateCommand } from './tours.service'; +import { InjectRepository } from '@mikro-orm/nestjs'; +import { Injectable } from '@nestjs/common'; +import { + plannableTourTypeSchema, + walkStationsOf, + type TourChangeSaveRequest, + type TourDetailResponse, + type TourListItem, + type TourWaypoint, +} from '@trek/shared'; + +/** The refusal of a save naming a proposal other than the one these changes were prepared as. */ +export const TOUR_CHANGE_NOT_PREPARED = 'tour_change_not_prepared'; + +/** What a save writes: the tour as read, the points after the changes and the way on foot through them. */ +interface ChangedTour { + tour: TourListItem; + points: readonly DraftPoint[]; + way: FootWay; + preparedOn: number; +} + +/** The step a save recorded: its id and the revision it raised the tour to. */ +interface SavedStep { + id: number; + revision: number; +} + +/** + * The write of a confirmed change: the points after it, the way on foot the + * router planned through them, and the tour's own name, type and options as + * they are, so a change of stations changes nothing else. Without a way the + * walk keeps its stations only, as a created one does (CW-11): no line, no + * distance and no walking time until the planner plans the way. + */ +function writeOf({ tour, points, way, preparedOn }: ChangedTour): TourCreateCommand { + const routed = way.kind === 'routed' ? way : null; + return { + name: tour.name, + // Only a type that takes changes gets here, and each of them is one the planner draws. + tour_type: plannableTourTypeSchema.parse(tour.tour_type), + route_geometry: routed?.line ?? null, + duration_seconds: routed?.seconds ?? null, + waypoints: points.map(asWaypoint), + max_hiking_difficulty: tour.max_hiking_difficulty, + hiking_preferences: tour.hiking_preferences, + bike_profile: tour.bike_profile, + boat_type: tour.boat_type, + water_context: tour.water_context, + paddle_speed_kmh: tour.paddle_speed_kmh, + portage_speed_kmh: tour.portage_speed_kmh, + paddle_lake_style: tour.paddle_lake_style, + expected_revision: preparedOn, + }; +} + +/** The saved tour as the answer gives it, read back through the read the app uses. */ +function savedAnswer(detail: TourDetailResponse, step: SavedStep, { way, preparedOn }: ChangedTour) { + return { + saved: true as const, + step: { id: step.id, base_revision: preparedOn, revision: step.revision }, + tour: { + place_id: detail.tour.place_id, + name: detail.tour.name, + tour_type: detail.tour.tour_type, + revision: step.revision, + distance_km: detail.tour.distance, + walking_minutes: detail.tour.duration, + route_pending: detail.tour.route_pending === true, + }, + foot_way: + way.kind === 'routed' + ? { planned: true as const } + : { planned: false as const, why: way.why, position: way.position }, + stations: walkStationsOf(detail.waypoints).map((station, index) => ({ + position: index + 1, + key: station.key ?? null, + name: station.name ?? null, + })), + }; +} + +/** + * MCP-T03: a prepared change the traveller confirmed, saved whole or not at + * all through the write the planner's save uses. The save names the revision + * the proposal was prepared on (P2C-03, the one revision a tour has) and the + * proposal's id, so only the very proposal shown is saved, and never over a + * tour that has moved on. The changes are laid again on the tour as it is, by + * the same rules: first to plan the way on foot, then once more inside the + * write, so a station visited or a place gone in between refuses the save + * instead of being written over, and a failure leaves the tour as it was. + * + * The write records the step: the proposal, both revisions, the changes and + * the tour as it was before them (its line, its points, its measures), so it + * can be followed and taken back (MCP-T04). Nothing of the conversation is + * kept. A new city walk has no revision to name: it is created through + * create_city_walk (CW-11), never saved as a change. + */ +@Injectable() +export class TourChangeSaveService { + constructor( + private readonly uow: UnitOfWork, + private readonly tours: ToursService, + private readonly preview: TourChangePreviewService, + private readonly footWays: CityWalkFootWayService, + @InjectRepository(TourChangeSteps) private readonly stepsRepo: TourChangeStepsRepository, + ) {} + + async save(tripId: number, placeId: number, userId: number, request: TourChangeSaveRequest) { + const { changes, expected_revision: preparedOn, proposal_id: proposalId } = request; + if (proposalId !== proposalIdOf({ tripId, placeId, revision: preparedOn, changes })) { + throw new DomainError( + 422, + 'These changes are not the proposal that was prepared. Nothing was saved: save the changes and the revision exactly as prepare_tour_change answered them, or prepare the change again.', + { code: TOUR_CHANGE_NOT_PREPARED }, + ); + } + const laid = await this.preview.lay(tripId, placeId, changes, preparedOn); + const way = await this.footWays.plan(userId, tripId, laid.outcome.points); + const changed: ChangedTour = { tour: laid.tour, points: laid.outcome.points, way, preparedOn }; + const write = await this.tours.routeWrite(writeOf(changed)); + const saved = await this.uow.transactional(async () => { + // Laid once more on the tour as it is now: a tour moved on, a station done or a place gone refuses here. + const now = await this.preview.lay(tripId, placeId, changes, preparedOn); + const before = await this.beforeState(tripId, placeId, now); + const { revision, visitsDropped } = await this.tours.replaceRoute(tripId, placeId, write); + const id = await this.stepsRepo.record({ + place_id: placeId, + proposal_id: proposalId, + base_revision: preparedOn, + revision, + changes: encodeJson(TOUR_CHANGE_STEP_CHANGES, changes), + before_state: before, + saved_by: userId, + }); + return { step: { id, revision }, visitsDropped }; + }); + const detail = await this.tours.announceRouteUpdate(String(tripId), placeId, saved.visitsDropped); + return savedAnswer(detail, saved.step, changed); + } + + /** The tour as it is before the change, as the step keeps it: its line, its points and its measures. */ + private async beforeState( + tripId: number, + placeId: number, + { tour, waypoints }: { tour: TourListItem; waypoints: TourWaypoint[] }, + ): Promise { + const line = await this.tours.savedLine(tripId, placeId); + return encodeJson(TOUR_CHANGE_STEP_BEFORE, { + route_geometry: line.length > 0 ? line : null, + waypoints, + distance: tour.distance, + duration: tour.duration, + elevation_gain: tour.elevation_gain, + elevation_loss: tour.elevation_loss, + }); + } +} diff --git a/server/src/nest/tours/tour-change.mcp.ts b/server/src/nest/tours/tour-change.mcp.ts index 69142eef6..34f28b2b2 100644 --- a/server/src/nest/tours/tour-change.mcp.ts +++ b/server/src/nest/tours/tour-change.mcp.ts @@ -1,13 +1,30 @@ import { ADDON_IDS } from '../../addons'; -import { noAccess } from '../../mcp/tools/_shared'; -import { McpController, Tool, TOOL_ANNOTATIONS_READONLY, errorResult, ok, type McpContext } from '../../nest-mcp'; +import { noAccess, permissionDenied } from '../../mcp/tools/_shared'; +import { + McpController, + Tool, + TOOL_ANNOTATIONS_NON_IDEMPOTENT, + TOOL_ANNOTATIONS_READONLY, + errorResult, + ok, + type McpContext, +} from '../../nest-mcp'; import { addonGate } from '../addons/addon-gate'; import { AddonsService } from '../addons/addons.service'; import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service'; import { TripAccessService } from '../trip-membership/trip-access.service'; import { TourChangePreviewService } from './tour-change-preview.service'; +import { TourChangeSaveService } from './tour-change-save.service'; import { NotFoundException } from '@nestjs/common'; -import { idSchema, tourChangeRequestSchema, type TourChangeRequest } from '@trek/shared'; +import { + idSchema, + tourChangeRequestSchema, + tourChangeSaveRequestSchema, + type TourChangeRequest, + type TourChangeSaveRequest, +} from '@trek/shared'; + +const save = tourChangeSaveRequestSchema.shape; /** * MCP-T02: a change to a saved tour prepared for the traveller to review, @@ -18,21 +35,26 @@ import { idSchema, tourChangeRequestSchema, type TourChangeRequest } from '@trek * assistant whether this traveller may save changes to the trip's places at * all, so a proposal is never offered as something they could apply when * they cannot. + * + * MCP-T03: the proposal the traveller confirmed is saved with 'place_edit' + * on the trip, as the planner's save over REST asks, whole or not at all and + * never over a tour that moved on since it was prepared. */ @McpController() export class TourChangeMcp { constructor( private readonly preview: TourChangePreviewService, + private readonly saver: TourChangeSaveService, private readonly access: TripAccessService, private readonly guards: McpToolGuardsService, - // Read by the addon gate of the tool here. + // Read by the addon gate of every tool here. readonly addons: AddonsService, ) {} @Tool({ name: 'prepare_tour_change', description: - "Prepare a change to a saved tour for the traveller to review, without saving anything: no tour, station, visit or day plan changes, and turning it down keeps the plan as it is. Read the tour with get_tour_plan first. A city walk (tour_type walk) takes changes so far; every other type, and a walk that is a recorded track without stations of its own, is refused with the reason. changes apply in order, each naming saved stations by their key: add (a new station after after_key; a spontaneous stop is an add after the station the traveller is at), remove, replace (a new station in the place of key) and move (key to after after_key). Stops put after the same station keep the order given. A station protected, or visited or skipped on any day of the walk, is never removed, replaced or moved, nothing is put before the last station already done, and a leg drawn by hand stays as drawn (the closing leg of a loop included); such a change is refused, and so is the whole proposal, with every reason in the error. The answer: prepared_only (always true), base (the tour and the revision it was prepared on: a save has to name it, and a tour changed since is refused rather than overwritten, so prepare again), stations_after (each with its key, null for a new one, change kept, added, replaced or moved, by_change, was_position and what it replaces), removed, effects (walking: the whole way on foot before and after planned by TREK's router and the change in metres and minutes, the distance and walking time stored with the walk apart; stays: known minutes and unknown stays before and after; legs: each leg after, new ones marked), data_gaps (what could not be worked out and why: no way from the router, stays not known, no arrival times, shaping points dropped because the leg they steered changed), warnings (a must station removed or replaced), for_later (each new station not yet a place of the trip as create_place takes it, to keep it as a place instead; a station with linked_place_id already is one and is left out) and waypoints_after. Show every change and its effects to the traveller and save nothing without their yes. can_save says whether they may change the trip's places.", + "Prepare a change to a saved tour for the traveller to review, without saving anything: no tour, station, visit or day plan changes, and turning it down keeps the plan as it is. Read the tour with get_tour_plan first. A city walk (tour_type walk) takes changes so far; every other type, and a walk that is a recorded track without stations of its own, is refused with the reason. changes apply in order, each naming saved stations by their key: add (a new station after after_key; a spontaneous stop is an add after the station the traveller is at), remove, replace (a new station in the place of key) and move (key to after after_key). Stops put after the same station keep the order given. A station protected, or visited or skipped on any day of the walk, is never removed, replaced or moved, nothing is put before the last station already done, and a leg drawn by hand stays as drawn (the closing leg of a loop included); such a change is refused, and so is the whole proposal, with every reason in the error. The answer: prepared_only (always true), base (the tour and the revision it was prepared on: a save has to name it, and a tour changed since is refused rather than overwritten, so prepare again), stations_after (each with its key, null for a new one, change kept, added, replaced or moved, by_change, was_position and what it replaces), removed, effects (walking: the whole way on foot before and after planned by TREK's router and the change in metres and minutes, the distance and walking time stored with the walk apart; stays: known minutes and unknown stays before and after; legs: each leg after, new ones marked), data_gaps (what could not be worked out and why: no way from the router, stays not known, no arrival times, shaping points dropped because the leg they steered changed), warnings (a must station removed or replaced), for_later (each new station not yet a place of the trip as create_place takes it, to keep it as a place instead; a station with linked_place_id already is one and is left out) and waypoints_after. proposal_id names this very proposal for save_tour_change. Show every change and its effects to the traveller and save nothing without their yes. can_save says whether they may change the trip's places.", inputSchema: { tripId: idSchema, placeId: idSchema.describe('The place id of the tour, as list_tours gives it'), @@ -56,4 +78,34 @@ export class TourChangeMcp { throw err; } } + + @Tool({ + name: 'save_tour_change', + description: + "Save a change to a saved tour that prepare_tour_change prepared and the traveller confirmed, whole or not at all. Call it only after the traveller said yes to the proposal exactly as it was shown to them, with confirmed true, the same changes, expected_revision set to base.revision and the proposal_id of that proposal; anything else is refused and nothing is saved. Before anything is written the trip permission and the tour are checked again: a tour changed since the proposal was prepared is refused as a conflict (prepare the change again from get_tour_plan, never retry the old one), and a change the rules no longer allow (a station visited or skipped meanwhile, a place of the trip gone) is refused with every reason. The save writes the stations, their keys and the way on foot as the planner's save does, planned by the same router as the proposal: where the router gives no way the walk keeps its stations only (route_pending true, distance and walking time null) until the planner plans it. Stations kept keep their keys and their visits. The answer: saved, step (its id, base_revision and the revision the save raised the tour to; the step keeps the changes and the tour as it was before, for taking it back later, and nothing of the conversation), the tour as now saved (revision, distance_km, walking_minutes, route_pending), foot_way (planned, or why not) and the stations in order with their keys. Read the result with get_tour_plan. A new walk has no revision yet: create it with create_city_walk instead. Needs place editing permission on the trip.", + inputSchema: { + tripId: idSchema, + placeId: idSchema.describe('The place id of the tour, as list_tours gives it'), + changes: save.changes, + expected_revision: save.expected_revision, + proposal_id: save.proposal_id, + confirmed: save.confirmed, + }, + annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT, + when: addonGate(ADDON_IDS.TOURS), + access: { group: 'trips', mode: 'write' }, + }) + async saveTourChange( + { tripId, placeId, ...request }: { tripId: number; placeId: number } & TourChangeSaveRequest, + ctx: McpContext, + ) { + if (!(await this.access.findAccessible(tripId, ctx.userId))) return noAccess(); + if (!(await this.guards.hasTripPermission('place_edit', tripId, ctx.userId))) return permissionDenied(); + try { + return ok(await this.saver.save(tripId, placeId, ctx.userId, request)); + } catch (err: unknown) { + if (err instanceof NotFoundException) return errorResult('Tour not found.'); + throw err; + } + } } diff --git a/server/src/nest/tours/tour-revision.ts b/server/src/nest/tours/tour-revision.ts index d947a65dd..6b48464cc 100644 --- a/server/src/nest/tours/tour-revision.ts +++ b/server/src/nest/tours/tour-revision.ts @@ -22,7 +22,19 @@ export async function advanceTourRevision( if (next !== null) return next; const current = await toursRepo.revisionOf(placeId); if (current === undefined) throw new NotFoundException('Tour not found'); - throw new DomainError(409, 'This tour was changed in the meantime', { + throw tourRevisionConflict(current); +} + +/** + * The 409 of a write based on a revision the tour has moved past, naming the + * revision it is at now. A caller with its own words for it (a confirmed + * change saved over MCP, MCP-T03) passes them; the code and details stay. + */ +export function tourRevisionConflict( + current: number | null, + message = 'This tour was changed in the meantime', +): DomainError { + return new DomainError(409, message, { code: TOUR_REVISION_CONFLICT, details: { code: TOUR_REVISION_CONFLICT, revision: current }, }); diff --git a/server/src/nest/tours/tours.module.ts b/server/src/nest/tours/tours.module.ts index 951f0d431..c4a8d6f23 100644 --- a/server/src/nest/tours/tours.module.ts +++ b/server/src/nest/tours/tours.module.ts @@ -1,4 +1,5 @@ import { Places } from '../../db/entities/Places.entity'; +import { TourChangeSteps } from '../../db/entities/TourChangeSteps.entity'; import { TourObstacleReviews } from '../../db/entities/TourObstacleReviews.entity'; import { TourSources } from '../../db/entities/TourSources.entity'; import { TourStationVisits } from '../../db/entities/TourStationVisits.entity'; @@ -20,6 +21,7 @@ import { CityWalkBriefingService } from './city-walk-briefing.service'; import { CityWalkFootWayService } from './city-walk-foot-way.service'; import { CityWalkMcp } from './city-walk.mcp'; import { TourChangePreviewService } from './tour-change-preview.service'; +import { TourChangeSaveService } from './tour-change-save.service'; import { TourChangeMcp } from './tour-change.mcp'; import { TourGpxExportService } from './tour-gpx-export.service'; import { TourHorseRoutesController } from './tour-horse-routes.controller'; @@ -63,7 +65,9 @@ import { Module } from '@nestjs/common'; * tours' own repositories, is planned on foot by RoadtripModule's router and * is created through ToursService (CW-11). A change to a saved city walk an * assistant prepares for review is laid onto its saved points and planned on - * foot by the same router, and saves nothing (MCP-T02). + * foot by the same router, and saves nothing (MCP-T02); once the traveller + * confirmed it, it is saved through the tour's own write, which records the + * step for a later undo (MCP-T03). */ @Module({ imports: [ @@ -92,6 +96,7 @@ import { Module } from '@nestjs/common'; TourSources, TourObstacleReviews, TourStationVisits, + TourChangeSteps, Places, ]), ], @@ -126,6 +131,7 @@ import { Module } from '@nestjs/common'; CityWalkFootWayService, CityWalkBriefingService, TourChangePreviewService, + TourChangeSaveService, ToursMcp, TourLibraryMcp, TourWalkImportMcp, diff --git a/server/src/nest/tours/tours.service.ts b/server/src/nest/tours/tours.service.ts index 837540897..620030713 100644 --- a/server/src/nest/tours/tours.service.ts +++ b/server/src/nest/tours/tours.service.ts @@ -95,6 +95,13 @@ export type TourCreateCommand = Omit & { route_geometry: GeometryPoint[] | null; }; +/** A route write checked and ready for its transaction: the write, where the tour starts and its fields. */ +export interface TourRouteWrite { + input: TourCreateCommand; + start: GeometryPoint; + fields: TourUpdate; +} + /** The way on foot the server's router planned for a city walk (CW-11): its line, and how long it walks in seconds. */ interface CityWalkFootRoute { line: Array<[number, number]>; @@ -705,44 +712,75 @@ export class ToursService { const tid = toRowId(tripId); const pid = toRowId(placeId); if (tid === null || pid === null) throw new NotFoundException('Tour not found'); + const write = await this.routeWrite(input); + const { visitsDropped } = await this.uow.transactional(() => this.replaceRoute(tid, pid, write)); + return await this.announceRouteUpdate(tripId, pid, visitsDropped, socketId); + } + + /** + * A route write checked before its transaction opens: its type switched on, + * its texts and times valid, where it starts and the tour fields it writes. + * A city walk saved from a confirmed change (MCP-T03) may come without a + * line, as a created one may (CW-11): it is then at its first point, and the + * planner plans the way when it is opened. + */ + async routeWrite(input: TourCreateCommand): Promise { await this.assertTourTypeEnabled(input.tour_type); this.validateInformationalMetadata(input); this.validatePlannedTimes(input); - const start = input.route_geometry[0]; - const fields = this.routeFields(input); + const start = input.route_geometry ? input.route_geometry[0] : firstPointOf(input.waypoints); + if (!start) throw badRequest('A tour needs at least two points'); + return { input, start, fields: this.routeFields(input) }; + } - const visitsDropped = await this.uow.transactional(async () => { - // The trip predicate is the cross-trip boundary. It runs inside the write - // so an id from another accessible trip cannot be moved into the current - // one, and a place deleted meanwhile answers 404 instead of failing a - // waypoint insert on its foreign key. - if (!(await this.toursRepo.findInTrip(tid, pid))) throw new NotFoundException('Tour not found'); - // P2C-03: refused here, before anything is written, while the draft's tour has moved on. - await advanceTourRevision(this.toursRepo, pid, input.expected_revision); - // An import planned over for the first time keeps the track it was recorded with. - await this.toursRepo.keepRecordedTrack(pid); - const placeUpdated = await this.placesRepo.updateTourRoute(pid, tid, { - name: input.name, - lat: start[0], - lng: start[1], - route_geometry: JSON.stringify(input.route_geometry), - ...(input.description !== undefined ? { description: input.description } : {}), - ...(input.website !== undefined ? { website: input.website } : {}), - }); - if (!placeUpdated) throw new NotFoundException('Tour not found'); - if (!(await this.toursRepo.updateInTrip(tid, pid, fields))) throw new NotFoundException('Tour not found'); - await this.waypointsRepo.deleteForPlace(pid); - const waypoints = keyedWaypoints(await this.linkedWaypoints(tid, input.waypoints)); - await this.waypointsRepo.insertForPlace(pid, waypoints); - // CW-03: a station that is gone takes its visits with it; a tour that is no city walk keeps none. - const stations = input.tour_type === 'walk' ? walkStationsOf(waypoints).map((point) => point.key) : []; - return await this.visitsRepo.dropStationsNotIn(pid, stations); + /** + * A saved tour's route, points and visits written inside the caller's transaction, so what else a + * caller records of the change (MCP-T03) commits or rolls back with it. Answers the revision it + * raised the tour to and the day assignments whose visits went with a station that is gone. + */ + async replaceRoute( + tripId: number, + placeId: number, + { input, start, fields }: TourRouteWrite, + ): Promise<{ revision: number; visitsDropped: number[] }> { + // The trip predicate is the cross-trip boundary. It runs inside the write + // so an id from another accessible trip cannot be moved into the current + // one, and a place deleted meanwhile answers 404 instead of failing a + // waypoint insert on its foreign key. + if (!(await this.toursRepo.findInTrip(tripId, placeId))) throw new NotFoundException('Tour not found'); + // P2C-03: refused here, before anything is written, while the draft's tour has moved on. + const revision = await advanceTourRevision(this.toursRepo, placeId, input.expected_revision); + // An import planned over for the first time keeps the track it was recorded with. + await this.toursRepo.keepRecordedTrack(placeId); + const placeUpdated = await this.placesRepo.updateTourRoute(placeId, tripId, { + name: input.name, + lat: start[0], + lng: start[1], + route_geometry: input.route_geometry ? JSON.stringify(input.route_geometry) : null, + ...(input.description !== undefined ? { description: input.description } : {}), + ...(input.website !== undefined ? { website: input.website } : {}), }); + if (!placeUpdated) throw new NotFoundException('Tour not found'); + if (!(await this.toursRepo.updateInTrip(tripId, placeId, fields))) throw new NotFoundException('Tour not found'); + await this.waypointsRepo.deleteForPlace(placeId); + const waypoints = keyedWaypoints(await this.linkedWaypoints(tripId, input.waypoints)); + await this.waypointsRepo.insertForPlace(placeId, waypoints); + // CW-03: a station that is gone takes its visits with it; a tour that is no city walk keeps none. + const stations = input.tour_type === 'walk' ? walkStationsOf(waypoints).map((point) => point.key) : []; + return { revision, visitsDropped: await this.visitsRepo.dropStationsNotIn(placeId, stations) }; + } - const result = await this.getTour(tripId, placeId); - const place = await this.placesRepo.findWithTagsAndRatings(pid); + /** After a route write committed: the tour read back, and the trip told of it and of the visits it took along. */ + async announceRouteUpdate( + tripId: string, + placeId: number, + visitsDropped: readonly number[], + socketId?: string, + ): Promise { + const result = await this.getTour(tripId, String(placeId)); + const place = await this.placesRepo.findWithTagsAndRatings(placeId); if (!place) throw new Error('Updated tour could not be loaded'); - this.broadcastToursChanged(tripId, [pid], socketId); + this.broadcastToursChanged(tripId, [placeId], socketId); try { this.places.broadcast(tripId, 'place:updated', { place }, socketId); } catch { diff --git a/server/tests/unit/db/entities-index.test.ts b/server/tests/unit/db/entities-index.test.ts index 5c152e4ba..9bb6a3b52 100644 --- a/server/tests/unit/db/entities-index.test.ts +++ b/server/tests/unit/db/entities-index.test.ts @@ -40,14 +40,15 @@ describe('ALL_ENTITIES', () => { * + paddle_regions, paddle_features and tour_obstacle_reviews, the paddle obstacle data and decisions * + trip_journal_links and trip_journal_link_members, the journal a trip shows and who it is open to * + paddle_water_graphs and paddle_region_requests, a region's water network and the regions that wait for an admin - * + tour_station_visits, how far a city walk got on one of its days) + * + tour_station_visits, how far a city walk got on one of its days + * + tour_change_steps, the changes to a tour saved from a confirmed proposal) * is registered, and no two share a class name (a silent duplicate would * shadow one of them in `orm.getMetadata()`, exactly the kind of drift * `entity-schema-parity.test.ts` can't see — it iterates the metadata map, * which de-duplicates by class name). */ - it('ENT-003: exactly 143 entities, every class name unique', () => { - expect(ALL_ENTITIES.length).toBe(143); + it('ENT-003: exactly 144 entities, every class name unique', () => { + expect(ALL_ENTITIES.length).toBe(144); const names = ALL_ENTITIES.map((s) => s.meta.className); expect(new Set(names).size).toBe(names.length); }); diff --git a/server/tests/unit/db/entity-class-fields.test.ts b/server/tests/unit/db/entity-class-fields.test.ts index ecec0a7c7..09dab2082 100644 --- a/server/tests/unit/db/entity-class-fields.test.ts +++ b/server/tests/unit/db/entity-class-fields.test.ts @@ -36,7 +36,9 @@ import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; // + paddle_region_requests: region_id, name, reason (a request is only ever filed whole). // + tour_station_visits: assignment_id, station_key, status (a visit is only ever written whole, by the // day plan that checks a city walk's station off). -const KNOWN_UNINITIALISED_NOT_NULL_SCALARS = 497; +// + tour_change_steps: id, place_id, proposal_id, base_revision, revision, changes, before_state (a step is +// only ever written whole, by the save of a confirmed change). +const KNOWN_UNINITIALISED_NOT_NULL_SCALARS = 504; describe('entity class fields never carry a NaN initialiser (task-4-review-shape.md, Important 1)', () => { it('CLASSFIELD-001: new X() has no own property whose value is NaN, across all 133 entities', () => { diff --git a/server/tests/unit/db/legacy-baseline.test.ts b/server/tests/unit/db/legacy-baseline.test.ts index a3e26bb7c..ddf3bea1a 100644 --- a/server/tests/unit/db/legacy-baseline.test.ts +++ b/server/tests/unit/db/legacy-baseline.test.ts @@ -70,6 +70,7 @@ const UNNUMBERED = [ 'Migration20200101045510_a_tour_can_be_a_run', 'Migration20200101045520_a_tour_can_be_a_ski_track', 'Migration20200101045600_a_tour_can_be_ridden_on_horseback', + 'Migration20200101045700_a_tour_records_the_changes_saved_from_a_proposal', ]; // The last step the positional runner ever had; planLegacyBaseline refuses a // schema_version past it. diff --git a/server/tests/unit/db/repositories/TourChangeSteps.repository.test.ts b/server/tests/unit/db/repositories/TourChangeSteps.repository.test.ts new file mode 100644 index 000000000..eff7bedea --- /dev/null +++ b/server/tests/unit/db/repositories/TourChangeSteps.repository.test.ts @@ -0,0 +1,96 @@ +/** + * `TourChangeStepsRepository` (MCP-T03): a step saved from a confirmed + * proposal recorded whole with its id answered, one step per revision of a + * tour, its revision the next after the one it was prepared on, and the steps + * going with their tour. + */ +import { TourChangeSteps } from '../../../../src/db/entities/TourChangeSteps.entity'; +import { Tours } from '../../../../src/db/entities/Tours.entity'; +import type { + TourChangeStepsRepository, + TourChangeStepWrite, +} from '../../../../src/db/repositories/TourChangeSteps.repository'; +import { createSnapshotTestDb } from '../../../helpers/db-mock'; +import { makePlace } from '../../../helpers/factories/places'; +import { countRows, deleteRows, findRow } from '../../../helpers/factories/rows'; +import { makeTour } from '../../../helpers/factories/tours'; +import { makeTrip } from '../../../helpers/factories/trips'; +import { makeUser } from '../../../helpers/factories/users'; +import { resetTestDb } from '../../../helpers/test-db'; +import { createTestOrm, type TestOrm } from '../../../helpers/test-orm'; + +import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; + +const testDb = createSnapshotTestDb(); +let t: TestOrm; +let steps: TourChangeStepsRepository; + +beforeAll(async () => { + t = await createTestOrm(testDb); + steps = t.repo(TourChangeSteps); +}); +beforeEach(() => { + resetTestDb(testDb); + t.clear(); +}); +afterAll(async () => { + await t.close(); + testDb.close(); +}); + +async function walk() { + const { user } = await makeUser(t); + const trip = await makeTrip(t, user.id); + const place = await makePlace(t, trip.id, { name: 'Old town' }); + await makeTour(t, place.id, { tourTypeRef: 'walk', revision: 4 }); + return { user, place }; +} + +function step(placeId: number, overrides: Partial = {}): TourChangeStepWrite { + return { + place_id: placeId, + proposal_id: '0123456789abcdef0123456789abcdef', + base_revision: 3, + revision: 4, + changes: '[{"op":"remove","key":"stop-park"}]', + before_state: '{"route_geometry":null,"waypoints":[]}', + saved_by: null, + ...overrides, + }; +} + +describe('TourChangeStepsRepository', () => { + it('TCSTEP-001: records a step whole and answers its id, the time it was saved filled in', async () => { + const { user, place } = await walk(); + const id = await steps.record(step(place.id, { saved_by: user.id })); + + const row = await findRow(t, TourChangeSteps, { id }); + expect(row).toMatchObject({ + proposal_id: '0123456789abcdef0123456789abcdef', + base_revision: 3, + revision: 4, + changes: '[{"op":"remove","key":"stop-park"}]', + before_state: '{"route_geometry":null,"waypoints":[]}', + saved_by: user.id, + }); + expect(row?.saved_at).toEqual(expect.any(String)); + expect(await steps.record(step(place.id, { base_revision: 4, revision: 5 }))).toBeGreaterThan(id); + }); + + it('TCSTEP-002: one step per revision of a tour, always the revision after the one it was prepared on', async () => { + const { place } = await walk(); + await steps.record(step(place.id)); + + await expect(steps.record(step(place.id, { proposal_id: 'fedcba9876543210fedcba9876543210' }))).rejects.toThrow(); + await expect(steps.record(step(place.id, { base_revision: 4, revision: 6 }))).rejects.toThrow(); + await expect(steps.record(step(place.id, { base_revision: 0, revision: 1 }))).rejects.toThrow(); + expect(await countRows(t, TourChangeSteps, {})).toBe(1); + }); + + it('TCSTEP-003: the steps go with their tour', async () => { + const { place } = await walk(); + await steps.record(step(place.id)); + await deleteRows(t, Tours, { place: place.id }); + expect(await countRows(t, TourChangeSteps, {})).toBe(0); + }); +}); diff --git a/server/tests/unit/nest/tour-change-save.mcp.test.ts b/server/tests/unit/nest/tour-change-save.mcp.test.ts new file mode 100644 index 000000000..d17ef83d1 --- /dev/null +++ b/server/tests/unit/nest/tour-change-save.mcp.test.ts @@ -0,0 +1,409 @@ +/** + * TourChangeMcp.saveTourChange (MCP-T03): a prepared change to a saved city + * walk the traveller confirmed, saved whole or not at all through the tour's + * own write. The save names the proposal and the revision it was prepared on; + * a tour that moved on is a conflict, a change the rules no longer allow is + * refused inside the write, and a failure leaves the tour as it was. Every + * save records its step with the tour as it was before. The suite takes its + * subject from the container over a snapshot test db, plans the way on foot + * with a stub of the road trip's router and seeds through the ORM factories. + */ +import { DayAssignments } from '../../../src/db/entities/DayAssignments.entity'; +import { Places } from '../../../src/db/entities/Places.entity'; +import { TourChangeSteps } from '../../../src/db/entities/TourChangeSteps.entity'; +import { TourStationVisits } from '../../../src/db/entities/TourStationVisits.entity'; +import { TourWaypoints } from '../../../src/db/entities/TourWaypoints.entity'; +import { Tours } from '../../../src/db/entities/Tours.entity'; +import { TourChangeStepsRepository } from '../../../src/db/repositories/TourChangeSteps.repository'; +import type { McpContext } from '../../../src/nest-mcp'; +import { getEntry, type ClassRef } from '../../../src/nest-mcp/metadata'; +import { catchDomainError, DomainError } from '../../../src/nest/common/domain-error'; +import { McpToolGuardsService } from '../../../src/nest/mcp-shared/mcp-tool-guards.service'; +import { RoadtripRouterService } from '../../../src/nest/roadtrip/roadtrip-router.service'; +import { TOUR_CHANGE_REFUSED } from '../../../src/nest/tours/tour-change-preview.service'; +import { proposalIdOf } from '../../../src/nest/tours/tour-change-proposal'; +import { TOUR_CHANGE_NOT_PREPARED } from '../../../src/nest/tours/tour-change-save.service'; +import { TourChangeMcp } from '../../../src/nest/tours/tour-change.mcp'; +import { ToursMcp } from '../../../src/nest/tours/tours.mcp'; +import { ToursModule } from '../../../src/nest/tours/tours.module'; +import { ToursService } from '../../../src/nest/tours/tours.service'; +import { createSnapshotTestDb } from '../../helpers/db-mock'; +import { makeDayAssignment } from '../../helpers/factories/itinerary'; +import { makePlace } from '../../helpers/factories/places'; +import { countRows, findRow, findRows, updateRows } from '../../helpers/factories/rows'; +import { addTourWaypoints, makeTour, markStationVisit } from '../../helpers/factories/tours'; +import { makeDay, makeTrip } from '../../helpers/factories/trips'; +import { makeUser } from '../../helpers/factories/users'; +import { resetTestDb } from '../../helpers/test-db'; +import { createTestModule, type TestModule } from '../../helpers/test-module'; +import { MikroORM } from '@mikro-orm/core'; +import { stationKeySchema, TOUR_REVISION_CONFLICT, type TourChange, type TourChangeSaveRequest } from '@trek/shared'; + +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; + +const db = createSnapshotTestDb(); +const route = vi.fn(); +let t: TestModule; +let mcp: TourChangeMcp; +const orm = () => t.get(MikroORM); + +/** Every leg the router plans is 500 m and 400 s on foot, along the points asked. */ +async function routedOnFoot( + _userId: number, + _tripId: number, + _dayId: number, + points: Array<{ lat: number; lng: number }>, +) { + return { + leg: { line: points.map((point): [number, number] => [point.lat, point.lng]) }, + parts: points.slice(1).map(() => ({ distance: 500, duration: 400 })), + snapped: points.map(() => ({ meters: 4 })), + avoidMissed: [], + }; +} + +beforeAll(async () => { + t = await createTestModule({ + db, + imports: [ToursModule], + overrides: [{ provide: RoadtripRouterService, useValue: { route } }], + }); + mcp = t.get(TourChangeMcp); +}); +beforeEach(() => { + resetTestDb(db); + route.mockReset().mockImplementation(routedOnFoot); + t.realtime.broadcastMock.mockReset(); +}); +afterAll(async () => { + await t.close(); + db.close(); +}); + +const ctx = (userId: number) => ({ userId }) as McpContext; + +/** The JSON a tool answered, or the error text it refused with. */ +function read(result: { content: Array<{ type: string; text: string }>; isError?: boolean }) { + const text = result.content[0]?.text ?? ''; + return result.isError ? { error: text } : JSON.parse(text); +} + +/** The refusal a save raised, which the registry answers with its text. */ +async function refusal(promise: Promise): Promise { + const err = await catchDomainError(() => promise); + if (!(err instanceof DomainError)) throw new Error('no refusal'); + return err; +} + +/** + * A city walk at revision 3 from the gate past the protected cathedral and + * the market to the park (a must), on day 1 where the gate was visited and + * the market skipped; a bakery of the trip. + */ +async function fixture() { + const { user } = await makeUser(orm()); + const { user: outsider } = await makeUser(orm()); + const trip = await makeTrip(orm(), user.id); + const day1 = await makeDay(orm(), trip.id, { day_number: 1 }); + const walk = await makePlace(orm(), trip.id, { name: 'Old town', route_geometry: '[[48.1,11.5],[48.112,11.5]]' }); + await makeTour(orm(), walk.id, { tourTypeRef: 'walk', revision: 3, distance: 1.6, duration: 22 }); + await addTourWaypoints(orm(), walk.id, [ + { lat: 48.1, lng: 11.5, station_key: 'stop-gate', name: 'Gate', stay_minutes: 0 }, + { lat: 48.104, lng: 11.5, station_key: 'stop-cathedral', name: 'Cathedral', stay_minutes: 30, protected: 1 }, + { lat: 48.108, lng: 11.5, station_key: 'stop-market', name: 'Market', stay_minutes: 20 }, + { lat: 48.112, lng: 11.5, station_key: 'stop-park', name: 'Park', importance: 'must' }, + ]); + const onDay1 = await makeDayAssignment(orm(), day1.id, walk.id); + await markStationVisit(orm(), onDay1.id, 'stop-gate', 'visited'); + await markStationVisit(orm(), onDay1.id, 'stop-market', 'skipped'); + const bakery = await makePlace(orm(), trip.id, { name: 'Bakery', lat: 48.109, lng: 11.501 }); + return { user, outsider, trip, walk, onDay1, bakery }; +} + +/** A stop at the bakery after the market and a beer garden in the place of the park. */ +const stopAndSwap = (bakeryId: number): TourChange[] => [ + { + op: 'add', + station: { lat: 48.109, lng: 11.501, name: 'Bakery', stay_minutes: 15, linked_place_id: bakeryId }, + after_key: 'stop-market', + }, + { op: 'replace', key: 'stop-park', station: { lat: 48.113, lng: 11.502, name: 'Beer garden' } }, +]; + +/** What a refused or failed save must leave as it is. */ +const untouched = async (placeId: number) => [ + await countRows(orm(), Places, {}), + await countRows(orm(), TourWaypoints, {}), + await countRows(orm(), TourStationVisits, {}), + await countRows(orm(), DayAssignments, {}), + await countRows(orm(), TourChangeSteps, {}), + (await findRow(orm(), Tours, { place: placeId }))?.revision, + (await findRow(orm(), Places, { id: placeId }))?.route_geometry, + (await findRows(orm(), TourWaypoints, { place: placeId }, { sequence: 'asc' })).map((point) => point.station_key), +]; + +/** Prepares the changes as the traveller is shown them, and the save of exactly that proposal. */ +async function prepared(tripId: number, placeId: number, changes: TourChange[], userId: number) { + const proposal = read(await mcp.prepareTourChange({ tripId, placeId, changes }, ctx(userId))); + const request: TourChangeSaveRequest = { + changes, + expected_revision: proposal.base.revision, + proposal_id: proposal.proposal_id, + confirmed: true, + }; + return { proposal, request }; +} + +const saveAs = (tripId: number, placeId: number, request: TourChangeSaveRequest, userId: number) => + mcp.saveTourChange({ tripId, placeId, ...request }, ctx(userId)); + +describe('TourChangeMcp.saveTourChange (MCP-T03)', () => { + it('TCHG-SAVE-001: the confirmed proposal is saved whole, its step recorded with the tour as it was, and read back the same', async () => { + const f = await fixture(); + const changes = stopAndSwap(f.bakery.id); + const { proposal, request } = await prepared(f.trip.id, f.walk.id, changes, f.user.id); + expect(proposal.proposal_id).toMatch(/^[0-9a-f]{32}$/); + route.mockClear(); + + const saved = read(await saveAs(f.trip.id, f.walk.id, request, f.user.id)); + + expect(saved).toMatchObject({ + saved: true, + step: { id: expect.any(Number), base_revision: 3, revision: 4 }, + tour: { place_id: f.walk.id, name: 'Old town', tour_type: 'walk', revision: 4, walking_minutes: 27 }, + foot_way: { planned: true }, + }); + expect(saved.tour.distance_km).toBeGreaterThan(0); + expect(saved.tour.route_pending).toBe(false); + expect(saved.stations.map((s: { name: string }) => s.name)).toEqual([ + 'Gate', + 'Cathedral', + 'Market', + 'Bakery', + 'Beer garden', + ]); + expect(saved.stations.slice(0, 3).map((s: { key: string }) => s.key)).toEqual([ + 'stop-gate', + 'stop-cathedral', + 'stop-market', + ]); + expect(saved.stations.every((s: { key: string }) => stationKeySchema.safeParse(s.key).success)).toBe(true); + // The way on foot is planned once more through exactly the points saved. + const after: Array<[number, number]> = [ + [48.1, 11.5], + [48.104, 11.5], + [48.108, 11.5], + [48.109, 11.501], + [48.113, 11.502], + ]; + expect(route).toHaveBeenCalledExactlyOnceWith( + f.user.id, + f.trip.id, + 0, + after.map(([lat, lng]) => ({ lat, lng })), + 'walking', + [], + ); + + // Saved: the line, the points with the kept keys and the link, the measures, the visits as they were. + expect((await findRow(orm(), Places, { id: f.walk.id }))?.route_geometry).toBe(JSON.stringify(after)); + const points = await findRows(orm(), TourWaypoints, { place: f.walk.id }, { sequence: 'asc' }); + expect(points.map((point) => [point.name, point.role])).toEqual([ + ['Gate', 'start'], + ['Cathedral', 'via'], + ['Market', 'via'], + ['Bakery', 'via'], + ['Beer garden', 'end'], + ]); + const tour = await findRow(orm(), Tours, { place: f.walk.id }); + expect([tour?.revision, tour?.duration]).toEqual([4, 27]); + expect(await countRows(orm(), TourStationVisits, {})).toBe(2); + + // The step: the proposal, both revisions, the changes and the tour before them; nothing else. + const step = await findRow(orm(), TourChangeSteps, { id: saved.step.id }); + expect(step).toMatchObject({ + proposal_id: request.proposal_id, + base_revision: 3, + revision: 4, + saved_by: f.user.id, + }); + expect(JSON.parse(step?.changes ?? '')).toEqual(changes); + const before = JSON.parse(step?.before_state ?? ''); + expect(before).toMatchObject({ + route_geometry: [ + [48.1, 11.5], + [48.112, 11.5], + ], + distance: 1.6, + duration: 22, + elevation_gain: null, + elevation_loss: null, + }); + expect(before.waypoints.map((point: { key: string }) => point.key)).toEqual([ + 'stop-gate', + 'stop-cathedral', + 'stop-market', + 'stop-park', + ]); + + // Every client of the trip is told; no day lost a visit. + const events = t.realtime.broadcastMock.mock.calls.map(([tripId, event]) => [tripId, event]); + expect(events).toEqual([ + [String(f.trip.id), 'tours:changed'], + [String(f.trip.id), 'place:updated'], + ]); + + // The tour read again says what the save answered. + const plan = read(await t.get(ToursMcp).getTourPlan({ tripId: f.trip.id, placeId: f.walk.id }, ctx(f.user.id))); + expect(plan.tour.revision).toBe(4); + expect(plan.waypoints.map((point: { key: string }) => point.key)).toEqual( + saved.stations.map((s: { key: string }) => s.key), + ); + expect(plan.waypoints[3]).toMatchObject({ name: 'Bakery', stay_minutes: 15, linked_place_id: f.bakery.id }); + }); + + it('TCHG-SAVE-002: a tour changed since the proposal was prepared is a conflict, and nothing is written', async () => { + const f = await fixture(); + const { request } = await prepared(f.trip.id, f.walk.id, stopAndSwap(f.bakery.id), f.user.id); + await updateRows(orm(), Tours, { place: f.walk.id }, { revision: 4 }); + const before = await untouched(f.walk.id); + route.mockClear(); + + const err = await refusal(saveAs(f.trip.id, f.walk.id, request, f.user.id)); + + expect([err.getStatus(), err.code, err.details?.revision]).toEqual([409, TOUR_REVISION_CONFLICT, 4]); + expect(err.publicMessage).toContain('(revision 3 then, 4 now). Nothing was saved'); + expect(route).not.toHaveBeenCalled(); + expect(await untouched(f.walk.id)).toEqual(before); + expect(t.realtime.broadcastMock).not.toHaveBeenCalled(); + }); + + it('TCHG-SAVE-003: anything but the proposal as prepared is refused before the tour is read', async () => { + const f = await fixture(); + const { request } = await prepared(f.trip.id, f.walk.id, stopAndSwap(f.bakery.id), f.user.id); + const before = await untouched(f.walk.id); + route.mockClear(); + const altered: TourChangeSaveRequest[] = [ + { ...request, changes: [{ op: 'remove', key: 'stop-park' }] }, + { ...request, expected_revision: 2 }, + { ...request, proposal_id: 'f'.repeat(32) }, + ]; + for (const body of altered) { + const err = await refusal(saveAs(f.trip.id, f.walk.id, body, f.user.id)); + expect([err.getStatus(), err.code]).toEqual([422, TOUR_CHANGE_NOT_PREPARED]); + expect(err.publicMessage).toMatch(/^These changes are not the proposal that was prepared\. Nothing was saved/); + } + // The same proposal saved for another tour of the trip is another proposal. + const err = await refusal(saveAs(f.trip.id, f.bakery.id, request, f.user.id)); + expect(err.code).toBe(TOUR_CHANGE_NOT_PREPARED); + expect(route).not.toHaveBeenCalled(); + expect(await untouched(f.walk.id)).toEqual(before); + }); + + it('TCHG-SAVE-004: a station done meanwhile refuses the save, also when it happens while the way is planned', async () => { + const f = await fixture(); + const changes: TourChange[] = [{ op: 'remove', key: 'stop-park' }]; + const { request } = await prepared(f.trip.id, f.walk.id, changes, f.user.id); + + // Ticked off while the router plans the way: the write lays the changes again and refuses. + route.mockImplementationOnce(async (...args: Parameters) => { + await markStationVisit(orm(), f.onDay1.id, 'stop-park', 'visited'); + return await routedOnFoot(...args); + }); + const during = await refusal(saveAs(f.trip.id, f.walk.id, request, f.user.id)); + expect([during.getStatus(), during.code]).toEqual([422, TOUR_CHANGE_REFUSED]); + expect(during.publicMessage).toMatch(/^Nothing was prepared and nothing changed\.\nChange 1 \(remove stop-park\)/); + expect((await findRow(orm(), Tours, { place: f.walk.id }))?.revision).toBe(3); + expect(await countRows(orm(), TourWaypoints, { place: f.walk.id })).toBe(4); + expect(await countRows(orm(), TourChangeSteps, {})).toBe(0); + + // Once done, the save is refused before the way is planned at all. + route.mockClear(); + const after = await refusal(saveAs(f.trip.id, f.walk.id, request, f.user.id)); + expect(after.code).toBe(TOUR_CHANGE_REFUSED); + expect(route).not.toHaveBeenCalled(); + expect(t.realtime.broadcastMock).not.toHaveBeenCalled(); + }); + + it('TCHG-SAVE-005: a failure inside the write rolls the whole save back', async () => { + const f = await fixture(); + const { request } = await prepared(f.trip.id, f.walk.id, stopAndSwap(f.bakery.id), f.user.id); + const before = await untouched(f.walk.id); + const failure = new Error('disk full'); + const record = vi.spyOn(TourChangeStepsRepository.prototype, 'record').mockRejectedValueOnce(failure); + try { + await expect(saveAs(f.trip.id, f.walk.id, request, f.user.id)).rejects.toBe(failure); + } finally { + record.mockRestore(); + } + expect(await untouched(f.walk.id)).toEqual(before); + expect(t.realtime.broadcastMock).not.toHaveBeenCalled(); + }); + + it('TCHG-SAVE-006: without a way from the router the walk keeps its stations only and waits for the planner', async () => { + const f = await fixture(); + const { request } = await prepared(f.trip.id, f.walk.id, [{ op: 'remove', key: 'stop-park' }], f.user.id); + route.mockReset().mockRejectedValue(new Error('Routing provider returned 503')); + + const saved = read(await saveAs(f.trip.id, f.walk.id, request, f.user.id)); + + expect(saved.foot_way).toEqual({ planned: false, why: 'router_unavailable', position: null }); + expect(saved.tour).toMatchObject({ revision: 4, distance_km: null, walking_minutes: null, route_pending: true }); + const place = await findRow(orm(), Places, { id: f.walk.id }); + expect([place?.route_geometry, place?.lat, place?.lng]).toEqual([null, 48.1, 11.5]); + expect(saved.stations.map((s: { key: string }) => s.key)).toEqual(['stop-gate', 'stop-cathedral', 'stop-market']); + }); + + it('TCHG-SAVE-007: only a traveller of the trip with place editing saves, and only a tour', async () => { + const f = await fixture(); + const changes: TourChange[] = [{ op: 'remove', key: 'stop-park' }]; + const { request } = await prepared(f.trip.id, f.walk.id, changes, f.user.id); + const before = await untouched(f.walk.id); + + expect(read(await saveAs(f.trip.id, f.walk.id, request, f.outsider.id))).toEqual({ + error: 'Trip not found or access denied.', + }); + const guard = vi.spyOn(t.get(McpToolGuardsService), 'hasTripPermission').mockResolvedValueOnce(false); + try { + expect(read(await saveAs(f.trip.id, f.walk.id, request, f.user.id))).toEqual({ + error: 'You do not have permission to perform this action on this trip.', + }); + expect(guard).toHaveBeenCalledWith('place_edit', f.trip.id, f.user.id); + } finally { + guard.mockRestore(); + } + const proposalId = proposalIdOf({ tripId: f.trip.id, placeId: f.bakery.id, revision: 3, changes }); + const noTour = { ...request, proposal_id: proposalId }; + expect(read(await saveAs(f.trip.id, f.bakery.id, noTour, f.user.id))).toEqual({ error: 'Tour not found.' }); + expect(await untouched(f.walk.id)).toEqual(before); + }); + + it('TCHG-SAVE-008: the write of a tour needs a point to start at when it comes without a line', async () => { + const err = await refusal( + t.get(ToursService).routeWrite({ name: 'Old town', tour_type: 'walk', route_geometry: null, waypoints: [] }), + ); + expect([err.getStatus(), err.publicMessage]).toEqual([400, 'A tour needs at least two points']); + }); + + it('TCHG-SAVE-009: save_tour_change writes, on the Tours addon, and takes nothing without the yes', async () => { + const entry = getEntry(TourChangeMcp as unknown as ClassRef, 'saveTourChange'); + if (!entry || entry.kind !== 'tool') throw new Error('saveTourChange is no MCP tool'); + const { options } = entry; + expect([options.name, options.annotations?.readOnlyHint, options.access]).toEqual([ + 'save_tour_change', + false, + { group: 'trips', mode: 'write' }, + ]); + const terms = ['confirmed', 'expected_revision', 'proposal_id', 'conflict', 'get_tour_plan', 'create_city_walk']; + for (const term of terms) expect(options.description).toContain(term); + // The fields are zod schemas from @trek/shared, read here by what they answer. + const shape = options.inputSchema as Record; + const fields = ['tripId', 'placeId', 'changes', 'expected_revision', 'proposal_id', 'confirmed']; + expect(Object.keys(shape)).toEqual(fields); + const yes = [true, false, undefined].map((value) => shape.confirmed?.safeParse(value).success); + expect(yes).toEqual([true, false, false]); + const isAddonEnabled = vi.fn(async (id: string) => id === 'tours'); + expect(await options.when?.(ctx(1), { addons: { isAddonEnabled } })).toBe(true); + expect(isAddonEnabled).toHaveBeenCalledWith('tours'); + }); +}); diff --git a/server/tests/unit/utils/json-column.test.ts b/server/tests/unit/utils/json-column.test.ts index c0fa8245d..be83459a1 100644 --- a/server/tests/unit/utils/json-column.test.ts +++ b/server/tests/unit/utils/json-column.test.ts @@ -8,7 +8,10 @@ import { OAUTH_CONSENT_SCOPES, OAUTH_TOKEN_SCOPES, RESERVATION_METADATA, + TOUR_CHANGE_STEP_BEFORE, + TOUR_CHANGE_STEP_CHANGES, USER_MFA_BACKUP_CODES, + type TourChangeStepBefore, } from '../../../src/db/json-columns'; import { decodeJson, @@ -133,4 +136,24 @@ describe('the declared columns', () => { expect(decodeJson(JOURNEY_ENTRY_PROS_CONS, 'null')).toBeNull(); expect(decodeJson(JOURNEY_ENTRY_PROS_CONS, '{bad')).toBeNull(); }); + + it('JSONCOL-034: a saved tour change keeps its changes and the tour before, a broken row claims neither', () => { + const changes = [{ op: 'remove', key: 'stop-park' }]; + expect(decodeJson(TOUR_CHANGE_STEP_CHANGES, JSON.stringify(changes))).toEqual(changes); + expect(decodeJson(TOUR_CHANGE_STEP_CHANGES, '[{"op":"rename"}]')).toEqual([]); + const before: TourChangeStepBefore = { + route_geometry: [ + [48.1, 11.5, 520], + [48.2, 11.6], + ], + waypoints: [{ lat: 48.1, lng: 11.5, role: 'start', sequence: 0, key: 'stop-gate', name: 'Gate' }], + distance: 1.6, + duration: 22, + elevation_gain: null, + elevation_loss: null, + }; + expect(decodeJson(TOUR_CHANGE_STEP_BEFORE, encodeJson(TOUR_CHANGE_STEP_BEFORE, before))).toEqual(before); + expect(decodeJson(TOUR_CHANGE_STEP_BEFORE, '{"waypoints":"none"}')).toBeNull(); + expect(decodeJson(TOUR_CHANGE_STEP_BEFORE, '{bad')).toBeNull(); + }); }); diff --git a/shared/src/tours/tour-change.schema.spec.ts b/shared/src/tours/tour-change.schema.spec.ts index 15be176f2..fd86a7fda 100644 --- a/shared/src/tours/tour-change.schema.spec.ts +++ b/shared/src/tours/tour-change.schema.spec.ts @@ -1,4 +1,10 @@ -import { TOUR_CHANGE_MAX, TOUR_CHANGE_OPS, tourChangeRequestSchema, tourChangeSchema } from './tour-change.schema'; +import { + TOUR_CHANGE_MAX, + TOUR_CHANGE_OPS, + tourChangeRequestSchema, + tourChangeSaveRequestSchema, + tourChangeSchema, +} from './tour-change.schema'; import { describe, expect, it } from 'vitest'; @@ -34,3 +40,32 @@ describe('a change to a saved tour prepared for review (MCP-T02)', () => { } }); }); + +describe('a prepared change the traveller confirmed, sent to be saved (MCP-T03)', () => { + const save = { + changes: [{ op: 'remove', key: 'stop-park' }], + expected_revision: 3, + proposal_id: '0123456789abcdef0123456789abcdef', + confirmed: true, + }; + + it('takes the changes with the revision and the proposal they were prepared as, and the yes', () => { + expect(tourChangeSaveRequestSchema.parse(save)).toEqual(save); + }); + + it('refuses a save without the yes, without the revision, of no proposal, or with anything more', () => { + const refused = [ + { ...save, confirmed: false }, + { ...save, confirmed: undefined }, + { ...save, expected_revision: undefined }, + { ...save, expected_revision: 0 }, + { ...save, proposal_id: 'not-a-proposal' }, + { ...save, proposal_id: '0123456789ABCDEF0123456789ABCDEF' }, + { ...save, note: 'the traveller said so in the chat' }, + { ...save, changes: [] }, + ]; + for (const body of refused) { + expect(tourChangeSaveRequestSchema.safeParse(body).success, JSON.stringify(body)).toBe(false); + } + }); +}); diff --git a/shared/src/tours/tour-change.schema.ts b/shared/src/tours/tour-change.schema.ts index 7d9a855c6..512fd6041 100644 --- a/shared/src/tours/tour-change.schema.ts +++ b/shared/src/tours/tour-change.schema.ts @@ -1,5 +1,5 @@ import { cityWalkStationSchema } from './tour-walk-briefing.schema'; -import { stationKeySchema } from './tours.schema'; +import { stationKeySchema, tourRevisionSchema } from './tours.schema'; import { z } from 'zod'; @@ -46,3 +46,31 @@ export const tourChangeRequestSchema = z }) .strict(); export type TourChangeRequest = z.infer; + +/** + * MCP-T03: the proposal_id a preparation answers, binding a save to the very + * proposal the traveller was shown: the same tour, revision and changes. + */ +export const tourChangeProposalIdSchema = z + .string() + .regex(/^[0-9a-f]{32}$/) + .describe('The proposal_id prepare_tour_change answered with the proposal the traveller confirmed'); + +/** + * MCP-T03: a prepared change the traveller confirmed, sent to be saved. It + * repeats the changes and the revision of the proposal they were shown, names + * that proposal and says yes in so many words: a save without the yes, or of + * anything other than what was prepared, is refused. + */ +export const tourChangeSaveRequestSchema = tourChangeRequestSchema + .extend({ + expected_revision: tourRevisionSchema.describe( + 'base.revision of the proposal: the revision of the tour it was prepared on', + ), + proposal_id: tourChangeProposalIdSchema, + confirmed: z + .literal(true) + .describe('true only once the traveller said yes to exactly this proposal as it was shown to them'), + }) + .strict(); +export type TourChangeSaveRequest = z.infer; diff --git a/wiki/MCP-Addon-Tools.md b/wiki/MCP-Addon-Tools.md index 51748d694..011ea9624 100644 --- a/wiki/MCP-Addon-Tools.md +++ b/wiki/MCP-Addon-Tools.md @@ -233,7 +233,7 @@ Each tool asks for the scope of what it writes, so the scopes differ per tool. T ### Tours _(Tours addon required)_ -The saved tours of a trip, as the Tours list and the tour detail show them, how far a city walk got on a day, paddle legs planned on the water, a city walk prepared from a briefing, and a change to a saved city walk prepared for review. `take_over_tour` and `create_city_walk` write; every other tool here only reads. Tours someone is still drawing in the planner are not saved yet and do not show here. +The saved tours of a trip, as the Tours list and the tour detail show them, how far a city walk got on a day, paddle legs planned on the water, a city walk prepared from a briefing, and a change to a saved city walk prepared for review and saved once the traveller confirmed it. `take_over_tour`, `create_city_walk` and `save_tour_change` write; every other tool here only reads. Tours someone is still drawing in the planner are not saved yet and do not show here. | Tool | Description | Scope | |---|---|---| @@ -248,7 +248,8 @@ The saved tours of a trip, as the Tours list and the tour detail show them, how | `preview_google_maps_walk` | Reads the stops of a Google Maps walking route shared as a link (a `maps.app.goo.gl` short link or a `google.com/maps/dir` link), in order, for a city walk: each stop's name and position, or no position when the link gives none (nothing is looked up or guessed), and the long link that was read. Only a route Google Maps marks as on foot is read; a car, bike or transit route, or a link that does not say, is refused. A short link is followed only to Google Maps itself. Nothing is saved, and the line drawn in Google Maps is not taken over: TREK plans the way on foot between the stops again. Needs `place_edit` on the trip | `trips:read` | | `propose_city_walk` | Proposes a city walk from a briefing without saving anything: start (and end), the time frame on a day, the themes (`culture`, `parks`, `food`, `mixed`), the stations the traveller wished for, the places of the trip it may use and fixed appointments. Places for the themes come from the same search as `search_tour_pois`, up to 8, each with its source, theme and category; nothing is made up. Answers the stations in order with the walking distance and time from the one before, the arrival and leave times, the stay where the traveller gave it (unknown otherwise, which makes later arrivals the earliest possible), and the opening hours checked against the arrival on that day. The order is chosen on estimates; the walk in that order is then planned on foot by the router TREK uses for road trips (the traveller's own routing server where they set one), whose legs give the distances and times. Where it gives no way, because the router cannot be reached or a station lies more than 300 m off every way, the times are estimates over the straight distance with an allowance for streets at 4.5 km/h, and the answer says so and why. Walking and stays are kept apart, a complete total only when every stay is known, with the minutes the frame leaves for the stays not known. Lists free time, the wishes and appointments that cannot be kept as asked, what was left out and why, and `stations_for_create` for `create_city_walk` | `trips:read` | | `create_city_walk` | Creates a city walk from stations the traveller confirmed (for instance `stations_for_create` of `propose_city_walk`), through the same creation the planner's save uses: a key for every station, its name, stay, importance, protection, opening hours and the place of the trip it stands for. The way on foot is planned by the same router as for the proposal, and the walk is saved with it, its distance and its walking time. Where the router gives no way, the walk is saved with its stations only (`route_pending`), says why, and has no line, distance or walking time until someone opens it in the planner, where TREK plans the way on foot and the walk can be saved with it. Needs `place_edit` on the trip | `trips:write` | -| `prepare_tour_change` | Prepares a change to a saved city walk for the traveller to review, without saving anything: no tour, station, visit or day plan changes, and turning it down keeps the plan as it was. The changes apply in order and name the saved stations by their keys: `add` (a new station after a station; a spontaneous stop on the way is an add after the station the traveller is at), `remove`, `replace` and `move`. A protected station, or one visited or skipped on any day of the walk, is never removed, replaced or moved, nothing is put before the last station already done, and a leg drawn by hand stays as drawn, the closing leg of a loop included; such a change is refused, and with it the whole proposal, naming every change and why. Another type of tour, or a walk that is a recorded track without stations of its own, is refused as well: so far only a city walk takes prepared changes. The answer names the tour and the `revision` it was prepared on (a save has to name it, and a tour changed since is refused rather than overwritten), the stations after the change with what changed each and where it stood before, what was removed, the way on foot before and after planned by the same router TREK uses for city walks and its change in metres and minutes (the distance and walking time stored with the walk given apart), the stays as far as known before and after, every leg after the change with the new ones marked, `data_gaps` for what could not be worked out (no way from the router, stays not known, no arrival times, shaping points dropped because the leg they steered changed), a warning for a station of importance must removed or replaced, each new station that is not yet a place of the trip as `create_place` takes it to keep it as a place instead (a station linked to a place of the trip is left out), and whether the traveller may change the trip's places at all (`can_save`) | `trips:read` | +| `prepare_tour_change` | Prepares a change to a saved city walk for the traveller to review, without saving anything: no tour, station, visit or day plan changes, and turning it down keeps the plan as it was. The changes apply in order and name the saved stations by their keys: `add` (a new station after a station; a spontaneous stop on the way is an add after the station the traveller is at), `remove`, `replace` and `move`. A protected station, or one visited or skipped on any day of the walk, is never removed, replaced or moved, nothing is put before the last station already done, and a leg drawn by hand stays as drawn, the closing leg of a loop included; such a change is refused, and with it the whole proposal, naming every change and why. Another type of tour, or a walk that is a recorded track without stations of its own, is refused as well: so far only a city walk takes prepared changes. The answer names the tour and the `revision` it was prepared on (a save has to name it, and a tour changed since is refused rather than overwritten), the stations after the change with what changed each and where it stood before, what was removed, the way on foot before and after planned by the same router TREK uses for city walks and its change in metres and minutes (the distance and walking time stored with the walk given apart), the stays as far as known before and after, every leg after the change with the new ones marked, `data_gaps` for what could not be worked out (no way from the router, stays not known, no arrival times, shaping points dropped because the leg they steered changed), a warning for a station of importance must removed or replaced, each new station that is not yet a place of the trip as `create_place` takes it to keep it as a place instead (a station linked to a place of the trip is left out), whether the traveller may change the trip's places at all (`can_save`) and the `proposal_id` a save of it names | `trips:read` | +| `save_tour_change` | Saves a change `prepare_tour_change` prepared, once the traveller said yes to it exactly as shown: the same changes, the `revision` it was prepared on, its `proposal_id` and `confirmed` set to true. Anything else is refused and nothing is saved. The trip permission and the tour are checked again before anything is written: a tour changed since the proposal was prepared is refused as a conflict rather than overwritten (prepare the change again), and a change the rules no longer allow, a station visited or skipped meanwhile or a place of the trip gone, is refused with every reason. The change is saved whole or not at all, through the same write the planner's save uses: the stations in their order, the stations kept keep their keys and their visits, and the way on foot is planned by the same router as the proposal; where it gives no way the walk keeps its stations only (`route_pending`) until someone opens it in the planner. Every save is kept as a step of the tour with the revision it was prepared on and the one it raised the tour to, the changes and the tour as it was before them, so it can be followed and taken back later; nothing of the conversation is kept. A new walk has no revision yet and is made with `create_city_walk`. Needs `place_edit` on the trip | `trips:write` | | `plan_paddle_route` | Plans the legs of a paddle tour on the water network TREK builds from the paddle regions an admin added: each leg (`from`, `to`) along rivers and canals and across lakes, carried round weirs, dams and waterfalls, through or round locks (`lock_policy`), with `boat` and `lake_style` (`direct` or `shore`). Answers per leg its length, how far its ends were moved onto the water and its stretches (water, lake, whitewater, boat pass, portage with its source, lock, gap); a leg that cannot be planned says why (`not_on_water`, `no_region`, `cross_region`, `no_water_path`, `busy`, `timeout`). Nothing is saved; a planning aid, never a safety or legal clearance | `trips:read` | ### Road trip _(Road trip addon required)_