Fix jumbled character order by dscho · Pull Request #124 · git-for-windows/msys2-runtime

dscho · 2026-02-22T23:08:42Z

Add a UI test attempting to replicate the jumbled keystroke order reported in git-for-windows/git#5632. I think that worked, and probably catches even more edge cases. While at it, fix the flakes in the existing UI tests. Oh, and of course fix the bug(s)! Since I did all of this with tremendous assistance especially during the debugging phase, I had to come up with a quite good AGENTS.md, which is thrown in with this PR for good measure.

This fixes git-for-windows/git#5632

The existing method to capture text from Windows Terminal emulates mouse movements, dragging across the entire window with finicky pixel calculations for title bar height, scroll bar width and padding, then right-clicks to copy. This is fragile: if the window geometry changes, if another window gets focus, or if the title bar height differs between OS versions, the capture silently gets the wrong text. Windows Terminal's exportBuffer action avoids all of that by writing the complete scrollback buffer to a file on a keybinding, with no dependence on pixel positions or window focus. To use it, WT must run in portable mode with a settings.json that defines the action and keybinding. Add setup-portable-wt.ps1, which downloads WT (when not already present), creates the .portable marker and writes settings.json with Ctrl+Shift+F12 bound to exportBuffer. It accepts a -DestDir parameter so CI can use $RUNNER_TEMP while local development uses $TEMP. When running inside GitHub Actions it also appends the WT directory to $GITHUB_PATH. In the CI workflow, replace the inline "Install Windows Terminal" step with a call to the setup script (which is available after checkout). In the AHK test library, add CaptureBufferFromWindowsTerminal() which triggers the keybinding, waits for the export file, and returns the buffer contents. The export file is written into the script directory so it gets uploaded as a build artifact on failure. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Now that CaptureBufferFromWindowsTerminal() is available, switch WaitForRegExInWindowsTerminal() to use it instead of the mouse-drag based CaptureTextFromWindowsTerminal(). This also lets us drop the WheelDown scrolling that was needed because the mouse-drag method could only capture the visible portion of the terminal: exportBuffer writes the entire scrollback, so there is nothing to scroll to. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

ci: add an AutoHotKey-based integration test The issue reported in microsoft/git#730 was fixed, but due to missing tests for the issue a regression slipped in within mere weeks. Let's add an integration test that will (hopefully) prevent this issue from regressing again. This integration test is implement as an AutoHotKey script. It might look unnatural to use a script language designed to implement global keyboard shortcuts, but it is a quite powerful approach. While there are miles between the ease of developing AutoHotKey scripts and developing, say, Playwright tests, there is a decent integration into VS Code (including single-step debugging), and AutoHotKey's own development and community are quite vibrant and friendly. I had looked at alternatives to AutoHotKey, such as WinAppDriver, SikuliX, nut.js and AutoIt, in particular searching for a solution that would have a powerful recording feature similar to Playwright, but did not find any that is 1) mature, 2) well-maintained, 3) open source and 4) would be easy to integrate into a GitHub workflow. In the end, AutoHotKey appeared my clearest preference. So how is the test implemented? It lives in `ui-test/` and requires AutoHotKey v2 as well as Windows Terminal (the Legacy Prompt would not reproduce the problem). It then follows the reproducer I gave to the Cygwin team: 1. initialize a Git repository 2. install a `pre-commit` hook 3. this hook shall spawn a non-Cygwin/MSYS2 process in the background 4. that background process shall print to the console after Git exits 5. open a Command Prompt in Windows Terminal 6. run `git commit` 7. wait until the background process is done printing 8. press the Cursor Up key 9. observe that the Command Prompt does not react (in the test, it _does_ expect a reaction: the previous command in the command history should be shown, i.e. `git commit`) In my reproducer, I then also suggested to press the Enter key and to observe that now the "More ?" prompt is shown, but no input is accepted, until Ctrl+Z is pressed. Naturally, the test should not expect _that_ ;-) There were a couple of complications I needed to face when developing this test: - I did not find any easy macro recorder for AutoHotKey that I liked. It would not have helped much, anyway, because intentions are hard to record. - Before I realized that there is excellent AutoHotKey support in VS Code via the AutoHotKey++ and AutoHotKey Debug extensions, I struggled quite a bit to get the syntax right. - Windows Terminal does not use classical Win32 controls that AutoHotKey knows well. To capture the terminal text, we use Windows Terminal's exportBuffer action, which writes the entire scrollback to a file on a keybinding (Ctrl+Shift+F12). This requires running WT in portable mode with a settings.json that defines the action, which the setup script takes care of. - Despite my expectations, `ExitApp` would not actually exit AutoHotKey before the spawned process exits and/or the associated window is closed. For good measure, run this test both on windows-2022 (corresponding to Windows 10) and on windows-2025 (corresponding to Windows 11). Co-authored-by: Eu-Pin Tien <eu-pin.tien@diamond.ac.uk> Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Now that all callers have been switched to CaptureBufferFromWindowsTerminal(), remove the old CaptureTextFromWindowsTerminal() function entirely. It relied on emulating mouse movements to drag-select the visible portion of the terminal and copy it via right-click, which was fragile and required hard-coded pixel offsets for the title bar height, scroll bar width, and padding. None of that complexity is needed anymore. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

This file documents the layered fork structure of this repository (Cygwin → MSYS2 → Git for Windows), the merging-rebase strategy that keeps the main branch fast-forwarding, the build system and its bootstrap chicken-and-egg nature (msys-2.0.dll is the POSIX emulation layer that its own GCC depends on), the CI pipeline, key directories and files, development guidelines, and external resources. The intent is to give AI coding agents enough context to work competently on this codebase without hallucinating about its structure or purpose. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

The SSH clone in the ctrl-c test occasionally fails with "early EOF" / "unexpected disconnect while reading sideband packet" on CI runners. This is a transient SSH/network issue unrelated to the MSYS2 runtime, but it causes the entire test to fail. Wrap the second clone (the one that verifies cloning completes successfully, as opposed to the one that tests Ctrl+C interruption) in a retry loop with up to five attempts. On each failure, clean up the partial clone directory and restart sshd (which may have exited after the broken connection), then try again. The regex pattern now accepts either "Receiving objects: .*, done." (success) or "fatal: early EOF" (transient failure) followed by the PowerShell prompt. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Start implementing UI-based tests by adding an AutoHotKey library AutoHotKey is not only a convenient way to add keyboard shortcuts for functionality (or applications) that does not come with shortcuts, but it is in general a powerful language to remote control GUI elements. We will use this language to implement a couple of automated tests that should hopefully prevent regressions as we have experienced in the past (for example, a regression that was fixed and immediately re-broken, which went unnoticed for months). So let's start by adding a library of useful functions, to be extended as needed. Note: As AutoHotKey is a GUI application, it does not expect to have stdout/stderr attached to it, therefore the `Info()` function added in this commit writes all the messages into `.log` files adjacent to the per-test working directories. But AutoHotKey _can_ have stdout/stderr attached to it, via redirection. In PowerShell, for example, appending `| Out-Default` to the invocation will make stdout/stderr available to AutoHotKey scripts (via the unintuitive syntax `FileAppend "text`n", "*"` (and `"**"` for stderr). The `Info()` function will detect when stdout is available and if it is, will also write to it, in addition to the `.log` file. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

AI-assisted coding is a reality nowadays, as is the all-too-common practice to toss a task over the wall to an AI coding agent and accept its outcome without even bothering to verify. To get better results with either approach (explicitly avoiding to characterize the latter to be even remotely okay), let's add an AGENTS.md file to give AI a "leg up". Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

The current way of capturing the output in a Windows Terminal is brittle, and easily improved (which this topic branch does). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

The PTY architecture section contained several claims that were proven incorrect by Takashi Yano's v7 patch series for the keystroke reordering bug (https://inbox.sourceware.org/cygwin-patches/20260325130453.62246-1-takashi.yano@nifty.ne.jp/). This session (git-for-windows#124) spent multiple weeks attempting to fix the bug with a 4-patch series that was ultimately replaced entirely by Takashi's 9-patch series (merged via c6d1b52813). The AGENTS.md content reflected the incorrect mental model that led to our wrong patches. The most consequential error was the claim that transfer_input() calls in master::write() "steal readline's data" and should be removed. In fact, those transfers are essential: when a native process does not read stdin, keystrokes accumulate in conhost's console input buffer as INPUT_RECORD events. At cleanup, transfer_input(to_cyg) reads them back via ReadConsoleInputA() and writes them to the cyg pipe for bash's readline. Removing the transfers lost data instead of fixing the reordering. The reordering itself was caused by split delivery: some keystrokes went to the nat pipe (fast path) while others leaked to the cyg pipe (line_edit fallthrough) during transitions. The bytes that went directly to the cyg pipe arrived at readline before the bytes that were transferred from the nat pipe at cleanup, producing scrambled output. Takashi's fix was to drop the nat_fg() check from to_be_read_from_nat_pipe() (v7 7/7), ensuring keystrokes always go to the nat pipe while switch_to_nat_pipe is set, and to add pipe_sw_mutex synchronization to to_be_read_from_nat_pipe() (v7 6/7) so the master cannot read inconsistent state during pipe switching. Key corrections to AGENTS.md: The "Designed Keystroke Lifecycle" section is entirely new. It documents the full roundtrip that keystrokes take when a native process is in the foreground: master writes to nat pipe, conhost stores INPUT_RECORDs, cleanup transfers them back to cyg pipe, readline receives them. This roundtrip was completely absent from the previous version and its absence led directly to the wrong diagnosis (we assumed keystrokes sent to conhost were "consumed" and "lost"). The to_be_read_from_nat_pipe() description previously stated that the function checks nat_fg(). The corrected version explicitly states that nat_fg() must NOT be checked, because doing so creates a gap between native process exit and cleanup where keystrokes fall through to the cyg pipe, causing the split delivery that produces reordering. The transfer_input() description previously warned that it "is only correct at process-group boundaries" and that "per-keystroke transfers in master::write() were historically a source of character reordering." Both claims were wrong. The transfers in master::write() (specifically the pcon_start completion block) serve the essential purpose of moving typeahead to the nat pipe after pcon initialization. Takashi's v7 4/7 adds an input_transferred_to_cyg synchronization event so that transferred bytes go through line_edit() in the forward thread, ensuring proper line discipline processing. The reset_switch_to_nat_pipe() description previously documented a "two-level guard" that checked pcon_activated and switch_to_nat_pipe when the owner is self. That was our incorrect patch 3/4. Takashi's correct design is simpler: only return early if a DIFFERENT process owns the nat pipe and is alive. When the owner is self or dead, proceed to cleanup. The oscillation section previously claimed that "transfer code paths have been removed" and that "setpgid_aux() in the slave is now the sole authority for pipe transfers." Both were wrong. The correct design principle is that keystrokes must always go to the nat pipe while switch_to_nat_pipe is set, regardless of oscillation. The transfers remain in place and are essential. The mutex section now documents attach_mutex (the third cross-process mutex, used by get_winpid_to_hand_over() per Takashi's v7 5/7) and notes the consistent lock ordering (pipe_sw_mutex first, then input_mutex). The debugging tips now include "study existing upstream patches before writing fixes" and "never remove transfer_input() calls without understanding what they transfer," both learned the hard way from this session's failures. Assisted-by: Claude Opus 4.6 (1M context) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

dscho · 2026-03-27T16:00:49Z

Updated to @tyan0's v7, including a separate test for the cmd.exe angle.

The nat pipe ownership hand-over mechanism relies on the console process list - the set of processes attached to a console, enumerable via `GetConsoleProcessList()`. For non-cygwin process in pcon_activated case, this list contains all processes attached to the pseudo console. Otherwise, it contains all processes attached to the invisible console. 04f386e (Cygwin: console: Inherit pcon hand over from parent pty, 2024-10-31) added a last-resort fallback in `get_winpid_to_hand_over()` that hands nat pipe ownership to any process in the console process list, including Cygwin processes. This fallback is needed when a Cygwin process on the pseudo console (that might be exec'ed from non- cygwin process) must take over management of an active pseudo console after the original owner exits. When the pseudo console is disabled, this fallback incorrectly finds a Cygwin process (such as the shell) and assigns it nat pipe ownership, because both the original nat pipe owner and the shell are assosiated with the same invisible console. Since there is no console for that process to manage, ownership never gets released, input stays stuck on the nat pipe. Only the third (last-resort) call in the cascade needs guarding: the first two calls filter for native (non-Cygwin) processes via the `nat` parameter, and handing ownership to another native process is fine regardless of pcon state. It is only the fallback to Cygwin processes that is dangerous without an active pseudo console. Guard the fallback with a `pcon_activated` check, since handing nat pipe ownership to a Cygwin process only makes sense when there is an active pseudo console for it to manage. Fixes: 04f386e ("Cygwin: console: Inherit pcon hand over from parent pty") Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp> Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Cherry-picked-from: 699c6892f1 (Cygwin: pty: Fix nat pipe hand-over when pcon is disabled, 2026-03-03) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Currently, pipe_sw_mutex is held in the process which is running in console inherited from pseudo console until the process ends. Due to this behaviour, the process may cause deadlock when it attempts to acquire input_mutex in set_input_mode() called via close_ctty(). This deadlock occurs because the pty master acquire input_mutex first and acquire pipe_sw_mutex next while the process exiting acquire pipe_sw_mutex first. To avoid this deadlock, this patch releases pipe_sw_mutex in pcon_hand_over_proc(). In addition, pointless pipe_sw_mutex acquire/release is drppped in pcon_hand_over_proc(). Fixes: 04f386e ("Cygwin: console: Inherit pcon hand over from parent pty") Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp> Reviewed-by: Corinna Vinschen <corinna@vinschen.de> Cherry-picked-from: 9ef8e3ad3b (Cygwin: console: Release pipe_sw_mutex in pcon_hand_over_proc(), 2026-03-24) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Cygwin maintains POSIX line discipline for its own processes: input goes through `line_edit()` before reaching the reading process. Native (non-Cygwin) processes must not receive line-edited input; they expect raw console input instead. To support both, the PTY keeps two independent pipe pairs for input: a "cyg" pipe for Cygwin processes and a "nat" pipe for native ones. The runtime switches between the two as the foreground process changes. The PTY tracks which process "owns" the nat pipe session via the shared-memory field `nat_pipe_owner_pid`. Only one process is the owner at any time. When `setup_for_non_cygwin_app()` finds that the current owner is still alive, it leaves ownership with that process rather than claiming it for the new one. This means that a Cygwin-spawned native process can go through `cleanup_for_non_cygwin_app()` without being the nat pipe owner. Before this fix, that cleanup called `transfer_input(to_cyg)` unconditionally, draining the pseudo console's input buffer even though another process still owned the session. Keystrokes that the user had typed were moved to the cyg pipe prematurely, so the actual owner found an empty console input buffer and appeared to lose all input. When looking for the next owner of the console in `cleanup_for_non_cygwin_app()` (via `get_winpid_to_hand_over()`), and when transferring the input back to the cyg pipe, guard both with a `nat_pipe_owner_self()` check so that only the actual owner performs these operations. Non-owner processes skip straight to detaching from the pseudo console without disturbing the input buffer. Fixes: f9542a2 ("Cygwin: pty: Re-fix the last bug regarding nat-pipe.") Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp> Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Applied-from: https://inbox.sourceware.org/cygwin-patches/20260328110050.1928-1-takashi.yano@nifty.ne.jp/ Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

In Windows 11, key event with wRepeatCount == 0 is fixed-up to wRepeatCount == 1 in conhost.exe. https://github.com/microsoft/terminal/blob/v1.25.622.0/src/host/inputBuffer.cpp#L406 The console master thread (`cons_master_thread`) reads INPUT_RECORDs from the console input buffer, processes signal-generating events, and writes the remaining records back. After the writeback, it peeks the buffer and uses `inrec_eq()` to verify that conhost stored the records faithfully. On Windows 11, conhost normalizes `wRepeatCount` from 0 to 1 on readback, causing `inrec_eq()` to report a mismatch and triggering an unnecessary fixup path. Treat 0 and 1 as equivalent for comparison purposes. Addresses: git-for-windows/git#5632 Fixes: ff4440f ("Cygwin: console: Introduce new thread which handles input signal.") Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp> Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

The PTY architecture section contained several claims that were proven incorrect by Takashi Yano's v7 patch series for the keystroke reordering bug (https://inbox.sourceware.org/cygwin-patches/20260325130453.62246-1-takashi.yano@nifty.ne.jp/). This session (git-for-windows#124) spent multiple weeks attempting to fix the bug with a 4-patch series that was ultimately replaced entirely by Takashi's 9-patch series (merged via c6d1b52813). The AGENTS.md content reflected the incorrect mental model that led to our wrong patches. The most consequential error was the claim that transfer_input() calls in master::write() "steal readline's data" and should be removed. In fact, those transfers are essential: when a native process does not read stdin, keystrokes accumulate in conhost's console input buffer as INPUT_RECORD events. At cleanup, transfer_input(to_cyg) reads them back via ReadConsoleInputA() and writes them to the cyg pipe for bash's readline. Removing the transfers lost data instead of fixing the reordering. The reordering itself was caused by split delivery: some keystrokes went to the nat pipe (fast path) while others leaked to the cyg pipe (line_edit fallthrough) during transitions. The bytes that went directly to the cyg pipe arrived at readline before the bytes that were transferred from the nat pipe at cleanup, producing scrambled output. Takashi's fix was to drop the nat_fg() check from to_be_read_from_nat_pipe() (v7 7/7), ensuring keystrokes always go to the nat pipe while switch_to_nat_pipe is set, and to add pipe_sw_mutex synchronization to to_be_read_from_nat_pipe() (v7 6/7) so the master cannot read inconsistent state during pipe switching. Key corrections to AGENTS.md: The "Designed Keystroke Lifecycle" section is entirely new. It documents the full roundtrip that keystrokes take when a native process is in the foreground: master writes to nat pipe, conhost stores INPUT_RECORDs, cleanup transfers them back to cyg pipe, readline receives them. This roundtrip was completely absent from the previous version and its absence led directly to the wrong diagnosis (we assumed keystrokes sent to conhost were "consumed" and "lost"). The to_be_read_from_nat_pipe() description previously stated that the function checks nat_fg(). The corrected version explicitly states that nat_fg() must NOT be checked, because doing so creates a gap between native process exit and cleanup where keystrokes fall through to the cyg pipe, causing the split delivery that produces reordering. The transfer_input() description previously warned that it "is only correct at process-group boundaries" and that "per-keystroke transfers in master::write() were historically a source of character reordering." Both claims were wrong. The transfers in master::write() (specifically the pcon_start completion block) serve the essential purpose of moving typeahead to the nat pipe after pcon initialization. Takashi's v7 4/7 adds an input_transferred_to_cyg synchronization event so that transferred bytes go through line_edit() in the forward thread, ensuring proper line discipline processing. The reset_switch_to_nat_pipe() description previously documented a "two-level guard" that checked pcon_activated and switch_to_nat_pipe when the owner is self. That was our incorrect patch 3/4. Takashi's correct design is simpler: only return early if a DIFFERENT process owns the nat pipe and is alive. When the owner is self or dead, proceed to cleanup. The oscillation section previously claimed that "transfer code paths have been removed" and that "setpgid_aux() in the slave is now the sole authority for pipe transfers." Both were wrong. The correct design principle is that keystrokes must always go to the nat pipe while switch_to_nat_pipe is set, regardless of oscillation. The transfers remain in place and are essential. The mutex section now documents attach_mutex (the third cross-process mutex, used by get_winpid_to_hand_over() per Takashi's v7 5/7) and notes the consistent lock ordering (pipe_sw_mutex first, then input_mutex). The debugging tips now include "study existing upstream patches before writing fixes" and "never remove transfer_input() calls without understanding what they transfer," both learned the hard way from this session's failures. Assisted-by: Claude Opus 4.6 (1M context) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

dscho · 2026-03-28T14:09:43Z

Aaaand updated to @tyan0's v8 including the fixup for v8 2/7

Since the parent commit introduced logging to stdout as long as that handle is available, and since the GitHub workflow runs those tests by redirecting the output via `| Out-File` precisely so that that handle _is_ available, we no longer need to show the contents of that log file explicitly: It will have been shown already. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Since logging to `stdout` in CI runs now works, there is no need to write out the logs _again_. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

…errupted Since logging to `stdout` in CI runs now works, there is no need to write out the logs _again_. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

My original plan to let the UI tests write their output directly into the workflow run's log was foiled by the fact that GUI applications have no stdout connected to them, and therefore you cannot write to them. The same is true for AutoHotKey, which _is_ a GUI program (not a Console program), not even when passing `/ErrorStdout`. To work around that, the current workflow definition calls `type <file>.log` after each UI test. This is a bit unideal because it one has to wait for the entire test to finish to see even the first character of the log. Which is a long wait in case of 20 iterations going all wrong. What I failed to notice is that when redirecting its output (e.g. in PowerShell via `| Out-Default`, as it is done in the `ui-tests` workflow definition), stdout _is_ connected, and can be written to using the (admittedly confusing) syntax: `FileAppend "text`n", "*"`. However, it is not _quite_ as easy as that: When the output is _not_ redirected, trying to run that statement will lead to an exception being thrown about an invalid handle. And it is all too easy to run the UI tests forgetting to redirect the output. So let's detect the situation by catching this particular condition: if `stdout` is connected, write both to it and to the `.log` file, if it is not connected, write only to the `.log` file. Note: Given that in POSIX shell, a construct like `first-program | second-program` has the exit status of the `second-program` and ignores whether the `first-program` fails or succeeds, a reader might be worried that `AutoHotKey ... | Out-Default` might "succeed" by mistake if the AutoHotKey invocation fails. But that is not so: PowerShell implements the `set -o pipefail` logic of Bash, where failures on either side of the pipe cause the entire invocation to fail. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

The PTY architecture section contained several claims that were proven incorrect by Takashi Yano's v7 patch series for the keystroke reordering bug (https://inbox.sourceware.org/cygwin-patches/20260325130453.62246-1-takashi.yano@nifty.ne.jp/). This session (git-for-windows#124) spent multiple weeks attempting to fix the bug with a 4-patch series that was ultimately replaced entirely by Takashi's 9-patch series (merged via c6d1b52813). The AGENTS.md content reflected the incorrect mental model that led to our wrong patches. The most consequential error was the claim that transfer_input() calls in master::write() "steal readline's data" and should be removed. In fact, those transfers are essential: when a native process does not read stdin, keystrokes accumulate in conhost's console input buffer as INPUT_RECORD events. At cleanup, transfer_input(to_cyg) reads them back via ReadConsoleInputA() and writes them to the cyg pipe for bash's readline. Removing the transfers lost data instead of fixing the reordering. The reordering itself was caused by split delivery: some keystrokes went to the nat pipe (fast path) while others leaked to the cyg pipe (line_edit fallthrough) during transitions. The bytes that went directly to the cyg pipe arrived at readline before the bytes that were transferred from the nat pipe at cleanup, producing scrambled output. Takashi's fix was to drop the nat_fg() check from to_be_read_from_nat_pipe() (v7 7/7), ensuring keystrokes always go to the nat pipe while switch_to_nat_pipe is set, and to add pipe_sw_mutex synchronization to to_be_read_from_nat_pipe() (v7 6/7) so the master cannot read inconsistent state during pipe switching. Key corrections to AGENTS.md: The "Designed Keystroke Lifecycle" section is entirely new. It documents the full roundtrip that keystrokes take when a native process is in the foreground: master writes to nat pipe, conhost stores INPUT_RECORDs, cleanup transfers them back to cyg pipe, readline receives them. This roundtrip was completely absent from the previous version and its absence led directly to the wrong diagnosis (we assumed keystrokes sent to conhost were "consumed" and "lost"). The to_be_read_from_nat_pipe() description previously stated that the function checks nat_fg(). The corrected version explicitly states that nat_fg() must NOT be checked, because doing so creates a gap between native process exit and cleanup where keystrokes fall through to the cyg pipe, causing the split delivery that produces reordering. The transfer_input() description previously warned that it "is only correct at process-group boundaries" and that "per-keystroke transfers in master::write() were historically a source of character reordering." Both claims were wrong. The transfers in master::write() (specifically the pcon_start completion block) serve the essential purpose of moving typeahead to the nat pipe after pcon initialization. Takashi's v7 4/7 adds an input_transferred_to_cyg synchronization event so that transferred bytes go through line_edit() in the forward thread, ensuring proper line discipline processing. The reset_switch_to_nat_pipe() description previously documented a "two-level guard" that checked pcon_activated and switch_to_nat_pipe when the owner is self. That was our incorrect patch 3/4. Takashi's correct design is simpler: only return early if a DIFFERENT process owns the nat pipe and is alive. When the owner is self or dead, proceed to cleanup. The oscillation section previously claimed that "transfer code paths have been removed" and that "setpgid_aux() in the slave is now the sole authority for pipe transfers." Both were wrong. The correct design principle is that keystrokes must always go to the nat pipe while switch_to_nat_pipe is set, regardless of oscillation. The transfers remain in place and are essential. The mutex section now documents attach_mutex (the third cross-process mutex, used by get_winpid_to_hand_over() per Takashi's v7 5/7) and notes the consistent lock ordering (pipe_sw_mutex first, then input_mutex). The debugging tips now include "study existing upstream patches before writing fixes" and "never remove transfer_input() calls without understanding what they transfer," both learned the hard way from this session's failures. Assisted-by: Claude Opus 4.6 (1M context) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

In Windows 11, pseudo console has an undesired key conversion that the Ctrl-H is translated into Ctrl-Backspace (not Backspace). The reverse VT input path in conhost's `_DoControlCharacter()` maps the byte 0x08 to a Ctrl+Backspace key event (VK_BACK with LEFT_CTRL_PRESSED and character 0x7F). This was introduced in PR #3935 (Jan 2020) to make Ctrl+Backspace delete whole words. In September 2022, PR #13894 rewrote the forward path to properly implement DECBKM (Backarrow Key Mode), but the reverse path was never updated to match, breaking the roundtrip. Due to this behaviour, inrec_eq() in cons_master_thread() fails to compare backspace/Ctrl-H events in the input record sequence. This patch is a workaround for the issue that replaces Ctrl-H with backspace (0x7f), which will be translated into Ctrl-H in pseudo console. Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp> Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

If the console is originating from pseudo console, the input into console is coming from PTY master. This is because: When the pseudo console is active, and a cygwin process is started from non-cygwin process, `cons_master_thread()` runs inside the Cygwin process that inherited the pseudo console from its parent PTY. It reads all `INPUT_RECORD`s from the console input buffer via `ReadConsoleInputW()`, processes signal-generating events (e.g. Ctrl+C), and writes the remaining records back via `WriteConsoleInputW()`. Meanwhile, the PTY master process (e.g. mintty) calls `fhandler_pty_master::write()`, which writes keystrokes to `to_slave_nat` (one end of the nat pipe). Conhost reads from the other end of that pipe, parses the byte stream through its VT input path, and inserts the resulting `INPUT_RECORD`s into the console input buffer. If `cons_master_thread()` reads the buffer and removes a signal record while conhost is simultaneously inserting new records from the PTY master's write, the verify step (`inrec_eq()`) finds records in the buffer that were not part of the original read, reports a mismatch, and enters the fixup path. That fixup path itself can disturb the record order, turning what was merely an interference into an actual problem. Acquiring the PTY's `input_mutex` in `cons_master_thread()` prevents `fhandler_pty_master::write()` from feeding new bytes into the pipe while the read-process-writeback-verify cycle is in progress. Use parent input_mutex as well as input_mutex in console device in cons_master_thread(). Fixes: 04f386e ("Cygwin: console: Inherit pcon hand over from parent pty") Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp> Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

When keystrokes travel through the nat pipe during a native process session, they bypass POSIX line discipline entirely. When they are transferred back to the cyg pipe at cleanup (via `transfer_input(to_cyg)`), they arrive as raw bytes. If the terminal is in canonical mode at that point, VERASE and VKILL characters in those raw bytes have no effect because `line_edit()` was never applied to them. The result: backspace typed while a native process was running fails to erase the preceding character once the input reaches bash's readline. The fix applies `line_edit()` to the transferred bytes before they reach the reading process. The right place to do this is the master's forward thread (`pty_master_fwd_thread()`), because it runs in the master process alongside `fhandler_pty_master::write()` and shares access to the readahead buffer and `line_edit()` state. Calling `line_edit()` from the slave (where `transfer_input()` runs) would not work because that state belongs to the master. To coordinate: `transfer_input(to_cyg)` writes the raw bytes to the cyg pipe's slave end (`to_slave`), then signals a new cross-process event (`input_transferred_to_cyg`) and spin-waits for the forward thread to clear it. The forward thread is converted from synchronous to overlapped I/O so it can wait on both the `from_slave_nat` read completion and the transfer event simultaneously. When the event fires, it reads the transferred bytes from the cyg pipe's master end (`from_master`), processes them through `line_edit()`, and clears the event. The spin-wait in `transfer_input()` holds `input_mutex` (from its caller), which blocks `fhandler_pty_master::write()` from injecting new keystrokes until the forward thread has finished applying `line_edit()` to the transferred bytes. Fixes: 10d083c ("Cygwin: pty: Inherit typeahead data between two input pipes.") Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp> Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

The master process (e.g. mintty) temporarily attaches to the pseudo console's conhost in `transfer_input()` so it can read INPUT_RECORDs via `ReadConsoleInputA()`. During that brief window, `get_console_process_id()` inside `get_winpid_to_hand_over()` calls `GetConsoleProcessList()`, which sees the master among the console's attached processes and may select it as the handover target. That is wrong because the master will detach immediately after the read. Until now, `attach_mutex` was a process-local unnamed mutex, so the slave's `get_winpid_to_hand_over()` could not serialize with the master's temporary attachment. Make `attach_mutex` a cross-process named mutex (`ATTACH_MUTEX`) shared within the PTY, and acquire it around the `get_console_process_id()` calls in `get_winpid_to_hand_over()`. This ensures the console process list enumeration never observes the master while it is temporarily attached. Fixes: 1e6c51d ("Cygwin: pty: Reorganize the code path of setting up and closing pcon.") Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp> Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

`to_be_read_from_nat_pipe()` reads several shared-memory fields (`switch_to_nat_pipe`, `pcon_activated`, `pty_input_state`) to decide whether keystrokes should go to the nat pipe. It is called from `master::write()` on every keystroke. Without synchronization, the slave can be in the middle of a pipe switch (changing these fields in `setup_for_non_cygwin_app()`, `cleanup_for_non_cygwin_app()`, or `setpgid_aux()`) while the master reads a half-updated snapshot, making an inconsistent routing decision that sends keystrokes to the wrong pipe. Guard `to_be_read_from_nat_pipe()` with `pipe_sw_mutex` so it always reads a consistent state. The spin-wait at entry handles the pseudo console initialization case: when `pipe_sw_mutex` is held by the slave during `setup_pseudoconsole()` and `pcon_start` is set, the function returns false immediately, routing keystrokes to the cyg pipe through `line_edit()` where the CSI6n response handler expects them. Acquiring `pipe_sw_mutex` inside `to_be_read_from_nat_pipe()` creates a lock ordering constraint: `master::write()` holds `input_mutex` before calling `to_be_read_from_nat_pipe()`, so the master's lock order is `input_mutex` then `pipe_sw_mutex`. Previously, `cleanup_for_non_cygwin_app()` and `setpgid_aux()` acquired `pipe_sw_mutex` first and then `input_mutex` (for `transfer_input()`), which is the reverse order and would deadlock. Restructure both functions to release `pipe_sw_mutex` before acquiring `input_mutex`, maintaining a consistent lock order throughout. Fixes: bb42852 ("Cygwin: pty: Implement new pseudo console support.") Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp> Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

While a non-cygwin app has exited but the stub process has not yet terminated, `nat_fg()` returns false because no non-cygwin app is running. In this window, pty input goes to the cyg pipe. Due to this, the keystroke order is swapped unexpectedly: 1) start non-cygwin app 2) press 'a' ('a' goes to nat pipe) 3) non-cygwin app exits 4) press 'b' ('b' goes to cyg pipe) 5) the stub process for non-cygwin app transfers input in nat pipe to cyg pipe ('a' goes to cyg pipe) 6) the result in the cyg pipe is "ba" Fix this by dropping the `nat_fg()` check from `to_be_read_from_nat_pipe()`. The function now returns true when `!pcon_start && switch_to_nat_pipe && !masked`. Each component has a specific purpose: - `!pcon_start`: keystrokes go through the CSI6n response handler during pseudo console initialization rather than the fast path. - `switch_to_nat_pipe`: this session-level flag stays true from `setup_for_non_cygwin_app()` through `cleanup_for_non_cygwin_app()`, spanning the entire native process lifetime including the post-exit cleanup window. - `!masked` (`TTY_SLAVE_READING` event does not exist): keystrokes go to the Cygwin pipe when a Cygwin process is actively reading from the slave, since that process expects POSIX-processed input. Removing `nat_fg()` is safe because conhost's input buffer accumulates keystrokes as INPUT_RECORDs during the post-exit window, and `transfer_input(to_cyg)` in `cleanup_for_non_cygwin_app()` reads them back via `ReadConsoleInputA()` and writes them to the cyg pipe. Those transferred bytes then go through `line_edit()` in the master's forward thread (via `input_transferred_to_cyg` from an earlier patch in this series), ensuring proper POSIX line discipline processing. Additionally, add a `nat_fg()` check to the disable_pcon transfer path in `master::write()`. That transfer moves cyg pipe data to the nat pipe when a Cygwin child exits and a native process regains the foreground with pcon disabled. Without pcon, there is no conhost buffer to accumulate keystrokes (the nat pipe is a raw pipe), so keystrokes must only go there when a native process is genuinely in the foreground and ready to read them. The `nat_fg()` guard prevents the transfer from stealing readline's data during the post-exit window. Fixes: f206417 ("Cygwin: pty: Reduce unecessary input transfer.") Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp> Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

This should fix git-for-windows/git#5632 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

The existing UI test infrastructure only supports Windows Terminal, but the keystroke reordering bug reported in git-for-windows/git#5632 manifests most reliably in mintty, which uses a different PTY code path. To write a reproducer for that bug, we need library functions that can launch mintty and read back what it displayed. An initial attempt used mintty's `-l` flag to write a terminal log file, then read back that log with ANSI escape sequences stripped. This approach turned out to be unreliable: mintty buffers its log output, so content that is already visible on screen (such as the `$ ` prompt) may not have been flushed to the log file yet. Polling for a prompt that is already displayed but not yet logged leads to an indefinite wait. Instead, LaunchMintty() configures mintty's Ctrl+F5 keybinding to trigger the `export-html` action, which writes an HTML snapshot of the current screen to a file. This is instantaneous and always reflects exactly what is on screen. The function uses window-class enumeration to identify the newly-created mintty window among any pre-existing instances and returns its handle. CaptureBufferFromMintty() sends Ctrl+F5 to trigger the export, reads the resulting HTML file, extracts the `<body>` content, strips HTML tags, and decodes common entities to return plain text suitable for substring matching. It accepts an optional window title to activate the correct mintty instance before sending the keystroke. Note that AHK's ControlSend cannot be used here because mintty passes the raw keycodes through to the terminal session rather than interpreting them as window-level shortcuts, so WinActivate followed by Send is the only way to trigger the export action. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

This adds an AutoHotKey test that reliably reproduces the keystroke reordering bug described at git-for-windows/git#5632 where characters typed into a bash prompt arrive in the wrong order. The root cause lies in the MSYS2 runtime's PTY input handling: when a non-MSYS2 process (such as PowerShell or cmd.exe) runs in the foreground of a PTY, the transfer_input() function in pty.cc can reorder bytes across pipe buffer boundaries. This is particularly visible when backspace characters get separated from the characters they were meant to delete. The test exploits this by launching a PowerShell process that saturates all CPU cores with tight cmd.exe loops while simultaneously running an MSYS2 sleep.exe in the foreground. While this stress process runs, the test rapidly types characters interleaved with backspaces at 1ms key delay. It walks through the full alphanumeric test string in chunks of two characters, appending two sacrificial characters (",;", chosen for visual dissimilarity with the letters so that they can be spotted very easily in the middle of the output) after each chunk followed by two backspaces to delete them. If the PTY delivers the bytes in order, the backspaces cleanly remove the ",;" and the result matches the original test string. If transfer_input() reorders bytes across buffer boundaries, the backspaces land in the wrong position and the "," or ";" characters leak through, producing visibly jumbled output like "GH,ABCDEFIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789". Using small chunks maximizes the number of buffer boundaries that the PTY must handle, which makes the reordering more dramatic and reliable across different environments. The cpu-stress.ps1 helper script uses cygpath to locate sleep.exe via its POSIX path so that the script works in both the full SDK (where MSYS2 tools live under a Git SDK directory) and in CI (where they live under C:\Program Files\Git). With the current runtime, the bug triggers on the very first iteration in every run tested. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

The fix for keystroke reordering that occurs when the pseudo console oscillates between creation and teardown removes a transfer_input(to_nat) call from the "non-pcon xfer" code path in master::write(). That code path was specifically added in Cygwin commit acc44e0 ("Cygwin: pty: Add missing input transfer when switch_to_pcon_in state", 2021-12-11) to fix a different input routing bug that manifested only when the pseudo console was disabled: https://inbox.sourceware.org/cygwin-patches/20211212130347.10080-1-takashi.yano@nifty.ne.jp/ That patch was in response to a report about spurious ANSI escape sequences ("ghost-typing") appearing in the terminal after quitting vim when git.exe (a native process) spawned vim.exe (a Cygwin process) with the pseudo console disabled: https://inbox.sourceware.org/cygwin-patches/nycvar.QRO.7.76.6.2112092345060.90@tvgsbejvaqbjf.bet/ git-for-windows/git#3579 The transfer was originally introduced as part of a broader effort to preserve typeahead across the two input pipes, starting with Cygwin commit 10d083c ("Cygwin: pty: Inherit typeahead data between two input pipes", 2021-01-28): https://inbox.sourceware.org/cygwin-patches/20210128032614.1678-2-takashi.yano@nifty.ne.jp/ which itself was prompted by a user report about typed characters disappearing while native programs were running: https://inbox.sourceware.org/cygwin/7e3d947e-b178-30a3-589f-b48e6003fbb3@googlemail.com/ Since removing that transfer could theoretically regress the disable_pcon scenario, extend the existing keystroke-order test to also run with MSYS=disable_pcon set. The test reuses the same RunKeystrokeTest() helper and stress command, just with fewer iterations since the disable_pcon code path is simpler and more deterministic. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

An early iteration of the "Fix out-of-order keystrokes" patch series had a bug where it would prevent native processes from receiving _any_ keystrokes under certain circumstances. Let's also specifically verify that this is _not_ the case, and prevent regressing on it. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Takashi Yano's "Add workaround for handling of backspace when pcon enabled" patch works around a conhost.exe bug where byte 0x08 (Ctrl+H) is mapped to a Ctrl+Backspace key event, which performs word-wise deletion instead of single-character deletion. When asked how the bug was originally discovered (https://inbox.sourceware.org/cygwin-patches/c4dc071d-fa7e-ed2e-0c14-3fddb5240f1c@gmx.de/), Takashi explained that Ctrl+H on cmd.exe running in pseudo console erases a word, not a character, on Windows 11 (https://inbox.sourceware.org/cygwin-patches/20260328191514.360fed717ef42a086bac019b@nifty.ne.jp/). This was confirmed in a MinTTY session: launching cmd.exe, typing "abc" and pressing Ctrl+H deletes all three characters instead of just "c" (https://inbox.sourceware.org/cygwin-patches/463c3df7-3810-ed9a-9f7c-c2cf4fd6a7b7@gmx.de/). Add a verification step inside the existing cmd.exe test phase: after confirming the long test string arrived unjumbled, type "echo Expresso" followed by Ctrl+H and Enter. If Ctrl+H correctly deletes only the trailing "o", cmd.exe executes "echo Express" and prints "Express". If conhost.exe mistranslates the keypress into Ctrl+Backspace, the entire word "Expresso" is erased, cmd.exe runs bare "echo" and prints "ECHO is on." instead, causing the test to fail. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Let's fix the often out-of-order keystrokes, as reported in git-for-windows/git#5632. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Update the PTY architecture section to reflect the lessons learned from investigating and fixing the character reordering bug (git-for-windows/git#5632). The master::write() input routing description previously documented three code paths, but the "non-pcon transfer" path (Path 2) was removed as part of the fix because it stole readline's buffered data from the cyg pipe during pseudo console oscillation gaps. Update the section to describe the current two-path structure: the pcon+nat fast path (which now flushes stale readahead before writing) and the line_edit fallthrough (whose accept_input routing now includes a pcon_activated guard). Add a new "Pseudo Console Oscillation" subsection documenting the rapid pcon activate/deactivate cycles that occur when native processes spawn short-lived Cygwin children. This phenomenon was the root cause of the reordering bug and is a critical pattern for future PTY debugging. Correct the transfer_input() guidance: the previous text stated that transfer_input() "must accompany state changes" as an unconditional invariant. In practice, per-keystroke transfers in master::write() were actively harmful; the correct transfer points are setpgid_aux() at process-group boundaries and cleanup_for_non_cygwin_app() at session end. Update the reset_switch_to_nat_pipe() description to reflect the restructured guard that prevents bg_check() from prematurely tearing down active pseudo console sessions when bash itself is the nat_pipe_owner. Add the Cygwin mailing list archives (cygwin, cygwin-patches, cygwin-developers) to the external resources section, since commit messages in this codebase frequently reference discussions on those lists. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Let's verify the fix for the out-of-order keystrokes, in CI builds, so that any regression will be caught quite early. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Document the Cygwin commit message conventions (subject prefix, Fixes and Addresses trailers, trailer ordering) as observed in upstream commits by Corinna Vinschen and Takashi Yano. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

The PTY architecture section contained several claims that were proven incorrect by Takashi Yano's v7 patch series for the keystroke reordering bug (https://inbox.sourceware.org/cygwin-patches/20260325130453.62246-1-takashi.yano@nifty.ne.jp/). This session (git-for-windows#124) spent multiple weeks attempting to fix the bug with a 4-patch series that was ultimately replaced entirely by Takashi's 9-patch series (merged via c6d1b52813). The AGENTS.md content reflected the incorrect mental model that led to our wrong patches. The most consequential error was the claim that transfer_input() calls in master::write() "steal readline's data" and should be removed. In fact, those transfers are essential: when a native process does not read stdin, keystrokes accumulate in conhost's console input buffer as INPUT_RECORD events. At cleanup, transfer_input(to_cyg) reads them back via ReadConsoleInputA() and writes them to the cyg pipe for bash's readline. Removing the transfers lost data instead of fixing the reordering. The reordering itself was caused by split delivery: some keystrokes went to the nat pipe (fast path) while others leaked to the cyg pipe (line_edit fallthrough) during transitions. The bytes that went directly to the cyg pipe arrived at readline before the bytes that were transferred from the nat pipe at cleanup, producing scrambled output. Takashi's fix was to drop the nat_fg() check from to_be_read_from_nat_pipe() (v7 7/7), ensuring keystrokes always go to the nat pipe while switch_to_nat_pipe is set, and to add pipe_sw_mutex synchronization to to_be_read_from_nat_pipe() (v7 6/7) so the master cannot read inconsistent state during pipe switching. Key corrections to AGENTS.md: The "Designed Keystroke Lifecycle" section is entirely new. It documents the full roundtrip that keystrokes take when a native process is in the foreground: master writes to nat pipe, conhost stores INPUT_RECORDs, cleanup transfers them back to cyg pipe, readline receives them. This roundtrip was completely absent from the previous version and its absence led directly to the wrong diagnosis (we assumed keystrokes sent to conhost were "consumed" and "lost"). The to_be_read_from_nat_pipe() description previously stated that the function checks nat_fg(). The corrected version explicitly states that nat_fg() must NOT be checked, because doing so creates a gap between native process exit and cleanup where keystrokes fall through to the cyg pipe, causing the split delivery that produces reordering. The transfer_input() description previously warned that it "is only correct at process-group boundaries" and that "per-keystroke transfers in master::write() were historically a source of character reordering." Both claims were wrong. The transfers in master::write() (specifically the pcon_start completion block) serve the essential purpose of moving typeahead to the nat pipe after pcon initialization. Takashi's v7 4/7 adds an input_transferred_to_cyg synchronization event so that transferred bytes go through line_edit() in the forward thread, ensuring proper line discipline processing. The reset_switch_to_nat_pipe() description previously documented a "two-level guard" that checked pcon_activated and switch_to_nat_pipe when the owner is self. That was our incorrect patch 3/4. Takashi's correct design is simpler: only return early if a DIFFERENT process owns the nat pipe and is alive. When the owner is self or dead, proceed to cleanup. The oscillation section previously claimed that "transfer code paths have been removed" and that "setpgid_aux() in the slave is now the sole authority for pipe transfers." Both were wrong. The correct design principle is that keystrokes must always go to the nat pipe while switch_to_nat_pipe is set, regardless of oscillation. The transfers remain in place and are essential. The mutex section now documents attach_mutex (the third cross-process mutex, used by get_winpid_to_hand_over() per Takashi's v7 5/7) and notes the consistent lock ordering (pipe_sw_mutex first, then input_mutex). The debugging tips now include "study existing upstream patches before writing fixes" and "never remove transfer_input() calls without understanding what they transfer," both learned the hard way from this session's failures. Assisted-by: Claude Opus 4.6 (1M context) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

dscho · 2026-03-29T11:09:55Z

Force-pushed to match the commits that made it into Cygwin's cygwin-3_6-branch:

Range-diff

1: 3d8be6b ! 6: 7c8969de88 Cygwin: pty: Fix nat pipe hand-over when pcon is disabled

@@ Commit message
     Fixes: 04f386e9af99 ("Cygwin: console: Inherit pcon hand over from parent pty")
     Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
     Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
-    Cherry-picked-from: 699c6892f1 (Cygwin: pty: Fix nat pipe hand-over when pcon is disabled, 2026-03-03)
-    Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
+    (cherry picked from commit d1129facdc50a2968925dbbad64c03bde298ef67)
 
  ## winsup/cygwin/fhandler/pty.cc ##
 @@ winsup/cygwin/fhandler/pty.cc: fhandler_pty_slave::get_winpid_to_hand_over (tty *ttyp,

2: a0f8f2b ! 12: c641981e99 Cygwin: console: Release pipe_sw_mutex in pcon_hand_over_proc()

@@ Commit message
     Fixes: 04f386e9af99 ("Cygwin: console: Inherit pcon hand over from parent pty")
     Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
     Reviewed-by: Corinna Vinschen <corinna@vinschen.de>
-    Cherry-picked-from: 9ef8e3ad3b (Cygwin: console: Release pipe_sw_mutex in pcon_hand_over_proc(), 2026-03-24)
-    Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
+    (cherry picked from commit 9ef8e3ad3bec51afddb26e472857962bd1b028e3)
 
  ## winsup/cygwin/fhandler/console.cc ##
 @@ winsup/cygwin/fhandler/console.cc: fhandler_console::pcon_hand_over_proc (void)

3: 79ae2b0 ! 13: ab7f796373 Cygwin: pty: Fix input transfer when multiple non-cygwin apps exist

@@ Commit message
     Fixes: f9542a2e8e75 ("Cygwin: pty: Re-fix the last bug regarding nat-pipe.")
     Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
     Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
-    Applied-from: https://inbox.sourceware.org/cygwin-patches/20260328110050.1928-1-takashi.yano@nifty.ne.jp/
-    Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
+    (cherry picked from commit b790b19dccb7b93f2ec78dbc8353f6da58776083)
 
  ## winsup/cygwin/fhandler/pty.cc ##
 @@ winsup/cygwin/fhandler/pty.cc: fhandler_pty_slave::cleanup_for_non_cygwin_app (handle_set_t *p, tty *ttyp,

4: 10aff7d ! 14: f3914f1e23 Cygwin: console: Fix master thread

@@ Commit message
     Fixes: ff4440fcf768 ("Cygwin: console: Introduce new thread which handles input signal.")
     Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
     Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
-    Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
+    (cherry picked from commit 95b477fb2df76beca1469decf71242c24b223ac5)
 
  ## winsup/cygwin/fhandler/console.cc ##
 @@ winsup/cygwin/fhandler/console.cc: inrec_eq (const INPUT_RECORD *a, const INPUT_RECORD *b, DWORD n)

5: 4f08a75 ! 15: 5fd695db64 Cygwin: pty: Add workaround for handling of backspace when pcon enabled

@@ Commit message
 
     Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
     Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
-    Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
+    (cherry picked from commit 9ac383f5c2139107570e69a051ab06e56414f91d)
 
  ## winsup/cygwin/fhandler/console.cc ##
 @@ winsup/cygwin/fhandler/console.cc: inrec_eq (const INPUT_RECORD *a, const INPUT_RECORD *b, DWORD n)

6: b9d7627 ! 16: 57634e7a19 Cygwin: console: Use input_mutex in the parent PTY in master thread

@@ Commit message
     Fixes: 04f386e9af99 ("Cygwin: console: Inherit pcon hand over from parent pty")
     Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
     Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
-    Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
+    (cherry picked from commit c4fb720afcf120a028f776dde47cacd2d91f3a14)
 
  ## winsup/cygwin/fhandler/console.cc ##
 @@ winsup/cygwin/fhandler/console.cc: fhandler_console::console_state NO_COPY

7: 15c111b ! 17: 4c7b5b71f9 Cygwin: pty: Apply line_edit() for transferred input to to_cyg

@@ Commit message
     Fixes: 10d083c745dd ("Cygwin: pty: Inherit typeahead data between two input pipes.")
     Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
     Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
-    Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
+    (cherry picked from commit a0b38a81b9be482a0058e8f4f5477eeae2f2c012)
 
  ## winsup/cygwin/fhandler/pty.cc ##
 @@ winsup/cygwin/fhandler/pty.cc: atexit_func (void)

8: 9e78b35 ! 18: 07ddd855a8 Cygwin: pty: Guard get_winpid_to_hand_over() with attach_mutex

@@ Commit message
     Fixes: 1e6c51d74136 ("Cygwin: pty: Reorganize the code path of setting up and closing pcon.")
     Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
     Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
-    Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
+    (cherry picked from commit 3adbd41f5babda5a42430fb25d9a02205c416542)
 
  ## winsup/cygwin/fhandler/pty.cc ##
 @@ winsup/cygwin/fhandler/pty.cc: fhandler_pty_slave::open (int flags, mode_t)

9: d235511 ! 19: db700bb753 Cygwin: pty: Guard to_be_read_from_nat_pipe() by pipe_sw_mutex

@@ Commit message
     Fixes: bb4285206207 ("Cygwin: pty: Implement new pseudo console support.")
     Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
     Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
-    Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
+    (cherry picked from commit 2f705b879fac45e5cd761fad8dd7de6b388aee2a)
 
  ## winsup/cygwin/fhandler/pty.cc ##
 @@ winsup/cygwin/fhandler/pty.cc: fhandler_pty_slave::mask_switch_to_nat_pipe (bool mask, bool xfer)

10: 76f644d ! 20: 6f31acbbc4 Cygwin: pty: Drop nat_fg() check from to_be_read_from_nat_pipe()

@@ Commit message
     Fixes: f20641789427 ("Cygwin: pty: Reduce unecessary input transfer.")
     Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
     Reviewed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
-    Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
+    (cherry picked from commit 6413c19fd85bd7d6dc42368e22cebe86b730b3b6)
 
  ## winsup/cygwin/fhandler/pty.cc ##
 @@ winsup/cygwin/fhandler/pty.cc: fhandler_pty_common::to_be_read_from_nat_pipe (void)

dscho · 2026-03-29T11:10:03Z

/open pr

The workflow run was started

…dd896ab324c52970f7d03f9ab0dfe5 Corresponds to git-for-windows/msys2-runtime#124: Fix jumbled character order Add a UI test attempting to replicate the jumbled keystroke order reported in git-for-windows/git#5632. I think that worked, and probably catches even more edge cases. While at it, fix the flakes in the existing UI tests. Oh, and of course fix the bug(s)! Since I did all of this with _tremendous_ assistance especially during the debugging phase, I had to come up with a quite good `AGENTS.md`, which is thrown in with this PR for good measure. This fixes git-for-windows/git#5632

In some circumstances, when typing while a still-running program is about to terminate, [the typed characters could arrive out of order in Git Bash](git-for-windows/git#5632). This bug [was fixed](git-for-windows/msys2-runtime#124). Signed-off-by: gitforwindowshelper[bot] <gitforwindowshelper-bot@users.noreply.github.com>

dscho self-assigned this Feb 22, 2026

dscho force-pushed the fix-jumbled-character-order branch from 64aac43 to 30780f5 Compare February 26, 2026 17:41

dscho marked this pull request as ready for review February 26, 2026 18:16

dscho requested review from mjcheetham and rimrul February 26, 2026 18:18

dscho mentioned this pull request Feb 27, 2026

Fix out-of-order keystrokes cygwingitgadget/cygwin#6

Closed

This comment was marked as resolved.

Sign in to view

dscho mentioned this pull request Mar 3, 2026

[New cygwin version] cygwin-3.6.7 git-for-windows/git#6113

Closed

dscho force-pushed the fix-jumbled-character-order branch from 30780f5 to dbfe8f8 Compare March 5, 2026 09:18

dscho and others added 9 commits March 27, 2026 16:41

Merge branch 'ahk-test-improvements'

aff073a

The current way of capturing the output in a Windows Terminal is brittle, and easily improved (which this topic branch does). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

dscho force-pushed the fix-jumbled-character-order branch from dbfe8f8 to e9e3023 Compare March 27, 2026 15:58

tyan0 added 4 commits March 28, 2026 14:23

dscho force-pushed the fix-jumbled-character-order branch from e9e3023 to 4d3ffb7 Compare March 28, 2026 14:07

dscho added 2 commits March 28, 2026 15:29

fixup! ui-tests: minimize Log window

dc927cd

Since logging to `stdout` in CI runs now works, there is no need to write out the logs _again_. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

dscho added 2 commits March 28, 2026 15:30

fixup! ui-tests: verify that a sleep in Windows Terminal can be int…

50c1ac0

…errupted Since logging to `stdout` in CI runs now works, there is no need to write out the logs _again_. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

dscho force-pushed the fix-jumbled-character-order branch from 4d3ffb7 to 5d6ff6a Compare March 28, 2026 14:31

tyan0 and others added 17 commits March 29, 2026 11:55

Merge branch 'fix-jumbled-keys'

4ad886b

This should fix git-for-windows/git#5632 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Merge branch 'fix-jumbled-keys'

85c357e

Let's fix the often out-of-order keystrokes, as reported in git-for-windows/git#5632. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Merge branch 'fix-jumbled-keys-with-ui-tests'

1a3b0a5

Let's verify the fix for the out-of-order keystrokes, in CI builds, so that any regression will be caught quite early. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

dscho force-pushed the fix-jumbled-character-order branch from 5d6ff6a to fb42d71 Compare March 29, 2026 09:57

gitforwindowshelper bot mentioned this pull request Mar 29, 2026

msys2-runtime: update to 3.6.7-4 git-for-windows/MSYS2-packages#281

Merged

dscho merged commit 44c2330 into git-for-windows:main Mar 29, 2026
31 checks passed

dscho deleted the fix-jumbled-character-order branch March 29, 2026 12:53

dscho mentioned this pull request Mar 29, 2026

git bash loses / mangles order of keyboard input received while busy git-for-windows/git#5632

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix jumbled character order#124

Fix jumbled character order#124
dscho merged 34 commits intogit-for-windows:mainfrom
dscho:fix-jumbled-character-order

dscho commented Feb 22, 2026 •

edited

Loading

Uh oh!

This comment was marked as resolved.

dscho commented Mar 27, 2026

Uh oh!

dscho commented Mar 28, 2026

Uh oh!

dscho commented Mar 29, 2026

Uh oh!

dscho commented Mar 29, 2026 •

edited by gitforwindowshelper bot

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

dscho commented Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

dscho commented Mar 27, 2026

Uh oh!

dscho commented Mar 28, 2026

Uh oh!

dscho commented Mar 29, 2026

Uh oh!

dscho commented Mar 29, 2026 • edited by gitforwindowshelper bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dscho commented Feb 22, 2026 •

edited

Loading

dscho commented Mar 29, 2026 •

edited by gitforwindowshelper bot

Loading