From 2d0eca7938c5c9797ead3df04f34cafa5e7c3977 Mon Sep 17 00:00:00 2001 From: Maurice Date: Mon, 10 Aug 2026 15:42:27 +0200 Subject: [PATCH] Let MCP addon gates read an injected service Nine *.mcp.ts files reached through addons.bridge for one boolean each, and platform.routes plus the legacy registrars still do. That was never a preference: a decorator's options object is built while the class is being defined, so the `when:` closure had no `this` to reach an injected service through, and the only thing in scope was a module-level singleton that builds its own AddonsService outside the container. Nine domains hid an edge to the addons domain from the module graph to answer "is this addon on". @trek/nest-mcp hands the gate its declaring instance now, resolved at attach rather than captured at module load, and addons/addon-gate.ts turns that into addonGate(ADDON_IDS.X) over the injected service. The option is declared with method syntax so a predicate written against its own controller class stays assignable. Two more edges went the same way, both checked for cycles first: reservations.mcp injects AssignmentsService (AssignmentsModule never reaches back), atlas.mcp and journey.mcp inject AuthService. The tests that forced a toggle by mocking the bridge module now write the addon row the admin panel writes, so they exercise the real read. Four in-container uses stay, each documented in the nest README: places.mcp (a real Days -> Places -> Assignments cycle), budget/packing/costs against trips.bridge, user-cleanup against budget.bridge, and the two multer interceptor options, whose fix is MulterModule.registerAsync. --- nest-mcp/README.md | 4 +- nest-mcp/src/registry.ts | 10 +++-- nest-mcp/src/types.ts | 14 ++++++- nest-mcp/tests/attach.test.ts | 27 ++++++++++++ server/src/nest/README.md | 29 +++++++++++++ server/src/nest/addons/addon-gate.ts | 40 ++++++++++++++++++ server/src/nest/atlas/atlas.mcp.ts | 35 +++++++--------- server/src/nest/atlas/atlas.module.ts | 4 +- server/src/nest/budget/budget.mcp.ts | 6 ++- server/src/nest/budget/budget.module.ts | 3 +- server/src/nest/collab/collab.mcp.ts | 15 ++++--- server/src/nest/collab/collab.module.ts | 3 +- .../src/nest/collections/collections.mcp.ts | 6 ++- server/src/nest/journey/journey.mcp.ts | 42 +++++++++---------- server/src/nest/journey/journey.module.ts | 3 +- server/src/nest/packing/packing.mcp.ts | 11 +++-- server/src/nest/packing/packing.module.ts | 3 +- server/src/nest/places/places.mcp.ts | 11 ++--- .../src/nest/reservations/reservations.mcp.ts | 15 ++++--- .../nest/reservations/reservations.module.ts | 3 +- server/src/nest/todo/todo.mcp.ts | 11 +++-- server/src/nest/todo/todo.module.ts | 3 +- server/src/nest/trips/trips.mcp.ts | 11 ++--- server/src/nest/trips/trips.module.ts | 3 +- server/src/nest/vacay/vacay.mcp.ts | 11 +++-- server/src/nest/vacay/vacay.module.ts | 3 +- server/tests/helpers/mcp-test-controllers.ts | 31 +++++++++----- server/tests/helpers/test-db.ts | 32 ++++++++++++++ .../tests/unit/mcp/tools-addon-gating.test.ts | 33 +++++++-------- .../unit/mcp/tools-atlas-expanded.test.ts | 7 ++-- .../unit/mcp/tools-collab-polls-chat.test.ts | 7 ++-- server/tests/unit/mcp/tools-journey.test.ts | 10 ++--- server/tests/unit/mcp/tools-prompts.test.ts | 15 +++++-- server/tests/unit/mcp/tools-vacay.test.ts | 10 ++--- 34 files changed, 333 insertions(+), 138 deletions(-) create mode 100644 server/src/nest/addons/addon-gate.ts diff --git a/nest-mcp/README.md b/nest-mcp/README.md index 3f2d47dc0..d8df04a3c 100644 --- a/nest-mcp/README.md +++ b/nest-mcp/README.md @@ -42,7 +42,7 @@ Every decorator accepts `access` and `when`: - **Declarative** — `access: { group: 'things', mode: 'read' | 'write' }`, resolved by the `accessPolicy` given once at `McpModule.forRoot(...)`. `group` is typed as `McpAccessGroup` — plain `string` until the host augments `McpAccessGroupRegistry` (see below), after which only the host's registered groups compile. - **Predicate** — `access: (ctx) => boolean`, bypasses the policy. -- **Availability gate** — `when: (ctx) => boolean`, evaluated *before* `access` (both must pass). Use it for feature/addon toggles so scope markers stay declarative: `when: () => isAddonEnabled(...)`, `access: { group: 'packing', mode: 'read' }`. +- **Availability gate** — `when: (ctx, self) => boolean`, evaluated *before* `access` (both must pass). Use it for feature/addon toggles so scope markers stay declarative: `when: (_ctx, self: PackingMcp) => self.addons.isAddonEnabled(...)`, `access: { group: 'packing', mode: 'read' }`. `self` is the `@McpController()` instance, handed in at attach time — the options object is built when the class is defined, so without it a gate can only reach a module-level singleton, and hosts end up constructing a second copy of a service outside their own container to answer a toggle. - **Omitted `access`** — the entry is always registered (subject to `when`). ### Result helpers & annotation presets @@ -128,7 +128,7 @@ registry.attach(server, ctx); ## Migrating a TREK MCP domain (recipe) 1. Create `server/src/nest//.mcp.ts`: an `@McpController()` class injecting the domain's Nest service. Port each `server.registerTool(...)` from `server/src/mcp/tools/.ts` to a `@Tool()` method **byte-identically** (names, descriptions, schemas, annotations, error strings, `ok()` payloads). -2. Replace the registrar's `canRead/canWrite` registration-time gates with `access: { group, mode }` (TREK's policy in `server/src/mcp/nest-mcp-policy.ts` implements the `scopes.ts` semantics). Addon gates become `when: () => isAddonEnabled(ADDON_IDS.X)` alongside the declarative `access`. +2. Replace the registrar's `canRead/canWrite` registration-time gates with `access: { group, mode }` (TREK's policy in `server/src/mcp/nest-mcp-policy.ts` implements the `scopes.ts` semantics). Addon gates become `when: (_ctx, self: XMcp) => self.addons.isAddonEnabled(ADDON_IDS.X)` alongside the declarative `access`. 3. Add the class to the domain module's `providers: []`. 4. Delete the legacy registrar file and its call in `server/src/mcp/tools.ts`. 5. Add one line constructing the instance in `server/tests/helpers/mcp-test-controllers.ts` and keep the domain's existing unit tests green — behavior must be indistinguishable to a client. diff --git a/nest-mcp/src/registry.ts b/nest-mcp/src/registry.ts index 03a69f7e1..ba8d09271 100644 --- a/nest-mcp/src/registry.ts +++ b/nest-mcp/src/registry.ts @@ -103,7 +103,7 @@ export class McpRegistry { attach(server: McpServer, ctx: McpContext): void { const registrar = server as unknown as LooseRegistrar; for (const { entry, instance } of this.bound) { - if (!this.allowed(entry, ctx)) continue; + if (!this.allowed(entry, ctx, instance)) continue; const handler = (instance as unknown as Record)[entry.methodName]; switch (entry.kind) { case 'tool': @@ -180,8 +180,12 @@ export class McpRegistry { }; } - private allowed(entry: McpEntry, ctx: McpContext): boolean { - if (entry.options.when && !entry.options.when(ctx)) return false; + private allowed(entry: McpEntry, ctx: McpContext, instance: object): boolean { + // The declaring instance goes in so the gate can read an injected + // collaborator. It is resolved here, at attach, rather than captured when + // the options object was built — the class body runs long before the + // container exists. + if (entry.options.when && !entry.options.when(ctx, instance)) return false; const access = entry.options.access; if (access === undefined) return true; if (typeof access === 'function') return access(ctx); diff --git a/nest-mcp/src/types.ts b/nest-mcp/src/types.ts index 30efb7ff7..7ebd2fb6e 100644 --- a/nest-mcp/src/types.ts +++ b/nest-mcp/src/types.ts @@ -81,8 +81,20 @@ interface McpEntryOptionsBase { * "is this feature on at all" checks like addon toggles. Keeps `access` * purely about permissions, so declarative scope markers still compose * with runtime feature gates instead of being replaced by predicates. + * + * `self` is the `@McpController()` instance the entry was declared on, + * handed in at attach time. Without it the gate could only reach a + * module-level singleton, because the options object is built when the class + * is defined and there is no `this` yet — which is how a host ends up + * constructing a second copy of a service outside its own container just to + * answer "is this addon on". With it, the predicate reads an injected + * collaborator and the dependency is visible in the module graph. + * + * Declared with method syntax on purpose: that makes the parameter bivariant, + * so a predicate written against its own controller class stays assignable + * here. The registry always passes the instance that declared the entry. */ - when?: (ctx: McpContext) => boolean; + when?(ctx: McpContext, self: object): boolean; /** Omitted ⇒ the entry is always registered (subject to `when`). */ access?: McpAccess; } diff --git a/nest-mcp/tests/attach.test.ts b/nest-mcp/tests/attach.test.ts index 7209955a7..553e73d0c 100644 --- a/nest-mcp/tests/attach.test.ts +++ b/nest-mcp/tests/attach.test.ts @@ -98,6 +98,17 @@ class WhenMcp { async probe() { return { content: [{ type: 'text', text: 'ok' }] }; } + + // The gate reads the DECLARING INSTANCE rather than a module-level value. + // This is the shape a host needs to answer "is this addon on" from an + // injected service: the options object is built when the class is defined, + // so without `self` the closure has nothing but imports to reach for. + @Tool({ name: 'self_gated_tool', when: (_ctx, self: WhenMcp) => self.featureOn }) + async selfGated() { + return { content: [{ type: 'text', text: 'ok' }] }; + } + + featureOn = false; } const policy: McpAccessPolicy = ({ mode }, ctx) => @@ -191,6 +202,22 @@ describe('McpRegistry.attach', () => { expect(tools).toEqual(['probe_tool', 'when_only_tool', 'when_scoped_tool']); }); + it('hands `when` the instance that declared the entry, resolved at attach', async () => { + const controller = new WhenMcp(); + const registry = createTestRegistry([controller], { accessPolicy: policy }); + + harness = await createAttachHarness(registry, { userId: 7 }); + expect((await harness.client.listTools()).tools.map((t) => t.name)).not.toContain('self_gated_tool'); + await harness.cleanup(); + + // Flipped AFTER registration: a gate that had captured a value when the + // class was defined could not see this, which is exactly why hosts reached + // for a module-level singleton instead of an injected one. + controller.featureOn = true; + harness = await createAttachHarness(registry, { userId: 7 }); + expect((await harness.client.listTools()).tools.map((t) => t.name)).toContain('self_gated_tool'); + }); + it('skips entries whose `when` gate fails, even when access would pass', async () => { const registry = createTestRegistry([new WhenMcp()], { accessPolicy: policy }); harness = await createAttachHarness(registry, { userId: 7, canRead: true, allow: false }); diff --git a/server/src/nest/README.md b/server/src/nest/README.md index f31bba18e..b3025df75 100644 --- a/server/src/nest/README.md +++ b/server/src/nest/README.md @@ -172,6 +172,35 @@ bridge rebuilds its services with `new`, so adding one constructor parameter to a service means hand-editing every bridge that constructs it — five of them in one PR, most recently. `app.get()` costs nothing there and returns the container singleton rather than a second instance. + +**A bridge imported from INSIDE the container hides an edge from the module +graph**, which is a different problem from serving a genuine outside consumer. +Nine `*.mcp.ts` files did that with `addons.bridge` for one boolean each, and +not by preference: a decorator's options object is built while the class is +being defined, so the `when:` closure had no `this` to reach an injected service +through. `@trek/nest-mcp` hands the gate its declaring instance now, and +`addons/addon-gate.ts` turns that into `addonGate(ADDON_IDS.X)` over an injected +`AddonsService`. `reservations.mcp.ts` (assignments) and `atlas.mcp.ts` / +`journey.mcp.ts` (auth) went the same way. + +Four in-container uses are left, each for a reason that injection does not fix: + +- `places.mcp.ts` → `assignments.bridge`: a real cycle, `DaysModule → + PlacesModule → AssignmentsModule → DaysModule`. +- `budget.mcp.ts` / `packing.mcp.ts` / `costs.rpc.ts` → `trips.bridge`: + `TripsModule` imports both, so importing it back closes the loop. +- `auth/user-cleanup.service.ts` → `budget.bridge`: `BudgetModule` imports + `AuthModule`, same shape. +- `files.controller.ts` / `journey.controller.ts` → `files.bridge`: multer's + interceptor options are module-scope literals evaluated before any container + exists. The fix is `MulterModule.registerAsync`, not a different import. + +The lazy `import('../notifications/notifications.bridge')` senders in six +services are resolvable by injection — nothing cycles — but the sweep is its own +change: those services have ~41 hand-wired `new` sites across the suites, and +`tests/` is outside `tsconfig`'s `include`, so a missed argument would not fail +to compile. It would land as `undefined` inside a fire-and-forget send whose +`.catch(() => {})` swallows the TypeError. - `app-config/` — the `@nestjs/config` binding (`AppConfigModule`, global). Never read `process.env` in a module (ESLint enforces this): inject a boot-stable namespace via its `registerAs` token (`@Inject(mcpConfig.KEY) … ConfigType<…>`) diff --git a/server/src/nest/addons/addon-gate.ts b/server/src/nest/addons/addon-gate.ts new file mode 100644 index 000000000..cbac391cf --- /dev/null +++ b/server/src/nest/addons/addon-gate.ts @@ -0,0 +1,40 @@ +import type { McpContext } from '@trek/nest-mcp'; +import type { AddonsService } from './addons.service'; +import { ADDON_IDS } from '../../addons'; + +/** + * The `when:` gates for addon-scoped MCP surfaces. + * + * They used to close over `addons.bridge`, which builds its own AddonsService + * outside the container. That was never a preference — a decorator's options + * object is built while the class is being defined, so the closure had no + * `this` to reach an injected service through, and nine domains hid an edge to + * the addons domain from the module graph to answer one boolean. + * + * `@trek/nest-mcp` now hands the gate the declaring controller instance, so the + * only thing a domain has to do is expose the service it already injects: + * + * constructor(private readonly vacay: VacayService, readonly addons: AddonsService) {} + * const vacayAddonOn = addonGate(ADDON_IDS.VACAY); + * + * `addons` is public rather than private because the gate is a module-level + * function in the same file, not a method — TypeScript's `private` is scoped to + * the class body. + */ +export interface HasAddons { + readonly addons: AddonsService; +} + +/** Gate an MCP entry on an addon being enabled. */ +export function addonGate(addonId: string) { + return (_ctx: McpContext, self: HasAddons): boolean => self.addons.isAddonEnabled(addonId); +} + +/** + * Gate on one of collab's sub-feature flags. Every collab entry rides both the + * addon toggle and its own flag — notes, polls or chat. + */ +export function collabFeatureGate(feature: 'chat' | 'notes' | 'polls') { + return (_ctx: McpContext, self: HasAddons): boolean => + self.addons.isAddonEnabled(ADDON_IDS.COLLAB) && self.addons.getCollabFeatures()[feature]; +} diff --git a/server/src/nest/atlas/atlas.mcp.ts b/server/src/nest/atlas/atlas.mcp.ts index b793b4bc6..2aca20b68 100644 --- a/server/src/nest/atlas/atlas.mcp.ts +++ b/server/src/nest/atlas/atlas.mcp.ts @@ -5,23 +5,16 @@ import { demoDenied, ok, } from '@trek/nest-mcp'; import { z } from 'zod'; -// auth.bridge, not an injected AuthService. The module cycle that used to force -// this is gone (getTravelStats moved to AtlasService, so AuthModule no longer -// imports AtlasModule), but swapping one call site for a direct AuthModule -// import would pull the whole auth chain into every graph that touches Atlas, -// to reach a single isDemoUser. Eight other *.mcp.ts have the same shape with -// addons.bridge; they get resolved together once the nest-mcp registry can -// resolve a `when:` predicate lazily, at registry.attach instead of at module -// load. Same documented trade-off as places.mcp.ts keeping assignments.bridge. -import { isDemoUser } from '../auth/auth.bridge'; -import { isAddonEnabled } from '../addons/addons.bridge'; import { ADDON_IDS } from '../../addons'; import { AtlasService } from './atlas.service'; +import { addonGate } from '../addons/addon-gate'; +import { AddonsService } from '../addons/addons.service'; +import { AuthService } from '../auth/auth.service'; /** Legacy registrar gate: the whole atlas surface (tools AND resources) rides * the atlas addon — unlike the REST controller, which is deliberately ungated * (see atlas.controller.ts). */ -const atlasAddonOn = () => isAddonEnabled(ADDON_IDS.ATLAS); +const atlasAddonOn = addonGate(ADDON_IDS.ATLAS); function jsonContent(uri: string, data: unknown) { return { @@ -48,7 +41,11 @@ function jsonContent(uri: string, data: unknown) { */ @McpController() export class AtlasMcp { - constructor(private readonly atlas: AtlasService) {} + constructor( + private readonly atlas: AtlasService, + readonly addons: AddonsService, + private readonly auth: AuthService, + ) {} // ── Bucket list ───────────────────────────────────────────────────────── @@ -70,7 +67,7 @@ export class AtlasMcp { { name, lat, lng, country_code, notes }: { name: string; lat?: number; lng?: number; country_code?: string; notes?: string }, ctx: McpContext, ) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); const item = this.atlas.createBucketItem(ctx.userId, { name, lat, lng, country_code, notes }); return ok({ item }); } @@ -86,7 +83,7 @@ export class AtlasMcp { access: { group: 'atlas', mode: 'write' }, }) async deleteBucketListItem({ itemId }: { itemId: number }, ctx: McpContext) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); const deleted = this.atlas.deleteBucketItem(ctx.userId, itemId); if (!deleted) return { content: [{ type: 'text' as const, text: 'Bucket list item not found.' }], isError: true }; return ok({ success: true }); @@ -105,7 +102,7 @@ export class AtlasMcp { access: { group: 'atlas', mode: 'write' }, }) async markCountryVisited({ country_code }: { country_code: string }, ctx: McpContext) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); this.atlas.markCountry(ctx.userId, country_code.toUpperCase()); return ok({ success: true, country_code: country_code.toUpperCase() }); } @@ -121,7 +118,7 @@ export class AtlasMcp { access: { group: 'atlas', mode: 'write' }, }) async unmarkCountryVisited({ country_code }: { country_code: string }, ctx: McpContext) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); this.atlas.unmarkCountry(ctx.userId, country_code.toUpperCase()); return ok({ success: true, country_code: country_code.toUpperCase() }); } @@ -170,7 +167,7 @@ export class AtlasMcp { { regionCode, regionName, countryCode }: { regionCode: string; regionName: string; countryCode: string }, ctx: McpContext, ) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); // Post-fold quirk fix: uppercase both codes, matching the REST controller // (the legacy registrar passed them through verbatim, so a lowercase mark // created a row REST's uppercased unmark could never hit). @@ -195,7 +192,7 @@ export class AtlasMcp { access: { group: 'atlas', mode: 'write' }, }) async unmarkRegionVisited({ regionCode }: { regionCode: string }, ctx: McpContext) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); // Post-fold quirk fix: uppercase, matching the REST controller. this.atlas.unmarkRegion(ctx.userId, regionCode.toUpperCase()); return ok({ success: true }); @@ -246,7 +243,7 @@ export class AtlasMcp { }, ctx: McpContext, ) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); const item = this.atlas.updateBucketItem(ctx.userId, itemId, { name, notes, lat, lng, country_code, target_date }); if (!item) return { content: [{ type: 'text' as const, text: 'Bucket list item not found.' }], isError: true }; return ok({ item }); diff --git a/server/src/nest/atlas/atlas.module.ts b/server/src/nest/atlas/atlas.module.ts index a44756d63..785ed6286 100644 --- a/server/src/nest/atlas/atlas.module.ts +++ b/server/src/nest/atlas/atlas.module.ts @@ -5,6 +5,8 @@ import { AtlasService } from './atlas.service'; import { AtlasRpc } from './atlas.rpc'; import { PluginGuardsModule } from '../plugins/host/plugin-guards.module'; import { AtlasMcp } from './atlas.mcp'; +import { AddonsModule } from '../addons/addons.module'; +import { AuthModule } from '../auth/auth.module'; /** * Atlas addon domain (L7 leaf module). Registered in AppModule. Exports @@ -17,7 +19,7 @@ import { AtlasMcp } from './atlas.mcp'; * data. See the comment in that file. */ @Module({ - imports: [PluginGuardsModule], + imports: [AuthModule, PluginGuardsModule, AddonsModule], controllers: [AtlasController, TravelStatsController], providers: [AtlasService, AtlasMcp, AtlasRpc], exports: [AtlasService], diff --git a/server/src/nest/budget/budget.mcp.ts b/server/src/nest/budget/budget.mcp.ts index c6be8b37b..bfd2e6f72 100644 --- a/server/src/nest/budget/budget.mcp.ts +++ b/server/src/nest/budget/budget.mcp.ts @@ -6,16 +6,17 @@ import { } from '@trek/nest-mcp'; import { z } from 'zod'; import { AuthService } from '../auth/auth.service'; -import { isAddonEnabled } from '../addons/addons.bridge'; import { ADDON_IDS } from '../../addons'; import { safeBroadcast, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; import { getTripOwner, listMembers, getTripSummary } from '../trips/trips.bridge'; import { DatabaseService } from '../database/database.service'; import { BudgetService } from './budget.service'; import { ExchangeRatesService } from './exchange-rates.service'; +import { addonGate } from '../addons/addon-gate'; +import { AddonsService } from '../addons/addons.service'; /** Legacy registrar gate: the whole budget surface rides the budget addon. */ -const budgetAddonOn = () => isAddonEnabled(ADDON_IDS.BUDGET); +const budgetAddonOn = addonGate(ADDON_IDS.BUDGET); /** Reusable Zod shape for the per-payer amounts on a budget item. */ const payersSchema = z.array(z.object({ @@ -69,6 +70,7 @@ export class BudgetMcp { private readonly exchangeRates: ExchangeRatesService, private readonly db: DatabaseService, private readonly auth: AuthService, + readonly addons: AddonsService, ) {} // --- BUDGET --- diff --git a/server/src/nest/budget/budget.module.ts b/server/src/nest/budget/budget.module.ts index f330be0e1..b39388024 100644 --- a/server/src/nest/budget/budget.module.ts +++ b/server/src/nest/budget/budget.module.ts @@ -9,11 +9,12 @@ 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 { AddonsModule } from '../addons/addons.module'; /** Budget domain (S4 — Phase 2 trip sub-domain). Registered in AppModule. * BudgetMcp carries the decorator-registered MCP tools + resources. */ @Module({ - imports: [PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule], + imports: [PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, AddonsModule], controllers: [BudgetController], providers: [BudgetService, ExchangeRatesService, BudgetMcp, ExchangeRatesRpc, CostsRpc], // For in-container consumers (CostsRpc, TripsService, diff --git a/server/src/nest/collab/collab.mcp.ts b/server/src/nest/collab/collab.mcp.ts index 716237745..59dce4852 100644 --- a/server/src/nest/collab/collab.mcp.ts +++ b/server/src/nest/collab/collab.mcp.ts @@ -6,19 +6,20 @@ import { } from '@trek/nest-mcp'; import { z } from 'zod'; import { AuthService } from '../auth/auth.service'; -import { isAddonEnabled, getCollabFeatures } from '../addons/addons.bridge'; import { ADDON_IDS } from '../../addons'; import { safeBroadcast, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; import { CollabService } from './collab.service'; +import { collabFeatureGate } from '../addons/addon-gate'; +import { AddonsService } from '../addons/addons.service'; /** * Legacy registrar gates: the whole collab surface rides the collab addon * (whole-registrar early return), and each tool/resource additionally rides * its sub-feature flag from getCollabFeatures() — notes, polls or chat. */ -const collabNotesOn = () => isAddonEnabled(ADDON_IDS.COLLAB) && getCollabFeatures().notes; -const collabPollsOn = () => isAddonEnabled(ADDON_IDS.COLLAB) && getCollabFeatures().polls; -const collabChatOn = () => isAddonEnabled(ADDON_IDS.COLLAB) && getCollabFeatures().chat; +const collabNotesOn = collabFeatureGate('notes'); +const collabPollsOn = collabFeatureGate('polls'); +const collabChatOn = collabFeatureGate('chat'); function parseId(value: string | string[]): number | null { const n = Number(Array.isArray(value) ? value[0] : value); @@ -59,7 +60,11 @@ function jsonContent(uri: string, data: unknown) { */ @McpController() export class CollabMcp { - constructor(private readonly collab: CollabService, private readonly auth: AuthService) {} + constructor( + private readonly collab: CollabService, + private readonly auth: AuthService, + readonly addons: AddonsService, + ) {} // --- COLLAB NOTES --- diff --git a/server/src/nest/collab/collab.module.ts b/server/src/nest/collab/collab.module.ts index 7420f8e42..89a0057d4 100644 --- a/server/src/nest/collab/collab.module.ts +++ b/server/src/nest/collab/collab.module.ts @@ -7,9 +7,10 @@ import { RealtimeModule } from '../realtime/realtime.module'; import { CollabMcp } from './collab.mcp'; import { PermissionsModule } from '../permissions/permissions.module'; import { AuthModule } from '../auth/auth.module'; +import { AddonsModule } from '../addons/addons.module'; @Module({ - imports: [PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule], + imports: [PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, AddonsModule], controllers: [CollabController], providers: [CollabService, CollabMcp, CollabRpc], // For in-container consumers (CollabRpc). diff --git a/server/src/nest/collections/collections.mcp.ts b/server/src/nest/collections/collections.mcp.ts index 0883974eb..573bc3cad 100644 --- a/server/src/nest/collections/collections.mcp.ts +++ b/server/src/nest/collections/collections.mcp.ts @@ -6,7 +6,6 @@ import { } from '@trek/nest-mcp'; import { z } from 'zod'; import { AuthService } from '../auth/auth.service'; -import { isAddonEnabled } from '../addons/addons.bridge'; import { ADDON_IDS } from '../../addons'; import { DatabaseService } from '../database/database.service'; import { CollectionsService } from './collections.service'; @@ -23,6 +22,8 @@ import type { CollectionLabelCreateRequest, CollectionLabelUpdateRequest, CollectionLabelAssignRequest, CollectionStatus, CollectionRole, } from '@trek/shared'; +import { addonGate } from '../addons/addon-gate'; +import { AddonsService } from '../addons/addons.service'; /** Convert a thrown service error (httpError carries `.message`) into MCP error text. */ function errText(err: unknown): string { @@ -37,7 +38,7 @@ function fail(err: unknown) { /** Post-fold quirk fix: the legacy registrar registered unconditionally while * REST (CollectionsAddonGuard) and the plugin host (requireAddon) gate on the * collections addon — the decorator port now gates too. */ -const collectionsAddonOn = () => isAddonEnabled(ADDON_IDS.COLLECTIONS); +const collectionsAddonOn = addonGate(ADDON_IDS.COLLECTIONS); /** * Collections MCP surface (#1435) — ported 1:1 from the legacy registrar @@ -57,6 +58,7 @@ export class CollectionsMcp { private readonly collections: CollectionsService, private readonly db: DatabaseService, private readonly auth: AuthService, + readonly addons: AddonsService, ) {} private denyDemo(userId: number) { diff --git a/server/src/nest/journey/journey.mcp.ts b/server/src/nest/journey/journey.mcp.ts index e8df81845..efbd657ea 100644 --- a/server/src/nest/journey/journey.mcp.ts +++ b/server/src/nest/journey/journey.mcp.ts @@ -5,18 +5,16 @@ import { demoDenied, ok, type McpContext, } from '@trek/nest-mcp'; import { z } from 'zod'; -// auth.bridge, not an injected AuthService: same documented in-container -// exception as atlas.mcp.ts and places.mcp.ts — AuthModule's graph is large and -// pulling it into JourneyModule for one isDemoUser call is not worth it. -import { isDemoUser } from '../auth/auth.bridge'; -import { isAddonEnabled } from '../addons/addons.bridge'; import { ADDON_IDS } from '../../addons'; import { JourneyDomainService } from './journey-domain.service'; import { JourneyShareService } from './journey-share.service'; import type { JourneyContributor } from '../../types'; +import { addonGate } from '../addons/addon-gate'; +import { AddonsService } from '../addons/addons.service'; +import { AuthService } from '../auth/auth.service'; /** Legacy registrar gate: the whole journey surface rode the journey addon. */ -const journeyAddonOn = () => isAddonEnabled(ADDON_IDS.JOURNEY); +const journeyAddonOn = addonGate(ADDON_IDS.JOURNEY); function notFound(msg: string) { return { content: [{ type: 'text' as const, text: msg }], isError: true }; @@ -38,6 +36,8 @@ export class JourneyMcp { constructor( private readonly journey: JourneyDomainService, private readonly share: JourneyShareService, + readonly addons: AddonsService, + private readonly auth: AuthService, ) {} // ── Read ──────────────────────────────────────────────────────────────── @@ -137,7 +137,7 @@ export class JourneyMcp { { title, subtitle, trip_ids }: { title: string; subtitle?: string; trip_ids?: number[] }, ctx: McpContext, ) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); const journey = this.journey.createJourney(ctx.userId, { title, subtitle, trip_ids }); // Return the fully-hydrated journey (entries/contributors/trips/stats/my_role), // matching get_journey, rather than the bare row. @@ -161,7 +161,7 @@ export class JourneyMcp { { journeyId, title, subtitle, status }: { journeyId: number; title?: string; subtitle?: string; status?: string }, ctx: McpContext, ) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); const journey = this.journey.updateJourney(journeyId, ctx.userId, { title, subtitle, status }); if (!journey) return notFound('Journey not found or access denied.'); return ok({ journey }); @@ -176,7 +176,7 @@ export class JourneyMcp { access: { group: 'journey', mode: 'write' }, }) deleteJourney({ journeyId }: { journeyId: number }, ctx: McpContext) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.journey.deleteJourney(journeyId, ctx.userId)) return notFound('Journey not found or access denied.'); return ok({ success: true }); } @@ -190,7 +190,7 @@ export class JourneyMcp { access: { group: 'journey', mode: 'write' }, }) addJourneyTrip({ journeyId, tripId }: { journeyId: number; tripId: number }, ctx: McpContext) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.journey.canAccessJourney(journeyId, ctx.userId)) return notFound('Journey not found or access denied.'); return ok({ success: this.journey.addTripToJourney(journeyId, tripId, ctx.userId) }); } @@ -204,7 +204,7 @@ export class JourneyMcp { access: { group: 'journey', mode: 'write' }, }) removeJourneyTrip({ journeyId, tripId }: { journeyId: number; tripId: number }, ctx: McpContext) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); const success = this.journey.removeTripFromJourney(journeyId, tripId, ctx.userId); if (!success) return notFound('Journey not found or access denied.'); return ok({ success }); @@ -231,7 +231,7 @@ export class JourneyMcp { { journeyId, ...data }: { journeyId: number; entry_date: string; title?: string; story?: string; entry_time?: string; location_name?: string; mood?: string; sort_order?: number }, ctx: McpContext, ) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); const entry = this.journey.createEntry(journeyId, ctx.userId, data); if (!entry) return notFound('Journey not found or access denied.'); // Return through the listEntries enrichment (parsed tags/pros_cons, photos, source_trip_name). @@ -258,7 +258,7 @@ export class JourneyMcp { { entryId, ...data }: { entryId: number; title?: string; story?: string; entry_date?: string; entry_time?: string; mood?: string }, ctx: McpContext, ) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); const entry = this.journey.updateEntry(entryId, ctx.userId, data, undefined); if (!entry) return notFound('Entry not found or access denied.'); // Return through the listEntries enrichment (parsed tags/pros_cons, photos), matching create_journey_entry. @@ -275,7 +275,7 @@ export class JourneyMcp { access: { group: 'journey', mode: 'write' }, }) deleteJourneyEntry({ entryId }: { entryId: number }, ctx: McpContext) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.journey.deleteEntry(entryId, ctx.userId, undefined)) return notFound('Entry not found or access denied.'); return ok({ success: true }); } @@ -292,7 +292,7 @@ export class JourneyMcp { access: { group: 'journey', mode: 'write' }, }) reorderJourneyEntries({ journeyId, orderedIds }: { journeyId: number; orderedIds: number[] }, ctx: McpContext) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); const success = this.journey.reorderEntries(journeyId, ctx.userId, orderedIds, undefined); if (!success) return notFound('Journey not found, access denied, or entry IDs do not belong to this journey.'); return ok({ success: true }); @@ -314,7 +314,7 @@ export class JourneyMcp { { journeyId, targetUserId, role }: { journeyId: number; targetUserId: number; role: 'editor' | 'viewer' }, ctx: McpContext, ) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.journey.addContributor(journeyId, ctx.userId, targetUserId, role)) return notFound('Journey not found or access denied.'); return ok({ success: true }); } @@ -335,7 +335,7 @@ export class JourneyMcp { { journeyId, targetUserId, role }: { journeyId: number; targetUserId: number; role: 'editor' | 'viewer' }, ctx: McpContext, ) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.journey.updateContributorRole(journeyId, ctx.userId, targetUserId, role)) return notFound('Journey not found or access denied.'); return ok({ success: true }); } @@ -352,7 +352,7 @@ export class JourneyMcp { access: { group: 'journey', mode: 'write' }, }) removeJourneyContributor({ journeyId, targetUserId }: { journeyId: number; targetUserId: number }, ctx: McpContext) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.journey.removeContributor(journeyId, ctx.userId, targetUserId)) return notFound('Journey not found or access denied.'); return ok({ success: true }); } @@ -372,7 +372,7 @@ export class JourneyMcp { { journeyId, hide_skeletons }: { journeyId: number; hide_skeletons?: boolean }, ctx: McpContext, ) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); const result = this.journey.updateJourneyPreferences(journeyId, ctx.userId, { hide_skeletons }); if (!result) return notFound('Journey not found or access denied.'); // Return the service result ({ hide_skeletons }), matching the REST route. @@ -403,7 +403,7 @@ export class JourneyMcp { access: { group: 'journey', mode: 'share' }, }) createJourneyShareLink({ journeyId }: { journeyId: number }, ctx: McpContext) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); const shareLink = this.share.createOrUpdateJourneyShareLink(journeyId, ctx.userId, {}); if (!shareLink) return notFound('Journey not found or access denied.'); return ok({ shareLink }); @@ -418,7 +418,7 @@ export class JourneyMcp { access: { group: 'journey', mode: 'share' }, }) deleteJourneyShareLink({ journeyId }: { journeyId: number }, ctx: McpContext) { - if (isDemoUser(ctx.userId)) return demoDenied(); + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); if (!this.share.deleteJourneyShareLink(journeyId, ctx.userId)) return notFound('Journey not found or access denied.'); return ok({ success: true }); } diff --git a/server/src/nest/journey/journey.module.ts b/server/src/nest/journey/journey.module.ts index f4c5c358e..da1b12ce4 100644 --- a/server/src/nest/journey/journey.module.ts +++ b/server/src/nest/journey/journey.module.ts @@ -6,10 +6,11 @@ import { AddonsModule } from '../addons/addons.module'; import { MemoriesModule } from '../memories/memories.module'; import { JourneyDomainModule } from './journey-domain.module'; import { JourneyMcp } from './journey.mcp'; +import { AuthModule } from '../auth/auth.module'; @Module({ // MemoriesModule: the journey gallery streams provider assets and uploads to Immich. - imports: [AddonsModule, MemoriesModule, JourneyDomainModule], + imports: [AuthModule, AddonsModule, MemoriesModule, JourneyDomainModule], controllers: [JourneyController, JourneyPublicController], providers: [JourneyService, JourneyMcp], }) diff --git a/server/src/nest/packing/packing.mcp.ts b/server/src/nest/packing/packing.mcp.ts index 120d41a83..c49bbe197 100644 --- a/server/src/nest/packing/packing.mcp.ts +++ b/server/src/nest/packing/packing.mcp.ts @@ -6,7 +6,6 @@ import { } from '@trek/nest-mcp'; import { z } from 'zod'; import { AuthService } from '../auth/auth.service'; -import { isAddonEnabled } from '../addons/addons.bridge'; import { ADDON_IDS } from '../../addons'; import { safeBroadcast, noAccess, hasTripPermission, permissionDenied, isAdminUser, adminRequired } from '../../mcp/tools/_shared'; import { PackingService } from './packing.service'; @@ -14,9 +13,11 @@ import { PackingService } from './packing.service'; // so injecting it here would close a module cycle. Same documented in-container // exception as atlas.mcp.ts, and the legacy registrar reached for it the same way. import { getTripSummary } from '../trips/trips.bridge'; +import { addonGate } from '../addons/addon-gate'; +import { AddonsService } from '../addons/addons.service'; /** Legacy registrar gate: the whole packing surface rides the packing addon. */ -const packingAddonOn = () => isAddonEnabled(ADDON_IDS.PACKING); +const packingAddonOn = addonGate(ADDON_IDS.PACKING); function parseId(value: string | string[]): number | null { const n = Number(Array.isArray(value) ? value[0] : value); @@ -37,7 +38,11 @@ function parseId(value: string | string[]): number | null { */ @McpController() export class PackingMcp { - constructor(private readonly packing: PackingService, private readonly auth: AuthService) {} + constructor( + private readonly packing: PackingService, + private readonly auth: AuthService, + readonly addons: AddonsService, + ) {} // --- PACKING --- diff --git a/server/src/nest/packing/packing.module.ts b/server/src/nest/packing/packing.module.ts index 628d99203..14d7bf533 100644 --- a/server/src/nest/packing/packing.module.ts +++ b/server/src/nest/packing/packing.module.ts @@ -9,12 +9,13 @@ import { RealtimeModule } from '../realtime/realtime.module'; import { PermissionsModule } from '../permissions/permissions.module'; import { AuditModule } from '../audit/audit.module'; import { AuthModule } from '../auth/auth.module'; +import { AddonsModule } from '../addons/addons.module'; /** Packing domain (S2 — Phase 2 trip sub-domain). Registered in AppModule. * Exports PackingService for in-container consumers (TripsService bundle, * PackingRpc). */ @Module({ - imports: [PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, AuditModule], + imports: [PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, AddonsModule, AuditModule], controllers: [PackingController, AdminPackingTemplatesController], providers: [PackingService, PackingMcp, PackingRpc], exports: [PackingService], diff --git a/server/src/nest/places/places.mcp.ts b/server/src/nest/places/places.mcp.ts index 2525f619d..f9b9da53b 100644 --- a/server/src/nest/places/places.mcp.ts +++ b/server/src/nest/places/places.mcp.ts @@ -30,11 +30,12 @@ function parseId(value: string | string[]): number | null { * 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: AssignmentsModule imports DaysModule, and DaysModule now - * imports PlacesModule for days.mcp.ts's place creation, so injecting here - * would close a DaysModule → PlacesModule → AssignmentsModule → DaysModule - * cycle. Same reason reservations.mcp.ts uses that bridge from inside the - * container, and the same trade trips.bridge.ts documents. + * 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. */ @McpController() export class PlacesMcp { diff --git a/server/src/nest/reservations/reservations.mcp.ts b/server/src/nest/reservations/reservations.mcp.ts index 0b1ec8462..5328ba59e 100644 --- a/server/src/nest/reservations/reservations.mcp.ts +++ b/server/src/nest/reservations/reservations.mcp.ts @@ -6,12 +6,12 @@ import { import { z } from 'zod'; import { AuthService } from '../auth/auth.service'; import { BudgetService } from '../budget/budget.service'; -import { placeExists, getAssignmentForTrip } from '../assignments/assignments.bridge'; import { safeBroadcast, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; import { ReservationsService } from './reservations.service'; import { DaysService } from '../days/days.service'; import { findByIata } from '../airports/airports.data'; import type { EndpointInput } from './reservations.service'; +import { AssignmentsService } from '../assignments/assignments.service'; const TRANSPORT_TYPES = ['flight', 'train', 'car', 'cruise'] as const; @@ -88,6 +88,9 @@ export class ReservationsMcp { private readonly days: DaysService, private readonly budget: BudgetService, private readonly auth: AuthService, + // Appended, not inserted: the hand-wired MCP harnesses build this + // positionally. + private readonly assignments: AssignmentsService, ) {} @Tool({ @@ -130,13 +133,13 @@ export class ReservationsMcp { // Validate that all referenced IDs belong to this trip if (day_id && !this.days.getDay(day_id, tripId)) return errorResult('day_id does not belong to this trip.'); - if (place_id && !placeExists(place_id, tripId)) + if (place_id && !this.assignments.placeExists(place_id, tripId)) return errorResult('place_id does not belong to this trip.'); if (start_day_id && !this.days.getDay(start_day_id, tripId)) return errorResult('start_day_id does not belong to this trip.'); if (end_day_id && !this.days.getDay(end_day_id, tripId)) return errorResult('end_day_id does not belong to this trip.'); - if (assignment_id && !getAssignmentForTrip(assignment_id, tripId)) + if (assignment_id && !this.assignments.getAssignmentForTrip(assignment_id, tripId)) return errorResult('assignment_id does not belong to this trip.'); const createAccommodation = (type === 'hotel' && place_id && start_day_id && end_day_id) @@ -203,9 +206,9 @@ export class ReservationsMcp { const existing = this.reservations.getReservation(reservationId, tripId); if (!existing) return errorResult('Reservation not found.'); - if (place_id != null && !placeExists(place_id, tripId)) + if (place_id != null && !this.assignments.placeExists(place_id, tripId)) return errorResult('place_id does not belong to this trip.'); - if (assignment_id != null && !getAssignmentForTrip(assignment_id, tripId)) + if (assignment_id != null && !this.assignments.getAssignmentForTrip(assignment_id, tripId)) return errorResult('assignment_id does not belong to this trip.'); const { reservation } = this.reservations.update(reservationId, tripId, { @@ -295,7 +298,7 @@ export class ReservationsMcp { if (!current) return errorResult('Reservation not found.'); if (current.type !== 'hotel') return errorResult('Reservation is not of type hotel.'); - if (!placeExists(place_id, tripId)) + if (!this.assignments.placeExists(place_id, tripId)) return errorResult('place_id does not belong to this trip.'); if (!this.days.getDay(start_day_id, tripId)) return errorResult('start_day_id does not belong to this trip.'); diff --git a/server/src/nest/reservations/reservations.module.ts b/server/src/nest/reservations/reservations.module.ts index 251a7845e..a05dd4765 100644 --- a/server/src/nest/reservations/reservations.module.ts +++ b/server/src/nest/reservations/reservations.module.ts @@ -10,6 +10,7 @@ import { RealtimeModule } from '../realtime/realtime.module'; import { ReservationsMcp } from './reservations.mcp'; import { UpcomingReservationsController } from './upcoming-reservations.controller'; import { AuthModule } from '../auth/auth.module'; +import { AssignmentsModule } from '../assignments/assignments.module'; /** * Reservations + accommodations domain (S5 — Phase 2 trip sub-domain). @@ -20,7 +21,7 @@ import { AuthModule } from '../auth/auth.module'; @Module({ // DaysModule: ReservationsMcp injects DaysService for its nine getDay calls. // BudgetModule: ReservationsService + ReservationsMcp inject BudgetService (budget-sync seam). - imports: [DaysModule, PermissionsModule, BudgetModule, AuthModule, RealtimeModule, PluginGuardsModule], + imports: [DaysModule, AssignmentsModule, PermissionsModule, BudgetModule, AuthModule, RealtimeModule, PluginGuardsModule], controllers: [ReservationsController, UpcomingReservationsController], providers: [ReservationsService, ReservationsMcp, ReservationsRpc], // For in-container consumers (ReservationsRpc, TripsService, BookingImportService). diff --git a/server/src/nest/todo/todo.mcp.ts b/server/src/nest/todo/todo.mcp.ts index 66feac2ea..a98e66f02 100644 --- a/server/src/nest/todo/todo.mcp.ts +++ b/server/src/nest/todo/todo.mcp.ts @@ -6,13 +6,14 @@ import { } from '@trek/nest-mcp'; import { z } from 'zod'; import { AuthService } from '../auth/auth.service'; -import { isAddonEnabled } from '../addons/addons.bridge'; import { ADDON_IDS } from '../../addons'; import { safeBroadcast, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; import { TodoService } from './todo.service'; +import { addonGate } from '../addons/addon-gate'; +import { AddonsService } from '../addons/addons.service'; /** Legacy registrar gate: the whole todo surface rides the packing addon. */ -const packingAddonOn = () => isAddonEnabled(ADDON_IDS.PACKING); +const packingAddonOn = addonGate(ADDON_IDS.PACKING); function parseId(value: string | string[]): number | null { const n = Number(Array.isArray(value) ? value[0] : value); @@ -30,7 +31,11 @@ function parseId(value: string | string[]): number | null { */ @McpController() export class TodoMcp { - constructor(private readonly todos: TodoService, private readonly auth: AuthService) {} + constructor( + private readonly todos: TodoService, + private readonly auth: AuthService, + readonly addons: AddonsService, + ) {} @Tool({ name: 'list_todos', diff --git a/server/src/nest/todo/todo.module.ts b/server/src/nest/todo/todo.module.ts index df839db36..00f2b4c90 100644 --- a/server/src/nest/todo/todo.module.ts +++ b/server/src/nest/todo/todo.module.ts @@ -7,11 +7,12 @@ 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 { AddonsModule } from '../addons/addons.module'; /** To-do domain (S3 — Phase 2 trip sub-domain). Registered in AppModule. * Exports TodoService for in-container consumers (TripsService bundle). */ @Module({ - imports: [PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule], + imports: [PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, AddonsModule], controllers: [TodoController], providers: [TodoService, TodoMcp, TodoRpc], exports: [TodoService], diff --git a/server/src/nest/trips/trips.mcp.ts b/server/src/nest/trips/trips.mcp.ts index 052b75465..cac913881 100644 --- a/server/src/nest/trips/trips.mcp.ts +++ b/server/src/nest/trips/trips.mcp.ts @@ -9,13 +9,13 @@ import { AuthService } from '../auth/auth.service'; import { CalendarService } from '../calendar/calendar.service'; import { TripMembersService } from '../trip-members/trip-members.service'; import { TripReadModelService } from '../trip-read-model/trip-read-model.service'; -import { isAddonEnabled, getCollabFeatures } from '../addons/addons.bridge'; import { ADDON_IDS } from '../../addons'; import { safeBroadcast, MAX_MCP_TRIP_DAYS, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; import { canRead, canReadTrips, canDeleteTrips } from '../../mcp/scopes'; import { TripsService, NotFoundError, ValidationError } from './trips.service'; import { TodoService } from '../todo/todo.service'; import { CollabService } from '../collab/collab.service'; +import { AddonsService } from '../addons/addons.service'; function parseId(value: string | string[]): number | null { const n = Number(Array.isArray(value) ? value[0] : value); @@ -71,6 +71,7 @@ export class TripsMcp { private readonly calendar: CalendarService, private readonly members: TripMembersService, private readonly readModel: TripReadModelService, + private readonly addons: AddonsService, ) {} // --- TRIPS --- @@ -212,10 +213,10 @@ export class TripsMcp { if (!summary) return noAccess(); const R = canReadTrips(ctx.scopes); // Addon availability gates - const packingEnabled = isAddonEnabled(ADDON_IDS.PACKING); - const budgetEnabled = isAddonEnabled(ADDON_IDS.BUDGET); - const collabEnabled = isAddonEnabled(ADDON_IDS.COLLAB); - const collabFeatures = collabEnabled ? getCollabFeatures() : null; + const packingEnabled = this.addons.isAddonEnabled(ADDON_IDS.PACKING); + const budgetEnabled = this.addons.isAddonEnabled(ADDON_IDS.BUDGET); + const collabEnabled = this.addons.isAddonEnabled(ADDON_IDS.COLLAB); + const collabFeatures = collabEnabled ? this.addons.getCollabFeatures() : null; // Scope gates — sections not covered by the client's OAuth scopes are omitted. // Core trip data (metadata, days, members, accommodations) is always included // because this tool is always registered and needed for navigation. diff --git a/server/src/nest/trips/trips.module.ts b/server/src/nest/trips/trips.module.ts index bd0403e6b..fffd8dcc5 100644 --- a/server/src/nest/trips/trips.module.ts +++ b/server/src/nest/trips/trips.module.ts @@ -24,11 +24,12 @@ import { CalendarModule } from '../calendar/calendar.module'; import { AccommodationsModule } from '../accommodations/accommodations.module'; import { TripMembersModule } from '../trip-members/trip-members.module'; import { TripReadModelModule } from '../trip-read-model/trip-read-model.module'; +import { AddonsModule } from '../addons/addons.module'; /** Trips aggregate root (C1 — Phase 3). Uses exact strangler prefixes so it does * not capture the nested sub-domain mounts (collab, files, ...). */ @Module({ - imports: [TodoModule, PackingModule, FilesModule, ReservationsModule, DaysModule, PermissionsModule, AuditModule, BudgetModule, CollabModule, VacayModule, PlacesModule, AuthModule, AppConfigModule, UnsplashModule, RealtimeModule, PluginGuardsModule, TripMembershipModule, CalendarModule, AccommodationsModule, TripMembersModule, TripReadModelModule], + imports: [TodoModule, PackingModule, FilesModule, ReservationsModule, DaysModule, PermissionsModule, AuditModule, BudgetModule, CollabModule, VacayModule, PlacesModule, AuthModule, AppConfigModule, UnsplashModule, RealtimeModule, PluginGuardsModule, AddonsModule, TripMembershipModule, CalendarModule, AccommodationsModule, TripMembersModule, TripReadModelModule], controllers: [TripsController], providers: [TripsService, TripsMcp, TripsRpc], // Exported for FeedsModule (ICS feeds) and PluginsModule (RPC host injection). diff --git a/server/src/nest/vacay/vacay.mcp.ts b/server/src/nest/vacay/vacay.mcp.ts index d2c440766..a6c81b558 100644 --- a/server/src/nest/vacay/vacay.mcp.ts +++ b/server/src/nest/vacay/vacay.mcp.ts @@ -7,12 +7,13 @@ import { } from '@trek/nest-mcp'; import { z } from 'zod'; import { AuthService } from '../auth/auth.service'; -import { isAddonEnabled } from '../addons/addons.bridge'; import { ADDON_IDS } from '../../addons'; import { VacayService } from './vacay.service'; +import { addonGate } from '../addons/addon-gate'; +import { AddonsService } from '../addons/addons.service'; /** Legacy registrar gate: the whole vacay surface rides the vacay addon. */ -const vacayAddonOn = () => isAddonEnabled(ADDON_IDS.VACAY); +const vacayAddonOn = addonGate(ADDON_IDS.VACAY); /** * Vacay MCP surface — ported 1:1 from the legacy registrars: the 26 tools from @@ -30,7 +31,11 @@ const vacayAddonOn = () => isAddonEnabled(ADDON_IDS.VACAY); */ @McpController() export class VacayMcp { - constructor(private readonly vacay: VacayService, private readonly auth: AuthService) {} + constructor( + private readonly vacay: VacayService, + private readonly auth: AuthService, + readonly addons: AddonsService, + ) {} @Tool({ name: 'get_vacay_plan', diff --git a/server/src/nest/vacay/vacay.module.ts b/server/src/nest/vacay/vacay.module.ts index 2274a71c2..a356f64ef 100644 --- a/server/src/nest/vacay/vacay.module.ts +++ b/server/src/nest/vacay/vacay.module.ts @@ -5,6 +5,7 @@ import { VacayRpc } from './vacay.rpc'; import { PluginGuardsModule } from '../plugins/host/plugin-guards.module'; import { VacayMcp } from './vacay.mcp'; import { AuthModule } from '../auth/auth.module'; +import { AddonsModule } from '../addons/addons.module'; /** * Vacay addon domain (S1 — Phase 2 trip sub-domain). Registered in AppModule. @@ -12,7 +13,7 @@ import { AuthModule } from '../auth/auth.module'; * VacayMcp carries the DI-discovered MCP tools/resources. */ @Module({ - imports: [AuthModule, PluginGuardsModule], + imports: [AuthModule, PluginGuardsModule, AddonsModule], controllers: [VacayController], providers: [VacayService, VacayMcp, VacayRpc], exports: [VacayService], diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index 1f6b71caf..9ba22ff0f 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -65,6 +65,7 @@ import { TripReadModelService } from '../../src/nest/trip-read-model/trip-read-m import { PlacePhotoCacheService } from '../../src/nest/place-photos/place-photo-cache.service'; import { RuntimeEnvService } from '../../src/nest/app-config/runtime-env.service'; import { makeNotificationsService, makeNotificationPreferencesService } from './notifications'; +import { AddonsService } from '../../src/nest/addons/addons.service'; /** * Hand-wired counterpart of the boot-time discovery in McpRegistryService, @@ -116,6 +117,14 @@ export function createMcpTestRegistry(): McpRegistry { new FilesService(dbService, permissionsService, realtimeService), ); const calendarService = new CalendarService(dbService, reservationsService); + // The nine addon-gated surfaces read their toggle off an injected service now + // rather than off addons.bridge's own instance, so the harness has to supply + // 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. + const assignmentsService = new AssignmentsService(dbService, permissionsService, realtimeService, queryHelpersService, journeyDomain); return createTestRegistry( [ new TagsMcp(new TagsService(dbService), authService), @@ -124,24 +133,24 @@ export function createMcpTestRegistry(): McpRegistry { new WeatherMcp(new WeatherService()), new AirportsMcp(), new AuthMcp(), - new TodoMcp(todoService, authService), - new PackingMcp(packingService, authService), - new BudgetMcp(budgetService, exchangeRatesService, dbService, authService), - new ReservationsMcp(reservationsService, daysService, budgetService, authService), + new TodoMcp(todoService, authService, addonsService), + new PackingMcp(packingService, authService, addonsService), + new BudgetMcp(budgetService, exchangeRatesService, dbService, authService, addonsService), + new ReservationsMcp(reservationsService, daysService, budgetService, authService, assignmentsService), new DayNotesMcp(new DayNotesService(dbService, permissionsService, realtimeService), authService), new DaysMcp(daysService, authService), new AccommodationsMcp(accommodationsService, dbService, placesService, authService), - new AssignmentsMcp(new AssignmentsService(dbService, permissionsService, realtimeService, queryHelpersService, journeyDomain), daysService, authService), - new CollabMcp(collabService, authService), - new VacayMcp(new VacayService(dbService, realtimeService), authService), - new TripsMcp(tripsService, todoService, collabService, authService, calendarService, membersService, readModelService), + new AssignmentsMcp(assignmentsService, daysService, authService), + new CollabMcp(collabService, authService, addonsService), + new VacayMcp(new VacayService(dbService, realtimeService), authService, addonsService), + new TripsMcp(tripsService, todoService, collabService, authService, calendarService, membersService, readModelService, addonsService), new ShareMcp(new ShareService(dbService, new SettingsService(dbService), permissionsService, queryHelpersService), authService), new MapsMcp(mapsService), new PlacesMcp(placesService, mapsService, dbService, authService, journeyDomain), - new CollectionsMcp(new CollectionsService(dbService, permissionsService, realtimeService), dbService, authService), + new CollectionsMcp(new CollectionsService(dbService, permissionsService, realtimeService), dbService, authService, addonsService), new TransitMcp(new TransitService(), daysService, reservationsService, dbService, authService), - new AtlasMcp(new AtlasService(dbService)), - new JourneyMcp(journeyDomain, new JourneyShareService(dbService, journeyDomain)), + new AtlasMcp(new AtlasService(dbService), addonsService, authService), + new JourneyMcp(journeyDomain, new JourneyShareService(dbService, journeyDomain), addonsService, authService), new NotificationsMcp(makeNotificationsService(dbService, realtimeService), authService), ], { accessPolicy: trekMcpAccessPolicy, validateAccess: trekMcpValidateAccess }, diff --git a/server/tests/helpers/test-db.ts b/server/tests/helpers/test-db.ts index 8262f8029..171a2b779 100644 --- a/server/tests/helpers/test-db.ts +++ b/server/tests/helpers/test-db.ts @@ -138,6 +138,38 @@ const DEFAULT_PHOTO_PROVIDERS = [ { id: 'synologyphotos', name: 'Synology Photos', enabled: 1 }, ]; +/** + * Flip an addon in the test DB. + * + * The MCP `when:` gates used to be mocked at the module boundary + * (`vi.mock('addons.bridge')`), which worked only because the gate closed over + * a module-level singleton. They read their controller's injected AddonsService + * now, so a test toggles the same row the admin panel writes — which also means + * these cases exercise the real read instead of a stub of it. + * + * `resetTestDb` deliberately leaves the addons table alone, so a toggle + * survives into the next case: set what a case needs rather than assuming the + * seeded default. + */ +export function setAddonEnabled(db: Database.Database, addonId: string, enabled: boolean): void { + db.prepare( + 'INSERT INTO addons (id, name, type, enabled) VALUES (?, ?, ?, ?) ' + + 'ON CONFLICT(id) DO UPDATE SET enabled = excluded.enabled', + ).run(addonId, addonId, 'global', enabled ? 1 : 0); +} + +/** Collab's sub-feature flags are opt-out app_settings, not addon rows. */ +export function setCollabFeature( + db: Database.Database, + feature: 'chat' | 'notes' | 'polls' | 'whatsnext', + enabled: boolean, +): void { + db.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)").run( + `collab_${feature}_enabled`, + enabled ? 'true' : 'false', + ); +} + function seedDefaults(db: Database.Database): void { const insertCat = db.prepare('INSERT OR IGNORE INTO categories (name, color, icon) VALUES (?, ?, ?)'); for (const cat of DEFAULT_CATEGORIES) insertCat.run(cat.name, cat.color, cat.icon); diff --git a/server/tests/unit/mcp/tools-addon-gating.test.ts b/server/tests/unit/mcp/tools-addon-gating.test.ts index a41c27b9d..a87489144 100644 --- a/server/tests/unit/mcp/tools-addon-gating.test.ts +++ b/server/tests/unit/mcp/tools-addon-gating.test.ts @@ -32,20 +32,14 @@ vi.mock('../../../src/config', () => ({ const { broadcastMock } = vi.hoisted(() => ({ broadcastMock: vi.fn() })); vi.mock('../../../src/websocket', () => ({ broadcast: broadcastMock })); -const { isAddonEnabledMock } = vi.hoisted(() => { - const isAddonEnabledMock = vi.fn().mockReturnValue(true); - return { isAddonEnabledMock }; -}); -vi.mock('../../../src/nest/addons/addons.bridge', () => ({ - isAddonEnabled: isAddonEnabledMock, - getCollabFeatures: vi.fn().mockReturnValue({ chat: true, notes: true, polls: true, whatsnext: true }), -})); import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip } from '../../helpers/factories'; import { createMcpHarness, parseToolResult, type McpHarness } from '../../helpers/mcp-harness'; +import { setAddonEnabled } from '../../helpers/test-db'; +import { ADDON_IDS } from '../../../src/addons'; beforeAll(() => { createTables(testDb); @@ -55,7 +49,12 @@ beforeAll(() => { beforeEach(() => { resetTestDb(testDb); broadcastMock.mockClear(); - isAddonEnabledMock.mockReturnValue(true); + // The summary reads these three off the injected AddonsService, so the case + // that wants one off writes the row the admin panel writes. resetTestDb + // leaves the addons table alone, so each case restates what it needs. + for (const id of [ADDON_IDS.BUDGET, ADDON_IDS.PACKING, ADDON_IDS.COLLAB]) { + setAddonEnabled(testDb, id, true); + } }); afterAll(() => { @@ -95,7 +94,7 @@ describe('get_trip_summary — addon gating', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id, { title: 'No Budget Trip' }); - isAddonEnabledMock.mockImplementation((id: string) => id !== 'budget'); + setAddonEnabled(testDb, ADDON_IDS.BUDGET, false); await withHarness(user.id, async (h) => { const result = await h.client.callTool({ name: 'get_trip_summary', arguments: { tripId: trip.id } }); @@ -112,7 +111,7 @@ describe('get_trip_summary — addon gating', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id, { title: 'No Packing Trip' }); - isAddonEnabledMock.mockImplementation((id: string) => id !== 'packing'); + setAddonEnabled(testDb, ADDON_IDS.PACKING, false); await withHarness(user.id, async (h) => { const result = await h.client.callTool({ name: 'get_trip_summary', arguments: { tripId: trip.id } }); @@ -128,7 +127,7 @@ describe('get_trip_summary — addon gating', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id, { title: 'No Collab Trip' }); - isAddonEnabledMock.mockImplementation((id: string) => id !== 'collab'); + setAddonEnabled(testDb, ADDON_IDS.COLLAB, false); await withHarness(user.id, async (h) => { const result = await h.client.callTool({ name: 'get_trip_summary', arguments: { tripId: trip.id } }); @@ -150,7 +149,7 @@ describe('Budget tools — addon gating', () => { it('when budget addon disabled, create_budget_item is not registered', async () => { const { user } = createUser(testDb); - isAddonEnabledMock.mockImplementation((id: string) => id !== 'budget'); + setAddonEnabled(testDb, ADDON_IDS.BUDGET, false); await withHarness(user.id, async (h) => { const result = await h.client.callTool({ name: 'create_budget_item', arguments: { tripId: 1, name: 'Test', total_price: 100 } }); @@ -167,7 +166,7 @@ describe('Packing tools — addon gating', () => { it('when packing addon disabled, create_packing_item is not registered', async () => { const { user } = createUser(testDb); - isAddonEnabledMock.mockImplementation((id: string) => id !== 'packing'); + setAddonEnabled(testDb, ADDON_IDS.PACKING, false); await withHarness(user.id, async (h) => { const result = await h.client.callTool({ name: 'create_packing_item', arguments: { tripId: 1, name: 'Sunscreen' } }); @@ -184,7 +183,7 @@ describe('Collab tools — addon gating', () => { it('when collab addon disabled, create_collab_note is not registered', async () => { const { user } = createUser(testDb); - isAddonEnabledMock.mockImplementation((id: string) => id !== 'collab'); + setAddonEnabled(testDb, ADDON_IDS.COLLAB, false); await withHarness(user.id, async (h) => { const result = await h.client.callTool({ name: 'create_collab_note', arguments: { tripId: 1, title: 'Test Note' } }); @@ -201,7 +200,7 @@ describe('Atlas tools — addon gating', () => { it('when atlas addon disabled, mark_country_visited is not registered', async () => { const { user } = createUser(testDb); - isAddonEnabledMock.mockImplementation((id: string) => id !== 'atlas'); + setAddonEnabled(testDb, ADDON_IDS.ATLAS, false); await withHarness(user.id, async (h) => { const result = await h.client.callTool({ name: 'mark_country_visited', arguments: { country_code: 'FR' } }); @@ -212,7 +211,7 @@ describe('Atlas tools — addon gating', () => { it('when atlas addon disabled, create_bucket_list_item is not registered', async () => { const { user } = createUser(testDb); - isAddonEnabledMock.mockImplementation((id: string) => id !== 'atlas'); + setAddonEnabled(testDb, ADDON_IDS.ATLAS, false); await withHarness(user.id, async (h) => { const result = await h.client.callTool({ name: 'create_bucket_list_item', arguments: { name: 'Paris' } }); diff --git a/server/tests/unit/mcp/tools-atlas-expanded.test.ts b/server/tests/unit/mcp/tools-atlas-expanded.test.ts index 5aaf5db63..5320520ff 100644 --- a/server/tests/unit/mcp/tools-atlas-expanded.test.ts +++ b/server/tests/unit/mcp/tools-atlas-expanded.test.ts @@ -35,16 +35,14 @@ vi.mock('../../../src/config', () => ({ const { broadcastMock } = vi.hoisted(() => ({ broadcastMock: vi.fn() })); vi.mock('../../../src/websocket', () => ({ broadcast: broadcastMock })); -vi.mock('../../../src/nest/addons/addons.bridge', () => ({ - isAddonEnabled: vi.fn().mockReturnValue(true), - getCollabFeatures: vi.fn().mockReturnValue({ chat: true, notes: true, polls: true, whatsnext: true }), -})); import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createBucketListItem, createVisitedCountry } from '../../helpers/factories'; import { createMcpHarness, parseToolResult, parseResourceResult, type McpHarness } from '../../helpers/mcp-harness'; +import { setAddonEnabled } from '../../helpers/test-db'; +import { ADDON_IDS } from '../../../src/addons'; beforeAll(() => { createTables(testDb); @@ -52,6 +50,7 @@ beforeAll(() => { }); beforeEach(() => { + setAddonEnabled(testDb, ADDON_IDS.ATLAS, true); resetTestDb(testDb); broadcastMock.mockClear(); delete process.env.DEMO_MODE; diff --git a/server/tests/unit/mcp/tools-collab-polls-chat.test.ts b/server/tests/unit/mcp/tools-collab-polls-chat.test.ts index 38df7c2cb..c4577fa06 100644 --- a/server/tests/unit/mcp/tools-collab-polls-chat.test.ts +++ b/server/tests/unit/mcp/tools-collab-polls-chat.test.ts @@ -37,16 +37,14 @@ vi.mock('../../../src/config', () => ({ const { broadcastMock } = vi.hoisted(() => ({ broadcastMock: vi.fn() })); vi.mock('../../../src/websocket', () => ({ broadcast: broadcastMock })); -vi.mock('../../../src/nest/addons/addons.bridge', () => ({ - isAddonEnabled: vi.fn().mockReturnValue(true), - getCollabFeatures: vi.fn().mockReturnValue({ chat: true, notes: true, polls: true, whatsnext: true }), -})); import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip } from '../../helpers/factories'; import { createMcpHarness, parseToolResult, parseResourceResult, type McpHarness } from '../../helpers/mcp-harness'; +import { setAddonEnabled } from '../../helpers/test-db'; +import { ADDON_IDS } from '../../../src/addons'; beforeAll(() => { createTables(testDb); @@ -54,6 +52,7 @@ beforeAll(() => { }); beforeEach(() => { + setAddonEnabled(testDb, ADDON_IDS.COLLAB, true); resetTestDb(testDb); broadcastMock.mockClear(); delete process.env.DEMO_MODE; diff --git a/server/tests/unit/mcp/tools-journey.test.ts b/server/tests/unit/mcp/tools-journey.test.ts index bba0788fd..aa5faf38e 100644 --- a/server/tests/unit/mcp/tools-journey.test.ts +++ b/server/tests/unit/mcp/tools-journey.test.ts @@ -34,15 +34,12 @@ vi.mock('../../../src/config', () => ({ const { broadcastMock } = vi.hoisted(() => ({ broadcastMock: vi.fn() })); vi.mock('../../../src/websocket', () => ({ broadcast: broadcastMock, broadcastToUser: broadcastMock })); -vi.mock('../../../src/nest/addons/addons.bridge', async (importOriginal) => { - const original = await importOriginal() as Record; - return { ...original, isAddonEnabled: vi.fn().mockReturnValue(true) }; -}); - import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip } from '../../helpers/factories'; +import { setAddonEnabled } from '../../helpers/test-db'; +import { ADDON_IDS } from '../../../src/addons'; import { createMcpHarness, parseToolResult, type McpHarness } from '../../helpers/mcp-harness'; beforeAll(() => { @@ -52,6 +49,9 @@ beforeAll(() => { beforeEach(() => { resetTestDb(testDb); + // The `when:` gate reads the injected AddonsService, so the toggle is the row + // the admin panel writes — and this addon ships disabled by default. + setAddonEnabled(testDb, ADDON_IDS.JOURNEY, true); broadcastMock.mockClear(); delete process.env.DEMO_MODE; }); diff --git a/server/tests/unit/mcp/tools-prompts.test.ts b/server/tests/unit/mcp/tools-prompts.test.ts index 3449a89fd..e20590643 100644 --- a/server/tests/unit/mcp/tools-prompts.test.ts +++ b/server/tests/unit/mcp/tools-prompts.test.ts @@ -42,10 +42,14 @@ const { isAddonEnabledMock } = vi.hoisted(() => { const isAddonEnabledMock = vi.fn().mockReturnValue(true); return { isAddonEnabledMock }; }); -vi.mock('../../../src/nest/addons/addons.bridge', () => ({ +// Was a module mock over addons.bridge. The `when:` gates read the controller's +// injected AddonsService now, so the toggle goes in through the constructor — +// which is the point of the change: the dependency is in the signature instead +// of behind an import path. +const addonsStub = { isAddonEnabled: isAddonEnabledMock, - getCollabFeatures: vi.fn().mockReturnValue({ chat: true, notes: true, polls: true, whatsnext: true }), -})); + getCollabFeatures: () => ({ chat: true, notes: true, polls: true, whatsnext: true }), +} as unknown as AddonsService; const { mockGetTripSummary } = vi.hoisted(() => ({ mockGetTripSummary: vi.fn(), @@ -77,6 +81,7 @@ import type { AuthService } from '../../../src/nest/auth/auth.service'; import type { TripsService } from '../../../src/nest/trips/trips.service'; import type { TodoService } from '../../../src/nest/todo/todo.service'; import type { CollabService } from '../../../src/nest/collab/collab.service'; +import { AddonsService } from '../../../src/nest/addons/addons.service'; // The trip-summary prompt moved to the DI-discovered TripsMcp — its cases below // exercise it through a hand-built registry over a stub TripsService whose @@ -97,6 +102,7 @@ const tripsMcp = new TripsMcp( undefined as never, undefined as never, readModelStub, + addonsStub, ); // The three remaining prompts moved to their domains' @McpController classes: @@ -107,11 +113,14 @@ const authStub = { isDemoUser: () => false } as unknown as AuthService; const packingMcp = new PackingMcp( new PackingService(promptDbs(), new PermissionsService(promptDbs()), new RealtimeService()), authStub, + addonsStub, ); const budgetMcp = new BudgetMcp( new BudgetService(promptDbs(), new PermissionsService(promptDbs()), new ExchangeRatesService(), new RealtimeService()), new ExchangeRatesService(), promptDbs(), + undefined as never, + addonsStub, ); const authMcp = new AuthMcp(); diff --git a/server/tests/unit/mcp/tools-vacay.test.ts b/server/tests/unit/mcp/tools-vacay.test.ts index 6ba5a970b..1bb36341d 100644 --- a/server/tests/unit/mcp/tools-vacay.test.ts +++ b/server/tests/unit/mcp/tools-vacay.test.ts @@ -42,11 +42,6 @@ vi.mock('../../../src/config', () => ({ const { broadcastMock } = vi.hoisted(() => ({ broadcastMock: vi.fn() })); vi.mock('../../../src/websocket', () => ({ broadcast: broadcastMock })); -vi.mock('../../../src/nest/addons/addons.bridge', () => ({ - isAddonEnabled: vi.fn().mockReturnValue(true), - getCollabFeatures: vi.fn().mockReturnValue({ chat: true, notes: true, polls: true, whatsnext: true }), -})); - // share_vacay_calendar fires a user notification after inserting; stub it out vi.mock('../../../src/nest/notifications/notifications.bridge', () => ({ send: vi.fn().mockResolvedValue(undefined) })); @@ -54,6 +49,8 @@ import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser } from '../../helpers/factories'; +import { setAddonEnabled } from '../../helpers/test-db'; +import { ADDON_IDS } from '../../../src/addons'; import { createMcpHarness, parseToolResult, parseResourceResult, type McpHarness } from '../../helpers/mcp-harness'; import { VacayService } from '../../../src/nest/vacay/vacay.service'; @@ -73,6 +70,9 @@ beforeAll(() => { beforeEach(() => { resetTestDb(testDb); + // The `when:` gate reads the injected AddonsService, so the toggle is the row + // the admin panel writes — and this addon ships disabled by default. + setAddonEnabled(testDb, ADDON_IDS.VACAY, true); broadcastMock.mockClear(); delete process.env.DEMO_MODE; });