Skip to content

msal: restore visible console parenting on Windows - #2464

Merged
mjcheetham merged 6 commits into
git-ecosystem:mainfrom
mjcheetham:parent-windowing
Oct 5, 2026
Merged

mjcheetham merged 6 commits into
git-ecosystem:mainfrom
mjcheetham:parent-windowing

Conversation

@mjcheetham

Copy link
Copy Markdown
Contributor

Windows authentication dialogs need an appropriate parent window, but Git is not always launched from a visible terminal. GUI applications and background processes can retain a hidden console with a valid HWND. Automatically selecting that invisible owner can leave authentication UI out of view, making the operation appear stalled while it waits for credentials.

GCM 3.0 also accidentally reversed the parent-selection preference used in 2.x: interactive Windows broker authentication creates a progress window before considering an existing console owner. This introduces unnecessary temporary UI and disconnects authentication dialogs from the terminal that initiated the operation.

This PR restores console-first parenting while rejecting invisible automatically discovered owners. The selection order becomes:

  1. An explicit HWND supplied through GCM_MODAL_PARENTHWND.
  2. The console’s root-owner HWND, provided IsWindowVisible returns true.
  3. A GCM-created progress window, if a parent is required.
  4. No parent otherwise.

The visibility check applies after walking the window parent/owner chain. This matters for ConPTY, where the console HWND may be a hidden compatibility window while its root owner is the visible terminal.

Explicit HWNDs remain the caller’s responsibility and are not visibility-filtered. Minimised owners also remain eligible: this change deliberately does not add an IsIconic check or replace a minimised owner with an independent progress window.

The series adds regression coverage for parent precedence, hidden or missing owners, and progress-window cleanup, and documents GCM_MODAL_PARENTHWND for applications integrating Git into their own GUI.

Ensure that we only consider a console window (or hosting terminal in
case of ConPTY) a valid parent for MSAL windows if it is visible.

This will prevent MSAL windows from being hidden too. Note that we do
**not** apply the visibility check to the optional explicitly provided
parent window (via `GCM_MODEL_PARENTHWND)` - the caller should be the
one to decide if their handle is good to use or not.

Finally, note that we do **not** check for a minimised parent window
(via `IsIconic(HWND)`) because we should probably also remain minimised
if our parent was.

Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Extract the console parent lookup logic to a static method in
preparation for allowing tests to mock it.

Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Allow testing of the parent window adapter by allowing mocks of the core
platform-specific APIs:

* creation of stub window
* window visibilty checking
* get console window

Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
In previous versions of GCM (2.x) we preferred to parent MSAL windows to
the console window, if possible, over creating our own stub 'progress'
window.

In GCM 3.0.0 we accidentally reversed that preference, which means in
practice we always used the stub window and never the console window.

Let's reverse that back to the desired order:

1. Explicit parent (via `GCM_MODAL_PARENTHWND`)
2. Console window parent (if present and visible)
3. Stub progress window (if required by the caller)
4. None

Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Add comprehensive tests of the MSAL parent window adapter, and the
precedence logic for selecting (and creating) an appropriate parent
window.

Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
@mjcheetham
mjcheetham requested a review from a team as a code owner October 5, 2026 13:55
@mjcheetham mjcheetham added platform:windows Specific to the Windows platform auth:entra Specific to Microsoft Entra Authentication gui Specific to graphical user interface controls labels Oct 5, 2026
@mjcheetham
mjcheetham requested a review from dscho October 5, 2026 13:58
@mjcheetham
mjcheetham merged commit 2d60006 into git-ecosystem:main Oct 5, 2026
26 checks passed
mjcheetham added a commit that referenced this pull request Oct 5, 2026
This follows up on #2464.

Visible-console parenting currently lives inside the MSAL adapter, so
_other_ in-process authentication dialogs remain unparented unless
`GCM_MODAL_PARENTHWND` is supplied. Those prompts can appear
disconnected from the terminal that initiated the Git operation.

This PR moves console-parent discovery into a shared platform helper and
uses it in the authentication and UI-helper parent handle lookups. This
extends the fallback beyond MSAL while keeping the two lookup paths
consistent. MSAL retains responsibility for creating a progress window
when broker authentication requires a parent and no usable handle was
resolved.

Explicit HWNDs still take precedence and are not visibility-filtered.
Automatically discovered owners must be visible, with visibility checked
after resolving the console's root owner to accommodate ConPTY
compatibility windows.

Update the environment variable documentation to describe the fallback
to the console-parent window.

Tested behaviour on Windows Terminal, conhost.exe (classic terminal
host), and Git Bash/MSYS2's terminal.
@mjcheetham
mjcheetham deleted the parent-windowing branch October 6, 2026 11:12
@mjcheetham mjcheetham mentioned this pull request Oct 6, 2026
mjcheetham added a commit that referenced this pull request Oct 6, 2026
**Bug Fixes:**

- Prompt for default OS account on Microsoft DevBox (#2459)
- Restore Entra broker parent window handling (#2464)

**Features:**

- Support `authtype` Git capability (#2457)
- Add console parent window parenting for non-Entra UI (#2465)

**Other changes:**

- Drop Windows x86 release binaries (#2456)
- Documentation updates (#2460)
- Fix release workflow _VERSION_ file parsing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auth:entra Specific to Microsoft Entra Authentication gui Specific to graphical user interface controls platform:windows Specific to the Windows platform

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants