fix(#27): hand terminal foreground to interactive child (tcsetpgrp) — fixes TUI hang, preserves single-SIGINT#32
Merged
Conversation
Interactive full-screen TUIs (Nx, turbo, vitest --ui) hung unkillably under `nub run <script>` and `nub <file>`. Since #26 (f41f9a3) the child runs in its own process group (setpgid at exec) so a terminal Ctrl-C is delivered exactly once, but that group was never made the controlling terminal's foreground group — so the TUI's raw-mode terminal read raised SIGTTIN and the process stopped, and Ctrl-C went to nub rather than the TUI. No tcsetpgrp existed anywhere. Fix (all #[cfg(unix)]; Windows path unchanged): after spawning the interactive child (stdin is a TTY + inherited stdio), `foreground_child` calls tcsetpgrp(STDIN, child_pgid) to hand the terminal foreground to the child's group, bracketed by a SIGTTOU ignore. A RAII ForegroundGuard restores nub's own group as foreground on child exit. While the child owns the terminal, nub suppresses its now-redundant SIGINT forward (the kernel delivers Ctrl-C straight to the foreground child — preserving #26's exactly-once); every other signal still forwards, and the non-TTY `kill -INT <nub>` path never suppresses, so it still relays. This re-converges with how a shell runs a foreground job. Wired into `status_forwarding_signals` (single-package `nub run`) and `spawn_node` (`nub <file>`); the streamed/`-r` path pipes stdio and is untouched. Off a TTY `foreground_child` is a no-op (prior behavior). Validation: - New in-process PTY repro test: without the fix the own-group TUI child SIGTTIN-stops (the #27 hang); with it the child reads the terminal and exits cleanly. Runs the worker as a fresh re-exec'd process (not a fork of the multithreaded test harness). - #26 single-SIGINT regression test still green (delivered exactly once, child exits 130). - Non-TTY no-op test: off a TTY no tcsetpgrp is attempted and SIGINT forwarding is not suppressed. - Full nub-core suite green (276); clippy + fmt clean. - Manually verified on macOS via a real pty (script -q): a raw-mode node program reads stdin under `nub run` and exits cleanly, no hang. Fixes #27 Claude-Session: https://claude.ai/code/session_01YRvztkcr4fzfg9rD5edUwa
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Fixes the Unix job-control regression where interactive full-screen TUIs (e.g. Nx/Turbo) hang under nub run <script> / nub <file> by handing the controlling terminal’s foreground process group to the interactive child, and restoring it on exit.
Changes:
- Add a Unix-only
tcsetpgrpforeground handoff (foreground_child+ForegroundGuard) after spawning interactive children so TUIs can read from the terminal and receive Ctrl-C directly. - Adjust SIGINT-forwarding logic to avoid double-delivery when foreground is handed off (with new tests covering the PTY repro + non-TTY behavior).
- Add a PTY-based regression test that re-execs the test binary to safely reproduce SIGTTIN stop behavior and validate the fix.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+106
to
+110
| // While the child owns the terminal foreground, the kernel | ||
| // already delivered a TTY Ctrl-C to it directly — so a | ||
| // SIGINT forward here would double it (issue #26). Suppress | ||
| // SIGINT only; every other signal still forwards. | ||
| if signo == SIGINT && SUPPRESS_SIGINT_FORWARD.load(Ordering::SeqCst) { |
Comment on lines
+187
to
+193
| // nub's own (current) foreground group, to restore on the child's exit. If we | ||
| // can't read it, don't attempt the handoff — we'd have nothing to restore to. | ||
| // SAFETY: tcgetpgrp on a TTY fd; returns -1 on error, which we treat as opt-out. | ||
| let nub_pgrp = unsafe { libc::tcgetpgrp(libc::STDIN_FILENO) }; | ||
| if nub_pgrp < 0 { | ||
| return None; | ||
| } |
Comment on lines
+175
to
+178
| /// Only meaningful when stdin is a real TTY and stdio is inherited — callers gate | ||
| /// on `foreground_handoff_applicable`. Returns `None` (no-op) off a TTY, off Unix, | ||
| /// or if the handoff syscalls fail, in which case behavior is exactly the prior | ||
| /// own-group-without-tcsetpgrp path (correct for non-interactive children). |
| /// own-group-without-tcsetpgrp path (correct for non-interactive children). | ||
| #[cfg(unix)] | ||
| #[must_use] | ||
| pub fn foreground_child(child_pid: u32) -> Option<ForegroundGuard> { |
| /// re-enables SIGINT forwarding. Pair with [`foreground_child`]; must outlive the | ||
| /// child's `wait()`. | ||
| #[cfg(unix)] | ||
| pub struct ForegroundGuard { |
Comment on lines
+2109
to
+2120
| // New session → no controlling terminal, then claim the slave as ours. | ||
| // SAFETY: setsid()/ioctl(TIOCSCTTY)/dup2 on the fresh session leader (us). | ||
| unsafe { | ||
| if libc::setsid() < 0 { | ||
| return 2; | ||
| } | ||
| libc::ioctl(slave, libc::TIOCSCTTY as libc::c_ulong, 0); | ||
| libc::dup2(slave, libc::STDIN_FILENO); | ||
| if slave > 2 { | ||
| libc::close(slave); | ||
| } | ||
| } |
This was referenced Jun 21, 2026
colinhacks
added a commit
that referenced
this pull request
Jun 23, 2026
#95) The #27 fix (PR #32) hands the controlling terminal's foreground group to an interactive child via tcsetpgrp and suppresses nub's now-redundant SIGINT forward while the child owns the terminal — to preserve #26's exactly-once SIGINT delivery. That exactly-once guarantee UNDER the hand-off was verified only by reasoning: no test sent a terminal Ctrl-C while the hand-off was active and counted deliveries. The existing #26 test forwards a `kill` directly (never exercising the real TTY routing or the suppress), and the #27 pty test only proves the no-SIGTTIN-stop half. Add a pty test that reproduces nub's full interactive topology on a real controlling terminal: own-group child with a SIGINT trap that counts EVERY delivery (does not exit on the first, so a double is still counted), nub's signal forwarder registered, and `foreground_child` applied. It writes a literal ^C (0x03) to the pty master so the line discipline raises SIGINT for the terminal's foreground group (the child), then asserts the child saw SIGINT exactly once. A negative control (injecting a redundant forward) was confirmed to make the test report a double and fail, so the assertion is load-bearing. Refs #26, #27. Claude-Session: https://claude.ai/code/session_01YRvztkcr4fzfg9rD5edUwa
colinhacks
added a commit
that referenced
this pull request
Jun 23, 2026
#95) The #27 fix (PR #32) hands the controlling terminal's foreground group to an interactive child via tcsetpgrp and suppresses nub's now-redundant SIGINT forward while the child owns the terminal — to preserve #26's exactly-once SIGINT delivery. That exactly-once guarantee UNDER the hand-off was verified only by reasoning: no test sent a terminal Ctrl-C while the hand-off was active and counted deliveries. The existing #26 test forwards a `kill` directly (never exercising the real TTY routing or the suppress), and the #27 pty test only proves the no-SIGTTIN-stop half. Add a pty test that reproduces nub's full interactive topology on a real controlling terminal: own-group child with a SIGINT trap that counts EVERY delivery (does not exit on the first, so a double is still counted), nub's signal forwarder registered, and `foreground_child` applied. It writes a literal ^C (0x03) to the pty master so the line discipline raises SIGINT for the terminal's foreground group (the child), then asserts the child saw SIGINT exactly once. A negative control (injecting a redundant forward) was confirmed to make the test report a double and fail, so the assertion is load-bearing. Refs #26, #27.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem (#27)
Interactive full-screen TUIs hang unkillably under
nub run <script>andnub <file>— reported with Nx (NX_TUI) and corroborated withturbo dev. Ctrl-C does nothing; the user mustCtrl-Z+kill -9.NX_TUI=false(no TUI) works fine.Root cause
Since #26 (
f41f9a3, v0.1.6) the script/file child runs in its own process group (setpgid(0,0)at exec) so a terminal Ctrl-C is delivered exactly once. But that group was never made the controlling terminal's foreground group — there is notcsetpgrpanywhere. So an own-group child is a background group w.r.t. the TTY: a full-screen TUI's raw-mode terminal read raises SIGTTIN and the process stops (the hang), and keyboard input / Ctrl-C is delivered by the kernel to the foreground group (nub), never the TUI. Non-TUI programs don't read the terminal, so background-group placement was harmless — exactly the reported asymmetry.Fix
All
#[cfg(unix)]; the Windows path is untouched.foreground_child(child_pid) -> Option<ForegroundGuard>: on the interactive path (stdin is a TTY + inherited stdio), after spawn,tcsetpgrp(STDIN, child_pgid)hands the terminal foreground to the child's group (the child is its own group leader, sopgid == pid), bracketed by a localSIGTTOUignore. A RAIIForegroundGuardrestores nub's own group as foreground on child exit.SUPPRESS_SIGINT_FORWARDatomic); SIGTERM/SIGHUP/USR1/USR2/QUIT still forward. The non-TTYkill -INT <nub>path never suppresses, so it still relays — both SIGINT emitted twice on Ctrl+C #26 cases stay correct.status_forwarding_signals(single-packagenub run) andspawn_node(nub <file>). The streamed/-rpath (spawn_script_prefixed) pipes stdio → no TTY handoff, untouched. Off a TTYforeground_childis a no-op (exact prior behavior).This re-converges with how a shell runs a foreground job — what npm/pnpm rely on (they don't
setpgidat all).Validation
interactive_tui_child_can_read_terminal_only_with_foreground_handoff): drives the real syscall topology in an isolated PTY session (setsid+openpty+TIOCSCTTY, spawns an own-groupsh -c 'read x'that blocks on the terminal). The worker runs as a fresh re-exec'd process (not a fork of the multithreaded test harness). Asserts: without the fix the child SIGTTIN-stops (the Does not work with Nx TUI #27 hang); with it the child reads the keystroke and exits cleanly.sigint_reaches_an_own_group_child_exactly_oncestill green — SIGINT fires exactly once, child exits 130.foreground_child_is_noop_off_a_tty— off a TTY notcsetpgrpis attempted and SIGINT forwarding is not suppressed.cargo test -p nub-core→ 276 passed; clippy + fmt clean; full workspace build green.nub rununder a real pty (script -q) ran asetRawMode(true)node program reading stdin — printed READY, received input, exited 0, no hang.cfg(unix)paths). Windows unaffected (cfg-gated).Fixes #27
https://claude.ai/code/session_01YRvztkcr4fzfg9rD5edUwa