From 6d266fc7e89d7e7ddee3ec4235152e9a9bcaba0d Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Thu, 30 Apr 2026 15:37:17 -0400 Subject: [PATCH] fix: remove raw write re-exports from facade (cache bypass) PR-Agent #1150 review flagged that re-exporting saveUnifiedConfig/mutateUnifiedConfig/updateUnifiedConfig allows callers to bypass the cache. Only export the cache-coherent wrappers (saveConfig/mutateConfig/updateConfig). Raw functions still available via direct import from unified-config-loader if needed. - Remove raw write re-exports from facade - Add test verifying raw writes are NOT exported - Add test verifying cache-coherent wrappers ARE exported --- .../__tests__/config-loader-facade.test.ts | 21 ++++++++++++++++--- src/config/config-loader-facade.ts | 14 +++++++------ 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/src/config/__tests__/config-loader-facade.test.ts b/src/config/__tests__/config-loader-facade.test.ts index aff186a5..837ce030 100644 --- a/src/config/__tests__/config-loader-facade.test.ts +++ b/src/config/__tests__/config-loader-facade.test.ts @@ -63,9 +63,6 @@ describe('config-loader-facade', () => { expect(typeof facade.loadUnifiedConfig).toBe('function'); expect(typeof facade.loadOrCreateUnifiedConfig).toBe('function'); - expect(typeof facade.saveUnifiedConfig).toBe('function'); - expect(typeof facade.mutateUnifiedConfig).toBe('function'); - expect(typeof facade.updateUnifiedConfig).toBe('function'); }); it('should export all path/format utilities', async () => { @@ -115,6 +112,24 @@ describe('config-loader-facade', () => { }); }); + describe('cache coherence', () => { + it('should NOT export raw write functions that bypass cache', async () => { + const facade = (await importFacade()) as Record; + + expect(facade.saveUnifiedConfig).toBeUndefined(); + expect(facade.mutateUnifiedConfig).toBeUndefined(); + expect(facade.updateUnifiedConfig).toBeUndefined(); + }); + + it('should export cache-coherent write wrappers instead', async () => { + const facade = await importFacade(); + + expect(typeof facade.saveConfig).toBe('function'); + expect(typeof facade.mutateConfig).toBe('function'); + expect(typeof facade.updateConfig).toBe('function'); + }); + }); + describe('memoization', () => { it('getCachedConfig returns equivalent config on repeated calls', async () => { const facade = await importFacade(); diff --git a/src/config/config-loader-facade.ts b/src/config/config-loader-facade.ts index b3fee310..e6b17ad2 100644 --- a/src/config/config-loader-facade.ts +++ b/src/config/config-loader-facade.ts @@ -2,21 +2,23 @@ * Config Loader Facade * * Single import path for all config loading operations. - * Re-exports everything from unified-config-loader and config-manager, - * and adds memoization for loadOrCreateUnifiedConfig to reduce file I/O. + * Re-exports read-only functions from unified-config-loader and config-manager, + * and provides cache-coherent write wrappers that keep the memoization cache in sync. + * + * IMPORTANT: Raw write functions (saveUnifiedConfig, mutateUnifiedConfig, + * updateUnifiedConfig) are NOT re-exported here. Use the cache-coherent + * wrappers (saveConfig, mutateConfig, updateConfig) instead. If you need + * the raw functions, import directly from './unified-config-loader'. * * Usage: * import { getCachedConfig, saveConfig, mutateConfig } from '../config/config-loader-facade'; * import { getCcsDir, loadSettings } from '../config/config-loader-facade'; */ -// Re-export all functions from unified-config-loader +// Re-export read-only functions from unified-config-loader export { loadUnifiedConfig, loadOrCreateUnifiedConfig, - saveUnifiedConfig, - mutateUnifiedConfig, - updateUnifiedConfig, getConfigYamlPath, getConfigJsonPath, hasUnifiedConfig,