From 4f1fd82ed722bec8228687b91ab4d065747c10eb Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Wed, 28 Jan 2026 21:56:04 +0000 Subject: [PATCH] fix: propagate correct agent exit code (#21718) The reaper (PID 1) now returns the child's exit code instead of always exiting 0. Signal termination uses the standard Unix convention of 128 + signal number. fixes #21661 --- agent/reaper/reaper_stub.go | 4 ++-- agent/reaper/reaper_test.go | 39 +++++++++++++++++++++++++++++++++++-- agent/reaper/reaper_unix.go | 24 +++++++++++++++++++---- cli/agent.go | 6 +++--- 4 files changed, 62 insertions(+), 11 deletions(-) diff --git a/agent/reaper/reaper_stub.go b/agent/reaper/reaper_stub.go index 8cd87ab0bf..da4d871fc5 100644 --- a/agent/reaper/reaper_stub.go +++ b/agent/reaper/reaper_stub.go @@ -7,6 +7,6 @@ func IsInitProcess() bool { return false } -func ForkReap(_ ...Option) error { - return nil +func ForkReap(_ ...Option) (int, error) { + return 0, nil } diff --git a/agent/reaper/reaper_test.go b/agent/reaper/reaper_test.go index 84246fba06..7ef3f0a50b 100644 --- a/agent/reaper/reaper_test.go +++ b/agent/reaper/reaper_test.go @@ -32,12 +32,13 @@ func TestReap(t *testing.T) { } pids := make(reap.PidCh, 1) - err := reaper.ForkReap( + exitCode, err := reaper.ForkReap( reaper.WithPIDCallback(pids), // Provide some argument that immediately exits. reaper.WithExecArgs("/bin/sh", "-c", "exit 0"), ) require.NoError(t, err) + require.Equal(t, 0, exitCode) cmd := exec.Command("tail", "-f", "/dev/null") err = cmd.Start() @@ -65,6 +66,36 @@ func TestReap(t *testing.T) { } } +//nolint:paralleltest +func TestForkReapExitCodes(t *testing.T) { + if testutil.InCI() { + t.Skip("Detected CI, skipping reaper tests") + } + + tests := []struct { + name string + command string + expectedCode int + }{ + {"exit 0", "exit 0", 0}, + {"exit 1", "exit 1", 1}, + {"exit 42", "exit 42", 42}, + {"exit 255", "exit 255", 255}, + {"SIGKILL", "kill -9 $$", 128 + 9}, + {"SIGTERM", "kill -15 $$", 128 + 15}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + exitCode, err := reaper.ForkReap( + reaper.WithExecArgs("/bin/sh", "-c", tt.command), + ) + require.NoError(t, err) + require.Equal(t, tt.expectedCode, exitCode, "exit code mismatch for %q", tt.command) + }) + } +} + //nolint:paralleltest // Signal handling. func TestReapInterrupt(t *testing.T) { // Don't run the reaper test in CI. It does weird @@ -84,13 +115,17 @@ func TestReapInterrupt(t *testing.T) { defer signal.Stop(usrSig) go func() { - errC <- reaper.ForkReap( + exitCode, err := reaper.ForkReap( reaper.WithPIDCallback(pids), reaper.WithCatchSignals(os.Interrupt), // Signal propagation does not extend to children of children, so // we create a little bash script to ensure sleep is interrupted. reaper.WithExecArgs("/bin/sh", "-c", fmt.Sprintf("pid=0; trap 'kill -USR2 %d; kill -TERM $pid' INT; sleep 10 &\npid=$!; kill -USR1 %d; wait", os.Getpid(), os.Getpid())), ) + // The child exits with 128 + SIGTERM (15) = 143, but the trap catches + // SIGINT and sends SIGTERM to the sleep process, so exit code varies. + _ = exitCode + errC <- err }() require.Equal(t, <-usrSig, syscall.SIGUSR1) diff --git a/agent/reaper/reaper_unix.go b/agent/reaper/reaper_unix.go index 35ce9bfaa1..255077284c 100644 --- a/agent/reaper/reaper_unix.go +++ b/agent/reaper/reaper_unix.go @@ -40,7 +40,10 @@ func catchSignals(pid int, sigs []os.Signal) { // the reaper and an exec.Command waiting for its process to complete. // The provided 'pids' channel may be nil if the caller does not care about the // reaped children PIDs. -func ForkReap(opt ...Option) error { +// +// Returns the child's exit code (using 128+signal for signal termination) +// and any error from Wait4. +func ForkReap(opt ...Option) (int, error) { opts := &options{ ExecArgs: os.Args, } @@ -53,7 +56,7 @@ func ForkReap(opt ...Option) error { pwd, err := os.Getwd() if err != nil { - return xerrors.Errorf("get wd: %w", err) + return 1, xerrors.Errorf("get wd: %w", err) } pattrs := &syscall.ProcAttr{ @@ -72,7 +75,7 @@ func ForkReap(opt ...Option) error { //#nosec G204 pid, err := syscall.ForkExec(opts.ExecArgs[0], opts.ExecArgs, pattrs) if err != nil { - return xerrors.Errorf("fork exec: %w", err) + return 1, xerrors.Errorf("fork exec: %w", err) } go catchSignals(pid, opts.CatchSignals) @@ -82,5 +85,18 @@ func ForkReap(opt ...Option) error { for xerrors.Is(err, syscall.EINTR) { _, err = syscall.Wait4(pid, &wstatus, 0, nil) } - return err + + // Convert wait status to exit code using standard Unix conventions: + // - Normal exit: use the exit code + // - Signal termination: use 128 + signal number + var exitCode int + switch { + case wstatus.Exited(): + exitCode = wstatus.ExitStatus() + case wstatus.Signaled(): + exitCode = 128 + int(wstatus.Signal()) + default: + exitCode = 1 + } + return exitCode, err } diff --git a/cli/agent.go b/cli/agent.go index 7788a5fcca..58efeb0c18 100644 --- a/cli/agent.go +++ b/cli/agent.go @@ -136,7 +136,7 @@ func workspaceAgent() *serpent.Command { // to do this else we fork bomb ourselves. //nolint:gocritic args := append(os.Args, "--no-reap") - err := reaper.ForkReap( + exitCode, err := reaper.ForkReap( reaper.WithExecArgs(args...), reaper.WithCatchSignals(StopSignals...), ) @@ -145,8 +145,8 @@ func workspaceAgent() *serpent.Command { return xerrors.Errorf("fork reap: %w", err) } - logger.Info(ctx, "reaper process exiting") - return nil + logger.Info(ctx, "reaper child process exited", slog.F("exit_code", exitCode)) + return ExitError(exitCode, nil) } // Handle interrupt signals to allow for graceful shutdown,