From 42ba5d48494da573ef5858b1edb5b199011d83be Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 28 Aug 2026 13:40:38 +0200 Subject: [PATCH] fix(mcp): keep the categories module out of the auth graph The admin gate on the three new category tools reached for AuthService, which drags the storage registry in behind it, and the categories e2e builds a container around one module against a two-table database. It stopped booting. budget/ already solved this: isDemoUserId reads RuntimeEnvService and the users table directly, and its module comments say why AuthModule is absent. Categories now does the same, which also takes eight hand-built collaborators out of its unit test. The e2e gained RealtimeModule, which is global in the app graph but not in a partial container, the way days.e2e.test.ts already imports it. --- server/src/nest/categories/categories.mcp.ts | 18 +++++-- .../src/nest/categories/categories.module.ts | 11 ++-- server/tests/e2e/categories.e2e.test.ts | 6 ++- server/tests/helpers/mcp-test-controllers.ts | 53 ++++++++++++++++++- .../tests/unit/mcp/tools-categories.test.ts | 30 +++-------- 5 files changed, 83 insertions(+), 35 deletions(-) diff --git a/server/src/nest/categories/categories.mcp.ts b/server/src/nest/categories/categories.mcp.ts index e87162a17..ba47bf1f6 100644 --- a/server/src/nest/categories/categories.mcp.ts +++ b/server/src/nest/categories/categories.mcp.ts @@ -6,7 +6,9 @@ import { } from '../../nest-mcp'; import { z } from 'zod'; import { createCategoryRequestSchema, updateCategoryRequestSchema } from '@trek/shared'; -import { AuthService } from '../auth/auth.service'; +import { DatabaseService } from '../database/database.service'; +import { RuntimeEnvService } from '../app-config/runtime-env.service'; +import { isDemoUserId } from '../common/demo-write'; import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service'; import { adminRequired } from '../../mcp/tools/_shared'; import { CategoriesService } from './categories.service'; @@ -31,10 +33,16 @@ import { CategoriesService } from './categories.service'; export class CategoriesMcp { constructor( private readonly categories: CategoriesService, - private readonly auth: AuthService, + private readonly db: DatabaseService, + private readonly env: RuntimeEnvService, private readonly guards: McpToolGuardsService, ) {} + /** The AuthService.isDemoUser check without the auth graph (demo-write.ts). */ + private isDemoUser(userId: number): boolean { + return isDemoUserId(this.env, this.db, userId); + } + @Tool({ name: 'list_categories', description: 'List all available place categories with their id, name, icon and color. Use category_id when creating or updating places.', @@ -59,7 +67,7 @@ export class CategoriesMcp { access: { group: 'places', mode: 'write' }, }) async createCategory({ name, color, icon }: { name: string; color?: string; icon?: string }, ctx: McpContext) { - if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); + if (this.isDemoUser(ctx.userId)) return demoDenied(); // The palette is instance-wide; the REST route restricts management to admins. Match it. if (!this.guards.isAdminUser(ctx.userId)) return adminRequired(); const category = this.categories.create(ctx.userId, name, color, icon); @@ -79,7 +87,7 @@ export class CategoriesMcp { access: { group: 'places', mode: 'write' }, }) async updateCategory({ categoryId, name, color, icon }: { categoryId: number; name?: string; color?: string; icon?: string }, ctx: McpContext) { - if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); + if (this.isDemoUser(ctx.userId)) return demoDenied(); if (!this.guards.isAdminUser(ctx.userId)) return adminRequired(); if (!this.categories.getById(categoryId)) return errorResult('Category not found'); const category = this.categories.update(categoryId, name, color, icon); @@ -96,7 +104,7 @@ export class CategoriesMcp { access: { group: 'places', mode: 'write' }, }) async deleteCategory({ categoryId }: { categoryId: number }, ctx: McpContext) { - if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); + if (this.isDemoUser(ctx.userId)) return demoDenied(); if (!this.guards.isAdminUser(ctx.userId)) return adminRequired(); if (!this.categories.getById(categoryId)) return errorResult('Category not found'); this.categories.remove(categoryId); diff --git a/server/src/nest/categories/categories.module.ts b/server/src/nest/categories/categories.module.ts index 8a11459b9..22644ecf7 100644 --- a/server/src/nest/categories/categories.module.ts +++ b/server/src/nest/categories/categories.module.ts @@ -1,5 +1,5 @@ import { Module } from '@nestjs/common'; -import { AuthModule } from '../auth/auth.module'; +import { AppConfigModule } from '../app-config/app-config.module'; import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; import { CategoriesController } from './categories.controller'; import { CategoriesMcp } from './categories.mcp'; @@ -8,9 +8,12 @@ import { CategoriesRpc } from './categories.rpc'; /** Categories domain (L4 leaf module). Registered in AppModule. */ @Module({ - // CategoriesMcp gates its three admin tools on AuthService, and McpSharedModule - // is deliberately not @Global, so both are imported rather than inherited. - imports: [AuthModule, McpSharedModule], + // AuthModule is deliberately absent: CategoriesMcp's demo guard reads + // RuntimeEnvService and the users table rather than AuthService, which keeps + // the partial e2e TestingModule for this domain down to two tables. + // McpSharedModule is not @Global, and AppConfigModule is imported explicitly + // for the same reason budget/ does it. + imports: [McpSharedModule, AppConfigModule], controllers: [CategoriesController], providers: [CategoriesService, CategoriesMcp, CategoriesRpc], // For in-container consumers (CategoriesRpc). diff --git a/server/tests/e2e/categories.e2e.test.ts b/server/tests/e2e/categories.e2e.test.ts index 098af48ec..1a872cc17 100644 --- a/server/tests/e2e/categories.e2e.test.ts +++ b/server/tests/e2e/categories.e2e.test.ts @@ -34,6 +34,7 @@ vi.mock('../../src/db/database', () => ({ db, closeDb: () => {}, reinitialize: ( import { CategoriesModule } from '../../src/nest/categories/categories.module'; import { DatabaseModule } from '../../src/nest/database/database.module'; +import { RealtimeModule } from '../../src/nest/realtime/realtime.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; import { ZodValidationPipe } from '../../src/nest/common/zod-validation.pipe'; @@ -49,7 +50,10 @@ describe('Categories e2e (real JwtAuthGuard + AdminGuard + temp SQLite)', () => let app: Awaited>; async function build() { - const moduleRef = await Test.createTestingModule({ imports: [DatabaseModule, CategoriesModule] }).compile(); + // RealtimeModule is @Global in the app graph but not in a partial container, +// and CategoriesModule now pulls McpSharedModule in for the admin tools, whose +// guard service takes it. days.e2e.test.ts imports it for the same reason. + const moduleRef = await Test.createTestingModule({ imports: [DatabaseModule, RealtimeModule, CategoriesModule] }).compile(); const nest = moduleRef.createNestApplication(); nest.use(cookieParser()); nest.useGlobalFilters(new TrekExceptionFilter()); diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index c563aa420..9e130b48e 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -40,6 +40,7 @@ import { ReservationsReadRepository } from '../../src/nest/reservations/reservat import { TagsMcp } from '../../src/nest/tags/tags.mcp'; import { TagsService } from '../../src/nest/tags/tags.service'; import { SettingsService } from '../../src/nest/settings/settings.service'; +import { SettingsMcp } from '../../src/nest/settings/settings.mcp'; import { ShareMcp } from '../../src/nest/share/share.mcp'; import { ShareService } from '../../src/nest/share/share.service'; import { TodoMcp } from '../../src/nest/todo/todo.mcp'; @@ -47,6 +48,7 @@ import { TodoService } from '../../src/nest/todo/todo.service'; import { TransitMcp } from '../../src/nest/transit/transit.mcp'; import { TransitService } from '../../src/nest/transit/transit.service'; import { FilesService } from '../../src/nest/files/files.service'; +import { FilesMcp } from '../../src/nest/files/files.mcp'; import { TripsMcp } from '../../src/nest/trips/trips.mcp'; import { TripsService } from '../../src/nest/trips/trips.service'; import { VacayMcp } from '../../src/nest/vacay/vacay.mcp'; @@ -64,6 +66,10 @@ import { WebauthnConfigService } from '../../src/nest/auth/webauthn-config.servi import { TripMembershipService } from '../../src/nest/trip-membership/trip-membership.service'; import { MailerService } from '../../src/nest/notifications/mailer/mailer.service'; import { CalendarService } from '../../src/nest/calendar/calendar.service'; +import { FeedsMcp } from '../../src/nest/feeds/feeds.mcp'; +import { FeedsService } from '../../src/nest/feeds/feeds.service'; +import { TripInviteMcp } from '../../src/nest/trip-invite/trip-invite.mcp'; +import { TripInviteService } from '../../src/nest/trip-invite/trip-invite.service'; import { AccommodationsService } from '../../src/nest/accommodations/accommodations.service'; import { AccommodationsMcp } from '../../src/nest/accommodations/accommodations.mcp'; import { TripMembersService } from '../../src/nest/trip-members/trip-members.service'; @@ -76,7 +82,32 @@ import { AddonsService } from '../../src/nest/addons/addons.service'; import { notificationsStub } from './notifications'; import { EphemeralTokenService } from '../../src/nest/auth/ephemeral-token.service'; import { AllowedFileTypesService } from '../../src/nest/files/allowed-file-types.service'; +import { MemoriesMcp } from '../../src/nest/memories/memories.mcp'; +import { ImmichService } from '../../src/nest/memories/immich.service'; +import { SynologyService } from '../../src/nest/memories/synology.service'; +import { MemoriesAccessService } from '../../src/nest/memories/memories-access.service'; +import { PhotoCaptureBackfillService } from '../../src/nest/memories/photo-capture-backfill.service'; +import { PhotoResolverService } from '../../src/nest/memories/photo-resolver.service'; +import { ThumbnailService } from '../../src/nest/memories/thumbnail.service'; +import { TrekPhotoCacheService } from '../../src/nest/memories/trek-photo-cache.service'; +import { PhotoProviderRegistry } from '../../src/nest/memories/photo-provider.registry'; +import { ImmichPhotoProvider } from '../../src/nest/memories/providers/immich.provider'; +import { SynologyPhotoProvider } from '../../src/nest/memories/providers/synology.provider'; +import { AuditService } from '../../src/nest/audit/audit.service'; import { makeStorageFixture } from './storage-fixture'; +// No plugin supervisor in this harness, so PluginHooks is built over an inert runtime +// and the warnings tool answers empty by default; the trip-warnings suite spies on +// PluginHooks.prototype to play the provider fan-out. +import { TripWarningsMcp } from '../../src/nest/plugins/contributions/trip-warnings.mcp'; +import { PluginHooks } from '../../src/nest/plugins/plugin-hooks.service'; +import type { PluginRuntimeService } from '../../src/nest/plugins/plugin-runtime.service'; +import { AirtrailMcp } from '../../src/nest/integrations/airtrail.mcp'; +import { AirtrailService } from '../../src/nest/integrations/airtrail.service'; +import { AirtrailClient } from '../../src/nest/integrations/airtrail.client'; +import { AirtrailImportService } from '../../src/nest/integrations/airtrail-import.service'; +import { ReservationImportMcp } from '../../src/nest/reservation-import/reservation-import.mcp'; +import { HelpMcp } from '../../src/nest/help/help.mcp'; +import { AddonsMcp } from '../../src/nest/addons/addons.mcp'; /** * Hand-wired counterpart of the boot-time discovery in McpRegistryService, @@ -157,10 +188,19 @@ export function createMcpTestRegistry(): McpRegistry { const addonsService = new AddonsService(dbService); // One instance, three consumers: AssignmentsMcp, ReservationsMcp and PlacesMcp. const assignmentsService = new AssignmentsService(dbService, permissionsService, realtimeService, queryHelpersService, journeyDomain); + // The two photo providers, shared by MemoriesMcp (which browses them) and by + // the capture backfill JourneyMcp schedules after a provider photo is attached + // (which asks them when and where it was taken). Built for real rather than + // stubbed: an empty provider registry would make the backfill answer "unknown + // provider" for every id and hide a wiring mistake behind a caught error. + const immichService = new ImmichService(dbService, new AuditService(dbService), new MemoriesAccessService(dbService), generalStorage); + const synologyService = new SynologyService(dbService, new MemoriesAccessService(dbService), notificationsStub()); + const trekPhotos = new TrekPhotosRepository(dbService); + const captureBackfill = new PhotoCaptureBackfillService(new PhotoResolverService(trekPhotos, new ThumbnailService(addonsService, generalStorage, dbService), new TrekPhotoCacheService(dbService, generalStorage), new PhotoProviderRegistry([new ImmichPhotoProvider(immichService), new SynologyPhotoProvider(synologyService)]), generalStorage), trekPhotos, generalStorage); return createTestRegistry( [ new TagsMcp(new TagsService(dbService), authService), - new CategoriesMcp(new CategoriesService(dbService), authService, guards), + new CategoriesMcp(new CategoriesService(dbService), dbService, new RuntimeEnvService(), guards), // The weather and airport tools left the legacy mapsWeather registrar. new WeatherMcp(new WeatherService()), new AirportsMcp(), @@ -171,6 +211,7 @@ export function createMcpTestRegistry(): McpRegistry { new ReservationsMcp(reservationsService, daysService, budgetService, authService, assignmentsService, guards), new DayNotesMcp(new DayNotesService(dbService, permissionsService, realtimeService), authService, guards), new DaysMcp(daysService, authService, guards), + new FilesMcp(new FilesService(dbService, permissionsService, realtimeService, new EphemeralTokenService(), generalStorage), authService, guards), new AccommodationsMcp(accommodationsService, dbService, placesService, authService, guards), new AssignmentsMcp(assignmentsService, daysService, authService, guards), new CollabMcp(collabService, authService, addonsService, guards), @@ -178,13 +219,21 @@ export function createMcpTestRegistry(): McpRegistry { new TripsMcp(tripsService, todoService, collabService, authService, calendarService, membersService, readModelService, addonsService, guards), new TripPromptsMcp(tripsService, readModelService, packingService, addonsService), new ShareMcp(new ShareService(dbService, new SettingsService(dbService), permissionsService, queryHelpersService, placePhotoCache), authService, guards), + new FeedsMcp(new FeedsService(dbService, calendarService), dbService, new RuntimeEnvService(), guards), + new TripInviteMcp(new TripInviteService(dbService, permissionsService, new TripMembershipService(dbService)), dbService, new RuntimeEnvService(), guards, new AuditService(dbService)), new MapsMcp(mapsService), new PlacesMcp(placesService, mapsService, dbService, authService, journeyDomain, assignmentsService, guards), new CollectionsMcp(new CollectionsService(dbService, permissionsService, realtimeService, notificationsStub(), generalStorage), dbService, authService, addonsService), new TransitMcp(new TransitService(), daysService, reservationsService, dbService, authService, guards), new AtlasMcp(new AtlasService(dbService), addonsService, authService), - new JourneyMcp(journeyDomain, new JourneyShareService(dbService, journeyDomain, new SettingsService(dbService)), addonsService, authService), + new JourneyMcp(journeyDomain, new JourneyShareService(dbService, journeyDomain, new SettingsService(dbService)), addonsService, authService, captureBackfill), + new MemoriesMcp(immichService, synologyService, dbService), new NotificationsMcp(makeNotificationsService(dbService, realtimeService), authService), + new AirtrailMcp(new AirtrailService(dbService, new AuditService(dbService), new AirtrailClient()), addonsService), + new ReservationImportMcp(new AirtrailImportService(dbService, realtimeService, reservationsService, new AirtrailClient(), new AirtrailService(dbService, new AuditService(dbService), new AirtrailClient())), dbService, authService, guards, addonsService), + new SettingsMcp(new SettingsService(dbService), authService), + new HelpMcp(), new AddonsMcp(addonsService), + new TripWarningsMcp(new PluginHooks({ providersOf: () => [], invokeHook: async () => [] } as unknown as PluginRuntimeService), dbService), ], { accessPolicy: trekMcpAccessPolicy, validateAccess: trekMcpValidateAccess }, ); diff --git a/server/tests/unit/mcp/tools-categories.test.ts b/server/tests/unit/mcp/tools-categories.test.ts index 7068b4f97..63dde98a7 100644 --- a/server/tests/unit/mcp/tools-categories.test.ts +++ b/server/tests/unit/mcp/tools-categories.test.ts @@ -46,18 +46,10 @@ import { trekMcpAccessPolicy, trekMcpValidateAccess } from '../../../src/mcp/nes import { CategoriesMcp } from '../../../src/nest/categories/categories.mcp'; import { CategoriesService } from '../../../src/nest/categories/categories.service'; import { DatabaseService } from '../../../src/nest/database/database.service'; +import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { McpToolGuardsService } from '../../../src/nest/mcp-shared/mcp-tool-guards.service'; -import { AuthService } from '../../../src/nest/auth/auth.service'; -import { BudgetService } from '../../../src/nest/budget/budget.service'; -import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service'; -import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service'; -import { WebauthnConfigService } from '../../../src/nest/auth/webauthn-config.service'; -import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service'; -import { MailerService } from '../../../src/nest/notifications/mailer/mailer.service'; -import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.service'; -import { AllowedFileTypesService } from '../../../src/nest/files/allowed-file-types.service'; beforeAll(() => { createTables(testDb); @@ -82,23 +74,15 @@ async function withHarness( try { await fn(h); } finally { await h.cleanup(); } } -// The write tools reach the auth and guard collaborators, so they run against a -// controller wired here rather than through the shared harness. Everything -// points at this file's DB, which is what lets the admin gate and the demo gate -// be driven from the users table instead of from a stub. +// The write tools reach the guard collaborator, so they run against a controller +// wired here rather than through the shared harness. Everything points at this +// file's DB, which is what lets the admin gate and the demo gate be driven from +// the users table instead of from a stub. const categoriesDb = new DatabaseService(testDb); const categoriesMcp = new CategoriesMcp( new CategoriesService(categoriesDb), - new AuthService( - categoriesDb, - new PermissionsService(categoriesDb), - new TripMembershipService(categoriesDb), - new WebauthnConfigService(categoriesDb), - new UserCleanupService(categoriesDb, new BudgetService(categoriesDb, new PermissionsService(categoriesDb), new ExchangeRatesService(), new RealtimeService())), - new MailerService(categoriesDb), - new EphemeralTokenService(), - new AllowedFileTypesService(categoriesDb), - ), + categoriesDb, + new RuntimeEnvService(), new McpToolGuardsService(categoriesDb, new PermissionsService(categoriesDb), new RealtimeService()), );