Consolidate atomic JSON replacement

scripts/app-layout.ts

@@ -5,6 +5,7 @@ import path from 'node:path'
 import { rendererContentSecurityPolicy } from '../src/renderer-security'
 
 export const runtimeModuleNames = [
+  'atomic-json',
   'about-panel',
   'agent-guidance',
   'agent-reviewer-guidance',

src/atomic-json.ts

@@ -0,0 +1,40 @@
+import { randomBytes } from 'node:crypto'
+import fs from 'node:fs/promises'
+import path from 'node:path'
+
+export interface AtomicJsonReplacementOptions {
+  platform?: NodeJS.Platform
+}
+
+function errorCode(error: unknown): unknown {
+  return error !== null && typeof error === 'object'
+    ? Reflect.get(error, 'code')
+    : null
+}
+
+export async function replaceJsonFile(
+  filePath: string,
+  value: unknown,
+  { platform = process.platform }: AtomicJsonReplacementOptions = {}
+): Promise<void> {
+  const contents = `${JSON.stringify(value, null, 2)}\n`
+  const temporaryPath = path.join(
+    path.dirname(filePath),
+    `.${path.basename(filePath)}-${String(process.pid)}-${randomBytes(6).toString('hex')}.tmp`
+  )
+
+  try {
+    await fs.writeFile(temporaryPath, contents, {
+      encoding: 'utf8',
+      flag: 'wx',
+      flush: true,
+      mode: 0o600
+    })
+    if (platform !== 'win32') await fs.chmod(temporaryPath, 0o600)
+    await fs.rename(temporaryPath, filePath)
+  } finally {
+    await fs.unlink(temporaryPath).catch((error: unknown) => {
+      if (errorCode(error) !== 'ENOENT') throw error
+    })
+  }
+}

src/review-store.ts

@@ -2,7 +2,7 @@ import { createHash, randomBytes } from 'node:crypto'
 import fs from 'node:fs/promises'
 import path from 'node:path'
 import { isDeepStrictEqual } from 'node:util'
-
+import { replaceJsonFile } from './atomic-json'
 import { MAXIMUM_ATTACHMENT_BYTES } from './attachment-limits'
 import { guidance } from './agent-guidance'
 import { reviewerGuidance } from './agent-reviewer-guidance'
@@ -1690,23 +1690,7 @@ export class ReviewStore {
   }
 
   async writeFile(filePath: string, artifact: unknown): Promise<void> {
-    const temporaryPath = path.join(
-      path.dirname(filePath),
-      `.review-${String(process.pid)}-${randomBytes(6).toString('hex')}.tmp`
-    )
-
-    try {
-      await fs.writeFile(
-        temporaryPath,
-        `${JSON.stringify(artifact, null, 2)}\n`,
-        { encoding: 'utf8', flag: 'wx', flush: true }
-      )
-      await fs.rename(temporaryPath, filePath)
-    } finally {
-      await fs.unlink(temporaryPath).catch((error: unknown) => {
-        if (errorCode(error) !== 'ENOENT') throw error
-      })
-    }
+    await replaceJsonFile(filePath, artifact)
   }
 
   serialize<T>(key: string, operation: () => Promise<T>): Promise<T> {

src/service-endpoint.ts

@@ -3,6 +3,8 @@ import fs from 'node:fs/promises'
 import os from 'node:os'
 import path from 'node:path'
 
+import { replaceJsonFile } from './atomic-json'
+
 export const CAPABILITY_TOKEN_PATTERN = /^[A-Za-z0-9_-]{43}$/
 export const SERVICE_INSTANCE_PATTERN =
   /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i
@@ -128,28 +130,7 @@ async function writePrivateJson(
   value: unknown,
   platform: NodeJS.Platform
 ): Promise<void> {
-  const temporaryPath = path.join(
-    path.dirname(filePath),
-    `.${path.basename(filePath)}-${String(process.pid)}-${randomBytes(6).toString('hex')}.tmp`
-  )
-  try {
-    await fs.writeFile(temporaryPath, `${JSON.stringify(value, null, 2)}\n`, {
-      encoding: 'utf8',
-      flag: 'wx',
-      flush: true,
-      mode: 0o600
-    })
-    if (platform !== 'win32') await fs.chmod(temporaryPath, 0o600)
-    await fs.rename(temporaryPath, filePath)
-  } finally {
-    await fs.unlink(temporaryPath).catch((error: unknown) => {
-      if (
-        error === null ||
-        typeof error !== 'object' ||
-        Reflect.get(error, 'code') !== 'ENOENT'
-      ) throw error
-    })
-  }
+  await replaceJsonFile(filePath, value, { platform })
 }
 
 export interface PublishServiceConnectionOptions {

src/settings-store.ts

@@ -1,6 +1,7 @@
 import fs from 'node:fs/promises'
 import path from 'node:path'
 
+import { replaceJsonFile } from './atomic-json'
 import { DEFAULT_SETTINGS, normalizeSettings, updateSettings } from './settings'
 
 interface SettingsReadResult {
@@ -18,15 +19,13 @@ export class SettingsStore {
   settings: MarkoverSettings
   private readonly initialSettings: MarkoverSettings
   private writer: Promise<unknown>
-  private writeSequence: number
 
   constructor(filePath: string, initialSettings: unknown = DEFAULT_SETTINGS) {
     this.filePath = filePath
     this.lastRecoveryWarning = null
     this.initialSettings = normalizeSettings(initialSettings)
     this.settings = { ...this.initialSettings }
     this.writer = Promise.resolve()
-    this.writeSequence = 0
   }
 
   private async readResult(): Promise<SettingsReadResult> {
@@ -67,25 +66,12 @@ export class SettingsStore {
   }
 
   async update(patch: unknown): Promise<MarkoverSettings> {
-    const sequence = ++this.writeSequence
     const write = this.writer.catch(() => undefined).then(async () => {
       await fs.mkdir(path.dirname(this.filePath), { recursive: true })
-      const temporaryPath = `${this.filePath}.${String(process.pid)}.${String(sequence)}.tmp`
-      try {
-        const snapshot = updateSettings(this.settings, patch)
-        await fs.writeFile(
-          temporaryPath,
-          `${JSON.stringify(snapshot, null, 2)}\n`,
-          'utf8'
-        )
-        await fs.rename(temporaryPath, this.filePath)
-        this.settings = snapshot
-        return { ...snapshot }
-      } finally {
-        await fs.unlink(temporaryPath).catch((error: unknown) => {
-          if (errorProperty(error, 'code') !== 'ENOENT') throw error
-        })
-      }
+      const snapshot = updateSettings(this.settings, patch)
+      await replaceJsonFile(this.filePath, snapshot)
+      this.settings = snapshot
+      return { ...snapshot }
     })
     this.writer = write.then(() => undefined, () => undefined)
     return write

src/workspace-store.ts

@@ -1,6 +1,7 @@
 import fs from 'node:fs/promises'
 import path from 'node:path'
 
+import { replaceJsonFile } from './atomic-json'
 import {
   cloneWorkspaceState,
   defaultWorkspaceState,
@@ -23,7 +24,6 @@ export class WorkspaceStore {
   private writer: Promise<void> = Promise.resolve()
   private latestWrite: Promise<void> = Promise.resolve()
   private latestSnapshot: MarkoverWorkspaceState | null = null
-  private writeSequence = 0
 
   constructor(filePath: string) {
     this.filePath = filePath
@@ -66,23 +66,10 @@ export class WorkspaceStore {
   }
 
   private enqueueWrite(snapshot: MarkoverWorkspaceState): Promise<void> {
-    const sequence = ++this.writeSequence
     const write = this.writer.catch(() => undefined).then(async () => {
       await fs.mkdir(path.dirname(this.filePath), { recursive: true })
-      const temporaryPath = `${this.filePath}.${String(process.pid)}.${String(sequence)}.tmp`
-      try {
-        await fs.writeFile(
-          temporaryPath,
-          `${JSON.stringify(snapshot, null, 2)}\n`,
-          { encoding: 'utf8', mode: 0o600 }
-        )
-        await fs.rename(temporaryPath, this.filePath)
-        this.state = cloneWorkspaceState(snapshot)
-      } finally {
-        await fs.unlink(temporaryPath).catch((error: unknown) => {
-          if (errorProperty(error, 'code') !== 'ENOENT') throw error
-        })
-      }
+      await replaceJsonFile(this.filePath, snapshot)
+      this.state = cloneWorkspaceState(snapshot)
     })
     this.writer = write.then(() => undefined, () => undefined)
     this.latestWrite = write

test/atomic-json.test.ts

@@ -0,0 +1,46 @@
+import assert from 'node:assert/strict'
+import fs from 'node:fs/promises'
+import os from 'node:os'
+import path from 'node:path'
+import test from 'node:test'
+
+import { replaceJsonFile } from '../src/atomic-json'
+
+test('atomically replaces private pretty-printed JSON without temporary files', async (t) => {
+  const directory = await fs.mkdtemp(path.join(os.tmpdir(), 'markover-atomic-json-'))
+  t.after(() => fs.rm(directory, { recursive: true, force: true }))
+  const filePath = path.join(directory, 'state.json')
+  await fs.writeFile(filePath, '{}\n', { mode: 0o666 })
+
+  await replaceJsonFile(filePath, { current: true })
+
+  assert.equal(await fs.readFile(filePath, 'utf8'), '{\n  "current": true\n}\n')
+  assert.deepEqual(await fs.readdir(directory), ['state.json'])
+  if (process.platform !== 'win32') {
+    assert.equal((await fs.stat(filePath)).mode & 0o777, 0o600)
+  }
+})
+
+test('preserves the destination when JSON serialization fails before replacement', async (t) => {
+  const directory = await fs.mkdtemp(path.join(os.tmpdir(), 'markover-atomic-json-'))
+  t.after(() => fs.rm(directory, { recursive: true, force: true }))
+  const filePath = path.join(directory, 'state.json')
+  await fs.writeFile(filePath, '{"current":true}\n')
+
+  await assert.rejects(replaceJsonFile(filePath, { unsupported: 1n }), TypeError)
+
+  assert.equal(await fs.readFile(filePath, 'utf8'), '{"current":true}\n')
+  assert.deepEqual(await fs.readdir(directory), ['state.json'])
+})
+
+test('cleans the private temporary file when rename fails', async (t) => {
+  const directory = await fs.mkdtemp(path.join(os.tmpdir(), 'markover-atomic-json-'))
+  t.after(() => fs.rm(directory, { recursive: true, force: true }))
+  const destination = path.join(directory, 'state.json')
+  await fs.mkdir(destination)
+
+  await assert.rejects(replaceJsonFile(destination, { current: true }))
+
+  assert.deepEqual(await fs.readdir(directory), ['state.json'])
+  assert.deepEqual(await fs.readdir(destination), [])
+})

test/review-store.test.ts

@@ -2025,6 +2025,12 @@ test('rejects unsafe IDs and leaves no temporary files', async (t) => {
 
   const entries = await fs.readdir(store.reviewDirectory(created.review.id))
   assert.deepEqual(entries, ['review.json'])
+  if (process.platform !== 'win32') {
+    assert.equal(
+      (await fs.stat(store.reviewPath(created.review.id))).mode & 0o777,
+      0o600
+    )
+  }
 })
 
 test('publishes complete sessions and ignores incomplete review directories', async (t) => {

test/settings.test.ts

@@ -418,6 +418,9 @@ test('settings store serializes rapid updates without losing the latest values',
   assert.equal(saved.appearance, 'dark')
   assert.equal(saved.treeDensity, 'compact')
   assert.deepEqual(await fs.readdir(directory), ['settings.json'])
+  if (process.platform !== 'win32') {
+    assert.equal((await fs.stat(filePath)).mode & 0o777, 0o600)
+  }
 })
 
 test('settings store loads offline manual edits after restart', async (t) => {