Centralize atomic artifact writes

internal/adapters/scriptorium/runner.go

@@ -3,12 +3,9 @@ package scriptorium
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"io"
-	"os"
 	"os/exec"
-	"path/filepath"
 	"time"
 )
 
@@ -207,37 +204,6 @@ func (r Runner) runArgs(req RunRequest) []string {
 	return args
 }
 
-func SaveRenderResult(path string, result *RenderResult) error {
-	if result == nil {
-		return fmt.Errorf("render result is required")
-	}
-	data, err := json.MarshalIndent(result, "", "  ")
-	if err != nil {
-		return fmt.Errorf("marshal render result: %w", err)
-	}
-	if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
-		return fmt.Errorf("create preflight directory %q: %w", filepath.Dir(path), err)
-	}
-	tmp, err := os.CreateTemp(filepath.Dir(path), "."+filepath.Base(path)+".*.tmp")
-	if err != nil {
-		return fmt.Errorf("create temporary preflight file: %w", err)
-	}
-	tmpName := tmp.Name()
-	defer os.Remove(tmpName)
-
-	if _, err := tmp.Write(data); err != nil {
-		tmp.Close()
-		return fmt.Errorf("write temporary preflight file: %w", err)
-	}
-	if err := tmp.Close(); err != nil {
-		return fmt.Errorf("close temporary preflight file: %w", err)
-	}
-	if err := os.Rename(tmpName, path); err != nil {
-		return fmt.Errorf("save preflight %q: %w", path, err)
-	}
-	return nil
-}
-
 type limitedBuffer struct {
 	data      []byte
 	limit     int

internal/adapters/weatherapi/client.go

@@ -11,14 +11,13 @@ import (
 	"io"
 	"net/http"
 	"net/url"
-	"os"
 	"path"
-	"path/filepath"
 	"strconv"
 	"strings"
 	"time"
 
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/config"
+	"gitea.maximumdirect.net/eric/weatherreporter/internal/fileutil"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/forecast"
 )
 
@@ -398,29 +397,8 @@ func sourceHash(raw json.RawMessage) (string, error) {
 }
 
 func SaveBundle(path string, bundle *forecast.Bundle) error {
-	data, err := json.MarshalIndent(bundle, "", "  ")
-	if err != nil {
-		return fmt.Errorf("marshal forecast bundle: %w", err)
-	}
-	if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
-		return fmt.Errorf("create bundle directory %q: %w", filepath.Dir(path), err)
-	}
-	tmp, err := os.CreateTemp(filepath.Dir(path), "."+filepath.Base(path)+".*.tmp")
-	if err != nil {
-		return fmt.Errorf("create temporary bundle file: %w", err)
-	}
-	tmpName := tmp.Name()
-	defer os.Remove(tmpName)
-
-	if _, err := tmp.Write(data); err != nil {
-		tmp.Close()
-		return fmt.Errorf("write temporary bundle file: %w", err)
-	}
-	if err := tmp.Close(); err != nil {
-		return fmt.Errorf("close temporary bundle file: %w", err)
-	}
-	if err := os.Rename(tmpName, path); err != nil {
-		return fmt.Errorf("save bundle %q: %w", path, err)
+	if err := fileutil.WriteJSONAtomic(path, bundle); err != nil {
+		return fmt.Errorf("save bundle: %w", err)
 	}
 	return nil
 }

internal/app/app.go

@@ -4,7 +4,6 @@ package app
 import (
 	"context"
 	"fmt"
-	"os"
 	"path/filepath"
 	"time"
 
@@ -13,6 +12,7 @@ import (
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/briefing"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/changes"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/config"
+	"gitea.maximumdirect.net/eric/weatherreporter/internal/fileutil"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/forecast"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/promptinput"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/report"
@@ -450,7 +450,7 @@ func GenerateReport(ctx context.Context, req ReportRequest) (*ReportResult, erro
 	preflightPath := paths.Preflight
 	if renderResult != nil {
 		var err error
-		preflightPath, err = store.SavePreflight(ctx, req.Resolved, renderResult)
+		preflightPath, err = store.SavePreflight(ctx, req.Resolved, preflightArtifact(renderResult))
 		if err != nil {
 			return nil, err
 		}
@@ -480,7 +480,7 @@ func GenerateReport(ctx context.Context, req ReportRequest) (*ReportResult, erro
 		OutputPath:      reportPath,
 	})
 	if runErr == nil && req.OutputPath != "" && req.OutputPath != reportPath {
-		if err := copyFileAtomic(reportPath, req.OutputPath); err != nil {
+		if err := fileutil.CopyFileAtomic(reportPath, req.OutputPath); err != nil {
 			return nil, err
 		}
 	}
@@ -608,29 +608,16 @@ func recentChanges(ctx context.Context, store state.Store, priorSnapshot *state.
 	}
 }
 
-func copyFileAtomic(source string, target string) error {
-	data, err := os.ReadFile(source)
-	if err != nil {
-		return fmt.Errorf("read rendered report %q: %w", source, err)
+func preflightArtifact(result *scriptorium.RenderResult) state.PreflightArtifact {
+	if result == nil {
+		return state.PreflightArtifact{}
 	}
-	if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil {
-		return fmt.Errorf("create report output directory %q: %w", filepath.Dir(target), err)
+	return state.PreflightArtifact{
+		Command:         append([]string(nil), result.Command...),
+		Stdout:          result.Stdout,
+		Stderr:          result.Stderr,
+		StdoutTruncated: result.StdoutTruncated,
+		StderrTruncated: result.StderrTruncated,
+		ExitCode:        result.ExitCode,
 	}
-	tmp, err := os.CreateTemp(filepath.Dir(target), "."+filepath.Base(target)+".*.tmp")
-	if err != nil {
-		return fmt.Errorf("create temporary report output file: %w", err)
-	}
-	tmpName := tmp.Name()
-	defer os.Remove(tmpName)
-	if _, err := tmp.Write(data); err != nil {
-		tmp.Close()
-		return fmt.Errorf("write temporary report output file: %w", err)
-	}
-	if err := tmp.Close(); err != nil {
-		return fmt.Errorf("close temporary report output file: %w", err)
-	}
-	if err := os.Rename(tmpName, target); err != nil {
-		return fmt.Errorf("save report output %q: %w", target, err)
-	}
-	return nil
 }

internal/briefing/package.go

@@ -2,12 +2,10 @@
 package briefing
 
 import (
-	"encoding/json"
 	"fmt"
-	"os"
-	"path/filepath"
 	"time"
 
+	"gitea.maximumdirect.net/eric/weatherreporter/internal/fileutil"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/forecast"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/report"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/timeutil"
@@ -78,29 +76,8 @@ func BuildMetadata(ctx BuildContext) Metadata {
 }
 
 func Save(path string, pkg Package) error {
-	data, err := json.MarshalIndent(pkg, "", "  ")
-	if err != nil {
-		return fmt.Errorf("marshal briefing package: %w", err)
-	}
-	if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
-		return fmt.Errorf("create briefing directory %q: %w", filepath.Dir(path), err)
-	}
-	tmp, err := os.CreateTemp(filepath.Dir(path), "."+filepath.Base(path)+".*.tmp")
-	if err != nil {
-		return fmt.Errorf("create temporary briefing file: %w", err)
-	}
-	tmpName := tmp.Name()
-	defer os.Remove(tmpName)
-
-	if _, err := tmp.Write(data); err != nil {
-		tmp.Close()
-		return fmt.Errorf("write temporary briefing file: %w", err)
-	}
-	if err := tmp.Close(); err != nil {
-		return fmt.Errorf("close temporary briefing file: %w", err)
-	}
-	if err := os.Rename(tmpName, path); err != nil {
-		return fmt.Errorf("save briefing %q: %w", path, err)
+	if err := fileutil.WriteJSONAtomic(path, pkg); err != nil {
+		return fmt.Errorf("save briefing package: %w", err)
 	}
 	return nil
 }

internal/fileutil/fileutil.go

@@ -0,0 +1,48 @@
+// Package fileutil provides narrow filesystem helpers for durable artifacts.
+package fileutil
+
+import (
+	"encoding/json"
+	"fmt"
+	"os"
+	"path/filepath"
+)
+
+func WriteFileAtomic(path string, data []byte) error {
+	if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
+		return fmt.Errorf("create directory %q: %w", filepath.Dir(path), err)
+	}
+	tmp, err := os.CreateTemp(filepath.Dir(path), "."+filepath.Base(path)+".*.tmp")
+	if err != nil {
+		return fmt.Errorf("create temporary file for %q: %w", path, err)
+	}
+	tmpName := tmp.Name()
+	defer os.Remove(tmpName)
+	if _, err := tmp.Write(data); err != nil {
+		tmp.Close()
+		return fmt.Errorf("write temporary file for %q: %w", path, err)
+	}
+	if err := tmp.Close(); err != nil {
+		return fmt.Errorf("close temporary file for %q: %w", path, err)
+	}
+	if err := os.Rename(tmpName, path); err != nil {
+		return fmt.Errorf("save %q: %w", path, err)
+	}
+	return nil
+}
+
+func WriteJSONAtomic(path string, value any) error {
+	data, err := json.MarshalIndent(value, "", "  ")
+	if err != nil {
+		return fmt.Errorf("marshal %q: %w", path, err)
+	}
+	return WriteFileAtomic(path, data)
+}
+
+func CopyFileAtomic(source string, target string) error {
+	data, err := os.ReadFile(source)
+	if err != nil {
+		return fmt.Errorf("read %q: %w", source, err)
+	}
+	return WriteFileAtomic(target, data)
+}

internal/fileutil/fileutil_test.go

@@ -0,0 +1,103 @@
+package fileutil
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+func TestWriteFileAtomicCreatesParentDirectory(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "nested", "artifact.txt")
+
+	if err := WriteFileAtomic(path, []byte("artifact")); err != nil {
+		t.Fatalf("WriteFileAtomic() error = %v", err)
+	}
+
+	data, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatalf("ReadFile() error = %v", err)
+	}
+	if string(data) != "artifact" {
+		t.Fatalf("data = %q, want artifact", data)
+	}
+}
+
+func TestWriteFileAtomicOverwritesTarget(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "artifact.txt")
+	if err := WriteFileAtomic(path, []byte("old")); err != nil {
+		t.Fatalf("WriteFileAtomic() initial error = %v", err)
+	}
+
+	if err := WriteFileAtomic(path, []byte("new")); err != nil {
+		t.Fatalf("WriteFileAtomic() overwrite error = %v", err)
+	}
+
+	data, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatalf("ReadFile() error = %v", err)
+	}
+	if string(data) != "new" {
+		t.Fatalf("data = %q, want new", data)
+	}
+}
+
+func TestWriteFileAtomicCleansTemporaryFileAfterRenameError(t *testing.T) {
+	dir := t.TempDir()
+	target := filepath.Join(dir, "target")
+	if err := os.Mkdir(target, 0o755); err != nil {
+		t.Fatalf("Mkdir() error = %v", err)
+	}
+
+	err := WriteFileAtomic(target, []byte("data"))
+	if err == nil {
+		t.Fatal("WriteFileAtomic() error = nil, want rename error")
+	}
+	if !strings.Contains(err.Error(), "save") {
+		t.Fatalf("error = %q, want save context", err.Error())
+	}
+	matches, err := filepath.Glob(filepath.Join(dir, ".target.*.tmp"))
+	if err != nil {
+		t.Fatalf("Glob() error = %v", err)
+	}
+	if len(matches) != 0 {
+		t.Fatalf("temporary files = %v, want none", matches)
+	}
+}
+
+func TestWriteJSONAtomic(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "artifact.json")
+
+	if err := WriteJSONAtomic(path, map[string]string{"status": "ok"}); err != nil {
+		t.Fatalf("WriteJSONAtomic() error = %v", err)
+	}
+
+	data, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatalf("ReadFile() error = %v", err)
+	}
+	if string(data) != "{\n  \"status\": \"ok\"\n}" {
+		t.Fatalf("json = %q, want indented object", data)
+	}
+}
+
+func TestCopyFileAtomic(t *testing.T) {
+	dir := t.TempDir()
+	source := filepath.Join(dir, "source.txt")
+	target := filepath.Join(dir, "nested", "target.txt")
+	if err := os.WriteFile(source, []byte("copied"), 0o600); err != nil {
+		t.Fatalf("WriteFile() error = %v", err)
+	}
+
+	if err := CopyFileAtomic(source, target); err != nil {
+		t.Fatalf("CopyFileAtomic() error = %v", err)
+	}
+
+	data, err := os.ReadFile(target)
+	if err != nil {
+		t.Fatalf("ReadFile() error = %v", err)
+	}
+	if string(data) != "copied" {
+		t.Fatalf("data = %q, want copied", data)
+	}
+}

internal/promptinput/package.go

@@ -2,14 +2,12 @@
 package promptinput
 
 import (
-	"encoding/json"
 	"fmt"
-	"os"
-	"path/filepath"
 	"time"
 
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/briefing"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/changes"
+	"gitea.maximumdirect.net/eric/weatherreporter/internal/fileutil"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/forecast"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/report"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/timeutil"
@@ -117,29 +115,8 @@ func Save(path string, pkg Package) error {
 	if err := Validate(pkg); err != nil {
 		return err
 	}
-	data, err := json.MarshalIndent(pkg, "", "  ")
-	if err != nil {
-		return fmt.Errorf("marshal data package: %w", err)
-	}
-	if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
-		return fmt.Errorf("create data package directory %q: %w", filepath.Dir(path), err)
-	}
-	tmp, err := os.CreateTemp(filepath.Dir(path), "."+filepath.Base(path)+".*.tmp")
-	if err != nil {
-		return fmt.Errorf("create temporary data package file: %w", err)
-	}
-	tmpName := tmp.Name()
-	defer os.Remove(tmpName)
-
-	if _, err := tmp.Write(data); err != nil {
-		tmp.Close()
-		return fmt.Errorf("write temporary data package file: %w", err)
-	}
-	if err := tmp.Close(); err != nil {
-		return fmt.Errorf("close temporary data package file: %w", err)
-	}
-	if err := os.Rename(tmpName, path); err != nil {
-		return fmt.Errorf("save data package %q: %w", path, err)
+	if err := fileutil.WriteJSONAtomic(path, pkg); err != nil {
+		return fmt.Errorf("save data package: %w", err)
 	}
 	return nil
 }

internal/state/filesystem.go

@@ -10,9 +10,9 @@ import (
 	"strings"
 	"time"
 
-	"gitea.maximumdirect.net/eric/weatherreporter/internal/adapters/scriptorium"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/briefing"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/config"
+	"gitea.maximumdirect.net/eric/weatherreporter/internal/fileutil"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/promptinput"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/report"
 )
@@ -99,7 +99,7 @@ func (s *FilesystemStore) SaveBriefing(_ context.Context, resolved report.Resolv
 	if err != nil {
 		return "", err
 	}
-	if err := writeJSONAtomic(paths.Briefing, pkg); err != nil {
+	if err := fileutil.WriteJSONAtomic(paths.Briefing, pkg); err != nil {
 		return "", err
 	}
 	return paths.Briefing, nil
@@ -113,21 +113,18 @@ func (s *FilesystemStore) SaveDataPackage(_ context.Context, resolved report.Res
 	if err := promptinput.Validate(pkg); err != nil {
 		return "", err
 	}
-	if err := writeJSONAtomic(paths.DataPackage, pkg); err != nil {
+	if err := fileutil.WriteJSONAtomic(paths.DataPackage, pkg); err != nil {
 		return "", err
 	}
 	return paths.DataPackage, nil
 }
 
-func (s *FilesystemStore) SavePreflight(_ context.Context, resolved report.Resolved, result *scriptorium.RenderResult) (string, error) {
-	if result == nil {
-		return "", fmt.Errorf("render result is required")
-	}
+func (s *FilesystemStore) SavePreflight(_ context.Context, resolved report.Resolved, artifact PreflightArtifact) (string, error) {
 	paths, err := s.Paths(resolved)
 	if err != nil {
 		return "", err
 	}
-	if err := writeJSONAtomic(paths.Preflight, result); err != nil {
+	if err := fileutil.WriteJSONAtomic(paths.Preflight, artifact); err != nil {
 		return "", err
 	}
 	return paths.Preflight, nil
@@ -161,7 +158,7 @@ func (s *FilesystemStore) SaveMetadata(_ context.Context, metadata Metadata) (st
 	if path == "" {
 		return "", fmt.Errorf("metadata path cannot be resolved")
 	}
-	if err := writeJSONAtomic(path, metadata); err != nil {
+	if err := fileutil.WriteJSONAtomic(path, metadata); err != nil {
 		return "", err
 	}
 	return path, nil
@@ -362,33 +359,6 @@ func validateRelativeDir(name string, value string) error {
 	return nil
 }
 
-func writeJSONAtomic(path string, value any) error {
-	data, err := json.MarshalIndent(value, "", "  ")
-	if err != nil {
-		return fmt.Errorf("marshal %q: %w", path, err)
-	}
-	if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
-		return fmt.Errorf("create directory %q: %w", filepath.Dir(path), err)
-	}
-	tmp, err := os.CreateTemp(filepath.Dir(path), "."+filepath.Base(path)+".*.tmp")
-	if err != nil {
-		return fmt.Errorf("create temporary file for %q: %w", path, err)
-	}
-	tmpName := tmp.Name()
-	defer os.Remove(tmpName)
-	if _, err := tmp.Write(data); err != nil {
-		tmp.Close()
-		return fmt.Errorf("write temporary file for %q: %w", path, err)
-	}
-	if err := tmp.Close(); err != nil {
-		return fmt.Errorf("close temporary file for %q: %w", path, err)
-	}
-	if err := os.Rename(tmpName, path); err != nil {
-		return fmt.Errorf("save %q: %w", path, err)
-	}
-	return nil
-}
-
 func readJSON(path string, target any) error {
 	data, err := os.ReadFile(path)
 	if err != nil {

internal/state/filesystem_test.go

@@ -9,7 +9,6 @@ import (
 	"testing"
 	"time"
 
-	"gitea.maximumdirect.net/eric/weatherreporter/internal/adapters/scriptorium"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/briefing"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/config"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/promptinput"
@@ -56,7 +55,7 @@ func TestSaveArtifactsAndMetadataRoundTrip(t *testing.T) {
 	if err != nil {
 		t.Fatalf("SaveDataPackage() error = %v", err)
 	}
-	preflightPath, err := store.SavePreflight(context.Background(), resolved, &scriptorium.RenderResult{Stdout: `{"ok":true}`})
+	preflightPath, err := store.SavePreflight(context.Background(), resolved, PreflightArtifact{Stdout: `{"ok":true}`})
 	if err != nil {
 		t.Fatalf("SavePreflight() error = %v", err)
 	}
@@ -67,6 +66,17 @@ func TestSaveArtifactsAndMetadataRoundTrip(t *testing.T) {
 	if err := os.WriteFile(renderedReportPath, []byte("# Daily Report\n"), 0o600); err != nil {
 		t.Fatalf("write rendered report: %v", err)
 	}
+	var preflight PreflightArtifact
+	preflightData, err := os.ReadFile(preflightPath)
+	if err != nil {
+		t.Fatalf("read preflight: %v", err)
+	}
+	if err := json.Unmarshal(preflightData, &preflight); err != nil {
+		t.Fatalf("decode preflight: %v", err)
+	}
+	if preflight.Stdout != `{"ok":true}` {
+		t.Fatalf("preflight stdout = %q, want render stdout", preflight.Stdout)
+	}
 	paths, err := store.Paths(resolved)
 	if err != nil {
 		t.Fatalf("Paths() error = %v", err)

internal/state/store.go

@@ -4,7 +4,6 @@ package state
 import (
 	"context"
 
-	"gitea.maximumdirect.net/eric/weatherreporter/internal/adapters/scriptorium"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/briefing"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/promptinput"
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/report"
@@ -14,7 +13,7 @@ type Store interface {
 	Paths(report.Resolved) (ArtifactPaths, error)
 	SaveBriefing(context.Context, report.Resolved, briefing.Package) (string, error)
 	SaveDataPackage(context.Context, report.Resolved, promptinput.Package) (string, error)
-	SavePreflight(context.Context, report.Resolved, *scriptorium.RenderResult) (string, error)
+	SavePreflight(context.Context, report.Resolved, PreflightArtifact) (string, error)
 	PrepareRenderedReport(context.Context, report.Resolved) (string, error)
 	SaveMetadata(context.Context, Metadata) (string, error)
 	FindPriorSnapshot(context.Context, report.Resolved) (*PriorSnapshot, error)
@@ -26,3 +25,12 @@ type PriorSnapshot struct {
 	Metadata     Metadata
 	BriefingPath string
 }
+
+type PreflightArtifact struct {
+	Command         []string `json:"command"`
+	Stdout          string   `json:"stdout"`
+	Stderr          string   `json:"stderr"`
+	StdoutTruncated bool     `json:"stdoutTruncated,omitempty"`
+	StderrTruncated bool     `json:"stderrTruncated,omitempty"`
+	ExitCode        int      `json:"exitCode"`
+}