fix(dialog): the Windows picker takes the keyboard, not just the z-order - #142
Merged
Conversation
Top-most won the z-order but not the *active* window: the browser stayed foreground, so the dialog opened on top with no keyboard focus — the user had to click it before typing or arrowing around. `SetForegroundWindow` is blocked by the same foreground lock that put the dialog behind the browser in the first place, so the script attaches our input thread to the current foreground thread first (the documented way past the lock), sets the owner form foreground, then detaches. Best-effort in a try/catch: a host that can't compile the P/Invoke still gets the top-most dialog, just unfocused, rather than no dialog at all. That block needs C# string literals, and quoting embedded double quotes through a Windows command line into PowerShell's own parser is the minefield `apps/cli/src/powershell.ts` avoids with a temp `.ps1`. The script now travels as `-EncodedCommand` (base64 UTF-16LE) instead of `-Command` — one quote-proof argv element, no temp file. Newlines come free with it, so the script reads as a script.
The module spec still credited the invisible top-most owner for getting the picker in front — but top-most only wins the z-order. Measured on Windows 11 with an unrelated app in front, that dialog came up unfocused in 3/3 runs: visible, with the keyboard still in the browser. Record what actually works (AttachThreadInput to the foreground thread, then SetForegroundWindow on the owner form), the measurements for both, and that the grab is best-effort by design. Two test assertions to match: the attach/detach calls are paired (a stuck attachment would hand us another app's keyboard state for the picker's lifetime), and the grab stays inside its catch so a host that can't compile the P/Invoke still gets a dialog. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
danyaberezun
requested review from
Olga Lavrichenko (OLavrik) and
Rinat S (rsolmano)
as code owners
July 27, 2026 23:00
danyaberezun
enabled auto-merge (squash)
July 27, 2026 23:13
danyaberezun
disabled auto-merge
July 28, 2026 00:37
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
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.
Follow-up to #141 (merged). The dialog now opens in front of the browser, but without focus — you have to click it before typing or arrowing around.
Why top-most wasn't enough
TopMostwins the z-order; it doesn't make the window active. The browser stays the foreground window, so the folder dialog opens on top of it with no keyboard.SetForegroundWindowis blocked by the same foreground lock that put the dialog behind the browser to begin with — a process that isn't already foreground can't just take it. The documented way past it is to attach our input thread to the current foreground thread first:Best-effort, in a
try/catch: a host that can't compile the P/Invoke (ConstrainedLanguage, no compiler) still gets the top-most dialog — just unfocused, as today — rather than no dialog at all.-Command→-EncodedCommandThat block needs C# string literals, and quoting embedded double quotes through a Windows command line into PowerShell's own parser is exactly the minefield
apps/cli/src/powershell.tsdocuments and avoids with a temp.ps1. The script now travels as-EncodedCommand(base64 UTF-16LE): one quote-proof argv element, no temp file to write or clean up. Newlines come free with it, so the script reads as a script instead of a;-joined one-liner.Verified on Windows 11
Measured on a real Windows 11 host, driving
selectDirectory()from a background Bun process with an unrelated app holding the foreground — the same conditions that make the bug visible:main(#141, top-most owner only)Focus was read with
GetForegroundWindow()against the dialog'shwnd, filtered to a live picker process (so windows left behind by earlier runs can't be mistaken for a fresh one). Keyboard delivery was checked end to end, not just inferred from the focus flag:{ENTER}sent to no particular window returned the highlighted folder ({"path":"C:\Users\danya\Desktop"}), and{ESC}returned{"path":null}at exit 0 — cancel still reads as cancel, not as a failure.Also green:
typecheck,lint, unit tests (10/10 in the dialog suite), and — from #141's run — 115/115 e2e on Linux.Scope
Three files. The two source files, plus
dialog/SPEC.md— taking you up on the offer in the last revision: the spec had credited the invisible top-most owner for getting the picker in front, which the table above shows isn't what does it. It now records the foreground lock, both measurements, and that the grab is best-effort by design, so the next person doesn't re-derive it from the P/Invoke.The unit test decodes the base64 payload before asserting on the script, and pins
AttachThreadInput/SetForegroundWindow($owner.Handle), the paired attach/detach (a stuck attachment would hand us another app's keyboard state for as long as the picker lives), and thecatchthat keeps the grab optional.