From 1a945baebe86243bd3fd666dc8ccca7dd0961031 Mon Sep 17 00:00:00 2001 From: jubnl Date: Tue, 11 Aug 2026 16:46:52 +0200 Subject: [PATCH] refactor(server): shrink assignments.bridge to its two live exports --- server/src/nest/README.md | 2 +- .../nest/assignments/assignments.bridge.ts | 41 +++++++------------ .../notifications/notifications.instance.ts | 8 ++-- server/src/nest/realtime/realtime.gateway.ts | 4 +- server/src/nest/realtime/realtime.service.ts | 2 +- server/src/nest/realtime/ws-state.ts | 4 +- server/src/nest/trips/trips.bridge.ts | 21 +++++----- .../unit/nest/assignments.service.test.ts | 4 +- 8 files changed, 37 insertions(+), 49 deletions(-) diff --git a/server/src/nest/README.md b/server/src/nest/README.md index 1c8b324f7..9a2def156 100644 --- a/server/src/nest/README.md +++ b/server/src/nest/README.md @@ -152,7 +152,7 @@ tools, which never pass through an HTTP guard. In the five domains piloted for It replaced `setupWebSocket(server)`, which index.ts kicked off with a dynamic import after listen(). - `ws-state.ts` holds the socket registry, and that is **module state on - purpose**. Seventeen `*.bridge.ts` files and `notifications.instance.ts` + purpose**. The out-of-container bridges and `notifications.instance.ts` build `new RealtimeService()` outside the container; rooms on a provider instance would leave their broadcasts going to an empty map, with no error and no log. Same reasoning as the geo throttle cursor. diff --git a/server/src/nest/assignments/assignments.bridge.ts b/server/src/nest/assignments/assignments.bridge.ts index 82bd53247..3d227507a 100644 --- a/server/src/nest/assignments/assignments.bridge.ts +++ b/server/src/nest/assignments/assignments.bridge.ts @@ -8,25 +8,22 @@ import { JourneyDomainService } from '../journey/journey-domain.service'; import { TrekPhotosRepository } from '../photos/trek-photos.repository'; /** - * Non-Nest entry point for the assignments domain — for code running OUTSIDE - * the Nest container (the legacy places and reservations MCP registrars in - * src/mcp/tools/places.ts and src/mcp/tools/reservations.ts; the assignment - * MCP tools moved to the DI-discovered assignments.mcp.ts, and the plugin RPC - * surface, ItineraryRpc, injects AssignmentsService directly). Exports only the - * legacy services/assignmentService names still consumed outside the - * container, 1:1, so repointing a consumer is an import-path-only diff. Inside - * the container, inject AssignmentsService instead. Delete this file when - * those legacy consumers migrate. + * 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. * - * Unlike todo/packing, the instance is built lazily on first call, NOT at - * module scope: this bridge sits on the mcp tools fan-out while its service - * has a deep legacy-services import closure, so a module-level - * `new AssignmentsService` would crash with an uninitialized class binding if - * a services→mcp import edge ever forms a cycle through here (one existed via - * adminService importing the ../mcp barrel until that was rerouted to - * mcp/sessionManager). 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.) + * 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); @@ -45,11 +42,3 @@ export function createAssignment(dayId: string | number, placeId: string | numbe export function dayExists(dayId: string | number, tripId: string | number) { return assignments().dayExists(dayId, tripId); } - -export function placeExists(placeId: string | number, tripId: string | number) { - return assignments().placeExists(placeId, tripId); -} - -export function getAssignmentForTrip(id: string | number, tripId: string | number) { - return assignments().getAssignmentForTrip(id, tripId); -} diff --git a/server/src/nest/notifications/notifications.instance.ts b/server/src/nest/notifications/notifications.instance.ts index cc55fb5f9..c0b874c5a 100644 --- a/server/src/nest/notifications/notifications.instance.ts +++ b/server/src/nest/notifications/notifications.instance.ts @@ -9,10 +9,10 @@ import { NotificationsService } from './notifications.service'; /** * The out-of-container NotificationsService, for the bridges that hand-build - * their collaborators (trips/reservations/airtrail/packing — the permanent - * cycle-dodges). The reminder crons that used to reach it through a dedicated - * send bridge inject the container singleton now (ReminderJobsService); that - * bridge is gone. + * their collaborators (trips.bridge and airtrail.bridge — the permanent + * cycle-dodges; this file lives exactly as long as either does). The reminder + * crons that used to reach it through a dedicated send bridge inject the + * container singleton now (ReminderJobsService); that bridge is gone. * * One instance, module-scoped, because the channel registry is: the built-ins * registered in the constructor and the channels the plugin runtime pushes in diff --git a/server/src/nest/realtime/realtime.gateway.ts b/server/src/nest/realtime/realtime.gateway.ts index 7cdcba016..6eb24698b 100644 --- a/server/src/nest/realtime/realtime.gateway.ts +++ b/server/src/nest/realtime/realtime.gateway.ts @@ -34,8 +34,8 @@ const HEARTBEAT_INTERVAL = 30_000; * than an interval nobody stops. * * What did NOT move is the socket registry. It stays module-scoped in - * ws-state.ts because seventeen bridges construct `new RealtimeService()` - * outside the container and must see the same rooms; see the note there. + * ws-state.ts because the out-of-container bridges construct + * `new RealtimeService()` and must see the same rooms; see the note there. * * The wire protocol is unchanged, down to the frame names. TrekWsAdapter maps * `{ type }` onto @SubscribeMessage, because the stock adapter dispatches on diff --git a/server/src/nest/realtime/realtime.service.ts b/server/src/nest/realtime/realtime.service.ts index 1b66e27e1..852ac97f6 100644 --- a/server/src/nest/realtime/realtime.service.ts +++ b/server/src/nest/realtime/realtime.service.ts @@ -22,7 +22,7 @@ import { broadcast, broadcastToUser, getOnlineUserIds } from '../../websocket'; * method wasn't asked for — many test mocks provide `broadcast` only. * * The service is deliberately dependency-free so module-level bridge code - * (todo.bridge.ts pattern) can construct it with a bare `new RealtimeService()`. + * (the trips.bridge.ts pattern) can construct it with a bare `new RealtimeService()`. */ @Injectable() export class RealtimeService { diff --git a/server/src/nest/realtime/ws-state.ts b/server/src/nest/realtime/ws-state.ts index 488e7d403..51cb7b155 100644 --- a/server/src/nest/realtime/ws-state.ts +++ b/server/src/nest/realtime/ws-state.ts @@ -6,8 +6,8 @@ import { User } from '../../types'; * The socket registry: rooms, per-socket identity, and the three fan-out * primitives. * - * MODULE state, not provider state, and that is load-bearing. Seventeen - * `*.bridge.ts` files and notifications.instance.ts build `new + * MODULE state, not provider state, and that is load-bearing. The + * out-of-container bridges and notifications.instance.ts build `new * RealtimeService()` by hand, outside the container, because they run outside * it. If the rooms lived on a provider instance, those broadcasts would go to * an empty map: no error, no log, just a client that stops updating. Same diff --git a/server/src/nest/trips/trips.bridge.ts b/server/src/nest/trips/trips.bridge.ts index 7ce3f38f8..a345c137b 100644 --- a/server/src/nest/trips/trips.bridge.ts +++ b/server/src/nest/trips/trips.bridge.ts @@ -28,16 +28,17 @@ import { notificationsInstance } from '../notifications/notifications.instance'; import { EphemeralTokenService } from '../auth/ephemeral-token.service'; /** - * Non-Nest entry point for the trip domain — for the two consumers that cannot - * inject TripsService: the legacy packing-list/budget-overview prompt - * registrar in src/mcp/tools/prompts.ts (outside the container) and - * budget.mcp.ts's owner/member lookups (injecting TripsService there would - * need a forwardRef'd TripsModule↔BudgetModule cycle, so it stays on the - * bridge seam migration-graph.md planned). Exports only the legacy - * services/tripService names they consume, 1:1, so repointing was an - * import-path-only diff. Everything else injects TripsService. Delete this - * file when the prompts registrar migrates and the budget MCP surface can - * inject TripsService without a module cycle. + * Verified-permanent cycle-dodge for the trip domain. Three in-container + * consumers cannot inject the backing services: budget.mcp.ts (getTripOwner / + * listMembers / getTripSummary), packing.mcp.ts (getTripSummary) and + * costs.rpc.ts (listTripsForUser). Every direction was checked 2026-08-11 and + * every one closes a real module cycle — TripsModule imports BudgetModule and + * PackingModule, TripReadModelModule imports both, and TripMembersModule + * imports BudgetModule, so none of TripsService, TripReadModelService or + * TripMembersService can be injected into the budget/packing surfaces without + * forwardRef. Exports only what those three consume, 1:1. Everything else + * injects the services. This file dies if the trip read model ever stops + * importing the budget/packing domains, not before. * * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto * the shared better-sqlite3 singleton. diff --git a/server/tests/unit/nest/assignments.service.test.ts b/server/tests/unit/nest/assignments.service.test.ts index 74c84be5b..fa3d280ca 100644 --- a/server/tests/unit/nest/assignments.service.test.ts +++ b/server/tests/unit/nest/assignments.service.test.ts @@ -51,7 +51,7 @@ 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, placeExists, getAssignmentForTrip } from '../../../src/nest/assignments/assignments.bridge'; +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'; @@ -404,9 +404,7 @@ 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); - expect(placeExists(place.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 }); - expect(getAssignmentForTrip(created!.id, trip.id)).toMatchObject({ id: created!.id, day_id: day.id }); }); });