From 8b7b91d81499d82b658883f796cdb2f7f8a67c0a Mon Sep 17 00:00:00 2001 From: jubnl Date: Tue, 11 Aug 2026 16:42:49 +0200 Subject: [PATCH] refactor(server): let user-cleanup inject BudgetService and delete budget.bridge --- server/src/nest/auth/auth.bridge.ts | 8 +++-- server/src/nest/auth/auth.module.ts | 8 +++-- server/src/nest/auth/user-cleanup.service.ts | 20 ++++++------ server/src/nest/budget/budget.bridge.ts | 32 ------------------- server/src/nest/budget/budget.mcp.ts | 28 +++++++++------- server/src/nest/budget/budget.module.ts | 11 +++++-- server/src/nest/budget/budget.service.ts | 7 ++-- .../src/nest/budget/exchange-rates.service.ts | 7 ++-- server/src/nest/common/demo-write.ts | 12 +++++++ server/src/nest/trips/trips.bridge.ts | 2 +- server/tests/helpers/mcp-test-controllers.ts | 12 +++---- server/tests/helpers/plugin-host.ts | 2 +- server/tests/unit/mcp/tools-prompts.test.ts | 3 +- server/tests/unit/nest/admin.service.test.ts | 4 ++- .../nest/admin.version-notification.test.ts | 4 ++- server/tests/unit/nest/auth.service.test.ts | 5 ++- .../tests/unit/nest/budget.service.db.test.ts | 23 ++++++------- .../unit/nest/exchange-rates.service.test.ts | 12 +++---- server/tests/unit/nest/oidc.service.test.ts | 5 ++- .../tests/unit/nest/passkey.service.test.ts | 5 ++- .../unit/nest/trip-members.service.test.ts | 4 +-- .../unit/nest/trip-read-model.service.test.ts | 2 +- server/tests/unit/nest/trips.service.test.ts | 4 +-- .../unit/nest/user-cleanup.service.test.ts | 6 ++-- 24 files changed, 117 insertions(+), 109 deletions(-) delete mode 100644 server/src/nest/budget/budget.bridge.ts diff --git a/server/src/nest/auth/auth.bridge.ts b/server/src/nest/auth/auth.bridge.ts index 81fae1ce1..971c79c1e 100644 --- a/server/src/nest/auth/auth.bridge.ts +++ b/server/src/nest/auth/auth.bridge.ts @@ -2,6 +2,9 @@ import { db } from '../../db/database'; import { DatabaseService } from '../database/database.service'; import { PermissionsService } from '../permissions/permissions.service'; import { TripMembershipService } from '../trip-membership/trip-membership.service'; +import { BudgetService } from '../budget/budget.service'; +import { ExchangeRatesService } from '../budget/exchange-rates.service'; +import { RealtimeService } from '../realtime/realtime.service'; import { AuthService } from './auth.service'; import { TokenService } from '../tokens/token.service'; import { MailerService } from '../notifications/mailer/mailer.service'; @@ -21,7 +24,8 @@ import { EphemeralTokenService } from './ephemeral-token.service'; * verifyJwtToken is login identity and deliberately stays on AuthService * (see token.service.ts), so the whole AuthService collaborator graph below * exists to serve that one call. WebauthnConfigService and UserCleanupService - * are constructed only because AuthService takes them — nothing this file + * (with the BudgetService it injects) are constructed only because AuthService + * takes them — nothing this file * exports reaches either one. The pending-MFA and reset-throttle maps are * module-scoped in auth.service.ts, so this instance and the container * singleton share one copy. @@ -36,7 +40,7 @@ const auth = new AuthService( permissions, new TripMembershipService(dbs), new WebauthnConfigService(dbs), - new UserCleanupService(dbs), + new UserCleanupService(dbs, new BudgetService(dbs, permissions, new ExchangeRatesService(), new RealtimeService())), new MailerService(dbs), new EphemeralTokenService(), ); diff --git a/server/src/nest/auth/auth.module.ts b/server/src/nest/auth/auth.module.ts index f970ad50c..059add436 100644 --- a/server/src/nest/auth/auth.module.ts +++ b/server/src/nest/auth/auth.module.ts @@ -12,6 +12,7 @@ import { AuthMcp } from './auth.mcp'; import { UserCleanupService } from './user-cleanup.service'; import { WebauthnConfigService } from './webauthn-config.service'; import { AppConfigModule } from '../app-config/app-config.module'; +import { BudgetModule } from '../budget/budget.module'; import { AuditModule } from '../audit/audit.module'; import { MailerModule } from '../notifications/mailer/mailer.module'; import { PermissionsModule } from '../permissions/permissions.module'; @@ -31,7 +32,10 @@ import { EphemeralTokenModule } from './ephemeral-token.module'; * AtlasModule is deliberately absent. getTravelStats was the only reason for it * and now belongs to AtlasService; the direction is reversed, so AtlasModule * imports AuthModule and atlas.mcp.ts can inject AuthService instead of routing - * through auth.bridge. AuthService is + * through auth.bridge. BudgetModule went the same way: it stopped importing + * AuthModule when BudgetMcp's demo guard moved off AuthService, so this module + * imports it and UserCleanupService injects BudgetService (which retired + * budget.bridge). AuthService is * exported for the in-container consumers (the domain *.mcp.ts demo guards, * OidcService, PasskeyEnabledGuard); PasskeyService for AdminService's passkey * reset; UserCleanupService for the two account-deletion paths (AdminService) @@ -39,7 +43,7 @@ import { EphemeralTokenModule } from './ephemeral-token.module'; * through auth.bridge.ts. */ @Module({ - imports: [EphemeralTokenModule, RateLimitModule, AuditModule, PermissionsModule, TripMembershipModule, MailerModule, AppConfigModule, TokensModule], + imports: [EphemeralTokenModule, RateLimitModule, AuditModule, PermissionsModule, TripMembershipModule, MailerModule, AppConfigModule, TokensModule, BudgetModule], controllers: [AuthPublicController, AuthController, PasskeyController], providers: [AuthService, UserProfileService, RegistrationInvitesService, PasskeyService, UserCleanupService, WebauthnConfigService, AuthMcp], exports: [AuthService, RegistrationInvitesService, PasskeyService, UserCleanupService], diff --git a/server/src/nest/auth/user-cleanup.service.ts b/server/src/nest/auth/user-cleanup.service.ts index 0313070c6..987855f2a 100644 --- a/server/src/nest/auth/user-cleanup.service.ts +++ b/server/src/nest/auth/user-cleanup.service.ts @@ -1,15 +1,10 @@ import fs from 'node:fs'; import { Injectable } from '@nestjs/common'; import { DatabaseService } from '../database/database.service'; -// budget.bridge, not an injected BudgetService: BudgetModule imports AuthModule -// (BudgetMcp's demo guard), so AuthModule importing BudgetModule back would -// close a module cycle — and breaking it the other way would drag -// BudgetController, BudgetMcp and its trips.bridge/MCP `_shared` chain into -// every graph that imports AuthModule, including the single-domain e2e -// TestingModules. Same documented in-container exception as atlas.mcp.ts. -// The bridge instance runs its SQL over the shared db Proxy, and the re-split -// keeps no state, so the second BudgetService is behaviourally identical. -import { removeUserFromBudgetItems } from '../budget/budget.bridge'; +// Injected since BudgetModule dropped its AuthModule import (BudgetMcp's demo +// guard reads RuntimeEnvService + the users table now), which un-closed the +// AuthModule -> BudgetModule cycle that used to force budget.bridge here. +import { BudgetService } from '../budget/budget.service'; import { pluginsDataRoot } from '../plugins/paths'; /** @@ -26,7 +21,10 @@ import { pluginsDataRoot } from '../plugins/paths'; */ @Injectable() export class UserCleanupService { - constructor(private readonly db: DatabaseService) {} + constructor( + private readonly db: DatabaseService, + private readonly budget: BudgetService, + ) {} /** * Erase a user's PLUGIN-held data on account deletion. Two parts: @@ -68,7 +66,7 @@ export class UserCleanupService { private cleanupUserReferences(userId: number): void { this.db.run('UPDATE trip_members SET invited_by = NULL WHERE invited_by = ?', userId); this.db.run('UPDATE budget_items SET paid_by_user_id = NULL WHERE paid_by_user_id = ?', userId); - removeUserFromBudgetItems(userId); + this.budget.removeUserFromBudgetItems(userId); this.db.run('DELETE FROM share_tokens WHERE created_by = ?', userId); this.db.run('DELETE FROM journey_share_tokens WHERE created_by = ?', userId); // Owned journeys cascade-delete their entries/contributors/share_tokens/photos via journey_id FKs diff --git a/server/src/nest/budget/budget.bridge.ts b/server/src/nest/budget/budget.bridge.ts deleted file mode 100644 index 6a906f995..000000000 --- a/server/src/nest/budget/budget.bridge.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { db } from '../../db/database'; -import { DatabaseService } from '../database/database.service'; -import { RealtimeService } from '../realtime/realtime.service'; -import { BudgetService } from './budget.service'; -import { PermissionsService } from '../permissions/permissions.service'; -import { ExchangeRatesService } from './exchange-rates.service'; - -/** - * One in-container exception, and nothing else left: - * UserCleanupService (nest/auth) takes removeUserFromBudgetItems from here - * because BudgetModule imports AuthModule for BudgetMcp's demo guard, so - * AuthModule cannot import BudgetModule back — see the note there. (The budget MCP - * tools and resources moved to the DI-discovered budget.mcp.ts, the plugin - * surface, CostsRpc, injects BudgetService directly, and the trip - * domain folded into the DI-native TripsService, which injects BudgetService — - * its bridge exports were pruned with it). Exports only the legacy - * services/budgetService names still consumed outside the container, 1:1, so - * Inside the container, inject BudgetService instead. The create_transport - * registrar was the last outside-container consumer; it moved into - * reservations.mcp.ts, so this file dies the moment BudgetMcp stops needing - * AuthService. - * - * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto - * the shared better-sqlite3 singleton, and ExchangeRatesService keeps its FX - * cache module-scoped on purpose so this instance and the DI singleton share - * one cache. - */ -const budget = new BudgetService(new DatabaseService(db), new PermissionsService(new DatabaseService(db)), new ExchangeRatesService(), new RealtimeService()); - -export function removeUserFromBudgetItems(userId: number): void { - budget.removeUserFromBudgetItems(userId); -} diff --git a/server/src/nest/budget/budget.mcp.ts b/server/src/nest/budget/budget.mcp.ts index bfd2e6f72..7f7430bcb 100644 --- a/server/src/nest/budget/budget.mcp.ts +++ b/server/src/nest/budget/budget.mcp.ts @@ -5,7 +5,8 @@ import { demoDenied, errorResult, ok, } from '@trek/nest-mcp'; import { z } from 'zod'; -import { AuthService } from '../auth/auth.service'; +import { RuntimeEnvService } from '../app-config/runtime-env.service'; +import { isDemoUserId } from '../common/demo-write'; import { ADDON_IDS } from '../../addons'; import { safeBroadcast, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; import { getTripOwner, listMembers, getTripSummary } from '../trips/trips.bridge'; @@ -69,10 +70,15 @@ export class BudgetMcp { private readonly budget: BudgetService, private readonly exchangeRates: ExchangeRatesService, private readonly db: DatabaseService, - private readonly auth: AuthService, + private readonly env: RuntimeEnvService, readonly addons: AddonsService, ) {} + /** The AuthService.isDemoUser check without the auth graph (demo-write.ts). */ + private isDemoUser(userId: number): boolean { + return isDemoUserId(this.env, this.db, userId); + } + // --- BUDGET --- @Tool({ @@ -100,7 +106,7 @@ export class BudgetMcp { }, ctx: McpContext, ) { - if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); + if (this.isDemoUser(ctx.userId)) return demoDenied(); if (!this.budget.verifyTripAccess(tripId, ctx.userId)) return noAccess(); if (!hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); const members = resolveMemberIds(tripId, member_ids); @@ -125,7 +131,7 @@ export class BudgetMcp { access: { group: 'budget', mode: 'write' }, }) async deleteBudgetItem({ tripId, itemId }: { tripId: number; itemId: number }, ctx: McpContext) { - if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); + if (this.isDemoUser(ctx.userId)) return demoDenied(); if (!this.budget.verifyTripAccess(tripId, ctx.userId)) return noAccess(); if (!hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); const deleted = this.budget.deleteBudgetItem(itemId, tripId); @@ -162,7 +168,7 @@ export class BudgetMcp { }, ctx: McpContext, ) { - if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); + if (this.isDemoUser(ctx.userId)) return demoDenied(); if (!this.budget.verifyTripAccess(tripId, ctx.userId)) return noAccess(); if (!hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); // Freeze-then-write composite (no-op while the schema has no currency input, @@ -196,7 +202,7 @@ export class BudgetMcp { }, ctx: McpContext, ) { - if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); + if (this.isDemoUser(ctx.userId)) return demoDenied(); if (!this.budget.verifyTripAccess(tripId, ctx.userId)) return noAccess(); if (!hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); // Omitted userIds → default to the whole trip, matching create_budget_item. @@ -227,7 +233,7 @@ export class BudgetMcp { access: { group: 'budget', mode: 'write' }, }) async setBudgetItemMembers({ tripId, itemId, userIds }: { tripId: number; itemId: number; userIds: number[] }, ctx: McpContext) { - if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); + if (this.isDemoUser(ctx.userId)) return demoDenied(); if (!this.budget.verifyTripAccess(tripId, ctx.userId)) return noAccess(); if (!hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); const result = this.budget.updateMembers(itemId, tripId, userIds); @@ -251,7 +257,7 @@ export class BudgetMcp { access: { group: 'budget', mode: 'write' }, }) async toggleBudgetMemberPaid({ tripId, itemId, memberId, paid }: { tripId: number; itemId: number; memberId: number; paid: boolean }, ctx: McpContext) { - if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); + if (this.isDemoUser(ctx.userId)) return demoDenied(); if (!this.budget.verifyTripAccess(tripId, ctx.userId)) return noAccess(); if (!hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); const member = this.budget.toggleMemberPaid(itemId, tripId, memberId, paid); @@ -314,7 +320,7 @@ export class BudgetMcp { { tripId, from_user_id, to_user_id, amount }: { tripId: number; from_user_id: number; to_user_id: number; amount: number }, ctx: McpContext, ) { - if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); + if (this.isDemoUser(ctx.userId)) return demoDenied(); if (!this.budget.verifyTripAccess(tripId, ctx.userId)) return noAccess(); if (!hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); // Freeze-then-write composite, same as the REST path (no-op while the @@ -342,7 +348,7 @@ export class BudgetMcp { { tripId, settlementId, from_user_id, to_user_id, amount }: { tripId: number; settlementId: number; from_user_id: number; to_user_id: number; amount: number }, ctx: McpContext, ) { - if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); + if (this.isDemoUser(ctx.userId)) return demoDenied(); if (!this.budget.verifyTripAccess(tripId, ctx.userId)) return noAccess(); if (!hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); // Freeze-then-write composite, same as the REST path. @@ -364,7 +370,7 @@ export class BudgetMcp { access: { group: 'budget', mode: 'write' }, }) async deleteSettlement({ tripId, settlementId }: { tripId: number; settlementId: number }, ctx: McpContext) { - if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); + if (this.isDemoUser(ctx.userId)) return demoDenied(); if (!this.budget.verifyTripAccess(tripId, ctx.userId)) return noAccess(); if (!hasTripPermission('budget_edit', tripId, ctx.userId)) return permissionDenied(); const deleted = this.budget.deleteSettlement(settlementId, tripId); diff --git a/server/src/nest/budget/budget.module.ts b/server/src/nest/budget/budget.module.ts index b39388024..659e18c8d 100644 --- a/server/src/nest/budget/budget.module.ts +++ b/server/src/nest/budget/budget.module.ts @@ -8,13 +8,18 @@ import { CostsRpc } from './costs.rpc'; import { PluginGuardsModule } from '../plugins/host/plugin-guards.module'; import { RealtimeModule } from '../realtime/realtime.module'; import { PermissionsModule } from '../permissions/permissions.module'; -import { AuthModule } from '../auth/auth.module'; +import { AppConfigModule } from '../app-config/app-config.module'; import { AddonsModule } from '../addons/addons.module'; /** Budget domain (S4 — Phase 2 trip sub-domain). Registered in AppModule. - * BudgetMcp carries the decorator-registered MCP tools + resources. */ + * BudgetMcp carries the decorator-registered MCP tools + resources. + * AuthModule is deliberately absent (BudgetMcp's demo guard reads + * RuntimeEnvService + the users table, not AuthService) — that absence is + * what lets AuthModule import BudgetModule for UserCleanupService. + * AppConfigModule is @Global in the app graph; the explicit import keeps the + * partial e2e TestingModules resolving RuntimeEnvService. */ @Module({ - imports: [PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, AddonsModule], + imports: [PermissionsModule, AppConfigModule, RealtimeModule, PluginGuardsModule, AddonsModule], controllers: [BudgetController], providers: [BudgetService, ExchangeRatesService, BudgetMcp, ExchangeRatesRpc, CostsRpc], // For in-container consumers (CostsRpc, TripsService, diff --git a/server/src/nest/budget/budget.service.ts b/server/src/nest/budget/budget.service.ts index a3966847d..eeea15821 100644 --- a/server/src/nest/budget/budget.service.ts +++ b/server/src/nest/budget/budget.service.ts @@ -22,10 +22,9 @@ type SettlementRow = { * services/budgetService.ts: identical statements, the `||` falsy-coercion * defaults, the COALESCE / CASE WHEN sentinel conventions on update and the * post-write re-selects). Trip access, the 'budget_edit' permission and the - * WebSocket broadcast keep their legacy call paths. Non-Nest consumers (the - * legacy tripService/userCleanupService and the legacy MCP trips/transports - * registrars) go through budget.bridge.ts instead of importing this class - * directly. + * WebSocket broadcast keep their legacy call paths. (budget.bridge.ts, the + * former non-Nest entry point, is deleted — its last consumer, + * UserCleanupService, injects this class now.) * * Quirk fixes on top of the relocated legacy behavior: every multi-statement * write now runs in db.transaction() ("transactions are not optional"), diff --git a/server/src/nest/budget/exchange-rates.service.ts b/server/src/nest/budget/exchange-rates.service.ts index bd4626963..d13510029 100644 --- a/server/src/nest/budget/exchange-rates.service.ts +++ b/server/src/nest/budget/exchange-rates.service.ts @@ -15,9 +15,10 @@ import { Injectable } from '@nestjs/common'; * Moved from the legacy `services/exchangeRateService.ts` (the budget-domain * fold): identical cache/coalescing behavior, `||` falsy-coercion defaults and * null degradation. The cache and inflight maps are deliberately MODULE-scoped, - * not instance fields — the DI singleton and the budget.bridge instance wrap - * the same upstream feed, so they must share one cache (same reasoning as - * the permissions.service.ts cache). + * not instance fields — the DI singleton and the out-of-container instances + * (the trips/airtrail/auth bridges each construct their own) wrap the same + * upstream feed, so they must share one cache (same reasoning as the + * permissions-cache.ts cache). * * Post-migration fixes on top of the relocated legacy behavior: the outbound * fetch carries an AbortSignal timeout and a response-size cap, the response is diff --git a/server/src/nest/common/demo-write.ts b/server/src/nest/common/demo-write.ts index 73ee8224a..28360717e 100644 --- a/server/src/nest/common/demo-write.ts +++ b/server/src/nest/common/demo-write.ts @@ -1,4 +1,5 @@ import { RuntimeEnvService } from '../app-config/runtime-env.service'; +import { DatabaseService } from '../database/database.service'; import { isDemoEmail } from './demo'; /** The exact 403 body the six upload endpoints have always returned. */ @@ -22,3 +23,14 @@ export const DEMO_WRITE_ERROR = { export function isDemoWriteBlocked(env: RuntimeEnvService, email: string | null | undefined): boolean { return env.isDemoMode() && isDemoEmail(email); } + +/** + * The user-id flavour of the demo guard — the AuthService.isDemoUser body, + * relocated so surfaces that hold only a user id (the *.mcp.ts demo guards) + * don't need the whole auth graph for one check. + */ +export function isDemoUserId(env: RuntimeEnvService, db: DatabaseService, userId: number): boolean { + if (!env.isDemoMode()) return false; + const user = db.get<{ email: string }>('SELECT email FROM users WHERE id = ?', userId); + return isDemoEmail(user?.email); +} diff --git a/server/src/nest/trips/trips.bridge.ts b/server/src/nest/trips/trips.bridge.ts index 66a43b081..7ce3f38f8 100644 --- a/server/src/nest/trips/trips.bridge.ts +++ b/server/src/nest/trips/trips.bridge.ts @@ -64,7 +64,7 @@ const trips = new TripsService( realtime, new UnsplashService(dbs(), new RuntimeEnvService()), ); -const members = new TripMembersService(dbs(), budget, new UserCleanupService(dbs()), permissions, realtime, notificationsInstance()); +const members = new TripMembersService(dbs(), budget, new UserCleanupService(dbs(), budget), permissions, realtime, notificationsInstance()); const readModel = new TripReadModelService( dbs(), members, diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index ed3e80b31..dfe45f6b3 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -87,20 +87,20 @@ export function createMcpTestRegistry(): McpRegistry { // dropped and four collaborators took its place, but this call site kept the // old shape, so `membership` and `webauthn` held the wrong objects and // userCleanup/mailer/tokens were undefined. + const realtimeService = new RealtimeService(); + const exchangeRatesService = new ExchangeRatesService(); + const budgetService = new BudgetService(dbService, permissionsService, exchangeRatesService, realtimeService); const authService = new AuthService( dbService, permissionsService, new TripMembershipService(dbService), new WebauthnConfigService(dbService), - new UserCleanupService(dbService), + new UserCleanupService(dbService, budgetService), new MailerService(dbService), new EphemeralTokenService(), ); - const realtimeService = new RealtimeService(); const queryHelpersService = new QueryHelpersService(dbService); const daysService = new DaysService(dbService, permissionsService, realtimeService, queryHelpersService); - const exchangeRatesService = new ExchangeRatesService(); - const budgetService = new BudgetService(dbService, permissionsService, exchangeRatesService, realtimeService); const todoService = new TodoService(dbService, permissionsService, realtimeService); const packingService = new PackingService(dbService, permissionsService, realtimeService, notificationsStub()); const collabService = new CollabService(dbService, permissionsService, realtimeService, notificationsStub()); @@ -121,7 +121,7 @@ export function createMcpTestRegistry(): McpRegistry { ); const reservationsService = new ReservationsService(dbService, permissionsService, budgetService, realtimeService, notificationsStub()); const accommodationsService = new AccommodationsService(dbService, permissionsService, realtimeService); - const membersService = new TripMembersService(dbService, budgetService, new UserCleanupService(dbService), permissionsService, realtimeService, notificationsStub()); + const membersService = new TripMembersService(dbService, budgetService, new UserCleanupService(dbService, budgetService), permissionsService, realtimeService, notificationsStub()); const tripsService = new TripsService( dbService, reservationsService, @@ -156,7 +156,7 @@ export function createMcpTestRegistry(): McpRegistry { new AuthMcp(), new TodoMcp(todoService, authService, addonsService), new PackingMcp(packingService, authService, addonsService), - new BudgetMcp(budgetService, exchangeRatesService, dbService, authService, addonsService), + new BudgetMcp(budgetService, exchangeRatesService, dbService, new RuntimeEnvService(), addonsService), new ReservationsMcp(reservationsService, daysService, budgetService, authService, assignmentsService), new DayNotesMcp(new DayNotesService(dbService, permissionsService, realtimeService), authService), new DaysMcp(daysService, authService), diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index 871431e37..f81fdd49a 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -108,7 +108,7 @@ export function createPluginRpcHostFactory(dbs: DatabaseService): PluginRpcHostF const oauth = new PluginOAuthService(dbs); const accommodations = new AccommodationsService(dbs, permissions, realtime); const trips = new TripsService(dbs, reservations, days, permissions, budget, vacay, realtime, unsplash); - const members = new TripMembersService(dbs, budget, new UserCleanupService(dbs), permissions, realtime, notificationsStub()); + const members = new TripMembersService(dbs, budget, new UserCleanupService(dbs, budget), permissions, realtime, notificationsStub()); const guards = new PluginGuards(dbs, permissions, addons); const registry = createTestPluginRegistry([ diff --git a/server/tests/unit/mcp/tools-prompts.test.ts b/server/tests/unit/mcp/tools-prompts.test.ts index d79d0fd66..2bfcb81b4 100644 --- a/server/tests/unit/mcp/tools-prompts.test.ts +++ b/server/tests/unit/mcp/tools-prompts.test.ts @@ -72,6 +72,7 @@ import { PackingMcp } from '../../../src/nest/packing/packing.mcp'; import { PackingService } from '../../../src/nest/packing/packing.service'; import { BudgetMcp } from '../../../src/nest/budget/budget.mcp'; import { BudgetService } from '../../../src/nest/budget/budget.service'; +import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service'; import { AuthMcp } from '../../../src/nest/auth/auth.mcp'; import { DatabaseService } from '../../../src/nest/database/database.service'; @@ -120,7 +121,7 @@ const budgetMcp = new BudgetMcp( new BudgetService(promptDbs(), new PermissionsService(promptDbs()), new ExchangeRatesService(), new RealtimeService()), new ExchangeRatesService(), promptDbs(), - undefined as never, + new RuntimeEnvService(), addonsStub, ); const authMcp = new AuthMcp(); diff --git a/server/tests/unit/nest/admin.service.test.ts b/server/tests/unit/nest/admin.service.test.ts index 5c803b8a2..5409da503 100644 --- a/server/tests/unit/nest/admin.service.test.ts +++ b/server/tests/unit/nest/admin.service.test.ts @@ -70,6 +70,8 @@ import { AuthService } from '../../../src/nest/auth/auth.service'; import { PasskeyService } from '../../../src/nest/auth/passkey.service'; import { PackingService } from '../../../src/nest/packing/packing.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; +import { BudgetService } from '../../../src/nest/budget/budget.service'; +import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service'; import { NotificationsService } from '../../../src/nest/notifications/notifications.service'; import { AdminService } from '../../../src/nest/admin/admin.service'; import { VersionCheckJob } from '../../../src/nest/admin/version-check.job'; @@ -82,7 +84,7 @@ const dbs = new DatabaseService(testDb); const realtime = new RealtimeService(); const permissions = new PermissionsService(dbs); const webauthn = new WebauthnConfigService(dbs); -const userCleanup = new UserCleanupService(dbs); +const userCleanup = new UserCleanupService(dbs, new BudgetService(dbs, permissions, new ExchangeRatesService(), realtime)); // Positional and previously wrong: an AtlasService sat in the membership slot // and the mailer was missing entirely, so `auth` was built with its last four // collaborators shifted by one. Nothing failed, because none of the cases below diff --git a/server/tests/unit/nest/admin.version-notification.test.ts b/server/tests/unit/nest/admin.version-notification.test.ts index 80c158736..9710104ae 100644 --- a/server/tests/unit/nest/admin.version-notification.test.ts +++ b/server/tests/unit/nest/admin.version-notification.test.ts @@ -53,6 +53,8 @@ import { AuthService } from '../../../src/nest/auth/auth.service'; import { PasskeyService } from '../../../src/nest/auth/passkey.service'; import { PackingService } from '../../../src/nest/packing/packing.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; +import { BudgetService } from '../../../src/nest/budget/budget.service'; +import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service'; import { NotificationsService } from '../../../src/nest/notifications/notifications.service'; import { AdminService } from '../../../src/nest/admin/admin.service'; import { makeNotificationsService, makeNotificationPreferencesService } from '../../helpers/notifications'; @@ -62,7 +64,7 @@ const dbs = new DatabaseService(testDb); const realtime = new RealtimeService(); const permissions = new PermissionsService(dbs); const webauthn = new WebauthnConfigService(dbs); -const userCleanup = new UserCleanupService(dbs); +const userCleanup = new UserCleanupService(dbs, new BudgetService(dbs, permissions, new ExchangeRatesService(), realtime)); // Positional and previously wrong: an AtlasService sat in the membership slot // and the mailer was missing entirely, so `auth` was built with its last four // collaborators shifted by one. Nothing failed, because the version-check path diff --git a/server/tests/unit/nest/auth.service.test.ts b/server/tests/unit/nest/auth.service.test.ts index 05f428d5c..24e84b102 100644 --- a/server/tests/unit/nest/auth.service.test.ts +++ b/server/tests/unit/nest/auth.service.test.ts @@ -69,6 +69,9 @@ import { AuthService } from '../../../src/nest/auth/auth.service'; import { TokenService } from '../../../src/nest/tokens/token.service'; import * as authBridge from '../../../src/nest/auth/auth.bridge'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; +import { BudgetService } from '../../../src/nest/budget/budget.service'; +import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service'; +import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { verifyJwtAndLoadUser } from '../../../src/nest/auth/jwt-verify'; import { authenticator } from 'otplib'; @@ -98,7 +101,7 @@ const svc = new AuthService( new PermissionsService(new DatabaseService(testDb)), membershipStub, new WebauthnConfigService(new DatabaseService(testDb)), - new UserCleanupService(new DatabaseService(testDb)), + new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new ExchangeRatesService(), new RealtimeService())), mailerStub, new EphemeralTokenService(), ); diff --git a/server/tests/unit/nest/budget.service.db.test.ts b/server/tests/unit/nest/budget.service.db.test.ts index 3a9771153..7cd2ec8c0 100644 --- a/server/tests/unit/nest/budget.service.db.test.ts +++ b/server/tests/unit/nest/budget.service.db.test.ts @@ -2,8 +2,8 @@ * DB-backed unit tests for BudgetService trip-scoping (BUDGET-SVC-DB-001+). * Uses a real in-memory SQLite DB so the SQL WHERE clauses are exercised. * BUDGET-SVC-DB-001 through 014 moved 1:1 from the legacy - * tests/unit/services/budgetServiceDb.test.ts; 015–018 pin the budget.bridge - * delegation (the bridge instance runs over the same mocked db Proxy); + * tests/unit/services/budgetServiceDb.test.ts; 015–018 pinned the deleted + * budget.bridge's delegation and now pin the same paths on the service; * 019–020 pin the post-fold quirk fixes (COALESCE(display_name) on * settlements, transactional multi-statement writes). */ @@ -41,9 +41,9 @@ const { RATES } = vi.hoisted(() => ({ EUR: { EUR: 1, USD: 1.1429, RUB: 87.63 }, } as Record>, })); -// Constructor-injected since the fold; the class is mocked at the module path so -// the budget.bridge instance (which news up its own ExchangeRatesService) sees -// the same deterministic rates as the SUT. +// Constructor-injected since the fold; the class is mocked at the module path +// so every separately-constructed instance sees the same deterministic rates +// as the SUT. vi.mock('../../../src/nest/budget/exchange-rates.service', () => ({ ExchangeRatesService: class { async getRates(base: string) { @@ -60,7 +60,6 @@ import { BudgetService } from '../../../src/nest/budget/budget.service'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service'; -import * as bridge from '../../../src/nest/budget/budget.bridge'; import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { TripMembersService } from '../../../src/nest/trip-members/trip-members.service'; @@ -89,7 +88,7 @@ const dbs = () => new DatabaseService(testDb); const membersSvc = new TripMembersService( dbs(), budget, - new UserCleanupService(dbs()), + new UserCleanupService(dbs(), budget), new PermissionsService(dbs()), new RealtimeService(), notificationsStub(), @@ -401,12 +400,10 @@ describe('rebaseTripCurrency', () => { }); }); -describe('budget.bridge delegation', () => { - // Every bridge export except removeUserFromBudgetItems has been pruned as its - // last outside-container consumer migrated (legacy tripService, the trips MCP - // registrar, services/userCleanupService, and the create_transport registrar, - // which moved into reservations.mcp.ts). 015-018 pin the same behavior on the - // service. +describe('composite service paths (ex budget.bridge delegation)', () => { + // budget.bridge is deleted — UserCleanupService injects BudgetService now + // that BudgetModule no longer imports AuthModule. 015-018 kept their IDs and + // pin the same behavior directly on the service. it('BUDGET-SVC-DB-015: listBudgetItems returns the hydrated list', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); diff --git a/server/tests/unit/nest/exchange-rates.service.test.ts b/server/tests/unit/nest/exchange-rates.service.test.ts index 5571c9f54..f272010a4 100644 --- a/server/tests/unit/nest/exchange-rates.service.test.ts +++ b/server/tests/unit/nest/exchange-rates.service.test.ts @@ -5,9 +5,9 @@ * coverage gate. 001–012 pin the fetch/cache behavior (including the parity * quirks kept on purpose: `|| 'EUR'` falsy coercion, stale-cache fallback, the * `>1 keys` failure heuristic); 019 pins the module-scoped cache shared across - * instances (the DI singleton and the budget.bridge instance wrap one feed — - * originally pinned via the exchange-rates.bridge, deleted with the budget - * fold); 021–022 pin the post-fold quirk fixes (AbortSignal timeout, + * instances (the DI singleton and the out-of-container bridge instances wrap + * one feed — originally pinned via the exchange-rates.bridge, deleted with the + * budget fold); 021–022 pin the post-fold quirk fixes (AbortSignal timeout, * response-size cap, logged failures). 013–018 and 020 covered the dead * convertWithRates export and were removed with it in the quirk fixes. * @@ -140,12 +140,12 @@ describe('ExchangeRatesService.getRates', () => { }); describe('cross-instance cache sharing', () => { - it('FX-SVC-019: a second instance (the budget.bridge shape) serves the module-scoped cache', async () => { + it('FX-SVC-019: a second instance (the out-of-container bridge shape) serves the module-scoped cache', async () => { // Prime the cache through the DI-style instance… const primed = await svc.getRates('AUD'); expect(fetchMock).toHaveBeenCalledTimes(1); - // …then a separately-constructed instance (as budget.bridge news up) must - // serve the same cached feed instead of refetching. + // …then a separately-constructed instance (as the trips/airtrail/auth + // bridges new up) must serve the same cached feed instead of refetching. expect(await new ExchangeRatesService().getRates('AUD')).toBe(primed); expect(fetchMock).toHaveBeenCalledTimes(1); }); diff --git a/server/tests/unit/nest/oidc.service.test.ts b/server/tests/unit/nest/oidc.service.test.ts index dfaf17b4c..060b5423c 100644 --- a/server/tests/unit/nest/oidc.service.test.ts +++ b/server/tests/unit/nest/oidc.service.test.ts @@ -83,6 +83,9 @@ import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; +import { BudgetService } from '../../../src/nest/budget/budget.service'; +import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service'; +import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service'; import { AuthService } from '../../../src/nest/auth/auth.service'; import { WebauthnConfigService } from '../../../src/nest/auth/webauthn-config.service'; @@ -105,7 +108,7 @@ const auth = new AuthService( new PermissionsService(new DatabaseService(testDb)), membership, new WebauthnConfigService(new DatabaseService(testDb)), - new UserCleanupService(new DatabaseService(testDb)), + new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new ExchangeRatesService(), new RealtimeService())), mailerStub, new EphemeralTokenService(), ); diff --git a/server/tests/unit/nest/passkey.service.test.ts b/server/tests/unit/nest/passkey.service.test.ts index 44b844be0..f84ed289e 100644 --- a/server/tests/unit/nest/passkey.service.test.ts +++ b/server/tests/unit/nest/passkey.service.test.ts @@ -75,6 +75,9 @@ import { resetTestDb } from '../../helpers/test-db'; import { createUser } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; +import { BudgetService } from '../../../src/nest/budget/budget.service'; +import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service'; +import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service'; import { AuthService } from '../../../src/nest/auth/auth.service'; import { PasskeyService } from '../../../src/nest/auth/passkey.service'; @@ -107,7 +110,7 @@ const auth = new AuthService( new PermissionsService(new DatabaseService(testDb)), new TripMembershipService(new DatabaseService(testDb)), new WebauthnConfigService(new DatabaseService(testDb)), - new UserCleanupService(new DatabaseService(testDb)), + new UserCleanupService(new DatabaseService(testDb), new BudgetService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new ExchangeRatesService(), new RealtimeService())), mailerStub, new EphemeralTokenService(), ); diff --git a/server/tests/unit/nest/trip-members.service.test.ts b/server/tests/unit/nest/trip-members.service.test.ts index 7b3d4bc97..a37806f9f 100644 --- a/server/tests/unit/nest/trip-members.service.test.ts +++ b/server/tests/unit/nest/trip-members.service.test.ts @@ -68,7 +68,7 @@ import { notificationsStub } from '../../helpers/notifications'; const dbs = () => new DatabaseService(testDb); const budgetSvc = new BudgetService(dbs(), new PermissionsService(dbs()), new ExchangeRatesService(), new RealtimeService()); -const roster = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs()), new PermissionsService(dbs()), new RealtimeService(), notificationsStub(notifySend)); +const roster = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc), new PermissionsService(dbs()), new RealtimeService(), notificationsStub(notifySend)); /** * A roster whose connection reports "no such row" for the first result of every @@ -98,7 +98,7 @@ function rosterWithMissingRow(match: string) { }, }); const frozen = { connection: conn, canAccessTrip: dbMock.canAccessTrip, isOwner: dbMock.isOwner } as unknown as DatabaseService; - return new TripMembersService(frozen, budgetSvc, new UserCleanupService(dbs()), new PermissionsService(dbs()), new RealtimeService(), notificationsStub(notifySend)); + return new TripMembersService(frozen, budgetSvc, new UserCleanupService(dbs(), budgetSvc), new PermissionsService(dbs()), new RealtimeService(), notificationsStub(notifySend)); } beforeAll(() => { diff --git a/server/tests/unit/nest/trip-read-model.service.test.ts b/server/tests/unit/nest/trip-read-model.service.test.ts index fdb3171bc..7b189c37c 100644 --- a/server/tests/unit/nest/trip-read-model.service.test.ts +++ b/server/tests/unit/nest/trip-read-model.service.test.ts @@ -90,7 +90,7 @@ const placesSvc = new PlacesService( new JourneyDomainService(dbs(), new RealtimeService(), new TrekPhotosRepository(dbs())), ); const accommodationsSvc = new AccommodationsService(dbs(), new PermissionsService(dbs()), new RealtimeService()); -const membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs()), new PermissionsService(dbs()), new RealtimeService(), notificationsStub()); +const membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc), new PermissionsService(dbs()), new RealtimeService(), notificationsStub()); const buildReadModel = (database: DatabaseService, roster: TripMembersService = membersSvc) => new TripReadModelService( diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts index 78219402c..a8cb5f8ea 100644 --- a/server/tests/unit/nest/trips.service.test.ts +++ b/server/tests/unit/nest/trips.service.test.ts @@ -120,7 +120,7 @@ const svc = new TripsService( new RealtimeService(), undefined as never, // unsplash — not exercised here ); -const membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs()), new PermissionsService(dbs()), new RealtimeService(), notificationsStub()); +const membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc), new PermissionsService(dbs()), new RealtimeService(), notificationsStub()); const readModelSvc = new TripReadModelService( dbs(), membersSvc, daysSvc, accommodationsSvc, budgetSvc, new PackingService(dbs(), new PermissionsService(dbs()), new RealtimeService(), notificationsStub()), @@ -953,7 +953,7 @@ describe('quirk fixes', () => { /** Same frozen connection, for the guest deletion that now lives on the roster. */ function failingMembers(match: string) { return new TripMembersService( - failingConnection(match), budgetSvc, new UserCleanupService(dbs()), + failingConnection(match), budgetSvc, new UserCleanupService(dbs(), budgetSvc), new PermissionsService(dbs()), new RealtimeService(), notificationsStub(), ); diff --git a/server/tests/unit/nest/user-cleanup.service.test.ts b/server/tests/unit/nest/user-cleanup.service.test.ts index ec19e4888..f8cdfa1af 100644 --- a/server/tests/unit/nest/user-cleanup.service.test.ts +++ b/server/tests/unit/nest/user-cleanup.service.test.ts @@ -52,7 +52,7 @@ import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service' const dbs = new DatabaseService(testDb); const budget = new BudgetService(dbs, new PermissionsService(dbs), new ExchangeRatesService(), new RealtimeService()); -const svc = new UserCleanupService(dbs); +const svc = new UserCleanupService(dbs, budget); const installPlugin = (id: string, permissions: string[] | null) => { testDb.prepare('INSERT INTO plugins (id, name, version, permissions) VALUES (?, ?, ?, ?)') @@ -146,7 +146,7 @@ describe('erasePluginUserData', () => { const slim = new (require('better-sqlite3'))(':memory:'); slim.exec('CREATE TABLE users (id INTEGER PRIMARY KEY)'); slim.prepare('INSERT INTO users (id) VALUES (1)').run(); - const slimSvc = new UserCleanupService(new DatabaseService(slim)); + const slimSvc = new UserCleanupService(new DatabaseService(slim), budget); expect(() => slimSvc.erasePluginUserData(1)).not.toThrow(); @@ -215,7 +215,7 @@ describe('deleteUserCompletely', () => { return realRun(sql, ...params); }); - expect(() => new UserCleanupService(failing).deleteUserCompletely(victim.id)).toThrow('boom'); + expect(() => new UserCleanupService(failing, budget).deleteUserCompletely(victim.id)).toThrow('boom'); expect(testDb.prepare('SELECT id FROM users WHERE id = ?').get(victim.id)).toBeDefined(); expect((testDb.prepare('SELECT invited_by FROM trip_members WHERE user_id = ?').get(owner.id) as { invited_by: number | null }).invited_by).toBe(victim.id);