From c595eb2b228aed598f0ee5ed736cccf985dcac8f Mon Sep 17 00:00:00 2001 From: Jeremy Date: Tue, 7 Apr 2026 12:39:08 -0500 Subject: [PATCH] =?UTF-8?q?fix(gsd):=20write=20safety=20=E2=80=94=20atomic?= =?UTF-8?q?=20writes=20and=20randomized=20tmp=20paths=20(wave=204/5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three write-safety fixes: 1. json-persistence.ts: Fixed .tmp suffix replaced with randomized suffix using crypto.randomBytes(4). Prevents concurrent-write data loss when two callers write the same JSON file simultaneously (metrics ledger at risk during parallel slice execution). 2. undo.ts: Raw writeFileSync on PLAN.md replaced with atomicWriteSync. Prevents crash mid-write from corrupting PLAN.md permanently. 3. triage-resolution.ts: All 6 writeFileSync calls replaced with atomicWriteSync. Covers PLAN.md inject, REPLAN-TRIGGER.md, REGRESSION.md, and CONTEXT-DRAFT.md writes. --- src/resources/extensions/gsd/json-persistence.ts | 9 ++++++--- src/resources/extensions/gsd/triage-resolution.ts | 15 ++++++++------- src/resources/extensions/gsd/undo.ts | 5 +++-- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/resources/extensions/gsd/json-persistence.ts b/src/resources/extensions/gsd/json-persistence.ts index 8c6c2776c..6aeef5720 100644 --- a/src/resources/extensions/gsd/json-persistence.ts +++ b/src/resources/extensions/gsd/json-persistence.ts @@ -1,5 +1,6 @@ -import { existsSync, readFileSync, writeFileSync, mkdirSync, renameSync } from "node:fs"; +import { existsSync, readFileSync, writeFileSync, mkdirSync, renameSync, unlinkSync } from "node:fs"; import { dirname } from "node:path"; +import { randomBytes } from "node:crypto"; /** * Load a JSON file with validation, returning a default on failure. @@ -51,9 +52,11 @@ export function loadJsonFileOrNull( export function saveJsonFile(filePath: string, data: T): void { try { mkdirSync(dirname(filePath), { recursive: true }); - const tmp = filePath + ".tmp"; + // Use randomized tmp suffix to prevent concurrent-write data loss + const tmp = `${filePath}.tmp.${randomBytes(4).toString("hex")}`; writeFileSync(tmp, JSON.stringify(data, null, 2) + "\n", "utf-8"); renameSync(tmp, filePath); + // No cleanup needed — renameSync atomically removes tmp on success } catch { // Non-fatal — don't let persistence failures break operation } @@ -66,7 +69,7 @@ export function saveJsonFile(filePath: string, data: T): void { export function writeJsonFileAtomic(filePath: string, data: T): void { try { mkdirSync(dirname(filePath), { recursive: true }); - const tmp = filePath + ".tmp"; + const tmp = `${filePath}.tmp.${randomBytes(4).toString("hex")}`; writeFileSync(tmp, JSON.stringify(data, null, 2), "utf-8"); renameSync(tmp, filePath); } catch { diff --git a/src/resources/extensions/gsd/triage-resolution.ts b/src/resources/extensions/gsd/triage-resolution.ts index 2e5d68ae6..4befa1ad6 100644 --- a/src/resources/extensions/gsd/triage-resolution.ts +++ b/src/resources/extensions/gsd/triage-resolution.ts @@ -10,7 +10,8 @@ * Also provides detectFileOverlap() for surfacing downstream impact on quick tasks. */ -import { existsSync, mkdirSync, readFileSync, writeFileSync, unlinkSync } from "node:fs"; +import { existsSync, mkdirSync, readFileSync, unlinkSync } from "node:fs"; +import { atomicWriteSync } from "./atomic-write.js"; import { join } from "node:path"; import { createRequire } from "node:module"; import { gsdRoot, milestonesDir } from "./paths.js"; @@ -65,10 +66,10 @@ export function executeInject( const filesSection = content.indexOf("## Files Likely Touched"); if (filesSection !== -1) { const updated = content.slice(0, filesSection) + newTask + "\n\n" + content.slice(filesSection); - writeFileSync(planPath, updated, "utf-8"); + atomicWriteSync(planPath, updated, "utf-8"); } else { // No Files section — append at end - writeFileSync(planPath, content.trimEnd() + "\n\n" + newTask + "\n", "utf-8"); + atomicWriteSync(planPath, content.trimEnd() + "\n\n" + newTask + "\n", "utf-8"); } return newId; @@ -105,7 +106,7 @@ export function executeReplan( `will detect it and enter the replanning-slice phase.`, ].join("\n"); - writeFileSync(triggerPath, content, "utf-8"); + atomicWriteSync(triggerPath, content, "utf-8"); // Also write replan_triggered_at column for DB-backed detection try { @@ -183,7 +184,7 @@ export function executeBacktrack( `3. Resume auto-mode — the state machine will re-enter discussion for the target`, ].join("\n"); - writeFileSync(triggerPath, content, "utf-8"); + atomicWriteSync(triggerPath, content, "utf-8"); // If we have a valid target, also reset that milestone's completion status // so deriveState() will re-enter it as the active milestone. @@ -194,7 +195,7 @@ export function executeBacktrack( // Write a regression marker so the state machine knows this milestone // needs re-discussion, not just re-execution const regressionPath = join(targetDir, `${targetMilestoneId}-REGRESSION.md`); - writeFileSync(regressionPath, [ + atomicWriteSync(regressionPath, [ `# Milestone Regression`, ``, `**From:** ${currentMilestoneId}`, @@ -361,7 +362,7 @@ export function ensureDeferMilestoneDir( ``, ].join("\n"); - writeFileSync( + atomicWriteSync( join(msDir, `${targetMilestone}-CONTEXT-DRAFT.md`), draftContent, "utf-8", diff --git a/src/resources/extensions/gsd/undo.ts b/src/resources/extensions/gsd/undo.ts index 5d68c5e82..6443634c6 100644 --- a/src/resources/extensions/gsd/undo.ts +++ b/src/resources/extensions/gsd/undo.ts @@ -4,9 +4,10 @@ // handleResetSlice: Reset a slice and all its tasks, re-rendering plan + roadmap. import type { ExtensionCommandContext, ExtensionAPI } from "@gsd/pi-coding-agent"; -import { existsSync, readFileSync, writeFileSync, unlinkSync, readdirSync } from "node:fs"; +import { existsSync, readFileSync, unlinkSync, readdirSync } from "node:fs"; import { join, basename } from "node:path"; import { nativeRevertCommit, nativeRevertAbort } from "./native-git-bridge.js"; +import { atomicWriteSync } from "./atomic-write.js"; import { parseUnitId } from "./unit-id.js"; import { deriveState } from "./state.js"; import { invalidateAllCaches } from "./cache.js"; @@ -393,7 +394,7 @@ export function uncheckTaskInPlan(basePath: string, mid: string, sid: string, ti const regex = new RegExp(`^(\\s*-\\s*)\\[x\\](\\s*\\**${tid}\\**[:\\s])`, "mi"); if (regex.test(content)) { content = content.replace(regex, "$1[ ]$2"); - writeFileSync(planFile, content, "utf-8"); + atomicWriteSync(planFile, content); return true; } return false;