Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 55 additions & 1 deletion docs/adr/2026-07-08-per-project-library-locking.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Per-project library locking: two lock kinds, one ordering rule

- Status: accepted
- Status: accepted, amended 2026-08-14 (locks guard local OPFS
consistency only, never network IO — see the Amendment section)
- Date: 2026-07-10 (planned 2026-07-08)

## Context
Expand Down Expand Up @@ -100,3 +101,56 @@ now per-project in blast radius.
- Empty directory husks (the flusher removes files, never directories)
are swept at the end of catalog transactions, while the catalog lock
is still held.

## Amendment (2026-08-14): locks never span network IO; sync works from a snapshot

The decision above says what each lock *guards* and left what a holder
may *do* implicit. Cloud sync then read the guarantee it needed — single
writer over `/history/<uid>/**`, which it writes `/cloud-binding.json`
into — and took `lp-project:<uid>` for the whole trip, network included.
Example seeding publishes at 0 ms, so the first click on a fresh example
raced its own publish and was told `This project is open in another tab`
with one tab open
([defect](../defects/2026-08-14-sync-holds-the-project-lock-across-the-network.md)).

That is worse than slow. This model deliberately makes a project lock's
*refusal* a user-facing claim — the "open in another tab" answer is the
refusal, by design — so a hold whose length is set by somebody else's
latency does not merely delay the UI, it makes the product state
something false.

**A project lock guards local OPFS consistency and nothing else, and is
therefore never held across network IO.** A caller whose work is mostly
elsewhere takes the snapshot under the lock and does the elsewhere-work
outside it. Concretely, a cloud sync trip is three phases:

1. **Snapshot** — acquire (polling: the hold being waited out is local
work now, so waiting is cheap and refusing is wrong), mount the
project's package and history subtrees, release. Memory-primary
mounting already reads the whole subtree, so the mount *is* the
snapshot at no extra cost. It must be taken inside the one hold: a set
read across two holds could pair a pre-save package with a post-save
history and publish a version that never existed.
2. **Publish** — the round trip, from the snapshot, with no lock held.
3. **Bank** — reacquire briefly and flush what the trip wrote, or, when
the project was opened in this tab meanwhile (a first click landing
mid-publish — the ordinary case), replay those writes into the open's
live store, which owns those files now.

Consequences of the fold:

- A project that changed while its publish was in flight is not a
conflict and not an error: the trip that is running stands, and the
change earns another trip. `SyncQueue` already worked this way ("one
attempt against a consistent snapshot"); the snapshot is now literal.
- The write-back lands only the paths the trip dirtied — whole-file
atomic writes, the same property that makes lock-free reads safe — so a
save that arrived mid-publish is not clobbered by the stale copy.
- Deleting or renaming a project no longer waits out a publish.
- The open path's compensation for this tab's own trip
(`await_sync_handoff`) drops from a round-trip-shaped 3 s to a
local-work-shaped 500 ms, with the acquire ladder behind it.

The rule generalizes past sync: **take the snapshot under the lock, do
the foreign-latency work outside it.** Anything that wants a project lock
across an await it does not control is asking the wrong question.
18 changes: 18 additions & 0 deletions docs/adr/2026-07-16-preview-host.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,21 @@ instead of reshaping the service contract.
- A future GPU fluid solver (wanted for CPU-bound sims) will change
per-slot cost assumptions; the scheduler's budgets are config for
that reason.

## Amendment (2026-08-14, first-click open resilience)

Three boot-time behaviors this ADR described changed; the steady-state
design above is untouched.

- **Pool members boot one at a time from a `Pending` state**, not all at
construction — and never while a user-initiated open is in flight
(`app::open_priority`). Preview boots yield to the click.
- **`Dead` is no longer terminal for the page**: a dead member revives
lazily on demand after a cooldown, within a bounded exponential
budget (see `slot_policy::dead_worker_next`). Budget-exhausted Dead
is final, with the retries named in the reason.
- **Workers no longer fetch or compile the engine wasm themselves**: the
page compiles one shared `WebAssembly.Module` and posts it to each
worker, which only instantiates (boot protocol v2 —
`2026-08-14-browser-worker-boot-protocol-v2.md`). The per-worker
WebGPU device request at boot is unchanged.
119 changes: 119 additions & 0 deletions docs/adr/2026-08-14-browser-worker-boot-protocol-v2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
# Browser-worker boot protocol v2: shared module, phase statuses, inactivity timeout

- Status: accepted
- Date: 2026-08-14
- Plan: lp2025/2026-08-14-1859-first-click-open-resilience (P5)

## Context

Every browser worker (the sim plus each preview-pool member) boots the
same fw-browser engine binary. Under protocol v1 each worker fetched and
compiled its own copy from `/pkg/fw_browser_bg.wasm` inside `boot()`,
and the host waited out a flat, inline 5-second budget (`0..200` polls
× 25 ms) that silently included the whole network fetch. A fresh
/explore page plus one click raced three concurrent multi-MB downloads
against that fixed timer; on a slow or cache-disabled connection the
fetch alone exceeded it, every worker stuck at `booting`, and the
failure surfaced as `timed out waiting for browser worker boot; last
worker status was booting` — the diagnosed core of the first-click
demo failures (see `docs/defects/`, first-click entries).

## Decision

Three coupled changes, one protocol bump (internal boundary — the
worker script ships in the same build as the host; no cross-version
compatibility is promised, matching the repo's wire-compat posture
during heavy development).

### 1. Page-side shared module (`engine_cache`)

The page fetches the engine wasm ONCE (`engine_cache.js`: streaming
read with byte progress, then `WebAssembly.compile`), caches the
compiled `WebAssembly.Module` in a thread-local for the page lifetime,
and delivers it to each booting worker by `postMessage` structured
clone in a raw `boot_module` message (a `Module` cannot ride the serde
envelope path — same reasoning as `attach_surface`). Workers only
instantiate. Concurrent demands share one in-flight promise; a failed
attempt is evicted so the retry ladders (sim connect, preview Dead-pool
revival) drive refetches.

The `Boot` envelope carries `module_delivery`:

- `"message"` — a `boot_module` message accompanies the boot. The
worker tolerates either arrival order (the boot handler awaits a
waiter list the `boot_module` handler resolves).
- `"path"` — v1 behavior, per-worker fetch+compile from the URL. This
is the standing fallback whenever page-side compile fails, and the
reason the path branch is kept tested rather than deleted.

### 2. Phase statuses

The worker posts a status envelope at every boot phase transition:

```text
booting → instantiating → gpu-init → runtime-create → ready
```

(plus the existing `error` / `fatal`). These strings are protocol, not
logging: the host's timeout and the opening-frame UI both key off them.
`booting` covers the glue-JS import (and, under `"message"` delivery,
waiting for the module message); `instantiating` covers
`wasm-bindgen`'s init (under `"path"` delivery this includes the
worker-side fetch — the long pole); `gpu-init` is the one-per-worker
WebGPU adapter+device request; `runtime-create` is first runtime
creation and output drain.

### 3. Inactivity-based timeout (`boot_wait`)

The host no longer bounds TOTAL boot time. `BootWaitClock` (pure,
native-tested) fails a boot only when no status CHANGE has been
observed for the current phase's budget:

- `BOOT_PHASE_INACTIVITY_MS` = 20 s for every phase, generous because
activity resets it — a dead worker posts nothing and still fails in
seconds of quiet, while a slow-but-alive phase keeps going.
- `BOOT_PATH_INSTANTIATE_INACTIVITY_MS` = 120 s for `instantiating`
under `"path"` delivery only, because that phase contains an
unbounded, progress-silent network fetch.

A re-posted unchanged status is NOT activity (the sticky fatal re-post
must not keep a boot alive). Idle time accumulates in nominal poll
intervals, so browser timer throttling stretches rather than shortens
the budget. Timeout errors name the quiet phase and lapsed budget.

Page-side fetch progress is observable via `engine_asset_phase()`
(`Idle → Fetching{received, total?} → Compiling → Ready | Failed`);
`total` is reported indeterminate for content-encoded responses whose
Content-Length would not match streamed bytes. The opening-frame UI
renders this directly; `warm_engine_cache()` lets the app shell start
the fetch at page load, before any worker exists.

## Alternatives considered

- **Bigger flat timeout.** Still a guess racing an unbounded network;
makes real failures take that long to surface. Rejected.
- **`<link rel="preload">` only.** Warms the HTTP cache (and Chrome's
compiled-code cache) but keeps N compiles on cold paths and gives the
UI no progress signal. Kept as a complement (the app shell warms the
cache), insufficient alone.
- **Sharing one worker between sim and previews.** Rejected previously
for blast-radius reasons (preview-host ADR 2026-07-16); unchanged.
- **Transferring the module.** Structured clone of a
`WebAssembly.Module` shares compiled code without a transfer list;
there is nothing to transfer.

## Consequences

- Cold page: one fetch + one compile serve all workers; worker boots
are CPU-bound and fast, and slow networks show progress instead of
dying at 5 s.
- The fallback path (`"path"`) survives page-side compile failure and
carries the generous fetch budget, so v1 behavior remains reachable
and tested.
- The status vocabulary is load-bearing for `boot_wait` and the
opening-frame UI; renaming a phase is a protocol change and must
update both plus this ADR.
- An exhausted sim retry ladder is no longer bounded by attempts × 5 s;
each attempt fails only on genuine inactivity, so ladder worst case
is attempts × phase budget. Acceptable: quiet means dead, and honest
progress never burns the ladder.
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
---
status: fixed
found: 2026-08-14 # how: live-debugging (demo repro, deployed site)
fixed: this change # P1 of the first-click-open-resilience plan
area: lpa-studio-web library_host_opfs + lpa-studio-core project_controller
class: lifecycle-ownership
related:
- ../adr/2026-07-08-per-project-library-locking.md
- 2026-08-14-sync-holds-the-project-lock-across-the-network.md
- 2026-08-14-worker-boot-timeout-races-the-wasm-fetch.md
---
# An open that fails after acquiring keeps the project lock forever

**Symptom** — Clicking a gallery card failed, and every later click on
that same project then failed differently: `This project is open in this
tab — close it before changing it`, with nothing open. No amount of
retrying helped; only a page reload cleared it. In the live demo the
first failure was a worker boot timeout (`timed out waiting for browser
worker boot`), so the two defects presented as one unrecoverable dead
end.

**Root cause** — Opening a project is two halves owned by two layers.
`OpfsLibraryHost::open_project` (`lpa-studio-web/src/library_host_opfs.rs`)
does the first half: acquire `lp-project:<uid>`, mount the package and
history subtrees, spawn their write-behind flushers, and register the uid
in the host's open map. The second half is the caller's — migrate the
package if needed, read every file, push it to the runtime
(`ProjectController::open_opened_package`) — and it can fail at each
step.

The lock's release path ran only through `close_project`, and the only
thing that ever queued a close was `context.active`: the controller
pushes the *previously* active project onto `pending_close` when a new
one becomes active. A project whose open failed never became active, so
no close was ever queued for it, so `close_project` never ran. The lock
(plus the mount and two flush loops) then lived as long as the page.
Worse, the leaked registration is what the *next* attempt trips over:
`open_project` refuses a uid already in its own open map with
`OpenInThisTab`, which is why the symptom mutates from "boot failed" to
"open in this tab" and never recovers.

A second instance of the same shape sat inside the host itself: the uid
was parsed into a `PrefixedUid` *after* the registration, so a malformed
uid leaked in exactly the same way.

**Fix** — `OpenedProject` now carries an `OpenReceipt`
(`lpa-studio-core/src/app/library/library_host.rs`): an RAII drop guard
holding the host's teardown. Committed once the project reaches
`context.active`; dropped uncommitted — which every `?` between the two
halves does — it runs `OpenRegistry::release_open`, the one named
teardown that `close_project` also awaits (unregister, stop flushers,
flush, release the lock, ping other tabs). The uid parse moved ahead of
the acquire, so that failure now refuses before any lock exists. Hosts
that hold nothing hand back `OpenReceipt::nothing_to_undo`.

**Regression coverage** —
`a_failed_open_gives_the_project_back_to_the_library`
(`lpa-studio-core/src/app/studio/studio_edit_e2e_tests.rs`): a below-floor
package refused by the open pre-flight must leave
`MemoryLibraryHost::abandoned_projects` naming it, with no close ever
queued; the happy-path open test now also asserts the receipt was
committed rather than dropped. Lock-level: `a_failed_open_leaves_the_
project_reopenable` in `lpa-fs-opfs/tests/library_locks.rs` (browser gate
only — `just check test` compiles no wasm32).

**Lesson** — When a resource is acquired in one layer and the operation
it guards completes in another, the release cannot hang off the
*success* state. `pending_close` is derived from "what is active", which
is a fact that only exists once everything worked; every failure path
therefore falls outside it by construction, and no amount of care at the
individual call sites fixes that (there were three, and P6's supersede
would have added a fourth). The shape that does fix it is a receipt the
second layer must either commit or give back, so that `?` is already
correct — and one named teardown both endings call, so a superseded open
later cannot invent its own.
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
---
status: fixed # registration race in P1; the hold itself in P2
found: 2026-08-14 # how: live-debugging (demo repro, deployed site)
fixed: 2026-08-14 # P2 of the first-click-open-resilience plan
area: lpa-studio-web cloud/sync/sync_engine + library_host_opfs
class: lock-held-across-foreign-latency
related:
- ../adr/2026-07-08-per-project-library-locking.md
- 2026-08-14-post-acquire-open-failure-leaks-the-project-lock.md
---
# Cloud sync holds a project's lock across a network round trip

**Symptom** — First click on a freshly seeded example, signed in, on a
slow connection: `This project is open in another tab — close it first`,
with exactly one tab open. Clicking again a few seconds later worked.

**Root cause** — `lp-project:<uid>` is a *local* single-writer guard: it
is what makes memory-primary write-behind correct for one project's
`/packages` and `/history` subtrees. The cloud sync driver needs that
guarantee too (it writes `/cloud-binding.json`), so `mount_for_sync`
takes the same lock — and then holds it for the whole trip, network
included: `run_one` mounts, awaits `run_mounted` (fetch, compare, upload,
record), and only then calls `SyncMount::release`
(`lpa-studio-web/src/cloud/sync/sync_engine.rs`). The lock's hold time is
therefore set by the round trip, not by the local writes it guards, and
on a slow link it is seconds.

Seeding an example publishes immediately — `SyncTrigger::Installed` has a
`delay_ms` of `0.0` — so the very first click on an example races its own
publish. The open's compensation, `await_sync_handoff`, waits up to 3 s
(30 × 100 ms) for this tab's own trip to end, and past that bound the
ordinary refusal takes over: a slow trip becomes "open in another tab"
about a project only this tab has ever touched.

The handoff wait had a hole of its own: `mount_for_sync` registered the
uid in the `syncing` set *after* awaiting the acquire, so an open polling
that set in between saw no trip in flight, skipped the wait entirely, and
was refused instantly by the lock the driver was about to take.

**Fix** — in two parts.

P1 closed the registration hole: the uid goes into `syncing` before the
acquire is awaited, and the drop guard un-registers it if the acquire is
refused.

P2 removed the hold itself, per the plan's D1. `mount_for_sync` now
acquires (polling, since the hold it waits out is local work),
mounts both subtrees — memory-primary mounting reads the whole subtree,
so the mount *is* the snapshot — and **releases before returning**. The
trip runs against that snapshot with no lock held; `SyncMount::finish`
banks what it wrote under a second short hold, or hands those writes to
the open's live store when the project was opened in this tab meanwhile
(the first click landing mid-publish — the ordinary case). With no long
holds left, `await_sync_handoff` dropped from 3 s to 500 ms; the open
path's ~500 ms acquire ladder (P1) absorbs what is left. The locking ADR
is amended to say the rule out loud.

**Regression coverage** —
`lpa-fs-opfs/tests/library_locks.rs::a_publishing_sync_trip_does_not_hold_the_project`
plays the trip's new shape and asserts an open-style acquire wins while
the publish is still in flight (and that an *instant* shot does too —
nothing is held). Its companion,
`a_snapshot_banks_only_what_the_trip_wrote`, pins the property that makes
publishing from a copy safe: the write-back lands only the paths the trip
dirtied, so a save that arrived mid-publish survives.
`sync_queue::work_arriving_mid_flight_earns_another_trip` (host) covers
the other half — a project that changed mid-publish earns its own trip.
The registration order is still not directly testable without a browser:
it is an ordering inside one `async fn`, asserted only by the sequence in
`hold_for_sync`.

**Lesson** — A lock's *name* says what it protects; only its hold says
what it costs. This one is documented as guarding local OPFS writes, and
every reasoning about it (including the "refusal doubles as the open-in-
another-tab answer" rule) assumed hold times in the tens of milliseconds
— then one caller held it across a network operation the guarded
invariant has nothing to do with, and the refusal became a lie the UI
repeated verbatim. When a lock's refusal is also a *user-facing claim*,
holding it across foreign latency does not just slow things down, it
makes the product state something false. Watch for it wherever a
guard-for-local-consistency is taken by a caller whose work is mostly
elsewhere: take the snapshot under the lock, do the elsewhere-work
outside it.
Loading
Loading