Repository navigation
fix(cli): preview --background reports a preview that cannot start instead of waiting it out - #5437
Merged
Merged
Conversation
Contributor
Edit accuracy: accurate 2061 (base branch 2061), smooth 1665 of thoseThe gate passes. Quarantined, measured but not gated (0) |
somanshreddy
left a comment
Contributor
There was a problem hiding this comment.
Reviewed at f5cdcc2e. Clean; one nit.
Checked
- No false "exited" for a healthy preview. The detached child is the CLI itself (
process.execPath+--foreground --no-open --force-new).startedServeralready counts a server as ours only if its PID is the child or a descendant of it, which the ancestry walk can't see once the child has died. So a child that exits before ready was already a failure on main, just after ~10 s. The earlyexitstop changes only the timing and the message. - Skipping the kill is right. On an observed
exit, the dead PID is no longer killed.terminateProcessTreeon a dead PID could only hit a reused one, and any reparented grandchild was already out of reach before this change. preview.ts:s.stop(...)in the embedded-mode catch clears the spinner's timer, so the failed foreground child can actually exit.--jsondoesn't start a spinner, so the guard is correct.
Verification
previewLifecycle.test.ts+preview.startFailure.test.ts: 44/44. The second file needspackages/parsersbuilt first.- All 5 mutants killed:
- drop the
!exitedWith()loop stop - kill on exit anyway
- drop the spinner stop
- drop the log tail
- drop the VT stripping
- drop the
- Required checks are green. No reviews or CRs.
Nit
logTailreads the last 5 lines of a log that every launch for the project appends to. If this child dies after printing fewer than 5 lines, the tail also shows the previous launch's lines, which can mislead.- Fix: record the log size before spawning and tail only what this child wrote.
Review by Somu
jrusso1020
approved these changes
Oct 10, 2026
jrusso1020
left a comment
Collaborator
There was a problem hiding this comment.
Approve at f5cdcc2e. A preview that can't listen now exits, and the launcher reports it as soon as it does, with the real error. A healthy start is unchanged.
Checked
- A healthy preview can't trip the new early exit.
- The spawned child is the CLI itself with
--foreground --force-new, so it never exits early by reusing another server. - Embedded mode serves in-process. Dev and local-studio modes wait on their Vite child (
waitForStudioChildClose), so the child stays alive while Vite serves. - The child exiting therefore means no server is coming. A server already had to be the child or one of its descendants to count.
- The spawned child is the CLI itself with
- An exited child isn't killed again (
startBackgroundPreview), so a reused PID can't be hit. A child that stays up and never answers still gets the old kill and message. - Spinner:
s.stopruns only when not--json, which matches where the spinner is started. - Real runs from this branch on Linux, built from source, as a normal user:
preview --background --port 80 --no-openexits 1 withbackground preview exited (exit code 1) before it was ready; see <log>, followed bylisten EACCES: permission denied 127.0.0.1:80. It took 4.3 s on a cold box, including CLI startup.- A healthy
--port 47731 --jsonstarted (ready: true) in 1.9 s, and--stopstopped it.
- Mutants (each reverted afterwards). Each of these fails one test:
- removing the spinner stop;
- removing
!exitedWith()from the wait loop; - disabling the exited-child error branch.
Nits
- The log is appended by every launch for the project, so the five-line tail can include lines from an earlier run when this one wrote fewer. Recording the log size before spawning and tailing only past it would fix it.
logTailreads the whole log file. It's fine at today's sizes, but the file is never rotated.
Tests: preview.startFailure.test.ts and previewLifecycle.test.ts 44/44. The first file needs the workspace packages built before it loads locally.
— Rames
miguel-heygen
enabled auto-merge
October 10, 2026 22:11
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.
What changes for a user
When
hyperframes preview --backgroundstarts a preview (the built-in studio server that npm installs run) that can't start its server (for example it isn't allowed to listen on the port), the command now says so within about a second, with the real error:Why it waited
The detached preview printed the listen error but never exited: its "Starting studio" spinner was still running, and the spinner's timer kept the process alive. The launcher kept polling for about 10 seconds for a server that would never come, killed the process and gave only the generic message. The real error sat in the log file that every launch for the project appends to.
What changed
preview.ts: when the built-in studio server can't listen, the startup spinner is stopped, so the failed preview exits. The dev and local-studio modes (Vite) have their own startup path and are not changed here.previewLifecycle.ts: the launcher listens for the detached child'sexitevent and stops waiting as soon as it fires. The error names the exit code or signal and appends the last five lines of the log, with terminal control codes stripped. An exited child is not killed again, so a reused PID can't be hit. A child that keeps running but never becomes reachable is handled as before.Checked
preview.startFailure.test.ts: a server that can't listen stops the startup spinner. It fails without the spinner fix (expected "vi.fn()" to be called once, but got 0 times).previewLifecycle.test.ts: a child that writes a bind error and exits is reported at once with its exit and log line, with fewer than five scans and no kill. It fails on main.--port 80as a normal user fails in 0.8 s with thelisten EACCESline, as above.