[mikeland73]

Summary

Fixes #3026. Pressing Ctrl-C during devbox run brought the shell prompt back right away, while the command was still shutting down and printing output. Devbox has never handled SIGINT (nothing in the repo imported os/signal), so this isn't a regression. The terminal sends SIGINT to the whole foreground process group, so devbox died immediately while the command handled the signal on its own schedule.

Changes:

• nix.RunScript catches SIGINT and SIGQUIT while the command runs.
  • If devbox is in the terminal's foreground process group (tcgetpgrp(/dev/tty) == getpgrp()), the command already got the signal from the terminal, so devbox waits for it to exit. It deliberately does not forward the signal: a second SIGINT makes servers like uvicorn/FastAPI force-quit.
  • Otherwise (e.g. kill -INT <devbox pid> with no terminal), devbox resets the handler and re-raises the signal, so it exits exactly as before.
  • Signals that were already ignored at startup are left alone. signal.Notify would otherwise un-ignore them for devbox and for the command (e.g. devbox run x & in a script).
• usererr.ExitError.ExitCode reports a signal death as 128+N, the way a shell does. Now that devbox outlives an interrupted command, it would otherwise exit 255 instead of the 130 it gets today.
• midcobra debug middleware no longer prints Error: error running script "x" in Devbox: signal: interrupt after Ctrl-C. Devbox used to die before it could print this, so output stays as it was.

Out of scope: devbox still doesn't forward SIGTERM (e.g. docker stop). Doing that properly means signalling the command's process group, since real scripts run as sh -c script.sh, and that has its own trade-offs.

How was it tested?

• Added TestRunScriptWaitsForCommandOnCtrlC: the test binary re-runs itself as a helper under a pty (via creack/pty, already in the module graph) and runs a script that traps INT and takes 0.5s to shut down. The test sends ^C and checks that RunScript returns only after the script finishes, with its exit code. It fails on main (signal: interrupt) and passes here, including with -race.
• Added TestExitErrorExitCode (exit 3 → 3, SIGINT → 130 + Interrupted(), SIGTERM → 143).
• Manual comparison under a pty, 0.19.0 vs this branch, with a script that traps INT and sleeps 1s before exiting:
  • 0.19.0: ^C server: got INT, then PROMPT BACK, then server: shutdown complete.
  • This branch: ^C server: got INT, then server: shutdown complete, then PROMPT BACK (devbox exit 0).
  • A plain sleep 30 script exits 130 with no error message.
• With no terminal, kill -INT <devbox pid> still kills devbox immediately, the same as 0.19.0. With SIGINT ignored by the parent (trap '' INT), the command still ignores it, the same as 0.19.0.
• go test -race ./internal/nix ./internal/boxcli/... ./internal/devbox passes, as do the run-related testscripts. Lint is clean. go mod vendor output is unchanged (only the x/sys and creack/pty indirect markers moved), so vendor-hash doesn't need updating.

Community Contribution License

All community contributions in this pull request are licensed to the project
maintainers under the terms of the
Apache 2 License.

By creating this pull request, I represent that I have the right to license the
contributions to the project maintainers under the Apache 2 License as stated in
the
Community Contribution License.

🤖 Generated with Claude Code

[claude]

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

[kadaan]

There is an old issue I filed that this seems related to: #1815
Would be nice to know if this fixes that too.

[mikeland73]

@kadaan I think it does, but some reports in that thread are hard to repo, so I don't know if it fixes everything. I'm inclined to close both issues once this is merged

[mikeland73]

@claude ptal

[claude]

Claude finished @mikeland73's task in 11s —— View job

---

I'm not sure what "patal" means. It may be a typo, or the rest of the message may have been cut off. I haven't changed anything on the branch.

What would you like me to do on this PR? For example:

• Review the changes in internal/nix/run.go, usererr/exiterr.go and the tests.
• Look into a specific problem, such as a failing CI job.
• Make a particular code change.

Reply with @claude and the request, and I'll take it from there.
· branch mikeland73/fix-run-signal-wait

[claude]

Claude finished @mikeland73's task in 38s —— View job

---

Review of #3033

I read the full diff. I did not run the build or tests in this session. Nothing blocks the merge, and I have a few small notes.

What looks right

• Signal handling: Subscribing to signals before cmd.Start() closes the window where devbox could be killed before it starts waiting. The size-1 buffered channel is enough, because the goroutine only needs to know that a signal arrived. The defer order (close(done), then signal.Stop) is safe.
• Ignored signals: Skipping signals that were ignored at startup (signal.Ignored) avoids the surprising un-ignore for background jobs.
• Not forwarding: Not forwarding SIGINT is the right call. The terminal has already delivered it to the whole foreground process group, and a second SIGINT would force-quit servers like uvicorn.
• Non-terminal case: signal.Reset followed by syscall.Kill(self, sig) keeps the old behavior for kill -INT <pid>.
• Exit codes: ExitError.ExitCode() returning 128+N keeps the exit code at 130 now that devbox outlives the command. The midcobra change keeps the output as it was.
• Test: The pty-based test is a good regression test. The test binary re-runs itself as the helper, and pty.Start gives it a controlling terminal where it is the foreground process group. The go.mod change is just moving creack/pty and x/sys to direct dependencies.

Notes (non-blocking)

1. Platforms: run.go now uses syscall.Kill and unix.* without build tags, so internal/nix stops compiling on Windows. .goreleaser.yaml only builds linux and darwin, so this is fine today. If Windows support is ever wanted, split this into run_unix.go and run_windows.go.
2. Commands that leave the process group: inForegroundProcessGroup() being true means devbox assumes the command got the signal. A command that moves itself into its own process group (a job-control shell, setsid, a process manager) won't get the Ctrl-C. Devbox will then wait until the command exits, and Ctrl-C will appear to do nothing. A second Ctrl-C is swallowed the same way. This matches how sh and npm run behave, so it is acceptable, but a one-line comment on handleInterrupt would help.
3. Commands that ignore SIGINT: Previously Ctrl-C killed devbox and left the command orphaned. Now devbox blocks until the command exits, which is the shell-like behavior you want. Users can still use kill -9 or Ctrl-. Ctrl-\ is also caught and waited on, so it is not an escape hatch for a stuck command.
4. Interrupted(): It only matches SIGINT. A command killed by Ctrl-\ (SIGQUIT) still prints the Error: ... signal: quit debug line. This is probably fine, since a quit usually warrants the message.
5. Test timing: The test's 100ms drain sleep and the 0.5s shutdown are fine. If this ever flakes on slow CI, lengthening the drain is the first thing to try.

Overall this looks good to merge.