refactor: Extract path resolution logic into helper function (#6910)

packages/core/src/core/prompts.test.ts

@@ -5,7 +5,7 @@
  */
 
 import { describe, it, expect, vi, beforeEach } from 'vitest';
-import { getCoreSystemPrompt } from './prompts.js';
+import { getCoreSystemPrompt, resolvePathFromEnv } from './prompts.js';
 import { isGitRepository } from '../utils/gitUtils.js';
 import fs from 'node:fs';
 import os from 'node:os';
@@ -270,3 +270,153 @@ describe('Core System Prompt (prompts.ts)', () => {
     });
   });
 });
+
+describe('resolvePathFromEnv helper function', () => {
+  beforeEach(() => {
+    vi.resetAllMocks();
+  });
+
+  describe('when envVar is undefined, empty, or whitespace', () => {
+    it('should return null for undefined', () => {
+      const result = resolvePathFromEnv(undefined);
+      expect(result).toEqual({
+        isSwitch: false,
+        value: null,
+        isDisabled: false,
+      });
+    });
+
+    it('should return null for empty string', () => {
+      const result = resolvePathFromEnv('');
+      expect(result).toEqual({
+        isSwitch: false,
+        value: null,
+        isDisabled: false,
+      });
+    });
+
+    it('should return null for whitespace only', () => {
+      const result = resolvePathFromEnv('   \n\t  ');
+      expect(result).toEqual({
+        isSwitch: false,
+        value: null,
+        isDisabled: false,
+      });
+    });
+  });
+
+  describe('when envVar is a boolean-like string', () => {
+    it('should handle "0" as disabled switch', () => {
+      const result = resolvePathFromEnv('0');
+      expect(result).toEqual({
+        isSwitch: true,
+        value: '0',
+        isDisabled: true,
+      });
+    });
+
+    it('should handle "false" as disabled switch', () => {
+      const result = resolvePathFromEnv('false');
+      expect(result).toEqual({
+        isSwitch: true,
+        value: 'false',
+        isDisabled: true,
+      });
+    });
+
+    it('should handle "1" as enabled switch', () => {
+      const result = resolvePathFromEnv('1');
+      expect(result).toEqual({
+        isSwitch: true,
+        value: '1',
+        isDisabled: false,
+      });
+    });
+
+    it('should handle "true" as enabled switch', () => {
+      const result = resolvePathFromEnv('true');
+      expect(result).toEqual({
+        isSwitch: true,
+        value: 'true',
+        isDisabled: false,
+      });
+    });
+
+    it('should be case-insensitive for boolean values', () => {
+      expect(resolvePathFromEnv('FALSE')).toEqual({
+        isSwitch: true,
+        value: 'false',
+        isDisabled: true,
+      });
+      expect(resolvePathFromEnv('TRUE')).toEqual({
+        isSwitch: true,
+        value: 'true',
+        isDisabled: false,
+      });
+    });
+  });
+
+  describe('when envVar is a file path', () => {
+    it('should resolve absolute paths', () => {
+      const result = resolvePathFromEnv('/absolute/path/file.txt');
+      expect(result).toEqual({
+        isSwitch: false,
+        value: path.resolve('/absolute/path/file.txt'),
+        isDisabled: false,
+      });
+    });
+
+    it('should resolve relative paths', () => {
+      const result = resolvePathFromEnv('relative/path/file.txt');
+      expect(result).toEqual({
+        isSwitch: false,
+        value: path.resolve('relative/path/file.txt'),
+        isDisabled: false,
+      });
+    });
+
+    it('should expand tilde to home directory', () => {
+      const homeDir = '/Users/test';
+      vi.spyOn(os, 'homedir').mockReturnValue(homeDir);
+
+      const result = resolvePathFromEnv('~/documents/file.txt');
+      expect(result).toEqual({
+        isSwitch: false,
+        value: path.resolve(path.join(homeDir, 'documents/file.txt')),
+        isDisabled: false,
+      });
+    });
+
+    it('should handle standalone tilde', () => {
+      const homeDir = '/Users/test';
+      vi.spyOn(os, 'homedir').mockReturnValue(homeDir);
+
+      const result = resolvePathFromEnv('~');
+      expect(result).toEqual({
+        isSwitch: false,
+        value: path.resolve(homeDir),
+        isDisabled: false,
+      });
+    });
+
+    it('should handle os.homedir() errors gracefully', () => {
+      vi.spyOn(os, 'homedir').mockImplementation(() => {
+        throw new Error('Cannot resolve home directory');
+      });
+      const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
+
+      const result = resolvePathFromEnv('~/documents/file.txt');
+      expect(result).toEqual({
+        isSwitch: false,
+        value: null,
+        isDisabled: false,
+      });
+      expect(consoleSpy).toHaveBeenCalledWith(
+        'Could not resolve home directory for path: ~/documents/file.txt',
+        expect.any(Error),
+      );
+
+      consoleSpy.mockRestore();
+    });
+  });
+});

packages/core/src/core/prompts.ts

@@ -19,29 +19,78 @@ import process from 'node:process';
 import { isGitRepository } from '../utils/gitUtils.js';
 import { MemoryTool, GEMINI_CONFIG_DIR } from '../tools/memoryTool.js';
 
+export function resolvePathFromEnv(envVar?: string): {
+  isSwitch: boolean;
+  value: string | null;
+  isDisabled: boolean;
+} {
+  // Handle the case where the environment variable is not set, empty, or just whitespace.
+  const trimmedEnvVar = envVar?.trim();
+  if (!trimmedEnvVar) {
+    return { isSwitch: false, value: null, isDisabled: false };
+  }
+
+  const lowerEnvVar = trimmedEnvVar.toLowerCase();
+  // Check if the input is a common boolean-like string.
+  if (['0', 'false', '1', 'true'].includes(lowerEnvVar)) {
+    // If so, identify it as a "switch" and return its value.
+    const isDisabled = ['0', 'false'].includes(lowerEnvVar);
+    return { isSwitch: true, value: lowerEnvVar, isDisabled };
+  }
+
+  // If it's not a switch, treat it as a potential file path.
+  let customPath = trimmedEnvVar;
+
+  // Safely expand the tilde (~) character to the user's home directory.
+  if (customPath.startsWith('~/') || customPath === '~') {
+    try {
+      const home = os.homedir(); // This is the call that can throw an error.
+      if (customPath === '~') {
+        customPath = home;
+      } else {
+        customPath = path.join(home, customPath.slice(2));
+      }
+    } catch (error) {
+      // If os.homedir() fails, we catch the error instead of crashing.
+      console.warn(
+        `Could not resolve home directory for path: ${trimmedEnvVar}`,
+        error,
+      );
+      // Return null to indicate the path resolution failed.
+      return { isSwitch: false, value: null, isDisabled: false };
+    }
+  }
+
+  // Return it as a non-switch with the fully resolved absolute path.
+  return {
+    isSwitch: false,
+    value: path.resolve(customPath),
+    isDisabled: false,
+  };
+}
+
 export function getCoreSystemPrompt(userMemory?: string): string {
-  // if GEMINI_SYSTEM_MD is set (and not 0|false), override system prompt from file
-  // default path is .gemini/system.md but can be modified via custom path in GEMINI_SYSTEM_MD
+  // A flag to indicate whether the system prompt override is active.
   let systemMdEnabled = false;
+  // The default path for the system prompt file. This can be overridden.
   let systemMdPath = path.resolve(path.join(GEMINI_CONFIG_DIR, 'system.md'));
-  const systemMdVar = process.env['GEMINI_SYSTEM_MD'];
-  if (systemMdVar) {
-    const systemMdVarLower = systemMdVar.toLowerCase();
-    if (!['0', 'false'].includes(systemMdVarLower)) {
-      systemMdEnabled = true; // enable system prompt override
-      if (!['1', 'true'].includes(systemMdVarLower)) {
-        let customPath = systemMdVar;
-        if (customPath.startsWith('~/')) {
-          customPath = path.join(os.homedir(), customPath.slice(2));
-        } else if (customPath === '~') {
-          customPath = os.homedir();
-        }
-        systemMdPath = path.resolve(customPath); // use custom path from GEMINI_SYSTEM_MD
-      }
-      // require file to exist when override is enabled
-      if (!fs.existsSync(systemMdPath)) {
-        throw new Error(`missing system prompt file '${systemMdPath}'`);
-      }
+  // Resolve the environment variable to get either a path or a switch value.
+  const systemMdResolution = resolvePathFromEnv(
+    process.env['GEMINI_SYSTEM_MD'],
+  );
+
+  // Proceed only if the environment variable is set and is not disabled.
+  if (systemMdResolution.value && !systemMdResolution.isDisabled) {
+    systemMdEnabled = true;
+
+    // We update systemMdPath to this new custom path.
+    if (!systemMdResolution.isSwitch) {
+      systemMdPath = systemMdResolution.value;
+    }
+
+    // require file to exist when override is enabled
+    if (!fs.existsSync(systemMdPath)) {
+      throw new Error(`missing system prompt file '${systemMdPath}'`);
     }
   }
   const basePrompt = systemMdEnabled
@@ -266,25 +315,19 @@ Your core function is efficient and safe assistance. Balance extreme conciseness
 `.trim();
 
   // if GEMINI_WRITE_SYSTEM_MD is set (and not 0|false), write base system prompt to file
-  const writeSystemMdVar = process.env['GEMINI_WRITE_SYSTEM_MD'];
-  if (writeSystemMdVar) {
-    const writeSystemMdVarLower = writeSystemMdVar.toLowerCase();
-    if (!['0', 'false'].includes(writeSystemMdVarLower)) {
-      if (['1', 'true'].includes(writeSystemMdVarLower)) {
-        fs.mkdirSync(path.dirname(systemMdPath), { recursive: true });
-        fs.writeFileSync(systemMdPath, basePrompt); // write to default path, can be modified via GEMINI_SYSTEM_MD
-      } else {
-        let customPath = writeSystemMdVar;
-        if (customPath.startsWith('~/')) {
-          customPath = path.join(os.homedir(), customPath.slice(2));
-        } else if (customPath === '~') {
-          customPath = os.homedir();
-        }
-        const resolvedPath = path.resolve(customPath);
-        fs.mkdirSync(path.dirname(resolvedPath), { recursive: true });
-        fs.writeFileSync(resolvedPath, basePrompt); // write to custom path from GEMINI_WRITE_SYSTEM_MD
-      }
-    }
+  const writeSystemMdResolution = resolvePathFromEnv(
+    process.env['GEMINI_WRITE_SYSTEM_MD'],
+  );
+
+  // Check if the feature is enabled. This proceeds only if the environment
+  // variable is set and is not explicitly '0' or 'false'.
+  if (writeSystemMdResolution.value && !writeSystemMdResolution.isDisabled) {
+    const writePath = writeSystemMdResolution.isSwitch
+      ? systemMdPath
+      : writeSystemMdResolution.value;
+
+    fs.mkdirSync(path.dirname(writePath), { recursive: true });
+    fs.writeFileSync(writePath, basePrompt);
   }
 
   const memorySuffix =