Skip to content

Let the editor open with a window on Windows - #201

Merged
juanmaguitar merged 1 commit into
trunkfrom
fix/editor-hidden-window-windows
Aug 10, 2026
Merged

Let the editor open with a window on Windows#201
juanmaguitar merged 1 commit into
trunkfrom
fix/editor-hidden-window-windows

Conversation

@juanmaguitar

@juanmaguitar juanmaguitar commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

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: true on 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_WINDOW it sets STARTF_USESHOWWINDOW with SW_HIDE in the new process's STARTUPINFO, and an application that honors nCmdShow when creating its first window starts invisible. Electron apps do, so VS Code and Cursor both. The spawn itself succeeds, the child emits spawn, awaitLaunch returns 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 detached is 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 — windowsHide is a no-op there, and macOS goes through /usr/bin/open rather 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.

  1. Open the app, select a site, and open the Open directory in menu under the site path. It lists Show in Explorer, then Visual Studio Code — if the editor is not listed, detection did not find it, which is a different problem from this one.
  2. Choose Visual Studio Code. It opens with a visible window, showing the site folder.
  3. Repeat through Other application… and pick Code.exe by 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.
  4. Close VS Code, then close the app entirely, and repeat step 2 from a fresh launch. The window appears again — the editor is spawned detached, so this also confirms it still outlives the app rather than being killed with it.

What must not have happened:

  • No console window flashes at any point during steps 2 and 3. That is what windowsHide was 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.
  • No orphaned editor process. After step 4, closing VS Code's window should leave nothing behind in Task Manager under "Visual Studio Code" — the symptom of this bug was precisely a windowless process sitting there, so the old failure is easy to recognise.

To watch the bug fail to reproduce, do step 2 on a build from trunk first: 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 Windows in test/editor-launch.test.cjs. It drives the win32 branch from any machine by injecting platform, the house pattern from win-spawn-patch.test.cjs. I checked it fails on the old code — windowsHide: true was 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 + a start wrapper, or go through shell.openPath? Both would work around the flag rather than remove the thing that was wrong. shell.openPath in 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 .exe which 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.js documents 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:

  • Nothing re-applies windowsHide to this spawn. hideChildWindows() is called only in the four runner child processes; the main process never requires the module. win-spawn-patch.js self-applies only under WPTK_SPAWN_PATCH=1, never touches windowsHide, and its resolveSpawnTarget returns null for a .exe anyway.
  • No console-flash regression at this call site: resolveLaunch produces /usr/bin/open on macOS and the editor .exe elsewhere, not the cmd.exe/grunt.cmd grandchildren the patch exists for.
  • No kill-tree interaction: the child is unref'd and never registered with killChildTree, so detached here is about outliving the app, not signalling a group.
  • The regression test genuinely fails on the old code.

Follow-up 1 — #202 (cross-platform, 🔵): the Windows rule in .github/instructions/code-review.instructions.md states windowsHide: true flatly, 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, 🔵): patchChildProcess deliberately overrides an explicit windowsHide: false, so a future call to hideChildWindows() in the main process would silently reinstate this bug, and no test would catch it — test/editor-launch.test.cjs injects its own spawn, and test/runner-wiring.test.cjs only 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.

@juanmaguitar
juanmaguitar force-pushed the fix/editor-hidden-window-windows branch from ef2f698 to d51aa7a Compare August 9, 2026 13:15
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
juanmaguitar force-pushed the fix/editor-hidden-window-windows branch from d51aa7a to 560b217 Compare August 9, 2026 16:11
@juanmaguitar
juanmaguitar merged commit cf2a735 into trunk Aug 10, 2026
3 checks passed
@juanmaguitar
juanmaguitar deleted the fix/editor-hidden-window-windows branch August 10, 2026 06:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Opening a site in the editor on Windows launches it with a hidden window

1 participant