DavidAPierce · Sep 30, 2026 · Sep 25, 2026 · Sep 25, 2026 · Sep 25, 2026 · Sep 25, 2026
@@ -0,0 +1,116 @@
+/**
+ * @license
+ * Copyright 2026 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import fs from 'node:fs';
+import fsp from 'node:fs/promises';
+import os from 'node:os';
+import path from 'node:path';
+import { StandardFileSystemService } from './fileSystemService.js';
+
+/**
+ * These tests exercise the real filesystem on purpose: the behaviour under
+ * test is what a concurrent observer can see on disk while a write is in
+ * flight, which a mocked `fs` cannot express.
+ */
+describe('StandardFileSystemService atomicity', () => {
+  let dir: string;
+  let service: StandardFileSystemService;
+
+  beforeEach(async () => {
+    dir = await fsp.mkdtemp(path.join(os.tmpdir(), 'gemini-atomic-write-'));
+    service = new StandardFileSystemService();
+  });
+
+  afterEach(async () => {
+    await fsp.rm(dir, { recursive: true, force: true });
+  });
+
+  it('never exposes a partially written file to a concurrent observer', async () => {
+    // Large enough that the underlying write is split into several chunks;
+    // old and new are the same length so any other size is a partial state.
+    const SIZE = 16 * 1024 * 1024;
+    const filePath = path.join(dir, 'large.txt');
+    await fsp.writeFile(filePath, 'o'.repeat(SIZE), 'utf-8');
+
+    // The write runs on the libuv threadpool, so a *synchronous* loop on the
+    // main thread is what actually catches the destination mid-write. An
+    // async reader tends to be scheduled only before or after it.
+    const sizes = new Set<number>();
+    let settled = false;
+    const write = service
+      .writeTextFile(filePath, 'n'.repeat(SIZE))
+      .finally(() => {
+        settled = true;
+      });
+
+    let spins = 0;
+    while (!settled && spins < 2_000_000) {
+      try {
+        sizes.add(fs.statSync(filePath).size);
+      } catch {
+        // The destination may briefly not exist while being replaced.
+        sizes.add(-1);
+      }
+      spins++;
+      if (spins % 200 === 0) {
+        await new Promise((resolve) => setImmediate(resolve));
+      }
+    }
+    await write;
+
+    // Guard against a vacuous pass where the observer never ran.
+    expect(sizes.size).toBeGreaterThan(0);
+
+    const partialStates = [...sizes].filter(
+      (size) => size !== SIZE && size !== -1,
+    );
+    expect(partialStates).toEqual([]);
+  });
+
+  it('writes the requested content', async () => {
+    const filePath = path.join(dir, 'content.txt');
+
+    await service.writeTextFile(filePath, 'hello');
+
+    await expect(fsp.readFile(filePath, 'utf-8')).resolves.toBe('hello');
+  });
+
+  it.skipIf(process.platform === 'win32')(
+    'preserves the permissions of an existing file',
+    async () => {
+      const filePath = path.join(dir, 'secret.txt');
+      await fsp.writeFile(filePath, 'before', { mode: 0o600 });
+      await fsp.chmod(filePath, 0o600);
+
+      await service.writeTextFile(filePath, 'after');
+
+      const stats = await fsp.stat(filePath);
+      expect(stats.mode & 0o777).toBe(0o600);
+    },
+  );
+
+  it('leaves no temporary files behind on success', async () => {
+    const filePath = path.join(dir, 'clean.txt');
+
+    await service.writeTextFile(filePath, 'done');
+
+    await expect(fsp.readdir(dir)).resolves.toEqual(['clean.txt']);
+  });
+
+  it('updates the target of a symlink without replacing the symlink itself', async () => {
+    const targetPath = path.join(dir, 'target.txt');
+    const symlinkPath = path.join(dir, 'link.txt');
+    await fsp.writeFile(targetPath, 'original');
+    await fsp.symlink(targetPath, symlinkPath);
+
+    await service.writeTextFile(symlinkPath, 'updated');
+
+    await expect(fsp.readFile(targetPath, 'utf-8')).resolves.toBe('updated');
+    const lstat = await fsp.lstat(symlinkPath);
+    expect(lstat.isSymbolicLink()).toBe(true);
+  });
+});
@@ -6,12 +6,19 @@
 
 import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
 import fs from 'node:fs/promises';
+import path from 'node:path';
 import { StandardFileSystemService } from './fileSystemService.js';
 
 vi.mock('fs/promises');
 
+function escapeRegex(str: string): string {
+  return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+}
+
 describe('StandardFileSystemService', () => {
   let fileSystem: StandardFileSystemService;
+  const targetFile = path.resolve('/test/file.txt');
+  const tmpPattern = new RegExp(`^${escapeRegex(targetFile)}\\..*\\.tmp$`);
 
   beforeEach(() => {
     vi.resetAllMocks();
@@ -27,33 +34,115 @@ describe('StandardFileSystemService', () => {
       const testContent = 'Hello, World!';
       vi.mocked(fs.readFile).mockResolvedValue(testContent);
 
-      const result = await fileSystem.readTextFile('/test/file.txt');
+      const result = await fileSystem.readTextFile(targetFile);
 
-      expect(fs.readFile).toHaveBeenCalledWith('/test/file.txt', 'utf-8');
+      expect(fs.readFile).toHaveBeenCalledWith(targetFile, 'utf-8');
       expect(result).toBe(testContent);
     });
 
     it('should propagate fs.readFile errors', async () => {
       const error = new Error('ENOENT: File not found');
       vi.mocked(fs.readFile).mockRejectedValue(error);
 
-      await expect(fileSystem.readTextFile('/test/file.txt')).rejects.toThrow(
+      await expect(fileSystem.readTextFile(targetFile)).rejects.toThrow(
         'ENOENT: File not found',
       );
     });
   });
 
   describe('writeTextFile', () => {
-    it('should write file content using fs', async () => {
+    it('should write to a sibling temp file and rename it into place', async () => {
+      vi.mocked(fs.writeFile).mockResolvedValue();
+      vi.mocked(fs.rename).mockResolvedValue();
+      vi.mocked(fs.stat).mockRejectedValue(new Error('ENOENT'));
+
+      await fileSystem.writeTextFile(targetFile, 'Hello, World!');
+
+      const [tmpPath, content, options] = vi.mocked(fs.writeFile).mock
+        .calls[0] as [string, string, { encoding: string }];
+      expect(content).toBe('Hello, World!');
+      expect(options.encoding).toBe('utf-8');
+      expect(tmpPath).toMatch(tmpPattern);
+      expect(fs.rename).toHaveBeenCalledWith(tmpPath, targetFile);
+    });
+
+    it('should match temp file patterns with Windows-style backslash paths', () => {
+      const winTarget = 'C:\\test\\folder\\file.txt';
+      const winTmpPattern = new RegExp(
+        `^${escapeRegex(winTarget)}\\..*\\.tmp$`,
+      );
+      const sampleWinTmp =
+        'C:\\test\\folder\\file.txt.12345678-1234-1234-1234-123456789abc.tmp';
+      expect(sampleWinTmp).toMatch(winTmpPattern);
+    });
+
+    it('should create the temp file with the destination permissions', async () => {
       vi.mocked(fs.writeFile).mockResolvedValue();
+      vi.mocked(fs.rename).mockResolvedValue();
+      vi.mocked(fs.chmod).mockResolvedValue();
+      vi.mocked(fs.stat).mockResolvedValue({
+        mode: 0o600,
+      } as unknown as Awaited<ReturnType<typeof fs.stat>>);
 
-      await fileSystem.writeTextFile('/test/file.txt', 'Hello, World!');
+      const secretFile = path.resolve('/test/secret.txt');
+      await fileSystem.writeTextFile(secretFile, 'Hello, World!');
 
-      expect(fs.writeFile).toHaveBeenCalledWith(
-        '/test/file.txt',
-        'Hello, World!',
-        'utf-8',
+      // Creating the temp file already restricted means the content is never
+      // briefly readable through a wider default mode.
+      const [, , options] = vi.mocked(fs.writeFile).mock.calls[0];
+      expect(options).toEqual({ encoding: 'utf-8', mode: 0o600 });
+    });
+
+    it('should still write the file when chmod is not permitted', async () => {
+      vi.mocked(fs.writeFile).mockResolvedValue();
+      vi.mocked(fs.rename).mockResolvedValue();
+      vi.mocked(fs.rm).mockResolvedValue();
+      vi.mocked(fs.stat).mockResolvedValue({
+        mode: 0o600,
+      } as unknown as Awaited<ReturnType<typeof fs.stat>>);
+      // FAT32/exFAT, some NFS/CIFS mounts and restricted sandboxes reject chmod.
+      vi.mocked(fs.chmod).mockRejectedValue(
+        Object.assign(new Error('operation not supported'), {
+          code: 'ENOTSUP',
+        }),
       );
+
+      await expect(
+        fileSystem.writeTextFile(targetFile, 'Hello, World!'),
+      ).resolves.toBeUndefined();
+
+      expect(fs.rename).toHaveBeenCalled();
+      expect(fs.rm).not.toHaveBeenCalled();
+    });
+
+    it('should remove the temp file when the write fails', async () => {
+      vi.mocked(fs.writeFile).mockRejectedValue(new Error('ENOSPC'));
+      vi.mocked(fs.rm).mockResolvedValue();
+
+      await expect(
+        fileSystem.writeTextFile(targetFile, 'Hello, World!'),
+      ).rejects.toThrow('ENOSPC');
+
+      expect(fs.rename).not.toHaveBeenCalled();
+      const [removed] = vi.mocked(fs.rm).mock.calls[0] as [string];
+      expect(removed).toMatch(tmpPattern);
+    });
+
+    it('should retry rename when encountering EACCES', async () => {
+      vi.mocked(fs.writeFile).mockResolvedValue();
+      vi.mocked(fs.stat).mockRejectedValue(new Error('ENOENT'));
+      const eaccesError = Object.assign(new Error('permission denied'), {
+        code: 'EACCES',
+      });
+      vi.mocked(fs.rename)
+        .mockRejectedValueOnce(eaccesError)
+        .mockResolvedValueOnce();
+
+      await expect(
+        fileSystem.writeTextFile(targetFile, 'Hello, World!'),
+      ).resolves.toBeUndefined();
+
+      expect(fs.rename).toHaveBeenCalledTimes(2);
     });
   });
 });
@@ -4,7 +4,11 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import { randomUUID } from 'node:crypto';
 import fs from 'node:fs/promises';
+import path from 'node:path';
+import { isNodeError } from '../utils/errors.js';
+import { resolveToRealPath } from '../utils/paths.js';
 
 /**
  * Interface for file system operations that may be delegated to different implementations
@@ -27,6 +31,9 @@ export interface FileSystemService {
   writeTextFile(filePath: string, content: string): Promise<void>;
 }
 
+/** Rename retries, for transient Windows lock errors. */
+const RENAME_MAX_RETRIES = 5;
+
 /**
  * Standard file system implementation
  */
@@ -35,7 +42,98 @@ export class StandardFileSystemService implements FileSystemService {
     return fs.readFile(filePath, 'utf-8');
   }
 
+  /**
+   * Writes `content` to `filePath` atomically.
+   *
+   * A plain `fs.writeFile` truncates the destination and then streams the
+   * content in chunks, so anything reading the file concurrently can observe
+   * a truncated prefix. Writing to a sibling temp file and renaming it into
+   * place means an observer sees either the old file or the new one.
+   */
   async writeTextFile(filePath: string, content: string): Promise<void> {
-    await fs.writeFile(filePath, content, 'utf-8');
+    // When filePath is a symlink, resolve to its real target path so that the
+    // rename updates the underlying target file rather than replacing the
+    // symlink itself with a regular file.
+    let realPath = filePath;
+    try {
+      realPath = resolveToRealPath(filePath);
+    } catch {
+      try {
+        const dir = path.dirname(filePath);
+        const base = path.basename(filePath);
+        realPath = path.join(resolveToRealPath(dir), base);
+      } catch {
+        realPath = filePath;
+      }
+    }
+
+    // The temp file must share a directory with the destination so that the
+    // rename stays within one filesystem, and must be uniquely named so that
+    // concurrent writers do not clobber each other's temp file.
+    const tmpPath = `${realPath}.${randomUUID()}.tmp`;
+
+    // A fresh temp file does not inherit the destination's permissions, so
+    // without this, replacing a 0600 file would silently widen it to the
+    // default mode.
+    const existingMode = await this.getFileMode(realPath);
+
+    try {
+      // Create the temp file already carrying the destination's mode, so the
+      // content is never briefly readable through a wider default mode. The
+      // mode is masked by umask, so this can only be more restrictive.
+      await fs.writeFile(tmpPath, content, {
+        encoding: 'utf-8',
+        ...(existingMode !== undefined ? { mode: existingMode } : {}),
+      });
+
+      if (existingMode !== undefined) {
+        try {
+          // Correct any narrowing that umask applied above. Best effort: some
+          // filesystems (FAT32, exFAT, a few NFS/CIFS mounts) and restricted
+          // sandboxes reject chmod with EPERM/ENOTSUP, and permissions must
+          // not be the reason a write fails.
+          await fs.chmod(tmpPath, existingMode);
+        } catch {
+          // Keep whatever mode the temp file was created with.
+        }
+      }
+
+      await this.renameWithRetry(tmpPath, realPath);
+    } catch (error) {
+      await fs.rm(tmpPath, { force: true }).catch(() => {
+        // Best effort: the original error is the one worth reporting.
+      });
+      throw error;
+    }
+  }
+
+  private async getFileMode(filePath: string): Promise<number | undefined> {
+    try {
+      const stats = await fs.stat(filePath);
+      return stats.mode & 0o777;
+    } catch {
+      // New file, or a destination we cannot stat; keep the default mode.
+      return undefined;
+    }
+  }
+
+  private async renameWithRetry(from: string, to: string): Promise<void> {
+    for (let attempt = 0; attempt < RENAME_MAX_RETRIES; attempt++) {
+      try {
+        await fs.rename(from, to);
+        return;
+      } catch (error: unknown) {
+        // Windows can transiently refuse a rename while another process has
+        // the destination open (antivirus, editors, watchers).
+        const code = isNodeError(error) ? error.code : '';
+        const isRetryable =
+          code === 'EBUSY' || code === 'EPERM' || code === 'EACCES';
+        if (!isRetryable || attempt === RENAME_MAX_RETRIES - 1) {
+          throw error;
+        }
+        const delayMs = Math.pow(2, attempt) * 50;
+        await new Promise((resolve) => setTimeout(resolve, delayMs));
+      }
+    }
   }
 }