Repository navigation
CLI: Skip setting admin credentials right after the installer set them - #5512
Merged
Merged
Conversation
chubes4
marked this pull request as ready for review
October 28, 2026 04:04
Collaborator
📊 Performance Test ResultsComparing d0e251a 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) |
1 task done
chubes4
added a commit
that referenced
this pull request
Oct 28, 2026
…metry lets the process exit (#5522) ## Related issues - Related to #5512 and #5506 (site creation timing). ## 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 create` finishes, 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. - The CLI sends a `done` message once a command has finished successfully, from one place after the command handler returns (`apps/cli/index.ts`). - `runCliCommand` resolves 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 start, stop and delete keep finishing on exit for now. Their tail is smaller and they need a separate look. - The process keeps running in the background until the request completes, so no events or stats are lost. - `site create` reports its result before an import finishes, so `runCliCommand` keeps resolving with that result but only once `done` arrives. - The preview manager ignores the new message instead of logging it as an invalid event. Measured with a production CLI build forked over IPC like the app does, with telemetry on, time from `done` to the process exiting: | command | runs | time after `done` | |---|---|---| | `site create` | 5 | 338–421 ms | | `site start` / `stop`, telemetry opted out | 3 each | 7–9 ms | That 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.ts` - The new test resolves a create from a command that reports `done` and never exits. It times out without this change. - Manually, in a packaged build with analytics on: create a site, and the app shows it as created as soon as the CLI reports it. ## 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 each step of
site create, wrote the change and the test, and ran the A/B timings below. I reviewed the diff and the measurements.Proposed Changes
When a site is created, the first start installs WordPress with
wp core install, which creates the admin user with the configured username, password and email. The same start then sends those credentials again to the site's admin API (set_admin_password). That request is a full WordPress load plus awp_check_passwordagainst the bcrypt hash the installer just wrote. On my machine it took 250–290 ms, and it changes nothing.installWordPressnow returns whether it ran the installer.startServerskips the credentials request when the installer ran on this start and no Blueprint ran after it. A Blueprint can change users, so Blueprint creates still apply the credentials afterwards.Measured with the built CLI on macOS, alternating trunk and this branch, from
Checking PHP binary…toWordPress server startedduringsite create:This branch was faster in all 8 pairs (−319 ms median). Local timings on a busy machine, so CI's
siteCreationmetric is the number to trust.Testing Instructions
npm run cli:buildnpm test -- --tagsFilter='e2e' apps/cli/commands/site/tests/create.e2e.test.ts--admin-username navigator --admin-password e2e-created-passwordand signs in towp-login.phpwith them. It fails if the installer doesn't set the credentials the start no longer re-applies (checked by making the installer ignoreadminUsername).site-management.e2e.test.tsstill covers changing the password on an existing site and signing in with it.node apps/cli/dist/cli/main.mjs site create --path ~/Studio/creds-check --admin-password secret123 --skip-browser, then sign in at/wp-adminasadmin/secret123.Pre-merge Checklist