From 759f28911958689ff2b56f03409545b6fec81dec Mon Sep 17 00:00:00 2001
From: kaitranntt <kaitran.ntt@gmail.com>
Date: Thu, 5 Feb 2026 15:09:47 -0500
Subject: [PATCH] fix(cliproxy): fix discoverExistingAccounts test failures

- Skip saving registry when no new accounts discovered (prevents empty
  accounts.json creation for invalid files)
- Skip merging empty provider sections to prevent empty provider entries
- Update test module cache clearing to include new modular submodules
  (accounts/registry, accounts/index) for proper test isolation
---
 src/cliproxy/accounts/registry.ts              | 14 ++++++++++++++
 .../cliproxy/account-manager-discover.test.js  | 18 +++++++++++++++---
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/src/cliproxy/accounts/registry.ts b/src/cliproxy/accounts/registry.ts
index 2e885d5e..553864d5 100644
--- a/src/cliproxy/accounts/registry.ts
+++ b/src/cliproxy/accounts/registry.ts
@@ -402,6 +402,9 @@ export function discoverExistingAccounts(): void {
   const registry = loadAccountsRegistry();
   const files = fs.readdirSync(authDir);
 
+  // Track whether any accounts were discovered (to avoid saving empty registry)
+  let discoveredCount = 0;
+
   for (const file of files) {
     if (!file.endsWith('.json')) continue;
 
@@ -467,6 +470,7 @@ export function discoverExistingAccounts(): void {
           // Update if missing or changed
           if (existingEntry && existingEntry[1].projectId !== projectIdValue) {
             existingEntry[1].projectId = projectIdValue;
+            discoveredCount++; // Count projectId updates as changes
           }
         }
         continue;
@@ -529,17 +533,27 @@ export function discoverExistingAccounts(): void {
       }
 
       providerAccounts.accounts[accountId] = accountMeta;
+      discoveredCount++;
     } catch {
       // Skip invalid files
       continue;
     }
   }
 
+  // Only save if at least one account was discovered or updated
+  // This prevents creating accounts.json with empty provider sections
+  if (discoveredCount === 0) {
+    return;
+  }
+
   // Reload-merge pattern: reduce race condition with concurrent OAuth registration
   // Reload fresh registry and merge discovered accounts (fresh registry wins on conflicts)
   const freshRegistry = loadAccountsRegistry();
   for (const [providerName, discovered] of Object.entries(registry.providers)) {
     if (!discovered) continue;
+    // Skip empty provider sections (no accounts discovered)
+    if (Object.keys(discovered.accounts).length === 0) continue;
+
     const prov = providerName as CLIProxyProvider;
     if (!freshRegistry.providers[prov]) {
       freshRegistry.providers[prov] = discovered;
diff --git a/tests/unit/cliproxy/account-manager-discover.test.js b/tests/unit/cliproxy/account-manager-discover.test.js
index 3f48f1e1..1a93f117 100644
--- a/tests/unit/cliproxy/account-manager-discover.test.js
+++ b/tests/unit/cliproxy/account-manager-discover.test.js
@@ -28,10 +28,18 @@ describe('Account Manager - discoverExistingAccounts', () => {
     originalCcsHome = process.env.CCS_HOME;
     process.env.CCS_HOME = testDir;
 
-    // Clear cache and reload
+    // Clear cache and reload - must clear all modular submodules too
     delete require.cache[require.resolve('../../../dist/cliproxy/account-manager')];
     delete require.cache[require.resolve('../../../dist/cliproxy/config-generator')];
     delete require.cache[require.resolve('../../../dist/utils/config-manager')];
+    // Clear new modular structure (accounts/ and config/ submodules)
+    delete require.cache[require.resolve('../../../dist/cliproxy/accounts/index')];
+    delete require.cache[require.resolve('../../../dist/cliproxy/accounts/registry')];
+    delete require.cache[require.resolve('../../../dist/cliproxy/accounts/token-file-ops')];
+    delete require.cache[require.resolve('../../../dist/cliproxy/accounts/query')];
+    delete require.cache[require.resolve('../../../dist/cliproxy/accounts/types')];
+    delete require.cache[require.resolve('../../../dist/cliproxy/config/index')];
+    delete require.cache[require.resolve('../../../dist/cliproxy/config/path-resolver')];
     accountManager = require('../../../dist/cliproxy/account-manager');
   });
 
@@ -247,8 +255,10 @@ describe('Account Manager - discoverExistingAccounts', () => {
         email: '',
       });
 
-      // Reload module to clear in-memory cache
+      // Reload module to clear in-memory cache (including all submodules)
       delete require.cache[require.resolve('../../../dist/cliproxy/account-manager')];
+      delete require.cache[require.resolve('../../../dist/cliproxy/accounts/index')];
+      delete require.cache[require.resolve('../../../dist/cliproxy/accounts/registry')];
       accountManager = require('../../../dist/cliproxy/account-manager');
       accountManager.discoverExistingAccounts();
 
@@ -329,8 +339,10 @@ describe('Account Manager - discoverExistingAccounts', () => {
         email: '',
       });
 
-      // Reload module to pick up pre-existing accounts.json
+      // Reload module to pick up pre-existing accounts.json (including all submodules)
       delete require.cache[require.resolve('../../../dist/cliproxy/account-manager')];
+      delete require.cache[require.resolve('../../../dist/cliproxy/accounts/index')];
+      delete require.cache[require.resolve('../../../dist/cliproxy/accounts/registry')];
       accountManager = require('../../../dist/cliproxy/account-manager');
       accountManager.discoverExistingAccounts();