fix(pty): ctrl-z hangs when running with a PTY by caiocdcs · Pull Request #1135 · nolabs-ai/nono

caiocdcs · 2026-06-12T16:51:46Z

Linked Issue

Closes #989

Summary

Ctrl-Z froze the terminal in PTY-backed sessions because setsid() in setup_child_pty creates a new session for the sandboxed child, orphaning its process group. The kernel refuses to deliver SIGTSTP to orphaned PGs.

Fix: Intercept Ctrl-Z byte (0x1A) at the PtyProxy level in filter_client_input(). Send uncatchable SIGSTOP to the child (bash ignores SIGTSTP), restore terminal to cooked mode, then stop nono via raise(SIGTSTP). On fg: restore raw mode, forward SIGCONT + SIGWINCH for TUI redraw.

Changed:

crates/nono-cli/src/pty_proxy.rs (+20): Ctrl-Z detection, suspension flag, take_suspension_request(), pub(crate) restore_terminal + saved_termios
crates/nono-cli/src/exec_strategy.rs (+87): handle_pty_suspension() function, called from all three supervisor loops

Agent Disclosure

AI-assisted investigation, root cause analysis, and TUI redraw SIGWINCH fix. All code was pair-programmed with human direction and review.

Test Plan

make build, make clippy, make fmt-check -- clean
make test-lib -- 717 passed
Manual: nono run --allow /tmp -- bash -i -> Ctrl-Z suspends -> fg resumes (macOS, fish+bash)
Manual: tested with opencode, screen clears correctly after resume

Checklist

An issue exists and is linked above
All commits are signed-off, using DCO
All new code follows the project's coding standards (CLAUDE.md) and is covered by tests
Public-facing changes are paired with documentation updates
Release note has been added to CHANGELOG.md if needed

github-actions · 2026-06-12T16:52:00Z

PR Review Summary

Size

Metric	Value
Lines added	+289
Lines removed	-9
Total changed	298
Classification	Medium (50–300 lines)

Affected crates

crates/nono-cli — CLI changes. Verify argument parsing, flag documentation, and UX behaviour across supported platforms.

Blast radius — Contained

This PR touches: source code

_{Updated automatically on each push to this PR.}

gemini-code-assist

Code Review

This pull request implements manual Ctrl-Z suspension handling for the PTY proxy to address job control limitations in orphaned process groups. When Ctrl-Z is intercepted, the proxy stops the child process, restores the terminal, suspends itself, and properly resumes terminal states and child execution upon receiving SIGCONT. The review feedback highlights two critical issues: first, waitpid should explicitly handle EINTR to avoid premature exits and terminal hangs when interrupted by signals; second, SIGWINCH should be sent to the foreground process group of the PTY rather than the child shell process directly, ensuring nested TUI applications redraw correctly upon resumption.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

lukehinds · 2026-06-15T15:27:21Z

how are you getting on @caiocdcs ? is this still a draft pr?

caiocdcs · 2026-06-15T17:24:08Z

Yes, I'm still checking some gemini comments. I hope I have some time to do this week.

…#989) setsid() in setup_child_pty orphans the child process group, preventing kernel delivery of SIGTSTP. PtyProxy now intercepts Ctrl-Z (0x1A), sends uncatchable SIGSTOP to the child, restores terminal, then SIGTSTP-stops nono. On fg: raw mode restored, SIGCONT + SIGWINCH forwarded. Signed-off-by: Caio Silva <caio@cdcs.dev>

Signed-off-by: Caio Silva <caio@cdcs.dev>

TUI apps using the kitty keyboard protocol (opencode/opentui) send Ctrl-Z as a CSI-u escape (\x1b[122;5u or \x1b[90;5u) rather than raw 0x1A. Detect both for terminal clients by reusing the existing enhanced-key matcher (control_key_candidates(0x1a) is already [90, 122]); socket-client detach behavior is unchanged. Distinguish nested jobs from the orphaned direct child in handle_pty_suspension: a nested job (e.g. vim under bash -i) is not orphaned, so forward a plain SIGTSTP and let the inner shell drive suspend/resume; only the direct child gets the manual SIGSTOP + self-suspend dance. This fixes a hang where waitpid() blocked on a stopped process that was not nono's child. On resume, repaint the alternate screen from nono's captured vt100 state (mirroring the client re-attach replay) instead of emitting a bare alt-screen-enter and relying on SIGWINCH, which opencode ignores. Use the clearing terminal restore on suspend so repeated Ctrl-Z/fg cycles do not accumulate cursor drift on the normal screen. Signed-off-by: Caio Silva <caio@cdcs.dev>

* fix(pty): suspend Ctrl-Z manually in orphaned PTY sessions (nolabs-ai#989) setsid() in setup_child_pty orphans the child process group, preventing kernel delivery of SIGTSTP. PtyProxy now intercepts Ctrl-Z (0x1A), sends uncatchable SIGSTOP to the child, restores terminal, then SIGTSTP-stops nono. On fg: raw mode restored, SIGCONT + SIGWINCH forwarded. Signed-off-by: Caio Silva <caio@cdcs.dev> * fix(pty): retry suspension waitpid on EINTR Signed-off-by: Caio Silva <caio@cdcs.dev> * fix(pty): send SIGWINCH to the foreground process group on resume Signed-off-by: Caio Silva <caio@cdcs.dev> * refactor(pty): expose PTY master fd through a getter Signed-off-by: Caio Silva <caio@cdcs.dev> * fix(pty): detect CSI-u Ctrl-Z and handle nested jobs TUI apps using the kitty keyboard protocol (opencode/opentui) send Ctrl-Z as a CSI-u escape (\x1b[122;5u or \x1b[90;5u) rather than raw 0x1A. Detect both for terminal clients by reusing the existing enhanced-key matcher (control_key_candidates(0x1a) is already [90, 122]); socket-client detach behavior is unchanged. Distinguish nested jobs from the orphaned direct child in handle_pty_suspension: a nested job (e.g. vim under bash -i) is not orphaned, so forward a plain SIGTSTP and let the inner shell drive suspend/resume; only the direct child gets the manual SIGSTOP + self-suspend dance. This fixes a hang where waitpid() blocked on a stopped process that was not nono's child. On resume, repaint the alternate screen from nono's captured vt100 state (mirroring the client re-attach replay) instead of emitting a bare alt-screen-enter and relying on SIGWINCH, which opencode ignores. Use the clearing terminal restore on suspend so repeated Ctrl-Z/fg cycles do not accumulate cursor drift on the normal screen. Signed-off-by: Caio Silva <caio@cdcs.dev> --------- Signed-off-by: Caio Silva <caio@cdcs.dev> Co-authored-by: Luke Hinds <lukehinds@gmail.com> (cherry picked from commit 4179ce0) Signed-off-by: oscarmackjr-twg <oscar.mack.jr@gmail.com>

These pre-existed on the v3.1/v3.2 branch but were never caught because all milestone verification ran on a Windows host (cargo check never compiles the Unix cfg branches; tags were never pushed so CI never ran). All three are botched-cherry-pick / wrong-symbol artifacts — none weaken sandbox policy. - E0432 exec_strategy.rs: drop dead import of nonexistent `nono::SupervisorListener` (the import line was its only occurrence; unused). - E0609 exec_strategy.rs (1338 child, 1642 parent): `unix_socket_allowlist` lives on `SupervisorConfig`, not `ExecConfig` (CR-01 / 718fe59 read it on the wrong struct). Read the same grant slice via `config.caps.unix_socket_capabilities()` — byte-identical to how `supervised_runtime.rs:367` populates `SupervisorConfig.unix_socket_allowlist`, so the child/parent `af_unix_send_filter_action` predicate stays in lockstep. No grant added, removed, narrowed, or widened. - E0425 pty_proxy.rs: restore `fn drain_terminal_output` verbatim from upstream/main (isatty guard + tcdrain EINTR loop). Cherry-pick 1f4fd33 (nolabs-ai#1135 ctrl-z) brought the call sites but dropped the definition. Verification: CI (Test ubuntu-latest + macos-latest) is the sole authoritative verifier — this host lacks the cross C compiler for a local linux-gnu cargo check. Debug session: .planning/debug/ci-linux-cfg-compile-errors.md Signed-off-by: Oscar Mack Jr <oscar.mack.jr@gmail.com>

Unmasked after the previous compile-error fixes. Identical set on Test (ubuntu) and Test (macos) → every item is Windows-only-used or genuinely dead. Per-item triage (no blanket allow): Windows-only items get `#[cfg_attr(not(target_os = "windows"), allow(dead_code))]` (existing codebase pattern); provably-dead items removed. - cfg_attr/allow (used only under #[cfg(windows)] callers): classify_daemon_request, is_pipe_not_found (agent_cli); rollback_root (rollback_session); cert_trust non-Windows stubs; SubsystemState::Ok/Broken (health); EVENT_LOG_SOURCE/ EventLogLevel/build_event_log_message (telemetry/windows). - import cfg-gated to windows: RejectStage (audit_integrity). - imports dropped (unused on all platforms): UnixSocketCapability/UnixSocketMode (exec_strategy — field uses fully-qualified nono::path), nono::SandboxViolation (profile_save_runtime). - removed (zero callers any platform; nolabs-ai#1135 cherry-pick leftovers): in_alt_screen, shutdown_attach_listener (pty_proxy). - allow(dead_code) preserved: sandbox_violations/ignored_denial_paths (security-relevant violation tracking, exec_strategy is non-Windows-only). - AUDIT_LEDGER_FILENAME/maybe_migrate_legacy_audit_ledger: broadened to plain allow — their only caller module audit_ledger.rs is an orphan (no mod decl, uncompiled); pre-existing wiring gap, noted not fixed here. No runtime logic changed — only attributes, import cfg-gating, and removal of provably-dead items. Windows cargo check --workspace green. CI (Test ubuntu+macos) is the cross-target verifier. Signed-off-by: Oscar Mack Jr <oscar.mack.jr@gmail.com>

* fix(pty): suspend Ctrl-Z manually in orphaned PTY sessions (nolabs-ai#989) setsid() in setup_child_pty orphans the child process group, preventing kernel delivery of SIGTSTP. PtyProxy now intercepts Ctrl-Z (0x1A), sends uncatchable SIGSTOP to the child, restores terminal, then SIGTSTP-stops nono. On fg: raw mode restored, SIGCONT + SIGWINCH forwarded. Signed-off-by: Caio Silva <caio@cdcs.dev> * fix(pty): retry suspension waitpid on EINTR Signed-off-by: Caio Silva <caio@cdcs.dev> * fix(pty): send SIGWINCH to the foreground process group on resume Signed-off-by: Caio Silva <caio@cdcs.dev> * refactor(pty): expose PTY master fd through a getter Signed-off-by: Caio Silva <caio@cdcs.dev> * fix(pty): detect CSI-u Ctrl-Z and handle nested jobs TUI apps using the kitty keyboard protocol (opencode/opentui) send Ctrl-Z as a CSI-u escape (\x1b[122;5u or \x1b[90;5u) rather than raw 0x1A. Detect both for terminal clients by reusing the existing enhanced-key matcher (control_key_candidates(0x1a) is already [90, 122]); socket-client detach behavior is unchanged. Distinguish nested jobs from the orphaned direct child in handle_pty_suspension: a nested job (e.g. vim under bash -i) is not orphaned, so forward a plain SIGTSTP and let the inner shell drive suspend/resume; only the direct child gets the manual SIGSTOP + self-suspend dance. This fixes a hang where waitpid() blocked on a stopped process that was not nono's child. On resume, repaint the alternate screen from nono's captured vt100 state (mirroring the client re-attach replay) instead of emitting a bare alt-screen-enter and relying on SIGWINCH, which opencode ignores. Use the clearing terminal restore on suspend so repeated Ctrl-Z/fg cycles do not accumulate cursor drift on the normal screen. Signed-off-by: Caio Silva <caio@cdcs.dev> --------- Signed-off-by: Caio Silva <caio@cdcs.dev> Co-authored-by: Luke Hinds <lukehinds@gmail.com>

github-actions Bot added nono-cli size/medium labels Jun 12, 2026

gemini-code-assist Bot reviewed Jun 12, 2026

View reviewed changes

Comment thread crates/nono-cli/src/exec_strategy.rs

Comment thread crates/nono-cli/src/exec_strategy.rs Outdated

caiocdcs closed this Jun 12, 2026

caiocdcs deleted the 989-fix-ctrl-z-hang branch June 12, 2026 16:59

caiocdcs restored the 989-fix-ctrl-z-hang branch June 12, 2026 16:59

caiocdcs reopened this Jun 12, 2026

github-actions Bot added size/medium and removed size/medium labels Jun 12, 2026

caiocdcs changed the title ~~989: fix Ctrl-Z hang in PTY sessions~~ fix: Ctrl-Z hangs when running with a PTY Jun 12, 2026

github-actions Bot added size/medium bug Something isn't working labels Jun 12, 2026

caiocdcs changed the title ~~fix: Ctrl-Z hangs when running with a PTY~~ fix(pty): Ctrl-Z hangs when running with a PTY Jun 12, 2026

github-actions Bot added size/medium and removed size/medium labels Jun 12, 2026

caiocdcs marked this pull request as draft June 14, 2026 18:08

lukehinds changed the title ~~fix(pty): Ctrl-Z hangs when running with a PTY~~ fix(pty): ctrl-z hangs when running with a PTY Jun 14, 2026

caiocdcs added 5 commits June 17, 2026 11:56

fix(pty): retry suspension waitpid on EINTR

de5cf35

Signed-off-by: Caio Silva <caio@cdcs.dev>

fix(pty): send SIGWINCH to the foreground process group on resume

c0d2d06

Signed-off-by: Caio Silva <caio@cdcs.dev>

refactor(pty): expose PTY master fd through a getter

45585fa

Signed-off-by: Caio Silva <caio@cdcs.dev>

caiocdcs force-pushed the 989-fix-ctrl-z-hang branch from 55f0cb9 to 7d22ded Compare June 17, 2026 10:57

github-actions Bot removed the size/medium label Jun 17, 2026

github-actions Bot added the size/medium label Jun 17, 2026

lukehinds marked this pull request as ready for review June 17, 2026 11:38

Merge branch 'main' into 989-fix-ctrl-z-hang

8557917

github-actions Bot added size/medium and removed size/medium labels Jun 17, 2026

Merge branch 'main' into 989-fix-ctrl-z-hang

b7fc56b

github-actions Bot added size/medium and removed size/medium labels Jun 18, 2026

Merge branch 'main' into 989-fix-ctrl-z-hang

735154d

github-actions Bot added size/medium and removed size/medium labels Jun 18, 2026

caiocdcs mentioned this pull request Jun 18, 2026

Ctrl-Z hangs when running with a PTY (agent process group is orphaned) #989

Closed

lukehinds approved these changes Jun 18, 2026

View reviewed changes

lukehinds merged commit 4179ce0 into nolabs-ai:main Jun 18, 2026
14 checks passed

Navigation Menu

Uh oh!

fix(pty): ctrl-z hangs when running with a PTY - #1135

fix(pty): ctrl-z hangs when running with a PTY#1135
lukehinds merged 8 commits into
nolabs-ai:mainfrom
caiocdcs:989-fix-ctrl-z-hang

caiocdcs commented Jun 12, 2026

Uh oh!

github-actions Bot commented Jun 12, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Uh oh!

lukehinds commented Jun 15, 2026

Uh oh!

caiocdcs commented Jun 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

caiocdcs commented Jun 12, 2026

Linked Issue

Summary

Agent Disclosure

Test Plan

Checklist

Uh oh!

github-actions Bot commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review Summary

Size

Affected crates

Blast radius — Contained

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

lukehinds commented Jun 15, 2026

Uh oh!

caiocdcs commented Jun 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Jun 12, 2026 •

edited

Loading