From f0ea7a39a7a451d928709afe94ba49efbbd76dec Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Fri, 5 Jun 2026 15:43:13 -0400 Subject: [PATCH] Retry Codex trust config writes on Windows locks (#4710) --- src/main/codex-accounts/fs-utils.test.ts | 55 +++++++++++++++++++++++- src/main/codex-accounts/fs-utils.ts | 28 +++++++----- src/main/codex/config-toml-trust.ts | 16 ++----- 3 files changed, 75 insertions(+), 24 deletions(-) diff --git a/src/main/codex-accounts/fs-utils.test.ts b/src/main/codex-accounts/fs-utils.test.ts index 706a1bed3..0932ef412 100644 --- a/src/main/codex-accounts/fs-utils.test.ts +++ b/src/main/codex-accounts/fs-utils.test.ts @@ -1,8 +1,12 @@ import { describe, expect, it } from 'vitest' -import { existsSync, mkdtempSync, readFileSync, rmSync, statSync } from 'node:fs' +import { existsSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { writeFileAtomically } from './fs-utils' +import { + copyFileWithWindowsRetry, + renameFileWithWindowsRetry, + writeFileAtomically +} from './fs-utils' describe('writeFileAtomically', () => { let dir: string @@ -78,3 +82,50 @@ describe('writeFileAtomically', () => { } }) }) + +describe('retrying file operations', () => { + let dir: string + + function setup(): string { + dir = mkdtempSync(join(tmpdir(), 'orca-fs-utils-')) + return dir + } + + function cleanup(): void { + if (dir) { + rmSync(dir, { recursive: true, force: true }) + } + } + + it('renames a file through the retry wrapper', () => { + setup() + try { + const source = join(dir, 'source.txt') + const target = join(dir, 'target.txt') + writeFileSync(source, 'data', 'utf-8') + + renameFileWithWindowsRetry(source, target) + + expect(existsSync(source)).toBe(false) + expect(readFileSync(target, 'utf-8')).toBe('data') + } finally { + cleanup() + } + }) + + it('copies a file through the retry wrapper', () => { + setup() + try { + const source = join(dir, 'source.txt') + const target = join(dir, 'target.txt') + writeFileSync(source, 'data', 'utf-8') + + copyFileWithWindowsRetry(source, target) + + expect(readFileSync(source, 'utf-8')).toBe('data') + expect(readFileSync(target, 'utf-8')).toBe('data') + } finally { + cleanup() + } + }) +}) diff --git a/src/main/codex-accounts/fs-utils.ts b/src/main/codex-accounts/fs-utils.ts index 592256e89..7773e8bd2 100644 --- a/src/main/codex-accounts/fs-utils.ts +++ b/src/main/codex-accounts/fs-utils.ts @@ -1,5 +1,5 @@ import { randomUUID } from 'node:crypto' -import { renameSync, rmSync, writeFileSync } from 'node:fs' +import { copyFileSync, renameSync, rmSync, writeFileSync } from 'node:fs' import { dirname } from 'node:path' import { grantDirAcl, isPermissionError } from '../win32-utils' @@ -11,7 +11,7 @@ export function writeFileAtomically( const tmpPath = `${targetPath}.${process.pid}.${randomUUID()}.tmp` try { writeFileSync(tmpPath, contents, { encoding: 'utf-8', mode: options?.mode }) - renameWithRetry(tmpPath, targetPath) + renameFileWithWindowsRetry(tmpPath, targetPath) } catch (error) { rmSync(tmpPath, { force: true }) // Why: on Windows, Chromium's renderer initialization calls @@ -26,7 +26,7 @@ export function writeFileAtomically( const retryTmpPath = `${targetPath}.${process.pid}.${randomUUID()}.tmp` try { writeFileSync(retryTmpPath, contents, { encoding: 'utf-8', mode: options?.mode }) - renameWithRetry(retryTmpPath, targetPath) + renameFileWithWindowsRetry(retryTmpPath, targetPath) return } catch { rmSync(retryTmpPath, { force: true }) @@ -39,16 +39,24 @@ export function writeFileAtomically( } } -// Why: on Windows, renameSync can fail with EPERM/EACCES/EBUSY if another -// process (antivirus, Claude CLI, Codex CLI) holds the target file open. -// A short retry avoids transient failures without masking real permission -// errors. Total backoff (~750ms) covers typical AV scan windows seen in -// issue #1507. -function renameWithRetry(source: string, target: string): void { +// Why: on Windows, file replacement and backup-copy operations can fail with +// EPERM/EACCES/EBUSY if another process (antivirus, Claude CLI, Codex CLI) +// holds the target file open. A short retry avoids transient failures without +// masking real permission errors. Total backoff (~750ms) covers typical AV +// scan windows seen in issue #1507. +export function renameFileWithWindowsRetry(source: string, target: string): void { + runFileOperationWithWindowsRetry(() => renameSync(source, target)) +} + +export function copyFileWithWindowsRetry(source: string, target: string): void { + runFileOperationWithWindowsRetry(() => copyFileSync(source, target)) +} + +function runFileOperationWithWindowsRetry(operation: () => void): void { const maxAttempts = process.platform === 'win32' ? 6 : 1 for (let attempt = 1; attempt <= maxAttempts; attempt++) { try { - renameSync(source, target) + operation() return } catch (error) { const code = (error as NodeJS.ErrnoException).code diff --git a/src/main/codex/config-toml-trust.ts b/src/main/codex/config-toml-trust.ts index fc96ff899..bb301548b 100644 --- a/src/main/codex/config-toml-trust.ts +++ b/src/main/codex/config-toml-trust.ts @@ -1,17 +1,9 @@ /* eslint-disable max-lines -- Why: Codex hook trust parsing, hashing, and byte-preserving TOML edits share one fragile file-format contract; splitting would make the compatibility shim harder to audit. */ -import { - copyFileSync, - existsSync, - mkdirSync, - readFileSync, - renameSync, - realpathSync, - unlinkSync, - writeFileSync -} from 'fs' +import { existsSync, mkdirSync, readFileSync, realpathSync, unlinkSync, writeFileSync } from 'fs' import { dirname, join } from 'path' import { createHash, randomUUID } from 'crypto' import { escapeRegex } from '../../shared/string-utils' +import { copyFileWithWindowsRetry, renameFileWithWindowsRetry } from '../codex-accounts/fs-utils' // Why: Codex 0.129+ gates each hook on a `trusted_hash` entry in // ~/.codex/config.toml under [hooks.state.""]. Without it the hook is in @@ -576,9 +568,9 @@ export function writeConfigAtomically(configPath: string, contents: string): voi try { writeFileSync(tmpPath, contents, 'utf-8') if (existsSync(configPath)) { - copyFileSync(configPath, `${configPath}.bak`) + copyFileWithWindowsRetry(configPath, `${configPath}.bak`) } - renameSync(tmpPath, configPath) + renameFileWithWindowsRetry(tmpPath, configPath) renamed = true } finally { if (!renamed && existsSync(tmpPath)) {