Start all commands of a pipeline before waiting for any of them

PipeCommands ran every command in its own goroutine, each doing
Start/read-stderr/Wait, with nothing ordering one goroutine's Start
against another's Wait. That ordering matters: StdoutPipe registers the
parent's read end in cmd.parentIOPipes, and Cmd.Wait closes those
descriptors when it returns. The next command's Stdin is that very
*os.File, and exec passes a user-supplied *os.File through untouched, so
Start hands the child whatever the fd happens to be at that moment. If
the producer finished and got reaped before the consumer's goroutine
reached Start, that fd was already closed, File.Fd() returned -1, and
the child was started with fd 0 closed -- reading nothing at all.

The only caller is the pre-2.35 fallback in SaveStagedChanges, which
pipes `git stash show -p` into `git apply -R`. Losing that race left
git apply with an empty patch, so it failed with "unrecognized input",
the following `git stash drop` never ran, and the user was left with a
stray stash entry. This turned up as a flaky stash/stash_staged on the
git 2.32.0 CI job; the newer-git jobs take the `git stash push --staged`
path and never reach this code.

Starting every command up front removes the race, and collecting stderr
into buffers lets exec's own copying goroutines do the work. That also
fixes two lesser problems in the same function: finalErrors was appended
to from several goroutines without synchronization, and a failed Start
was only logged, so a pipeline that never ran reported success.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

pkg/commands/oscommands/os.go

@@ -1,12 +1,12 @@
 package oscommands
 
 import (
+	"bytes"
 	"io"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"strings"
-	"sync"
 
 	"github.com/go-errors/errors"
 	"github.com/samber/lo"
@@ -228,37 +228,47 @@ func (c *OSCommand) PipeCommands(cmdObjs ...*CmdObj) error {
 	// keeping this here in case I adapt this code for some other purpose in the future
 	// cmds[len(cmds)-1].Stdout = os.Stdout
 
-	finalErrors := []string{}
-
-	wg := sync.WaitGroup{}
-	wg.Add(len(cmds))
-
-	for _, cmd := range cmds {
-		go utils.Safe(func() {
-			stderr, err := cmd.StderrPipe()
-			if err != nil {
-				c.Log.Error(err)
-			}
-
-			if err := cmd.Start(); err != nil {
-				c.Log.Error(err)
-			}
-
-			if b, err := io.ReadAll(stderr); err == nil {
-				if len(b) > 0 {
-					finalErrors = append(finalErrors, string(b))
-				}
-			}
-
-			if err := cmd.Wait(); err != nil {
-				c.Log.Error(err)
-			}
-
-			wg.Done()
-		})
+	stderrs := make([]bytes.Buffer, len(cmds))
+	for i := range cmds {
+		cmds[i].Stderr = &stderrs[i]
 	}
 
-	wg.Wait()
+	// Start every command before waiting for any of them: waiting for a command
+	// closes our end of the pipe that feeds the next one, and a command that
+	// hasn't been started by then would inherit a closed stdin.
+	started := 0
+	var startErr error
+	for _, cmd := range cmds {
+		if err := cmd.Start(); err != nil {
+			startErr = err
+			break
+		}
+
+		started++
+	}
+
+	finalErrors := []string{}
+
+	if startErr != nil {
+		c.Log.Error(startErr)
+		finalErrors = append(finalErrors, startErr.Error())
+
+		// Without the rest of the pipeline to drain them, the commands we did
+		// start could block forever writing to a full pipe.
+		for _, cmd := range cmds[:started] {
+			_ = cmd.Process.Kill()
+		}
+	}
+
+	for i, cmd := range cmds[:started] {
+		if err := cmd.Wait(); err != nil {
+			c.Log.Error(err)
+		}
+
+		if stderrs[i].Len() > 0 {
+			finalErrors = append(finalErrors, stderrs[i].String())
+		}
+	}
 
 	if len(finalErrors) > 0 {
 		return errors.New(strings.Join(finalErrors, "\n"))