From 254ebbd4df2607e698328ec8e50729595f4fff56 Mon Sep 17 00:00:00 2001 From: Mike Landau Date: Thu, 29 Oct 2026 15:53:52 -0700 Subject: [PATCH] fix(devbox): wait for the command to exit when devbox run is interrupted devbox never handled SIGINT, so Ctrl-C killed it right away while the command it started (which gets the same SIGINT from the terminal) was still shutting down. The shell prompt came back early and the command's shutdown output landed after it. RunScript now catches SIGINT and SIGQUIT while the command runs. If devbox is in the terminal's foreground process group, the command got the signal too, so devbox just waits for it. Otherwise (e.g. kill -INT without a terminal) it falls back to the default action and exits as before. Signals that were already ignored are left alone so the command still inherits the ignore. Since devbox now outlives an interrupted command, report signal deaths the way a shell does (128+N, so 130 for Ctrl-C instead of 255) and skip the "Error: ... signal: interrupt" message. Fixes #3026 Co-Authored-By: Claude Opus 5.5 (1M context) --- go.mod | 4 +- internal/boxcli/midcobra/debug.go | 6 ++ internal/boxcli/usererr/exiterr.go | 17 ++++ internal/boxcli/usererr/exiterr_test.go | 37 +++++++++ internal/nix/run.go | 69 +++++++++++++++- internal/nix/run_test.go | 105 ++++++++++++++++++++++++ 6 files changed, 235 insertions(+), 3 deletions(-) create mode 100644 internal/boxcli/usererr/exiterr_test.go create mode 100644 internal/nix/run_test.go diff --git a/go.mod b/go.mod index 31feb127f49..035214dd0bf 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/Masterminds/semver/v3 v3.5.0 github.com/bmatcuk/doublestar/v4 v4.10.3 github.com/briandowns/spinner v1.23.2 + github.com/creack/pty v1.1.24 github.com/denisbrodbeck/machineid v1.0.1 github.com/f1bonacc1/process-compose v1.122.0 github.com/fatih/color v1.19.0 @@ -34,6 +35,7 @@ require ( golang.org/x/exp v0.0.0-20261014143641-f0251eb4aa3e golang.org/x/mod v0.42.0 golang.org/x/sync v0.24.0 + golang.org/x/sys v0.49.0 golang.org/x/tools v0.52.0 gopkg.in/yaml.v3 v3.0.1 ) @@ -101,7 +103,6 @@ require ( github.com/clipperhouse/uax29/v2 v2.7.0 // indirect github.com/codeclysm/extract/v4 v4.0.0 // indirect github.com/cpuguy83/go-md2man/v2 v2.0.7 // indirect - github.com/creack/pty v1.1.24 // indirect github.com/curioswitch/go-reassign v0.3.0 // indirect github.com/daixiang0/gci v0.13.7 // indirect github.com/dave/dst v0.28.0 // indirect @@ -274,7 +275,6 @@ require ( go4.org v0.0.0-20230225012048-214862532bf5 // indirect golang.org/x/exp/typeparams v0.0.0-20260811152304-ee035b5b010f // indirect golang.org/x/oauth2 v0.33.0 // indirect - golang.org/x/sys v0.49.0 // indirect golang.org/x/term v0.47.0 // indirect golang.org/x/text v0.43.0 // indirect google.golang.org/protobuf v1.36.12 // indirect diff --git a/internal/boxcli/midcobra/debug.go b/internal/boxcli/midcobra/debug.go index c6c9b7ca98b..673349366ee 100644 --- a/internal/boxcli/midcobra/debug.go +++ b/internal/boxcli/midcobra/debug.go @@ -52,6 +52,12 @@ func (d *DebugMiddleware) postRun(cmd *cobra.Command, args []string, runErr erro if runErr == nil { return } + // The user pressed Ctrl-C and the command has exited. The exit code (130) + // is all there is to report. + var execErr *usererr.ExitError + if errors.As(runErr, &execErr) && execErr.Interrupted() { + return + } if userErr, hasUserErr := usererr.Extract(runErr); hasUserErr { if usererr.IsWarning(userErr) { ux.Fwarning(cmd.ErrOrStderr(), runErr.Error()) diff --git a/internal/boxcli/usererr/exiterr.go b/internal/boxcli/usererr/exiterr.go index 2f7b0d25363..c7edaffbc9c 100644 --- a/internal/boxcli/usererr/exiterr.go +++ b/internal/boxcli/usererr/exiterr.go @@ -6,6 +6,7 @@ package usererr import ( "errors" "os/exec" + "syscall" ) // ExitError is an ExitError for a command run on behalf of a user @@ -27,3 +28,19 @@ func NewExecError(source error) error { } return &ExitError{ExitError: exitErr} } + +// ExitCode returns the command's exit code or, if a signal killed it, 128 plus +// the signal number, the same as a shell reports it (e.g. 130 for Ctrl-C). +func (e *ExitError) ExitCode() int { + if status, ok := e.Sys().(syscall.WaitStatus); ok && status.Signaled() { + return 128 + int(status.Signal()) + } + return e.ExitError.ExitCode() +} + +// Interrupted reports whether the command was killed by SIGINT, which usually +// means the user pressed Ctrl-C. +func (e *ExitError) Interrupted() bool { + status, ok := e.Sys().(syscall.WaitStatus) + return ok && status.Signaled() && status.Signal() == syscall.SIGINT +} diff --git a/internal/boxcli/usererr/exiterr_test.go b/internal/boxcli/usererr/exiterr_test.go new file mode 100644 index 00000000000..0a475d79f8c --- /dev/null +++ b/internal/boxcli/usererr/exiterr_test.go @@ -0,0 +1,37 @@ +// Copyright 2024 Jetify Inc. and contributors. All rights reserved. +// Use of this source code is governed by the license in the LICENSE file. + +package usererr + +import ( + "errors" + "os/exec" + "testing" +) + +func TestExitErrorExitCode(t *testing.T) { + tests := map[string]struct { + script string + wantCode int + wantInterrupted bool + }{ + "exit status": {script: "exit 3", wantCode: 3}, + "killed by SIGINT": {script: "kill -INT $$", wantCode: 130, wantInterrupted: true}, + "killed by SIGTERM": {script: "kill -TERM $$", wantCode: 143}, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + err := NewExecError(exec.Command("sh", "-c", test.script).Run()) + var exitErr *ExitError + if !errors.As(err, &exitErr) { + t.Fatalf("got error %v (%T), want *ExitError", err, err) + } + if got := exitErr.ExitCode(); got != test.wantCode { + t.Errorf("got exit code %d, want %d", got, test.wantCode) + } + if got := exitErr.Interrupted(); got != test.wantInterrupted { + t.Errorf("got Interrupted() = %t, want %t", got, test.wantInterrupted) + } + }) + } +} diff --git a/internal/nix/run.go b/internal/nix/run.go index 2625cddc6f1..4bde070e52e 100644 --- a/internal/nix/run.go +++ b/internal/nix/run.go @@ -9,6 +9,10 @@ import ( "log/slog" "os" "os/exec" + "os/signal" + "syscall" + + "golang.org/x/sys/unix" "go.jetify.com/devbox/internal/boxcli/usererr" "go.jetify.com/devbox/internal/cmdutil" @@ -35,5 +39,68 @@ func RunScript(projectDir, cmdWithArgs string, env map[string]string) error { slog.Debug("executing script", "cmd", cmd.Args) // Report error as exec error when executing scripts. - return usererr.NewExecError(cmd.Run()) + return usererr.NewExecError(runWaitingOnInterrupt(cmd)) +} + +// runWaitingOnInterrupt runs cmd. If the user presses Ctrl-C (or Ctrl-\), +// devbox stays alive until cmd exits, so that the shell prompt only comes back +// once the command has finished shutting down, the same as running the command +// directly or with `npm run`. +func runWaitingOnInterrupt(cmd *exec.Cmd) error { + // Subscribe before starting the command so that devbox can't be killed + // between starting it and waiting for it. Leave ignored signals alone: + // Notify would stop ignoring them, in devbox and in the command (e.g. in a + // background job of a non-interactive shell). + signals := make(chan os.Signal, 1) + for _, sig := range []os.Signal{syscall.SIGINT, syscall.SIGQUIT} { + if !signal.Ignored(sig) { + signal.Notify(signals, sig) + } + } + defer signal.Stop(signals) + + if err := cmd.Start(); err != nil { + return err + } + + done := make(chan struct{}) + defer close(done) + go func() { + for { + select { + case sig := <-signals: + handleInterrupt(sig) + case <-done: + return + } + } + }() + return cmd.Wait() +} + +func handleInterrupt(sig os.Signal) { + // The terminal sends Ctrl-C and Ctrl-\ to its whole foreground process + // group, which includes the command, so it's enough to wait for it. + if inForegroundProcessGroup() { + slog.Debug("waiting for command to exit after interrupt", "signal", sig) + return + } + // The signal came from somewhere else (e.g. kill -INT ), and + // the command didn't get it. Exit the way devbox did before it handled + // interrupts. + signal.Reset(sig) + _ = syscall.Kill(os.Getpid(), sig.(syscall.Signal)) +} + +// inForegroundProcessGroup reports whether devbox (and so the command it +// started, which shares its process group) is in the foreground process group +// of its controlling terminal. +func inForegroundProcessGroup() bool { + tty, err := os.Open("/dev/tty") + if err != nil { + return false // no controlling terminal + } + defer tty.Close() + pgrp, err := unix.IoctlGetInt(int(tty.Fd()), unix.TIOCGPGRP) + return err == nil && pgrp == unix.Getpgrp() } diff --git a/internal/nix/run_test.go b/internal/nix/run_test.go new file mode 100644 index 00000000000..979a96c7699 --- /dev/null +++ b/internal/nix/run_test.go @@ -0,0 +1,105 @@ +// Copyright 2024 Jetify Inc. and contributors. All rights reserved. +// Use of this source code is governed by the license in the LICENSE file. + +package nix + +import ( + "bytes" + "errors" + "fmt" + "io" + "os" + "os/exec" + "strings" + "sync" + "testing" + "time" + + "github.com/creack/pty" + + "go.jetify.com/devbox/internal/boxcli/usererr" +) + +const runScriptHelperEnv = "DEVBOX_TEST_RUN_SCRIPT_HELPER" + +// TestRunScriptHelper plays the part of `devbox run` for +// TestRunScriptWaitsForCommandOnCtrlC, which runs it in a terminal. +func TestRunScriptHelper(t *testing.T) { + if os.Getenv(runScriptHelperEnv) == "" { + t.Skip("helper process for TestRunScriptWaitsForCommandOnCtrlC") + } + script := `trap 'echo command: shutting down; sleep 0.5; echo command: shutdown complete; exit 3' INT +echo command: started +while true; do sleep 0.1; done` + err := RunScript(t.TempDir(), script, map[string]string{"PATH": os.Getenv("PATH")}) + var exitErr *usererr.ExitError + if errors.As(err, &exitErr) { + fmt.Printf("RunScript returned exit code %d\n", exitErr.ExitCode()) + } else { + fmt.Printf("RunScript returned %v\n", err) + } + os.Exit(0) +} + +// TestRunScriptWaitsForCommandOnCtrlC checks that pressing Ctrl-C doesn't kill +// devbox while the command is still shutting down (#3026). +func TestRunScriptWaitsForCommandOnCtrlC(t *testing.T) { + cmd := exec.Command(os.Args[0], "-test.run=^TestRunScriptHelper$") + cmd.Env = append(os.Environ(), runScriptHelperEnv+"=1") + terminal, err := pty.Start(cmd) // the helper's controlling terminal + if err != nil { + t.Fatal(err) + } + defer terminal.Close() + + var outMu sync.Mutex + var out bytes.Buffer + go func() { + buf := make([]byte, 1024) + for { + n, err := terminal.Read(buf) + outMu.Lock() + out.Write(buf[:n]) + outMu.Unlock() + if err != nil { + return + } + } + }() + output := func() string { + outMu.Lock() + defer outMu.Unlock() + return out.String() + } + + deadline := time.Now().Add(10 * time.Second) + for !strings.Contains(output(), "command: started") { + if time.Now().After(deadline) { + t.Fatalf("timed out waiting for the command to start, output:\n%s", output()) + } + time.Sleep(10 * time.Millisecond) + } + if _, err := io.WriteString(terminal, "\x03"); err != nil { // Ctrl-C + t.Fatal(err) + } + + waitErr := make(chan error, 1) + go func() { waitErr <- cmd.Wait() }() + select { + case err := <-waitErr: + if err != nil { + t.Fatalf("devbox didn't exit cleanly: %v, output:\n%s", err, output()) + } + case <-time.After(10 * time.Second): + t.Fatalf("timed out waiting for devbox to exit, output:\n%s", output()) + } + + // Give the reader a moment to drain what's left in the terminal. + time.Sleep(100 * time.Millisecond) + got := output() + complete := strings.Index(got, "command: shutdown complete") + returned := strings.Index(got, "RunScript returned exit code 3") + if complete == -1 || returned == -1 || returned < complete { + t.Errorf("RunScript should return the command's exit code after it shuts down, output:\n%s", got) + } +}