feat(FN-1336): add write-through cache to GlobalSettingsStore
- Add in-memory cache to GlobalSettingsStore for fast settings reads - Add getSettingsFast() to TaskStore that uses the cached settings - Update GET /settings route to use getSettingsFast() for faster responses - Add invalidateCache() method for testing and external process scenarios - Document write-through cache pattern in .fusion/memory.md
This commit is contained in:
@@ -35,6 +35,7 @@
|
||||
- The null-as-delete pattern for settings: In `TaskStore.updateSettings()`, `null` values in the settings patch are treated as "delete this key from settings" (since `JSON.stringify` drops `undefined` keys). This allows the frontend to explicitly clear a setting by sending `null`. The key is deleted from both `config.settings` and `projectPatch` before merging, so cleared settings fall back to `DEFAULT_SETTINGS`.
|
||||
- `TaskStore.logEntry()`, `addComment()`, `addSteeringComment()`, `pauseTask()` accept an optional `RunMutationContext` parameter for audit trail correlation. Always pass it when the caller is an engine module (executor, heartbeat monitor) to maintain the audit trail. The executor constructs a synthetic `runContext` with `runId: "exec-{taskId}-{timestamp}-{random}"` since it doesn't use `AgentHeartbeatRun`.
|
||||
- **Run-Audit Instrumentation (FN-1404)**: The engine instruments mutation calls with audit events via `createRunAuditor()` from `run-audit.ts`. Each active run (heartbeat, executor, merger) creates an `EngineRunContext` with `runId`, `agentId`, `taskId`, and `phase`. The auditor no-ops cleanly when no run context exists (backward compatible with manual/non-run paths). Use `generateSyntheticRunId()` for executor/merger synthetic IDs. Audit events are emitted for git mutations (worktree/branch/create/remove/reset), database mutations (task:update/move/comment/assign/checkout), and filesystem mutations (file:capture-modified).
|
||||
- **Write-through cache pattern (FN-1336)**: When adding caching to a store, use write-through invalidation (update cache in setter, return cached value in getter). For `GlobalSettingsStore`, the cache survives for the lifetime of the process since it's a singleton per server instance. Add `invalidateCache()` for testing and edge cases where external processes modify the file.
|
||||
|
||||
## Color Theme System
|
||||
|
||||
|
||||
@@ -270,4 +270,58 @@ describe("GlobalSettingsStore", () => {
|
||||
expect(existsSync(tmpPath)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("in-memory cache", () => {
|
||||
it("returns cached result on second getSettings() call without reading disk", async () => {
|
||||
await store.init();
|
||||
await writeFile(join(dir, "settings.json"), JSON.stringify({ themeMode: "light" }));
|
||||
|
||||
// First call reads from disk
|
||||
const first = await store.getSettings();
|
||||
expect(first.themeMode).toBe("light");
|
||||
|
||||
// Modify the file externally
|
||||
await writeFile(join(dir, "settings.json"), JSON.stringify({ themeMode: "dark" }));
|
||||
|
||||
// Second call should return cached result (light), not the new disk value (dark)
|
||||
const second = await store.getSettings();
|
||||
expect(second.themeMode).toBe("light");
|
||||
});
|
||||
|
||||
it("updateSettings() updates the cache", async () => {
|
||||
await store.init();
|
||||
|
||||
// First call populates cache
|
||||
const first = await store.getSettings();
|
||||
expect(first.themeMode).toBe("dark");
|
||||
|
||||
// Update settings
|
||||
await store.updateSettings({ themeMode: "light" });
|
||||
|
||||
// getSettings should return updated cached value
|
||||
const second = await store.getSettings();
|
||||
expect(second.themeMode).toBe("light");
|
||||
});
|
||||
|
||||
it("invalidateCache() forces re-read from disk", async () => {
|
||||
await store.init();
|
||||
await writeFile(join(dir, "settings.json"), JSON.stringify({ themeMode: "light" }));
|
||||
|
||||
// First call reads from disk
|
||||
const first = await store.getSettings();
|
||||
expect(first.themeMode).toBe("light");
|
||||
|
||||
// Modify the file externally
|
||||
await writeFile(join(dir, "settings.json"), JSON.stringify({ themeMode: "system" }));
|
||||
|
||||
// Without invalidation, should return cached value
|
||||
const cached = await store.getSettings();
|
||||
expect(cached.themeMode).toBe("light");
|
||||
|
||||
// After invalidation, should re-read from disk
|
||||
store.invalidateCache();
|
||||
const afterInvalidate = await store.getSettings();
|
||||
expect(afterInvalidate.themeMode).toBe("system");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -59,6 +59,9 @@ export class GlobalSettingsStore {
|
||||
private readonly settingsPath: string;
|
||||
private readonly dir: string;
|
||||
|
||||
/** Write-through cache for settings. Invalidated on every updateSettings() call. */
|
||||
private cachedSettings: GlobalSettings | null = null;
|
||||
|
||||
/** Promise chain for serializing read-modify-write cycles */
|
||||
private lock: Promise<void> = Promise.resolve();
|
||||
|
||||
@@ -105,12 +108,19 @@ export class GlobalSettingsStore {
|
||||
}
|
||||
|
||||
/**
|
||||
* Read global settings from disk. Returns defaults merged with persisted values.
|
||||
* Read global settings. Returns cached value if available, otherwise reads
|
||||
* from disk and caches the result. This avoids repeated filesystem reads for
|
||||
* settings that are accessed frequently.
|
||||
*
|
||||
* If the file doesn't exist or is invalid, returns defaults without throwing.
|
||||
*/
|
||||
async getSettings(): Promise<GlobalSettings> {
|
||||
if (this.cachedSettings !== null) {
|
||||
return this.cachedSettings;
|
||||
}
|
||||
const parsed = await this.readRaw();
|
||||
return { ...DEFAULT_GLOBAL_SETTINGS, ...parsed } as GlobalSettings;
|
||||
this.cachedSettings = { ...DEFAULT_GLOBAL_SETTINGS, ...parsed } as GlobalSettings;
|
||||
return this.cachedSettings;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -130,7 +140,9 @@ export class GlobalSettingsStore {
|
||||
const merged = { ...DEFAULT_GLOBAL_SETTINGS, ...raw, ...patch };
|
||||
await mkdir(this.dir, { recursive: true });
|
||||
await this.atomicWrite(merged as GlobalSettings);
|
||||
return merged as GlobalSettings;
|
||||
// Update the write-through cache
|
||||
this.cachedSettings = merged as GlobalSettings;
|
||||
return this.cachedSettings;
|
||||
});
|
||||
}
|
||||
|
||||
@@ -141,6 +153,15 @@ export class GlobalSettingsStore {
|
||||
return this.settingsPath;
|
||||
}
|
||||
|
||||
/**
|
||||
* Invalidate the in-memory cache. Forces the next getSettings() call to
|
||||
* re-read from disk. Useful for testing and edge cases where external
|
||||
* processes modify the settings file.
|
||||
*/
|
||||
invalidateCache(): void {
|
||||
this.cachedSettings = null;
|
||||
}
|
||||
|
||||
// ── Private helpers ─────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
|
||||
@@ -872,6 +872,62 @@ describe("TaskStore", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("getSettingsFast()", () => {
|
||||
it("returns the same merged result as getSettings()", async () => {
|
||||
await store.updateGlobalSettings({ themeMode: "light", ntfyEnabled: true });
|
||||
await store.updateSettings({ maxConcurrent: 5, autoMerge: false });
|
||||
|
||||
const fast = await store.getSettingsFast();
|
||||
const regular = await store.getSettings();
|
||||
|
||||
expect(fast.maxConcurrent).toBe(5);
|
||||
expect(fast.autoMerge).toBe(false);
|
||||
expect(fast.themeMode).toBe("light");
|
||||
expect(fast.ntfyEnabled).toBe(true);
|
||||
expect(fast.maxWorktrees).toBe(4); // default
|
||||
|
||||
// Should match getSettings()
|
||||
expect(fast).toEqual(regular);
|
||||
});
|
||||
|
||||
it("returns defaults when no config row exists", async () => {
|
||||
// Delete the config row
|
||||
const db = (store as any).db;
|
||||
db.prepare("DELETE FROM config WHERE id = 1").run();
|
||||
|
||||
const settings = await store.getSettingsFast();
|
||||
|
||||
// Should return defaults merged with global settings
|
||||
expect(settings.maxWorktrees).toBe(4); // default
|
||||
expect(settings.pollIntervalMs).toBe(15000); // default
|
||||
// Global settings should still be present
|
||||
expect(settings.themeMode).toBe("dark"); // global default
|
||||
});
|
||||
|
||||
it("includes global settings merged with project settings", async () => {
|
||||
await store.updateGlobalSettings({ themeMode: "system", colorTheme: "ocean" });
|
||||
await store.updateSettings({ maxConcurrent: 10 });
|
||||
|
||||
const settings = await store.getSettingsFast();
|
||||
|
||||
// Project settings override
|
||||
expect(settings.maxConcurrent).toBe(10);
|
||||
// Global settings are included
|
||||
expect(settings.themeMode).toBe("system");
|
||||
expect(settings.colorTheme).toBe("ocean");
|
||||
});
|
||||
|
||||
it("does not call listWorkflowSteps (fast-path)", async () => {
|
||||
await store.updateSettings({ maxConcurrent: 3 });
|
||||
|
||||
const settings = await store.getSettingsFast();
|
||||
|
||||
// If we got here without errors, the fast path works
|
||||
expect(settings.maxConcurrent).toBe(3);
|
||||
// listWorkflowSteps should not be called by getSettingsFast
|
||||
});
|
||||
});
|
||||
|
||||
// ── Prompt Overrides Tests ─────────────────────────────────────────
|
||||
|
||||
describe("promptOverrides settings", () => {
|
||||
|
||||
@@ -605,6 +605,31 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Fast-path settings read that skips the expensive workflow steps query.
|
||||
*
|
||||
* This method reads only the `settings` column from the SQLite config row
|
||||
* (avoiding `readConfig()` which always calls `listWorkflowSteps()`), and
|
||||
* uses the cached global settings from `GlobalSettingsStore`. Use this for
|
||||
* read-heavy paths like the settings page that don't need workflow steps.
|
||||
*
|
||||
* Note: Do NOT use this method when you need workflow steps — use `getSettings()` instead.
|
||||
*/
|
||||
async getSettingsFast(): Promise<Settings> {
|
||||
const [globalSettings, row] = await Promise.all([
|
||||
this.globalSettingsStore.getSettings(),
|
||||
this.db.prepare("SELECT settings FROM config WHERE id = 1").get() as { settings?: string } | undefined,
|
||||
]);
|
||||
|
||||
const projectSettings = row?.settings ? fromJson<Settings>(row.settings) : undefined;
|
||||
|
||||
return {
|
||||
...DEFAULT_SETTINGS,
|
||||
...globalSettings,
|
||||
...projectSettings,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Get settings separated by scope. Returns both the global and
|
||||
* project-level settings independently (useful for the UI to show
|
||||
|
||||
@@ -70,6 +70,7 @@ function createMockStore(overrides: Partial<TaskStore> = {}): TaskStore {
|
||||
archiveTask: vi.fn(),
|
||||
unarchiveTask: vi.fn(),
|
||||
getSettings: vi.fn().mockResolvedValue({}),
|
||||
getSettingsFast: vi.fn().mockResolvedValue({}),
|
||||
updateSettings: vi.fn(),
|
||||
updateGlobalSettings: vi.fn(),
|
||||
getSettingsByScope: vi.fn().mockResolvedValue({ global: {}, project: {} }),
|
||||
@@ -253,7 +254,7 @@ describe("Standardized error responses", () => {
|
||||
|
||||
it("returns 500 errors as { error } and logs to console.error", async () => {
|
||||
const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined);
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockRejectedValueOnce(new Error("Config read failed"));
|
||||
(store.getSettingsFast as ReturnType<typeof vi.fn>).mockRejectedValueOnce(new Error("Config read failed"));
|
||||
|
||||
const res = await GET(buildApp(), "/api/settings");
|
||||
|
||||
@@ -8593,7 +8594,7 @@ describe("GET /settings", () => {
|
||||
|
||||
it("returns persisted settings merged with defaults", async () => {
|
||||
const persistedSettings = { maxConcurrent: 5, autoMerge: false };
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({ ...DEFAULT_SETTINGS, ...persistedSettings });
|
||||
(store.getSettingsFast as ReturnType<typeof vi.fn>).mockResolvedValue({ ...DEFAULT_SETTINGS, ...persistedSettings });
|
||||
|
||||
const res = await GET(buildApp(), "/api/settings");
|
||||
|
||||
@@ -8604,7 +8605,7 @@ describe("GET /settings", () => {
|
||||
});
|
||||
|
||||
it("injects githubTokenConfigured as true when token is configured", async () => {
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue(DEFAULT_SETTINGS);
|
||||
(store.getSettingsFast as ReturnType<typeof vi.fn>).mockResolvedValue(DEFAULT_SETTINGS);
|
||||
|
||||
const res = await GET(buildApp(), "/api/settings");
|
||||
|
||||
@@ -8617,7 +8618,7 @@ describe("GET /settings", () => {
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(store)); // no githubToken option
|
||||
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue(DEFAULT_SETTINGS);
|
||||
(store.getSettingsFast as ReturnType<typeof vi.fn>).mockResolvedValue(DEFAULT_SETTINGS);
|
||||
|
||||
const res = await GET(app, "/api/settings");
|
||||
|
||||
@@ -8626,7 +8627,7 @@ describe("GET /settings", () => {
|
||||
});
|
||||
|
||||
it("returns 500 on store error", async () => {
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockRejectedValue(new Error("Config read failed"));
|
||||
(store.getSettingsFast as ReturnType<typeof vi.fn>).mockRejectedValue(new Error("Config read failed"));
|
||||
|
||||
const res = await GET(buildApp(), "/api/settings");
|
||||
|
||||
@@ -8912,7 +8913,7 @@ describe("PUT /settings", () => {
|
||||
expect(updateRes.status).toBe(200);
|
||||
|
||||
// Then, verify GET /settings returns the persisted values
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue(updatedSettings);
|
||||
(store.getSettingsFast as ReturnType<typeof vi.fn>).mockResolvedValue(updatedSettings);
|
||||
const getRes = await GET(buildApp(), "/api/settings");
|
||||
|
||||
expect(getRes.status).toBe(200);
|
||||
|
||||
@@ -1536,7 +1536,7 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
||||
router.get("/settings", async (req, res) => {
|
||||
try {
|
||||
const scopedStore = await getScopedStore(req);
|
||||
const settings = await scopedStore.getSettings();
|
||||
const settings = await scopedStore.getSettingsFast();
|
||||
// Inject server-side configuration flags
|
||||
res.json({
|
||||
...settings,
|
||||
|
||||
Reference in New Issue
Block a user