diff --git a/server/src/nest/assignments/assignments-domain.module.ts b/server/src/nest/assignments/assignments-domain.module.ts new file mode 100644 index 000000000..377744971 --- /dev/null +++ b/server/src/nest/assignments/assignments-domain.module.ts @@ -0,0 +1,22 @@ +import { Module } from '@nestjs/common'; +import { JourneyDomainModule } from '../journey/journey-domain.module'; +import { PermissionsModule } from '../permissions/permissions.module'; +import { QueryHelpersModule } from '../query-helpers/query-helpers.module'; +import { RealtimeModule } from '../realtime/realtime.module'; +import { AssignmentsService } from './assignments.service'; + +/** + * The assignments SERVICE, split from the controller/MCP/RPC surfaces (the + * journey-domain precedent). AssignmentsService itself never touches + * DaysService — only AssignmentsMcp does — so the service can live in a module + * that stays off the DaysModule → PlacesModule → AssignmentsModule loop. + * That is what lets PlacesModule import this and places.mcp.ts inject the + * service, which retired assignments.bridge. Everything here is a leaf: + * none of the four imports reaches the days or places domains. + */ +@Module({ + imports: [PermissionsModule, QueryHelpersModule, JourneyDomainModule, RealtimeModule], + providers: [AssignmentsService], + exports: [AssignmentsService], +}) +export class AssignmentsDomainModule {} diff --git a/server/src/nest/assignments/assignments.bridge.ts b/server/src/nest/assignments/assignments.bridge.ts deleted file mode 100644 index 3d227507a..000000000 --- a/server/src/nest/assignments/assignments.bridge.ts +++ /dev/null @@ -1,44 +0,0 @@ -import { db } from '../../db/database'; -import { DatabaseService } from '../database/database.service'; -import { RealtimeService } from '../realtime/realtime.service'; -import { AssignmentsService } from './assignments.service'; -import { PermissionsService } from '../permissions/permissions.service'; -import { QueryHelpersService } from '../query-helpers/query-helpers.service'; -import { JourneyDomainService } from '../journey/journey-domain.service'; -import { TrekPhotosRepository } from '../photos/trek-photos.repository'; - -/** - * Verified-permanent cycle-dodge for the assignments domain. The one consumer - * is places.mcp.ts, and the cycle is real: DaysModule → PlacesModule → - * AssignmentsModule → DaysModule, so PlacesModule cannot import - * AssignmentsModule back (see the note in places.mcp.ts). Everything else — - * the DI-discovered assignments.mcp.ts, the plugin RPC surface, ItineraryRpc, - * the controllers — injects AssignmentsService. Only the two exports - * places.mcp.ts consumes survive; this file dies if that module cycle is ever - * broken (e.g. an assignments read-model split), not before. - * - * The instance is built lazily on first call, NOT at module scope: the - * consumer is an in-container module evaluated during container assembly, so - * a module-level `new AssignmentsService` would crash with an uninitialized - * class binding if an import edge ever forms a cycle through here. By the - * time any bridge function is called, all modules have finished evaluating. - * (`db` is the reinitialize-proof Proxy onto the shared better-sqlite3 - * singleton.) - */ -function journeyDomain(): JourneyDomainService { - const dbs = new DatabaseService(db); - return new JourneyDomainService(dbs, new RealtimeService(), new TrekPhotosRepository(dbs)); -} - -let instance: AssignmentsService | undefined; -function assignments(): AssignmentsService { - return (instance ??= new AssignmentsService(new DatabaseService(db), new PermissionsService(new DatabaseService(db)), new RealtimeService(), new QueryHelpersService(new DatabaseService(db)), journeyDomain())); -} - -export function createAssignment(dayId: string | number, placeId: string | number, notes: string | null) { - return assignments().createAssignment(dayId, placeId, notes); -} - -export function dayExists(dayId: string | number, tripId: string | number) { - return assignments().dayExists(dayId, tripId); -} diff --git a/server/src/nest/assignments/assignments.module.ts b/server/src/nest/assignments/assignments.module.ts index fe63078c0..3106fd735 100644 --- a/server/src/nest/assignments/assignments.module.ts +++ b/server/src/nest/assignments/assignments.module.ts @@ -1,10 +1,8 @@ import { Module } from '@nestjs/common'; -import { JourneyDomainModule } from '../journey/journey-domain.module'; -import { DaysModule } from '../days/days.module'; import { PermissionsModule } from '../permissions/permissions.module'; -import { QueryHelpersModule } from '../query-helpers/query-helpers.module'; +import { DaysModule } from '../days/days.module'; import { DayAssignmentsController, AssignmentOpsController } from './assignments.controller'; -import { AssignmentsService } from './assignments.service'; +import { AssignmentsDomainModule } from './assignments-domain.module'; import { ItineraryRpc } from './itinerary.rpc'; import { PluginGuardsModule } from '../plugins/host/plugin-guards.module'; import { RealtimeModule } from '../realtime/realtime.module'; @@ -14,14 +12,17 @@ import { AuthModule } from '../auth/auth.module'; /** * Assignments domain (S7 — Phase 2 trip sub-domain). The day-assignments mount * sits under the /api/trips/:tripId/days prefix (S6); the per-assignment ops use - * the /api/trips/:tripId/assignments prefix. AssignmentsService is exported for - * the plugin RPC surface (ItineraryRpc injects it). + * the /api/trips/:tripId/assignments prefix. AssignmentsService lives in + * AssignmentsDomainModule (re-exported here for ReservationsModule and the + * plugin RPC surface) so PlacesModule can reach it without closing the + * DaysModule → PlacesModule → AssignmentsModule loop this module is on. */ @Module({ // DaysModule: AssignmentsMcp injects DaysService for the target-day checks. - imports: [DaysModule, PermissionsModule, QueryHelpersModule, AuthModule, JourneyDomainModule, RealtimeModule, PluginGuardsModule], + // PermissionsModule: the controllers' TripAccessGuard injects PermissionsService. + imports: [AssignmentsDomainModule, DaysModule, PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule], controllers: [DayAssignmentsController, AssignmentOpsController], - providers: [AssignmentsService, AssignmentsMcp, ItineraryRpc], - exports: [AssignmentsService], + providers: [AssignmentsMcp, ItineraryRpc], + exports: [AssignmentsDomainModule], }) export class AssignmentsModule {} diff --git a/server/src/nest/assignments/assignments.service.ts b/server/src/nest/assignments/assignments.service.ts index 3479e1bfe..85523dffa 100644 --- a/server/src/nest/assignments/assignments.service.ts +++ b/server/src/nest/assignments/assignments.service.ts @@ -19,9 +19,8 @@ type Trip = TripAccess; * uses). Trip access rides DatabaseService.canAccessTrip; mutations use * 'day_edit'. The batch tag/participant loaders are injected as * QueryHelpersService (shared with the day, share and place services). - * Non-Nest consumers (the legacy places/reservations MCP registrars) go - * through assignments.bridge.ts; the plugin RPC host and the assignments MCP - * tools inject this class. + * Every consumer injects this class (assignments.bridge.ts is deleted — + * PlacesMcp injects it from AssignmentsDomainModule now). */ @Injectable() export class AssignmentsService { diff --git a/server/src/nest/places/places.mcp.ts b/server/src/nest/places/places.mcp.ts index f9b9da53b..dff83a539 100644 --- a/server/src/nest/places/places.mcp.ts +++ b/server/src/nest/places/places.mcp.ts @@ -6,7 +6,7 @@ import { } from '@trek/nest-mcp'; import { z } from 'zod'; import { AuthService } from '../auth/auth.service'; -import { createAssignment, dayExists } from '../assignments/assignments.bridge'; +import { AssignmentsService } from '../assignments/assignments.service'; import { JourneyDomainService } from '../journey/journey-domain.service'; import { safeBroadcast, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; import { DatabaseService } from '../database/database.service'; @@ -29,13 +29,11 @@ function parseId(value: string | string[]): number | null { * `search_place` stays in this controller even though it calls MapsService: * its legacy gate is `places:read`, not the geo group that maps.mcp.ts carries. * - * The two assignments imports stay on assignments.bridge rather than injecting - * AssignmentsService, and this is now the ONLY MCP controller left on that - * bridge: AssignmentsModule imports DaysModule, and DaysModule imports - * PlacesModule for days.mcp.ts's place creation, so injecting here would close - * a DaysModule → PlacesModule → AssignmentsModule → DaysModule cycle. A real - * cycle, not the module-load problem the others had — reservations.mcp.ts sits - * outside that loop and injects the service now. + * AssignmentsService is injected from AssignmentsDomainModule — the split that + * retired assignments.bridge. The full AssignmentsModule sits on the + * DaysModule → PlacesModule → AssignmentsModule loop (its MCP class injects + * DaysService), but the service itself never did, so its own module stays off + * the loop and PlacesModule can import it. */ @McpController() export class PlacesMcp { @@ -45,6 +43,7 @@ export class PlacesMcp { private readonly db: DatabaseService, private readonly auth: AuthService, private readonly journey: JourneyDomainService, + private readonly assignments: AssignmentsService, ) {} @Tool({ @@ -123,11 +122,11 @@ export class PlacesMcp { if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.db.canAccessTrip(tripId, ctx.userId)) return noAccess(); if (!hasTripPermission('place_edit', tripId, ctx.userId)) return permissionDenied(); - if (!dayExists(dayId, tripId)) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true }; + if (!this.assignments.dayExists(dayId, tripId)) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true }; try { const result = this.db.transaction(() => { const place = this.places.create(String(tripId), { name, description, lat, lng, address, category_id, google_place_id, google_ftid, osm_id, notes: place_notes, website, phone, price, currency }); - const assignment = createAssignment(dayId, place.id, assignment_notes ?? null); + const assignment = this.assignments.createAssignment(dayId, place.id, assignment_notes ?? null); return { place, assignment }; }); safeBroadcast(tripId, 'place:created', { place: result.place }); diff --git a/server/src/nest/places/places.module.ts b/server/src/nest/places/places.module.ts index df2c0f759..5cb78a8c4 100644 --- a/server/src/nest/places/places.module.ts +++ b/server/src/nest/places/places.module.ts @@ -7,6 +7,7 @@ import { PluginGuardsModule } from '../plugins/host/plugin-guards.module'; import { RealtimeModule } from '../realtime/realtime.module'; import { PlacesMcp } from './places.mcp'; import { PermissionsModule } from '../permissions/permissions.module'; +import { AssignmentsDomainModule } from '../assignments/assignments-domain.module'; import { AppConfigModule } from '../app-config/app-config.module'; import { UnsplashModule } from '../unsplash/unsplash.module'; import { PlacePhotosModule } from '../place-photos/place-photos.module'; @@ -23,7 +24,7 @@ import { AuthModule } from '../auth/auth.module'; * is no places.bridge.ts: nothing outside the container consumes this domain. */ @Module({ - imports: [PermissionsModule, QueryHelpersModule, MapsModule, AuthModule, AppConfigModule, UnsplashModule, PlacePhotosModule, JourneyDomainModule, RealtimeModule, PluginGuardsModule], + imports: [PermissionsModule, QueryHelpersModule, MapsModule, AuthModule, AppConfigModule, UnsplashModule, PlacePhotosModule, JourneyDomainModule, RealtimeModule, PluginGuardsModule, AssignmentsDomainModule], controllers: [PlacesController], providers: [PlacesService, PlacesMcp, PlacesRpc], exports: [PlacesService], diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index c25fed79d..c5ec91aa7 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -144,8 +144,7 @@ export function createMcpTestRegistry(): McpRegistry { // one — against the same test DB, which is what makes the `when:` gates // answer truthfully here instead of against the process-wide singleton. const addonsService = new AddonsService(dbService); - // One instance, two consumers: AssignmentsMcp and, since it stopped reaching - // through assignments.bridge, ReservationsMcp. + // One instance, three consumers: AssignmentsMcp, ReservationsMcp and PlacesMcp. const assignmentsService = new AssignmentsService(dbService, permissionsService, realtimeService, queryHelpersService, journeyDomain); return createTestRegistry( [ @@ -169,7 +168,7 @@ export function createMcpTestRegistry(): McpRegistry { new TripPromptsMcp(tripsService, readModelService, packingService, addonsService), new ShareMcp(new ShareService(dbService, new SettingsService(dbService), permissionsService, queryHelpersService, placePhotoCache), authService), new MapsMcp(mapsService), - new PlacesMcp(placesService, mapsService, dbService, authService, journeyDomain), + new PlacesMcp(placesService, mapsService, dbService, authService, journeyDomain, assignmentsService), new CollectionsMcp(new CollectionsService(dbService, permissionsService, realtimeService, notificationsStub()), dbService, authService, addonsService), new TransitMcp(new TransitService(), daysService, reservationsService, dbService, authService), new AtlasMcp(new AtlasService(dbService), addonsService, authService), diff --git a/server/tests/unit/nest/assignments.service.test.ts b/server/tests/unit/nest/assignments.service.test.ts index fa3d280ca..bda5951c4 100644 --- a/server/tests/unit/nest/assignments.service.test.ts +++ b/server/tests/unit/nest/assignments.service.test.ts @@ -5,8 +5,8 @@ * relocated SQL — MAX(order_index)+1 ordering, post-write re-selects, the * time-driven auto-sort with its '99:99' sentinel — plus the hardening that * followed the move: transactional writes, the DB-derived move source day, - * empty-string time clearing, the unified compact tag projection, and the - * assignments.bridge delegation). Uses a real in-memory SQLite DB so SQL + * empty-string time clearing, and the unified compact tag projection; the + * bridge-delegation case died with assignments.bridge). Uses a real in-memory SQLite DB so SQL * logic is exercised faithfully. */ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; @@ -51,7 +51,6 @@ import { createUser, createTrip, addTripMember, createDay, createPlace, createDa import { DatabaseService, type TripAccess } from '../../../src/nest/database/database.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { AssignmentsService } from '../../../src/nest/assignments/assignments.service'; -import { createAssignment, dayExists } from '../../../src/nest/assignments/assignments.bridge'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service'; import { JourneyDomainService } from '../../../src/nest/journey/journey-domain.service'; @@ -398,13 +397,5 @@ describe('setLegTransportMode', () => { }); }); -// ── bridge delegation ───────────────────────────────────────────────────────── - -describe('assignments.bridge', () => { - it('ASG-SVC-026: bridge exports delegate to the same SQL over the shared connection', () => { - const { trip, day, place } = fixture(); - expect(dayExists(day.id, trip.id)).toBe(true); - const created = createAssignment(day.id, place.id, null); - expect(created).toMatchObject({ day_id: day.id, place_id: place.id, order_index: 0 }); - }); -}); +// ASG-SVC-026 pinned the deleted assignments.bridge and died with it — the +// same paths are covered on the service above and via places.mcp injection.