fix(hooks): use shared renameWithRetry in writeWarnState (ecc-context-monitor)

Mirror the previous commit's Windows-EPERM retry on the companion
`writeWarnState` in `scripts/hooks/ecc-context-monitor.js`. Same
race: two PostToolUse subprocesses writing concurrent debounce
state racing on `MoveFileExW`, target-in-use throwing EPERM on
Windows even though each writer's tmp path is now unique.

Implementation: import `renameWithRetry` from `scripts/lib/session-bridge.js`
(exported in the previous commit) instead of duplicating the helper.
The retry policy, backoff schedule, and main-thread `Atomics.wait`
strategy stay identical to `writeBridgeAtomic`.

Three writers in the repo now share the same atomic-write contract:
- `writeBridgeAtomic` (scripts/lib/session-bridge.js) — round 1 +
  this round's retry
- `writeWarnState` (this file) — round 1 + this round's retry via shared helper
- `writeCostWarningIfChanged` (scripts/hooks/ecc-metrics-bridge.js) —
  out of scope for this PR (already uses unique tmp suffix; a future
  consolidation could move it to the shared helper too).

Local: `yarn test` green, `yarn lint` clean. The companion test
suite for `ecc-context-monitor.js` does not currently exercise
concurrent `writeWarnState` writes, but the helper it now uses is
covered by the `tests/lib/session-bridge.test.js` concurrent-write
regression added in round 1's last commit.

scripts/hooks/ecc-context-monitor.js

@@ -9,10 +9,11 @@
 
 'use strict';
 
+const crypto = require('crypto');
 const fs = require('fs');
 const os = require('os');
 const path = require('path');
-const { sanitizeSessionId, readBridge } = require('../lib/session-bridge');
+const { sanitizeSessionId, readBridge, renameWithRetry } = require('../lib/session-bridge');
 
 const CONTEXT_WARNING_PCT = 35;
 const CONTEXT_CRITICAL_PCT = 25;
@@ -61,15 +62,30 @@ function readWarnState(sessionId) {
 }
 
 /**
- * Write debounce state.
+ * Write debounce state atomically (unique-suffix tmp then rename).
+ *
+ * The tmp path includes `process.pid` plus a random nonce so concurrent
+ * PostToolUse subprocesses writing to the same session's warn-state
+ * file do not clobber each other's tmp mid-write. Without the unique
+ * suffix, two writers race over a shared `${target}.tmp` and produce
+ * either a corrupted payload or an ENOENT throw on the second rename.
+ *
+ * Same pattern as `writeBridgeAtomic` in `scripts/lib/session-bridge.js`
+ * and `writeCostWarningIfChanged` in `scripts/hooks/ecc-metrics-bridge.js`.
+ *
  * @param {string} sessionId
  * @param {object} state
  */
 function writeWarnState(sessionId, state) {
   const target = getWarnPath(sessionId);
-  const tmp = `${target}.tmp`;
+  const tmp = `${target}.${process.pid}.${crypto.randomBytes(4).toString('hex')}.tmp`;
   fs.writeFileSync(tmp, JSON.stringify(state), 'utf8');
-  fs.renameSync(tmp, target);
+  try {
+    renameWithRetry(tmp, target);
+  } catch (err) {
+    try { fs.unlinkSync(tmp); } catch { /* ignore */ }
+    throw err;
+  }
 }
 
 /**

scripts/lib/session-bridge.js

@@ -8,6 +8,7 @@
  * without scanning large JSONL logs on every invocation.
  */
 
+const crypto = require('crypto');
 const fs = require('fs');
 const os = require('os');
 const path = require('path');
@@ -51,15 +52,80 @@ function readBridge(sessionId) {
 }
 
 /**
- * Write bridge data atomically (write .tmp then rename).
+ * Write bridge data atomically (write unique-suffix tmp then rename).
+ *
+ * The tmp path includes `process.pid` plus a random nonce so concurrent
+ * writers (e.g. PostToolUse `ecc-metrics-bridge` and the background
+ * `ecc-statusline`, both writing to the same session bridge) do not
+ * clobber each other's tmp file mid-write. With a fixed `.tmp` suffix
+ * two writers could both call `writeFileSync` against the same path
+ * before either reaches `renameSync`, causing one writer's payload to
+ * silently overwrite the other and the second `renameSync` to throw
+ * ENOENT once the rename consumes the file.
+ *
+ * Same pattern already used by `writeCostWarningIfChanged` in
+ * `scripts/hooks/ecc-metrics-bridge.js` (commit 9b1d8918) for the
+ * cost-warning cache; this commit applies it to the session-bridge
+ * primitive too.
+ *
  * @param {string} sessionId - Already-sanitized session ID
  * @param {object} data
  */
 function writeBridgeAtomic(sessionId, data) {
   const target = getBridgePath(sessionId);
-  const tmp = `${target}.tmp`;
+  const tmp = `${target}.${process.pid}.${crypto.randomBytes(4).toString('hex')}.tmp`;
   fs.writeFileSync(tmp, JSON.stringify(data), 'utf8');
-  fs.renameSync(tmp, target);
+  try {
+    renameWithRetry(tmp, target);
+  } catch (err) {
+    try { fs.unlinkSync(tmp); } catch { /* ignore */ }
+    throw err;
+  }
+}
+
+/**
+ * Replace a file via rename, retrying briefly on transient OS-level errors.
+ *
+ * POSIX `rename(2)` is atomic between source and destination, so concurrent
+ * writers each rename onto the same target without conflict. Windows
+ * `MoveFileExW` is different: it fails with EPERM/EACCES/EBUSY if the
+ * target is currently being renamed by *another* process — a short race
+ * window that fires reliably under our PostToolUse + statusline concurrency.
+ *
+ * To stay portable, retry up to 5 times with exponential backoff (20 ms,
+ * 40, 80, 160, 320) on the Windows-only transient codes. POSIX runs hit
+ * the first try and exit immediately. Other error codes (ENOENT, ENOSPC,
+ * EROFS, …) re-throw without retry — they are not transient.
+ *
+ * Sleep uses `Atomics.wait` on a throwaway SharedArrayBuffer so the
+ * retry path does not busy-spin the CPU. This works on the main thread
+ * in Node ≥ 17 (and on workers in earlier versions).
+ *
+ * @param {string} tmp
+ * @param {string} target
+ */
+function renameWithRetry(tmp, target) {
+  const RETRY_CODES = new Set(['EPERM', 'EACCES', 'EBUSY']);
+  const MAX_ATTEMPTS = 5;
+  for (let attempt = 0; ; attempt++) {
+    try {
+      fs.renameSync(tmp, target);
+      return;
+    } catch (err) {
+      if (attempt + 1 >= MAX_ATTEMPTS || !RETRY_CODES.has(err.code)) {
+        throw err;
+      }
+      const delayMs = 20 << attempt;
+      try {
+        Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, delayMs);
+      } catch {
+        // Atomics.wait throws on the main thread in some older runtimes;
+        // fall back to a brief busy-wait so the retry path still has a delay.
+        const until = Date.now() + delayMs;
+        while (Date.now() < until) { /* spin */ }
+      }
+    }
+  }
 }
 
 /**
@@ -76,6 +142,7 @@ module.exports = {
   getBridgePath,
   readBridge,
   writeBridgeAtomic,
+  renameWithRetry,
   resolveSessionId,
   MAX_SESSION_ID_LENGTH
 };

tests/lib/session-bridge.test.js

@@ -135,6 +135,130 @@ function runTests() {
     passed++;
   else failed++;
 
+  // Concurrency contract: two processes writing to the same session
+  // bridge must not throw ENOENT and must never leave a corrupt JSON
+  // file behind. The previous implementation used a fixed `${target}.tmp`
+  // suffix; with concurrent writers it raced over a shared tmp path,
+  // producing both ENOENT on rename and (occasionally) a half-written
+  // payload on the destination.
+  //
+  // This test exercises the atomic-rename primitive only — it does NOT
+  // attempt to defend against the read-modify-write race in callers,
+  // which is a separate concern. Each subprocess writes its own
+  // independent payload N times; we assert (a) every process exits 0
+  // (no ENOENT bubbled up) and (b) the final file is always parseable
+  // JSON whose contents match one of the two writers' last payloads.
+
+  if (
+    test('concurrent writeBridgeAtomic does not throw ENOENT or corrupt the bridge file', () => {
+      // Spawn two child processes that BOTH stay alive at the same time
+      // and call writeBridgeAtomic in a tight loop. `spawnSync` would
+      // run them sequentially (blocking on each), which would never
+      // exercise the race the fix targets. Instead a sync runner script
+      // launches both as async `spawn` children inside its own process,
+      // waits for both to exit, and reports their statuses on stdout —
+      // and the test calls *that* runner via `spawnSync`. The runner is
+      // the only place that needs the event loop.
+      const { spawnSync } = require('child_process');
+      const path = require('path');
+      const testId = `test-bridge-race-${Date.now()}-${process.pid}`;
+      const writerPath = path.join(__dirname, '..', '__tmp_bridge_writer.js');
+      const runnerPath = path.join(__dirname, '..', '__tmp_bridge_race_runner.js');
+      const bridgeLib = path.join(__dirname, '..', '..', 'scripts', 'lib', 'session-bridge');
+      fs.writeFileSync(
+        writerPath,
+        [
+          "const { writeBridgeAtomic } = require(" + JSON.stringify(bridgeLib) + ");",
+          "const [, , sid, tag] = process.argv;",
+          "for (let i = 0; i < 200; i++) {",
+          "  writeBridgeAtomic(sid, { writer: tag, i });",
+          "}",
+        ].join('\n'),
+        'utf8'
+      );
+      fs.writeFileSync(
+        runnerPath,
+        [
+          "'use strict';",
+          "const { spawn } = require('child_process');",
+          "const [, , writerPath, sid] = process.argv;",
+          "const c1 = spawn(process.execPath, [writerPath, sid, 'A'], { stdio: ['ignore','pipe','pipe'] });",
+          "const c2 = spawn(process.execPath, [writerPath, sid, 'B'], { stdio: ['ignore','pipe','pipe'] });",
+          "const exits = {};",
+          "const stderrs = { A: '', B: '' };",
+          "c1.stderr.on('data', chunk => { stderrs.A += chunk.toString(); });",
+          "c2.stderr.on('data', chunk => { stderrs.B += chunk.toString(); });",
+          "let done = 0;",
+          "function onExit(tag) { return function(code) { exits[tag] = code; if (++done === 2) finish(); }; }",
+          "c1.on('exit', onExit('A'));",
+          "c2.on('exit', onExit('B'));",
+          "function finish() {",
+          "  process.stdout.write(JSON.stringify({ exits, stderrs }));",
+          "  process.exit(0);",
+          "}",
+        ].join('\n'),
+        'utf8'
+      );
+      try {
+        const result = spawnSync('node', [runnerPath, writerPath, testId], { encoding: 'utf8' });
+        assert.strictEqual(result.status, 0,
+          `race runner should exit 0, got ${result.status}: ${result.stderr}`);
+        const parsed = JSON.parse(result.stdout);
+        assert.strictEqual(parsed.exits.A, 0,
+          `writer A should exit 0 (no ENOENT), got ${parsed.exits.A}: ${parsed.stderrs.A}`);
+        assert.strictEqual(parsed.exits.B, 0,
+          `writer B should exit 0 (no ENOENT), got ${parsed.exits.B}: ${parsed.stderrs.B}`);
+        // Final file must be parseable JSON and belong to one of the writers.
+        const final = readBridge(testId);
+        assert.ok(final && typeof final === 'object',
+          `expected parseable JSON object, got: ${JSON.stringify(final)}`);
+        assert.ok(final.writer === 'A' || final.writer === 'B',
+          `expected last-writer-wins payload, got: ${JSON.stringify(final)}`);
+      } finally {
+        try { fs.unlinkSync(getBridgePath(testId)); } catch { /* ignore */ }
+        try { fs.unlinkSync(writerPath); } catch { /* ignore */ }
+        try { fs.unlinkSync(runnerPath); } catch { /* ignore */ }
+      }
+    })
+  )
+    passed++;
+  else failed++;
+
+  if (
+    test('writeBridgeAtomic cleans up its tmp file on renameSync failure', () => {
+      // Trigger renameSync failure by passing a sessionId whose path is
+      // already a directory. The tmp file exists at this point; the fix
+      // must not leak it behind.
+      const path = require('path');
+      const testId = `test-bridge-cleanup-${Date.now()}-${process.pid}`;
+      const target = getBridgePath(testId);
+      const os = require('os');
+      const tmpDir = os.tmpdir();
+      // Plant a directory at the target path so renameSync (target.tmp → target) fails.
+      fs.mkdirSync(target);
+      try {
+        assert.throws(
+          () => writeBridgeAtomic(testId, { x: 1 }),
+          // renameSync of a regular file onto an existing directory throws
+          // EISDIR on Linux, EPERM on macOS, ENOTDIR on some BSDs. Accept
+          // any of those so the test stays portable across CI runners.
+          /EISDIR|EPERM|ENOTDIR|ENOENT/,
+          'expected rename failure to surface'
+        );
+        // Count any leaked tmp files. The pid+nonce suffix is unique per
+        // call, so we look for any matching pattern under os.tmpdir().
+        const prefix = path.basename(target) + '.' + process.pid + '.';
+        const leaked = fs.readdirSync(tmpDir).filter(f => f.startsWith(prefix) && f.endsWith('.tmp'));
+        assert.strictEqual(leaked.length, 0,
+          `expected no leaked tmp files after rename failure, found: ${leaked.join(', ')}`);
+      } finally {
+        try { fs.rmdirSync(target); } catch { /* ignore */ }
+      }
+    })
+  )
+    passed++;
+  else failed++;
+
   // resolveSessionId tests
   console.log('\nresolveSessionId:');