Skip to content

fix(webv2): let the browser-driving scripts run on Windows - #235

Merged
Pfannkuchensack merged 1 commit into
mainfrom
fix/webv2-preview-scripts-on-windows
Sep 9, 2026
Merged

Pfannkuchensack merged 1 commit into
mainfrom
fix/webv2-preview-scripts-on-windows

Conversation

@Pfannkuchensack

Copy link
Copy Markdown
Member

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 preview and
tears it down with process.kill(-pid, ...). Neither half works on Windows, so all three fail
before a browser opens:

Error: spawn pnpm ENOENT

pnpm on Windows is a .cmd shim, and Node refuses to exec one without shell: true.

shell: true on its own would trade that for a worse failure, which is why this does something
else. It puts cmd.exe between the script and vite, so preview.pid is the shell's — teardown
signals 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.mjs starts vite's own entry under the
same Node — no shim, no wrapper, preview.pid is the server — and gives teardown a Windows branch,
since there are no process groups there: taskkill /T walks the tree, and its "no such process"
status is mapped to ESRCH, which run-project-file-journey.mjs relies on to tell an
already-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.json for #146, where the performance gate could not be
checked locally at all.

QA Instructions

On Windows, from invokeai/frontend/webv2:

pnpm test:performance:build && pnpm test:performance:browser
pnpm test:project-files

Both pass here and leave nothing listening on 4176/4180 afterwards; on main both stop at
spawn pnpm ENOENT. pnpm test:fixtures covers 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:browser now starts on Windows and gets as far as the topbar journey,
where it times out waiting for an Invoke from button. That is a pre-existing failure this makes
visible 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 taskkill status-to-ESRCH mapping is the part
most worth a second opinion — it is hand-written, and preview-server.test.mjs pins it (the test
was 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

  • The PR has a short but descriptive title, suitable for a changelog
  • Meaningful regression coverage added / updated where needed; obsolete tests/code removed
  • Persisted-state and API changes include required migrations / compatibility validation
  • Relevant performance/efficiency opportunities considered; material claims have evidence
  • Material review findings resolved and relevant checks rerun — self-review only, see Review
  • Documentation added / updated (if applicable)
  • Updated What's New copy (if doing a release after this PR)

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
Pfannkuchensack merged commit bfccc6e into main Sep 9, 2026
19 of 20 checks passed
@Pfannkuchensack
Pfannkuchensack deleted the fix/webv2-preview-scripts-on-windows branch September 9, 2026 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant