Skip to content

src: add --no-restore-terminal-state - #66540

Open
fatihguzeldev wants to merge 1 commit into
nodejs:mainfrom
fatihguzeldev:tty-restore-terminal-state
Open

fatihguzeldev wants to merge 1 commit into
nodejs:mainfrom
fatihguzeldev:tty-restore-terminal-state

Conversation

@fatihguzeldev

Copy link
Copy Markdown

Restoring startup terminal settings on exit can overwrite changes made
by another process sharing the terminal, such as an interactive pager.

Add --no-restore-terminal-state as a POSIX opt-out while preserving
libuv's terminal mode cleanup and restoration of O_NONBLOCK.
The default behavior is unchanged.

Includes documentation and pseudo-TTY tests covering exit paths,
NODE_OPTIONS, option precedence, and raw-mode cleanup.

Refs: #66440
Refs: #66134

Tested on macOS arm64: release build, focused pseudo-TTY and CLI tests,
raw-mode cleanup, less 710 reproduction, and lint checks passed.

Restoring startup terminal settings on exit can overwrite changes made
by another process sharing the terminal, such as an interactive pager.

Add a POSIX opt-out that preserves libuv's terminal mode cleanup and
restoration of the O_NONBLOCK flag.

Refs: nodejs#66440
Signed-off-by: Fatih Güzel <fatihguzeldev@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/config
  • @nodejs/startup

@codecov

codecov Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.39%. Comparing base (019e869) to head (464554f).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
src/node.cc 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66540      +/-   ##
==========================================
- Coverage   90.40%   90.39%   -0.01%     
==========================================
  Files         791      791              
  Lines      276011   276017       +6     
  Branches    52983    52977       -6     
==========================================
- Hits       249529   249519      -10     
- Misses      16891    16902      +11     
- Partials     9591     9596       +5     
Files with missing lines Coverage Δ
src/node_options.cc 81.73% <100.00%> (+0.02%) ⬆️
src/node_options.h 95.67% <100.00%> (+0.01%) ⬆️
src/node.cc 78.87% <75.00%> (-0.05%) ⬇️

... and 20 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@avih avih mentioned this pull request Oct 5, 2026
@avih

avih commented Oct 5, 2026

Copy link
Copy Markdown

Thanks for the suggested fix.

While it can help if one knows to manually use this flag and override the default behavior, is there a good reason to perform this reset by default even if no tty fd was used at all from-start-to-finish of this node instance?

After all, when node exits, it already knows (or technically can know) whether something was read-from and/or written-to a tty fd, and surely this reset is never required if such tty fd was not touched at all?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants