Skip to content

Spring cleaning: upstream Rook seams, fix the transformed-await gate, harden the OPFS pool - #76

Open
MiguelsPizza wants to merge 1 commit into
mainfrom
alex/spring-cleaning
Open

MiguelsPizza wants to merge 1 commit into
mainfrom
alex/spring-cleaning

Conversation

@MiguelsPizza

@MiguelsPizza MiguelsPizza commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Spring cleaning with three goals: move the runtime behaviour Rook (think-browser-host) still hand-writes upstream, close the coverage gaps an audit of the gating and termination paths found, and fix what those audits turned up. Every bug below was reproduced first, with real workerd as the oracle where it applies.

Bugs fixed

  1. Transformed awaits released the input gate on every await. Under doRuntimeAwaitTransform, storage awaits and plain-value awaits hopped through a MessageChannel and resumed under a fresh input lock, so two concurrent storage-only read-modify-writes lost an update and the implicit transaction committed at every await. workerd holds the gate across both. The fix in src/gate.ts resumes inline when the awaited promise settles while the actor still holds a lock in the critical section that captured it, and hops otherwise. Transformed awaits are about 7x faster as a side effect. The remaining differences from workerd are listed in docs/gating-coverage.md (Transform section). Two new conformance lanes run the whole suite through the transform on Node and Chromium.
  2. The OPFS SAH pool never rolled back a hot journal after a crash. sqlite-wasm's opfs-sahpool VFS answers xCheckReservedLock with "held" unconditionally, so a worker terminated mid-transaction left its uncommitted pages over committed rows on reopen (284 of 2304 rows in the new browser spec). New installSqliteWasmHost() in backends/sqlite-wasm.ts patches that answer for the one-connection-per-file contract this backend has.
  3. A retried pool install could delete the pool. The driver removes the pool directory when installOpfsSAHPoolVfs fails, and a busy worker keeps its sync access handles for about two seconds after terminate(); 3 of 24 retried recoveries came back empty. The same helper waits for the previous owner's handles before installing.
  4. Facet bundles bound 7 of the 12 scope-bound globals. new WebSocketPair() in a facet fell through to the root's installed global, so a facet's frame went out while its commit was pending. facetScopeBanner() now generates the banner from the shared ACTOR_SCOPE_GLOBALS list.
  5. A socket rehydrated after a Worker restart bypassed the output gate. getWebSockets() handed the actor the raw MessagePortWebSocket, so send() and close() did not wait for pending storage writes. The registry now wraps raw sockets in one stable AcceptedWebSocket.
  6. Smaller fixes: withEnvAndExports left a scope installed after a bad second argument; AlarmScheduler raised two unhandled AbortErrors with a reject-on-abort timer; createActorContainer leaked open handles on a later failure; a throwing FacetHost.abort became an unhandled rejection; rowsWritten reported a stale count after DDL; copyFrom could miss a journal appearing during export; MessagePortWebSocket skipped later listeners after a throwing handler and hung when the bridge threw.

Moved upstream from Rook

New in @mcp-b/do-runtime Replaces in Rook
facetScopeBanner({ registry }), ACTOR_SCOPE_GLOBALS the template literal in wxt.config.ts
Vite plugin resolves the two imports it injects ahead of host aliases the @mcp-b/do-runtime/gate alias
@mcp-b/do-runtime/cloudflare-email (EmailMessage in Miniflare's shape) host/shims/cloudflare-email.ts
connectMessagePortWebSocket(bridge, url, fetch) (/browser) host/transport.ts
Offscreen adapter ready() / replaceUnready() the probe loop and rebuild block in background/host/offscreen-document.ts
createBrowserAlarmProjector(), parseBrowserAlarmProjection the projection queue in alarms.worker.ts (which has the same stuck-idle-scheduler bug the review caught here)
installSqliteWasmHost(sqlite3, options) (/backends/sqlite-wasm) the three raw installOpfsSAHPoolVfs sites

Coverage added

  • Conformance suite: 74 → 81 rows per lane, now on five lanes (workerd, node, browser, node-transformed, browser-transformed). New rows: stream callbacks re-entering the actor, throwing blockConcurrencyWhile, sockets after hibernation, fetched bodies gated per chunk (hosts stream two chunks), deleteAlarm, list() options, plain-value awaits holding the gate. Weak rows fixed: the _cf_ refusal now hits _cf_KV, rich values are asserted by content, the fixed sleeps in alarms and hibernation are gone, and the Node host installs the shared actor scope.
  • Browser smoke specs: hibernation across a worker restart, actor crash mid-transaction (journal replay), alarm recovery across restarts.
  • Extension e2e: chrome.alarms-only wake with the service worker killed mid-alarm. Vibe e2e asserts page errors.
  • Unit: 940 → 980.

Maintenance

CI builds only what the e2e scripts don't build themselves; changeset:publish runs changeset publish; sdk:pack writes to .sdk-pack/; @vitest/coverage-v8 and its 14 orphans are gone from the lockfile; docs corrected (decisions 8/20, workerd-sync, gating ledger, migrations, ActorPorts). pnpm install after the lockfile prune also moved chat and @chat-adapter/* to the 4.38.0 the vendored manifests have pinned since #69; the lockfile had drifted.

Behaviour changes to know about

  • serveMessagePortWebSockets takes one form, (bridge, url) => Promise<void>.
  • Bridge close reasons are generic: "No WebSocket route matched" (1011), "WebSocket upgrade rejected" (1008, with the response text when it is printable ASCII of at most 123 bytes), "WebSocket connection failed" (1011). A socket that finishes connecting after stop() closes 1001 "MessagePort transport closed".
  • Code that relied on a storage-only await letting other events in now blocks them, as on workerd.
  • A pool install while another context holds the directory waits up to 10 s, then throws a NoModificationAllowedError ("still held by another context"), instead of failing at once. The three example hosts install through the helper now; vibe's retry loop and sqlite-storage.ts are gone.

Rook adoption

After the release, bump the catalog pin and its minimumReleaseAgeExclude entry together, then:

wxt.config.ts

  1. Line 10: also import facetScopeBanner.
  2. Lines 284-290: replace the template literal with facetScopeBanner({ registry: "__rookFacetScopes" }); keep the /worker/facets/ check. The error text becomes "facet module has no scope named …" (nothing asserts it). Run the e2e: five more names are now bound in facet bundles.

vite-aliases.ts
3. Delete the @mcp-b/do-runtime/gate alias (126-129); the plugin resolves it ahead of aliases.
4. Point cloudflare:email (134) at ${dep("@mcp-b/do-runtime")}/dist/cloudflare-email.js and delete host/shims/cloudflare-email.ts. Rook's shim exposed .raw; the package keeps it under "EmailMessage::raw", and no binding reads it.
5. Keep 92-101 and 130-133: the plugin does not resolve cloudflare:* or async_hooks, and wxt dev pre-bundling only honours aliases.

think-host.worker.ts
6. Delete host/transport.ts. At 1075-1077, inside runWithWorkerActivity, call connectMessagePortWebSocket(socket, message.url, (r) => routeAgentRequest(r, live.env, { onBeforeConnect: withWebSocketUpgrade })) from @mcp-b/do-runtime/browser. No open message is added (the relay's strict schema would close 1002 on it). Delete or update the "Agent socket routing" tests in agent-transport.integration.test.ts, which assert the old reason strings.
7. Line 655: installSqliteWasmHost(sqlite3, { name: "rook_actor", directory: …, clearOnInit: false, initialCapacity: 64 }) and return the result directly. Make the same change in the three test fixtures under offscreen/worker/host/fixtures/browser/ (await-transform.worker.ts:29, web-push-delivery.worker.ts:175, persisted-state-inspection.worker.ts:25): the last one reopens a real actor directory with clearOnInit: false inside a 10 s expect.poll, which is exactly the retried-install hazard, and with the helper the poll becomes one await.
8. Lines 515-522 need no change; the actor now receives the gated wrapper, and the WebSocket.prototype.….call(ws) attachment workaround is no longer needed.

background/host/offscreen-document.ts
9. Pass the probe loop as ready (it already rejects after 5 s) and unresponsiveLongEnoughToRebuild as replaceUnready. ensure() becomes: maintenance check, await networkReady, offscreenDocument.ensure(). Delete offscreenHostPromise and the rebuild block. Keep the 30 s pacing.

alarms.worker.ts
10. Keep the pool and __rook_alarm_projection; the UPDATE-then-SELECT becomes nextGeneration (it must survive Worker restarts and draws must land in order). Create createBrowserAlarmProjector({ nextGeneration, … }), wire projectWake: wakes.projectWake and fire(t) → wakes.acknowledge(t). Delete projection, projectionTail, projectedActiveDeliveries, projectionResult, the waiters list, settleWaiters and rejectWaiters. Line 67: installSqliteWasmHost(sqlite3, { name: "rook_alarms", directory: storageDirectory, clearOnInit: false, initialCapacity: 4 }).pool.
11. browserAlarmProjectionSchema (think-app/contracts/contracts.ts:199-204) is only used by tests, and BrowserAlarmCoordinator validates every projection itself, so replace it with import type { BrowserAlarmProjection } from "@mcp-b/do-runtime/browser/alarm-coordinator" (think-app already depends on do-runtime, package.json:51) and delete fixtures/alarm-bridge.contract.test.ts. No parser re-export is needed. In alarm-worker.integration.test.ts keep one real-worker case; the projector rows are covered by src/browser/alarm-coordinator.test.ts. Keep alarm-facet-reentry.

actor-backup.ts
12. Line 62: installSqliteWasmHost(sqlite3, { name: \rook_backup_${uuid}`, directory: …, clearOnInit: false, initialCapacity: 0 }).pool; pauseVfs, reserveMinimumCapacityand callback-formimportDb` still typecheck.

docs/shim-surface.md
13. Line numbers as on Rook main: delete the host/shims/cloudflare-email.ts row (37). In "Runtime imports" (54), cloudflare:email now aliases @mcp-b/do-runtime/cloudflare-email and the gate import is resolved by doRuntimeAwaitTransform. In "Facet bundles" (63), the banner now comes from facetScopeBanner; keep the row, since codeSplitting: false and keepNames remain Rook's. Update "Agent sockets and routing" (78) and "MV3 offscreen readiness" (79) after steps 6 and 9, and docs/prior-art.md:35, which still names host/transport.ts.

Finally, run Rook's host browser lane (it runs the transform): any test that expected interleaving across a storage await now serialises like workerd.

Follow-ups (not in this PR)

  • Next minor (breaking): drop the ./conformance and ./server/alarm-scheduler subpaths; make WorkerLoader type-only; un-export __gate and markWebSocketUsed; opaque FacetTree; gate WritableStream sinks.
  • AlarmScheduler: a non-abort timer rejection surfaces as an unhandled rejection instead of taskFailure() (pre-existing).
  • rowsWritten for statements with RETURNING still uses the total_changes() delta (pre-existing).
  • Node conformance host: a stream callback pulled by an outside consumer gets the actor lock but not the AsyncLocalStorage context.
  • Report sqlite-wasm's xCheckReservedLock and install-failure cleanup upstream; the helper's // ponytail: comment names the upgrade path.

Verification

Full CI mirror run locally on the final tree (pnpm install --frozen-lockfile, check:oracle, typecheck, test:unit, check:package, the five conformance lanes, the vibe production build, test:examples, sdk:pack): all pass. One workerd run flipped the order of two back-to-back requests in the §1.5 critical-section row; the assertion now accepts either order and rejects any interleaving, and the lane passed three further runs.

Lane Result
unit 981 / 981
conformance workerd / node / node-transformed / browser-transformed 81 / 81 each
conformance browser (suite + smoke specs) 92 / 92
extension e2e all 92 checks
vibe-platform e2e all 16 steps

🤖 Generated with Claude Code

…ait gate, harden the OPFS pool

Bugs fixed, each reproduced first with workerd as the oracle:
- Transformed awaits released the input gate on every await (lost updates
  in concurrent storage-only read-modify-writes; implicit transaction
  committed at every await). src/gate.ts now resumes inline when the
  awaited promise settles while the actor still holds a lock in the
  critical section that captured it, and hops otherwise. Two new lanes
  run the conformance suite through the transform.
- The OPFS SAH pool never rolled back a hot journal after a crash, and a
  retried install could delete the pool. installSqliteWasmHost() waits for
  the previous owner's handles and patches xCheckReservedLock.
- Facet bundles bound 7 of the 12 scope-bound globals; facetScopeBanner()
  now generates the banner from the shared ACTOR_SCOPE_GLOBALS list.
- A socket rehydrated after a Worker restart bypassed the output gate; the
  registry now hands the actor one stable AcceptedWebSocket wrapper.
- withEnvAndExports scope leak, AlarmScheduler abort rejections,
  createActorContainer handle leak, FacetHost.abort unhandled rejection,
  stale rowsWritten after DDL, copyFrom sidecar window,
  MessagePortWebSocket listener skip and bridge hang.

Moved upstream from Rook: facetScopeBanner, plugin resolution of the
injected imports, @mcp-b/do-runtime/cloudflare-email,
connectMessagePortWebSocket, offscreen ready()/replaceUnready(),
createBrowserAlarmProjector + parseBrowserAlarmProjection,
installSqliteWasmHost.

Coverage: conformance 74 -> 81 rows on five lanes; three browser
termination smoke specs; service-worker-kill alarm e2e; weak rows fixed.

Maintenance: CI builds only what e2e does not; changeset:publish;
sdk:pack -> .sdk-pack/; coverage-v8 dropped (the reinstall also moved
chat/@chat-adapter to the vendored 4.38.0 pin and sourcemap-codec to
1.6.0 inside trace-mapping); docs corrected.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant