From c4c8b170187161d74867144d422f0b1e137a73f6 Mon Sep 17 00:00:00 2001 From: Cameron Date: Tue, 10 Feb 2026 20:16:36 -0800 Subject: [PATCH] fix: support server.api config, preserve api across onboard, fix load error logging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three bugs fixed: 1. server.api is now the canonical location for API server config (port, host, CORS). Users naturally nest api under server -- this now works. Top-level api still accepted with a deprecation warning. 2. Onboarding no longer silently drops api and attachments config when saving. Both interactive and non-interactive paths now preserve unmanaged top-level fields from the existing config. 3. When YAML parsing fails, the log no longer misleadingly says "Loaded from lettabot.yaml". A didLoadFail() flag enables accurate status reporting without changing 17+ loadConfig() call sites. Written by Cameron ◯ Letta Code "The map is not the territory, but a good map sure helps you find port 6702." --- docs/configuration.md | 38 ++++--- src/config/io.test.ts | 234 +++++++++++++++++++++++++++++++++++++++++- src/config/io.ts | 36 +++++-- src/config/types.ts | 7 ++ src/main.ts | 10 +- src/onboard.ts | 19 +++- 6 files changed, 309 insertions(+), 35 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 0af420a..973f103 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -23,6 +23,10 @@ For global installs (`npm install -g`), either: server: mode: cloud # 'cloud' or 'selfhosted' apiKey: letta_... # Required for cloud mode + api: + port: 8080 # Default: 8080 (or PORT env var) + # host: 0.0.0.0 # Uncomment for Docker/Railway + # corsOrigin: https://my.app # Uncomment for cross-origin access # Agent settings (single agent mode) # For multiple agents, use `agents:` array instead -- see Multi-Agent section @@ -87,11 +91,6 @@ attachments: maxMB: 20 maxAgeDays: 14 -# API server (health checks, CLI messaging) -api: - port: 8080 # Default: 8080 (or PORT env var) - # host: 0.0.0.0 # Uncomment for Docker/Railway - # corsOrigin: https://my.app # Uncomment for cross-origin access ``` ## Server Configuration @@ -226,7 +225,7 @@ agents: cron: true ``` -The `server:`, `transcription:`, `attachments:`, and `api:` sections remain at the top level (shared across all agents). +The `server:` (including `server.api:`), `transcription:`, and `attachments:` sections remain at the top level (shared across all agents). ### Known limitations @@ -449,9 +448,9 @@ The top-level `polling` section takes priority if both are present. |--------------|--------------------------| | `GMAIL_ACCOUNT` | `polling.gmail.account` (comma-separated list allowed) | | `POLLING_INTERVAL_MS` | `polling.intervalMs` | -| `PORT` | `api.port` | -| `API_HOST` | `api.host` | -| `API_CORS_ORIGIN` | `api.corsOrigin` | +| `PORT` | `server.api.port` | +| `API_HOST` | `server.api.host` | +| `API_CORS_ORIGIN` | `server.api.corsOrigin` | ## Transcription Configuration @@ -478,18 +477,25 @@ Attachments are stored in `/tmp/lettabot/attachments/`. The built-in API server provides health checks, CLI messaging, and a chat endpoint for programmatic agent access. +Configure it under `server.api:` in your `lettabot.yaml`: + ```yaml -api: - port: 9090 # Default: 8080 - host: 0.0.0.0 # Default: 127.0.0.1 (localhost only) - corsOrigin: "*" # Default: same-origin only +server: + mode: selfhosted + baseUrl: http://localhost:8283 + api: + port: 9090 # Default: 8080 + host: 0.0.0.0 # Default: 127.0.0.1 (localhost only) + corsOrigin: "*" # Default: same-origin only ``` | Option | Type | Default | Description | |--------|------|---------|-------------| -| `api.port` | number | `8080` | Port for the API/health server | -| `api.host` | string | `127.0.0.1` | Bind address. Use `0.0.0.0` for Docker/Railway | -| `api.corsOrigin` | string | _(none)_ | CORS origin header for cross-origin access | +| `server.api.port` | number | `8080` | Port for the API/health server | +| `server.api.host` | string | `127.0.0.1` | Bind address. Use `0.0.0.0` for Docker/Railway | +| `server.api.corsOrigin` | string | _(none)_ | CORS origin header for cross-origin access | + +> **Note:** Top-level `api:` is still accepted for backward compatibility but deprecated. Move it under `server:` to avoid warnings. ### Chat Endpoint diff --git a/src/config/io.test.ts b/src/config/io.test.ts index e9742f8..cfb5716 100644 --- a/src/config/io.test.ts +++ b/src/config/io.test.ts @@ -1,10 +1,10 @@ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; -import { mkdtempSync, existsSync, readFileSync, rmSync } from 'node:fs'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { mkdtempSync, existsSync, readFileSync, writeFileSync, rmSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; import YAML from 'yaml'; -import { saveConfig, loadConfig } from './io.js'; -import { normalizeAgents } from './types.js'; +import { saveConfig, loadConfig, configToEnv, didLoadFail } from './io.js'; +import { normalizeAgents, DEFAULT_CONFIG } from './types.js'; import type { LettaBotConfig } from './types.js'; describe('saveConfig with agents[] format', () => { @@ -149,3 +149,229 @@ describe('saveConfig with agents[] format', () => { expect(parsed.agents[0].providers).toBeUndefined(); }); }); + +describe('server.api config (canonical location)', () => { + it('configToEnv should read port from server.api', () => { + const config: LettaBotConfig = { + ...DEFAULT_CONFIG, + server: { + mode: 'selfhosted', + baseUrl: 'http://localhost:6701', + api: { port: 6702, host: '0.0.0.0', corsOrigin: '*' }, + }, + }; + + const env = configToEnv(config); + + expect(env.PORT).toBe('6702'); + expect(env.API_HOST).toBe('0.0.0.0'); + expect(env.API_CORS_ORIGIN).toBe('*'); + }); + + it('configToEnv should fall back to top-level api (deprecated)', () => { + const config: LettaBotConfig = { + ...DEFAULT_CONFIG, + server: { mode: 'selfhosted', baseUrl: 'http://localhost:6701' }, + api: { port: 8081 }, + }; + + const env = configToEnv(config); + + expect(env.PORT).toBe('8081'); + }); + + it('server.api should take precedence over top-level api', () => { + const config: LettaBotConfig = { + ...DEFAULT_CONFIG, + server: { + mode: 'selfhosted', + baseUrl: 'http://localhost:6701', + api: { port: 9090 }, + }, + api: { port: 8081 }, + }; + + const env = configToEnv(config); + + expect(env.PORT).toBe('9090'); + }); + + it('should not set PORT when no api config is present', () => { + const config: LettaBotConfig = { + ...DEFAULT_CONFIG, + server: { mode: 'selfhosted', baseUrl: 'http://localhost:6701' }, + }; + + const env = configToEnv(config); + + expect(env.PORT).toBeUndefined(); + }); + + it('server.api should survive save/load roundtrip in YAML', () => { + const tmpDir = mkdtempSync(join(tmpdir(), 'lettabot-api-test-')); + const configPath = join(tmpDir, 'lettabot.yaml'); + + try { + const config = { + server: { + mode: 'selfhosted' as const, + baseUrl: 'http://localhost:6701', + api: { port: 6702, host: '0.0.0.0' }, + }, + agents: [{ + name: 'TestBot', + channels: {}, + }], + }; + + saveConfig(config, configPath); + + const raw = readFileSync(configPath, 'utf-8'); + const parsed = YAML.parse(raw); + + // server.api should be in the YAML under server + expect(parsed.server.api).toBeDefined(); + expect(parsed.server.api.port).toBe(6702); + expect(parsed.server.api.host).toBe('0.0.0.0'); + + // Should NOT have top-level api + expect(parsed.api).toBeUndefined(); + } finally { + rmSync(tmpDir, { recursive: true, force: true }); + } + }); +}); + +describe('didLoadFail', () => { + it('should return false initially', () => { + // loadConfig hasn't been called with a bad file, so it should be false + // (or whatever state it was left in from previous test) + // Call loadConfig with a valid env to reset + const originalEnv = process.env.LETTABOT_CONFIG; + const tmpDir = mkdtempSync(join(tmpdir(), 'lettabot-fail-test-')); + const configPath = join(tmpDir, 'lettabot.yaml'); + + try { + writeFileSync(configPath, 'server:\n mode: cloud\n', 'utf-8'); + process.env.LETTABOT_CONFIG = configPath; + loadConfig(); + expect(didLoadFail()).toBe(false); + } finally { + process.env.LETTABOT_CONFIG = originalEnv; + rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it('should return true after a parse error', () => { + const originalEnv = process.env.LETTABOT_CONFIG; + const tmpDir = mkdtempSync(join(tmpdir(), 'lettabot-fail-test-')); + const configPath = join(tmpDir, 'lettabot.yaml'); + + try { + // Write invalid YAML + writeFileSync(configPath, 'server:\n api: port: 6702\n', 'utf-8'); + process.env.LETTABOT_CONFIG = configPath; + + // Suppress console output during test + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + const config = loadConfig(); + + expect(didLoadFail()).toBe(true); + // Should return default config on failure + expect(config.server.mode).toBe(DEFAULT_CONFIG.server.mode); + + errorSpy.mockRestore(); + warnSpy.mockRestore(); + } finally { + process.env.LETTABOT_CONFIG = originalEnv; + rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it('should reset to false on successful load after a failure', () => { + const originalEnv = process.env.LETTABOT_CONFIG; + const tmpDir = mkdtempSync(join(tmpdir(), 'lettabot-fail-test-')); + const badPath = join(tmpDir, 'bad.yaml'); + const goodPath = join(tmpDir, 'good.yaml'); + + try { + // First: load bad file + writeFileSync(badPath, 'server:\n api: port: 6702\n', 'utf-8'); + process.env.LETTABOT_CONFIG = badPath; + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + loadConfig(); + expect(didLoadFail()).toBe(true); + errorSpy.mockRestore(); + warnSpy.mockRestore(); + + // Then: load good file + writeFileSync(goodPath, 'server:\n mode: selfhosted\n baseUrl: http://localhost:6701\n', 'utf-8'); + process.env.LETTABOT_CONFIG = goodPath; + loadConfig(); + expect(didLoadFail()).toBe(false); + } finally { + process.env.LETTABOT_CONFIG = originalEnv; + rmSync(tmpDir, { recursive: true, force: true }); + } + }); +}); + +describe('loadConfig deprecation warning for top-level api', () => { + it('should warn when top-level api is present without server.api', () => { + const originalEnv = process.env.LETTABOT_CONFIG; + const tmpDir = mkdtempSync(join(tmpdir(), 'lettabot-deprecation-test-')); + const configPath = join(tmpDir, 'lettabot.yaml'); + + try { + writeFileSync(configPath, 'server:\n mode: cloud\napi:\n port: 9090\n', 'utf-8'); + process.env.LETTABOT_CONFIG = configPath; + + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + const config = loadConfig(); + + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('Top-level `api:` is deprecated') + ); + // The top-level api should still be loaded + expect(config.api?.port).toBe(9090); + + warnSpy.mockRestore(); + } finally { + process.env.LETTABOT_CONFIG = originalEnv; + rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it('should not warn when server.api is used (canonical location)', () => { + const originalEnv = process.env.LETTABOT_CONFIG; + const tmpDir = mkdtempSync(join(tmpdir(), 'lettabot-deprecation-test-')); + const configPath = join(tmpDir, 'lettabot.yaml'); + + try { + writeFileSync(configPath, 'server:\n mode: selfhosted\n baseUrl: http://localhost:6701\n api:\n port: 6702\n', 'utf-8'); + process.env.LETTABOT_CONFIG = configPath; + + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + const config = loadConfig(); + + // Should NOT have deprecated warning + expect(warnSpy).not.toHaveBeenCalledWith( + expect.stringContaining('Top-level `api:` is deprecated') + ); + // server.api should be loaded + expect(config.server.api?.port).toBe(6702); + // top-level api should be undefined + expect(config.api).toBeUndefined(); + + warnSpy.mockRestore(); + } finally { + process.env.LETTABOT_CONFIG = originalEnv; + rmSync(tmpDir, { recursive: true, force: true }); + } + }); +}); diff --git a/src/config/io.ts b/src/config/io.ts index 7dcebc6..3d1acac 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -45,10 +45,18 @@ export function resolveConfigPath(): string { return DEFAULT_CONFIG_PATH; } +/** + * Whether the last loadConfig() call failed to parse the config file. + * Used to avoid misleading "Loaded from" messages when the file exists but has syntax errors. + */ +let _lastLoadFailed = false; +export function didLoadFail(): boolean { return _lastLoadFailed; } + /** * Load config from YAML file */ export function loadConfig(): LettaBotConfig { + _lastLoadFailed = false; const configPath = resolveConfigPath(); if (!existsSync(configPath)) { @@ -65,15 +73,24 @@ export function loadConfig(): LettaBotConfig { fixLargeGroupIds(content, parsed); // Merge with defaults - return { + const config = { ...DEFAULT_CONFIG, ...parsed, server: { ...DEFAULT_CONFIG.server, ...parsed.server }, agent: { ...DEFAULT_CONFIG.agent, ...parsed.agent }, channels: { ...DEFAULT_CONFIG.channels, ...parsed.channels }, }; + + // Deprecation warning: top-level api should be moved under server + if (config.api && !config.server.api) { + console.warn('[Config] WARNING: Top-level `api:` is deprecated. Move it under `server:`.'); + } + + return config; } catch (err) { - console.error(`[Config] Failed to load ${configPath}:`, err); + _lastLoadFailed = true; + console.error(`[Config] Failed to parse ${configPath}:`, err); + console.warn('[Config] Using default configuration. Check your YAML syntax.'); return { ...DEFAULT_CONFIG }; } } @@ -283,15 +300,16 @@ export function configToEnv(config: LettaBotConfig): Record { env.ATTACHMENTS_MAX_AGE_DAYS = String(config.attachments.maxAgeDays); } - // API server - if (config.api?.port !== undefined) { - env.PORT = String(config.api.port); + // API server (server.api is canonical, top-level api is deprecated fallback) + const apiConfig = config.server.api ?? config.api; + if (apiConfig?.port !== undefined) { + env.PORT = String(apiConfig.port); } - if (config.api?.host) { - env.API_HOST = config.api.host; + if (apiConfig?.host) { + env.API_HOST = apiConfig.host; } - if (config.api?.corsOrigin) { - env.API_CORS_ORIGIN = config.api.corsOrigin; + if (apiConfig?.corsOrigin) { + env.API_CORS_ORIGIN = apiConfig.corsOrigin; } return env; diff --git a/src/config/types.ts b/src/config/types.ts index 3774238..629c9b9 100644 --- a/src/config/types.ts +++ b/src/config/types.ts @@ -56,6 +56,12 @@ export interface LettaBotConfig { baseUrl?: string; // Only for cloud mode apiKey?: string; + // API server config (port, host, CORS) — canonical location + api?: { + port?: number; // Default: 8080 (or PORT env var) + host?: string; // Default: 127.0.0.1 (secure). Use '0.0.0.0' for Docker/Railway + corsOrigin?: string; // CORS origin. Default: same-origin only + }; }; // Multi-agent configuration @@ -117,6 +123,7 @@ export interface LettaBotConfig { }; // API server (health checks, CLI messaging) + /** @deprecated Use server.api instead */ api?: { port?: number; // Default: 8080 (or PORT env var) host?: string; // Default: 127.0.0.1 (secure). Use '0.0.0.0' for Docker/Railway diff --git a/src/main.ts b/src/main.ts index 036dcf4..e990cfb 100644 --- a/src/main.ts +++ b/src/main.ts @@ -14,12 +14,16 @@ import { createApiServer } from './api/server.js'; import { loadOrGenerateApiKey } from './api/auth.js'; // Load YAML config and apply to process.env (overrides .env values) -import { loadConfig, applyConfigToEnv, syncProviders, resolveConfigPath } from './config/index.js'; +import { loadConfig, applyConfigToEnv, syncProviders, resolveConfigPath, didLoadFail } from './config/index.js'; import { isLettaCloudUrl } from './utils/server.js'; import { getDataDir, getWorkingDir, hasRailwayVolume } from './utils/paths.js'; const yamlConfig = loadConfig(); -const configSource = existsSync(resolveConfigPath()) ? resolveConfigPath() : 'defaults + environment variables'; -console.log(`[Config] Loaded from ${configSource}`); +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}`); +} if (yamlConfig.agents?.length) { console.log(`[Config] Mode: ${yamlConfig.server.mode}, Agents: ${yamlConfig.agents.map(a => a.name).join(', ')}`); } else { diff --git a/src/onboard.ts b/src/onboard.ts index 286d947..b915ed7 100644 --- a/src/onboard.ts +++ b/src/onboard.ts @@ -113,14 +113,18 @@ function readConfigFromEnv(existingConfig: any): any { }; } -async function saveConfigFromEnv(config: any, configPath: string): Promise { +async function saveConfigFromEnv(config: any, configPath: string, existingConfig?: LettaBotConfig): Promise { const { saveConfig } = await import('./config/index.js'); + // Resolve API server config from existing config (server.api is canonical, top-level api is fallback) + const existingApiConfig = existingConfig?.server?.api ?? existingConfig?.api; + const lettabotConfig: Partial & Pick = { server: { mode: isLettaCloudUrl(config.baseUrl) ? 'cloud' : 'selfhosted', baseUrl: config.baseUrl, apiKey: config.apiKey, + ...(existingApiConfig ? { api: existingApiConfig } : {}), }, agents: [{ name: config.agentName, @@ -196,6 +200,8 @@ async function saveConfigFromEnv(config: any, configPath: string): Promise }, }, }], + // Preserve unmanaged top-level fields from existing config + ...(existingConfig?.attachments ? { attachments: existingConfig.attachments } : {}), }; saveConfig(lettabotConfig); @@ -1370,8 +1376,8 @@ export async function onboard(options?: { nonInteractive?: boolean }): Promise & Pick = { server: { mode: config.authMethod === 'selfhosted' ? 'selfhosted' : 'cloud', ...(config.authMethod === 'selfhosted' && config.baseUrl ? { baseUrl: config.baseUrl } : {}), ...(config.apiKey ? { apiKey: config.apiKey } : {}), + // Preserve API server config (port, host, CORS) + ...(existingApiConfig ? { api: existingApiConfig } : {}), }, agents: [agentConfig], ...(config.transcription.enabled && config.transcription.apiKey ? { @@ -1782,6 +1793,8 @@ export async function onboard(options?: { nonInteractive?: boolean }): Promise