Fix regression causing credential prompts to hide behind other windows more often

[Tyrie Vella (tyrielv)]

Problem and Context

Background GVFS processes such as GVFS.Mount.exe run with a hidden console window. When git launches the Git Credential Manager (GCM) from such a process, GCM parents its interactive sign-in prompt to the hidden console. The prompt then has no foreground right and sinks behind other windows, so the user never sees it and the mount appears to hang waiting for credentials that were never visibly requested.

This is a recent regression. The change that set CreateNoWindow=false to eliminate the per-child conhost.exe allocation (commit 6c01575f, "Eliminate conhost.exe from git.exe and hooks process launches") also stopped detaching git from the parent's hidden console. Before that change, auth-capable git commands ran with CreateNoWindow=true, which gave GCM no console to parent to, so the prompt surfaced on top. Since the change, background mounts inherit the hidden console and the prompt is hidden behind other windows. We have since seen an uptick in both user reports and telemetry of mounts stuck in this hidden-prompt state.

Those indefinite waits are also the direct cause of the auth-timeout and contention work in #2082 — a hidden prompt that no one answers holds the shared prefetch lock and HTTP capacity until it times out. This PR addresses the root cause by making the prompt visible again for the two commands that can raise it; #2082 bounds the blast radius for the case where a prompt still goes unanswered.

Changes

• Add GVFSPlatform.CurrentProcessHasVisibleConsoleWindow(). The Windows implementation checks GetAncestor(GetConsoleWindow(), GA_ROOTOWNER) and IsWindowVisible — it keys on the console window and its root owner, not an arbitrary GUI window.
• Add a mayRequireAuth parameter to GitProcess.GetGitProcess. When set and the current process has no visible console window, git is launched detached from the console (CreateNoWindow=true). GCM then parents its prompt to a topmost stub window instead of the hidden console. A process with a visible console (e.g. gvfs.exe run from a classic console) keeps the current behavior so the prompt parents to that window.
• Wire mayRequireAuth: true to exactly the two credential fill sites (TryGetCredential url fill and TryGetCertificatePassword cert fill). credential approve/reject and every other git command GVFS runs keep the default false, so the per-child conhost cost eliminated in 6c01575f stays avoided for every non-authenticating call. See the verb audit below.
• Log hasVisibleConsoleWindow from TryGetCredential so a credential fill that does not surface a prompt stays diagnosable from telemetry.
• Guard the native console probe (DllNotFoundException/EntryPointNotFoundException) so a binding failure keeps the previous behavior (CreateNoWindow=false) instead of aborting the fill.
• Tests: add wiring tests asserting only credential fill (url and cert) is flagged mayRequireAuth, plus a direct GetGitProcess truth-table test for the detach formula across console-visibility and mayRequireAuth combinations.

Why only credential fill is flagged — GetGitProcess verb audit

git credential fill is the only command run through GitProcess.GetGitProcess that can raise an interactive GCM prompt. GetGitProcess also sets GIT_TERMINAL_PROMPT=0 and GCM_VALIDATE=0 unconditionally on every spawned process, so no git verb can raise a terminal prompt; the GUI GCM prompt is the only possibility, and only fill triggers it. No fetch/pull/push/clone/ls-remote runs through GitProcess — GVFS transfers objects through its own read-object → named-pipe → HTTP protocol, not git transport.

Verb (through GetGitProcess)	Caller	Credential-helper op	Can raise GCM prompt?
credential fill (repo url)	TryGetCredential	get (interactive)	Yes — mayRequireAuth: true
credential fill (protocol=cert)	TryGetCertificatePassword	get (interactive)	Yes — mayRequireAuth: true
credential approve	TryStoreCredential	store	No (stores an obtained credential)
credential reject	TryDeleteCredential	erase	No (erases a stored credential)
init, --version, config (all forms), rev-parse, name-rev, diff-tree, branch, checkout -f, reset, status, diff --cached, checkout-index, unpack-objects, pack-objects, commit-graph write/verify, index-pack, multi-pack-index write/verify/expire/repack, verify-pack, remote add, cat-file, ls-tree, ls-files, symbolic-ref, update-ref, read-tree, prune-packed	various	local only	No (no authenticated network I/O; remote add writes local config)

Some local verbs run with the read-object hook enabled; a missing object can make that hook ask GVFS to hydrate over HTTP, and if that path needs credentials GVFS starts a separate credential fill — itself covered above. GVFS.Hooks spawns git via ProcessHelper.Run (not GetGitProcess) for local verbs that do not use GCM.

Conclusion: wiring mayRequireAuth: true to the two credential fill sites covers every path that can surface a hidden prompt.

Known gaps and deliberate decisions

• Credential-helper output parsing (ParseValue) — a missing prefix is not guarded for -1, so malformed credential fill output can mis-parse. Pre-existing; not touched here. No follow-up planned yet.
• Raw helper stdout on parse failure — on the exit-0-but-parse-fail path the credential methods log raw helper stdout, which can include a plaintext secret. Pre-existing and out of scope here; it is addressed by open PR Do not trace raw credential output #2101 (Do not trace raw credential output).
• Console-visibility value is threaded, not re-probed — the probe runs once per credential-fill call and the result flows to both the telemetry field and the CreateNoWindow decision, so the two cannot diverge. This adds a nullable parameter to three methods; a deliberate choice over re-probing at the decision site. No follow-up planned.
• Certificate-fill success-path telemetry is untested — the test mock's activity.Stop discards its metadata, so only the failure path is asserted for the certificate flow. The runtime stamps the same value on both paths. Follow-up possible if the mock gains Stop-metadata capture.
• executingProcess assignment races under concurrency — the field is assigned before the lock in InvokeGitImpl, so concurrent invocations can race. Pre-existing; credential fill is effectively serialized today. Addressed by a separate follow-up change.

[Nathan Baird (ShiningMassXAcc)]

Thanks for chasing this down — the background-mount case is correctly detected and the fix does address the reported bug. Two things I would like to resolve before it merges, both stemming from one claim in the commit message.

The Windows Terminal claim does not match the code's actual behavior

The commit message states that "a non-displayed pseudo-console such as a foreground gvfs.exe run from Windows Terminal, is treated as having no visible window and also detaches git." That is not what happens. Under a ConPTY, GetAncestor(GetConsoleWindow(), GA_ROOTOWNER) walks from the PseudoConsoleWindow up to the hosting CASCADIA_HOSTING_WINDOW_CLASS window, which is visible — so CurrentProcessHasVisibleConsoleWindow() returns true and git is not detached there.

I measured all three cases on Windows 11, replicating the exact probe logic from this PR (GetConsoleWindow -> GetAncestor(..., GA_ROOTOWNER) -> IsWindowVisible):

scenario	GetConsoleWindow()	GA_ROOTOWNER	probe
Start-Process -WindowStyle Normal	PseudoConsoleWindow (visible)	WindowsTerminal / CASCADIA_HOSTING_WINDOW_CLASS (visible)	True
hidden console (Mount-like)	ConsoleWindowClass (hidden)	self (hidden)	False
Windows Terminal tab	PseudoConsoleWindow (visible)	WindowsTerminal / CASCADIA_HOSTING_WINDOW_CLASS (visible)	True

Note the first row: Windows Terminal is the default terminal host on Windows 11, so even -WindowStyle Normal is hosted by it. The classic-conhost branch the description leans on is rarer than it implies, and the only case that actually yields False is a genuinely hidden ConsoleWindowClass — which is to say GVFS.Mount, exactly the bug being fixed.

Two consequences:

1. The permanent record is wrong about the most common interactive path. Worth correcting the commit message and PR description before merge, since the XML doc on GVFSPlatform.CurrentProcessHasVisibleConsoleWindow is accurate and the two will otherwise disagree.

2. The non-detached ConPTY path looks unevaluated. Because the description assumed Windows Terminal detaches, that branch was presumably not analyzed. There, git and GCM inherit the pseudo-console, and GCM parents its prompt to whatever it derives from GetConsoleWindow() — which is a zero-size, off-screen PseudoConsoleWindow that nonetheless reports IsWindowVisible == true. If GCM does not perform its own GA_ROOTOWNER walk, the prompt ends up owned by a window the user is not looking at: the same class of problem this PR is fixing, for interactive gvfs clone / gvfs mount under the default Windows 11 terminal.

   Could you confirm what GCM actually parents its prompt to in the ConPTY case? This is not a regression from this change — CreateNoWindow is unchanged on that path — but it determines whether the fix is complete or only covers the service scenario.

I am happy to share the probe script if it is useful for verifying the ConPTY question.

[Nathan Baird (ShiningMassXAcc)]

Re-review of 58edd7c8. I checked each of the eight inline points against the code rather than against the replies — all eight are genuinely fixed, and a few go further than I asked.

#	Original point	Verified
1 (High)	hasVisibleConsoleWindow was logged only after the ExitCodeIsFailure early return, so it never fired on the timeout path it was added for	Probe hoisted above the invoke in both fills, and errorData is built before the Operation timed out branch, so both failure branches carry it
2	Success-path log re-probed, so telemetry could diverge from the value GetGitProcess acted on	Probed once and threaded through InvokeGitAgainstDotGitFolder → InvokeGitImpl → GetGitProcess; the second P/Invoke is gone
3	TryGetCertificatePassword is mayRequireAuth: true with no logging	Logged on both paths, with a test
4	The "untestable" catch branch is reachable via MockPlatform	ThrowOnConsoleProbe added, and the test asserts CreateNoWindow == false when the probe throws
5	hasVisibleConsoleWindow intermediate was untrue at its point of use	Removed
6	Pre-existing comment rewrapped at two sites	Reverted
7	Three LF→CRLF lines in MockPlatform.cs	Gone — --numstat and --numstat --ignore-all-space are now byte-identical across all six files
8	MockGitProcess was last-write-wins per command string	InvocationsRun records every call, and MayRequireAuthFor asserts repeated runs agree

Beyond the asks: the test proving a caller-supplied false forces detach without probing the platform is a good addition, and the "Known gaps" section now matches the code.

I built and ran the unit suite at 58edd7c8 locally — 985 passed, 0 failed (the 11 skips are the native-hook tests that need the C++ build). All seven new tests pass in both fixture parameterizations, and CI is green across 35 checks.

That leaves the two points from my review body, which were the blocking part of the last pass.

1. The commit message still carries the claim I measured as incorrect

You corrected the PR description — its probe bullet now accurately says the check keys on the console window and its root owner. But a6c33d23 was not amended, so its message body still reads:

Detection keys on the console window, not an arbitrary GUI window. A process whose console is hidden, or a non-displayed pseudo-console such as a foreground gvfs.exe run from Windows Terminal, is treated as having no visible window and also detaches git.

Measured on Windows 11, a foreground gvfs.exe under Windows Terminal is not detached. GetConsoleWindow() returns a PseudoConsoleWindow, and the GA_ROOTOWNER walk the probe performs resolves to the visible CASCADIA_HOSTING_WINDOW_CLASS host window, so IsWindowVisible returns true and CreateNoWindow stays false:

Scenario	GetConsoleWindow()	GA_ROOTOWNER	Probe
Start-Process -WindowStyle Normal	PseudoConsoleWindow (visible)	CASCADIA_HOSTING_WINDOW_CLASS (visible)	true
Windows Terminal tab	PseudoConsoleWindow (visible)	CASCADIA_HOSTING_WINDOW_CLASS (visible)	true
Hidden console (Mount-like)	ConsoleWindowClass (hidden)	self (hidden)	false

The only false case is a genuinely hidden ConsoleWindowClass — the GVFS.Mount case this PR fixes. Since Windows Terminal is the default host on Windows 11, that paragraph describes the most common interactive path and gets it backwards. The code and its XML doc are correct; only this paragraph is wrong.

This is worth an amend rather than letting it ride, because this repo merges with merge commits rather than squashing, so a6c33d23's body lands on master verbatim and is what git log and git blame will surface. Someone reading it later would conclude the Windows Terminal path is already handled by detaching, and could build on that.

2. Withdrawing my ConPTY question as blocking

I asked what the credential manager parents its prompt to on the non-detached pseudo-console path. Revisiting it: I have no measurement showing an actual defect there, it is not a regression from this change (CreateNoWindow is unchanged on that path), and the reports motivating this fix are all the hidden-mount case. A foreground terminal run also holds foreground rights, which is the real reason the hidden-mount case fails and an interactive run does not. So I am not holding the PR on it — noting it only as something to revisit if interactive-clone prompt reports ever appear.

Approving on that basis: the change is correct, well covered by tests, and green. Please fold the commit-message correction in before merging.