[PHP] Keep OPFS mounts attached when final journal flushes fail - #4003
Merged
Conversation
This was referenced Jul 10, 2026
adamziel
force-pushed
the
file-editor-surface-wiring
branch
from
July 10, 2026 20:33
5540f06 to
6061c98
Compare
adamziel
force-pushed
the
opfs-final-flush-contract
branch
from
July 10, 2026 20:35
d8ba5e8 to
0ba7a39
Compare
adamziel
force-pushed
the
opfs-final-flush-contract
branch
from
July 10, 2026 23:58
0ba7a39 to
3c4cb2f
Compare
adamziel
marked this pull request as ready for review
July 11, 2026 00:00
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates OPFS unmount behavior so mounts remain retryable when the final journal flush fails, and ensures mount tracking is only cleared when the mount is truly detached.
Changes:
- Introduces
MountStillActiveErrorto represent “unmount failed but mount intentionally remains active.” - Moves final flush responsibility into the mount’s
unmount()and adds adiscard()path for setup rollback. - Updates remote endpoint and tests to retain mount tracking only when an unmount reports “still active,” while surfacing the original persistence error.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/playground/remote/src/lib/playground-worker-endpoint.ts | Keeps OPFS mount tracking when unmount signals “still active,” and rethrows the underlying persistence error. |
| packages/playground/remote/src/lib/playground-worker-endpoint.spec.ts | Adjusts tests for new unmount semantics and still-active behavior. |
| packages/playground/remote/src/lib/playground-client.ts | Documents that unmountOpfs leaves mounts attached when flushing fails. |
| packages/php-wasm/web/src/lib/directory-handle-mount.ts | Implements atomic final flush within unmount(), adds discard(), and replays only the failed suffix on partial flush failures. |
| packages/php-wasm/web/src/lib/directory-handle-mount.spec.ts | Adds regression tests for discard() waiting, unmount retryability, and failed-suffix replay. |
| packages/php-wasm/universal/src/lib/php.ts | Adds MountStillActiveError and updates mount registry cleanup rules accordingly. |
| packages/php-wasm/universal/src/lib/index.ts | Re-exports MountStillActiveError. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
adamziel
force-pushed
the
opfs-final-flush-contract
branch
from
July 11, 2026 00:14
3c4cb2f to
5973d59
Compare
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.
Without this PR, a failed journal write drops the entire OPFS mount. With this PR, the mount stays mounted.
While PHP is running, WordPress files live in MEMFS. Playground records each filesystem change—such as writing, renaming, or deleting a file—in a journal and copies those changes to OPFS. That's a journal flush. A flush takes the entries it is about to process out of the live queue, then applies them one at a time. This leaves the live queue free to record file changes that happen while the OPFS writes are running.
Before this PR, Playground never put journal entries back in the queue when one of those writes failed. If the first entry succeeded, the second failed, and the third had not run yet, the second and third entries were simply forgotten.
After a failure, the bubbling exception would trigger unmounting the OPFS directory. Unmount stopped the change recording entirely and made our failure permanent. The OPFS directory was still on disk, but Playground had forgotten the live mount and its pending writes, so the caller had nothing left to retry.
This PR puts the failed entry and every entry after it back in the journal. Entries that already succeeded stay completed; replaying a rename or delete is not always safe. The file-change listener stays active, PHP and the worker keep the mount registered, and
unmountOpfs()rejects with the original failure. CallingunmountOpfs()again retries the pending journal entries.Implementation
unmount()is the normal shutdown path: write every pending change to OPFS, then stop recording changes and unregister the mount.discard()is lower-level cleanup: stop recording changes without starting a final flush, and wait for any flush already running. Failed initial MEMFS-to-OPFS setup usesdiscard()because it never produced a valid mount worth retrying. A successfulunmount()uses the same cleanup only after the journal is empty. That is why both functions exist, and whydiscard()is not public.MountStillActiveErroris the internal signal for “unmount failed, but the mount is still valid.” PHP and the worker keep their registry entries only for that error. The public caller still receives the original flush failure. Other unmount errors clear the registries because they do not promise that the mount remains usable.The public
DirectoryHandleMountAPI remainsflush()andunmount().Testing
The tests cover failure partway through a flush, file changes arriving while a flush finishes, retrying a failed unmount, failed initial setup, and mount-registry cleanup: