Let the editor open with a window on Windows - #201
Merged
Conversation
juanmaguitar
force-pushed
the
fix/editor-hidden-window-windows
branch
from
August 9, 2026 13:15
ef2f698 to
d51aa7a
Compare
The editor was spawned with `windowsHide: true`, the same option the app uses for its console children, where it stops Windows allocating a visible console for every npm and grunt subprocess. For a GUI application it means something else. The flag fills the new process's STARTUPINFO with "start hidden", and an application that honors that value when it creates its first window starts invisible. VS Code does. The spawn still succeeds, so the app reported the launch as fine and the contributor got no window and no notice — the button did nothing, with nothing to debug. The runners keep the flag: it is applied to them through hide-child-windows.js, which this does not touch. Only the editor loses it, because there the window is the point of the click. Fixes #181 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
juanmaguitar
force-pushed
the
fix/editor-hidden-window-windows
branch
from
August 9, 2026 16:11
d51aa7a to
560b217
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
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.
Why
On Windows, choosing a detected editor from Open directory in appears to do nothing. No editor window, and no error notice either — the app reports success. The editor is in fact running: Task Manager shows Visual Studio Code under Background processes, holding ~400 MB, with no window anywhere.
(Reported against the buttons #158 shipped; #209 has since replaced them with the menu, and the bug is in the spawn underneath both.)
That is the failure shape this project treats as an architectural bug rather than a cosmetic one. A Contributor Day newcomer clicks a button, nothing happens, and there is nothing on screen to diagnose.
What changes
The root cause is
windowsHide: trueon the editor's spawn — the same option the app passes to its console children, where it stops Windows allocating a visible console for every npm and grunt subprocess.For a GUI application the flag means something else. Alongside
CREATE_NO_WINDOWit setsSTARTF_USESHOWWINDOWwithSW_HIDEin the new process's STARTUPINFO, and an application that honorsnCmdShowwhen creating its first window starts invisible. Electron apps do, so VS Code and Cursor both. The spawn itself succeeds, the child emitsspawn,awaitLaunchreturns ok, and the renderer correctly has nothing to report.So the editor spawn drops the flag. Nothing else does: the runners keep it through
hide-child-windows.js, which this does not touch. That patch is applied only inside the four runner child processes and never in the main process, so nothing re-adds the flag to this call site.The comment at the call site was also wrong in a way worth correcting while here — it claimed the options match what main.js uses everywhere else, but
detachedis unconditional here and conditional in the runners, for a real reason: the runners are killed as a process group, and this child is released rather than ever signalled.How to test this
Platform: Windows. The bug does not exist on macOS or Linux —
windowsHideis a no-op there, and macOS goes through/usr/bin/openrather than spawning the editor directly. Buildkite builds a signed artifact for this branch; check it matches the head commit, since rebasing onto current trunk invalidated the earlier ones.Starting state: a Windows machine with Visual Studio Code installed by the user installer, so it lands at
%LOCALAPPDATA%\Programs\Microsoft VS Code\Code.exe. Any site in the app will do; it does not need to be initialized.Code.exeby hand. It opens with a window too — that path reaches the same spawn, and it is the one a contributor with an editor detection misses will take.What must not have happened:
windowsHidewas doing correctly for the runners, and the concern with removing it. Then run Install npm dependencies on a site and watch: still no console flashes there either, since that path is unaffected.To watch the bug fail to reproduce, do step 2 on a build from
trunkfirst: the menu item does nothing, and Task Manager shows Visual Studio Code as a background process with no window.The regression test is
the editor is not asked to start hidden on Windowsintest/editor-launch.test.cjs. It drives the win32 branch from any machine by injectingplatform, the house pattern fromwin-spawn-patch.test.cjs. I checked it fails on the old code —windowsHide: truewas present in the recorded spawn options — and passes on the new. The existing launch-options assertion was updated in the same direction rather than left pinning the old behaviour.Risks and limitations
Self-review came back 0 [fix here] · 2 [follow-up], both filed rather than deferred silently (#202, #203).
I could not test this by hand myself — I have no Windows machine. The reproduction and the confirmation of the running-but-windowless process came from a maintainer's VM; the fix itself has only been verified by the suite and by reading the Win32 process-creation semantics. A reviewer on Windows driving the steps above is what would actually close that gap.
The one behaviour change beyond the fix: a contributor who points the picker at a console-mode editor now gets its console window, where before it was hidden. That is the wanted behaviour — a terminal editor with a hidden terminal is the same bug — but it is a change, not a no-op.
Related
Fixes #181. Follow-ups filed: #202, #203.
Design decisions and alternatives considered
Why not keep the flag and add
shell: false+ astartwrapper, or go throughshell.openPath? Both would work around the flag rather than remove the thing that was wrong.shell.openPathin particular would hand the folder to whatever the OS has registered for a directory, which is Explorer — that is the other button.Why not make it conditional — hide for console editors, show for GUI ones? There is no reliable way to ask a
.exewhich subsystem it targets without reading its PE header, and the app has no business doing that. The simpler rule is correct: this call site launches the contributor's editor, and an editor's window is the point of the click.Why the flag stays for the runners. They exist to collect output that is streamed to the renderer; their console windows are pure noise, and
hide-child-windows.jsdocuments why the patch has to reach grandchildren. Nothing about that reasoning applies to the editor.Review outcome (required — see AGENTS.md)
0 [fix here] · 2 [follow-up] — both follow-ups filed as issues rather than fixed here.
The judgement pass ran in a subagent with fresh context, per
.claude/skills/self-review/SKILL.md. What it verified rather than assumed:windowsHideto this spawn.hideChildWindows()is called only in the four runner child processes; the main process never requires the module.win-spawn-patch.jsself-applies only underWPTK_SPAWN_PATCH=1, never toucheswindowsHide, and itsresolveSpawnTargetreturnsnullfor a.exeanyway.resolveLaunchproduces/usr/bin/openon macOS and the editor.exeelsewhere, not thecmd.exe/grunt.cmdgrandchildren the patch exists for.unref'd and never registered withkillChildTree, sodetachedhere is about outliving the app, not signalling a group.Follow-up 1 — #202 (cross-platform, 🔵): the Windows rule in
.github/instructions/code-review.instructions.mdstateswindowsHide: trueflatly, without the GUI/console distinction that caused this. As written it will give the same wrong advice to the next GUI spawn someone adds.Follow-up 2 — #203 (tests/architecture, 🔵):
patchChildProcessdeliberately overrides an explicitwindowsHide: false, so a future call tohideChildWindows()in the main process would silently reinstate this bug, and no test would catch it —test/editor-launch.test.cjsinjects its ownspawn, andtest/runner-wiring.test.cjsonly asserts the runners do call the patch.One style note from the review was fixed before opening: the comment claiming parity with main.js's other spawn options, described under What changes.
Screenshots or recording
Nothing on screen changed inside the app — the menu, its items and the notice area are untouched. What changes is off-app: whether VS Code's own window appears after the choice.
The before state is visible in #181: Task Manager filtered to "code", showing Visual Studio Code under Background processes (1) with Apps (0) — the editor running with no window.