fix(codegen): write generated files atomically to survive interrupted generation

api/generate.go

@@ -2,10 +2,12 @@ package api
 
 import (
 	"fmt"
+	"go/parser"
+	"go/token"
+	"os"
 	"path/filepath"
 	"regexp"
 	"strings"
-	"syscall"
 
 	"golang.org/x/tools/imports"
 
@@ -27,6 +29,32 @@ var (
 	) // regex to grab the version number from a url
 )
 
+// maskGeneratedOutput hides an existing generated output file from the type
+// loader for the duration of this generation run (see the comment at the top
+// of generate). The mask is a package-clause-only stub — NOT empty bytes,
+// which would be a Go parse error and break loading the rest of the package —
+// so the package name is read from the real file's own package clause. A
+// missing/unparseable file needs no mask (there is nothing stale to bind to;
+// generation will (re)create it), matching how the old unlink was a no-op on
+// a missing file.
+func maskGeneratedOutput(cfg *config.Config, filename string) {
+	if filename == "" {
+		return
+	}
+	abs, err := filepath.Abs(filename)
+	if err != nil {
+		return
+	}
+	if _, err := os.Stat(abs); err != nil {
+		return // nothing on disk — nothing stale to mask
+	}
+	f, err := parser.ParseFile(token.NewFileSet(), abs, nil, parser.PackageClauseOnly)
+	if err != nil || f.Name == nil {
+		return // can't determine the package — leave it visible rather than corrupt the load
+	}
+	cfg.MaskGeneratedFile(abs, "package "+f.Name.Name+"\n")
+}
+
 // Generate generates GraphQL code based on the provided config.
 func Generate(cfg *config.Config, option ...Option) error {
 	return generate(cfg, nil, option...)
@@ -55,9 +83,22 @@ func generate(
 	incrementalOpts *codegen.IncrementalOptions,
 	option ...Option,
 ) error {
-	_ = syscall.Unlink(cfg.Exec.Filename)
+	// MASK gqlgen's own previous outputs from the type loader, WITHOUT deleting
+	// them from disk. If a stale generated model file is visible while the
+	// schema loads, autobind finds the previously-generated types in it and
+	// binds them as if they were user-written models — so modelgen skips
+	// (re)generating them, the freshly-written model file comes out (near-)empty,
+	// and the exec build then fails with "unable to find type" (every testserver
+	// config that autobinds its own model package hits this). Before this
+	// change, api.Generate handled that by syscall.Unlink-ing the outputs up
+	// front — but a deleted-then-interrupted generation left the user with NO
+	// generated file at all (#2345, #3505). An overlay gives the loader the
+	// same "these files don't exist yet" view with no destructive disk write:
+	// the real files stay intact until the atomic rename replaces them, and
+	// templates.write unmasks each file once its new contents are on disk.
+	maskGeneratedOutput(cfg, cfg.Exec.Filename)
 	if cfg.Model.IsDefined() {
-		_ = syscall.Unlink(cfg.Model.Filename)
+		maskGeneratedOutput(cfg, cfg.Model.Filename)
 	}
 
 	plugins := []plugin.Plugin{}

api/generate_test.go

@@ -413,3 +413,50 @@ func TestBuildPattern(t *testing.T) {
 		})
 	}
 }
+
+// TestGenerateAtomicWritePreservesOutputOnFailure reproduces issue #2345/#3505: when generation
+// FAILS mid-run (after the output file would have been deleted, before it's rewritten), the
+// PRE-EXISTING generated file must SURVIVE intact — not be left absent. Before the atomic-write
+// fix, api/generate.go unlinked the exec + model outputs at the very start of Generate, so a
+// failure anywhere in the long schema-load/plugin/render chain left the file ABSENT (invisible
+// to `go build` until the next regen, and the root of the recovery-desyncs-sibling-golden class).
+// Now the write is atomic (write-to-temp + os.Rename), and the upfront unlink is gone, so a
+// mid-generation failure leaves the prior file untouched.
+func TestGenerateAtomicWritePreservesOutputOnFailure(t *testing.T) {
+	wd, err := os.Getwd()
+	require.NoError(t, err)
+
+	workDir := filepath.Join(wd, "testdata", "default")
+	t.Cleanup(func() {
+		cleanup(workDir)
+		t.Chdir(wd)
+	})
+	t.Chdir(workDir)
+
+	cfg, err := config.LoadConfigFromDefaultLocations()
+	require.NoError(t, err)
+
+	// Pre-create the exec output with known content — the "previously generated" file that must
+	// survive a failed regen.
+	execPath := cfg.Exec.Filename
+	preExisting := []byte(
+		"// the previously-generated file — must survive a failed regen\npackage graph\n",
+	)
+	require.NoError(t, os.MkdirAll(filepath.Dir(execPath), 0o755))
+	require.NoError(t, os.WriteFile(execPath, preExisting, 0o644))
+
+	// Run Generate with an erroring schema mutator: it fails AFTER schema load but BEFORE the
+	// exec file is rewritten — the exact window where the old upfront-unlink left the file absent.
+	err = Generate(cfg, AddPlugin(&testSchemaMutator{name: "error-mutator", shouldError: true}))
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "deliberate schema mutation error")
+
+	// The pre-existing file must be INTACT — not absent, not truncated, not empty.
+	got, readErr := os.ReadFile(execPath)
+	require.NoError(
+		t,
+		readErr,
+		"exec output must be PRESENT after a failed regen (was deleted under the old unlink-first behavior)",
+	)
+	require.Equal(t, preExisting, got, "exec output must be UNCHANGED after a failed regen")
+}

codegen/config/config.go

@@ -78,6 +78,13 @@ type Config struct {
 	Sources          []*ast.Source  `yaml:"-"`
 	Packages         *code.Packages `yaml:"-"`
 	Schema           *ast.Schema    `yaml:"-"`
+
+	// packagesOverlay is the loader overlay shared by every Packages instance
+	// this Config creates (LoadSchema recreates c.Packages, so the overlay must
+	// outlive any single instance). Entries mask gqlgen's own stale generated
+	// outputs from autobind during a generation run — see MaskGeneratedFile and
+	// api.Generate. Keyed by absolute file path.
+	packagesOverlay map[string][]byte
 }
 
 // boolOrFalse returns the value of a *bool pointer, or false if nil.
@@ -337,12 +344,29 @@ func CompleteConfig(config *Config) error {
 	return nil
 }
 
+// MaskGeneratedFile registers a loader overlay masking absPath with the given
+// stub contents for every Packages instance this Config creates — including
+// the one LoadSchema recreates — so gqlgen's own stale outputs can be hidden
+// from autobind for the whole generation run (see api.Generate). Held on the
+// Config (not just the current Packages) because LoadSchema rebuilds
+// c.Packages, which would otherwise silently drop masks set before it.
+func (c *Config) MaskGeneratedFile(absPath, contents string) {
+	if c.packagesOverlay == nil {
+		c.packagesOverlay = map[string][]byte{}
+	}
+	c.packagesOverlay[absPath] = []byte(contents)
+	if c.Packages != nil {
+		c.Packages.MaskFile(absPath, contents)
+	}
+}
+
 func (c *Config) Init() error {
 	if c.Packages == nil {
 		c.Packages = code.NewPackages(
 			code.WithBuildTags(c.GoBuildTags...),
 			code.PackagePrefixToCache("github.com/99designs/gqlgen/graphql"),
 			code.WithPreloadNames(templatePackageNames...),
+			code.WithOverlay(c.packagesOverlay),
 		)
 	}
 
@@ -1168,6 +1192,7 @@ func (c *Config) LoadSchema() error {
 			code.WithBuildTags(c.GoBuildTags...),
 			code.PackagePrefixToCache("github.com/99designs/gqlgen/graphql"),
 			code.WithPreloadNames(templatePackageNames...),
+			code.WithOverlay(c.packagesOverlay),
 		)
 	}
 

codegen/templates/templates.go

@@ -18,6 +18,7 @@ import (
 	"strings"
 	"sync"
 	"text/template"
+	"time"
 	"unicode"
 
 	"github.com/99designs/gqlgen/internal/code"
@@ -736,10 +737,119 @@ func write(filename string, b []byte, packages *code.Packages, opts imports.Prun
 	// Skip write if content is unchanged - preserves mtime for Go build cache
 	existing, readErr := os.ReadFile(filename)
 	if readErr == nil && bytes.Equal(existing, formatted) {
+		// The on-disk file IS the current output — later loads must see it.
+		unmask(packages, filename)
 		return nil
 	}
 
-	return os.WriteFile(filename, formatted, 0o644)
+	// Write atomically: render to a temp file in the SAME directory as the
+	// destination, fsync it, then rename it into place on success. Rename is
+	// atomic on the same filesystem, so a reader (or a build) never observes a
+	// half-written file, and an interruption/panic/OOM between formatting and the
+	// rename leaves the PRE-EXISTING file intact instead of an empty or absent
+	// one. This also removes the need for the upfront syscall.Unlink of the output
+	// in api/generate.go (the unlink deleted the file before generation began, so
+	// any interruption during generation left it absent — see issue #2345/#3505).
+	//
+	// This is the standard write-temp-then-rename pattern used by
+	// google/renameio, natefinch/atomic, tailscale/atomicfile and moby/sys (see
+	// https://github.com/99designs/gqlgen/pull/4262#issuecomment-5011193760 for
+	// the tailscale/atomicfile reference this follows most closely):
+	//   - the temp lives next to the destination so the rename is same-filesystem;
+	//   - the temp is fsync'd before the rename so a crash can't expose a
+	//     zero-length file even though the rename itself succeeded (without fsync
+	//     the directory entry can be durable before the data is, per ext4's Ts'o);
+	//   - on non-Windows, the temp's permissions are set to match the existing
+	//     destination file (preserving any user/repo mode and avoiding spurious
+	//     mode churn across regens), defaulting to 0o644 for a brand-new file —
+	//     the same mode os.WriteFile used here before this change. On Windows the
+	//     permission bits are SKIPPED entirely (not attempted, not defaulted) —
+	//     matching tailscale/atomicfile's own "perm argument is ignored on
+	//     Windows" contract: Windows has no POSIX owner/group/other mode, only a
+	//     coarse read-only attribute, so os.FileMode there doesn't mean the same
+	//     thing and forcing a POSIX bit pattern onto it is a category error, not
+	//     a portability nicety.
+	dir := filepath.Dir(filename)
+	tmp, err := os.CreateTemp(dir, filepath.Base(filename)+".*.tmp")
+	if err != nil {
+		return fmt.Errorf("failed to create temp file: %w", err)
+	}
+	tmpName := tmp.Name()
+	cleanup := func() { _ = os.Remove(tmpName) }
+	if _, err := tmp.Write(formatted); err != nil {
+		_ = tmp.Close()
+		cleanup()
+		return fmt.Errorf("failed to write temp file: %w", err)
+	}
+	if runtime.GOOS != "windows" {
+		perm := os.FileMode(0o644)
+		if fi, err := os.Stat(filename); err == nil && fi.Mode().IsRegular() {
+			perm = fi.Mode().Perm()
+		}
+		// Set permissions before fsync so the mode change is flushed with the
+		// data. Chmod sets the exact bits (umask is not applied), like the
+		// references.
+		if err := tmp.Chmod(perm); err != nil {
+			_ = tmp.Close()
+			cleanup()
+			return fmt.Errorf("failed to set temp file permissions: %w", err)
+		}
+	}
+	if err := tmp.Sync(); err != nil {
+		_ = tmp.Close()
+		cleanup()
+		return fmt.Errorf("failed to sync temp file: %w", err)
+	}
+	if err := tmp.Close(); err != nil {
+		cleanup()
+		return fmt.Errorf("failed to close temp file: %w", err)
+	}
+	if err := renameWithRetry(tmpName, filename); err != nil {
+		cleanup()
+		return fmt.Errorf("failed to rename temp file: %w", err)
+	}
+	// The new contents are on disk — remove any loader mask api.Generate placed
+	// over this output (see maskGeneratedOutput), so later loads in this same
+	// run (e.g. the exec build reloading the model package after modelgen wrote
+	// it) see the just-generated types instead of the empty stub.
+	unmask(packages, filename)
+	return nil
+}
+
+// unmask lifts api.Generate's loader mask (maskGeneratedOutput) for a just-
+// written (or confirmed-current) output file — from here on, disk is truth.
+func unmask(packages *code.Packages, filename string) {
+	if abs, err := filepath.Abs(filename); err == nil {
+		packages.UnmaskFile(abs)
+	}
+}
+
+// renameWithRetry wraps os.Rename with a few short retries on Windows. Go's
+// os.Rename on Windows already calls MoveFileEx with MOVEFILE_REPLACE_EXISTING
+// — the same underlying API natefinch/atomic's ReplaceFile wraps — so it is
+// not a weaker primitive; what it doesn't do is retry. MoveFileEx can fail
+// TRANSIENTLY with "Access is denied" when something else (a virus scanner, a
+// search indexer) briefly holds an open handle on the destination right after
+// it was read (the unchanged-content check above just opened it) or on the
+// freshly-written temp file. This is a well-known Windows quirk; a handful of
+// short, bounded retries resolves it without materially slowing down the
+// common (non-Windows, non-contended) case, where the first attempt always
+// succeeds.
+func renameWithRetry(oldpath, newpath string) error {
+	if runtime.GOOS != "windows" {
+		return os.Rename(oldpath, newpath)
+	}
+	var err error
+	for i := range 5 {
+		if err = os.Rename(oldpath, newpath); err == nil {
+			return nil
+		}
+		if !errors.Is(err, os.ErrPermission) {
+			return err
+		}
+		time.Sleep(time.Duration(i+1) * 10 * time.Millisecond)
+	}
+	return err
 }
 
 var pkgReplacer = strings.NewReplacer(