test(studio): the edit accuracy bench uses the port its server bound

[miguel-heygen]

What

The edit accuracy bench failed a case whenever its Studio port was busy. startServer (edit-accuracy case.mjs) asks the CLI for --port N; when another process holds N, the CLI binds the next free port and announces it, and the bench then threw studio moved from port N to N+1, so the case scored as an error without running. A port that something else was serving on failed the same way earlier (port N is already serving).

startServer now returns { child, port } with the port the CLI announced and answers on, and runOne (run.mjs) opens Studio at that port. The asked-for port is only a starting point.

Why

On a shared bench machine, three rotate cases errored this way in one full run while another process held one job's port; each passed 3 of 3 when rerun alone. A case's verdict must not depend on what else is listening.

Related work

Refs the manual-edit bench work (#4760 added the bench).

How

The CLI binds with a retry on EADDRINUSE and only then prints http://localhost:<port>, so the announced port is the one this server holds. The bench waits for that announcement and for /api/projects to answer on that port, as before. The "already serving" pre-check and the "moved" throw go, since both treated a busy port as a failure.

Test plan

• Bench harness only; nothing in Studio or the player changes.
• server.test.mjs (new): a stand-in CLI binds a port the OS picks, not the one asked for, and announces it. With the old startServer it fails with studio moved from port 1 to <n>; with this change it passes 3 of 3, and it removes its temp dir.
• Real bench, one case (move-none-px-r0-root-z100, --jobs 1 --port 47612) while another HTTP server held 47612: before, ERROR studio moved from port 47612 to 47613 in 1.1 s; after, accurate in 3 of 3 runs (one run flagged for smoothness only, on a loaded machine).

Before

Main's harness, one real case, while another HTTP server holds the asked-for port: the case errors without running.

After

This PR's harness, same held port, three runs: Studio starts on the next port and the case is measured and accurate each time.

Size

Two functions in the bench harness and one test. It ships alone: it fixes a flake in the harness every bench run uses, and no open PR touches these lines.

[jrusso1020]

Reviewed at 73829351: case.mjs (startServer/stopServer and the helpers around them), run.mjs's runOne, the new server.test.mjs, and the CLI's findPortAndServe in portUtils.ts, which the description relies on. Approving.

What I checked:

• The announced port is the bound one. findPortAndServe binds with server.listen(port, host) and moves to the next port on EADDRINUSE, and preview builds its URL from result.port. The bench always passes --force-new, so the already-running reuse branch can't return another project's server. The scan loop prints nothing, so the first http://localhost:N in the log is the real announcement.
• Readiness is checked on that port. up(announced) fetches /api/projects on 127.0.0.1, the same loopback host preview binds by default. Number(undefined) is NaN, so the loop keeps waiting until the URL appears.
• Every caller is updated. run.mjs is the only production caller. It stops served.child on both paths and opens Studio at served.port. liveServers still tracks the child, so killServers covers a server that moved.
• Dropping the "already serving" pre-check is safe. A leftover server on the asked port now only pushes the new one to the next port. It can't be measured by mistake, because the bench talks only to the port that was announced.

Tests I ran:

• server.test.mjs passes 3 of 3 at this head.
• With case.mjs restored to main, it fails with studio moved from port 1 to 34505, so the test discriminates.

Nit (not blocking): announcedPort takes the first localhost URL in the whole log. If preview ever prints another localhost URL before its own, for example a hint about a busy port, the bench would wait on the wrong port and time out after 60 s. It wouldn't mis-measure, because up() has to answer. Matching the last URL, or the exact announcement line, would remove that dependence.

Verdict: APPROVE
Reasoning: The bench now follows the port the CLI actually bound, which the CLI only announces after a successful listen. The new test fails on the old harness and passes here, and nothing outside the bench changes.

— Rames Jusso

[github-actions]

Edit accuracy: accurate 1216 (base branch 1216), smooth 1092 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (1)

• seqnudge-none-pct-r0-nested-z100 (fixed by fix(studio): playhead keyframe edits keep their ease, undo names its edit and never waits on videos #4889): pass, passes

[jrusso1020]

Re-reviewed at b454628c. I approved 73829351, and that commit is an ancestor of this head. Since then, the two commits change only case.mjs (+8). Approving.

What I checked:

• The Windows stop now reaches the server. On win32, signalGroup calls the CLI's own terminateWindowsProcessTree, which runs taskkill /PID <pid> /T /F, instead of process.kill(-pid).
  • Its rejection message ends in status <code>, so /status 128$/ matches only the "process already gone" exit.
  • Any other failure is re-thrown into a voided promise. It surfaces as an unhandled rejection, as the commit says it should.
  • The relative import resolves to packages/cli/src/utils/processTree.ts. The bench runs under bun and the tests under vitest, and both load .ts.
• windowsHide: true goes with detached: true, which on Windows would otherwise open a console window for the server.
• Linux and macOS behave as before. The process.kill(-pid) path is unchanged.
• Signal-safe cleanup still runs. killServers() from run.mjs's exit and signal handlers starts taskkill synchronously, because spawn creates the process before process.exit. It doesn't depend on the promise settling.

Tests I ran:

• At this head, all of tests/e2e/edit-accuracy/ passes: 9 files, 58 of 58 (vitest).
• The Windows branch, simulated. I set process.platform to win32 and put a stand-in taskkill on PATH that SIGKILLs /PID and exits 128 when the PID is gone.
  • startServer → stopServer returned in 234 ms with the child dead, after one taskkill /PID <pid> /T /F.
  • A second taskkill against a PID that was already gone exited 128 and was swallowed.

Nit (not blocking, and on main before this PR): stopServer returns early only on child.exitCode !== null. A server that dies from a signal has exitCode === null and signalCode set, and once("exit") never fires again. So stopServer on it signals twice and then waits on exited forever.

• I reproduced this here. After a SIGKILL to the server, stopServer was still waiting after 8 s.
• The bench hits this if the server crashes from a signal mid-case, for example under the OOM killer.
• Checking child.exitCode !== null || child.signalCode !== null would close it.

Verdict: APPROVE
Reasoning: The delta only gives the bench a Windows way to stop its server, reusing the CLI's existing taskkill helper, and leaves POSIX untouched. A simulated Windows stop ends the server on the first taskkill, and the edit-accuracy suite passes.

— Rames Jusso