Merge pull request #153 from Lykhoyda/fix/issue-111-atomic-writer-unique-tmp

fix(gh-111): unique .tmp suffixes + age-bounded cleanupOrphans

Author: Lykhoyda

.claude-plugin/marketplace.json

@@ -9,7 +9,7 @@
     {
       "name": "rn-dev-agent",
       "description": "AI agent that fully tests React Native features on simulator/emulator — navigates the app, verifies UI, walks user flows, and confirms internal state.",
-      "version": "0.44.39",
+      "version": "0.44.40",
       "source": "./",
       "category": "mobile-development",
       "homepage": "https://github.com/Lykhoyda/rn-dev-agent"

.claude-plugin/plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "rn-dev-agent",
-  "version": "0.44.39",
+  "version": "0.44.40",
   "description": "AI agent that fully tests React Native features on simulator/emulator — navigates the app, verifies UI, walks user flows, and confirms internal state.",
   "author": {
     "name": "Anton Lykhoyda",

CHANGELOG.md

@@ -4,6 +4,27 @@ All notable changes to rn-dev-agent will be documented in this file.
 
 Format follows [Keep a Changelog](https://keepachangelog.com/).
 
+## [0.44.40] — 2026-05-13
+
+### Hardened (GH #111 — atomic-writer concurrent pairWrite races)
+
+- **`pairWrite` now uses unique `.tmp.<pid>.<time36>.<rand36>` suffixes** so
+  two concurrent writers against the same action don't share a tmp namespace.
+  Without this, `cleanupOrphans` could `unlinkSync` writer A's in-flight tmp
+  file mid-rename, producing an opaque ENOENT for the user. Gemini flagged
+  the failure mode at conf 82 in the PR #109 multi-LLM review.
+- **`cleanupOrphans` is age-bounded**: scans the target directory for files
+  matching the path prefix and only unlinks orphans older than
+  `ORPHAN_MAX_AGE_MS = 5 minutes`. Concurrent writer's fresh tmp file
+  preserved; crashed process's stale tmp file becomes eligible for sweep
+  after 5 minutes.
+- **New `_readdir` test seam** for deterministic cleanup mocking.
+- 7 new regression tests cover unique-stamp generation, stale-orphan sweep,
+  fresh-orphan preservation, prefix anchoring, missing-dir tolerance, the
+  exported constant value, and round-trip orphan-free state across 5
+  repeated `pairWrite` calls. Three existing tests updated to match the
+  new `.tmp.<stamp>` filename shape. Suite: 1312 → 1319 passing.
+
 ## [0.44.39] — 2026-05-13
 
 ### Hardened (GH #110 — agent-device test seam fuse)

scripts/cdp-bridge/dist/domain/atomic-writer.js

@@ -34,8 +34,8 @@
 //
 // Test seam: the public API is on a single exported object so tests can
 // `mock.method(atomicWriter, '_writeFile', ...)` to inject failures.
-import { writeFileSync, renameSync, statSync, mkdirSync, existsSync, unlinkSync, } from 'node:fs';
-import { dirname } from 'node:path';
+import { writeFileSync, renameSync, statSync, mkdirSync, existsSync, unlinkSync, readdirSync, } from 'node:fs';
+import { dirname, basename } from 'node:path';
 // Multi-LLM review of PR #109 findings 1+2: `finalMtimeMs = _stat(yaml)`
 // breaks the safety invariant in two scenarios — (a) slow writes where
 // the actual YAML mtime exceeds `projectedMtimeMs` and step 5 happens
@@ -56,6 +56,23 @@ import { dirname } from 'node:path';
  * 1 second satisfies both with a safe margin.
  */
 export const FUTURE_MTIME_BUFFER_MS = 1_000;
+/**
+ * GH #111: orphan .tmp files older than this threshold are eligible for
+ * cleanup. Any process that's been alive for 5 minutes hasn't crashed
+ * mid-pairWrite — the orphan must be from a prior crashed run. Concurrent
+ * pairWrite calls don't collide because each call uses a unique stamp,
+ * but a stale orphan from a crashed process would otherwise stick around
+ * forever. 5 minutes is conservative (allows long fsync queues / CI
+ * antivirus stalls).
+ */
+export const ORPHAN_MAX_AGE_MS = 5 * 60 * 1_000;
+/** Generate a unique tmp-file stamp per pairWrite call. Crash-resistant
+ *  and concurrent-safe — two pairWrites for the same action path won't
+ *  collide because each owns its own tmp namespace. */
+function generateTmpStamp() {
+    const rand = Math.random().toString(36).slice(2, 10);
+    return `${process.pid}.${Date.now().toString(36)}.${rand}`;
+}
 /**
  * Atomic write of a (YAML, sidecar) pair using sidecar-first ordering.
  * Returns the resolved paths plus the final on-disk mtime. Throws if any
@@ -76,8 +93,13 @@ export const FUTURE_MTIME_BUFFER_MS = 1_000;
 function pairWriteImpl(yamlPath, yamlContent, sidecarPath, state) {
     ensureDir(yamlPath);
     ensureDir(sidecarPath);
-    const yamlTmp = `${yamlPath}.tmp`;
-    const sidecarTmp = `${sidecarPath}.tmp`;
+    // GH #111: unique stamp per call so two concurrent pairWrites against
+    // the same action id never share a tmp namespace. Without this, B's
+    // cleanupOrphans could unlink A's in-flight .tmp file and produce an
+    // opaque ENOENT during A's rename.
+    const stamp = generateTmpStamp();
+    const yamlTmp = `${yamlPath}.tmp.${stamp}`;
+    const sidecarTmp = `${sidecarPath}.tmp.${stamp}`;
     // Step 1+2: sidecar with projected future mtime, atomic rename.
     const projectedMtimeMs = Date.now() + FUTURE_MTIME_BUFFER_MS;
     const projectedState = {
@@ -123,21 +145,41 @@ function ensureDir(filePath) {
         atomicWriter._mkdir(dir);
 }
 /**
- * Best-effort cleanup of orphaned `.tmp` files left by a crashed previous
- * call. Called by `pairWrite` before each operation. Idempotent.
+ * Best-effort cleanup of orphaned `.tmp.<stamp>` files left by a crashed
+ * previous call (GH #111). Called by `pairWrite` before each operation.
+ * Idempotent.
+ *
+ * Only removes `.tmp.<stamp>` files matching the target path's prefix
+ * AND older than ORPHAN_MAX_AGE_MS — concurrent writers' fresh tmp files
+ * are untouched. A crashed process's stale tmp file becomes eligible
+ * after 5 minutes, well past any plausible pairWrite duration.
  *
- * Routes through `atomicWriter._unlink` / `_exists` so PR #109 review
- * finding (E) — "tests can't simulate mkdir/unlink failures" — is
- * resolved: the seam is now complete across all fs operations the
- * writer performs.
+ * Routes through `atomicWriter._readdir` / `_statMtimeMs` / `_unlink`
+ * so tests can mock cleanup behavior deterministically.
  */
 function cleanupOrphans(yamlPath, sidecarPath) {
-    for (const orphan of [`${yamlPath}.tmp`, `${sidecarPath}.tmp`]) {
-        if (atomicWriter._exists(orphan)) {
+    const now = Date.now();
+    for (const targetPath of [yamlPath, sidecarPath]) {
+        const dir = dirname(targetPath);
+        const prefix = `${basename(targetPath)}.tmp.`;
+        let entries;
+        try {
+            entries = atomicWriter._readdir(dir);
+        }
+        catch {
+            continue; // dir doesn't exist yet — no orphans possible
+        }
+        for (const entry of entries) {
+            if (!entry.startsWith(prefix))
+                continue;
+            const orphanPath = `${dir}/${entry}`;
             try {
-                atomicWriter._unlink(orphan);
+                const mtimeMs = atomicWriter._statMtimeMs(orphanPath);
+                if (now - mtimeMs < ORPHAN_MAX_AGE_MS)
+                    continue; // fresh — likely a concurrent writer's
+                atomicWriter._unlink(orphanPath);
             }
-            catch { /* ignore */ }
+            catch { /* best-effort */ }
         }
     }
 }
@@ -172,6 +214,10 @@ export const atomicWriter = {
     _unlink(path) {
         unlinkSync(path);
     },
+    /** Underlying `fs.readdirSync(path)`. Used by GH #111 prefix-scan cleanup. */
+    _readdir(path) {
+        return readdirSync(path);
+    },
     /**
      * Atomic pair-write. Cleans up any orphaned `.tmp` files before
      * starting. Throws on the first failed step — caller decides whether

scripts/cdp-bridge/package.json

@@ -1,6 +1,6 @@
 {
   "name": "rn-dev-agent-cdp",
-  "version": "0.38.34",
+  "version": "0.38.35",
   "type": "module",
   "main": "dist/index.js",
   "scripts": {

scripts/cdp-bridge/src/domain/atomic-writer.ts

@@ -42,8 +42,9 @@ import {
   mkdirSync,
   existsSync,
   unlinkSync,
+  readdirSync,
 } from 'node:fs';
-import { dirname } from 'node:path';
+import { dirname, basename } from 'node:path';
 import type { ActionRuntimeState } from './reusable-action.js';
 
 // Multi-LLM review of PR #109 findings 1+2: `finalMtimeMs = _stat(yaml)`
@@ -68,6 +69,25 @@ import type { ActionRuntimeState } from './reusable-action.js';
  */
 export const FUTURE_MTIME_BUFFER_MS = 1_000;
 
+/**
+ * GH #111: orphan .tmp files older than this threshold are eligible for
+ * cleanup. Any process that's been alive for 5 minutes hasn't crashed
+ * mid-pairWrite — the orphan must be from a prior crashed run. Concurrent
+ * pairWrite calls don't collide because each call uses a unique stamp,
+ * but a stale orphan from a crashed process would otherwise stick around
+ * forever. 5 minutes is conservative (allows long fsync queues / CI
+ * antivirus stalls).
+ */
+export const ORPHAN_MAX_AGE_MS = 5 * 60 * 1_000;
+
+/** Generate a unique tmp-file stamp per pairWrite call. Crash-resistant
+ *  and concurrent-safe — two pairWrites for the same action path won't
+ *  collide because each owns its own tmp namespace. */
+function generateTmpStamp(): string {
+  const rand = Math.random().toString(36).slice(2, 10);
+  return `${process.pid}.${Date.now().toString(36)}.${rand}`;
+}
+
 export interface PairWriteResult {
   yamlPath: string;
   sidecarPath: string;
@@ -103,8 +123,13 @@ function pairWriteImpl(
   ensureDir(yamlPath);
   ensureDir(sidecarPath);
 
-  const yamlTmp = `${yamlPath}.tmp`;
-  const sidecarTmp = `${sidecarPath}.tmp`;
+  // GH #111: unique stamp per call so two concurrent pairWrites against
+  // the same action id never share a tmp namespace. Without this, B's
+  // cleanupOrphans could unlink A's in-flight .tmp file and produce an
+  // opaque ENOENT during A's rename.
+  const stamp = generateTmpStamp();
+  const yamlTmp = `${yamlPath}.tmp.${stamp}`;
+  const sidecarTmp = `${sidecarPath}.tmp.${stamp}`;
 
   // Step 1+2: sidecar with projected future mtime, atomic rename.
   const projectedMtimeMs = Date.now() + FUTURE_MTIME_BUFFER_MS;
@@ -155,18 +180,37 @@ function ensureDir(filePath: string): void {
 }
 
 /**
- * Best-effort cleanup of orphaned `.tmp` files left by a crashed previous
- * call. Called by `pairWrite` before each operation. Idempotent.
+ * Best-effort cleanup of orphaned `.tmp.<stamp>` files left by a crashed
+ * previous call (GH #111). Called by `pairWrite` before each operation.
+ * Idempotent.
+ *
+ * Only removes `.tmp.<stamp>` files matching the target path's prefix
+ * AND older than ORPHAN_MAX_AGE_MS — concurrent writers' fresh tmp files
+ * are untouched. A crashed process's stale tmp file becomes eligible
+ * after 5 minutes, well past any plausible pairWrite duration.
  *
- * Routes through `atomicWriter._unlink` / `_exists` so PR #109 review
- * finding (E) — "tests can't simulate mkdir/unlink failures" — is
- * resolved: the seam is now complete across all fs operations the
- * writer performs.
+ * Routes through `atomicWriter._readdir` / `_statMtimeMs` / `_unlink`
+ * so tests can mock cleanup behavior deterministically.
  */
 function cleanupOrphans(yamlPath: string, sidecarPath: string): void {
-  for (const orphan of [`${yamlPath}.tmp`, `${sidecarPath}.tmp`]) {
-    if (atomicWriter._exists(orphan)) {
-      try { atomicWriter._unlink(orphan); } catch { /* ignore */ }
+  const now = Date.now();
+  for (const targetPath of [yamlPath, sidecarPath]) {
+    const dir = dirname(targetPath);
+    const prefix = `${basename(targetPath)}.tmp.`;
+    let entries: string[];
+    try {
+      entries = atomicWriter._readdir(dir);
+    } catch {
+      continue; // dir doesn't exist yet — no orphans possible
+    }
+    for (const entry of entries) {
+      if (!entry.startsWith(prefix)) continue;
+      const orphanPath = `${dir}/${entry}`;
+      try {
+        const mtimeMs = atomicWriter._statMtimeMs(orphanPath);
+        if (now - mtimeMs < ORPHAN_MAX_AGE_MS) continue; // fresh — likely a concurrent writer's
+        atomicWriter._unlink(orphanPath);
+      } catch { /* best-effort */ }
     }
   }
 }
@@ -202,6 +246,10 @@ export const atomicWriter = {
   _unlink(path: string): void {
     unlinkSync(path);
   },
+  /** Underlying `fs.readdirSync(path)`. Used by GH #111 prefix-scan cleanup. */
+  _readdir(path: string): string[] {
+    return readdirSync(path);
+  },
 
   /**
    * Atomic pair-write. Cleans up any orphaned `.tmp` files before