fix: keep built binary after app shutdown (#911)

Air could delete build.entrypoint while stopping the previous app process when stop_on_error was enabled. After builds were changed to keep the old app alive until a replacement build succeeds, that cleanup could remove the freshly rebuilt binary before it was started, causing issue #910.

Failed builds should not delete the last executable either. Keeping the binary in place makes stop_on_error stop the process without destroying the artifact a later rebuild or manual restart may need.

Windows needs different sequencing because running executables are locked. Stop the old process before build.cmd on Windows, while keeping the retained-app behavior on Unix platforms.

Validation: go test ./runner -run 'TestAddPlatformOverridesForInit|TestPlatformBuildOverridesSelection|TestShouldStopBinBeforeBuild|TestStopBinBeforeBuildIfNeeded|TestBuildRunKeepsIssue910GoBuildEntrypoint' -count=1 -v; go test ./runner -run 'TestBuildRunKeepsIssue910GoBuildEntrypoint|TestShouldStopBinBeforeBuild|TestBuildRunKeeps.*Binary.*StopOnError|TestBuildRunStopsExistingBinWhenBuildFailsWithStopOnError|TestBuildRunStopsExistingBinAfterSuccessfulBuild|TestRebuild$' -count=1 -v; go test ./...; make check

🤖 Generated with [OpenAI Codex](https://openai.com/codex)

Co-authored-by: Marius van Niekerk <mariusvniekerk@mbp-marius-kenn.emperor-gopher.ts.net>
Co-authored-by: OpenAI Codex <noreply@openai.com>

runner/common.go

@@ -1,6 +1,10 @@
 package runner
 
 const (
-	//PlatformWindows const for windows
+	// PlatformWindows is the GOOS value for Windows.
 	PlatformWindows = "windows"
+	// PlatformDarwin is the GOOS value for macOS.
+	PlatformDarwin = "darwin"
+	// PlatformLinux is the GOOS value for Linux.
+	PlatformLinux = "linux"
 )

runner/config.go

@@ -425,9 +425,9 @@ func platformBuildOverrides(build *cfgBuild, goos string) *cfgBuildOverrides {
 	switch goos {
 	case PlatformWindows:
 		return build.Windows
-	case "darwin":
+	case PlatformDarwin:
 		return build.Darwin
-	case "linux":
+	case PlatformLinux:
 		return build.Linux
 	default:
 		return nil
@@ -483,9 +483,9 @@ func addPlatformOverridesForInit(cfg *Config, goos string) {
 	switch goos {
 	case PlatformWindows:
 		cfg.Build.Windows = override
-	case "darwin":
+	case PlatformDarwin:
 		cfg.Build.Darwin = override
-	case "linux":
+	case PlatformLinux:
 		cfg.Build.Linux = override
 	}
 }

runner/config_test.go

@@ -369,6 +369,28 @@ func TestAddPlatformOverridesForInit(t *testing.T) {
 	if !reflect.DeepEqual(cfg.Build.Windows.Entrypoint, entrypoint{`tmp\main.exe`}) {
 		t.Fatalf("windows entrypoint mismatch: got %v", cfg.Build.Windows.Entrypoint)
 	}
+
+	cfg = defaultConfigBase()
+	cfg.Build.Cmd = "darwin-cmd"
+	setEntrypointFromBin(&cfg)
+	addPlatformOverridesForInit(&cfg, PlatformDarwin)
+	if cfg.Build.Darwin == nil {
+		t.Fatal("expected darwin overrides to be set")
+	}
+	if cfg.Build.Darwin.Cmd != "go build -o ./tmp/main ." {
+		t.Fatalf("darwin cmd mismatch: got %s", cfg.Build.Darwin.Cmd)
+	}
+
+	cfg = defaultConfigBase()
+	cfg.Build.Cmd = "linux-cmd"
+	setEntrypointFromBin(&cfg)
+	addPlatformOverridesForInit(&cfg, PlatformLinux)
+	if cfg.Build.Linux == nil {
+		t.Fatal("expected linux overrides to be set")
+	}
+	if cfg.Build.Linux.Cmd != "go build -o ./tmp/main ." {
+		t.Fatalf("linux cmd mismatch: got %s", cfg.Build.Linux.Cmd)
+	}
 }
 
 func TestDefaultConfigForOS(t *testing.T) {
@@ -382,7 +404,7 @@ func TestDefaultConfigForOS(t *testing.T) {
 		t.Fatalf("windows bin mismatch: got %q", winCfg.Build.Bin)
 	}
 
-	linuxCfg := defaultConfigForOS("linux")
+	linuxCfg := defaultConfigForOS(PlatformLinux)
 	if linuxCfg.Build.Cmd != "go build -o ./tmp/main ." {
 		t.Fatalf("linux cmd mismatch: got %q", linuxCfg.Build.Cmd)
 	}
@@ -511,10 +533,10 @@ func TestPlatformBuildOverridesSelection(t *testing.T) {
 	if got := platformBuildOverrides(build, PlatformWindows); got != win {
 		t.Fatalf("windows override mismatch: got %v", got)
 	}
-	if got := platformBuildOverrides(build, "darwin"); got != darwin {
+	if got := platformBuildOverrides(build, PlatformDarwin); got != darwin {
 		t.Fatalf("darwin override mismatch: got %v", got)
 	}
-	if got := platformBuildOverrides(build, "linux"); got != linux {
+	if got := platformBuildOverrides(build, PlatformLinux); got != linux {
 		t.Fatalf("linux override mismatch: got %v", got)
 	}
 	if got := platformBuildOverrides(build, "freebsd"); got != nil {

runner/engine.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"sync"
 	"sync/atomic"
@@ -540,6 +541,8 @@ func (e *Engine) buildRun() {
 	default:
 	}
 
+	e.stopBinBeforeBuildIfNeeded(runtime.GOOS)
+
 	e.loadEnvFile()
 
 	var err error
@@ -586,6 +589,18 @@ func (e *Engine) buildRun() {
 	}
 }
 
+func shouldStopBinBeforeBuild(goos string) bool {
+	return goos == PlatformWindows
+}
+
+func (e *Engine) stopBinBeforeBuildIfNeeded(goos string) {
+	// Windows locks running executables, so direct builds to entrypoint need
+	// the old process stopped before build.cmd can overwrite the binary.
+	if shouldStopBinBeforeBuild(goos) {
+		e.stopBin()
+	}
+}
+
 func (e *Engine) flushEvents() {
 	for {
 		select {
@@ -717,20 +732,6 @@ func (e *Engine) runBin() error {
 			} else {
 				e.mainDebug("cmd killed, pid: %d", pid)
 			}
-
-			if e.config.Build.StopOnError {
-				relBinPath := e.config.rel(e.config.binPath())
-				if relBinPath == "" || strings.HasPrefix(relBinPath, "..") {
-					return
-				}
-				cmdBinPath := cmdPath(relBinPath)
-				if _, err = os.Stat(cmdBinPath); os.IsNotExist(err) {
-					return
-				}
-				if err = os.Remove(cmdBinPath); err != nil {
-					e.mainLog("failed to remove %s, error: %s", relBinPath, err)
-				}
-			}
 		}()
 
 		return shutdown

runner/engine_test.go

@@ -302,6 +302,112 @@ func TestBuildRunStopsExistingBinAfterSuccessfulBuild(t *testing.T) {
 	}
 }
 
+func TestBuildRunKeepsFreshBinaryAfterSuccessfulBuildWithStopOnError(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("uses POSIX shell commands")
+	}
+
+	tmpDir := t.TempDir()
+	chdir(t, tmpDir)
+
+	require.NoError(t, os.Mkdir("tmp", 0o755))
+	binPath := filepath.Join("tmp", "dev")
+	initialScript := []byte("#!/bin/sh\ntrap 'exit 0' INT TERM\nwhile :; do sleep 1; done\n")
+	require.NoError(t, os.WriteFile(binPath, initialScript, 0o755))
+	require.NoError(t, os.WriteFile("next.sh", initialScript, 0o755))
+
+	engine, err := NewEngine("", nil, true)
+	require.NoError(t, err)
+	engine.config.Log.Silent = true
+	engine.config.Build.Cmd = "cp ./next.sh ./tmp/dev && chmod +x ./tmp/dev"
+	engine.config.Build.Entrypoint = entrypoint{"./tmp/dev"}
+	engine.config.Build.StopOnError = true
+	engine.config.Build.SendInterrupt = true
+	engine.config.Build.KillDelay = 100 * time.Millisecond
+
+	require.NoError(t, engine.runBin())
+	defer engine.stopBin()
+
+	err = waitForCondition(t, time.Second, func() bool {
+		started := false
+		engine.withLock(func() {
+			started = engine.binStopCh != nil
+		})
+		return started
+	}, "initial binary process to start")
+	require.NoError(t, err)
+
+	engine.buildRun()
+
+	err = waitForCondition(t, time.Second, func() bool {
+		started := false
+		engine.withLock(func() {
+			started = engine.binStopCh != nil
+		})
+		return started
+	}, "rebuilt binary process to start")
+	require.NoError(t, err)
+	require.FileExists(t, binPath)
+}
+
+func TestBuildRunKeepsIssue910GoBuildEntrypoint(t *testing.T) {
+	if runtime.GOOS == PlatformWindows {
+		t.Skip("issue #910 was reported on Linux; Windows stops before build because running executables are locked")
+	}
+
+	tmpDir := t.TempDir()
+	chdir(t, tmpDir)
+
+	require.NoError(t, os.Mkdir("tmp", 0o755))
+	require.NoError(t, os.WriteFile("go.mod", []byte("module issue910.test\n\ngo 1.17\n"), 0o644))
+	require.NoError(t, os.WriteFile("main.go", []byte(`package main
+
+import "time"
+
+func main() {
+	for {
+		time.Sleep(time.Second)
+	}
+}
+`), 0o644))
+
+	binPath := filepath.Join("tmp", "dev")
+	require.NoError(t, exec.Command("go", "build", "-o", binPath, ".").Run())
+
+	engine, err := NewEngine("", nil, true)
+	require.NoError(t, err)
+	engine.config.Log.Silent = true
+	engine.config.Build.Cmd = "go build -o ./tmp/dev .; echo 'build ok'; sleep 1"
+	engine.config.Build.Entrypoint = entrypoint{"./tmp/dev"}
+	engine.config.Build.StopOnError = true
+	engine.config.Build.SendInterrupt = true
+	engine.config.Build.KillDelay = 500 * time.Millisecond
+
+	require.NoError(t, engine.runBin())
+	defer engine.stopBin()
+
+	err = waitForCondition(t, time.Second, func() bool {
+		started := false
+		engine.withLock(func() {
+			started = engine.binStopCh != nil
+		})
+		return started
+	}, "initial issue #910 binary process to start")
+	require.NoError(t, err)
+
+	engine.buildRun()
+
+	err = waitForCondition(t, time.Second, func() bool {
+		started := false
+		engine.withLock(func() {
+			started = engine.binStopCh != nil
+		})
+		return started
+	}, "rebuilt issue #910 binary process to start")
+	require.NoError(t, err)
+	require.FileExists(t, binPath)
+}
+
 func TestBuildRunStopsExistingBinWhenBuildFailsWithStopOnError(t *testing.T) {
 	engine, err := NewEngine("", nil, true)
 	require.NoError(t, err)
@@ -328,6 +434,96 @@ func TestBuildRunStopsExistingBinWhenBuildFailsWithStopOnError(t *testing.T) {
 	}
 }
 
+func TestBuildRunKeepsBinaryAfterFailedBuildWithStopOnError(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("uses POSIX shell commands")
+	}
+
+	tmpDir := t.TempDir()
+	chdir(t, tmpDir)
+
+	require.NoError(t, os.Mkdir("tmp", 0o755))
+	binPath := filepath.Join("tmp", "dev")
+	script := []byte("#!/bin/sh\ntrap 'exit 0' INT TERM\nwhile :; do sleep 1; done\n")
+	require.NoError(t, os.WriteFile(binPath, script, 0o755))
+
+	engine, err := NewEngine("", nil, true)
+	require.NoError(t, err)
+	engine.config.Log.Silent = true
+	engine.config.Build.Cmd = "false"
+	engine.config.Build.Entrypoint = entrypoint{"./tmp/dev"}
+	engine.config.Build.StopOnError = true
+	engine.config.Build.SendInterrupt = true
+	engine.config.Build.KillDelay = 100 * time.Millisecond
+
+	require.NoError(t, engine.runBin())
+	defer engine.stopBin()
+
+	err = waitForCondition(t, time.Second, func() bool {
+		started := false
+		engine.withLock(func() {
+			started = engine.binStopCh != nil
+		})
+		return started
+	}, "initial binary process to start")
+	require.NoError(t, err)
+
+	engine.buildRun()
+
+	require.FileExists(t, binPath)
+}
+
+func TestShouldStopBinBeforeBuild(t *testing.T) {
+	tests := []struct {
+		goos string
+		want bool
+	}{
+		{goos: PlatformWindows, want: true},
+		{goos: PlatformDarwin, want: false},
+		{goos: PlatformLinux, want: false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.goos, func(t *testing.T) {
+			assert.Equal(t, tt.want, shouldStopBinBeforeBuild(tt.goos))
+		})
+	}
+}
+
+func TestStopBinBeforeBuildIfNeeded(t *testing.T) {
+	engine, err := NewEngine("", nil, true)
+	require.NoError(t, err)
+	engine.config.Log.Silent = true
+
+	stopped := make(chan struct{})
+	shutdown := make(chan chan int, 1)
+	engine.binStopCh = shutdown
+	go func() {
+		closer := <-shutdown
+		close(closer)
+		close(stopped)
+	}()
+
+	engine.stopBinBeforeBuildIfNeeded(PlatformWindows)
+
+	select {
+	case <-stopped:
+	case <-time.After(time.Second):
+		t.Fatal("expected Windows pre-build stop to stop the existing binary")
+	}
+
+	shutdown = make(chan chan int, 1)
+	engine.binStopCh = shutdown
+
+	engine.stopBinBeforeBuildIfNeeded(PlatformLinux)
+
+	select {
+	case <-shutdown:
+		t.Fatal("non-Windows pre-build step should keep the existing binary running")
+	default:
+	}
+}
+
 func GetPort() (int, func()) {
 	l, err := net.Listen("tcp", "127.0.0.1:0")
 	if err != nil {