Extract the CLI spawn helper to @studio/common - #3954
Merged
Conversation
Moves the fork/spawn mechanics behind `createCliRunner` in @studio/common/lib/cli-process so non-desktop hosts can fork the CLI the same way; the desktop's execute-command now delegates to it and re-exports the same surface. No behavior change. First slice of splitting the studio ui PoC (#3953). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
youknowriad
commented
Jun 25, 2026
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
📊 Performance Test ResultsComparing 5237439 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) |
This was referenced Jul 3, 2026
Closed
gavande1
added a commit
that referenced
this pull request
Jul 6, 2026
…per-command CLI shutdown (#4082) ## Related issues - AINFRA-2588 (Investigate Studio Windows E2E hangs in Buildkite) - Supersedes the split verification work in #4075; overlaps with #4070 (included as-is) and #4061 (its tilde fix is included; its daemon isolation is not) ## Proposed Changes Windows E2E has been broken since June 29 by two independent regressions that merged the same day. This PR carries the minimal combination that fixes both, plus CI reporting fixes so failures can't hide: **1. Per-command CLI shutdown handling (from #4070).** #3954's shared `killAll()` on `will-quit` runs one listener after the quit handler that spawns `site stop --all`, killing the stop command before it stops any site. Sites leaked into the machine-global process-manager daemon until its capacity cap was exhausted — the 3-hour hangs. Restoring per-child quit handlers lets the quit-time stop survive; verified on CI: quit-time stops complete in under a second (previously a 20-second timeout on every session) and zero capacity errors. **2. PHP INI tilde fixes.** #3988 passed the site-url prepend file (reprint's runtime: constants + SQLite loader) to PHP as an unquoted `-d auto_prepend_file=` value. On machines where the temp path contains a Windows 8.3 short name (e.g. `C:\Users\BUILDK~1\...` — any username over 8 characters), PHP's INI parser fails on the `~` (`syntax error, unexpected '~'`), keeps only the prefix, and every request dies with `Fatal error: Failed opening required 'C:\Users\BUILDK'` before WordPress boots. This broke every page load of every native-PHP site on affected machines and caused the ~25 Windows E2E failures. Fixed at both ends: - `auto_prepend_file` is now quoted and backslash-normalized via the existing `toPhpIniPath()`, like every other path directive. - `getPhpSafeTmpDir()` resolves the Windows short name to its long form for every temp path handed to PHP (opcache dir, phpMyAdmin config/sessions, site-url prepend dir). **3. Honest CI reporting.** The mac/Windows/Linux E2E jobs all posted to the same "E2E Tests" GitHub status and the last writer won, so a fast green mac job masked a failing or still-running Windows job. The notify now lives on the E2E group: one status, pending until every platform finishes. Also: `run-e2e-tests.sh` traps termination so a canceled/timed-out job can't record exit status 0 and turn the build green (observed in build 18744). **4. Windows E2E re-enabled** with a 100-minute job cap. Prior verification of these fixes together (#4075, build 18789): 47 passed / 0 failed / 26 minutes — the first green Windows E2E since June 29. This PR's own CI re-verifies the combination as extracted here. Deliberately not included, pending their own review: #4041 (bounded daemon socket requests, daemon force-settle, leaked-daemon reaping) and #4061's per-home daemon isolation. This PR's CI run doubles as the experiment showing whether they are required for green E2E or are hardening. ## Testing Instructions - **CI**: all three E2E platforms should pass; the "E2E Tests" GitHub status stays pending until mac, Windows, and Linux all finish, then reports one combined result. The Windows job should show no `syntax error, unexpected '~'` in daemon logs, no `site stop --all command timed out` lines, and no `CAPACITY_LIMIT_REACHED` errors. - **Unit**: `npm test -- apps/cli/tests/ apps/studio/src/tests/` passes. - **Manual (Windows)**: on a machine whose user profile path gets 8.3-mangled (username over 8 characters), create and open a native-PHP site — pages render instead of a PHP fatal. ## Pre-merge Checklist - [x] Have you checked for TypeScript, React or other console errors? --------- Co-authored-by: Gergely Csecsey <gergely.csecsey@automattic.com>
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
2 participants
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.
Proposed Changes
Pure refactor, no behavior change. Moves the CLI fork/spawn mechanics into
@studio/common/lib/cli-process(createCliRunner) so hosts other than the desktop can fork the Studio CLI the same way. The desktop'sexecute-command.tsnow delegates to it and re-exportsCliCommandError/CliCommandResult, so existing imports are unchanged.This is the foundational slice of splitting the
studio uiproof-of-concept (#3953) into reviewable pieces — the agent run-manager and site-operations extractions build on this.Testing Instructions
npm run typecheckis clean fortools/common,apps/studio,apps/cli.Pre-merge Checklist