Skip to content

Fail fast with wallet_locked when the wallet is open in another tab#56

Merged
jamaljsr merged 4 commits into
mainfrom
pr/wallet-multi-tab-feasibility-6b8e9c
Jul 24, 2026
Merged

Fail fast with wallet_locked when the wallet is open in another tab#56
jamaljsr merged 4 commits into
mainfrom
pr/wallet-multi-tab-feasibility-6b8e9c

Conversation

@jamaljsr

@jamaljsr jamaljsr commented Jul 23, 2026

Copy link
Copy Markdown
Member

Summary

Opening a second tab of a Wavelength wallet used to crash it with a raw SQLite stack trace: the daemon's OPFS databases are exclusive to one runtime, and the second tab tripped straight over the handles the first one held (#48). This adds a fast, typed cross-tab guard so the second tab fails cleanly instead.

The consumer-facing surface:

  • Two new WavelengthErrorCodes: wallet_locked (another tab or window is already running the wallet for this origin) and runtime_lock_unavailable (the browser refused or dropped the lock request itself, which says nothing about other tabs).
  • On the web transports, start() takes a Web Lock before booting the daemon and releases it on stop() or runtime exit, so a second tab is detected before anything opens a database. It fails start() immediately with wallet_locked rather than booting into corruption.
  • The engine surfaces wallet_locked as an error phase, and the demo shows actionable copy ("close the other tab") with a retry instead of a wipe hatch. The React bindings need no changes.

Technical Notes

  • Lease-based lock. RuntimeLock is an explicit idle | acquiring | held | settling state machine. Each acquire() mints a lease, and a release only frees the grant when handed the lease that still owns it, so a superseded session's teardown can't free the lock a live session depends on.
  • Serialized lifecycle. start() and stop() run through a per-client queue, so a host's overlapping calls (a double-click, a stop issued mid-start) can't interleave: two starts can't share a lease, and a stop can't release the lock while a start is still opening the databases.
  • Origin-wide scope, not per-dataDir. A single origin-wide lock name. The daemon opens several independently-configured OPFS stores (the wallet DB under dataDir, the swap DB at its own swapDatabaseFileName, plus daemon-default paths), so keying by dataDir alone would let two tabs that differ only in dataDir but share, say, the default swap DB boot together and collide on it. One runtime per origin is the guarantee; true multi-profile concurrency is a leader-tab follow-up.
  • Classifier is a backstop, not the guarantee. For browsers without the Web Locks API, a narrow message classifier maps daemon contention failures to wallet_locked, gated to the start verb. On every browser that ships Web Locks the pre-check rejects the second tab first, so the classifier is never consulted there.
  • Why keep the Web Lock given the daemon fails closed (Fail closed when the wasm wallet database can't persist to OPFS wavelength#1046): the daemon-side require_persistent surfaces contention as a real error, but only after exhausting its busy-timeout retry budget (~30s), so relying on it alone would leave the second tab hanging. The Web Lock is the fast path; the daemon fail-closed is the correctness backstop. Complementary, and this PR doesn't depend on it.
  • Breaking: callFacade('start') / callFacade('stop') now throw. Lifecycle must go through the typed start()/stop(), which is where the lock is taken and released; the raw facade escape hatch rejects those verbs so they can't bypass it. Anyone driving lifecycle through callFacade must switch to the typed methods.
  • Known limitation. The lock is per client instance, so the intended model is one client per tab. A dispose-then-immediately-recreate in the same tab, or two concurrent main-thread clients in one realm, can briefly strand or falsely trip the lock; a module-level per-realm coordinator is the follow-up if that becomes a real use case.

Steps to Test

CI covers build, typecheck, and the unit suites. The end-to-end check is the hermetic Playwright smoke test, which CI does not run:

  1. Build the wasm runtime: WAVELENGTH_DIR="$(cd ../wavelength && pwd)" pnpm --filter web-wallet-demo run wasm:local
  2. pnpm --filter web-wallet-demo run build
  3. pnpm --filter web-wallet-demo run test - exercises the second-tab lock, stop-and-handoff, and a refused lock request showing a retry rather than the wipe hatch.

For a manual check, open the demo in two tabs: the second shows the "already open in another tab" message, and closing or stopping the first lets the second take over.

Related Issues & Pull Requests

Closes #48

Depends on:

@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

@jamaljsr
jamaljsr force-pushed the pr/wallet-multi-tab-feasibility-6b8e9c branch 2 times, most recently from 3456072 to 6d59849 Compare July 23, 2026 22:32
@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

jamaljsr added 3 commits July 24, 2026 11:30
A start attempt while another tab of the same origin already runs the
wallet is an expected condition, not a runtime fault. Give it a stable
machine-readable code so transports can tag it and consumers can branch
to actionable copy without string-matching SQLite lock messages.
The daemon's OPFS SQLite handles are exclusive per origin, so a second
tab that starts the runtime used to boot the full wasm stack only to die
deep inside migrations with a raw SQLITE_CANTOPEN trace (issue #48).

Guard the start verb with a held Web Lock instead: the first tab to
start the runtime takes the lock, and a second tab's start() now rejects
with the typed wallet_locked error before the daemon opens any OPFS
database. Config validation runs before the acquisition, and start()
rechecks disposal after it, so neither a config that can never reach the
daemon nor a client disposed mid-acquire takes the lock or boots a
runtime.

One rule governs the release: the lock is handed back only once the
daemon is provably down. A clean stop is the graceful proof and keeps
the worker for a fast restart, releasing through the core client's
afterDaemonStopped hook. Every other end proves it by force. The worker
transport terminates the worker, which takes its nested SQLite worker
with it and frees the OPFS handles, then releases; that single teardown
path (killWorker) covers a failed start, a fatal runtime exit, a worker
error, and disposal, and a retry gets a fresh worker rather than a dead
one. The main-thread transport cannot terminate a Go runtime running on
the page, so it decides by probing the runtime rather than classifying
error codes: if wavewalletdkCall never became callable the runtime never
opened a database and the lock is released, otherwise it asks the daemon
to stop and retains the lock unless that stop is acknowledged. It also
fails fast on a restart, since a main-thread runtime that exited cannot
boot another in the same page.

Terminal-state handling is defined once, in the engine's phase machine,
so the transports do not have to order their events. A runtime that dies
because a start failed announces both the failure and the stop, and which
lands first depends on transport timing; the machine lets an error phase
outrank the runtimeStopped that follows, so the actionable failure always
survives to the host. Rounds of narrower fixes had alternated the emit
order and kept breaking one case to serve the other; making the machine
authoritative retires that whole class. runtimeStopped fires on every
dead-runtime path, worker failed start included, so a host keying
liveness off it is never left thinking a killed runtime is still up, and
request() rejects once the worker is gone so a call posted to it cannot
hang forever.

Browsers without the Web Locks API skip the pre-check and get a one-time
warning through the host's log channel and the console; for that path,
and for races between two starting tabs, both clients also classify raw
locked-database failure messages onto the same wallet_locked code at the
worker fatal, worker RPC, and facade error boundaries. The classifier
matches cross-context contention specifically, so neither the daemon's
generic fail-closed wording nor the SQLite worker's own duplicate-open
guard sends a sole tab hunting for a window that does not exist, and a
storage failure that looks lock-shaped but does not classify is warned
so the daemon rewording its errors cannot silently disable the guidance.
Branch the error phase on the wallet_locked code: instead of the raw
runtime trace, the second tab gets a heading naming the condition, copy
telling the user to close the other tab (or stop its runtime) and press
Try again, and no wipe escape hatch, which cannot help there.

Extend the smoke test with the multi-tab flow: a second tab fails fast
with the friendly message while the first holds the runtime, then takes
the wallet over after the first tab closes, landing on the unlock screen
of the same wallet.
@jamaljsr
jamaljsr force-pushed the pr/wallet-multi-tab-feasibility-6b8e9c branch from 6d59849 to d0d17a0 Compare July 24, 2026 16:55
@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

The reference page's error-code table and signature literal are hand
maintained (gen:api-docs only writes the wallet.proto method data), so
both drifted when the codes were added. The phases guide tells consumers
to branch on err.code, which makes an undocumented code invisible to the
people meant to handle it.
@jamaljsr
jamaljsr force-pushed the pr/wallet-multi-tab-feasibility-6b8e9c branch from d0d17a0 to 2f8dcd4 Compare July 24, 2026 17:32
@jamaljsr
jamaljsr merged commit d3ee97a into main Jul 24, 2026
3 checks passed
@jamaljsr
jamaljsr deleted the pr/wallet-multi-tab-feasibility-6b8e9c branch July 24, 2026 18:05
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.

web: surface a clear 'wallet already open in another tab' error instead of raw SQLite migration trace

1 participant