diff --git a/server/src/nest/admin/admin.bridge.ts b/server/src/nest/admin/admin.bridge.ts index a1d3d81cf..74e6dc3a0 100644 --- a/server/src/nest/admin/admin.bridge.ts +++ b/server/src/nest/admin/admin.bridge.ts @@ -49,6 +49,7 @@ const admin = new AdminService( permissions, new NotificationsService(dbs, realtime, mailer, new WebhookService(dbs), new NtfyService(dbs), notifPrefs), userCleanup, + new RealtimeService(), ); export function checkAndNotifyVersion(): Promise { diff --git a/server/src/nest/admin/admin.service.ts b/server/src/nest/admin/admin.service.ts index fd10075f2..74c28ce99 100644 --- a/server/src/nest/admin/admin.service.ts +++ b/server/src/nest/admin/admin.service.ts @@ -23,6 +23,7 @@ import { validatePassword } from '../common/passwordPolicy'; import { UserCleanupService } from '../auth/user-cleanup.service'; import { DatabaseService } from '../database/database.service'; import { AddonsService } from '../addons/addons.service'; +import { RealtimeService } from '../realtime/realtime.service'; import { PasskeyService } from '../auth/passkey.service'; import { AuthService } from '../auth/auth.service'; import { PermissionsService } from '../permissions/permissions.service'; @@ -73,6 +74,7 @@ export class AdminService { private readonly permissions: PermissionsService, private readonly notifications: NotificationsService, private readonly userCleanup: UserCleanupService, + private readonly realtime: RealtimeService, ) {} // ── User CRUD ────────────────────────────────────────────────────────────── @@ -89,9 +91,10 @@ export class AdminService { ); let onlineUserIds = new Set(); try { - // Lazy require: the ws singleton must not become a load-time dependency. - const { getOnlineUserIds } = require('../../websocket'); - onlineUserIds = getOnlineUserIds(); + // The catch stays here rather than in the facade: an admin list that shows + // everyone as offline is a better answer than a 500, and that judgement + // belongs to this caller, not to every future one. + onlineUserIds = this.realtime.getOnlineUserIds(); } catch { /* */ } diff --git a/server/src/nest/realtime/realtime.service.ts b/server/src/nest/realtime/realtime.service.ts index e8ad9468c..1b66e27e1 100644 --- a/server/src/nest/realtime/realtime.service.ts +++ b/server/src/nest/realtime/realtime.service.ts @@ -5,7 +5,7 @@ import type { TrekWsTripEventName, TrekWsUserEventName, } from '@trek/shared'; -import { broadcast, broadcastToUser } from '../../websocket'; +import { broadcast, broadcastToUser, getOnlineUserIds } from '../../websocket'; /** * Injectable facade over the websocket module singleton (roadmap Phase 0 item 3). @@ -89,4 +89,18 @@ export class RealtimeService { ): void { broadcastToUser(...args); } + + /** + * Which users currently hold an open socket. + * + * AdminService read this through a lazy `require('../../websocket')` inside a + * try/catch, which is what the facade exists to avoid. Deliberately NOT + * guarded here: a guard on the facade would swallow the failure for every + * future caller, and the one caller that wants a degraded answer rather than + * an error keeps its own catch. Delegates at call time, like its two + * siblings, so per-file vi.mock stubs keep flowing through. + */ + getOnlineUserIds(): Set { + return getOnlineUserIds(); + } } diff --git a/server/tests/unit/nest/admin.service.test.ts b/server/tests/unit/nest/admin.service.test.ts index 3a2edcb75..f9ad564e9 100644 --- a/server/tests/unit/nest/admin.service.test.ts +++ b/server/tests/unit/nest/admin.service.test.ts @@ -63,6 +63,7 @@ import { SettingsService } from '../../../src/nest/settings/settings.service'; import { AtlasService } from '../../../src/nest/atlas/atlas.service'; import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service'; import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service'; +import { MailerService } from '../../../src/nest/notifications/mailer/mailer.service'; import { WebauthnConfigService } from '../../../src/nest/auth/webauthn-config.service'; import { AuthService } from '../../../src/nest/auth/auth.service'; import { PasskeyService } from '../../../src/nest/auth/passkey.service'; @@ -79,7 +80,11 @@ const realtime = new RealtimeService(); const permissions = new PermissionsService(dbs); const webauthn = new WebauthnConfigService(dbs); const userCleanup = new UserCleanupService(dbs); -const auth = new AuthService(dbs, permissions, new AtlasService(dbs), new TripMembershipService(dbs), webauthn, userCleanup); +// Positional and previously wrong: an AtlasService sat in the membership slot +// and the mailer was missing entirely, so `auth` was built with its last four +// collaborators shifted by one. Nothing failed, because none of the cases below +// reach a path that uses them. +const auth = new AuthService(dbs, permissions, new TripMembershipService(dbs), webauthn, userCleanup, new MailerService(dbs)); const svc = new AdminService( dbs, new AddonsService(dbs), @@ -88,6 +93,7 @@ const svc = new AdminService( permissions, makeNotificationsService(dbs, realtime), userCleanup, + realtime, ); // Legacy free-function names bound to the service, so the moved cases below read @@ -100,8 +106,6 @@ const getStats = () => svc.getStats(); const getPermissions = () => svc.getPermissions(); const savePermissions = (p: Record) => svc.savePermissions(p); const getAuditLog = (q: { limit?: string; offset?: string }) => svc.getAuditLog(q); -const getOidcSettings = () => svc.getOidcSettings(); -const updateOidcSettings = (d: Parameters[0]) => svc.updateOidcSettings(d); const saveDemoBaseline = () => svc.saveDemoBaseline(); const getGithubReleases = (perPage?: string, page?: string) => svc.getGithubReleases(perPage, page); const checkVersion = () => svc.checkVersion(); diff --git a/server/tests/unit/plugins/plugins-service.test.ts b/server/tests/unit/plugins/plugins-service.test.ts index 136e648a7..d214c76b5 100644 --- a/server/tests/unit/plugins/plugins-service.test.ts +++ b/server/tests/unit/plugins/plugins-service.test.ts @@ -20,6 +20,7 @@ vi.mock('../../../src/db/database', () => ({ db: testDb })); import { db as dbConn } from '../../../src/db/database'; import { DatabaseService } from '../../../src/nest/database/database.service'; +import { AddonsService } from '../../../src/nest/addons/addons.service'; import { PluginsService } from '../../../src/nest/plugins/plugins.service'; import { PluginsController } from '../../../src/nest/plugins/plugins.controller'; import { PluginsFeedController } from '../../../src/nest/plugins/plugins-feed.controller'; @@ -40,7 +41,7 @@ describe('PluginsService.list', () => { .run('flight', 'Flight', 'desc', 'widget', 'inactive', '1.0.0'); process.env.TREK_PLUGINS_ENABLED = 'true'; - const out = new PluginsService(new DatabaseService(dbConn)).list(); + const out = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list(); expect(out.enabled).toBe(true); expect(out.plugins).toHaveLength(1); expect(out.plugins[0]).toMatchObject({ id: 'flight', name: 'Flight', status: 'inactive' }); @@ -51,14 +52,14 @@ describe('PluginsService.list', () => { .prepare('INSERT INTO plugins (id, name, description, type, status, version) VALUES (?,?,?,?,?,?)') .run('flight', 'Flight', 'desc', 'widget', 'inactive', '1.0.0'); - const out = new PluginsService(new DatabaseService(dbConn)).list(); + const out = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list(); expect(out.enabled).toBe(true); expect(out.plugins).toHaveLength(1); }); it('reports disabled when the kill switch is off (TREK_PLUGINS_ENABLED=false)', () => { process.env.TREK_PLUGINS_ENABLED = 'false'; - const out = new PluginsService(new DatabaseService(dbConn)).list(); + const out = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list(); expect(out.enabled).toBe(false); expect(out.plugins).toEqual([]); }); @@ -78,7 +79,7 @@ describe('PluginsService.list', () => { const key = 'RWTvBn0aBcDeFgHiJkLmNoPqRsTuVwXyZ0123456789abcd'; insert('signed-one', 'acme/signed-one', key); - const p = new PluginsService(new DatabaseService(dbConn)).list().plugins[0]; + const p = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list().plugins[0]; expect(p.signed).toBe(true); // Short head…tail, for eyeballing against what the author reads out over the // phone. NOT a confidentiality measure — the key is public, and the re-trust @@ -88,7 +89,7 @@ describe('PluginsService.list', () => { it('reports unsigned for a registry plugin with no pinned key', () => { insert('plain', 'acme/plain', null); - const p = new PluginsService(new DatabaseService(dbConn)).list().plugins[0]; + const p = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list().plugins[0]; expect(p.signed).toBe(false); expect(p.keyFingerprint).toBeNull(); }); @@ -96,7 +97,7 @@ describe('PluginsService.list', () => { it('reports unsigned for a sideloaded and a dev-linked plugin (they carry no key)', () => { insert('uploaded', 'local:upload', null); insert('linked', 'local:link', null); - const plugins = new PluginsService(new DatabaseService(dbConn)).list().plugins; + const plugins = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list().plugins; expect(plugins.map((p) => [p.id, p.signed, p.source_repo])).toEqual([ ['linked', false, 'local:link'], ['uploaded', false, 'local:upload'], @@ -110,14 +111,14 @@ describe('PluginsService.list', () => { .prepare('UPDATE plugins SET update_block_code = ?, update_block_detail = ?, update_block_version = ? WHERE id = ?') .run('SIGNATURE_KEY_CHANGED', 'the key changed', '2.0.0', 'blocked'); - const byId = Object.fromEntries(new PluginsService(new DatabaseService(dbConn)).list().plugins.map((p) => [p.id, p])); + const byId = Object.fromEntries(new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list().plugins.map((p) => [p.id, p])); expect(byId.blocked.updateBlock).toEqual({ code: 'SIGNATURE_KEY_CHANGED', detail: 'the key changed', version: '2.0.0' }); expect(byId.fine.updateBlock).toBeNull(); }); it('never leaks the raw pinned key into the list response (only the fingerprint)', () => { insert('signed-one', 'acme/signed-one', 'RWTvBn0aBcDeFgHiJkLmNoPqRsTuVwXyZ0123456789abcd'); - const p = new PluginsService(new DatabaseService(dbConn)).list().plugins[0] as Record; + const p = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).list().plugins[0] as Record; expect(p.author_pubkey).toBeUndefined(); }); }); @@ -238,7 +239,7 @@ describe('PluginsService instance config', () => { testDb.prepare("INSERT INTO plugins (id, name, status, config) VALUES ('x','X','inactive','{}')").run(); testDb.prepare("INSERT INTO plugin_settings_fields (plugin_id, field_key, scope, secret) VALUES ('x','api_key','instance',1)").run(); - const svc = new PluginsService(new DatabaseService(dbConn)); + const svc = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))); const masked = svc.updateInstanceConfig('x', { api_key: 'super-secret', server: 'https://h' }); // client gets the masked view expect(masked.api_key).toBe('••••••••'); @@ -259,8 +260,8 @@ describe('PluginsService instance config', () => { }); it('throws for an unknown plugin', () => { - expect(() => new PluginsService(new DatabaseService(dbConn)).updateInstanceConfig('nope', {})).toThrow(/not found/); - expect(() => new PluginsService(new DatabaseService(dbConn)).getInstanceConfig('nope')).toThrow(/not found/); + expect(() => new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).updateInstanceConfig('nope', {})).toThrow(/not found/); + expect(() => new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))).getInstanceConfig('nope')).toThrow(/not found/); }); }); @@ -268,7 +269,7 @@ describe('PluginsService error log', () => { beforeEach(() => testDb.exec('DELETE FROM plugin_error_log')); it('lists and clears a plugin error log', () => { testDb.prepare("INSERT INTO plugin_error_log (plugin_id, level, message) VALUES ('p','error','boom')").run(); - const svc = new PluginsService(new DatabaseService(dbConn)); + const svc = new PluginsService(new DatabaseService(dbConn), new AddonsService(new DatabaseService(dbConn))); expect(svc.errors('p')).toEqual([{ ts: '2026-01-01', level: 'error', message: 'boom' }]); svc.clearErrors('p'); expect(svc.errors('p')).toEqual([]);