Skip to content

[Website] Keep background startup work from racing ZIP imports - #4213

Open
adamziel wants to merge 1 commit into
trunkfrom
adamziel/stabilize-opfs-zip-tests
Open

[Website] Keep background startup work from racing ZIP imports#4213
adamziel wants to merge 1 commit into
trunkfrom
adamziel/stabilize-opfs-zip-tests

Conversation

@adamziel

@adamziel adamziel commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

ZIP imports now declare before boot that they will replace the installed
WordPress files.

The client carries that intent on the remote.html URL. The throwaway
installation never starts automatic static-file backfilling or WordPress
update-check prefetching. After the import, one explicit backfill completes the
imported filesystem before the page refreshes and the initial OPFS copy starts.

No cancellation path or filesystem mutex is needed because the conflicting
work is never scheduled. Existing callers omit the intent, so their boot timing
and callbacks are unchanged.

The OPFS save helper now fills custom site names in one operation. Its previous
clear-then-type sequence could lose leading characters under parallel load and
fail before the ZIP import began.

Testing

  • While viewing a saved Playground with no temporary Playground, import a ZIP
    and confirm the imported site remains available after switching away and
    back.
  • Repeated the saved-site ZIP import 30 times with three Chromium workers. All
    30 runs passed without retries.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Prevents ZIP imports from racing background static-asset backfills and avoids update-check prefetch mutating the SQLite DB during first-boot imports, while also stabilizing dock/panel close callbacks and drag/drop listeners.

Changes:

  • Await static-asset backfills before/after ZIP import steps and dedupe concurrent backfills per PHP instance.
  • Skip WordPress update-check prefetch when first-boot initializer work is pending.
  • Stabilize UI close callback wiring to avoid listener churn during drag renders.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
packages/playground/website/src/lib/state/redux/site-management-api-middleware.ts Adds explicit backfill waits around ZIP import/refresh to prevent path recreation races.
packages/playground/website/src/lib/state/redux/site-first-boot-initializer.ts Exposes a predicate to detect pending first-boot work.
packages/playground/website/src/lib/state/redux/boot-site-client.ts Passes skipUpdateCheckPrefetch when first-boot work is pending.
packages/playground/website/src/lib/state/redux/boot-site-client.spec.ts Asserts the new skipUpdateCheckPrefetch option is propagated.
packages/playground/website/src/components/site-manager/index.tsx Uses a stable close callback reference for panes.
packages/playground/remote/src/lib/worker-utils.ts Dedupes concurrent static backfills via WeakMap + extracts helper.
packages/playground/remote/src/lib/worker-utils.spec.ts Adds a test verifying backfill deduping for the same PHP instance.
packages/playground/client/src/index.ts Adds skipUpdateCheckPrefetch option (documented as internal).
packages/playground/client/src/blueprints-v1-handler.ts Wires skipUpdateCheckPrefetch into update-check prefetch gating.
packages/playground/client/src/blueprints-v1-handler.spec.ts Adds a test verifying update prefetch is skipped when requested.

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

Comment thread packages/playground/client/src/blueprints-v1-handler.spec.ts Outdated
Comment thread packages/playground/remote/src/lib/worker-utils.spec.ts Outdated
Comment thread packages/playground/website/src/lib/state/redux/site-management-api-middleware.ts Outdated
Comment thread packages/playground/website/src/lib/state/redux/site-management-api-middleware.ts Outdated
Comment thread packages/playground/client/src/index.ts Outdated
@adamziel
adamziel force-pushed the adamziel/stabilize-opfs-zip-tests branch from 68e0338 to 9bf944a Compare July 30, 2026 00:55
adamziel added a commit that referenced this pull request Jul 30, 2026
The Saved Playgrounds panel now receives a stable close callback.
Renders
during a drag no longer remove and reinstall its document-level drop
listeners
before the drop.

The separate ZIP startup race remains isolated in #4213.

## Testing

- Repeated the page-drop import through its drag-enter, drag-leave,
drag-over,
  and drop sequence; every drop reached the ZIP importer.
@adamziel
adamziel force-pushed the adamziel/stabilize-opfs-zip-tests branch from 9bf944a to 92c0ad8 Compare July 30, 2026 12:05
@adamziel
adamziel force-pushed the adamziel/stabilize-opfs-zip-tests branch from 92c0ad8 to b8c4ae9 Compare July 30, 2026 13:37
brandonpayton added a commit that referenced this pull request Aug 6, 2026
## Why this change is needed

@adamziel reported that flaky end-to-end tests were causing more CI jobs
to be
canceled.

The canceled jobs shared the same pattern: one storage test failed late
in a
Playwright serial group, so Playwright retried the whole group from its
first
test. In one case, a 33-test storage group restarted more than once and
the job
reached the existing 30-minute limit before it could finish. The OPFS
tests had
the same retry behavior.

Adding another ordinary shard would not help because Playwright keeps a
serial
group together. Increasing the timeout or reducing the three retries
would
also leave the whole-group replay in place.

Examples:

-
https://github.com/WordPress/wordpress-playground/actions/runs/30863319632/job/91858108020
-
https://github.com/WordPress/wordpress-playground/actions/runs/30860081972/job/91848066181

## Why this approach

The storage tests must not run at the same time because they share
browser
storage. A dedicated storage lane with one worker preserves that
isolation.
Using Playwright's default test mode inside that lane means a failure
retries
only the failed test instead of replaying every earlier test in its
group.

This directly addresses the cancellation pattern while keeping the
existing
30-minute job timeout and three retries.

## What this changes

- Changes the OPFS tests and default-storage tests from serial mode to
default
  mode, allowing Playwright to retry one failed test.
- Adds `@storage` to the top-level OPFS suite and the default-storage
group.
  CI routes tests only by this tag, so it does not need file-path rules.
- Keeps three ordinary shards per browser and excludes storage tests
from them.
- Adds one unsharded storage lane per browser with `workers: 1`.
- Runs the Nx CI-style target as two sequential groups: ordinary tests
use the
normal three workers, then storage tests use one worker. Missing or
unsupported
  group values produce a clear error.
- Gives only the three storage matrix entries a `storage` test group.
Entries
without a group use the ordinary-test behavior, so the matrix does not
repeat
  a `regular` label nine times.
- Adds a `-storage` suffix only to storage artifacts. Ordinary artifact
names
  remain unchanged.

The tradeoff is three additional install/build runners, one per browser.
This
is the same job-count increase as changing from three to four ordinary
shards,
but it removes the whole-group retry amplification that caused the
cancellations.

## Verification

- Collected all Chromium tests with the tag-only selector and confirmed
that
each test is in exactly one lane: 182 ordinary tests and 59 storage
tests.
  The ordinary shards contain 61, 61, and 60 tests.
- Confirmed the top-level OPFS suite contributes all 26 tagged tests.
- Confirmed the Nx target resolves to two sequential commands with
explicit
  groups, and a missing or unsupported group exits with a clear error.
- Ran all 59 Chromium storage tests locally with one worker; all passed.
- The full CI run passed for Chromium, Firefox, and WebKit storage
lanes.
- Ran the Chromium storage lane two more times after it first passed.
All three
attempts passed. Two attempts had one flaky ZIP-import test, and
Playwright
retried only that test instead of restarting the storage suite. This
confirms
  the new retry scope works as intended.
- Confirmed the simplified matrix has 12 entries: 9 without a group and
3 with
  the `storage` group.
- Ran the website typecheck, lint, formatting checks, YAML parsing, and
workflow
  validation.

The repetitions also exposed a separate race in two ZIP-import tests. PR
#4213
addresses that underlying race. It remains separate so this PR stays
focused on
making retries safe and preventing CI cancellations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants