diff --git a/.changeset/config-reload-isolation.md b/.changeset/config-reload-isolation.md new file mode 100644 index 00000000..85c03959 --- /dev/null +++ b/.changeset/config-reload-isolation.md @@ -0,0 +1,7 @@ +--- +'@when/web': patch +'@when/worker': patch +'@when/cli': patch +--- + +Isolate configuration loading inputs and adopt a reusable, serialized reload monitor based on Directory's approach. Preserve existing path conventions and restart rules, retain accepted settings after invalid edits, and make cleanup available before startup reconciliation. diff --git a/apps/web/src/lib/server/boot.test.ts b/apps/web/src/lib/server/boot.test.ts new file mode 100644 index 00000000..7e56f58b --- /dev/null +++ b/apps/web/src/lib/server/boot.test.ts @@ -0,0 +1,111 @@ +import { afterEach, beforeEach, expect, test, vi } from 'vitest'; +import { EventEmitter } from 'node:events'; +import { mkdtemp, rm, stat, writeFile } from 'node:fs/promises'; +import type { ConfigPathOptions } from '@when/config'; +import type { Stats } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { stringify } from 'yaml'; +import { bootApp } from './boot'; +import { getState } from './state'; +import { validConfig } from './__fixtures__/valid-config'; + +type Listener = (current: Stats, previous: Stats) => Promise; +const mocks = vi.hoisted(() => ({ + watchFile: vi.fn<(path: string, options: object, listener: Listener) => void>(), + unwatchFile: vi.fn(), + loadConfigFile: vi.fn(), + makeAuth: vi.fn(), + openDb: vi.fn(), + runMigrations: vi.fn().mockResolvedValue([]) +})); +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, watchFile: mocks.watchFile, unwatchFile: mocks.unwatchFile }; +}); +vi.mock('@when/config', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, loadConfigFile: mocks.loadConfigFile }; +}); +vi.mock('@when/db', () => ({ openDb: mocks.openDb, runMigrations: mocks.runMigrations })); +vi.mock('@when/jobs', () => ({ initOpenWorkflow: vi.fn() })); +vi.mock('@when/calendar', () => ({ setLogger: vi.fn() })); +vi.mock('./auth', () => ({ makeAuth: mocks.makeAuth })); +vi.mock('./logger', () => ({ + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), fatal: vi.fn() } +})); + +let directory: string; +const database = {}; + +function emitShutdown(): void { + EventEmitter.prototype.emit.call(process, 'sveltekit:shutdown'); +} + +beforeEach(async () => { + vi.resetAllMocks(); + const { loadConfigFile } = await vi.importActual('@when/config'); + mocks.loadConfigFile.mockImplementation(loadConfigFile); + mocks.openDb.mockReturnValue(database); + mocks.runMigrations.mockResolvedValue([]); + vi.spyOn(process, 'exit').mockImplementation(() => undefined as never); + directory = await mkdtemp(join(tmpdir(), 'when-app-boot-')); + await writeFile(join(directory, 'when.yaml'), stringify(validConfig)); +}); + +afterEach(async () => { + emitShutdown(); + vi.restoreAllMocks(); + await rm(directory, { recursive: true, force: true }); +}); + +test('initial load and reload keep bare-filename database roots and existing resources', async () => { + const path = join(directory, 'when.yaml'); + await bootApp({ cwd: directory, env: { CONFIG_PATH: 'when.yaml' } }); + const previous = await stat(path); + const next = { ...validConfig, user: { ...validConfig.user, name: 'Reloaded owner' } }; + await writeFile(path, stringify(next)); + const listener = mocks.watchFile.mock.calls[0][2]; + await listener(await stat(path), previous); + + const state = getState(); + expect(state.config.user.name).toBe('Reloaded owner'); + expect(state.config.database.app).toBe(join(directory, 'data', 'when.sqlite')); + expect(state.config.database.queue).toBe(join(directory, 'data', 'openworkflow.sqlite')); + expect(state.db).toBe(database); + expect(mocks.openDb).toHaveBeenCalledExactlyOnceWith(join(directory, 'data', 'when.sqlite')); + expect(mocks.makeAuth).toHaveBeenCalledOnce(); + expect(process.exit).not.toHaveBeenCalled(); +}); + +test('shutdown during startup reconciliation already owns stop and prevents late application', async () => { + const { loadConfigFile } = await vi.importActual('@when/config'); + const reading = Promise.withResolvers(); + const release = Promise.withResolvers(); + mocks.loadConfigFile + .mockImplementationOnce(loadConfigFile) + .mockImplementationOnce(async (path?: string, options?: ConfigPathOptions) => { + reading.resolve(); + await release.promise; + return loadConfigFile(path, options); + }); + const startup = bootApp({ cwd: directory, env: { CONFIG_PATH: 'when.yaml' } }); + try { + await reading.promise; + await writeFile( + join(directory, 'when.yaml'), + stringify({ ...validConfig, user: { ...validConfig.user, name: 'Too late' } }) + ); + emitShutdown(); + expect(mocks.unwatchFile).toHaveBeenCalledExactlyOnceWith( + join(directory, 'when.yaml'), + mocks.watchFile.mock.calls[0][2] + ); + } finally { + release.resolve(); + await startup; + } + expect(getState().config.user.name).toBe(validConfig.user.name); + expect(mocks.makeAuth).toHaveBeenCalledOnce(); + expect(process.exit).not.toHaveBeenCalled(); +}); diff --git a/apps/web/src/lib/server/boot.ts b/apps/web/src/lib/server/boot.ts index bde2b62e..b8d44b64 100644 --- a/apps/web/src/lib/server/boot.ts +++ b/apps/web/src/lib/server/boot.ts @@ -1,16 +1,24 @@ import { makeAuth } from './auth'; -import { getState, setState } from './state'; +import { setState } from './state'; +import { resolve } from 'node:path'; import { bootConfig } from './config/boot'; import { applyConfig } from './config/reload'; import { openDb, runMigrations } from '@when/db'; import { initOpenWorkflow } from '@when/jobs'; import { setLogger } from '@when/calendar'; -import { watchConfig } from '@when/config'; +import { + loadConfigFile, + resolveConfigPath, + startConfigReloading, + type ConfigPathOptions +} from '@when/config'; import { logger } from './logger'; -export async function bootApp(): Promise { +export async function bootApp(options: ConfigPathOptions = {}): Promise { setLogger(logger); - const config = await bootConfig(); + const context = { env: options.env ?? process.env, cwd: options.cwd ?? process.cwd() }; + const selectedPath = resolveConfigPath(context); + const config = await bootConfig(selectedPath, context); makeAuth(config); const db = openDb(config.database.app); const applied = await runMigrations(db); @@ -18,12 +26,17 @@ export async function bootApp(): Promise { initOpenWorkflow({ dbPath: config.database.queue }); setState({ config, db }); - const stopWatching = watchConfig((result) => - applyConfig(result, { - current: () => getState().config, - swap: (next) => setState({ config: next, db }), - restart: () => process.exit(0) - }) - ); - process.once('sveltekit:shutdown', stopWatching); + const monitor = startConfigReloading({ + path: resolve(context.cwd, selectedPath), + initial: config, + load: () => loadConfigFile(selectedPath, context), + onChange: (next, previous) => + applyConfig(next, previous, { + swap: (next) => setState({ config: next, db }), + restart: () => process.exit(0) + }), + onError: (error) => logger.error({ err: error }, 'config reload failed; keeping current config') + }); + process.once('sveltekit:shutdown', monitor.stop); + await monitor.ready; } diff --git a/apps/web/src/lib/server/config/boot.test.ts b/apps/web/src/lib/server/config/boot.test.ts index aadfe54a..6fbf9fd4 100644 --- a/apps/web/src/lib/server/config/boot.test.ts +++ b/apps/web/src/lib/server/config/boot.test.ts @@ -26,10 +26,17 @@ test('invalid config throws ConfigError', async () => { await expect(bootConfig(cfgPath)).rejects.toBeInstanceOf(ConfigError); }); -test('valid config loads and returns typed object', async () => { - const cfgPath = join(dir, 'config.yaml'); - await writeFile(cfgPath, stringify(validConfig)); - const cfg = await bootConfig(cfgPath); - expect(cfg.user.name).toBe('Jane Doe'); - expect(cfg.meetings['30-min-chat'].title).toBe('30 minute chat'); +test('boot uses the supplied file context without relocating bare-filename databases', async () => { + const raw = { + ...validConfig, + auth: { credentials: { username: 'admin', password: '${BOOT_PASSWORD}' } } + }; + await writeFile(join(dir, 'when.yaml'), stringify(raw)); + const cfg = await bootConfig(undefined, { + cwd: dir, + env: { CONFIG_PATH: 'when.yaml', BOOT_PASSWORD: 'synthetic-boot-password' } + }); + expect(cfg.auth.credentials?.password).toBe('synthetic-boot-password'); + expect(cfg.database.app).toBe(join(dir, 'data', 'when.sqlite')); + expect(cfg.database.queue).toBe(join(dir, 'data', 'openworkflow.sqlite')); }); diff --git a/apps/web/src/lib/server/config/boot.ts b/apps/web/src/lib/server/config/boot.ts index ae552c18..517a43ea 100644 --- a/apps/web/src/lib/server/config/boot.ts +++ b/apps/web/src/lib/server/config/boot.ts @@ -1,17 +1,22 @@ import { access } from 'node:fs/promises'; +import { resolve } from 'node:path'; import { logger } from '../logger'; import { allCalendars, ConfigError, - loadConfig, + loadConfigFile, MissingEnvVarsError, resolveConfigPath } from '@when/config'; -import type { WhenConfiguration } from '@when/config'; +import type { ConfigPathOptions, WhenConfiguration } from '@when/config'; -export async function bootConfig(path: string = resolveConfigPath()): Promise { - const exists = await access(path).then( +export async function bootConfig( + selectedPath?: string, + options: ConfigPathOptions = {} +): Promise { + const path = selectedPath ?? resolveConfigPath(options); + const exists = await access(resolve(options.cwd ?? process.cwd(), path)).then( () => true, () => false ); @@ -23,7 +28,7 @@ export async function bootConfig(path: string = resolveConfigPath()): Promise = {}): WhenConfiguration { - return { - auth: { credentials: { username: 'admin', password: 'p' } }, - database: { app: '/data/app.sqlite', queue: '/data/queue.sqlite' }, - url: { app: 'http://localhost:5173', internal: '', worker: 'http://when-worker:9000' }, - user: { name: 'A', email: 'a@example.com', timezone: 'UTC' }, - ...overrides - } as unknown as WhenConfiguration; +function effects() { + return { swap: vi.fn(), restart: vi.fn() }; } -function effects(current: WhenConfiguration) { - return { current: () => current, swap: vi.fn(), restart: vi.fn() }; -} - -test('applies a soft change via swap without restarting', () => { - const fx = effects(cfg({ user: { name: 'Old' } })); - const next = cfg({ user: { name: 'New' } }); - - applyConfig({ ok: true, config: next }, fx); +test('accepts a soft change through swap without restarting', () => { + const fx = effects(); + const next = { ...validConfig, user: { ...validConfig.user, name: 'Updated owner' } }; + expect(applyConfig(next, validConfig, fx)).toBe(true); expect(fx.swap).toHaveBeenCalledWith(next); expect(fx.restart).not.toHaveBeenCalled(); }); -test('restarts when auth changes', () => { - const fx = effects(cfg()); - const next = cfg({ auth: { credentials: { username: 'admin', password: 'changed' } } }); - - applyConfig({ ok: true, config: next }, fx); +test('rejects changed authentication and requests a restart without swapping', () => { + const fx = effects(); + const next = { + ...validConfig, + auth: { credentials: { username: 'admin', password: 'changed-password' } } + }; + expect(applyConfig(next, validConfig, fx)).toBe(false); expect(fx.restart).toHaveBeenCalledOnce(); expect(fx.swap).not.toHaveBeenCalled(); }); -test('restarts when the database section changes', () => { - const fx = effects(cfg()); - const next = cfg({ database: { app: '/other/app.sqlite', queue: '/data/queue.sqlite' } }); - - applyConfig({ ok: true, config: next }, fx); +test('rejects changed database settings and requests a restart', () => { + const fx = effects(); + const next = { + ...validConfig, + database: { ...validConfig.database, app: '/other/app.sqlite' } + }; + expect(applyConfig(next, validConfig, fx)).toBe(false); expect(fx.restart).toHaveBeenCalledOnce(); -}); - -test.for(['app', 'internal', 'worker'] as const)('handles a changed url.%s', (key) => { - const current = cfg(); - const fx = effects(current); - const next = cfg({ url: { ...current.url, [key]: 'https://changed.example.com' } }); - - applyConfig({ ok: true, config: next }, fx); - - if (key === 'app') { - expect(fx.restart).toHaveBeenCalledOnce(); - expect(fx.swap).not.toHaveBeenCalled(); - } else { - expect(fx.swap).toHaveBeenCalledWith(next); - expect(fx.restart).not.toHaveBeenCalled(); - } -}); - -test('keeps the current config when the reload failed', () => { - const fx = effects(cfg()); - - applyConfig({ ok: false, error: new Error('bad config') }, fx); - expect(fx.swap).not.toHaveBeenCalled(); +}); + +test('rejects a changed public app URL because authentication was built for the original', () => { + const fx = effects(); + const next = { ...validConfig, url: { ...validConfig.url, app: 'https://changed.example.com' } }; + + expect(applyConfig(next, validConfig, fx)).toBe(false); + expect(fx.restart).toHaveBeenCalledOnce(); + expect(fx.swap).not.toHaveBeenCalled(); +}); + +test('accepts internal service URL changes without restarting', () => { + const fx = effects(); + const next = { + ...validConfig, + url: { + ...validConfig.url, + internal: 'https://internal.example.com', + worker: 'http://worker.example.com:9000' + } + }; + + expect(applyConfig(next, validConfig, fx)).toBe(true); + expect(fx.swap).toHaveBeenCalledWith(next); + expect(fx.restart).not.toHaveBeenCalled(); +}); + +test('applies a declaration-order-only meeting change', () => { + const meeting = validConfig.meetings['30-min-chat']; + const previous = { ...validConfig, meetings: { first: meeting, second: meeting } }; + const next = { ...previous, meetings: { second: meeting, first: meeting } }; + let current: WhenConfiguration = previous; + const fx = { + swap: (config: WhenConfiguration) => { + current = config; + }, + restart: vi.fn() + }; + + expect(applyConfig(next, previous, fx)).toBe(true); + expect(Object.keys(current.meetings)).toEqual(['second', 'first']); expect(fx.restart).not.toHaveBeenCalled(); }); diff --git a/apps/web/src/lib/server/config/reload.ts b/apps/web/src/lib/server/config/reload.ts index 71e5448e..5abd4b97 100644 --- a/apps/web/src/lib/server/config/reload.ts +++ b/apps/web/src/lib/server/config/reload.ts @@ -1,8 +1,7 @@ -import { configChanged, type ReloadResult, type WhenConfiguration } from '@when/config'; +import { configChanged, type WhenConfiguration } from '@when/config'; import { logger } from '../logger'; export interface ReloadEffects { - current: () => WhenConfiguration; swap: (config: WhenConfiguration) => void; restart: () => void; } @@ -10,20 +9,17 @@ export interface ReloadEffects { // Auth (including its canonical URL) and DB/queue connections are built at boot. const RESTART_KEYS: (keyof WhenConfiguration)[] = ['auth', 'database']; -export function applyConfig(result: ReloadResult, fx: ReloadEffects): void { - if (!result.ok) { - logger.error({ err: result.error }, 'config reload failed; keeping current config'); - return; - } - const current = fx.current(); - if ( - configChanged(current, result.config, RESTART_KEYS) || - current.url.app !== result.config.url.app - ) { +export function applyConfig( + next: WhenConfiguration, + previous: WhenConfiguration, + fx: ReloadEffects +): boolean { + if (configChanged(previous, next, RESTART_KEYS) || previous.url.app !== next.url.app) { logger.warn('config change requires a restart'); fx.restart(); - return; + return false; } - fx.swap(result.config); + fx.swap(next); logger.info('config reloaded'); + return true; } diff --git a/apps/worker/src/index.test.ts b/apps/worker/src/index.test.ts index 9113a40b..aa47d383 100644 --- a/apps/worker/src/index.test.ts +++ b/apps/worker/src/index.test.ts @@ -1,11 +1,13 @@ import { EventEmitter } from 'node:events'; +import { resolve } from 'node:path'; import { afterEach, beforeEach, expect, test, vi } from 'vitest'; -import type { ReloadResult, WhenConfiguration } from '@when/config'; +import type { ConfigReloadOptions, WhenConfiguration } from '@when/config'; import { expirePendingAppointments, refreshCalendars, syncCalendars } from '@when/jobs'; const mocks = vi.hoisted(() => ({ - loadConfig: vi.fn(), - watchConfig: vi.fn(), + loadConfigFile: vi.fn(), + resolveConfigPath: vi.fn(), + startConfigReloading: vi.fn(), stopWatch: vi.fn(), openDb: vi.fn(), runMigrations: vi.fn(), @@ -26,9 +28,18 @@ const mocks = vi.hoisted(() => ({ logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() } })); +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, watchFile: vi.fn(), unwatchFile: vi.fn() }; +}); vi.mock('@when/config', async (importOriginal) => { const actual = await importOriginal(); - return { ...actual, loadConfig: mocks.loadConfig, watchConfig: mocks.watchConfig }; + return { + ...actual, + loadConfigFile: mocks.loadConfigFile, + resolveConfigPath: mocks.resolveConfigPath, + startConfigReloading: mocks.startConfigReloading + }; }); vi.mock('@when/db', () => ({ openDb: mocks.openDb, runMigrations: mocks.runMigrations })); vi.mock('@when/jobs', async (importOriginal) => { @@ -59,7 +70,9 @@ const worker = { start: mocks.start, stop: mocks.stop }; const client = { newWorker: mocks.newWorker, runWorkflow: mocks.runWorkflow }; let events: string[]; let signals: Map void>; -let reload: (result: ReloadResult) => void; +let reloadOptions: ConfigReloadOptions; +let monitoringReady: Promise; +let stopMonitoring: (() => void) | undefined; let server: EventEmitter & { listening: boolean; listen: ReturnType; @@ -74,16 +87,21 @@ function finishListening() { async function boot() { await import('./index.js'); await vi.waitFor(() => { - expect(mocks.watchConfig).toHaveBeenCalledOnce(); + expect(mocks.startConfigReloading).toHaveBeenCalledOnce(); expect(signals.has('SIGTERM')).toBe(true); expect(signals.has('SIGINT')).toBe(true); }); } -beforeEach(() => { +beforeEach(async () => { vi.resetModules(); vi.resetAllMocks(); + const { startConfigReloading } = + await vi.importActual('@when/config'); + stopMonitoring = undefined; vi.stubEnv('PORT', '19099'); + vi.stubEnv('CONFIG_PATH', 'when.yaml'); + vi.stubEnv('WHEN_WORKER_CONFIG_TEST', 'synthetic-context'); events = []; signals = new Map(); vi.spyOn(process, 'on').mockImplementation((event, listener) => { @@ -91,7 +109,8 @@ beforeEach(() => { return process; }); vi.spyOn(process, 'exit').mockImplementation(() => undefined as never); - mocks.loadConfig.mockResolvedValue(config); + mocks.loadConfigFile.mockResolvedValue(config); + mocks.resolveConfigPath.mockReturnValue('when.yaml'); mocks.openDb.mockReturnValue(db); mocks.runMigrations.mockResolvedValue([]); mocks.createMailer.mockReturnValue(mailer); @@ -105,13 +124,19 @@ beforeEach(() => { events.push(`enqueue:${spec.name}`); return { result: mocks.result }; }); - mocks.stopWatch.mockImplementation(() => { - events.push('watch:stop'); - }); - mocks.watchConfig.mockImplementation((callback: typeof reload) => { - reload = callback; - return mocks.stopWatch; - }); + mocks.startConfigReloading.mockImplementation( + (options: ConfigReloadOptions) => { + reloadOptions = options; + const monitor = startConfigReloading(options); + stopMonitoring = monitor.stop; + monitoringReady = monitor.ready; + mocks.stopWatch.mockImplementation(() => { + events.push('watch:stop'); + monitor.stop(); + }); + return { stop: mocks.stopWatch, ready: monitor.ready }; + } + ); mocks.stop.mockImplementation(async () => { events.push('worker:stop'); }); @@ -143,6 +168,7 @@ beforeEach(() => { }); afterEach(() => { + stopMonitoring?.(); vi.restoreAllMocks(); vi.unstubAllEnvs(); }); @@ -158,7 +184,7 @@ test('registers workflows before starting one default worker, then enqueues all await vi.waitFor(() => expect(mocks.start).toHaveBeenCalledOnce()); expect(mocks.runWorkflow).not.toHaveBeenCalled(); expect(mocks.createHealthServer).not.toHaveBeenCalled(); - expect(mocks.loadConfig).toHaveBeenCalledWith(); + expect(mocks.loadConfigFile.mock.calls[0][0]).toBe('when.yaml'); expect(mocks.openDb).toHaveBeenCalledWith(config.database.app); expect(mocks.runMigrations).toHaveBeenCalledWith(db); expect(mocks.createMailer).toHaveBeenCalledWith(config, mocks.logger); @@ -175,7 +201,7 @@ test('registers workflows before starting one default worker, then enqueues all expect(mocks.newWorker).toHaveBeenCalledWith(); started.resolve(); - await vi.waitFor(() => expect(mocks.watchConfig).toHaveBeenCalledOnce()); + await vi.waitFor(() => expect(mocks.startConfigReloading).toHaveBeenCalledOnce()); expect(mocks.runWorkflow.mock.calls).toEqual([ [refreshCalendars, {}], [expirePendingAppointments, {}], @@ -216,14 +242,109 @@ test.each([0, 1, 2])( enqueued.resolve(); await vi.waitFor(() => expect(server.listen).toHaveBeenCalledOnce()); expect(mocks.runWorkflow).toHaveBeenCalledTimes(3); - expect(mocks.watchConfig).not.toHaveBeenCalled(); + expect(mocks.startConfigReloading).not.toHaveBeenCalled(); expect(signals.size).toBe(0); finishListening(); - await vi.waitFor(() => expect(mocks.watchConfig).toHaveBeenCalledOnce()); + await vi.waitFor(() => expect(mocks.startConfigReloading).toHaveBeenCalledOnce()); expect(signals.size).toBe(2); } ); +test('reconciliation keeps the original selected path and loading context', async () => { + const cwd = process.cwd(); + mocks.runMigrations.mockImplementation(async () => { + vi.stubEnv('CONFIG_PATH', 'other.yaml'); + return []; + }); + + await boot(); + await monitoringReady; + + expect(mocks.resolveConfigPath).toHaveBeenCalledOnce(); + expect(reloadOptions.path).toBe(resolve(cwd, 'when.yaml')); + expect(mocks.loadConfigFile).toHaveBeenCalledTimes(2); + for (const [path, options] of mocks.loadConfigFile.mock.calls) { + expect(path).toBe('when.yaml'); + expect(options.cwd).toBe(cwd); + expect(options.env.WHEN_WORKER_CONFIG_TEST).toBe('synthetic-context'); + } + expect(mocks.loadConfigFile.mock.calls[0][1] === mocks.loadConfigFile.mock.calls[1][1]).toBe( + true + ); +}); + +test('reconciliation updates the shared context and health config without rebuilding resources', async () => { + const next = { ...config, user: { ...config.user, name: 'Updated owner' } }; + mocks.loadConfigFile.mockResolvedValueOnce(config).mockResolvedValueOnce(next); + + await boot(); + await monitoringReady; + + const ctx = mocks.setWorkerContext.mock.calls[0][0]; + const getConfig = mocks.createHealthServer.mock.calls[0][0] as () => WhenConfiguration; + expect(ctx.config).toBe(next); + expect(getConfig()).toBe(next); + expect(mocks.createMailer).toHaveBeenCalledOnce(); + expect(mocks.openDb).toHaveBeenCalledOnce(); + expect(mocks.initOpenWorkflow).toHaveBeenCalledOnce(); + expect(process.exit).not.toHaveBeenCalled(); +}); + +test('an invalid reconciliation keeps the usable config and later soft edits still apply', async () => { + const failure = new Error('invalid configuration'); + mocks.loadConfigFile.mockResolvedValueOnce(config).mockRejectedValueOnce(failure); + + await boot(); + await monitoringReady; + + const getConfig = mocks.createHealthServer.mock.calls[0][0] as () => WhenConfiguration; + expect(getConfig()).toBe(config); + expect(mocks.logger.error).toHaveBeenCalledWith( + { err: failure }, + 'config reload failed; keeping current config' + ); + const next = { ...config, user: { ...config.user, name: 'Corrected owner' } }; + expect(reloadOptions.onChange(next, config)).toBe(true); + expect(getConfig()).toBe(next); + expect(process.exit).not.toHaveBeenCalled(); +}); + +test('a restart during initial reconciliation already owns watcher cleanup and signals', async () => { + const next = { ...config, database: { ...config.database, app: '/new.sqlite' } }; + mocks.loadConfigFile.mockResolvedValueOnce(config).mockResolvedValueOnce(next); + + await import('./index.js'); + await vi.waitFor(() => expect(process.exit).toHaveBeenCalledWith(0)); + + expect(signals.has('SIGTERM')).toBe(true); + expect(signals.has('SIGINT')).toBe(true); + expect(mocks.stopWatch).toHaveBeenCalledOnce(); + expect(mocks.stopWatch).toHaveBeenCalledBefore(server.close); + expect(mocks.stopWatch).toHaveBeenCalledBefore(mocks.stop); + expect(mocks.stop).toHaveBeenCalledBefore(mocks.closeOpenWorkflow); + expect(mocks.closeOpenWorkflow).toHaveBeenCalledBefore(mocks.destroy); + const getConfig = mocks.createHealthServer.mock.calls[0][0] as () => WhenConfiguration; + expect(getConfig()).toBe(config); +}); + +test('SIGTERM during pending reconciliation stops monitoring before cleanup and suppresses late apply', async () => { + const pending = Promise.withResolvers(); + mocks.loadConfigFile.mockResolvedValueOnce(config).mockReturnValueOnce(pending.promise); + await boot(); + await vi.waitFor(() => expect(mocks.loadConfigFile).toHaveBeenCalledTimes(2)); + + signals.get('SIGTERM')!(); + await vi.waitFor(() => expect(process.exit).toHaveBeenCalledWith(0)); + expect(mocks.stopWatch).toHaveBeenCalledBefore(server.close); + expect(mocks.stopWatch).toHaveBeenCalledBefore(mocks.stop); + + pending.resolve({ ...config, user: { ...config.user, name: 'Too late' } }); + await monitoringReady; + const getConfig = mocks.createHealthServer.mock.calls[0][0] as () => WhenConfiguration; + expect(getConfig()).toBe(config); + expect(mocks.logger.info).not.toHaveBeenCalledWith('config reloaded'); +}); + test('signals and restart-required config changes share one safe resource shutdown', async () => { await boot(); const stopped = Promise.withResolvers(); @@ -247,9 +368,12 @@ test('signals and restart-required config changes share one safe resource shutdo return server; }); events = []; + const next = { ...config, database: { ...config.database, app: '/new.sqlite' } }; + expect(reloadOptions.onChange(next, config)).toBe(false); + const getConfig = mocks.createHealthServer.mock.calls[0][0] as () => WhenConfiguration; + expect(getConfig()).toBe(config); signals.get('SIGTERM')!(); signals.get('SIGINT')!(); - reload({ ok: true, config: { ...config, database: { ...config.database, app: '/new.sqlite' } } }); expect(events).toEqual(['watch:stop', 'server:close', 'worker:stop']); expect(mocks.closeOpenWorkflow).not.toHaveBeenCalled(); expect(mocks.destroy).not.toHaveBeenCalled(); @@ -283,7 +407,7 @@ test('signals and restart-required config changes share one safe resource shutdo } }); -test.each(['migration', 'start', 'enqueue', 'listen'] as const)( +test.each(['migration', 'start', 'enqueue', 'listen', 'watch'] as const)( '%s failure awaits cleanup of partially created resources before exiting', async (stage) => { const failure = new Error(`${stage} failed`); @@ -296,6 +420,11 @@ test.each(['migration', 'start', 'enqueue', 'listen'] as const)( return server; }); } + if (stage === 'watch') { + mocks.startConfigReloading.mockImplementation(() => { + throw failure; + }); + } const databaseClosed = Promise.withResolvers(); mocks.destroy.mockImplementation(async () => { events.push('db:close'); @@ -304,7 +433,13 @@ test.each(['migration', 'start', 'enqueue', 'listen'] as const)( await import('./index.js'); await vi.waitFor(() => expect(mocks.destroy).toHaveBeenCalledOnce()); expect(process.exit).not.toHaveBeenCalled(); - expect(mocks.watchConfig).not.toHaveBeenCalled(); + if (stage === 'watch') { + expect(mocks.startConfigReloading).toHaveBeenCalledOnce(); + expect(mocks.stopWatch).not.toHaveBeenCalled(); + expect(server.close).toHaveBeenCalledOnce(); + } else { + expect(mocks.startConfigReloading).not.toHaveBeenCalled(); + } expect(signals.size).toBe(0); if (stage === 'migration') { expect(mocks.initOpenWorkflow).not.toHaveBeenCalled(); @@ -314,7 +449,9 @@ test.each(['migration', 'start', 'enqueue', 'listen'] as const)( expect(mocks.stop).toHaveBeenCalledBefore(mocks.closeOpenWorkflow); expect(mocks.closeOpenWorkflow).toHaveBeenCalledBefore(mocks.destroy); } - if (stage !== 'listen') expect(mocks.createHealthServer).not.toHaveBeenCalled(); + if (stage !== 'listen' && stage !== 'watch') { + expect(mocks.createHealthServer).not.toHaveBeenCalled(); + } databaseClosed.resolve(); await vi.waitFor(() => expect(process.exit).toHaveBeenCalledWith(1)); expect(mocks.logger.error).toHaveBeenCalledWith( @@ -330,10 +467,8 @@ test.each(['SIGTERM', 'SIGINT', 'config-change'])( await boot(); mocks.stop.mockRejectedValue(new Error('active work did not stop')); if (trigger === 'config-change') { - reload({ - ok: true, - config: { ...config, smtp: { ...config.smtp, host: 'new.example.test' } } - }); + const next = { ...config, smtp: { ...config.smtp, host: 'new.example.test' } }; + expect(reloadOptions.onChange(next, config)).toBe(false); } else { signals.get(trigger)!(); } diff --git a/apps/worker/src/index.ts b/apps/worker/src/index.ts index 84015293..0bb7e1d2 100644 --- a/apps/worker/src/index.ts +++ b/apps/worker/src/index.ts @@ -1,4 +1,5 @@ -import { loadConfig } from '@when/config'; +import { resolve } from 'node:path'; +import { loadConfigFile, resolveConfigPath } from '@when/config'; import { openDb } from '@when/db'; import { closeOpenWorkflow } from '@when/jobs'; import { logger } from './services/logger.js'; @@ -12,7 +13,10 @@ import { } from './services/startup.js'; async function main(): Promise { - const config = await loadConfig(); + const options = { env: process.env, cwd: process.cwd() }; + const selectedPath = resolveConfigPath(options); + const loadConfiguration = () => loadConfigFile(selectedPath, options); + const config = await loadConfiguration(); logger.info({ user: config.user.email }, 'config loaded'); const db = openDb(config.database.app); @@ -27,7 +31,10 @@ async function main(): Promise { const client = await startWorkflowWorker(ctx, resources); await enqueueStartupMaintenance(client); await startHealthServer(ctx, resources); - watchConfigAndSignals(ctx, resources, shutdown); + await watchConfigAndSignals(ctx, resources, shutdown, { + path: resolve(options.cwd, selectedPath), + load: loadConfiguration + }); } catch (error) { try { await shutdown(); diff --git a/apps/worker/src/services/config-reload.test.ts b/apps/worker/src/services/config-reload.test.ts index 7e3f5032..0324316e 100644 --- a/apps/worker/src/services/config-reload.test.ts +++ b/apps/worker/src/services/config-reload.test.ts @@ -22,12 +22,14 @@ function fakeCtx(config: WhenConfiguration): WorkerContext { } test('applies a soft change in place without restarting', () => { - const ctx = fakeCtx(cfg({ user: { name: 'Old' } })); + const previous = cfg({ user: { name: 'Old' } }); + const ctx = fakeCtx(previous); const next = cfg({ user: { name: 'New' } }); const onRestart = vi.fn(); - applyConfig({ ok: true, config: next }, ctx, onRestart); + const accepted = applyConfig(next, previous, ctx, onRestart); + expect(accepted).toBe(true); expect(ctx.config).toBe(next); expect(onRestart).not.toHaveBeenCalled(); }); @@ -38,29 +40,22 @@ test('restarts when a restart-required key (smtp) changes', () => { const next = cfg({ smtp: { host: 'new.example.com', port: 587, username: 'u', password: 'p' } }); const onRestart = vi.fn(); - applyConfig({ ok: true, config: next }, ctx, onRestart); + const accepted = applyConfig(next, old, ctx, onRestart); + expect(accepted).toBe(false); expect(onRestart).toHaveBeenCalledOnce(); - expect(ctx.config).toBe(old); // not swapped — the reboot picks up the change + expect(ctx.config).toBe(old); }); -test('restarts when the database section changes', () => { - const ctx = fakeCtx(cfg()); +test('restarts when the database section changes without swapping config', () => { + const previous = cfg(); + const ctx = fakeCtx(previous); const next = cfg({ database: { app: '/other/app.sqlite', queue: '/data/queue.sqlite' } }); const onRestart = vi.fn(); - applyConfig({ ok: true, config: next }, ctx, onRestart); + const accepted = applyConfig(next, previous, ctx, onRestart); + expect(accepted).toBe(false); expect(onRestart).toHaveBeenCalledOnce(); -}); - -test('keeps the current config when the reload failed', () => { - const old = cfg(); - const ctx = fakeCtx(old); - const onRestart = vi.fn(); - - applyConfig({ ok: false, error: new Error('bad config') }, ctx, onRestart); - - expect(ctx.config).toBe(old); - expect(onRestart).not.toHaveBeenCalled(); + expect(ctx.config).toBe(previous); }); diff --git a/apps/worker/src/services/config-reload.ts b/apps/worker/src/services/config-reload.ts index 183bdd5e..fd84557c 100644 --- a/apps/worker/src/services/config-reload.ts +++ b/apps/worker/src/services/config-reload.ts @@ -1,20 +1,22 @@ -import { configChanged, type ReloadResult, type WhenConfiguration } from '@when/config'; +import { configChanged, type WhenConfiguration } from '@when/config'; import type { WorkerContext } from './context.js'; // Changes under these keys can't be hot-applied (the mailer and DB/queue // connections are built at boot), so a reboot picks them up instead. const RESTART_KEYS: (keyof WhenConfiguration)[] = ['smtp', 'database']; -export function applyConfig(result: ReloadResult, ctx: WorkerContext, onRestart: () => void): void { - if (!result.ok) { - ctx.logger.error({ err: result.error }, 'config reload failed; keeping current config'); - return; - } - if (configChanged(ctx.config, result.config, RESTART_KEYS)) { +export function applyConfig( + next: WhenConfiguration, + previous: WhenConfiguration, + ctx: WorkerContext, + onRestart: () => void +): boolean { + if (configChanged(previous, next, RESTART_KEYS)) { ctx.logger.warn('config change requires a restart'); onRestart(); - return; + return false; } - ctx.config = result.config; + ctx.config = next; ctx.logger.info('config reloaded'); + return true; } diff --git a/apps/worker/src/services/startup.ts b/apps/worker/src/services/startup.ts index e7ba3b61..963f134c 100644 --- a/apps/worker/src/services/startup.ts +++ b/apps/worker/src/services/startup.ts @@ -1,5 +1,9 @@ import { once } from 'node:events'; -import { watchConfig, type WhenConfiguration } from '@when/config'; +import { + startConfigReloading, + type ConfigReloadOptions, + type WhenConfiguration +} from '@when/config'; import { runMigrations } from '@when/db'; import { expirePendingAppointments, @@ -67,11 +71,12 @@ export async function startHealthServer( ctx.logger.info({ port }, 'health server listening'); } -export function watchConfigAndSignals( +export async function watchConfigAndSignals( ctx: WorkerContext, resources: ShutdownResources, - shutdown: () => Promise -): void { + shutdown: () => Promise, + source: Pick, 'path' | 'load'> +): Promise { const requestShutdown = (signal: string): void => { ctx.logger.info({ signal }, 'worker shutting down'); void shutdown().then( @@ -86,9 +91,17 @@ export function watchConfigAndSignals( ); }; - resources.stopWatch = watchConfig((result) => - applyConfig(result, ctx, () => requestShutdown('config-change')) - ); + const monitoring = startConfigReloading({ + ...source, + initial: ctx.config, + onChange: (next, previous) => + applyConfig(next, previous, ctx, () => requestShutdown('config-change')), + onError: (error) => { + ctx.logger.error({ err: error }, 'config reload failed; keeping current config'); + } + }); + resources.stopWatch = monitoring.stop; process.on('SIGTERM', () => requestShutdown('SIGTERM')); process.on('SIGINT', () => requestShutdown('SIGINT')); + await monitoring.ready; } diff --git a/docs/architecture.md b/docs/architecture.md index 8fc8c175..497db70e 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -9,16 +9,27 @@ This is the map. The code has the street names. Node 26, strict TypeScript and pnpm workspaces. SvelteKit serves web; `node:sqlite` and Kysely handle data. OpenWorkflow runs background jobs. -| Path | Job | -| --------------------- | --------------------------------------------- | -| `apps/web` | Booking, admin and HTTP routes. | -| `apps/worker` | Calendar I/O, video links and email. | -| `apps/cli` | Config validation and operator commands. | -| `packages/config` | TypeBox schema, YAML loading and validation. | -| `packages/db` | App database, queries and migrations. | -| `packages/jobs` | Workflow contracts and queue client. | -| `packages/calendar` | Google/CalDAV adapters and local ICS helpers. | -| `packages/video-chat` | Google Meet and Nextcloud Talk adapters. | +| Path | Job | +| --------------------- | ---------------------------------------------- | +| `apps/web` | Booking, admin and HTTP routes. | +| `apps/worker` | Calendar I/O, video links and email. | +| `apps/cli` | Config validation and operator commands. | +| `packages/config` | TypeBox schema, loading and reload monitoring. | +| `packages/db` | App database, queries and migrations. | +| `packages/jobs` | Workflow contracts and queue client. | +| `packages/calendar` | Google/CalDAV adapters and local ICS helpers. | +| `packages/video-chat` | Google Meet and Nextcloud Talk adapters. | + +## Configuration + +The config package validates When's YAML and provides a schema-independent reload +monitor. Each process selects its loading context once. The monitor serializes +reads and tracks the last accepted value; the app decides whether to apply an +edit or restart boot-time resources. Invalid edits keep the accepted configuration. + +The monitor returns cleanup before reconciling startup changes, so shutdown owns +it immediately. Web uses SvelteKit's shutdown hook; worker keeps its normal +resource shutdown order. There is no separate package-global configuration cache. ## A booking diff --git a/packages/config/src/compare.ts b/packages/config/src/compare.ts new file mode 100644 index 00000000..f6d9302d --- /dev/null +++ b/packages/config/src/compare.ts @@ -0,0 +1,10 @@ +import { isDeepStrictEqual } from 'node:util'; +import type { WhenConfiguration } from './schema.js'; + +export function configChanged( + previous: WhenConfiguration, + next: WhenConfiguration, + keys: (keyof WhenConfiguration)[] +): boolean { + return keys.some((key) => !isDeepStrictEqual(previous[key], next[key])); +} diff --git a/packages/config/src/database.test.ts b/packages/config/src/database.test.ts index ce7c86ca..485f2fec 100644 --- a/packages/config/src/database.test.ts +++ b/packages/config/src/database.test.ts @@ -8,7 +8,7 @@ import { validConfig } from './__fixtures__/valid-config.js'; test('database paths default when the section is omitted', () => { const { database: _omit, ...withoutDatabase } = validConfig; - const config = validateConfig(withoutDatabase); + const config = validateConfig(withoutDatabase, { env: {} }); expect(config.database).toEqual({ app: './data/when.sqlite', queue: './data/openworkflow.sqlite' @@ -16,35 +16,52 @@ test('database paths default when the section is omitted', () => { }); let dir: string; -const saved = { ...process.env }; beforeEach(async () => { dir = await mkdtemp(join(tmpdir(), 'when-config-')); - delete process.env.WHEN_DATABASE_PATH; - delete process.env.WHEN_QUEUE_DB_PATH; }); afterEach(async () => { - process.env = { ...saved }; await rm(dir, { recursive: true, force: true }); }); -test('loadConfigFile resolves relative db paths against the deployment root (config dir parent)', async () => { - const configDir = join(dir, 'config'); - await mkdir(configDir); - const path = join(configDir, 'when.yaml'); - await writeFile(path, stringify(validConfig)); - const config = await loadConfigFile(path); +test('an explicit relative file is read from cwd and resolves db paths against the deployment root', async () => { + await mkdir(join(dir, 'config')); + await writeFile(join(dir, 'config', 'when.yaml'), stringify(validConfig)); + const config = await loadConfigFile('config/when.yaml', { + cwd: dir, + env: { CONFIG_PATH: 'not-the-selected-file.yaml' } + }); expect(config.database.app).toBe(join(dir, 'data', 'when.sqlite')); expect(config.database.queue).toBe(join(dir, 'data', 'openworkflow.sqlite')); }); -test('WHEN_DATABASE_PATH / WHEN_QUEUE_DB_PATH override the resolved paths', async () => { - process.env.WHEN_DATABASE_PATH = '/var/lib/when/app.sqlite'; - process.env.WHEN_QUEUE_DB_PATH = '/var/lib/when/jobs.sqlite'; +test('a bare CONFIG_PATH keeps data under cwd and uses the supplied interpolation environment', async () => { + const raw = { + ...validConfig, + smtp: { ...validConfig.smtp, password: '${SMTP_PASSWORD}' } + }; + await writeFile(join(dir, 'when.yaml'), stringify(raw)); + const config = await loadConfigFile(undefined, { + cwd: dir, + env: { CONFIG_PATH: 'when.yaml', SMTP_PASSWORD: 'context-password' } + }); + expect(config.smtp.password).toBe('context-password'); + expect(config.database.app).toBe(join(dir, 'data', 'when.sqlite')); + expect(config.database.queue).toBe(join(dir, 'data', 'openworkflow.sqlite')); + expect(config.meetings['30-min-chat'].daily_booking_limit).toBeNull(); +}); + +test.for([ + { app: '/var/lib/when/app.sqlite', queue: '/var/lib/when/jobs.sqlite' }, + { app: './app.sqlite', queue: './jobs.sqlite' }, + { app: '', queue: '' } +])('database environment overrides are returned unchanged: %j', async ({ app, queue }) => { const path = join(dir, 'config.yaml'); await writeFile(path, stringify(validConfig)); - const config = await loadConfigFile(path); - expect(config.database.app).toBe('/var/lib/when/app.sqlite'); - expect(config.database.queue).toBe('/var/lib/when/jobs.sqlite'); + const config = await loadConfigFile(path, { + cwd: dir, + env: { WHEN_DATABASE_PATH: app, WHEN_QUEUE_DB_PATH: queue } + }); + expect(config.database).toEqual({ app, queue }); }); diff --git a/packages/config/src/index.ts b/packages/config/src/index.ts index 6dc897c7..26f657da 100644 --- a/packages/config/src/index.ts +++ b/packages/config/src/index.ts @@ -6,8 +6,10 @@ export { ConfigError } from './load.js'; export type { ConfigIssue } from './load.js'; -export { loadConfig, reloadConfig, watchConfig, configChanged } from './store.js'; -export type { ReloadResult } from './store.js'; +export type { ConfigEnvironment, ConfigLoadOptions, ConfigPathOptions } from './options.js'; +export { startConfigReloading } from './monitor.js'; +export type { ConfigReloadOptions } from './monitor.js'; +export { configChanged } from './compare.js'; export { interpolate, MissingEnvVarsError } from './interpolate.js'; export { checkCrossRefs } from './cross-refs.js'; export { allCalendars, findCalendar, calendarNames, calendarTarget } from './calendars.js'; diff --git a/packages/config/src/interpolate.test.ts b/packages/config/src/interpolate.test.ts index 2c6cc4f0..f80360de 100644 --- a/packages/config/src/interpolate.test.ts +++ b/packages/config/src/interpolate.test.ts @@ -59,12 +59,9 @@ test('uses ${VAR:-default} fallback when the var is unset', () => { expect(result).toEqual({ url: 'http://localhost:8080' }); }); -test('prefers the env value over a ${VAR:-default} fallback', () => { - const result = interpolate( - { url: '${APP_URL:-http://localhost:8080}' }, - { APP_URL: 'https://x' } - ); - expect(result).toEqual({ url: 'https://x' }); +test.for(['https://x', ''])('set env values override fallbacks, even empty: %s', (value) => { + const result = interpolate({ url: '${APP_URL:-http://localhost:8080}' }, { APP_URL: value }); + expect(result).toEqual({ url: value }); }); test('${VAR:-} yields an empty string when unset (no throw)', () => { diff --git a/packages/config/src/interpolate.ts b/packages/config/src/interpolate.ts index 9485c7be..99a2f2f7 100644 --- a/packages/config/src/interpolate.ts +++ b/packages/config/src/interpolate.ts @@ -1,3 +1,5 @@ +import type { ConfigEnvironment } from './options.js'; + const ENV_REF = /\$\$|\$\{([A-Z_][A-Z0-9_]*)(?::-([^}]*))?\}/g; export class MissingEnvVarsError extends Error { @@ -9,7 +11,7 @@ export class MissingEnvVarsError extends Error { } } -export function interpolate(node: T, env: NodeJS.ProcessEnv = process.env): T { +export function interpolate(node: T, env: ConfigEnvironment = process.env): T { const missing = new Set(); const result = walk(node, env, missing); if (missing.size > 0) { @@ -18,7 +20,7 @@ export function interpolate(node: T, env: NodeJS.ProcessEnv = process.env): T return result as T; } -function walk(node: unknown, env: NodeJS.ProcessEnv, missing: Set): unknown { +function walk(node: unknown, env: ConfigEnvironment, missing: Set): unknown { if (typeof node === 'string') { return node.replace(ENV_REF, (match, name: string, fallback: string | undefined) => { if (match === '$$') return '$'; diff --git a/packages/config/src/load.test.ts b/packages/config/src/load.test.ts index 7ca6fcb0..4e272ae7 100644 --- a/packages/config/src/load.test.ts +++ b/packages/config/src/load.test.ts @@ -1,8 +1,15 @@ import { expect, test } from 'vitest'; -import { mkdtemp, writeFile, rm } from 'node:fs/promises'; +import { mkdir, mkdtemp, writeFile, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { ConfigError, loadConfigFile, validateConfig, validateStructure } from './load.js'; +import { stringify } from 'yaml'; +import { + ConfigError, + loadConfigFile, + loadConfigFileStructure, + validateConfig, + validateStructure +} from './load.js'; import { MissingEnvVarsError } from './interpolate.js'; import { validConfig } from './__fixtures__/valid-config.js'; import type { WhenConfiguration } from './schema.js'; @@ -53,19 +60,44 @@ test('an unknown version is rejected', () => { test('validateStructure accepts unset secret env refs that validateConfig rejects', () => { const raw = clone(validConfig) as WhenConfiguration; raw.auth = { credentials: { username: 'admin', password: '${WHEN_UNSET_PW_TEST}' } }; - const prev = process.env.WHEN_UNSET_PW_TEST; - delete process.env.WHEN_UNSET_PW_TEST; + expect(() => validateConfig(raw, { env: {} })).toThrow(MissingEnvVarsError); + const cfg = validateStructure(raw); + expect(cfg.auth).toEqual({ + credentials: { username: 'admin', password: '${WHEN_UNSET_PW_TEST}' } + }); +}); + +test('structural file loading uses context for selection, not interpolation or database overrides', async () => { + const directory = await mkdtemp(join(tmpdir(), 'when-config-structure-')); + const raw = { + ...validConfig, + auth: { credentials: { username: 'admin', password: '${WHEN_ADMIN_PASSWORD}' } }, + database: { app: '${DB_PATH}', queue: './jobs.sqlite' } + }; + const options = { + cwd: directory, + env: { + CONFIG_PATH: 'config/when.yaml', + WHEN_ADMIN_PASSWORD: 'context-password', + DB_PATH: './resolved.sqlite', + WHEN_DATABASE_PATH: '/override.sqlite', + WHEN_QUEUE_DB_PATH: '/override-jobs.sqlite' + } + }; try { - // full validation interpolates and fails on the missing var... - expect(() => validateConfig(clone(raw))).toThrow(MissingEnvVarsError); - // ...but structural validation leaves the ref intact and passes. - const cfg = validateStructure(clone(raw)); - expect(cfg.auth).toEqual({ - credentials: { username: 'admin', password: '${WHEN_UNSET_PW_TEST}' } + await mkdir(join(directory, 'config')); + await writeFile(join(directory, 'config', 'when.yaml'), stringify(raw)); + const structural = await loadConfigFileStructure(undefined, options); + expect(structural.auth.credentials?.password).toBe('${WHEN_ADMIN_PASSWORD}'); + expect(structural.database).toEqual(raw.database); + const runtime = await loadConfigFile(undefined, options); + expect(runtime.auth.credentials?.password).toBe('context-password'); + expect(runtime.database).toEqual({ + app: '/override.sqlite', + queue: '/override-jobs.sqlite' }); } finally { - if (prev === undefined) delete process.env.WHEN_UNSET_PW_TEST; - else process.env.WHEN_UNSET_PW_TEST = prev; + await rm(directory, { recursive: true, force: true }); } }); @@ -145,20 +177,12 @@ test.for([ test('structural session-secret references stay unresolved; runtime checks the resolved length', () => { const raw = { ...clone(validConfig), auth: { ...validConfig.auth, secret: '${AUTH_SECRET}' } }; - const previous = process.env.AUTH_SECRET; - delete process.env.AUTH_SECRET; - try { - expect(validateStructure(raw).auth.secret).toBe('${AUTH_SECRET}'); - expect(() => validateConfig(raw)).toThrow(MissingEnvVarsError); - process.env.AUTH_SECRET = 'a'.repeat(32); - expect(validateConfig(raw).auth.secret).toBe(process.env.AUTH_SECRET); - for (const value of ['', 'a'.repeat(31)]) { - process.env.AUTH_SECRET = value; - expect(() => validateConfig(raw)).toThrow(ConfigError); - } - } finally { - if (previous === undefined) delete process.env.AUTH_SECRET; - else process.env.AUTH_SECRET = previous; + expect(validateStructure(raw).auth.secret).toBe('${AUTH_SECRET}'); + expect(() => validateConfig(raw, { env: {} })).toThrow(MissingEnvVarsError); + const secret = 'a'.repeat(32); + expect(validateConfig(raw, { env: { AUTH_SECRET: secret } }).auth.secret).toBe(secret); + for (const value of ['', 'a'.repeat(31)]) { + expect(() => validateConfig(raw, { env: { AUTH_SECRET: value } })).toThrow(ConfigError); } }); @@ -297,34 +321,19 @@ test('video_chat rejects invalid value', () => { test('env vars are interpolated before validation', () => { const raw = clone(validConfig) as WhenConfiguration; raw.auth = { credentials: { username: 'admin', password: '${ADMIN_PW_TEST}' } }; - // Stash and restore process.env for the test - const prev = process.env.ADMIN_PW_TEST; - process.env.ADMIN_PW_TEST = 'real-password'; - try { - const cfg = validateConfig(raw); - expect(cfg.auth).toEqual({ - credentials: { username: 'admin', password: 'real-password' } - }); - } finally { - if (prev === undefined) delete process.env.ADMIN_PW_TEST; - else process.env.ADMIN_PW_TEST = prev; - } + const cfg = validateConfig(raw, { env: { ADMIN_PW_TEST: 'real-password' } }); + expect(cfg.auth).toEqual({ + credentials: { username: 'admin', password: 'real-password' } + }); }); test('password defaults to WHEN_ADMIN_PASSWORD if omitted', () => { const raw = clone(validConfig) as unknown as { auth: { credentials: { password?: string } } }; delete raw.auth.credentials.password; - const prev = process.env.WHEN_ADMIN_PASSWORD; - process.env.WHEN_ADMIN_PASSWORD = 'defaulted-password'; - try { - const cfg = validateConfig(raw); - expect(cfg.auth).toEqual({ - credentials: { username: 'admin', password: 'defaulted-password' } - }); - } finally { - if (prev === undefined) delete process.env.WHEN_ADMIN_PASSWORD; - else process.env.WHEN_ADMIN_PASSWORD = prev; - } + const cfg = validateConfig(raw, { env: { WHEN_ADMIN_PASSWORD: 'defaulted-password' } }); + expect(cfg.auth).toEqual({ + credentials: { username: 'admin', password: 'defaulted-password' } + }); }); test('smtp.port defaults to 587 when omitted', () => { @@ -371,27 +380,15 @@ test('meeting require_approval defaults to true when omitted', () => { test('user timezone defaults to the TZ env var when omitted', () => { const raw = clone(validConfig) as unknown as { user: { timezone?: string } }; delete raw.user.timezone; - const prev = process.env.TZ; - process.env.TZ = 'America/Chicago'; - try { - expect(validateConfig(raw).user.timezone).toBe('America/Chicago'); - } finally { - if (prev === undefined) delete process.env.TZ; - else process.env.TZ = prev; - } + expect(validateConfig(raw, { env: { TZ: 'America/Chicago' } }).user.timezone).toBe( + 'America/Chicago' + ); }); test('user timezone defaults to UTC when omitted and TZ is unset', () => { const raw = clone(validConfig) as unknown as { user: { timezone?: string } }; delete raw.user.timezone; - const prev = process.env.TZ; - delete process.env.TZ; - try { - expect(validateConfig(raw).user.timezone).toBe('UTC'); - } finally { - if (prev === undefined) delete process.env.TZ; - else process.env.TZ = prev; - } + expect(validateConfig(raw, { env: {} }).user.timezone).toBe('UTC'); }); test('a meeting must name its schedule and booking_calendar', () => { diff --git a/packages/config/src/load.ts b/packages/config/src/load.ts index f9965e3c..8aace362 100644 --- a/packages/config/src/load.ts +++ b/packages/config/src/load.ts @@ -8,6 +8,7 @@ import { interpolate } from './interpolate.js'; import { withDerivedDefaults } from './normalize.js'; import { checkCrossRefs } from './cross-refs.js'; import { resolveConfigPath, resolveDeploymentRoot } from './paths.js'; +import type { ConfigEnvironment, ConfigLoadOptions, ConfigPathOptions } from './options.js'; const { WhenConfigurationSchema } = schemas; type WhenConfiguration = schemas.WhenConfiguration; @@ -45,11 +46,11 @@ export class ConfigError extends Error { } } -export function validateConfig(raw: unknown): WhenConfiguration { +export function validateConfig(raw: unknown, options: ConfigLoadOptions = {}): WhenConfiguration { const withDefaults = structuredClone(raw) ?? {}; Value.Default(WhenConfigurationSchema, subschemas, withDefaults); - const interpolated = interpolate(withDefaults); + const interpolated = interpolate(withDefaults, options.env ?? process.env); const normalized = withDerivedDefaults(interpolated) as WhenConfiguration; const config = checkAgainstSchema(normalized); if (config.auth.secret !== undefined && config.auth.secret.length < 32) { @@ -87,8 +88,8 @@ function checkAgainstSchema(normalized: WhenConfiguration): WhenConfiguration { return normalized; } -async function readAndParse(path: string): Promise { - const source = await readFile(path, 'utf8'); +async function readAndParse(path: string, cwd: string): Promise { + const source = await readFile(resolve(cwd, path), 'utf8'); const lineCounter = new LineCounter(); try { return parseYaml(source, { lineCounter, prettyErrors: false, logLevel: 'error' }); @@ -103,25 +104,37 @@ async function readAndParse(path: string): Promise { } export async function loadConfigFile( - path: string = resolveConfigPath() + path?: string, + options: ConfigPathOptions = {} ): Promise { - const config = validateConfig(await readAndParse(path)); - resolveDatabasePaths(config, path); + const env = options.env ?? process.env; + const cwd = options.cwd ?? process.cwd(); + const selectedPath = path ?? resolveConfigPath({ env, cwd }); + const config = validateConfig(await readAndParse(selectedPath, cwd), { env }); + resolveDatabasePaths(config, selectedPath, cwd, env); return config; } // Read and structurally validate a config file without interpolating env refs. export async function loadConfigFileStructure( - path: string = resolveConfigPath() + path?: string, + options: ConfigPathOptions = {} ): Promise { - return validateStructure(await readAndParse(path)); + const cwd = options.cwd ?? process.cwd(); + const selectedPath = path ?? resolveConfigPath(options); + return validateStructure(await readAndParse(selectedPath, cwd)); } // Resolve relative db paths against the deployment root, in place; env vars override. -function resolveDatabasePaths(config: WhenConfiguration, configPath: string): void { - const root = resolveDeploymentRoot(configPath); - config.database.app = process.env.WHEN_DATABASE_PATH ?? resolve(root, config.database.app); - config.database.queue = process.env.WHEN_QUEUE_DB_PATH ?? resolve(root, config.database.queue); +function resolveDatabasePaths( + config: WhenConfiguration, + configPath: string, + cwd: string, + env: ConfigEnvironment +): void { + const root = resolve(cwd, resolveDeploymentRoot(configPath)); + config.database.app = env.WHEN_DATABASE_PATH ?? resolve(root, config.database.app); + config.database.queue = env.WHEN_QUEUE_DB_PATH ?? resolve(root, config.database.queue); } function toIssue(err: ValueError): ConfigIssue { diff --git a/packages/config/src/monitor.integration.test.ts b/packages/config/src/monitor.integration.test.ts new file mode 100644 index 00000000..04872d1f --- /dev/null +++ b/packages/config/src/monitor.integration.test.ts @@ -0,0 +1,38 @@ +import { mkdtemp, readFile, rename, rm, stat, utimes, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { expect, test, vi } from 'vitest'; +import { startConfigReloading } from './monitor.js'; + +test('observes successive atomic saves even when modification time is preserved', async () => { + const directory = await mkdtemp(join(tmpdir(), 'when-monitor-')); + const path = join(directory, 'when.yaml'); + let accepted = 'initial'; + const errors: unknown[] = []; + await writeFile(path, accepted); + const original = await stat(path); + const monitor = startConfigReloading({ + path, + initial: accepted, + load: (path) => readFile(path, 'utf8'), + onChange(next) { + accepted = next; + return true; + }, + onError: (error) => errors.push(error) + }); + try { + await monitor.ready; + for (const next of ['first save', 'second save']) { + const temporary = join(directory, 'next.yaml'); + await writeFile(temporary, next); + await utimes(temporary, original.atime, original.mtime); + await rename(temporary, path); + await vi.waitFor(() => expect(accepted).toBe(next), { timeout: 4000, interval: 50 }); + } + expect(errors).toEqual([]); + } finally { + monitor.stop(); + await rm(directory, { recursive: true, force: true }); + } +}); diff --git a/packages/config/src/monitor.test.ts b/packages/config/src/monitor.test.ts new file mode 100644 index 00000000..40c53eae --- /dev/null +++ b/packages/config/src/monitor.test.ts @@ -0,0 +1,136 @@ +import type { Stats } from 'node:fs'; +import { beforeEach, expect, test, vi } from 'vitest'; +import { startConfigReloading } from './monitor.js'; + +type Listener = (current: Stats, previous: Stats) => Promise; +const filesystem = vi.hoisted(() => ({ + watchFile: vi.fn<(path: string, options: object, listener: Listener) => void>(), + unwatchFile: vi.fn() +})); +vi.mock('node:fs', () => filesystem); + +const path = '/synthetic/when.yaml'; + +function options() { + return { + path, + initial: 0, + load: vi.fn<(path: string) => Promise>().mockResolvedValue(1), + onChange: vi.fn<(next: number, previous: number) => boolean>().mockReturnValue(true), + onError: vi.fn<(error: unknown) => void>() + }; +} + +function listener(index = 0): Listener { + return filesystem.watchFile.mock.calls[index]![2]; +} + +function change(index = 0): Promise { + const previous = { ino: 10, mtimeMs: 100, ctimeMs: 100, size: 50 } as Stats; + const current = { ...previous, ctimeMs: 101 }; + return listener(index)(current, previous); +} + +beforeEach(() => vi.resetAllMocks()); + +test('returns cleanup before reading and waits for initial reconciliation', async () => { + const input = options(); + const read = Promise.withResolvers(); + input.load.mockReturnValueOnce(read.promise); + const monitor = startConfigReloading(input); + const ready = vi.fn(); + void monitor.ready.then(ready); + expect(input.load).not.toHaveBeenCalled(); + await Promise.resolve(); + expect(input.load).toHaveBeenCalledWith(path); + expect(ready).not.toHaveBeenCalled(); + read.resolve(1); + await monitor.ready; + expect(input.onChange).toHaveBeenCalledExactlyOnceWith(1, 0); + monitor.stop(); +}); + +test('can stop startup reconciliation before its scheduled read begins', async () => { + const input = options(); + const monitor = startConfigReloading(input); + monitor.stop(); + await monitor.ready; + expect(input.load).not.toHaveBeenCalled(); + expect(input.onChange).not.toHaveBeenCalled(); + expect(input.onError).not.toHaveBeenCalled(); +}); + +test('rejected edits and failed reads retain the accepted baseline until recovery', async () => { + const input = options(); + const monitor = startConfigReloading(input); + await monitor.ready; + + input.load.mockResolvedValueOnce(2); + input.onChange.mockReturnValueOnce(false); + await change(); + const failure = new Error('configuration unreadable'); + input.load.mockRejectedValueOnce(failure); + await change(); + input.load.mockResolvedValueOnce(3).mockResolvedValueOnce(4); + await change(); + await change(); + + expect(input.onChange.mock.calls).toEqual([ + [1, 0], + [2, 1], + [3, 1], + [4, 3] + ]); + expect(input.onError).toHaveBeenCalledExactlyOnceWith(failure); + monitor.stop(); +}); + +test('serializes a slow read and coalesces edits into one follow-up', async () => { + const input = options(); + const first = Promise.withResolvers(); + const second = Promise.withResolvers(); + const readingAgain = Promise.withResolvers(); + input.load.mockReturnValueOnce(first.promise).mockImplementationOnce(() => { + readingAgain.resolve(); + return second.promise; + }); + const monitor = startConfigReloading(input); + await Promise.resolve(); + for (let index = 0; index < 5; index++) void change(); + expect(input.load).toHaveBeenCalledOnce(); + first.resolve(1); + await readingAgain.promise; + expect(input.onChange).toHaveBeenCalledExactlyOnceWith(1, 0); + second.resolve(2); + await monitor.ready; + expect(input.load).toHaveBeenCalledTimes(2); + expect(input.onChange).toHaveBeenLastCalledWith(2, 1); + monitor.stop(); +}); + +test('stopping drops late and queued work without stopping another monitor on the path', async () => { + const first = options(); + const second = options(); + const firstMonitor = startConfigReloading(first); + const secondMonitor = startConfigReloading(second); + await Promise.all([firstMonitor.ready, secondMonitor.ready]); + + const read = Promise.withResolvers(); + first.load.mockReturnValueOnce(read.promise); + const pending = change(0); + await Promise.resolve(); + void change(0); + firstMonitor.stop(); + firstMonitor.stop(); + read.resolve(2); + await pending; + await change(0); + await change(1); + + expect(first.load).toHaveBeenCalledTimes(2); + expect(first.onChange).toHaveBeenCalledOnce(); + expect(first.onError).not.toHaveBeenCalled(); + expect(second.load).toHaveBeenCalledTimes(2); + expect(filesystem.unwatchFile).toHaveBeenCalledExactlyOnceWith(path, listener(0)); + secondMonitor.stop(); +}); diff --git a/packages/config/src/monitor.ts b/packages/config/src/monitor.ts new file mode 100644 index 00000000..e93c7529 --- /dev/null +++ b/packages/config/src/monitor.ts @@ -0,0 +1,78 @@ +import { watchFile, unwatchFile, type Stats } from 'node:fs'; + +export interface ConfigReloadOptions { + path: string; + initial: T; + load: (path: string) => Promise; + /** Return true only after accepting the candidate. */ + onChange: (next: T, previous: T) => boolean; + /** The caller owns safe diagnostics for raw failures. */ + onError: (error: unknown) => void; +} + +export function startConfigReloading({ + path, + initial, + load, + onChange, + onError +}: ConfigReloadOptions): { stop: () => void; ready: Promise } { + let accepted = initial; + let stopped = false; + let pending = false; + let activeReload: Promise | undefined; + + async function runPendingReloads(): Promise { + try { + while (!stopped && pending) { + pending = false; + try { + const next = await load(path); + if (stopped) return; + if (onChange(next, accepted) && !stopped) accepted = next; + } catch (error) { + if (stopped) return; + try { + onError(error); + } catch { + // A failed diagnostic must not stop subsequent reloads. + } + } + } + } finally { + activeReload = undefined; + } + } + + function scheduleReload(): Promise { + if (stopped) return Promise.resolve(); + pending = true; + // Defer reads so callers own cleanup before reconciliation can apply a change. + activeReload ??= Promise.resolve().then(runPendingReloads); + return activeReload; + } + + function listener(current: Stats, previous: Stats): Promise { + if ( + stopped || + (current.ino === previous.ino && + current.mtimeMs === previous.mtimeMs && + current.ctimeMs === previous.ctimeMs && + current.size === previous.size) + ) { + return Promise.resolve(); + } + return scheduleReload(); + } + + function stop(): void { + if (stopped) return; + stopped = true; + pending = false; + unwatchFile(path, listener); + } + + // Poll the path so atomic saves and deletion/recreation remain observable. + watchFile(path, { interval: 1000, persistent: false }, listener); + return { stop, ready: scheduleReload() }; +} diff --git a/packages/config/src/options.ts b/packages/config/src/options.ts new file mode 100644 index 00000000..f6e61995 --- /dev/null +++ b/packages/config/src/options.ts @@ -0,0 +1,9 @@ +export type ConfigEnvironment = Readonly>; + +export interface ConfigLoadOptions { + readonly env?: ConfigEnvironment; +} + +export interface ConfigPathOptions extends ConfigLoadOptions { + readonly cwd?: string; +} diff --git a/packages/config/src/paths.test.ts b/packages/config/src/paths.test.ts index 7d928374..127a44c2 100644 --- a/packages/config/src/paths.test.ts +++ b/packages/config/src/paths.test.ts @@ -4,38 +4,31 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { resolveConfigPath, resolveDeploymentRoot, resolvePublicDir } from './paths.js'; -const savedEnv = { ...process.env }; -const savedCwd = process.cwd(); let dir: string; beforeEach(async () => { - delete process.env.NODE_ENV; - delete process.env.CONFIG_PATH; dir = await mkdtemp(join(tmpdir(), 'when-paths-')); - process.chdir(dir); }); afterEach(async () => { - process.chdir(savedCwd); - process.env = { ...savedEnv }; await rm(dir, { recursive: true, force: true }); }); -test('CONFIG_PATH overrides everything', () => { - process.env.CONFIG_PATH = '/custom/where.yaml'; - process.env.NODE_ENV = 'production'; - expect(resolveConfigPath()).toBe('/custom/where.yaml'); +test.for(['/custom/where.yaml', 'when.yaml'])('CONFIG_PATH overrides everything: %s', (path) => { + const options = { cwd: dir, env: { CONFIG_PATH: path, NODE_ENV: 'production' } }; + expect(resolveConfigPath(options)).toBe(path); }); test('production defaults to the container config path', () => { - process.env.NODE_ENV = 'production'; - expect(resolveConfigPath()).toBe('/app/config/when.yaml'); + expect(resolveConfigPath({ cwd: dir, env: { NODE_ENV: 'production' } })).toBe( + '/app/config/when.yaml' + ); }); -test('dev uses config/when.yaml in the cwd when present', async () => { +test('dev uses config/when.yaml in the supplied cwd when present', async () => { await mkdir(join(dir, 'config'), { recursive: true }); await writeFile(join(dir, 'config', 'when.yaml'), ''); - expect(resolveConfigPath()).toBe(join(dir, 'config', 'when.yaml')); + expect(resolveConfigPath({ cwd: dir, env: {} })).toBe(join(dir, 'config', 'when.yaml')); }); test('dev falls back to the monorepo root config/when.yaml', async () => { @@ -43,24 +36,34 @@ test('dev falls back to the monorepo root config/when.yaml', async () => { await writeFile(join(dir, 'config', 'when.yaml'), ''); const cwd = join(dir, 'apps', 'web'); await mkdir(cwd, { recursive: true }); - process.chdir(cwd); - expect(resolveConfigPath()).toBe(join(dir, 'config', 'when.yaml')); + expect(resolveConfigPath({ cwd, env: {} })).toBe(join(dir, 'config', 'when.yaml')); }); test('dev falls back to cwd config/when.yaml when nothing exists', () => { - expect(resolveConfigPath()).toBe(join(dir, 'config', 'when.yaml')); + expect(resolveConfigPath({ cwd: dir, env: {} })).toBe(join(dir, 'config', 'when.yaml')); }); test('resolveDeploymentRoot is the config directory parent', () => { expect(resolveDeploymentRoot('/srv/when/config/when.yaml')).toBe('/srv/when'); }); -test('resolvePublicDir defaults to the sibling public dir under the deployment root', () => { - delete process.env.WHEN_PUBLIC_DIR; - expect(resolvePublicDir('/srv/when/config/when.yaml')).toBe(join('/srv/when', 'public')); -}); +test.for(['/srv/when/config/when.yaml', 'config/when.yaml', 'when.yaml'])( + 'public defaults stay under the deployment root for selected path %s', + (path) => { + const options = { cwd: '/srv/when', env: { CONFIG_PATH: path } }; + expect(resolvePublicDir(undefined, options)).toBe('/srv/when/public'); + expect(resolvePublicDir(resolveConfigPath(options), options)).toBe('/srv/when/public'); + } +); -test('WHEN_PUBLIC_DIR overrides the resolved public dir', () => { - process.env.WHEN_PUBLIC_DIR = '/var/lib/when/public'; - expect(resolvePublicDir('/srv/when/config/when.yaml')).toBe('/var/lib/when/public'); -}); +test.for(['/var/lib/when/public', './custom-public', ''])( + 'WHEN_PUBLIC_DIR overrides the resolved public dir without normalization: %s', + (publicDir) => { + expect( + resolvePublicDir('/srv/when/config/when.yaml', { + cwd: dir, + env: { WHEN_PUBLIC_DIR: publicDir } + }) + ).toBe(publicDir); + } +); diff --git a/packages/config/src/paths.ts b/packages/config/src/paths.ts index 9e726d2c..8865bd1f 100644 --- a/packages/config/src/paths.ts +++ b/packages/config/src/paths.ts @@ -1,25 +1,35 @@ import { existsSync } from 'node:fs'; import { dirname, join, resolve } from 'node:path'; +import type { ConfigPathOptions } from './options.js'; -export function resolveConfigPath(): string { - if (process.env.CONFIG_PATH) return process.env.CONFIG_PATH; - if (process.env.NODE_ENV === 'production') return '/app/config/when.yaml'; +export function resolveConfigPath(options: ConfigPathOptions = {}): string { + const env = options.env ?? process.env; + const cwd = options.cwd ?? process.cwd(); + if (env.CONFIG_PATH) return env.CONFIG_PATH; + if (env.NODE_ENV === 'production') return '/app/config/when.yaml'; - const local = join(process.cwd(), 'config', 'when.yaml'); + const local = join(cwd, 'config', 'when.yaml'); if (existsSync(local)) return local; - const root = join(process.cwd(), '..', '..', 'config', 'when.yaml'); + const root = join(cwd, '..', '..', 'config', 'when.yaml'); if (existsSync(root)) return root; return local; } // config/, data/, and public/ are siblings under the deployment root, so it is the config dir's parent. -export function resolveDeploymentRoot(configPath: string = resolveConfigPath()): string { - return dirname(dirname(configPath)); +export function resolveDeploymentRoot( + configPath?: string, + options: ConfigPathOptions = {} +): string { + // Keep the selected representation: a bare filename historically gives '.', not cwd's parent. + return dirname(dirname(configPath ?? resolveConfigPath(options))); } // Server-only: reaches process.env/fs — client code must `import type`, never call this. -export function resolvePublicDir(configPath: string = resolveConfigPath()): string { - return process.env.WHEN_PUBLIC_DIR ?? resolve(resolveDeploymentRoot(configPath), 'public'); +export function resolvePublicDir(configPath?: string, options: ConfigPathOptions = {}): string { + const env = options.env ?? process.env; + const cwd = options.cwd ?? process.cwd(); + const selectedPath = configPath ?? resolveConfigPath({ env, cwd }); + return env.WHEN_PUBLIC_DIR ?? resolve(cwd, resolveDeploymentRoot(selectedPath), 'public'); } diff --git a/packages/config/src/store.test.ts b/packages/config/src/store.test.ts deleted file mode 100644 index 3feaeff6..00000000 --- a/packages/config/src/store.test.ts +++ /dev/null @@ -1,93 +0,0 @@ -import { afterEach, beforeEach, expect, test, vi } from 'vitest'; -import { mkdtemp, rm, utimes, writeFile } from 'node:fs/promises'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; -import { stringify } from 'yaml'; -import { - loadConfig, - reloadConfig, - watchConfig, - configChanged, - type ReloadResult -} from './store.js'; -import { validConfig } from './__fixtures__/valid-config.js'; - -let dir: string; -let path: string; -const stops: Array<() => void> = []; - -function renamed(name: string): typeof validConfig { - return { ...validConfig, user: { ...validConfig.user, name } }; -} - -async function write(config: unknown, bumpMtime = false): Promise { - await writeFile(path, stringify(config)); - if (bumpMtime) { - const future = new Date(Date.now() + 2000); - await utimes(path, future, future); - } -} - -beforeEach(async () => { - dir = await mkdtemp(join(tmpdir(), 'when-store-')); - path = join(dir, 'config.yaml'); - await write(validConfig); -}); - -afterEach(async () => { - while (stops.length) stops.pop()?.(); - await rm(dir, { recursive: true, force: true }); -}); - -test('loadConfig returns the cached config for the same path', async () => { - const first = await loadConfig(path); - await write(renamed('Changed')); // a cached loader must not observe this - const second = await loadConfig(path); - expect(second).toBe(first); - expect(second.user.name).toBe(validConfig.user.name); -}); - -test('reloadConfig picks up a valid change and swaps the cache', async () => { - await loadConfig(path); - await write(renamed('Renamed')); - const result = await reloadConfig(); - expect(result.ok).toBe(true); - if (result.ok) expect(result.config.user.name).toBe('Renamed'); - const cached = await loadConfig(path); - expect(cached.user.name).toBe('Renamed'); -}); - -test('reloadConfig keeps the current config when the new file is invalid', async () => { - const good = await loadConfig(path); - await write({}); // valid YAML, fails schema (missing required top-level keys) - const result = await reloadConfig(); - expect(result.ok).toBe(false); - const cached = await loadConfig(path); - expect(cached).toBe(good); -}); - -test('watchConfig invokes the callback with a reload result on change', async () => { - await loadConfig(path); - const results: ReloadResult[] = []; - stops.push(watchConfig((r) => results.push(r))); - await write(renamed('Watched'), true); - await vi.waitFor(() => expect(results.length).toBeGreaterThan(0), { - timeout: 5000, - interval: 200 - }); - const last = results.at(-1); - expect(last?.ok).toBe(true); - if (last?.ok) expect(last.config.user.name).toBe('Watched'); -}); - -test('configChanged flags a changed subtree and ignores key order', () => { - const changedSmtp = { ...validConfig, smtp: { ...validConfig.smtp, host: 'new.example.com' } }; - expect(configChanged(validConfig, changedSmtp, ['smtp'])).toBe(true); - expect(configChanged(validConfig, changedSmtp, ['auth'])).toBe(false); - - const reordered = { - ...validConfig, - smtp: { password: 'secret', username: 'mailer', port: 587, host: 'smtp.example.com' } - }; - expect(configChanged(validConfig, reordered, ['smtp'])).toBe(false); -}); diff --git a/packages/config/src/store.ts b/packages/config/src/store.ts deleted file mode 100644 index b1ebeece..00000000 --- a/packages/config/src/store.ts +++ /dev/null @@ -1,59 +0,0 @@ -import { watchFile, unwatchFile, type Stats } from 'node:fs'; -import { isDeepStrictEqual } from 'node:util'; -import { loadConfigFile } from './load.js'; -import { resolveConfigPath } from './paths.js'; -import type { WhenConfiguration } from './schema.js'; - -let current: WhenConfiguration | null = null; -let currentPath: string | null = null; -let watchedPath: string | null = null; - -export async function loadConfig(path: string = resolveConfigPath()): Promise { - if (current && currentPath === path) return current; - current = await loadConfigFile(path); - currentPath = path; - return current; -} - -export type ReloadResult = { ok: true; config: WhenConfiguration } | { ok: false; error: unknown }; - -// On failure the cache is kept, so a broken edit never replaces a good config. -export async function reloadConfig(): Promise { - const path = currentPath ?? resolveConfigPath(); - try { - const config = await loadConfigFile(path); - current = config; - currentPath = path; - return { ok: true, config }; - } catch (error) { - return { ok: false, error }; - } -} - -// Polls (stat) rather than inotify: editors save when.yaml via atomic rename, -// which swaps the inode so a file-level fs.watch misses every edit after the -// first. Polling watches the path, not the inode, so it survives the swap. -export function watchConfig(onReload: (result: ReloadResult) => void): () => void { - const path = currentPath ?? resolveConfigPath(); - if (watchedPath === path) return stopWatch; - stopWatch(); - watchedPath = path; - watchFile(path, { interval: 1000 }, (curr: Stats, prev: Stats) => { - if (curr.mtimeMs === prev.mtimeMs) return; - void reloadConfig().then(onReload); - }); - return stopWatch; -} - -function stopWatch(): void { - if (watchedPath) unwatchFile(watchedPath); - watchedPath = null; -} - -export function configChanged( - a: WhenConfiguration, - b: WhenConfiguration, - keys: (keyof WhenConfiguration)[] -): boolean { - return keys.some((key) => !isDeepStrictEqual(a[key], b[key])); -}