Fix Windows E2E: combined status reporting, PHP INI tilde fixes, and per-command CLI shutdown by gavande1 · Pull Request #4082 · Automattic/studio

gavande1 · 2026-07-06T08:19:53Z

Related issues

AINFRA-2588 (Investigate Studio Windows E2E hangs in Buildkite)
Supersedes the split verification work in Fix Windows E2E hangs: restore per-command CLI shutdown and isolate the daemon per home #4075; overlaps with Test restoring per-command CLI shutdown handling #4070 (included as-is) and Isolate the process-manager daemon per home on Windows #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

Have you checked for TypeScript, React or other console errors?

…ows agent

wpmobilebot · 2026-07-06T08:50:34Z

📊 Performance Test Results

Comparing 7125696 vs trunk

app-size

Metric	trunk	`7125696`	Diff	Change
App Size (Mac)	1345.22 MB	1345.22 MB	0.00 MB	⚪ 0.0%

site-editor

Metric	trunk	`7125696`	Diff	Change
load	743 ms	1107 ms	+364 ms	🔴 49.0%

site-startup

Metric	trunk	`7125696`	Diff	Change
siteCreation	6486 ms	6492 ms	+6 ms	⚪ 0.0%
siteStartup	2389 ms	2395 ms	+6 ms	⚪ 0.0%

Results are median values from multiple test runs.

Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff)

os.tmpdir() can resolve to a Windows 8.3 short name (e.g. C:\Users\BUILDK~1\...) when the account name exceeds 8 chars, as on CI's buildkite-agent. That path was handed to PHP as auto_prepend_file (and the phpMyAdmin config), and PHP's parser chokes on the tilde with "syntax error, unexpected '~'", so every site failed to serve and its tests timed out on ERR_CONNECTION_REFUSED. Local dev with a short username produced no tilde, which is why this only reproduced on CI. Add getPhpSafeTmpDir() (long-form tmpdir on Windows) and use it for every temp path handed to PHP, and quote the auto_prepend_file directive to match the others. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vfk7xXMfsABh5JMYX51wiS

fredrikekelund

Good call on abstracting the fs.realpathSync.native() call 👍

#4070 and this are almost identical now – except the additional getPhpSafeTmpDir() calls here, and the toPhpIniPath() call. Landing this PR and closing #4070 and #4075 seems like the best way forward

fredrikekelund · 2026-07-06T09:58:45Z

#4070 and this are almost identical now

I forgot that I also added an affordance to the other PR to trigger the appropriate "stop sites" behavior when quitting the app during E2E tests. If that change isn't needed to make CI green again, though, then we don't need it for now

gcsecsey

Happy to see the E2E tests passing on this.

…ize CI comments

gavande1 · 2026-07-06T10:12:56Z

Thanks for the review. I addressed the inline feedback in ee6a7c0: renamed the helper and removed the redundant comments, and the CI config comments are also gone as of 7125696. I agree with landing this PR as the root fix and will close #4070 and #4075 after it merges; the final CI run on the latest head is in progress.

Copilot

Pull request overview

This PR restores Windows E2E stability by fixing CLI shutdown behavior during app quit, hardening native-PHP temp/INI path handling on Windows (8.3 tilde paths), and making CI status reporting accurately reflect the whole E2E matrix.

Changes:

Switch Studio’s CLI execution back to per-command child process shutdown handling (instead of a shared killAll() on will-quit).
Fix native-PHP temp-path handling on Windows by resolving os.tmpdir() to its long form and quoting/normalizing auto_prepend_file in PHP -d args.
Update Buildkite E2E job reporting to a single combined GitHub status context and ensure canceled jobs fail rather than reporting success.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
apps/studio/src/modules/cli/lib/execute-command.ts	Replaces shared CLI runner usage with a per-command fork + per-child quit cleanup.
apps/cli/lib/native-php/tmp-dir.ts	Adds helper to resolve Windows temp dir short paths to long form for PHP compatibility.
apps/cli/lib/native-php/site-setup.ts	Uses the resolved temp dir for site-url prepend file temp directory creation.
apps/cli/lib/native-php/phpmyadmin.ts	Uses the resolved temp dir for phpMyAdmin env/session paths.
apps/cli/lib/native-php/config.ts	Reuses resolved temp dir for opcache cache dir and quotes/normalizes `auto_prepend_file`.
.buildkite/pipeline.yml	Re-enables Windows E2E, adds timeouts, and moves GitHub status notification to the E2E group for combined reporting.
.buildkite/commands/run-e2e-tests.sh	Adds termination trapping so canceled/timed-out runs exit non-zero.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

gavande1 · 2026-07-06T10:43:13Z

+		// Only callers that opted-in with a `logPrefix` get stdout echoed to
+		// the main-process console. Commands like `preview list --format json`
+		// dump large structured payloads on stdout that would otherwise spam
+		// `npm start` output every time snapshots are fetched.
+		const logPrefix = options.logPrefix ? `[CLI - site ID ${ options.logPrefix }]` : null;
+		child.stdout?.on( 'data', ( data: Buffer ) => {


This restores the pre-#3954 file verbatim, and the same prefix exists in tools/common today. I agree it is worth cleaning up, but as a follow-up rather than widening this revert.

I opened #4108 as the follow-up for this.

gavande1 · 2026-07-06T10:43:15Z

+	function appQuitHandler() {
+		const pid = child.pid;
+		child.removeAllListeners();
+


In this app the prevented quit always ends in app.exit() within seconds, so the stale-listener window is bounded to shutdown. Since this PR intentionally restores the battle-tested pre-#3954 behavior, I would rather keep it verbatim and revisit listener hygiene in the shared createCliRunner as a follow-up.

I opened #4109 as the follow-up for this.

## Related issues - N/A ## How AI was used in this PR Claude cross-checked the auto-generated 1.14.0 changelog against the merged PRs, classifying them as user-facing or internal, and drafting the 1.14.0 wording. I reviewed the changes before raising the PR. ## Proposed Changes - Replace the generated `1.14.0` release-note block with a shorter, user-facing summary. - Grouped the Studio Code changes and folded the shipped dependency bumps into a single "Updated multiple dependencies" line. - Omitted changes not yet visible to users: - the `studio ui` browser surface and `refresh_browser` preview tool - both part of the not-yet-released agentic UI (#3953, #4027) - internal-only PRs (CI/test infra and CI dependency bumps, the Studio apps design doc, release tooling, and internal state/vendor cleanups) - For the Windows E2E PR (#4082), surfaced only the user-facing part: native-PHP sites failing to load on Windows when the profile path uses an 8.3 short name. ## Testing Instructions - Read the `1.14.0` section of `RELEASE-NOTES.txt` - Confirm the groupings and wording read well and are accurate - Cross-check against the merged-PR list for anything important that should be surfaced or reworded ## Pre-merge Checklist - [ ] Have you checked for TypeScript, React or other console errors? --------- Co-authored-by: Fredrik Rombach Ekelund <fredrik@f26d.dev>

## Related issues - Follow-up of #4082 ## How AI was used in this PR Claude wrote the code, guided by the author. The author tested it locally and manually reviewed it before pushing. ## Proposed Changes The stdout echo prefix hard-coded the words "site ID", but `logPrefix` is also used for non-site contexts. For example, the CLI events subscriber passes "events", which produced the misleading line `[CLI - site ID events]`. The prefix is now the neutral `[CLI - <prefix>]`, which matches the format `CliServerProcess` already uses for its own log lines. ## Testing Instructions - Run `npm start` and start a site. CLI stdout lines should be prefixed with `[CLI - <site ID>]`, and events subscriber lines with `[CLI - events]`. ## Pre-merge Checklist - [x] Have you checked for TypeScript, React or other console errors?

## Related issues - Follow-up of #4082 ## How AI was used in this PR Claude wrote the code, guided by the author. The author tested it locally and manually reviewed it before pushing. ## Proposed Changes The quit-time kill paths (`appQuitHandler` in the desktop and `killChild` in the shared runner) called `child.removeAllListeners()` before killing the child, which also removed the `close` handler. Since `will-quit` can be prevented and the app can keep running, this left a stale `will-quit` listener attached and left callers awaiting the command hanging forever. The kill paths now only kill the child and let the `close` handler run: it detaches the `will-quit` listener and emits `failure`, so awaiting callers settle. ## Testing Instructions - Run `npm start`, start a site, and quit the app. Child CLI processes should terminate as before, and the console should show a `failure` warning from the events subscriber instead of silence. - `npm test -- apps/studio/src/tests/index.test.ts` passes. ## Pre-merge Checklist - [x] Have you checked for TypeScript, React or other console errors?

…nt (#4124) ## Related issues - Fixes STU-1871 - Re-adds #3961, reverted in #4029 ## How AI was used in this PR Claude investigated the revert history and the current e2e harness, restored the test from git history, adapted it to the updated harness conventions, and verified all suites locally. I reviewed all changes. ## Proposed Changes PR #3961 added CLI e2e coverage for `studio site create --blueprint`, but was reverted in #4029 as part of the AINFRA-2588 Windows E2E hang investigation. The actual causes were later found and fixed in #4082. The blueprint tests were not at fault, so this PR re-adds them, updated for the harness as it exists today: - The 4 original cases (installTheme, installPlugin, runPHP, wp-cli) now create sites with `--runtime sandbox`, matching the other CLI e2e suites: it keeps runs hermetic, since native PHP would download its binary into the isolated config dir on first run. - Applying a Blueprint boots the process-manager daemon even with `--no-start`, and since #3989 each test env gets an isolated daemon — so teardown now runs `site stop --all` before removing the env, like the start/stop and site-management suites, to avoid orphaning a daemon per case. - Re-adds #3961's harness fix: `runCli` isolates the legacy Electron appdata dir (`E2E` + `E2E_APP_DATA_PATH`). Without it, the Studio-compatibility startup migration finds a developer's real pre-split `appdata-v1.json` and exits 1, failing every CLI e2e suite locally (CI passes only because agents have no legacy config). This also stops e2e runs from bumping production stats. ## Testing Instructions 1. Build the CLI: `npm run cli:build`. 2. Run the new suite: `npm test -- apps/cli/commands/site/tests/create-blueprint.e2e.test.ts --tagsFilter='e2e'` Confirm all four tests pass. 3. Confirm the fast suite is unaffected: `npm test -- apps/cli/commands/site/tests --tagsFilter='!e2e'` — the e2e cases are skipped. 4. Regression check for the harness fix — the sibling suites still pass: `npm test -- apps/cli/commands/site/tests/create.e2e.test.ts --tagsFilter='e2e'` (on a machine with a leftover `~/Library/Application Support/Studio/appdata-v1.json`, this fails without the harness change and passes with it). ## Pre-merge Checklist - [x] Have you checked for TypeScript, React or other console errors?

gavande1 added 2 commits July 6, 2026 13:48

Quote auto_prepend_file so 8.3 short paths don't break PHP's INI parsing

b7be43d

Report one combined E2E GitHub status and fail canceled E2E jobs

3eec23e

gavande1 requested a review from a team as a code owner July 6, 2026 08:19

github-actions Bot assigned gavande1 Jul 6, 2026

Enable Windows E2E with a timeout cap to verify the INI fix on a Wind…

a7e3ffc

…ows agent

gcsecsey and others added 2 commits July 6, 2026 14:27

Restore per-command CLI shutdown handling

40acdb8

gavande1 changed the title ~~Report one combined E2E status and quote auto_prepend_file for 8.3 short paths~~ Jul 6, 2026

fredrikekelund approved these changes Jul 6, 2026

View reviewed changes

gcsecsey approved these changes Jul 6, 2026

View reviewed changes

Comment thread apps/cli/lib/native-php/config.ts Outdated

gavande1 added 3 commits July 6, 2026 15:33

Address review: rename tmp-dir helper, drop redundant comments, final…

ee6a7c0

…ize CI comments

Merge branch 'trunk' into fix-combined-e2e-status-and-ini-quoting

31757df

Drop explanatory comments from CI config

7125696

iangmaia requested a review from Copilot July 6, 2026 10:23

Copilot started reviewing on behalf of iangmaia July 6, 2026 10:23 View session

Copilot AI reviewed Jul 6, 2026

View reviewed changes

gavande1 merged commit 8b39649 into trunk Jul 6, 2026
28 checks passed

gavande1 deleted the fix-combined-e2e-status-and-ini-quoting branch July 6, 2026 10:46

fredrikekelund mentioned this pull request Jul 7, 2026

Force-kill stalled E2E teardown (verify Windows E2E hang fix) #4017

Closed

1 task

gcsecsey mentioned this pull request Jul 7, 2026

Studio 1.14.0 release notes #4107

Merged

1 task

This was referenced Jul 7, 2026

Use a neutral CLI log prefix instead of hard-coded 'site ID' #4108

Merged

Keep CLI child close handlers alive when killing on quit #4109

Merged

gcsecsey mentioned this pull request Jul 8, 2026

Re-add CLI integration test for creating a site from a custom Blueprint #4124

Merged

1 task

gavande1 mentioned this pull request Jul 16, 2026

open_basedir restriction / fatal error on Windows when username is longer than 8 characters (8.3 short name mismatch) #4080

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix Windows E2E: combined status reporting, PHP INI tilde fixes, and per-command CLI shutdown - #4082

Fix Windows E2E: combined status reporting, PHP INI tilde fixes, and per-command CLI shutdown#4082
gavande1 merged 8 commits into
trunkfrom
fix-combined-e2e-status-and-ini-quoting

gavande1 commented Jul 6, 2026 •

edited

Loading

wpmobilebot commented Jul 6, 2026 •

edited

Loading

fredrikekelund left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fredrikekelund commented Jul 6, 2026

gcsecsey left a comment

Uh oh!

gavande1 commented Jul 6, 2026

Copilot AI left a comment

gavande1 Jul 6, 2026

gavande1 Jul 7, 2026

gavande1 Jul 6, 2026 •

edited

Loading

gavande1 Jul 7, 2026

Uh oh!

Labels

5 participants

Uh oh!

Conversation

gavande1 commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related issues

Proposed Changes

Testing Instructions

Pre-merge Checklist

wpmobilebot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Performance Test Results

app-size

site-editor

site-startup

fredrikekelund left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fredrikekelund commented Jul 6, 2026

gcsecsey left a comment

Choose a reason for hiding this comment

Uh oh!

gavande1 commented Jul 6, 2026

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

gavande1 Jul 6, 2026

Choose a reason for hiding this comment

gavande1 Jul 7, 2026

Choose a reason for hiding this comment

gavande1 Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

gavande1 Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

Labels

5 participants

gavande1 commented Jul 6, 2026 •

edited

Loading

wpmobilebot commented Jul 6, 2026 •

edited

Loading

gavande1 Jul 6, 2026 •

edited

Loading