Fix various issues around initial tty sizes + add test coverage - #40722
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to ensure the initial TTY size is applied correctly for container init/exec processes by moving TTY sizing to âstart timeâ (rather than âcreate timeâ), and adds both unit and E2E coverage (including ConPTY/pseudoconsole scenarios) to validate sizing behavior and error handling.
Changes:
- Introduces
WSLCProcessStartOptionsfor passing TTY size (and detach keys) at container start/exec time, and threads TTY size through VM/root-namespace process creation. - Updates wslc console attach/relay flows to consistently configure interactive console mode and to pass console state through helpers.
- Adds new TTY-size tests (invalid sizes rejected; expected size observed; pseudoconsole resize observed) across unit + E2E suites.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/windows/WSLCTests.cpp | Adds unit coverage for invalid TTY sizes and correct init/exec TTY sizing; updates Start calls to use new start options. |
| test/windows/wslc/e2e/WSLCExecutor.h | Adds PseudoConsole support and exposes stdout data + resize helpers for interactive sessions. |
| test/windows/wslc/e2e/WSLCExecutor.cpp | Implements ConPTY-backed interactive runs, stdout snapshotting, and pseudoconsole resizing support. |
| test/windows/wslc/e2e/WSLCE2EHelpers.h | Declares helper to validate pseudoconsole TTY sizing. |
| test/windows/wslc/e2e/WSLCE2EHelpers.cpp | Implements retry-based verification of initial + resized TTY size via pseudoconsole output. |
| test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp | Adds E2E test verifying container run -it respects initial/resized pseudoconsole size. |
| test/windows/wslc/e2e/WSLCE2EContainerExecTests.cpp | Adds E2E test verifying container exec -it respects initial/resized pseudoconsole size. |
| src/windows/wslcsession/WSLCVirtualMachine.h | Extends Linux process creation APIs to accept explicit TTY size. |
| src/windows/wslcsession/WSLCVirtualMachine.cpp | Threads TTY size through process creation and PTY fork paths. |
| src/windows/wslcsession/WSLCSession.h | Updates CreateRootNamespaceProcess signature to accept TTY size. |
| src/windows/wslcsession/WSLCSession.cpp | Passes TTY size through to VM process creation. |
| src/windows/wslcsession/WSLCContainer.h | Updates container Start/Exec APIs to take WSLCProcessStartOptions. |
| src/windows/wslcsession/WSLCContainer.cpp | Uses start options for detach keys and sets console size on exec; resizes container TTY after start. |
| src/windows/wslcsession/ServiceProcessLauncher.cpp | Passes stored rows/cols into VM process creation. |
| src/windows/wslc/services/SessionService.cpp | Ensures interactive console mode is configured and passes console state into attach flow. |
| src/windows/wslc/services/ContainerService.cpp | Computes and passes TTY size at start/exec time and threads console state through attach paths. |
| src/windows/wslc/services/ConsoleService.h | Updates console attach/relay signatures to accept a ConsoleState&. |
| src/windows/wslc/services/ConsoleService.cpp | Applies interactive mode configuration via passed-in ConsoleState. |
| src/windows/service/inc/wslc.idl | Introduces WSLCProcessStartOptions and changes COM method signatures to accept it / pass TTY sizing separately. |
| src/windows/service/exe/PluginManager.cpp | Updates call to new CreateRootNamespaceProcess signature. |
| src/windows/common/WSLCProcessLauncher.h | Changes default TTY size defaults (24x80) and stores size separately from process options. |
| src/windows/common/WSLCProcessLauncher.cpp | Moves TTY sizing into start options for exec/root namespace process creation. |
| src/windows/common/WslClient.cpp | Ensures interactive mode is configured for debug shell console usage. |
| src/windows/common/WSLCContainerLauncher.h | Exposes SetTtySize for container launcher. |
| src/windows/common/WSLCContainerLauncher.cpp | Passes TTY size via start options when starting a created container. |
| src/windows/common/svccomm.cpp | Ensures interactive mode is configured before reading console window size. |
| src/windows/common/ConsoleState.h | Adds SetInteractiveMode() and tracks whether interactive config was applied. |
| src/windows/common/ConsoleState.cpp | Splits interactive console configuration into an explicit method and improves state restoration semantics. |
David Bennett (dkbennett)
left a comment
David Bennett (dkbennett)
left a comment
There was a problem hiding this comment.
I don't expect this will cause any problems with VT related things in the pipeline, as its just a swap from test-defined sequences to common library, the actual tests themselves arent' really changed so this "should just work" (famous last words!).
Looks like some nice improvements to interactive sessions to support pseudoconsoles and ignoring sequences which we can expand upon further as needed in the future!
| // The container attach prompt appears twice. | ||
| session.ExpectStdout(expectedAttachPrompt); | ||
| session.ExpectStdout(expectedAttachPrompt); | ||
| // Ignore resize-repaint messages. Those are emitted when the the tty initial size is set, which can happen before or after we start running commands. |
There was a problem hiding this comment.
This solves my mystery of why the attach prompt is sent twice!
|
|
||
| void WSLCInteractiveSession::ExpectStdout(const std::string& expected) | ||
| { | ||
| if (m_ignoreSequence.has_value()) |
There was a problem hiding this comment.
OK I see, the resize sequence can appear at any time in the console output, as it is resized, so those sequences are ignored so we can skip to the parts that matter. So setting ignore sequence on the prompt allows us to skip one or more duplicate prompts that may appear in the event of resizing happening.
I would expect a future improvement might be to make ignoreSequence a vector of sequences to ignore and if we find a match for any one of them we consume and move on, but as we only need one right now, or can update it later in the test as needed, this is good for current needs.
Summary of the Pull Request
This change fixes various codepaths were the initial tty size of a command wouldn't be correctly set.
To change this properly, the tty size is now passed when a container is started, not created. This is required to properly support the
wslc create, followed bywslc start -acase, since the tty size might be different between the create and start call.PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed