feat(config): persist config.patch updates atomically
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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": {
|
||||
|
||||
+1
-1
@@ -34,7 +34,7 @@ export async function runSetup(configPath: string): Promise<void> {
|
||||
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<void>(resolve => daemon.lifecycle.onShutdown(async () => resolve()));
|
||||
return;
|
||||
}
|
||||
|
||||
+1
-1
@@ -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(', ')}`);
|
||||
|
||||
@@ -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';
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
@@ -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 <path>.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);
|
||||
}
|
||||
+6
-2
@@ -54,7 +54,11 @@ export interface DaemonContext {
|
||||
browserManager?: BrowserManager;
|
||||
}
|
||||
|
||||
export async function startDaemon(config: Config): Promise<DaemonContext> {
|
||||
export interface StartDaemonOptions {
|
||||
configPath?: string;
|
||||
}
|
||||
|
||||
export async function startDaemon(config: Config, options?: StartDaemonOptions): Promise<DaemonContext> {
|
||||
// ── Log level ──
|
||||
setLogLevel(config.log_level);
|
||||
|
||||
@@ -144,7 +148,7 @@ export async function startDaemon(config: Config): Promise<DaemonContext> {
|
||||
let channelAgents: ReturnType<typeof createMessageRouter>['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,
|
||||
});
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -4,6 +4,7 @@ import type { Config } from '../../config/index.js';
|
||||
|
||||
export interface ConfigHandlerDeps {
|
||||
config: Config;
|
||||
persistConfig?: (nextConfig: Config) => Promise<void> | 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<string, unknown>)) {
|
||||
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<string, unknown>)[key];
|
||||
}
|
||||
Object.assign(deps.config as Record<string, unknown>, draft as Record<string, unknown>);
|
||||
|
||||
return makeResponse(request.id, { applied, rejected, persisted: Boolean(deps.persistConfig) });
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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> | void;
|
||||
/** Optional callback for system.restart. Should trigger graceful shutdown + process restart. */
|
||||
restart?: () => Promise<void>;
|
||||
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);
|
||||
}
|
||||
|
||||
@@ -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';
|
||||
|
||||
Reference in New Issue
Block a user