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

[webel-crew]

api.Generate unlinks the exec + model output files (syscall.Unlink on cfg.Exec.Filename / cfg.Model.Filename) at the very start of generation, but the rewrite only happens much later in templates.Render → os.WriteFile. If generation is interrupted anywhere in that window — a build/compile failure, a timeout, an OOM on a large generated.go, a killed process — the output file is left deleted rather than merely stale. That's invisible to go build until the next successful regen, and reported in #2345 (and as a symptom in #3505).

This makes the write atomic and drops the upfront unlink: templates renders to a temp file in the same directory and os.Renames it into place on success, so an interruption leaves the previous file intact instead of absent, and a reader never sees a half-written file. Removing the syscall.Unlink is safe because os.Rename already replaces the file atomically (and os.WriteFile truncates) — the pre-delete was redundant. Successful runs are unchanged, including the unchanged-content short-circuit.

Added TestGenerateAtomicWritePreservesOutputOnFailure (fails on master, passes here); the existing api and codegen/templates suites still pass.

I have:

• Added tests covering the bug / feature (see testing)
• Updated any relevant documentation — n/a: no docs page describes the file-write/lifecycle behavior this touches

[StevenACoffman]

Ok, I'm comparing this to implementations like:

• https://github.com/google/renameio
• https://github.com/natefinch/atomic
• https://github.com/tailscale/tailscale/blob/main/atomicfile/atomicfile.go (walkthrough)
• https://github.com/sassoftware/relic/tree/master/lib/atomicfile
• https://github.com/moby/sys/tree/main/atomicwriter

[webel-crew]

Thanks — reading those five against this change was the right prompt, and it surfaced two real gaps, both now fixed in ced36a94.

Where this already matched. All five write to a temp in the same directory (so os.Rename is same-filesystem/atomic), rename into place on success, and Remove the temp on every failure path — the natefinch/atomic / tailscale/atomicfile shape, inlined. The old syscall.Unlink → os.WriteFile is gone, so a process interruption (OOM, killed build, panic, mid-chain compile error) leaves the previous file intact instead of absent — the #2345/#3505 mode.

Gap 1 — permissions regression (fixed). os.CreateTemp makes the temp 0o600 and os.Rename carries that mode through, so every regenerated file silently dropped 0644 → 0600. renameio (WithExistingPermissions) and natefinch chmod the temp to the destination's existing mode; tailscale/moby chmod to the requested perm. Now chmods the temp to the existing destination's mode, defaulting to 0o644 for a brand-new file (the exact mode os.WriteFile used here before) — so regens don't churn modes.

Gap 2 — fsync before rename (added). renameio, natefinch, tailscale, and moby all Sync() first (renameio quotes Ts'o: without fsync the directory entry can be durable before the data is, so a crash can expose a zero-length file even after a successful rename); relic doesn't. The original PR fixed process interruption — the reported bug — where fsync isn't needed (page-cache bytes survive a killed process). Added fsync anyway for the power-loss case, since it's cheap (a few files per run) and makes the write match the references.

Dep vs inline. Kept inline/dependency-free to match gqlgen's no-file-IO-deps style and the ~20-line scope of the lighter refs (tailscale's is explicitly internal; moby's is heavier than this needs). If maintainers would rather take a vetted dep, google/renameio is the one I'd reach for — happy to switch if you'd prefer that.

Tests. Added TestWriteIsAtomicPreservesPermissionsAndLeavesNoTemp (mode preserved on existing files, 0o644 default for new, no temp left behind) and TestWriteIsAtomicAndUnchangedShortCircuit (mtime-preserving short-circuit still fires, content change still writes atomically); the existing api/codegen/templates suites still pass.

[StevenACoffman]

I don't think you understand some of the nuances of the implementations I linked to.
This is the tailscale implementation:

import (
	"fmt"
	"os"
	"path/filepath"
	"runtime"
)
// WriteFile writes data to filename+some suffix, then renames it into filename.
// The perm argument is ignored on Windows, but if the target filename already
// exists then the target file's attributes and ACLs are preserved. If the target
// filename already exists but is not a regular file, WriteFile returns an error.
func WriteFile(filename string, data []byte, perm os.FileMode) (err error) {
	fi, err := os.Stat(filename)
	if err == nil && !fi.Mode().IsRegular() {
		return fmt.Errorf("%s already exists and is not a regular file", filename)
	}
	f, err := os.CreateTemp(filepath.Dir(filename), filepath.Base(filename)+".tmp")
	if err != nil {
		return err
	}
	tmpName := f.Name()
	defer func() {
		if err != nil {
			f.Close()
			os.Remove(tmpName)
		}
	}()
	if _, err := f.Write(data); err != nil {
		return err
	}
	if runtime.GOOS != "windows" {
		if err := f.Chmod(perm); err != nil {
			return err
		}
	}
	if err := f.Sync(); err != nil {
		return err
	}
	if err := f.Close(); err != nil {
		return err
	}
	return Rename(tmpName, filename)
}


// Rename srcFile to dstFile, similar to [os.Rename] but preserving file
// attributes and ACLs on Windows.
func Rename(srcFile, dstFile string) error { return rename(srcFile, dstFile) }

This will write to a temporary file first, then do an atomic rename to the final destination.

The temporary file is created in the same directory as the target, to give the best chance of being able to do an atomic rename. You can’t do an atomic rename across filesystem boundaries; using the same directory ensures both files are on the same filesystem.

To handle concurrent writes, I normally insert a random UUID into the temporary filename, so different processes write to different tempfiles. This is handled automatically by Go’s os.CreateTemp function, which adds a random string to the end of the filename.

The Rename() function has different logic for Windows and non-Windows systems:

• On non-Windows, it uses os.Rename(). The Go documentation notes that “even within the same directory, on non-Unix platforms Rename is not an atomic operation”.
• On Windows, it makes a syscall to the ReplaceFileW function. A cursory Internet search is conflicted on whether this is a truly atomic rename, although it concurs that it’s the best option on Windows.

This is the windows file:

package atomicfile

import (
	"os"

	"golang.org/x/sys/windows"
)

func rename(srcFile, destFile string) error {
	// Use replaceFile when possible to preserve the original file's attributes and ACLs.
	if err := replaceFile(destFile, srcFile); err == nil || err != windows.ERROR_FILE_NOT_FOUND {
		return err
	}
	// destFile doesn't exist. Just do a normal rename.
	return os.Rename(srcFile, destFile)
}

func replaceFile(destFile, srcFile string) error {
	destFile16, err := windows.UTF16PtrFromString(destFile)
	if err != nil {
		return err
	}

	srcFile16, err := windows.UTF16PtrFromString(srcFile)
	if err != nil {
		return err
	}

	return replaceFileW(destFile16, srcFile16, nil, 0, nil, nil)
}

Which then calls this fileto make the sys call:

package atomicfile

import (
	"syscall"
	"unsafe"

	"golang.org/x/sys/windows"
)

var _ unsafe.Pointer

// Do the interface allocations only once for common
// Errno values.
const (
	errnoERROR_IO_PENDING = 997
)

var (
	errERROR_IO_PENDING error = syscall.Errno(errnoERROR_IO_PENDING)
	errERROR_EINVAL     error = syscall.EINVAL
)

// errnoErr returns common boxed Errno values, to prevent
// allocations at runtime.
func errnoErr(e syscall.Errno) error {
	switch e {
	case 0:
		return errERROR_EINVAL
	case errnoERROR_IO_PENDING:
		return errERROR_IO_PENDING
	}
	// TODO: add more here, after collecting data on the common
	// error values see on Windows. (perhaps when running
	// all.bat?)
	return e
}

var (
	modkernel32 = windows.NewLazySystemDLL("kernel32.dll")

	procReplaceFileW = modkernel32.NewProc("ReplaceFileW")
)

func replaceFileW(replaced *uint16, replacement *uint16, backup *uint16, flags uint32, exclude unsafe.Pointer, reserved unsafe.Pointer) (err error) {
	r1, _, e1 := syscall.SyscallN(procReplaceFileW.Addr(), uintptr(unsafe.Pointer(replaced)), uintptr(unsafe.Pointer(replacement)), uintptr(unsafe.Pointer(backup)), uintptr(flags), uintptr(exclude), uintptr(reserved))
	if int32(r1) == 0 {
		err = errnoErr(e1)
	}
	return
}

And this is the not windows file:

//go:build !windows

package atomicfile

import (
	"os"
)

func rename(srcFile, destFile string) error {
	return os.Rename(srcFile, destFile)
}

[webel-crew]

CI is fully green now. The check-generate failures were a bug in this PR:

The upfront syscall.Unlink this PR removed was load-bearing. Every codegen/testserver fixture (and the federation examples) autobinds the same package its models are generated into. With the unlink gone, the stale models-gen.go was still visible at schema-load time, so autobind bound the previously-generated types as if they were user-written models → modelgen skipped regenerating them → the rewritten model file silently lost them (I watched followschema's 436-line models-gen.go collapse to a bare package clause) → the exec build's reload failed with unable to find type. Fully deterministic — base and master pass go generate ./..., every commit of this branch failed; the "different type each run" in CI was just map iteration order choosing which missing type to report first.

The fix (c7cbb9ef) keeps both properties: rather than deleting the outputs (crash-unsafe — the #2345 hole) or leaving them visible (the autobind regression), api.Generate now masks its own existing outputs from the type loader via packages.Config.Overlay — each is overlaid with a package-clause-only stub, and templates.write unmasks per-file the moment the new contents are renamed into place. The loader sees the same "outputs don't exist yet" world the old unlink provided, while disk is only ever touched by the atomic rename — kill the process at any instant and the previous files are intact. Significantly: the stub keeps the real file's package clause (empty overlay contents would be a parse error and poison loading the rest of the package), and the overlay map lives on config.Config because LoadSchema recreates c.Packages mid-run — a mask held only by the first instance would silently vanish.

Verified with go generate ./... run twice back-to-back: both exit 0 and the tree stays clean (regenerated output identical to what's committed), which is exactly what the check-generate gate asserts. The single-package repro (codegen/testserver/followschema: rm -f resolver.go && go run ../../../testdata/gqlgen.go -config gqlgen.yml -stub stub.go on a clean tree) fails without the commit and passes with it.

No standalone regression test in the diff: I built a minimal self-autobind fixture and it does not reproduce — the trigger needs the testserver fixtures' richer interface/implementor topology — and check-generate over the full corpus is already the deterministic gate that caught this.

Earlier in the thread I suggested this looked like a pre-existing flake — that was wrong, based on a local bisect against base/master. Happy to adjust the overlay approach if you'd rather see it shaped differently.