fix(config): fall back to copy+unlink on EXDEV when writing config (#61)

src/config/loader.ts

@@ -1,11 +1,36 @@
-import { readFileSync, writeFileSync, renameSync, existsSync } from 'fs';
+import { readFileSync, writeFileSync, renameSync, existsSync, copyFileSync, unlinkSync } 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';
 
+/**
+ * Write `content` to `target` atomically when possible. The file is first
+ * written to `target + '.tmp'` and then renamed into place. On `EXDEV`
+ * (cross-device link not permitted — e.g. Windows when the temp dir is on
+ * a different drive than the config dir, or *nix when $TMPDIR is on a
+ * different mount than $HOME), fall back to copyFileSync + unlinkSync.
+ * The fallback is not atomic, but a concurrent reader will see either
+ * the old file or the fully-written new file — never a partial write.
+ */
+function atomicWriteFileSync(target: string, content: string): void {
+  const tmp = target + '.tmp';
+  writeFileSync(tmp, content, { mode: 0o600 });
+  try {
+    renameSync(tmp, target);
+  } catch (err) {
+    const e = err as NodeJS.ErrnoException;
+    if (e.code === 'EXDEV') {
+      copyFileSync(tmp, target);
+      unlinkSync(tmp);
+    } else {
+      throw err;
+    }
+  }
+}
+
 export function readConfigFile(): ConfigFile {
   const path = getConfigPath();
   if (!existsSync(path)) return {};
@@ -23,9 +48,7 @@ export function readConfigFile(): ConfigFile {
 export async function writeConfigFile(data: Record<string, unknown>): Promise<void> {
   await ensureConfigDir();
   const path = getConfigPath();
-  const tmp = path + '.tmp';
-  writeFileSync(tmp, JSON.stringify(data, null, 2) + '\n', { mode: 0o600 });
-  renameSync(tmp, path);
+  atomicWriteFileSync(path, JSON.stringify(data, null, 2) + '\n');
 }
 
 export function loadConfig(flags: GlobalFlags): Config {

test/config/loader-exdev.test.ts

@@ -0,0 +1,118 @@
+import { describe, it, expect, beforeEach, afterEach, mock } from 'bun:test';
+import { tmpdir } from 'os';
+import { join } from 'path';
+
+// We replace fs.renameSync so we can simulate EXDEV (cross-device link
+// not permitted). The factory uses `require` to reach the real fs module
+// (mock.module only intercepts ESM imports, not CommonJS require), and
+// captures the real renameSync so the test can still call it when
+// exercising the happy path.
+//
+// mock.module is hoisted by Bun above all other imports, so by the time
+// src/config/loader.ts is imported below, the loader sees the mocked
+// renameSync.
+
+let realRenameSync: typeof import('fs').renameSync;
+
+const renameMock = mock((src: string, dst: string) => {
+  return realRenameSync(src, dst);
+});
+
+mock.module('fs', () => {
+  // eslint-disable-next-line @typescript-eslint/no-require-imports
+  const realFs = require('fs') as typeof import('fs');
+  realRenameSync = realFs.renameSync;
+  return {
+    ...realFs,
+    renameSync: renameMock,
+  };
+});
+
+// At this point `fs` is the mocked module (the real fs is captured in
+// the mock factory's closure). The mock spreads the real fs, so all
+// non-renameSync functions are the real ones and work as expected in
+// the test body.
+import * as fs from 'fs';
+
+import { writeConfigFile } from '../../src/config/loader';
+
+function makeExdevError(): NodeJS.ErrnoException {
+  const err = new Error('EXDEV: cross-device link not permitted') as NodeJS.ErrnoException;
+  err.code = 'EXDEV';
+  err.errno = -18;
+  return err;
+}
+
+describe('writeConfigFile EXDEV fallback', () => {
+  let testDir: string;
+  let originalHome: string | undefined;
+  let originalRegion: string | undefined;
+  let originalConfigDir: string | undefined;
+  let configPath: string;
+  let tmpPath: string;
+
+  beforeEach(() => {
+    testDir = fs.mkdtempSync(join(fs.realpathSync(tmpdir()), 'mmx-exdev-'));
+    originalHome = process.env.HOME;
+    originalRegion = process.env.MINIMAX_REGION;
+    originalConfigDir = process.env.MMX_CONFIG_DIR;
+    const mmxDir = join(testDir, '.mmx');
+    process.env.HOME = testDir;
+    process.env.MMX_CONFIG_DIR = mmxDir;
+    delete process.env.MINIMAX_REGION;
+    configPath = join(mmxDir, 'config.json');
+    tmpPath = configPath + '.tmp';
+    // Reset mock to a pass-through for each test.
+    renameMock.mockImplementation((src: string, dst: string) => realRenameSync(src, dst));
+  });
+
+  afterEach(() => {
+    // Reset the mock to pass-through so it doesn't leak its last-set
+    // implementation (e.g. throwing EACCES) into other test files that
+    // share this Bun process and import fs.
+    renameMock.mockImplementation((src: string, dst: string) => realRenameSync(src, dst));
+    process.env.HOME = originalHome;
+    if (originalRegion === undefined) delete process.env.MINIMAX_REGION;
+    else process.env.MINIMAX_REGION = originalRegion;
+    if (originalConfigDir === undefined) delete process.env.MMX_CONFIG_DIR;
+    else process.env.MMX_CONFIG_DIR = originalConfigDir;
+    if (testDir) fs.rmSync(testDir, { recursive: true, force: true });
+  });
+
+  it('falls back to copy+unlink when rename throws EXDEV', async () => {
+    // Simulate the temp file living on a different mount than the config dir.
+    renameMock.mockImplementation(() => {
+      throw makeExdevError();
+    });
+
+    await writeConfigFile({ foo: 'bar' });
+
+    // The destination file should exist with the expected contents.
+    expect(fs.existsSync(configPath)).toBe(true);
+    const written = JSON.parse(fs.readFileSync(configPath, 'utf-8'));
+    expect(written).toEqual({ foo: 'bar' });
+
+    // The .tmp file should have been cleaned up.
+    expect(fs.existsSync(tmpPath)).toBe(false);
+  });
+
+  it('uses rename normally when EXDEV is not thrown (happy path)', async () => {
+    // Default pass-through mock is in effect from beforeEach.
+    await writeConfigFile({ hello: 'world' });
+
+    expect(fs.existsSync(configPath)).toBe(true);
+    const written = JSON.parse(fs.readFileSync(configPath, 'utf-8'));
+    expect(written).toEqual({ hello: 'world' });
+    expect(fs.existsSync(tmpPath)).toBe(false);
+  });
+
+  it('propagates non-EXDEV errors from rename', async () => {
+    renameMock.mockImplementation(() => {
+      const err = new Error('EACCES: permission denied') as NodeJS.ErrnoException;
+      err.code = 'EACCES';
+      throw err;
+    });
+
+    await expect(writeConfigFile({ a: 1 })).rejects.toThrow('EACCES');
+  });
+});