fix(compass-app): stop runStackUp/Down hanging on a fire-and-return stack (SEA-1685)

go/cmd/compass-app/embedded.go

@@ -17,7 +17,6 @@
 package main
 
 import (
-	"bytes"
 	"context"
 	"errors"
 	"fmt"
@@ -145,6 +144,37 @@ func stackUpArgs(p embeddedParams) []string {
 	return args
 }
 
+// captureStderr wires cmd.Stderr to a temp *os.File and returns a reader for the
+// bytes captured so far plus a cleanup that closes and removes the file.
+// Capturing to an *os.File — not a bytes.Buffer — is load-bearing for the
+// fire-and-return stack commands: `compass-stack up` exits 0 once the stack is
+// Ready while its postgres/server/runner children keep running. os/exec backs a
+// non-*os.File stderr writer with an OS pipe whose copy goroutine Cmd.Wait
+// blocks on until EOF, and those lingering children inherit the pipe's
+// write-end, so EOF never arrives and Wait hangs forever. An *os.File is dup'd
+// straight into the child (no pipe, no goroutine), so Wait returns the instant
+// compass-stack itself exits; and the children write to a plain file that never
+// EPIPEs, so capturing this way never signals the very stack the app must keep
+// alive.
+func captureStderr(cmd *exec.Cmd) (read func() string, cleanup func(), err error) {
+	f, err := os.CreateTemp("", "compass-stack-stderr-*")
+	if err != nil {
+		return nil, nil, fmt.Errorf("creating stderr capture file: %w", err)
+	}
+	cmd.Stderr = f
+	read = func() string {
+		// Best-effort: an unreadable capture degrades to the generic
+		// "compass-stack ... failed" error, and never blocks surfacing.
+		b, _ := os.ReadFile(f.Name())
+		return strings.TrimSpace(string(b))
+	}
+	cleanup = func() {
+		_ = f.Close()
+		_ = os.Remove(f.Name())
+	}
+	return read, cleanup, nil
+}
+
 // runStackUp is the real stackUp seam: it execs the compass-stack binary at bin
 // with the given argv and waits for it to exit 0 (up is fire-and-return, so
 // Run returning nil means the stack reached Ready and its children keep
@@ -155,14 +185,17 @@ func runStackUp(bin string) func(ctx context.Context, args []string) error {
 		//nolint:gosec // G204: bin is operator/PATH-resolved (resolveStackBin) and
 		// the argv is pipeline-assembled (stackUpArgs), not user input.
 		cmd := exec.CommandContext(ctx, bin, args...)
-		var stderr bytes.Buffer
-		cmd.Stderr = &stderr
+		stderr, cleanup, capErr := captureStderr(cmd)
+		if capErr != nil {
+			return capErr
+		}
+		defer cleanup()
 		if err := cmd.Run(); err != nil {
 			if ctx.Err() == context.DeadlineExceeded || errors.Is(err, context.DeadlineExceeded) {
 				return fmt.Errorf("compass-stack up exceeded the %s bring-up window "+
 					"(a cold agent-image pull from GHCR can take longer on first run): %w", bringUpTimeout, err)
 			}
-			if msg := strings.TrimSpace(stderr.String()); msg != "" {
+			if msg := stderr(); msg != "" {
 				return fmt.Errorf("compass-stack up failed: %w: %s", err, msg)
 			}
 			return fmt.Errorf("compass-stack up failed: %w", err)
@@ -201,14 +234,17 @@ func runStackDown(bin string) func(ctx context.Context, args []string) error {
 		//nolint:gosec // G204: bin is operator/PATH-resolved (resolveStackBin) and
 		// the argv is pipeline-assembled (stackDownArgs), not user input.
 		cmd := exec.CommandContext(ctx, bin, args...)
-		var stderr bytes.Buffer
-		cmd.Stderr = &stderr
+		stderr, cleanup, capErr := captureStderr(cmd)
+		if capErr != nil {
+			return capErr
+		}
+		defer cleanup()
 		if err := cmd.Run(); err != nil {
 			if ctx.Err() == context.DeadlineExceeded || errors.Is(err, context.DeadlineExceeded) {
 				return fmt.Errorf("compass-stack down exceeded the %s teardown window "+
 					"(attach, SIGTERM the child tree, wait the server drain): %w", stackDownTimeout, err)
 			}
-			if msg := strings.TrimSpace(stderr.String()); msg != "" {
+			if msg := stderr(); msg != "" {
 				return fmt.Errorf("compass-stack down failed: %w: %s", err, msg)
 			}
 			return fmt.Errorf("compass-stack down failed: %w", err)

go/cmd/compass-app/embedded_test.go

@@ -300,6 +300,47 @@ func TestRunStackUpZeroExitSucceeds(t *testing.T) {
 	}
 }
 
+// TestRunStackUpReturnsWhileChildrenLinger is the regression guard for the
+// fire-and-return hang: `compass-stack up` exits 0 once the stack is Ready while
+// its postgres/server/runner children keep running, and those children inherit
+// the exec'd command's stderr. If runStackUp captured stderr into a bytes.Buffer
+// (os/exec's pipe + copy-goroutine path), cmd.Wait would block until the pipe
+// hit EOF — which the lingering children hold open — so Run would hang for the
+// children's whole lifetime. Capturing to a temp *os.File (captureStderr) makes
+// Run return the instant the top-level child exits, regardless of survivors.
+//
+// Driven with /bin/sh that backgrounds a long sleep (a stand-in for the
+// reparented stack children) holding stderr, then exits 0. Pre-fix this blocks
+// for the sleep's 60s; the fix returns immediately. The assertion is that Run
+// completes well under the sleep — a plain wall-clock bound, but the pre-fix gap
+// (60s vs milliseconds) is enormous, so it is not flaky. The backgrounded sleep
+// is cleaned up via its own short lifetime; the test spawns nothing it must kill
+// (rule://process-safety — never pkill).
+func TestRunStackUpReturnsWhileChildrenLinger(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), embeddedTestTimeout)
+	defer cancel()
+
+	// A short-lived grandchild that outlives its parent and inherits stderr: the
+	// exact fire-and-return shape of `compass-stack up`. sleep 5 is far longer
+	// than any correct runStackUp (which returns at the parent's exit, ~ms) and
+	// well past the 1s assertion below, yet short enough that a regressed run's
+	// leaked grandchild self-reaps in seconds rather than a minute.
+	stackUp := runStackUp("/bin/sh")
+	start := time.Now()
+	err := stackUp(ctx, []string{"-c", "sleep 5 & exit 0"})
+	elapsed := time.Since(start)
+
+	if err != nil {
+		t.Fatalf("stackUp with a lingering child err = %v, want nil", err)
+	}
+	// Generous bound: the fix returns in single-digit ms, while a regressed Run
+	// blocks until the grandchild exits (~5s) — far past 1s. Anything under a
+	// second proves Run did not wait on the grandchild.
+	if elapsed > time.Second {
+		t.Fatalf("stackUp took %s with a lingering child — Run waited on the inherited stderr pipe (the fire-and-return hang regressed)", elapsed)
+	}
+}
+
 // TestRunStackDownNonZeroExitSurfacesStderr: the real stackDown seam surfaces a
 // non-zero exit as an error carrying the child's stderr, mirroring runStackUp.
 // Driven with /bin/sh printing to stderr and exiting 1 — no real compass-stack