fix(e2e): stop wp-env warm-up JSON parse flakes

[mokagio]

I run into E2E failures from time to time in this repo over the past few weeks. The last time this happened, in the build for PR #533 which I was reviewing, I decided to task an agent to investigate with /goal E2E green consistently. This is the result.

The approach makes sense to me. While one should avoid polling in tests, I think it's acceptable in the context of E2E . But, I'm not familiar withe the Playground / wp-env system that's in use for the tests, so I can't really judge.

I'm sharing this in the hope it helps getting the E2E back to stable, even if just as a starting point.

AI-generated details below.

---

What?

Makes the Web E2E job resilient to wp-env warm-up hiccups, which were failing builds intermittently (e.g. build 2401).

Why?

Every failure in the flaky runs traced to wp-env-fixtures.js, where response.json() threw on a non-JSON body returned by the editor-settings/assets REST endpoints during warm-up — either a PHP notice printed into the response (<br /><b>Warning...) or a transient 500. Editor settings are cached in module scope, but only on success, so a single early hiccup poisoned the first fetch and failed every early spec while later ones passed — the classic flaky signature (7 failed / 41 passed with all failures in editor-load, editor-title, editor-error).

How?

Two complementary layers:

• .wp-env.json: set WP_DEBUG_DISPLAY: false so PHP notices never leak into REST bodies. WP_DEBUG_LOG stays on, so they still land in debug.log.
• e2e/wp-env-fixtures.js: route both fetches through a fetchJson helper that reads the body as text, retries transient 5xx/parse failures, and includes a body snippet in the final error instead of an opaque Unexpected token '<'.

Testing Instructions

This is a draft to exercise CI — the goal is a green (and repeatably green) Web E2E job.

Locally: make wp-env-start then make test-e2e. Before this change the settings-fetch failures reproduce on a cold instance; after, the suite passes (verified: 47/49 passed, the 2 remaining being local-only media-upload timeouts under fullyParallel workers, which pass at CI's workers: 1).

[wpmobilebot]

XCFramework Build

This PR's XCFramework is available for testing. Add the following to your Package.swift:

.package(url: "https://github.com/wordpress-mobile/GutenbergKit", branch: "pr-build/540")

_{Built from ec128f8}

[Copilot]

Pull request overview

Improves stability of the Web E2E suite during wp-env warm-up by preventing PHP notices from corrupting REST responses and by making the fixtures/settings fetch more resilient to transient failures.

Changes:

• Adds a fetchJson helper in e2e/wp-env-fixtures.js that retries transient HTTP/parse failures and surfaces a helpful body snippet on final failure.
• Extends bin/wp-env-setup.sh to wait until the editor settings endpoint returns valid JSON before proceeding to run tests.
• Updates .wp-env.json to disable WP_DEBUG_DISPLAY so PHP notices don’t leak into REST response bodies.

Reviewed changes

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

File	Description
e2e/wp-env-fixtures.js	Routes editor settings/assets fetches through a retrying JSON fetch helper to reduce warm-up flakes.
bin/wp-env-setup.sh	Adds a readiness gate for the editor settings REST endpoint to avoid early warm-up failures.
.wp-env.json	Disables debug display to keep REST responses clean while preserving debug logging.

---

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

[iangmaia]

@claude review this PR.

[claude]

Claude finished @iangmaia's task in 0s —— View job

---

I'll analyze this and get back to you.

[iangmaia]

Claude finished @iangmaia's task in 0s —— View job

I'll analyze this and get back to you.

FYI #533 isn't merged so ofc this won't trigger the new Claude workflow.

[mokagio]

@claude review this PR, now that #533 landed and the workflow is secured.

[claude]

Claude finished @mokagio's task in 0s —— View job

---

I'll analyze this and get back to you.

[mokagio]

@iangmaia looks like the Claude review run but never went past reporting that it started? I had a quick look at the logs, https://github.com/wordpress-mobile/GutenbergKit/actions/runs/28994702797/job/86041799638, but couldn't make much of it. Haven't passed it to my AI for further investigation, but can do that later if you don't have the bandwidth.

[iangmaia]

@iangmaia looks like the Claude review run but never went past reporting that it started? I had a quick look at the logs, https://github.com/wordpress-mobile/GutenbergKit/actions/runs/28994702797/job/86041799638, but couldn't make much of it. Haven't passed it to my AI for further investigation, but can do that later if you don't have the bandwidth.

It seems to be a problem with the token, which doesn't surface properly. See anthropics/claude-code-action#1249.
I'll create a new token and update it in the action secrets.

[iangmaia]

@claude review this PR.

[claude]

Claude finished @iangmaia's task in 1m 16s —— View job

---

PR Review: fix(e2e): stop wp-env warm-up JSON parse flakes

• Read changed files (.wp-env.json, bin/wp-env-setup.sh, e2e/wp-env-fixtures.js)
• Analyze diff vs origin/trunk
• Provide review feedback

---

Overall this is a solid, well-reasoned fix. The root-cause diagnosis (PHP notices / transient 5xx corrupting the first REST body, poisoning module-scoped caching only-on-success) is convincing, and the three layers address it at the right places. The code is clean, the JSDoc is genuinely helpful, and the error messages now surface a body snippet instead of the opaque Unexpected token '<'. Nice work. A few thoughts below — none are blockers.

🟡 fetchJson retries every error, not just transient ones

e2e/wp-env-fixtures.js:78-83 — the catch treats all failures identically, so a genuinely permanent error (e.g. 401/403 from a bad authHeader, or 404 from a wrong route) is retried the full 15× 2s = 30 s before failing. The JSDoc and PR description both say "retry transient 5xx/parse failures," so the code is slightly broader than advertised.

This isn't wrong for CI resilience, but failing fast on non-retryable status codes would make a real misconfiguration surface in seconds instead of after a 30 s hang, and would match the stated intent:

if ( ! response.ok ) {
    const message = `HTTP ${ response.status } ${ response.statusText } — ${ body.slice( 0, 200 ) }`;
    // 4xx (except 408/429) won't fix themselves on retry — fail fast.
    if ( response.status < 500 && response.status !== 408 && response.status !== 429 ) {
        throw new Error( `Failed to fetch ${ label }: ${ message }` );
    }
    throw new Error( message );
}

(A network-level fetch() rejection and a JSON parse failure should still retry, since those are the transient warm-up cases.) Fix this →

🟢 Redundant readiness gating (by design, worth a note)

bin/wp-env-setup.sh:183-208 gates on the settings endpoint returning valid JSON, and fetchJson then retries the same endpoint again. That's intentional belt-and-suspenders and I think it's justified for a flaky suite — but note the setup gate only covers wp-block-editor/v1/settings, not the wpcom/v2/editor-assets endpoint, so the in-test retry still carries real weight for assets. No change needed; just flagging that the two layers aren't fully redundant.

🟢 Minor: duplicated inline Node JSON-parse snippet

bin/wp-env-setup.sh now embeds the same process.stdin … JSON.parse node snippet twice (app-password extraction at ~L128 and the readiness gate at ~L190). Not worth a helper for two occurrences, but if a third appears it'd be worth extracting.

🟢 Nits

• e2e/wp-env-fixtures.js:87 — lastError.message is safe here since MAX_FETCH_ATTEMPTS is 15 (the loop always runs at least once and assigns lastError before this line). Fine as-is; just noting the invariant isn't obvious.
• The 200-char body snippet in error messages could echo part of the editor-settings payload into CI logs. For local/CI test infra against localhost this is a non-issue, but worth keeping in mind if these fixtures are ever pointed at a shared instance.

Testing & standards

• .wp-env.json — WP_DEBUG_DISPLAY: false while keeping WP_DEBUG_LOG: true is exactly the right call: notices stay in debug.log but never leak into REST bodies. 👍
• The polling-in-tests tradeoff the author raised is reasonable for E2E warm-up; bounded retries with a hard cap and a diagnostic final error is the acceptable form of it.

Great targeted fix — my only real suggestion is the fast-fail-on-4xx tweak; everything else is optional polish.

[mokagio]

Thanks for the review — addressed in eeb6fc8 and ec128f8.

🟡 fetchJson retrying every error — fixed. A NonRetryableError marker now short-circuits the loop: a 4xx other than 408/429 fails immediately with the labelled message, while 5xx, network-level rejections, and JSON parse failures still retry. That matches what the JSDoc claimed and drops a bad authHeader or wrong route from a 30s hang to instant.

The three 🟢 items — left as-is, in line with your own "no change needed" framing:

• The readiness-gate overlap is deliberate, and your point stands that bin/wp-env-setup.sh only gates wp-block-editor/v1/settings, so the in-test retry still carries the weight for wpcom/v2/editor-assets.
• The duplicated Node JSON-parse snippet is still only two occurrences.
• The lastError.message invariant is now moot: the non-retryable path throws directly rather than falling through to that line.

On the body-snippet-in-logs note — keeping it. These fixtures point at a local wp-env, and that snippet is the entire point of the original change.

Beyond the review: retry/fast-fail branching is exactly the logic that regresses silently, so it now has 9 Vitest cases (success, 5xx retry, PHP-notice-in-body retry, network rejection, 401/404 fast-fail, 408/429 retry, budget exhaustion). Two things fell out of that:

• e2e/** was excluded from Vitest wholesale, and Playwright's default testMatch also claims *.test.js, so both runners would have fought over the file. Each is now narrowed to *.spec.js. Playwright still collects the same 49 tests in 20 files.
• wp-env-fixtures.js runs readCredentials() at module scope, so importing it anywhere without a provisioned wp-env throws — which broke the JS test job on the first push. fetchJson now lives in e2e/fetch-json.js, free of that side effect. Verified by running the suite with .wp-env.credentials.json removed, reproducing CI's environment rather than trusting mine.

Build is green, Web E2E included.

---

Posted by Claude Code (Opus 4.8) on behalf of @mokagio with approval.