Skip to content

[PHP] Keep OPFS mounts attached when final journal flushes fail - #4003

Merged
adamziel merged 1 commit into
trunkfrom
opfs-final-flush-contract
Jul 11, 2026
Merged

[PHP] Keep OPFS mounts attached when final journal flushes fail#4003
adamziel merged 1 commit into
trunkfrom
opfs-final-flush-contract

Conversation

@adamziel

@adamziel adamziel commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

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. Calling unmountOpfs() 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 uses discard() because it never produced a valid mount worth retrying. A successful unmount() uses the same cleanup only after the journal is empty. That is why both functions exist, and why discard() is not public.

MountStillActiveError is 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 DirectoryHandleMount API remains flush() and unmount().

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:

npm exec nx -- run-many -t test,typecheck,lint \
  --projects=php-wasm-universal,php-wasm-web,playground-remote

npm exec nx -- run-many -t build \
  --projects=php-wasm-universal,php-wasm-web,playground-remote

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.

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 MountStillActiveError to represent “unmount failed but mount intentionally remains active.”
  • Moves final flush responsibility into the mount’s unmount() and adds a discard() 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.

Comment thread packages/playground/remote/src/lib/playground-worker-endpoint.ts
Comment thread packages/php-wasm/web/src/lib/directory-handle-mount.ts
@adamziel
adamziel force-pushed the opfs-final-flush-contract branch from 3c4cb2f to 5973d59 Compare July 11, 2026 00:14
@adamziel
adamziel merged commit 20a82d3 into trunk Jul 11, 2026
53 checks passed
@adamziel
adamziel deleted the opfs-final-flush-contract branch July 11, 2026 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment