fix(#4091): publish update-check cache via atomic temp+rename (#4313)

* test(#4091): RED — worker cache write must be rename-atomic

Structural regression guard: gsd-check-update-worker.js must not write the
shared per-package cache in place; assert temp+rename publish shape plus a
behavioral no-residue e2e.

* fix(#4091): publish update-check cache via atomic temp+rename

writeFileSync on the shared per-package cache truncates before writing, so
concurrent cross-runtime readers (statusline/banner) could see a torn or
empty record. Stage under a unique same-directory temp and rename into
place — POSIX rename(2) is atomic, so readers see the old or new record,
never a partial one. Degrade policy (#3582) unchanged: errors swallowed,
temp best-effort removed.

* refactor(#4091): hoist temp path, tighten uniqueness assertion (review)

* changeset(#4091): add fragment (pr backfill to follow)

* changeset(#4091): backfill PR 4313

---------

Co-authored-by: sim <sim@local>

.changeset/vivid-pumas-roam.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 4313
+---
+**Update-check cache is now published atomically** — the statusline update segment no longer intermittently goes blank when several runtimes (Claude Code, Codex, Cursor, ...) share one machine. (#4091)

hooks/gsd-check-update-worker.js

@@ -156,5 +156,22 @@ const result = {
 };
 
 if (cacheFile) {
-  try { fs.writeFileSync(cacheFile, JSON.stringify(result)); } catch (e) {}
-}
+  // #4091: the cache file is shared per-PACKAGE across every runtime's worker
+  // (#607/#1421), so concurrent statusline/banner readers parse it while this
+  // worker writes it. A direct writeFileSync truncates before writing — a
+  // reader landing mid-write sees a torn/empty record (its JSON.parse catch
+  // swallows it, so the symptom is an intermittently blank update segment).
+  // Publish atomically instead: stage under a unique same-directory temp
+  // (same filesystem, so rename(2) is atomic — readers see the old or the new
+  // record, never a partial one), then renameSync into place. Failure policy
+  // is unchanged (#3582 degrade): any error is swallowed and the temp, if
+  // left behind, is best-effort removed.
+  const tmp = cacheFile + '.tmp-' + process.pid;
+  try {
+    fs.writeFileSync(tmp, JSON.stringify(result));
+    fs.renameSync(tmp, cacheFile);
+  } catch (e) {
+    try {
+      fs.rmSync(tmp, { force: true });
+    } catch (e2) {}
+  }}

tests/gsd-check-update-worker-atomic-cache.test.cjs

@@ -0,0 +1,118 @@
+/**
+ * Tests for the atomic cache publish in gsd-check-update-worker.js (#4091).
+ *
+ * Background (issue #4091):
+ *   The worker used to write its result cache in place
+ *   (`fs.writeFileSync(cacheFile, ...)`), which truncates before writing. The
+ *   cache file is deliberately per-PACKAGE (#607/#1421), so on a machine with
+ *   several runtimes (Claude Code + Codex + Cursor, ...) every runtime's
+ *   SessionStart worker writes the SAME file while every statusline/banner
+ *   refresh reads it. A reader landing mid-write saw an empty or truncated
+ *   record; the readers' JSON.parse catch swallowed it, so the symptom was an
+ *   intermittently blank update segment.
+ *
+ * The fix publishes via write-temp-then-rename — rename(2) over a file in the
+ * same directory is atomic, so readers see either the old or the new record,
+ * never a torn one.
+ *
+ * Source-grep policy: the torn write is a concurrency defect that cannot be
+ * observed on POSIX CI without a genuine multi-process race (forbidden by
+ * repo test rules), so the contract is asserted structurally on the write
+ * shape — same policy as the #3103 platform-gate tests in
+ * gsd-check-update-worker-platform-gate.test.cjs (structural-regression-guard).
+ */
+
+'use strict';
+
+const { test, describe } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('fs');
+const path = require('path');
+const { runHook: runHookSeam } = require('./helpers/process-seam.cjs');
+const { createTempDir, cleanup } = require('./helpers.cjs');
+
+const WORKER_PATH = path.join(__dirname, '..', 'hooks', 'gsd-check-update-worker.js');
+
+// allow-test-rule: structural-regression-guard (#4091)
+// Feeds the real worker source (readFileSync) into the structural assertions
+// below. The behavior it guards — rename-atomic publish of a file shared
+// across runtime processes — needs a multi-process race to observe, which
+// repo test rules forbid, so a structural assertion on the write shape is
+// the minimum-cost contract (same policy as the #3103 platform gate).
+function codeOnly(file) {
+  return fs.readFileSync(file, 'utf8')
+    // eslint-disable-next-line local/no-unbounded-quantifier -- parses this repo's own bounded hooks source, not adversarial input
+    .replace(/\/\*[\s\S]*?\*\//g, '')
+    // eslint-disable-next-line local/no-unbounded-quantifier -- parses this repo's own hooks source, not adversarial input
+    .replace(/(^|[^:])\/\/[^\r\n]*/g, '$1');
+}
+
+describe('gsd-check-update-worker.js: atomic cache publish (#4091)', () => {
+  test('worker must not write the shared cache file in place', () => {
+    const src = codeOnly(WORKER_PATH);
+    // The only legitimate direct write target is a temp stage; the cache file
+    // itself must never be the writeFileSync destination.
+    assert.ok(
+      !/writeFileSync\(\s*cacheFile\b/.test(src),
+      'writeFileSync(cacheFile, ...) truncates the shared per-package cache in place; ' +
+        'concurrent readers see a torn/empty record (#4091). Stage a temp file and rename it.',
+    );
+  });
+
+  test('worker publishes the cache via rename-atomic replace (torn-write guard #4091)', () => {
+    const src = codeOnly(WORKER_PATH);
+    // Stage: write a unique temp derived from cacheFile (same directory =>
+    // same filesystem => POSIX rename(2) atomicity).
+    assert.match(
+      src,
+      /const\s+\w+\s*=\s*cacheFile\s*\+\s*['"][^'"]*tmp[^'"]*['"]/,
+      'cache publish must stage a temp path derived from cacheFile (same-directory rename is atomic)',
+    );
+    // Publish: renameSync(tmp, cacheFile).
+    assert.match(
+      src,
+      /renameSync\(\s*\w+\s*,\s*cacheFile\s*\)/,
+      'cache publish must rename the staged temp into place, never write the cache file directly',
+    );
+  });
+
+  test('temp stage path is same-directory by construction', () => {
+    const src = codeOnly(WORKER_PATH);
+    // A cross-filesystem temp (os.tmpdir()) would make the rename non-atomic
+    // (copy+unlink fallback on some platforms) — rejected in #4091 diagnosis.
+    assert.ok(
+      !/os\.tmpdir\(\)/.test(src),
+      'temp stage must be derived from cacheFile (same directory), not os.tmpdir()',
+    );
+    assert.match(
+      src,
+      /cacheFile\s*\+\s*['"][^'"]*['"]\s*\+\s*process\.pid/,
+      'temp stage name must be unique per process (process.pid) so concurrent workers never share a stage path',
+    );
+  });
+
+  // Behavioral end-to-end: the worker (run as a real child process, same seam
+  // as the #3582 cold-tree test) must leave a parseable cache and no temp
+  // residue. Guards the happy path of the rename publish.
+  test('worker run leaves a valid cache and no temp residue', (t) => {
+    const cacheDir = createTempDir('gsd-worker-atomic-');
+    t.after(() => cleanup(cacheDir));
+    const cacheFile = path.join(cacheDir, 'cache.json');
+    // Pre-existing record from "another runtime" — the rename must replace it,
+    // not truncate it (readers between runs always see a complete record).
+    fs.writeFileSync(cacheFile, JSON.stringify({ update_available: false, installed: '0.0.1', latest: 'unknown', checked: 1, package_name: null }), 'utf8');
+
+    const env = {
+      ...process.env,
+      GSD_CACHE_FILE: cacheFile,
+      GSD_PROJECT_VERSION_FILE: path.join(cacheDir, 'no-such-project', 'VERSION'),
+      GSD_GLOBAL_VERSION_FILE: path.join(cacheDir, 'no-such-global', 'VERSION'),
+    };
+    const r = runHookSeam(WORKER_PATH, [], { env, timeoutMs: 15000 });
+    assert.equal(r.exitCode, 0, `worker must exit 0; stderr: ${r.stderr}`);
+    const cache = JSON.parse(fs.readFileSync(cacheFile, 'utf8'));
+    assert.equal(cache.installed, '0.0.0', 'worker record replaced the pre-existing one');
+    const residue = fs.readdirSync(cacheDir).filter((f) => f.startsWith('cache.json.tmp') || f.includes('.tmp-'));
+    assert.deepEqual(residue, [], 'no temp stage files may remain after a successful publish');
+  });
+});