Merge pull request #162 from han-dreamer/fix/config-write-exdev

fix: handle cross-device config writes

Author: NianJiuZst

src/config/loader.ts

@@ -1,11 +1,41 @@
-import { readFileSync, writeFileSync, renameSync, existsSync } from 'fs';
+import { copyFileSync, existsSync, readFileSync, renameSync, unlinkSync, writeFileSync } from 'fs';
 import { parseConfigFile, REGIONS, type Config, type ConfigFile, type Region } from './schema';
 import { ensureConfigDir, getConfigPath } from './paths';
 import { detectOutputFormat, type OutputFormat } from '../output/formatter';
 import { CLIError } from '../errors/base';
 import { ExitCode } from '../errors/codes';
 import type { GlobalFlags } from '../types/flags';
 
+interface RenameWithFallbackOps {
+  rename: (from: string, to: string) => void;
+  copy: (from: string, to: string) => void;
+  unlink: (path: string) => void;
+}
+
+function isCrossDeviceError(err: unknown): err is Error & { code: 'EXDEV' } {
+  return err instanceof Error
+    && 'code' in err
+    && (err as { code?: string }).code === 'EXDEV';
+}
+
+export function renameWithCrossDeviceFallback(
+  from: string,
+  to: string,
+  ops: RenameWithFallbackOps = {
+    rename: renameSync,
+    copy: copyFileSync,
+    unlink: unlinkSync,
+  },
+): void {
+  try {
+    ops.rename(from, to);
+  } catch (err) {
+    if (!isCrossDeviceError(err)) throw err;
+    ops.copy(from, to);
+    ops.unlink(from);
+  }
+}
+
 export function readConfigFile(): ConfigFile {
   const path = getConfigPath();
   if (!existsSync(path)) return {};
@@ -20,12 +50,15 @@ export function readConfigFile(): ConfigFile {
   }
 }
 
-export async function writeConfigFile(data: Record<string, unknown>): Promise<void> {
+export async function writeConfigFile(
+  data: Record<string, unknown>,
+  renameOps?: RenameWithFallbackOps,
+): Promise<void> {
   await ensureConfigDir();
   const path = getConfigPath();
   const tmp = path + '.tmp';
   writeFileSync(tmp, JSON.stringify(data, null, 2) + '\n', { mode: 0o600 });
-  renameSync(tmp, path);
+  renameWithCrossDeviceFallback(tmp, path, renameOps);
 }
 
 export function loadConfig(flags: GlobalFlags): Config {

test/config/loader.test.ts

@@ -1,8 +1,8 @@
-import { describe, it, expect, beforeEach, afterEach } from 'bun:test';
-import { mkdirSync, rmSync } from 'fs';
+import { describe, it, expect, beforeEach, afterEach, mock } from 'bun:test';
+import { copyFileSync, mkdirSync, readFileSync, rmSync, unlinkSync } from 'fs';
 import { join } from 'path';
 import { tmpdir } from 'os';
-import { loadConfig } from '../../src/config/loader';
+import { loadConfig, renameWithCrossDeviceFallback, writeConfigFile } from '../../src/config/loader';
 import { CLIError } from '../../src/errors/base';
 import type { GlobalFlags } from '../../src/types/flags';
 
@@ -57,3 +57,102 @@ describe('loadConfig', () => {
     expect(config.baseUrl).toBe('https://api.minimaxi.com');
   });
 });
+
+describe('writeConfigFile', () => {
+  const testDir = join(tmpdir(), `mmx-config-write-test-${Date.now()}`);
+  const originalConfigDir = process.env.MMX_CONFIG_DIR;
+
+  beforeEach(() => {
+    process.env.MMX_CONFIG_DIR = testDir;
+    mkdirSync(testDir, { recursive: true });
+  });
+
+  afterEach(() => {
+    if (originalConfigDir === undefined) delete process.env.MMX_CONFIG_DIR;
+    else process.env.MMX_CONFIG_DIR = originalConfigDir;
+    rmSync(testDir, { recursive: true, force: true });
+    mock.restore();
+  });
+
+  it('falls back to copy and unlink when rename crosses devices', async () => {
+    const calls: string[] = [];
+    const renameMock = mock((from: string, _to: string) => {
+      calls.push(`rename:${from}`);
+      const err = new Error('cross-device link not permitted') as NodeJS.ErrnoException;
+      err.code = 'EXDEV';
+      err.path = from;
+      throw err;
+    });
+    const copyMock = mock((from: string, _to: string) => {
+      calls.push(`copy:${from}`);
+    });
+    const unlinkMock = mock((path: string) => {
+      calls.push(`unlink:${path}`);
+    });
+
+    renameWithCrossDeviceFallback('config.json.tmp', 'config.json', {
+      rename: renameMock,
+      copy: copyMock,
+      unlink: unlinkMock,
+    });
+
+    expect(renameMock).toHaveBeenCalledTimes(1);
+    expect(copyMock).toHaveBeenCalledTimes(1);
+    expect(unlinkMock).toHaveBeenCalledTimes(1);
+    expect(calls).toEqual([
+      'rename:config.json.tmp',
+      'copy:config.json.tmp',
+      'unlink:config.json.tmp',
+    ]);
+  });
+
+  it('writes the config file when the final rename crosses devices', async () => {
+    const renameMock = mock((from: string, _to: string) => {
+      const err = new Error('cross-device link not permitted') as Error & {
+        code?: string;
+        path?: string;
+      };
+      err.code = 'EXDEV';
+      err.path = from;
+      throw err;
+    });
+    const copyMock = mock((from: string, to: string) => copyFileSync(from, to));
+    const unlinkMock = mock((path: string) => unlinkSync(path));
+
+    await writeConfigFile({ region: 'cn', output: 'json' }, {
+      rename: renameMock,
+      copy: copyMock,
+      unlink: unlinkMock,
+    });
+
+    const configPath = join(testDir, 'config.json');
+    const parsed = JSON.parse(readFileSync(configPath, 'utf-8'));
+
+    expect(renameMock).toHaveBeenCalledTimes(1);
+    expect(copyMock).toHaveBeenCalledTimes(1);
+    expect(unlinkMock).toHaveBeenCalledTimes(1);
+    expect(parsed.region).toBe('cn');
+    expect(parsed.output).toBe('json');
+  });
+
+  it('rethrows non-EXDEV rename errors', () => {
+    const renameMock = mock(() => {
+      const err = new Error('permission denied') as NodeJS.ErrnoException;
+      err.code = 'EACCES';
+      throw err;
+    });
+
+    expect(() => renameWithCrossDeviceFallback('config.json.tmp', 'config.json', {
+      rename: renameMock,
+      copy: mock(() => {}),
+      unlink: mock(() => {}),
+    })).toThrow('permission denied');
+  });
+
+  it('uses atomic rename on the normal path', async () => {
+    await writeConfigFile({ output: 'json' });
+
+    const configPath = join(testDir, 'config.json');
+    expect(JSON.parse(readFileSync(configPath, 'utf-8')).output).toBe('json');
+  });
+});