diff --git a/src/config/loader.ts b/src/config/loader.ts index b3d7cdfc..77680441 100644 --- a/src/config/loader.ts +++ b/src/config/loader.ts @@ -1,4 +1,4 @@ -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'; @@ -6,6 +6,36 @@ 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): Promise { +export async function writeConfigFile( + data: Record, + renameOps?: RenameWithFallbackOps, +): Promise { 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 { diff --git a/test/config/loader.test.ts b/test/config/loader.test.ts index 38a160ee..1fcd8158 100644 --- a/test/config/loader.test.ts +++ b/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'); + }); +});