From 615b218ff7026a6e41ff22967c548fa4ed2e3408 Mon Sep 17 00:00:00 2001
From: "N. Taylor Mullen" <ntaylormullen@google.com>
Date: Sun, 4 Jan 2026 21:19:33 -0800
Subject: [PATCH] fix(cli): mock fs.readdir in consent tests for Windows
 compatibility (#15904)

---
 .../cli/src/config/extensions/consent.test.ts | 61 +++++++++++++------
 .../core/src/utils/shell-permissions.test.ts  | 31 ++++++++++
 2 files changed, 72 insertions(+), 20 deletions(-)

diff --git a/packages/cli/src/config/extensions/consent.test.ts b/packages/cli/src/config/extensions/consent.test.ts
index 72a0b79fb6..ccb569f43e 100644
--- a/packages/cli/src/config/extensions/consent.test.ts
+++ b/packages/cli/src/config/extensions/consent.test.ts
@@ -27,12 +27,26 @@ const mockReadline = vi.hoisted(() => ({
   }),
 }));
 
+const mockReaddir = vi.hoisted(() => vi.fn());
+const originalReaddir = vi.hoisted(() => ({
+  current: null as typeof fs.readdir | null,
+}));
+
 // Mocking readline for non-interactive prompts
 vi.mock('node:readline', () => ({
   default: mockReadline,
   createInterface: mockReadline.createInterface,
 }));
 
+vi.mock('node:fs/promises', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('node:fs/promises')>();
+  originalReaddir.current = actual.readdir;
+  return {
+    ...actual,
+    readdir: mockReaddir,
+  };
+});
+
 vi.mock('@google/gemini-cli-core', async (importOriginal) => {
   const actual =
     await importOriginal<typeof import('@google/gemini-cli-core')>();
@@ -49,6 +63,10 @@ describe('consent', () => {
 
   beforeEach(async () => {
     vi.clearAllMocks();
+    if (originalReaddir.current) {
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      mockReaddir.mockImplementation(originalReaddir.current as any);
+    }
     tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'consent-test-'));
   });
 
@@ -328,7 +346,7 @@ describe('consent', () => {
 
       it('should show a warning if the skill directory cannot be read', async () => {
         const lockedDir = path.join(tempDir, 'locked');
-        await fs.mkdir(lockedDir, { recursive: true, mode: 0o000 });
+        await fs.mkdir(lockedDir, { recursive: true });
 
         const skill: SkillDefinition = {
           name: 'locked-skill',
@@ -337,26 +355,29 @@ describe('consent', () => {
           body: 'body',
         };
 
-        const requestConsent = vi.fn().mockResolvedValue(true);
-        try {
-          await maybeRequestConsentOrFail(
-            baseConfig,
-            requestConsent,
-            false,
-            undefined,
-            false,
-            [skill],
-          );
+        // Mock readdir to simulate a permission error.
+        // We do this instead of using fs.mkdir(..., { mode: 0o000 }) because
+        // directory permissions work differently on Windows and 0o000 doesn't
+        // effectively block access there, leading to test failures in Windows CI.
+        mockReaddir.mockRejectedValueOnce(
+          new Error('EACCES: permission denied, scandir'),
+        );
 
-          expect(requestConsent).toHaveBeenCalledWith(
-            expect.stringContaining(
-              `    (Location: ${skill.location}) ${chalk.red('⚠️ (Could not count items in directory)')}`,
-            ),
-          );
-        } finally {
-          // Restore permissions so cleanup works
-          await fs.chmod(lockedDir, 0o700);
-        }
+        const requestConsent = vi.fn().mockResolvedValue(true);
+        await maybeRequestConsentOrFail(
+          baseConfig,
+          requestConsent,
+          false,
+          undefined,
+          false,
+          [skill],
+        );
+
+        expect(requestConsent).toHaveBeenCalledWith(
+          expect.stringContaining(
+            `    (Location: ${skill.location}) ${chalk.red('⚠️ (Could not count items in directory)')}`,
+          ),
+        );
       });
     });
   });
diff --git a/packages/core/src/utils/shell-permissions.test.ts b/packages/core/src/utils/shell-permissions.test.ts
index 7f7cf1f46e..a80afdbd7a 100644
--- a/packages/core/src/utils/shell-permissions.test.ts
+++ b/packages/core/src/utils/shell-permissions.test.ts
@@ -33,6 +33,15 @@ vi.mock('os', () => ({
   homedir: mockHomedir,
 }));
 
+const mockSpawnSync = vi.hoisted(() => vi.fn());
+vi.mock('node:child_process', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('node:child_process')>();
+  return {
+    ...actual,
+    spawnSync: mockSpawnSync,
+  };
+});
+
 const mockQuote = vi.hoisted(() => vi.fn());
 vi.mock('shell-quote', () => ({
   quote: mockQuote,
@@ -464,12 +473,34 @@ describeWindowsOnly('PowerShell integration', () => {
   });
 
   it('should block commands when PowerShell parser reports errors', () => {
+    // Mock spawnSync to avoid the overhead of spawning a real PowerShell process,
+    // which can lead to timeouts in CI environments even on Windows.
+    mockSpawnSync.mockReturnValue({
+      status: 0,
+      stdout: JSON.stringify({ success: false }),
+    });
+
     const { allowed, reason } = isCommandAllowed('Get-ChildItem |', config);
     expect(allowed).toBe(false);
     expect(reason).toBe(
       'Command rejected because it could not be parsed safely',
     );
   });
+
+  it('should allow valid commands through PowerShell parser', () => {
+    // Mock spawnSync to avoid the overhead of spawning a real PowerShell process,
+    // which can lead to timeouts in CI environments even on Windows.
+    mockSpawnSync.mockReturnValue({
+      status: 0,
+      stdout: JSON.stringify({
+        success: true,
+        commands: [{ name: 'Get-ChildItem', text: 'Get-ChildItem' }],
+      }),
+    });
+
+    const { allowed } = isCommandAllowed('Get-ChildItem', config);
+    expect(allowed).toBe(true);
+  });
 });
 
 describe('isShellInvocationAllowlisted', () => {