Skip to content

fix(process-registry): detach stdin from background subprocesses to p…#27999

Closed
LifeJiggy wants to merge 1 commit into
NousResearch:mainfrom
LifeJiggy:fix/keyboard-freeze-recovery
Closed

fix(process-registry): detach stdin from background subprocesses to p…#27999
LifeJiggy wants to merge 1 commit into
NousResearch:mainfrom
LifeJiggy:fix/keyboard-freeze-recovery

Conversation

@LifeJiggy

Copy link
Copy Markdown
Contributor

What does this PR do?
Background process non-PTY path used stdin=subprocess.PIPE unconditionally, creating an orphan pipe that was never written to and never closed. Child processes that read stdin would block indefinitely, competing with the parent's prompt_toolkit event loop for terminal ownership and causing complete keyboard lockout.

Change to stdin=subprocess.DEVNULL so children get immediate EOF on stdin reads instead of blocking forever. For interactive stdin, the PTY path (which has its own independent PTY via ptyprocess.PtyProcess.spawn) should be used instead.
Related Issue

Fixes #17959

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tools/process_registry.py — Use subprocess.DEVNULL for background subprocess stdin instead of subprocess.PIPE to prevent orphan pipe blocking

How to Test

  1. Run a background terminal command that reads stdin (e.g., bash -c 'read -t 1 line; echo got: "$line"' with background=True)
  2. Verify keyboard input (typing, Ctrl+C) remains responsive during the background process
  3. Verify the background process exits cleanly without blocking on stdin

…revent keyboard freeze

Background process non-PTY path used stdin=subprocess.PIPE unconditionally,
creating an orphan pipe that was never written to and never closed. Child
processes that read stdin would block indefinitely, competing with the
parent's prompt_toolkit event loop for terminal ownership and causing
complete keyboard lockout.

Change to stdin=subprocess.DEVNULL so children get immediate EOF on stdin
reads instead of blocking forever. For interactive stdin, the PTY path
(which has its own independent PTY via ptyprocess.PtyProcess.spawn) should
be used instead.

Fixes NousResearch#17959
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists tool/terminal Terminal execution and process management labels May 18, 2026
@BoardJames-Bot

Copy link
Copy Markdown

BoardJames triage: this looks shared/systemic rather than branch-local. All non-test checks are green (including both Docker builds); the only blocker is the full Tests / test job cancelling after ~20m with no failed-log details, matching the current shared full-suite timeout/cancellation pattern. Local smoke on the one changed file passed: python -m py_compile tools/process_registry.py. Next action is shared CI/main unblock + rerun, not a process-registry branch fix.

@outsourc-e outsourc-e left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intent makes sense, but as submitted this breaks the existing non-PTY stdin contract. Switching background non-PTY sessions from PIPE to DEVNULL means submit/write/close can no longer work for those sessions, and the current test suite still encodes that capability. Please preserve the stdin helper contract or narrow the behavior change so keyboard-freeze prevention does not disable existing non-PTY stdin flows.

@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #28315 (cherry-picked onto current main with your authorship preserved via rebase-merge — commit 214b953). Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Medium — degraded but workaround exists tool/terminal Terminal execution and process management type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Keyboard input freezes completely — Ctrl+C ineffective, terminal must be killed

5 participants