fix(#1540): platformWriteSync retries transient rename locks instead of truncating readers (#1541)

* fix(core): platformWriteSync must retry transient rename locks, not truncate readers (#1540)

platformWriteSync fell back to a non-atomic fs.writeFileSync(filePath) on ANY
error from the temp+rename path. On Windows, renameSync onto a target a reader
holds open throws EPERM/EBUSY/EACCES (the common case for hot files like
STATE.md), so the fallback fired and a concurrent reader saw the file
mid-truncation.

Mirror the capability-ledger rename-retry idiom: retry transient lock errnos
(EPERM/EBUSY/EACCES) with a bounded Atomics.wait backoff; on a persistent lock,
surface the error rather than do the truncating non-atomic write. Genuinely
unrenameable cases (EXDEV cross-device) and tmp-write failures still fall back.

Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz

* chore(changeset): Fixed fragment for #1541 (platformWriteSync rename retry)

Claude-Session: https://claude.ai/code/session_01R88n7Q54bAaVHFkDbbH1yz

---------

Co-authored-by: Tom Boucher <trekkie@nomorestars.com>

.changeset/wise-ibex-dart.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 1541
+---
+Atomic file writes now retry a transient rename lock on Windows (a reader holding the target open) instead of falling back to a non-atomic write that could let a concurrent reader observe a truncated STATE.md/ROADMAP.md.

src/shell-command-projection.cts

@@ -550,17 +550,71 @@ export function normalizeContent(filePath: string, content: string, opts: { enco
   return { content: normalized, encoding };
 }
 
+// Rename errnos that are transient on Windows: a concurrent reader (or an AV
+// scanner / indexer) holding the target open makes renameSync fail briefly.
+// Same idiom as capability-ledger.cts / capability-consent.cts.
+const RENAME_RETRY_ERRNOS = new Set(['EPERM', 'EBUSY', 'EACCES']);
+const RENAME_MAX_ATTEMPTS = 3;
+const RENAME_RETRY_BACKOFF_MS = 50;
+
+/** Synchronous best-effort backoff sleep (Atomics.wait — same idiom as io.cts). */
+let _renameSleepBuf: Int32Array | null = null;
+function renameBackoff(): void {
+  if (_renameSleepBuf === null) _renameSleepBuf = new Int32Array(new SharedArrayBuffer(4));
+  Atomics.wait(_renameSleepBuf, 0, 0, RENAME_RETRY_BACKOFF_MS);
+}
+
+/**
+ * Atomic publish with bounded retry on transient Windows lock errnos.
+ * Returns null on success, or the final error if every attempt failed.
+ */
+function atomicRenameWithRetry(tmpPath: string, filePath: string): NodeJS.ErrnoException | null {
+  let renameErr: NodeJS.ErrnoException | null = null;
+  for (let attempt = 1; attempt <= RENAME_MAX_ATTEMPTS; attempt++) {
+    try {
+      fs.renameSync(tmpPath, filePath);
+      return null;
+    } catch (err) {
+      renameErr = err as NodeJS.ErrnoException;
+      if (attempt < RENAME_MAX_ATTEMPTS && RENAME_RETRY_ERRNOS.has(renameErr.code ?? '')) {
+        renameBackoff();
+        continue;
+      }
+      break;
+    }
+  }
+  return renameErr;
+}
+
 export function platformWriteSync(filePath: string, content: string, opts: { encoding?: BufferEncoding } = {}): void {
   const { content: normalized, encoding } = normalizeContent(filePath, content, opts);
   fs.mkdirSync(path.dirname(filePath), { recursive: true });
   const tmpPath = filePath + '.tmp.' + process.pid;
+
+  // Step 1: write the sibling tmp file. If THIS fails, nothing was published, so a
+  // direct fallback write cannot truncate a concurrent reader of an existing file.
   try {
     fs.writeFileSync(tmpPath, normalized, encoding);
-    fs.renameSync(tmpPath, filePath);
   } catch {
     try { fs.unlinkSync(tmpPath); } catch { /* already gone */ }
     fs.writeFileSync(filePath, normalized, encoding);
+    return;
   }
+
+  // Step 2: atomic publish, retrying transient Windows locks.
+  const renameErr = atomicRenameWithRetry(tmpPath, filePath);
+  if (renameErr === null) return;
+
+  try { fs.unlinkSync(tmpPath); } catch { /* already gone */ }
+  if (RENAME_RETRY_ERRNOS.has(renameErr.code ?? '')) {
+    // A live reader still holds the target open after every retry. A non-atomic
+    // direct write here would truncate that reader (the exact corruption this seam
+    // exists to prevent), so surface the error instead of falling back.
+    throw renameErr;
+  }
+  // Atomic publish is genuinely impossible here (e.g. EXDEV cross-device move):
+  // fall back to a direct write to preserve write availability.
+  fs.writeFileSync(filePath, normalized, encoding);
 }
 
 export function platformReadSync(filePath: string, opts: { encoding?: BufferEncoding; required?: boolean } = {}): string | null {

tests/feat-3595-fs-fault-injection-atomic-write.test.cjs

@@ -98,6 +98,71 @@ test('platformWriteSync recovers when renameSync fails (EXDEV cross-device fallb
   assert.deepEqual(orphanTmpFiles(dir), [], 'tmp file must be cleaned up after rename failure');
 });
 
+// ─── #1540: transient Windows lock (EPERM/EBUSY/EACCES) is RETRIED, never
+//            fallen back to a non-atomic truncating write ───────────────────
+
+test('platformWriteSync retries a transient EPERM rename and publishes atomically (#1540)', (t) => {
+  const dir = mkScratch('eperm-transient');
+  t.after(() => cleanup(dir));
+  const file = path.join(dir, 'STATE.md');
+
+  // A reader briefly holds the target open → rename throws EPERM once, then clears.
+  let renameCalls = 0;
+  const originalRename = fs.renameSync;
+  const renameMock = mock.method(fs, 'renameSync', (src, dest) => {
+    renameCalls++;
+    if (renameCalls === 1) {
+      const err = new Error('EPERM: a reader holds the target open');
+      err.code = 'EPERM';
+      throw err;
+    }
+    return originalRename.call(fs, src, dest);
+  });
+  t.after(() => renameMock.mock.restore());
+
+  platformWriteSync(file, 'published\n');
+
+  assert.equal(renameCalls, 2, 'rename retried after a transient EPERM (not a single-shot non-atomic fallback)');
+  assert.equal(fs.statSync(file).isFile(), true);
+  assert.ok(fs.statSync(file).size > 0, 'target published, not truncated');
+  assert.deepEqual(orphanTmpFiles(dir), [], 'atomic publish leaves no tmp orphan');
+});
+
+test('platformWriteSync surfaces a PERSISTENT EPERM instead of truncating a concurrent reader (#1540)', (t) => {
+  const dir = mkScratch('eperm-persistent');
+  t.after(() => cleanup(dir));
+  const file = path.join(dir, 'STATE.md');
+  // A reader is mid-read on `file` with known content. The old blanket fallback
+  // would non-atomically writeFileSync over it — truncating the reader. The fix
+  // must surface the error and leave the existing file byte-for-byte intact.
+  fs.writeFileSync(file, 'OLD CONTENT A READER IS MID-READ ON\n');
+  const sizeBefore = fs.statSync(file).size;
+
+  let renameCalls = 0;
+  const renameMock = mock.method(fs, 'renameSync', () => {
+    renameCalls++;
+    const err = new Error('EPERM: reader holds the target open');
+    err.code = 'EPERM';
+    throw err;
+  });
+  t.after(() => renameMock.mock.restore());
+
+  let caught;
+  try {
+    platformWriteSync(file, 'NEW CONTENT\n');
+  } catch (err) {
+    caught = err;
+  }
+
+  assert.ok(caught, 'a persistent rename lock must surface as an error, not a silent truncating write');
+  assert.equal(caught.code, 'EPERM');
+  assert.equal(renameCalls, 3, 'rename retried up to the bounded limit before surfacing');
+  // Negative proof: the concurrent reader's file was NOT truncated/overwritten.
+  assert.equal(fs.statSync(file).size, sizeBefore, 'target left intact — no non-atomic write happened');
+  assert.equal(fs.readFileSync(file, 'utf-8'), 'OLD CONTENT A READER IS MID-READ ON\n');
+  assert.deepEqual(orphanTmpFiles(dir), [], 'tmp cleaned up after surfacing the error');
+});
+
 // ─── Tmp write failure → falls back to direct write ─────────────────────────
 
 test('platformWriteSync falls back when initial tmp writeFileSync fails (ENOSPC)', (t) => {
@@ -383,13 +448,12 @@ test('platformWriteSync survives a concurrent collision on the same target path'
 
   // First write completes normally.
   platformWriteSync(file, '{"writer":"first"}\n');
-  // Second write: inject a transient rename failure on the first
-  // attempt, then succeed via fallback. Capture the real renameSync
-  // BEFORE installing the mock so subsequent calls (defensive — the
-  // fallback path bypasses rename, so the second call shouldn't fire)
-  // delegate to the real implementation. The previous form referenced
-  // a non-existent `fs.renameSync.wrapped` property — that branch
-  // would silently no-op instead of delegating.
+  // Second write: inject a transient EBUSY on the first rename attempt,
+  // then succeed on the bounded retry (#1540). Capture the real renameSync
+  // BEFORE installing the mock so the retry attempt delegates to the real
+  // implementation. The previous form referenced a non-existent
+  // `fs.renameSync.wrapped` property — that branch would silently no-op
+  // instead of delegating.
   let renameCalls = 0;
   const originalRename = fs.renameSync;
   const renameMock = mock.method(fs, 'renameSync', (src, dest) => {
@@ -405,7 +469,7 @@ test('platformWriteSync survives a concurrent collision on the same target path'
 
   platformWriteSync(file, '{"writer":"second"}\n');
 
-  // The fallback path wrote 'second' content directly.
+  // The bounded retry re-published the 'second' content atomically.
   const final = fs.readFileSync(file, 'utf-8');
   // Must be valid JSON — never a half-merged corruption.
   assert.doesNotThrow(() => JSON.parse(final), 'file must remain parseable after the contested write');