Skip to content

Replace Comlink in Playground CLI with lifecycle-aware RPC - #4251

Draft
brandonpayton wants to merge 14 commits into
trunkfrom
emdash/comlink-replace-and-transcend-8rcji
Draft

Replace Comlink in Playground CLI with lifecycle-aware RPC#4251
brandonpayton wants to merge 14 commits into
trunkfrom
emdash/comlink-replace-and-transcend-8rcji

Conversation

@brandonpayton

@brandonpayton brandonpayton commented Aug 7, 2026

Copy link
Copy Markdown
Member

Motivation for the change, related issues

This is the first, deliberately narrow adoption of a lifecycle-aware RPC
implementation. It moves only Playground CLI's worker boundaries while keeping
the existing @php-wasm/universal package-root RPC implementation and all other
repository consumers unchanged.

The CLI exercises asynchronous worker calls and the synchronous FileLockManager
channel without requiring a coordinated browser/client/remote rollout.

What changed

  • Added an opt-in @php-wasm/universal/playground-rpc entry with ESM, CommonJS,
    declarations, and source maps.
  • Switched the CLI's v1 and v2 Blueprint handlers/workers and run-cli worker
    channel to that entry.
  • Gave the v1 FileLockManager channel its own MessagePort, matching the
    synchronous transport's dedicated-port requirement.
  • Kept the package-root RPC exports, browser client, remote iframe, PHP web
    workers, Studio, Telex, and other consumers on the existing implementation.
  • Added protocol, compatibility, and provenance records under
    packages/php-wasm/universal/.

The new entry uses the marker wordpress-playground-rpc and protocol version 1.
Here, “version” means the on-the-wire envelope format—not the npm package version.
Messages include a session identity, request correlation, kind, and protocol
version. Peers that recognize the marker but use different versions fail
explicitly. Legacy peers use a different marker and do not participate in that
exchange; the separate package entry prevents that pairing in the CLI.

The CLI packages both ends of each adopted channel together, so it upgrades them
as one unit. The new and legacy entries are not wire-compatible and must not share
a channel.

Behavior and compatibility

The adopted CLI paths support asynchronous calls and nested property reads,
receiver preservation, callbacks and callback failures, structured-cloned values,
explicit transfer lists and policies, streams, PHP response values, explicit
release, endpoint termination, and bounded synchronous calls.

Each consumed endpoint owns one session. Release, abort, port closure, or an
observed worker exit rejects pending and future work, detaches listeners, closes
owned ports, and errors returned streams. Repeated cleanup is safe. CLI disposal
awaits remote disposal, explicitly releases proxies, and still terminates every
worker and cleans up server/temp resources if another cleanup step fails.

A final review also closed two nested-lifetime gaps: a synchronous remote proxy can
be re-exposed to a nested worker, and a MessagePort-backed stream can be cancelled
before its producer emits its first chunk.

No CLI call site demonstrated a need for remote assignment, remote construction,
generic proxy marking, automatic finalization, synthetic .bind() behavior, or
per-operation cancellation, so those general-purpose proxy features are not added.

No CLI public command, option, or return shape is intended to change. Telex,
Studio, browser Playground, direct package-root consumers, and separately deployed
iframe/worker peers are unaffected by this staged adoption. wp-env and other CLI
consumers use the new implementation internally when they use this CLI build.

CI failures addressed

The original revision's built-package CommonJS job timed out for every PHP version.
I reproduced that failure from the packed artifacts on Node 22 and traced it to a
valid SharedArrayBuffer crossing Jest's VM realm and failing an instanceof
check. Transport validation now uses intrinsic brand checks for cross-realm
SharedArrayBuffer, ArrayBuffer, Uint8Array, and ReadableStream values, with
a node:vm regression test. The packed suite subsequently passed all PHP versions.

The first narrowed post-push run exposed a separate timing assumption in the sync
endpoint-loss test. It made a blocking call before Node delivered the port-close
event and therefore observed the documented bounded timeout. The test now
subscribes before worker termination and waits for that observable close event
before asserting immediate endpoint rejection.

The original revision's browser RPC failures exercised browser integration that
has been removed from this narrowed PR. An unrelated cookie-expiration assertion
was not changed.

Local validation

  • Universal tests:
    • Node 20.20.2: 226 passed, 4 legacy tests skipped
    • Node 22.23.1: 226 passed, 4 legacy tests skipped
    • Node 24.15.0: 230 passed
    • Node 26.5.1: 230 passed
  • Playground CLI: 13 files, 188 tests passed.
  • Built CommonJS package suite on Node 22: 3 files, 89 tests passed, including
    every packaged PHP version.
  • Bundle checks passed for Node require, Node dynamic import, and Chromium.
  • Packed playground-rpc imports passed in both CommonJS and ESM.
  • Universal and CLI build, typecheck, lint, and self-host packaging targets
    passed.
  • The packed CLI contains the complete repository LICENSE, byte-for-byte.

Fresh CI

The PR was rebased through GitHub's update-branch API onto trunk at
98392c708; no force-push command was used. The rebased head is 3faef7989.

GitHub Actions run 31215115734
completed successfully with no failed jobs. All 26 checks scoped to this rollout
passed:

  • built npm packages and the unbuilt Playground CLI check;
  • build and lint/typecheck;
  • Playground CLI on Ubuntu, macOS, and Windows;
  • asyncify and JSPI file locking on Ubuntu, macOS, and Windows; and
  • all seven asyncify and six JSPI unit shards, including asyncify 1/7.

The rebased run also passed all three regular Chromium shards and the dedicated
Chromium storage lane added on trunk.

The preceding run's Chromium shard 3 had failed before tests because the
Playwright browser download received HTTP 403 AccessDenied from the CDN. A
fresh-runner retry passed both browser installation and the full shard without a
repository change, confirming transient runner/CDN behavior rather than an RPC or
E2E test failure.

Records and artifact boundary

See RPC-COMPATIBILITY.md, RPC-PROTOCOL.md, and RPC-PROVENANCE.md in
packages/php-wasm/universal/ for the detailed boundary and recorded process
history. The provenance record preserves engineering evidence and does not claim
legal clearance.

Because this PR intentionally retains the legacy implementation, it does not make
a repository-wide or package-wide claim that legacy implementation signatures
have been removed. That cleanup belongs to a later migration after the remaining
consumers move.

@brandonpayton brandonpayton left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Note for Codex:
This PR is ridiculously large. We need to do this more gradually. For this PR, let's leave our comlink fork in place and used by most of the repo.

As a first attempt, let's just switch RPC implementations for Playground CLI. If that still seems like too much, we will further limit the scope of this PR.

Let's also try to make this PR as simple as possible without compromising the integrity of the system or its holistic purpose.

@brandonpayton brandonpayton changed the title Replace Comlink with lifecycle-aware Playground RPC Replace Comlink in Playground CLI with lifecycle-aware RPC Aug 7, 2026
@brandonpayton

brandonpayton commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

I narrowed this to the Playground CLI rollout requested in the review:

  • The existing package-root RPC implementation remains in place and is still used
    by every non-CLI consumer.
  • The new implementation is available only through the opt-in
    @php-wasm/universal/playground-rpc subpath.
  • Only the CLI's v1/v2 Blueprint worker boundaries and run-cli use the new
    entry. Browser client, remote iframe, PHP web worker, Studio, Telex, and other
    integrations are unchanged.
  • The v1 FileLockManager now uses a dedicated synchronous MessagePort, as v2
    already did.

The review-focused changes are split into scope restoration, CLI adoption,
documentation, cross-realm transport compatibility, RPC lifetime/stream fixes,
and deterministic CLI cleanup commits. A final audit added regressions for a
sync proxy forwarded through two workers and for cancelling a port-backed stream
before its first chunk.

Final CI run 31143630752
has all 26 RPC/CLI-scoped checks green: packaged and unbuilt integrations,
build/lint/typecheck, CLI on Ubuntu/macOS/Windows, all six file-locking jobs, and
all 13 asyncify/JSPI unit shards. This includes asyncify 1/7, whose earlier
endpoint-loss timing failure is fixed.

The run's sole red job was
Chromium shard 3:
the Playwright Chrome download received HTTP 403 AccessDenied from the CDN and
no test executed. I classified that as runner/CDN infrastructure, not a failure
of this change.

— Codex

@brandonpayton
brandonpayton force-pushed the emdash/comlink-replace-and-transcend-8rcji branch from 1810fb5 to 3faef79 Compare August 7, 2026 20:16
@brandonpayton

Copy link
Copy Markdown
Member Author

Rebased this PR onto current trunk (98392c708) using GitHub's
update-branch rebase operation. No force-push command was used. The new PR head is
3faef7989 and includes the dedicated E2E storage lanes from #4249.

I also followed up on the remaining CI failure:

  • The original Chromium shard 3 failed while downloading Playwright's browser:
    the CDN returned HTTP 403 AccessDenied, and the build/test steps were skipped.
  • I reran that job on a fresh runner with debug logging. Browser installation and
    the full Chromium shard both passed without a repository change.
  • Fresh rebased CI run 31215115734
    completed successfully with no failed jobs. All 26 RPC/CLI-scoped checks pass,
    as do all three regular Chromium shards and the new Chromium storage lane.

The evidence identifies the original failure as transient runner/CDN behavior,
not an RPC or E2E test defect, so I did not add a speculative workflow or retry
change.

— Codex

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.

1 participant