Skip to content

fix: set PTY raw mode before process start#3837

Merged
lox merged 2 commits intomainfrom
fix/pty-raw-before-start
Apr 20, 2026
Merged

fix: set PTY raw mode before process start#3837
lox merged 2 commits intomainfrom
fix/pty-raw-before-start

Conversation

@lox
Copy link
Copy Markdown
Contributor

@lox lox commented Apr 20, 2026

Summary

  • apply PTY raw mode before starting the child process, so early output cannot be transformed by the default line discipline
  • keep PTY sizing and controlling-terminal setup in the PTY helper
  • add regression coverage for output written before raw mode used to be applied

Problem

We were seeing a startup race in the pty-raw path.

The child process was being started before the PTY master was put into raw mode, so output written immediately on startup could still be processed with the default PTY line discipline. When that happened, \n was rewritten to \r\n, which made TestProcessOutputPTY_PTYRawExperiment fail intermittently.

Testing

  • go test ./process
  • go test ./process -run TestProcessOutputPTY_PTYRawExperimentWritesBeforeRawMode -count=1
  • go test ./process -failfast -count=100 -run TestProcessOutputPTY_PTYRawExperiment

@lox lox requested review from a team as code owners April 20, 2026 06:58
@lox lox force-pushed the fix/pty-raw-before-start branch from 8393fbd to cbd8485 Compare April 20, 2026 07:07
Copy link
Copy Markdown
Contributor

@DrJosh9000 DrJosh9000 left a comment

Choose a reason for hiding this comment

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

LGTM with one style comment split over two suggestions

Comment thread process/pty.go Outdated
Comment thread process/pty.go Outdated
@lox lox enabled auto-merge April 20, 2026 08:10
@lox lox merged commit 1ef434e into main Apr 20, 2026
3 checks passed
@lox lox deleted the fix/pty-raw-before-start branch April 20, 2026 08:12
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.

2 participants