From d6a8fb3912c7ecb4b293e57101d99d75d91e8a46 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 8 May 2026 09:30:37 +0000 Subject: [PATCH 1/2] =?UTF-8?q?=E2=9A=A1=20optimize=20saveConfig=20to=20us?= =?UTF-8?q?e=20asynchronous=20I/O?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace synchronous `fs` methods with `fs.promises` in `lib/config.js`. - Refactor `saveConfig`, `loadConfig`, `saveTokenCache`, `loadTokenCache`, `clearTokenCache`, and `migrateOldConfig` to be fully asynchronous. - Ensure secure file permissions (0o600) and directory permissions (0o700) are maintained using `fs.promises.chmod` and `mkdirp` modes. - Update `tests/config.test.js` to support and verify asynchronous file operations. - Improve event loop responsiveness by avoiding blocking I/O in the CLI's async flows. Co-authored-by: cmuench <211294+cmuench@users.noreply.github.com> --- lib/config.js | 23 +++++++++--------- tests/config.test.js | 55 +++++++++++++++++++++++++------------------- 2 files changed, 43 insertions(+), 35 deletions(-) diff --git a/lib/config.js b/lib/config.js index a74025d..4b4c367 100644 --- a/lib/config.js +++ b/lib/config.js @@ -30,8 +30,9 @@ async function migrateOldConfig() { if (fs.existsSync(OLD_CONFIG_FILE)) { try { console.log(`Migrating config from ${OLD_CONFIG_FILE} to ${CONFIG_FILE}`); - await mkdirp(CONFIG_DIR); - fs.copyFileSync(OLD_CONFIG_FILE, CONFIG_FILE); + await mkdirp(CONFIG_DIR, { mode: 0o700 }); + await fs.promises.copyFile(OLD_CONFIG_FILE, CONFIG_FILE); + await fs.promises.chmod(CONFIG_FILE, 0o600); } catch (e) { console.error("Error migrating config:", e.message); } @@ -44,7 +45,7 @@ export async function loadConfig() { if (!fs.existsSync(CONFIG_FILE)) { return { $schema: "https://mage-remote-run.muench.dev/config.schema.json", profiles: {}, activeProfile: null, plugins: [] }; } - const data = fs.readFileSync(CONFIG_FILE, 'utf-8'); + const data = await fs.promises.readFile(CONFIG_FILE, 'utf-8'); const config = JSON.parse(data); if (!config.plugins) { config.plugins = []; @@ -58,9 +59,9 @@ export async function loadConfig() { export async function saveConfig(config) { try { - await mkdirp(CONFIG_DIR); - fs.writeFileSync(CONFIG_FILE, JSON.stringify(config, null, 2), { mode: 0o600 }); - fs.chmodSync(CONFIG_FILE, 0o600); + await mkdirp(CONFIG_DIR, { mode: 0o700 }); + await fs.promises.writeFile(CONFIG_FILE, JSON.stringify(config, null, 2), { mode: 0o600 }); + await fs.promises.chmod(CONFIG_FILE, 0o600); if (process.env.DEBUG) { console.log(`Configuration saved to ${CONFIG_FILE}`); } @@ -98,7 +99,7 @@ export async function loadTokenCache() { if (!fs.existsSync(TOKEN_CACHE_FILE)) { return {}; } - const data = fs.readFileSync(TOKEN_CACHE_FILE, 'utf-8'); + const data = await fs.promises.readFile(TOKEN_CACHE_FILE, 'utf-8'); return JSON.parse(data); } catch (e) { console.error("Error loading token cache:", e.message); @@ -108,9 +109,9 @@ export async function loadTokenCache() { export async function saveTokenCache(cache) { try { - await mkdirp(CACHE_DIR); - fs.writeFileSync(TOKEN_CACHE_FILE, JSON.stringify(cache, null, 2), { mode: 0o600 }); - fs.chmodSync(TOKEN_CACHE_FILE, 0o600); + await mkdirp(CACHE_DIR, { mode: 0o700 }); + await fs.promises.writeFile(TOKEN_CACHE_FILE, JSON.stringify(cache, null, 2), { mode: 0o600 }); + await fs.promises.chmod(TOKEN_CACHE_FILE, 0o600); } catch (e) { console.error("Error saving token cache:", e.message); throw e; @@ -120,7 +121,7 @@ export async function saveTokenCache(cache) { export async function clearTokenCache() { try { if (fs.existsSync(TOKEN_CACHE_FILE)) { - fs.unlinkSync(TOKEN_CACHE_FILE); + await fs.promises.unlink(TOKEN_CACHE_FILE); } } catch (e) { console.error("Error clearing token cache:", e.message); diff --git a/tests/config.test.js b/tests/config.test.js index 58ccbfa..8e2bb01 100644 --- a/tests/config.test.js +++ b/tests/config.test.js @@ -22,7 +22,14 @@ jest.unstable_mockModule('fs', () => ({ writeFileSync: jest.fn(), copyFileSync: jest.fn(), chmodSync: jest.fn(), - unlinkSync: jest.fn() + unlinkSync: jest.fn(), + promises: { + readFile: jest.fn(), + writeFile: jest.fn(), + chmod: jest.fn(), + unlink: jest.fn(), + copyFile: jest.fn() + } } })); @@ -77,7 +84,7 @@ describe('Config Management', () => { it('should load and parse config file', async () => { fs.existsSync.mockReturnValue(true); - fs.readFileSync.mockReturnValue(JSON.stringify({ + fs.promises.readFile.mockResolvedValue(JSON.stringify({ profiles: { test: {} }, activeProfile: 'test' })); @@ -89,12 +96,12 @@ describe('Config Management', () => { activeProfile: 'test', plugins: [] }); - expect(fs.readFileSync).toHaveBeenCalledWith(EXPECTED_CONFIG_FILE, 'utf-8'); + expect(fs.promises.readFile).toHaveBeenCalledWith(EXPECTED_CONFIG_FILE, 'utf-8'); }); it('should handle JSON parse errors', async () => { fs.existsSync.mockReturnValue(true); - fs.readFileSync.mockReturnValue('invalid json'); + fs.promises.readFile.mockResolvedValue('invalid json'); const config = await configMod.loadConfig(); @@ -104,7 +111,7 @@ describe('Config Management', () => { it('should ensure plugins array exists', async () => { fs.existsSync.mockReturnValue(true); - fs.readFileSync.mockReturnValue(JSON.stringify({ + fs.promises.readFile.mockResolvedValue(JSON.stringify({ profiles: {} })); @@ -120,13 +127,13 @@ describe('Config Management', () => { await configMod.saveConfig(config); - expect(mkdirp).toHaveBeenCalledWith(EXPECTED_CONFIG_DIR); - expect(fs.writeFileSync).toHaveBeenCalledWith( + expect(mkdirp).toHaveBeenCalledWith(EXPECTED_CONFIG_DIR, { mode: 0o700 }); + expect(fs.promises.writeFile).toHaveBeenCalledWith( EXPECTED_CONFIG_FILE, JSON.stringify(config, null, 2), { mode: 0o600 } ); - expect(fs.chmodSync).toHaveBeenCalledWith(EXPECTED_CONFIG_FILE, 0o600); + expect(fs.promises.chmod).toHaveBeenCalledWith(EXPECTED_CONFIG_FILE, 0o600); }); it('should log debug message if DEBUG env var is set', async () => { @@ -163,14 +170,14 @@ describe('Config Management', () => { await configMod.addProfile('NewProfile', { url: 'http://test.com' }); // Verify saveConfig was called with correct data - expect(fs.writeFileSync).toHaveBeenCalledWith( + expect(fs.promises.writeFile).toHaveBeenCalledWith( EXPECTED_CONFIG_FILE, expect.stringContaining('"NewProfile":'), expect.anything() ); // Verify activeProfile was set - const saveCall = fs.writeFileSync.mock.calls[0]; + const saveCall = fs.promises.writeFile.mock.calls[0]; const savedConfig = JSON.parse(saveCall[1]); expect(savedConfig.activeProfile).toBe('NewProfile'); expect(savedConfig.profiles['NewProfile']).toEqual({ url: 'http://test.com' }); @@ -179,7 +186,7 @@ describe('Config Management', () => { it('should add profile but NOT change active if one exists', async () => { // Mock loadConfig to return existing profile fs.existsSync.mockReturnValue(true); - fs.readFileSync.mockReturnValue(JSON.stringify({ + fs.promises.readFile.mockResolvedValue(JSON.stringify({ profiles: { 'Existing': {} }, activeProfile: 'Existing', plugins: [] @@ -187,7 +194,7 @@ describe('Config Management', () => { await configMod.addProfile('NewProfile', { url: 'http://test.com' }); - const saveCall = fs.writeFileSync.mock.calls[0]; + const saveCall = fs.promises.writeFile.mock.calls[0]; const savedConfig = JSON.parse(saveCall[1]); expect(savedConfig.activeProfile).toBe('Existing'); expect(savedConfig.profiles['NewProfile']).toEqual({ url: 'http://test.com' }); @@ -197,7 +204,7 @@ describe('Config Management', () => { describe('getActiveProfile', () => { it('should return active profile with name', async () => { fs.existsSync.mockReturnValue(true); - fs.readFileSync.mockReturnValue(JSON.stringify({ + fs.promises.readFile.mockResolvedValue(JSON.stringify({ profiles: { 'MyProfile': { type: 'saas' } }, activeProfile: 'MyProfile' })); @@ -209,7 +216,7 @@ describe('Config Management', () => { it('should return null if no active profile set', async () => { fs.existsSync.mockReturnValue(true); - fs.readFileSync.mockReturnValue(JSON.stringify({ + fs.promises.readFile.mockResolvedValue(JSON.stringify({ profiles: { 'MyProfile': {} }, activeProfile: null })); @@ -221,7 +228,7 @@ describe('Config Management', () => { it('should return null if active profile does not exist in profiles', async () => { fs.existsSync.mockReturnValue(true); - fs.readFileSync.mockReturnValue(JSON.stringify({ + fs.promises.readFile.mockResolvedValue(JSON.stringify({ profiles: { 'Other': {} }, activeProfile: 'Missing' })); @@ -241,12 +248,12 @@ describe('Config Management', () => { describe('Token Cache', () => { it('should load token cache', async () => { fs.existsSync.mockReturnValue(true); - fs.readFileSync.mockReturnValue(JSON.stringify({ token: 'abc' })); + fs.promises.readFile.mockResolvedValue(JSON.stringify({ token: 'abc' })); const cache = await configMod.loadTokenCache(); expect(cache).toEqual({ token: 'abc' }); - expect(fs.readFileSync).toHaveBeenCalledWith(EXPECTED_TOKEN_CACHE_FILE, 'utf-8'); + expect(fs.promises.readFile).toHaveBeenCalledWith(EXPECTED_TOKEN_CACHE_FILE, 'utf-8'); }); it('should return empty object if token cache missing', async () => { @@ -259,7 +266,7 @@ describe('Config Management', () => { it('should handle errors loading token cache', async () => { fs.existsSync.mockReturnValue(true); - fs.readFileSync.mockReturnValue('invalid json'); + fs.promises.readFile.mockResolvedValue('invalid json'); const cache = await configMod.loadTokenCache(); @@ -271,13 +278,13 @@ describe('Config Management', () => { const cache = { token: 'abc' }; await configMod.saveTokenCache(cache); - expect(mkdirp).toHaveBeenCalledWith(EXPECTED_CACHE_DIR); - expect(fs.writeFileSync).toHaveBeenCalledWith( + expect(mkdirp).toHaveBeenCalledWith(EXPECTED_CACHE_DIR, { mode: 0o700 }); + expect(fs.promises.writeFile).toHaveBeenCalledWith( EXPECTED_TOKEN_CACHE_FILE, JSON.stringify(cache, null, 2), { mode: 0o600 } ); - expect(fs.chmodSync).toHaveBeenCalledWith(EXPECTED_TOKEN_CACHE_FILE, 0o600); + expect(fs.promises.chmod).toHaveBeenCalledWith(EXPECTED_TOKEN_CACHE_FILE, 0o600); }); it('should handle errors saving token cache', async () => { @@ -293,7 +300,7 @@ describe('Config Management', () => { await configMod.clearTokenCache(); - expect(fs.unlinkSync).toHaveBeenCalledWith(EXPECTED_TOKEN_CACHE_FILE); + expect(fs.promises.unlink).toHaveBeenCalledWith(EXPECTED_TOKEN_CACHE_FILE); }); it('should do nothing if token cache to clear does not exist', async () => { @@ -301,12 +308,12 @@ describe('Config Management', () => { await configMod.clearTokenCache(); - expect(fs.unlinkSync).not.toHaveBeenCalled(); + expect(fs.promises.unlink).not.toHaveBeenCalled(); }); it('should handle errors clearing token cache', async () => { fs.existsSync.mockReturnValue(true); - fs.unlinkSync.mockImplementation(() => { throw new Error('Unlink failed'); }); + fs.promises.unlink.mockImplementation(() => { throw new Error('Unlink failed'); }); await expect(configMod.clearTokenCache()).rejects.toThrow('Unlink failed'); expect(consoleErrorSpy).toHaveBeenCalledWith(expect.stringContaining('Error clearing token cache'), 'Unlink failed'); From c9affed99ab8a1b5e9fbd4cbdb485f042ad1aa7e Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sun, 17 May 2026 13:40:22 +0000 Subject: [PATCH 2/2] =?UTF-8?q?=E2=9A=A1=20optimize=20saveConfig=20to=20us?= =?UTF-8?q?e=20asynchronous=20I/O=20(resolved=20conflicts)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace synchronous `fs` methods with `fs.promises` in `lib/config.js`. - Refactor `saveConfig`, `loadConfig`, `saveTokenCache`, `loadTokenCache`, `clearTokenCache`, and `migrateOldConfig` to be fully asynchronous. - Ensure secure file permissions (0o600) and directory permissions (0o700) are maintained using `fs.promises.chmod` and `mkdirp` modes. - Update `tests/config.test.js` to support and verify asynchronous file operations. - Verified branch is up to date with origin/main. Co-authored-by: cmuench <211294+cmuench@users.noreply.github.com>