Repository navigation
Studio: Finish site creation when the CLI reports done, not when telemetry lets the process exit - #5522
Merged
Conversation
… lets the process exit
Collaborator
📊 Performance Test ResultsComparing e5466d2 vs trunk app-size
site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff) |
chubes4
changed the title
Merged
1 task done
chubes4
added a commit
that referenced
this pull request
Oct 28, 2026
… its process to exit (#5524) ## Related issues - Follows #5522. ## How AI was used in this PR Claude (via the Pi coding agent in a Roadie session) traced the start/stop timing, wrote the change and the test, and measured the CLI message ordering below. I reviewed the diff and the measurements. ## Proposed Changes Starting or stopping a site from the app resolves when the `site start`/`site stop` process exits. After finishing, both commands send a Tracks event, so the status button keeps its spinner while the request completes. Since #5522, the start/stop could resolve on the CLI's `done` message instead, but that loses a race. The site stays "transitioning" until the event releasing its start/stop operation arrives, and that event reaches the app in the same millisecond as `done` (8 of 8 runs). When `done` wins, the app sees the operation still held and refetches the whole site list. In a CI run with only that change, `siteStartup` went from 1,594 to 1,868 ms (build 24349). This PR removes the race instead of waiting it out: - `site start` and `site stop` report the site after the operation is released, with its running state. - `startSite`/`stopSite` resolve with that site when the command reports `done`, and the app writes it into the cached list the same way it applies a site event. - With nothing to show, such as a site serving its PHP error page or a stop that failed, the app reads the list as before. - This replaces the check for whether the list already showed the site, so start/stop no longer depend on the order of events and command results. Measured with a production CLI build forked over IPC: | | | |---|---| | reported site after start / already running | `running: true`, no operation | | reported site after stop / already stopped | `running: false`, no operation | | failed stop | no result, rejects as before | | time from `done` to exit, telemetry on (from #5522) | start 161–174 ms, stop 165–326 ms | That tail is the time the app no longer waits after a start or stop. It depends on the user's connection. CI skips Tracks, so CI's performance metrics should stay flat for `siteStartup`. ## Testing Instructions - `npm test -- apps/ui/src/data/queries/use-sites.test.tsx` - The updated test starts and stops a site whose release event hasn't arrived yet. It shows the reported site without refetching the list, and fails without this change. - Manually, in a packaged build with analytics on: start and stop a site, and the status button settles as soon as the site is running or stopped. ## Pre-merge Checklist - [x] Have you checked for TypeScript, React or other console errors?
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.
Related issues
How AI was used in this PR
Claude (via the Pi coding agent in a Roadie session) traced where the app waits on the CLI, wrote the change and the tests, and measured the timings below. I reviewed the diff and the measurements.
Proposed Changes
The app treats a CLI command as finished when its process exits. After
site createfinishes, it sends a Tracks event, and Node keeps the process alive until that request completes. So the create flow waits on a telemetry request after the site is ready.donemessage once a command has finished successfully, from one place after the command handler returns (apps/cli/index.ts).runCliCommandresolves on that message, and still on exit for anything that doesn't send it. Failures still wait for the exit and reject with the command's error.site createreports its result before an import finishes, sorunCliCommandkeeps resolving with that result but only oncedonearrives.Measured with a production CLI build forked over IPC like the app does, with telemetry on, time from
doneto the process exiting:donesite createsite start/stop, telemetry opted outThat is the time the app no longer waits. It depends on the user's latency to WordPress.com; these numbers are from a fast connection. CI's performance metrics won't show it, because Tracks requests are skipped in E2E runs.
Testing Instructions
npm test -- packages/common/lib/tests/cli-process.test.tsdoneand never exits. It times out without this change.Pre-merge Checklist