refactor: consolidate atomic-write primitives onto fsutil (iss-32)

.abcd/work/issues/open/iss-79-storeoriginal-inline-atomic-write.md

@@ -0,0 +1,11 @@
+---
+schema_version: 1
+id: "iss-79"
+slug: "storeoriginal-inline-atomic-write"
+severity: "minor"
+category: "tech-debt"
+source: "agent-finding"
+found_during: "2026-07-12 /abcd:run iss-32 review"
+---
+
+fifth divergent atomic write: internal/core/memory/ingest.go storeOriginal (~:806) is an inline temp(.memtmp,O_EXCL)+fsync+rename durable write that iss-32's consolidation left untouched — it is inline, not a named func, so TestNoNonCanonicalAtomicWritePrimitives (name-based) cannot catch it, and it was outside iss-32's named 4-copy corpus. It differs from canonical fsutil.WriteFileAtomic: no parent-dir fsync, no explicit chmod. Consolidate it onto fsutil.WriteFileAtomic(target, material.rawBytes, 0644) (keeping the pre-existing sources-dir symlink guard), OR extend the canonical-primitive detector to catch inline temp+rename sequences. Safe today (has its own symlink guard); this is a completeness/consistency follow-up.

.abcd/work/issues/open/iss-32-atomic-write-consolidation.md → .abcd/work/issues/resolved/iss-32-atomic-write-consolidation.md

@@ -7,6 +7,7 @@ category: "tech-debt"
 source: "agent-finding"
 found_during: "2026-07-08 multi-agent review"
 found_at: "internal/fsutil/fsutil.go"
+resolution: "Consolidated the four named atomic-write/real-dir copies onto internal/fsutil (one-canonical-primitive), armed behind TestNoNonCanonicalAtomicWritePrimitives + a new fsutil crash-safety suite (fsutil had zero tests). Deleted ahoy writeFileAtomic+isRealDir, capture writeFileAtomic; memory durableWrite -> thin writeStringAtomic adapter over fsutil.WriteFileAtomic. Added fsutil.WriteFileAtomicPreserveMode. ruthless PROMOTE, security PASS (symlink-refusal preserved, O_EXCL->CreateTemp stronger, no mode downgrade). A 5th inline copy (memory/ingest.go storeOriginal), outside this issue's named corpus, is filed as iss-79."
 ---
 
 atomic-write consolidation: four independent temp-file+rename implementations with divergent durability — internal/fsutil.WriteFileAtomic (file fsync + parent-dir fsync), ahoy marker.go writeFileAtomic (no parent fsync, mode-preserving), capture roots.go writeFileAtomic (no parent fsync despite a doc comment claiming durability), memory writer.go durableWrite (parent fsync, O_EXCL naming); isRealDir is likewise duplicated (ahoy store.go vs fsutil). internal/fsutil itself has zero tests. Detector (per one-canonical-primitive): a lint flagging private redefinitions of the canonical names (writeFileAtomic, isRealDir) outside internal/fsutil, plus a fsutil crash-safety test suite; consolidation then drains behind the armed lint. Acceptance corpus: the three non-canonical copies and the duplicate isRealDir.

CHANGELOG.md

@@ -129,6 +129,15 @@ called out in a **Breaking** section.
   named intent EXISTS and points back at this spec (bidirectional agreement).
 - The issue ledger moved from `.abcd/development/activity/issues` to
   `.abcd/work/issues` (the committed shared-working tier).
+- The atomic-write and real-directory primitives are consolidated onto
+  `internal/fsutil` (iss-32): the ahoy, capture, and memory store writers no
+  longer keep their own divergent temp-file+rename copies. Two observable
+  effects of routing through the canonical primitive: the ahoy and capture
+  writers now fsync the parent directory after the rename (a crash-durability
+  strengthening they previously lacked), and memory pages are written at a
+  fixed `0644` (an explicit chmod, where the old writer left the mode subject to
+  the process umask). A `TestNoNonCanonicalAtomicWritePrimitives` guard keeps a
+  fifth copy from reappearing.
 
 ### Removed
 

internal/core/ahoy/apply.go

@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+
+	"github.com/REPPL/abcd-cli/internal/fsutil"
 	"sort"
 	"strings"
 	"time"
@@ -281,7 +283,7 @@ func (a *applyCtx) stepHistory() {
 	}
 	repoDir := filepath.Join(root, sha)
 	transcripts := filepath.Join(repoDir, "transcripts")
-	if a.approved[SafeAutocreate] && !isRealDir(transcripts) {
+	if a.approved[SafeAutocreate] && !fsutil.IsRealDir(transcripts) {
 		if err := os.MkdirAll(transcripts, 0o755); err == nil {
 			a.note(transcripts)
 		}

internal/core/ahoy/detect.go

@@ -4,6 +4,8 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+
+	"github.com/REPPL/abcd-cli/internal/fsutil"
 	"sort"
 
 	"github.com/REPPL/abcd-cli/internal/core/identity"
@@ -83,7 +85,7 @@ func classify(cwd string, id RepoIdentity, idx *historyIndex) (FolderKind, map[s
 	registered := indexHasRoot(idx, id.RootSHA)
 	signals["index_registered"] = registered
 
-	abcdDir := isRealDir(filepath.Join(cwd, ".abcd"))
+	abcdDir := fsutil.IsRealDir(filepath.Join(cwd, ".abcd"))
 	signals["abcd_dir"] = abcdDir
 
 	markerFired := false
@@ -259,7 +261,7 @@ func detectHistoryStore(rootSHA string) []Gap {
 		return gaps
 	}
 	repoDir := filepath.Join(root, rootSHA)
-	if !isRealDir(filepath.Join(repoDir, "transcripts")) {
+	if !fsutil.IsRealDir(filepath.Join(repoDir, "transcripts")) {
 		gaps = append(gaps, Gap{
 			ID: "history.transcripts_missing", Category: SafeAutocreate, Scope: "repo",
 			Title:   "history transcripts/ dir missing",

internal/core/ahoy/gitignore.go

@@ -4,6 +4,8 @@ import (
 	"bytes"
 	"os"
 	"path/filepath"
+
+	"github.com/REPPL/abcd-cli/internal/fsutil"
 	"strings"
 )
 
@@ -170,7 +172,7 @@ func applyVisibilityBlock(cwd, visibility string) (bool, error) {
 	if absent {
 		eol := "\n"
 		body := strings.Join(canonicalGitignoreBlock(visibility), eol) + eol
-		if err := writeFileAtomic(path, []byte(body)); err != nil {
+		if err := fsutil.WriteFileAtomicPreserveMode(path, []byte(body)); err != nil {
 			return false, err
 		}
 		return true, nil
@@ -195,7 +197,7 @@ func applyVisibilityBlock(cwd, visibility string) (bool, error) {
 	if bytes.Equal(newBytes, raw) {
 		return false, nil // byte-identical — preserve mtime
 	}
-	if err := writeFileAtomic(path, newBytes); err != nil {
+	if err := fsutil.WriteFileAtomicPreserveMode(path, newBytes); err != nil {
 		return false, err
 	}
 	return true, nil

internal/core/ahoy/marker.go

@@ -4,7 +4,8 @@ import (
 	"bytes"
 	_ "embed"
 	"os"
-	"path/filepath"
+
+	"github.com/REPPL/abcd-cli/internal/fsutil"
 	"regexp"
 )
 
@@ -108,7 +109,7 @@ func installMarkerFile(targetPath string) (wrote bool, ok bool) {
 
 	if absent {
 		body := append(append([]byte{}, synth...), eol...)
-		if err := writeFileAtomic(targetPath, body); err != nil {
+		if err := fsutil.WriteFileAtomicPreserveMode(targetPath, body); err != nil {
 			return false, false
 		}
 		return true, true
@@ -117,7 +118,7 @@ func installMarkerFile(targetPath string) (wrote bool, ok bool) {
 	matches := markerBlockRe.FindAllIndex(existing, -1)
 	if len(matches) == 0 {
 		body := composeMarkerInsertion(existing, synth, eol)
-		if err := writeFileAtomic(targetPath, body); err != nil {
+		if err := fsutil.WriteFileAtomicPreserveMode(targetPath, body); err != nil {
 			return false, false
 		}
 		return true, true
@@ -127,7 +128,7 @@ func installMarkerFile(targetPath string) (wrote bool, ok bool) {
 		return false, true // current — no write, mtime preserved
 	}
 	body := composeMarkerReplacement(existing, matches, synth)
-	if err := writeFileAtomic(targetPath, body); err != nil {
+	if err := fsutil.WriteFileAtomicPreserveMode(targetPath, body); err != nil {
 		return false, false
 	}
 	return true, true
@@ -201,7 +202,7 @@ func removeMarkerFile(targetPath string) (wrote bool, ok bool) {
 		return false, true // no block — untouched
 	}
 	body := composeMarkerRemoval(existing, matches)
-	if err := writeFileAtomic(targetPath, body); err != nil {
+	if err := fsutil.WriteFileAtomicPreserveMode(targetPath, body); err != nil {
 		return false, false
 	}
 	return true, true
@@ -296,44 +297,3 @@ func concat(parts ...[]byte) []byte {
 	}
 	return out
 }
-
-// writeFileAtomic writes data to path via a temp file + rename, preserving the
-// existing file's mode when present. Parent dirs are created as needed.
-func writeFileAtomic(path string, data []byte) error {
-	dir := filepath.Dir(path)
-	if err := os.MkdirAll(dir, 0o755); err != nil {
-		return err
-	}
-	mode := os.FileMode(0o644)
-	if fi, err := os.Stat(path); err == nil {
-		mode = fi.Mode().Perm()
-	}
-	tmp, err := os.CreateTemp(dir, ".abcd-tmp-*")
-	if err != nil {
-		return err
-	}
-	tmpName := tmp.Name()
-	if _, err := tmp.Write(data); err != nil {
-		tmp.Close()
-		os.Remove(tmpName)
-		return err
-	}
-	if err := tmp.Sync(); err != nil {
-		tmp.Close()
-		os.Remove(tmpName)
-		return err
-	}
-	if err := tmp.Close(); err != nil {
-		os.Remove(tmpName)
-		return err
-	}
-	if err := os.Chmod(tmpName, mode); err != nil {
-		os.Remove(tmpName)
-		return err
-	}
-	if err := os.Rename(tmpName, path); err != nil {
-		os.Remove(tmpName)
-		return err
-	}
-	return nil
-}

internal/core/ahoy/store.go

@@ -5,6 +5,8 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+
+	"github.com/REPPL/abcd-cli/internal/fsutil"
 	"strings"
 
 	"github.com/REPPL/abcd-cli/internal/core"
@@ -116,11 +118,6 @@ func isDir(p string) bool {
 	return err == nil && fi.IsDir()
 }
 
-func isRealDir(p string) bool {
-	fi, err := os.Lstat(p)
-	return err == nil && fi.IsDir() && fi.Mode()&os.ModeSymlink == 0
-}
-
 // ---------------------------------------------------------------------------
 // ~/.abcd/history store
 // ---------------------------------------------------------------------------
@@ -293,7 +290,7 @@ func writeJSON(path string, v any) error {
 		return err
 	}
 	data = append(data, '\n')
-	return writeFileAtomic(path, data)
+	return fsutil.WriteFileAtomicPreserveMode(path, data)
 }
 
 // writeConfig persists a config map deterministically.

internal/core/capture/roots.go

@@ -113,41 +113,3 @@ func readWithChecksum(path string) (string, string, error) {
 	}
 	return string(data), sha256Hex(data), nil
 }
-
-// writeFileAtomic writes data to path via a temp file + fsync + rename,
-// preserving the existing file mode (default 0644).
-func writeFileAtomic(path string, data []byte) error {
-	dir := filepath.Dir(path)
-	mode := os.FileMode(0o644)
-	if fi, err := os.Stat(path); err == nil {
-		mode = fi.Mode().Perm()
-	}
-	tmp, err := os.CreateTemp(dir, ".abcd-iss-tmp-*")
-	if err != nil {
-		return err
-	}
-	tmpName := tmp.Name()
-	if _, err := tmp.Write(data); err != nil {
-		tmp.Close()
-		os.Remove(tmpName)
-		return err
-	}
-	if err := tmp.Sync(); err != nil {
-		tmp.Close()
-		os.Remove(tmpName)
-		return err
-	}
-	if err := tmp.Close(); err != nil {
-		os.Remove(tmpName)
-		return err
-	}
-	if err := os.Chmod(tmpName, mode); err != nil {
-		os.Remove(tmpName)
-		return err
-	}
-	if err := os.Rename(tmpName, path); err != nil {
-		os.Remove(tmpName)
-		return err
-	}
-	return nil
-}

internal/core/capture/workflow.go

@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+
+	"github.com/REPPL/abcd-cli/internal/fsutil"
 	"sort"
 	"strconv"
 	"strings"
@@ -110,7 +112,7 @@ func commitCapture(req CaptureRequest, issID, slug, placeholder string) (Capture
 	if checksum != emptyChecksum {
 		return CaptureResult{}, fmt.Errorf("%w: placeholder %s changed since reservation", ErrChecksumMismatch, placeholder)
 	}
-	if err := writeFileAtomic(placeholder, []byte(content)); err != nil {
+	if err := fsutil.WriteFileAtomicPreserveMode(placeholder, []byte(content)); err != nil {
 		return CaptureResult{}, err
 	}
 	return CaptureResult{ID: issID, Slug: slug, Path: placeholder, Status: StateOpen}, nil
@@ -204,7 +206,7 @@ func commitTransition(src, dst, newContent, expected string) error {
 	if current != expected {
 		return fmt.Errorf("%w: %s changed since it was read", ErrChecksumMismatch, src)
 	}
-	if err := writeFileAtomic(dst, []byte(newContent)); err != nil {
+	if err := fsutil.WriteFileAtomicPreserveMode(dst, []byte(newContent)); err != nil {
 		return err
 	}
 	if err := os.Remove(src); err != nil && !os.IsNotExist(err) {

internal/core/memory/coverage.go

@@ -561,7 +561,7 @@ func writeCoverageIndex(path string, result coverageResult, budget quotationBudg
 	if err := os.MkdirAll(dirOf(path), 0o755); err != nil {
 		return nil, err
 	}
-	if err := durableWrite(path, marshalIndentNoEscape(payload)); err != nil {
+	if err := writeStringAtomic(path, marshalIndentNoEscape(payload)); err != nil {
 		return nil, err
 	}
 	return payload, nil

internal/core/memory/lint.go

@@ -464,10 +464,10 @@ func Lint(req LintRequest) (LintResult, error) {
 		"generated_at":   generatedAt,
 		"store_path":     mem,
 	}
-	if err := durableWrite(filepath.Join(reportDir, "report.json"), marshalIndentNoEscape(reportFields)); err != nil {
+	if err := writeStringAtomic(filepath.Join(reportDir, "report.json"), marshalIndentNoEscape(reportFields)); err != nil {
 		return LintResult{}, err
 	}
-	if err := durableWrite(filepath.Join(reportDir, "report.md"), renderLintReportMD(reportFields)); err != nil {
+	if err := writeStringAtomic(filepath.Join(reportDir, "report.md"), renderLintReportMD(reportFields)); err != nil {
 		return LintResult{}, err
 	}