Open
Conversation
dscho
force-pushed
the
fix-jumbled-character-order
branch
from
64aac43 to
30780f5
Compare
This comment was marked as resolved.
This comment was marked as resolved.
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>
…e transfer
When rapidly typing at a Cygwin terminal while a native Windows
program runs and exits (e.g. typing while a short-lived git command
prints output in a mintty terminal), keystrokes can arrive at bash in
the wrong order: typing "git log" may display "igt olg" or similar
scrambled output.
Background: the pseudo console's two-pipe architecture
------------------------------------------------------
The Cygwin PTY maintains two independent pairs of input pipes:
"cyg" pipe (to_slave / from_master):
For Cygwin processes. The master calls line_edit(), which
implements POSIX line discipline (handling backspace, Ctrl-C,
canonical-mode buffering, echo), then accept_input() writes
the processed bytes to this pipe. The Cygwin slave (e.g.
bash) reads from the other end.
"nat" pipe (to_slave_nat / from_master_nat):
For native Windows console programs. When the "pseudo console"
(abbreviated "pcon" in the code) is active, Windows' conhost.exe
wraps this pipe and provides the Win32 Console API
(ReadConsoleInput, etc.) to the native app. The master writes
raw bytes directly to this pipe.
Both pipes are needed because Cygwin processes and native Windows
programs have incompatible expectations. Cygwin processes read
POSIX byte streams after line discipline processing. Native
programs call ReadConsoleInput() for structured key-down/key-up
events -- something a plain pipe cannot provide.
A shared-memory variable `pty_input_state` (values: to_cyg or
to_nat) tracks which pipe currently receives new input. The
function transfer_input() moves pending data between the two pipes
when the state changes. The flag `pcon_activated` indicates whether
a Windows pseudo console is currently running.
The problem: oscillation
------------------------
Each time a native program starts or exits in a PTY, the pseudo
console activates or deactivates. Even a single such transition --
running git.exe and then returning to the bash prompt -- can trigger
the bug. The effect is amplified when transitions happen in quick
succession (e.g. a shell script calling several short-lived native
commands), creating many oscillation cycles:
(1) native program starts: pcon_activated=true, state=to_nat
(2) native program exits: pcon deactivated, state=to_cyg
(3) next native program: pcon reactivated, state=to_nat
Git for Windows' AutoHotKey-based UI tests create this pattern
deliberately by having PowerShell invoke Cygwin utilities in a tight
loop to achieve near-100% reproduction.
During each transition, master::write() -- which routes every
keystroke from the terminal emulator -- must decide which pipe to
use.
How the transfer steals readline's data
---------------------------------------
In master::write(), after the pcon+nat fast code path and before
calling line_edit(), there was a code block that runs when:
to_be_read_from_nat_pipe() is true (a native app is "in charge")
&& pcon_activated is false (pcon momentarily OFF)
&& pty_input_state is to_cyg (input going to cyg pipe)
When all three conditions hold, it calls transfer_input(to_nat) to
move ALL pending data from the cyg pipe to the nat pipe. The intent
was to handle a specific scenario where, with pseudo console
disabled, input lingered in the wrong pipe after a Cygwin child
exited.
The problem is that during oscillation step (2), these conditions
are also true -- and the cyg pipe contains bash's readline buffer
with the partially-typed command line. On every keystroke during
the gap, this code reads ALL of readline's buffered input out of the
cyg pipe and pushes it into the nat pipe:
Keystroke 'g' arrives during oscillation gap (step 2):
|
v
transfer_input(to_nat) <--- reads readline's prior "it"
| from cyg pipe, writes to nat
v
line_edit('g')
|
v
accept_input() ---> nat pipe <--- 'g' also goes to nat pipe
|
v
pcon reactivates (step 3):
|
v
readline reads cyg pipe, but "it" is gone -- it was moved
to the nat pipe. Later keystrokes that went correctly to
the cyg pipe appear before the stolen ones.
Result: "git" arrives at bash as "tgi" or similar scramble.
The fix
-------
Remove the transfer_input() call entirely. The original commit's
comment says it was needed "when cygwin-app which is started from
non-cygwin app is terminated if pseudo console is disabled." That
scenario is already handled by setpgid_aux() in the slave process,
which performs the transfer at the correct moment: the process-group
change when the Cygwin child exits. The per-keystroke transfer in
master::write() was redundant for that use case and catastrophic
during oscillation.
This single change addresses the majority of the reordering (from
"virtually every character scrambled" to roughly one stray character
per five iterations in testing). Subsequent commits in this series
address the remaining code paths that can still displace readline
data during oscillation.
Regression note: the removed code was added in response to a
ghost-typing report where vim's ANSI escape responses appeared at
the bash prompt after exiting vim with MSYS=disable_pcon:
https://inbox.sourceware.org/cygwin-patches/nycvar.QRO.7.76.6.2112092345060.90@tvgsbejvaqbjf.bet/
Since setpgid_aux() still handles the pipe transfer at process-group
boundaries, the disable_pcon scenario is covered. Tested with
MSYS=disable_pcon and Git for Windows' AutoHotKey-based UI tests
without regressions.
Addresses: git-for-windows/git#5632
Fixes: acc44e0 ("Cygwin: pty: Add missing input transfer when switch_to_pcon_in state.")
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>
…e data
After the previous commit addressed the worst data-stealing code
path, roughly one in five test iterations still shows a stray
character. Another transfer code path in master::write() is
responsible.
When a native process becomes the PTY foreground, the pseudo console
must be initialized. During this "pcon_start" phase, master::write()
enters a polling loop that feeds keystrokes to the nascent pseudo
console. When the loop completes (pcon_start becomes false), there
was a block of code that:
1. Flushed the master's readahead buffer via accept_input()
2. Called transfer_input(to_nat) to move all cyg pipe data
to the nat pipe
The intent was to preserve typeahead: characters typed during pcon
initialization should eventually reach the native process. But
during pseudo console oscillation (the rapid pcon on/off cycles
described in the previous commit), this fires on every pcon
re-initialization -- and the "typeahead" it transfers includes
readline's entire editing buffer.
Worse, if the terminal emulator was mid-way through sending a rapid
editing sequence like "XY<BS><BS>" (type two characters, then erase
them with backspace), the readahead flush fires after buffering "X"
but before the backspaces arrive. The orphaned "X" gets pushed to
the cyg pipe via accept_input(), where readline sees it as genuine
input -- producing a stray character that the user never intended.
Remove the accept_input() and transfer_input() calls entirely.
Keep `pcon_start_pid = 0`, which marks the end of initialization.
The readahead data belongs to the Cygwin process (bash is in
canonical mode during command entry) and will be delivered naturally
when line_edit() encounters a newline or when readline switches the
terminal to raw mode after the foreground command exits. The
setpgid_aux() code path in the slave process still handles the
steady-state cyg-to-nat transfer at process-group boundaries.
Combined with the previous commit, Git for Windows' AutoHotKey-based
UI tests now pass cleanly in the vast majority of iterations.
Regression note: the removed code was motivated by a 2020 bug report
about lost typeahead with native processes:
https://inbox.sourceware.org/cygwin/7e3d947e-b178-30a3-589f-b48e6003fbb3@googlemail.com/
Since the pcon_start window is brief (a few milliseconds) and
setpgid_aux() handles the steady-state transfer, the risk of
typeahead loss is low.
Addresses: git-for-windows/git#5632
Fixes: 10d083c ("Cygwin: pty: Inherit typeahead data between two input pipes.")
Fixes: f206417 ("Cygwin: pty: Reduce unecessary input transfer.")
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>
… oscillation
The two preceding commits removed transfer code paths that stole
readline's data during pseudo console oscillation. This commit
addresses the oscillation itself: a guard function that tears down
active pseudo console sessions prematurely, causing more frequent
oscillation cycles and thus more opportunities for the remaining
(less harmful) timing issues to manifest.
The function reset_switch_to_nat_pipe() runs from bg_check() in the
slave process. Its purpose is to clean up the nat pipe state when
no native process is using the pseudo console anymore. Its guard
logic was:
if (!nat_pipe_owner_self(pid) && process_alive(pid))
return; /* someone else owns it, don't reset */
/* fall through to destructive cleanup: clear pty_input_state,
nat_pipe_owner_pid, switch_to_nat_pipe, pcon_activated */
The nat_pipe_owner_pid is set to bash's own PID during
setup_for_non_cygwin_app() (because bash is the process that calls
exec() to launch the native program). When bg_check() runs and
calls reset_switch_to_nat_pipe(), nat_pipe_owner_self() returns
true -- the first condition becomes false, the && short-circuits,
and the function falls through to the destructive cleanup. It
clears pcon_activated, switch_to_nat_pipe, pty_input_state, and
nat_pipe_owner_pid -- even though the native process is still
alive and actively using the pseudo console.
This forced every subsequent code path to re-initialize the pseudo
console from scratch, creating exactly the rapid oscillation
described in the earlier commits.
Restructure the guard into two separate checks:
if (process_alive(pid))
{
if (!nat_pipe_owner_self(pid))
return; /* someone else owns it */
if (pcon_activated || switch_to_nat_pipe)
return; /* we own it, but session is still active */
}
/* fall through: owner died or session ended */
When a different process owns the nat pipe, the behavior is
unchanged. When bash itself is the owner, the function now also
returns early if pcon_activated or switch_to_nat_pipe is still set.
Both flags are checked because during pseudo console handovers
between parent and child native processes, pcon_activated is
briefly false while switch_to_nat_pipe remains true.
The cleanup still runs when it should: when the owner process has
exited or when both flags indicate the session has truly ended.
Regression note: this change is strictly more conservative -- it
adds conditions that prevent cleanup, never removes them. Every
scenario where the original code returned early still returns early.
Addresses: git-for-windows/git#5632
Fixes: 919dea6 ("Cygwin: pty: Fix a race issue in startup of pseudo console.")
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>
This should fix git-for-windows/git#5632 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>
…fast path
This final commit in the series addresses two remaining edge cases
where characters can escape through unintended routing during pseudo
console oscillation.
Part 1: accept_input() routing guard
-------------------------------------
accept_input() writes data from the readahead buffer to one of the
two PTY input pipes. Its routing condition was:
if (to_be_read_from_nat_pipe()
&& pty_input_state == to_nat)
write_to = to_slave_nat; /* nat pipe */
else
write_to = to_slave; /* cyg pipe */
A comment in the code documents the intention: "This code is reached
if non-cygwin app is foreground and pseudo console is NOT enabled."
But the condition does not actually check pcon_activated. During
pseudo console oscillation, accept_input() can route data to the
nat pipe even while the pseudo console IS active. When pcon is
active, input for native processes flows through conhost.exe, not
through direct pipe writes. Routing data to the nat pipe via
accept_input() during pcon activation either duplicates what
conhost already delivers or displaces data that should have stayed
in the cyg pipe.
Add `&& !pcon_activated` to make the code match its own documented
invariant.
Part 2: readahead flush in the pcon+nat fast code path
------------------------------------------------------
The pcon+nat fast code path in master::write() handles the common
case where a native app is in the foreground with pcon active. It
writes keystrokes directly to the nat pipe via WriteFile(), bypassing
line_edit() entirely.
If a previous call to master::write() went through line_edit()
(because pcon was momentarily inactive during oscillation),
line_edit() may have left data in the readahead buffer via
unget_readahead(). Without flushing this stale readahead, it
persists until the next line_edit() call, at which point
accept_input() emits it -- potentially after newer characters that
went through the fast code path, breaking chronological order.
Add an accept_input() call at the top of the pcon+nat fast code path
to flush any stale readahead before the current keystroke is written
via WriteFile().
Together with the three preceding commits, this eliminates the
character reordering reported in git-for-windows/git#5632.
Addresses: git-for-windows/git#5632
Fixes: 3d46583 ("Cygwin: pty: Update some comments in pty code.")
Assisted-by: Claude Opus 4.6
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 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>
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>
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 ("XY") after each chunk followed by two backspaces to delete them. If the PTY delivers the bytes in order, the backspaces cleanly remove the "XY" 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 "X" or "Y" characters leak through, producing visibly jumbled output like "GHXABCDEFIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789". 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>
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>
The preceding commit added a UI test for keystroke reordering that occurs when the pseudo console oscillates between creation and teardown. The fix for that reordering (in a separate commit) 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>
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>
dscho
force-pushed
the
fix-jumbled-character-order
branch
from
30780f5 to
dbfe8f8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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