From e80ae79b7765bfdf447e1b2fdcda7d03ec4a79e8 Mon Sep 17 00:00:00 2001 From: jubnl Date: Sat, 1 Aug 2026 12:45:35 +0200 Subject: [PATCH] chore: move transitItineraryService into nest/transit as pure helpers --- migrate.md | 4 +- migration-graph.md | 80 +++- server/src/nest/README.md | 22 +- .../transit/transit-itinerary.helpers.ts} | 17 +- server/src/nest/transit/transit.helpers.ts | 9 +- server/src/nest/transit/transit.mcp.ts | 5 +- server/src/nest/transit/transit.service.ts | 2 +- .../nest/transit-itinerary.helpers.test.ts | 364 ++++++++++++++++++ 8 files changed, 468 insertions(+), 35 deletions(-) rename server/src/{services/transitItineraryService.ts => nest/transit/transit-itinerary.helpers.ts} (92%) create mode 100644 server/tests/unit/nest/transit-itinerary.helpers.test.ts diff --git a/migrate.md b/migrate.md index d7d54a387..db7c6c776 100644 --- a/migrate.md +++ b/migrate.md @@ -28,11 +28,11 @@ Wave 4 — the coupled cluster (order matters here) dayService; the budget/day coupling lives in the Nest wrapper's budget-sync seam and the MCP registrars, which keep their legacy imports until those domains migrate. Reservations went first as the frontier residue fold; dayService (done 2026-07 — the 592-line service folded into the wrapper `DaysService`, the accommodations seam in `nest/reservations/` now injects it, and the hand-rolled reorder/insert transactions became `db.transaction()`) followed; budgetService followed (done 2026-07 — the 755-line money core folded into the wrapper `BudgetService` (settlement maths, FX freeze/rebase, items/members/payers CRUD; the raw settlement writes became `insertSettlement`/`applySettlementUpdate` so the MCP paths keep skipping the freeze); the 11-tool mcp registrar + 3 budget resources moved to the decorator-driven `budget.mcp.ts`; TripsService/ReservationsService/BookingImportService now inject `BudgetService`; a 4-export `budget.bridge.ts` serves tripService, userCleanupService and the legacy trips/transports registrars; `exchange-rates.bridge.ts` was deleted with its last consumers; and the controller adopted `budget.dto.ts` over the shared schemas, clearing all nine budget allow-list entries — **tripService is now unblocked**). -9. tripService (done 2026-07 — the 1121-line hub folded into the wrapper `TripsService`: generateDays, the updateTrip date-shift transaction, members/guests (#973/#1362), the ICS export, copyTripById and getTripSummary; its six bridge imports became injected services (CollabService/VacayService joined the constructor); the 10-tool trips registrar + 3 trip resources + the trip-summary prompt moved to the decorator-driven `trips.mcp.ts` (first `@Prompt`; the static-token deprecation notice rides the `registry.attach` ctx) and the 3 share-link tools to `share.mcp.ts` on the `canShareTrips` predicate; FeedsService and the plugin RPC host inject `TripsService`; a 3-export `trips.bridge.ts` serves the legacy prompts registrar and budget.mcp.ts's owner/member seam; todo/share/collab/vacay bridges deleted with their last consumers; the controller adopted `trips.dto.ts`, clearing all seven trip allow-list entries). placeService followed (done 2026-07 — the 1029-line place core folded into the wrapper `PlacesService` with its pure half in `places.helpers.ts`; the 10-tool registrar + the trip-places resource moved to the decorator-driven `places.mcp.ts` (`search_place` injects `MapsService`); TripsService/DaysMcp/BookingImportService/the plugin RPC host all inject it, so the domain needs **no bridge at all**; the sibling `placeEnrichment` fold absorbed the last `maps.bridge` consumer and deleted that bridge too, and the DTO ratchet cleared all seven `PlacesController` allow-list entries — **transitItineraryService is the last step-4 link**). +9. tripService (done 2026-07 — the 1121-line hub folded into the wrapper `TripsService`: generateDays, the updateTrip date-shift transaction, members/guests (#973/#1362), the ICS export, copyTripById and getTripSummary; its six bridge imports became injected services (CollabService/VacayService joined the constructor); the 10-tool trips registrar + 3 trip resources + the trip-summary prompt moved to the decorator-driven `trips.mcp.ts` (first `@Prompt`; the static-token deprecation notice rides the `registry.attach` ctx) and the 3 share-link tools to `share.mcp.ts` on the `canShareTrips` predicate; FeedsService and the plugin RPC host inject `TripsService`; a 3-export `trips.bridge.ts` serves the legacy prompts registrar and budget.mcp.ts's owner/member seam; todo/share/collab/vacay bridges deleted with their last consumers; the controller adopted `trips.dto.ts`, clearing all seven trip allow-list entries). placeService followed (done 2026-07 — the 1029-line place core folded into the wrapper `PlacesService` with its pure half in `places.helpers.ts`; the 10-tool registrar + the trip-places resource moved to the decorator-driven `places.mcp.ts` (`search_place` injects `MapsService`); TripsService/DaysMcp/BookingImportService/the plugin RPC host all inject it, so the domain needs **no bridge at all**; the sibling `placeEnrichment` fold absorbed the last `maps.bridge` consumer and deleted that bridge too, and the DTO ratchet cleared all seven `PlacesController` allow-list entries). transitItineraryService followed (done 2026-08 — the last step-4 link and the first pure-helpers relocation with no service fold: the 287-line module is 100% pure (no SQL, no DB, no broadcasts), so the schemas + endpoint/metadata builders moved byte-identical to `nest/transit/transit-itinerary.helpers.ts` (the schemas must stay module-level plain exports — `transit.mcp.ts` consumes them inside `@Tool` decorators, which evaluate before the container exists), its sole consumer `transit.mcp.ts` was a one-import repoint, and a new 21-case `TRANSIT-ITIN-*` characterization suite pins the previously untested superRefine error strings and builder output — **step 4 is complete; the notifications fan-in heads the frontier**). Wave 5 — the heavyweights, last -10. adminService (14 Nest + 11 MCP consumers), journeyService/atlasService/vacayService (done 2026-07 — pulled forward as a zero-dependency frontier member, see migration-graph.md)/collectionsService/mapsService (done 2026-07 — pulled forward as step 4 of the dependency-honest order: the 1429-line geo core folded into the wrapper `MapsService`, pure helpers to `maps.helpers.ts`, the 3 geo MCP tools to the decorator-driven `maps.mcp.ts` (weather/airport tools stay in the legacy registrar), BookingImportService injects it, and a 3-export `maps.bridge.ts` serves placeEnrichment + the places registrar — transitService → placeService now unblocked)/transitService (done 2026-07 — pulled forward as step 5 of the dependency-honest order: the first fully SQL-free domain fold — the 333-line Transitous/MOTIS proxy became a dep-free `TransitService` (no `DatabaseService`); pure `deriveTransitStats` + mode whitelist + itinerary types moved to `transit.helpers.ts` so the downstream `transitItineraryService` needs no bridge; the whole 3-tool registrar moved to the decorator-driven `transit.mcp.ts`, its days/reservations bridge imports becoming injected `DaysService`/`ReservationsService` — zero bridge files, **placeService → transitItineraryService now unblocked**), and finally the auth stack (authService at 1494 lines with 18 MCP consumers, plus +10. adminService (14 Nest + 11 MCP consumers), journeyService/atlasService/vacayService (done 2026-07 — pulled forward as a zero-dependency frontier member, see migration-graph.md)/collectionsService/mapsService (done 2026-07 — pulled forward as step 4 of the dependency-honest order: the 1429-line geo core folded into the wrapper `MapsService`, pure helpers to `maps.helpers.ts`, the 3 geo MCP tools to the decorator-driven `maps.mcp.ts` (weather/airport tools stay in the legacy registrar), BookingImportService injects it, and a 3-export `maps.bridge.ts` serves placeEnrichment + the places registrar — transitService → placeService now unblocked)/transitService (done 2026-07 — pulled forward as step 5 of the dependency-honest order: the first fully SQL-free domain fold — the 333-line Transitous/MOTIS proxy became a dep-free `TransitService` (no `DatabaseService`); pure `deriveTransitStats` + mode whitelist + itinerary types moved to `transit.helpers.ts` so the downstream `transitItineraryService` needed no bridge; the whole 3-tool registrar moved to the decorator-driven `transit.mcp.ts`, its days/reservations bridge imports becoming injected `DaysService`/`ReservationsService` — zero bridge files; placeService → transitItineraryService followed, both done 2026-07/08), and finally the auth stack (authService at 1494 lines with 18 MCP consumers, plus oauthService/oidcService/passkeyService) — highest risk, most consumers, security-sensitive; do it when the pattern is thoroughly proven. backupService also stays late: it owns closeDb/reinitialize lifecycle, which is deliberately outside the DI wrapper. diff --git a/migration-graph.md b/migration-graph.md index e3ee0171f..b4e9729be 100644 --- a/migration-graph.md +++ b/migration-graph.md @@ -1,8 +1,19 @@ # Legacy `src/services/` dependency graph -Generated from the actual imports in `server/src` on **2026-07-30** (after the -placeService migration — the third and final link of step 4 in the -dependency-honest order below: the 1029-line place core (CRUD + ratings SQL, +Generated from the actual imports in `server/src` on **2026-08-01** (after the +transitItineraryService relocation — the fourth and final link of step 4 in the +dependency-honest order below, and the first pure-helpers relocation with no +service fold at all: the 287-line module is 100% pure — no SQL, no DB, no +broadcasts — so its Zod itinerary schemas + endpoint/metadata builders moved +byte-identical to `nest/transit/transit-itinerary.helpers.ts` (the schemas must +stay module-level plain exports: `transit.mcp.ts` consumes them inside +`@Tool({ inputSchema })` decorators, which evaluate at module load before any +container exists), its sole consumer — the in-container `transit.mcp.ts` — was +a one-import repoint, and a new 21-case `TRANSIT-ITIN-*` characterization suite +pins the previously untested superRefine error strings, `??` time fallbacks, +coordinate tolerances and builder output. **Step 4 is complete**; the +notifications fan-in heads the frontier. A day earlier, the placeService +migration (2026-07-30): the 1029-line place core (CRUD + ratings SQL, the GPX/KML/KMZ importers, the Google/Naver list importers) folded into the DI-native `PlacesService`; the pure pieces — the frozen XML parsers, the KMZ unpacker, the dedup predicates, the Google hex-id parsers, `reclaimPhotoCache` @@ -96,9 +107,17 @@ How to read it: `TRANSIT_API_BASE` and lazy User-Agent memo stay module-scoped on purpose; the pure `deriveTransitStats` + `SCHEDULED_TRANSIT_MODES` + itinerary types are plain exports in `nest/transit/transit.helpers.ts` (maps.helpers - precedent) consumed by the legacy `transitItineraryService`; the 3-tool + precedent), consumed since the 2026-08 relocation by the colocated + `transit-itinerary.helpers.ts`; the 3-tool registrar moved to `transit.mcp.ts` and was deleted — no bridge ever existed for this domain), + transit-itinerary (the pure-helpers relocation closing step 4: schemas + + endpoint/metadata builders moved byte-identical from the legacy + `transitItineraryService` to `nest/transit/transit-itinerary.helpers.ts` — + plain exports, no service, no bridge, no DTO, no plugin-host work; the + `distanceService`/`timezoneService` helper imports stay plain, the + `EndpointInput` type import repointed from `reservations.bridge` to + `reservations.service` directly), place (the 1029-line place core folded into `PlacesService`, its pure half into `nest/places/places.helpers.ts`, and the whole MCP surface — 10 tools + the trip-places resource — onto `places.mcp.ts`; the `placeEnrichment` helper was @@ -113,7 +132,7 @@ How to read it: - **Domain migration targets** (the wave material): adminService, airportService, atlasService, authService, backupService, collectionsService, journeyService, journeyShareService, notificationService, oauthService, - oidcService, passkeyService, transitItineraryService, weatherService, wikiService. + oidcService, passkeyService, weatherService, wikiService. - **Cross-cutting Wave-2 targets:** permissions and auditLog are done (2026-07) — see the DI-native list above; only tripAccess remains (delete, don't migrate). - **Helpers that stay as plain modules** (pure/infra, not wave material): avatarUrl, @@ -143,7 +162,6 @@ flowchart TD airport["airportService (boot special case)"]:::ready wiki[wikiService]:::ready collections[collectionsService]:::ready - transitItin["transitItineraryService (stats/types → transit.helpers; bridge tax zero)"]:::ready oauth[oauthService]:::ready end @@ -170,12 +188,15 @@ flowchart TD memories --> notifSvc & admin ``` -(The `placeService` node is gone since the 2026-07-30 fold — its legacy imports were +(The `transitItineraryService` node is gone since the 2026-08-01 relocation — +its `transitService` edge had already become the pure +`nest/transit/transit.helpers` import when the transit domain went DI-native, +and the relocation moved the whole module into `nest/transit/` as +`transit-itinerary.helpers.ts`, so nothing was left to bridge or repoint beyond +the one `transit.mcp.ts` import. The `placeService` node is gone since the +2026-07-30 fold — its legacy imports were all helpers, and the former `placeService → mapsService` edge (via `placeEnrichment`) -died with the helper rather than becoming a repoint. The -`transitItineraryService → transitService` edge became the pure -`nest/transit/transit.helpers` import when the transit domain went DI-native — -helpers never block — so it sits alone at the head of the frontier; +died with the helper rather than becoming a repoint. `notificationService → notifications cluster` is a hard import — the former `mapsService/transitService/webauthnConfig/ oauthService → notifications` edges were only `getAppUrl`/`getMcpSafeUrl` and died with the @@ -208,7 +229,7 @@ already-migrated collab/packing/trips/reservations/vacay services.) | `conflictResult` | (none) | (none) | nest/packing/packing.controller.ts, nest/packing/packing.service.ts, nest/places/places.controller.ts, nest/places/places.service.ts, nest/plugins/host/plugin-host-deps.factory.ts | (none) | | `cookie` | (none) | (none) | nest/auth/auth-public.controller.ts, nest/auth/auth.service.ts, nest/auth/passkey.controller.ts, nest/oidc/oidc.controller.ts, nest/oidc/oidc.service.ts | (none) | | `demo` | (none) | authService | nest/auth/auth.controller.ts, nest/collections/collections.controller.ts, nest/files/files.controller.ts, nest/places/places.controller.ts, nest/plugins/host/plugin-host-deps.factory.ts, nest/trips/trips.controller.ts | middleware/auth.ts, middleware/mfaPolicy.ts | -| `distanceService` | (none) | authService, transitItineraryService | (none) | (none) | +| `distanceService` | (none) | authService | nest/transit/transit-itinerary.helpers.ts | (none) | | `ephemeralTokens` | (none) | authService | nest/files/files.service.ts | index.ts, websocket.ts | | `inAppNotificationActions` | (none) | inAppNotifications | (none) | (none) | | `inAppNotifications` | avatarUrl, inAppNotificationActions, notificationPreferencesService | notificationService | nest/notifications/notifications.service.ts | mcp/resources.ts, mcp/tools/notifications.ts | @@ -227,8 +248,7 @@ already-migrated collab/packing/trips/reservations/vacay services.) | `placeImage` | (none) | collectionsService | nest/collections/collections.controller.ts, nest/common/place-image-upload.ts, nest/places/places.controller.ts, nest/places/places.service.ts | (none) | | `placePhotoCache` | (none) | (none) | nest/maps/maps.service.ts, nest/places/places.helpers.ts, nest/share/share.service.ts | scheduler.ts (lazy) | | `queryHelpers` | (none) | (none) | nest/assignments/assignments.service.ts, nest/days/days.service.ts, nest/places/places.service.ts, nest/share/share.service.ts | (none) | -| `timezoneService` | (none) | airtrail/airtrailMapper, transitItineraryService | nest/trips/trips.service.ts | (none) | -| `transitItineraryService` | distanceService, timezoneService (+ the pure `nest/transit/transit.helpers`, type-only `reservations.bridge`) | (none) | nest/transit/transit.mcp.ts | (none) | +| `timezoneService` | (none) | airtrail/airtrailMapper | nest/transit/transit-itinerary.helpers.ts, nest/trips/trips.service.ts | (none) | | `tripAccess` | (none) | (none) | nest/booking-import/booking-import.service.ts, nest/budget/budget.service.ts, nest/collab/collab.service.ts, nest/days/day-notes.service.ts, nest/integrations/airtrail-import.controller.ts, nest/packing/packing.service.ts, nest/reservations/reservations.service.ts, nest/todo/todo.service.ts | (none) | | `tripMembership` | (none) | authService, oidcService | nest/plugins/host/plugin-host-deps.factory.ts, nest/trip-invite/trip-invite.service.ts | (none) | | `unsplashService` | apiKeyCrypto | (none) | nest/places/places.service.ts, nest/trips/trips.controller.ts, nest/trips/trips.service.ts | (none) | @@ -261,9 +281,14 @@ already-migrated collab/packing/trips/reservations/vacay services.) **Ready frontier** (all legacy deps are helpers, `tripAccess`, or lazy sends): +With step 4 closed by the 2026-08 transit-itinerary relocation, the pick is no +longer on this table: the dependency-honest order's next step is the +**notifications cluster → `notificationService`** fan-in (step 3, deferred +while step 4 ran) — it must precede its admin/memories dependents. The +frontier candidates below can still go any time: + | Candidate | Why now / why not | Bridge tax (legacy dependents + out-of-container) | |---|---|---| -| **transitItineraryService** ← pick | Frontier-ready since the transit fold (its transit edge is the pure `transit.helpers` import; distance/timezone are helpers, the reservations edge is type-only bridge) and now the **last unfinished link of step 4**; its only consumer is the in-container `transit.mcp.ts`, so the fold is a repoint | (none — `nest/transit/transit.mcp.ts` injects/repoints) | | **collectionsService** | Frontier-ready — legacy deps are the placeImage helper + `permissions.bridge`, plus a call-time lazy `import()` of notificationService that a migrated service keeps (collab precedent). Its dedup helpers are collection-scoped ports of the ones that just moved to `places.helpers.ts` — a fold could share them | `mcp/tools/collections.ts` | | **atlasService** | Zero deps, but its legacy dependent is `authService` (Wave-5) → bridge lives long | `authService`; `mcp/tools/atlas.ts`, `mcp/resources.ts` | | **oauthService** | Dependency-free since the Phase 0 addons extraction (adminService edge was only `isAddonEnabled` → `addons.bridge`) — but the coherence order keeps it after admin so the `mcp/oauthProvider.ts` merge (`mcp-2`) lands with it | `mcp/index.ts`, `mcp/oauthProvider.ts` | @@ -313,7 +338,10 @@ Wave-2 `permissions` + `auditLog` pair were the first frontier picks — all don plugin host inject it, so **no bridge**; the sibling `placeEnrichment` fold deleted `maps.bridge` with its last consumer, and the DTO ratchet cleared all seven `PlacesController` allow-list entries) - → **`transitItineraryService`** (the last link — frontier-ready, bridge tax zero) + → `transitItineraryService` (done 2026-08 — the last link: the 100%-pure + module relocated byte-identical to `nest/transit/transit-itinerary.helpers.ts` + as plain exports, `transit.mcp.ts` repointed, no service/bridge/DTO work, a + new 21-case `TRANSIT-ITIN-*` characterization suite — **step 4 complete**) 5. `atlasService` → `authService` (+ oidc/passkey) → `adminService` → `oauthService` 6. `memories/` cluster → `journeyService` → `journeyShareService`; `collectionsService` (frontier-ready since the permissions fold — can also go any time) @@ -385,6 +413,26 @@ Wave-2 `permissions` + `auditLog` pair were the first frontier picks — all don now reached only from `nest/places/*` (kmlImport, placeImage, placePhotoCache, unsplashService) are the residue this fold could have absorbed and did not; none of them blocks anything, so they stay as plain modules for now. +- (2026-08-01 regeneration, post transit-itinerary relocation) The "bridge tax zero" and + "the fold is a repoint" predictions held exactly — and the shape went one step further + than the frontier row implied: there was no fold at all. The module turned out to be + 100% pure (no SQL, no DB access, no broadcasts), so every recipe step except the move + itself was a no-op, and a hard constraint settled the target shape: the Zod schemas are + consumed inside `transit.mcp.ts`'s `@Tool({ inputSchema })` decorators, which evaluate + at module load — before any container exists — so they must stay module-level plain + exports (maps.helpers precedent), not members of an injectable. The type-only + `reservations.bridge` import repointed to `reservations.service` directly (the bridge + only re-exports `EndpointInput`), so the relocation removed one of + `reservations.bridge.ts`'s remaining import sites without touching its runtime exports. + One risk the graph had not tracked: the legacy module had **no direct test suite** — its + 12 superRefine error strings and the endpoint/metadata builder were pinned only through + the 9 MCP transit tool cases — so the relocation added a 21-case `TRANSIT-ITIN-*` + characterization suite (the file now sits inside the ≥80% `src/nest/**` coverage gate). + Still open, now tracked here: the client's `TransitSearchPanel.tsx` hand-duplicates the + `endpoints`/`metadata.transit` build with observable divergences from + `buildTransitReservationParts` (the scheduled-time fallback one is a real defect, fixed + in the trailing commits below); the durable cure is lifting the contract to + `shared/src/`, which remains follow-up work, not part of the relocation. ## Quirks fixed after the place fold (the trailing `fix(server)` commit) diff --git a/server/src/nest/README.md b/server/src/nest/README.md index 304627d18..73ba8f6c7 100644 --- a/server/src/nest/README.md +++ b/server/src/nest/README.md @@ -29,7 +29,7 @@ mount to Nest and leaves the sibling trip routes (days, places, ...) on Express. - **DI-native services (legacy `src/services/*` deleted):** tags, categories, todo, packing, day-notes, trip-invite, assignments, share, settings, files, collab, vacay, reservations, day, permissions, audit, budget, trip, maps, - transit, place — see the migration recipe below. + transit, place, transit-itinerary — see the migration recipe below. ## Cross-cutting Foundation pieces @@ -244,7 +244,8 @@ Transitous/MOTIS proxy became a dep-free `TransitService` (no frozen-at-import `TRANSIT_API_BASE` and lazy User-Agent memo staying module-scoped on purpose; the pure `deriveTransitStats` + mode whitelist + itinerary types moved to `transit.helpers.ts` (maps.helpers precedent) so the -downstream legacy `transitItineraryService` needs no bridge; the whole 3-tool +downstream legacy `transitItineraryService` needed no bridge (since relocated +into the domain as `transit-itinerary.helpers.ts`); the whole 3-tool `mcp/tools/transit.ts` registrar moved to `transit.mcp.ts` — the two geo search tools on `access: { group: 'geo', mode: 'read' }` and `create_transit_journey` on `reservations:write`, with its days/reservations @@ -278,9 +279,22 @@ empty-list short-circuit stays reachable, and retired three bespoke 400 strings — 'Place name is required', 'ids must be an array of numbers' and 'URL is required' — in favour of the pipe envelope, accepting that a malformed body now 400s ahead of the trip-access 404 (the todo/trips trade). +transitItineraryService followed (the first pure-helpers relocation with no +service fold at all — the 287-line module is 100% pure, so the recipe's SQL / +bridge / DTO / plugin-host steps were all no-ops: the Zod itinerary schemas + +endpoint/metadata builders moved byte-identical to +`transit-itinerary.helpers.ts`, next to `transit.helpers.ts`; the schemas +**must** stay module-level plain exports because `transit.mcp.ts` consumes +them inside `@Tool({ inputSchema })` decorators, which evaluate at module load +before any container exists; the sole consumer — the in-container +`transit.mcp.ts` — was a one-import repoint, closing step 4 of the +dependency-honest order; the legacy module had no direct suite, so a new +21-case `TRANSIT-ITIN-*` characterization suite now pins all 12 superRefine +error strings, the `??` time fallbacks, the coordinate tolerances and the +reservation endpoint/metadata builder). Repeat these steps per -service (next up: **transitItineraryService** — -or the notifications fan-in — per the dependency-honest order in +service (next up: **the notifications fan-in** — +per the dependency-honest order in `migration-graph.md`). This is a **pure relocation** — byte-identical SQL, statuses, bodies, and error strings. The plugin RPC host is **no longer a diff --git a/server/src/services/transitItineraryService.ts b/server/src/nest/transit/transit-itinerary.helpers.ts similarity index 92% rename from server/src/services/transitItineraryService.ts rename to server/src/nest/transit/transit-itinerary.helpers.ts index d48da6afa..744745248 100644 --- a/server/src/services/transitItineraryService.ts +++ b/server/src/nest/transit/transit-itinerary.helpers.ts @@ -1,7 +1,16 @@ -import { haversineKm } from './distanceService'; -import type { EndpointInput } from '../nest/reservations/reservations.bridge'; -import { localParts, resolveTimeZone } from './timezoneService'; -import { deriveTransitStats, type TransitItinerary, type TransitLeg } from '../nest/transit/transit.helpers'; +/** + * Pure transit-itinerary helpers — the wire schemas for provider itineraries + * plus the endpoint/metadata builder for automated transit reservations. + * Moved 1:1 from the legacy services/transitItineraryService.ts (identical + * schemas, tolerances, error strings and output shapes). Kept as plain + * exports (the maps.helpers.ts precedent): the schemas are consumed inside + * transit.mcp.ts's @Tool inputSchema decorators, which evaluate at module + * load — before any DI container exists — so they must stay module-level. + */ +import { haversineKm } from '../../services/distanceService'; +import type { EndpointInput } from '../reservations/reservations.service'; +import { localParts, resolveTimeZone } from '../../services/timezoneService'; +import { deriveTransitStats, type TransitItinerary, type TransitLeg } from './transit.helpers'; import { z } from 'zod'; diff --git a/server/src/nest/transit/transit.helpers.ts b/server/src/nest/transit/transit.helpers.ts index d6fe07f98..12f30cb31 100644 --- a/server/src/nest/transit/transit.helpers.ts +++ b/server/src/nest/transit/transit.helpers.ts @@ -1,9 +1,8 @@ /** - * Pure transit helpers shared by TransitService and plain-module code outside - * the container (the legacy services/transitItineraryService imports - * deriveTransitStats + the itinerary types). Kept as plain functions/consts - * (the maps.helpers.ts / files.constants.ts precedent) so no bridge is needed — - * helpers never block a migration. + * Pure transit helpers shared by TransitService and the itinerary + * validation/reservation builders in transit-itinerary.helpers.ts. Kept as + * plain functions/consts (the maps.helpers.ts / files.constants.ts + * precedent) — helpers never block a migration. */ // Modes the client may request — a strict whitelist so the proxy can't be used diff --git a/server/src/nest/transit/transit.mcp.ts b/server/src/nest/transit/transit.mcp.ts index 6d09ce649..c3bfccae8 100644 --- a/server/src/nest/transit/transit.mcp.ts +++ b/server/src/nest/transit/transit.mcp.ts @@ -12,7 +12,7 @@ import { transitCoordinatesSchema, transitItinerarySchema, transitPlaceSchema, -} from '../../services/transitItineraryService'; +} from './transit-itinerary.helpers'; import { hasTripPermission, noAccess, permissionDenied, safeBroadcast } from '../../mcp/tools/_shared'; import { RateLimitService } from '../auth/rate-limit.service'; import { DatabaseService } from '../database/database.service'; @@ -54,8 +54,7 @@ function rateLimit(userId: number, bucket: string, max: number) { * scope group — there is no geo:write) and the `canWrite(scopes, * 'reservations')` early return on create_transit_journey. The * days/reservations bridge imports became injected services; the itinerary - * schemas/helpers still come from the legacy transitItineraryService until - * that domain migrates. + * schemas/helpers live in the colocated transit-itinerary.helpers.ts. */ @McpController() export class TransitMcp { diff --git a/server/src/nest/transit/transit.service.ts b/server/src/nest/transit/transit.service.ts index 3799b40ab..7e05d4c0a 100644 --- a/server/src/nest/transit/transit.service.ts +++ b/server/src/nest/transit/transit.service.ts @@ -126,7 +126,7 @@ function mapStop(p: MotisPlaceRaw | undefined, kind: 'departure' | 'arrival'): T // A missing coordinate becomes 0 (null island) rather than null on // purpose: TransitLegStop.lat/lng are non-nullable across the wire // contract (transitStopSchema, the client picker) — making them nullable - // would ripple through transitItineraryService's validation and drop + // would ripple through transit-itinerary.helpers' validation and drop // otherwise-usable itineraries. MOTIS places carry coordinates in // practice; endpoint stops are re-checked by transitCoordinatesMatch. lat: typeof p?.lat === 'number' ? p.lat : 0, diff --git a/server/tests/unit/nest/transit-itinerary.helpers.test.ts b/server/tests/unit/nest/transit-itinerary.helpers.test.ts new file mode 100644 index 000000000..26783e207 --- /dev/null +++ b/server/tests/unit/nest/transit-itinerary.helpers.test.ts @@ -0,0 +1,364 @@ +/** + * Characterization tests for the pure transit-itinerary helpers — + * TRANSIT-ITIN-001 through TRANSIT-ITIN-021. The module moved 1:1 from the + * legacy services/transitItineraryService.ts, which had no direct suite (its + * behaviour was pinned only through the MCP transit tools); these cases pin + * every superRefine error string and path, the ?? time fallbacks, the + * coordinate tolerances and the reservation endpoint/metadata builder so the + * relocation is provably byte-identical. Everything under test is pure — no + * DB, no container; the db mock only satisfies distanceService's module-top + * import of the connection proxy. + */ +import { describe, it, expect, vi } from 'vitest'; + +vi.mock('../../../src/db/database', () => ({ db: {} })); + +import { + buildTransitReservationParts, + cleanTransitItineraryNames, + effectiveTransitStopTime, + transitCoordinatesMatch, + transitItinerarySchema, +} from '../../../src/nest/transit/transit-itinerary.helpers'; +import type { TransitItinerary, TransitLeg, TransitLegStop } from '../../../src/nest/transit/transit.helpers'; + +// All fixtures sit in Asia/Tokyo (+09:00) so local dates/times are +// deterministic regardless of the host timezone. +const stop = (name: string, lat: number, lng: number, time: string | null, extra: Partial = {}): TransitLegStop => ({ + name, + lat, + lng, + time, + scheduledTime: null, + track: null, + ...extra, +}); + +const leg = (mode: string, from: TransitLegStop, to: TransitLegStop, duration: number, extra: Partial = {}): TransitLeg => ({ + mode, + from, + to, + duration, + distance: null, + headsign: null, + line: null, + lineColor: null, + lineTextColor: null, + agency: null, + intermediateStops: 0, + geometry: null, + geometryPrecision: 6, + ...extra, +}); + +const at = (hhmm: string) => `2026-04-01T${hhmm}:00+09:00`; + +const namba = { name: 'Namba', lat: 34.667, lng: 135.501 }; +const umeda = { name: 'Umeda', lat: 34.702, lng: 135.496 }; + +/** Canonical valid single-leg itinerary: one RAIL leg spanning the itinerary. */ +const singleLeg = (overrides: Partial = {}): TransitItinerary => ({ + startTime: at('09:00'), + endTime: at('09:30'), + duration: 1800, + transfers: 0, + walkSeconds: 0, + legs: [leg('RAIL', stop('Namba', 34.667, 135.501, at('09:00')), stop('Umeda', 34.702, 135.496, at('09:30')), 1800)], + ...overrides, +}); + +/** Two connected RAIL legs (transfer at Umeda), 09:00 → 09:58. */ +const twoLegs = (overrides: Partial = {}): TransitItinerary => ({ + startTime: at('09:00'), + endTime: at('09:58'), + duration: 3480, + transfers: 1, + walkSeconds: 0, + legs: [ + leg('RAIL', stop('Namba', 34.667, 135.501, at('09:00')), stop('Umeda', 34.702, 135.496, at('09:26')), 1560), + leg('RAIL', stop('Umeda', 34.702, 135.496, at('09:30')), stop('Kyoto', 35.0116, 135.7681, at('09:58')), 1680), + ], + ...overrides, +}); + +const issuesOf = (itinerary: TransitItinerary) => { + const result = transitItinerarySchema.safeParse(itinerary); + return result.success ? [] : result.error.issues.map((issue) => ({ message: issue.message, path: issue.path })); +}; + +describe('transitItinerarySchema', () => { + it('TRANSIT-ITIN-001: accepts a canonical itinerary (single leg and two-leg transfer)', () => { + expect(transitItinerarySchema.safeParse(singleLeg()).success).toBe(true); + expect(transitItinerarySchema.safeParse(twoLegs()).success).toBe(true); + }); + + it('TRANSIT-ITIN-002: rejects endTime not after startTime', () => { + expect(issuesOf(singleLeg({ endTime: at('09:00') }))).toContainEqual({ + message: 'endTime must be after startTime', + path: ['endTime'], + }); + }); + + it('TRANSIT-ITIN-003: rejects an all-WALK itinerary', () => { + const walkOnly = singleLeg(); + walkOnly.legs = [{ ...walkOnly.legs[0], mode: 'WALK' }]; + expect(issuesOf(walkOnly)).toContainEqual({ + message: 'At least one scheduled transit leg is required', + path: ['legs'], + }); + }); + + it('TRANSIT-ITIN-004: rejects transfers exceeding the transit leg count', () => { + expect(issuesOf(singleLeg({ transfers: 5 }))).toContainEqual({ + message: 'transfers exceeds the number of transit legs', + path: ['transfers'], + }); + }); + + it('TRANSIT-ITIN-005: caps combined geometry at 60000 chars (per-leg 60000 alone passes)', () => { + const half = 'x'.repeat(40_000); + const oversized = twoLegs(); + oversized.legs = [ + { ...oversized.legs[0], geometry: half }, + { ...oversized.legs[1], geometry: half }, + ]; + expect(issuesOf(oversized)).toContainEqual({ + message: 'Combined transit geometry is too large', + path: ['legs'], + }); + + // Exactly 60000 combined is not "too large" (strict >), and a single leg + // may carry the full budget without tripping the per-leg max. + const boundary = singleLeg(); + boundary.legs = [{ ...boundary.legs[0], geometry: 'x'.repeat(60_000) }]; + expect(transitItinerarySchema.safeParse(boundary).success).toBe(true); + }); + + it('TRANSIT-ITIN-006: rejects a leg missing departure or arrival times', () => { + const timeless = singleLeg(); + timeless.legs = [{ ...timeless.legs[0], from: stop('Namba', 34.667, 135.501, null) }]; + expect(issuesOf(timeless)).toContainEqual({ + message: 'Every leg requires departure and arrival times', + path: ['legs', 0], + }); + }); + + it('TRANSIT-ITIN-007: rejects a leg arriving before it departs', () => { + const backwards = singleLeg({ endTime: at('10:00') }); + backwards.legs = [ + leg('RAIL', stop('Namba', 34.667, 135.501, at('09:30')), stop('Umeda', 34.702, 135.496, at('09:10')), 1800), + ]; + expect(issuesOf(backwards)).toContainEqual({ + message: 'Leg arrival must not precede departure', + path: ['legs', 0], + }); + }); + + it('TRANSIT-ITIN-008: rejects a duration off by more than the 60s tolerance (exactly 60s passes)', () => { + const drifted = singleLeg(); + drifted.legs = [{ ...drifted.legs[0], duration: 1919 }]; // 119s off the 1800s wall-clock + expect(issuesOf(drifted)).toContainEqual({ + message: 'Leg duration does not match its times', + path: ['legs', 0, 'duration'], + }); + + const withinTolerance = singleLeg(); + withinTolerance.legs = [{ ...withinTolerance.legs[0], duration: 1860 }]; // exactly 60s off + expect(transitItinerarySchema.safeParse(withinTolerance).success).toBe(true); + }); + + it('TRANSIT-ITIN-009: rejects leg times outside the itinerary window', () => { + expect(issuesOf(singleLeg({ startTime: at('09:10'), duration: 1200 }))).toContainEqual({ + message: 'Leg times must stay within the itinerary', + path: ['legs', 0], + }); + }); + + it('TRANSIT-ITIN-010: rejects adjacent legs more than 1km apart', () => { + const gapped = twoLegs(); + // Second leg departs ~55km away from the first leg's arrival stop. + gapped.legs = [gapped.legs[0], { ...gapped.legs[1], from: stop('Kobe', 34.69, 135.19, at('09:30')) }]; + expect(issuesOf(gapped)).toContainEqual({ + message: 'Adjacent legs are not connected', + path: ['legs', 1, 'from'], + }); + }); + + it('TRANSIT-ITIN-011: rejects adjacent legs out of chronological order', () => { + const overlapping = twoLegs(); + // Second leg departs at 09:20, before the first leg arrives at 09:26. + overlapping.legs = [ + overlapping.legs[0], + leg('RAIL', stop('Umeda', 34.702, 135.496, at('09:20')), stop('Kyoto', 35.0116, 135.7681, at('09:48')), 1680), + ]; + overlapping.endTime = at('09:48'); + overlapping.duration = 2880; + expect(issuesOf(overlapping)).toContainEqual({ + message: 'Adjacent legs are not chronological', + path: ['legs', 1, 'from', 'time'], + }); + }); + + it('TRANSIT-ITIN-012: rejects a first leg starting away from the itinerary start', () => { + expect(issuesOf(singleLeg({ startTime: at('08:50'), duration: 2400 }))).toContainEqual({ + message: 'First leg must start with the itinerary', + path: ['legs', 0, 'from', 'time'], + }); + }); + + it('TRANSIT-ITIN-013: rejects a last leg ending away from the itinerary end', () => { + expect(issuesOf(singleLeg({ endTime: at('09:40'), duration: 2400 }))).toContainEqual({ + message: 'Last leg must end with the itinerary', + path: ['legs', 0, 'to', 'time'], + }); + }); + + it('TRANSIT-ITIN-014: accepts provider modes outside the request whitelist, rejects non-uppercase tokens', () => { + const airplane = singleLeg(); + airplane.legs = [{ ...airplane.legs[0], mode: 'AIRPLANE' }]; + expect(transitItinerarySchema.safeParse(airplane).success).toBe(true); + + const lowercase = singleLeg(); + lowercase.legs = [{ ...lowercase.legs[0], mode: 'rail' }]; + const result = transitItinerarySchema.safeParse(lowercase); + expect(result.success).toBe(false); + expect(result.error!.issues.some((issue) => issue.path.join('.') === 'legs.0.mode')).toBe(true); + }); +}); + +describe('effectiveTransitStopTime', () => { + it('TRANSIT-ITIN-015: prefers realtime, falls back to scheduled via ?? (empty string passes through)', () => { + expect(effectiveTransitStopTime({ time: at('09:00'), scheduledTime: at('08:55') })).toBe(at('09:00')); + expect(effectiveTransitStopTime({ time: null, scheduledTime: at('08:55') })).toBe(at('08:55')); + expect(effectiveTransitStopTime({ scheduledTime: at('08:55') })).toBe(at('08:55')); + expect(effectiveTransitStopTime({ time: null, scheduledTime: null })).toBeNull(); + expect(effectiveTransitStopTime({})).toBeNull(); + // ?? not || — an empty string is a value, not a miss. + expect(effectiveTransitStopTime({ time: '', scheduledTime: at('08:55') })).toBe(''); + }); +}); + +describe('transitCoordinatesMatch', () => { + it('TRANSIT-ITIN-016: matches within 100m, rejects beyond', () => { + expect(transitCoordinatesMatch(namba, { ...namba })).toBe(true); + // ~55m north — inside the 0.1km tolerance. + expect(transitCoordinatesMatch(namba, { ...namba, lat: namba.lat + 0.0005 })).toBe(true); + // ~555m north — outside. + expect(transitCoordinatesMatch(namba, { ...namba, lat: namba.lat + 0.005 })).toBe(false); + }); +}); + +describe('cleanTransitItineraryNames', () => { + it('TRANSIT-ITIN-017: replaces START/END sentinels without mutating the input', () => { + const itinerary = singleLeg(); + itinerary.legs = [ + leg('RAIL', stop('START', 34.667, 135.501, at('09:00')), stop('END', 34.702, 135.496, at('09:30')), 1800), + ]; + const cleaned = cleanTransitItineraryNames(itinerary, 'Namba', 'Umeda'); + expect(cleaned.legs[0].from.name).toBe('Namba'); + expect(cleaned.legs[0].to.name).toBe('Umeda'); + // Non-sentinel names pass through untouched. + const untouched = cleanTransitItineraryNames(singleLeg(), 'A', 'B'); + expect(untouched.legs[0].from.name).toBe('Namba'); + expect(untouched.legs[0].to.name).toBe('Umeda'); + // The input itinerary is not mutated. + expect(itinerary.legs[0].from.name).toBe('START'); + expect(itinerary.legs[0].to.name).toBe('END'); + }); +}); + +describe('buildTransitReservationParts', () => { + it('TRANSIT-ITIN-018: builds ordered endpoints and the transit metadata for a walk+rail+rail journey', () => { + const kyoto = { name: 'Kyoto', lat: 35.0116, lng: 135.7681 }; + const itinerary: TransitItinerary = { + startTime: at('09:00'), + endTime: at('09:58'), + duration: 3480, + transfers: 1, + walkSeconds: 300, + legs: [ + leg('WALK', stop('Namba entrance', 34.667, 135.501, at('09:00')), stop('Namba station', 34.6675, 135.5015, at('09:05')), 300), + leg( + 'RAIL', + stop('Namba station', 34.6675, 135.5015, at('09:06')), + // Realtime-less transfer stop: the scheduledTime must drive both the + // stop endpoint and the metadata leg times (?? fallback). + stop('Osaka', 34.702, 135.496, null, { scheduledTime: at('09:26'), track: '3' }), + 1200, + ), + leg('RAIL', stop('Osaka', 34.702, 135.496, at('09:30')), stop('Kyoto', 35.0116, 135.7681, at('09:58')), 1680, { + line: 'Special Rapid', + geometry: 'abc', + geometryPrecision: 5, + }), + ], + }; + + const { endpoints, metadata } = buildTransitReservationParts(namba, kyoto, itinerary); + + expect(endpoints).toEqual([ + { role: 'from', sequence: 0, name: 'Namba', code: null, lat: 34.667, lng: 135.501, timezone: 'Asia/Tokyo', local_date: '2026-04-01', local_time: '09:00' }, + { role: 'stop', sequence: 1, name: 'Osaka', code: null, lat: 34.702, lng: 135.496, timezone: 'Asia/Tokyo', local_date: '2026-04-01', local_time: '09:26' }, + { role: 'to', sequence: 2, name: 'Kyoto', code: null, lat: 35.0116, lng: 135.7681, timezone: 'Asia/Tokyo', local_date: '2026-04-01', local_time: '09:58' }, + ]); + + expect(metadata).toEqual({ + transit: { + provider: 'transitous', + duration: 3480, + transfers: 1, + walk_seconds: 300, + legs: [ + { + mode: 'WALK', line: null, line_color: null, line_text_color: null, headsign: null, agency: null, + duration: 300, stops: 0, + from: { name: 'Namba entrance', time: '09:00', track: null }, + to: { name: 'Namba station', time: '09:05', track: null }, + geometry: null, geometry_precision: 6, + }, + { + mode: 'RAIL', line: null, line_color: null, line_text_color: null, headsign: null, agency: null, + duration: 1200, stops: 0, + from: { name: 'Namba station', time: '09:06', track: null }, + to: { name: 'Osaka', time: '09:26', track: '3' }, + geometry: null, geometry_precision: 6, + }, + { + mode: 'RAIL', line: 'Special Rapid', line_color: null, line_text_color: null, headsign: null, agency: null, + duration: 1680, stops: 0, + from: { name: 'Osaka', time: '09:30', track: null }, + to: { name: 'Kyoto', time: '09:58', track: null }, + geometry: 'abc', geometry_precision: 5, + }, + ], + }, + }); + }); + + it('TRANSIT-ITIN-019: a transfer stop with no times at all yields null local date/time', () => { + const kyoto = { name: 'Kyoto', lat: 35.0116, lng: 135.7681 }; + const itinerary = twoLegs(); + itinerary.legs = [ + { ...itinerary.legs[0], to: stop('Umeda', 34.702, 135.496, null) }, + itinerary.legs[1], + ]; + const { endpoints, metadata } = buildTransitReservationParts(namba, kyoto, itinerary); + expect(endpoints[1]).toEqual({ + role: 'stop', sequence: 1, name: 'Umeda', code: null, lat: 34.702, lng: 135.496, + timezone: 'Asia/Tokyo', local_date: null, local_time: null, + }); + expect(metadata.transit.legs[0].to.time).toBeNull(); + }); + + it('TRANSIT-ITIN-020: an unresolvable endpoint timezone throws with the coordinates in the message', () => { + expect(() => buildTransitReservationParts({ name: 'Nowhere', lat: NaN, lng: 135.501 }, umeda, singleLeg())).toThrow( + 'Unable to resolve timezone for NaN,135.501.', + ); + }); + + it('TRANSIT-ITIN-021: an unconvertible itinerary time throws with the ISO string and timezone', () => { + expect(() => buildTransitReservationParts(namba, umeda, singleLeg({ startTime: 'not-a-date' }))).toThrow( + 'Unable to convert not-a-date to local time in Asia/Tokyo.', + ); + }); +});