fix(webv2): let the browser-driving scripts run on Windows - #235
Merged
Merged
Conversation
The performance measurement, the accessibility journeys and the project-file journey each start `pnpm exec vite preview` and tear it down with `process.kill(-pid)`. Neither half works on Windows: pnpm there is a `.cmd` shim, which Node refuses to exec without `shell: true`, so all three die with `spawn pnpm ENOENT` before a browser opens. `shell: true` alone would trade that for a worse failure. It puts cmd.exe between the script and vite, so the teardown signals the shell and leaves the server holding the port -- the next run then fails on `--strictPort`, or worse, measures against the stale build the orphan is still serving. Observed while developing this. So spawn vite's own entry under this Node instead, which needs no shell and keeps the child a plain process, and give teardown a Windows branch: `taskkill /T` for the tree, mapping its "no such process" status to ESRCH, which the project-file journey relies on to tell an already-exited server from a real failure. That mapping is what the new test pins. Also renames the journey's `killProcessGroup` seam to `killPreview`: there is no process group on Windows, and the name was the only thing still claiming it. Verified on Windows: the measurement and the project-file journey now pass and leave no listener behind, where before neither started.
Pfannkuchensack
requested review from
JPPhoto,
blessedcoolant and
lstein
as code owners
September 9, 2026 22:06
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.
Summary
Three scripts drive a real browser against a built app: the architecture performance measurement,
the accessibility journeys and the project-file journey. Each starts
pnpm exec vite previewandtears it down with
process.kill(-pid, ...). Neither half works on Windows, so all three failbefore a browser opens:
pnpmon Windows is a.cmdshim, and Node refuses to exec one withoutshell: true.shell: trueon its own would trade that for a worse failure, which is why this does somethingelse. It puts
cmd.exebetween the script and vite, sopreview.pidis the shell's — teardownsignals the shell and leaves vite holding the port. The next run then fails on
--strictPort, or,if it connects anyway, measures against whatever build the orphan is still serving. That is not
hypothetical: it happened while developing this, and produced a confusing chunk-hash mismatch two
runs later.
So this avoids the shell entirely.
scripts/preview-server.mjsstarts vite's own entry under thesame Node — no shim, no wrapper,
preview.pidis the server — and gives teardown a Windows branch,since there are no process groups there:
taskkill /Twalks the tree, and its "no such process"status is mapped to
ESRCH, whichrun-project-file-journey.mjsrelies on to tell analready-exited server from a real failure.
Linux and macOS keep exactly the behaviour they had:
detached: true, group kill, same signals.Related Issues / Discussions
Found while re-measuring
browser-baseline.jsonfor #146, where the performance gate could not bechecked locally at all.
QA Instructions
On Windows, from
invokeai/frontend/webv2:Both pass here and leave nothing listening on 4176/4180 afterwards; on
mainboth stop atspawn pnpm ENOENT.pnpm test:fixturescovers the new module (26 tests).On Linux and macOS the same three scripts should behave exactly as before — that is the half worth
a reviewer's eye, since this PR cannot be verified on Windows and Linux by the same person easily.
pnpm test:accessibility:browsernow starts on Windows and gets as far as the topbar journey,where it times out waiting for an
Invoke frombutton. That is a pre-existing failure this makesvisible for the first time rather than a regression — the script could not run on Windows at all
before, and CI runs
check:architecture, which does not include it. Not addressed here.Review
Agent-assisted. Independent review subagents were not run for this one; it is small and was
verified by execution rather than inspection. The
taskkillstatus-to-ESRCHmapping is the partmost worth a second opinion — it is hand-written, and
preview-server.test.mjspins it (the testwas made to fail on purpose by changing the status code before being trusted).
Compatibility / Rollout
No product code. Developer tooling only; CI runs on Linux and is unaffected.
Checklist
What's Newcopy (if doing a release after this PR)