From 0220ec10ddf258e77b04bf0793bc533350ffb4e0 Mon Sep 17 00:00:00 2001 From: William Valentin Date: Sun, 15 Feb 2026 22:03:21 -0800 Subject: [PATCH] feat(config): persist config.patch updates atomically --- .../2026-02-16-codebase-audit-report.md | 1 + docs/plans/state.json | 23 ++++++++ src/cli/setup.ts | 2 +- src/cli/start.ts | 2 +- src/config/index.ts | 1 + src/config/persistence.test.ts | 52 +++++++++++++++++++ src/config/persistence.ts | 26 ++++++++++ src/daemon/index.ts | 8 ++- src/daemon/services.ts | 9 +++- src/gateway/handlers/config.ts | 29 ++++++++++- src/gateway/handlers/handlers.test.ts | 48 +++++++++++++++-- src/gateway/server.ts | 7 ++- src/gateway/ui/pages/settings.js | 8 +++ 13 files changed, 205 insertions(+), 11 deletions(-) create mode 100644 src/config/persistence.test.ts create mode 100644 src/config/persistence.ts diff --git a/docs/plans/analysis/2026-02-16-codebase-audit-report.md b/docs/plans/analysis/2026-02-16-codebase-audit-report.md index ee34675..aecab99 100644 --- a/docs/plans/analysis/2026-02-16-codebase-audit-report.md +++ b/docs/plans/analysis/2026-02-16-codebase-audit-report.md @@ -15,6 +15,7 @@ Scope: Production-risk-first audit of bugs, code improvements, and feature oppor - ✅ F-009 addressed: gateway now enforces per-connection WebSocket ingress rate limits with deterministic throttle errors and close-on-repeated-violation behavior. - ✅ F-008 addressed: WhatsApp Chromium launch is now sandboxed by default; no-sandbox mode is behind explicit `whatsapp.no_sandbox: true` opt-in. - ✅ F-014 addressed: `ModelRouter.setOnTierChange` now preserves existing listeners instead of replacing them, removing destructive listener-setter behavior. +- ✅ F-002 addressed: `config.patch` now supports durable persistence via atomic write + backup when daemon has a concrete config path, and response includes `persisted`/`persistError` so UI can distinguish runtime-only vs disk-persisted updates. ## Executive Summary diff --git a/docs/plans/state.json b/docs/plans/state.json index 217e699..00c90cf 100644 --- a/docs/plans/state.json +++ b/docs/plans/state.json @@ -2524,6 +2524,29 @@ "docs/plans/analysis/2026-02-16-codebase-audit-report.md" ], "test_status": "pnpm test:run src/models/router.test.ts + pnpm typecheck passing" + }, + "audit-followup-config-patch-persistence": { + "status": "completed", + "date": "2026-02-16", + "updated": "2026-02-16", + "summary": "Implemented durable config.patch persistence when config path is known: patches are applied to a draft, atomically written to disk with .bak backup, then committed to runtime config only on successful persist. Added persisted/persistError response fields and settings UI messaging for runtime-only saves.", + "files_created": [ + "src/config/persistence.ts", + "src/config/persistence.test.ts" + ], + "files_modified": [ + "src/config/index.ts", + "src/gateway/handlers/config.ts", + "src/gateway/handlers/handlers.test.ts", + "src/gateway/server.ts", + "src/daemon/services.ts", + "src/daemon/index.ts", + "src/cli/start.ts", + "src/cli/setup.ts", + "src/gateway/ui/pages/settings.js", + "docs/plans/analysis/2026-02-16-codebase-audit-report.md" + ], + "test_status": "pnpm test:run src/gateway/handlers/handlers.test.ts src/config/persistence.test.ts + pnpm typecheck passing" } }, "overall_progress": { diff --git a/src/cli/setup.ts b/src/cli/setup.ts index e02ffc2..868439c 100644 --- a/src/cli/setup.ts +++ b/src/cli/setup.ts @@ -34,7 +34,7 @@ export async function runSetup(configPath: string): Promise { const { startDaemon } = await import('../daemon/index.js'); const { loadConfig } = await import('../config/index.js'); const config = loadConfig(configPath); - const daemon = await startDaemon(config); + const daemon = await startDaemon(config, { configPath }); await new Promise(resolve => daemon.lifecycle.onShutdown(async () => resolve())); return; } diff --git a/src/cli/start.ts b/src/cli/start.ts index edd4393..9fb69c7 100644 --- a/src/cli/start.ts +++ b/src/cli/start.ts @@ -44,7 +44,7 @@ export function registerStartCommand(program: Command): void { // Dynamic import to avoid loading daemon code for other commands const { startDaemon } = await import('../daemon/index.js'); - const daemon = await startDaemon(config); + const daemon = await startDaemon(config, { configPath }); if (config.telegram) { console.log(`Allowed Telegram chat IDs: ${config.telegram.allowed_chat_ids.join(', ')}`); diff --git a/src/config/index.ts b/src/config/index.ts index 5da0c79..7200740 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -1,2 +1,3 @@ export { loadConfig, deepMerge } from './loader.js'; +export { persistConfig } from './persistence.js'; export { configSchema, MODEL_PROVIDERS, type ModelProvider, type Config, type TelegramConfig, type ModelConfig, type CronJobConfig, type AgentsConfig, type CompactionConfig, type ToolProfile, type ToolOverrideConfig, type ToolsConfig, type SandboxConfig, type AgentConfigEntry, type RoutingConfig, type ServerConfig } from './schema.js'; diff --git a/src/config/persistence.test.ts b/src/config/persistence.test.ts new file mode 100644 index 0000000..660a6a7 --- /dev/null +++ b/src/config/persistence.test.ts @@ -0,0 +1,52 @@ +import { describe, it, expect, afterEach } from 'vitest'; +import { mkdtempSync, readFileSync, rmSync, writeFileSync, existsSync } from 'fs'; +import { join } from 'path'; +import { tmpdir } from 'os'; +import { persistConfig } from './persistence.js'; +import { configSchema } from './schema.js'; + +const testRoots: string[] = []; + +afterEach(() => { + while (testRoots.length > 0) { + const dir = testRoots.pop(); + if (!dir) {continue;} + try { + rmSync(dir, { recursive: true, force: true }); + } catch {} + } +}); + +function makeConfig() { + return configSchema.parse({ + telegram: { bot_token: 'test-token', allowed_chat_ids: [1] }, + models: { default: { provider: 'anthropic', model: 'claude-3' } }, + hooks: { confirm: ['shell.exec'], log: [], silent: [] }, + }); +} + +describe('persistConfig', () => { + it('writes config to target path', () => { + const dir = mkdtempSync(join(tmpdir(), 'flynn-config-persist-')); + testRoots.push(dir); + const configPath = join(dir, 'config.yaml'); + + persistConfig(configPath, makeConfig()); + const written = readFileSync(configPath, 'utf-8'); + expect(written).toContain('telegram:'); + expect(written).toContain('models:'); + }); + + it('creates .bak when overwriting existing config', () => { + const dir = mkdtempSync(join(tmpdir(), 'flynn-config-persist-')); + testRoots.push(dir); + const configPath = join(dir, 'config.yaml'); + + writeFileSync(configPath, 'legacy: true\n', 'utf-8'); + persistConfig(configPath, makeConfig()); + + const backupPath = `${configPath}.bak`; + expect(existsSync(backupPath)).toBe(true); + expect(readFileSync(backupPath, 'utf-8')).toContain('legacy: true'); + }); +}); diff --git a/src/config/persistence.ts b/src/config/persistence.ts new file mode 100644 index 0000000..a6397b6 --- /dev/null +++ b/src/config/persistence.ts @@ -0,0 +1,26 @@ +import { copyFileSync, existsSync, mkdirSync, renameSync, writeFileSync } from 'fs'; +import { dirname } from 'path'; +import { stringify } from 'yaml'; +import type { Config } from './schema.js'; + +/** + * Persist config atomically: + * 1) Backup existing file to .bak + * 2) Write new YAML to temp file + * 3) Rename temp file into place + */ +export function persistConfig(configPath: string, config: Config): void { + const dir = dirname(configPath); + mkdirSync(dir, { recursive: true }); + + const yaml = stringify(config); + const tmpPath = `${configPath}.tmp-${process.pid}-${Date.now()}`; + const backupPath = `${configPath}.bak`; + + if (existsSync(configPath)) { + copyFileSync(configPath, backupPath); + } + + writeFileSync(tmpPath, yaml, 'utf-8'); + renameSync(tmpPath, configPath); +} diff --git a/src/daemon/index.ts b/src/daemon/index.ts index d5b2abf..afef8d6 100644 --- a/src/daemon/index.ts +++ b/src/daemon/index.ts @@ -54,7 +54,11 @@ export interface DaemonContext { browserManager?: BrowserManager; } -export async function startDaemon(config: Config): Promise { +export interface StartDaemonOptions { + configPath?: string; +} + +export async function startDaemon(config: Config, options?: StartDaemonOptions): Promise { // ── Log level ── setLogLevel(config.log_level); @@ -144,7 +148,7 @@ export async function startDaemon(config: Config): Promise { let channelAgents: ReturnType['agents'] | null = null; const gateway = createGateway({ - config, sessionManager, modelRouter, systemPrompt, toolRegistry, toolExecutor, + config, configPath: options?.configPath, sessionManager, modelRouter, systemPrompt, toolRegistry, toolExecutor, channelRegistry, pairingManager, lifecycle, memoryStore, getChannelAgents: () => channelAgents, commandRegistry, intentRegistry, routingPolicy, }); diff --git a/src/daemon/services.ts b/src/daemon/services.ts index 0c93263..dff3fa3 100644 --- a/src/daemon/services.ts +++ b/src/daemon/services.ts @@ -1,4 +1,5 @@ import type { Config } from '../config/index.js'; +import { persistConfig } from '../config/index.js'; import type { Lifecycle } from './lifecycle.js'; import type { ToolRegistry, ToolExecutor } from '../tools/index.js'; import type { AgentOrchestrator } from '../backends/index.js'; @@ -279,6 +280,7 @@ export function initPairingManager(config: Config, store?: PairingStore): Pairin export interface GatewayDeps { config: Config; + configPath?: string; sessionManager: SessionManager; modelRouter: ModelRouter; systemPrompt: string; @@ -295,7 +297,7 @@ export interface GatewayDeps { } export function createGateway(deps: GatewayDeps): GatewayServer { - const { config, sessionManager, modelRouter, systemPrompt, toolRegistry, toolExecutor, channelRegistry, pairingManager, lifecycle, getChannelAgents } = deps; + const { config, configPath, sessionManager, modelRouter, systemPrompt, toolRegistry, toolExecutor, channelRegistry, pairingManager, lifecycle, getChannelAgents } = deps; const gateway = new GatewayServer({ port: config.server.port, @@ -324,6 +326,11 @@ export function createGateway(deps: GatewayDeps): GatewayServer { routingPolicy: deps.routingPolicy, uiDir: resolve(import.meta.dirname, '../gateway/ui'), config, + persistConfig: configPath + ? async (nextConfig) => { + persistConfig(configPath, nextConfig); + } + : undefined, channelRegistry, pairingManager, memoryStore: deps.memoryStore, diff --git a/src/gateway/handlers/config.ts b/src/gateway/handlers/config.ts index 0cb4e4e..08eb6e4 100644 --- a/src/gateway/handlers/config.ts +++ b/src/gateway/handlers/config.ts @@ -4,6 +4,7 @@ import type { Config } from '../../config/index.js'; export interface ConfigHandlerDeps { config: Config; + persistConfig?: (nextConfig: Config) => Promise | void; } /** @@ -140,6 +141,7 @@ export function createConfigHandlers(deps: ConfigHandlerDeps) { const applied: string[] = []; const rejected: string[] = []; + const draft = JSON.parse(JSON.stringify(deps.config)) as Config; for (const [key, value] of Object.entries(patches as Record)) { const patcher = PATCHABLE_KEYS[key]; @@ -147,7 +149,7 @@ export function createConfigHandlers(deps: ConfigHandlerDeps) { rejected.push(key); continue; } - const ok = patcher(deps.config, value); + const ok = patcher(draft, value); if (ok) { applied.push(key); } else { @@ -155,7 +157,30 @@ export function createConfigHandlers(deps: ConfigHandlerDeps) { } } - return makeResponse(request.id, { applied, rejected }); + if (applied.length === 0) { + return makeResponse(request.id, { applied, rejected, persisted: false }); + } + + if (deps.persistConfig) { + try { + await deps.persistConfig(draft); + } catch (err) { + return makeResponse(request.id, { + applied: [], + rejected, + persisted: false, + persistError: err instanceof Error ? err.message : String(err), + }); + } + } + + // Update in-memory runtime config only after a successful persist (or when persistence is not configured). + for (const key of Object.keys(deps.config)) { + delete (deps.config as Record)[key]; + } + Object.assign(deps.config as Record, draft as Record); + + return makeResponse(request.id, { applied, rejected, persisted: Boolean(deps.persistConfig) }); }, }; } diff --git a/src/gateway/handlers/handlers.test.ts b/src/gateway/handlers/handlers.test.ts index dd1283a..4810358 100644 --- a/src/gateway/handlers/handlers.test.ts +++ b/src/gateway/handlers/handlers.test.ts @@ -718,9 +718,10 @@ describe('config handlers', () => { }; const result = await handlers['config.patch'](req) as GatewayResponse; - const r = result.result as { applied: string[]; rejected: string[] }; + const r = result.result as { applied: string[]; rejected: string[]; persisted: boolean }; expect(r.applied).toEqual(['hooks.confirm', 'hooks.log']); expect(r.rejected).toEqual([]); + expect(r.persisted).toBe(false); // Verify the config was actually mutated expect(config.hooks.confirm).toEqual(['shell.exec', 'file.write']); expect(config.hooks.log).toEqual(['file.read']); @@ -741,9 +742,10 @@ describe('config handlers', () => { }; const result = await handlers['config.patch'](req) as GatewayResponse; - const r = result.result as { applied: string[]; rejected: string[] }; + const r = result.result as { applied: string[]; rejected: string[]; persisted: boolean }; expect(r.applied).toEqual(['hooks.confirm']); expect(r.rejected).toEqual(['telegram.bot_token']); + expect(r.persisted).toBe(false); }); it('config.patch rejects invalid value types', async () => { @@ -760,9 +762,49 @@ describe('config handlers', () => { }; const result = await handlers['config.patch'](req) as GatewayResponse; - const r = result.result as { applied: string[]; rejected: string[] }; + const r = result.result as { applied: string[]; rejected: string[]; persisted: boolean }; expect(r.applied).toEqual([]); expect(r.rejected).toEqual(['hooks.confirm']); + expect(r.persisted).toBe(false); + }); + + it('config.patch persists changes when persistence callback is provided', async () => { + const config = makeConfig(); + const persist = vi.fn(); + const handlers = createConfigHandlers({ config: config as any, persistConfig: persist as any }); + const req: GatewayRequest = { + id: 6, + method: 'config.patch', + params: { patches: { 'hooks.confirm': ['shell.exec', 'file.write'] } }, + }; + const result = await handlers['config.patch'](req) as GatewayResponse; + const r = result.result as { applied: string[]; rejected: string[]; persisted: boolean }; + + expect(r.applied).toEqual(['hooks.confirm']); + expect(r.rejected).toEqual([]); + expect(r.persisted).toBe(true); + expect(persist).toHaveBeenCalledTimes(1); + expect(config.hooks.confirm).toEqual(['shell.exec', 'file.write']); + }); + + it('config.patch does not mutate runtime config when persistence fails', async () => { + const config = makeConfig(); + const before = [...config.hooks.confirm]; + const persist = vi.fn().mockRejectedValue(new Error('disk full')); + const handlers = createConfigHandlers({ config: config as any, persistConfig: persist as any }); + const req: GatewayRequest = { + id: 7, + method: 'config.patch', + params: { patches: { 'hooks.confirm': ['file.write'] } }, + }; + const result = await handlers['config.patch'](req) as GatewayResponse; + const r = result.result as { applied: string[]; rejected: string[]; persisted: boolean; persistError?: string }; + + expect(r.applied).toEqual([]); + expect(r.rejected).toEqual([]); + expect(r.persisted).toBe(false); + expect(r.persistError).toContain('disk full'); + expect(config.hooks.confirm).toEqual(before); }); it('config.patch requires patches object', async () => { diff --git a/src/gateway/server.ts b/src/gateway/server.ts index f440c1b..20acb3d 100644 --- a/src/gateway/server.ts +++ b/src/gateway/server.ts @@ -59,6 +59,8 @@ export interface GatewayServerConfig { authHttp?: boolean; uiDir?: string; config?: Config; + /** Optional persistence callback for config.patch updates. */ + persistConfig?: (nextConfig: Config) => Promise | void; /** Optional callback for system.restart. Should trigger graceful shutdown + process restart. */ restart?: () => Promise; channelRegistry?: ChannelRegistry; @@ -193,7 +195,10 @@ export class GatewayServer { // Config handlers (only if config object is provided) if (this.config.config) { - const configHandlers = createConfigHandlers({ config: this.config.config }); + const configHandlers = createConfigHandlers({ + config: this.config.config, + persistConfig: this.config.persistConfig, + }); for (const [method, handler] of Object.entries(configHandlers)) { this.router.register(method, handler); } diff --git a/src/gateway/ui/pages/settings.js b/src/gateway/ui/pages/settings.js index 51e0387..ca27f57 100644 --- a/src/gateway/ui/pages/settings.js +++ b/src/gateway/ui/pages/settings.js @@ -155,10 +155,18 @@ async function saveHooks() { const applied = result.applied ?? []; const rejected = result.rejected ?? []; + const persisted = result.persisted === true; + const persistError = result.persistError; if (rejected.length > 0) { status.textContent = `Partially saved. Rejected: ${rejected.join(', ')}`; status.className = 'text-sm text-error'; + } else if (persistError) { + status.textContent = `Save failed: ${persistError}`; + status.className = 'text-sm text-error'; + } else if (!persisted) { + status.textContent = `Saved in runtime only (${applied.length} updated)`; + status.className = 'text-sm text-muted'; } else { status.textContent = `Saved (${applied.length} updated)`; status.className = 'text-sm text-success';