fix: centralize strict config loading and fail fast on invalid config (#280)
This commit is contained in:
@@ -9,8 +9,8 @@
|
||||
*/
|
||||
|
||||
// Config loaded from lettabot.yaml
|
||||
import { loadConfig, applyConfigToEnv, serverModeLabel } from './config/index.js';
|
||||
const config = loadConfig();
|
||||
import { loadAppConfigOrExit, applyConfigToEnv, serverModeLabel } from './config/index.js';
|
||||
const config = loadAppConfigOrExit();
|
||||
applyConfigToEnv(config);
|
||||
import { existsSync, readFileSync, writeFileSync } from 'node:fs';
|
||||
import { dirname, resolve } from 'node:path';
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
*/
|
||||
|
||||
import * as p from '@clack/prompts';
|
||||
import { loadConfig, saveConfig, resolveConfigPath } from '../config/index.js';
|
||||
import { loadAppConfigOrExit, saveConfig, resolveConfigPath } from '../config/index.js';
|
||||
import {
|
||||
CHANNELS,
|
||||
getChannelHint,
|
||||
@@ -46,7 +46,7 @@ function getChannelDetails(id: ChannelId, channelConfig: any): string | undefine
|
||||
}
|
||||
|
||||
function getChannelStatus(): ChannelStatus[] {
|
||||
const config = loadConfig();
|
||||
const config = loadAppConfigOrExit();
|
||||
|
||||
return CHANNELS.map(ch => {
|
||||
const channelConfig = config.channels[ch.id as keyof typeof config.channels];
|
||||
@@ -202,7 +202,7 @@ export async function addChannel(channelId?: string): Promise<void> {
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
const config = loadConfig();
|
||||
const config = loadAppConfigOrExit();
|
||||
const existingConfig = config.channels[channelId as keyof typeof config.channels];
|
||||
|
||||
// Get and run the setup function
|
||||
@@ -230,7 +230,7 @@ export async function removeChannel(channelId?: string): Promise<void> {
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
const config = loadConfig();
|
||||
const config = loadAppConfigOrExit();
|
||||
const channelConfig = config.channels[channelId as keyof typeof config.channels];
|
||||
|
||||
if (!channelConfig?.enabled) {
|
||||
|
||||
@@ -10,8 +10,8 @@
|
||||
*/
|
||||
|
||||
// Config loaded from lettabot.yaml
|
||||
import { loadConfig, applyConfigToEnv } from '../config/index.js';
|
||||
const config = loadConfig();
|
||||
import { loadAppConfigOrExit, applyConfigToEnv } from '../config/index.js';
|
||||
const config = loadAppConfigOrExit();
|
||||
applyConfigToEnv(config);
|
||||
|
||||
// Types
|
||||
|
||||
@@ -7,8 +7,8 @@
|
||||
*/
|
||||
|
||||
// Config loaded from lettabot.yaml
|
||||
import { loadConfig, applyConfigToEnv } from '../config/index.js';
|
||||
const config = loadConfig();
|
||||
import { loadAppConfigOrExit, applyConfigToEnv } from '../config/index.js';
|
||||
const config = loadAppConfigOrExit();
|
||||
applyConfigToEnv(config);
|
||||
import { fetchHistory, isValidLimit, parseFetchArgs } from './history-core.js';
|
||||
import { loadLastTarget } from './shared.js';
|
||||
|
||||
@@ -11,8 +11,8 @@
|
||||
*/
|
||||
|
||||
// Config loaded from lettabot.yaml
|
||||
import { loadConfig, applyConfigToEnv } from '../config/index.js';
|
||||
const config = loadConfig();
|
||||
import { loadAppConfigOrExit, applyConfigToEnv } from '../config/index.js';
|
||||
const config = loadAppConfigOrExit();
|
||||
applyConfigToEnv(config);
|
||||
import { existsSync, readFileSync } from 'node:fs';
|
||||
import { loadLastTarget } from './shared.js';
|
||||
|
||||
@@ -10,8 +10,8 @@
|
||||
*/
|
||||
|
||||
// Config loaded from lettabot.yaml
|
||||
import { loadConfig, applyConfigToEnv } from '../config/index.js';
|
||||
const config = loadConfig();
|
||||
import { loadAppConfigOrExit, applyConfigToEnv } from '../config/index.js';
|
||||
const config = loadAppConfigOrExit();
|
||||
applyConfigToEnv(config);
|
||||
import { loadLastTarget } from './shared.js';
|
||||
|
||||
|
||||
@@ -1,2 +1,3 @@
|
||||
export * from './types.js';
|
||||
export * from './io.js';
|
||||
export * from './runtime.js';
|
||||
|
||||
@@ -3,7 +3,7 @@ import { mkdtempSync, existsSync, readFileSync, writeFileSync, rmSync } from 'no
|
||||
import { join } from 'node:path';
|
||||
import { tmpdir } from 'node:os';
|
||||
import YAML from 'yaml';
|
||||
import { saveConfig, loadConfig, configToEnv, didLoadFail } from './io.js';
|
||||
import { saveConfig, loadConfig, loadConfigStrict, configToEnv, didLoadFail } from './io.js';
|
||||
import { normalizeAgents, DEFAULT_CONFIG } from './types.js';
|
||||
import type { LettaBotConfig } from './types.js';
|
||||
|
||||
@@ -375,3 +375,45 @@ describe('loadConfig deprecation warning for top-level api', () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('loadConfigStrict', () => {
|
||||
it('should throw on parse error and set didLoadFail', () => {
|
||||
const originalEnv = process.env.LETTABOT_CONFIG;
|
||||
const tmpDir = mkdtempSync(join(tmpdir(), 'lettabot-strict-test-'));
|
||||
const configPath = join(tmpDir, 'lettabot.yaml');
|
||||
|
||||
try {
|
||||
writeFileSync(configPath, 'server:\n api: port: 6702\n', 'utf-8');
|
||||
process.env.LETTABOT_CONFIG = configPath;
|
||||
|
||||
expect(() => loadConfigStrict()).toThrow();
|
||||
expect(didLoadFail()).toBe(true);
|
||||
} finally {
|
||||
process.env.LETTABOT_CONFIG = originalEnv;
|
||||
rmSync(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('should throw when both top-level api and server.api are present', () => {
|
||||
const originalEnv = process.env.LETTABOT_CONFIG;
|
||||
const tmpDir = mkdtempSync(join(tmpdir(), 'lettabot-strict-test-'));
|
||||
const configPath = join(tmpDir, 'lettabot.yaml');
|
||||
|
||||
try {
|
||||
writeFileSync(
|
||||
configPath,
|
||||
'server:\n mode: api\n api:\n host: 0.0.0.0\napi:\n port: 9090\n',
|
||||
'utf-8'
|
||||
);
|
||||
process.env.LETTABOT_CONFIG = configPath;
|
||||
|
||||
expect(() => loadConfigStrict()).toThrow(
|
||||
/both top-level `api` and `server\.api` are set/
|
||||
);
|
||||
expect(didLoadFail()).toBe(true);
|
||||
} finally {
|
||||
process.env.LETTABOT_CONFIG = originalEnv;
|
||||
rmSync(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -53,19 +53,11 @@ export function resolveConfigPath(): string {
|
||||
let _lastLoadFailed = false;
|
||||
export function didLoadFail(): boolean { return _lastLoadFailed; }
|
||||
|
||||
/**
|
||||
* Load config from YAML file
|
||||
*/
|
||||
export function loadConfig(): LettaBotConfig {
|
||||
_lastLoadFailed = false;
|
||||
const configPath = resolveConfigPath();
|
||||
function hasObject(value: unknown): value is Record<string, unknown> {
|
||||
return !!value && typeof value === 'object' && !Array.isArray(value);
|
||||
}
|
||||
|
||||
if (!existsSync(configPath)) {
|
||||
return { ...DEFAULT_CONFIG };
|
||||
}
|
||||
|
||||
try {
|
||||
const content = readFileSync(configPath, 'utf-8');
|
||||
function parseAndNormalizeConfig(content: string): LettaBotConfig {
|
||||
const parsed = YAML.parse(content) as Partial<LettaBotConfig>;
|
||||
|
||||
// Fix instantGroups: YAML parses large numeric IDs (e.g. Discord snowflakes)
|
||||
@@ -73,6 +65,14 @@ export function loadConfig(): LettaBotConfig {
|
||||
// Re-extract from document AST to preserve the original string representation.
|
||||
fixLargeGroupIds(content, parsed);
|
||||
|
||||
// Reject ambiguous API server configuration. During migration from top-level
|
||||
// `api` to `server.api`, having both can silently drop fields.
|
||||
if (hasObject(parsed.api) && hasObject(parsed.server) && hasObject(parsed.server.api)) {
|
||||
throw new Error(
|
||||
'Conflicting API config: both top-level `api` and `server.api` are set. Remove top-level `api` and keep only `server.api`.'
|
||||
);
|
||||
}
|
||||
|
||||
// Merge with defaults and canonicalize server mode.
|
||||
const merged = {
|
||||
...DEFAULT_CONFIG,
|
||||
@@ -96,14 +96,51 @@ export function loadConfig(): LettaBotConfig {
|
||||
}
|
||||
|
||||
return config;
|
||||
}
|
||||
|
||||
/**
|
||||
* Load config from YAML file
|
||||
*/
|
||||
export function loadConfig(): LettaBotConfig {
|
||||
_lastLoadFailed = false;
|
||||
const configPath = resolveConfigPath();
|
||||
|
||||
if (!existsSync(configPath)) {
|
||||
return { ...DEFAULT_CONFIG };
|
||||
}
|
||||
|
||||
try {
|
||||
const content = readFileSync(configPath, 'utf-8');
|
||||
return parseAndNormalizeConfig(content);
|
||||
} catch (err) {
|
||||
_lastLoadFailed = true;
|
||||
console.error(`[Config] Failed to parse ${configPath}:`, err);
|
||||
console.warn('[Config] Using default configuration. Check your YAML syntax.');
|
||||
console.error(`[Config] Failed to load ${configPath}:`, err);
|
||||
console.warn('[Config] Using default configuration. Check your YAML syntax and field locations.');
|
||||
return { ...DEFAULT_CONFIG };
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Strict config loader. Throws on invalid YAML/schema instead of silently
|
||||
* falling back to defaults.
|
||||
*/
|
||||
export function loadConfigStrict(): LettaBotConfig {
|
||||
_lastLoadFailed = false;
|
||||
const configPath = resolveConfigPath();
|
||||
|
||||
if (!existsSync(configPath)) {
|
||||
return { ...DEFAULT_CONFIG };
|
||||
}
|
||||
|
||||
try {
|
||||
const content = readFileSync(configPath, 'utf-8');
|
||||
return parseAndNormalizeConfig(content);
|
||||
} catch (err) {
|
||||
_lastLoadFailed = true;
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Save config to YAML file
|
||||
*/
|
||||
|
||||
62
src/config/runtime.test.ts
Normal file
62
src/config/runtime.test.ts
Normal file
@@ -0,0 +1,62 @@
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { mkdtempSync, writeFileSync, rmSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { loadAppConfigOrExit } from './runtime.js';
|
||||
import { didLoadFail } from './io.js';
|
||||
|
||||
describe('loadAppConfigOrExit', () => {
|
||||
it('should load valid config without exiting', () => {
|
||||
const originalEnv = process.env.LETTABOT_CONFIG;
|
||||
const tmpDir = mkdtempSync(join(tmpdir(), 'lettabot-runtime-test-'));
|
||||
const configPath = join(tmpDir, 'lettabot.yaml');
|
||||
|
||||
try {
|
||||
writeFileSync(configPath, 'server:\n mode: api\n', 'utf-8');
|
||||
process.env.LETTABOT_CONFIG = configPath;
|
||||
|
||||
const config = loadAppConfigOrExit(((code: number): never => {
|
||||
throw new Error(`unexpected-exit:${code}`);
|
||||
}));
|
||||
|
||||
expect(config.server.mode).toBe('api');
|
||||
expect(didLoadFail()).toBe(false);
|
||||
} finally {
|
||||
process.env.LETTABOT_CONFIG = originalEnv;
|
||||
rmSync(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('should log and exit on invalid config', () => {
|
||||
const originalEnv = process.env.LETTABOT_CONFIG;
|
||||
const tmpDir = mkdtempSync(join(tmpdir(), 'lettabot-runtime-test-'));
|
||||
const configPath = join(tmpDir, 'lettabot.yaml');
|
||||
|
||||
try {
|
||||
writeFileSync(configPath, 'server:\n api: port: 6702\n', 'utf-8');
|
||||
process.env.LETTABOT_CONFIG = configPath;
|
||||
|
||||
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
const exit = (code: number): never => {
|
||||
throw new Error(`exit:${code}`);
|
||||
};
|
||||
|
||||
expect(() => loadAppConfigOrExit(exit)).toThrow('exit:1');
|
||||
expect(didLoadFail()).toBe(true);
|
||||
expect(errorSpy).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
expect.stringContaining('Failed to load'),
|
||||
expect.anything()
|
||||
);
|
||||
expect(errorSpy).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
expect.stringContaining('Fix the errors above')
|
||||
);
|
||||
|
||||
errorSpy.mockRestore();
|
||||
} finally {
|
||||
process.env.LETTABOT_CONFIG = originalEnv;
|
||||
rmSync(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
19
src/config/runtime.ts
Normal file
19
src/config/runtime.ts
Normal file
@@ -0,0 +1,19 @@
|
||||
import type { LettaBotConfig } from './types.js';
|
||||
import { loadConfigStrict, resolveConfigPath } from './io.js';
|
||||
|
||||
export type ExitFn = (code: number) => never;
|
||||
|
||||
/**
|
||||
* Load config for app/CLI entrypoints. On invalid config, print one
|
||||
* consistent error and terminate.
|
||||
*/
|
||||
export function loadAppConfigOrExit(exitFn: ExitFn = process.exit): LettaBotConfig {
|
||||
try {
|
||||
return loadConfigStrict();
|
||||
} catch (err) {
|
||||
const configPath = resolveConfigPath();
|
||||
console.error(`[Config] Failed to load ${configPath}:`, err);
|
||||
console.error(`[Config] Fix the errors above in ${configPath} and restart.`);
|
||||
return exitFn(1);
|
||||
}
|
||||
}
|
||||
13
src/main.ts
13
src/main.ts
@@ -15,23 +15,18 @@ import { loadOrGenerateApiKey } from './api/auth.js';
|
||||
|
||||
// Load YAML config and apply to process.env (overrides .env values)
|
||||
import {
|
||||
loadConfig,
|
||||
loadAppConfigOrExit,
|
||||
applyConfigToEnv,
|
||||
syncProviders,
|
||||
resolveConfigPath,
|
||||
didLoadFail,
|
||||
isDockerServerMode,
|
||||
serverModeLabel,
|
||||
} from './config/index.js';
|
||||
import { isLettaApiUrl } from './utils/server.js';
|
||||
import { getDataDir, getWorkingDir, hasRailwayVolume } from './utils/paths.js';
|
||||
const yamlConfig = loadConfig();
|
||||
if (didLoadFail()) {
|
||||
console.warn(`[Config] Fix the errors above in ${resolveConfigPath()} and restart.`);
|
||||
} else {
|
||||
const configSource = existsSync(resolveConfigPath()) ? resolveConfigPath() : 'defaults + environment variables';
|
||||
console.log(`[Config] Loaded from ${configSource}`);
|
||||
}
|
||||
const yamlConfig = loadAppConfigOrExit();
|
||||
const configSource = existsSync(resolveConfigPath()) ? resolveConfigPath() : 'defaults + environment variables';
|
||||
console.log(`[Config] Loaded from ${configSource}`);
|
||||
if (yamlConfig.agents?.length) {
|
||||
console.log(`[Config] Mode: ${serverModeLabel(yamlConfig.server.mode)}, Agents: ${yamlConfig.agents.map(a => a.name).join(', ')}`);
|
||||
} else {
|
||||
|
||||
@@ -1258,8 +1258,8 @@ export async function onboard(options?: { nonInteractive?: boolean }): Promise<v
|
||||
const env: Record<string, string> = {};
|
||||
|
||||
// Load existing config if available
|
||||
const { loadConfig, resolveConfigPath } = await import('./config/index.js');
|
||||
const existingConfig = loadConfig();
|
||||
const { loadAppConfigOrExit, resolveConfigPath } = await import('./config/index.js');
|
||||
const existingConfig = loadAppConfigOrExit();
|
||||
const configPath = resolveConfigPath();
|
||||
const hasExistingConfig = existsSync(configPath);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user