fix: clearcut logging (retry #3744) (#3751)

packages/core/src/utils/paths.ts

@@ -9,6 +9,7 @@ import os from 'os';
 import * as crypto from 'crypto';
 
 export const GEMINI_DIR = '.gemini';
+export const GOOGLE_ACCOUNTS_FILENAME = 'google_accounts.json';
 const TMP_DIR_NAME = 'tmp';
 
 /**

packages/core/src/utils/user_account.test.ts

@@ -0,0 +1,237 @@
+/**
+ * @license
+ * Copyright 2025 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { vi, describe, it, expect, beforeEach, afterEach, Mock } from 'vitest';
+import {
+  cacheGoogleAccount,
+  getCachedGoogleAccount,
+  clearCachedGoogleAccount,
+  getLifetimeGoogleAccounts,
+} from './user_account.js';
+import * as fs from 'node:fs';
+import * as os from 'node:os';
+import path from 'node:path';
+
+vi.mock('os', async (importOriginal) => {
+  const os = await importOriginal<typeof import('os')>();
+  return {
+    ...os,
+    homedir: vi.fn(),
+  };
+});
+
+describe('user_account', () => {
+  let tempHomeDir: string;
+  const accountsFile = () =>
+    path.join(tempHomeDir, '.gemini', 'google_accounts.json');
+  beforeEach(() => {
+    tempHomeDir = fs.mkdtempSync(
+      path.join(os.tmpdir(), 'gemini-cli-test-home-'),
+    );
+    (os.homedir as Mock).mockReturnValue(tempHomeDir);
+  });
+  afterEach(() => {
+    fs.rmSync(tempHomeDir, { recursive: true, force: true });
+    vi.clearAllMocks();
+  });
+
+  describe('cacheGoogleAccount', () => {
+    it('should create directory and write initial account file', async () => {
+      await cacheGoogleAccount('test1@google.com');
+
+      // Verify Google Account ID was cached
+      expect(fs.existsSync(accountsFile())).toBe(true);
+      expect(fs.readFileSync(accountsFile(), 'utf-8')).toBe(
+        JSON.stringify({ active: 'test1@google.com', old: [] }, null, 2),
+      );
+    });
+
+    it('should update active account and move previous to old', async () => {
+      fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
+      fs.writeFileSync(
+        accountsFile(),
+        JSON.stringify(
+          { active: 'test2@google.com', old: ['test1@google.com'] },
+          null,
+          2,
+        ),
+      );
+
+      await cacheGoogleAccount('test3@google.com');
+
+      expect(fs.readFileSync(accountsFile(), 'utf-8')).toBe(
+        JSON.stringify(
+          {
+            active: 'test3@google.com',
+            old: ['test1@google.com', 'test2@google.com'],
+          },
+          null,
+          2,
+        ),
+      );
+    });
+
+    it('should not add a duplicate to the old list', async () => {
+      fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
+      fs.writeFileSync(
+        accountsFile(),
+        JSON.stringify(
+          { active: 'test1@google.com', old: ['test2@google.com'] },
+          null,
+          2,
+        ),
+      );
+      await cacheGoogleAccount('test2@google.com');
+      await cacheGoogleAccount('test1@google.com');
+
+      expect(fs.readFileSync(accountsFile(), 'utf-8')).toBe(
+        JSON.stringify(
+          { active: 'test1@google.com', old: ['test2@google.com'] },
+          null,
+          2,
+        ),
+      );
+    });
+
+    it('should handle corrupted JSON by starting fresh', async () => {
+      fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
+      fs.writeFileSync(accountsFile(), 'not valid json');
+      const consoleDebugSpy = vi
+        .spyOn(console, 'debug')
+        .mockImplementation(() => {});
+
+      await cacheGoogleAccount('test1@google.com');
+
+      expect(consoleDebugSpy).toHaveBeenCalled();
+      expect(JSON.parse(fs.readFileSync(accountsFile(), 'utf-8'))).toEqual({
+        active: 'test1@google.com',
+        old: [],
+      });
+    });
+  });
+
+  describe('getCachedGoogleAccount', () => {
+    it('should return the active account if file exists and is valid', () => {
+      fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
+      fs.writeFileSync(
+        accountsFile(),
+        JSON.stringify({ active: 'active@google.com', old: [] }, null, 2),
+      );
+      const account = getCachedGoogleAccount();
+      expect(account).toBe('active@google.com');
+    });
+
+    it('should return null if file does not exist', () => {
+      const account = getCachedGoogleAccount();
+      expect(account).toBeNull();
+    });
+
+    it('should return null if file is empty', () => {
+      fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
+      fs.writeFileSync(accountsFile(), '');
+      const account = getCachedGoogleAccount();
+      expect(account).toBeNull();
+    });
+
+    it('should return null and log if file is corrupted', () => {
+      fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
+      fs.writeFileSync(accountsFile(), '{ "active": "test@google.com"'); // Invalid JSON
+      const consoleDebugSpy = vi
+        .spyOn(console, 'debug')
+        .mockImplementation(() => {});
+
+      const account = getCachedGoogleAccount();
+
+      expect(account).toBeNull();
+      expect(consoleDebugSpy).toHaveBeenCalled();
+    });
+  });
+
+  describe('clearCachedGoogleAccount', () => {
+    it('should set active to null and move it to old', async () => {
+      fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
+      fs.writeFileSync(
+        accountsFile(),
+        JSON.stringify(
+          { active: 'active@google.com', old: ['old1@google.com'] },
+          null,
+          2,
+        ),
+      );
+
+      await clearCachedGoogleAccount();
+
+      const stored = JSON.parse(fs.readFileSync(accountsFile(), 'utf-8'));
+      expect(stored.active).toBeNull();
+      expect(stored.old).toEqual(['old1@google.com', 'active@google.com']);
+    });
+
+    it('should handle empty file gracefully', async () => {
+      fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
+      fs.writeFileSync(accountsFile(), '');
+      await clearCachedGoogleAccount();
+      const stored = JSON.parse(fs.readFileSync(accountsFile(), 'utf-8'));
+      expect(stored.active).toBeNull();
+      expect(stored.old).toEqual([]);
+    });
+  });
+
+  describe('getLifetimeGoogleAccounts', () => {
+    it('should return 0 if the file does not exist', () => {
+      expect(getLifetimeGoogleAccounts()).toBe(0);
+    });
+
+    it('should return 0 if the file is empty', () => {
+      fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
+      fs.writeFileSync(accountsFile(), '');
+      expect(getLifetimeGoogleAccounts()).toBe(0);
+    });
+
+    it('should return 0 if the file is corrupted', () => {
+      fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
+      fs.writeFileSync(accountsFile(), 'invalid json');
+      const consoleDebugSpy = vi
+        .spyOn(console, 'debug')
+        .mockImplementation(() => {});
+
+      expect(getLifetimeGoogleAccounts()).toBe(0);
+      expect(consoleDebugSpy).toHaveBeenCalled();
+    });
+
+    it('should return 1 if there is only an active account', () => {
+      fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
+      fs.writeFileSync(
+        accountsFile(),
+        JSON.stringify({ active: 'test1@google.com', old: [] }),
+      );
+      expect(getLifetimeGoogleAccounts()).toBe(1);
+    });
+
+    it('should correctly count old accounts when active is null', () => {
+      fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
+      fs.writeFileSync(
+        accountsFile(),
+        JSON.stringify({
+          active: null,
+          old: ['test1@google.com', 'test2@google.com'],
+        }),
+      );
+      expect(getLifetimeGoogleAccounts()).toBe(2);
+    });
+
+    it('should correctly count both active and old accounts', () => {
+      fs.mkdirSync(path.dirname(accountsFile()), { recursive: true });
+      fs.writeFileSync(
+        accountsFile(),
+        JSON.stringify({
+          active: 'test3@google.com',
+          old: ['test1@google.com', 'test2@google.com'],
+        }),
+      );
+      expect(getLifetimeGoogleAccounts()).toBe(3);
+    });
+  });
+});

packages/core/src/utils/user_account.ts

@@ -0,0 +1,115 @@
+/**
+ * @license
+ * Copyright 2025 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import path from 'node:path';
+import { promises as fsp, existsSync, readFileSync } from 'node:fs';
+import * as os from 'os';
+import { GEMINI_DIR, GOOGLE_ACCOUNTS_FILENAME } from './paths.js';
+
+interface UserAccounts {
+  active: string | null;
+  old: string[];
+}
+
+function getGoogleAccountsCachePath(): string {
+  return path.join(os.homedir(), GEMINI_DIR, GOOGLE_ACCOUNTS_FILENAME);
+}
+
+async function readAccounts(filePath: string): Promise<UserAccounts> {
+  try {
+    const content = await fsp.readFile(filePath, 'utf-8');
+    if (!content.trim()) {
+      return { active: null, old: [] };
+    }
+    return JSON.parse(content) as UserAccounts;
+  } catch (error) {
+    if (error instanceof Error && 'code' in error && error.code === 'ENOENT') {
+      // File doesn't exist, which is fine.
+      return { active: null, old: [] };
+    }
+    // File is corrupted or not valid JSON, start with a fresh object.
+    console.debug('Could not parse accounts file, starting fresh.', error);
+    return { active: null, old: [] };
+  }
+}
+
+export async function cacheGoogleAccount(email: string): Promise<void> {
+  const filePath = getGoogleAccountsCachePath();
+  await fsp.mkdir(path.dirname(filePath), { recursive: true });
+
+  const accounts = await readAccounts(filePath);
+
+  if (accounts.active && accounts.active !== email) {
+    if (!accounts.old.includes(accounts.active)) {
+      accounts.old.push(accounts.active);
+    }
+  }
+
+  // If the new email was in the old list, remove it
+  accounts.old = accounts.old.filter((oldEmail) => oldEmail !== email);
+
+  accounts.active = email;
+  await fsp.writeFile(filePath, JSON.stringify(accounts, null, 2), 'utf-8');
+}
+
+export function getCachedGoogleAccount(): string | null {
+  try {
+    const filePath = getGoogleAccountsCachePath();
+    if (existsSync(filePath)) {
+      const content = readFileSync(filePath, 'utf-8').trim();
+      if (!content) {
+        return null;
+      }
+      const accounts: UserAccounts = JSON.parse(content);
+      return accounts.active;
+    }
+    return null;
+  } catch (error) {
+    console.debug('Error reading cached Google Account:', error);
+    return null;
+  }
+}
+
+export function getLifetimeGoogleAccounts(): number {
+  try {
+    const filePath = getGoogleAccountsCachePath();
+    if (!existsSync(filePath)) {
+      return 0;
+    }
+
+    const content = readFileSync(filePath, 'utf-8').trim();
+    if (!content) {
+      return 0;
+    }
+    const accounts: UserAccounts = JSON.parse(content);
+    let count = accounts.old.length;
+    if (accounts.active) {
+      count++;
+    }
+    return count;
+  } catch (error) {
+    console.debug('Error reading lifetime Google Accounts:', error);
+    return 0;
+  }
+}
+
+export async function clearCachedGoogleAccount(): Promise<void> {
+  const filePath = getGoogleAccountsCachePath();
+  if (!existsSync(filePath)) {
+    return;
+  }
+
+  const accounts = await readAccounts(filePath);
+
+  if (accounts.active) {
+    if (!accounts.old.includes(accounts.active)) {
+      accounts.old.push(accounts.active);
+    }
+    accounts.active = null;
+  }
+
+  await fsp.writeFile(filePath, JSON.stringify(accounts, null, 2), 'utf-8');
+}

packages/core/src/utils/user_id.test.ts

@@ -5,7 +5,7 @@
  */
 
 import { describe, it, expect } from 'vitest';
-import { getInstallationId, getGoogleAccountId } from './user_id.js';
+import { getInstallationId } from './user_id.js';
 
 describe('user_id', () => {
   describe('getInstallationId', () => {
@@ -21,31 +21,4 @@ describe('user_id', () => {
       expect(secondCall).toBe(installationId);
     });
   });
-
-  describe('getGoogleAccountId', () => {
-    it('should return a non-empty string', async () => {
-      const result = await getGoogleAccountId();
-
-      expect(result).toBeDefined();
-      expect(typeof result).toBe('string');
-
-      // Should be consistent on subsequent calls
-      const secondCall = await getGoogleAccountId();
-      expect(secondCall).toBe(result);
-    });
-
-    it('should return empty string when no Google Account ID is cached, or a valid ID when cached', async () => {
-      // The function can return either an empty string (if no cached ID) or a valid Google Account ID (if cached)
-      const googleAccountIdResult = await getGoogleAccountId();
-
-      expect(googleAccountIdResult).toBeDefined();
-      expect(typeof googleAccountIdResult).toBe('string');
-
-      // Should be either empty string or a numeric string (Google Account ID)
-      if (googleAccountIdResult !== '') {
-        // If we have a cached ID, it should be a numeric string
-        expect(googleAccountIdResult).toMatch(/^\d+$/);
-      }
-    });
-  });
 });

packages/core/src/utils/user_id.ts

@@ -56,27 +56,3 @@ export function getInstallationId(): string {
     return '123456789';
   }
 }
-
-/**
- * Retrieves the obfuscated Google Account ID for the currently authenticated user.
- * When OAuth is available, returns the user's cached Google Account ID. Otherwise, returns the installation ID.
- * @returns A string ID for the user (Google Account ID if available, otherwise installation ID).
- */
-export async function getGoogleAccountId(): Promise<string> {
-  // Try to get cached Google Account ID first
-  try {
-    // Dynamic import to avoid circular dependencies
-    const { getCachedGoogleAccountId } = await import(
-      '../code_assist/oauth2.js'
-    );
-    const googleAccountId = getCachedGoogleAccountId();
-    if (googleAccountId) {
-      return googleAccountId;
-    }
-  } catch (error) {
-    // If there's any error accessing Google Account ID, just return empty string
-    console.debug('Could not get cached Google Account ID:', error);
-  }
-
-  return '';
-}