fix: reap workspace test tmux after abrupt exits

context/testing.md

@@ -433,6 +433,9 @@ instead of serializing the whole test.
   (`frontend/tests/e2e-full/support/e2eServer.ts::cleanupE2ETmuxDir`).
 - Real-tmux Go test binaries install signal cleanup because termination skips
   code after `m.Run` and `t.Cleanup` (`internal/testutil/testsignal.Install`).
+- Long-lived package-level tmux servers must use `testtmux.Owner`; its reaper
+  reaps servers after uncatchable owner death, which signal cleanup cannot handle.
+  (`internal/testutil/testtmux/reaper_unix.go::startOwnerReaper`)
 
 Windows test binaries that launch restart-durable PTY processes must contain
 their descendants in a kill-on-close Job Object; test timeouts bypass normal

internal/server/workspacetest/testmain_cleanup_test.go

@@ -0,0 +1,93 @@
+package workspacetest
+
+import (
+	"os"
+	"os/exec"
+	"runtime"
+	"strconv"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	"go.kenn.io/forge/internal/procutil"
+)
+
+const abruptWorkspaceTestTmuxOwnerPID = "KENN_FORGE_WORKSPACETEST_ABRUPT_OWNER_PID"
+
+func runAbruptWorkspaceTestTmuxOwnerHelper() {
+	pidPath := os.Getenv(abruptWorkspaceTestTmuxOwnerPID)
+	if pidPath == "" {
+		return
+	}
+	args := append(append([]string(nil), workspaceTestTmuxCommand[1:]...),
+		"new-session", "-d", "-s", "abrupt-owner", "sleep", "300",
+	)
+	if output, err := procutil.Command(
+		workspaceTestTmuxCommand[0], args...,
+	).CombinedOutput(); err != nil {
+		_, _ = os.Stderr.Write(output)
+		os.Exit(2)
+	}
+	args = append(append([]string(nil), workspaceTestTmuxCommand[1:]...),
+		"display-message", "-p", "#{pid}",
+	)
+	output, err := procutil.Command(
+		workspaceTestTmuxCommand[0], args...,
+	).CombinedOutput()
+	if err != nil {
+		_, _ = os.Stderr.Write(output)
+		os.Exit(2)
+	}
+	if err := os.WriteFile(pidPath, []byte(strings.TrimSpace(string(output))), 0o600); err != nil {
+		os.Exit(2)
+	}
+	select {}
+}
+
+func TestWorkspaceTestTmuxStopsServerAfterAbruptOwnerExit(t *testing.T) {
+	if runtime.GOOS != "darwin" && runtime.GOOS != "linux" {
+		t.Skip("private test tmux owners require Darwin or Linux")
+	}
+	if _, err := exec.LookPath("tmux"); err != nil {
+		t.Skipf("tmux is unavailable: %v", err)
+	}
+	require := require.New(t)
+	pidPath := t.TempDir() + "/server.pid"
+	owner := procutil.Command(os.Args[0], "-test.run=^$")
+	owner.Env = append(os.Environ(), abruptWorkspaceTestTmuxOwnerPID+"="+pidPath)
+	require.NoError(owner.Start())
+	t.Cleanup(func() {
+		if owner.ProcessState == nil {
+			_ = owner.Process.Kill()
+			_, _ = owner.Process.Wait()
+		}
+	})
+
+	var serverPID int
+	require.Eventually(func() bool {
+		content, err := os.ReadFile(pidPath)
+		if err != nil {
+			return false
+		}
+		serverPID, err = strconv.Atoi(string(content))
+		return err == nil
+	}, 5*time.Second, 25*time.Millisecond)
+	t.Cleanup(func() {
+		if !processIsGone(serverPID) {
+			_ = procutil.Command("/bin/kill", "-KILL", strconv.Itoa(serverPID)).Run()
+		}
+	})
+
+	require.NoError(owner.Process.Kill())
+	require.Error(owner.Wait())
+	require.Eventually(
+		func() bool { return processIsGone(serverPID) },
+		5*time.Second,
+		25*time.Millisecond,
+	)
+}
+
+func processIsGone(pid int) bool {
+	return procutil.Command("/bin/kill", "-0", strconv.Itoa(pid)).Run() != nil
+}

internal/server/workspacetest/testmain_test.go

@@ -1,26 +1,25 @@
 package workspacetest
 
 import (
-	"context"
 	"errors"
 	"fmt"
 	"os"
 	"os/exec"
-	"path/filepath"
 	"slices"
-	"strings"
 	"testing"
-	"time"
 
-	"go.kenn.io/forge/internal/procutil"
 	"go.kenn.io/forge/internal/testutil/gitsafe"
 	"go.kenn.io/forge/internal/testutil/processjob"
 	"go.kenn.io/forge/internal/testutil/testsignal"
+	"go.kenn.io/forge/internal/testutil/testtmux"
 )
 
 var workspaceTestTmuxCommand []string
 
 func TestMain(m *testing.M) {
+	if code, ok := testtmux.CommandWrapperExitCode(); ok {
+		os.Exit(code)
+	}
 	if slices.Contains(os.Args, workspaceRuntimeHelperMarker) {
 		os.Exit(m.Run())
 	}
@@ -33,6 +32,7 @@ func TestMain(m *testing.M) {
 		fmt.Fprintf(os.Stderr, "configure isolated workspace test tmux: %v\n", err)
 		os.Exit(1)
 	}
+	runAbruptWorkspaceTestTmuxOwnerHelper()
 	runCleanup, stopSignalCleanup := testsignal.Install(cleanupTmux, func(err error) {
 		fmt.Fprintf(os.Stderr, "cleanup isolated workspace test tmux: %v\n", err)
 	})
@@ -55,50 +55,19 @@ func configureWorkspaceTestTmux() (func() error, error) {
 	if err != nil {
 		return nil, fmt.Errorf("find tmux: %w", err)
 	}
-
-	tmuxDir, err := os.MkdirTemp("/tmp", "kenn-forge-workspacetest-tmux-*")
+	if !testtmux.Supported() {
+		return func() error { return nil }, nil
+	}
+	owner, err := testtmux.New()
 	if err != nil {
-		return nil, fmt.Errorf("create tmux socket directory: %w", err)
+		return nil, fmt.Errorf("initialize private test tmux owner: %w", err)
 	}
-	socket := filepath.Join(tmuxDir, "tmux.sock")
-	workspaceTestTmuxCommand = []string{
-		tmuxPath, "-f", "/dev/null", "-S", socket,
+	workspaceTestTmuxCommand, err = owner.CommandForRun(tmuxPath)
+	if err != nil {
+		return nil, errors.Join(
+			fmt.Errorf("register workspace test tmux server: %w", err),
+			owner.Cleanup(),
+		)
 	}
-
-	return func() error {
-		var errs []error
-		if _, statErr := os.Stat(socket); statErr == nil {
-			ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
-			out, killErr := procutil.CombinedOutput(
-				ctx,
-				procutil.CommandContext(
-					ctx, tmuxPath, "-f", "/dev/null", "-S", socket,
-					"kill-server",
-				),
-				"workspace test tmux cleanup",
-			)
-			cancel()
-			msg := strings.TrimSpace(string(out))
-			if killErr != nil && !workspaceTestTmuxServerAbsent(msg) {
-				errs = append(errs, fmt.Errorf(
-					"kill private tmux server: %w: %s",
-					killErr, msg,
-				))
-			}
-		} else if !errors.Is(statErr, os.ErrNotExist) {
-			errs = append(errs, fmt.Errorf("inspect tmux socket: %w", statErr))
-		}
-		if removeErr := os.RemoveAll(tmuxDir); removeErr != nil {
-			errs = append(errs, fmt.Errorf(
-				"remove tmux socket directory: %w", removeErr,
-			))
-		}
-		return errors.Join(errs...)
-	}, nil
-}
-
-func workspaceTestTmuxServerAbsent(msg string) bool {
-	return strings.Contains(msg, "no server running") ||
-		(strings.Contains(msg, "error connecting to") &&
-			strings.Contains(msg, "No such file or directory"))
+	return owner.Cleanup, nil
 }

internal/testutil/testtmux/owner.go

@@ -59,6 +59,7 @@ type Owner struct {
 	root         string
 	runDir       string
 	admissionDir string
+	reaper       *ownerReaper
 	mu           sync.Mutex
 	servers      map[string]registeredServer
 	closed       bool
@@ -144,6 +145,16 @@ func newAt(root string) (*Owner, error) {
 			admissionDir: admissionDir,
 			servers:      make(map[string]registeredServer),
 		}
+		owner.reaper, err = startOwnerReaper(runDir)
+		if err != nil {
+			cleanupErr := errors.Join(
+				fmt.Errorf("start private tmux owner reaper: %w", err),
+				os.RemoveAll(runDir),
+				os.RemoveAll(admissionDir),
+			)
+			owner = nil
+			return cleanupErr
+		}
 		return nil
 	})
 	return owner, err
@@ -225,38 +236,74 @@ func publishOwnerState(
 // Command registers a private socket before returning a tmux command prefix.
 func (o *Owner) Command(t testing.TB, tmuxPath string) []string {
 	t.Helper()
+	command, server, err := o.register(tmuxPath)
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		require.NoError(t, o.release(server))
+	})
+	return command
+}
+
+// CommandForRun registers a private socket for the lifetime of the owner.
+func (o *Owner) CommandForRun(tmuxPath string) ([]string, error) {
+	command, _, err := o.register(tmuxPath)
+	return command, err
+}
+
+func (o *Owner) register(tmuxPath string) ([]string, registeredServer, error) {
 	o.mu.Lock()
 	defer o.mu.Unlock()
-	require.False(t, o.closed, "private tmux owner is cleaning up")
+	if o.closed {
+		return nil, registeredServer{}, errors.New("private tmux owner is cleaning up")
+	}
 
 	nonce, err := randomToken(6)
-	require.NoError(t, err)
+	if err != nil {
+		return nil, registeredServer{}, err
+	}
 	serverDir := filepath.Join(o.runDir, "server-"+nonce)
-	require.NoError(t, os.Mkdir(serverDir, 0o700))
+	if err := os.Mkdir(serverDir, 0o700); err != nil {
+		return nil, registeredServer{}, fmt.Errorf("create tmux test server directory: %w", err)
+	}
+	removeServerDir := true
+	defer func() {
+		if removeServerDir {
+			_ = os.RemoveAll(serverDir)
+		}
+	}()
 	socket := filepath.Join(serverDir, "tmux.sock")
 	executable, err := os.Executable()
-	require.NoError(t, err)
+	if err != nil {
+		return nil, registeredServer{}, fmt.Errorf("resolve tmux test wrapper executable: %w", err)
+	}
 	metadata, err := json.Marshal(wrapperMetadata{
 		TmuxPath:     tmuxPath,
 		AdmissionDir: o.admissionDir,
 	})
-	require.NoError(t, err)
-	require.NoError(t, os.WriteFile(
+	if err != nil {
+		return nil, registeredServer{}, fmt.Errorf("encode tmux test wrapper metadata: %w", err)
+	}
+	if err := os.WriteFile(
 		filepath.Join(serverDir, wrapperMetadataName), metadata, 0o600,
-	))
+	); err != nil {
+		return nil, registeredServer{}, fmt.Errorf("write tmux test wrapper metadata: %w", err)
+	}
 	wrapperPath := filepath.Join(serverDir, wrapperExecutableName)
-	require.NoError(t, os.Symlink(executable, wrapperPath))
+	if err := os.Symlink(executable, wrapperPath); err != nil {
+		return nil, registeredServer{}, fmt.Errorf("link tmux test wrapper: %w", err)
+	}
 	server := registeredServer{tmuxPath: tmuxPath, socket: socket}
 	o.servers[socket] = server
-	t.Cleanup(func() {
-		require.NoError(t, o.release(server))
-	})
-	return []string{wrapperPath, "-f", "/dev/null", "-S", socket}
+	removeServerDir = false
+	return []string{wrapperPath, "-f", "/dev/null", "-S", socket}, server, nil
 }
 
 // CommandWrapperExitCode runs a private tmux command when the current test
 // binary was invoked through an Owner command wrapper.
 func CommandWrapperExitCode() (int, bool) {
+	if code, ok := ownerReaperExitCode(); ok {
+		return code, true
+	}
 	if filepath.Base(os.Args[0]) != wrapperExecutableName {
 		return 0, false
 	}
@@ -326,6 +373,17 @@ func runWrappedTmux(metadata wrapperMetadata, args []string) int {
 // Cleanup stops every registered server and removes this owner's run.
 func (o *Owner) Cleanup() error {
 	o.cleanup.Do(func() {
+		defer func() {
+			if o.reaper == nil {
+				return
+			}
+			if o.cleanupErr != nil {
+				o.cleanupErr = errors.Join(o.cleanupErr, o.reaper.cancel())
+				return
+			}
+			o.cleanupErr = o.reaper.stop()
+		}()
+
 		o.mu.Lock()
 		o.closed = true
 		o.mu.Unlock()

internal/testutil/testtmux/owner_test.go

@@ -761,7 +761,7 @@ esac
 	select {
 	case cleanupErr := <-cleanupDone:
 		require.NoError(cleanupErr)
-	case <-time.After(cleanupTimeout + 2*time.Second):
+	case <-time.After(2*cleanupTimeout + 2*time.Second):
 		require.Fail("cleanup did not terminate a stalled admitted tmux startup")
 		_ = startup.Process.Kill()
 		require.NoError(<-cleanupDone)